test(invariants): de-flake — seed fixture sampled detected_at after caller's pushed_at#99
Merged
Merged
Conversation
…aller's pushed_at
`invariants-runtime-proof.test.ts` intermittently failed the
`timestamps-monotonic` predicate ("returned 1 (want 0)") and passed on a
bare re-run (PR #98, run 29550755556).
Root cause is the seed fixture, not the predicate. JS evaluates argument
expressions before the call, so
seedItem({ state: 'pushed', pushed_at: Date.now() })
sampled `pushed_at` *before* seedItem's own `detected_at` read. When the
millisecond ticked between the two reads, `pushed_at` landed 1ms BEFORE
`detected_at` and tripped the invariant. Measured ~1 in 42k on an idle
machine (477/20M); a loaded runner's scheduler gap widens the window,
which is why re-runs went green. Six tests shared the pattern —
handleDismiss just drew the short straw.
The predicate is already `>=`-tolerant (strict `<` only), so equal stamps
never tripped it; loosening it would have masked a real invariant instead
of fixing anything. Production code is unaffected: every mutation path
stamps pushed_at/resolved_at at mutation time, always after insert.
Fix: the fixture derives its default `detected_at` from the earliest
stamp on the row, so a caller-sampled timestamp can't be post-dated. An
explicit `detected_at` still wins, keeping the time-reversal
counter-examples constructible. Fixture moved to a shared helper so the
regression test pins the real thing rather than a copy.
Verified: the regression test fails on the pre-fix fixture with the exact
CI assertion, and all 30 tests in the suite pass under a clock that ticks
on every read. Full suite unchanged vs origin/main (2113 vs 2111 passed,
0 failed; the 45 module-resolution failures are a local-node_modules
artifact present identically on main).
Same root-cause family as #92 (2026-07-01): two clock reads assumed to be
one instant.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
topcoder1
enabled auto-merge (squash)
July 17, 2026 04:14
|
No issues found. Test-only + docs: the |
|
Coverage Floor — mode:
|
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.
Fixes the intermittent
invariants-runtime-proof.test.tsfailure that has been blocking nanoclaw PRs on the requiredcicheck (seen on #98, run 29550755556; passed on a bare re-run).Root cause — the fixture, not the predicate
JS evaluates argument expressions before the call, so:
pushed_at(T0) was sampled beforedetected_at(T1). When the millisecond ticked between the two reads,pushed_atlanded 1ms beforedetected_atand trippedtimestamps-monotonic.Measured the inversion directly: 477 in 20,000,000 iterations (~1 in 42k) on an idle machine. A loaded runner's scheduler gap between the two reads widens that window, which is why bare re-runs went green and it read as "just flaky". Six tests shared the pattern (lines 136/142/150/163/169/254) —
handleDismissjust drew the short straw.The originally suspected fix would have been wrong. The predicate is already
>=-tolerant (it trips on strict<only), so equal timestamps never failed it — 99.998% of runs produce exactly equal stamps. Loosening it would have masked a real invariant while leaving the bug in place. Production code is unaffected: every mutation path stampspushed_at/resolved_atat mutation time, always after insert.Same root-cause family as #92 (2026-07-01 trust-graduation): two clock reads assumed to be one instant.
Fix
The fixture derives its default
detected_atfrom the earliest stamp on the row, so a caller-sampled timestamp can't be post-dated. An explicitdetected_atstill wins, keeping the time-reversal counter-examples constructible. Fixture moved to a shared helper so the regression test pins the real thing rather than a copy.Verification
predicate returned 1 (want 0)), passes after.handleDismissfail deterministically pre-fix. Covers all six susceptible tests, not just the one that failed.origin/mainbaseline: 2113 passed / 0 failed (baseline 2111 / 0). The 45 module-resolution failures appear identically on untouchedmain— a localnode_modulesartifact, not this change.prettier --checkandeslintclean on all touched files; no tsc errors in them.Codex rounds: 2 (zero findings both rounds; round 2 adversarially probed whether
Math.minmasks real violations, whether counter-examples still construct, and whether the regression test is tautological — all clear).Auto-merge rationale: safe — test-only + docs. Every changed file matches
safe_test(**/__tests__/**,tests/**) ortrivial(**/*.md) in.github/risk-paths.yml; none matchblocked:orsensitive:. No production code changes, so runtime behavior is unchanged.🤖 Generated with Claude Code