Skip to content

Verify Git tree parsing and plugin release admission - #1140

Draft
jmecom wants to merge 3 commits into
mainfrom
jm/verus-git-import
Draft

jmecom wants to merge 3 commits into
mainfrom
jm/verus-git-import

Conversation

@jmecom

@jmecom jmecom commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Two Git bugs

The plugin importer removed symlinks and submodules before checking for signed-release files. A symlink named plugin.signature.json could 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.

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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant