diff --git a/cli/src/services/mutation_trace/runtime/coordinator.rs b/cli/src/services/mutation_trace/runtime/coordinator.rs index 4df950cc..e5f2a960 100644 --- a/cli/src/services/mutation_trace/runtime/coordinator.rs +++ b/cli/src/services/mutation_trace/runtime/coordinator.rs @@ -1,24 +1,21 @@ use std::path::Path; -use std::time::Duration; use anyhow::Result; use uuid::Uuid; use crate::services::agent_trace_db::repository::RepositoryAgentTraceDb; -use crate::services::checkout::{get_or_create_checkout_id, resolve_git_dir}; use crate::services::mutation_trace::protocol; use crate::services::mutation_trace::store::{CasResult, DurableTransition, MutationTraceStore}; use crate::services::mutation_trace::types::{ self, ActorKind, AttemptId, Boundary, EventId, MutationEvent, ScopeId, TreeId, WorktreeId, }; -use super::external_taint::ExternalTaintMarker; use super::git_snapshot::GitSnapshotService; -use super::worktree_lock::{acquire_inner, WorktreeLockError}; +use super::protected_worktree::{ProtectedWorktree, ProtectedWorktreeError}; -const MAX_CAS_RETRY_ATTEMPTS: u32 = 5; +pub use super::protected_worktree::ExternalTaintOperation; -const WORKTREE_LOCK_TIMEOUT: Duration = Duration::from_secs(10); +pub(super) const MAX_CAS_RETRY_ATTEMPTS: u32 = 5; #[derive(Clone, Debug)] pub enum RuntimeBoundary { @@ -49,18 +46,6 @@ pub struct CoordinateOutcome { pub mutation_event: Option, } -/// Which pre-commit [`ExternalTaintMarker`] operation failed while coordinating a -/// boundary. Both happen **before** any protected work, so no -/// [`CoordinateOutcome`] exists yet. A marker-clear failure happens *after* a -/// durable commit and is reported through -/// [`CoordinateError::MarkerClearAfterCommit`] instead, which carries the -/// committed outcome. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum ExternalTaintOperation { - Inspect, - Persist, -} - #[derive(Debug)] pub enum CoordinateError { SnapshotFailure { @@ -161,16 +146,6 @@ impl SnapshotCapture for GitSnapshotService { } } -/// Coordinates one mutation-cursor runtime boundary end to end. -/// -/// The entrypoint owns the whole protected operation: it resolves `git_dir`, -/// acquires the [`WorktreeLock`](super::worktree_lock::WorktreeLock), arms the -/// worktree-local [`ExternalTaintMarker`] write-ahead — **before** acquiring the -/// Agent Trace DB — and only then invokes the caller-supplied `open_db` -/// provider, captures a snapshot, runs the snapshot / recovery / protocol / CAS -/// pipeline, and clears the marker on complete success. Any failure after the -/// marker is armed — including `open_db` returning `Err` — leaves the marker in -/// place for the next invocation. pub fn coordinate

