From 3c64b0b9a736b8d6880638ba3463c9d093d304a4 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Mon, 19 Oct 2026 10:43:22 -0700 Subject: [PATCH 1/2] fix(cli): respect npm registry config in update check Fixes: https://github.com/microsoft/playwright-cli/issues/478 --- playwright-cli.js | 68 +++++++++++++++++++++++++++++++- tests/integration.spec.ts | 83 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 149 insertions(+), 2 deletions(-) diff --git a/playwright-cli.js b/playwright-cli.js index a3cc4ed5..044548e3 100755 --- a/playwright-cli.js +++ b/playwright-cli.js @@ -18,6 +18,7 @@ // @ts-check const fs = require('fs'); +const os = require('os'); const path = require('path'); const { program } = require('playwright-core/lib/tools/cli-client/program'); @@ -60,7 +61,8 @@ async function fetchLatestVersion() { const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), 1500); try { - const res = await fetch(`https://registry.npmjs.org/${packageJson.name}/latest`, { signal: controller.signal }); + const url = npmRegistry().replace(/\/?$/, '/') + packageJson.name.replace('/', '%2f') + '/latest'; + const res = await fetch(url, { signal: controller.signal }); if (!res.ok) return undefined; const json = await res.json(); @@ -73,6 +75,70 @@ async function fetchLatestVersion() { } } +// Resolves the registry npm would use for this package: the scoped registry wins +// over the default one, each looked up in env, project, user and global npmrc. +function npmRegistry() { + const env = npmEnvConfig(); + const configs = [env, ...npmrcFiles(env).map(readNpmrc)]; + const scope = packageJson.name.split('/')[0]; + for (const key of [`${scope}:registry`, 'registry']) { + for (const config of configs) { + if (config[key]) + return config[key]; + } + } + return 'https://registry.npmjs.org/'; +} + +function npmEnvConfig() { + /** @type {Record} */ + const config = {}; + for (const [name, value] of Object.entries(process.env)) { + if (value && /^npm_config_/i.test(name)) + config[name.substring('npm_config_'.length).toLowerCase()] = value; + } + return config; +} + +/** + * @param {Record} env + */ +function npmrcFiles(env) { + const nodeDir = path.dirname(process.execPath); + const globalPrefix = env.prefix || process.env.PREFIX || (process.platform === 'win32' ? nodeDir : path.dirname(nodeDir)); + return [ + path.join(npmProjectDir(), '.npmrc'), + env.userconfig || path.join(os.homedir(), '.npmrc'), + env.globalconfig || path.join(globalPrefix, 'etc', 'npmrc'), + ]; +} + +function npmProjectDir() { + for (let dir = process.cwd(); ; dir = path.dirname(dir)) { + if (fs.existsSync(path.join(dir, 'package.json')) || fs.existsSync(path.join(dir, 'node_modules'))) + return dir; + if (path.dirname(dir) === dir) + return process.cwd(); + } +} + +/** + * @param {string} file + */ +function readNpmrc(file) { + /** @type {Record} */ + const config = {}; + try { + for (const line of fs.readFileSync(file, 'utf8').split(/\r?\n/)) { + const match = line.match(/^\s*([^#;=\s][^=]*?)\s*=\s*(.*?)\s*$/); + if (match) + config[match[1]] = match[2].replace(/^(['"])(.*)\1$/, '$2').replace(/\$\{([^}]+)\}/g, (_, name) => process.env[name] || ''); + } + } catch { + } + return config; +} + /** * * @param {string} current diff --git a/tests/integration.spec.ts b/tests/integration.spec.ts index 9e898a26..0fe6a49d 100644 --- a/tests/integration.spec.ts +++ b/tests/integration.spec.ts @@ -15,9 +15,11 @@ */ import fs from 'fs'; +import http from 'http'; import path from 'path'; import { spawn } from 'child_process'; -import { test, expect } from '@playwright/test'; +import { test as baseTest, expect } from '@playwright/test'; +import type { AddressInfo } from 'net'; type CliResult = { output: string; @@ -25,6 +27,41 @@ type CliResult = { exitCode: number | null; }; +type NpmRegistry = { + url: string; + status: number; + requests: string[]; +}; + +const test = baseTest.extend<{ npmRegistry: NpmRegistry }>({ + npmRegistry: async ({}, use) => { + const registry: NpmRegistry = { url: '', status: 200, requests: [] }; + const server = http.createServer((req, res) => { + registry.requests.push(req.url!); + res.statusCode = registry.status; + res.setHeader('content-type', 'application/json'); + res.end(JSON.stringify({ version: '99.0.0' })); + }); + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + registry.url = `http://127.0.0.1:${(server.address() as AddressInfo).port}`; + await use(registry); + await new Promise(resolve => server.close(resolve)); + }, +}); + +// Enables the update check and isolates it from the npm config of the machine running the tests. +function updateCheckEnv(env: Record = {}): Record { + return { + 'CI': '', + 'NO_UPDATE_NOTIFIER': '', + 'npm_config_registry': '', + 'npm_config_@playwright:registry': '', + 'npm_config_userconfig': test.info().outputPath('user-npmrc'), + 'npm_config_globalconfig': test.info().outputPath('global-npmrc'), + ...env, + }; +} + async function runCli(args: string[], env: Record = {}): Promise { const cliPath = path.join(__dirname, '../playwright-cli.js'); @@ -136,3 +173,47 @@ test('caches the update check in the default registry directory', async ({}) => expect(found).toHaveLength(1); expect(JSON.parse(fs.readFileSync(found[0], 'utf8')).lastCheck).toEqual(expect.any(Number)); }); + +test('update check uses the registry from npm config env', async ({ npmRegistry }) => { + expect(await runCli(['--version'], updateCheckEnv({ npm_config_registry: npmRegistry.url }))).toEqual(expect.objectContaining({ + error: expect.stringContaining('Update available for @playwright/cli'), + exitCode: 0, + })); + expect(npmRegistry.requests).toEqual(['/@playwright%2fcli/latest']); +}); + +test('update check prefers the scoped registry from project .npmrc', async ({ npmRegistry }) => { + fs.writeFileSync(test.info().outputPath('package.json'), '{}'); + fs.writeFileSync(test.info().outputPath('.npmrc'), [ + `registry=${npmRegistry.url}/default/`, + `@playwright:registry=${npmRegistry.url}/scoped/`, + ].join('\n')); + fs.writeFileSync(test.info().outputPath('user-npmrc'), `registry=${npmRegistry.url}/user/`); + + expect(await runCli(['--version'], updateCheckEnv())).toEqual(expect.objectContaining({ + error: expect.stringContaining('Update available for @playwright/cli'), + exitCode: 0, + })); + expect(npmRegistry.requests).toEqual(['/scoped/@playwright%2fcli/latest']); +}); + +test('update check uses the registry from user .npmrc', async ({ npmRegistry }) => { + fs.writeFileSync(test.info().outputPath('user-npmrc'), `registry=${npmRegistry.url}/user/`); + + expect(await runCli(['--version'], updateCheckEnv())).toEqual(expect.objectContaining({ + error: expect.stringContaining('Update available for @playwright/cli'), + exitCode: 0, + })); + expect(npmRegistry.requests).toEqual(['/user/@playwright%2fcli/latest']); +}); + +test('update check failure in the configured registry is silent', async ({ npmRegistry }) => { + npmRegistry.status = 401; + const packageJson = JSON.parse(fs.readFileSync(path.join(__dirname, '../package.json'), 'utf8')); + expect(await runCli(['--version'], updateCheckEnv({ npm_config_registry: npmRegistry.url }))).toEqual({ + output: packageJson.version, + error: '', + exitCode: 0, + }); + expect(npmRegistry.requests).toEqual(['/@playwright%2fcli/latest']); +}); From a7234dbdd3d73573eb0bf168ed5555f67bd164d7 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Mon, 19 Oct 2026 11:13:22 -0700 Subject: [PATCH 2/2] fix(cli): resolve latest version via npm view on setup commands --- playwright-cli.js | 120 ++++++++++++-------------------------- tests/integration.spec.ts | 104 ++++++++++++++++++++------------- 2 files changed, 102 insertions(+), 122 deletions(-) diff --git a/playwright-cli.js b/playwright-cli.js index 044548e3..b3bdb4ea 100755 --- a/playwright-cli.js +++ b/playwright-cli.js @@ -18,8 +18,8 @@ // @ts-check const fs = require('fs'); -const os = require('os'); const path = require('path'); +const { spawn } = require('child_process'); const { program } = require('playwright-core/lib/tools/cli-client/program'); const coreBundle = require('playwright-core/lib/coreBundle'); @@ -29,6 +29,7 @@ const { checkInstalledSkills, frame } = require('./skillCheck'); const packageJson = require('./package.json'); const ONE_DAY_MS = 24 * 60 * 60 * 1000; +const UPDATE_CHECK_COMMANDS = new Set(['open', 'attach', 'list', 'install', 'install-browser']); main(); @@ -41,13 +42,17 @@ async function checkForUpdates() { if (process.env.NO_UPDATE_NOTIFIER || process.env.CI) return; + // The check spawns npm, so keep its cost to the commands that start a workflow. + const command = process.argv.slice(2).find(arg => !arg.startsWith('-')); + if (!command || !UPDATE_CHECK_COMMANDS.has(command)) + return; + const cache = readCache(); const stale = !cache || (Date.now() - cache.lastCheck) > ONE_DAY_MS; if (!stale) return; writeCache({ lastCheck: Date.now() }); - const command = process.argv.slice(2).find(arg => !arg.startsWith('-')); if (command !== 'install') checkInstalledSkills(); @@ -56,87 +61,38 @@ async function checkForUpdates() { printNotice(packageJson.version, latest); } -async function fetchLatestVersion() { - try { - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), 1500); - try { - const url = npmRegistry().replace(/\/?$/, '/') + packageJson.name.replace('/', '%2f') + '/latest'; - const res = await fetch(url, { signal: controller.signal }); - if (!res.ok) - return undefined; - const json = await res.json(); - return typeof json.version === 'string' ? json.version : undefined; - } finally { +// Asks npm, so that the user's registry, proxy and auth configuration is respected. +/** @returns {Promise} */ +function fetchLatestVersion() { + return new Promise(resolve => { + const child = spawn(`npm view ${packageJson.name} version --json --no-update-notifier`, { + shell: true, + stdio: ['ignore', 'pipe', 'ignore'], + }); + let stdout = ''; + child.stdout.on('data', data => stdout += data); + // On Windows, killing the shell leaves npm running: stop listening to it + // so that the CLI does not wait for npm before exiting. + const timeout = setTimeout(() => { + child.kill(); + child.unref(); + child.stdout.destroy(); + resolve(undefined); + }, 1500); + child.on('error', () => { clearTimeout(timeout); - } - } catch { - return undefined; - } -} - -// Resolves the registry npm would use for this package: the scoped registry wins -// over the default one, each looked up in env, project, user and global npmrc. -function npmRegistry() { - const env = npmEnvConfig(); - const configs = [env, ...npmrcFiles(env).map(readNpmrc)]; - const scope = packageJson.name.split('/')[0]; - for (const key of [`${scope}:registry`, 'registry']) { - for (const config of configs) { - if (config[key]) - return config[key]; - } - } - return 'https://registry.npmjs.org/'; -} - -function npmEnvConfig() { - /** @type {Record} */ - const config = {}; - for (const [name, value] of Object.entries(process.env)) { - if (value && /^npm_config_/i.test(name)) - config[name.substring('npm_config_'.length).toLowerCase()] = value; - } - return config; -} - -/** - * @param {Record} env - */ -function npmrcFiles(env) { - const nodeDir = path.dirname(process.execPath); - const globalPrefix = env.prefix || process.env.PREFIX || (process.platform === 'win32' ? nodeDir : path.dirname(nodeDir)); - return [ - path.join(npmProjectDir(), '.npmrc'), - env.userconfig || path.join(os.homedir(), '.npmrc'), - env.globalconfig || path.join(globalPrefix, 'etc', 'npmrc'), - ]; -} - -function npmProjectDir() { - for (let dir = process.cwd(); ; dir = path.dirname(dir)) { - if (fs.existsSync(path.join(dir, 'package.json')) || fs.existsSync(path.join(dir, 'node_modules'))) - return dir; - if (path.dirname(dir) === dir) - return process.cwd(); - } -} - -/** - * @param {string} file - */ -function readNpmrc(file) { - /** @type {Record} */ - const config = {}; - try { - for (const line of fs.readFileSync(file, 'utf8').split(/\r?\n/)) { - const match = line.match(/^\s*([^#;=\s][^=]*?)\s*=\s*(.*?)\s*$/); - if (match) - config[match[1]] = match[2].replace(/^(['"])(.*)\1$/, '$2').replace(/\$\{([^}]+)\}/g, (_, name) => process.env[name] || ''); - } - } catch { - } - return config; + resolve(undefined); + }); + child.on('close', code => { + clearTimeout(timeout); + try { + const version = JSON.parse(stdout); + resolve(code === 0 && typeof version === 'string' ? version : undefined); + } catch { + resolve(undefined); + } + }); + }); } /** diff --git a/tests/integration.spec.ts b/tests/integration.spec.ts index 0fe6a49d..db73783a 100644 --- a/tests/integration.spec.ts +++ b/tests/integration.spec.ts @@ -30,35 +30,44 @@ type CliResult = { type NpmRegistry = { url: string; status: number; + hang: boolean; requests: string[]; }; const test = baseTest.extend<{ npmRegistry: NpmRegistry }>({ npmRegistry: async ({}, use) => { - const registry: NpmRegistry = { url: '', status: 200, requests: [] }; + const registry: NpmRegistry = { url: '', status: 200, hang: false, requests: [] }; const server = http.createServer((req, res) => { registry.requests.push(req.url!); + if (registry.hang) + return; res.statusCode = registry.status; res.setHeader('content-type', 'application/json'); - res.end(JSON.stringify({ version: '99.0.0' })); + res.end(JSON.stringify({ + 'name': '@playwright/cli', + 'dist-tags': { latest: '99.0.0' }, + 'versions': { '99.0.0': { name: '@playwright/cli', version: '99.0.0' } }, + })); }); await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); registry.url = `http://127.0.0.1:${(server.address() as AddressInfo).port}`; await use(registry); + server.closeAllConnections(); await new Promise(resolve => server.close(resolve)); }, }); -// Enables the update check and isolates it from the npm config of the machine running the tests. -function updateCheckEnv(env: Record = {}): Record { +// Enables the update check and points npm at the test registry, isolated from +// the npm config of the machine running the tests. +function updateCheckEnv(npmRegistry: NpmRegistry): Record { return { 'CI': '', 'NO_UPDATE_NOTIFIER': '', - 'npm_config_registry': '', + 'npm_config_registry': npmRegistry.url, 'npm_config_@playwright:registry': '', 'npm_config_userconfig': test.info().outputPath('user-npmrc'), 'npm_config_globalconfig': test.info().outputPath('global-npmrc'), - ...env, + 'npm_config_cache': test.info().outputPath('npm-cache'), }; } @@ -110,7 +119,7 @@ test('open data URL', async ({}) => { })); }); -test('warns when installed skill is out of date', async ({}) => { +test('warns when installed skill is out of date', async ({ npmRegistry }) => { expect(await runCli(['install', '--skills'], { NO_UPDATE_NOTIFIER: '1' })).toEqual(expect.objectContaining({ exitCode: 0, })); @@ -118,17 +127,17 @@ test('warns when installed skill is out of date', async ({}) => { const skillFile = path.join(test.info().outputPath(), '.claude', 'skills', 'playwright-cli', 'SKILL.md'); fs.appendFileSync(skillFile, 'x'); - const env = { CI: '', NO_UPDATE_NOTIFIER: '' }; - expect(await runCli(['--help'], env)).toEqual(expect.objectContaining({ + const env = updateCheckEnv(npmRegistry); + expect(await runCli(['list'], env)).toEqual(expect.objectContaining({ error: expect.stringContaining('does not match the tool version'), })); - expect(await runCli(['--help'], env)).toEqual(expect.objectContaining({ + expect(await runCli(['list'], env)).toEqual(expect.objectContaining({ error: expect.not.stringContaining('does not match the tool version'), })); }); -test('does not warn when installed skill only differs in line endings', async ({}) => { +test('does not warn when installed skill only differs in line endings', async ({ npmRegistry }) => { expect(await runCli(['install', '--skills'], { NO_UPDATE_NOTIFIER: '1' })).toEqual(expect.objectContaining({ exitCode: 0, })); @@ -136,19 +145,18 @@ test('does not warn when installed skill only differs in line endings', async ({ const skillFile = path.join(test.info().outputPath(), '.claude', 'skills', 'playwright-cli', 'SKILL.md'); fs.writeFileSync(skillFile, fs.readFileSync(skillFile, 'utf8').replace(/\n/g, '\r\n')); - expect(await runCli(['--help'], { CI: '', NO_UPDATE_NOTIFIER: '' })).toEqual(expect.objectContaining({ + expect(await runCli(['list'], updateCheckEnv(npmRegistry))).toEqual(expect.objectContaining({ error: expect.not.stringContaining('does not match the tool version'), })); }); -test('caches the update check in the default registry directory', async ({}) => { +test('caches the update check in the default registry directory', async ({ npmRegistry }) => { // Redirect the home/cache directories so the real user cache is untouched, and // leave PLAYWRIGHT_CLI_INSTALLATION_FOR_TEST empty so the default path is used. const home = test.info().outputPath('home'); fs.mkdirSync(home, { recursive: true }); const env = { - CI: '', - NO_UPDATE_NOTIFIER: '', + ...updateCheckEnv(npmRegistry), PLAYWRIGHT_CLI_INSTALLATION_FOR_TEST: '', HOME: home, USERPROFILE: home, @@ -156,7 +164,7 @@ test('caches the update check in the default registry directory', async ({}) => LOCALAPPDATA: path.join(home, 'AppData', 'Local'), }; - expect(await runCli(['--version'], env)).toEqual(expect.objectContaining({ exitCode: 0 })); + expect(await runCli(['list'], env)).toEqual(expect.objectContaining({ exitCode: 0 })); const found: string[] = []; const walk = (dir: string) => { @@ -174,46 +182,62 @@ test('caches the update check in the default registry directory', async ({}) => expect(JSON.parse(fs.readFileSync(found[0], 'utf8')).lastCheck).toEqual(expect.any(Number)); }); -test('update check uses the registry from npm config env', async ({ npmRegistry }) => { - expect(await runCli(['--version'], updateCheckEnv({ npm_config_registry: npmRegistry.url }))).toEqual(expect.objectContaining({ +test('update check asks npm for the latest version on setup commands only', async ({ npmRegistry }) => { + const env = updateCheckEnv(npmRegistry); + expect(await runCli(['--version'], env)).toEqual(expect.objectContaining({ + error: '', + exitCode: 0, + })); + expect(npmRegistry.requests).toEqual([]); + + expect(await runCli(['list'], env)).toEqual(expect.objectContaining({ error: expect.stringContaining('Update available for @playwright/cli'), exitCode: 0, })); - expect(npmRegistry.requests).toEqual(['/@playwright%2fcli/latest']); + expect(npmRegistry.requests).toEqual(['/@playwright%2fcli']); }); -test('update check prefers the scoped registry from project .npmrc', async ({ npmRegistry }) => { - fs.writeFileSync(test.info().outputPath('package.json'), '{}'); - fs.writeFileSync(test.info().outputPath('.npmrc'), [ - `registry=${npmRegistry.url}/default/`, - `@playwright:registry=${npmRegistry.url}/scoped/`, - ].join('\n')); - fs.writeFileSync(test.info().outputPath('user-npmrc'), `registry=${npmRegistry.url}/user/`); - - expect(await runCli(['--version'], updateCheckEnv())).toEqual(expect.objectContaining({ - error: expect.stringContaining('Update available for @playwright/cli'), +test('update check on install does not warn about the skills being installed', async ({ npmRegistry }) => { + expect(await runCli(['install', '--skills'], { NO_UPDATE_NOTIFIER: '1' })).toEqual(expect.objectContaining({ exitCode: 0, })); - expect(npmRegistry.requests).toEqual(['/scoped/@playwright%2fcli/latest']); + + const skillFile = path.join(test.info().outputPath(), '.claude', 'skills', 'playwright-cli', 'SKILL.md'); + fs.appendFileSync(skillFile, 'x'); + + const result = await runCli(['install', '--skills'], updateCheckEnv(npmRegistry)); + expect(result.exitCode).toBe(0); + expect(result.error).toContain('Update available for @playwright/cli'); + expect(result.error).not.toContain('does not match the tool version'); + expect(npmRegistry.requests).toEqual(['/@playwright%2fcli']); }); -test('update check uses the registry from user .npmrc', async ({ npmRegistry }) => { - fs.writeFileSync(test.info().outputPath('user-npmrc'), `registry=${npmRegistry.url}/user/`); +test('update check uses the scoped registry from project .npmrc', async ({ npmRegistry }) => { + fs.writeFileSync(test.info().outputPath('package.json'), '{}'); + fs.writeFileSync(test.info().outputPath('.npmrc'), `@playwright:registry=${npmRegistry.url}/scoped/`); - expect(await runCli(['--version'], updateCheckEnv())).toEqual(expect.objectContaining({ + expect(await runCli(['list'], updateCheckEnv(npmRegistry))).toEqual(expect.objectContaining({ error: expect.stringContaining('Update available for @playwright/cli'), exitCode: 0, })); - expect(npmRegistry.requests).toEqual(['/user/@playwright%2fcli/latest']); + expect(npmRegistry.requests).toEqual(['/scoped/@playwright%2fcli']); }); -test('update check failure in the configured registry is silent', async ({ npmRegistry }) => { +test('update check failure is silent', async ({ npmRegistry }) => { npmRegistry.status = 401; - const packageJson = JSON.parse(fs.readFileSync(path.join(__dirname, '../package.json'), 'utf8')); - expect(await runCli(['--version'], updateCheckEnv({ npm_config_registry: npmRegistry.url }))).toEqual({ - output: packageJson.version, + expect(await runCli(['list'], updateCheckEnv(npmRegistry))).toEqual(expect.objectContaining({ error: '', exitCode: 0, - }); - expect(npmRegistry.requests).toEqual(['/@playwright%2fcli/latest']); + })); + expect(npmRegistry.requests).toEqual(['/@playwright%2fcli']); +}); + +test('update check does not wait for a hanging registry', async ({ npmRegistry }) => { + npmRegistry.hang = true; + const start = Date.now(); + expect(await runCli(['list'], updateCheckEnv(npmRegistry))).toEqual(expect.objectContaining({ + error: '', + exitCode: 0, + })); + expect(Date.now() - start).toBeLessThan(10000); });