From 9231c221c9ec0e8fe5fcf3b4aa31abfda21b8cd4 Mon Sep 17 00:00:00 2001 From: TURKI Date: Tue, 20 Oct 2026 20:29:47 +0300 Subject: [PATCH 1/4] fix(cli): resolve the update check command like program() does The update check picked the command with `argv.find(arg => !arg.startsWith('-'))`, which returns the value of a preceding option rather than the command. For the documented `playwright-cli --config path/to/config.json open example.com`, that is `path/to/config.json`, so neither the version notice nor the stale skill warning ever ran. Parse argv with the same minimist options program() uses instead. Those files are internal to playwright-core and not in its "exports" map, so resolve them by path and fall back to the old heuristic if they ever move, keeping the check best-effort rather than letting it break every command. --- playwright-cli.js | 23 ++++++++++++++++++++++- tests/integration.spec.ts | 9 +++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/playwright-cli.js b/playwright-cli.js index b3bdb4e..426395b 100755 --- a/playwright-cli.js +++ b/playwright-cli.js @@ -43,7 +43,7 @@ async function checkForUpdates() { 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('-')); + const command = currentCommand(); if (!command || !UPDATE_CHECK_COMMANDS.has(command)) return; @@ -61,6 +61,26 @@ async function checkForUpdates() { printNotice(packageJson.version, latest); } +// Parses argv the same way program() does, so that an option value cannot be +// mistaken for the command: in `--config file.json open`, minimist consumes +// `file.json` as the value of `--config` and the command is still `open`. +// These files are internal to playwright-core and not in its "exports" map, so +// resolve them by path and fall back to the first positional-looking argument. +/** @returns {string | undefined} */ +function currentCommand() { + const argv = process.argv.slice(2); + try { + const coreDir = path.dirname(require.resolve('playwright-core/package.json')); + const cliClientDir = path.join(coreDir, 'lib', 'tools', 'cli-client'); + const { minimist } = require(path.join(cliClientDir, 'minimist.js')); + const help = require(path.join(cliClientDir, 'help.json')); + const boolean = [...help.booleanOptions, 'all', 'g', 'help', 'json', 'raw', 'version']; + return minimist(argv, { boolean, string: ['_'] })._?.[0]; + } catch { + return argv.find(arg => !arg.startsWith('-')); + } +} + // Asks npm, so that the user's registry, proxy and auth configuration is respected. /** @returns {Promise} */ function fetchLatestVersion() { @@ -68,6 +88,7 @@ function fetchLatestVersion() { const child = spawn(`npm view ${packageJson.name} version --json --no-update-notifier`, { shell: true, stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, }); let stdout = ''; child.stdout.on('data', data => stdout += data); diff --git a/tests/integration.spec.ts b/tests/integration.spec.ts index db73783..e98f0a5 100644 --- a/tests/integration.spec.ts +++ b/tests/integration.spec.ts @@ -212,6 +212,15 @@ test('update check on install does not warn about the skills being installed', a expect(npmRegistry.requests).toEqual(['/@playwright%2fcli']); }); +test('update check runs when a global option precedes the command', async ({ npmRegistry }) => { + // `--session` takes a value, so the command is the token after it, not `work`. + expect(await runCli(['--session', 'work', 'list'], updateCheckEnv(npmRegistry))).toEqual(expect.objectContaining({ + error: expect.stringContaining('Update available for @playwright/cli'), + exitCode: 0, + })); + expect(npmRegistry.requests).toEqual(['/@playwright%2fcli']); +}); + 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/`); From e48453cb1a80b7095bea6663a9203b039423beb2 Mon Sep 17 00:00:00 2001 From: TURKI Date: Tue, 20 Oct 2026 20:29:59 +0300 Subject: [PATCH 2/4] fix(cli): compare the whole installed skill, not just SKILL.md The skill ships SKILL.md plus ten reference guides, but the staleness check only compared SKILL.md, so an out-of-date file under references/ was never reported even though it misleads the agent just as much. Walk the bundled skill directory and compare every markdown file; a file missing from the installation now reads as out of date too. --- skillCheck.js | 43 ++++++++++++++++++++++++++++++++++----- tests/integration.spec.ts | 14 +++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/skillCheck.js b/skillCheck.js index df986df..c925ef5 100644 --- a/skillCheck.js +++ b/skillCheck.js @@ -41,11 +41,38 @@ function readSkill(file) { return fs.existsSync(file) ? fs.readFileSync(file, 'utf8').replace(/\r\n/g, '\n') : null; } +/** + * Lists the markdown files of a skill directory, relative to it. + * @param {string} dir + * @returns {string[]} + */ +function skillFiles(dir) { + /** @type {string[]} */ + const files = []; + /** + * @param {string} current + * @param {string} prefix + */ + const walk = (current, prefix) => { + for (const entry of fs.readdirSync(current, { withFileTypes: true })) { + const relativePath = prefix ? path.join(prefix, entry.name) : entry.name; + if (entry.isDirectory()) + walk(path.join(current, entry.name), relativePath); + else if (entry.name.endsWith('.md')) + files.push(relativePath); + } + }; + walk(dir, ''); + return files; +} + /** * @param {string[]} lines * @returns {string} */ function frame(lines) { + if (!lines.length) + return ''; const width = Math.max(...lines.map(line => line.length)); const top = '╔' + '═'.repeat(width + 2) + '╗'; const bottom = '╚' + '═'.repeat(width + 2) + '╝'; @@ -55,14 +82,20 @@ function frame(lines) { function checkInstalledSkills() { try { - const bundled = readSkill(bundledSkillFile()); - if (!bundled) + const bundledFile = bundledSkillFile(); + if (!readSkill(bundledFile)) return; + const bundledDir = path.dirname(bundledFile); + // The skill is SKILL.md plus its reference guides, so compare them all: a + // stale reference file misleads the agent just as much as a stale SKILL.md. + const files = skillFiles(bundledDir); for (const target of installedSkillTargets()) { - const installed = readSkill(path.join(target.dir, 'SKILL.md')); - if (installed === null) + if (readSkill(path.join(target.dir, 'SKILL.md')) === null) continue; - if (installed !== bundled) { + // A missing file reads as null and therefore counts as out of date. + const outOfDate = files.some(file => + readSkill(path.join(target.dir, file)) !== readSkill(path.join(bundledDir, file))); + if (outOfDate) { process.stderr.write(frame([ `The playwright-cli skill at '${path.relative(process.cwd(), target.dir)}'`, `does not match the tool version.`, diff --git a/tests/integration.spec.ts b/tests/integration.spec.ts index e98f0a5..ddb2624 100644 --- a/tests/integration.spec.ts +++ b/tests/integration.spec.ts @@ -221,6 +221,20 @@ test('update check runs when a global option precedes the command', async ({ npm expect(npmRegistry.requests).toEqual(['/@playwright%2fcli']); }); +test('update check warns when an installed reference file is out of date', async ({ npmRegistry }) => { + expect(await runCli(['install', '--skills'], { NO_UPDATE_NOTIFIER: '1' })).toEqual(expect.objectContaining({ + exitCode: 0, + })); + + const referencesDir = path.join(test.info().outputPath(), '.claude', 'skills', 'playwright-cli', 'references'); + const reference = fs.readdirSync(referencesDir).find(name => name.endsWith('.md'))!; + fs.appendFileSync(path.join(referencesDir, reference), 'x'); + + expect(await runCli(['list'], updateCheckEnv(npmRegistry))).toEqual(expect.objectContaining({ + error: expect.stringContaining('does not match the tool version'), + })); +}); + 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/`); From d21dd2d53b9d3e06c56abbc0f2697f0c920c4aaf Mon Sep 17 00:00:00 2001 From: TURKI Date: Tue, 20 Oct 2026 20:30:22 +0300 Subject: [PATCH 3/4] fix(scripts): do not delete the skills folder before the copy succeeds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit update.js removed skills/playwright-cli and then copied the generated skill over it, with the whole block wrapped in a catch that reported "Generated skills directory not found" and exited 0. Any failure in fs.cp therefore left the committed skills deleted, blamed a cause that was not the real one, and still looked successful — during the documented roll flow. Stage the copy next to the target and swap it in, restoring the previous folder if the swap fails, and let real errors surface with a non-zero exit. --- scripts/update.js | 66 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 53 insertions(+), 13 deletions(-) diff --git a/scripts/update.js b/scripts/update.js index 1bdb054..af36e33 100644 --- a/scripts/update.js +++ b/scripts/update.js @@ -1,3 +1,19 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + const { execSync } = require('child_process'); const fs = require('fs/promises'); const path = require('path'); @@ -10,29 +26,53 @@ function run(command, options = {}) { execSync(command, { stdio: 'inherit', cwd: rootDir, ...options }); } +async function exists(file) { + try { + await fs.access(file); + return true; + } catch { + return false; + } +} + async function main() { - // 2. Run playwright-cli install-skills + // 1. Regenerate the skills from the installed Playwright version. console.log('\n=== Running playwright-cli install --skills ===\n'); run('node playwright-cli.js install --skills'); - // 3. Move generated skills into the existing skills folder + // 2. Move generated skills into the existing skills folder console.log('\n=== Updating skills folder ===\n'); const generatedSkillsDir = path.join(rootDir, '.claude', 'skills', 'playwright-cli'); const targetSkillsDir = path.join(skillsDir, 'playwright-cli'); + if (!await exists(generatedSkillsDir)) + throw new Error(`Generated skills directory not found at ${generatedSkillsDir}`); + + // Stage the copy next to the target and swap it in, so that a failure midway + // cannot leave the committed skills folder deleted. + const stagingDir = `${targetSkillsDir}.new`; + await fs.rm(stagingDir, { recursive: true, force: true }); + await fs.cp(generatedSkillsDir, stagingDir, { recursive: true }); + + const backupDir = `${targetSkillsDir}.old`; + await fs.rm(backupDir, { recursive: true, force: true }); + if (await exists(targetSkillsDir)) + await fs.rename(targetSkillsDir, backupDir); try { - await fs.access(generatedSkillsDir); - // Remove existing skills and copy new ones - await fs.rm(targetSkillsDir, { recursive: true, force: true }); - await fs.cp(generatedSkillsDir, targetSkillsDir, { recursive: true }); - console.log(`Copied skills from ${generatedSkillsDir} to ${targetSkillsDir}`); - - // Clean up generated skills directory - await fs.rm(generatedSkillsDir, { recursive: true }); - console.log('Cleaned up generated skills directory'); - } catch { - console.warn('Warning: Generated skills directory not found at', generatedSkillsDir); + await fs.rename(stagingDir, targetSkillsDir); + } catch (error) { + // Put the previous skills back rather than leaving nothing behind. + if (await exists(backupDir)) + await fs.rename(backupDir, targetSkillsDir); + throw error; } + await fs.rm(backupDir, { recursive: true, force: true }); + console.log(`Copied skills from ${generatedSkillsDir} to ${targetSkillsDir}`); + + // 3. Clean up generated skills directory + await fs.rm(generatedSkillsDir, { recursive: true }); + console.log('Cleaned up generated skills directory'); + console.log('\n=== Update complete! ===\n'); } From 647510dcc332bc801a870c03fe9751328ea0c76e Mon Sep 17 00:00:00 2001 From: TURKI Date: Tue, 20 Oct 2026 20:30:22 +0300 Subject: [PATCH 4/4] chore: ignore the .playwright config directory `install` writes .playwright/cli.config.json, so running scripts/update.js during a roll leaves an untracked directory behind. --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index 68a3eb1..49f0d62 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,5 @@ node_modules/ .npmrc # Playwright CLI output (may contain credentials) .playwright-cli/ +# Default config written by `install`, regenerated by scripts/update.js +/.playwright/