From c6aadcb57790482fb3e76a19c73265359e8fd197 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 19 Sep 2026 06:30:28 +0000 Subject: [PATCH 01/10] fix(code-index): keep sealed generation when proof expires A seal or clone backfill outlives the 30s freshness window. Expiry and a predecessor witness used to clear the newer generation and reseal it. Unchanged sealed bytes now rebind that proof instead. Co-authored-by: Zack Jackson --- .../src/code_index_scheduler/reconcile.rs | 183 +++++++++++++++--- .../code_index_scheduler/registry/mount.rs | 29 +-- .../code_index_scheduler/tests/reconcile.rs | 133 +++++++++++++ 3 files changed, 299 insertions(+), 46 deletions(-) diff --git a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/reconcile.rs b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/reconcile.rs index c2c7eecdde..878c9b53e9 100644 --- a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/reconcile.rs +++ b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/reconcile.rs @@ -608,6 +608,37 @@ impl SourceFreshnessFenceV1 { }) && self.snapshot_is_recently_verified(&state, project_root, shutting_down) } + + /// Whether the last completed proof was sealed from exactly this snapshot. + /// + /// Clock age is not part of the answer. A seal or clone backfill can + /// outlive the admission window without the snapshot changing identity. + pub(super) fn proof_describes_snapshot( + &self, + snapshot_content_identity: &ContentDigest, + ) -> bool { + let state = self.snapshot(); + state.verified_against_source + && state.source_witness.as_ref().is_some_and(|witness| { + witness + .content_manifest + .describes_snapshot(snapshot_content_identity) + }) + } + + /// Refresh the admission clock and the git-metadata sample after the + /// sealed digests still matched. The content witness and reconciled + /// epoch stay put: this is the same proof, not a new generation. + fn rebind_admission_clock(&self, git_metadata: identity::GitMetadataFingerprintV1) { + let micros = now_micros().0; + let mut state = self.state.lock().unwrap_or_else(PoisonError::into_inner); + state.git_metadata = git_metadata; + state.last_reconciled_at = Instant::now(); + state.verified_against_source = true; + state.freshness_unknown = false; + self.last_reconciled_at_micros + .store(micros, Ordering::Release); + } } /// What the cheap Git/stat freshness ladder concluded about the retained @@ -1737,6 +1768,45 @@ impl CodeIndexWorktreeSchedulerV1 { Ok(Some(outcome)) } + /// Record that `metadata` is the generation the live worktree still seals. + /// + /// The in-memory fence takes this snapshot. The disk witness, when one + /// exists, is rewritten to this generation id so the next open does not + /// treat the predecessor's proof as a reason to drop it and reseal. + fn accept_unchanged_sealed_snapshot( + &mut self, + metadata: &VerifiedSealedTextGenerationMetadataV1, + git_metadata: identity::GitMetadataFingerprintV1, + stat_signature: String, + source_manifest: SourceContentManifestV1, + prior_witness: Option<&RestoreFreshnessWitnessV1>, + ) -> CodeIndexReconcileOutcomeV1 { + let snapshot_content_identity = metadata.snapshot().content_identity.clone(); + self.latest_content_identity = Some(snapshot_content_identity.clone()); + self.mark_reconciled_retained_generation_state( + git_metadata.clone(), + Some(ReconciledSourceWitnessV1 { + stat_signature: stat_signature.clone(), + content_manifest: source_manifest, + }), + ); + if let Some(prior) = prior_witness { + RestoreFreshnessWitnessV1 { + generation_id: metadata.manifest().generation_id.as_str().to_owned(), + git_metadata_signature: git_metadata.stable_signature(), + stat_signature, + repository_parse_identity_digest: prior.repository_parse_identity_digest.clone(), + ignored_source_admissions_digest: prior.ignored_source_admissions_digest.clone(), + ignored_source_paths: Vec::new(), + } + .persist(&self.store_root); + } + CodeIndexReconcileOutcomeV1::Noop(CodeIndexNoopEvidenceV1 { + snapshot_content_identity, + overflow_reconciled: false, + }) + } + pub(super) fn reconcile_retained_text_generation_with( &mut self, metadata: &VerifiedSealedTextGenerationMetadataV1, @@ -1758,10 +1828,14 @@ impl CodeIndexWorktreeSchedulerV1 { .observe_retained_text_compatibility(metadata) .is_reusable(); let witness = RestoreFreshnessWitnessV1::load(&self.store_root); - if witness.as_ref().is_some_and(|witness| { - witness.generation_id != metadata.manifest().generation_id.as_str() - || !witness.ignored_source_paths.is_empty() - }) || !self.ignored_source_admissions.is_empty() + // A predecessor freshness witness is not a reason to drop this + // generation. It names the proof that sealed an earlier snapshot. + // Ignored-source rosters still require the complete capture: their + // digest is not the ordinary file manifest this path compares. + if witness + .as_ref() + .is_some_and(|witness| !witness.ignored_source_paths.is_empty()) + || !self.ignored_source_admissions.is_empty() { return Ok(None); } @@ -1788,37 +1862,36 @@ impl CodeIndexWorktreeSchedulerV1 { // generation's sealed file digests; its matching stat signature is // the negative cache that lets a moved tree skip the byte comparison. let source_manifest = SourceContentManifestV1::for_snapshot(metadata.snapshot()); - if retained_is_reusable + let sealed_bytes_match = retained_is_reusable && !has_hints - && let Some(witness) = witness.as_ref() - && witness.git_metadata_signature == sampled_metadata.stable_signature() - && witness.stat_signature == sampled_sweep.signature && sampled_sweep.content_matches( &self.project_root, &source_manifest, &self.shutting_down, - ) - { - let snapshot_content_identity = metadata.snapshot().content_identity.clone(); - self.latest_content_identity = Some(snapshot_content_identity.clone()); - self.mark_reconciled_retained_generation_state( - sampled_metadata, - Some(ReconciledSourceWitnessV1 { - stat_signature: sampled_sweep.signature, - content_manifest: source_manifest, - }), ); - return Ok(Some(CodeIndexReconcileOutcomeV1::Noop( - CodeIndexNoopEvidenceV1 { - snapshot_content_identity, - overflow_reconciled: false, - }, + let quiet_witness = sealed_bytes_match + && witness.as_ref().is_some_and(|witness| { + witness.git_metadata_signature == sampled_metadata.stable_signature() + && witness.stat_signature == sampled_sweep.signature + }); + // Graph-on refuses to decode the sealed generation just because the + // predecessor witness, or a git-index mtime this seal itself moved, + // does not name this generation. The sealed digests are the proof. + // Graph-off still captures so a metadata-only drift is verified + // without a full decode when the quiet witness is absent. + if sealed_bytes_match && (quiet_witness || !rebuild_changed_source_without_decode) { + return Ok(Some(self.accept_unchanged_sealed_snapshot( + metadata, + sampled_metadata, + sampled_sweep.signature, + source_manifest, + witness.as_ref(), ))); } - // A compatible generation whose witness did not prove a quiet tree - // falls through to the full graph-on reconcile. An incompatible - // lightweight owner rebuilds here without decoding the retained graph. + // A compatible generation whose bytes moved falls through to the full + // graph-on reconcile. An incompatible lightweight owner rebuilds here + // without decoding the retained graph. if retained_is_reusable && !rebuild_changed_source_without_decode { return Ok(None); } @@ -2813,6 +2886,51 @@ impl CodeIndexWorktreeSchedulerV1 { .source_currency_witness_for(generation_id, snapshot_content_identity) } + /// Bind a sealed snapshot to the source proof, renewing an expired clock + /// when the sealed digests still match. + /// + /// The admission window is 30s. A graph seal and the clone-fingerprint + /// backfill both outlive it under load. Treating that expiry as "this + /// generation is not the proof" cleared the serving witness and the next + /// pass resealed the same snapshot. A hook epoch or a digest mismatch + /// still refuses; only an unchanged sealed snapshot keeps its generation. + pub(super) fn currency_witness_for_sealed_snapshot( + &self, + generation_id: &CodeGenerationId, + snapshot_content_identity: &ContentDigest, + ) -> Option { + if self.shutting_down.load(Ordering::Acquire) { + return None; + } + if self.freshness_fence.serves_recently_verified_source( + snapshot_content_identity, + &self.project_root, + &self.shutting_down, + ) { + return self + .freshness_fence + .source_currency_witness_for(generation_id, snapshot_content_identity); + } + if !self + .freshness_fence + .proof_describes_snapshot(snapshot_content_identity) + || self.freshness_fence.source_change_pending() + { + return None; + } + let freshness = self.freshness_fence.snapshot(); + if !self.source_witness_matches_worktree(&freshness) { + return None; + } + // Sample after the walk. `gix::open` inside the digest comparison can + // move index metadata; storing the post-walk sample is what keeps the + // next probe from calling that side effect a new generation. + let git_metadata = identity::GitMetadataFingerprintV1::capture(&self.project_root); + self.freshness_fence.rebind_admission_clock(git_metadata); + self.freshness_fence + .source_currency_witness_for(generation_id, snapshot_content_identity) + } + /// A cheap stat-level (path, mtime, size) signature of the present source /// candidates. It opens gix and runs stat-based status (no byte reads, no /// content hashing). A changed signature skips straight to reconcile; an @@ -3252,6 +3370,19 @@ impl CodeIndexWorktreeSchedulerV1 { self.publication.sealed_decode_count() } + /// Age the admission clock past its own threshold without touching source. + #[cfg(test)] + pub(super) fn expire_source_proof_for_test(&self) { + let mut state = self + .freshness_fence + .state + .lock() + .unwrap_or_else(PoisonError::into_inner); + state.last_reconciled_at = Instant::now() + .checked_sub(state.staleness_threshold + Duration::from_secs(1)) + .unwrap_or_else(Instant::now); + } + #[cfg(any(test, feature = "test-helpers"))] pub fn poison_decoded_publication_cache_for_test(&self) { self.publication.poison_decoded_cache_for_test(); diff --git a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/registry/mount.rs b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/registry/mount.rs index ac239ce49c..0a726b2573 100644 --- a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/registry/mount.rs +++ b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/registry/mount.rs @@ -1801,8 +1801,6 @@ impl CodeIndexSchedulerRegistryV1 { let text_generation = Arc::clone(&worker_text_generation); let serving_seats = Arc::clone(&worker_serving_seats); let serving_generation_changed = worker_serving_generation_changed.clone(); - let source_freshness = worker_source_freshness.clone(); - let project_root = worker_project_root.clone(); let text_latest = latest.clone(); let latest = latest.clone(); let shutting_down = Arc::clone(&worker_shutting_down); @@ -1836,13 +1834,14 @@ impl CodeIndexSchedulerRegistryV1 { // proofs to the seat. Asking the fence whether it // has verified *this* sealed snapshot is what makes // the binding truthful for a seat this pass did not - // publish. - let pass_proves_latest = source_freshness - .serves_recently_verified_source( - &latest.generation().snapshot().content_identity, - &project_root, - &shutting_down, - ); + // publish. An expired clock, or a git-index sample + // this seal moved, is not a different snapshot: + // dropping the witness here is how a newer + // generation stayed unserved through clone backfill. + let sealed_currency = scheduler.currency_witness_for_sealed_snapshot( + &latest.generation().manifest().generation_id, + &latest.generation().snapshot().content_identity, + ); let mut serving = serving_generation .write() .unwrap_or_else(std::sync::PoisonError::into_inner); @@ -1883,17 +1882,7 @@ impl CodeIndexSchedulerRegistryV1 { *serving_source_witness .write() .unwrap_or_else(std::sync::PoisonError::into_inner) = - pass_proves_latest - .then(|| { - source_freshness.source_currency_witness_for( - &latest.generation().manifest().generation_id, - &latest - .generation() - .snapshot() - .content_identity, - ) - }) - .flatten(); + sealed_currency; } // The durable pointer names a successor, so no // proof of this seat's currency exists to bind. diff --git a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/tests/reconcile.rs b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/tests/reconcile.rs index 188d869742..5a3e412f32 100644 --- a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/tests/reconcile.rs +++ b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/tests/reconcile.rs @@ -8173,6 +8173,139 @@ fn graph_off_stale_witness_reconciles_unchanged_source_without_full_decode() { ); } +/// The disk freshness witness names whichever generation last persisted it. +/// A later seal of the same bytes used to return `None` the moment that id +/// disagreed, and the graph-on caller then decoded and resealed. Under load +/// that reseal outlived the admission window, the swap cleared the witness, +/// and the newer generation never became current. Unchanged sealed bytes +/// keep the generation and rewrite the witness onto it. Moved bytes still +/// refuse, without publishing a substitute. +#[test] +fn predecessor_freshness_witness_keeps_the_sealed_generation() { + let fixture = GitFixture::new(ALPHA_LIB_V1); + let store = TempDir::new().expect("store root"); + let mut scheduler = scheduler( + &fixture, + store.path().to_path_buf(), + Arc::new(SharedCodeIndexBytePoolV1::default()), + ); + let seeded = published(scheduler.reconcile_now().expect("seed retained generation")); + let metadata = scheduler + .servable_retained_text_generation() + .expect("publication store") + .expect("authenticated retained text generation") + .metadata() + .clone(); + let generation_id = metadata.manifest().generation_id.clone(); + let mut witness = + RestoreFreshnessWitnessV1::load(store.path()).expect("the seal persisted a proof"); + assert_eq!(witness.generation_id, generation_id.as_str()); + witness.generation_id = "generation.predecessor".to_owned(); + witness.persist(store.path()); + let index_path = fixture.path().join(".git/index"); + let index_mtime = std::fs::metadata(&index_path) + .expect("git index metadata") + .modified() + .expect("git index mtime"); + filetime::set_file_mtime( + &index_path, + filetime::FileTime::from_system_time(index_mtime + Duration::from_secs(2)), + ) + .expect("advance only the git index mtime"); + + let decodes_before = scheduler.sealed_decode_count(); + let outcome = scheduler + .reconcile_retained_text_generation_with(&metadata, false) + .expect("graph-on retained reconcile") + .expect("unchanged sealed bytes must not be dropped"); + let CodeIndexReconcileOutcomeV1::Noop(evidence) = outcome else { + panic!("predecessor proof must not reseal the same snapshot: {outcome:?}"); + }; + assert_eq!( + evidence.snapshot_content_identity, seeded.snapshot_content_identity, + "the noop names the generation that was already sealed" + ); + assert_eq!( + scheduler.sealed_decode_count(), + decodes_before, + "keeping the sealed generation must not decode it again" + ); + assert_eq!( + RestoreFreshnessWitnessV1::load(store.path()) + .expect("rebound proof") + .generation_id, + generation_id.as_str(), + "the disk proof must name the sealed generation, not the predecessor" + ); + assert_eq!( + scheduler + .source_currency_witness_for(&generation_id, &metadata.snapshot().content_identity,) + .map(|witness| witness.generation_id), + Some(generation_id.clone()), + "the in-memory proof must admit the sealed generation" + ); + + fixture.edit( + "src/lib.rs", + "pub fn changed_after_predecessor_proof() -> u32 { 2 }\n", + ); + let refused = scheduler + .reconcile_retained_text_generation_with(&metadata, false) + .expect("changed source is a typed refusal, not an error"); + assert!( + refused.is_none(), + "moved bytes must not keep the sealed generation: {refused:?}" + ); + assert_eq!( + scheduler + .publication + .read_publication_pointer() + .expect("read pointer") + .expect("active pointer") + .generation_id, + generation_id.as_str(), + "refusing the moved bytes must not publish a substitute generation" + ); +} + +/// Clone backfill and the seal itself outlive the 30s admission window. Expiry +/// is a request to re-check the sealed digests, not a reason to drop the +/// generation those digests already name. A byte change after expiry still drops it. +#[test] +fn expired_proof_keeps_the_sealed_generation_until_bytes_move() { + let fixture = GitFixture::new(ALPHA_LIB_V1); + let store = TempDir::new().expect("store root"); + let mut scheduler = scheduler( + &fixture, + store.path().to_path_buf(), + Arc::new(SharedCodeIndexBytePoolV1::default()), + ); + let seeded = published(scheduler.reconcile_now().expect("seed retained generation")); + scheduler.expire_source_proof_for_test(); + assert_eq!( + scheduler + .currency_witness_for_sealed_snapshot( + &seeded.generation_id, + &seeded.snapshot_content_identity, + ) + .map(|witness| witness.generation_id), + Some(seeded.generation_id.clone()), + "an expired proof must keep the generation whose sealed bytes still match" + ); + + fixture.edit("src/lib.rs", "pub fn alpha() -> u32 { 9 }\n"); + scheduler.expire_source_proof_for_test(); + assert!( + scheduler + .currency_witness_for_sealed_snapshot( + &seeded.generation_id, + &seeded.snapshot_content_identity, + ) + .is_none(), + "an expired proof must drop the generation once its sealed bytes moved" + ); +} + /// A query freshness probe against a restored owner that no pass has verified /// yet must report "not current", the restart's first pass is still the /// remedy, without minting an observed source change: no overflow hint and no From 08c3f3c24dc7199cbdaaa67bbab99238221c820c Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 19 Sep 2026 06:46:30 +0000 Subject: [PATCH 02/10] docs(code-index): leave clone copy out of witness rebind The seat-swap witness does not own the lexical full-copy. That copy stays on the retained successor driver. Co-authored-by: Zack Jackson --- .../src/code_index_scheduler/reconcile.rs | 9 ++++----- .../src/code_index_scheduler/registry/mount.rs | 7 ++++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/reconcile.rs b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/reconcile.rs index 878c9b53e9..2ba6cbacf4 100644 --- a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/reconcile.rs +++ b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/reconcile.rs @@ -2889,11 +2889,10 @@ impl CodeIndexWorktreeSchedulerV1 { /// Bind a sealed snapshot to the source proof, renewing an expired clock /// when the sealed digests still match. /// - /// The admission window is 30s. A graph seal and the clone-fingerprint - /// backfill both outlive it under load. Treating that expiry as "this - /// generation is not the proof" cleared the serving witness and the next - /// pass resealed the same snapshot. A hook epoch or a digest mismatch - /// still refuses; only an unchanged sealed snapshot keeps its generation. + /// The admission window is 30s. This does not move the clone-successor + /// copy off the publication advance. It only stops an expired clock, or a + /// predecessor disk witness, from clearing the generation those digests + /// already name. A hook epoch or a digest mismatch still refuses. pub(super) fn currency_witness_for_sealed_snapshot( &self, generation_id: &CodeGenerationId, diff --git a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/registry/mount.rs b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/registry/mount.rs index 0a726b2573..3d91e3a619 100644 --- a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/registry/mount.rs +++ b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/registry/mount.rs @@ -1835,9 +1835,10 @@ impl CodeIndexSchedulerRegistryV1 { // has verified *this* sealed snapshot is what makes // the binding truthful for a seat this pass did not // publish. An expired clock, or a git-index sample - // this seal moved, is not a different snapshot: - // dropping the witness here is how a newer - // generation stayed unserved through clone backfill. + // this seal moved, is not a different snapshot. + // Dropping the witness here cleared the newer + // generation. The lexical full-copy is not decided + // on this swap. let sealed_currency = scheduler.currency_witness_for_sealed_snapshot( &latest.generation().manifest().generation_id, &latest.generation().snapshot().content_identity, From c1633dd96bb0f70d89d5a33cc64c3d5c35d71dfa Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 07:54:37 +0000 Subject: [PATCH 03/10] fix(code-index): refuse a sealed generation the checkout moved past Dropping the witness generation-id guard let the graph-on retained reconcile return `Noop` whenever the sealed file digests still matched, including after a commit or branch switch that touches no indexed byte (an empty or docs-only commit). `accept_unchanged_sealed_snapshot` then persisted the new git-metadata sample onto the retained generation's witness, so the stale `reference`/`source_revision` attribution stayed masked until code bytes moved. `finish_retained_reconcile` rebuilds on exactly that drift, and `branch_generations` resolves generations by the commit they sealed, so the retained generation must not outlive it. Gate the accept on the attribution a fresh capture would seal: HEAD's ref must still match the snapshot's, and a snapshot that sealed a revision must still name HEAD's commit. `self.identity` is re-resolved a few lines above, so this adds no walk. A snapshot sealed from a dirty tree carries no revision and keeps the fast path. Verified by `a_moved_commit_refuses_the_sealed_generation_despite_identical_bytes`, which fails on the parent commit with `Some(Noop(..))`. Co-Authored-By: Claude Fable 5.1 --- .../src/code_index_scheduler/reconcile.rs | 20 +++++- .../code_index_scheduler/tests/reconcile.rs | 63 +++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/reconcile.rs b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/reconcile.rs index 2ba6cbacf4..b686c00dc6 100644 --- a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/reconcile.rs +++ b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/reconcile.rs @@ -1874,12 +1874,30 @@ impl CodeIndexWorktreeSchedulerV1 { witness.git_metadata_signature == sampled_metadata.stable_signature() && witness.stat_signature == sampled_sweep.signature }); + // Identical source bytes do not make a moved commit or branch the same + // generation. `finish_retained_reconcile` rebuilds on exactly this + // drift, and branch-scoped reads resolve generations by their sealed + // `source_revision`, so accepting here would leave the retained + // generation attributed to a commit the checkout has left for as long + // as the bytes hold still. `self.identity` was re-resolved above, so + // this costs no extra walk. A snapshot sealed without a revision + // (a dirty capture) has no commit attribution to invalidate. + let sealed_attribution_is_current = metadata.snapshot().reference.as_ref() + == self.identity.head_ref() + && metadata + .snapshot() + .source_revision + .as_ref() + .is_none_or(|sealed| self.identity.head_commit() == Some(sealed)); // Graph-on refuses to decode the sealed generation just because the // predecessor witness, or a git-index mtime this seal itself moved, // does not name this generation. The sealed digests are the proof. // Graph-off still captures so a metadata-only drift is verified // without a full decode when the quiet witness is absent. - if sealed_bytes_match && (quiet_witness || !rebuild_changed_source_without_decode) { + if sealed_bytes_match + && sealed_attribution_is_current + && (quiet_witness || !rebuild_changed_source_without_decode) + { return Ok(Some(self.accept_unchanged_sealed_snapshot( metadata, sampled_metadata, diff --git a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/tests/reconcile.rs b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/tests/reconcile.rs index 5a3e412f32..da03d8b338 100644 --- a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/tests/reconcile.rs +++ b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/tests/reconcile.rs @@ -10510,3 +10510,66 @@ fn serving_swap_seats_a_generation_whose_publication_moved_while_it_activated() "neither refusing arm writes the serving slot" ); } + +/// Unchanged source bytes are not a reason to keep a generation the checkout +/// has committed past. An empty (or docs-only) commit moves HEAD without +/// touching one indexed byte, and `finish_retained_reconcile` rebuilds on +/// exactly that `source_revision` drift because branch-scoped reads resolve +/// generations by the commit they sealed. Accepting the sealed snapshot here +/// would pin the stale attribution for as long as the bytes hold still. +#[test] +fn a_moved_commit_refuses_the_sealed_generation_despite_identical_bytes() { + let fixture = GitFixture::new(ALPHA_LIB_V1); + let store = TempDir::new().expect("store root"); + let mut scheduler = scheduler( + &fixture, + store.path().to_path_buf(), + Arc::new(SharedCodeIndexBytePoolV1::default()), + ); + published(scheduler.reconcile_now().expect("seed retained generation")); + let metadata = scheduler + .servable_retained_text_generation() + .expect("publication store") + .expect("authenticated retained text generation") + .metadata() + .clone(); + let sealed_revision = metadata + .snapshot() + .source_revision + .clone() + .expect("a clean seed seals its commit"); + git( + fixture.path(), + &["commit", "-qm", "docs only", "--allow-empty"], + ); + let moved_head = + CommitId::new(git_stdout(fixture.path(), &["rev-parse", "HEAD"])).expect("moved HEAD"); + assert_ne!(sealed_revision, moved_head, "the fixture must move HEAD"); + + let refused = scheduler + .reconcile_retained_text_generation_with(&metadata, false) + .expect("graph-on retained reconcile"); + assert!( + refused.is_none(), + "a moved commit must not keep the generation sealed at {sealed_revision:?}: {refused:?}" + ); + + // The refusal is what hands the pass to the authoritative capture, and + // that capture is what re-attributes the generation to the new commit. + published( + scheduler + .reconcile_now() + .expect("rebuild at the moved commit"), + ); + assert_eq!( + scheduler + .servable_retained_text_generation() + .expect("publication store") + .expect("authenticated retained text generation") + .metadata() + .snapshot() + .source_revision, + Some(moved_head), + "the rebuilt generation must name the commit the checkout is on" + ); +} From 76c58af7e25dde6daa330bab74af5ee281d98c61 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 19 Sep 2026 06:36:04 +0000 Subject: [PATCH 04/10] fix(code-index): keep clone copy off the freshness receipt The advance that installs exact and lexical owners also copied the prior lexical artifact into the clone successor. That copy ran under reconcile_in_progress, so status stayed non-current for the copy. Leave the successor pending; the retained driver starts it after the seat. Co-authored-by: Zack Jackson --- .../src/code_index_scheduler/serving.rs | 17 ++-- .../src/code_index_scheduler/tests/serving.rs | 89 +++++++++++++++++++ 2 files changed, 100 insertions(+), 6 deletions(-) diff --git a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/serving.rs b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/serving.rs index 394cc15419..bf72387e38 100644 --- a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/serving.rs +++ b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/serving.rs @@ -3273,7 +3273,7 @@ impl LatestCodeTextGenerationV1 { )); }; drop(slot); - let mut publish_claim = TextHeadOpenClaimV1::new(&self.text_projection_build); + let _publish_claim = TextHeadOpenClaimV1::new(&self.text_projection_build); let CodeTextArtifactBuildV1 { builder, source, @@ -3312,7 +3312,6 @@ impl LatestCodeTextGenerationV1 { ) .map_err(map_text_artifact_error)?; let needs_clone_successor = !reader.has_clone_fingerprints(); - let prior = reader.verified_artifact().clone(); // Match the cold-open path: install owners first, then publish Ready. // Publishing Ready before a failed install (admission ceiling / shrink) // would leave dashboard/MCP progress claiming a ready generation that @@ -3320,10 +3319,16 @@ impl LatestCodeTextGenerationV1 { self.install_artifact_owners(reader, reader_reservation)?; self.publish_text_progress_phase(CodeIndexBuildPhaseV1::Ready, 0, 0); if needs_clone_successor { - let source = store.open_sealed_source(&sealed_identity, control)?; - let build = - self.begin_clone_successor(descriptor, prior, sealed_identity, source, control)?; - drop(publish_claim.install(TextHeadOpenBuildV1::CloneSuccessor(build))); + // `begin_clone_successor` copies the whole prior lexical artifact + // before the first page walk. Doing that here kept this advance, + // and the publication pass awaiting it, inside `reconcile_in_progress` + // for the copy. Exact and lexical serving are already installed; + // the copy is not a freshness precondition. Leave the slot pending + // so the retained driver starts the successor after the seat, + // without the receipt guard. The claim stays armed: its drop + // restores only `HeadOpening`, so `CloneSuccessorPending` survives + // and parked wakes are notified. + self.text_projection_build.retain_clone_successor_retry()?; return Ok(false); } Ok(true) diff --git a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/tests/serving.rs b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/tests/serving.rs index 306fa115a6..03ebce7c9e 100644 --- a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/tests/serving.rs +++ b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/tests/serving.rs @@ -853,6 +853,95 @@ fn clone_successor_keeps_lexical_owners_ready_and_cas_replaces_v14() { assert_eq!(v16_revision, 16); } +/// Exact and lexical readiness is not the clone-successor copy. +/// +/// The publication advance that installs those owners used to call +/// `begin_clone_successor` before returning, and that call copies the whole +/// prior lexical artifact. The freshness receipt awaits that advance, so +/// status stayed non-current for the copy. The successor must still be +/// reported as backfill, and the next advance is what writes its staging file. +#[test] +fn lexical_readiness_leaves_the_clone_successor_uncopied() { + let fixture = GitFixture::new(&[( + "src/lib.rs", + "pub fn alpha() { one(); two(); three(); four(); five(); six(); seven(); eight(); nine(); ten(); }\n", + )]); + let store = TempDir::new().expect("store root"); + let mut scheduler = scheduler( + &fixture, + store.path().to_path_buf(), + Arc::new(SharedCodeIndexBytePoolV1::default()), + ); + published(scheduler.reconcile_now().expect("publish generation")); + let latest = scheduler.latest_complete().expect("latest generation"); + while !latest.query_owners_are_ready() { + latest.advance_text_serving(1).expect("advance V14 build"); + } + let tracedecay_contracts::code_index_freshness::CodeCloneIndexStatusV1::Backfilling { + observation, + } = latest.clone_index_status(false, None) + else { + panic!( + "a generation without clone fingerprints must report backfill once lexical owners serve, got {:?}", + latest.clone_index_status(false, None) + ); + }; + assert_eq!(observation.coverage.completed_source_pages, 0); + assert!( + observation.coverage.total_source_pages > 0, + "the pending successor must name the sealed page count it has not visited" + ); + // Status falls back to the published artifact's bytes when the successor + // has not created a staging file, so the bytes field cannot prove the + // copy stayed off this advance. The slot and the artifacts directory can. + assert!( + matches!( + &*latest.text_projection_build.lock_slot(), + super::super::CodeTextProjectionSlotV1::CloneSuccessorPending + ), + "owner readiness must leave the successor pending" + ); + let staging_names = |root: &std::path::Path| { + std::fs::read_dir(code_text_artifacts_root(root)) + .expect("artifacts root") + .map(|entry| entry.expect("artifact entry").file_name()) + .filter(|name| name.to_string_lossy().ends_with(".staging")) + .collect::>() + }; + assert!( + staging_names(store.path()).is_empty(), + "owner readiness copied the prior lexical artifact: {:?}", + staging_names(store.path()) + ); + + latest + .advance_text_serving(1) + .expect("the retained successor advance copies the prior artifact"); + assert!(latest.query_owners_are_ready()); + assert!( + !matches!( + &*latest.text_projection_build.lock_slot(), + super::super::CodeTextProjectionSlotV1::CloneSuccessorPending + ), + "the next advance must take the pending successor" + ); + + while latest.text_projection_needs_work() { + latest + .advance_text_serving(16) + .expect("finish clone successor"); + } + let revision: i64 = rusqlite::Connection::open(active_text_artifact_path(store.path())) + .expect("open finished artifact") + .query_row( + "SELECT format_revision FROM artifact_state WHERE singleton = 1", + [], + |row| row.get(0), + ) + .expect("read finished revision"); + assert_eq!(revision, 16); +} + #[test] fn clone_status_distinguishes_unavailable_backfill_partial_ready_and_stale() { let fixture = GitFixture::new(&[( From 3510cf76e149cfdc6357699f9b3e765dadfab45e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 19 Sep 2026 06:17:59 +0000 Subject: [PATCH 05/10] fix(code-index): stop continuation receipts looking like probe wakes A worker continuation occupied the pending slot with a wall-clock arrival. The follow-up pass then published an event-to-ready receipt, and a suppressed freshness probe that raced the stamp was charged with it. Continuations stay visible to freshness but are not external arrivals. Co-authored-by: Zack Jackson (cherry picked from commit aecc70bb31e194b35e3b91c28dd12d84b0d1c69e) --- .../src/code_index_scheduler/registry.rs | 130 +++++++++++++++--- 1 file changed, 113 insertions(+), 17 deletions(-) diff --git a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/registry.rs b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/registry.rs index ab44191731..4413afd940 100644 --- a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/registry.rs +++ b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/registry.rs @@ -1266,11 +1266,17 @@ enum ColdMountAdmissionV1 { /// One exact worktree's pending worker wake. `micros == 0` means no pending /// arrival, and every nonzero arrival is held by one nonzero owner token. +/// +/// `attributable` is false for a worker-owned continuation: the slot stays +/// nonzero so freshness still sees the follow-up, but the instant is not an +/// external wake. Publishing it as [`CodeIndexArrivalV1::Observed`] fabricated +/// an event-to-ready receipt for a pass nobody requested. struct PendingWakeStateV1 { micros: u64, trigger: u64, owner: u64, next_owner: u64, + attributable: bool, } /// The single synchronization authority for one worktree's coalesced wake. @@ -1348,6 +1354,7 @@ impl Default for PendingWakeStateV1 { trigger: 0, owner: 0, next_owner: 1, + attributable: false, } } } @@ -1385,6 +1392,7 @@ impl PendingWakeClaimV1 { let claimed_micros = u64::try_from(now_micros().0).unwrap_or(u64::MAX); let owner = state.next_owner(); state.micros = claimed_micros; + state.attributable = true; state.owner = owner; drop(state); Some(Self { @@ -1426,6 +1434,7 @@ impl Drop for PendingWakeClaimV1 { state.micros = 0; state.trigger = 0; state.owner = 0; + state.attributable = false; } } } @@ -1878,6 +1887,7 @@ impl CodeIndexSchedulerRegistryV1 { pending_wake.micros = 0; pending_wake.owner = 0; pending_wake.trigger = 0; + pending_wake.attributable = false; } } } @@ -2065,9 +2075,13 @@ impl CodeIndexSchedulerRegistryV1 { .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); state.owner = state.next_owner(); - if state.micros == 0 { + // A worker continuation occupies the slot without an external instant. + // This wake is the arrival; keep an already-observed one so a later + // stamp cannot shorten the wait that wake already took. + if state.micros == 0 || !state.attributable { state.micros = wake_micros; } + state.attributable = true; state.trigger = Self::pack_trigger(trigger); drop(state); wake.notify_one(); @@ -2085,28 +2099,47 @@ impl CodeIndexSchedulerRegistryV1 { .state .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); - if state.micros != 0 { + // An unattributable continuation is not an arrival. Upgrade it: the + // caller that just proved work is the event the receipt must name. + if state.micros != 0 && state.attributable { return false; } state.owner = state.next_owner(); state.micros = wake_micros; + state.attributable = true; state.trigger = Self::pack_trigger(trigger); drop(state); wake.notify_one(); true } - /// Queue worker-owned continuation work through the same pending-arrival - /// authority as external wakes. This keeps readiness truthful while the - /// continuation waits for shared admission; a bare `Notify` permit is not - /// observable by freshness readers. + /// Queue worker-owned continuation work so freshness still sees it. + /// + /// A bare `Notify` permit is not observable by freshness readers, so the + /// pending slot stays nonzero. That slot is not an external arrival: the + /// worker decided to continue work an earlier wake already claimed. + /// Stamping a wall-clock instant here made the follow-up pass publish an + /// event-to-ready receipt, and a suppressed freshness probe that raced the + /// stamp was charged with it. fn note_worker_continuation(pending_wake: &PendingWakeV1, wake: &tokio::sync::Notify) { - if !Self::note_wake_if_idle(pending_wake, wake, CodeIndexCadenceTriggerV1::BusyFollowUp) { - // This pass may have consumed the permit for an arrival it has not - // claimed yet. Keep that observable arrival and replenish its - // coalesced permit so the continuation cannot sleep behind it. + let mut state = pending_wake + .state + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if state.micros != 0 { + // An arrival is already queued, or a continuation already occupies + // the slot. Replenish the coalesced permit so the worker cannot + // sleep behind work it has not claimed. + drop(state); wake.notify_one(); + return; } + state.owner = state.next_owner(); + state.micros = u64::try_from(now_micros().0).unwrap_or(u64::MAX); + state.attributable = false; + state.trigger = Self::pack_trigger(CodeIndexCadenceTriggerV1::BusyFollowUp); + drop(state); + wake.notify_one(); } /// Stamp a continuation while `reconcile_in_progress` still reports this pass. @@ -2135,20 +2168,27 @@ impl CodeIndexSchedulerRegistryV1 { pending_wake: &PendingWakeV1, default_trigger: CodeIndexCadenceTriggerV1, ) -> (CodeIndexArrivalV1, CodeIndexCadenceTriggerV1) { - let (wake_micros, packed_trigger) = { + let (wake_micros, packed_trigger, attributable) = { let mut state = pending_wake .state .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); let wake_micros = state.micros; let packed_trigger = state.trigger; + let attributable = state.attributable; state.micros = 0; state.trigger = 0; state.owner = 0; - (wake_micros, packed_trigger) + state.attributable = false; + (wake_micros, packed_trigger, attributable) }; - if wake_micros == 0 { - return (CodeIndexArrivalV1::Unavailable, default_trigger); + if wake_micros == 0 || !attributable { + let trigger = if wake_micros == 0 { + default_trigger + } else { + Self::unpack_trigger(packed_trigger) + }; + return (CodeIndexArrivalV1::Unavailable, trigger); } let trigger = Self::unpack_trigger(packed_trigger); match i64::try_from(wake_micros) { @@ -2178,12 +2218,14 @@ impl CodeIndexSchedulerRegistryV1 { .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); // A wake that arrived while this pass ran is newer, so the restored - // arrival remains the earliest and stays authoritative. - if state.micros != 0 && state.micros <= wake_micros { + // arrival remains the earliest and stays authoritative. A continuation + // occupying the slot is not an arrival and must not hide this one. + if state.attributable && state.micros != 0 && state.micros <= wake_micros { return; } state.owner = state.next_owner(); state.micros = wake_micros; + state.attributable = true; state.trigger = Self::pack_trigger(trigger); } @@ -3557,7 +3599,9 @@ mod feedback_document_path_tests { #[cfg(test)] mod text_slice_fairness_tests { - use super::{CodeIndexCadenceTriggerV1, CodeIndexSchedulerRegistryV1, PendingWakeV1}; + use super::{ + CodeIndexArrivalV1, CodeIndexCadenceTriggerV1, CodeIndexSchedulerRegistryV1, PendingWakeV1, + }; #[test] fn pending_reconcile_is_serviced_between_bounded_text_slices() { @@ -3587,6 +3631,58 @@ mod text_slice_fairness_tests { "text continuation resumes only after reconcile claims the pending arrival" ); } + + #[test] + fn worker_continuation_stays_pending_without_an_observed_arrival() { + let pending = PendingWakeV1::default(); + let wake = tokio::sync::Notify::new(); + CodeIndexSchedulerRegistryV1::note_worker_continuation(&pending, &wake); + assert!( + pending.has_pending_arrival(), + "freshness must still see the continuation while it waits" + ); + + let (arrival, trigger) = CodeIndexSchedulerRegistryV1::take_pending_arrival( + &pending, + CodeIndexCadenceTriggerV1::Mount, + ); + assert_eq!( + arrival, + CodeIndexArrivalV1::Unavailable, + "a worker continuation is not an external wake and must not publish \ + an event-to-ready sample" + ); + assert_eq!(trigger, CodeIndexCadenceTriggerV1::BusyFollowUp); + assert!( + !pending.has_pending_arrival(), + "claiming the continuation clears the slot" + ); + } + + #[test] + fn an_external_wake_replaces_an_unattributable_continuation() { + let pending = PendingWakeV1::default(); + let wake = tokio::sync::Notify::new(); + CodeIndexSchedulerRegistryV1::note_worker_continuation(&pending, &wake); + assert!( + CodeIndexSchedulerRegistryV1::note_wake_if_idle( + &pending, + &wake, + CodeIndexCadenceTriggerV1::QueryAdmission, + ), + "a real wake must replace the continuation placeholder" + ); + + let (arrival, trigger) = CodeIndexSchedulerRegistryV1::take_pending_arrival( + &pending, + CodeIndexCadenceTriggerV1::Mount, + ); + assert!( + matches!(arrival, CodeIndexArrivalV1::Observed { wake_micros } if wake_micros > 1), + "the receipt names the external wake, not the continuation slot: {arrival:?}" + ); + assert_eq!(trigger, CodeIndexCadenceTriggerV1::QueryAdmission); + } } #[cfg(test)] From 2b803caed912432023de85ace4bca085de9b2deb Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 19 Sep 2026 16:45:15 +0000 Subject: [PATCH 06/10] fix(retention): bound store locks by cancel and deadlines Exclusive store locks blocked in File::lock, which cannot observe the caller's cancel flag or carried deadline, so a cancelled retention pass waited for a peer's critical section. Acquisition now polls try_lock until the deadline and answers GenerationStoreBusy or Cancelled instead. A free lock is still taken even past the deadline, so an uncontended caller is never refused. The text-artifact candidate scan also treats a vanished file as absent: verify_unreferenced_completed_text_artifact returns whether the artifact was there, the census skips a candidate that is already gone, and the two sites that verify a descriptor the durable index names turn absence into UnsafeState rather than silence. Conflict resolution, crates/tracedecay-code-index-retention/src/ code_index_generations.rs and code_index_generations/generation_scan.rs: resolved to batch C/D's landed design and dropped this commit's competing one. Both fix the same defect, a path unlinked between readdir and open. Batch C/D maps that NotFound to GenerationStoreBusy via deferred_if_absent (690e84365d, narrowed by d6d8665a80) and defers the pass; this commit returned Option and skipped the entry. Those two call sites sit in sweep_unreferenced_generation_segments, the loop that builds live_segments, and the code below it removes every segment that set does not contain. Skipping a vanished manifest there under-counts the live set and deletes segments a live generation still references, so deferring is the answer that cannot lose data. Fix Root Causes decided it: the deferral addresses the same race without trading a loud failure for a silent deletion. Its test, 6eecf5f918 ("assert a vanished census open is absent"), asserts the dropped design and is not folded. Batch C/D's deferral already satisfies that test's stated contract, that a vanished census open is not CodeGenerationRetentionErrorV1::Storage. The text_artifacts.rs staging-sidecar hunk from this commit was already present verbatim in batch C/D and merged as identical content. (cherry picked from commit 7c22827e65a9650d7ce5fe96124d73c0bf9f4ab4) Co-Authored-By: Claude Fable 5.1 --- .../src/code_index_generations/locking.rs | 93 ++++++++++++++++--- .../tests/graph_replay_pool_lock_tests.rs | 47 ++++++++++ .../code_index_generations/text_artifacts.rs | 41 ++++++-- .../repository/graph_publication/support.rs | 86 ++++++++--------- 4 files changed, 199 insertions(+), 68 deletions(-) diff --git a/crates/tracedecay-code-index-retention/src/code_index_generations/locking.rs b/crates/tracedecay-code-index-retention/src/code_index_generations/locking.rs index 1d8d867a75..538e8e43aa 100644 --- a/crates/tracedecay-code-index-retention/src/code_index_generations/locking.rs +++ b/crates/tracedecay-code-index-retention/src/code_index_generations/locking.rs @@ -1,7 +1,11 @@ use std::fs::{File, OpenOptions}; use std::path::{Path, PathBuf}; +use std::time::Instant; -use super::{CodeGenerationRetentionErrorV1, SCOPE_RETENTION_LOCK_FILE, STORE_LOCK_FILE, storage}; +use super::{ + CodeGenerationRetentionErrorV1, GRAPH_REPLAY_POOL_ACQUIRE_BUDGET, + GRAPH_REPLAY_POOL_ACQUIRE_POLL, SCOPE_RETENTION_LOCK_FILE, STORE_LOCK_FILE, storage, +}; pub struct CodeGenerationStoreLockV1 { file: File, @@ -36,7 +40,33 @@ impl Drop for CodeGenerationStoreLockV1 { pub fn acquire_code_generation_store_lock( store_root: &Path, ) -> Result { - lock_file(store_root, STORE_LOCK_FILE, true) + acquire_code_generation_store_lock_checked( + store_root, + Instant::now() + GRAPH_REPLAY_POOL_ACQUIRE_BUDGET, + &|| false, + ) +} + +/// Exclusive generation-store lock that stops at `deadline` or cancellation. +/// +/// A free lock is taken even when the deadline has already elapsed, so a +/// caller that only needs one uncontended critical section is not refused. +/// A held lock returns [`CodeGenerationRetentionErrorV1::GenerationStoreBusy`] +/// or [`CodeGenerationRetentionErrorV1::Cancelled`] instead of blocking in +/// `File::lock`, which cannot observe either signal. +pub fn acquire_code_generation_store_lock_checked( + store_root: &Path, + deadline: Instant, + is_cancelled: &dyn Fn() -> bool, +) -> Result { + lock_file_checked( + store_root, + STORE_LOCK_FILE, + true, + deadline, + is_cancelled, + CodeGenerationRetentionErrorV1::GenerationStoreBusy, + ) } /// Try to hold the generation store as a reader for one bounded read of @@ -81,24 +111,63 @@ pub fn try_acquire_code_generation_store_lock( pub(super) fn acquire_scope_retention_lock( store_root: &Path, ) -> Result { - lock_file(store_root, SCOPE_RETENTION_LOCK_FILE, false) + acquire_scope_retention_lock_checked( + store_root, + Instant::now() + GRAPH_REPLAY_POOL_ACQUIRE_BUDGET, + &|| false, + ) +} + +pub(super) fn acquire_scope_retention_lock_checked( + store_root: &Path, + deadline: Instant, + is_cancelled: &dyn Fn() -> bool, +) -> Result { + lock_file_checked( + store_root, + SCOPE_RETENTION_LOCK_FILE, + false, + deadline, + is_cancelled, + CodeGenerationRetentionErrorV1::GenerationStoreBusy, + ) } #[hotpath::measure(label = "code_index_retention.lock")] -fn lock_file( +fn lock_file_checked( store_root: &Path, lock_file: &str, generation_store: bool, + deadline: Instant, + is_cancelled: &dyn Fn() -> bool, + busy: CodeGenerationRetentionErrorV1, ) -> Result { let store_root = canonical_store_root(store_root)?; - let lock = open_lock_file(&store_root.join(lock_file))?; - lock.lock().map_err(storage)?; - Ok(CodeGenerationStoreLockV1 { - file: lock, - store_root, - generation_store, - shared: false, - }) + let deadline = deadline.min(Instant::now() + GRAPH_REPLAY_POOL_ACQUIRE_BUDGET); + loop { + if is_cancelled() { + return Err(CodeGenerationRetentionErrorV1::Cancelled); + } + let lock = open_lock_file(&store_root.join(lock_file))?; + match lock.try_lock().map_err(std::io::Error::from) { + Ok(()) => { + return Ok(CodeGenerationStoreLockV1 { + file: lock, + store_root, + generation_store, + shared: false, + }); + } + Err(error) if tracedecay_private_fs::is_lock_contended(&error) => { + if Instant::now() >= deadline { + return Err(busy); + } + let remaining = deadline.saturating_duration_since(Instant::now()); + std::thread::park_timeout(remaining.min(GRAPH_REPLAY_POOL_ACQUIRE_POLL)); + } + Err(error) => return Err(storage(error)), + } + } } fn canonical_store_root(store_root: &Path) -> Result { diff --git a/crates/tracedecay-code-index-retention/src/code_index_generations/tests/graph_replay_pool_lock_tests.rs b/crates/tracedecay-code-index-retention/src/code_index_generations/tests/graph_replay_pool_lock_tests.rs index 44f0b28e87..9a0e6f9fb9 100644 --- a/crates/tracedecay-code-index-retention/src/code_index_generations/tests/graph_replay_pool_lock_tests.rs +++ b/crates/tracedecay-code-index-retention/src/code_index_generations/tests/graph_replay_pool_lock_tests.rs @@ -1,6 +1,7 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use std::time::{Duration, Instant}; +use super::super::locking::acquire_code_generation_store_lock_checked; use super::*; fn ensure_replay_pool(pool_root: &std::path::Path) { @@ -306,6 +307,52 @@ fn checked_acquire_returns_busy_when_the_carried_deadline_has_elapsed() { drop(publisher); } +#[test] +fn store_lock_returns_cancelled_without_waiting_out_the_budget() { + let (_root, store) = isolated_pool(); + let holder = hold_replay_pool(&store); + let started = Instant::now(); + let error = match acquire_code_generation_store_lock_checked( + &store, + Instant::now() + GRAPH_REPLAY_POOL_ACQUIRE_BUDGET, + &|| true, + ) { + Ok(_) => panic!("cancellation must win a held store lock"), + Err(error) => error, + }; + + assert!(matches!(error, CodeGenerationRetentionErrorV1::Cancelled)); + assert!( + started.elapsed() < Duration::from_millis(20), + "cancelled store lock must not poll the budget, took {:?}", + started.elapsed() + ); + drop(holder); +} + +#[test] +fn store_lock_returns_busy_when_the_carried_deadline_has_elapsed() { + let (_root, store) = isolated_pool(); + let holder = hold_replay_pool(&store); + let started = Instant::now(); + let error = match acquire_code_generation_store_lock_checked(&store, Instant::now(), &|| false) + { + Ok(_) => panic!("an elapsed deadline must defer a held store lock"), + Err(error) => error, + }; + + assert!(matches!( + error, + CodeGenerationRetentionErrorV1::GenerationStoreBusy + )); + assert!( + started.elapsed() < Duration::from_millis(20), + "an expired held store lock must not poll, took {:?}", + started.elapsed() + ); + drop(holder); +} + #[test] fn checked_acquire_takes_a_free_pool_and_releases_without_leak() { let (_root, pool) = isolated_pool(); diff --git a/crates/tracedecay-code-index-retention/src/code_index_generations/text_artifacts.rs b/crates/tracedecay-code-index-retention/src/code_index_generations/text_artifacts.rs index d56bcbb6ac..a6e90c8cd9 100644 --- a/crates/tracedecay-code-index-retention/src/code_index_generations/text_artifacts.rs +++ b/crates/tracedecay-code-index-retention/src/code_index_generations/text_artifacts.rs @@ -442,13 +442,15 @@ pub(super) fn plan_collectable_text_artifacts_cancellable( } else { verification }; - verify_unreferenced_completed_text_artifact( + if !verify_unreferenced_completed_text_artifact( &path, digest, metadata.len(), candidate_verification, is_cancelled, - )?; + )? { + continue; + } Some(CodeTextArtifactRetentionCandidateV1 { artifact_file: file_name, kind: CodeTextArtifactRetentionKindV1::Completed, @@ -554,13 +556,19 @@ pub(super) fn verify_completed_text_artifact( is_cancelled: &dyn Fn() -> bool, ) -> Result<(), CodeGenerationRetentionErrorV1> { let digest = sha256_file_component(&descriptor.artifact_digest, "text artifact")?; - verify_unreferenced_completed_text_artifact( + if !verify_unreferenced_completed_text_artifact( path, digest, descriptor.artifact_size_bytes, verification, is_cancelled, - ) + )? { + return Err(CodeGenerationRetentionErrorV1::UnsafeState(format!( + "code text artifact '{}' disappeared while its identity was being verified", + path.display() + ))); + } + Ok(()) } /// A content-addressed path is trusted only after the open file and its path @@ -573,15 +581,23 @@ pub(super) fn verify_unreferenced_completed_text_artifact( expected_size_bytes: u64, verification: GenerationDigestVerificationV1, is_cancelled: &dyn Fn() -> bool, -) -> Result<(), CodeGenerationRetentionErrorV1> { - let before = std::fs::symlink_metadata(path).map_err(storage)?; +) -> Result { + let before = match std::fs::symlink_metadata(path) { + Ok(metadata) => metadata, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(false), + Err(error) => return Err(storage(error)), + }; if !before.file_type().is_file() || before.len() != expected_size_bytes { return Err(CodeGenerationRetentionErrorV1::UnsafeState(format!( "code text artifact '{}' has an invalid regular-file identity", path.display() ))); } - let file = File::open(path).map_err(storage)?; + let file = match File::open(path) { + Ok(file) => file, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(false), + Err(error) => return Err(storage(error)), + }; if !path_still_names_open_file(path, &file, &before)? { return Err(CodeGenerationRetentionErrorV1::UnsafeState(format!( "code text artifact '{}' changed while its identity was being verified", @@ -602,7 +618,7 @@ pub(super) fn verify_unreferenced_completed_text_artifact( path.display() ))); } - Ok(()) + Ok(true) } /// `active_pointer` is the pointer the store carries *now*, which is not @@ -861,13 +877,18 @@ pub(super) fn stage_collectable_text_artifacts_cancellable( } else { GenerationDigestVerificationV1::Full }; - verify_unreferenced_completed_text_artifact( + if !verify_unreferenced_completed_text_artifact( &source, digest, candidate.size_bytes, candidate_verification, is_cancelled, - )?; + )? { + return Err(CodeGenerationRetentionErrorV1::UnsafeState(format!( + "text-artifact candidate '{}' disappeared before quarantine", + candidate.artifact_file + ))); + } } if observe_cancel(is_cancelled) { return Err(CodeGenerationRetentionErrorV1::Cancelled); diff --git a/crates/tracedecay-rusqlite-runtime/src/repository/graph_publication/support.rs b/crates/tracedecay-rusqlite-runtime/src/repository/graph_publication/support.rs index c101e90f35..a698675b02 100644 --- a/crates/tracedecay-rusqlite-runtime/src/repository/graph_publication/support.rs +++ b/crates/tracedecay-rusqlite-runtime/src/repository/graph_publication/support.rs @@ -1,5 +1,4 @@ use std::collections::HashMap; -use std::time::Duration; use tracedecay_store::{ GraphDependencyGenerationIdentityV1, GraphGenerationIdV1, GraphNamespaceV1, @@ -28,8 +27,6 @@ use super::{ REPLAY_READER_ACQUIRE_SLICE, TOMBSTONE_COLUMNS, }; -const BEGIN_BUSY_ATTEMPT_BUDGET: u32 = 64; - /// Maximum owner sequences bound into one `IN (...)` dependency lookup. /// /// This is **not** `REFERENCED_ANCHOR_BATCH` from @@ -64,32 +61,36 @@ pub(super) fn begin( handle: &ExactSqlHandle, context: &GraphPublicationOperationContextV1<'_>, ) -> GraphPublicationStoreResultV1 { - let mut busy_attempts = 0_u32; - loop { - ensure_not_interrupted(context)?; - match hotpath::measure_block!("rusqlite.graph_publication.begin_immediate", { - handle.begin_immediate() - }) { - Ok(transaction) => { - ensure_not_interrupted(context)?; - return Ok(transaction); - } - Err(ExactSqlError::Busy) => { - busy_attempts = busy_attempts.saturating_add(1); - if busy_attempts >= BEGIN_BUSY_ATTEMPT_BUDGET { - return Err(GraphPublicationStoreErrorV1::Infrastructure); - } - std::thread::sleep(Duration::from_millis(1)); - ensure_not_interrupted(context)?; - } - Err(_) => { - ensure_not_interrupted(context)?; - return Err(GraphPublicationStoreErrorV1::Infrastructure); - } + // `begin_immediate` already waits the writer lock budget and observes + // channel close. Repeating that wait multiplied a 64ms acquire into + // several seconds and reported `Infrastructure` after the caller's + // deadline or cancellation had already fired between sleeps. One attempt + // is the lock answer; interruption stays `Interrupted`. + ensure_not_interrupted(context)?; + match hotpath::measure_block!("rusqlite.graph_publication.begin_immediate", { + handle.begin_immediate() + }) { + Ok(transaction) => { + ensure_not_interrupted(context)?; + Ok(transaction) + } + Err(ExactSqlError::Busy) => Err(busy_after_deadline(context)), + Err(_) => { + ensure_not_interrupted(context)?; + Err(GraphPublicationStoreErrorV1::Infrastructure) } } } +fn busy_after_deadline( + context: &GraphPublicationOperationContextV1<'_>, +) -> GraphPublicationStoreErrorV1 { + match context.interruption() { + Some(reason) => GraphPublicationStoreErrorV1::Interrupted(reason), + None => GraphPublicationStoreErrorV1::Infrastructure, + } +} + pub(super) fn ensure_owner( handle: &ExactSqlHandle, projection: &GraphProjectionIdentityV1, @@ -128,29 +129,22 @@ pub(super) fn begin_read( handle: &ExactSqlHandle, context: &GraphPublicationOperationContextV1<'_>, ) -> GraphPublicationStoreResultV1 { - let mut busy_attempts = 0_u32; - loop { - ensure_not_interrupted(context)?; - match hotpath::measure_block!("rusqlite.graph_publication.begin_read_snapshot", { - handle.begin_read_snapshot(REPLAY_READER_ACQUIRE_SLICE) - }) { - Ok(snapshot) => { - ensure_not_interrupted(context)?; - return Ok(ExactPublicationRead::Snapshot(snapshot)); - } - Err(ExactSqlError::Busy) => { - busy_attempts = busy_attempts.saturating_add(1); - if busy_attempts >= BEGIN_BUSY_ATTEMPT_BUDGET { - break; - } - std::thread::sleep(Duration::from_millis(1)); - ensure_not_interrupted(context)?; - } - Err(_) => { - ensure_not_interrupted(context)?; - break; + ensure_not_interrupted(context)?; + match hotpath::measure_block!("rusqlite.graph_publication.begin_read_snapshot", { + handle.begin_read_snapshot(REPLAY_READER_ACQUIRE_SLICE) + }) { + Ok(snapshot) => { + ensure_not_interrupted(context)?; + return Ok(ExactPublicationRead::Snapshot(snapshot)); + } + Err(ExactSqlError::Busy) => { + if let Some(reason) = context.interruption() { + return Err(GraphPublicationStoreErrorV1::Interrupted(reason)); } } + Err(_) => { + ensure_not_interrupted(context)?; + } } hotpath::measure_block!("rusqlite.graph_publication.begin_deferred", { handle From 2283a446b5762aff76609229e2469699b074b38c Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 08:15:48 +0000 Subject: [PATCH 07/10] fix(graph-publication): bound begin by wall clock, not one attempt Dropping the 64-attempt busy loop stopped a cancelled caller waiting several seconds, but `ExactSqlError::Busy` answers two questions with one variant: `map_writer_send_error` returns it when the exact-SQL command queue is full, and `begin_immediate` returns it when the writer's own 64ms `EXACT_SQL_WRITE_LOCK_ACQUIRE_LIMIT` loop is exhausted. Treating the first as a final lock answer turned ordinary queue backpressure into `Infrastructure`, which publication reads as `unavailable`. Bound the acquisition by one 64ms wall clock instead of an attempt count. An exhausted lock attempt has already spent that window inside `begin_immediate`, so it still gets exactly one attempt and is never multiplied; a queue refusal returns at once and retries while the window lasts, re-checking interruption each pass. `begin_read` uses the same budget, restoring several context-checked 10ms reader slices before the deferred fallback, and now checks interruption immediately before that fallback, which waits the writer without consulting the context. Resolves the Codex review findings on PR #1821 (P2 admission busy, P1 reader cancellability). Co-Authored-By: Claude Fable 5.1 (cherry picked from commit 014ba1d87481cb2fa726aa22cf4298bdb4e21745) --- .../repository/graph_publication/support.rs | 105 +++++++++++------- 1 file changed, 62 insertions(+), 43 deletions(-) diff --git a/crates/tracedecay-rusqlite-runtime/src/repository/graph_publication/support.rs b/crates/tracedecay-rusqlite-runtime/src/repository/graph_publication/support.rs index a698675b02..a8a90bf26d 100644 --- a/crates/tracedecay-rusqlite-runtime/src/repository/graph_publication/support.rs +++ b/crates/tracedecay-rusqlite-runtime/src/repository/graph_publication/support.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::time::{Duration, Instant}; use tracedecay_store::{ GraphDependencyGenerationIdentityV1, GraphGenerationIdV1, GraphNamespaceV1, @@ -56,39 +57,64 @@ use super::{ /// exceeded the row cap in one chunk. const GRAPH_REPLAY_DEPENDENCY_BATCH: usize = 38; -#[hotpath::measure(label = "rusqlite.graph_publication.begin")] -pub(super) fn begin( - handle: &ExactSqlHandle, +/// Wall-clock budget for one begin acquisition, retries included. +/// +/// `ExactSqlError::Busy` answers two different questions with one variant: the +/// exact-SQL command queue was full (`map_writer_send_error`), or the writer's +/// own `EXACT_SQL_WRITE_LOCK_ACQUIRE_LIMIT` lock loop was exhausted. Only the +/// first is worth another attempt. A wall-clock budget separates them without a +/// second error variant: an exhausted lock attempt has already spent this whole +/// window inside `begin_immediate`, so it gets exactly one attempt and its +/// 64ms answer is never multiplied into seconds, while a queue refusal returns +/// at once and still has the window to drain in. +const BEGIN_ACQUIRE_BUDGET: Duration = Duration::from_millis(64); + +/// Pause between admission retries, matching the writer's own busy pause. +const BEGIN_BUSY_RETRY_PAUSE: Duration = Duration::from_millis(1); + +/// Runs `attempt` until it answers, the budget is spent, or the caller is +/// interrupted. `Ok(None)` means no answer within the budget; the caller owns +/// what that means for its own operation. +fn acquire_within_begin_budget( context: &GraphPublicationOperationContextV1<'_>, -) -> GraphPublicationStoreResultV1 { - // `begin_immediate` already waits the writer lock budget and observes - // channel close. Repeating that wait multiplied a 64ms acquire into - // several seconds and reported `Infrastructure` after the caller's - // deadline or cancellation had already fired between sleeps. One attempt - // is the lock answer; interruption stays `Interrupted`. - ensure_not_interrupted(context)?; - match hotpath::measure_block!("rusqlite.graph_publication.begin_immediate", { - handle.begin_immediate() - }) { - Ok(transaction) => { - ensure_not_interrupted(context)?; - Ok(transaction) - } - Err(ExactSqlError::Busy) => Err(busy_after_deadline(context)), - Err(_) => { - ensure_not_interrupted(context)?; - Err(GraphPublicationStoreErrorV1::Infrastructure) + mut attempt: impl FnMut() -> Result, +) -> GraphPublicationStoreResultV1> { + let deadline = Instant::now() + BEGIN_ACQUIRE_BUDGET; + loop { + ensure_not_interrupted(context)?; + match attempt() { + Ok(value) => { + ensure_not_interrupted(context)?; + return Ok(Some(value)); + } + Err(ExactSqlError::Busy) => { + if let Some(reason) = context.interruption() { + return Err(GraphPublicationStoreErrorV1::Interrupted(reason)); + } + if Instant::now() >= deadline { + return Ok(None); + } + std::thread::sleep(BEGIN_BUSY_RETRY_PAUSE); + } + Err(_) => { + ensure_not_interrupted(context)?; + return Ok(None); + } } } } -fn busy_after_deadline( +#[hotpath::measure(label = "rusqlite.graph_publication.begin")] +pub(super) fn begin( + handle: &ExactSqlHandle, context: &GraphPublicationOperationContextV1<'_>, -) -> GraphPublicationStoreErrorV1 { - match context.interruption() { - Some(reason) => GraphPublicationStoreErrorV1::Interrupted(reason), - None => GraphPublicationStoreErrorV1::Infrastructure, - } +) -> GraphPublicationStoreResultV1 { + acquire_within_begin_budget(context, || { + hotpath::measure_block!("rusqlite.graph_publication.begin_immediate", { + handle.begin_immediate() + }) + })? + .ok_or(GraphPublicationStoreErrorV1::Infrastructure) } pub(super) fn ensure_owner( @@ -129,23 +155,16 @@ pub(super) fn begin_read( handle: &ExactSqlHandle, context: &GraphPublicationOperationContextV1<'_>, ) -> GraphPublicationStoreResultV1 { - ensure_not_interrupted(context)?; - match hotpath::measure_block!("rusqlite.graph_publication.begin_read_snapshot", { - handle.begin_read_snapshot(REPLAY_READER_ACQUIRE_SLICE) - }) { - Ok(snapshot) => { - ensure_not_interrupted(context)?; - return Ok(ExactPublicationRead::Snapshot(snapshot)); - } - Err(ExactSqlError::Busy) => { - if let Some(reason) = context.interruption() { - return Err(GraphPublicationStoreErrorV1::Interrupted(reason)); - } - } - Err(_) => { - ensure_not_interrupted(context)?; - } + if let Some(snapshot) = acquire_within_begin_budget(context, || { + hotpath::measure_block!("rusqlite.graph_publication.begin_read_snapshot", { + handle.begin_read_snapshot(REPLAY_READER_ACQUIRE_SLICE) + }) + })? { + return Ok(ExactPublicationRead::Snapshot(snapshot)); } + // The deferred fallback waits the writer without consulting `context`, so + // this is the last point that can answer a cancelled or expired caller. + ensure_not_interrupted(context)?; hotpath::measure_block!("rusqlite.graph_publication.begin_deferred", { handle .begin_deferred() From 22b263c045f34ea2d6110cfa85817e5ce7b2f1ef Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 08:23:11 +0000 Subject: [PATCH 08/10] test(retention): observe pool acquires per thread, not per process `checked_acquire_returns_busy_when_the_carried_deadline_has_elapsed` asserts the exact `(1, 0)` acquire shape after resetting a counter, but GRAPH_REPLAY_POOL_ACQUIRE_TRIES/WAITS were process-wide statics. The harness runs the other pool-acquire tests in parallel on their own threads, so any acquire landing between this test's reset and its read inflated the proof: it failed 2 of 25 `-p tracedecay-code-index-retention --lib` runs with `left: (5, 3)`. An acquire runs on its caller's thread, so the observation belongs there. Move both counters to `thread_local!` cells. The assertion is unchanged and now proves only the acquire the test performed; 30 consecutive suite runs are clean. Co-Authored-By: Claude Fable 5.1 (cherry picked from commit 1dca5ba27271b4b096ba95db2cfd7747bd59285e) --- .../generation_transactions.rs | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/crates/tracedecay-code-index-retention/src/code_index_generations/generation_transactions.rs b/crates/tracedecay-code-index-retention/src/code_index_generations/generation_transactions.rs index dc00b2bbb6..a87577aeee 100644 --- a/crates/tracedecay-code-index-retention/src/code_index_generations/generation_transactions.rs +++ b/crates/tracedecay-code-index-retention/src/code_index_generations/generation_transactions.rs @@ -2,14 +2,14 @@ //! //! Quarantined generations are journaled, then hard-linked into the replay pool before the receipt is durable. +#[cfg(test)] +use std::cell::Cell; use std::collections::BTreeSet; use std::fs::File; use std::io::Read; #[cfg(unix)] use std::os::unix::fs::MetadataExt; use std::path::{Path, PathBuf}; -#[cfg(test)] -use std::sync::atomic::{AtomicUsize, Ordering}; use std::time::Instant; use sha2::{Digest, Sha256}; @@ -264,23 +264,30 @@ pub(super) fn acquire_graph_replay_pool_lock_checked( GraphReplayPoolLockV1::acquire_exclusive(pool_root, deadline, is_cancelled) } +// Per-thread, not process-wide: an acquire runs on its caller's thread, and +// the test harness runs the other acquire tests in parallel on their own +// threads. Shared statics let any concurrent acquire land between a test's +// reset and its read, which is what turned the exact `(1, 0)` proof into an +// occasional `(5, 3)`. #[cfg(test)] -static GRAPH_REPLAY_POOL_ACQUIRE_TRIES: AtomicUsize = AtomicUsize::new(0); -#[cfg(test)] -static GRAPH_REPLAY_POOL_ACQUIRE_WAITS: AtomicUsize = AtomicUsize::new(0); +thread_local! { + static GRAPH_REPLAY_POOL_ACQUIRE_TRIES: Cell = const { Cell::new(0) }; + static GRAPH_REPLAY_POOL_ACQUIRE_WAITS: Cell = const { Cell::new(0) }; +} #[cfg(test)] pub(super) fn reset_graph_replay_pool_acquire_observation() { - GRAPH_REPLAY_POOL_ACQUIRE_TRIES.store(0, Ordering::SeqCst); - GRAPH_REPLAY_POOL_ACQUIRE_WAITS.store(0, Ordering::SeqCst); + GRAPH_REPLAY_POOL_ACQUIRE_TRIES.with(|tries| tries.set(0)); + GRAPH_REPLAY_POOL_ACQUIRE_WAITS.with(|waits| waits.set(0)); } -/// `(non_blocking_tries, wait_for_exclusive_calls)` since the last reset. +/// `(non_blocking_tries, wait_for_exclusive_calls)` on this thread since the +/// last reset. #[cfg(test)] pub(super) fn graph_replay_pool_acquire_observation() -> (usize, usize) { ( - GRAPH_REPLAY_POOL_ACQUIRE_TRIES.load(Ordering::SeqCst), - GRAPH_REPLAY_POOL_ACQUIRE_WAITS.load(Ordering::SeqCst), + GRAPH_REPLAY_POOL_ACQUIRE_TRIES.with(Cell::get), + GRAPH_REPLAY_POOL_ACQUIRE_WAITS.with(Cell::get), ) } @@ -307,7 +314,7 @@ impl GraphReplayPoolLockV1 { // the budget is gone. Windows lock-conflict is `Ok(None)` via // `is_lock_contended`, not Storage. #[cfg(test)] - GRAPH_REPLAY_POOL_ACQUIRE_TRIES.fetch_add(1, Ordering::SeqCst); + GRAPH_REPLAY_POOL_ACQUIRE_TRIES.with(|tries| tries.set(tries.get() + 1)); match try_acquire_code_generation_store_lock(pool_root)? { Some(guard) => { crate::hotpath_observe::retention_replay_pool_acquired(); @@ -327,7 +334,7 @@ impl GraphReplayPoolLockV1 { fn wait_for_exclusive(deadline: Instant) { #[cfg(test)] - GRAPH_REPLAY_POOL_ACQUIRE_WAITS.fetch_add(1, Ordering::SeqCst); + GRAPH_REPLAY_POOL_ACQUIRE_WAITS.with(|waits| waits.set(waits.get() + 1)); crate::hotpath_observe::retention_replay_pool_acquire_wait(); let remaining = deadline.saturating_duration_since(Instant::now()); if remaining.is_zero() { From db2a28a1ab0a2cc4395f8c27f9c33bf725589409 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 16:51:33 +0000 Subject: [PATCH 09/10] test(retention): plan an unpublished store for a missing root preparation_defers_when_the_scope_root_does_not_exist_yet asserted GenerationStoreBusy for a store directory that does not exist. 690e84365d made that deferral to stop a missing path being Storage(NotFound). 218ae5cbf3 then chose a different answer for this one input: a store directory that has not been created is an unpublished plan, because a waiter can plan against latest_generation_id before cold open creates the scoped store. It did not update this test, so the assertion has been stale since. The expectation now names the contract 218ae5cbf3 established, and expect() still fails the test if the call returns any error, which is the defect 690e84365d fixed. It also pins the plan shape a waiter reads: no active generation, no pointer, nothing collectable, and the caller's readable sources carried through. Verified failing on origin/ci/pr-batch-d with the crate otherwise unmodified, so this is not a regression from the four folded deltas. Co-Authored-By: Claude Fable 5.1 --- .../src/code_index_generations/tests.rs | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/crates/tracedecay-code-index-retention/src/code_index_generations/tests.rs b/crates/tracedecay-code-index-retention/src/code_index_generations/tests.rs index d74da1bc10..76dda355ef 100644 --- a/crates/tracedecay-code-index-retention/src/code_index_generations/tests.rs +++ b/crates/tracedecay-code-index-retention/src/code_index_generations/tests.rs @@ -1548,19 +1548,22 @@ fn idle_maintenance_preparation_stays_metadata_only() { } #[test] -fn preparation_defers_when_the_scope_root_does_not_exist_yet() { +fn preparation_plans_an_unpublished_store_when_the_scope_root_is_missing() { let parent = tempfile::TempDir::new().expect("parent"); let missing = parent.path().join("not-created"); - let error = prepare_next_code_generation_retention_cancellable( - &missing, - &BTreeSet::new(), - &|| false, - None, - ) - .expect_err("an unpublished scope root has no census"); + let sources = BTreeSet::from([CodeGenerationId::new("generation.waiter").expect("id")]); + let plan = + prepare_next_code_generation_retention_cancellable(&missing, &sources, &|| false, None) + .expect("a missing scope root is the publisher's create window, not a failure"); + assert_eq!(plan.active_generation_id, None); + assert_eq!(plan.active_pointer, None); assert!( - matches!(error, CodeGenerationRetentionErrorV1::GenerationStoreBusy), - "a missing scope root is the publisher's create window, not a storage failure: {error:?}" + !plan.has_collectable_work(), + "a store that was never published has nothing to collect: {plan:?}" + ); + assert_eq!( + plan.vector_readable_sources, sources, + "the caller's readable sources survive the unpublished plan" ); } From e38482573aa45fa4678b404d3e51082c8ad28ba1 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 17:18:18 +0000 Subject: [PATCH 10/10] refactor(retention): drop the unused checked-lock scaffolding Findings from the deslop and no-comments pass over this batch's added lines. Every change is to a line this batch introduced. locking.rs carried a wrapper pair and a one-valued parameter that nothing used. acquire_scope_retention_lock_checked had exactly one caller, the five-line wrapper directly above it, so the two collapse into one function. lock_file_checked had no unchecked sibling to distinguish it from, and its `busy` parameter was GenerationStoreBusy at both call sites. Only acquire_code_generation_store_lock_checked keeps its deadline and cancel parameters, because the pool-lock tests call it with both to prove the busy and cancelled answers. It drops to pub(super): mod locking is private and the crate re-export never named it. registry.rs: note_worker_continuation's doc said stamping a wall-clock instant was the defect, but the function still stamps one and must, or has_pending_arrival stops seeing the continuation. The defect was publishing that stamp as attributable, so the doc now names attributable = false. graph_publication/support.rs: acquire_within_begin_budget returns Ok(None) for a non-Busy error as well as for an expired budget, which its doc did not say. The Busy arm also re-implemented ensure_not_interrupted, which the same loop already calls four lines above. Co-Authored-By: Claude Fable 5.1 --- .../src/code_index_generations/locking.rs | 35 ++++--------------- .../src/code_index_scheduler/registry.rs | 9 +++-- .../repository/graph_publication/support.rs | 10 +++--- 3 files changed, 15 insertions(+), 39 deletions(-) diff --git a/crates/tracedecay-code-index-retention/src/code_index_generations/locking.rs b/crates/tracedecay-code-index-retention/src/code_index_generations/locking.rs index 538e8e43aa..8b33b98a08 100644 --- a/crates/tracedecay-code-index-retention/src/code_index_generations/locking.rs +++ b/crates/tracedecay-code-index-retention/src/code_index_generations/locking.rs @@ -54,19 +54,12 @@ pub fn acquire_code_generation_store_lock( /// A held lock returns [`CodeGenerationRetentionErrorV1::GenerationStoreBusy`] /// or [`CodeGenerationRetentionErrorV1::Cancelled`] instead of blocking in /// `File::lock`, which cannot observe either signal. -pub fn acquire_code_generation_store_lock_checked( +pub(super) fn acquire_code_generation_store_lock_checked( store_root: &Path, deadline: Instant, is_cancelled: &dyn Fn() -> bool, ) -> Result { - lock_file_checked( - store_root, - STORE_LOCK_FILE, - true, - deadline, - is_cancelled, - CodeGenerationRetentionErrorV1::GenerationStoreBusy, - ) + lock_file(store_root, STORE_LOCK_FILE, true, deadline, is_cancelled) } /// Try to hold the generation store as a reader for one bounded read of @@ -111,36 +104,22 @@ pub fn try_acquire_code_generation_store_lock( pub(super) fn acquire_scope_retention_lock( store_root: &Path, ) -> Result { - acquire_scope_retention_lock_checked( - store_root, - Instant::now() + GRAPH_REPLAY_POOL_ACQUIRE_BUDGET, - &|| false, - ) -} - -pub(super) fn acquire_scope_retention_lock_checked( - store_root: &Path, - deadline: Instant, - is_cancelled: &dyn Fn() -> bool, -) -> Result { - lock_file_checked( + lock_file( store_root, SCOPE_RETENTION_LOCK_FILE, false, - deadline, - is_cancelled, - CodeGenerationRetentionErrorV1::GenerationStoreBusy, + Instant::now() + GRAPH_REPLAY_POOL_ACQUIRE_BUDGET, + &|| false, ) } #[hotpath::measure(label = "code_index_retention.lock")] -fn lock_file_checked( +fn lock_file( store_root: &Path, lock_file: &str, generation_store: bool, deadline: Instant, is_cancelled: &dyn Fn() -> bool, - busy: CodeGenerationRetentionErrorV1, ) -> Result { let store_root = canonical_store_root(store_root)?; let deadline = deadline.min(Instant::now() + GRAPH_REPLAY_POOL_ACQUIRE_BUDGET); @@ -160,7 +139,7 @@ fn lock_file_checked( } Err(error) if tracedecay_private_fs::is_lock_contended(&error) => { if Instant::now() >= deadline { - return Err(busy); + return Err(CodeGenerationRetentionErrorV1::GenerationStoreBusy); } let remaining = deadline.saturating_duration_since(Instant::now()); std::thread::park_timeout(remaining.min(GRAPH_REPLAY_POOL_ACQUIRE_POLL)); diff --git a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/registry.rs b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/registry.rs index 4413afd940..b548f36d21 100644 --- a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/registry.rs +++ b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/registry.rs @@ -2116,11 +2116,10 @@ impl CodeIndexSchedulerRegistryV1 { /// Queue worker-owned continuation work so freshness still sees it. /// /// A bare `Notify` permit is not observable by freshness readers, so the - /// pending slot stays nonzero. That slot is not an external arrival: the - /// worker decided to continue work an earlier wake already claimed. - /// Stamping a wall-clock instant here made the follow-up pass publish an - /// event-to-ready receipt, and a suppressed freshness probe that raced the - /// stamp was charged with it. + /// slot is stamped like an arrival. It is not one. The worker decided to + /// continue work an earlier wake already claimed. `attributable = false` + /// keeps that stamp out of the event-to-ready receipt, which otherwise + /// charged a suppressed freshness probe that raced it. fn note_worker_continuation(pending_wake: &PendingWakeV1, wake: &tokio::sync::Notify) { let mut state = pending_wake .state diff --git a/crates/tracedecay-rusqlite-runtime/src/repository/graph_publication/support.rs b/crates/tracedecay-rusqlite-runtime/src/repository/graph_publication/support.rs index a8a90bf26d..9b77210313 100644 --- a/crates/tracedecay-rusqlite-runtime/src/repository/graph_publication/support.rs +++ b/crates/tracedecay-rusqlite-runtime/src/repository/graph_publication/support.rs @@ -72,9 +72,9 @@ const BEGIN_ACQUIRE_BUDGET: Duration = Duration::from_millis(64); /// Pause between admission retries, matching the writer's own busy pause. const BEGIN_BUSY_RETRY_PAUSE: Duration = Duration::from_millis(1); -/// Runs `attempt` until it answers, the budget is spent, or the caller is -/// interrupted. `Ok(None)` means no answer within the budget; the caller owns -/// what that means for its own operation. +/// Runs `attempt` until it answers or the caller is interrupted. +/// `Ok(None)` means no value: the budget expired under `Busy`, or the attempt +/// failed outright. The caller owns what that means for its own operation. fn acquire_within_begin_budget( context: &GraphPublicationOperationContextV1<'_>, mut attempt: impl FnMut() -> Result, @@ -88,9 +88,7 @@ fn acquire_within_begin_budget( return Ok(Some(value)); } Err(ExactSqlError::Busy) => { - if let Some(reason) = context.interruption() { - return Err(GraphPublicationStoreErrorV1::Interrupted(reason)); - } + ensure_not_interrupted(context)?; if Instant::now() >= deadline { return Ok(None); }