From a806a727cb7b00a4a95b6ea498646359591a6b80 Mon Sep 17 00:00:00 2001 From: David Abram Date: Wed, 2 Sep 2026 23:34:54 +0200 Subject: [PATCH 1/6] runtime: Add mutation-scope integration plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Define the protected-worktree extraction, abandonment entrypoint, store seam, regression coverage, and adapter contract for safely retiring stale mutation scopes while preserving coordinate behavior. Plan: mutation-scope-runtime-integration (T01–T05) --- .../mutation-scope-runtime-integration.md | 362 ++++++++++++++++++ 1 file changed, 362 insertions(+) create mode 100644 context/plans/mutation-scope-runtime-integration.md diff --git a/context/plans/mutation-scope-runtime-integration.md b/context/plans/mutation-scope-runtime-integration.md new file mode 100644 index 00000000..9ade374d --- /dev/null +++ b/context/plans/mutation-scope-runtime-integration.md @@ -0,0 +1,362 @@ +# Plan: mutation-scope-runtime-integration + +## Change summary + +Make the already-verified `protocol::abandon()` action reachable from production +runtime code, so a future harness adapter has a safe way to end a mutation scope +it can prove is stale but for which it never observed a trustworthy final +worktree boundary. Today `cli/src/services/mutation_trace/runtime/` exposes only +`coordinate()` — an *observed*-boundary path (`Start`/`Advance`/`Close`/`Flush`) +that always captures a Git snapshot. A dead agent process has no terminal +observation, so an adapter has no way to retire its scope. A dead execution can +therefore leave its scope `Active` indefinitely. This is unsafe: a later `Start` +observes the worktree before activating its successor — `commit` computes +`active_scopes`/`attribution` against the state as it existed *before* the same +call's own scope-lifecycle transition — so changes made after the dead execution +may be incorrectly classified as `AiExclusive` to the stale scope. Once another +scope starts, subsequent overlapping intervals also become `AiContended` against +the zombie. + +This change adds a second runtime entrypoint, `abandon_scope()`, that shares +`coordinate()`'s safety prefix (worktree lock → external-taint fence → checkout +identity → DB) but deliberately takes **no Git snapshot**: abandonment means the +final mutation boundary was never observed, and snapshotting would silently give +it `Close`'s observation semantics. It extracts that shared prefix into one +internal `ProtectedWorktree` primitive so the two entrypoints cannot drift, +exposes the smallest read seam the new path needs from `MutationTraceStore`, and +records the mutation-scope lifecycle contract every later harness adapter (Codex, +Claude Code, OpenCode, Pi) must uphold. + +This extends existing behavior and preserves it: `coordinate()`'s externally +observable ordering, error variants, and outcomes are unchanged, `Abandon` does +**not** become a `RuntimeBoundary` variant, and nothing here changes +`spec/mutation_cursor.qnt`, `protocol.rs` semantics, the mutation-trace SQL +schema, migrations, or `diff_traces`. No harness is wired by this plan. + +## Acceptance criteria + +How this plan is proven complete. Each criterion is observable and names the +check that proves it. `/validate` runs these checks; no task in the stack +performs final validation. + +- [ ] AC1: The `coordinate()` path runs through the shared protected-worktree + primitive with its current externally observable ordering and error semantics + intact: worktree lock → external marker inspect/persist → checkout identity → + DB provider → snapshot/recovery/protocol/CAS → explicit marker clear, with + `CoordinateError::LockAcquisition`, `ExternalTaintMarker { Inspect | Persist }`, + `AgentTraceDbUnavailable`, and `MarkerClearAfterCommit { source, committed }` + produced on exactly the same conditions as before. + - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::coordinator::` — the pre-existing fence, lock-contention, and `MarkerClearAfterCommit` tests pass unmodified in assertion content. +- [ ] AC2: Abandoning an `Active` scope changes only that scope's status to + `Abandoned`, advances its worktree `revision` by exactly one, sets + `needs_rebaseline = true`, leaves `cursor_tree` / `tainted` / `failure_kind` + unchanged, and writes no `mutation_trace_events`, + `mutation_trace_event_active_scopes`, or `mutation_trace_processed_events` row. + - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::scope_runtime::` — a test reads the durable rows back after a successful abandonment and asserts each field and each absent row. +- [ ] AC3: `abandon_scope()` performs no Git snapshot, tree pin, tree diff, ref + reconciliation, scope registration, or worktree initialization; it reads and + transitions only already-durable mutation-scope state. + - Validate: inspect `cli/src/services/mutation_trace/runtime/scope_runtime.rs` — it names none of `GitSnapshotService`, `SnapshotCapture`, `capture_tree`, `pin_tree`, `diff_trees`, `reconcile_worktree`, `initialize_worktree`, or `register_scope`; confirm with `rg -n 'GitSnapshotService|SnapshotCapture|capture_tree|pin_tree|diff_trees|reconcile_worktree|initialize_worktree|register_scope' cli/src/services/mutation_trace/runtime/scope_runtime.rs` returning no non-test hit. +- [ ] AC4: A target scope already `Closed` or `Abandoned` settles as a successful + terminal no-op: no revision change, no row write, and the external-taint marker + is cleared. + - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::scope_runtime::` — separate `Closed` and `Abandoned` tests assert the outcome variant, the unchanged durable revision, and that the marker file is gone. +- [ ] AC5: A target `ScopeId` with no durable row, and one whose row is + `NeverSeen`, both return the recovery-required outcome, leave the + external-taint marker armed on disk, and commit no normal abandonment — no + durable row changes, and the worktree revision does not advance. This + deliberately forces the next `coordinate()` into conservative strong recovery + (see **Design decisions**, D1). + - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::scope_runtime::` — separate missing-scope and `NeverSeen` tests assert the reason variant, that `/sce/mutation-cursor-tainted` still exists, and that the worktree revision and every scope status are unchanged. +- [ ] AC6: An external-taint marker already present when `abandon_scope()` is + called returns the recovery-required outcome for that reason without clearing + the marker and without invoking the caller-supplied DB provider. + - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::scope_runtime::` — a test arms the marker first, passes a provider that sets a flag and returns `Err`, and asserts the recovery-required reason, the still-present marker, and that the provider flag was never set. +- [ ] AC7: A target scope whose durable `worktree_id` is not the `WorktreeId` this + invocation derived from its own checkout is rejected as an error, and neither + the scope row nor either worktree row is modified. + - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::` — a two-linked-worktree test abandons worktree A's scope through worktree B's checkout, asserts the error, and asserts both worktree revisions and the scope status are unchanged. +- [ ] AC8: A CAS conflict makes `abandon_scope()` reload the durable projection and + recompute the abandonment from that fresh state, bounded by the same retry limit + the coordinator uses; when the competing writer left the scope terminal, the + retry settles as the terminal no-op outcome rather than overwriting it. + - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::` — a real-thread CAS-race test against one on-disk DB asserts the settled outcome and that the scope's final status is the competitor's, not a second abandonment. +- [ ] AC9: An `Active` target on a worktree at `revision: u64::MAX` produces an + explicit revision-exhaustion error, never an abandonment success and never a + terminal no-op. + - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::scope_runtime::` — a test seeds `u64::MAX` and asserts the distinct error variant. +- [ ] AC10: With no inherited marker, a DB-provider `Err` or a persistence failure + after the marker is armed leaves the marker on disk; a successful abandonment + and a proven-terminal no-op each clear it; a `clear()` failure after either + completes returns an error that carries the already-completed outcome rather + than reporting the durable transition as failed. + - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::` — one test per row of that table, the clear-failure test asserting the carried outcome matches the durable state. +- [ ] AC11: Against a real Git repository and a real repository-scoped Agent Trace + DB: `Start(A)` → edit → `abandon_scope(A)` → a further unobserved edit → + `coordinate(Start(B))` leaves the worktree cursor at the tree observed at + `Start(B)`, emits no `MutationEvent` for the ambiguous A→B interval, leaves A + `Abandoned`, and leaves B `Active`. + - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::tests::` — the end-to-end sequence test asserts the cursor tree, the absence of any `mutation_trace_events` row for the gap, and both scope statuses. +- [ ] AC12: Abandoning stale scope A while unrelated scope B is legitimately + `Active` on the same worktree leaves B `Active` through the subsequent + `needs_rebaseline` recovery. + - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::tests::` — the surviving-scope test asserts B's status after the next `coordinate()` call. +- [ ] AC13: `runtime/mod.rs` re-exports `coordinate`, `CoordinateError`, + `CoordinateOutcome`, `RuntimeBoundary`, `abandon_scope`, `AbandonScopeError`, + and `AbandonScopeOutcome` (with its reason type) at `pub(crate)`, and exports + nothing from `git_snapshot`, `external_taint`, `worktree_lock`, + `ref_reconciliation`, or the protected-worktree primitive. + - Validate: inspect `cli/src/services/mutation_trace/runtime/mod.rs` for exactly those re-exports, and `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` stays clean. +- [ ] AC14: `spec/mutation_cursor.qnt`, the Quint refinement matrix in + `cli/src/services/mutation_trace/mod.rs`, `protocol.rs`'s transition semantics, + `004_mutation_trace_protocol.sql`, and the migration set carry no change + attributable to this PR. The baseline is this PR's own base branch, `ref-rec`, + not `main`: the mutation-cursor protocol, the Quint model, the migration, and + the runtime coordinator already exist on `ref-rec`, so a `main` baseline would + report the entire stack below this PR as if it were this PR's change. + - Validate: `git fetch origin` first, then `git diff --stat origin/ref-rec...HEAD -- spec/mutation_cursor.qnt cli/src/services/mutation_trace/protocol.rs cli/migrations/agent-trace-repository/` produces empty output. Use `origin/ref-rec`, not a local `ref-rec`, which can lag behind a rewritten base and would report the whole rebased stack as this PR's change. Additionally, the `mutation-trace-quint-connect` and Quint checks inside `nix flake check` stay green. +- [ ] AC15: `context/cli/mutation-scope-runtime.md` exists and states, in the + repository's own terms: a mutation scope is one independently mutation-capable + execution (so concurrent main agent and subagent need distinct `ScopeId`s); + `Start`/`Advance`/`Close` semantics including that a failed tool still requires + `Advance` and that a `ScopeId` is never reused after a terminal status; + `abandon_scope()` requires positive staleness evidence and must not be inferred + from `ActorKind`; the `abandon` → `coordinate(Start(successor))` sequence and + what each abandonment outcome implies for it; that abandonment is not a + `RuntimeBoundary` and needs no Quint change; that a missing or `NeverSeen` + target deliberately forces conservative strong recovery which may invalidate + other live scopes on the worktree, with the reason that outranks the lost + evidence (**Design decisions**, D1); and that `AiExclusive(scope)` means scope + exclusivity, not standalone proof that no human edited the worktree. + - Validate: read `context/cli/mutation-scope-runtime.md` and confirm each of those statements is present and consistent with the shipped code. + +### Full validation + +- `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::` +- `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml` +- `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` +- `./scripts/run-cli-cargo.sh fmt --manifest-path cli/Cargo.toml -- --check` +- `nix flake check` +- `nix run .#pkl-check-generated` + +### Context sync + +- `context/cli/mutation-scope-runtime.md` — new: the mutation-scope lifecycle and + harness-adapter contract, including the attribution boundary. +- `context/cli/mutation-trace-runtime-coordinator.md` — the protected-worktree + primitive, the two-entrypoint runtime surface, and the `pub(crate)` export seam. +- `context/cli/mutation-trace-external-taint.md` — the fence's abandonment-path + completion semantics (clear on abandoned/terminal, stay armed on + recovery-required, marker-clear-after-completion). +- `context/cli/mutation-trace-protocol.md` — that `protocol::abandon` now has a + production call site, and that abandonment is not a `RuntimeBoundary`. +- `context/cli/mutation-trace-store.md` — the new bounded scope read seam. +- `context/context-map.md` and `context/overview.md` — index and status lines for + the new module and context file. +- `context/patterns.md` — repair the recorded unit-testing pattern, which still + describes unique `std::env::temp_dir()` paths as the mutation-trace fixture + convention while `runtime/tests.rs` has moved to RAII `tempfile::TempDir`. + +## Task context synchronization lifecycle + +Persist this field in every plan; this is durable plan state, not chat state: + +- **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/mutation_trace/runtime/` (new + `protected_worktree.rs` and `scope_runtime.rs`, edits to `coordinator.rs`, + `mod.rs`, `tests.rs`), the read seam in + `cli/src/services/mutation_trace/store.rs`, and the durable context files listed + under **Context sync**. +- **Out of scope:** Codex, Claude Code, OpenCode, and Pi hook/plugin/extension + wiring; each harness's concrete `ScopeId` / `EventId` format; harness-specific + stale-process detection; adding `Abandon` to `RuntimeBoundary`; changes to + `spec/mutation_cursor.qnt`, `protocol::abandon`, the mutation-trace SQL schema, + or migrations; a new mutation-cursor table; `diff_traces` redesign; + mutation-history retention; repository-scoped unowned checkout-identity ref + cleanup; human-vs-AI authorship proof; a daemon or background liveness monitor; + a `mutation_scope_adapter.qnt` model. +- **Constraints:** + - The protected prefix ordering is safety-critical and must not move: worktree + lock **before** external-marker inspect/persist **before** checkout identity, + DB acquisition, and any runtime work. The guard never clears the marker in + `Drop`; only an explicit successful completion clears it. + - `abandon_scope()` must classify the target's durable state *before* invoking + `protocol::abandon`. `abandon` is a guarded no-op for `NeverSeen`, `Closed`, + `Abandoned`, an unknown scope, a missing `WorktreeState`, and + `revision == u64::MAX` alike, and returns an unchanged state in every case — + the runtime cannot recover the reason by diffing its output. + - The bounded retry limit is the coordinator's existing + `MAX_CAS_RETRY_ATTEMPTS` (5, no backoff); reuse it rather than introducing a + second constant. + - `#[allow(dead_code)]` on `pub mod mutation_trace` in + `cli/src/services/mod.rs` covers unused items, not necessarily unused + `pub(crate) use` re-exports. `clippy --all-targets -- -D warnings` is the + gate; keep it green using the module's existing allowance precedent rather + than by adding a placeholder consumer. + - Filesystem-touching tests follow the repository's Nix-sandbox-safe inline + `#[cfg(test)] mod tests` convention with RAII `tempfile::TempDir` fixtures, as + `runtime/tests.rs` already does. +- **Non-goal:** a general-purpose "runtime operation" abstraction over + `coordinate()` and `abandon_scope()`. The two paths deliberately differ (one + observes, one does not); the shared piece is the protected prefix only. + +## Design decisions + +Decided before implementation. Do not reopen these during T01–T05; a change of +mind is a new plan revision, not a task-time judgement call. + +### D1: A missing or `NeverSeen` target forces conservative strong recovery, and that may invalidate other live scopes + +`abandon_scope()` on a `ScopeId` with no durable row, or one whose row is still +`NeverSeen`, returns the recovery-required outcome and leaves the external-taint +marker armed. The next `coordinate()` on that worktree therefore performs +*inherited-taint* recovery, and `protocol::recover` abandons **every** live scope +on a worktree recovering from external taint — not only the scope the adapter +named. + +That consequence is accepted deliberately, not overlooked. The reasoning: + +```text +execution lifecycle not durably observed + ↓ +filesystem interval may contain unknown mutations + ↓ +cannot safely preserve exclusive attribution assumptions + ↓ +force conservative recovery +``` + +A missing row means the scope's `Start` never committed while the execution may +well have run and edited files; a `NeverSeen` row means the identity exists but +no accepted `Start` was ever observed for it. Neither proves the execution +mutated nothing. The runtime cannot bound what happened inside that interval, so +it cannot let any scope keep an exclusivity claim that spans it. + +**The tradeoff, stated plainly:** this is a false-negative cost. Legitimately +live mutation scopes on the same worktree can be abandoned by a recovery they did +nothing to cause, and the evidence for their in-flight intervals is discarded. +That cost is acceptable because the alternative is a false positive — attributing +an interval exclusively to a scope while an unobserved execution may have been +mutating the same worktree. Preserving attribution safety outranks preserving +potentially valid evidence. + +This is *not* in tension with AC12. AC12 covers the `Abandoned` outcome, whose +`needs_rebaseline`-only recovery preserves live scopes by design. D1 covers the +recovery-required outcomes, where the stronger external-taint recovery is the +whole point. + +### D2: The DB is never consulted before the external-taint fence is armed + +The protected ordering stays exactly as `coordinate()` already has it: + +```text +WorktreeLock + ↓ +inspect / persist external-taint marker + ↓ +checkout identity + ↓ +DB + ↓ +runtime operation +``` + +The alternative of resolving the target scope first, so an unresolvable +`ScopeId` could be rejected without arming the fence — + +```text +lock + → DB lookup + → maybe arm marker +``` + +— is **rejected**. Any failure between reading the DB and establishing the fence +(process death, `SIGKILL`, an I/O error, a panic) reopens exactly the uncertainty +window the external-taint marker exists to close: the invocation would have +touched durable state, or decided something about it, with no worktree-local +signal left behind for the next invocation. The fence must be armed write-ahead +of every fallible step that follows it, including the scope lookup that decides +D1's outcome. + +The one lookup-order concession already in the plan is the inherited-marker +short-circuit: when a marker was *already* present on entry, `abandon_scope()` +returns recovery-required without invoking the DB provider at all (AC6). That +does not weaken the fence — the fence is already armed by an earlier invocation, +which is precisely why there is nothing left for this one to decide. + +## Assumptions + +- Module and type names follow the change request's suggestions + (`runtime/protected_worktree.rs`, `runtime/scope_runtime.rs`, + `ProtectedWorktree::acquire`, `abandon_scope`, `AbandonScopeOutcome`, + `AbandonScopeError`); adjust to whatever reads best beside the existing + `coordinator.rs` naming, since only the semantic distinctions are contractual. +- `abandon_scope()` takes the same caller-supplied + `open_db: impl FnOnce() -> anyhow::Result` provider + shape as `coordinate()`, so DB acquisition falls inside the same fence. +- The recovery-required reasons are modelled as a distinct enum + (`InheritedExternalTaint`, `MissingScope`, `NeverSeenScope`, + `MissingWorktreeState`) carried in the outcome, so callers and tests can match + on them. +- `abandon_scope()` reuses `coordinator.rs`'s `WORKTREE_LOCK_TIMEOUT` rather than + declaring its own, matching the reconciliation pass's precedent of an + operation-appropriate timeout only where the operation genuinely differs. + +## Task stack + +- [ ] T01: `Extract the protected-worktree runtime guard` (status:todo) + - Task ID: T01 + - Scope: In — new `cli/src/services/mutation_trace/runtime/protected_worktree.rs` owning git-dir resolution, `WorktreeLock` acquisition, external-marker inspect + persist, checkout identity, `WorktreeId` derivation, and an explicit `complete`/`clear` step; refactor `coordinator.rs`'s `coordinate_inner` / `coordinate_protected` prefix onto it; keep the `on_lock_contention` test seam working. Out — any new entrypoint, any store change, any behavior change to the pipeline below the prefix, any `runtime/mod.rs` export change. + - Dependencies: none + - Done when: the guard exposes the derived `WorktreeId`, whether a marker was already present before this invocation, and an explicit completion that clears the marker; it never clears the marker in `Drop`; it holds the `WorktreeLock` for its own lifetime; `coordinate()` produces identical outcomes and identical `CoordinateError` variants on every existing path, with the existing coordinator fence/lock tests passing without assertion changes; focused tests cover the guard itself for lock-timeout failure, an inherited marker being reported, a fresh marker being armed, and the marker surviving a dropped guard that was never completed. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::`; `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings`; `git diff` on `coordinator.rs` shows no reordering of lock → marker → checkout → DB. + - Context synchronization: pending + +- [ ] T02: `Expose a bounded scope read on MutationTraceStore` (status:todo) + - Task ID: T02 + - Scope: In — promote the existing private `MutationTraceStore::load_scope` to the smallest public read seam `scope_runtime` needs (one `mutation_trace_scopes` row → `Option`), with a doc comment stating it is a cold-path read that never widens into a projection; tests for an existing scope, a missing scope, and a scope belonging to another worktree. Out — any change to `load_worktree`'s hook-boundary semantics or error contract, any schema/migration change, any new query, any write path. + - Dependencies: none + - Done when: the read returns the durable `ScopeState` (status, `actor_kind`, `worktree_id`) for a known `ScopeId` and `None` for an unknown one, without consulting `mutation_trace_events` or the worktree row; `load_worktree`'s existing behavior, including its `Err` on a mismatched or missing effective referenced scope, is untouched; the mismatched-worktree case is proven to be the caller's decision, not a store-level rejection. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::store::`; `git diff cli/migrations/` is empty. + - Context synchronization: pending + +- [ ] T03: `Implement abandon_scope() on the protected runtime path` (status:todo) + - Task ID: T03 + - Scope: In — new `cli/src/services/mutation_trace/runtime/scope_runtime.rs` with `abandon_scope`, `AbandonScopeOutcome`, its recovery-reason enum, and `AbandonScopeError`, built on T01's guard and T02's read; inline `#[cfg(test)] mod tests` against a real temp-file `RepositoryAgentTraceDb` covering active abandonment, `Closed`/`Abandoned` terminal no-op, missing and `NeverSeen` recovery-required, inherited-marker short-circuit before the DB provider, worktree-identity rejection, revision exhaustion, CAS reload/recompute/retry, DB-provider failure, and marker-clear-after-completion. Out — any Git snapshot, pin, diff, reconciliation, scope registration, or worktree initialization; any `runtime/mod.rs` export; any real-Git cross-worktree test (T04). + - Dependencies: T01, T02 + - Done when: an inherited marker returns recovery-required for that reason without invoking `open_db` and without clearing the marker; a missing or `NeverSeen` scope returns recovery-required, writes nothing, and leaves the marker armed, per **Design decisions** D1 — the scope lookup happens after the fence is armed, never before it (D2); a `Closed`/`Abandoned` scope returns the terminal no-op with the current revision and clears the marker; an `Active` scope belonging to another `WorktreeId` is an error that writes nothing; an `Active` scope on a worktree at `u64::MAX` returns a distinct revision-exhaustion error; otherwise the durable transition sets the scope `Abandoned`, advances revision by exactly one, sets `needs_rebaseline`, writes no event or processed-event row, and clears the marker; a `CasResult::Conflict` reloads and recomputes from fresh state within `MAX_CAS_RETRY_ATTEMPTS`, settling as the terminal no-op when a competitor won; a failing `clear()` after a completed abandonment or terminal no-op returns an error carrying that completed outcome; the module names no Git-snapshot or reconciliation symbol. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::scope_runtime::`; `rg -n 'GitSnapshotService|SnapshotCapture|capture_tree|pin_tree|diff_trees|reconcile_worktree|initialize_worktree|register_scope' cli/src/services/mutation_trace/runtime/scope_runtime.rs` returns nothing; `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings`. + - Context synchronization: pending + +- [ ] T04: `Add cross-runtime abandonment safety regressions` (status:todo) + - Task ID: T04 + - Scope: In — integration tests in `cli/src/services/mutation_trace/runtime/tests.rs` driving `coordinate()` and `abandon_scope()` together against real `git init` / `git worktree add` repositories and a real repository-scoped Agent Trace DB: the abandon → unobserved edit → successor `Start` rebaseline sequence with no evidence for the gap; a concurrently active unrelated scope surviving that recovery; abandoning a scope through the wrong checkout; and a real-thread CAS race between `abandon_scope()` and a competing writer. Out — the single-module cases T03 already covers with a temp-file DB and no real Git. + - Dependencies: T03 + - Done when: `Start(A)` → edit → `abandon_scope(A)` → unobserved edit → `coordinate(Start(B))` leaves the cursor at the tree observed at `Start(B)`, emits no `mutation_trace_events` row for the A→B interval, leaves A `Abandoned` and B `Active`; a second scope B active across `abandon_scope(A)` is still `Active` after the next `coordinate()`; abandoning worktree A's scope through worktree B's checkout errors and changes no row in either worktree; the CAS-race test settles deterministically on the competitor's terminal status without a second abandonment. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::tests::`; `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::`. + - Context synchronization: pending + +- [ ] T05: `Export the runtime seam and record the adapter contract` (status:todo) + - Task ID: T05 + - Scope: In — `pub(crate) use` re-exports in `runtime/mod.rs` for `coordinate`, `CoordinateError`, `CoordinateOutcome`, `RuntimeBoundary`, `abandon_scope`, `AbandonScopeError`, `AbandonScopeOutcome` and its reason type; keeping `git_snapshot`, `external_taint`, `worktree_lock`, `ref_reconciliation`, and `protected_worktree` unexported; new `context/cli/mutation-scope-runtime.md` plus its `context/context-map.md` and `context/overview.md` index entries. Out — any harness, hook, or command wiring; any change to the runtime implementation; the per-task context updates T01–T04 each own for their own domain files. + - Dependencies: T04 + - Done when: the seven names above are reachable as `crate::services::mutation_trace::runtime::*` and nothing from the five private modules is; `clippy --all-targets -- -D warnings` is clean with no placeholder consumer added to satisfy it; `context/cli/mutation-scope-runtime.md` states the scope-identity rule, the `Start`/`Advance`/`Close` semantics, the positive-evidence requirement for `abandon_scope()` and the prohibition on inferring staleness from `ActorKind`, the successor-scope sequence and what each outcome implies for it (including that a failed abandonment must not be treated as a safely started successor), that abandonment is not a `RuntimeBoundary` and requires no Quint change, the D1 strong-recovery tradeoff for a missing or `NeverSeen` target, and the `AiExclusive` attribution boundary; `context-map.md` and `overview.md` name the new module and file. + - Verify: `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings`; `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml`; read `context/cli/mutation-scope-runtime.md` against the shipped `scope_runtime.rs` signatures. + - Context synchronization: pending + +## Open questions + +- T04's CAS-race regression is the one test in this plan that needs real OS + threads against one on-disk DB. `coordinator.rs` already has that machinery for + its own CAS tests. If reusing it means exporting a test helper across modules, + the cheaper option is to leave the abandonment CAS race in `scope_runtime.rs`'s + own inline tests (T03) and drop it from T04, since nothing about the race needs + real Git. Not blocking — T03's `Done when` already covers the behavior. From 8f2bd72f48569c97029bd14f8a2e70e9230e8777 Mon Sep 17 00:00:00 2001 From: David Abram Date: Thu, 3 Sep 2026 11:55:36 +0200 Subject: [PATCH 2/6] runtime: Extract the protected-worktree safety prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mutation-trace runtime now centralizes git-dir resolution, worktree locking, external-taint fencing, checkout identity derivation, and successful marker completion in `ProtectedWorktree`. Refactor `coordinate()` to use the guard while preserving existing error and lock-ordering semantics, and document the shared prefix and its test boundary. Plan: `mutation-scope-runtime-integration` — T01 --- .../mutation_trace/runtime/coordinator.rs | 82 ++--- .../services/mutation_trace/runtime/mod.rs | 1 + .../runtime/protected_worktree.rs | 314 ++++++++++++++++++ .../runtime/ref_reconciliation.rs | 5 - context/cli/mutation-trace-external-taint.md | 32 +- .../cli/mutation-trace-protected-worktree.md | 93 ++++++ .../cli/mutation-trace-ref-reconciliation.md | 6 +- .../cli/mutation-trace-runtime-coordinator.md | 74 ++--- context/context-map.md | 3 +- context/patterns.md | 2 +- .../mutation-scope-runtime-integration.md | 78 ++++- 11 files changed, 561 insertions(+), 129 deletions(-) create mode 100644 cli/src/services/mutation_trace/runtime/protected_worktree.rs create mode 100644 context/cli/mutation-trace-protected-worktree.md diff --git a/cli/src/services/mutation_trace/runtime/coordinator.rs b/cli/src/services/mutation_trace/runtime/coordinator.rs index 4df950cc..79933c7c 100644 --- a/cli/src/services/mutation_trace/runtime/coordinator.rs +++ b/cli/src/services/mutation_trace/runtime/coordinator.rs @@ -1,24 +1,21 @@ use std::path::Path; -use std::time::Duration; use anyhow::Result; use uuid::Uuid; use crate::services::agent_trace_db::repository::RepositoryAgentTraceDb; -use crate::services::checkout::{get_or_create_checkout_id, resolve_git_dir}; use crate::services::mutation_trace::protocol; use crate::services::mutation_trace::store::{CasResult, DurableTransition, MutationTraceStore}; use crate::services::mutation_trace::types::{ self, ActorKind, AttemptId, Boundary, EventId, MutationEvent, ScopeId, TreeId, WorktreeId, }; -use super::external_taint::ExternalTaintMarker; use super::git_snapshot::GitSnapshotService; -use super::worktree_lock::{acquire_inner, WorktreeLockError}; +use super::protected_worktree::{ProtectedWorktree, ProtectedWorktreeError}; -const MAX_CAS_RETRY_ATTEMPTS: u32 = 5; +pub use super::protected_worktree::ExternalTaintOperation; -const WORKTREE_LOCK_TIMEOUT: Duration = Duration::from_secs(10); +const MAX_CAS_RETRY_ATTEMPTS: u32 = 5; #[derive(Clone, Debug)] pub enum RuntimeBoundary { @@ -49,18 +46,6 @@ pub struct CoordinateOutcome { pub mutation_event: Option, } -/// Which pre-commit [`ExternalTaintMarker`] operation failed while coordinating a -/// boundary. Both happen **before** any protected work, so no -/// [`CoordinateOutcome`] exists yet. A marker-clear failure happens *after* a -/// durable commit and is reported through -/// [`CoordinateError::MarkerClearAfterCommit`] instead, which carries the -/// committed outcome. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum ExternalTaintOperation { - Inspect, - Persist, -} - #[derive(Debug)] pub enum CoordinateError { SnapshotFailure { @@ -161,16 +146,6 @@ impl SnapshotCapture for GitSnapshotService { } } -/// Coordinates one mutation-cursor runtime boundary end to end. -/// -/// The entrypoint owns the whole protected operation: it resolves `git_dir`, -/// acquires the [`WorktreeLock`](super::worktree_lock::WorktreeLock), arms the -/// worktree-local [`ExternalTaintMarker`] write-ahead — **before** acquiring the -/// Agent Trace DB — and only then invokes the caller-supplied `open_db` -/// provider, captures a snapshot, runs the snapshot / recovery / protocol / CAS -/// pipeline, and clears the marker on complete success. Any failure after the -/// marker is armed — including `open_db` returning `Err` — leaves the marker in -/// place for the next invocation. pub fn coordinate

