Repository navigation
Conversation
…in the measurements lane
"real broker keeps 100,000 joined channels and delivers outside the first
batch" asserts wall-clock time: about 20 s of single-threaded CPU has to
finish inside a 30 s vi.waitFor budget. Vitest hashes files to shards, so
the test always shared shard 2/6 with InboxPage.test.tsx and
vite-config.test.mjs, under two workers. Main passed at 22.5 s; the two
failing jobs reached the budget at 33.9 s and 32.8 s
({"status":"connected","requests":50|67,"sockets":1}: expected +0 to be
100000), and their whole shard ran 440-446 s against 366 s on main.
Locally the test took 18 s unthrottled and failed 2 of 3 at ~35 s under
CPU throttling. The budget is a documented performance contract and is
not raised.
Move the body into joinedBeyondFirstBatch(count, batches), shared by two
tests in the same file:
- "real broker delivers outside the first joined batch" joins
LIVE_BATCH_SIZE + 1 channels and runs in the shared shards. It keeps
the behavioural contract: one socket, the first live-only REQ names a
full batch that excludes the last channel, an event in that channel is
delivered, and swapping it for a new channel establishes the new one
and retires the old route. The lone overflow channel replays inside
its own REQ (one limit-500 filter per channel, as live.ts sends a
batch that fits one replay REQ), so it expects one live-only batch
REQ, not two.
- The 100,000-channel check keeps its 49 live-only REQs, the <= 1024
open-wire check and its 30 s / 60 s budgets, gated with
test.skipIf(!process.env.BUZZ_BROKER_SCALE) like the
CLAUDE_PROTECTION_BIN gate in claude-code/protection.live.test.ts.
Run the gated check as a step of the Browser measurements job, which
already runs serially on an isolated runner and is already in CI
required's needs, with BUZZ_BROKER_SCALE set and -t "100,000" through
ci-test-report.mjs; its report joins the job's evidence artifact under
test-results/broker-scale. A title filter that matches nothing is a
green run of zero tests for both vitest (exit 0, 56 skipped) and
ci-test-report.mjs (complete), so the step then requires
numPassedTests === 1. docs/contributing.md lists the step under the
lane. The required gate is unchanged: no job was added, and the
measurements result it already checks now carries the scale check.
Measured on an idle 32-core Linux host, Vitest 5.0.4, this checkout:
- Scale check alone: before 14.8 s (vitest run -t "100,000", 15.3 s
total); after, with the exact lane command, 15.0 s test time, 16.6 s
elapsed, 1 passed / 55 skipped, guard exit 0. Same work, so the time
is unchanged; the lane adds the isolation.
- Whole file, as a shared shard runs it: before 22.2 s (55 tests);
after 7.5 s (55 passed, 1 skipped). The new case passed 10 of 10
fresh runs at 231-277 ms.
- Guard: with the gate unset, -t "100,000" exits 0 with 56 skipped and
the guard exits 1.
Hosted before, from the last green main run (43166170448): Browser
measurements 10 m 34 s, JavaScript (2/6) 7 m 31 s. The lane gains about
the scale check's run plus vitest startup; shard 2's drop is confirmed
only by the job summary once CI runs this change. Biome is clean on the
test file and the workflow parses.
Not addressed here: InboxPage.test.tsx shares the same slow shard with no
race found and needs fixture-cost profiling rather than a timeout change.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: mtoohey <mtoohey@squareup.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
kalvinnchau
reviewed
Nov 13, 2026
kalvinnchau
left a comment
kalvinnchau
left a comment
Contributor
There was a problem hiding this comment.
🤖 One test failure-isolation issue found.
… list
1db16e62d added the Broker scale check step to the Browser measurements
job and added test-results/broker-scale to the Measurement evidence
upload, but tests/integration/browser-ci.test.mjs pins that step's path
list. The "classic-scrollbar cases run exactly once, after the
measurements, without clearing their evidence" case therefore failed in
the Rust and tool integration lane (All Node integration tests), which
CI required depends on:
actual: [ 'test-results/broker-scale', 'test-results/browser',
'test-results/browser-classic-scrollbars',
'test-results/perf' ]
expected: [ 'test-results/browser',
'test-results/browser-classic-scrollbars',
'test-results/perf' ]
Add test-results/broker-scale to the expected list. The workflow is
unchanged; this is a test-expectation change only.
That assertion is the only pin of the measurements job's steps or paths.
The rest of the file names that job only in the required gate's needs
list and MEASUREMENTS result, which 1db16e62d did not change, and no
other test, script or check reads .github/workflows/ci.yml or names the
job's steps, artifact or evidence directories; the other workflow tests
under tests/integration read desktop-build.yml, release.yml and
desktop-stable.yml.
Verified on macOS with Node v24.18.0, running only this file:
`./bin/node --test tests/integration/browser-ci.test.mjs`
- before: 6 passed, 1 failed, with the assertion above.
- after: 7 passed, 0 failed (231 s).
Biome is clean on the file. Not run: the lane's other integration test
files, the Linux container the lane uses in CI, and any build.
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…s own timeout
"real broker delivers outside the first joined batch" ran under Vitest's
default 5 s test timeout, while the helper it shares with the
100,000-channel check, joinedBeyondFirstBatch, polled for up to 30 s in
its establishment wait and again in its swap wait. When channel
establishment regressed, the test timed out before the helper's finally
block ran. The helper kept polling, held its fetch stub, subscription
and server, and unstubbed globals only later, during a following test.
Reproduced by limiting production joined subscriptions to the first
batch (the loop that adds batch routes in src/features/relay/live.ts
stopped after one batch), running only this file:
- the small case failed with "Test timed out in 5000ms" at 5002 ms;
- "broker renewals retain establishment signals while channel status
stays live", which runs next, failed with "Local HTTP fixture did not
reach expected state";
- 53 passed, 2 failed, 1 skipped.
The helper now takes the timeout of those two waits from its caller:
- The small case passes 5 s, the allowance the default test timeout
gave it as a whole, and sets a 20 s test timeout. Its longest failing
run is both budgeted waits plus the 1 s default of the delivery wait
between them, 11 s, so a failed wait throws and the cleanup finishes
inside the test.
- The 100,000-channel check passes 30 s and keeps its 60 s test
timeout, its 49 live-only REQs and its open-wire ceiling. Its
behaviour is unchanged, and so are the workflow and the gate.
The helper's other waits were checked and left alone: the delivery wait
uses vi.waitFor's 1 s default, and the session fetch, the socket
delivery and the harness close are single local operations, not polls.
With the same mutation after the change, the small case fails on its
own assertion at 5013 ms ({"status":"connected","requests":11,
"sockets":1}: expected 2048 to be 2049), the renewal test passes, and
the file reports 54 passed, 1 failed, 1 skipped. A temporary test-only
edit that made the swap wait unsatisfiable failed the same way at
5122 ms, again with 54 passed. Both edits were reverted; this commit
changes only the test file.
Verified on macOS under load, Node v24.18.0, Vitest 5.0.4, running only
this file:
- `vitest run browser-host/relay-broker-live.test.mjs`: 55 passed,
1 skipped, before and after; the small case takes about 170 ms.
- `BUZZ_BROKER_SCALE=1 vitest run browser-host/relay-broker-live.test.mjs
-t "100,000"`: 1 passed, 55 skipped, 14.0 s test time.
- Biome is clean on the file.
Not run: the wider suite, the Linux container CI uses, and the
measurements lane's own report command.
Not addressed: the scale check's own waits can add up to 61 s against
its 60 s test timeout, as before this branch. Those budgets are a
performance contract and the lane runs that test alone, so they are
left as they are.
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
kazemek21
approved these changes
Nov 13, 2026
kazemek21
left a comment
kazemek21
left a comment
Contributor
There was a problem hiding this comment.
Kyle reviewed this PR and the Nexus review-tracker notes and approved with no comments.
Posted by Codex (gpt-6.1-sol) on behalf of kylez
wesbillman
reviewed
Nov 13, 2026
wesbillman
left a comment
wesbillman
left a comment
Collaborator
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, at ac0b1d270d02adc7ae33c5b841efd52a2441237a against base/merge-base 7f03764beb8eb0ddcb4ff9ec0b8bda3e07699716.
No new actionable findings. The shared helper retains the scale assertions, delivery beyond the first batch, channel replacement and wire ceiling. The isolated step remains required through the measurements job, checks exactly one passed test and preserves its report artifact. The earlier timeout-cleanup P3 is addressed in source: the small case’s 5-second waits now fit within its explicit 20-second test timeout with cleanup headroom.
I inspected hosted CI run 43378285977: the scale test passed once in 14.37 s (16.66 s command wall time). The broker file in JavaScript shard 2 had 55 passes and one scale-test skip; the small overflow case passed in 178 ms. The measurements job took 8m32s and shard 2 took 6m00s. Shard 2 reported 3,582 passes/12 skips, 289.60 s command wall time and 420.47 s summed test execution; its slowest file remained InboxPage.test.tsx at 110.37 s.
Source-only review; no local tests or failure-injection checks were run. That hosted run tested merge 60255d20 with this head and earlier base 7c786bb4, not the current target-base combination. A matched before/after timing comparison and failure-isolation behavior remain unverified.
This branch has not been deployed
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.
Why
browser-host/relay-broker-live.test.mjs, "real broker keeps 100,000 joined channels and delivers outside the first batch", times out intermittently in the shared JavaScript shard.This PR edits
.github/workflows/ci.yml. It adds one step to the existing Browser measurements job. The required gate is unchanged: no job was added or removed, and the measurements result thatCI requiredalready checks now carries the scale check.Root cause
The test asserts wall-clock time: about 20 s of single-threaded CPU has to finish inside a 30 s
vi.waitForbudget. Vitest hashes files to shards, so the test always shared shard 2/6 withInboxPage.test.tsxandvite-config.test.mjs, under two workers.mainpassed at 22.5 s.main.The 30 s budget is a documented performance contract, so it is not raised.
Change
The test body moves into a helper shared by two tests in the same file.
test.skipIf(!process.env.BUZZ_BROKER_SCALE), the same pattern the Claude Code protection live test uses.The gated check runs as a step of the Browser measurements job, which already runs serially on an isolated runner. The step sets
BUZZ_BROKER_SCALE, filters by title throughci-test-report.mjs, and uploads its report with the job's evidence artifact. A title filter that matches nothing is a green run of zero tests, so the step then requires exactly one passed test.docs/contributing.mdlists the step under the lane.No budget, assertion or failure path was weakened, and no engine was skipped.
Evidence
Measured locally on an idle 32-core Linux host, Vitest 5.0.4.
vitest run -t "100,000", 15.3 s total)-t "100,000"Hosted, before, from a green
mainrun: Browser measurements 10 m 34 s, JavaScript (2/6) 7 m 31 s. Biome is clean on the test file and the workflow parses. The pre-push type check and the file's related unit tests passed on this branch's head.tests/integration/browser-ci.test.mjspins the upload paths of the Measurement evidence step, in the case "classic-scrollbar cases run exactly once, after the measurements, without clearing their evidence". The second commit addstest-results/broker-scaleto that expected list. It changes the test expectation only; the workflow is as in the first commit../bin/node --test tests/integration/browser-ci.test.mjs, macOS, Node v24.18.0That assertion is the only pin of the measurements job's steps or paths. The lane's other integration test files and the Linux container the lane uses in CI were not run locally.
Not confirmed, not run
buzz-review-completedis not set.Left alone
InboxPage.test.tsxshares the same slow shard with no race found. It needs fixture-cost profiling rather than a timeout change.ci.yml; feat(pages): add shared editing for people and agents #2604 and Use relay sidebar API for unread and read-state authority #427 edit the test file; fix(desktop): run just desktop on the native code path #1547, Verify Git tree parsing and plugin release admission #1140 and Use relay sidebar API for unread and read-state authority #427 editdocs/contributing.md. A trial merge of each against this change is clean.🤖 Generated with Claude Code