diff --git a/cli/src/services/hooks/codex_mutation_scope/boundary_lock.rs b/cli/src/services/hooks/codex_mutation_scope/boundary_lock.rs deleted file mode 100644 index eb41286d3..000000000 --- a/cli/src/services/hooks/codex_mutation_scope/boundary_lock.rs +++ /dev/null @@ -1,195 +0,0 @@ -use std::path::{Path, PathBuf}; -use std::time::Duration; - -use super::os_lock::{AdvisoryLockError, OsAdvisoryLock}; -use super::state::adapter_state_dir; - -const ADAPTER_BOUNDARY_LOCK_FILE: &str = "codex-mutation-scope-boundary.lock"; -const BOUNDARY_LOCK_WHAT: &str = "adapter-boundary"; - -pub(crate) const DEFAULT_BOUNDARY_LOCK_TIMEOUT: Duration = Duration::from_secs(10); - -pub(crate) fn boundary_lock_path(git_dir: &Path) -> PathBuf { - adapter_state_dir(git_dir).join(ADAPTER_BOUNDARY_LOCK_FILE) -} - -pub(crate) struct AdapterBoundaryLock { - _inner: OsAdvisoryLock, -} - -impl AdapterBoundaryLock { - pub(crate) fn acquire( - git_dir: &Path, - timeout: Duration, - ) -> Result { - let inner = OsAdvisoryLock::acquire( - &adapter_state_dir(git_dir), - boundary_lock_path(git_dir), - timeout, - BOUNDARY_LOCK_WHAT, - )?; - Ok(AdapterBoundaryLock { _inner: inner }) - } -} - -#[cfg(test)] -mod tests { - use std::io::Write as _; - use std::process::Command; - use std::sync::atomic::{AtomicU64, Ordering}; - use std::sync::mpsc; - use std::thread; - - use super::*; - - static NEXT_ID: AtomicU64 = AtomicU64::new(0); - - fn unique_git_dir(label: &str) -> PathBuf { - let id = NEXT_ID.fetch_add(1, Ordering::Relaxed); - std::env::temp_dir().join(format!( - "sce-codex-mutation-scope-boundary-{label}-{}-{id}", - std::process::id() - )) - } - - const CHILD_ENV_GIT_DIR: &str = "SCE_BOUNDARY_LOCK_CHILD_GIT_DIR"; - const CHILD_ENV_TIMEOUT_MS: &str = "SCE_BOUNDARY_LOCK_CHILD_TIMEOUT_MS"; - const CHILD_HELPER_PATH: &str = - "services::hooks::codex_mutation_scope::boundary_lock::tests::boundary_lock_child_helper"; - - #[test] - #[ignore = "subprocess helper, driven by process_death_releases_the_boundary_lock"] - fn boundary_lock_child_helper() { - let git_dir = PathBuf::from( - std::env::var(CHILD_ENV_GIT_DIR).expect("child helper needs a git dir in the env"), - ); - let timeout = Duration::from_millis( - std::env::var(CHILD_ENV_TIMEOUT_MS) - .expect("child helper needs a timeout in the env") - .parse() - .expect("timeout must parse"), - ); - match AdapterBoundaryLock::acquire(&git_dir, timeout) { - Ok(lock) => { - print!("ACQUIRED"); - std::io::stdout().flush().expect("flush stdout"); - std::mem::forget(lock); - } - Err(AdvisoryLockError::TimedOut { .. }) => { - print!("TIMEOUT"); - std::io::stdout().flush().expect("flush stdout"); - } - Err(other) => panic!("unexpected child lock error: {other}"), - } - } - - fn run_child(git_dir: &Path, timeout: Duration) -> String { - let exe = std::env::current_exe().expect("test executable path should resolve"); - let output = Command::new(exe) - .args(["--exact", "--ignored", "--nocapture", CHILD_HELPER_PATH]) - .env(CHILD_ENV_GIT_DIR, git_dir) - .env(CHILD_ENV_TIMEOUT_MS, timeout.as_millis().to_string()) - .output() - .expect("the child test process should spawn"); - assert!( - output.status.success(), - "child process failed: stdout={:?} stderr={:?}", - String::from_utf8_lossy(&output.stdout), - String::from_utf8_lossy(&output.stderr), - ); - let stdout = String::from_utf8_lossy(&output.stdout); - if stdout.contains("ACQUIRED") { - "ACQUIRED".to_string() - } else if stdout.contains("TIMEOUT") { - "TIMEOUT".to_string() - } else { - panic!("child produced no lock verdict: {stdout}"); - } - } - - #[test] - fn process_death_releases_the_boundary_lock() { - let git_dir = unique_git_dir("process-death"); - std::fs::create_dir_all(&git_dir).expect("git dir should be created"); - - let held = AdapterBoundaryLock::acquire(&git_dir, Duration::from_secs(5)) - .expect("the parent should acquire the boundary lock immediately"); - - assert_eq!( - run_child(&git_dir, Duration::from_millis(300)), - "TIMEOUT", - "a second OS process must not acquire the boundary lock while the parent holds it", - ); - - drop(held); - - assert_eq!( - run_child(&git_dir, Duration::from_millis(300)), - "ACQUIRED", - "once the parent releases, a child process acquires it", - ); - - let reacquired = AdapterBoundaryLock::acquire(&git_dir, Duration::from_secs(2)) - .expect("the child's process exit must have released the boundary lock"); - drop(reacquired); - - let _ = std::fs::remove_dir_all(&git_dir); - } - - #[test] - fn a_leftover_boundary_lock_file_alone_does_not_block_acquisition() { - let git_dir = unique_git_dir("leftover-file"); - std::fs::create_dir_all(adapter_state_dir(&git_dir)).expect("state dir should be created"); - std::fs::write(boundary_lock_path(&git_dir), b"leftover") - .expect("leftover lock file should be writable"); - - AdapterBoundaryLock::acquire(&git_dir, Duration::from_millis(200)) - .expect("a lock file with no live OS owner must not block a new acquirer"); - - let _ = std::fs::remove_dir_all(&git_dir); - } - - #[test] - fn a_second_in_process_acquirer_blocks_until_the_first_releases() { - let git_dir = unique_git_dir("in-process-contention"); - std::fs::create_dir_all(&git_dir).expect("git dir should be created"); - - let holder = AdapterBoundaryLock::acquire(&git_dir, Duration::from_secs(5)) - .expect("first acquirer should succeed immediately"); - - let (tx, rx) = mpsc::channel(); - let git_dir_clone = git_dir.clone(); - let handle = thread::spawn(move || { - let result = AdapterBoundaryLock::acquire(&git_dir_clone, Duration::from_secs(5)); - let _ = tx.send(()); - result.is_ok() - }); - - assert!( - rx.recv_timeout(Duration::from_millis(300)).is_err(), - "the second acquirer must not proceed while the first holds the boundary lock", - ); - - drop(holder); - - rx.recv_timeout(Duration::from_secs(5)) - .expect("the second acquirer should complete once the first releases"); - assert!(handle - .join() - .expect("second acquirer thread should not panic")); - - let _ = std::fs::remove_dir_all(&git_dir); - } - - #[test] - fn the_boundary_lock_path_is_distinct_from_the_state_lock_and_lives_under_sce() { - let git_dir = unique_git_dir("path-shape"); - let path = boundary_lock_path(&git_dir); - assert!(path.starts_with(adapter_state_dir(&git_dir))); - assert!(path.ends_with(ADAPTER_BOUNDARY_LOCK_FILE)); - assert_ne!( - path.file_name(), - Path::new("codex-mutation-scope-state.lock").file_name(), - ); - } -} diff --git a/cli/src/services/hooks/codex_mutation_scope/lifecycle.rs b/cli/src/services/hooks/codex_mutation_scope/lifecycle.rs index 5eb417bd1..abef09050 100644 --- a/cli/src/services/hooks/codex_mutation_scope/lifecycle.rs +++ b/cli/src/services/hooks/codex_mutation_scope/lifecycle.rs @@ -1,6 +1,6 @@ use std::path::{Path, PathBuf}; -use anyhow::{anyhow, Result}; +use anyhow::Result; use crate::services::hooks; use crate::services::hooks::codex::bash_policy::{ @@ -12,7 +12,6 @@ use crate::services::hooks::{ use crate::services::mutation_trace::runtime::resolve_git_dir; use crate::services::observability::traits::Logger; -use super::boundary_lock::{AdapterBoundaryLock, DEFAULT_BOUNDARY_LOCK_TIMEOUT}; use super::events::CODEX_TRACKED_TOOL_BASH; use super::state; use super::{ @@ -250,8 +249,7 @@ pub(super) fn with_boundary_lock( git_dir: &Path, operation: impl FnOnce() -> Result, ) -> Result { - let _boundary = AdapterBoundaryLock::acquire(git_dir, DEFAULT_BOUNDARY_LOCK_TIMEOUT) - .map_err(|error| anyhow!("Failed to acquire adapter boundary lock: {error}"))?; + let _boundary = state::BOUNDARY_LOCK.acquire(&state::adapter_state_dir(git_dir))?; operation() } diff --git a/cli/src/services/hooks/codex_mutation_scope/mod.rs b/cli/src/services/hooks/codex_mutation_scope/mod.rs index fc904317c..365535907 100644 --- a/cli/src/services/hooks/codex_mutation_scope/mod.rs +++ b/cli/src/services/hooks/codex_mutation_scope/mod.rs @@ -1,10 +1,8 @@ #![allow(dead_code)] -mod boundary_lock; mod events; pub(crate) mod health; mod lifecycle; -mod os_lock; mod payload; pub(crate) mod state; diff --git a/cli/src/services/hooks/codex_mutation_scope/os_lock.rs b/cli/src/services/hooks/codex_mutation_scope/os_lock.rs deleted file mode 100644 index dc567c1b7..000000000 --- a/cli/src/services/hooks/codex_mutation_scope/os_lock.rs +++ /dev/null @@ -1,95 +0,0 @@ -use std::fs::{File, OpenOptions, TryLockError}; -use std::path::{Path, PathBuf}; -use std::time::{Duration, Instant}; - -use anyhow::Context; - -const LOCK_POLL_INTERVAL: Duration = Duration::from_millis(20); - -#[derive(Debug)] -pub(crate) enum AdvisoryLockError { - TimedOut { - path: PathBuf, - timeout: Duration, - what: &'static str, - }, - Io(anyhow::Error), -} - -impl std::fmt::Display for AdvisoryLockError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - AdvisoryLockError::TimedOut { - path, - timeout, - what, - } => write!( - f, - "Timed out after {timeout:?} waiting for the {what} lock '{}'", - path.display() - ), - AdvisoryLockError::Io(source) => write!(f, "{source}"), - } - } -} - -impl std::error::Error for AdvisoryLockError {} - -pub(crate) struct OsAdvisoryLock { - file: File, -} - -impl OsAdvisoryLock { - pub(crate) fn acquire( - parent_dir: &Path, - lock_path: PathBuf, - timeout: Duration, - what: &'static str, - ) -> Result { - std::fs::create_dir_all(parent_dir) - .with_context(|| { - format!( - "Failed to create {what} lock directory '{}'", - parent_dir.display() - ) - }) - .map_err(AdvisoryLockError::Io)?; - - let file = OpenOptions::new() - .write(true) - .create(true) - .truncate(false) - .open(&lock_path) - .with_context(|| format!("Failed to open {what} lock file '{}'", lock_path.display())) - .map_err(AdvisoryLockError::Io)?; - - let deadline = Instant::now() + timeout; - loop { - match file.try_lock() { - Ok(()) => return Ok(OsAdvisoryLock { file }), - Err(TryLockError::WouldBlock) => { - let now = Instant::now(); - if now >= deadline { - return Err(AdvisoryLockError::TimedOut { - path: lock_path, - timeout, - what, - }); - } - std::thread::sleep(LOCK_POLL_INTERVAL.min(deadline - now)); - } - Err(TryLockError::Error(source)) => { - return Err(AdvisoryLockError::Io(anyhow::Error::new(source).context( - format!("Failed to acquire {what} lock '{}'", lock_path.display()), - ))); - } - } - } - } -} - -impl Drop for OsAdvisoryLock { - fn drop(&mut self) { - let _ = self.file.unlock(); - } -} diff --git a/cli/src/services/hooks/codex_mutation_scope/state.rs b/cli/src/services/hooks/codex_mutation_scope/state.rs index e7ab576e5..8d81a8e0a 100644 --- a/cli/src/services/hooks/codex_mutation_scope/state.rs +++ b/cli/src/services/hooks/codex_mutation_scope/state.rs @@ -1,20 +1,21 @@ use std::fs::OpenOptions; use std::io::Write as _; use std::path::{Path, PathBuf}; -use std::time::Duration; use anyhow::{anyhow, Context, Result}; use serde::{Deserialize, Serialize}; -use super::os_lock::{AdvisoryLockError, OsAdvisoryLock}; +use crate::services::hooks::mutation_scope_lock::{AdapterLockSpec, OsAdvisoryLock}; + use super::{format_codex_scope_id, AttemptKey}; const SCE_STATE_DIR: &str = "sce"; const ADAPTER_STATE_FILE: &str = "codex-mutation-scope-state.json"; -const ADAPTER_STATE_LOCK_FILE: &str = "codex-mutation-scope-state.lock"; -const STATE_LOCK_WHAT: &str = "adapter-state"; -const DEFAULT_LOCK_TIMEOUT: Duration = Duration::from_secs(10); +pub(crate) const STATE_LOCK: AdapterLockSpec = + AdapterLockSpec::state("codex-mutation-scope-state.lock"); +pub(crate) const BOUNDARY_LOCK: AdapterLockSpec = + AdapterLockSpec::boundary("codex-mutation-scope-boundary.lock"); const ADAPTER_STATE_VERSION: u32 = 3; @@ -124,26 +125,6 @@ pub(crate) fn state_path(git_dir: &Path) -> PathBuf { adapter_state_dir(git_dir).join(ADAPTER_STATE_FILE) } -fn lock_path(git_dir: &Path) -> PathBuf { - adapter_state_dir(git_dir).join(ADAPTER_STATE_LOCK_FILE) -} - -struct AdapterStateLock { - _inner: OsAdvisoryLock, -} - -impl AdapterStateLock { - fn acquire(git_dir: &Path, timeout: Duration) -> Result { - let inner = OsAdvisoryLock::acquire( - &adapter_state_dir(git_dir), - lock_path(git_dir), - timeout, - STATE_LOCK_WHAT, - )?; - Ok(AdapterStateLock { _inner: inner }) - } -} - pub(crate) fn read_state(git_dir: &Path) -> Result { let path = state_path(git_dir); if !path.exists() { @@ -240,9 +221,8 @@ where Ok(()) } -fn acquire_lock(git_dir: &Path) -> Result { - AdapterStateLock::acquire(git_dir, DEFAULT_LOCK_TIMEOUT) - .map_err(|err| anyhow!("Failed to acquire adapter-state lock: {err}")) +fn acquire_lock(git_dir: &Path) -> Result { + STATE_LOCK.acquire(&adapter_state_dir(git_dir)) } fn allocate_pending_start( @@ -456,6 +436,7 @@ pub(crate) fn arm_mark_active_failure_for_tests() { mod tests { use std::sync::atomic::{AtomicU64, Ordering}; use std::thread; + use std::time::Duration; use super::*; @@ -1294,28 +1275,6 @@ mod tests { remove_test_git_dir(&git_dir); } - #[test] - fn a_leftover_lock_file_with_no_active_os_lock_does_not_block_a_new_acquirer() { - let git_dir = unique_test_git_dir("leftover-lock-file"); - let dir = adapter_state_dir(&git_dir); - std::fs::create_dir_all(&dir).expect("state dir should be created"); - std::fs::write(lock_path(&git_dir), b"leftover") - .expect("leftover lock file should be writable"); - - let decision = admit_tracked_attempt( - &git_dir, - &key("session-1", None, "exec-1"), - "turn-1", - "Bash", - ); - assert!( - matches!(decision, Ok(AdmitDecision::Admitted(_))), - "a lock file with no active OS lock held against it must not block a new acquirer" - ); - - remove_test_git_dir(&git_dir); - } - const PARALLEL_ADMISSION_COUNT: u64 = 6; #[test] @@ -1374,42 +1333,6 @@ mod tests { remove_test_git_dir(&git_dir); } - #[test] - fn a_second_acquirer_blocks_until_the_first_releases() { - use std::sync::mpsc; - - let git_dir = unique_test_git_dir("lock-contention"); - std::fs::create_dir_all(&git_dir).expect("git dir should be created"); - - let holder = AdapterStateLock::acquire(&git_dir, Duration::from_secs(5)) - .expect("first acquirer should succeed immediately"); - - let (result_tx, result_rx) = mpsc::channel(); - let git_dir_clone = git_dir.clone(); - let handle = thread::spawn(move || { - let result = AdapterStateLock::acquire(&git_dir_clone, Duration::from_secs(5)); - let _ = result_tx.send(()); - result - }); - - assert!( - result_rx.recv_timeout(Duration::from_millis(300)).is_err(), - "second acquirer should not succeed while the first still holds the lock" - ); - - drop(holder); - - result_rx - .recv_timeout(Duration::from_secs(5)) - .expect("second acquirer should complete once the first releases the lock"); - assert!(handle - .join() - .expect("second acquirer thread should not panic") - .is_ok()); - - remove_test_git_dir(&git_dir); - } - #[test] fn every_locked_helper_releases_the_state_lock_before_returning() { let git_dir = unique_test_git_dir("lock-released-between-helpers"); @@ -1430,7 +1353,8 @@ mod tests { .expect("relinquish_recovery_flush must acquire the lock"); drop( - AdapterStateLock::acquire(&git_dir, Duration::from_millis(200)) + STATE_LOCK + .acquire_with_timeout(&adapter_state_dir(&git_dir), Duration::from_millis(200)) .expect("the state lock must be free once every helper has returned"), ); @@ -1446,7 +1370,6 @@ mod tests { let sce_dir = git_dir.join(SCE_STATE_DIR); assert!(state_path(&git_dir).starts_with(&sce_dir)); - assert!(lock_path(&git_dir).starts_with(&sce_dir)); let mut found_state_file = false; for entry in std::fs::read_dir(&sce_dir).expect("sce dir should be readable") { diff --git a/cli/src/services/hooks/codex_mutation_scope/tests.rs b/cli/src/services/hooks/codex_mutation_scope/tests.rs index 5aa72d25e..2418d4953 100644 --- a/cli/src/services/hooks/codex_mutation_scope/tests.rs +++ b/cli/src/services/hooks/codex_mutation_scope/tests.rs @@ -2249,10 +2249,9 @@ mod driver { } assert!( - !crate::services::hooks::codex_mutation_scope::boundary_lock::boundary_lock_path( - &git_dir - ) - .exists(), + !state::BOUNDARY_LOCK + .path(&state::adapter_state_dir(&git_dir)) + .exists(), "Test M: no untracked tool may create the adapter boundary lock", ); assert!( @@ -2299,10 +2298,9 @@ mod driver { "an untracked PostToolUse resolves no git dir and creates no adapter state directory", ); assert!( - !crate::services::hooks::codex_mutation_scope::boundary_lock::boundary_lock_path( - &git_dir - ) - .exists(), + !state::BOUNDARY_LOCK + .path(&state::adapter_state_dir(&git_dir)) + .exists(), "an untracked PostToolUse must not create the adapter boundary lock", ); @@ -2359,10 +2357,9 @@ mod driver { state lock, or boundary lock", ); assert!( - !crate::services::hooks::codex_mutation_scope::boundary_lock::boundary_lock_path( - &git_dir - ) - .exists(), + !state::BOUNDARY_LOCK + .path(&state::adapter_state_dir(&git_dir)) + .exists(), "a complete successful MCP lifecycle creates no boundary lock", ); @@ -2485,7 +2482,8 @@ mod driver { } fn boundary_lock_exists(git_dir: &Path) -> bool { - crate::services::hooks::codex_mutation_scope::boundary_lock::boundary_lock_path(git_dir) + state::BOUNDARY_LOCK + .path(&state::adapter_state_dir(git_dir)) .exists() } diff --git a/cli/src/services/hooks/mod.rs b/cli/src/services/hooks/mod.rs index a42fad7b2..e8b00793f 100644 --- a/cli/src/services/hooks/mod.rs +++ b/cli/src/services/hooks/mod.rs @@ -29,6 +29,7 @@ mod claude_transforms; mod commit_hooks; mod conversation_trace; mod diff_trace; +mod mutation_scope_lock; mod runtime; pub mod claude_bridge_session; diff --git a/cli/src/services/hooks/mutation_scope_lock.rs b/cli/src/services/hooks/mutation_scope_lock.rs new file mode 100644 index 000000000..78d6b20eb --- /dev/null +++ b/cli/src/services/hooks/mutation_scope_lock.rs @@ -0,0 +1,396 @@ +use std::fs::{File, OpenOptions, TryLockError}; +use std::path::{Path, PathBuf}; +use std::time::{Duration, Instant}; + +use anyhow::{anyhow, Context}; + +const LOCK_POLL_INTERVAL: Duration = Duration::from_millis(20); + +pub(crate) const DEFAULT_STATE_LOCK_TIMEOUT: Duration = Duration::from_secs(10); +pub(crate) const DEFAULT_BOUNDARY_LOCK_TIMEOUT: Duration = Duration::from_secs(10); + +const STATE_LOCK_WHAT: &str = "adapter-state"; +const BOUNDARY_LOCK_WHAT: &str = "adapter-boundary"; + +const STATE_LOCK_FAILURE_CONTEXT: &str = "Failed to acquire adapter-state lock"; +const BOUNDARY_LOCK_FAILURE_CONTEXT: &str = "Failed to acquire adapter boundary lock"; + +#[derive(Debug)] +pub(crate) enum AdvisoryLockError { + TimedOut { + path: PathBuf, + timeout: Duration, + what: &'static str, + }, + Io(anyhow::Error), +} + +impl std::fmt::Display for AdvisoryLockError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + AdvisoryLockError::TimedOut { + path, + timeout, + what, + } => write!( + f, + "Timed out after {timeout:?} waiting for the {what} lock '{}'", + path.display() + ), + AdvisoryLockError::Io(source) => write!(f, "{source}"), + } + } +} + +impl std::error::Error for AdvisoryLockError {} + +pub(crate) struct OsAdvisoryLock { + file: File, +} + +impl OsAdvisoryLock { + fn acquire( + parent_dir: &Path, + lock_path: PathBuf, + timeout: Duration, + what: &'static str, + ) -> Result { + std::fs::create_dir_all(parent_dir) + .with_context(|| { + format!( + "Failed to create {what} lock directory '{}'", + parent_dir.display() + ) + }) + .map_err(AdvisoryLockError::Io)?; + + let file = OpenOptions::new() + .write(true) + .create(true) + .truncate(false) + .open(&lock_path) + .with_context(|| format!("Failed to open {what} lock file '{}'", lock_path.display())) + .map_err(AdvisoryLockError::Io)?; + + let deadline = Instant::now() + timeout; + loop { + match file.try_lock() { + Ok(()) => return Ok(OsAdvisoryLock { file }), + Err(TryLockError::WouldBlock) => { + let now = Instant::now(); + if now >= deadline { + return Err(AdvisoryLockError::TimedOut { + path: lock_path, + timeout, + what, + }); + } + std::thread::sleep(LOCK_POLL_INTERVAL.min(deadline - now)); + } + Err(TryLockError::Error(source)) => { + return Err(AdvisoryLockError::Io(anyhow::Error::new(source).context( + format!("Failed to acquire {what} lock '{}'", lock_path.display()), + ))); + } + } + } + } +} + +impl Drop for OsAdvisoryLock { + fn drop(&mut self) { + let _ = self.file.unlock(); + } +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) struct AdapterLockSpec { + file_name: &'static str, + what: &'static str, + default_timeout: Duration, + failure_context: &'static str, +} + +impl AdapterLockSpec { + pub(crate) const fn state(file_name: &'static str) -> Self { + AdapterLockSpec { + file_name, + what: STATE_LOCK_WHAT, + default_timeout: DEFAULT_STATE_LOCK_TIMEOUT, + failure_context: STATE_LOCK_FAILURE_CONTEXT, + } + } + + pub(crate) const fn boundary(file_name: &'static str) -> Self { + AdapterLockSpec { + file_name, + what: BOUNDARY_LOCK_WHAT, + default_timeout: DEFAULT_BOUNDARY_LOCK_TIMEOUT, + failure_context: BOUNDARY_LOCK_FAILURE_CONTEXT, + } + } + + pub(crate) fn path(&self, adapter_state_dir: &Path) -> PathBuf { + adapter_state_dir.join(self.file_name) + } + + pub(crate) fn acquire(&self, adapter_state_dir: &Path) -> anyhow::Result { + self.acquire_with_timeout(adapter_state_dir, self.default_timeout) + .map_err(|error| anyhow!("{}: {error}", self.failure_context)) + } + + pub(crate) fn acquire_with_timeout( + &self, + adapter_state_dir: &Path, + timeout: Duration, + ) -> Result { + OsAdvisoryLock::acquire( + adapter_state_dir, + self.path(adapter_state_dir), + timeout, + self.what, + ) + } +} + +#[cfg(test)] +mod tests { + use std::io::Write as _; + use std::process::Command; + use std::sync::atomic::{AtomicU64, Ordering}; + use std::sync::mpsc; + use std::thread; + + use super::*; + use crate::services::hooks::{ + codex_mutation_scope, opencode_mutation_scope, pi_mutation_scope, + }; + + const TEST_LOCK: AdapterLockSpec = AdapterLockSpec::boundary("test-mutation-scope.lock"); + + static NEXT_ID: AtomicU64 = AtomicU64::new(0); + + fn unique_lock_dir(label: &str) -> PathBuf { + let id = NEXT_ID.fetch_add(1, Ordering::Relaxed); + std::env::temp_dir().join(format!( + "sce-mutation-scope-lock-{label}-{}-{id}", + std::process::id() + )) + } + + const CHILD_ENV_LOCK_DIR: &str = "SCE_MUTATION_SCOPE_LOCK_CHILD_DIR"; + const CHILD_ENV_TIMEOUT_MS: &str = "SCE_MUTATION_SCOPE_LOCK_CHILD_TIMEOUT_MS"; + const CHILD_HELPER_PATH: &str = + "services::hooks::mutation_scope_lock::tests::mutation_scope_lock_child_helper"; + + #[test] + #[ignore = "subprocess helper, driven by process_death_releases_the_lock"] + fn mutation_scope_lock_child_helper() { + let lock_dir = PathBuf::from( + std::env::var(CHILD_ENV_LOCK_DIR).expect("child helper needs a lock dir in the env"), + ); + let timeout = Duration::from_millis( + std::env::var(CHILD_ENV_TIMEOUT_MS) + .expect("child helper needs a timeout in the env") + .parse() + .expect("timeout must parse"), + ); + match TEST_LOCK.acquire_with_timeout(&lock_dir, timeout) { + Ok(lock) => { + print!("ACQUIRED"); + std::io::stdout().flush().expect("flush stdout"); + std::mem::forget(lock); + } + Err(AdvisoryLockError::TimedOut { .. }) => { + print!("TIMEOUT"); + std::io::stdout().flush().expect("flush stdout"); + } + Err(other) => panic!("unexpected child lock error: {other}"), + } + } + + fn run_child(lock_dir: &Path, timeout: Duration) -> String { + let exe = std::env::current_exe().expect("test executable path should resolve"); + let output = Command::new(exe) + .args(["--exact", "--ignored", "--nocapture", CHILD_HELPER_PATH]) + .env(CHILD_ENV_LOCK_DIR, lock_dir) + .env(CHILD_ENV_TIMEOUT_MS, timeout.as_millis().to_string()) + .output() + .expect("the child test process should spawn"); + assert!( + output.status.success(), + "child process failed: stdout={:?} stderr={:?}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + let stdout = String::from_utf8_lossy(&output.stdout); + if stdout.contains("ACQUIRED") { + "ACQUIRED".to_string() + } else if stdout.contains("TIMEOUT") { + "TIMEOUT".to_string() + } else { + panic!("child produced no lock verdict: {stdout}"); + } + } + + #[test] + fn process_death_releases_the_lock() { + let lock_dir = unique_lock_dir("process-death"); + + let held = TEST_LOCK + .acquire_with_timeout(&lock_dir, Duration::from_secs(5)) + .expect("the parent should acquire the lock immediately"); + + assert_eq!( + run_child(&lock_dir, Duration::from_millis(300)), + "TIMEOUT", + "a second OS process must not acquire the lock while the parent holds it", + ); + + drop(held); + + assert_eq!( + run_child(&lock_dir, Duration::from_millis(300)), + "ACQUIRED", + "once the parent releases, a child process acquires it", + ); + + let reacquired = TEST_LOCK + .acquire_with_timeout(&lock_dir, Duration::from_secs(2)) + .expect("the child's process exit must have released the lock"); + drop(reacquired); + + let _ = std::fs::remove_dir_all(&lock_dir); + } + + #[test] + fn a_leftover_lock_file_alone_does_not_block_acquisition_and_is_never_truncated_or_removed() { + let lock_dir = unique_lock_dir("leftover-file"); + std::fs::create_dir_all(&lock_dir).expect("lock dir should be created"); + let lock_path = TEST_LOCK.path(&lock_dir); + std::fs::write(&lock_path, b"leftover").expect("leftover lock file should be writable"); + + let lock = TEST_LOCK + .acquire_with_timeout(&lock_dir, Duration::from_millis(200)) + .expect("a lock file with no live OS owner must not block a new acquirer"); + drop(lock); + + assert_eq!( + std::fs::read(&lock_path).expect("the lock file must survive release"), + b"leftover", + "ownership must never be expressed by truncating or removing the lock file", + ); + + let _ = std::fs::remove_dir_all(&lock_dir); + } + + #[test] + fn a_second_in_process_acquirer_blocks_until_the_first_releases() { + let lock_dir = unique_lock_dir("in-process-contention"); + + let holder = TEST_LOCK + .acquire_with_timeout(&lock_dir, Duration::from_secs(5)) + .expect("first acquirer should succeed immediately"); + + let (tx, rx) = mpsc::channel(); + let lock_dir_clone = lock_dir.clone(); + let handle = thread::spawn(move || { + let result = TEST_LOCK.acquire_with_timeout(&lock_dir_clone, Duration::from_secs(5)); + let _ = tx.send(()); + result.is_ok() + }); + + assert!( + rx.recv_timeout(Duration::from_millis(300)).is_err(), + "the second acquirer must not proceed while the first holds the lock", + ); + + drop(holder); + + rx.recv_timeout(Duration::from_secs(5)) + .expect("the second acquirer should complete once the first releases"); + assert!(handle + .join() + .expect("second acquirer thread should not panic")); + + let _ = std::fs::remove_dir_all(&lock_dir); + } + + #[test] + fn a_contended_acquire_times_out_with_a_role_specific_diagnostic() { + let lock_dir = unique_lock_dir("timeout"); + let state_lock = AdapterLockSpec::state("test-mutation-scope-state.lock"); + + let _boundary_holder = TEST_LOCK + .acquire_with_timeout(&lock_dir, Duration::from_secs(5)) + .expect("boundary holder should acquire immediately"); + let _state_holder = state_lock + .acquire_with_timeout(&lock_dir, Duration::from_secs(5)) + .expect("state holder should acquire immediately"); + + for (spec, what) in [ + (TEST_LOCK, "adapter-boundary"), + (state_lock, "adapter-state"), + ] { + let timeout = Duration::from_millis(100); + let started = Instant::now(); + let Err(error) = spec.acquire_with_timeout(&lock_dir, timeout) else { + panic!("a held {what} lock must not be acquired a second time"); + }; + assert!( + started.elapsed() < Duration::from_secs(5), + "a contended acquire must give up at its timeout", + ); + let expected = format!( + "Timed out after {timeout:?} waiting for the {what} lock '{}'", + spec.path(&lock_dir).display() + ); + assert_eq!(error.to_string(), expected); + assert!(matches!( + &error, + AdvisoryLockError::TimedOut { path, timeout: t, what: w } + if *path == spec.path(&lock_dir) && *t == timeout && *w == what + )); + } + + let _ = std::fs::remove_dir_all(&lock_dir); + } + + #[test] + fn adapter_lock_identities_are_exact_distinct_and_below_the_adapter_state_dir() { + let git_dir = unique_lock_dir("adapter-identities"); + let rows = [ + ( + codex_mutation_scope::state::adapter_state_dir(&git_dir), + codex_mutation_scope::state::STATE_LOCK, + codex_mutation_scope::state::BOUNDARY_LOCK, + "codex-mutation-scope-state.lock", + "codex-mutation-scope-boundary.lock", + ), + ( + opencode_mutation_scope::state::adapter_state_dir(&git_dir), + opencode_mutation_scope::state::STATE_LOCK, + opencode_mutation_scope::state::BOUNDARY_LOCK, + "opencode-mutation-scope-state.lock", + "opencode-mutation-scope-boundary.lock", + ), + ( + pi_mutation_scope::state::adapter_state_dir(&git_dir), + pi_mutation_scope::state::STATE_LOCK, + pi_mutation_scope::state::BOUNDARY_LOCK, + "pi-mutation-scope-state.lock", + "pi-mutation-scope-boundary.lock", + ), + ]; + + let sce_dir = git_dir.join("sce"); + for (state_dir, state_lock, boundary_lock, state_file, boundary_file) in rows { + assert_eq!(state_dir, sce_dir); + assert_eq!(state_lock, AdapterLockSpec::state(state_file)); + assert_eq!(boundary_lock, AdapterLockSpec::boundary(boundary_file)); + assert_eq!(state_lock.path(&state_dir), sce_dir.join(state_file)); + assert_eq!(boundary_lock.path(&state_dir), sce_dir.join(boundary_file)); + assert_ne!(state_lock.path(&state_dir), boundary_lock.path(&state_dir)); + } + } +} diff --git a/cli/src/services/hooks/opencode_mutation_scope/boundary_lock.rs b/cli/src/services/hooks/opencode_mutation_scope/boundary_lock.rs deleted file mode 100644 index 863343a12..000000000 --- a/cli/src/services/hooks/opencode_mutation_scope/boundary_lock.rs +++ /dev/null @@ -1,109 +0,0 @@ -use std::path::{Path, PathBuf}; -use std::time::Duration; - -use super::os_lock::{AdvisoryLockError, OsAdvisoryLock}; -use super::state::adapter_state_dir; - -const ADAPTER_BOUNDARY_LOCK_FILE: &str = "opencode-mutation-scope-boundary.lock"; -const BOUNDARY_LOCK_WHAT: &str = "adapter-boundary"; - -pub(crate) const DEFAULT_BOUNDARY_LOCK_TIMEOUT: Duration = Duration::from_secs(10); - -pub(crate) fn boundary_lock_path(git_dir: &Path) -> PathBuf { - adapter_state_dir(git_dir).join(ADAPTER_BOUNDARY_LOCK_FILE) -} - -pub(crate) struct AdapterBoundaryLock { - _inner: OsAdvisoryLock, -} - -impl AdapterBoundaryLock { - pub(crate) fn acquire( - git_dir: &Path, - timeout: Duration, - ) -> Result { - let inner = OsAdvisoryLock::acquire( - &adapter_state_dir(git_dir), - boundary_lock_path(git_dir), - timeout, - BOUNDARY_LOCK_WHAT, - )?; - Ok(AdapterBoundaryLock { _inner: inner }) - } -} - -#[cfg(test)] -mod tests { - use std::sync::atomic::{AtomicU64, Ordering}; - use std::sync::mpsc; - use std::thread; - - use super::*; - - static NEXT_ID: AtomicU64 = AtomicU64::new(0); - - fn unique_git_dir(label: &str) -> PathBuf { - let id = NEXT_ID.fetch_add(1, Ordering::Relaxed); - std::env::temp_dir().join(format!( - "sce-opencode-mutation-scope-boundary-{label}-{}-{id}", - std::process::id() - )) - } - - #[test] - fn a_leftover_boundary_lock_file_alone_does_not_block_acquisition() { - let git_dir = unique_git_dir("leftover-file"); - std::fs::create_dir_all(adapter_state_dir(&git_dir)).expect("state dir should be created"); - std::fs::write(boundary_lock_path(&git_dir), b"leftover") - .expect("leftover lock file should be writable"); - - AdapterBoundaryLock::acquire(&git_dir, Duration::from_millis(200)) - .expect("a lock file with no live OS owner must not block a new acquirer"); - - let _ = std::fs::remove_dir_all(&git_dir); - } - - #[test] - fn a_second_in_process_acquirer_blocks_until_the_first_releases() { - let git_dir = unique_git_dir("in-process-contention"); - std::fs::create_dir_all(&git_dir).expect("git dir should be created"); - - let holder = AdapterBoundaryLock::acquire(&git_dir, Duration::from_secs(5)) - .expect("first acquirer should succeed immediately"); - - let (tx, rx) = mpsc::channel(); - let git_dir_clone = git_dir.clone(); - let handle = thread::spawn(move || { - let result = AdapterBoundaryLock::acquire(&git_dir_clone, Duration::from_secs(5)); - let _ = tx.send(()); - result.is_ok() - }); - - assert!( - rx.recv_timeout(Duration::from_millis(300)).is_err(), - "the second acquirer must not proceed while the first holds the boundary lock", - ); - - drop(holder); - - rx.recv_timeout(Duration::from_secs(5)) - .expect("the second acquirer should complete once the first releases"); - assert!(handle - .join() - .expect("second acquirer thread should not panic")); - - let _ = std::fs::remove_dir_all(&git_dir); - } - - #[test] - fn the_boundary_lock_path_is_distinct_from_the_state_lock_and_lives_under_sce() { - let git_dir = unique_git_dir("path-shape"); - let path = boundary_lock_path(&git_dir); - assert!(path.starts_with(adapter_state_dir(&git_dir))); - assert!(path.ends_with(ADAPTER_BOUNDARY_LOCK_FILE)); - assert_ne!( - path.file_name(), - Path::new("opencode-mutation-scope-state.lock").file_name(), - ); - } -} diff --git a/cli/src/services/hooks/opencode_mutation_scope/lifecycle.rs b/cli/src/services/hooks/opencode_mutation_scope/lifecycle.rs index 8f8a7492c..a1d5215a7 100644 --- a/cli/src/services/hooks/opencode_mutation_scope/lifecycle.rs +++ b/cli/src/services/hooks/opencode_mutation_scope/lifecycle.rs @@ -1,12 +1,11 @@ use std::path::{Path, PathBuf}; -use anyhow::{anyhow, bail, Result}; +use anyhow::{bail, Result}; use crate::services::hooks; use crate::services::mutation_trace::runtime::resolve_git_dir; use crate::services::observability::traits::Logger; -use super::boundary_lock::{AdapterBoundaryLock, DEFAULT_BOUNDARY_LOCK_TIMEOUT}; use super::events::{ opencode_scope_close_event_id, opencode_scope_provenance, opencode_scope_start_event_id, parse_opencode_hook_event, AttemptKey, OpenCodeHookEvent, OpenCodeScopeProvenance, @@ -202,8 +201,7 @@ pub(super) fn with_boundary_lock( git_dir: &Path, operation: impl FnOnce() -> Result, ) -> Result { - let _boundary = AdapterBoundaryLock::acquire(git_dir, DEFAULT_BOUNDARY_LOCK_TIMEOUT) - .map_err(|error| anyhow!("Failed to acquire adapter boundary lock: {error}"))?; + let _boundary = state::BOUNDARY_LOCK.acquire(&state::adapter_state_dir(git_dir))?; operation() } diff --git a/cli/src/services/hooks/opencode_mutation_scope/mod.rs b/cli/src/services/hooks/opencode_mutation_scope/mod.rs index bae7d60bd..cca8c28b0 100644 --- a/cli/src/services/hooks/opencode_mutation_scope/mod.rs +++ b/cli/src/services/hooks/opencode_mutation_scope/mod.rs @@ -1,10 +1,8 @@ #![allow(dead_code)] -mod boundary_lock; mod events; pub(crate) mod health; mod lifecycle; -mod os_lock; mod payload; pub(crate) mod state; diff --git a/cli/src/services/hooks/opencode_mutation_scope/os_lock.rs b/cli/src/services/hooks/opencode_mutation_scope/os_lock.rs deleted file mode 100644 index dc567c1b7..000000000 --- a/cli/src/services/hooks/opencode_mutation_scope/os_lock.rs +++ /dev/null @@ -1,95 +0,0 @@ -use std::fs::{File, OpenOptions, TryLockError}; -use std::path::{Path, PathBuf}; -use std::time::{Duration, Instant}; - -use anyhow::Context; - -const LOCK_POLL_INTERVAL: Duration = Duration::from_millis(20); - -#[derive(Debug)] -pub(crate) enum AdvisoryLockError { - TimedOut { - path: PathBuf, - timeout: Duration, - what: &'static str, - }, - Io(anyhow::Error), -} - -impl std::fmt::Display for AdvisoryLockError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - AdvisoryLockError::TimedOut { - path, - timeout, - what, - } => write!( - f, - "Timed out after {timeout:?} waiting for the {what} lock '{}'", - path.display() - ), - AdvisoryLockError::Io(source) => write!(f, "{source}"), - } - } -} - -impl std::error::Error for AdvisoryLockError {} - -pub(crate) struct OsAdvisoryLock { - file: File, -} - -impl OsAdvisoryLock { - pub(crate) fn acquire( - parent_dir: &Path, - lock_path: PathBuf, - timeout: Duration, - what: &'static str, - ) -> Result { - std::fs::create_dir_all(parent_dir) - .with_context(|| { - format!( - "Failed to create {what} lock directory '{}'", - parent_dir.display() - ) - }) - .map_err(AdvisoryLockError::Io)?; - - let file = OpenOptions::new() - .write(true) - .create(true) - .truncate(false) - .open(&lock_path) - .with_context(|| format!("Failed to open {what} lock file '{}'", lock_path.display())) - .map_err(AdvisoryLockError::Io)?; - - let deadline = Instant::now() + timeout; - loop { - match file.try_lock() { - Ok(()) => return Ok(OsAdvisoryLock { file }), - Err(TryLockError::WouldBlock) => { - let now = Instant::now(); - if now >= deadline { - return Err(AdvisoryLockError::TimedOut { - path: lock_path, - timeout, - what, - }); - } - std::thread::sleep(LOCK_POLL_INTERVAL.min(deadline - now)); - } - Err(TryLockError::Error(source)) => { - return Err(AdvisoryLockError::Io(anyhow::Error::new(source).context( - format!("Failed to acquire {what} lock '{}'", lock_path.display()), - ))); - } - } - } - } -} - -impl Drop for OsAdvisoryLock { - fn drop(&mut self) { - let _ = self.file.unlock(); - } -} diff --git a/cli/src/services/hooks/opencode_mutation_scope/state.rs b/cli/src/services/hooks/opencode_mutation_scope/state.rs index a8c0a0d85..fc9bbab6e 100644 --- a/cli/src/services/hooks/opencode_mutation_scope/state.rs +++ b/cli/src/services/hooks/opencode_mutation_scope/state.rs @@ -1,12 +1,12 @@ use std::fs::OpenOptions; use std::io::Write as _; use std::path::{Path, PathBuf}; -use std::time::Duration; use anyhow::{anyhow, Context, Result}; use serde::{Deserialize, Serialize}; -use super::os_lock::{AdvisoryLockError, OsAdvisoryLock}; +use crate::services::hooks::mutation_scope_lock::{AdapterLockSpec, OsAdvisoryLock}; + use super::{format_opencode_scope_id, AttemptKey}; use crate::services::hooks::mutation_scope_owner::{ current_process_owner, is_definitely_dead, ProcessOwner, @@ -14,10 +14,11 @@ use crate::services::hooks::mutation_scope_owner::{ const SCE_STATE_DIR: &str = "sce"; const ADAPTER_STATE_FILE: &str = "opencode-mutation-scope-state.json"; -const ADAPTER_STATE_LOCK_FILE: &str = "opencode-mutation-scope-state.lock"; -const STATE_LOCK_WHAT: &str = "adapter-state"; -const DEFAULT_LOCK_TIMEOUT: Duration = Duration::from_secs(10); +pub(crate) const STATE_LOCK: AdapterLockSpec = + AdapterLockSpec::state("opencode-mutation-scope-state.lock"); +pub(crate) const BOUNDARY_LOCK: AdapterLockSpec = + AdapterLockSpec::boundary("opencode-mutation-scope-boundary.lock"); const ADAPTER_STATE_VERSION: u32 = 1; @@ -119,26 +120,6 @@ pub(crate) fn state_path(git_dir: &Path) -> PathBuf { adapter_state_dir(git_dir).join(ADAPTER_STATE_FILE) } -fn lock_path(git_dir: &Path) -> PathBuf { - adapter_state_dir(git_dir).join(ADAPTER_STATE_LOCK_FILE) -} - -struct AdapterStateLock { - _inner: OsAdvisoryLock, -} - -impl AdapterStateLock { - fn acquire(git_dir: &Path, timeout: Duration) -> Result { - let inner = OsAdvisoryLock::acquire( - &adapter_state_dir(git_dir), - lock_path(git_dir), - timeout, - STATE_LOCK_WHAT, - )?; - Ok(AdapterStateLock { _inner: inner }) - } -} - pub(crate) fn read_state(git_dir: &Path) -> Result { let path = state_path(git_dir); if !path.exists() { @@ -235,9 +216,8 @@ where Ok(()) } -fn acquire_lock(git_dir: &Path) -> Result { - AdapterStateLock::acquire(git_dir, DEFAULT_LOCK_TIMEOUT) - .map_err(|err| anyhow!("Failed to acquire adapter-state lock: {err}")) +fn acquire_lock(git_dir: &Path) -> Result { + STATE_LOCK.acquire(&adapter_state_dir(git_dir)) } fn allocate_pending_start( @@ -520,6 +500,7 @@ pub(crate) fn set_attempt_owner_for_tests( mod tests { use std::sync::atomic::{AtomicU64, Ordering}; use std::thread; + use std::time::Duration; use super::*; @@ -1136,7 +1117,6 @@ mod tests { let sce_dir = git_dir.join(SCE_STATE_DIR); assert!(state_path(&git_dir).starts_with(&sce_dir)); - assert!(lock_path(&git_dir).starts_with(&sce_dir)); remove_test_git_dir(&git_dir); } diff --git a/cli/src/services/hooks/pi_mutation_scope/boundary_lock.rs b/cli/src/services/hooks/pi_mutation_scope/boundary_lock.rs deleted file mode 100644 index b8665bc6e..000000000 --- a/cli/src/services/hooks/pi_mutation_scope/boundary_lock.rs +++ /dev/null @@ -1,109 +0,0 @@ -use std::path::{Path, PathBuf}; -use std::time::Duration; - -use super::os_lock::{AdvisoryLockError, OsAdvisoryLock}; -use super::state::adapter_state_dir; - -const ADAPTER_BOUNDARY_LOCK_FILE: &str = "pi-mutation-scope-boundary.lock"; -const BOUNDARY_LOCK_WHAT: &str = "adapter-boundary"; - -pub(crate) const DEFAULT_BOUNDARY_LOCK_TIMEOUT: Duration = Duration::from_secs(10); - -pub(crate) fn boundary_lock_path(git_dir: &Path) -> PathBuf { - adapter_state_dir(git_dir).join(ADAPTER_BOUNDARY_LOCK_FILE) -} - -pub(crate) struct AdapterBoundaryLock { - _inner: OsAdvisoryLock, -} - -impl AdapterBoundaryLock { - pub(crate) fn acquire( - git_dir: &Path, - timeout: Duration, - ) -> Result { - let inner = OsAdvisoryLock::acquire( - &adapter_state_dir(git_dir), - boundary_lock_path(git_dir), - timeout, - BOUNDARY_LOCK_WHAT, - )?; - Ok(AdapterBoundaryLock { _inner: inner }) - } -} - -#[cfg(test)] -mod tests { - use std::sync::atomic::{AtomicU64, Ordering}; - use std::sync::mpsc; - use std::thread; - - use super::*; - - static NEXT_ID: AtomicU64 = AtomicU64::new(0); - - fn unique_git_dir(label: &str) -> PathBuf { - let id = NEXT_ID.fetch_add(1, Ordering::Relaxed); - std::env::temp_dir().join(format!( - "sce-pi-mutation-scope-boundary-{label}-{}-{id}", - std::process::id() - )) - } - - #[test] - fn a_leftover_boundary_lock_file_alone_does_not_block_acquisition() { - let git_dir = unique_git_dir("leftover-file"); - std::fs::create_dir_all(adapter_state_dir(&git_dir)).expect("state dir should be created"); - std::fs::write(boundary_lock_path(&git_dir), b"leftover") - .expect("leftover lock file should be writable"); - - AdapterBoundaryLock::acquire(&git_dir, Duration::from_millis(200)) - .expect("a lock file with no live OS owner must not block a new acquirer"); - - let _ = std::fs::remove_dir_all(&git_dir); - } - - #[test] - fn a_second_in_process_acquirer_blocks_until_the_first_releases() { - let git_dir = unique_git_dir("in-process-contention"); - std::fs::create_dir_all(&git_dir).expect("git dir should be created"); - - let holder = AdapterBoundaryLock::acquire(&git_dir, Duration::from_secs(5)) - .expect("first acquirer should succeed immediately"); - - let (tx, rx) = mpsc::channel(); - let git_dir_clone = git_dir.clone(); - let handle = thread::spawn(move || { - let result = AdapterBoundaryLock::acquire(&git_dir_clone, Duration::from_secs(5)); - let _ = tx.send(()); - result.is_ok() - }); - - assert!( - rx.recv_timeout(Duration::from_millis(300)).is_err(), - "the second acquirer must not proceed while the first holds the boundary lock", - ); - - drop(holder); - - rx.recv_timeout(Duration::from_secs(5)) - .expect("the second acquirer should complete once the first releases"); - assert!(handle - .join() - .expect("second acquirer thread should not panic")); - - let _ = std::fs::remove_dir_all(&git_dir); - } - - #[test] - fn the_boundary_lock_path_is_distinct_from_the_state_lock_and_lives_under_sce() { - let git_dir = unique_git_dir("path-shape"); - let path = boundary_lock_path(&git_dir); - assert!(path.starts_with(adapter_state_dir(&git_dir))); - assert!(path.ends_with(ADAPTER_BOUNDARY_LOCK_FILE)); - assert_ne!( - path.file_name(), - Path::new("pi-mutation-scope-state.lock").file_name(), - ); - } -} diff --git a/cli/src/services/hooks/pi_mutation_scope/lifecycle.rs b/cli/src/services/hooks/pi_mutation_scope/lifecycle.rs index 4d6202ebf..d02dc258f 100644 --- a/cli/src/services/hooks/pi_mutation_scope/lifecycle.rs +++ b/cli/src/services/hooks/pi_mutation_scope/lifecycle.rs @@ -1,12 +1,11 @@ use std::path::{Path, PathBuf}; -use anyhow::{anyhow, bail, Result}; +use anyhow::{bail, Result}; use crate::services::hooks; use crate::services::mutation_trace::runtime::resolve_git_dir; use crate::services::observability::traits::Logger; -use super::boundary_lock::{AdapterBoundaryLock, DEFAULT_BOUNDARY_LOCK_TIMEOUT}; use super::state::{self, AdmitDecision, RecoveryFlushCompletion}; use super::{ abandon_payload, flush_payload, parse_pi_hook_event, pi_scope_close_event_id, @@ -162,8 +161,7 @@ pub(super) fn with_boundary_lock( git_dir: &Path, operation: impl FnOnce() -> Result, ) -> Result { - let _boundary = AdapterBoundaryLock::acquire(git_dir, DEFAULT_BOUNDARY_LOCK_TIMEOUT) - .map_err(|error| anyhow!("Failed to acquire adapter boundary lock: {error}"))?; + let _boundary = state::BOUNDARY_LOCK.acquire(&state::adapter_state_dir(git_dir))?; operation() } diff --git a/cli/src/services/hooks/pi_mutation_scope/mod.rs b/cli/src/services/hooks/pi_mutation_scope/mod.rs index 017e6bcf4..cc35fafa2 100644 --- a/cli/src/services/hooks/pi_mutation_scope/mod.rs +++ b/cli/src/services/hooks/pi_mutation_scope/mod.rs @@ -1,10 +1,8 @@ #![allow(dead_code)] -mod boundary_lock; mod events; pub(crate) mod health; mod lifecycle; -mod os_lock; mod payload; pub(crate) mod state; diff --git a/cli/src/services/hooks/pi_mutation_scope/os_lock.rs b/cli/src/services/hooks/pi_mutation_scope/os_lock.rs deleted file mode 100644 index dc567c1b7..000000000 --- a/cli/src/services/hooks/pi_mutation_scope/os_lock.rs +++ /dev/null @@ -1,95 +0,0 @@ -use std::fs::{File, OpenOptions, TryLockError}; -use std::path::{Path, PathBuf}; -use std::time::{Duration, Instant}; - -use anyhow::Context; - -const LOCK_POLL_INTERVAL: Duration = Duration::from_millis(20); - -#[derive(Debug)] -pub(crate) enum AdvisoryLockError { - TimedOut { - path: PathBuf, - timeout: Duration, - what: &'static str, - }, - Io(anyhow::Error), -} - -impl std::fmt::Display for AdvisoryLockError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - AdvisoryLockError::TimedOut { - path, - timeout, - what, - } => write!( - f, - "Timed out after {timeout:?} waiting for the {what} lock '{}'", - path.display() - ), - AdvisoryLockError::Io(source) => write!(f, "{source}"), - } - } -} - -impl std::error::Error for AdvisoryLockError {} - -pub(crate) struct OsAdvisoryLock { - file: File, -} - -impl OsAdvisoryLock { - pub(crate) fn acquire( - parent_dir: &Path, - lock_path: PathBuf, - timeout: Duration, - what: &'static str, - ) -> Result { - std::fs::create_dir_all(parent_dir) - .with_context(|| { - format!( - "Failed to create {what} lock directory '{}'", - parent_dir.display() - ) - }) - .map_err(AdvisoryLockError::Io)?; - - let file = OpenOptions::new() - .write(true) - .create(true) - .truncate(false) - .open(&lock_path) - .with_context(|| format!("Failed to open {what} lock file '{}'", lock_path.display())) - .map_err(AdvisoryLockError::Io)?; - - let deadline = Instant::now() + timeout; - loop { - match file.try_lock() { - Ok(()) => return Ok(OsAdvisoryLock { file }), - Err(TryLockError::WouldBlock) => { - let now = Instant::now(); - if now >= deadline { - return Err(AdvisoryLockError::TimedOut { - path: lock_path, - timeout, - what, - }); - } - std::thread::sleep(LOCK_POLL_INTERVAL.min(deadline - now)); - } - Err(TryLockError::Error(source)) => { - return Err(AdvisoryLockError::Io(anyhow::Error::new(source).context( - format!("Failed to acquire {what} lock '{}'", lock_path.display()), - ))); - } - } - } - } -} - -impl Drop for OsAdvisoryLock { - fn drop(&mut self) { - let _ = self.file.unlock(); - } -} diff --git a/cli/src/services/hooks/pi_mutation_scope/state.rs b/cli/src/services/hooks/pi_mutation_scope/state.rs index 049786361..a64c37aff 100644 --- a/cli/src/services/hooks/pi_mutation_scope/state.rs +++ b/cli/src/services/hooks/pi_mutation_scope/state.rs @@ -1,12 +1,12 @@ use std::fs::OpenOptions; use std::io::Write as _; use std::path::{Path, PathBuf}; -use std::time::Duration; use anyhow::{anyhow, Context, Result}; use serde::{Deserialize, Serialize}; -use super::os_lock::{AdvisoryLockError, OsAdvisoryLock}; +use crate::services::hooks::mutation_scope_lock::{AdapterLockSpec, OsAdvisoryLock}; + use super::{format_pi_scope_id, AttemptKey}; use crate::services::hooks::mutation_scope_owner::{ current_process_owner, is_definitely_dead, ProcessOwner, @@ -14,10 +14,11 @@ use crate::services::hooks::mutation_scope_owner::{ const SCE_STATE_DIR: &str = "sce"; const ADAPTER_STATE_FILE: &str = "pi-mutation-scope-state.json"; -const ADAPTER_STATE_LOCK_FILE: &str = "pi-mutation-scope-state.lock"; -const STATE_LOCK_WHAT: &str = "adapter-state"; -const DEFAULT_LOCK_TIMEOUT: Duration = Duration::from_secs(10); +pub(crate) const STATE_LOCK: AdapterLockSpec = + AdapterLockSpec::state("pi-mutation-scope-state.lock"); +pub(crate) const BOUNDARY_LOCK: AdapterLockSpec = + AdapterLockSpec::boundary("pi-mutation-scope-boundary.lock"); const ADAPTER_STATE_VERSION: u32 = 2; @@ -126,26 +127,6 @@ pub(crate) fn state_path(git_dir: &Path) -> PathBuf { adapter_state_dir(git_dir).join(ADAPTER_STATE_FILE) } -fn lock_path(git_dir: &Path) -> PathBuf { - adapter_state_dir(git_dir).join(ADAPTER_STATE_LOCK_FILE) -} - -struct AdapterStateLock { - _inner: OsAdvisoryLock, -} - -impl AdapterStateLock { - fn acquire(git_dir: &Path, timeout: Duration) -> Result { - let inner = OsAdvisoryLock::acquire( - &adapter_state_dir(git_dir), - lock_path(git_dir), - timeout, - STATE_LOCK_WHAT, - )?; - Ok(AdapterStateLock { _inner: inner }) - } -} - pub(crate) fn read_state(git_dir: &Path) -> Result { let path = state_path(git_dir); if !path.exists() { @@ -242,9 +223,8 @@ where Ok(()) } -fn acquire_lock(git_dir: &Path) -> Result { - AdapterStateLock::acquire(git_dir, DEFAULT_LOCK_TIMEOUT) - .map_err(|err| anyhow!("Failed to acquire adapter-state lock: {err}")) +fn acquire_lock(git_dir: &Path) -> Result { + STATE_LOCK.acquire(&adapter_state_dir(git_dir)) } fn allocate_pending_start( @@ -965,7 +945,6 @@ mod tests { let sce_dir = git_dir.join(SCE_STATE_DIR); assert!(state_path(&git_dir).starts_with(&sce_dir)); - assert!(lock_path(&git_dir).starts_with(&sce_dir)); remove_test_git_dir(&git_dir); } diff --git a/context/cli/codex-mutation-scope-integration.md b/context/cli/codex-mutation-scope-integration.md index 96bb7fd60..98753bf23 100644 --- a/context/cli/codex-mutation-scope-integration.md +++ b/context/cli/codex-mutation-scope-integration.md @@ -142,7 +142,10 @@ Version-3 JSON stores `next_attempt_seq`, recovery generation, tool, lane turn, and `pending_start`/`active` phase. Writes stage, `sync_data`, rename, and sync the directory where supported. The boundary lock serializes state-to-ingress-to-state across hook processes (boundary lock then state lock); -file existence alone is never ownership. +file existence alone is never ownership. Both locks are `AdapterLockSpec` +identities (`state::STATE_LOCK`, `state::BOUNDARY_LOCK`) over the single +OS advisory-lock implementation in `cli/src/services/hooks/mutation_scope_lock.rs`, +shared with the OpenCode and Pi adapters. Positive cleanup arms `recovery_pending`, abandons matching tracked scopes via generic `abandon`, and removes settled attempts. Known attempts keep tracked diff --git a/context/cli/doctor-recovery-safety-model.md b/context/cli/doctor-recovery-safety-model.md index f68551503..9185e9684 100644 --- a/context/cli/doctor-recovery-safety-model.md +++ b/context/cli/doctor-recovery-safety-model.md @@ -25,7 +25,7 @@ each real adapter releases before every mutation-scope seam call. | `Health = Healthy \| Recovering \| Blocked \| Invalid` via `adapterHealth(phase, recovery)` | each adapter's own `classify_health`, computed from every attempt's phase plus the one shared recovery fact — never from a single attempt in isolation | | `FixState = NotAttempted \| RepairAttempted \| RepairCompleted \| ReportedFixed \| ReportedManual` (adapter-wide) | doctor's one fix-result lifecycle per adapter target (`assess_repairability` -> `repair_blocked` -> re-diagnose -> one `DoctorFixResultRecord`) | | `OwnerReading = SeenAlive \| SeenDead \| SeenUnknown` | `mutation_scope_owner::is_definitely_dead`'s `Alive`/`Dead`/`Unknown` result (PID + `/proc` start-time liveness, never time-based) | -| `stateTxn: StateTxnHolder` | OpenCode's `AdapterStateLock` transaction (and Claude's equivalent short state-file lock) around one re-read/re-prove/durable-transition step — **not** OpenCode's coarser `AdapterBoundaryLock`, which this model has no dedicated variable for (see below) | +| `stateTxn: StateTxnHolder` | OpenCode's adapter-state lock (`state::STATE_LOCK`) transaction (and Claude's equivalent short state-file lock) around one re-read/re-prove/durable-transition step — **not** OpenCode's coarser adapter-boundary lock (`state::BOUNDARY_LOCK`), which this model has no dedicated variable for (see below) | `RecoveryState`/`FixState` are single adapter-wide variables, not per-attempt: both real adapters report one health value and one fix result @@ -36,7 +36,7 @@ remains `PendingAbandon`, so a second obligation can join an in-flight recovery pipeline without disturbing it. **Scope boundary:** `stateTxn` verifies only the short per-transaction lock. -OpenCode's `AdapterBoundaryLock` — which serializes a repair's whole +OpenCode's adapter-boundary lock — which serializes a repair's whole lifecycle, seam calls included — is a separate, coarser concern outside this model's variables; T03 is responsible for acquiring it around the whole `repair_blocked` call, wrapping (not replacing) the `stateTxn`-shaped diff --git a/context/cli/opencode-mutation-scope-adapter-lifecycle.md b/context/cli/opencode-mutation-scope-adapter-lifecycle.md index 8fd03e27e..5c30c3ca3 100644 --- a/context/cli/opencode-mutation-scope-adapter-lifecycle.md +++ b/context/cli/opencode-mutation-scope-adapter-lifecycle.md @@ -132,8 +132,8 @@ it changes none of the ordinary hook lifecycle above and none of `mutation_scope_owner::is_definitely_dead` proves dead; otherwise `ManualOnly` (no owner recorded, a live owner, or unprovable liveness). - `repair_blocked(git_dir, repository_root, logger, seam)` acquires the - `AdapterBoundaryLock`, normalizes an orphaned `Flushing`, then — in one - `AdapterStateLock` transaction — re-reads state fresh and re-evaluates + adapter-boundary lock (`state::BOUNDARY_LOCK`), normalizes an orphaned `Flushing`, then — in one + adapter-state lock (`state::STATE_LOCK`) transaction — re-reads state fresh and re-evaluates liveness against the *current* state rather than trusting any earlier read. Only attempts still `PendingStart` with a still-dead owner at that moment transition to `PendingAbandon`, using the same durable diff --git a/context/context-map.md b/context/context-map.md index 3b65cc836..4287d2f3f 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -114,7 +114,7 @@ Additional mutation-scope integration context: - `context/cli/codex-mutation-scope-health.md` (the `doctor-mutation-scope-health` plan's T03: `classify_health` in `cli/src/services/hooks/codex_mutation_scope/health.rs` maps the adapter's `RecoveryState`/attempt-state combinations onto `healthy | recovering | blocked | invalid`, proven by driving the real dispatch/recovery functions rather than reading enum names — `Pending` with attempts empty, `Pending` with unresolved attempts, and an orphaned `Flushing` are all `recovering`: the production `PreToolUse` order runs `sweep_stale_lane_predecessors` before the global `RecoveryBlocked` gate, so a same-`(session_id, turn_id)`-lane successor can retry and clear a stuck attempt and self-admit through a flush, even though an unrelated session/lane still denies fail-closed in the meantime; `Flushing` with unresolved attempts and any read/parse failure are `invalid`) - `context/sce/codex-apply-patch-diff-runtime.md` (the complementary Codex `PostToolUse(apply_patch)` parsing, path containment, normalization, and `diff_traces` evidence contract) - `context/cli/opencode-mutation-scope-adapter-lifecycle.md` (the T04 OpenCode adapter runtime detail split out of the integration doc: attempt phases `PendingStart` → `Active` → `PendingAbandon`, per-`git-dir` boundary lock, durable `/sce/` state, write-ahead fail-closed `Start`, `Close` on successful `ToolExecuteAfter`, exact-`ToolError`-only `Abandon` with durable terminal intent + ambiguity-consuming `flush` + generation-tracked recovery barrier that retries transient cleanup failures, non-authoritative broad async events, no same-session sweep, no TTL, no protocol/Quint change) -- `context/cli/opencode-mutation-scope-integration.md` (the third concrete harness producer, reachable by a real OpenCode session as of the `opencode-mutation-scope-integration` plan's T05 and now proven end-to-end through real Git/DB production-path regressions as of T06: OpenCode tool lifecycle frozen (T01) against `opencode-ai@1.15.4` / `@opencode-ai/plugin@1.15.4` (upstream `v1.15.4`), with the probe fixtures/report under `cli/src/services/hooks/opencode_mutation_scope/fixtures/`; `(sessionID, callID)` scope identity, `bash`/`write`/`edit`/`apply_patch` tracked with the `gpt-`-model patch gate making `edit`/`write` vs `apply_patch` mutually exclusive per session, `task` delegation, MCP/plugin/unknown tools untracked by explicit allowlist; `bash` Start on `shell.env` (post-permission, pre-spawn), file-tool Start write-ahead on `tool.execute.before`, Close on successful `tool.execute.after` (fires for non-zero exit / 127 / timeout, not for rejection / interrupt / validation failure); OpenCode scopes confirmation-required like Codex (shipped in T02 as the generic `requires_boundary_confirmation` predicate); `chat.params` model provenance keyed by `sessionID` else `NULL`; sequential plugin dispatch in explicit-`plugin`-array order with fail-closed `tool.execute.before` / `shell.env` barriers (Probes A/B/C PROVEN); SIGINT/SIGKILL leave no terminal hook and orphan child processes so no TTL is safe; OpenCode persistence is global-user-scoped not checkout-local; T03 added `cli/src/services/hooks/opencode_mutation_scope/mod.rs` (strict wire-event parsing, `classify_tool`, `AttemptKey`, frozen `oc-tool-v1|s=:|c=:` `ScopeId` with no attempt-seq, `oc_` provenance) and the hidden `sce hooks opencode-mutation-scope` command; T04 added the full lifecycle (`state.rs` durable checkout-local attempt state under `/sce/` with attempt phases `PendingStart` → `Active` → `PendingAbandon`, `os_lock.rs`/`boundary_lock.rs`, generation-tracked recovery barrier, write-ahead fail-closed `Start`, `Close`, `Abandon` on exact `ToolError` `(session_id, call_id)` evidence only — broad async `SessionIdle`/`SessionError`/`SessionDeleted`/`ServerDisposed` events retire nothing (D10/D11); `ToolError` first persists the doomed attempt as `PendingAbandon` **and** the recovery generation in one write before any seam call (`begin_terminal_cleanup`), then `resolve_recovery` drives an ineligible `flush` (while doomed + sibling scopes still resolve it `IneligibleUnscoped`), then the generic `abandon`, then removes the attempt only on `abandon` success, then a second `flush` for the rebaseline; a transient failure of any step relinquishes recovery to `Pending` and leaves the attempt `PendingAbandon` so the next recovery-capable boundary (a new tracked `Start` claims the flush; a duplicate `ToolError` retries) replays the whole idempotent sequence rather than forgetting the scope; a replayed `Start` for a `PendingAbandon` identity fails closed and never returns it to `Active`; so a concurrent `Abandon(A)`→`Close(B)` can never yield `AiExclusive(B)` over A's interval while B keeps its later intervals; the ambiguity flush runs alongside live siblings, not deferred to `attempts.is_empty()`; `ToolError` carries `tool_name` so untracked/delegation errors are zero-footprint; no same-session sweep, no TTL; no protocol/Quint change — the generic `flush`/`abandon`/confirmation-required semantics already suffice; the full T04 recovery detail now lives in `context/cli/opencode-mutation-scope-adapter-lifecycle.md`) driving the generic in-process ingress seam; T05 added the generated `sce-mutation-scope.ts` transport plugin (`config/lib/mutation-scope-plugin/`, emitted by `config/pkl/generate.pkl`, registered last in `config/pkl/renderers/common.pkl`), installed as the final OpenCode plugin by the `config_merge` append after arbitrary user plugins, with `sce doctor`'s `inspect_opencode_plugin_ordering_health` flagging a non-last position; the plugin maps `write`/`edit`/`apply_patch` `tool.execute.before` → fail-closed `ToolExecuteBefore`, `shell.env` → fail-closed `ShellEnv` bash Start, `tool.execute.after` → best-effort Close, tool-part `error` → best-effort `ToolError`, observes the model per turn from `chat.params` (`providerID/api.id`, ignoring the `title` agent, replacing rather than merging the cached per-session model so a later turn with no valid model clears it instead of reusing stale evidence), throws on any failure to establish a tracked Start (non-zero adapter exit, spawn failure, timeout, or missing `sce` CLI) so every transport failure fails closed; the broad async `SessionIdle`/`SessionError`/`ServerDisposed` events are not forwarded at all (T04's dispatch is a no-op for them) and `session.deleted` only clears the plugin's local model cache — full plugin-transport/model-provenance detail split into [`opencode-mutation-scope-plugin-transport.md`](cli/opencode-mutation-scope-plugin-transport.md); T06 added real Git/DB production-path regressions in `cli/src/services/hooks/mod.rs` (`services::hooks::tests::mutation_provenance_e2e`) proving tracked-tool success, model-present/model-missing provenance, task/unknown-tool zero-footprint, concurrent reject-and-confirm, and OpenCode+Codex/Claude overlap down to `mutation_ai_patch` and Agent Trace output; a live `apply_patch` fixture against a real credentialed OpenCode CLI session remains outstanding for `/validate` (a credential gap, not a soundness gap)) +- `context/cli/opencode-mutation-scope-integration.md` (the third concrete harness producer, reachable by a real OpenCode session as of the `opencode-mutation-scope-integration` plan's T05 and now proven end-to-end through real Git/DB production-path regressions as of T06: OpenCode tool lifecycle frozen (T01) against `opencode-ai@1.15.4` / `@opencode-ai/plugin@1.15.4` (upstream `v1.15.4`), with the probe fixtures/report under `cli/src/services/hooks/opencode_mutation_scope/fixtures/`; `(sessionID, callID)` scope identity, `bash`/`write`/`edit`/`apply_patch` tracked with the `gpt-`-model patch gate making `edit`/`write` vs `apply_patch` mutually exclusive per session, `task` delegation, MCP/plugin/unknown tools untracked by explicit allowlist; `bash` Start on `shell.env` (post-permission, pre-spawn), file-tool Start write-ahead on `tool.execute.before`, Close on successful `tool.execute.after` (fires for non-zero exit / 127 / timeout, not for rejection / interrupt / validation failure); OpenCode scopes confirmation-required like Codex (shipped in T02 as the generic `requires_boundary_confirmation` predicate); `chat.params` model provenance keyed by `sessionID` else `NULL`; sequential plugin dispatch in explicit-`plugin`-array order with fail-closed `tool.execute.before` / `shell.env` barriers (Probes A/B/C PROVEN); SIGINT/SIGKILL leave no terminal hook and orphan child processes so no TTL is safe; OpenCode persistence is global-user-scoped not checkout-local; T03 added `cli/src/services/hooks/opencode_mutation_scope/mod.rs` (strict wire-event parsing, `classify_tool`, `AttemptKey`, frozen `oc-tool-v1|s=:|c=:` `ScopeId` with no attempt-seq, `oc_` provenance) and the hidden `sce hooks opencode-mutation-scope` command; T04 added the full lifecycle (`state.rs` durable checkout-local attempt state under `/sce/` with attempt phases `PendingStart` → `Active` → `PendingAbandon`, adapter state/boundary locks (now `AdapterLockSpec` identities over the shared `hooks/mutation_scope_lock.rs` OS advisory lock), generation-tracked recovery barrier, write-ahead fail-closed `Start`, `Close`, `Abandon` on exact `ToolError` `(session_id, call_id)` evidence only — broad async `SessionIdle`/`SessionError`/`SessionDeleted`/`ServerDisposed` events retire nothing (D10/D11); `ToolError` first persists the doomed attempt as `PendingAbandon` **and** the recovery generation in one write before any seam call (`begin_terminal_cleanup`), then `resolve_recovery` drives an ineligible `flush` (while doomed + sibling scopes still resolve it `IneligibleUnscoped`), then the generic `abandon`, then removes the attempt only on `abandon` success, then a second `flush` for the rebaseline; a transient failure of any step relinquishes recovery to `Pending` and leaves the attempt `PendingAbandon` so the next recovery-capable boundary (a new tracked `Start` claims the flush; a duplicate `ToolError` retries) replays the whole idempotent sequence rather than forgetting the scope; a replayed `Start` for a `PendingAbandon` identity fails closed and never returns it to `Active`; so a concurrent `Abandon(A)`→`Close(B)` can never yield `AiExclusive(B)` over A's interval while B keeps its later intervals; the ambiguity flush runs alongside live siblings, not deferred to `attempts.is_empty()`; `ToolError` carries `tool_name` so untracked/delegation errors are zero-footprint; no same-session sweep, no TTL; no protocol/Quint change — the generic `flush`/`abandon`/confirmation-required semantics already suffice; the full T04 recovery detail now lives in `context/cli/opencode-mutation-scope-adapter-lifecycle.md`) driving the generic in-process ingress seam; T05 added the generated `sce-mutation-scope.ts` transport plugin (`config/lib/mutation-scope-plugin/`, emitted by `config/pkl/generate.pkl`, registered last in `config/pkl/renderers/common.pkl`), installed as the final OpenCode plugin by the `config_merge` append after arbitrary user plugins, with `sce doctor`'s `inspect_opencode_plugin_ordering_health` flagging a non-last position; the plugin maps `write`/`edit`/`apply_patch` `tool.execute.before` → fail-closed `ToolExecuteBefore`, `shell.env` → fail-closed `ShellEnv` bash Start, `tool.execute.after` → best-effort Close, tool-part `error` → best-effort `ToolError`, observes the model per turn from `chat.params` (`providerID/api.id`, ignoring the `title` agent, replacing rather than merging the cached per-session model so a later turn with no valid model clears it instead of reusing stale evidence), throws on any failure to establish a tracked Start (non-zero adapter exit, spawn failure, timeout, or missing `sce` CLI) so every transport failure fails closed; the broad async `SessionIdle`/`SessionError`/`ServerDisposed` events are not forwarded at all (T04's dispatch is a no-op for them) and `session.deleted` only clears the plugin's local model cache — full plugin-transport/model-provenance detail split into [`opencode-mutation-scope-plugin-transport.md`](cli/opencode-mutation-scope-plugin-transport.md); T06 added real Git/DB production-path regressions in `cli/src/services/hooks/mod.rs` (`services::hooks::tests::mutation_provenance_e2e`) proving tracked-tool success, model-present/model-missing provenance, task/unknown-tool zero-footprint, concurrent reject-and-confirm, and OpenCode+Codex/Claude overlap down to `mutation_ai_patch` and Agent Trace output; a live `apply_patch` fixture against a real credentialed OpenCode CLI session remains outstanding for `/validate` (a credential gap, not a soundness gap)) - `context/cli/opencode-mutation-scope-plugin-transport.md` (the T05 generated `sce-mutation-scope.ts` plugin's model-provenance observation from per-session `chat.params` and its sequential, last-registered position in OpenCode's plugin ordering — detail split out of `opencode-mutation-scope-integration.md` for the repository's per-file line budget) - `context/cli/opencode-mutation-scope-health.md` (the `doctor-mutation-scope-health` plan's T04: `classify_health` in `cli/src/services/hooks/opencode_mutation_scope/health.rs` maps the adapter's `RecoveryState`/`AttemptPhase` combinations onto `healthy | recovering | blocked | invalid`, proven by driving the real dispatch/recovery functions — `Clear` + `PendingAbandon` is `invalid`; any state containing a `PendingStart` attempt is `blocked` (except that `Clear` + `PendingAbandon` retains `invalid` precedence), because no boundary sweeps a stale `PendingStart` on behalf of an unrelated call, unlike Pi's dead-owner sweep or Codex's same-lane retry, so a crashed owning process leaves it a permanent wedge for new admissions even while a `Pending`/`Flushing` recovery for another attempt is also outstanding, since `resolve_recovery` never retires `PendingStart`; `Pending`/`Flushing` with no `PendingStart` outstanding is `recovering`; `Clear` with ordinary `Active`/no attempts is `healthy`; any read/parse failure is `invalid`) - `context/cli/pi-mutation-scope-integration.md` (the fourth concrete harness adapter, `cli/src/services/hooks/pi_mutation_scope/`, hidden command `sce hooks pi-mutation-scope`, driven by the canonical generated extension in ordinary Pi sessions: Pi's `bash`/`edit`/`write` tracked-tool allowlist with `tool_call` as the single universal pre-execution gate for every tool including `bash`; the `pi-tool-v1|n=|s=:|c=:` `ScopeId` whose checkout-local monotonic attempt sequence stops a reused `toolCallId` from ever reactivating a terminal scope; the `PendingStart`→`Executed`→`Closed`/`PendingAbandon` attempt-phase machine with no `Active` phase, because `PendingStart` is Pi's normal resting state for an entire in-flight execution (`tool_execution_start` fires unconditionally before the fail-closed `tool_call` gate and proves nothing; `tool_result` is the sole `Executed` evidence, and `tool_execution_end` with no preceding `tool_result` abandons rather than closes); why admission's fail-closed "uncertain attempt" check covers only a lingering `PendingAbandon` or non-`Clear` recovery state and never a sibling's `PendingStart`, unlike the other three adapters, so concurrent Pi tool calls stay distinct live scopes; a positive-process-death-only stale-`PendingStart`/`Executed` recovery (`getppid()`-captured owner plus Linux `/proc`-start-time PID-reuse proofing, no TTL/sweep), run on every tracked Start admission by independently inspecting persisted attempts rather than only ones matching the incoming key, and adapter reconciliation with the external-mutation guard's forced recovery, both added by a later task in this same plan; and the harness-neutral `external-mutation-guard` mechanism this task also built for Pi's `user_bash`, detailed separately in `context/cli/mutation-trace-external-mutation-guard.md`)