From cacf79a8d2e31620021c0d294e1a4fdb431fbc27 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 17:51:24 +0000 Subject: [PATCH] fix(code-index): hold the head-open claim across the clone copy `HeadOpening` is the exclusive claim on one generation's text projection: `advance_artifact_text_serving` parks every other wake while a claim is live. `open_published_text_artifact` broke that claim from the inside. It called `retain_clone_successor_retry` before `begin_clone_successor`, so the slot became `CloneSuccessorPending` for the whole copy of the prior lexical artifact, with the slot lock released. `CloneSuccessorPending` is one of the states the park loop breaks out of, so a concurrent wake took a second `HeadOpening` on top of the running open. Two wakes then drove the same clone staging database, and whichever open resolved second found the slot already reset by the other's claim. The losing wake reported `Contract("clone-successor retry requires an active head-open claim")`, which the clone lanes map to `Internal`, so `tracedecay_redundancy` answered `reason_code=search_failed retryable=false`. The earlier fix mapped `AuthorityUnavailable` to `generation_unverified` on the assumption that the remaining refusals were transient authority failures; they were this contract violation, and a retryable verdict would only have hidden it. Keep the slot in `HeadOpening` for the copy. A successful begin resolves the claim to `BuildingCloneSuccessor` anyway, so the marker only mattered when the begin failed: the owners installed just above would otherwise let the next wake short-circuit on a plain `Idle` and never owe the successor again. Park it on that path only. Evidence: with the redundancy journey pinned to two CPUs under busy-loop load, `graph_query_test::redundancy_pull_request_scope_shares_one_budget_and_resumes_changed_families` failed 10 of 20 iterations before this change and 20 of 20 pass after. The new scheduler test drives four concurrent wakes from `CloneSuccessorPending`; without the fix it reports `lane authority is unavailable: no such table: clone_successor_state`, the second wake having dropped the table the first was still writing. Co-Authored-By: Claude Fable 5.1 --- .../src/code_index_scheduler/serving.rs | 35 ++++++-- .../src/code_index_scheduler/tests/serving.rs | 82 +++++++++++++++++++ 2 files changed, 112 insertions(+), 5 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 cb082cfd60..e5f360fbe6 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 @@ -2769,11 +2769,36 @@ impl LatestCodeTextGenerationV1 { self.install_artifact_owners(reader, reader_reservation)?; self.publish_text_progress_snapshot(ready_progress); if needs_clone_successor { - self.text_projection_build.retain_clone_successor_retry()?; - return self - .begin_clone_successor(descriptor, prior, sealed_identity, source, control) - .map(TextHeadOpenOutcomeV1::BuildCloneSuccessor) - .map(Some); + // `begin_clone_successor` copies the whole prior lexical + // artifact with the slot lock released, so this wake's + // head-open claim has to span it. Parking + // `CloneSuccessorPending` before the copy published a + // takeable state mid-claim: `advance_artifact_text_serving` + // leaves its park loop on that state, so a concurrent wake + // took a second `HeadOpening` on top of this open and both + // drove the same staging database. Whichever open resolved + // second then found the slot already reset and failed the + // clone lane closed. A successful begin resolves the claim + // to `BuildingCloneSuccessor` anyway, so only a failed one + // needs the retry marker: the owners installed above would + // otherwise let the next wake short-circuit on a plain + // `Idle` and never owe the successor again. + return match self.begin_clone_successor( + descriptor, + prior, + sealed_identity, + source, + control, + ) { + Ok(build) => Ok(Some(TextHeadOpenOutcomeV1::BuildCloneSuccessor(build))), + Err(error) => { + // The claim still owns `HeadOpening`, so this only + // parks the marker; the begin failure is the one + // worth reporting. + let _ = self.text_projection_build.retain_clone_successor_retry(); + Err(error) + } + }; } drop(source); Ok(Some(TextHeadOpenOutcomeV1::Served)) 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 03ebce7c9e..21f8baa73e 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 @@ -942,6 +942,88 @@ fn lexical_readiness_leaves_the_clone_successor_uncopied() { assert_eq!(revision, 16); } +/// Only one wake at a time may own a head-open claim. +/// +/// `open_published_text_artifact` used to park `CloneSuccessorPending` +/// before `begin_clone_successor` copied the whole prior artifact, and +/// `advance_artifact_text_serving` leaves its park loop on that state. A +/// concurrent wake took a second `HeadOpening` on top of the first open, +/// both drove the same staging database, and whichever open resolved second +/// found the slot already reset and refused with `clone-successor retry +/// requires an active head-open claim`. The clone lanes report that refusal +/// as a non-retryable `search_failed`. +#[test] +fn concurrent_wakes_never_overlap_the_clone_successor_head_open() { + let sources = (0..24) + .map(|index| { + ( + format!("src/module_{index}.rs"), + format!( + "pub fn alpha_{index}() {{ one(); two(); three(); four(); five(); six(); seven(); eight(); nine(); ten(); }}\npub fn beta_{index}() {{ one(); two(); three(); four(); five(); six(); seven(); eight(); nine(); ten(); }}\n" + ), + ) + }) + .collect::>(); + let files = sources + .iter() + .map(|(path, contents)| (path.as_str(), contents.as_str())) + .collect::>(); + let fixture = GitFixture::new(&files); + 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 lexical build"); + } + assert!( + matches!( + &*latest.text_projection_build.lock_slot(), + super::super::CodeTextProjectionSlotV1::CloneSuccessorPending + ), + "the successor must still be owed when the wakes start" + ); + + let workers = (0..4) + .map(|_| { + let latest = latest.clone(); + thread::spawn(move || { + let mut advances = 0_usize; + while latest.text_projection_needs_work() && advances < 400 { + latest.advance_text_serving(1)?; + advances += 1; + } + Ok(()) + }) + }) + .collect::>(); + for worker in workers { + worker + .join() + .expect("wake thread joins") + .unwrap_or_else(|error: RetrievalPortError| { + panic!("a concurrent wake failed the text projection: {error}") + }); + } + + assert!(!latest.text_projection_needs_work()); + 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, "the clone successor must have sealed"); +} + #[test] fn clone_status_distinguishes_unavailable_backfill_partial_ready_and_stale() { let fixture = GitFixture::new(&[(