From 1ee84d3bf0c16f27046b51053ab946e828d6793b Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 19 Sep 2026 07:09:55 +0000 Subject: [PATCH 1/3] fix(admission): make NotApplicable a terminal no-op Closed NotApplicable already finishes the replay record, but the shared pass classifier still backed off on it and the MCP hook boundary still published it as a JSON-RPC failure. Stop that pass without retry or a failure log, and return a successful no-op on the wire. Co-authored-by: Zack Jackson --- CHANGELOG.md | 4 + .../src/profile_host_admission_replay.rs | 41 ++++++++++ .../tracedecay-host-admission/src/replay.rs | 60 +++++++++++++- .../server/project_host_admission_replay.rs | 40 ++++++++++ crates/tracedecay-mcp/src/tool_errors.rs | 78 ++++++++++++++++++- 5 files changed, 221 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc4448d9bf..99bbe0890d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1710,6 +1710,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- *(admission)* `NotApplicable` is a terminal no-op. Replay no longer backs + off on that closed status, and the MCP hook-runtime boundary returns a + successful no-op instead of a JSON-RPC failure. + - *(code-index)* the background worker consults the typed publication-authority park instead of a loop-local bool, so a park it has not yet observed still stops reconcile. Branch publication handles `NotApplicable` as a closed diff --git a/crates/tracedecay-daemon-service/src/profile_host_admission_replay.rs b/crates/tracedecay-daemon-service/src/profile_host_admission_replay.rs index f8d79e2cd4..255e0622ef 100644 --- a/crates/tracedecay-daemon-service/src/profile_host_admission_replay.rs +++ b/crates/tracedecay-daemon-service/src/profile_host_admission_replay.rs @@ -841,6 +841,10 @@ impl ProfileHostAdmissionReplayWorker { // Non-retryable failure: stop until the next explicit kick. break; } + ReplayPassDecision::TerminalNoop => { + consecutive_retryable = 0; + break; + } ReplayPassDecision::Requeue => { consecutive_retryable = 0; } @@ -1391,6 +1395,43 @@ mod tests { registry.shutdown().await; } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn not_applicable_pending_record_does_not_back_off() { + let temp = tempfile::TempDir::new().unwrap(); + let profile_root = temp.path().join("profile"); + std::fs::create_dir_all(&profile_root).unwrap(); + let db_path = tracedecay_sessions::runtime::user_sessions_db_path(&profile_root); + let (runtime, _) = + tracedecay_host_admission::HostAdmissionRuntime::open_for_database(&db_path).unwrap(); + let broker = Arc::new(tracedecay_host_admission::HostAdmissionBroker::new(runtime)); + broker.admit("test:pending", b"pending").await.unwrap(); + let registry = ProfileHostAdmissionReplayRegistry::default(); + let pass_override = Arc::new(|| { + Box::pin(async { HostAdmissionOutcome::not_applicable("code_index_not_applicable") }) + as std::pin::Pin + Send>> + }); + + registry + .ensure_with_pass_override(&db_path, &profile_root, &broker, pass_override) + .await; + tokio::time::timeout(Duration::from_secs(1), async { + while registry.pass_count(&db_path).await == 0 { + tokio::task::yield_now().await; + } + }) + .await + .expect("profile replay must attempt the not-applicable record"); + tokio::time::sleep(Duration::from_millis(150)).await; + let passes = registry.pass_count(&db_path).await; + assert!(passes >= 1); + assert_eq!(registry.backoff_count(&db_path).await, 0); + tokio::time::sleep(Duration::from_millis(150)).await; + + assert_eq!(registry.pass_count(&db_path).await, passes); + assert_eq!(registry.backoff_count(&db_path).await, 0); + registry.shutdown().await; + } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn shutdown_cancels_and_joins_an_in_flight_pass() { let temp = tempfile::TempDir::new().unwrap(); diff --git a/crates/tracedecay-host-admission/src/replay.rs b/crates/tracedecay-host-admission/src/replay.rs index ceffe51442..b9b1588c39 100644 --- a/crates/tracedecay-host-admission/src/replay.rs +++ b/crates/tracedecay-host-admission/src/replay.rs @@ -7,7 +7,7 @@ use std::time::Duration; -use tracedecay_sessions::admission::HostAdmissionOutcome; +use tracedecay_sessions::admission::{HostAdmissionOutcome, HostAdmissionStatus}; const MAX_BACKOFF: Duration = Duration::from_secs(2); const INITIAL_BACKOFF: Duration = Duration::from_millis(25); @@ -28,6 +28,7 @@ pub fn replay_backoff(attempt: u32, shift_cap: u32) -> Duration { } /// How a worker should proceed after one replay pass. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum ReplayPassDecision { /// The spool shrank and more work remains. Yield and re-run immediately. ProgressPending, @@ -37,15 +38,33 @@ pub enum ReplayPassDecision { Stop, /// Re-evaluate the work condition without backoff. Requeue, + /// Closed `NotApplicable` left the spool unchanged. Stop until the next + /// kick without backoff and without a failure log. + TerminalNoop, } /// Classify one replay pass from its pending-count delta and outcome. +/// +/// `NotApplicable` already closes the replay record. It must not enter the +/// retryable backoff arm, even when `is_replay_progress` is true and the +/// spool did not shrink: that arm is for work that may succeed later. A shrink +/// with records still pending continues immediately; a drained spool requeues; +/// an unchanged spool stops until the next external kick. pub fn classify_replay_pass( pending_before: usize, pending_after: usize, outcome: &HostAdmissionOutcome, ) -> ReplayPassDecision { let made_progress = pending_after < pending_before; + if outcome.status == HostAdmissionStatus::NotApplicable { + if made_progress && pending_after > 0 { + return ReplayPassDecision::ProgressPending; + } + if pending_after == 0 { + return ReplayPassDecision::Requeue; + } + return ReplayPassDecision::TerminalNoop; + } if made_progress && pending_after > 0 { ReplayPassDecision::ProgressPending } else if !made_progress @@ -58,3 +77,42 @@ pub fn classify_replay_pass( ReplayPassDecision::Requeue } } + +#[cfg(test)] +mod tests { + use super::{ReplayPassDecision, classify_replay_pass}; + use tracedecay_sessions::admission::HostAdmissionOutcome; + + #[test] + fn not_applicable_is_a_terminal_noop_unless_the_spool_actually_moves() { + let closed = HostAdmissionOutcome::not_applicable("code_index_not_applicable"); + let mut flagged_retryable = closed.clone(); + flagged_retryable.retryable = true; + + assert_eq!( + classify_replay_pass(2, 2, &closed), + ReplayPassDecision::TerminalNoop + ); + assert_eq!( + classify_replay_pass(2, 3, &flagged_retryable), + ReplayPassDecision::TerminalNoop + ); + assert_eq!( + classify_replay_pass(2, 1, &closed), + ReplayPassDecision::ProgressPending + ); + assert_eq!( + classify_replay_pass(1, 0, &closed), + ReplayPassDecision::Requeue + ); + + assert_eq!( + classify_replay_pass(2, 2, &HostAdmissionOutcome::accepted_for_replay()), + ReplayPassDecision::Backoff + ); + assert_eq!( + classify_replay_pass(2, 2, &HostAdmissionOutcome::spool_corrupted()), + ReplayPassDecision::Stop + ); + } +} diff --git a/crates/tracedecay-mcp/src/server/project_host_admission_replay.rs b/crates/tracedecay-mcp/src/server/project_host_admission_replay.rs index 6d4f8c5a1d..851e75b05a 100644 --- a/crates/tracedecay-mcp/src/server/project_host_admission_replay.rs +++ b/crates/tracedecay-mcp/src/server/project_host_admission_replay.rs @@ -203,6 +203,10 @@ impl ProjectHostAdmissionReplayWorker { ); break; } + ReplayPassDecision::TerminalNoop => { + consecutive_retryable = 0; + break; + } ReplayPassDecision::Requeue => { consecutive_retryable = 0; if self.dirty.load(Ordering::Acquire) || pending_after > 0 { @@ -275,6 +279,42 @@ mod tests { task.shutdown().await; } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn not_applicable_pending_record_stops_without_backoff() { + let temp = tempfile::TempDir::new().unwrap(); + let (runtime, _) = tracedecay_host_admission::HostAdmissionRuntime::open( + temp.path(), + tracedecay_host_admission::SpoolBounds::default(), + ) + .unwrap(); + let broker = Arc::new(tracedecay_host_admission::HostAdmissionBroker::new(runtime)); + broker.admit("test:pending", b"pending").await.unwrap(); + let passes = Arc::new(AtomicUsize::new(0)); + let passes_for_run = Arc::clone(&passes); + let pass: PassFn = Arc::new(move || { + let passes = Arc::clone(&passes_for_run); + Box::pin(async move { + passes.fetch_add(1, Ordering::AcqRel); + HostAdmissionOutcome::not_applicable("code_index_not_applicable") + }) + }); + let task = ProjectHostAdmissionReplayTask::start(broker, pass); + + tokio::time::timeout(Duration::from_secs(1), async { + while task.pass_count() == 0 { + tokio::task::yield_now().await; + } + }) + .await + .expect("project replay must attempt the not-applicable record"); + tokio::time::sleep(Duration::from_millis(100)).await; + + assert_eq!(passes.load(Ordering::Acquire), 1); + assert_eq!(task.pass_count(), 1); + assert_eq!(task.backoff_count(), 0); + task.shutdown().await; + } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn dropping_task_aborts_an_in_flight_pass_without_an_arc_cycle() { let temp = tempfile::TempDir::new().unwrap(); diff --git a/crates/tracedecay-mcp/src/tool_errors.rs b/crates/tracedecay-mcp/src/tool_errors.rs index 758792e860..e0ab8cd06c 100644 --- a/crates/tracedecay-mcp/src/tool_errors.rs +++ b/crates/tracedecay-mcp/src/tool_errors.rs @@ -44,6 +44,28 @@ fn value_has_semantic_error(value: &Value) -> bool { }) } +/// Successful MCP tool result for a closed `NotApplicable` admission. +/// +/// The status already means the demand does not apply. Publishing it as a +/// JSON-RPC error makes hosts retry a terminal no-op. +fn hook_runtime_not_applicable_noop(error: &TraceDecayError) -> Option { + let status = error.hook_runtime_status()?; + if HostAdmissionStatus::from_wire(status) != Some(HostAdmissionStatus::NotApplicable) { + return None; + } + let (reason_code, _, _) = error.hook_runtime_context()?; + let body = json!({ + "tool": "tracedecay_hook_runtime", + "status": status, + "reason_code": reason_code, + "retryable": false, + }); + Some(json!({ + "content": [{ "type": "text", "text": body.to_string() }], + "isError": false, + })) +} + /// Projects a hook-runtime error onto the structured JSON-RPC data object. /// /// The status is whatever the admission authority reported, carried through @@ -169,6 +191,11 @@ pub fn tool_error_response(id: Value, tool_name: &str, error: &TraceDecayError) Some(data), ); } + if tool_name == "tracedecay_hook_runtime" + && let Some(result) = hook_runtime_not_applicable_noop(error) + { + return JsonRpcResponse::success(id, result); + } if tool_name == "tracedecay_hook_runtime" && let Some(data) = structured_hook_error_data(error) { @@ -342,7 +369,7 @@ pub fn serialize_response_line(resp: &JsonRpcResponse) -> String { #[cfg(test)] mod tests { - use serde_json::json; + use serde_json::{Value, json}; use tracedecay_domain::errors::TraceDecayError; use super::tool_error_response; @@ -367,4 +394,53 @@ mod tests { "application_surface_invalid_request" ); } + + #[test] + fn not_applicable_hook_runtime_is_a_successful_noop_and_other_statuses_stay_errors() { + use tracedecay_sessions::admission::HostAdmissionStatus; + + let closed = TraceDecayError::hook_runtime_with_status( + "code_index_not_applicable", + true, + "projectless Hermes receipt host admission failed", + HostAdmissionStatus::NotApplicable.as_wire(), + ); + let wire = serde_json::to_value(tool_error_response( + json!(4), + "tracedecay_hook_runtime", + &closed, + )) + .expect("JSON-RPC wire response"); + + assert_eq!(wire["jsonrpc"], "2.0"); + assert_eq!(wire["id"], 4); + assert!(wire.get("error").is_none()); + assert_eq!(wire["result"]["isError"], false); + let text = wire["result"]["content"][0]["text"] + .as_str() + .expect("tool text"); + let body: Value = serde_json::from_str(text).expect("noop body"); + assert_eq!(body["tool"], "tracedecay_hook_runtime"); + assert_eq!(body["status"], "not_applicable"); + assert_eq!(body["reason_code"], "code_index_not_applicable"); + assert_eq!(body["retryable"], false); + assert!(body.get("error").is_none()); + assert!(!text.contains("failed")); + + let unavailable = TraceDecayError::hook_runtime_with_status( + "authority_unavailable", + true, + "host admission authority is unavailable", + HostAdmissionStatus::Unavailable.as_wire(), + ); + let failure = serde_json::to_value(tool_error_response( + json!(5), + "tracedecay_hook_runtime", + &unavailable, + )) + .expect("JSON-RPC wire response"); + assert!(failure.get("result").is_none()); + assert_eq!(failure["error"]["code"], -32603); + assert_eq!(failure["error"]["data"]["status"], "unavailable"); + } } From 1196ebad4fe9772a756ffe6eeb76cd17c3164a9c Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 08:22:00 +0000 Subject: [PATCH 2/3] style(session-temporal): keep the test module last and rustfmt master Master run 35431539771 failed Check formatting (projector.rs, query.rs) and Clippy (items_after_test_module in query.rs) after #1844/#1845 merged without CI. Co-Authored-By: Claude Fable 5.1 --- .../projector.rs | 8 ++--- .../src/query.rs | 29 +++++++++---------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/projector.rs b/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/projector.rs index 10818733c1..632898ca24 100644 --- a/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/projector.rs +++ b/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/projector.rs @@ -119,11 +119,11 @@ impl SessionTemporalRefreshProjector for CanonicalSessionTemporalProjector { // Empty remaining range is a durable no-op: terminalize with an // empty complete progress batch instead of deferring forever. Ok(None) => canonical_noop_complete_effect(&recovery), - Err(error) if error.is_storage() => Err( - SessionTemporalRefreshProjectorError::retryable(format!( + Err(error) if error.is_storage() => { + Err(SessionTemporalRefreshProjectorError::retryable(format!( "source_busy: {error}" - )), - ), + ))) + } Err(_) => Err(SessionTemporalRefreshProjectorError::terminal( "projector_failed", )), diff --git a/crates/tracedecay-session-temporal-store/src/query.rs b/crates/tracedecay-session-temporal-store/src/query.rs index 367b14bb79..ae0ab6b4de 100644 --- a/crates/tracedecay-session-temporal-store/src/query.rs +++ b/crates/tracedecay-session-temporal-store/src/query.rs @@ -246,8 +246,7 @@ pub(super) async fn read_observations( // ceiling. Split until a single observation remains; that // observation is then a typed storage failure, not a retry // that looks like a busy source. - if observation_prefetch_exceeded_materialization_limit(&error) && chunk.len() > 1 - { + if observation_prefetch_exceeded_materialization_limit(&error) && chunk.len() > 1 { let mid = start + chunk.len() / 2; pending.push((mid, end)); pending.push((start, mid)); @@ -289,11 +288,18 @@ fn observation_prefetch_exceeded_materialization_limit(error: &SessionStoreError } } +/// The error `read_observation` raises for an id the store does not hold, reused +/// by callers that resolve prefetched observations out of a batch map. +pub(super) fn missing_observation(observation_id: &CanonicalObservationIdV1) -> SessionStoreError { + storage_message( + PERSIST_OPERATION, + format!("source observation {} is missing", observation_id.as_str()), + ) +} + #[cfg(test)] mod tests { - use super::{ - PERSIST_OPERATION, observation_prefetch_exceeded_materialization_limit, storage, - }; + use super::{PERSIST_OPERATION, observation_prefetch_exceeded_materialization_limit, storage}; #[test] fn materialization_limit_is_the_prefetch_split_signal() { @@ -308,15 +314,8 @@ mod tests { assert!(observation_prefetch_exceeded_materialization_limit( &exceeded )); - assert!(!observation_prefetch_exceeded_materialization_limit(&locked)); + assert!(!observation_prefetch_exceeded_materialization_limit( + &locked + )); } } - -/// The error `read_observation` raises for an id the store does not hold, reused -/// by callers that resolve prefetched observations out of a batch map. -pub(super) fn missing_observation(observation_id: &CanonicalObservationIdV1) -> SessionStoreError { - storage_message( - PERSIST_OPERATION, - format!("source observation {} is missing", observation_id.as_str()), - ) -} From c30cc2d6dcbea0d0ec5ad8ad13ee84d3f0905bb8 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 09:38:40 +0000 Subject: [PATCH 3/3] fix(mcp): drop the unreachable NotApplicable wire no-op The hook-runtime error boundary gained a branch that turned a `not_applicable` admission status into a successful JSON-RPC result, but no production path can produce that error. The only production constructor of `HostAdmissionStatus::NotApplicable` is `code_index_host_outcome`; its outcome reaches either the hook-event notification path (which returns no JSON-RPC response at all) or a replay worker pass, and neither converts an outcome into a `TraceDecayError`. The one hook-runtime call site that maps an outcome to an error, `hermes.rs`, is gated on `!commits_replay_record()`, and `NotApplicable` commits. Removing the branch and its synthetic test leaves the reachable half of the change: `classify_replay_pass` treating a closed `NotApplicable` that did not shrink the spool as `TerminalNoop`. Resolves the Codex P1 review finding on #1843 and corrects the CHANGELOG entry, which claimed a wire behaviour change that does not occur. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 6 +- crates/tracedecay-mcp/src/tool_errors.rs | 78 +----------------------- 2 files changed, 4 insertions(+), 80 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99bbe0890d..7c42d0b1c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1710,9 +1710,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- *(admission)* `NotApplicable` is a terminal no-op. Replay no longer backs - off on that closed status, and the MCP hook-runtime boundary returns a - successful no-op instead of a JSON-RPC failure. +- *(admission)* `NotApplicable` is a terminal no-op in the shared replay-pass + decision. A closed status that leaves the spool unchanged now stops until + the next kick instead of entering the retryable backoff arm. - *(code-index)* the background worker consults the typed publication-authority park instead of a loop-local bool, so a park it has not yet observed still diff --git a/crates/tracedecay-mcp/src/tool_errors.rs b/crates/tracedecay-mcp/src/tool_errors.rs index e0ab8cd06c..758792e860 100644 --- a/crates/tracedecay-mcp/src/tool_errors.rs +++ b/crates/tracedecay-mcp/src/tool_errors.rs @@ -44,28 +44,6 @@ fn value_has_semantic_error(value: &Value) -> bool { }) } -/// Successful MCP tool result for a closed `NotApplicable` admission. -/// -/// The status already means the demand does not apply. Publishing it as a -/// JSON-RPC error makes hosts retry a terminal no-op. -fn hook_runtime_not_applicable_noop(error: &TraceDecayError) -> Option { - let status = error.hook_runtime_status()?; - if HostAdmissionStatus::from_wire(status) != Some(HostAdmissionStatus::NotApplicable) { - return None; - } - let (reason_code, _, _) = error.hook_runtime_context()?; - let body = json!({ - "tool": "tracedecay_hook_runtime", - "status": status, - "reason_code": reason_code, - "retryable": false, - }); - Some(json!({ - "content": [{ "type": "text", "text": body.to_string() }], - "isError": false, - })) -} - /// Projects a hook-runtime error onto the structured JSON-RPC data object. /// /// The status is whatever the admission authority reported, carried through @@ -191,11 +169,6 @@ pub fn tool_error_response(id: Value, tool_name: &str, error: &TraceDecayError) Some(data), ); } - if tool_name == "tracedecay_hook_runtime" - && let Some(result) = hook_runtime_not_applicable_noop(error) - { - return JsonRpcResponse::success(id, result); - } if tool_name == "tracedecay_hook_runtime" && let Some(data) = structured_hook_error_data(error) { @@ -369,7 +342,7 @@ pub fn serialize_response_line(resp: &JsonRpcResponse) -> String { #[cfg(test)] mod tests { - use serde_json::{Value, json}; + use serde_json::json; use tracedecay_domain::errors::TraceDecayError; use super::tool_error_response; @@ -394,53 +367,4 @@ mod tests { "application_surface_invalid_request" ); } - - #[test] - fn not_applicable_hook_runtime_is_a_successful_noop_and_other_statuses_stay_errors() { - use tracedecay_sessions::admission::HostAdmissionStatus; - - let closed = TraceDecayError::hook_runtime_with_status( - "code_index_not_applicable", - true, - "projectless Hermes receipt host admission failed", - HostAdmissionStatus::NotApplicable.as_wire(), - ); - let wire = serde_json::to_value(tool_error_response( - json!(4), - "tracedecay_hook_runtime", - &closed, - )) - .expect("JSON-RPC wire response"); - - assert_eq!(wire["jsonrpc"], "2.0"); - assert_eq!(wire["id"], 4); - assert!(wire.get("error").is_none()); - assert_eq!(wire["result"]["isError"], false); - let text = wire["result"]["content"][0]["text"] - .as_str() - .expect("tool text"); - let body: Value = serde_json::from_str(text).expect("noop body"); - assert_eq!(body["tool"], "tracedecay_hook_runtime"); - assert_eq!(body["status"], "not_applicable"); - assert_eq!(body["reason_code"], "code_index_not_applicable"); - assert_eq!(body["retryable"], false); - assert!(body.get("error").is_none()); - assert!(!text.contains("failed")); - - let unavailable = TraceDecayError::hook_runtime_with_status( - "authority_unavailable", - true, - "host admission authority is unavailable", - HostAdmissionStatus::Unavailable.as_wire(), - ); - let failure = serde_json::to_value(tool_error_response( - json!(5), - "tracedecay_hook_runtime", - &unavailable, - )) - .expect("JSON-RPC wire response"); - assert!(failure.get("result").is_none()); - assert_eq!(failure["error"]["code"], -32603); - assert_eq!(failure["error"]["data"]["status"], "unavailable"); - } }