fix: make veto-deadline recovery test deterministic#460
Merged
Conversation
threads_scheduler_recovers_veto_claimed_before_deadline staged a proposal with a real 2-second veto window and raced the wall clock twice: the interrupted reject had to durably record claimed_at within 2s of staging (slow Windows runners exceed this, so recovery correctly reported the veto window closed and the test failed), and a fixed 2.1s sleep had to outlast the deadline. Drive both conditions through durable state instead, following the sibling scheduler tests' idioms: stage with a 300s window backdated 10 minutes so the deadline has already elapsed (deleting the sleep), and pin the durable decisionRequest.claimed_at inside the window to model a timely claim. Recovery judges veto timeliness by the durable claimed_at alone, so the covered scenario — timely claim, post-deadline replay — is unchanged; it just no longer races the scheduler's clock. Test-only change; the test now runs in ~60ms instead of 2.1s+. Closes #455 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Val Alexander <68980965+BunsDev@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a flaky Windows CI test by making api::tests::threads_scheduler_recovers_veto_claimed_before_deadline deterministic, shifting the scenario from wall-clock races to durable, backdated scheduler state.
Changes:
- Expands the test veto window (2s → 300s) and backdates
staged_atso the veto deadline is deterministically elapsed without sleeping. - Replaces the fixed sleep with direct mutation of the durable
decisionRequest.claimedAttimestamp to model a timely claim during recovery.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+9654
to
+9666
| let raw = std::fs::read_to_string(&pending_path)?; | ||
| let mut value: Value = serde_json::from_str(&raw)?; | ||
| let mut request = proposal_decision_request(&value)? | ||
| .context("interrupted reject left a durable decision request")?; | ||
| request.claimed_at = staged_at + time::Duration::seconds(1); | ||
| value | ||
| .as_object_mut() | ||
| .context("pending proposal is a JSON object")? | ||
| .insert( | ||
| "decisionRequest".to_string(), | ||
| serde_json::to_value(&request)?, | ||
| ); | ||
| std::fs::write(&pending_path, serde_json::to_vec_pretty(&value)?)?; |
Comment on lines
+9629
to
9633
| let (pending_path, proposal_id) = stage_scheduled_reviewed_edit( | ||
| home, | ||
| coven_threads_core::ApprovalPath::FamiliarCoherence { veto }, | ||
| time::OffsetDateTime::now_utc(), | ||
| staged_at, | ||
| )?; |
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.
Closes #455.
What
Makes
api::tests::threads_scheduler_recovers_veto_claimed_before_deadlinedeterministic. It has been failing repeatedly onwindows-latest(three consecutive failures on PR #459's runs, plus the occurrences tracked in #455) and is currently the top CI flake.Root cause
The test staged a proposal with a real 2-second veto window and relied on two wall-clock races:
POST /rejectdurably recordsclaimed_at = now_utc()before theClaimBeforeValidationfailpoint fires. Recovery judges veto timeliness by that durableclaimed_atagainstveto_deadline = staged_at + 2s(proposal_decision_semanticsreject arm →proposal-veto-window-closed). On a slow Windows runner, staging + oneGET /threads/proposals+ thePOSTcan take longer than 2 s, so the claim lands after the deadline and recovery legitimately vetoes nothing —recovered == 0, the assertion fails. This is the observed CI failure.The issue suggested widening the sleep, but the sleep isn't the racing part — the stage-to-claim latency is, and no fixed window is safe against arbitrary runner stalls.
Fix
Drive both timing conditions through durable state instead of the wall clock, following the idioms the sibling scheduler tests already use (300 s window, backdated
staged_at):staged_at = now - 10 minand a 300 s window, so the veto deadline has already elapsed when recovery runs — the 2.1 s sleep is deleted.decisionRequest.claimed_attostaged_at + 1s(round-tripping the module's ownProposalDecisionRequesttype), modelling a claim that landed inside the window. Recovery readsclaimed_atverbatim from the durable request, so this exercises exactly the recovery-semantics path the test exists to cover: timely claim, post-deadline replay.No production code changes — test-only diff.
Verification
cargo test -p coven-cli threads_scheduler— 5/5 pass; the test now runs in ~60 ms (was ~2.1 s+).cargo fmt --check,cargo clippy --workspace --all-targets -- -D warnings,cargo test --workspace --locked,python scripts/check-secrets.py— all green.