Repository navigation
Delete a wall-clock bound that could never have fired - #293
Merged
pixexid merged 1 commit intoAug 19, 2026
Merged
Conversation
The checkout-adversary test asserted `Date.now() - started < 2_000`, in the file GH-161 measured inflating 34x between local and CI. It was not a proxy for a real property and it was not a needed smoke check: it was unreachable. Mutating plugin load to resolve the checkout eagerly does not fail the test at 2,000 ms, or at the explicit 5,000 ms timeout, or at the 15 s suite default PR #286 introduced. `readHead` calls readFileSync on the adversary's FIFO `.git/HEAD`, which blocks the worker synchronously — nothing downstream of the await runs and vitest cannot interrupt it. The run was killed at 90 s having emitted no test output at all. Removing the assertion reproduces that mutation result exactly, which is the proof it was carrying nothing. What proves the invariant is `resolves.toEqual([undefined, undefined])`: it checks the value as well as the termination, and it is not weather-dependent. The explicit `}, 5_000)` goes with it, so the test inherits the suite default rather than keeping a second hand-chosen number in this file. Closes #287
pixexid
marked this pull request as ready for review
August 19, 2026 03:09
pixexid
deleted the
bb/gh-287-checkout-adversary-wall-clock-thr_2rbixgj33q
branch
August 19, 2026 03:09
This was referenced Aug 19, 2026
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.
The assertion was option 2 — a smoke check the resolution already covers — but the reason is sharper than "redundant": it was unreachable.
expect(Date.now() - started).toBeLessThan(2_000)sat in the file GH-161 measured inflating 34x between local and CI, with an explicit}, 5_000)opting it out of the 15 s suite default PR #286 introduced.Why it is not option 1
The invariant is laziness:
readDiagnosticDivergenceis a closure defined at load and invoked only by thedoctorRPC and CLI. So the real property is "plugin load performs no checkout I/O", and the wall-clock bound is a proxy for it.Mutating plugin load to call
readDiagnosticDivergence()eagerly does not fail at 2,000 ms, at 5,000 ms, or at 15 s.readHeadcallsreadFileSyncon the adversary's FIFO.git/HEAD, which blocks the worker synchronously — nothing downstream of theawaitruns and vitest cannot interrupt a blocked worker. The mutated run was killed at 90 s having emitted no test output at all.An assertion that control never reaches is not a proxy for anything, so there is no real property to swap it for and no observable counter to add to production source.
Mutation evidence
Identical mutation behaviour with and without the assertion is the proof it was carrying nothing.
What proves the invariant instead
resolves.toEqual([undefined, undefined])— it checks the returned value as well as termination, and it is not weather-dependent. The explicit}, 5_000)goes with the assertion so the test inherits the suite default rather than keeping a second hand-chosen number in this file.No production source changed. No other test touched.
npm run verifyexit 0 — 367 passed, 2 skipped.Closes #287
Acceptance: complete
Review tier: C