diff --git a/cli/Cargo.lock b/cli/Cargo.lock index ada08e1cd..186c4fdd3 100644 --- a/cli/Cargo.lock +++ b/cli/Cargo.lock @@ -200,7 +200,7 @@ checksum = "d64a66d21a80182b35b1741997a6d2456911f54b7eb1918aa4e2382fc205268d" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -302,7 +302,7 @@ checksum = "3b43422f69d8ff38f95f1b2bb76517c91589a924d1559a0e935d7c8ce0274c11" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -337,7 +337,7 @@ checksum = "9035ad2d096bed7955a320ee7e2230574d28fd3c3a0f186cbea1ff3c7eed5dbb" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -413,7 +413,7 @@ dependencies = [ "regex", "rustc-hash 1.1.0", "shlex 1.3.0", - "syn", + "syn 2.0.118", "which", ] @@ -527,7 +527,7 @@ dependencies = [ "proc-macro2", "quote", "rustversion", - "syn", + "syn 2.0.118", ] [[package]] @@ -583,7 +583,7 @@ checksum = "f9abbd1bc6865053c427f7198e6af43bfdedc55ab791faed4fbd361d789575ff" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -727,7 +727,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -989,7 +989,7 @@ dependencies = [ "proc-macro2", "quote", "strsim", - "syn", + "syn 2.0.118", ] [[package]] @@ -1000,7 +1000,7 @@ checksum = "ac3984ec7bd6cfa798e62b4a642426a5be0e68f9401cfc2a01e3fa9ea2fcdb8d" dependencies = [ "darling_core", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -1055,7 +1055,7 @@ dependencies = [ "defmt-parser", "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -1095,7 +1095,7 @@ dependencies = [ "proc-macro2", "quote", "rustc_version", - "syn", + "syn 2.0.118", ] [[package]] @@ -1150,7 +1150,7 @@ checksum = "1ac70aa55017e108007fbaf5aa0f54b021c98f92ff8af59d42eda9da96e3dd4f" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -1225,7 +1225,7 @@ checksum = "67c78a4d8fdf9953a5c9d458f9efe940fd97a0cab0941c075a813ac594733827" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -1449,7 +1449,7 @@ checksum = "e835b70203e41293343137df5c0664546da5745f82ec9b84d40be8336958447b" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -2168,7 +2168,7 @@ dependencies = [ "jiff-core", "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -2213,7 +2213,7 @@ dependencies = [ "quote", "rustc_version", "simd_cesu8", - "syn", + "syn 2.0.118", ] [[package]] @@ -2232,7 +2232,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "38c0b942f458fe50cdac086d2f946512305e5631e720728f2a61aabcd47a6264" dependencies = [ "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -2378,7 +2378,7 @@ checksum = "32d59e20403c7d08fe62b4376edfe5c7fb2ef1e6b1465379686d0f21c8df444b" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -2521,7 +2521,7 @@ checksum = "db5b29714e950dbb20d5e6f74f9dcec4edbcc1067bb7f8ed198c097b8c1a818b" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -2915,7 +2915,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "479ca8adacdd7ce8f1fb39ce9ecccbfe93a3f1344b3d0d97f20bc0196208f62b" dependencies = [ "proc-macro2", - "syn", + "syn 2.0.118", ] [[package]] @@ -2956,7 +2956,7 @@ dependencies = [ "itertools 0.14.0", "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -3041,7 +3041,7 @@ checksum = "814c6e076fa655b78d7127fda492f3fa980b8827fda16c33d891b57c8ccd735f" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -3205,7 +3205,7 @@ checksum = "b7186006dcb21920990093f30e3dea63b7d6e977bf1256be20c3563a5db070da" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -3602,7 +3602,7 @@ checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -3626,7 +3626,7 @@ checksum = "175ee3e80ae9982737ca543e96133087cbd9a485eecc3bc4de9c1a37b47ea59c" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -3680,7 +3680,7 @@ dependencies = [ "darling", "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -3927,7 +3927,7 @@ dependencies = [ "proc-macro2", "quote", "rustversion", - "syn", + "syn 2.0.118", ] [[package]] @@ -3972,6 +3972,17 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "syn" +version = "3.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8593e8e72159ed2257d083c7a454a85cbf854f37a0966d8d483aff8c8a3ebcee" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + [[package]] name = "sync_wrapper" version = "1.0.2" @@ -3989,7 +4000,7 @@ checksum = "728a70f3dbaf5bab7f0c4b1ac8d7ae5ea60a4b5549c8a5914361c99147a709d2" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -4176,7 +4187,7 @@ checksum = "ebc4ee7f67670e9b64d05fa4253e753e016c6c95ff35b89b7941d6b856dec1d5" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -4256,10 +4267,22 @@ dependencies = [ "pin-project-lite", "signal-hook-registry", "socket2", + "tokio-macros", "tracing", "windows-sys 0.61.2", ] +[[package]] +name = "tokio-macros" +version = "2.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "78773a2a397f451582ce068015985c33193cf6dea8b74d2a639fe457b2f07b0e" +dependencies = [ + "proc-macro2", + "quote", + "syn 3.0.6", +] + [[package]] name = "tokio-rustls" version = "0.26.4" @@ -4420,7 +4443,7 @@ checksum = "7490cfa5ec963746568740651ac6781f701c9c5ea257c58e057f3ba8cf69e8da" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -4577,7 +4600,7 @@ checksum = "63bd5f0f7186707a8b4cf54bbaddff9056c218823befab15508ef6e7502c2340" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -4620,7 +4643,7 @@ checksum = "592f57f212b0ce58c876ab760ef84b219719ee2ade738b53f180db87c0232432" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -4707,7 +4730,7 @@ checksum = "cf808357c6ed7e13ba0f3277ec8d8f21b2d501274895104263985330c726c1c5" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -4935,7 +4958,7 @@ dependencies = [ "bumpalo", "proc-macro2", "quote", - "syn", + "syn 2.0.118", "wasm-bindgen-shared", ] @@ -5041,7 +5064,7 @@ checksum = "053e2e040ab57b9dc951b72c264860db7eb3b0200ba345b4e4c3b14f67855ddf" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -5052,7 +5075,7 @@ checksum = "3f316c4a2570ba26bbec722032c4099d8c8bc095efccdc15688708623367e358" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -5318,7 +5341,7 @@ checksum = "de844c262c8848816172cef550288e7dc6c7b7814b4ee56b3e1553f275f1858e" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", "synstructure", ] @@ -5378,7 +5401,7 @@ dependencies = [ "proc-macro-crate", "proc-macro2", "quote", - "syn", + "syn 2.0.118", "zbus_names", "zvariant", "zvariant_utils", @@ -5412,7 +5435,7 @@ checksum = "1ae7f38b72ec2a254e2b87ef277cf2cd4fb97cbebf944faa6f33354da0867930" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -5432,7 +5455,7 @@ checksum = "11532158c46691caf0f2593ea8358fed6bbf68a0315e80aae9bd41fbade684a1" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", "synstructure", ] @@ -5474,7 +5497,7 @@ checksum = "625dc425cab0dca6dc3c3319506e6593dcb08a9f387ea3b284dbd52a92c40555" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -5534,7 +5557,7 @@ dependencies = [ "proc-macro-crate", "proc-macro2", "quote", - "syn", + "syn 2.0.118", "zvariant_utils", ] @@ -5547,6 +5570,6 @@ dependencies = [ "proc-macro2", "quote", "serde", - "syn", + "syn 2.0.118", "winnow 1.0.3", ] diff --git a/cli/Cargo.toml b/cli/Cargo.toml index f04b8b750..61826d60b 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -43,7 +43,7 @@ reqwest = { version = "0.13", default-features = false, features = ["json", "for serde = { version = "1", features = ["derive"] } serde_json = "1" sha2 = "0.11" -tokio = { version = "1", default-features = false, features = ["rt", "io-util", "sync", "time"] } +tokio = { version = "1", default-features = false, features = ["rt", "io-util", "sync", "time", "macros", "rt-multi-thread"] } toml = "0.9" tracing = "0.1" diff --git a/cli/src/app.rs b/cli/src/app.rs index 5e931c052..580be1507 100644 --- a/cli/src/app.rs +++ b/cli/src/app.rs @@ -29,9 +29,6 @@ struct AppRuntime { /// Lightweight borrowed view of the CLI runtime dependencies. /// /// `AppContext` does **not** own its dependencies; it borrows them from `AppRuntime`. -/// It is intended for short-lived, command-scoped use and should not be stored -/// long-term (e.g., in structs or across await points) because its lifetime is -/// tied to the owning `AppRuntime`. pub struct AppContext< 'a, L: LoggerTrait = services::observability::Logger, @@ -239,24 +236,24 @@ impl AppRuntime { } } -pub fn run(args: I) -> ExitCode +pub async fn run(args: I) -> ExitCode where I: IntoIterator, { - run_with_dependency_check(args, || Ok(())) + run_with_dependency_check(args, || Ok(())).await } -fn run_with_dependency_check(args: I, dependency_check: F) -> ExitCode +async fn run_with_dependency_check(args: I, dependency_check: F) -> ExitCode where I: IntoIterator, F: FnOnce() -> anyhow::Result<()>, { let mut stdout = io::stdout(); let mut stderr = io::stderr(); - run_with_dependency_check_and_streams(args, dependency_check, &mut stdout, &mut stderr) + run_with_dependency_check_and_streams(args, dependency_check, &mut stdout, &mut stderr).await } -fn run_with_dependency_check_and_streams( +async fn run_with_dependency_check_and_streams( args: I, dependency_check: F, stdout: &mut StdoutW, @@ -269,13 +266,13 @@ where StderrW: Write, { app_support::render_run_outcome( - try_run_with_dependency_check(args, dependency_check, stderr), + try_run_with_dependency_check(args, dependency_check, stderr).await, stdout, stderr, ) } -fn try_run_with_dependency_check( +async fn try_run_with_dependency_check( args: I, dependency_check: F, stderr: &mut StderrW, @@ -287,19 +284,18 @@ where { let result = perform_dependency_check(dependency_check) .and_then(|()| build_startup_context()) - .and_then(initialize_runtime) - .map(|runtime| { + .and_then(initialize_runtime); + + match result { + Ok(runtime) => { let startup_diagnostic = runtime.startup_diagnostic.clone(); - let result = run_command_lifecycle(args, &runtime, stderr); + let result = run_command_lifecycle(args, &runtime, stderr).await; RunOutcome { logger: Some(runtime.logger), startup_diagnostic, result, } - }); - - match result { - Ok(outcome) => outcome, + } Err(error) => RunOutcome { result: Err(error), logger: None, @@ -349,7 +345,7 @@ fn initialize_runtime(startup: StartupContext) -> Result { }) } -fn run_command_lifecycle( +async fn run_command_lifecycle( args: I, runtime: &AppRuntime, stderr: &mut StderrW, @@ -359,22 +355,44 @@ where StderrW: Write, { let context = runtime.context(); + run_command_lifecycle_with_context(args, &runtime.registry, &context, stderr).await +} + +async fn run_command_lifecycle_with_context( + args: I, + registry: &services::command_registry::CommandRegistry, + context: &AppContext<'_, L, T>, + stderr: &mut StderrW, +) -> Result +where + I: IntoIterator, + L: LoggerTrait, + T: TelemetryTrait, + StderrW: Write, +{ let mut args = Some(args.into_iter().collect::>()); - context.telemetry().with_default_subscriber(&mut || { - context.logger().info( - "sce.app.start", - "Starting command dispatch", - &[("component", services::observability::NAME)], - None, - ); - let Some(command_args) = args.take() else { - return Err(CliError::runtime(anyhow::Error::msg( - REPEATED_COMMAND_DISPATCH_ERROR, - ))); - }; - let command = parse_command_phase(command_args, &runtime.registry, &context)?; - app_support::execute_command_phase(&command, &context, stderr) - }) + let mut stderr = Some(stderr); + context + .telemetry() + .with_default_subscriber(&mut || { + let command_inputs = args.take().zip(stderr.take()); + async move { + context.logger().info( + "sce.app.start", + "Starting command dispatch", + &[("component", services::observability::NAME)], + None, + ); + let Some((command_args, stderr)) = command_inputs else { + return Err(CliError::runtime(anyhow::Error::msg( + REPEATED_COMMAND_DISPATCH_ERROR, + ))); + }; + let command = parse_command_phase(command_args, registry, context)?; + app_support::execute_command_phase(&command, context, stderr).await + } + }) + .await } fn parse_command_phase( @@ -396,3 +414,306 @@ where ); Ok(command) } + +#[cfg(test)] +mod tests { + use std::future::{poll_fn, Future}; + use std::sync::{Arc, Mutex}; + + use super::*; + + #[derive(Default)] + struct RecordingScope { + active: bool, + polls: usize, + events: Vec, + } + + struct RecordingLogger(Arc>); + + impl LoggerTrait for RecordingLogger { + fn info(&self, event: &str, _: &str, _: &[(&str, &str)], _: Option<&str>) { + let mut scope = self.0.lock().unwrap(); + assert!( + scope.active, + "dispatch logging must occur inside a telemetry poll" + ); + scope.events.push(event.to_string()); + } + fn debug( + &self, + event: &str, + message: &str, + fields: &[(&str, &str)], + session: Option<&str>, + ) { + self.info(event, message, fields, session); + } + fn warn(&self, _: &str, _: &str, _: &[(&str, &str)], _: Option<&str>) {} + fn error(&self, _: &str, _: &str, _: &[(&str, &str)], _: Option<&str>) {} + fn log_cli_error(&self, _: &CliError, _: Option<&str>) {} + } + + struct RecordingTelemetry { + scope: Arc>, + repeat: bool, + } + + impl RecordingTelemetry { + async fn poll_scoped(&self, future: F) -> F::Output { + let mut future = std::pin::pin!(future); + poll_fn(|cx| { + { + let mut scope = self.scope.lock().unwrap(); + assert!(!scope.active); + scope.active = true; + scope.polls += 1; + scope.events.push("telemetry.poll.start".into()); + } + let result = future.as_mut().poll(cx); + let mut scope = self.scope.lock().unwrap(); + scope.events.push("telemetry.poll.end".into()); + scope.active = false; + result + }) + .await + } + } + + impl TelemetryTrait for RecordingTelemetry { + async fn with_default_subscriber(&self, action: &mut F) -> Result + where + F: FnMut() -> Fut, + Fut: Future>, + { + let result = self.poll_scoped(action()).await; + if self.repeat { + assert!(result.is_ok()); + self.poll_scoped(action()).await + } else { + result + } + } + } + + #[tokio::test(flavor = "multi_thread")] + async fn telemetry_scopes_every_poll_of_yielding_work() { + let scope = Arc::new(Mutex::new(RecordingScope::default())); + let telemetry = RecordingTelemetry { + scope: scope.clone(), + repeat: false, + }; + let result = telemetry + .with_default_subscriber(&mut || async { + NoopTelemetry + .with_default_subscriber(&mut || async { + assert!(scope.lock().unwrap().active); + tokio::task::yield_now().await; + assert!(scope.lock().unwrap().active); + assert_eq!( + tokio::runtime::Handle::current().runtime_flavor(), + tokio::runtime::RuntimeFlavor::MultiThread + ); + Ok("finished".into()) + }) + .await + }) + .await + .unwrap(); + assert_eq!(result, "finished"); + let scope = scope.lock().unwrap(); + assert!(scope.polls >= 2); + assert!(!scope.active); + assert_eq!(scope.events.len(), scope.polls * 2); + } + + #[tokio::test(flavor = "multi_thread")] + async fn lifecycle_telemetry_preserves_events_errors_and_repeat_guard() { + for (args, repeat, expected_events, expected_error) in [ + ( + vec!["sce", "help"], + false, + vec![ + "sce.app.start", + "sce.command.raw_args", + "sce.command.parsed", + "sce.command.dispatch_start", + "sce.command.dispatch_end", + "sce.command.completed", + ], + None, + ), + ( + vec!["sce", "--invalid-boundary-option"], + false, + vec!["sce.app.start", "sce.command.raw_args"], + Some(services::error::FailureClass::Parse), + ), + ( + vec![ + "sce", + "config", + "validate", + "--config", + "/nonexistent-sce-boundary-config.json", + ], + false, + vec![ + "sce.app.start", + "sce.command.raw_args", + "sce.command.parsed", + "sce.command.dispatch_start", + ], + Some(services::error::FailureClass::Runtime), + ), + ( + vec!["sce", "help"], + true, + vec![ + "sce.app.start", + "sce.command.raw_args", + "sce.command.parsed", + "sce.command.dispatch_start", + "sce.command.dispatch_end", + "sce.command.completed", + "sce.app.start", + ], + Some(services::error::FailureClass::Runtime), + ), + ] { + let scope = Arc::new(Mutex::new(RecordingScope::default())); + let logger = RecordingLogger(scope.clone()); + let telemetry = RecordingTelemetry { + scope: scope.clone(), + repeat, + }; + let fs = services::capabilities::StdFsOps; + let git = services::capabilities::ProcessGitOps; + let context = AppContext::new(&logger, &telemetry, &fs, &git, None); + let result = run_command_lifecycle_with_context( + args.into_iter().map(String::from), + &services::command_registry::CommandRegistry::default(), + &context, + &mut Vec::new(), + ) + .await; + if let Some(class) = expected_error { + let error = result.unwrap_err(); + assert_eq!(error.class(), class); + if repeat { + assert_eq!(error.to_string(), REPEATED_COMMAND_DISPATCH_ERROR); + } + } else { + assert_eq!(result.unwrap(), services::help::help_text()); + } + let scope = scope.lock().unwrap(); + assert!(!scope.active); + let events = scope + .events + .iter() + .filter(|event| event.starts_with("sce.")) + .map(String::as_str) + .collect::>(); + assert_eq!(events, expected_events); + } + } + + #[test] + fn app_output_boundary_uses_isolated_process() { + let sandbox = tempfile::tempdir().unwrap(); + let output = std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "app::tests::isolated_app_output_boundary", + "--nocapture", + ]) + .current_dir(sandbox.path()) + .env("SCE_APP_BOUNDARY_CHILD", "1") + .env("HOME", sandbox.path()) + .env("XDG_CONFIG_HOME", sandbox.path().join("config")) + .env("XDG_STATE_HOME", sandbox.path().join("state")) + .env("XDG_CACHE_HOME", sandbox.path().join("cache")) + .env("NO_COLOR", "1") + .env_remove("SCE_CONFIG_FILE") + .output() + .unwrap(); + assert!( + output.status.success(), + "{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + } + + #[tokio::test(flavor = "multi_thread")] + async fn isolated_app_output_boundary() { + if std::env::var_os("SCE_APP_BOUNDARY_CHILD").is_none() { + return; + } + for command in ["help", "unknown-boundary-command", "version"] { + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + let code = run_with_dependency_check_and_streams( + ["sce", command].map(String::from), + || { + assert_eq!( + tokio::runtime::Handle::current().runtime_flavor(), + tokio::runtime::RuntimeFlavor::MultiThread + ); + Ok(()) + }, + &mut stdout, + &mut stderr, + ) + .await; + assert_eq!(code, ExitCode::SUCCESS); + let expected = if command == "version" { + format!( + "shared-context-engineering {} ({})\n", + services::version::PACKAGE_VERSION, + option_env!("SCE_GIT_COMMIT").unwrap_or("unknown") + ) + } else { + format!("{}\n", services::help::help_text()) + }; + assert_eq!(stdout, expected.as_bytes()); + assert_eq!(stderr, b""); + } + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + let code = run_with_dependency_check_and_streams( + ["sce", "help"].map(String::from), + || anyhow::bail!("boundary dependency unavailable"), + &mut stdout, + &mut stderr, + ) + .await; + assert_eq!(code, ExitCode::from(5)); + assert_eq!(stdout, b""); + assert_eq!(stderr, b"Error [SCE-ERR-DEPENDENCY]: Failed to initialize dependency checks: boundary dependency unavailable Try: verify required runtime dependencies and environment setup, then retry.\n"); + + stdout.clear(); + stderr.clear(); + let missing_config = std::env::current_dir().unwrap().join("missing-config.json"); + let code = run_with_dependency_check_and_streams( + [ + "sce", + "config", + "validate", + "--config", + missing_config.to_str().unwrap(), + ] + .map(String::from), + || Ok(()), + &mut stdout, + &mut stderr, + ) + .await; + assert_eq!(code, ExitCode::from(4)); + assert_eq!(stdout, b""); + assert_eq!( + stderr, + b"An unexpected error occurred. Check the log files for more details.\n" + ); + } +} diff --git a/cli/src/main.rs b/cli/src/main.rs index 5f4ee707d..8ad8eac70 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -9,6 +9,7 @@ mod services; use std::process::ExitCode; -fn main() -> ExitCode { - app::run(std::env::args()) +#[tokio::main(flavor = "multi_thread")] +async fn main() -> ExitCode { + app::run(std::env::args()).await } diff --git a/cli/src/services/agent_trace_sync/control_plane.rs b/cli/src/services/agent_trace_sync/control_plane.rs index 11b5a1632..6ab29a341 100644 --- a/cli/src/services/agent_trace_sync/control_plane.rs +++ b/cli/src/services/agent_trace_sync/control_plane.rs @@ -1057,13 +1057,17 @@ mod tests { Ok(Some(valid_stored_tokens("valid-access-token"))) } - fn save(&self, _token: &TokenResponse) -> Result { - panic!("no token refresh expected in this test"); + fn save(&self, token: &TokenResponse) -> Result { + let runtime = tokio::runtime::Builder::new_current_thread() + .build() + .expect("build nested test runtime"); + runtime.block_on(async {}); + Ok(valid_stored_tokens(&token.access_token)) } } - #[test] - fn credential_store_operations_run_outside_the_async_runtime() { + #[tokio::test(flavor = "multi_thread")] + async fn credential_store_operations_run_outside_the_async_runtime() { let client = AuthenticatedControlPlaneClient::with_credential_store( test_http_client(), "http://127.0.0.1:1", @@ -1072,9 +1076,19 @@ mod tests { Box::new(RuntimeCheckingCredentialStore), ); - let access_token = block_on(client.resolve_access_token()).expect("token should load"); + let access_token = client + .resolve_access_token() + .await + .expect("token should load"); assert_eq!(access_token, "valid-access-token"); + let token: TokenResponse = serde_json::from_value(token_response_json("saved-token")) + .expect("fixture token should deserialize"); + let saved = client + .save_credentials(&token) + .await + .expect("token should save"); + assert_eq!(saved.access_token, "saved-token"); } /// A plain-`http://` loopback test double never needs TLS root diff --git a/cli/src/services/app_support.rs b/cli/src/services/app_support.rs index 418aa986d..494b22112 100644 --- a/cli/src/services/app_support.rs +++ b/cli/src/services/app_support.rs @@ -101,7 +101,7 @@ pub(crate) fn log_startup_configuration( } } -pub(crate) fn execute_command_phase( +pub(crate) async fn execute_command_phase( command: &RuntimeCommand, context: &C, stderr: &mut W, @@ -118,7 +118,7 @@ where &[("command", command_name.as_ref())], None, ); - let dispatch_result = command.execute_with_stderr(context, stderr); + let dispatch_result = command.execute_with_stderr(context, stderr).await; if dispatch_result.is_ok() { logger.debug( "sce.command.dispatch_end", diff --git a/cli/src/services/auth_command/command.rs b/cli/src/services/auth_command/command.rs index 5e7ac22a6..2ce5e0448 100644 --- a/cli/src/services/auth_command/command.rs +++ b/cli/src/services/auth_command/command.rs @@ -6,7 +6,7 @@ pub struct AuthCommand { } impl AuthCommand { - pub fn execute(&self, _context: &C) -> Result { - auth_command::run_auth_subcommand(self.request) + pub async fn execute(&self, _context: &C) -> Result { + auth_command::run_auth_subcommand(self.request).await } } diff --git a/cli/src/services/auth_command/mod.rs b/cli/src/services/auth_command/mod.rs index 7ac4d2b7a..007810d2b 100644 --- a/cli/src/services/auth_command/mod.rs +++ b/cli/src/services/auth_command/mod.rs @@ -1,7 +1,7 @@ pub mod command; +use std::future::Future; use std::io::Write; -use std::sync::OnceLock; use anyhow::{anyhow, Context, Result}; use serde_json::json; @@ -20,8 +20,6 @@ pub const NAME: &str = "auth"; pub type AuthFormat = OutputFormat; -static AUTH_RUNTIME: OnceLock = OnceLock::new(); - #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum AuthSubcommand { Login { format: AuthFormat }, @@ -34,54 +32,63 @@ pub struct AuthRequest { pub subcommand: AuthSubcommand, } -pub fn run_auth_subcommand(request: AuthRequest) -> Result { - run_auth_subcommand_with(request, run_login, run_logout, run_whoami) +pub async fn run_auth_subcommand(request: AuthRequest) -> Result { + run_auth_subcommand_with(request, run_login, run_logout, run_whoami).await } -fn run_auth_subcommand_with( +async fn run_auth_subcommand_with( request: AuthRequest, login: L, logout: O, whoami: S, ) -> Result where - L: FnOnce(AuthFormat) -> Result, - O: FnOnce(AuthFormat) -> Result, - S: FnOnce(AuthFormat) -> Result, + L: FnOnce(AuthFormat) -> LF, + LF: Future>, + O: FnOnce(AuthFormat) -> OF, + OF: Future>, + S: FnOnce(AuthFormat) -> SF, + SF: Future>, { match request.subcommand { - AuthSubcommand::Login { format } => login(format), - AuthSubcommand::Logout { format } => logout(format), - AuthSubcommand::Whoami { format } => whoami(format), + AuthSubcommand::Login { format } => login(format).await, + AuthSubcommand::Logout { format } => logout(format).await, + AuthSubcommand::Whoami { format } => whoami(format).await, } } -pub fn run_login(format: AuthFormat) -> Result { +pub async fn run_login(format: AuthFormat) -> Result { let client = reqwest::Client::new(); - let runtime = shared_runtime().map_err(unexpected_auth_command_error)?; let client_id = resolve_login_client_id().map_err(unexpected_auth_command_error)?; - let stored_tokens = token_storage::load_tokens().map_err(auth_storage_error)?; + let stored_tokens = run_credential_operation(token_storage::load_tokens).await?; + let client = &client; + let client_id = client_id.as_str(); run_login_with_stored_credentials( format, stored_tokens, - |stored_tokens| maybe_renew_stored_credentials(runtime, &client, &client_id, stored_tokens), - |format| match format { - AuthFormat::Text => run_text_login_with_runtime(runtime, &client, &client_id), - AuthFormat::Json => run_login_json(runtime, &client, &client_id, format), + |stored_tokens| async move { + maybe_renew_stored_credentials(client, client_id, stored_tokens).await + }, + |format| async move { + match format { + AuthFormat::Text => run_text_login(client, client_id).await, + AuthFormat::Json => run_login_json(client, client_id, format).await, + } }, ) + .await } -pub fn run_logout(format: AuthFormat) -> Result { - let deleted = token_storage::delete_tokens().map_err(auth_storage_error)?; +pub async fn run_logout(format: AuthFormat) -> Result { + let deleted = run_credential_operation(token_storage::delete_tokens).await?; render_logout_result(deleted, format).map_err(unexpected_auth_command_error) } -pub fn run_whoami(format: AuthFormat) -> Result { - if token_storage::load_tokens() - .map_err(auth_storage_error)? +pub async fn run_whoami(format: AuthFormat) -> Result { + if run_credential_operation(token_storage::load_tokens) + .await? .is_none() { return render_unauthenticated_whoami(format).map_err(unexpected_auth_command_error); @@ -98,92 +105,75 @@ pub fn run_whoami(format: AuthFormat) -> Result { auth::WORKOS_DEFAULT_BASE_URL, auth_config.workos_client_id.value.unwrap_or_default(), ); - let profile = shared_runtime() - .map_err(unexpected_auth_command_error)? - .block_on(client.me()) + let profile = client + .me() + .await .map_err(|error| map_whoami_control_plane_error(&error))?; render_whoami_result(&profile, format).map_err(unexpected_auth_command_error) } -fn shared_runtime() -> Result<&'static tokio::runtime::Runtime> { - if let Some(runtime) = AUTH_RUNTIME.get() { - return Ok(runtime); - } - - let runtime = tokio::runtime::Builder::new_current_thread() - .enable_io() - .enable_time() - .build() - .context("failed to create auth command runtime. Try: rerun the command; if the issue persists, verify the local Tokio runtime environment.")?; - - Ok(AUTH_RUNTIME.get_or_init(|| runtime)) -} - -fn maybe_renew_stored_credentials( - runtime: &tokio::runtime::Runtime, +async fn maybe_renew_stored_credentials( client: &reqwest::Client, client_id: &str, - stored_tokens: &StoredTokens, + stored_tokens: StoredTokens, ) -> Result, CliError> { - match runtime.block_on(auth::ensure_valid_token_returning_token( + match auth::ensure_valid_token_returning_token( client, auth::WORKOS_DEFAULT_BASE_URL, client_id, - stored_tokens, - )) { - Ok(token) => token_storage::save_tokens(&token) - .map(Some) - .map_err(auth_storage_error), + &stored_tokens, + ) + .await + { + Ok(token) => run_credential_operation(move || token_storage::save_tokens(&token)) + .await + .map(Some), Err(_) => Ok(None), } } -fn run_login_with_stored_credentials( +async fn run_login_with_stored_credentials( format: AuthFormat, stored_tokens: Option, renew: R, device_login: D, ) -> Result where - R: FnOnce(&StoredTokens) -> Result, CliError>, - D: FnOnce(AuthFormat) -> Result, + R: FnOnce(StoredTokens) -> RF, + RF: Future, CliError>>, + D: FnOnce(AuthFormat) -> DF, + DF: Future>, { if let Some(stored_tokens) = stored_tokens { - if let Some(renewed_tokens) = renew(&stored_tokens)? { + if let Some(renewed_tokens) = renew(stored_tokens).await? { return render_login_refresh_result(&renewed_tokens, format) .map_err(unexpected_auth_command_error); } } - device_login(format) + device_login(format).await } -fn run_text_login_with_runtime( - runtime: &tokio::runtime::Runtime, - client: &reqwest::Client, - client_id: &str, -) -> Result { - let authorization = runtime - .block_on(auth::request_device_authorization( - client, - auth::WORKOS_DEFAULT_BASE_URL, - client_id, - )) - .map_err(map_login_error)?; +async fn run_text_login(client: &reqwest::Client, client_id: &str) -> Result { + let authorization = + auth::request_device_authorization(client, auth::WORKOS_DEFAULT_BASE_URL, client_id) + .await + .map_err(map_login_error)?; write_login_prompt(&authorization).map_err(unexpected_auth_command_error)?; - let token = runtime - .block_on(auth::complete_device_auth_flow_returning_token( - client, - auth::WORKOS_DEFAULT_BASE_URL, - client_id, - &authorization, - )) - .map_err(map_login_error)?; + let token = auth::complete_device_auth_flow_returning_token( + client, + auth::WORKOS_DEFAULT_BASE_URL, + client_id, + &authorization, + ) + .await + .map_err(map_login_error)?; - let stored_tokens = token_storage::save_tokens(&token).map_err(auth_storage_error)?; + let stored_tokens = + run_credential_operation(move || token_storage::save_tokens(&token)).await?; render_login_result( &DeviceAuthFlowResult { @@ -195,30 +185,27 @@ fn run_text_login_with_runtime( .map_err(unexpected_auth_command_error) } -fn run_login_json( - runtime: &tokio::runtime::Runtime, +async fn run_login_json( client: &reqwest::Client, client_id: &str, format: AuthFormat, ) -> Result { - let authorization = runtime - .block_on(auth::request_device_authorization( - client, - auth::WORKOS_DEFAULT_BASE_URL, - client_id, - )) - .map_err(map_login_error)?; - - let token = runtime - .block_on(auth::complete_device_auth_flow_returning_token( - client, - auth::WORKOS_DEFAULT_BASE_URL, - client_id, - &authorization, - )) - .map_err(map_login_error)?; - - let stored_tokens = token_storage::save_tokens(&token).map_err(auth_storage_error)?; + let authorization = + auth::request_device_authorization(client, auth::WORKOS_DEFAULT_BASE_URL, client_id) + .await + .map_err(map_login_error)?; + + let token = auth::complete_device_auth_flow_returning_token( + client, + auth::WORKOS_DEFAULT_BASE_URL, + client_id, + &authorization, + ) + .await + .map_err(map_login_error)?; + + let stored_tokens = + run_credential_operation(move || token_storage::save_tokens(&token)).await?; render_login_result( &DeviceAuthFlowResult { @@ -448,6 +435,17 @@ fn map_whoami_control_plane_error(error: &ControlPlaneError) -> CliError { ) } +async fn run_credential_operation(operation: F) -> Result +where + T: Send + 'static, + F: FnOnce() -> Result + Send + 'static, +{ + tokio::task::spawn_blocking(operation) + .await + .map_err(|error| unexpected_auth_command_error(error.into()))? + .map_err(auth_storage_error) +} + fn auth_storage_error(error: crate::services::token_storage::TokenStorageError) -> CliError { CliError::user_with_source(UserError::AuthStorageUnavailable, error) } @@ -460,6 +458,187 @@ fn unexpected_auth_command_error(error: anyhow::Error) -> CliError { mod tests { use super::*; + fn stored_tokens() -> StoredTokens { + StoredTokens { + access_token: "secret-access".into(), + refresh_token: "secret-refresh".into(), + token_type: "Bearer".into(), + scope: None, + expires_in: 3600, + stored_at_unix_seconds: 100, + } + } + + async fn completed_action(format: AuthFormat, name: &str) -> Result { + assert_eq!(format, AuthFormat::Json); + assert_eq!( + tokio::runtime::Handle::current().runtime_flavor(), + tokio::runtime::RuntimeFlavor::MultiThread + ); + tokio::task::yield_now().await; + Ok(name.to_string()) + } + + #[tokio::test(flavor = "multi_thread")] + async fn auth_dispatch_awaits_only_the_selected_action() { + for (subcommand, expected) in [ + ( + AuthSubcommand::Login { + format: AuthFormat::Json, + }, + "login", + ), + ( + AuthSubcommand::Logout { + format: AuthFormat::Json, + }, + "logout", + ), + ( + AuthSubcommand::Whoami { + format: AuthFormat::Json, + }, + "whoami", + ), + ] { + let result = run_auth_subcommand_with( + AuthRequest { subcommand }, + |format| async move { + assert_eq!(expected, "login"); + completed_action(format, "login").await + }, + |format| async move { + assert_eq!(expected, "logout"); + completed_action(format, "logout").await + }, + |format| async move { + assert_eq!(expected, "whoami"); + completed_action(format, "whoami").await + }, + ) + .await + .expect("selected action should complete"); + assert_eq!(result, expected); + } + } + + #[tokio::test(flavor = "multi_thread")] + async fn login_helper_awaits_owned_renewal_and_redacts_credentials() { + let report = run_login_with_stored_credentials( + AuthFormat::Json, + Some(stored_tokens()), + |tokens| async move { + completed_action(AuthFormat::Json, "renew").await?; + Ok(Some(tokens)) + }, + |_| async { panic!("renewed credentials must skip device login") }, + ) + .await + .expect("renewal should complete"); + assert_eq!( + report, + render_login_refresh_result(&stored_tokens(), AuthFormat::Json).unwrap() + ); + assert!(!report.contains("secret-access")); + assert!(!report.contains("secret-refresh")); + } + + #[tokio::test(flavor = "multi_thread")] + async fn login_helper_awaits_device_fallback_after_unsuccessful_renewal() { + let renewal_completed = std::cell::Cell::new(false); + let renewed = &renewal_completed; + let result = run_login_with_stored_credentials( + AuthFormat::Json, + Some(stored_tokens()), + |_| async { + completed_action(AuthFormat::Json, "renew").await?; + renewed.set(true); + Ok(None) + }, + |format| async move { + assert!(renewed.get()); + completed_action(format, "device login completed").await + }, + ) + .await + .expect("fallback should complete"); + assert_eq!(result, "device login completed"); + } + + fn check_blocking_credential_context() { + let runtime = tokio::runtime::Builder::new_current_thread() + .build() + .expect("build credential fixture runtime"); + runtime.block_on(async {}); + } + + #[tokio::test(flavor = "multi_thread")] + async fn auth_credential_load_save_delete_are_isolated_and_awaited() { + let credentials = std::sync::Arc::new(std::sync::Mutex::new(Some(stored_tokens()))); + let load_store = credentials.clone(); + let loaded = run_credential_operation(move || { + check_blocking_credential_context(); + Ok(load_store.lock().unwrap().clone()) + }) + .await + .expect("load should complete") + .unwrap(); + let save_store = credentials.clone(); + let saved = run_credential_operation(move || { + check_blocking_credential_context(); + let mut tokens = loaded; + tokens.access_token = "saved-access".into(); + *save_store.lock().unwrap() = Some(tokens.clone()); + Ok(tokens) + }) + .await + .expect("save should complete"); + assert_eq!(saved.access_token, "saved-access"); + assert_eq!( + credentials.lock().unwrap().as_ref().unwrap().access_token, + "saved-access" + ); + let delete_store = credentials.clone(); + let deleted = run_credential_operation(move || { + check_blocking_credential_context(); + Ok(delete_store.lock().unwrap().take().is_some()) + }) + .await + .expect("delete should complete"); + assert!(deleted); + assert!(credentials.lock().unwrap().is_none()); + } + + #[tokio::test(flavor = "multi_thread")] + async fn auth_credential_storage_and_join_failures_keep_typed_sources() { + let storage_error = run_credential_operation::<(), _>(|| { + Err(token_storage::TokenStorageError::Database( + "fixture failure".into(), + )) + }) + .await + .unwrap_err(); + let join_error = + run_credential_operation::<(), _>(|| panic!("credential worker fixture failure")) + .await + .unwrap_err(); + for (mapped, expected) in [ + (storage_error, UserError::AuthStorageUnavailable), + (join_error, UserError::UnexpectedFailure), + ] { + match mapped { + CliError::User { + error, + source: Some(source), + } => { + assert_eq!(error, expected); + assert!(source.to_string().contains("fixture failure")); + } + _ => panic!("credential failure lost its typed source"), + } + } + } + #[test] fn logout_text_reports_whether_credentials_were_removed() { assert_eq!( diff --git a/cli/src/services/command_registry.rs b/cli/src/services/command_registry.rs index 07f261903..77dc373dc 100644 --- a/cli/src/services/command_registry.rs +++ b/cli/src/services/command_registry.rs @@ -55,15 +55,19 @@ impl RuntimeCommand { } #[allow(dead_code)] - pub fn execute(&self, context: &C) -> Result + pub async fn execute(&self, context: &C) -> Result where C: HasLogger + ContextWithRepoRoot, { let mut stderr = std::io::sink(); - self.execute_with_stderr(context, &mut stderr) + self.execute_with_stderr(context, &mut stderr).await } - pub fn execute_with_stderr(&self, context: &C, stderr: &mut W) -> Result + pub async fn execute_with_stderr( + &self, + context: &C, + stderr: &mut W, + ) -> Result where C: HasLogger + ContextWithRepoRoot, W: Write, @@ -71,15 +75,38 @@ impl RuntimeCommand { match self { Self::Help(_) => Ok(services::help::help_text()), Self::HelpText(command) => Ok(command.execute(context)), - Self::Auth(command) => command.execute(context), + Self::Auth(command) => command.execute(context).await, Self::Config(command) => command.execute(context), - Self::Setup(command) => command.execute(context), - Self::Doctor(command) => command.execute(context), - Self::Hooks(command) => command.execute(context), + Self::Setup(command) => { + if command.request.context_only { + command.execute(context) + } else { + tokio::task::block_in_place(|| command.execute(context)) + } + } + Self::Doctor(command) => tokio::task::block_in_place(|| command.execute(context)), + Self::Hooks(command) => match &command.subcommand { + services::hooks::HookSubcommand::PreCommit + | services::hooks::HookSubcommand::PostRewrite { .. } => command.execute(context), + services::hooks::HookSubcommand::CommitMsg { .. } + | services::hooks::HookSubcommand::PostCommit { .. } + | services::hooks::HookSubcommand::DiffTrace + | services::hooks::HookSubcommand::ConversationTrace + | services::hooks::HookSubcommand::Codex + | services::hooks::HookSubcommand::ClaudeModelState + | services::hooks::HookSubcommand::MutationScope + | services::hooks::HookSubcommand::ClaudeMutationScope + | services::hooks::HookSubcommand::CodexMutationScope + | services::hooks::HookSubcommand::OpenCodeMutationScope + | services::hooks::HookSubcommand::PiMutationScope + | services::hooks::HookSubcommand::ExternalMutationGuard => { + tokio::task::block_in_place(|| command.execute(context)) + } + }, Self::Policy(command) => command.execute(), Self::Version(command) => command.execute(context), Self::Completion(command) => Ok(command.execute(context)), - Self::Sync(command) => command.execute_with_stderr(context, stderr), + Self::Sync(command) => command.execute_with_stderr(context, stderr).await, } } } diff --git a/cli/src/services/hooks/tests.rs b/cli/src/services/hooks/tests.rs index 260d5887e..8abfc31d9 100644 --- a/cli/src/services/hooks/tests.rs +++ b/cli/src/services/hooks/tests.rs @@ -4033,3 +4033,129 @@ fn post_commit_checkpoint_failure_is_fail_open_and_logs_warning() { )] ); } + +#[test] +fn async_dispatch_doctor_and_hook_persist_before_completion() { + use std::io::Write; + use std::process::Stdio; + + let repo = init_attribution_git_repo("async-dispatch"); + let sandbox = tempfile::tempdir().unwrap(); + let payload = json!({ + "sessionID": "async-boundary-session", + "diff": valid_patch_text("src/boundary.rs", "completed"), + "time": 1_800_000_000_000_u64, + "model_id": "test/model", + "tool_name": "opencode", + "tool_version": null, + }); + let mut child = Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "services::hooks::tests::isolated_async_dispatch_boundary", + "--nocapture", + ]) + .current_dir(&repo) + .env("SCE_HOOK_BOUNDARY_CHILD", "1") + .env("HOME", sandbox.path()) + .env("XDG_CONFIG_HOME", sandbox.path().join("config")) + .env("XDG_STATE_HOME", sandbox.path().join("state")) + .env("XDG_DATA_HOME", sandbox.path().join("data")) + .env("XDG_CACHE_HOME", sandbox.path().join("cache")) + .env("NO_COLOR", "1") + .env_remove("SCE_DISABLED") + .env_remove("SCE_CONFIG_FILE") + .env_remove("SCE_ATTRIBUTION_HOOKS_DISABLED") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + child + .stdin + .take() + .unwrap() + .write_all(payload.to_string().as_bytes()) + .unwrap(); + let output = child.wait_with_output().unwrap(); + fs::remove_dir_all(&repo).unwrap(); + assert!( + output.status.success(), + "{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn isolated_async_dispatch_boundary() { + use crate::app::AppContext; + use crate::services::capabilities::{ProcessGitOps, StdFsOps}; + use crate::services::command_registry::{default_runtime_command, RuntimeCommand}; + use crate::services::observability::traits::{NoopLogger, NoopTelemetry}; + + if std::env::var_os("SCE_HOOK_BOUNDARY_CHILD").is_none() { + return; + } + assert_eq!( + tokio::runtime::Handle::current().runtime_flavor(), + tokio::runtime::RuntimeFlavor::MultiThread + ); + let repo = std::env::current_dir().unwrap(); + let storage_context = AgentTraceStorageContext { + repository_root: &repo, + explicit_repository_id: None, + repository_remote: "origin", + }; + let state_root = crate::services::default_paths::resolve_state_data_root().unwrap(); + let db_path = tokio::task::block_in_place(|| { + let storage = + resolve_agent_trace_storage_at_state_root(&storage_context, &state_root).unwrap(); + storage.db_path.clone() + }); + let context = AppContext::new( + &NoopLogger, + &NoopTelemetry, + &StdFsOps, + &ProcessGitOps, + Some(repo.clone()), + ); + let doctor = default_runtime_command("doctor").unwrap(); + let doctor_output = doctor.execute(&context).await.unwrap(); + assert!(doctor_output.contains("Agent Trace"), "{doctor_output}"); + assert!( + !doctor_output.contains("schema is not ready"), + "{doctor_output}" + ); + let hook = RuntimeCommand::Hooks(crate::services::hooks::command::HooksCommand { + subcommand: HookSubcommand::DiffTrace, + }); + assert_eq!( + hook.execute(&context).await.unwrap(), + "diff-trace hook intake persisted payload to AgentTraceDb." + ); + tokio::task::block_in_place(|| { + let db = RepositoryAgentTraceDb::open_without_migrations_at(&db_path).unwrap(); + let rows = db + .query_map( + "SELECT session_id, model_id, tool_name FROM diff_traces ORDER BY id", + (), + |row| { + Ok(( + row.get::(0)?, + row.get::(1)?, + row.get::(2)?, + )) + }, + ) + .unwrap(); + assert_eq!( + rows, + vec![( + "oc_async-boundary-session".to_string(), + "test/model".to_string(), + "opencode".to_string() + )] + ); + }); +} diff --git a/cli/src/services/observability/traits.rs b/cli/src/services/observability/traits.rs index 0515e080a..330854885 100644 --- a/cli/src/services/observability/traits.rs +++ b/cli/src/services/observability/traits.rs @@ -1,3 +1,5 @@ +use std::future::Future; + use crate::services::error::CliError; pub trait Logger: Send + Sync { @@ -37,10 +39,10 @@ pub trait Logger: Send + Sync { } pub trait Telemetry: Send + Sync { - fn with_default_subscriber( - &self, - action: &mut dyn FnMut() -> Result, - ) -> Result; + async fn with_default_subscriber(&self, action: &mut F) -> Result + where + F: FnMut() -> Fut, + Fut: Future>; } #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] @@ -137,10 +139,11 @@ impl Logger for super::Logger { pub struct NoopTelemetry; impl Telemetry for NoopTelemetry { - fn with_default_subscriber( - &self, - action: &mut dyn FnMut() -> Result, - ) -> Result { - action() + async fn with_default_subscriber(&self, action: &mut F) -> Result + where + F: FnMut() -> Fut, + Fut: Future>, + { + action().await } } diff --git a/cli/src/services/sync/command.rs b/cli/src/services/sync/command.rs index 5df113a0b..3455877fc 100644 --- a/cli/src/services/sync/command.rs +++ b/cli/src/services/sync/command.rs @@ -44,24 +44,29 @@ fn classify_sync_error(err: TraceSyncError) -> CliError { impl SyncCommand { #[allow(dead_code)] - pub fn execute(&self, context: &C) -> Result + pub async fn execute(&self, context: &C) -> Result where C: ContextWithRepoRoot, { let mut stderr = std::io::sink(); - self.execute_with_stderr(context, &mut stderr) + self.execute_with_stderr(context, &mut stderr).await } - pub fn execute_with_stderr(&self, context: &C, stderr: &mut W) -> Result + pub async fn execute_with_stderr( + &self, + context: &C, + stderr: &mut W, + ) -> Result where C: ContextWithRepoRoot, W: Write, { let clock = SystemSyncProgressClock; self.execute_with_stderr_and_clock(context, stderr, &clock) + .await } - fn execute_with_stderr_and_clock( + async fn execute_with_stderr_and_clock( &self, context: &C, stderr: &mut W, @@ -78,7 +83,8 @@ impl SyncCommand { crate::services::output_format::OutputFormat::Text => { let mut progress = IndicatifProgressReporter::new(stderr); let result = - run_current_sync_with_progress_and_clock(&repo_root, &mut progress, clock); + run_current_sync_with_progress_and_clock(&repo_root, &mut progress, clock) + .await; if result.is_ok() { progress.finish_successfully(); } @@ -86,7 +92,7 @@ impl SyncCommand { } crate::services::output_format::OutputFormat::Json => { let mut progress = NoopProgressReporter; - run_current_sync_with_progress_and_clock(&repo_root, &mut progress, clock) + run_current_sync_with_progress_and_clock(&repo_root, &mut progress, clock).await } } .map_err(classify_sync_error)?; diff --git a/cli/src/services/sync/sync.rs b/cli/src/services/sync/sync.rs index 3658d9e6e..31730a9fe 100644 --- a/cli/src/services/sync/sync.rs +++ b/cli/src/services/sync/sync.rs @@ -7,16 +7,15 @@ use std::fmt; use std::future::{poll_fn, Future}; use std::path::Path; use std::rc::Rc; -use std::sync::OnceLock; use std::task::Poll; -use anyhow::Context; use chrono::{DateTime, SecondsFormat, Utc}; -use tokio::runtime::Runtime; use crate::services::agent_trace_db::repository::RepositoryAgentTraceDb; use crate::services::agent_trace_export::{AgentTraceExportReader, AGENT_TRACE_EXPORT_BATCH_SIZE}; -use crate::services::agent_trace_storage::{resolve_agent_trace_storage, AgentTraceStorageContext}; +use crate::services::agent_trace_storage::{ + resolve_agent_trace_storage, AgentTraceStorageContext, ResolvedAgentTraceStorage, +}; use crate::services::agent_trace_sync::control_plane::{ AgentTraceCursors, AgentTraceIngestionBatchRequest, AgentTraceIngestionBatchResponse, AgentTraceIngestionStateRequest, AuthenticatedControlPlaneClient, ControlPlaneError, @@ -29,8 +28,6 @@ use crate::services::auth; use crate::services::config; use crate::services::sync::progress::{NoopProgressReporter, ProgressReporter}; -static SYNC_RUNTIME: OnceLock = OnceLock::new(); - #[derive(Clone, Debug, PartialEq, Eq)] pub struct AgentTraceSyncReport { pub repository_id: String, @@ -152,14 +149,30 @@ impl TraceSyncError { } } +struct SyncStorageGuard(Option); + +impl SyncStorageGuard { + fn storage(&self) -> &ResolvedAgentTraceStorage { + self.0 + .as_ref() + .expect("sync storage remains owned until cleanup") + } +} + +impl Drop for SyncStorageGuard { + fn drop(&mut self) { + tokio::task::block_in_place(|| drop(self.0.take())); + } +} + #[allow(dead_code)] -pub fn run_current_sync(repo_root: &Path) -> Result { +pub async fn run_current_sync(repo_root: &Path) -> Result { let mut progress = NoopProgressReporter; - run_current_sync_with_progress(repo_root, &mut progress) + run_current_sync_with_progress(repo_root, &mut progress).await } /// Production entry point with an injectable progress sink. -pub fn run_current_sync_with_progress( +pub async fn run_current_sync_with_progress( repo_root: &Path, progress: &mut S, ) -> Result @@ -167,11 +180,11 @@ where S: ProgressReporter, { let clock = SystemSyncProgressClock; - run_current_sync_with_progress_and_clock(repo_root, progress, &clock) + run_current_sync_with_progress_and_clock(repo_root, progress, &clock).await } /// Production sync entry point with injectable progress sink and clock. -pub fn run_current_sync_with_progress_and_clock( +pub async fn run_current_sync_with_progress_and_clock( repo_root: &Path, progress: &mut S, clock: &C, @@ -183,14 +196,14 @@ where progress.report(SyncProgressEvent::Started { timestamp: timestamp(clock), }); - let result = run_current_sync_without_progress(repo_root, progress); + let result = run_current_sync_without_progress(repo_root, progress).await; progress.report(SyncProgressEvent::Finished { timestamp: timestamp(clock), }); result } -fn run_current_sync_without_progress( +async fn run_current_sync_without_progress( repo_root: &Path, progress: &mut S, ) -> Result @@ -204,29 +217,37 @@ where explicit_repository_id: storage_config.repository_id.as_deref(), repository_remote: &storage_config.repository_remote, }; - let storage = resolve_agent_trace_storage(&context) + let storage = tokio::task::block_in_place(|| resolve_agent_trace_storage(&context)) .map_err(|error| TraceSyncError::Runtime(format!("{error:#}")))?; + let storage_guard = SyncStorageGuard(Some(storage)); + let result = async { + let storage = storage_guard.storage(); + + let auth_config = config::resolve_auth_runtime_config(repo_root) + .map_err(|error| TraceSyncError::Runtime(format!("{error:#}")))?; + let client = AuthenticatedControlPlaneClient::new( + reqwest::Client::new(), + auth_config.control_plane_base_url.value.unwrap_or_default(), + auth::WORKOS_DEFAULT_BASE_URL, + auth_config.workos_client_id.value.unwrap_or_default(), + ); - let auth_config = config::resolve_auth_runtime_config(repo_root) - .map_err(|error| TraceSyncError::Runtime(format!("{error:#}")))?; - let client = AuthenticatedControlPlaneClient::new( - reqwest::Client::new(), - auth_config.control_plane_base_url.value.unwrap_or_default(), - auth::WORKOS_DEFAULT_BASE_URL, - auth_config.workos_client_id.value.unwrap_or_default(), - ); - - run_sync_against_without_progress( - &storage.metadata.repository_id, - &storage.metadata.source_instance_id, - &storage.db, - &client, - progress, - ) + run_sync_against_without_progress( + &storage.metadata.repository_id, + &storage.metadata.source_instance_id, + &storage.db, + &client, + progress, + ) + .await + } + .await; + drop(storage_guard); + result } #[cfg(test)] -pub(crate) fn run_sync_against( +pub(crate) async fn run_sync_against( repository_id: &str, source_instance_id: &str, db: &RepositoryAgentTraceDb, @@ -234,10 +255,11 @@ pub(crate) fn run_sync_against( ) -> Result { let mut progress = NoopProgressReporter; run_sync_against_with_progress(repository_id, source_instance_id, db, client, &mut progress) + .await } #[cfg(test)] -pub(crate) fn run_sync_against_with_progress( +pub(crate) async fn run_sync_against_with_progress( repository_id: &str, source_instance_id: &str, db: &RepositoryAgentTraceDb, @@ -256,10 +278,11 @@ where progress, &clock, ) + .await } #[cfg(test)] -pub(crate) fn run_sync_against_with_progress_and_clock( +pub(crate) async fn run_sync_against_with_progress_and_clock( repository_id: &str, source_instance_id: &str, db: &RepositoryAgentTraceDb, @@ -275,14 +298,15 @@ where timestamp: timestamp(clock), }); let result = - run_sync_against_without_progress(repository_id, source_instance_id, db, client, progress); + run_sync_against_without_progress(repository_id, source_instance_id, db, client, progress) + .await; progress.report(SyncProgressEvent::Finished { timestamp: timestamp(clock), }); result } -fn run_sync_against_without_progress( +async fn run_sync_against_without_progress( repository_id: &str, source_instance_id: &str, db: &RepositoryAgentTraceDb, @@ -292,16 +316,9 @@ fn run_sync_against_without_progress( where S: ProgressReporter, { - let runtime = shared_runtime()?; let reader = AgentTraceExportReader::new(db); - runtime.block_on(run_sync_async( - repository_id, - source_instance_id, - &reader, - client, - progress, - )) + run_sync_async(repository_id, source_instance_id, &reader, client, progress).await } async fn run_sync_async<'a, S>( @@ -567,21 +584,6 @@ fn cursor_for_stream(cursors: &AgentTraceCursors, stream: IngestionStream) -> i6 } } -fn shared_runtime() -> Result<&'static Runtime, TraceSyncError> { - if let Some(runtime) = SYNC_RUNTIME.get() { - return Ok(runtime); - } - - let runtime = tokio::runtime::Builder::new_current_thread() - .enable_io() - .enable_time() - .build() - .context("failed to create sync command runtime") - .map_err(|error| TraceSyncError::Runtime(format!("{error:#}")))?; - - Ok(SYNC_RUNTIME.get_or_init(|| runtime)) -} - #[cfg(test)] mod tests { use std::fs; @@ -604,6 +606,106 @@ mod tests { use crate::services::auth::TokenResponse; use crate::services::token_storage::StoredTokens; + struct TestDb(Option); + + fn storage_guard_fixture(sandbox: &Path) -> SyncStorageGuard { + let repo = sandbox.join("repo"); + fs::create_dir_all(&repo).unwrap(); + let output = std::process::Command::new("git") + .args(["init", "-q"]) + .current_dir(&repo) + .output() + .unwrap(); + assert!(output.status.success()); + let context = AgentTraceStorageContext { + repository_root: &repo, + explicit_repository_id: Some("sync-cleanup-boundary"), + repository_remote: "origin", + }; + let storage = tokio::task::block_in_place(|| { + crate::services::agent_trace_storage::resolve_agent_trace_storage_at_state_root( + &context, + &sandbox.join("state"), + ) + .unwrap() + }); + SyncStorageGuard(Some(storage)) + } + + #[tokio::test(flavor = "multi_thread")] + async fn storage_guard_cleans_up_on_error_before_returning() { + let sandbox = tempfile::tempdir().unwrap(); + let mut db_path = PathBuf::new(); + let result: Result<(), TraceSyncError> = async { + let guard = storage_guard_fixture(sandbox.path()); + db_path = guard.storage().db_path.clone(); + seed_one_row_per_stream(&guard.storage().db); + tokio::task::yield_now().await; + Err(TraceSyncError::Runtime( + "fixture failure after storage opened".into(), + ))? + } + .await; + assert!( + matches!(result, Err(TraceSyncError::Runtime(reason)) if reason == "fixture failure after storage opened") + ); + tokio::task::block_in_place(|| { + let db = RepositoryAgentTraceDb::open_without_migrations_at(&db_path).unwrap(); + assert_eq!(diff_trace_rows(&db).len(), 1); + }); + } + + #[tokio::test(flavor = "multi_thread")] + async fn storage_guard_cleans_up_when_polled_future_is_abandoned() { + let sandbox = tempfile::tempdir().unwrap(); + let db_path = RefCell::new(None); + let mut future = Box::pin(async { + let guard = storage_guard_fixture(sandbox.path()); + seed_one_row_per_stream(&guard.storage().db); + *db_path.borrow_mut() = Some(guard.storage().db_path.clone()); + std::future::pending::<()>().await; + drop(guard); + }); + poll_fn(|cx| { + assert!(future.as_mut().poll(cx).is_pending()); + Poll::Ready(()) + }) + .await; + assert!(db_path.borrow().is_some()); + drop(future); + tokio::task::block_in_place(|| { + let db = RepositoryAgentTraceDb::open_without_migrations_at( + db_path.borrow().as_ref().unwrap(), + ) + .unwrap(); + assert_eq!(diff_trace_rows(&db).len(), 1); + }); + } + + impl TestDb { + fn new(path: &Path) -> Self { + Self(Some(tokio::task::block_in_place(|| { + RepositoryAgentTraceDb::new_at(path).expect("test DB should open") + }))) + } + } + + impl std::ops::Deref for TestDb { + type Target = RepositoryAgentTraceDb; + + fn deref(&self) -> &Self::Target { + self.0 + .as_ref() + .expect("test DB remains owned until cleanup") + } + } + + impl Drop for TestDb { + fn drop(&mut self) { + tokio::task::block_in_place(|| drop(self.0.take())); + } + } + fn unique_test_db_path(label: &str) -> PathBuf { let nonce = SystemTime::now() .duration_since(UNIX_EPOCH) @@ -860,10 +962,10 @@ mod tests { } } - #[test] - fn progress_events_cover_batches_empty_streams_and_fixed_order() { + #[tokio::test(flavor = "multi_thread")] + async fn progress_events_cover_batches_empty_streams_and_fixed_order() { let db_path = unique_test_db_path("progress-events"); - let db = RepositoryAgentTraceDb::new_at(&db_path).expect("test DB should open"); + let db = TestDb::new(&db_path); let metadata = db .verify_or_initialize_repository_metadata("repo-progress-events") .expect("metadata should initialize"); @@ -904,6 +1006,7 @@ mod tests { &mut |event| events.borrow_mut().push(event), &clock, ) + .await .expect("sync should succeed"); assert_eq!(report.streams.messages.uploaded, 201); @@ -959,10 +1062,10 @@ mod tests { remove_test_db(&db_path); } - #[test] - fn full_sync_uploads_three_remote_streams_and_second_run_is_naturally_incremental() { + #[tokio::test(flavor = "multi_thread")] + async fn full_sync_uploads_three_remote_streams_and_second_run_is_naturally_incremental() { let db_path = unique_test_db_path("full-sync"); - let db = RepositoryAgentTraceDb::new_at(&db_path).expect("test DB should open"); + let db = TestDb::new(&db_path); let metadata = db .verify_or_initialize_repository_metadata("repo-full-sync") .expect("metadata should initialize"); @@ -984,6 +1087,7 @@ mod tests { &db, &client, ) + .await .expect("first sync should succeed"); let first_run_requests = server.captured_requests(); @@ -1019,6 +1123,7 @@ mod tests { &db, &client, ) + .await .expect("second sync should succeed"); let all_requests = server.captured_requests(); @@ -1071,10 +1176,10 @@ mod tests { remove_test_db(&db_path); } - #[test] - fn sync_succeeds_when_local_diff_trace_row_is_rejected_by_export_reader() { + #[tokio::test(flavor = "multi_thread")] + async fn sync_succeeds_when_local_diff_trace_row_is_rejected_by_export_reader() { let db_path = unique_test_db_path("rejected-diff-trace"); - let db = RepositoryAgentTraceDb::new_at(&db_path).expect("test DB should open"); + let db = TestDb::new(&db_path); let metadata = db .verify_or_initialize_repository_metadata("repo-rejected-diff-trace") .expect("metadata should initialize"); @@ -1093,6 +1198,7 @@ mod tests { &db, &client, ) + .await .expect("sync should not read local diff_traces rows"); let requests = server.captured_requests(); @@ -1108,10 +1214,10 @@ mod tests { remove_test_db(&db_path); } - #[test] - fn diff_traces_report_echoes_server_cursor_without_uploading_newer_local_rows() { + #[tokio::test(flavor = "multi_thread")] + async fn diff_traces_report_echoes_server_cursor_without_uploading_newer_local_rows() { let db_path = unique_test_db_path("diff-traces-compat"); - let db = RepositoryAgentTraceDb::new_at(&db_path).expect("test DB should open"); + let db = TestDb::new(&db_path); let metadata = db .verify_or_initialize_repository_metadata("repo-diff-traces-compat") .expect("metadata should initialize"); @@ -1130,6 +1236,7 @@ mod tests { &db, &client, ) + .await .expect("sync should succeed"); let requests = server.captured_requests(); @@ -1145,10 +1252,10 @@ mod tests { remove_test_db(&db_path); } - #[test] - fn concurrent_sync_overlaps_all_three_stream_batches_after_one_state_request() { + #[tokio::test(flavor = "multi_thread")] + async fn concurrent_sync_overlaps_all_three_stream_batches_after_one_state_request() { let db_path = unique_test_db_path("concurrent-overlap"); - let db = RepositoryAgentTraceDb::new_at(&db_path).expect("test DB should open"); + let db = TestDb::new(&db_path); let metadata = db .verify_or_initialize_repository_metadata("repo-concurrent-overlap") .expect("metadata should initialize"); @@ -1167,6 +1274,7 @@ mod tests { &db, &client, ) + .await .expect("concurrent sync should succeed"); assert_eq!(server.state_request_count(), 1); @@ -1191,10 +1299,10 @@ mod tests { remove_test_db(&db_path); } - #[test] - fn concurrent_sync_keeps_batches_sequential_within_one_stream() { + #[tokio::test(flavor = "multi_thread")] + async fn concurrent_sync_keeps_batches_sequential_within_one_stream() { let db_path = unique_test_db_path("concurrent-ordering"); - let db = RepositoryAgentTraceDb::new_at(&db_path).expect("test DB should open"); + let db = TestDb::new(&db_path); let metadata = db .verify_or_initialize_repository_metadata("repo-concurrent-ordering") .expect("metadata should initialize"); @@ -1243,6 +1351,7 @@ mod tests { &db, &client, ) + .await .expect("ordered multi-batch sync should succeed"); assert_eq!(server.state_request_count(), 1); @@ -1256,10 +1365,10 @@ mod tests { remove_test_db(&db_path); } - #[test] - fn invalid_state_cursor_fails_before_any_batch_request() { + #[tokio::test(flavor = "multi_thread")] + async fn invalid_state_cursor_fails_before_any_batch_request() { let db_path = unique_test_db_path("invalid-cursor"); - let db = RepositoryAgentTraceDb::new_at(&db_path).expect("test DB should open"); + let db = TestDb::new(&db_path); let metadata = db .verify_or_initialize_repository_metadata("repo-invalid-cursor") .expect("metadata should initialize"); @@ -1280,6 +1389,7 @@ mod tests { &db, &client, ) + .await .expect_err("out-of-range /state cursor should fail sync"); assert!(matches!( @@ -1295,10 +1405,10 @@ mod tests { remove_test_db(&db_path); } - #[test] - fn terminal_batch_status_fails_without_state_reconciliation() { + #[tokio::test(flavor = "multi_thread")] + async fn terminal_batch_status_fails_without_state_reconciliation() { let db_path = unique_test_db_path("terminal-batch"); - let db = RepositoryAgentTraceDb::new_at(&db_path).expect("test DB should open"); + let db = TestDb::new(&db_path); let metadata = db .verify_or_initialize_repository_metadata("repo-terminal-batch") .expect("metadata should initialize"); @@ -1318,6 +1428,7 @@ mod tests { &db, &client, ) + .await .expect_err("a terminal 404 /batch response should fail the sync"); assert!( @@ -1351,10 +1462,10 @@ mod tests { remove_test_db(&db_path); } - #[test] - fn progress_events_end_after_terminal_failure() { + #[tokio::test(flavor = "multi_thread")] + async fn progress_events_end_after_terminal_failure() { let db_path = unique_test_db_path("progress-failure"); - let db = RepositoryAgentTraceDb::new_at(&db_path).expect("test DB should open"); + let db = TestDb::new(&db_path); let metadata = db .verify_or_initialize_repository_metadata("repo-progress-failure") .expect("metadata should initialize"); @@ -1380,6 +1491,7 @@ mod tests { &mut |event| events.borrow_mut().push(event), &clock, ) + .await .expect_err("terminal batch failure should be reported"); assert!(matches!(error, TraceSyncError::Stream { .. })); @@ -1397,10 +1509,10 @@ mod tests { remove_test_db(&db_path); } - #[test] - fn malformed_2xx_batch_response_still_reconciles_via_state() { + #[tokio::test(flavor = "multi_thread")] + async fn malformed_2xx_batch_response_still_reconciles_via_state() { let db_path = unique_test_db_path("malformed-batch"); - let db = RepositoryAgentTraceDb::new_at(&db_path).expect("test DB should open"); + let db = TestDb::new(&db_path); let metadata = db .verify_or_initialize_repository_metadata("repo-malformed-batch") .expect("metadata should initialize"); @@ -1423,6 +1535,7 @@ mod tests { &db, &client, ) + .await .expect("an undecodable 2xx /batch body should still reconcile via /state and succeed"); assert_eq!(server.call_count(), 4); @@ -1449,10 +1562,10 @@ mod tests { remove_test_db(&db_path); } - #[test] - fn forbidden_state_response_fails_without_mutating_local_metadata() { + #[tokio::test(flavor = "multi_thread")] + async fn forbidden_state_response_fails_without_mutating_local_metadata() { let db_path = unique_test_db_path("forbidden"); - let db = RepositoryAgentTraceDb::new_at(&db_path).expect("test DB should open"); + let db = TestDb::new(&db_path); let metadata_before = db .verify_or_initialize_repository_metadata("repo-forbidden") .expect("metadata should initialize"); @@ -1470,6 +1583,7 @@ mod tests { &db, &client, ) + .await .expect_err("403 should fail the sync"); assert!(matches!( diff --git a/context/architecture.md b/context/architecture.md index 9eef4416d..cf1a7c13d 100644 --- a/context/architecture.md +++ b/context/architecture.md @@ -120,11 +120,11 @@ The repository includes a new placeholder Rust binary crate at `cli/`. - `cli/src/main.rs` is the executable entrypoint (`sce`) and delegates to `app::run`. - `cli/src/cli_schema.rs` defines the clap-based CLI schema using derive macros for all top-level commands and subcommands, including the top-level `sync` command, and renders command-local help text for the `auth` command tree (`auth`, `auth login`, `auth logout`, `auth whoami`). - `cli/src/app.rs` provides the clap-based argument dispatch loop with deterministic help/setup execution, bare-command help routing for `sce auth` and `sce config`, successful unknown-command fallback to top-level or closest-parent help, centralized stream routing (`stdout` success payloads, `stderr` redacted diagnostics), stable class-based exit-code mapping (`2` parse, `3` validation, `4` runtime, `5` dependency), and stable class-based stderr diagnostic codes (`SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, `SCE-ERR-DEPENDENCY`) with default `Try:` remediation injection when missing. -- The app runtime now moves through explicit startup phases in `cli/src/app.rs`: dependency bootstrapping (`perform_dependency_check`), startup context construction (`build_startup_context`), runtime initialization (`initialize_runtime`), command parse/execute inside telemetry subscriber context (`run_command_lifecycle`, `parse_command_phase` plus `services::app_support::execute_command_phase`), and final output rendering through `services::app_support::render_run_outcome`. `AppRuntime` owns the concrete production logger, no-op telemetry runtime, filesystem ops, git ops, static `CommandRegistry`, and startup-diagnostic state across those phases; `RunOutcome` carries final render data with an optional generic logger implementing `services::observability::traits::Logger`, so render support can log classified errors without production-logger type coupling. If a telemetry implementation attempts to invoke the command action more than once, dispatch returns a runtime-classified error instead of panicking or reusing consumed arguments. -- `AppContext` is the CLI's borrowed dependency view in `cli/src/app.rs`: it is generic over logger, telemetry, filesystem, and git capability implementations and stores references plus an optional `repo_root: Option` instead of owning `Arc` trait objects. Because it borrows from `AppRuntime`, `AppContext` is a lightweight, short-lived view and must not be stored long-term (e.g., in structs or across await points). Startup creates a context view over `AppRuntime`'s concrete production dependencies with `repo_root` set to `None`; command paths can derive repo-root-scoped context views through the `ContextWithRepoRoot` accessor trait / `AppContext::with_repo_root(...)`, which reuses the same borrowed dependencies while attaching the resolved root. Narrow accessor traits expose associated concrete capability types for logger, telemetry, fs, and git (`&Self::...`) plus repo-root access, so call sites can express capability requirements without erasing the borrowed dependencies back to trait objects; lifecycle providers consume the repo-root accessor rather than the full context type. +- The app runtime now moves through explicit startup phases in `cli/src/app.rs`: dependency bootstrapping (`perform_dependency_check`), startup context construction (`build_startup_context`), runtime initialization (`initialize_runtime`), directly awaited command parse/execute inside the telemetry polling scope (`run_command_lifecycle`, `parse_command_phase` plus `services::app_support::execute_command_phase`), and final output rendering through `services::app_support::render_run_outcome`. `AppRuntime` owns the concrete production logger, no-op telemetry runtime, filesystem ops, git ops, static `CommandRegistry`, and startup-diagnostic state across those phases; `RunOutcome` carries final render data with an optional generic logger implementing `services::observability::traits::Logger`, so render support can log classified errors without production-logger type coupling. If a telemetry implementation attempts to invoke the command action more than once, dispatch returns a runtime-classified error instead of panicking or reusing consumed arguments. +- `AppContext` is the CLI's borrowed dependency view in `cli/src/app.rs`: it is generic over logger, telemetry, filesystem, and git capability implementations and stores references plus an optional `repo_root: Option` instead of owning `Arc` trait objects. Because it borrows from `AppRuntime`, `AppContext` is command-scoped: borrowing across directly awaited execution is safe while the owner lives, but detached work cannot retain that borrowed view. No new `Send`, `Sync`, or `'static` command bounds are required. Startup creates a context view over `AppRuntime`'s concrete production dependencies with `repo_root` set to `None`; command paths can derive repo-root-scoped context views through the `ContextWithRepoRoot` accessor trait / `AppContext::with_repo_root(...)`, which reuses the same borrowed dependencies while attaching the resolved root. Narrow accessor traits expose associated concrete capability types for logger, telemetry, fs, and git (`&Self::...`) plus repo-root access, so call sites can express capability requirements without erasing the borrowed dependencies back to trait objects; lifecycle providers consume the repo-root accessor rather than the full context type. - Command parse-time conversion and run-time handling are separated by an internal static `RuntimeCommand` seam. `cli/src/services/command_registry.rs` defines the `RuntimeCommand` enum with variants for help/help-text, version, completion, auth, config, setup, doctor, hooks, policy, and sync, plus a deterministic `CommandRegistry` name catalog populated by `build_default_registry()`. `parse_command_phase` in `cli/src/app.rs` delegates clap-output conversion to `cli/src/services/parse/command_runtime.rs`, which owns clap error classification, help rendering bridges, longest-valid-parent traversal for unknown command paths, and parsed-request-to-enum conversion while returning concrete enum values. Service-owned `command.rs` modules define command payload structs and generic execution methods with narrow context requirements: context-free commands accept any context, hooks requires logger access, setup/doctor require repo-root scoping, and central dispatch requires the union of logger plus repo-root-scoping capabilities. `services::app_support::execute_command_phase` emits lifecycle logs around `RuntimeCommand::execute_with_stderr(...)`; the enum performs the only central dispatch match and delegates business behavior to the service-owned command structs, with sync receiving the app-owned stderr writer for format-gated progress. - Startup observability bootstrapping in `cli/src/app.rs` still tolerates invalid default-discovered config files by continuing with degraded defaults plus `sce.config.invalid_config` warn-level logs, but the warning/logging work is now isolated behind the startup-context and runtime-initialization phases rather than one inline startup function. -- `cli/src/services/observability.rs` provides deterministic runtime observability controls and rendering for app lifecycle logs, including shared config-resolved threshold/format, explicit config-file/default `log_to_file`, and `log_dir` inputs with precedence `env > config file > defaults` for non-flag observability keys, stable event identifiers, severity filtering, the forced-emission warning path used for invalid discovered config startup diagnostics, error-specific stderr suppression when file logging is enabled while non-error records and file-write diagnostics remain on stderr, redaction-safe emission through the shared security helper, and log-directory writes with bounded retention. Config resolution also carries a positive config-file/default-only `log_file_retention_limit` (`10` by default) into startup observability config and `sce config show`; the concrete logger stores that resolved value and threads it through primary and v2 cleanup. When `log_dir` resolves from `SCE_LOG_DIR`, config, or the `/sce/logs` default, each enabled or forced log operation selects `/sce-.log` or `/sce--.log` using the machine-local date and optional logger session context, with deterministic percent-encoding for unsafe session filename bytes; after successfully writing a newly created selected file, retention keeps the configured number of newest direct regular `*.log` files by mtime plus path/name tie-break and fails open on cleanup errors. Its `observability::traits` submodule exposes the current `Logger` API with `Option<&str>` session context plus object-safe `Telemetry` trait boundaries and `NoopLogger`; the concrete observability `Logger` and `NoopTelemetry` implement those capability traits; `NoopTelemetry` installs no tracing subscriber. `services::app_support::render_run_outcome` consumes the logger through that trait boundary when logging classified errors and stdout-write failures. +- `cli/src/services/observability.rs` provides deterministic runtime observability controls and rendering for app lifecycle logs, including shared config-resolved threshold/format, explicit config-file/default `log_to_file`, and `log_dir` inputs with precedence `env > config file > defaults` for non-flag observability keys, stable event identifiers, severity filtering, the forced-emission warning path used for invalid discovered config startup diagnostics, error-specific stderr suppression when file logging is enabled while non-error records and file-write diagnostics remain on stderr, redaction-safe emission through the shared security helper, and log-directory writes with bounded retention. Config resolution also carries a positive config-file/default-only `log_file_retention_limit` (`10` by default) into startup observability config and `sce config show`; the concrete logger stores that resolved value and threads it through primary and v2 cleanup. When `log_dir` resolves from `SCE_LOG_DIR`, config, or the `/sce/logs` default, each enabled or forced log operation selects `/sce-.log` or `/sce--.log` using the machine-local date and optional logger session context, with deterministic percent-encoding for unsafe session filename bytes; after successfully writing a newly created selected file, retention keeps the configured number of newest direct regular `*.log` files by mtime plus path/name tie-break and fails open on cleanup errors. Its `observability::traits` submodule exposes the current `Logger` API with `Option<&str>` session context plus the generic async, deliberately non-object-safe `Telemetry` capability and `NoopLogger`; the concrete observability `Logger` and `NoopTelemetry` implement those capability traits; `NoopTelemetry` installs no tracing subscriber. `services::app_support::render_run_outcome` consumes the logger through that trait boundary when logging classified errors and stdout-write failures. - `cli/src/services/observability.rs` no longer owns duplicate log enums or parsing helpers; it consumes the canonical primitive seam from `cli/src/services/config/mod.rs` and stays focused on concrete `Logger` behavior plus the `NoopTelemetry`-backed telemetry capability boundary. - `cli/src/cli_schema.rs` is now the canonical owner for top-level command metadata for the real clap-backed command set (`auth`, `config`, `setup`, `doctor`, `hooks`, `policy`, `sync`, `version`, `completion`), including the slim top-level help purpose text and per-command visibility on `sce`, `sce help`, and `sce --help`; `cli/src/command_surface.rs` remains the custom top-level help renderer and known-command classifier, adding the synthetic `help` row plus the ASCII banner while consuming that shared metadata instead of maintaining a parallel command catalog. - `cli/src/services/default_paths.rs` is the canonical production path catalog for the CLI: it resolves config/state/cache roots with platform-aware XDG or `dirs` fallbacks through an internal `roots` seam, exposes named default paths for current persisted artifacts and database/log files (global config, auth tokens, auth DB, local DB, default observability log directory, and the sole Agent Trace DB path helper `agent_trace_db_path_for_repository` under `repos//agent-trace.db`; the former global-sentinel and per-checkout Agent Trace path helpers were removed by the `retire-legacy-agent-trace-db` plan), and owns canonical repo-relative, embedded-asset, install, hook, and context-path accessors so non-test production path definitions have one shared owner. Compile-time generated payload paths are owned by `build.rs` under `OUT_DIR`, not by the default-path catalog. Current production consumers such as config discovery, observability config resolution, doctor reporting, setup/install flows, database adapters, checkout identity, Agent Trace storage resolution, and local hook runtime path resolution consume this shared catalog rather than defining owned path literals in their own modules. @@ -151,7 +151,7 @@ The repository includes a new placeholder Rust binary crate at `cli/`. - `cli/src/services/resilience.rs` defines bounded retry/timeout/backoff execution policy (`RetryPolicy`, `run_with_retry`) for transient operation hardening with deterministic failure messaging and retry observability. - `context/cli/agent-trace-auto-sync.md` documents the sync-owned, one-shot post-commit launcher boundary: it reuses `sce sync`, has no daemon or local retry machinery, and fails open when child startup cannot be completed; doctor reports this capability without invoking the launcher. - `cli/src/services/sync/progress.rs` owns the sync-local, consumer-typed progress seam: generic `ProgressReporter` supports event delivery plus explicit successful finalization, closure-based collectors, and a no-op implementation alongside the fixed `indicatif` stderr presentation adapter. `cli/src/services/sync/sync.rs` owns `SyncProgressEvent` and its payload semantics for the three remotely synchronized streams (`messages`, `parts`, `agent_traces`; `diff_traces` is captured locally but not synchronized), while `sync/command.rs` selects the terminal adapter for text and the no-op reporter for JSON. There is no top-level `cli/src/services/progress/` module; sync orchestration depends only on its sync-owned contract, so terminal-library details stay at the sync presentation boundary. -- `sce sync [--format text|json]` is implemented: `cli/src/services/sync/sync.rs` resolves repository-scoped Agent Trace storage, authenticates against the control plane with stored WorkOS credentials, uses the config-resolved `control_plane_base_url` with baked default `https://sce.crocoderlab.dev`, calls the ingestion `/state` endpoint once, then starts the `messages`/`parts`/`diff_traces`/`agent_traces` capture-stream state machines concurrently via `AgentTraceExportReader` and a shared per-stream reconciliation engine. Batches and cursor refreshes remain sequential within each stream, while fixed stream order is retained for final and stream-completion reporting; `cli/src/services/sync/render_sync.rs` renders the converged `AgentTraceSyncReport` as concise per-stream text or `camelCase` JSON without a nested subcommand field (see `context/cli/sync-command.md`). Local DB bootstrap and setup-time repository-scoped Agent Trace DB initialization otherwise still flow through lifecycle providers aggregated by setup, while repository-scoped DB health/repair flows through the doctor surface. The former trace database inspection and nested sync surfaces are unavailable. +- `sce sync [--format text|json]` is implemented: `cli/src/services/sync/sync.rs` resolves repository-scoped Agent Trace storage, authenticates against the control plane with stored WorkOS credentials, uses the config-resolved `control_plane_base_url` with baked default `https://sce.crocoderlab.dev`, calls the ingestion `/state` endpoint once, then starts the three remote `messages`/`parts`/`agent_traces` stream state machines concurrently (`diff_traces` remains a local-only compatibility report) via `AgentTraceExportReader` and a shared per-stream reconciliation engine. Batches and cursor refreshes remain sequential within each stream, while fixed stream order is retained for final and stream-completion reporting; `cli/src/services/sync/render_sync.rs` renders the converged `AgentTraceSyncReport` as concise per-stream text or `camelCase` JSON without a nested subcommand field (see `context/cli/sync-command.md`). Local DB bootstrap and setup-time repository-scoped Agent Trace DB initialization otherwise still flow through lifecycle providers aggregated by setup, while repository-scoped DB health/repair flows through the doctor surface. The former trace database inspection and nested sync surfaces are unavailable. - `cli/src/services/patch.rs` defines the standalone patch domain model (`ParsedPatch`, `PatchFileChange`, `FileChangeKind`, `PatchHunk`, `TouchedLine`, `TouchedLineKind`) for in-memory parsed unified-diff representation, capturing only touched lines (added/removed) plus minimal per-file/per-hunk metadata while excluding non-hunk headers and unchanged context lines. All types are `serde`-serializable/deserializable with `snake_case` JSON field naming. The module also provides `parse_patch`, a public parser function that converts raw unified-diff text (both `Index:` SVN-style and `diff --git` git-style formats) into `ParsedPatch` structs, with `ParseError` for actionable malformed-input diagnostics. Storage-agnostic JSON load helpers (`load_patch_from_json` for string input, `load_patch_from_json_bytes` for byte input) reconstruct `ParsedPatch` from serialized JSON content with `PatchLoadError` for actionable deserialization diagnostics. Its patch-set operations now include deterministic ordered combination plus target-shaped intersection that prefers exact touched-line matches and falls back to historical `kind`+`content` matching when incremental diffs and canonical post-commit diffs have drifted line numbers; `parse_patch`, `combine_patches`, and `intersect_patches` are consumed by the active post-commit hook runtime. - `cli/src/services/structured_patch.rs` defines the synchronous structured editor-hook derivation seam. It derives Claude `PostToolUse` `Write` structured-update hunks, `Write` `tool_input.content` create fallback, and `Edit` structured-patch payloads into canonical `ParsedPatch` values plus Claude session/tool metadata, returning deterministic skip reasons for unsupported events/tools/payload shapes. The module is pure and side-effect-free. It is wired into `sce hooks diff-trace` for Claude payload classification at intake (T04) and into `RepositoryAgentTraceDb::recent_diff_trace_patches` for post-commit structured payload parsing dispatch at read time (T05). - `cli/src/services/` contains module boundaries for command_registry, lifecycle, auth_command, config, setup, doctor, hooks, checkout identity, bash_policy, version, completion, help, patch, sync, SCE web URL helpers, shared database infrastructure, local DB adapters, encrypted auth DB adapters, and Agent Trace DB adapters with explicit trait seams for future implementations. `cli/src/services/checkout/` owns checkout ID file infrastructure (the per-checkout Agent Trace DB opener/path helper was removed by the `retire-legacy-agent-trace-db` plan); active setup/hooks use `agent_trace_storage` to establish checkout identity as diagnostics and initialize/open the repository-scoped DB, while `sce doctor` surfaces checkout identity facts plus credential-safe repository Agent Trace DB metadata. There is no checkout-scoped discovery or former trace inspection surface; any pre-migration `agent-trace-*.db` files on disk are never touched and no longer inspectable via the CLI. `cli/src/services/bash_policy.rs` owns both the CLI-agnostic evaluator logic and the hidden `sce policy bash` command adapter used by OpenCode, Claude, and Pi callers; Codex delegates in-process to the same evaluator for its native `PreToolUse(Bash)` response. `cli/src/services/command_registry.rs` defines the static `RuntimeCommand` enum, deterministic `CommandRegistry` name catalog, and `build_default_registry()` function for command dispatch metadata. Service-owned command modules own the runtime command payload structs for help/help-text, version, completion, auth, config, setup, doctor, hooks, policy, and sync. @@ -165,8 +165,34 @@ The repository includes a new placeholder Rust binary crate at `cli/`. - The config-lib check source preserves repo-relative access to shared CLI patch fixtures: Nix copies a filtered repo-shaped source containing `config/lib/**` plus `cli/src/services/structured_patch/fixtures`, then runs Bun/Biome from `config/lib/`. - `flake.nix` exposes the **native** development package as `packages.sce` (`packages.default = packages.sce`) plus `apps.sce` and `apps.default`, all targeting `${packages.sce}/bin/sce`; this keeps repo-local and remote flake run/install flows (`nix run .`, `nix run github:crocoder-dev/shared-context-engineering`, `nix profile install github:crocoder-dev/shared-context-engineering`) aligned to the native CLI output. The static-musl release binary is a distinct output, `packages.sce-release` (`apps.sce-release`); see the native/release split note above and `context/sce/cli-release-artifact-contract.md`. - `biome.json` at the repository root is the canonical Biome configuration for the current JS tooling slice and deliberately scopes coverage to `npm/**` plus the shared `config/lib/**` plugin package root while excluding package-local `node_modules/**`; `flake.nix` exposes Biome through the default dev shell rather than through package-local installs. -- `cli/Cargo.toml` keeps crates.io publication-ready package metadata for the `shared-context-engineering` crate, and `cli/README.md` is the Cargo install surface for crates.io (`cargo install shared-context-engineering --locked`) and local checkout (`./scripts/run-cli-cargo.sh install --path cli --locked`) guidance. Direct `cargo install --git` is unsupported because it cannot invoke the repository's pre-Cargo producer. The published crate installs the `sce` binary. Tokio remains intentionally constrained (`default-features = false`) with current-thread runtime usage plus timer-backed bounded resilience wrappers for retry/timeout behavior. -- `cli/Cargo.toml` now declares Tokio's `time` feature directly alongside the existing constrained current-thread runtime setup (`rt`, `io-util`, `time`) instead of relying on transitive enablement. +- `cli/Cargo.toml` keeps crates.io publication-ready package metadata for the `shared-context-engineering` crate, and `cli/README.md` is the Cargo install surface for crates.io (`cargo install shared-context-engineering --locked`) and local checkout (`./scripts/run-cli-cargo.sh install --path cli --locked`) guidance. Direct `cargo install --git` is unsupported because it cannot invoke the repository's pre-Cargo producer. The published crate installs the `sce` binary. Tokio remains intentionally constrained (`default-features = false`) with application-level multi-thread execution, retained DB-owned current-thread runtimes and timer-backed bounded resilience wrappers for retry/timeout behavior. +- `cli/Cargo.toml` explicitly enables Tokio `rt`, `io-util`, `sync`, `time`, `macros`, and `rt-multi-thread`, retaining `default-features = false`. `main` directly awaits async `app::run` under `#[tokio::main(flavor = "multi_thread")]`; only the synchronous Turso adapters retain their DB-owned current-thread runtimes. + +## Application execution runtime + +PR1 directly awaits static command execution under one application-level multi-thread Tokio runtime. Parsing, rendering and lifecycle providers remain synchronous; auth/sync own no command runtime. This boundary does not promise one runtime object process-wide. See the [runtime ownership decision](decisions/2026-10-07-application-owned-async-command-runtime.md) and the [command blocking-scope inventory](cli/cli-command-surface.md#application-runtime-dispatch). + +```mermaid +flowchart TD + A[Application Tokio multi-thread runtime] --> B[Awaited static command dispatch] + B --> C[Awaited auth, sync and control-plane HTTP] + C --> D[Awaited credential spawn_blocking] + B --> E[Direct synchronous commands] + B --> F[DB-capable Setup, Doctor and Hooks blocking scopes] + C --> G[Sync storage construction and cleanup guard blocking scopes] + D --> H[Synchronous Turso adapter with DB-owned runtime] + F --> H + G --> H +``` + +Sync retains borrowed storage across HTTP awaits and drops its owning guard before returning; guard Drop also handles error, cancellation and unwind. PR2 is future work: async Turso removes DB-owned runtimes, `build_current_thread_runtime`, `block_on_isolated` and the DB-related `block_in_place` scopes together. + +```mermaid +flowchart TD + A[PR2 future application Tokio runtime] --> B[Awaited static command dispatch] + B --> C[Async auth, sync and HTTP] + B --> D[Async Turso without DB-owned runtime or bridge] +``` ## Build / devShell / CI performance (flake-speedup) diff --git a/context/cli/capability-traits.md b/context/cli/capability-traits.md index 0922ff695..c919e0143 100644 --- a/context/cli/capability-traits.md +++ b/context/cli/capability-traits.md @@ -11,7 +11,7 @@ The CLI exposes broad capability traits in `cli/src/services/capabilities.rs` an - `resolve_repository_root` uses `git rev-parse --show-toplevel`. - `resolve_hooks_directory` uses `git rev-parse --git-path hooks` and resolves relative hook paths against the provided repository root. - `AppRuntime` owns concrete `StdFsOps` and `ProcessGitOps` production dependencies alongside concrete observability dependencies; these dependencies are borrowed by `AppContext` rather than stored behind type-erased runtime containers. -- `AppContext` is generic over logger, telemetry, filesystem, and git capability implementations and borrows them from the runtime; it is passed through static `RuntimeCommand::execute` (enum defined in `cli/src/services/command_registry.rs`) behind narrow accessor bounds. Because it borrows from `AppRuntime`, `AppContext` is a lightweight, short-lived view and must not be stored long-term (e.g., in structs or across await points). Its inherent `logger()` / `telemetry()` / `fs()` / `git()` helpers return the concrete generic references (`&L`, `&T`, `&F`, `&G`), and `HasLogger`, `HasTelemetry`, `HasFs`, and `HasGit` use associated capability types that return `&Self::{Capability}` instead of object-erased `&dyn ...` values. This is the current static-DI boundary: command and service call sites express narrow capability requirements while preserving concrete borrowed dependency types. `ContextWithRepoRoot` derives a repository-scoped context while preserving the borrowed runtime capability objects. +- `AppContext` is generic over logger, telemetry, filesystem, and git capability implementations and borrows them from the runtime; it is passed through directly awaited static `RuntimeCommand::execute` (enum defined in `cli/src/services/command_registry.rs`) behind narrow accessor bounds. Because it borrows from `AppRuntime`, `AppContext` is command-scoped: borrowing across directly awaited execution is safe while the owner lives, but detached work cannot retain that borrowed view. No new `Send`, `Sync`, or `'static` command bounds are required. Its inherent `logger()` / `telemetry()` / `fs()` / `git()` helpers return the concrete generic references (`&L`, `&T`, `&F`, `&G`), and `HasLogger`, `HasTelemetry`, `HasFs`, and `HasGit` use associated capability types that return `&Self::{Capability}` instead of object-erased `&dyn ...` values. This is the current static-DI boundary: command and service call sites express narrow capability requirements while preserving concrete borrowed dependency types. `ContextWithRepoRoot` derives a repository-scoped context while preserving the borrowed runtime capability objects. - Current command execution bounds are capability-oriented: central dispatch requires logger access plus repo-root scoping, context-free commands accept any context, hooks require logger access, and setup/doctor require repo-root scoping. - Test-only `UnimplementedFsOps` and `UnimplementedGitOps` stubs are available under `capabilities::test_stubs` for tests that need to satisfy trait bounds before providing focused fakes. diff --git a/context/cli/cli-command-surface.md b/context/cli/cli-command-surface.md index e1d856640..bf9a4a377 100644 --- a/context/cli/cli-command-surface.md +++ b/context/cli/cli-command-surface.md @@ -14,14 +14,30 @@ Operator onboarding currently comes from `sce --help`, command-local `--help` ou - Service domains: `cli/src/services/{agent_trace,agent_trace_db,auth,auth_command,auth_db,completion,config,db,default_paths,hooks,local_db,observability,output_format,resilience,security,setup,style,sync,token_storage,version}` plus the split doctor module at `cli/src/services/doctor/{mod,command,inspect,render,fixes,types}.rs`; service-owned `command.rs` files own command payload structs for help/version/completion/auth/config/setup/doctor/hooks/policy/sync, and `cli/src/services/command_registry.rs` owns the static `RuntimeCommand` enum that dispatches them - Service lifecycle: `cli/src/services/lifecycle.rs` defines lifecycle-owned health/setup result types, the `ServiceLifecycle` trait with `diagnose`, `fix`, and `setup` methods for concrete providers, and the static `LifecycleProvider` enum used by `doctor`/`setup` to dispatch across `config`, `local_db`, `auth_db`, `agent_trace_db`, and `hooks` without boxed provider aggregation +## Application runtime dispatch + +`RuntimeCommand::execute_with_stderr` directly awaits Auth and Sync. The temporary `block_in_place` scopes protect construction and destruction of legacy DB-owned runtimes; ordinary filesystem, Git, parsing and rendering work alone does not require a scope. Lifecycle providers and synchronous command APIs remain synchronous. + +| Path | Execution and lifetime reason | +| --- | --- | +| Help, HelpText, Version, Completion, Policy, Config Show/Validate | Direct synchronous execution; no runtime-owning DB resources. | +| Setup | Context-only requests execute directly. Other requests execute inside one blocking scope because LocalDb/AuthDb/AgentTraceDb lifecycle setup constructs and destroys DB adapters. | +| Doctor Diagnose/Fix | One blocking scope around execution; AgentTraceDb diagnosis opens a local DB even without repair. | +| Hooks PreCommit/PostRewrite | Direct synchronous execution; these paths do not own DB resources. | +| Hooks CommitMsg, PostCommit, DiffTrace, ConversationTrace, Codex, ClaudeModelState, MutationScope, ClaudeMutationScope, CodexMutationScope, OpenCodeMutationScope, PiMutationScope, ExternalMutationGuard | One blocking scope around each invocation, including its error cleanup; these paths can open or persist through runtime-backed DB adapters. | +| Sync storage construction | One blocking scope around `resolve_agent_trace_storage`, including partially constructed resources on failure. | +| Sync storage destruction | A concrete `SyncStorageGuard` owns storage across HTTP awaits. Its Drop takes and destroys storage inside one blocking scope on completion, error, cancellation or unwind; normal execution explicitly drops the guard before returning. | + +Auth's direct credential load/save/delete and control-plane credential load/save use awaited owned `spawn_blocking` closures. Refresh and HTTP stay async. Hooks finish foreground persistence before returning; the existing post-commit auto-sync child remains detached. See [application execution runtime](../architecture.md#application-execution-runtime), [capability borrowing](capability-traits.md), and [sync](sync-command.md). + ## Onboarding documentation - `sce --help` includes a slim top-level command list and quick-start examples for `setup`, `doctor`, and `version`; `auth` is visible while `hooks` remains implemented in code but hidden from `sce`, `sce help`, and `sce --help` for this phase. - `cli/src/cli_schema.rs` owns the real top-level command catalog metadata for clap-backed commands (purpose text plus `show_in_top_level_help`), while `command_surface::help_text()` consumes that catalog and adds the synthetic `help` row plus the ASCII banner. -- `cli/src/app.rs` now owns an explicit startup lifecycle (`perform_dependency_check` -> `build_startup_context` -> `initialize_runtime` -> `run_command_lifecycle` -> `render_run_outcome`) so dependency bootstrap, config-backed runtime initialization, command parsing/execution, and final stream rendering are no longer coordinated inside one monolithic startup function. +- `cli/src/app.rs` now owns an explicit startup lifecycle (`perform_dependency_check` -> `build_startup_context` -> `initialize_runtime` -> `run_command_lifecycle` -> `render_run_outcome`) with command execution directly awaited under the application-level multi-thread Tokio runtime before final stream rendering. See [application execution runtime](../architecture.md#application-execution-runtime) for runtime ownership and temporary DB lifetime scopes. - `cli/src/app.rs` also routes clap output through an internal static `RuntimeCommand` enum defined in `cli/src/services/command_registry.rs`, so parse-time conversion and run-time command execution stay separated while avoiding boxed command trait objects. - Command-local help is available for implemented commands including bare `sce auth`, `sce auth --help`, `sce auth login --help`, `sce setup --help`, `sce doctor --help`, and `sce completion --help`; when stdout color is enabled those help payloads now reuse the shared heading/command/placeholder styling pass while non-TTY and `NO_COLOR` flows stay plain text. Human-readable stderr diagnostics and interactive setup prompt text now follow the same shared styling policy on their respective terminal streams. -- Current repository verification guidance for this CLI slice prefers the root Nix entrypoints: `nix flake check` for routine validation, `nix build .#sce` / `nix run .#sce -- --help` for native packaged installability, and targeted `nix develop -c sh -c 'cd cli && '` only when a narrower Rust-only check is explicitly needed. +- Current repository verification guidance for this CLI slice uses `nix flake check` for normal repository checks and the separate `nix build .#ci-checks` for the release build (static musl on Linux, native on Darwin) plus Linux portability validation of the real release binary. Both tiers verify changes to the execution/build model. Native packaged installability uses `nix build .#sce` / `nix run .#sce -- --help`; targeted Cargo checks use `nix develop -c ./scripts/run-cli-cargo.sh ...` so generated payload preparation remains intact. ## Setup behavior selections @@ -92,7 +108,7 @@ An interactive `setup` run instead resolves the selection through an `inquire` m - Argument parsing is handled by `clap` derive macros in `cli/src/cli_schema.rs` and dispatched from `cli/src/app.rs`. - `cli/src/app.rs` now runs commands through explicit phases with `StartupContext`, `AppRuntime`, and generic `RunOutcome` carrying startup-derived observability config, logger/telemetry state, registry state, and final render data across the lifecycle without hardcoding render support to the production logger type. -- `parse_command_phase` delegates clap output conversion to `cli/src/services/parse/command_runtime.rs`, which returns concrete `RuntimeCommand` enum variants; `services::app_support::execute_command_phase` emits lifecycle logs around `RuntimeCommand::execute(...)`, and the enum delegates behavior to service-owned command payload structs. +- `parse_command_phase` delegates clap output conversion to `cli/src/services/parse/command_runtime.rs`, which returns concrete `RuntimeCommand` enum variants; `services::app_support::execute_command_phase` emits lifecycle logs around directly awaited `RuntimeCommand::execute_with_stderr(...)`, and the enum delegates behavior to service-owned command payload structs. - Top-level failures are classified into stable exit-code classes owned by `cli/src/app.rs`: `2` parse, `3` validation, `4` runtime, and `5` dependency. - User-facing diagnostics are rendered on `stderr` as `Error [SCE-ERR-]: ...` with class-default `Try:` remediation appended only when missing; when stderr color is enabled the heading, error code, and diagnostic body all render through shared stderr styling helpers. - Unknown commands and subcommands are successful help requests: an unknown top-level token returns the same stdout payload as `sce --help`, while an unknown token below a valid command returns the closest valid parent's existing help payload (including the custom `auth` help). Unknown options, extra positional arguments, and other parse/validation failures retain deterministic actionable diagnostics and their existing failure classes. @@ -119,7 +135,7 @@ An interactive `setup` run instead resolves the selection through an `inquire` m - `cli/src/services/default_paths.rs` defines the canonical per-user persisted-location seam for config/state/cache roots plus named default file paths for current persisted artifacts (`global config`, `auth tokens`, `local DB`, `agent trace DB`) used by config discovery, token storage, database adapters, and doctor diagnostics; its internal `roots` seam now owns the platform-aware root-directory resolution so non-test production modules consume shared path accessors instead of resolving owned roots directly. - `cli/src/services/agent_trace.rs` defines the canonical Rust SCE web base URL and helpers for Agent Trace conversation URLs, persisted Agent Trace trace URLs, Agent Trace session URLs, and setup-created repo-local config schema URLs. - `cli/src/services/token_storage.rs` defines WorkOS token persistence (`save_tokens`, `load_tokens`, `delete_tokens`) via the encrypted `AuthDb` `auth_credentials` table using a `OnceLock` lazy singleton with constant integer row ID `1`. `TokenStorageError` exposes `PathResolution` and `Database` variants. No JSON file I/O remains. -- `cli/src/services/auth_command/mod.rs` defines the auth command orchestration surface (`AuthRequest`, `AuthSubcommand`, `run_auth_subcommand`) for `login`, `logout`, and `whoami`, including shared text/JSON rendering, login's stored-token validation and refresh path for any stored credential, device-flow fallback after absent or unsuccessfully renewed credentials, source-level typed `CliError` propagation through `run_login_with_stored_credentials` for renewal, token-save, and device-login failures, token-storage-backed logout deletion with idempotent absent-credential success, unauthenticated whoami state reporting, Control Plane `/me` profile retrieval for authenticated whoami, flat safe-field rendering with optional-name/null-value handling, exact logged-out text guidance, precedence-aware client-ID guidance sourced from the shared auth-runtime resolver instead of env-only assumptions, and a lazily initialized current-thread Tokio runtime with both I/O and time enabled so the auth flows can drive the WorkOS device/refresh paths without the prior I/O-disabled panic; `cli/src/services/auth_command/command.rs` owns the `AuthCommand` payload used by the static `RuntimeCommand` enum. +- `cli/src/services/auth_command/mod.rs` defines the auth command orchestration surface (`AuthRequest`, `AuthSubcommand`, `run_auth_subcommand`) for `login`, `logout`, and `whoami`, including shared text/JSON rendering, login's stored-token validation and refresh path for any stored credential, device-flow fallback after absent or unsuccessfully renewed credentials, source-level typed `CliError` propagation through `run_login_with_stored_credentials` for renewal, token-save, and device-login failures, token-storage-backed logout deletion with idempotent absent-credential success, unauthenticated whoami state reporting, Control Plane `/me` profile retrieval for authenticated whoami, flat safe-field rendering with optional-name/null-value handling, exact logged-out text guidance, precedence-aware client-ID guidance sourced from the shared auth-runtime resolver instead of env-only assumptions, and directly awaited WorkOS/Control Plane HTTP on the application runtime; credential load/save/delete remain synchronous APIs invoked through awaited owned `spawn_blocking` closures; `cli/src/services/auth_command/command.rs` owns the `AuthCommand` payload used by the static `RuntimeCommand` enum. - `cli/src/app.rs` parses `auth`, `config`, `setup`, `doctor`, `hooks`, `policy`, `sync`, `version`, and `completion` into service-owned runtime command handlers so runtime messages are sourced from domain modules instead of inline strings. ## Local and Agent Trace Turso adapter behavior @@ -145,24 +161,24 @@ An interactive `setup` run instead resolves the selection through an `inquire` m - `doctor` command aggregates `diagnose`/`fix` across all registered lifecycle providers. - `setup` command aggregates `setup` across all registered lifecycle providers in order (config → local_db → auth_db → agent_trace_db → hooks). -## Parser-focused tests +## Execution and parser tests -- `cli/src/app.rs` unit tests cover default-help behavior, auth/config/setup/hooks routing, auth bare/help/nested-help routing, command-local `--help` routing for `doctor`/`hooks`, and failure paths for unknown commands/options and extra arguments. -- `cli/src/app.rs` additionally validates setup contract routing for interactive default, explicit target flags, and mutually-exclusive setup flag failures. +- `cli/src/app.rs` tests cover captured help/version/unknown-command output and dependency/command-error exit behavior under multi-thread Tokio, with process-isolated configuration/state. Recording telemetry checks awaited polling, lifecycle event order, failure paths and repeat protection through the private static lifecycle seam. +- `cli/src/services/parse/command_runtime.rs` tests cover sync/hook request conversion, hidden hook help entries, auth help, nearest-parent help for unknown commands and unknown-option classification. - `cli/src/services/local_db/mod.rs` tests cover in-memory and file-backed local Turso initialization plus execute/query smoke checks. - `cli/src/services/resilience.rs` tests lock deterministic sync retry behavior for transient failures, timeout classification, capped backoff, and actionable terminal error messaging. - `cli/src/services/setup/mod.rs` and `cli/src/services/hooks/mod.rs` include contract-focused tests for setup flag parsing/validation, interactive selection/cancellation dispatch, setup run messaging, and hook runtime argument/IO/finalization behavior. - `cli/src/services/token_storage.rs` tests cover token save/load round-trips, missing-file handling, token deletion outcomes, invalid JSON corruption handling, and Unix `0600` file-permission enforcement. - `cli/src/services/auth.rs` tests cover WorkOS device/token payload shape parsing, RFC 8628 device and refresh grant constant wiring, terminal OAuth error mapping with `Try:` guidance, polling decision handling for `authorization_pending`/`slow_down`/terminal outcomes, token-expiry evaluation, and refresh-token re-login guidance for terminal refresh errors. -- `cli/src/services/auth_command/mod.rs` tests cover auth subcommand dispatch, idempotent logout text/JSON results for absent and present credentials, unauthenticated whoami text/JSON guidance, typed authenticated whoami failure mappings with preserved sources, safe authenticated whoami JSON fields, stored valid/expired/absent-credential login routing, failed-renewal fallback, login-labeled renewal reports, `Try:` guidance preservation, and runtime-I/O readiness for the login flow. +- `cli/src/services/auth_command/mod.rs` retains renderer/classification coverage for logout, unauthenticated whoami, and typed authenticated whoami failures. Multi-thread async regressions cover awaited selected dispatch, owned renewal with secret redaction, device fallback, and blocking credential load/save/delete with preserved storage and worker-failure classification. - `cli/src/services/setup/mod.rs` tests also verify embedded-manifest completeness against runtime `config/` trees, deterministic sorted path normalization, and target-scoped iterator behavior (`OpenCode`, `Claude`, `Both`); sandbox-sensitive filesystem install coverage has been removed from the unit-test slice for later integration-test coverage. -- `cli/src/services/doctor/` unit coverage is intentionally limited to flake-safe output-shape assertions; filesystem, git, and real repair-flow coverage is deferred to future integration tests so `nix flake check` stays sandbox-safe. +- The isolated repository fixture in `cli/src/services/hooks/tests.rs` awaits Doctor Diagnose and DiffTrace through the real enum dispatcher, exercises command-local DB cleanup, and reads the persisted hook row after completion. Sync storage-guard tests cover cleanup on early error and abandonment of a polled future; existing sync overlap, sequential-batch, progress and cursor assertions remain authoritative. ## Dependency baseline - `cli/Cargo.toml` currently declares runtime dependencies on `anyhow`, `chrono`, `clap`, `clap_complete`, `dirs`, `hmac`, `inquire`, `jsonschema`, `keyring-core`, `murmur3`, `owo-colors`, `rand`, `reqwest`, `serde`, `serde_json`, `sha2`, `tokio`, `tracing`, `turso`, and `uuid`, plus target-specific keyring backend dependencies for Linux/FreeBSD, macOS, and Windows. No CLI dev-dependencies are currently declared. -- `tokio` is pinned with `default-features = false` and keeps a constrained runtime footprint for current-thread `Runtime::block_on` usage, plus timer-backed bounded retry/timeout behavior in resilience-wrapped operations. -- `cli/src/services/auth.rs` provides HTTP-only async functions for the WorkOS Device Authorization Flow (`request_device_authorization`, `poll_for_device_token`, `complete_device_auth_flow_returning_token`) and token-refresh operations (`ensure_valid_token_returning_token`, `renew_stored_token_from_refresh_token`, `is_stored_token_expired`): it requests device codes, polls at fixed API interval (adding 5 seconds on `slow_down`), maps RFC 8628 terminal errors to actionable `Try:` guidance, checks token expiry from persisted `stored_at_unix_seconds + expires_in` with a bounded skew guard, refreshes expired access tokens through `/oauth/token` using `grant_type=refresh_token`, and retries transient refresh failures via the shared resilience wrapper. Token storage (`save_tokens`, `load_tokens`) is called by the caller in `cli/src/services/auth_command/mod.rs` outside of any tokio `block_on` context to avoid nested-runtime panics. +- `tokio` is pinned with `default-features = false` and enables explicit `rt`, `io-util`, `sync`, `time`, `macros`, and `rt-multi-thread` for application execution and retained synchronous DB bridging, plus timer-backed bounded retry/timeout behavior in resilience-wrapped operations. +- `cli/src/services/auth.rs` provides HTTP-only async functions for the WorkOS Device Authorization Flow (`request_device_authorization`, `poll_for_device_token`, `complete_device_auth_flow_returning_token`) and token-refresh operations (`ensure_valid_token_returning_token`, `renew_stored_token_from_refresh_token`, `is_stored_token_expired`): it requests device codes, polls at fixed API interval (adding 5 seconds on `slow_down`), maps RFC 8628 terminal errors to actionable `Try:` guidance, checks token expiry from persisted `stored_at_unix_seconds + expires_in` with a bounded skew guard, refreshes expired access tokens through `/oauth/token` using `grant_type=refresh_token`, and retries transient refresh failures via the shared resilience wrapper. Token storage (`save_tokens`, `load_tokens`) is called by the caller in `cli/src/services/auth_command/mod.rs` in awaited owned `spawn_blocking` closures, isolating the synchronous DB/keyring work and retaining storage error classification. ## Scope boundary for this phase diff --git a/context/cli/execution-glossary.md b/context/cli/execution-glossary.md new file mode 100644 index 000000000..609e2be93 --- /dev/null +++ b/context/cli/execution-glossary.md @@ -0,0 +1,9 @@ +# CLI execution glossary + +- `command loop`: The `clap` derive-based parser + dispatcher in `cli/src/cli_schema.rs`, `cli/src/services/parse/command_runtime.rs`, and `cli/src/app.rs` that routes `help`, `config`, `setup`, `doctor`, `auth`, `hooks`, `policy`, `sync`, `version`, and `completion`, executes implemented command flows, emits command-local help payloads for supported subcommand trees, resolves unknown command paths to the longest valid parent's help surface, and returns deterministic actionable errors for unknown options and other invalid invocation. +- `RuntimeCommand seam`: Internal static command-execution abstraction in `cli/src/services/command_registry.rs` where clap-parsed commands are represented as a `RuntimeCommand` enum with variants for `Help`, `HelpText`, `Version`, `Completion`, `Auth`, `Config`, `Setup`, `Doctor`, `Hooks`, `Policy`, and `Sync`. The enum owns `name()` and async `execute(...)` dispatch, directly awaited under the application-level multi-thread Tokio runtime (distinct from retained DB-owned runtimes), delegating behavior to service-owned command payload structs while avoiding boxed `dyn RuntimeCommand` handles. Parsed request construction lives in `cli/src/services/parse/command_runtime.rs` when user-provided options or subcommands are required. +- `logger trait boundary`: `services::observability::traits::Logger` mirrors the current observability logger API (`info`, `debug`, `warn`, `error`, `log_cli_error`) for generic command/runtime bounds and tests, with each method accepting `Option<&str>` session context for file routing; the concrete `services::observability::Logger` implements it and `NoopLogger` remains available for side-effect-free tests. +- `telemetry trait boundary`: `services::observability::traits::Telemetry` defines the `with_default_subscriber` command-lifecycle capability used by generic app-runtime bounds and tests. The generic async callback deliberately makes this capability non-object-safe and statically selected through `HasTelemetry::Telemetry`. Production `NoopTelemetry` awaits the action once and installs no subscriber. Future static telemetry must scope every poll using future-aware subscriber attachment or an explicitly chosen global subscriber; a thread-local guard across await is insufficient. +- `AppContext`: Generic borrowed dependency view in `cli/src/app.rs` passed through static command dispatch. `AppRuntime` owns the concrete production logger, `NoopTelemetry` (no tracing subscriber), filesystem, and git implementations; `AppContext` stores references to those dependencies plus an optional `repo_root: Option`, not owned `Arc` trait objects. Because it borrows from `AppRuntime`, `AppContext` is command-scoped: borrowing across directly awaited execution is safe while the owner lives, but detached work cannot retain that borrowed view. No new `Send`, `Sync`, or `'static` command bounds are required. The `repo_root` field is `None` at startup and command paths can derive a repo-root-scoped context with `AppContext::with_repo_root(...)` / `ContextWithRepoRoot`, preserving the borrowed runtime dependencies while attaching the resolved root. Narrow accessor traits (`HasLogger`, `HasTelemetry`, `HasFs`, `HasGit`, `HasRepoRoot`) let command and lifecycle call sites express capability requirements without depending on the full production context type; logger/telemetry/fs/git accessors use associated concrete capability types and return `&Self::{Capability}` rather than object-erased `&dyn ...` values. + +See [application execution runtime](../architecture.md#application-execution-runtime), [capability traits](capability-traits.md), and [observability](../sce/cli-observability-contract.md). diff --git a/context/cli/sync-command.md b/context/cli/sync-command.md index 8f4df4865..4b96cd2be 100644 --- a/context/cli/sync-command.md +++ b/context/cli/sync-command.md @@ -6,6 +6,8 @@ control-plane ingestion API. The former `sce trace` command group and its database discovery, shell, list, status, and nested sync invocations are no longer available; no compatibility alias is retained. +Sync is directly awaited on the application-level multi-thread Tokio runtime. Its storage constructor uses a blocking scope; a concrete guard keeps the synchronous DB alive across HTTP awaits and destroys it in a blocking scope before return, including error, cancellation and unwind cleanup. Progress remains alive through the await and presentation finalization occurs only on success. See [application execution runtime](../architecture.md#application-execution-runtime). + The Clap surface is defined in `cli/src/cli_schema.rs` and dispatched through the static `RuntimeCommand::Sync` variant. The sync-owned command boundary lives under `cli/src/services/sync/`; shared storage, export, authentication, and diff --git a/context/context-map.md b/context/context-map.md index 07e001232..97c46ac5a 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -2,6 +2,8 @@ Primary context files: +Application runtime ownership: [architecture](architecture.md#application-execution-runtime), [accepted decision](decisions/2026-10-07-application-owned-async-command-runtime.md). + - `context/overview.md` - `context/architecture.md` - `context/patterns.md` @@ -9,7 +11,7 @@ Primary context files: Feature/domain context: -- `context/cli/cli-command-surface.md` (CLI command surface including top-level help with ASCII art banner and gradient rendering, setup install flow with the repeatable `sce setup --workflow ` optional-workflow selection and its interactive post-target multi-select, WorkOS device authorization flow + token storage behavior including stored-credential renewal through `sce auth login`, attribution-only hook routing with validated post-commit `--remote-url` plumbing plus DB-backed `diff-trace` dual persistence and post-commit Agent Trace payload persistence including range `content_hash`, setup-owned local DB + repository-scoped Agent Trace DB bootstrap plus doctor DB health coverage with credential-safe repository identity diagnostics, centralized Rust SCE web URL helpers in `services::agent_trace`, nested flake release package/app installability, Cargo local install + crates.io readiness policy, hidden `sce policy bash` command adapter for bash-policy hook callers, and top-level `sce sync` command wiring for current-repository Agent Trace synchronization; static `RuntimeCommand` enum dispatch lives in `services/command_registry.rs`, command payload structs for help/version/completion/auth/config/setup/doctor/hooks/policy/sync are owned by their respective `services/{name}/command.rs` files, and clap-to-runtime conversion lives in `services/parse/command_runtime.rs`) +- `context/cli/cli-command-surface.md` (CLI command surface including top-level help with ASCII art banner and gradient rendering, setup install flow with the repeatable `sce setup --workflow ` optional-workflow selection and its interactive post-target multi-select, WorkOS device authorization flow + token storage behavior including stored-credential renewal through `sce auth login`, attribution-only hook routing with validated post-commit `--remote-url` plumbing plus DB-backed `diff-trace` dual persistence and post-commit Agent Trace payload persistence including range `content_hash`, setup-owned local DB + repository-scoped Agent Trace DB bootstrap plus doctor DB health coverage with credential-safe repository identity diagnostics, centralized Rust SCE web URL helpers in `services::agent_trace`, nested flake release package/app installability, Cargo local install + crates.io readiness policy, hidden `sce policy bash` command adapter for bash-policy hook callers, and top-level `sce sync` command wiring for current-repository Agent Trace synchronization; directly awaited static `RuntimeCommand` enum dispatch under the application runtime lives in `services/command_registry.rs`, command payload structs for help/version/completion/auth/config/setup/doctor/hooks/policy/sync are owned by their respective `services/{name}/command.rs` files, and clap-to-runtime conversion lives in `services/parse/command_runtime.rs`) - `context/cli/default-path-catalog.md` (canonical production CLI path-ownership contract centered on `cli/src/services/default_paths.rs`, including persisted auth/config files, named DB paths for auth/local/repository-scoped Agent Trace databases, the default observability log-dir accessor consumed by config resolution with Linux `${XDG_STATE_HOME:-~/.local/state}/sce/logs` fallback semantics, repo-relative, embedded-asset, install, hook, and context-path families plus the regression guard that keeps production path ownership centralized) - `context/cli/repository-identity.md` (repository identity module in `cli/src/services/repository_identity/`: pure scheme-neutral `host[:port]/path` canonicalization for SCP/`ssh://`/HTTPS/`git://` remote URLs with credential stripping, hostname lowercasing, default-port removal, and query/fragment/trailing-`.git` cleanup, trim-only explicit-identity handling, `sha256("sce-repository-id-v1\0" + canonical_identity)` repository IDs, credential-safe fieldless errors, plus the `resolve` runtime submodule applying explicit-config-then-configured-remote precedence with `git config --get remote..url` lookup, `RepositoryIdentitySource` provenance, and `.sce/config.json`-guidance resolution errors that never echo URLs; consumed by the T04 `agent_trace_storage` resolver) - `context/cli/agent-trace-storage.md` (repository-scoped Agent Trace storage resolver in `cli/src/services/agent_trace_storage/`: `AgentTraceStorageContext` inputs mirroring the `agent_trace.*` config keys, shared degradation of invalid default-discovered config before identity fallback while explicit selections remain fatal, `ResolvedAgentTraceStorage` carrying repository identity + `/sce/repos//agent-trace.db` path + open `RepositoryAgentTraceDb` + typed `RepositoryMetadata`, `resolve_agent_trace_storage{,_at_state_root}` setup/lifecycle entrypoints with idempotent concurrent-safe first open via bounded fast-path-then-migrate retry plus narrow one-file schema migration-metadata repair and repository metadata validation, a separate no-migration `resolve_agent_trace_storage_for_hook_runtime{,_at_state_root}` pair that high-frequency hook callers use exclusively and that never runs migration `002` or any migration, path-unsafe repository ID rejection in `default_paths::agent_trace_db_path_for_repository{,_at}`, strict never-touch boundary for any pre-migration checkout-scoped/global DB files, and active hook/runtime plus Agent Trace lifecycle setup call-site consumption after T08) @@ -21,7 +23,7 @@ Feature/domain context: - `context/cli/sync-command.md` (the top-level `sce sync` command: repository-scoped Agent Trace storage resolution, WorkOS-authenticated three-stream control-plane synchronization (`messages`, `parts`, `agent_traces`; `diff_traces` stays local with a `streams.diffTraces` compatibility no-op entry) through the sync-owned consumer-typed `services::sync::progress` reporter contract with sync-owned events, its generic/no-op contract and `indicatif` presentation adapter for aligned stderr progress with independent stream completion, explicit successful finalization, JSON stdout silence, and rejection of the removed `sce trace` command group) - `context/cli/agent-trace-sync-command.md` (composed local-to-control-plane `sce sync` architecture: the `hooks/plugins → repository Agent Trace DB → AgentTraceExportReader → sce sync → HTTPS + WorkOS Bearer → control plane` data flow, the `sce auth login` / `cd ` / `sce sync` user flow, the no-local-cursor/no-`agent-trace-sync.db`/no-Turso-Sync/no-`BridgeLock`/no-local-DWH invariants, the four-local-capture-streams versus three-remote-streams boundary with its `diff_traces` release gate, and `401`/`409`/ambiguous-batch-failure recovery semantics) - `context/cli/config-precedence-contract.md` (implemented `sce config` show/validate command contract, deterministic `flags > env > config file > defaults` resolution order, focused `config/resolver.rs` ownership for config discovery/merge/runtime precedence plus default-discovered invalid-file degradation, focused `config/render.rs` ownership for `show`/`validate` text+JSON output construction, canonical `$schema` acceptance for startup-loaded `sce/config.json` files, shared auth-key env/config/optional baked-default support starting with `workos_client_id`, shared runtime resolution for flat logging observability keys including config-file/default `log_to_file`, `log_dir` / `SCE_LOG_DIR` with `/sce/logs` defaulting plus config-file/default-only positive `log_file_retention_limit`, config-file-only `agent_trace.repository_id`/`agent_trace.repository_remote` repository-identity keys with default remote `origin`, `agent_trace.auto_sync` resolution with omitted-value fallback `false`, explicit-false opt-out, and setup-written explicit `true` bootstrap behavior for the post-commit trigger boundary, the catalog-derived `integrations.optional_workflows` optional-workflow selection key, JSON-pointer-prefixed schema-validation errors, canonical Pkl-generated `sce/config.json` schema ownership plus CLI embedding/reuse contract including `policies.attribution_hooks.enabled` default-true/explicit-false opt-out metadata, config-file selection order, `show` provenance output, and trimmed `validate` output contract) -- `context/cli/capability-traits.md` (current broad CLI capability seam in `cli/src/services/capabilities.rs`, including `FsOps`/`StdFsOps`, `GitOps`/`ProcessGitOps`, git root/hooks resolution behavior, compile-time-typed borrowed AppContext wiring with associated-type narrow capability accessors plus `ContextWithRepoRoot` repo-root-scoped context derivation, generic command execution bounds, and test-only unimplemented stubs; current service internals do not consume fs/git traits until later lifecycle migration tasks) +- `context/cli/capability-traits.md` (current broad CLI capability seam in `cli/src/services/capabilities.rs`, including `FsOps`/`StdFsOps`, `GitOps`/`ProcessGitOps`, git root/hooks resolution behavior, compile-time-typed borrowed AppContext wiring with scoped borrowing across directly awaited commands with associated-type narrow capability accessors plus `ContextWithRepoRoot` repo-root-scoped context derivation, generic command execution bounds, and test-only unimplemented stubs; current service internals do not consume fs/git traits until later lifecycle migration tasks) - `context/cli/service-lifecycle.md` (current compile-safe lifecycle seam in `cli/src/services/lifecycle.rs`, including default no-op `ServiceLifecycle` diagnose/fix/setup methods against narrow `HasRepoRoot`, lifecycle-owned health/fix/setup result types with generic setup messages, doctor/setup adapter boundaries, the static `LifecycleProvider` enum catalog/dispatcher, hook/config/local_db/auth_db/agent_trace_db lifecycle providers including setup-time repository-scoped Agent Trace DB initialization plus checkout identity diagnostics, implemented doctor aggregation over diagnose/fix providers, and implemented setup aggregation over `setup` providers in order config → local_db → auth_db → agent_trace_db → hooks when requested) - `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` is invoked only through the runtime, and the runtime is now driven by the generic `sce hooks mutation-scope` CLI ingress and a first Claude Code adapter driver, registered by `sce setup` and reachable by a real Claude Code session, now proven against real Git repositories and a real Agent Trace DB by the `claude-mutation-scope-integration` plan's T08 regressions) - `context/cli/mutation-trace-runtime-materialization.md` (how the finite, pre-populated Quint `SCOPES`/`WORKTREES` universes map onto the Rust protocol's unbounded identity space: the coordinator/store layer materializes scope and worktree state, scope identity is immutable once materialized, and a missing scope is distinct from a `NeverSeen` scope; `protocol.rs` only transitions already-known identities) @@ -48,7 +50,7 @@ Feature/domain context: - `context/sce/cli-version-command-contract.md` (implemented `sce version` contract for deterministic human and machine-readable runtime identification) - `context/sce/cli-shell-completion-contract.md` (implemented `sce completion` contract for deterministic Bash/Zsh/Fish completion script generation) - `context/sce/claude-raw-hook-capture.md` (removed feature: the former `sce hooks claude-capture` raw-capture route and its supporting types, replaced by the active `diff-trace` and `conversation-trace` intakes) -- `context/sce/cli-observability-contract.md` (implemented config-backed runtime observability contract for the flat logging config-file shape with explicit config-file/default `log_to_file`, `log_dir` / `SCE_LOG_DIR` env-over-config-over-`/sce/logs` fallback, append-only local-date/session log file routing with a one-time complete-record `-v2.log` fallback on primary open/append/flush failure, creation-triggered retention of direct regular `*.log` files to 10 entries, reliable producer-native diff-trace/conversation-trace session routing and hook-specific non-duplicated Agent Trace DB-open error events, deterministic session filename sanitization, concrete `Logger` behavior plus the `NoopTelemetry`-backed telemetry capability boundary, logger and object-safe telemetry trait boundaries, AppContext observability wiring, generic `RunOutcome` final rendering, runtime-classified repeated telemetry action protection, operator-facing `sce config show` observability reporting, and the trimmed `sce config validate` status-only validation surface) +- `context/sce/cli-observability-contract.md` (implemented config-backed runtime observability contract for the flat logging config-file shape with explicit config-file/default `log_to_file`, `log_dir` / `SCE_LOG_DIR` env-over-config-over-`/sce/logs` fallback, append-only local-date/session log file routing with a one-time complete-record `-v2.log` fallback on primary open/append/flush failure, creation-triggered retention of direct regular `*.log` files to 10 entries, reliable producer-native diff-trace/conversation-trace session routing and hook-specific non-duplicated Agent Trace DB-open error events, deterministic session filename sanitization, concrete `Logger` behavior plus the `NoopTelemetry`-backed telemetry capability boundary, logger and deliberately non-object-safe generic async telemetry boundaries with static capability dispatch and future-aware polling scope, AppContext observability wiring, generic `RunOutcome` final rendering, runtime-classified repeated telemetry action protection, operator-facing `sce config show` observability reporting, and the trimmed `sce config validate` status-only validation surface) - `context/sce/shared-context-code-workflow.md` (canonical `/next-task` task-synchronization lifecycle and validation-only `/validate` lifecycle, package-local phase references with single-skill control flow, and the task-synchronization-scoped `sce-decision` sibling invocation with ADR reuse/blocker propagation) - `context/sce/shared-context-plan-workflow.md` (canonical `/change-to-plan` workflow, package-local context-load/plan-authoring/template references, clarification/readiness gate contract, and one-task/one-atomic-commit task slicing) - [Context workflow rules](sce/context-workflow-rules.md) (canonical bootstrap, ongoing context maintenance, task synchronization, hygiene, discoverability, and feature-existence rules) diff --git a/context/decisions/2026-10-07-application-owned-async-command-runtime.md b/context/decisions/2026-10-07-application-owned-async-command-runtime.md new file mode 100644 index 000000000..5f722f0fc --- /dev/null +++ b/context/decisions/2026-10-07-application-owned-async-command-runtime.md @@ -0,0 +1,82 @@ +# Decision: Application-owned async command runtime + +Date: 2026-10-07 +Status: Accepted +Plan: `context/plans/cli-single-tokio-runtime-pr1.md` +Task: `T02` + +## Context + +Auth and sync previously drove async HTTP through their own current-thread +Tokio runtimes. Activating an application runtime while retaining those bridges +would nest runtimes. The synchronous Turso adapter separately owns runtimes; +its isolated `block_on` bridge protects reentry but runtime destruction still +requires a blocking region. + +## Decision + +Drive commands through directly awaited static dispatch under one +application-level multi-thread Tokio runtime, retaining synchronous Turso +adapter ownership until the separate PR2 persistence migration. + +## Rationale + +The connected app/auth/sync migration establishes one async execution owner +without erasing concrete capabilities or requiring borrowed command futures +to be `Send` or `'static`. Caller-side lifetime scopes accommodate existing +persistence without changing its APIs or policies. + +## Alternatives considered + +- **Activate the entrypoint before migrating commands** — nests auth/sync runtimes. +- **Convert persistence in the same change** — exceeds the PR1 migration boundary. +- **Box or spawn command futures** — obscures static dispatch and constrains + borrowed writers/context and non-`Send` sync progress unnecessarily. +- **Wrap every synchronous command in blocking scopes** — hides the specific + DB lifetimes requiring compatibility handling. + +## Compatibility and risks + +- CLI output, error classification, refresh, sync and foreground-hook contracts + remain unchanged. +- DB-capable Setup, Doctor and Hooks construct and destroy owning values inside + `block_in_place`; context-only Setup and DB-free hooks execute directly. +- Sync constructs storage in a blocking scope and owns it in a cleanup guard + whose blocking Drop runs on normal completion, error, cancellation or unwind. +- Credential operations cross awaited `spawn_blocking` boundaries. +- Generic async telemetry deliberately loses object safety; concrete associated + capability dispatch remains supported. Future telemetry must attach subscriber + scope to polling rather than hold a thread-local guard across awaits. + +## Guardrails + +- Keep synchronous parsing, rendering, lifecycle providers and ordinary command + implementations; introduce no blanket blocking dispatch or detached command. +- Leave DB runtime construction, isolated bridging and persistence policies intact. +- Retain borrowed dependencies until directly awaited command completion. + +## Consequences + +- Auth and sync own no production command runtime or `block_on` bridge. +- One application-level runtime does not mean one runtime object process-wide. +- Multi-thread Tokio is required for the temporary DB lifetime scopes. + +## Follow-up + +- T03/T04 establish focused auth, telemetry, output and cleanup regressions; + T05 reviews staged architecture documentation. +- PR2 converts persistence and removes DB-owned runtimes, bridging and DB-related + blocking scopes together; it is not implemented by PR1. + +## References + +- Plan: [CLI single Tokio runtime — PR1](../plans/cli-single-tokio-runtime-pr1.md) +- Task: `T02` +- Current-state context: [Architecture](../architecture.md), + [Capability traits](../cli/capability-traits.md), + [Observability](../sce/cli-observability-contract.md), + [Shared Turso adapter](../sce/shared-turso-db.md) +- Evidence: [Entrypoint](../../cli/src/main.rs), + [Static dispatch](../../cli/src/services/command_registry.rs), + [Sync orchestration](../../cli/src/services/sync/sync.rs) +- Related decision: [Historical CLI refactor draft](cli-refactor-decisions.md) diff --git a/context/glossary.md b/context/glossary.md index db8d0163a..cb456b320 100644 --- a/context/glossary.md +++ b/context/glossary.md @@ -1,4 +1,5 @@ # Glossary +- CLI execution terminology: [command dispatch, runtime dependencies and telemetry](cli/execution-glossary.md). - `pkl-check-generated`: Flake app exposed as `nix run .#pkl-check-generated`; canonical ephemeral-generation check that rejects committed target/schema/mirror outputs, evaluates exact workflow metadata, the generated artifact contract, semantic layout/path/inventory/content/parity/observational checks, and the optional-workflow manifest's content against the catalog, requires the shared helper-composition rule and SCE-scoped workflow prohibitions, enforces ordered catalog-derived OpenCode skill permissions plus explicit-permission artifact integrity, rejects stale sibling-package references or unresolved internalization tokens in workflow entrypoint `SKILL.md` documents, proves contract failures through checked-in negative fixtures, and delegates deterministic generation plus payload/input inventories to the generated-input producer while preserving its established inventory report. - `repo-level verification preference`: Current repository guidance that contributor-facing validation/check flows should prefer `nix flake check`; direct Cargo verification commands are secondary and used only when explicitly requested or for narrow targeted debugging, while `cargo fmt` remains the explicit autofix path. Required quick checks after each completed task in this repo are `nix run .#pkl-check-generated` and `nix flake check`. - disposable plan lifecycle: Policy where `context/plans/` holds active execution artifacts only; completed plans are disposable and durable outcomes must be reflected in current-state context files and/or `context/decisions/`. @@ -69,8 +70,6 @@ - `auth login stored-credential renewal`: The `sce auth login` behavior that first validates every stored credential through the existing non-forced token path, preserves valid credentials, refreshes expired credentials, and falls back to device authorization when credentials are absent or renewal fails. Renewal reports retain `login` labels in text and JSON; credential renewal is not exposed as a public subcommand. The auth command boundary treats missing-credential logout/whoami calls as successful state queries, classifies authenticated Control Plane failures as `UserError::NotAuthenticated`, token-storage plus `AuthError::Io`/`Storage` failures as `UserError::AuthStorageUnavailable` (`auth.storage_unavailable`), and approved user-facing rendering/prompt fallbacks as `UserError::UnexpectedFailure` (`general.unexpected_failure`); fixed catalog messages expose no storage or implementation details while technical sources remain available for structured observability, and remaining auth-command failures use `UnexpectedFailure` with preserved technical sources. - `command surface contract`: The current top-level command/help catalog split where `cli/src/cli_schema.rs` owns the real clap-backed command metadata (top-level purpose text plus help visibility for `auth`, `config`, `setup`, `doctor`, `hooks`, `policy`, `sync`, `version`, and `completion`) and `cli/src/command_surface.rs` consumes that catalog for the custom banner/help surface plus known-command classification, while still adding the synthetic `help` row. - `top-level help visibility metadata`: Per-command `show_in_top_level_help` metadata in `cli/src/cli_schema.rs` that controls whether a known command appears in `sce`, `sce help`, and `sce --help` without affecting direct invocation; the current hidden top-level commands are `hooks` and `policy`, while `auth` is visible, and `cli/src/command_surface.rs` renders the curated top-level help list from that shared metadata. -- `command loop`: The `clap` derive-based parser + dispatcher in `cli/src/cli_schema.rs`, `cli/src/services/parse/command_runtime.rs`, and `cli/src/app.rs` that routes `help`, `config`, `setup`, `doctor`, `auth`, `hooks`, `policy`, `sync`, `version`, and `completion`, executes implemented command flows, emits command-local help payloads for supported subcommand trees, resolves unknown command paths to the longest valid parent's help surface, and returns deterministic actionable errors for unknown options and other invalid invocation. -- `RuntimeCommand seam`: Internal static command-execution abstraction in `cli/src/services/command_registry.rs` where clap-parsed commands are represented as a `RuntimeCommand` enum with variants for `Help`, `HelpText`, `Version`, `Completion`, `Auth`, `Config`, `Setup`, `Doctor`, `Hooks`, `Policy`, and `Sync`. The enum owns `name()` and `execute(...)` dispatch, delegating behavior to service-owned command payload structs while avoiding boxed `dyn RuntimeCommand` handles. Parsed request construction lives in `cli/src/services/parse/command_runtime.rs` when user-provided options or subcommands are required. - `sce dependency baseline`: Current crate dependency set declared in `cli/Cargo.toml` (`anyhow`, `chrono`, `clap`, `clap_complete`, `dirs`, `hmac`, `indicatif`, `inquire`, `jsonschema`, `keyring-core`, `murmur3`, `owo-colors`, `rand`, `reqwest`, `serde`, `serde_json`, `sha2`, `tokio`, `tracing`, `turso`, `uuid`, plus target-specific keyring backends), validated through normal compile/test coverage. `[dev-dependencies]` now carries `quint-connect` (dev/test-only, for the in-progress mutation-cursor Quint Connect model-based-testing harness; see `context/plans/mutation-cursor-quint-connect.md`). - `progress reporter contract`: Library-independent, consumer-typed sync seam in `cli/src/services/sync/progress.rs` where `ProgressReporter` carries an arbitrary consumer event type, supports explicit successful finalization and closure-based collectors, and provides `NoopProgressReporter` for callers without human progress output. The sync-owned `SyncProgressEvent` in `cli/src/services/sync/sync.rs` carries lifecycle, accepted-batch, and stream-completion payloads for the three concurrent remote Agent Trace streams (`messages`, `parts`, `agent_traces`); `services::sync::progress` also owns the fixed `indicatif` terminal adapter and focused contract tests, while `sync/command.rs` selects text versus JSON behavior. No top-level `services::progress` module exists. - `remote sync streams`: The three Agent Trace streams `sce sync` uploads to the control plane — `messages`, `parts`, `agent_traces` — as distinct from the four local capture streams, which also include the local-only `diff_traces`. `streams.diffTraces` in `sce sync --format json` is a compatibility no-op entry echoing the server cursor, not a synchronized stream. See `context/cli/agent-trace-sync-command.md`. @@ -130,11 +129,8 @@ - `SCE_LOG_FILE`: Optional runtime env key for `sce` observability file sink path; when set, rendered observability lines are mirrored to this file path with parent-directory auto-create behavior. - `SCE_LOG_FILE_MODE`: Optional runtime env key controlling `SCE_LOG_FILE` write policy; allowed values are `truncate` and `append`, defaults to `truncate`, and requires `SCE_LOG_FILE`. - `SCE_LOG_DIR`: Optional runtime env key for `sce` observability log-directory configuration; when set, it overrides config-file `log_dir` and the `/sce/logs` default and must be non-empty. -- `logger trait boundary`: `services::observability::traits::Logger` mirrors the current observability logger API (`info`, `debug`, `warn`, `error`, `log_cli_error`) for generic command/runtime bounds and tests, with each method accepting `Option<&str>` session context for file routing; the concrete `services::observability::Logger` implements it and `NoopLogger` remains available for side-effect-free tests. -- `telemetry trait boundary`: `services::observability::traits::Telemetry` defines the `with_default_subscriber` command-lifecycle capability used by generic app-runtime bounds and tests. Production currently uses `NoopTelemetry`, which executes the action directly and installs no tracing subscriber; the boundary is retained for future real telemetry/OTEL integration. - `app startup phases`: Current `cli/src/app.rs` execution model that separates dependency checking, startup-context construction, runtime initialization, command parse/execute, and output rendering into named helpers while preserving the CLI's existing exit-code, stderr-diagnostic, and degraded-startup behavior; output rendering and execution-phase logging helpers live in `cli/src/services/app_support.rs`. - `RunOutcome`: Generic final render payload in `cli/src/services/app_support.rs` (`RunOutcome`) carrying a command result, optional startup diagnostic, and optional logger implementing the logger trait boundary. Production construction in `cli/src/app.rs` uses the concrete observability logger, while rendering is not hardcoded to that production type. -- `AppContext`: Generic borrowed dependency view in `cli/src/app.rs` passed through static command dispatch. `AppRuntime` owns the concrete production logger, `NoopTelemetry` (no tracing subscriber), filesystem, and git implementations; `AppContext` stores references to those dependencies plus an optional `repo_root: Option`, not owned `Arc` trait objects. Because it borrows from `AppRuntime`, `AppContext` is a lightweight, short-lived view and must not be stored long-term (e.g., in structs or across await points). The `repo_root` field is `None` at startup and command paths can derive a repo-root-scoped context with `AppContext::with_repo_root(...)` / `ContextWithRepoRoot`, preserving the borrowed runtime dependencies while attaching the resolved root. Narrow accessor traits (`HasLogger`, `HasTelemetry`, `HasFs`, `HasGit`, `HasRepoRoot`) let command and lifecycle call sites express capability requirements without depending on the full production context type; logger/telemetry/fs/git accessors use associated concrete capability types and return `&Self::{Capability}` rather than object-erased `&dyn ...` values. - `CommandRegistry`: Static command-name catalog in `cli/src/services/command_registry.rs`; populated by `build_default_registry()` and carried by `AppRuntime` during command dispatch. It exposes deterministic command-name membership for the current top-level command catalog (`help`, `auth`, `config`, `setup`, `doctor`, `hooks`, `policy`, `sync`, `version`, and `completion`) while actual parsed command payloads are represented by the `RuntimeCommand` enum rather than zero-arg boxed constructors. - `ServiceLifecycle`: Compile-safe lifecycle trait seam in `cli/src/services/lifecycle.rs` with default no-op generic `diagnose`, `fix`, and `setup` methods over `C: HasRepoRoot`; it exposes lifecycle-owned health, fix, and setup result types, while doctor/setup adapt those records at orchestration boundaries before rendering command-owned output. The hooks service has `HooksLifecycle` for hook rollout diagnosis/fix/setup, the config service has `ConfigLifecycle` for global/repo-local config validation plus repo-local config bootstrap, local_db has `LocalDbLifecycle` for canonical local DB path health/bootstrap/setup, auth_db has `AuthDbLifecycle` for canonical auth DB path health/bootstrap/setup, and agent_trace_db has `AgentTraceDbLifecycle` for repository identity resolution, checkout identity setup for diagnostics, setup-time repository-scoped Agent Trace DB initialization, and repository Agent Trace DB path health/bootstrap, returning an actionable "requires a Git repository" diagnostic outside repository context (the former global parent fallback was removed by the `retire-legacy-agent-trace-db` plan). Doctor runtime aggregates the static provider catalog for `diagnose` and `fix`; setup command aggregates providers for `setup` in order (config → local_db → auth_db → agent_trace_db → hooks when requested). - `lifecycle provider catalog`: Shared factory in `cli/src/services/lifecycle.rs` (`lifecycle_providers(include_hooks)`) that returns static `LifecycleProvider` enum values in deterministic config → local_db → auth_db → agent_trace_db → hooks order, used by doctor with hooks included and by setup with hooks included only when requested. The enum owns concrete-provider dispatch for `id`, `diagnose`, `fix`, and `setup` without boxed provider trait objects or `&dyn HasRepoRoot` lifecycle context erasure. diff --git a/context/overview.md b/context/overview.md index 57f262c22..d0917981f 100644 --- a/context/overview.md +++ b/context/overview.md @@ -66,6 +66,7 @@ The current doctor presentation contract supersedes the earlier output-shape sca The command loop now enforces a stable exit-code contract in `cli/src/app.rs`: `2` parse failures, `3` invocation validation failures, `4` runtime failures, and `5` dependency startup failures. Auth command orchestration routes authenticated Control Plane authentication failures, token-storage failures, and approved user-facing fallback failures through typed `CliError::User` catalog entries (`NotAuthenticated`, `AuthStorageUnavailable`, `NotGitRepository`, and `UnexpectedFailure`) while retaining technical sources for observability; missing-credential logout/whoami state queries succeed with their documented text/JSON reports, and internal auth failures remain runtime errors. The same runtime also emits stable CLI stderr diagnostics: internal failures use `SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, or `SCE-ERR-DEPENDENCY` in deterministic `Error []: ...` diagnostics with class-default `Try:` remediation appended when missing, while expected failures emit only their redacted, unstyled catalog messages. The command boundary's former flat, string-only `ClassifiedError` has been replaced by typed `CliError` in `cli/src/services/error.rs`: `CliError::User` carries a closed catalog (`NotAuthenticated`, `NotGitRepository`, `NotGitRemote`, authentication-storage `AuthStorageUnavailable`, and general `UnexpectedFailure`) for expected, deliberately-explained failures rendered without the technical source, wrapper, styling, or automatic `Try:` guidance; the general entry renders one fixed static log-files guidance sentence without dynamic path text. `CliError::Internal` carries a live `anyhow::Error` source rendered as the real error chain with the existing styled wrapper and class-default remediation; `app_support` is the sole owner of the distinct terminal paths, and `sce sync` classifies authentication, credential-storage, and all other sync failures as cataloged user errors while preserving their technical sources for observability. Setup preflight errors preserve technical sources, including the configured remote name, while raw remote URLs remain out of user-facing diagnostics. See `context/sce/cli-error-code-taxonomy.md` for the full contract. The app runtime now also includes a structured observability baseline in `cli/src/services/observability.rs`: deterministic env-controlled log threshold/format (`SCE_LOG_LEVEL` defaults to `error`; `SCE_LOG_FORMAT` defaults to `text`), default-backed log-directory routing (`SCE_LOG_DIR` / config-file `log_dir` / `/sce/logs`) with per-operation machine-local dated file selection, optional session filename partitioning, and creation-triggered retention of direct regular `*.log` files to 10 entries, stable lifecycle event IDs, normal logger records routed only to files when file logging is enabled and only error-level logger records routed to `stderr` when file logging is disabled, direct file-write diagnostics remaining on `stderr`, and `observability::traits` boundaries for logger and telemetry behavior. +One application-level multi-thread Tokio runtime directly awaits static command dispatch and auth/sync HTTP before output rendering. Ordinary synchronous commands remain direct; DB-backed paths use temporary blocking lifetime scopes while Turso remains synchronous with DB-owned runtimes. See [application execution runtime](architecture.md#application-execution-runtime). The app command dispatcher now enforces a centralized stdout/stderr stream contract in `cli/src/app.rs`: command success payloads are emitted on stdout only, while redacted user-facing diagnostics and text-mode sync progress are emitted on stderr; JSON sync remains silent. `cli/src/app.rs` also now runs through explicit startup phases — dependency check, observability config resolution, runtime initialization, command parse/execute, and output rendering — with the app runtime carrying logger/telemetry plus static command-catalog state across those phases while preserving the existing exit-code and degraded-startup contracts. Within that lifecycle, `parse_command_phase` delegates clap-to-runtime conversion to `cli/src/services/parse/command_runtime.rs`, which returns a static `RuntimeCommand` enum, `services::app_support::execute_command_phase` logs around enum-owned `execute_with_stderr(...)` dispatch, and generic `RunOutcome` rendering logs classified errors through the logger trait boundary without coupling render support to the production logger type. Command payload structs for `help`, `version`, `completion`, `auth`, `config`, `setup`, `doctor`, `hooks`, and `sync` live in service-owned `command.rs` files; `cli/src/services/command_registry.rs` owns the deterministic static command-name catalog and enum variants instead of boxed command trait objects. The CLI now also enforces a shared output-format parser contract in `cli/src/services/output_format.rs`, with canonical `--format ` parsing and command-specific actionable invalid-value guidance reused by `config` and `version` services. A compile-safe service lifecycle seam also exists in `cli/src/services/lifecycle.rs`: `ServiceLifecycle` exposes default no-op `diagnose`, `fix`, and `setup` methods against the narrow `HasRepoRoot` accessor, uses lifecycle-owned health/fix/setup result types, and owns the shared static `LifecycleProvider` enum catalog/factory with deterministic config → local*db → auth_db → agent_trace_db → hooks ordering and no boxed provider aggregation. Hooks has a `services/hooks/lifecycle.rs` provider for hook rollout diagnosis/fix/setup, config has a `services/config/lifecycle.rs` provider for global/repo-local config validation plus repo-local config bootstrap, local_db has a `services/local_db/lifecycle.rs` provider for canonical local DB path health, parent-directory readiness/bootstrap, and `LocalDb::new()` setup, auth_db has a `services/auth_db/lifecycle.rs` provider for canonical auth DB path health, parent-directory readiness/bootstrap, and `AuthDb::new()` setup, and agent_trace_db has a `services/agent_trace_db/lifecycle.rs` provider for repository-scoped Agent Trace DB setup and repository DB path health/parent readiness from resolved repository identity, returning an actionable "requires a Git repository" diagnostic outside repository context (no global/checkout fallback path). Doctor runtime aggregates the full shared provider catalog for `diagnose` and `fix` and adapts lifecycle records into doctor-owned output records; setup command aggregates the shared provider catalog for `setup` with hooks included only when requested and adapts lifecycle setup outcomes before rendering setup-owned messages. Agent Trace lifecycle setup now resolves repository storage and initializes `/sce/repos//agent-trace.db` via `RepositoryAgentTraceDb`; hook runtime lazy initialization uses the same repository storage resolver when setup has not prepared the DB or schema metadata is incomplete. diff --git a/context/patterns.md b/context/patterns.md index ffb4a6893..8ab42f2eb 100644 --- a/context/patterns.md +++ b/context/patterns.md @@ -1,5 +1,9 @@ # Patterns +## Awaited command execution + +- Keep borrowed context and writers alive through directly awaited static dispatch. Scope blocking only around documented DB construction/destruction lifetimes; isolate credential operations with awaited owned `spawn_blocking` closures. See [application execution runtime](architecture.md#application-execution-runtime). + ## Config generation tooling - Use the Nix dev shell as the canonical toolchain entrypoint for generation work. diff --git a/context/plans/cli-single-tokio-runtime-pr1.md b/context/plans/cli-single-tokio-runtime-pr1.md new file mode 100644 index 000000000..7c3c664c9 --- /dev/null +++ b/context/plans/cli-single-tokio-runtime-pr1.md @@ -0,0 +1,373 @@ +# Plan: CLI single Tokio runtime — PR1 + +## 1. Current-state findings + +This PR changes the execution model: one application-level Tokio runtime drives directly awaited command execution. It removes auth/sync command runtime ownership while retaining synchronous service lifecycles and the existing Turso adapter's DB-owned runtimes until PR2. It does not promise exactly one `tokio::runtime::Runtime` object anywhere in the process. The repository does not already provide this boundary. The smallest useful migration is the app/dispatch/auth/sync call chain; converting persistence or every command would increase scope without satisfying an additional PR1 requirement. + +Inspection baseline: `cli/src/main.rs`, `app.rs`, `services/app_support.rs`, `services/command_registry.rs`, `services/observability/traits.rs`, `services/auth_command/{command.rs,mod.rs}`, `services/auth.rs`, `services/sync/{command.rs,sync.rs,progress.rs,render_sync.rs,auto_sync.rs}`, `services/agent_trace_sync/{mod.rs,control_plane.rs,test_http_server.rs}`, `services/db/mod.rs`, `cli/Cargo.toml`, `cli/Cargo.lock`, `nix/flatpak/cargo-sources.nix`, `flake.nix`, and `.sce/config.json`, informed by the loaded durable-context brief. + +- `main` calls synchronous `app::run`. The app separates dependency checking, startup resolution, concrete dependency initialization, command parsing/execution, and final output rendering. `try_run_with_dependency_check` currently embeds command execution in a synchronous `Result::map` closure. +- `AppRuntime` owns logger, `NoopTelemetry`, filesystem, Git, catalog and startup diagnostic. `AppContext` borrows the concrete dependencies. Capability traits use associated types; `ContextWithRepoRoot` preserves them. `RuntimeCommand` is an enum; `CommandRegistry` supplies deterministic names. `LifecycleProvider` is also static enum dispatch. None requires type erasure. +- `run_command_lifecycle` wraps dispatch with `Telemetry::with_default_subscriber`, currently a synchronous `&mut dyn FnMut() -> Result`. `NoopTelemetry` is the production implementation. Arguments are taken from an `Option` so repeated invocation returns `REPEATED_COMMAND_DISPATCH_ERROR`. The wrapper must await execution rather than return an unpolled future after its scope ends. +- The current telemetry callback is object-safe, but production selects concrete `NoopTelemetry` through `HasTelemetry::Telemetry`. Source inspection finds no active `dyn Telemetry`, boxed or `Arc` telemetry consumers. Introducing a generic future callback deliberately changes trait object safety; it does not change today's concrete dispatch. +- Auth has `AUTH_RUNTIME`, a `OnceLock`, and `shared_runtime()` constructing a current-thread runtime. `run_login`, renewal/device helpers and `run_whoami` call `block_on` over already-async WorkOS/Control Plane operations. Logout and render/config/storage helpers are synchronous. +- Sync has `SYNC_RUNTIME`, `OnceLock`, `shared_runtime()`, and `runtime.block_on(run_sync_async(...))`. Eight orchestration wrappers, including test-only wrappers, propagate into this bridge. The underlying stream engine is already async. +- Sync makes one initial `/state` call, then polls messages, parts and agent_traces concurrently through `try_join_three`. It retains `Rc>` for borrowed progress; the future is intentionally not required to be `Send`. Each stream processes batches sequentially. `diff_traces` is a local compatibility report entry that echoes the server cursor, not a fourth uploaded stream. +- `AuthenticatedControlPlaneClient::load_credentials` and `save_credentials` are the two production `spawn_blocking` sites. They isolate synchronous storage/keyring work; refresh locking and HTTP remain async. Auth's existing direct `token_storage` calls are synchronous and its auth DB singleton is separate persistence infrastructure. +- `services/db/mod.rs` owns `build_current_thread_runtime`, `block_on_isolated`, and the synchronous `TursoDb`/`EncryptedTursoDb` APIs. The bridge detects `Handle::try_current()` and invokes its runtime on a fresh scoped thread when called inside Tokio. PR1 makes that path more common. `TursoCore` also owns the adapter runtime with no custom blocking Drop. Inspection of the pinned Tokio 1.52.3 source (`runtime/blocking/pool.rs:263`, `runtime/blocking/shutdown.rs:37–52`) shows runtime shutdown calls `wait(None)`, which requires a blocking region and panics inside async execution even without spawned blocking workers. Therefore directly dropping command-local DB storage in the new async lifecycle is a concrete incompatibility, not just a speculative risk. Caller-side construction/destruction boundaries are required while DB production remains unchanged. +- Direct Tokio features are currently `rt`, `io-util`, `sync`, `time`, with `default-features = false`. The lockfile pins Tokio 1.52.3 and does not currently include `tokio-macros`. Adding the entrypoint macro also affects checked-in Flatpak Cargo sources and their fixed-output hash. +- Existing sync orchestration tests live inline in `services/sync/sync.rs`; control-plane tests have a test-only `block_on` helper and `RuntimeCheckingCredentialStore`. Stream engine tests also have test-only runtimes. Those test fixtures are not production runtime ownership. `app.rs` currently has no inline tests; `app_support.rs` tests centralized error rendering but not the complete command boundary. +- Existing post-commit auto-sync deliberately launches a detached child process. Preserve this behavior; no other hook becomes a background Tokio task. +- `.sce/config.json` prefers `nix flake check` over direct Cargo verification and requires ad-hoc utilities through Nix. Targeted Rust tests, when necessary, use `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml ...`. Generated assistant output must remain ephemeral. +- Current `flake.nix:1514–1529` explicitly separates normal `nix flake check` from `nix build .#ci-checks`: the latter forces the release build (static musl on Linux, native on Darwin), plus Linux portability validation of the real release binary. `.github/workflows/pr-ci.yml:81` builds this aggregate. The loaded context's older description of flake checks alone as complete validation is stale; PR1's Tokio/lock/Flatpak/native build changes require both tiers. + +The loaded context reports relevant drift: `context/architecture.md` describes four concurrently uploaded streams and omits the already-present Tokio `sync` feature. `context/cli/capability-traits.md` prohibits borrowing across await points without distinguishing scoped borrowing. Correct these touched descriptions in the documentation task; avoid unrelated metadata cleanup. + +## 2. Architectural decisions + +1. Use `#[tokio::main(flavor = "multi_thread")] async fn main() -> ExitCode { app::run(std::env::args()).await }`. Add only `macros` and `rt-multi-thread` to the existing explicit Tokio feature list. Keep defaults disabled; do not add `full`, tune worker counts, or add a scheduler library. +2. Await the complete command before rendering/returning. Keep startup computations, parsing, formatting, output writers, domain operations and synchronous command implementations synchronous wherever they do not propagate async work. +3. Keep direct enum dispatch. Await auth/sync directly. Execute ordinary synchronous arms directly from the async caller, including Config after inspecting its dependency graph. Use `tokio::task::block_in_place` only for the DB-capable paths classified below, so legacy runtime-backed values are constructed and destroyed in a blocking region without requiring `Send`, `'static`, or detachment. Do not blanket synchronous commands, add `async-trait`, new `BoxFuture`, command trait objects or a `Send` requirement. Existing stream-level `SyncFuture` is outside command dispatch and remains unchanged. +4. PR1 intentionally drops object safety from the `Telemetry` capability. The current production architecture uses static dispatch through `AppRuntime`, `AppContext`, and `HasTelemetry::Telemetry`, and there are no active `dyn Telemetry` consumers. The future OTel implementation is expected to be statically dispatched as well. Adapt `with_default_subscriber` to a statically dispatched async method taking generic `FnMut() -> Fut`, with `Fut: Future>`; `NoopTelemetry` awaits once. Keep the wrapper around future polling, the repeat guard, error classification and lifecycle event ordering. Do not introduce boxed futures or dynamic dispatch to preserve historical object safety; do not implement OTel, a new exporter, subscriber installation or new logging behavior. + + **Async subscriber scoping:** A future real `Telemetry` implementation must scope tracing to the command future itself, not hold a thread-local subscriber guard across `.await`. On a multi-thread Tokio runtime, a future may be polled on different worker threads, so `tracing::subscriber::with_default` / `set_default` are not sufficient as an async execution scope. Prefer tracing's future-aware subscriber attachment (`tracing::instrument::WithSubscriber::with_subscriber(...)`) or an intentionally installed process-global subscriber when appropriate. PR1 still uses `NoopTelemetry`; it only changes the capability shape so awaited command execution is enclosed by the telemetry abstraction. + +5. Preserve scoped borrowed DI. `&AppContext` through a directly awaited command is sound: `AppRuntime` outlives the context and the future; dispatch does not detach or require `'static`. `#[tokio::main]` does not require its main future to be `Send`. Generic `&C` and `&mut W` arguments may be borrowed until completion without new `Send + Sync` bounds. Resolve a repository root before sync I/O and pass the resulting path and needed dependencies, rather than storing the context in orchestration state. Update the blanket await prohibition to this precise scoped rule. Introduce explicit lifetimes only if a compiler error demonstrates a need. +6. The telemetry closure needs a single concrete returned future without escaping reborrows of captured mutable writers. Put both collected arguments and the mutable stderr reference in `Option`s, take them at closure invocation, then construct an `async move` block. Copy shared runtime/context references into the block. A second call returns the same runtime-classified repeat error; preserve its existing log behavior and avoid panics. Do not replace `FnMut` with `FnOnce` and lose the guard. +7. Auth callback helpers should accept generic futures. For `run_login_with_stored_credentials`, pass owned `StoredTokens` to the renewal callback, then borrow within its `async move` future. This avoids a future-returning closure tied to a short-lived callback argument without HRTB boxing or token cloning. Logout becomes an async orchestration wrapper solely to await its synchronous credential operation. +8. Retain both control-plane `spawn_blocking` calls and their awaited join/error mapping. Wrap auth's direct `token_storage::load_tokens`, `save_tokens`, and `delete_tokens` call sites in awaited owned `spawn_blocking` closures too, preserving the synchronous token-storage API and existing storage error classification; map a failed blocking join through existing unexpected-failure handling. Move tokens into save closures rather than retaining borrowed callback input. This isolates actual keyring/runtime resources and is a narrow caller adaptation, not a storage redesign. +9. Leave DB production code and policies unchanged, including `build_current_thread_runtime()`, `block_on_isolated()` and DB-owned runtime construction. These adapter runtimes are an explicit exception to one **application-level** runtime. Sync must construct storage inside `block_in_place`, retain borrowed reader access across async HTTP, and destroy storage inside a caller-side blocking scope after the awaited result, including error returns. Keep ownership in a small private concrete sync-side cleanup guard installed before subsequent fallible work; take/drop storage inside its blocking Drop scope on explicit normal cleanup, early return, cancellation or unwind. Prefer one cleanup implementation over separate normal/error/drop call sites. No DB API, runtime helper or retry changes. Async tests must similarly construct/drop their DB fixtures within blocking regions. PR2 removes async Turso's obsolete DB-owned runtime/bridge and these DB-related workarounds together. + +### Command dependency classification and blocking-scope inventory + +The classification is based on current production callees, not the fact that a method is synchronous. DB-owning locals must be created and dropped within each indicated scope, including cleanup on error. Existing command/service APIs remain synchronous. + +| Variant / path | PR1 mechanism | Existing dependency evidence and reason | +| --- | --- | --- | +| Help, HelpText, Version, Completion, Policy | Direct synchronous call | `command_registry.rs:72–81` calls text rendering, version rendering, completion rendering or policy processing; these do not construct Turso adapters or own Tokio runtimes. | +| Config (Show and Validate) | Direct synchronous call | `config/command.rs` → `config/mod.rs:25–38` → `resolve_runtime_config` and show/validate renderers. File/config resolution is synchronous but does not open DBs or own runtimes. No async filesystem rewrite or blocking island is justified. | +| Auth | Direct await | Existing async WorkOS/control-plane HTTP; actual credential/keyring operations stay in awaited `spawn_blocking` closures. | +| Sync | Direct await, with only the two storage lifetime scopes below | Existing async stream orchestration; DB-owning `AgentTraceStorage` outlives HTTP awaits. | +| Setup | One scoped `block_in_place` around its existing execute call | `setup/command.rs` iterates `lifecycle_providers`; LocalDb/AuthDb setup constructs and drops `TursoDb`/`EncryptedTursoDb` (`local_db/lifecycle.rs:52`, `auth_db/lifecycle.rs:52`), and AgentTraceDb setup resolves owning storage (`agent_trace_db/lifecycle.rs:95`). The execute scope contains those lifetimes and error cleanup. `context_only` exits before providers; that branch should execute directly using the existing request flag. | +| Doctor (Diagnose and Fix) | One scoped `block_in_place` around its execute call | `doctor/mod.rs:run_doctor_with_context` → `execute_doctor_with_lifecycle_providers` → synchronous diagnose/fix providers. AgentTraceDb diagnose opens a command-local `RepositoryAgentTraceDb` (`agent_trace_db/lifecycle.rs:157`) even without `--fix`; fix/setup can construct more adapters. Both modes require safe local runtime destruction. | +| Hooks: PreCommit and PostRewrite | Direct synchronous call | `commit_hooks.rs:32–43,524–544` are no-op state/config/Git/stdin handlers. Their misleading `with_trace` names do not imply database access. | +| Hooks: CommitMsg, PostCommit, DiffTrace, ConversationTrace, Codex, ClaudeModelState, MutationScope, ClaudeMutationScope, CodexMutationScope, OpenCodeMutationScope, PiMutationScope, ExternalMutationGuard | One scoped `block_in_place` around the existing execute call for DB-capable subcommands | CommitMsg conditionally calls `staged_diff_has_ai_overlap`, which opens a DB; PostCommit trace/intersection/checkpoint opens owning adapters (`commit_hooks.rs:184–199,306,339`). Trace handlers open DBs (`diff_trace.rs:481`, `conversation_trace.rs:127`, Codex stop/user-prompt handlers and `claude_model_state.rs:147`); mutation-scope adapters persist through existing runtime seams, including ExternalMutationGuard (`mutation_scope.rs:516–527`). Keep the whole synchronous invocation in scope so fail-open/error cleanup and resource destruction remain foreground. Do not add separate per-adapter wrapping. | +| Sync storage constructor | One `block_in_place` in `sync/sync.rs` | `resolve_agent_trace_storage` constructs the adapter runtime and can drop partially built DB resources on failure. Immediately transfer successful storage into the concrete cleanup guard. | +| Sync storage cleanup guard Drop | One `block_in_place` in `sync/sync.rs` | The guard owns the DB runtime across awaits; its scope takes and drops storage safely on every exit. Explicitly drop the guard after the awaited operation result, so no command returns before storage cleanup. | + +This is the production call-site inventory: Setup (DB-capable request), Doctor, DB-capable Hooks, sync construction, sync guard Drop. Request/subcommand classification can live beside enum dispatch and must be exhaustive over the current types. Any additional production `block_in_place` introduced during implementation requires its own existing runtime-backed dependency and lifetime justification here. Ordinary filesystem, Git, parsing and rendering do not by themselves justify one. Test-only fixture scopes are identified separately. PR2 deletes the DB-related scopes after converting Turso; PR1 leaves the DB bridge intact. + +Assumptions: use default multi-thread worker configuration, existing Rust generic async support and current repository test fixtures. No new library or public CLI option is needed. Open questions: None; the request supplies the migration boundary and behavioral constraints. + +## 3. Scope + +In scope: entrypoint, minimal Tokio/lock/Flatpak metadata, async app and execution plumbing, narrow telemetry callback signature adaptation, auth/sync orchestration signatures and awaits, required caller-side DB/keyring blocking scopes, related regression tests, and current architecture context. + +Atomic migration requirement: the first commit that enables the top-level runtime must also remove auth/sync `block_on` ownership and propagate all necessary signatures. Landing `main` first with the old command bridges would cause nested-runtime panics. Dependency preparation can precede this atomic boundary. + +## 4. Explicit non-goals + +- No async Turso API, persistence migration, changes to `block_on_isolated` or DB runtime construction, retry/deadline/busy-timeout adjustments, contention behavior changes, transaction replay changes, WAL changes or migration changes. +- No async `LifecycleProvider`/`ServiceLifecycle` APIs, generic conversion of every service, async filesystem/Git rewrite, new background hook tasks, sync scheduling redesign or cancellation policy. +- No changed WorkOS refresh rules, credential formats, authentication error behavior, HTTP retries, cursor authority, reconciliation policies, telemetry implementation or progress design. +- No dynamic DI/command dispatch, `async-trait`, `BoxFuture` command architecture, replacement of `AppRuntime`/`AppContext`, global Tokio handle singleton, command-owned runtime convenience helper, or additional application-level runtime. +- No OpenTelemetry implementation, new telemetry exporter, blanket `block_in_place` dispatch or new integration-test framework. Existing domain suites remain authoritative; additions prove changed execution/lifetime boundaries. +- No new flags, output/help/error strings, JSON fields, exit codes or user-visible behaviors. No unrelated generated config edits or context cleanup. No implementation, tests execution or commits during plan authoring. + +## 5. Ordered implementation tasks + +### Task context synchronization lifecycle + +Every task carries `pending | synced | blocked`. A completed task must be `synced` before another task starts or the plan finishes. Record transitions in this file. For `blocked`, persist **Blocker**, **Required action**, and **Retry condition** next to the status; never infer synchronization from chat history. Task-local verification is part of each change; final full validation is specified under section 7 rather than a separate task. + +- [x] T01: `Prepare minimal Tokio features and packaging parity` (status:done) + - Task ID: T01 + - Goal: Make the existing build support one macro-based application-level multi-thread runtime. + - Files likely affected: `cli/Cargo.toml`, `cli/Cargo.lock`, `packaging/flatpak/cargo-sources.json`, `nix/flatpak/cargo-sources.nix`. + - Scope: In — required features and resulting lock/package metadata. Out — entrypoint activation, version upgrades or unrelated lock churn. + - Exact change: Retain `default-features = false` and `rt`, `io-util`, `sync`, `time`; add `macros`, `rt-multi-thread`. Resolve only the necessary lockfile additions using Cargo through the Nix dev shell. Regenerate Flatpak Cargo sources with `nix run .#regenerate-cargo-sources`; refresh the generator fixed-output hash in `nix/flatpak/cargo-sources.nix` from the regenerated derivation's reported expected hash when required. Keep the existing generator pipeline; do not hand-edit crate entries or replace the pinning scheme. + - Behavioral invariants: No execution behavior changes; no broad features, crate upgrades, config generation output or CLI contracts change. + - Done when: The macro/multi-thread features and minimal lock changes are available, and Flatpak metadata matches the lockfile. + - Verification: `nix develop -c ./scripts/run-cli-cargo.sh build --manifest-path cli/Cargo.toml`; inspect dependency diff for only required additions; build `.#checks.x86_64-linux.cargo-sources-parity` on the repository's Linux validation platform. + - Dependencies: none. + - Completed: 2026-10-07 + - Files changed: `cli/Cargo.toml`, `cli/Cargo.lock`, `packaging/flatpak/cargo-sources.json`, `nix/flatpak/cargo-sources.nix`, `context/plans/cli-single-tokio-runtime-pr1.md`. + - Result: Added only Tokio `macros` and `rt-multi-thread`, retaining disabled defaults and existing features. Cargo added `tokio-macros 2.7.2` and its `syn 3.0.6` dependency; existing versions/checksums remain unchanged, with required `syn 2.0.118` disambiguation. Regenerated Flatpak sources through the existing app and refreshed its reported fixed-output hash to `sha256-ZLcI8Basvo3MrOan7XnIUqq8UUFQkijzLGw/So3RFi0=`. Entrypoint remains synchronous. + - Verify: Passed — `nix develop -c ./scripts/run-cli-cargo.sh build --manifest-path cli/Cargo.toml` (dev build completed); baseline/current lockfile comparison using Nix Python confirmed only the two required additions and no existing package upgrades/checksum changes; `nix build .#checks.x86_64-linux.cargo-sources-parity` succeeded; `git diff --check` passed. + - Context impact: Build/dependency documentation only: explicit Tokio features now include macro and multi-thread support, without activating the application runtime. No execution, persistence, or CLI contract changes. + - Context synchronization: synced + - Context synchronization evidence: Updated `context/architecture.md` to describe the explicit feature set and unchanged synchronous entrypoint/runtime ownership; verified `context/overview.md`, `context/glossary.md`, `context/patterns.md`, and `context/context-map.md` against the dependency-only change. Existing Flatpak generation guidance remains accurate. No architecture decision qualified; enabling features is reversible build preparation. Architecture remains 220 lines; `git diff --check` passed. + +- [x] T02: `Migrate the complete app, dispatch, auth and sync execution boundary atomically` (status:done) + - Task ID: T02 + - Goal: Auth/sync async I/O reaches the one application-level runtime through static dispatch. + - Files likely affected: `cli/src/main.rs`, `app.rs`, `services/app_support.rs`, `services/command_registry.rs`, `services/observability/traits.rs`, `services/auth_command/{command.rs,mod.rs}`, `services/sync/{command.rs,sync.rs}`; narrowly affected test callers discovered by signature search. + - Scope: In — one connected signature/await migration and existing caller/test adaptation. Out — new domain behavior, persistence changes or stream refactors. + - Exact change, app: Make `main`, `app::run`, `run_with_dependency_check`, `run_with_dependency_check_and_streams`, `try_run_with_dependency_check`, `run_command_lifecycle`, `app_support::execute_command_phase`, `RuntimeCommand::execute`, and `RuntimeCommand::execute_with_stderr` async. Await their delegates. Apply section 2's dependency classification: direct synchronous Help/HelpText/Version/Completion/Policy/Config; direct Setup context-only and Hooks PreCommit/PostRewrite; direct awaits for Auth/Sync; narrowly guarded DB-capable Setup, Doctor and Hooks. Reuse their exact existing execute calls, with simple exhaustive request/subcommand classification and no domain branch rewrite. Keep each introduced blocking call's documented dependency/lifetime reason; do not wrap the entire synchronous match or add runtime/Handle block_on. Restructure the synchronous startup `Result` chain so startup errors retain their current `RunOutcome` and command execution can be awaited before moving `runtime.logger` into the outcome. Keep `perform_dependency_check`, `build_startup_context`, `initialize_runtime`, `parse_command_phase`, `render_run_outcome`, registry construction and all rendering helpers sync. + - Exact change, telemetry/context: Apply section 2's intentionally non-object-safe generic async telemetry signature to `Telemetry::with_default_subscriber` and concrete `NoopTelemetry`, preserving static `HasTelemetry::Telemetry` selection. Preserve single-use argument/stderr extraction and event order; await parsing/execution inside its action future. Retain context/runtime ownership through completion and update the `AppContext` doc comment to allow scoped awaited borrowing. Do not spawn the command, add erased futures, preserve historical object safety through boxing, or implement OTel. + - Exact change, auth: Make `AuthCommand::execute`, `run_auth_subcommand`, `run_auth_subcommand_with`, `run_login`, `run_logout`, `run_whoami`, `maybe_renew_stored_credentials`, `run_login_with_stored_credentials`, `run_text_login_with_runtime` and `run_login_json` async. Rename the now runtime-free text helper to `run_text_login`; remove its runtime argument and every other auth runtime argument. Replace existing `block_on` calls with awaits on the same operations, with the same error mapping and storage/prompt ordering. Adapt injected dispatch callbacks to `FnOnce(AuthFormat) -> Fut` (separate generic future types), renewal/device callbacks similarly, with owned renewal input as described above. Await owned `spawn_blocking` closures around existing direct load/save/delete operations, preserving storage errors and mapping join failures through existing unexpected-auth-error classification. Keep token storage APIs, config resolution, prompt writes and renderers sync. Delete `AUTH_RUNTIME`, its `OnceLock` import and `shared_runtime()` only. + - Exact change, sync: Make `SyncCommand::execute`, `execute_with_stderr`, `execute_with_stderr_and_clock` async. Await sync before `finish_successfully` and final rendering. Make `run_current_sync`, `run_current_sync_with_progress`, `run_current_sync_with_progress_and_clock`, `run_current_sync_without_progress`, `run_sync_against`, `run_sync_against_with_progress`, `run_sync_against_with_progress_and_clock`, `run_sync_against_without_progress` async; await delegates. Replace `runtime.block_on(run_sync_async(...))` with `run_sync_async(...).await`. Remove `SYNC_RUNTIME`, `shared_runtime()` and unused runtime/OnceLock imports. Leave `run_sync_async`, `sync_one_stream`, `try_join_three`, `Rc>`, state acquisition, cursor classification, retries and progress ordering otherwise unchanged. + - Exact change, sync storage lifetime: Run `resolve_agent_trace_storage` within its individually justified `block_in_place` scope, including its failure cleanup. Own successful storage immediately in a private concrete caller-side cleanup guard before any subsequent fallible setup/await, exposing borrowed metadata/DB to unchanged orchestration. Evaluate the complete async operation result, then explicitly drop the guard before returning it. Guard Drop takes and drops storage in the second individually justified blocking scope; reuse this cleanup on normal completion, early return, unwind and cancellation rather than duplicating scopes. Do not change storage or DB API types, retries, `build_current_thread_runtime`, `block_on_isolated`, DB-owned runtime or stream futures. + - Exact change, tests: Convert inline sync tests that call migrated wrappers to `#[tokio::test(flavor = "multi_thread")] async fn` and await calls. Construct and destroy their owned DB fixtures via `block_in_place`/the test cleanup scope so runtime shutdown is safe; do not disable failing fixture cleanup. Retain every assertion and all fixtures. Adapt any existing injected auth callbacks with ready async closures. Pure renderer/classification/parser tests remain synchronous. Do not wrap async command calls in a newly constructed runtime while already in an async test. + - Behavioral invariants: All CLI contracts; static DI/enums; one initial state call; three concurrent remote streams with sequential batches; compatibility diff report; awaited hooks; subscriber scope, repeated-dispatch error and log order; unchanged auth refresh/persistence and control-plane blocking boundaries. + - Done when: App execution is directly awaited; auth/sync contain no production runtime construction/block_on helper; every existing caller compiles and retains assertions; DB production diff is empty. + - Verification: Build through `scripts/run-cli-cargo.sh` under Nix. Run focused `services::sync::sync::tests` and `services::auth_command::tests` via the same wrapper. Inspect the runtime cleanup searches in section 7 and unchanged DB/control-plane production diffs. + - Dependencies: T01. + - Completed: 2026-10-07 + - Files changed: `cli/src/main.rs`, `cli/src/app.rs`, `cli/src/services/app_support.rs`, `cli/src/services/command_registry.rs`, `cli/src/services/observability/traits.rs`, `cli/src/services/auth_command/command.rs`, `cli/src/services/auth_command/mod.rs`, `cli/src/services/sync/command.rs`, `cli/src/services/sync/sync.rs`, `context/plans/cli-single-tokio-runtime-pr1.md` (clean baseline at `7906f5c81951e67795684b31d2cd0b5ed0f8b7b6`). + - Result: Activated the multi-thread application runtime and directly awaited the complete app/static-dispatch/auth/sync chain. Telemetry now uses a generic async action with single-use arguments/stderr and preserved events/error classification. Classified DB-capable Setup/Doctor/Hooks use scoped blocking; sync owns storage in a concrete cleanup guard with blocking construction/destruction. Direct auth credential load/save/delete operations use awaited owned blocking closures. Removed both command runtimes; adapted all eleven sync tests to multi-thread async execution with safe DB fixture lifetimes and retained assertions. DB and control-plane production diffs are empty. + - Verify: Passed — Nix-wrapped `scripts/run-cli-cargo.sh build --manifest-path cli/Cargo.toml`; focused wrapper tests `services::sync::sync::tests` (11 passed) and `services::auth_command::tests` (5 passed); runtime ownership audit found only the application entrypoint, retained DB adapter runtimes and existing test fixtures; each of five production `block_in_place` sites matches section 2's inventory, with two additional test-only fixture scopes. No replacement runtime bridge, detached command, erased command future, or new command `Send` bound; DB/control-plane production diffs empty; `git diff --check` passed. + - Done-check evidence: App and dispatch delegates await completion before rendering; targeted auth/sync cleanup search has no runtime-helper matches; focused tests compile every existing test caller and preserve sync overlap/order/progress/cursor assertions; unchanged DB production diff confirms persistence boundary. + - Context impact: root — application runtime ownership, scoped borrowing, generic async telemetry and auth/sync orchestration now differ from durable synchronous descriptions; retained synchronous persistence and temporary caller blocking scopes must remain explicit. + - Deviation: Per the user's instruction to remove comments from generated code, added no Rust comments and removed the obsolete AppContext await-prohibition sentences instead of replacing them; scoped borrowing belongs in durable context. + - Context synchronization: synced + - Context synchronization evidence: Edited and verified all five root files (`overview.md`, `architecture.md`, `glossary.md`, `patterns.md`, `context-map.md`) against the awaited execution boundary. Updated capability, command-surface, sync, observability and shared-Turso owners; moved five execution terms to linked `context/cli/execution-glossary.md` so the root glossary remains at 250 lines. Recorded Accepted `context/decisions/2026-10-07-application-owned-async-command-runtime.md`; existing ADRs remain unchanged. Architecture diagram distinguishes the application runtime from retained DB-owned runtimes and documents PR2 as future work. Focused checks verified all 12 written context/ADR files at or below 250 lines, newly added relative links resolve, no new Rust comments, unchanged DB/control-plane production code, and clean patch whitespace. T03/T04 regression additions and T05 documentation review remain pending; no final-plan validation was run. + +- [x] T03: `Prove auth and credential blocking behavior under the application-level runtime` (status:done) + - Task ID: T03 + - Goal: Auth command migration preserves refresh/fallback/output and existing credential isolation. + - Files likely affected: inline tests in `services/auth_command/mod.rs`, `services/agent_trace_sync/control_plane.rs`; reuse `services/agent_trace_sync/test_http_server.rs` only if fixture extension is necessary; narrow test injection seam in auth orchestration if required. + - Scope: In — behavioral regression tests and minimal test seams. Out — live WorkOS/keyring access, auth policy changes, replacing storage architecture. + - Exact change: Retain the existing synchronous auth renderer/classification tests (logout text/JSON, unauthenticated whoami text/JSON and typed authenticated-whoami failures). There are currently no injected auth dispatch/login tests to convert. Add a focused multi-thread Tokio test using the existing `run_auth_subcommand_with` / `run_login_with_stored_credentials` injection seams after their signature migration: prove the selected async action/helper is polled and awaited to completion without another command runtime or nested-runtime panic. Reuse the existing local HTTP fixture if actual network-backed async work is needed, plus owned isolated credential callbacks to exercise new blocking storage callers; add only a narrow missing test seam, never live user keyring access. Keep renewal/fallback/persistence policy evidence in existing lower-level auth/control-plane suites rather than inventing an existing command test matrix or duplicating it. Do not change production endpoint/credential resolution. + - Exact change, control plane: Retain `credential_store_operations_run_outside_the_async_runtime` and its `RuntimeCheckingCredentialStore`. Extend or adapt that fixture under multi-thread Tokio only where needed to prove both load and save remain isolated in awaited blocking callbacks (including its existing nested-runtime check), and preserve join/error propagation. Reuse existing expired-token, 401 and refresh single-flight tests and their simultaneous requests/counts; do not add parallel copies of those matrices. Cover new direct auth load/save/delete blocking caller isolation with the same test approach if existing injections do not already reach them. + - Behavioral invariants: WorkOS loading/refresh, refresh-once/retry-once, persistence and typed failures, secret redaction, prompt/report behavior. Both existing control-plane `spawn_blocking` calls remain intact; auth's synchronous storage API is isolated by its awaited caller closures. + - Done when: New focused auth dispatch/helper coverage proves awaited execution inside an existing multi-thread runtime; existing renderer/classification tests retain their exact assertions; credential load/save and new direct-auth storage caller isolation are proven using owned fixtures; existing lower-level refresh/401 assertions remain exact. + - Verification: Targeted wrapper tests for `services::auth_command::tests` and `services::agent_trace_sync::control_plane::tests`, especially `expired_token_is_refreshed_and_saved`, `concurrent_expired_tokens_share_one_refresh_and_save`, `unexpected_401_refreshes_once_and_retries_once_on_success`, `unexpected_401_twice_fails_without_a_third_attempt`, and the blocking-isolation tests. + - Dependencies: T02. + - Completed: 2026-10-07 + - Files changed: `cli/src/services/auth_command/mod.rs`, `cli/src/services/agent_trace_sync/control_plane.rs`, `context/plans/cli-single-tokio-runtime-pr1.md` (clean baseline at `09473589e811eedf06f99f4f59951d18f39cb313`). + - Result: Added five multi-thread async auth regressions covering selected dispatch completion, owned renewal/redaction, awaited device fallback, owned blocking load/save/delete fixtures, and typed storage/join failures. Extracted the existing auth blocking closure and unchanged error mapping into one private generic credential-operation seam used by all six production call sites. Extended the existing control-plane nested-runtime fixture to prove both load and save complete safely under multi-thread Tokio; production control-plane code is unchanged. + - Verify: Passed — `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::auth_command::tests` (10 passed); the same wrapper with `services::agent_trace_sync::control_plane::tests` (28 passed, including expired-token refresh, concurrent single-flight, both 401 retry cases and blocking isolation); Nix-wrapped `cargo fmt` applied formatting; `git diff --check` passed. Initial test compilation exposed a borrowed callback-format capture; corrected it with an owned async capture and reran both modules successfully. + - Done-check evidence: All three selected dispatch actions and login renewal/fallback yield before returning inside a multi-thread runtime. Owned credential callbacks construct/poll/drop nested fixture runtimes and expose saved/deleted state before completion. Storage and panic-join failures preserve typed sources. Existing five auth renderer/classification tests and lower-level refresh/401 assertions are unchanged; control-plane production and DB/auth/token-storage diffs are empty. + - Context impact: local — regression evidence and a private reusable auth credential test seam; application ownership, auth policy, storage isolation/error classification and public output contracts remain unchanged. + - Context synchronization: synced + - Context synchronization evidence: Verified all five root files (`overview.md`, `architecture.md`, `glossary.md`, `patterns.md`, `context-map.md`) against the unchanged application ownership and credential blocking contracts. Updated the existing auth test-coverage statement in `context/cli/cli-command-surface.md`; retained its canonical auth behavior and the control-plane credential boundary in `context/cli/agent-trace-sync-command.md`. The edited domain owner remains 170 lines and all relative links resolve; `git diff --check` passed. No architecture decision qualified: the private test seam preserves the established blocking and error policies. No final-plan validation was run. + +- [x] T04: `Prove the new async execution and resource-lifetime boundary` (status:done) + - Task ID: T04 + - Goal: Detect lifecycle/output/nested-runtime regressions at the real boundary. + - Files likely affected: focused inline tests in `cli/src/app.rs` and/or `services/command_registry.rs`; existing `services/app_support.rs` and `services/observability/traits.rs` test locations; one representative existing hook fixture; `services/sync/sync.rs` tests. Reuse existing DB fixtures without requiring a new DB test module or framework. + - Scope: In — behavioral tests at migrated boundaries. Out — DB production or hook semantic changes, generalized test framework. + - Exact change, app: Use a small set of `run_with_dependency_check_and_streams` tests with captured byte writers to prove execution inside a running multi-thread Tokio runtime, a representative direct synchronous success, successful unknown-command help, and representative error output/exit behavior. Use existing dependency-check injection and output assertions to cover the changed startup-to-await-to-render boundary; retain authoritative app_support/parser/renderer suites for the remaining exit classes, help, diagnostics and output matrices. Assert exact current bytes/classes rather than bless changed snapshots. Reuse filesystem/environment/cwd guards or existing subprocess fixtures only where required; do not build a new general CLI integration harness. + - Exact change, telemetry: Add a recording test implementation supporting the generic async wrapper; assert dispatch logs before/after the awaited work, error paths omit success events, and a repeated action invokes no second parse/command, returning the same `REPEATED_COMMAND_DISPATCH_ERROR`. Ensure the telemetry abstraction surrounds every poll of the command future. Do not implement future telemetry by keeping a thread-local `tracing` default guard alive across `.await`; use a future-aware subscriber attachment or an intentionally global subscriber. + - Exact change, Turso/hooks: Reuse a temporary repository/state DB fixture to invoke one representative DB-backed synchronous command (prefer Doctor Diagnose) through async enum/app dispatch, proving safe construction, synchronous use and destruction before completion. Add one focused cleanup/error regression for the sync-owned storage guard, and verify its Drop can safely handle an abandoned future if existing tests do not exercise that new lifetime path. Invoke one representative persistence-backed hook through the changed dispatch boundary using its existing fixture and assert the persisted row is visible when command completion is observed. Do not add Claude/Codex/OpenCode/Pi/Git top-level test matrices or retest migrations/encryption/contention/WAL; existing suites retain those domain assertions. Any adapter-specific addition requires evidence that existing coverage misses a distinct changed boundary. Preserve the existing detached post-commit auto-sync process test and avoid real user keyrings. + - Exact change, sync: Keep existing async-converted orchestration tests as the proof that sync runs without nested-runtime panic. In particular retain `concurrent_sync_overlaps_all_three_stream_batches_after_one_state_request` and `concurrent_sync_keeps_batches_sequential_within_one_stream` and all progress/cursor/reconciliation assertions. Reuse existing command output/progress fixtures to verify completion follows awaited success; add only a missing changed-boundary assertion, not a duplicate sync matrix. + - Behavioral invariants: DB code/policies stay unchanged; no nested runtime or runtime-drop panic; deterministic output/classification; synchronous hook completion; original concurrency and retry policies. + - Done when: The focused boundary checks prove multi-thread application execution, direct synchronous output/exit preservation, safe DB-owning command cleanup, hook persistence before completion and telemetry polling scope; adapted auth/sync tests prove no nested command runtime and unchanged concurrency. Existing domain assertions are retained without a new test framework. + - Verification: Focused Nix-wrapped tests for the changed app/registry boundary, existing app_support, telemetry, representative hook and sync cleanup/concurrency fixtures. Review production DB diff as empty and production blocking sites against section 2's inventory. A runtime panic fails acceptance; do not change DB policies or weaken assertions to accommodate it. Final normal and CI/release validation remain in section 7, not this task. + - Dependencies: T02, T03. + - Completed: 2026-10-07 + - Files changed: `cli/src/app.rs`, `cli/src/services/hooks/tests.rs`, `cli/src/services/sync/sync.rs`, `context/plans/cli-single-tokio-runtime-pr1.md` (clean baseline at `c8c8fc12f8d46a8189ae391a284608036bdac39b`). + - Result: Added captured app-stream regressions for direct version/help, successful unknown-command help, dependency failure and command failure with exact bytes/exit classes. Extracted a private statically typed lifecycle seam for recording telemetry; covered every poll of yielding Noop work, dispatch event ordering, parse/command errors and repeat protection without a second parse/dispatch. Reused the hook repository fixture in an isolated child with multi-thread enum dispatch through Doctor Diagnose and DiffTrace; queried the persisted row after completion. Added real sync storage-guard regressions for early error and abandonment after polling, with safe DB reopening. Existing auth/sync/hook assertions and production DB policies remain unchanged. + - Verify: Passed — focused Nix-wrapped `scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml` filters `app::tests` (4), `services::command_registry::tests` (3), `services::app_support::tests` (5), `services::hooks::tests` (78), `services::auth_command::tests` (10), and `services::sync::` (34, including all 13 orchestration tests and detached auto-sync coverage). Final app and exact representative-hook runs passed after tightening child environment isolation and adding command-error bytes. Nix-wrapped `cargo fmt` applied formatting; `git diff --check` passed. DB, hook command/dispatch and auto-sync production diffs are empty against the pre-migration baseline; all five production blocking sites still match section 2's inventory, with new scopes confined to fixtures. Initial compilation required preserving concrete AppContext accessors in the private seam; an initial version expectation was corrected to the existing package-name output, then the affected tests passed. No full-plan validation was run. + - Done-check evidence: Runtime-flavor assertions run during injected app startup, awaited Noop work and Doctor/hook dispatch. Captured output preserves success, runtime exit 4 and dependency exit 5; telemetry logging is asserted inside poll scopes, failure paths omit success events, and repeat returns the exact existing runtime error with one parse/command. Seeded repository storage is diagnosed and dropped safely, the hook row is visible after await, and guard cleanup succeeds on early return and dropping a pending future. Existing eleven sync concurrency/progress/cursor/reconciliation tests and ten auth tests pass without nested command runtimes. + - Context impact: local — regression coverage and a private static lifecycle test seam; runtime ownership, telemetry event semantics, CLI output/exit contracts, hook persistence and DB policies remain unchanged. + - Context synchronization: synced + - Context synchronization evidence: Verified all five root files (`overview.md`, `architecture.md`, `glossary.md`, `patterns.md`, `context-map.md`) against unchanged application runtime ownership, scoped borrowing, static telemetry, foreground hooks and retained DB bridging. Corrected the existing execution/parser coverage section in `context/cli/cli-command-surface.md` to name actual test owners and the focused async/resource-lifetime checks. The observability and sync owners already describe polling/repeat protection and storage cleanup accurately. The edited domain owner remains 170 lines with resolving relative links; `git diff --check` passed. No architecture decision qualified: tests and the private lifecycle seam preserve the accepted boundaries. No final-plan validation was run. + +- [x] T05: `Document the staged runtime architecture and scoped DI borrowing` (status:done) + - Task ID: T05 + - Goal: Durable context describes the completed PR1 architecture and PR2 boundary accurately. + - Files likely affected: `context/architecture.md`, `context/glossary.md`, `context/overview.md`, `context/patterns.md`, `context/cli/cli-command-surface.md`, `context/cli/capability-traits.md`, `context/cli/sync-command.md`, `context/cli/agent-trace-sync-command.md`, `context/sce/cli-observability-contract.md`, `context/sce/shared-turso-db.md`; narrowly update `context/sce/cli-exit-code-contract.md`, `context/sce/cli-stdout-stderr-contract.md`, `context/context-map.md` only where boundary ownership/annotations need adjustment. + - Scope: In — new execution boundary and touched context drift. Out — unrelated docs, new ADR workflow, documenting PR2 as implemented. + - Exact change: Record one application-level Tokio multi-thread runtime, direct async enum dispatch, direct ordinary synchronous commands, individually justified temporary DB blocking islands, awaited auth/sync/control-plane HTTP, retained credential `spawn_blocking`, synchronous lifecycle providers and the unchanged synchronous Turso adapter with its DB-owned current-thread runtime. Include PR1/PR2 diagrams from section 7 and clearly mark PR2 as future work deleting DB-owned runtime, `build_current_thread_runtime`, `block_on_isolated` and DB-related `block_in_place` scopes. Replace blanket AppContext await prohibition with scoped borrowing/owner lifetime guidance. Describe the async telemetry polling scope and repeat guard, and remove stale active-Telemetry object-safety requirements from the observability contract, glossary and architecture: the capability intentionally becomes non-object-safe/static, with expected future static OTel and no OTel implementation here. Correct architecture's stream count and explicit Tokio features. Preserve exit/output contracts while refreshing affected helper ownership references. Link existing owners rather than creating duplicate runtime documentation. + - Behavioral invariants: Documentation cannot claim Turso async, sole runtime object process-wide, new detached hooks, changed output or changed refresh/sync policies. + - Done when: Durable owners match final code and clearly separate PR1 application-level runtime ownership from retained PR2 DB infrastructure. + - Verification: Inspect every touched paragraph against source, section 2's blocking inventory and section 9; search touched docs for stale auth/sync command-runtime ownership, blanket borrow-across-await prohibition, active Telemetry object-safety claims and premature claims that async Turso/one process-wide runtime object are implemented. Ensure validation-tier descriptions distinguish normal checks from the CI/release aggregate. + - Dependencies: T02, T03, T04. + - Completed: 2026-10-07 + - Files changed: `context/architecture.md`, `context/cli/cli-command-surface.md`, `context/context-map.md`, `context/sce/cli-exit-code-contract.md`, `context/sce/cli-stdout-stderr-contract.md`, `context/sce/shared-turso-db.md`, `context/plans/cli-single-tokio-runtime-pr1.md` (clean baseline at `39884477da7491422ca8431f699698a6ccb7e83d`). + - Result: Completed the existing staged runtime owner with a future PR2 diagram and explicit bridge/blocking-scope removal boundary. Documented each of the five production blocking scopes and direct synchronous branches in the command owner; clarified retained credential isolation and foreground hook completion. Corrected the context-map's remaining object-safe telemetry annotation and obsolete exit/output helper ownership. Preserved already accurate scoped borrowing, async telemetry polling/repeat guard, stream count and explicit Tokio features from T02–T04 synchronization. Refreshed command verification guidance to distinguish normal checks, the separate CI/release aggregate and generated-payload-aware targeted Cargo execution. + - Verify: Passed — inspected all changed paragraphs against `main.rs`, `app.rs`, `app_support.rs`, `command_registry.rs`, `parse/command_runtime.rs`, `error.rs`, auth/sync orchestration, telemetry traits, retained DB bridge and `flake.nix`, plus section 2's blocking inventory. Nix Python checks found no stale execution claims across 14 applicable owners, confirmed PR1/PR2 diagrams and future removal names, and confirmed validation-tier descriptions. All six edited durable owners are at or below 250 lines (architecture 246, command surface 186, context map 183, exit contract 28, stream contract 31, shared Turso 138); all their relative links and anchors resolve. Baseline-relative Git comparison confirms only the listed context/plan files changed, with no staged/untracked or code/generated-input changes; `git diff --check` passed. No full-plan validation was run. + - Done-check evidence: Final source matches the documented one application-level multi-thread runtime, directly awaited static auth/sync dispatch, scoped borrowed dependencies and polling telemetry, direct ordinary commands, five justified DB lifetime scopes and retained synchronous Turso infrastructure. PR2 is labeled future work; numeric exit/output, refresh/sync policies and existing detached auto-sync behavior are unchanged. + - Context impact: root — documentation completes the existing cross-cutting execution owners and index without changing implementation, terminology or the accepted architecture decision. + - Context synchronization: synced + - Context synchronization evidence: Verified all five root owners: `overview.md` retains application/DB runtime ownership and separate validation tiers; `architecture.md` carries staged diagrams and linked lifetime inventory; `glossary.md` delegates scoped borrowing/static telemetry to the execution glossary; `patterns.md` retains directly awaited borrowing and credential isolation; `context-map.md` indexes the owners with the corrected telemetry annotation. Execution already updated the six applicable durable owners, so no further context edit was warranted. Source review and focused Nix Python link/anchor, line-count, stale-claim and root-fact checks passed, as did `git diff --check`. No new architecture decision qualified: documentation preserves the existing Accepted runtime ADR without changing its constraint. No final-plan validation was run. + +## 6. File-by-file impact + +| File/surface | Planned impact | +| --- | --- | +| `cli/Cargo.toml` | Two Tokio feature additions, defaults still disabled. | +| `cli/Cargo.lock` | Minimal macro dependency resolution; retain existing versions where possible. | +| `packaging/flatpak/cargo-sources.json`, `nix/flatpak/cargo-sources.nix` | Generator parity and resulting fixed-output hash. | +| `cli/src/main.rs` | Sole application-level runtime macro and awaited `app::run`. | +| `cli/src/app.rs` | Five async run/lifecycle functions, ownership-safe startup flow, scoped borrowed context, new app tests. | +| `cli/src/services/app_support.rs` | Async execution phase; render/error/output helpers unchanged; regression coverage. | +| `cli/src/services/command_registry.rs` | Async execute methods, direct Auth/Sync awaits, direct ordinary sync arms, request/subcommand classification for only documented DB-capable blocking islands; catalog/enum/other command APIs retained. | +| `cli/src/services/observability/traits.rs` | Intentionally non-object-safe generic async telemetry action/polling with static concrete dispatch, Noop adaptation and focused regression tests. | +| `cli/src/services/auth_command/command.rs` | Async command execute wrapper. | +| `cli/src/services/auth_command/mod.rs` | Async orchestration/helper callbacks including logout, owned credential blocking callers, runtime removal, tests; sync rendering/storage APIs retained. | +| `cli/src/services/sync/command.rs` | Async wrappers; awaited progress lifetime and completion. | +| `cli/src/services/sync/sync.rs` | Eight async wrappers, runtime removal, caller blocking construction/destruction with concrete cleanup guard, inline test fixture adaptation; engine/join unchanged. | +| `cli/src/services/agent_trace_sync/control_plane.rs` | Narrow existing credential-isolation test adaptation only as needed; production refresh/HTTP/blocking policies retained. | +| `cli/src/services/agent_trace_sync/test_http_server.rs` | Reuse fixture; extend only if required by focused tests. | +| `cli/src/services/db/mod.rs` | No production or required test changes; reuse existing DB fixtures/suites to verify the caller boundary. | +| One representative existing hook test fixture | Narrow runtime-context dispatch/persistence-before-completion regression; existing per-adapter matrices remain authoritative and production adapters unchanged. | +| Durable context owners listed in T05 | Accurate current async application boundary and temporary DB exception. | + +Inspected unchanged production surfaces: `services/auth.rs` (already async HTTP), `services/token_storage.rs`, `services/agent_trace_sync/mod.rs`, `services/sync/{progress.rs,render_sync.rs,auto_sync.rs}`, `services/lifecycle`, synchronous service-owned command implementations, persistence/mutation trace/hook DB services, parser, error catalog and output format. Any newly discovered signature caller may receive a narrow await adaptation only. + +## 7. Tests / validation + +Use `.sce/config.json` and `AGENTS.md` policy. Planning runs no tests; commands below are implementation/final verification instructions. Full checks are listed once here. Keep exact existing assertions; runtime migration is not permission to reduce coverage or replace concurrency overlap tests with mere successful completion. + +Targeted Rust template when needed: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml `. For a single exact test use its fully qualified name plus `-- --exact`. Do not use bare Cargo or bypass generated-payload preparation. + +Required existing sync regressions, all retained in `services/sync/sync.rs`: `progress_events_cover_batches_empty_streams_and_fixed_order`, `full_sync_uploads_three_remote_streams_and_second_run_is_naturally_incremental`, `sync_succeeds_when_local_diff_trace_row_is_rejected_by_export_reader`, `diff_traces_report_echoes_server_cursor_without_uploading_newer_local_rows`, `concurrent_sync_overlaps_all_three_stream_batches_after_one_state_request`, `concurrent_sync_keeps_batches_sequential_within_one_stream`, `invalid_state_cursor_fails_before_any_batch_request`, `terminal_batch_status_fails_without_state_reconciliation`, `progress_events_end_after_terminal_failure`, `malformed_2xx_batch_response_still_reconciles_via_state`, `forbidden_state_response_fails_without_mutating_local_metadata`. + +Keep control-plane tests for storage isolation, token reuse/refresh/single-flight, exact state request count/body, one 401 refresh and retry, failure after second 401, status classification and safe diagnostics. Retain engine-level `agent_trace_sync` retry/reconciliation tests and DB contention/WAL/migration tests in their existing validation tier; do not reproduce these matrices in new app tests. Test-only `block_on` helpers may remain where invoked outside another runtime. DB fixtures moved into async tests require safe lifetime cleanup, while unchanged purely synchronous fixtures retain their current lifecycle. + +New/adapted execution-boundary checks are limited to the following outcomes: application polling occurs inside a multi-thread runtime; auth and sync complete without nested-runtime panic; one representative DB-backed synchronous command constructs/uses/drops its DB safely; one representative hook persists before completion; telemetry scopes future polling; captured stdout/stderr and representative exit classes retain their current contracts; existing sync overlap and sequential-batch assertions still pass. Reuse existing fixture/test modules. An assertion of `Handle::runtime_flavor() == RuntimeFlavor::MultiThread` can prove the boundary flavor; source ownership audit proves there is one application-level runtime, not a misleading process-wide runtime-object count. + +Production cleanup check (expect no matches in command files; distinguish separately reviewed test-only helpers): + +```sh +nix shell nixpkgs#ripgrep -c rg -n 'AUTH_RUNTIME|SYNC_RUNTIME|shared_runtime|OnceLock|tokio::runtime|Builder::new_current_thread|block_on' cli/src/services/auth_command/command.rs cli/src/services/auth_command/mod.rs cli/src/services/sync/command.rs cli/src/services/sync/sync.rs +``` + +Because inline tests may deliberately use runtime fixtures, inspect any matching test-only lines against their `#[cfg(test)]` boundary; do not declare success from a raw match count. Prefer `#[tokio::test]` for new command tests so these command files need no replacement bridge helpers. A no-match `rg` exits 1 and is expected here. + +Audit the entire source for runtime ownership, blocking and spawn sites: + +```sh +nix shell nixpkgs#ripgrep -c rg -n 'tokio::runtime|\bRuntime\b|Builder::new_current_thread|block_on|shared_runtime|AUTH_RUNTIME|SYNC_RUNTIME|OnceLock|tokio::spawn|spawn_blocking|block_in_place' cli/src +nix shell nixpkgs#ripgrep -c rg -n '#\[tokio::main|rt-multi-thread|macros|default-features' cli/src/main.rs cli/Cargo.toml +nix shell nixpkgs#ripgrep -c rg -n 'build_current_thread_runtime|block_on_isolated|spawn_blocking' cli/src/services/db/mod.rs cli/src/services/agent_trace_sync/control_plane.rs +``` + +Classify matches as application entrypoint, retained DB bridge, unchanged persistence singleton, existing test fixture or existing blocking credential work. Expected production construction owners after PR1: `main` for application execution and shared DB for temporary adapter runtimes. Match every newly introduced production `block_in_place` to section 2's inventory, its exact call site and resource lifetime; unclassified/general synchronous dispatch wrapping fails acceptance. Review the PR production diff to prove DB bridge/policies and the two credential blocking calls unchanged. Inspect command futures for no `tokio::spawn`, dynamic async dispatch or artificial `Send` bounds. Search `cli/src` for `dyn Telemetry`/boxed telemetry and inspect `HasTelemetry` plus the generic async trait to prove intentional non-object-safe static selection. + +### Full validation + +- `nix flake check` — normal repository checks, including Rust tests/clippy/fmt, config/plugin checks, native-package validation and Flatpak Cargo-source parity. This does not build the long-running release tier. +- `nix build .#ci-checks` — the separate long-running CI/release tier: static-musl release build on Linux (native release on Darwin), plus Linux release portability validation. Both this and normal checks are required because PR1 changes Tokio features/lockfile, generated Flatpak Cargo sources and native CLI execution. Use the repository's supported platform matrix through CI. +- `git diff --check` — patch whitespace integrity. +- `nix run .#pkl-check-generated` only if implementation unexpectedly touches canonical generation inputs; this plan does not require such edits or committed generated trees. + +### Context sync + +T05's durable owners must describe the completed execution model. Preserve current static DI/lifecycle, hook, auth/sync and persistence contracts. Existing map annotations change only if descriptions require it. No new context bootstrap or unrelated cleanup is needed. + +Expected documented architecture (PR2 is the next migration, not implemented in this plan): + +```text +PR1 + +SCE process +└── one application-level Tokio multi-thread runtime + ├── async app lifecycle and static command dispatch + ├── async auth + ├── async sync + ├── async control-plane HTTP + │ └── awaited spawn_blocking for credential storage + ├── direct synchronous commands + └── temporary blocking islands (individually justified DB lifetimes) + └── current synchronous Turso adapter + └── DB-owned current-thread runtime / block_on_isolated bridge + +PR2 (future) + +SCE process +└── application-level Tokio runtime + ├── async auth + ├── async sync + ├── async HTTP + └── async Turso + +deleted by PR2: +- DB-owned runtime +- build_current_thread_runtime +- block_on_isolated +- DB-related block_in_place workarounds +``` + +## 8. Risks and migration hazards + +- A partially migrated entrypoint nests auth/sync runtimes. Avoid with T02's atomic signature/ownership change, not a temporary bridge helper. +- Borrowing itself is safe while the owner lives; spawning a borrowed command or requiring `Send` would unnecessarily break the current `Rc` progress model. Await directly and keep runtime/context alive until completion. +- Future-returning `FnMut` closures cannot return arbitrary mutable reborrows of their captured environment. Move taken command inputs/writer references into the returned future; use owned renewal inputs. Do not respond with boxed command futures or pervasive lifetime rewrites. +- A subscriber guard surrounding future creation does not necessarily surround polling. Await inside the telemetry abstraction and verify log ordering/repeat behavior. No new telemetry semantics are authorized. +- `tracing` default subscriber guards are thread-local. Holding one across `.await` on the multi-thread Tokio runtime would not guarantee that subsequent polls see the same subscriber. PR1's `NoopTelemetry` does not expose this bug, so the plan must preserve the future-aware scoping requirement for the upcoming OTel implementation. +- The generic async Telemetry callback deliberately loses object safety. Static associated capability dispatch supports it today; stale documentation must not drive new boxing or `async-trait`. Future OTel is expected to follow the same static model and remains out of scope. +- The database bridge protects `block_on` reentry but does not protect runtime Drop: pinned Tokio shutdown panics when directly dropped in async context. Required caller-side `block_in_place` construction/destruction, sync cleanup guard, and awaited owned auth credential closures address this without DB production changes. Multi-thread Tokio is required for `block_in_place`; new command integration tests must use that flavor, while pure lower-level test runtimes may remain current-thread. +- More synchronous DB/filesystem work now occurs in a Tokio-entered context, and the existing bridge can create scoped threads. This is intentional PR1 staging; no claim of fully nonblocking persistence or performance improvement. Avoid opportunistic batching/backoff changes. +- Sync progress must remain alive across the await and finish only on success. Keep Started/Finished behavior even on failure, JSON suppression, stream ordering, and final report content. +- Global auth storage and environment/cwd mutation can contaminate tests. Use existing isolated state fixtures/subprocesses and guards; do not use a real user's database or credential store. +- Blanket blocking dispatch obscures which legacy resource lifetime requires an island. Keep pure/config/non-DB hook execution direct, document each introduced scope, and delete the DB workarounds only with PR2's runtime ownership removal. `block_in_place` requires multi-thread Tokio and is a temporary lifetime compatibility measure, not an async filesystem/Git migration. +- Tokio macro dependencies alter reproducible packaging. Minimal lock edits without matching fixed-output metadata can fail normal checks; keep T01 parity in the same commit. Release/native consequences also require the separate `nix build .#ci-checks` tier. +- Top-level runtime initialization failures now belong to the entrypoint, whereas removed command-runtime creation failures disappear. Do not invent CLI diagnostic rewrites for ordinary execution; preserve existing classified operation failures and prove normal/error output behavior in regression tests. + +## 9. Acceptance criteria + +- [x] AC1: Exactly one application-level multi-thread Tokio runtime drives `app::run`; direct Tokio defaults remain disabled and only the required features are added. + - Validate: Inspect `cli/src/main.rs` for the multi-thread `#[tokio::main]` and awaited `app::run`; inspect Cargo feature/lock diff and classify all production runtime-construction matches using section 7's source audit. +- [x] AC2: The app lifecycle and `RuntimeCommand` execution await auth/sync while retaining concrete enum dispatch, concrete capabilities and synchronous parsing/rendering/lifecycle providers. + - Validate: Inspect the exact T02 call chain, auth/sync match arms and capability bounds; run app/static registry/parser tests through the full suite; confirm no new erased command future/trait objects or command spawn. +- [x] AC3: Auth and sync contain no production `AUTH_RUNTIME`, `SYNC_RUNTIME`, runtime-owning OnceLocks, `shared_runtime()` or command `block_on` calls. + - Validate: Run section 7's targeted cleanup search, inspect test boundaries for any matches, and verify no replacement command runtime infrastructure in the full source audit. +- [x] AC4: Existing Turso production APIs/runtime bridge and all database policies remain unchanged while persistence-backed commands safely execute from the application-level runtime. + - Validate: Inspect DB/persistence production diff as empty, including `build_current_thread_runtime`, `block_on_isolated` and DB-owned runtime; run focused T04 representative command/cleanup tests and existing migrations/WAL/contention suites through their normal tiers. A runtime panic fails acceptance. +- [x] AC5: Auth token loading/renewal/persistence, refresh single-flight and 401 retry contracts work from an existing application-level runtime; credential load/save remain awaited blocking operations. + - Validate: Run T03 async auth/credential tests and existing named control-plane refresh/401 tests; inspect both production `spawn_blocking` bodies and join/error mappings as unchanged. +- [x] AC6: Sync preserves one initial state call, three concurrent remote streams, sequential per-stream batches, cursors/reconciliation/retries, diff compatibility and progress/output. + - Validate: Run section 7's eleven named sync tests under multi-thread async execution plus engine/control-plane regressions; assert actual HTTP overlap/order/counts and progress/JSON invariants. +- [x] AC7: Synchronous commands retain stdout/stderr routing, exit codes, help/unknown-command behavior, JSON/text output, diagnostics, redaction and typed errors. + - Validate: Run T04 app byte/output/exit tests and unchanged app_support/parser/rendering tests; inspect output/error/catalog diffs for no semantic changes. +- [x] AC8: All hook adapters preserve completion, persistence, payloads and foreground semantics; only the existing explicit post-commit auto-sync child remains detached. + - Validate: Run existing Claude/Codex/OpenCode/Pi/Git hook suites unchanged plus T04's one representative async-dispatch persistence/completion assertion; inspect hook/auto-sync production diff and audit no new hook `tokio::spawn`. Add adapter-specific tests only if a distinct changed boundary lacks existing coverage. +- [x] AC9: Telemetry intentionally becomes non-object-safe and remains statically selected through `HasTelemetry::Telemetry`; its scope surrounds awaited polling with unchanged events/repeat error, and borrowed AppContext dependencies outlive command awaits. No dynamic async abstraction is introduced. + - Validate: Inspect generic async Telemetry signature, concrete Noop/AppRuntime/HasTelemetry selection and no active `dyn Telemetry` consumers/new `async-trait`/`BoxFuture` command dispatch. Run T04 polling/repeat tests and inspect T05's object-safety/borrowing corrections. Validate that the async telemetry API does not encode a thread-local `with_default`/`set_default` guard held across `.await`, and that the documented future OTel implementation is expected to attach its subscriber to the future or install it globally by explicit design. +- [x] AC10: Reproducible packaging and durable context represent PR1 accurately, including retained synchronous Turso bridging for PR2. + - Validate: Cargo-source parity and full validation under section 7 pass; inspect every T05 owner against final code and the staged architecture diagram. No document claims async persistence. +- [x] AC11: Every production `block_in_place` introduced by PR1 has an individual documented reason tied to an existing runtime-backed dependency and its safe construction/destruction; ordinary synchronous commands run directly. + - Validate: Run section 7's source audit and reconcile each introduced call with section 2's inventory and actual lifetime/cleanup path. Inspect direct Help/HelpText/Version/Completion/Policy/Config, context-only Setup and non-DB Hooks branches. No blanket synchronous wrapper or unclassified blocking site is accepted. +- [x] AC12: Both normal repository and long-running CI/release validation pass for the changed execution/build model. + - Validate: Section 7's `nix flake check`, `nix build .#ci-checks` and `git diff --check` all pass; retain targeted Nix-wrapped Rust test evidence and Cargo-source parity. Do not treat normal flake checks alone as the entire current CI tier. + +## Validation Report + +**Status:** validated + +**Date:** 2026-10-07 + +### Commands run + +- `nix flake check` -> exit 0 (all normal checks passed on x86_64-linux; Rust suite: 1766 passed, 0 failed, 4 ignored; tests, Clippy, formatting, generated-input/config checks and Cargo-source parity passed). +- `nix build .#ci-checks` -> exit 0 (static-musl release binary and Linux release portability audit passed). +- `git diff --check` -> exit 0 (no whitespace errors before or after validation recording). +- Section 7 targeted auth/sync cleanup search through `nix shell nixpkgs#ripgrep -c rg` -> exit 0 (four matches, all inside auth's `#[cfg(test)]` module; no production command runtime bridge). +- Section 7 full `cli/src` runtime/blocking/spawn audit through `nix shell nixpkgs#ripgrep -c rg` -> exit 0 (application entrypoint, retained DB bridge/persistence singleton and test fixtures classified; five production DB blocking scopes and three credential blocking call sites). +- Section 7 entrypoint/Cargo feature search through `nix shell nixpkgs#ripgrep -c rg` -> exit 0 (multi-thread macro and explicit features with disabled Tokio defaults). +- Section 7 DB bridge/control-plane blocking search through `nix shell nixpkgs#ripgrep -c rg` -> exit 0 (retained bridge and both unchanged credential load/save closures). +- `nix shell nixpkgs#ripgrep -c rg -n 'dyn Telemetry|dyn.*Telemetry|async-trait|BoxFuture|set_default' cli/src cli/Cargo.toml` -> exit 0 (only unrelated existing keyring default-store calls matched; no erased telemetry/command futures or tracing subscriber guard). +- `git diff b57dcaf6..HEAD` inspections of affected source, Cargo metadata and T05 context owners -> exit 0 (minimal feature/lock additions, awaited static execution, documented lifetime scopes; DB/persistence, hook production, auth HTTP, output/error/catalog/parser and generation inputs unchanged). +- `git status --short --untracked-files=all` -> exit 0 (clean before validation recording; no leftover debug files, scaffolding or generated target trees introduced by PR1). +- `nix log /nix/store/yk26hnvcpn85912d7801a4aadqnxvz7z-sce-cli-tests-test-0.4.0-pre-alpha-v9.drv` with tail and Nix-wrapped module/ignored-result filtering -> exit 0 (confirmed all required named auth/control-plane/sync/app/representative-hook tests passed). +- `nix log /nix/store/q1b755h9yamxbkql78rzdjqd5clxr3yd-sce-cli-clippy-clippy-0.4.0-pre-alpha-v9.drv` -> exit 0 (successful Clippy derivation inspected). +- `nix log /nix/store/q3v7krk78p2qsnydfv6rcvyzbxnnrhxc-sce-release-portability-audit.drv` -> exit 0 (release binary has no forbidden Linux `/nix/store/` runtime references). + +### Success-criteria verification + +- [x] AC1: One application-level multi-thread runtime with minimal Tokio features -> `main.rs:12` awaits `app::run`; Cargo adds only `macros`/`rt-multi-thread`, defaults remain disabled; lock adds only tokio-macros 2.7.2 and syn 3.0.6. Source construction audit identifies only the entrypoint and retained shared DB adapter in production. +- [x] AC2: Awaited concrete app/command execution -> inspected main/app/app_support/registry/auth/sync delegates and concrete capability bounds; Auth/Sync are directly awaited; parsing, rendering and lifecycle providers remain synchronous. App, registry and parser suites passed; no detached or erased command future. +- [x] AC3: No auth/sync production runtime ownership -> targeted search matches only auth test flavor assertions and nested fixture runtime; removed AUTH_RUNTIME/SYNC_RUNTIME/shared_runtime/command block_on have no replacement production owner. +- [x] AC4: Unchanged persistence with safe async-call-site lifetimes -> baseline-relative DB/persistence production diff empty; representative Doctor/hook dispatch and both storage-guard error/abandonment regressions passed alongside normal DB migration/WAL/contention checks. No runtime panic failed the suite. +- [x] AC5: Auth and credential contracts -> all 10 auth and 28 control-plane tests passed, including owned blocking operations, storage/join errors, expired-token refresh, concurrent single-flight, credential isolation and both 401 cases. Production control-plane load/save closures and join mappings unchanged. +- [x] AC6: Sync contracts -> all 11 section 7 named regressions and both new cleanup tests passed. Original overlap, sequential-batch, request-count, cursor/reconciliation, diff compatibility and progress assertions retained; engine/control-plane, renderer and progress suites passed; JSON still selects NoopProgressReporter. +- [x] AC7: Output/error contracts -> captured exact app bytes and success/runtime/dependency exits passed, as did unchanged app_support/parser/rendering/security suites; reviewed rendering, error/catalog and parser production diffs as unchanged. +- [x] AC8: Foreground hooks -> unchanged adapter suites and representative awaited Doctor/DiffTrace persistence test passed; hook/auto-sync production diff empty; no new production hook Tokio spawn; existing detached auto-sync coverage passed. +- [x] AC9: Static telemetry and scoped borrowing -> generic async Telemetry/Noop/HasTelemetry and live AppRuntime owner inspected; yielding polling, event/error/repeat tests passed. No erased dispatch, command Send bound or thread-local tracing guard; T05 owners explicitly require future-aware or deliberately global subscriber selection. +- [x] AC10: Packaging and accurate durable context -> Cargo-source parity and both validation tiers passed. Inspected overview, architecture, glossary/execution glossary, patterns, context map, command/capability/sync/control-plane, observability, shared-Turso and exit/output owners; PR1 is application-level async with synchronous Turso retained, and PR2 is labeled future work. Canonical generation inputs unchanged. +- [x] AC11: Individually justified blocking scopes -> registry Setup:84, Doctor:87, DB Hooks:103 and sync storage construction:220/guard Drop:164 match section 2 and durable inventory. Direct ordinary commands, context-only Setup and PreCommit/PostRewrite inspected; additional scopes are test-only fixtures. +- [x] AC12: Both validation tiers -> `nix flake check`, `nix build .#ci-checks` and whitespace verification passed; current derivation logs retain named regression evidence alongside the plan's prior targeted Nix-wrapped test evidence. + +### Failed checks and follow-ups + +- None. + +### Residual risks + +- Execution was verified on x86_64-linux. Nix omitted aarch64-linux, aarch64-darwin and x86_64-darwin; this session did not run the remote platform matrix. +- Normal Rust validation leaves three existing ignored lock-contention characterization tests unexecuted; the fourth ignored entry is a subprocess helper. No ignored tests were added by PR1. +- Synchronous Turso and its DB-owned runtimes remain intentionally retained until PR2; the five caller blocking scopes are temporary compatibility boundaries. diff --git a/context/sce/cli-exit-code-contract.md b/context/sce/cli-exit-code-contract.md index 839113cdd..14701ec34 100644 --- a/context/sce/cli-exit-code-contract.md +++ b/context/sce/cli-exit-code-contract.md @@ -15,11 +15,11 @@ The contract is intentionally class-based so automation can branch on failure ca ## Classification ownership -- `cli/src/app.rs` owns classification via `FailureClass` and maps it to numeric codes through `FailureClass::exit_code`. -- Top-level parse failures are classified in `parse_command`/`parse_subcommand`. -- Command-local argument validation failures are classified in `parse_config_subcommand`, `parse_setup_subcommand`, `parse_hooks_subcommand`, and `parse_non_setup_subcommand`. -- Runtime failures are classified in `dispatch` when service execution fails after valid parsing/validation. -- Dependency failures are classified in startup via the dependency-check closure passed to `run_with_dependency_check`. +- `cli/src/services/error.rs` owns `FailureClass`, its numeric `exit_code` mapping, and `CliError::class()` for internal and typed user errors. +- The synchronous `services::parse::command_runtime::parse_runtime_command` owns clap conversion/classification; command-local parsers retain invocation validation. `app::parse_command_phase` delegates to that boundary. +- Service-owned command methods return classified runtime errors through directly awaited `RuntimeCommand::execute_with_stderr` and `app_support::execute_command_phase`. +- `app::perform_dependency_check` classifies startup dependency failures before parsing, using the closure passed to `run_with_dependency_check`. +- After awaited execution completes, synchronous `app_support::render_run_outcome` / `exit_with_error` renders diagnostics and maps `error.class().exit_code()` to the process result. The application runtime changes execution ownership, not numeric classes. ## Determinism requirements diff --git a/context/sce/cli-observability-contract.md b/context/sce/cli-observability-contract.md index 6ed58cfa6..ddbcbbc82 100644 --- a/context/sce/cli-observability-contract.md +++ b/context/sce/cli-observability-contract.md @@ -70,11 +70,11 @@ Runtime observability consumes the shared resolved observability config from `cl - `cli/src/services/observability/traits.rs` exposes the `services::observability::traits::Logger` trait with the current logging API: `info`, `debug`, `warn`, `error`, and `log_cli_error`, each accepting `Option<&str>` session context used only for file routing. - The concrete `services::observability::Logger` implements the trait while retaining the existing inherent methods and behavior. - `NoopLogger` is available from the same traits module for tests and future dependency-injected services that need a logger without side effects. -- The same traits module exposes object-safe `services::observability::traits::Telemetry` with the current app subscriber boundary: `with_default_subscriber` for command-lifecycle execution. -- Production currently uses `NoopTelemetry` from the same traits module. Its `with_default_subscriber` implementation invokes the action directly and installs no tracing subscriber or tracer provider; the boundary is retained for future telemetry/OTEL integration. +- The same traits module exposes deliberately non-object-safe `services::observability::traits::Telemetry`: async `with_default_subscriber` accepts generic `FnMut() -> Fut` actions and awaits command-lifecycle execution through static associated capability dispatch. +- Production currently uses `NoopTelemetry` from the same traits module. Its `with_default_subscriber` implementation awaits the action once and installs no tracing subscriber or tracer provider. Future statically dispatched telemetry/OTEL must attach the subscriber to the command future (for example `WithSubscriber::with_subscriber`) or deliberately install it globally; holding a thread-local `with_default`/`set_default` guard across awaits does not scope every poll on multi-thread Tokio. - `cli/src/app.rs` stores the concrete production `Logger` and `NoopTelemetry` as `AppRuntime` fields, creates borrowed `AppContext` views for command execution, and exposes logger/telemetry access through associated-type context accessors instead of owned `Arc` fields or object-erased accessor return values. - Final stream rendering uses `RunOutcome` in `cli/src/services/app_support.rs`, so classified-error and stdout-write-failure logging depends on the logger trait boundary rather than the concrete production logger type. -- `run_command_lifecycle` expects the telemetry subscriber action to execute command dispatch at most once; if a telemetry implementation invokes the action again, the app returns a `SCE-ERR-RUNTIME` classified error rather than panicking or reparsing consumed arguments. +- `run_command_lifecycle` expects the telemetry subscriber action to execute command dispatch at most once; arguments and the mutable stderr reference are taken from single-use options before constructing the action future; if a telemetry implementation invokes the action again, the app returns a `SCE-ERR-RUNTIME` classified error rather than panicking or reparsing consumed arguments. ## Log directory config safety contract diff --git a/context/sce/cli-stdout-stderr-contract.md b/context/sce/cli-stdout-stderr-contract.md index cb1549a4f..0165e79b4 100644 --- a/context/sce/cli-stdout-stderr-contract.md +++ b/context/sce/cli-stdout-stderr-contract.md @@ -14,11 +14,10 @@ This document defines the implemented stream contract for CLI command payload an ## Implementation surface -- `run_with_dependency_check_and_streams(...)` is the app-level stream boundary for production and tests. -- `try_run_with_dependency_check(...)` performs parse + dispatch and returns payload text or classified errors. -- `dispatch(...)` returns payload text for each command path rather than writing directly to process streams. -- `write_stdout_payload(...)` handles success payload writes. -- `write_error_diagnostic(...)` handles redacted error writes. +- Async `app::run_with_dependency_check_and_streams(...)` borrows the output writers until execution and rendering finish. +- `app::try_run_with_dependency_check(...)` completes synchronous startup and directly awaits `run_command_lifecycle`, returning a `RunOutcome` containing payload text or a classified error, logger and optional startup diagnostic. +- `app_support::execute_command_phase(...)` awaits static `RuntimeCommand::execute_with_stderr(...)`, which returns the completed payload. Sync borrows the app's stderr writer for format-gated progress through its await. +- Synchronous `app_support::render_run_outcome(...)` owns final emission after execution completes; `write_stdout_payload(...)` handles success payload writes and `write_error_diagnostic(...)` handles redacted error writes in the same module. See also: `context/sce/cli-error-code-taxonomy.md` for the canonical error-code classes and `Try:` remediation injection rules. diff --git a/context/sce/shared-turso-db.md b/context/sce/shared-turso-db.md index 4b5507b0b..3eb41d477 100644 --- a/context/sce/shared-turso-db.md +++ b/context/sce/shared-turso-db.md @@ -36,6 +36,10 @@ - `collect_db_path_health()` emits common parent/path health problems for DB-backed services. - `bootstrap_db_parent()` creates the resolved DB parent directory for repair/setup flows. +## Application runtime compatibility + +The adapter remains synchronous and owns a current-thread Tokio runtime built by `build_current_thread_runtime`. When Tokio is already entered, `block_on_isolated` polls that runtime on a fresh scoped thread to protect reentry; this does not protect runtime destruction inside async execution. DB-capable dispatch and sync storage guards establish blocking construction/destruction scopes at the caller without changing DB APIs, retry/WAL/migration policies or credential storage. See the [dispatch lifetime inventory](../cli/cli-command-surface.md#application-runtime-dispatch) and [application execution runtime](../architecture.md#application-execution-runtime). PR2 is future work converting Turso to async and removing DB-owned runtimes, `build_current_thread_runtime`, `block_on_isolated`, and the DB-related `block_in_place` compatibility scopes together. + ## Transactional primitives `TursoDb` offers two generic multi-statement transaction primitives beyond diff --git a/nix/flatpak/cargo-sources.nix b/nix/flatpak/cargo-sources.nix index a2f56870d..258103e90 100644 --- a/nix/flatpak/cargo-sources.nix +++ b/nix/flatpak/cargo-sources.nix @@ -33,7 +33,7 @@ let outputHashMode = "flat"; outputHashAlgo = "sha256"; - outputHash = "sha256-6nNnt2G23RlrK0uZrtMGOovuq2kyLzmPyshiRA2Dj68="; + outputHash = "sha256-ZLcI8Basvo3MrOan7XnIUqq8UUFQkijzLGw/So3RFi0="; }; regenerateApp = pkgs.writeShellApplication { diff --git a/packaging/flatpak/cargo-sources.json b/packaging/flatpak/cargo-sources.json index 29277517f..c6bd0fcd2 100644 --- a/packaging/flatpak/cargo-sources.json +++ b/packaging/flatpak/cargo-sources.json @@ -5277,6 +5277,19 @@ "dest": "cargo/vendor/syn-2.0.118", "dest-filename": ".cargo-checksum.json" }, + { + "type": "archive", + "archive-type": "tar-gzip", + "url": "https://static.crates.io/crates/syn/syn-3.0.6.crate", + "sha256": "8593e8e72159ed2257d083c7a454a85cbf854f37a0966d8d483aff8c8a3ebcee", + "dest": "cargo/vendor/syn-3.0.6" + }, + { + "type": "inline", + "contents": "{\"package\": \"8593e8e72159ed2257d083c7a454a85cbf854f37a0966d8d483aff8c8a3ebcee\", \"files\": {}}", + "dest": "cargo/vendor/syn-3.0.6", + "dest-filename": ".cargo-checksum.json" + }, { "type": "archive", "archive-type": "tar-gzip", @@ -5576,6 +5589,19 @@ "dest": "cargo/vendor/tokio-1.52.3", "dest-filename": ".cargo-checksum.json" }, + { + "type": "archive", + "archive-type": "tar-gzip", + "url": "https://static.crates.io/crates/tokio-macros/tokio-macros-2.7.2.crate", + "sha256": "78773a2a397f451582ce068015985c33193cf6dea8b74d2a639fe457b2f07b0e", + "dest": "cargo/vendor/tokio-macros-2.7.2" + }, + { + "type": "inline", + "contents": "{\"package\": \"78773a2a397f451582ce068015985c33193cf6dea8b74d2a639fe457b2f07b0e\", \"files\": {}}", + "dest": "cargo/vendor/tokio-macros-2.7.2", + "dest-filename": ".cargo-checksum.json" + }, { "type": "archive", "archive-type": "tar-gzip",