From 740052a1ce9ed9140e8a102619a960f579d5ffe8 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 04:37:09 +0000 Subject: [PATCH 01/17] fix(sessions): refuse an authoritative zero from a partial generation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `map_execution_error` mapped `SessionTemporalExecutionError::Empty` to `CompleteZero` whatever freshness rode with it. After a hook ingest commits rows, the project store publishes the temporal generation but its relation receipt is applied later by the LCM summary convergence page, so `root_readiness` answers `Partial { generation_lag: 1 }` and the candidate cohort is empty. The first `tracedecay_message_search` then reported `outcome: complete_zero` — "nothing exists, and that is final" — for a transcript the store had already committed. `map_report` already refuses that for an empty ranked page: a `Partial` generation returns the typed partial outcome instead. The execution-error path owed the same refusal. It now answers `Partial { items: [], omitted: generation_lag }`, which the retained message-search surface already renders as `outcome: "partial"`, so a caller re-reads rather than believing the zero. `production_codex_hook_ingest_survives_message_search_reopen` loops on that typed partial the way the refresh assertions loop on `running`, and still fails on an empty `complete_zero` or any other outcome. Measured on `taskset -c 0,1`, 6 concurrent copies, 42 runs: 25 failures before, 0 after. Co-Authored-By: Claude Fable 5.1 --- .../src/session/retrieval.rs | 26 +++++++ .../mcp_handler_test/session_search_test.rs | 69 +++++++++++++------ 2 files changed, 73 insertions(+), 22 deletions(-) diff --git a/crates/tracedecay-session-memory/src/session/retrieval.rs b/crates/tracedecay-session-memory/src/session/retrieval.rs index 2d35c62ea6..da4653155a 100644 --- a/crates/tracedecay-session-memory/src/session/retrieval.rs +++ b/crates/tracedecay-session-memory/src/session/retrieval.rs @@ -565,6 +565,19 @@ fn map_execution_error( SessionTemporalExecutionError::Denied => SessionRetrievalOutcome::Denied, SessionTemporalExecutionError::Unavailable => SessionRetrievalOutcome::Unavailable, SessionTemporalExecutionError::ResetRequired => SessionRetrievalOutcome::ResetRequired, + // A partial generation is a projection still converging, not an + // authoritative empty root: answering `CompleteZero` there publishes + // "nothing exists, and that is final" for rows the store has already + // committed but not yet published. `map_report` refuses that for an + // empty ranked page; the execution-error path owes the same refusal, + // so the caller re-reads instead of believing the zero. + SessionTemporalExecutionError::Empty { + freshness: freshness @ SessionDataFreshness::Partial { generation_lag }, + } => SessionRetrievalOutcome::Partial { + items: Vec::new(), + freshness, + omitted: generation_lag.max(1), + }, SessionTemporalExecutionError::Empty { freshness } => { SessionRetrievalOutcome::CompleteZero { freshness } } @@ -1039,6 +1052,19 @@ mod tests { ); } + #[test] + fn partial_generation_empty_execution_is_never_an_authoritative_zero() { + let freshness = SessionDataFreshness::Partial { generation_lag: 1 }; + assert_eq!( + map_execution_error(SessionTemporalExecutionError::Empty { freshness }), + SessionRetrievalOutcome::Partial { + items: Vec::new(), + freshness, + omitted: 1, + } + ); + } + #[test] fn persisted_reset_and_unavailable_remain_distinct() { assert_eq!( diff --git a/crates/tracedecay/tests/mcp_suite/mcp_handler_test/session_search_test.rs b/crates/tracedecay/tests/mcp_suite/mcp_handler_test/session_search_test.rs index 09534aa0b3..11c742efb2 100644 --- a/crates/tracedecay/tests/mcp_suite/mcp_handler_test/session_search_test.rs +++ b/crates/tracedecay/tests/mcp_suite/mcp_handler_test/session_search_test.rs @@ -80,6 +80,51 @@ fn write_production_codex_rollouts(home: &Path, project: &Path, count: usize) { async fn production_codex_message_search( harness: &ProductionProjectCompositionHarnessV1, project: &Path, +) -> Value { + // A `partial` generation is the store saying "still converging", the same + // not-ready contract as `stale`: re-read it. Every other outcome answers + // now, so an empty `complete_zero` still fails the assertions below. + let payload = tokio::time::timeout(std::time::Duration::from_secs(30), async { + loop { + let payload = production_codex_message_search_once(harness, project).await; + if payload["outcome"] != "partial" + || payload["results"] + .as_array() + .is_some_and(|results| !results.is_empty()) + { + break payload; + } + tokio::task::yield_now().await; + } + }) + .await + .expect("production Codex message search convergence deadline"); + assert!( + payload["results"].as_array().is_some_and(|results| { + results.iter().any(|result| { + result["message"]["text"] + .as_str() + .is_some_and(|text| text.contains("cobalt orchard scheduler migration")) + }) + }), + "production Codex message search was empty after completed ingest: {payload}" + ); + assert!( + payload["results"].as_array().is_some_and(|results| { + results.iter().any(|result| { + result["message"]["text"].as_str() + == Some("The cobalt orchard scheduler migration is ready for review") + }) + }), + "production Codex message search did not hydrate the exact assistant message: {payload}" + ); + payload +} + +#[cfg(feature = "test-transport")] +async fn production_codex_message_search_once( + harness: &ProductionProjectCompositionHarnessV1, + project: &Path, ) -> Value { let response = harness .call_tool( @@ -106,30 +151,10 @@ async fn production_codex_message_search( .expect("production message search JSON"); // Retained tools respond with the full evidence envelope; the search // payload the assertions consume lives under `outcome.value.payload`. - let payload = envelope + envelope .pointer("/outcome/value/payload") .cloned() - .unwrap_or(envelope); - assert!( - payload["results"].as_array().is_some_and(|results| { - results.iter().any(|result| { - result["message"]["text"] - .as_str() - .is_some_and(|text| text.contains("cobalt orchard scheduler migration")) - }) - }), - "production Codex message search was empty after completed ingest: {payload}" - ); - assert!( - payload["results"].as_array().is_some_and(|results| { - results.iter().any(|result| { - result["message"]["text"].as_str() - == Some("The cobalt orchard scheduler migration is ready for review") - }) - }), - "production Codex message search did not hydrate the exact assistant message: {payload}" - ); - payload + .unwrap_or(envelope) } #[cfg(feature = "test-transport")] From 5e03dfa9c549dee51943a6355706a77b316cefe2 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 04:53:04 +0000 Subject: [PATCH 02/17] fix(daemon): compare version identity per semver build rules `tracedecay update` installed v0.1.0-beta.47, restarted the daemon, then refused the binary it had just installed: protocol identity mismatch (name=tracedecay, version=0.1.0-beta.47+84598a0b..., expected version=0.1.0-beta.47) and the daemon logged `daemon_version_skew` on every readiness poll, with the same two strings. Both sides were one binary. One value drove both symptoms. The GitHub-release upgrade path reports the release it installed as the bare tag (`upgrade.rs` `run_versioned_upgrade`), while the package-manager path reports the installed binary's own `--version`. That value becomes the maintenance window's `expected_version` (`service.rs` `adopt_maintenance_outcome`), which readiness compared to the daemon's advertised `build_version()` with a raw string `==`, and which `query_daemon_identity_stream` also sends as the probe's own `client_version`, where `client_version_skew` compared it with a second raw `==`. Build metadata never changes SemVer precedence, so a side reporting only the release is less specific, not different. `versions_name_same_build` is now the one comparison every identity check runs: equal release precedence, and the same commit whenever both sides name one. That keeps the skew 367a44ad00 added this comparison to catch, two checkout builds of one release differing only by commit, while the release tag and the binary it ships resolve to one identity. Tests cover both halves and fail without the fix: readiness classifies 0.1.0-beta.47+ as Ready against a bare 0.1.0-beta.47, a stale release and a second commit of the same release stay mismatches, and unparseable versions still compare literally. Co-Authored-By: Claude Fable 5.1 --- .../src/service/probe.rs | 54 ++++++++++++- .../src/handshake.rs | 77 ++++++++++++++++++- crates/tracedecay-daemon-protocol/src/lib.rs | 1 + crates/tracedecay/src/daemon/core_proxy.rs | 2 +- 4 files changed, 131 insertions(+), 3 deletions(-) diff --git a/crates/tracedecay-daemon-control/src/service/probe.rs b/crates/tracedecay-daemon-control/src/service/probe.rs index 3cec2582bd..c974b6a920 100644 --- a/crates/tracedecay-daemon-control/src/service/probe.rs +++ b/crates/tracedecay-daemon-control/src/service/probe.rs @@ -215,7 +215,9 @@ fn classify_daemon_protocol_identity( match identity { Ok((name, version)) if name.as_deref() == Some("tracedecay") - && version.as_deref() == Some(expected_version) => + && version.as_deref().is_some_and(|version| { + tracedecay_daemon_protocol::versions_name_same_build(version, expected_version) + }) => { DaemonProtocolState::Ready } @@ -638,6 +640,56 @@ fn missing_loopback_authority() -> TraceDecayError { } } +#[cfg(test)] +mod identity_classification_tests { + use super::{DaemonProtocolState, classify_daemon_protocol_identity}; + + const SHA: &str = "84598a0b9c841b914565f46b20bb6c765706e8e5"; + + /// The identity a `tracedecay` daemon reporting `version` answers with. + fn identity(version: &str) -> (Option, Option) { + (Some("tracedecay".to_owned()), Some(version.to_owned())) + } + + /// `tracedecay update` installs a release and then waits for the daemon + /// that binary starts. The release path knows the version it installed as + /// the bare release, while the daemon names the commit it was built from, + /// so readiness used to refuse the very binary it had just installed. + #[test] + fn a_daemon_naming_its_commit_is_ready_against_its_bare_release() { + assert_eq!( + classify_daemon_protocol_identity( + Ok(identity(&format!("0.1.0-beta.47+{SHA}"))), + "0.1.0-beta.47", + ), + DaemonProtocolState::Ready + ); + } + + /// A genuinely stale daemon is still refused, whichever side names a + /// commit. + #[test] + fn a_different_build_is_still_an_identity_mismatch() { + let stale = format!("0.1.0-beta.46+{SHA}"); + assert_eq!( + classify_daemon_protocol_identity(Ok(identity(&stale)), "0.1.0-beta.47"), + DaemonProtocolState::IdentityMismatch { + name: Some("tracedecay".to_owned()), + version: Some(stale), + expected_version: "0.1.0-beta.47".to_owned(), + } + ); + let other_commit = format!("0.1.0-beta.47+{}", "b".repeat(40)); + assert!(matches!( + classify_daemon_protocol_identity( + Ok(identity(&other_commit)), + &format!("0.1.0-beta.47+{SHA}"), + ), + DaemonProtocolState::IdentityMismatch { .. } + )); + } +} + #[cfg(test)] mod timeout_classification_tests { use std::io::{self, Cursor, Read, Write}; diff --git a/crates/tracedecay-daemon-protocol/src/handshake.rs b/crates/tracedecay-daemon-protocol/src/handshake.rs index 231c3ffa85..9bc27c73c6 100644 --- a/crates/tracedecay-daemon-protocol/src/handshake.rs +++ b/crates/tracedecay-daemon-protocol/src/handshake.rs @@ -146,12 +146,41 @@ impl DaemonHandshakeRefusal { /// Old clients send no version (empty string); that is indistinguishable from /// "same version before this field existed", so it never counts as skew. pub fn client_version_skew(client_version: &str, daemon_version: &str) -> Option { - if client_version.is_empty() || client_version == daemon_version { + if client_version.is_empty() || versions_name_same_build(client_version, daemon_version) { return None; } Some(client_version.to_string()) } +/// Whether two reported versions name the same binary, the one comparison +/// every version identity check in the product runs. +/// +/// A version is `"{release}"` or `"{release}+{full sha}[.dirty]"`, and `SemVer` +/// requires build metadata to be ignored for precedence. A side that reports +/// only the release is therefore **less specific**, not different: the release +/// tag `v0.1.0-beta.47` and the binary that names itself +/// `0.1.0-beta.47+` are one identity, and treating them as a mismatch is +/// what failed `tracedecay update`'s own readiness wait against the daemon it +/// had just installed. +/// +/// When both sides do name a commit they must name the same one, which keeps +/// the skew this comparison was added to catch (367a44ad00): two checkout +/// builds of one release differ only by commit, and a daemon left running from +/// the previous build is exactly that case. +#[must_use] +pub fn versions_name_same_build(left: &str, right: &str) -> bool { + let (Some(left_release), Some(right_release)) = (release_version(left), release_version(right)) + else { + // Neither side is a version this comparison understands, so refuse to + // guess and fall back to the literal texts. + return left == right; + }; + left_release.cmp_precedence(&right_release) == std::cmp::Ordering::Equal + && (left_release.build == right_release.build + || left_release.build.is_empty() + || right_release.build.is_empty()) +} + fn release_version(version: &str) -> Option { semver::Version::parse(version.strip_prefix('v').unwrap_or(version)).ok() } @@ -227,6 +256,52 @@ mod handshake_refusal_tests { ); } + /// The observed `tracedecay update` failure: the release path reports the + /// bare release it installed while the daemon that binary starts names its + /// own commit, so readiness compared `0.1.0-beta.47` against + /// `0.1.0-beta.47+` and refused the daemon it had just installed. + #[test] + fn a_bare_release_and_its_own_build_are_one_identity() { + let build = "0.1.0-beta.47+84598a0b9c841b914565f46b20bb6c765706e8e5"; + assert!(versions_name_same_build("0.1.0-beta.47", build)); + assert!(versions_name_same_build(build, "0.1.0-beta.47")); + assert!( + versions_name_same_build("v0.1.0-beta.47", build), + "the GitHub release tag names the same identity as the binary it ships" + ); + assert_eq!(client_version_skew("0.1.0-beta.47", build), None); + assert_eq!(client_version_skew(build, "0.1.0-beta.47"), None); + } + + /// Build metadata still separates two builds of one release, the skew this + /// comparison exists to catch. + #[test] + fn two_commits_of_one_release_stay_distinguishable() { + let older = "0.1.0-beta.47+aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + let newer = "0.1.0-beta.47+bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; + assert!(!versions_name_same_build(older, newer)); + assert_eq!(client_version_skew(older, newer), Some(older.to_owned())); + assert!(!versions_name_same_build( + "0.1.0-beta.46", + "0.1.0-beta.47+aaaa" + )); + assert!( + !versions_name_same_build( + older, + "0.1.0-beta.47+aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.dirty" + ), + "a dirty worktree is not the commit it was built from" + ); + } + + /// Nothing that fails to parse may be declared a match by accident. + #[test] + fn unparseable_versions_compare_literally() { + assert!(versions_name_same_build("not-a-version", "not-a-version")); + assert!(!versions_name_same_build("not-a-version", "0.1.0-beta.47")); + assert!(!versions_name_same_build("", "0.1.0-beta.47")); + } + #[test] fn foreign_lines_never_parse_as_refusal_frames() { assert_eq!(DaemonHandshakeRefusal::from_line("{}"), None); diff --git a/crates/tracedecay-daemon-protocol/src/lib.rs b/crates/tracedecay-daemon-protocol/src/lib.rs index bc9112b66a..10b4fe309b 100644 --- a/crates/tracedecay-daemon-protocol/src/lib.rs +++ b/crates/tracedecay-daemon-protocol/src/lib.rs @@ -99,6 +99,7 @@ pub use contract::{ pub use handshake::{ DAEMON_HANDSHAKE_REFUSAL_PROTOCOL, DaemonHandshake, DaemonHandshakeRefusal, DaemonHandshakeRefusalReason, MovedStoreAdoption, client_version_skew, version_skew_action, + versions_name_same_build, }; pub use lsp_wire::{ ConnectionLocalRequestSequence, FramePoll, FrameSend, LspFrame, LspSessionAccess, diff --git a/crates/tracedecay/src/daemon/core_proxy.rs b/crates/tracedecay/src/daemon/core_proxy.rs index d9b3a4a712..025ef26848 100644 --- a/crates/tracedecay/src/daemon/core_proxy.rs +++ b/crates/tracedecay/src/daemon/core_proxy.rs @@ -909,7 +909,7 @@ fn daemon_version_skew_warning_for_request( client_version: &str, ) -> Option { let daemon_version = proxy_initialize_metadata_for_request(request, responses).daemon_version?; - if daemon_version == client_version { + if tracedecay_daemon_protocol::versions_name_same_build(&daemon_version, client_version) { return None; } let action = version_skew_action(&daemon_version, client_version); From cc103942321088e3f3fa7c3be8afff492bf7f436 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 04:07:59 +0000 Subject: [PATCH 03/17] fix(sessions): retire refused refresh progress instead of retrying A daemon spent eleven hours emitting 70,985 identical warnings: session temporal refresh pass will retry class=Storage retry_attempt=25107 error="Storage { operation: \"persist session; refresh progress\", source: Sqlite { code: Some; ( 19 ); , extended_code:; Some; ( 1811 ); , message: \"invalid session refresh progress\" } }" Three workers each resubmitted the same progress row about once a second for the whole process lifetime, pinning two tokio workers. Two independent defects combined: 1. The worker used `SessionStoreError::is_storage` as its retryability predicate. That only says the failure came from the storage adapter. A `SQLITE_CONSTRAINT` abort is a schema-contract trigger refusing this exact row, and an exact-SQL materialization ceiling (6,627 more warnings on the same loop, reported as an untyped `Runtime` message) refuses this exact statement. Replaying either unchanged can only spin at the backoff cap, which for `Storage` is 800ms forever. 2. Even classified terminal, the projection arm only counted the error and left the operation `running`, so the next pass rediscovered it and rebuilt the same row. Only the projector's terminal errors durably failed a refresh. `Error::is_deterministic_refusal` now names the two engine failures that cannot be replayed, `QueryLimitExceeded` maps to the typed `InvalidOperation` its sibling limit refusal already used, and the worker retires a refused refresh through the existing terminal-attempt and discovery-suppression machinery. `validate_successor` also admitted an equal committed frontier where the durable guard requires a strict advance, so a stalled successor reached the trigger as a constraint abort instead of typed state. It now matches the guard. Co-Authored-By: Claude Fable 5.1 --- .../src/db/engine/error.rs | 25 +++ .../src/db/engine/tests.rs | 40 ++++ .../worker.rs | 200 ++++++++++++------ .../worker_tests.rs | 76 ++++++- .../tracedecay-store/src/session/refresh.rs | 7 +- .../store_suite/session_contract/refresh.rs | 19 ++ 6 files changed, 301 insertions(+), 66 deletions(-) diff --git a/crates/tracedecay-runtime-core/src/db/engine/error.rs b/crates/tracedecay-runtime-core/src/db/engine/error.rs index 2955a7610e..d2570b249a 100644 --- a/crates/tracedecay-runtime-core/src/db/engine/error.rs +++ b/crates/tracedecay-runtime-core/src/db/engine/error.rs @@ -95,6 +95,12 @@ impl From for Error { ExactSqlError::RequestLimitExceeded => { Self::InvalidOperation("SQL request exceeds migration limits".to_owned()) } + // A materialization ceiling is a property of the submitted + // statement, not of the engine: the untyped `Runtime` fallback + // made callers read it as a transient storage fault and replay it. + ExactSqlError::QueryLimitExceeded => Self::InvalidOperation( + "exact SQL query materialization exceeded its limit".to_owned(), + ), ExactSqlError::AuthorityDenied(message) => Self::InvalidOperation(message), ExactSqlError::Sqlite { operation, @@ -142,4 +148,23 @@ impl Error { _ => None, } } + + /// True when replaying this exact statement can never succeed. + /// + /// A `SQLITE_CONSTRAINT` abort is a schema-contract trigger or constraint + /// refusing this exact row, and `InvalidOperation` is an admission or + /// materialization ceiling refusing this exact statement. Neither is a + /// transient engine condition, so a caller that retries one spins until + /// something else changes the durable state. + #[hotpath::skip] + pub const fn is_deterministic_refusal(&self) -> bool { + match self { + Self::InvalidOperation(_) => true, + Self::StatementBatch { source, .. } => source.is_deterministic_refusal(), + _ => matches!(self.sqlite_code(), Some(SQLITE_CONSTRAINT)), + } + } } + +/// `SQLITE_CONSTRAINT`: a constraint or `RAISE(ABORT)` trigger refused the row. +const SQLITE_CONSTRAINT: i32 = 19; diff --git a/crates/tracedecay-runtime-core/src/db/engine/tests.rs b/crates/tracedecay-runtime-core/src/db/engine/tests.rs index a3c02d8f47..5e51bd0a70 100644 --- a/crates/tracedecay-runtime-core/src/db/engine/tests.rs +++ b/crates/tracedecay-runtime-core/src/db/engine/tests.rs @@ -386,3 +386,43 @@ async fn transaction_statement_batch_reports_the_exact_failed_statement() { } mod async_writer; + +#[test] +fn deterministic_refusals_are_distinguished_from_transient_engine_faults() { + use tracedecay_rusqlite_runtime::exact_sql::ExactSqlError; + + // A schema-contract trigger abort refuses this exact row for good. + let constraint = Error::Sqlite { + operation: "execute", + code: Some(19), + extended_code: Some(1811), + message: "invalid session refresh progress".to_owned(), + }; + assert!(constraint.is_deterministic_refusal()); + assert!( + Error::StatementBatch { + index: 0, + source: Box::new(constraint), + } + .is_deterministic_refusal() + ); + + // A materialization ceiling refuses this exact statement for good, and + // must not arrive as an untyped `Runtime` message. + let limit = Error::from(ExactSqlError::QueryLimitExceeded); + assert!(matches!(limit, Error::InvalidOperation(_))); + assert!(limit.is_deterministic_refusal()); + + // Contention and I/O faults stay retryable. + assert!(!Error::Busy.is_deterministic_refusal()); + assert!(!Error::Runtime("writer restarted".to_owned()).is_deterministic_refusal()); + assert!( + !Error::Sqlite { + operation: "execute", + code: Some(5), + extended_code: Some(5), + message: "database is locked".to_owned(), + } + .is_deterministic_refusal() + ); +} diff --git a/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/worker.rs b/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/worker.rs index ed7fcde8e0..076da4e3cc 100644 --- a/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/worker.rs +++ b/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/worker.rs @@ -25,6 +25,7 @@ use super::wake::{ SessionTemporalRefreshWakeState, TerminalAttemptGuard, }; use tracedecay_global_db::{RegisteredGlobalDb, RegisteredGlobalDbLeaseV1}; +use tracedecay_runtime_core::db::engine::Error as EngineError; use tracedecay_session_temporal_store::{ SessionRefreshRecoveryV1, SessionRefreshRestartStateV1, SessionTemporalStore, }; @@ -634,12 +635,22 @@ async fn session_projection_refresh( run_session_temporal_refresh_pass(database, state, projector, policy).await } -fn classify_store_error(error: &SessionStoreError) -> SessionTemporalRefreshRetryClass { - if error.is_storage() { - SessionTemporalRefreshRetryClass::Storage - } else { - SessionTemporalRefreshRetryClass::Projector - } +/// True when replaying this store failure unchanged could still succeed. +/// +/// `is_storage` only says the failure came from the storage adapter; it does +/// not say the failure is transient. A schema-contract trigger refusing the +/// submitted row, or an exact-SQL ceiling refusing the submitted statement, is +/// deterministic: the worker resubmits the identical request every pass, so +/// treating it as retryable is an unbounded spin at the backoff cap rather +/// than a recovery. Those are terminal, and the caller durably fails the +/// refresh instead of retrying it. +fn is_retryable_storage(error: &SessionStoreError) -> bool { + let SessionStoreError::Storage { source, .. } = error else { + return false; + }; + source + .downcast_ref::() + .is_none_or(|engine| !engine.is_deterministic_refusal()) } pub async fn process_refresh_begin_requests( @@ -667,7 +678,7 @@ pub async fn process_refresh_begin_requests( tracedecay_store::SessionRefreshDispositionV1::Joined => report.joined += 1, } } - Err(error) if error.is_storage() => { + Err(error) if is_retryable_storage(&error) => { report.last_error = Some(format!("{error:?}")); report.retryable_errors += 1; report.observe_retry(SessionTemporalRefreshRetryClass::Storage); @@ -709,7 +720,7 @@ pub async fn begin_admitted_session_refreshes( { Ok(page) => page, Err(error) => { - if classify_store_error(&error) == SessionTemporalRefreshRetryClass::Storage { + if is_retryable_storage(&error) { report.last_error = Some(format!("{error:?}")); report.retryable_errors += 1; report.observe_retry(SessionTemporalRefreshRetryClass::Storage); @@ -767,7 +778,7 @@ async fn complete_ready_refresh( Ok(_) => { report.completed += 1; } - Err(error) if error.is_storage() => { + Err(error) if is_retryable_storage(&error) => { report.last_error = Some(format!("{error:?}")); report.retryable_errors += 1; report.observe_retry(SessionTemporalRefreshRetryClass::Storage); @@ -796,6 +807,48 @@ fn record_projector_error( } } +/// Typed failure recorded when the durable contract refuses the projected +/// progress row. It is not a projector fault: the row was well formed for the +/// state the projector read, and the durable state disagrees. +const REFRESH_PROGRESS_REFUSED: &str = "refresh_progress_refused"; + +/// Builds the durable failure request that retires one running refresh. +fn durable_failure_request( + recovery: &SessionRefreshRecoveryV1, + failure_code: String, +) -> Option { + let (frontier, coverage) = match recovery.progress() { + Some(progress) => (progress.frontier(), *progress.coverage()), + None => ( + SessionRefreshFrontierV1::new( + recovery.target_frontier().observed_through(), + recovery.source_frontier(), + ) + .ok()?, + zero_refresh_coverage(), + ), + }; + let request = SessionRefreshFailureRequestV1::new( + recovery.operation_id().clone(), + recovery.session_id().clone(), + frontier, + coverage, + failure_code, + ) + .ok()?; + Some( + match recovery + .progress() + .and_then(SessionRefreshProgressV1::source_coverage) + .cloned() + .or_else(|| recovery.source_coverage(frontier.committed_through()).ok()) + { + Some(source_coverage) => request.with_source_coverage(source_coverage), + None => request, + }, + ) +} + pub async fn apply_refresh_effect( store: &SessionTemporalStore<'_, tracedecay_global_db::RegisteredGlobalDb>, state: &SessionTemporalRefreshWakeState, @@ -814,43 +867,66 @@ pub async fn apply_refresh_effect( .await { Ok(_) => report.projected_batches += 1, - Err(error) if error.is_storage() => { + Err(error) if is_retryable_storage(&error) => { report.last_error = Some(format!("{error:?}")); report.retryable_errors += 1; report.observe_retry(SessionTemporalRefreshRetryClass::Storage); } Err(error) => { + // A refused progress row is not work the next pass can + // finish: rediscovery hands the projector the same durable + // state and the same row comes back refused. Retire the + // operation so it leaves `running` and a fresh refresh can + // be admitted, instead of resubmitting it forever. report.last_error = Some(format!("{error:?}")); - report.terminal_errors += 1; + match durable_failure_request( + recovery, + durable_projector_failure_code(REFRESH_PROGRESS_REFUSED), + ) { + Some(request) => { + apply_fail_effect(store, state, recovery, request, report).await; + } + None => report.terminal_errors += 1, + } } } } SessionTemporalRefreshEffect::Fail(request) => { - if !state.claim_terminal_attempt(recovery) { - return; - } - let mut attempt = TerminalAttemptGuard::new(state, recovery); - match store.fail_session_refresh(request).await { - Ok(_) => { - report.failed += 1; - state.record_terminal_discovery_failure(recovery); - } - Err(error) if error.is_storage() => { - report.last_error = Some(format!("{error:?}")); - report.retryable_errors += 1; - report.observe_retry(SessionTemporalRefreshRetryClass::Storage); - } - Err(error) => { - attempt.retain(); - report.last_error = Some(format!("{error:?}")); - report.terminal_errors += 1; - } - } + apply_fail_effect(store, state, recovery, request, report).await; } SessionTemporalRefreshEffect::Deferred => report.deferred += 1, } } +async fn apply_fail_effect( + store: &SessionTemporalStore<'_, tracedecay_global_db::RegisteredGlobalDb>, + state: &SessionTemporalRefreshWakeState, + recovery: &SessionRefreshRecoveryV1, + request: SessionRefreshFailureRequestV1, + report: &mut SessionTemporalRefreshPassReport, +) { + if !state.claim_terminal_attempt(recovery) { + return; + } + let mut attempt = TerminalAttemptGuard::new(state, recovery); + match store.fail_session_refresh(request).await { + Ok(_) => { + report.failed += 1; + state.record_terminal_discovery_failure(recovery); + } + Err(error) if is_retryable_storage(&error) => { + report.last_error = Some(format!("{error:?}")); + report.retryable_errors += 1; + report.observe_retry(SessionTemporalRefreshRetryClass::Storage); + } + Err(error) => { + attempt.retain(); + report.last_error = Some(format!("{error:?}")); + report.terminal_errors += 1; + } + } +} + async fn project_running_refresh( database: &RegisteredGlobalDbLeaseV1, store: &SessionTemporalStore<'_, tracedecay_global_db::RegisteredGlobalDb>, @@ -894,35 +970,7 @@ async fn project_running_refresh( Err(error) => { let failure_code = durable_projector_failure_code(&error.code); report.last_error = Some(failure_code.clone()); - let (frontier, coverage) = if let Some(progress) = recovery.progress() { - (progress.frontier(), *progress.coverage()) - } else { - let Ok(frontier) = SessionRefreshFrontierV1::new( - recovery.target_frontier().observed_through(), - recovery.source_frontier(), - ) else { - report.terminal_errors += 1; - return; - }; - (frontier, zero_refresh_coverage()) - }; - let request = if let Ok(request) = SessionRefreshFailureRequestV1::new( - recovery.operation_id().clone(), - recovery.session_id().clone(), - frontier, - coverage, - failure_code, - ) { - match recovery - .progress() - .and_then(SessionRefreshProgressV1::source_coverage) - .cloned() - .or_else(|| recovery.source_coverage(frontier.committed_through()).ok()) - { - Some(source_coverage) => request.with_source_coverage(source_coverage), - None => request, - } - } else { + let Some(request) = durable_failure_request(recovery, failure_code) else { report.terminal_errors += 1; return; }; @@ -960,7 +1008,7 @@ async fn running_refreshes( Ok(recoveries) => Some(recoveries), Err(error) => { report.last_error = Some(format!("{error:?}")); - if classify_store_error(&error) == SessionTemporalRefreshRetryClass::Storage { + if is_retryable_storage(&error) { report.retryable_errors += 1; report.observe_retry(SessionTemporalRefreshRetryClass::Storage); } else { @@ -1121,6 +1169,36 @@ mod tests { use tracedecay_sessions::runtime::{SessionMessageRecord, SessionRecord}; use tracedecay_store::ParseOffset; + #[test] + fn deterministic_storage_refusals_are_not_retryable() { + // The schema-contract trigger that refused eleven hours of identical + // progress rows in #1794: transport-level `Storage`, but replaying it + // can never succeed. + let refused = SessionStoreError::storage( + "persist session refresh progress", + EngineError::Sqlite { + operation: "execute", + code: Some(19), + extended_code: Some(1811), + message: "invalid session refresh progress".to_owned(), + }, + ); + assert!(!is_retryable_storage(&refused)); + + // Contention is the transient case the retry loop exists for. + assert!(is_retryable_storage(&SessionStoreError::storage( + "persist session refresh progress", + EngineError::Busy, + ))); + + // Typed contract failures were already terminal and stay terminal. + assert!(!is_retryable_storage( + &SessionStoreError::InvalidStateTransition { + context: "refresh progress successor", + } + )); + } + #[test] fn dropping_worker_instrumentation_clears_pending_state_once() { let state = SessionTemporalRefreshWakeState::default(); diff --git a/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/worker_tests.rs b/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/worker_tests.rs index cd3c8ed6c0..f119795e41 100644 --- a/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/worker_tests.rs +++ b/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/worker_tests.rs @@ -1,16 +1,21 @@ use std::sync::Arc; use std::time::Duration; -use tracedecay_domain::SessionId; +use tracedecay_domain::{SessionId, UtcMicros}; use tracedecay_global_db::tests::harness::RegisteredGlobalDbHarness; use tracedecay_session_temporal_store::SessionTemporalStore; use tracedecay_store::{ - SessionRefreshBeginOrJoinRequestV1, SessionRefreshFrontierV1, SessionRefreshStore, + SessionRefreshBeginOrJoinRequestV1, SessionRefreshFrontierV1, SessionRefreshProgressV1, + SessionRefreshStore, SessionTemporalProjectionBatchV1, }; -use super::projector::{CanonicalSessionTemporalProjector, SessionTemporalRefreshPolicy}; +use super::projector::{ + CanonicalSessionTemporalProjector, SessionTemporalRefreshEffect, SessionTemporalRefreshPolicy, + zero_refresh_coverage, +}; +use super::registry::SessionTemporalRefreshPassReport; use super::wake::{SessionTemporalRefreshRetryClass, SessionTemporalRefreshWakeState}; -use super::worker::run_session_temporal_refresh_pass; +use super::worker::{apply_refresh_effect, run_session_temporal_refresh_pass}; async fn begin_empty_refreshes( store: &SessionTemporalStore<'_, tracedecay_global_db::RegisteredGlobalDb>, @@ -80,3 +85,66 @@ async fn retryable_recovery_stops_the_pass_and_preserves_unattempted_work() { assert_eq!(report.backlog, Some(2)); assert_eq!(state.pending_recovery_operations().len(), 1); } + +#[tokio::test] +async fn refused_projection_progress_retires_the_refresh_instead_of_retrying() { + let harness = RegisteredGlobalDbHarness::open("refresh-refused-progress-retires").await; + let store = SessionTemporalStore::new(harness.registered.as_ref()); + begin_empty_refreshes(&store, ["refused-progress"]).await; + let recovery = store + .running_session_refreshes() + .await + .expect("recoveries") + .pop() + .expect("one running recovery"); + let state = SessionTemporalRefreshWakeState::default(); + let mut report = SessionTemporalRefreshPassReport::default(); + + // Progress that claims a second committed batch while submitting the + // first one. The durable contract refuses it, and every later pass would + // hand the projector the same state and rebuild the same refused row. + let progress = SessionRefreshProgressV1::new( + recovery.operation_id().clone(), + recovery.session_id().clone(), + SessionRefreshFrontierV1::new(0, 0).expect("empty frontier"), + zero_refresh_coverage(), + 2, + 0, + UtcMicros(1), + ); + let batch = SessionTemporalProjectionBatchV1::new( + recovery.session_id().clone(), + recovery.candidate_generation(), + recovery.frozen_watermarks().clone(), + vec![], + vec![], + vec![], + ) + .expect("batch") + .with_checkpoint(0, 0, 0) + .expect("checkpoint"); + + apply_refresh_effect( + &store, + &state, + &recovery, + SessionTemporalRefreshEffect::Projection { progress, batch }, + &mut report, + ) + .await; + + assert_eq!( + report.failed, 1, + "a refused progress row must retire the refresh, not stay running" + ); + assert_eq!(report.retryable_errors, 0); + assert_eq!(report.terminal_errors, 0); + assert!( + store + .running_session_refreshes() + .await + .expect("recoveries") + .is_empty(), + "the retired refresh must not be rediscovered" + ); +} diff --git a/crates/tracedecay-store/src/session/refresh.rs b/crates/tracedecay-store/src/session/refresh.rs index 33b52a7e8d..c6c8696dc9 100644 --- a/crates/tracedecay-store/src/session/refresh.rs +++ b/crates/tracedecay-store/src/session/refresh.rs @@ -282,8 +282,13 @@ impl SessionRefreshProgressV1 { } let current = self.coverage; let candidate = next.coverage; + // The durable guard admits a successor only when it strictly advances + // the committed frontier. Accepting an equal frontier here let a + // producer submit a row the trigger then refused as a SQLite + // constraint abort, which the worker read as transient storage and + // resubmitted forever. Refuse it as typed state instead. if self.frontier.observed_through != next.frontier.observed_through - || next.frontier.committed_through < self.frontier.committed_through + || next.frontier.committed_through <= self.frontier.committed_through || next.committed_batches < self.committed_batches || next.committed_records < self.committed_records || candidate.visible < current.visible diff --git a/crates/tracedecay-store/tests/store_suite/session_contract/refresh.rs b/crates/tracedecay-store/tests/store_suite/session_contract/refresh.rs index f4ffc7aeaa..0de934cb97 100644 --- a/crates/tracedecay-store/tests/store_suite/session_contract/refresh.rs +++ b/crates/tracedecay-store/tests/store_suite/session_contract/refresh.rs @@ -225,6 +225,25 @@ fn refresh_frontiers_and_progress_are_monotonic_and_terminal() { }) )); + // The durable guard admits a successor only when it strictly advances the + // committed frontier, so a stalled successor must be refused here rather + // than deferred to a SQLite constraint abort the caller reads as storage. + let stalled = SessionRefreshProgressV1::new( + operation_id(), + session_id.clone(), + SessionRefreshFrontierV1::new(10, 8).unwrap(), + coverage(), + 2, + 8, + UtcMicros(101), + ); + assert!(matches!( + initial.validate_successor(&stalled), + Err(SessionStoreError::InvalidStateTransition { + context: "refresh progress successor" + }) + )); + let terminal = SessionRefreshReceiptV1::completed( SessionRefreshCompletionRequestV1::new( operation_id(), From 8b755233b8904cc5813e57c5ff6755d05f33c14d Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 04:23:52 +0000 Subject: [PATCH 04/17] perf(code-index): scan clone postings once per resumed page A daemon mounted /fast/projects/tracedecay at 15:25:35, decided the sealed generation predated the build and had to be rebuilt, and then recorded no further scheduler progress for eleven hours. A full thread dump taken while it was stuck shows two blocking-pool threads in registry::mount_worktree_inner -> LatestCodeTextGenerationV1::advance_text_serving_inner -> CodeLexicalCloneSuccessorV1::verify_resumed_page -> verify_clone_fingerprint_page_rows -> sqlite3_step / sqlite3BtreeNext / readDbPage so the mount was not blocked on a seat, a permit or a store lock: it was running SQL, which is why the process also sat at 100% on two workers. `clone_exact_postings` and `clone_fingerprint_postings` are WITHOUT ROWID tables whose primary keys start at `class` and `language`, so `WHERE symbol_occurrence_id = ?` has no index and scans the whole table. Resume verification issued two of those per clone body, making the pass quadratic in a repository's postings. Both tables are now read once per page and bucketed by occurrence. Each primary key is unique within one occurrence, so a sorted bucket reproduces the `ORDER BY` the per-body queries used and the comparisons stay byte-identical; `clone_body_payloads` and `clone_occurrences` are already keyed by their lookup column and are untouched. Resume verification is covered by `v16_clone_payloads_are_content_addressed_and_postings_page`, which drops and reopens a successor mid-corpus and then compares finished section digests. Co-Authored-By: Claude Fable 5.1 --- .../projection/artifact/clone_successor.rs | 141 +++++++++++++----- 1 file changed, 106 insertions(+), 35 deletions(-) diff --git a/crates/tracedecay-query/src/retrieval/lexical/projection/artifact/clone_successor.rs b/crates/tracedecay-query/src/retrieval/lexical/projection/artifact/clone_successor.rs index 922ea3552b..ec0ea59a26 100644 --- a/crates/tracedecay-query/src/retrieval/lexical/projection/artifact/clone_successor.rs +++ b/crates/tracedecay-query/src/retrieval/lexical/projection/artifact/clone_successor.rs @@ -1,3 +1,4 @@ +use std::collections::{HashMap, HashSet}; use std::io; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -559,11 +560,112 @@ fn verify_copied_source_page( Ok(()) } +type CloneExactRowV1 = (i64, i64, String, String); + +/// Postings for one page's occurrences, read with one scan per table. +/// +/// `clone_exact_postings` and `clone_fingerprint_postings` are +/// `WITHOUT ROWID` tables keyed from `class`/`language`, so a +/// `WHERE symbol_occurrence_id = ?` lookup has no index to use and scans the +/// whole table. Issuing one per body made resume verification quadratic in +/// the postings a repository has: a daemon spent eleven hours inside +/// `verify_clone_fingerprint_page_rows` on one mount without recording a +/// single scheduler pass. Scanning once per page and bucketing by occurrence +/// keeps the comparisons byte-identical while paying the scan once. +struct ClonePagePostingsV1 { + exact: HashMap>, + fingerprints: HashMap>, +} + +impl ClonePagePostingsV1 { + fn read( + connection: &Connection, + occurrences: &HashSet<&str>, + control: &dyn CodeIndexExecutionControlV1, + ) -> Result { + let mut exact: HashMap> = HashMap::new(); + let mut statement = connection + .prepare( + "SELECT symbol_occurrence_id, class, normalization_revision, digest, payload_digest FROM clone_exact_postings", + ) + .map_err(sqlite_error)?; + let mut rows = statement.query([]).map_err(sqlite_error)?; + while let Some(row) = rows.next().map_err(sqlite_error)? { + let occurrence: String = row.get(0).map_err(sqlite_error)?; + if !occurrences.contains(occurrence.as_str()) { + continue; + } + exact.entry(occurrence).or_default().push(( + row.get(1).map_err(sqlite_error)?, + row.get(2).map_err(sqlite_error)?, + row.get(3).map_err(sqlite_error)?, + row.get(4).map_err(sqlite_error)?, + )); + } + drop(rows); + drop(statement); + checkpoint(control)?; + + let mut fingerprints: HashMap> = HashMap::new(); + let mut statement = connection + .prepare( + "SELECT symbol_occurrence_id, language, class, normalization_revision, fingerprint, token_position, payload_digest, body_digest FROM clone_fingerprint_postings", + ) + .map_err(sqlite_error)?; + let mut rows = statement.query([]).map_err(sqlite_error)?; + while let Some(row) = rows.next().map_err(sqlite_error)? { + let occurrence: String = row.get(0).map_err(sqlite_error)?; + if !occurrences.contains(occurrence.as_str()) { + continue; + } + fingerprints.entry(occurrence).or_default().push(( + row.get(1).map_err(sqlite_error)?, + row.get(2).map_err(sqlite_error)?, + row.get(3).map_err(sqlite_error)?, + row.get(4).map_err(sqlite_error)?, + row.get(5).map_err(sqlite_error)?, + row.get(6).map_err(sqlite_error)?, + row.get(7).map_err(sqlite_error)?, + )); + } + drop(rows); + drop(statement); + checkpoint(control)?; + + // Each table's primary key is unique within one occurrence, so sorting + // a bucket reproduces the `ORDER BY` the per-body queries used. + for rows in exact.values_mut() { + rows.sort(); + } + for rows in fingerprints.values_mut() { + rows.sort(); + } + Ok(Self { + exact, + fingerprints, + }) + } + + fn exact_for(&self, occurrence: &str) -> &[CloneExactRowV1] { + self.exact.get(occurrence).map_or(&[], Vec::as_slice) + } + + fn fingerprints_for(&self, occurrence: &str) -> &[CloneFingerprintRowV1] { + self.fingerprints.get(occurrence).map_or(&[], Vec::as_slice) + } +} + fn verify_clone_page_rows( connection: &Connection, page: &VerifiedSealedLexicalPageV1, control: &dyn CodeIndexExecutionControlV1, ) -> Result<(), CodeLexicalArtifactErrorV1> { + let occurrences = page + .clone_bodies() + .iter() + .map(|body| body.occurrence.symbol_occurrence_id.as_str()) + .collect::>(); + let postings = ClonePagePostingsV1::read(connection, &occurrences, control)?; for body in page.clone_bodies() { checkpoint(control)?; let expected_payload = serde_json::to_vec(&body.payload) @@ -631,24 +733,12 @@ fn verify_clone_page_rows( ) }) .collect::>(); - let mut statement = connection - .prepare( - "SELECT class, normalization_revision, digest, payload_digest FROM clone_exact_postings WHERE symbol_occurrence_id = ?1 ORDER BY class, normalization_revision, digest", - ) - .map_err(sqlite_error)?; - let stored_postings = statement - .query_map([body.occurrence.symbol_occurrence_id.as_str()], |row| { - Ok((row.get(0)?, row.get(1)?, row.get(2)?, row.get(3)?)) - }) - .map_err(sqlite_error)? - .collect::, _>>() - .map_err(sqlite_error)?; - if stored_postings != expected_postings { + if postings.exact_for(body.occurrence.symbol_occurrence_id.as_str()) != expected_postings { return Err(CodeLexicalArtifactErrorV1::Corrupt( "resumed clone postings differ from their sealed source page".to_owned(), )); } - verify_clone_fingerprint_page_rows(connection, body)?; + verify_clone_fingerprint_page_rows(&postings, body)?; } Ok(()) } @@ -656,7 +746,7 @@ fn verify_clone_page_rows( type CloneFingerprintRowV1 = (String, i64, i64, i64, i64, String, String); fn verify_clone_fingerprint_page_rows( - connection: &Connection, + postings: &ClonePagePostingsV1, body: &CodeIndexCloneBodyV1, ) -> Result<(), CodeLexicalArtifactErrorV1> { let mut expected = Vec::new(); @@ -679,26 +769,7 @@ fn verify_clone_fingerprint_page_rows( } } expected.sort(); - let mut statement = connection - .prepare( - "SELECT language, class, normalization_revision, fingerprint, token_position, payload_digest, body_digest FROM clone_fingerprint_postings WHERE symbol_occurrence_id = ?1 ORDER BY language, class, normalization_revision, fingerprint, token_position", - ) - .map_err(sqlite_error)?; - let stored = statement - .query_map([body.occurrence.symbol_occurrence_id.as_str()], |row| { - Ok(( - row.get(0)?, - row.get(1)?, - row.get(2)?, - row.get(3)?, - row.get(4)?, - row.get(5)?, - row.get(6)?, - )) - }) - .map_err(sqlite_error)? - .collect::, _>>() - .map_err(sqlite_error)?; + let stored = postings.fingerprints_for(body.occurrence.symbol_occurrence_id.as_str()); if stored != expected { return Err(CodeLexicalArtifactErrorV1::Corrupt( "resumed clone fingerprints differ from their sealed source page".to_owned(), From e2384db4d1a3c4088ad3d18cbdd990a0bbfd5d6c Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 05:45:11 +0000 Subject: [PATCH 05/17] fix(sessions): retire only refused progress, not a cancelled pass cc10394232 made every non-retryable failure of the projection batch persist retire the running refresh. A cancelled worker control also fails that persist, and it is not a refusal: the row was never submitted, and the next pass may hold a live control. Retiring it through `claim_terminal_attempt` under a cancelled state claimed nothing and counted nothing, so `cancelled_worker_control_prevents_projection_batch_persistence` observed zero terminal errors on CI. Only a deterministic engine refusal (constraint abort, exact-SQL ceiling) retires the refresh; every other terminal error counts as before and leaves the operation running. Co-Authored-By: Claude Fable 5.1 --- .../worker.rs | 34 +++++++++++++++---- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/worker.rs b/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/worker.rs index 076da4e3cc..365f4d76f1 100644 --- a/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/worker.rs +++ b/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/worker.rs @@ -645,12 +645,25 @@ async fn session_projection_refresh( /// than a recovery. Those are terminal, and the caller durably fails the /// refresh instead of retrying it. fn is_retryable_storage(error: &SessionStoreError) -> bool { - let SessionStoreError::Storage { source, .. } = error else { - return false; - }; - source - .downcast_ref::() - .is_none_or(|engine| !engine.is_deterministic_refusal()) + matches!(error, SessionStoreError::Storage { .. }) && !is_deterministic_refusal(error) +} + +/// True when the durable contract refused the exact submitted row or +/// statement: a typed store refusal, or an engine failure that replays +/// identically. Only such a refusal retires a running refresh. An +/// interrupted pass (cancelled control, deadline, budget) and transient +/// storage leave the operation for the next pass, which may hold a +/// different control. +fn is_deterministic_refusal(error: &SessionStoreError) -> bool { + match error { + SessionStoreError::Cancelled + | SessionStoreError::DeadlineExceeded + | SessionStoreError::BudgetExceeded { .. } => false, + SessionStoreError::Storage { source, .. } => source + .downcast_ref::() + .is_some_and(EngineError::is_deterministic_refusal), + _ => true, + } } pub async fn process_refresh_begin_requests( @@ -872,7 +885,7 @@ pub async fn apply_refresh_effect( report.retryable_errors += 1; report.observe_retry(SessionTemporalRefreshRetryClass::Storage); } - Err(error) => { + Err(error) if is_deterministic_refusal(&error) => { // A refused progress row is not work the next pass can // finish: rediscovery hands the projector the same durable // state and the same row comes back refused. Retire the @@ -889,6 +902,13 @@ pub async fn apply_refresh_effect( None => report.terminal_errors += 1, } } + Err(error) => { + // Cancelled control, budget ceiling: this pass could not + // persist, but the row itself was not refused, so the + // operation stays `running` for the next pass. + report.last_error = Some(format!("{error:?}")); + report.terminal_errors += 1; + } } } SessionTemporalRefreshEffect::Fail(request) => { From f2ab8080cf69883588860ed7b3e065576bdd193e Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 05:51:31 +0000 Subject: [PATCH 06/17] perf(code-index): index clone postings by occurrence for resume replay 8b755233b8 cut resume verification from one postings scan per body to one per page, but a restart still replays every committed page, so a daemon resumed against a large repository sat inside `verify_clone_page_rows` for over thirty minutes with no scheduler progress: N pages times a full scan of both postings tables. The clone successor now installs `symbol_occurrence_id` indexes on both postings tables when it opens (idempotent, so a copied prior built without them gains them), and page verification reads postings per occurrence through them. Nothing digests or enumerates the index schema; the receipt records the file size after the indexes exist. Co-Authored-By: Claude Fable 5.1 --- .../projection/artifact/clone_successor.rs | 91 +++++++++++-------- 1 file changed, 52 insertions(+), 39 deletions(-) diff --git a/crates/tracedecay-query/src/retrieval/lexical/projection/artifact/clone_successor.rs b/crates/tracedecay-query/src/retrieval/lexical/projection/artifact/clone_successor.rs index ec0ea59a26..bb09cccdfa 100644 --- a/crates/tracedecay-query/src/retrieval/lexical/projection/artifact/clone_successor.rs +++ b/crates/tracedecay-query/src/retrieval/lexical/projection/artifact/clone_successor.rs @@ -61,6 +61,7 @@ impl CodeLexicalCloneSuccessorV1 { memory_budget_bytes: usize, ) -> Result { let connection = open_builder_connection(staging_path, memory_budget_bytes)?; + ensure_clone_occurrence_indexes(&connection)?; let mutation_gate = register_builder_mutation_gate(&connection)?; let (prior_digest, format_revision): (String, i64) = connection .query_row( @@ -431,6 +432,24 @@ fn reset_clone_tables(connection: &Connection) -> Result<(), CodeLexicalArtifact .map_err(sqlite_error) } +/// Lookup indexes for resume verification, which reads postings by +/// occurrence. Both postings tables are keyed from `class`/`language`, so +/// without these every per-occurrence read is a full table scan; replaying N +/// committed pages after a restart then costs N scans of every posting the +/// repository has, and a daemon sat inside that replay for hours. A prior +/// artifact copied from a build that predates the indexes gains them here, +/// and nothing digests or enumerates the index schema. +fn ensure_clone_occurrence_indexes( + connection: &Connection, +) -> Result<(), CodeLexicalArtifactErrorV1> { + connection + .execute_batch( + "CREATE INDEX IF NOT EXISTS clone_exact_postings_by_occurrence ON clone_exact_postings(symbol_occurrence_id); + CREATE INDEX IF NOT EXISTS clone_fingerprint_postings_by_occurrence ON clone_fingerprint_postings(symbol_occurrence_id);", + ) + .map_err(sqlite_error) +} + fn append_clone_rows( transaction: &rusqlite::Transaction<'_>, page: &VerifiedSealedLexicalPageV1, @@ -562,16 +581,12 @@ fn verify_copied_source_page( type CloneExactRowV1 = (i64, i64, String, String); -/// Postings for one page's occurrences, read with one scan per table. +/// Postings for one page's occurrences, read through the occurrence indexes +/// `ensure_clone_occurrence_indexes` installs and bucketed by occurrence. /// -/// `clone_exact_postings` and `clone_fingerprint_postings` are -/// `WITHOUT ROWID` tables keyed from `class`/`language`, so a -/// `WHERE symbol_occurrence_id = ?` lookup has no index to use and scans the -/// whole table. Issuing one per body made resume verification quadratic in -/// the postings a repository has: a daemon spent eleven hours inside -/// `verify_clone_fingerprint_page_rows` on one mount without recording a -/// single scheduler pass. Scanning once per page and bucketing by occurrence -/// keeps the comparisons byte-identical while paying the scan once. +/// The postings tables are keyed from `class`/`language`; before those +/// indexes existed a per-occurrence read scanned the whole table, and a +/// daemon spent eleven hours replaying committed pages after a restart. struct ClonePagePostingsV1 { exact: HashMap>, fingerprints: HashMap>, @@ -586,49 +601,47 @@ impl ClonePagePostingsV1 { let mut exact: HashMap> = HashMap::new(); let mut statement = connection .prepare( - "SELECT symbol_occurrence_id, class, normalization_revision, digest, payload_digest FROM clone_exact_postings", + "SELECT class, normalization_revision, digest, payload_digest FROM clone_exact_postings WHERE symbol_occurrence_id = ?1", ) .map_err(sqlite_error)?; - let mut rows = statement.query([]).map_err(sqlite_error)?; - while let Some(row) = rows.next().map_err(sqlite_error)? { - let occurrence: String = row.get(0).map_err(sqlite_error)?; - if !occurrences.contains(occurrence.as_str()) { - continue; + for occurrence in occurrences { + checkpoint(control)?; + let mut rows = statement.query([occurrence]).map_err(sqlite_error)?; + while let Some(row) = rows.next().map_err(sqlite_error)? { + exact.entry((*occurrence).to_owned()).or_default().push(( + row.get(0).map_err(sqlite_error)?, + row.get(1).map_err(sqlite_error)?, + row.get(2).map_err(sqlite_error)?, + row.get(3).map_err(sqlite_error)?, + )); } - exact.entry(occurrence).or_default().push(( - row.get(1).map_err(sqlite_error)?, - row.get(2).map_err(sqlite_error)?, - row.get(3).map_err(sqlite_error)?, - row.get(4).map_err(sqlite_error)?, - )); } - drop(rows); drop(statement); - checkpoint(control)?; let mut fingerprints: HashMap> = HashMap::new(); let mut statement = connection .prepare( - "SELECT symbol_occurrence_id, language, class, normalization_revision, fingerprint, token_position, payload_digest, body_digest FROM clone_fingerprint_postings", + "SELECT language, class, normalization_revision, fingerprint, token_position, payload_digest, body_digest FROM clone_fingerprint_postings WHERE symbol_occurrence_id = ?1", ) .map_err(sqlite_error)?; - let mut rows = statement.query([]).map_err(sqlite_error)?; - while let Some(row) = rows.next().map_err(sqlite_error)? { - let occurrence: String = row.get(0).map_err(sqlite_error)?; - if !occurrences.contains(occurrence.as_str()) { - continue; + for occurrence in occurrences { + checkpoint(control)?; + let mut rows = statement.query([occurrence]).map_err(sqlite_error)?; + while let Some(row) = rows.next().map_err(sqlite_error)? { + fingerprints + .entry((*occurrence).to_owned()) + .or_default() + .push(( + row.get(0).map_err(sqlite_error)?, + row.get(1).map_err(sqlite_error)?, + row.get(2).map_err(sqlite_error)?, + row.get(3).map_err(sqlite_error)?, + row.get(4).map_err(sqlite_error)?, + row.get(5).map_err(sqlite_error)?, + row.get(6).map_err(sqlite_error)?, + )); } - fingerprints.entry(occurrence).or_default().push(( - row.get(1).map_err(sqlite_error)?, - row.get(2).map_err(sqlite_error)?, - row.get(3).map_err(sqlite_error)?, - row.get(4).map_err(sqlite_error)?, - row.get(5).map_err(sqlite_error)?, - row.get(6).map_err(sqlite_error)?, - row.get(7).map_err(sqlite_error)?, - )); } - drop(rows); drop(statement); checkpoint(control)?; From f77e763d10cbea35711f8aba40ce9115ea81f886 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 05:40:59 +0000 Subject: [PATCH 07/17] test(global-db): gate analytics append on abandonment, not poll `single_analytics_append_commits_in_one_writer_dispatch` asserted the append future was still `Poll::Pending` after its first poll. That poll enqueues the INSERT on the writer OS thread via a synchronous `try_send` and then awaits a `bounded(1)` reply channel, so the writer can execute, autocommit and answer before the caller first polls the receiver. The assertion was racing that thread, not checking a contract: 9/60 failures under `taskset -c 0,1` with 6 concurrent copies, matching the CI flake in run 35422336760 (TRY 1 fail, TRY 2 pass). The contract commit 4fc14398f0 added is that a *detached* append still commits: the writer runs the request and only then replies, ignoring a dropped receiver. Make the test enforce that with a gate it owns -- scope the future so it is dropped right after the single poll, before anyone reads the reply -- and keep the existing commit and no-retained-transaction checks. Restoring the transaction-framed append fails the restructured test 30/30 with "single append did not autocommit", so the regression guard is intact and now deterministic. Co-Authored-By: Claude Fable 5.1 (cherry picked from commit ea8113159372c2f4ab1f7c1e8138ce4c5d846494) (cherry picked from commit 80341240b4e94c5fbe075d024337c87b256cc3f3) --- crates/tracedecay-global-db/src/tests.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/crates/tracedecay-global-db/src/tests.rs b/crates/tracedecay-global-db/src/tests.rs index 39200507c3..5072da10d1 100644 --- a/crates/tracedecay-global-db/src/tests.rs +++ b/crates/tracedecay-global-db/src/tests.rs @@ -739,12 +739,19 @@ async fn single_analytics_append_commits_in_one_writer_dispatch() { metadata_json: None, }; - let append = harness.registered.append_analytics_event(&event); - tokio::pin!(append); - assert!(matches!( - futures_util::poll!(&mut append), - std::task::Poll::Pending - )); + // One poll hands the INSERT to the writer thread, which autocommits it and + // only afterwards answers the reply channel. Whether that answer has already + // arrived when the poll returns is a race with that thread, so the poll's own + // result is not the contract; abandoning the future before anyone reads the + // reply is. A single-dispatch autocommit survives that; a transaction-framed + // append would roll back and never reach the inspection connection below. + { + let append = harness.registered.append_analytics_event(&event); + tokio::pin!(append); + if let std::task::Poll::Ready(result) = futures_util::poll!(&mut append) { + result.expect("single analytics append"); + } + } let deadline = std::time::Instant::now() + std::time::Duration::from_secs(1); loop { @@ -758,7 +765,7 @@ async fn single_analytics_append_commits_in_one_writer_dispatch() { } assert!( std::time::Instant::now() < deadline, - "single append did not autocommit while its future remained unpolled" + "single append did not autocommit after its future was abandoned" ); std::thread::sleep(std::time::Duration::from_millis(5)); } From 53d7f9d81b815d4e749dd6449b631acd6b9300f4 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 05:45:16 +0000 Subject: [PATCH 08/17] fix(retention): skip an artifact reclaimed during the scan `mounted_code_generation_retention_continues_capped_segment_reclamation` failed both nextest tries at ~3.9 s on master run 35422072661 with `code generation retention plan: Storage("No such file or directory (os error 2)")`; the identical tree passed the same partition minutes earlier on run 35420910183, so it is a race. Under `taskset -c 0,1` with 6 concurrent copies it reproduced 4/78. Instrumenting every enumerate-then-open loop in the planner with the path and a re-stat named the vanishing file on both ENOENT reproductions: the text-artifact inventory's `read_dir` + `symlink_metadata` loop, on `.text-artifact-.staging` and on a `.staging-journal` sidecar, both `present_now=false` while their siblings were still listed. `prepare_next_code_generation_retention_cancellable` scans without the generation-store lock on purpose - the full-digest read routinely covers several GiB and must not pin the daemon writer gate - and the text-artifact builder retires a whole `.staging` family (`discard_incompatible_staging` -> `retire_text_artifact_staging_family`) under that lock in the background pass tail, the same tail fenced in 656b5328e7 and 760e221dda. So an entry the listing just named can be gone before the scan stats it, and the whole plan failed. Production reads that as `retention_plan_failed` and fails the pass with a loud degraded log, so every publish that raced a maintenance tick lost the tick. An entry that vanished mid-scan is already reclaimed, which is what the inventory would have planned for it anyway: skip it, the way this loop already skips an absent active-staging path fifteen lines above. Completed artifacts the durable index references are verified before this scan and stay fail-closed. The unit test retires the staging family from inside the scan's own cancellation probe and asserts the plan survives without naming the vanished files; it fails with the ENOENT before this change. Co-Authored-By: Claude Fable 5.1 (cherry picked from commit 012965c52514790e56d5531e51b48acb808cd1c5) --- .../src/code_index_generations/tests.rs | 46 +++++++++++++++++++ .../code_index_generations/text_artifacts.rs | 17 ++++++- 2 files changed, 62 insertions(+), 1 deletion(-) 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 2dbe91a0c7..15ae71fb0e 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 @@ -997,6 +997,52 @@ fn text_artifact_retention_collects_staging_database_sidecars_with_their_owner() ); } +/// The inventory scans the artifact root without the generation-store lock, so +/// the text-artifact builder can retire a `.staging` family between the +/// directory listing and the stat. A vanished entry is already reclaimed and +/// must leave the plan intact rather than failing it with a storage error. +#[test] +fn text_artifact_inventory_skips_an_entry_reclaimed_during_the_scan() { + let store = tempfile::TempDir::new().expect("artifact store"); + let artifacts_root = code_text_artifacts_root(store.path()); + std::fs::create_dir_all(&artifacts_root).expect("create artifact root"); + let staging_family = ["a", "b", "c"] + .into_iter() + .map(|seed| { + let path = artifacts_root.join(format!(".text-artifact-{}.staging", seed.repeat(64))); + std::fs::write(&path, b"staging").expect("write staging evidence"); + path + }) + .collect::>(); + + // The scan probes cancellation once on entry and once per directory entry, + // before it takes that entry. Retiring from the third probe on leaves the + // listing already taken and one entry already inspected, so every further + // name the scan holds names a file that is gone from disk. + let probes = std::sync::atomic::AtomicUsize::new(0); + let retire_during_the_scan = || { + if probes.fetch_add(1, std::sync::atomic::Ordering::Relaxed) >= 2 { + for path in &staging_family { + let _ = std::fs::remove_file(path); + } + } + false + }; + + let inventory = plan_collectable_text_artifacts_cancellable( + store.path(), + None, + GenerationDigestVerificationV1::Full, + &retire_during_the_scan, + ) + .expect("an entry reclaimed mid-scan leaves the store plannable"); + assert!( + inventory.candidates.len() < staging_family.len(), + "an entry that vanished before its stat is reclaimed, not planned: {:?}", + inventory.candidates + ); +} + #[test] fn applied_retention_refuses_a_busy_generation_store_and_retries() { let (store, _) = fixture_store(2); 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 c62e73dc92..bc4bfe5588 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 @@ -394,7 +394,22 @@ pub(super) fn plan_collectable_text_artifacts_cancellable( ) })?; let path = entry.path(); - let metadata = std::fs::symlink_metadata(&path).map_err(storage)?; + // This inventory reads the artifact root without the generation-store + // lock, so an entry the listing just named can already be gone: the + // text-artifact builder retires a `.staging` family (the staging + // database and its `-journal`/`-wal`/`-shm` sidecars) under that lock + // while this scan runs. A vanished entry is reclaimed, which is what + // this inventory would have planned anyway, so it is not a candidate + // and not a failure. Failing the plan here turned every publish that + // raced a maintenance tick into a loud `retention_plan_failed` pass + // (master run 35422072661, `Storage("No such file or directory")`). + // A completed artifact the durable index *references* is verified + // above, before this scan, and stays fail-closed if it disappears. + let metadata = match std::fs::symlink_metadata(&path) { + Ok(metadata) => metadata, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue, + Err(error) => return Err(storage(error)), + }; if !metadata.file_type().is_file() { return Err(CodeGenerationRetentionErrorV1::UnsafeState(format!( "code text artifact inventory path '{}' is not a regular file", From b130bcea0e45d57d0dbfc11a411f6e137704cc44 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 05:45:17 +0000 Subject: [PATCH 09/17] test(daemon): defer the retention plan while the store is busy The same 6-copy `taskset -c 0,1` reproduction of `mounted_code_generation_retention_continues_capped_segment_reclamation` also failed with `code generation retention plan: GenerationStoreBusy` (1 of 4 failures in 78 runs). That is not a failure. The planner's recovery probes the generation-store lock with `try_acquire_code_generation_store_lock` and answers `GenerationStoreBusy` when a writer owns the store; production maintenance consumes it with `defer_generation_store_busy` and comes back on the next tick. The route under test stays mounted, and `publish_code_edit` returns as soon as the serving generation id changes, so the pass tail that sealed that generation can still own the store when the test plans - the same pass-tail exposure as 760e221dda. Consume the typed busy answer under a bounded wait instead of reading it as fatal. Every assertion on the resulting plan is unchanged, and any other error still panics. Co-Authored-By: Claude Fable 5.1 (cherry picked from commit 833a3f30a2c77c426864123c0065bc4d8b664586) --- .../generation_retention_test.rs | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/crates/tracedecay/src/daemon/production_harness/generation_retention_test.rs b/crates/tracedecay/src/daemon/production_harness/generation_retention_test.rs index b0d224dfec..6415ef3209 100644 --- a/crates/tracedecay/src/daemon/production_harness/generation_retention_test.rs +++ b/crates/tracedecay/src/daemon/production_harness/generation_retention_test.rs @@ -11,7 +11,8 @@ use super::journey_test_support::git; use super::*; use crate::daemon::maintenance::project_store_maintenance_lease; use tracedecay_code_index_retention::code_index_generations::{ - MAX_CODE_GENERATION_RETENTION_BATCH_V1, prepare_next_code_generation_retention_cancellable, + CodeGenerationRetentionErrorV1, MAX_CODE_GENERATION_RETENTION_BATCH_V1, + prepare_next_code_generation_retention_cancellable, }; use tracedecay_maintenance::tick::{MaintenanceContinuation, MaintenanceTickOutcome}; @@ -114,13 +115,29 @@ async fn mounted_code_generation_retention_continues_capped_segment_reclamation( &canonical_root, ); let graph_replay_pool_root = graph.db().database_path().with_extension("graph-replay"); - let plan = prepare_next_code_generation_retention_cancellable( - &code_store_root, - &BTreeSet::new(), - &|| false, - Some(&graph_replay_pool_root), - ) - .expect("code generation retention plan"); + // The planner probes the generation-store lock and answers + // `GenerationStoreBusy` whenever a writer owns the store; production + // maintenance defers that tick and comes back. This route stays mounted, + // so the pass tail that publishes the edits above can still own the store + // here. Consume the same typed answer instead of reading it as a failure. + let plan = tokio::time::timeout(Duration::from_secs(30), async { + loop { + match prepare_next_code_generation_retention_cancellable( + &code_store_root, + &BTreeSet::new(), + &|| false, + Some(&graph_replay_pool_root), + ) { + Ok(plan) => return plan, + Err(CodeGenerationRetentionErrorV1::GenerationStoreBusy) => { + tokio::time::sleep(Duration::from_millis(25)).await; + } + Err(error) => panic!("code generation retention plan: {error:?}"), + } + } + }) + .await + .expect("code generation retention plan converges"); let first_candidate = plan .collectable_generations .iter() From 63c0784994fbc63ed9552465a0b8a2d6353247d1 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 05:59:14 +0000 Subject: [PATCH 10/17] fix(mcp): attribute risky sites by byte span, not line `tracedecay_unsafe_patterns` picked `enclosing` as the smallest-line-span symbol covering the match's line. When several declarations share one line every candidate has span 1, so the winner was whichever symbol the page happened to yield first, and pages arrive in occurrence order over per-project digests. `#[test] fn a() { x.unwrap(); } pub fn b() { panic!(); }` therefore reported `@test` (the annotation-usage node the Rust extractor names `::@name`), `a`, or `b` at random: 7 of 12 identical local runs disagreed, which is what broke the CI job deterministically on its runner. Resolve the enclosing declaration by the byte range the graph already publishes in `CodeGraphSymbolBindingV1::source_span`, containing the offset of the matched construct. That is stable, names the innermost declaration that really contains the site, and drops attributes for free: `#[test]` spans only its own bytes, so it can never contain a call. Co-Authored-By: Claude Fable 5.1 (cherry picked from commit 178afe414b370165fd8910620085e922d7e9fe27) --- .../src/handlers/analysis/mod.rs | 10 +- .../src/handlers/analysis/unsafe_patterns.rs | 133 ++++++++++-------- 2 files changed, 87 insertions(+), 56 deletions(-) diff --git a/crates/tracedecay-mcp/src/handlers/analysis/mod.rs b/crates/tracedecay-mcp/src/handlers/analysis/mod.rs index e3f3ffe05b..d85bbfcd53 100644 --- a/crates/tracedecay-mcp/src/handlers/analysis/mod.rs +++ b/crates/tracedecay-mcp/src/handlers/analysis/mod.rs @@ -43,7 +43,7 @@ use tracedecay_code_index::graph_projection::CodeGraphSemanticEdgeV1; use tracedecay_code_index::lineage::LineageSymbolRecordV1; use tracedecay_domain::code_intelligence::NodeKind; use tracedecay_domain::errors::{Result, TraceDecayError}; -use tracedecay_domain::{RelationEdgeKindV1, SymbolOccurrenceId}; +use tracedecay_domain::{RelationEdgeKindV1, SourceSpan, SymbolOccurrenceId}; use tracedecay_graph_query::VerifiedGraphQuery; fn path_is_rust(path: &str) -> bool { @@ -63,6 +63,9 @@ const ANALYSIS_RELATION_BUDGET: usize = 2_000_000; struct VerifiedAnalysisSymbol { occurrence: SymbolOccurrenceId, path: String, + /// Byte range the declaration occupies in its file. Line numbers cannot + /// separate two declarations that share one line; this can. + source_span: Option, metadata: LineageSymbolRecordV1, } @@ -89,6 +92,10 @@ fn verified_analysis_symbols( page.symbols .into_iter() .map(|symbol| { + let source_span = symbol + .binding + .as_ref() + .and_then(|binding| binding.source_span); let path = symbol .binding .and_then(|binding| binding.logical_path) @@ -109,6 +116,7 @@ fn verified_analysis_symbols( Ok(VerifiedAnalysisSymbol { occurrence: symbol.occurrence, path, + source_span, metadata, }) }) diff --git a/crates/tracedecay-mcp/src/handlers/analysis/unsafe_patterns.rs b/crates/tracedecay-mcp/src/handlers/analysis/unsafe_patterns.rs index 920b28a21c..b1dc182a08 100644 --- a/crates/tracedecay-mcp/src/handlers/analysis/unsafe_patterns.rs +++ b/crates/tracedecay-mcp/src/handlers/analysis/unsafe_patterns.rs @@ -41,23 +41,28 @@ fn source_may_contain_unsafe_kind(source: &str, kind: &str) -> bool { } } -fn line_matches_unsafe_kind(line: &str, kind: &str) -> bool { +/// Byte offset of the risky construct within `line`, when the line has one. +/// +/// The offset is what lets a match be attributed to the declaration that +/// actually contains it: two declarations can share a line, so a line number +/// alone cannot say which one a site belongs to. +fn line_matches_unsafe_kind(line: &str, kind: &str) -> Option { let trimmed = line.trim_start(); if trimmed.starts_with("//") || trimmed.starts_with("///") { - return false; + return None; } match kind { "unwrap" => contains_method_call(line, "unwrap", true), "expect" => contains_method_call(line, "expect", false), - "panic" => line.contains("panic!("), - "todo" => line.contains("todo!("), - "unimplemented" => line.contains("unimplemented!("), + "panic" => line.find("panic!("), + "todo" => line.find("todo!("), + "unimplemented" => line.find("unimplemented!("), "unsafe_block" => contains_unsafe_block_start(line), - _ => false, + _ => None, } } -fn contains_method_call(line: &str, method: &str, empty_parens: bool) -> bool { +fn contains_method_call(line: &str, method: &str, empty_parens: bool) -> Option { let needle = format!(".{method}"); let bytes = line.as_bytes(); let mut start = 0usize; @@ -69,18 +74,18 @@ fn contains_method_call(line: &str, method: &str, empty_parens: bool) -> bool { if is_word_boundary && next == Some(b'(') { if empty_parens { if line[after + 1..].trim_start().starts_with(')') { - return true; + return Some(abs); } } else { - return true; + return Some(abs); } } start = abs + needle.len(); } - false + None } -fn contains_unsafe_block_start(line: &str) -> bool { +fn contains_unsafe_block_start(line: &str) -> Option { let bytes = line.as_bytes(); let mut start = 0usize; while let Some(pos) = line[start..].find("unsafe") { @@ -97,12 +102,28 @@ fn contains_unsafe_block_start(line: &str) -> bool { || rest.starts_with("impl ") || rest.starts_with("trait ") { - return true; + return Some(abs); } } start = abs + "unsafe".len(); } - false + None +} + +/// Innermost declaration whose source range covers `match_byte`. +/// +/// Byte containment, not line containment: an attribute such as `#[test]` and +/// the two functions on `#[test] fn a() {…} fn b() {…}` all sit on one line, +/// and only the byte range says which of them the site is inside. Selecting by +/// line also had no stable order to break ties with, since symbols arrive in +/// occurrence order and occurrence ids are per-project digests. +fn enclosing_declaration(nodes: &[VerifiedAnalysisSymbol], match_byte: u64) -> Option { + nodes + .iter() + .filter_map(|node| node.source_span.map(|span| (node, span))) + .filter(|(_, span)| span.start_byte <= match_byte && match_byte < span.end_byte) + .min_by_key(|(_, span)| span.end_byte.saturating_sub(span.start_byte)) + .map(|(node, _)| node.metadata.qualified_name.clone()) } fn path_looks_like_test(path: &str) -> bool { @@ -216,7 +237,16 @@ pub async fn handle_unsafe_patterns( // Masking can erase every raw hit (all of them in comments or // string literals), so the file's nodes are fetched only once a // real match survives. - for (idx, (line, masked_line)) in source.lines().zip(masked.lines()).enumerate() { + // Split inclusively so each line keeps its own byte offset; + // masking preserves byte layout, so the two sides stay aligned. + let mut line_start = 0usize; + for (idx, (line, masked_line)) in source + .split_inclusive('\n') + .zip(masked.split_inclusive('\n')) + .enumerate() + { + let line_offset = line_start; + line_start += line.len(); let line_no = (idx as u32) + 1; // A mixed test/production line is not wholly test scope, // so keep its production risk visible. @@ -225,16 +255,10 @@ pub async fn handle_unsafe_patterns( continue; } for kind in &kinds { - if line_matches_unsafe_kind(masked_line, kind) { + if let Some(column) = line_matches_unsafe_kind(masked_line, kind) { let nodes = symbols_by_file.get(file).map_or(&[][..], Vec::as_slice); - let enclosing = nodes - .iter() - .filter(|n| { - n.metadata.start_line.saturating_add(1) <= line_no - && line_no <= n.end_line().saturating_add(1) - }) - .min_by_key(|n| n.metadata.line_span) - .map(|n| n.metadata.qualified_name.clone()); + let enclosing = + enclosing_declaration(nodes, (line_offset + column) as u64); *by_kind.entry(kind.clone()).or_insert(0) += 1; matches.push(json!({ "kind": kind, @@ -314,7 +338,7 @@ mod unsafe_pattern_detection_tests { for line in lines { for kind in kinds { - if line_matches_unsafe_kind(line, kind) { + if line_matches_unsafe_kind(line, kind).is_some() { assert!( source_may_contain_unsafe_kind(line, kind), "prefilter would drop a real {kind} site: {line:?}" @@ -337,47 +361,46 @@ mod unsafe_pattern_detection_tests { fn detects_unsafe_block_inside_safe_fn() { // An `unsafe { }` block living inside an otherwise-safe function, the // exact shape the audit fixture plants. - assert!(line_matches_unsafe_kind( - " unsafe { *ptr as usize }", - "unsafe_block" - )); - assert!(contains_unsafe_block_start(" unsafe { *ptr as usize }")); + assert!(line_matches_unsafe_kind(" unsafe { *ptr as usize }", "unsafe_block").is_some()); + assert!(contains_unsafe_block_start(" unsafe { *ptr as usize }").is_some()); } #[test] fn detects_unsafe_fn_impl_and_trait() { - assert!(line_matches_unsafe_kind( - "pub unsafe fn raw(&self) {", - "unsafe_block" - )); - assert!(line_matches_unsafe_kind( - "unsafe impl Send for Foo {}", - "unsafe_block" - )); - assert!(line_matches_unsafe_kind( - "unsafe trait Zeroable {}", - "unsafe_block" - )); + assert!(line_matches_unsafe_kind("pub unsafe fn raw(&self) {", "unsafe_block").is_some()); + assert!(line_matches_unsafe_kind("unsafe impl Send for Foo {}", "unsafe_block").is_some()); + assert!(line_matches_unsafe_kind("unsafe trait Zeroable {}", "unsafe_block").is_some()); } #[test] fn ignores_safe_code_and_comments() { // Plain safe code has no unsafe markers. - assert!(!line_matches_unsafe_kind( - "let x = total as usize;", - "unsafe_block" - )); + assert!(line_matches_unsafe_kind("let x = total as usize;", "unsafe_block").is_none()); // The word appears only in a comment/doc line: not a real unsafe site. - assert!(!line_matches_unsafe_kind( - "// this is not unsafe { } really", - "unsafe_block" - )); - assert!(!line_matches_unsafe_kind( - "/// drop the needless unsafe block", - "unsafe_block" - )); + assert!( + line_matches_unsafe_kind("// this is not unsafe { } really", "unsafe_block").is_none() + ); + assert!( + line_matches_unsafe_kind("/// drop the needless unsafe block", "unsafe_block") + .is_none() + ); // A substring of a longer identifier must not trip the word-boundary check. - assert!(!contains_unsafe_block_start("let unsafely = 1;")); - assert!(!contains_unsafe_block_start("let make_unsafe_thing = 2;")); + assert!(contains_unsafe_block_start("let unsafely = 1;").is_none()); + assert!(contains_unsafe_block_start("let make_unsafe_thing = 2;").is_none()); + } + + /// The reported offset is what attributes a site to a declaration, so it + /// has to point at the construct itself, not at the start of the line. + #[test] + fn reports_where_on_the_line_the_site_is() { + let line = "#[test] fn a() { Some(5).unwrap(); } pub fn b() { panic!(); }"; + assert_eq!( + line_matches_unsafe_kind(line, "unwrap"), + Some(line.find(".unwrap()").expect("unwrap call")) + ); + assert_eq!( + line_matches_unsafe_kind(line, "panic"), + Some(line.find("panic!(").expect("panic call")) + ); } } From 6d8ef97dabc2604a98209731d99bde80069e5164 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 06:02:31 +0000 Subject: [PATCH 11/17] test(application): keep git auto-maintenance out of the fixture `adapter_leaves_repository_byte_identical` snapshots `.git` before and after the read-only adapter calls. The fixture's `git commit` spawns a detached `git maintenance run --auto`, whose `.git/objects/maintenance.lock` was still present for the first walk and gone for the second, so the byte-identical assertion failed on both nextest tries with the lock as the only difference. Disable auto maintenance and gc for every fixture git call, as the scheduler and daemon fixtures already do. Co-Authored-By: Claude Fable 5.1 --- crates/tracedecay-application/src/git_intelligence.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/tracedecay-application/src/git_intelligence.rs b/crates/tracedecay-application/src/git_intelligence.rs index 5046bbf93d..ed8ff8af72 100644 --- a/crates/tracedecay-application/src/git_intelligence.rs +++ b/crates/tracedecay-application/src/git_intelligence.rs @@ -1762,6 +1762,14 @@ mod tests { "user.email=fixture@example.com", "-c", "commit.gpgsign=false", + // `git commit` spawns a detached `git maintenance run --auto` + // that holds `.git/objects/maintenance.lock` after the commit + // returns; the byte-identical snapshot must not see it appear + // or vanish between its two walks. + "-c", + "maintenance.auto=false", + "-c", + "gc.auto=0", ]) .args(args) .current_dir(self.path()) From b05cab24a99c15b5074dc97e1cfa015bed2d7040 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 06:29:55 +0000 Subject: [PATCH 12/17] fix(daemon): mount the published branch worktree's query authority MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An explicitly published branch worktree is never a project-open route, so nothing mounted a core query authority on it. An exact branch read could only borrow one already mounted on a peer checkout of the same repository (`mount_query_authority_from_project_peer`), and that peer's own mount is deferred until it seats a text generation. A read taken right after the publication sealed its provenance therefore failed closed with a non-retryable `authority_unavailable`, even though the branch generation was committed and servable. That is the CI flake in the tracedecay-cli `core_cli_suite` linked-worktree branch-add journey, where a freshly added peer worktree searches the just-published branch before it has seated anything of its own. Mount the published branch's own authority from the project's durable cursor-key authority, reusing the existing project-open mount rather than adding a second mount path. It runs inside the daemon-owned publication task after the generation is committed, so the admitting `branch add` caller — which returns at admission, before activation even starts — never waits for it and never pays for the profile session-registry lock. An earlier attempt (51402cdf8d) mounted on the admitting caller's path and correlated with mount and publish failures elsewhere; this one stays off that critical section. Best effort by design: the generation is already committed, so a missing session mount or cursor key must not retract it, and the exact read still falls back to borrowing a peer authority when the mount could not run. Co-Authored-By: Claude Fable 5.1 (cherry picked from commit 6d887a0c3ce25c2838f2943200e133a610cdf2e8) --- crates/tracedecay/src/daemon/branch_add.rs | 109 ++++++++++++++++++- crates/tracedecay/src/daemon/branch_admin.rs | 18 +++ 2 files changed, 126 insertions(+), 1 deletion(-) diff --git a/crates/tracedecay/src/daemon/branch_add.rs b/crates/tracedecay/src/daemon/branch_add.rs index 4e2d3793f7..0b7b5f17e2 100644 --- a/crates/tracedecay/src/daemon/branch_add.rs +++ b/crates/tracedecay/src/daemon/branch_add.rs @@ -1,4 +1,4 @@ -use std::path::Path; +use std::path::{Path, PathBuf}; use std::sync::Arc; use tracedecay_application::pr_tracking::{ @@ -153,6 +153,9 @@ async fn activate_and_track_manual_branch( let graph = Arc::clone(graph); let schedulers = schedulers.clone(); let branch = branch.to_owned(); + let published_data_root = data_root.clone(); + let published_schedulers = schedulers.clone(); + let published_registries = administration.session_runtime_registries(); administration .admit_manual_branch_publication(|cancellation, admitted| async move { @@ -215,6 +218,15 @@ async fn activate_and_track_manual_branch( tracked } .await; + if matches!(&result, Ok(outcome) if *outcome != BranchAddOutcome::Deferred) { + mount_published_branch_query_authority( + published_registries.as_ref(), + &published_schedulers, + &published_data_root, + &branch, + ) + .await; + } match &result { Ok(outcome) => log_daemon_event( "manual_branch_publication", @@ -237,6 +249,101 @@ async fn activate_and_track_manual_branch( .await } +/// Mounts the checked-in core query authority on the branch worktree this +/// publication sealed, from the project's own durable cursor-key authority. +/// +/// An explicitly published branch worktree is never a project-open route, so +/// nothing else mounts its query authority: an exact branch read could only +/// borrow one already mounted on a peer checkout of the same repository +/// (`mount_query_authority_from_project_peer`), and that peer's own mount is +/// deferred until it seats a text generation. A read taken right after this +/// publication sealed its provenance therefore failed closed with a +/// non-retryable `authority_unavailable` even though the branch generation was +/// published and servable. Mounting here makes the generation this journey +/// publishes queryable without depending on an unrelated worktree's +/// activation order. +/// +/// Runs inside the daemon-owned publication task, after the generation it +/// serves is committed: the admitting `branch add` caller returns at admission +/// and never waits for this, and the profile's session-registry lock is taken +/// here rather than on that caller's path. +/// +/// Best effort by design: the branch generation is already committed, so a +/// missing session mount or cursor key must not retract it. The exact branch +/// read falls back to borrowing a peer authority when this could not run. +#[cfg(unix)] +#[hotpath::measure(label = "daemon.branch_add.query_authority", future = true)] +async fn mount_published_branch_query_authority( + registries: Option<&(super::branch_admin::SharedSessionRuntimeRegistries, PathBuf)>, + schedulers: &CodeIndexSchedulerRegistryV1, + data_root: &Path, + branch: &str, +) { + let Some((registries, profile_root)) = registries else { + return; + }; + let Some(source) = + tracedecay_runtime_core::branch_meta::load_branch_meta(data_root).and_then(|meta| { + meta.branches + .get(branch) + .and_then(|entry| entry.graph_source.clone()) + }) + else { + return; + }; + let worktree_root = PathBuf::from(&source.worktree_root); + let Ok(project_id) = tracedecay_domain::ProjectId::new(source.project_id.clone()) else { + return; + }; + let Ok(scope) = + tracedecay_code_index_runtime::resolved_scope_for_project(&worktree_root, &project_id) + else { + return; + }; + let sessions = { + let registries = registries.lock().await; + registries + .get(profile_root) + .map(|entry| Arc::clone(&entry.registry)) + }; + let Some(sessions) = sessions.and_then(|registry| registry.get().cloned()) else { + return; + }; + let Some(session_db) = sessions.mounted_project_sessions(&project_id).await else { + return; + }; + let cursor_keys = match session_db.load_session_cursor_key_provider_result().await { + Ok(cursor_keys) => cursor_keys, + Err(error) => { + tracing::debug!( + event = "branch_query_authority_mount", + outcome = "unavailable", + branch = %branch, + reason = %error, + "durable query cursor key is unavailable for the published branch" + ); + return; + } + }; + if let Err(error) = + tracedecay_code_index_runtime::code_index_scheduler::query_runtime::mount_core_query_authority_on_project_open( + schedulers, + &worktree_root, + &scope, + &cursor_keys, + ) + .await + { + tracing::debug!( + event = "branch_query_authority_mount", + outcome = "unavailable", + branch = %branch, + reason = %error, + "published branch query authority is unavailable; exact reads fall back to a peer" + ); + } +} + #[cfg(unix)] #[hotpath::measure(label = "daemon.branch_add.owner", future = true)] pub(super) async fn activate_and_track_manual_branch_owned( diff --git a/crates/tracedecay/src/daemon/branch_admin.rs b/crates/tracedecay/src/daemon/branch_admin.rs index f65a2c3924..899dc6fcfc 100644 --- a/crates/tracedecay/src/daemon/branch_admin.rs +++ b/crates/tracedecay/src/daemon/branch_admin.rs @@ -944,6 +944,24 @@ impl StoreAdministration { registry.mounted_session_databases().await } + /// The profile's session-runtime registry map and its canonical root, + /// taken without locking either. + /// + /// Branch publication resolves the project's durable cursor-key authority + /// through this inside its own background task, so an explicitly published + /// branch can mount its own query authority without the admitting caller + /// paying for a registry lock it never reads. + #[cfg(unix)] + pub(super) fn session_runtime_registries( + &self, + ) -> Option<(SharedSessionRuntimeRegistries, std::path::PathBuf)> { + let profile_root = self + .profile_identity() + .and_then(|identity| authority::canonical_identity_path(identity.profile_root())) + .ok()?; + Some((Arc::clone(&self.session_runtime_registries), profile_root)) + } + #[hotpath::measure(label = "daemon.branch_admin.mounted_project_servers", future = true)] pub(super) async fn mounted_project_servers(&self) -> Vec> { let Ok(profile_root) = self From 2579de9da1e39d66f28ba4b2ad6415c62c1ecd94 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 19 Sep 2026 06:35:55 +0000 Subject: [PATCH 13/17] fix(code-index): idle means the pass tail already ran The worker dropped its admission permit and reconcile_in_progress, then still renamed the active pointer and stamped BusyFollowUp. Those two signals are now idle only after that tail: continuations are stamped while the pass is visible, and published text projection re-takes the permit before the pointer rename. The pass is dropped only across that permit wait, so a holder waiting on the flag cannot deadlock. Co-authored-by: Zack Jackson --- .../src/code_index_scheduler/registry.rs | 15 ++ .../code_index_scheduler/registry/mount.rs | 165 +++++++++++++++--- .../src/code_index_scheduler/tests/mod.rs | 13 +- 3 files changed, 156 insertions(+), 37 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 a5392c83b1..ab44191731 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 @@ -2109,6 +2109,21 @@ impl CodeIndexSchedulerRegistryV1 { } } + /// Stamp a continuation while `reconcile_in_progress` still reports this pass. + /// + /// Callers that already released the worker's pass guard use this so a + /// reader waiting for the counter to hit zero cannot observe an empty + /// slot and then lose to `BusyFollowUp`. The stamp is the idle boundary; + /// the guard lives only for the note. + fn note_visible_worker_continuation( + passes: &Arc, + pending_wake: &PendingWakeV1, + wake: &tokio::sync::Notify, + ) { + let _visible = super::ReconcilePassGuard::enter(passes); + Self::note_worker_continuation(pending_wake, wake); + } + /// Claim the pending wake as one reconcile's arrival, at the instant the /// scheduler dequeues it. /// 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..f488bde488 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 @@ -1031,12 +1031,16 @@ impl CodeIndexSchedulerRegistryV1 { started_micros, ); } - // Source reconciliation is complete: release the background - // admission permit before HeadOpening / graph work so sibling - // stores can start. Keep `reconcile_pass` through text - // seating, dropping it made `reconcile_in_progress` lie while - // this worker still owned graph try_lock, which deadlocked - // tests that hold the scheduler mutex and wait for that flag. + // Source reconciliation is complete. Release the admission + // permit only across HeadOpening's scheduler-mutex wait: a + // holder of that mutex must be able to run, and an + // ignored-dependency owner still needs this permit before it + // can take the mutex. The publication's text projection + // re-acquires the permit before it renames the active pointer. + // Keep `reconcile_pass` through text seating; dropping it + // made `reconcile_in_progress` lie while this worker still + // owned graph try_lock, which deadlocked tests that hold the + // scheduler mutex and wait for that flag. drop(_background_reconcile_admission); // A publication must first reopen its own lightweight text // owner: publication moved the durable pointer, so the prior @@ -1110,6 +1114,49 @@ impl CodeIndexSchedulerRegistryV1 { && !graph_activation_deferred && let Some(text) = graph_text.clone() { + // Head opening released the permit so it could wait on + // the scheduler mutex. Take it back for the pointer + // rename. Drop the pass across that wait: a caller + // holding the permit and waiting for the pass would + // otherwise deadlock, and the pass is re-entered + // before the rename so idle still means the pointer + // write has finished. + let resume_pass = reconcile_pass.is_some(); + drop(reconcile_pass.take()); + let Ok(_text_artifact_admission) = hotpath::future!( + Arc::clone(&worker_background_reconcile_admission).acquire_owned(), + label = "daemon.code_index.admission_wait" + ) + .await + else { + tracing::info!( + event = "code_index_worker_shutdown_observed", + phase = "published_text_projection", + "code-index worker observed shutdown and stopped its pass" + ); + Self::join_retained_text_projection_on_worker_exit( + &mut retained_text_projection, + ) + .await; + return; + }; + if worker_shutting_down.load(Ordering::Acquire) { + tracing::info!( + event = "code_index_worker_shutdown_observed", + phase = "published_text_projection", + "code-index worker observed shutdown and stopped its pass" + ); + Self::join_retained_text_projection_on_worker_exit( + &mut retained_text_projection, + ) + .await; + return; + } + if resume_pass { + reconcile_pass = Some(super::super::ReconcilePassGuard::enter( + &worker_reconcile_in_progress, + )); + } let projection = tokio::spawn(Self::drive_text_projection( text, Arc::clone(&worker_shutting_down), @@ -1168,7 +1215,21 @@ impl CodeIndexSchedulerRegistryV1 { // A successor-only retained projection holds no pass guard of // its own; keeping the worker's guard through graph seat would // report rebuild_in_flight for clone backfill that is not - // exact/lexical work. + // exact/lexical work. Stamp the continuation this projection + // already owes before that drop: the slot, not a later note, + // is what an idle reader observes. + if let Some(outcome) = published_text_projection_outcome.as_ref() { + let schedule_continuation = match outcome { + PublishedTextProjectionOutcomeV1::Finished => graph_text + .as_ref() + .is_some_and(LatestCodeTextGenerationV1::text_projection_needs_work), + PublishedTextProjectionOutcomeV1::Unfinished => true, + PublishedTextProjectionOutcomeV1::Shutdown => false, + }; + if schedule_continuation { + Self::note_worker_continuation(&worker_pending_wake, &worker_wake); + } + } if retained_text_projection.is_none() || retained_projection_successor_only { drop(reconcile_pass.take()); } @@ -1296,6 +1357,14 @@ impl CodeIndexSchedulerRegistryV1 { .filter(|retained| retained.uses_partitioned_manifest()) .cloned() { + // Every outcome of this attempt schedules one successor. + // Stamp it before the recovery await, while the pass is + // visible, so the wait cannot be sampled as an idle slot. + Self::note_visible_worker_continuation( + &worker_reconcile_in_progress, + &worker_pending_wake, + &worker_wake, + ); retained_graph_head_recovery_attempted = true; let generation_id = retained.metadata().manifest().generation_id.clone(); let replay_scheduler = Arc::clone(&worker_scheduler); @@ -1400,8 +1469,8 @@ impl CodeIndexSchedulerRegistryV1 { // all and never published the successor generation. The // `retained_graph_head_recovery_attempted` guard above is // now false for every later pass, so this cannot spin - // another retained-recovery Noop. - Self::note_worker_continuation(&worker_pending_wake, &worker_wake); + // another retained-recovery Noop. The successor was + // stamped before this await. } // A recovered revision-7 verified head already serves its // native graph from the retained text owner, and that owner @@ -1458,6 +1527,8 @@ impl CodeIndexSchedulerRegistryV1 { let graph_text = graph_text.clone(); let shutting_down = Arc::clone(&worker_shutting_down); let prepare_passes = Arc::clone(&worker_reconcile_in_progress); + let prepare_pending_wake = Arc::clone(&worker_pending_wake); + let prepare_wake = Arc::clone(&worker_wake); match hotpath::future!( tokio::task::spawn_blocking(move || { let decoder = Self::lock_scheduler_for_graph_step( @@ -1513,6 +1584,18 @@ impl CodeIndexSchedulerRegistryV1 { )? .1 .take_ignored_roster_refusal_rebuild(); + if roster_refusal_rebuild { + // One pass, claimed from the scheduler, so + // a refusal that keeps reproducing cannot + // spin this worker. Stamp before this + // closure drops the step guard: the result + // is observed only after the slot is set. + Self::note_visible_worker_continuation( + &prepare_passes, + &prepare_pending_wake, + &prepare_wake, + ); + } let replay_binding = match latest.as_ref() { Some(latest) => Some( Self::lock_scheduler_for_graph_step( @@ -1545,15 +1628,6 @@ impl CodeIndexSchedulerRegistryV1 { the sealed generation cannot seat" ); } - if roster_refusal_rebuild { - // One pass, claimed from the scheduler, so - // a refusal that keeps reproducing cannot - // spin this worker. - Self::note_worker_continuation( - &worker_pending_wake, - &worker_wake, - ); - } Ok((outcome, latest, replay_binding)) } Ok(Err(error)) => { @@ -1714,7 +1788,14 @@ impl CodeIndexSchedulerRegistryV1 { .as_ref() .is_some_and(LatestCodeTextGenerationV1::text_projection_needs_work) { - Self::note_worker_continuation(&worker_pending_wake, &worker_wake); + // Already stamped before optional graph. Re-enter + // the pass so a reader that cleared the slot + // during graph still cannot sample the stamp. + Self::note_visible_worker_continuation( + &worker_reconcile_in_progress, + &worker_pending_wake, + &worker_wake, + ); } // Large text projections can outlive the bounded // source proof established before publication. The @@ -1784,7 +1865,11 @@ impl CodeIndexSchedulerRegistryV1 { "the publication's text owner did not finish its projection; \ the sealed generation stays unseated until it does" ); - Self::note_worker_continuation(&worker_pending_wake, &worker_wake); + Self::note_visible_worker_continuation( + &worker_reconcile_in_progress, + &worker_pending_wake, + &worker_wake, + ); } } // Keep the pass lifetime around the post-projection source @@ -1965,7 +2050,11 @@ impl CodeIndexSchedulerRegistryV1 { if text_latest.text_projection_needs_work() && !text_latest.query_owners_are_ready() { - Self::note_worker_continuation(&worker_pending_wake, &worker_wake); + Self::note_visible_worker_continuation( + &worker_reconcile_in_progress, + &worker_pending_wake, + &worker_wake, + ); } } Ok(Err(error)) => { @@ -1994,7 +2083,27 @@ impl CodeIndexSchedulerRegistryV1 { } // The source proof and serving witness are now published as // one lifecycle. Optional receipts do not keep source - // verification in flight. + // verification in flight. A clone-backfill continuation this + // pass already knows about is stamped first, so the drop is + // not an empty slot. + if clone_backfill_waiting_for_source + && matches!( + &result, + Ok((Ok(CodeIndexReconcileOutcomeV1::Noop(_)), _, _)) + ) + && worker_text_generation + .read() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .is_some() + && worker_source_freshness + .ready_without_stat(&worker_project_root, &worker_shutting_down) + { + Self::note_visible_worker_continuation( + &worker_reconcile_in_progress, + &worker_pending_wake, + &worker_wake, + ); + } drop(reconcile_pass.take()); if let Ok((Ok(outcome), _, _)) = &result { // A pass that ran to a terminal outcome proves neither the @@ -2046,12 +2155,8 @@ impl CodeIndexSchedulerRegistryV1 { ); } worker_serving_generation_changed.send_replace(()); - // The retained slice was checked before reconciliation - // renewed this proof. Preserve its wake now that source - // is current, without requiring another query arrival. - if clone_backfill_waiting_for_source { - Self::note_worker_continuation(&worker_pending_wake, &worker_wake); - } + // The clone-backfill continuation was stamped before + // this pass dropped `reconcile_in_progress`. } } else { // Surface bounded non-terminal failure without new project-path data. @@ -2269,7 +2374,6 @@ impl CodeIndexSchedulerRegistryV1 { PublishedTextProjectionOutcomeV1::Unfinished } }; - drop(reconcile_pass.take()); match outcome { PublishedTextProjectionOutcomeV1::Finished if !retained_head_recovered_without_complete_replay @@ -2316,6 +2420,9 @@ impl CodeIndexSchedulerRegistryV1 { Self::note_worker_continuation(&worker_pending_wake, &worker_wake); } } + // The continuation is already in the slot. Dropping here + // is the first moment this pass looks idle. + drop(reconcile_pass.take()); } if worker_shutting_down.load(Ordering::Acquire) { tracing::info!( diff --git a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/tests/mod.rs b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/tests/mod.rs index c82155ff9b..8cf50ea13c 100644 --- a/crates/tracedecay-code-index-runtime/src/code_index_scheduler/tests/mod.rs +++ b/crates/tracedecay-code-index-runtime/src/code_index_scheduler/tests/mod.rs @@ -1338,14 +1338,11 @@ async fn settled_owner_with_idle_admission( /// done disturbing it. /// /// The caller must already hold the single background admission, so no further -/// pass can start. One pass can still be finishing: the worker releases that -/// admission halfway through its body and drops its `reconcile_pass` guard -/// before the branches that call `note_worker_continuation`, so both -/// `reconcile_in_progress` and the slot read quiet while the tail is still -/// about to stamp `BusyFollowUp` into it. [`wait_for_settled_owner`] samples -/// exactly those two, so it cannot see that tail. With the admission held the -/// tail is finite and unrepeatable, so clearing until the slot survives a quiet -/// window is the proof the settle cannot give. +/// pass can start. A pass stamps `BusyFollowUp` before it drops +/// `reconcile_in_progress`, but a notify already banked by that pass can still +/// be claimed the moment the permit is released. Clearing until the slot +/// survives a quiet window is the proof the settle cannot give once that +/// release is the next thing that happens. async fn clear_pending_wake_until_quiet( registry: &CodeIndexSchedulerRegistryV1, scope: &tracedecay_contracts::ResolvedScope, From de5f920d76cda1930851036920a3972d651e7c57 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 06:38:41 +0000 Subject: [PATCH 14/17] fix(mcp): attribute field sites by byte span, not line `tracedecay_field_sites` picked each site's `enclosing` declaration by filtering the file's symbols to those whose line range covers the site's line and taking the smallest `line_span`. When several declarations share one line every candidate has span 1, so `min_by_key` broke the tie on the order the graph page yielded symbols, and pages arrive in occurrence order over per-project digests: a coin flip per run, the same defect 63c0784994 fixed in `tracedecay_unsafe_patterns`. Resolve `enclosing` by the byte range the graph already publishes in `CodeGraphSymbolBindingV1::source_span` containing the site's own byte offset, innermost containing span winning. `FieldSite` already carried that offset (`byte`, the end of the field name, keyed the same way as the receiver-type map), and masking preserves byte layout, so no matcher change was needed. `enclosing_declaration` moves from `unsafe_patterns` up to the shared `analysis` module and now returns the symbol rather than its name, since the field-site qualifier check needs the occurrence id. Both handlers then apply one attribution rule instead of two copies that can drift. Co-Authored-By: Claude Fable 5.1 (cherry picked from commit 71522df546dae3b7cff729d60988b2f7bb13b750) --- .../src/handlers/analysis/field_sites.rs | 89 +++++++++++++++++-- .../src/handlers/analysis/mod.rs | 19 ++++ .../src/handlers/analysis/unsafe_patterns.rs | 22 +---- 3 files changed, 105 insertions(+), 25 deletions(-) diff --git a/crates/tracedecay-mcp/src/handlers/analysis/field_sites.rs b/crates/tracedecay-mcp/src/handlers/analysis/field_sites.rs index 99d43cfcc3..fa15cd71fc 100644 --- a/crates/tracedecay-mcp/src/handlers/analysis/field_sites.rs +++ b/crates/tracedecay-mcp/src/handlers/analysis/field_sites.rs @@ -91,13 +91,12 @@ pub async fn handle_field_sites( for site in sites { let line_text = line_at(&source, site.byte).unwrap_or(""); - let enclosing = nodes - .iter() - .filter(|n| { - let line = site.line.saturating_sub(1); - n.metadata.start_line <= line && line <= n.end_line() - }) - .min_by_key(|n| n.metadata.line_span); + // Attribute by byte containment: a read and a write of the + // same field can share one line, and two declarations can + // too, so a line number cannot say which declaration a + // site is inside. Masking preserves byte layout, so the + // offset the scan reports indexes `source` unchanged. + let enclosing = enclosing_declaration(nodes, site.byte as u64); if let Some(scope) = &qualified_scope { if !scope.target_exists { continue; @@ -648,3 +647,79 @@ fn line_is_comment(source: &str, byte: usize) -> bool { let trimmed = line.trim_start(); trimmed.starts_with("//") } + +#[cfg(test)] +mod field_site_attribution_tests { + use super::*; + use tracedecay_domain::{ComplexityAnalysisV1, SourceSpan}; + + fn digest(byte: char) -> T + where + T: TryFrom, + >::Error: std::fmt::Debug, + { + T::try_from(format!("sha256:{}", byte.to_string().repeat(64))).expect("digest") + } + + fn declaration(name: &str, span: std::ops::Range) -> VerifiedAnalysisSymbol { + VerifiedAnalysisSymbol { + occurrence: SymbolOccurrenceId::new(format!("occurrence.{name}")).expect("occurrence"), + path: "src/lib.rs".to_owned(), + source_span: Some(SourceSpan { + start_byte: span.start as u64, + end_byte: span.end as u64, + }), + metadata: LineageSymbolRecordV1 { + occurrence: SymbolOccurrenceId::new(format!("occurrence.{name}")) + .expect("occurrence"), + identity: digest('1'), + qualified_name: name.to_owned(), + simple_name: name.to_owned(), + kind: "function".to_owned(), + visibility: "private".to_owned(), + branches: 0, + loops: 0, + max_nesting: 0, + complexity_analysis: ComplexityAnalysisV1::Complete, + // Both declarations live on line 1: the shape that made + // line-based attribution a coin flip. + line_span: 1, + start_line: 0, + signature: None, + docstring: None, + is_async: false, + derives: Vec::new(), + skip_test_coverage: false, + file_identity: digest('2'), + content_digest: digest('3'), + }, + } + } + + /// A read and a write of one field, inside two functions that share a + /// line, each belong to the function whose bytes contain them. Every + /// candidate has `line_span == 1` here, so the old smallest-line-span + /// selection had nothing to break the tie with and returned whichever + /// symbol the graph page happened to yield first. + #[test] + fn attributes_a_read_and_a_write_sharing_one_line() { + let source = "fn r(s: &S) -> u32 { s.count } fn w(s: &mut S) { s.count = 1; }"; + let write_start = source.find("fn w").expect("second function"); + let nodes = vec![ + declaration("r", 0..write_start), + declaration("w", write_start..source.len()), + ]; + + let sites = find_field_references(source, "count"); + assert_eq!(sites.len(), 2, "one read and one write: {sites:?}"); + assert!(matches!(sites[0].kind, FieldRefKind::Read)); + assert!(matches!(sites[1].kind, FieldRefKind::Write)); + assert_eq!(sites[0].line, sites[1].line, "both sites share one line"); + + for (site, expected) in sites.iter().zip(["r", "w"]) { + let enclosing = enclosing_declaration(&nodes, site.byte as u64) + .map(|node| node.metadata.qualified_name.as_str()); + assert_eq!(enclosing, Some(expected), "site {site:?}"); + } + } +} diff --git a/crates/tracedecay-mcp/src/handlers/analysis/mod.rs b/crates/tracedecay-mcp/src/handlers/analysis/mod.rs index d85bbfcd53..dcd9a84d82 100644 --- a/crates/tracedecay-mcp/src/handlers/analysis/mod.rs +++ b/crates/tracedecay-mcp/src/handlers/analysis/mod.rs @@ -77,6 +77,25 @@ impl VerifiedAnalysisSymbol { } } +/// Innermost declaration whose source range covers `match_byte`. +/// +/// Byte containment, not line containment: an attribute such as `#[test]` and +/// the two functions on `#[test] fn a() {…} fn b() {…}` all sit on one line, +/// and only the byte range says which of them the site is inside. Selecting by +/// line also had no stable order to break ties with, since symbols arrive in +/// occurrence order and occurrence ids are per-project digests. +fn enclosing_declaration( + nodes: &[VerifiedAnalysisSymbol], + match_byte: u64, +) -> Option<&VerifiedAnalysisSymbol> { + nodes + .iter() + .filter_map(|node| node.source_span.map(|span| (node, span))) + .filter(|(_, span)| span.start_byte <= match_byte && match_byte < span.end_byte) + .min_by_key(|(_, span)| span.end_byte.saturating_sub(span.start_byte)) + .map(|(node, _)| node) +} + fn verified_analysis_symbols( graph: &VerifiedGraphQuery, scope_prefix: Option<&str>, diff --git a/crates/tracedecay-mcp/src/handlers/analysis/unsafe_patterns.rs b/crates/tracedecay-mcp/src/handlers/analysis/unsafe_patterns.rs index b1dc182a08..c0d587c1e8 100644 --- a/crates/tracedecay-mcp/src/handlers/analysis/unsafe_patterns.rs +++ b/crates/tracedecay-mcp/src/handlers/analysis/unsafe_patterns.rs @@ -8,7 +8,8 @@ use tracedecay_domain::errors::{Result, TraceDecayError}; use tracedecay_graph_query::VerifiedGraphQuery; use super::{ - VerifiedAnalysisSymbol, path_is_rust, verified_analysis_symbols, verified_analysis_unavailable, + VerifiedAnalysisSymbol, enclosing_declaration, path_is_rust, verified_analysis_symbols, + verified_analysis_unavailable, }; use crate::ToolResult; use crate::handlers::support::{effective_path, rendered_tool_result}; @@ -110,22 +111,6 @@ fn contains_unsafe_block_start(line: &str) -> Option { None } -/// Innermost declaration whose source range covers `match_byte`. -/// -/// Byte containment, not line containment: an attribute such as `#[test]` and -/// the two functions on `#[test] fn a() {…} fn b() {…}` all sit on one line, -/// and only the byte range says which of them the site is inside. Selecting by -/// line also had no stable order to break ties with, since symbols arrive in -/// occurrence order and occurrence ids are per-project digests. -fn enclosing_declaration(nodes: &[VerifiedAnalysisSymbol], match_byte: u64) -> Option { - nodes - .iter() - .filter_map(|node| node.source_span.map(|span| (node, span))) - .filter(|(_, span)| span.start_byte <= match_byte && match_byte < span.end_byte) - .min_by_key(|(_, span)| span.end_byte.saturating_sub(span.start_byte)) - .map(|(node, _)| node.metadata.qualified_name.clone()) -} - fn path_looks_like_test(path: &str) -> bool { path.starts_with("tests/") || path.contains("/tests/") @@ -258,7 +243,8 @@ pub async fn handle_unsafe_patterns( if let Some(column) = line_matches_unsafe_kind(masked_line, kind) { let nodes = symbols_by_file.get(file).map_or(&[][..], Vec::as_slice); let enclosing = - enclosing_declaration(nodes, (line_offset + column) as u64); + enclosing_declaration(nodes, (line_offset + column) as u64) + .map(|node| node.metadata.qualified_name.clone()); *by_kind.entry(kind.clone()).or_insert(0) += 1; matches.push(json!({ "kind": kind, From aab865a6a4dfb42edb1c35a2bfa2ec8bd7a305d7 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 06:44:53 +0000 Subject: [PATCH 15/17] test(runtime): settle the worker before sampling elapsed freshness `elapsed_freshness_window_alone_does_not_make_dashboard_state_stale` read the dashboard right after `wait_for_dashboard_ready`, which joins only the running pass. The mount leaves clone backfill behind, and the wakes that drain it leave a banked permit whose no-op pass projects `Verifying` instead of `Fresh` at the sample (CI run 35425541839, both tries). Use the single-permit registry, settle the mount-era chain, hold the admission so no pass can start, and prove the pending-wake slot stays empty, as the text-progress test already does. Co-Authored-By: Claude Fable 5.1 --- .../code_index_scheduler/tests/reconcile.rs | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) 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 138157469d..0ac4e3a71f 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 @@ -4022,7 +4022,9 @@ async fn diagnostics_change_generation_advances_for_out_of_band_git_drift() { async fn elapsed_freshness_window_alone_does_not_make_dashboard_state_stale() { let fixture = GitFixture::new(&[("src/main.rs", "fn main() {}\n")]); let store = TempDir::new().expect("store root"); - let registry = CodeIndexSchedulerRegistryV1::new(1); + // Single-permit admission: holding it below parks the background worker, + // which the host's default bound cannot do. + let registry = CodeIndexSchedulerRegistryV1::with_background_reconcile_permits(1, 1); registry .mount_worktree( test_project_id(), @@ -4033,18 +4035,33 @@ async fn elapsed_freshness_window_alone_does_not_make_dashboard_state_stale() { .expect("mount daemon-owned scheduler"); wait_for_initial_generation(®istry, fixture.path()).await; wait_for_dashboard_ready(®istry, fixture.path()).await; + // The mount leaves clone backfill behind, and the wakes that drain it + // leave a banked permit whose no-op pass projects `Verifying` instead of + // `Fresh` (CI run 35425541839). Settle the mount-era chain, hold the + // admission so no pass can start under the sample, and prove the + // pending-wake slot stays empty, exactly as the text-progress test does. + drain_clone_backfill(®istry, fixture.path()).await; + settled_owner_with_idle_admission(®istry, fixture.path()).await; + let _quiet_owner = quiesced_background_reconcile_admission(®istry, fixture.path()).await; let canonical = fixture.path().canonicalize().expect("canonical fixture"); - { + let scope = { let mounted = registry.mounted.lock().await; - mounted - .get(&canonical) - .expect("mounted worktree") + let worktree = mounted.get(&canonical).expect("mounted worktree"); + worktree .scheduler .lock() .expect("scheduler") .policy .staleness_threshold = Duration::ZERO; - } + tracedecay_contracts::ResolvedScope::new( + test_project_id(), + worktree.repository_id.clone(), + worktree.worktree_id.clone(), + None, + ) + .expect("resolved scope") + }; + clear_pending_wake_until_quiet(®istry, &scope).await; let projected = registry .dashboard_freshness(fixture.path()) From 15f25b34ddec8e6c17fc3600c2a8fc8125a38392 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 06:44:53 +0000 Subject: [PATCH 16/17] test(cli): keep the hotpath metrics port out of the quiet-pipeline test Under `--all-features` the binary binds the hotpath metrics port on start. When a sibling test's daemon already holds it, the bind error lands on stderr and `shipped_binary_stops_quietly_when_a_pipeline_reader_exits` fails on its empty-stderr assertion. Turn the metrics server off for this command, as `hotpath_command` already does. Co-Authored-By: Claude Fable 5.1 --- crates/tracedecay-cli/tests/core_cli_suite/cli_boundary.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/tracedecay-cli/tests/core_cli_suite/cli_boundary.rs b/crates/tracedecay-cli/tests/core_cli_suite/cli_boundary.rs index 4df4e34dd5..827cdaece2 100644 --- a/crates/tracedecay-cli/tests/core_cli_suite/cli_boundary.rs +++ b/crates/tracedecay-cli/tests/core_cli_suite/cli_boundary.rs @@ -17,6 +17,10 @@ fn shipped_binary_stops_quietly_when_a_pipeline_reader_exits() { let output = Command::new("sh") .args(["-c", r#""$TRACEDECAY_BIN" tool | head -n 4"#]) .env("TRACEDECAY_BIN", env!("CARGO_BIN_EXE_tracedecay")) + // A hotpath-enabled binary binds its metrics port on start; when a + // sibling test's daemon already holds it, the bind failure lands on + // stderr and breaks the quiet-pipeline assertion below. + .env("HOTPATH_METRICS_SERVER_OFF", "true") .output() .expect("tracedecay tool pipeline should run"); From c534d03cf9be71f979bdec597cbf2692f1d16ec4 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 07:37:07 +0000 Subject: [PATCH 17/17] fix(code-index): keep admission ahead of the publication gate The published text projection re-acquired the background admission permit before renaming the active pointer, while `_build_publication` was still held for the rest of the worker iteration. `run_ignored_dependency_admission` takes the same admission *before* that per-worktree gate, so the two orders invert: an ignored-dependency owner holds the permit and waits for the gate, the worker holds the gate and waits for the permit. That stalls the publication pass until the dependency request hits its own deadline and refuses, and with every permit consumed it is a cycle. The repository already states the invariant the re-acquire broke: `background_worker_waits_for_global_admission_before_publication_gate` ("global admission wait must not hold the per-worktree publication gate"). The re-acquire also bought nothing for this PR's claim. `reconcile_pass` is already held across the whole published text projection, so the pointer rename was inside the pass an idle reader samples; dropping the pass across the new permit wait instead opened a fresh window where `reconcile_in_progress` reads zero before the rename has run. Drop the re-acquire and keep the pass guard continuous, which is what makes "idle means the pass tail already ran" true here. The continuation-ordering half of the change is untouched: stamps still land before the pass goes idle. Co-Authored-By: Claude Fable 5.1 --- .../code_index_scheduler/registry/mount.rs | 65 +++++-------------- 1 file changed, 15 insertions(+), 50 deletions(-) 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 f488bde488..b6f9c306d2 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 @@ -1031,13 +1031,15 @@ impl CodeIndexSchedulerRegistryV1 { started_micros, ); } - // Source reconciliation is complete. Release the admission - // permit only across HeadOpening's scheduler-mutex wait: a - // holder of that mutex must be able to run, and an - // ignored-dependency owner still needs this permit before it - // can take the mutex. The publication's text projection - // re-acquires the permit before it renames the active pointer. - // Keep `reconcile_pass` through text seating; dropping it + // Source reconciliation is complete: release the background + // admission permit before HeadOpening / graph work so sibling + // stores can start. The permit is never re-acquired inside + // this pass: `_build_publication` is held for the rest of the + // iteration, and `run_ignored_dependency_admission` takes the + // admission *before* that same gate, so waiting on admission + // here would invert that order (see + // `background_worker_waits_for_global_admission_before_publication_gate`). + // Keep `reconcile_pass` through text seating, dropping it // made `reconcile_in_progress` lie while this worker still // owned graph try_lock, which deadlocked tests that hold the // scheduler mutex and wait for that flag. @@ -1114,49 +1116,12 @@ impl CodeIndexSchedulerRegistryV1 { && !graph_activation_deferred && let Some(text) = graph_text.clone() { - // Head opening released the permit so it could wait on - // the scheduler mutex. Take it back for the pointer - // rename. Drop the pass across that wait: a caller - // holding the permit and waiting for the pass would - // otherwise deadlock, and the pass is re-entered - // before the rename so idle still means the pointer - // write has finished. - let resume_pass = reconcile_pass.is_some(); - drop(reconcile_pass.take()); - let Ok(_text_artifact_admission) = hotpath::future!( - Arc::clone(&worker_background_reconcile_admission).acquire_owned(), - label = "daemon.code_index.admission_wait" - ) - .await - else { - tracing::info!( - event = "code_index_worker_shutdown_observed", - phase = "published_text_projection", - "code-index worker observed shutdown and stopped its pass" - ); - Self::join_retained_text_projection_on_worker_exit( - &mut retained_text_projection, - ) - .await; - return; - }; - if worker_shutting_down.load(Ordering::Acquire) { - tracing::info!( - event = "code_index_worker_shutdown_observed", - phase = "published_text_projection", - "code-index worker observed shutdown and stopped its pass" - ); - Self::join_retained_text_projection_on_worker_exit( - &mut retained_text_projection, - ) - .await; - return; - } - if resume_pass { - reconcile_pass = Some(super::super::ReconcilePassGuard::enter( - &worker_reconcile_in_progress, - )); - } + // `reconcile_pass` is held across this projection, so + // the pointer rename is inside the pass a reader + // samples. Taking the admission permit back here + // instead would deadlock against an + // ignored-dependency owner that already holds it and + // is waiting for `_build_publication`. let projection = tokio::spawn(Self::drive_text_projection( text, Arc::clone(&worker_shutting_down),