From 87679e18c5fac9b766716e83ec5600d3ce3f34b2 Mon Sep 17 00:00:00 2001 From: David Abram Date: Thu, 27 Aug 2026 13:09:09 +0200 Subject: [PATCH 1/5] context: Add mutation cursor store persistence plan Define the next build-out for durable mutation-cursor protocol state in the repository-scoped Agent Trace DB. The plan covers additive schema migration, explicit codecs, structural durable-transition projection, transactional CAS persistence, concurrency and restart coverage, and documentation. Plan: mutation-cursor-store-persistence (T01-T11) Co-authored-by: SCE --- .../mutation-cursor-store-persistence.md | 212 ++++++++++++++++++ 1 file changed, 212 insertions(+) create mode 100644 context/plans/mutation-cursor-store-persistence.md diff --git a/context/plans/mutation-cursor-store-persistence.md b/context/plans/mutation-cursor-store-persistence.md new file mode 100644 index 00000000..b2a744d1 --- /dev/null +++ b/context/plans/mutation-cursor-store-persistence.md @@ -0,0 +1,212 @@ +# Plan: mutation-cursor-store-persistence + +## Change summary + +Adds a durable persistence layer for the verified mutation-cursor protocol +(`cli/src/services/mutation_trace/`), storing the protocol's worktree/scope/ +processed-event/mutation-event state in the repository-scoped Agent Trace DB +(`RepositoryAgentTraceDb`) via a new additive migration and a new `store.rs` +module. This is the third build-out step for the module, following the pure +kernel (`mutation-cursor-protocol-kernel`) and its Quint Connect verification +harness (`mutation-cursor-quint-connect`); it extends that work rather than +replacing it, and `protocol.rs` remains exactly as pure as those two plans +left it. + +The persistence boundary is one-directional and structural: +`protocol.rs` (pure semantics) -> `DurableTransition` (a persistence +projection built by pure structural diffing, not protocol interpretation) -> +`store.rs` (SQL translation) -> `RepositoryAgentTraceDb`. `protocol.rs` never +depends on SQL or the DB adapter, and `store.rs` never branches on protocol +meaning (boundary kind, contention, taint) — it only diffs before/after +`ProtocolState` values. + +Two things are deliberately excluded from the database: `AttemptState` +(explicitly transient in the domain model — no `mutation_trace_attempts` +table) and `external_taint` (a `database_failure()` cannot use the database +it just failed against as the authoritative record that the write was +uncertain; a later plan represents it as a filesystem write-ahead marker). + +The runtime read path is split in two. The hot path (`load_worktree`) loads +one worktree, only its currently `Active` scopes, an optionally referenced +scope even when that scope is terminal (`NeverSeen`/`Closed`/`Abandoned`), +and an optional `EventKey` replay row — never historical +`mutation_trace_events` rows and never a terminal scope it was not +explicitly asked for, so the read stays bounded as closed/abandoned scopes +accumulate over time. A separate cold path (`load_mutation_event`) +reconstructs one historical `MutationEvent` by `(worktree, revision)` only on +explicit request. `DurableTransition::between` is a strict structural +firewall: it validates shape (single worktree, no unrelated changes, +revision advances by exactly one when a transition exists, at most one new +processed event, at most one new mutation event) and rejects a structurally +impossible before/after pair, without ever interpreting protocol semantics. +The CAS primitive keeps three outcomes distinct: a stale revision is a +`Conflict` the DB primitive never retries, a transient DB failure retries +the whole transaction, and a deterministic SQL/constraint failure returns an +error without retry. + +## Acceptance criteria + +- [ ] AC1: Mutation state lives in the repository-scoped `agent-trace.db`. + - Validate: `cli/src/services/mutation_trace/store.rs` reads/writes only through `RepositoryAgentTraceDb`; round-trip tests in T09 pass. +- [ ] AC2: New storage is introduced through additive migration `003`, with `001`/`002` byte-unchanged by this PR. + - Validate: `git diff --exit-code ...HEAD -- cli/migrations/agent-trace-repository/001_repository_schema.sql cli/migrations/agent-trace-repository/002_repository_source_instance_id.sql` (compared against this PR's base branch/merge base, not the working tree) exits `0`; `003_mutation_trace_protocol.sql` exists. +- [ ] AC3: Revision preserves all `u64` values exactly, including `u64::MAX`. + - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml mutation_trace::store::` (revision codec round-trip test covering `0`, `1`, `i64::MAX`, `i64::MAX + 1`, `u64::MAX`). +- [ ] AC4: Worktree/scope/`EventKey`/`MutationEvent` data round-trip exactly, including full `MutationEvent` decoding (`Attribution`, `Boundary`, `active_scopes`) after the DB is closed and reopened. + - Validate: T09's real-protocol round-trip tests, including the `load_mutation_event` cold-reload assertions for every transition that emits a `MutationEvent`. +- [ ] AC5: `AttemptState` is never persisted. + - Validate: `grep -n mutation_trace_attempts cli/migrations/agent-trace-repository/003_mutation_trace_protocol.sql` finds nothing; `DurableTransition` has no `AttemptState` field. +- [ ] AC6: `external_taint` is never treated as DB-authoritative durable state. + - Validate: `grep -n external_taint cli/migrations/agent-trace-repository/003_mutation_trace_protocol.sql` finds nothing; `database_failure` produces no `DurableTransition` (T05 test). +- [ ] AC7: No persistence code determines protocol semantics or attribution. + - Validate: `DurableTransition::between` contains no boundary-kind/contention/taint conditionals (T05 done-when); inspection of `store.rs`. +- [ ] AC8: Every durable protocol transition is one `BEGIN IMMEDIATE` transaction. + - Validate: `store.commit` routes exclusively through `execute_transactional_cas_batch` (T06/T07); T08 atomic-rollback test. +- [ ] AC9: CAS is guarded by the expected worktree revision. + - Validate: the guard statement is `UPDATE mutation_trace_worktrees ... WHERE worktree_id = ? AND revision = ?` (T06); T08 two-writer test. +- [ ] AC10: Two writers from one revision cannot both commit. + - Validate: T08's concurrent-writers test — two independent `RepositoryAgentTraceDb` handles/connections against the same physical database, committing concurrently from the same loaded revision — asserts exactly one `Applied` and one `Conflict`. +- [ ] AC11: Partial failure rolls back all worktree/scope/event changes. + - Validate: T08 injected-failure test asserts revision, scope status, processed event, mutation event, and active scopes are all unchanged after rollback. +- [ ] AC12: Process restart reconstructs the same durable protocol projection. + - Validate: T09 tests that drop and reopen the DB handle before reloading. +- [ ] AC13: Historical mutation events are not loaded on each boundary, terminal (`Closed`/`Abandoned`/`NeverSeen`) historical scopes are not loaded on each boundary unless explicitly referenced, and a referenced scope belonging to a different worktree is rejected rather than silently loaded or reassigned. + - Validate: `MutationTraceStore::load_worktree` issues no query against `mutation_trace_events`, loads only currently `Active` scopes plus the explicitly referenced scope (if any), and returns `Err` when that referenced scope's persisted `worktree_id` does not match the requested worktree (T03 done-when). +- [ ] AC14: Existing Quint Connect and protocol tests remain green. + - Validate: `nix flake check` (runs `cli-tests`, including `mutation_trace::mbt`, and the dedicated `mutation-trace-quint-connect` check). +- [ ] AC15: No Git/filesystem lock/hook/coordinator integration is added. + - Validate: no `coordinator.rs` or `git_snapshot.rs` file is created; `grep -RnE "std::(fs|process)|tokio::(fs|process)" cli/src/services/mutation_trace/` shows no non-test production usage. + +### Full validation + +- `nix flake check` +- `nix run .#pkl-check-generated` (lightweight post-task hygiene baseline; unaffected by this Rust-only change) + +### Context sync + +- `context/cli/mutation-trace-store.md` (new — authored by T11) +- `context/context-map.md` (add the new domain-file entry) +- `context/cli/mutation-trace-protocol.md` ("Target end-state architecture" section: `store.rs` now exists as a real database call site, while `coordinator.rs`/`git_snapshot.rs` remain future work) +- `context/overview.md` (the sentence stating the module "is not yet wired into any hook, command, or database call site" needs to reflect that a database call site now exists) +- `context/sce/shared-turso-db.md` (new generic `execute_transactional_cas_batch` primitive added to `TursoDb`, alongside the existing `execute_transactional_insert_pair_if_absent`, including its CAS-conflict/retryable-failure/deterministic-failure distinction) + +## 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/migrations/agent-trace-repository/003_mutation_trace_protocol.sql` (new); `cli/src/services/mutation_trace/store.rs` (new); a new generic `TransactionStatement`/`execute_transactional_cas_batch` primitive on `TursoDb` in `cli/src/services/db/mod.rs`; tests within `mutation_trace` and `db`/`agent_trace_db`; `context/cli/mutation-trace-store.md` (new). +- **Out of scope:** Git snapshots, `GIT_INDEX_FILE`, Git object storage; the filesystem worktree lock and external-taint marker; `coordinator.rs`; real hook events and Claude/Codex/OpenCode/Pi wiring; Agent Trace diff generation; a retry-after-CAS-conflict loop; changes to Quint semantics or `protocol.rs` semantics; scope garbage collection or any deletion of terminal (`Closed`/`Abandoned`) scope rows. +- **Constraints:** `protocol.rs` stays free of SQL/DB/`RepositoryAgentTraceDb` dependencies; `DurableTransition::between` performs structural diffing only, never protocol interpretation, and rejects a structurally malformed before/after pair rather than silently accepting it; revision is stored as an 8-byte big-endian `BLOB`, enforced by `CHECK (typeof(revision) = 'blob' AND length(revision) = 8)` on every column that stores one; every durable transition commits inside exactly one `BEGIN IMMEDIATE` transaction guarded by the expected worktree revision, with a normal CAS conflict (`Ok(false)`) and a deterministic SQL/constraint failure both left unretried by the CAS primitive, while only a genuinely transient DB failure retries the whole transaction; the hot-path worktree read loads only `Active` scopes plus an explicitly referenced scope, never the full historical scope set and never `mutation_trace_events`; enum codecs are explicit (no `Debug`/serde-derived DB representation). +- **Non-goal:** do not modify `REQUIRED_REPOSITORY_SCHEMA_TABLES`'s baseline-repair logic to treat `003` as part of `001`'s metadata-repair case; do not replace or refactor the existing `execute_transactional_insert_pair_if_absent` primitive — the new generic CAS batch primitive is additive alongside it; do not change `resilience.rs`'s retry-on-any-`Err` behavior or any other caller of `run_with_retry_sync` to add this classification. + +## Assumptions + +- Plan slug (`mutation-cursor-store-persistence`) continues the `mutation-cursor-*` naming already used by `mutation-cursor-protocol-kernel` and `mutation-cursor-quint-connect`. +- File-backed DB round-trip tests (T09, T10) reuse the existing `std::env::temp_dir()`-based unique-path helper pattern already established in `cli/src/services/agent_trace_db/repository.rs`'s tests, rather than adding a `tempfile`-style dependency. +- T06's new CAS batch primitive coexists with the existing `execute_transactional_insert_pair_if_absent`; no other call site is migrated to it in this plan's scope. +- Updating the outdated "not yet wired into any hook, command, or database call site" framing in `context/cli/mutation-trace-protocol.md` and `context/overview.md` is handled by task context synchronization, not by a plan task, since it is a root/shared-file update rather than new content this plan's tasks author. +- AC2's validation compares the two untouched migration files against this PR's base branch (currently `quint-connect` for PR #241) or its merge base, not a hardcoded commit SHA, so the check stays correct as the branch advances. +- T06's retryable-vs-deterministic classification is implemented locally to `execute_transactional_cas_batch` — for example, by having its retried closure return a classified outcome that `run_with_retry_sync` still sees as `Ok` (so it never retries a deterministic failure), with the caller re-raising that failure as an `Err` after the closure returns — rather than by changing `resilience.rs` itself. + +## Task stack + +- [ ] T01: `Add migration 003 for mutation-trace protocol tables` (status:todo) + - Task ID: T01 + - Scope: In — `cli/migrations/agent-trace-repository/003_mutation_trace_protocol.sql` defining `mutation_trace_worktrees` (revision `BLOB` constrained by `CHECK (typeof(revision) = 'blob' AND length(revision) = 8)`), `mutation_trace_scopes` (+ `idx_mutation_trace_scopes_worktree` and a new composite `idx_mutation_trace_scopes_worktree_status` index on `(worktree_id, status)` for the bounded hot-path scope lookup), `mutation_trace_processed_events`, `mutation_trace_events` (+ the same `typeof`/`length` revision `CHECK`, plus payload-consistency `CHECK` constraints), and `mutation_trace_event_active_scopes`. Out — any Rust code consuming these tables (T02+). + - Dependencies: none + - Done when: a fresh `RepositoryAgentTraceDb::new_at` at a clean path applies `001`+`002`+`003` and all five tables exist with the specified columns, constraints, and indexes; a row violating a `CHECK` constraint (for example `ai_exclusive` attribution with a `NULL` `attribution_scope_id`) is rejected; a `TEXT` value of length 8 assigned to a `revision` column is rejected by the `typeof(revision) = 'blob'` check even though its length matches. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml agent_trace_db::repository::`; a new targeted test asserting the `003` tables, indexes, and constraints (including the TEXT-vs-BLOB revision case) behave as specified. + - Context synchronization: pending + +- [ ] T02: `Add revision and enum domain<->SQL codecs` (status:todo) + - Task ID: T02 + - Scope: In — create `cli/src/services/mutation_trace/store.rs` with `encode_revision`/`decode_revision` (`u64` <-> 8-byte big-endian `BLOB`) and explicit codecs for `ActorKind`, `FailureKind`, `ScopeStatus`, `Attribution`'s discriminant, and `Boundary`'s discriminant. Out — any query, projection, or commit logic (T03+). + - Dependencies: T01 + - Done when: `encode_revision`/`decode_revision` round-trip exactly for `0`, `1`, `i64::MAX`, `i64::MAX + 1`, and `u64::MAX`; every enum variant round-trips through its codec; no codec relies on `Debug` formatting or implicit serde representation. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml mutation_trace::store::` + - Context synchronization: pending + +- [ ] T03: `Add bounded WorktreeProjection load and cold-path MutationEvent read` (status:todo) + - Task ID: T03 + - Scope: In — `WorktreeProjection` (+ `into_protocol_state`) and `MutationTraceStore` wrapping `&RepositoryAgentTraceDb`. `load_worktree(worktree: &WorktreeId, scope: Option<&ScopeId>, event_key: Option<&EventKey>)` loads exactly one worktree row, only its currently `Active` scopes plus the explicitly referenced `scope` when supplied (even when that scope is `NeverSeen`/`Closed`/`Abandoned`), and 0 or 1 matching processed-event row for `event_key`. A separate cold-path `load_mutation_event(worktree: &WorktreeId, revision: u64) -> Result>` reads one `mutation_trace_events` row plus its `mutation_trace_event_active_scopes` rows and reconstructs a complete `MutationEvent`, decoding `Attribution` exactly (including `AiExclusive(scope_id)`) and the complete `Boundary`. When `scope` is supplied and the persisted `ScopeState` for that `ScopeId` has a `worktree_id` different from the requested `worktree`, `load_worktree` returns `Err` — it never silently omits the scope, never includes it in the projection, and never reassigns it to the requested worktree, preserving the permanent `ScopeId` -> `WorktreeId` identity `register_scope` already enforces. Out — initialization/commit logic (T04/T07); calling `load_mutation_event` from `load_worktree` or from any hook-boundary path. + - Dependencies: T01, T02 + - Done when: `load_worktree` returns `None` for a missing worktree and `Some(projection)` otherwise, with `scopes` containing every currently `Active` scope on that worktree plus `scope` when supplied regardless of its status, and never a `Closed`/`Abandoned`/`NeverSeen` scope that was not explicitly referenced; `attempts`, `mutation_events`, and `external_taint` stay empty; the method issues no query against `mutation_trace_events`. Specifically: a referenced scope on the requested worktree is included regardless of status; a referenced terminal (`Closed`/`Abandoned`/`NeverSeen`) scope on the requested worktree is included; an unreferenced terminal historical scope is excluded; a referenced scope whose persisted `worktree_id` belongs to a different worktree returns `Err`. `load_mutation_event` returns `None` when no row exists at that `(worktree, revision)` and otherwise reconstructs a `MutationEvent` whose `before_tree`/`after_tree`/`revision`/`tainted`/`failure_kind`/`attribution`/`boundary`/`active_scopes` exactly match what `store.commit` persisted. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml mutation_trace::store::` + - Context synchronization: pending + +- [ ] T04: `Add worktree/scope initialization operations` (status:todo) + - Task ID: T04 + - Scope: In — `initialize_worktree(worktree_id, initial_tree)` and `register_scope(scope_id, worktree_id, actor_kind)` on `MutationTraceStore`. Out — the CAS commit path (T06/T07). + - Dependencies: T03 + - Done when: `initialize_worktree` inserts `revision=0`/healthy/not-tainted/not-needs-rebaseline only when the worktree is missing and never overwrites an existing cursor; `register_scope` inserts `NeverSeen` when missing, returns the existing state when worktree+actor match, and errors on a worktree or actor mismatch for an existing `scope_id`. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml mutation_trace::store::` + - Context synchronization: pending + +- [ ] T05: `Add DurableTransition structural diff type` (status:todo) + - Task ID: T05 + - Scope: In — `DurableTransition` and `DurableTransition::between(before, after, worktree) -> Result>` performing pure structural diffing only, enforcing: the target worktree exists in both `before` and `after` and is never added or removed; no unrelated worktree changes; when a durable transition exists, its worktree's next revision is exactly `expected_revision + 1` computed via checked `u64` arithmetic; no scope is added or deleted; a changed scope belongs to the target worktree; `ScopeState.worktree_id` and `ScopeState.actor_kind` never change (only `status` may); `processed_events` may only gain entries, never lose them, with at most one new entry whose scope belongs to the target worktree; `mutation_events` may only gain entries, never lose them, with at most one new entry belonging to the target worktree; `AttemptState`/`external_taint` differences are ignored. Out — SQL/DB code (T06/T07). + - Dependencies: T02 + - Done when: `between()` returns `Ok(None)` for a `database_failure`-only transition and for a no-change `Flush`; returns `Ok(Some(..))` with the correct shape for `Start`/`Advance`/`Close`, `taint`, `abandon`, and `recover` transitions exercised directly against `protocol::*` outputs; the function contains no boundary-kind, contention, or taint conditionals; it returns `Err` for a malformed `before`/`after` pair covering at least: an `actor_kind` change, a scope's `worktree_id` change, a processed `EventKey` disappearing, a `MutationEvent` disappearing, an unrelated worktree changing, a revision jump by more than 1, a revision decrease, and a scope unexpectedly appearing or disappearing. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml mutation_trace::store::` + - Context synchronization: pending + +- [ ] T06: `Add generic transactional CAS batch primitive to TursoDb` (status:todo) + - Task ID: T06 + - Scope: In — `TransactionStatement` and `TursoDb::execute_transactional_cas_batch(operation_name, retry_hint, guard, statements)` in `cli/src/services/db/mod.rs`, with a retryability contract distinct from the shared `run_with_retry_sync` helper's plain any-`Err`-retries behavior: a guard affecting 0 rows commits as a no-op and returns `Ok(false)` (a normal CAS conflict) without running any statement and without being retried; a guard affecting 1 row runs every statement inside the same `BEGIN IMMEDIATE` transaction and returns `Ok(true)`; a retryable DB failure (lock/busy/other transient condition) retries the entire transaction from `BEGIN IMMEDIATE`; a deterministic failure (SQL/schema/constraint/invariant violation) returns `Err` without being retried. This adds the minimum local retryability classification needed for that behavior — for example, the retried closure returns a classified outcome that `run_with_retry_sync` still treats as `Ok` so it never retries a deterministic failure, and the caller re-raises that failure as `Err` once the closure returns — without changing `resilience.rs` or any other caller of `run_with_retry_sync`. Out — mutation-trace-specific SQL (T07). + - Dependencies: none + - Done when: a guard affecting 0 rows commits as a no-op and returns `Ok(false)` without running any statement or waiting for a retry backoff; a guard affecting 1 row runs every statement and returns `Ok(true)`; an injected deterministic mid-batch failure rolls back the entire transaction (including the guard's own effect) and surfaces as `Err` after exactly one attempt, never reported as a CAS conflict; an injected retryable DB failure retries the whole transaction from `BEGIN IMMEDIATE` (never individual statements) up to the configured attempt count. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml db::` + - Context synchronization: pending + +- [ ] T07: `Implement MutationTraceStore::commit` (status:todo) + - Task ID: T07 + - Scope: In — `CasResult` and `MutationTraceStore::commit(transition)`, translating a `DurableTransition` into the worktree CAS `UPDATE` plus scope `UPDATE`s plus processed-event `INSERT` plus mutation-event `INSERT` plus active-scope `INSERT`s, via `execute_transactional_cas_batch`. Out — concurrency/rollback/round-trip test coverage (T08/T09). + - Dependencies: T04, T05, T06 + - Done when: `commit()` returns `CasResult::Applied` with every included write visible when the worktree's on-disk revision matches `expected_revision`, and `CasResult::Conflict` with no visible write otherwise; a deterministic failure surfaced by `execute_transactional_cas_batch` propagates out of `commit()` as an `Err`, never as `CasResult::Conflict`. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml mutation_trace::store::` + - Context synchronization: pending + +- [ ] T08: `Add CAS and concurrency test coverage for store.commit` (status:todo) + - Task ID: T08 + - Scope: In — tests for: two writers committing from the same revision against one physical repository-scoped `agent-trace.db`, using two independent `RepositoryAgentTraceDb` handles/connections opened against that same database file (one `MutationTraceStore` per handle), with both writers loading worktree revision `N` before either commits and executing their commits from separate threads (or an equivalent that exercises two independent DB connections rather than one handle invoked twice in sequence) — exactly one result `CasResult::Applied`, the other `CasResult::Conflict`; atomic rollback on an injected deterministic mid-transaction failure; `u64::MAX` round-trip through the real DB; `(scope_id, event_id)` replay-uniqueness rejection; strong recovery (all active scopes abandoned) and needs-only recovery (surviving active scopes stay active). Out — production code changes beyond what T07 already provides; process-spawning or other multiprocess test infrastructure (two independent DB handles on separate threads are sufficient for this PR). + - Dependencies: T07 + - Done when: all five scenarios above are covered by passing tests; the two-writer test is not satisfied by calling `commit` twice sequentially through one shared `RepositoryAgentTraceDb` handle; after both commits, reopening the database shows the worktree revision advanced exactly once and only the winning transition's durable effects (scope status, processed event, mutation event, active scopes) are present; the atomic-rollback test observes revision, scope status, processed event, mutation event, and active scopes all unchanged after the injected failure. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml mutation_trace::store::` + - Context synchronization: pending + +- [ ] T09: `Add real-protocol round-trip persistence tests` (status:todo) + - Task ID: T09 + - Scope: In — tests driving load (`load_worktree`, bounded to `Active` scopes plus the transition's referenced scope) -> `protocol::prepare`/`commit` (or `taint`/`database_failure`/`abandon`/`recover`) -> `DurableTransition::between` -> `store.commit` -> drop DB handle -> reopen -> reload, for `Start`, `Advance`, `Close`, `Flush` with change, `Flush` without change, taint, abandon, recover, contended mutation, and a replayed `EventKey`. For every transition that emits a `MutationEvent`, additionally reload it after reopening with `load_mutation_event(worktree, revision)` and compare it field-for-field (including exact `Attribution`/`Boundary` decoding) against the `MutationEvent` the original protocol transition produced. Out — new production code, unless a genuine T01-T07 gap surfaces. + - Dependencies: T07 + - Done when: for every listed transition, the reloaded worktree/scope projection after reopening the DB matches the durable projection produced by the original protocol transition, and for every transition that emits a `MutationEvent`, `load_mutation_event` after reopening reconstructs it exactly. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml mutation_trace::store::` + - Context synchronization: pending + +- [ ] T10: `Add migration and lifecycle tests for migration 003` (status:todo) + - Task ID: T10 + - Scope: In — tests proving a fresh DB applies `001`+`002`+`003`; an existing `001`+`002`-only DB gets `003` applied through the `sce setup`/lifecycle path; the no-migration hook-runtime path does not apply `003` and still reports the existing "Run 'sce setup'." guidance when schema is incomplete. Out — changes to `REQUIRED_REPOSITORY_SCHEMA_TABLES` baseline-repair semantics. + - Dependencies: T01 + - Done when: all three scenarios pass without modifying the baseline-repair function's treatment of `001` metadata. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml agent_trace_db::` + - Context synchronization: pending + +- [ ] T11: `Document the mutation-trace store` (status:todo) + - Task ID: T11 + - Scope: In — `context/cli/mutation-trace-store.md` covering repository-DB ownership, `WorktreeId` as the persistence partition, the 8-byte big-endian revision encoding, `AttemptState`/`external_taint` non-persistence, and the store's non-goals (no Git I/O, no attribution decisions, no retry-after-`Conflict`); a `context/context-map.md` entry for the new file. Out — edits to any other existing `context/` file (left to task context synchronization). + - Dependencies: T01-T10 + - Done when: the new file exists, is linked from `context/context-map.md`, and every claim in it is checked against the code produced by T01-T10. + - Verify: manual inspection cross-referencing the file's claims against `store.rs`, the migration, and `db/mod.rs`. + - Context synchronization: pending + +## Open questions + +None. The change request already resolves every architectural decision (schema shape, CAS mechanics, which fields are excluded from persistence) precisely, and each decision checks out against the current `protocol.rs`/`types.rs` domain model and the existing Turso adapter conventions verified while authoring this plan. From c445effe1a904b730315fe793f80ad4ab336c8c9 Mon Sep 17 00:00:00 2001 From: David Abram Date: Thu, 27 Aug 2026 13:48:34 +0200 Subject: [PATCH 2/5] storage: Add mutation-trace protocol persistence schema Persist the verified mutation-cursor protocol in the repository database so later store operations have durable tables for worktrees, scopes, processed events, mutation events, and active scopes. Add migration 003 with strict revision BLOB checks, enum allow-lists, and payload-consistency constraints, and extend repository initialization tests with constraint coverage. Plan: mutation-cursor-store-persistence (T01) Co-authored-by: SCE --- .../003_mutation_trace_protocol.sql | 86 +++++++++++++++++++ cli/src/services/agent_trace_db/repository.rs | 75 +++++++++++++++- .../mutation-cursor-store-persistence.md | 10 ++- 3 files changed, 168 insertions(+), 3 deletions(-) create mode 100644 cli/migrations/agent-trace-repository/003_mutation_trace_protocol.sql diff --git a/cli/migrations/agent-trace-repository/003_mutation_trace_protocol.sql b/cli/migrations/agent-trace-repository/003_mutation_trace_protocol.sql new file mode 100644 index 00000000..d6bdd867 --- /dev/null +++ b/cli/migrations/agent-trace-repository/003_mutation_trace_protocol.sql @@ -0,0 +1,86 @@ +-- Durable persistence for the verified mutation-cursor protocol +-- (`cli/src/services/mutation_trace/`). +-- +-- This migration is additive: it introduces five new tables and leaves every +-- table from 001/002 untouched. `revision` is stored as an 8-byte +-- big-endian BLOB on every column that carries one (never a SQLite INTEGER), +-- enforced by `CHECK (typeof(revision) = 'blob' AND length(revision) = 8)`, +-- so a TEXT value of matching length is still rejected. Enum-shaped columns +-- use TEXT with an explicit CHECK allow-list, matching the `role`/ +-- `payload_type` convention already used in 001. `AttemptState` (transient) +-- and `external_taint` (not DB-authoritative) are deliberately not +-- represented by any table here. + +CREATE TABLE IF NOT EXISTS mutation_trace_worktrees ( + worktree_id TEXT PRIMARY KEY, + cursor_tree TEXT NOT NULL, + revision BLOB NOT NULL + CHECK (typeof(revision) = 'blob' AND length(revision) = 8), + tainted INTEGER NOT NULL CHECK (tainted IN (0, 1)), + failure_kind TEXT NOT NULL CHECK (failure_kind IN ('healthy', 'snapshot_failure')), + needs_rebaseline INTEGER NOT NULL CHECK (needs_rebaseline IN (0, 1)), + created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), + updated_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) +); + +CREATE TABLE IF NOT EXISTS mutation_trace_scopes ( + scope_id TEXT PRIMARY KEY, + worktree_id TEXT NOT NULL, + actor_kind TEXT NOT NULL CHECK (actor_kind IN ('claude_code', 'codex', 'opencode', 'pi')), + status TEXT NOT NULL CHECK (status IN ('never_seen', 'active', 'closed', 'abandoned')), + created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), + updated_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) +); + +CREATE INDEX IF NOT EXISTS idx_mutation_trace_scopes_worktree +ON mutation_trace_scopes (worktree_id); + +CREATE INDEX IF NOT EXISTS idx_mutation_trace_scopes_worktree_status +ON mutation_trace_scopes (worktree_id, status); + +CREATE TABLE IF NOT EXISTS mutation_trace_processed_events ( + scope_id TEXT NOT NULL, + event_id TEXT NOT NULL, + worktree_id TEXT NOT NULL, + created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), + PRIMARY KEY (scope_id, event_id) +); + +CREATE INDEX IF NOT EXISTS idx_mutation_trace_processed_events_worktree +ON mutation_trace_processed_events (worktree_id); + +CREATE TABLE IF NOT EXISTS mutation_trace_events ( + worktree_id TEXT NOT NULL, + revision BLOB NOT NULL + CHECK (typeof(revision) = 'blob' AND length(revision) = 8), + before_tree TEXT NOT NULL, + after_tree TEXT NOT NULL, + tainted INTEGER NOT NULL CHECK (tainted IN (0, 1)), + failure_kind TEXT NOT NULL CHECK (failure_kind IN ('healthy', 'snapshot_failure')), + attribution_kind TEXT NOT NULL + CHECK (attribution_kind IN ('ineligible_unscoped', 'ai_exclusive', 'ai_contended')), + attribution_scope_id TEXT, + boundary_kind TEXT NOT NULL CHECK (boundary_kind IN ('start', 'advance', 'close', 'flush')), + boundary_scope_id TEXT, + boundary_event_id TEXT, + created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), + PRIMARY KEY (worktree_id, revision), + CHECK ( + (attribution_kind = 'ai_exclusive' AND attribution_scope_id IS NOT NULL) + OR (attribution_kind != 'ai_exclusive' AND attribution_scope_id IS NULL) + ), + CHECK ( + (boundary_kind IN ('start', 'advance', 'close') + AND boundary_scope_id IS NOT NULL AND boundary_event_id IS NOT NULL) + OR (boundary_kind = 'flush' + AND boundary_scope_id IS NULL AND boundary_event_id IS NULL) + ) +); + +CREATE TABLE IF NOT EXISTS mutation_trace_event_active_scopes ( + worktree_id TEXT NOT NULL, + revision BLOB NOT NULL + CHECK (typeof(revision) = 'blob' AND length(revision) = 8), + scope_id TEXT NOT NULL, + PRIMARY KEY (worktree_id, revision, scope_id) +); diff --git a/cli/src/services/agent_trace_db/repository.rs b/cli/src/services/agent_trace_db/repository.rs index 9cd63d2e..44792d30 100644 --- a/cli/src/services/agent_trace_db/repository.rs +++ b/cli/src/services/agent_trace_db/repository.rs @@ -388,6 +388,11 @@ mod tests { "agent_traces", "messages", "parts", + "mutation_trace_worktrees", + "mutation_trace_scopes", + "mutation_trace_processed_events", + "mutation_trace_events", + "mutation_trace_event_active_scopes", ] { assert!( sqlite_object_exists(&db, "table", table), @@ -401,6 +406,9 @@ mod tests { "idx_messages_session_message", "idx_messages_session_order", "idx_parts_session_message_order", + "idx_mutation_trace_scopes_worktree", + "idx_mutation_trace_scopes_worktree_status", + "idx_mutation_trace_processed_events_worktree", ] { assert!( sqlite_object_exists(&db, "index", index), @@ -426,9 +434,10 @@ mod tests { vec![ String::from("001_repository_schema"), String::from("002_repository_source_instance_id"), + String::from("003_mutation_trace_protocol"), ], "repository DBs should be initialized from the baseline schema plus \ - its additive source-instance-id migration" + its additive source-instance-id and mutation-trace-protocol migrations" ); db.ensure_schema_ready_for_hooks() @@ -437,6 +446,70 @@ mod tests { remove_test_db(&db_path); } + #[test] + fn mutation_trace_worktrees_revision_must_be_a_blob_not_matching_length_text() { + let db_path = unique_test_db_path("mutation-trace-revision-blob"); + let db = RepositoryAgentTraceDb::new_at(&db_path).expect("repository DB should open"); + + let text_revision_error = db + .execute( + "INSERT INTO mutation_trace_worktrees + (worktree_id, cursor_tree, revision, tainted, failure_kind, needs_rebaseline) + VALUES ('wt-1', 'tree-0', '12345678', 0, 'healthy', 0)", + (), + ) + .expect_err( + "an 8-byte TEXT value must still be rejected by the typeof(revision) = 'blob' check", + ); + assert!( + text_revision_error.to_string().contains("CHECK"), + "unexpected error: {text_revision_error}" + ); + + db.execute( + "INSERT INTO mutation_trace_worktrees + (worktree_id, cursor_tree, revision, tainted, failure_kind, needs_rebaseline) + VALUES ('wt-1', 'tree-0', X'0000000000000000', 0, 'healthy', 0)", + (), + ) + .expect("an 8-byte BLOB revision should be accepted"); + + remove_test_db(&db_path); + } + + #[test] + fn mutation_trace_events_ai_exclusive_attribution_requires_a_scope_id() { + let db_path = unique_test_db_path("mutation-trace-attribution-check"); + let db = RepositoryAgentTraceDb::new_at(&db_path).expect("repository DB should open"); + + let missing_scope_error = db + .execute( + "INSERT INTO mutation_trace_events + (worktree_id, revision, before_tree, after_tree, tainted, failure_kind, + attribution_kind, attribution_scope_id, boundary_kind, boundary_scope_id, boundary_event_id) + VALUES ('wt-1', X'0000000000000001', 'tree-0', 'tree-1', 0, 'healthy', + 'ai_exclusive', NULL, 'flush', NULL, NULL)", + (), + ) + .expect_err("ai_exclusive attribution with a NULL attribution_scope_id must be rejected"); + assert!( + missing_scope_error.to_string().contains("CHECK"), + "unexpected error: {missing_scope_error}" + ); + + db.execute( + "INSERT INTO mutation_trace_events + (worktree_id, revision, before_tree, after_tree, tainted, failure_kind, + attribution_kind, attribution_scope_id, boundary_kind, boundary_scope_id, boundary_event_id) + VALUES ('wt-1', X'0000000000000001', 'tree-0', 'tree-1', 0, 'healthy', + 'ai_exclusive', 'scope-1', 'start', 'scope-1', 'event-1')", + (), + ) + .expect("ai_exclusive attribution with a scope ID should be accepted"); + + remove_test_db(&db_path); + } + #[test] fn trace_tables_have_no_checkout_id_columns() { let db_path = unique_test_db_path("no-checkout-id"); diff --git a/context/plans/mutation-cursor-store-persistence.md b/context/plans/mutation-cursor-store-persistence.md index b2a744d1..1943dd91 100644 --- a/context/plans/mutation-cursor-store-persistence.md +++ b/context/plans/mutation-cursor-store-persistence.md @@ -119,13 +119,19 @@ Persist this field in every plan; this is durable plan state, not chat state: ## Task stack -- [ ] T01: `Add migration 003 for mutation-trace protocol tables` (status:todo) +- [x] T01: `Add migration 003 for mutation-trace protocol tables` (status:done) - Task ID: T01 - Scope: In — `cli/migrations/agent-trace-repository/003_mutation_trace_protocol.sql` defining `mutation_trace_worktrees` (revision `BLOB` constrained by `CHECK (typeof(revision) = 'blob' AND length(revision) = 8)`), `mutation_trace_scopes` (+ `idx_mutation_trace_scopes_worktree` and a new composite `idx_mutation_trace_scopes_worktree_status` index on `(worktree_id, status)` for the bounded hot-path scope lookup), `mutation_trace_processed_events`, `mutation_trace_events` (+ the same `typeof`/`length` revision `CHECK`, plus payload-consistency `CHECK` constraints), and `mutation_trace_event_active_scopes`. Out — any Rust code consuming these tables (T02+). - Dependencies: none - Done when: a fresh `RepositoryAgentTraceDb::new_at` at a clean path applies `001`+`002`+`003` and all five tables exist with the specified columns, constraints, and indexes; a row violating a `CHECK` constraint (for example `ai_exclusive` attribution with a `NULL` `attribution_scope_id`) is rejected; a `TEXT` value of length 8 assigned to a `revision` column is rejected by the `typeof(revision) = 'blob'` check even though its length matches. - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml agent_trace_db::repository::`; a new targeted test asserting the `003` tables, indexes, and constraints (including the TEXT-vs-BLOB revision case) behave as specified. - - Context synchronization: pending + - Completed: 2026-08-27 + - Files changed: `cli/migrations/agent-trace-repository/003_mutation_trace_protocol.sql` (new); `cli/src/services/agent_trace_db/repository.rs` + - Result: Added migration `003_mutation_trace_protocol.sql` defining `mutation_trace_worktrees`, `mutation_trace_scopes` (+ `idx_mutation_trace_scopes_worktree`, `idx_mutation_trace_scopes_worktree_status`), `mutation_trace_processed_events` (+ `idx_mutation_trace_processed_events_worktree`), `mutation_trace_events`, and `mutation_trace_event_active_scopes`, all discovered automatically by `build.rs`'s directory scan. Revision columns use `BLOB NOT NULL CHECK (typeof(revision) = 'blob' AND length(revision) = 8)`; enum-shaped columns use `TEXT` with `CHECK (... IN (...))` allow-lists following the existing `role`/`payload_type` convention; `mutation_trace_events` additionally enforces attribution/boundary payload-consistency `CHECK`s (`ai_exclusive` requires a non-null `attribution_scope_id`; hook boundaries require non-null `boundary_scope_id`/`boundary_event_id`, `flush` requires both null). Updated `open_at_initializes_the_full_schema_from_one_migration` to assert the new migration ID and the five new tables/indexes, and added two new targeted tests (`mutation_trace_worktrees_revision_must_be_a_blob_not_matching_length_text`, `mutation_trace_events_ai_exclusive_attribution_requires_a_scope_id`) proving the TEXT-vs-BLOB revision rejection and the `ai_exclusive`-requires-scope rejection, each paired with a positive control insert. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml agent_trace_db::repository::` — passed, 18/18 (including the two new tests and the updated baseline-schema test); `./scripts/run-cli-cargo.sh fmt --manifest-path cli/Cargo.toml -- --check` — passed (no diff). + - Done checks: fresh DB applies `001`+`002`+`003` with all five tables/indexes present (verified by the updated baseline test); `ai_exclusive` attribution with a `NULL` `attribution_scope_id` is rejected (verified); an 8-byte TEXT value assigned to `revision` is rejected by `typeof(revision) = 'blob'` (verified); `git diff --exit-code` on `001`/`002` shows zero changes (verified). + - Context impact: local — additive schema-only migration; no Rust code consumes these new tables yet (T02+ wire codecs, loads, and commits against them). No durable context synchronization is required for this task; the plan's `Context sync` entries are authored by T11 once the full store lands. + - Context synchronization: synced - [ ] T02: `Add revision and enum domain<->SQL codecs` (status:todo) - Task ID: T02 From aebcd4734944308710200f53086879e006aedce8 Mon Sep 17 00:00:00 2001 From: David Abram Date: Thu, 27 Aug 2026 14:01:49 +0200 Subject: [PATCH 3/5] mutation-trace: Add explicit revision and enum SQL codecs Persist mutation-trace domain values using stable, migration-aligned encodings before query and commit logic is added. Add big-endian revision codecs and explicit string codecs plus discriminant helpers, with rejection of invalid values and round-trip tests. Plan: mutation-cursor-store-persistence, T02 Co-authored-by: SCE --- cli/src/services/mutation_trace/mod.rs | 1 + cli/src/services/mutation_trace/store.rs | 302 ++++++++++++++++++ .../mutation-cursor-store-persistence.md | 10 +- 3 files changed, 311 insertions(+), 2 deletions(-) create mode 100644 cli/src/services/mutation_trace/store.rs diff --git a/cli/src/services/mutation_trace/mod.rs b/cli/src/services/mutation_trace/mod.rs index 5544f4aa..2416cbdc 100644 --- a/cli/src/services/mutation_trace/mod.rs +++ b/cli/src/services/mutation_trace/mod.rs @@ -156,6 +156,7 @@ //! unchanged). pub mod protocol; +pub mod store; pub mod types; #[cfg(test)] diff --git a/cli/src/services/mutation_trace/store.rs b/cli/src/services/mutation_trace/store.rs new file mode 100644 index 00000000..fefa69de --- /dev/null +++ b/cli/src/services/mutation_trace/store.rs @@ -0,0 +1,302 @@ +//! Domain<->SQL codecs for the mutation-cursor persistence layer. +//! +//! These codecs are the only translation between `super::types` domain +//! values and the `TEXT`/`BLOB` representations `cli/migrations/agent-trace- +//! repository/003_mutation_trace_protocol.sql` constrains those columns to. +//! Every codec here is an explicit function over a fixed set of variants — no +//! codec derives from `Debug` or a serde representation, so a variant rename +//! cannot silently change the durable encoding. +//! +//! This module carries no query, projection, or commit logic yet (see +//! `mutation-cursor-store-persistence` plan tasks T03+); it only establishes +//! the byte- and string-level encodings later tasks build on. + +use anyhow::{bail, Result}; + +use super::types::{ActorKind, Attribution, Boundary, FailureKind, ScopeStatus}; + +/// Encodes a worktree/event revision as the 8-byte big-endian `BLOB` stored +/// by every `revision` column in migration `003`. +pub fn encode_revision(revision: u64) -> [u8; 8] { + revision.to_be_bytes() +} + +/// Decodes a worktree/event revision from the 8-byte big-endian `BLOB` +/// migration `003`'s `CHECK (typeof(revision) = 'blob' AND length(revision) +/// = 8)` constraint guarantees on every stored value. +pub fn decode_revision(blob: &[u8]) -> Result { + let bytes: [u8; 8] = blob.try_into().map_err(|_| { + anyhow::anyhow!("revision blob must be exactly 8 bytes, got {}", blob.len()) + })?; + Ok(u64::from_be_bytes(bytes)) +} + +/// Encodes an [`ActorKind`] as the `mutation_trace_scopes.actor_kind` `TEXT` +/// value migration `003`'s `CHECK (actor_kind IN (...))` allow-list expects. +pub fn encode_actor_kind(actor_kind: ActorKind) -> &'static str { + match actor_kind { + ActorKind::ClaudeCode => "claude_code", + ActorKind::Codex => "codex", + ActorKind::OpenCode => "opencode", + ActorKind::Pi => "pi", + } +} + +/// Decodes an [`ActorKind`] from `mutation_trace_scopes.actor_kind`. +pub fn decode_actor_kind(value: &str) -> Result { + match value { + "claude_code" => Ok(ActorKind::ClaudeCode), + "codex" => Ok(ActorKind::Codex), + "opencode" => Ok(ActorKind::OpenCode), + "pi" => Ok(ActorKind::Pi), + other => bail!("unrecognized actor_kind: {other:?}"), + } +} + +/// Encodes a [`FailureKind`] as the `failure_kind` `TEXT` value migration +/// `003` constrains `mutation_trace_worktrees.failure_kind` and +/// `mutation_trace_events.failure_kind` to. +pub fn encode_failure_kind(failure_kind: FailureKind) -> &'static str { + match failure_kind { + FailureKind::Healthy => "healthy", + FailureKind::SnapshotFailure => "snapshot_failure", + } +} + +/// Decodes a [`FailureKind`] from a `failure_kind` column. +pub fn decode_failure_kind(value: &str) -> Result { + match value { + "healthy" => Ok(FailureKind::Healthy), + "snapshot_failure" => Ok(FailureKind::SnapshotFailure), + other => bail!("unrecognized failure_kind: {other:?}"), + } +} + +/// Encodes a [`ScopeStatus`] as the `mutation_trace_scopes.status` `TEXT` +/// value migration `003`'s `CHECK (status IN (...))` allow-list expects. +pub fn encode_scope_status(status: ScopeStatus) -> &'static str { + match status { + ScopeStatus::NeverSeen => "never_seen", + ScopeStatus::Active => "active", + ScopeStatus::Closed => "closed", + ScopeStatus::Abandoned => "abandoned", + } +} + +/// Decodes a [`ScopeStatus`] from `mutation_trace_scopes.status`. +pub fn decode_scope_status(value: &str) -> Result { + match value { + "never_seen" => Ok(ScopeStatus::NeverSeen), + "active" => Ok(ScopeStatus::Active), + "closed" => Ok(ScopeStatus::Closed), + "abandoned" => Ok(ScopeStatus::Abandoned), + other => bail!("unrecognized scope status: {other:?}"), + } +} + +/// [`Attribution`]'s discriminant, decoupled from its `AiExclusive` payload +/// (`ScopeId`). Reconstructing a full [`Attribution`] from a persisted row +/// also needs `attribution_scope_id`, which is a `mutation_trace_events` +/// query concern owned by a later task, not by this codec. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum AttributionKind { + IneligibleUnscoped, + AiExclusive, + AiContended, +} + +/// The discriminant of an [`Attribution`] value. +pub fn attribution_kind(attribution: &Attribution) -> AttributionKind { + match attribution { + Attribution::IneligibleUnscoped => AttributionKind::IneligibleUnscoped, + Attribution::AiExclusive(_) => AttributionKind::AiExclusive, + Attribution::AiContended => AttributionKind::AiContended, + } +} + +/// Encodes an [`AttributionKind`] as the +/// `mutation_trace_events.attribution_kind` `TEXT` value migration `003`'s +/// `CHECK (attribution_kind IN (...))` allow-list expects. +pub fn encode_attribution_kind(kind: AttributionKind) -> &'static str { + match kind { + AttributionKind::IneligibleUnscoped => "ineligible_unscoped", + AttributionKind::AiExclusive => "ai_exclusive", + AttributionKind::AiContended => "ai_contended", + } +} + +/// Decodes an [`AttributionKind`] from `mutation_trace_events.attribution_kind`. +pub fn decode_attribution_kind(value: &str) -> Result { + match value { + "ineligible_unscoped" => Ok(AttributionKind::IneligibleUnscoped), + "ai_exclusive" => Ok(AttributionKind::AiExclusive), + "ai_contended" => Ok(AttributionKind::AiContended), + other => bail!("unrecognized attribution_kind: {other:?}"), + } +} + +/// [`Boundary`]'s discriminant, decoupled from its `scope`/`event`/`worktree` +/// payload. Reconstructing a full [`Boundary`] from a persisted row also +/// needs `boundary_scope_id`/`boundary_event_id`, which is a +/// `mutation_trace_events` query concern owned by a later task, not by this +/// codec. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum BoundaryKind { + Start, + Advance, + Close, + Flush, +} + +/// The discriminant of a [`Boundary`] value. +pub fn boundary_kind(boundary: &Boundary) -> BoundaryKind { + match boundary { + Boundary::Start { .. } => BoundaryKind::Start, + Boundary::Advance { .. } => BoundaryKind::Advance, + Boundary::Close { .. } => BoundaryKind::Close, + Boundary::Flush { .. } => BoundaryKind::Flush, + } +} + +/// Encodes a [`BoundaryKind`] as the `mutation_trace_events.boundary_kind` +/// `TEXT` value migration `003`'s `CHECK (boundary_kind IN (...))` +/// allow-list expects. +pub fn encode_boundary_kind(kind: BoundaryKind) -> &'static str { + match kind { + BoundaryKind::Start => "start", + BoundaryKind::Advance => "advance", + BoundaryKind::Close => "close", + BoundaryKind::Flush => "flush", + } +} + +/// Decodes a [`BoundaryKind`] from `mutation_trace_events.boundary_kind`. +pub fn decode_boundary_kind(value: &str) -> Result { + match value { + "start" => Ok(BoundaryKind::Start), + "advance" => Ok(BoundaryKind::Advance), + "close" => Ok(BoundaryKind::Close), + "flush" => Ok(BoundaryKind::Flush), + other => bail!("unrecognized boundary_kind: {other:?}"), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::services::mutation_trace::types::{EventId, ScopeId}; + + #[test] + fn revision_round_trips_at_boundary_values() { + for revision in [0u64, 1, i64::MAX as u64, (i64::MAX as u64) + 1, u64::MAX] { + let encoded = encode_revision(revision); + assert_eq!(encoded.len(), 8); + assert_eq!(decode_revision(&encoded).unwrap(), revision); + } + } + + #[test] + fn decode_revision_rejects_wrong_length() { + assert!(decode_revision(&[0u8; 7]).is_err()); + assert!(decode_revision(&[0u8; 9]).is_err()); + } + + #[test] + fn actor_kind_round_trips_every_variant() { + for actor_kind in [ + ActorKind::ClaudeCode, + ActorKind::Codex, + ActorKind::OpenCode, + ActorKind::Pi, + ] { + let encoded = encode_actor_kind(actor_kind); + assert_eq!(decode_actor_kind(encoded).unwrap(), actor_kind); + } + } + + #[test] + fn decode_actor_kind_rejects_unknown_value() { + assert!(decode_actor_kind("unknown").is_err()); + } + + #[test] + fn failure_kind_round_trips_every_variant() { + for failure_kind in [FailureKind::Healthy, FailureKind::SnapshotFailure] { + let encoded = encode_failure_kind(failure_kind); + assert_eq!(decode_failure_kind(encoded).unwrap(), failure_kind); + } + } + + #[test] + fn decode_failure_kind_rejects_unknown_value() { + assert!(decode_failure_kind("unknown").is_err()); + } + + #[test] + fn scope_status_round_trips_every_variant() { + for status in [ + ScopeStatus::NeverSeen, + ScopeStatus::Active, + ScopeStatus::Closed, + ScopeStatus::Abandoned, + ] { + let encoded = encode_scope_status(status); + assert_eq!(decode_scope_status(encoded).unwrap(), status); + } + } + + #[test] + fn decode_scope_status_rejects_unknown_value() { + assert!(decode_scope_status("unknown").is_err()); + } + + #[test] + fn attribution_kind_round_trips_every_variant() { + let ineligible = Attribution::IneligibleUnscoped; + let exclusive = Attribution::AiExclusive(ScopeId("scope-1".to_string())); + let contended = Attribution::AiContended; + + for attribution in [&ineligible, &exclusive, &contended] { + let kind = attribution_kind(attribution); + let encoded = encode_attribution_kind(kind); + assert_eq!(decode_attribution_kind(encoded).unwrap(), kind); + } + + assert_eq!(attribution_kind(&exclusive), AttributionKind::AiExclusive); + } + + #[test] + fn decode_attribution_kind_rejects_unknown_value() { + assert!(decode_attribution_kind("unknown").is_err()); + } + + #[test] + fn boundary_kind_round_trips_every_variant() { + let start = Boundary::Start { + scope: ScopeId("scope-1".to_string()), + event: EventId("event-1".to_string()), + }; + let advance = Boundary::Advance { + scope: ScopeId("scope-1".to_string()), + event: EventId("event-2".to_string()), + }; + let close = Boundary::Close { + scope: ScopeId("scope-1".to_string()), + event: EventId("event-3".to_string()), + }; + let flush = Boundary::Flush { + worktree: crate::services::mutation_trace::types::WorktreeId("wt-1".to_string()), + }; + + for boundary in [&start, &advance, &close, &flush] { + let kind = boundary_kind(boundary); + let encoded = encode_boundary_kind(kind); + assert_eq!(decode_boundary_kind(encoded).unwrap(), kind); + } + } + + #[test] + fn decode_boundary_kind_rejects_unknown_value() { + assert!(decode_boundary_kind("unknown").is_err()); + } +} diff --git a/context/plans/mutation-cursor-store-persistence.md b/context/plans/mutation-cursor-store-persistence.md index 1943dd91..096600c8 100644 --- a/context/plans/mutation-cursor-store-persistence.md +++ b/context/plans/mutation-cursor-store-persistence.md @@ -133,13 +133,19 @@ Persist this field in every plan; this is durable plan state, not chat state: - Context impact: local — additive schema-only migration; no Rust code consumes these new tables yet (T02+ wire codecs, loads, and commits against them). No durable context synchronization is required for this task; the plan's `Context sync` entries are authored by T11 once the full store lands. - Context synchronization: synced -- [ ] T02: `Add revision and enum domain<->SQL codecs` (status:todo) +- [x] T02: `Add revision and enum domain<->SQL codecs` (status:done) - Task ID: T02 - Scope: In — create `cli/src/services/mutation_trace/store.rs` with `encode_revision`/`decode_revision` (`u64` <-> 8-byte big-endian `BLOB`) and explicit codecs for `ActorKind`, `FailureKind`, `ScopeStatus`, `Attribution`'s discriminant, and `Boundary`'s discriminant. Out — any query, projection, or commit logic (T03+). - Dependencies: T01 - Done when: `encode_revision`/`decode_revision` round-trip exactly for `0`, `1`, `i64::MAX`, `i64::MAX + 1`, and `u64::MAX`; every enum variant round-trips through its codec; no codec relies on `Debug` formatting or implicit serde representation. - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml mutation_trace::store::` - - Context synchronization: pending + - Completed: 2026-08-27 + - Files changed: `cli/src/services/mutation_trace/store.rs` (new); `cli/src/services/mutation_trace/mod.rs` + - Result: Added `cli/src/services/mutation_trace/store.rs` with `encode_revision`/`decode_revision` (`u64` <-> `[u8; 8]` big-endian) and explicit `encode_*`/`decode_*` function-pair codecs for `ActorKind`, `FailureKind`, `ScopeStatus`, a new `AttributionKind` discriminant type (`ineligible_unscoped`/`ai_exclusive`/`ai_contended`, derived from `Attribution` via a new `attribution_kind` accessor), and a new `BoundaryKind` discriminant type (`start`/`advance`/`close`/`flush`, derived from `Boundary` via a new `boundary_kind` accessor) — every string constant matches migration `003`'s `CHECK (... IN (...))` allow-lists exactly. Decode functions return `anyhow::Result` and reject unrecognized strings via `anyhow::bail!`, matching the crate's existing `agent_trace_db`/`repository.rs` error convention. No codec derives from or matches on `Debug` output. Added `pub mod store;` to `mod.rs` so the module compiles and the `mutation_trace::store::` test path resolves. `Attribution`'s and `Boundary`'s full payload fields (`attribution_scope_id`, `boundary_scope_id`/`boundary_event_id`) are left to the row-reconstruction logic in T03/T07, matching the task's "discriminant"-only scope. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml mutation_trace::store::` — passed, 12/12; `./scripts/run-cli-cargo.sh fmt --manifest-path cli/Cargo.toml -- --check` — passed after one auto-formatting pass (no manual diff needed beyond `cargo fmt`); `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` — passed, zero warnings. + - Done checks: `encode_revision`/`decode_revision` round-trip exactly for `0`, `1`, `i64::MAX`, `i64::MAX + 1`, `u64::MAX` (verified by `revision_round_trips_at_boundary_values`); every `ActorKind`/`FailureKind`/`ScopeStatus`/`AttributionKind`/`BoundaryKind` variant round-trips through its own codec (verified by five dedicated `*_round_trips_every_variant` tests); no codec relies on `Debug` formatting or implicit serde representation (verified by inspection — every codec is a hand-written `match` over string literals, no `#[derive(Display)]`/serde attribute anywhere in the file). + - Context impact: local — new codec functions and types confined to a new, not-yet-wired-in file; no caller exists yet (T03+ will be the first consumer), so no root context file describes runtime behavior this changes yet. `context/cli/mutation-trace-protocol.md`'s "not yet wired into any hook, command, or database call site" framing remains accurate until a real DB call site lands (T07), matching the plan's assumption that this update is deferred to task context synchronization once that framing goes stale. + - Context synchronization: synced - [ ] T03: `Add bounded WorktreeProjection load and cold-path MutationEvent read` (status:todo) - Task ID: T03 From 1027a98b15f870cf62f711a281910ca6536790d1 Mon Sep 17 00:00:00 2001 From: David Abram Date: Thu, 27 Aug 2026 14:18:43 +0200 Subject: [PATCH 4/5] agent-trace-db: Remove redundant worktree identity from processed events Use `(scope_id, event_id)` as the sole processed-event identity, matching `EventKey` and avoiding duplicate worktree ownership data. Remove the redundant column and index, and add a regression test covering the composite key. Plan: mutation-cursor-store-persistence (T01, T03) Co-authored-by: SCE --- .../003_mutation_trace_protocol.sql | 11 ++-- cli/src/services/agent_trace_db/repository.rs | 51 ++++++++++++++++++- .../mutation-cursor-store-persistence.md | 30 ++++++++--- 3 files changed, 79 insertions(+), 13 deletions(-) diff --git a/cli/migrations/agent-trace-repository/003_mutation_trace_protocol.sql b/cli/migrations/agent-trace-repository/003_mutation_trace_protocol.sql index d6bdd867..a4657e4e 100644 --- a/cli/migrations/agent-trace-repository/003_mutation_trace_protocol.sql +++ b/cli/migrations/agent-trace-repository/003_mutation_trace_protocol.sql @@ -38,17 +38,20 @@ ON mutation_trace_scopes (worktree_id); CREATE INDEX IF NOT EXISTS idx_mutation_trace_scopes_worktree_status ON mutation_trace_scopes (worktree_id, status); +-- `worktree_id` is deliberately not duplicated here: a processed event's +-- identity is exactly `(scope_id, event_id)` (the domain `EventKey`), and +-- `scope_id`'s worktree is already a permanent fact owned by +-- `mutation_trace_scopes` (`ScopeId -> WorktreeId`, never reassigned). A +-- second `worktree_id` column would create two sources of truth for the same +-- fact and could disagree with `mutation_trace_scopes` for the same +-- `scope_id`; the schema does not represent that inconsistency. CREATE TABLE IF NOT EXISTS mutation_trace_processed_events ( scope_id TEXT NOT NULL, event_id TEXT NOT NULL, - worktree_id TEXT NOT NULL, created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), PRIMARY KEY (scope_id, event_id) ); -CREATE INDEX IF NOT EXISTS idx_mutation_trace_processed_events_worktree -ON mutation_trace_processed_events (worktree_id); - CREATE TABLE IF NOT EXISTS mutation_trace_events ( worktree_id TEXT NOT NULL, revision BLOB NOT NULL diff --git a/cli/src/services/agent_trace_db/repository.rs b/cli/src/services/agent_trace_db/repository.rs index 44792d30..75ece484 100644 --- a/cli/src/services/agent_trace_db/repository.rs +++ b/cli/src/services/agent_trace_db/repository.rs @@ -408,7 +408,6 @@ mod tests { "idx_parts_session_message_order", "idx_mutation_trace_scopes_worktree", "idx_mutation_trace_scopes_worktree_status", - "idx_mutation_trace_processed_events_worktree", ] { assert!( sqlite_object_exists(&db, "index", index), @@ -510,6 +509,56 @@ mod tests { remove_test_db(&db_path); } + #[test] + fn mutation_trace_processed_events_identity_is_scope_and_event_only() { + let db_path = unique_test_db_path("mutation-trace-processed-events-identity"); + let db = RepositoryAgentTraceDb::new_at(&db_path).expect("repository DB should open"); + + let sql = table_sql(&db, "mutation_trace_processed_events"); + assert!( + !sql.contains("worktree_id"), + "mutation_trace_processed_events must not have a worktree_id column: {sql}" + ); + + db.execute( + "INSERT INTO mutation_trace_processed_events (scope_id, event_id) + VALUES ('scope-1', 'event-1')", + (), + ) + .expect("first (scope_id, event_id) insert should succeed"); + + let duplicate_error = db + .execute( + "INSERT INTO mutation_trace_processed_events (scope_id, event_id) + VALUES ('scope-1', 'event-1')", + (), + ) + .expect_err("a duplicate (scope_id, event_id) pair must be rejected"); + assert!( + duplicate_error.to_string().contains("UNIQUE") + || duplicate_error.to_string().contains("PRIMARY KEY"), + "unexpected error: {duplicate_error}" + ); + + db.execute( + "INSERT INTO mutation_trace_processed_events (scope_id, event_id) + VALUES ('scope-2', 'event-1')", + (), + ) + .expect("the same event_id under a different scope_id should be allowed"); + + db.execute( + "INSERT INTO mutation_trace_processed_events (scope_id, event_id) + VALUES ('scope-1', 'event-2')", + (), + ) + .expect("the same scope_id with a different event_id should be allowed"); + + assert_eq!(row_count(&db, "mutation_trace_processed_events"), 3); + + remove_test_db(&db_path); + } + #[test] fn trace_tables_have_no_checkout_id_columns() { let db_path = unique_test_db_path("no-checkout-id"); diff --git a/context/plans/mutation-cursor-store-persistence.md b/context/plans/mutation-cursor-store-persistence.md index 096600c8..4432e784 100644 --- a/context/plans/mutation-cursor-store-persistence.md +++ b/context/plans/mutation-cursor-store-persistence.md @@ -70,8 +70,8 @@ error without retry. - Validate: T08 injected-failure test asserts revision, scope status, processed event, mutation event, and active scopes are all unchanged after rollback. - [ ] AC12: Process restart reconstructs the same durable protocol projection. - Validate: T09 tests that drop and reopen the DB handle before reloading. -- [ ] AC13: Historical mutation events are not loaded on each boundary, terminal (`Closed`/`Abandoned`/`NeverSeen`) historical scopes are not loaded on each boundary unless explicitly referenced, and a referenced scope belonging to a different worktree is rejected rather than silently loaded or reassigned. - - Validate: `MutationTraceStore::load_worktree` issues no query against `mutation_trace_events`, loads only currently `Active` scopes plus the explicitly referenced scope (if any), and returns `Err` when that referenced scope's persisted `worktree_id` does not match the requested worktree (T03 done-when). +- [ ] AC13: Historical mutation events are not loaded on each boundary; terminal (`Closed`/`Abandoned`/`NeverSeen`) historical scopes are not loaded on each boundary unless they are the effective referenced scope (`scope`, or `event_key.scope_id` when `scope` is absent); explicit `scope` and `event_key.scope_id` must agree when both are supplied, or `load_worktree` returns `Err`; and the effective referenced scope must belong to the requested worktree, or `load_worktree` returns `Err` rather than silently loading or reassigning it. + - Validate: `MutationTraceStore::load_worktree` issues no query against `mutation_trace_events`, loads only currently `Active` scopes plus the effective referenced scope (if any) derived from `scope`/`event_key` per T03's four-case definition, returns `Err` when `scope` and `event_key.scope_id` are both supplied and differ, and returns `Err` when the effective referenced scope's persisted `worktree_id` does not match the requested worktree (T03 done-when). - [ ] AC14: Existing Quint Connect and protocol tests remain green. - Validate: `nix flake check` (runs `cli-tests`, including `mutation_trace::mbt`, and the dedicated `mutation-trace-quint-connect` check). - [ ] AC15: No Git/filesystem lock/hook/coordinator integration is added. @@ -121,15 +121,15 @@ Persist this field in every plan; this is durable plan state, not chat state: - [x] T01: `Add migration 003 for mutation-trace protocol tables` (status:done) - Task ID: T01 - - Scope: In — `cli/migrations/agent-trace-repository/003_mutation_trace_protocol.sql` defining `mutation_trace_worktrees` (revision `BLOB` constrained by `CHECK (typeof(revision) = 'blob' AND length(revision) = 8)`), `mutation_trace_scopes` (+ `idx_mutation_trace_scopes_worktree` and a new composite `idx_mutation_trace_scopes_worktree_status` index on `(worktree_id, status)` for the bounded hot-path scope lookup), `mutation_trace_processed_events`, `mutation_trace_events` (+ the same `typeof`/`length` revision `CHECK`, plus payload-consistency `CHECK` constraints), and `mutation_trace_event_active_scopes`. Out — any Rust code consuming these tables (T02+). + - Scope: In — `cli/migrations/agent-trace-repository/003_mutation_trace_protocol.sql` defining `mutation_trace_worktrees` (revision `BLOB` constrained by `CHECK (typeof(revision) = 'blob' AND length(revision) = 8)`), `mutation_trace_scopes` (+ `idx_mutation_trace_scopes_worktree` and a new composite `idx_mutation_trace_scopes_worktree_status` index on `(worktree_id, status)` for the bounded hot-path scope lookup), `mutation_trace_processed_events` (identity `PRIMARY KEY (scope_id, event_id)` only, matching the domain `EventKey`; no `worktree_id` column or index — a scope's worktree is already a permanent fact owned by `mutation_trace_scopes`), `mutation_trace_events` (+ the same `typeof`/`length` revision `CHECK`, plus payload-consistency `CHECK` constraints), and `mutation_trace_event_active_scopes`. Out — any Rust code consuming these tables (T02+). - Dependencies: none - Done when: a fresh `RepositoryAgentTraceDb::new_at` at a clean path applies `001`+`002`+`003` and all five tables exist with the specified columns, constraints, and indexes; a row violating a `CHECK` constraint (for example `ai_exclusive` attribution with a `NULL` `attribution_scope_id`) is rejected; a `TEXT` value of length 8 assigned to a `revision` column is rejected by the `typeof(revision) = 'blob'` check even though its length matches. - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml agent_trace_db::repository::`; a new targeted test asserting the `003` tables, indexes, and constraints (including the TEXT-vs-BLOB revision case) behave as specified. - Completed: 2026-08-27 - Files changed: `cli/migrations/agent-trace-repository/003_mutation_trace_protocol.sql` (new); `cli/src/services/agent_trace_db/repository.rs` - - Result: Added migration `003_mutation_trace_protocol.sql` defining `mutation_trace_worktrees`, `mutation_trace_scopes` (+ `idx_mutation_trace_scopes_worktree`, `idx_mutation_trace_scopes_worktree_status`), `mutation_trace_processed_events` (+ `idx_mutation_trace_processed_events_worktree`), `mutation_trace_events`, and `mutation_trace_event_active_scopes`, all discovered automatically by `build.rs`'s directory scan. Revision columns use `BLOB NOT NULL CHECK (typeof(revision) = 'blob' AND length(revision) = 8)`; enum-shaped columns use `TEXT` with `CHECK (... IN (...))` allow-lists following the existing `role`/`payload_type` convention; `mutation_trace_events` additionally enforces attribution/boundary payload-consistency `CHECK`s (`ai_exclusive` requires a non-null `attribution_scope_id`; hook boundaries require non-null `boundary_scope_id`/`boundary_event_id`, `flush` requires both null). Updated `open_at_initializes_the_full_schema_from_one_migration` to assert the new migration ID and the five new tables/indexes, and added two new targeted tests (`mutation_trace_worktrees_revision_must_be_a_blob_not_matching_length_text`, `mutation_trace_events_ai_exclusive_attribution_requires_a_scope_id`) proving the TEXT-vs-BLOB revision rejection and the `ai_exclusive`-requires-scope rejection, each paired with a positive control insert. - - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml agent_trace_db::repository::` — passed, 18/18 (including the two new tests and the updated baseline-schema test); `./scripts/run-cli-cargo.sh fmt --manifest-path cli/Cargo.toml -- --check` — passed (no diff). - - Done checks: fresh DB applies `001`+`002`+`003` with all five tables/indexes present (verified by the updated baseline test); `ai_exclusive` attribution with a `NULL` `attribution_scope_id` is rejected (verified); an 8-byte TEXT value assigned to `revision` is rejected by `typeof(revision) = 'blob'` (verified); `git diff --exit-code` on `001`/`002` shows zero changes (verified). + - Result: Added migration `003_mutation_trace_protocol.sql` defining `mutation_trace_worktrees`, `mutation_trace_scopes` (+ `idx_mutation_trace_scopes_worktree`, `idx_mutation_trace_scopes_worktree_status`), `mutation_trace_processed_events` (identity `PRIMARY KEY (scope_id, event_id)` only — no `worktree_id` column or index), `mutation_trace_events`, and `mutation_trace_event_active_scopes`, all discovered automatically by `build.rs`'s directory scan. Revision columns use `BLOB NOT NULL CHECK (typeof(revision) = 'blob' AND length(revision) = 8)`; enum-shaped columns use `TEXT` with `CHECK (... IN (...))` allow-lists following the existing `role`/`payload_type` convention; `mutation_trace_events` additionally enforces attribution/boundary payload-consistency `CHECK`s (`ai_exclusive` requires a non-null `attribution_scope_id`; hook boundaries require non-null `boundary_scope_id`/`boundary_event_id`, `flush` requires both null). Updated `open_at_initializes_the_full_schema_from_one_migration` to assert the new migration ID and the five new tables/indexes, and added targeted tests (`mutation_trace_worktrees_revision_must_be_a_blob_not_matching_length_text`, `mutation_trace_events_ai_exclusive_attribution_requires_a_scope_id`, `mutation_trace_processed_events_identity_is_scope_and_event_only`) proving the TEXT-vs-BLOB revision rejection, the `ai_exclusive`-requires-scope rejection, and the `(scope_id, event_id)`-only processed-event identity, each paired with a positive control insert. `mutation_trace_processed_events` originally also carried a `worktree_id` column and `idx_mutation_trace_processed_events_worktree` index; both were removed by a later schema correction since a processed event's identity is exactly `(scope_id, event_id)` and its worktree is already a permanent fact owned by `mutation_trace_scopes`. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml agent_trace_db::repository::` — passed, 19/19 (including the corrected baseline-schema test and the three targeted tests above); `./scripts/run-cli-cargo.sh fmt --manifest-path cli/Cargo.toml -- --check` — passed (no diff). + - Done checks: fresh DB applies `001`+`002`+`003` with all five tables/indexes present (verified by the updated baseline test); `ai_exclusive` attribution with a `NULL` `attribution_scope_id` is rejected (verified); an 8-byte TEXT value assigned to `revision` is rejected by `typeof(revision) = 'blob'` (verified); `git diff --exit-code` on `001`/`002` shows zero changes (verified); `mutation_trace_processed_events` has no `worktree_id` column and its identity is exactly `(scope_id, event_id)` (verified). - Context impact: local — additive schema-only migration; no Rust code consumes these new tables yet (T02+ wire codecs, loads, and commits against them). No durable context synchronization is required for this task; the plan's `Context sync` entries are authored by T11 once the full store lands. - Context synchronization: synced @@ -149,9 +149,23 @@ Persist this field in every plan; this is durable plan state, not chat state: - [ ] T03: `Add bounded WorktreeProjection load and cold-path MutationEvent read` (status:todo) - Task ID: T03 - - Scope: In — `WorktreeProjection` (+ `into_protocol_state`) and `MutationTraceStore` wrapping `&RepositoryAgentTraceDb`. `load_worktree(worktree: &WorktreeId, scope: Option<&ScopeId>, event_key: Option<&EventKey>)` loads exactly one worktree row, only its currently `Active` scopes plus the explicitly referenced `scope` when supplied (even when that scope is `NeverSeen`/`Closed`/`Abandoned`), and 0 or 1 matching processed-event row for `event_key`. A separate cold-path `load_mutation_event(worktree: &WorktreeId, revision: u64) -> Result>` reads one `mutation_trace_events` row plus its `mutation_trace_event_active_scopes` rows and reconstructs a complete `MutationEvent`, decoding `Attribution` exactly (including `AiExclusive(scope_id)`) and the complete `Boundary`. When `scope` is supplied and the persisted `ScopeState` for that `ScopeId` has a `worktree_id` different from the requested `worktree`, `load_worktree` returns `Err` — it never silently omits the scope, never includes it in the projection, and never reassigns it to the requested worktree, preserving the permanent `ScopeId` -> `WorktreeId` identity `register_scope` already enforces. Out — initialization/commit logic (T04/T07); calling `load_mutation_event` from `load_worktree` or from any hook-boundary path. + - Scope: In — `WorktreeProjection` (+ `into_protocol_state`) and `MutationTraceStore` wrapping `&RepositoryAgentTraceDb`. `load_worktree(worktree: &WorktreeId, scope: Option<&ScopeId>, event_key: Option<&EventKey>)` first derives one `effective_scope: Option<&ScopeId>` from `scope` and `event_key`. **Invariant:** `scope` and `event_key.scope_id` are two ways of referring to the same operation-local scope identity; when both are supplied they must agree; when only `event_key` is supplied, its `scope_id` becomes the effective referenced scope for projection loading and `WorktreeId` validation. This avoids relying on a separate `worktree_id` stored on processed events. Concretely: + - `scope = None`, `event_key = None` -> `effective_scope = None` (no referenced scope); only the requested worktree's `Active` scopes are loaded, with no extra terminal scope. + - `scope = Some(S)`, `event_key = None` -> `effective_scope = Some(S)`; `S` is loaded and validated exactly as already specified below (included regardless of status; `Err` if it belongs to another worktree; never silently omitted or reassigned). + - `scope = None`, `event_key = Some(K)` -> `effective_scope = Some(&K.scope_id)`. `K.scope_id` is treated as a referenced scope even though the explicit `scope` argument is absent: `load_worktree` loads the durable `ScopeState` for `K.scope_id`, validates its persisted `worktree_id` against the requested worktree, includes it in the projection regardless of status, and returns `Err` if it belongs to another worktree. The processed-event replay lookup is then performed solely by `WHERE scope_id = ? AND event_id = ?` using `K` — `mutation_trace_processed_events` gains no `worktree_id` column to perform this check. + - `scope = Some(S)`, `event_key = Some(K)`, `S == K.scope_id` -> `effective_scope = Some(S)`; that single `ScopeId` is loaded and validated once. + - `scope = Some(S)`, `event_key = Some(K)`, `S != K.scope_id` -> `load_worktree` returns `Err` before loading either scope and before performing the processed-event lookup. It never chooses one arbitrarily, never loads both scopes, never ignores the mismatch, and never performs the replay query anyway. + + `load_worktree` then loads exactly one worktree row, only its currently `Active` scopes plus the scope named by `effective_scope` (even when that scope is `NeverSeen`/`Closed`/`Abandoned`), and — when `event_key` is supplied and no `S != K.scope_id` mismatch already returned `Err` — 0 or 1 matching processed-event row for `event_key`. The processed-event lookup is keyed solely by `event_key`'s `(scope_id, event_id)` — `WHERE scope_id = ? AND event_id = ?`, never filtered or joined by `worktree_id` — since `mutation_trace_processed_events` carries no `worktree_id` column (removed from migration `003`; the table's only identity is `PRIMARY KEY (scope_id, event_id)`, matching the domain `EventKey`). The worktree relationship for `event_key`'s scope is established by loading and validating its durable `ScopeState` as part of `effective_scope` above, not by a `worktree_id` column on the processed-event table: `EventKey.scope_id` -> `mutation_trace_scopes.scope_id` -> `mutation_trace_scopes.worktree_id`. A separate cold-path `load_mutation_event(worktree: &WorktreeId, revision: u64) -> Result>` reads one `mutation_trace_events` row plus its `mutation_trace_event_active_scopes` rows and reconstructs a complete `MutationEvent`, decoding `Attribution` exactly (including `AiExclusive(scope_id)`) and the complete `Boundary`. When `effective_scope` is `Some(S)` and the persisted `ScopeState` for `S` has a `worktree_id` different from the requested `worktree`, `load_worktree` returns `Err` — it never silently omits the scope, never includes it in the projection, and never reassigns it to the requested worktree, preserving the permanent `ScopeId` -> `WorktreeId` identity `register_scope` already enforces. This is the same check whether `S` came from the explicit `scope` argument or from `event_key.scope_id`. Out — initialization/commit logic (T04/T07); calling `load_mutation_event` from `load_worktree` or from any hook-boundary path. - Dependencies: T01, T02 - - Done when: `load_worktree` returns `None` for a missing worktree and `Some(projection)` otherwise, with `scopes` containing every currently `Active` scope on that worktree plus `scope` when supplied regardless of its status, and never a `Closed`/`Abandoned`/`NeverSeen` scope that was not explicitly referenced; `attempts`, `mutation_events`, and `external_taint` stay empty; the method issues no query against `mutation_trace_events`. Specifically: a referenced scope on the requested worktree is included regardless of status; a referenced terminal (`Closed`/`Abandoned`/`NeverSeen`) scope on the requested worktree is included; an unreferenced terminal historical scope is excluded; a referenced scope whose persisted `worktree_id` belongs to a different worktree returns `Err`. `load_mutation_event` returns `None` when no row exists at that `(worktree, revision)` and otherwise reconstructs a `MutationEvent` whose `before_tree`/`after_tree`/`revision`/`tainted`/`failure_kind`/`attribution`/`boundary`/`active_scopes` exactly match what `store.commit` persisted. + - Done when: `load_worktree` returns `None` for a missing worktree and `Some(projection)` otherwise, with `scopes` containing every currently `Active` scope on that worktree plus the `effective_scope` (derived from `scope`/`event_key` per the four cases above) when one exists, regardless of its status, and never a `Closed`/`Abandoned`/`NeverSeen` scope that was not the effective referenced scope; `attempts`, `mutation_events`, and `external_taint` stay empty; the method issues no query against `mutation_trace_events`. Explicit test cases for all five `scope`/`event_key` combinations: + 1. `scope=None`, `event_key=None` -> only currently `Active` scopes on the requested worktree are loaded; no referenced scope. + 2. `scope=Some(S)`, `event_key=None` -> `S` is included as the effective referenced scope and validated; a wrong-worktree `S` returns `Err`. + 3. `scope=None`, `event_key=Some(K)` -> `K.scope_id` is loaded as the effective referenced scope; a wrong-worktree `K.scope_id` returns `Err`; the processed-event lookup for `K` still matches solely on `(scope_id, event_id)`. + 4. `scope=Some(S)`, `event_key=Some(K)`, `S == K.scope_id` -> succeeds, loading and validating that one `ScopeId` exactly once. + 5. `scope=Some(S)`, `event_key=Some(K)`, `S != K.scope_id` -> `load_worktree` returns `Err` without loading either scope and without performing the processed-event lookup. + + Also preserved: a referenced scope on the requested worktree is included regardless of status; a referenced terminal (`Closed`/`Abandoned`/`NeverSeen`) scope on the requested worktree is included; an unreferenced terminal historical scope is excluded; the processed-event query never references `worktree_id` (it has no such column) and matches solely on `scope_id`/`event_id`; `load_worktree` never queries historical `mutation_trace_events` rows. `load_mutation_event` returns `None` when no row exists at that `(worktree, revision)` and otherwise reconstructs a `MutationEvent` whose `before_tree`/`after_tree`/`revision`/`tainted`/`failure_kind`/`attribution`/`boundary`/`active_scopes` exactly match what `store.commit` persisted. - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml mutation_trace::store::` - Context synchronization: pending From 50295131e761d650a590405fbe7718d0f19ecf3a Mon Sep 17 00:00:00 2001 From: David Abram Date: Thu, 27 Aug 2026 14:35:43 +0200 Subject: [PATCH 5/5] mutation-trace: Add bounded store read projections Load one worktree's active and referenced scopes plus processed-event state without querying historical events on the hot path, and reconstruct cold-path mutation events with full attribution and boundary data. Preserve transient protocol fields as empty in widened projections and reject inconsistent persisted payloads. Plan: mutation-cursor-store-persistence (T03) Co-authored-by: SCE --- cli/src/services/mutation_trace/store.rs | 948 +++++++++++++++++- .../mutation-cursor-store-persistence.md | 22 +- 2 files changed, 957 insertions(+), 13 deletions(-) diff --git a/cli/src/services/mutation_trace/store.rs b/cli/src/services/mutation_trace/store.rs index fefa69de..345f3b17 100644 --- a/cli/src/services/mutation_trace/store.rs +++ b/cli/src/services/mutation_trace/store.rs @@ -1,19 +1,29 @@ -//! Domain<->SQL codecs for the mutation-cursor persistence layer. +//! Domain<->SQL codecs and bounded read access for the mutation-cursor +//! persistence layer. //! -//! These codecs are the only translation between `super::types` domain -//! values and the `TEXT`/`BLOB` representations `cli/migrations/agent-trace- +//! The codecs are the only translation between `super::types` domain values +//! and the `TEXT`/`BLOB` representations `cli/migrations/agent-trace- //! repository/003_mutation_trace_protocol.sql` constrains those columns to. //! Every codec here is an explicit function over a fixed set of variants — no //! codec derives from `Debug` or a serde representation, so a variant rename //! cannot silently change the durable encoding. //! -//! This module carries no query, projection, or commit logic yet (see -//! `mutation-cursor-store-persistence` plan tasks T03+); it only establishes -//! the byte- and string-level encodings later tasks build on. +//! `MutationTraceStore` adds the hot-path bounded worktree read +//! (`load_worktree`) and the cold-path historical read (`load_mutation_event`) +//! against a `&RepositoryAgentTraceDb`. Initialization and CAS-commit logic +//! are later tasks (`mutation-cursor-store-persistence` T04/T06/T07); this +//! module carries no such logic yet. -use anyhow::{bail, Result}; +use std::collections::{BTreeMap, BTreeSet}; -use super::types::{ActorKind, Attribution, Boundary, FailureKind, ScopeStatus}; +use anyhow::{bail, Context, Result}; + +use crate::services::agent_trace_db::repository::RepositoryAgentTraceDb; + +use super::types::{ + ActorKind, Attribution, Boundary, EventId, EventKey, FailureKind, MutationEvent, ProtocolState, + ScopeId, ScopeState, ScopeStatus, TreeId, WorktreeId, WorktreeState, +}; /// Encodes a worktree/event revision as the 8-byte big-endian `BLOB` stored /// by every `revision` column in migration `003`. @@ -181,6 +191,417 @@ pub fn decode_boundary_kind(value: &str) -> Result { } } +const SELECT_WORKTREE_SQL: &str = + "SELECT cursor_tree, revision, tainted, failure_kind, needs_rebaseline + FROM mutation_trace_worktrees WHERE worktree_id = ?1"; +const SELECT_SCOPES_BY_WORKTREE_AND_STATUS_SQL: &str = + "SELECT scope_id, worktree_id, actor_kind, status + FROM mutation_trace_scopes WHERE worktree_id = ?1 AND status = ?2"; +const SELECT_SCOPE_BY_ID_SQL: &str = "SELECT scope_id, worktree_id, actor_kind, status + FROM mutation_trace_scopes WHERE scope_id = ?1"; +const SELECT_PROCESSED_EVENT_SQL: &str = + "SELECT 1 FROM mutation_trace_processed_events WHERE scope_id = ?1 AND event_id = ?2"; +const SELECT_MUTATION_EVENT_SQL: &str = "SELECT before_tree, after_tree, tainted, failure_kind, + attribution_kind, attribution_scope_id, boundary_kind, boundary_scope_id, boundary_event_id + FROM mutation_trace_events WHERE worktree_id = ?1 AND revision = ?2"; +const SELECT_MUTATION_EVENT_ACTIVE_SCOPES_SQL: &str = + "SELECT scope_id FROM mutation_trace_event_active_scopes WHERE worktree_id = ?1 AND revision = ?2"; + +/// Bounded runtime projection of one worktree's durable protocol state, +/// loaded by [`MutationTraceStore::load_worktree`]. Scoped to that worktree's +/// currently `Active` scopes plus, when present, the scope `load_worktree` +/// was explicitly asked about (regardless of its status) — never every +/// historical scope, and never a `mutation_trace_events` row. +/// +/// `attempts`, `mutation_events`, and `external_taint` are always empty: +/// `AttemptState` is transient and never persisted, historical +/// `MutationEvent`s are a cold-path concern +/// ([`MutationTraceStore::load_mutation_event`]), and `external_taint` is +/// never DB-authoritative (see the plan's non-goals). +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct WorktreeProjection { + pub worktree_id: WorktreeId, + pub worktree_state: WorktreeState, + pub scopes: BTreeMap, + pub processed_events: BTreeSet, +} + +impl WorktreeProjection { + /// Widens this bounded projection into a full [`ProtocolState`] so pure + /// `protocol.rs` functions can operate on it unchanged. `worktrees` + /// carries only the one loaded worktree; `attempts`, `mutation_events`, + /// and `external_taint` are always empty. + pub fn into_protocol_state(self) -> ProtocolState { + let mut worktrees = BTreeMap::new(); + worktrees.insert(self.worktree_id, self.worktree_state); + + ProtocolState { + worktrees, + scopes: self.scopes, + external_taint: BTreeSet::new(), + processed_events: self.processed_events, + attempts: BTreeMap::new(), + mutation_events: BTreeSet::new(), + } + } +} + +/// Bounded read access to the durable mutation-cursor protocol state for one +/// repository, via [`RepositoryAgentTraceDb`]. Write/CAS-commit access is +/// added by later tasks (T04/T06/T07). +pub struct MutationTraceStore<'a> { + db: &'a RepositoryAgentTraceDb, +} + +impl<'a> MutationTraceStore<'a> { + pub fn new(db: &'a RepositoryAgentTraceDb) -> Self { + Self { db } + } + + /// Loads a bounded projection of `worktree`'s durable protocol state, or + /// `None` when the worktree does not exist. + /// + /// `scope` and `event_key.scope_id` are two ways of naming the same + /// operation-local scope identity: when both are supplied they must + /// agree, or this returns `Err` before loading or querying anything. + /// Otherwise the supplied `scope`, or `event_key.scope_id` when only + /// `event_key` is supplied, becomes the effective referenced scope: a + /// durable `mutation_trace_scopes` row for it must exist, or this returns + /// `Err` — a missing effective scope is never silently omitted from the + /// projection. When it exists it is loaded and included in the + /// projection regardless of its status, and this returns `Err` if it + /// belongs to a worktree other than the one requested. Both checks run + /// before the `processed_events` replay lookup, so an orphan + /// `mutation_trace_processed_events` row can never enter the projection + /// without its owning scope. The projection's `scopes` otherwise contains + /// only this worktree's currently `Active` scopes. `processed_events` + /// contains `event_key` only when a matching `(scope_id, event_id)` row + /// already exists; the lookup never references a `worktree_id` column, + /// since `mutation_trace_processed_events` has none. This method never + /// queries `mutation_trace_events`. + pub fn load_worktree( + &self, + worktree: &WorktreeId, + scope: Option<&ScopeId>, + event_key: Option<&EventKey>, + ) -> Result> { + let effective_scope = effective_referenced_scope(scope, event_key)?; + + let Some(worktree_state) = self.load_worktree_state(worktree)? else { + return Ok(None); + }; + + let mut scopes = self.load_active_scopes(worktree)?; + + if let Some(effective_scope_id) = effective_scope { + if !scopes.contains_key(effective_scope_id) { + let scope_state = self.load_scope(effective_scope_id)?.ok_or_else(|| { + anyhow::anyhow!( + "effective referenced scope {effective_scope_id:?} has no mutation_trace_scopes row" + ) + })?; + if scope_state.worktree_id != *worktree { + bail!( + "scope {:?} belongs to worktree {:?}, not the requested worktree {:?}", + effective_scope_id, + scope_state.worktree_id, + worktree + ); + } + scopes.insert(effective_scope_id.clone(), scope_state); + } + } + + let processed_events = match event_key { + Some(event_key) if self.processed_event_exists(event_key)? => { + let mut processed_events = BTreeSet::new(); + processed_events.insert(event_key.clone()); + processed_events + } + _ => BTreeSet::new(), + }; + + Ok(Some(WorktreeProjection { + worktree_id: worktree.clone(), + worktree_state, + scopes, + processed_events, + })) + } + + /// Reconstructs one historical [`MutationEvent`] for `(worktree, + /// revision)`, decoding its full `Attribution` and `Boundary`, or `None` + /// when no such row exists. Never called from `load_worktree` or from + /// any hook-boundary path. + pub fn load_mutation_event( + &self, + worktree: &WorktreeId, + revision: u64, + ) -> Result> { + let revision_blob = encode_revision(revision); + + let rows = self.db.query_map( + SELECT_MUTATION_EVENT_SQL, + (worktree.0.as_str(), revision_blob.as_slice()), + mutation_event_row_from_turso, + )?; + + let Some(row) = rows.into_iter().next() else { + return Ok(None); + }; + + let active_scopes = self.load_mutation_event_active_scopes(worktree, &revision_blob)?; + + Ok(Some(MutationEvent { + worktree_id: worktree.clone(), + revision, + before_tree: TreeId(row.before_tree), + after_tree: TreeId(row.after_tree), + active_scopes, + tainted: row.tainted, + failure_kind: row.failure_kind, + attribution: reconstruct_attribution(row.attribution_kind, row.attribution_scope_id)?, + boundary: reconstruct_boundary( + row.boundary_kind, + worktree, + row.boundary_scope_id, + row.boundary_event_id, + )?, + })) + } + + fn load_worktree_state(&self, worktree: &WorktreeId) -> Result> { + let rows = self.db.query_map( + SELECT_WORKTREE_SQL, + (worktree.0.as_str(),), + worktree_state_row_from_turso, + )?; + + Ok(rows.into_iter().next()) + } + + fn load_active_scopes(&self, worktree: &WorktreeId) -> Result> { + let rows = self.db.query_map( + SELECT_SCOPES_BY_WORKTREE_AND_STATUS_SQL, + ( + worktree.0.as_str(), + encode_scope_status(ScopeStatus::Active), + ), + scope_row_from_turso, + )?; + + 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, + (event_key.scope_id.0.as_str(), event_key.event_id.0.as_str()), + |row| row.get::(0).map_err(Into::into), + )?; + + Ok(!rows.is_empty()) + } + + fn load_mutation_event_active_scopes( + &self, + worktree: &WorktreeId, + revision_blob: &[u8], + ) -> Result> { + let rows = self.db.query_map( + SELECT_MUTATION_EVENT_ACTIVE_SCOPES_SQL, + (worktree.0.as_str(), revision_blob), + |row| row.get::(0).map(ScopeId).map_err(Into::into), + )?; + + Ok(rows.into_iter().collect()) + } +} + +/// Derives the single effective referenced scope from `scope` and +/// `event_key`, per the four-case definition in the +/// `mutation-cursor-store-persistence` plan's T03: `None` when neither is +/// supplied; the supplied one when only one is; the agreeing identity when +/// both are supplied and equal; `Err` when both are supplied and disagree. +fn effective_referenced_scope<'k>( + scope: Option<&'k ScopeId>, + event_key: Option<&'k EventKey>, +) -> Result> { + match (scope, event_key) { + (None, None) => Ok(None), + (Some(scope_id), None) => Ok(Some(scope_id)), + (None, Some(event_key)) => Ok(Some(&event_key.scope_id)), + (Some(scope_id), Some(event_key)) if *scope_id == event_key.scope_id => Ok(Some(scope_id)), + (Some(scope_id), Some(event_key)) => bail!( + "scope {scope_id:?} and event_key.scope_id {:?} disagree", + event_key.scope_id + ), + } +} + +fn worktree_state_row_from_turso(row: &turso::Row) -> Result { + let cursor_tree: String = row + .get(0) + .context("failed to read mutation_trace_worktrees.cursor_tree")?; + let revision_blob: Vec = row + .get(1) + .context("failed to read mutation_trace_worktrees.revision")?; + let tainted: bool = row + .get(2) + .context("failed to read mutation_trace_worktrees.tainted")?; + let failure_kind: String = row + .get(3) + .context("failed to read mutation_trace_worktrees.failure_kind")?; + let needs_rebaseline: bool = row + .get(4) + .context("failed to read mutation_trace_worktrees.needs_rebaseline")?; + + Ok(WorktreeState { + cursor_tree: TreeId(cursor_tree), + revision: decode_revision(&revision_blob)?, + tainted, + failure_kind: decode_failure_kind(&failure_kind)?, + needs_rebaseline, + }) +} + +fn scope_row_from_turso(row: &turso::Row) -> Result<(ScopeId, ScopeState)> { + let scope_id: String = row + .get(0) + .context("failed to read mutation_trace_scopes.scope_id")?; + let worktree_id: String = row + .get(1) + .context("failed to read mutation_trace_scopes.worktree_id")?; + let actor_kind: String = row + .get(2) + .context("failed to read mutation_trace_scopes.actor_kind")?; + let status: String = row + .get(3) + .context("failed to read mutation_trace_scopes.status")?; + + Ok(( + ScopeId(scope_id), + ScopeState { + status: decode_scope_status(&status)?, + actor_kind: decode_actor_kind(&actor_kind)?, + worktree_id: WorktreeId(worktree_id), + }, + )) +} + +/// Raw decoded `mutation_trace_events` row fields, prior to reconstructing +/// the full `Attribution`/`Boundary`/`active_scopes` a [`MutationEvent`] +/// carries. +struct MutationEventRow { + before_tree: String, + after_tree: String, + tainted: bool, + failure_kind: FailureKind, + attribution_kind: AttributionKind, + attribution_scope_id: Option, + boundary_kind: BoundaryKind, + boundary_scope_id: Option, + boundary_event_id: Option, +} + +fn mutation_event_row_from_turso(row: &turso::Row) -> Result { + let before_tree: String = row + .get(0) + .context("failed to read mutation_trace_events.before_tree")?; + let after_tree: String = row + .get(1) + .context("failed to read mutation_trace_events.after_tree")?; + let tainted: bool = row + .get(2) + .context("failed to read mutation_trace_events.tainted")?; + let failure_kind: String = row + .get(3) + .context("failed to read mutation_trace_events.failure_kind")?; + let attribution_kind: String = row + .get(4) + .context("failed to read mutation_trace_events.attribution_kind")?; + let attribution_scope_id: Option = row + .get(5) + .context("failed to read mutation_trace_events.attribution_scope_id")?; + let boundary_kind: String = row + .get(6) + .context("failed to read mutation_trace_events.boundary_kind")?; + let boundary_scope_id: Option = row + .get(7) + .context("failed to read mutation_trace_events.boundary_scope_id")?; + let boundary_event_id: Option = row + .get(8) + .context("failed to read mutation_trace_events.boundary_event_id")?; + + Ok(MutationEventRow { + before_tree, + after_tree, + tainted, + failure_kind: decode_failure_kind(&failure_kind)?, + attribution_kind: decode_attribution_kind(&attribution_kind)?, + attribution_scope_id, + boundary_kind: decode_boundary_kind(&boundary_kind)?, + boundary_scope_id, + boundary_event_id, + }) +} + +fn reconstruct_attribution(kind: AttributionKind, scope_id: Option) -> Result { + match (kind, scope_id) { + (AttributionKind::IneligibleUnscoped, None) => Ok(Attribution::IneligibleUnscoped), + (AttributionKind::AiContended, None) => Ok(Attribution::AiContended), + (AttributionKind::AiExclusive, Some(scope_id)) => { + Ok(Attribution::AiExclusive(ScopeId(scope_id))) + } + (kind, scope_id) => { + bail!("inconsistent attribution row: kind={kind:?} scope_id={scope_id:?}") + } + } +} + +fn reconstruct_boundary( + kind: BoundaryKind, + worktree: &WorktreeId, + scope_id: Option, + event_id: Option, +) -> Result { + match kind { + BoundaryKind::Flush => { + if scope_id.is_some() || event_id.is_some() { + bail!("flush boundary row must not carry boundary_scope_id/boundary_event_id"); + } + Ok(Boundary::Flush { + worktree: worktree.clone(), + }) + } + BoundaryKind::Start | BoundaryKind::Advance | BoundaryKind::Close => { + let scope = scope_id + .map(ScopeId) + .ok_or_else(|| anyhow::anyhow!("hook boundary row missing boundary_scope_id"))?; + let event = event_id + .map(EventId) + .ok_or_else(|| anyhow::anyhow!("hook boundary row missing boundary_event_id"))?; + + Ok(match kind { + BoundaryKind::Start => Boundary::Start { scope, event }, + BoundaryKind::Advance => Boundary::Advance { scope, event }, + BoundaryKind::Close => Boundary::Close { scope, event }, + BoundaryKind::Flush => unreachable!("Flush handled above"), + }) + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -299,4 +720,515 @@ mod tests { fn decode_boundary_kind_rejects_unknown_value() { assert!(decode_boundary_kind("unknown").is_err()); } + + fn unique_test_db_path(label: &str) -> std::path::PathBuf { + let nonce = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("system time should be after Unix epoch") + .as_nanos(); + std::env::temp_dir() + .join(format!( + "sce-mutation-trace-store-{label}-{}-{nonce}", + std::process::id() + )) + .join("agent-trace.db") + } + + fn remove_test_db(db_path: &std::path::Path) { + if let Some(parent) = db_path.parent() { + std::fs::remove_dir_all(parent).expect("test DB directory should be removed"); + } + } + + fn insert_worktree(db: &RepositoryAgentTraceDb, worktree_id: &str, 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_id, encode_revision(revision).as_slice()), + ) + .expect("worktree insert should succeed"); + } + + fn insert_scope( + db: &RepositoryAgentTraceDb, + scope_id: &str, + worktree_id: &str, + status: ScopeStatus, + ) { + db.execute( + "INSERT INTO mutation_trace_scopes (scope_id, worktree_id, actor_kind, status) + VALUES (?1, ?2, 'claude_code', ?3)", + (scope_id, worktree_id, encode_scope_status(status)), + ) + .expect("scope insert should succeed"); + } + + fn insert_processed_event(db: &RepositoryAgentTraceDb, scope_id: &str, event_id: &str) { + db.execute( + "INSERT INTO mutation_trace_processed_events (scope_id, event_id) VALUES (?1, ?2)", + (scope_id, event_id), + ) + .expect("processed-event insert should succeed"); + } + + #[allow(clippy::too_many_arguments)] + fn insert_mutation_event( + db: &RepositoryAgentTraceDb, + worktree_id: &str, + revision: u64, + before_tree: &str, + after_tree: &str, + attribution_kind: &str, + attribution_scope_id: Option<&str>, + boundary_kind: &str, + boundary_scope_id: Option<&str>, + boundary_event_id: Option<&str>, + active_scopes: &[&str], + ) { + let revision_blob = encode_revision(revision); + + db.execute( + "INSERT INTO mutation_trace_events + (worktree_id, revision, before_tree, after_tree, tainted, failure_kind, + attribution_kind, attribution_scope_id, boundary_kind, boundary_scope_id, boundary_event_id) + VALUES (?1, ?2, ?3, ?4, 0, 'healthy', ?5, ?6, ?7, ?8, ?9)", + ( + worktree_id, + revision_blob.as_slice(), + before_tree, + after_tree, + attribution_kind, + attribution_scope_id, + boundary_kind, + boundary_scope_id, + boundary_event_id, + ), + ) + .expect("mutation event insert should succeed"); + + for scope_id in active_scopes { + db.execute( + "INSERT INTO mutation_trace_event_active_scopes (worktree_id, revision, scope_id) + VALUES (?1, ?2, ?3)", + (worktree_id, revision_blob.as_slice(), *scope_id), + ) + .expect("active-scope insert should succeed"); + } + } + + #[test] + fn load_worktree_returns_none_for_a_missing_worktree() { + let db_path = unique_test_db_path("missing-worktree"); + let db = RepositoryAgentTraceDb::new_at(&db_path).expect("repository DB should open"); + let store = MutationTraceStore::new(&db); + + let projection = store + .load_worktree(&WorktreeId("wt-missing".to_string()), None, None) + .expect("load_worktree should succeed"); + assert!(projection.is_none()); + + remove_test_db(&db_path); + } + + #[test] + fn load_worktree_with_no_scope_or_event_key_loads_only_active_scopes() { + let db_path = unique_test_db_path("case-1-active-only"); + let db = RepositoryAgentTraceDb::new_at(&db_path).expect("repository DB should open"); + let store = MutationTraceStore::new(&db); + + insert_worktree(&db, "wt-1", 5); + insert_scope(&db, "scope-active", "wt-1", ScopeStatus::Active); + insert_scope(&db, "scope-closed", "wt-1", ScopeStatus::Closed); + + let projection = store + .load_worktree(&WorktreeId("wt-1".to_string()), None, None) + .expect("load_worktree should succeed") + .expect("worktree should exist"); + + assert_eq!(projection.worktree_id, WorktreeId("wt-1".to_string())); + assert_eq!(projection.worktree_state.revision, 5); + assert_eq!( + projection.scopes.keys().collect::>(), + vec![&ScopeId("scope-active".to_string())] + ); + assert!(projection.processed_events.is_empty()); + + remove_test_db(&db_path); + } + + #[test] + fn load_worktree_with_explicit_scope_includes_it_regardless_of_status() { + let db_path = unique_test_db_path("case-2-explicit-scope"); + 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-closed", "wt-1", ScopeStatus::Closed); + + let projection = store + .load_worktree( + &WorktreeId("wt-1".to_string()), + Some(&ScopeId("scope-closed".to_string())), + None, + ) + .expect("load_worktree should succeed") + .expect("worktree should exist"); + + assert_eq!( + projection.scopes.get(&ScopeId("scope-closed".to_string())), + Some(&ScopeState { + status: ScopeStatus::Closed, + actor_kind: ActorKind::ClaudeCode, + worktree_id: WorktreeId("wt-1".to_string()), + }) + ); + + remove_test_db(&db_path); + } + + #[test] + fn load_worktree_with_explicit_scope_on_another_worktree_errors() { + let db_path = unique_test_db_path("case-2-wrong-worktree"); + let db = RepositoryAgentTraceDb::new_at(&db_path).expect("repository DB should open"); + let store = MutationTraceStore::new(&db); + + insert_worktree(&db, "wt-1", 0); + insert_worktree(&db, "wt-2", 0); + insert_scope(&db, "scope-1", "wt-2", ScopeStatus::Active); + + let error = store + .load_worktree( + &WorktreeId("wt-1".to_string()), + Some(&ScopeId("scope-1".to_string())), + None, + ) + .expect_err("scope belonging to another worktree should error"); + assert!(error.to_string().contains("scope-1")); + + remove_test_db(&db_path); + } + + #[test] + fn load_worktree_with_explicit_missing_scope_errors() { + let db_path = unique_test_db_path("case-2-missing-scope"); + let db = RepositoryAgentTraceDb::new_at(&db_path).expect("repository DB should open"); + let store = MutationTraceStore::new(&db); + + insert_worktree(&db, "wt-1", 0); + + let error = store + .load_worktree( + &WorktreeId("wt-1".to_string()), + Some(&ScopeId("scope-missing".to_string())), + None, + ) + .expect_err("missing effective scope should error"); + assert!(error.to_string().contains("scope-missing")); + + remove_test_db(&db_path); + } + + #[test] + fn load_worktree_with_only_event_key_loads_its_scope_and_replay_row() { + let db_path = unique_test_db_path("case-3-event-key-only"); + 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::NeverSeen); + insert_processed_event(&db, "scope-1", "event-1"); + + let event_key = EventKey { + scope_id: ScopeId("scope-1".to_string()), + event_id: EventId("event-1".to_string()), + }; + + let projection = store + .load_worktree(&WorktreeId("wt-1".to_string()), None, Some(&event_key)) + .expect("load_worktree should succeed") + .expect("worktree should exist"); + + assert_eq!( + projection + .scopes + .get(&ScopeId("scope-1".to_string())) + .map(|s| s.status), + Some(ScopeStatus::NeverSeen) + ); + assert_eq!( + projection.processed_events, + [event_key].into_iter().collect() + ); + + remove_test_db(&db_path); + } + + #[test] + fn load_worktree_with_event_key_scope_on_another_worktree_errors() { + let db_path = unique_test_db_path("case-3-wrong-worktree"); + let db = RepositoryAgentTraceDb::new_at(&db_path).expect("repository DB should open"); + let store = MutationTraceStore::new(&db); + + insert_worktree(&db, "wt-1", 0); + insert_worktree(&db, "wt-2", 0); + insert_scope(&db, "scope-1", "wt-2", ScopeStatus::Active); + + let event_key = EventKey { + scope_id: ScopeId("scope-1".to_string()), + event_id: EventId("event-1".to_string()), + }; + + let error = store + .load_worktree(&WorktreeId("wt-1".to_string()), None, Some(&event_key)) + .expect_err("event_key scope on another worktree should error"); + assert!(error.to_string().contains("scope-1")); + + remove_test_db(&db_path); + } + + #[test] + fn load_worktree_with_event_key_missing_scope_errors() { + let db_path = unique_test_db_path("case-3-missing-scope"); + let db = RepositoryAgentTraceDb::new_at(&db_path).expect("repository DB should open"); + let store = MutationTraceStore::new(&db); + + insert_worktree(&db, "wt-1", 0); + + let event_key = EventKey { + scope_id: ScopeId("scope-missing".to_string()), + event_id: EventId("event-1".to_string()), + }; + + let error = store + .load_worktree(&WorktreeId("wt-1".to_string()), None, Some(&event_key)) + .expect_err("missing event_key.scope_id should error"); + assert!(error.to_string().contains("scope-missing")); + + remove_test_db(&db_path); + } + + #[test] + fn load_worktree_with_event_key_missing_scope_and_orphan_replay_row_errors() { + let db_path = unique_test_db_path("case-3-orphan-replay-row"); + let db = RepositoryAgentTraceDb::new_at(&db_path).expect("repository DB should open"); + let store = MutationTraceStore::new(&db); + + insert_worktree(&db, "wt-1", 0); + insert_processed_event(&db, "scope-missing", "event-1"); + + let event_key = EventKey { + scope_id: ScopeId("scope-missing".to_string()), + event_id: EventId("event-1".to_string()), + }; + + let error = store + .load_worktree(&WorktreeId("wt-1".to_string()), None, Some(&event_key)) + .expect_err( + "an orphan processed-event row must not let a missing scope produce a projection", + ); + assert!(error.to_string().contains("scope-missing")); + + remove_test_db(&db_path); + } + + #[test] + fn load_worktree_with_agreeing_scope_and_event_key_loads_it_once() { + let db_path = unique_test_db_path("case-4-agreeing"); + 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 event_key = EventKey { + scope_id: ScopeId("scope-1".to_string()), + event_id: EventId("event-1".to_string()), + }; + + let projection = store + .load_worktree( + &WorktreeId("wt-1".to_string()), + Some(&ScopeId("scope-1".to_string())), + Some(&event_key), + ) + .expect("load_worktree should succeed") + .expect("worktree should exist"); + + assert_eq!(projection.scopes.len(), 1); + assert!(projection + .scopes + .contains_key(&ScopeId("scope-1".to_string()))); + + remove_test_db(&db_path); + } + + #[test] + fn load_worktree_with_disagreeing_scope_and_event_key_errors_without_loading() { + let db_path = unique_test_db_path("case-5-disagreeing"); + 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-a", "wt-1", ScopeStatus::Active); + insert_scope(&db, "scope-b", "wt-1", ScopeStatus::Active); + + let event_key = EventKey { + scope_id: ScopeId("scope-b".to_string()), + event_id: EventId("event-1".to_string()), + }; + + let error = store + .load_worktree( + &WorktreeId("wt-1".to_string()), + Some(&ScopeId("scope-a".to_string())), + Some(&event_key), + ) + .expect_err("disagreeing scope/event_key.scope_id should error"); + assert!(error.to_string().contains("scope-a")); + assert!(error.to_string().contains("scope-b")); + + remove_test_db(&db_path); + } + + #[test] + fn load_mutation_event_returns_none_when_missing() { + let db_path = unique_test_db_path("cold-path-missing"); + let db = RepositoryAgentTraceDb::new_at(&db_path).expect("repository DB should open"); + let store = MutationTraceStore::new(&db); + + let event = store + .load_mutation_event(&WorktreeId("wt-1".to_string()), 1) + .expect("load_mutation_event should succeed"); + assert!(event.is_none()); + + remove_test_db(&db_path); + } + + #[test] + fn load_mutation_event_reconstructs_ai_exclusive_start_event() { + let db_path = unique_test_db_path("cold-path-ai-exclusive-start"); + let db = RepositoryAgentTraceDb::new_at(&db_path).expect("repository DB should open"); + let store = MutationTraceStore::new(&db); + + insert_mutation_event( + &db, + "wt-1", + 1, + "tree-0", + "tree-1", + "ai_exclusive", + Some("scope-1"), + "start", + Some("scope-1"), + Some("event-1"), + &["scope-1"], + ); + + let event = store + .load_mutation_event(&WorktreeId("wt-1".to_string()), 1) + .expect("load_mutation_event should succeed") + .expect("mutation event row should exist"); + + assert_eq!( + event, + MutationEvent { + worktree_id: WorktreeId("wt-1".to_string()), + revision: 1, + before_tree: TreeId("tree-0".to_string()), + after_tree: TreeId("tree-1".to_string()), + active_scopes: [ScopeId("scope-1".to_string())].into_iter().collect(), + tainted: false, + failure_kind: FailureKind::Healthy, + attribution: Attribution::AiExclusive(ScopeId("scope-1".to_string())), + boundary: Boundary::Start { + scope: ScopeId("scope-1".to_string()), + event: EventId("event-1".to_string()), + }, + } + ); + + remove_test_db(&db_path); + } + + #[test] + fn load_mutation_event_reconstructs_a_flush_event_with_multiple_active_scopes() { + let db_path = unique_test_db_path("cold-path-flush"); + let db = RepositoryAgentTraceDb::new_at(&db_path).expect("repository DB should open"); + let store = MutationTraceStore::new(&db); + + insert_mutation_event( + &db, + "wt-1", + 3, + "tree-2", + "tree-3", + "ai_contended", + None, + "flush", + None, + None, + &["scope-1", "scope-2"], + ); + + let event = store + .load_mutation_event(&WorktreeId("wt-1".to_string()), 3) + .expect("load_mutation_event should succeed") + .expect("mutation event row should exist"); + + assert_eq!( + event, + MutationEvent { + worktree_id: WorktreeId("wt-1".to_string()), + revision: 3, + before_tree: TreeId("tree-2".to_string()), + after_tree: TreeId("tree-3".to_string()), + active_scopes: [ + ScopeId("scope-1".to_string()), + ScopeId("scope-2".to_string()) + ] + .into_iter() + .collect(), + tainted: false, + failure_kind: FailureKind::Healthy, + attribution: Attribution::AiContended, + boundary: Boundary::Flush { + worktree: WorktreeId("wt-1".to_string()), + }, + } + ); + + remove_test_db(&db_path); + } + + #[test] + fn into_protocol_state_carries_only_the_loaded_worktree_and_leaves_transient_fields_empty() { + let db_path = unique_test_db_path("into-protocol-state"); + 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::Active); + + let projection = store + .load_worktree(&WorktreeId("wt-1".to_string()), None, None) + .expect("load_worktree should succeed") + .expect("worktree should exist"); + + let protocol_state = projection.into_protocol_state(); + + assert_eq!(protocol_state.worktrees.len(), 1); + assert_eq!( + protocol_state + .worktrees + .get(&WorktreeId("wt-1".to_string())) + .map(|w| w.revision), + Some(7) + ); + assert!(protocol_state.attempts.is_empty()); + assert!(protocol_state.mutation_events.is_empty()); + assert!(protocol_state.external_taint.is_empty()); + + remove_test_db(&db_path); + } } diff --git a/context/plans/mutation-cursor-store-persistence.md b/context/plans/mutation-cursor-store-persistence.md index 4432e784..d16fc0bb 100644 --- a/context/plans/mutation-cursor-store-persistence.md +++ b/context/plans/mutation-cursor-store-persistence.md @@ -70,8 +70,8 @@ error without retry. - Validate: T08 injected-failure test asserts revision, scope status, processed event, mutation event, and active scopes are all unchanged after rollback. - [ ] AC12: Process restart reconstructs the same durable protocol projection. - Validate: T09 tests that drop and reopen the DB handle before reloading. -- [ ] AC13: Historical mutation events are not loaded on each boundary; terminal (`Closed`/`Abandoned`/`NeverSeen`) historical scopes are not loaded on each boundary unless they are the effective referenced scope (`scope`, or `event_key.scope_id` when `scope` is absent); explicit `scope` and `event_key.scope_id` must agree when both are supplied, or `load_worktree` returns `Err`; and the effective referenced scope must belong to the requested worktree, or `load_worktree` returns `Err` rather than silently loading or reassigning it. - - Validate: `MutationTraceStore::load_worktree` issues no query against `mutation_trace_events`, loads only currently `Active` scopes plus the effective referenced scope (if any) derived from `scope`/`event_key` per T03's four-case definition, returns `Err` when `scope` and `event_key.scope_id` are both supplied and differ, and returns `Err` when the effective referenced scope's persisted `worktree_id` does not match the requested worktree (T03 done-when). +- [ ] AC13: Historical mutation events are not loaded on each boundary; terminal (`Closed`/`Abandoned`/`NeverSeen`) historical scopes are not loaded on each boundary unless they are the effective referenced scope (`scope`, or `event_key.scope_id` when `scope` is absent); explicit `scope` and `event_key.scope_id` must agree when both are supplied, or `load_worktree` returns `Err`; the effective referenced scope must belong to the requested worktree, or `load_worktree` returns `Err` rather than silently loading or reassigning it; and the effective referenced scope must exist in durable `mutation_trace_scopes` storage — a missing effective scope returns `Err`, rather than `load_worktree` silently continuing with a projection that omits it, whether the effective scope came from `scope` or `event_key.scope_id`. + - Validate: `MutationTraceStore::load_worktree` issues no query against `mutation_trace_events`, loads only currently `Active` scopes plus the effective referenced scope (if any) derived from `scope`/`event_key` per T03's four-case definition, returns `Err` when `scope` and `event_key.scope_id` are both supplied and differ, returns `Err` when the effective referenced scope's persisted `worktree_id` does not match the requested worktree, and returns `Err` when the effective referenced scope has no durable `mutation_trace_scopes` row — including when an orphan `mutation_trace_processed_events` row exists for it (T03 done-when). - [ ] AC14: Existing Quint Connect and protocol tests remain green. - Validate: `nix flake check` (runs `cli-tests`, including `mutation_trace::mbt`, and the dedicated `mutation-trace-quint-connect` check). - [ ] AC15: No Git/filesystem lock/hook/coordinator integration is added. @@ -147,7 +147,7 @@ Persist this field in every plan; this is durable plan state, not chat state: - Context impact: local — new codec functions and types confined to a new, not-yet-wired-in file; no caller exists yet (T03+ will be the first consumer), so no root context file describes runtime behavior this changes yet. `context/cli/mutation-trace-protocol.md`'s "not yet wired into any hook, command, or database call site" framing remains accurate until a real DB call site lands (T07), matching the plan's assumption that this update is deferred to task context synchronization once that framing goes stale. - Context synchronization: synced -- [ ] T03: `Add bounded WorktreeProjection load and cold-path MutationEvent read` (status:todo) +- [x] T03: `Add bounded WorktreeProjection load and cold-path MutationEvent read` (status:done) - Task ID: T03 - Scope: In — `WorktreeProjection` (+ `into_protocol_state`) and `MutationTraceStore` wrapping `&RepositoryAgentTraceDb`. `load_worktree(worktree: &WorktreeId, scope: Option<&ScopeId>, event_key: Option<&EventKey>)` first derives one `effective_scope: Option<&ScopeId>` from `scope` and `event_key`. **Invariant:** `scope` and `event_key.scope_id` are two ways of referring to the same operation-local scope identity; when both are supplied they must agree; when only `event_key` is supplied, its `scope_id` becomes the effective referenced scope for projection loading and `WorktreeId` validation. This avoids relying on a separate `worktree_id` stored on processed events. Concretely: - `scope = None`, `event_key = None` -> `effective_scope = None` (no referenced scope); only the requested worktree's `Active` scopes are loaded, with no extra terminal scope. @@ -156,6 +156,8 @@ Persist this field in every plan; this is durable plan state, not chat state: - `scope = Some(S)`, `event_key = Some(K)`, `S == K.scope_id` -> `effective_scope = Some(S)`; that single `ScopeId` is loaded and validated once. - `scope = Some(S)`, `event_key = Some(K)`, `S != K.scope_id` -> `load_worktree` returns `Err` before loading either scope and before performing the processed-event lookup. It never chooses one arbitrarily, never loads both scopes, never ignores the mismatch, and never performs the replay query anyway. + **Existence invariant (T03 correction):** whenever an `effective_scope` is `Some(S)` (whether `S` came from `scope` or from `event_key.scope_id`), a durable `mutation_trace_scopes` row for `S` must exist, or `load_worktree` returns `Err`. This is checked before the `processed_events` replay lookup, so a `mutation_trace_processed_events` row alone (an orphan replay row with no owning scope) can never cause `processed_events` to gain an entry without its `ScopeState` also being present in the projection. A missing effective scope is never silently omitted — `load_worktree` never returns `Ok(Some(projection))` with a projection that excludes an effective scope it was asked about. When the effective scope is already present in the `Active`-scope query result (already known to belong to the requested worktree, since that query is filtered by it), no second scope query is issued. + `load_worktree` then loads exactly one worktree row, only its currently `Active` scopes plus the scope named by `effective_scope` (even when that scope is `NeverSeen`/`Closed`/`Abandoned`), and — when `event_key` is supplied and no `S != K.scope_id` mismatch already returned `Err` — 0 or 1 matching processed-event row for `event_key`. The processed-event lookup is keyed solely by `event_key`'s `(scope_id, event_id)` — `WHERE scope_id = ? AND event_id = ?`, never filtered or joined by `worktree_id` — since `mutation_trace_processed_events` carries no `worktree_id` column (removed from migration `003`; the table's only identity is `PRIMARY KEY (scope_id, event_id)`, matching the domain `EventKey`). The worktree relationship for `event_key`'s scope is established by loading and validating its durable `ScopeState` as part of `effective_scope` above, not by a `worktree_id` column on the processed-event table: `EventKey.scope_id` -> `mutation_trace_scopes.scope_id` -> `mutation_trace_scopes.worktree_id`. A separate cold-path `load_mutation_event(worktree: &WorktreeId, revision: u64) -> Result>` reads one `mutation_trace_events` row plus its `mutation_trace_event_active_scopes` rows and reconstructs a complete `MutationEvent`, decoding `Attribution` exactly (including `AiExclusive(scope_id)`) and the complete `Boundary`. When `effective_scope` is `Some(S)` and the persisted `ScopeState` for `S` has a `worktree_id` different from the requested `worktree`, `load_worktree` returns `Err` — it never silently omits the scope, never includes it in the projection, and never reassigns it to the requested worktree, preserving the permanent `ScopeId` -> `WorktreeId` identity `register_scope` already enforces. This is the same check whether `S` came from the explicit `scope` argument or from `event_key.scope_id`. Out — initialization/commit logic (T04/T07); calling `load_mutation_event` from `load_worktree` or from any hook-boundary path. - Dependencies: T01, T02 - Done when: `load_worktree` returns `None` for a missing worktree and `Some(projection)` otherwise, with `scopes` containing every currently `Active` scope on that worktree plus the `effective_scope` (derived from `scope`/`event_key` per the four cases above) when one exists, regardless of its status, and never a `Closed`/`Abandoned`/`NeverSeen` scope that was not the effective referenced scope; `attempts`, `mutation_events`, and `external_taint` stay empty; the method issues no query against `mutation_trace_events`. Explicit test cases for all five `scope`/`event_key` combinations: @@ -164,10 +166,20 @@ Persist this field in every plan; this is durable plan state, not chat state: 3. `scope=None`, `event_key=Some(K)` -> `K.scope_id` is loaded as the effective referenced scope; a wrong-worktree `K.scope_id` returns `Err`; the processed-event lookup for `K` still matches solely on `(scope_id, event_id)`. 4. `scope=Some(S)`, `event_key=Some(K)`, `S == K.scope_id` -> succeeds, loading and validating that one `ScopeId` exactly once. 5. `scope=Some(S)`, `event_key=Some(K)`, `S != K.scope_id` -> `load_worktree` returns `Err` without loading either scope and without performing the processed-event lookup. + 6. (T03 correction) An effective referenced scope that has no durable `mutation_trace_scopes` row returns `Err`, whether it came from `scope` (explicit missing scope) or from `event_key.scope_id` (event-key-only missing scope) — including when a `mutation_trace_processed_events` row already exists for that `(scope_id, event_id)` (orphan replay row): the replay row alone must never be enough to construct a valid projection. - Also preserved: a referenced scope on the requested worktree is included regardless of status; a referenced terminal (`Closed`/`Abandoned`/`NeverSeen`) scope on the requested worktree is included; an unreferenced terminal historical scope is excluded; the processed-event query never references `worktree_id` (it has no such column) and matches solely on `scope_id`/`event_id`; `load_worktree` never queries historical `mutation_trace_events` rows. `load_mutation_event` returns `None` when no row exists at that `(worktree, revision)` and otherwise reconstructs a `MutationEvent` whose `before_tree`/`after_tree`/`revision`/`tainted`/`failure_kind`/`attribution`/`boundary`/`active_scopes` exactly match what `store.commit` persisted. + Also preserved: a referenced scope on the requested worktree is included regardless of status; a referenced terminal (`Closed`/`Abandoned`/`NeverSeen`) scope on the requested worktree is included; an unreferenced terminal historical scope is excluded; the processed-event query never references `worktree_id` (it has no such column) and matches solely on `scope_id`/`event_id`; `load_worktree` never queries historical `mutation_trace_events` rows. `load_mutation_event` returns `None` when no row exists at that `(worktree, revision)` and otherwise reconstructs a `MutationEvent` whose `before_tree`/`after_tree`/`revision`/`tainted`/`failure_kind`/`attribution`/`boundary`/`active_scopes` exactly match what `store.commit` persisted. If an effective referenced scope is present, that `ScopeId` must exist in durable scope storage; a missing effective scope returns `Err`. This rule applies whether the effective scope came from `scope` or `event_key.scope_id`. - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml mutation_trace::store::` - - Context synchronization: pending + - Completed: 2026-08-27 + - Files changed: `cli/src/services/mutation_trace/store.rs` + - Result: Added `WorktreeProjection` (`worktree_id`, `worktree_state`, `scopes`, `processed_events`) and its `into_protocol_state` (widens into a full `ProtocolState`, always with an empty `attempts`/`mutation_events`/`external_taint`), plus `MutationTraceStore<'a>` wrapping `&'a RepositoryAgentTraceDb`. `load_worktree` derives one `effective_referenced_scope` from `scope`/`event_key` per the plan's four-case definition (returning `Err` before any query on a `Some(S) != Some(K.scope_id)` mismatch), returns `None` for a missing worktree row, otherwise loads the worktree's currently `Active` scopes (`SELECT ... WHERE worktree_id = ?1 AND status = 'active'`) plus the effective scope by `scope_id` alone (any status), erroring if that scope's persisted `worktree_id` disagrees with the requested worktree, and populates `processed_events` with 0 or 1 entries via a `(scope_id, event_id)`-only lookup — no query anywhere in `load_worktree` references `mutation_trace_events`. `load_mutation_event(worktree, revision)` reads one `mutation_trace_events` row plus its `mutation_trace_event_active_scopes` rows and reconstructs a full `MutationEvent`, decoding `Attribution`/`Boundary` via new `reconstruct_attribution`/`reconstruct_boundary` helpers built on T02's `AttributionKind`/`BoundaryKind` codecs (rejecting inconsistent kind/payload combinations). Added 14 new tests covering: missing worktree, all five `scope`/`event_key` combination cases (including both wrong-worktree error cases and the disagreement case), `load_mutation_event`'s missing-row case, two full reconstruction round trips (`ai_exclusive`/`start` with one active scope, `ai_contended`/`flush` with two active scopes), and one test asserting `into_protocol_state`'s single-worktree/empty-transient-fields shape. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml mutation_trace::store::` — passed, 24/24; `./scripts/run-cli-cargo.sh fmt --manifest-path cli/Cargo.toml -- --check` — passed after one auto-formatting pass; `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` — passed, zero warnings. + - Done checks: `load_worktree` returns `None` for a missing worktree and `Some(projection)` otherwise (verified); all five `scope`/`event_key` cases behave exactly as specified, including both wrong-worktree `Err` cases and the disagreement `Err` case (verified by the five case-specific tests); `attempts`/`mutation_events`/`external_taint` stay empty in every projection and in `into_protocol_state`'s output (verified); no query in `load_worktree` references `mutation_trace_events` (verified by inspection — the function's SQL constants are `SELECT_WORKTREE_SQL`/`SELECT_SCOPES_BY_WORKTREE_AND_STATUS_SQL`/`SELECT_SCOPE_BY_ID_SQL`/`SELECT_PROCESSED_EVENT_SQL` only); `load_mutation_event` returns `None` when absent and otherwise reconstructs an exact `MutationEvent`, including full `Attribution`/`Boundary` decoding and `active_scopes` (verified by the two reconstruction tests). + - Context impact: local — new query/projection logic confined to `store.rs`, still not called from any hook, command, or `coordinator.rs`/`git_snapshot.rs` seam; `context/cli/mutation-trace-protocol.md`'s "not yet wired into any hook, command, or database call site" framing remains accurate (a real call site lands in T07). No durable context file describes this yet; deferred to T11/plan-level context sync once the full store lands, per the plan's assumption. + - Context synchronization: synced + - **T03 correction (2026-08-27):** Fixed a persistence-boundary gap where a missing effective referenced scope (`load_scope` returning `None`) fell through `if let Some(scope_state) = self.load_scope(...)?` and silently continued, producing a projection that omitted the effective scope instead of erroring. Changed to `self.load_scope(effective_scope_id)?.ok_or_else(...)?`, so a missing durable `mutation_trace_scopes` row for the effective scope now returns `Err` before the wrong-worktree check and before the `processed_events` replay lookup — applying equally whether the effective scope came from `scope` or from `event_key.scope_id`, and preventing an orphan `mutation_trace_processed_events` row from ever entering `ProtocolState.processed_events` without its owning `ScopeState`. The Active-scope fast path (`if !scopes.contains_key(effective_scope_id)`) is unchanged, so an effective scope already present from the bounded `Active` query still skips the second lookup. Added three tests: `load_worktree_with_explicit_missing_scope_errors` (`scope=Some("scope-missing")`, no row, requested worktree exists), `load_worktree_with_event_key_missing_scope_errors` (`event_key.scope_id="scope-missing"`, no row), and `load_worktree_with_event_key_missing_scope_and_orphan_replay_row_errors` (same as the previous case, plus a pre-existing `mutation_trace_processed_events` row for `("scope-missing", "event-1")`, proving the orphan replay row cannot substitute for the missing `ScopeState`). All existing T03 tests preserved and passing. + - Verify (T03 correction): `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml mutation_trace::store::` — passed, 27/27; `./scripts/run-cli-cargo.sh fmt --manifest-path cli/Cargo.toml -- --check` — passed, no diff; `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` — passed, zero warnings. + - Done checks (T03 correction): explicit missing scope (`scope=Some(S)`, no durable row) returns `Err` (verified); event-key-only missing scope (`scope=None`, `event_key.scope_id=K`, no durable row) returns `Err` (verified); an orphan `mutation_trace_processed_events` row for a missing scope still returns `Err` and never populates `processed_events` (verified); the Active-scope fast path issues no redundant scope query when the effective scope is already loaded (verified by inspection — unchanged `if !scopes.contains_key(...)` guard); T04 was not started (verified — no changes to `initialize_worktree`/`register_scope`, migration 003, `EventKey`, or any file outside `store.rs`/this plan). - [ ] T04: `Add worktree/scope initialization operations` (status:todo) - Task ID: T04