Fix vendor prefetch breaker-test flake and outage over-requests - #510

Problem

api::vendor_prefetch::tests::an_outage_mid_list_opens_the_breaker_at_the_same_package (socket-patch-core unit test) failed in test (macos-latest) on CI run 36878455088 (job 110423824031, branch agent/fix-yarn-berry-hosted-auth-leak, SHA 0c461dcd). That branch doesn't touch the prefetch:

panicked at crates/socket-patch-core/src/api/vendor_prefetch.rs:894:9:
assertion `left == right` failed: a mid-list outage must not be amplified by the plan either
  left: 16
 right: 12
(4675 passed, 1 failed)

A single failure in test turns the whole CI run red (about 100 jobs and about 215 job-minutes per attempt, with the windows/macos test legs taking 18–25 min), and yarn-classic-matrix, cargo-vex-matrix and others wait on it with needs: test.

Local reproduction (unchanged main, debug build, 4 vCPU): 0/200 when idle; 11/640 and 4/640 with 16 copies running in parallel plus 8 busy-loop CPU hogs. The failures were 14, 15 and 16 requests against 12, the same signature as CI.

Root cause

The test depends on timing, and the timing-independent version of the same scenario exposes a real over-request bug in the prefetch.

1. The test's race. The doc comment says "the task here is running ahead of a loop still on the granted packages". Nothing enforces that. It holds only if package 3's 503 retry ladder lands in the prefetch task before the mock server releases package 1's grant (a 20 ms delay). Under load the ladder is slower, so the loop reaches packages 1 and 2 first. With the barrier not yet set, the widened reach admits packages 5–7. That is the documented "loop caught up with the window" cost, which the strict assertion doesn't allow for.
2. The bug the race was hiding. I made the doc's premise deterministic: before each call, the loop waits until the task has seen the failure it is about to meet, the way the real loop pauses to write between packages. Under that pacing the plan sends 21 requests where serial sends 12, every time. Here is why. The loop consumes package 3's failure and calls package 4. Lookahead::arrive then lifts the barrier, but the reach is still 6, a size earned by answers that came before the outage. So packages 4, 5, 6 and 7 all start their retry ladders at once, against a service the loop's own breaker is one failure from giving up on. The module docs promise this case ("an outage part-way down a list … costs what the serial loop paid, as long as the task is running ahead of the loop") costs nothing extra. On a real outage, that means about three extra 3-attempt ladders hit the patch service per outage.

Fix

crates/socket-patch-core/src/api/vendor_prefetch.rs:

• Lookahead::arrive: when the loop moves past the failed position and the barrier lifts, the reach drops to 1. Only the loop's own position may be requested until a good answer reopens the reach (next_reach(1) is SLOW_START). Outcomes are unchanged, since every decision is still made at consumption time. The only thing removed is speculative requests against a failing service. The module and field docs are updated to say so.
• Test: a new run_paced helper waits on the lookahead's own moved notifier (30 s safety timeout) instead of on wall-clock delays. The test's request-count assertion is unchanged (planned == serial == 12), and so are its outcome and breaker-count assertions. Only the scheduling assumption they rested on now holds by construction.

No test removed, no #[ignore], no timeout raised, no retry added.

Proof

• Without the fix, the paced test fails 5/5 (left: 18/19/21/21/21, right: 12), so it now pins the bug.
• With the fix, the paced test passed 0 failures in 1600 runs (16 parallel copies plus 8 CPU hogs). Before, the same stress gave 4–11 failures per 640. The whole api::vendor_prefetch module (24 tests) gave 0 failures in 200 loaded runs.
• cargo test -p socket-patch-core --lib: 4715 passed. Four tests fail identically on unchanged main in this container because it runs as uid 0: relax_loop_must_not_traverse_symlinked_root, an_unremovable_hidden_lock_keeps_every_store_entry, wire_write_failure_maps_error_and_leaves_lock_untouched and wire_failure_rolls_back_already_written_files all expect permission errors.
• rustfmt --check is clean on the touched file. cargo clippy -p socket-patch-core --all-targets -- -D warnings reports nothing in vendor_prefetch.rs. The errors it does report are in maven_repo.rs, nuget_feed.rs, yarn_berry_lock.rs and bun_binary.rs, and they are the same on unchanged main with this toolchain.

Where the tests run

Nothing removed or moved. The test still runs in ci.yml's test (3 OSes), test-release and coverage jobs.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WDqizZLqhYTcaakGpnyv1r

Generated by Claude Code

Generated by Claude Code