test: e2e_mcp_browser asserts a 2s wall-clock propagation budget on a debug build under pre-commit load (flaky) #1046

Problem

src/simlin-serve/tests/integration/e2e_mcp_browser.rs::mcp_edit_propagates_to_browser_within_one_second asserts a 2 second wall-clock budget on a window that includes a debug-build server's full edit/compile/write round-trip, and it runs inside the pre-commit hook's parallel cargo test. It fails intermittently on timing alone, after the propagation it exists to check has already succeeded.

Observed 2026-09-02 during the pre-commit hook run on the compiler-unification-v2 branch (PR #1040; the branch does not touch simlin-serve):

MCP edit -> browser WS propagation must be under 2s (AC5.3 budget + 1s slack); observed 2.24s

It passed on rerun. Nothing was wrong with the product: the projectChanged event had arrived and every field assertion on it (type, source=agent, path, version=1) had passed before the timing assertion fired.

Where the budget lives

All in src/simlin-serve/tests/integration/e2e_mcp_browser.rs:

• line 285: let edit_started = Instant::now(); -- the clock starts before the EditModel HTTP POST is sent.
• lines 313-317: timeout(Duration::from_secs(2), ws.next()) -- a 2 s wait-for-condition on the WS frame (this one starts only after the HTTP response returns).
• line 318: let propagation = edit_started.elapsed(); -- measured after WS receipt.
• lines 344-348: assert!(propagation < Duration::from_secs(2), ...) -- the assertion that fired.

So the measured window is: HTTP request -> server parses the fixture, applies the patch through apply_canonical_json, re-validates/compiles for the diagnostics path, serializes, atomic-writes, broadcasts -> HTTP response -> WS frame delivered. The dominant term is the server's own compute, and the server is a debug-build subprocess (CARGO_BIN_EXE_simlin-serve) sharing the machine with the rest of the run.

It is the only elapsed()-based assertion in the simlin-serve test suite (rg -n 'elapsed\(\)' src/simlin-serve/tests returns just this line). Every sibling WS/broadcast test uses a wait-for-condition timeout, which trips only when the event never arrives.

Why it flakes

The pre-commit hook (scripts/pre-commit:160) runs cargo test at the workspace root under a 180 s cap. libtest runs the ~150 tests in the simlin-serve integration binary concurrently across all cores, and several of those (smoke.rs, dual_port_smoke.rs, e2e_live_update.rs, this test) each spawn their own debug simlin-serve binary, alongside the FSEvents watcher tests. Under that contention a debug-build edit round-trip drifting past 2 s is not a regression signal; it is the expected variance of an unpinned CPU budget.

The repo's rule (root CLAUDE.md Hard Rules; docs/dev/rust.md "Test time budgets") is that tests stay well inside their budgets on a debug build. A sub-2 s hard wall-clock assertion inverts that: the test is designed to fail when the machine is slow.

The requirement being asserted is the wrong kind of requirement for this test

The AC5.3 the test cites is server-rewrite.AC5.3 in docs/design-plans/2026-04-05-server-rewrite.md:57:

A Claude desktop client configured against the URL can call list_projects, get_project, apply_edit, simulate, and create_project tools and observe results in the browser within one second

That is a product-level latency criterion for a human using a real client against a release binary on an otherwise idle machine. Asserting it from a debug-build integration test that runs under cargo test parallel load cannot establish it (a pass there says nothing about release latency) and cannot fail meaningfully (a fail there says nothing about a regression). The test's real value -- the MCP edit routes through the same merge primitive as the browser save handler and the WS subscriber observes ProjectChanged { source: agent, version: 1 }, and ReadModel then sees the new variable -- is fully carried by the ordering/field assertions and does not need a latency bound.

Note also that the test's own comment and code disagree: lines 307-312 say the assertion is "under 1s ... with 1s headroom" and the test name says within_one_second, while the code asserts < 2s and the message says "AC5.3 budget + 1s slack". Whichever fix lands should make name, comment, and assertion agree.

Why it matters

Same class as closed #968 (diagram Slate test flaky under parallel pre-commit load) and #916 (watcher tests, fixed timeouts vs FSEvents): a spurious red in the canonical pre-commit gate on a branch that is actually green blocks the commit (--no-verify is prohibited) and teaches contributors to re-run until green, which erodes the gate for every other test. This one is worse than a timeout flake because it fails after observing a correct outcome.

Components affected

• src/simlin-serve/tests/integration/e2e_mcp_browser.rs (lines 226, 285, 307-318, 344-348)
• Surfaces through scripts/pre-commit (Rust test step) and .github/workflows/ci.yaml.

Possible approaches

Not mutually exclusive; (1) is the recommended fix and is a few-line change.

1. Assert propagation, not latency. Delete the edit_started / propagation measurement and the < 2s assertion at lines 344-348. Widen the WS wait-for-condition at line 313 to a generous ceiling that only a genuinely broken broadcast path would hit (the file already accepts up to 15 s for startup; e2e_live_update.rs uses 5 s for the same frame). The ordering and field assertions that follow already pin the behavior the test exists for. Rename the test to mcp_edit_propagates_to_browser and rewrite the comment at 307-312 to match.

2. If a latency number is wanted, measure only the broadcast leg and do not gate on it. Start the clock after the EditModel HTTP response returns (excluding the server's edit/compile/write work, which is the debug-build- and load-dependent part) and print or log the value rather than asserting a bound. A real latency SLO belongs in a release-mode lane (ci: add a release-mode lane so C-LEARN-scale tests can be un-ignored #657) or a benchmark, where the binary and the machine conditions match the claim.

3. Sweep the same pattern in siblings while there. api_save.rs lines 906, 994, 998, 1053 use tokio::time::timeout(Duration::from_secs(1), rx.recv()) on an in-process broadcast channel. Lower risk (the broadcast is sent inside the handler before the response returns, so the message is normally already buffered), but 1 s is still a hard wall-clock on a debug build under load; widening them costs nothing on the passing path.

Relationship to existing tracking (duplicates checked)

• test: simlin-serve watcher integration tests are flaky on macOS (fixed wall-clock timeouts vs FSEvents latency) #916 (watcher integration tests flaky: fixed wall-clock timeouts vs FSEvents latency) -- different files (watcher_smoke.rs, watcher_merge.rs, diagnostics_events.rs), different mechanism (platform event delivery), and those are wait-for-condition timeouts, not an elapsed-time assertion on a delivered result. Fixing test: simlin-serve watcher integration tests are flaky on macOS (fixed wall-clock timeouts vs FSEvents latency) #916 via a deterministic event source would not touch this test.
• test: simlin-serve watcher test runtime scales with fseventsd health, can bust the pre-commit cap on passing tests #945 (watcher test runtime scales with fseventsd health, can bust the 180 s cap) -- about total runtime of passing tests, not a single test's latency assertion.
• test: diagram variable-details-cancel blur-commit test is flaky under parallel pre-commit load #968 (closed; diagram Slate test flaky under parallel pre-commit load) -- same failure class, different package.
• ci: add a release-mode lane so C-LEARN-scale tests can be un-ignored #657 (release-mode CI lane) -- where a genuine latency check for server-rewrite.AC5.3 could live.
• docs/tech-debt.md has no entry for this test or for simlin-serve wall-clock assertions (entry 37 is the deterministic macOS pre-existing-file watcher gap; entry 68 is pysimlin Hypothesis).

Discovery

Observed 2026-09-02 while the pre-commit hook ran for PR #1040 (compiler-unification-v2), whose changes are confined to simlin-engine. The test was introduced in PR #476 (commit 6be5ab7, "Add simlin-serve binary and refactor simlin-mcp into core library") and moved into the consolidated integration harness in b842996; the timing assertion has been in place since #476.