From 67cba705bae68b4e9a30c066a100a421a54735d0 Mon Sep 17 00:00:00 2001 From: David Abram Date: Fri, 18 Sep 2026 23:36:59 +0200 Subject: [PATCH 1/6] context: Add mutation-scope health diagnostic plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Document the need for `sce doctor` to surface persisted mutation-scope runtime health instead of exposing only hook registration state. Define the shared status contract, adapter-owned classification investigations, doctor rendering requirements, and validation/context synchronization work while keeping recovery behavior unchanged. Plan: doctor-mutation-scope-health (T01–T06) Co-authored-by: SCE --- context/plans/doctor-mutation-scope-health.md | 319 ++++++++++++++++++ 1 file changed, 319 insertions(+) create mode 100644 context/plans/doctor-mutation-scope-health.md diff --git a/context/plans/doctor-mutation-scope-health.md b/context/plans/doctor-mutation-scope-health.md new file mode 100644 index 00000000..96143e5e --- /dev/null +++ b/context/plans/doctor-mutation-scope-health.md @@ -0,0 +1,319 @@ +# 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, table-driven behavioral tests + cover every meaningful reachable `RecoveryState`/attempt-phase + combination and prove — from actual future admission/recovery behavior, + not only the classifier's output — why each is `healthy | recovering | + blocked | invalid`. The adapter's classifier implements exactly that + proven mapping before `doctor` is allowed to report `blocked` for that + adapter. Classifications are never inferred from enum/variant names. + - 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 + +- [ ] T01: `Define the shared mutation-scope health status contract` (status:todo) + - 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`) with doc comments that restate the precise semantics in [Health status definitions](#health-status-definitions) (not just the enum names), and a 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 type compiles, derives whatever traits its consumers need (`Debug`/`Clone`/`PartialEq`/`serde::Serialize` as required by JSON rendering), its doc comments match the plan's health-status definitions exactly (including that `recovering` requires a proven future self-healing path and `blocked` requires the absence of one), and it has unit tests only for its own invariants (e.g. any ordering/aggregation helper it exposes). + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml scope_health` + - Context synchronization: pending + +- [ ] T02: `Classify Claude mutation-scope health` (status:todo) + - 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: pending + +- [ ] T03: `Investigate and classify Codex mutation-scope health` (status:todo) + - 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 test module records, per row, the semantic answer to "what future event gets this state out?" alongside the resulting status; a classifier function implements exactly that proven mapping against the T01 status type. Out — OpenCode, Pi, doctor wiring. + - Dependencies: T01 + - Done when: the table-driven behavioral tests in AC4 pass for Codex — each asserting both the classifier result and the actual admission/recovery consequence that justifies it (e.g. seed/create the state, assert `classify_health(...)`, then invoke the next normal boundary and assert it either advances recovery or remains denied) — the classifier matches them exactly, and absent/malformed state files classify per AC5. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml codex_mutation_scope` + - Context synchronization: pending + +- [ ] T04: `Investigate and classify OpenCode mutation-scope health` (status:todo) + - 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 table-driven behavioral tests in AC4 pass for OpenCode (classifier result plus actual admission/recovery consequence per row, as in T03), the classifier matches them exactly, and absent/malformed state files classify per AC5. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml opencode_mutation_scope` + - Context synchronization: pending + +- [ ] T05: `Investigate and classify Pi mutation-scope health` (status:todo) + - 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: the table-driven behavioral tests in AC4 pass for Pi (classifier result plus actual admission/recovery consequence per row, as in T03, including at least one dead-owner and one live-owner case), the classifier matches them exactly, and absent/malformed state files classify per AC5. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml pi_mutation_scope` + - Context synchronization: pending + +- [ ] 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. From be3d58284ddbf0489065d6543cfbdbe83fd8f8b7 Mon Sep 17 00:00:00 2001 From: David Abram Date: Sat, 19 Sep 2026 00:07:54 +0200 Subject: [PATCH 2/6] hooks: Add mutation-scope health contract Define shared Healthy, Recovering, Blocked, and Invalid statuses plus a per-adapter reason and detail record, and register the new module for upcoming doctor classification work. Record completion of T01 in the doctor-mutation-scope-health plan. Plan: doctor-mutation-scope-health (T01) Co-authored-by: SCE --- cli/src/services/hooks/mod.rs | 1 + .../services/hooks/mutation_scope_health.rs | 94 +++++++++++++++++++ context/plans/doctor-mutation-scope-health.md | 13 ++- 3 files changed, 104 insertions(+), 4 deletions(-) create mode 100644 cli/src/services/hooks/mutation_scope_health.rs diff --git a/cli/src/services/hooks/mod.rs b/cli/src/services/hooks/mod.rs index 52a19c67..79c6bb2a 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 00000000..783f2eb3 --- /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/context/plans/doctor-mutation-scope-health.md b/context/plans/doctor-mutation-scope-health.md index 96143e5e..c1b94266 100644 --- a/context/plans/doctor-mutation-scope-health.md +++ b/context/plans/doctor-mutation-scope-health.md @@ -263,13 +263,18 @@ classifier stays pure and read-only regardless. ## Task stack -- [ ] T01: `Define the shared mutation-scope health status contract` (status:todo) +- [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`) with doc comments that restate the precise semantics in [Health status definitions](#health-status-definitions) (not just the enum names), and a 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. + - 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 type compiles, derives whatever traits its consumers need (`Debug`/`Clone`/`PartialEq`/`serde::Serialize` as required by JSON rendering), its doc comments match the plan's health-status definitions exactly (including that `recovering` requires a proven future self-healing path and `blocked` requires the absence of one), and it has unit tests only for its own invariants (e.g. any ordering/aggregation helper it exposes). + - 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: pending + - 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. - [ ] T02: `Classify Claude mutation-scope health` (status:todo) - Task ID: T02 From fd3b0afbc6600e6f2c157a222885ec6f12494492 Mon Sep 17 00:00:00 2001 From: David Abram Date: Sat, 19 Sep 2026 01:43:55 +0200 Subject: [PATCH 3/6] hooks: Add Claude mutation-scope health classification Expose a read-only health classifier so doctor checks can distinguish healthy, recovering, blocked, and invalid persisted mutation-scope state. Treat unresolved recovery attempts as blocked because the recovery barrier cannot self-clear while attempts remain, and surface state read errors as invalid. Ref: context/plans/doctor-mutation-scope-health.md (T02) Co-authored-by: SCE --- .../hooks/claude_mutation_scope/health.rs | 227 ++++++++++++++++++ .../hooks/claude_mutation_scope/mod.rs | 1 + .../hooks/claude_mutation_scope/state.rs | 2 +- .../cli/claude-mutation-scope-integration.md | 62 +++++ context/plans/doctor-mutation-scope-health.md | 9 +- 5 files changed, 298 insertions(+), 3 deletions(-) create mode 100644 cli/src/services/hooks/claude_mutation_scope/health.rs 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 00000000..0f2dff25 --- /dev/null +++ b/cli/src/services/hooks/claude_mutation_scope/health.rs @@ -0,0 +1,227 @@ +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 5081c5cf..aa4c72c0 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 aa920de5..ad19ef48 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/context/cli/claude-mutation-scope-integration.md b/context/cli/claude-mutation-scope-integration.md index 3256f88c..995c2f50 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/plans/doctor-mutation-scope-health.md b/context/plans/doctor-mutation-scope-health.md index c1b94266..b5f96883 100644 --- a/context/plans/doctor-mutation-scope-health.md +++ b/context/plans/doctor-mutation-scope-health.md @@ -276,13 +276,18 @@ classifier stays pure and read-only regardless. - 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. -- [ ] T02: `Classify Claude mutation-scope health` (status:todo) +- [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: pending + - 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. - [ ] T03: `Investigate and classify Codex mutation-scope health` (status:todo) - Task ID: T03 From 75035d0fd67395d65c6bc5b2d0abdc67ec5c3a90 Mon Sep 17 00:00:00 2001 From: David Abram Date: Sat, 19 Sep 2026 02:08:35 +0200 Subject: [PATCH 4/6] runtime: Add Codex mutation-scope health classifier Surface Codex recovery-state combinations through the shared health vocabulary so doctor can distinguish normal lifecycle state, self-healing recovery, persistent admission blockage, and invalid persisted state. The read-only classifier is backed by behavioral tests that drive real admission, recovery, and failed-cleanup paths, including repeated global denial for unresolved attempts and orphaned flush reclamation. Plan: `context/plans/doctor-mutation-scope-health.md` (T03) Co-authored-by: SCE --- .../hooks/claude_mutation_scope/health.rs | 3 +- .../hooks/codex_mutation_scope/health.rs | 264 ++++++++++++++++++ .../hooks/codex_mutation_scope/mod.rs | 193 +++++++++++++ .../hooks/codex_mutation_scope/state.rs | 2 +- context/cli/codex-mutation-scope-health.md | 88 ++++++ .../cli/codex-mutation-scope-integration.md | 3 +- context/context-map.md | 1 + context/plans/doctor-mutation-scope-health.md | 10 +- 8 files changed, 559 insertions(+), 5 deletions(-) create mode 100644 cli/src/services/hooks/codex_mutation_scope/health.rs create mode 100644 context/cli/codex-mutation-scope-health.md diff --git a/cli/src/services/hooks/claude_mutation_scope/health.rs b/cli/src/services/hooks/claude_mutation_scope/health.rs index 0f2dff25..22f22a4d 100644 --- a/cli/src/services/hooks/claude_mutation_scope/health.rs +++ b/cli/src/services/hooks/claude_mutation_scope/health.rs @@ -209,7 +209,8 @@ mod tests { ); for attempt_number in 1..=2 { - let outcome = apply_recovery_barrier(&git_dir, repository_root, None, &unreachable_seam); + 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" 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 00000000..52968cb8 --- /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 f09afdcd..beaba255 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 7e0162f7..e7ab576e 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/context/cli/codex-mutation-scope-health.md b/context/cli/codex-mutation-scope-health.md new file mode 100644 index 00000000..8d5997ae --- /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 2585c36e..96bb7fd6 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/context-map.md b/context/context-map.md index 1f5c159a..4b52b99a 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -107,6 +107,7 @@ 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)) diff --git a/context/plans/doctor-mutation-scope-health.md b/context/plans/doctor-mutation-scope-health.md index b5f96883..cac0a00f 100644 --- a/context/plans/doctor-mutation-scope-health.md +++ b/context/plans/doctor-mutation-scope-health.md @@ -289,13 +289,19 @@ classifier stays pure and read-only regardless. - 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. -- [ ] T03: `Investigate and classify Codex mutation-scope health` (status:todo) +- [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 test module records, per row, the semantic answer to "what future event gets this state out?" alongside the resulting status; a classifier function implements exactly that proven mapping against the T01 status type. Out — OpenCode, Pi, doctor wiring. - Dependencies: T01 - Done when: the table-driven behavioral tests in AC4 pass for Codex — each asserting both the classifier result and the actual admission/recovery consequence that justifies it (e.g. seed/create the state, assert `classify_health(...)`, then invoke the next normal boundary and assert it either advances recovery or remains denied) — the classifier matches them exactly, and absent/malformed state files classify per AC5. - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml codex_mutation_scope` - - Context synchronization: pending + - 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. - [ ] T04: `Investigate and classify OpenCode mutation-scope health` (status:todo) - Task ID: T04 From 2eaf705cabeaacfe0f3e40e06d8575c73f61f7b4 Mon Sep 17 00:00:00 2001 From: David Abram Date: Sat, 19 Sep 2026 09:34:43 +0200 Subject: [PATCH 5/6] hooks: Add OpenCode mutation-scope health classification Classify persisted OpenCode adapter state as healthy, recovering, blocked, or invalid, including stale PendingStart and structurally impossible PendingAbandon states. Exercise the real dispatch and recovery paths with focused tests, and document the state mapping for doctor integration. Ref: doctor-mutation-scope-health T04 Co-authored-by: SCE --- .../hooks/opencode_mutation_scope/health.rs | 723 ++++++++++++++++++ .../hooks/opencode_mutation_scope/mod.rs | 1 + .../hooks/opencode_mutation_scope/state.rs | 2 +- context/cli/opencode-mutation-scope-health.md | 211 +++++ .../opencode-mutation-scope-integration.md | 2 + context/context-map.md | 1 + context/plans/doctor-mutation-scope-health.md | 43 +- 7 files changed, 969 insertions(+), 14 deletions(-) create mode 100644 cli/src/services/hooks/opencode_mutation_scope/health.rs create mode 100644 context/cli/opencode-mutation-scope-health.md 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 00000000..f861646b --- /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 e4da0060..8a42f86b 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 1d7d8272..ec77f17a 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/context/cli/opencode-mutation-scope-health.md b/context/cli/opencode-mutation-scope-health.md new file mode 100644 index 00000000..df91473a --- /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 27b455f9..9ec86df3 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/context-map.md b/context/context-map.md index 4b52b99a..14970dbf 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -112,6 +112,7 @@ Additional mutation-scope integration context: - `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 — `Pending`/`Flushing` are always `recovering` regardless of attempt state, because `admit_tracked_attempt`'s flush claim and `resolve_recovery`'s retry-every-`PendingAbandon`-attempt loop are not scoped to any particular session or call (stronger than Codex's same-lane-only sweep); a `Clear`-recovery attempt stuck in `PendingStart` is `blocked` instead — 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; `Clear` with a `PendingAbandon` attempt and any read/parse failure are `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/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`) diff --git a/context/plans/doctor-mutation-scope-health.md b/context/plans/doctor-mutation-scope-health.md index cac0a00f..536c115d 100644 --- a/context/plans/doctor-mutation-scope-health.md +++ b/context/plans/doctor-mutation-scope-health.md @@ -175,13 +175,22 @@ classifier stays pure and read-only regardless. 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, table-driven behavioral tests - cover every meaningful reachable `RecoveryState`/attempt-phase - combination and prove — from actual future admission/recovery behavior, - not only the classifier's output — why each is `healthy | recovering | - blocked | invalid`. The adapter's classifier implements exactly that - proven mapping before `doctor` is allowed to report `blocked` for that - adapter. Classifications are never inferred from enum/variant names. +- [ ] 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 @@ -291,9 +300,9 @@ classifier stays pure and read-only regardless. - [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 test module records, per row, the semantic answer to "what future event gets this state out?" alongside the resulting status; a classifier function implements exactly that proven mapping against the T01 status type. Out — OpenCode, Pi, doctor wiring. + - 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: the table-driven behavioral tests in AC4 pass for Codex — each asserting both the classifier result and the actual admission/recovery consequence that justifies it (e.g. seed/create the state, assert `classify_health(...)`, then invoke the next normal boundary and assert it either advances recovery or remains denied) — the classifier matches them exactly, and absent/malformed state files classify per AC5. + - 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 @@ -303,19 +312,27 @@ classifier stays pure and read-only regardless. - 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. -- [ ] T04: `Investigate and classify OpenCode mutation-scope health` (status:todo) +- [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 table-driven behavioral tests in AC4 pass for OpenCode (classifier result plus actual admission/recovery consequence per row, as in T03), the classifier matches them exactly, and absent/malformed state files classify per AC5. + - 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: pending + - 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). - [ ] T05: `Investigate and classify Pi mutation-scope health` (status:todo) - 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: the table-driven behavioral tests in AC4 pass for Pi (classifier result plus actual admission/recovery consequence per row, as in T03, including at least one dead-owner and one live-owner case), the classifier matches them exactly, and absent/malformed state files classify per AC5. + - 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: pending From 6b6da8d64f344f53a86c083eeab31d696fec9f88 Mon Sep 17 00:00:00 2001 From: David Abram Date: Sat, 19 Sep 2026 20:32:31 +0200 Subject: [PATCH 6/6] hooks: Add Pi mutation-scope health classification Classify persisted Pi adapter state as healthy, recovering, or invalid, including dead-owner attempts and structurally impossible PendingAbandon states. Exercise the real dispatch and recovery paths with focused tests, and document the state mapping for doctor integration. Plan: `context/plans/doctor-mutation-scope-health.md` (T05) Co-authored-by: SCE --- .../hooks/pi_mutation_scope/health.rs | 766 ++++++++++++++++++ .../services/hooks/pi_mutation_scope/mod.rs | 3 +- .../services/hooks/pi_mutation_scope/state.rs | 2 +- context/cli/pi-mutation-scope-health.md | 123 +++ context/cli/pi-mutation-scope-integration.md | 3 + context/context-map.md | 3 +- context/plans/doctor-mutation-scope-health.md | 9 +- 7 files changed, 904 insertions(+), 5 deletions(-) create mode 100644 cli/src/services/hooks/pi_mutation_scope/health.rs create mode 100644 context/cli/pi-mutation-scope-health.md 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 00000000..d7051bed --- /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 bfa4e855..a75a3e74 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 c7d4bc2e..3c9ca81d 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/pi-mutation-scope-health.md b/context/cli/pi-mutation-scope-health.md new file mode 100644 index 00000000..b44729c7 --- /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 522d779e..21d08eaa 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 14970dbf..e3fc9c1e 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -112,8 +112,9 @@ Additional mutation-scope integration context: - `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 — `Pending`/`Flushing` are always `recovering` regardless of attempt state, because `admit_tracked_attempt`'s flush claim and `resolve_recovery`'s retry-every-`PendingAbandon`-attempt loop are not scoped to any particular session or call (stronger than Codex's same-lane-only sweep); a `Clear`-recovery attempt stuck in `PendingStart` is `blocked` instead — 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; `Clear` with a `PendingAbandon` attempt and any read/parse failure are `invalid`) +- `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 index 536c115d..4c467452 100644 --- a/context/plans/doctor-mutation-scope-health.md +++ b/context/plans/doctor-mutation-scope-health.md @@ -328,13 +328,18 @@ classifier stays pure and read-only regardless. - 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). -- [ ] T05: `Investigate and classify Pi mutation-scope health` (status:todo) +- [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: pending + - 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