( repository_root: &Path, boundary: &RuntimeBoundary, @@ -203,37 +178,20 @@ where L: FnMut(u32), R: FnMut(u32) -> Result<()>, { - let git_dir = resolve_git_dir(repository_root).map_err(CoordinateError::Other)?; - - let _lock = acquire_inner(&git_dir, WORKTREE_LOCK_TIMEOUT, on_lock_contention) - .map_err(lock_acquisition)?; - - let marker = ExternalTaintMarker::new(&git_dir); - let inherited_external_taint = - marker - .exists() - .map_err(|source| CoordinateError::ExternalTaintMarker { - operation: ExternalTaintOperation::Inspect, - source, - })?; - marker - .persist() - .map_err(|source| CoordinateError::ExternalTaintMarker { - operation: ExternalTaintOperation::Persist, - source, - })?; + let protected = ProtectedWorktree::acquire_inner(repository_root, on_lock_contention) + .map_err(protected_worktree_failure)?; let outcome = coordinate_protected( repository_root, - &git_dir, + protected.worktree_id(), boundary, open_db, - inherited_external_taint, + protected.inherited_external_taint(), after_load, after_recovery, )?; - match marker.clear() { + match protected.complete() { Ok(()) => Ok(outcome), Err(source) => Err(CoordinateError::MarkerClearAfterCommit { source, @@ -244,7 +202,7 @@ where fn coordinate_protected( repository_root: &Path, - git_dir: &Path, + worktree_id: &WorktreeId, boundary: &RuntimeBoundary, open_db: P, inherited_external_taint: bool, @@ -256,9 +214,6 @@ where L: FnMut(u32), R: FnMut(u32) -> Result<()>, { - let checkout_id = get_or_create_checkout_id(git_dir).map_err(CoordinateError::Other)?; - let worktree_id = WorktreeId(checkout_id); - let db = open_db().map_err(CoordinateError::AgentTraceDbUnavailable)?; let snapshot = GitSnapshotService::new(repository_root).map_err(CoordinateError::Other)?; @@ -266,7 +221,7 @@ where coordinate_boundary_inner( &db, &snapshot, - &worktree_id, + worktree_id, boundary, inherited_external_taint, after_load, @@ -274,8 +229,17 @@ where ) } -fn lock_acquisition(error: WorktreeLockError) -> CoordinateError { - CoordinateError::LockAcquisition(anyhow::Error::new(error)) +fn protected_worktree_failure(error: ProtectedWorktreeError) -> CoordinateError { + match error { + ProtectedWorktreeError::GitDirResolution(source) + | ProtectedWorktreeError::CheckoutIdentity(source) => CoordinateError::Other(source), + ProtectedWorktreeError::LockAcquisition(source) => { + CoordinateError::LockAcquisition(anyhow::Error::new(source)) + } + ProtectedWorktreeError::ExternalTaintMarker { operation, source } => { + CoordinateError::ExternalTaintMarker { operation, source } + } + } } #[cfg(test)] @@ -532,8 +496,12 @@ mod tests { use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::mpsc; use std::thread; + use std::time::Duration; use super::*; + use crate::services::checkout::{get_or_create_checkout_id, resolve_git_dir}; + use crate::services::mutation_trace::runtime::external_taint::ExternalTaintMarker; + use crate::services::mutation_trace::runtime::worktree_lock::acquire_inner; use crate::services::mutation_trace::store::encode_revision; use crate::services::mutation_trace::types::{Attribution, EventKey, FailureKind, ScopeStatus}; diff --git a/cli/src/services/mutation_trace/runtime/mod.rs b/cli/src/services/mutation_trace/runtime/mod.rs index 2d88705c..fc41f5e2 100644 --- a/cli/src/services/mutation_trace/runtime/mod.rs +++ b/cli/src/services/mutation_trace/runtime/mod.rs @@ -1,8 +1,19 @@ mod coordinator; mod external_taint; mod git_snapshot; +mod protected_worktree; mod ref_reconciliation; +mod scope_runtime; mod worktree_lock; #[cfg(test)] mod tests; + +#[allow(unused_imports)] +pub(crate) use coordinator::{ + coordinate, CoordinateError, CoordinateOutcome, ExternalTaintOperation, RuntimeBoundary, +}; +#[allow(unused_imports)] +pub(crate) use scope_runtime::{ + abandon_scope, AbandonRecoveryReason, AbandonScopeError, AbandonScopeOutcome, +}; diff --git a/cli/src/services/mutation_trace/runtime/protected_worktree.rs b/cli/src/services/mutation_trace/runtime/protected_worktree.rs new file mode 100644 index 00000000..2d462fc3 --- /dev/null +++ b/cli/src/services/mutation_trace/runtime/protected_worktree.rs @@ -0,0 +1,314 @@ +use std::path::Path; +use std::time::Duration; + +use crate::services::checkout::{get_or_create_checkout_id, resolve_git_dir}; +use crate::services::mutation_trace::types::WorktreeId; + +use super::external_taint::ExternalTaintMarker; +use super::worktree_lock::{acquire_inner, WorktreeLock, WorktreeLockError}; + +pub const WORKTREE_LOCK_TIMEOUT: Duration = Duration::from_secs(10); + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ExternalTaintOperation { + Inspect, + Persist, +} + +#[derive(Debug)] +pub enum ProtectedWorktreeError { + GitDirResolution(anyhow::Error), + LockAcquisition(WorktreeLockError), + ExternalTaintMarker { + operation: ExternalTaintOperation, + source: anyhow::Error, + }, + CheckoutIdentity(anyhow::Error), +} + +impl std::fmt::Display for ProtectedWorktreeError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ProtectedWorktreeError::ExternalTaintMarker { operation, source } => write!( + f, + "External-taint marker {operation:?} operation failed before any \ + protected runtime work began: {source}" + ), + ProtectedWorktreeError::LockAcquisition(source) => write!(f, "{source}"), + ProtectedWorktreeError::GitDirResolution(source) + | ProtectedWorktreeError::CheckoutIdentity(source) => write!(f, "{source}"), + } + } +} + +impl std::error::Error for ProtectedWorktreeError {} + +#[derive(Debug)] +pub struct ProtectedWorktree { + marker: ExternalTaintMarker, + inherited_external_taint: bool, + worktree_id: WorktreeId, + _lock: WorktreeLock, +} + +impl ProtectedWorktree { + pub fn acquire(repository_root: &Path) -> Result { + Self::acquire_inner(repository_root, || {}) + } + + pub(super) fn acquire_inner( + repository_root: &Path, + on_lock_contention: F, + ) -> Result + where + F: FnOnce(), + { + Self::acquire_with_timeout(repository_root, WORKTREE_LOCK_TIMEOUT, on_lock_contention) + } + + fn acquire_with_timeout( + repository_root: &Path, + lock_timeout: Duration, + on_lock_contention: F, + ) -> Result + where + F: FnOnce(), + { + let git_dir = + resolve_git_dir(repository_root).map_err(ProtectedWorktreeError::GitDirResolution)?; + + let lock = acquire_inner(&git_dir, lock_timeout, on_lock_contention) + .map_err(ProtectedWorktreeError::LockAcquisition)?; + + let marker = ExternalTaintMarker::new(&git_dir); + let inherited_external_taint = + marker + .exists() + .map_err(|source| ProtectedWorktreeError::ExternalTaintMarker { + operation: ExternalTaintOperation::Inspect, + source, + })?; + marker + .persist() + .map_err(|source| ProtectedWorktreeError::ExternalTaintMarker { + operation: ExternalTaintOperation::Persist, + source, + })?; + + let checkout_id = get_or_create_checkout_id(&git_dir) + .map_err(ProtectedWorktreeError::CheckoutIdentity)?; + + Ok(Self { + marker, + inherited_external_taint, + worktree_id: WorktreeId(checkout_id), + _lock: lock, + }) + } + + #[must_use] + pub fn worktree_id(&self) -> &WorktreeId { + &self.worktree_id + } + + #[must_use] + pub fn inherited_external_taint(&self) -> bool { + self.inherited_external_taint + } + + pub fn complete(self) -> anyhow::Result<()> { + self.marker.clear() + } +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + use std::process::Command; + use std::sync::mpsc; + use std::thread; + + use super::*; + + struct TestRepo { + _temp_dir: tempfile::TempDir, + repo_root: PathBuf, + git_dir: PathBuf, + } + + impl TestRepo { + fn new(label: &str) -> Self { + let temp_dir = tempfile::Builder::new() + .prefix(&format!("sce-protected-worktree-{label}-")) + .tempdir() + .expect("test temp directory should be created"); + let repo_root = temp_dir.path().join("repo"); + std::fs::create_dir_all(&repo_root).expect("repository directory should be created"); + run_git(&repo_root, &["init", "--quiet"]); + let git_dir = resolve_git_dir(&repo_root).expect("git dir should resolve"); + Self { + _temp_dir: temp_dir, + repo_root, + git_dir, + } + } + + fn marker(&self) -> ExternalTaintMarker { + ExternalTaintMarker::new(&self.git_dir) + } + + fn marker_exists(&self) -> bool { + self.marker() + .exists() + .expect("marker existence should resolve") + } + } + + fn run_git(dir: &std::path::Path, args: &[&str]) { + let output = Command::new("git") + .args(args) + .current_dir(dir) + .output() + .expect("git command should spawn"); + assert!( + output.status.success(), + "git {args:?} failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + + #[test] + fn acquire_arms_a_fresh_marker_and_reports_no_inherited_taint() { + let repo = TestRepo::new("fresh-marker"); + + let guard = ProtectedWorktree::acquire(&repo.repo_root) + .expect("the prefix should be establishable on a clean worktree"); + + assert!( + !guard.inherited_external_taint(), + "a worktree with no marker on entry must not report inherited taint" + ); + assert!( + !guard.worktree_id().0.is_empty(), + "the guard must expose the derived WorktreeId" + ); + assert!( + repo.marker_exists(), + "acquire must arm the external-taint marker write-ahead" + ); + + guard + .complete() + .expect("completion should clear the marker"); + assert!( + !repo.marker_exists(), + "an explicit completion must clear the marker it armed" + ); + } + + #[test] + fn acquire_reports_a_marker_inherited_from_an_earlier_invocation() { + let repo = TestRepo::new("inherited-marker"); + repo.marker() + .persist() + .expect("the earlier invocation's marker should arm"); + + let guard = ProtectedWorktree::acquire(&repo.repo_root) + .expect("an inherited marker must not fail the prefix"); + + assert!( + guard.inherited_external_taint(), + "a marker present on entry must be reported as inherited taint" + ); + assert!( + repo.marker_exists(), + "the inherited marker must stay armed for the protected operation" + ); + } + + #[test] + fn a_dropped_guard_releases_the_lock_but_leaves_the_marker_armed() { + let repo = TestRepo::new("dropped-guard"); + + let guard = ProtectedWorktree::acquire(&repo.repo_root) + .expect("the prefix should be establishable on a clean worktree"); + drop(guard); + + assert!( + repo.marker_exists(), + "dropping a guard that never completed must leave the fence armed" + ); + + ProtectedWorktree::acquire(&repo.repo_root) + .expect("a dropped guard must have released the worktree lock"); + } + + #[test] + fn acquire_fails_with_lock_acquisition_while_the_lock_is_still_held() { + let repo = TestRepo::new("lock-timeout"); + + let held = acquire_inner(&repo.git_dir, Duration::from_secs(5), || {}) + .expect("the test should hold the worktree lock before the guard runs"); + + let error = ProtectedWorktree::acquire_with_timeout( + &repo.repo_root, + Duration::from_millis(250), + || {}, + ) + .expect_err("the prefix must not be establishable while the lock is held"); + assert!( + matches!( + error, + ProtectedWorktreeError::LockAcquisition(WorktreeLockError::TimedOut { .. }) + ), + "expected a LockAcquisition timeout, got {error:?}" + ); + assert!( + !repo.marker_exists(), + "a prefix that never acquired the lock must not have armed the fence" + ); + + drop(held); + } + + #[test] + fn the_lock_is_held_for_the_whole_guard_lifetime() { + let repo = TestRepo::new("lock-lifetime"); + + let guard = ProtectedWorktree::acquire(&repo.repo_root) + .expect("the prefix should be establishable on a clean worktree"); + + let (contention_tx, contention_rx) = mpsc::channel(); + let (result_tx, result_rx) = mpsc::channel(); + let repo_root = repo.repo_root.clone(); + let worker = thread::spawn(move || { + let result = ProtectedWorktree::acquire_inner(&repo_root, move || { + contention_tx + .send(()) + .expect("contention signal channel should still be open"); + }); + result_tx + .send(()) + .expect("result signal channel should still be open"); + result + }); + + contention_rx + .recv_timeout(Duration::from_secs(5)) + .expect("a second acquirer should observe the held worktree lock"); + assert!( + result_rx.recv_timeout(Duration::from_millis(300)).is_err(), + "a second acquirer must not establish the prefix while the guard is alive" + ); + + drop(guard); + + result_rx + .recv_timeout(Duration::from_secs(5)) + .expect("the second acquirer should proceed once the guard is dropped"); + worker + .join() + .expect("the second acquirer thread should not panic") + .expect("the second acquirer should succeed once the lock is released"); + } +} diff --git a/cli/src/services/mutation_trace/runtime/ref_reconciliation.rs b/cli/src/services/mutation_trace/runtime/ref_reconciliation.rs index fc10094f..5b93462a 100644 --- a/cli/src/services/mutation_trace/runtime/ref_reconciliation.rs +++ b/cli/src/services/mutation_trace/runtime/ref_reconciliation.rs @@ -12,11 +12,6 @@ use crate::services::mutation_trace::types::{TreeId, WorktreeId}; use super::git_snapshot::{GitSnapshotService, PinInventoryError, PinnedRef}; use super::worktree_lock::{acquire_inner, WorktreeLockError}; -/// Bounded wait for the worktree's `WorktreeLock` before a reconciliation pass -/// gives up. Its value intentionally matches the coordinator's private -/// `WORKTREE_LOCK_TIMEOUT` but is deliberately **not** a shared abstraction: -/// there is no semantic reason the two timeouts must always stay identical, so -/// each module owns its own constant. const RECONCILIATION_LOCK_TIMEOUT: Duration = Duration::from_secs(10); /// Outcome counts of one successful reconciliation pass. diff --git a/cli/src/services/mutation_trace/runtime/scope_runtime.rs b/cli/src/services/mutation_trace/runtime/scope_runtime.rs new file mode 100644 index 00000000..63d6d3fe --- /dev/null +++ b/cli/src/services/mutation_trace/runtime/scope_runtime.rs @@ -0,0 +1,1109 @@ +use std::path::Path; + +use crate::services::agent_trace_db::repository::RepositoryAgentTraceDb; +use crate::services::mutation_trace::protocol; +use crate::services::mutation_trace::store::{CasResult, DurableTransition, MutationTraceStore}; +use crate::services::mutation_trace::types::{ScopeId, ScopeStatus, WorktreeId}; + +use super::coordinator::MAX_CAS_RETRY_ATTEMPTS; +use super::protected_worktree::{ + ExternalTaintOperation, ProtectedWorktree, ProtectedWorktreeError, +}; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum AbandonRecoveryReason { + InheritedExternalTaint, + MissingScope, + NeverSeenScope, + MissingWorktreeState, +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum AbandonScopeOutcome { + Abandoned { + worktree_id: WorktreeId, + scope: ScopeId, + revision: u64, + }, + AlreadyTerminal { + worktree_id: WorktreeId, + scope: ScopeId, + status: ScopeStatus, + revision: u64, + }, + RecoveryRequired { + worktree_id: WorktreeId, + scope: ScopeId, + reason: AbandonRecoveryReason, + }, +} + +#[derive(Debug)] +pub enum AbandonScopeError { + LockAcquisition(anyhow::Error), + ExternalTaintMarker { + operation: ExternalTaintOperation, + source: anyhow::Error, + }, + AgentTraceDbUnavailable(anyhow::Error), + WorktreeIdentityMismatch { + scope: ScopeId, + scope_worktree_id: WorktreeId, + invoking_worktree_id: WorktreeId, + }, + RevisionExhausted { + worktree_id: WorktreeId, + revision: u64, + }, + CasConflictExhausted { + attempts: u32, + }, + MarkerClearAfterCompletion { + source: anyhow::Error, + completed: Box, + }, + Other(anyhow::Error), +} + +impl std::fmt::Display for AbandonScopeError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + AbandonScopeError::ExternalTaintMarker { operation, source } => write!( + f, + "External-taint marker {operation:?} operation failed before any \ + mutation-scope state was read: {source}" + ), + AbandonScopeError::AgentTraceDbUnavailable(source) => { + write!(f, "Repository Agent Trace DB is unavailable: {source}") + } + AbandonScopeError::WorktreeIdentityMismatch { + scope, + scope_worktree_id, + invoking_worktree_id, + } => write!( + f, + "Scope {scope:?} belongs to worktree {scope_worktree_id:?} and cannot \ + be abandoned through worktree {invoking_worktree_id:?}" + ), + AbandonScopeError::RevisionExhausted { + worktree_id, + revision, + } => write!( + f, + "Scope on worktree {worktree_id:?} cannot be abandoned because its \ + revision ({revision}) cannot be advanced" + ), + AbandonScopeError::CasConflictExhausted { attempts } => { + write!(f, "Exhausted {attempts} CAS-conflict retry attempts") + } + AbandonScopeError::MarkerClearAfterCompletion { source, .. } => write!( + f, + "Mutation scope settled durably, but clearing the external-taint \ + marker failed: {source}" + ), + AbandonScopeError::LockAcquisition(source) | AbandonScopeError::Other(source) => { + write!(f, "{source}") + } + } + } +} + +impl std::error::Error for AbandonScopeError {} + +pub fn abandon_scope

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