test(code-index): fence the expired-proof read with the scheduler - #1867
Conversation
The admission permit and pass counter both read idle between the worker dropping its pass guard and the graph tail picking up its own, so a renewal already committed to run re-proved the aged source before the test read it. Hold the scheduler mutex, which every renewing step takes and no read does, across the aging and the read. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2fdc566cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pub(crate) async fn scheduler_for_root( | ||
| &self, | ||
| project_root: &Path, | ||
| ) -> Option<Arc<std::sync::Mutex<super::super::CodeIndexWorktreeSchedulerV1>>> { |
There was a problem hiding this comment.
CodeIndexSchedulerRegistryV1::scheduler_handle in registry.rs already canonicalizes the root and clones the same Arc<Mutex<CodeIndexWorktreeSchedulerV1>>, and it is enabled under cfg(test). Adding this identical accessor creates a second test-only registry surface that can drift from the existing authority; call scheduler_handle from hold_scheduler_for_root and remove this duplicate.
AGENTS.md reference: AGENTS.md:L124-L130
Useful? React with 👍 / 👎.
1 participant
long_text_projection_renews_source_before_seating_and_noop_follow_up_settlesfailed TRY 1 and passed TRY 2 on master run 35462174752's predecessor at 17:31 UTC and again on PR #1807's run 35465539562. The nextest ci profile marks that red.The test ages the source proof and reads it back, expecting the read to decline. The fence it held was the background admission permit plus the pass counter. Neither closes the window: the worker releases the permit after source reconciliation and drops its pass guard before the graph tail, and the tail's two renewing steps take a guard only once a blocking thread reaches their closure. On a loaded box the test settled inside that gap, aged the proof, and the tail renewed it before the read.
Every renewing step takes the worktree's scheduler mutex and no read does, so the test now holds that mutex across the aging and the read through a new
cfg(test)accessor. Production code is unchanged.Reproduced under CPU contention (two spinners and the test pinned to the same two cores) at 4 failures in 30 runs before the change and 0 in 100 after, run twice. Forcing the elapsed arm of
snapshot_is_recently_verifiedtrue makes the test fail at the same assertion, so it still detects a served expired proof.Two other tests in the same module,
reopened_current_text_generation_resolves_publication_identity_without_graph_seatandresident_memory_graph_refusal_seats_text_serving_without_graph, fail together about one run in four under the same contention on unmodified master. They are not touched here and need their own fix.🤖 Generated with Claude Code