diff --git a/cli/src/services/hooks/claude_mutation_scope/health.rs b/cli/src/services/hooks/claude_mutation_scope/health.rs index 35e1a97fe..88218fe45 100644 --- a/cli/src/services/hooks/claude_mutation_scope/health.rs +++ b/cli/src/services/hooks/claude_mutation_scope/health.rs @@ -149,48 +149,6 @@ mod tests { 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"); @@ -768,8 +726,58 @@ mod tests { } #[test] - fn clear_recovery_with_pending_abandon_is_invalid() { - let git_dir = unique_test_git_dir("invalid-clear-pending-abandon"); + fn health_classification_table_covers_every_recovery_pending_and_attempt_phase_shape() { + use state::AttemptPhase::{Active, PendingAbandon, PendingStart}; + use MutationScopeHealthStatus::{Blocked, Healthy, Invalid, Recovering}; + + let rows: [(bool, &[state::AttemptPhase], MutationScopeHealthStatus); 12] = [ + (false, &[], Healthy), + (false, &[PendingStart], Healthy), + (false, &[Active], Healthy), + (false, &[PendingAbandon], Invalid), + (false, &[PendingAbandon, PendingStart], Invalid), + (false, &[PendingAbandon, Active], Invalid), + (true, &[], Recovering), + (true, &[PendingStart], Blocked), + (true, &[Active], Blocked), + (true, &[PendingAbandon], Blocked), + (true, &[PendingAbandon, PendingStart], Blocked), + (true, &[PendingAbandon, Active], Blocked), + ]; + + for (index, (recovery_pending, phases, expected)) in rows.into_iter().enumerate() { + let git_dir = unique_test_git_dir(&format!("table-{index}")); + let attempts: Vec = (1u64..) + .zip(phases) + .map(|(attempt_seq, phase)| { + hand_constructed_attempt(attempt_seq, &format!("toolu_{attempt_seq}"), *phase) + }) + .collect(); + let next_attempt_seq = attempts.last().map_or(1, |attempt| attempt.attempt_seq + 1); + write_hand_constructed_state( + &git_dir, + &state::AdapterState { + version: 1, + next_attempt_seq, + recovery_pending, + attempts, + }, + ); + + assert_eq!( + classify_health(&git_dir).status, + expected, + "row {index}: recovery_pending={recovery_pending} phases={phases:?}", + ); + + remove_test_git_dir(&git_dir); + } + } + + #[test] + fn assess_repairability_is_manual_only_for_an_impossible_clear_recovery_with_a_pending_abandon_attempt( + ) { + let git_dir = unique_test_git_dir("assess-invalid-clear-pending-abandon"); write_hand_constructed_state( &git_dir, &state::AdapterState { @@ -784,13 +792,6 @@ mod tests { }, ); - let health = classify_health(&git_dir); - assert_eq!( - health.status, - MutationScopeHealthStatus::Invalid, - "recovery_pending == false with an unresolved PendingAbandon attempt is a \ - structurally impossible state and must never classify Healthy" - ); assert_eq!( assess_repairability(&git_dir), Repairability::ManualOnly, @@ -799,31 +800,4 @@ mod tests { remove_test_git_dir(&git_dir); } - - #[test] - fn invalid_takes_priority_over_blocked_in_mixed_impossible_state() { - let git_dir = unique_test_git_dir("invalid-priority-mixed"); - write_hand_constructed_state( - &git_dir, - &state::AdapterState { - version: 1, - next_attempt_seq: 3, - recovery_pending: false, - attempts: vec![ - hand_constructed_attempt(1, "toolu_1", state::AttemptPhase::PendingAbandon), - hand_constructed_attempt(2, "toolu_2", state::AttemptPhase::PendingStart), - ], - }, - ); - - let health = classify_health(&git_dir); - assert_eq!( - health.status, - MutationScopeHealthStatus::Invalid, - "Clear + unresolved PendingAbandon must take priority over an otherwise-Blocked \ - shape from a coexisting PendingStart attempt" - ); - - remove_test_git_dir(&git_dir); - } } diff --git a/cli/src/services/hooks/mutation_scope_health.rs b/cli/src/services/hooks/mutation_scope_health.rs index 2fbd2ec26..122628d98 100644 --- a/cli/src/services/hooks/mutation_scope_health.rs +++ b/cli/src/services/hooks/mutation_scope_health.rs @@ -45,63 +45,3 @@ impl MutationScopeAdapterHealth { 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 repairability_variants_are_distinct() { - assert_ne!(Repairability::AutoFixable, Repairability::ManualOnly); - assert_eq!(Repairability::AutoFixable, Repairability::AutoFixable); - } - - #[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 index 45dbfaef9..4eb00cf52 100644 --- a/cli/src/services/hooks/opencode_mutation_scope/health.rs +++ b/cli/src/services/hooks/opencode_mutation_scope/health.rs @@ -232,19 +232,6 @@ mod tests { 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"); @@ -608,38 +595,6 @@ mod tests { 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"); @@ -683,13 +638,17 @@ mod tests { fn write_matrix_state( git_dir: &Path, recovery: RecoveryState, + has_active: bool, 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)]; + let mut attempts = Vec::new(); + if has_active { + attempts.push(matrix_attempt("call-active", AttemptPhase::Active)); + } if has_pending_abandon { attempts.push(matrix_attempt("call-abandon", AttemptPhase::PendingAbandon)); } @@ -736,17 +695,26 @@ mod tests { 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); + for has_active in [false, true] { + let git_dir = unique_test_git_dir(&format!("matrix-{index}-{has_active}")); + write_matrix_state( + &git_dir, + recovery, + has_active, + has_pending_abandon, + has_pending_start, + ); + + assert_eq!( + classify_health(&git_dir).status, + expected, + "row {index}: recovery={recovery:?} has_active={has_active} \ + 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/pi_mutation_scope/health.rs b/cli/src/services/hooks/pi_mutation_scope/health.rs index 587eeaec3..84d6abddd 100644 --- a/cli/src/services/hooks/pi_mutation_scope/health.rs +++ b/cli/src/services/hooks/pi_mutation_scope/health.rs @@ -213,10 +213,11 @@ mod tests { } #[test] - fn clear_recovery_is_healthy_with_a_live_owner_pending_start_attempt() { + fn clear_recovery_is_healthy_with_a_live_owner_pending_start_attempt_never_swept_by_an_unrelated_start( + ) { 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( + let attempt = state::seed_attempt_for_tests( &git_dir, &key("ses-1", "call-1"), "bash", @@ -228,6 +229,30 @@ mod tests { MutationScopeHealthStatus::Healthy ); + let seam = RecordingSeam::new(); + drive(&git_dir, &seam, &tool_call_event("ses-c", "call-c")) + .expect("an unrelated session's Start must proceed alongside a live-owner attempt"); + assert_eq!( + seam.calls.lock().expect("seam mutex").clone(), + vec!["start".to_string()], + "no D10 sweep may fire for an owner that is still alive" + ); + let after = state::read_state(&git_dir).expect("state readable"); + assert_eq!( + after + .attempts + .iter() + .find(|a| a.scope_id == attempt.scope_id) + .expect("the live-owner attempt is still tracked") + .phase, + AttemptPhase::PendingStart, + ); + + assert_eq!( + classify_health(&git_dir).status, + MutationScopeHealthStatus::Healthy + ); + remove_test_git_dir(&git_dir); } @@ -504,41 +529,6 @@ mod tests { 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"); @@ -583,24 +573,27 @@ mod tests { fn write_matrix_state( git_dir: &Path, recovery: RecoveryState, + bystander_owner: Option, 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, - )]; + let mut attempts = Vec::new(); + if let Some(owner) = bystander_owner { + attempts.push(matrix_attempt( + "call-bystander", + AttemptPhase::PendingStart, + owner, + )); + } if has_pending_abandon { attempts.push(matrix_attempt( "call-abandon", AttemptPhase::PendingAbandon, - live_owner, + crate::services::hooks::mutation_scope_owner::current_process_owner(), )); } if has_dead_owner_attempt { @@ -640,6 +633,15 @@ mod tests { instance_token: None, }; let live_owner = crate::services::hooks::mutation_scope_owner::current_process_owner(); + let uncertain_owner = ProcessOwner { + pid: std::process::id().cast_signed(), + instance_token: None, + }; + let bystanders = [ + ("none", None), + ("live-owner", Some(live_owner)), + ("uncertain-owner", Some(uncertain_owner)), + ]; let clear = RecoveryState::Clear; let pending = RecoveryState::Pending { generation: 1 }; @@ -663,24 +665,27 @@ mod tests { 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); + for (bystander, bystander_owner) in bystanders { + let git_dir = unique_test_git_dir(&format!("matrix-{index}-{bystander}")); + write_matrix_state( + &git_dir, + recovery, + bystander_owner, + has_pending_abandon, + has_dead_owner_attempt, + dead_owner, + ); + + assert_eq!( + classify_health(&git_dir).status, + expected, + "row {index}: recovery={recovery:?} bystander={bystander} \ + has_pending_abandon={has_pending_abandon} \ + has_dead_owner_attempt={has_dead_owner_attempt}", + ); + + remove_test_git_dir(&git_dir); + } } } diff --git a/context/cli/doctor-recovery-safety-model.md b/context/cli/doctor-recovery-safety-model.md index 397726603..364c3a19b 100644 --- a/context/cli/doctor-recovery-safety-model.md +++ b/context/cli/doctor-recovery-safety-model.md @@ -54,7 +54,7 @@ each adapter's real dispatch/recovery functions, not reading enum names. | `DoctorNeverAbandonsALiveOwner` — doctor cannot abandon a potentially live attempt | `opencode_mutation_scope::health::tests::repair_blocked_is_a_safe_no_op_when_the_pending_start_owner_is_live`, `opencode_mutation_scope::health::tests::assess_repairability_is_manual_only_when_the_pending_start_owner_is_live` | | `UnknownOwnerNeverProvesDeath` — an unprovable/unknown owner is never proof of death | `opencode_mutation_scope::health::tests::assess_repairability_is_manual_only_for_a_legacy_pending_start_attempt_with_no_recorded_owner`; `doctor::inspect::tests::full_report_multi_adapter_diagnose_names_doctor_fix_for_one_target_and_the_real_path_for_the_other` (an unowned OpenCode `PendingStart` stays `ManualOnly` in the same report where Claude's proven-dead-equivalent state is `AutoFixable`) | | `RecoveryNeverClearedWithUnresolvedAbandon` — doctor cannot clear recovery while unresolved lifecycle evidence exists | `claude_mutation_scope::health::tests::repair_blocked_removes_only_successfully_abandoned_attempts_and_keeps_recovery_pending_when_one_fails` | -| `NoOrdinaryTransitionProducesInvalidHealth` — `Invalid` is reserved for structurally impossible states, never an ordinary transition's output | `claude_mutation_scope::health::tests::clear_recovery_with_pending_abandon_is_invalid`, `claude_mutation_scope::health::tests::invalid_takes_priority_over_blocked_in_mixed_impossible_state`, `opencode_mutation_scope::health::tests::clear_recovery_with_a_pending_abandon_attempt_is_a_structurally_impossible_state_classified_invalid` | +| `NoOrdinaryTransitionProducesInvalidHealth` — `Invalid` is reserved for structurally impossible states, never an ordinary transition's output | `claude_mutation_scope::health::tests::health_classification_table_covers_every_recovery_pending_and_attempt_phase_shape` (the `recovery_pending == false` + `PendingAbandon` rows, alone and alongside a `PendingStart`/`Active` attempt), `opencode_mutation_scope::health::tests::health_classification_matrix_covers_all_twelve_recovery_and_attempt_phase_combinations` (the `Clear` + `PendingAbandon` rows) | | `DoctorRepairProducesOnlyOrdinaryLifecycleShapes` — a doctor repair cannot produce a state outside the adapter's own reachable transition set | `opencode_mutation_scope::health::tests::repair_blocked_clears_a_dead_owner_pending_start_end_to_end`; `claude_mutation_scope::health::tests::repair_blocked_removes_only_successfully_abandoned_attempts_and_keeps_recovery_pending_when_one_fails` | | `RemovedAttemptsAreNeverResurrected` — a terminal/removed attempt is never resurrected | `claude_mutation_scope::state::tests::removing_an_already_removed_attempt_is_a_safe_no_op`, `opencode_mutation_scope::state::state_conformance::removing_an_already_removed_attempt_is_a_safe_no_op` (shared contract in `cli/src/services/hooks/mutation_scope_state_conformance.rs`, also instantiated for Codex and Pi), `opencode_mutation_scope::tests::regression_f_start_replay_for_a_pending_abandon_identity_never_reactivates` | | `InterruptedRecoveryStaysInOrdinaryRetryableState` — an interrupted repair remains fail-closed and retryable | `claude_mutation_scope::health::tests::repair_blocked_interrupted_by_a_failing_seam_leaves_state_a_later_repair_completes_without_duplication`, `opencode_mutation_scope::health::tests::repair_blocked_interrupted_before_the_seam_resolves_leaves_state_the_ordinary_recovery_path_completes_without_duplication` | diff --git a/context/cli/opencode-mutation-scope-health.md b/context/cli/opencode-mutation-scope-health.md index 82174cde0..5ad1afd08 100644 --- a/context/cli/opencode-mutation-scope-health.md +++ b/context/cli/opencode-mutation-scope-health.md @@ -44,8 +44,10 @@ 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 +the classifier's output for every row against hand-built state, each row both +with and without an `Active` attempt (proving it never changes the result, and +covering the empty-attempts and lone-`PendingAbandon` shapes). That matrix is +the single owner of pure state-shape classification; 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: diff --git a/context/cli/pi-mutation-scope-health.md b/context/cli/pi-mutation-scope-health.md index 8c9f03bb0..8aee4e70b 100644 --- a/context/cli/pi-mutation-scope-health.md +++ b/context/cli/pi-mutation-scope-health.md @@ -52,6 +52,8 @@ process-instance identity cannot be positively established, per `is_definitely_dead` in the shared `hooks/mutation_scope_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_a_live_owner_pending_start_attempt_never_swept_by_an_unrelated_start` +and `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`