Skip to content

fix(build): playwright extension parses the Playwright 1.58+ install --dry-run output - #7

Open
anurag6569201 wants to merge 1 commit into
qa/agent-triggerdotdev-trigger-dev/pr-13-4881/basefrom
qa/agent-triggerdotdev-trigger-dev/pr-13-4881/head
Open

fix(build): playwright extension parses the Playwright 1.58+ install --dry-run output#7
anurag6569201 wants to merge 1 commit into
qa/agent-triggerdotdev-trigger-dev/pr-13-4881/basefrom
qa/agent-triggerdotdev-trigger-dev/pr-13-4881/head

Conversation

@anurag6569201

Copy link
Copy Markdown

Fixes triggerdotdev#3089

Problem

The playwright build extension reads npx playwright install --dry-run to find each browser's install directory and download URL. Playwright 1.58 changed the per-browser header, so the extension's grep finds nothing and the image build fails at that step with exit code 1:

< 1.58   browser: chromium-headless-shell version 143.0.7499.4
>= 1.58  Chrome Headless Shell 151.0.7922.34 (playwright chromium-headless-shell v1234)

There is a second problem hiding behind the first. The old blocks were five lines (install location, download url, two fallback urls, blank), so grep -A5 was exact. The Chrome-for-Testing blocks in 1.58+ (chromium and chromium-headless-shell, the default install) are three lines (install location, download url, blank; Firefox and WebKit still print the fallbacks), so -A5 runs into the next browser's Install location: line. The downstream grep "Install location:" | cut | xargs then joins two paths and basename returns the next browser's directory: with the header grep alone fixed, chromium-headless-shell gets unpacked into firefox-<build>/. Any grep-only fix (including the pnpm patch shared in triggerdotdev#3089) has this bug.

Fix

  • The header match accepts both formats: grep -E "browser: <name> |\(playwright <name> v". The trailing space / v keep chromium from matching the chromium-headless-shell block in either format.
  • The context window is narrowed to -A2, the two lines the extension actually reads. Both formats put Install location: and Download url: immediately after the header.
  • A unit test asserts the generated header pattern selects each browser's block in both formats and never another browser's.

I left the sed "s/mac-arm64/linux/g" rewrite alone, and I do not think the "dead linux64 URLs" report in triggerdotdev#3089 is reachable from the extension. The --dry-run executes inside the Linux build container, so the URL it prints is already the Linux one and there is nothing for the rewrite to match. Checked by running the same command in a container:

$ docker run --rm --platform linux/amd64 node:22-bookworm-slim \
    sh -c 'npx -y playwright@1.62.0 install --dry-run'
Chrome Headless Shell 151.0.7922.34 (playwright chromium-headless-shell v1234)
  Download url:        https://cdn.playwright.dev/builds/cft/151.0.7922.34/linux64/chrome-headless-shell-linux64.zip
Firefox 153.0 (playwright firefox v1538)
  Download url:        https://cdn.playwright.dev/dbazure/download/playwright/builds/firefox/1538/firefox-debian-12.zip

Rewriting mac-arm64 to linux64 would also be wrong for < 1.58, where the Linux artifact is …-linux.zip, so changing it would trade a dead branch for an incorrect one.

Verification

Ran the generated extraction (grep -A2 -m1 -E … | grep "Install location:" | cut | xargs | basename, and the same for Download url:) against real --dry-run output from Playwright 1.57.0 and 1.62.0, for all four browser keys:

output browser directory url
1.57 chromium chromium-1200 chromium-linux.zip
1.57 chromium-headless-shell chromium_headless_shell-1200 chromium-headless-shell-linux.zip
1.57 firefox firefox-1497 firefox-linux.zip
1.57 webkit webkit-2227 webkit-ubuntu-20.04.zip
1.62 chromium chromium-1234 chrome-linux.zip
1.62 chromium-headless-shell chromium_headless_shell-1234 chrome-headless-shell-linux.zip
1.62 firefox firefox-1538 firefox-linux.zip
1.62 webkit webkit-2336 webkit-mac-26-arm64.zip

(The webkit 1.62 URL shows the mac name only because my sample output came from a Mac; in the build container the dry-run prints Linux URLs.)

End to end: built a node:22-bookworm-slim image (linux/amd64) that runs the extension's generated RUN steps verbatim for chromium-headless-shell against Playwright 1.62.0, then launched the browser through Playwright with PLAYWRIGHT_BROWSERS_PATH=/ms-playwright:

install dir: chromium_headless_shell-1234
Downloading from https://cdn.playwright.dev/builds/cft/151.0.7922.34/linux64/chrome-headless-shell-linux64.zip
/ms-playwright/chromium_headless_shell-1234/chrome-headless-shell-linux64
title: pw-e2e-ok | version: 151.0.7922.34

So the Chrome-for-Testing archive layout unpacks into the directory Playwright's registry expects, and the browser starts.

Changeset

@trigger.dev/build patch.

🤖 Generated with Claude Code

Source merge-base: e3db7a820f6166dba740dd7849029c31d040b03a
Source head: 3720e4b1e9769380db87783cc3c5e3238696fb1b

@shipwright-agent

Copy link
Copy Markdown

⛔ Shipwright · Blocked

Recommendation: do not merge PR #7 · Tier T1
Checks: 0 total · 0 needing attention

Next step: resolve the blocking findings before merge.

Findings (5)

  • CRITICAL The new regex pattern 'browser: ${browser} |(playwright ${browser} v' is built by interpolating the browser name directly into a regex string without escaping regex metacharacters · packages/build/src/extensions/playwright.ts:210
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • CRITICAL The change reduces the grep context window from '-A5' to '-A2' based on the assumption that only the two lines after the header are read. · packages/build/src/extensions/playwright.ts:333
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • CRITICAL The generated shell command interpolates 'dryRunHeaderPattern(browser)' into a 'RUN grep -E "..."' instruction. · packages/build/src/extensions/playwright.ts:333
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The new exported function 'dryRunHeaderPattern' returns a regex string with a trailing space and 'v' that are load-bearing for disambiguation, but the function name and JSDoc do no · packages/build/src/extensions/playwright.test.ts:1
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The test hardcodes 'browsers' from 'Object.keys(HEADERS["1.57"])' and assumes the 1.62 headers have the same keys. · packages/build/src/extensions/playwright.test.ts:17
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

Fireworks usage: 7,191 input · 776 output · 7,967 total tokens · $0.0021 · 13s · 0 fix iteration(s)

Open the Shipwright check for full evidence and the audit bundle. Use /shipwright rerun to verify again.

*/
export function dryRunHeaderPattern(browser: string): string {
return `browser: ${browser} |\\(playwright ${browser} v`;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipwright · CRITICAL

The new regex pattern 'browser: ${browser} |(playwright ${browser} v' is built by interpolating the browser name directly into a regex string without escaping regex metacharacters

Impact: The new regex pattern 'browser: ${browser} |(playwright ${browser} v' is built by interpolating the browser name directly into a regex string without escaping regex metacharacters. If a browser name ever contains a regex special character (e.g., a dot, plus, or parenthesis), the generated RegExp will either fail to compile or match unintended text, breaking the deploy image build. The existing code already interpol…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant