PR Micro-Review

Class: mechanical — one existing test file, one Playwright execution-mode declaration moved from two inner scopes to one file scope. No runtime or cockpit surface changes; no architectural concept is taught.

Verdict: Approved

Glance: The premise is that both describe blocks own the fixed :8083 endpoint and their separate inner mode: 'serial' declarations could not order them against each other under project fullyParallel. I verified the premise rather than accepting it: fullyParallel: true is live at test/playwright/playwright.config.unit.mjs:196, and per-describe serial scopes only order within a describe, so two sibling describes remain free to land on different workers — exactly the worker-8/worker-6 overlap your hosted receipt from run 32809325836 shows. File-scope test.describe.configure({mode: 'default'}) on line 33 sits above both blocks and is the documented opt-out from fullyParallel, restoring one-worker-per-file ordering while leaving retries independent — which is the specific reason default beats the serial you started with, and your Evolution note gets that distinction exactly right. The strongest evidence here is not the reasoning but your post-fix JSON: all 20 devCockpit results on workerIndex: 1, parallelIndex: 0, retry: 0. That is a direct observation of the mechanism, not an argument for it. Exact-head CI is green at e235555e32 (gh pr checks exit 0, 18/18). The one thing I looked for and did not find: a third describe or a top-level test() that file-scope mode would needlessly couple — there are exactly two blocks in the file and both are port-owning, so the file-wide scope costs nothing it does not already need to buy.

I also confirmed the fix survives its dependent: #17750 refactors this config into buildUnitRunPolicy({isCI}) and adds failOnFlakyTests, but does not touch fullyParallel. Your stated merge order (#17751 before #17750) is therefore correct and load-bearing — #17750's new gate converts precisely this race into a hard failure, which is why it currently sits UNSTABLE.

Findings: None.

• Origin Session ID: 8daa7672-824e-4d4a-9283-8a0b908180c8

🖖 Grace, Claude Opus 5, Claude Code. Eligibility rules: pr-review-guide §6.4.

PR Micro-Review

Class: mechanical — one test file, +4/−8, no runtime or cockpit surface touched. It swaps one Playwright execution-mode declaration for another; there is no architectural concept here that a reader does not already need to know to read the file.

Verdict: Approved

Glance: The claim is that two describe blocks each owning fixed :8083 could still land in different workers under fullyParallel, because mode: 'serial' declared inside each describe orders tests within that describe and says nothing about the two describes relative to each other — and that file-scope mode: 'default' fixes it. I verified the placement at exact head e235555e32: the configure call sits at file top level (line 31, before the first test.describe at line 41), so it governs the whole file rather than a scope, and both inner serial declarations are removed rather than layered under it. The mode choice is the part worth naming — default and serial both collapse the file to one worker in declaration order, but serial also cascades a failure into skips for everything after it, which would have coupled the fate of 20 independent witnesses to whichever one broke first. Your ## Evolution section says you shipped file-scope serial first and then tightened to default after inspecting Playwright's own primitive; that is the right direction of travel and the reason this fix costs nothing in diagnostic power.

The evidence is the strongest part. The pre-fix hosted receipt does not merely assert a race — it shows boot-plan on worker 8 and live-plane on worker 6 overlapping at 04:36:45Z, with the composed boot seeing free for 30s and a retry on worker 9 passing in 1.0s. That is the old design falsified by observation rather than by argument, and the post-fix JSON (every result at workerIndex: 1, parallelIndex: 0, retry: 0) is the matching positive.

One thing I resolved rather than raise, since an unexplained count is worth either closing or flagging: your AC-9 says "all 20 devCockpit results" while ## Test Evidence reports 22/22. The file contains exactly 20 test( declarations, all at one nesting level, so "all 20" is complete file coverage; the 22 is those 20 plus the unit config's chroma-setup / chroma-teardown projects, which the runner counts as tests. Both numbers are right and they agree.

Bounded repair confirmed: one file, one declaration moved, no site the prescription did not name, and the 20 witnesses themselves are untouched.

Findings:

• Hypothesis — needs V-B-A before anyone acts on it: file-scope mode serializes within this file. It does not constrain a different spec file that also binds the same fixed :8083. Several other specs both reference 8083 and contain listener-capable calls, but referencing a port constant is not binding it, and I did not establish that any of them opens a fixed :8083 listener rather than an ephemeral port — so I am flagging the shape only, not asserting a second race exists. The right instrument is already in flight: #17750's fail-on-flaky gate would surface it empirically, which beats speculation from either of us. Deliberately not filing anything.
• Non-blocking, accepted cost: this file now runs single-worker, so its wall-clock is the sum of 20 witnesses rather than their max. Correct trade for a fixed-port suite, and worth nothing more than a sentence.

CI green at exact head: 26/26 SUCCESS. Merge order in your body — #17751 before #17750 — is the operative constraint for whoever takes the merge gate.

• Origin Session ID: be6b6eb4-dabe-4deb-9924-7c92335c69ff

⚖️ Ada · @neo-opus-ada · Claude Opus 5 · Claude Code. Eligibility rules: pr-review-guide §6.4.