Repository navigation
Conversation
Co-authored-by: Codex <[email protected]> Signed-off-by: jm <[email protected]>
Co-authored-by: Codex <[email protected]> Signed-off-by: jm <[email protected]>
Co-authored-by: Codex <[email protected]> Signed-off-by: jm <[email protected]>
matt2e
added a commit
that referenced
this pull request
Nov 14, 2026
…in the measurements lane (#3447) ## 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. | Sighting | Lane | |---|---| | [7 Nov, PR, passed on re-run](https://github.com/block/buzz-app/actions/runs/42415071330/job/128531273095) | JavaScript (2/6) | | [9 Nov, PR, passed on re-run](https://github.com/block/buzz-app/actions/runs/42600556170/job/129103660721) | JavaScript (2/6) | **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 that `CI required` already 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.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, with the broker connected and no channels reported joined yet. - Their whole shard ran 440 to 446 s against 366 s on `main`. - Locally the test took 18 s unthrottled and failed 2 of 3 at about 35 s under CPU throttling. 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. - **"real broker delivers outside the first joined batch"** joins one channel more than a live batch 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, so this case expects one live-only batch REQ, not two. - **The 100,000-channel check** keeps its 49 live-only REQs, its open-wire ceiling and its 30 s and 60 s budgets. It is gated with `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 through `ci-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.md` lists 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. | Measurement | Before | After | |---|---|---| | Scale check alone | 14.8 s test time (`vitest run -t "100,000"`, 15.3 s total) | 15.0 s test time, 16.6 s elapsed with the exact lane command; 1 passed, 55 skipped, guard exit 0 | | Whole file, as a shared shard runs it | 22.2 s, 55 tests | 7.5 s, 55 passed and 1 skipped | | New behavioural case | | 10 of 10 fresh runs passed, 231 to 277 ms | | Guard with the gate unset, `-t "100,000"` | | vitest exits 0 with 56 skipped; the guard exits 1 | Hosted, before, from a green `main` run: 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.mjs` pins 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 adds `test-results/broker-scale` to that expected list. It changes the test expectation only; the workflow is as in the first commit. | Integration test file | Before the second commit | After | |---|---|---| | `./bin/node --test tests/integration/browser-ci.test.mjs`, macOS, Node v24.18.0 | 6 passed, 1 failed on the upload-path assertion | 7 passed, 0 failed | That 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 - Hosted timings after the change are not known yet. The measurements lane gains about the scale check's run plus Vitest startup. The drop in shard 2 is confirmed only by the job summary once CI runs this PR. Reviewers should compare both there. - The local timings are from Linux and are not hosted CI numbers. - No human has tested this yet, so `buzz-review-completed` is not set. ## Left alone - `InboxPage.test.tsx` shares the same slow shard with no race found. It needs fixture-cost profiling rather than a timeout change. - Open PRs #3050, #3046, #2883, #2836, #2014, #1140, #1105 and #873 also edit `ci.yml`; #2604 and #427 edit the test file; #1547, #1140 and #427 edit `docs/contributing.md`. A trial merge of each against this change is clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: mtoohey <[email protected]> Signed-off-by: Matt Toohey <[email protected]> Co-authored-by: Claude Fable 5.1 <[email protected]>
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.
Two Git bugs
The plugin importer removed symlinks and submodules before checking for signed-release files. A symlink named
plugin.signature.jsoncould therefore disappear from that check, allowing the package to be treated as unsigned. The importer now checks whether either release marker exists before deciding how to load the package. Unsigned plugins remain supported when both markers are absent.Native Projects also replaced invalid filename bytes with
�. This could make two different Git filenames appear identical and cause a request to return the wrong file. It now rejects filenames it cannot represent faithfully.Verus
Both readers now use
buzz-git-tree, a shared Rust implementation that preserves the original filename and object-ID bytes. Cargo executes the same function bodies that Verus checks.The proof connects package selection to the complete parsed tree: an unsigned result requires both release markers to be absent, and selected package files must be regular files in the same directory, within the size limit. Parsing and lookup preserve the association between each path and its original Git object.
Both bugs were reproduced on the old code. The regressions pass after these changes, and a faithful version of the old admission rule fails verification.
Validation and limits
CI passed at
93d6700, including a fresh 39 verified, 0 errors result. Linux and macOS regressions passed; five independent reviews found no blockers. Verification runs through Cargo in the existing Rust CI job.The proof covers parsing, lookup and package selection. UTF-8 conversion, Git I/O, signature checking and installation remain outside it; the Node development reader is unchanged. Extra tree metadata can reach the existing output-byte limits sooner. The PR remains draft pending human native testing and reviewer approval.