fix: bump @playwright/test to 1.62.1 so browser installs stop hanging - #637
Merged
Conversation
Playwright 1.59.1 cannot install its browsers on macOS/arm64. The failure
is in EXTRACTION, not the download, which is why waiting it out never
worked and why `worktree-setup.sh`'s timeout message ("no progress")
sends readers to the network:
1.59.1 / chromium-1217 -> stalls at exactly 84 files / 448K, 3 runs
1.62.1 / chromium-1234 -> completes in ~2 min, 3 runs
In every stalled run the full 173MB zip had already landed in
$TMPDIR/playwright-download-*, and sampling the process showed
oopDownloadBrowserMain.js idle at 0% CPU with every libuv worker parked
in __psynch_cvwait. Same machine, same network, minutes apart.
That hang is what blocks `scripts/worktree-setup.sh`: it kills the
install at its timeout and exits 1, so a fresh worktree never gets
browsers. With this bump the script runs to completion.
Verified against the real stack, not just the install:
smoke (--project=chromium) 70 passed, 0 failed
wizard (--project=wizard) all 14 scenarios passed
Note the suite must be run with MOCK_HA_PORT=8123. e2e/tests/mock-ha.spec.ts
and e2e/tests/health-recovery.spec.ts hardcode http://localhost:8123 while
docker-compose.ci.yml publishes ${MOCK_HA_PORT:-8123} and run-e2e.sh derives
that port from the worktree directory name -- so 7 tests fail in any worktree
whose hash is not 8023. Pre-existing and unrelated to this bump; left alone
here rather than widening the diff.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016tGBHZEAfKzP9qoJU4NdNY
johanzander
added a commit
that referenced
this pull request
Aug 17, 2026
…alse protection claim Five findings from /code-review on this PR, plus one from #637's review that belongs in this file. verify-sandbox.sh - Both new cache probes printed SKIP, counting NO failure, when the cache directory could not be created -- which under the sandbox is precisely the blocked condition being tested. A fresh machine exited 0 reporting the config safe while npm and Playwright were both about to fail. "Cannot be created" is now folded into the same check() and FAILS. - PLAYWRIGHT_BROWSERS_PATH=0 is Playwright's documented sentinel for "no shared cache", not a path. Treating it as one ran `mkdir -p 0` inside the repo and reported a PASS that measured nothing. Now skipped explicitly. quality-check.sh / CLAUDE.md - The comment claimed GitHub refuses `git push origin --delete release-X.Y`. It does not: the live rulesets cover ~DEFAULT_BRANCH, beta-release-* and tags only, so `release-X.Y` -- the stable hotfix branch the release skill pushes and tags -- is unguarded at BOTH layers. Documented as a known residual with the fix named (a release-* ruleset, not an ask rule) rather than left as a false claim. Creating that ruleset needs approval, so it is not done here. - The feature-branch residual was justified as "damage bounded to a branch nobody has merged". That does not survive this repo's own conventions: ~20 worktrees push in parallel as the SAME identity, so a misaimed --force destroys another agent's commits and closes its PR, and the reflog that would recover it is in a different worktree. Still accepted, now for the real reason. worktree-setup.sh - Sharing compared the two LOCKFILES but never checked that the tree being shared was installed from the lockfile beside it. A dependency bump merged to main updates main's package-lock.json and not its node_modules, so every new worktree would share a tree built from the old lockfile while this script reported success -- which is how the Playwright bump (#637) would have silently reintroduced the very install hang it fixes. Adds install_matches_lockfile(), comparing package-lock.json against node_modules/.package-lock.json. Optional deps are excluded: npm legitimately omits them, and counting that as staleness made a freshly installed e2e tree report STALE over `fsevents` alone -- which would push every worktree through a full install, the exact cost the sharing exists to avoid. Verified against real trees: main/e2e MATCHES, bump worktree MATCHES, and a simulated post-#637 state (1.62.1 lockfile beside a 1.59.1 install) correctly reports STALE. It also flags main/frontend as genuinely stale -- 84 real version mismatches, so that one wants an `npm ci`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016tGBHZEAfKzP9qoJU4NdNY
johanzander
added a commit
that referenced
this pull request
Aug 18, 2026
…iewer" The Stage 4 bot only ever acts when triggered by an `@claude-bot review` comment. So "green, no verdict yet" has two completely different meanings, and only one of them belongs to the reviewer: request NEWER than last push -> the bot is genuinely working: reviewer request OLDER, or absent -> nobody has asked: DISPATCHER Collapsing those parked six of eleven open PRs on someone who had not been asked and was never going to act. Measured on the live fleet: #637, #635 never requested at all #620 requested 17:40:53, pushed 18:33:05 #619 requested 10:50:40, pushed 21:53:00 #614 requested 06:55:38, pushed 07:08:20 #490 requested 08-15 13:41, pushed 08-16 15:30 Every one had been reported as `awaiting-review [reviewer]`. After this change the same fleet shows zero PRs waiting on the reviewer and zero waiting on the maintainer — the pipeline owes an action on all of them. That is the failure this whole branch is about, seen from the other side. The gate stops a confused loop asking too often; this surfaces the loop that stopped asking at all. Both are the same lost state — whether the last verdict has been consumed — and both are recoverable from the PR rather than from a session that died. `comments` joins the field set for this. It fits inside the GraphQL node budget at --limit 30, which is already bounded by `commits`. Verified by mutation: disabling the request-feed check reddens test_a_review_never_requested_is_the_dispatchers_turn_not_the_reviewers and test_a_push_after_the_last_request_owes_a_new_round. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
|
@claude-bot review |
![bess-manager-claude-bot[bot]](https://avatars.githubusercontent.com/in/3024553?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review: bump @playwright/test 1.59.1 → 1.62.1
Root cause match: Yes. The PR documents a specific, reproducible measurement (3 stalled runs on 1.59.1, all stalling at exactly 84 files/448K during extraction, not download — confirmed via sample showing oopDownloadBrowserMain.js idle at 0% CPU) versus 3 clean completions on 1.62.1 (~2 min each). This directly explains why worktree-setup.sh's INSTALL_TIMEOUT (scripts/worktree-setup.sh:168) kills the install and exits 1 on 1.59.1. No linked GitHub issue, but the evidence trail in the PR body stands on its own.
Regression coverage: No new test was added, which is appropriate here — this is an infra/tooling fix (a browser-install hang), not a behavior change in application code, so there's no code path for a unit/E2E test to cover. The PR instead re-ran the actual E2E suites as verification (smoke: 70/0, wizard: 14/14) plus quality-check.sh clean. That's the right form of evidence for this kind of change.
Minimality / scope: Yes. Diff is exactly e2e/package.json (one version bump) and e2e/package-lock.json (the matching lockfile regeneration) — no unrelated changes bundled. Verified the lockfile's new dist.integrity hash for @playwright/test@1.62.1 (sha512-DTcUc8qii+...) matches the real npm registry entry — no supply-chain concern.
Rule violations: None found.
- N/A checks: no Python/backend code touched (architecture/error-handling/typing rules), no new classes, no hardcoded entity IDs, no exception handling changes.
docs/agents/rules.mdDebugging Protocol scope-assessment (step 9): this is a single-file-pair, single-purpose dependency bump with no structural ambiguity (no new/moved responsibility, single plausible owner) — I don't think a scope-assessment statement was needed here, but flagging that the PR body doesn't explicitly state one in case the maintainer wants that as a matter of consistency.
One thing verified independently (not a blocker): Playwright 1.62.1 raises engines.node to >=20. Checked .github/workflows/ci.yml — the e2e job pins node-version: "20" (ci.yml:171, :200), so CI is unaffected. docker-compose.ci.yml doesn't build its own Node image for e2e (mounts config files only), so no separate image to bump.
No blockers. Approving.
johanzander
marked this pull request as ready for review
August 18, 2026 05:34
johanzander
added a commit
that referenced
this pull request
Aug 18, 2026
…ard pushes server-side (#635) * fix: unblock sandboxed worktree setup, stop E2E dirtying the tree, guard pushes server-side Three environment problems, each with a measured cause. 1. `worktree-setup.sh` could not install Playwright browsers under the sandbox. Writes are `allowOnly` and the browser cache is not in it: mkdir ~/Library/Caches/ms-playwright/probe -> Operation not permitted Probing further turned up a second blocked cache with the same shape: `~/.npm/_cacache`, which breaks every `npm install`/`npm ci`. Neither failure names the sandbox — npm blames "root-owned files" and sends you to `sudo`, and the Playwright step is bounded by a timeout that blames a slow download. Both caches are now in `allowWrite`, both are probed by `verify-sandbox.sh`, and both scripts say what the error actually means. 2. `e2e/ci-wizard-settings.json` was tracked but is pure runtime scratch: `run-e2e.sh` and all 14 wizard steps in `ci.yml` truncate it to `{}` before mounting it, so nothing ever read the committed content, while the container wrote the wizard's result back through the read-write mount. Untracked and gitignored; no consumer changes. The real `ci-bess-settings*.json` fixtures still write back — documented, not changed. 3. `git push` no longer prompts. The blanket ask existed because prefix globbing cannot reach a marker at an arbitrary argument position, but that was compensating for having no guard at the layer that can see a ref update. Four GitHub rulesets now refuse those spellings server-side (main, beta-release-*, all tags, and beta's default branch), all with empty bypass lists — load-bearing, since local pushes authenticate as the owner, not as bess-agent. Feature branches stay unprotected on purpose. `quality-check.sh` pins push in MUST_NOT_BE_GUARDED so the prompt cannot be reinstated out of caution. Sandbox settings are captured once at session start, so the allowWrite change takes effect in a fresh session; `verify-sandbox.sh` confirms it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016tGBHZEAfKzP9qoJU4NdNY * docs: correct the `git push -u` advice — it cannot set upstream under the sandbox The previous bullet recommended `git push -u` as the way to record an upstream after `checkout -b --no-track`. It cannot: `-u` writes `.git/config`, which the same denial covers. Measured while pushing this branch — the ref lands and the command then reports "unable to write upstream branch configuration", which reads as a failed push and is not one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016tGBHZEAfKzP9qoJU4NdNY * docs: stop calling the Playwright hang "the #556 hang" #556 is a CLOSED issue about caching worktree dependencies -- the work during which the hang was first observed, not a ticket tracking it. The existing comments in worktree-setup.sh say that correctly ("observed while working #556"); the shorthand added in the previous commit did not, and pointed readers at an unrelated closed issue. Replaces it with the measurement instead: 1.59.1 stalls at exactly 84 files with the 173MB zip already downloaded and the extractor idle at 0% CPU, while 1.62.1 completes on the same machine. It is extraction, not the network, and there is no issue open for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016tGBHZEAfKzP9qoJU4NdNY * fix: address code review — probe failures that reported SKIP, and a false protection claim Five findings from /code-review on this PR, plus one from #637's review that belongs in this file. verify-sandbox.sh - Both new cache probes printed SKIP, counting NO failure, when the cache directory could not be created -- which under the sandbox is precisely the blocked condition being tested. A fresh machine exited 0 reporting the config safe while npm and Playwright were both about to fail. "Cannot be created" is now folded into the same check() and FAILS. - PLAYWRIGHT_BROWSERS_PATH=0 is Playwright's documented sentinel for "no shared cache", not a path. Treating it as one ran `mkdir -p 0` inside the repo and reported a PASS that measured nothing. Now skipped explicitly. quality-check.sh / CLAUDE.md - The comment claimed GitHub refuses `git push origin --delete release-X.Y`. It does not: the live rulesets cover ~DEFAULT_BRANCH, beta-release-* and tags only, so `release-X.Y` -- the stable hotfix branch the release skill pushes and tags -- is unguarded at BOTH layers. Documented as a known residual with the fix named (a release-* ruleset, not an ask rule) rather than left as a false claim. Creating that ruleset needs approval, so it is not done here. - The feature-branch residual was justified as "damage bounded to a branch nobody has merged". That does not survive this repo's own conventions: ~20 worktrees push in parallel as the SAME identity, so a misaimed --force destroys another agent's commits and closes its PR, and the reflog that would recover it is in a different worktree. Still accepted, now for the real reason. worktree-setup.sh - Sharing compared the two LOCKFILES but never checked that the tree being shared was installed from the lockfile beside it. A dependency bump merged to main updates main's package-lock.json and not its node_modules, so every new worktree would share a tree built from the old lockfile while this script reported success -- which is how the Playwright bump (#637) would have silently reintroduced the very install hang it fixes. Adds install_matches_lockfile(), comparing package-lock.json against node_modules/.package-lock.json. Optional deps are excluded: npm legitimately omits them, and counting that as staleness made a freshly installed e2e tree report STALE over `fsevents` alone -- which would push every worktree through a full install, the exact cost the sharing exists to avoid. Verified against real trees: main/e2e MATCHES, bump worktree MATCHES, and a simulated post-#637 state (1.62.1 lockfile beside a 1.59.1 install) correctly reports STALE. It also flags main/frontend as genuinely stale -- 84 real version mismatches, so that one wants an `npm ci`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016tGBHZEAfKzP9qoJU4NdNY --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Playwright 1.59.1 cannot install its browsers on this machine. The failure is in extraction, not the download — which is why waiting it out never worked, and why
worktree-setup.sh's timeout message ("made no progress") sends readers to the network.The measurement
chromium-1217chromium-1234Same machine, same network, minutes apart. In every stalled run:
$TMPDIR/playwright-download-*sampleing the process showedoopDownloadBrowserMain.jsidle at 0% CPU, every libuv worker parked in__psynch_cvwaitSo it is not throttling, not TLS, and not the sandbox — the fetch completes and the unpack never finishes.
Why it matters
This is what blocks
scripts/worktree-setup.sh. It kills the install at its timeout and exits 1, so a fresh worktree never gets browsers. With this bump the script runs to completion.Verification
Not just the install — the real stack, via
e2e/run-e2e.sh:--project=chromium)--project=wizard)scripts/quality-check.sh— 0 errors, 0 warnings.One thing to know when running this locally
The suite must be run with
MOCK_HA_PORT=8123.e2e/tests/mock-ha.spec.ts:3ande2e/tests/health-recovery.spec.ts:14hardcodehttp://localhost:8123, whiledocker-compose.ci.yml:55publishes${MOCK_HA_PORT:-8123}andrun-e2e.shderives that port from the worktree directory name. CI leavesMOCK_HA_PORTunset so it lands on 8123 and passes; any worktree whose name hashes elsewhere fails 7 tests withECONNREFUSED ::1:8123.That is exactly what the first run here did (7 failed / 63 passed). Forcing the port turned it into 70/0 with no code change, which is also what rules the bump out as the cause.
Pre-existing and unrelated, so not fixed here. Two further local-runnability gaps turned up alongside it, also untouched:
run-e2e.shcallsdocker compose; on podman it needsDOCKER_HOSTpointed at the machine socketrun-e2e.shuses GNUtimeout, which macOS does not shipHappy to fix all three in a follow-up if you want local
run-e2e.shto work out of the box.