Compare admission-model refusal time to the clock value, not its last reading - #393
Merged
Merged
Conversation
The model asserted a justified refusal had consumed a clock reading at or
past the bound. Simulated time also advances without a reading: sleeps add
their delay and the test commit hook adds commitDelayMs. A one-millisecond
budget can therefore expire on the deadline's field check before any
subsequent reading, leaving lastRead at 0 while now is already past the
bound. CI seed 797548962 reached exactly that: acquire({timeoutMs:1,
captureRemainingMs:1, increments:[], commitDelayMs:1}) refused correctly
at now=1 with lastRead=0, and the assertion rejected a legitimate refusal.
Compare now instead, which is what the deadline itself observes; a refusal
while a slot was free is still unjustified unless simulated time reached
the bound.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was successfully 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
1 participant
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.
Summary
Fixes a wrong-field assertion in the browser-admission
fc.commandsmodel that intermittently failstest 6/8onmainand on every lane merged past #392.run()asserted that a refusal while a slot was available must have consumed a clock reading at or past the bound (lastRead >= bound). But simulated time advances without a reading:sleepadds its delay and the test commit hook addscommitDelayMsdirectly toclock.now. A 1 ms budget can expire on the deadline's field check before any subsequent reading —lastReadstill 0 whilenowis already past the bound.Observed on #391's
test 6/8, seed797548962, counterexampleacquire({"timeoutMs":1,"captureRemainingMs":1,"increments":[],"commitDelayMs":1}): the commit hook advancednowto 1, the capture deadline's field check fired before the nextnow()read, production refused legitimately — and the assertion rejected it atlastRead=0.The fix compares
real.clock.now, which is what the deadline itself observes. A refusal while a slot was free still must be justified by simulated time reaching the bound — the check is not weakened. Same correction at the capture-launch assertion (line 469).Test plan
lastReadonly updates inreadClock;sleep/afterCreateCommitForTestadvancenowfield-only;deadline.throwIfUnavailablechecks the field beforethrowIfUnavailablereadsnow()test 6/8passes (local replay blocked by host load; the failing schedule runs the real bound state helper)Generated with Devin