From 24e8fd040949564e2c07fccfe6d243e386815b6e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 15:17:12 +0000 Subject: [PATCH 1/5] fix(scripts): make check:playwright-browser-revision verify real binary presence (#312) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The check could report ok:true even when the pinned Chromium revision had no launchable binary anywhere on disk. The "no root forced" branch skipped the disk entirely and unconditionally passed; the designated-container branch only compared directory names, so a same-named empty directory also passed. Both are now real, disk-verified checks against Playwright's own executable layout, reproduced live against this session's own container (PLAYWRIGHT_BROWSERS_PATH=/opt/pw-browsers with only chromium-1194 present while playwright-core@1.62.1 pins 1234 — the old check reported OK for that exact state). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01QLbw9qpfjv5CeNz6XpmteN --- scripts/check-playwright-browser-revision.mjs | 187 +++++++++++++-- .../check-playwright-browser-revision.test.ts | 216 +++++++++++++++++- 2 files changed, 377 insertions(+), 26 deletions(-) diff --git a/scripts/check-playwright-browser-revision.mjs b/scripts/check-playwright-browser-revision.mjs index 5dda976d24..b918742f9e 100644 --- a/scripts/check-playwright-browser-revision.mjs +++ b/scripts/check-playwright-browser-revision.mjs @@ -1,7 +1,23 @@ #!/usr/bin/env node /** - * Fail closed when a designated container browser root cannot satisfy the - * Playwright revision pinned by the installed playwright-core package (#255). + * Fail closed unless the pinned Chromium revision (from the installed + * playwright-core package) has a launchable binary actually present on disk — + * not merely a same-named directory, and not merely "no path is forced" (#312). + * + * Earlier versions of this check only inspected directory *names* under a + * forced `/opt/pw-browsers` container root, and skipped the disk entirely + * whenever no such root was forced — silently reporting `ok: true` even when + * the managed cache (or an explicit `PLAYWRIGHT_BROWSERS_PATH`) had no + * matching Chromium binary at all. That produced a false "OK" that was read + * as a green light for `verify:ui` before two Playwright runs died at + * preflight (docs/outstanding-issues.md #312, session 2026-08-12). + * + * This check now always resolves the effective browsers root — an explicit + * `PLAYWRIGHT_BROWSERS_PATH` override if set, otherwise Playwright's own + * default managed-cache directory — and verifies a real executable exists + * for the pinned revision inside it, using the same executable layout table + * Playwright itself ships (mirrored below; keep in sync with + * `playwright-core`'s `EXECUTABLE_PATHS`). * * Local managed caches (`~/.cache/ms-playwright`) are fine. The trap is * PLAYWRIGHT_BROWSERS_PATH=/opt/pw-browsers with PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD @@ -9,11 +25,46 @@ * pointing PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH at the stale shell is forbidden. */ import { existsSync, readFileSync, readdirSync } from "node:fs"; +import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; const DEFAULT_CONTAINER_ROOT = "/opt/pw-browsers"; +// Mirrors playwright-core's `EXECUTABLE_PATHS.chromium` (full Chrome for +// Testing, installed under a `chromium-` directory). +const CHROMIUM_EXECUTABLE_LAYOUTS = Object.freeze({ + linux: { + x64: [["chrome-linux64", "chrome"]], + arm64: [["chrome-linux", "chrome"]], + }, + darwin: { + x64: [["chrome-mac-x64", "Google Chrome for Testing.app", "Contents", "MacOS", "Google Chrome for Testing"]], + arm64: [["chrome-mac-arm64", "Google Chrome for Testing.app", "Contents", "MacOS", "Google Chrome for Testing"]], + }, + win32: [["chrome-win64", "chrome.exe"]], +}); + +// Mirrors playwright-core's `EXECUTABLE_PATHS["chromium-headless-shell"]` +// (installed under a `chromium_headless_shell-` directory — the +// binary the default headless chromium/chromium-mockups projects launch). +const CHROMIUM_HEADLESS_SHELL_EXECUTABLE_LAYOUTS = Object.freeze({ + linux: { + x64: [["chrome-headless-shell-linux64", "chrome-headless-shell"]], + arm64: [["chrome-linux", "headless_shell"]], + }, + darwin: { + x64: [["chrome-headless-shell-mac-x64", "chrome-headless-shell"]], + arm64: [["chrome-headless-shell-mac-arm64", "chrome-headless-shell"]], + }, + win32: [["chrome-headless-shell-win64", "chrome-headless-shell.exe"]], +}); + +function layoutsForPlatform(table, platform, architecture) { + if (platform === "linux" || platform === "darwin") return table[platform]?.[architecture] ?? []; + return table[platform] ?? []; +} + export function readExpectedChromiumRevision(projectRoot = process.cwd()) { const browsersJsonPath = path.join(projectRoot, "node_modules", "playwright-core", "browsers.json"); if (!existsSync(browsersJsonPath)) { @@ -43,17 +94,81 @@ export function listInstalledChromiumRevisions(browsersRoot) { return [...revisions].sort(); } +/** + * Playwright's own default managed-cache directory when no + * `PLAYWRIGHT_BROWSERS_PATH` override is set — mirrors + * `playwright-core`'s `defaultRegistryDirectory` computation exactly + * (Linux: `$XDG_CACHE_HOME || ~/.cache`; macOS: `~/Library/Caches`; + * Windows: `%LOCALAPPDATA% || ~/AppData/Local`), each joined with + * `ms-playwright`. Accepts `env`/`homeDirectory`/`platform` so tests never + * depend on the real host's actual cache directory or files within it. + */ +export function resolveDefaultManagedBrowsersRoot( + env = process.env, + homeDirectory = os.homedir(), + platform = process.platform, +) { + if (platform === "linux") { + return path.join(env.XDG_CACHE_HOME?.trim() || path.join(homeDirectory, ".cache"), "ms-playwright"); + } + if (platform === "darwin") { + return path.join(homeDirectory, "Library", "Caches", "ms-playwright"); + } + if (platform === "win32") { + return path.join(env.LOCALAPPDATA?.trim() || path.join(homeDirectory, "AppData", "Local"), "ms-playwright"); + } + return path.join(homeDirectory, ".cache", "ms-playwright"); +} + +/** + * The real, load-bearing check this file exists for: does a launchable + * Chromium binary for `revision` actually exist under `browsersRoot`? A + * same-named directory with no executable inside it (partial/corrupt + * install) must not count — that was the residual gap even in the old + * "container-aligned" path, which only checked directory names (#312). + */ +export function findInstalledChromiumBinary( + browsersRoot, + revision, + { platform = process.platform, architecture = process.arch, fileExists = existsSync } = {}, +) { + if (!browsersRoot || !revision) return null; + const candidates = [ + ...layoutsForPlatform(CHROMIUM_EXECUTABLE_LAYOUTS, platform, architecture).map((layout) => ({ + dir: `chromium-${revision}`, + layout, + })), + ...layoutsForPlatform(CHROMIUM_HEADLESS_SHELL_EXECUTABLE_LAYOUTS, platform, architecture).map((layout) => ({ + dir: `chromium_headless_shell-${revision}`, + layout, + })), + ]; + for (const candidate of candidates) { + const executable = path.join(browsersRoot, candidate.dir, ...candidate.layout); + if (fileExists(executable)) return executable; + } + return null; +} + /** * @param {{ * projectRoot?: string, * env?: NodeJS.ProcessEnv, * containerBrowsersRoot?: string, + * defaultManagedBrowsersRoot?: string, + * platform?: string, + * architecture?: string, + * fileExists?: (path: string) => boolean, * }} [options] */ export function playwrightBrowserRevisionCheck(options = {}) { const projectRoot = options.projectRoot ?? process.cwd(); const env = options.env ?? process.env; const containerBrowsersRoot = options.containerBrowsersRoot ?? DEFAULT_CONTAINER_ROOT; + const platform = options.platform ?? process.platform; + const architecture = options.architecture ?? process.arch; + const fileExists = options.fileExists ?? existsSync; + const expected = readExpectedChromiumRevision(projectRoot); if (!expected.ok) { return { @@ -70,37 +185,71 @@ export function playwrightBrowserRevisionCheck(options = {}) { const designatedContainer = exposedRoot.replaceAll("\\", "/") === containerBrowsersRoot.replaceAll("\\", "/") && downloadsDisabled; - if (!designatedContainer) { + // Any PLAYWRIGHT_BROWSERS_PATH override — the designated download-disabled + // container or any other forced path — points at the exact root Playwright + // will actually launch from. With no override, fall back to Playwright's + // own default managed-cache location so a plain, "unconstrained" run still + // gets checked against real disk state instead of being trusted on version + // metadata alone. This is the #312 fix: the previous version skipped this + // check entirely whenever no container root was forced. + const defaultManagedBrowsersRoot = options.defaultManagedBrowsersRoot ?? resolveDefaultManagedBrowsersRoot(env); + const browsersRoot = exposedRoot || defaultManagedBrowsersRoot; + + const installed = listInstalledChromiumRevisions(browsersRoot); + const revisionDirectoryPresent = installed.includes(expected.revision); + const binaryPath = findInstalledChromiumBinary(browsersRoot, expected.revision, { + platform, + architecture, + fileExists, + }); + + if (revisionDirectoryPresent && binaryPath) { return { ok: true, - status: "managed-or-unconstrained", - message: - "No designated container browser root is forced; use the Playwright-managed cache or install matching browsers.", + status: designatedContainer ? "container-aligned" : "installed", + message: designatedContainer + ? `Container browsers at ${exposedRoot} include a launchable chromium revision ${expected.revision} binary (${binaryPath}).` + : `Chromium revision ${expected.revision} is installed and launchable at ${binaryPath}.`, expectedRevision: expected.revision, - installedRevisions: [], + installedRevisions: installed, + binaryPath, }; } - const installed = listInstalledChromiumRevisions(exposedRoot); - if (installed.includes(expected.revision)) { + if (designatedContainer) { return { - ok: true, - status: "container-aligned", - message: `Container browsers at ${exposedRoot} include chromium revision ${expected.revision}.`, + ok: false, + status: "container-revision-drift", + message: [ + `Playwright browser revision drift (#255): lock/playwright-core expects chromium-${expected.revision},`, + `but ${exposedRoot} only has: ${installed.length ? installed.map((r) => `chromium-${r}`).join(", ") : "(none)"}.`, + "Do not set PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH to a mismatched shell.", + "Delegate browser proof to CI Production UI, or refresh the image / run `npx playwright install` into a matching cache,", + "or unset PLAYWRIGHT_BROWSERS_PATH and PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD so the managed cache can be used.", + ].join(" "), expectedRevision: expected.revision, installedRevisions: installed, }; } + // Unconstrained / plain-managed-cache path: downloads are not disabled + // here, so the actionable fix is normally `npx playwright install + // chromium`, not a container image refresh (#312). + const rootDescription = exposedRoot + ? `${browsersRoot} (from PLAYWRIGHT_BROWSERS_PATH)` + : `${browsersRoot} (Playwright's default managed cache)`; return { ok: false, - status: "container-revision-drift", + status: revisionDirectoryPresent ? "binary-missing" : "not-installed", message: [ - `Playwright browser revision drift (#255): lock/playwright-core expects chromium-${expected.revision},`, - `but ${exposedRoot} only has: ${installed.length ? installed.map((r) => `chromium-${r}`).join(", ") : "(none)"}.`, - "Do not set PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH to a mismatched shell.", - "Delegate browser proof to CI Production UI, or refresh the image / run `npx playwright install` into a matching cache,", - "or unset PLAYWRIGHT_BROWSERS_PATH and PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD so the managed cache can be used.", + revisionDirectoryPresent + ? `Playwright expects chromium revision ${expected.revision}: a matching directory exists at ${rootDescription} but no launchable Chromium binary was found inside it (partial or corrupt install).` + : `Playwright expects chromium revision ${expected.revision}, but no matching install was found at ${rootDescription}` + + (installed.length + ? ` (found instead: ${installed.map((r) => `chromium-${r}`).join(", ")}).` + : " (no chromium revisions installed at all).") + + "", + "Run `npx playwright install chromium` to install the pinned revision, or point PLAYWRIGHT_BROWSERS_PATH at a cache that already has it.", ].join(" "), expectedRevision: expected.revision, installedRevisions: installed, @@ -143,7 +292,7 @@ if (isDirectRun()) { } else if (result.ok) { console.log(`Playwright browser revision check OK (${result.status}): ${result.message}`); } else { - console.error(result.message); + console.error(`Playwright browser revision check FAILED (${result.status}): ${result.message}`); } process.exit(result.ok ? 0 : 1); } diff --git a/tests/check-playwright-browser-revision.test.ts b/tests/check-playwright-browser-revision.test.ts index f3f802ba33..313ae449c9 100644 --- a/tests/check-playwright-browser-revision.test.ts +++ b/tests/check-playwright-browser-revision.test.ts @@ -1,10 +1,12 @@ import { describe, expect, it } from "vitest"; import { + findInstalledChromiumBinary, listInstalledChromiumRevisions, playwrightBrowserRevisionCheck, readExpectedChromiumRevision, + resolveDefaultManagedBrowsersRoot, } from "../scripts/check-playwright-browser-revision.mjs"; -import { mkdtempSync, mkdirSync, writeFileSync } from "node:fs"; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; @@ -15,12 +17,97 @@ describe("check-playwright-browser-revision", () => { expect(expected.revision).toMatch(/^\d+$/); }); - it("passes when no designated container browser root is forced", () => { - const result = playwrightBrowserRevisionCheck({ - env: { NODE_ENV: "test" }, - }); - expect(result.ok).toBe(true); - expect(result.status).toBe("managed-or-unconstrained"); + it("fails closed (#312) when no browsers root — forced or default — has a matching binary on disk", () => { + // This is the exact false-"OK" regression: no PLAYWRIGHT_BROWSERS_PATH is + // forced, so the old check trusted "unconstrained" as a pass without ever + // looking at disk. An isolated, guaranteed-empty default cache directory + // must now report failure, not a green light. + const emptyDefaultRoot = mkdtempSync(path.join(tmpdir(), "pw-empty-default-")); + const projectRoot = mkdtempSync(path.join(tmpdir(), "pw-project-")); + try { + mkdirSync(path.join(projectRoot, "node_modules", "playwright-core"), { recursive: true }); + writeFileSync( + path.join(projectRoot, "node_modules", "playwright-core", "browsers.json"), + JSON.stringify({ browsers: [{ name: "chromium", revision: "1234" }] }), + ); + + const result = playwrightBrowserRevisionCheck({ + projectRoot, + env: { NODE_ENV: "test" }, + defaultManagedBrowsersRoot: emptyDefaultRoot, + }); + + expect(result.ok).toBe(false); + expect(result.status).toBe("not-installed"); + expect(result.expectedRevision).toBe("1234"); + expect(result.installedRevisions).toEqual([]); + expect(result.message).toContain("npx playwright install chromium"); + } finally { + rmSync(emptyDefaultRoot, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + rmSync(projectRoot, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } + }); + + it("passes (#312) when the default managed cache actually has a launchable chromium binary", () => { + const root = mkdtempSync(path.join(tmpdir(), "pw-default-installed-")); + const projectRoot = mkdtempSync(path.join(tmpdir(), "pw-project-")); + try { + const binary = path.join(root, "chromium-1234", "chrome-linux64", "chrome"); + mkdirSync(path.dirname(binary), { recursive: true }); + writeFileSync(binary, ""); + + mkdirSync(path.join(projectRoot, "node_modules", "playwright-core"), { recursive: true }); + writeFileSync( + path.join(projectRoot, "node_modules", "playwright-core", "browsers.json"), + JSON.stringify({ browsers: [{ name: "chromium", revision: "1234" }] }), + ); + + const result = playwrightBrowserRevisionCheck({ + projectRoot, + env: { NODE_ENV: "test" }, + defaultManagedBrowsersRoot: root, + platform: "linux", + architecture: "x64", + }); + + expect(result.ok).toBe(true); + expect(result.status).toBe("installed"); + expect(result.binaryPath).toBe(binary); + } finally { + rmSync(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + rmSync(projectRoot, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } + }); + + it("fails closed (#312) when a revision directory exists but has no real binary inside it (partial/corrupt install)", () => { + // Directory-name matching alone is not enough — this is the residual gap + // that survived even in the old "container-aligned" path. + const root = mkdtempSync(path.join(tmpdir(), "pw-empty-dir-")); + const projectRoot = mkdtempSync(path.join(tmpdir(), "pw-project-")); + try { + mkdirSync(path.join(root, "chromium-1234"), { recursive: true }); + + mkdirSync(path.join(projectRoot, "node_modules", "playwright-core"), { recursive: true }); + writeFileSync( + path.join(projectRoot, "node_modules", "playwright-core", "browsers.json"), + JSON.stringify({ browsers: [{ name: "chromium", revision: "1234" }] }), + ); + + const result = playwrightBrowserRevisionCheck({ + projectRoot, + env: { NODE_ENV: "test" }, + defaultManagedBrowsersRoot: root, + platform: "linux", + architecture: "x64", + }); + + expect(result.ok).toBe(false); + expect(result.status).toBe("binary-missing"); + expect(result.installedRevisions).toEqual(["1234"]); + } finally { + rmSync(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + rmSync(projectRoot, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } }); it("fails closed on /opt/pw-browsers revision drift without suggesting a mismatched executable (#255)", () => { @@ -51,6 +138,81 @@ describe("check-playwright-browser-revision", () => { expect(result.message).not.toMatch(/set PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH to .*1194/); }); + it("passes when the designated container actually has a launchable binary for the pinned revision", () => { + const root = mkdtempSync(path.join(tmpdir(), "pw-container-aligned-")); + const projectRoot = mkdtempSync(path.join(tmpdir(), "pw-project-")); + try { + const binary = path.join( + root, + "chromium_headless_shell-1234", + "chrome-headless-shell-linux64", + "chrome-headless-shell", + ); + mkdirSync(path.dirname(binary), { recursive: true }); + writeFileSync(binary, ""); + + mkdirSync(path.join(projectRoot, "node_modules", "playwright-core"), { recursive: true }); + writeFileSync( + path.join(projectRoot, "node_modules", "playwright-core", "browsers.json"), + JSON.stringify({ browsers: [{ name: "chromium", revision: "1234" }] }), + ); + + const result = playwrightBrowserRevisionCheck({ + projectRoot, + containerBrowsersRoot: root, + platform: "linux", + architecture: "x64", + env: { + NODE_ENV: "test", + PLAYWRIGHT_BROWSERS_PATH: root, + PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: "1", + }, + }); + + expect(result.ok).toBe(true); + expect(result.status).toBe("container-aligned"); + expect(result.binaryPath).toBe(binary); + } finally { + rmSync(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + rmSync(projectRoot, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } + }); + + it("fails closed when the designated container has only an empty revision directory, not a real binary (#312)", () => { + // Same drift status as a fully-missing revision: a same-named empty + // directory must not be mistaken for an installed, launchable browser. + const root = mkdtempSync(path.join(tmpdir(), "pw-container-empty-")); + const projectRoot = mkdtempSync(path.join(tmpdir(), "pw-project-")); + try { + mkdirSync(path.join(root, "chromium_headless_shell-1234"), { recursive: true }); + + mkdirSync(path.join(projectRoot, "node_modules", "playwright-core"), { recursive: true }); + writeFileSync( + path.join(projectRoot, "node_modules", "playwright-core", "browsers.json"), + JSON.stringify({ browsers: [{ name: "chromium", revision: "1234" }] }), + ); + + const result = playwrightBrowserRevisionCheck({ + projectRoot, + containerBrowsersRoot: root, + platform: "linux", + architecture: "x64", + env: { + NODE_ENV: "test", + PLAYWRIGHT_BROWSERS_PATH: root, + PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: "1", + }, + }); + + expect(result.ok).toBe(false); + expect(result.status).toBe("container-revision-drift"); + expect(result.installedRevisions).toEqual(["1234"]); + } finally { + rmSync(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + rmSync(projectRoot, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } + }); + it("lists installed chromium revisions from a browsers root", () => { const root = mkdtempSync(path.join(tmpdir(), "pw-list-")); mkdirSync(path.join(root, "chromium-1234")); @@ -58,4 +220,44 @@ describe("check-playwright-browser-revision", () => { mkdirSync(path.join(root, "firefox-1000")); expect(listInstalledChromiumRevisions(root)).toEqual(["1234"]); }); + + it("finds the actual chromium binary for a revision, not just its directory", () => { + const root = mkdtempSync(path.join(tmpdir(), "pw-find-binary-")); + try { + expect(findInstalledChromiumBinary(root, "1234", { platform: "linux", architecture: "x64" })).toBeNull(); + + const binary = path.join(root, "chromium-1234", "chrome-linux64", "chrome"); + mkdirSync(path.dirname(binary), { recursive: true }); + writeFileSync(binary, ""); + expect(findInstalledChromiumBinary(root, "1234", { platform: "linux", architecture: "x64" })).toBe(binary); + } finally { + rmSync(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } + }); + + it("resolves the platform-specific default managed-cache directory", () => { + const baseEnv = { NODE_ENV: "test" as const }; + expect(resolveDefaultManagedBrowsersRoot(baseEnv, "/home/dev", "linux").replaceAll("\\", "/")).toBe( + "/home/dev/.cache/ms-playwright", + ); + expect( + resolveDefaultManagedBrowsersRoot( + { ...baseEnv, XDG_CACHE_HOME: "/custom/cache" }, + "/home/dev", + "linux", + ).replaceAll("\\", "/"), + ).toBe("/custom/cache/ms-playwright"); + expect(resolveDefaultManagedBrowsersRoot(baseEnv, "/Users/dev", "darwin").replaceAll("\\", "/")).toBe( + "/Users/dev/Library/Caches/ms-playwright", + ); + expect(resolveDefaultManagedBrowsersRoot(baseEnv, "C:/Users/dev", "win32").replaceAll("\\", "/")).toBe( + "C:/Users/dev/AppData/Local/ms-playwright", + ); + expect( + resolveDefaultManagedBrowsersRoot({ ...baseEnv, LOCALAPPDATA: "C:/custom" }, "C:/Users/dev", "win32").replaceAll( + "\\", + "/", + ), + ).toBe("C:/custom/ms-playwright"); + }); }); From e4c0928f25d21f2da90210f3d9ddde8fa3404d59 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 15:20:35 +0000 Subject: [PATCH 2/5] docs(ledger): record review entry for PR #1965 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01QLbw9qpfjv5CeNz6XpmteN --- ...bac65652b1b8008514d5c4f4b2c2e82254fec86eb5c50ee6784.record.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/branch-review-records/1cd9399241d3cbac65652b1b8008514d5c4f4b2c2e82254fec86eb5c50ee6784.record.md diff --git a/docs/branch-review-records/1cd9399241d3cbac65652b1b8008514d5c4f4b2c2e82254fec86eb5c50ee6784.record.md b/docs/branch-review-records/1cd9399241d3cbac65652b1b8008514d5c4f4b2c2e82254fec86eb5c50ee6784.record.md new file mode 100644 index 0000000000..94e188293b --- /dev/null +++ b/docs/branch-review-records/1cd9399241d3cbac65652b1b8008514d5c4f4b2c2e82254fec86eb5c50ee6784.record.md @@ -0,0 +1 @@ +| 2026-08-14 | PR #1965 (claude/playwright-browser-revision-check) | 24e8fd040949564e2c07fccfe6d243e386815b6e | scripts/check-playwright-browser-revision.mjs, tests/check-playwright-browser-revision.test.ts — ledger #312 fix | authored — verified real Chromium binary presence check on disk; reproduced original false-OK bug live against this session's /opt/pw-browsers container (chromium-1194 present, pinned revision 1234 missing) | verify:pr-local (runtime, lock-parity, format, lint, typecheck, test 602 files/6518 passed/4 skipped, rag:fixtures, medication-interactions PASS; medication-lexicon-report pre-existing unrelated FAIL); focused vitest check-playwright-browser-revision.test.ts 10/10, playwright-browser-preflight.test.ts 12/12 | From 1a08cc10e09303c998f02f03e5c54fa5b120d980 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Sat, 15 Aug 2026 02:11:10 +0800 Subject: [PATCH 3/5] fix(playwright): validate headless-shell cache paths --- scripts/check-playwright-browser-revision.mjs | 73 ++++++---- .../check-playwright-browser-revision.test.ts | 130 +++++++++++++++++- 2 files changed, 166 insertions(+), 37 deletions(-) diff --git a/scripts/check-playwright-browser-revision.mjs b/scripts/check-playwright-browser-revision.mjs index b918742f9e..d9c719371e 100644 --- a/scripts/check-playwright-browser-revision.mjs +++ b/scripts/check-playwright-browser-revision.mjs @@ -24,27 +24,13 @@ * where a newer lock expects chromium-1234 but the image only ships 1194 — * pointing PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH at the stale shell is forbidden. */ -import { existsSync, readFileSync, readdirSync } from "node:fs"; +import { accessSync, constants, existsSync, readFileSync, readdirSync, statSync } from "node:fs"; import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; const DEFAULT_CONTAINER_ROOT = "/opt/pw-browsers"; -// Mirrors playwright-core's `EXECUTABLE_PATHS.chromium` (full Chrome for -// Testing, installed under a `chromium-` directory). -const CHROMIUM_EXECUTABLE_LAYOUTS = Object.freeze({ - linux: { - x64: [["chrome-linux64", "chrome"]], - arm64: [["chrome-linux", "chrome"]], - }, - darwin: { - x64: [["chrome-mac-x64", "Google Chrome for Testing.app", "Contents", "MacOS", "Google Chrome for Testing"]], - arm64: [["chrome-mac-arm64", "Google Chrome for Testing.app", "Contents", "MacOS", "Google Chrome for Testing"]], - }, - win32: [["chrome-win64", "chrome.exe"]], -}); - // Mirrors playwright-core's `EXECUTABLE_PATHS["chromium-headless-shell"]` // (installed under a `chromium_headless_shell-` directory — the // binary the default headless chromium/chromium-mockups projects launch). @@ -120,9 +106,37 @@ export function resolveDefaultManagedBrowsersRoot( return path.join(homeDirectory, ".cache", "ms-playwright"); } +/** + * Resolve the cache root with the same special/relative-path semantics that + * playwright-core uses. `PLAYWRIGHT_BROWSERS_PATH=0` opts into the installed + * package's `.local-browsers`; relative overrides resolve from INIT_CWD (or + * the invoking working directory), not from a literal directory named "0". + */ +export function resolvePlaywrightBrowsersRoot({ + env = process.env, + defaultManagedBrowsersRoot = resolveDefaultManagedBrowsersRoot(env), + playwrightCoreRoot, + workingDirectory = process.cwd(), +} = {}) { + const configured = env.PLAYWRIGHT_BROWSERS_PATH?.trim() ?? ""; + if (configured === "0") return path.join(playwrightCoreRoot, ".local-browsers"); + if (!configured) return defaultManagedBrowsersRoot; + return path.isAbsolute(configured) ? configured : path.resolve(env.INIT_CWD?.trim() || workingDirectory, configured); +} + +export function isLaunchableFile(filePath, platform = process.platform) { + try { + if (!statSync(filePath).isFile()) return false; + if (platform !== "win32") accessSync(filePath, constants.X_OK); + return true; + } catch { + return false; + } +} + /** * The real, load-bearing check this file exists for: does a launchable - * Chromium binary for `revision` actually exist under `browsersRoot`? A + * headless-shell binary for `revision` actually exist under `browsersRoot`? A * same-named directory with no executable inside it (partial/corrupt * install) must not count — that was the residual gap even in the old * "container-aligned" path, which only checked directory names (#312). @@ -130,22 +144,18 @@ export function resolveDefaultManagedBrowsersRoot( export function findInstalledChromiumBinary( browsersRoot, revision, - { platform = process.platform, architecture = process.arch, fileExists = existsSync } = {}, + { platform = process.platform, architecture = process.arch, fileIsLaunchable = isLaunchableFile } = {}, ) { if (!browsersRoot || !revision) return null; - const candidates = [ - ...layoutsForPlatform(CHROMIUM_EXECUTABLE_LAYOUTS, platform, architecture).map((layout) => ({ - dir: `chromium-${revision}`, - layout, - })), - ...layoutsForPlatform(CHROMIUM_HEADLESS_SHELL_EXECUTABLE_LAYOUTS, platform, architecture).map((layout) => ({ + const candidates = layoutsForPlatform(CHROMIUM_HEADLESS_SHELL_EXECUTABLE_LAYOUTS, platform, architecture).map( + (layout) => ({ dir: `chromium_headless_shell-${revision}`, layout, - })), - ]; + }), + ); for (const candidate of candidates) { const executable = path.join(browsersRoot, candidate.dir, ...candidate.layout); - if (fileExists(executable)) return executable; + if (fileIsLaunchable(executable, platform)) return executable; } return null; } @@ -158,7 +168,7 @@ export function findInstalledChromiumBinary( * defaultManagedBrowsersRoot?: string, * platform?: string, * architecture?: string, - * fileExists?: (path: string) => boolean, + * workingDirectory?: string, * }} [options] */ export function playwrightBrowserRevisionCheck(options = {}) { @@ -167,7 +177,6 @@ export function playwrightBrowserRevisionCheck(options = {}) { const containerBrowsersRoot = options.containerBrowsersRoot ?? DEFAULT_CONTAINER_ROOT; const platform = options.platform ?? process.platform; const architecture = options.architecture ?? process.arch; - const fileExists = options.fileExists ?? existsSync; const expected = readExpectedChromiumRevision(projectRoot); if (!expected.ok) { @@ -193,14 +202,18 @@ export function playwrightBrowserRevisionCheck(options = {}) { // metadata alone. This is the #312 fix: the previous version skipped this // check entirely whenever no container root was forced. const defaultManagedBrowsersRoot = options.defaultManagedBrowsersRoot ?? resolveDefaultManagedBrowsersRoot(env); - const browsersRoot = exposedRoot || defaultManagedBrowsersRoot; + const browsersRoot = resolvePlaywrightBrowsersRoot({ + env, + defaultManagedBrowsersRoot, + playwrightCoreRoot: path.dirname(expected.browsersJsonPath), + workingDirectory: options.workingDirectory, + }); const installed = listInstalledChromiumRevisions(browsersRoot); const revisionDirectoryPresent = installed.includes(expected.revision); const binaryPath = findInstalledChromiumBinary(browsersRoot, expected.revision, { platform, architecture, - fileExists, }); if (revisionDirectoryPresent && binaryPath) { diff --git a/tests/check-playwright-browser-revision.test.ts b/tests/check-playwright-browser-revision.test.ts index 313ae449c9..247505044b 100644 --- a/tests/check-playwright-browser-revision.test.ts +++ b/tests/check-playwright-browser-revision.test.ts @@ -48,13 +48,18 @@ describe("check-playwright-browser-revision", () => { } }); - it("passes (#312) when the default managed cache actually has a launchable chromium binary", () => { + it("passes (#312) when the default managed cache has a launchable headless-shell binary", () => { const root = mkdtempSync(path.join(tmpdir(), "pw-default-installed-")); const projectRoot = mkdtempSync(path.join(tmpdir(), "pw-project-")); try { - const binary = path.join(root, "chromium-1234", "chrome-linux64", "chrome"); + const binary = path.join( + root, + "chromium_headless_shell-1234", + "chrome-headless-shell-linux64", + "chrome-headless-shell", + ); mkdirSync(path.dirname(binary), { recursive: true }); - writeFileSync(binary, ""); + writeFileSync(binary, "", { mode: 0o755 }); mkdirSync(path.join(projectRoot, "node_modules", "playwright-core"), { recursive: true }); writeFileSync( @@ -149,7 +154,7 @@ describe("check-playwright-browser-revision", () => { "chrome-headless-shell", ); mkdirSync(path.dirname(binary), { recursive: true }); - writeFileSync(binary, ""); + writeFileSync(binary, "", { mode: 0o755 }); mkdirSync(path.join(projectRoot, "node_modules", "playwright-core"), { recursive: true }); writeFileSync( @@ -213,6 +218,112 @@ describe("check-playwright-browser-revision", () => { } }); + it("fails closed when only full Chrome is present because the default projects launch headless shell", () => { + const root = mkdtempSync(path.join(tmpdir(), "pw-full-chrome-only-")); + const projectRoot = mkdtempSync(path.join(tmpdir(), "pw-project-")); + try { + const fullChrome = path.join(root, "chromium-1234", "chrome-linux64", "chrome"); + mkdirSync(path.dirname(fullChrome), { recursive: true }); + writeFileSync(fullChrome, "", { mode: 0o755 }); + mkdirSync(path.join(projectRoot, "node_modules", "playwright-core"), { recursive: true }); + writeFileSync( + path.join(projectRoot, "node_modules", "playwright-core", "browsers.json"), + JSON.stringify({ browsers: [{ name: "chromium", revision: "1234" }] }), + ); + + const result = playwrightBrowserRevisionCheck({ + projectRoot, + env: { NODE_ENV: "test" }, + defaultManagedBrowsersRoot: root, + platform: "linux", + architecture: "x64", + }); + + expect(result.ok).toBe(false); + expect(result.status).toBe("binary-missing"); + } finally { + rmSync(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + rmSync(projectRoot, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } + }); + + it("uses playwright-core's package-local cache when PLAYWRIGHT_BROWSERS_PATH=0", () => { + const projectRoot = mkdtempSync(path.join(tmpdir(), "pw-project-")); + try { + const coreRoot = path.join(projectRoot, "node_modules", "playwright-core"); + const binary = path.join( + coreRoot, + ".local-browsers", + "chromium_headless_shell-1234", + "chrome-headless-shell-linux64", + "chrome-headless-shell", + ); + mkdirSync(path.dirname(binary), { recursive: true }); + writeFileSync(binary, "", { mode: 0o755 }); + writeFileSync( + path.join(coreRoot, "browsers.json"), + JSON.stringify({ browsers: [{ name: "chromium", revision: "1234" }] }), + ); + + const result = playwrightBrowserRevisionCheck({ + projectRoot, + env: { NODE_ENV: "test", PLAYWRIGHT_BROWSERS_PATH: "0" }, + platform: "linux", + architecture: "x64", + }); + + expect(result.ok).toBe(true); + expect(result.status).toBe("installed"); + expect(result.binaryPath).toBe(binary); + } finally { + rmSync(projectRoot, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } + }); + + it("fails closed when the expected headless-shell path is a directory or a non-executable file", () => { + const root = mkdtempSync(path.join(tmpdir(), "pw-not-executable-")); + const projectRoot = mkdtempSync(path.join(tmpdir(), "pw-project-")); + try { + const binary = path.join( + root, + "chromium_headless_shell-1234", + "chrome-headless-shell-linux64", + "chrome-headless-shell", + ); + mkdirSync(binary, { recursive: true }); + mkdirSync(path.join(projectRoot, "node_modules", "playwright-core"), { recursive: true }); + writeFileSync( + path.join(projectRoot, "node_modules", "playwright-core", "browsers.json"), + JSON.stringify({ browsers: [{ name: "chromium", revision: "1234" }] }), + ); + + const directoryResult = playwrightBrowserRevisionCheck({ + projectRoot, + env: { NODE_ENV: "test" }, + defaultManagedBrowsersRoot: root, + platform: "linux", + architecture: "x64", + }); + expect(directoryResult.ok).toBe(false); + expect(directoryResult.status).toBe("binary-missing"); + + rmSync(binary, { recursive: true, force: true }); + writeFileSync(binary, "", { mode: 0o644 }); + const nonExecutableResult = playwrightBrowserRevisionCheck({ + projectRoot, + env: { NODE_ENV: "test" }, + defaultManagedBrowsersRoot: root, + platform: "linux", + architecture: "x64", + }); + expect(nonExecutableResult.ok).toBe(false); + expect(nonExecutableResult.status).toBe("binary-missing"); + } finally { + rmSync(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + rmSync(projectRoot, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } + }); + it("lists installed chromium revisions from a browsers root", () => { const root = mkdtempSync(path.join(tmpdir(), "pw-list-")); mkdirSync(path.join(root, "chromium-1234")); @@ -221,14 +332,19 @@ describe("check-playwright-browser-revision", () => { expect(listInstalledChromiumRevisions(root)).toEqual(["1234"]); }); - it("finds the actual chromium binary for a revision, not just its directory", () => { + it("finds the actual headless-shell binary for a revision, not just its directory", () => { const root = mkdtempSync(path.join(tmpdir(), "pw-find-binary-")); try { expect(findInstalledChromiumBinary(root, "1234", { platform: "linux", architecture: "x64" })).toBeNull(); - const binary = path.join(root, "chromium-1234", "chrome-linux64", "chrome"); + const binary = path.join( + root, + "chromium_headless_shell-1234", + "chrome-headless-shell-linux64", + "chrome-headless-shell", + ); mkdirSync(path.dirname(binary), { recursive: true }); - writeFileSync(binary, ""); + writeFileSync(binary, "", { mode: 0o755 }); expect(findInstalledChromiumBinary(root, "1234", { platform: "linux", architecture: "x64" })).toBe(binary); } finally { rmSync(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); From 03f6b4bb771a8e27d0ac2a0f1ae82b37e6ac9f9d Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Sat, 15 Aug 2026 02:14:05 +0800 Subject: [PATCH 4/5] docs(ledger): record PR #1965 browser check review --- ...c93cfbe6f9c54463ec90d9dcbbfa158d1876e0ed8744f2c0162.record.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/branch-review-records/91679c0f8d364c93cfbe6f9c54463ec90d9dcbbfa158d1876e0ed8744f2c0162.record.md diff --git a/docs/branch-review-records/91679c0f8d364c93cfbe6f9c54463ec90d9dcbbfa158d1876e0ed8744f2c0162.record.md b/docs/branch-review-records/91679c0f8d364c93cfbe6f9c54463ec90d9dcbbfa158d1876e0ed8744f2c0162.record.md new file mode 100644 index 0000000000..999a5c168f --- /dev/null +++ b/docs/branch-review-records/91679c0f8d364c93cfbe6f9c54463ec90d9dcbbfa158d1876e0ed8744f2c0162.record.md @@ -0,0 +1 @@ +| 2026-08-14 | claude/playwright-browser-revision-check | 3447238f1c66154dca9b567924fb0a2f57a28c84 | PR #1965: Playwright browser revision check | fixed | prettier; targeted Vitest 5 passed; independent Codex adversarial review: 3 P2 fixed; full Vitest unavailable (cached runtime lacks playwright-core browsers.json) | From e795f2d942605d68701235f391e0adcb01aa227d Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Sat, 15 Aug 2026 02:34:12 +0800 Subject: [PATCH 5/5] docs(ledger): record PR #1965 CI correction --- ...570212c29f7afe5379d101d7e3ada1bb102dcd1631e83d7bf55.record.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/branch-review-records/7060c1fcf1c15570212c29f7afe5379d101d7e3ada1bb102dcd1631e83d7bf55.record.md diff --git a/docs/branch-review-records/7060c1fcf1c15570212c29f7afe5379d101d7e3ada1bb102dcd1631e83d7bf55.record.md b/docs/branch-review-records/7060c1fcf1c15570212c29f7afe5379d101d7e3ada1bb102dcd1631e83d7bf55.record.md new file mode 100644 index 0000000000..4ef15deaa1 --- /dev/null +++ b/docs/branch-review-records/7060c1fcf1c15570212c29f7afe5379d101d7e3ada1bb102dcd1631e83d7bf55.record.md @@ -0,0 +1 @@ +| 2026-08-14 | PR #1965 | a535862933966cede9c0f7f11734167a93b67c62 | Playwright browser-revision preflight | fixed | Prettier; 12 focused browser-check tests passed; test-runner safety covered by exact-head CI; full local suite blocked by incomplete cached dependencies; merged main |