Repository navigation
Make the browser acceptance deterministic under CI load - #76
Merged
Merged
Conversation
Two independent faults made the local Worker browser acceptance flaky in CI while it passed locally. First, a port collision. The smoke step kills only the `bun run dev` wrapper, and Wrangler releases port 8787 a moment later. The acceptance then spawned its own Worker with output discarded and never checked that it was alive, so its health poll passed against the smoke step's Worker while that one was shutting down. Journeys later failed with a page fetch error or a missing Ready status depending on when the stranger died. The script now refuses to start if anything answers on its port, tracks the Worker's exit and names it in the failure, and the smoke step waits until the port is released before ending. Second, a real client defect that the scenario's deliberate 8x CPU throttle exposes. `useAgentChat`'s stale client-tool-result cleanup effect dispatched a functional setState on every messages change even when it had nothing to prune. While a client falls behind a streamed turn, React cannot bail that update out eagerly, so every synchronous store commit ends with a pending update and React's nested-update counter climbs until it throws #185; the AI SDK swallows the throw into chat `error` and the resumed turn shows "Something went wrong". Reproduced locally at 16x and 32x with an instrumented react-dom; upstream tracks it as cloudflare/agents#2217 and has not shipped a fix (agents@0.22.0 and main still carry the effect). A `bun patch` dispatches only when a stale entry exists. With it, the throttled replay reaches Ready at 16x, 24x, and 32x instead of erroring. The scenario's turns now wait with an explicit bound measured on the production build (3.7s at 16x, 10s at 24x, 16s at 32x on an M-series laptop) instead of Playwright's default five seconds; the assertions themselves are unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
Author
|
CI on this branch: run 34486682158 passed on four consecutive attempts (14:06, 14:09, 14:13, and the fourth), each with the browser acceptance step green in 65–80 s. On |
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
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.
Summary
The local Worker browser acceptance failed three times in CI on 2026-09-10 while passing locally and on reruns. Two independent faults, both reproduced and fixed at their real boundary. One of the fixes is a dependency patch and needs Stephen's explicit approval per the fleet AGENTS.md rule on patching upstream internals; see the last section.
1. Port collision between the smoke step and the acceptance
The smoke step kills the
bun run devwrapper and returns as soon as it exits; Wrangler releases port 8787 a moment later. The acceptance then spawned its own Worker with output discarded and never checked it was alive. Locally reproduced: a secondwrangler devon a bound port dies with "Address already in use", so the acceptance's health poll passed against the smoke step's Worker while it shut down. Depending on when that stranger died, the journey failed withnet::ERR_CONNECTION_REFUSED, "Failed to fetch" (CI attempt 2), or a missing Ready status.scripts/agent-studio-browser-acceptance.ts: refuses to start if anything answers on the port ("Port 8787 is already in use; the browser acceptance must start its own local Worker"), tracks the Worker's exit, fails the health wait or the running journey with the Worker's exit reason, and keeps only the bind-failure signal from Wrangler's stderr..github/workflows/ci.yml: after the smoke, the step waits until nothing answers on 8787 (bounded at 30 s) before ending.2. React #185 in the throttled resume scenario (real client defect)
The resumed-history scenario throttles its page 8x on purpose. Under load it ended in "Something went wrong". With an instrumented
react-domdev build the chain is: each replay chunk causes a synchronous store commit; after every commit,useAgentChat's stale client-tool-result cleanup effect dispatchessetClientToolResults(prev => …)even when it prunes nothing; React cannot bail that update out eagerly while the fiber has pending work, so each commit ends with a Default-lane update pending and the nested-update counter climbs (no reset) until the 51st chunk throwsMaximum update depth exceeded. The AI SDK catches it into chaterror. Reproduced at 16x and 32x on this laptop; at 8x the throttle batches chunks so the counter never gets there, which is why CI sat on the edge.Upstream tracks exactly this as cloudflare/agents#2217 (open since 2026-09-04, same diagnosis and fix);
agents@0.22.0and currentmainstill carry the effect, so there is no version to upgrade to. The patch mirrors the map in a ref and dispatches only when a stale entry exists; the updater still recomputes fromprev. With the patch the replay reaches Ready at 16x, 24x, and 32x.3. Measured bound for the throttled scenario
Ready after the 100-chunk replay on the production build with the patch: 3.7 s at 16x, 10 s at 24x, 16 s at 32x. CI runners are slower than this laptop, so 8x there lands in that range, past Playwright's default 5 s. The scenario's turns now wait up to 60 s with a comment recording the measurements. The assertions are unchanged: Ready, the exact full replayed and continued text, no page errors, one acknowledgement.
Verification
bun install --frozen-lockfileapplies the patch;bun run checkpasses (469 backend, 287 frontend, 22 tool tests, build, staging dry run).bun run test:browser -- --no-buildpasses at the scenario's 8x, and an instrumented copy passes at 24x (Ready 10 s after replay).Needs Stephen's approval
patches/agents@0.20.1.patchchanges upstream hook internals. Behavior changed: the cleanup effect no longer schedules a React update when there is nothing to prune. Why supported extension points cannot do it: the effect lives insideuseAgentChatand runs on every messages change; no option disables or replaces it, andexperimental_throttleonly lowers the odds (upstream's own advice on #1732), it does not remove the accumulator. Impact: none on tool-result rendering (the updater is unchanged when it runs); upstream's own regression test noted the replayed prefix paints a few tens of milliseconds later without the accidental extra render. Upgrade path: when bumpingagents, check whether #2217 has landed and drop the patch. AGENTS.md now records both patches.🤖 Generated with Claude Code