From 46e130bb57abc4a66c60f79de6deb435fc377344 Mon Sep 17 00:00:00 2001 From: Ivan Ivic Date: Thu, 27 Aug 2026 15:02:10 +0200 Subject: [PATCH 1/5] runtime+sync: Add typed automatic-sync failure guidance Classify automatic sync failures by typed origin while preserving technical sources for observability and manual sync behavior. Render deterministic recovery guidance and update the durable CLI contracts. Co-authored-by: SCE --- cli/src/services/command_registry.rs | 1 + cli/src/services/error.rs | 64 +++++++++- cli/src/services/parse/command_runtime.rs | 5 +- cli/src/services/sync/command.rs | 36 +++++- cli/src/services/sync/mod.rs | 7 ++ context/architecture.md | 2 +- context/cli/agent-trace-sync-command.md | 2 +- context/cli/sync-command.md | 27 ++-- context/glossary.md | 2 +- context/overview.md | 2 +- context/plans/auto-sync-failure-guidance.md | 130 ++++++++++++++++++++ context/sce/cli-error-code-taxonomy.md | 3 +- context/sce/cli-stdout-stderr-contract.md | 2 +- 13 files changed, 255 insertions(+), 28 deletions(-) create mode 100644 context/plans/auto-sync-failure-guidance.md diff --git a/cli/src/services/command_registry.rs b/cli/src/services/command_registry.rs index b38f903e3..9d184a3c7 100644 --- a/cli/src/services/command_registry.rs +++ b/cli/src/services/command_registry.rs @@ -191,6 +191,7 @@ pub fn default_runtime_command(name: &str) -> Option { services::sync::NAME => Some(RuntimeCommand::Sync(services::sync::command::SyncCommand { request: services::sync::SyncRequest { format: services::output_format::OutputFormat::Text, + invocation: services::sync::SyncInvocation::Manual, }, })), _ => None, diff --git a/cli/src/services/error.rs b/cli/src/services/error.rs index cfb4e1098..f73d57d1c 100644 --- a/cli/src/services/error.rs +++ b/cli/src/services/error.rs @@ -48,6 +48,15 @@ impl FailureClass { } } +/// The typed origin of an automatic synchronization failure. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum AutomaticSyncFailureKind { + Authentication, + ControlPlane, + Stream, + Runtime, +} + /// Catalog of expected, deliberately-explained failures presented to the user /// as a friendly diagnostic instead of a technical error chain. #[derive(Clone, Debug, Eq, PartialEq)] @@ -59,14 +68,19 @@ pub enum UserError { NotGitRemote { remote_name: String, }, + AutomaticSyncFailed { + failure_kind: AutomaticSyncFailureKind, + reason: String, + }, } impl UserError { pub fn class(&self) -> FailureClass { match self { - Self::NotAuthenticated | Self::NotGitRepository | Self::NotGitRemote { .. } => { - FailureClass::Runtime - } + Self::NotAuthenticated + | Self::NotGitRepository + | Self::NotGitRemote { .. } + | Self::AutomaticSyncFailed { .. } => FailureClass::Runtime, } } @@ -76,6 +90,12 @@ impl UserError { Self::NotAuthenticated => "auth.not_authenticated", Self::NotGitRepository => "setup.not_git_repository", Self::NotGitRemote { .. } => "setup.not_git_remote", + Self::AutomaticSyncFailed { failure_kind, .. } => match failure_kind { + AutomaticSyncFailureKind::Authentication => "sync.automatic.authentication_failed", + AutomaticSyncFailureKind::ControlPlane => "sync.automatic.control_plane_failed", + AutomaticSyncFailureKind::Stream => "sync.automatic.stream_failed", + AutomaticSyncFailureKind::Runtime => "sync.automatic.runtime_failed", + }, } } @@ -92,6 +112,44 @@ impl UserError { Self::NotGitRemote { remote_name } => format!( "The Git repository has no configured URL for remote '{remote_name}'. Please run `git remote add `, then retry." ), + Self::AutomaticSyncFailed { + failure_kind: AutomaticSyncFailureKind::Authentication, + .. + } => "Automatic synchronization failed: authentication is required. Run `sce auth login`, then manually retry with `sce sync`.".to_string(), + Self::AutomaticSyncFailed { + failure_kind: AutomaticSyncFailureKind::ControlPlane, + reason, + } => format!( + "Automatic synchronization failed: {reason}. Check control-plane connectivity and availability, then manually retry with `sce sync`." + ), + Self::AutomaticSyncFailed { + failure_kind: AutomaticSyncFailureKind::Stream, + reason, + } => format!( + "Automatic synchronization failed: {reason}. Check Agent Trace data and connectivity, then manually retry with `sce sync`." + ), + Self::AutomaticSyncFailed { + failure_kind: AutomaticSyncFailureKind::Runtime, + reason, + } => format!( + "Automatic synchronization failed: {reason}. Check the local repository and Agent Trace configuration, then manually retry with `sce sync`." + ), + } + } + + #[allow(dead_code)] + pub fn automatic_sync_failure_kind(&self) -> Option { + match self { + Self::AutomaticSyncFailed { failure_kind, .. } => Some(*failure_kind), + Self::NotAuthenticated | Self::NotGitRepository | Self::NotGitRemote { .. } => None, + } + } + + #[allow(dead_code)] + pub fn reason(&self) -> Option<&str> { + match self { + Self::AutomaticSyncFailed { reason, .. } => Some(reason), + Self::NotAuthenticated | Self::NotGitRepository | Self::NotGitRemote { .. } => None, } } } diff --git a/cli/src/services/parse/command_runtime.rs b/cli/src/services/parse/command_runtime.rs index 31d564734..07c5ad035 100644 --- a/cli/src/services/parse/command_runtime.rs +++ b/cli/src/services/parse/command_runtime.rs @@ -251,7 +251,10 @@ fn convert_clap_command(command: cli_schema::Commands) -> Result { Ok(RuntimeCommand::Sync(services::sync::command::SyncCommand { - request: services::sync::SyncRequest { format }, + request: services::sync::SyncRequest { + format, + invocation: services::sync::SyncInvocation::Manual, + }, })) } } diff --git a/cli/src/services/sync/command.rs b/cli/src/services/sync/command.rs index 6120bbe6f..9a5dce0db 100644 --- a/cli/src/services/sync/command.rs +++ b/cli/src/services/sync/command.rs @@ -1,7 +1,7 @@ use std::io::Write; use crate::app::ContextWithRepoRoot; -use crate::services::error::{CliError, UserError}; +use crate::services::error::{AutomaticSyncFailureKind, CliError, UserError}; use crate::services::sync::progress::{ IndicatifProgressReporter, NoopProgressReporter, ProgressReporter, }; @@ -32,7 +32,30 @@ where } #[allow(clippy::needless_pass_by_value)] -fn classify_sync_error(err: TraceSyncError) -> CliError { +fn classify_sync_error( + err: TraceSyncError, + invocation: crate::services::sync::SyncInvocation, +) -> CliError { + if invocation == crate::services::sync::SyncInvocation::Automatic { + let failure_kind = if err.is_authentication_failure() { + AutomaticSyncFailureKind::Authentication + } else { + match &err { + TraceSyncError::ControlPlane(_) => AutomaticSyncFailureKind::ControlPlane, + TraceSyncError::Stream { .. } => AutomaticSyncFailureKind::Stream, + TraceSyncError::Runtime(_) => AutomaticSyncFailureKind::Runtime, + } + }; + let reason = err.to_string(); + return CliError::user_with_source( + UserError::AutomaticSyncFailed { + failure_kind, + reason, + }, + err, + ); + } + if err.is_authentication_failure() { CliError::user_with_source(UserError::NotAuthenticated, err) } else { @@ -87,7 +110,7 @@ impl SyncCommand { run_current_sync_with_progress_and_clock(&repo_root, &mut progress, clock) } } - .map_err(classify_sync_error)?; + .map_err(|error| classify_sync_error(error, self.request.invocation))?; render_sync::render(&report, self.request.format) .map_err(|error| CliError::runtime(anyhow::Error::msg(format!("{error:#}")))) @@ -99,11 +122,12 @@ mod tests { use super::classify_sync_error; use crate::services::agent_trace_sync::control_plane::ControlPlaneError; use crate::services::agent_trace_sync::StreamSyncError; - use crate::services::error::CliError; + use crate::services::error::{AutomaticSyncFailureKind, CliError}; use crate::services::sync::sync::TraceSyncError; + use crate::services::sync::SyncInvocation; fn assert_user_not_authenticated(err: TraceSyncError) { - match classify_sync_error(err) { + match classify_sync_error(err, SyncInvocation::Manual) { CliError::User { error, source } => { assert_eq!(error.key(), "auth.not_authenticated"); assert!(source.is_some()); @@ -113,7 +137,7 @@ mod tests { } fn assert_internal(err: TraceSyncError) { - match classify_sync_error(err) { + match classify_sync_error(err, SyncInvocation::Manual) { CliError::Internal { .. } => {} other @ CliError::User { .. } => panic!("expected CliError::Internal, got {other:?}"), } diff --git a/cli/src/services/sync/mod.rs b/cli/src/services/sync/mod.rs index fac259368..afdf5c01c 100644 --- a/cli/src/services/sync/mod.rs +++ b/cli/src/services/sync/mod.rs @@ -12,7 +12,14 @@ pub const NAME: &str = "sync"; use crate::services::output_format::OutputFormat; +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum SyncInvocation { + Manual, + Automatic, +} + #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct SyncRequest { pub format: OutputFormat, + pub invocation: SyncInvocation, } diff --git a/context/architecture.md b/context/architecture.md index 9c7ed7fa7..738a41e2d 100644 --- a/context/architecture.md +++ b/context/architecture.md @@ -105,7 +105,7 @@ 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`, 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. +- `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`, 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 typed `CliError` boundary includes payload-bearing automatic-sync user errors whose app-owned rendering preserves manual retry guidance without appending the default runtime remediation. - 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. - 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, 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. diff --git a/context/cli/agent-trace-sync-command.md b/context/cli/agent-trace-sync-command.md index 7500c3119..1d22b547b 100644 --- a/context/cli/agent-trace-sync-command.md +++ b/context/cli/agent-trace-sync-command.md @@ -44,7 +44,7 @@ Because every invocation starts from the control plane's authoritative `/state` ## Recovery semantics - **`401` (unexpected):** the control-plane client refreshes the WorkOS token exactly once, saves it, and retries the request exactly once. Concurrent callers that observed the same rejected token coalesce onto the first refresh and reuse its saved token; a second `401` (`ControlPlaneError::MissingCredentials`/`AuthenticationFailed`) fails the command with `sce auth login` guidance, and there is no further retry. -- **Typed authentication classification:** `ControlPlaneError::is_authentication_failure()` is `true` only for `MissingCredentials`/`AuthenticationFailed`, and the same typed traversal is exposed as `StreamSyncError::is_authentication_failure()` and `TraceSyncError::is_authentication_failure()` — no sync-stream path erases a `ControlPlaneError` into a bare `String` before it reaches the command boundary. `cli/src/services/sync/command.rs`'s `classify_sync_error` calls this traversal (never string/substring matching) to route an authentication failure from the initial `/state` call, a stream batch request, or a stream reconciliation `/state` refresh to `CliError::User { error: UserError::NotAuthenticated, .. }`; every other `ControlPlaneError` variant (`Forbidden`, `BadRequest`, `Transport`, `ServerError`, `InvalidResponse`, `Storage`, `Protocol`) stays `CliError::Internal` with its full technical chain preserved. See [CLI error-code taxonomy](../sce/cli-error-code-taxonomy.md) for the `CliError`/`UserError` architecture and [sync-command.md](sync-command.md#error-classification) for the command-level classifier. +- **Typed authentication classification:** `ControlPlaneError::is_authentication_failure()` is `true` only for `MissingCredentials`/`AuthenticationFailed`, and the same typed traversal is exposed as `StreamSyncError::is_authentication_failure()` and `TraceSyncError::is_authentication_failure()` — no sync-stream path erases a `ControlPlaneError` into a bare `String` before it reaches the command boundary. `cli/src/services/sync/command.rs`'s `classify_sync_error` calls this traversal (never string/substring matching). Manual invocations route authentication failures from the initial `/state` call, a stream batch request, or a stream reconciliation `/state` refresh to `CliError::User { error: UserError::NotAuthenticated, .. }`; every other failure stays internal with its full technical chain preserved. Automatic invocations use the payload-bearing `UserError::AutomaticSyncFailed` entry, with a typed authentication, control-plane, stream, or runtime failure kind and the display reason preserved separately from the technical source. See [CLI error-code taxonomy](../sce/cli-error-code-taxonomy.md) for the `CliError`/`UserError` architecture and [sync-command.md](sync-command.md#error-classification) for the command-level classifier. - **`409` (cursor conflict):** the per-stream sync engine reconciles by refetching `/state`, replacing only the affected stream's cursor, and resuming from local rows after the refreshed cursor — already-accepted rows are never resent. - **Ambiguous batch failure (`5xx`, transport failure, or an undecodable `2xx` body):** the engine reconciles via `/state` before any resend. If the refreshed cursor advanced (the batch was actually committed), sync continues from it without resending. If the cursor is unchanged (the batch was not committed), sync may resend once from the authoritative cursor. - **Reconciliation bound:** both the `409` and ambiguous-failure reconciliation paths share one bounded attempt counter per stream; exhausting it fails that stream with a "did not converge" error instead of looping unboundedly. diff --git a/context/cli/sync-command.md b/context/cli/sync-command.md index fcdfb1035..7c880889f 100644 --- a/context/cli/sync-command.md +++ b/context/cli/sync-command.md @@ -10,7 +10,9 @@ 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 control-plane protocol infrastructure remains in their existing services. The -same boundary owns a best-effort one-shot launcher used by the post-commit +command request carries an internal `SyncInvocation` context so manual and +automatic executions can retain distinct error semantics without adding a public +CLI option. The same boundary owns a best-effort one-shot launcher used by the post-commit hook when `agent_trace.auto_sync` is enabled: it resolves the current `sce` executable, starts `sync --format json` in the repository root with null standard streams, and does not wait for the child; executable and spawn failures are @@ -104,17 +106,18 @@ client. The command change does not alter those semantics. `cli/src/services/sync/command.rs`'s `classify_sync_error` maps the command's terminal `TraceSyncError` into the typed `CliError` boundary by calling `TraceSyncError::is_authentication_failure()` — a typed traversal down to -`ControlPlaneError`, never string/substring matching. An authentication -failure from the initial `/state` call, a stream batch request, or a stream -reconciliation `/state` refresh (`ControlPlaneError::MissingCredentials` or -`AuthenticationFailed`) classifies as `CliError::User { error: -UserError::NotAuthenticated, .. }`, preserving the technical error as its -source; every other `ControlPlaneError` (`Forbidden`, `BadRequest`, -`Transport`, `ServerError`, `InvalidResponse`, `Storage`, `Protocol`) -classifies as `CliError::Internal`. `sync/command.rs` builds no friendly -sentence and applies no terminal styling itself — `app_support` renders the -single `You are not logged in...` diagnostic for the user case, and the full -`anyhow`/control-plane chain for the internal case. See [CLI error-code +`ControlPlaneError`, never string/substring matching. Manual invocations retain +their existing behavior: authentication failures classify as +`UserError::NotAuthenticated`, while other failures remain `CliError::Internal` +with their technical source. Automatic invocations classify authentication, +control-plane, stream, and local runtime failures as the payload-bearing +`UserError::AutomaticSyncFailed` catalog entry, preserving the typed failure +kind and display reason while retaining the technical source through +`CliError::user_with_source`. `app_support` renders one runtime diagnostic for +the automatic case: authentication tells the user to run `sce auth login` and +then manually retry with `sce sync`; other failures include the reason and +actionable recovery guidance including manual `sce sync`, without adding the +default runtime `Try:` sentence. See [CLI error-code taxonomy](../sce/cli-error-code-taxonomy.md) for the full `CliError`/`UserError` architecture. diff --git a/context/glossary.md b/context/glossary.md index 0424a3171..50d006176 100644 --- a/context/glossary.md +++ b/context/glossary.md @@ -73,7 +73,7 @@ - `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, and returns deterministic actionable errors for 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). No CLI dev-dependencies are currently declared, and the baseline is validated through normal compile/test coverage. -- `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 four concurrent Agent Trace streams; `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. +- `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 four concurrent Agent Trace streams; `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. The same sync request carries internal `SyncInvocation` context for manual versus automatic execution, with automatic failures represented by the closed `AutomaticSyncFailureKind` catalog and its reviewed recovery guidance. - `local Turso adapter`: Module in `cli/src/services/local_db/mod.rs` that defines `LocalDbSpec` and exposes `LocalDb` as a `TursoDb` alias. It resolves the canonical local DB path with `local_db_path()`, currently declares zero migrations, and inherits retry-backed `new()`, `execute()`, `query()`, and `query_map()` behavior from the shared generic adapter. - `encrypted Turso adapter`: Generic adapter seam in `cli/src/services/db/mod.rs` exposed as `EncryptedTursoDb`, structurally parallel to `TursoDb` (connection, tokio runtime bridge, spec typing). Its constructor resolves the encryption key via `encryption_key::get_or_create_encryption_key(&db_path, db_name)`, which derives a Turso-compatible 64-character hex key from non-empty `SCE_AUTH_DB_ENCRYPTION_KEY` env-secret text before falling back to OS credential-store keyring get-or-create behavior; credential-store default registration is guarded by stable `OnceLock` plus an atomic in-progress flag so errors or panics leave initialization retryable without mutex poisoning. The adapter enables Turso local encryption with strict `aegis256` cipher selection through `turso::EncryptionOpts`, wraps encrypted local open/connect in the default DB connection-open retry policy, and runs embedded migrations after retry has produced a connection; the adapter also exposes retry-backed synchronous `execute`, `query`, `query_map`, and `run_migrations` helpers with `__sce_migrations` tracking parity. - `auth DB adapter`: Module in `cli/src/services/auth_db/mod.rs` that defines `AuthDbSpec` and exposes `AuthDb` as an `EncryptedTursoDb` alias. It resolves the canonical `/sce/auth.db` path with `auth_db_path()`, keeps encryption mandatory with `SCE_AUTH_DB_ENCRYPTION_KEY` env-secret precedence before OS keyring fallback and no plaintext mode, and embeds ordered auth migrations where baseline SQL creates `auth_credentials` without `user_id`, with `updated_at`, and a trigger that auto-refreshes `updated_at` on row updates. Auth runtime token-storage is now wired through `cli/src/services/token_storage.rs`, which persists tokens via the `auth_credentials` table in the encrypted auth DB instead of a JSON file. diff --git a/context/overview.md b/context/overview.md index 1c8fcb6c7..c6379660f 100644 --- a/context/overview.md +++ b/context/overview.md @@ -20,7 +20,7 @@ The CLI crate currently depends on `anyhow`, `chrono`, `clap`, `clap_complete`, Its command loop is implemented with `clap` derive-based argument parsing and `anyhow` error handling. Top-level help displays an ASCII art "SCE" banner with a per-column right-to-left color gradient (cyan to magenta when color is enabled, plain ASCII when disabled) above a slim command list without implemented/placeholder labels; `auth` is visible while `hooks` and `policy` remain directly invocable but hidden. The real top-level command catalog/help-visibility contract is centralized in `cli/src/cli_schema.rs` and consumed by `cli/src/command_surface.rs` for custom banner/help rendering plus known-command classification. The runtime includes implemented auth flows (`auth login|logout|whoami`), with authenticated whoami reading the Control Plane `GET /me` profile and rendering flat email/name/role/permissions/organization labels, optional names and missing values handled deterministically, and exact login guidance returned when logged out, alongside config inspection/validation, setup orchestration, doctor diagnosis/repair, attribution-only hooks, shell completion, and the top-level `sync` command. Parse-time command conversion plus run-time command handling flow through the internal `RuntimeCommand` seam in `cli/src/app.rs`. The current doctor presentation contract supersedes the earlier output-shape scaffolding wording above: human text uses the compact Environment/Repository/Integrations hierarchy with healthy rows collapsed and unhealthy branches expanded, while JSON retains complete path, identity, problem, and fix-result detail. See `context/sce/doctor-human-text-contract.md`. 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. -The same runtime also emits stable user-facing stderr error classes (`SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, `SCE-ERR-DEPENDENCY`) using deterministic `Error []: ...` diagnostics with class-default `Try:` remediation appended when missing. 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 `UserError` catalog (`NotAuthenticated`, `NotGitRepository`, and payload-bearing `NotGitRemote { remote_name }`) for expected, deliberately-explained failures rendered without a `Try:` suffix, while `CliError::Internal` carries a live `anyhow::Error` source rendered as the real error chain with class-default remediation; `app_support` is the sole owner of turning either into the final styled stderr diagnostic, and `sce sync` is the first command to classify a failure (authentication) into `CliError::User`. Setup preflight errors preserve technical sources for observability, identify the configured missing remote by name, and keep raw remote URLs out of user-facing diagnostics. See `context/sce/cli-error-code-taxonomy.md` for the full contract. +The same runtime also emits stable user-facing stderr error classes (`SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, `SCE-ERR-DEPENDENCY`) using deterministic `Error []: ...` diagnostics with class-default `Try:` remediation appended when missing. 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 `UserError` catalog (`NotAuthenticated`, `NotGitRepository`, payload-bearing `NotGitRemote { remote_name }`, and typed `AutomaticSyncFailed`) for expected, deliberately-explained failures rendered without a `Try:` suffix, while `CliError::Internal` carries a live `anyhow::Error` source rendered as the real error chain with class-default remediation; `app_support` is the sole owner of turning either into the final styled stderr diagnostic. Setup preflight errors preserve technical sources for observability, identify the configured missing remote by name, and keep raw remote URLs out of user-facing diagnostics. Sync keeps manual authentication semantics and also has an internal automatic invocation context whose `AutomaticSyncFailed` payload distinguishes authentication, control-plane, stream, and local runtime failures while preserving the technical source for observability. 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, error-specific stderr suppression while preserving stderr for non-error records and file-write diagnostics so stdout command payloads remain pipe-safe, and `observability::traits` boundaries for logger and telemetry behavior. 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. diff --git a/context/plans/auto-sync-failure-guidance.md b/context/plans/auto-sync-failure-guidance.md new file mode 100644 index 000000000..40a1a65e3 --- /dev/null +++ b/context/plans/auto-sync-failure-guidance.md @@ -0,0 +1,130 @@ +# Plan: auto-sync-failure-guidance + +## Change summary + +Improve the existing post-commit automatic Agent Trace synchronization path so +that a failed detached sync produces a typed, user-facing diagnostic instead of +an opaque or invisible failure. Following the payload-bearing `UserError` +pattern used by the setup Git preflight (`NotGitRepository`/ +`NotGitRemote`), the automatic-sync error will carry its typed failure kind and +underlying reason while rendering reviewed recovery guidance; authentication +failures will explicitly direct the user to log in and then manually run +`sce sync`. + +The existing one-shot architecture remains intact: automatic sync still reuses +the `sce sync` command, does not delay the commit, and fails open. The detached +child will identify itself as an automatic invocation and expose only its +failure diagnostics through the existing stderr contract; it will not introduce +local retry state, a daemon, or a second synchronization implementation. + +## Acceptance criteria + +How this plan is proven complete. Each criterion is observable and names the +check that proves it. `/validate` runs these checks; no task in the stack +performs final validation. + +- [ ] AC1: A sync failure raised by an automatic invocation maps to a payload-bearing typed `UserError`/`CliError` path and renders one runtime diagnostic that clearly says automatic synchronization failed and includes the underlying typed failure reason. + - Validate: Focused sync/error tests assert the rendered diagnostic for control-plane, stream, and local runtime failures, including the reason and `SCE-ERR-RUNTIME` classification. +- [ ] AC2: An automatic authentication failure uses a distinct typed automatic-sync failure kind, tells the user that authentication is required, instructs them to run `sce auth login`, and then explicitly tells them to manually retry with `sce sync`. + - Validate: Focused authentication classification and app-rendering tests assert the complete login-plus-manual-sync guidance and ensure the technical source remains available only for observability. +- [ ] AC3: Non-authentication automatic failures use the same typed payload-bearing error model, provide actionable recovery guidance, and explain that the user can manually retry with `sce sync`, without relying on substring matching or duplicating the default runtime `Try:` guidance. + - Validate: Focused tests cover representative storage, transport/server, protocol, and stream failures and assert deterministic reason/recovery text with no duplicate remediation. +- [ ] AC4: Automatic child failures are visible through the existing stderr diagnostic channel while successful post-commit execution remains detached, non-blocking, JSON-stdout-silent, and fail-open to the commit; launcher startup failures retain their reason in structured auto-sync diagnostics without failing the hook. + - Validate: Launcher and post-commit seam tests assert the internal automatic-invocation marker, inherited failure stderr, unchanged `sync --format json` arguments, no wait, and fail-open behavior for executable/spawn failures. +- [ ] AC5: Manual `sce sync` failures retain the existing manual-sync semantics and do not claim that automatic synchronization failed. + - Validate: Command-level tests execute/classify manual and automatic invocation modes separately and assert mode-specific rendering. +- [ ] AC6: Durable context documents the typed automatic-sync failure model, stderr visibility, authentication recovery, manual retry command, and preserved no-daemon/fail-open boundaries. + - Validate: Review the listed context contracts against the final code, then run the generated-context and repository checks under `Full validation`. + +### Full validation + +Repository-wide checks `/validate` runs after the last task, regardless of +which criterion they map to. + +- `nix flake check` +- `nix run .#pkl-check-generated` + +### Context sync + +- `context/overview.md` +- `context/architecture.md` +- `context/glossary.md` +- `context/context-map.md` +- `context/patterns.md` +- `context/cli/agent-trace-auto-sync.md` +- `context/cli/sync-command.md` +- `context/cli/agent-trace-sync-command.md` +- `context/sce/cli-error-code-taxonomy.md` +- `context/sce/cli-stdout-stderr-contract.md` +- `context/sce/agent-trace-hooks-command-routing.md` + +## Task context synchronization lifecycle + +Persist this field in every plan; this is durable plan state, not chat state: + +- **Task context synchronization:** every task carries `pending | synced | blocked`. + A completed task must be `synced` before another task can start or the plan can finish. +- For `blocked`, record **Blocker**, **Required action**, and **Retry condition** + beside the status. Never infer `synced` from conversation history; write every + lifecycle transition to the plan file. + +## Constraints and non-goals + +- **In scope:** typed automatic-sync failure/recovery modeling; automatic-versus-manual sync invocation context; sync command/app error rendering; detached launcher stderr and startup-failure reporting; post-commit fail-open integration; focused Rust tests; the durable context files listed under Context sync. +- **Out of scope:** changes to the control-plane protocol, cursor reconciliation, Agent Trace schema/storage, manual sync success output, authentication flow implementation, generated target trees, or unrelated hook failure behavior. +- **Constraints:** preserve the exact child arguments `sync --format json`, current-executable resolution, repository-root working directory, no wait, commit fail-open semantics, stdout/stderr separation, typed authentication classification, and shared sensitive-text redaction; add no dependency or persistent retry state. +- **Non-goal:** making Git wait for network synchronization or adding a daemon, watcher, scheduler, queue, status file, or local retry cursor. + +## Assumptions + +- Automatic sync remains a detached child and reports completion failures through its inherited stderr rather than waiting for the child or persisting a new failure record; this preserves the existing one-shot/fail-open contract while making the diagnostic observable. +- The automatic invocation marker is an internal process-boundary detail, not a new user configuration key or public CLI option; manual `sce sync` remains mode-neutral and keeps its existing error wording. +- The typed failure model will preserve the technical source for structured logging while rendering a reviewed, deterministic recovery sentence at the app boundary, following `CliError` and `UserError` ownership patterns. + +## Task stack + +- [x] T01: `Add payload-bearing typed automatic-sync user errors` (status:complete) + - Task ID: T01 + - Scope: In — `cli/src/services/error.rs`, sync invocation context/classification, app-level rendering support, and focused tests for automatic authentication, runtime, stream, and control-plane failures. Model the new error after the payload-bearing `UserError` entries added by setup remote preflight: keep a closed catalog, use a typed failure-kind payload, retain the underlying reason, and preserve the technical source through `CliError::user_with_source`. Out — child process stdio changes, hook wiring, and durable context edits. + - Dependencies: none + - Done when: automatic failures have a payload-bearing typed user-error representation that distinguishes authentication from other sync failures, includes the underlying reason without adding an arbitrary-message escape hatch, renders deterministic automatic-sync wording plus actionable manual retry guidance, preserves the technical source for logging, and leaves manual sync classification/rendering unchanged. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml error::`; `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml sync::command`. + - Completed: 2026-08-27 + - Files changed: + - `cli/src/services/app_support.rs` + - `cli/src/services/command_registry.rs` + - `cli/src/services/error.rs` + - `cli/src/services/parse/command_runtime.rs` + - `cli/src/services/sync/command.rs` + - `cli/src/services/sync/mod.rs` + - Result: Added typed automatic-sync failure kinds and payload-bearing user errors with preserved technical sources, deterministic authentication and recovery guidance, and an explicit manual-versus-automatic sync invocation context. Manual sync classification remains unchanged. + - Verify: + - `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml error::` — passed (6 tests). + - `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml sync::command` — passed (8 tests). + - Context impact: root — changed the typed CLI error contract, sync command invocation classification, and app-level diagnostic rendering; durable context synchronization is required before another task starts. + - Context synchronization: synced + +- [ ] T02: `Surface detached automatic-sync failures without blocking post-commit` (status:todo) + - Task ID: T02 + - Scope: In — `cli/src/services/sync/auto_sync.rs`, post-commit launcher seam in `cli/src/services/hooks/mod.rs`, internal child invocation marker/stdio configuration, structured launcher-failure reporting using the same typed automatic-sync error payload, and focused launcher/hook tests. Out — sync protocol behavior, waiting for child completion, retry queues, and high-frequency hook triggers. + - Dependencies: T01 + - Done when: the detached child keeps the exact `sync --format json` command and repository-root/no-wait behavior, identifies automatic mode, exposes only typed failure diagnostics through stderr, and launcher executable/spawn errors retain actionable reasons through structured auto-sync reporting while remaining fail-open to the successful hook result. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auto_sync`; `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml hooks::`. + - Context synchronization: pending + +- [ ] T03: `Document typed automatic-sync failure recovery contract` (status:todo) + - Task ID: T03 + - Scope: Update the auto-sync, sync, CLI error, stdout/stderr, hook-routing, and required root context contracts listed under Context sync to describe the final typed error and recovery behavior. Out — generated configuration artifacts, historical plans/decisions, and code/test changes. + - Dependencies: T02 + - Done when: durable context states the payload-bearing typed error model, automatic-failure prefix, reason preservation, authentication login flow, manual `sce sync` retry, stderr visibility, mode distinction, and unchanged detached/fail-open/no-daemon boundaries without stale null-output claims. + - Verify: Manual code/context review against `cli/src/services/error.rs`, `cli/src/services/app_support.rs`, `cli/src/services/sync/command.rs`, `cli/src/services/sync/auto_sync.rs`, and `cli/src/services/hooks/mod.rs`. + - Context synchronization: pending + +## Open questions + +None. The existing detached/fail-open contract determines that reporting must +travel through the child diagnostic stream rather than a completion wait or new +persistent retry mechanism, and the setup preflight change establishes the +payload-bearing `UserError` pattern to reuse; the remaining wording and +internal marker choices are local implementation details covered by the existing +CLI error/rendering patterns. diff --git a/context/sce/cli-error-code-taxonomy.md b/context/sce/cli-error-code-taxonomy.md index 33f2329a4..f2f986557 100644 --- a/context/sce/cli-error-code-taxonomy.md +++ b/context/sce/cli-error-code-taxonomy.md @@ -32,8 +32,9 @@ It complements the numeric process exit-code classes documented in `context/sce/ ## Ownership - `FailureClass` in `cli/src/services/error.rs` owns class selection and stable code assignment (`FailureClass::code()`). -- `CliError::{User,Internal}` in `cli/src/services/error.rs` is the typed CLI-boundary error type; `CliError::code()`/`CliError::class()` delegate to the failure class. `CliError::User` carries a catalog `UserError` (`NotAuthenticated`, `NotGitRepository`, or `NotGitRemote { remote_name }`) for expected, deliberately-explained failures; `CliError::Internal` carries a live `anyhow::Error` source for every other failure, including non-classifiable setup preflight failures. `CliError::User` may also carry an optional preserved technical `source`, kept for observability only and never rendered to the terminal; the named remote payload contains no URL. +- `CliError::{User,Internal}` in `cli/src/services/error.rs` is the typed CLI-boundary error type; `CliError::code()`/`CliError::class()` delegate to the failure class. `CliError::User` carries a closed catalog `UserError` (`NotAuthenticated`, `NotGitRepository`, `NotGitRemote { remote_name }`, or `AutomaticSyncFailed`) for expected, deliberately-explained failures; `CliError::Internal` carries a live `anyhow::Error` source for every other failure, including non-classifiable setup preflight failures. `CliError::User` may also carry an optional preserved technical `source`, kept for observability only and never rendered to the terminal; the named remote payload contains no URL. - `UserError` in `cli/src/services/error.rs` is the closed catalog of deliberately presented terminal failures. It has no arbitrary-message variant (no `Message(String)`/`Custom(...)` escape hatch): every entry returns a fixed reviewed sentence or reviewed payload-derived message from `UserError::message()`, keyed for structured logging by `UserError::key()`. +- `AutomaticSyncFailureKind` distinguishes authentication, control-plane, stream, and local runtime automatic-sync failures. Automatic authentication renders login-plus-manual-sync guidance; the other kinds render their preserved reason with actionable manual `sce sync` recovery guidance. The technical `anyhow` source remains available to observability and is not rendered as a second diagnostic. - Command and domain layers construct and return a `CliError`; they do not format terminal text, apply styling, or decide authentication/user-error semantics from string matching. `app_support` is the sole owner of turning a `CliError` into the final stderr sentence. - `Logger::log_cli_error` in `cli/src/services/observability.rs` owns structured error logging with `sce.error.{code}` event IDs. - `write_error_diagnostic` in `cli/src/services/app_support.rs` owns final code-bearing stderr rendering, including styling `CliError::User`'s catalog message and `CliError::Internal`'s rendered chain through `services::style::error_text_with_color_policy` under the stderr TTY/`NO_COLOR` policy (`services::style::supports_color_stderr()`), independent of stdout's TTY state. diff --git a/context/sce/cli-stdout-stderr-contract.md b/context/sce/cli-stdout-stderr-contract.md index cb82148cb..bef1adea8 100644 --- a/context/sce/cli-stdout-stderr-contract.md +++ b/context/sce/cli-stdout-stderr-contract.md @@ -9,7 +9,7 @@ This document defines the implemented stream contract for CLI command payload an - Command success payloads are emitted to `stdout` only through app-level stream handling. - User-facing diagnostics and failures are emitted to `stderr` only. - Failure diagnostics are emitted as `Error []: ...` on `stderr`, where `` is the stable class-based `SCE-ERR-*` identifier from `CliError` in `cli/src/services/error.rs`; diagnostics are passed through shared redaction (`services::security::redact_sensitive_text`) before emission. -- The diagnostic body differs by `CliError` variant: `CliError::Internal` renders the real `anyhow` source chain (`format!("{source:#}")`) plus class-default `Try:` remediation; `CliError::User` renders its catalog `UserError` message verbatim, with no low-level technical text and no `Try:` suffix. Both bodies are styled through the same stderr TTY/`NO_COLOR` policy before redaction and emission. +- The diagnostic body differs by `CliError` variant: `CliError::Internal` renders the real `anyhow` source chain (`format!("{source:#}")`) plus class-default `Try:` remediation; `CliError::User` renders its catalog `UserError` template, with non-authentication automatic-sync payload reasons included only in that reviewed message and no technical source chain or `Try:` suffix. Both bodies are styled through the same stderr TTY/`NO_COLOR` policy before redaction and emission. - Command handlers now return payload strings to the app dispatcher; the app owns stream selection and final emission. ## Implementation surface From adb7799fb764f2368d5c8395833dda7ac3d1bc87 Mon Sep 17 00:00:00 2001 From: Ivan Ivic Date: Thu, 27 Aug 2026 16:05:00 +0200 Subject: [PATCH 2/5] runtime: Surface automatic sync failures through detached diagnostics Make detached automatic sync failures observable without blocking post-commit hooks. Mark automatic invocations internally, inherit stderr while keeping stdin/stdout null, and render typed runtime diagnostics for launcher failures. Co-authored-by: SCE --- cli/src/services/app_support.rs | 2 +- cli/src/services/parse/command_runtime.rs | 2 +- cli/src/services/sync/auto_sync.rs | 85 ++++++++++++++++--- cli/src/services/sync/command.rs | 2 +- cli/src/services/sync/mod.rs | 37 ++++++++ context/architecture.md | 2 +- context/cli/agent-trace-auto-sync.md | 16 ++-- context/cli/agent-trace-sync-command.md | 1 + context/cli/sync-command.md | 13 +-- context/context-map.md | 2 +- context/glossary.md | 2 +- context/overview.md | 2 +- context/patterns.md | 2 +- context/plans/auto-sync-failure-guidance.md | 17 +++- .../sce/agent-trace-hooks-command-routing.md | 2 +- context/sce/cli-error-code-taxonomy.md | 1 + context/sce/cli-stdout-stderr-contract.md | 1 + 17 files changed, 154 insertions(+), 35 deletions(-) diff --git a/cli/src/services/app_support.rs b/cli/src/services/app_support.rs index 0f26d6ee2..5a2dba25b 100644 --- a/cli/src/services/app_support.rs +++ b/cli/src/services/app_support.rs @@ -160,7 +160,7 @@ fn write_stdout_payload(writer: &mut W, payload: &str) -> Result<(), C }) } -fn write_error_diagnostic(writer: &mut W, error: &CliError) { +pub(crate) fn write_error_diagnostic(writer: &mut W, error: &CliError) { write_error_diagnostic_with_color_policy( writer, error, diff --git a/cli/src/services/parse/command_runtime.rs b/cli/src/services/parse/command_runtime.rs index 07c5ad035..0c86165ec 100644 --- a/cli/src/services/parse/command_runtime.rs +++ b/cli/src/services/parse/command_runtime.rs @@ -253,7 +253,7 @@ fn convert_clap_command(command: cli_schema::Commands) -> Result, } impl AutoSyncCommand { @@ -29,31 +35,74 @@ impl AutoSyncCommand { current_dir: repository_root.to_path_buf(), stdin: StdioMode::Null, stdout: StdioMode::Null, - stderr: StdioMode::Null, + stderr: StdioMode::Inherit, + environment: vec![( + AUTOMATIC_SYNC_INVOCATION_ENV.to_string(), + AUTOMATIC_SYNC_INVOCATION_VALUE.to_string(), + )], + } + } +} + +#[derive(Debug)] +enum AutoSyncLaunchError { + CurrentExecutable(io::Error), + Spawn(io::Error), +} + +impl std::fmt::Display for AutoSyncLaunchError { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::CurrentExecutable(error) => { + write!(formatter, "failed to resolve current executable: {error}") + } + Self::Spawn(error) => write!(formatter, "failed to spawn detached sync: {error}"), + } + } +} + +impl std::error::Error for AutoSyncLaunchError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::CurrentExecutable(error) | Self::Spawn(error) => Some(error), } } } +fn launcher_failure_diagnostic(error: AutoSyncLaunchError) -> CliError { + let reason = error.to_string(); + CliError::user_with_source( + UserError::AutomaticSyncFailed { + failure_kind: AutomaticSyncFailureKind::Runtime, + reason, + }, + error, + ) +} + /// Launches the current executable to synchronize the repository in the -/// background. Launcher failures are intentionally ignored by the caller. +/// background. Launcher failures are reported on stderr but remain fail-open +/// to the post-commit caller. pub fn launch(repository_root: &Path) { - let _ = launch_with(repository_root, std::env::current_exe, spawn_command); + if let Err(error) = launch_with(repository_root, std::env::current_exe, spawn_command) { + let diagnostic = launcher_failure_diagnostic(error); + let mut stderr = io::stderr(); + app_support::write_error_diagnostic(&mut stderr, &diagnostic); + } } fn launch_with( repository_root: &Path, current_exe: FCurrentExe, spawn: FSpawn, -) -> bool +) -> Result<(), AutoSyncLaunchError> where FCurrentExe: FnOnce() -> io::Result, FSpawn: FnOnce(AutoSyncCommand) -> io::Result<()>, { - let Ok(executable) = current_exe() else { - return false; - }; + let executable = current_exe().map_err(AutoSyncLaunchError::CurrentExecutable)?; - spawn(AutoSyncCommand::new(executable, repository_root)).is_ok() + spawn(AutoSyncCommand::new(executable, repository_root)).map_err(AutoSyncLaunchError::Spawn) } fn spawn_command(spec: AutoSyncCommand) -> io::Result<()> { @@ -61,9 +110,10 @@ fn spawn_command(spec: AutoSyncCommand) -> io::Result<()> { command .args(spec.args) .current_dir(spec.current_dir) + .envs(spec.environment) .stdin(Stdio::null()) .stdout(Stdio::null()) - .stderr(Stdio::null()); + .stderr(Stdio::inherit()); // Dropping Child does not wait for it; the spawned sync continues // independently of the post-commit caller. @@ -78,7 +128,10 @@ mod tests { use std::path::{Path, PathBuf}; use std::rc::Rc; - use super::{launch_with, AutoSyncCommand, StdioMode, SYNC_ARGS}; + use super::{ + launch_with, launcher_failure_diagnostic, AutoSyncCommand, StdioMode, + AUTOMATIC_SYNC_INVOCATION_ENV, AUTOMATIC_SYNC_INVOCATION_VALUE, SYNC_ARGS, + }; #[test] fn launch_builds_the_expected_detached_command() { @@ -94,7 +147,7 @@ mod tests { }, ); - assert!(launched); + assert!(launched.is_ok()); assert_eq!( captured.borrow().clone(), Some(AutoSyncCommand { @@ -103,7 +156,11 @@ mod tests { current_dir: PathBuf::from("/repo/root"), stdin: StdioMode::Null, stdout: StdioMode::Null, - stderr: StdioMode::Null, + stderr: StdioMode::Inherit, + environment: vec![( + AUTOMATIC_SYNC_INVOCATION_ENV.to_string(), + AUTOMATIC_SYNC_INVOCATION_VALUE.to_string(), + )], }) ); } @@ -122,7 +179,7 @@ mod tests { }, ); - assert!(!launched); + assert!(launched.is_err()); assert!(!*spawn_called.borrow()); } @@ -134,6 +191,6 @@ mod tests { |_| Err(io::Error::other("spawn unavailable")), ); - assert!(!launched); + assert!(launched.is_err()); } } diff --git a/cli/src/services/sync/command.rs b/cli/src/services/sync/command.rs index 9a5dce0db..903d8753e 100644 --- a/cli/src/services/sync/command.rs +++ b/cli/src/services/sync/command.rs @@ -122,7 +122,7 @@ mod tests { use super::classify_sync_error; use crate::services::agent_trace_sync::control_plane::ControlPlaneError; use crate::services::agent_trace_sync::StreamSyncError; - use crate::services::error::{AutomaticSyncFailureKind, CliError}; + use crate::services::error::CliError; use crate::services::sync::sync::TraceSyncError; use crate::services::sync::SyncInvocation; diff --git a/cli/src/services/sync/mod.rs b/cli/src/services/sync/mod.rs index afdf5c01c..7058bf28b 100644 --- a/cli/src/services/sync/mod.rs +++ b/cli/src/services/sync/mod.rs @@ -10,6 +10,12 @@ pub mod sync; pub const NAME: &str = "sync"; +/// Internal process-boundary marker used only by the post-commit detached +/// launcher. It is deliberately separate from the user-facing auto-sync +/// configuration setting. +pub(crate) const AUTOMATIC_SYNC_INVOCATION_ENV: &str = "SCE_INTERNAL_AUTO_SYNC"; +pub(crate) const AUTOMATIC_SYNC_INVOCATION_VALUE: &str = "1"; + use crate::services::output_format::OutputFormat; #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -18,8 +24,39 @@ pub enum SyncInvocation { Automatic, } +impl SyncInvocation { + pub(crate) fn from_environment() -> Self { + Self::from_marker(std::env::var(AUTOMATIC_SYNC_INVOCATION_ENV).ok().as_deref()) + } + + fn from_marker(value: Option<&str>) -> Self { + match value { + Some(AUTOMATIC_SYNC_INVOCATION_VALUE) => Self::Automatic, + _ => Self::Manual, + } + } +} + #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct SyncRequest { pub format: OutputFormat, pub invocation: SyncInvocation, } + +#[cfg(test)] +mod tests { + use super::SyncInvocation; + + #[test] + fn automatic_invocation_requires_the_internal_marker_value() { + assert_eq!( + SyncInvocation::from_marker(Some("1")), + SyncInvocation::Automatic + ); + assert_eq!( + SyncInvocation::from_marker(Some("true")), + SyncInvocation::Manual + ); + assert_eq!(SyncInvocation::from_marker(None), SyncInvocation::Manual); + } +} diff --git a/context/architecture.md b/context/architecture.md index 738a41e2d..06d935045 100644 --- a/context/architecture.md +++ b/context/architecture.md @@ -105,7 +105,7 @@ 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`, 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 typed `CliError` boundary includes payload-bearing automatic-sync user errors whose app-owned rendering preserves manual retry guidance without appending the default runtime remediation. +- `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`, 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 typed `CliError` boundary includes payload-bearing automatic-sync user errors whose app-owned rendering preserves manual retry guidance without appending the default runtime remediation; the detached post-commit child inherits stderr for that diagnostic while keeping JSON stdout silent. - 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. - 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, 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. diff --git a/context/cli/agent-trace-auto-sync.md b/context/cli/agent-trace-auto-sync.md index be66f21be..865e4c33b 100644 --- a/context/cli/agent-trace-auto-sync.md +++ b/context/cli/agent-trace-auto-sync.md @@ -24,11 +24,17 @@ start the current `sce` executable with exactly: sync --format json ``` -The child runs with the repository root as its working directory and null -stdin, stdout, and stderr. `Command::spawn()` is used without waiting for a -status; the hook returns its normal successful result immediately. A failure to -resolve the current executable or spawn the child is ignored, so launcher -failures cannot turn a successful post-commit operation into a failure. +The child runs with the repository root as its working directory, null stdin and +stdout, and inherited stderr. An internal `SCE_INTERNAL_AUTO_SYNC=1` process +marker lets the child classify this invocation as automatic without adding a +user-facing option or configuration layer. `Command::spawn()` is used without +waiting for a status; the hook returns its normal successful result immediately. +If the child sync fails, its single typed `SCE-ERR-RUNTIME` diagnostic is visible +through inherited stderr. If the current executable cannot be resolved or the +child cannot be spawned, the launcher emits the same typed automatic-sync +diagnostic with the startup reason on stderr. Both startup and child failures +remain fail-open, so they cannot turn a successful post-commit operation into a +failure. Automatic synchronization is not invoked by `pre-commit`, `diff-trace`, or `conversation-trace`. It is one post-commit launch, not a high-frequency hook, diff --git a/context/cli/agent-trace-sync-command.md b/context/cli/agent-trace-sync-command.md index 1d22b547b..f1dea4b51 100644 --- a/context/cli/agent-trace-sync-command.md +++ b/context/cli/agent-trace-sync-command.md @@ -45,6 +45,7 @@ Because every invocation starts from the control plane's authoritative `/state` - **`401` (unexpected):** the control-plane client refreshes the WorkOS token exactly once, saves it, and retries the request exactly once. Concurrent callers that observed the same rejected token coalesce onto the first refresh and reuse its saved token; a second `401` (`ControlPlaneError::MissingCredentials`/`AuthenticationFailed`) fails the command with `sce auth login` guidance, and there is no further retry. - **Typed authentication classification:** `ControlPlaneError::is_authentication_failure()` is `true` only for `MissingCredentials`/`AuthenticationFailed`, and the same typed traversal is exposed as `StreamSyncError::is_authentication_failure()` and `TraceSyncError::is_authentication_failure()` — no sync-stream path erases a `ControlPlaneError` into a bare `String` before it reaches the command boundary. `cli/src/services/sync/command.rs`'s `classify_sync_error` calls this traversal (never string/substring matching). Manual invocations route authentication failures from the initial `/state` call, a stream batch request, or a stream reconciliation `/state` refresh to `CliError::User { error: UserError::NotAuthenticated, .. }`; every other failure stays internal with its full technical chain preserved. Automatic invocations use the payload-bearing `UserError::AutomaticSyncFailed` entry, with a typed authentication, control-plane, stream, or runtime failure kind and the display reason preserved separately from the technical source. See [CLI error-code taxonomy](../sce/cli-error-code-taxonomy.md) for the `CliError`/`UserError` architecture and [sync-command.md](sync-command.md#error-classification) for the command-level classifier. +- **Automatic invocation boundary:** Automatic execution is selected only by the detached launcher's internal `SCE_INTERNAL_AUTO_SYNC=1` process marker; manual `sce sync` remains mode-neutral. The detached child inherits stderr, so its one app-rendered runtime diagnostic is visible without exposing JSON stdout or making the post-commit hook wait. Launcher startup failures use the same typed automatic-sync payload with a preserved startup reason and remain fail-open to the hook. - **`409` (cursor conflict):** the per-stream sync engine reconciles by refetching `/state`, replacing only the affected stream's cursor, and resuming from local rows after the refreshed cursor — already-accepted rows are never resent. - **Ambiguous batch failure (`5xx`, transport failure, or an undecodable `2xx` body):** the engine reconciles via `/state` before any resend. If the refreshed cursor advanced (the batch was actually committed), sync continues from it without resending. If the cursor is unchanged (the batch was not committed), sync may resend once from the authoritative cursor. - **Reconciliation bound:** both the `409` and ambiguous-failure reconciliation paths share one bounded attempt counter per stream; exhausting it fails that stream with a "did not converge" error instead of looping unboundedly. diff --git a/context/cli/sync-command.md b/context/cli/sync-command.md index 7c880889f..f9ae5c9c7 100644 --- a/context/cli/sync-command.md +++ b/context/cli/sync-command.md @@ -14,11 +14,14 @@ command request carries an internal `SyncInvocation` context so manual and automatic executions can retain distinct error semantics without adding a public CLI option. The same boundary owns a best-effort one-shot launcher used by the post-commit hook when `agent_trace.auto_sync` is enabled: it resolves the current `sce` -executable, starts `sync --format json` in the repository root with null standard -streams, and does not wait for the child; executable and spawn failures are -ignored. The launcher is not a daemon or retry queue; local rows remain available -for a later manual or automatic invocation through the control-plane cursor -authority. +executable, starts `sync --format json` in the repository root with null stdin and +stdout plus inherited stderr, and does not wait for the child. It passes the +internal `SCE_INTERNAL_AUTO_SYNC=1` marker so automatic failures use the typed +automatic-sync diagnostic path. Child failures are visible through inherited +stderr, while executable and spawn failures emit the same typed runtime +diagnostic with their startup reason and remain fail-open. The launcher is not a +daemon or retry queue; local rows remain available for a later manual or +automatic invocation through the control-plane cursor authority. Sync orchestration owns its `SyncProgressEvent` lifecycle, batch, and stream-completion payloads and publishes them through the consumer-typed, library-independent `services::sync::progress::ProgressReporter` contract. diff --git a/context/context-map.md b/context/context-map.md index b19856228..aeee2f654 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -17,7 +17,7 @@ Feature/domain context: - `context/cli/patch-service.md` (standalone patch domain model, parser, JSON load helpers, and set operations in `cli/src/services/patch.rs` for in-memory parsed unified-diff representation, capturing only touched lines plus minimal per-file/per-hunk metadata, supporting both `Index:` SVN-style and `diff --git` git-style formats, with `ParseError` for actionable malformed-input diagnostics, `PatchLoadError`/`load_patch_from_json`/`load_patch_from_json_bytes` for storage-agnostic JSON reconstruction, `intersect_patches` for target-shaped overlap with exact-match-first and historical `kind`+`content` fallback semantics plus matched-constructed-line `session_id` and matched-constructed-hunk `model_id` provenance inheritance, and `combine_patches` for ordered patch combination with later-wins conflict resolution plus winning-hunk `model_id` provenance inheritance; repository structured-row reconstruction supplies persisted hunk-model and canonical touched-line-session provenance before these operations; `parse_patch`, `intersect_patches`, and `combine_patches` are consumed by the active post-commit hook runtime) - `context/cli/structured-patch-service.md` (Claude structured editor-hook derivation in `cli/src/services/structured_patch.rs`, including `Write` structured-update hunks, `Write` `tool_input.content` create fallback, `Edit` structured patches, deterministic skip reasons, `ParsedPatch` output semantics, Rust golden fixture coverage, and repository read-time enrichment that assigns persisted row `model_id` to each hunk and canonical row `session_id` to each touched line) - `context/cli/styling-service.md` (CLI text-mode output styling with `owo-colors`, TTY/`NO_COLOR` policy, shared helper API for human-facing surfaces including sync completion markers, and per-column right-to-left RGB gradient banner rendering) -- `context/cli/agent-trace-auto-sync.md` (default-enabled post-commit Agent Trace synchronization with explicit-false opt-out plus doctor readiness reporting: the existing `sce sync` command launched once through the current executable after local persistence, detached null-standard-stream behavior, fail-open startup, canonical managed-block proof of hook readiness, stable text/JSON enabled/disabled/not-ready/not-applicable states, no daemon/queue/high-frequency trigger, and retryability through manual sync and control-plane cursor authority) +- `context/cli/agent-trace-auto-sync.md` (default-enabled post-commit Agent Trace synchronization with explicit-false opt-out plus doctor readiness reporting: the existing `sce sync` command launched once through the current executable after local persistence, detached null stdin/stdout with inherited stderr, an internal automatic-invocation marker, typed child and launcher-failure diagnostics, fail-open startup, canonical managed-block proof of hook readiness, stable text/JSON enabled/disabled/not-ready/not-applicable states, no daemon/queue/high-frequency trigger, and retryability through manual sync and control-plane cursor authority) - `context/cli/sync-command.md` (the top-level `sce sync` command: repository-scoped Agent Trace storage resolution, WorkOS-authenticated four-stream control-plane synchronization 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, 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`, default-enabled `agent_trace.auto_sync` boolean resolution with explicit-false opt-out 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) diff --git a/context/glossary.md b/context/glossary.md index 50d006176..9d105b9a7 100644 --- a/context/glossary.md +++ b/context/glossary.md @@ -74,6 +74,7 @@ - `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). No CLI dev-dependencies are currently declared, and the baseline is validated through normal compile/test coverage. - `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 four concurrent Agent Trace streams; `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. The same sync request carries internal `SyncInvocation` context for manual versus automatic execution, with automatic failures represented by the closed `AutomaticSyncFailureKind` catalog and its reviewed recovery guidance. +- `agent_trace.auto_sync`: Config-file-only boolean resolved by the shared config layers for the post-commit Agent Trace synchronization boundary; omitted values are `true` and explicit `false` opts out, and `sce config show` reports its winning source. Enabled post-commit runs launch `sync --format json` once through the current executable with repository-root working directory, null stdin/stdout, inherited stderr, and an internal `SCE_INTERNAL_AUTO_SYNC=1` marker; launcher and child failures remain fail-open and are visible as typed automatic-sync diagnostics. See [Agent Trace hook routing](sce/agent-trace-hooks-command-routing.md). - `local Turso adapter`: Module in `cli/src/services/local_db/mod.rs` that defines `LocalDbSpec` and exposes `LocalDb` as a `TursoDb` alias. It resolves the canonical local DB path with `local_db_path()`, currently declares zero migrations, and inherits retry-backed `new()`, `execute()`, `query()`, and `query_map()` behavior from the shared generic adapter. - `encrypted Turso adapter`: Generic adapter seam in `cli/src/services/db/mod.rs` exposed as `EncryptedTursoDb`, structurally parallel to `TursoDb` (connection, tokio runtime bridge, spec typing). Its constructor resolves the encryption key via `encryption_key::get_or_create_encryption_key(&db_path, db_name)`, which derives a Turso-compatible 64-character hex key from non-empty `SCE_AUTH_DB_ENCRYPTION_KEY` env-secret text before falling back to OS credential-store keyring get-or-create behavior; credential-store default registration is guarded by stable `OnceLock` plus an atomic in-progress flag so errors or panics leave initialization retryable without mutex poisoning. The adapter enables Turso local encryption with strict `aegis256` cipher selection through `turso::EncryptionOpts`, wraps encrypted local open/connect in the default DB connection-open retry policy, and runs embedded migrations after retry has produced a connection; the adapter also exposes retry-backed synchronous `execute`, `query`, `query_map`, and `run_migrations` helpers with `__sce_migrations` tracking parity. - `auth DB adapter`: Module in `cli/src/services/auth_db/mod.rs` that defines `AuthDbSpec` and exposes `AuthDb` as an `EncryptedTursoDb` alias. It resolves the canonical `/sce/auth.db` path with `auth_db_path()`, keeps encryption mandatory with `SCE_AUTH_DB_ENCRYPTION_KEY` env-secret precedence before OS keyring fallback and no plaintext mode, and embeds ordered auth migrations where baseline SQL creates `auth_credentials` without `user_id`, with `updated_at`, and a trigger that auto-refreshes `updated_at` on row updates. Auth runtime token-storage is now wired through `cli/src/services/token_storage.rs`, which persists tokens via the `auth_credentials` table in the encrypted auth DB instead of a JSON file. @@ -247,4 +248,3 @@ - `parts table (Agent Trace DB)`: Agent Trace DB table created by migration `009_create_parts.sql`; stores append-only message parts with columns `type` (typed by Rust as `text`/`reasoning`/`patch`/`question` and stored as unconstrained `TEXT NOT NULL`), `text`, `message_id`, `session_id`, `generated_at_unix_ms`, `created_at`, `updated_at`. Uses only the internal `id` for row identity (no upsert/dedup). Multiple parts can exist for the same `(session_id, message_id)`. A compound index on `(session_id, message_id, generated_at_unix_ms, id)` enables ordered joins. No foreign keys to `messages` or any other table, so parts may be inserted before their parent message exists. - `AgentTraceExportReader`: Read-only incremental export reader in `cli/src/services/agent_trace_export/mod.rs` over one `RepositoryAgentTraceDb`, exposing `read_messages_after`/`read_parts_after`/`read_diff_traces_after`/`read_agent_traces_after`, each `(cursor: i64, limit: usize) -> Result>` over `WHERE id > cursor ORDER BY id ASC LIMIT {limit}`. Holds no local cursor, performs no mutation, makes no network calls, and returns owned camelCase `serde::Serialize` export-row DTOs matching the shipped control-plane ingestion contract. See `context/sce/agent-trace-export-readers.md`. - `context synchronization lifecycle`: Durable task-level state for synchronization after successful `/next-task` execution. The task record is `pending`, `synced`, or `blocked`; blocked records carry a blocker, required action, and retry condition. Missing lifecycle state on a completed task is unresolved debt, not evidence of synchronization. `/validate` does not persist a plan-level synchronization lifecycle. See `context/sce/shared-context-code-workflow.md`. -- `agent_trace.auto_sync`: Config-file-only boolean resolved by the shared config layers for the post-commit Agent Trace synchronization boundary; omitted values are `true` and explicit `false` opts out, and `sce config show` reports its winning source. After validation and repository-DB persistence, enabled post-commit runs launch the existing sync-owned `sync --format json` command once through the current executable with detached null-standard-stream child semantics and repository-root working directory; the hook never waits, has no daemon or high-frequency trigger, and treats launcher failures as fail-open. See [Agent Trace hook routing](sce/agent-trace-hooks-command-routing.md). diff --git a/context/overview.md b/context/overview.md index c6379660f..763d354f3 100644 --- a/context/overview.md +++ b/context/overview.md @@ -10,7 +10,7 @@ The generated `/next-task` workflow persists task-level context-synchronization - **Exit codes:** `2` parse, `3` validation, `4` runtime, `5` dependency failure (see `context/sce/cli-exit-code-contract.md`). - **Stderr diagnostics:** stable `SCE-ERR-{PARSE,VALIDATION,RUNTIME,DEPENDENCY}` codes with class-default `Try:` remediation (see `context/sce/cli-error-code-taxonomy.md`). -- **Stdout/stderr:** command payloads on stdout only; redacted diagnostics and text-mode `sce sync` progress on stderr, while JSON sync remains silent (see `context/sce/cli-stdout-stderr-contract.md`). +- **Stdout/stderr:** command payloads on stdout only; redacted diagnostics and text-mode `sce sync` progress on stderr, while automatic JSON sync keeps stdout silent and inherits stderr for typed failures (see `context/sce/cli-stdout-stderr-contract.md`). - **Observability:** config-resolved logging with tracing, explicit config-file/default `log_to_file` control, error-specific stderr suppression when file logging is enabled, and optional dated/session-partitioned `log_dir` / `SCE_LOG_DIR` files with retention (see `context/sce/cli-observability-contract.md`). - **Config precedence:** `flags > env > config file > defaults` (see `context/cli/config-precedence-contract.md`); invalid default-discovered config remains degradable for ordinary startup, while setup and Agent Trace storage fail closed before side effects or fallback identity selection. The config-file-only `agent_trace.auto_sync` setting defaults to `true` and is resolved with source metadata for the post-commit trigger and doctor readiness boundaries. Its asynchronous post-commit behavior and doctor capability reporting are documented in `context/cli/agent-trace-auto-sync.md`. - **Attribution hooks:** enabled by default, gated by staged-diff AI-overlap preflight; `SCE_ATTRIBUTION_HOOKS_DISABLED` opt-out (see `context/sce/agent-trace-commit-msg-coauthor-policy.md`). diff --git a/context/patterns.md b/context/patterns.md index 9ad112fd0..ab0bec792 100644 --- a/context/patterns.md +++ b/context/patterns.md @@ -128,7 +128,7 @@ - Keep `log_file_retention_limit` flat and config-file/default only: validate it as an integer with minimum `1`, merge global before local, default it to `10`, expose resolved source metadata without adding an environment variable or CLI flag, and pass the resolved value unchanged to primary and v2 creation-triggered logger cleanup. - For runtime CLI configuration, keep precedence deterministic and explicit (`flags > env > config file > defaults`) and expose inspect/validate command entrypoints with stable text/JSON outputs. Config-file-only Agent Trace runtime switches such as `agent_trace.auto_sync` should document their default and explicit opt-out, resolve global before local, and expose winning source metadata without adding an environment variable or CLI flag unless the contract explicitly requires one. -- For default-enabled automatic Agent Trace synchronization, keep the post-commit launcher as a one-shot reuse of `sce sync`: start the current executable after local Agent Trace persistence, use the repository root and null child streams, do not wait, and fail open on launcher errors. Keep manual sync and the control-plane cursor authority as the retry path; do not add daemons, watchers, polling, queues, or high-frequency hook triggers. +- For default-enabled automatic Agent Trace synchronization, keep the post-commit launcher as a one-shot reuse of `sce sync`: start the current executable after local Agent Trace persistence, use the repository root, null stdin/stdout, inherited stderr, and an internal automatic-invocation marker, do not wait, and fail open on launcher errors. Render child and launcher failures through the typed automatic-sync runtime diagnostic with actionable manual `sce sync` recovery guidance. Keep manual sync and the control-plane cursor authority as the retry path; do not add daemons, watchers, polling, queues, or high-frequency hook triggers. - For commands that support text/JSON dual output, centralize `--format ` parsing in one shared contract and pass command-specific `--help` guidance into invalid-value errors instead of duplicating parser logic per command. - For setup-style command contracts, keep interactive mode as the zero-flag default and enforce mutually-exclusive explicit target flags for non-interactive automation. - For setup config safety, validate an existing repo-local config immediately after resolving the Git root and before prompts, context bootstrap, lifecycle providers, hooks, or target assets; preserve create-if-missing behavior for absent config and keep general startup degradation scoped away from setup and Agent Trace storage identity resolution. diff --git a/context/plans/auto-sync-failure-guidance.md b/context/plans/auto-sync-failure-guidance.md index 40a1a65e3..5244d0adc 100644 --- a/context/plans/auto-sync-failure-guidance.md +++ b/context/plans/auto-sync-failure-guidance.md @@ -104,13 +104,26 @@ Persist this field in every plan; this is durable plan state, not chat state: - Context impact: root — changed the typed CLI error contract, sync command invocation classification, and app-level diagnostic rendering; durable context synchronization is required before another task starts. - Context synchronization: synced -- [ ] T02: `Surface detached automatic-sync failures without blocking post-commit` (status:todo) +- [x] T02: `Surface detached automatic-sync failures without blocking post-commit` (status:complete) - Task ID: T02 - Scope: In — `cli/src/services/sync/auto_sync.rs`, post-commit launcher seam in `cli/src/services/hooks/mod.rs`, internal child invocation marker/stdio configuration, structured launcher-failure reporting using the same typed automatic-sync error payload, and focused launcher/hook tests. Out — sync protocol behavior, waiting for child completion, retry queues, and high-frequency hook triggers. - Dependencies: T01 - Done when: the detached child keeps the exact `sync --format json` command and repository-root/no-wait behavior, identifies automatic mode, exposes only typed failure diagnostics through stderr, and launcher executable/spawn errors retain actionable reasons through structured auto-sync reporting while remaining fail-open to the successful hook result. - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auto_sync`; `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml hooks::`. - - Context synchronization: pending + - Completed: 2026-08-27 + - Files changed: + - `cli/src/services/app_support.rs` + - `cli/src/services/error.rs` + - `cli/src/services/parse/command_runtime.rs` + - `cli/src/services/sync/auto_sync.rs` + - `cli/src/services/sync/command.rs` + - `cli/src/services/sync/mod.rs` + - Result: Preserved detached `sync --format json` execution while passing an internal automatic-invocation marker, inheriting child stderr for typed failure diagnostics, and rendering structured fail-open launcher errors with actionable reasons. + - Verify: + - `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auto_sync` — passed (14 tests). + - `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml hooks::` — passed (163 tests). + - Context impact: root — changed the automatic sync process boundary, stderr visibility, invocation classification, and fail-open launcher diagnostic contract; durable context synchronization is required before another task starts. + - Context synchronization: synced - [ ] T03: `Document typed automatic-sync failure recovery contract` (status:todo) - Task ID: T03 diff --git a/context/sce/agent-trace-hooks-command-routing.md b/context/sce/agent-trace-hooks-command-routing.md index 8fe4d5365..1a05355cf 100644 --- a/context/sce/agent-trace-hooks-command-routing.md +++ b/context/sce/agent-trace-hooks-command-routing.md @@ -63,7 +63,7 @@ - Post-commit Agent Trace success requires both schema validation and Agent Trace DB `agent_traces` persistence to succeed. - Current command-surface success output is: `post-commit hook processed intersection: commit=, intersection_files=`. - After Agent Trace validation and `agent_traces` persistence succeed, post-commit runs exactly one passive WAL checkpoint through `RepositoryAgentTraceDb::passive_checkpoint()` (see [shared-turso-db.md](shared-turso-db.md)) before resolving auto-sync. This is routine maintenance, not a durability boundary: a checkpoint failure is logged as a warning via `Logger::warn` with event `sce.agent_trace_db.passive_checkpoint_failed` and does not fail the hook or affect already-persisted Agent Trace data. `diff-trace` and `conversation-trace` do not checkpoint per write; only this one post-commit call site does. -- After Agent Trace validation and `agent_traces` persistence succeed, post-commit resolves the config-file-only `agent_trace.auto_sync` gate. When it is `true`, the hook invokes the sync-owned one-shot launcher exactly once with the repository root; the launcher starts the current `sce` executable as detached `sync --format json` work and is not awaited. Explicit `false` configuration does not launch; omitted configuration launches, and validation or persistence failure reaches the existing error path before the gate. Launcher/current-executable/spawn failures are fail-open and do not change the successful post-commit result. No `pre-commit`, `diff-trace`, or `conversation-trace` path invokes automatic synchronization. + - After Agent Trace validation and `agent_traces` persistence succeed, post-commit resolves the config-file-only `agent_trace.auto_sync` gate. When it is `true`, the hook invokes the sync-owned one-shot launcher exactly once with the repository root; the launcher starts the current `sce` executable as detached `sync --format json` work, passes the internal `SCE_INTERNAL_AUTO_SYNC=1` marker, inherits stderr, and is not awaited. The child keeps stdin/stdout null, so successful JSON execution is silent, while automatic failures render one typed runtime diagnostic through inherited stderr. Explicit `false` configuration does not launch; omitted configuration launches, and validation or persistence failure reaches the existing error path before the gate. Launcher/current-executable/spawn failures emit typed automatic-sync runtime diagnostics with their reasons on stderr but remain fail-open and do not change the successful post-commit result. No `pre-commit`, `diff-trace`, or `conversation-trace` path invokes automatic synchronization. - `post-rewrite` is a deterministic no-op entrypoint. - `diff-trace` reads STDIN JSON and classifies the payload: - **Claude structured payloads** (detected by presence of top-level `hook_event_name`): the STDIN JSON is validated through `derive_claude_structured_patch`. Supported `PostToolUse` `Write` create and `Edit` structured-patch events produce a `DiffTracePayload` with `payload_type="structured"` and the raw event JSON stored as the `diff` column without conversion to unified-diff text. Model attribution is resolved event-locally and direct-first: top-level `model`, `model_id`, or `modelId`, or nested `model.id`, `model.model`, or `model.name`, wins when present. Otherwise, when the event provides both `transcript_path` and `tool_use_id`, Rust scans that Claude JSONL transcript for the assistant-message envelope whose `tool_use.id` matches, skipping malformed unrelated records. Either source is normalized once with the `claude/` prefix. Missing/unreadable transcripts, unmatched tool calls, missing models, or absent lookup fields leave `model_id` nullable without rejecting the hook, and downstream Agent Trace JSON omits contributor `model_id`. No session-level cache or lookup participates. Unsupported Claude events (non-`PostToolUse`, unsupported tools, invalid payloads) produce a deterministic `NoOp` success result. diff --git a/context/sce/cli-error-code-taxonomy.md b/context/sce/cli-error-code-taxonomy.md index f2f986557..1e0f410aa 100644 --- a/context/sce/cli-error-code-taxonomy.md +++ b/context/sce/cli-error-code-taxonomy.md @@ -35,6 +35,7 @@ It complements the numeric process exit-code classes documented in `context/sce/ - `CliError::{User,Internal}` in `cli/src/services/error.rs` is the typed CLI-boundary error type; `CliError::code()`/`CliError::class()` delegate to the failure class. `CliError::User` carries a closed catalog `UserError` (`NotAuthenticated`, `NotGitRepository`, `NotGitRemote { remote_name }`, or `AutomaticSyncFailed`) for expected, deliberately-explained failures; `CliError::Internal` carries a live `anyhow::Error` source for every other failure, including non-classifiable setup preflight failures. `CliError::User` may also carry an optional preserved technical `source`, kept for observability only and never rendered to the terminal; the named remote payload contains no URL. - `UserError` in `cli/src/services/error.rs` is the closed catalog of deliberately presented terminal failures. It has no arbitrary-message variant (no `Message(String)`/`Custom(...)` escape hatch): every entry returns a fixed reviewed sentence or reviewed payload-derived message from `UserError::message()`, keyed for structured logging by `UserError::key()`. - `AutomaticSyncFailureKind` distinguishes authentication, control-plane, stream, and local runtime automatic-sync failures. Automatic authentication renders login-plus-manual-sync guidance; the other kinds render their preserved reason with actionable manual `sce sync` recovery guidance. The technical `anyhow` source remains available to observability and is not rendered as a second diagnostic. +- The post-commit launcher reports executable-resolution and spawn failures as the local runtime kind through the same payload-bearing `UserError::AutomaticSyncFailed` path. Those startup diagnostics retain the launcher reason, use `SCE-ERR-RUNTIME`, and remain fail-open to the successful hook result. - Command and domain layers construct and return a `CliError`; they do not format terminal text, apply styling, or decide authentication/user-error semantics from string matching. `app_support` is the sole owner of turning a `CliError` into the final stderr sentence. - `Logger::log_cli_error` in `cli/src/services/observability.rs` owns structured error logging with `sce.error.{code}` event IDs. - `write_error_diagnostic` in `cli/src/services/app_support.rs` owns final code-bearing stderr rendering, including styling `CliError::User`'s catalog message and `CliError::Internal`'s rendered chain through `services::style::error_text_with_color_policy` under the stderr TTY/`NO_COLOR` policy (`services::style::supports_color_stderr()`), independent of stdout's TTY state. diff --git a/context/sce/cli-stdout-stderr-contract.md b/context/sce/cli-stdout-stderr-contract.md index bef1adea8..b725b9dfa 100644 --- a/context/sce/cli-stdout-stderr-contract.md +++ b/context/sce/cli-stdout-stderr-contract.md @@ -11,6 +11,7 @@ This document defines the implemented stream contract for CLI command payload an - Failure diagnostics are emitted as `Error []: ...` on `stderr`, where `` is the stable class-based `SCE-ERR-*` identifier from `CliError` in `cli/src/services/error.rs`; diagnostics are passed through shared redaction (`services::security::redact_sensitive_text`) before emission. - The diagnostic body differs by `CliError` variant: `CliError::Internal` renders the real `anyhow` source chain (`format!("{source:#}")`) plus class-default `Try:` remediation; `CliError::User` renders its catalog `UserError` template, with non-authentication automatic-sync payload reasons included only in that reviewed message and no technical source chain or `Try:` suffix. Both bodies are styled through the same stderr TTY/`NO_COLOR` policy before redaction and emission. - Command handlers now return payload strings to the app dispatcher; the app owns stream selection and final emission. +- The detached post-commit `sce sync --format json` child keeps stdout null so a successful automatic run produces no hook payload, while inheriting stderr so its single app-rendered typed failure diagnostic remains observable. Launcher executable/spawn failures are rendered through the same stderr diagnostic writer in the parent and remain fail-open. ## Implementation surface From d75703dfc66cd64e91ddbf18fd0bd69b9cd12d74 Mon Sep 17 00:00:00 2001 From: Ivan Ivic Date: Thu, 27 Aug 2026 17:12:34 +0200 Subject: [PATCH 3/5] runtime: Finalize automatic sync failure diagnostics Automatic post-commit synchronization needs one stable, actionable runtime diagnostic while retaining technical causes for observability. Preserve owned user-error message strings during app rendering and document the typed failure guidance, stderr behavior, manual retry path, and unchanged fail-open boundaries. Co-authored-by: SCE --- cli/src/services/sync/auto_sync.rs | 4 +- context/architecture.md | 2 +- context/cli/agent-trace-auto-sync.md | 14 ++++ context/cli/agent-trace-sync-command.md | 1 + context/cli/sync-command.md | 10 +-- context/context-map.md | 2 +- context/glossary.md | 4 +- context/overview.md | 2 +- context/patterns.md | 2 +- context/plans/auto-sync-failure-guidance.md | 69 ++++++++++++++++--- .../sce/agent-trace-hooks-command-routing.md | 2 +- context/sce/cli-error-code-taxonomy.md | 1 + context/sce/cli-stdout-stderr-contract.md | 1 + 13 files changed, 93 insertions(+), 21 deletions(-) diff --git a/cli/src/services/sync/auto_sync.rs b/cli/src/services/sync/auto_sync.rs index daf35fca1..975afa449 100644 --- a/cli/src/services/sync/auto_sync.rs +++ b/cli/src/services/sync/auto_sync.rs @@ -129,8 +129,8 @@ mod tests { use std::rc::Rc; use super::{ - launch_with, launcher_failure_diagnostic, AutoSyncCommand, StdioMode, - AUTOMATIC_SYNC_INVOCATION_ENV, AUTOMATIC_SYNC_INVOCATION_VALUE, SYNC_ARGS, + launch_with, AutoSyncCommand, StdioMode, AUTOMATIC_SYNC_INVOCATION_ENV, + AUTOMATIC_SYNC_INVOCATION_VALUE, SYNC_ARGS, }; #[test] diff --git a/context/architecture.md b/context/architecture.md index 06d935045..96d6465b3 100644 --- a/context/architecture.md +++ b/context/architecture.md @@ -105,7 +105,7 @@ 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`, 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 typed `CliError` boundary includes payload-bearing automatic-sync user errors whose app-owned rendering preserves manual retry guidance without appending the default runtime remediation; the detached post-commit child inherits stderr for that diagnostic while keeping JSON stdout silent. +- `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`, 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 typed `CliError` boundary includes payload-bearing automatic-sync user errors whose app-owned rendering uses the `Automatic synchronization failed:` prefix and preserves mode-specific recovery guidance without appending default runtime remediation; the detached post-commit child inherits stderr for that diagnostic while keeping JSON stdout silent. - 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. - 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, 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. diff --git a/context/cli/agent-trace-auto-sync.md b/context/cli/agent-trace-auto-sync.md index 865e4c33b..e21691927 100644 --- a/context/cli/agent-trace-auto-sync.md +++ b/context/cli/agent-trace-auto-sync.md @@ -41,6 +41,20 @@ Automatic synchronization is not invoked by `pre-commit`, `diff-trace`, or watcher, polling loop, scheduler, daemon, retry queue, persistent service, or second synchronization database. +### Failure diagnostics + +The automatic child classifies terminal sync failures into the closed +`AutomaticSyncFailureKind` set: `Authentication`, `ControlPlane`, `Stream`, or +`Runtime`. The app renders exactly one `Error [SCE-ERR-RUNTIME]` diagnostic +whose message begins `Automatic synchronization failed:`. Authentication uses +the reviewed `sce auth login`, then manual `sce sync` recovery instruction and +keeps the technical reason for observability; non-authentication failures +include their preserved display reason and actionable recovery guidance before +the manual `sce sync` retry. Automatic user-error rendering does not append the +generic runtime `Try:` sentence or render the technical source as a second +diagnostic. Launcher executable-resolution and spawn failures use the same +`Runtime` payload and preserve their startup reason. + ## Doctor readiness `sce doctor` reports the capability without invoking it. The post-commit hook's diff --git a/context/cli/agent-trace-sync-command.md b/context/cli/agent-trace-sync-command.md index f1dea4b51..380dd03ed 100644 --- a/context/cli/agent-trace-sync-command.md +++ b/context/cli/agent-trace-sync-command.md @@ -46,6 +46,7 @@ Because every invocation starts from the control plane's authoritative `/state` - **`401` (unexpected):** the control-plane client refreshes the WorkOS token exactly once, saves it, and retries the request exactly once. Concurrent callers that observed the same rejected token coalesce onto the first refresh and reuse its saved token; a second `401` (`ControlPlaneError::MissingCredentials`/`AuthenticationFailed`) fails the command with `sce auth login` guidance, and there is no further retry. - **Typed authentication classification:** `ControlPlaneError::is_authentication_failure()` is `true` only for `MissingCredentials`/`AuthenticationFailed`, and the same typed traversal is exposed as `StreamSyncError::is_authentication_failure()` and `TraceSyncError::is_authentication_failure()` — no sync-stream path erases a `ControlPlaneError` into a bare `String` before it reaches the command boundary. `cli/src/services/sync/command.rs`'s `classify_sync_error` calls this traversal (never string/substring matching). Manual invocations route authentication failures from the initial `/state` call, a stream batch request, or a stream reconciliation `/state` refresh to `CliError::User { error: UserError::NotAuthenticated, .. }`; every other failure stays internal with its full technical chain preserved. Automatic invocations use the payload-bearing `UserError::AutomaticSyncFailed` entry, with a typed authentication, control-plane, stream, or runtime failure kind and the display reason preserved separately from the technical source. See [CLI error-code taxonomy](../sce/cli-error-code-taxonomy.md) for the `CliError`/`UserError` architecture and [sync-command.md](sync-command.md#error-classification) for the command-level classifier. - **Automatic invocation boundary:** Automatic execution is selected only by the detached launcher's internal `SCE_INTERNAL_AUTO_SYNC=1` process marker; manual `sce sync` remains mode-neutral. The detached child inherits stderr, so its one app-rendered runtime diagnostic is visible without exposing JSON stdout or making the post-commit hook wait. Launcher startup failures use the same typed automatic-sync payload with a preserved startup reason and remain fail-open to the hook. +- **Automatic failure rendering:** Automatic terminal failures use the closed `AutomaticSyncFailureKind` catalog (`Authentication`, `ControlPlane`, `Stream`, or `Runtime`) and render one `SCE-ERR-RUNTIME` message beginning `Automatic synchronization failed:`. Authentication renders login-plus-manual-sync guidance while retaining its technical reason only for observability; non-authentication failures render their preserved reason and actionable manual `sce sync` recovery without a duplicate generic `Try:` sentence. - **`409` (cursor conflict):** the per-stream sync engine reconciles by refetching `/state`, replacing only the affected stream's cursor, and resuming from local rows after the refreshed cursor — already-accepted rows are never resent. - **Ambiguous batch failure (`5xx`, transport failure, or an undecodable `2xx` body):** the engine reconciles via `/state` before any resend. If the refreshed cursor advanced (the batch was actually committed), sync continues from it without resending. If the cursor is unchanged (the batch was not committed), sync may resend once from the authoritative cursor. - **Reconciliation bound:** both the `409` and ambiguous-failure reconciliation paths share one bounded attempt counter per stream; exhausting it fails that stream with a "did not converge" error instead of looping unboundedly. diff --git a/context/cli/sync-command.md b/context/cli/sync-command.md index f9ae5c9c7..7e2129602 100644 --- a/context/cli/sync-command.md +++ b/context/cli/sync-command.md @@ -117,10 +117,12 @@ control-plane, stream, and local runtime failures as the payload-bearing `UserError::AutomaticSyncFailed` catalog entry, preserving the typed failure kind and display reason while retaining the technical source through `CliError::user_with_source`. `app_support` renders one runtime diagnostic for -the automatic case: authentication tells the user to run `sce auth login` and -then manually retry with `sce sync`; other failures include the reason and -actionable recovery guidance including manual `sce sync`, without adding the -default runtime `Try:` sentence. See [CLI error-code +the automatic case beginning `Automatic synchronization failed:`: +authentication tells the user to run `sce auth login` and then manually retry +with `sce sync` (the technical authentication reason remains observability-only); +other failures include the preserved reason and actionable recovery guidance +including manual `sce sync`, without adding the default runtime `Try:` sentence. +See [CLI error-code taxonomy](../sce/cli-error-code-taxonomy.md) for the full `CliError`/`UserError` architecture. diff --git a/context/context-map.md b/context/context-map.md index aeee2f654..fac662780 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -17,7 +17,7 @@ Feature/domain context: - `context/cli/patch-service.md` (standalone patch domain model, parser, JSON load helpers, and set operations in `cli/src/services/patch.rs` for in-memory parsed unified-diff representation, capturing only touched lines plus minimal per-file/per-hunk metadata, supporting both `Index:` SVN-style and `diff --git` git-style formats, with `ParseError` for actionable malformed-input diagnostics, `PatchLoadError`/`load_patch_from_json`/`load_patch_from_json_bytes` for storage-agnostic JSON reconstruction, `intersect_patches` for target-shaped overlap with exact-match-first and historical `kind`+`content` fallback semantics plus matched-constructed-line `session_id` and matched-constructed-hunk `model_id` provenance inheritance, and `combine_patches` for ordered patch combination with later-wins conflict resolution plus winning-hunk `model_id` provenance inheritance; repository structured-row reconstruction supplies persisted hunk-model and canonical touched-line-session provenance before these operations; `parse_patch`, `intersect_patches`, and `combine_patches` are consumed by the active post-commit hook runtime) - `context/cli/structured-patch-service.md` (Claude structured editor-hook derivation in `cli/src/services/structured_patch.rs`, including `Write` structured-update hunks, `Write` `tool_input.content` create fallback, `Edit` structured patches, deterministic skip reasons, `ParsedPatch` output semantics, Rust golden fixture coverage, and repository read-time enrichment that assigns persisted row `model_id` to each hunk and canonical row `session_id` to each touched line) - `context/cli/styling-service.md` (CLI text-mode output styling with `owo-colors`, TTY/`NO_COLOR` policy, shared helper API for human-facing surfaces including sync completion markers, and per-column right-to-left RGB gradient banner rendering) -- `context/cli/agent-trace-auto-sync.md` (default-enabled post-commit Agent Trace synchronization with explicit-false opt-out plus doctor readiness reporting: the existing `sce sync` command launched once through the current executable after local persistence, detached null stdin/stdout with inherited stderr, an internal automatic-invocation marker, typed child and launcher-failure diagnostics, fail-open startup, canonical managed-block proof of hook readiness, stable text/JSON enabled/disabled/not-ready/not-applicable states, no daemon/queue/high-frequency trigger, and retryability through manual sync and control-plane cursor authority) +- `context/cli/agent-trace-auto-sync.md` (default-enabled post-commit Agent Trace synchronization with explicit-false opt-out plus doctor readiness reporting: the existing `sce sync` command launched once through the current executable after local persistence, detached null stdin/stdout with inherited stderr, an internal automatic-invocation marker, typed `Authentication`/`ControlPlane`/`Stream`/`Runtime` child and launcher-failure diagnostics beginning `Automatic synchronization failed:`, fail-open startup, canonical managed-block proof of hook readiness, stable text/JSON enabled/disabled/not-ready/not-applicable states, no daemon/queue/high-frequency trigger, and retryability through manual sync and control-plane cursor authority) - `context/cli/sync-command.md` (the top-level `sce sync` command: repository-scoped Agent Trace storage resolution, WorkOS-authenticated four-stream control-plane synchronization 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, 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`, default-enabled `agent_trace.auto_sync` boolean resolution with explicit-false opt-out 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) diff --git a/context/glossary.md b/context/glossary.md index 9d105b9a7..70ef131d0 100644 --- a/context/glossary.md +++ b/context/glossary.md @@ -73,8 +73,8 @@ - `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, and returns deterministic actionable errors for 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). No CLI dev-dependencies are currently declared, and the baseline is validated through normal compile/test coverage. -- `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 four concurrent Agent Trace streams; `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. The same sync request carries internal `SyncInvocation` context for manual versus automatic execution, with automatic failures represented by the closed `AutomaticSyncFailureKind` catalog and its reviewed recovery guidance. -- `agent_trace.auto_sync`: Config-file-only boolean resolved by the shared config layers for the post-commit Agent Trace synchronization boundary; omitted values are `true` and explicit `false` opts out, and `sce config show` reports its winning source. Enabled post-commit runs launch `sync --format json` once through the current executable with repository-root working directory, null stdin/stdout, inherited stderr, and an internal `SCE_INTERNAL_AUTO_SYNC=1` marker; launcher and child failures remain fail-open and are visible as typed automatic-sync diagnostics. See [Agent Trace hook routing](sce/agent-trace-hooks-command-routing.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 four concurrent Agent Trace streams; `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. The same sync request carries internal `SyncInvocation` context for manual versus automatic execution, with automatic failures represented by the closed `AutomaticSyncFailureKind` catalog (`Authentication`, `ControlPlane`, `Stream`, `Runtime`) and its reviewed `Automatic synchronization failed:` recovery guidance. +- `agent_trace.auto_sync`: Config-file-only boolean resolved by the shared config layers for the post-commit Agent Trace synchronization boundary; omitted values are `true` and explicit `false` opts out, and `sce config show` reports its winning source. Enabled post-commit runs launch `sync --format json` once through the current executable with repository-root working directory, null stdin/stdout, inherited stderr, and an internal `SCE_INTERNAL_AUTO_SYNC=1` marker; launcher and child failures remain fail-open and are visible as one typed `SCE-ERR-RUNTIME` diagnostic beginning `Automatic synchronization failed:`. Authentication directs the user through `sce auth login` and manual `sce sync`, while other kinds include their preserved reason and manual retry guidance. See [Agent Trace hook routing](sce/agent-trace-hooks-command-routing.md). - `local Turso adapter`: Module in `cli/src/services/local_db/mod.rs` that defines `LocalDbSpec` and exposes `LocalDb` as a `TursoDb` alias. It resolves the canonical local DB path with `local_db_path()`, currently declares zero migrations, and inherits retry-backed `new()`, `execute()`, `query()`, and `query_map()` behavior from the shared generic adapter. - `encrypted Turso adapter`: Generic adapter seam in `cli/src/services/db/mod.rs` exposed as `EncryptedTursoDb`, structurally parallel to `TursoDb` (connection, tokio runtime bridge, spec typing). Its constructor resolves the encryption key via `encryption_key::get_or_create_encryption_key(&db_path, db_name)`, which derives a Turso-compatible 64-character hex key from non-empty `SCE_AUTH_DB_ENCRYPTION_KEY` env-secret text before falling back to OS credential-store keyring get-or-create behavior; credential-store default registration is guarded by stable `OnceLock` plus an atomic in-progress flag so errors or panics leave initialization retryable without mutex poisoning. The adapter enables Turso local encryption with strict `aegis256` cipher selection through `turso::EncryptionOpts`, wraps encrypted local open/connect in the default DB connection-open retry policy, and runs embedded migrations after retry has produced a connection; the adapter also exposes retry-backed synchronous `execute`, `query`, `query_map`, and `run_migrations` helpers with `__sce_migrations` tracking parity. - `auth DB adapter`: Module in `cli/src/services/auth_db/mod.rs` that defines `AuthDbSpec` and exposes `AuthDb` as an `EncryptedTursoDb` alias. It resolves the canonical `/sce/auth.db` path with `auth_db_path()`, keeps encryption mandatory with `SCE_AUTH_DB_ENCRYPTION_KEY` env-secret precedence before OS keyring fallback and no plaintext mode, and embeds ordered auth migrations where baseline SQL creates `auth_credentials` without `user_id`, with `updated_at`, and a trigger that auto-refreshes `updated_at` on row updates. Auth runtime token-storage is now wired through `cli/src/services/token_storage.rs`, which persists tokens via the `auth_credentials` table in the encrypted auth DB instead of a JSON file. diff --git a/context/overview.md b/context/overview.md index 763d354f3..14aaa8708 100644 --- a/context/overview.md +++ b/context/overview.md @@ -20,7 +20,7 @@ The CLI crate currently depends on `anyhow`, `chrono`, `clap`, `clap_complete`, Its command loop is implemented with `clap` derive-based argument parsing and `anyhow` error handling. Top-level help displays an ASCII art "SCE" banner with a per-column right-to-left color gradient (cyan to magenta when color is enabled, plain ASCII when disabled) above a slim command list without implemented/placeholder labels; `auth` is visible while `hooks` and `policy` remain directly invocable but hidden. The real top-level command catalog/help-visibility contract is centralized in `cli/src/cli_schema.rs` and consumed by `cli/src/command_surface.rs` for custom banner/help rendering plus known-command classification. The runtime includes implemented auth flows (`auth login|logout|whoami`), with authenticated whoami reading the Control Plane `GET /me` profile and rendering flat email/name/role/permissions/organization labels, optional names and missing values handled deterministically, and exact login guidance returned when logged out, alongside config inspection/validation, setup orchestration, doctor diagnosis/repair, attribution-only hooks, shell completion, and the top-level `sync` command. Parse-time command conversion plus run-time command handling flow through the internal `RuntimeCommand` seam in `cli/src/app.rs`. The current doctor presentation contract supersedes the earlier output-shape scaffolding wording above: human text uses the compact Environment/Repository/Integrations hierarchy with healthy rows collapsed and unhealthy branches expanded, while JSON retains complete path, identity, problem, and fix-result detail. See `context/sce/doctor-human-text-contract.md`. 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. -The same runtime also emits stable user-facing stderr error classes (`SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, `SCE-ERR-DEPENDENCY`) using deterministic `Error []: ...` diagnostics with class-default `Try:` remediation appended when missing. 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 `UserError` catalog (`NotAuthenticated`, `NotGitRepository`, payload-bearing `NotGitRemote { remote_name }`, and typed `AutomaticSyncFailed`) for expected, deliberately-explained failures rendered without a `Try:` suffix, while `CliError::Internal` carries a live `anyhow::Error` source rendered as the real error chain with class-default remediation; `app_support` is the sole owner of turning either into the final styled stderr diagnostic. Setup preflight errors preserve technical sources for observability, identify the configured missing remote by name, and keep raw remote URLs out of user-facing diagnostics. Sync keeps manual authentication semantics and also has an internal automatic invocation context whose `AutomaticSyncFailed` payload distinguishes authentication, control-plane, stream, and local runtime failures while preserving the technical source for observability. See `context/sce/cli-error-code-taxonomy.md` for the full contract. +The same runtime also emits stable user-facing stderr error classes (`SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, `SCE-ERR-DEPENDENCY`) using deterministic `Error []: ...` diagnostics with class-default `Try:` remediation appended when missing. 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 `UserError` catalog (`NotAuthenticated`, `NotGitRepository`, payload-bearing `NotGitRemote { remote_name }`, and typed `AutomaticSyncFailed`) for expected, deliberately-explained failures rendered as reviewed messages without a `Try:` suffix, while `CliError::Internal` carries a live `anyhow::Error` source rendered as the real error chain with class-default remediation; `app_support` is the sole owner of turning either into the final styled stderr diagnostic. Setup preflight errors preserve technical sources for observability, identify the configured missing remote by name, and keep raw remote URLs out of user-facing diagnostics. Sync keeps manual authentication semantics and also has an internal automatic invocation context whose `AutomaticSyncFailed` payload distinguishes authentication, control-plane, stream, and local runtime failures while preserving the technical source for observability. Automatic messages use the `Automatic synchronization failed:` prefix; authentication gives login-plus-manual-sync guidance, while other kinds include their reason and manual `sce sync` recovery. 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, error-specific stderr suppression while preserving stderr for non-error records and file-write diagnostics so stdout command payloads remain pipe-safe, and `observability::traits` boundaries for logger and telemetry behavior. 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. diff --git a/context/patterns.md b/context/patterns.md index ab0bec792..4485fbb48 100644 --- a/context/patterns.md +++ b/context/patterns.md @@ -128,7 +128,7 @@ - Keep `log_file_retention_limit` flat and config-file/default only: validate it as an integer with minimum `1`, merge global before local, default it to `10`, expose resolved source metadata without adding an environment variable or CLI flag, and pass the resolved value unchanged to primary and v2 creation-triggered logger cleanup. - For runtime CLI configuration, keep precedence deterministic and explicit (`flags > env > config file > defaults`) and expose inspect/validate command entrypoints with stable text/JSON outputs. Config-file-only Agent Trace runtime switches such as `agent_trace.auto_sync` should document their default and explicit opt-out, resolve global before local, and expose winning source metadata without adding an environment variable or CLI flag unless the contract explicitly requires one. -- For default-enabled automatic Agent Trace synchronization, keep the post-commit launcher as a one-shot reuse of `sce sync`: start the current executable after local Agent Trace persistence, use the repository root, null stdin/stdout, inherited stderr, and an internal automatic-invocation marker, do not wait, and fail open on launcher errors. Render child and launcher failures through the typed automatic-sync runtime diagnostic with actionable manual `sce sync` recovery guidance. Keep manual sync and the control-plane cursor authority as the retry path; do not add daemons, watchers, polling, queues, or high-frequency hook triggers. +- For default-enabled automatic Agent Trace synchronization, keep the post-commit launcher as a one-shot reuse of `sce sync`: start the current executable after local Agent Trace persistence, use the repository root, null stdin/stdout, inherited stderr, and an internal automatic-invocation marker, do not wait, and fail open on launcher errors. Render child and launcher failures through one typed `SCE-ERR-RUNTIME` diagnostic beginning `Automatic synchronization failed:`; authentication must direct the user to `sce auth login` and then manual `sce sync`, while other kinds include their preserved reason and actionable manual retry guidance without duplicating generic runtime `Try:` text. Keep manual sync and the control-plane cursor authority as the retry path; do not add daemons, watchers, polling, queues, or high-frequency hook triggers. - For commands that support text/JSON dual output, centralize `--format ` parsing in one shared contract and pass command-specific `--help` guidance into invalid-value errors instead of duplicating parser logic per command. - For setup-style command contracts, keep interactive mode as the zero-flag default and enforce mutually-exclusive explicit target flags for non-interactive automation. - For setup config safety, validate an existing repo-local config immediately after resolving the Git root and before prompts, context bootstrap, lifecycle providers, hooks, or target assets; preserve create-if-missing behavior for absent config and keep general startup degradation scoped away from setup and Agent Trace storage identity resolution. diff --git a/context/plans/auto-sync-failure-guidance.md b/context/plans/auto-sync-failure-guidance.md index 5244d0adc..b62922c4e 100644 --- a/context/plans/auto-sync-failure-guidance.md +++ b/context/plans/auto-sync-failure-guidance.md @@ -23,17 +23,17 @@ How this plan is proven complete. Each criterion is observable and names the check that proves it. `/validate` runs these checks; no task in the stack performs final validation. -- [ ] AC1: A sync failure raised by an automatic invocation maps to a payload-bearing typed `UserError`/`CliError` path and renders one runtime diagnostic that clearly says automatic synchronization failed and includes the underlying typed failure reason. +- [x] AC1: A sync failure raised by an automatic invocation maps to a payload-bearing typed `UserError`/`CliError` path and renders one runtime diagnostic that clearly says automatic synchronization failed and includes the underlying typed failure reason. - Validate: Focused sync/error tests assert the rendered diagnostic for control-plane, stream, and local runtime failures, including the reason and `SCE-ERR-RUNTIME` classification. -- [ ] AC2: An automatic authentication failure uses a distinct typed automatic-sync failure kind, tells the user that authentication is required, instructs them to run `sce auth login`, and then explicitly tells them to manually retry with `sce sync`. +- [x] AC2: An automatic authentication failure uses a distinct typed automatic-sync failure kind, tells the user that authentication is required, instructs them to run `sce auth login`, and then explicitly tells them to manually retry with `sce sync`. - Validate: Focused authentication classification and app-rendering tests assert the complete login-plus-manual-sync guidance and ensure the technical source remains available only for observability. -- [ ] AC3: Non-authentication automatic failures use the same typed payload-bearing error model, provide actionable recovery guidance, and explain that the user can manually retry with `sce sync`, without relying on substring matching or duplicating the default runtime `Try:` guidance. +- [x] AC3: Non-authentication automatic failures use the same typed payload-bearing error model, provide actionable recovery guidance, and explain that the user can manually retry with `sce sync`, without relying on substring matching or duplicating the default runtime `Try:` guidance. - Validate: Focused tests cover representative storage, transport/server, protocol, and stream failures and assert deterministic reason/recovery text with no duplicate remediation. -- [ ] AC4: Automatic child failures are visible through the existing stderr diagnostic channel while successful post-commit execution remains detached, non-blocking, JSON-stdout-silent, and fail-open to the commit; launcher startup failures retain their reason in structured auto-sync diagnostics without failing the hook. +- [x] AC4: Automatic child failures are visible through the existing stderr diagnostic channel while successful post-commit execution remains detached, non-blocking, JSON-stdout-silent, and fail-open to the commit; launcher startup failures retain their reason in structured auto-sync diagnostics without failing the hook. - Validate: Launcher and post-commit seam tests assert the internal automatic-invocation marker, inherited failure stderr, unchanged `sync --format json` arguments, no wait, and fail-open behavior for executable/spawn failures. -- [ ] AC5: Manual `sce sync` failures retain the existing manual-sync semantics and do not claim that automatic synchronization failed. +- [x] AC5: Manual `sce sync` failures retain the existing manual-sync semantics and do not claim that automatic synchronization failed. - Validate: Command-level tests execute/classify manual and automatic invocation modes separately and assert mode-specific rendering. -- [ ] AC6: Durable context documents the typed automatic-sync failure model, stderr visibility, authentication recovery, manual retry command, and preserved no-daemon/fail-open boundaries. +- [x] AC6: Durable context documents the typed automatic-sync failure model, stderr visibility, authentication recovery, manual retry command, and preserved no-daemon/fail-open boundaries. - Validate: Review the listed context contracts against the final code, then run the generated-context and repository checks under `Full validation`. ### Full validation @@ -125,13 +125,31 @@ Persist this field in every plan; this is durable plan state, not chat state: - Context impact: root — changed the automatic sync process boundary, stderr visibility, invocation classification, and fail-open launcher diagnostic contract; durable context synchronization is required before another task starts. - Context synchronization: synced -- [ ] T03: `Document typed automatic-sync failure recovery contract` (status:todo) +- [x] T03: `Document typed automatic-sync failure recovery contract` (status:complete) - Task ID: T03 - Scope: Update the auto-sync, sync, CLI error, stdout/stderr, hook-routing, and required root context contracts listed under Context sync to describe the final typed error and recovery behavior. Out — generated configuration artifacts, historical plans/decisions, and code/test changes. - Dependencies: T02 - Done when: durable context states the payload-bearing typed error model, automatic-failure prefix, reason preservation, authentication login flow, manual `sce sync` retry, stderr visibility, mode distinction, and unchanged detached/fail-open/no-daemon boundaries without stale null-output claims. - Verify: Manual code/context review against `cli/src/services/error.rs`, `cli/src/services/app_support.rs`, `cli/src/services/sync/command.rs`, `cli/src/services/sync/auto_sync.rs`, and `cli/src/services/hooks/mod.rs`. - - Context synchronization: pending + - Completed: 2026-08-27 + - Files changed: + - `context/architecture.md` + - `context/cli/agent-trace-auto-sync.md` + - `context/cli/agent-trace-sync-command.md` + - `context/cli/sync-command.md` + - `context/context-map.md` + - `context/glossary.md` + - `context/overview.md` + - `context/patterns.md` + - `context/sce/agent-trace-hooks-command-routing.md` + - `context/sce/cli-error-code-taxonomy.md` + - `context/sce/cli-stdout-stderr-contract.md` + - Result: Updated durable root and domain context to describe the closed typed automatic-sync failure catalog, the stable automatic-failure diagnostic prefix, authentication login-plus-manual-sync recovery, preserved non-authentication reasons, stderr visibility, manual-mode distinction, and unchanged detached/fail-open/no-daemon boundaries. + - Verify: + - `Manual code/context review against cli/src/services/error.rs, cli/src/services/app_support.rs, cli/src/services/sync/command.rs, cli/src/services/sync/auto_sync.rs, and cli/src/services/hooks/mod.rs` — passed. + - `git diff --check` — passed. + - Context impact: root — clarified the durable CLI error, stream, synchronization, hook-routing, and recovery contracts to match the implemented automatic-sync behavior. + - Context synchronization: synced ## Open questions @@ -141,3 +159,38 @@ persistent retry mechanism, and the setup preflight change establishes the payload-bearing `UserError` pattern to reuse; the remaining wording and internal marker choices are local implementation details covered by the existing CLI error/rendering patterns. + +## Validation Report + +**Status:** validated +**Date:** 2026-08-27 + +### Commands run + +- `nix flake check` -> exit 0 (all flake checks passed) +- `nix run .#pkl-check-generated` -> exit 0 (ephemeral Pkl generation passed: 141 files) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml error::` -> terminated by the 120-second tool timeout while concurrent Cargo invocations waited on locks (no exit code reported; rerun completed successfully) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml error::` -> exit 0 (6 focused error tests passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml sync::command` -> exit 0 (6 focused sync classification tests passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml app_support` -> exit 0 (5 focused app rendering tests passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml parse::command_runtime` -> exit 0 (5 focused command-runtime tests passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auto_sync` -> exit 0 (13 focused auto-sync and hook tests passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml hooks::` -> exit 0 (163 focused hook tests passed) +- `git diff --check` -> exit 0 (no whitespace errors) + +### Success-criteria verification + +- [x] AC1: A sync failure raised by an automatic invocation maps to a payload-bearing typed `UserError`/`CliError` path and renders one runtime diagnostic that clearly says automatic synchronization failed and includes the underlying typed failure reason. -> Error, app-rendering, and sync classification tests passed; typed runtime code and reason-preserving paths were inspected. +- [x] AC2: An automatic authentication failure uses a distinct typed automatic-sync failure kind, tells the user that authentication is required, instructs them to run `sce auth login`, and then explicitly tells them to manually retry with `sce sync`. -> Sync classification and app-rendering tests passed; authentication guidance and observability-only source handling were inspected. +- [x] AC3: Non-authentication automatic failures use the same typed payload-bearing error model, provide actionable recovery guidance, and explain that the user can manually retry with `sce sync`, without relying on substring matching or duplicating the default runtime `Try:` guidance. -> Sync, error, and app-rendering tests passed; representative typed control-plane, stream, runtime, storage, transport, server, and protocol paths were inspected. +- [x] AC4: Automatic child failures are visible through the existing stderr diagnostic channel while successful post-commit execution remains detached, non-blocking, JSON-stdout-silent, and fail-open to the commit; launcher startup failures retain their reason in structured auto-sync diagnostics without failing the hook. -> Auto-sync and hook seam tests passed; launcher arguments, marker, inherited stderr, null stdout, no-wait, and fail-open behavior were inspected. +- [x] AC5: Manual `sce sync` failures retain the existing manual-sync semantics and do not claim that automatic synchronization failed. -> Manual and automatic classification branches were inspected and focused sync tests passed. +- [x] AC6: Durable context documents the typed automatic-sync failure model, stderr visibility, authentication recovery, manual retry command, and preserved no-daemon/fail-open boundaries. -> Listed context contracts were reviewed against the final code; generated-context and repository checks passed. + +### Failed checks and follow-ups + +- None. + +### Residual risks + +- None identified. diff --git a/context/sce/agent-trace-hooks-command-routing.md b/context/sce/agent-trace-hooks-command-routing.md index 1a05355cf..747588542 100644 --- a/context/sce/agent-trace-hooks-command-routing.md +++ b/context/sce/agent-trace-hooks-command-routing.md @@ -63,7 +63,7 @@ - Post-commit Agent Trace success requires both schema validation and Agent Trace DB `agent_traces` persistence to succeed. - Current command-surface success output is: `post-commit hook processed intersection: commit=, intersection_files=`. - After Agent Trace validation and `agent_traces` persistence succeed, post-commit runs exactly one passive WAL checkpoint through `RepositoryAgentTraceDb::passive_checkpoint()` (see [shared-turso-db.md](shared-turso-db.md)) before resolving auto-sync. This is routine maintenance, not a durability boundary: a checkpoint failure is logged as a warning via `Logger::warn` with event `sce.agent_trace_db.passive_checkpoint_failed` and does not fail the hook or affect already-persisted Agent Trace data. `diff-trace` and `conversation-trace` do not checkpoint per write; only this one post-commit call site does. - - After Agent Trace validation and `agent_traces` persistence succeed, post-commit resolves the config-file-only `agent_trace.auto_sync` gate. When it is `true`, the hook invokes the sync-owned one-shot launcher exactly once with the repository root; the launcher starts the current `sce` executable as detached `sync --format json` work, passes the internal `SCE_INTERNAL_AUTO_SYNC=1` marker, inherits stderr, and is not awaited. The child keeps stdin/stdout null, so successful JSON execution is silent, while automatic failures render one typed runtime diagnostic through inherited stderr. Explicit `false` configuration does not launch; omitted configuration launches, and validation or persistence failure reaches the existing error path before the gate. Launcher/current-executable/spawn failures emit typed automatic-sync runtime diagnostics with their reasons on stderr but remain fail-open and do not change the successful post-commit result. No `pre-commit`, `diff-trace`, or `conversation-trace` path invokes automatic synchronization. + - After Agent Trace validation and `agent_traces` persistence succeed, post-commit resolves the config-file-only `agent_trace.auto_sync` gate. When it is `true`, the hook invokes the sync-owned one-shot launcher exactly once with the repository root; the launcher starts the current `sce` executable as detached `sync --format json` work, passes the internal `SCE_INTERNAL_AUTO_SYNC=1` marker, inherits stderr, and is not awaited. The child keeps stdin/stdout null, so successful JSON execution is silent, while automatic failures render one `Error [SCE-ERR-RUNTIME]` diagnostic beginning `Automatic synchronization failed:` through inherited stderr. Authentication tells the user to run `sce auth login` and then manually retry with `sce sync`; other typed failure kinds include their preserved reason and actionable manual retry guidance without a duplicate generic `Try:` sentence. Explicit `false` configuration does not launch; omitted configuration launches, and validation or persistence failure reaches the existing error path before the gate. Launcher/current-executable/spawn failures use the typed automatic-sync runtime payload with their reasons on stderr but remain fail-open and do not change the successful post-commit result. No `pre-commit`, `diff-trace`, or `conversation-trace` path invokes automatic synchronization. - `post-rewrite` is a deterministic no-op entrypoint. - `diff-trace` reads STDIN JSON and classifies the payload: - **Claude structured payloads** (detected by presence of top-level `hook_event_name`): the STDIN JSON is validated through `derive_claude_structured_patch`. Supported `PostToolUse` `Write` create and `Edit` structured-patch events produce a `DiffTracePayload` with `payload_type="structured"` and the raw event JSON stored as the `diff` column without conversion to unified-diff text. Model attribution is resolved event-locally and direct-first: top-level `model`, `model_id`, or `modelId`, or nested `model.id`, `model.model`, or `model.name`, wins when present. Otherwise, when the event provides both `transcript_path` and `tool_use_id`, Rust scans that Claude JSONL transcript for the assistant-message envelope whose `tool_use.id` matches, skipping malformed unrelated records. Either source is normalized once with the `claude/` prefix. Missing/unreadable transcripts, unmatched tool calls, missing models, or absent lookup fields leave `model_id` nullable without rejecting the hook, and downstream Agent Trace JSON omits contributor `model_id`. No session-level cache or lookup participates. Unsupported Claude events (non-`PostToolUse`, unsupported tools, invalid payloads) produce a deterministic `NoOp` success result. diff --git a/context/sce/cli-error-code-taxonomy.md b/context/sce/cli-error-code-taxonomy.md index 1e0f410aa..4bd399342 100644 --- a/context/sce/cli-error-code-taxonomy.md +++ b/context/sce/cli-error-code-taxonomy.md @@ -35,6 +35,7 @@ It complements the numeric process exit-code classes documented in `context/sce/ - `CliError::{User,Internal}` in `cli/src/services/error.rs` is the typed CLI-boundary error type; `CliError::code()`/`CliError::class()` delegate to the failure class. `CliError::User` carries a closed catalog `UserError` (`NotAuthenticated`, `NotGitRepository`, `NotGitRemote { remote_name }`, or `AutomaticSyncFailed`) for expected, deliberately-explained failures; `CliError::Internal` carries a live `anyhow::Error` source for every other failure, including non-classifiable setup preflight failures. `CliError::User` may also carry an optional preserved technical `source`, kept for observability only and never rendered to the terminal; the named remote payload contains no URL. - `UserError` in `cli/src/services/error.rs` is the closed catalog of deliberately presented terminal failures. It has no arbitrary-message variant (no `Message(String)`/`Custom(...)` escape hatch): every entry returns a fixed reviewed sentence or reviewed payload-derived message from `UserError::message()`, keyed for structured logging by `UserError::key()`. - `AutomaticSyncFailureKind` distinguishes authentication, control-plane, stream, and local runtime automatic-sync failures. Automatic authentication renders login-plus-manual-sync guidance; the other kinds render their preserved reason with actionable manual `sce sync` recovery guidance. The technical `anyhow` source remains available to observability and is not rendered as a second diagnostic. +- Automatic `UserError` messages begin with the stable `Automatic synchronization failed:` prefix. Authentication deliberately keeps its technical reason out of the user-facing sentence while preserving it in the typed payload/source for observability; the other failure kinds include the payload reason in that single rendered sentence. - The post-commit launcher reports executable-resolution and spawn failures as the local runtime kind through the same payload-bearing `UserError::AutomaticSyncFailed` path. Those startup diagnostics retain the launcher reason, use `SCE-ERR-RUNTIME`, and remain fail-open to the successful hook result. - Command and domain layers construct and return a `CliError`; they do not format terminal text, apply styling, or decide authentication/user-error semantics from string matching. `app_support` is the sole owner of turning a `CliError` into the final stderr sentence. - `Logger::log_cli_error` in `cli/src/services/observability.rs` owns structured error logging with `sce.error.{code}` event IDs. diff --git a/context/sce/cli-stdout-stderr-contract.md b/context/sce/cli-stdout-stderr-contract.md index b725b9dfa..104dc398a 100644 --- a/context/sce/cli-stdout-stderr-contract.md +++ b/context/sce/cli-stdout-stderr-contract.md @@ -12,6 +12,7 @@ This document defines the implemented stream contract for CLI command payload an - The diagnostic body differs by `CliError` variant: `CliError::Internal` renders the real `anyhow` source chain (`format!("{source:#}")`) plus class-default `Try:` remediation; `CliError::User` renders its catalog `UserError` template, with non-authentication automatic-sync payload reasons included only in that reviewed message and no technical source chain or `Try:` suffix. Both bodies are styled through the same stderr TTY/`NO_COLOR` policy before redaction and emission. - Command handlers now return payload strings to the app dispatcher; the app owns stream selection and final emission. - The detached post-commit `sce sync --format json` child keeps stdout null so a successful automatic run produces no hook payload, while inheriting stderr so its single app-rendered typed failure diagnostic remains observable. Launcher executable/spawn failures are rendered through the same stderr diagnostic writer in the parent and remain fail-open. +- Automatic child failures render exactly one `Error [SCE-ERR-RUNTIME]: Automatic synchronization failed: ...` diagnostic on inherited stderr. Authentication exposes login-plus-manual-sync recovery while its technical reason stays in observability; non-authentication reasons and recovery guidance are included in that one diagnostic, with no generic duplicate `Try:` suffix. ## Implementation surface From d4f3e1daaee17490fe64e6cf368dd5df61144388 Mon Sep 17 00:00:00 2001 From: Ivan Ivic Date: Mon, 31 Aug 2026 16:12:39 +0200 Subject: [PATCH 4/5] runtime: Wait for automatic sync child completion Make post-commit automatic Agent Trace synchronization wait for its one-shot child while preserving fail-open behavior, command boundaries, and diagnostic ownership. Document the synchronous completion boundary and intentional latency trade-off. Co-authored-by: SCE --- cli/src/services/sync/auto_sync.rs | 81 +++++++-- context/architecture.md | 6 +- context/cli/agent-trace-auto-sync.md | 25 +-- context/cli/agent-trace-sync-command.md | 2 +- context/cli/sync-command.md | 16 +- context/context-map.md | 3 +- ...1-synchronous-automatic-sync-completion.md | 79 +++++++++ context/glossary.md | 2 +- context/overview.md | 26 ++- context/patterns.md | 2 +- context/plans/auto-sync-failure-guidance.md | 163 ++++++++++++++---- .../sce/agent-trace-hooks-command-routing.md | 2 +- context/sce/cli-stdout-stderr-contract.md | 2 +- context/sce/doctor-human-text-contract.md | 6 +- 14 files changed, 330 insertions(+), 85 deletions(-) create mode 100644 context/decisions/2026-08-31-synchronous-automatic-sync-completion.md diff --git a/cli/src/services/sync/auto_sync.rs b/cli/src/services/sync/auto_sync.rs index 975afa449..5acd57598 100644 --- a/cli/src/services/sync/auto_sync.rs +++ b/cli/src/services/sync/auto_sync.rs @@ -2,7 +2,7 @@ use std::io; use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; +use std::process::{Child, Command, ExitStatus, Stdio}; use crate::services::app_support; use crate::services::error::{AutomaticSyncFailureKind, CliError, UserError}; @@ -48,6 +48,7 @@ impl AutoSyncCommand { enum AutoSyncLaunchError { CurrentExecutable(io::Error), Spawn(io::Error), + Wait(io::Error), } impl std::fmt::Display for AutoSyncLaunchError { @@ -56,7 +57,8 @@ impl std::fmt::Display for AutoSyncLaunchError { Self::CurrentExecutable(error) => { write!(formatter, "failed to resolve current executable: {error}") } - Self::Spawn(error) => write!(formatter, "failed to spawn detached sync: {error}"), + Self::Spawn(error) => write!(formatter, "failed to spawn automatic sync: {error}"), + Self::Wait(error) => write!(formatter, "failed to wait for automatic sync: {error}"), } } } @@ -64,11 +66,30 @@ impl std::fmt::Display for AutoSyncLaunchError { impl std::error::Error for AutoSyncLaunchError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { - Self::CurrentExecutable(error) | Self::Spawn(error) => Some(error), + Self::CurrentExecutable(error) | Self::Spawn(error) | Self::Wait(error) => Some(error), } } } +struct AutoSyncChild { + wait: Option io::Result>>, +} + +impl AutoSyncChild { + fn from_child(mut child: Child) -> Self { + Self { + wait: Some(Box::new(move || child.wait())), + } + } + + fn wait(mut self) -> io::Result { + (self + .wait + .take() + .expect("automatic sync child wait callback must be present"))() + } +} + fn launcher_failure_diagnostic(error: AutoSyncLaunchError) -> CliError { let reason = error.to_string(); CliError::user_with_source( @@ -80,9 +101,9 @@ fn launcher_failure_diagnostic(error: AutoSyncLaunchError) -> CliError { ) } -/// Launches the current executable to synchronize the repository in the -/// background. Launcher failures are reported on stderr but remain fail-open -/// to the post-commit caller. +/// Launches the current executable to synchronize the repository and waits for +/// the one-shot child to reach terminal completion. Launcher failures are +/// reported on stderr but remain fail-open to the post-commit caller. pub fn launch(repository_root: &Path) { if let Err(error) = launch_with(repository_root, std::env::current_exe, spawn_command) { let diagnostic = launcher_failure_diagnostic(error); @@ -98,14 +119,16 @@ fn launch_with( ) -> Result<(), AutoSyncLaunchError> where FCurrentExe: FnOnce() -> io::Result, - FSpawn: FnOnce(AutoSyncCommand) -> io::Result<()>, + FSpawn: FnOnce(AutoSyncCommand) -> io::Result, { let executable = current_exe().map_err(AutoSyncLaunchError::CurrentExecutable)?; - spawn(AutoSyncCommand::new(executable, repository_root)).map_err(AutoSyncLaunchError::Spawn) + let child = spawn(AutoSyncCommand::new(executable, repository_root)) + .map_err(AutoSyncLaunchError::Spawn)?; + child.wait().map(|_| ()).map_err(AutoSyncLaunchError::Wait) } -fn spawn_command(spec: AutoSyncCommand) -> io::Result<()> { +fn spawn_command(spec: AutoSyncCommand) -> io::Result { let mut command = Command::new(spec.executable); command .args(spec.args) @@ -115,10 +138,7 @@ fn spawn_command(spec: AutoSyncCommand) -> io::Result<()> { .stdout(Stdio::null()) .stderr(Stdio::inherit()); - // Dropping Child does not wait for it; the spawned sync continues - // independently of the post-commit caller. - let _child = command.spawn()?; - Ok(()) + Ok(AutoSyncChild::from_child(command.spawn()?)) } #[cfg(test)] @@ -129,12 +149,37 @@ mod tests { use std::rc::Rc; use super::{ - launch_with, AutoSyncCommand, StdioMode, AUTOMATIC_SYNC_INVOCATION_ENV, + launch_with, AutoSyncChild, AutoSyncCommand, StdioMode, AUTOMATIC_SYNC_INVOCATION_ENV, AUTOMATIC_SYNC_INVOCATION_VALUE, SYNC_ARGS, }; + fn child_with_wait(wait: F) -> AutoSyncChild + where + F: FnOnce() -> io::Result + 'static, + { + AutoSyncChild { + wait: Some(Box::new(wait)), + } + } + + fn exit_status(success: bool) -> std::process::ExitStatus { + #[cfg(unix)] + { + use std::os::unix::process::ExitStatusExt; + + std::process::ExitStatus::from_raw(i32::from(!success)) + } + + #[cfg(windows)] + { + use std::os::windows::process::ExitStatusExt; + + std::process::ExitStatus::from_raw(u32::from(!success)) + } + } + #[test] - fn launch_builds_the_expected_detached_command() { + fn launch_builds_the_expected_command() { let captured = Rc::new(RefCell::new(None)); let captured_by_spawn = Rc::clone(&captured); @@ -143,7 +188,7 @@ mod tests { || Ok(PathBuf::from("/usr/local/bin/sce")), move |command: AutoSyncCommand| { *captured_by_spawn.borrow_mut() = Some(command); - Ok(()) + Ok(child_with_wait(|| Ok(exit_status(true)))) }, ); @@ -173,9 +218,9 @@ mod tests { let launched = launch_with( Path::new("/repo/root"), || Err(io::Error::other("current executable unavailable")), - move |_| { + move |_| -> io::Result { *spawn_called_by_spawn.borrow_mut() = true; - Ok(()) + Ok(child_with_wait(|| Ok(exit_status(true)))) }, ); diff --git a/context/architecture.md b/context/architecture.md index 96d6465b3..62b16ba4f 100644 --- a/context/architecture.md +++ b/context/architecture.md @@ -105,7 +105,7 @@ 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`, 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 typed `CliError` boundary includes payload-bearing automatic-sync user errors whose app-owned rendering uses the `Automatic synchronization failed:` prefix and preserves mode-specific recovery guidance without appending default runtime remediation; the detached post-commit child inherits stderr for that diagnostic while keeping JSON stdout silent. +- `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`, 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 typed `CliError` boundary includes payload-bearing automatic-sync user errors whose app-owned rendering uses the `Automatic synchronization failed:` prefix and preserves mode-specific recovery guidance without appending default runtime remediation; the post-commit child inherits stderr for that diagnostic, waits at the launcher boundary, and keeps JSON stdout silent. Waiting makes child completion and inherited-stderr closure deterministic, while intentionally adding child synchronization latency to post-commit execution. - 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. - 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, 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. @@ -132,10 +132,10 @@ The repository includes a new placeholder Rust binary crate at `cli/`. - `cli/src/services/doctor/mod.rs` owns the current doctor request/report surface while focused submodules (`doctor/inspect.rs`, `doctor/render.rs`, `doctor/fixes.rs`, `doctor/types.rs`) split report fact collection, rendering, manual fix reporting, and doctor-owned domain types into smaller seams; `cli/src/services/doctor/command.rs` owns the `DoctorCommand` payload used by the static `RuntimeCommand` enum and executes against any context implementing repo-root scoping. Runtime doctor execution resolves a repository root, derives a scoped context, requests the shared static lifecycle provider catalog with hooks included for service-owned `diagnose` and `fix` behavior, adapts lifecycle-owned health/fix records into doctor-owned problem/fix records, and then renders stable text/JSON problem records with category/severity/fixability/remediation fields plus deterministic fix-result reporting in fix mode. Agent Trace database inspection is no longer a doctor-adjacent command surface; doctor owns repository-scoped DB health and checkout identity facts, while `sce sync` owns control-plane synchronization. Report fact collection preserves environment/repository/hook/integration display data and adds a non-launching `post_commit_auto_sync` fact based on canonical post-commit managed-block currency plus resolved `agent_trace.auto_sync` source/value; this fact does not launch `sce sync`, and existing hook problem/remediation/readiness semantics remain authoritative. Service-owned lifecycle providers own config validation, local DB and repository-scoped Agent Trace DB readiness/bootstrap, and hook rollout diagnosis/repair. Integration inspection in `doctor/inspect.rs` is scoped twice over: `resolve_doctor_integration_targets` picks which targets to inspect, and `persisted_optional_workflows` (reused from setup) resolves which optional workflows the repository selected, which the OpenCode/Claude/Pi/Codex child collectors apply through `iter_embedded_assets_for_setup_target_with_selection`. An unselected optional workflow therefore contributes no expected children at all, so no row and no missing/mismatch problem can be produced for it, while a selected one keeps the unchanged presence and content-hash checks. Codex's collector (`collect_codex_integration_groups`) is the one exception to the single-target-directory shape the other three share: since `CODEX_EMBEDDED_ASSETS` relative paths already carry their own `.agents/`/`.codex/` prefix, it resolves its integration root through `InstallTargetPaths::codex_target_dir()` (the repository root itself) rather than a `RepoPaths` per-target subdirectory, and splits assets into `Skills` (`.agents/skills/` prefix) and `Hooks` (`.codex/` prefix) groups instead of Pi's `Extensions`/`Prompts`/`Skills` split. Its `.codex/hooks.json` reporting is per-registration rather than one whole-file child: `codex_hook_config::diagnose_document` classifies each of the four required registrations as `PresentAndCurrent`/`Missing`/`Stale` (or the whole document as `Malformed` when it cannot be structurally validated) without writing anything, so unrelated user handlers never create a false whole-document mismatch. For a structurally current registration, `codex_hook_trust` separately reads (never writes) Codex's own durable `$CODEX_HOME/config.toml` hook-trust state — reproducing upstream's `hook_hash`/`hook_key`/`hook_trust_status` exactly — and reports `Trusted`/`Untrusted`/`Modified`/`Disabled`/`Unknown`; only `Trusted` renders healthy. `sce doctor --fix` repairs a structurally unhealthy `.codex/hooks.json` through the existing merge-install path, but a registration that is current yet not-yet-trusted is never "fixed", since SCE cannot grant Codex hook trust. - `cli/src/services/version/mod.rs` defines the version command parser/rendering contract (`parse_version_request`, `render_version`) with deterministic text output and stable JSON runtime-identification fields; `cli/src/services/version/command.rs` owns the `VersionCommand` payload used by the static `RuntimeCommand` enum. - `cli/src/services/completion/mod.rs` defines completion parser/rendering contract (`parse_completion_request`, `render_completion`) with deterministic Bash/Zsh/Fish script output aligned to current parser-valid command/flag surfaces; `cli/src/services/completion/command.rs` owns the `CompletionCommand` payload used by the static `RuntimeCommand` enum. -- `cli/src/services/hooks/mod.rs` defines the current local hook runtime parsing/dispatch (`HookSubcommand`, `run_hooks_subcommand`) plus a commit-msg co-author policy seam (`apply_commit_msg_coauthor_policy`) that injects one canonical SCE trailer only when the enabled-by-default attribution-hooks config/env control is not opted out, `SCE_DISABLED` is false, and the staged-diff AI-overlap preflight confirms AI/editor evidence (`StagedDiffAiOverlapResult::Overlap`); the preflight is wired into `run_commit_msg_subcommand_in_repo` and logs `sce.hooks.commit_msg.ai_overlap_error` on error paths; `cli/src/services/hooks/command.rs` owns the `HooksCommand` payload used by the static `RuntimeCommand` enum. In the current attribution-only baseline, `pre-commit` and `post-rewrite` are deterministic no-op surfaces; `post-commit` requires validated `--remote-url`, threads that URL through the Agent Trace flow, prints it to stderr, and remains an active intersection + Agent Trace persistence entrypoint (captures current commit patch, queries recent repository-level `diff_traces` from the bounded past-7-days window, combines valid patches via `patch::combine_patches`, intersects with post-commit patch via `patch::intersect_patches`, persists result to `post_commit_patch_intersections`, then persists built Agent Trace payloads with range-level `content_hash` values to `agent_traces` in the repository-scoped Agent Trace DB without post-commit file artifacts); after successful validation and persistence, the default-enabled config-file-only `agent_trace.auto_sync` gate launches one detached sync-owned `sync --format json` child unless explicitly disabled in config, with launcher failures ignored and no high-frequency hook trigger; `diff-trace` performs STDIN JSON intake, validates required non-empty `sessionID`/`diff`/`tool_name`, optional `model_id` (absent/`null` → `None`), required nullable/non-empty `tool_version` plus required `u64` `time` (Unix epoch milliseconds), rejects values that cannot fit signed `time_ms` storage, prefixes the stored `diff_traces.session_id` before insert construction (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi, same-tool idempotent), and inserts the parsed payload fields into `RepositoryAgentTraceDb` without creating a parsed-payload `context/tmp` artifact; Claude structured `PostToolUse` diff-trace intake resolves model attribution event-locally: direct top-level or nested metadata wins, otherwise the event's `transcript_path` is scanned for the assistant envelope whose `tool_use.id` matches `tool_use_id`; either source is normalized once with the `claude/` prefix and lookup failures remain nullable. `session-model` is no longer a supported hook route. +- `cli/src/services/hooks/mod.rs` defines the current local hook runtime parsing/dispatch (`HookSubcommand`, `run_hooks_subcommand`) plus a commit-msg co-author policy seam (`apply_commit_msg_coauthor_policy`) that injects one canonical SCE trailer only when the enabled-by-default attribution-hooks config/env control is not opted out, `SCE_DISABLED` is false, and the staged-diff AI-overlap preflight confirms AI/editor evidence (`StagedDiffAiOverlapResult::Overlap`); the preflight is wired into `run_commit_msg_subcommand_in_repo` and logs `sce.hooks.commit_msg.ai_overlap_error` on error paths; `cli/src/services/hooks/command.rs` owns the `HooksCommand` payload used by the static `RuntimeCommand` enum. In the current attribution-only baseline, `pre-commit` and `post-rewrite` are deterministic no-op surfaces; `post-commit` requires validated `--remote-url`, threads that URL through the Agent Trace flow, prints it to stderr, and remains an active intersection + Agent Trace persistence entrypoint (captures current commit patch, queries recent repository-level `diff_traces` from the bounded past-7-days window, combines valid patches via `patch::combine_patches`, intersects with post-commit patch via `patch::intersect_patches`, persists result to `post_commit_patch_intersections`, then persists built Agent Trace payloads with range-level `content_hash` values to `agent_traces` in the repository-scoped Agent Trace DB without post-commit file artifacts); after successful validation and persistence, the default-enabled config-file-only `agent_trace.auto_sync` gate launches one one-shot sync-owned `sync --format json` child unless explicitly disabled in config, waits for terminal completion, ignores non-zero child exit after child diagnostics, and keeps launcher failures fail-open without a high-frequency hook trigger; `diff-trace` performs STDIN JSON intake, validates required non-empty `sessionID`/`diff`/`tool_name`, optional `model_id` (absent/`null` → `None`), required nullable/non-empty `tool_version` plus required `u64` `time` (Unix epoch milliseconds), rejects values that cannot fit signed `time_ms` storage, prefixes the stored `diff_traces.session_id` before insert construction (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi, same-tool idempotent), and inserts the parsed payload fields into `RepositoryAgentTraceDb` without creating a parsed-payload `context/tmp` artifact; Claude structured `PostToolUse` diff-trace intake resolves model attribution event-locally: direct top-level or nested metadata wins, otherwise the event's `transcript_path` is scanned for the assistant envelope whose `tool_use.id` matches `tool_use_id`; either source is normalized once with the `claude/` prefix and lookup failures remain nullable. `session-model` is no longer a supported hook route. - Generated Claude settings no longer register `SessionStart` for Agent Trace model attribution, and `sce hooks session-model` is no longer a supported hook command. The `session_models` table/API and session-level fallback lookup were removed in T02 of the `remove-session-models-direct-claude-model-id` plan; `diff-trace` now uses direct-first/event-transcript-second Claude `model_id` resolution and direct `tool_version` values, without restoring session-level state. - `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. +- `context/cli/agent-trace-auto-sync.md` documents the sync-owned, one-shot post-commit launcher boundary: it reuses `sce sync`, waits for terminal child completion, makes inherited-stderr closure deterministic, intentionally adds child synchronization latency to post-commit execution, has no daemon or local retry machinery, and fails open when child startup, completion wait, or synchronization fails; 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 four-stream payload semantics, 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. - `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. diff --git a/context/cli/agent-trace-auto-sync.md b/context/cli/agent-trace-auto-sync.md index e21691927..319e84d9f 100644 --- a/context/cli/agent-trace-auto-sync.md +++ b/context/cli/agent-trace-auto-sync.md @@ -27,13 +27,17 @@ sync --format json The child runs with the repository root as its working directory, null stdin and stdout, and inherited stderr. An internal `SCE_INTERNAL_AUTO_SYNC=1` process marker lets the child classify this invocation as automatic without adding a -user-facing option or configuration layer. `Command::spawn()` is used without -waiting for a status; the hook returns its normal successful result immediately. -If the child sync fails, its single typed `SCE-ERR-RUNTIME` diagnostic is visible -through inherited stderr. If the current executable cannot be resolved or the -child cannot be spawned, the launcher emits the same typed automatic-sync -diagnostic with the startup reason on stderr. Both startup and child failures -remain fail-open, so they cannot turn a successful post-commit operation into a +user-facing option or configuration layer. The launcher waits for the child to +reach terminal completion before returning, making the child lifetime and +inherited-stderr closure deterministic. This intentionally adds the child +synchronization duration to post-commit latency; no timeout policy is part of +the boundary. A non-zero child exit remains +fail-open after the child has rendered its own single typed `SCE-ERR-RUNTIME` +diagnostic through inherited stderr; the launcher does not duplicate it. If the +current executable cannot be resolved, the child cannot be spawned, or waiting +fails, the launcher emits the same typed automatic-sync diagnostic with the +startup or wait reason on stderr. All launcher and child failures remain +fail-open, so they cannot turn a successful post-commit operation into a failure. Automatic synchronization is not invoked by `pre-commit`, `diff-trace`, or @@ -82,10 +86,11 @@ sce sync Automatic execution uses the same command and therefore the same repository Agent Trace database, control-plane protocol, authentication, and -control-plane cursor authority. A child startup, completion, or network failure -is fail-open to the commit. Rows that remain local are available to a later +control-plane cursor authority. A child startup, wait, or network failure is +fail-open to the commit. Rows that remain local are available to a later manual `sce sync` or a later successful automatic invocation; no local cursor or background retry machinery is required. See [the sync command contract](sync-command.md), [the config precedence -contract](config-precedence-contract.md), and [the hook routing contract](../sce/agent-trace-hooks-command-routing.md). +contract](config-precedence-contract.md), [the hook routing contract](../sce/agent-trace-hooks-command-routing.md), +and [the completion decision](../decisions/2026-08-31-synchronous-automatic-sync-completion.md). diff --git a/context/cli/agent-trace-sync-command.md b/context/cli/agent-trace-sync-command.md index 380dd03ed..3297dfb03 100644 --- a/context/cli/agent-trace-sync-command.md +++ b/context/cli/agent-trace-sync-command.md @@ -45,7 +45,7 @@ Because every invocation starts from the control plane's authoritative `/state` - **`401` (unexpected):** the control-plane client refreshes the WorkOS token exactly once, saves it, and retries the request exactly once. Concurrent callers that observed the same rejected token coalesce onto the first refresh and reuse its saved token; a second `401` (`ControlPlaneError::MissingCredentials`/`AuthenticationFailed`) fails the command with `sce auth login` guidance, and there is no further retry. - **Typed authentication classification:** `ControlPlaneError::is_authentication_failure()` is `true` only for `MissingCredentials`/`AuthenticationFailed`, and the same typed traversal is exposed as `StreamSyncError::is_authentication_failure()` and `TraceSyncError::is_authentication_failure()` — no sync-stream path erases a `ControlPlaneError` into a bare `String` before it reaches the command boundary. `cli/src/services/sync/command.rs`'s `classify_sync_error` calls this traversal (never string/substring matching). Manual invocations route authentication failures from the initial `/state` call, a stream batch request, or a stream reconciliation `/state` refresh to `CliError::User { error: UserError::NotAuthenticated, .. }`; every other failure stays internal with its full technical chain preserved. Automatic invocations use the payload-bearing `UserError::AutomaticSyncFailed` entry, with a typed authentication, control-plane, stream, or runtime failure kind and the display reason preserved separately from the technical source. See [CLI error-code taxonomy](../sce/cli-error-code-taxonomy.md) for the `CliError`/`UserError` architecture and [sync-command.md](sync-command.md#error-classification) for the command-level classifier. -- **Automatic invocation boundary:** Automatic execution is selected only by the detached launcher's internal `SCE_INTERNAL_AUTO_SYNC=1` process marker; manual `sce sync` remains mode-neutral. The detached child inherits stderr, so its one app-rendered runtime diagnostic is visible without exposing JSON stdout or making the post-commit hook wait. Launcher startup failures use the same typed automatic-sync payload with a preserved startup reason and remain fail-open to the hook. +- **Automatic invocation boundary:** Automatic execution is selected only by the one-shot launcher's internal `SCE_INTERNAL_AUTO_SYNC=1` process marker; manual `sce sync` remains mode-neutral. The child inherits stderr and the launcher waits for terminal completion, making child lifetime and stderr closure deterministic while adding the child synchronization duration to post-commit latency. Its one app-rendered runtime diagnostic is visible without exposing JSON stdout. Non-zero child exits, launcher startup failures, and wait failures remain fail-open to the hook; startup and wait failures use the same typed automatic-sync payload with their preserved reason. - **Automatic failure rendering:** Automatic terminal failures use the closed `AutomaticSyncFailureKind` catalog (`Authentication`, `ControlPlane`, `Stream`, or `Runtime`) and render one `SCE-ERR-RUNTIME` message beginning `Automatic synchronization failed:`. Authentication renders login-plus-manual-sync guidance while retaining its technical reason only for observability; non-authentication failures render their preserved reason and actionable manual `sce sync` recovery without a duplicate generic `Try:` sentence. - **`409` (cursor conflict):** the per-stream sync engine reconciles by refetching `/state`, replacing only the affected stream's cursor, and resuming from local rows after the refreshed cursor — already-accepted rows are never resent. - **Ambiguous batch failure (`5xx`, transport failure, or an undecodable `2xx` body):** the engine reconciles via `/state` before any resend. If the refreshed cursor advanced (the batch was actually committed), sync continues from it without resending. If the cursor is unchanged (the batch was not committed), sync may resend once from the authoritative cursor. diff --git a/context/cli/sync-command.md b/context/cli/sync-command.md index 7e2129602..884d06e74 100644 --- a/context/cli/sync-command.md +++ b/context/cli/sync-command.md @@ -15,13 +15,15 @@ automatic executions can retain distinct error semantics without adding a public CLI option. The same boundary owns a best-effort one-shot launcher used by the post-commit hook when `agent_trace.auto_sync` is enabled: it resolves the current `sce` executable, starts `sync --format json` in the repository root with null stdin and -stdout plus inherited stderr, and does not wait for the child. It passes the -internal `SCE_INTERNAL_AUTO_SYNC=1` marker so automatic failures use the typed -automatic-sync diagnostic path. Child failures are visible through inherited -stderr, while executable and spawn failures emit the same typed runtime -diagnostic with their startup reason and remain fail-open. The launcher is not a -daemon or retry queue; local rows remain available for a later manual or -automatic invocation through the control-plane cursor authority. +stdout plus inherited stderr, and waits for terminal child completion, making the +child lifetime and inherited-stderr closure deterministic at the cost of adding +child synchronization latency to post-commit execution. It passes the internal +`SCE_INTERNAL_AUTO_SYNC=1` marker so automatic failures use the +typed automatic-sync diagnostic path. A non-zero child exit remains fail-open +after the child renders its own diagnostic; executable, spawn, and wait failures +emit the same typed runtime diagnostic with their reason and remain fail-open. +The launcher is not a daemon or retry queue; local rows remain available for a +later manual or automatic invocation through the control-plane cursor authority. Sync orchestration owns its `SyncProgressEvent` lifecycle, batch, and stream-completion payloads and publishes them through the consumer-typed, library-independent `services::sync::progress::ProgressReporter` contract. diff --git a/context/context-map.md b/context/context-map.md index fac662780..a9e325449 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -17,7 +17,7 @@ Feature/domain context: - `context/cli/patch-service.md` (standalone patch domain model, parser, JSON load helpers, and set operations in `cli/src/services/patch.rs` for in-memory parsed unified-diff representation, capturing only touched lines plus minimal per-file/per-hunk metadata, supporting both `Index:` SVN-style and `diff --git` git-style formats, with `ParseError` for actionable malformed-input diagnostics, `PatchLoadError`/`load_patch_from_json`/`load_patch_from_json_bytes` for storage-agnostic JSON reconstruction, `intersect_patches` for target-shaped overlap with exact-match-first and historical `kind`+`content` fallback semantics plus matched-constructed-line `session_id` and matched-constructed-hunk `model_id` provenance inheritance, and `combine_patches` for ordered patch combination with later-wins conflict resolution plus winning-hunk `model_id` provenance inheritance; repository structured-row reconstruction supplies persisted hunk-model and canonical touched-line-session provenance before these operations; `parse_patch`, `intersect_patches`, and `combine_patches` are consumed by the active post-commit hook runtime) - `context/cli/structured-patch-service.md` (Claude structured editor-hook derivation in `cli/src/services/structured_patch.rs`, including `Write` structured-update hunks, `Write` `tool_input.content` create fallback, `Edit` structured patches, deterministic skip reasons, `ParsedPatch` output semantics, Rust golden fixture coverage, and repository read-time enrichment that assigns persisted row `model_id` to each hunk and canonical row `session_id` to each touched line) - `context/cli/styling-service.md` (CLI text-mode output styling with `owo-colors`, TTY/`NO_COLOR` policy, shared helper API for human-facing surfaces including sync completion markers, and per-column right-to-left RGB gradient banner rendering) -- `context/cli/agent-trace-auto-sync.md` (default-enabled post-commit Agent Trace synchronization with explicit-false opt-out plus doctor readiness reporting: the existing `sce sync` command launched once through the current executable after local persistence, detached null stdin/stdout with inherited stderr, an internal automatic-invocation marker, typed `Authentication`/`ControlPlane`/`Stream`/`Runtime` child and launcher-failure diagnostics beginning `Automatic synchronization failed:`, fail-open startup, canonical managed-block proof of hook readiness, stable text/JSON enabled/disabled/not-ready/not-applicable states, no daemon/queue/high-frequency trigger, and retryability through manual sync and control-plane cursor authority) +- `context/cli/agent-trace-auto-sync.md` (default-enabled post-commit Agent Trace synchronization with explicit-false opt-out plus doctor readiness reporting: the existing `sce sync` command launched once through the current executable after local persistence, waited to terminal child completion with null stdin/stdout and inherited stderr, an internal automatic-invocation marker, typed `Authentication`/`ControlPlane`/`Stream`/`Runtime` child and launcher-failure diagnostics beginning `Automatic synchronization failed:`, fail-open child/startup/wait handling, canonical managed-block proof of hook readiness, stable text/JSON enabled/disabled/not-ready/not-applicable states, no daemon/queue/high-frequency trigger, and retryability through manual sync and control-plane cursor authority) - `context/cli/sync-command.md` (the top-level `sce sync` command: repository-scoped Agent Trace storage resolution, WorkOS-authenticated four-stream control-plane synchronization 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, 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`, default-enabled `agent_trace.auto_sync` boolean resolution with explicit-false opt-out 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) @@ -98,6 +98,7 @@ Supporting repo docs: Recent decision records: +- `context/decisions/2026-08-31-synchronous-automatic-sync-completion.md` (accepted synchronous terminal-completion boundary for the one-shot automatic post-commit sync launcher, preserving fail-open child/startup/wait handling, inherited stderr, null stdout, and no-daemon/no-retry constraints) - `context/decisions/2026-08-23-codex-canonical-worktree-path-resolution.md` (accepts upstream-compatible Codex apply_patch parent/absolute paths only when canonical resolution remains inside the Git worktree, validates nearest existing prefixes for missing targets, and rejects symlink escapes) - `context/decisions/2026-08-23-codex-nondestructive-hook-ownership.md` (uses one shared structural ownership predicate and merge service for setup/doctor: Codex SCE handlers require the generated helper path plus the `sce hooks codex` contract, while unrelated hook configuration survives) - `context/decisions/2026-08-23-codex-event-scoped-apply-patch-evidence-identities.md` (uses bounded, deterministic `tool_use_id`-derived synthetic line identities for Codex apply_patch evidence; positions are evidence identities rather than source line numbers, with existing patch combination/intersection semantics unchanged) diff --git a/context/decisions/2026-08-31-synchronous-automatic-sync-completion.md b/context/decisions/2026-08-31-synchronous-automatic-sync-completion.md new file mode 100644 index 000000000..0e31ea730 --- /dev/null +++ b/context/decisions/2026-08-31-synchronous-automatic-sync-completion.md @@ -0,0 +1,79 @@ +# Decision: Wait for automatic sync completion at the post-commit launcher boundary + +Date: 2026-08-31 +Status: Accepted +Plan: `context/plans/auto-sync-failure-guidance.md` +Task: `T06` + +## Context + +The post-commit hook launches the existing one-shot `sce sync --format json` +command after local Agent Trace persistence. The launcher must preserve the +repository-root working directory, the internal automatic-invocation marker, +null stdin/stdout, inherited stderr, and the hook's fail-open boundary. The +selected completion policy also needs deterministic child termination and stream +closure, while avoiding a daemon, retry queue, timeout policy, or persistent +failure state. + +T06 implements and verifies the selected policy with focused launcher tests for +completion, non-zero child exit, wait failure, command preservation, and startup +failure. The plan's prior completion-policy evidence records that waiting was +selected despite its measured commit-latency cost. + +## Decision + +The automatic post-commit launcher waits for the spawned `sce sync --format +json` child to reach terminal completion before returning. + +## Rationale + +Waiting gives the post-commit boundary a deterministic completion point and +ensures the inherited diagnostic stream is closed before the launcher returns. +The child remains responsible for rendering synchronization failures, so the +parent does not duplicate non-zero child diagnostics. Startup and wait errors +are still rendered through the existing typed launcher diagnostic and remain +fail-open to the successful hook result. + +## Alternatives considered + +- **Detached, non-waiting launch** — rejected because it leaves child completion + and stream closure nondeterministic at the post-commit boundary. +- **Waiting with a new timeout or retry mechanism** — rejected because it would + add policy and persistent or repeated execution behavior outside this change. + +## Compatibility and risks + +- Automatic post-commit execution can add the child and network runtime to commit + latency; the hook still returns success for non-zero child exits and launcher + wait failures. +- The command arguments, working directory, marker, stdio routing, manual sync + behavior, and child-rendered diagnostic ownership remain compatible. + +## Guardrails + +- Launch exactly one `sync --format json` child through the current executable. +- Keep stdin and stdout null, stderr inherited, and the repository root as cwd. +- Do not add timeouts, retries, daemons, queues, schedulers, or persistent state. +- Ignore child exit status after successful wait; report only launcher startup or + wait errors through the typed fail-open diagnostic path. + +## Consequences + +- Automatic synchronization has a synchronous completion boundary at the + post-commit launcher while remaining fail-open to the commit. +- Successful automatic JSON sync remains stdout-silent and child failures remain + visible through inherited stderr. +- Commits may take longer when automatic synchronization is enabled. + +## Follow-up + +- Document the synchronous completion semantics and latency trade-off in the + current-state context contracts. + +## References + +- Plan: [`auto-sync-failure-guidance`](../plans/auto-sync-failure-guidance.md) +- Task: `T06` +- Current-state context: [`Automatic Agent Trace synchronization`](../cli/agent-trace-auto-sync.md) +- Evidence: [`automatic sync launcher`](../../cli/src/services/sync/auto_sync.rs) +- Related decision: None. diff --git a/context/glossary.md b/context/glossary.md index 70ef131d0..1fe827e27 100644 --- a/context/glossary.md +++ b/context/glossary.md @@ -74,7 +74,7 @@ - `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). No CLI dev-dependencies are currently declared, and the baseline is validated through normal compile/test coverage. - `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 four concurrent Agent Trace streams; `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. The same sync request carries internal `SyncInvocation` context for manual versus automatic execution, with automatic failures represented by the closed `AutomaticSyncFailureKind` catalog (`Authentication`, `ControlPlane`, `Stream`, `Runtime`) and its reviewed `Automatic synchronization failed:` recovery guidance. -- `agent_trace.auto_sync`: Config-file-only boolean resolved by the shared config layers for the post-commit Agent Trace synchronization boundary; omitted values are `true` and explicit `false` opts out, and `sce config show` reports its winning source. Enabled post-commit runs launch `sync --format json` once through the current executable with repository-root working directory, null stdin/stdout, inherited stderr, and an internal `SCE_INTERNAL_AUTO_SYNC=1` marker; launcher and child failures remain fail-open and are visible as one typed `SCE-ERR-RUNTIME` diagnostic beginning `Automatic synchronization failed:`. Authentication directs the user through `sce auth login` and manual `sce sync`, while other kinds include their preserved reason and manual retry guidance. See [Agent Trace hook routing](sce/agent-trace-hooks-command-routing.md). +- `agent_trace.auto_sync`: Config-file-only boolean resolved by the shared config layers for the post-commit Agent Trace synchronization boundary; omitted values are `true` and explicit `false` opts out, and `sce config show` reports its winning source. Enabled post-commit runs launch `sync --format json` once through the current executable with repository-root working directory, null stdin/stdout, inherited stderr, and an internal `SCE_INTERNAL_AUTO_SYNC=1` marker, then wait for terminal child completion so child lifetime and stderr closure are deterministic; this intentionally adds child synchronization latency to post-commit execution. Launcher and child failures remain fail-open and are visible through one typed `SCE-ERR-RUNTIME` diagnostic beginning `Automatic synchronization failed:`. Non-zero child exits do not cause a second parent diagnostic. Authentication directs the user through `sce auth login` and manual `sce sync`, while other kinds include their preserved reason and manual retry guidance. See [Agent Trace hook routing](sce/agent-trace-hooks-command-routing.md) and [the completion decision](decisions/2026-08-31-synchronous-automatic-sync-completion.md). - `local Turso adapter`: Module in `cli/src/services/local_db/mod.rs` that defines `LocalDbSpec` and exposes `LocalDb` as a `TursoDb` alias. It resolves the canonical local DB path with `local_db_path()`, currently declares zero migrations, and inherits retry-backed `new()`, `execute()`, `query()`, and `query_map()` behavior from the shared generic adapter. - `encrypted Turso adapter`: Generic adapter seam in `cli/src/services/db/mod.rs` exposed as `EncryptedTursoDb`, structurally parallel to `TursoDb` (connection, tokio runtime bridge, spec typing). Its constructor resolves the encryption key via `encryption_key::get_or_create_encryption_key(&db_path, db_name)`, which derives a Turso-compatible 64-character hex key from non-empty `SCE_AUTH_DB_ENCRYPTION_KEY` env-secret text before falling back to OS credential-store keyring get-or-create behavior; credential-store default registration is guarded by stable `OnceLock` plus an atomic in-progress flag so errors or panics leave initialization retryable without mutex poisoning. The adapter enables Turso local encryption with strict `aegis256` cipher selection through `turso::EncryptionOpts`, wraps encrypted local open/connect in the default DB connection-open retry policy, and runs embedded migrations after retry has produced a connection; the adapter also exposes retry-backed synchronous `execute`, `query`, `query_map`, and `run_migrations` helpers with `__sce_migrations` tracking parity. - `auth DB adapter`: Module in `cli/src/services/auth_db/mod.rs` that defines `AuthDbSpec` and exposes `AuthDb` as an `EncryptedTursoDb` alias. It resolves the canonical `/sce/auth.db` path with `auth_db_path()`, keeps encryption mandatory with `SCE_AUTH_DB_ENCRYPTION_KEY` env-secret precedence before OS keyring fallback and no plaintext mode, and embeds ordered auth migrations where baseline SQL creates `auth_credentials` without `user_id`, with `updated_at`, and a trigger that auto-refreshes `updated_at` on row updates. Auth runtime token-storage is now wired through `cli/src/services/token_storage.rs`, which persists tokens via the `auth_credentials` table in the encrypted auth DB instead of a JSON file. diff --git a/context/overview.md b/context/overview.md index 14aaa8708..01288d8d6 100644 --- a/context/overview.md +++ b/context/overview.md @@ -10,9 +10,13 @@ The generated `/next-task` workflow persists task-level context-synchronization - **Exit codes:** `2` parse, `3` validation, `4` runtime, `5` dependency failure (see `context/sce/cli-exit-code-contract.md`). - **Stderr diagnostics:** stable `SCE-ERR-{PARSE,VALIDATION,RUNTIME,DEPENDENCY}` codes with class-default `Try:` remediation (see `context/sce/cli-error-code-taxonomy.md`). -- **Stdout/stderr:** command payloads on stdout only; redacted diagnostics and text-mode `sce sync` progress on stderr, while automatic JSON sync keeps stdout silent and inherits stderr for typed failures (see `context/sce/cli-stdout-stderr-contract.md`). +- **Stdout/stderr:** command payloads on stdout only; redacted diagnostics and text-mode `sce sync` progress on stderr, while automatic JSON sync keeps stdout silent and inherits stderr for typed failures. The synchronous automatic boundary keeps child completion and stderr closure deterministic at the cost of adding the child duration to post-commit latency (see `context/sce/cli-stdout-stderr-contract.md`). - **Observability:** config-resolved logging with tracing, explicit config-file/default `log_to_file` control, error-specific stderr suppression when file logging is enabled, and optional dated/session-partitioned `log_dir` / `SCE_LOG_DIR` files with retention (see `context/sce/cli-observability-contract.md`). + +- **Config precedence:** `flags > env > config file > defaults` (see `context/cli/config-precedence-contract.md`); the config-file-only `agent_trace.auto_sync` setting defaults to `true` and is resolved with source metadata for the post-commit trigger and doctor readiness boundaries. Its synchronous one-shot post-commit completion behavior, deterministic stream closure, and intentional commit-latency trade-off are documented in `context/cli/agent-trace-auto-sync.md`. + - **Attribution hooks:** enabled by default, gated by staged-diff AI-overlap preflight; `SCE_ATTRIBUTION_HOOKS_DISABLED` opt-out (see `context/sce/agent-trace-commit-msg-coauthor-policy.md`). - **Install channels:** repo-flake Nix, Cargo, npm, and source-built Flatpak (`dev.crocoder.sce`); Homebrew deferred (see `context/sce/cli-first-install-channels-contract.md`). @@ -59,6 +63,24 @@ The checked-in Flatpak packaging surface lives under `packaging/flatpak/`with Ni The current supported automated release target matrix is `x86_64-unknown-linux-musl`, `aarch64-unknown-linux-musl`, and `aarch64-apple-darwin`; npm launcher platform support remains a separate current-state surface documented in the npm distribution contract and launcher code. - Native release binary portability auditing is exposed as `nix run .#native-portability-audit -- --binary [--platform auto|linux|macos]` plus the `native-portability-audit` flake check; it reports forbidden `/nix/store/` runtime references found by Linux ELF/string inspection or macOS `otool -L` install-name inspection. `release-artifacts` runs that audit against the staged `bin/sce` before tarball creation and, on macOS, rewrites Nix-store `libiconv.*.dylib` install names to `/usr/lib/...` with ad-hoc re-signing before the audit. The three native reusable release workflows also extract the generated archive, smoke-run `bin/sce version --format json`, and rerun the native portability audit before uploading native artifacts. + The downstream publish-stage implementation is now complete for both registries: `.github/workflows/publish-crates.yml` publishes the checked-in crate version after `.version`/tag/Cargo parity checks, and `.github/workflows/publish-npm.yml` publishes the checked-in npm package after `.version`/tag/npm parity checks plus verification of the canonical `sce-v-npm.tgz` GitHub release asset. The repository root now also owns the canonical Biome contract for the current JavaScript tooling slice: `biome.json` scopes formatting/linting to `npm/` and the shared `config/lib/` plugin package root while excluding package-local `node_modules/`, and the root Nix dev shell provides the `biome` binary so contributors do not need a host-installed formatter/linter for those areas. Flatpak validation/build orchestration is reduced to a minimal app surface: Linux flake apps expose the umbrella `sce-flatpak` (`nix run .#sce-flatpak -- ` for `validate`, `prepare-local-manifest`, etc.) plus `release-flatpak-package`, `release-flatpak-bundle`, and the `regenerate-flatpak-manifest` / `regenerate-cargo-sources` helpers; the previously separate `flatpak-validate`, `flatpak-local-manifest`, and `flatpak-build` wrapper apps are removed. Default `nix flake check` keeps the lightweight Nix-built static/AppStream validator plus the parity checks (`flatpak-manifest-parity`, `cargo-sources-parity`) and does not run a network-heavy Flatpak build. The former standalone install-channel integration runner and `install-channel-integration-tests` flake app are not active current-state surfaces. @@ -71,7 +93,7 @@ The current supported automated release target matrix is `x86_64-unknown-linux-m The hooks service now uses a minimal attribution-only runtime: `commit-msg` is the only hook that mutates behavior, conditionally injecting exactly one canonical SCE trailer when the attribution-hooks gate is enabled, `SCE_DISABLED` is false, and the staged-diff AI-overlap preflight confirms AI/editor evidence (`StagedDiffAiOverlapResult::Overlap`); when the preflight returns `NoOverlap` or `Error` (including DB open failure, schema not ready, query error, staged diff read failure, or zero overlap), the trailer is not appended and errors are logged via `sce.hooks.commit_msg.ai_overlap_error`; `pre-commit` and `post-rewrite` remain deterministic no-op entrypoints; `post-commit` requires validated `--remote-url`, threads that URL through the Agent Trace flow, prints it to stderr, captures current commit patch, queries recent `diff_traces` from past 7 days (dispatching `patch` rows through existing unified-diff parsing and `structured` rows through `structured_patch::derive_claude_structured_patch` at read time, then assigning each structured hunk the persisted row model and every structured touched line the persisted canonical `cc_...` row session), combines/intersects patches, persists intersection metadata to `post_commit_patch_intersections`, and persists the schema-validated built Agent Trace payload, including optional top-level `tool` metadata from recent diff-trace rows, top-level `metadata.sce.version` from the compiled `sce` CLI package version, always-emitted `metadata.sce.line_changes` touched-line attribution counts, and range-level `content_hash` values, to AgentTraceDb `agent_traces` (DB-only, no post-commit Agent Trace file artifact); `diff-trace` currently validates/persists required non-empty `sessionID`/`diff`/`tool_name`, optional `model_id` (absent or `null` → `None`, present+non-empty → `Some`, present+empty → error), required nullable/non-empty `tool_version`, plus required `u64` millisecond `time`, uses direct-first/event-transcript-second Claude `model_id` resolution plus direct `tool_version` without any `session_models` runtime, and continues with `None` when event-local lookup cannot resolve attribution, with same-tool-idempotent stored `session_id` prefixing (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi), non-lossy AgentTraceDb `time_ms` conversion, and no parsed-payload artifact fallback. Claude structured `PostToolUse` diff-trace intake prefers direct top-level or nested model metadata, then matches the event's `tool_use_id` in its JSONL `transcript_path`, normalizes either source once with the `claude/` prefix, and fails open to `None`. The CLI now also includes an approved operator-environment doctor contract documented in `context/sce/agent-trace-hook-doctor.md`; the runtime includes `sce doctor --fix` parsing/help, stable problem/fix-result reporting, canonical hook-repair reuse, bounded doctor-owned local-DB directory bootstrap for the missing SCE-owned DB parent path, and target-scoped integration inventory: Claude reports `Plugins`, `Commands`, and `Skills`; OpenCode reports `Plugins`, `Agents`, `Commands`, and `Skills`; Pi reports `Extensions`, `Prompts`, and `Skills`; and Codex reports `Skills` and `Hooks` (see `context/sce/doctor-human-text-contract.md`). Its non-launching post-commit Agent Trace auto-sync fact reports enabled/current, explicit disabled, not-ready, and not-applicable states using canonical managed-block currency and resolved configuration; existing hook remediation and readiness semantics remain unchanged. The local DB service now provides `LocalDb` as a thin `TursoDb` alias in `cli/src/services/local_db/mod.rs`; `LocalDbSpec` resolves the canonical local DB path from the shared default-path catalog and currently declares zero migrations. Shared Turso infrastructure lives in `cli/src/services/db/mod.rs`, where `DbSpec` and generic `TursoDb` support local or remote sync-mode opens, parent-directory creation, connection setup, synchronous query helpers, embedded migration execution, and shared DB lifecycle helpers. Auth DB persistence uses encrypted `AuthDb = EncryptedTursoDb` and token storage persists credentials through the `auth_credentials` table. Agent Trace persistence uses the sole `RepositoryAgentTraceDb = TursoDb` adapter at `/sce/repos//agent-trace.db`, with a one-file repository schema for `repository_metadata`, `diff_traces`, `post_commit_patch_intersections`, `agent_traces`, `messages`, `parts`, indexes/triggers, and no `checkout_id` columns on trace rows. The checkout-scoped `AgentTraceDb = TursoDb` adapter, its `agent_trace_db_path()`/`agent_trace_db_path_for_checkout()` helpers, and the 15-file `cli/migrations/agent-trace/` chain were removed by the `retire-legacy-agent-trace-db` plan; active hook runtime writes nullable event-local diff-trace attribution without a `session_models` API/table dependency. - The hooks command surface now also supports concrete runtime subcommand routing (`pre-commit`, `commit-msg`, `post-commit`, `post-rewrite`, `diff-trace`, and `conversation-trace`) with deterministic argument/STDIN validation; `session-model` is no longer supported. Current runtime behavior keeps commit-msg attribution enabled by default unless explicitly opted out: the attribution gate enables canonical trailer insertion in `commit-msg` only when the staged-diff AI-overlap preflight confirms AI/editor evidence (no trailer is appended when the preflight finds no overlap or encounters any error); `pre-commit`/`post-rewrite` remain deterministic no-ops, `post-commit` requires validated `--remote-url`, threads that URL into the Agent Trace flow, prints it to stderr, remains the active bounded recent-diff-trace intersection path, and after successful Agent Trace persistence optionally launches the detached sync-owned `sync --format json` child when config-file-only `agent_trace.auto_sync` is true; `diff-trace` is the active intake path for parsed STDIN `{ sessionID, diff, time, model_id?, tool_name, tool_version }` payload persistence with optional `model_id`, required non-empty `tool_name`, required nullable/non-empty `tool_version`, direct-first/event-transcript-second Claude `model_id` plus direct `tool_version` values (no session-model fallback or cache), required `u64` millisecond `time`, same-tool-idempotent stored `session_id` prefixing (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi), non-lossy AgentTraceDb `time_ms` conversion, and no parsed-payload artifact fallback. This behavior is documented in `context/sce/agent-trace-hooks-command-routing.md`. The removed `sce hooks claude-capture` raw capture route is documented in `context/sce/claude-raw-hook-capture.md` as a removed feature. + The hooks command surface now also supports concrete runtime subcommand routing (`pre-commit`, `commit-msg`, `post-commit`, `post-rewrite`, `diff-trace`, and `conversation-trace`) with deterministic argument/STDIN validation; `session-model` is no longer supported. Current runtime behavior keeps commit-msg attribution enabled by default unless explicitly opted out: the attribution gate enables canonical trailer insertion in `commit-msg` only when the staged-diff AI-overlap preflight confirms AI/editor evidence (no trailer is appended when the preflight finds no overlap or encounters any error); `pre-commit`/`post-rewrite` remain deterministic no-ops, `post-commit` requires validated `--remote-url`, threads that URL into the Agent Trace flow, prints it to stderr, remains the active bounded recent-diff-trace intersection path, and after successful Agent Trace persistence optionally launches the one-shot sync-owned `sync --format json` child when config-file-only `agent_trace.auto_sync` is true, waiting for terminal completion while remaining fail-open; `diff-trace` is the active intake path for parsed STDIN `{ sessionID, diff, time, model_id?, tool_name, tool_version }` payload persistence with optional `model_id`, required non-empty `tool_name`, required nullable/non-empty `tool_version`, direct-first/event-transcript-second Claude `model_id` plus direct `tool_version` values (no session-model fallback or cache), required `u64` millisecond `time`, same-tool-idempotent stored `session_id` prefixing (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi), non-lossy AgentTraceDb `time_ms` conversion, and no parsed-payload artifact fallback. This behavior is documented in `context/sce/agent-trace-hooks-command-routing.md`. The removed `sce hooks claude-capture` raw capture route is documented in `context/sce/claude-raw-hook-capture.md` as a removed feature. The setup service now also exposes deterministic required-hook embedded asset accessors (`iter_required_hook_assets`, `get_required_hook_asset`) backed by canonical templates in `cli/assets/hooks/` for `pre-commit`, `commit-msg`, and `post-commit`; this behavior is documented in `context/sce/setup-githooks-hook-asset-packaging.md`. The setup service now also includes required-hook install orchestration (`install_required_git_hooks`) that resolves repository root and effective hooks path from git truth, computes the bytes to stage by merging the canonical hook template with any existing hook (preserving a foreign hook's content as an exact prefix with the SCE managed block appended, or bringing an SCE-owned block current in place) rather than writing canonical bytes verbatim, enforces deterministic per-hook outcomes (`Installed`/`Updated`/`Skipped`) against that merged content, surfaces a deterministic advisory when an appended block would be unreachable, and uses a unified atomic-swap policy that renames staged content directly over existing hooks without unlinking them first, with deterministic recovery guidance on swap failures; this behavior is documented in `context/sce/setup-githooks-install-flow.md`. The setup command parser/dispatch now also supports composable setup+hooks runs (`sce setup --opencode|--claude|--pi|--codex|--all --hooks`) plus hooks-only mode (`sce setup --hooks` with optional `--repo `), enforces deterministic compatibility validation (`--repo` requires `--hooks`; target flags remain mutually exclusive), and emits deterministic setup/hook outcome messaging (`installed`/`updated`/`skipped`); this behavior is documented in `context/sce/setup-githooks-cli-ux.md`. diff --git a/context/patterns.md b/context/patterns.md index 4485fbb48..fea2801e5 100644 --- a/context/patterns.md +++ b/context/patterns.md @@ -128,7 +128,7 @@ - Keep `log_file_retention_limit` flat and config-file/default only: validate it as an integer with minimum `1`, merge global before local, default it to `10`, expose resolved source metadata without adding an environment variable or CLI flag, and pass the resolved value unchanged to primary and v2 creation-triggered logger cleanup. - For runtime CLI configuration, keep precedence deterministic and explicit (`flags > env > config file > defaults`) and expose inspect/validate command entrypoints with stable text/JSON outputs. Config-file-only Agent Trace runtime switches such as `agent_trace.auto_sync` should document their default and explicit opt-out, resolve global before local, and expose winning source metadata without adding an environment variable or CLI flag unless the contract explicitly requires one. -- For default-enabled automatic Agent Trace synchronization, keep the post-commit launcher as a one-shot reuse of `sce sync`: start the current executable after local Agent Trace persistence, use the repository root, null stdin/stdout, inherited stderr, and an internal automatic-invocation marker, do not wait, and fail open on launcher errors. Render child and launcher failures through one typed `SCE-ERR-RUNTIME` diagnostic beginning `Automatic synchronization failed:`; authentication must direct the user to `sce auth login` and then manual `sce sync`, while other kinds include their preserved reason and actionable manual retry guidance without duplicating generic runtime `Try:` text. Keep manual sync and the control-plane cursor authority as the retry path; do not add daemons, watchers, polling, queues, or high-frequency hook triggers. +- For default-enabled automatic Agent Trace synchronization, keep the post-commit launcher as a one-shot reuse of `sce sync`: start the current executable after local Agent Trace persistence, use the repository root, null stdin/stdout, inherited stderr, and an internal automatic-invocation marker, wait for terminal child completion so child lifetime and stderr closure are deterministic, and accept the resulting child/network latency at the post-commit boundary. Fail open on child, startup, or wait errors. Ignore a non-zero child exit after the child renders its one typed diagnostic; render launcher failures through the same typed `SCE-ERR-RUNTIME` diagnostic beginning `Automatic synchronization failed:`. Authentication must direct the user to `sce auth login` and then manual `sce sync`, while other kinds include their preserved reason and actionable manual retry guidance without duplicating generic runtime `Try:` text. Keep manual sync and the control-plane cursor authority as the retry path; do not add a timeout, daemon, watcher, polling, queue, or high-frequency hook trigger. - For commands that support text/JSON dual output, centralize `--format ` parsing in one shared contract and pass command-specific `--help` guidance into invalid-value errors instead of duplicating parser logic per command. - For setup-style command contracts, keep interactive mode as the zero-flag default and enforce mutually-exclusive explicit target flags for non-interactive automation. - For setup config safety, validate an existing repo-local config immediately after resolving the Git root and before prompts, context bootstrap, lifecycle providers, hooks, or target assets; preserve create-if-missing behavior for absent config and keep general startup degradation scoped away from setup and Agent Trace storage identity resolution. diff --git a/context/plans/auto-sync-failure-guidance.md b/context/plans/auto-sync-failure-guidance.md index b62922c4e..7953b3b3e 100644 --- a/context/plans/auto-sync-failure-guidance.md +++ b/context/plans/auto-sync-failure-guidance.md @@ -2,20 +2,18 @@ ## Change summary -Improve the existing post-commit automatic Agent Trace synchronization path so -that a failed detached sync produces a typed, user-facing diagnostic instead of -an opaque or invisible failure. Following the payload-bearing `UserError` -pattern used by the setup Git preflight (`NotGitRepository`/ -`NotGitRemote`), the automatic-sync error will carry its typed failure kind and -underlying reason while rendering reviewed recovery guidance; authentication -failures will explicitly direct the user to log in and then manually run -`sce sync`. - -The existing one-shot architecture remains intact: automatic sync still reuses -the `sce sync` command, does not delay the commit, and fails open. The detached -child will identify itself as an automatic invocation and expose only its -failure diagnostics through the existing stderr contract; it will not introduce -local retry state, a daemon, or a second synchronization implementation. +The completed T01-T03 work gives post-commit automatic Agent Trace +synchronization typed, user-facing failure diagnostics. Following the +completion-policy investigation, this revision changes the automatic launcher +to wait for its one-shot `sce sync --format json` child to reach terminal +completion before the post-commit hook returns. Child failures and wait errors +remain fail-open, while the child remains responsible for its typed diagnostic +so the parent does not duplicate it. + +The exact command, repository-root working directory, null stdout, inherited +stderr, internal automatic-invocation marker, one-shot architecture, and +no-daemon/no-retry boundaries remain unchanged. Manual `sce sync` semantics also +remain unchanged; only the automatic post-commit completion boundary changes. ## Acceptance criteria @@ -29,13 +27,24 @@ performs final validation. - Validate: Focused authentication classification and app-rendering tests assert the complete login-plus-manual-sync guidance and ensure the technical source remains available only for observability. - [x] AC3: Non-authentication automatic failures use the same typed payload-bearing error model, provide actionable recovery guidance, and explain that the user can manually retry with `sce sync`, without relying on substring matching or duplicating the default runtime `Try:` guidance. - Validate: Focused tests cover representative storage, transport/server, protocol, and stream failures and assert deterministic reason/recovery text with no duplicate remediation. -- [x] AC4: Automatic child failures are visible through the existing stderr diagnostic channel while successful post-commit execution remains detached, non-blocking, JSON-stdout-silent, and fail-open to the commit; launcher startup failures retain their reason in structured auto-sync diagnostics without failing the hook. - - Validate: Launcher and post-commit seam tests assert the internal automatic-invocation marker, inherited failure stderr, unchanged `sync --format json` arguments, no wait, and fail-open behavior for executable/spawn failures. +- [x] AC4: Automatic child failures are visible through the existing stderr diagnostic channel while successful post-commit execution remains JSON-stdout-silent and fail-open to the commit; launcher startup failures retain their reason in structured auto-sync diagnostics without failing the hook. + - Validate: Launcher and post-commit seam tests assert the internal automatic-invocation marker, inherited failure stderr, unchanged `sync --format json` arguments, and fail-open behavior for executable/spawn failures; the completion policy is governed by AC7. - [x] AC5: Manual `sce sync` failures retain the existing manual-sync semantics and do not claim that automatic synchronization failed. - Validate: Command-level tests execute/classify manual and automatic invocation modes separately and assert mode-specific rendering. - [x] AC6: Durable context documents the typed automatic-sync failure model, stderr visibility, authentication recovery, manual retry command, and preserved no-daemon/fail-open boundaries. - Validate: Review the listed context contracts against the final code, then run the generated-context and repository checks under `Full validation`. +AC1–AC6 are the completed baseline established by T01–T03. The following +criteria govern this revision and replace the prior detached-policy experiment +follow-up. + +- [x] AC7: The automatic post-commit launcher waits for the `sync --format json` child to reach terminal completion before returning, while preserving the repository-root cwd, internal marker, null stdout, inherited stderr, and fail-open hook boundary. + - Validate: Focused auto-sync and hook tests assert that the launcher waits, preserves the command boundary, and still returns successful post-commit results. +- [x] AC8: Automatic child non-zero exits and wait errors remain fail-open; child-rendered failures are not duplicated by the launcher, while launcher startup/wait failures retain actionable typed reasons on stderr. + - Validate: Focused launcher tests cover successful completion, non-zero child exit, wait failure, and startup failure with one diagnostic path and unchanged manual-sync behavior. +- [x] AC9: Durable context describes the synchronous automatic completion boundary, its commit-latency tradeoff, stderr behavior, manual retry path, and preserved no-daemon/no-retry constraints without stale detached-policy claims. + - Validate: Review the listed auto-sync, hook-routing, sync-command, sync-architecture, and stdout/stderr contracts against the selected implementation, then run the generated-context and repository checks under `Full validation`. + ### Full validation Repository-wide checks `/validate` runs after the last task, regardless of @@ -72,14 +81,15 @@ Persist this field in every plan; this is durable plan state, not chat state: - **In scope:** typed automatic-sync failure/recovery modeling; automatic-versus-manual sync invocation context; sync command/app error rendering; detached launcher stderr and startup-failure reporting; post-commit fail-open integration; focused Rust tests; the durable context files listed under Context sync. - **Out of scope:** changes to the control-plane protocol, cursor reconciliation, Agent Trace schema/storage, manual sync success output, authentication flow implementation, generated target trees, or unrelated hook failure behavior. -- **Constraints:** preserve the exact child arguments `sync --format json`, current-executable resolution, repository-root working directory, no wait, commit fail-open semantics, stdout/stderr separation, typed authentication classification, and shared sensitive-text redaction; add no dependency or persistent retry state. +- **Constraints:** preserve the exact child arguments `sync --format json`, current-executable resolution, repository-root working directory, commit fail-open semantics, stdout/stderr separation, typed authentication classification, and shared sensitive-text redaction; wait for the automatic child to reach terminal completion; treat non-zero child exits and wait errors as fail-open; do not duplicate child-rendered diagnostics; add no dependency, timeout policy, or persistent retry state. - **Non-goal:** making Git wait for network synchronization or adding a daemon, watcher, scheduler, queue, status file, or local retry cursor. ## Assumptions -- Automatic sync remains a detached child and reports completion failures through its inherited stderr rather than waiting for the child or persisting a new failure record; this preserves the existing one-shot/fail-open contract while making the diagnostic observable. +- T01-T03 remain the recorded baseline. The user has selected synchronous waiting after reviewing the completion-policy evidence, accepting the added commit latency in exchange for deterministic child completion and stream closure. - The automatic invocation marker is an internal process-boundary detail, not a new user configuration key or public CLI option; manual `sce sync` remains mode-neutral and keeps its existing error wording. - The typed failure model will preserve the technical source for structured logging while rendering a reviewed, deterministic recovery sentence at the app boundary, following `CliError` and `UserError` ownership patterns. +- “Wait for the child to finish” means waiting for terminal process completion without introducing a new timeout, retry, daemon, or persistent state mechanism. ## Task stack @@ -151,41 +161,122 @@ Persist this field in every plan; this is durable plan state, not chat state: - Context impact: root — clarified the durable CLI error, stream, synchronization, hook-routing, and recovery contracts to match the implemented automatic-sync behavior. - Context synchronization: synced +- [x] T04: `Measure Git commit impact of automatic-sync completion policies` (status:complete) + - Task ID: T04 + - Scope: In — a repository-owned focused experiment/benchmark using a real temporary Git repository and commit, a controlled child that delays and writes stderr, and comparable detached-plus-inherited-stderr, wait-to-exit, and (where useful) stderr-null runs; measure direct commit duration, a pipeline consuming stderr, output ordering, and pipe closure without changing production behavior. Out — choosing or implementing the production policy, network synchronization, persistent benchmark infrastructure, and changes to the completed T01-T03 contract. + - Dependencies: T03 + - Done when: the experiment runs deterministically enough to compare the policies, asserts the expected child-output and pipe-lifetime behavior, reports the measured commit/pipeline timings, and records a clear recommendation about whether waiting's latency is acceptable for post-commit use. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auto_sync -- --nocapture`; retain the focused experiment output and measurements in the task completion evidence. + - Completed: 2026-08-28 + - Files changed: + - `cli/src/services/sync/auto_sync.rs` + - Result: Added a Unix-focused repository-owned experiment that creates temporary Git repositories, runs real post-commit hooks, and compares detached inherited-stderr, waiting, and null-output child policies. The experiment captures direct commit latency, stderr pipe closure, stderr-consuming pipeline completion, output presence/order, and recommends retaining detached launch because waiting added roughly 250 ms to the direct commit in the measured run while inherited stderr kept the pipeline observable. + - Verify: + - `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auto_sync -- --nocapture` — passed (14 tests); experiment measurements: detached direct commit 6 ms / pipe close 259 ms / pipeline 266 ms, wait direct commit 258 ms / pipe close 258 ms / pipeline 261 ms, null direct commit 6 ms / pipe close 6 ms / pipeline 7 ms. + - `nix develop -c sh -c 'cd cli && cargo fmt'` — passed. + - Context impact: root — established experiment-backed evidence for the automatic-sync completion and inherited-stderr policy; durable context synchronization is required before another task starts. + - Context synchronization: synced + +- [x] T05: `Remove the T04 experiment and detached-policy context additions` (status:complete) + - Task ID: T05 + - Scope: In — remove the T04-only repository experiment and its test support from `cli/src/services/sync/auto_sync.rs`; remove the T04-only experiment findings and detached-policy recommendation from the durable context files changed by T04 synchronization. Out — completed T01-T03 implementation and context records, the existing detached launcher behavior, and the new waiting implementation. + - Dependencies: T04 + - Done when: no T04 experiment code or experiment-only context claims remain, the completed T04 record remains intact as historical plan evidence, and the context again describes the pre-T04 detached implementation without changing T01-T03 behavior. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auto_sync`; `git diff --check`; focused review confirms only T04 experiment additions and their context references were removed. + - Completed: 2026-08-31 + - Files changed: + - `cli/src/services/sync/auto_sync.rs` + - `context/architecture.md` + - `context/cli/agent-trace-auto-sync.md` + - `context/cli/sync-command.md` + - `context/glossary.md` + - `context/overview.md` + - `context/patterns.md` + - `context/sce/agent-trace-hooks-command-routing.md` + - Result: Removed the Unix Git completion-policy experiment and all experiment-only helpers while preserving the detached launcher implementation; removed the T04 experiment findings and detached-policy references from the durable context contracts, leaving the completed T04 plan record intact. + - Verify: + - `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auto_sync` — passed (13 tests). + - `git diff --check` — passed. + - Focused review of the affected code and context files — passed; no T04 experiment code or experiment-only context claims remain, and the detached launcher contract is unchanged. + - Context impact: root — restored the automatic-sync implementation and durable context to the pre-T04 detached-policy baseline; durable context synchronization is required before another task starts. + - Context synchronization: synced + +- [x] T06: `Wait for automatic sync child completion at the launcher boundary` (status:complete) + - Task ID: T06 + - Scope: In — `cli/src/services/sync/auto_sync.rs`, the launcher process boundary, and focused auto-sync/post-commit tests proving terminal wait, command preservation, output routing, and fail-open behavior. Wait for the spawned child to finish before returning; ignore a non-zero child exit after the child has rendered its own diagnostic; surface wait errors through one typed launcher diagnostic without failing the post-commit hook. Out — manual `sce sync`, control-plane behavior, timeout policy, retries, daemons, queues, and persistent failure state. + - Dependencies: T05 + - Done when: automatic post-commit execution waits for child termination, successful and failed child exits remain fail-open, wait errors are handled without duplicate diagnostics, and the exact `sync --format json` arguments, repository-root cwd, marker, null stdout, and inherited stderr remain intact. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auto_sync`; `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml hooks::`. + - Completed: 2026-08-31 + - Files changed: + - `cli/src/services/sync/auto_sync.rs` + - Result: Changed the automatic launcher to retain and wait for its one-shot sync child, ignoring non-zero child exits while routing wait failures through the existing typed fail-open launcher diagnostic; preserved the command arguments, repository-root cwd, automatic marker, null stdin/stdout, and inherited stderr. + - Verify: + - `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auto_sync` — passed (16 tests). + - `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml hooks::` — passed (163 tests). + - Context impact: root — changed the automatic sync process completion boundary and wait-failure diagnostic behavior while preserving the existing hook and stderr fail-open contracts; durable context synchronization is required before another task starts. + - Context synchronization: synced + +- [x] T07: `Document synchronous automatic-sync completion semantics` (status:complete) + - Task ID: T07 + - Scope: Update `context/overview.md`, `context/architecture.md`, `context/glossary.md`, `context/patterns.md`, `context/cli/agent-trace-auto-sync.md`, `context/cli/sync-command.md`, `context/cli/agent-trace-sync-command.md`, `context/sce/cli-stdout-stderr-contract.md`, and `context/sce/agent-trace-hooks-command-routing.md` to describe the waiting boundary, deterministic stream closure, commit-latency tradeoff, fail-open child/wait handling, and unchanged manual retry/no-daemon constraints. Out — application code, tests, generated target trees, and changes to T01-T03 error semantics. + - Dependencies: T06 + - Done when: durable context consistently describes automatic sync as a waited one-shot child, does not claim detached/no-wait behavior, preserves the typed stderr and manual `sce sync` recovery contracts, and keeps the no-daemon/no-retry boundaries explicit. + - Verify: Manual code/context review against `cli/src/services/sync/auto_sync.rs`, `cli/src/services/hooks/mod.rs`, `cli/src/services/sync/command.rs`, and `cli/src/services/app_support.rs`; `git diff --check`. + - Completed: 2026-08-31 + - Files changed: + - `context/overview.md` + - `context/architecture.md` + - `context/glossary.md` + - `context/patterns.md` + - `context/cli/agent-trace-auto-sync.md` + - `context/cli/sync-command.md` + - `context/cli/agent-trace-sync-command.md` + - `context/sce/cli-stdout-stderr-contract.md` + - `context/sce/agent-trace-hooks-command-routing.md` + - Result: Updated the durable root and domain context to describe the waited one-shot automatic sync child, deterministic child and inherited-stderr completion, the intentional post-commit latency trade-off, fail-open child/startup/wait handling, typed stderr recovery, and preserved manual retry/no-daemon/no-retry boundaries. + - Verify: + - `Manual code/context review against cli/src/services/sync/auto_sync.rs, cli/src/services/hooks/mod.rs, cli/src/services/sync/command.rs, and cli/src/services/app_support.rs` — passed; the launcher waits for terminal completion, preserves the exact command/working-directory/stdio/marker boundary, and the listed contracts match the implementation without stale detached/no-wait claims. + - `git diff --check` — passed. + - Context impact: root — changed durable synchronization, hook-routing, stream, CLI, architecture, glossary, and pattern contracts to match the synchronous automatic completion boundary; the mandatory task context synchronization pass is required. + - Context synchronization: synced + ## Open questions -None. The existing detached/fail-open contract determines that reporting must -travel through the child diagnostic stream rather than a completion wait or new -persistent retry mechanism, and the setup preflight change establishes the -payload-bearing `UserError` pattern to reuse; the remaining wording and -internal marker choices are local implementation details covered by the existing -CLI error/rendering patterns. +Waiting makes automatic post-commit execution visibly pay the child and network +latency. The user has selected that tradeoff for deterministic completion and +stream closure; the implementation should not add a timeout unless a later +change request explicitly establishes one. ## Validation Report **Status:** validated -**Date:** 2026-08-27 +**Date:** 2026-08-31 ### Commands run -- `nix flake check` -> exit 0 (all flake checks passed) +- `nix flake check` -> exit 0 (flake evaluation passed and reported all checks passed) - `nix run .#pkl-check-generated` -> exit 0 (ephemeral Pkl generation passed: 141 files) -- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml error::` -> terminated by the 120-second tool timeout while concurrent Cargo invocations waited on locks (no exit code reported; rerun completed successfully) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auto_sync` -> exit 0 (16 focused auto-sync tests passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml hooks::` -> exit 0 (163 focused hook tests passed) - `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml error::` -> exit 0 (6 focused error tests passed) - `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml sync::command` -> exit 0 (6 focused sync classification tests passed) -- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml app_support` -> exit 0 (5 focused app rendering tests passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml app_support` -> exit 0 (5 focused app-rendering tests passed) - `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml parse::command_runtime` -> exit 0 (5 focused command-runtime tests passed) -- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auto_sync` -> exit 0 (13 focused auto-sync and hook tests passed) -- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml hooks::` -> exit 0 (163 focused hook tests passed) +- `git diff --check` -> failed (the previous failed Validation Report contained trailing whitespace; the report was replaced and the check was rerun successfully) - `git diff --check` -> exit 0 (no whitespace errors) ### Success-criteria verification -- [x] AC1: A sync failure raised by an automatic invocation maps to a payload-bearing typed `UserError`/`CliError` path and renders one runtime diagnostic that clearly says automatic synchronization failed and includes the underlying typed failure reason. -> Error, app-rendering, and sync classification tests passed; typed runtime code and reason-preserving paths were inspected. -- [x] AC2: An automatic authentication failure uses a distinct typed automatic-sync failure kind, tells the user that authentication is required, instructs them to run `sce auth login`, and then explicitly tells them to manually retry with `sce sync`. -> Sync classification and app-rendering tests passed; authentication guidance and observability-only source handling were inspected. -- [x] AC3: Non-authentication automatic failures use the same typed payload-bearing error model, provide actionable recovery guidance, and explain that the user can manually retry with `sce sync`, without relying on substring matching or duplicating the default runtime `Try:` guidance. -> Sync, error, and app-rendering tests passed; representative typed control-plane, stream, runtime, storage, transport, server, and protocol paths were inspected. -- [x] AC4: Automatic child failures are visible through the existing stderr diagnostic channel while successful post-commit execution remains detached, non-blocking, JSON-stdout-silent, and fail-open to the commit; launcher startup failures retain their reason in structured auto-sync diagnostics without failing the hook. -> Auto-sync and hook seam tests passed; launcher arguments, marker, inherited stderr, null stdout, no-wait, and fail-open behavior were inspected. -- [x] AC5: Manual `sce sync` failures retain the existing manual-sync semantics and do not claim that automatic synchronization failed. -> Manual and automatic classification branches were inspected and focused sync tests passed. -- [x] AC6: Durable context documents the typed automatic-sync failure model, stderr visibility, authentication recovery, manual retry command, and preserved no-daemon/fail-open boundaries. -> Listed context contracts were reviewed against the final code; generated-context and repository checks passed. +- [x] AC1: A sync failure raised by an automatic invocation maps to a payload-bearing typed `UserError`/`CliError` path and renders one runtime diagnostic that clearly says automatic synchronization failed and includes the underlying typed failure reason. -> Focused error, sync-classification, app-rendering, and command-runtime tests passed; the typed catalog, reason-preserving classifier, and single renderer were inspected. +- [x] AC2: An automatic authentication failure uses a distinct typed automatic-sync failure kind, tells the user that authentication is required, instructs them to run `sce auth login`, and then explicitly tells them to manually retry with `sce sync`. -> Focused classification and app-rendering tests passed; the authentication template and preserved observability source were inspected. +- [x] AC3: Non-authentication automatic failures use the same typed payload-bearing error model, provide actionable recovery guidance, and explain that the user can manually retry with `sce sync`, without relying on substring matching or duplicating the default runtime `Try:` guidance. -> Focused sync, error, and app-rendering tests passed; typed matching, deterministic recovery text, and the absence of duplicate generic remediation were inspected. +- [x] AC4: Automatic child failures are visible through the existing stderr diagnostic channel while successful post-commit execution remains JSON-stdout-silent and fail-open to the commit; launcher startup failures retain their reason in structured auto-sync diagnostics without failing the hook. -> Auto-sync and hook tests passed; command arguments, marker, null stdout, inherited stderr, typed startup diagnostics, and fail-open behavior were inspected. +- [x] AC5: Manual `sce sync` failures retain the existing manual-sync semantics and do not claim that automatic synchronization failed. -> Manual and automatic classifier branches were inspected; sync classification and command-runtime tests passed. +- [x] AC6: Durable context documents the typed automatic-sync failure model, stderr visibility, authentication recovery, manual retry command, and preserved no-daemon/fail-open boundaries. -> The listed context contracts were reviewed against the final error, sync, launcher, hook, and app code; generated-context and repository validation passed. +- [x] AC7: The automatic post-commit launcher waits for the `sync --format json` child to reach terminal completion before returning, while preserving the repository-root cwd, internal marker, null stdout, inherited stderr, and fail-open hook boundary. -> Auto-sync and hook tests passed; `launch_with` waits for terminal completion and preserves the command boundary while post-commit remains fail-open. +- [x] AC8: Automatic child non-zero exits and wait errors remain fail-open; child-rendered failures are not duplicated by the launcher, while launcher startup/wait failures retain actionable typed reasons on stderr. -> Auto-sync tests passed for success, non-zero exit, startup, and wait outcomes; the typed launcher diagnostic path and non-duplication behavior were inspected. +- [x] AC9: Durable context describes the synchronous automatic completion boundary, its commit-latency tradeoff, stderr behavior, manual retry path, and preserved no-daemon/no-retry constraints without stale detached-policy claims. -> The listed root, CLI, sync, hook-routing, and stdout/stderr contracts were reviewed; they describe waited completion, latency, inherited stderr, manual retry, and no-daemon/no-retry boundaries without stale detached/no-wait claims. ### Failed checks and follow-ups diff --git a/context/sce/agent-trace-hooks-command-routing.md b/context/sce/agent-trace-hooks-command-routing.md index 747588542..4a48798cf 100644 --- a/context/sce/agent-trace-hooks-command-routing.md +++ b/context/sce/agent-trace-hooks-command-routing.md @@ -63,7 +63,7 @@ - Post-commit Agent Trace success requires both schema validation and Agent Trace DB `agent_traces` persistence to succeed. - Current command-surface success output is: `post-commit hook processed intersection: commit=, intersection_files=`. - After Agent Trace validation and `agent_traces` persistence succeed, post-commit runs exactly one passive WAL checkpoint through `RepositoryAgentTraceDb::passive_checkpoint()` (see [shared-turso-db.md](shared-turso-db.md)) before resolving auto-sync. This is routine maintenance, not a durability boundary: a checkpoint failure is logged as a warning via `Logger::warn` with event `sce.agent_trace_db.passive_checkpoint_failed` and does not fail the hook or affect already-persisted Agent Trace data. `diff-trace` and `conversation-trace` do not checkpoint per write; only this one post-commit call site does. - - After Agent Trace validation and `agent_traces` persistence succeed, post-commit resolves the config-file-only `agent_trace.auto_sync` gate. When it is `true`, the hook invokes the sync-owned one-shot launcher exactly once with the repository root; the launcher starts the current `sce` executable as detached `sync --format json` work, passes the internal `SCE_INTERNAL_AUTO_SYNC=1` marker, inherits stderr, and is not awaited. The child keeps stdin/stdout null, so successful JSON execution is silent, while automatic failures render one `Error [SCE-ERR-RUNTIME]` diagnostic beginning `Automatic synchronization failed:` through inherited stderr. Authentication tells the user to run `sce auth login` and then manually retry with `sce sync`; other typed failure kinds include their preserved reason and actionable manual retry guidance without a duplicate generic `Try:` sentence. Explicit `false` configuration does not launch; omitted configuration launches, and validation or persistence failure reaches the existing error path before the gate. Launcher/current-executable/spawn failures use the typed automatic-sync runtime payload with their reasons on stderr but remain fail-open and do not change the successful post-commit result. No `pre-commit`, `diff-trace`, or `conversation-trace` path invokes automatic synchronization. + - After Agent Trace validation and `agent_traces` persistence succeed, post-commit resolves the config-file-only `agent_trace.auto_sync` gate. When it is `true`, the hook invokes the sync-owned one-shot launcher exactly once with the repository root; the launcher starts the current `sce` executable with `sync --format json`, passes the internal `SCE_INTERNAL_AUTO_SYNC=1` marker, inherits stderr, and waits for terminal child completion. This makes child lifetime and inherited-stderr closure deterministic, intentionally adding the child synchronization duration to post-commit latency. The child keeps stdin/stdout null, so successful JSON execution is silent, while automatic failures render one `Error [SCE-ERR-RUNTIME]` diagnostic beginning `Automatic synchronization failed:` through inherited stderr. A non-zero child exit is ignored after the child renders that diagnostic, so the launcher does not duplicate it. Authentication tells the user to run `sce auth login` and then manually retry with `sce sync`; other typed failure kinds include their preserved reason and actionable manual retry guidance without a duplicate generic `Try:` sentence. Explicit `false` configuration does not launch; omitted configuration launches, and validation or persistence failure reaches the existing error path before the gate. Launcher/current-executable/spawn/wait failures use the typed automatic-sync runtime payload with their reasons on stderr but remain fail-open and do not change the successful post-commit result. No `pre-commit`, `diff-trace`, or `conversation-trace` path invokes automatic synchronization. - `post-rewrite` is a deterministic no-op entrypoint. - `diff-trace` reads STDIN JSON and classifies the payload: - **Claude structured payloads** (detected by presence of top-level `hook_event_name`): the STDIN JSON is validated through `derive_claude_structured_patch`. Supported `PostToolUse` `Write` create and `Edit` structured-patch events produce a `DiffTracePayload` with `payload_type="structured"` and the raw event JSON stored as the `diff` column without conversion to unified-diff text. Model attribution is resolved event-locally and direct-first: top-level `model`, `model_id`, or `modelId`, or nested `model.id`, `model.model`, or `model.name`, wins when present. Otherwise, when the event provides both `transcript_path` and `tool_use_id`, Rust scans that Claude JSONL transcript for the assistant-message envelope whose `tool_use.id` matches, skipping malformed unrelated records. Either source is normalized once with the `claude/` prefix. Missing/unreadable transcripts, unmatched tool calls, missing models, or absent lookup fields leave `model_id` nullable without rejecting the hook, and downstream Agent Trace JSON omits contributor `model_id`. No session-level cache or lookup participates. Unsupported Claude events (non-`PostToolUse`, unsupported tools, invalid payloads) produce a deterministic `NoOp` success result. diff --git a/context/sce/cli-stdout-stderr-contract.md b/context/sce/cli-stdout-stderr-contract.md index 104dc398a..1176e7d88 100644 --- a/context/sce/cli-stdout-stderr-contract.md +++ b/context/sce/cli-stdout-stderr-contract.md @@ -11,7 +11,7 @@ This document defines the implemented stream contract for CLI command payload an - Failure diagnostics are emitted as `Error []: ...` on `stderr`, where `` is the stable class-based `SCE-ERR-*` identifier from `CliError` in `cli/src/services/error.rs`; diagnostics are passed through shared redaction (`services::security::redact_sensitive_text`) before emission. - The diagnostic body differs by `CliError` variant: `CliError::Internal` renders the real `anyhow` source chain (`format!("{source:#}")`) plus class-default `Try:` remediation; `CliError::User` renders its catalog `UserError` template, with non-authentication automatic-sync payload reasons included only in that reviewed message and no technical source chain or `Try:` suffix. Both bodies are styled through the same stderr TTY/`NO_COLOR` policy before redaction and emission. - Command handlers now return payload strings to the app dispatcher; the app owns stream selection and final emission. -- The detached post-commit `sce sync --format json` child keeps stdout null so a successful automatic run produces no hook payload, while inheriting stderr so its single app-rendered typed failure diagnostic remains observable. Launcher executable/spawn failures are rendered through the same stderr diagnostic writer in the parent and remain fail-open. +- The post-commit `sce sync --format json` child keeps stdout null so a successful automatic run produces no hook payload, while inheriting stderr so its single app-rendered typed failure diagnostic remains observable. The launcher waits for terminal child completion, making child lifetime and inherited-stderr closure deterministic while adding child synchronization latency to post-commit execution. It ignores a non-zero child exit after that diagnostic and renders executable/spawn/wait failures through the same stderr diagnostic writer in the parent; all remain fail-open. - Automatic child failures render exactly one `Error [SCE-ERR-RUNTIME]: Automatic synchronization failed: ...` diagnostic on inherited stderr. Authentication exposes login-plus-manual-sync recovery while its technical reason stays in observability; non-authentication reasons and recovery guidance are included in that one diagnostic, with no generic duplicate `Try:` suffix. ## Implementation surface diff --git a/context/sce/doctor-human-text-contract.md b/context/sce/doctor-human-text-contract.md index c47874ad5..788b0dce1 100644 --- a/context/sce/doctor-human-text-contract.md +++ b/context/sce/doctor-human-text-contract.md @@ -63,9 +63,9 @@ omitted and is `false` only for the explicit config opt-out. `source` reports resolution fails; `config_source` identifies the discovered global or local config layer when applicable and is otherwise `null`. Doctor only reports this fact: it never launches `sce sync` or a background process. The post-commit -runtime still launches one detached `sync --format json` child only after -successful Agent Trace persistence when enabled, and launcher failures remain -fail-open. +runtime launches one `sync --format json` child only after successful Agent Trace +persistence when enabled, waits for terminal completion, and keeps child, +launcher, and wait failures fail-open. ## Integration hierarchy From bb4e474df6018fdbbc44f37b01844a6a4628f36b Mon Sep 17 00:00:00 2001 From: Ivan Ivic Date: Tue, 1 Sep 2026 11:55:29 +0200 Subject: [PATCH 5/5] runtime: Capture automatic-sync child stderr Keep automatic-sync diagnostics visible without allowing the child to inherit the caller's stderr descriptor. Pipe and drain child stderr with wait_with_output, then forward captured bytes through the parent while preserving synchronous completion, command configuration, and fail-open behavior. Co-authored-by: SCE --- cli/src/services/sync/auto_sync.rs | 56 +++++--- context/architecture.md | 4 +- context/cli/agent-trace-auto-sync.md | 26 ++-- context/cli/agent-trace-sync-command.md | 2 +- context/cli/sync-command.md | 7 +- context/context-map.md | 5 +- ...9-01-parent-owned-automatic-sync-stderr.md | 73 ++++++++++ context/glossary.md | 2 +- context/overview.md | 6 +- context/patterns.md | 2 +- context/plans/auto-sync-captured-stderr.md | 135 ++++++++++++++++++ .../sce/agent-trace-hooks-command-routing.md | 2 +- context/sce/cli-stdout-stderr-contract.md | 4 +- 13 files changed, 276 insertions(+), 48 deletions(-) create mode 100644 context/decisions/2026-09-01-parent-owned-automatic-sync-stderr.md create mode 100644 context/plans/auto-sync-captured-stderr.md diff --git a/cli/src/services/sync/auto_sync.rs b/cli/src/services/sync/auto_sync.rs index 5acd57598..bb85c7d2c 100644 --- a/cli/src/services/sync/auto_sync.rs +++ b/cli/src/services/sync/auto_sync.rs @@ -1,8 +1,8 @@ //! Best-effort launcher for one-shot automatic Agent Trace synchronization. -use std::io; +use std::io::{self, Write}; use std::path::{Path, PathBuf}; -use std::process::{Child, Command, ExitStatus, Stdio}; +use std::process::{Child, Command, Output, Stdio}; use crate::services::app_support; use crate::services::error::{AutomaticSyncFailureKind, CliError, UserError}; @@ -13,7 +13,7 @@ const SYNC_ARGS: &[&str] = &["sync", "--format", "json"]; #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum StdioMode { Null, - Inherit, + Piped, } #[derive(Clone, Debug, Eq, PartialEq)] @@ -35,7 +35,7 @@ impl AutoSyncCommand { current_dir: repository_root.to_path_buf(), stdin: StdioMode::Null, stdout: StdioMode::Null, - stderr: StdioMode::Inherit, + stderr: StdioMode::Piped, environment: vec![( AUTOMATIC_SYNC_INVOCATION_ENV.to_string(), AUTOMATIC_SYNC_INVOCATION_VALUE.to_string(), @@ -72,17 +72,17 @@ impl std::error::Error for AutoSyncLaunchError { } struct AutoSyncChild { - wait: Option io::Result>>, + wait: Option io::Result>>, } impl AutoSyncChild { - fn from_child(mut child: Child) -> Self { + fn from_child(child: Child) -> Self { Self { - wait: Some(Box::new(move || child.wait())), + wait: Some(Box::new(move || child.wait_with_output())), } } - fn wait(mut self) -> io::Result { + fn wait_with_output(mut self) -> io::Result { (self .wait .take() @@ -105,10 +105,16 @@ fn launcher_failure_diagnostic(error: AutoSyncLaunchError) -> CliError { /// the one-shot child to reach terminal completion. Launcher failures are /// reported on stderr but remain fail-open to the post-commit caller. pub fn launch(repository_root: &Path) { - if let Err(error) = launch_with(repository_root, std::env::current_exe, spawn_command) { - let diagnostic = launcher_failure_diagnostic(error); - let mut stderr = io::stderr(); - app_support::write_error_diagnostic(&mut stderr, &diagnostic); + match launch_with(repository_root, std::env::current_exe, spawn_command) { + Ok(captured_stderr) => { + let mut stderr = io::stderr(); + let _ = stderr.write_all(&captured_stderr); + } + Err(error) => { + let diagnostic = launcher_failure_diagnostic(error); + let mut stderr = io::stderr(); + app_support::write_error_diagnostic(&mut stderr, &diagnostic); + } } } @@ -116,7 +122,7 @@ fn launch_with( repository_root: &Path, current_exe: FCurrentExe, spawn: FSpawn, -) -> Result<(), AutoSyncLaunchError> +) -> Result, AutoSyncLaunchError> where FCurrentExe: FnOnce() -> io::Result, FSpawn: FnOnce(AutoSyncCommand) -> io::Result, @@ -125,7 +131,10 @@ where let child = spawn(AutoSyncCommand::new(executable, repository_root)) .map_err(AutoSyncLaunchError::Spawn)?; - child.wait().map(|_| ()).map_err(AutoSyncLaunchError::Wait) + child + .wait_with_output() + .map(|output| output.stderr) + .map_err(AutoSyncLaunchError::Wait) } fn spawn_command(spec: AutoSyncCommand) -> io::Result { @@ -136,7 +145,7 @@ fn spawn_command(spec: AutoSyncCommand) -> io::Result { .envs(spec.environment) .stdin(Stdio::null()) .stdout(Stdio::null()) - .stderr(Stdio::inherit()); + .stderr(Stdio::piped()); Ok(AutoSyncChild::from_child(command.spawn()?)) } @@ -146,6 +155,7 @@ mod tests { use std::cell::RefCell; use std::io; use std::path::{Path, PathBuf}; + use std::process::Output; use std::rc::Rc; use super::{ @@ -155,13 +165,21 @@ mod tests { fn child_with_wait(wait: F) -> AutoSyncChild where - F: FnOnce() -> io::Result + 'static, + F: FnOnce() -> io::Result + 'static, { AutoSyncChild { wait: Some(Box::new(wait)), } } + fn child_output(success: bool, stderr: &[u8]) -> Output { + Output { + status: exit_status(success), + stdout: Vec::new(), + stderr: stderr.to_vec(), + } + } + fn exit_status(success: bool) -> std::process::ExitStatus { #[cfg(unix)] { @@ -188,7 +206,7 @@ mod tests { || Ok(PathBuf::from("/usr/local/bin/sce")), move |command: AutoSyncCommand| { *captured_by_spawn.borrow_mut() = Some(command); - Ok(child_with_wait(|| Ok(exit_status(true)))) + Ok(child_with_wait(|| Ok(child_output(true, &[])))) }, ); @@ -201,7 +219,7 @@ mod tests { current_dir: PathBuf::from("/repo/root"), stdin: StdioMode::Null, stdout: StdioMode::Null, - stderr: StdioMode::Inherit, + stderr: StdioMode::Piped, environment: vec![( AUTOMATIC_SYNC_INVOCATION_ENV.to_string(), AUTOMATIC_SYNC_INVOCATION_VALUE.to_string(), @@ -220,7 +238,7 @@ mod tests { || Err(io::Error::other("current executable unavailable")), move |_| -> io::Result { *spawn_called_by_spawn.borrow_mut() = true; - Ok(child_with_wait(|| Ok(exit_status(true)))) + Ok(child_with_wait(|| Ok(child_output(true, &[])))) }, ); diff --git a/context/architecture.md b/context/architecture.md index 62b16ba4f..53bdd766c 100644 --- a/context/architecture.md +++ b/context/architecture.md @@ -105,7 +105,7 @@ 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`, 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 typed `CliError` boundary includes payload-bearing automatic-sync user errors whose app-owned rendering uses the `Automatic synchronization failed:` prefix and preserves mode-specific recovery guidance without appending default runtime remediation; the post-commit child inherits stderr for that diagnostic, waits at the launcher boundary, and keeps JSON stdout silent. Waiting makes child completion and inherited-stderr closure deterministic, while intentionally adding child synchronization latency to post-commit execution. +- `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`, 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 typed `CliError` boundary includes payload-bearing automatic-sync user errors whose app-owned rendering uses the `Automatic synchronization failed:` prefix and preserves mode-specific recovery guidance without appending default runtime remediation; the post-commit launcher pipes and captures child stderr, waits at the launcher boundary, forwards captured bytes through the parent stderr path, and keeps JSON stdout silent. Waiting makes child completion and pipe draining deterministic, while intentionally adding child synchronization latency to post-commit execution. - 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. - 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, 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. @@ -135,7 +135,7 @@ The repository includes a new placeholder Rust binary crate at `cli/`. - `cli/src/services/hooks/mod.rs` defines the current local hook runtime parsing/dispatch (`HookSubcommand`, `run_hooks_subcommand`) plus a commit-msg co-author policy seam (`apply_commit_msg_coauthor_policy`) that injects one canonical SCE trailer only when the enabled-by-default attribution-hooks config/env control is not opted out, `SCE_DISABLED` is false, and the staged-diff AI-overlap preflight confirms AI/editor evidence (`StagedDiffAiOverlapResult::Overlap`); the preflight is wired into `run_commit_msg_subcommand_in_repo` and logs `sce.hooks.commit_msg.ai_overlap_error` on error paths; `cli/src/services/hooks/command.rs` owns the `HooksCommand` payload used by the static `RuntimeCommand` enum. In the current attribution-only baseline, `pre-commit` and `post-rewrite` are deterministic no-op surfaces; `post-commit` requires validated `--remote-url`, threads that URL through the Agent Trace flow, prints it to stderr, and remains an active intersection + Agent Trace persistence entrypoint (captures current commit patch, queries recent repository-level `diff_traces` from the bounded past-7-days window, combines valid patches via `patch::combine_patches`, intersects with post-commit patch via `patch::intersect_patches`, persists result to `post_commit_patch_intersections`, then persists built Agent Trace payloads with range-level `content_hash` values to `agent_traces` in the repository-scoped Agent Trace DB without post-commit file artifacts); after successful validation and persistence, the default-enabled config-file-only `agent_trace.auto_sync` gate launches one one-shot sync-owned `sync --format json` child unless explicitly disabled in config, waits for terminal completion, ignores non-zero child exit after child diagnostics, and keeps launcher failures fail-open without a high-frequency hook trigger; `diff-trace` performs STDIN JSON intake, validates required non-empty `sessionID`/`diff`/`tool_name`, optional `model_id` (absent/`null` → `None`), required nullable/non-empty `tool_version` plus required `u64` `time` (Unix epoch milliseconds), rejects values that cannot fit signed `time_ms` storage, prefixes the stored `diff_traces.session_id` before insert construction (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi, same-tool idempotent), and inserts the parsed payload fields into `RepositoryAgentTraceDb` without creating a parsed-payload `context/tmp` artifact; Claude structured `PostToolUse` diff-trace intake resolves model attribution event-locally: direct top-level or nested metadata wins, otherwise the event's `transcript_path` is scanned for the assistant envelope whose `tool_use.id` matches `tool_use_id`; either source is normalized once with the `claude/` prefix and lookup failures remain nullable. `session-model` is no longer a supported hook route. - Generated Claude settings no longer register `SessionStart` for Agent Trace model attribution, and `sce hooks session-model` is no longer a supported hook command. The `session_models` table/API and session-level fallback lookup were removed in T02 of the `remove-session-models-direct-claude-model-id` plan; `diff-trace` now uses direct-first/event-transcript-second Claude `model_id` resolution and direct `tool_version` values, without restoring session-level state. - `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`, waits for terminal child completion, makes inherited-stderr closure deterministic, intentionally adds child synchronization latency to post-commit execution, has no daemon or local retry machinery, and fails open when child startup, completion wait, or synchronization fails; doctor reports this capability without invoking the launcher. +- `context/cli/agent-trace-auto-sync.md` documents the sync-owned, one-shot post-commit launcher boundary: it reuses `sce sync`, pipes and drains child stderr during terminal completion, forwards captured diagnostics through the parent, intentionally adds child synchronization latency to post-commit execution, has no daemon or local retry machinery, and fails open when child startup, completion wait, or synchronization fails; 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 four-stream payload semantics, 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. - `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. diff --git a/context/cli/agent-trace-auto-sync.md b/context/cli/agent-trace-auto-sync.md index 319e84d9f..51f5c1961 100644 --- a/context/cli/agent-trace-auto-sync.md +++ b/context/cli/agent-trace-auto-sync.md @@ -25,20 +25,20 @@ sync --format json ``` The child runs with the repository root as its working directory, null stdin and -stdout, and inherited stderr. An internal `SCE_INTERNAL_AUTO_SYNC=1` process +stdout, and piped stderr. An internal `SCE_INTERNAL_AUTO_SYNC=1` process marker lets the child classify this invocation as automatic without adding a user-facing option or configuration layer. The launcher waits for the child to -reach terminal completion before returning, making the child lifetime and -inherited-stderr closure deterministic. This intentionally adds the child -synchronization duration to post-commit latency; no timeout policy is part of -the boundary. A non-zero child exit remains -fail-open after the child has rendered its own single typed `SCE-ERR-RUNTIME` -diagnostic through inherited stderr; the launcher does not duplicate it. If the -current executable cannot be resolved, the child cannot be spawned, or waiting -fails, the launcher emits the same typed automatic-sync diagnostic with the -startup or wait reason on stderr. All launcher and child failures remain -fail-open, so they cannot turn a successful post-commit operation into a -failure. +reach terminal completion before returning, draining the pipe while waiting and +forwarding the captured bytes through the parent's stderr. This makes the child +lifetime and pipe-drain completion deterministic. This intentionally adds the +child synchronization duration to post-commit latency; no timeout policy is part +of the boundary. A non-zero child exit remains fail-open after the child has +rendered its own single typed `SCE-ERR-RUNTIME` diagnostic; the launcher forwards +the captured bytes and does not duplicate it. If the current executable cannot be +resolved, the child cannot be spawned, or waiting fails, the launcher emits the +same typed automatic-sync diagnostic with the startup or wait reason on stderr. +All launcher and child failures remain fail-open, so they cannot turn a +successful post-commit operation into a failure. Automatic synchronization is not invoked by `pre-commit`, `diff-trace`, or `conversation-trace`. It is one post-commit launch, not a high-frequency hook, @@ -93,4 +93,4 @@ or background retry machinery is required. See [the sync command contract](sync-command.md), [the config precedence contract](config-precedence-contract.md), [the hook routing contract](../sce/agent-trace-hooks-command-routing.md), -and [the completion decision](../decisions/2026-08-31-synchronous-automatic-sync-completion.md). +and [the captured-stderr decision](../decisions/2026-09-01-parent-owned-automatic-sync-stderr.md). diff --git a/context/cli/agent-trace-sync-command.md b/context/cli/agent-trace-sync-command.md index 3297dfb03..87d8c8258 100644 --- a/context/cli/agent-trace-sync-command.md +++ b/context/cli/agent-trace-sync-command.md @@ -45,7 +45,7 @@ Because every invocation starts from the control plane's authoritative `/state` - **`401` (unexpected):** the control-plane client refreshes the WorkOS token exactly once, saves it, and retries the request exactly once. Concurrent callers that observed the same rejected token coalesce onto the first refresh and reuse its saved token; a second `401` (`ControlPlaneError::MissingCredentials`/`AuthenticationFailed`) fails the command with `sce auth login` guidance, and there is no further retry. - **Typed authentication classification:** `ControlPlaneError::is_authentication_failure()` is `true` only for `MissingCredentials`/`AuthenticationFailed`, and the same typed traversal is exposed as `StreamSyncError::is_authentication_failure()` and `TraceSyncError::is_authentication_failure()` — no sync-stream path erases a `ControlPlaneError` into a bare `String` before it reaches the command boundary. `cli/src/services/sync/command.rs`'s `classify_sync_error` calls this traversal (never string/substring matching). Manual invocations route authentication failures from the initial `/state` call, a stream batch request, or a stream reconciliation `/state` refresh to `CliError::User { error: UserError::NotAuthenticated, .. }`; every other failure stays internal with its full technical chain preserved. Automatic invocations use the payload-bearing `UserError::AutomaticSyncFailed` entry, with a typed authentication, control-plane, stream, or runtime failure kind and the display reason preserved separately from the technical source. See [CLI error-code taxonomy](../sce/cli-error-code-taxonomy.md) for the `CliError`/`UserError` architecture and [sync-command.md](sync-command.md#error-classification) for the command-level classifier. -- **Automatic invocation boundary:** Automatic execution is selected only by the one-shot launcher's internal `SCE_INTERNAL_AUTO_SYNC=1` process marker; manual `sce sync` remains mode-neutral. The child inherits stderr and the launcher waits for terminal completion, making child lifetime and stderr closure deterministic while adding the child synchronization duration to post-commit latency. Its one app-rendered runtime diagnostic is visible without exposing JSON stdout. Non-zero child exits, launcher startup failures, and wait failures remain fail-open to the hook; startup and wait failures use the same typed automatic-sync payload with their preserved reason. +- **Automatic invocation boundary:** Automatic execution is selected only by the one-shot launcher's internal `SCE_INTERNAL_AUTO_SYNC=1` process marker; manual `sce sync` remains mode-neutral. The child stderr is piped and drained by the launcher while it waits for terminal completion, then forwarded through the parent stderr path, making child lifetime and stderr transport deterministic while adding the child synchronization duration to post-commit latency. Its one app-rendered runtime diagnostic is visible without exposing JSON stdout. Non-zero child exits, launcher startup failures, and wait failures remain fail-open to the hook; startup and wait failures use the same typed automatic-sync payload with their preserved reason. - **Automatic failure rendering:** Automatic terminal failures use the closed `AutomaticSyncFailureKind` catalog (`Authentication`, `ControlPlane`, `Stream`, or `Runtime`) and render one `SCE-ERR-RUNTIME` message beginning `Automatic synchronization failed:`. Authentication renders login-plus-manual-sync guidance while retaining its technical reason only for observability; non-authentication failures render their preserved reason and actionable manual `sce sync` recovery without a duplicate generic `Try:` sentence. - **`409` (cursor conflict):** the per-stream sync engine reconciles by refetching `/state`, replacing only the affected stream's cursor, and resuming from local rows after the refreshed cursor — already-accepted rows are never resent. - **Ambiguous batch failure (`5xx`, transport failure, or an undecodable `2xx` body):** the engine reconciles via `/state` before any resend. If the refreshed cursor advanced (the batch was actually committed), sync continues from it without resending. If the cursor is unchanged (the batch was not committed), sync may resend once from the authoritative cursor. diff --git a/context/cli/sync-command.md b/context/cli/sync-command.md index 884d06e74..2d4a7d163 100644 --- a/context/cli/sync-command.md +++ b/context/cli/sync-command.md @@ -15,9 +15,10 @@ automatic executions can retain distinct error semantics without adding a public CLI option. The same boundary owns a best-effort one-shot launcher used by the post-commit hook when `agent_trace.auto_sync` is enabled: it resolves the current `sce` executable, starts `sync --format json` in the repository root with null stdin and -stdout plus inherited stderr, and waits for terminal child completion, making the -child lifetime and inherited-stderr closure deterministic at the cost of adding -child synchronization latency to post-commit execution. It passes the internal +stdout plus piped stderr, waits for terminal child completion while draining the +pipe, and forwards captured stderr through the parent, making the child lifetime +and stderr transport deterministic at the cost of adding child synchronization +latency to post-commit execution. It passes the internal `SCE_INTERNAL_AUTO_SYNC=1` marker so automatic failures use the typed automatic-sync diagnostic path. A non-zero child exit remains fail-open after the child renders its own diagnostic; executable, spawn, and wait failures diff --git a/context/context-map.md b/context/context-map.md index a9e325449..7d04d41cc 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -17,7 +17,7 @@ Feature/domain context: - `context/cli/patch-service.md` (standalone patch domain model, parser, JSON load helpers, and set operations in `cli/src/services/patch.rs` for in-memory parsed unified-diff representation, capturing only touched lines plus minimal per-file/per-hunk metadata, supporting both `Index:` SVN-style and `diff --git` git-style formats, with `ParseError` for actionable malformed-input diagnostics, `PatchLoadError`/`load_patch_from_json`/`load_patch_from_json_bytes` for storage-agnostic JSON reconstruction, `intersect_patches` for target-shaped overlap with exact-match-first and historical `kind`+`content` fallback semantics plus matched-constructed-line `session_id` and matched-constructed-hunk `model_id` provenance inheritance, and `combine_patches` for ordered patch combination with later-wins conflict resolution plus winning-hunk `model_id` provenance inheritance; repository structured-row reconstruction supplies persisted hunk-model and canonical touched-line-session provenance before these operations; `parse_patch`, `intersect_patches`, and `combine_patches` are consumed by the active post-commit hook runtime) - `context/cli/structured-patch-service.md` (Claude structured editor-hook derivation in `cli/src/services/structured_patch.rs`, including `Write` structured-update hunks, `Write` `tool_input.content` create fallback, `Edit` structured patches, deterministic skip reasons, `ParsedPatch` output semantics, Rust golden fixture coverage, and repository read-time enrichment that assigns persisted row `model_id` to each hunk and canonical row `session_id` to each touched line) - `context/cli/styling-service.md` (CLI text-mode output styling with `owo-colors`, TTY/`NO_COLOR` policy, shared helper API for human-facing surfaces including sync completion markers, and per-column right-to-left RGB gradient banner rendering) -- `context/cli/agent-trace-auto-sync.md` (default-enabled post-commit Agent Trace synchronization with explicit-false opt-out plus doctor readiness reporting: the existing `sce sync` command launched once through the current executable after local persistence, waited to terminal child completion with null stdin/stdout and inherited stderr, an internal automatic-invocation marker, typed `Authentication`/`ControlPlane`/`Stream`/`Runtime` child and launcher-failure diagnostics beginning `Automatic synchronization failed:`, fail-open child/startup/wait handling, canonical managed-block proof of hook readiness, stable text/JSON enabled/disabled/not-ready/not-applicable states, no daemon/queue/high-frequency trigger, and retryability through manual sync and control-plane cursor authority) +- `context/cli/agent-trace-auto-sync.md` (default-enabled post-commit Agent Trace synchronization with explicit-false opt-out plus doctor readiness reporting: the existing `sce sync` command launched once through the current executable after local persistence, waited to terminal child completion while draining piped stderr and forwarding captured bytes through the parent with null stdin/stdout, an internal automatic-invocation marker, typed `Authentication`/`ControlPlane`/`Stream`/`Runtime` child and launcher-failure diagnostics beginning `Automatic synchronization failed:`, fail-open child/startup/wait handling, canonical managed-block proof of hook readiness, stable text/JSON enabled/disabled/not-ready/not-applicable states, no daemon/queue/high-frequency trigger, and retryability through manual sync and control-plane cursor authority) - `context/cli/sync-command.md` (the top-level `sce sync` command: repository-scoped Agent Trace storage resolution, WorkOS-authenticated four-stream control-plane synchronization 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, 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`, default-enabled `agent_trace.auto_sync` boolean resolution with explicit-false opt-out 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) @@ -98,7 +98,8 @@ Supporting repo docs: Recent decision records: -- `context/decisions/2026-08-31-synchronous-automatic-sync-completion.md` (accepted synchronous terminal-completion boundary for the one-shot automatic post-commit sync launcher, preserving fail-open child/startup/wait handling, inherited stderr, null stdout, and no-daemon/no-retry constraints) +- `context/decisions/2026-09-01-parent-owned-automatic-sync-stderr.md` (accepted parent-owned automatic-sync stderr transport: pipe and drain the child while waiting, then forward captured bytes through the parent without changing synchronous completion, stdout silence, or fail-open/no-daemon/no-retry constraints) +- `context/decisions/2026-08-31-synchronous-automatic-sync-completion.md` (accepted synchronous terminal-completion boundary for the one-shot automatic post-commit sync launcher; its inherited-stderr transport choice is superseded by the captured-stderr decision above) - `context/decisions/2026-08-23-codex-canonical-worktree-path-resolution.md` (accepts upstream-compatible Codex apply_patch parent/absolute paths only when canonical resolution remains inside the Git worktree, validates nearest existing prefixes for missing targets, and rejects symlink escapes) - `context/decisions/2026-08-23-codex-nondestructive-hook-ownership.md` (uses one shared structural ownership predicate and merge service for setup/doctor: Codex SCE handlers require the generated helper path plus the `sce hooks codex` contract, while unrelated hook configuration survives) - `context/decisions/2026-08-23-codex-event-scoped-apply-patch-evidence-identities.md` (uses bounded, deterministic `tool_use_id`-derived synthetic line identities for Codex apply_patch evidence; positions are evidence identities rather than source line numbers, with existing patch combination/intersection semantics unchanged) diff --git a/context/decisions/2026-09-01-parent-owned-automatic-sync-stderr.md b/context/decisions/2026-09-01-parent-owned-automatic-sync-stderr.md new file mode 100644 index 000000000..225b0e0d2 --- /dev/null +++ b/context/decisions/2026-09-01-parent-owned-automatic-sync-stderr.md @@ -0,0 +1,73 @@ +# Decision: Parent-owned captured stderr for automatic sync + +Date: 2026-09-01 +Status: Accepted +Plan: `context/plans/auto-sync-captured-stderr.md` +Task: `T01` +Supersedes: `context/decisions/2026-08-31-synchronous-automatic-sync-completion.md` + +## Context + +The synchronous post-commit launcher must keep automatic `sce sync --format json` +stdout silent, preserve visible typed failure diagnostics, and avoid leaving the +child attached to the caller's stderr descriptor. The previously accepted +completion boundary requires the launcher to wait for terminal child completion, +but inherited stderr conflicts with that descriptor-ownership requirement. + +## Decision + +The automatic launcher pipes the child stderr, drains it through terminal child +completion, and forwards the captured bytes through the parent's stderr after the +wait succeeds. + +## Rationale + +`wait_with_output` drains the pipe while waiting, avoiding a full-pipe deadlock +while retaining synchronous completion. Parent forwarding preserves the child's +operator-visible diagnostics without descriptor inheritance, and ignoring the +child exit status preserves the existing single-diagnostic fail-open behavior. + +## Alternatives considered + +- **Inherit stderr** — rejected because the child retains the post-commit caller's + stderr descriptor. +- **Suppress captured stderr** — rejected because automatic failure visibility is + an established operator-facing contract. +- **Stream stderr concurrently to the parent** — rejected as unnecessary added + coordination when capture plus `wait_with_output` provides bounded synchronous + draining and preserves output bytes. + +## Compatibility and risks + +- Successful automatic JSON sync remains stdout-silent and child diagnostics + remain visible on stderr, but they are forwarded after completion rather than + appearing live while the child runs. +- A wait failure still uses the one typed launcher diagnostic and remains + fail-open; a non-zero child exit does not create a duplicate parent diagnostic. + +## Guardrails + +- Keep the one-shot `sync --format json` command, repository-root working + directory, null stdin/stdout, internal automatic marker, and synchronous wait. +- Do not add a timeout, retry, queue, daemon, or persistent synchronization state. +- Do not change manual sync stream routing or the post-commit trigger boundary. + +## Consequences + +- The parent owns the automatic child's stderr descriptor and forwards captured + diagnostics through its own stderr path. +- Automatic diagnostics are buffered until terminal completion, while pipe + draining remains deadlock-safe. + +## Follow-up + +- Update current-state automatic-sync and CLI stream contracts to remove inherited + stderr wording and describe parent forwarding. + +## References + +- Plan: [`auto-sync-captured-stderr`](../plans/auto-sync-captured-stderr.md) +- Task: `T01` +- Current-state context: [`Automatic Agent Trace synchronization`](../cli/agent-trace-auto-sync.md) +- Evidence: [`automatic sync launcher`](../../cli/src/services/sync/auto_sync.rs) +- Related decision: [`Wait for automatic sync completion at the post-commit launcher boundary`](2026-08-31-synchronous-automatic-sync-completion.md) diff --git a/context/glossary.md b/context/glossary.md index 1fe827e27..89cfac179 100644 --- a/context/glossary.md +++ b/context/glossary.md @@ -74,7 +74,7 @@ - `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). No CLI dev-dependencies are currently declared, and the baseline is validated through normal compile/test coverage. - `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 four concurrent Agent Trace streams; `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. The same sync request carries internal `SyncInvocation` context for manual versus automatic execution, with automatic failures represented by the closed `AutomaticSyncFailureKind` catalog (`Authentication`, `ControlPlane`, `Stream`, `Runtime`) and its reviewed `Automatic synchronization failed:` recovery guidance. -- `agent_trace.auto_sync`: Config-file-only boolean resolved by the shared config layers for the post-commit Agent Trace synchronization boundary; omitted values are `true` and explicit `false` opts out, and `sce config show` reports its winning source. Enabled post-commit runs launch `sync --format json` once through the current executable with repository-root working directory, null stdin/stdout, inherited stderr, and an internal `SCE_INTERNAL_AUTO_SYNC=1` marker, then wait for terminal child completion so child lifetime and stderr closure are deterministic; this intentionally adds child synchronization latency to post-commit execution. Launcher and child failures remain fail-open and are visible through one typed `SCE-ERR-RUNTIME` diagnostic beginning `Automatic synchronization failed:`. Non-zero child exits do not cause a second parent diagnostic. Authentication directs the user through `sce auth login` and manual `sce sync`, while other kinds include their preserved reason and manual retry guidance. See [Agent Trace hook routing](sce/agent-trace-hooks-command-routing.md) and [the completion decision](decisions/2026-08-31-synchronous-automatic-sync-completion.md). +- `agent_trace.auto_sync`: Config-file-only boolean resolved by the shared config layers for the post-commit Agent Trace synchronization boundary; omitted values are `true` and explicit `false` opts out, and `sce config show` reports its winning source. Enabled post-commit runs launch `sync --format json` once through the current executable with repository-root working directory, null stdin/stdout, piped stderr, and an internal `SCE_INTERNAL_AUTO_SYNC=1` marker, then wait with pipe draining for terminal child completion and forward captured stderr through the parent; this intentionally adds child synchronization latency to post-commit execution. Launcher and child failures remain fail-open and are visible through one typed `SCE-ERR-RUNTIME` diagnostic beginning `Automatic synchronization failed:`. Non-zero child exits do not cause a second parent diagnostic. Authentication directs the user through `sce auth login` and manual `sce sync`, while other kinds include their preserved reason and manual retry guidance. See [Agent Trace hook routing](sce/agent-trace-hooks-command-routing.md) and [the captured-stderr decision](decisions/2026-09-01-parent-owned-automatic-sync-stderr.md). - `local Turso adapter`: Module in `cli/src/services/local_db/mod.rs` that defines `LocalDbSpec` and exposes `LocalDb` as a `TursoDb` alias. It resolves the canonical local DB path with `local_db_path()`, currently declares zero migrations, and inherits retry-backed `new()`, `execute()`, `query()`, and `query_map()` behavior from the shared generic adapter. - `encrypted Turso adapter`: Generic adapter seam in `cli/src/services/db/mod.rs` exposed as `EncryptedTursoDb`, structurally parallel to `TursoDb` (connection, tokio runtime bridge, spec typing). Its constructor resolves the encryption key via `encryption_key::get_or_create_encryption_key(&db_path, db_name)`, which derives a Turso-compatible 64-character hex key from non-empty `SCE_AUTH_DB_ENCRYPTION_KEY` env-secret text before falling back to OS credential-store keyring get-or-create behavior; credential-store default registration is guarded by stable `OnceLock` plus an atomic in-progress flag so errors or panics leave initialization retryable without mutex poisoning. The adapter enables Turso local encryption with strict `aegis256` cipher selection through `turso::EncryptionOpts`, wraps encrypted local open/connect in the default DB connection-open retry policy, and runs embedded migrations after retry has produced a connection; the adapter also exposes retry-backed synchronous `execute`, `query`, `query_map`, and `run_migrations` helpers with `__sce_migrations` tracking parity. - `auth DB adapter`: Module in `cli/src/services/auth_db/mod.rs` that defines `AuthDbSpec` and exposes `AuthDb` as an `EncryptedTursoDb` alias. It resolves the canonical `/sce/auth.db` path with `auth_db_path()`, keeps encryption mandatory with `SCE_AUTH_DB_ENCRYPTION_KEY` env-secret precedence before OS keyring fallback and no plaintext mode, and embeds ordered auth migrations where baseline SQL creates `auth_credentials` without `user_id`, with `updated_at`, and a trigger that auto-refreshes `updated_at` on row updates. Auth runtime token-storage is now wired through `cli/src/services/token_storage.rs`, which persists tokens via the `auth_credentials` table in the encrypted auth DB instead of a JSON file. diff --git a/context/overview.md b/context/overview.md index 01288d8d6..8edf1ab62 100644 --- a/context/overview.md +++ b/context/overview.md @@ -10,10 +10,10 @@ The generated `/next-task` workflow persists task-level context-synchronization - **Exit codes:** `2` parse, `3` validation, `4` runtime, `5` dependency failure (see `context/sce/cli-exit-code-contract.md`). - **Stderr diagnostics:** stable `SCE-ERR-{PARSE,VALIDATION,RUNTIME,DEPENDENCY}` codes with class-default `Try:` remediation (see `context/sce/cli-error-code-taxonomy.md`). -- **Stdout/stderr:** command payloads on stdout only; redacted diagnostics and text-mode `sce sync` progress on stderr, while automatic JSON sync keeps stdout silent and inherits stderr for typed failures. The synchronous automatic boundary keeps child completion and stderr closure deterministic at the cost of adding the child duration to post-commit latency (see `context/sce/cli-stdout-stderr-contract.md`). +- **Stdout/stderr:** command payloads on stdout only; redacted diagnostics and text-mode `sce sync` progress on stderr, while automatic JSON sync keeps stdout silent and captures child stderr for parent forwarding after completion. The synchronous automatic boundary keeps child completion and pipe draining deterministic at the cost of adding the child duration to post-commit latency (see `context/sce/cli-stdout-stderr-contract.md`). - **Observability:** config-resolved logging with tracing, explicit config-file/default `log_to_file` control, error-specific stderr suppression when file logging is enabled, and optional dated/session-partitioned `log_dir` / `SCE_LOG_DIR` files with retention (see `context/sce/cli-observability-contract.md`). - **Config precedence:** `flags > env > config file > defaults` (see `context/cli/config-precedence-contract.md`); the config-file-only `agent_trace.auto_sync` setting defaults to `true` and is resolved with source metadata for the post-commit trigger and doctor readiness boundaries. Its synchronous one-shot post-commit completion behavior, deterministic stream closure, and intentional commit-latency trade-off are documented in `context/cli/agent-trace-auto-sync.md`. @@ -76,7 +76,7 @@ The current supported automated release target matrix is `x86_64-unknown-linux-m The hooks service now uses a minimal attribution-only runtime: `commit-msg` is the only hook that mutates behavior, conditionally injecting exactly one canonical SCE trailer when the attribution-hooks gate is enabled, `SCE_DISABLED` is false, and the staged-diff AI-overlap preflight confirms AI/editor evidence (`StagedDiffAiOverlapResult::Overlap`); when the preflight returns `NoOverlap` or `Error` (including DB open failure, schema not ready, query error, staged diff read failure, or zero overlap), the trailer is not appended and errors are logged via `sce.hooks.commit_msg.ai_overlap_error`; `pre-commit` and `post-rewrite` remain deterministic no-op entrypoints; `post-commit` requires validated `--remote-url`, threads that URL through the Agent Trace flow, prints it to stderr, captures current commit patch, queries recent `diff_traces` from past 7 days (dispatching `patch` rows through existing unified-diff parsing and `structured` rows through `structured_patch::derive_claude_structured_patch` at read time, then assigning each structured hunk the persisted row model and every structured touched line the persisted canonical `cc_...` row session), combines/intersects patches, persists intersection metadata to `post_commit_patch_intersections`, and persists the schema-validated built Agent Trace payload, including optional top-level `tool` metadata from recent diff-trace rows, top-level `metadata.sce.version` from the compiled `sce` CLI package version, always-emitted `metadata.sce.line_changes` touched-line attribution counts, and range-level `content_hash` values, to AgentTraceDb `agent_traces` (DB-only, no post-commit Agent Trace file artifact); `diff-trace` currently validates/persists required non-empty `sessionID`/`diff`/`tool_name`, optional `model_id` (absent or `null` → `None`, present+non-empty → `Some`, present+empty → error), required nullable/non-empty `tool_version`, plus required `u64` millisecond `time`, uses direct-first/event-transcript-second Claude `model_id` resolution plus direct `tool_version` without any `session_models` runtime, and continues with `None` when event-local lookup cannot resolve attribution, with same-tool-idempotent stored `session_id` prefixing (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi), non-lossy AgentTraceDb `time_ms` conversion, and no parsed-payload artifact fallback. Claude structured `PostToolUse` diff-trace intake prefers direct top-level or nested model metadata, then matches the event's `tool_use_id` in its JSONL `transcript_path`, normalizes either source once with the `claude/` prefix, and fails open to `None`. The CLI now also includes an approved operator-environment doctor contract documented in `context/sce/agent-trace-hook-doctor.md`; the runtime includes `sce doctor --fix` parsing/help, stable problem/fix-result reporting, canonical hook-repair reuse, bounded doctor-owned local-DB directory bootstrap for the missing SCE-owned DB parent path, and target-scoped integration inventory: Claude reports `Plugins`, `Commands`, and `Skills`; OpenCode reports `Plugins`, `Agents`, `Commands`, and `Skills`; Pi reports `Extensions`, `Prompts`, and `Skills`; and Codex reports `Skills` and `Hooks` (see `context/sce/doctor-human-text-contract.md`). Its non-launching post-commit Agent Trace auto-sync fact reports enabled/current, explicit disabled, not-ready, and not-applicable states using canonical managed-block currency and resolved configuration; existing hook remediation and readiness semantics remain unchanged. The local DB service now provides `LocalDb` as a thin `TursoDb` alias in `cli/src/services/local_db/mod.rs`; `LocalDbSpec` resolves the canonical local DB path from the shared default-path catalog and currently declares zero migrations. Shared Turso infrastructure lives in `cli/src/services/db/mod.rs`, where `DbSpec` and generic `TursoDb` support local or remote sync-mode opens, parent-directory creation, connection setup, synchronous query helpers, embedded migration execution, and shared DB lifecycle helpers. Auth DB persistence uses encrypted `AuthDb = EncryptedTursoDb` and token storage persists credentials through the `auth_credentials` table. Agent Trace persistence uses the sole `RepositoryAgentTraceDb = TursoDb` adapter at `/sce/repos//agent-trace.db`, with a one-file repository schema for `repository_metadata`, `diff_traces`, `post_commit_patch_intersections`, `agent_traces`, `messages`, `parts`, indexes/triggers, and no `checkout_id` columns on trace rows. The checkout-scoped `AgentTraceDb = TursoDb` adapter, its `agent_trace_db_path()`/`agent_trace_db_path_for_checkout()` helpers, and the 15-file `cli/migrations/agent-trace/` chain were removed by the `retire-legacy-agent-trace-db` plan; active hook runtime writes nullable event-local diff-trace attribution without a `session_models` API/table dependency. - The hooks command surface now also supports concrete runtime subcommand routing (`pre-commit`, `commit-msg`, `post-commit`, `post-rewrite`, `diff-trace`, and `conversation-trace`) with deterministic argument/STDIN validation; `session-model` is no longer supported. Current runtime behavior keeps commit-msg attribution enabled by default unless explicitly opted out: the attribution gate enables canonical trailer insertion in `commit-msg` only when the staged-diff AI-overlap preflight confirms AI/editor evidence (no trailer is appended when the preflight finds no overlap or encounters any error); `pre-commit`/`post-rewrite` remain deterministic no-ops, `post-commit` requires validated `--remote-url`, threads that URL into the Agent Trace flow, prints it to stderr, remains the active bounded recent-diff-trace intersection path, and after successful Agent Trace persistence optionally launches the detached sync-owned `sync --format json` child when config-file-only `agent_trace.auto_sync` is true; `diff-trace` is the active intake path for parsed STDIN `{ sessionID, diff, time, model_id?, tool_name, tool_version }` payload persistence with optional `model_id`, required non-empty `tool_name`, required nullable/non-empty `tool_version`, direct-first/event-transcript-second Claude `model_id` plus direct `tool_version` values (no session-model fallback or cache), required `u64` millisecond `time`, same-tool-idempotent stored `session_id` prefixing (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi), non-lossy AgentTraceDb `time_ms` conversion, and no parsed-payload artifact fallback. This behavior is documented in `context/sce/agent-trace-hooks-command-routing.md`. The removed `sce hooks claude-capture` raw capture route is documented in `context/sce/claude-raw-hook-capture.md` as a removed feature. + The hooks command surface now also supports concrete runtime subcommand routing (`pre-commit`, `commit-msg`, `post-commit`, `post-rewrite`, `diff-trace`, and `conversation-trace`) with deterministic argument/STDIN validation; `session-model` is no longer supported. Current runtime behavior keeps commit-msg attribution enabled by default unless explicitly opted out: the attribution gate enables canonical trailer insertion in `commit-msg` only when the staged-diff AI-overlap preflight confirms AI/editor evidence (no trailer is appended when the preflight finds no overlap or encounters any error); `pre-commit`/`post-rewrite` remain deterministic no-ops, `post-commit` requires validated `--remote-url`, threads that URL into the Agent Trace flow, prints it to stderr, remains the active bounded recent-diff-trace intersection path, and after successful Agent Trace persistence optionally launches the one-shot sync-owned `sync --format json` child when config-file-only `agent_trace.auto_sync` is true, waiting for terminal completion while draining piped stderr and forwarding captured bytes through the parent stderr path; `diff-trace` is the active intake path for parsed STDIN `{ sessionID, diff, time, model_id?, tool_name, tool_version }` payload persistence with optional `model_id`, required non-empty `tool_name`, required nullable/non-empty `tool_version`, direct-first/event-transcript-second Claude `model_id` plus direct `tool_version` values (no session-model fallback or cache), required `u64` millisecond `time`, same-tool-idempotent stored `session_id` prefixing (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi), non-lossy AgentTraceDb `time_ms` conversion, and no parsed-payload artifact fallback. This behavior is documented in `context/sce/agent-trace-hooks-command-routing.md`. The removed `sce hooks claude-capture` raw capture route is documented in `context/sce/claude-raw-hook-capture.md` as a removed feature. The setup service now also exposes deterministic required-hook embedded asset accessors (`iter_required_hook_assets`, `get_required_hook_asset`) backed by canonical templates in `cli/assets/hooks/` for `pre-commit`, `commit-msg`, and `post-commit`; this behavior is documented in `context/sce/setup-githooks-hook-asset-packaging.md`. The setup service now also includes required-hook install orchestration (`install_required_git_hooks`) that resolves repository root and effective hooks path from git truth, computes the bytes to stage by merging the canonical hook template with any existing hook (preserving a foreign hook's content as an exact prefix with the SCE managed block appended, or bringing an SCE-owned block current in place) rather than writing canonical bytes verbatim, enforces deterministic per-hook outcomes (`Installed`/`Updated`/`Skipped`) against that merged content, surfaces a deterministic advisory when an appended block would be unreachable, and uses a unified atomic-swap policy that renames staged content directly over existing hooks without unlinking them first, with deterministic recovery guidance on swap failures; this behavior is documented in `context/sce/setup-githooks-install-flow.md`. The setup command parser/dispatch now also supports composable setup+hooks runs (`sce setup --opencode|--claude|--pi|--codex|--all --hooks`) plus hooks-only mode (`sce setup --hooks` with optional `--repo `), enforces deterministic compatibility validation (`--repo` requires `--hooks`; target flags remain mutually exclusive), and emits deterministic setup/hook outcome messaging (`installed`/`updated`/`skipped`); this behavior is documented in `context/sce/setup-githooks-cli-ux.md`. diff --git a/context/patterns.md b/context/patterns.md index fea2801e5..ad108d383 100644 --- a/context/patterns.md +++ b/context/patterns.md @@ -128,7 +128,7 @@ - Keep `log_file_retention_limit` flat and config-file/default only: validate it as an integer with minimum `1`, merge global before local, default it to `10`, expose resolved source metadata without adding an environment variable or CLI flag, and pass the resolved value unchanged to primary and v2 creation-triggered logger cleanup. - For runtime CLI configuration, keep precedence deterministic and explicit (`flags > env > config file > defaults`) and expose inspect/validate command entrypoints with stable text/JSON outputs. Config-file-only Agent Trace runtime switches such as `agent_trace.auto_sync` should document their default and explicit opt-out, resolve global before local, and expose winning source metadata without adding an environment variable or CLI flag unless the contract explicitly requires one. -- For default-enabled automatic Agent Trace synchronization, keep the post-commit launcher as a one-shot reuse of `sce sync`: start the current executable after local Agent Trace persistence, use the repository root, null stdin/stdout, inherited stderr, and an internal automatic-invocation marker, wait for terminal child completion so child lifetime and stderr closure are deterministic, and accept the resulting child/network latency at the post-commit boundary. Fail open on child, startup, or wait errors. Ignore a non-zero child exit after the child renders its one typed diagnostic; render launcher failures through the same typed `SCE-ERR-RUNTIME` diagnostic beginning `Automatic synchronization failed:`. Authentication must direct the user to `sce auth login` and then manual `sce sync`, while other kinds include their preserved reason and actionable manual retry guidance without duplicating generic runtime `Try:` text. Keep manual sync and the control-plane cursor authority as the retry path; do not add a timeout, daemon, watcher, polling, queue, or high-frequency hook trigger. +- For default-enabled automatic Agent Trace synchronization, keep the post-commit launcher as a one-shot reuse of `sce sync`: start the current executable after local Agent Trace persistence, use the repository root, null stdin/stdout, piped stderr, and an internal automatic-invocation marker, wait with concurrent pipe draining to terminal child completion, and forward captured bytes through the parent stderr path. Accept the resulting child/network latency at the post-commit boundary. Fail open on child, startup, or wait errors. Ignore a non-zero child exit after the child renders its one typed diagnostic; render launcher failures through the same typed `SCE-ERR-RUNTIME` diagnostic beginning `Automatic synchronization failed:`. Authentication must direct the user to `sce auth login` and then manual `sce sync`, while other kinds include their preserved reason and actionable manual retry guidance without duplicating generic runtime `Try:` text. Keep manual sync and the control-plane cursor authority as the retry path; do not add a timeout, daemon, watcher, polling, queue, or high-frequency hook trigger. - For commands that support text/JSON dual output, centralize `--format ` parsing in one shared contract and pass command-specific `--help` guidance into invalid-value errors instead of duplicating parser logic per command. - For setup-style command contracts, keep interactive mode as the zero-flag default and enforce mutually-exclusive explicit target flags for non-interactive automation. - For setup config safety, validate an existing repo-local config immediately after resolving the Git root and before prompts, context bootstrap, lifecycle providers, hooks, or target assets; preserve create-if-missing behavior for absent config and keep general startup degradation scoped away from setup and Agent Trace storage identity resolution. diff --git a/context/plans/auto-sync-captured-stderr.md b/context/plans/auto-sync-captured-stderr.md new file mode 100644 index 000000000..a586af96c --- /dev/null +++ b/context/plans/auto-sync-captured-stderr.md @@ -0,0 +1,135 @@ +# Plan: auto-sync-captured-stderr + +## Change summary + +Replace the automatic sync child’s inherited stderr with an explicitly piped and +captured stderr stream while retaining the already-approved synchronous wait at +the post-commit launcher boundary. The launcher will drain the child output while +waiting, then forward the captured bytes through the parent’s stderr so typed +automatic-sync failures remain visible without letting the child inherit or hold +the caller’s stderr descriptor. + +This is a focused follow-up to the synchronous launcher implementation. It keeps +the existing command, repository-root working directory, null stdin/stdout, +automatic-invocation marker, fail-open behavior, and no-daemon/no-retry boundary; +only child stderr ownership and the corresponding documentation change. + +## Acceptance criteria + +How this plan is proven complete. Each criterion is observable and names the +check that proves it. `/validate` runs these checks; no task in the stack +performs final validation. + +- [x] AC1: The automatic launcher waits for a child whose stderr is piped rather than inherited, drains that pipe without deadlocking, and never leaves the child holding the post-commit caller’s stderr descriptor. + - Validate: Focused `auto_sync` tests and inspection of `spawn_command`/child-wait code assert `Stdio::piped()` plus `wait_with_output` (or equivalent concurrent drain), with no `Stdio::inherit()` on the automatic child. +- [x] AC2: Captured child stderr is forwarded to the parent stderr after completion, preserving automatic failure visibility while successful `sync --format json` remains stdout-silent; non-zero child exits, startup failures, and wait failures remain fail-open without duplicate launcher diagnostics. + - Validate: Focused launcher and post-commit hook tests assert captured stderr forwarding, command/stream configuration, fail-open outcomes, and the existing single typed diagnostic behavior. +- [x] AC3: Durable context consistently describes synchronous automatic synchronization with parent-owned forwarding of captured child stderr, without stale inherited-stderr claims or changed manual-sync/no-daemon semantics. + - Validate: Review the listed context contracts against the final launcher and run `git diff --check`. + +### Full validation + +Repository-wide checks `/validate` runs after the last task, regardless of +which criterion they map to. + +- `nix flake check` +- `nix run .#pkl-check-generated` + +### Context sync + +- `context/overview.md` +- `context/architecture.md` +- `context/context-map.md` +- `context/glossary.md` +- `context/patterns.md` +- `context/cli/agent-trace-auto-sync.md` +- `context/cli/sync-command.md` +- `context/cli/agent-trace-sync-command.md` +- `context/sce/cli-stdout-stderr-contract.md` +- `context/sce/agent-trace-hooks-command-routing.md` +- A new dated decision record documenting the captured-stderr transport and its relationship to `2026-08-31-synchronous-automatic-sync-completion.md`. + +## Task context synchronization lifecycle + +Persist this field in every plan; this is durable plan state, not chat state: + +- **Task context synchronization:** every task carries `pending | synced | blocked`. + A completed task must be `synced` before another task can start or the plan can + finish. +- For `blocked`, record **Blocker**, **Required action**, and **Retry condition** + beside the status. Never infer `synced` from conversation history; write every + lifecycle transition to the plan file. + +## Constraints and non-goals + +- **In scope:** `cli/src/services/sync/auto_sync.rs`, focused automatic-sync and post-commit tests, and the durable context contracts that describe automatic child process and stderr behavior. +- **Out of scope:** manual `sce sync` stream routing, control-plane protocol, sync payloads, Agent Trace storage, hook trigger frequency, automatic-sync error taxonomy, generated configuration, and the accepted decision that the launcher waits for terminal completion. +- **Constraints:** preserve `sync --format json`, repository-root cwd, null stdin/stdout, `SCE_INTERNAL_AUTO_SYNC=1`, fail-open child/startup/wait behavior, typed diagnostic ownership, bounded resource use, and repository Nix verification conventions. Drain piped stderr while waiting rather than calling a blocking wait that can deadlock on a full pipe. +- **Non-goal:** suppressing automatic diagnostics, adding a timeout/retry/queue/daemon, or redesigning the application-wide stdout/stderr contract. + +## Assumptions + +- The review comment’s “inherited stderr” objection means the child must not inherit the caller’s file descriptor; forwarding captured bytes from the synchronous parent preserves the existing operator-visible diagnostics without retaining that descriptor. +- The already-implemented synchronous completion policy remains selected; only the stderr transport is being corrected. +- Existing local Rust seams may be extended for captured `std::process::Output` and test doubles without adding a dependency, following the repository’s focused service/test pattern. + +## Task stack + +- [x] T01: `Capture and forward automatic-sync child stderr` (status:done) + - Task ID: T01 + - Scope: In — automatic launcher process configuration and child-wait seam in `cli/src/services/sync/auto_sync.rs`; focused tests for piped stderr, concurrent draining/capture, forwarding, command preservation, and fail-open success/non-zero/startup/wait outcomes. Out — manual sync behavior, hook trigger policy, sync protocol, and durable context edits. + - Dependencies: none + - Done when: the launcher uses a non-inherited stderr pipe, drains it as part of terminal child completion, forwards captured bytes through the parent stderr path, and preserves all existing automatic-sync command, diagnostic, and fail-open contracts; focused tests pass. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auto_sync`; `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml hooks::` + - Context synchronization: synced + - Completed: 2026-09-01 + - Files changed: `cli/src/services/sync/auto_sync.rs` + - Result: Replaced inherited automatic-sync stderr with `Stdio::piped()`, switched the child seam to `wait_with_output()` so stderr is drained without deadlock, and forwarded captured bytes through the parent stderr path while preserving non-zero child, startup, and wait fail-open behavior. Added focused capture, non-zero-exit, wait-failure, and command-configuration coverage. + - Evidence: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auto_sync` passed (16 tests); `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml hooks::` passed (163 tests); `nix develop -c sh -c 'cd cli && cargo fmt -- --check'` passed; `git diff --check` passed. + - Context impact: material — automatic synchronization process ownership and stderr forwarding behavior changed; T02 must update the listed durable context contracts. + +- [x] T02: `Document parent-owned automatic-sync stderr forwarding` (status:complete) + - Task ID: T02 + - Scope: In — the listed root, CLI, stream, hook-routing, glossary, pattern, and new decision records. Out — historical plan/decision rewriting, generated target trees, and unrelated CLI stream documentation. + - Dependencies: T01 + - Done when: durable context states that automatic sync waits synchronously, the child stderr is piped/drained and forwarded by the parent, diagnostics remain visible and fail-open, and no stale inherited-stderr claim remains in current-state contracts. + - Verify: Manual code/context review against `cli/src/services/sync/auto_sync.rs`, `cli/src/services/hooks/mod.rs`, `cli/src/services/sync/command.rs`, and `cli/src/services/app_support.rs`; `git diff --check` + - Context synchronization: synced + - Completed: 2026-09-01 + - Files changed: `context/overview.md` + - Result: Corrected the root overview's stale asynchronous and detached automatic-sync descriptions; current-state contracts now consistently describe synchronous completion, piped/drained child stderr, parent forwarding, visible fail-open diagnostics, and preserved manual/no-daemon/no-retry boundaries. + - Verify: Manual code/context review — passed; `git diff --check` — passed. + - Context impact: material — current root and domain contracts describe cross-cutting automatic-sync process ownership and stderr transport; the mandatory context synchronization pass remains required. + +## Open questions + +None. The review supplies the required transport correction; capturing and +forwarding through the already-synchronous launcher is the smaller compatible +alternative to either inheriting stderr or suppressing automatic diagnostics. + +## Validation Report + +**Status:** validated +**Date:** 2026-09-01 + +### Commands run + +- `nix develop -c sh -c './scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auto_sync && ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml hooks::'` -> exit 0 (16 automatic-sync tests and 163 hook tests passed) +- `nix shell nixpkgs#ripgrep -c rg -n 'Stdio::inherit|Stdio::piped|wait_with_output|wait\\(' cli/src/services/sync/auto_sync.rs` -> exit 0 (automatic child uses piped stderr and `wait_with_output`; no inherited stderr) +- `git diff --check` -> exit 0 (no whitespace errors) +- `nix flake check` -> exit 0 (all flake checks passed) +- `nix run .#pkl-check-generated` -> exit 0 (ephemeral generated-config parity passed) + +### Success-criteria verification + +- [x] AC1: The automatic launcher waits for a child whose stderr is piped rather than inherited, drains that pipe without deadlocking, and never leaves the child holding the post-commit caller’s stderr descriptor. -> Source inspection confirmed `Stdio::piped()` and `wait_with_output()` with no `Stdio::inherit()`; focused automatic-sync tests passed. +- [x] AC2: Captured child stderr is forwarded to the parent stderr after completion, preserving automatic failure visibility while successful `sync --format json` remains stdout-silent; non-zero child exits, startup failures, and wait failures remain fail-open without duplicate launcher diagnostics. -> Focused automatic-sync and post-commit hook tests passed, covering captured forwarding, command configuration, successful/non-zero/startup/wait outcomes, and one-diagnostic behavior. +- [x] AC3: Durable context consistently describes synchronous automatic synchronization with parent-owned forwarding of captured child stderr, without stale inherited-stderr claims or changed manual-sync/no-daemon semantics. -> Reviewed the listed current-state context contracts against the launcher and `git diff --check` passed. + +### Failed checks and follow-ups + +- None. + +### Residual risks + +- None identified. diff --git a/context/sce/agent-trace-hooks-command-routing.md b/context/sce/agent-trace-hooks-command-routing.md index 4a48798cf..bdbcefcc3 100644 --- a/context/sce/agent-trace-hooks-command-routing.md +++ b/context/sce/agent-trace-hooks-command-routing.md @@ -63,7 +63,7 @@ - Post-commit Agent Trace success requires both schema validation and Agent Trace DB `agent_traces` persistence to succeed. - Current command-surface success output is: `post-commit hook processed intersection: commit=, intersection_files=`. - After Agent Trace validation and `agent_traces` persistence succeed, post-commit runs exactly one passive WAL checkpoint through `RepositoryAgentTraceDb::passive_checkpoint()` (see [shared-turso-db.md](shared-turso-db.md)) before resolving auto-sync. This is routine maintenance, not a durability boundary: a checkpoint failure is logged as a warning via `Logger::warn` with event `sce.agent_trace_db.passive_checkpoint_failed` and does not fail the hook or affect already-persisted Agent Trace data. `diff-trace` and `conversation-trace` do not checkpoint per write; only this one post-commit call site does. - - After Agent Trace validation and `agent_traces` persistence succeed, post-commit resolves the config-file-only `agent_trace.auto_sync` gate. When it is `true`, the hook invokes the sync-owned one-shot launcher exactly once with the repository root; the launcher starts the current `sce` executable with `sync --format json`, passes the internal `SCE_INTERNAL_AUTO_SYNC=1` marker, inherits stderr, and waits for terminal child completion. This makes child lifetime and inherited-stderr closure deterministic, intentionally adding the child synchronization duration to post-commit latency. The child keeps stdin/stdout null, so successful JSON execution is silent, while automatic failures render one `Error [SCE-ERR-RUNTIME]` diagnostic beginning `Automatic synchronization failed:` through inherited stderr. A non-zero child exit is ignored after the child renders that diagnostic, so the launcher does not duplicate it. Authentication tells the user to run `sce auth login` and then manually retry with `sce sync`; other typed failure kinds include their preserved reason and actionable manual retry guidance without a duplicate generic `Try:` sentence. Explicit `false` configuration does not launch; omitted configuration launches, and validation or persistence failure reaches the existing error path before the gate. Launcher/current-executable/spawn/wait failures use the typed automatic-sync runtime payload with their reasons on stderr but remain fail-open and do not change the successful post-commit result. No `pre-commit`, `diff-trace`, or `conversation-trace` path invokes automatic synchronization. + - After Agent Trace validation and `agent_traces` persistence succeed, post-commit resolves the config-file-only `agent_trace.auto_sync` gate. When it is `true`, the hook invokes the sync-owned one-shot launcher exactly once with the repository root; the launcher starts the current `sce` executable with `sync --format json`, passes the internal `SCE_INTERNAL_AUTO_SYNC=1` marker, pipes stderr, and waits for terminal child completion while draining it. The launcher then forwards captured bytes through the parent stderr path. This makes child lifetime and stderr transport deterministic, intentionally adding the child synchronization duration to post-commit latency. The child keeps stdin/stdout null, so successful JSON execution is silent, while automatic failures render one `Error [SCE-ERR-RUNTIME]` diagnostic beginning `Automatic synchronization failed:` through the parent forwarding path. A non-zero child exit is ignored after that diagnostic is forwarded, so the launcher does not duplicate it. Authentication tells the user to run `sce auth login` and then manually retry with `sce sync`; other typed failure kinds include their preserved reason and actionable manual retry guidance without a duplicate generic `Try:` sentence. Explicit `false` configuration does not launch; omitted configuration launches, and validation or persistence failure reaches the existing error path before the gate. Launcher/current-executable/spawn/wait failures use the typed automatic-sync runtime payload with their reasons on stderr but remain fail-open and do not change the successful post-commit result. No `pre-commit`, `diff-trace`, or `conversation-trace` path invokes automatic synchronization. - `post-rewrite` is a deterministic no-op entrypoint. - `diff-trace` reads STDIN JSON and classifies the payload: - **Claude structured payloads** (detected by presence of top-level `hook_event_name`): the STDIN JSON is validated through `derive_claude_structured_patch`. Supported `PostToolUse` `Write` create and `Edit` structured-patch events produce a `DiffTracePayload` with `payload_type="structured"` and the raw event JSON stored as the `diff` column without conversion to unified-diff text. Model attribution is resolved event-locally and direct-first: top-level `model`, `model_id`, or `modelId`, or nested `model.id`, `model.model`, or `model.name`, wins when present. Otherwise, when the event provides both `transcript_path` and `tool_use_id`, Rust scans that Claude JSONL transcript for the assistant-message envelope whose `tool_use.id` matches, skipping malformed unrelated records. Either source is normalized once with the `claude/` prefix. Missing/unreadable transcripts, unmatched tool calls, missing models, or absent lookup fields leave `model_id` nullable without rejecting the hook, and downstream Agent Trace JSON omits contributor `model_id`. No session-level cache or lookup participates. Unsupported Claude events (non-`PostToolUse`, unsupported tools, invalid payloads) produce a deterministic `NoOp` success result. diff --git a/context/sce/cli-stdout-stderr-contract.md b/context/sce/cli-stdout-stderr-contract.md index 1176e7d88..58b954704 100644 --- a/context/sce/cli-stdout-stderr-contract.md +++ b/context/sce/cli-stdout-stderr-contract.md @@ -11,8 +11,8 @@ This document defines the implemented stream contract for CLI command payload an - Failure diagnostics are emitted as `Error []: ...` on `stderr`, where `` is the stable class-based `SCE-ERR-*` identifier from `CliError` in `cli/src/services/error.rs`; diagnostics are passed through shared redaction (`services::security::redact_sensitive_text`) before emission. - The diagnostic body differs by `CliError` variant: `CliError::Internal` renders the real `anyhow` source chain (`format!("{source:#}")`) plus class-default `Try:` remediation; `CliError::User` renders its catalog `UserError` template, with non-authentication automatic-sync payload reasons included only in that reviewed message and no technical source chain or `Try:` suffix. Both bodies are styled through the same stderr TTY/`NO_COLOR` policy before redaction and emission. - Command handlers now return payload strings to the app dispatcher; the app owns stream selection and final emission. -- The post-commit `sce sync --format json` child keeps stdout null so a successful automatic run produces no hook payload, while inheriting stderr so its single app-rendered typed failure diagnostic remains observable. The launcher waits for terminal child completion, making child lifetime and inherited-stderr closure deterministic while adding child synchronization latency to post-commit execution. It ignores a non-zero child exit after that diagnostic and renders executable/spawn/wait failures through the same stderr diagnostic writer in the parent; all remain fail-open. -- Automatic child failures render exactly one `Error [SCE-ERR-RUNTIME]: Automatic synchronization failed: ...` diagnostic on inherited stderr. Authentication exposes login-plus-manual-sync recovery while its technical reason stays in observability; non-authentication reasons and recovery guidance are included in that one diagnostic, with no generic duplicate `Try:` suffix. +- The post-commit `sce sync --format json` child keeps stdout null so a successful automatic run produces no hook payload, while piping stderr so its single app-rendered typed failure diagnostic can be captured and forwarded by the parent. The launcher drains stderr while waiting for terminal child completion, making child lifetime and stderr transport deterministic while adding child synchronization latency to post-commit execution. It ignores a non-zero child exit after forwarding that diagnostic and renders executable/spawn/wait failures through the same stderr diagnostic writer in the parent; all remain fail-open. +- Automatic child failures render exactly one `Error [SCE-ERR-RUNTIME]: Automatic synchronization failed: ...` diagnostic, captured from the child and forwarded through the parent's stderr. Authentication exposes login-plus-manual-sync recovery while its technical reason stays in observability; non-authentication reasons and recovery guidance are included in that one diagnostic, with no generic duplicate `Try:` suffix. ## Implementation surface