diff --git a/cli/src/services/hooks/claude_mutation_scope/health.rs b/cli/src/services/hooks/claude_mutation_scope/health.rs new file mode 100644 index 000000000..22f22a4d3 --- /dev/null +++ b/cli/src/services/hooks/claude_mutation_scope/health.rs @@ -0,0 +1,228 @@ +use std::path::Path; + +use crate::services::hooks::mutation_scope_health::{ + MutationScopeAdapterHealth, MutationScopeHealthStatus, +}; +use crate::services::mutation_trace::types::ActorKind; + +use super::state; + +pub(crate) fn classify_health(git_dir: &Path) -> MutationScopeAdapterHealth { + let state = match state::read_state(git_dir) { + Ok(state) => state, + Err(error) => { + return MutationScopeAdapterHealth::new( + ActorKind::ClaudeCode, + MutationScopeHealthStatus::Invalid, + "Claude mutation-scope state file could not be read or parsed.", + ) + .with_detail(error.to_string()); + } + }; + + if !state.recovery_pending { + return MutationScopeAdapterHealth::new( + ActorKind::ClaudeCode, + MutationScopeHealthStatus::Healthy, + "No persisted recovery condition blocks mutation-capable admission.", + ); + } + + if state.attempts.is_empty() { + return MutationScopeAdapterHealth::new( + ActorKind::ClaudeCode, + MutationScopeHealthStatus::Recovering, + "Recovery is pending with no unresolved attempts; the next tracked PreToolUse call flushes and clears it automatically.", + ); + } + + MutationScopeAdapterHealth::new( + ActorKind::ClaudeCode, + MutationScopeHealthStatus::Blocked, + "Recovery is pending with unresolved attempts; the recovery barrier's flush path only runs once attempts are empty, so future tracked PreToolUse calls deny without self-clearing.", + ) +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + use std::sync::atomic::{AtomicU64, Ordering}; + + use anyhow::anyhow; + + use super::super::{abandon_attempt, apply_recovery_barrier, AttemptKey, BarrierOutcome}; + use super::*; + use crate::services::observability::traits::Logger; + + static NEXT_TEST_GIT_DIR_ID: AtomicU64 = AtomicU64::new(0); + + fn unique_test_git_dir(label: &str) -> PathBuf { + let id = NEXT_TEST_GIT_DIR_ID.fetch_add(1, Ordering::Relaxed); + std::env::temp_dir().join(format!( + "sce-claude-mutation-scope-health-{label}-{}-{id}", + std::process::id() + )) + } + + fn remove_test_git_dir(git_dir: &Path) { + let _ = std::fs::remove_dir_all(git_dir); + } + + fn key(tool_use_id: &str) -> AttemptKey { + AttemptKey { + session_id: "session-1".to_string(), + agent_id: None, + tool_use_id: tool_use_id.to_string(), + } + } + + fn failing_seam( + _root: &Path, + _payload: &str, + _logger: Option<&dyn Logger>, + ) -> anyhow::Result { + Err(anyhow!("seam failure injected by test")) + } + + fn unreachable_seam( + _root: &Path, + payload: &str, + _logger: Option<&dyn Logger>, + ) -> anyhow::Result { + panic!( + "the ingress seam must not be called while the recovery barrier is armed with non-empty attempts: {payload}" + ); + } + + #[test] + fn absent_state_file_is_healthy() { + let git_dir = unique_test_git_dir("absent"); + + let health = classify_health(&git_dir); + + assert_eq!(health.adapter, ActorKind::ClaudeCode); + assert_eq!(health.status, MutationScopeHealthStatus::Healthy); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn recovery_pending_false_is_healthy_even_with_live_attempts() { + let git_dir = unique_test_git_dir("recovery-false"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + state::allocate_attempt(&git_dir, &key("toolu_1"), "Write") + .expect("allocation should succeed"); + + let health = classify_health(&git_dir); + + assert_eq!(health.status, MutationScopeHealthStatus::Healthy); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn recovery_pending_true_with_empty_attempts_is_recovering() { + let git_dir = unique_test_git_dir("recovering"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + state::mark_recovery_pending(&git_dir).expect("marking recovery pending should succeed"); + + let health = classify_health(&git_dir); + + assert_eq!(health.status, MutationScopeHealthStatus::Recovering); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn recovery_pending_true_with_non_empty_attempts_is_blocked() { + let git_dir = unique_test_git_dir("blocked"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + state::allocate_attempt(&git_dir, &key("toolu_1"), "Write") + .expect("allocation should succeed"); + state::mark_recovery_pending(&git_dir).expect("marking recovery pending should succeed"); + + let health = classify_health(&git_dir); + + assert_eq!(health.status, MutationScopeHealthStatus::Blocked); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn malformed_state_file_is_invalid_with_the_read_error_surfaced() { + let git_dir = unique_test_git_dir("malformed"); + let path = state::state_path(&git_dir); + std::fs::create_dir_all(path.parent().expect("state path has a parent")) + .expect("state dir should be created"); + std::fs::write(&path, b"not json").expect("malformed file should be writable"); + + let health = classify_health(&git_dir); + + assert_eq!(health.status, MutationScopeHealthStatus::Invalid); + assert!( + health + .detail + .as_deref() + .unwrap_or_default() + .contains("malformed"), + "the read error must be surfaced in the detail: {:?}", + health.detail + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn stale_non_empty_attempts_after_a_failed_abandon_stays_blocked_across_repeated_pre_tool_use_ac3( + ) { + let git_dir = unique_test_git_dir("blocked-regression"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + let repository_root = git_dir.as_path(); + + let allocated = state::allocate_attempt(&git_dir, &key("toolu_1"), "Write") + .expect("allocating the live attempt should succeed"); + + let error = abandon_attempt( + &git_dir, + repository_root, + &allocated.attempt, + None, + &failing_seam, + ) + .expect_err("the injected abandon seam failure must propagate"); + assert!(error.to_string().contains("seam failure")); + + let seeded_state = state::read_state(&git_dir).expect("state should be readable"); + assert!( + seeded_state.recovery_pending, + "mark_recovery_pending must have run before the seam call failed" + ); + assert_eq!( + seeded_state.attempts.len(), + 1, + "remove_attempt must never have run because the seam call failed" + ); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Blocked + ); + + for attempt_number in 1..=2 { + let outcome = + apply_recovery_barrier(&git_dir, repository_root, None, &unreachable_seam); + assert!( + matches!(outcome, BarrierOutcome::Deny), + "PreToolUse call #{attempt_number} must be denied without self-clearing" + ); + } + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Blocked, + "the classifier must still report Blocked after repeated denial" + ); + + remove_test_git_dir(&git_dir); + } +} diff --git a/cli/src/services/hooks/claude_mutation_scope/mod.rs b/cli/src/services/hooks/claude_mutation_scope/mod.rs index 5081c5cf8..aa4c72c04 100644 --- a/cli/src/services/hooks/claude_mutation_scope/mod.rs +++ b/cli/src/services/hooks/claude_mutation_scope/mod.rs @@ -1,5 +1,6 @@ #![allow(dead_code)] +pub(crate) mod health; pub(crate) mod state; use std::path::{Path, PathBuf}; diff --git a/cli/src/services/hooks/claude_mutation_scope/state.rs b/cli/src/services/hooks/claude_mutation_scope/state.rs index aa920de54..ad19ef48a 100644 --- a/cli/src/services/hooks/claude_mutation_scope/state.rs +++ b/cli/src/services/hooks/claude_mutation_scope/state.rs @@ -72,7 +72,7 @@ fn state_dir(git_dir: &Path) -> PathBuf { git_dir.join(SCE_STATE_DIR) } -fn state_path(git_dir: &Path) -> PathBuf { +pub(crate) fn state_path(git_dir: &Path) -> PathBuf { state_dir(git_dir).join(ADAPTER_STATE_FILE) } diff --git a/cli/src/services/hooks/codex_mutation_scope/health.rs b/cli/src/services/hooks/codex_mutation_scope/health.rs new file mode 100644 index 000000000..52968cb83 --- /dev/null +++ b/cli/src/services/hooks/codex_mutation_scope/health.rs @@ -0,0 +1,264 @@ +use std::path::Path; + +use crate::services::hooks::mutation_scope_health::{ + MutationScopeAdapterHealth, MutationScopeHealthStatus, +}; +use crate::services::mutation_trace::types::ActorKind; + +use super::state::{self, RecoveryState}; + +pub(crate) fn classify_health(git_dir: &Path) -> MutationScopeAdapterHealth { + let state = match state::read_state(git_dir) { + Ok(state) => state, + Err(error) => { + return MutationScopeAdapterHealth::new( + ActorKind::Codex, + MutationScopeHealthStatus::Invalid, + "Codex mutation-scope state file could not be read or parsed.", + ) + .with_detail(error.to_string()); + } + }; + + match state.recovery { + RecoveryState::Clear => MutationScopeAdapterHealth::new( + ActorKind::Codex, + MutationScopeHealthStatus::Healthy, + "No persisted recovery condition blocks mutation-capable admission.", + ), + RecoveryState::Pending { .. } if state.attempts.is_empty() => { + MutationScopeAdapterHealth::new( + ActorKind::Codex, + MutationScopeHealthStatus::Recovering, + "Recovery is pending with no unresolved attempts; the next tracked PreToolUse call claims and completes the flush automatically.", + ) + } + RecoveryState::Pending { .. } => MutationScopeAdapterHealth::new( + ActorKind::Codex, + MutationScopeHealthStatus::Recovering, + "Recovery is pending with unresolved attempts; an unrelated tracked PreToolUse remains denied by the global recovery barrier, but a later tracked PreToolUse in the same (session_id, turn_id) lane retries the stale predecessor's abandonment through the ordinary same-lane sweep, which can clear the attempt and advance recovery to a flush without manual intervention.", + ), + RecoveryState::Flushing { .. } if state.attempts.is_empty() => { + MutationScopeAdapterHealth::new( + ActorKind::Codex, + MutationScopeHealthStatus::Recovering, + "A recovery flush is in progress; an orphaned flush is reclaimed and its flush retried automatically on the next tracked PreToolUse boundary.", + ) + } + RecoveryState::Flushing { .. } => MutationScopeAdapterHealth::new( + ActorKind::Codex, + MutationScopeHealthStatus::Invalid, + "Persisted state has a recovery flush in progress with unresolved attempts outstanding, a combination the adapter's state machine cannot legitimately produce.", + ), + } +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + use std::sync::atomic::{AtomicU64, Ordering}; + + use super::super::AttemptKey; + use super::*; + + static NEXT_TEST_GIT_DIR_ID: AtomicU64 = AtomicU64::new(0); + + fn unique_test_git_dir(label: &str) -> PathBuf { + let id = NEXT_TEST_GIT_DIR_ID.fetch_add(1, Ordering::Relaxed); + std::env::temp_dir().join(format!( + "sce-codex-mutation-scope-health-{label}-{}-{id}", + std::process::id() + )) + } + + fn remove_test_git_dir(git_dir: &Path) { + let _ = std::fs::remove_dir_all(git_dir); + } + + fn key(tool_use_id: &str) -> AttemptKey { + AttemptKey { + session_id: "session-1".to_string(), + agent_id: None, + tool_use_id: tool_use_id.to_string(), + } + } + + #[test] + fn absent_state_file_is_healthy() { + let git_dir = unique_test_git_dir("absent"); + + let health = classify_health(&git_dir); + + assert_eq!(health.adapter, ActorKind::Codex); + assert_eq!(health.status, MutationScopeHealthStatus::Healthy); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn clear_recovery_is_healthy_even_with_live_attempts() { + let git_dir = unique_test_git_dir("clear-with-attempts"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + state::seed_attempt_for_tests( + &git_dir, + &key("exec-1"), + "turn-1", + "Bash", + state::AttemptPhase::Active, + ); + + let health = classify_health(&git_dir); + + assert_eq!(health.status, MutationScopeHealthStatus::Healthy); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn pending_recovery_with_empty_attempts_is_recovering_and_the_next_admission_claims_the_flush() + { + let git_dir = unique_test_git_dir("pending-empty-recovering"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + let generation = state::arm_recovery(&git_dir).expect("arming recovery should succeed"); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering + ); + + assert_eq!( + state::admit_tracked_attempt(&git_dir, &key("exec-new"), "turn-1", "Bash") + .expect("admit should not error"), + state::AdmitDecision::FlushClaimed { generation }, + "Recovering must be proven by the next admission actually claiming the flush" + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn pending_non_empty_recovery_denies_unrelated_admission_without_losing_the_recovery_path() { + let git_dir = unique_test_git_dir("pending-non-empty-recovering"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + state::seed_attempt_for_tests( + &git_dir, + &key("exec-stuck"), + "turn-1", + "Bash", + state::AttemptPhase::Active, + ); + state::arm_recovery(&git_dir).expect("arming recovery should succeed"); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering, + "a proven same-lane self-healing path exists even though this state denies unrelated admission" + ); + + for attempt_number in 1..=2 { + assert_eq!( + state::admit_tracked_attempt(&git_dir, &key("exec-other"), "turn-2", "Bash") + .expect("admit should not error"), + state::AdmitDecision::RecoveryBlocked, + "admission #{attempt_number} in an unrelated lane must be denied without self-clearing" + ); + } + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering, + "the classifier must still report Recovering after repeated unrelated denial; \ + Recovering does not mean every future call succeeds, only that a proven normal \ + self-healing route exists" + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn orphaned_flushing_with_no_attempts_is_recovering_and_reclaimed_by_the_next_admission() { + let git_dir = unique_test_git_dir("orphaned-flushing-recovering"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + let generation = state::arm_recovery(&git_dir).expect("arming recovery should succeed"); + assert_eq!( + state::admit_tracked_attempt(&git_dir, &key("seed"), "seed-turn", "Bash") + .expect("seeding the flush claim should not error"), + state::AdmitDecision::FlushClaimed { generation }, + ); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering + ); + + state::normalize_recovery_after_boundary_lock_acquired(&git_dir) + .expect("normalize should succeed"); + assert_eq!( + state::admit_tracked_attempt(&git_dir, &key("exec-new"), "turn-1", "Bash") + .expect("admit should not error"), + state::AdmitDecision::FlushClaimed { generation }, + "Recovering must be proven by the reclaimed flush being retried" + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn flushing_with_unresolved_attempts_is_a_structurally_impossible_state_classified_invalid() { + let git_dir = unique_test_git_dir("flushing-non-empty-invalid"); + let dir = state::adapter_state_dir(&git_dir); + std::fs::create_dir_all(&dir).expect("state dir should be created"); + std::fs::write( + state::state_path(&git_dir), + serde_json::json!({ + "version": 3, + "next_attempt_seq": 2, + "next_recovery_generation": 2, + "recovery": { "phase": "flushing", "generation": 1 }, + "attempts": [{ + "attempt_seq": 1, + "scope_id": "cx-tool-v1|n=1|s=9:session-1|a=0:|t=6:exec-1", + "session_id": "session-1", + "turn_id": "turn-1", + "agent_id": null, + "tool_use_id": "exec-1", + "tool_name": "Bash", + "phase": "active", + }], + }) + .to_string(), + ) + .expect("hand-seeded state file should be writable"); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Invalid + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn malformed_state_file_is_invalid_with_the_read_error_surfaced() { + let git_dir = unique_test_git_dir("malformed"); + let dir = state::adapter_state_dir(&git_dir); + std::fs::create_dir_all(&dir).expect("state dir should be created"); + std::fs::write(state::state_path(&git_dir), b"not json") + .expect("malformed file should be writable"); + + let health = classify_health(&git_dir); + + assert_eq!(health.status, MutationScopeHealthStatus::Invalid); + assert!( + health + .detail + .as_deref() + .unwrap_or_default() + .contains("malformed"), + "the read error must be surfaced in the detail: {:?}", + health.detail + ); + + remove_test_git_dir(&git_dir); + } +} diff --git a/cli/src/services/hooks/codex_mutation_scope/mod.rs b/cli/src/services/hooks/codex_mutation_scope/mod.rs index f09afdcd3..beaba255f 100644 --- a/cli/src/services/hooks/codex_mutation_scope/mod.rs +++ b/cli/src/services/hooks/codex_mutation_scope/mod.rs @@ -1,6 +1,7 @@ #![allow(dead_code)] mod boundary_lock; +pub(crate) mod health; mod os_lock; pub(crate) mod state; @@ -2229,6 +2230,198 @@ mod tests { remove_test_git_dir(&git_dir); } + #[test] + fn pending_non_empty_recovery_denies_unrelated_admission_without_losing_the_recovery_path() + { + use crate::services::hooks::mutation_scope_health::MutationScopeHealthStatus; + + let git_dir = unique_test_git_dir("health-recovering-unrelated-denied"); + let resolver = fixed_resolver(git_dir.clone()); + seed_attempt( + &git_dir, + "session-1", + None, + "exec-main", + state::AttemptPhase::Active, + ); + + let seam = seam_failing_on("abandon"); + let error = run_codex_mutation_scope_from_payload_with( + &session_end_payload("session-1"), + None, + &resolver, + &seam, + ) + .expect_err("a failed abandonment during cleanup must propagate"); + assert!(error.to_string().contains("abandon")); + + assert_eq!( + health::classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering, + "a stuck non-empty attempt with recovery armed is Recovering: unrelated \ + admission stays fail-closed, but a same-lane successor can still retry \ + the stale predecessor's abandonment" + ); + + for attempt_number in 1..=2 { + let output = drive( + &pre_tool_use_json(&[ + (SESSION_ID_FIELD, Value::String("session-2".to_string())), + (TURN_ID_FIELD, Value::String("turn-2".to_string())), + ( + TOOL_USE_ID_FIELD, + Value::String("exec-unrelated".to_string()), + ), + ]), + &resolver, + &unreachable_seam, + ); + assert_eq!( + output, + pre_tool_use_deny_json(FAIL_CLOSED_DENY_REASON), + "PreToolUse call #{attempt_number} for an unrelated session must still be denied" + ); + } + + assert_eq!( + health::classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering, + "the classifier must still report Recovering after repeated denial from an \ + unrelated session; Recovering does not mean every future call succeeds, only \ + that a proven normal self-healing route exists" + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn same_lane_successor_retries_abandon_and_reaches_healthy_after_a_failed_lifecycle_abandon_ac4( + ) { + use crate::services::hooks::mutation_scope_health::MutationScopeHealthStatus; + + let git_dir = unique_test_git_dir("health-same-lane-self-heal"); + let resolver = fixed_resolver(git_dir.clone()); + + seed_attempt_in_turn( + &git_dir, + "session-1", + "turn-1", + None, + "exec-a", + state::AttemptPhase::Active, + ); + + let failing_abandon_seam = seam_failing_on("abandon"); + let output_b = drive( + &pre_tool_use_json(&[(TOOL_USE_ID_FIELD, Value::String("exec-b".to_string()))]), + &resolver, + &failing_abandon_seam, + ); + assert_eq!( + output_b, + pre_tool_use_deny_json(FAIL_CLOSED_DENY_REASON), + "B fails closed when the same-lane sweep's retried abandon of A fails" + ); + + let after_b = read_state(&git_dir); + assert_eq!( + after_b.attempts.len(), + 1, + "A remains persisted after the failed same-lane sweep" + ); + assert_eq!(after_b.attempts[0].tool_use_id, "exec-a"); + assert!(!after_b.recovery.is_clear()); + + assert_eq!( + health::classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering + ); + + let seam_calls = Arc::new(Mutex::new(Vec::new())); + let recording = recording_seam(Arc::clone(&seam_calls)); + let output_c = drive( + &pre_tool_use_json(&[(TOOL_USE_ID_FIELD, Value::String("exec-c".to_string()))]), + &resolver, + &recording, + ); + assert_eq!( + output_c, "", + "C's tracked Start proceeds once the same-lane sweep clears A and the \ + quiescent flush completes" + ); + + let operations: Vec = seam_calls + .lock() + .expect("recording seam mutex") + .iter() + .filter_map(|payload| { + serde_json::from_str::(payload) + .ok() + .and_then(|value| { + value + .get("operation") + .and_then(Value::as_str) + .map(str::to_string) + }) + }) + .collect(); + let abandon_index = operations + .iter() + .position(|operation| operation == "abandon"); + let flush_index = operations.iter().position(|operation| operation == "flush"); + let start_index = operations.iter().position(|operation| operation == "start"); + assert!( + abandon_index.is_some() && flush_index.is_some() && start_index.is_some(), + "expected abandon(A), flush, and start(C) seam calls, got {operations:?}" + ); + assert!( + abandon_index < flush_index && flush_index < start_index, + "expected abandon(A) -> flush -> start(C) ordering, got {operations:?}" + ); + + let final_state = read_state(&git_dir); + assert!(final_state.recovery.is_clear()); + assert_eq!(final_state.attempts.len(), 1); + assert_eq!(final_state.attempts[0].tool_use_id, "exec-c"); + + assert_eq!( + health::classify_health(&git_dir).status, + MutationScopeHealthStatus::Healthy + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn health_classifies_recovering_then_healthy_once_the_next_pre_tool_use_flushes_ac4() { + use crate::services::hooks::mutation_scope_health::MutationScopeHealthStatus; + + let git_dir = unique_test_git_dir("health-recovering-then-healthy"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + let resolver = fixed_resolver(git_dir.clone()); + state::arm_recovery(&git_dir).expect("arming the barrier should succeed"); + + assert_eq!( + health::classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering + ); + + let output = drive( + &pre_tool_use_json(&[(TOOL_USE_ID_FIELD, Value::String("exec-new".to_string()))]), + &resolver, + &ok_seam, + ); + assert_eq!(output, ""); + + assert_eq!( + health::classify_health(&git_dir).status, + MutationScopeHealthStatus::Healthy, + "a successful flush clears recovery and returns the adapter to Healthy" + ); + + remove_test_git_dir(&git_dir); + } + #[test] fn recovery_barrier_denies_new_tracked_pre_tool_use_while_attempts_remain_ac12() { let git_dir = unique_test_git_dir("barrier-attempts-remain"); diff --git a/cli/src/services/hooks/codex_mutation_scope/state.rs b/cli/src/services/hooks/codex_mutation_scope/state.rs index 7e0162f7f..e7ab576e5 100644 --- a/cli/src/services/hooks/codex_mutation_scope/state.rs +++ b/cli/src/services/hooks/codex_mutation_scope/state.rs @@ -120,7 +120,7 @@ pub(crate) fn adapter_state_dir(git_dir: &Path) -> PathBuf { git_dir.join(SCE_STATE_DIR) } -fn state_path(git_dir: &Path) -> PathBuf { +pub(crate) fn state_path(git_dir: &Path) -> PathBuf { adapter_state_dir(git_dir).join(ADAPTER_STATE_FILE) } diff --git a/cli/src/services/hooks/mod.rs b/cli/src/services/hooks/mod.rs index 52a19c673..79c6bb2ad 100644 --- a/cli/src/services/hooks/mod.rs +++ b/cli/src/services/hooks/mod.rs @@ -48,6 +48,7 @@ pub mod codex_mutation_scope; pub mod command; pub mod lifecycle; pub mod mutation_scope; +pub mod mutation_scope_health; pub mod opencode_mutation_scope; pub mod pi_mutation_scope; diff --git a/cli/src/services/hooks/mutation_scope_health.rs b/cli/src/services/hooks/mutation_scope_health.rs new file mode 100644 index 000000000..783f2eb3f --- /dev/null +++ b/cli/src/services/hooks/mutation_scope_health.rs @@ -0,0 +1,94 @@ +use crate::services::mutation_trace::types::ActorKind; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[allow(dead_code)] +pub(crate) enum MutationScopeHealthStatus { + Healthy, + Recovering, + Blocked, + Invalid, +} + +#[derive(Clone, Debug, Eq, PartialEq)] +#[allow(dead_code)] +pub(crate) struct MutationScopeAdapterHealth { + pub(crate) adapter: ActorKind, + pub(crate) status: MutationScopeHealthStatus, + pub(crate) reason: String, + pub(crate) detail: Option, +} + +#[allow(dead_code)] +impl MutationScopeAdapterHealth { + pub(crate) fn new( + adapter: ActorKind, + status: MutationScopeHealthStatus, + reason: impl Into, + ) -> Self { + Self { + adapter, + status, + reason: reason.into(), + detail: None, + } + } + + pub(crate) fn with_detail(mut self, detail: impl Into) -> Self { + self.detail = Some(detail.into()); + self + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn health_status_variants_are_distinct() { + assert_ne!( + MutationScopeHealthStatus::Healthy, + MutationScopeHealthStatus::Recovering + ); + assert_ne!( + MutationScopeHealthStatus::Recovering, + MutationScopeHealthStatus::Blocked + ); + assert_ne!( + MutationScopeHealthStatus::Blocked, + MutationScopeHealthStatus::Invalid + ); + assert_eq!( + MutationScopeHealthStatus::Healthy, + MutationScopeHealthStatus::Healthy + ); + } + + #[test] + fn new_adapter_health_has_no_detail_by_default() { + let health = MutationScopeAdapterHealth::new( + ActorKind::ClaudeCode, + MutationScopeHealthStatus::Healthy, + "no persisted recovery problem", + ); + + assert_eq!(health.adapter, ActorKind::ClaudeCode); + assert_eq!(health.status, MutationScopeHealthStatus::Healthy); + assert_eq!(health.reason, "no persisted recovery problem"); + assert_eq!(health.detail, None); + } + + #[test] + fn with_detail_attaches_machine_detail_without_changing_status_or_reason() { + let health = MutationScopeAdapterHealth::new( + ActorKind::Codex, + MutationScopeHealthStatus::Invalid, + "state file failed to parse", + ) + .with_detail("unexpected EOF at byte 12"); + + assert_eq!(health.adapter, ActorKind::Codex); + assert_eq!(health.status, MutationScopeHealthStatus::Invalid); + assert_eq!(health.reason, "state file failed to parse"); + assert_eq!(health.detail.as_deref(), Some("unexpected EOF at byte 12")); + } +} diff --git a/cli/src/services/hooks/opencode_mutation_scope/health.rs b/cli/src/services/hooks/opencode_mutation_scope/health.rs new file mode 100644 index 000000000..f861646b2 --- /dev/null +++ b/cli/src/services/hooks/opencode_mutation_scope/health.rs @@ -0,0 +1,723 @@ +use std::path::Path; + +use crate::services::hooks::mutation_scope_health::{ + MutationScopeAdapterHealth, MutationScopeHealthStatus, +}; +use crate::services::mutation_trace::types::ActorKind; + +use super::state::{self, AttemptPhase, RecoveryState}; + +pub(crate) fn classify_health(git_dir: &Path) -> MutationScopeAdapterHealth { + let state = match state::read_state(git_dir) { + Ok(state) => state, + Err(error) => { + return MutationScopeAdapterHealth::new( + ActorKind::OpenCode, + MutationScopeHealthStatus::Invalid, + "OpenCode mutation-scope state file could not be read or parsed.", + ) + .with_detail(error.to_string()); + } + }; + + let has_pending_abandon = state + .attempts + .iter() + .any(|attempt| attempt.phase == AttemptPhase::PendingAbandon); + let has_pending_start = state + .attempts + .iter() + .any(|attempt| attempt.phase == AttemptPhase::PendingStart); + + match state.recovery { + RecoveryState::Clear if has_pending_abandon => MutationScopeAdapterHealth::new( + ActorKind::OpenCode, + MutationScopeHealthStatus::Invalid, + "Persisted state has a PendingAbandon attempt with recovery already Clear, a combination the adapter's recovery-flush state machine cannot legitimately produce (a PendingAbandon attempt is only ever removed as part of the same recovery flush that clears recovery to Clear).", + ), + _ if has_pending_start => MutationScopeAdapterHealth::new( + ActorKind::OpenCode, + MutationScopeHealthStatus::Blocked, + "A tracked attempt is stuck in PendingStart; only that same call's own ToolExecuteAfter/ToolError boundary retires a PendingStart attempt, and resolve_recovery only retries attempts already in PendingAbandon, so a concurrently Pending/Flushing recovery generation for an unrelated attempt can clear without ever touching this one, leaving future tracked admissions from other calls denied without self-clearing.", + ), + RecoveryState::Clear => MutationScopeAdapterHealth::new( + ActorKind::OpenCode, + MutationScopeHealthStatus::Healthy, + "No persisted recovery condition blocks mutation-capable admission.", + ), + RecoveryState::Pending { .. } => MutationScopeAdapterHealth::new( + ActorKind::OpenCode, + MutationScopeHealthStatus::Recovering, + "Recovery is pending; the next tracked admission from any call claims the flush and retries any outstanding abandonment automatically, regardless of which call performs it.", + ), + RecoveryState::Flushing { .. } => MutationScopeAdapterHealth::new( + ActorKind::OpenCode, + MutationScopeHealthStatus::Recovering, + "An orphaned recovery flush is reclaimed from Flushing to Pending by the next tracked adapter boundary. A subsequent recovery-capable tracked admission claims the pending generation and retries recovery automatically.", + ), + } +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + use std::sync::atomic::{AtomicU64, Ordering}; + use std::sync::Mutex; + + use serde_json::{json, Value}; + + use super::super::{run_opencode_mutation_scope_from_payload_with_seams, AttemptKey}; + use super::*; + use crate::services::observability::traits::Logger; + + const FAIL_CLOSED_MESSAGE: &str = + "SCE could not establish OpenCode mutation attribution for this tool execution."; + const CWD: &str = "/repo/opencode-checkout"; + + static NEXT_TEST_GIT_DIR_ID: AtomicU64 = AtomicU64::new(0); + + fn unique_test_git_dir(label: &str) -> PathBuf { + let id = NEXT_TEST_GIT_DIR_ID.fetch_add(1, Ordering::Relaxed); + std::env::temp_dir().join(format!( + "sce-opencode-mutation-scope-health-{label}-{}-{id}", + std::process::id() + )) + } + + fn remove_test_git_dir(git_dir: &Path) { + let _ = std::fs::remove_dir_all(git_dir); + } + + fn key(call_id: &str) -> AttemptKey { + AttemptKey { + session_id: "ses-main".to_string(), + call_id: call_id.to_string(), + } + } + + fn tool_before(tool_name: &str, call_id: &str) -> String { + json!({ + "hook_event_name": "ToolExecuteBefore", + "session_id": "ses-main", + "call_id": call_id, + "cwd": CWD, + "tool_name": tool_name, + }) + .to_string() + } + + fn tool_error(tool_name: &str, call_id: &str) -> String { + json!({ + "hook_event_name": "ToolError", + "session_id": "ses-main", + "call_id": call_id, + "cwd": CWD, + "tool_name": tool_name, + }) + .to_string() + } + + struct RecordingSeam { + calls: Mutex>, + fail_operations: Vec, + fail_operation_occurrence: Option<(String, usize)>, + } + + impl RecordingSeam { + fn new() -> Self { + Self { + calls: Mutex::new(Vec::new()), + fail_operations: Vec::new(), + fail_operation_occurrence: None, + } + } + + fn failing_on(operations: &[&str]) -> Self { + Self { + fail_operations: operations.iter().map(|op| (*op).to_string()).collect(), + ..Self::new() + } + } + + fn failing_on_nth_occurrence(operation: &str, occurrence: usize) -> Self { + Self { + fail_operation_occurrence: Some((operation.to_string(), occurrence)), + ..Self::new() + } + } + + fn handle(&self, payload: &str) -> anyhow::Result { + let operation = operation_of(payload); + let occurrence = { + let mut calls = self.calls.lock().expect("seam mutex"); + calls.push(operation.clone()); + calls + .iter() + .filter(|candidate| *candidate == &operation) + .count() + }; + if self.fail_operations.contains(&operation) { + anyhow::bail!("seam failure injected by test for '{operation}'"); + } + if let Some((target, target_occurrence)) = &self.fail_operation_occurrence { + if target == &operation && *target_occurrence == occurrence { + anyhow::bail!( + "seam failure injected by test for '{operation}' occurrence {occurrence}" + ); + } + } + Ok(String::new()) + } + } + + fn operation_of(payload: &str) -> String { + let value: Value = serde_json::from_str(payload).expect("seam payload is JSON"); + value + .get("operation") + .and_then(Value::as_str) + .expect("seam payload has an operation") + .to_string() + } + + fn drive(git_dir: &Path, seam: &RecordingSeam, payload: &str) -> anyhow::Result { + let resolver = |_cwd: &str| Ok(git_dir.to_path_buf()); + let seam_fn = + |_root: &Path, payload: &str, _logger: Option<&dyn Logger>| seam.handle(payload); + run_opencode_mutation_scope_from_payload_with_seams(payload, None, &resolver, &seam_fn) + } + + #[test] + fn absent_state_file_is_healthy() { + let git_dir = unique_test_git_dir("absent"); + + let health = classify_health(&git_dir); + + assert_eq!(health.adapter, ActorKind::OpenCode); + assert_eq!(health.status, MutationScopeHealthStatus::Healthy); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn clear_recovery_is_healthy_even_with_active_attempts() { + let git_dir = unique_test_git_dir("clear-with-active"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + state::seed_attempt_for_tests(&git_dir, &key("call-1"), "write", AttemptPhase::Active); + + let health = classify_health(&git_dir); + + assert_eq!(health.status, MutationScopeHealthStatus::Healthy); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn pending_start_with_clear_recovery_is_blocked_and_denies_repeated_unrelated_admissions_ac4() { + let git_dir = unique_test_git_dir("pending-start-blocked"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + + let failing_start = RecordingSeam::failing_on(&["start"]); + drive(&git_dir, &failing_start, &tool_before("write", "call-1")) + .expect_err("a failed Start seam must fail closed, leaving the attempt PendingStart"); + assert_eq!( + state::read_state(&git_dir) + .expect("state readable") + .attempts[0] + .phase, + AttemptPhase::PendingStart, + ); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Blocked, + "a stale PendingStart with no recovery in progress has no automatic sweep for unrelated admissions" + ); + + let healthy = RecordingSeam::new(); + for (index, call_id) in ["call-2", "call-3"].into_iter().enumerate() { + let error = + drive(&git_dir, &healthy, &tool_before("write", call_id)).expect_err(&format!( + "unrelated admission #{} must be denied without self-clearing", + index + 1 + )); + assert!(error.to_string().contains(FAIL_CLOSED_MESSAGE)); + } + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Blocked, + "the classifier must still report Blocked after repeated unrelated denial" + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn pending_recovery_with_a_pending_start_attempt_is_blocked_even_though_an_unrelated_pending_abandon_can_still_clear_ac4( + ) { + let git_dir = unique_test_git_dir("pending-recovery-with-pending-start"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + + { + let ok_seam = RecordingSeam::new(); + drive(&git_dir, &ok_seam, &tool_before("write", "call-a")).expect("A starts"); + } + + let failing_start = RecordingSeam::failing_on(&["start"]); + drive(&git_dir, &failing_start, &tool_before("write", "call-b")) + .expect_err("B's Start seam fails, leaving B PendingStart"); + assert_eq!( + state::read_state(&git_dir) + .expect("state readable") + .attempts + .iter() + .find(|a| a.call_id == "call-b") + .expect("B is tracked") + .phase, + AttemptPhase::PendingStart, + ); + + let failing_flush = RecordingSeam::failing_on(&["flush"]); + drive(&git_dir, &failing_flush, &tool_error("write", "call-a")) + .expect("A's terminal cleanup returns best-effort even though its flush fails"); + + let seeded = state::read_state(&git_dir).expect("state readable"); + assert_eq!(seeded.attempts.len(), 2, "both A and B are still tracked"); + assert_eq!( + seeded + .attempts + .iter() + .find(|a| a.call_id == "call-a") + .expect("A is tracked") + .phase, + AttemptPhase::PendingAbandon, + ); + assert_eq!( + seeded + .attempts + .iter() + .find(|a| a.call_id == "call-b") + .expect("B is tracked") + .phase, + AttemptPhase::PendingStart, + ); + assert!( + matches!(seeded.recovery, RecoveryState::Pending { .. }), + "the failed ambiguity flush relinquishes Flushing back to Pending: {:?}", + seeded.recovery + ); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Blocked, + "B's PendingStart has no automatic sweep, so this state has no future path back to \ + normal admission that doesn't depend on B's own missing terminal event, even though \ + A's PendingAbandon under the same Pending recovery generation could still self-heal \ + on its own" + ); + + let healthy = RecordingSeam::new(); + let error = drive(&git_dir, &healthy, &tool_before("write", "call-c")) + .expect_err("C is unrelated to both A and B and must still be denied"); + assert!(error.to_string().contains(FAIL_CLOSED_MESSAGE)); + + let resolved = state::read_state(&git_dir).expect("state readable"); + assert!( + resolved.recovery.is_clear(), + "A's recovery generation advanced to completion via C's admission attempt: {:?}", + resolved.recovery + ); + assert!( + resolved.attempts.iter().all(|a| a.call_id != "call-a"), + "A was cleaned up by the same recovery resolution that denied C" + ); + assert_eq!( + resolved + .attempts + .iter() + .find(|a| a.call_id == "call-b") + .expect("B is still tracked") + .phase, + AttemptPhase::PendingStart, + "B's PendingStart survives the recovery generation that cleared A" + ); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Blocked, + "recovery for A fully resolved to Clear, but B's PendingStart durably wedges the \ + adapter: this is the reachable Pending+PendingStart -> Clear+PendingStart counterexample" + ); + + let error = drive(&git_dir, &healthy, &tool_before("write", "call-d")) + .expect_err("D is denied again; the adapter never self-clears without B's own event"); + assert!(error.to_string().contains(FAIL_CLOSED_MESSAGE)); + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Blocked, + "repeated unrelated denial must not change the classification" + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn orphaned_flushing_with_a_pending_start_attempt_is_blocked_not_recovering_ac4() { + let git_dir = unique_test_git_dir("orphaned-flushing-with-pending-start"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + + { + let ok_seam = RecordingSeam::new(); + drive(&git_dir, &ok_seam, &tool_before("write", "call-a")).expect("A starts"); + } + + let failing_start = RecordingSeam::failing_on(&["start"]); + drive(&git_dir, &failing_start, &tool_before("write", "call-b")) + .expect_err("B's Start seam fails, leaving B PendingStart"); + + let doomed = state::read_state(&git_dir) + .expect("state readable") + .attempts + .into_iter() + .find(|a| a.call_id == "call-a") + .expect("A is tracked"); + + // Simulate a crash between begin_terminal_cleanup arming Flushing and + // resolve_recovery ever running: this is the only way to observe a + // literal orphaned `Flushing` at rest, since every in-process caller of + // begin_terminal_cleanup always calls resolve_recovery immediately + // afterward under the same boundary lock. A real process crash at this + // exact point is a legitimately persisted, production-reachable shape. + state::begin_terminal_cleanup(&git_dir, std::slice::from_ref(&doomed.scope_id)) + .expect("seeding an orphaned flush should succeed"); + let seeded = state::read_state(&git_dir).expect("state readable"); + assert!(matches!(seeded.recovery, RecoveryState::Flushing { .. })); + assert_eq!( + seeded + .attempts + .iter() + .find(|a| a.call_id == "call-b") + .expect("B is tracked") + .phase, + AttemptPhase::PendingStart, + ); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Blocked, + "the next boundary reclaims Flushing -> Pending and can advance A's recovery, but \ + B's PendingStart has no reclaim path, so admission stays durably wedged" + ); + + let healthy = RecordingSeam::new(); + let error = drive(&git_dir, &healthy, &tool_before("write", "call-c")) + .expect_err("C is unrelated to both A and B and must still be denied"); + assert!(error.to_string().contains(FAIL_CLOSED_MESSAGE)); + + let resolved = state::read_state(&git_dir).expect("state readable"); + assert!( + resolved.recovery.is_clear(), + "the orphaned flush was reclaimed and A's recovery generation completed: {:?}", + resolved.recovery + ); + assert!(resolved.attempts.iter().all(|a| a.call_id != "call-a")); + assert_eq!( + resolved + .attempts + .iter() + .find(|a| a.call_id == "call-b") + .expect("B is still tracked") + .phase, + AttemptPhase::PendingStart, + ); + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Blocked, + "A's orphaned-flush recovery reaching Clear does not rescue B's PendingStart" + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn pending_recovery_with_pending_abandon_attempts_is_recovering_and_an_unrelated_admission_clears_it( + ) { + let git_dir = unique_test_git_dir("pending-non-empty-recovering"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + + { + let ok_seam = RecordingSeam::new(); + drive(&git_dir, &ok_seam, &tool_before("write", "call-1")).expect("Start"); + } + + let failing = RecordingSeam::failing_on(&["abandon"]); + drive(&git_dir, &failing, &tool_error("write", "call-1")) + .expect("a terminal failure whose abandon fails still returns best-effort"); + + let seeded = state::read_state(&git_dir).expect("state readable"); + assert_eq!( + seeded.attempts.len(), + 1, + "the doomed attempt is not forgotten" + ); + assert_eq!(seeded.attempts[0].phase, AttemptPhase::PendingAbandon); + assert!(matches!(seeded.recovery, RecoveryState::Pending { .. })); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering, + "a proven self-healing path exists even though this state currently denies admission" + ); + + for attempt_number in 1..=2 { + let error = drive( + &git_dir, + &failing, + &tool_before("write", &format!("call-retry-{attempt_number}")), + ) + .expect_err("admission while recovery is unresolved must stay fail-closed"); + assert!(error.to_string().contains(FAIL_CLOSED_MESSAGE)); + } + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering, + "repeated denial under a still-failing seam must not change the classification" + ); + + let healthy = RecordingSeam::new(); + drive(&git_dir, &healthy, &tool_before("write", "call-2")) + .expect("an unrelated call's admission resolves recovery once the seam succeeds"); + + let resolved = state::read_state(&git_dir).expect("state readable"); + assert!(resolved.recovery.is_clear()); + assert!(resolved.attempts.iter().all(|a| a.call_id != "call-1")); + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Healthy + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn pending_recovery_with_empty_attempts_is_recovering_and_the_next_admission_clears_it() { + let git_dir = unique_test_git_dir("pending-empty-recovering"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + + { + let ok_seam = RecordingSeam::new(); + drive(&git_dir, &ok_seam, &tool_before("edit", "call-1")).expect("Start"); + } + + let rebaseline_failing = RecordingSeam::failing_on_nth_occurrence("flush", 2); + drive(&git_dir, &rebaseline_failing, &tool_error("edit", "call-1")) + .expect("a terminal failure whose rebaseline flush fails still returns"); + + let seeded = state::read_state(&git_dir).expect("state readable"); + assert!( + seeded.attempts.is_empty(), + "the abandon succeeded so the attempt was removed before the rebaseline flush failed" + ); + assert!(matches!(seeded.recovery, RecoveryState::Pending { .. })); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering + ); + + let healthy = RecordingSeam::new(); + drive(&git_dir, &healthy, &tool_before("edit", "call-2")).expect("retry admits new work"); + + let resolved = state::read_state(&git_dir).expect("state readable"); + assert!(resolved.recovery.is_clear()); + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Healthy + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn orphaned_flushing_with_pending_abandon_attempts_is_recovering_and_reclaimed_by_the_next_boundary( + ) { + let git_dir = unique_test_git_dir("orphaned-flushing-recovering"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + + let attempt = state::seed_attempt_for_tests( + &git_dir, + &key("call-stuck"), + "write", + AttemptPhase::Active, + ); + state::begin_terminal_cleanup(&git_dir, std::slice::from_ref(&attempt.scope_id)) + .expect("seeding an orphaned flush should succeed"); + assert!(matches!( + state::read_state(&git_dir) + .expect("state readable") + .recovery, + RecoveryState::Flushing { .. } + )); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering, + "a crash mid-flush leaves an orphaned Flushing that the next boundary reclaims" + ); + + let healthy = RecordingSeam::new(); + drive(&git_dir, &healthy, &tool_before("write", "call-new")) + .expect("the orphaned flush is reclaimed and retried, then the new call is admitted"); + + let resolved = state::read_state(&git_dir).expect("state readable"); + assert!(resolved.recovery.is_clear()); + assert!(resolved.attempts.iter().all(|a| a.call_id != "call-stuck")); + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Healthy + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn clear_recovery_with_a_pending_abandon_attempt_is_a_structurally_impossible_state_classified_invalid( + ) { + let git_dir = unique_test_git_dir("clear-with-pending-abandon-invalid"); + let dir = state::adapter_state_dir(&git_dir); + std::fs::create_dir_all(&dir).expect("state dir should be created"); + std::fs::write( + state::state_path(&git_dir), + serde_json::json!({ + "version": 1, + "next_recovery_generation": 1, + "recovery": { "phase": "clear" }, + "attempts": [{ + "scope_id": "oc-tool-v1|s=8:ses-main|c=6:call-1", + "session_id": "ses-main", + "call_id": "call-1", + "tool_name": "write", + "phase": "pending_abandon", + }], + }) + .to_string(), + ) + .expect("hand-seeded state file should be writable"); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Invalid + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn malformed_state_file_is_invalid_with_the_read_error_surfaced() { + let git_dir = unique_test_git_dir("malformed"); + let dir = state::adapter_state_dir(&git_dir); + std::fs::create_dir_all(&dir).expect("state dir should be created"); + std::fs::write(state::state_path(&git_dir), b"not json") + .expect("malformed file should be writable"); + + let health = classify_health(&git_dir); + + assert_eq!(health.status, MutationScopeHealthStatus::Invalid); + assert!( + health + .detail + .as_deref() + .unwrap_or_default() + .contains("malformed"), + "the read error must be surfaced in the detail: {:?}", + health.detail + ); + + remove_test_git_dir(&git_dir); + } + + fn matrix_attempt(call_id: &str, phase: AttemptPhase) -> state::AdapterAttempt { + let key = AttemptKey { + session_id: "ses-main".to_string(), + call_id: call_id.to_string(), + }; + + state::AdapterAttempt { + scope_id: super::super::format_opencode_scope_id(&key), + session_id: key.session_id, + call_id: key.call_id, + tool_name: "write".to_string(), + phase, + } + } + + fn write_matrix_state( + git_dir: &Path, + recovery: RecoveryState, + has_pending_abandon: bool, + has_pending_start: bool, + ) { + std::fs::create_dir_all(state::adapter_state_dir(git_dir)) + .expect("adapter state dir should be created"); + + let mut attempts = vec![matrix_attempt("call-active", AttemptPhase::Active)]; + if has_pending_abandon { + attempts.push(matrix_attempt("call-abandon", AttemptPhase::PendingAbandon)); + } + if has_pending_start { + attempts.push(matrix_attempt("call-start", AttemptPhase::PendingStart)); + } + + let state = state::AdapterState { + version: 1, + next_recovery_generation: 2, + recovery, + attempts, + }; + std::fs::write( + state::state_path(git_dir), + serde_json::to_string(&state).expect("matrix state serializes"), + ) + .expect("hand-built matrix state should be writable"); + } + + #[test] + fn health_classification_matrix_covers_all_twelve_recovery_and_attempt_phase_combinations() { + use MutationScopeHealthStatus::{Blocked, Healthy, Invalid, Recovering}; + + let clear = RecoveryState::Clear; + let pending = RecoveryState::Pending { generation: 1 }; + let flushing = RecoveryState::Flushing { generation: 1 }; + + let rows = [ + (clear, false, false, Healthy), + (clear, false, true, Blocked), + (clear, true, false, Invalid), + (clear, true, true, Invalid), + (pending, false, false, Recovering), + (pending, false, true, Blocked), + (pending, true, false, Recovering), + (pending, true, true, Blocked), + (flushing, false, false, Recovering), + (flushing, false, true, Blocked), + (flushing, true, false, Recovering), + (flushing, true, true, Blocked), + ]; + + for (index, (recovery, has_pending_abandon, has_pending_start, expected)) in + rows.into_iter().enumerate() + { + let git_dir = unique_test_git_dir(&format!("matrix-{index}")); + write_matrix_state(&git_dir, recovery, has_pending_abandon, has_pending_start); + + assert_eq!( + classify_health(&git_dir).status, + expected, + "row {index}: recovery={recovery:?} has_pending_abandon={has_pending_abandon} \ + has_pending_start={has_pending_start}", + ); + + remove_test_git_dir(&git_dir); + } + } +} diff --git a/cli/src/services/hooks/opencode_mutation_scope/mod.rs b/cli/src/services/hooks/opencode_mutation_scope/mod.rs index e4da0060a..8a42f86b8 100644 --- a/cli/src/services/hooks/opencode_mutation_scope/mod.rs +++ b/cli/src/services/hooks/opencode_mutation_scope/mod.rs @@ -1,6 +1,7 @@ #![allow(dead_code)] mod boundary_lock; +pub(crate) mod health; mod os_lock; pub(crate) mod state; diff --git a/cli/src/services/hooks/opencode_mutation_scope/state.rs b/cli/src/services/hooks/opencode_mutation_scope/state.rs index 1d7d82725..ec77f17ac 100644 --- a/cli/src/services/hooks/opencode_mutation_scope/state.rs +++ b/cli/src/services/hooks/opencode_mutation_scope/state.rs @@ -110,7 +110,7 @@ pub(crate) fn adapter_state_dir(git_dir: &Path) -> PathBuf { git_dir.join(SCE_STATE_DIR) } -fn state_path(git_dir: &Path) -> PathBuf { +pub(crate) fn state_path(git_dir: &Path) -> PathBuf { adapter_state_dir(git_dir).join(ADAPTER_STATE_FILE) } diff --git a/cli/src/services/hooks/pi_mutation_scope/health.rs b/cli/src/services/hooks/pi_mutation_scope/health.rs new file mode 100644 index 000000000..d7051bed2 --- /dev/null +++ b/cli/src/services/hooks/pi_mutation_scope/health.rs @@ -0,0 +1,766 @@ +use std::path::Path; + +use crate::services::hooks::mutation_scope_health::{ + MutationScopeAdapterHealth, MutationScopeHealthStatus, +}; +use crate::services::mutation_trace::types::ActorKind; + +use super::process_owner::is_definitely_dead; +use super::state::{self, AttemptPhase, RecoveryState}; + +pub(crate) fn classify_health(git_dir: &Path) -> MutationScopeAdapterHealth { + let state = match state::read_state(git_dir) { + Ok(state) => state, + Err(error) => { + return MutationScopeAdapterHealth::new( + ActorKind::Pi, + MutationScopeHealthStatus::Invalid, + "Pi mutation-scope state file could not be read or parsed.", + ) + .with_detail(error.to_string()); + } + }; + + let has_pending_abandon = state + .attempts + .iter() + .any(|attempt| attempt.phase == AttemptPhase::PendingAbandon); + let has_dead_owner_live_attempt = state.attempts.iter().any(|attempt| { + matches!( + attempt.phase, + AttemptPhase::PendingStart | AttemptPhase::Executed + ) && is_definitely_dead(&attempt.owner) + }); + + match state.recovery { + RecoveryState::Clear if has_pending_abandon => MutationScopeAdapterHealth::new( + ActorKind::Pi, + MutationScopeHealthStatus::Invalid, + "Persisted state has a PendingAbandon attempt with recovery already Clear, a combination the adapter's recovery-flush state machine cannot legitimately produce (a PendingAbandon attempt is only ever created and removed as part of the same recovery-flush generation that arms and then clears RecoveryState).", + ), + RecoveryState::Clear if has_dead_owner_live_attempt => MutationScopeAdapterHealth::new( + ActorKind::Pi, + MutationScopeHealthStatus::Recovering, + "A tracked PendingStart/Executed attempt's recorded owner process is positively dead. The D10 stale-owner sweep (reconcile_stale_owners) runs unconditionally on every future tracked Start from any session, before that session's own admission is even considered, and automatically retires the dead-owner attempt through the ordinary flush/abandon/flush recovery sequence.", + ), + RecoveryState::Clear => MutationScopeAdapterHealth::new( + ActorKind::Pi, + MutationScopeHealthStatus::Healthy, + "No persisted recovery condition blocks mutation-capable admission. A live or unprovably-dead PendingStart/Executed attempt never blocks an unrelated tracked admission on this adapter.", + ), + RecoveryState::Pending { .. } => MutationScopeAdapterHealth::new( + ActorKind::Pi, + MutationScopeHealthStatus::Recovering, + "Recovery is pending. A subsequent recovery-capable tracked Start whose key is not already represented by a nonterminal attempt can claim the pending generation and retry every outstanding PendingAbandon attempt through the ordinary recovery path. A duplicate Start for an already-tracked PendingStart/Executed key may be idempotently reused before the recovery-state gate, so not every individual Start necessarily advances recovery. Pending is Recovering because an ordinary future tracked admission can advance it without manual intervention; an unrelated stuck PendingStart/Executed attempt, if any, does not prevent this resolution, since this adapter never gates admission on another attempt's PendingStart/Executed phase.", + ), + RecoveryState::Flushing { .. } => MutationScopeAdapterHealth::new( + ActorKind::Pi, + MutationScopeHealthStatus::Recovering, + "An orphaned recovery flush is reclaimed from Flushing to Pending by the very next tracked adapter boundary (Start, ToolExecutionEnd, or ToolExecutionAbandon all normalize it before doing anything else). A subsequent recovery-capable fresh tracked Start can then claim the reclaimed generation and retry recovery automatically, but a duplicate Start for an already-tracked nonterminal key is not guaranteed to be the one that does so.", + ), + } +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + use std::sync::atomic::{AtomicU64, Ordering}; + use std::sync::Mutex; + + use serde_json::{json, Value}; + + use super::super::process_owner::ProcessOwner; + use super::super::{ + force_attempt_owner_dead_for_tests, run_pi_mutation_scope_from_payload_with_seams, + AttemptKey, + }; + use super::*; + use crate::services::observability::traits::Logger; + + const FAIL_CLOSED_MESSAGE: &str = + "SCE could not establish Pi mutation attribution for this tool execution."; + const CWD: &str = "/repo/pi-checkout"; + + static NEXT_TEST_GIT_DIR_ID: AtomicU64 = AtomicU64::new(0); + + fn unique_test_git_dir(label: &str) -> PathBuf { + let id = NEXT_TEST_GIT_DIR_ID.fetch_add(1, Ordering::Relaxed); + std::env::temp_dir().join(format!( + "sce-pi-mutation-scope-health-{label}-{}-{id}", + std::process::id() + )) + } + + fn remove_test_git_dir(git_dir: &Path) { + let _ = std::fs::remove_dir_all(git_dir); + } + + fn key(session_id: &str, tool_call_id: &str) -> AttemptKey { + AttemptKey { + session_id: session_id.to_string(), + tool_call_id: tool_call_id.to_string(), + } + } + + struct RecordingSeam { + calls: Mutex>, + fail_operations: Vec, + fail_once_operations: Mutex>, + } + + impl RecordingSeam { + fn new() -> Self { + Self { + calls: Mutex::new(Vec::new()), + fail_operations: Vec::new(), + fail_once_operations: Mutex::new(Vec::new()), + } + } + + fn failing_on(operations: &[&str]) -> Self { + Self { + fail_operations: operations.iter().map(|op| (*op).to_string()).collect(), + ..Self::new() + } + } + + fn failing_once_on(operations: &[&str]) -> Self { + Self { + fail_once_operations: Mutex::new( + operations.iter().map(|op| (*op).to_string()).collect(), + ), + ..Self::new() + } + } + + fn handle(&self, payload: &str) -> anyhow::Result { + let operation = operation_of(payload); + self.calls + .lock() + .expect("seam mutex") + .push(operation.clone()); + if self.fail_operations.contains(&operation) { + anyhow::bail!("seam failure injected by test for '{operation}'"); + } + let mut once = self.fail_once_operations.lock().expect("seam mutex"); + if let Some(position) = once.iter().position(|candidate| candidate == &operation) { + once.remove(position); + anyhow::bail!("transient seam failure injected once by test for '{operation}'"); + } + Ok(String::new()) + } + } + + fn operation_of(payload: &str) -> String { + let value: Value = serde_json::from_str(payload).expect("seam payload is JSON"); + value + .get("operation") + .and_then(Value::as_str) + .expect("seam payload has an operation") + .to_string() + } + + fn drive(git_dir: &Path, seam: &RecordingSeam, payload: &str) -> anyhow::Result { + let resolver = |_cwd: &str| Ok(git_dir.to_path_buf()); + let seam_fn = + |_root: &Path, payload: &str, _logger: Option<&dyn Logger>| seam.handle(payload); + run_pi_mutation_scope_from_payload_with_seams(payload, None, &resolver, &seam_fn) + } + + fn tool_call_event(session_id: &str, tool_call_id: &str) -> String { + json!({ + "hook_event_name": "ToolCall", + "session_id": session_id, + "tool_call_id": tool_call_id, + "cwd": CWD, + "tool_name": "bash", + }) + .to_string() + } + + fn tool_result_event(session_id: &str, tool_call_id: &str) -> String { + json!({ + "hook_event_name": "ToolResult", + "session_id": session_id, + "tool_call_id": tool_call_id, + "cwd": CWD, + "tool_name": "bash", + }) + .to_string() + } + + fn tool_execution_end_event(session_id: &str, tool_call_id: &str) -> String { + json!({ + "hook_event_name": "ToolExecutionEnd", + "session_id": session_id, + "tool_call_id": tool_call_id, + "cwd": CWD, + "tool_name": "bash", + }) + .to_string() + } + + #[test] + fn absent_state_file_is_healthy() { + let git_dir = unique_test_git_dir("absent"); + + let health = classify_health(&git_dir); + + assert_eq!(health.adapter, ActorKind::Pi); + assert_eq!(health.status, MutationScopeHealthStatus::Healthy); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn clear_recovery_is_healthy_with_a_live_owner_pending_start_attempt() { + let git_dir = unique_test_git_dir("clear-live-owner"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + state::seed_attempt_for_tests( + &git_dir, + &key("ses-1", "call-1"), + "bash", + AttemptPhase::PendingStart, + ); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Healthy + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn clear_recovery_is_healthy_with_an_uncertain_owner_pending_start_attempt_never_swept_by_an_unrelated_start( + ) { + let git_dir = unique_test_git_dir("clear-uncertain-owner"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + let attempt = state::seed_attempt_for_tests( + &git_dir, + &key("ses-a", "call-a"), + "bash", + AttemptPhase::PendingStart, + ); + state::set_attempt_owner_for_tests( + &git_dir, + &attempt.scope_id, + ProcessOwner { + pid: std::process::id().cast_signed(), + instance_token: None, + }, + ); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Healthy, + "a live pid with no instance-token evidence is conservatively treated as alive, never dead" + ); + + let seam = RecordingSeam::new(); + drive(&git_dir, &seam, &tool_call_event("ses-c", "call-c")).expect( + "an unrelated session's Start must proceed without touching an uncertain-owner attempt", + ); + assert_eq!( + seam.calls.lock().expect("seam mutex").clone(), + vec!["start".to_string()], + "no D10 sweep may fire for an owner that cannot be positively proven dead" + ); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Healthy + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn clear_recovery_with_a_dead_owner_pending_start_attempt_is_recovering_and_an_unrelated_session_start_sweeps_it_ac4( + ) { + let git_dir = unique_test_git_dir("clear-dead-owner-pending-start"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + let seam = RecordingSeam::new(); + + drive(&git_dir, &seam, &tool_call_event("ses-a", "call-a")).expect("A starts"); + let scope_a = state::read_state(&git_dir) + .expect("state readable") + .attempts[0] + .scope_id + .clone(); + force_attempt_owner_dead_for_tests(&git_dir, &scope_a); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering, + "a dead-owner PendingStart attempt is unfinished recovery work with a proven \ + automatic sweep, not a durable wedge" + ); + + drive(&git_dir, &seam, &tool_call_event("ses-b", "call-b")) + .expect("B's Start must recover A's stale owner without ever replaying A's own key"); + + assert_eq!( + seam.calls.lock().expect("seam mutex").clone(), + vec!["start", "flush", "abandon", "flush", "start"], + "D10: the dead-owner attempt must be retired through the ordinary flush/abandon/flush \ + sequence before B's own triggering Start is admitted" + ); + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Healthy, + "once the sweep completes and B is admitted as an ordinary live attempt, no recovery \ + condition remains" + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn clear_recovery_with_a_dead_owner_executed_attempt_is_recovering_and_is_swept_without_a_synthetic_close( + ) { + let git_dir = unique_test_git_dir("clear-dead-owner-executed"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + let seam = RecordingSeam::new(); + + drive(&git_dir, &seam, &tool_call_event("ses-a", "call-a")).expect("A starts"); + drive(&git_dir, &seam, &tool_result_event("ses-a", "call-a")) + .expect("A's tool_result marks Executed"); + let state = state::read_state(&git_dir).expect("state readable"); + assert_eq!(state.attempts[0].phase, AttemptPhase::Executed); + force_attempt_owner_dead_for_tests(&git_dir, &state.attempts[0].scope_id); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering + ); + + drive(&git_dir, &seam, &tool_call_event("ses-b", "call-b")) + .expect("B's Start must recover A's dead Executed attempt"); + + assert_eq!( + seam.calls.lock().expect("seam mutex").clone(), + vec!["start", "flush", "abandon", "flush", "start"], + "ToolResult marks Executed locally without touching the seam" + ); + assert!( + !seam + .calls + .lock() + .expect("seam mutex") + .contains(&"close".to_string()), + "a dead Executed attempt must never be given a synthetic delayed Close" + ); + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Healthy + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn pending_recovery_from_a_failed_terminal_abandon_is_recovering_and_self_heals_on_the_next_start( + ) { + let git_dir = unique_test_git_dir("pending-failed-abandon"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + let crashing = RecordingSeam::failing_once_on(&["abandon"]); + + drive(&git_dir, &crashing, &tool_call_event("ses-1", "call-1")).expect("Start"); + drive( + &git_dir, + &crashing, + &tool_execution_end_event("ses-1", "call-1"), + ) + .expect("a transient abandon failure mid-recovery must not surface an error"); + + let seeded = state::read_state(&git_dir).expect("state readable"); + assert_eq!(seeded.recovery, RecoveryState::Pending { generation: 1 }); + assert_eq!(seeded.attempts[0].phase, AttemptPhase::PendingAbandon); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering, + "a proven self-healing path exists even though this state currently denies admission" + ); + + let still_failing = RecordingSeam::failing_on(&["abandon"]); + for attempt_number in 1..=2 { + let error = drive( + &git_dir, + &still_failing, + &tool_call_event("ses-other", &format!("call-retry-{attempt_number}")), + ) + .expect_err("admission while recovery is unresolved must stay fail-closed"); + assert!(error.to_string().contains(FAIL_CLOSED_MESSAGE)); + } + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering, + "repeated denial under a still-failing seam must not change the classification" + ); + + let healthy = RecordingSeam::new(); + drive(&git_dir, &healthy, &tool_call_event("ses-2", "call-2")) + .expect("recovery must self-heal and complete on the next successful invocation"); + + let resolved = state::read_state(&git_dir).expect("state readable"); + assert!(resolved.recovery.is_clear()); + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Healthy + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn pending_recovery_from_an_interrupted_dead_owner_sweep_is_recovering_and_denies_the_triggering_start_until_resumed( + ) { + let git_dir = unique_test_git_dir("pending-interrupted-sweep"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + let seam = RecordingSeam::new(); + + drive(&git_dir, &seam, &tool_call_event("ses-a", "call-a")).expect("A's Start"); + let scope_a = state::read_state(&git_dir) + .expect("state readable") + .attempts[0] + .scope_id + .clone(); + force_attempt_owner_dead_for_tests(&git_dir, &scope_a); + + let crashing = RecordingSeam::failing_once_on(&["abandon"]); + drive(&git_dir, &crashing, &tool_call_event("ses-b", "call-b")).expect_err( + "a Start that triggers a stale-owner recovery which fails mid-way must not commit", + ); + + let seeded = state::read_state(&git_dir).expect("state readable"); + assert_eq!(seeded.recovery, RecoveryState::Pending { generation: 1 }); + assert!(!seeded.attempts.iter().any(|a| a.session_id == "ses-b")); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering, + "an interrupted dead-owner sweep still has a proven resumption path on the next \ + tracked Start" + ); + + drive(&git_dir, &crashing, &tool_call_event("ses-b", "call-b")).expect( + "the next boundary-lock acquisition must resume and complete the pending recovery, \ + then admit B", + ); + + let resolved = state::read_state(&git_dir).expect("state readable"); + assert!(resolved.recovery.is_clear()); + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Healthy + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn orphaned_flushing_is_recovering_and_reclaimed_by_the_next_boundary() { + let git_dir = unique_test_git_dir("orphaned-flushing"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + + let attempt = state::seed_attempt_for_tests( + &git_dir, + &key("ses-stuck", "call-stuck"), + "bash", + AttemptPhase::PendingStart, + ); + state::begin_terminal_cleanup(&git_dir, std::slice::from_ref(&attempt.scope_id)) + .expect("seeding an orphaned flush should succeed"); + assert!(matches!( + state::read_state(&git_dir) + .expect("state readable") + .recovery, + RecoveryState::Flushing { .. } + )); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering, + "a crash mid-flush leaves an orphaned Flushing that the next boundary reclaims" + ); + + let healthy = RecordingSeam::new(); + drive(&git_dir, &healthy, &tool_call_event("ses-new", "call-new")) + .expect("the orphaned flush is reclaimed and retried, then the new call is admitted"); + + let resolved = state::read_state(&git_dir).expect("state readable"); + assert!(resolved.recovery.is_clear()); + assert!(resolved + .attempts + .iter() + .all(|a| a.session_id != "ses-stuck")); + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Healthy + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn clear_recovery_with_a_pending_abandon_attempt_is_a_structurally_impossible_state_classified_invalid( + ) { + let git_dir = unique_test_git_dir("clear-with-pending-abandon-invalid"); + let dir = state::adapter_state_dir(&git_dir); + std::fs::create_dir_all(&dir).expect("state dir should be created"); + std::fs::write( + state::state_path(&git_dir), + serde_json::json!({ + "version": 2, + "next_attempt_seq": 2, + "next_recovery_generation": 1, + "recovery": { "phase": "clear" }, + "attempts": [{ + "attempt_seq": 1, + "scope_id": "pi-tool-v1|n=1|s=5:ses-1|c=6:call-1", + "session_id": "ses-1", + "tool_call_id": "call-1", + "tool_name": "bash", + "phase": "pending_abandon", + "owner": { "pid": 999_999, "instance_token": null }, + }], + }) + .to_string(), + ) + .expect("hand-seeded state file should be writable"); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Invalid + ); + + remove_test_git_dir(&git_dir); + } + + #[test] + fn malformed_state_file_is_invalid_with_the_read_error_surfaced() { + let git_dir = unique_test_git_dir("malformed"); + let dir = state::adapter_state_dir(&git_dir); + std::fs::create_dir_all(&dir).expect("state dir should be created"); + std::fs::write(state::state_path(&git_dir), b"not json") + .expect("malformed file should be writable"); + + let health = classify_health(&git_dir); + + assert_eq!(health.status, MutationScopeHealthStatus::Invalid); + assert!( + health + .detail + .as_deref() + .unwrap_or_default() + .contains("malformed"), + "the read error must be surfaced in the detail: {:?}", + health.detail + ); + + remove_test_git_dir(&git_dir); + } + + fn matrix_attempt( + call_id: &str, + phase: AttemptPhase, + owner: ProcessOwner, + ) -> state::AdapterAttempt { + let attempt_key = key("ses-main", call_id); + state::AdapterAttempt { + attempt_seq: 1, + scope_id: super::super::format_pi_scope_id(&attempt_key, 1), + session_id: attempt_key.session_id, + tool_call_id: attempt_key.tool_call_id, + tool_name: "bash".to_string(), + phase, + owner, + } + } + + fn write_matrix_state( + git_dir: &Path, + recovery: RecoveryState, + has_pending_abandon: bool, + has_dead_owner_attempt: bool, + live_owner: ProcessOwner, + dead_owner: ProcessOwner, + ) { + std::fs::create_dir_all(state::adapter_state_dir(git_dir)) + .expect("adapter state dir should be created"); + + let mut attempts = vec![matrix_attempt( + "call-live", + AttemptPhase::PendingStart, + live_owner, + )]; + if has_pending_abandon { + attempts.push(matrix_attempt( + "call-abandon", + AttemptPhase::PendingAbandon, + live_owner, + )); + } + if has_dead_owner_attempt { + attempts.push(matrix_attempt( + "call-dead", + AttemptPhase::Executed, + dead_owner, + )); + } + + let state = state::AdapterState { + version: 2, + next_attempt_seq: 2, + next_recovery_generation: 2, + recovery, + attempts, + }; + std::fs::write( + state::state_path(git_dir), + serde_json::to_string(&state).expect("matrix state serializes"), + ) + .expect("hand-built matrix state should be writable"); + } + + #[test] + fn health_classification_matrix_covers_all_twelve_recovery_and_attempt_condition_combinations() + { + use MutationScopeHealthStatus::{Healthy, Invalid, Recovering}; + + let mut dead_child = std::process::Command::new("true") + .spawn() + .expect("spawning 'true' should succeed"); + let dead_pid = i32::try_from(dead_child.id()).expect("pid fits in i32"); + dead_child.wait().expect("child should exit and be reaped"); + let dead_owner = ProcessOwner { + pid: dead_pid, + instance_token: None, + }; + let live_owner = super::super::process_owner::current_process_owner(); + + let clear = RecoveryState::Clear; + let pending = RecoveryState::Pending { generation: 1 }; + let flushing = RecoveryState::Flushing { generation: 1 }; + + let rows = [ + (clear, false, false, Healthy), + (clear, false, true, Recovering), + (clear, true, false, Invalid), + (clear, true, true, Invalid), + (pending, false, false, Recovering), + (pending, false, true, Recovering), + (pending, true, false, Recovering), + (pending, true, true, Recovering), + (flushing, false, false, Recovering), + (flushing, false, true, Recovering), + (flushing, true, false, Recovering), + (flushing, true, true, Recovering), + ]; + + for (index, (recovery, has_pending_abandon, has_dead_owner_attempt, expected)) in + rows.into_iter().enumerate() + { + let git_dir = unique_test_git_dir(&format!("matrix-{index}")); + write_matrix_state( + &git_dir, + recovery, + has_pending_abandon, + has_dead_owner_attempt, + live_owner, + dead_owner, + ); + + assert_eq!( + classify_health(&git_dir).status, + expected, + "row {index}: recovery={recovery:?} has_pending_abandon={has_pending_abandon} \ + has_dead_owner_attempt={has_dead_owner_attempt}", + ); + + remove_test_git_dir(&git_dir); + } + } + + #[test] + fn pending_recovery_reuses_a_duplicate_start_for_an_existing_nonterminal_key_without_advancing_recovery_then_a_fresh_start_recovers( + ) { + let git_dir = unique_test_git_dir("pending-duplicate-start-vs-fresh-start"); + std::fs::create_dir_all(&git_dir).expect("git dir should be created"); + + let setup = RecordingSeam::new(); + drive(&git_dir, &setup, &tool_call_event("ses-b", "call-b")).expect("B's Start"); + drive(&git_dir, &setup, &tool_call_event("ses-a", "call-a")).expect("A's Start"); + + let crashing_abandon = RecordingSeam::failing_once_on(&["abandon"]); + drive( + &git_dir, + &crashing_abandon, + &tool_execution_end_event("ses-a", "call-a"), + ) + .expect("a transient abandon failure mid-recovery must not surface an error"); + + let seeded = state::read_state(&git_dir).expect("state readable"); + assert_eq!(seeded.recovery, RecoveryState::Pending { generation: 1 }); + let phase_by_session = |session_id: &str| { + seeded + .attempts + .iter() + .find(|attempt| attempt.session_id == session_id) + .expect("attempt for session must exist") + .phase + }; + assert_eq!(phase_by_session("ses-a"), AttemptPhase::PendingAbandon); + assert_eq!(phase_by_session("ses-b"), AttemptPhase::PendingStart); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering + ); + + let duplicate = RecordingSeam::new(); + drive(&git_dir, &duplicate, &tool_call_event("ses-b", "call-b")) + .expect("a duplicate Start for an already-tracked nonterminal key stays idempotent"); + + assert_eq!( + duplicate.calls.lock().expect("seam mutex").clone(), + vec!["start".to_string()], + "a duplicate Start for B's own key must be reused without touching recovery" + ); + let after_duplicate = state::read_state(&git_dir).expect("state readable"); + assert_eq!( + after_duplicate.recovery, + RecoveryState::Pending { generation: 1 }, + "recovery must remain Pending: the duplicate Start never reached the recovery-state gate" + ); + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Recovering, + "reusing B's key must not change the classification" + ); + + let fresh = RecordingSeam::new(); + drive(&git_dir, &fresh, &tool_call_event("ses-c", "call-c")).expect( + "C's fresh Start must claim and complete the pending recovery, then be admitted", + ); + + assert_eq!( + fresh.calls.lock().expect("seam mutex").clone(), + vec!["flush", "abandon", "flush", "start"], + "C claims the Pending generation, resolve_recovery retires A, then C's own Start is admitted" + ); + let resolved = state::read_state(&git_dir).expect("state readable"); + assert!(resolved.recovery.is_clear()); + assert!(!resolved.attempts.iter().any(|a| a.session_id == "ses-a")); + assert!(resolved.attempts.iter().any(|a| a.session_id == "ses-b")); + assert!(resolved.attempts.iter().any(|a| a.session_id == "ses-c")); + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Healthy + ); + + remove_test_git_dir(&git_dir); + } +} diff --git a/cli/src/services/hooks/pi_mutation_scope/mod.rs b/cli/src/services/hooks/pi_mutation_scope/mod.rs index bfa4e855b..a75a3e749 100644 --- a/cli/src/services/hooks/pi_mutation_scope/mod.rs +++ b/cli/src/services/hooks/pi_mutation_scope/mod.rs @@ -1,8 +1,9 @@ #![allow(dead_code)] mod boundary_lock; +pub(crate) mod health; mod os_lock; -mod process_owner; +pub(crate) mod process_owner; pub(crate) mod state; use std::path::{Path, PathBuf}; diff --git a/cli/src/services/hooks/pi_mutation_scope/state.rs b/cli/src/services/hooks/pi_mutation_scope/state.rs index c7d4bc2eb..3c9ca81d6 100644 --- a/cli/src/services/hooks/pi_mutation_scope/state.rs +++ b/cli/src/services/hooks/pi_mutation_scope/state.rs @@ -120,7 +120,7 @@ pub(crate) fn adapter_state_dir(git_dir: &Path) -> PathBuf { git_dir.join(SCE_STATE_DIR) } -fn state_path(git_dir: &Path) -> PathBuf { +pub(crate) fn state_path(git_dir: &Path) -> PathBuf { adapter_state_dir(git_dir).join(ADAPTER_STATE_FILE) } diff --git a/context/cli/claude-mutation-scope-integration.md b/context/cli/claude-mutation-scope-integration.md index 3256f88c8..995c2f502 100644 --- a/context/cli/claude-mutation-scope-integration.md +++ b/context/cli/claude-mutation-scope-integration.md @@ -180,6 +180,68 @@ runs one `{"operation":"flush"}` through the seam — one worktree-level recovery/rebaseline boundary. Only a successful `flush` clears `recovery_pending`; a failed `flush` stays fail-closed. +## Mutation-scope health + +`claude_mutation_scope::health::classify_health` is a read-only diagnostic +classifier over the checkout-local adapter state described above. It returns +the shared `healthy | recovering | blocked | invalid` health vocabulary that +the planned doctor integration will consume. It reads the same +`state::read_state` result the adapter itself uses and maps it as: + +| Persisted state | Status | Reason | +| --- | --- | --- | +| `recovery_pending == false` | `Healthy` | no persisted recovery barrier is armed | +| `recovery_pending == true && attempts.is_empty()` | `Recovering` | the recovery barrier's own flush path can clear this automatically | +| `recovery_pending == true && attempts` non-empty | `Blocked` | the recovery barrier's flush path never runs from this shape, and nothing else advances it | +| `read_state` fails (malformed JSON, unsupported version, read error) | `Invalid` | the state cannot be safely interpreted | + +**Healthy** covers the absence of a state file (`read_state`'s default) as +well as an explicit `recovery_pending == false`. This is recovery health +specifically, not "no active tool calls" — the adapter may still have live +`attempts` in `pending_start`/`active` phase; live attempts alone, with +`recovery_pending == false`, are still `Healthy`. + +**Recovering** (`recovery_pending == true && attempts.is_empty()`) reflects +actual adapter behavior, not the `recovery_pending` name alone: the next +mutation-capable `PreToolUse` reaches [the recovery barrier](#the-recovery-barrier) +above, finds `attempts` empty, runs `{"operation":"flush"}` through the seam, +and — on success — calls `clear_recovery_pending` before proceeding. This is +a normal, self-healing admission path with no manual intervention. + +**Blocked** (`recovery_pending == true && attempts` non-empty) is the exact +shape of the incident that motivated this classifier and the wider +`doctor-mutation-scope-health` plan. `apply_recovery_barrier()` sees +`recovery_pending == true` with non-empty `attempts` and returns `Deny`. It +does not retry the abandon that left those attempts stale, does not remove +them, does not flush, and does not clear `recovery_pending` — the barrier's +only self-healing transition (flush) is gated on `attempts.is_empty()`, which +this shape never satisfies. So once the hook invocation that produced this +persisted state has returned, every subsequent ordinary mutation-capable +`PreToolUse` continues to deny without advancing recovery — a durable +repository-wide lockout, not merely "currently denied." + +**Invalid** applies when `state::read_state` cannot safely read or parse the +state file — malformed JSON or an unsupported/invalid persisted version, per +the existing state reader. A fail-closed but structurally valid state (i.e. +`Blocked`) is never reported as `Invalid`. + +**Read-only boundary.** This classifier is diagnostic only: it reads the +existing checkout-local adapter state and nothing else. It never modifies +`attempts`, never clears `recovery_pending`, never calls `flush` or +`abandon`, and never alters mutation attribution. Recovery/repair is a +separate concern this classifier does not perform. + +**Observation semantics.** Classification is a snapshot of persisted state, +read the same way `state::read_state` reads it for the adapter's own use. +There is a narrow window in which a currently executing hook has already +persisted `recovery_pending = true` with non-empty `attempts` but is still +about to complete its abandon/remove sequence — the classifier does not +attempt to prove global process liveness across that window. `Blocked` means +that, if the operation that produced the observed durable state has stopped +progressing, future ordinary adapter lifecycle events have no self-healing +path from that state — not a claim that no process anywhere could possibly +still be mid-write. + ## Raw cwd is authoritative The runtime's repository root is the raw payload's `cwd`, never diff --git a/context/cli/codex-mutation-scope-health.md b/context/cli/codex-mutation-scope-health.md new file mode 100644 index 000000000..8d5997ae3 --- /dev/null +++ b/context/cli/codex-mutation-scope-health.md @@ -0,0 +1,88 @@ +# Codex mutation-scope health classification + +`cli/src/services/hooks/codex_mutation_scope/health.rs` maps the adapter's +persisted `/sce/codex-mutation-scope-state.json` (see +[codex-mutation-scope-integration.md](codex-mutation-scope-integration.md#recovery-and-durable-state)) +onto the shared, doctor-facing `healthy | recovering | blocked | invalid` +vocabulary. The classifier is pure and read-only: it never writes state, and +`doctor` wiring is a separate task. + +Every mapping below was proven by driving the adapter's real dispatch and +recovery functions (`admit_tracked_attempt`, `sweep_stale_lane_predecessors`, +`cleanup_attempts_matching`, `normalize_recovery_after_boundary_lock_acquired`), +never inferred from `RecoveryState` variant names. + +## The mapping + +| Persisted shape | Status | Why | +| --- | --- | --- | +| No state file | `healthy` | No adapter attempt has ever run; absence is not a problem. | +| `Clear`, any attempts | `healthy` | Live `PendingStart`/`Active` attempts are ordinary in-flight lifecycle state, not a recovery condition; new tracked admission still proceeds outside the exact same `(session_id, turn_id)` lane. | +| `Pending { generation }`, attempts empty | `recovering` | The next tracked `PreToolUse`, from any session, claims the flush (`AdmitDecision::FlushClaimed`) and clears recovery on success. | +| `Pending { generation }`, attempts non-empty | `recovering` | See [Why non-empty `Pending` is recovering, not blocked](#why-non-empty-pending-is-recovering-not-blocked). | +| `Flushing { generation }`, attempts empty | `recovering` | The only reachable path into `Flushing` always leaves attempts empty. An orphaned `Flushing` (its owning process gone) is reclaimed to `Pending` by `normalize_recovery_after_boundary_lock_acquired` on the next process to acquire the boundary lock, then retried as a fresh flush. | +| `Flushing { generation }`, attempts non-empty | `invalid` | Structurally impossible through production behavior: the only transition into `Flushing` requires attempts to already be empty, and `admit_tracked_attempt` refuses every new attempt while `Flushing`. A hand-seeded or corrupted file matching this shape is reported `invalid`, not `blocked`. | +| Read/parse error (missing file aside) | `invalid` | Malformed JSON, unsupported version, or a read failure; the error is surfaced in the health record's detail. | + +## Why non-empty `Pending` is recovering, not blocked + +This is the shape of the incident this feature exists to surface (see the +`doctor-mutation-scope-health` plan's change summary), and Codex can reach it +the same way Claude can: `abandon_attempt` calls `state::arm_recovery` *before* +invoking the ingress `abandon` seam, then removes the attempt only after the +seam call succeeds. If the seam call fails — during `PostToolUse` close, +`Stop`/`Interrupt`/`SubagentStop`/`SessionEnd` cleanup, or same-lane +predecessor sweeping — the error propagates and `remove_attempt` never runs, +leaving `Pending { generation }` with the stale attempt still present. + +The production `PreToolUse` order is: + +```text +with_boundary_lock + -> normalize_recovery_after_boundary_lock_acquired + -> sweep_stale_lane_predecessors + -> admit_or_recover + -> admit_tracked_attempt +``` + +The global `RecoveryBlocked` decision lives inside `admit_tracked_attempt`, +which `admit_or_recover` calls only *after* `sweep_stale_lane_predecessors` +has already run for this call. So a normal future tracked `PreToolUse` sharing +the stuck attempt's exact `(session_id, turn_id)` lane retries the stale +predecessor's abandonment (via the ordinary same-lane sweep) *before* it ever +reaches the recovery barrier. If that retried abandon succeeds, the stale +attempt is removed, `attempts` becomes empty, and the same call's own +`admit_tracked_attempt` invocation observes `Pending` with no attempts, +claims the flush (`AdmitDecision::FlushClaimed`), and — once the flush seam +call succeeds — clears recovery and admits itself. That is an existing, +ordinary, automatic recovery path, proven by driving it end to end; it is not +inferred from the enum variant name. + +This does not mean the state is harmless in the meantime. Until that same-lane +successor arrives (or the stuck attempt's own turn produces another matching +lifecycle event), every *unrelated* tracked `PreToolUse` — a different session, +or the same session with a different `turn_id` — still reaches +`admit_tracked_attempt` with `attempts` non-empty and is denied with +`RecoveryBlocked`. The classifier's job is to distinguish "this persisted +state has a proven normal self-healing route" (`recovering`) from "no future +ordinary event can advance it" (`blocked`); it is not to claim every future +call will succeed. Non-empty `Pending` satisfies the former, not the latter, +so it is `recovering`. + +Proven in `cli/src/services/hooks/codex_mutation_scope/mod.rs`: + +- `pending_non_empty_recovery_denies_unrelated_admission_without_losing_the_recovery_path`: + a failed abandon during `SessionEnd` cleanup leaves the state in this shape, + `classify_health` reports `recovering`, and two successive real `PreToolUse` + calls from an unrelated session are both denied without the ingress seam + ever being invoked — `recovering` does not mean unrelated admission + succeeds, only that a proven self-healing route exists. +- `same_lane_successor_retries_abandon_and_reaches_healthy_after_a_failed_lifecycle_abandon_ac4`: + the real same-lane self-healing route that invalidates the old `blocked` + classification. A tracked attempt `A` is stranded by a failed abandon; a + later same-lane `PreToolUse` `B` retries the abandon and also fails closed + (`recovering` persists); a further same-lane `PreToolUse` `C`, through the + ordinary same-lane sweep this time succeeding, retries and clears `A`'s + abandon, claims and completes the flush, and is itself admitted and started + — driving the real `abandon(A) -> flush -> start(C)` seam-call ordering and + ending with `classify_health` reporting `healthy`. diff --git a/context/cli/codex-mutation-scope-integration.md b/context/cli/codex-mutation-scope-integration.md index 2585c36ec..96bb7fd60 100644 --- a/context/cli/codex-mutation-scope-integration.md +++ b/context/cli/codex-mutation-scope-integration.md @@ -148,7 +148,8 @@ Positive cleanup arms `recovery_pending`, abandons matching tracked scopes via generic `abandon`, and removes settled attempts. Known attempts keep tracked `PreToolUse` denied; once empty, one generic `flush` re-baselines and clears recovery. Flush failure leaves the barrier armed. Untracked events never enter -or are blocked by this barrier. +or are blocked by this barrier. Doctor health classification built on this +state machine is proven in [codex-mutation-scope-health.md](codex-mutation-scope-health.md). The cleanup matrix is identity-scoped: `Stop` covers the session's main agent, `Interrupt` and `SessionEnd` cover the session, and `SubagentStop` covers one diff --git a/context/cli/opencode-mutation-scope-health.md b/context/cli/opencode-mutation-scope-health.md new file mode 100644 index 000000000..df91473a5 --- /dev/null +++ b/context/cli/opencode-mutation-scope-health.md @@ -0,0 +1,211 @@ +# OpenCode mutation-scope health classification + +`cli/src/services/hooks/opencode_mutation_scope/health.rs` maps the adapter's +persisted `/sce/opencode-mutation-scope-state.json` (see +[opencode-mutation-scope-integration.md](opencode-mutation-scope-integration.md#adapter-lifecycle-and-recovery)) +onto the shared, doctor-facing `healthy | recovering | blocked | invalid` +vocabulary. The classifier is pure and read-only: it never writes state, and +`doctor` wiring is a separate task. + +Every mapping below was proven by driving the adapter's real dispatch and +recovery functions (`admit_tracked_attempt`, `establish_tracked_start`, +`resolve_recovery`, `normalize_recovery_after_boundary_lock_acquired`, +`begin_terminal_cleanup`), never inferred from `RecoveryState`/`AttemptPhase` +variant names. + +## The mapping + +`has_pending_start` and `has_pending_abandon` below mean "at least one +persisted attempt is in that phase," independent of how many other attempts +exist in other phases (including each other). The classifier's evaluation +order matters: a `PendingAbandon` under `Clear` is checked first (it is +structurally impossible, so it wins as `invalid` even if a `PendingStart` is +also present), then `PendingStart` is checked *regardless of `RecoveryState`* +(so it takes precedence over `Pending`/`Flushing` recovery, not just over +`Clear`), and only once both attempt-phase checks are exhausted does the +`RecoveryState` alone decide `healthy` vs. `recovering`. + +| Persisted shape | Status | Why | +| --- | --- | --- | +| No state file | `healthy` | No adapter attempt has ever run; absence is not a problem. | +| `Clear`, no `PendingStart`, no `PendingAbandon` (attempts empty or only `Active`) | `healthy` | Live `Active` attempts are ordinary in-flight lifecycle state, not a recovery condition. | +| An attempt is `PendingStart`, for *any* `RecoveryState` (`Clear`, `Pending`, or `Flushing`, with or without a concurrently outstanding `PendingAbandon`) | `blocked` | See [Why a stale `PendingStart` is blocked, not recovering — even alongside a Pending/Flushing recovery generation](#why-a-stale-pendingstart-is-blocked-not-recovering--even-alongside-a-pendingflushing-recovery-generation). | +| `Pending { generation }`, no `PendingStart` (attempts empty or only `PendingAbandon`) | `recovering` | See [Why `Pending` is recovering when no `PendingStart` is outstanding](#why-pending-is-recovering-when-no-pendingstart-is-outstanding). | +| `Flushing { generation }`, no `PendingStart` (attempts empty or only `PendingAbandon`) | `recovering` | Every `PreToolUse`-equivalent boundary (`ToolExecuteBefore`, `ShellEnv`, `ToolExecuteAfter`, `ToolError`) calls `normalize_recovery_after_boundary_lock_acquired` under the same boundary lock before doing anything else, which unconditionally reclaims an observed `Flushing` back to `Pending`. A live in-progress flush is never observable at rest (the boundary lock is held for the whole flush); only an orphaned one (crashed mid-flush) is ever seen by doctor. The next tracked boundary reclaims it to `Pending`; a subsequent recovery-capable tracked admission then claims that pending generation and retries `resolve_recovery`. | +| `Clear`, an attempt is `PendingAbandon` (with or without a `PendingStart` also present) | `invalid` | Structurally impossible through production behavior: `PendingAbandon` is set only inside `begin_terminal_cleanup`, which atomically arms `Flushing` in the same write; recovery only returns to `Clear` via `complete_recovery_flush`, which runs only after every `PendingAbandon` attempt in `resolve_recovery`'s loop has already been abandoned and removed. A hand-seeded or corrupted file matching this shape is reported `invalid`. | +| Read/parse error (missing file aside) | `invalid` | Malformed JSON, unsupported version, or a read failure; the error is surfaced in the health record's detail. | + +### The state matrix in full + +The classifier's two boolean attempt-phase facts (`has_pending_start`, +`has_pending_abandon`) times the three `RecoveryState` variants give twelve +combinations. `Active` attempts never affect the answer, so they are omitted +below. Reachability was checked by driving the real dispatcher, not inferred. +All twelve cells are additionally locked down as a completeness proof by +`health_classification_matrix_covers_all_twelve_recovery_and_attempt_phase_combinations` +in `cli/src/services/hooks/opencode_mutation_scope/health.rs`, which asserts +the classifier's output for every row against hand-built state (including an +always-present `Active` attempt, proving it never changes the result); the +real-dispatch regressions below separately prove *why* the semantically +meaningful equivalence classes have those classifications by driving +production code, not just the classifier's output: + +| `RecoveryState` | `PendingAbandon`? | `PendingStart`? | Reachable in production? | Status | What advances it | +| --- | --- | --- | --- | --- | --- | +| `Clear` | no | no | yes (idle / only `Active`) | `healthy` | n/a | +| `Clear` | no | yes | yes | `blocked` | only that exact call's own `ToolExecuteAfter`/`ToolError`, or a duplicate `Start` redelivery — nothing else | +| `Clear` | yes | no | **no** (structurally impossible) | `invalid` | n/a — hand-seeded/corrupted only | +| `Clear` | yes | yes | **no** (structurally impossible, same reason) | `invalid` | n/a — hand-seeded/corrupted only | +| `Pending` | no | no | yes (abandon succeeded, rebaseline flush then failed) | `recovering` | next tracked admission retries the rebaseline flush | +| `Pending` | yes | no | yes (ambiguity flush or abandon itself failed) | `recovering` | next tracked admission retries `resolve_recovery` for every `PendingAbandon` attempt | +| `Pending` | no | yes | yes (as above, with an unrelated stale `PendingStart` also outstanding) | `blocked` | recovery itself still advances and can reach `Clear`, but the `PendingStart` survives it — see the critical regression below | +| `Pending` | yes | yes | yes (this task's critical regression) | `blocked` | same as above: `resolve_recovery` clears the `PendingAbandon`, `PendingStart` is untouched | +| `Flushing` | no | no | yes (orphaned crash mid-flush, no doomed attempts left) | `recovering` | next tracked boundary reclaims `Flushing` → `Pending`; a subsequent recovery-capable tracked admission claims that generation and runs `resolve_recovery` | +| `Flushing` | yes | no | yes (orphaned crash mid-flush) | `recovering` | same reclaim-then-claim-and-retry path | +| `Flushing` | no | yes | yes (orphaned crash mid-flush, unrelated stale `PendingStart`) | `blocked` | recovery is reclaimed and can still clear, `PendingStart` does not | +| `Flushing` | yes | yes | yes (this task's Case B) | `blocked` | same as the `Pending`/mixed row: recovery clears, `PendingStart` survives | + +The four `blocked` rows are one equivalence class under a single invariant: +`resolve_recovery` (`cli/src/services/hooks/opencode_mutation_scope/mod.rs`) +only ever iterates attempts already in `PendingAbandon`; it never inspects or +retires a `PendingStart`. So whenever any `PendingStart` is outstanding, +*no* value of `RecoveryState` — `Clear`, `Pending`, or `Flushing`, and +regardless of whether a `PendingAbandon` is also outstanding — has a future +ordinary event that retires it. The classifier therefore checks +`has_pending_start` as one condition spanning all of `RecoveryState`, not as +a per-recovery-state special case; see +[`classify_health`](../../cli/src/services/hooks/opencode_mutation_scope/health.rs). + +## Why `Pending` is recovering when no `PendingStart` is outstanding + +Unlike Codex's same-lane-scoped sweep, OpenCode's recovery retry is not scoped +to any particular session or call. `admit_tracked_attempt`'s `Pending { g }` +arm unconditionally transitions to `Flushing { g }` and returns +`AdmitDecision::FlushClaimed`, regardless of which call is being admitted or +whether any `PendingAbandon` attempts are outstanding: + +```text +establish_tracked_start + -> with_boundary_lock + -> normalize_recovery_after_boundary_lock_acquired + -> admit_or_recover + -> admit_tracked_attempt (Pending{g} -> Flushing{g}, FlushClaimed) + -> resolve_recovery (flush, abandon every PendingAbandon, flush, complete) +``` + +`resolve_recovery` re-reads the persisted attempts and retries `abandon` for +**every** attempt still in `PendingAbandon`, not just one tied to the admitting +call. So the very next tracked admission — from any session, any call — always +attempts full recovery resolution. If every seam call in that retry succeeds, +recovery clears to `Clear`, the stale attempts are removed, and the admitting +call itself proceeds. If any step fails, `relinquish_recovery_flush` returns +the state to `Pending` and the call is denied, but the same automatic retry +fires again on the next tracked admission. This holds whether `attempts` is +empty (a prior rebaseline-flush failure after a successful abandon, e.g. an +interrupted `regression_c`-shaped sequence) or non-empty with only +`PendingAbandon` attempts (an abandon itself failed, e.g. +`regression_a`/`regression_b`-shaped sequences): either way, a normal future +tracked admission has a proven, existing path to advance recovery itself +all the way to `Clear`. + +That guarantee is about `RecoveryState` alone, though, and it is **not** +sufficient by itself to guarantee a return to normal admission: `PendingStart` +is a second, independent durable-wedge condition that `resolve_recovery` +never inspects (see +[Why a stale `PendingStart` is blocked, not recovering](#why-a-stale-pendingstart-is-blocked-not-recovering--even-alongside-a-pendingflushing-recovery-generation) +below). So this section's claim is scoped precisely: `Pending`/`Flushing` +recovery is `recovering` only when no `PendingStart` attempt is also +outstanding. When one is, the classifier reports `blocked` even while +recovery is actively advancing (or has already reached `Clear`) for an +unrelated `PendingAbandon`, because reaching `Clear` on the recovery +generation does not by itself restore normal admission. + +Proven in `cli/src/services/hooks/opencode_mutation_scope/health.rs`: + +- `pending_recovery_with_pending_abandon_attempts_is_recovering_and_an_unrelated_admission_clears_it`: + a failed `abandon` during `ToolError` cleanup leaves `Pending` with a + `PendingAbandon` attempt and no `PendingStart`; `classify_health` reports + `recovering`; two further admissions under the same still-failing seam are + denied without changing the classification; then an unrelated call, once + the seam succeeds, clears the stale attempt and is itself admitted. +- `pending_recovery_with_empty_attempts_is_recovering_and_the_next_admission_clears_it`: + a successful `abandon` followed by a failed rebaseline `flush` leaves + `Pending` with `attempts` already empty; the next unrelated call's admission + still resolves it. +- `orphaned_flushing_with_pending_abandon_attempts_is_recovering_and_reclaimed_by_the_next_boundary`: + a `Flushing` state seeded to simulate a crash mid-flush, with no + `PendingStart` present, is reclaimed to `Pending` and retried by the next + tracked admission. + +## Why a stale `PendingStart` is blocked, not recovering — even alongside a Pending/Flushing recovery generation + +This is the OpenCode-specific shape of the incident this feature exists to +surface (see the `doctor-mutation-scope-health` plan's change summary). +`AttemptPhase::PendingStart` sits outside the `RecoveryState` machine +entirely: `admit_tracked_attempt`'s `RecoveryState::Clear` arm denies every +*new* admission with `AdmitDecision::UncertainAttemptBlocked` whenever any +attempt is still `PendingStart`, and — critically — `resolve_recovery`'s +retry loop (the mechanism that drives `Pending`/`Flushing` back to `Clear`) +only ever iterates attempts already in `AttemptPhase::PendingAbandon`; it +never inspects or retires a `PendingStart` attempt, regardless of which +recovery generation is in flight or whether that generation is for a +completely unrelated call. Nothing in the adapter clears a `PendingStart` +attempt on behalf of an unrelated call. The only two paths that retire a +`PendingStart` attempt are keyed to that exact `(session_id, call_id)`: + +- a duplicate `ToolExecuteBefore` redelivery for the same key (idempotent + replay, not a recovery mechanism), or +- that same key's own `ToolExecuteAfter`/`ToolError`, which drives + `abandon_and_consume` for that specific attempt. + +If the process that owns that call has died before either of those arrives — +there is no equivalent of Pi's `ProcessOwner`/`is_definitely_dead()` liveness +check in OpenCode, and `server_disposed_cannot_sweep_another_processes_attempt` +proves `ServerDisposed` is deliberately inert for this — no future ordinary +lifecycle event from any other call can ever clear it. This is the same +"valid persisted state + fail-closed admission + no reachable self-healing +transition" shape as the Claude incident, just triggered by `PendingStart` +instead of a non-empty `attempts` list under `recovery_pending`. + +**This holds even when recovery is simultaneously `Pending` or `Flushing` for +an unrelated `PendingAbandon` attempt.** Recovery reaching `Clear` retires +only the `PendingAbandon` attempts it processed; it says nothing about any +`PendingStart` attempt that was never in its retry loop. A state can +therefore make visible progress on one axis (recovery generation advancing, +even completing) while remaining durably wedged on the other (an unrelated +`PendingStart` that never gets swept). The classifier's ordering reflects +this: `has_pending_start` is checked as a single condition spanning every +`RecoveryState` value, not nested inside the `Clear` arm. + +Proven in `cli/src/services/hooks/opencode_mutation_scope/health.rs`: + +- `pending_start_with_clear_recovery_is_blocked_and_denies_repeated_unrelated_admissions_ac4`: + a `Start` whose seam call fails leaves the attempt `PendingStart` with + recovery `Clear`; `classify_health` reports `blocked`; two further + admissions from unrelated calls, under a since-healthy seam, are both + denied without self-clearing; and `classify_health` still reports `blocked` + afterward. +- `pending_recovery_with_a_pending_start_attempt_is_blocked_even_though_an_unrelated_pending_abandon_can_still_clear_ac4` + (the critical regression this task exists to fix): call A starts and is + then abandoned via `ToolError`; its terminal cleanup arms recovery + `Flushing`, but the ambiguity flush fails and `relinquish_recovery_flush` + leaves it `Pending`, with A `PendingAbandon`. Independently, call B's own + `Start` seam failed earlier, leaving B `PendingStart` under what was then + `Clear` recovery. The persisted state — `Pending` recovery, A + `PendingAbandon`, B `PendingStart` — is asserted `blocked`, not + `recovering`. An unrelated call C then triggers real recovery resolution: + `resolve_recovery` abandons A and drives recovery to `Clear`, but C is + still denied `UncertainAttemptBlocked` because B is untouched; the + persisted state is asserted to be `Clear` + B still `PendingStart`, and + `classify_health` still reports `blocked`. A further unrelated call D is + denied again, proving the adapter does not self-clear. +- `orphaned_flushing_with_a_pending_start_attempt_is_blocked_not_recovering_ac4` + (the `Flushing` analog / Case B): the same shape, but the crash is + captured as an orphaned `Flushing` (via a direct `begin_terminal_cleanup` + call simulating a crash before `resolve_recovery` ever ran — the only way + to observe a literal `Flushing` at rest, since every in-process caller + calls `resolve_recovery` immediately afterward under the same boundary + lock). `classify_health` reports `blocked` while `Flushing`; a subsequent + unrelated call reclaims `Flushing` → `Pending`, resolves A's recovery to + `Clear`, and is itself denied because B's `PendingStart` survives; + `classify_health` still reports `blocked` afterward. diff --git a/context/cli/opencode-mutation-scope-integration.md b/context/cli/opencode-mutation-scope-integration.md index 27b455f92..9ec86df30 100644 --- a/context/cli/opencode-mutation-scope-integration.md +++ b/context/cli/opencode-mutation-scope-integration.md @@ -184,6 +184,8 @@ generation-tracked recovery barrier that retries transient cleanup failures, and no same-session sweep or TTL. Broad asynchronous lifecycle events retire nothing. No protocol or Quint change. Full detail: [`opencode-mutation-scope-adapter-lifecycle.md`](opencode-mutation-scope-adapter-lifecycle.md). +Doctor health classification built on this state machine is proven in +[opencode-mutation-scope-health.md](opencode-mutation-scope-health.md). ## Attribution boundary diff --git a/context/cli/pi-mutation-scope-health.md b/context/cli/pi-mutation-scope-health.md new file mode 100644 index 000000000..b44729c72 --- /dev/null +++ b/context/cli/pi-mutation-scope-health.md @@ -0,0 +1,123 @@ +# Pi mutation-scope health classification + +`cli/src/services/hooks/pi_mutation_scope/health.rs` maps the adapter's +persisted `/sce/pi-mutation-scope-state.json` (see +[pi-mutation-scope-integration.md](pi-mutation-scope-integration.md#durable-state)) +onto the shared, doctor-facing `healthy | recovering | blocked | invalid` +vocabulary. The classifier is pure and read-only: it never writes state, and +`doctor` wiring is a separate task. + +Every mapping below was proven by driving the adapter's real dispatch and +recovery functions (`admit_or_recover`, `reconcile_stale_owners`, +`resolve_recovery`, `normalize_recovery_after_boundary_lock_acquired`), never +inferred from `RecoveryState`/`AttemptPhase` variant names or from the field +name `is_definitely_dead`. + +## The mapping + +| Persisted shape | Status | Why | +| --- | --- | --- | +| No state file | `healthy` | No adapter attempt has ever run; absence is not a problem. | +| `Clear`, only live- or uncertain-owner `PendingStart`/`Executed` attempts, no `PendingAbandon` | `healthy` | See [Why a sibling's `PendingStart`/`Executed` never makes this `blocked`](#why-a-siblings-pendingstartexecuted-never-makes-this-blocked). | +| `Clear`, at least one dead-owner `PendingStart`/`Executed` attempt, no `PendingAbandon` | `recovering` | See [Why a dead-owner attempt is `recovering`, not `blocked` or `healthy`](#why-a-dead-owner-attempt-is-recovering-not-blocked-or-healthy). | +| `Clear`, any `PendingAbandon` attempt | `invalid` | Structurally impossible through production behavior: a `PendingAbandon` attempt is only ever created by `begin_terminal_cleanup`, which arms `RecoveryState::Flushing` in the same write, and is only ever removed by `resolve_recovery`'s loop, which runs before `complete_recovery_flush` can transition recovery back to `Clear`. A hand-seeded or corrupted file matching this shape is reported `invalid`. | +| `Pending { generation }`, any attempt composition | `recovering` | A recovery-capable fresh tracked `tool_call` — one whose `(session_id, tool_call_id)` key is not already represented by a nonterminal attempt — claims the flush (`AdmitDecision::FlushClaimed`) and retries every currently outstanding `PendingAbandon` attempt through `resolve_recovery`, regardless of which session performs it and regardless of any co-existing `PendingStart`/`Executed` attempt. A duplicate `tool_call` for an already-tracked `PendingStart`/`Executed` key is instead reused idempotently by `admit_tracked_attempt` *before* `RecoveryState` is ever inspected, so not every individual `tool_call` necessarily advances recovery — but an ordinary future fresh `tool_call` has a proven automatic path, which is why this is `recovering` rather than `blocked`. | +| `Flushing { generation }`, any attempt composition | `recovering` | An orphaned `Flushing` (its owning process crashed between `begin_terminal_cleanup` and `resolve_recovery`) is reclaimed to `Pending` by `normalize_recovery_after_boundary_lock_acquired`, which runs at the very start of every tracked adapter boundary — `tool_call`, `tool_execution_end`, and `ToolExecutionAbandon` alike — before anything else. From `Pending`, a subsequent recovery-capable fresh `tool_call` claims and retries it as above; a duplicate `tool_call` for an already-tracked nonterminal key is not guaranteed to be the one that does so. | +| Read/parse error (missing file aside) | `invalid` | Malformed JSON, unsupported version, or a read failure; the error is surfaced in the health record's detail. | + +Unlike Codex and OpenCode, Pi's investigation found **no persisted shape +reachable through ordinary production dispatch that classifies `blocked`**. +The reasons are specific to Pi's design and are explained below. + +## Why a sibling's `PendingStart`/`Executed` never makes this `blocked` + +OpenCode's `admit_tracked_attempt` denies a brand-new admission +(`UncertainAttemptBlocked`) whenever *any* tracked attempt anywhere in state +is `PendingStart`, because OpenCode's `PendingStart` is a narrow +crash-recovery window — see +[opencode-mutation-scope-health.md](opencode-mutation-scope-health.md). Pi's +`PendingStart` is architecturally different +([pi-mutation-scope-integration.md](pi-mutation-scope-integration.md#why-pendingstart-never-blocks-a-sibling-admission)): +it is the adapter's normal, possibly long-lived resting state for a tool +call's entire execution window, so Pi's `admit_tracked_attempt` never +inspects a sibling attempt's `PendingStart`/`Executed` phase when deciding +whether to admit a different key. Pi's only same-state admission gate is a +`PendingAbandon` attempt (`UncertainAttemptBlocked`), and that combination is +itself proven unreachable under `RecoveryState::Clear` (see the mapping +table above), so in practice `UncertainAttemptBlocked` never fires against +production-reachable state. + +A live-owner or uncertain-owner attempt (a live pid whose exact +process-instance identity cannot be positively established, per +`is_definitely_dead` in `process_owner.rs`) is therefore ordinary in-flight +state: it never blocks any other admission, and this adapter's D10 +stale-owner sweep leaves it completely untouched (proven by +`clear_recovery_is_healthy_with_an_uncertain_owner_pending_start_attempt_never_swept_by_an_unrelated_start` +in `health.rs`'s own test module, and by +`an_owner_that_cannot_be_positively_proven_dead_is_never_abandoned_by_an_unrelated_start` +in `mod.rs`). It is `healthy`, not merely "not yet observed to be a problem." + +## Why a dead-owner attempt is `recovering`, not `blocked` or `healthy` + +`reconcile_stale_owners` — the D10 sweep — runs unconditionally at the start +of `admit_or_recover`, before that call's own key is even considered, for +*every* tracked `tool_call` from *any* session. It repeatedly collects every +`PendingStart`/`Executed` attempt whose recorded `ProcessOwner` is positively +proven dead (any session, any prior process — not only one matching the +incoming key) and retires each batch together through the existing D8 +flush/abandon/flush recovery sequence before the triggering call is admitted. + +This is unfinished recovery work — the attempt's owning process is gone and +the scope was never legitimately closed — so it is not `healthy`. But because +the sweep is proven to run automatically on the very next tracked admission +from any session, with no manual state-file surgery required, it has the +proven self-healing path the `recovering` status exists to describe, so it is +not `blocked` either. + +Proven in `health.rs`'s own test module by driving real dispatch end to end: + +- `clear_recovery_with_a_dead_owner_pending_start_attempt_is_recovering_and_an_unrelated_session_start_sweeps_it_ac4`: + session A starts, its recorded owner is forced dead, `classify_health` + reports `recovering`, then an unrelated session B's `tool_call` drives the + real `flush -> abandon -> flush -> start` sequence and `classify_health` + reports `healthy` once B is admitted. +- `clear_recovery_with_a_dead_owner_executed_attempt_is_recovering_and_is_swept_without_a_synthetic_close`: + the same proof for a dead-owner `Executed` attempt, additionally asserting + no synthetic `close` operation is ever sent for it (D9: the current Git + tree no longer represents the original terminal-observation time). +- `pending_recovery_from_an_interrupted_dead_owner_sweep_is_recovering_and_denies_the_triggering_start_until_resumed`: + a transient seam failure mid-sweep leaves recovery durably `Pending` and + denies the very call that triggered it, but `classify_health` still reports + `recovering`, and the next tracked `tool_call` resumes and completes the + interrupted sweep. + +## Why `Pending`/`Flushing` are always `recovering`, unrelated attempts included + +Unlike Codex, where only a same-`(session_id, turn_id)`-lane successor can +retry a stuck predecessor's abandonment before reaching the global recovery +gate, Pi's `admit_tracked_attempt` lets *any* caller whose key is not already +represented by a nonterminal attempt claim a `Pending` generation's flush, +and `resolve_recovery` retries *every* currently outstanding `PendingAbandon` +attempt in one pass regardless of which session or call originally doomed +it. Combined with the D10 sweep above (which can itself arm or re-arm the +same recovery generation for a dead-owner attempt), every reachable +`Pending`/`Flushing` shape has a proven any-caller resolution path — it is +just not guaranteed to be the very next `tool_call` received, since a +duplicate `tool_call` for an already-tracked `PendingStart`/`Executed` key is +reused idempotently before the recovery-state gate is ever reached (proven +by +`pending_recovery_reuses_a_duplicate_start_for_an_existing_nonterminal_key_without_advancing_recovery_then_a_fresh_start_recovers` +in `health.rs`'s own test module). An unrelated live/uncertain-owner +`PendingStart`/`Executed` attempt sitting alongside such a recovery does not +change this: `resolve_recovery` never inspects it, and (per the section +above) it was never a source of denial for other calls in the first place. + +Proven in `health.rs`'s own test module: +`pending_recovery_from_a_failed_terminal_abandon_is_recovering_and_self_heals_on_the_next_start` +(a terminal-abandon seam failure leaves `Pending`, repeated unrelated +`tool_call`s are denied without changing the classification, then a working +seam self-heals) and +`orphaned_flushing_is_recovering_and_reclaimed_by_the_next_boundary` (a +simulated crash between `begin_terminal_cleanup` and `resolve_recovery` +leaves a literal orphaned `Flushing`, reclaimed and resolved by the next +tracked `tool_call`). diff --git a/context/cli/pi-mutation-scope-integration.md b/context/cli/pi-mutation-scope-integration.md index 522d779e2..21d08eaac 100644 --- a/context/cli/pi-mutation-scope-integration.md +++ b/context/cli/pi-mutation-scope-integration.md @@ -270,6 +270,9 @@ files, holding `next_attempt_seq`, a recovery generation/phase, and the live attempt list. It is bookkeeping only, never attribution evidence, and is never held while invoking the generic mutation-scope runtime. +Doctor health classification built on this state machine is proven in +[pi-mutation-scope-health.md](pi-mutation-scope-health.md). + See also [`mutation-scope-hook-ingress.md`](mutation-scope-hook-ingress.md), [`mutation-scope-runtime.md`](mutation-scope-runtime.md), and [`mutation-trace-external-mutation-guard.md`](mutation-trace-external-mutation-guard.md) diff --git a/context/context-map.md b/context/context-map.md index 1f5c159ab..e3fc9c1e3 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -107,11 +107,14 @@ Additional mutation-scope integration context: - `context/cli/mutation-scope-provenance.md` (observational `ScopeProvenance` keyed by `ScopeId`: insert-once canonical session plus nullable first-observed model, admission-bounded creation while the owning scope is `NeverSeen`, exact producer snapshots for Claude and Codex, read-time enrichment of mutation-AI lines and conservative Agent Trace hunk model agreement, and the explicit boundary that mutation protocol attribution proves ownership while scope provenance describes the owning scope; real Claude/Codex `Bash` persistence regressions cover the full path) - `context/cli/codex-mutation-scope-integration.md` (the second concrete harness adapter: Codex tracked/delegation/untracked classification, partial-by-tool-surface coverage, identity and checkout-local recovery state, write-ahead fail-closed lifecycle, cleanup signals, boundary-aware attribution confirmation, Codex setup/doctor ownership, and the scope provenance it sends with every tracked `Start` — the `cx_`-prefixed canonical session plus the normalized `model` read off the same `PreToolUse` payload, for both `Bash` and `apply_patch`, with a deliberately lenient `model` read so an absent, blank, or non-string model records no model instead of denying a mutation-capable tool) +- `context/cli/codex-mutation-scope-health.md` (the `doctor-mutation-scope-health` plan's T03: `classify_health` in `cli/src/services/hooks/codex_mutation_scope/health.rs` maps the adapter's `RecoveryState`/attempt-state combinations onto `healthy | recovering | blocked | invalid`, proven by driving the real dispatch/recovery functions rather than reading enum names — `Pending` with attempts empty, `Pending` with unresolved attempts, and an orphaned `Flushing` are all `recovering`: the production `PreToolUse` order runs `sweep_stale_lane_predecessors` before the global `RecoveryBlocked` gate, so a same-`(session_id, turn_id)`-lane successor can retry and clear a stuck attempt and self-admit through a flush, even though an unrelated session/lane still denies fail-closed in the meantime; `Flushing` with unresolved attempts and any read/parse failure are `invalid`) - `context/sce/codex-apply-patch-diff-runtime.md` (the complementary Codex `PostToolUse(apply_patch)` parsing, path containment, normalization, and `diff_traces` evidence contract) - `context/cli/opencode-mutation-scope-adapter-lifecycle.md` (the T04 OpenCode adapter runtime detail split out of the integration doc: attempt phases `PendingStart` → `Active` → `PendingAbandon`, per-`git-dir` boundary lock, durable `/sce/` state, write-ahead fail-closed `Start`, `Close` on successful `ToolExecuteAfter`, exact-`ToolError`-only `Abandon` with durable terminal intent + ambiguity-consuming `flush` + generation-tracked recovery barrier that retries transient cleanup failures, non-authoritative broad async events, no same-session sweep, no TTL, no protocol/Quint change) - `context/cli/opencode-mutation-scope-integration.md` (the third concrete harness producer, reachable by a real OpenCode session as of the `opencode-mutation-scope-integration` plan's T05 and now proven end-to-end through real Git/DB production-path regressions as of T06: OpenCode tool lifecycle frozen (T01) against `opencode-ai@1.15.4` / `@opencode-ai/plugin@1.15.4` (upstream `v1.15.4`), with the probe fixtures/report under `cli/src/services/hooks/opencode_mutation_scope/fixtures/`; `(sessionID, callID)` scope identity, `bash`/`write`/`edit`/`apply_patch` tracked with the `gpt-`-model patch gate making `edit`/`write` vs `apply_patch` mutually exclusive per session, `task` delegation, MCP/plugin/unknown tools untracked by explicit allowlist; `bash` Start on `shell.env` (post-permission, pre-spawn), file-tool Start write-ahead on `tool.execute.before`, Close on successful `tool.execute.after` (fires for non-zero exit / 127 / timeout, not for rejection / interrupt / validation failure); OpenCode scopes confirmation-required like Codex (shipped in T02 as the generic `requires_boundary_confirmation` predicate); `chat.params` model provenance keyed by `sessionID` else `NULL`; sequential plugin dispatch in explicit-`plugin`-array order with fail-closed `tool.execute.before` / `shell.env` barriers (Probes A/B/C PROVEN); SIGINT/SIGKILL leave no terminal hook and orphan child processes so no TTL is safe; OpenCode persistence is global-user-scoped not checkout-local; T03 added `cli/src/services/hooks/opencode_mutation_scope/mod.rs` (strict wire-event parsing, `classify_tool`, `AttemptKey`, frozen `oc-tool-v1|s=:|c=:` `ScopeId` with no attempt-seq, `oc_` provenance) and the hidden `sce hooks opencode-mutation-scope` command; T04 added the full lifecycle (`state.rs` durable checkout-local attempt state under `/sce/` with attempt phases `PendingStart` → `Active` → `PendingAbandon`, `os_lock.rs`/`boundary_lock.rs`, generation-tracked recovery barrier, write-ahead fail-closed `Start`, `Close`, `Abandon` on exact `ToolError` `(session_id, call_id)` evidence only — broad async `SessionIdle`/`SessionError`/`SessionDeleted`/`ServerDisposed` events retire nothing (D10/D11); `ToolError` first persists the doomed attempt as `PendingAbandon` **and** the recovery generation in one write before any seam call (`begin_terminal_cleanup`), then `resolve_recovery` drives an ineligible `flush` (while doomed + sibling scopes still resolve it `IneligibleUnscoped`), then the generic `abandon`, then removes the attempt only on `abandon` success, then a second `flush` for the rebaseline; a transient failure of any step relinquishes recovery to `Pending` and leaves the attempt `PendingAbandon` so the next recovery-capable boundary (a new tracked `Start` claims the flush; a duplicate `ToolError` retries) replays the whole idempotent sequence rather than forgetting the scope; a replayed `Start` for a `PendingAbandon` identity fails closed and never returns it to `Active`; so a concurrent `Abandon(A)`→`Close(B)` can never yield `AiExclusive(B)` over A's interval while B keeps its later intervals; the ambiguity flush runs alongside live siblings, not deferred to `attempts.is_empty()`; `ToolError` carries `tool_name` so untracked/delegation errors are zero-footprint; no same-session sweep, no TTL; no protocol/Quint change — the generic `flush`/`abandon`/confirmation-required semantics already suffice; the full T04 recovery detail now lives in `context/cli/opencode-mutation-scope-adapter-lifecycle.md`) driving the generic in-process ingress seam; T05 added the generated `sce-mutation-scope.ts` transport plugin (`config/lib/mutation-scope-plugin/`, emitted by `config/pkl/generate.pkl`, registered last in `config/pkl/renderers/common.pkl`), installed as the final OpenCode plugin by the `config_merge` append after arbitrary user plugins, with `sce doctor`'s `inspect_opencode_plugin_ordering_health` flagging a non-last position; the plugin maps `write`/`edit`/`apply_patch` `tool.execute.before` → fail-closed `ToolExecuteBefore`, `shell.env` → fail-closed `ShellEnv` bash Start, `tool.execute.after` → best-effort Close, tool-part `error` → best-effort `ToolError`, observes the model per turn from `chat.params` (`providerID/api.id`, ignoring the `title` agent, replacing rather than merging the cached per-session model so a later turn with no valid model clears it instead of reusing stale evidence), throws on any failure to establish a tracked Start (non-zero adapter exit, spawn failure, timeout, or missing `sce` CLI) so every transport failure fails closed; the broad async `SessionIdle`/`SessionError`/`ServerDisposed` events are not forwarded at all (T04's dispatch is a no-op for them) and `session.deleted` only clears the plugin's local model cache — full plugin-transport/model-provenance detail split into [`opencode-mutation-scope-plugin-transport.md`](cli/opencode-mutation-scope-plugin-transport.md); T06 added real Git/DB production-path regressions in `cli/src/services/hooks/mod.rs` (`services::hooks::tests::mutation_provenance_e2e`) proving tracked-tool success, model-present/model-missing provenance, task/unknown-tool zero-footprint, concurrent reject-and-confirm, and OpenCode+Codex/Claude overlap down to `mutation_ai_patch` and Agent Trace output; a live `apply_patch` fixture against a real credentialed OpenCode CLI session remains outstanding for `/validate` (a credential gap, not a soundness gap)) - `context/cli/opencode-mutation-scope-plugin-transport.md` (the T05 generated `sce-mutation-scope.ts` plugin's model-provenance observation from per-session `chat.params` and its sequential, last-registered position in OpenCode's plugin ordering — detail split out of `opencode-mutation-scope-integration.md` for the repository's per-file line budget) +- `context/cli/opencode-mutation-scope-health.md` (the `doctor-mutation-scope-health` plan's T04: `classify_health` in `cli/src/services/hooks/opencode_mutation_scope/health.rs` maps the adapter's `RecoveryState`/`AttemptPhase` combinations onto `healthy | recovering | blocked | invalid`, proven by driving the real dispatch/recovery functions — `Clear` + `PendingAbandon` is `invalid`; any state containing a `PendingStart` attempt is `blocked` (except that `Clear` + `PendingAbandon` retains `invalid` precedence), because no boundary sweeps a stale `PendingStart` on behalf of an unrelated call, unlike Pi's dead-owner sweep or Codex's same-lane retry, so a crashed owning process leaves it a permanent wedge for new admissions even while a `Pending`/`Flushing` recovery for another attempt is also outstanding, since `resolve_recovery` never retires `PendingStart`; `Pending`/`Flushing` with no `PendingStart` outstanding is `recovering`; `Clear` with ordinary `Active`/no attempts is `healthy`; any read/parse failure is `invalid`) - `context/cli/pi-mutation-scope-integration.md` (the fourth concrete harness adapter, `cli/src/services/hooks/pi_mutation_scope/`, hidden command `sce hooks pi-mutation-scope`, driven by the canonical generated extension in ordinary Pi sessions: Pi's `bash`/`edit`/`write` tracked-tool allowlist with `tool_call` as the single universal pre-execution gate for every tool including `bash`; the `pi-tool-v1|n=|s=:|c=:` `ScopeId` whose checkout-local monotonic attempt sequence stops a reused `toolCallId` from ever reactivating a terminal scope; the `PendingStart`→`Executed`→`Closed`/`PendingAbandon` attempt-phase machine with no `Active` phase, because `PendingStart` is Pi's normal resting state for an entire in-flight execution (`tool_execution_start` fires unconditionally before the fail-closed `tool_call` gate and proves nothing; `tool_result` is the sole `Executed` evidence, and `tool_execution_end` with no preceding `tool_result` abandons rather than closes); why admission's fail-closed "uncertain attempt" check covers only a lingering `PendingAbandon` or non-`Clear` recovery state and never a sibling's `PendingStart`, unlike the other three adapters, so concurrent Pi tool calls stay distinct live scopes; a positive-process-death-only stale-`PendingStart`/`Executed` recovery (`getppid()`-captured owner plus Linux `/proc`-start-time PID-reuse proofing, no TTL/sweep), run on every tracked Start admission by independently inspecting persisted attempts rather than only ones matching the incoming key, and adapter reconciliation with the external-mutation guard's forced recovery, both added by a later task in this same plan; and the harness-neutral `external-mutation-guard` mechanism this task also built for Pi's `user_bash`, detailed separately in `context/cli/mutation-trace-external-mutation-guard.md`) +- `context/cli/pi-mutation-scope-health.md` (the `doctor-mutation-scope-health` plan's T05: `classify_health` in `cli/src/services/hooks/pi_mutation_scope/health.rs` maps the adapter's `RecoveryState`/`AttemptPhase`/owner-liveness combinations onto `healthy | recovering | blocked | invalid`, proven by driving the real dispatch/recovery functions — unlike Codex and OpenCode, Pi's investigation found no persisted shape reachable through ordinary production dispatch that classifies `blocked`: `admit_tracked_attempt` never gates admission on a sibling's `PendingStart`/`Executed` phase (only on a co-existing `PendingAbandon`, itself proven unreachable under `Clear`), and the D10 dead-owner sweep (`reconcile_stale_owners`) unconditionally retires a dead-owner `PendingStart`/`Executed` attempt on the next tracked `tool_call` from any session, so such an attempt is `recovering`, not `blocked`; `Pending`/`Flushing` are always `recovering` regardless of attempt composition, since a recovery-capable fresh tracked Start — one whose key is not already represented by a nonterminal attempt — can claim and retry the flush, not lane- or key-scoped like Codex, though a duplicate Start for an already-tracked `PendingStart`/`Executed` key is instead reused idempotently before the recovery-state gate and so does not itself advance recovery; `Clear` with a `PendingAbandon` attempt and any read/parse failure are `invalid`) - `context/cli/mutation-trace-external-mutation-guard.md` (the harness-neutral, not-Pi-specific D13 external-mutation supervisor process, `run_external_mutation_guard` in `cli/src/services/mutation_trace/runtime/external_mutation_guard.rs`, hidden `sce hooks external-mutation-guard`, Unix-only: acquires a `ProtectedWorktree`, reports it armed, spawns the human shell command as its own child in its own process group with a `dup()`-before-`spawn()` fd handed to the child so the `WorktreeLock` flock survives the supervisor's own death via the child's independent descriptor, finishes exclusively on the shell's own `wait()` (never on a cancel/control-channel signal), and only then forces `database_failure`+`recover` on the already-held worktree and calls `ProtectedWorktree::complete()` — reusing existing runtime primitives with zero `protocol.rs`/Quint change; called by the canonical Pi extension for human `user_bash`) Working areas: diff --git a/context/plans/doctor-mutation-scope-health.md b/context/plans/doctor-mutation-scope-health.md new file mode 100644 index 000000000..4c4674522 --- /dev/null +++ b/context/plans/doctor-mutation-scope-health.md @@ -0,0 +1,357 @@ +# Plan: doctor-mutation-scope-health + +## Change summary + +`sce doctor` currently only checks whether each harness's mutation-scope hook +*registration* is installed and structurally current (files, hook entries, +Codex trust/policy state). It has no visibility into the mutation-scope +*runtime* state each adapter persists at +`/sce/{adapter}-mutation-scope-state.json`. + +This plan is motivated by a real failure observed in the Claude +mutation-scope adapter. During an abandonment, the adapter's shared cleanup +path ran `mark_recovery_pending()` then called the seam's `abandon` +operation; the seam call failed, so `remove_attempt()` never ran (the error +short-circuited the cleanup helper). The repository was left with persisted +state approximately `recovery_pending = true` with one or more stale +`attempts` left over from an old session. Every subsequent mutation-capable +`PreToolUse` then hit the adapter's recovery barrier, which — with +`recovery_pending == true` and `attempts` non-empty — returns `Deny` +immediately. Crucially, the barrier's *only* self-healing path +(`{"operation":"flush"}` through the seam, which clears `recovery_pending`) +only runs when `attempts.is_empty()`; it never retries the failed abandon and +never removes the stale attempts on its own. The result was a persistent, +repository-wide lockout of mutation-capable Claude tools that required manual +state-file repair to clear — with no operator-facing signal anywhere in `sce +doctor`, in either its JSON or human text output. The only way to discover it +was to read the private state file directly and correlate it with test names +in the adapter's own source. + +This plan extends existing behavior rather than replacing it: it adds a new, +adapter-owned diagnostic classifier per harness (Claude, Codex, OpenCode, Pi) +that maps that harness's own persisted state into one shared, generic status — +`healthy | recovering | blocked | invalid` — and wires `sce doctor` to report +that status per adapter in both `--format json` and the human text output, +using the existing `[PASS]`/`[WARN]`/`[FAIL]`/`[MISS]` vocabulary. `doctor` +never inspects or interprets a `RecoveryState` enum or an `attempts` list +itself; it only consumes the four-way status each adapter module already +computed. + +The feature detects two genuinely different conditions and must not conflate +them: + +- **recovery in progress / automatically recoverable** — the persisted state + reflects unfinished recovery work, but an existing normal lifecycle event + can advance it without manual intervention (`Recovering`); +- **a durable recovery wedge** — the persisted state blocks new mutation work + and, once the hook call that produced it has returned, no future ordinary + adapter event can clear it on its own (`Blocked`). + +Not every `recovery_pending` (or equivalently-shaped) state is stuck. Claude's +`recovery_pending == true && attempts.is_empty()` is `Recovering`: the next +mutation-capable `PreToolUse` flushes and clears it automatically. Only +`recovery_pending == true && attempts non-empty` — the exact shape of the real +incident above — is `Blocked`. Codex, OpenCode, and Pi use a different, +generation-based `RecoveryState` state machine with additional attempt phases +(including `PendingAbandon` for OpenCode and Pi, and `ProcessOwner`-based +liveness for Pi); which combinations are transiently self-healing versus +genuinely stuck is not yet established for those three adapters, so this plan +includes that investigation, proven with tests that exercise the adapters' +real state-machine functions, before each adapter's classifier is +implemented. + +## Health status definitions + +Every adapter classifier returns exactly one of these four statuses. They are +shared, generic, and defined once (`healthy | recovering | blocked | +invalid`); no adapter redefines them. + +### `healthy` + +The persisted adapter state is valid and no recovery condition prevents normal +tracked-tool admission. Normal future mutation-capable work can proceed +without first completing a recovery operation. Absence of a state file is +`healthy` (see AC5) — it means "this adapter has no persisted recovery +problem," not "the adapter has already been exercised." + +### `recovering` + +The persisted adapter state is valid and currently reflects unfinished +recovery work, **but** the adapter has an existing normal lifecycle/admission +path that can make progress from this state without manual state surgery. If +currently executing hook calls finish, and future ordinary lifecycle/admission +events occur, the adapter has a defined automatic path that can eventually +clear the condition. + +Examples may include: recovery pending with no unresolved attempts, where the +next tracked admission claims or performs a flush; a positively dead owner +that an existing stale-owner recovery boundary can retire; another proven +transient generation/recovery state whose normal successor path advances it. + +Do not classify something `recovering` merely because its enum variant is +named `Pending` or `Flushing`. Each `recovering` classification must be proven +by driving the adapter's real recovery path and observing it advance — see +[T03–T05](#task-stack) and AC4. + +### `blocked` + +The persisted adapter state is valid but new tracked mutation work is denied, +**and**, assuming any hook invocation that produced the state has already +returned, future ordinary lifecycle/admission events have no normal automatic +path that can clear the blocker. In other words: valid persisted state + a +fail-closed admission decision + no reachable self-healing path from future +normal events = `blocked`. + +This is the status that must detect the real Claude incident. `blocked` does +not need to prove that no process anywhere on the machine could possibly still +be finishing a write at the exact instant doctor reads the file (see +[Read-only observation semantics](#read-only-observation-semantics) below); it +means that the persisted state, treated as the durable state from which the +next ordinary event must continue, has no normal self-healing transition. + +Do not weaken this plan into a classifier that merely reports "currently +fail-closed." A transient recovery state that denies admission right now but +has a proven future self-healing path is `recovering`, not `blocked`. The +distinction the classifier exists to draw is exactly this one: does the +persisted state have a normal future self-recovery path, or not. + +### `invalid` + +The state file exists but cannot be safely interpreted: malformed JSON, +unsupported version, read error, or a structurally impossible persisted +combination proven unreachable by the adapter's own state machine (proven by +tests/code, not assumed). `invalid` is never used merely because admission is +currently blocked — that is `blocked`. `invalid` means the state cannot be +trusted or interpreted at all, not that it is a durable wedge. + +### Read-only observation semantics + +Doctor is a read-only snapshot. There is a narrow observation window in which +doctor reads a state shape that a currently executing hook process is about +to change — for example Claude may briefly persist `recovery_pending = true` +with non-empty `attempts` between `mark_recovery_pending()`, the `abandon` +seam call, and `remove_attempt()`, all inside one still-running hook process. + +This plan does not attempt to close that window by weakening `blocked`. +Health classifies the durable state observed at inspection time according to +whether normal future adapter lifecycle events can recover from that +persisted state, assuming the operation that produced it has stopped +progressing. This feature is intended to expose durable wedges; it is not a +distributed-process liveness oracle, and it does not prove that no currently +executing process could write a newer state milliseconds later. The +classifier stays pure and read-only regardless. + +## Acceptance criteria + +- [ ] AC1: `sce doctor --format json` reports mutation-scope health + (`healthy | recovering | blocked | invalid`) for every configured/ + detected integration target whose mutation-scope adapter applies (see + AC5 for the absent-state-file case), with a stable machine-readable + reason/detail attached whenever the status is not `healthy`. Doctor does + not render mutation-scope health for an integration target it did not + resolve as configured/detected. + - Validate: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml doctor` (JSON-shape assertions), plus manual `sce doctor --format json` against a repository with hand-seeded adapter state files in each status. +- [ ] AC2: `sce doctor` human text output maps `healthy -> [PASS]`, + `recovering -> [WARN]`, `blocked -> [FAIL]`, `invalid -> [FAIL]`. + Healthy rows follow the existing compact healthy-row contract + (`context/sce/doctor-human-text-contract.md`); `recovering`, `blocked`, + and `invalid` rows expand with a short human reason. + - Validate: rendering unit tests in `cli/src/services/doctor/render.rs`'s test module asserting the row shape for each of the four statuses. +- [ ] AC3: For the Claude adapter, tests prove: + - `recovery_pending == false` -> `Healthy`. + - `recovery_pending == true` with `attempts` empty -> `Recovering`, **and** + the next normal recovery-capable boundary (the adapter's own + `{"operation":"flush"}` recovery-barrier path) actually flushes and clears + `recovery_pending`. + - `recovery_pending == true` with `attempts` non-empty -> `Blocked`, **and** + after the operation that created the state has returned, ordinary future + mutation-capable `PreToolUse` calls deny without advancing it (repeated + denial, not just one). + - Include a regression reproducing the actual failure sequence: allocate/ + persist a live attempt, mark recovery pending, simulate an `abandon` seam + failure so the stale attempt remains, then assert the next tracked + `PreToolUse` is denied and a second, later tracked `PreToolUse` is *still* + denied (it never becomes `recovering` or self-clears). Doctor/classifier + must report `Blocked` for that persisted state. This PR diagnoses the + incident; it does not fix the underlying Claude recovery-barrier bug. + - Validate: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml claude_mutation_scope` (or equivalent module path chosen in T02). +- [ ] AC4: For each of Codex, OpenCode, and Pi: + 1. a table-driven classification matrix covers every meaningful reachable + `RecoveryState`/attempt-phase combination; + 2. real-dispatch behavioral tests prove the future admission/recovery + semantics for every distinct semantic equivalence class used by that + matrix — multiple matrix rows may rely on the same behavioral proof when + they are equivalent under a demonstrated invariant; + 3. the classifier implements exactly those proven semantics; and + 4. classifications are never inferred merely from enum/variant names. + + The matrix test is the exhaustiveness layer (every meaningful persisted + state combination is classified); the real-dispatch regressions are the + behavioral-justification layer (what ordinary future admission/lifecycle + behavior actually does for each distinct equivalence class). Neither + substitutes for the other, and AC4 is not satisfied by classifier-only + testing. + - Validate: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml codex_mutation_scope`, `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml opencode_mutation_scope`, `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml pi_mutation_scope` (or equivalent module paths chosen in T03–T05). +- [ ] AC5: A harness whose mutation-scope state file does not exist in this + repository (never run, or never set up) reports `healthy`. A state file + that exists but fails to parse (malformed JSON, unsupported version, read + failure) reports `invalid` with the read error surfaced in the detail/ + reason. A structurally impossible parseable state may be reported as + `invalid` only when tests/code prove the adapter cannot legitimately + persist it. This holds for every adapter. + - Validate: per-adapter unit tests for the absent-file and malformed-file cases (same test modules as AC3/AC4). +- [ ] AC6: Doctor consistency — for every adapter, `recovering` produces a + `Warning`-severity `DoctorProblem` and overall readiness may remain + `ready`; `blocked` and `invalid` each produce an `Error`-severity + `DoctorProblem` and overall readiness is `not_ready`. JSON health + status, `DoctorProblem` severity, top-level readiness, the human-text + row, and the summary warning/blocking-problem counts never contradict + each other. + - Validate: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml doctor`, including readiness/summary-count assertions for a `recovering`-only repository and a `blocked`/`invalid` repository. + +### Full validation + +- `nix flake check` + +### Context sync + +- `context/sce/agent-trace-hook-doctor.md` (doctor's canonical health-and-repair contract must describe this new runtime-liveness facet alongside its existing structural-registration checks, including the new `Warning`/`Error` problem-severity mapping for `recovering`/`blocked`/`invalid`) +- `context/sce/doctor-human-text-contract.md` (must document the new status rows and their `[PASS]`/`[WARN]`/`[FAIL]` vocabulary mapping) +- `context/cli/claude-mutation-scope-integration.md`, `context/cli/codex-mutation-scope-integration.md`, `context/cli/opencode-mutation-scope-integration.md`, `context/cli/pi-mutation-scope-integration.md` (each adapter's proven recovering-vs-blocked mapping and the reasoning behind it belongs in that adapter's own authoritative doc) +- `context/context-map.md`, only if navigation actually needs updating +- Do not create a new shared mutation-scope-health context document unless implementation reveals shared semantics that cannot cleanly live in the existing docs listed above; the [Health status definitions](#health-status-definitions) section of this plan is the shared contract's origin and should migrate into `context/sce/agent-trace-hook-doctor.md` during T06 context sync rather than spawning a new file. + +## Task context synchronization lifecycle + +- **Task context synchronization:** every task carries `pending | synced | blocked`. + A completed task must be `synced` before another task can start or the plan can + finish. +- For `blocked`, record **Blocker**, **Required action**, and **Retry condition** + beside the status. Never infer `synced` from conversation history; write every + lifecycle transition to the plan file. + +## Constraints and non-goals + +- **In scope:** `cli/src/services/hooks/claude_mutation_scope/`, + `cli/src/services/hooks/codex_mutation_scope/`, + `cli/src/services/hooks/opencode_mutation_scope/`, + `cli/src/services/hooks/pi_mutation_scope/` (new read-only health-classifier + functions and their tests only); a new small shared status-type module used + by all four adapters and by doctor; `cli/src/services/doctor/{types,inspect,render,mod}.rs` + to consume and render the four-way status and wire it into the existing + `DoctorProblem`/readiness model; the context docs listed above. +- **Out of scope:** any change to the underlying recovery/barrier protocol + logic, `RecoveryState` transitions, attempt terminal semantics, attribution + semantics, or fail-closed behavior — this plan only reads and classifies + existing persisted state, never changes when or how a barrier arms, clears, + or retries. It does not fix the Claude recovery-barrier bug described in the + change summary. +- **Constraints:** classifier functions are pure and read-only — they must + never write to an adapter's state file (doctor is diagnostic-only, matching + the existing `ServiceLifecycle::diagnose` vs `fix` split); doctor reports an + adapter's mutation-scope health only for a target it already detects or has + configured, consistent with its existing configured/detected/empty target + resolution; the shared status type and its `healthy | recovering | blocked | + invalid` vocabulary is defined once and reused by all four adapters, not + redefined per adapter; non-`healthy` statuses participate in the existing + `DoctorProblem`/readiness model rather than existing as a parallel + display-only system (see AC6). +- **Non-goal:** this plan does not add a `doctor --fix` remediation path for a + `blocked` or `invalid` mutation-scope state. Given the barrier's deliberate + fail-closed design (D12/D19: a lost abandonment must not be silently + forgotten), automatic repair is a separate decision this plan does not make; + the goal here is making the stuck state visible, not resolving it + automatically. Specifically, `sce doctor --fix` must not delete state files, + delete attempts, clear `recovery_pending`/`RecoveryState`, increment or + reset generations, fabricate an abandon, or reset process-owner evidence. + For `blocked`/`invalid` records, T06 must supply deterministic manual + remediation guidance and must not casually recommend deleting the state + file; if no safe generic recovery command currently exists for a given + adapter/status, the remediation text must say so rather than inventing one. + Designing that safe canonical recovery operation is a separate change. + +## Task stack + +- [x] T01: `Define the shared mutation-scope health status contract` (status:done) + - Task ID: T01 + - Scope: In — a new small module (e.g. `cli/src/services/mutation_trace/scope_health.rs` or a peer location chosen at implementation time) defining the `MutationScopeHealthStatus` enum (`Healthy`, `Recovering`, `Blocked`, `Invalid`) and the shared per-adapter health record (adapter identity, status, human-readable reason, and any machine detail doctor needs to render or serialize it). Out — any adapter-specific classification logic, and any doctor wiring. + - Dependencies: none + - Done when: the shared `MutationScopeHealthStatus` enum exists; the per-adapter `MutationScopeAdapterHealth` record exists; the type derives whatever traits its current consumers need (`Debug`/`Clone`/`PartialEq`/`serde::Serialize` as required by JSON rendering); the record carries adapter identity, status, a human-readable reason, and optional machine detail; focused unit tests cover the shared type/record behavior; and no adapter-specific classifier logic or doctor integration is introduced. The detailed semantics of `Healthy`/`Recovering`/`Blocked`/`Invalid` remain authoritative in this plan's [Health status definitions](#health-status-definitions) and are synchronized into durable context by T06; they do not need to be duplicated as comments in the Rust type. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml scope_health` + - Context synchronization: synced + - Completed: 2026-09-19 + - Files changed: `cli/src/services/hooks/mutation_scope_health.rs`, `cli/src/services/hooks/mod.rs` + - Result: Added the shared `MutationScopeHealthStatus` enum (`Healthy`/`Recovering`/`Blocked`/`Invalid`) and `MutationScopeAdapterHealth` record (adapter identity via the existing `ActorKind`, status, human-readable reason, optional machine detail) in a new `cli/src/services/hooks/mutation_scope_health.rs` module, registered in `hooks/mod.rs`. No adapter classification logic or doctor wiring was added. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml scope_health` — pass (3 tests). + - Context impact: domain — no root context file currently describes mutation-scope health status; T06's context sync is where the shared status vocabulary migrates into `context/sce/agent-trace-hook-doctor.md` per the plan's context-sync section. This task introduces no new adapter-facing or user-facing contract by itself. + +- [x] T02: `Classify Claude mutation-scope health` (status:done) + - Task ID: T02 + - Scope: In — a read-only function in `cli/src/services/hooks/claude_mutation_scope/` that reads `claude-mutation-scope-state.json` via the existing `state::read_state` and maps it to the T01 status type using the proven rule from AC3 (`recovery_pending && attempts non-empty` → `Blocked`, because the recovery barrier's only self-healing path — `{"operation":"flush"}` — only runs when `attempts.is_empty()`, so a non-empty `attempts` list has no reachable future self-healing transition; `recovery_pending && attempts empty` → `Recovering`, because that flush path does run and does clear `recovery_pending`; neither → `Healthy`; a `read_state` error → `Invalid` with the error surfaced); absent-file handling (already `Ok(AdapterState::default())` in `read_state`) must classify as `Healthy`. Out — doctor wiring, other adapters, any change to the recovery barrier itself. + - Dependencies: T01 + - Done when: unit tests cover all four resulting statuses, the absent-file case, and the AC3 regression scenario (stale non-empty-`attempts` state persists across two successive tracked `PreToolUse` calls without self-clearing). + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml claude_mutation_scope` + - Context synchronization: synced + - Completed: 2026-09-19 + - Files changed: `cli/src/services/hooks/claude_mutation_scope/health.rs` (new), `cli/src/services/hooks/claude_mutation_scope/mod.rs`, `cli/src/services/hooks/claude_mutation_scope/state.rs` + - Result: Added `classify_health(git_dir) -> MutationScopeAdapterHealth` in a new `claude_mutation_scope/health.rs`, registered as `pub(crate) mod health;` in `mod.rs`. It calls the existing `state::read_state` and maps: a read/parse error → `Invalid` with the error surfaced via `.with_detail(...)`; `recovery_pending == false` (including the absent-file default) → `Healthy`; `recovery_pending == true` with empty `attempts` → `Recovering`; `recovery_pending == true` with non-empty `attempts` → `Blocked`. Made `state::state_path` `pub(crate)` (was module-private) so the malformed-file test could target the real state file path without duplicating the filename constant. No doctor wiring, other-adapter logic, or recovery-barrier behavior was touched. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml claude_mutation_scope` — pass (119 tests, including the new `health::tests` module: absent-file, `recovery_pending=false` with live attempts, empty-attempts recovering, non-empty-attempts blocked, malformed-file invalid, and the AC3 regression driving a real failed-abandon seam call then asserting `apply_recovery_barrier` denies twice in a row with `classify_health` reporting `Blocked` throughout). + - Context impact: domain — `context/cli/claude-mutation-scope-integration.md` now records the proven Healthy / Recovering / Blocked / Invalid mapping in a new "Mutation-scope health" section adjacent to "Abandonment cleanup signals" / "The recovery barrier", including the behavioral reason for Recovering (the recovery barrier's own flush + `clear_recovery_pending` path) versus Blocked (the flush path never runs when `attempts` is non-empty, so ordinary future `PreToolUse` calls keep denying without advancing recovery — the exact incident shape), the read-only boundary, and the observation-window nuance. + +- [x] T03: `Investigate and classify Codex mutation-scope health` (status:done) + - Task ID: T03 + - Scope: In — for every reachable `RecoveryState` value in `cli/src/services/hooks/codex_mutation_scope/state.rs` (`Clear`, `Pending { generation }`, `Flushing { generation }`) and its interaction with attempt state, answer for each: (1) is this combination actually reachable through production behavior; (2) is it valid persisted state; (3) once the hook call that created it has returned, what happens on the next ordinary tracked admission/lifecycle event; (4) does an existing code path advance recovery; (5) can it return to normal admission without manual state-file surgery; (6) is progress dependent on an event that can no longer occur; (7) does the next Start merely deny forever; (8) for owner-aware paths, can positive process-death evidence trigger existing recovery. Drive the adapter's real dispatch/barrier logic (the way its own existing test suite does) to answer these, not the enum names. A table-driven classification matrix covers every meaningful persisted `RecoveryState`/attempt-phase combination for Codex, and real-dispatch behavioral tests prove the future admission/recovery semantics for every distinct semantic equivalence class represented by that matrix — multiple matrix rows may share one behavioral proof when a demonstrated invariant makes them semantically equivalent; a classifier function implements exactly that proven mapping against the T01 status type. Out — OpenCode, Pi, doctor wiring. + - Dependencies: T01 + - Done when: a table-driven classification matrix covers every meaningful persisted `RecoveryState`/attempt-phase combination for Codex; real-dispatch behavioral tests prove every distinct semantic equivalence class represented by that matrix, and multiple matrix rows may share one behavioral proof when equivalence follows from a demonstrated invariant; the classifier matches those proven semantics exactly; and absent/malformed state handling satisfies AC5. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml codex_mutation_scope` + - Context synchronization: synced + - Completed: 2026-09-19 + - Files changed: `cli/src/services/hooks/codex_mutation_scope/health.rs` (new), `cli/src/services/hooks/codex_mutation_scope/mod.rs`, `cli/src/services/hooks/codex_mutation_scope/state.rs` + - Result: Investigated every reachable `(RecoveryState, attempts)` combination by reading and driving `codex_mutation_scope`'s real dispatch/recovery code (`admit_tracked_attempt`, `sweep_stale_lane_predecessors`, `cleanup_attempts_matching`, `normalize_recovery_after_boundary_lock_acquired`) and its existing test suite (notably `lifecycle_cleanup_with_a_failed_abandon_keeps_the_attempt_tracked_d12`, `recovery_barrier_flushes_once_quiescent_then_starts_ac12`, `test_i_orphaned_flushing_is_reclaimed_and_flush_is_retried_once`, `test20_recovery_pending_blocks_a_tracked_successor_until_recovery_succeeds_ac12`). Added `classify_health(git_dir) -> MutationScopeAdapterHealth` in a new `codex_mutation_scope/health.rs`, registered as `pub(crate) mod health;` in `mod.rs`: `RecoveryState::Clear` → `Healthy` (attempts alone are ordinary in-flight lifecycle state, not a recovery condition); `Pending{g}` with attempts empty → `Recovering`, proven by driving `admit_tracked_attempt` and observing `FlushClaimed`; `Flushing{g}` with attempts empty → `Recovering`, proven by driving `normalize_recovery_after_boundary_lock_acquired` then `admit_tracked_attempt` and observing the reclaimed generation retried as `FlushClaimed`; `Flushing{g}` with attempts non-empty is structurally impossible through production behavior (the only transition into `Flushing` requires attempts to already be empty, and `admit_tracked_attempt` refuses all new attempts while `Flushing`) and is classified `Invalid` if ever observed in a hand-seeded file; a read/parse error → `Invalid` with the error surfaced. Made `state::state_path` `pub(crate)` (was module-private) so `health.rs`'s malformed/hand-seeded-file tests could target the real state file path, mirroring T02. + - **Correction (this task, applied after initial completion):** `Pending{g}` with attempts non-empty was initially classified `Blocked` on the theory that the global `RecoveryBlocked` gate in `admit_tracked_attempt` denies every tracked `PreToolUse` unconditionally. That theory was wrong: the real `PreToolUse` order is `with_boundary_lock -> normalize_recovery_after_boundary_lock_acquired -> sweep_stale_lane_predecessors -> admit_or_recover -> admit_tracked_attempt`, so `sweep_stale_lane_predecessors` — which retries the abandonment of a stale same-`(session_id, turn_id)`-lane predecessor — always runs *before* the `RecoveryBlocked` gate is reached for that same call. A same-lane successor can therefore retry and clear the stuck attempt, driving `Pending` to empty and self-admitting through `FlushClaimed`, entirely through existing ordinary lifecycle behavior. This is a proven normal self-healing route, so the correct classification is `Recovering`, not `Blocked`; unrelated admission (a different session, or the same session with a different `turn_id`) still denies fail-closed in the meantime, but that fail-closed behavior does not by itself make the state `Blocked` under this plan's [Health status definitions](#health-status-definitions). The classifier, its reason text, and the state-level `health.rs` test were corrected accordingly, and a new real same-lane driver regression (`same_lane_successor_retries_abandon_and_reaches_healthy_after_a_failed_lifecycle_abandon_ac4`) now proves the `abandon(A) -> flush -> start(C)` self-healing path end to end. No recovery/barrier production behavior changed; only the classifier's mapping and its supporting tests/docs were corrected. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml codex_mutation_scope` — pass, including the `health::tests` module (absent-file, Clear-with-attempts, Pending-empty-recovering, `pending_non_empty_recovery_denies_unrelated_admission_without_losing_the_recovery_path` [Recovering], orphaned-Flushing-recovering, hand-seeded Flushing-non-empty-invalid, malformed-file-invalid) and the `tests::driver` real-dispatch regressions: `pending_non_empty_recovery_denies_unrelated_admission_without_losing_the_recovery_path` (a failed `SessionEnd` abandon leaves the state `Recovering`, and two successive real `PreToolUse` calls from an unrelated session are both denied without the seam being invoked, while the classifier keeps reporting `Recovering` throughout), `health_classifies_recovering_then_healthy_once_the_next_pre_tool_use_flushes_ac4` (`Pending` + empty attempts self-heals to `Healthy`), and the new `same_lane_successor_retries_abandon_and_reaches_healthy_after_a_failed_lifecycle_abandon_ac4` (a same-`(session_id, turn_id)`-lane successor retries and clears a stale predecessor's abandon, then a further same-lane call drives the real `abandon(A) -> flush -> start(C)` seam-call ordering to `Healthy`). + - Context impact: domain — `context/cli/codex-mutation-scope-health.md` is the canonical detailed owner of the proven Healthy/Recovering/Blocked/Invalid mapping and its reasoning (the global `RecoveryBlocked` gate, the same-lane sweep running *before* that gate for the current call, and the orphaned-flush reclaim path), corrected to remove the false claim that the recovery gate runs before the same-lane sweep and to reclassify non-empty `Pending` as `Recovering`; `context/cli/codex-mutation-scope-integration.md` links to it from its "Recovery and durable state" section rather than duplicating the mapping; `context/context-map.md` registers the health document. + +- [x] T04: `Investigate and classify OpenCode mutation-scope health` (status:done) + - Task ID: T04 + - Scope: In — the same investigation and classifier work as T03 (the same eight questions, the same "what future event gets this state out?" requirement, the same `Clear`/`Pending { generation }`/`Flushing { generation }` `RecoveryState` coverage), applied to `cli/src/services/hooks/opencode_mutation_scope/state.rs`, including its additional `PendingAbandon` attempt phase — investigate whether an unresolved `PendingAbandon` has any existing automatic successor path, or whether it can only be classified `Blocked` once its recovery boundary is proven to have no future normal advance. Out — Codex, Pi, doctor wiring. + - Dependencies: T01 + - Done when: the full OpenCode classification matrix is table-driven and complete; every distinct semantic equivalence class underlying that matrix is proven through real adapter dispatch/recovery behavior; the classifier matches those proven semantics exactly; and absent/malformed state handling satisfies AC5. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml opencode_mutation_scope` + - Context synchronization: synced + - Completed: 2026-09-19 + - Files changed: `cli/src/services/hooks/opencode_mutation_scope/health.rs` (new), `cli/src/services/hooks/opencode_mutation_scope/mod.rs`, `cli/src/services/hooks/opencode_mutation_scope/state.rs`, `context/cli/opencode-mutation-scope-health.md` (new), `context/cli/opencode-mutation-scope-integration.md`, `context/context-map.md` + - Result: Investigated every reachable `(RecoveryState, AttemptPhase)` combination by reading and driving `opencode_mutation_scope`'s real dispatch/recovery code (`admit_tracked_attempt`, `establish_tracked_start`, `resolve_recovery`, `normalize_recovery_after_boundary_lock_acquired`, `begin_terminal_cleanup`) and its existing regression suite (`regression_a_abandon_failure_preserves_terminal_intent_then_recovers`, `regression_b_ambiguity_flush_failure_blocks_new_starts_then_recovers`, `regression_c_rebaseline_flush_failure_is_recoverable_without_poison`, `a_close_before_start_confirmation_consumes_rather_than_closes`, `server_disposed_cannot_sweep_another_processes_attempt`). Added `classify_health(git_dir) -> MutationScopeAdapterHealth` in a new `opencode_mutation_scope/health.rs`, registered as `pub(crate) mod health;` in `mod.rs`: `RecoveryState::Pending{g}` and `RecoveryState::Flushing{g}` are always `Recovering` regardless of attempt state — unlike Codex's same-lane-scoped sweep, `admit_tracked_attempt`'s flush claim and `resolve_recovery`'s retry-every-`PendingAbandon`-attempt loop are not scoped to any particular session or call, so the very next tracked admission from *any* call always has a proven path to advance recovery, and an orphaned `Flushing` is always reclaimed to `Pending` by `normalize_recovery_after_boundary_lock_acquired` on the next boundary; `RecoveryState::Clear` with an attempt stuck in `PendingStart` is `Blocked` — this state sits outside the `RecoveryState` machine entirely, only that exact `(session_id, call_id)`'s own `ToolExecuteAfter`/`ToolError` (or a duplicate `Start` redelivery) retires it, no boundary sweeps a stale `PendingStart` on behalf of an unrelated call (proven inert for `ServerDisposed` by the adapter's own existing test), and OpenCode has no process-liveness detection analogous to Pi's `is_definitely_dead()`, so a crashed owning process leaves it a permanent wedge — the same "valid state + fail-closed admission + no reachable self-healing path" shape as the Claude incident this plan exists to surface; `RecoveryState::Clear` with a `PendingAbandon` attempt is structurally impossible (proven unreachable: `PendingAbandon` is only ever set atomically with `Flushing` in `begin_terminal_cleanup`, and recovery only returns to `Clear` after every `PendingAbandon` attempt has already been removed in `resolve_recovery`) and is classified `Invalid` if hand-seeded; a read/parse error → `Invalid` with the error surfaced. Made `state::state_path` `pub(crate)` (was module-private) so the malformed/hand-seeded-file tests could target the real state file path, mirroring T02/T03. No doctor wiring, other-adapter logic, or recovery-barrier production behavior was touched. + - **Correction (this task, applied after initial completion):** the initial classifier's claim that `Pending`/`Flushing` are unconditionally `Recovering` "regardless of attempt state" was wrong. `resolve_recovery` only ever iterates and retires attempts already in `AttemptPhase::PendingAbandon`; it never inspects a `PendingStart` attempt. A reachable production sequence — A starts and is later abandoned via `ToolError` while B's own `Start` seam has already failed and left B `PendingStart` — persists `Pending` recovery (A `PendingAbandon`) alongside B's unrelated `PendingStart`, and the original classifier reported that `Recovering`. It is not: once recovery resolves A to `Clear`, the durable state becomes `Clear` + B `PendingStart`, which the classifier itself already (correctly) reports as `Blocked` — so the "Recovering" verdict was for a state with no ordinary future path back to normal admission, violating this plan's Recovering definition. The same shape recurs for orphaned `Flushing`. Fixed by reordering `classify_health`'s match so `has_pending_start` is checked as one condition spanning every `RecoveryState` value (`Clear`, `Pending`, and `Flushing` alike, whether or not a `PendingAbandon` is also outstanding), ahead of the `Pending`/`Flushing` → `Recovering` arms; only the `Clear` + `PendingAbandon` → `Invalid` structural-impossibility check still runs first. `Pending`/`Flushing` remain `Recovering` exactly when no `PendingStart` is outstanding. Two real-dispatch regressions were added: `pending_recovery_with_a_pending_start_attempt_is_blocked_even_though_an_unrelated_pending_abandon_can_still_clear_ac4` (the `Pending` critical regression: drives A through start/abandon with a failing ambiguity flush to leave `Pending` + mixed `PendingAbandon`/`PendingStart`, asserts `Blocked`, then drives an unrelated call C through real recovery resolution to `Clear` + `PendingStart` and asserts a further unrelated D is still denied and `Blocked` persists) and `orphaned_flushing_with_a_pending_start_attempt_is_blocked_not_recovering_ac4` (the `Flushing` analog, using a direct `begin_terminal_cleanup` call to simulate a crash before `resolve_recovery` ever ran, since that is the only way to observe a literal `Flushing` at rest). The full twelve-cell `(RecoveryState, has_pending_abandon, has_pending_start)` matrix — reachability, status, and what future event advances each cell — is recorded in `context/cli/opencode-mutation-scope-health.md`. No recovery/barrier production behavior changed; only the classifier's mapping, its reason text, and its supporting tests/docs were corrected. + - **Correction (this task, second pass — AC4 completeness gap closed):** the prior completion record claimed the "table-driven behavioral tests in AC4 pass for OpenCode" but the suite only ever drove individually named real-dispatch scenarios; no test actually enumerated all twelve `(RecoveryState, has_pending_abandon, has_pending_start)` cells as a table. Added `health_classification_matrix_covers_all_twelve_recovery_and_attempt_phase_combinations` in `cli/src/services/hooks/opencode_mutation_scope/health.rs`: a `[(RecoveryState, bool, bool, MutationScopeHealthStatus); 12]` table (via two small test-only helpers, `matrix_attempt` and `write_matrix_state`) that hand-builds adapter state for every cell — always including one `Active` attempt to also prove active attempts never change the classification — and asserts `classify_health` against the documented matrix. This is a completeness proof over the classifier's output, not a substitute for the existing real-dispatch regressions, which remain and continue to prove *why* the `Blocked` (mixed `PendingStart`), `Recovering` (`Pending`/`Flushing` with no `PendingStart`), and `Invalid` (`Clear` + `PendingAbandon`) equivalence classes have those classifications by driving production code. Also corrected an overstated claim in the `Flushing` reason string and in `context/cli/opencode-mutation-scope-health.md`: the previous wording implied a single "next tracked event" both reclaims an orphaned `Flushing` to `Pending` *and* retries `resolve_recovery`. In production these are two distinct steps performed by two distinct mechanisms — `normalize_recovery_after_boundary_lock_acquired` (run by *any* tracked adapter boundary) performs only the `Flushing` → `Pending` reclaim, while `resolve_recovery` (run only as part of a recovery-capable tracked *admission*, i.e. `admit_tracked_attempt`'s `Pending{g}` → `Flushing{g}` claim followed by `resolve_recovery`) is what actually claims the generation and retries the outstanding `PendingAbandon` abandonment. The classifier's `Flushing` reason text and the context doc's table/prose were reworded to state both steps explicitly; no classifier logic, recovery/barrier production behavior, or classification result changed. No T05/Pi work, doctor wiring, `doctor --fix`, protocol behavior, stale-`PendingStart` cleanup, or attribution-semantics change was made. + - **Correction (this task, third pass — AC4/Done-When wording alignment and matrix fixture cleanup):** the plan's AC4 and T04 Done When text still described the verification structure as one table-driven test proving "classifier result plus actual admission/recovery consequence per row," which no longer matched what the suite actually does (a classifier-completeness matrix over hand-built state, separate from representative real-dispatch regressions per semantic equivalence class). AC4 was rewritten to require, per adapter: (1) a table-driven classification matrix covering every meaningful reachable `RecoveryState`/attempt-phase combination; (2) real-dispatch behavioral tests proving the future admission/recovery semantics for every distinct semantic equivalence class used by that matrix, allowing multiple rows to share one behavioral proof under a demonstrated invariant; (3) the classifier matching those proven semantics exactly; (4) no classification inferred merely from enum/variant names — and now states explicitly that the matrix is the exhaustiveness layer and the real-dispatch regressions are the behavioral-justification layer, neither substituting for the other. T04's Done When was reworded to match: it no longer says "actual admission/recovery consequence per row" and instead requires the matrix be table-driven and complete, every distinct semantic equivalence class be proven through real adapter dispatch/recovery behavior, and absent/malformed state handling satisfy AC5. No wording elsewhere in this record overstated per-row behavioral execution, so no further correction to the Result/prior-correction prose was needed. Separately, `matrix_attempt` in `health.rs`'s test module was hand-building `scope_id` with a literal `format!("oc-tool-v1|s=8:ses-main|c=6:{call_id}")`, which produced invalid length prefixes for call IDs such as `call-active`/`call-abandon`/`call-start` (the classifier never inspects `scope_id`, so this did not affect any test result, but the fixture was not production-shaped). Changed `matrix_attempt` to build an `AttemptKey` and derive `scope_id` via the real production formatter, `super::super::format_opencode_scope_id(&key)`, instead of duplicating the encoding — no second test-only formatter was introduced. No classifier logic, recovery/barrier production behavior, or classification result changed by this pass. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml opencode_mutation_scope` — pass (103 tests, including the `health::tests` module: absent-file, clear-with-active-attempts healthy, `PendingStart`-blocked with two repeated unrelated denials proven via real dispatch, `Pending`-with-`PendingAbandon`-only-attempts recovering with repeated-denial-then-clearance proven via real dispatch, `Pending`-with-empty-attempts recovering, orphaned-`Flushing`-with-`PendingAbandon`-only recovering and reclaimed, the corrected `Pending`+mixed-`PendingStart` critical regression proven blocked before and after real recovery resolution, the corrected orphaned-`Flushing`+`PendingStart` regression proven blocked before and after real recovery resolution, hand-seeded `Clear`+`PendingAbandon` invalid, malformed-file invalid, and the `health_classification_matrix_covers_all_twelve_recovery_and_attempt_phase_combinations` table-driven completeness test, now built on production-shaped fixtures via `format_opencode_scope_id`). `cargo fmt --manifest-path cli/Cargo.toml --check` and `cargo clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` both clean. + - Context impact: domain — `context/cli/opencode-mutation-scope-health.md` is the canonical detailed owner of the proven Healthy/Recovering/Blocked/Invalid mapping, corrected to state that `Pending`/`Flushing` are `Recovering` only when no `PendingStart` attempt is also outstanding (a stale `PendingStart` is `Blocked` regardless of `RecoveryState`, not just under `Clear`), including the full twelve-cell state matrix and reachability answers; the matrix section now also points at the new table-driven completeness test, and every `Flushing`-reclaim mention now distinguishes the next-boundary reclaim from the tracked-admission-driven `resolve_recovery` retry instead of conflating them into one "next tracked event"; `context/cli/opencode-mutation-scope-integration.md` links to it from its "Adapter lifecycle and recovery" section rather than duplicating the mapping; `context/context-map.md` registers the health document (unchanged by this correction). + +- [x] T05: `Investigate and classify Pi mutation-scope health` (status:done) + - Task ID: T05 + - Scope: In — the same investigation and classifier work as T03, applied to `cli/src/services/hooks/pi_mutation_scope/state.rs`, explicitly accounting for `AttemptPhase::PendingStart`/`Executed`/`PendingAbandon`, `RecoveryState::Clear`/`Pending { generation }`/`Flushing { generation }`, `ProcessOwner`, and `is_definitely_dead()`. A live persisted attempt is not automatically `Blocked`. A stale `PendingStart`/`Executed` attempt whose owner is positively dead (`is_definitely_dead`) may still be `Recovering` if the existing next-boundary stale-owner recovery path (the D10 sweep) can retire it automatically — prove this by driving that sweep, not by asserting it from the field name. A `PendingAbandon` is classified `Blocked` only if investigation proves that, once the event that created it has returned, no ordinary future boundary can resume and complete cleanup; otherwise it is `Recovering`. Out — Codex, OpenCode, doctor wiring. + - Dependencies: T01 + - Done when: a complete table-driven Pi classification matrix covers every meaningful persisted `RecoveryState`/`AttemptPhase`/owner-liveness combination; real-dispatch behavioral tests prove every distinct semantic equivalence class represented by that matrix, including the required live-owner and definitely-dead-owner cases; multiple matrix rows may share one behavioral proof where a demonstrated invariant makes their future behavior equivalent; the classifier matches those proven semantics exactly; and absent/malformed state handling satisfies AC5. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml pi_mutation_scope` + - Context synchronization: synced + - Completed: 2026-09-19 + - Files changed: `cli/src/services/hooks/pi_mutation_scope/health.rs` (new), `cli/src/services/hooks/pi_mutation_scope/mod.rs`, `cli/src/services/hooks/pi_mutation_scope/state.rs` + - Result: Investigated every reachable `(RecoveryState, AttemptPhase, owner-liveness)` combination by reading and driving `pi_mutation_scope`'s real dispatch/recovery code (`admit_or_recover`, `reconcile_stale_owners` — the unconditional D10 stale-owner sweep that runs on every tracked Start from any session, before that session's own admission is even considered — `resolve_recovery`, `normalize_recovery_after_boundary_lock_acquired`) and its existing D8/D10/D12 test suite (notably `a_pending_start_attempt_never_blocks_a_concurrent_new_admission`, `an_owner_that_cannot_be_positively_proven_dead_is_never_abandoned_by_an_unrelated_start`, `a_dead_pending_start_attempt_is_recovered_by_an_unrelated_fresh_session_start`, `an_interrupted_stale_owner_recovery_remains_pending_and_denies_the_triggering_start_until_resumed`, `a_terminal_recovery_flush_failure_leaves_a_pending_recovery_and_denies_new_admission`). Key finding, specific to Pi and different from Codex/OpenCode: Pi's `admit_tracked_attempt` never gates a new admission on another attempt's `PendingStart`/`Executed` phase (only on a co-existing `PendingAbandon`, via `UncertainAttemptBlocked`, which is itself proven structurally unreachable in production because `PendingAbandon` only ever coexists with `RecoveryState != Clear`), and the D10 sweep unconditionally retires any dead-owner `PendingStart`/`Executed` attempt on the very next tracked Start regardless of session — so, unlike OpenCode's stuck-`PendingStart` wedge, Pi has no persisted-state combination reachable through ordinary production dispatch that classifies `Blocked`; only hand-seeded, code-proven-impossible combinations classify `Invalid`. Added `classify_health(git_dir) -> MutationScopeAdapterHealth` in a new `pi_mutation_scope/health.rs`, registered as `pub(crate) mod health;` in `mod.rs`: a read/parse error → `Invalid` with the error surfaced; `RecoveryState::Clear` with a `PendingAbandon` attempt present → `Invalid` (structurally impossible: `resolve_recovery` always removes every `PendingAbandon` attempt before `complete_recovery_flush` can transition to `Clear`); `RecoveryState::Clear` with a dead-owner (`is_definitely_dead`) `PendingStart`/`Executed` attempt and no `PendingAbandon` → `Recovering` (the D10 sweep is unfinished recovery work with a proven automatic path, even though it does not currently block anything); `RecoveryState::Clear` otherwise (including live- or uncertain-owner `PendingStart`/`Executed` attempts, which are never swept and never block unrelated admission) → `Healthy`; `RecoveryState::Pending`/`Flushing` with any otherwise valid attempt composition → `Recovering`, because a subsequent recovery-capable tracked Start whose key is not already represented by a nonterminal attempt can claim and retry the recovery generation without manual intervention — not lane- or key-scoped, unlike Codex, so a fresh caller from any session can perform it; an orphaned `Flushing` is first reclaimed to `Pending` by the next tracked adapter boundary. A duplicate Start for an already-tracked `PendingStart`/`Executed` key may be idempotently reused before `RecoveryState` is ever inspected and therefore does not necessarily advance recovery on its own; live/uncertain `PendingStart`/`Executed` attempts do not block unrelated admission, so this remains `Recovering`, not `Blocked`. Made `state::state_path` `pub(crate)` (was module-private) and `process_owner` a `pub(crate) mod` (was module-private) so `health.rs`'s tests could target the real state file path and construct `ProcessOwner` fixtures, mirroring T02–T04. No recovery/barrier production behavior changed; only the new classifier and its supporting tests were added. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml pi_mutation_scope` — pass (102 tests, including the new `health::tests` module: absent-file, clear-with-live-owner-attempt healthy, clear-with-uncertain-owner-attempt healthy with a real-dispatch proof that an unrelated Start never sweeps it, clear-with-dead-owner-`PendingStart` recovering with a real-dispatch D10-sweep proof reaching `Healthy`, clear-with-dead-owner-`Executed` recovering with a real-dispatch proof it is swept without a synthetic Close, pending-from-a-failed-terminal-abandon recovering with repeated-denial-then-self-heal proven via real dispatch, pending-from-an-interrupted-dead-owner-sweep recovering with the triggering call itself denied then resumed on retry, orphaned-Flushing recovering and reclaimed, hand-seeded `Clear`+`PendingAbandon` invalid, malformed-file invalid, a `health_classification_matrix_covers_all_twelve_recovery_and_attempt_condition_combinations` table-driven completeness test over `(RecoveryState, has_pending_abandon, has_dead_owner_attempt)`, and `pending_recovery_reuses_a_duplicate_start_for_an_existing_nonterminal_key_without_advancing_recovery_then_a_fresh_start_recovers` — proves via real dispatch that a `Pending` recovery with a live `PendingStart` key (B) and a `PendingAbandon` key (A) stays `Pending`/`Recovering` and its state untouched when a duplicate Start for B's own already-tracked key is reused idempotently, and only completes (`FlushClaimed` → `resolve_recovery` → `Clear` → `Healthy`) once a fresh Start for an untracked key (C) claims and retries the generation). `cargo fmt --manifest-path cli/Cargo.toml --check` and `SCE_CLI_PACKAGE_FALLBACK=1 cargo clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` both clean. + - Context impact: domain — `context/cli/pi-mutation-scope-health.md` is now the canonical detailed owner of the Pi health mapping and its behavioral justification (the D10 sweep's unconditional any-session reach, the unreachability of `UncertainAttemptBlocked`/`Blocked` in production, and the `Pending`/`Flushing` recovery path); `context/cli/pi-mutation-scope-integration.md` links to it instead of duplicating that mapping; `context/context-map.md` registers the document. Corrected nuance: `Pending`/`Flushing` have a normal recovery-capable future tracked-Start path (a proven any-caller resolution once claimed), but not every duplicate Start is required to advance recovery — `admit_tracked_attempt` reuses a duplicate Start for an already-tracked nonterminal (`PendingStart`/`Executed`) key idempotently, before `RecoveryState` is ever inspected, so only a Start whose key is not already tracked can claim the pending generation. + +- [ ] T06: `Report mutation-scope health in doctor JSON and human text output` (status:todo) + - Task ID: T06 + - Scope: In — `cli/src/services/doctor/types.rs` (a new report field carrying one health record per detected/configured adapter, plus stable `DoctorProblem` metadata for non-healthy records: category — a new stable `ProblemKind`/category value scoped to mutation-scope health rather than overloading an unrelated existing kind — severity, fixability, summary/detail, remediation, and next_action); `cli/src/services/doctor/inspect.rs` (calling each of the four adapters' T02–T05 classifiers for the targets doctor already detects/configures, never writing to any state file, and mapping `recovering` to a `Warning`-severity `DoctorProblem` and `blocked`/`invalid` to an `Error`-severity `DoctorProblem` per AC6); `cli/src/services/doctor/render.rs` (JSON serialization and human-text rendering using `[PASS]`/`[WARN]`/`[FAIL]` per AC2, and the existing healthy-row-collapse convention); `cli/src/services/doctor/mod.rs` if aggregation/readiness wiring is needed there. Every `blocked`/`invalid` record's remediation is `manual_only` deterministic guidance (never a recommendation to delete the state file) unless a safe generic recovery command already exists for that specific case, and states plainly when no such command exists yet. Out — any change to the classifiers themselves (T01–T05 own that); any `doctor --fix` mutation of adapter recovery state. + - Dependencies: T02, T03, T04, T05 + - Done when: AC1, AC2, and AC6 pass for all four adapters, including the collapsed-healthy-row case, an expanded row for each non-healthy status, and readiness/summary-count consistency for `recovering`-only and `blocked`/`invalid` repositories. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml doctor` + - Context synchronization: pending + +## Open questions + +None. The scope, the shared status vocabulary, the precise `healthy | +recovering | blocked | invalid` semantics, the per-adapter ownership boundary, +the Codex/OpenCode/Pi behavioral-investigation requirement, and the doctor +problem/readiness wiring were all resolved during clarification.