( repository_root: &Path, boundary: &RuntimeBoundary, @@ -203,37 +178,20 @@ where L: FnMut(u32), R: FnMut(u32) -> Result<()>, { - let git_dir = resolve_git_dir(repository_root).map_err(CoordinateError::Other)?; - - let _lock = acquire_inner(&git_dir, WORKTREE_LOCK_TIMEOUT, on_lock_contention) - .map_err(lock_acquisition)?; - - let marker = ExternalTaintMarker::new(&git_dir); - let inherited_external_taint = - marker - .exists() - .map_err(|source| CoordinateError::ExternalTaintMarker { - operation: ExternalTaintOperation::Inspect, - source, - })?; - marker - .persist() - .map_err(|source| CoordinateError::ExternalTaintMarker { - operation: ExternalTaintOperation::Persist, - source, - })?; + let protected = ProtectedWorktree::acquire_inner(repository_root, on_lock_contention) + .map_err(protected_worktree_failure)?; let outcome = coordinate_protected( repository_root, - &git_dir, + protected.worktree_id(), boundary, open_db, - inherited_external_taint, + protected.inherited_external_taint(), after_load, after_recovery, )?; - match marker.clear() { + match protected.complete() { Ok(()) => Ok(outcome), Err(source) => Err(CoordinateError::MarkerClearAfterCommit { source, @@ -244,7 +202,7 @@ where fn coordinate_protected( repository_root: &Path, - git_dir: &Path, + worktree_id: &WorktreeId, boundary: &RuntimeBoundary, open_db: P, inherited_external_taint: bool, @@ -256,9 +214,6 @@ where L: FnMut(u32), R: FnMut(u32) -> Result<()>, { - let checkout_id = get_or_create_checkout_id(git_dir).map_err(CoordinateError::Other)?; - let worktree_id = WorktreeId(checkout_id); - let db = open_db().map_err(CoordinateError::AgentTraceDbUnavailable)?; let snapshot = GitSnapshotService::new(repository_root).map_err(CoordinateError::Other)?; @@ -266,7 +221,7 @@ where coordinate_boundary_inner( &db, &snapshot, - &worktree_id, + worktree_id, boundary, inherited_external_taint, after_load, @@ -274,8 +229,17 @@ where ) } -fn lock_acquisition(error: WorktreeLockError) -> CoordinateError { - CoordinateError::LockAcquisition(anyhow::Error::new(error)) +fn protected_worktree_failure(error: ProtectedWorktreeError) -> CoordinateError { + match error { + ProtectedWorktreeError::GitDirResolution(source) + | ProtectedWorktreeError::CheckoutIdentity(source) => CoordinateError::Other(source), + ProtectedWorktreeError::LockAcquisition(source) => { + CoordinateError::LockAcquisition(anyhow::Error::new(source)) + } + ProtectedWorktreeError::ExternalTaintMarker { operation, source } => { + CoordinateError::ExternalTaintMarker { operation, source } + } + } } #[cfg(test)] @@ -532,8 +496,12 @@ mod tests { use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::mpsc; use std::thread; + use std::time::Duration; use super::*; + use crate::services::checkout::{get_or_create_checkout_id, resolve_git_dir}; + use crate::services::mutation_trace::runtime::external_taint::ExternalTaintMarker; + use crate::services::mutation_trace::runtime::worktree_lock::acquire_inner; use crate::services::mutation_trace::store::encode_revision; use crate::services::mutation_trace::types::{Attribution, EventKey, FailureKind, ScopeStatus}; diff --git a/cli/src/services/mutation_trace/runtime/mod.rs b/cli/src/services/mutation_trace/runtime/mod.rs index 2d88705c..2edbf4f5 100644 --- a/cli/src/services/mutation_trace/runtime/mod.rs +++ b/cli/src/services/mutation_trace/runtime/mod.rs @@ -1,6 +1,7 @@ mod coordinator; mod external_taint; mod git_snapshot; +mod protected_worktree; mod ref_reconciliation; mod worktree_lock; diff --git a/cli/src/services/mutation_trace/runtime/protected_worktree.rs b/cli/src/services/mutation_trace/runtime/protected_worktree.rs new file mode 100644 index 00000000..2d462fc3 --- /dev/null +++ b/cli/src/services/mutation_trace/runtime/protected_worktree.rs @@ -0,0 +1,314 @@ +use std::path::Path; +use std::time::Duration; + +use crate::services::checkout::{get_or_create_checkout_id, resolve_git_dir}; +use crate::services::mutation_trace::types::WorktreeId; + +use super::external_taint::ExternalTaintMarker; +use super::worktree_lock::{acquire_inner, WorktreeLock, WorktreeLockError}; + +pub const WORKTREE_LOCK_TIMEOUT: Duration = Duration::from_secs(10); + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ExternalTaintOperation { + Inspect, + Persist, +} + +#[derive(Debug)] +pub enum ProtectedWorktreeError { + GitDirResolution(anyhow::Error), + LockAcquisition(WorktreeLockError), + ExternalTaintMarker { + operation: ExternalTaintOperation, + source: anyhow::Error, + }, + CheckoutIdentity(anyhow::Error), +} + +impl std::fmt::Display for ProtectedWorktreeError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ProtectedWorktreeError::ExternalTaintMarker { operation, source } => write!( + f, + "External-taint marker {operation:?} operation failed before any \ + protected runtime work began: {source}" + ), + ProtectedWorktreeError::LockAcquisition(source) => write!(f, "{source}"), + ProtectedWorktreeError::GitDirResolution(source) + | ProtectedWorktreeError::CheckoutIdentity(source) => write!(f, "{source}"), + } + } +} + +impl std::error::Error for ProtectedWorktreeError {} + +#[derive(Debug)] +pub struct ProtectedWorktree { + marker: ExternalTaintMarker, + inherited_external_taint: bool, + worktree_id: WorktreeId, + _lock: WorktreeLock, +} + +impl ProtectedWorktree { + pub fn acquire(repository_root: &Path) -> Result { + Self::acquire_inner(repository_root, || {}) + } + + pub(super) fn acquire_inner( + repository_root: &Path, + on_lock_contention: F, + ) -> Result + where + F: FnOnce(), + { + Self::acquire_with_timeout(repository_root, WORKTREE_LOCK_TIMEOUT, on_lock_contention) + } + + fn acquire_with_timeout( + repository_root: &Path, + lock_timeout: Duration, + on_lock_contention: F, + ) -> Result + where + F: FnOnce(), + { + let git_dir = + resolve_git_dir(repository_root).map_err(ProtectedWorktreeError::GitDirResolution)?; + + let lock = acquire_inner(&git_dir, lock_timeout, on_lock_contention) + .map_err(ProtectedWorktreeError::LockAcquisition)?; + + let marker = ExternalTaintMarker::new(&git_dir); + let inherited_external_taint = + marker + .exists() + .map_err(|source| ProtectedWorktreeError::ExternalTaintMarker { + operation: ExternalTaintOperation::Inspect, + source, + })?; + marker + .persist() + .map_err(|source| ProtectedWorktreeError::ExternalTaintMarker { + operation: ExternalTaintOperation::Persist, + source, + })?; + + let checkout_id = get_or_create_checkout_id(&git_dir) + .map_err(ProtectedWorktreeError::CheckoutIdentity)?; + + Ok(Self { + marker, + inherited_external_taint, + worktree_id: WorktreeId(checkout_id), + _lock: lock, + }) + } + + #[must_use] + pub fn worktree_id(&self) -> &WorktreeId { + &self.worktree_id + } + + #[must_use] + pub fn inherited_external_taint(&self) -> bool { + self.inherited_external_taint + } + + pub fn complete(self) -> anyhow::Result<()> { + self.marker.clear() + } +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + use std::process::Command; + use std::sync::mpsc; + use std::thread; + + use super::*; + + struct TestRepo { + _temp_dir: tempfile::TempDir, + repo_root: PathBuf, + git_dir: PathBuf, + } + + impl TestRepo { + fn new(label: &str) -> Self { + let temp_dir = tempfile::Builder::new() + .prefix(&format!("sce-protected-worktree-{label}-")) + .tempdir() + .expect("test temp directory should be created"); + let repo_root = temp_dir.path().join("repo"); + std::fs::create_dir_all(&repo_root).expect("repository directory should be created"); + run_git(&repo_root, &["init", "--quiet"]); + let git_dir = resolve_git_dir(&repo_root).expect("git dir should resolve"); + Self { + _temp_dir: temp_dir, + repo_root, + git_dir, + } + } + + fn marker(&self) -> ExternalTaintMarker { + ExternalTaintMarker::new(&self.git_dir) + } + + fn marker_exists(&self) -> bool { + self.marker() + .exists() + .expect("marker existence should resolve") + } + } + + fn run_git(dir: &std::path::Path, args: &[&str]) { + let output = Command::new("git") + .args(args) + .current_dir(dir) + .output() + .expect("git command should spawn"); + assert!( + output.status.success(), + "git {args:?} failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + + #[test] + fn acquire_arms_a_fresh_marker_and_reports_no_inherited_taint() { + let repo = TestRepo::new("fresh-marker"); + + let guard = ProtectedWorktree::acquire(&repo.repo_root) + .expect("the prefix should be establishable on a clean worktree"); + + assert!( + !guard.inherited_external_taint(), + "a worktree with no marker on entry must not report inherited taint" + ); + assert!( + !guard.worktree_id().0.is_empty(), + "the guard must expose the derived WorktreeId" + ); + assert!( + repo.marker_exists(), + "acquire must arm the external-taint marker write-ahead" + ); + + guard + .complete() + .expect("completion should clear the marker"); + assert!( + !repo.marker_exists(), + "an explicit completion must clear the marker it armed" + ); + } + + #[test] + fn acquire_reports_a_marker_inherited_from_an_earlier_invocation() { + let repo = TestRepo::new("inherited-marker"); + repo.marker() + .persist() + .expect("the earlier invocation's marker should arm"); + + let guard = ProtectedWorktree::acquire(&repo.repo_root) + .expect("an inherited marker must not fail the prefix"); + + assert!( + guard.inherited_external_taint(), + "a marker present on entry must be reported as inherited taint" + ); + assert!( + repo.marker_exists(), + "the inherited marker must stay armed for the protected operation" + ); + } + + #[test] + fn a_dropped_guard_releases_the_lock_but_leaves_the_marker_armed() { + let repo = TestRepo::new("dropped-guard"); + + let guard = ProtectedWorktree::acquire(&repo.repo_root) + .expect("the prefix should be establishable on a clean worktree"); + drop(guard); + + assert!( + repo.marker_exists(), + "dropping a guard that never completed must leave the fence armed" + ); + + ProtectedWorktree::acquire(&repo.repo_root) + .expect("a dropped guard must have released the worktree lock"); + } + + #[test] + fn acquire_fails_with_lock_acquisition_while_the_lock_is_still_held() { + let repo = TestRepo::new("lock-timeout"); + + let held = acquire_inner(&repo.git_dir, Duration::from_secs(5), || {}) + .expect("the test should hold the worktree lock before the guard runs"); + + let error = ProtectedWorktree::acquire_with_timeout( + &repo.repo_root, + Duration::from_millis(250), + || {}, + ) + .expect_err("the prefix must not be establishable while the lock is held"); + assert!( + matches!( + error, + ProtectedWorktreeError::LockAcquisition(WorktreeLockError::TimedOut { .. }) + ), + "expected a LockAcquisition timeout, got {error:?}" + ); + assert!( + !repo.marker_exists(), + "a prefix that never acquired the lock must not have armed the fence" + ); + + drop(held); + } + + #[test] + fn the_lock_is_held_for_the_whole_guard_lifetime() { + let repo = TestRepo::new("lock-lifetime"); + + let guard = ProtectedWorktree::acquire(&repo.repo_root) + .expect("the prefix should be establishable on a clean worktree"); + + let (contention_tx, contention_rx) = mpsc::channel(); + let (result_tx, result_rx) = mpsc::channel(); + let repo_root = repo.repo_root.clone(); + let worker = thread::spawn(move || { + let result = ProtectedWorktree::acquire_inner(&repo_root, move || { + contention_tx + .send(()) + .expect("contention signal channel should still be open"); + }); + result_tx + .send(()) + .expect("result signal channel should still be open"); + result + }); + + contention_rx + .recv_timeout(Duration::from_secs(5)) + .expect("a second acquirer should observe the held worktree lock"); + assert!( + result_rx.recv_timeout(Duration::from_millis(300)).is_err(), + "a second acquirer must not establish the prefix while the guard is alive" + ); + + drop(guard); + + result_rx + .recv_timeout(Duration::from_secs(5)) + .expect("the second acquirer should proceed once the guard is dropped"); + worker + .join() + .expect("the second acquirer thread should not panic") + .expect("the second acquirer should succeed once the lock is released"); + } +} diff --git a/cli/src/services/mutation_trace/runtime/ref_reconciliation.rs b/cli/src/services/mutation_trace/runtime/ref_reconciliation.rs index fc10094f..5b93462a 100644 --- a/cli/src/services/mutation_trace/runtime/ref_reconciliation.rs +++ b/cli/src/services/mutation_trace/runtime/ref_reconciliation.rs @@ -12,11 +12,6 @@ use crate::services::mutation_trace::types::{TreeId, WorktreeId}; use super::git_snapshot::{GitSnapshotService, PinInventoryError, PinnedRef}; use super::worktree_lock::{acquire_inner, WorktreeLockError}; -/// Bounded wait for the worktree's `WorktreeLock` before a reconciliation pass -/// gives up. Its value intentionally matches the coordinator's private -/// `WORKTREE_LOCK_TIMEOUT` but is deliberately **not** a shared abstraction: -/// there is no semantic reason the two timeouts must always stay identical, so -/// each module owns its own constant. const RECONCILIATION_LOCK_TIMEOUT: Duration = Duration::from_secs(10); /// Outcome counts of one successful reconciliation pass. diff --git a/context/cli/mutation-trace-external-taint.md b/context/cli/mutation-trace-external-taint.md index caef5b1e..a6f6c91c 100644 --- a/context/cli/mutation-trace-external-taint.md +++ b/context/cli/mutation-trace-external-taint.md @@ -40,7 +40,7 @@ sync is best-effort). The marker is never removed via `Drop`; only an explicit `clear()` removes it. It is never authoritative for normal cursor state. Every method — `new`/`exists`/`persist`/`clear` — is now reached by the -`coordinate()` fence (below), so the module carries no `allow(dead_code)`. +`ProtectedWorktree` prefix (below), so the module carries no `allow(dead_code)`. Inline `#[cfg(test)] mod tests` follows the unique-`std::env::temp_dir()`-path precedent (see [`../patterns.md`](../patterns.md)): marker path is worktree @@ -59,18 +59,21 @@ coordinate(repository_root, boundary, open_db) open_db: impl FnOnce() -> anyhow::Result ``` -Order inside the held `WorktreeLock`: +The fence's arming and clearing is owned by the shared `ProtectedWorktree` +prefix ([`mutation-trace-protected-worktree.md`](mutation-trace-protected-worktree.md)), +not by `coordinate()` directly, so every runtime entrypoint built on that prefix +inherits the same fence semantics. Order inside the held `WorktreeLock`: ```text -resolve git_dir → acquire WorktreeLock - → ExternalTaintMarker::new(git_dir) - → inherited_external_taint = marker.exists()? - → marker.persist()? ← fence armed here, write-ahead - → get_or_create_checkout_id → WorktreeId +resolve git_dir → acquire WorktreeLock ┐ + → ExternalTaintMarker::new(git_dir) │ + → inherited_external_taint = marker.exists()? │ ProtectedWorktree::acquire + → marker.persist()? ← fence armed here │ + → get_or_create_checkout_id → WorktreeId ┘ → open_db() ← DB acquired INSIDE the fence → GitSnapshotService::new → coordinate_boundary(&db, .., inherited_external_taint) - → marker.clear()? ← only on a successful outcome + → ProtectedWorktree::complete() ← clears; only on a successful outcome ``` **Safety invariant:** no failure after the marker is armed — including a @@ -80,11 +83,12 @@ Arming *before* `open_db()` is the whole point: if the DB open fails, the marker is already on disk, so a later invocation that opens the DB successfully still sees the inherited signal instead of trusting a lost interval. -The marker is cleared only by `coordinate()`'s success path (`marker.clear()` -after an `Ok` outcome). Every error path — snapshot failure, DB provider `Err`, -checkout-identity failure, DB read/write failure, CAS exhaustion, scope-identity -conflict, unexpected error — returns with the marker present. No `Drop` clears -it. +The marker is cleared only by `coordinate()`'s success path, through +`ProtectedWorktree::complete()` after an `Ok` outcome. Every error path — +snapshot failure, DB provider `Err`, checkout-identity failure, DB read/write +failure, CAS exhaustion, scope-identity conflict, unexpected error — returns +with the marker present, because the guard's `Drop` releases the worktree lock +but never clears the marker. ### `CoordinateError` variants @@ -181,5 +185,7 @@ and a later invocation still recovers off the still-armed marker. ``` See also: [`mutation-trace-runtime-coordinator.md`](mutation-trace-runtime-coordinator.md), +[`mutation-trace-protected-worktree.md`](mutation-trace-protected-worktree.md) +(the shared prefix that owns arming and clearing this fence), [`mutation-trace-protocol.md`](mutation-trace-protocol.md), [`checkout-identity.md`](checkout-identity.md). diff --git a/context/cli/mutation-trace-protected-worktree.md b/context/cli/mutation-trace-protected-worktree.md new file mode 100644 index 00000000..7911cfa5 --- /dev/null +++ b/context/cli/mutation-trace-protected-worktree.md @@ -0,0 +1,93 @@ +# Mutation-cursor protected-worktree prefix (`runtime::protected_worktree`) + +The shared safety prefix every mutation-cursor runtime entrypoint runs behind, +in `cli/src/services/mutation_trace/runtime/protected_worktree.rs`. It was +extracted from `coordinate()`'s own critical section so that a second entrypoint +built on the same guarantees cannot drift from the first — the ordering below is +safety-critical, and one owner is the mechanism that keeps it single-sourced. + +Extracted by the `mutation-scope-runtime-integration` plan +(`context/plans/mutation-scope-runtime-integration.md`) ahead of the +`abandon_scope()` entrypoint that will share it. + +## The fixed order + +```mermaid +flowchart TD + A["resolve git_dir
(checkout::resolve_git_dir)"] --> B["acquire WorktreeLock
(bounded WORKTREE_LOCK_TIMEOUT, 10s)"] + B --> C["ExternalTaintMarker::exists()
→ inherited_external_taint"] + C --> D["ExternalTaintMarker::persist()
fence armed, write-ahead"] + D --> E["get_or_create_checkout_id
→ WorktreeId"] + E --> F["caller's runtime operation
(DB provider, snapshot, protocol, CAS)"] + F --> G["complete() clears the marker
(lock still held)"] +``` + +The fence is armed **write-ahead of every fallible step that follows it**, +including the DB acquisition and any durable-state lookup. A process that dies +anywhere past that point leaves the worktree-local signal behind for the next +invocation to recover from. See +[`mutation-trace-external-taint.md`](mutation-trace-external-taint.md) for the +marker primitive itself and the recovery it triggers. + +## Surface + +`ProtectedWorktree::acquire(repository_root) -> Result` runs the whole prefix. The guard then exposes: + +- `worktree_id() -> &WorktreeId` — the durable identity derived from this + checkout. No caller ever supplies a `WorktreeId`; it is always derived here. +- `inherited_external_taint() -> bool` — whether a marker was already present + on entry, i.e. whether some earlier invocation never proved a trustworthy + durable completion. +- `complete(self) -> anyhow::Result<()>` — clears the marker while the worktree + lock is still held, then releases the lock as the guard is consumed. This is + the **only** thing that clears the marker. + +`WORKTREE_LOCK_TIMEOUT` (10s) is owned by this module. `pub(super) +acquire_inner(repository_root, on_lock_contention)` carries the lock-contention +test seam; a private timeout-overriding constructor serves the guard's own +tests. + +**`Drop` releases only the lock. It never clears the marker** — so a guard +abandoned by any failure, panic, or early return leaves the fence armed, which +is precisely the conservative outcome the fence exists to produce. + +## Error contract + +`ProtectedWorktreeError` carries one variant per prefix step, so a caller can +map it onto its own error surface without losing which safety step failed: + +| Variant | Raised at | Fence state | +| --- | --- | --- | +| `GitDirResolution(anyhow::Error)` | before the lock | untouched | +| `LockAcquisition(WorktreeLockError)` | lock acquire/timeout | untouched | +| `ExternalTaintMarker { operation: Inspect \| Persist, source }` | fence inspect/arm | left as it was | +| `CheckoutIdentity(anyhow::Error)` | after the fence is armed | **armed** | + +`ExternalTaintOperation` lives here, beside the fence step that produces it, and +is re-exported by `coordinator.rs` so `CoordinateError::ExternalTaintMarker` +keeps naming it. `coordinate()` maps `GitDirResolution` and `CheckoutIdentity` +onto `CoordinateError::Other`, `LockAcquisition` onto +`CoordinateError::LockAcquisition`, and the fence variant onto +`CoordinateError::ExternalTaintMarker` with the same `operation` — the exact +variants that step produced before the extraction. + +## Testing boundary + +Inline `#[cfg(test)] mod tests` uses RAII `tempfile::TempDir` fixtures over real +`git init` repositories (see [`../patterns.md`](../patterns.md)): a clean +worktree arms a fresh marker and reports no inherited taint, then `complete()` +clears it; a marker present on entry is reported as inherited and left armed; a +guard dropped without completing leaves the fence armed while releasing the +lock; the lock is proven held for the guard's whole lifetime through the +`on_lock_contention` seam; and a prefix that times out against a held lock fails +with `LockAcquisition` having armed nothing. + +The coordinator's own pre-existing fence and lock regressions +([`mutation-trace-runtime-coordinator.md`](mutation-trace-runtime-coordinator.md#testing-boundary)) +pass unchanged through the guard, which is what proves `coordinate()`'s +externally observable ordering and error semantics survived the extraction. + +See also: [`mutation-trace-runtime-coordinator.md`](mutation-trace-runtime-coordinator.md), +[`mutation-trace-external-taint.md`](mutation-trace-external-taint.md), +[`checkout-identity.md`](checkout-identity.md). diff --git a/context/cli/mutation-trace-ref-reconciliation.md b/context/cli/mutation-trace-ref-reconciliation.md index db8dc8a3..dc74874c 100644 --- a/context/cli/mutation-trace-ref-reconciliation.md +++ b/context/cli/mutation-trace-ref-reconciliation.md @@ -163,8 +163,10 @@ repository-wide read — no repository-global lock is needed. Reconciliation holds the **same** `/sce/mutation-cursor.lock` `WorktreeLock` that `coordinate()` holds across `pin → CAS → return`, acquired via `worktree_lock::acquire_inner` and bounded by the module-owned -`RECONCILIATION_LOCK_TIMEOUT` (`Duration::from_secs(10)`, matching the -coordinator's private `WORKTREE_LOCK_TIMEOUT` by intent, not a shared constant). +`RECONCILIATION_LOCK_TIMEOUT` (`Duration::from_secs(10)`, matching +`runtime::protected_worktree`'s private `WORKTREE_LOCK_TIMEOUT` by intent, not a +shared constant — there is no semantic reason the two must stay identical, so +each module owns its own). Mutual exclusion on that one file makes the pin → DB-CAS race structurally impossible: the reconciler's inventory → diff → delete runs wholly before `coordinate()` takes the lock (nothing pinned yet) or wholly after it releases diff --git a/context/cli/mutation-trace-runtime-coordinator.md b/context/cli/mutation-trace-runtime-coordinator.md index c5744dad..d7280de7 100644 --- a/context/cli/mutation-trace-runtime-coordinator.md +++ b/context/cli/mutation-trace-runtime-coordinator.md @@ -9,12 +9,11 @@ Git worktree, built by the `mutation-cursor-runtime-coordinator` plan `cli/src/services/mutation_trace/runtime/` is a private submodule (`pub(crate) mod runtime;` in `mutation_trace/mod.rs`), registered under the same `#[allow(dead_code)]` precedent as the rest of `mutation_trace`. -`coordinator::coordinate()` is the public entrypoint, but `runtime/mod.rs` -still declares `mod coordinator;` privately, so `coordinate()` is reachable -only from within `runtime` itself (its own tests) for now; a `pub(crate)` -re-export is deferred until a harness adapter needs it. `mod -ref_reconciliation;` and its `reconcile_worktree` entrypoint are private the -same way. Nothing under `runtime/` is wired into any hook, command, or +Every submodule — `coordinator`, `protected_worktree`, `ref_reconciliation`, +and the rest — is declared privately in `runtime/mod.rs`, so `coordinate()` and +`reconcile_worktree` are reachable only from within `runtime` itself (its own +tests) for now; a `pub(crate)` re-export is deferred until a harness adapter +needs it. Nothing under `runtime/` is wired into any hook, command, or `diff_traces` insertion yet. `runtime` depends on `protocol`/`store`/`types` and on `services::checkout`, @@ -23,12 +22,6 @@ documented convention. ## Current code surface -The per-worktree runtime lock, the isolated Git snapshot service, the -coordinator's protocol-integration pipeline, and the public `coordinate()` -entrypoint (lock, external-taint fence, checkout identity, and DB provider -around that pipeline) all exist, with `runtime/tests.rs` exercising the public -API end to end. Only harness/command wiring remains. - - `cli/src/services/mutation_trace/runtime/worktree_lock.rs` — `WorktreeLock::acquire(git_dir: &Path, timeout: Duration) -> Result` opens/creates @@ -64,6 +57,11 @@ API end to end. Only harness/command wiring remains. namespace no current worktree owns, via a deleted worktree or checkout-id metadata loss/recreation, is future repository-scoped work). Full contract in [`mutation-trace-ref-reconciliation.md`](mutation-trace-ref-reconciliation.md). +- `cli/src/services/mutation_trace/runtime/protected_worktree.rs` — the shared + safety prefix every runtime entrypoint runs behind (`ProtectedWorktree`: + resolve `git_dir` → `WorktreeLock` → external-taint fence → `WorktreeId`, plus + an explicit `complete()` as the only thing that clears the marker). Full + contract in [`mutation-trace-protected-worktree.md`](mutation-trace-protected-worktree.md). - `cli/src/services/mutation_trace/runtime/coordinator.rs` — the composition point that drives `protocol.rs`/`store.rs`/`git_snapshot.rs` together. Its `SnapshotCapture` trait (`capture(&self) -> Result`, `pin(&self, @@ -81,14 +79,12 @@ API end to end. Only harness/command wiring remains. protected operation. It does **not** receive an already-open DB handle: `open_db: impl FnOnce() -> anyhow::Result` is a caller-supplied provider it invokes itself, so DB acquisition falls inside - the external-taint fence. The critical section: resolve `git_dir` via - `checkout::resolve_git_dir`, acquire the `WorktreeLock` (bounded 10s, held - for the whole call), arm the `ExternalTaintMarker` write-ahead, resolve - checkout identity via `checkout::get_or_create_checkout_id` and wrap it as - `WorktreeId` — no caller-supplied `WorktreeId` or `Boundary` is ever - accepted — invoke `open_db()`, construct `GitSnapshotService`, delegate to - the internal generic-over-`SnapshotCapture` pipeline, and clear the marker - only on a successful outcome. Identity flows + the external-taint fence. The critical section is the `ProtectedWorktree` + prefix above — no caller-supplied `WorktreeId` or `Boundary` is ever + accepted — then `open_db()`, `GitSnapshotService`, the internal + generic-over-`SnapshotCapture` pipeline, and `ProtectedWorktree::complete()` + only on a successful outcome; `ProtectedWorktreeError` maps onto exactly the + `CoordinateError` variants that step already produced. Identity flows `repository_root → git_dir → WorktreeLock → checkout ID → WorktreeId`; the DB is not on that chain. (`coordinate()` is a one-line delegation to the `pub(super) coordinate_inner(.., on_lock_contention, after_load, after_recovery)` @@ -130,12 +126,11 @@ API end to end. Only harness/command wiring remains. `Conflict`, reporting `persisted_taint: false` only once every bounded attempt has been exhausted. -The runtime lock guards the coordinator's own critical section (external-taint -marker arming/clearing, snapshot capture, worktree/scope materialization, -recovery, and the CAS retry loop): `coordinate()` acquires it before arming the -marker and resolving checkout identity, and holds it until the call returns. It -is held on every `coordinate()` call, unlike the checkout-identity-creation -lock. `ref_reconciliation::reconcile_worktree` acquires the **same** lock file +The runtime lock guards the whole critical section — fence arming/clearing, +snapshot capture, worktree/scope materialization, recovery, and the CAS retry +loop — and is held on every `coordinate()` call, unlike the +checkout-identity-creation lock. +`ref_reconciliation::reconcile_worktree` acquires the **same** lock file (bounded by its own `RECONCILIATION_LOCK_TIMEOUT`) before it inventories pins, reads durable roots, or deletes anything. @@ -178,6 +173,10 @@ never blocking a fresh acquirer — each test uses a unique inline-unit-test precedent as `cli/src/services/checkout/mod.rs` and `cli/src/services/mutation_trace/store.rs` (see `context/patterns.md`). +`ProtectedWorktree`'s inline tests use RAII `tempfile::TempDir` fixtures over +real `git init` repositories; coverage in +[`mutation-trace-protected-worktree.md`](mutation-trace-protected-worktree.md#testing-boundary). + `GitSnapshotService`'s inline `#[cfg(test)] mod tests` in `git_snapshot.rs` uses the same precedent, extended to real per-test `git init` repositories; coverage in [`mutation-trace-snapshot-service.md`](mutation-trace-snapshot-service.md). @@ -229,15 +228,15 @@ entrypoint. ## Status -The lock, snapshot service, protocol-integration pipeline, and the public -`coordinate()` entrypoint (resolve `git_dir` → `WorktreeLock` → arm the -external-taint marker → checkout identity → caller-supplied DB provider → -pipeline → clear the marker on success) are all implemented, with -`runtime/tests.rs` covering the public API end to end; an inherited external-taint -marker is now overlaid onto `database_failure` recovery on the next invocation. A -`pub(crate)` re-export of `coordinate()` beyond `runtime` and harness/command -wiring remain future work tracked by the `mutation-cursor-external-taint` and -`mutation-cursor-runtime-coordinator` plans. +The `ProtectedWorktree` prefix, lock, snapshot service, protocol-integration +pipeline, and the public `coordinate()` entrypoint (prefix → DB provider → +pipeline → `complete()` on success) are all implemented, with `runtime/tests.rs` +covering the public API end to end; an inherited external-taint marker is +overlaid onto `database_failure` recovery on the next invocation. A `pub(crate)` +re-export of `coordinate()`, a second `abandon_scope()` entrypoint sharing the +same prefix, and harness/command wiring remain future work +(`mutation-cursor-external-taint`, `mutation-cursor-runtime-coordinator`, +`mutation-scope-runtime-integration`). See also: [`mutation-trace-ref-reconciliation.md`](mutation-trace-ref-reconciliation.md) (the per-worktree snapshot-ref maintenance pass under the same `WorktreeLock`), @@ -246,5 +245,6 @@ See also: [`mutation-trace-ref-reconciliation.md`](mutation-trace-ref-reconcilia [`mutation-trace-protocol.md`](mutation-trace-protocol.md), [`mutation-trace-store.md`](mutation-trace-store.md), [`mutation-trace-external-taint.md`](mutation-trace-external-taint.md) -(the `/sce/mutation-cursor-tainted` write-ahead fence armed by -`coordinate()`), [`checkout-identity.md`](checkout-identity.md). +(the `/sce/mutation-cursor-tainted` write-ahead fence), +[`mutation-trace-protected-worktree.md`](mutation-trace-protected-worktree.md) +(the shared prefix that arms it), [`checkout-identity.md`](checkout-identity.md). diff --git a/context/context-map.md b/context/context-map.md index 6a16a998..b2177859 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -27,7 +27,8 @@ Feature/domain context: - `context/cli/mutation-trace-revision-refinement.md` (the Quint `revision: int` → Rust `WorktreeState::revision: u64` bounded-integer refinement: the private `next_revision` checked-arithmetic helper `commit`/`taint`/`abandon`/`recover` all route through instead of a raw `+ 1`, so a worktree at `revision: u64::MAX` is a guarded no-op/rejection rather than a silent wrap to `0`) - `context/cli/mutation-trace-quint-connect.md` (`#[cfg(test)]`-only Quint Connect model-based-testing harness in `cli/src/services/mutation_trace/mbt/` continuously checking `protocol.rs` against `spec/mutation_cursor.qnt`: the verification-only `mbtAction`/`MbtAction` record-payload transport excluded from comparison, the operation-identity-vs-`MbtStutter` distinction on guarded/no-op branches with its two deterministic regressions, finite ID mapping, the AC5 comparable-state field list, `randomPrepare` staying a single `step` branch, deterministic/generated (500×30, seed-reproducible) test coverage, and the two Nix checks — generic `checks.cli-tests` and dedicated `checks.mutation-trace-quint-connect` — that both require the pinned Quint binary plus the top-level `spec/` directory in `workspaceSrc`'s Nix fileset) - `context/cli/mutation-trace-store.md` (durable persistence for the mutation-cursor protocol in `cli/src/services/mutation_trace/store.rs`, built by the `mutation-cursor-store-persistence` plan: the one-directional `protocol.rs` -> `DurableTransition::between` (pure structural diff) -> `store.rs` (SQL translation) -> `RepositoryAgentTraceDb` boundary; migration `004_mutation_trace_protocol.sql`'s five tables; `AttemptState`/`external_taint` non-persistence; the 8-byte big-endian `BLOB` revision encoding plus explicit non-`Debug` enum codecs; the bounded hot-path `load_worktree` read vs. the cold-path `load_mutation_event`; the cold-path read-only `load_tree_roots` (one worktree) / `load_all_tree_roots` (repository-wide) durable-tree-SHA queries for ref reconciliation, each a single-statement `UNION` of `cursor_tree`/`before_tree`/`after_tree` read from one DB snapshot; `commit`'s single-`BEGIN IMMEDIATE` CAS batch via `TursoDb::execute_transactional_cas_batch`, distinguishing `Conflict`/retryable-transient/deterministic-`Err` outcomes; and the store's non-goals — no Git/filesystem I/O, no attribution/boundary-kind decisions, no retry-after-`Conflict` loop, no row deletion) -- `context/cli/mutation-trace-runtime-coordinator.md` (imperative-shell runtime layer in `cli/src/services/mutation_trace/runtime/`, built by the `mutation-cursor-runtime-coordinator` plan: the per-worktree OS advisory lock, `runtime::worktree_lock::WorktreeLock::acquire(git_dir, timeout)` at `/sce/mutation-cursor.lock`, bounded `try_lock()` polling with a distinct matchable timeout error and RAII release, and its distinction from the separate checkout-identity-creation lock; the isolated Git snapshot service `runtime::git_snapshot::GitSnapshotService` (documented in `mutation-trace-snapshot-service.md`); and `runtime::coordinator`'s protocol-integration pipeline (`RuntimeBoundary`/`CoordinateOutcome`/`CoordinateError`, the load → recover-if-needed → prepare/commit CAS-retry loop, and its own bounded snapshot-failure taint-retry loop) plus the public `coordinate(repository_root, boundary, open_db)` entrypoint that acquires the `WorktreeLock`, arms the external-taint write-ahead fence, derives `WorktreeId` from `checkout::get_or_create_checkout_id`, invokes the caller-supplied `open_db` provider so DB acquisition falls inside that fence, drives the pipeline under one held lock, and clears the marker only on success; `runtime/tests.rs` covers the public `coordinate()` API end to end against real linked worktrees and a real Agent Trace DB; an inherited external-taint marker is overlaid onto `protocol::database_failure` recovery on the next invocation, against the single captured snapshot and re-injected across a losing recovery CAS; the private `runtime::ref_reconciliation` per-worktree snapshot-ref maintenance pass (documented in `mutation-trace-ref-reconciliation.md`) shares the same `WorktreeLock`; a `pub(crate)` re-export of `coordinate()` beyond `runtime` and harness/command wiring remain future work) +- `context/cli/mutation-trace-runtime-coordinator.md` (imperative-shell runtime layer in `cli/src/services/mutation_trace/runtime/`, built by the `mutation-cursor-runtime-coordinator` plan: the per-worktree OS advisory lock, `runtime::worktree_lock::WorktreeLock::acquire(git_dir, timeout)` at `/sce/mutation-cursor.lock`, bounded `try_lock()` polling with a distinct matchable timeout error and RAII release, and its distinction from the separate checkout-identity-creation lock; the isolated Git snapshot service `runtime::git_snapshot::GitSnapshotService` (documented in `mutation-trace-snapshot-service.md`); and `runtime::coordinator`'s protocol-integration pipeline (`RuntimeBoundary`/`CoordinateOutcome`/`CoordinateError`, the load → recover-if-needed → prepare/commit CAS-retry loop, and its own bounded snapshot-failure taint-retry loop) plus the public `coordinate(repository_root, boundary, open_db)` entrypoint that runs the shared `runtime::protected_worktree` prefix (`WorktreeLock` -> external-taint write-ahead fence -> `WorktreeId`, extracted so a second entrypoint cannot drift from it and documented in `mutation-trace-protected-worktree.md`), invokes the caller-supplied `open_db` provider so DB acquisition falls inside that fence, drives the pipeline under one held lock, and clears the marker through `ProtectedWorktree::complete()` only on success; `runtime/tests.rs` covers the public `coordinate()` API end to end against real linked worktrees and a real Agent Trace DB; an inherited external-taint marker is overlaid onto `protocol::database_failure` recovery on the next invocation, against the single captured snapshot and re-injected across a losing recovery CAS; the private `runtime::ref_reconciliation` per-worktree snapshot-ref maintenance pass (documented in `mutation-trace-ref-reconciliation.md`) shares the same `WorktreeLock`; a `pub(crate)` re-export of `coordinate()` beyond `runtime` and harness/command wiring remain future work) +- `context/cli/mutation-trace-protected-worktree.md` (the shared safety prefix every mutation-cursor runtime entrypoint runs behind, in `cli/src/services/mutation_trace/runtime/protected_worktree.rs`, extracted from `coordinate()` by the `mutation-scope-runtime-integration` plan so a second entrypoint cannot drift from it: `ProtectedWorktree::acquire(repository_root)` running the safety-critical fixed order resolve `git_dir` → `WorktreeLock` (module-owned 10s `WORKTREE_LOCK_TIMEOUT`) → `ExternalTaintMarker::exists()` → `persist()` (fence armed write-ahead of every fallible step that follows, including DB acquisition) → `get_or_create_checkout_id` as `WorktreeId`; the `worktree_id()` / `inherited_external_taint()` / consuming `complete()` surface, where `complete()` clears the marker under the still-held lock and is the only thing that ever clears it while `Drop` releases only the lock; and the one-variant-per-prefix-step `ProtectedWorktreeError` (`GitDirResolution` | `LockAcquisition` | `ExternalTaintMarker { operation, source }` | `CheckoutIdentity`) each entrypoint maps onto its own error surface — `coordinate()` onto exactly the `CoordinateError` variants that step produced before the extraction) - `context/cli/mutation-trace-ref-reconciliation.md` (the conservative per-worktree snapshot-ref reconciliation pass in `cli/src/services/mutation_trace/runtime/ref_reconciliation.rs`, built by the `mutation-cursor-ref-reconciliation` plan: `reconcile_worktree(repository_root, open_db)` → `pub(super) reconcile_worktree_inner(.., on_lock_contention)`, both returning `ReconciliationOutcome` (`Reconciled(ReconciliationReport { local_required, retained, deleted })` for a pass that ran | `SkippedNoCheckoutIdentity` — an `Ok`, not an `Err` — when no current checkout identity could be derived), a variant-per-fallible-step `ReconcileError` with no `Other`, and the module-owned `RECONCILIATION_LOCK_TIMEOUT`; the two-invariant model — a strictly per-worktree fail-closed local-consistency check via `load_tree_roots(W)` vs. a repository-wide deletion-safety set via `load_all_tree_roots()` so an `A`-owned ref is retained whenever any worktree still durably needs its tree — run entirely under the same `/sce/mutation-cursor.lock` `WorktreeLock` `coordinate()` holds, with the repository-wide read kept coherent by being one SQL statement / one DB snapshot rather than a repository-global lock; deletes only SCE-owned refs via one atomic `git update-ref --no-deref --stdin`, writes no `mutation_trace_*` row, never arms `ExternalTaintMarker`, runs no `git gc`; imperative durability maintenance below the verified Quint protocol; reclaims orphan/unreferenced refs only for the namespace of a checkout id a current worktree still derives — a namespace no current worktree owns (identity-based: a deleted linked worktree, or checkout-id metadata loss followed by `get_or_create_checkout_id` minting a fresh id on a still-present worktree) is beyond every per-worktree pass and left to a recorded future repository-scoped unowned-namespace operation, so the current pass does not bound all orphan-ref growth; `reconcile_worktree` has no `pub(crate)` re-export and no harness/command wiring yet) - `context/cli/mutation-trace-snapshot-service.md` (the isolated Git snapshot and ref-pinning service `runtime::git_snapshot::GitSnapshotService` in `cli/src/services/mutation_trace/runtime/git_snapshot.rs`: `new` resolving an absolute `git_dir`, `capture_tree` snapshotting staged/unstaged/untracked/deleted worktree state into the repository's normal object database via a throwaway temp index, `pin_tree` protecting a durable tree with a create-only idempotent **direct** `refs/sce/mutation-cursor//` ref, `diff_trees` emitting `patch.rs`-parseable raw diff text; plus the callerless reconciliation substrate — worktree-scoped `list_pins` inventory returning `Result, PinInventoryError>` that rejects a symbolic ref inside the namespace (mutation-cursor pins are direct refs) as `MalformedRef`, matchable separately from a `git for-each-ref` execution failure, and conditional-atomic `delete_pins` running one `git update-ref --no-deref --stdin` transaction of SHA-conditioned deletes — no-dereference so an inventory→delete direct-ref→symref race cannot escape the inventoried namespace, plus a fail-closed pre-check — that aborts whole if any ref changed since inventory; `REF_NAMESPACE` + `pin_ref_prefix` as the single source of truth for the pin path) - `context/cli/mutation-trace-external-taint.md` (the worktree-local mutation-cursor durability boundary in `cli/src/services/mutation_trace/runtime/external_taint.rs`, built by the `mutation-cursor-external-taint` plan: the `ExternalTaintMarker` primitive — `new(git_dir)`/`exists()`/`persist()`/`clear()` over an empty file at `/sce/mutation-cursor-tainted` whose existence is its entire state, `checkout::persist_checkout_id_inner`-style durability (`sync_data` plus best-effort `#[cfg(unix)]` parent-dir `sync_all`), idempotent persist/clear, `NotFound`-on-clear as success, no `Drop` deletion — as the concrete runtime refinement of the abstract `ProtocolState.external_taint`; armed by the reshaped `coordinate()` entrypoint write-ahead after the `WorktreeLock` and before Agent Trace DB acquisition (a caller-supplied DB provider closure), cleared only on a successful `CoordinateOutcome`, with dedicated fail-closed pre-commit `CoordinateError::ExternalTaintMarker` (`Inspect`/`Persist` only)/`AgentTraceDbUnavailable` variants plus a post-commit `MarkerClearAfterCommit { source, committed }` that carries the durable outcome so a failed trailing clear never hides a committed `MutationEvent`; an inherited marker seeds an invocation-local `external_taint_pending` flag that overlays `protocol::database_failure` onto each freshly loaded projection so `recover` runs once against the captured snapshot, held across a losing recovery CAS and cleared once it lands) diff --git a/context/patterns.md b/context/patterns.md index fd8db8ae..6ad26b9a 100644 --- a/context/patterns.md +++ b/context/patterns.md @@ -189,7 +189,7 @@ - Pure unit tests should test in-memory logic, parsing, validation, and data transformations without external dependencies; prefer mocking/faking external dependencies over creating real filesystem or database state when a fake is available and sufficient. - In-memory database tests (e.g., `LocalDatabaseTarget::InMemory`) are acceptable for unit tests since they don't touch the filesystem. -- A filesystem-touching `#[cfg(test)] mod tests` inline in the module under test is an established, Nix-sandbox-safe pattern in this codebase when every path is a unique, process-local path under `std::env::temp_dir()` (which resolves through `TMPDIR`, itself a writable per-build directory inside the Nix sandbox) — see `cli/src/services/mutation_trace/store.rs`'s `RepositoryAgentTraceDb`-backed tests and `cli/src/services/checkout/mod.rs`'s checkout-identity-lock tests. This is not integration-test-only territory in this repository. +- A filesystem-touching `#[cfg(test)] mod tests` inline in the module under test is an established, Nix-sandbox-safe pattern in this codebase when every path is a unique, process-local path under `TMPDIR` (itself a writable per-build directory inside the Nix sandbox). Prefer an RAII `tempfile::TempDir` fixture, which cleans itself up even when a test panics — as `cli/src/services/mutation_trace/store.rs`'s `RepositoryAgentTraceDb`-backed tests, `runtime/tests.rs`, and `runtime/protected_worktree.rs` do. Manually composed unique `std::env::temp_dir()` paths are the older form of the same pattern and remain in `cli/src/services/checkout/mod.rs`'s checkout-identity-lock tests and in `runtime/coordinator.rs`, `runtime/worktree_lock.rs`, and `runtime/external_taint.rs`; they leak their directory on a panicking test, so new tests should use `TempDir`. This is not integration-test-only territory in this repository. - Use integration tests instead of this inline pattern where the behavior genuinely cannot be made deterministic and isolated inside the Nix sandbox this way. - Do not depend on a shared or ambient path (`$HOME`, a fixed `/tmp` file name, the repository's own working tree) from a unit test; each test must construct its own unique, self-cleaning path. - When a unit test needs behavior that cannot be made Nix-sandbox-safe this way (for example, network access, or state shared across the whole test binary), delete it from the unit-test suite and reintroduce that coverage later as an integration test instead of keeping ignored tests in-tree. diff --git a/context/plans/mutation-scope-runtime-integration.md b/context/plans/mutation-scope-runtime-integration.md index 9ade374d..b03218c3 100644 --- a/context/plans/mutation-scope-runtime-integration.md +++ b/context/plans/mutation-scope-runtime-integration.md @@ -101,12 +101,22 @@ performs final validation. `Active` on the same worktree leaves B `Active` through the subsequent `needs_rebaseline` recovery. - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::tests::` — the surviving-scope test asserts B's status after the next `coordinate()` call. -- [ ] AC13: `runtime/mod.rs` re-exports `coordinate`, `CoordinateError`, - `CoordinateOutcome`, `RuntimeBoundary`, `abandon_scope`, `AbandonScopeError`, - and `AbandonScopeOutcome` (with its reason type) at `pub(crate)`, and exports - nothing from `git_snapshot`, `external_taint`, `worktree_lock`, - `ref_reconciliation`, or the protected-worktree primitive. - - Validate: inspect `cli/src/services/mutation_trace/runtime/mod.rs` for exactly those re-exports, and `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` stays clean. +- [ ] AC13: `runtime/mod.rs` re-exports, at `pub(crate)`, exactly `coordinate`, + `CoordinateError`, `CoordinateOutcome`, `ExternalTaintOperation`, + `RuntimeBoundary`, `abandon_scope`, `AbandonScopeError`, and + `AbandonScopeOutcome` (with its reason type). `ExternalTaintOperation` is part + of `CoordinateError::ExternalTaintMarker`'s own public shape — a crate-visible + `CoordinateError` a caller cannot match on is not a usable seam — so it must + cross the boundary alongside it. Since T01 it lives in `protected_worktree.rs` + and reaches the seam through `coordinator.rs`'s existing + `pub use super::protected_worktree::ExternalTaintOperation`, so the type + becomes crate-visible **without** `protected_worktree` becoming a public + module. Every `mod` declaration in `runtime/mod.rs` stays private, and nothing + else is re-exported from `git_snapshot`, `external_taint`, `worktree_lock`, + `ref_reconciliation`, or `protected_worktree` — in particular + `ProtectedWorktree`, `ProtectedWorktreeError`, and `WORKTREE_LOCK_TIMEOUT` + remain internal to `runtime`. + - Validate: inspect `cli/src/services/mutation_trace/runtime/mod.rs` for exactly those re-exports and confirm every `mod` declaration there is still private (`rg -n '^\s*(pub(\(crate\))?\s+)?mod |pub\(crate\) use' cli/src/services/mutation_trace/runtime/mod.rs`), and `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` stays clean. - [ ] AC14: `spec/mutation_cursor.qnt`, the Quint refinement matrix in `cli/src/services/mutation_trace/mod.rs`, `protocol.rs`'s transition semantics, `004_mutation_trace_protocol.sql`, and the migration set carry no change @@ -306,19 +316,61 @@ which is precisely why there is nothing left for this one to decide. (`InheritedExternalTaint`, `MissingScope`, `NeverSeenScope`, `MissingWorktreeState`) carried in the outcome, so callers and tests can match on them. -- `abandon_scope()` reuses `coordinator.rs`'s `WORKTREE_LOCK_TIMEOUT` rather than - declaring its own, matching the reconciliation pass's precedent of an - operation-appropriate timeout only where the operation genuinely differs. +- `abandon_scope()` goes through the same protected-worktree acquisition path as + `coordinate()` and therefore uses the `WORKTREE_LOCK_TIMEOUT` owned by + `runtime/protected_worktree.rs` (T01 moved the constant there with the prefix); + it must not declare a second mutation-scope lock timeout. `ref_reconciliation` + keeps its separately owned `RECONCILIATION_LOCK_TIMEOUT` — that remains + intentional, matching by value but not by ownership, since a reconciliation + pass is an operation that genuinely differs. ## Task stack -- [ ] T01: `Extract the protected-worktree runtime guard` (status:todo) +- [x] T01: `Extract the protected-worktree runtime guard` (status:done) - Task ID: T01 - Scope: In — new `cli/src/services/mutation_trace/runtime/protected_worktree.rs` owning git-dir resolution, `WorktreeLock` acquisition, external-marker inspect + persist, checkout identity, `WorktreeId` derivation, and an explicit `complete`/`clear` step; refactor `coordinator.rs`'s `coordinate_inner` / `coordinate_protected` prefix onto it; keep the `on_lock_contention` test seam working. Out — any new entrypoint, any store change, any behavior change to the pipeline below the prefix, any `runtime/mod.rs` export change. - Dependencies: none - Done when: the guard exposes the derived `WorktreeId`, whether a marker was already present before this invocation, and an explicit completion that clears the marker; it never clears the marker in `Drop`; it holds the `WorktreeLock` for its own lifetime; `coordinate()` produces identical outcomes and identical `CoordinateError` variants on every existing path, with the existing coordinator fence/lock tests passing without assertion changes; focused tests cover the guard itself for lock-timeout failure, an inherited marker being reported, a fresh marker being armed, and the marker surviving a dropped guard that was never completed. - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::`; `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings`; `git diff` on `coordinator.rs` shows no reordering of lock → marker → checkout → DB. - - Context synchronization: pending + - Completed: 2026-09-03 + - Files changed: + - `cli/src/services/mutation_trace/runtime/protected_worktree.rs` (new) + - `cli/src/services/mutation_trace/runtime/mod.rs` + - `cli/src/services/mutation_trace/runtime/coordinator.rs` + - `cli/src/services/mutation_trace/runtime/ref_reconciliation.rs` + - Result: `protected_worktree.rs` now owns the safety prefix as `ProtectedWorktree`, + running resolve `git_dir` → `WorktreeLock` → marker inspect → marker persist → + checkout identity → `WorktreeId` in the coordinator's existing order. It exposes + `worktree_id()`, `inherited_external_taint()`, and a consuming `complete()` that + clears the marker while the lock is still held; `Drop` releases only the lock and + never clears the marker. `acquire` uses the relocated `WORKTREE_LOCK_TIMEOUT` + (10s, value unchanged), `pub(super) acquire_inner` carries the `on_lock_contention` + seam, and a private `acquire_with_timeout` serves the guard's own timeout test. + `coordinate_inner` now acquires the guard and maps `ProtectedWorktreeError` onto + the pre-existing `CoordinateError` variants (git-dir resolution and checkout + identity → `Other`, lock → `LockAcquisition`, fence → `ExternalTaintMarker` with + the same operation); `coordinate_protected` lost its `git_dir` parameter and takes + the guard's `&WorktreeId`. `ExternalTaintOperation` moved to `protected_worktree.rs` + and is `pub use`-re-exported from `coordinator.rs`, so `CoordinateError`'s shape is + unchanged. No coordinator test assertion was modified — only test-module imports + were added. Per an explicit user instruction during implementation, every comment + this task wrote or touched was then removed from the code, including + `ref_reconciliation.rs`'s pre-existing `RECONCILIATION_LOCK_TIMEOUT` doc comment + (which had named `WORKTREE_LOCK_TIMEOUT`'s old home); the rationale it carried is + preserved in `context/cli/mutation-trace-ref-reconciliation.md`. + - Verify results: + - `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::` — passed: 96 passed, 0 failed (91 pre-existing plus the 5 new guard tests). + - `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` — passed, clean. + - `git diff` on `coordinator.rs` — confirmed: the prefix steps moved verbatim, lock → marker inspect → marker persist → checkout identity → `open_db` order intact, no reordering. + - Also run: `./scripts/run-cli-cargo.sh fmt --manifest-path cli/Cargo.toml -- --check` — passed after formatting the new file. + - Context impact: local to `cli/src/services/mutation_trace/runtime/`. No public + interface, schema, migration, spec, or protocol change; `coordinate()`'s signature, + outcomes, and error variants are unchanged. Durable context affected: + `context/cli/mutation-trace-runtime-coordinator.md` (the prefix is now the + `ProtectedWorktree` primitive rather than inline coordinator code) and + `context/cli/mutation-trace-external-taint.md` (the fence's arm/clear ownership + moved to that guard, whose `Drop` never clears). + - Context synchronization: synced - [ ] T02: `Expose a bounded scope read on MutationTraceStore` (status:todo) - Task ID: T02 @@ -346,9 +398,9 @@ which is precisely why there is nothing left for this one to decide. - [ ] T05: `Export the runtime seam and record the adapter contract` (status:todo) - Task ID: T05 - - Scope: In — `pub(crate) use` re-exports in `runtime/mod.rs` for `coordinate`, `CoordinateError`, `CoordinateOutcome`, `RuntimeBoundary`, `abandon_scope`, `AbandonScopeError`, `AbandonScopeOutcome` and its reason type; keeping `git_snapshot`, `external_taint`, `worktree_lock`, `ref_reconciliation`, and `protected_worktree` unexported; new `context/cli/mutation-scope-runtime.md` plus its `context/context-map.md` and `context/overview.md` index entries. Out — any harness, hook, or command wiring; any change to the runtime implementation; the per-task context updates T01–T04 each own for their own domain files. + - Scope: In — `pub(crate) use` re-exports in `runtime/mod.rs` for `coordinate`, `CoordinateError`, `CoordinateOutcome`, `ExternalTaintOperation`, `RuntimeBoundary`, `abandon_scope`, `AbandonScopeError`, `AbandonScopeOutcome` and its reason type — conceptually `pub(crate) use coordinator::{coordinate, CoordinateError, CoordinateOutcome, ExternalTaintOperation, RuntimeBoundary};` plus the `scope_runtime` names, `ExternalTaintOperation` riding through `coordinator`'s own `pub use` of it because `CoordinateError::ExternalTaintMarker` carries it; keeping the `git_snapshot`, `external_taint`, `worktree_lock`, `ref_reconciliation`, and `protected_worktree` **modules** private and re-exporting nothing else from any of them (`ProtectedWorktree`, `ProtectedWorktreeError`, and `WORKTREE_LOCK_TIMEOUT` stay internal); new `context/cli/mutation-scope-runtime.md` plus its `context/context-map.md` and `context/overview.md` index entries. Out — any harness, hook, or command wiring; any change to the runtime implementation, including moving `ExternalTaintOperation` or `WORKTREE_LOCK_TIMEOUT` back out of `protected_worktree.rs`; the per-task context updates T01–T04 each own for their own domain files. - Dependencies: T04 - - Done when: the seven names above are reachable as `crate::services::mutation_trace::runtime::*` and nothing from the five private modules is; `clippy --all-targets -- -D warnings` is clean with no placeholder consumer added to satisfy it; `context/cli/mutation-scope-runtime.md` states the scope-identity rule, the `Start`/`Advance`/`Close` semantics, the positive-evidence requirement for `abandon_scope()` and the prohibition on inferring staleness from `ActorKind`, the successor-scope sequence and what each outcome implies for it (including that a failed abandonment must not be treated as a safely started successor), that abandonment is not a `RuntimeBoundary` and requires no Quint change, the D1 strong-recovery tradeoff for a missing or `NeverSeen` target, and the `AiExclusive` attribution boundary; `context-map.md` and `overview.md` name the new module and file. + - Done when: the eight names above are reachable as `crate::services::mutation_trace::runtime::*`, including `ExternalTaintOperation` so a crate-level caller can match `CoordinateError::ExternalTaintMarker`; the five modules remain private and nothing else from them is reachable; `clippy --all-targets -- -D warnings` is clean with no placeholder consumer added to satisfy it; `context/cli/mutation-scope-runtime.md` states the scope-identity rule, the `Start`/`Advance`/`Close` semantics, the positive-evidence requirement for `abandon_scope()` and the prohibition on inferring staleness from `ActorKind`, the successor-scope sequence and what each outcome implies for it (including that a failed abandonment must not be treated as a safely started successor), that abandonment is not a `RuntimeBoundary` and requires no Quint change, the D1 strong-recovery tradeoff for a missing or `NeverSeen` target, and the `AiExclusive` attribution boundary; `context-map.md` and `overview.md` name the new module and file. - Verify: `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings`; `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml`; read `context/cli/mutation-scope-runtime.md` against the shipped `scope_runtime.rs` signatures. - Context synchronization: pending From d245b0e4ceb4037455d390df352d65630b768e8f Mon Sep 17 00:00:00 2001 From: David Abram Date: Thu, 3 Sep 2026 12:13:54 +0200 Subject: [PATCH 3/6] mutation-trace: Expose a bounded scope read Promote MutationTraceStore::load_scope to a public, cold-path single-row read that returns durable scope state without widening into a worktree projection or adjudicating worktree identity. Add coverage for known, missing, and cross-worktree scopes while preserving load_worktree's existing mismatch behavior. Document the store contract and mark the runtime-integration task complete in the shared context. Plan: mutation-scope-runtime-integration (T02) --- cli/src/services/mutation_trace/store.rs | 127 ++++++++++++++++-- context/cli/mutation-trace-store.md | 17 +++ context/context-map.md | 2 +- .../mutation-scope-runtime-integration.md | 42 +++++- 4 files changed, 175 insertions(+), 13 deletions(-) diff --git a/cli/src/services/mutation_trace/store.rs b/cli/src/services/mutation_trace/store.rs index 4ae9676a..213d1df1 100644 --- a/cli/src/services/mutation_trace/store.rs +++ b/cli/src/services/mutation_trace/store.rs @@ -720,6 +720,34 @@ impl<'a> MutationTraceStore<'a> { Ok(rows.into_iter().collect()) } + /// Loads the durable [`ScopeState`] for `scope_id` — its status, + /// `actor_kind`, and `worktree_id` — or `None` when no + /// `mutation_trace_scopes` row exists for it. + /// + /// A cold-path single-row read, and deliberately the narrowest scope seam + /// there is: it reads one `mutation_trace_scopes` row and nothing else. It + /// never consults `mutation_trace_events`, + /// `mutation_trace_processed_events`, or the scope's + /// `mutation_trace_worktrees` row, and it must not widen into a + /// projection — [`MutationTraceStore::load_worktree`] is the projection + /// seam, and a caller needing worktree state alongside a scope belongs + /// there instead. + /// + /// This never adjudicates worktree identity: a scope whose `worktree_id` + /// differs from the caller's own worktree is returned as-is, not rejected. + /// Comparing the two is the caller's decision, since the same row is a + /// legitimate read from its owning worktree and a cross-worktree reference + /// from any other. + pub fn load_scope(&self, scope_id: &ScopeId) -> Result> { + let rows = self.db.query_map( + SELECT_SCOPE_BY_ID_SQL, + (scope_id.0.as_str(),), + scope_row_from_turso, + )?; + + Ok(rows.into_iter().next().map(|(_, scope_state)| scope_state)) + } + fn load_worktree_state(&self, worktree: &WorktreeId) -> Result> { let rows = self.db.query_map( SELECT_WORKTREE_SQL, @@ -743,16 +771,6 @@ impl<'a> MutationTraceStore<'a> { Ok(rows.into_iter().collect()) } - fn load_scope(&self, scope_id: &ScopeId) -> Result> { - let rows = self.db.query_map( - SELECT_SCOPE_BY_ID_SQL, - (scope_id.0.as_str(),), - scope_row_from_turso, - )?; - - Ok(rows.into_iter().next().map(|(_, scope_state)| scope_state)) - } - fn processed_event_exists(&self, event_key: &EventKey) -> Result { let rows = self.db.query_map( SELECT_PROCESSED_EVENT_SQL, @@ -1577,6 +1595,95 @@ mod tests { assert!(error.to_string().contains("scope-b")); } + #[test] + fn load_scope_returns_the_durable_state_for_a_known_scope() { + let db_fixture = test_db_path("load-scope-known"); + let db_path = db_fixture.path(); + let db = RepositoryAgentTraceDb::new_at(db_path).expect("repository DB should open"); + let store = MutationTraceStore::new(&db); + + insert_worktree(&db, "wt-1", 7); + insert_scope(&db, "scope-1", "wt-1", ScopeStatus::Closed); + insert_mutation_event( + &db, + "wt-1", + 7, + "tree-0", + "tree-1", + "ai_exclusive", + Some("scope-1"), + "close", + Some("scope-1"), + Some("event-1"), + &["scope-1"], + ); + insert_processed_event(&db, "scope-1", "event-1"); + + let scope_state = store + .load_scope(&ScopeId("scope-1".to_string())) + .expect("load_scope should succeed") + .expect("known scope should be present"); + + assert_eq!( + scope_state, + ScopeState { + status: ScopeStatus::Closed, + actor_kind: ActorKind::ClaudeCode, + worktree_id: WorktreeId("wt-1".to_string()), + } + ); + } + + #[test] + fn load_scope_returns_none_for_an_unknown_scope() { + let db_fixture = test_db_path("load-scope-unknown"); + let db_path = db_fixture.path(); + let db = RepositoryAgentTraceDb::new_at(db_path).expect("repository DB should open"); + let store = MutationTraceStore::new(&db); + + insert_worktree(&db, "wt-1", 0); + insert_scope(&db, "scope-1", "wt-1", ScopeStatus::Active); + + let scope_state = store + .load_scope(&ScopeId("scope-missing".to_string())) + .expect("load_scope should succeed for an unknown scope"); + assert!(scope_state.is_none()); + } + + #[test] + fn load_scope_returns_a_scope_belonging_to_another_worktree() { + let db_fixture = test_db_path("load-scope-other-worktree"); + let db_path = db_fixture.path(); + let db = RepositoryAgentTraceDb::new_at(db_path).expect("repository DB should open"); + let store = MutationTraceStore::new(&db); + + insert_worktree(&db, "wt-1", 0); + insert_scope(&db, "scope-other", "wt-2", ScopeStatus::Active); + + let scope_state = store + .load_scope(&ScopeId("scope-other".to_string())) + .expect("load_scope should not reject a scope on another worktree") + .expect("the scope row should be returned as-is"); + + assert_eq!( + scope_state, + ScopeState { + status: ScopeStatus::Active, + actor_kind: ActorKind::ClaudeCode, + worktree_id: WorktreeId("wt-2".to_string()), + } + ); + + let error = store + .load_worktree( + &WorktreeId("wt-1".to_string()), + Some(&ScopeId("scope-other".to_string())), + None, + ) + .expect_err("load_worktree should still reject the cross-worktree scope"); + assert!(error.to_string().contains("scope-other")); + } + #[test] fn load_mutation_event_returns_none_when_missing() { let db_fixture = test_db_path("cold-path-missing"); diff --git a/context/cli/mutation-trace-store.md b/context/cli/mutation-trace-store.md index 3efba870..92e12f10 100644 --- a/context/cli/mutation-trace-store.md +++ b/context/cli/mutation-trace-store.md @@ -73,6 +73,23 @@ cold path: it reconstructs one historical `MutationEvent`, including full called from `load_worktree` or from any hook-boundary path, so a projection load never pays for the full historical event set. +`MutationTraceStore::load_scope(scope_id) -> Result>` is the +public single-scope read seam: one `mutation_trace_scopes` row by primary key, +returning the durable `status` / `actor_kind` / `worktree_id`, or `None` when no +row exists. It is a cold path and deliberately the narrowest scope read there +is — it consults neither `mutation_trace_events`, +`mutation_trace_processed_events`, nor the scope's `mutation_trace_worktrees` +row, and it must not widen into a projection; `load_worktree` remains the +projection seam for any caller that needs worktree state alongside a scope. + +Unlike `load_worktree`, it **never adjudicates worktree identity**: a scope whose +stored `worktree_id` differs from the caller's own worktree is returned as-is +rather than rejected. The same row is a legitimate read from its owning worktree +and a cross-worktree reference from any other, so the comparison — and the +decision of what a mismatch means — belongs to the caller. `load_worktree`'s +stricter contract is unchanged: an effective referenced scope on another +worktree is still an `Err` there. + ## Durable tree-root reads (ref reconciliation) `load_tree_roots(worktree)` and `load_all_tree_roots()` are two further diff --git a/context/context-map.md b/context/context-map.md index b2177859..f6e27ff6 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -26,7 +26,7 @@ Feature/domain context: - `context/cli/mutation-trace-protocol.md` (pure, dependency-free `cli/src/services/mutation_trace/` refinement of the verified `spec/mutation_cursor.qnt` mutation-cursor protocol: the full pure action set — `prepare`/`commit` transition logic, attribution/mutation-event materialization, snapshot-failure/database-failure taint actions, scope abandonment, and recovery with an explicit observed-tree input — plus cross-action sequence/invariant tests and a module-level Quint refinement matrix are all complete (`types.rs`, `protocol.rs`, `mod.rs`, `tests.rs`, registered with `#[allow(dead_code)]`); opaque-newtype identity refinement decisions vs. the Quint model's bounded enums, and the `coordinator.rs` target end-state seam this layout leaves room for but does not create — `store.rs`, `runtime/git_snapshot.rs`, and `runtime/coordinator.rs` (including its public `coordinate()` entrypoint) now exist, built out by the `mutation-cursor-store-persistence` and `mutation-cursor-runtime-coordinator` plans respectively; `protocol.rs`'s pure transitions are not yet wired into any hook or command) - `context/cli/mutation-trace-revision-refinement.md` (the Quint `revision: int` → Rust `WorktreeState::revision: u64` bounded-integer refinement: the private `next_revision` checked-arithmetic helper `commit`/`taint`/`abandon`/`recover` all route through instead of a raw `+ 1`, so a worktree at `revision: u64::MAX` is a guarded no-op/rejection rather than a silent wrap to `0`) - `context/cli/mutation-trace-quint-connect.md` (`#[cfg(test)]`-only Quint Connect model-based-testing harness in `cli/src/services/mutation_trace/mbt/` continuously checking `protocol.rs` against `spec/mutation_cursor.qnt`: the verification-only `mbtAction`/`MbtAction` record-payload transport excluded from comparison, the operation-identity-vs-`MbtStutter` distinction on guarded/no-op branches with its two deterministic regressions, finite ID mapping, the AC5 comparable-state field list, `randomPrepare` staying a single `step` branch, deterministic/generated (500×30, seed-reproducible) test coverage, and the two Nix checks — generic `checks.cli-tests` and dedicated `checks.mutation-trace-quint-connect` — that both require the pinned Quint binary plus the top-level `spec/` directory in `workspaceSrc`'s Nix fileset) -- `context/cli/mutation-trace-store.md` (durable persistence for the mutation-cursor protocol in `cli/src/services/mutation_trace/store.rs`, built by the `mutation-cursor-store-persistence` plan: the one-directional `protocol.rs` -> `DurableTransition::between` (pure structural diff) -> `store.rs` (SQL translation) -> `RepositoryAgentTraceDb` boundary; migration `004_mutation_trace_protocol.sql`'s five tables; `AttemptState`/`external_taint` non-persistence; the 8-byte big-endian `BLOB` revision encoding plus explicit non-`Debug` enum codecs; the bounded hot-path `load_worktree` read vs. the cold-path `load_mutation_event`; the cold-path read-only `load_tree_roots` (one worktree) / `load_all_tree_roots` (repository-wide) durable-tree-SHA queries for ref reconciliation, each a single-statement `UNION` of `cursor_tree`/`before_tree`/`after_tree` read from one DB snapshot; `commit`'s single-`BEGIN IMMEDIATE` CAS batch via `TursoDb::execute_transactional_cas_batch`, distinguishing `Conflict`/retryable-transient/deterministic-`Err` outcomes; and the store's non-goals — no Git/filesystem I/O, no attribution/boundary-kind decisions, no retry-after-`Conflict` loop, no row deletion) +- `context/cli/mutation-trace-store.md` (durable persistence for the mutation-cursor protocol in `cli/src/services/mutation_trace/store.rs`, built by the `mutation-cursor-store-persistence` plan: the one-directional `protocol.rs` -> `DurableTransition::between` (pure structural diff) -> `store.rs` (SQL translation) -> `RepositoryAgentTraceDb` boundary; migration `004_mutation_trace_protocol.sql`'s five tables; `AttemptState`/`external_taint` non-persistence; the 8-byte big-endian `BLOB` revision encoding plus explicit non-`Debug` enum codecs; the bounded hot-path `load_worktree` read vs. the cold-path `load_mutation_event`; the public cold-path single-row `load_scope` scope seam that returns one `mutation_trace_scopes` row as `Option` without widening into a projection and, unlike `load_worktree`, never adjudicates worktree identity (a cross-worktree scope is returned as-is, and the mismatch is the caller's decision); the cold-path read-only `load_tree_roots` (one worktree) / `load_all_tree_roots` (repository-wide) durable-tree-SHA queries for ref reconciliation, each a single-statement `UNION` of `cursor_tree`/`before_tree`/`after_tree` read from one DB snapshot; `commit`'s single-`BEGIN IMMEDIATE` CAS batch via `TursoDb::execute_transactional_cas_batch`, distinguishing `Conflict`/retryable-transient/deterministic-`Err` outcomes; and the store's non-goals — no Git/filesystem I/O, no attribution/boundary-kind decisions, no retry-after-`Conflict` loop, no row deletion) - `context/cli/mutation-trace-runtime-coordinator.md` (imperative-shell runtime layer in `cli/src/services/mutation_trace/runtime/`, built by the `mutation-cursor-runtime-coordinator` plan: the per-worktree OS advisory lock, `runtime::worktree_lock::WorktreeLock::acquire(git_dir, timeout)` at `/sce/mutation-cursor.lock`, bounded `try_lock()` polling with a distinct matchable timeout error and RAII release, and its distinction from the separate checkout-identity-creation lock; the isolated Git snapshot service `runtime::git_snapshot::GitSnapshotService` (documented in `mutation-trace-snapshot-service.md`); and `runtime::coordinator`'s protocol-integration pipeline (`RuntimeBoundary`/`CoordinateOutcome`/`CoordinateError`, the load → recover-if-needed → prepare/commit CAS-retry loop, and its own bounded snapshot-failure taint-retry loop) plus the public `coordinate(repository_root, boundary, open_db)` entrypoint that runs the shared `runtime::protected_worktree` prefix (`WorktreeLock` -> external-taint write-ahead fence -> `WorktreeId`, extracted so a second entrypoint cannot drift from it and documented in `mutation-trace-protected-worktree.md`), invokes the caller-supplied `open_db` provider so DB acquisition falls inside that fence, drives the pipeline under one held lock, and clears the marker through `ProtectedWorktree::complete()` only on success; `runtime/tests.rs` covers the public `coordinate()` API end to end against real linked worktrees and a real Agent Trace DB; an inherited external-taint marker is overlaid onto `protocol::database_failure` recovery on the next invocation, against the single captured snapshot and re-injected across a losing recovery CAS; the private `runtime::ref_reconciliation` per-worktree snapshot-ref maintenance pass (documented in `mutation-trace-ref-reconciliation.md`) shares the same `WorktreeLock`; a `pub(crate)` re-export of `coordinate()` beyond `runtime` and harness/command wiring remain future work) - `context/cli/mutation-trace-protected-worktree.md` (the shared safety prefix every mutation-cursor runtime entrypoint runs behind, in `cli/src/services/mutation_trace/runtime/protected_worktree.rs`, extracted from `coordinate()` by the `mutation-scope-runtime-integration` plan so a second entrypoint cannot drift from it: `ProtectedWorktree::acquire(repository_root)` running the safety-critical fixed order resolve `git_dir` → `WorktreeLock` (module-owned 10s `WORKTREE_LOCK_TIMEOUT`) → `ExternalTaintMarker::exists()` → `persist()` (fence armed write-ahead of every fallible step that follows, including DB acquisition) → `get_or_create_checkout_id` as `WorktreeId`; the `worktree_id()` / `inherited_external_taint()` / consuming `complete()` surface, where `complete()` clears the marker under the still-held lock and is the only thing that ever clears it while `Drop` releases only the lock; and the one-variant-per-prefix-step `ProtectedWorktreeError` (`GitDirResolution` | `LockAcquisition` | `ExternalTaintMarker { operation, source }` | `CheckoutIdentity`) each entrypoint maps onto its own error surface — `coordinate()` onto exactly the `CoordinateError` variants that step produced before the extraction) - `context/cli/mutation-trace-ref-reconciliation.md` (the conservative per-worktree snapshot-ref reconciliation pass in `cli/src/services/mutation_trace/runtime/ref_reconciliation.rs`, built by the `mutation-cursor-ref-reconciliation` plan: `reconcile_worktree(repository_root, open_db)` → `pub(super) reconcile_worktree_inner(.., on_lock_contention)`, both returning `ReconciliationOutcome` (`Reconciled(ReconciliationReport { local_required, retained, deleted })` for a pass that ran | `SkippedNoCheckoutIdentity` — an `Ok`, not an `Err` — when no current checkout identity could be derived), a variant-per-fallible-step `ReconcileError` with no `Other`, and the module-owned `RECONCILIATION_LOCK_TIMEOUT`; the two-invariant model — a strictly per-worktree fail-closed local-consistency check via `load_tree_roots(W)` vs. a repository-wide deletion-safety set via `load_all_tree_roots()` so an `A`-owned ref is retained whenever any worktree still durably needs its tree — run entirely under the same `/sce/mutation-cursor.lock` `WorktreeLock` `coordinate()` holds, with the repository-wide read kept coherent by being one SQL statement / one DB snapshot rather than a repository-global lock; deletes only SCE-owned refs via one atomic `git update-ref --no-deref --stdin`, writes no `mutation_trace_*` row, never arms `ExternalTaintMarker`, runs no `git gc`; imperative durability maintenance below the verified Quint protocol; reclaims orphan/unreferenced refs only for the namespace of a checkout id a current worktree still derives — a namespace no current worktree owns (identity-based: a deleted linked worktree, or checkout-id metadata loss followed by `get_or_create_checkout_id` minting a fresh id on a still-present worktree) is beyond every per-worktree pass and left to a recorded future repository-scoped unowned-namespace operation, so the current pass does not bound all orphan-ref growth; `reconcile_worktree` has no `pub(crate)` re-export and no harness/command wiring yet) diff --git a/context/plans/mutation-scope-runtime-integration.md b/context/plans/mutation-scope-runtime-integration.md index b03218c3..96654780 100644 --- a/context/plans/mutation-scope-runtime-integration.md +++ b/context/plans/mutation-scope-runtime-integration.md @@ -372,13 +372,51 @@ which is precisely why there is nothing left for this one to decide. moved to that guard, whose `Drop` never clears). - Context synchronization: synced -- [ ] T02: `Expose a bounded scope read on MutationTraceStore` (status:todo) +- [x] T02: `Expose a bounded scope read on MutationTraceStore` (status:done) - Task ID: T02 - Scope: In — promote the existing private `MutationTraceStore::load_scope` to the smallest public read seam `scope_runtime` needs (one `mutation_trace_scopes` row → `Option`), with a doc comment stating it is a cold-path read that never widens into a projection; tests for an existing scope, a missing scope, and a scope belonging to another worktree. Out — any change to `load_worktree`'s hook-boundary semantics or error contract, any schema/migration change, any new query, any write path. - Dependencies: none - Done when: the read returns the durable `ScopeState` (status, `actor_kind`, `worktree_id`) for a known `ScopeId` and `None` for an unknown one, without consulting `mutation_trace_events` or the worktree row; `load_worktree`'s existing behavior, including its `Err` on a mismatched or missing effective referenced scope, is untouched; the mismatched-worktree case is proven to be the caller's decision, not a store-level rejection. - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::store::`; `git diff cli/migrations/` is empty. - - Context synchronization: pending + - Completed: 2026-09-03 + - Files changed: + - `cli/src/services/mutation_trace/store.rs` + - Result: `MutationTraceStore::load_scope` is now `pub`, relocated from the private + helper block to sit beside the other public reads (after `load_all_tree_roots`, + before `load_worktree_state`). Its signature and body are byte-identical — + `pub fn load_scope(&self, scope_id: &ScopeId) -> Result>`, + one `SELECT_SCOPE_BY_ID_SQL` `query_map` through `scope_row_from_turso` — so + both existing internal callers (`register_scope`, `load_worktree`) are + unaffected and no new query was added. The new doc comment states that it is a + cold-path single-row read that reads one `mutation_trace_scopes` row and + nothing else, never consults `mutation_trace_events`, + `mutation_trace_processed_events`, or the scope's `mutation_trace_worktrees` + row, must not widen into a projection (naming `load_worktree` as the projection + seam), and never adjudicates worktree identity — a scope on another worktree is + returned as-is because comparing the two is the caller's decision. Three tests + were added to the existing inline `#[cfg(test)] mod tests`, using its + `test_db_path` / `insert_worktree` / `insert_scope` fixtures: + `load_scope_returns_the_durable_state_for_a_known_scope` (seeds an event row, + an active-scope row, and a processed-event row alongside the scope, then + asserts the exact `ScopeState`, proving those tables are not consulted), + `load_scope_returns_none_for_an_unknown_scope`, and + `load_scope_returns_a_scope_belonging_to_another_worktree` (a scope on `wt-2` + with no `wt-2` worktree row returns `Ok(Some(..))` carrying `wt-2`, while the + same scope through `load_worktree(wt-1, ..)` still errors — proving the + mismatch is the caller's decision and that `load_worktree`'s contract is + untouched). No schema, migration, or write path was touched. + - Verify results: + - `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::store::` — passed: 86 passed, 0 failed (83 pre-existing plus the 3 new tests); the three new tests were also run in isolation via the `...::tests::load_scope` filter and all passed. + - `git diff cli/migrations/` — empty, confirmed. + - Also run: `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` — passed, clean, with no placeholder consumer added for the newly public method. + - Also run: `./scripts/run-cli-cargo.sh fmt --manifest-path cli/Cargo.toml -- --check` — passed. + - Context impact: local to `cli/src/services/mutation_trace/store.rs`. One method + widened from private to `pub` on `MutationTraceStore`; no signature, schema, + migration, spec, protocol, or behavior change, and `load_worktree`'s error + contract is untouched. Durable context affected: + `context/cli/mutation-trace-store.md` (the new bounded scope read seam and its + deliberate non-adjudication of worktree identity). + - Context synchronization: synced - [ ] T03: `Implement abandon_scope() on the protected runtime path` (status:todo) - Task ID: T03 From efa46a0107b534d0c2e241e384d4f02e92b48b58 Mon Sep 17 00:00:00 2001 From: David Abram Date: Thu, 3 Sep 2026 13:00:42 +0200 Subject: [PATCH 4/6] runtime: Add protected scope abandonment entrypoint Add `abandon_scope` as the mutation-cursor runtime's second protected entrypoint for retiring scopes whose final worktree boundary was not observed, without assigning them snapshot semantics. Reuse the shared protected-worktree fence and CAS retry limit, classify durable scope state before applying `protocol::abandon`, retry conflicts from fresh state, and clear the fence only after settled outcomes while preserving it for recovery-required results and errors. Add focused TempDir-backed coverage and document the runtime, fence, testing, and plan updates. Plan: mutation-scope-runtime-integration (T03) Co-authored-by: SCE --- .../mutation_trace/runtime/coordinator.rs | 2 +- .../services/mutation_trace/runtime/mod.rs | 1 + .../mutation_trace/runtime/scope_runtime.rs | 1109 +++++++++++++++++ context/cli/mutation-trace-external-taint.md | 29 + .../cli/mutation-trace-protected-worktree.md | 10 +- .../cli/mutation-trace-runtime-coordinator.md | 116 +- .../cli/mutation-trace-scope-abandonment.md | 199 +++ context/context-map.md | 1 + context/patterns.md | 2 +- .../mutation-scope-runtime-integration.md | 73 +- 10 files changed, 1478 insertions(+), 64 deletions(-) create mode 100644 cli/src/services/mutation_trace/runtime/scope_runtime.rs create mode 100644 context/cli/mutation-trace-scope-abandonment.md diff --git a/cli/src/services/mutation_trace/runtime/coordinator.rs b/cli/src/services/mutation_trace/runtime/coordinator.rs index 79933c7c..e5f2a960 100644 --- a/cli/src/services/mutation_trace/runtime/coordinator.rs +++ b/cli/src/services/mutation_trace/runtime/coordinator.rs @@ -15,7 +15,7 @@ use super::protected_worktree::{ProtectedWorktree, ProtectedWorktreeError}; pub use super::protected_worktree::ExternalTaintOperation; -const MAX_CAS_RETRY_ATTEMPTS: u32 = 5; +pub(super) const MAX_CAS_RETRY_ATTEMPTS: u32 = 5; #[derive(Clone, Debug)] pub enum RuntimeBoundary { diff --git a/cli/src/services/mutation_trace/runtime/mod.rs b/cli/src/services/mutation_trace/runtime/mod.rs index 2edbf4f5..49e5cbda 100644 --- a/cli/src/services/mutation_trace/runtime/mod.rs +++ b/cli/src/services/mutation_trace/runtime/mod.rs @@ -3,6 +3,7 @@ mod external_taint; mod git_snapshot; mod protected_worktree; mod ref_reconciliation; +mod scope_runtime; mod worktree_lock; #[cfg(test)] diff --git a/cli/src/services/mutation_trace/runtime/scope_runtime.rs b/cli/src/services/mutation_trace/runtime/scope_runtime.rs new file mode 100644 index 00000000..07b0b204 --- /dev/null +++ b/cli/src/services/mutation_trace/runtime/scope_runtime.rs @@ -0,0 +1,1109 @@ +use std::path::Path; + +use crate::services::agent_trace_db::repository::RepositoryAgentTraceDb; +use crate::services::mutation_trace::protocol; +use crate::services::mutation_trace::store::{CasResult, DurableTransition, MutationTraceStore}; +use crate::services::mutation_trace::types::{ScopeId, ScopeStatus, WorktreeId}; + +use super::coordinator::MAX_CAS_RETRY_ATTEMPTS; +use super::protected_worktree::{ + ExternalTaintOperation, ProtectedWorktree, ProtectedWorktreeError, +}; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum AbandonRecoveryReason { + InheritedExternalTaint, + MissingScope, + NeverSeenScope, + MissingWorktreeState, +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum AbandonScopeOutcome { + Abandoned { + worktree_id: WorktreeId, + scope: ScopeId, + revision: u64, + }, + AlreadyTerminal { + worktree_id: WorktreeId, + scope: ScopeId, + status: ScopeStatus, + revision: u64, + }, + RecoveryRequired { + worktree_id: WorktreeId, + scope: ScopeId, + reason: AbandonRecoveryReason, + }, +} + +#[derive(Debug)] +pub enum AbandonScopeError { + LockAcquisition(anyhow::Error), + ExternalTaintMarker { + operation: ExternalTaintOperation, + source: anyhow::Error, + }, + AgentTraceDbUnavailable(anyhow::Error), + WorktreeIdentityMismatch { + scope: ScopeId, + scope_worktree_id: WorktreeId, + invoking_worktree_id: WorktreeId, + }, + RevisionExhausted { + worktree_id: WorktreeId, + revision: u64, + }, + CasConflictExhausted { + attempts: u32, + }, + MarkerClearAfterCompletion { + source: anyhow::Error, + completed: Box, + }, + Other(anyhow::Error), +} + +impl std::fmt::Display for AbandonScopeError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + AbandonScopeError::ExternalTaintMarker { operation, source } => write!( + f, + "External-taint marker {operation:?} operation failed before any \ + mutation-scope state was read: {source}" + ), + AbandonScopeError::AgentTraceDbUnavailable(source) => { + write!(f, "Repository Agent Trace DB is unavailable: {source}") + } + AbandonScopeError::WorktreeIdentityMismatch { + scope, + scope_worktree_id, + invoking_worktree_id, + } => write!( + f, + "Scope {scope:?} belongs to worktree {scope_worktree_id:?} and cannot \ + be abandoned through worktree {invoking_worktree_id:?}" + ), + AbandonScopeError::RevisionExhausted { + worktree_id, + revision, + } => write!( + f, + "Scope on worktree {worktree_id:?} cannot be abandoned because its \ + revision ({revision}) cannot be advanced" + ), + AbandonScopeError::CasConflictExhausted { attempts } => { + write!(f, "Exhausted {attempts} CAS-conflict retry attempts") + } + AbandonScopeError::MarkerClearAfterCompletion { source, .. } => write!( + f, + "Mutation scope settled durably, but clearing the external-taint \ + marker failed: {source}" + ), + AbandonScopeError::LockAcquisition(source) | AbandonScopeError::Other(source) => { + write!(f, "{source}") + } + } + } +} + +impl std::error::Error for AbandonScopeError {} + +pub fn abandon_scope

( + repository_root: &Path, + scope: &ScopeId, + open_db: P, +) -> Result +where + P: FnOnce() -> anyhow::Result, +{ + abandon_scope_inner(repository_root, scope, open_db, |_attempt| {}) +} + +fn abandon_scope_inner( + repository_root: &Path, + scope: &ScopeId, + open_db: P, + after_load: L, +) -> Result +where + P: FnOnce() -> anyhow::Result, + L: FnMut(u32), +{ + let protected = + ProtectedWorktree::acquire(repository_root).map_err(protected_worktree_failure)?; + + if protected.inherited_external_taint() { + return Ok(AbandonScopeOutcome::RecoveryRequired { + worktree_id: protected.worktree_id().clone(), + scope: scope.clone(), + reason: AbandonRecoveryReason::InheritedExternalTaint, + }); + } + + let outcome = abandon_protected(protected.worktree_id(), scope, open_db, after_load)?; + + if matches!(outcome, AbandonScopeOutcome::RecoveryRequired { .. }) { + return Ok(outcome); + } + + match protected.complete() { + Ok(()) => Ok(outcome), + Err(source) => Err(AbandonScopeError::MarkerClearAfterCompletion { + source, + completed: Box::new(outcome), + }), + } +} + +fn abandon_protected( + worktree_id: &WorktreeId, + scope: &ScopeId, + open_db: P, + mut after_load: L, +) -> Result +where + P: FnOnce() -> anyhow::Result, + L: FnMut(u32), +{ + let db = open_db().map_err(AbandonScopeError::AgentTraceDbUnavailable)?; + let store = MutationTraceStore::new(&db); + + for attempt_index in 0..MAX_CAS_RETRY_ATTEMPTS { + let Some(scope_state) = store.load_scope(scope).map_err(AbandonScopeError::Other)? else { + return Ok(recovery_required( + worktree_id, + scope, + AbandonRecoveryReason::MissingScope, + )); + }; + if scope_state.worktree_id != *worktree_id { + return Err(AbandonScopeError::WorktreeIdentityMismatch { + scope: scope.clone(), + scope_worktree_id: scope_state.worktree_id, + invoking_worktree_id: worktree_id.clone(), + }); + } + + let Some(projection) = store + .load_worktree(worktree_id, Some(scope), None) + .map_err(AbandonScopeError::Other)? + else { + return Ok(recovery_required( + worktree_id, + scope, + AbandonRecoveryReason::MissingWorktreeState, + )); + }; + + after_load(attempt_index); + + let state = projection.into_protocol_state(); + let revision = state + .worktrees + .get(worktree_id) + .map(|worktree_state| worktree_state.revision) + .ok_or_else(|| { + AbandonScopeError::Other(anyhow::anyhow!( + "worktree {worktree_id:?} missing from its own loaded projection" + )) + })?; + let status = state + .scopes + .get(scope) + .map(|loaded| loaded.status) + .ok_or_else(|| { + AbandonScopeError::Other(anyhow::anyhow!( + "scope {scope:?} missing from the projection that loaded it as its \ + effective referenced scope" + )) + })?; + + match status { + ScopeStatus::NeverSeen => { + return Ok(recovery_required( + worktree_id, + scope, + AbandonRecoveryReason::NeverSeenScope, + )) + } + ScopeStatus::Closed | ScopeStatus::Abandoned => { + return Ok(AbandonScopeOutcome::AlreadyTerminal { + worktree_id: worktree_id.clone(), + scope: scope.clone(), + status, + revision, + }) + } + ScopeStatus::Active => {} + } + + let abandoned = protocol::abandon(&state, scope); + + let Some(transition) = DurableTransition::between(&state, &abandoned, worktree_id) + .map_err(AbandonScopeError::Other)? + else { + return Err(AbandonScopeError::RevisionExhausted { + worktree_id: worktree_id.clone(), + revision, + }); + }; + + match store + .commit(&transition) + .map_err(AbandonScopeError::Other)? + { + CasResult::Applied => { + let next_revision = abandoned + .worktrees + .get(worktree_id) + .map(|worktree_state| worktree_state.revision) + .expect("the abandoned worktree's state must still be present after abandon"); + return Ok(AbandonScopeOutcome::Abandoned { + worktree_id: worktree_id.clone(), + scope: scope.clone(), + revision: next_revision, + }); + } + CasResult::Conflict => {} + } + } + + Err(AbandonScopeError::CasConflictExhausted { + attempts: MAX_CAS_RETRY_ATTEMPTS, + }) +} + +fn recovery_required( + worktree_id: &WorktreeId, + scope: &ScopeId, + reason: AbandonRecoveryReason, +) -> AbandonScopeOutcome { + AbandonScopeOutcome::RecoveryRequired { + worktree_id: worktree_id.clone(), + scope: scope.clone(), + reason, + } +} + +fn protected_worktree_failure(error: ProtectedWorktreeError) -> AbandonScopeError { + match error { + ProtectedWorktreeError::GitDirResolution(source) + | ProtectedWorktreeError::CheckoutIdentity(source) => AbandonScopeError::Other(source), + ProtectedWorktreeError::LockAcquisition(source) => { + AbandonScopeError::LockAcquisition(anyhow::Error::new(source)) + } + ProtectedWorktreeError::ExternalTaintMarker { operation, source } => { + AbandonScopeError::ExternalTaintMarker { operation, source } + } + } +} + +#[cfg(test)] +mod tests { + use std::cell::Cell; + use std::path::{Path, PathBuf}; + use std::process::Command; + + use super::*; + use crate::services::checkout::{get_or_create_checkout_id, resolve_git_dir}; + use crate::services::mutation_trace::runtime::external_taint::ExternalTaintMarker; + use crate::services::mutation_trace::store::{decode_revision, encode_revision}; + use crate::services::mutation_trace::types::WorktreeState; + + struct TestScopeRepo { + _temp_dir: tempfile::TempDir, + repo_root: PathBuf, + git_dir: PathBuf, + db_path: PathBuf, + } + + impl TestScopeRepo { + fn new(label: &str) -> Self { + let temp_dir = tempfile::Builder::new() + .prefix(&format!("sce-mutation-trace-scope-runtime-{label}-")) + .tempdir() + .expect("test temp directory should be created"); + let repo_root = temp_dir.path().join("repo"); + std::fs::create_dir_all(&repo_root).expect("repository directory should be created"); + run_git(&repo_root, &["init", "--quiet"]); + let git_dir = resolve_git_dir(&repo_root).expect("git dir should resolve"); + let db_path = temp_dir.path().join("agent-trace.db"); + RepositoryAgentTraceDb::new_at(&db_path) + .expect("the repository DB should open with schema"); + Self { + _temp_dir: temp_dir, + repo_root, + git_dir, + db_path, + } + } + + fn open_db(&self) -> anyhow::Result { + RepositoryAgentTraceDb::open_for_hooks_without_migrations_at(&self.db_path) + } + + fn db(&self) -> RepositoryAgentTraceDb { + self.open_db() + .expect("reopening the DB for assertions should succeed") + } + + fn worktree_id(&self) -> WorktreeId { + WorktreeId( + get_or_create_checkout_id(&self.git_dir) + .expect("the checkout identity should resolve"), + ) + } + + fn marker(&self) -> ExternalTaintMarker { + ExternalTaintMarker::new(&self.git_dir) + } + + fn marker_path(&self) -> PathBuf { + self.git_dir.join("sce").join("mutation-cursor-tainted") + } + + fn marker_exists(&self) -> bool { + self.marker() + .exists() + .expect("marker existence should resolve") + } + } + + fn run_git(dir: &Path, args: &[&str]) { + let output = Command::new("git") + .args(args) + .current_dir(dir) + .output() + .expect("git command should spawn"); + assert!( + output.status.success(), + "git {args:?} failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + + fn seed_worktree(db: &RepositoryAgentTraceDb, worktree: &WorktreeId, revision: u64) { + db.execute( + "INSERT INTO mutation_trace_worktrees + (worktree_id, cursor_tree, revision, tainted, failure_kind, needs_rebaseline) + VALUES (?1, 'tree-0', ?2, 0, 'healthy', 0)", + (worktree.0.as_str(), encode_revision(revision).as_slice()), + ) + .expect("worktree insert should succeed"); + } + + fn seed_scope( + db: &RepositoryAgentTraceDb, + scope: &ScopeId, + worktree: &WorktreeId, + status: ScopeStatus, + ) { + db.execute( + "INSERT INTO mutation_trace_scopes (scope_id, worktree_id, actor_kind, status) + VALUES (?1, ?2, 'claude_code', ?3)", + ( + scope.0.as_str(), + worktree.0.as_str(), + crate::services::mutation_trace::store::encode_scope_status(status), + ), + ) + .expect("scope insert should succeed"); + } + + fn bump_revision(db_path: &Path, worktree: &WorktreeId) { + let db = RepositoryAgentTraceDb::open_for_hooks_without_migrations_at(db_path) + .expect("the competing writer should open the DB"); + let current = read_worktree(&db, worktree).expect("the worktree row should exist"); + db.execute( + "UPDATE mutation_trace_worktrees SET revision = ?1 WHERE worktree_id = ?2", + ( + encode_revision(current.revision + 1).as_slice(), + worktree.0.as_str(), + ), + ) + .expect("the competing revision bump should succeed"); + } + + fn set_scope_status(db_path: &Path, scope: &ScopeId, status: ScopeStatus) { + let db = RepositoryAgentTraceDb::open_for_hooks_without_migrations_at(db_path) + .expect("the competing writer should open the DB"); + db.execute( + "UPDATE mutation_trace_scopes SET status = ?1 WHERE scope_id = ?2", + ( + crate::services::mutation_trace::store::encode_scope_status(status), + scope.0.as_str(), + ), + ) + .expect("the competing scope-status write should succeed"); + } + + fn arm_a_scope_status_write_collision(db_path: &Path) { + let db = RepositoryAgentTraceDb::open_for_hooks_without_migrations_at(db_path) + .expect("the competing writer should open the DB"); + db.execute( + "CREATE UNIQUE INDEX idx_test_one_scope_per_status + ON mutation_trace_scopes (status)", + (), + ) + .expect("the collision index should be created"); + } + + fn read_worktree(db: &RepositoryAgentTraceDb, worktree: &WorktreeId) -> Option { + let rows = db + .query_map( + "SELECT cursor_tree, revision, tainted, failure_kind, needs_rebaseline + FROM mutation_trace_worktrees WHERE worktree_id = ?1", + (worktree.0.as_str(),), + |row| { + let cursor_tree = row.get::(0)?; + let revision = row.get::>(1)?; + let tainted = row.get::(2)?; + let failure_kind = row.get::(3)?; + let needs_rebaseline = row.get::(4)?; + Ok(( + cursor_tree, + revision, + tainted, + failure_kind, + needs_rebaseline, + )) + }, + ) + .expect("the worktree read should succeed"); + + rows.into_iter().next().map( + |(cursor_tree, revision, tainted, failure_kind, needs_rebaseline)| WorktreeState { + cursor_tree: crate::services::mutation_trace::types::TreeId(cursor_tree), + revision: decode_revision(&revision).expect("the revision should decode"), + tainted: tainted != 0, + failure_kind: crate::services::mutation_trace::store::decode_failure_kind( + &failure_kind, + ) + .expect("the failure kind should decode"), + needs_rebaseline: needs_rebaseline != 0, + }, + ) + } + + fn read_scope_status(db: &RepositoryAgentTraceDb, scope: &ScopeId) -> Option { + let rows = db + .query_map( + "SELECT status FROM mutation_trace_scopes WHERE scope_id = ?1", + (scope.0.as_str(),), + |row| row.get::(0).map_err(Into::into), + ) + .expect("the scope read should succeed"); + + rows.into_iter().next().map(|status| { + crate::services::mutation_trace::store::decode_scope_status(&status) + .expect("the scope status should decode") + }) + } + + fn count_rows(db: &RepositoryAgentTraceDb, table: &str) -> i64 { + let rows = db + .query_map(&format!("SELECT COUNT(*) FROM {table}"), (), |row| { + row.get::(0).map_err(Into::into) + }) + .expect("the row count should succeed"); + + rows.into_iter().next().unwrap_or_default() + } + + #[test] + fn an_inherited_marker_requires_recovery_without_consulting_the_db_provider() { + let repo = TestScopeRepo::new("inherited-marker"); + let scope = ScopeId("scope-dead".to_string()); + repo.marker() + .persist() + .expect("the earlier invocation's marker should arm"); + + let provider_called = Cell::new(false); + let outcome = abandon_scope(&repo.repo_root, &scope, || { + provider_called.set(true); + Err(anyhow::anyhow!("the DB provider must never be invoked")) + }) + .expect("an inherited marker settles as a successful recovery-required outcome"); + + assert!( + matches!( + outcome, + AbandonScopeOutcome::RecoveryRequired { + reason: AbandonRecoveryReason::InheritedExternalTaint, + .. + } + ), + "expected InheritedExternalTaint, got {outcome:?}" + ); + assert!( + !provider_called.get(), + "the inherited-marker short-circuit must return before the DB provider runs" + ); + assert!( + repo.marker_exists(), + "the inherited marker must stay armed for the next invocation to recover" + ); + } + + #[test] + fn a_missing_scope_row_requires_recovery_and_leaves_the_fence_armed() { + let repo = TestScopeRepo::new("missing-scope"); + let worktree = repo.worktree_id(); + let scope = ScopeId("scope-never-registered".to_string()); + seed_worktree(&repo.db(), &worktree, 3); + + let outcome = abandon_scope(&repo.repo_root, &scope, || repo.open_db()) + .expect("a missing scope row settles as a recovery-required outcome"); + + assert!( + matches!( + outcome, + AbandonScopeOutcome::RecoveryRequired { + reason: AbandonRecoveryReason::MissingScope, + .. + } + ), + "expected MissingScope, got {outcome:?}" + ); + assert!( + repo.marker_exists(), + "a scope whose Start never committed must leave the fence armed" + ); + + let db = repo.db(); + assert_eq!( + read_worktree(&db, &worktree) + .expect("the worktree row should exist") + .revision, + 3, + "a recovery-required outcome must not advance the revision" + ); + assert_eq!(read_scope_status(&db, &scope), None); + } + + #[test] + fn a_never_seen_scope_requires_recovery_and_leaves_the_fence_armed() { + let repo = TestScopeRepo::new("never-seen-scope"); + let worktree = repo.worktree_id(); + let scope = ScopeId("scope-registered-only".to_string()); + { + let db = repo.db(); + seed_worktree(&db, &worktree, 3); + seed_scope(&db, &scope, &worktree, ScopeStatus::NeverSeen); + } + + let outcome = abandon_scope(&repo.repo_root, &scope, || repo.open_db()) + .expect("a NeverSeen scope settles as a recovery-required outcome"); + + assert!( + matches!( + outcome, + AbandonScopeOutcome::RecoveryRequired { + reason: AbandonRecoveryReason::NeverSeenScope, + .. + } + ), + "expected NeverSeenScope, got {outcome:?}" + ); + assert!( + repo.marker_exists(), + "a scope with no observed Start must leave the fence armed" + ); + + let db = repo.db(); + assert_eq!( + read_worktree(&db, &worktree) + .expect("the worktree row should exist") + .revision, + 3, + "a recovery-required outcome must not advance the revision" + ); + assert_eq!( + read_scope_status(&db, &scope), + Some(ScopeStatus::NeverSeen), + "a recovery-required outcome must not change any scope status" + ); + } + + #[test] + fn a_scope_whose_worktree_row_is_missing_requires_recovery() { + let repo = TestScopeRepo::new("missing-worktree-row"); + let worktree = repo.worktree_id(); + let scope = ScopeId("scope-orphan".to_string()); + seed_scope(&repo.db(), &scope, &worktree, ScopeStatus::Active); + + let outcome = abandon_scope(&repo.repo_root, &scope, || repo.open_db()) + .expect("a missing worktree row settles as a recovery-required outcome"); + + assert!( + matches!( + outcome, + AbandonScopeOutcome::RecoveryRequired { + reason: AbandonRecoveryReason::MissingWorktreeState, + .. + } + ), + "expected MissingWorktreeState, got {outcome:?}" + ); + assert!( + repo.marker_exists(), + "a worktree with no durable row must leave the fence armed" + ); + assert_eq!( + read_scope_status(&repo.db(), &scope), + Some(ScopeStatus::Active), + "no scope status may change when there is no worktree to transition" + ); + } + + #[test] + fn abandoning_a_live_scope_writes_only_the_scope_and_worktree_rows() { + let repo = TestScopeRepo::new("active-abandonment"); + let worktree = repo.worktree_id(); + let scope = ScopeId("scope-live".to_string()); + let bystander = ScopeId("scope-bystander".to_string()); + { + let db = repo.db(); + seed_worktree(&db, &worktree, 3); + seed_scope(&db, &scope, &worktree, ScopeStatus::Active); + seed_scope(&db, &bystander, &worktree, ScopeStatus::Active); + } + + let outcome = abandon_scope(&repo.repo_root, &scope, || repo.open_db()) + .expect("abandoning a live scope should succeed"); + + assert_eq!( + outcome, + AbandonScopeOutcome::Abandoned { + worktree_id: worktree.clone(), + scope: scope.clone(), + revision: 4, + } + ); + assert!( + !repo.marker_exists(), + "a settled abandonment must clear the marker it armed" + ); + + let db = repo.db(); + let worktree_state = read_worktree(&db, &worktree).expect("the worktree row should exist"); + assert_eq!(worktree_state.revision, 4, "the revision advances by one"); + assert!(worktree_state.needs_rebaseline); + assert_eq!( + worktree_state.cursor_tree, + crate::services::mutation_trace::types::TreeId("tree-0".to_string()), + "abandonment observes no tree, so the cursor is left where it was" + ); + assert!(!worktree_state.tainted); + assert_eq!( + worktree_state.failure_kind, + crate::services::mutation_trace::types::FailureKind::Healthy + ); + + assert_eq!( + read_scope_status(&db, &scope), + Some(ScopeStatus::Abandoned), + "the named scope is retired" + ); + assert_eq!( + read_scope_status(&db, &bystander), + Some(ScopeStatus::Active), + "abandonment retires only the scope it was given" + ); + + assert_eq!(count_rows(&db, "mutation_trace_events"), 0); + assert_eq!(count_rows(&db, "mutation_trace_event_active_scopes"), 0); + assert_eq!(count_rows(&db, "mutation_trace_processed_events"), 0); + } + + #[test] + fn a_closed_scope_settles_as_a_terminal_no_op() { + let repo = TestScopeRepo::new("closed-scope"); + let worktree = repo.worktree_id(); + let scope = ScopeId("scope-closed".to_string()); + { + let db = repo.db(); + seed_worktree(&db, &worktree, 7); + seed_scope(&db, &scope, &worktree, ScopeStatus::Closed); + } + + let outcome = abandon_scope(&repo.repo_root, &scope, || repo.open_db()) + .expect("an already-closed scope settles successfully"); + + assert_eq!( + outcome, + AbandonScopeOutcome::AlreadyTerminal { + worktree_id: worktree.clone(), + scope: scope.clone(), + status: ScopeStatus::Closed, + revision: 7, + } + ); + assert!( + !repo.marker_exists(), + "a proven-terminal no-op must clear the marker it armed" + ); + + let db = repo.db(); + assert_eq!( + read_worktree(&db, &worktree) + .expect("the worktree row should exist") + .revision, + 7, + "a terminal no-op writes nothing" + ); + assert_eq!(read_scope_status(&db, &scope), Some(ScopeStatus::Closed)); + } + + #[test] + fn an_already_abandoned_scope_settles_as_a_terminal_no_op() { + let repo = TestScopeRepo::new("abandoned-scope"); + let worktree = repo.worktree_id(); + let scope = ScopeId("scope-abandoned".to_string()); + { + let db = repo.db(); + seed_worktree(&db, &worktree, 7); + seed_scope(&db, &scope, &worktree, ScopeStatus::Abandoned); + } + + let outcome = abandon_scope(&repo.repo_root, &scope, || repo.open_db()) + .expect("an already-abandoned scope settles successfully"); + + assert_eq!( + outcome, + AbandonScopeOutcome::AlreadyTerminal { + worktree_id: worktree.clone(), + scope: scope.clone(), + status: ScopeStatus::Abandoned, + revision: 7, + } + ); + assert!( + !repo.marker_exists(), + "a proven-terminal no-op must clear the marker it armed" + ); + assert_eq!( + read_worktree(&repo.db(), &worktree) + .expect("the worktree row should exist") + .revision, + 7, + "a terminal no-op never abandons a scope a second time" + ); + } + + #[test] + fn a_scope_owned_by_another_worktree_is_rejected_without_writing() { + let repo = TestScopeRepo::new("cross-worktree"); + let worktree = repo.worktree_id(); + let other = WorktreeId("wt-other".to_string()); + let scope = ScopeId("scope-elsewhere".to_string()); + { + let db = repo.db(); + seed_worktree(&db, &worktree, 3); + seed_worktree(&db, &other, 11); + seed_scope(&db, &scope, &other, ScopeStatus::Active); + } + + let error = abandon_scope(&repo.repo_root, &scope, || repo.open_db()) + .expect_err("a scope owned by another worktree must be rejected"); + + match &error { + AbandonScopeError::WorktreeIdentityMismatch { + scope: rejected, + scope_worktree_id, + invoking_worktree_id, + } => { + assert_eq!(rejected, &scope); + assert_eq!(scope_worktree_id, &other); + assert_eq!(invoking_worktree_id, &worktree); + } + other => panic!("expected WorktreeIdentityMismatch, got {other:?}"), + } + + let db = repo.db(); + assert_eq!( + read_worktree(&db, &worktree) + .expect("this worktree's row should exist") + .revision, + 3 + ); + assert_eq!( + read_worktree(&db, &other) + .expect("the other worktree's row should exist") + .revision, + 11 + ); + assert_eq!(read_scope_status(&db, &scope), Some(ScopeStatus::Active)); + } + + #[test] + fn a_live_scope_on_an_exhausted_revision_is_a_distinct_error() { + let repo = TestScopeRepo::new("revision-exhausted"); + let worktree = repo.worktree_id(); + let scope = ScopeId("scope-live".to_string()); + { + let db = repo.db(); + seed_worktree(&db, &worktree, u64::MAX); + seed_scope(&db, &scope, &worktree, ScopeStatus::Active); + } + + let error = abandon_scope(&repo.repo_root, &scope, || repo.open_db()) + .expect_err("a worktree at the maximum revision cannot abandon"); + + match &error { + AbandonScopeError::RevisionExhausted { + worktree_id, + revision, + } => { + assert_eq!(worktree_id, &worktree); + assert_eq!(*revision, u64::MAX); + } + other => panic!("expected RevisionExhausted, got {other:?}"), + } + assert_eq!( + read_scope_status(&repo.db(), &scope), + Some(ScopeStatus::Active), + "the scope is still live and still needs retiring" + ); + } + + #[test] + fn a_cas_conflict_recomputes_the_abandonment_from_fresh_state() { + let repo = TestScopeRepo::new("cas-conflict-retry"); + let worktree = repo.worktree_id(); + let scope = ScopeId("scope-live".to_string()); + { + let db = repo.db(); + seed_worktree(&db, &worktree, 3); + seed_scope(&db, &scope, &worktree, ScopeStatus::Active); + } + + let attempts = Cell::new(0u32); + let outcome = abandon_scope_inner( + &repo.repo_root, + &scope, + || repo.open_db(), + |attempt| { + attempts.set(attempts.get() + 1); + if attempt == 0 { + bump_revision(&repo.db_path, &worktree); + } + }, + ) + .expect("a losing CAS attempt should retry and settle"); + + assert_eq!( + attempts.get(), + 2, + "the first attempt loses the CAS and the second recomputes from fresh state" + ); + assert_eq!( + outcome, + AbandonScopeOutcome::Abandoned { + worktree_id: worktree.clone(), + scope: scope.clone(), + revision: 5, + }, + "the retry advances from the competitor's revision, not the stale one" + ); + + let db = repo.db(); + assert_eq!( + read_worktree(&db, &worktree) + .expect("the worktree row should exist") + .revision, + 5 + ); + assert_eq!(read_scope_status(&db, &scope), Some(ScopeStatus::Abandoned)); + } + + #[test] + fn a_cas_conflict_whose_competitor_ended_the_scope_settles_as_a_terminal_no_op() { + let repo = TestScopeRepo::new("cas-conflict-terminal"); + let worktree = repo.worktree_id(); + let scope = ScopeId("scope-live".to_string()); + { + let db = repo.db(); + seed_worktree(&db, &worktree, 3); + seed_scope(&db, &scope, &worktree, ScopeStatus::Active); + } + + let outcome = abandon_scope_inner( + &repo.repo_root, + &scope, + || repo.open_db(), + |attempt| { + if attempt == 0 { + set_scope_status(&repo.db_path, &scope, ScopeStatus::Closed); + bump_revision(&repo.db_path, &worktree); + } + }, + ) + .expect("a competitor that ended the scope should settle the retry"); + + assert_eq!( + outcome, + AbandonScopeOutcome::AlreadyTerminal { + worktree_id: worktree.clone(), + scope: scope.clone(), + status: ScopeStatus::Closed, + revision: 4, + }, + "the retry must settle on the competitor's terminal status, not overwrite it" + ); + + let db = repo.db(); + assert_eq!( + read_scope_status(&db, &scope), + Some(ScopeStatus::Closed), + "a competitor's Close must never be overwritten by a second abandonment" + ); + assert_eq!( + read_worktree(&db, &worktree) + .expect("the worktree row should exist") + .revision, + 4 + ); + } + + #[test] + fn a_persistence_failure_rolls_back_the_whole_transition_and_leaves_the_fence_armed() { + let repo = TestScopeRepo::new("persistence-failure"); + let worktree = repo.worktree_id(); + let scope = ScopeId("scope-live".to_string()); + let bystander = ScopeId("scope-already-abandoned".to_string()); + { + let db = repo.db(); + seed_worktree(&db, &worktree, 3); + seed_scope(&db, &scope, &worktree, ScopeStatus::Active); + seed_scope(&db, &bystander, &worktree, ScopeStatus::Abandoned); + } + + let error = abandon_scope_inner( + &repo.repo_root, + &scope, + || repo.open_db(), + |attempt| { + if attempt == 0 { + arm_a_scope_status_write_collision(&repo.db_path); + } + }, + ) + .expect_err("a failing durable write must fail the abandonment"); + + assert!( + matches!(error, AbandonScopeError::Other(_)), + "a persistence failure surfaces as the store-error variant, got {error:?}" + ); + assert!( + format!("{error}").contains("mutation_trace_scopes"), + "the failure must be the transaction's scope-status write — the statement that \ + runs after the worktree CAS guard — not an earlier read or a settled conflict; \ + got {error}" + ); + assert!( + repo.marker_exists(), + "a persistence failure after the fence is armed must leave it armed" + ); + + let db = repo.db(); + let worktree_state = read_worktree(&db, &worktree).expect("the worktree row should exist"); + assert_eq!( + worktree_state.revision, 3, + "the transaction's worktree CAS guard ran before the failing statement, so a \ + partially applied revision here would mean the batch is not atomic" + ); + assert!( + !worktree_state.needs_rebaseline, + "the rolled-back transition must not leave the worktree needing rebaseline" + ); + assert_eq!( + worktree_state.cursor_tree, + crate::services::mutation_trace::types::TreeId("tree-0".to_string()) + ); + assert_eq!( + read_scope_status(&db, &scope), + Some(ScopeStatus::Active), + "the target scope must not be left Abandoned by a failed transition" + ); + assert_eq!( + read_scope_status(&db, &bystander), + Some(ScopeStatus::Abandoned) + ); + + assert_eq!(count_rows(&db, "mutation_trace_events"), 0); + assert_eq!(count_rows(&db, "mutation_trace_event_active_scopes"), 0); + assert_eq!(count_rows(&db, "mutation_trace_processed_events"), 0); + } + + #[test] + fn a_db_provider_failure_leaves_the_fence_armed() { + let repo = TestScopeRepo::new("db-provider-failure"); + let scope = ScopeId("scope-live".to_string()); + + let error = abandon_scope(&repo.repo_root, &scope, || { + Err(anyhow::anyhow!("simulated Agent Trace DB open failure")) + }) + .expect_err("a DB provider that returns Err must fail abandon_scope()"); + + assert!( + matches!(error, AbandonScopeError::AgentTraceDbUnavailable(_)), + "expected AgentTraceDbUnavailable, got {error:?}" + ); + assert!( + repo.marker_exists(), + "a DB-provider failure after arming must leave the fence armed" + ); + } + + #[test] + fn a_marker_clear_failure_carries_the_already_settled_outcome() { + let repo = TestScopeRepo::new("marker-clear-failure"); + let worktree = repo.worktree_id(); + let scope = ScopeId("scope-live".to_string()); + { + let db = repo.db(); + seed_worktree(&db, &worktree, 3); + seed_scope(&db, &scope, &worktree, ScopeStatus::Active); + } + + let marker_path = repo.marker_path(); + let error = abandon_scope(&repo.repo_root, &scope, || { + std::fs::remove_file(&marker_path) + .expect("the armed marker file should be present mid-invocation"); + std::fs::create_dir_all(marker_path.join("nested")) + .expect("planting a non-empty directory at the marker path should succeed"); + repo.open_db() + }) + .expect_err("clearing a marker that is now a non-empty directory must fail"); + + let completed = match error { + AbandonScopeError::MarkerClearAfterCompletion { completed, .. } => completed, + other => panic!("expected MarkerClearAfterCompletion, got {other:?}"), + }; + assert_eq!( + *completed, + AbandonScopeOutcome::Abandoned { + worktree_id: worktree.clone(), + scope: scope.clone(), + revision: 4, + }, + "the error must carry the outcome that already settled durably" + ); + assert!( + repo.marker_exists(), + "the marker stays logically armed after a post-completion clear failure" + ); + + let db = repo.db(); + let worktree_state = read_worktree(&db, &worktree).expect("the worktree row should exist"); + assert_eq!(worktree_state.revision, 4); + assert!(worktree_state.needs_rebaseline); + assert_eq!(read_scope_status(&db, &scope), Some(ScopeStatus::Abandoned)); + + std::fs::remove_dir_all(&marker_path) + .expect("removing the planted directory should succeed"); + } +} diff --git a/context/cli/mutation-trace-external-taint.md b/context/cli/mutation-trace-external-taint.md index a6f6c91c..25e68763 100644 --- a/context/cli/mutation-trace-external-taint.md +++ b/context/cli/mutation-trace-external-taint.md @@ -177,6 +177,33 @@ proves an attributable `Advance` commits durably, the returned (`worktree_id` / `revision` / `observed_tree`, and `mutation_event.is_some()`), and a later invocation still recovers off the still-armed marker. +## Abandonment-path completion + +`abandon_scope()` +([`mutation-trace-scope-abandonment.md`](mutation-trace-scope-abandonment.md)) +is the second entrypoint behind the same prefix, so it inherits the same fence +ordering. What differs is which of its settled results count as a proven +completion: + +| Result | Marker | +| --- | --- | +| `Abandoned` (durable transition landed) | **cleared** | +| `AlreadyTerminal` (scope proved `Closed`/`Abandoned`) | **cleared** | +| `RecoveryRequired` (inherited marker, missing/`NeverSeen` scope, missing worktree row) | **stays armed** | +| any error (DB provider, identity mismatch, revision exhaustion, CAS exhaustion) | **stays armed** | + +A `RecoveryRequired` outcome is a *success* that deliberately declines to clear +the fence: nothing durable was decided, so the next `coordinate()` must promote +the marker to `external_taint` and recover conservatively. The inherited-marker +case short-circuits before `open_db()` is ever invoked — the fence an earlier +invocation armed already covers this interval, so there is nothing left for this +one to decide. + +`AbandonScopeError::MarkerClearAfterCompletion { source, completed }` mirrors +`MarkerClearAfterCommit`: the abandonment already settled durably, so the +settled `AbandonScopeOutcome` rides along in `completed` rather than being lost, +and the marker stays logically armed. + ## On-disk layout addition ```text @@ -187,5 +214,7 @@ and a later invocation still recovers off the still-armed marker. See also: [`mutation-trace-runtime-coordinator.md`](mutation-trace-runtime-coordinator.md), [`mutation-trace-protected-worktree.md`](mutation-trace-protected-worktree.md) (the shared prefix that owns arming and clearing this fence), +[`mutation-trace-scope-abandonment.md`](mutation-trace-scope-abandonment.md) +(the second entrypoint behind that prefix), [`mutation-trace-protocol.md`](mutation-trace-protocol.md), [`checkout-identity.md`](checkout-identity.md). diff --git a/context/cli/mutation-trace-protected-worktree.md b/context/cli/mutation-trace-protected-worktree.md index 7911cfa5..5e3ab9dc 100644 --- a/context/cli/mutation-trace-protected-worktree.md +++ b/context/cli/mutation-trace-protected-worktree.md @@ -8,7 +8,11 @@ safety-critical, and one owner is the mechanism that keeps it single-sourced. Extracted by the `mutation-scope-runtime-integration` plan (`context/plans/mutation-scope-runtime-integration.md`) ahead of the -`abandon_scope()` entrypoint that will share it. +`abandon_scope()` entrypoint +([`mutation-trace-scope-abandonment.md`](mutation-trace-scope-abandonment.md)), +which now shares it: `coordinate()` observes a boundary and snapshots, while +`abandon_scope()` observes nothing and snapshots nothing, and this prefix is the +only piece they hold in common. ## The fixed order @@ -66,7 +70,8 @@ map it onto its own error surface without losing which safety step failed: `ExternalTaintOperation` lives here, beside the fence step that produces it, and is re-exported by `coordinator.rs` so `CoordinateError::ExternalTaintMarker` -keeps naming it. `coordinate()` maps `GitDirResolution` and `CheckoutIdentity` +keeps naming it; `AbandonScopeError::ExternalTaintMarker` carries the same type. +`coordinate()` maps `GitDirResolution` and `CheckoutIdentity` onto `CoordinateError::Other`, `LockAcquisition` onto `CoordinateError::LockAcquisition`, and the fence variant onto `CoordinateError::ExternalTaintMarker` with the same `operation` — the exact @@ -89,5 +94,6 @@ pass unchanged through the guard, which is what proves `coordinate()`'s externally observable ordering and error semantics survived the extraction. See also: [`mutation-trace-runtime-coordinator.md`](mutation-trace-runtime-coordinator.md), +[`mutation-trace-scope-abandonment.md`](mutation-trace-scope-abandonment.md), [`mutation-trace-external-taint.md`](mutation-trace-external-taint.md), [`checkout-identity.md`](checkout-identity.md). diff --git a/context/cli/mutation-trace-runtime-coordinator.md b/context/cli/mutation-trace-runtime-coordinator.md index d7280de7..ae7af924 100644 --- a/context/cli/mutation-trace-runtime-coordinator.md +++ b/context/cli/mutation-trace-runtime-coordinator.md @@ -8,13 +8,12 @@ Git worktree, built by the `mutation-cursor-runtime-coordinator` plan `cli/src/services/mutation_trace/runtime/` is a private submodule (`pub(crate) mod runtime;` in `mutation_trace/mod.rs`), registered under the -same `#[allow(dead_code)]` precedent as the rest of `mutation_trace`. -Every submodule — `coordinator`, `protected_worktree`, `ref_reconciliation`, -and the rest — is declared privately in `runtime/mod.rs`, so `coordinate()` and -`reconcile_worktree` are reachable only from within `runtime` itself (its own -tests) for now; a `pub(crate)` re-export is deferred until a harness adapter -needs it. Nothing under `runtime/` is wired into any hook, command, or -`diff_traces` insertion yet. +same `#[allow(dead_code)]` precedent as the rest of `mutation_trace`. Every +submodule is declared privately in `runtime/mod.rs`, so `coordinate()`, +`abandon_scope()`, and `reconcile_worktree` are reachable only from within +`runtime` itself (its own tests) for now; a `pub(crate)` re-export is deferred +until a harness adapter needs it, and nothing under `runtime/` is wired into any +hook, command, or `diff_traces` insertion yet. `runtime` depends on `protocol`/`store`/`types` and on `services::checkout`, never the reverse — this is a structural module boundary, not merely a @@ -37,31 +36,32 @@ documented convention. - `cli/src/services/mutation_trace/runtime/git_snapshot.rs` — the isolated Git snapshot and ref-pinning service (`GitSnapshotService`: `new`/`capture_tree`/`pin_tree`/`diff_trees`, plus the callerless - worktree-scoped `list_pins` pin inventory — - `Result, PinInventoryError>` — and conditional-atomic + worktree-scoped `list_pins` pin inventory and conditional-atomic `delete_pins` batch deletion). It writes tree/blob objects into the repository's normal, shared object database and protects durable trees with create-only, **direct** `refs/sce/mutation-cursor//` - refs; a symbolic ref inside that namespace is malformed and rejected, and - `delete_pins` uses no-dereference semantics so an inventory→delete ref-type - race cannot escape the inventoried namespace. Full contract in + refs. Full contract, including the namespace's symbolic-ref rejection and + `delete_pins`'s no-dereference semantics, in [`mutation-trace-snapshot-service.md`](mutation-trace-snapshot-service.md). - `cli/src/services/mutation_trace/runtime/ref_reconciliation.rs` — the conservative per-worktree snapshot-ref maintenance pass — `reconcile_worktree` / `pub(super) reconcile_worktree_inner` return `Result` (`ReconciliationOutcome` = `Reconciled(ReconciliationReport)` - | `SkippedNoCheckoutIdentity`). Under the worktree's `WorktreeLock` it deletes - only pins whose tree is a durable root of **no** worktree, fails closed if any - local root lacks a pin, and writes no `mutation_trace_*` row or taint marker - (only the namespace of a checkout id a current worktree still derives — a - namespace no current worktree owns, via a deleted worktree or checkout-id - metadata loss/recreation, is future repository-scoped work). Full contract in - [`mutation-trace-ref-reconciliation.md`](mutation-trace-ref-reconciliation.md). + ReconcileError>`. Under the worktree's `WorktreeLock` it deletes only pins + whose tree is a durable root of **no** worktree, fails closed if any local + root lacks a pin, and writes no `mutation_trace_*` row or taint marker. Full + contract, including the outcome variants and the namespaces it cannot reach, + in [`mutation-trace-ref-reconciliation.md`](mutation-trace-ref-reconciliation.md). - `cli/src/services/mutation_trace/runtime/protected_worktree.rs` — the shared safety prefix every runtime entrypoint runs behind (`ProtectedWorktree`: resolve `git_dir` → `WorktreeLock` → external-taint fence → `WorktreeId`, plus an explicit `complete()` as the only thing that clears the marker). Full contract in [`mutation-trace-protected-worktree.md`](mutation-trace-protected-worktree.md). +- `cli/src/services/mutation_trace/runtime/scope_runtime.rs` — the second + entrypoint behind that prefix, `abandon_scope(repository_root, scope, open_db) + -> Result`: it retires a scope whose + final boundary was never observed, captures **no** Git snapshot, and reuses + this module's `MAX_CAS_RETRY_ATTEMPTS`. Full contract in + [`mutation-trace-scope-abandonment.md`](mutation-trace-scope-abandonment.md). - `cli/src/services/mutation_trace/runtime/coordinator.rs` — the composition point that drives `protocol.rs`/`store.rs`/`git_snapshot.rs` together. Its `SnapshotCapture` trait (`capture(&self) -> Result`, `pin(&self, @@ -88,26 +88,25 @@ documented convention. `repository_root → git_dir → WorktreeLock → checkout ID → WorktreeId`; the DB is not on that chain. (`coordinate()` is a one-line delegation to the `pub(super) coordinate_inner(.., on_lock_contention, after_load, after_recovery)` - test seam — reachable from `runtime::tests`, invisible outside `runtime`; - production passes a no-op for all three. `after_load: impl FnMut(u32)` fires + test seam — reachable from `runtime::tests`, invisible outside `runtime`, + production passing a no-op for all three. `after_load: impl FnMut(u32)` fires each CAS attempt after `load_worktree` and before the real `store.commit` CAS; - the reconciliation pin→CAS lock-race regression uses it to pause a real - `coordinate()` between `pin` and CAS. No production behavior change.) A + see **Testing boundary** below. No production behavior change.) A `WorktreeLock` acquisition failure surfaces as - `CoordinateError::LockAcquisition`; pre-commit - marker-I/O and DB-provider failures have their own fail-closed variants, and a - post-commit `marker.clear()` failure surfaces as + `CoordinateError::LockAcquisition`; pre-commit marker-I/O and DB-provider + failures have their own fail-closed variants, and a post-commit + `marker.clear()` failure surfaces as `CoordinateError::MarkerClearAfterCommit { source, committed }` — the boundary did commit, so the durable `CoordinateOutcome` (with any `MutationEvent`) rides along in `committed` rather than being lost, and the marker stays armed. See [`mutation-trace-external-taint.md`](mutation-trace-external-taint.md) for the - fence ordering, the safety invariant, and the `CoordinateError` variants it - adds. The pipeline does, per invocation: capture and pin - exactly one Git snapshot; on failure, run a bounded taint-retry loop instead - (below) and return without touching the rest of the pipeline; on success, - idempotently materialize the worktree row and, for hook boundaries, the - scope row; then loop (bounded, `MAX_CAS_RETRY_ATTEMPTS = 5`, no backoff): - load durable state fresh, recover first if the worktree is tainted, needs + fence ordering, the safety invariant, and the variants it adds to both + entrypoints. The pipeline does, per invocation: capture and pin exactly one Git + snapshot; on failure, run a bounded taint-retry loop instead (below) and + return without touching the rest of the pipeline; on success, idempotently + materialize the worktree row and, for hook boundaries, the scope row; then + loop (bounded, `MAX_CAS_RETRY_ATTEMPTS = 5`, no backoff, shared with + `scope_runtime`): load durable state fresh, recover first if tainted, needs rebaseline, or inherited an external-taint marker (overlaid as `database_failure`; its CAS commit reuses the one captured tree), then `prepare`/`commit` the triggering boundary against that state (a second CAS @@ -116,11 +115,11 @@ documented convention. replayed attempt) is a successful return, not an error. A capture or pin failure is handled by its own bounded taint-retry loop: a - fresh `load_worktree` on every iteration, always evaluated after the - failure, never before it — so a worktree another caller materializes - concurrently while this invocation's own capture is still in flight is - still found and correctly tainted. No durable worktree row on that fresh - read means no taint to record (`persisted_taint: false`, no write); an + fresh `load_worktree` on every iteration, always evaluated after the failure, + never before it — so a worktree another caller materializes concurrently while + this invocation's own capture is still in flight is still found and correctly + tainted. No durable worktree row on that fresh read means no taint to record + (`persisted_taint: false`, no write); an already-tainted no-op reads back the current flag instead of assuming success; otherwise the loop commits the taint transition and retries on `Conflict`, reporting `persisted_taint: false` only once every bounded @@ -128,8 +127,8 @@ documented convention. The runtime lock guards the whole critical section — fence arming/clearing, snapshot capture, worktree/scope materialization, recovery, and the CAS retry -loop — and is held on every `coordinate()` call, unlike the -checkout-identity-creation lock. +loop — and is held on every `coordinate()` and `abandon_scope()` call, unlike +the checkout-identity-creation lock. `ref_reconciliation::reconcile_worktree` acquires the **same** lock file (bounded by its own `RECONCILIATION_LOCK_TIMEOUT`) before it inventories pins, reads durable roots, or deletes anything. @@ -144,7 +143,7 @@ locks guarding separate invariants, not one lock reused for two purposes: | | Path | Guards | Held by | Blocking behavior | | --- | --- | --- | --- | --- | | Checkout-identity lock | `/sce/checkout-id.lock` | "this checkout has at most one durable identity" | any caller of `get_or_create_checkout_id` | blocks indefinitely, no timeout — the critical section is a handful of filesystem syscalls | -| Mutation-cursor runtime lock | `/sce/mutation-cursor.lock` | the coordinator's entire runtime critical section | only the coordinator, on every invocation | bounded polling with a caller-supplied timeout — a stuck holder must not deadlock every future hook invocation | +| Mutation-cursor runtime lock | `/sce/mutation-cursor.lock` | every runtime entrypoint's critical section | every `runtime` entrypoint, on every invocation | bounded polling with a caller-supplied timeout — a stuck holder must not deadlock every future hook invocation | On-disk layout so far: @@ -170,12 +169,13 @@ across distinct worktree paths, timing out with a distinct matchable error while the lock is still held, and a leftover lock file with no active OS lock never blocking a fresh acquirer — each test uses a unique `std::env::temp_dir()` path, following the same filesystem-touching -inline-unit-test precedent as `cli/src/services/checkout/mod.rs` and -`cli/src/services/mutation_trace/store.rs` (see `context/patterns.md`). +inline-unit-test precedent as `cli/src/services/checkout/mod.rs` (see +[`../patterns.md`](../patterns.md)). -`ProtectedWorktree`'s inline tests use RAII `tempfile::TempDir` fixtures over -real `git init` repositories; coverage in -[`mutation-trace-protected-worktree.md`](mutation-trace-protected-worktree.md#testing-boundary). +`ProtectedWorktree`'s and `scope_runtime`'s inline tests use RAII +`tempfile::TempDir` fixtures over real `git init` repositories; coverage in +[`mutation-trace-protected-worktree.md`](mutation-trace-protected-worktree.md#testing-boundary) +and [`mutation-trace-scope-abandonment.md`](mutation-trace-scope-abandonment.md#testing-boundary). `GitSnapshotService`'s inline `#[cfg(test)] mod tests` in `git_snapshot.rs` uses the same precedent, extended to real per-test `git init` repositories; coverage @@ -229,22 +229,22 @@ entrypoint. ## Status The `ProtectedWorktree` prefix, lock, snapshot service, protocol-integration -pipeline, and the public `coordinate()` entrypoint (prefix → DB provider → -pipeline → `complete()` on success) are all implemented, with `runtime/tests.rs` -covering the public API end to end; an inherited external-taint marker is -overlaid onto `database_failure` recovery on the next invocation. A `pub(crate)` -re-export of `coordinate()`, a second `abandon_scope()` entrypoint sharing the -same prefix, and harness/command wiring remain future work -(`mutation-cursor-external-taint`, `mutation-cursor-runtime-coordinator`, -`mutation-scope-runtime-integration`). +pipeline, the public `coordinate()` entrypoint (prefix → DB provider → pipeline +→ `complete()` on success), and the second `abandon_scope()` entrypoint sharing +that prefix are all implemented, with `runtime/tests.rs` covering `coordinate()` +end to end; an inherited external-taint marker is overlaid onto +`database_failure` recovery on the next invocation. Real-Git abandonment +regressions, a `pub(crate)` re-export of either entrypoint, and harness/command +wiring remain future work (`mutation-cursor-external-taint`, +`mutation-cursor-runtime-coordinator`, `mutation-scope-runtime-integration`). See also: [`mutation-trace-ref-reconciliation.md`](mutation-trace-ref-reconciliation.md) (the per-worktree snapshot-ref maintenance pass under the same `WorktreeLock`), [`mutation-trace-snapshot-service.md`](mutation-trace-snapshot-service.md) (the `GitSnapshotService` capture/pin/diff/inventory/delete contract), -[`mutation-trace-protocol.md`](mutation-trace-protocol.md), -[`mutation-trace-store.md`](mutation-trace-store.md), -[`mutation-trace-external-taint.md`](mutation-trace-external-taint.md) +[`mutation-trace-scope-abandonment.md`](mutation-trace-scope-abandonment.md) +(the unobserved-boundary entrypoint), [`mutation-trace-protocol.md`](mutation-trace-protocol.md), +[`mutation-trace-store.md`](mutation-trace-store.md), [`mutation-trace-external-taint.md`](mutation-trace-external-taint.md) (the `/sce/mutation-cursor-tainted` write-ahead fence), [`mutation-trace-protected-worktree.md`](mutation-trace-protected-worktree.md) (the shared prefix that arms it), [`checkout-identity.md`](checkout-identity.md). diff --git a/context/cli/mutation-trace-scope-abandonment.md b/context/cli/mutation-trace-scope-abandonment.md new file mode 100644 index 00000000..211025ea --- /dev/null +++ b/context/cli/mutation-trace-scope-abandonment.md @@ -0,0 +1,199 @@ +# Mutation-scope abandonment (`runtime::scope_runtime`) + +The mutation-cursor runtime's second protected entrypoint, in +`cli/src/services/mutation_trace/runtime/scope_runtime.rs`. It retires a +mutation scope whose execution ended without a trustworthy final worktree +boundary — a dead agent process leaves no `Close` behind. + +A dead execution can leave its scope `Active` indefinitely. This is unsafe +because a later `Start` observes the worktree before activating its successor — +`commit` computes `active_scopes`/`attribution` against the state as it existed +*before* the same call's own scope-lifecycle transition — so changes made after +the dead execution may be incorrectly classified as `AiExclusive` to the stale +scope. Once another scope starts, subsequent overlapping intervals become +`AiContended` against the zombie: + +```text +scope A Active → A dies without Close → unobserved edit → Start(B) + + at Start(B): live = { A } → that interval may become AiExclusive(A) + after Start(B): live = { A, B } → later overlapping intervals: AiContended +``` + +Built by the `mutation-scope-runtime-integration` plan +(`context/plans/mutation-scope-runtime-integration.md`). It is the first +production call site for `protocol::abandon` +([`mutation-trace-protocol.md`](mutation-trace-protocol.md)). + +## Observed vs. unobserved boundaries + +`coordinate()` and `abandon_scope()` share the `ProtectedWorktree` prefix +([`mutation-trace-protected-worktree.md`](mutation-trace-protected-worktree.md)) +and deliberately diverge below it: + +| | `coordinate()` | `abandon_scope()` | +| --- | --- | --- | +| Boundary | observed (`Start`/`Advance`/`Close`/`Flush`) | **never observed** | +| Git snapshot / pin | always exactly one | **none** | +| Cursor tree | advanced to the observed tree | left untouched | +| Mutation evidence | may emit one `MutationEvent` | never | + +Abandonment means nobody saw where the worktree ended up. Capturing a snapshot +here would silently give it `Close`'s observation semantics, so the module names +none of `GitSnapshotService`, `SnapshotCapture`, `capture_tree`, `pin_tree`, +`diff_trees`, `reconcile_worktree`, `initialize_worktree`, or `register_scope`: +it reads and transitions only already-durable state. Abandonment is therefore +**not** a `RuntimeBoundary` variant and needs no change to +`spec/mutation_cursor.qnt`. + +## Surface + +```text +abandon_scope(repository_root, scope: &ScopeId, open_db) + open_db: impl FnOnce() -> anyhow::Result + -> Result +``` + +The DB-provider shape matches `coordinate()`'s, so DB acquisition falls inside +the same fence. No caller ever supplies a `WorktreeId`; it is always derived +from this checkout by the guard. + +`AbandonScopeOutcome` is one of: + +- `Abandoned { worktree_id, scope, revision }` — the scope moved to + `Abandoned`, the worktree revision advanced by exactly one, and + `needs_rebaseline` was set. `cursor_tree`, `tainted`, and `failure_kind` are + untouched, and no `mutation_trace_events`, + `mutation_trace_event_active_scopes`, or `mutation_trace_processed_events` + row is written. +- `AlreadyTerminal { worktree_id, scope, status, revision }` — the scope was + already `Closed` or `Abandoned`. A terminal scope can never be reactivated or + abandoned again, so this is a success with nothing written; `revision` is the + current, unchanged revision. +- `RecoveryRequired { worktree_id, scope, reason }` — nothing was written and + the fence stays armed, so the next boundary recovers conservatively. + +## Classification before transition + +`protocol::abandon` is a guarded no-op for `NeverSeen`, `Closed`, `Abandoned`, +an unknown scope, a missing `WorktreeState`, and `revision == u64::MAX` alike, +returning an unchanged state in every case — the runtime cannot recover *which* +of those happened by diffing its output. So this path classifies the target's +durable state first: + +```mermaid +flowchart TD + A["ProtectedWorktree::acquire"] --> B{"marker inherited?"} + B -- yes --> R1["RecoveryRequired: InheritedExternalTaint
DB provider never invoked"] + B -- no --> C["open_db()"] + C --> D["store.load_scope(scope)"] + D -- "no row" --> R2["RecoveryRequired: MissingScope"] + D -- "other worktree_id" --> E1["Err WorktreeIdentityMismatch"] + D -- "this worktree" --> F["store.load_worktree(wt, Some(scope), None)"] + F -- "no worktree row" --> R3["RecoveryRequired: MissingWorktreeState"] + F -- NeverSeen --> R4["RecoveryRequired: NeverSeenScope"] + F -- "Closed / Abandoned" --> T["AlreadyTerminal"] + F -- Active --> G["protocol::abandon → DurableTransition → CAS"] + G -- "no transition" --> E2["Err RevisionExhausted"] + G -- Conflict --> F + G -- Applied --> S["Abandoned"] +``` + +`load_scope` ([`mutation-trace-store.md`](mutation-trace-store.md)) comes first +precisely because the projection seam `load_worktree` treats both of that read's +outcomes as errors, and neither is one here: a scope with no row is a recovery +case, and a scope on another worktree is this path's own typed rejection. + +A `DurableTransition::between` that yields no transition for a scope already +proved `Active`, on a worktree present in the projection whose `external_taint` +is always empty, can only mean an unadvanceable revision — hence +`RevisionExhausted` rather than a silent success. + +`CasResult::Conflict` re-enters the loop and re-classifies from scratch, bounded +by the coordinator's own `MAX_CAS_RETRY_ATTEMPTS` (5, no backoff — one shared +constant, not a second one). A competitor that closed the scope meanwhile +therefore settles as `AlreadyTerminal`, never as a second abandonment +overwriting it. + +## Why a missing or `NeverSeen` target forces strong recovery + +`MissingScope` and `NeverSeenScope` leave the external-taint marker armed, so +the next `coordinate()` performs *inherited-taint* recovery — and +`protocol::recover` abandons **every** live scope on a worktree recovering from +external taint, not only the one named here. + +That is deliberate. A missing row means the scope's `Start` never committed +while its execution may well have run and edited files; a `NeverSeen` row means +the identity exists but no accepted `Start` was ever observed. Neither proves +the execution mutated nothing, and the runtime cannot bound what happened inside +that interval, so it cannot let any scope keep an exclusivity claim spanning it. + +The cost is a false negative: legitimately live scopes on the same worktree can +be abandoned by a recovery they did nothing to cause, and their in-flight +evidence is discarded. That is accepted because the alternative is a false +positive — attributing an interval exclusively to a scope while an unobserved +execution may have been mutating the same worktree. **Attribution safety +outranks preserving potentially valid evidence.** + +This does not contradict the normal `Abandoned` outcome: that one sets only +`needs_rebaseline`, whose recovery preserves live scopes by design. + +## Fence completion semantics + +The marker is cleared only for `Abandoned` and `AlreadyTerminal`. Every +`RecoveryRequired` and every error leaves it armed — see +[`mutation-trace-external-taint.md`](mutation-trace-external-taint.md#abandonment-path-completion). + +## Testing boundary + +Inline `#[cfg(test)] mod tests` uses an RAII `tempfile::TempDir` fixture holding +a real `git init` repository and a real temp-file `RepositoryAgentTraceDb` (see +[`../patterns.md`](../patterns.md)). Coverage: the inherited-marker +short-circuit proving a flag-setting DB provider is never invoked; missing, +`NeverSeen`, and missing-worktree-row recovery with the marker still on disk and +no row changed; a live abandonment asserting every durable field and all three +absent row kinds, plus an unrelated live scope left `Active`; separate `Closed` +and `Abandoned` terminal no-ops; cross-worktree rejection leaving both worktree +revisions and the scope status untouched; `u64::MAX` revision exhaustion with +the scope still live; a CAS conflict recomputing from the competitor's revision; +a CAS conflict whose competitor closed the scope settling as `AlreadyTerminal`; +a DB-provider `Err` leaving the fence armed; a persistence failure rolling the +whole transition back (below); and a `clear()` failure returning +`MarkerClearAfterCompletion` whose carried outcome matches the durable state. + +A private `abandon_scope_inner(.., after_load)` seam (mirroring +`coordinate_inner`) fires once per CAS attempt after the projection loads, so a +test can land a competing write inside the CAS window. Production passes a +no-op. + +### Proving the transition is all-or-nothing + +`MutationTraceStore::commit` runs the worktree CAS `UPDATE` as the transaction's +guard **before** the scope-status `UPDATE`, so a failure in that later statement +is the case where a partially applied worktree would show. The regression forces +exactly that: with the target `active` beside an already-`abandoned` bystander, +`after_load` creates a `UNIQUE` index on `mutation_trace_scopes(status)` — one +the seeded rows already satisfy, and that only the `abandoned` status the +transition is about to write violates. Nothing about the worktree row changes, so +the CAS guard still matches its expected revision and the commit reaches the +failing statement rather than settling as a conflict. + +After the resulting `Err`, the worktree is still at its original revision with +`needs_rebaseline` false and its cursor untouched, the target scope is still +`Active`, no `mutation_trace_events` / +`mutation_trace_event_active_scopes` / `mutation_trace_processed_events` row +exists, and the external-taint marker is still armed — the general rule that +**every** runtime error after the fence is armed leaves it armed. + +## Status + +The entrypoint, its outcome/error types, and its unit coverage exist. Real-Git +cross-worktree and successor-scope regressions, the `pub(crate)` re-export out +of `runtime`, and the harness-adapter contract document remain future work in +the same plan; no harness, hook, or command calls this yet. + +See also: [`mutation-trace-runtime-coordinator.md`](mutation-trace-runtime-coordinator.md), +[`mutation-trace-protected-worktree.md`](mutation-trace-protected-worktree.md), +[`mutation-trace-external-taint.md`](mutation-trace-external-taint.md), +[`mutation-trace-protocol.md`](mutation-trace-protocol.md), +[`mutation-trace-store.md`](mutation-trace-store.md). diff --git a/context/context-map.md b/context/context-map.md index f6e27ff6..806b2f53 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -29,6 +29,7 @@ Feature/domain context: - `context/cli/mutation-trace-store.md` (durable persistence for the mutation-cursor protocol in `cli/src/services/mutation_trace/store.rs`, built by the `mutation-cursor-store-persistence` plan: the one-directional `protocol.rs` -> `DurableTransition::between` (pure structural diff) -> `store.rs` (SQL translation) -> `RepositoryAgentTraceDb` boundary; migration `004_mutation_trace_protocol.sql`'s five tables; `AttemptState`/`external_taint` non-persistence; the 8-byte big-endian `BLOB` revision encoding plus explicit non-`Debug` enum codecs; the bounded hot-path `load_worktree` read vs. the cold-path `load_mutation_event`; the public cold-path single-row `load_scope` scope seam that returns one `mutation_trace_scopes` row as `Option` without widening into a projection and, unlike `load_worktree`, never adjudicates worktree identity (a cross-worktree scope is returned as-is, and the mismatch is the caller's decision); the cold-path read-only `load_tree_roots` (one worktree) / `load_all_tree_roots` (repository-wide) durable-tree-SHA queries for ref reconciliation, each a single-statement `UNION` of `cursor_tree`/`before_tree`/`after_tree` read from one DB snapshot; `commit`'s single-`BEGIN IMMEDIATE` CAS batch via `TursoDb::execute_transactional_cas_batch`, distinguishing `Conflict`/retryable-transient/deterministic-`Err` outcomes; and the store's non-goals — no Git/filesystem I/O, no attribution/boundary-kind decisions, no retry-after-`Conflict` loop, no row deletion) - `context/cli/mutation-trace-runtime-coordinator.md` (imperative-shell runtime layer in `cli/src/services/mutation_trace/runtime/`, built by the `mutation-cursor-runtime-coordinator` plan: the per-worktree OS advisory lock, `runtime::worktree_lock::WorktreeLock::acquire(git_dir, timeout)` at `/sce/mutation-cursor.lock`, bounded `try_lock()` polling with a distinct matchable timeout error and RAII release, and its distinction from the separate checkout-identity-creation lock; the isolated Git snapshot service `runtime::git_snapshot::GitSnapshotService` (documented in `mutation-trace-snapshot-service.md`); and `runtime::coordinator`'s protocol-integration pipeline (`RuntimeBoundary`/`CoordinateOutcome`/`CoordinateError`, the load → recover-if-needed → prepare/commit CAS-retry loop, and its own bounded snapshot-failure taint-retry loop) plus the public `coordinate(repository_root, boundary, open_db)` entrypoint that runs the shared `runtime::protected_worktree` prefix (`WorktreeLock` -> external-taint write-ahead fence -> `WorktreeId`, extracted so a second entrypoint cannot drift from it and documented in `mutation-trace-protected-worktree.md`), invokes the caller-supplied `open_db` provider so DB acquisition falls inside that fence, drives the pipeline under one held lock, and clears the marker through `ProtectedWorktree::complete()` only on success; `runtime/tests.rs` covers the public `coordinate()` API end to end against real linked worktrees and a real Agent Trace DB; an inherited external-taint marker is overlaid onto `protocol::database_failure` recovery on the next invocation, against the single captured snapshot and re-injected across a losing recovery CAS; the private `runtime::ref_reconciliation` per-worktree snapshot-ref maintenance pass (documented in `mutation-trace-ref-reconciliation.md`) shares the same `WorktreeLock`; a `pub(crate)` re-export of `coordinate()` beyond `runtime` and harness/command wiring remain future work) - `context/cli/mutation-trace-protected-worktree.md` (the shared safety prefix every mutation-cursor runtime entrypoint runs behind, in `cli/src/services/mutation_trace/runtime/protected_worktree.rs`, extracted from `coordinate()` by the `mutation-scope-runtime-integration` plan so a second entrypoint cannot drift from it: `ProtectedWorktree::acquire(repository_root)` running the safety-critical fixed order resolve `git_dir` → `WorktreeLock` (module-owned 10s `WORKTREE_LOCK_TIMEOUT`) → `ExternalTaintMarker::exists()` → `persist()` (fence armed write-ahead of every fallible step that follows, including DB acquisition) → `get_or_create_checkout_id` as `WorktreeId`; the `worktree_id()` / `inherited_external_taint()` / consuming `complete()` surface, where `complete()` clears the marker under the still-held lock and is the only thing that ever clears it while `Drop` releases only the lock; and the one-variant-per-prefix-step `ProtectedWorktreeError` (`GitDirResolution` | `LockAcquisition` | `ExternalTaintMarker { operation, source }` | `CheckoutIdentity`) each entrypoint maps onto its own error surface — `coordinate()` onto exactly the `CoordinateError` variants that step produced before the extraction) +- `context/cli/mutation-trace-scope-abandonment.md` (the mutation-cursor runtime's second protected entrypoint in `cli/src/services/mutation_trace/runtime/scope_runtime.rs`, built by the `mutation-scope-runtime-integration` plan and the first production call site for `protocol::abandon`: `abandon_scope(repository_root, scope, open_db) -> Result` retires a scope whose final worktree boundary was never observed, sharing `coordinate()`'s `ProtectedWorktree` prefix but deliberately capturing **no** Git snapshot, pin, diff, reconciliation, scope registration, or worktree initialization — so abandonment is not a `RuntimeBoundary` and needs no Quint change; the classify-before-transition order that recovers what `protocol::abandon`'s uniform guarded no-op cannot report (`load_scope` first for the missing-row and cross-worktree cases the projection seam treats as errors, then `load_worktree` for the `NeverSeen` / terminal / `Active` split); the `Abandoned` / `AlreadyTerminal` / `RecoveryRequired` outcomes and the `InheritedExternalTaint` | `MissingScope` | `NeverSeenScope` | `MissingWorktreeState` recovery reasons; the inherited-marker short-circuit that returns before the DB provider is ever invoked; the fence-completion rule that clears the marker only for a settled abandonment or proven-terminal no-op and leaves it armed for every recovery-required outcome and every error, with `MarkerClearAfterCompletion` carrying the already-settled outcome; the CAS retry bounded by the coordinator's shared `MAX_CAS_RETRY_ATTEMPTS`, settling on a competitor's terminal status rather than overwriting it; and the deliberate false-negative tradeoff whereby a missing or `NeverSeen` target forces conservative strong recovery that may abandon unrelated live scopes, because attribution safety outranks preserving potentially valid evidence) - `context/cli/mutation-trace-ref-reconciliation.md` (the conservative per-worktree snapshot-ref reconciliation pass in `cli/src/services/mutation_trace/runtime/ref_reconciliation.rs`, built by the `mutation-cursor-ref-reconciliation` plan: `reconcile_worktree(repository_root, open_db)` → `pub(super) reconcile_worktree_inner(.., on_lock_contention)`, both returning `ReconciliationOutcome` (`Reconciled(ReconciliationReport { local_required, retained, deleted })` for a pass that ran | `SkippedNoCheckoutIdentity` — an `Ok`, not an `Err` — when no current checkout identity could be derived), a variant-per-fallible-step `ReconcileError` with no `Other`, and the module-owned `RECONCILIATION_LOCK_TIMEOUT`; the two-invariant model — a strictly per-worktree fail-closed local-consistency check via `load_tree_roots(W)` vs. a repository-wide deletion-safety set via `load_all_tree_roots()` so an `A`-owned ref is retained whenever any worktree still durably needs its tree — run entirely under the same `/sce/mutation-cursor.lock` `WorktreeLock` `coordinate()` holds, with the repository-wide read kept coherent by being one SQL statement / one DB snapshot rather than a repository-global lock; deletes only SCE-owned refs via one atomic `git update-ref --no-deref --stdin`, writes no `mutation_trace_*` row, never arms `ExternalTaintMarker`, runs no `git gc`; imperative durability maintenance below the verified Quint protocol; reclaims orphan/unreferenced refs only for the namespace of a checkout id a current worktree still derives — a namespace no current worktree owns (identity-based: a deleted linked worktree, or checkout-id metadata loss followed by `get_or_create_checkout_id` minting a fresh id on a still-present worktree) is beyond every per-worktree pass and left to a recorded future repository-scoped unowned-namespace operation, so the current pass does not bound all orphan-ref growth; `reconcile_worktree` has no `pub(crate)` re-export and no harness/command wiring yet) - `context/cli/mutation-trace-snapshot-service.md` (the isolated Git snapshot and ref-pinning service `runtime::git_snapshot::GitSnapshotService` in `cli/src/services/mutation_trace/runtime/git_snapshot.rs`: `new` resolving an absolute `git_dir`, `capture_tree` snapshotting staged/unstaged/untracked/deleted worktree state into the repository's normal object database via a throwaway temp index, `pin_tree` protecting a durable tree with a create-only idempotent **direct** `refs/sce/mutation-cursor//` ref, `diff_trees` emitting `patch.rs`-parseable raw diff text; plus the callerless reconciliation substrate — worktree-scoped `list_pins` inventory returning `Result, PinInventoryError>` that rejects a symbolic ref inside the namespace (mutation-cursor pins are direct refs) as `MalformedRef`, matchable separately from a `git for-each-ref` execution failure, and conditional-atomic `delete_pins` running one `git update-ref --no-deref --stdin` transaction of SHA-conditioned deletes — no-dereference so an inventory→delete direct-ref→symref race cannot escape the inventoried namespace, plus a fail-closed pre-check — that aborts whole if any ref changed since inventory; `REF_NAMESPACE` + `pin_ref_prefix` as the single source of truth for the pin path) - `context/cli/mutation-trace-external-taint.md` (the worktree-local mutation-cursor durability boundary in `cli/src/services/mutation_trace/runtime/external_taint.rs`, built by the `mutation-cursor-external-taint` plan: the `ExternalTaintMarker` primitive — `new(git_dir)`/`exists()`/`persist()`/`clear()` over an empty file at `/sce/mutation-cursor-tainted` whose existence is its entire state, `checkout::persist_checkout_id_inner`-style durability (`sync_data` plus best-effort `#[cfg(unix)]` parent-dir `sync_all`), idempotent persist/clear, `NotFound`-on-clear as success, no `Drop` deletion — as the concrete runtime refinement of the abstract `ProtocolState.external_taint`; armed by the reshaped `coordinate()` entrypoint write-ahead after the `WorktreeLock` and before Agent Trace DB acquisition (a caller-supplied DB provider closure), cleared only on a successful `CoordinateOutcome`, with dedicated fail-closed pre-commit `CoordinateError::ExternalTaintMarker` (`Inspect`/`Persist` only)/`AgentTraceDbUnavailable` variants plus a post-commit `MarkerClearAfterCommit { source, committed }` that carries the durable outcome so a failed trailing clear never hides a committed `MutationEvent`; an inherited marker seeds an invocation-local `external_taint_pending` flag that overlays `protocol::database_failure` onto each freshly loaded projection so `recover` runs once against the captured snapshot, held across a losing recovery CAS and cleared once it lands) diff --git a/context/patterns.md b/context/patterns.md index 6ad26b9a..a8a04938 100644 --- a/context/patterns.md +++ b/context/patterns.md @@ -189,7 +189,7 @@ - Pure unit tests should test in-memory logic, parsing, validation, and data transformations without external dependencies; prefer mocking/faking external dependencies over creating real filesystem or database state when a fake is available and sufficient. - In-memory database tests (e.g., `LocalDatabaseTarget::InMemory`) are acceptable for unit tests since they don't touch the filesystem. -- A filesystem-touching `#[cfg(test)] mod tests` inline in the module under test is an established, Nix-sandbox-safe pattern in this codebase when every path is a unique, process-local path under `TMPDIR` (itself a writable per-build directory inside the Nix sandbox). Prefer an RAII `tempfile::TempDir` fixture, which cleans itself up even when a test panics — as `cli/src/services/mutation_trace/store.rs`'s `RepositoryAgentTraceDb`-backed tests, `runtime/tests.rs`, and `runtime/protected_worktree.rs` do. Manually composed unique `std::env::temp_dir()` paths are the older form of the same pattern and remain in `cli/src/services/checkout/mod.rs`'s checkout-identity-lock tests and in `runtime/coordinator.rs`, `runtime/worktree_lock.rs`, and `runtime/external_taint.rs`; they leak their directory on a panicking test, so new tests should use `TempDir`. This is not integration-test-only territory in this repository. +- A filesystem-touching `#[cfg(test)] mod tests` inline in the module under test is an established, Nix-sandbox-safe pattern in this codebase when every path is a unique, process-local path under `TMPDIR` (itself a writable per-build directory inside the Nix sandbox). Prefer an RAII `tempfile::TempDir` fixture, which cleans itself up even when a test panics — as `cli/src/services/mutation_trace/store.rs`'s `RepositoryAgentTraceDb`-backed tests, `runtime/tests.rs`, `runtime/protected_worktree.rs`, and `runtime/scope_runtime.rs` do. Manually composed unique `std::env::temp_dir()` paths are the older form of the same pattern and remain in `cli/src/services/checkout/mod.rs`'s checkout-identity-lock tests and in `runtime/coordinator.rs`, `runtime/worktree_lock.rs`, and `runtime/external_taint.rs`; they leak their directory on a panicking test, so new tests should use `TempDir`. This is not integration-test-only territory in this repository. - Use integration tests instead of this inline pattern where the behavior genuinely cannot be made deterministic and isolated inside the Nix sandbox this way. - Do not depend on a shared or ambient path (`$HOME`, a fixed `/tmp` file name, the repository's own working tree) from a unit test; each test must construct its own unique, self-cleaning path. - When a unit test needs behavior that cannot be made Nix-sandbox-safe this way (for example, network access, or state shared across the whole test binary), delete it from the unit-test suite and reintroduce that coverage later as an integration test instead of keeping ignored tests in-tree. diff --git a/context/plans/mutation-scope-runtime-integration.md b/context/plans/mutation-scope-runtime-integration.md index 96654780..28ae5576 100644 --- a/context/plans/mutation-scope-runtime-integration.md +++ b/context/plans/mutation-scope-runtime-integration.md @@ -418,13 +418,82 @@ which is precisely why there is nothing left for this one to decide. deliberate non-adjudication of worktree identity). - Context synchronization: synced -- [ ] T03: `Implement abandon_scope() on the protected runtime path` (status:todo) +- [x] T03: `Implement abandon_scope() on the protected runtime path` (status:done) - Task ID: T03 - Scope: In — new `cli/src/services/mutation_trace/runtime/scope_runtime.rs` with `abandon_scope`, `AbandonScopeOutcome`, its recovery-reason enum, and `AbandonScopeError`, built on T01's guard and T02's read; inline `#[cfg(test)] mod tests` against a real temp-file `RepositoryAgentTraceDb` covering active abandonment, `Closed`/`Abandoned` terminal no-op, missing and `NeverSeen` recovery-required, inherited-marker short-circuit before the DB provider, worktree-identity rejection, revision exhaustion, CAS reload/recompute/retry, DB-provider failure, and marker-clear-after-completion. Out — any Git snapshot, pin, diff, reconciliation, scope registration, or worktree initialization; any `runtime/mod.rs` export; any real-Git cross-worktree test (T04). - Dependencies: T01, T02 - Done when: an inherited marker returns recovery-required for that reason without invoking `open_db` and without clearing the marker; a missing or `NeverSeen` scope returns recovery-required, writes nothing, and leaves the marker armed, per **Design decisions** D1 — the scope lookup happens after the fence is armed, never before it (D2); a `Closed`/`Abandoned` scope returns the terminal no-op with the current revision and clears the marker; an `Active` scope belonging to another `WorktreeId` is an error that writes nothing; an `Active` scope on a worktree at `u64::MAX` returns a distinct revision-exhaustion error; otherwise the durable transition sets the scope `Abandoned`, advances revision by exactly one, sets `needs_rebaseline`, writes no event or processed-event row, and clears the marker; a `CasResult::Conflict` reloads and recomputes from fresh state within `MAX_CAS_RETRY_ATTEMPTS`, settling as the terminal no-op when a competitor won; a failing `clear()` after a completed abandonment or terminal no-op returns an error carrying that completed outcome; the module names no Git-snapshot or reconciliation symbol. - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::scope_runtime::`; `rg -n 'GitSnapshotService|SnapshotCapture|capture_tree|pin_tree|diff_trees|reconcile_worktree|initialize_worktree|register_scope' cli/src/services/mutation_trace/runtime/scope_runtime.rs` returns nothing; `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings`. - - Context synchronization: pending + - Completed: 2026-09-03 + - Files changed: + - `cli/src/services/mutation_trace/runtime/scope_runtime.rs` (new) + - `cli/src/services/mutation_trace/runtime/mod.rs` + - `cli/src/services/mutation_trace/runtime/coordinator.rs` + - Result: `scope_runtime.rs` adds the runtime's second protected entrypoint, + `abandon_scope(repository_root, &ScopeId, open_db)`, taking the same + caller-supplied DB-provider shape as `coordinate()`. It acquires T01's + `ProtectedWorktree` (so the prefix ordering, the `WORKTREE_LOCK_TIMEOUT`, and + the fence's arm/clear ownership are shared, not duplicated) and captures no Git + snapshot; the module names none of `GitSnapshotService`, `SnapshotCapture`, + `capture_tree`, `pin_tree`, `diff_trees`, `reconcile_worktree`, + `initialize_worktree`, or `register_scope`. An inherited marker short-circuits + to `RecoveryRequired { InheritedExternalTaint }` before `open_db` is called + (AC6, D2's stated concession); every other decision is made after the fence is + armed. Inside the fence, a bounded loop over the coordinator's existing + `MAX_CAS_RETRY_ATTEMPTS` runs T02's `load_scope` first — because the projection + seam treats both of that read's cases as errors and neither is one: a missing + row is D1's `MissingScope` recovery, and a foreign `worktree_id` is the typed + `WorktreeIdentityMismatch` rejection — then loads + `load_worktree(worktree, Some(scope), None)` (`None` -> `MissingWorktreeState`) + and classifies from that fresh projection: `NeverSeen` -> recovery-required, + `Closed`/`Abandoned` -> `AlreadyTerminal` with the current revision, `Active` -> + `protocol::abandon` + `DurableTransition::between` + `store.commit`. A `None` + transition on a proven-live scope in a projection whose `external_taint` is + always empty can only mean an unadvanceable revision, so it maps to + `RevisionExhausted`; `CasResult::Conflict` re-enters the loop and re-classifies + from scratch, so a competitor that closed the scope settles as `AlreadyTerminal` + rather than being overwritten. The marker is cleared only for `Abandoned` and + `AlreadyTerminal`; every `RecoveryRequired` and every error leaves it armed, and + a failing `clear()` returns `MarkerClearAfterCompletion { source, completed }` + carrying the already-settled outcome. Two supporting edits: `runtime/mod.rs` + gained the private `mod scope_runtime;` declaration (no `pub(crate) use` — that + remains T05's), and `coordinator.rs`'s `MAX_CAS_RETRY_ATTEMPTS` widened from + private to `pub(super)` so the retry limit is reused rather than redeclared, as + the plan's constraints require. A private `abandon_scope_inner` carries an + `after_load` seam (mirroring `coordinate_inner`) for the CAS tests. The inline + `#[cfg(test)] mod tests` uses an RAII `tempfile::TempDir` fixture holding a real + `git init` repository and a real temp-file `RepositoryAgentTraceDb`, with 14 + tests covering every `Done when` clause. A post-review amendment added the + 14th, `a_persistence_failure_rolls_back_the_whole_transition_and_leaves_the_fence_armed`, + closing AC10's remaining row: it drives the real `MutationTraceStore::commit` + path and, through the `after_load` seam, creates a `UNIQUE` index on + `mutation_trace_scopes(status)` that the seeded rows already satisfy and only + the `abandoned` status the transition is about to write violates. The worktree + row is untouched, so the transaction's CAS guard still matches and applies, + and the later scope-status statement then fails — proving the batch rolls the + guard back rather than leaving a partially updated worktree. No store or + runtime code was changed for it. Per an explicit user instruction during this + task, every comment this task wrote was then removed from + `scope_runtime.rs`; the rationale they carried is preserved in + `context/cli/mutation-trace-scope-abandonment.md`. + - Verify results: + - `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::scope_runtime::` — passed: 14 passed, 0 failed. + - `rg -n 'GitSnapshotService|SnapshotCapture|capture_tree|pin_tree|diff_trees|reconcile_worktree|initialize_worktree|register_scope' cli/src/services/mutation_trace/runtime/scope_runtime.rs` — no match (exit 1), run as `nix run nixpkgs#ripgrep --` per the repository's bash-tool policy. + - `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` — passed, clean, with no placeholder consumer added; the pre-existing `#[allow(dead_code)] pub mod mutation_trace;` in `cli/src/services/mod.rs` covers the not-yet-exported items. + - Also run: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::` — passed: 284 passed, 0 failed, confirming the coordinator, runtime, store, protocol, and MBT suites are unaffected. + - Also run: `./scripts/run-cli-cargo.sh fmt --manifest-path cli/Cargo.toml -- --check` — passed after formatting the new file. + - Context impact: local to `cli/src/services/mutation_trace/runtime/`. No public + interface, schema, migration, spec, or protocol change; `coordinate()`'s + signature, outcomes, and error variants are unchanged, `protocol::abandon` was + not touched, and nothing new is reachable outside `runtime` yet. Durable context + affected: `context/cli/mutation-trace-runtime-coordinator.md` (the runtime now + has two entrypoints over one protected prefix, and `MAX_CAS_RETRY_ATTEMPTS` is + the shared retry limit for both), `context/cli/mutation-trace-external-taint.md` + (the fence's abandonment-path completion semantics: cleared on + abandoned/terminal, left armed on recovery-required and on every error), and + `context/cli/mutation-trace-protocol.md` (`protocol::abandon` now has a + production call site, and abandonment is not a `RuntimeBoundary`). + - Context synchronization: synced - [ ] T04: `Add cross-runtime abandonment safety regressions` (status:todo) - Task ID: T04 From 1ffa6d446206343a70c895a114d7ab5a71937ffb Mon Sep 17 00:00:00 2001 From: David Abram Date: Thu, 3 Sep 2026 13:29:19 +0200 Subject: [PATCH 5/6] runtime: Add cross-runtime abandonment safety regressions Exercise scope abandonment and successor recovery against real Git worktrees and a repository-scoped Agent Trace DB. Cover ambiguous-gap rebaselining, unrelated live-scope preservation, cross-checkout rejection, and a real CAS race; widen the inner seam only to support the sibling-module race test. Record the completed integration task and its behavioral guarantees in the mutation-trace runtime and scope-abandonment context. Plan: mutation-scope-runtime-integration.md T04 Co-authored-by: SCE --- .../mutation_trace/runtime/scope_runtime.rs | 2 +- .../services/mutation_trace/runtime/tests.rs | 421 +++++++++++++++++- .../cli/mutation-trace-runtime-coordinator.md | 20 +- .../cli/mutation-trace-scope-abandonment.md | 46 +- .../mutation-scope-runtime-integration.md | 75 +++- 5 files changed, 544 insertions(+), 20 deletions(-) diff --git a/cli/src/services/mutation_trace/runtime/scope_runtime.rs b/cli/src/services/mutation_trace/runtime/scope_runtime.rs index 07b0b204..63d6d3fe 100644 --- a/cli/src/services/mutation_trace/runtime/scope_runtime.rs +++ b/cli/src/services/mutation_trace/runtime/scope_runtime.rs @@ -121,7 +121,7 @@ where abandon_scope_inner(repository_root, scope, open_db, |_attempt| {}) } -fn abandon_scope_inner( +pub(super) fn abandon_scope_inner( repository_root: &Path, scope: &ScopeId, open_db: P, diff --git a/cli/src/services/mutation_trace/runtime/tests.rs b/cli/src/services/mutation_trace/runtime/tests.rs index ad8c9f87..de683863 100644 --- a/cli/src/services/mutation_trace/runtime/tests.rs +++ b/cli/src/services/mutation_trace/runtime/tests.rs @@ -9,9 +9,13 @@ use crate::services::agent_trace_storage::{ resolve_agent_trace_storage_at_state_root, AgentTraceStorageContext, }; use crate::services::checkout::{read_checkout_id, resolve_git_dir}; -use crate::services::mutation_trace::store::{encode_revision, MutationTraceStore}; +use crate::services::mutation_trace::protocol; +use crate::services::mutation_trace::store::{ + encode_revision, CasResult, DurableTransition, MutationTraceStore, +}; use crate::services::mutation_trace::types::{ - ActorKind, EventId, FailureKind, ScopeId, ScopeStatus, + boundary_event_key, boundary_scope, ActorKind, AttemptId, Boundary, EventId, FailureKind, + ScopeId, ScopeStatus, }; use super::coordinator::{coordinate, coordinate_inner, CoordinateError, RuntimeBoundary}; @@ -20,6 +24,9 @@ use super::git_snapshot::GitSnapshotService; use super::ref_reconciliation::{ reconcile_worktree, reconcile_worktree_inner, ReconcileError, ReconciliationOutcome, }; +use super::scope_runtime::{ + abandon_scope, abandon_scope_inner, AbandonScopeError, AbandonScopeOutcome, +}; use super::worktree_lock::{acquire_inner, WorktreeLock}; fn run_git(dir: &Path, args: &[&str]) -> String { @@ -1849,3 +1856,413 @@ fn missing_checkout_identity_through_the_public_entrypoint_returns_skipped_outco "the skip must not create a checkout identity" ); } + +#[test] +#[allow(clippy::too_many_lines)] +fn an_abandoned_scope_rebaselines_the_successor_start_without_evidence_for_the_gap() { + let repo = TestRepo::new("public-abandon-then-successor-start"); + let ok_db = || repo.open_db(); + + let baseline = coordinate(&repo.repo_root, &RuntimeBoundary::Flush, ok_db) + .expect("the baseline observation should materialize the worktree"); + let worktree_id = baseline.worktree_id.clone(); + let tree_a = baseline.observed_tree.clone(); + + let scope_a = ScopeId("scope-dead-execution".to_string()); + let started = coordinate( + &repo.repo_root, + &RuntimeBoundary::Start { + scope: scope_a.clone(), + event: EventId("evt-a-start".to_string()), + actor_kind: ActorKind::ClaudeCode, + }, + ok_db, + ) + .expect("starting scope A should succeed"); + let first_gap_revision = started.revision; + + std::fs::write( + repo.repo_root.join("edited-by-a.txt"), + b"a mutation scope A made but never closed", + ) + .expect("the edit inside scope A should write"); + + let abandoned = abandon_scope(&repo.repo_root, &scope_a, ok_db) + .expect("abandoning the stale scope should succeed"); + let AbandonScopeOutcome::Abandoned { + revision: abandon_revision, + .. + } = abandoned + else { + panic!("expected a durable abandonment, got {abandoned:?}"); + }; + assert_eq!( + abandon_revision, + first_gap_revision + 1, + "abandonment advances the worktree revision by exactly one" + ); + + std::fs::write( + repo.repo_root.join("edited-after-a-died.txt"), + b"a mutation nobody observed a boundary for", + ) + .expect("the unobserved edit after abandonment should write"); + + let scope_b = ScopeId("scope-successor".to_string()); + let successor = coordinate( + &repo.repo_root, + &RuntimeBoundary::Start { + scope: scope_b.clone(), + event: EventId("evt-b-start".to_string()), + actor_kind: ActorKind::ClaudeCode, + }, + ok_db, + ) + .expect("the successor Start must rebaseline over the abandoned scope's gap"); + let tree_c = successor.observed_tree.clone(); + assert_ne!( + tree_c, tree_a, + "the working tree must have moved across the abandoned interval" + ); + assert!( + successor.mutation_event.is_none(), + "no evidence may be emitted for the A -> B interval, whose final boundary was never observed" + ); + + let db = repo.db(); + let store = MutationTraceStore::new(&db); + let projection = store + .load_worktree(&worktree_id, Some(&scope_b), None) + .expect("loading the worktree row should succeed") + .expect("the worktree row should exist"); + assert_eq!( + projection.worktree_state.cursor_tree, tree_c, + "the cursor must sit at the tree observed at Start(B), not at the pre-abandonment tree" + ); + assert!(!projection.worktree_state.needs_rebaseline); + assert!(!projection.worktree_state.tainted); + assert_eq!(projection.worktree_state.failure_kind, FailureKind::Healthy); + + let statuses = store + .load_worktree(&worktree_id, Some(&scope_a), None) + .expect("loading the worktree row should succeed") + .expect("the worktree row should exist"); + assert_eq!( + statuses.scopes.get(&scope_a).map(|state| state.status), + Some(ScopeStatus::Abandoned), + "the abandoned scope must stay Abandoned across the successor's recovery" + ); + assert_eq!( + projection.scopes.get(&scope_b).map(|state| state.status), + Some(ScopeStatus::Active), + "the successor scope must be Active after its Start" + ); + + assert_eq!( + row_count(&db, "mutation_trace_events"), + 0, + "an interval bounded by an abandonment and a rebaseline can produce no MutationEvent" + ); + for revision in first_gap_revision..=successor.revision { + assert!( + store + .load_mutation_event(&worktree_id, revision) + .expect("loading a mutation event should succeed") + .is_none(), + "revision {revision} spans the unobserved A -> B interval and must carry no evidence" + ); + } +} + +#[test] +fn abandoning_a_stale_scope_leaves_an_unrelated_live_scope_active_through_the_recovery() { + let repo = TestRepo::new("public-abandon-preserves-live-scope"); + let ok_db = || repo.open_db(); + + let baseline = coordinate(&repo.repo_root, &RuntimeBoundary::Flush, ok_db) + .expect("the baseline observation should materialize the worktree"); + let worktree_id = baseline.worktree_id.clone(); + + let stale = ScopeId("scope-stale".to_string()); + let live = ScopeId("scope-live".to_string()); + coordinate( + &repo.repo_root, + &RuntimeBoundary::Start { + scope: stale.clone(), + event: EventId("evt-stale-start".to_string()), + actor_kind: ActorKind::ClaudeCode, + }, + ok_db, + ) + .expect("starting the scope that will go stale should succeed"); + coordinate( + &repo.repo_root, + &RuntimeBoundary::Start { + scope: live.clone(), + event: EventId("evt-live-start".to_string()), + actor_kind: ActorKind::Codex, + }, + ok_db, + ) + .expect("starting the unrelated live scope should succeed"); + + let abandoned = abandon_scope(&repo.repo_root, &stale, ok_db) + .expect("abandoning the stale scope should succeed"); + assert!( + matches!(abandoned, AbandonScopeOutcome::Abandoned { .. }), + "expected a durable abandonment, got {abandoned:?}" + ); + + std::fs::write( + repo.repo_root.join("edited-by-the-live-scope.txt"), + b"the surviving scope keeps working", + ) + .expect("the live scope's edit should write"); + + let advanced = coordinate( + &repo.repo_root, + &RuntimeBoundary::Advance { + scope: live.clone(), + event: EventId("evt-live-advance".to_string()), + actor_kind: ActorKind::Codex, + }, + ok_db, + ) + .expect("the live scope must still be able to advance after the unrelated abandonment"); + assert!( + advanced.mutation_event.is_none(), + "the needs_rebaseline recovery consumes the ambiguous interval rather than attributing it" + ); + + let db = repo.db(); + let store = MutationTraceStore::new(&db); + let projection = store + .load_worktree(&worktree_id, Some(&live), None) + .expect("loading the worktree row should succeed") + .expect("the worktree row should exist"); + assert_eq!( + projection.scopes.get(&live).map(|state| state.status), + Some(ScopeStatus::Active), + "abandoning one scope must not abandon an unrelated scope that is legitimately live" + ); + + let stale_projection = store + .load_worktree(&worktree_id, Some(&stale), None) + .expect("loading the worktree row should succeed") + .expect("the worktree row should exist"); + assert_eq!( + stale_projection + .scopes + .get(&stale) + .map(|state| state.status), + Some(ScopeStatus::Abandoned), + "only the named scope may be abandoned" + ); +} + +#[test] +fn abandoning_a_scope_through_another_worktrees_checkout_is_rejected_without_writing() { + let repo = LinkedTestRepo::new("public-abandon-wrong-checkout"); + let ok_db = || repo.open_db(); + + let main = coordinate(&repo.main_root, &RuntimeBoundary::Flush, ok_db) + .expect("the main worktree should materialize"); + let linked = coordinate(&repo.linked_root, &RuntimeBoundary::Flush, ok_db) + .expect("the linked worktree should materialize"); + assert_ne!(main.worktree_id, linked.worktree_id); + + let scope = ScopeId("scope-on-main".to_string()); + let started = coordinate( + &repo.main_root, + &RuntimeBoundary::Start { + scope: scope.clone(), + event: EventId("evt-main-start".to_string()), + actor_kind: ActorKind::ClaudeCode, + }, + ok_db, + ) + .expect("starting a scope on the main worktree should succeed"); + + let error = abandon_scope(&repo.linked_root, &scope, ok_db) + .expect_err("a scope may only be abandoned through its own checkout"); + match &error { + AbandonScopeError::WorktreeIdentityMismatch { + scope: rejected, + scope_worktree_id, + invoking_worktree_id, + } => { + assert_eq!(rejected, &scope); + assert_eq!(scope_worktree_id, &main.worktree_id); + assert_eq!(invoking_worktree_id, &linked.worktree_id); + } + other => panic!("expected WorktreeIdentityMismatch, got {other:?}"), + } + + let db = repo.db(); + let store = MutationTraceStore::new(&db); + let main_projection = store + .load_worktree(&main.worktree_id, Some(&scope), None) + .expect("loading the main worktree row should succeed") + .expect("the main worktree row should exist"); + assert_eq!( + main_projection.worktree_state.revision, started.revision, + "a rejected cross-checkout abandonment may not advance the target worktree's revision" + ); + assert_eq!( + main_projection.scopes.get(&scope).map(|state| state.status), + Some(ScopeStatus::Active), + "the target scope must stay Active after the rejection" + ); + + let linked_projection = store + .load_worktree(&linked.worktree_id, None, None) + .expect("loading the linked worktree row should succeed") + .expect("the linked worktree row should exist"); + assert_eq!( + linked_projection.worktree_state.revision, linked.revision, + "a rejected cross-checkout abandonment may not advance the invoking worktree's revision" + ); + + let linked_git_dir = resolve_git_dir(&repo.linked_root).expect("linked git dir should resolve"); + assert!( + ExternalTaintMarker::new(&linked_git_dir) + .exists() + .expect("marker existence should resolve"), + "a failed abandonment leaves the invoking worktree's fence armed for the next recovery" + ); + let main_git_dir = resolve_git_dir(&repo.main_root).expect("main git dir should resolve"); + assert!( + !ExternalTaintMarker::new(&main_git_dir) + .exists() + .expect("marker existence should resolve"), + "the rejection touched only the invoking worktree's fence" + ); +} + +#[test] +#[allow(clippy::too_many_lines)] +fn a_real_thread_cas_race_settles_on_the_competitors_terminal_status() { + let repo = TestRepo::new("public-abandon-cas-race"); + let ok_db = || repo.open_db(); + + let baseline = coordinate(&repo.repo_root, &RuntimeBoundary::Flush, ok_db) + .expect("the baseline observation should materialize the worktree"); + let worktree_id = baseline.worktree_id.clone(); + + let scope = ScopeId("scope-raced".to_string()); + coordinate( + &repo.repo_root, + &RuntimeBoundary::Start { + scope: scope.clone(), + event: EventId("evt-race-start".to_string()), + actor_kind: ActorKind::ClaudeCode, + }, + ok_db, + ) + .expect("starting the raced scope should succeed"); + + let (release_tx, release_rx) = mpsc::channel::<()>(); + let (closed_tx, closed_rx) = mpsc::channel::(); + let competitor_db_path = repo.db_path.clone(); + let competitor_worktree = worktree_id.clone(); + let competitor_scope = scope.clone(); + let competitor = thread::spawn(move || { + release_rx + .recv() + .expect("the abandoning thread should release the competitor"); + let db = RepositoryAgentTraceDb::open_for_hooks_without_migrations_at(&competitor_db_path) + .expect("the competing writer should open its own DB handle"); + let store = MutationTraceStore::new(&db); + let boundary = Boundary::Close { + scope: competitor_scope.clone(), + event: EventId("evt-race-close".to_string()), + }; + let scope_ref = boundary_scope(&boundary); + let event_key = boundary_event_key(&boundary); + let projection = store + .load_worktree(&competitor_worktree, scope_ref.as_ref(), event_key.as_ref()) + .expect("the competitor's load should succeed") + .expect("the worktree row should exist"); + let state = projection.into_protocol_state(); + let observed_tree = state + .worktrees + .get(&competitor_worktree) + .expect("the competitor's worktree state should be present") + .cursor_tree + .clone(); + let attempt = AttemptId("attempt-competing-close".to_string()); + let prepared = protocol::prepare(&state, attempt.clone(), boundary, observed_tree); + let outcome = protocol::commit(&prepared, &attempt); + let transition = DurableTransition::between(&state, &outcome.state, &competitor_worktree) + .expect("the competitor's transition should diff") + .expect("a Close must produce a durable transition"); + assert!( + matches!( + store + .commit(&transition) + .expect("the competitor's commit should run"), + CasResult::Applied + ), + "the competitor wins the race while the abandonment is still between load and commit" + ); + let revision = outcome + .state + .worktrees + .get(&competitor_worktree) + .expect("the competitor's committed worktree state should be present") + .revision; + closed_tx + .send(revision) + .expect("the abandoning thread should still be waiting"); + }); + + let mut release_tx = Some(release_tx); + let mut competitor_revision = None; + let settled = abandon_scope_inner(&repo.repo_root, &scope, ok_db, |attempt| { + if attempt == 0 { + release_tx + .take() + .expect("the competitor is released exactly once") + .send(()) + .expect("the competitor thread should still be listening"); + competitor_revision = Some( + closed_rx + .recv() + .expect("the competitor should report its committed revision"), + ); + } + }) + .expect("a lost CAS against a competing Close should settle, not fail"); + let competitor_revision = + competitor_revision.expect("the competitor must have run on the first attempt"); + competitor + .join() + .expect("the competing writer thread should not panic"); + + assert_eq!( + settled, + AbandonScopeOutcome::AlreadyTerminal { + worktree_id: worktree_id.clone(), + scope: scope.clone(), + status: ScopeStatus::Closed, + revision: competitor_revision, + }, + "the retry must settle on the competitor's terminal status rather than abandon again" + ); + + let db = repo.db(); + let store = MutationTraceStore::new(&db); + let projection = store + .load_worktree(&worktree_id, Some(&scope), None) + .expect("loading the worktree row should succeed") + .expect("the worktree row should exist"); + assert_eq!( + projection.scopes.get(&scope).map(|state| state.status), + Some(ScopeStatus::Closed), + "a competitor's durable Close must never be overwritten by a second abandonment" + ); + assert_eq!( + projection.worktree_state.revision, competitor_revision, + "the settled no-op writes nothing, so the revision stays at the competitor's" + ); +} diff --git a/context/cli/mutation-trace-runtime-coordinator.md b/context/cli/mutation-trace-runtime-coordinator.md index ae7af924..96feeceb 100644 --- a/context/cli/mutation-trace-runtime-coordinator.md +++ b/context/cli/mutation-trace-runtime-coordinator.md @@ -214,17 +214,17 @@ surfaces `MarkerClearAfterCommit` with the matching committed outcome. The `runtime/tests.rs` is `runtime`'s own `#[cfg(test)] mod tests` of cross-module integration tests against real Git repositories (`git init`, `git worktree -add`) and real temp-file `RepositoryAgentTraceDb`s — the public `coordinate()`, -the public `reconcile_worktree` integration suite (detailed in -[`mutation-trace-ref-reconciliation.md`](mutation-trace-ref-reconciliation.md#testing-boundary)), and the `pub(super)` `coordinate_inner` / `reconcile_worktree_inner` lock-race seams. Two linked worktrees (different `git_dir` → +add`) and real temp-file `RepositoryAgentTraceDb`s — the public `coordinate()`; +`coordinate()` and `abandon_scope()` driven together (detailed in +[`mutation-trace-scope-abandonment.md`](mutation-trace-scope-abandonment.md#cross-runtime-regressions)); the public `reconcile_worktree` integration suite (detailed in +[`mutation-trace-ref-reconciliation.md`](mutation-trace-ref-reconciliation.md#testing-boundary)); and the `pub(super)` `coordinate_inner` / `reconcile_worktree_inner` / `abandon_scope_inner` seams. Two linked worktrees (different `git_dir` → different lock paths → different `WorktreeId`s) are proven independently locked by holding one worktree's `WorktreeLock` across a synchronous `coordinate()` call for the other and seeing it return `Ok` only after the guard drops; each call's provider closure opens the one shared repository-scoped DB path and both worktree rows coexist in it. A first-ever `agent_trace_storage` resolution and a -`coordinate()` call on one checkout converge on one checkout identity; and a -full baseline → snapshot-failing taint → recovery cycle runs through the public -entrypoint. +`coordinate()` call on one checkout converge on one checkout identity, and a full +baseline → snapshot-failing taint → recovery cycle runs through the public entrypoint. ## Status @@ -232,10 +232,10 @@ The `ProtectedWorktree` prefix, lock, snapshot service, protocol-integration pipeline, the public `coordinate()` entrypoint (prefix → DB provider → pipeline → `complete()` on success), and the second `abandon_scope()` entrypoint sharing that prefix are all implemented, with `runtime/tests.rs` covering `coordinate()` -end to end; an inherited external-taint marker is overlaid onto -`database_failure` recovery on the next invocation. Real-Git abandonment -regressions, a `pub(crate)` re-export of either entrypoint, and harness/command -wiring remain future work (`mutation-cursor-external-taint`, +end to end and both entrypoints driven together; an inherited external-taint +marker is overlaid onto `database_failure` recovery on the next invocation. A +`pub(crate)` re-export of either entrypoint and harness/command wiring remain +future work (`mutation-cursor-external-taint`, `mutation-cursor-runtime-coordinator`, `mutation-scope-runtime-integration`). See also: [`mutation-trace-ref-reconciliation.md`](mutation-trace-ref-reconciliation.md) diff --git a/context/cli/mutation-trace-scope-abandonment.md b/context/cli/mutation-trace-scope-abandonment.md index 211025ea..7721d9cf 100644 --- a/context/cli/mutation-trace-scope-abandonment.md +++ b/context/cli/mutation-trace-scope-abandonment.md @@ -161,9 +161,10 @@ a DB-provider `Err` leaving the fence armed; a persistence failure rolling the whole transition back (below); and a `clear()` failure returning `MarkerClearAfterCompletion` whose carried outcome matches the durable state. -A private `abandon_scope_inner(.., after_load)` seam (mirroring +A `pub(super) abandon_scope_inner(.., after_load)` seam (mirroring `coordinate_inner`) fires once per CAS attempt after the projection loads, so a -test can land a competing write inside the CAS window. Production passes a +test — in this module or in `runtime/tests.rs` — can land a competing write +inside the CAS window. It is invisible outside `runtime`; production passes a no-op. ### Proving the transition is all-or-nothing @@ -185,11 +186,46 @@ After the resulting `Err`, the worktree is still at its original revision with exists, and the external-taint marker is still armed — the general rule that **every** runtime error after the fence is armed leaves it armed. +### Cross-runtime regressions + +`runtime/tests.rs` ([`mutation-trace-runtime-coordinator.md`](mutation-trace-runtime-coordinator.md#testing-boundary)) +drives `coordinate()` and `abandon_scope()` together over real `git init` / +`git worktree add` repositories and one real repository-scoped Agent Trace DB, +covering what a single-module test with no real Git cannot: + +- **The successor sequence.** `Start(A)` → edit → `abandon_scope(A)` → + unobserved edit → `coordinate(Start(B))`. The abandonment's + `needs_rebaseline` sends the successor's invocation through recovery first, so + the cursor lands on the tree observed at `Start(B)`, A stays `Abandoned`, B + becomes `Active`, and the ambiguous A→B interval produces no + `mutation_trace_events` row at any revision. +- **An unrelated live scope survives it.** With B legitimately `Active` across + `abandon_scope(A)`, B is still `Active` after its next `Advance` — the + `needs_rebaseline` recovery preserves live scopes, unlike the external-taint + recovery a `RecoveryRequired` outcome forces (above). +- **Wrong checkout.** Abandoning worktree A's scope through worktree B's + checkout is `WorktreeIdentityMismatch`; neither worktree's revision moves and + the scope stays `Active`. The fence ends up armed only on the *invoking* + checkout, because the guard derives its `WorktreeId` from the caller's own + checkout before any scope is read — the rejection is the invoking worktree's + error, not the target's. +- **A real CAS race.** A competing OS thread with its own handle on the same + on-disk DB commits a genuine `Close` (`load_worktree` → + `protocol::prepare`/`commit` → `DurableTransition` → `store.commit`) while the + abandonment sits between its own load and commit. The abandonment loses the + CAS, reloads, and settles as `AlreadyTerminal { status: Closed }` at the + competitor's revision. That competitor writes through the store rather than + through `coordinate()` deliberately: `abandon_scope()` holds the worktree lock + for its whole body, so a competitor taking the same lock would serialize + behind it instead of racing. Within one worktree the lock is what actually + prevents this race; the CAS retry is defense in depth for anything that + reaches the store without it. + ## Status -The entrypoint, its outcome/error types, and its unit coverage exist. Real-Git -cross-worktree and successor-scope regressions, the `pub(crate)` re-export out -of `runtime`, and the harness-adapter contract document remain future work in +The entrypoint, its outcome/error types, its unit coverage, and its cross-runtime +regressions against real Git repositories all exist. The `pub(crate)` re-export +out of `runtime` and the harness-adapter contract document remain future work in the same plan; no harness, hook, or command calls this yet. See also: [`mutation-trace-runtime-coordinator.md`](mutation-trace-runtime-coordinator.md), diff --git a/context/plans/mutation-scope-runtime-integration.md b/context/plans/mutation-scope-runtime-integration.md index 28ae5576..51fc386f 100644 --- a/context/plans/mutation-scope-runtime-integration.md +++ b/context/plans/mutation-scope-runtime-integration.md @@ -495,13 +495,84 @@ which is precisely why there is nothing left for this one to decide. production call site, and abandonment is not a `RuntimeBoundary`). - Context synchronization: synced -- [ ] T04: `Add cross-runtime abandonment safety regressions` (status:todo) +- [x] T04: `Add cross-runtime abandonment safety regressions` (status:done) - Task ID: T04 - Scope: In — integration tests in `cli/src/services/mutation_trace/runtime/tests.rs` driving `coordinate()` and `abandon_scope()` together against real `git init` / `git worktree add` repositories and a real repository-scoped Agent Trace DB: the abandon → unobserved edit → successor `Start` rebaseline sequence with no evidence for the gap; a concurrently active unrelated scope surviving that recovery; abandoning a scope through the wrong checkout; and a real-thread CAS race between `abandon_scope()` and a competing writer. Out — the single-module cases T03 already covers with a temp-file DB and no real Git. - Dependencies: T03 - Done when: `Start(A)` → edit → `abandon_scope(A)` → unobserved edit → `coordinate(Start(B))` leaves the cursor at the tree observed at `Start(B)`, emits no `mutation_trace_events` row for the A→B interval, leaves A `Abandoned` and B `Active`; a second scope B active across `abandon_scope(A)` is still `Active` after the next `coordinate()`; abandoning worktree A's scope through worktree B's checkout errors and changes no row in either worktree; the CAS-race test settles deterministically on the competitor's terminal status without a second abandonment. - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::tests::`; `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::`. - - Context synchronization: pending + - Completed: 2026-09-03 + - Files changed: + - `cli/src/services/mutation_trace/runtime/tests.rs` + - `cli/src/services/mutation_trace/runtime/scope_runtime.rs` + - Result: `runtime/tests.rs` gained four public-entrypoint integration tests that + drive `coordinate()` and `abandon_scope()` together over real `git init` / + `git worktree add` repositories and a real repository-scoped Agent Trace DB, + reusing the file's existing RAII `TestRepo` / `LinkedTestRepo` fixtures. + `an_abandoned_scope_rebaselines_the_successor_start_without_evidence_for_the_gap` + runs baseline `Flush` → `Start(A)` → a real edit → `abandon_scope(A)` → a second + unobserved edit → `coordinate(Start(B))`, asserting the abandonment advanced the + revision by exactly one, that `Start(B)` observed a tree different from the + baseline, that the durable cursor sits at the tree observed at `Start(B)` with + `needs_rebaseline`/`tainted` cleared and `failure_kind` healthy, that A is + `Abandoned` and B `Active`, and that the whole gap carries no evidence — both + `mutation_trace_events` being empty and `load_mutation_event` being `None` for + every revision from `Start(A)` through `Start(B)`. + `abandoning_a_stale_scope_leaves_an_unrelated_live_scope_active_through_the_recovery` + starts two scopes with different `ActorKind`s, abandons only the stale one, then + drives a real `Advance` on the live one; the live scope is still `Active`, the + stale one `Abandoned`, and the `needs_rebaseline` recovery consumes the ambiguous + interval rather than attributing it — the AC12 counterpart to D1's stronger + external-taint recovery. + `abandoning_a_scope_through_another_worktrees_checkout_is_rejected_without_writing` + materializes two linked worktrees over one shared DB, starts a scope on the main + checkout, and abandons it through the linked checkout: the error is + `WorktreeIdentityMismatch` carrying both `WorktreeId`s, neither worktree's + revision moved, the scope is still `Active`, and the fence is armed only on the + invoking (linked) worktree, not on the target's. + `a_real_thread_cas_race_settles_on_the_competitors_terminal_status` runs a genuine + OS thread with its own DB handle against the same on-disk DB. Released through + `abandon_scope_inner`'s `after_load` seam while the abandonment sits between its + load and its commit, the competitor performs a real store-level `Close` — + `load_worktree` → `protocol::prepare` → `protocol::commit` → + `DurableTransition::between` → `MutationTraceStore::commit`, asserting + `CasResult::Applied` — and reports its committed revision back over a channel. The + competitor writes through the store rather than through `coordinate()` on purpose: + `abandon_scope()` holds the worktree lock for its whole body, so a competitor + taking the same lock would serialize instead of racing. The abandonment then loses + its CAS, reloads, re-classifies the now-`Closed` scope, and settles as + `AlreadyTerminal { status: Closed, revision: }`; the durable scope + status stays `Closed` and the revision stays at the competitor's, proving no second + abandonment was written. One supporting edit outside the test file: + `scope_runtime.rs`'s private `abandon_scope_inner` widened to `pub(super)` so the + sibling `tests` module can reach its `after_load` seam, exactly as + `coordinator.rs` already exposes `pub(super) fn coordinate_inner` for the same + reason. No behavior, signature, or runtime logic changed. The plan's **Open + questions** allowed dropping the CAS race from T04 if reuse required exporting a + test helper across modules; no helper was exported, so AC8's real-thread + requirement was implemented rather than dropped. Consistent with T01 and T03, no + comments were added to the code. + - Verify results: + - `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::tests::` — passed: 27 passed, 0 failed (23 pre-existing plus the 4 new integration tests). + - `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::` — passed: 288 passed, 0 failed (284 before this task plus the 4 new tests), confirming the coordinator, scope-runtime, store, protocol, and MBT suites are unaffected. + - Also run: `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` — passed, clean; the CAS-race test carries `#[allow(clippy::too_many_lines)]`, matching the file's existing precedent for long integration tests. + - Also run: `./scripts/run-cli-cargo.sh fmt --manifest-path cli/Cargo.toml -- --check` — passed after `cargo fmt`. + - Context impact: local to `cli/src/services/mutation_trace/runtime/`. Test-only + except for one visibility widening (`abandon_scope_inner` private -> + `pub(super)`), which is a sibling-module test seam, not a crate-visible surface. + No public interface, schema, migration, spec, or protocol change; `coordinate()` + and `abandon_scope()` behavior is untouched and nothing new is reachable outside + `runtime`. Durable context affected: + `context/cli/mutation-trace-runtime-coordinator.md` (the two entrypoints are now + covered by cross-runtime integration regressions, and the `pub(super)` + `*_inner` test seam is the recorded convention for both) and + `context/cli/mutation-trace-scope-abandonment.md` (the abandon → + successor-`Start` rebaseline sequence, the live-scope survival guarantee, the + cross-checkout rejection, and why a CAS competitor must bypass the worktree lock + to race at all). `context/patterns.md` is the plan's recorded repair target for + the mutation-trace unit-testing fixture convention, which these tests continue + to follow via RAII `tempfile::TempDir`. + - Context synchronization: synced - [ ] T05: `Export the runtime seam and record the adapter contract` (status:todo) - Task ID: T05 From 5a4cbd18977a6e04e128701f6477cbab6aa041a4 Mon Sep 17 00:00:00 2001 From: David Abram Date: Thu, 3 Sep 2026 13:44:57 +0200 Subject: [PATCH 6/6] runtime: Export mutation scope seam and adapter contract Expose the coordinator and scope-abandonment entrypoints with their crate-visible outcome and error types while keeping the runtime safety modules private. Record the lifecycle, staleness, recovery, and attribution rules future harness adapters must follow, and update the runtime indexes and plan status. Plan: mutation-scope-runtime-integration (T05) Co-authored-by: SCE --- .../services/mutation_trace/runtime/mod.rs | 9 + context/cli/mutation-scope-runtime.md | 238 ++++++++++++++++++ .../cli/mutation-trace-runtime-coordinator.md | 22 +- .../cli/mutation-trace-scope-abandonment.md | 11 +- context/context-map.md | 3 +- context/overview.md | 2 +- .../mutation-scope-runtime-integration.md | 156 ++++++++++-- 7 files changed, 406 insertions(+), 35 deletions(-) create mode 100644 context/cli/mutation-scope-runtime.md diff --git a/cli/src/services/mutation_trace/runtime/mod.rs b/cli/src/services/mutation_trace/runtime/mod.rs index 49e5cbda..fc41f5e2 100644 --- a/cli/src/services/mutation_trace/runtime/mod.rs +++ b/cli/src/services/mutation_trace/runtime/mod.rs @@ -8,3 +8,12 @@ mod worktree_lock; #[cfg(test)] mod tests; + +#[allow(unused_imports)] +pub(crate) use coordinator::{ + coordinate, CoordinateError, CoordinateOutcome, ExternalTaintOperation, RuntimeBoundary, +}; +#[allow(unused_imports)] +pub(crate) use scope_runtime::{ + abandon_scope, AbandonRecoveryReason, AbandonScopeError, AbandonScopeOutcome, +}; diff --git a/context/cli/mutation-scope-runtime.md b/context/cli/mutation-scope-runtime.md new file mode 100644 index 00000000..c34eec2e --- /dev/null +++ b/context/cli/mutation-scope-runtime.md @@ -0,0 +1,238 @@ +# Mutation-scope runtime: the harness-adapter contract + +The crate-visible surface of `cli/src/services/mutation_trace/runtime/`, and the +lifecycle contract every harness adapter (Codex, Claude Code, OpenCode, Pi) must +uphold when it drives that surface. + +Built by the `mutation-scope-runtime-integration` plan +(`context/plans/mutation-scope-runtime-integration.md`). **No harness is wired to +it yet.** This file is the contract a future adapter is written against, not a +description of shipped adapter behavior. + +The mechanics behind each entrypoint live in their own domain files: +[`mutation-trace-runtime-coordinator.md`](mutation-trace-runtime-coordinator.md) +(`coordinate()`), +[`mutation-trace-scope-abandonment.md`](mutation-trace-scope-abandonment.md) +(`abandon_scope()`), +[`mutation-trace-protected-worktree.md`](mutation-trace-protected-worktree.md) +(the shared safety prefix), and +[`mutation-trace-protocol.md`](mutation-trace-protocol.md) (the pure protocol). +This file is the layer above them: what an adapter is required to do, and why. + +## The exported seam + +`runtime/mod.rs` re-exports exactly nine names at `pub(crate)`, reachable as +`crate::services::mutation_trace::runtime::*`: + +| Name | From | Role | +| --- | --- | --- | +| `coordinate` | `coordinator` | the observed-boundary entrypoint | +| `RuntimeBoundary` | `coordinator` | `Start` / `Advance` / `Close` / `Flush` | +| `CoordinateOutcome` | `coordinator` | its success value | +| `CoordinateError` | `coordinator` | its error surface | +| `ExternalTaintOperation` | `coordinator` | `Inspect` / `Persist`, carried by `CoordinateError::ExternalTaintMarker` | +| `abandon_scope` | `scope_runtime` | the unobserved-boundary entrypoint | +| `AbandonScopeOutcome` | `scope_runtime` | its success value | +| `AbandonRecoveryReason` | `scope_runtime` | the reason inside `RecoveryRequired` | +| `AbandonScopeError` | `scope_runtime` | its error surface | + +`ExternalTaintOperation` crosses the boundary because it is part of +`CoordinateError`'s own public shape — a crate-visible error a caller cannot +match on is not a usable seam. It lives in `protected_worktree.rs` and reaches +the seam through `coordinator.rs`'s `pub use super::protected_worktree:: +ExternalTaintOperation`, so the type becomes crate-visible **without** +`protected_worktree` becoming a public module. + +Every `mod` declaration in `runtime/mod.rs` stays private. Nothing else is +reachable from `git_snapshot`, `external_taint`, `worktree_lock`, +`ref_reconciliation`, or `protected_worktree` — in particular `ProtectedWorktree`, +`ProtectedWorktreeError`, and `WORKTREE_LOCK_TIMEOUT` remain internal to +`runtime`, as does `reconcile_worktree`. An adapter drives the runtime only +through the two entrypoints; it never assembles the safety prefix itself. + +Both re-export statements carry `#[allow(unused_imports)]`, matching the +repository's existing precedent for a seam whose consumers do not exist yet +(`services/style.rs`, `services/hooks/codex/apply_patch/mod.rs`). The +module-level `#[allow(dead_code)] pub mod mutation_trace;` in `services/mod.rs` +covers unused *items*, not unused re-exports. No placeholder consumer was added +to satisfy `clippy --all-targets -- -D warnings`. + +## What a mutation scope is + +**A scope is one independently mutation-capable execution.** Not one session, +not one process, not one harness. + +The practical consequence: a main agent and a subagent that can each edit the +worktree concurrently are two scopes and must carry **distinct `ScopeId`s**. If +an adapter gives them one shared `ScopeId`, their intervals collapse into a +single exclusivity claim and the protocol can never report `AiContended` for two +executions that genuinely raced. + +A `ScopeId` is durably bound to one worktree for life. `abandon_scope()` rejects +a target whose durable `worktree_id` differs from the `WorktreeId` the invocation +derived from its own checkout (`AbandonScopeError::WorktreeIdentityMismatch`), +and writes nothing. + +## `Start` / `Advance` / `Close` + +Each is a `RuntimeBoundary` passed to `coordinate()`, which captures a Git +snapshot, drives the protocol, and advances the worktree cursor to the observed +tree. The interval between two consecutive observed boundaries is what the +protocol can attribute. + +- **`Start { scope, event, actor_kind }`** — the scope's first boundary. The + protocol observes it only from `ScopeStatus::NeverSeen`; an accepted, observing + `Start` transitions the scope to `Active`. The event it emits attributes to the + scopes live *before* the activation, so a `Start` never attributes the + preceding interval to the scope it is starting. +- **`Advance { scope, event, actor_kind }`** — every subsequent mutation + boundary. Accepted only while the scope is live. +- **`Close { scope, event, actor_kind }`** — the terminal observed boundary, + accepted from `NeverSeen` or live, transitioning the scope to `Closed`. Its + emitted event still attributes to the scope it is closing. +- **`Flush`** — a worktree-level observation carrying no scope and no fields; + the worktree is the one this invocation derived from its own checkout. + +All three scope-carrying variants supply `actor_kind`, and `coordinate()` +registers the scope's durable `(worktree_id, actor_kind)` identity on every one +of them, not only on `Start` — a mismatch against an existing row is +`CoordinateError::ScopeIdentityConflict`. + +Two obligations follow, and both are easy to get wrong: + +**A failed tool still requires `Advance`.** The boundary marks *an observation of +the worktree*, not a successful edit. A tool that errored may still have written +files (a partial write, a half-applied patch, a script that failed after its side +effect). Skipping the `Advance` does not discard those mutations — it folds them +into the next observed interval, where they are attributed to whatever was live +then. Emit the boundary on failure exactly as on success. + +**A `ScopeId` is never reused after a terminal status.** Once a scope is `Closed` +or `Abandoned`, a later `Start` on that same `ScopeId` does not observe — the +`NeverSeen` guard rejects it — so the scope is not reactivated and the boundary +silently fails to establish what the adapter thinks it established. A new +execution always gets a fresh `ScopeId`. + +## `abandon_scope()` requires positive staleness evidence + +```rust +abandon_scope(repository_root, &scope, open_db) + -> Result +``` + +Abandonment is for a scope the adapter can **prove** is stale: the execution's +final worktree boundary was never observed and never will be. The canonical +evidence is a dead process — a recorded PID that no longer exists, a supervisor +that reports the execution terminated, an explicit harness signal. + +**Staleness must never be inferred from `ActorKind`.** `ActorKind` names the +harness that owns a scope (`ClaudeCode`, `Codex`, `OpenCode`, `Pi`). It says +nothing about whether that scope's execution is still running. An adapter that +abandons every scope carrying some other harness's `ActorKind` — or every scope +carrying its own, on startup — destroys live executions' evidence and, through +D1 below, can force recovery that invalidates unrelated live scopes. Absence of +evidence that a scope is alive is not evidence that it is dead. + +Abandonment is deliberately **not** a `RuntimeBoundary` variant. It takes no Git +snapshot, moves no cursor, and emits no `MutationEvent`; it only transitions +already-durable state (status → `Abandoned`, `revision` + 1, +`needs_rebaseline = true`). Because it adds no new observation, it refines no new +Quint action and requires **no change to `spec/mutation_cursor.qnt`**. + +## The abandon → successor-`Start` sequence + +The reason to abandon is almost always to start a successor safely: + +```text +abandon_scope(A) → coordinate(Start(B)) +``` + +The abandonment sets `needs_rebaseline` on the worktree, so the successor +`Start(B)` re-baselines the cursor to the tree it observes and emits **no** +mutation evidence for the ambiguous A→B interval. That interval is discarded +rather than misattributed — which is the entire point. + +What each outcome implies for that successor: + +| Outcome | Durable effect | Then what | +| --- | --- | --- | +| `Abandoned { revision }` | A is `Abandoned`, revision +1, `needs_rebaseline` set | proceed to `Start(B)`; the gap is discarded | +| `AlreadyTerminal { status, revision }` | none — A was already `Closed`/`Abandoned` | proceed to `Start(B)`; a successful no-op | +| `RecoveryRequired { reason }` | none; the external-taint marker stays armed | see D1 — the next `coordinate()` performs the stronger inherited-taint recovery | +| `Err(_)` | none, or none beyond a settled outcome carried in the error | **do not treat this as a safely started successor** | + +That last row is the one that matters. A failed abandonment means the stale scope +may still be `Active`, so starting B anyway leaves the zombie live alongside it: +the interval before `Start(B)` can still be claimed `AiExclusive(A)`, and every +overlapping interval afterwards becomes `AiContended` against a scope whose +execution is dead. An adapter must surface the failure, not paper over it by +starting the successor. + +`AbandonScopeError::MarkerClearAfterCompletion { source, completed }` is the one +error that carries an already-settled outcome: the durable transition **did** +succeed and only the trailing marker clear failed. Read `completed` rather than +retrying the abandonment. + +## D1: a missing or `NeverSeen` target forces conservative strong recovery + +`abandon_scope()` on a `ScopeId` with no durable row (`MissingScope`), or one +whose row is still `NeverSeen` (`NeverSeenScope`), returns `RecoveryRequired` and +**leaves the external-taint marker armed**. The next `coordinate()` on that +worktree therefore performs *inherited-taint* recovery — and `protocol::recover` +abandons **every** live scope on a worktree recovering from external taint, not +only the scope the adapter named. + +```text +execution lifecycle not durably observed + ↓ +filesystem interval may contain unknown mutations + ↓ +cannot safely preserve exclusive attribution assumptions + ↓ +force conservative recovery +``` + +A missing row means the scope's `Start` never committed while the execution may +well have run and edited files; a `NeverSeen` row means the identity exists but +no accepted `Start` was ever observed for it. Neither proves the execution +mutated nothing, and the runtime cannot bound what happened inside that interval. + +**The tradeoff, stated plainly:** this is a false-negative cost. Legitimately +live scopes on the same worktree can be abandoned by a recovery they did nothing +to cause, and the evidence for their in-flight intervals is discarded. That cost +is accepted because the alternative is a false positive — attributing an interval +exclusively to a scope while an unobserved execution may have been mutating the +same worktree. **Attribution safety outranks preserving potentially valid +evidence.** + +This is why an adapter must not call `abandon_scope()` speculatively on +`ScopeId`s it cannot vouch for: the cost of a wrong guess is paid by other, +healthy scopes. + +It is not in tension with the ordinary `Abandoned` outcome, whose +`needs_rebaseline`-only recovery preserves live scopes by design. D1 covers only +the recovery-required outcomes, where the stronger recovery is the whole point. + +## The `AiExclusive` attribution boundary + +`Attribution::AiExclusive(scope)` means **exactly one mutation scope was live +over that interval**. It is a statement about scope exclusivity, and nothing +more. + +It is **not** standalone proof that no human edited the worktree. A developer +typing in their editor while an agent's scope is live produces mutations inside +that interval, and the protocol will still label the interval +`AiExclusive(scope)` — the runtime observes trees, not authorship. `AiContended` +likewise means two or more scopes overlapped, not that two humans disagreed. + +Consumers building human-vs-AI authorship claims need evidence beyond this +signal; the protocol deliberately does not supply it. The complementary states +are `AiContended` (more than one live scope) and `IneligibleUnscoped` (no live +scope, or the worktree is unhealthy, externally tainted, or needs rebaseline). + +## Status + +The seam is exported and the contract is recorded. No harness hook, plugin, +extension, or command calls either entrypoint yet; each harness's concrete +`ScopeId` / `EventId` format and its stale-process detection are still open, as +is any repository-scoped cleanup of unowned checkout identities. diff --git a/context/cli/mutation-trace-runtime-coordinator.md b/context/cli/mutation-trace-runtime-coordinator.md index 96feeceb..6bcd00bb 100644 --- a/context/cli/mutation-trace-runtime-coordinator.md +++ b/context/cli/mutation-trace-runtime-coordinator.md @@ -9,11 +9,11 @@ Git worktree, built by the `mutation-cursor-runtime-coordinator` plan `cli/src/services/mutation_trace/runtime/` is a private submodule (`pub(crate) mod runtime;` in `mutation_trace/mod.rs`), registered under the same `#[allow(dead_code)]` precedent as the rest of `mutation_trace`. Every -submodule is declared privately in `runtime/mod.rs`, so `coordinate()`, -`abandon_scope()`, and `reconcile_worktree` are reachable only from within -`runtime` itself (its own tests) for now; a `pub(crate)` re-export is deferred -until a harness adapter needs it, and nothing under `runtime/` is wired into any -hook, command, or `diff_traces` insertion yet. +submodule is declared privately in `runtime/mod.rs`, which re-exports +`coordinate` and `abandon_scope` at `pub(crate)` — reachable crate-wide, contract +in [`mutation-scope-runtime.md`](mutation-scope-runtime.md) — while +`reconcile_worktree` stays `runtime`-internal and nothing under `runtime/` is +wired into any hook, command, or `diff_traces` insertion yet. `runtime` depends on `protocol`/`store`/`types` and on `services::checkout`, never the reverse — this is a structural module boundary, not merely a @@ -231,12 +231,12 @@ baseline → snapshot-failing taint → recovery cycle runs through the public e The `ProtectedWorktree` prefix, lock, snapshot service, protocol-integration pipeline, the public `coordinate()` entrypoint (prefix → DB provider → pipeline → `complete()` on success), and the second `abandon_scope()` entrypoint sharing -that prefix are all implemented, with `runtime/tests.rs` covering `coordinate()` -end to end and both entrypoints driven together; an inherited external-taint -marker is overlaid onto `database_failure` recovery on the next invocation. A -`pub(crate)` re-export of either entrypoint and harness/command wiring remain -future work (`mutation-cursor-external-taint`, -`mutation-cursor-runtime-coordinator`, `mutation-scope-runtime-integration`). +that prefix are all implemented and both `pub(crate)` re-exported from +`runtime/mod.rs` ([`mutation-scope-runtime.md`](mutation-scope-runtime.md)), +with `runtime/tests.rs` covering `coordinate()` end to end and both entrypoints +driven together; an inherited external-taint marker is overlaid onto +`database_failure` recovery on the next invocation. Harness/command wiring +remains future work. See also: [`mutation-trace-ref-reconciliation.md`](mutation-trace-ref-reconciliation.md) (the per-worktree snapshot-ref maintenance pass under the same `WorktreeLock`), diff --git a/context/cli/mutation-trace-scope-abandonment.md b/context/cli/mutation-trace-scope-abandonment.md index 7721d9cf..528efb26 100644 --- a/context/cli/mutation-trace-scope-abandonment.md +++ b/context/cli/mutation-trace-scope-abandonment.md @@ -223,13 +223,14 @@ covering what a single-module test with no real Git cannot: ## Status -The entrypoint, its outcome/error types, its unit coverage, and its cross-runtime -regressions against real Git repositories all exist. The `pub(crate)` re-export -out of `runtime` and the harness-adapter contract document remain future work in -the same plan; no harness, hook, or command calls this yet. +The entrypoint, its outcome/error types, its unit coverage, its cross-runtime +regressions against real Git repositories, its `pub(crate)` re-export out of +`runtime`, and the harness-adapter contract document all exist; no harness, +hook, or command calls this yet. See also: [`mutation-trace-runtime-coordinator.md`](mutation-trace-runtime-coordinator.md), [`mutation-trace-protected-worktree.md`](mutation-trace-protected-worktree.md), [`mutation-trace-external-taint.md`](mutation-trace-external-taint.md), [`mutation-trace-protocol.md`](mutation-trace-protocol.md), -[`mutation-trace-store.md`](mutation-trace-store.md). +[`mutation-trace-store.md`](mutation-trace-store.md), +[`mutation-scope-runtime.md`](mutation-scope-runtime.md). diff --git a/context/context-map.md b/context/context-map.md index 806b2f53..27b05e9b 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -27,9 +27,10 @@ Feature/domain context: - `context/cli/mutation-trace-revision-refinement.md` (the Quint `revision: int` → Rust `WorktreeState::revision: u64` bounded-integer refinement: the private `next_revision` checked-arithmetic helper `commit`/`taint`/`abandon`/`recover` all route through instead of a raw `+ 1`, so a worktree at `revision: u64::MAX` is a guarded no-op/rejection rather than a silent wrap to `0`) - `context/cli/mutation-trace-quint-connect.md` (`#[cfg(test)]`-only Quint Connect model-based-testing harness in `cli/src/services/mutation_trace/mbt/` continuously checking `protocol.rs` against `spec/mutation_cursor.qnt`: the verification-only `mbtAction`/`MbtAction` record-payload transport excluded from comparison, the operation-identity-vs-`MbtStutter` distinction on guarded/no-op branches with its two deterministic regressions, finite ID mapping, the AC5 comparable-state field list, `randomPrepare` staying a single `step` branch, deterministic/generated (500×30, seed-reproducible) test coverage, and the two Nix checks — generic `checks.cli-tests` and dedicated `checks.mutation-trace-quint-connect` — that both require the pinned Quint binary plus the top-level `spec/` directory in `workspaceSrc`'s Nix fileset) - `context/cli/mutation-trace-store.md` (durable persistence for the mutation-cursor protocol in `cli/src/services/mutation_trace/store.rs`, built by the `mutation-cursor-store-persistence` plan: the one-directional `protocol.rs` -> `DurableTransition::between` (pure structural diff) -> `store.rs` (SQL translation) -> `RepositoryAgentTraceDb` boundary; migration `004_mutation_trace_protocol.sql`'s five tables; `AttemptState`/`external_taint` non-persistence; the 8-byte big-endian `BLOB` revision encoding plus explicit non-`Debug` enum codecs; the bounded hot-path `load_worktree` read vs. the cold-path `load_mutation_event`; the public cold-path single-row `load_scope` scope seam that returns one `mutation_trace_scopes` row as `Option` without widening into a projection and, unlike `load_worktree`, never adjudicates worktree identity (a cross-worktree scope is returned as-is, and the mismatch is the caller's decision); the cold-path read-only `load_tree_roots` (one worktree) / `load_all_tree_roots` (repository-wide) durable-tree-SHA queries for ref reconciliation, each a single-statement `UNION` of `cursor_tree`/`before_tree`/`after_tree` read from one DB snapshot; `commit`'s single-`BEGIN IMMEDIATE` CAS batch via `TursoDb::execute_transactional_cas_batch`, distinguishing `Conflict`/retryable-transient/deterministic-`Err` outcomes; and the store's non-goals — no Git/filesystem I/O, no attribution/boundary-kind decisions, no retry-after-`Conflict` loop, no row deletion) -- `context/cli/mutation-trace-runtime-coordinator.md` (imperative-shell runtime layer in `cli/src/services/mutation_trace/runtime/`, built by the `mutation-cursor-runtime-coordinator` plan: the per-worktree OS advisory lock, `runtime::worktree_lock::WorktreeLock::acquire(git_dir, timeout)` at `/sce/mutation-cursor.lock`, bounded `try_lock()` polling with a distinct matchable timeout error and RAII release, and its distinction from the separate checkout-identity-creation lock; the isolated Git snapshot service `runtime::git_snapshot::GitSnapshotService` (documented in `mutation-trace-snapshot-service.md`); and `runtime::coordinator`'s protocol-integration pipeline (`RuntimeBoundary`/`CoordinateOutcome`/`CoordinateError`, the load → recover-if-needed → prepare/commit CAS-retry loop, and its own bounded snapshot-failure taint-retry loop) plus the public `coordinate(repository_root, boundary, open_db)` entrypoint that runs the shared `runtime::protected_worktree` prefix (`WorktreeLock` -> external-taint write-ahead fence -> `WorktreeId`, extracted so a second entrypoint cannot drift from it and documented in `mutation-trace-protected-worktree.md`), invokes the caller-supplied `open_db` provider so DB acquisition falls inside that fence, drives the pipeline under one held lock, and clears the marker through `ProtectedWorktree::complete()` only on success; `runtime/tests.rs` covers the public `coordinate()` API end to end against real linked worktrees and a real Agent Trace DB; an inherited external-taint marker is overlaid onto `protocol::database_failure` recovery on the next invocation, against the single captured snapshot and re-injected across a losing recovery CAS; the private `runtime::ref_reconciliation` per-worktree snapshot-ref maintenance pass (documented in `mutation-trace-ref-reconciliation.md`) shares the same `WorktreeLock`; a `pub(crate)` re-export of `coordinate()` beyond `runtime` and harness/command wiring remain future work) +- `context/cli/mutation-trace-runtime-coordinator.md` (imperative-shell runtime layer in `cli/src/services/mutation_trace/runtime/`, built by the `mutation-cursor-runtime-coordinator` plan: the per-worktree OS advisory lock, `runtime::worktree_lock::WorktreeLock::acquire(git_dir, timeout)` at `/sce/mutation-cursor.lock`, bounded `try_lock()` polling with a distinct matchable timeout error and RAII release, and its distinction from the separate checkout-identity-creation lock; the isolated Git snapshot service `runtime::git_snapshot::GitSnapshotService` (documented in `mutation-trace-snapshot-service.md`); and `runtime::coordinator`'s protocol-integration pipeline (`RuntimeBoundary`/`CoordinateOutcome`/`CoordinateError`, the load → recover-if-needed → prepare/commit CAS-retry loop, and its own bounded snapshot-failure taint-retry loop) plus the public `coordinate(repository_root, boundary, open_db)` entrypoint that runs the shared `runtime::protected_worktree` prefix (`WorktreeLock` -> external-taint write-ahead fence -> `WorktreeId`, extracted so a second entrypoint cannot drift from it and documented in `mutation-trace-protected-worktree.md`), invokes the caller-supplied `open_db` provider so DB acquisition falls inside that fence, drives the pipeline under one held lock, and clears the marker through `ProtectedWorktree::complete()` only on success; `runtime/tests.rs` covers the public `coordinate()` API end to end against real linked worktrees and a real Agent Trace DB; an inherited external-taint marker is overlaid onto `protocol::database_failure` recovery on the next invocation, against the single captured snapshot and re-injected across a losing recovery CAS; the private `runtime::ref_reconciliation` per-worktree snapshot-ref maintenance pass (documented in `mutation-trace-ref-reconciliation.md`) shares the same `WorktreeLock`; `coordinate()` and `abandon_scope()` are now `pub(crate)` re-exported from `runtime/mod.rs` (documented in `mutation-scope-runtime.md`) while the runtime submodules themselves stay private; harness/command wiring remains future work) - `context/cli/mutation-trace-protected-worktree.md` (the shared safety prefix every mutation-cursor runtime entrypoint runs behind, in `cli/src/services/mutation_trace/runtime/protected_worktree.rs`, extracted from `coordinate()` by the `mutation-scope-runtime-integration` plan so a second entrypoint cannot drift from it: `ProtectedWorktree::acquire(repository_root)` running the safety-critical fixed order resolve `git_dir` → `WorktreeLock` (module-owned 10s `WORKTREE_LOCK_TIMEOUT`) → `ExternalTaintMarker::exists()` → `persist()` (fence armed write-ahead of every fallible step that follows, including DB acquisition) → `get_or_create_checkout_id` as `WorktreeId`; the `worktree_id()` / `inherited_external_taint()` / consuming `complete()` surface, where `complete()` clears the marker under the still-held lock and is the only thing that ever clears it while `Drop` releases only the lock; and the one-variant-per-prefix-step `ProtectedWorktreeError` (`GitDirResolution` | `LockAcquisition` | `ExternalTaintMarker { operation, source }` | `CheckoutIdentity`) each entrypoint maps onto its own error surface — `coordinate()` onto exactly the `CoordinateError` variants that step produced before the extraction) - `context/cli/mutation-trace-scope-abandonment.md` (the mutation-cursor runtime's second protected entrypoint in `cli/src/services/mutation_trace/runtime/scope_runtime.rs`, built by the `mutation-scope-runtime-integration` plan and the first production call site for `protocol::abandon`: `abandon_scope(repository_root, scope, open_db) -> Result` retires a scope whose final worktree boundary was never observed, sharing `coordinate()`'s `ProtectedWorktree` prefix but deliberately capturing **no** Git snapshot, pin, diff, reconciliation, scope registration, or worktree initialization — so abandonment is not a `RuntimeBoundary` and needs no Quint change; the classify-before-transition order that recovers what `protocol::abandon`'s uniform guarded no-op cannot report (`load_scope` first for the missing-row and cross-worktree cases the projection seam treats as errors, then `load_worktree` for the `NeverSeen` / terminal / `Active` split); the `Abandoned` / `AlreadyTerminal` / `RecoveryRequired` outcomes and the `InheritedExternalTaint` | `MissingScope` | `NeverSeenScope` | `MissingWorktreeState` recovery reasons; the inherited-marker short-circuit that returns before the DB provider is ever invoked; the fence-completion rule that clears the marker only for a settled abandonment or proven-terminal no-op and leaves it armed for every recovery-required outcome and every error, with `MarkerClearAfterCompletion` carrying the already-settled outcome; the CAS retry bounded by the coordinator's shared `MAX_CAS_RETRY_ATTEMPTS`, settling on a competitor's terminal status rather than overwriting it; and the deliberate false-negative tradeoff whereby a missing or `NeverSeen` target forces conservative strong recovery that may abandon unrelated live scopes, because attribution safety outranks preserving potentially valid evidence) +- `context/cli/mutation-scope-runtime.md` (the crate-visible mutation-trace runtime seam and the lifecycle contract every future harness adapter — Codex, Claude Code, OpenCode, Pi — must uphold, recorded by the `mutation-scope-runtime-integration` plan: the nine `pub(crate)` re-exports in `runtime/mod.rs` (`coordinate`, `RuntimeBoundary`, `CoordinateOutcome`, `CoordinateError`, `ExternalTaintOperation`, `abandon_scope`, `AbandonScopeOutcome`, `AbandonRecoveryReason`, `AbandonScopeError`), with `ExternalTaintOperation` riding through `coordinator.rs`'s own `pub use` because `CoordinateError::ExternalTaintMarker` carries it — so the type becomes crate-visible without `protected_worktree` becoming a public module — while every `mod` declaration stays private and `ProtectedWorktree`/`ProtectedWorktreeError`/`WORKTREE_LOCK_TIMEOUT`/`reconcile_worktree` stay internal, kept clippy-clean by `#[allow(unused_imports)]` on the re-exports per the `services/style.rs` precedent rather than a placeholder consumer; and the adapter obligations themselves — a scope is one independently mutation-capable execution so a concurrent main agent and subagent need distinct `ScopeId`s, `Start`/`Advance`/`Close` semantics including that a failed tool still requires `Advance` (the boundary is an observation, not a successful edit) and that a `ScopeId` is never reused after a terminal status (the `NeverSeen` guard silently refuses to reactivate it), `abandon_scope()` requiring positive staleness evidence and never inferring it from `ActorKind`, the `abandon` → `coordinate(Start(successor))` sequence with what each outcome implies for it including that a failed abandonment must never be treated as a safely started successor, that abandonment is not a `RuntimeBoundary` and needs no Quint change, the D1 tradeoff whereby a missing or `NeverSeen` target deliberately forces conservative strong recovery that may invalidate unrelated live scopes because attribution safety outranks preserving potentially valid evidence, and the attribution boundary that `AiExclusive(scope)` means scope exclusivity only and is not standalone proof that no human edited the worktree; no harness is wired to the seam yet) - `context/cli/mutation-trace-ref-reconciliation.md` (the conservative per-worktree snapshot-ref reconciliation pass in `cli/src/services/mutation_trace/runtime/ref_reconciliation.rs`, built by the `mutation-cursor-ref-reconciliation` plan: `reconcile_worktree(repository_root, open_db)` → `pub(super) reconcile_worktree_inner(.., on_lock_contention)`, both returning `ReconciliationOutcome` (`Reconciled(ReconciliationReport { local_required, retained, deleted })` for a pass that ran | `SkippedNoCheckoutIdentity` — an `Ok`, not an `Err` — when no current checkout identity could be derived), a variant-per-fallible-step `ReconcileError` with no `Other`, and the module-owned `RECONCILIATION_LOCK_TIMEOUT`; the two-invariant model — a strictly per-worktree fail-closed local-consistency check via `load_tree_roots(W)` vs. a repository-wide deletion-safety set via `load_all_tree_roots()` so an `A`-owned ref is retained whenever any worktree still durably needs its tree — run entirely under the same `/sce/mutation-cursor.lock` `WorktreeLock` `coordinate()` holds, with the repository-wide read kept coherent by being one SQL statement / one DB snapshot rather than a repository-global lock; deletes only SCE-owned refs via one atomic `git update-ref --no-deref --stdin`, writes no `mutation_trace_*` row, never arms `ExternalTaintMarker`, runs no `git gc`; imperative durability maintenance below the verified Quint protocol; reclaims orphan/unreferenced refs only for the namespace of a checkout id a current worktree still derives — a namespace no current worktree owns (identity-based: a deleted linked worktree, or checkout-id metadata loss followed by `get_or_create_checkout_id` minting a fresh id on a still-present worktree) is beyond every per-worktree pass and left to a recorded future repository-scoped unowned-namespace operation, so the current pass does not bound all orphan-ref growth; `reconcile_worktree` has no `pub(crate)` re-export and no harness/command wiring yet) - `context/cli/mutation-trace-snapshot-service.md` (the isolated Git snapshot and ref-pinning service `runtime::git_snapshot::GitSnapshotService` in `cli/src/services/mutation_trace/runtime/git_snapshot.rs`: `new` resolving an absolute `git_dir`, `capture_tree` snapshotting staged/unstaged/untracked/deleted worktree state into the repository's normal object database via a throwaway temp index, `pin_tree` protecting a durable tree with a create-only idempotent **direct** `refs/sce/mutation-cursor//` ref, `diff_trees` emitting `patch.rs`-parseable raw diff text; plus the callerless reconciliation substrate — worktree-scoped `list_pins` inventory returning `Result, PinInventoryError>` that rejects a symbolic ref inside the namespace (mutation-cursor pins are direct refs) as `MalformedRef`, matchable separately from a `git for-each-ref` execution failure, and conditional-atomic `delete_pins` running one `git update-ref --no-deref --stdin` transaction of SHA-conditioned deletes — no-dereference so an inventory→delete direct-ref→symref race cannot escape the inventoried namespace, plus a fail-closed pre-check — that aborts whole if any ref changed since inventory; `REF_NAMESPACE` + `pin_ref_prefix` as the single source of truth for the pin path) - `context/cli/mutation-trace-external-taint.md` (the worktree-local mutation-cursor durability boundary in `cli/src/services/mutation_trace/runtime/external_taint.rs`, built by the `mutation-cursor-external-taint` plan: the `ExternalTaintMarker` primitive — `new(git_dir)`/`exists()`/`persist()`/`clear()` over an empty file at `/sce/mutation-cursor-tainted` whose existence is its entire state, `checkout::persist_checkout_id_inner`-style durability (`sync_data` plus best-effort `#[cfg(unix)]` parent-dir `sync_all`), idempotent persist/clear, `NotFound`-on-clear as success, no `Drop` deletion — as the concrete runtime refinement of the abstract `ProtocolState.external_taint`; armed by the reshaped `coordinate()` entrypoint write-ahead after the `WorktreeLock` and before Agent Trace DB acquisition (a caller-supplied DB provider closure), cleared only on a successful `CoordinateOutcome`, with dedicated fail-closed pre-commit `CoordinateError::ExternalTaintMarker` (`Inspect`/`Persist` only)/`AgentTraceDbUnavailable` variants plus a post-commit `MarkerClearAfterCommit { source, committed }` that carries the durable outcome so a failed trailing clear never hides a committed `MutationEvent`; an inherited marker seeds an invocation-local `external_taint_pending` flag that overlays `protocol::database_failure` onto each freshly loaded projection so `recover` runs once against the captured snapshot, held across a losing recovery CAS and cleared once it lands) diff --git a/context/overview.md b/context/overview.md index fb465be0..e9ea04bc 100644 --- a/context/overview.md +++ b/context/overview.md @@ -2,7 +2,7 @@ This repository maintains shared assistant configuration for OpenCode, Claude, Pi, and Codex from a single canonical Pkl authoring source. One typed workflow catalog owns the six workflows' shared identity and target routing metadata, while canonical workflow/phase modules own behavior and migrated package-local documents, and target renderers own formatting. Generated target layouts are ephemeral: repository builds consume a pre-Cargo generated payload through `SCE_CLI_GENERATED_INPUT_DIR`, crates.io and Flatpak stage packaging-only fallbacks, and `config/.opencode`, `config/.claude`, `config/.pi`, `config/.agents`, and the generated working-tree SCE config schema are not committed; versioned SCE config schema snapshots live under `schema/v/`. The catalog also marks a workflow `optional` — currently only `brownfield` — which changes nothing about generation and is projected into a generated `config/optional-workflows.json` manifest for install-time consumers. `nix run .#pkl-check-generated` preserves its exact 141-path artifact, metadata/package, phase-reference, internal-reference, optional-workflow-manifest, workflow-orchestration, OpenCode-permission, required-path, and forbidden-path checks while delegating deterministic payload production and inventories to the shared generated-input producer; `nix flake check` runs the same contract. The target matrix contains one manual OpenCode profile plus Claude, Pi, and Codex; the former automated OpenCode profile has been removed. A fourth Pkl renderer, `config/pkl/renderers/codex-content.pkl`, also consumes the same canonical workflow composition to emit skills-only Codex output under `config/.agents/skills/**` (no per-target frontmatter, matching Pi, and no command/prompt layer). Each of the six catalog workflow skills (not `sce-decision`, which has no user-facing entrypoint on any target) additionally carries a Codex-only `agents/openai.yaml`, rendered by `config/pkl/renderers/codex-metadata.pkl` from the shared catalog's `title`/`description` plus an authored `default_prompt`, with `policy.allow_implicit_invocation: false` so these stateful lifecycle workflows activate only from explicit `$sce-` invocation or Codex's `/skills` discovery, never from conversational relevance alone. The Codex renderer also emits a Codex hook registration file (`config/.codex/hooks.json`) and its fail-open install-guidance hook script (`config/.codex/hooks/run-sce-or-show-install-guidance.sh`), both routing every registered lifecycle event through the single command `sce hooks codex`; the generated command resolves the Git root at invocation time and safely invokes the helper from nested cwd or spaced repository paths, failing open silently when Git-root resolution fails. `sce setup --codex` (and `--all`) now installs it as a fourth `SetupTarget`; setup merges the user-owned `.codex/hooks.json` through the shared structural Codex hook-config service, preserving unrelated valid handlers and rejecting malformed or Codex-invalid documents before the atomic swap. `sce doctor` diagnoses that file per required registration (`PresentAndCurrent`/`Missing`/`Stale`, or `Malformed` for the whole document) rather than by whole-document equality, and separately reports whether Codex has actually marked each structurally current registration trusted by reading (never writing) Codex's own `$CODEX_HOME/config.toml` hook-trust state; `sce doctor --fix` repairs structurally unhealthy registrations through the same merge service but never touches trust state (see `context/sce/doctor-human-text-contract.md`). `sce hooks codex` now exists as a typed dispatcher (`cli/src/services/hooks/codex/`): it parses the raw hook JSON into a `CodexHookEvent` and classifies `(hook_event_name, tool_name)` into `UserPromptSubmit`, `Stop`, `PreToolUse(Bash)`, `PostToolUse(apply_patch)`, or a fail-open `NoOp` fallthrough covering every other combination — including `apply_patch` under `PreToolUse`. `UserPromptSubmit` and `Stop` each capture one `messages`/`parts` row (`role="user"`/`role="assistant"`) into the repository Agent Trace DB under the idempotent `cx_` session prefix, `PreToolUse(Bash)` delegates to the existing Bash policy engine, and `PostToolUse(apply_patch)` parses Codex's own `apply_patch` text, resolves paths from the event `cwd` against the real Git root into safe repository-relative paths, normalizes Add/Update evidence into an SCE unified diff under deterministic event-scoped synthetic line identities derived from `tool_use_id`, and persists it as one `diff_traces` row when non-empty (see `context/sce/codex-integration-runtime.md`) — while invalid cwd/path resolution and malformed STDIN payloads fail open with empty stdout, reported model IDs are preserved without fabricated provider prefixes, and missing/blank apply_patch sessions produce no evidence. Delete-File operations and Bash-triggered filesystem mutations remain untracked for Codex. -It also includes a Rust CLI (`sce`) for Shared Context Engineering workflows: auth, config inspection, setup, doctor, Agent Trace hooks and synchronization, bash-policy evaluation, and repository-scoped Agent Trace storage infrastructure. See `context/architecture.md` for module-level boundaries and `context/context-map.md` for the full domain file index. A new pure, dependency-free `cli/src/services/mutation_trace/` module now exists as a Rust refinement of the verified `spec/mutation_cursor.qnt` mutation-cursor protocol; it currently carries domain types and the full pure action set — `prepare`/`commit` transition logic, attribution/mutation-event materialization, snapshot-failure/database-failure taint actions, scope abandonment, and recovery with an explicit observed-tree input — is registered with `#[allow(dead_code)]`. A separate `cli/src/services/mutation_trace/store.rs` persistence layer (built out by the `mutation-cursor-store-persistence` plan) now provides a real, CAS-guarded database call site against the repository-scoped Agent Trace DB, and a `cli/src/services/mutation_trace/runtime/` coordinator layer (built out by the `mutation-cursor-runtime-coordinator` plan) adds the per-worktree advisory lock, the isolated Git snapshot/ref-pinning service, and a public `coordinate()` entrypoint that drives the protocol against a real worktree under that lock, plus a per-worktree `ref_reconciliation` maintenance pass (built out by the `mutation-cursor-ref-reconciliation` plan) that, only for the namespace of a checkout id a current worktree still derives, reclaims orphaned SCE-owned snapshot refs under that same lock while retaining every tree any durable mutation-cursor state still references (a namespace no current worktree owns — via a deleted linked worktree or checkout-id metadata loss/recreation — is out of reach and left to future repository-scoped work); the module is still not wired into any hook or command (see `context/cli/mutation-trace-protocol.md`, `context/cli/mutation-trace-runtime-coordinator.md`, and `context/cli/mutation-trace-ref-reconciliation.md`). +It also includes a Rust CLI (`sce`) for Shared Context Engineering workflows: auth, config inspection, setup, doctor, Agent Trace hooks and synchronization, bash-policy evaluation, and repository-scoped Agent Trace storage infrastructure. See `context/architecture.md` for module-level boundaries and `context/context-map.md` for the full domain file index. A new pure, dependency-free `cli/src/services/mutation_trace/` module now exists as a Rust refinement of the verified `spec/mutation_cursor.qnt` mutation-cursor protocol; it currently carries domain types and the full pure action set — `prepare`/`commit` transition logic, attribution/mutation-event materialization, snapshot-failure/database-failure taint actions, scope abandonment, and recovery with an explicit observed-tree input — is registered with `#[allow(dead_code)]`. A separate `cli/src/services/mutation_trace/store.rs` persistence layer (built out by the `mutation-cursor-store-persistence` plan) now provides a real, CAS-guarded database call site against the repository-scoped Agent Trace DB, and a `cli/src/services/mutation_trace/runtime/` coordinator layer (built out by the `mutation-cursor-runtime-coordinator` plan) adds the per-worktree advisory lock, the isolated Git snapshot/ref-pinning service, and a public `coordinate()` entrypoint that drives the protocol against a real worktree under that lock, plus a per-worktree `ref_reconciliation` maintenance pass (built out by the `mutation-cursor-ref-reconciliation` plan) that, only for the namespace of a checkout id a current worktree still derives, reclaims orphaned SCE-owned snapshot refs under that same lock while retaining every tree any durable mutation-cursor state still references (a namespace no current worktree owns — via a deleted linked worktree or checkout-id metadata loss/recreation — is out of reach and left to future repository-scoped work). That runtime now has a second protected entrypoint alongside `coordinate()`: `cli/src/services/mutation_trace/runtime/scope_runtime.rs`'s `abandon_scope()` (built out by the `mutation-scope-runtime-integration` plan) retires a scope whose final worktree boundary was never observed — a dead agent process leaves no `Close` behind — sharing `coordinate()`'s extracted `runtime/protected_worktree.rs` safety prefix but deliberately capturing no Git snapshot, so abandonment is not a `RuntimeBoundary` and needs no Quint change. Both entrypoints are now reachable crate-wide through nine `pub(crate)` re-exports in `runtime/mod.rs` while every module behind them stays private, and the lifecycle contract every future harness adapter (Codex, Claude Code, OpenCode, Pi) must uphold is recorded in `context/cli/mutation-scope-runtime.md`; the module is still not wired into any hook or command (see `context/cli/mutation-trace-protocol.md`, `context/cli/mutation-trace-runtime-coordinator.md`, `context/cli/mutation-trace-ref-reconciliation.md`, `context/cli/mutation-trace-protected-worktree.md`, `context/cli/mutation-trace-scope-abandonment.md`, and `context/cli/mutation-scope-runtime.md`). The generated `/next-task` workflow persists task-level context-synchronization lifecycle state in each plan (`pending`, `synced`, or `blocked`) so unresolved task synchronization debt survives a session boundary and gates new implementation. Successful `/next-task` execution hands task synchronization an explicit, pre-edit-Git-baseline-relative changed-file list plus implementation, verification, done-check, plan-update, and context-impact evidence, recorded directly on the completed task (`Completed`, `Files changed`, `Result`, `Verify`, `Context impact`, `Context synchronization`); the five-file root context pass remains mandatory. A later-session sync-debt retry reads that same completed task record directly from the plan by plan path and task ID, with no separate persisted synchronization handoff. `/validate` is validation-only: it runs final checks, writes the Validation Report, and reports `validated`, `failed`, or `blocked` without plan-level context synchronization. diff --git a/context/plans/mutation-scope-runtime-integration.md b/context/plans/mutation-scope-runtime-integration.md index 51fc386f..5b582f20 100644 --- a/context/plans/mutation-scope-runtime-integration.md +++ b/context/plans/mutation-scope-runtime-integration.md @@ -39,7 +39,7 @@ How this plan is proven complete. Each criterion is observable and names the check that proves it. `/validate` runs these checks; no task in the stack performs final validation. -- [ ] AC1: The `coordinate()` path runs through the shared protected-worktree +- [x] AC1: The `coordinate()` path runs through the shared protected-worktree primitive with its current externally observable ordering and error semantics intact: worktree lock → external marker inspect/persist → checkout identity → DB provider → snapshot/recovery/protocol/CAS → explicit marker clear, with @@ -47,61 +47,61 @@ performs final validation. `AgentTraceDbUnavailable`, and `MarkerClearAfterCommit { source, committed }` produced on exactly the same conditions as before. - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::coordinator::` — the pre-existing fence, lock-contention, and `MarkerClearAfterCommit` tests pass unmodified in assertion content. -- [ ] AC2: Abandoning an `Active` scope changes only that scope's status to +- [x] AC2: Abandoning an `Active` scope changes only that scope's status to `Abandoned`, advances its worktree `revision` by exactly one, sets `needs_rebaseline = true`, leaves `cursor_tree` / `tainted` / `failure_kind` unchanged, and writes no `mutation_trace_events`, `mutation_trace_event_active_scopes`, or `mutation_trace_processed_events` row. - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::scope_runtime::` — a test reads the durable rows back after a successful abandonment and asserts each field and each absent row. -- [ ] AC3: `abandon_scope()` performs no Git snapshot, tree pin, tree diff, ref +- [x] AC3: `abandon_scope()` performs no Git snapshot, tree pin, tree diff, ref reconciliation, scope registration, or worktree initialization; it reads and transitions only already-durable mutation-scope state. - Validate: inspect `cli/src/services/mutation_trace/runtime/scope_runtime.rs` — it names none of `GitSnapshotService`, `SnapshotCapture`, `capture_tree`, `pin_tree`, `diff_trees`, `reconcile_worktree`, `initialize_worktree`, or `register_scope`; confirm with `rg -n 'GitSnapshotService|SnapshotCapture|capture_tree|pin_tree|diff_trees|reconcile_worktree|initialize_worktree|register_scope' cli/src/services/mutation_trace/runtime/scope_runtime.rs` returning no non-test hit. -- [ ] AC4: A target scope already `Closed` or `Abandoned` settles as a successful +- [x] AC4: A target scope already `Closed` or `Abandoned` settles as a successful terminal no-op: no revision change, no row write, and the external-taint marker is cleared. - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::scope_runtime::` — separate `Closed` and `Abandoned` tests assert the outcome variant, the unchanged durable revision, and that the marker file is gone. -- [ ] AC5: A target `ScopeId` with no durable row, and one whose row is +- [x] AC5: A target `ScopeId` with no durable row, and one whose row is `NeverSeen`, both return the recovery-required outcome, leave the external-taint marker armed on disk, and commit no normal abandonment — no durable row changes, and the worktree revision does not advance. This deliberately forces the next `coordinate()` into conservative strong recovery (see **Design decisions**, D1). - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::scope_runtime::` — separate missing-scope and `NeverSeen` tests assert the reason variant, that `/sce/mutation-cursor-tainted` still exists, and that the worktree revision and every scope status are unchanged. -- [ ] AC6: An external-taint marker already present when `abandon_scope()` is +- [x] AC6: An external-taint marker already present when `abandon_scope()` is called returns the recovery-required outcome for that reason without clearing the marker and without invoking the caller-supplied DB provider. - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::scope_runtime::` — a test arms the marker first, passes a provider that sets a flag and returns `Err`, and asserts the recovery-required reason, the still-present marker, and that the provider flag was never set. -- [ ] AC7: A target scope whose durable `worktree_id` is not the `WorktreeId` this +- [x] AC7: A target scope whose durable `worktree_id` is not the `WorktreeId` this invocation derived from its own checkout is rejected as an error, and neither the scope row nor either worktree row is modified. - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::` — a two-linked-worktree test abandons worktree A's scope through worktree B's checkout, asserts the error, and asserts both worktree revisions and the scope status are unchanged. -- [ ] AC8: A CAS conflict makes `abandon_scope()` reload the durable projection and +- [x] AC8: A CAS conflict makes `abandon_scope()` reload the durable projection and recompute the abandonment from that fresh state, bounded by the same retry limit the coordinator uses; when the competing writer left the scope terminal, the retry settles as the terminal no-op outcome rather than overwriting it. - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::` — a real-thread CAS-race test against one on-disk DB asserts the settled outcome and that the scope's final status is the competitor's, not a second abandonment. -- [ ] AC9: An `Active` target on a worktree at `revision: u64::MAX` produces an +- [x] AC9: An `Active` target on a worktree at `revision: u64::MAX` produces an explicit revision-exhaustion error, never an abandonment success and never a terminal no-op. - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::scope_runtime::` — a test seeds `u64::MAX` and asserts the distinct error variant. -- [ ] AC10: With no inherited marker, a DB-provider `Err` or a persistence failure +- [x] AC10: With no inherited marker, a DB-provider `Err` or a persistence failure after the marker is armed leaves the marker on disk; a successful abandonment and a proven-terminal no-op each clear it; a `clear()` failure after either completes returns an error that carries the already-completed outcome rather than reporting the durable transition as failed. - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::` — one test per row of that table, the clear-failure test asserting the carried outcome matches the durable state. -- [ ] AC11: Against a real Git repository and a real repository-scoped Agent Trace +- [x] AC11: Against a real Git repository and a real repository-scoped Agent Trace DB: `Start(A)` → edit → `abandon_scope(A)` → a further unobserved edit → `coordinate(Start(B))` leaves the worktree cursor at the tree observed at `Start(B)`, emits no `MutationEvent` for the ambiguous A→B interval, leaves A `Abandoned`, and leaves B `Active`. - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::tests::` — the end-to-end sequence test asserts the cursor tree, the absence of any `mutation_trace_events` row for the gap, and both scope statuses. -- [ ] AC12: Abandoning stale scope A while unrelated scope B is legitimately +- [x] AC12: Abandoning stale scope A while unrelated scope B is legitimately `Active` on the same worktree leaves B `Active` through the subsequent `needs_rebaseline` recovery. - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::tests::` — the surviving-scope test asserts B's status after the next `coordinate()` call. -- [ ] AC13: `runtime/mod.rs` re-exports, at `pub(crate)`, exactly `coordinate`, +- [x] AC13: `runtime/mod.rs` re-exports, at `pub(crate)`, exactly `coordinate`, `CoordinateError`, `CoordinateOutcome`, `ExternalTaintOperation`, `RuntimeBoundary`, `abandon_scope`, `AbandonScopeError`, and `AbandonScopeOutcome` (with its reason type). `ExternalTaintOperation` is part @@ -117,7 +117,7 @@ performs final validation. `ProtectedWorktree`, `ProtectedWorktreeError`, and `WORKTREE_LOCK_TIMEOUT` remain internal to `runtime`. - Validate: inspect `cli/src/services/mutation_trace/runtime/mod.rs` for exactly those re-exports and confirm every `mod` declaration there is still private (`rg -n '^\s*(pub(\(crate\))?\s+)?mod |pub\(crate\) use' cli/src/services/mutation_trace/runtime/mod.rs`), and `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` stays clean. -- [ ] AC14: `spec/mutation_cursor.qnt`, the Quint refinement matrix in +- [x] AC14: `spec/mutation_cursor.qnt`, the Quint refinement matrix in `cli/src/services/mutation_trace/mod.rs`, `protocol.rs`'s transition semantics, `004_mutation_trace_protocol.sql`, and the migration set carry no change attributable to this PR. The baseline is this PR's own base branch, `ref-rec`, @@ -125,7 +125,7 @@ performs final validation. the runtime coordinator already exist on `ref-rec`, so a `main` baseline would report the entire stack below this PR as if it were this PR's change. - Validate: `git fetch origin` first, then `git diff --stat origin/ref-rec...HEAD -- spec/mutation_cursor.qnt cli/src/services/mutation_trace/protocol.rs cli/migrations/agent-trace-repository/` produces empty output. Use `origin/ref-rec`, not a local `ref-rec`, which can lag behind a rewritten base and would report the whole rebased stack as this PR's change. Additionally, the `mutation-trace-quint-connect` and Quint checks inside `nix flake check` stay green. -- [ ] AC15: `context/cli/mutation-scope-runtime.md` exists and states, in the +- [x] AC15: `context/cli/mutation-scope-runtime.md` exists and states, in the repository's own terms: a mutation scope is one independently mutation-capable execution (so concurrent main agent and subagent need distinct `ScopeId`s); `Start`/`Advance`/`Close` semantics including that a failed tool still requires @@ -574,13 +574,88 @@ which is precisely why there is nothing left for this one to decide. to follow via RAII `tempfile::TempDir`. - Context synchronization: synced -- [ ] T05: `Export the runtime seam and record the adapter contract` (status:todo) +- [x] T05: `Export the runtime seam and record the adapter contract` (status:done) - Task ID: T05 - Scope: In — `pub(crate) use` re-exports in `runtime/mod.rs` for `coordinate`, `CoordinateError`, `CoordinateOutcome`, `ExternalTaintOperation`, `RuntimeBoundary`, `abandon_scope`, `AbandonScopeError`, `AbandonScopeOutcome` and its reason type — conceptually `pub(crate) use coordinator::{coordinate, CoordinateError, CoordinateOutcome, ExternalTaintOperation, RuntimeBoundary};` plus the `scope_runtime` names, `ExternalTaintOperation` riding through `coordinator`'s own `pub use` of it because `CoordinateError::ExternalTaintMarker` carries it; keeping the `git_snapshot`, `external_taint`, `worktree_lock`, `ref_reconciliation`, and `protected_worktree` **modules** private and re-exporting nothing else from any of them (`ProtectedWorktree`, `ProtectedWorktreeError`, and `WORKTREE_LOCK_TIMEOUT` stay internal); new `context/cli/mutation-scope-runtime.md` plus its `context/context-map.md` and `context/overview.md` index entries. Out — any harness, hook, or command wiring; any change to the runtime implementation, including moving `ExternalTaintOperation` or `WORKTREE_LOCK_TIMEOUT` back out of `protected_worktree.rs`; the per-task context updates T01–T04 each own for their own domain files. - Dependencies: T04 - Done when: the eight names above are reachable as `crate::services::mutation_trace::runtime::*`, including `ExternalTaintOperation` so a crate-level caller can match `CoordinateError::ExternalTaintMarker`; the five modules remain private and nothing else from them is reachable; `clippy --all-targets -- -D warnings` is clean with no placeholder consumer added to satisfy it; `context/cli/mutation-scope-runtime.md` states the scope-identity rule, the `Start`/`Advance`/`Close` semantics, the positive-evidence requirement for `abandon_scope()` and the prohibition on inferring staleness from `ActorKind`, the successor-scope sequence and what each outcome implies for it (including that a failed abandonment must not be treated as a safely started successor), that abandonment is not a `RuntimeBoundary` and requires no Quint change, the D1 strong-recovery tradeoff for a missing or `NeverSeen` target, and the `AiExclusive` attribution boundary; `context-map.md` and `overview.md` name the new module and file. - Verify: `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings`; `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml`; read `context/cli/mutation-scope-runtime.md` against the shipped `scope_runtime.rs` signatures. - - Context synchronization: pending + - Completed: 2026-09-03 + - Files changed: + - `cli/src/services/mutation_trace/runtime/mod.rs` + - `context/cli/mutation-scope-runtime.md` (new) + - `context/context-map.md` + - `context/overview.md` + - Result: `runtime/mod.rs` gained two `#[allow(unused_imports)] pub(crate) use` + blocks — `coordinator::{coordinate, CoordinateError, CoordinateOutcome, + ExternalTaintOperation, RuntimeBoundary}` and `scope_runtime::{abandon_scope, + AbandonRecoveryReason, AbandonScopeError, AbandonScopeOutcome}` — nine names + total (the plan's "eight names ... and its reason type" is `AbandonRecoveryReason` + counted separately, matching the file's own header count). Every `mod` + declaration in the file stays private; nothing else is re-exported. The + `#[allow(unused_imports)]` on each `use` block follows the repository's + existing precedent for a seam with no consumer yet (`services/style.rs`, + `services/hooks/codex/apply_patch/mod.rs`) rather than the module-level + `#[allow(dead_code)]` on `pub mod mutation_trace`, which covers unused items, + not unused re-exports, and rather than a placeholder consumer, which the + plan's constraints forbid. Reachability was proven by a temporary crate-level + test module importing and using all nine names through + `crate::services::mutation_trace::runtime::*` (compiled and passed), and + module privacy was proven by a second temporary test module attempting direct + imports of `protected_worktree`, `worktree_lock`, `git_snapshot`, + `external_taint`, and `ref_reconciliation`, each failing with `E0603: module + ... is private`; both probes were reverted before this task's own edit, and + `git status`/`git diff` on `cli/src/services/mod.rs` confirm it carries no + diff from this task. New `context/cli/mutation-scope-runtime.md` records the + adapter contract: the scope-identity rule (one independently mutation-capable + execution per `ScopeId`, so a concurrent main agent and subagent need distinct + ids); `Start`/`Advance`/`Close`/`Flush` semantics including that a failed tool + still requires `Advance` (a boundary observes the worktree, not a successful + edit) and that a `ScopeId` is never reused after a terminal status (the + `NeverSeen` guard silently refuses to reactivate it); that `coordinate()` + registers `(worktree_id, actor_kind)` identity on every scope-carrying + variant, not only `Start`, with a mismatch surfacing as + `CoordinateError::ScopeIdentityConflict`; the positive-evidence requirement + for `abandon_scope()` and the explicit prohibition on inferring staleness + from `ActorKind`; the abandon → `coordinate(Start(successor))` sequence as an + outcome table, including that a failed abandonment (an `Err`, `Abandoned` + excluded) must never be treated as a safely started successor; that + abandonment is not a `RuntimeBoundary` and needs no Quint change; D1's + strong-recovery tradeoff for a missing or `NeverSeen` target verbatim from the + plan's **Design decisions**; and the `AiExclusive` attribution boundary — that + it states scope exclusivity only, never standalone proof no human edited the + worktree. `context-map.md` gained one index entry in file order after + `mutation-trace-scope-abandonment.md` and before + `mutation-trace-ref-reconciliation.md`; `overview.md`'s existing + mutation-trace status paragraph gained a sentence naming the new + `abandon_scope()` entrypoint, the nine re-exports, and the new context file, + replacing its prior "the module is still not wired into any hook or command" + citation list with the expanded one including the two new files. A pre-write + cross-check against the shipped `scope_runtime.rs`/`coordinator.rs` found and + corrected two drafting errors before this record was written: `Flush` carries + no fields (not a `worktree` field — corrected from an earlier draft that + stated `Flush { worktree }`), and `coordinate()`'s `hook_identity` / + `register_scope` call runs for all three scope-carrying variants, not only + `Start`. No production runtime logic, signature, or behavior changed; no code + comments were added, consistent with T01/T03/T04's precedent on this plan. + - Verify results: + - `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` — passed, clean. + - `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml` — passed: 951 passed, 0 failed, 0 ignored. + - `./scripts/run-cli-cargo.sh fmt --manifest-path cli/Cargo.toml -- --check` — passed. + - `rg -n '^\s*(pub(\(crate\))?\s+)?mod |pub\(crate\) use' cli/src/services/mutation_trace/runtime/mod.rs` — confirmed every `mod` line has no `pub`/`pub(crate)` prefix and exactly two `pub(crate) use` blocks are present. + - Read `context/cli/mutation-scope-runtime.md` against the shipped `scope_runtime.rs`/`coordinator.rs` signatures — confirmed, after the two corrections above. + - Context impact: this task's own deliverable *is* the context change — a new + root-adjacent domain file plus its two index entries. No production code + behavior changed beyond the re-export visibility itself (a compile-time-only + seam widening); `coordinate()`'s and `abandon_scope()`'s signatures, outcomes, + and error variants are unchanged. Durable context affected: + `context/cli/mutation-scope-runtime.md` (new), `context/context-map.md`, + `context/overview.md`. Two sibling domain files this task's own change made + stale were also corrected during synchronization: + `context/cli/mutation-trace-runtime-coordinator.md` and + `context/cli/mutation-trace-scope-abandonment.md` (both previously described + the `pub(crate)` re-export as future work). + - Context synchronization: synced ## Open questions @@ -590,3 +665,50 @@ which is precisely why there is nothing left for this one to decide. the cheaper option is to leave the abandonment CAS race in `scope_runtime.rs`'s own inline tests (T03) and drop it from T04, since nothing about the race needs real Git. Not blocking — T03's `Done when` already covers the behavior. + +## Validation Report + +**Status:** validated +**Date:** 2026-09-03 + +### Commands run + +- `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::` -> exit 0 (288 passed; 0 failed; 663 filtered out) +- `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml` -> exit 0 (951 passed; 0 failed; 0 ignored) +- `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` -> exit 0 (clean, no placeholder consumer) +- `./scripts/run-cli-cargo.sh fmt --manifest-path cli/Cargo.toml -- --check` -> exit 0 (clean) +- `nix flake check` -> exit 0 (all checks passed, incl. `sce-cli-clippy`, `sce-cli-fmt`, `sce-cli-tests`, `sce-mutation-trace-quint-connect`) +- `nix run .#pkl-check-generated` -> exit 0 (141 files, inventory sha256 5516770e…) +- `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::scope_runtime::` -> exit 0 (14 passed) +- `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::coordinator::` -> exit 0 (26 passed, assertions unmodified) +- `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::runtime::tests::` -> exit 0 (27 passed) +- `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::mutation_trace::store::` -> exit 0 (86 passed) +- `rg -n 'GitSnapshotService|SnapshotCapture|capture_tree|pin_tree|diff_trees|reconcile_worktree|initialize_worktree|register_scope' cli/src/services/mutation_trace/runtime/scope_runtime.rs` -> exit 1 (no match) +- `rg -n '^\s*(pub(\(crate\))?\s+)?mod |pub\(crate\) use' cli/src/services/mutation_trace/runtime/mod.rs` -> exit 0 (every `mod` private; two `pub(crate) use` blocks) +- `git fetch origin && git diff --stat origin/ref-rec...HEAD -- spec/mutation_cursor.qnt cli/src/services/mutation_trace/protocol.rs cli/migrations/agent-trace-repository/` -> exit 0 (empty output; also empty for `cli/src/services/mutation_trace/mod.rs` and `004_mutation_trace_protocol.sql`) + +### Success-criteria verification + +- [x] AC1: `coordinate()` runs through the shared protected-worktree primitive with ordering and error semantics intact -> `services::mutation_trace::runtime::coordinator::` — 26 passed; the fence, lock-contention, and `MarkerClearAfterCommit` tests pass with assertion content unmodified (confirmed against T01 record). +- [x] AC2: Abandoning an `Active` scope changes only that scope's status, advances revision by one, sets `needs_rebaseline`, writes no event rows -> `scope_runtime::tests::abandoning_a_live_scope_writes_only_the_scope_and_worktree_rows` passed. +- [x] AC3: `abandon_scope()` performs no snapshot/pin/diff/reconciliation/registration/init -> `rg` over `scope_runtime.rs` returns no match (exit 1); the module reads only durable mutation-scope state. +- [x] AC4: A `Closed`/`Abandoned` target settles as a successful terminal no-op with the marker cleared -> `scope_runtime::tests::a_closed_scope_settles_as_a_terminal_no_op` and `…an_already_abandoned_scope_settles_as_a_terminal_no_op` passed. +- [x] AC5: A missing and a `NeverSeen` target both return recovery-required, leave the marker armed, commit no abandonment -> `scope_runtime::tests::a_missing_scope_row_requires_recovery_and_leaves_the_fence_armed` and `…a_never_seen_scope_requires_recovery_and_leaves_the_fence_armed` passed. +- [x] AC6: A pre-existing marker returns recovery-required for that reason without clearing it and without invoking the DB provider -> `scope_runtime::tests::an_inherited_marker_requires_recovery_without_consulting_the_db_provider` passed. +- [x] AC7: A target owned by another `WorktreeId` is rejected as an error writing nothing -> `runtime::tests::abandoning_a_scope_through_another_worktrees_checkout_is_rejected_without_writing` passed; `scope_runtime::tests::a_scope_owned_by_another_worktree_is_rejected_without_writing` passed. +- [x] AC8: A CAS conflict reloads and recomputes within the coordinator's retry limit; a competitor-terminal scope settles as the terminal no-op -> `runtime::tests::a_real_thread_cas_race_settles_on_the_competitors_terminal_status` and `scope_runtime::tests::a_cas_conflict_whose_competitor_ended_the_scope_settles_as_a_terminal_no_op` / `…a_cas_conflict_recomputes_the_abandonment_from_fresh_state` passed. +- [x] AC9: An `Active` target at `revision: u64::MAX` produces a distinct revision-exhaustion error -> `scope_runtime::tests::a_live_scope_on_an_exhausted_revision_is_a_distinct_error` passed. +- [x] AC10: Marker stays armed on DB-provider `Err` / persistence failure; cleared on abandonment and terminal no-op; a `clear()` failure returns an error carrying the completed outcome -> `scope_runtime::tests::a_db_provider_failure_leaves_the_fence_armed`, `…a_persistence_failure_rolls_back_the_whole_transition_and_leaves_the_fence_armed`, `…a_marker_clear_failure_carries_the_already_settled_outcome` passed. +- [x] AC11: Real Git + real repo-scoped DB: `Start(A)` → edit → `abandon_scope(A)` → edit → `coordinate(Start(B))` leaves the cursor at `Start(B)`'s tree, emits no event for the gap, A `Abandoned`, B `Active` -> `runtime::tests::an_abandoned_scope_rebaselines_the_successor_start_without_evidence_for_the_gap` passed. +- [x] AC12: Abandoning stale A leaves unrelated live B `Active` through the recovery -> `runtime::tests::abandoning_a_stale_scope_leaves_an_unrelated_live_scope_active_through_the_recovery` passed. +- [x] AC13: `runtime/mod.rs` re-exports exactly `coordinate`, `CoordinateError`, `CoordinateOutcome`, `ExternalTaintOperation`, `RuntimeBoundary`, `abandon_scope`, `AbandonRecoveryReason`, `AbandonScopeError`, `AbandonScopeOutcome`; every `mod` private -> inspected `mod.rs` (two `pub(crate) use` blocks, all `mod` lines private); `clippy --all-targets -- -D warnings` clean. +- [x] AC14: No change attributable to this PR in `spec/mutation_cursor.qnt`, the Quint refinement matrix, `protocol.rs`, `004_mutation_trace_protocol.sql`, or the migration set, baselined on `origin/ref-rec` -> `git diff --stat origin/ref-rec...HEAD` over those paths is empty; `nix flake check` Quint checks green. +- [x] AC15: `context/cli/mutation-scope-runtime.md` exists and states each required item -> read in full: scope-identity rule, `Start`/`Advance`/`Close` semantics (failed tool still requires `Advance`, no `ScopeId` reuse after terminal), positive-staleness-evidence requirement and the `ActorKind` prohibition, the abandon → successor-`Start` outcome table (including that a failed abandonment is not a safely started successor), abandonment is not a `RuntimeBoundary` and needs no Quint change, D1's strong-recovery tradeoff, and the `AiExclusive` attribution boundary — all present and consistent with the shipped `scope_runtime.rs` / `coordinator.rs`. + +### Failed checks and follow-ups + +- None. + +### Residual risks + +- No harness adapter exercises either entrypoint yet, so `abandon_scope()` has no production caller; the re-export blocks carry `#[allow(unused_imports)]` and coverage rests entirely on the inline and integration test suites. This is the plan's explicit end state, not a regression.