From 4407d32d63a5b78126652bf86839a4ade73f2eb6 Mon Sep 17 00:00:00 2001 From: stefanskoricdev Date: Thu, 20 Aug 2026 15:05:39 +0200 Subject: [PATCH 1/8] runtime: Classify sync failures with typed user-error catalog Expose credential-storage and unexpected sync failures through the closed user-error catalog, preserving technical sources for observability while emitting redacted, unstyled terminal messages. Add typed storage predicates across sync error layers. Co-authored-by: SCE --- .../agent_trace_sync/control_plane.rs | 6 +++ cli/src/services/agent_trace_sync/mod.rs | 11 +++++ cli/src/services/app_support.rs | 41 ++++++++-------- cli/src/services/error.rs | 43 +++++++++++++++- cli/src/services/sync/command.rs | 49 +++++++++++++++---- cli/src/services/sync/sync.rs | 9 ++++ context/architecture.md | 2 + context/cli/agent-trace-sync-command.md | 2 +- context/cli/styling-service.md | 15 +++--- context/cli/sync-command.md | 32 +++++++----- context/context-map.md | 1 + context/glossary.md | 7 +-- context/overview.md | 2 +- context/sce/cli-error-code-taxonomy.md | 11 +++-- context/sce/cli-stdout-stderr-contract.md | 4 +- 15 files changed, 172 insertions(+), 63 deletions(-) diff --git a/cli/src/services/agent_trace_sync/control_plane.rs b/cli/src/services/agent_trace_sync/control_plane.rs index ab994d82..11b5a163 100644 --- a/cli/src/services/agent_trace_sync/control_plane.rs +++ b/cli/src/services/agent_trace_sync/control_plane.rs @@ -184,6 +184,12 @@ impl ControlPlaneError { Self::MissingCredentials | Self::AuthenticationFailed(_) ) } + + /// True when the failure came from loading or saving local authentication + /// credentials, rather than from the control-plane request itself. + pub fn is_storage_failure(&self) -> bool { + matches!(self, Self::Storage(_)) + } } impl From for ControlPlaneError { diff --git a/cli/src/services/agent_trace_sync/mod.rs b/cli/src/services/agent_trace_sync/mod.rs index 7be8c685..6f51fcdf 100644 --- a/cli/src/services/agent_trace_sync/mod.rs +++ b/cli/src/services/agent_trace_sync/mod.rs @@ -125,6 +125,17 @@ impl StreamSyncError { Self::Read(_) | Self::InvalidResponse(_) | Self::DidNotConverge => false, } } + + /// True only when the underlying `ControlPlaneError` (from a `Refresh` + /// or `Terminal` failure) means local credential storage is unavailable. + /// `Read`, `InvalidResponse`, and `DidNotConverge` never carry a + /// `ControlPlaneError` and are never storage failures. + pub fn is_storage_failure(&self) -> bool { + match self { + Self::Refresh(error) | Self::Terminal(error) => error.is_storage_failure(), + Self::Read(_) | Self::InvalidResponse(_) | Self::DidNotConverge => false, + } + } } /// Outcome of a fully converged [`sync_stream`] run for one stream. diff --git a/cli/src/services/app_support.rs b/cli/src/services/app_support.rs index 14671d7e..418aa986 100644 --- a/cli/src/services/app_support.rs +++ b/cli/src/services/app_support.rs @@ -184,7 +184,12 @@ fn write_error_diagnostic_with_color_policy( } CliError::User { error: user_error, .. - } => user_error.message().to_string(), + } => { + let message = services::security::redact_sensitive_text(user_error.message()); + writeln!(writer, "{message}") + .expect("writing user error diagnostic to writer should not fail"); + return; + } }; let styled_message = services::style::error_text_with_color_policy( &services::security::redact_sensitive_text(&rendered), @@ -263,11 +268,12 @@ mod tests { let stderr_text = String::from_utf8(stderr).expect("stderr is valid utf8"); assert_eq!( - diagnostic_lines(&stderr_text).len(), - 1, - "exactly one terminal diagnostic must be written" + stderr_text, + "You are not logged in. Please log in using the `sce auth login` command.\n" ); - assert!(stderr_text.contains("You are not logged in")); + assert!(!stderr_text.contains("Error")); + assert!(!stderr_text.contains("SCE-ERR-")); + assert!(!stderr_text.contains("Try:")); assert!(!stderr_text.contains("missing credentials")); assert!(!stderr_text.to_lowercase().contains("control-plane")); } @@ -333,23 +339,16 @@ mod tests { } #[test] - fn user_error_diagnostic_is_styled_only_when_color_is_enabled() { + fn user_error_diagnostic_is_plain_in_every_color_policy_mode() { let error = CliError::user(UserError::NotAuthenticated); + let expected = "You are not logged in. Please log in using the `sce auth login` command.\n"; + + for color_enabled in [true, false] { + let mut stderr = Vec::new(); + write_error_diagnostic_with_color_policy(&mut stderr, &error, color_enabled); + let rendered = String::from_utf8(stderr).expect("stderr is valid utf8"); - let mut colored = Vec::new(); - write_error_diagnostic_with_color_policy(&mut colored, &error, true); - let colored_text = String::from_utf8(colored).expect("stderr is valid utf8"); - - let mut plain = Vec::new(); - write_error_diagnostic_with_color_policy(&mut plain, &error, false); - let plain_text = String::from_utf8(plain).expect("stderr is valid utf8"); - - // TTY-following (color_enabled: true) and redirected/NO_COLOR - // (color_enabled: false) diverge: only the enabled case carries ANSI. - assert_ne!(colored_text, plain_text); - assert!(!plain_text.contains('\u{1b}')); - assert!(colored_text.contains('\u{1b}')); - assert!(plain_text.contains("You are not logged in")); - assert!(colored_text.contains("You are not logged in")); + assert_eq!(rendered, expected); + } } } diff --git a/cli/src/services/error.rs b/cli/src/services/error.rs index 5e2b487c..f83d68ef 100644 --- a/cli/src/services/error.rs +++ b/cli/src/services/error.rs @@ -54,12 +54,17 @@ impl FailureClass { pub enum UserError { #[allow(dead_code)] NotAuthenticated, + AuthStorageUnavailable, + #[allow(dead_code)] + UnexpectedFailure, } impl UserError { pub fn class(self) -> FailureClass { match self { - Self::NotAuthenticated => FailureClass::Runtime, + Self::NotAuthenticated | Self::AuthStorageUnavailable | Self::UnexpectedFailure => { + FailureClass::Runtime + } } } @@ -67,6 +72,8 @@ impl UserError { pub fn key(self) -> &'static str { match self { Self::NotAuthenticated => "auth.not_authenticated", + Self::AuthStorageUnavailable => "auth.storage_unavailable", + Self::UnexpectedFailure => "general.unexpected_failure", } } @@ -75,6 +82,12 @@ impl UserError { Self::NotAuthenticated => { "You are not logged in. Please log in using the `sce auth login` command." } + Self::AuthStorageUnavailable => { + "Authentication storage is unavailable. Verify local credential storage is available, then retry the command." + } + Self::UnexpectedFailure => { + "An unexpected error occurred. Check the log files for more details." + } } } } @@ -190,6 +203,34 @@ mod tests { assert!(error.to_string().contains("You are not logged in")); } + #[test] + fn unexpected_failure_has_stable_runtime_catalog_mapping() { + let error = CliError::user(UserError::UnexpectedFailure); + + assert_eq!(error.class(), FailureClass::Runtime); + assert_eq!(error.code(), "SCE-ERR-RUNTIME"); + assert_eq!( + UserError::UnexpectedFailure.key(), + "general.unexpected_failure" + ); + assert_eq!( + UserError::UnexpectedFailure.message(), + "An unexpected error occurred. Check the log files for more details." + ); + assert_eq!( + error.to_string(), + "An unexpected error occurred. Check the log files for more details." + ); + } + + #[test] + fn unexpected_failure_has_one_static_safe_message() { + assert_eq!( + UserError::UnexpectedFailure.message(), + "An unexpected error occurred. Check the log files for more details." + ); + } + #[test] fn user_with_source_preserves_technical_source() { let error = CliError::user_with_source( diff --git a/cli/src/services/sync/command.rs b/cli/src/services/sync/command.rs index 6120bbe6..d007a89c 100644 --- a/cli/src/services/sync/command.rs +++ b/cli/src/services/sync/command.rs @@ -33,10 +33,12 @@ where #[allow(clippy::needless_pass_by_value)] fn classify_sync_error(err: TraceSyncError) -> CliError { - if err.is_authentication_failure() { + if err.is_storage_failure() { + CliError::user_with_source(UserError::AuthStorageUnavailable, err) + } else if err.is_authentication_failure() { CliError::user_with_source(UserError::NotAuthenticated, err) } else { - CliError::runtime(err) + CliError::user_with_source(UserError::UnexpectedFailure, err) } } @@ -112,10 +114,13 @@ mod tests { } } - fn assert_internal(err: TraceSyncError) { + fn assert_user_error(err: TraceSyncError, expected_key: &str) { match classify_sync_error(err) { - CliError::Internal { .. } => {} - other @ CliError::User { .. } => panic!("expected CliError::Internal, got {other:?}"), + CliError::User { error, source } => { + assert_eq!(error.key(), expected_key); + assert!(source.is_some()); + } + other @ CliError::Internal { .. } => panic!("expected CliError::User, got {other:?}"), } } @@ -152,25 +157,49 @@ mod tests { } #[test] - fn other_control_plane_errors_classify_as_internal() { + fn other_control_plane_errors_classify_as_unexpected_failure() { for error in [ ControlPlaneError::Forbidden("nope".to_string()), ControlPlaneError::BadRequest("bad".to_string()), ControlPlaneError::Transport("down".to_string()), ControlPlaneError::ServerError("500".to_string()), ControlPlaneError::InvalidResponse("garbage".to_string()), - ControlPlaneError::Storage("disk".to_string()), ControlPlaneError::Protocol { status: reqwest::StatusCode::NOT_FOUND, message: "route removed".to_string(), }, ] { - assert_internal(TraceSyncError::ControlPlane(error)); + assert_user_error( + TraceSyncError::ControlPlane(error), + "general.unexpected_failure", + ); } } #[test] - fn runtime_failure_classifies_as_internal() { - assert_internal(TraceSyncError::Runtime("local failure".to_string())); + fn credential_storage_failure_classifies_as_storage_unavailable() { + assert_user_error( + TraceSyncError::ControlPlane(ControlPlaneError::Storage("disk".to_string())), + "auth.storage_unavailable", + ); + } + + #[test] + fn runtime_failure_classifies_as_unexpected_failure() { + assert_user_error( + TraceSyncError::Runtime("local failure".to_string()), + "general.unexpected_failure", + ); + } + + #[test] + fn stream_storage_failure_does_not_classify_as_storage_unavailable() { + assert_user_error( + TraceSyncError::Stream { + stream: "prompts", + source: StreamSyncError::Terminal(ControlPlaneError::Storage("disk".to_string())), + }, + "general.unexpected_failure", + ); } } diff --git a/cli/src/services/sync/sync.rs b/cli/src/services/sync/sync.rs index 32f1b35d..c5930539 100644 --- a/cli/src/services/sync/sync.rs +++ b/cli/src/services/sync/sync.rs @@ -146,6 +146,15 @@ impl TraceSyncError { Self::Stream { source, .. } => source.is_authentication_failure(), } } + + /// True when the initial control-plane failure came from local credential + /// storage. Stream failures never carry storage errors. + pub fn is_storage_failure(&self) -> bool { + match self { + Self::ControlPlane(error) => error.is_storage_failure(), + Self::Runtime(_) | Self::Stream { .. } => false, + } + } } /// Resolves the current repository's Agent Trace storage (the same diff --git a/context/architecture.md b/context/architecture.md index ccfdb91c..f0d7ce8b 100644 --- a/context/architecture.md +++ b/context/architecture.md @@ -154,6 +154,8 @@ The repository includes a new placeholder Rust binary crate at `cli/`. - `cli/Cargo.toml` keeps crates.io publication-ready package metadata for the `shared-context-engineering` crate, and `cli/README.md` is the Cargo install surface for crates.io (`cargo install shared-context-engineering --locked`) and local checkout (`./scripts/run-cli-cargo.sh install --path cli --locked`) guidance. Direct `cargo install --git` is unsupported because it cannot invoke the repository's pre-Cargo producer. The published crate installs the `sce` binary. Tokio remains intentionally constrained (`default-features = false`) with current-thread runtime usage plus timer-backed bounded resilience wrappers for retry/timeout behavior. - `cli/Cargo.toml` now declares Tokio's `time` feature directly alongside the existing constrained current-thread runtime setup (`rt`, `io-util`, `time`) instead of relying on transitive enablement. +The `UserError::UnexpectedFailure` catalog entry (`general.unexpected_failure`) is owned by `cli/src/services/error.rs`; `sce sync` uses it for non-authentication and non-credential-storage failures, rendering one fixed log-files guidance sentence through `services::app_support` without exposing a technical error source, interpolating a path, or changing the closed catalog into an arbitrary-message surface. + ## Build / devShell / CI performance (flake-speedup) The current structure and durable before/after results for the native/release diff --git a/context/cli/agent-trace-sync-command.md b/context/cli/agent-trace-sync-command.md index 7500c311..8ac78c1f 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 failure classification:** `ControlPlaneError::is_authentication_failure()` is `true` only for `MissingCredentials`/`AuthenticationFailed`, while `ControlPlaneError::is_storage_failure()` identifies local credential-storage failures. `TraceSyncError` uses the storage predicate only for a direct initial control-plane failure; `StreamSyncError::is_storage_failure()` delegates storage classification for `Refresh` and `Terminal` failures, while other stream error variants return false. 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 these predicates (never string/substring matching) to route authentication failures from the initial `/state` or stream paths to `CliError::User { error: UserError::NotAuthenticated, .. }` and credential-storage failures from the initial `/state` call to `CliError::User { error: UserError::AuthStorageUnavailable, .. }`; every other `ControlPlaneError` variant (`Forbidden`, `BadRequest`, `Transport`, `ServerError`, `InvalidResponse`, `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. - **`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/styling-service.md b/context/cli/styling-service.md index a87a418c..de0d43f0 100644 --- a/context/cli/styling-service.md +++ b/context/cli/styling-service.md @@ -24,12 +24,13 @@ The CLI styling service in `cli/src/services/style.rs` provides deterministic te - `command_name(text: &str) -> String` - Styles command names (green) for help output - `clap_help(text: &str) -> String` - Post-processes command-local clap help text so stdout help surfaces reuse shared heading, command, and placeholder styling without changing plain-text output when color is disabled -### Error Diagnostics Styling +### Internal Error Diagnostics Styling -- `error_code(text: &str) -> String` - Styles error codes (red/bold) for stderr diagnostics +- `error_code(text: &str) -> String` - Styles error codes (red/bold) for internal stderr diagnostics - `error_code_with_color_policy(text: &str, color_enabled: bool) -> String` - Internal variant accepting an explicit color policy flag for testability -- `heading(text: &str) -> String` - Styles headings for both stdout and stderr output (cyan/bold) -- `error_text_with_color_policy(text: &str, color_enabled: bool) -> String` - Internal helper styling human-readable stderr diagnostic bodies (yellow) given an explicit color policy flag; `app_support::write_error_diagnostic` is the sole production caller, passing `supports_color_stderr()` +- `heading(text: &str) -> String` - Styles headings for both stdout and internal stderr output (cyan/bold) +- `error_text_with_color_policy(text: &str, color_enabled: bool) -> String` - Internal helper styling human-readable internal stderr diagnostic bodies (yellow) given an explicit color policy flag; `app_support::write_error_diagnostic` is the sole production caller, passing `supports_color_stderr()` +- Catalog messages for expected failures are intentionally emitted redacted but unstyled and without the internal diagnostic wrapper. ### Command Output Styling @@ -54,7 +55,7 @@ The CLI styling service in `cli/src/services/style.rs` provides deterministic te - Help output uses `supports_color()` for stdout TTY detection - Command-local help styling is applied after clap renders plain help text, covering `Usage:`, section headings, command rows, and placeholder tokens on stdout surfaces - Error diagnostics use `supports_color_stderr()` for stderr TTY detection -- Top-level app diagnostics and observability log-file write failures both render through the shared stderr styling helpers when stderr color is enabled. +- Top-level internal app diagnostics and observability log-file write failures render through the shared stderr styling helpers when stderr color is enabled; user catalog diagnostics intentionally bypass those helpers. ## Sync progress styling @@ -82,7 +83,7 @@ use crate::services::style::{heading, command_name, error_code, error_text_with_ println!("{}", heading("Usage:")); println!(" {}", command_name("sce setup")); -// Error diagnostics styling (stderr) +// Internal error diagnostics styling (stderr) eprintln!( "{} [{}]: {}", heading("Error"), @@ -90,6 +91,8 @@ eprintln!( error_text_with_color_policy(message, supports_color_stderr()) ); +// Catalog messages are redacted and written without styling or wrapper. + // Command output styling println!("{}", success("Setup completed successfully.")); println!("{} {}", label("Repository root:"), value("'/path/to/repo'")); diff --git a/context/cli/sync-command.md b/context/cli/sync-command.md index fcdfb103..f6da87f4 100644 --- a/context/cli/sync-command.md +++ b/context/cli/sync-command.md @@ -102,19 +102,25 @@ client. The command change does not alter those semantics. ## Error classification `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 +terminal `TraceSyncError` into the typed `CliError` boundary through typed +predicates that traverse 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, .. }`. A credential +storage failure (`ControlPlaneError::Storage`) from the initial `/state` call +classifies as `CliError::User { error: UserError::AuthStorageUnavailable, .. }`. +Stream failures never classify as credential-storage user errors; their +authentication failures still use `NotAuthenticated`. Both user cases preserve +the technical error as their optional source. Every other `ControlPlaneError` +(`Forbidden`, `BadRequest`, `Transport`, `ServerError`, `InvalidResponse`, +`Protocol`) and runtime failures classify as +`CliError::User { error: UserError::UnexpectedFailure, .. }`; the technical +source remains available for observability. Stream credential-storage failures +also use `UnexpectedFailure`, because storage classification applies only to +the initial control-plane failure. +`sync/command.rs` builds no friendly sentence and applies no terminal styling +itself — `app_support` renders the catalog message for user cases. 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 5aaa2a05..58d554dd 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -129,4 +129,5 @@ Recent decision records: - `context/decisions/2026-08-13-trace-sync-progress-stream-contract.md` (keeps trace-sync progress and lifecycle timestamps on stderr while preserving stdout payload and JSON silence) - `context/decisions/2026-08-18-consumer-typed-progress-reporter-boundary.md` (keeps the reusable reporter contract generic over consumer event types while sync owns `SyncProgressEvent`) - `context/decisions/2026-08-18-sync-owned-progress-reporter-contract.md` (makes `services::sync::progress` the sole owner of the generic progress contract, no-op reporter, sync adapter, and focused tests; no top-level progress service remains) +- `context/decisions/2026-08-20-general-unexpected-user-error-catalog.md` (records the closed `UserError` catalog entry with one static log-files guidance sentence, no dynamic path interpolation or arbitrary message variant; current `sce sync` adoption is documented in `context/sce/cli-error-code-taxonomy.md`) - `context/decisions/2026-08-07-git-hook-managed-block-cooperation.md` (SCE-installed git hooks are a bounded in-place editor, not an exclusive owner: hook ownership is decided structurally by the SCE managed-block marker pair or a legacy guidance-URL marker, a foreign hook's bytes are preserved as an exact prefix with the block appended after them, and coexistence with third-party hook managers is cooperative, not authoritative) diff --git a/context/glossary.md b/context/glossary.md index e43633ce..e5e24abd 100644 --- a/context/glossary.md +++ b/context/glossary.md @@ -67,7 +67,7 @@ - `Nix performance recommendations`: Repo-local operator guidance in `AGENTS.md` covering optional user-level `~/.config/nix/nix.conf` tuning (`max-jobs = auto`, `cores = 0`) and the explicit root/admin-only boundary for `/etc/nix/nix.conf` `auto-optimise-store = true`. - `log_to_file`: Flat SCE config-file boolean controlling file-log emission independently of stderr and tracing. It defaults to `true`, is surfaced with source metadata by `sce config show`, and resolves independently from `log_dir`; an omitted `log_dir` uses the default location, while an explicitly empty config value remains invalid. Set `log_to_file` to `false` to disable file logging without changing other logger destinations. See [CLI observability contract](sce/cli-observability-contract.md). - `sce` (CLI foundation): Rust binary crate at `cli/` with implemented auth command flows (`auth login|logout|whoami`) plus auth-local bare-command guidance (`sce auth`, `sce auth --help`), Control Plane `/me`-backed whoami profile output using flat email/name/role/permissions/organization labels, exact logged-out login guidance, implemented setup installation flow including lifecycle-aggregated local DB and Agent Trace DB bootstrap, implemented attribution-only `hooks` subcommand routing/validation entrypoints, and a fully implemented top-level `sce sync` command that synchronizes the current repository's Agent Trace DB with the control plane and renders the documented text/JSON output (see `context/cli/sync-command.md`). -- `auth login stored-credential renewal`: The `sce auth login` behavior that first validates every stored credential through the existing non-forced token path, preserves valid credentials, refreshes expired credentials, and falls back to device authorization when credentials are absent or renewal fails. Renewal reports retain `login` labels in text and JSON; credential renewal is not exposed as a public subcommand. +- `auth login stored-credential renewal`: The `sce auth login` behavior that first validates every stored credential through the existing non-forced token path, preserves valid credentials, refreshes expired credentials, and falls back to device authorization when credentials are absent or renewal fails. Renewal reports retain `login` labels in text and JSON; credential renewal is not exposed as a public subcommand. The related typed local WorkOS credential-storage failure (`ControlPlaneError::Storage`) is currently classified only at the `sce sync` command boundary as `UserError::AuthStorageUnavailable` (`auth.storage_unavailable`), with a fixed actionable terminal message that exposes no storage implementation details or automatic `Try:` suffix while preserving the technical source for structured observability; auth command classification is not yet enabled. - `command surface contract`: The current top-level command/help catalog split where `cli/src/cli_schema.rs` owns the real clap-backed command metadata (top-level purpose text plus help visibility for `auth`, `config`, `setup`, `doctor`, `hooks`, `policy`, `sync`, `version`, and `completion`) and `cli/src/command_surface.rs` consumes that catalog for the custom banner/help surface plus known-command classification, while still adding the synthetic `help` row. - `top-level help visibility metadata`: Per-command `show_in_top_level_help` metadata in `cli/src/cli_schema.rs` that controls whether a known command appears in `sce`, `sce help`, and `sce --help` without affecting direct invocation; the current hidden top-level commands are `hooks` and `policy`, while `auth` is visible, and `cli/src/command_surface.rs` renders the curated top-level help list from that shared metadata. - `command loop`: The `clap` derive-based parser + dispatcher in `cli/src/cli_schema.rs`, `cli/src/services/parse/command_runtime.rs`, and `cli/src/app.rs` that routes `help`, `config`, `setup`, `doctor`, `auth`, `hooks`, `policy`, `sync`, `version`, and `completion`, executes implemented command flows, emits command-local help payloads for supported subcommand trees, and returns deterministic actionable errors for invalid invocation. @@ -117,9 +117,10 @@ - `setup directory write-permission probe`: deterministic pre-write guard implemented in `cli/src/services/security.rs` (`ensure_directory_is_writable`) and used by setup install/hook flows to fail fast with actionable remediation when target directories are not writable. - `setup --repo canonical path guard`: setup-hook runtime behavior in `cli/src/services/setup/mod.rs` that canonicalizes and validates user-supplied `--repo` paths as existing directories before git-root/hooks-path resolution. - `sce exit-code class contract`: Stable top-level process exit-code mapping owned by `cli/src/app.rs` (`0` success, `2` parse failure, `3` validation failure, `4` runtime failure, `5` dependency failure) so automation can branch on failure class without parsing text errors. -- `sce stderr error-code taxonomy`: Stable user-facing diagnostic code classes emitted by `cli/src/app.rs` (`SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, `SCE-ERR-DEPENDENCY`) via `Error []: ...` stderr formatting. -- `class-default Try guidance injection`: `cli/src/app.rs` diagnostic behavior that appends `Try:` remediation text by failure class only when an error message does not already include `Try:` guidance. +- `sce stderr error-code taxonomy`: Stable internal failure diagnostic classes emitted by `cli/src/app.rs` (`SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, `SCE-ERR-DEPENDENCY`) via styled `Error []: ...` stderr formatting; expected catalog failures emit only their redacted, unstyled catalog message. +- `class-default Try guidance injection`: `cli/src/app.rs` diagnostic behavior that appends `Try:` remediation text by failure class only for internal failures when an error message does not already include `Try:` guidance. - `sce observability baseline`: App-runtime logging contract in `cli/src/services/observability.rs` and `cli/src/app.rs` with config-resolved observability inputs, deterministic env-over-config-file-over-default precedence for non-flag logging keys, default-backed `log_dir` / `SCE_LOG_DIR` file routing by machine-local date plus optional session filename partitioning, creation-triggered retention of direct regular `*.log` files to 10 entries, stable lifecycle `event_id` values, tracing for all emitted records, and error-specific stderr suppression when file logging is enabled. +- `general unexpected user error`: `UserError::UnexpectedFailure` (`general.unexpected_failure`) catalog entry with the fixed sentence `An unexpected error occurred. Check the log files for more details.`. `sce sync` uses it for non-authentication and non-credential-storage failures while preserving the technical source for observability; the message exposes no path or implementation details. - `sce stdout/stderr contract`: App-level stream routing contract in `cli/src/app.rs` where command success payloads are emitted on stdout only, while redacted user-facing diagnostics and text-mode `sce sync` progress are emitted on stderr; JSON sync emits no human progress. - `SCE_LOG_LEVEL`: Optional runtime env key for `sce` observability threshold; allowed values are `error`, `warn`, `info`, and `debug`, defaulting to `error` when unset. - `SCE_LOG_FORMAT`: Optional runtime env key for `sce` observability record format; allowed values are `text` and `json`, defaulting to `text` when unset. diff --git a/context/overview.md b/context/overview.md index 637ad8b8..25deab0a 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 (currently only `NotAuthenticated`) for expected, deliberately-explained failures rendered as a friendly sentence with no `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`. See `context/sce/cli-error-code-taxonomy.md` for the full contract. +The same runtime also emits stable CLI stderr diagnostics: internal failures use `SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, or `SCE-ERR-DEPENDENCY` in deterministic `Error []: ...` diagnostics with class-default `Try:` remediation appended when missing, while expected failures emit only their redacted, unstyled catalog messages. The command boundary's former flat, string-only `ClassifiedError` has been replaced by typed `CliError` in `cli/src/services/error.rs`: the expected-error variant carries a closed catalog (`NotAuthenticated`, the authentication-storage `AuthStorageUnavailable`, and the general `UnexpectedFailure`) for expected, deliberately-explained failures rendered without the technical source, wrapper, styling, or automatic `Try:` guidance; the general entry renders one fixed static log-files guidance sentence without dynamic path text. `CliError::Internal` carries a live `anyhow::Error` source rendered as the real error chain with the existing styled wrapper and class-default remediation; `app_support` is the sole owner of the distinct terminal paths, and `sce sync` classifies authentication, credential-storage, and all other sync failures as cataloged user errors while preserving their technical sources for observability. 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/sce/cli-error-code-taxonomy.md b/context/sce/cli-error-code-taxonomy.md index b43c0da4..d162f651 100644 --- a/context/sce/cli-error-code-taxonomy.md +++ b/context/sce/cli-error-code-taxonomy.md @@ -14,10 +14,11 @@ It complements the numeric process exit-code classes documented in `context/sce/ ## Rendering contract -- User-facing diagnostics are emitted on `stderr` as: `Error []: `. +- Catalog diagnostics are emitted on `stderr` as the redacted catalog message followed by a newline, without an `Error` label, `SCE-ERR-*` code, separator, `Try:` guidance, or ANSI styling. This is the terminal path for `CliError::User`. +- `CliError::Internal` diagnostics are emitted on `stderr` as the styled `Error []: ` wrapper. - Before stderr emission, all `CliError` instances are logged via `Logger::log_cli_error()` with event ID `sce.error.{code}` and fields `error_code`, `error_class`. - For `CliError::Internal`, if the rendered message does not already include `Try:`, runtime appends class-default remediation guidance; if it already contains `Try:`, runtime preserves the original remediation text and does not append a second one. -- For `CliError::User`, runtime renders the catalog message from `UserError` verbatim, with no class-default `Try:` appended. +- For `CliError::User`, runtime renders the catalog message from `UserError` without technical source text or class-default `Try:` remediation. The `UserError::UnexpectedFailure` entry renders the fixed message `An unexpected error occurred. Check the log files for more details.` without dynamic path interpolation. - Diagnostic text is still redaction-filtered through `services::security::redact_sensitive_text` before emission. ## Actionable parser/invocation guidance contract @@ -31,11 +32,11 @@ 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` (currently only `NotAuthenticated`) for expected, deliberately-explained failures; `CliError::Internal` carries a live `anyhow::Error` source for every other failure. `CliError::User` may also carry an optional preserved technical `source`, kept for observability only and never rendered to the terminal. -- `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 is a fixed, reviewed sentence returned by `UserError::message()`, keyed for structured logging by `UserError::key()`. +- `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`, `AuthStorageUnavailable`, or `UnexpectedFailure`) for expected, deliberately-explained failures; `CliError::Internal` carries a live `anyhow::Error` source for every other failure. `CliError::User` may also carry an optional preserved technical `source`, kept for observability only and never rendered to the terminal. +- `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 is a fixed, reviewed sentence returned by `UserError::message()`, keyed for structured logging by `UserError::key()`. `AuthStorageUnavailable` (`auth.storage_unavailable`) is currently used by `sce sync` for typed authentication credential-storage failures. `UnexpectedFailure` (`general.unexpected_failure`) is used by `sce sync` for its default failure classification; it renders one fixed, user-safe diagnostic sentence and has no automatic `Try:` suffix or dynamic path input. - 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. +- `write_error_diagnostic` in `cli/src/services/app_support.rs` owns final stderr rendering: it redacts and writes the catalog variant's message without a wrapper or styling, while `CliError::Internal` retains code-bearing rendering and styles its 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. - `run_with_dependency_check_and_streams` in `cli/src/app.rs` owns error logging before stderr emission. ## Determinism and testing diff --git a/context/sce/cli-stdout-stderr-contract.md b/context/sce/cli-stdout-stderr-contract.md index cb82148c..f89a5548 100644 --- a/context/sce/cli-stdout-stderr-contract.md +++ b/context/sce/cli-stdout-stderr-contract.md @@ -8,8 +8,8 @@ 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. +- `CliError::Internal` 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`. `CliError::User` failures emit only their redacted message and trailing newline on `stderr`, without the wrapper, code, guidance, or ANSI styling. All emitted diagnostic text is 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 and applies the stderr TTY/`NO_COLOR` styling policy; the catalog variant renders its `UserError` message after redaction, with no low-level technical text, wrapper, styling, or `Try:` suffix. - Command handlers now return payload strings to the app dispatcher; the app owns stream selection and final emission. ## Implementation surface From f64ea56344ce0d9d79b74151e3859ff0fd1ddec7 Mon Sep 17 00:00:00 2001 From: stefanskoricdev Date: Thu, 20 Aug 2026 16:57:25 +0200 Subject: [PATCH 2/8] auth: Implement typed command error classification Classify authentication, storage, and fallback failures through the shared CliError user-error catalog while preserving technical sources for observability. Co-authored-by: SCE --- cli/src/services/auth_command/command.rs | 2 +- cli/src/services/auth_command/mod.rs | 159 +++++++++++------------ cli/src/services/token_storage.rs | 6 - context/architecture.md | 2 +- context/cli/cli-command-surface.md | 6 +- context/glossary.md | 2 +- context/overview.md | 2 +- context/sce/cli-error-code-taxonomy.md | 5 +- 8 files changed, 87 insertions(+), 97 deletions(-) diff --git a/cli/src/services/auth_command/command.rs b/cli/src/services/auth_command/command.rs index 9c5abadb..5e7ac22a 100644 --- a/cli/src/services/auth_command/command.rs +++ b/cli/src/services/auth_command/command.rs @@ -7,6 +7,6 @@ pub struct AuthCommand { impl AuthCommand { pub fn execute(&self, _context: &C) -> Result { - auth_command::run_auth_subcommand(self.request).map_err(CliError::runtime) + auth_command::run_auth_subcommand(self.request) } } diff --git a/cli/src/services/auth_command/mod.rs b/cli/src/services/auth_command/mod.rs index 3e3c6763..9c8158aa 100644 --- a/cli/src/services/auth_command/mod.rs +++ b/cli/src/services/auth_command/mod.rs @@ -11,6 +11,7 @@ use crate::services::agent_trace_sync::control_plane::{ }; use crate::services::auth::{self, AuthError, DeviceAuthFlowResult}; use crate::services::config; +use crate::services::error::{CliError, UserError}; use crate::services::output_format::OutputFormat; use crate::services::style::{label, prompt_label, prompt_value, success, value}; use crate::services::token_storage::{self, StoredTokens}; @@ -33,7 +34,7 @@ pub struct AuthRequest { pub subcommand: AuthSubcommand, } -pub fn run_auth_subcommand(request: AuthRequest) -> Result { +pub fn run_auth_subcommand(request: AuthRequest) -> Result { run_auth_subcommand_with(request, run_login, run_logout, run_whoami) } @@ -42,11 +43,11 @@ fn run_auth_subcommand_with( login: L, logout: O, whoami: S, -) -> Result +) -> Result where - L: FnOnce(AuthFormat) -> Result, - O: FnOnce(AuthFormat) -> Result, - S: FnOnce(AuthFormat) -> Result, + L: FnOnce(AuthFormat) -> Result, + O: FnOnce(AuthFormat) -> Result, + S: FnOnce(AuthFormat) -> Result, { match request.subcommand { AuthSubcommand::Login { format } => login(format), @@ -55,15 +56,16 @@ where } } -pub fn run_login(format: AuthFormat) -> Result { +pub fn run_login(format: AuthFormat) -> Result { let client = reqwest::Client::new(); - let runtime = shared_runtime()?; + let runtime = shared_runtime().map_err(unexpected_auth_command_error)?; - let client_id = resolve_login_client_id()?; + let client_id = resolve_login_client_id().map_err(unexpected_auth_command_error)?; + let stored_tokens = token_storage::load_tokens().map_err(auth_storage_error)?; run_login_with_stored_credentials( format, - token_storage::load_tokens()?, + stored_tokens, |stored_tokens| maybe_renew_stored_credentials(runtime, &client, &client_id, stored_tokens), |format| match format { AuthFormat::Text => run_text_login_with_runtime(runtime, &client, &client_id), @@ -72,35 +74,39 @@ pub fn run_login(format: AuthFormat) -> Result { ) } -pub fn run_logout(format: AuthFormat) -> Result { - let deleted = token_storage::delete_tokens().map_err(|error| { - let guidance = auth_state_path_guidance( - "verify file permissions for the auth state directory and rerun 'sce auth logout'", - ); - anyhow!(format!("{error} Try: {guidance}")) - })?; - render_logout_result(deleted, format) +pub fn run_logout(format: AuthFormat) -> Result { + let deleted = token_storage::delete_tokens().map_err(auth_storage_error)?; + if !deleted { + return Err(CliError::user(UserError::NotAuthenticated)); + } + render_logout_success(format).map_err(unexpected_auth_command_error) } -pub fn run_whoami(format: AuthFormat) -> Result { - if token_storage::load_tokens()?.is_none() { - return render_unauthenticated_whoami(format); +pub fn run_whoami(format: AuthFormat) -> Result { + if token_storage::load_tokens() + .map_err(auth_storage_error)? + .is_none() + { + return Err(CliError::user(UserError::NotAuthenticated)); } let cwd = std::env::current_dir() - .context("failed to determine current directory for auth config resolution")?; - let auth_config = config::resolve_auth_runtime_config(&cwd)?; + .context("failed to determine current directory for auth config resolution") + .map_err(unexpected_auth_command_error)?; + let auth_config = + config::resolve_auth_runtime_config(&cwd).map_err(unexpected_auth_command_error)?; let client = AuthenticatedControlPlaneClient::new( reqwest::Client::new(), auth_config.control_plane_base_url.value.unwrap_or_default(), auth::WORKOS_DEFAULT_BASE_URL, auth_config.workos_client_id.value.unwrap_or_default(), ); - let profile = shared_runtime()? + let profile = shared_runtime() + .map_err(unexpected_auth_command_error)? .block_on(client.me()) - .map_err(|error| map_whoami_control_plane_error(&error))?; + .map_err(map_whoami_control_plane_error)?; - render_whoami_result(&profile, format) + render_whoami_result(&profile, format).map_err(unexpected_auth_command_error) } fn shared_runtime() -> Result<&'static tokio::runtime::Runtime> { @@ -122,14 +128,16 @@ fn maybe_renew_stored_credentials( client: &reqwest::Client, client_id: &str, stored_tokens: &StoredTokens, -) -> Result> { +) -> Result, CliError> { match runtime.block_on(auth::ensure_valid_token_returning_token( client, auth::WORKOS_DEFAULT_BASE_URL, client_id, stored_tokens, )) { - Ok(token) => Ok(Some(token_storage::save_tokens(&token)?)), + Ok(token) => token_storage::save_tokens(&token) + .map(Some) + .map_err(auth_storage_error), Err(_) => Ok(None), } } @@ -139,14 +147,15 @@ fn run_login_with_stored_credentials( stored_tokens: Option, renew: R, device_login: D, -) -> Result +) -> Result where - R: FnOnce(&StoredTokens) -> Result>, - D: FnOnce(AuthFormat) -> Result, + R: FnOnce(&StoredTokens) -> Result, CliError>, + D: FnOnce(AuthFormat) -> Result, { if let Some(stored_tokens) = stored_tokens { if let Some(renewed_tokens) = renew(&stored_tokens)? { - return render_login_refresh_result(&renewed_tokens, format); + return render_login_refresh_result(&renewed_tokens, format) + .map_err(unexpected_auth_command_error); } } @@ -157,16 +166,16 @@ fn run_text_login_with_runtime( runtime: &tokio::runtime::Runtime, client: &reqwest::Client, client_id: &str, -) -> Result { +) -> Result { let authorization = runtime .block_on(auth::request_device_authorization( client, auth::WORKOS_DEFAULT_BASE_URL, client_id, )) - .map_err(|e| map_login_error(&e))?; + .map_err(map_login_error)?; - write_login_prompt(&authorization)?; + write_login_prompt(&authorization).map_err(unexpected_auth_command_error)?; let token = runtime .block_on(auth::complete_device_auth_flow_returning_token( @@ -175,9 +184,9 @@ fn run_text_login_with_runtime( client_id, &authorization, )) - .map_err(|e| map_login_error(&e))?; + .map_err(map_login_error)?; - let stored_tokens = token_storage::save_tokens(&token)?; + let stored_tokens = token_storage::save_tokens(&token).map_err(auth_storage_error)?; render_login_result( &DeviceAuthFlowResult { @@ -186,6 +195,7 @@ fn run_text_login_with_runtime( }, AuthFormat::Text, ) + .map_err(unexpected_auth_command_error) } fn run_login_json( @@ -193,14 +203,14 @@ fn run_login_json( client: &reqwest::Client, client_id: &str, format: AuthFormat, -) -> Result { +) -> Result { let authorization = runtime .block_on(auth::request_device_authorization( client, auth::WORKOS_DEFAULT_BASE_URL, client_id, )) - .map_err(|e| map_login_error(&e))?; + .map_err(map_login_error)?; let token = runtime .block_on(auth::complete_device_auth_flow_returning_token( @@ -209,9 +219,9 @@ fn run_login_json( client_id, &authorization, )) - .map_err(|e| map_login_error(&e))?; + .map_err(map_login_error)?; - let stored_tokens = token_storage::save_tokens(&token)?; + let stored_tokens = token_storage::save_tokens(&token).map_err(auth_storage_error)?; render_login_result( &DeviceAuthFlowResult { @@ -220,6 +230,7 @@ fn run_login_json( }, format, ) + .map_err(unexpected_auth_command_error) } fn resolve_login_client_id() -> Result { @@ -260,11 +271,12 @@ fn write_login_prompt(authorization: &auth::DeviceAuthorizationResponse) -> Resu Ok(()) } -fn map_login_error(error: &AuthError) -> anyhow::Error { - anyhow!(with_try_guidance( - error.to_string(), - "verify the resolved WorkOS client ID source (WORKOS_CLIENT_ID, config file, or baked default), confirm network access, and rerun 'sce auth login'." - )) +fn map_login_error(error: AuthError) -> CliError { + let user_error = match &error { + AuthError::Io(_) | AuthError::Storage(_) => UserError::AuthStorageUnavailable, + _ => UserError::UnexpectedFailure, + }; + CliError::user_with_source(user_error, error) } fn render_login_result(result: &DeviceAuthFlowResult, format: AuthFormat) -> Result { @@ -316,41 +328,20 @@ fn render_login_refresh_result(tokens: &StoredTokens, format: AuthFormat) -> Res } } -fn render_logout_result(deleted: bool, format: AuthFormat) -> Result { +fn render_logout_success(format: AuthFormat) -> Result { match format { - AuthFormat::Text => Ok(if deleted { - success("Logged out") - } else { - value("No user logged in") - }), + AuthFormat::Text => Ok(success("Logged out")), AuthFormat::Json => serde_json::to_string_pretty(&json!({ "status": "ok", "command": NAME, "subcommand": "logout", "authenticated": false, - "credentials_removed": deleted, + "credentials_removed": true, })) .context("failed to serialize auth logout report to JSON. Try: rerun 'sce auth logout --format json'."), } } -fn render_unauthenticated_whoami(format: AuthFormat) -> Result { - match format { - AuthFormat::Text => Ok(format!( - "You are not logged in. Please log in using the {} command.", - success("sce auth login") - )), - AuthFormat::Json => serde_json::to_string_pretty(&json!({ - "status": "ok", - "command": NAME, - "subcommand": "whoami", - "authentication_state": "unauthenticated", - "has_stored_credentials": false, - })) - .context("failed to serialize auth whoami report to JSON. Try: rerun 'sce auth whoami --format json'."), - } -} - fn render_whoami_result(profile: &MeResponse, format: AuthFormat) -> Result { match format { AuthFormat::Text => { @@ -402,21 +393,25 @@ fn render_whoami_result(profile: &MeResponse, format: AuthFormat) -> Result anyhow::Error { - anyhow!("failed to fetch authenticated user information from the Control Plane: {error}") +fn map_whoami_control_plane_error(error: ControlPlaneError) -> CliError { + let user_error = if error.is_authentication_failure() { + UserError::NotAuthenticated + } else if error.is_storage_failure() { + UserError::AuthStorageUnavailable + } else { + UserError::UnexpectedFailure + }; + + CliError::user_with_source( + user_error, + anyhow!("failed to fetch authenticated user information from the Control Plane: {error}"), + ) } -fn with_try_guidance(message: String, guidance: &str) -> String { - if message.contains("Try:") { - message - } else { - format!("{message} Try: {guidance}") - } +fn auth_storage_error(error: crate::services::token_storage::TokenStorageError) -> CliError { + CliError::user_with_source(UserError::AuthStorageUnavailable, error) } -fn auth_state_path_guidance(action: &str) -> String { - match token_storage::token_file_path() { - Ok(path) => format!("{action}; expected path: '{}'", path.display()), - Err(_) => action.to_string(), - } +fn unexpected_auth_command_error(error: anyhow::Error) -> CliError { + CliError::user_with_source(UserError::UnexpectedFailure, error) } diff --git a/cli/src/services/token_storage.rs b/cli/src/services/token_storage.rs index c6b3a997..ea1dd508 100644 --- a/cli/src/services/token_storage.rs +++ b/cli/src/services/token_storage.rs @@ -1,5 +1,4 @@ use std::fmt; -use std::path::PathBuf; use std::sync::OnceLock; use std::time::{SystemTime, UNIX_EPOCH}; @@ -7,7 +6,6 @@ use serde::{Deserialize, Serialize}; use crate::services::auth::TokenResponse; use crate::services::auth_db::AuthDb; -use crate::services::default_paths::auth_db_path; /// Constant row ID for the single token row in `auth_credentials`. const DEFAULT_TOKEN_ROW_ID: i64 = 1; @@ -160,10 +158,6 @@ pub fn delete_tokens() -> Result { Ok(affected > 0) } -pub fn token_file_path() -> Result { - auth_db_path().map_err(|error| TokenStorageError::PathResolution(error.to_string())) -} - fn current_unix_timestamp_seconds() -> Result { Ok(SystemTime::now() .duration_since(UNIX_EPOCH) diff --git a/context/architecture.md b/context/architecture.md index f0d7ce8b..49a92089 100644 --- a/context/architecture.md +++ b/context/architecture.md @@ -121,7 +121,7 @@ The repository includes a new placeholder Rust binary crate at `cli/`. - `cli/src/services/capabilities.rs` defines the current broad CLI capability traits consumed by the borrowed, compile-time-typed `AppContext`: `FsOps` with `StdFsOps` for filesystem operations and `GitOps` with `ProcessGitOps` for git command execution plus repository-root/hooks-directory resolution. Existing service internals do not consume these traits directly yet; command execution uses narrow accessors and repo-root-scoped context derivation. - `cli/src/services/lifecycle.rs` defines the current compile-safe lifecycle seam. `ServiceLifecycle` has default no-op generic `diagnose`, `fix`, and `setup` methods over `C: HasRepoRoot`, with lifecycle-owned health, fix, and setup result types so the trait contract is not publicly anchored to doctor/setup module types or the full `AppContext` shape. The same module owns the static `LifecycleProvider` enum and shared `lifecycle_providers(include_hooks)` catalog/factory, returning providers in deterministic order (config → local_db → auth_db → agent_trace_db → hooks when requested); enum dispatch calls each concrete provider through generic context methods without boxed lifecycle-provider allocation or repo-root trait-object context erasure. Hooks exposes a `HooksLifecycle` provider in `cli/src/services/hooks/lifecycle.rs` for hook rollout diagnosis/fix/setup using lifecycle-owned health records plus the canonical required-hook installer. Config exposes a `ConfigLifecycle` provider in `cli/src/services/config/lifecycle.rs` for global/repo-local config validation and repo-local `.sce/config.json` bootstrap. local_db exposes a `LocalDbLifecycle` provider in `cli/src/services/local_db/lifecycle.rs` for canonical local DB path health, parent-directory readiness/bootstrap, and `LocalDb::new()` setup. auth_db exposes an `AuthDbLifecycle` provider in `cli/src/services/auth_db/lifecycle.rs` for canonical auth DB path health, parent-directory readiness/bootstrap, and `AuthDb::new()` setup. agent_trace_db exposes an `AgentTraceDbLifecycle` provider in `cli/src/services/agent_trace_db/lifecycle.rs` for setup-time repository-scoped Agent Trace storage initialization when a repo root is available and repository Agent Trace DB path health/fix from resolved repository identity, returning an actionable "requires a Git repository" diagnostic outside repository context (no global/checkout fallback path; the former fallback was removed by the `retire-legacy-agent-trace-db` plan). Doctor runtime aggregates the full provider catalog for `diagnose` and `fix` and adapts lifecycle records into doctor report/fix records at the orchestration boundary; setup command aggregates the shared catalog for `setup` with hooks included only when requested and adapts hook setup outcomes before rendering setup-owned messages. - Agent Trace lifecycle setup resolves `agent_trace.repository_id` / `agent_trace.repository_remote`, creates/reuses checkout identity for diagnostics, and initializes the repository-scoped DB through `agent_trace_storage::resolve_agent_trace_storage(...)`; hook runtime uses the same storage resolver and `RepositoryAgentTraceDb` lazy fast-path-then-migrate open with bounded retry plus narrow migration-metadata repair for concurrent first-open races. -- `cli/src/services/auth_command/mod.rs` defines the implemented auth command surface for `sce auth login|logout|whoami`, including device-flow login, stored-credential validation/renewal through login with device-flow fallback, logout, and Control Plane `/me`-backed whoami rendering in text/JSON formats; text mode uses flat `Email`, `First Name`, `Last Name`, `Role`, `Permissions`, and `Organization Name` labels, with optional names and missing role/permissions/workspace values handled deterministically. Logged-out text returns exact login guidance and renewal reports retain the `login` operation label. `cli/src/services/auth_command/command.rs` owns the `AuthCommand` payload used by the static `RuntimeCommand` enum. There is no public renewal or status subcommand. +- `cli/src/services/auth_command/mod.rs` defines the implemented auth command surface for `sce auth login|logout|whoami`, including device-flow login, stored-credential validation/renewal through login with device-flow fallback, logout, and Control Plane `/me`-backed whoami rendering in text/JSON formats; text mode uses flat `Email`, `First Name`, `Last Name`, `Role`, `Permissions`, and `Organization Name` labels, with optional names and missing role/permissions/workspace values handled deterministically. Expected auth failures are classified at this boundary as typed `CliError::User` entries (`NotAuthenticated` for missing/authentication failures, `AuthStorageUnavailable` for token-storage plus `AuthError::Io`/`Storage` failures, and `UnexpectedFailure` for approved user-facing rendering/prompt fallbacks), with technical sources retained for observability; remaining auth-command failures also use `UnexpectedFailure` rather than a separate runtime mapping. Stored-credential login applies that classification inside `run_login_with_stored_credentials` and its renewal/device-login call path before returning to the command caller. Logged-out text returns exact login guidance and renewal reports retain the `login` operation label. `cli/src/services/auth_command/command.rs` owns the `AuthCommand` payload used by the static `RuntimeCommand` enum. There is no public renewal or status subcommand. - `cli/src/services/db/mod.rs` provides the shared generic Turso infrastructure seam: `DbSpec` supplies a service-specific name, path, ordered embedded migrations, and config-file lookup key (`db_config_key()`), while `TursoDb` owns parent-directory creation, `Builder::new_local(...)` initialization (with `experimental_multiprocess_wal(true)` for safe concurrent access), Turso connection setup, tokio current-thread runtime bridging, retry-backed blocking `execute`/`query`/`query_values`/`query_map` wrappers, and generic migration execution with per-database `__sce_migrations` metadata. `TursoDb::new()` and `EncryptedTursoDb::new()` wrap only their local open/connect block in `run_with_retry_sync` using a config-driven connection-open policy resolved from the `DATABASE_RETRY_CONFIG` `OnceLock` with fallback to hardcoded defaults, while operation methods use a config-driven operation policy from the same source. `query_values()` returns fully fetched column names plus raw `turso::Value` rows for deterministic operator-facing rendering; `query_map()` retries the initial query and row-fetch loop, then applies caller row mapping after retry completion. Migration execution is not retried and uses batch execution so one migration file may contain multiple SQL statements while still recording one migration ID. The same module also provides `EncryptedTursoDb`, a structurally parallel encrypted adapter that resolves the encryption key through `encryption_key::get_or_create_encryption_key()`, enables Turso local encryption with strict `aegis256` cipher selection, and exposes retry-backed synchronous wrappers plus migration execution. `cli/src/services/db/encryption_key.rs` first derives a Turso-compatible 64-character hex key from non-empty `SCE_AUTH_DB_ENCRYPTION_KEY` env-secret text when present, otherwise falls back to keyring-backed credential-store get-or-create behavior; no plaintext auth DB fallback exists. - `cli/src/services/local_db/mod.rs` provides the concrete local DB spec and `LocalDb` type alias over the shared generic `TursoDb` adapter. `LocalDbSpec` resolves the deterministic persistent runtime DB target through the shared default-path seam and declares no local migrations; `TursoDb` supplies retry-backed blocking `execute`/`query`, parent-directory creation, Turso connection setup, tokio current-thread runtime bridging, and generic migration execution. - `cli/src/services/auth_db/mod.rs` provides the encrypted auth DB spec and `AuthDb` type alias over `EncryptedTursoDb`. `AuthDbSpec` resolves `/sce/auth.db` through the shared default-path seam and embeds ordered auth migrations. Auth DB lifecycle setup/doctor integration is wired through `AuthDbLifecycle`; auth command/token-storage reads/writes are directed through `token_storage.rs`. diff --git a/context/cli/cli-command-surface.md b/context/cli/cli-command-surface.md index 63891f8c..d99a3d68 100644 --- a/context/cli/cli-command-surface.md +++ b/context/cli/cli-command-surface.md @@ -59,7 +59,7 @@ Deferred or gated command surfaces currently avoid claiming unimplemented behavi `setup` defaults to an `inquire` interactive target selection (OpenCode, Claude, Pi, Codex, All) and accepts mutually-exclusive non-interactive target flags (`--opencode`, `--claude`, `--pi`, `--codex`, `--all`); the former `--both` flag was removed in favor of `--all` (opencode+claude+pi+codex); the interactive prompt title and target labels reuse shared prompt styling helpers when stdout color is enabled. `setup` also accepts `--bootstrap-context` as a standalone context-only mode that ensures the durable-context baseline without prompts or integration installs; every normal successful setup path also ensures that baseline after the Git gate. `setup` accepts a repeatable `--workflow ` flag selecting which optional workflows to install (currently only `brownfield`). Passing it makes the listed slugs the exact selection for that run; omitting it reuses the persisted `integrations.optional_workflows`, so a repeat run preserves an earlier opt-in. Unknown slugs fail request resolution with a validation error naming the embedded catalog's available slugs and write no files, and `--workflow` is rejected alongside `--bootstrap-context` or on a hooks-only run because neither installs target assets. The resolved selection filters the installed assets and is persisted; see [config precedence contract](config-precedence-contract.md) and [setup local bootstrap](../sce/setup-repo-local-config-bootstrap.md). An interactive `setup` run instead resolves the selection through an `inquire` multi-select shown after the target prompt, titled `Select optional workflows` with one `{title} — {description}` row per optional workflow using the shared prompt styling. Rows are unchecked when nothing is persisted and pre-checked from `integrations.optional_workflows` otherwise (a supplied `--workflow` list seeds them instead); the answered prompt is the run's exact selection. Cancelling either prompt yields the existing `Setup cancelled. No files were changed.` outcome, a non-TTY run keeps the existing actionable guidance, and the prompt is skipped when the catalog has no optional workflow. -`auth` now emits auth-local guidance for bare `sce auth` and `sce auth --help`, listing `login`, `logout`, and `whoami` plus copy-ready next steps. `sce auth login` uses the existing token-validation path whenever stored credentials are present: valid credentials are preserved, expired credentials are refreshed, and a failed renewal falls back to device authorization. First login without stored credentials still starts device authorization, and renewal reports remain labeled as `login` in text and JSON output. Authenticated `sce auth whoami` retrieves the authoritative profile from the Control Plane `GET /me` endpoint and renders flat text labels for `Email`, `First Name`, `Last Name`, `Role`, `Permissions`, and `Organization Name`; missing first/last names render empty and missing role, permissions, or workspace values render `none`. +`auth` now emits auth-local guidance for bare `sce auth` and `sce auth --help`, listing `login`, `logout`, and `whoami` plus copy-ready next steps. `sce auth login` uses the existing token-validation path whenever stored credentials are present: valid credentials are preserved, expired credentials are refreshed, and a failed renewal falls back to device authorization. First login without stored credentials still starts device authorization, and renewal reports remain labeled as `login` in text and JSON output. Expected authentication failures at the auth command boundary are typed as `CliError::User`: missing credentials from `logout`/`whoami` and Control Plane authentication failures from `whoami` render `NotAuthenticated`, token-storage plus `AuthError::Io`/`Storage` failures across `login`, `logout`, and `whoami` render `AuthStorageUnavailable`, and approved user-facing rendering/prompt fallbacks render `UnexpectedFailure`; stored-credential login renewal, token-save, and device-flow paths classify before returning through `run_login_with_stored_credentials`, and technical error chains remain attached for observability. Remaining auth-command failures are surfaced as `UnexpectedFailure` with their technical sources preserved. Authenticated `sce auth whoami` retrieves the authoritative profile from the Control Plane `GET /me` endpoint and renders flat text labels for `Email`, `First Name`, `Last Name`, `Role`, `Permissions`, and `Organization Name`; missing first/last names render empty and missing role, permissions, or workspace values render `none`. `setup`, `doctor`, `hooks`, `policy`, `sync`, `version`, and `completion` all support command-local `--help`/`-h` usage output via top-level parser routing in `cli/src/app.rs`. `setup` now also exposes compile-time embedded config assets for OpenCode/Claude/Pi/Codex targets, sourced from the generated `config/.opencode/**`, `config/.claude/**`, `config/.pi/**`, and (via a build-time staging merge of `config/.agents/**` + `config/.codex/**`) `config/codex-target/**` trees via `cli/build.rs` with normalized forward-slash relative paths and target-scoped iteration APIs; the embedded asset set includes the OpenCode bash-policy plugin wrapper plus Claude settings `PreToolUse` Bash policy hook, both delegating to the Rust `sce policy bash` path. `setup` additionally includes a repository-root install engine (`install_embedded_setup_assets`) that installs each embedded asset individually into `.opencode/`/`.claude/`/`.pi/` — stage next to the final destination, remove only that destination file if present, swap into place, with deterministic recovery guidance naming the failing asset's path on swap failure — never removing an integration target directory as a whole, while treating bash-policy enforcement files as first-class SCE-managed assets. Codex is the one target whose embedded relative paths already carry their own output-root prefix (`.agents/...`, `.codex/...`), so its destination root is the repository root itself rather than a single `.codex/`-style subdirectory. Its generated hook command resolves that repository root at invocation time, so Codex events from nested cwd and repositories with spaces reach the installed helper safely; Git-root failure is a silent successful no-op, while the helper preserves missing-CLI stderr guidance and STDIN forwarding. See [setup non-destructive per-asset install policy](../sce/setup-no-backup-policy-seam.md) for the full contract, including the pending pruning gap for deselected/stale assets. @@ -97,8 +97,8 @@ An interactive `setup` run instead resolves the selection through an `inquire` m - `cli/src/services/sync/sync.rs` implements `sce sync` orchestration (control-plane authentication, per-stream reconciliation, and report assembly); local DB initialization and health ownership remain split between setup and doctor. `cli/src/services/sync/command.rs` owns format-gated stderr progress and `cli/src/services/sync/render_sync.rs` owns text/JSON report rendering. See [agent-trace-sync-command.md](agent-trace-sync-command.md). - `cli/src/services/default_paths.rs` defines the canonical per-user persisted-location seam for config/state/cache roots plus named default file paths for current persisted artifacts (`global config`, `auth tokens`, `local DB`, `agent trace DB`) used by config discovery, token storage, database adapters, and doctor diagnostics; its internal `roots` seam now owns the platform-aware root-directory resolution so non-test production modules consume shared path accessors instead of resolving owned roots directly. - `cli/src/services/agent_trace.rs` defines the canonical Rust SCE web base URL and helpers for Agent Trace conversation URLs, persisted Agent Trace trace URLs, Agent Trace session URLs, and setup-created repo-local config schema URLs. -- `cli/src/services/token_storage.rs` defines WorkOS token persistence (`save_tokens`, `load_tokens`, `delete_tokens`) via the encrypted `AuthDb` `auth_credentials` table using a `OnceLock` lazy singleton with constant integer row ID `1`. `token_file_path()` returns the auth DB path. `TokenStorageError` exposes `PathResolution` and `Database` variants. No JSON file I/O remains. -- `cli/src/services/auth_command/mod.rs` defines the auth command orchestration surface (`AuthRequest`, `AuthSubcommand`, `run_auth_subcommand`) for `login`, `logout`, and `whoami`, including shared text/JSON rendering, login's stored-token validation and refresh path for any stored credential, device-flow fallback after absent or unsuccessfully renewed credentials, token-storage-backed logout deletion with path-aware remediation guidance, Control Plane `/me` profile retrieval for authenticated whoami, flat safe-field rendering with optional-name/null-value handling, exact logged-out text guidance, precedence-aware client-ID guidance sourced from the shared auth-runtime resolver instead of env-only assumptions, and a lazily initialized current-thread Tokio runtime with both I/O and time enabled so the auth flows can drive the WorkOS device/refresh paths without the prior I/O-disabled panic; `cli/src/services/auth_command/command.rs` owns the `AuthCommand` payload used by the static `RuntimeCommand` enum. +- `cli/src/services/token_storage.rs` defines WorkOS token persistence (`save_tokens`, `load_tokens`, `delete_tokens`) via the encrypted `AuthDb` `auth_credentials` table using a `OnceLock` lazy singleton with constant integer row ID `1`. `TokenStorageError` exposes `PathResolution` and `Database` variants. No JSON file I/O remains. +- `cli/src/services/auth_command/mod.rs` defines the auth command orchestration surface (`AuthRequest`, `AuthSubcommand`, `run_auth_subcommand`) for `login`, `logout`, and `whoami`, including shared text/JSON rendering, login's stored-token validation and refresh path for any stored credential, device-flow fallback after absent or unsuccessfully renewed credentials, source-level typed `CliError` propagation through `run_login_with_stored_credentials` for renewal, token-save, and device-login failures, token-storage-backed logout deletion, Control Plane `/me` profile retrieval for authenticated whoami, flat safe-field rendering with optional-name/null-value handling, exact logged-out text guidance, precedence-aware client-ID guidance sourced from the shared auth-runtime resolver instead of env-only assumptions, and a lazily initialized current-thread Tokio runtime with both I/O and time enabled so the auth flows can drive the WorkOS device/refresh paths without the prior I/O-disabled panic; `cli/src/services/auth_command/command.rs` owns the `AuthCommand` payload used by the static `RuntimeCommand` enum. - `cli/src/app.rs` parses `auth`, `config`, `setup`, `doctor`, `hooks`, `policy`, `sync`, `version`, and `completion` into service-owned runtime command handlers so runtime messages are sourced from domain modules instead of inline strings. ## Local and Agent Trace Turso adapter behavior diff --git a/context/glossary.md b/context/glossary.md index e5e24abd..1faa6172 100644 --- a/context/glossary.md +++ b/context/glossary.md @@ -67,7 +67,7 @@ - `Nix performance recommendations`: Repo-local operator guidance in `AGENTS.md` covering optional user-level `~/.config/nix/nix.conf` tuning (`max-jobs = auto`, `cores = 0`) and the explicit root/admin-only boundary for `/etc/nix/nix.conf` `auto-optimise-store = true`. - `log_to_file`: Flat SCE config-file boolean controlling file-log emission independently of stderr and tracing. It defaults to `true`, is surfaced with source metadata by `sce config show`, and resolves independently from `log_dir`; an omitted `log_dir` uses the default location, while an explicitly empty config value remains invalid. Set `log_to_file` to `false` to disable file logging without changing other logger destinations. See [CLI observability contract](sce/cli-observability-contract.md). - `sce` (CLI foundation): Rust binary crate at `cli/` with implemented auth command flows (`auth login|logout|whoami`) plus auth-local bare-command guidance (`sce auth`, `sce auth --help`), Control Plane `/me`-backed whoami profile output using flat email/name/role/permissions/organization labels, exact logged-out login guidance, implemented setup installation flow including lifecycle-aggregated local DB and Agent Trace DB bootstrap, implemented attribution-only `hooks` subcommand routing/validation entrypoints, and a fully implemented top-level `sce sync` command that synchronizes the current repository's Agent Trace DB with the control plane and renders the documented text/JSON output (see `context/cli/sync-command.md`). -- `auth login stored-credential renewal`: The `sce auth login` behavior that first validates every stored credential through the existing non-forced token path, preserves valid credentials, refreshes expired credentials, and falls back to device authorization when credentials are absent or renewal fails. Renewal reports retain `login` labels in text and JSON; credential renewal is not exposed as a public subcommand. The related typed local WorkOS credential-storage failure (`ControlPlaneError::Storage`) is currently classified only at the `sce sync` command boundary as `UserError::AuthStorageUnavailable` (`auth.storage_unavailable`), with a fixed actionable terminal message that exposes no storage implementation details or automatic `Try:` suffix while preserving the technical source for structured observability; auth command classification is not yet enabled. +- `auth login stored-credential renewal`: The `sce auth login` behavior that first validates every stored credential through the existing non-forced token path, preserves valid credentials, refreshes expired credentials, and falls back to device authorization when credentials are absent or renewal fails. Renewal reports retain `login` labels in text and JSON; credential renewal is not exposed as a public subcommand. The auth command boundary classifies missing credentials and Control Plane authentication failures as `UserError::NotAuthenticated`, token-storage plus `AuthError::Io`/`Storage` failures as `UserError::AuthStorageUnavailable` (`auth.storage_unavailable`), and approved user-facing rendering/prompt fallbacks as `UserError::UnexpectedFailure` (`general.unexpected_failure`); fixed catalog messages expose no storage or implementation details while technical sources remain available for structured observability, and remaining auth-command failures use `UnexpectedFailure` with preserved technical sources. - `command surface contract`: The current top-level command/help catalog split where `cli/src/cli_schema.rs` owns the real clap-backed command metadata (top-level purpose text plus help visibility for `auth`, `config`, `setup`, `doctor`, `hooks`, `policy`, `sync`, `version`, and `completion`) and `cli/src/command_surface.rs` consumes that catalog for the custom banner/help surface plus known-command classification, while still adding the synthetic `help` row. - `top-level help visibility metadata`: Per-command `show_in_top_level_help` metadata in `cli/src/cli_schema.rs` that controls whether a known command appears in `sce`, `sce help`, and `sce --help` without affecting direct invocation; the current hidden top-level commands are `hooks` and `policy`, while `auth` is visible, and `cli/src/command_surface.rs` renders the curated top-level help list from that shared metadata. - `command loop`: The `clap` derive-based parser + dispatcher in `cli/src/cli_schema.rs`, `cli/src/services/parse/command_runtime.rs`, and `cli/src/app.rs` that routes `help`, `config`, `setup`, `doctor`, `auth`, `hooks`, `policy`, `sync`, `version`, and `completion`, executes implemented command flows, emits command-local help payloads for supported subcommand trees, and returns deterministic actionable errors for invalid invocation. diff --git a/context/overview.md b/context/overview.md index 25deab0a..55c239bd 100644 --- a/context/overview.md +++ b/context/overview.md @@ -19,7 +19,7 @@ The generated `/next-task` workflow persists task-level context-synchronization The CLI crate currently depends on `anyhow`, `chrono`, `clap`, `clap_complete`, `dirs`, `hmac`, `indicatif`, `inquire`, `jsonschema`, `keyring-core`, `murmur3`, `owo-colors`, `rand`, `reqwest`, `serde`, `serde_json`, `sha2`, `tokio`, `tracing`, `turso`, and `uuid`, with target-specific keyring backend dependencies for Linux/FreeBSD, macOS, and Windows. No CLI dev-dependencies are currently declared. 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 command loop now enforces a stable exit-code contract in `cli/src/app.rs`: `2` parse failures, `3` invocation validation failures, `4` runtime failures, and `5` dependency startup failures. Auth command orchestration routes expected missing/authentication failures, token-storage failures, and approved user-facing fallback failures through typed `CliError::User` catalog entries (`NotAuthenticated`, `AuthStorageUnavailable`, and `UnexpectedFailure`) while retaining technical sources for observability; internal auth failures remain runtime errors. The same runtime also emits stable CLI stderr diagnostics: internal failures use `SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, or `SCE-ERR-DEPENDENCY` in deterministic `Error []: ...` diagnostics with class-default `Try:` remediation appended when missing, while expected failures emit only their redacted, unstyled catalog messages. The command boundary's former flat, string-only `ClassifiedError` has been replaced by typed `CliError` in `cli/src/services/error.rs`: the expected-error variant carries a closed catalog (`NotAuthenticated`, the authentication-storage `AuthStorageUnavailable`, and the general `UnexpectedFailure`) for expected, deliberately-explained failures rendered without the technical source, wrapper, styling, or automatic `Try:` guidance; the general entry renders one fixed static log-files guidance sentence without dynamic path text. `CliError::Internal` carries a live `anyhow::Error` source rendered as the real error chain with the existing styled wrapper and class-default remediation; `app_support` is the sole owner of the distinct terminal paths, and `sce sync` classifies authentication, credential-storage, and all other sync failures as cataloged user errors while preserving their technical sources for observability. 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. diff --git a/context/sce/cli-error-code-taxonomy.md b/context/sce/cli-error-code-taxonomy.md index d162f651..0c645724 100644 --- a/context/sce/cli-error-code-taxonomy.md +++ b/context/sce/cli-error-code-taxonomy.md @@ -33,11 +33,12 @@ It complements the numeric process exit-code classes documented in `context/sce/ - `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`, `AuthStorageUnavailable`, or `UnexpectedFailure`) for expected, deliberately-explained failures; `CliError::Internal` carries a live `anyhow::Error` source for every other failure. `CliError::User` may also carry an optional preserved technical `source`, kept for observability only and never rendered to the terminal. -- `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 is a fixed, reviewed sentence returned by `UserError::message()`, keyed for structured logging by `UserError::key()`. `AuthStorageUnavailable` (`auth.storage_unavailable`) is currently used by `sce sync` for typed authentication credential-storage failures. `UnexpectedFailure` (`general.unexpected_failure`) is used by `sce sync` for its default failure classification; it renders one fixed, user-safe diagnostic sentence and has no automatic `Try:` suffix or dynamic path input. -- 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. +- `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 is a fixed, reviewed sentence returned by `UserError::message()`, keyed for structured logging by `UserError::key()`. `NotAuthenticated` (`auth.not_authenticated`) covers missing credentials from `sce auth logout`/`whoami` and Control Plane authentication failures from `whoami`; `AuthStorageUnavailable` (`auth.storage_unavailable`) is used by `sce sync` and the `sce auth login`, `logout`, and `whoami` command boundary for token-storage plus `AuthError::Io`/`Storage` failures. Both auth-command mappings preserve technical sources for observability. `UnexpectedFailure` (`general.unexpected_failure`) is used by `sce sync` for its default failure classification and by the auth-command boundary for all non-storage failures, including rendering, prompt, runtime, configuration, and unrelated Control Plane failures; it renders one fixed, user-safe diagnostic sentence and has no automatic `Try:` suffix or dynamic path input. +- Command and domain layers construct and return a `CliError`; they do not format terminal text or apply styling. Auth command orchestration classifies expected authentication and credential-storage failures into the existing `UserError` catalog by typed domain variants, never by string matching, and preserves the original technical chain as the optional user-error source. `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 stderr rendering: it redacts and writes the catalog variant's message without a wrapper or styling, while `CliError::Internal` retains code-bearing rendering and styles its 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. - `run_with_dependency_check_and_streams` in `cli/src/app.rs` owns error logging before stderr emission. +- The auth-command typed user-error boundary is an accepted system-wide contract; see [the auth-command decision](../decisions/2026-08-20-auth-command-typed-user-errors.md) and [the auth fallback decision](../decisions/2026-08-20-auth-command-unexpected-fallbacks.md). ## Determinism and testing From b57c6a8f7400afb947fc0417abf381ac8a5868db Mon Sep 17 00:00:00 2001 From: stefanskoricdev Date: Fri, 21 Aug 2026 10:52:04 +0200 Subject: [PATCH 3/8] runtime: Add typed setup command error handling Expose missing Git repository failures through the stable user-error catalog while preserving technical sources for observability. Co-authored-by: SCE --- cli/src/services/auth_command/mod.rs | 4 ++-- cli/src/services/config/command.rs | 5 +++-- cli/src/services/doctor/command.rs | 5 +++-- cli/src/services/error.rs | 29 +++++++++++++++++++++++--- cli/src/services/setup/command.rs | 20 +++++++++++------- cli/src/services/version/command.rs | 5 +++-- context/overview.md | 4 ++-- context/sce/cli-error-code-taxonomy.md | 6 +++--- context/sce/setup-githooks-cli-ux.md | 2 +- 9 files changed, 55 insertions(+), 25 deletions(-) diff --git a/cli/src/services/auth_command/mod.rs b/cli/src/services/auth_command/mod.rs index 9c8158aa..cf442757 100644 --- a/cli/src/services/auth_command/mod.rs +++ b/cli/src/services/auth_command/mod.rs @@ -104,7 +104,7 @@ pub fn run_whoami(format: AuthFormat) -> Result { let profile = shared_runtime() .map_err(unexpected_auth_command_error)? .block_on(client.me()) - .map_err(map_whoami_control_plane_error)?; + .map_err(|error| map_whoami_control_plane_error(&error))?; render_whoami_result(&profile, format).map_err(unexpected_auth_command_error) } @@ -393,7 +393,7 @@ fn render_whoami_result(profile: &MeResponse, format: AuthFormat) -> Result CliError { +fn map_whoami_control_plane_error(error: &ControlPlaneError) -> CliError { let user_error = if error.is_authentication_failure() { UserError::NotAuthenticated } else if error.is_storage_failure() { diff --git a/cli/src/services/config/command.rs b/cli/src/services/config/command.rs index 0f7385a0..af7a6fff 100644 --- a/cli/src/services/config/command.rs +++ b/cli/src/services/config/command.rs @@ -1,5 +1,5 @@ use crate::services::config; -use crate::services::error::CliError; +use crate::services::error::{CliError, UserError}; pub struct ConfigCommand { pub subcommand: config::ConfigSubcommand, @@ -7,6 +7,7 @@ pub struct ConfigCommand { impl ConfigCommand { pub fn execute(&self, _context: &C) -> Result { - config::run_config_subcommand(self.subcommand.clone()).map_err(CliError::runtime) + config::run_config_subcommand(self.subcommand.clone()) + .map_err(|source| CliError::user_with_source(UserError::UnexpectedFailure, source)) } } diff --git a/cli/src/services/doctor/command.rs b/cli/src/services/doctor/command.rs index 3edf1029..a8b8f40c 100644 --- a/cli/src/services/doctor/command.rs +++ b/cli/src/services/doctor/command.rs @@ -1,6 +1,6 @@ use crate::app::ContextWithRepoRoot; use crate::services::doctor; -use crate::services::error::CliError; +use crate::services::error::{CliError, UserError}; pub struct DoctorCommand { pub request: doctor::DoctorRequest, @@ -8,6 +8,7 @@ pub struct DoctorCommand { impl DoctorCommand { pub fn execute(&self, context: &C) -> Result { - doctor::run_doctor_with_context(self.request, context).map_err(CliError::runtime) + doctor::run_doctor_with_context(self.request, context) + .map_err(|source| CliError::user_with_source(UserError::UnexpectedFailure, source)) } } diff --git a/cli/src/services/error.rs b/cli/src/services/error.rs index f83d68ef..86bd06d2 100644 --- a/cli/src/services/error.rs +++ b/cli/src/services/error.rs @@ -55,6 +55,7 @@ pub enum UserError { #[allow(dead_code)] NotAuthenticated, AuthStorageUnavailable, + NotGitRepository, #[allow(dead_code)] UnexpectedFailure, } @@ -62,9 +63,10 @@ pub enum UserError { impl UserError { pub fn class(self) -> FailureClass { match self { - Self::NotAuthenticated | Self::AuthStorageUnavailable | Self::UnexpectedFailure => { - FailureClass::Runtime - } + Self::NotAuthenticated + | Self::AuthStorageUnavailable + | Self::NotGitRepository + | Self::UnexpectedFailure => FailureClass::Runtime, } } @@ -73,6 +75,7 @@ impl UserError { match self { Self::NotAuthenticated => "auth.not_authenticated", Self::AuthStorageUnavailable => "auth.storage_unavailable", + Self::NotGitRepository => "setup.not_git_repository", Self::UnexpectedFailure => "general.unexpected_failure", } } @@ -85,6 +88,9 @@ impl UserError { Self::AuthStorageUnavailable => { "Authentication storage is unavailable. Verify local credential storage is available, then retry the command." } + Self::NotGitRepository => { + "This directory is not a Git repository. Run `git init`, then rerun `sce setup`." + } Self::UnexpectedFailure => { "An unexpected error occurred. Check the log files for more details." } @@ -203,6 +209,23 @@ mod tests { assert!(error.to_string().contains("You are not logged in")); } + #[test] + fn not_git_repository_has_stable_runtime_catalog_mapping() { + let error = CliError::user(UserError::NotGitRepository); + + assert_eq!(error.class(), FailureClass::Runtime); + assert_eq!(error.code(), "SCE-ERR-RUNTIME"); + assert_eq!( + UserError::NotGitRepository.key(), + "setup.not_git_repository" + ); + assert_eq!( + UserError::NotGitRepository.message(), + "This directory is not a Git repository. Run `git init`, then rerun `sce setup`." + ); + assert_eq!(error.to_string(), UserError::NotGitRepository.message()); + } + #[test] fn unexpected_failure_has_stable_runtime_catalog_mapping() { let error = CliError::user(UserError::UnexpectedFailure); diff --git a/cli/src/services/setup/command.rs b/cli/src/services/setup/command.rs index 86bb6fe3..ede78032 100644 --- a/cli/src/services/setup/command.rs +++ b/cli/src/services/setup/command.rs @@ -1,7 +1,7 @@ use anyhow::Context; use crate::app::ContextWithRepoRoot; -use crate::services::error::CliError; +use crate::services::error::{CliError, UserError}; use crate::services::lifecycle::{ lifecycle_providers, RequiredHookInstallStatus, RequiredHooksInstallOutcome, }; @@ -17,13 +17,13 @@ impl SetupCommand { Some(path) => path.clone(), None => std::env::current_dir() .context("Failed to determine current directory") - .map_err(CliError::runtime)?, + .map_err(unexpected_failure)?, }; // The repository root is resolved before any prompt so the interactive // optional-workflow prompt can pre-check the persisted selection. - let repository_root = - setup::ensure_git_repository(&setup_start_path).map_err(CliError::runtime)?; + let repository_root = setup::ensure_git_repository(&setup_start_path) + .map_err(|source| CliError::user_with_source(UserError::NotGitRepository, source))?; let setup_dispatch = if self.request.context_only { None @@ -40,7 +40,7 @@ impl SetupCommand { &setup::InquireSetupTargetPrompter, &optional_workflow_defaults, ) - .map_err(CliError::runtime)? + .map_err(unexpected_failure)? { setup::SetupDispatch::Proceed { mode: resolved_mode, @@ -58,7 +58,7 @@ impl SetupCommand { // Every successful setup path ensures the durable-context baseline exists. let context_message = - setup::bootstrap_context_baseline(&repository_root).map_err(CliError::runtime)?; + setup::bootstrap_context_baseline(&repository_root).map_err(unexpected_failure)?; sections.push(context_message); if self.request.context_only { @@ -73,7 +73,7 @@ impl SetupCommand { let providers = lifecycle_providers(self.request.install_hooks); for provider in &providers { - let outcome = provider.setup(&ctx).map_err(CliError::runtime)?; + let outcome = provider.setup(&ctx).map_err(unexpected_failure)?; sections.extend(outcome.messages); @@ -94,7 +94,7 @@ impl SetupCommand { let setup_message = setup::run_setup_for_mode(&repository_root, resolved_mode, optional_workflows) - .map_err(CliError::runtime)?; + .map_err(unexpected_failure)?; sections.push(setup_message); } @@ -102,6 +102,10 @@ impl SetupCommand { } } +fn unexpected_failure(source: impl Into) -> CliError { + CliError::user_with_source(UserError::UnexpectedFailure, source) +} + fn setup_required_hooks_outcome_from_lifecycle( outcome: &RequiredHooksInstallOutcome, ) -> setup::RequiredHooksInstallOutcome { diff --git a/cli/src/services/version/command.rs b/cli/src/services/version/command.rs index c5fd2ae4..5056ca32 100644 --- a/cli/src/services/version/command.rs +++ b/cli/src/services/version/command.rs @@ -1,4 +1,4 @@ -use crate::services::error::CliError; +use crate::services::error::{CliError, UserError}; use crate::services::version; pub struct VersionCommand { @@ -7,6 +7,7 @@ pub struct VersionCommand { impl VersionCommand { pub fn execute(&self, _context: &C) -> Result { - version::render_version(self.request).map_err(CliError::runtime) + version::render_version(self.request) + .map_err(|source| CliError::user_with_source(UserError::UnexpectedFailure, source)) } } diff --git a/context/overview.md b/context/overview.md index 55c239bd..8ffca3e1 100644 --- a/context/overview.md +++ b/context/overview.md @@ -19,8 +19,8 @@ The generated `/next-task` workflow persists task-level context-synchronization The CLI crate currently depends on `anyhow`, `chrono`, `clap`, `clap_complete`, `dirs`, `hmac`, `indicatif`, `inquire`, `jsonschema`, `keyring-core`, `murmur3`, `owo-colors`, `rand`, `reqwest`, `serde`, `serde_json`, `sha2`, `tokio`, `tracing`, `turso`, and `uuid`, with target-specific keyring backend dependencies for Linux/FreeBSD, macOS, and Windows. No CLI dev-dependencies are currently declared. 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. Auth command orchestration routes expected missing/authentication failures, token-storage failures, and approved user-facing fallback failures through typed `CliError::User` catalog entries (`NotAuthenticated`, `AuthStorageUnavailable`, and `UnexpectedFailure`) while retaining technical sources for observability; internal auth failures remain runtime errors. -The same runtime also emits stable CLI stderr diagnostics: internal failures use `SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, or `SCE-ERR-DEPENDENCY` in deterministic `Error []: ...` diagnostics with class-default `Try:` remediation appended when missing, while expected failures emit only their redacted, unstyled catalog messages. The command boundary's former flat, string-only `ClassifiedError` has been replaced by typed `CliError` in `cli/src/services/error.rs`: the expected-error variant carries a closed catalog (`NotAuthenticated`, the authentication-storage `AuthStorageUnavailable`, and the general `UnexpectedFailure`) for expected, deliberately-explained failures rendered without the technical source, wrapper, styling, or automatic `Try:` guidance; the general entry renders one fixed static log-files guidance sentence without dynamic path text. `CliError::Internal` carries a live `anyhow::Error` source rendered as the real error chain with the existing styled wrapper and class-default remediation; `app_support` is the sole owner of the distinct terminal paths, and `sce sync` classifies authentication, credential-storage, and all other sync failures as cataloged user errors while preserving their technical sources for observability. See `context/sce/cli-error-code-taxonomy.md` for the full contract. +The command loop now enforces a stable exit-code contract in `cli/src/app.rs`: `2` parse failures, `3` invocation validation failures, `4` runtime failures, and `5` dependency startup failures. Auth command orchestration routes expected missing/authentication failures, token-storage failures, and approved user-facing fallback failures through typed `CliError::User` catalog entries (`NotAuthenticated`, `AuthStorageUnavailable`, `NotGitRepository`, and `UnexpectedFailure`) while retaining technical sources for observability; internal auth failures remain runtime errors. +The same runtime also emits stable CLI stderr diagnostics: internal failures use `SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, or `SCE-ERR-DEPENDENCY` in deterministic `Error []: ...` diagnostics with class-default `Try:` remediation appended when missing, while expected failures emit only their redacted, unstyled catalog messages. The command boundary's former flat, string-only `ClassifiedError` has been replaced by typed `CliError` in `cli/src/services/error.rs`: the expected-error variant carries a closed catalog (`NotAuthenticated`, the authentication-storage `AuthStorageUnavailable`, the setup `NotGitRepository`, and the general `UnexpectedFailure`) for expected, deliberately-explained failures rendered without the technical source, wrapper, styling, or automatic `Try:` guidance; the general entry renders one fixed static log-files guidance sentence without dynamic path text. `CliError::Internal` carries a live `anyhow::Error` source rendered as the real error chain with the existing styled wrapper and class-default remediation; `app_support` is the sole owner of the distinct terminal paths, and `sce sync` classifies authentication, credential-storage, and all other sync failures as cataloged user errors while preserving their technical sources for observability. 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/sce/cli-error-code-taxonomy.md b/context/sce/cli-error-code-taxonomy.md index 0c645724..33d9a685 100644 --- a/context/sce/cli-error-code-taxonomy.md +++ b/context/sce/cli-error-code-taxonomy.md @@ -18,7 +18,7 @@ It complements the numeric process exit-code classes documented in `context/sce/ - `CliError::Internal` diagnostics are emitted on `stderr` as the styled `Error []: ` wrapper. - Before stderr emission, all `CliError` instances are logged via `Logger::log_cli_error()` with event ID `sce.error.{code}` and fields `error_code`, `error_class`. - For `CliError::Internal`, if the rendered message does not already include `Try:`, runtime appends class-default remediation guidance; if it already contains `Try:`, runtime preserves the original remediation text and does not append a second one. -- For `CliError::User`, runtime renders the catalog message from `UserError` without technical source text or class-default `Try:` remediation. The `UserError::UnexpectedFailure` entry renders the fixed message `An unexpected error occurred. Check the log files for more details.` without dynamic path interpolation. +- For `CliError::User`, runtime renders the catalog message from `UserError` without technical source text or class-default `Try:` remediation. The `UserError::NotGitRepository` entry renders the fixed setup guidance `This directory is not a Git repository. Run \`git init\`, then rerun \`sce setup\`.`. The `UserError::UnexpectedFailure` entry renders the fixed message `An unexpected error occurred. Check the log files for more details.` without dynamic path interpolation. - Diagnostic text is still redaction-filtered through `services::security::redact_sensitive_text` before emission. ## Actionable parser/invocation guidance contract @@ -32,8 +32,8 @@ 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`, `AuthStorageUnavailable`, or `UnexpectedFailure`) for expected, deliberately-explained failures; `CliError::Internal` carries a live `anyhow::Error` source for every other failure. `CliError::User` may also carry an optional preserved technical `source`, kept for observability only and never rendered to the terminal. -- `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 is a fixed, reviewed sentence returned by `UserError::message()`, keyed for structured logging by `UserError::key()`. `NotAuthenticated` (`auth.not_authenticated`) covers missing credentials from `sce auth logout`/`whoami` and Control Plane authentication failures from `whoami`; `AuthStorageUnavailable` (`auth.storage_unavailable`) is used by `sce sync` and the `sce auth login`, `logout`, and `whoami` command boundary for token-storage plus `AuthError::Io`/`Storage` failures. Both auth-command mappings preserve technical sources for observability. `UnexpectedFailure` (`general.unexpected_failure`) is used by `sce sync` for its default failure classification and by the auth-command boundary for all non-storage failures, including rendering, prompt, runtime, configuration, and unrelated Control Plane failures; it renders one fixed, user-safe diagnostic sentence and has no automatic `Try:` suffix or dynamic path input. +- `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`, `AuthStorageUnavailable`, `NotGitRepository`, or `UnexpectedFailure`) for expected, deliberately-explained failures; `CliError::Internal` carries a live `anyhow::Error` source for every other failure. `CliError::User` may also carry an optional preserved technical `source`, kept for observability only and never rendered to the terminal. +- `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 is a fixed, reviewed sentence returned by `UserError::message()`, keyed for structured logging by `UserError::key()`. `NotAuthenticated` (`auth.not_authenticated`) covers missing credentials from `sce auth logout`/`whoami` and Control Plane authentication failures from `whoami`; `AuthStorageUnavailable` (`auth.storage_unavailable`) is used by `sce sync` and the `sce auth login`, `logout`, and `whoami` command boundary for token-storage plus `AuthError::Io`/`Storage` failures. Both auth-command mappings preserve technical sources for observability. `NotGitRepository` (`setup.not_git_repository`) is used by the setup command when repository-root resolution fails and renders fixed Git-init/rerun guidance; its technical source is preserved for observability. `UnexpectedFailure` (`general.unexpected_failure`) is used by `sce sync`, the config-command boundary for config execution failures, the version and doctor command boundaries for service execution failures, the auth-command boundary, and remaining setup execution failures; it renders one fixed, user-safe diagnostic sentence and has no automatic `Try:` suffix or dynamic path input. - Command and domain layers construct and return a `CliError`; they do not format terminal text or apply styling. Auth command orchestration classifies expected authentication and credential-storage failures into the existing `UserError` catalog by typed domain variants, never by string matching, and preserves the original technical chain as the optional user-error source. `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 stderr rendering: it redacts and writes the catalog variant's message without a wrapper or styling, while `CliError::Internal` retains code-bearing rendering and styles its 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/setup-githooks-cli-ux.md b/context/sce/setup-githooks-cli-ux.md index 4350f1ff..401cc00f 100644 --- a/context/sce/setup-githooks-cli-ux.md +++ b/context/sce/setup-githooks-cli-ux.md @@ -25,7 +25,7 @@ Validation is deterministic and enforced during setup option resolution: - `--hooks` can be combined with exactly one target flag to run config install and required-hook install in one invocation - `--repo` may only be provided once and must include a value - `--repo` path is canonicalized and must resolve to an existing directory before hook setup runs -- repository-required hook flows fail before config or hook writes when the target directory is not a git repository, with actionable guidance to run `git init` and rerun `sce setup` +- repository-required hook flows fail before config or hook writes when the target directory is not a Git repository, rendering `This directory is not a Git repository. Run \`git init\`, then rerun \`sce setup\`.` from the closed runtime user-error catalog; the technical repository-resolution source is retained for observability - all `sce setup` modes (config-only, hooks-only, combined, and interactive) require the current directory to be inside a git repository before any setup writes begin; the `ensure_git_repository` preflight check in `cli/src/app.rs` enforces this gate consistently across all invocation shapes Target-install mode contract: From 238057962cd5288335f899d62cc9db6c18590243 Mon Sep 17 00:00:00 2001 From: David Abram Date: Wed, 26 Aug 2026 12:41:44 +0200 Subject: [PATCH 4/8] runtime+context: Propagate storage failures through sync streams Classify credential-storage failures from stream batches and refreshes as `auth.storage_unavailable` instead of generic unexpected failures while preserving the typed technical source for observability. Add focused terminal and refresh coverage and update the sync error documentation. Plan: fix-pr-223-error-classification-regressions (T01) Co-authored-by: SCE --- cli/src/services/sync/command.rs | 15 ++- cli/src/services/sync/sync.rs | 8 +- context/cli/agent-trace-sync-command.md | 2 +- context/cli/sync-command.md | 18 ++-- ...pr-223-error-classification-regressions.md | 97 +++++++++++++++++++ 5 files changed, 124 insertions(+), 16 deletions(-) create mode 100644 context/plans/fix-pr-223-error-classification-regressions.md diff --git a/cli/src/services/sync/command.rs b/cli/src/services/sync/command.rs index d007a89c..5df113a0 100644 --- a/cli/src/services/sync/command.rs +++ b/cli/src/services/sync/command.rs @@ -193,13 +193,24 @@ mod tests { } #[test] - fn stream_storage_failure_does_not_classify_as_storage_unavailable() { + fn stream_terminal_storage_failure_classifies_as_storage_unavailable() { assert_user_error( TraceSyncError::Stream { stream: "prompts", source: StreamSyncError::Terminal(ControlPlaneError::Storage("disk".to_string())), }, - "general.unexpected_failure", + "auth.storage_unavailable", + ); + } + + #[test] + fn stream_refresh_storage_failure_classifies_as_storage_unavailable() { + assert_user_error( + TraceSyncError::Stream { + stream: "prompts", + source: StreamSyncError::Refresh(ControlPlaneError::Storage("disk".to_string())), + }, + "auth.storage_unavailable", ); } } diff --git a/cli/src/services/sync/sync.rs b/cli/src/services/sync/sync.rs index c5930539..05d2883e 100644 --- a/cli/src/services/sync/sync.rs +++ b/cli/src/services/sync/sync.rs @@ -147,12 +147,14 @@ impl TraceSyncError { } } - /// True when the initial control-plane failure came from local credential - /// storage. Stream failures never carry storage errors. + /// True when the failure came from local credential storage, whether it + /// surfaced during the initial state request or a stream batch/refresh + /// path. pub fn is_storage_failure(&self) -> bool { match self { Self::ControlPlane(error) => error.is_storage_failure(), - Self::Runtime(_) | Self::Stream { .. } => false, + Self::Stream { source, .. } => source.is_storage_failure(), + Self::Runtime(_) => false, } } } diff --git a/context/cli/agent-trace-sync-command.md b/context/cli/agent-trace-sync-command.md index 8ac78c1f..6c0b1831 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 failure classification:** `ControlPlaneError::is_authentication_failure()` is `true` only for `MissingCredentials`/`AuthenticationFailed`, while `ControlPlaneError::is_storage_failure()` identifies local credential-storage failures. `TraceSyncError` uses the storage predicate only for a direct initial control-plane failure; `StreamSyncError::is_storage_failure()` delegates storage classification for `Refresh` and `Terminal` failures, while other stream error variants return false. 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 these predicates (never string/substring matching) to route authentication failures from the initial `/state` or stream paths to `CliError::User { error: UserError::NotAuthenticated, .. }` and credential-storage failures from the initial `/state` call to `CliError::User { error: UserError::AuthStorageUnavailable, .. }`; every other `ControlPlaneError` variant (`Forbidden`, `BadRequest`, `Transport`, `ServerError`, `InvalidResponse`, `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 failure classification:** `ControlPlaneError::is_authentication_failure()` is `true` only for `MissingCredentials`/`AuthenticationFailed`, while `ControlPlaneError::is_storage_failure()` identifies local credential-storage failures. `TraceSyncError` uses the storage predicate for direct initial control-plane failures and stream failures; `StreamSyncError::is_storage_failure()` delegates storage classification for `Refresh` and `Terminal` failures, while other stream error variants return false. 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 these predicates (never string/substring matching) to route authentication failures from the initial `/state` or stream paths to `CliError::User { error: UserError::NotAuthenticated, .. }` and credential-storage failures from the initial `/state`, stream batch, or stream refresh paths to `CliError::User { error: UserError::AuthStorageUnavailable, .. }`; every other `ControlPlaneError` variant (`Forbidden`, `BadRequest`, `Transport`, `ServerError`, `InvalidResponse`, `Protocol`) maps to `CliError::User { error: UserError::UnexpectedFailure, .. }`, with the full technical chain preserved as its optional 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 f6da87f4..d19426ae 100644 --- a/context/cli/sync-command.md +++ b/context/cli/sync-command.md @@ -108,17 +108,15 @@ 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, .. }`. A credential -storage failure (`ControlPlaneError::Storage`) from the initial `/state` call -classifies as `CliError::User { error: UserError::AuthStorageUnavailable, .. }`. -Stream failures never classify as credential-storage user errors; their -authentication failures still use `NotAuthenticated`. Both user cases preserve -the technical error as their optional source. Every other `ControlPlaneError` -(`Forbidden`, `BadRequest`, `Transport`, `ServerError`, `InvalidResponse`, -`Protocol`) and runtime failures classify as +storage failure (`ControlPlaneError::Storage`) from the initial `/state` call, +a stream batch request, or a stream reconciliation `/state` refresh classifies +as `CliError::User { error: UserError::AuthStorageUnavailable, .. }`. +Stream authentication failures still use `NotAuthenticated`. Both user cases +preserve the technical error as their optional source. Every other +`ControlPlaneError` (`Forbidden`, `BadRequest`, `Transport`, `ServerError`, +`InvalidResponse`, `Protocol`) and runtime failures classify as `CliError::User { error: UserError::UnexpectedFailure, .. }`; the technical -source remains available for observability. Stream credential-storage failures -also use `UnexpectedFailure`, because storage classification applies only to -the initial control-plane failure. +source remains available for observability. `sync/command.rs` builds no friendly sentence and applies no terminal styling itself — `app_support` renders the catalog message for user cases. See [CLI error-code taxonomy](../sce/cli-error-code-taxonomy.md) for the full `CliError`/`UserError` diff --git a/context/plans/fix-pr-223-error-classification-regressions.md b/context/plans/fix-pr-223-error-classification-regressions.md new file mode 100644 index 00000000..d2230777 --- /dev/null +++ b/context/plans/fix-pr-223-error-classification-regressions.md @@ -0,0 +1,97 @@ +# Plan: fix-pr-223-error-classification-regressions + +## Change summary + +Fix the three semantic regressions introduced by PR #223 at `b57c6a8f7400afb947fc0417abf381ac8a5868db`, without redesigning the typed `CliError`/closed `UserError` architecture. The work preserves technical error sources for observability, keeps classification in typed domain boundaries, and restores existing command output/exit semantics where an unauthenticated state is a successful query. + +The fixes are deliberately split into three independently testable atomic commits: propagate credential-storage classification through sync streams; type setup repository-root resolution before the CLI boundary; and restore idempotent `auth logout` plus unauthenticated `auth whoami` success paths while retaining typed mappings for genuine failures. + +## Acceptance criteria + +- [ ] AC1: Initial control-plane, stream-terminal, and stream-refresh `ControlPlaneError::Storage` failures all classify as `auth.storage_unavailable`; stream authentication remains `auth.not_authenticated`; other control-plane/runtime failures remain `general.unexpected_failure`, with technical `TraceSyncError` sources attached. + - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml sync::command` and `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml sync::`; inspect the classifier to confirm it remains unchanged and uses typed predicates rather than human-readable strings. +- [ ] AC2: Setup emits `setup.not_git_repository` only when the setup domain positively identifies a target as outside a Git repository; nonexistent, inaccessible, process, malformed-output, and unrelated filesystem failures classify as `general.unexpected_failure`, and both typed paths preserve technical sources. + - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml setup::`; inspect the setup classifier for typed `GitRepositoryResolutionError` matching with no CLI-layer string matching. +- [ ] AC3: `sce auth logout` with no stored credentials succeeds with the existing text and JSON state-query semantics, including `credentials_removed: false`; deleting stored credentials still succeeds with `credentials_removed: true`. + - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auth_command::` plus focused text/JSON assertions for absent and present credentials. +- [ ] AC4: `sce auth whoami` with no stored credentials succeeds with the existing unauthenticated text guidance and JSON payload (`authentication_state: unauthenticated`, `has_stored_credentials: false`), while authenticated `/me` failures retain typed `NotAuthenticated`, `AuthStorageUnavailable`, or `UnexpectedFailure` mappings and technical sources. + - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auth_command::` plus focused missing-credential and authenticated-failure assertions. +- [ ] AC5: Genuine auth storage failures retain `auth.storage_unavailable`, stored credentials rejected by the Control Plane retain `auth.not_authenticated`, and all genuine failures retain exit code `4`, stdout/stderr routing, and machine-readable JSON contracts. + - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auth_command::` and `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml app_support::`. +- [ ] AC6: The closed `UserError` catalog and typed-error architecture remain intact: no arbitrary message variant, no CLI-boundary human-readable string classification, no rollback to the pre-PR architecture, and no new ADR for this regression repair. + - Validate: inspect `cli/src/services/error.rs`, `cli/src/services/sync/command.rs`, and `cli/src/services/setup/command.rs`; confirm no `UserError::Message`/`Custom` variant and no CLI-layer error-string matching. +- [ ] AC7: Durable context accurately documents sync storage propagation, positive-only setup repository classification, and successful unauthenticated auth state queries, with no stale claim that missing logout/whoami credentials are `NotAuthenticated` failures. + - Validate: `nix run .#pkl-check-generated` and targeted inspection of the context files listed under Context sync. + +### Full validation + +- `./scripts/run-cli-cargo.sh fmt --manifest-path cli/Cargo.toml -- --check` +- `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml -- -D warnings` +- `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml` +- `nix run .#pkl-check-generated` +- `nix flake check` + +### Context sync + +- `context/cli/sync-command.md` — storage failures classify consistently across initial state resolution and stream execution. +- `context/cli/cli-command-surface.md` — setup repository classification and successful logged-out auth state-query behavior. +- `context/sce/cli-error-code-taxonomy.md` — positive-only `NotGitRepository` semantics and the distinction between unauthenticated state observation and authentication failure. +- `context/architecture.md` — corrected auth/setup boundary behavior where its current summary is stale. + +## 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/sync.rs`; `cli/src/services/sync/command.rs` tests; `cli/src/services/agent_trace_sync/mod.rs` comments/predicates as needed; `cli/src/services/setup/mod.rs`; `cli/src/services/setup/command.rs`; `cli/src/services/auth_command/mod.rs` and its focused tests; the listed durable context files. +- **Out of scope:** redesigning `CliError` or `UserError`; adding arbitrary user-message variants; broad typing of unrelated setup errors; changing `classify_sync_error`; changing genuine failure exit codes; changing machine-readable JSON contracts except to restore the documented successful logout/whoami payloads; creating an ADR; unrelated PR #223 cleanup. +- **Constraints:** preserve the closed `UserError` catalog; preserve technical sources; classify by typed variants/predicates at domain boundaries, never by matching human-readable strings at the CLI boundary; use no new dependency; keep each task independently testable and suitable for one atomic commit; retain the existing `4` runtime exit code for genuine failures. +- **Non-goal:** generalize repository-resolution typing to every setup operation or alter the typed-error architecture beyond these three regressions. + +## Assumptions + +- The suggested `GitRepositoryResolutionError` name and exact internal helper names are flexible; the repository's existing Rust naming and error conventions decide those local details. +- Auth tests may add a narrow pure/injected orchestration seam analogous to the existing auth dispatch test seam so missing-credential branches can be tested deterministically without relying on process-global encrypted storage; production storage behavior remains unchanged. +- The requested text and JSON outputs are the existing `main` semantics described in the request; successful logout with credentials retains its current success output while absent credentials render the existing no-user state. + +## Task stack + +- [x] T01: `Propagate credential-storage classification through stream sync errors` (status:done) + - Task ID: T01 + - Scope: In — change `TraceSyncError::is_storage_failure()` to traverse `StreamSyncError`, update its comment, and replace the regression test with terminal and refresh stream-storage cases that classify as `auth.storage_unavailable`; retain authentication, runtime, and other control-plane cases plus source-preservation assertions; update `context/cli/sync-command.md` to document storage classification across initial state, batch execution, and refresh. Out — changing `classify_sync_error()` or sync error architecture. + - Dependencies: none + - Done when: initial, terminal-stream, and refresh-stream `ControlPlaneError::Storage` all reach `UserError::AuthStorageUnavailable`, authentication classification is unchanged, technical sources remain attached, and the sync context rule is truthful. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml sync::command` — pass (9 tests); `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml sync::` — pass (66 tests). + - Context synchronization: synced + - Completed: 2026-08-26 + - Files changed: `cli/src/services/sync/sync.rs`, `cli/src/services/sync/command.rs`, `context/cli/sync-command.md` + - Result: Stream credential-storage failures now propagate through the typed sync error predicate and classify as `auth.storage_unavailable`; terminal and refresh cases are covered by focused tests with preserved technical sources. + - Context impact: domain — `context/cli/sync-command.md` now accurately documents typed credential-storage classification across all sync failure paths; no root context files require changes. + +- [ ] T02: `Type setup repository-root resolution before the CLI boundary` (status:todo) + - Task ID: T02 + - Scope: In — introduce a narrow setup-owned `GitRepositoryResolutionError` distinguishing positively identified non-Git directories from unexpected resolution failures; preserve the original technical source through `Display`/`Error`; return it from `ensure_git_repository`; classify it in `setup/command.rs` as `NotGitRepository` or `UnexpectedFailure`; add real non-Git-directory, nonexistent-path, and source-preservation tests; update setup taxonomy/context wording. Out — typing every later setup operation, changing setup success behavior, or matching strings in the command layer. + - Dependencies: none + - Done when: a valid temporary non-Git directory maps to `setup.not_git_repository`, a definitely nonexistent path maps to `general.unexpected_failure`, both `CliError::User` variants contain technical sources, and only the setup domain recognizes Git's diagnostic. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml setup::`; `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml -- -D warnings`. + - Context synchronization: pending + +- [ ] T03: `Restore idempotent auth state-query semantics` (status:todo) + - Task ID: T03 + - Scope: In — restore `render_logout_result(deleted, format)` and make absent-token logout a successful result; add `render_unauthenticated_whoami(format)` and make missing credentials a successful unauthenticated-state result; retain typed storage and authenticated Control Plane mappings, technical sources, existing successful JSON fields, and genuine failure behavior; add focused text/JSON tests for missing and removed credentials plus authenticated failure tests; update auth command surface, taxonomy, and architecture context wording. Out — changing login renewal/device flow, adding a new user-error catalog entry, or creating an ADR. + - Dependencies: none + - Done when: missing-token logout and whoami return `Ok(...)` with their existing text/JSON contracts, token deletion still reports success, authenticated `/me` and storage failures retain their typed errors and sources, and context no longer claims that observing logged-out state is `NotAuthenticated`. + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auth_command::`; `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml app_support::`. + - Context synchronization: pending + +## Open questions + +None. The request specifies the three regressions, the required typed boundaries, preserved contracts, tests, context updates, atomic commit messages, and final validation commands. The code inspection confirms the regressions are present at the stated PR head; no smaller change covers all three independent user-visible failures. From fbce55ca4fb3fd24b90bd5df662666b1d6f66fd6 Mon Sep 17 00:00:00 2001 From: David Abram Date: Wed, 26 Aug 2026 12:53:57 +0200 Subject: [PATCH 5/8] setup: Classify repository resolution failures before the CLI boundary Distinguish Git-confirmed non-repository directories from unexpected resolution failures so setup reports the correct stable user error without misclassifying missing or inaccessible paths. Preserve the technical source through the typed setup error and add focused classification and source-preservation tests. Plan: fix-pr-223-error-classification-regressions (T02) Co-authored-by: SCE --- cli/src/services/setup/command.rs | 63 ++++++++++++- cli/src/services/setup/mod.rs | 89 +++++++++++++++---- context/architecture.md | 2 +- context/cli/cli-command-surface.md | 2 +- context/glossary.md | 5 +- ...pr-223-error-classification-regressions.md | 10 ++- context/sce/cli-error-code-taxonomy.md | 2 +- 7 files changed, 147 insertions(+), 26 deletions(-) diff --git a/cli/src/services/setup/command.rs b/cli/src/services/setup/command.rs index ede78032..c81aa981 100644 --- a/cli/src/services/setup/command.rs +++ b/cli/src/services/setup/command.rs @@ -23,7 +23,7 @@ impl SetupCommand { // The repository root is resolved before any prompt so the interactive // optional-workflow prompt can pre-check the persisted selection. let repository_root = setup::ensure_git_repository(&setup_start_path) - .map_err(|source| CliError::user_with_source(UserError::NotGitRepository, source))?; + .map_err(classify_repository_resolution_error)?; let setup_dispatch = if self.request.context_only { None @@ -102,6 +102,17 @@ impl SetupCommand { } } +fn classify_repository_resolution_error( + source: setup::GitRepositoryResolutionError, +) -> CliError { + let user_error = match &source { + setup::GitRepositoryResolutionError::NotGitRepository(_) => UserError::NotGitRepository, + setup::GitRepositoryResolutionError::Unexpected(_) => UserError::UnexpectedFailure, + }; + + CliError::user_with_source(user_error, source) +} + fn unexpected_failure(source: impl Into) -> CliError { CliError::user_with_source(UserError::UnexpectedFailure, source) } @@ -130,3 +141,53 @@ fn setup_required_hooks_outcome_from_lifecycle( .collect(), } } + +#[cfg(test)] +mod tests { + use super::*; + use std::time::{SystemTime, UNIX_EPOCH}; + + fn unique_temp_dir(label: &str) -> std::path::PathBuf { + let nonce = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("system time should be after Unix epoch") + .as_nanos(); + let directory = std::env::temp_dir().join(format!( + "sce-setup-command-{label}-{}-{nonce}", + std::process::id() + )); + std::fs::create_dir_all(&directory).expect("create temporary directory"); + directory + } + + #[test] + fn repository_resolution_classification_preserves_technical_sources() { + let non_git_directory = unique_temp_dir("source-non-git"); + let non_git_error = setup::ensure_git_repository(&non_git_directory) + .expect_err("a non-Git directory should fail resolution"); + let non_git_cli_error = classify_repository_resolution_error(non_git_error); + + match non_git_cli_error { + CliError::User { + error: UserError::NotGitRepository, + source: Some(source), + } => assert!(source.to_string().contains("not a git repository")), + other => panic!("expected a sourced NotGitRepository error, got {other:?}"), + } + let _ = std::fs::remove_dir_all(&non_git_directory); + + let nonexistent_directory = unique_temp_dir("source-nonexistent"); + std::fs::remove_dir_all(&nonexistent_directory).expect("remove temporary directory"); + let unexpected_error = setup::ensure_git_repository(&nonexistent_directory) + .expect_err("a nonexistent path should fail resolution"); + let unexpected_cli_error = classify_repository_resolution_error(unexpected_error); + + match unexpected_cli_error { + CliError::User { + error: UserError::UnexpectedFailure, + source: Some(source), + } => assert!(source.to_string().contains("No such file or directory")), + other => panic!("expected a sourced UnexpectedFailure error, got {other:?}"), + } + } +} diff --git a/cli/src/services/setup/mod.rs b/cli/src/services/setup/mod.rs index 76e0d625..f2493585 100644 --- a/cli/src/services/setup/mod.rs +++ b/cli/src/services/setup/mod.rs @@ -23,6 +23,33 @@ fn repo_local_config_bootstrap_payload() -> String { pub const NAME: &str = "setup"; +/// Classifies repository-root resolution failures while retaining the +/// underlying technical error for the CLI's observability boundary. +#[derive(Debug)] +pub enum GitRepositoryResolutionError { + /// Git positively identified the target as outside a repository. + NotGitRepository(anyhow::Error), + /// Resolution failed for an unexpected filesystem, process, or output + /// reason. + Unexpected(anyhow::Error), +} + +impl std::fmt::Display for GitRepositoryResolutionError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::NotGitRepository(source) | Self::Unexpected(source) => write!(f, "{source:#}"), + } + } +} + +impl std::error::Error for GitRepositoryResolutionError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::NotGitRepository(source) | Self::Unexpected(source) => Some(source.as_ref()), + } + } +} + #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum SetupTarget { OpenCode, @@ -418,9 +445,11 @@ pub fn persisted_optional_workflows(repository_root: &Path) -> Vec { } /// Preflight check that verifies the given directory is inside a git repository. -/// Returns the resolved repository root path on success. -/// Returns an actionable error telling the operator to run `git init` on failure. -pub fn ensure_git_repository(directory: &Path) -> Result { +/// Returns the resolved repository root path on success, or a typed error that +/// distinguishes a Git-confirmed non-repository directory from other failures. +pub fn ensure_git_repository( + directory: &Path, +) -> std::result::Result { install::ensure_git_repository(directory) } @@ -855,6 +884,7 @@ mod install { cleanup_path_if_exists, concrete_targets_for, embedded_assets_for_concrete_target, hook_install_recovery_guidance, iter_embedded_assets_for_setup_target_with_selection, iter_required_hook_assets, setup_install_recovery_guidance, EmbeddedAsset, + GitRepositoryResolutionError, RequiredHookInstallResult, RequiredHookInstallStatus, RequiredHooksInstallOutcome, SetupInstallOutcome, SetupInstallTargetResult, SetupTarget, }; @@ -863,10 +893,12 @@ mod install { pub(super) fn prepare_setup_hooks_repository(repository_root: &Path) -> Result { let normalized_repository_root = normalize_user_repository_path(repository_root)?; - resolve_git_repository_root(&normalized_repository_root) + Ok(resolve_git_repository_root(&normalized_repository_root)?) } - pub(super) fn ensure_git_repository(directory: &Path) -> Result { + pub(super) fn ensure_git_repository( + directory: &Path, + ) -> std::result::Result { resolve_git_repository_root(directory) } @@ -1128,7 +1160,9 @@ mod install { Ok(canonical_repository_root) } - fn resolve_git_repository_root(repository_root: &Path) -> Result { + fn resolve_git_repository_root( + repository_root: &Path, + ) -> std::result::Result { let repository_root_output = run_git_command_in_directory( repository_root, &["rev-parse", "--show-toplevel"], @@ -1139,18 +1173,13 @@ mod install { } fn map_setup_non_git_repository_error( - repository_root: &Path, + _repository_root: &Path, error: anyhow::Error, - ) -> anyhow::Error { - let message = error.to_string(); - if message.contains("not a git repository") { - anyhow::anyhow!( - "Directory '{}' is not a git repository. Try: run 'git init' in '{}', then rerun 'sce setup'.", - repository_root.display(), - repository_root.display() - ) + ) -> GitRepositoryResolutionError { + if error.to_string().contains("not a git repository") { + GitRepositoryResolutionError::NotGitRepository(error) } else { - error + GitRepositoryResolutionError::Unexpected(error) } } @@ -1882,6 +1911,34 @@ mod tests { } } + #[test] + fn ensure_git_repository_classifies_real_non_git_directory() { + let directory = unique_temp_dir("non-git-directory"); + + let error = ensure_git_repository(&directory) + .expect_err("a directory without .git should be classified as non-Git"); + assert!(matches!( + &error, + GitRepositoryResolutionError::NotGitRepository(_) + )); + assert!(error.to_string().contains("not a git repository")); + assert!(std::error::Error::source(&error).is_some()); + + let _ = fs::remove_dir_all(&directory); + } + + #[test] + fn ensure_git_repository_classifies_nonexistent_path_as_unexpected() { + let directory = unique_temp_dir("nonexistent"); + fs::remove_dir_all(&directory).expect("remove temporary directory"); + + let error = ensure_git_repository(&directory) + .expect_err("a nonexistent path should remain an unexpected failure"); + assert!(matches!(&error, GitRepositoryResolutionError::Unexpected(_))); + assert!(error.to_string().contains("No such file or directory")); + assert!(std::error::Error::source(&error).is_some()); + } + #[test] fn resolve_setup_request_accepts_pi_target() { let request = resolve_setup_request(options_with(|options| { diff --git a/context/architecture.md b/context/architecture.md index 49a92089..b34e87bc 100644 --- a/context/architecture.md +++ b/context/architecture.md @@ -126,7 +126,7 @@ The repository includes a new placeholder Rust binary crate at `cli/`. - `cli/src/services/local_db/mod.rs` provides the concrete local DB spec and `LocalDb` type alias over the shared generic `TursoDb` adapter. `LocalDbSpec` resolves the deterministic persistent runtime DB target through the shared default-path seam and declares no local migrations; `TursoDb` supplies retry-backed blocking `execute`/`query`, parent-directory creation, Turso connection setup, tokio current-thread runtime bridging, and generic migration execution. - `cli/src/services/auth_db/mod.rs` provides the encrypted auth DB spec and `AuthDb` type alias over `EncryptedTursoDb`. `AuthDbSpec` resolves `/sce/auth.db` through the shared default-path seam and embeds ordered auth migrations. Auth DB lifecycle setup/doctor integration is wired through `AuthDbLifecycle`; auth command/token-storage reads/writes are directed through `token_storage.rs`. - `cli/src/services/agent_trace_db/mod.rs` owns the shared Agent Trace insert payloads, SQL constants, and typed row helpers (diff-trace/intersection/Agent Trace/message/part) plus `ensure_schema_ready_for_hooks()` consumed by the repository adapter. `cli/src/services/agent_trace_db/repository.rs` defines the sole `RepositoryAgentTraceDb` adapter over `TursoDb` with one fresh `agent-trace-repository/001_repository_schema.sql` baseline for `diff_traces` (including `payload_type`), `post_commit_patch_intersections`, `agent_traces`, `messages`, `parts`, indexes, and triggers, `repository_metadata` validation, no trace-table `checkout_id` columns, `agent_traces.agent_trace_id NOT NULL UNIQUE`, and `recent_diff_trace_patches(cutoff_time_ms, end_time_ms)` using the inclusive chronological parser without checkout filtering; structured-row reconstruction applies the persisted row `model_id` to every hunk and the persisted canonical `session_id` to every touched line before downstream combination and intersection. Active hook runtime, setup/lifecycle storage, and `sce sync` resolve through `agent_trace_storage` and use `RepositoryAgentTraceDb`. The checkout-scoped `AgentTraceDb`/`AgentTraceDbSpec` adapter, its `agent_trace_db_path()`/`agent_trace_db_path_for_checkout()` helpers, the 15-file `cli/migrations/agent-trace/` chain, and the former `sce trace --legacy` surface were removed by the `retire-legacy-agent-trace-db` plan. -- `cli/src/services/setup/mod.rs` defines the setup command contract (`SetupMode`, `SetupTarget`, `SetupRequest`, CLI flag parser/validator), an `inquire`-backed interactive target prompter (`InquireSetupTargetPrompter`), setup dispatch outcomes (proceed/cancelled), additive durable-context bootstrap (`bootstrap_context_baseline` for standalone `--bootstrap-context` and every normal successful setup path), and compile-time embedded asset access (`EmbeddedAsset`, target-scoped iterators, required-hook asset iterators/lookups). It also owns the install-time optional-workflow seam: the `OptionalWorkflow` type plus the build-generated `OPTIONAL_WORKFLOWS` catalog, a per-target `WorkflowAssetLayout` built from the existing `default_paths` command/skill directory constants (`command`/`commands`/`prompts` plus `skills`), and `iter_embedded_assets_for_setup_target_with_selection(target, selection)`, which yields every embedded asset except the `{command_dir}/{command_slug}.md` file and `{skills_dir}/{skill_slug}/` subtree of each optional workflow the selection omits. Membership is derived from the catalog's slugs rather than an enumerated file list, so a new optional workflow needs no Rust change. This filtered iterator is the only way embedded assets are enumerated; setup installs through it and doctor inspects through it, so there is no unfiltered enumeration path that could reintroduce an unselected workflow. The non-interactive selection flows through the repeatable `--workflow ` flag into `SetupRequest.optional_workflows: Option>` (`None` meaning the flag was absent); `validate_optional_workflow_slugs` checks each slug against `OPTIONAL_WORKFLOWS` during request resolution, before any write, and `run_setup_for_mode` resolves `None` to the persisted `integrations.optional_workflows` (exported as `persisted_optional_workflows`) before installing through the filtered iterator and persisting the resolved selection. The interactive selection flows through the same seam: `SetupTargetPrompter` carries `prompt_target` plus `prompt_optional_workflows(defaults)` (returning `None` for a cancelled prompt), `SetupDispatch::Proceed { mode, optional_workflows }` carries a prompted selection alongside the resolved mode, and `resolve_setup_dispatch(mode, prompter, defaults)` runs the workflow prompt only after an interactive target prompt, mapping either cancellation to `SetupDispatch::Cancelled`. The prompt module builds its `inquire::MultiSelect` from `optional_workflow_prompt_inputs(catalog, defaults)`, which returns `None` for an empty catalog (skipping the prompt) and otherwise catalog-ordered rows plus the indices to pre-check, ignoring ids absent from the catalog. `setup/command.rs` therefore resolves the repository root before dispatch, so a non-git directory fails before any prompt. For repository builds, `cli/build.rs` validates the `SCE_CLI_GENERATED_INPUT_DIR` payload and canonical-input inventories, copies the payload into Cargo `OUT_DIR/pkl-generated`, stages `cli/assets/hooks/**` under `OUT_DIR/static`, requires the staged `config/optional-workflows.json`, and generates both the setup manifest and the optional-workflow catalog (`optional_workflows.rs`, rejecting a manifest whose `schemaVersion` is not 1 or whose entries lack a non-empty `id`/`title`/`description`/`commandSlug`/`skillSlug`) in `OUT_DIR`; focused internal seams separate install-flow from prompt-flow logic; `cli/src/services/setup/command.rs` owns the `SetupCommand` payload used by the static `RuntimeCommand` enum and executes against any context implementing repo-root scoping. Its install engine/orchestrator (`install_assets_for_concrete_target_with_rename`) installs each embedded asset individually: it stages the asset's content next to its final destination and swaps it into place by renaming the staging file directly over the destination — never unlinking the destination first, since `fs::rename` already replaces an existing file atomically — with deterministic recovery guidance naming the failing asset's path on swap failure (the pre-existing destination content, if any, is untouched) and no backup artifact creation; it never removes an integration target directory as a whole, so files a repository owns inside `.opencode`/`.claude`/`.pi` — including nested inside an SCE-owned subdirectory such as `skills/` or `commands/` — survive a setup run; Codex has no single target directory this way, since `CODEX_EMBEDDED_ASSETS` relative paths already carry their own `.agents/`/`.codex/` prefix and `InstallTargetPaths::codex_target_dir()` resolves to the repository root itself, but the same per-asset stage/atomic-swap and never-delete-what-it-did-not-author guarantees still apply file by file. For the three assets that are merge targets — the Claude target's `settings.json`, the OpenCode target's `opencode.json`, and Codex's user-owned `.codex/hooks.json` — the content staged is not always the embedded asset's bytes. Claude and OpenCode use `cli/src/services/setup/config_merge.rs`, while Codex uses the shared `cli/src/services/codex_hook_config.rs` service for structural validation and canonical-registration merging. For the Claude and OpenCode targets, the content staged is the result of `cli/src/services/setup/config_merge.rs::merge_or_create_claude_settings`/`merge_or_create_opencode_config(existing_bytes, generated_bytes, source_path)`: each returns the generated document verbatim when no file exists yet, otherwise parses the existing file as JSON (a parse failure is a hard error naming `source_path`, and nothing is written) and merges it with a pure per-shape function that copies `$schema` from the generated document and preserves every other key from the existing file untouched. `merge_claude_settings` replaces, per hook event key the generated document declares, only the entries whose command contains the ownership marker `run-sce-or-show-install-guidance.sh`, preserving every event key and non-SCE hook entry from the existing file. `merge_opencode_config` merges the `plugin` array as a set: existing entries whose path starts with the ownership marker `./plugins/sce-` are dropped structurally — so a stale plugin path an older or renamed catalog once installed is still recognized and pruned even after the current generated document stops declaring it — and the generated document's `plugin` entries are appended after the surviving entries. Codex's merge validates the document shape, recognizes ownership only when both the generated helper path and the `sce hooks codex` command contract are present, and replaces stale or duplicate SCE handlers with exactly one current handler for each `UserPromptSubmit`, `Stop`, `PreToolUse/Bash`, and `PostToolUse/apply_patch` registration while preserving unrelated valid Codex fields, groups, and handlers. After that per-asset install loop, `prune_stale_assets_for_concrete_target` deletes every path the full embedded catalog for the concrete target claims but the resolved selection did not install (a deselected optional workflow, or an asset a newer catalog renamed or dropped), and `remove_empty_ancestor_directories` removes any parent directory left empty by that deletion, stopping at the target root or at a directory that still holds something such as a user file. It formats deterministic completion messaging; required-hook install orchestration (`install_required_git_hooks`, backed by the rename-injectable `install_required_git_hooks_with_rename`) is a third content-computation seam alongside the two JSON merge targets: `install_single_required_hook_with_rename` computes the bytes to stage with `cli/src/services/setup/hook_merge.rs::merge_or_create_hook(existing_bytes, canonical_bytes, hook_name)` rather than writing `hook_asset.bytes` verbatim — a foreign hook (no SCE managed block, no legacy guidance-URL marker) is kept as an exact byte prefix with the canonical block appended after it, an SCE-owned hook has only its block spliced in place or left unchanged, and a legacy pre-marker hook is replaced wholesale — then follows the same per-file stage/atomic-swap choreography as config-asset install: the staging file is renamed directly over the existing hook without unlinking it first, so a rename failure leaves the prior hook's bytes and executable bit intact, with deterministic recovery guidance on swap failure. `Installed`/`Updated`/`Skipped` are decided against the merged bytes plus the executable bit rather than the canonical asset's raw bytes, so an already-current foreign-plus-block hook reports `Skipped`; `RequiredHookInstallResult.unreachable_block_advisory` is set, and rendered as a named advisory line in setup's hook output, when an appended block follows a foreign hook's zero-indent `exec`/`exit` and so would never run. After the Git gate, setup always ensures the context baseline; context-only requests return there, while normal modes derive a repo-root-scoped context before aggregating static lifecycle provider `setup` dispatch across providers (config → local_db → auth_db → agent_trace_db → hooks when requested), so setup providers consume only repo-root access from the scoped context. +- `cli/src/services/setup/mod.rs` defines the setup command contract (`SetupMode`, `SetupTarget`, `SetupRequest`, CLI flag parser/validator), an `inquire`-backed interactive target prompter (`InquireSetupTargetPrompter`), setup dispatch outcomes (proceed/cancelled), additive durable-context bootstrap (`bootstrap_context_baseline` for standalone `--bootstrap-context` and every normal successful setup path), and compile-time embedded asset access (`EmbeddedAsset`, target-scoped iterators, required-hook asset iterators/lookups). It also owns the install-time optional-workflow seam: the `OptionalWorkflow` type plus the build-generated `OPTIONAL_WORKFLOWS` catalog, a per-target `WorkflowAssetLayout` built from the existing `default_paths` command/skill directory constants (`command`/`commands`/`prompts` plus `skills`), and `iter_embedded_assets_for_setup_target_with_selection(target, selection)`, which yields every embedded asset except the `{command_dir}/{command_slug}.md` file and `{skills_dir}/{skill_slug}/` subtree of each optional workflow the selection omits. Membership is derived from the catalog's slugs rather than an enumerated file list, so a new optional workflow needs no Rust change. This filtered iterator is the only way embedded assets are enumerated; setup installs through it and doctor inspects through it, so there is no unfiltered enumeration path that could reintroduce an unselected workflow. The non-interactive selection flows through the repeatable `--workflow ` flag into `SetupRequest.optional_workflows: Option>` (`None` meaning the flag was absent); `validate_optional_workflow_slugs` checks each slug against `OPTIONAL_WORKFLOWS` during request resolution, before any write, and `run_setup_for_mode` resolves `None` to the persisted `integrations.optional_workflows` (exported as `persisted_optional_workflows`) before installing through the filtered iterator and persisting the resolved selection. The interactive selection flows through the same seam: `SetupTargetPrompter` carries `prompt_target` plus `prompt_optional_workflows(defaults)` (returning `None` for a cancelled prompt), `SetupDispatch::Proceed { mode, optional_workflows }` carries a prompted selection alongside the resolved mode, and `resolve_setup_dispatch(mode, prompter, defaults)` runs the workflow prompt only after an interactive target prompt, mapping either cancellation to `SetupDispatch::Cancelled`. The prompt module builds its `inquire::MultiSelect` from `optional_workflow_prompt_inputs(catalog, defaults)`, which returns `None` for an empty catalog (skipping the prompt) and otherwise catalog-ordered rows plus the indices to pre-check, ignoring ids absent from the catalog. `setup/command.rs` therefore resolves the repository root before dispatch, mapping the setup domain's typed positive non-Git classification to `setup.not_git_repository` and other resolution failures to `general.unexpected_failure`, so a non-Git directory fails before any prompt without CLI-layer string matching. For repository builds, `cli/build.rs` validates the `SCE_CLI_GENERATED_INPUT_DIR` payload and canonical-input inventories, copies the payload into Cargo `OUT_DIR/pkl-generated`, stages `cli/assets/hooks/**` under `OUT_DIR/static`, requires the staged `config/optional-workflows.json`, and generates both the setup manifest and the optional-workflow catalog (`optional_workflows.rs`, rejecting a manifest whose `schemaVersion` is not 1 or whose entries lack a non-empty `id`/`title`/`description`/`commandSlug`/`skillSlug`) in `OUT_DIR`; focused internal seams separate install-flow from prompt-flow logic; `cli/src/services/setup/command.rs` owns the `SetupCommand` payload used by the static `RuntimeCommand` enum and executes against any context implementing repo-root scoping. Its install engine/orchestrator (`install_assets_for_concrete_target_with_rename`) installs each embedded asset individually: it stages the asset's content next to its final destination and swaps it into place by renaming the staging file directly over the destination — never unlinking the destination first, since `fs::rename` already replaces an existing file atomically — with deterministic recovery guidance naming the failing asset's path on swap failure (the pre-existing destination content, if any, is untouched) and no backup artifact creation; it never removes an integration target directory as a whole, so files a repository owns inside `.opencode`/`.claude`/`.pi` — including nested inside an SCE-owned subdirectory such as `skills/` or `commands/` — survive a setup run; Codex has no single target directory this way, since `CODEX_EMBEDDED_ASSETS` relative paths already carry their own `.agents/`/`.codex/` prefix and `InstallTargetPaths::codex_target_dir()` resolves to the repository root itself, but the same per-asset stage/atomic-swap and never-delete-what-it-did-not-author guarantees still apply file by file. For the three assets that are merge targets — the Claude target's `settings.json`, the OpenCode target's `opencode.json`, and Codex's user-owned `.codex/hooks.json` — the content staged is not always the embedded asset's bytes. Claude and OpenCode use `cli/src/services/setup/config_merge.rs`, while Codex uses the shared `cli/src/services/codex_hook_config.rs` service for structural validation and canonical-registration merging. For the Claude and OpenCode targets, the content staged is the result of `cli/src/services/setup/config_merge.rs::merge_or_create_claude_settings`/`merge_or_create_opencode_config(existing_bytes, generated_bytes, source_path)`: each returns the generated document verbatim when no file exists yet, otherwise parses the existing file as JSON (a parse failure is a hard error naming `source_path`, and nothing is written) and merges it with a pure per-shape function that copies `$schema` from the generated document and preserves every other key from the existing file untouched. `merge_claude_settings` replaces, per hook event key the generated document declares, only the entries whose command contains the ownership marker `run-sce-or-show-install-guidance.sh`, preserving every event key and non-SCE hook entry from the existing file. `merge_opencode_config` merges the `plugin` array as a set: existing entries whose path starts with the ownership marker `./plugins/sce-` are dropped structurally — so a stale plugin path an older or renamed catalog once installed is still recognized and pruned even after the current generated document stops declaring it — and the generated document's `plugin` entries are appended after the surviving entries. Codex's merge validates the document shape, recognizes ownership only when both the generated helper path and the `sce hooks codex` command contract are present, and replaces stale or duplicate SCE handlers with exactly one current handler for each `UserPromptSubmit`, `Stop`, `PreToolUse/Bash`, and `PostToolUse/apply_patch` registration while preserving unrelated valid Codex fields, groups, and handlers. After that per-asset install loop, `prune_stale_assets_for_concrete_target` deletes every path the full embedded catalog for the concrete target claims but the resolved selection did not install (a deselected optional workflow, or an asset a newer catalog renamed or dropped), and `remove_empty_ancestor_directories` removes any parent directory left empty by that deletion, stopping at the target root or at a directory that still holds something such as a user file. It formats deterministic completion messaging; required-hook install orchestration (`install_required_git_hooks`, backed by the rename-injectable `install_required_git_hooks_with_rename`) is a third content-computation seam alongside the two JSON merge targets: `install_single_required_hook_with_rename` computes the bytes to stage with `cli/src/services/setup/hook_merge.rs::merge_or_create_hook(existing_bytes, canonical_bytes, hook_name)` rather than writing `hook_asset.bytes` verbatim — a foreign hook (no SCE managed block, no legacy guidance-URL marker) is kept as an exact byte prefix with the canonical block appended after it, an SCE-owned hook has only its block spliced in place or left unchanged, and a legacy pre-marker hook is replaced wholesale — then follows the same per-file stage/atomic-swap choreography as config-asset install: the staging file is renamed directly over the existing hook without unlinking it first, so a rename failure leaves the prior hook's bytes and executable bit intact, with deterministic recovery guidance on swap failure. `Installed`/`Updated`/`Skipped` are decided against the merged bytes plus the executable bit rather than the canonical asset's raw bytes, so an already-current foreign-plus-block hook reports `Skipped`; `RequiredHookInstallResult.unreachable_block_advisory` is set, and rendered as a named advisory line in setup's hook output, when an appended block follows a foreign hook's zero-indent `exec`/`exit` and so would never run. After the Git gate, setup always ensures the context baseline; context-only requests return there, while normal modes derive a repo-root-scoped context before aggregating static lifecycle provider `setup` dispatch across providers (config → local_db → auth_db → agent_trace_db → hooks when requested), so setup providers consume only repo-root access from the scoped context. - `cli/src/services/setup/mod.rs` keeps those responsibilities inside one file for now, but the current ownership split is explicit: the inline `install` module owns repository-path normalization, staging/swap install behavior, required-hook installation, and filesystem safety guards, while the inline `prompt` module owns interactive target selection and prompt styling. - `cli/src/services/security.rs` provides shared security utilities for deterministic secret redaction (`redact_sensitive_text`) and directory write-permission probes (`ensure_directory_is_writable`) used by app/setup/observability surfaces. - `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. diff --git a/context/cli/cli-command-surface.md b/context/cli/cli-command-surface.md index d99a3d68..59f1a6a9 100644 --- a/context/cli/cli-command-surface.md +++ b/context/cli/cli-command-surface.md @@ -86,7 +86,7 @@ An interactive `setup` run instead resolves the selection through an `inquire` m ## Service contracts -- `cli/src/services/setup/mod.rs` defines setup parsing/selection contracts, additive `bootstrap_context_baseline`, and runtime install orchestration (`run_setup_for_mode`) over the embedded asset install engine; `cli/src/services/setup/command.rs` owns the setup runtime command handler. After the Git gate, setup always ensures the context baseline; context-only requests return there, while normal modes aggregate `ServiceLifecycle::setup` calls across registered providers (`config`, `local_db`, `auth_db`, `agent_trace_db`, `hooks`) in order, using a `ContextWithRepoRoot`-scoped context with resolved repository root. +- `cli/src/services/setup/mod.rs` defines setup parsing/selection contracts, additive `bootstrap_context_baseline`, and runtime install orchestration (`run_setup_for_mode`) over the embedded asset install engine; its setup-owned repository-root resolver returns a typed classification that recognizes only a positively identified non-Git directory as `setup.not_git_repository`, while nonexistent, inaccessible, process, and malformed-output failures remain `general.unexpected_failure`; `cli/src/services/setup/command.rs` owns the setup runtime command handler and maps those typed outcomes while preserving technical sources. After the Git gate, setup always ensures the context baseline; context-only requests return there, while normal modes aggregate `ServiceLifecycle::setup` calls across registered providers (`config`, `local_db`, `auth_db`, `agent_trace_db`, `hooks`) in order, using a `ContextWithRepoRoot`-scoped context with resolved repository root. - `cli/src/services/setup/mod.rs` now keeps its larger internal responsibilities behind focused inline support modules: `install` owns repository canonicalization, staging/swap install flows, required-hook installation, and repo/writeability guards, while `prompt` owns interactive target selection and styled prompt labels. - `cli/src/services/config/mod.rs` defines config parser/runtime contracts (`show`, `validate`, `--help`), strict config-file key/type validation, deterministic text/JSON rendering, repo-configured bash-policy preset/custom validation and reporting under `policies.bash`, and shared auth-key metadata that declares env key, config-file key, and optional baked-default eligibility for supported auth runtime values starting with `workos_client_id` (`WORKOS_CLIENT_ID` vs `workos_client_id`); auth-key provenance/preference metadata stays on `show`, while `validate` stays trimmed to validation status plus issues/warnings. `cli/src/services/config/lifecycle.rs` implements `ServiceLifecycle` for config health checks and setup (global/local config validation and repo-local config bootstrap). - `cli/src/services/doctor/mod.rs` defines the implemented doctor request/report contract (`DoctorRequest`, `DoctorMode`, `run_doctor_with_context`) while focused submodules under `cli/src/services/doctor/` handle runtime command dispatch (`command.rs`), diagnosis (`inspect.rs`), rendering (`render.rs`), fix execution (`fixes.rs`), and doctor-owned domain types (`types.rs`). Together they preserve explicit fix-mode parsing, checkout identity diagnostics, repository-scoped Agent Trace DB health, stable JSON problem and database-record rendering, deterministic fix-result reporting, and aggregation of `ServiceLifecycle::diagnose`/`ServiceLifecycle::fix` across registered providers (`config`, `local_db`, `auth_db`, `agent_trace_db`, `hooks`). The doctor module coordinates state-root/config/database reporting and validation, path-source detection plus required-hook presence/executable/content checks when a repository target is detected, canonical non-launching post-commit auto-sync readiness reporting from managed-block currency plus resolved config, repo-root installed OpenCode, Claude, Pi, and Codex integration inventory derived from embedded setup asset catalogs, typed target/area grouping with compact human status tokens (`[PASS]`, `[WARN]`, `[FAIL]`, `[MISS]`), and repair-mode delegation to service-owned fix implementations. The human renderer emits the compact `SCE doctor` / `SCE doctor fix` hierarchy, suppresses healthy paths and identity metadata, and expands only unhealthy branches; JSON retains the complete path, identity, problem, fix-result, and `post_commit_auto_sync` detail. Claude grouping is typed and path-based: `settings.json`/`hooks/**` as the `Plugins` area (including `.claude/hooks/run-sce-or-show-install-guidance.sh`), plus `Commands` and `Skills`; OpenCode retains `Plugins`, `Agents`, `Commands`, and `Skills`; Pi grouping includes `prompts/**`, `skills/**`, and `extensions/**` areas; Codex grouping includes `.agents/skills/**` as `Skills` and `.codex/hooks.json`/`.codex/hooks/**` as `Hooks` (the latter also carrying a Codex hook trust/review reminder when unhealthy). diff --git a/context/glossary.md b/context/glossary.md index 1faa6172..02a3abe9 100644 --- a/context/glossary.md +++ b/context/glossary.md @@ -100,8 +100,7 @@ - `sync command deferral` (historical): Former plan/state note that a user-invocable sync command was deferred to `0.4.0`; superseded first by nested `sce trace sync` and now by top-level `sce sync` (see `context/cli/sync-command.md`). Local DB bootstrap and setup-time repository-scoped Agent Trace DB initialization still flow through lifecycle providers aggregated by the setup command, hook runtime still keeps a lazy repository Agent Trace DB fallback for repositories where setup has not run or schema metadata is incomplete, and DB health/repair still flows through the doctor surface. - `CLI bounded resilience wrapper`: Shared policy in `cli/src/services/resilience.rs` (`RetryPolicy`, async `run_with_retry`, sync `run_with_retry_sync`) that applies deterministic retries/timeouts/capped backoff to transient operations, emits retry observability events, and returns actionable terminal failure guidance. The sync helper is currently wired into shared database constructors for local open/connect retry and into `TursoDb`/`EncryptedTursoDb` operation retry for `execute()`/`query()`/`query_map()`. - `setup service orchestration`: Setup execution logic in `cli/src/services/setup/command.rs` that resolves the repository root, always ensures the durable-context baseline, then for normal modes derives a repo-root-scoped `AppContext` from the runtime command context, dispatches `setup` through the static lifecycle provider catalog (config → local_db → auth_db → agent_trace_db → hooks when requested), handles interactive target selection for config asset installation, and emits deterministic success messaging per target. -- `setup target flags`: Mutually-exclusive `sce setup` target selectors (`--opencode`, `--claude`, `--pi`, `--codex`, `--all`) that force non-interactive mode for automation; `--all` expands to opencode+claude+pi+codex and replaced the removed `--both` flag. -- `setup mode contract`: `cli/src/services/setup/mod.rs` model where `SetupMode::Interactive` is the default and `SetupMode::NonInteractive(SetupTarget)` is selected only when exactly one target flag is provided. +- `setup target flags` and `setup mode contract`: Mutually-exclusive `sce setup` target selectors (`--opencode`, `--claude`, `--pi`, `--codex`, `--all`) force non-interactive `SetupMode::NonInteractive(SetupTarget)` when exactly one is provided; otherwise `SetupMode::Interactive` is the default. `--all` expands to opencode+claude+pi+codex and replaced the removed `--both` flag. - `setup interactive target prompt`: `inquire::Select` flow in `cli/src/services/setup/mod.rs` (`InquireSetupTargetPrompter`) that presents OpenCode, Claude, Pi, Codex, and All (OpenCode + Claude + Pi + Codex) when `sce setup` runs without target flags. - `setup dispatch outcome`: Execution model in `cli/src/services/setup/mod.rs` (`SetupDispatch`) where setup either proceeds with a selected/non-interactive target or exits as cancelled without file changes. - `setup embedded asset manifest`: Compile-time generated file index emitted by `cli/build.rs` into `OUT_DIR/setup_embedded_assets.rs`, embedding bytes from Pkl-generated `OUT_DIR/pkl-generated/config/.{opencode,claude,pi}/**` plus staged `OUT_DIR/static/hooks/**` as deterministic normalized relative-path entries consumed by `cli/src/services/setup/mod.rs`; `OPENCODE_EMBEDDED_ASSETS`, `CLAUDE_EMBEDDED_ASSETS`, and `PI_EMBEDDED_ASSETS` all back live setup targets. The manifest also carries `CODEX_EMBEDDED_ASSETS`, embedding Codex's two Pkl-generated output roots (`config/.agents/**`, `config/.codex/**`) merged by `cli/build.rs` into a build-time-only `OUT_DIR/pkl-generated/config/codex-target/` staging tree so its relative-path entries keep their `.agents/`/`.codex/` prefixes; `SetupTarget::Codex` now backs it as a fourth live setup target via `sce setup --codex`/`--all`, installing directly at the repository root (via `InstallTargetPaths::codex_target_dir()`) since its asset paths already carry their own output-root prefix, unlike the other three targets' single-subdirectory destinations. @@ -110,7 +109,7 @@ - `setup hook-merge seam`: Pure module `cli/src/services/setup/hook_merge.rs`, covering `pre-commit`, `commit-msg`, and `post-commit`. `merge_or_create_hook(existing: Option<&[u8]>, canonical: &[u8], hook_name: &str) -> Result` returns `canonical` verbatim (`HookMergeKind::Created`) when no hook exists; otherwise it locates the `SCE managed block` marker pair by exact line match. A hook already carrying a balanced marker pair identical to the canonical block returns its bytes unchanged (`AlreadyCurrent`); one whose block differs gets that block spliced in place between the same marker lines, leaving surrounding content untouched (`ManagedBlockReplaced`); a marker-free hook containing the legacy pre-marker guidance URL (`https://sce.crocoder.dev/docs/getting-started#install-cli`) is treated as SCE-owned wholesale and replaced entirely with `canonical` (also `ManagedBlockReplaced`); any other marker-free hook is foreign and kept as an exact byte prefix with the canonical block appended after it (`AppendedToForeign`). An unbalanced or partial marker pair is a hard, deterministic error naming `hook_name`, with no bytes returned. For the `AppendedToForeign` case, `HookMerge.unreachable_block_advisory` is set when the foreign hook's last non-blank, non-comment line sits at zero indentation and starts with `exec ` or `exit` — a narrow heuristic (no shell parsing) flagging that the appended block would never run. This module is pure and filesystem-free per "Unit testing in Nix sandbox"; required-hook install calls it (see `setup required-hook install orchestration`), and doctor hook inspection (`cli/src/services/hooks/lifecycle.rs`, `cli/src/services/doctor/inspect.rs`) also calls it, reporting a hook `Current` only when merging the canonical template into its on-disk bytes is a no-op — including treating an unbalanced or partial marker pair as `Stale` rather than `Unknown`, so `sce doctor --fix` repairs it. - `setup required-hook install orchestration`: Setup-service flow in `cli/src/services/setup/mod.rs` (`install_required_git_hooks`, backed by the rename-injectable `install_required_git_hooks_with_rename`) that resolves repository root + effective hooks directory via git truth, then for each hook computes the bytes to stage with the `setup hook-merge seam` (`hook_merge::merge_or_create_hook`) instead of writing the canonical asset verbatim, reports deterministic per-hook outcomes (`Installed`, `Updated`, `Skipped`) against that merged content plus the executable bit, enforces executable permissions, sets `RequiredHookInstallResult.unreachable_block_advisory` (rendered as a named advisory line) when an appended block would be unreachable, and uses the `setup atomic-swap` policy (see `setup atomic-swap`) — staged content is renamed directly over an existing hook without unlinking it first — with deterministic recovery guidance on swap failure. - `setup hooks CLI mode`: `sce setup` behavior activated by `--hooks` (with optional `--repo `), supporting both hooks-only runs and composable target+hooks runs in one invocation; implemented through `cli/src/services/setup/command.rs` + `cli/src/services/setup/mod.rs`, enforces deterministic compatibility validation (`--repo` requires `--hooks`; target flags remain mutually exclusive), and emits stable setup/hook status output. -- `setup repo gate`: Preflight check in `cli/src/services/setup/command.rs` that calls `cli/src/services/setup/mod.rs` (`ensure_git_repository`) before any setup writes begin; enforces that all `sce setup` modes (config-only, hooks-only, combined, and interactive) require the current directory to be inside a git repository, failing with actionable guidance to run `git init` and rerun `sce setup` when the precondition is not met. +- `setup repo gate`: Preflight check in `cli/src/services/setup/command.rs` that calls `cli/src/services/setup/mod.rs` (`ensure_git_repository`) before any setup writes begin; all `sce setup` modes (config-only, hooks-only, combined, and interactive) require a Git repository. The setup-owned `GitRepositoryResolutionError` maps only a positively identified non-Git directory to `setup.not_git_repository`; nonexistent, inaccessible, process, and malformed-output failures map to `general.unexpected_failure`, with technical sources preserved through the CLI user-error boundary. - `setup local bootstrap`: Pre-install setup bootstrap behavior now owned by lifecycle providers: `ConfigLifecycle::setup` creates missing `.sce/config.json` with the canonical schema-only payload (`{"$schema": "https://sce.crocoder.dev/config.json"}`), `LocalDbLifecycle::setup` initializes the canonical local DB via `LocalDb::new()`, and `AgentTraceDbLifecycle::setup` creates/reuses checkout identity, resolves repository identity, initializes the repository-scoped Agent Trace DB via `agent_trace_storage`, and records repository ID, checkout ID, and `database_path`; the setup command aggregates these calls before config/hooks dispatch across all normal setup modes after context baseline bootstrap. - `setup context baseline bootstrap`: Additive durable-context tree bootstrap in `cli/src/services/setup/mod.rs` (`bootstrap_context_baseline`) that create-if-missing writes neutral baseline Markdown files, working directories, and `context/tmp/.gitignore` via `RepoPaths` accessors. `sce setup --bootstrap-context` is the dedicated context-only mode and must be used alone; every normal successful setup path also ensures the same baseline after the Git gate and before lifecycle/config install work without overwriting existing content. - `CLI redaction-safe diagnostics contract`: baseline security behavior implemented via `cli/src/services/security.rs` (`redact_sensitive_text`) and applied to app-level errors, setup git-diagnostic surfacing, and observability output sinks so common secret-bearing token forms are masked before emission. diff --git a/context/plans/fix-pr-223-error-classification-regressions.md b/context/plans/fix-pr-223-error-classification-regressions.md index d2230777..912a6843 100644 --- a/context/plans/fix-pr-223-error-classification-regressions.md +++ b/context/plans/fix-pr-223-error-classification-regressions.md @@ -76,13 +76,17 @@ Persist this field in every plan; this is durable plan state, not chat state: - Result: Stream credential-storage failures now propagate through the typed sync error predicate and classify as `auth.storage_unavailable`; terminal and refresh cases are covered by focused tests with preserved technical sources. - Context impact: domain — `context/cli/sync-command.md` now accurately documents typed credential-storage classification across all sync failure paths; no root context files require changes. -- [ ] T02: `Type setup repository-root resolution before the CLI boundary` (status:todo) +- [x] T02: `Type setup repository-root resolution before the CLI boundary` (status:done) - Task ID: T02 - Scope: In — introduce a narrow setup-owned `GitRepositoryResolutionError` distinguishing positively identified non-Git directories from unexpected resolution failures; preserve the original technical source through `Display`/`Error`; return it from `ensure_git_repository`; classify it in `setup/command.rs` as `NotGitRepository` or `UnexpectedFailure`; add real non-Git-directory, nonexistent-path, and source-preservation tests; update setup taxonomy/context wording. Out — typing every later setup operation, changing setup success behavior, or matching strings in the command layer. - Dependencies: none - Done when: a valid temporary non-Git directory maps to `setup.not_git_repository`, a definitely nonexistent path maps to `general.unexpected_failure`, both `CliError::User` variants contain technical sources, and only the setup domain recognizes Git's diagnostic. - - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml setup::`; `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml -- -D warnings`. - - Context synchronization: pending + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml setup::` — passed (64 tests); `./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml -- -D warnings` — passed. + - Context synchronization: synced + - Completed: 2026-08-26 + - Files changed: `cli/src/services/setup/mod.rs`, `cli/src/services/setup/command.rs`, `context/cli/cli-command-surface.md`, `context/sce/cli-error-code-taxonomy.md`, `context/architecture.md` + - Result: Setup repository-root resolution now returns a typed classification, mapping only Git-confirmed non-repository directories to `NotGitRepository` and preserving technical sources while mapping other resolution failures to `UnexpectedFailure`; focused tests cover real non-Git and nonexistent paths plus both sourced CLI mappings. + - Context impact: domain — `context/cli/cli-command-surface.md`, `context/sce/cli-error-code-taxonomy.md`, and `context/architecture.md` now document positive-only setup repository classification and technical-source preservation; the five root context files require verification during synchronization. - [ ] T03: `Restore idempotent auth state-query semantics` (status:todo) - Task ID: T03 diff --git a/context/sce/cli-error-code-taxonomy.md b/context/sce/cli-error-code-taxonomy.md index 33d9a685..da58558d 100644 --- a/context/sce/cli-error-code-taxonomy.md +++ b/context/sce/cli-error-code-taxonomy.md @@ -33,7 +33,7 @@ It complements the numeric process exit-code classes documented in `context/sce/ - `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`, `AuthStorageUnavailable`, `NotGitRepository`, or `UnexpectedFailure`) for expected, deliberately-explained failures; `CliError::Internal` carries a live `anyhow::Error` source for every other failure. `CliError::User` may also carry an optional preserved technical `source`, kept for observability only and never rendered to the terminal. -- `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 is a fixed, reviewed sentence returned by `UserError::message()`, keyed for structured logging by `UserError::key()`. `NotAuthenticated` (`auth.not_authenticated`) covers missing credentials from `sce auth logout`/`whoami` and Control Plane authentication failures from `whoami`; `AuthStorageUnavailable` (`auth.storage_unavailable`) is used by `sce sync` and the `sce auth login`, `logout`, and `whoami` command boundary for token-storage plus `AuthError::Io`/`Storage` failures. Both auth-command mappings preserve technical sources for observability. `NotGitRepository` (`setup.not_git_repository`) is used by the setup command when repository-root resolution fails and renders fixed Git-init/rerun guidance; its technical source is preserved for observability. `UnexpectedFailure` (`general.unexpected_failure`) is used by `sce sync`, the config-command boundary for config execution failures, the version and doctor command boundaries for service execution failures, the auth-command boundary, and remaining setup execution failures; it renders one fixed, user-safe diagnostic sentence and has no automatic `Try:` suffix or dynamic path input. +- `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 is a fixed, reviewed sentence returned by `UserError::message()`, keyed for structured logging by `UserError::key()`. `NotAuthenticated` (`auth.not_authenticated`) covers missing credentials from `sce auth logout`/`whoami` and Control Plane authentication failures from `whoami`; `AuthStorageUnavailable` (`auth.storage_unavailable`) is used by `sce sync` and the `sce auth login`, `logout`, and `whoami` command boundary for token-storage plus `AuthError::Io`/`Storage` failures. Both auth-command mappings preserve technical sources for observability. `NotGitRepository` (`setup.not_git_repository`) is used by the setup command only when setup-owned repository-root resolution positively identifies a target as outside a Git repository; it renders fixed Git-init/rerun guidance and preserves the technical source for observability. Other repository-root resolution failures, including nonexistent, inaccessible, process, and malformed-output failures, map to `UnexpectedFailure` (`general.unexpected_failure`). `UnexpectedFailure` is also used by `sce sync`, the config-command boundary for config execution failures, the version and doctor command boundaries for service execution failures, the auth-command boundary, and remaining setup execution failures; it renders one fixed, user-safe diagnostic sentence and has no automatic `Try:` suffix or dynamic path input. - Command and domain layers construct and return a `CliError`; they do not format terminal text or apply styling. Auth command orchestration classifies expected authentication and credential-storage failures into the existing `UserError` catalog by typed domain variants, never by string matching, and preserves the original technical chain as the optional user-error source. `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 stderr rendering: it redacts and writes the catalog variant's message without a wrapper or styling, while `CliError::Internal` retains code-bearing rendering and styles its 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. From 99fcc93876da734e917c9ddef729bae59728ad52 Mon Sep 17 00:00:00 2001 From: David Abram Date: Wed, 26 Aug 2026 13:14:34 +0200 Subject: [PATCH 6/8] setup: Fix git repository error classification Distinguish a missing Git repository from other Git command failures using the command's leading diagnostic instead of matching any later occurrence of the phrase. Normalize Git's locale and preserve typed command errors so unexpected failures retain their original cause and diagnostics. Co-authored-by: SCE --- cli/src/services/setup/command.rs | 4 +- cli/src/services/setup/mod.rs | 188 ++++++++++++++++++++++++++---- 2 files changed, 164 insertions(+), 28 deletions(-) diff --git a/cli/src/services/setup/command.rs b/cli/src/services/setup/command.rs index c81aa981..d9025866 100644 --- a/cli/src/services/setup/command.rs +++ b/cli/src/services/setup/command.rs @@ -102,9 +102,7 @@ impl SetupCommand { } } -fn classify_repository_resolution_error( - source: setup::GitRepositoryResolutionError, -) -> CliError { +fn classify_repository_resolution_error(source: setup::GitRepositoryResolutionError) -> CliError { let user_error = match &source { setup::GitRepositoryResolutionError::NotGitRepository(_) => UserError::NotGitRepository, setup::GitRepositoryResolutionError::Unexpected(_) => UserError::UnexpectedFailure, diff --git a/cli/src/services/setup/mod.rs b/cli/src/services/setup/mod.rs index f2493585..a3d14cfd 100644 --- a/cli/src/services/setup/mod.rs +++ b/cli/src/services/setup/mod.rs @@ -50,6 +50,22 @@ impl std::error::Error for GitRepositoryResolutionError { } } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum GitExitKind { + NotRepository, + Other, +} + +const NOT_GIT_REPOSITORY_PREFIX: &str = "fatal: not a git repository"; + +fn classify_git_exit(stderr: &str) -> GitExitKind { + if stderr.starts_with(NOT_GIT_REPOSITORY_PREFIX) { + GitExitKind::NotRepository + } else { + GitExitKind::Other + } +} + #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum SetupTarget { OpenCode, @@ -881,10 +897,10 @@ mod install { use super::config_merge; use super::hook_merge; use super::{ - cleanup_path_if_exists, concrete_targets_for, embedded_assets_for_concrete_target, - hook_install_recovery_guidance, iter_embedded_assets_for_setup_target_with_selection, - iter_required_hook_assets, setup_install_recovery_guidance, EmbeddedAsset, - GitRepositoryResolutionError, + classify_git_exit, cleanup_path_if_exists, concrete_targets_for, + embedded_assets_for_concrete_target, hook_install_recovery_guidance, + iter_embedded_assets_for_setup_target_with_selection, iter_required_hook_assets, + setup_install_recovery_guidance, EmbeddedAsset, GitExitKind, GitRepositoryResolutionError, RequiredHookInstallResult, RequiredHookInstallStatus, RequiredHooksInstallOutcome, SetupInstallOutcome, SetupInstallTargetResult, SetupTarget, }; @@ -1168,18 +1184,26 @@ mod install { &["rev-parse", "--show-toplevel"], "Failed to resolve repository root. Ensure '--repo' points to an accessible git repository.", ) - .map_err(|error| map_setup_non_git_repository_error(repository_root, error))?; + .map_err(map_setup_repository_resolution_error)?; Ok(PathBuf::from(repository_root_output)) } - fn map_setup_non_git_repository_error( - _repository_root: &Path, - error: anyhow::Error, + fn map_setup_repository_resolution_error( + error: GitCommandError, ) -> GitRepositoryResolutionError { - if error.to_string().contains("not a git repository") { - GitRepositoryResolutionError::NotGitRepository(error) + let is_not_repository = matches!( + &error, + GitCommandError::NonZeroExit { + kind: GitExitKind::NotRepository, + .. + } + ); + let source = anyhow::Error::new(error); + + if is_not_repository { + GitRepositoryResolutionError::NotGitRepository(source) } else { - GitRepositoryResolutionError::Unexpected(error) + GitRepositoryResolutionError::Unexpected(source) } } @@ -1198,39 +1222,124 @@ mod install { Ok(repository_root.join(hooks_directory)) } + #[derive(Debug)] + enum GitCommandError { + Spawn { + context: String, + directory: PathBuf, + source: std::io::Error, + }, + NonZeroExit { + context: String, + directory: PathBuf, + status: std::process::ExitStatus, + kind: GitExitKind, + diagnostic: String, + }, + InvalidUtf8 { + context: String, + source: std::string::FromUtf8Error, + }, + EmptyOutput { + context: String, + directory: PathBuf, + }, + } + + impl std::fmt::Display for GitCommandError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Spawn { + context, + directory, + source, + } => write!( + f, + "{context} (directory: '{}'): {source}", + directory.display() + ), + Self::NonZeroExit { + context, + directory, + status, + diagnostic, + .. + } => write!( + f, + "{context} (directory: '{}', status: {status:?}) {diagnostic}", + directory.display() + ), + Self::InvalidUtf8 { context, source } => { + write!( + f, + "{context}: git command output contained invalid UTF-8: {source}" + ) + } + Self::EmptyOutput { context, directory } => write!( + f, + "{context} (directory: '{}'): git command returned empty output", + directory.display() + ), + } + } + } + + impl std::error::Error for GitCommandError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::Spawn { source, .. } => Some(source), + Self::InvalidUtf8 { source, .. } => Some(source), + Self::NonZeroExit { .. } | Self::EmptyOutput { .. } => None, + } + } + } + fn run_git_command_in_directory( repository_root: &Path, args: &[&str], context_message: &str, - ) -> Result { + ) -> std::result::Result { let output = Command::new("git") + .env("LC_ALL", "C") + .env("LANG", "C") + .env_remove("LANGUAGE") .args(args) .current_dir(repository_root) .output() - .with_context(|| { - format!( - "{} (directory: '{}')", - context_message, - repository_root.display() - ) + .map_err(|source| GitCommandError::Spawn { + context: context_message.to_string(), + directory: repository_root.to_path_buf(), + source, })?; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + let kind = classify_git_exit(&stderr); let diagnostic = if stderr.is_empty() { String::from("git command exited with a non-zero status") } else { redact_sensitive_text(&stderr) }; - bail!("{context_message} {diagnostic}"); + return Err(GitCommandError::NonZeroExit { + context: context_message.to_string(), + directory: repository_root.to_path_buf(), + status: output.status, + kind, + diagnostic, + }); } - let stdout = String::from_utf8(output.stdout) - .context("git command output contained invalid UTF-8")? - .trim() - .to_string(); + let stdout = + String::from_utf8(output.stdout).map_err(|source| GitCommandError::InvalidUtf8 { + context: context_message.to_string(), + source, + })?; + let stdout = stdout.trim().to_string(); if stdout.is_empty() { - bail!("{context_message} git command returned empty output"); + return Err(GitCommandError::EmptyOutput { + context: context_message.to_string(), + directory: repository_root.to_path_buf(), + }); } Ok(stdout) @@ -1911,6 +2020,32 @@ mod tests { } } + #[test] + fn git_not_repository_diagnostic_has_typed_kind() { + assert_eq!( + classify_git_exit( + "fatal: not a git repository (or any of the parent directories): .git" + ), + GitExitKind::NotRepository, + ); + } + + #[test] + fn unrelated_git_fatal_is_not_repository_failure() { + assert_eq!( + classify_git_exit("fatal: detected dubious ownership in repository at '/tmp/repo'"), + GitExitKind::Other, + ); + } + + #[test] + fn text_containing_phrase_later_is_not_misclassified() { + assert_eq!( + classify_git_exit("fatal: something else: previous error was not a git repository"), + GitExitKind::Other, + ); + } + #[test] fn ensure_git_repository_classifies_real_non_git_directory() { let directory = unique_temp_dir("non-git-directory"); @@ -1934,7 +2069,10 @@ mod tests { let error = ensure_git_repository(&directory) .expect_err("a nonexistent path should remain an unexpected failure"); - assert!(matches!(&error, GitRepositoryResolutionError::Unexpected(_))); + assert!(matches!( + &error, + GitRepositoryResolutionError::Unexpected(_) + )); assert!(error.to_string().contains("No such file or directory")); assert!(std::error::Error::source(&error).is_some()); } From aa7e3213261ec2e1834d302e61eab69f2e51da29 Mon Sep 17 00:00:00 2001 From: David Abram Date: Wed, 26 Aug 2026 13:27:23 +0200 Subject: [PATCH 7/8] auth: Restore idempotent state-query semantics Treat missing credentials as successful logout and whoami state queries while preserving typed failures for authenticated and storage errors. Add text/JSON regression coverage and update the documented command and error contracts. Plan: fix-pr-223-error-classification-regressions (T03) Co-authored-by: SCE --- cli/src/services/auth_command/mod.rs | 124 ++++++++++++++++-- context/architecture.md | 2 +- context/cli/cli-command-surface.md | 6 +- context/glossary.md | 2 +- context/overview.md | 2 +- ...pr-223-error-classification-regressions.md | 10 +- context/sce/cli-error-code-taxonomy.md | 2 +- 7 files changed, 130 insertions(+), 18 deletions(-) diff --git a/cli/src/services/auth_command/mod.rs b/cli/src/services/auth_command/mod.rs index cf442757..ec764715 100644 --- a/cli/src/services/auth_command/mod.rs +++ b/cli/src/services/auth_command/mod.rs @@ -76,10 +76,7 @@ pub fn run_login(format: AuthFormat) -> Result { pub fn run_logout(format: AuthFormat) -> Result { let deleted = token_storage::delete_tokens().map_err(auth_storage_error)?; - if !deleted { - return Err(CliError::user(UserError::NotAuthenticated)); - } - render_logout_success(format).map_err(unexpected_auth_command_error) + render_logout_result(deleted, format).map_err(unexpected_auth_command_error) } pub fn run_whoami(format: AuthFormat) -> Result { @@ -87,7 +84,7 @@ pub fn run_whoami(format: AuthFormat) -> Result { .map_err(auth_storage_error)? .is_none() { - return Err(CliError::user(UserError::NotAuthenticated)); + return render_unauthenticated_whoami(format).map_err(unexpected_auth_command_error); } let cwd = std::env::current_dir() @@ -328,20 +325,41 @@ fn render_login_refresh_result(tokens: &StoredTokens, format: AuthFormat) -> Res } } -fn render_logout_success(format: AuthFormat) -> Result { +fn render_logout_result(deleted: bool, format: AuthFormat) -> Result { match format { - AuthFormat::Text => Ok(success("Logged out")), + AuthFormat::Text => Ok(if deleted { + success("Logged out") + } else { + value("No user logged in") + }), AuthFormat::Json => serde_json::to_string_pretty(&json!({ "status": "ok", "command": NAME, "subcommand": "logout", "authenticated": false, - "credentials_removed": true, + "credentials_removed": deleted, })) .context("failed to serialize auth logout report to JSON. Try: rerun 'sce auth logout --format json'."), } } +fn render_unauthenticated_whoami(format: AuthFormat) -> Result { + match format { + AuthFormat::Text => Ok(format!( + "You are not logged in. Please log in using the {} command.", + success("sce auth login") + )), + AuthFormat::Json => serde_json::to_string_pretty(&json!({ + "status": "ok", + "command": NAME, + "subcommand": "whoami", + "authentication_state": "unauthenticated", + "has_stored_credentials": false, + })) + .context("failed to serialize auth whoami report to JSON. Try: rerun 'sce auth whoami --format json'."), + } +} + fn render_whoami_result(profile: &MeResponse, format: AuthFormat) -> Result { match format { AuthFormat::Text => { @@ -415,3 +433,93 @@ fn auth_storage_error(error: crate::services::token_storage::TokenStorageError) fn unexpected_auth_command_error(error: anyhow::Error) -> CliError { CliError::user_with_source(UserError::UnexpectedFailure, error) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn logout_text_reports_whether_credentials_were_removed() { + assert_eq!( + render_logout_result(false, AuthFormat::Text).expect("logout should render"), + "No user logged in" + ); + assert_eq!( + render_logout_result(true, AuthFormat::Text).expect("logout should render"), + "Logged out" + ); + } + + #[test] + fn logout_json_reports_whether_credentials_were_removed() { + let absent: serde_json::Value = serde_json::from_str( + &render_logout_result(false, AuthFormat::Json).expect("logout should render"), + ) + .expect("logout JSON should be valid"); + let present: serde_json::Value = serde_json::from_str( + &render_logout_result(true, AuthFormat::Json).expect("logout should render"), + ) + .expect("logout JSON should be valid"); + + assert_eq!(absent["status"], "ok"); + assert_eq!(absent["authenticated"], false); + assert_eq!(absent["credentials_removed"], false); + assert_eq!(present["credentials_removed"], true); + } + + #[test] + fn unauthenticated_whoami_renders_text_guidance() { + assert_eq!( + render_unauthenticated_whoami(AuthFormat::Text) + .expect("unauthenticated whoami should render"), + "You are not logged in. Please log in using the sce auth login command." + ); + } + + #[test] + fn unauthenticated_whoami_json_reports_state() { + let report: serde_json::Value = serde_json::from_str( + &render_unauthenticated_whoami(AuthFormat::Json) + .expect("unauthenticated whoami should render"), + ) + .expect("whoami JSON should be valid"); + + assert_eq!(report["status"], "ok"); + assert_eq!(report["command"], "auth"); + assert_eq!(report["subcommand"], "whoami"); + assert_eq!(report["authentication_state"], "unauthenticated"); + assert_eq!(report["has_stored_credentials"], false); + } + + #[test] + fn authenticated_whoami_failures_keep_typed_errors_and_sources() { + let cases = [ + ( + ControlPlaneError::AuthenticationFailed("expired".to_string()), + UserError::NotAuthenticated, + ), + ( + ControlPlaneError::Storage("database unavailable".to_string()), + UserError::AuthStorageUnavailable, + ), + ( + ControlPlaneError::Transport("connection refused".to_string()), + UserError::UnexpectedFailure, + ), + ]; + + for (control_plane_error, expected_user_error) in cases { + let mapped = map_whoami_control_plane_error(&control_plane_error); + match mapped { + CliError::User { + error, + source: Some(source), + } => { + assert_eq!(error, expected_user_error); + assert!(!source.to_string().is_empty()); + } + _ => panic!("authenticated whoami failure lost its typed source"), + } + } + } +} diff --git a/context/architecture.md b/context/architecture.md index b34e87bc..a073f25f 100644 --- a/context/architecture.md +++ b/context/architecture.md @@ -121,7 +121,7 @@ The repository includes a new placeholder Rust binary crate at `cli/`. - `cli/src/services/capabilities.rs` defines the current broad CLI capability traits consumed by the borrowed, compile-time-typed `AppContext`: `FsOps` with `StdFsOps` for filesystem operations and `GitOps` with `ProcessGitOps` for git command execution plus repository-root/hooks-directory resolution. Existing service internals do not consume these traits directly yet; command execution uses narrow accessors and repo-root-scoped context derivation. - `cli/src/services/lifecycle.rs` defines the current compile-safe lifecycle seam. `ServiceLifecycle` has default no-op generic `diagnose`, `fix`, and `setup` methods over `C: HasRepoRoot`, with lifecycle-owned health, fix, and setup result types so the trait contract is not publicly anchored to doctor/setup module types or the full `AppContext` shape. The same module owns the static `LifecycleProvider` enum and shared `lifecycle_providers(include_hooks)` catalog/factory, returning providers in deterministic order (config → local_db → auth_db → agent_trace_db → hooks when requested); enum dispatch calls each concrete provider through generic context methods without boxed lifecycle-provider allocation or repo-root trait-object context erasure. Hooks exposes a `HooksLifecycle` provider in `cli/src/services/hooks/lifecycle.rs` for hook rollout diagnosis/fix/setup using lifecycle-owned health records plus the canonical required-hook installer. Config exposes a `ConfigLifecycle` provider in `cli/src/services/config/lifecycle.rs` for global/repo-local config validation and repo-local `.sce/config.json` bootstrap. local_db exposes a `LocalDbLifecycle` provider in `cli/src/services/local_db/lifecycle.rs` for canonical local DB path health, parent-directory readiness/bootstrap, and `LocalDb::new()` setup. auth_db exposes an `AuthDbLifecycle` provider in `cli/src/services/auth_db/lifecycle.rs` for canonical auth DB path health, parent-directory readiness/bootstrap, and `AuthDb::new()` setup. agent_trace_db exposes an `AgentTraceDbLifecycle` provider in `cli/src/services/agent_trace_db/lifecycle.rs` for setup-time repository-scoped Agent Trace storage initialization when a repo root is available and repository Agent Trace DB path health/fix from resolved repository identity, returning an actionable "requires a Git repository" diagnostic outside repository context (no global/checkout fallback path; the former fallback was removed by the `retire-legacy-agent-trace-db` plan). Doctor runtime aggregates the full provider catalog for `diagnose` and `fix` and adapts lifecycle records into doctor report/fix records at the orchestration boundary; setup command aggregates the shared catalog for `setup` with hooks included only when requested and adapts hook setup outcomes before rendering setup-owned messages. - Agent Trace lifecycle setup resolves `agent_trace.repository_id` / `agent_trace.repository_remote`, creates/reuses checkout identity for diagnostics, and initializes the repository-scoped DB through `agent_trace_storage::resolve_agent_trace_storage(...)`; hook runtime uses the same storage resolver and `RepositoryAgentTraceDb` lazy fast-path-then-migrate open with bounded retry plus narrow migration-metadata repair for concurrent first-open races. -- `cli/src/services/auth_command/mod.rs` defines the implemented auth command surface for `sce auth login|logout|whoami`, including device-flow login, stored-credential validation/renewal through login with device-flow fallback, logout, and Control Plane `/me`-backed whoami rendering in text/JSON formats; text mode uses flat `Email`, `First Name`, `Last Name`, `Role`, `Permissions`, and `Organization Name` labels, with optional names and missing role/permissions/workspace values handled deterministically. Expected auth failures are classified at this boundary as typed `CliError::User` entries (`NotAuthenticated` for missing/authentication failures, `AuthStorageUnavailable` for token-storage plus `AuthError::Io`/`Storage` failures, and `UnexpectedFailure` for approved user-facing rendering/prompt fallbacks), with technical sources retained for observability; remaining auth-command failures also use `UnexpectedFailure` rather than a separate runtime mapping. Stored-credential login applies that classification inside `run_login_with_stored_credentials` and its renewal/device-login call path before returning to the command caller. Logged-out text returns exact login guidance and renewal reports retain the `login` operation label. `cli/src/services/auth_command/command.rs` owns the `AuthCommand` payload used by the static `RuntimeCommand` enum. There is no public renewal or status subcommand. +- `cli/src/services/auth_command/mod.rs` defines the implemented auth command surface for `sce auth login|logout|whoami`, including device-flow login, stored-credential validation/renewal through login with device-flow fallback, idempotent logout state reporting, and Control Plane `/me`-backed whoami rendering in text/JSON formats; text mode uses flat `Email`, `First Name`, `Last Name`, `Role`, `Permissions`, and `Organization Name` labels, with optional names and missing role/permissions/workspace values handled deterministically. Missing credentials from logout/whoami are successful state queries with documented text/JSON reports; authenticated Control Plane failures are classified at this boundary as typed `CliError::User` entries (`NotAuthenticated` for authentication failures, `AuthStorageUnavailable` for token-storage plus `AuthError::Io`/`Storage` failures, and `UnexpectedFailure` for approved user-facing rendering/prompt fallbacks), with technical sources retained for observability; remaining auth-command failures also use `UnexpectedFailure` rather than a separate runtime mapping. Stored-credential login applies that classification inside `run_login_with_stored_credentials` and its renewal/device-login call path before returning to the command caller. Logged-out text returns exact state-aware guidance, unauthenticated whoami returns its documented state report, and renewal reports retain the `login` operation label. `cli/src/services/auth_command/command.rs` owns the `AuthCommand` payload used by the static `RuntimeCommand` enum. There is no public renewal or status subcommand. - `cli/src/services/db/mod.rs` provides the shared generic Turso infrastructure seam: `DbSpec` supplies a service-specific name, path, ordered embedded migrations, and config-file lookup key (`db_config_key()`), while `TursoDb` owns parent-directory creation, `Builder::new_local(...)` initialization (with `experimental_multiprocess_wal(true)` for safe concurrent access), Turso connection setup, tokio current-thread runtime bridging, retry-backed blocking `execute`/`query`/`query_values`/`query_map` wrappers, and generic migration execution with per-database `__sce_migrations` metadata. `TursoDb::new()` and `EncryptedTursoDb::new()` wrap only their local open/connect block in `run_with_retry_sync` using a config-driven connection-open policy resolved from the `DATABASE_RETRY_CONFIG` `OnceLock` with fallback to hardcoded defaults, while operation methods use a config-driven operation policy from the same source. `query_values()` returns fully fetched column names plus raw `turso::Value` rows for deterministic operator-facing rendering; `query_map()` retries the initial query and row-fetch loop, then applies caller row mapping after retry completion. Migration execution is not retried and uses batch execution so one migration file may contain multiple SQL statements while still recording one migration ID. The same module also provides `EncryptedTursoDb`, a structurally parallel encrypted adapter that resolves the encryption key through `encryption_key::get_or_create_encryption_key()`, enables Turso local encryption with strict `aegis256` cipher selection, and exposes retry-backed synchronous wrappers plus migration execution. `cli/src/services/db/encryption_key.rs` first derives a Turso-compatible 64-character hex key from non-empty `SCE_AUTH_DB_ENCRYPTION_KEY` env-secret text when present, otherwise falls back to keyring-backed credential-store get-or-create behavior; no plaintext auth DB fallback exists. - `cli/src/services/local_db/mod.rs` provides the concrete local DB spec and `LocalDb` type alias over the shared generic `TursoDb` adapter. `LocalDbSpec` resolves the deterministic persistent runtime DB target through the shared default-path seam and declares no local migrations; `TursoDb` supplies retry-backed blocking `execute`/`query`, parent-directory creation, Turso connection setup, tokio current-thread runtime bridging, and generic migration execution. - `cli/src/services/auth_db/mod.rs` provides the encrypted auth DB spec and `AuthDb` type alias over `EncryptedTursoDb`. `AuthDbSpec` resolves `/sce/auth.db` through the shared default-path seam and embeds ordered auth migrations. Auth DB lifecycle setup/doctor integration is wired through `AuthDbLifecycle`; auth command/token-storage reads/writes are directed through `token_storage.rs`. diff --git a/context/cli/cli-command-surface.md b/context/cli/cli-command-surface.md index 59f1a6a9..083dd1f3 100644 --- a/context/cli/cli-command-surface.md +++ b/context/cli/cli-command-surface.md @@ -59,7 +59,7 @@ Deferred or gated command surfaces currently avoid claiming unimplemented behavi `setup` defaults to an `inquire` interactive target selection (OpenCode, Claude, Pi, Codex, All) and accepts mutually-exclusive non-interactive target flags (`--opencode`, `--claude`, `--pi`, `--codex`, `--all`); the former `--both` flag was removed in favor of `--all` (opencode+claude+pi+codex); the interactive prompt title and target labels reuse shared prompt styling helpers when stdout color is enabled. `setup` also accepts `--bootstrap-context` as a standalone context-only mode that ensures the durable-context baseline without prompts or integration installs; every normal successful setup path also ensures that baseline after the Git gate. `setup` accepts a repeatable `--workflow ` flag selecting which optional workflows to install (currently only `brownfield`). Passing it makes the listed slugs the exact selection for that run; omitting it reuses the persisted `integrations.optional_workflows`, so a repeat run preserves an earlier opt-in. Unknown slugs fail request resolution with a validation error naming the embedded catalog's available slugs and write no files, and `--workflow` is rejected alongside `--bootstrap-context` or on a hooks-only run because neither installs target assets. The resolved selection filters the installed assets and is persisted; see [config precedence contract](config-precedence-contract.md) and [setup local bootstrap](../sce/setup-repo-local-config-bootstrap.md). An interactive `setup` run instead resolves the selection through an `inquire` multi-select shown after the target prompt, titled `Select optional workflows` with one `{title} — {description}` row per optional workflow using the shared prompt styling. Rows are unchecked when nothing is persisted and pre-checked from `integrations.optional_workflows` otherwise (a supplied `--workflow` list seeds them instead); the answered prompt is the run's exact selection. Cancelling either prompt yields the existing `Setup cancelled. No files were changed.` outcome, a non-TTY run keeps the existing actionable guidance, and the prompt is skipped when the catalog has no optional workflow. -`auth` now emits auth-local guidance for bare `sce auth` and `sce auth --help`, listing `login`, `logout`, and `whoami` plus copy-ready next steps. `sce auth login` uses the existing token-validation path whenever stored credentials are present: valid credentials are preserved, expired credentials are refreshed, and a failed renewal falls back to device authorization. First login without stored credentials still starts device authorization, and renewal reports remain labeled as `login` in text and JSON output. Expected authentication failures at the auth command boundary are typed as `CliError::User`: missing credentials from `logout`/`whoami` and Control Plane authentication failures from `whoami` render `NotAuthenticated`, token-storage plus `AuthError::Io`/`Storage` failures across `login`, `logout`, and `whoami` render `AuthStorageUnavailable`, and approved user-facing rendering/prompt fallbacks render `UnexpectedFailure`; stored-credential login renewal, token-save, and device-flow paths classify before returning through `run_login_with_stored_credentials`, and technical error chains remain attached for observability. Remaining auth-command failures are surfaced as `UnexpectedFailure` with their technical sources preserved. Authenticated `sce auth whoami` retrieves the authoritative profile from the Control Plane `GET /me` endpoint and renders flat text labels for `Email`, `First Name`, `Last Name`, `Role`, `Permissions`, and `Organization Name`; missing first/last names render empty and missing role, permissions, or workspace values render `none`. +`auth` now emits auth-local guidance for bare `sce auth` and `sce auth --help`, listing `login`, `logout`, and `whoami` plus copy-ready next steps. `sce auth login` uses the existing token-validation path whenever stored credentials are present: valid credentials are preserved, expired credentials are refreshed, and a failed renewal falls back to device authorization. First login without stored credentials still starts device authorization, and renewal reports remain labeled as `login` in text and JSON output. Expected authentication failures at the auth command boundary are typed as `CliError::User`: missing credentials from `logout` and `whoami` are successful state queries with their documented text/JSON reports, Control Plane authentication failures from authenticated `whoami` render `NotAuthenticated`, token-storage plus `AuthError::Io`/`Storage` failures across `login`, `logout`, and `whoami` render `AuthStorageUnavailable`, and approved user-facing rendering/prompt fallbacks render `UnexpectedFailure`; stored-credential login renewal, token-save, and device-flow paths classify before returning through `run_login_with_stored_credentials`, and technical error chains remain attached for observability. Remaining auth-command failures are surfaced as `UnexpectedFailure` with their technical sources preserved. Authenticated `sce auth whoami` retrieves the authoritative profile from the Control Plane `GET /me` endpoint and renders flat text labels for `Email`, `First Name`, `Last Name`, `Role`, `Permissions`, and `Organization Name`; missing first/last names render empty and missing role, permissions, or workspace values render `none`. Unauthenticated `whoami` reports `authentication_state: unauthenticated` and `has_stored_credentials: false` in JSON and gives the existing login guidance in text. `setup`, `doctor`, `hooks`, `policy`, `sync`, `version`, and `completion` all support command-local `--help`/`-h` usage output via top-level parser routing in `cli/src/app.rs`. `setup` now also exposes compile-time embedded config assets for OpenCode/Claude/Pi/Codex targets, sourced from the generated `config/.opencode/**`, `config/.claude/**`, `config/.pi/**`, and (via a build-time staging merge of `config/.agents/**` + `config/.codex/**`) `config/codex-target/**` trees via `cli/build.rs` with normalized forward-slash relative paths and target-scoped iteration APIs; the embedded asset set includes the OpenCode bash-policy plugin wrapper plus Claude settings `PreToolUse` Bash policy hook, both delegating to the Rust `sce policy bash` path. `setup` additionally includes a repository-root install engine (`install_embedded_setup_assets`) that installs each embedded asset individually into `.opencode/`/`.claude/`/`.pi/` — stage next to the final destination, remove only that destination file if present, swap into place, with deterministic recovery guidance naming the failing asset's path on swap failure — never removing an integration target directory as a whole, while treating bash-policy enforcement files as first-class SCE-managed assets. Codex is the one target whose embedded relative paths already carry their own output-root prefix (`.agents/...`, `.codex/...`), so its destination root is the repository root itself rather than a single `.codex/`-style subdirectory. Its generated hook command resolves that repository root at invocation time, so Codex events from nested cwd and repositories with spaces reach the installed helper safely; Git-root failure is a silent successful no-op, while the helper preserves missing-CLI stderr guidance and STDIN forwarding. See [setup non-destructive per-asset install policy](../sce/setup-no-backup-policy-seam.md) for the full contract, including the pending pruning gap for deselected/stale assets. @@ -98,7 +98,7 @@ An interactive `setup` run instead resolves the selection through an `inquire` m - `cli/src/services/default_paths.rs` defines the canonical per-user persisted-location seam for config/state/cache roots plus named default file paths for current persisted artifacts (`global config`, `auth tokens`, `local DB`, `agent trace DB`) used by config discovery, token storage, database adapters, and doctor diagnostics; its internal `roots` seam now owns the platform-aware root-directory resolution so non-test production modules consume shared path accessors instead of resolving owned roots directly. - `cli/src/services/agent_trace.rs` defines the canonical Rust SCE web base URL and helpers for Agent Trace conversation URLs, persisted Agent Trace trace URLs, Agent Trace session URLs, and setup-created repo-local config schema URLs. - `cli/src/services/token_storage.rs` defines WorkOS token persistence (`save_tokens`, `load_tokens`, `delete_tokens`) via the encrypted `AuthDb` `auth_credentials` table using a `OnceLock` lazy singleton with constant integer row ID `1`. `TokenStorageError` exposes `PathResolution` and `Database` variants. No JSON file I/O remains. -- `cli/src/services/auth_command/mod.rs` defines the auth command orchestration surface (`AuthRequest`, `AuthSubcommand`, `run_auth_subcommand`) for `login`, `logout`, and `whoami`, including shared text/JSON rendering, login's stored-token validation and refresh path for any stored credential, device-flow fallback after absent or unsuccessfully renewed credentials, source-level typed `CliError` propagation through `run_login_with_stored_credentials` for renewal, token-save, and device-login failures, token-storage-backed logout deletion, Control Plane `/me` profile retrieval for authenticated whoami, flat safe-field rendering with optional-name/null-value handling, exact logged-out text guidance, precedence-aware client-ID guidance sourced from the shared auth-runtime resolver instead of env-only assumptions, and a lazily initialized current-thread Tokio runtime with both I/O and time enabled so the auth flows can drive the WorkOS device/refresh paths without the prior I/O-disabled panic; `cli/src/services/auth_command/command.rs` owns the `AuthCommand` payload used by the static `RuntimeCommand` enum. +- `cli/src/services/auth_command/mod.rs` defines the auth command orchestration surface (`AuthRequest`, `AuthSubcommand`, `run_auth_subcommand`) for `login`, `logout`, and `whoami`, including shared text/JSON rendering, login's stored-token validation and refresh path for any stored credential, device-flow fallback after absent or unsuccessfully renewed credentials, source-level typed `CliError` propagation through `run_login_with_stored_credentials` for renewal, token-save, and device-login failures, token-storage-backed logout deletion with idempotent absent-credential success, unauthenticated whoami state reporting, Control Plane `/me` profile retrieval for authenticated whoami, flat safe-field rendering with optional-name/null-value handling, exact logged-out text guidance, precedence-aware client-ID guidance sourced from the shared auth-runtime resolver instead of env-only assumptions, and a lazily initialized current-thread Tokio runtime with both I/O and time enabled so the auth flows can drive the WorkOS device/refresh paths without the prior I/O-disabled panic; `cli/src/services/auth_command/command.rs` owns the `AuthCommand` payload used by the static `RuntimeCommand` enum. - `cli/src/app.rs` parses `auth`, `config`, `setup`, `doctor`, `hooks`, `policy`, `sync`, `version`, and `completion` into service-owned runtime command handlers so runtime messages are sourced from domain modules instead of inline strings. ## Local and Agent Trace Turso adapter behavior @@ -133,7 +133,7 @@ An interactive `setup` run instead resolves the selection through an `inquire` m - `cli/src/services/setup/mod.rs` and `cli/src/services/hooks/mod.rs` include contract-focused tests for setup flag parsing/validation, interactive selection/cancellation dispatch, setup run messaging, and hook runtime argument/IO/finalization behavior. - `cli/src/services/token_storage.rs` tests cover token save/load round-trips, missing-file handling, token deletion outcomes, invalid JSON corruption handling, and Unix `0600` file-permission enforcement. - `cli/src/services/auth.rs` tests cover WorkOS device/token payload shape parsing, RFC 8628 device and refresh grant constant wiring, terminal OAuth error mapping with `Try:` guidance, polling decision handling for `authorization_pending`/`slow_down`/terminal outcomes, token-expiry evaluation, and refresh-token re-login guidance for terminal refresh errors. -- `cli/src/services/auth_command/mod.rs` tests cover auth subcommand dispatch, unauthenticated whoami guidance, safe authenticated whoami JSON fields, stored valid/expired/absent-credential login routing, failed-renewal fallback, login-labeled renewal reports, `Try:` guidance preservation, and runtime-I/O readiness for the login flow. Flat authenticated text rendering is implemented but currently has no dedicated regression test. +- `cli/src/services/auth_command/mod.rs` tests cover auth subcommand dispatch, idempotent logout text/JSON results for absent and present credentials, unauthenticated whoami text/JSON guidance, typed authenticated whoami failure mappings with preserved sources, safe authenticated whoami JSON fields, stored valid/expired/absent-credential login routing, failed-renewal fallback, login-labeled renewal reports, `Try:` guidance preservation, and runtime-I/O readiness for the login flow. - `cli/src/services/setup/mod.rs` tests also verify embedded-manifest completeness against runtime `config/` trees, deterministic sorted path normalization, and target-scoped iterator behavior (`OpenCode`, `Claude`, `Both`); sandbox-sensitive filesystem install coverage has been removed from the unit-test slice for later integration-test coverage. - `cli/src/services/doctor/` unit coverage is intentionally limited to flake-safe output-shape assertions; filesystem, git, and real repair-flow coverage is deferred to future integration tests so `nix flake check` stays sandbox-safe. diff --git a/context/glossary.md b/context/glossary.md index 02a3abe9..3c0e6ffa 100644 --- a/context/glossary.md +++ b/context/glossary.md @@ -67,7 +67,7 @@ - `Nix performance recommendations`: Repo-local operator guidance in `AGENTS.md` covering optional user-level `~/.config/nix/nix.conf` tuning (`max-jobs = auto`, `cores = 0`) and the explicit root/admin-only boundary for `/etc/nix/nix.conf` `auto-optimise-store = true`. - `log_to_file`: Flat SCE config-file boolean controlling file-log emission independently of stderr and tracing. It defaults to `true`, is surfaced with source metadata by `sce config show`, and resolves independently from `log_dir`; an omitted `log_dir` uses the default location, while an explicitly empty config value remains invalid. Set `log_to_file` to `false` to disable file logging without changing other logger destinations. See [CLI observability contract](sce/cli-observability-contract.md). - `sce` (CLI foundation): Rust binary crate at `cli/` with implemented auth command flows (`auth login|logout|whoami`) plus auth-local bare-command guidance (`sce auth`, `sce auth --help`), Control Plane `/me`-backed whoami profile output using flat email/name/role/permissions/organization labels, exact logged-out login guidance, implemented setup installation flow including lifecycle-aggregated local DB and Agent Trace DB bootstrap, implemented attribution-only `hooks` subcommand routing/validation entrypoints, and a fully implemented top-level `sce sync` command that synchronizes the current repository's Agent Trace DB with the control plane and renders the documented text/JSON output (see `context/cli/sync-command.md`). -- `auth login stored-credential renewal`: The `sce auth login` behavior that first validates every stored credential through the existing non-forced token path, preserves valid credentials, refreshes expired credentials, and falls back to device authorization when credentials are absent or renewal fails. Renewal reports retain `login` labels in text and JSON; credential renewal is not exposed as a public subcommand. The auth command boundary classifies missing credentials and Control Plane authentication failures as `UserError::NotAuthenticated`, token-storage plus `AuthError::Io`/`Storage` failures as `UserError::AuthStorageUnavailable` (`auth.storage_unavailable`), and approved user-facing rendering/prompt fallbacks as `UserError::UnexpectedFailure` (`general.unexpected_failure`); fixed catalog messages expose no storage or implementation details while technical sources remain available for structured observability, and remaining auth-command failures use `UnexpectedFailure` with preserved technical sources. +- `auth login stored-credential renewal`: The `sce auth login` behavior that first validates every stored credential through the existing non-forced token path, preserves valid credentials, refreshes expired credentials, and falls back to device authorization when credentials are absent or renewal fails. Renewal reports retain `login` labels in text and JSON; credential renewal is not exposed as a public subcommand. The auth command boundary treats missing-credential logout/whoami calls as successful state queries, classifies authenticated Control Plane failures as `UserError::NotAuthenticated`, token-storage plus `AuthError::Io`/`Storage` failures as `UserError::AuthStorageUnavailable` (`auth.storage_unavailable`), and approved user-facing rendering/prompt fallbacks as `UserError::UnexpectedFailure` (`general.unexpected_failure`); fixed catalog messages expose no storage or implementation details while technical sources remain available for structured observability, and remaining auth-command failures use `UnexpectedFailure` with preserved technical sources. - `command surface contract`: The current top-level command/help catalog split where `cli/src/cli_schema.rs` owns the real clap-backed command metadata (top-level purpose text plus help visibility for `auth`, `config`, `setup`, `doctor`, `hooks`, `policy`, `sync`, `version`, and `completion`) and `cli/src/command_surface.rs` consumes that catalog for the custom banner/help surface plus known-command classification, while still adding the synthetic `help` row. - `top-level help visibility metadata`: Per-command `show_in_top_level_help` metadata in `cli/src/cli_schema.rs` that controls whether a known command appears in `sce`, `sce help`, and `sce --help` without affecting direct invocation; the current hidden top-level commands are `hooks` and `policy`, while `auth` is visible, and `cli/src/command_surface.rs` renders the curated top-level help list from that shared metadata. - `command loop`: The `clap` derive-based parser + dispatcher in `cli/src/cli_schema.rs`, `cli/src/services/parse/command_runtime.rs`, and `cli/src/app.rs` that routes `help`, `config`, `setup`, `doctor`, `auth`, `hooks`, `policy`, `sync`, `version`, and `completion`, executes implemented command flows, emits command-local help payloads for supported subcommand trees, and returns deterministic actionable errors for invalid invocation. diff --git a/context/overview.md b/context/overview.md index 8ffca3e1..78eb73ff 100644 --- a/context/overview.md +++ b/context/overview.md @@ -19,7 +19,7 @@ The generated `/next-task` workflow persists task-level context-synchronization The CLI crate currently depends on `anyhow`, `chrono`, `clap`, `clap_complete`, `dirs`, `hmac`, `indicatif`, `inquire`, `jsonschema`, `keyring-core`, `murmur3`, `owo-colors`, `rand`, `reqwest`, `serde`, `serde_json`, `sha2`, `tokio`, `tracing`, `turso`, and `uuid`, with target-specific keyring backend dependencies for Linux/FreeBSD, macOS, and Windows. No CLI dev-dependencies are currently declared. 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. Auth command orchestration routes expected missing/authentication failures, token-storage failures, and approved user-facing fallback failures through typed `CliError::User` catalog entries (`NotAuthenticated`, `AuthStorageUnavailable`, `NotGitRepository`, and `UnexpectedFailure`) while retaining technical sources for observability; internal auth failures remain runtime errors. +The command loop now enforces a stable exit-code contract in `cli/src/app.rs`: `2` parse failures, `3` invocation validation failures, `4` runtime failures, and `5` dependency startup failures. Auth command orchestration routes authenticated Control Plane authentication failures, token-storage failures, and approved user-facing fallback failures through typed `CliError::User` catalog entries (`NotAuthenticated`, `AuthStorageUnavailable`, `NotGitRepository`, and `UnexpectedFailure`) while retaining technical sources for observability; missing-credential logout/whoami state queries succeed with their documented text/JSON reports, and internal auth failures remain runtime errors. The same runtime also emits stable CLI stderr diagnostics: internal failures use `SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, or `SCE-ERR-DEPENDENCY` in deterministic `Error []: ...` diagnostics with class-default `Try:` remediation appended when missing, while expected failures emit only their redacted, unstyled catalog messages. The command boundary's former flat, string-only `ClassifiedError` has been replaced by typed `CliError` in `cli/src/services/error.rs`: the expected-error variant carries a closed catalog (`NotAuthenticated`, the authentication-storage `AuthStorageUnavailable`, the setup `NotGitRepository`, and the general `UnexpectedFailure`) for expected, deliberately-explained failures rendered without the technical source, wrapper, styling, or automatic `Try:` guidance; the general entry renders one fixed static log-files guidance sentence without dynamic path text. `CliError::Internal` carries a live `anyhow::Error` source rendered as the real error chain with the existing styled wrapper and class-default remediation; `app_support` is the sole owner of the distinct terminal paths, and `sce sync` classifies authentication, credential-storage, and all other sync failures as cataloged user errors while preserving their technical sources for observability. 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. diff --git a/context/plans/fix-pr-223-error-classification-regressions.md b/context/plans/fix-pr-223-error-classification-regressions.md index 912a6843..feec31d3 100644 --- a/context/plans/fix-pr-223-error-classification-regressions.md +++ b/context/plans/fix-pr-223-error-classification-regressions.md @@ -88,13 +88,17 @@ Persist this field in every plan; this is durable plan state, not chat state: - Result: Setup repository-root resolution now returns a typed classification, mapping only Git-confirmed non-repository directories to `NotGitRepository` and preserving technical sources while mapping other resolution failures to `UnexpectedFailure`; focused tests cover real non-Git and nonexistent paths plus both sourced CLI mappings. - Context impact: domain — `context/cli/cli-command-surface.md`, `context/sce/cli-error-code-taxonomy.md`, and `context/architecture.md` now document positive-only setup repository classification and technical-source preservation; the five root context files require verification during synchronization. -- [ ] T03: `Restore idempotent auth state-query semantics` (status:todo) +- [x] T03: `Restore idempotent auth state-query semantics` (status:done) - Task ID: T03 - Scope: In — restore `render_logout_result(deleted, format)` and make absent-token logout a successful result; add `render_unauthenticated_whoami(format)` and make missing credentials a successful unauthenticated-state result; retain typed storage and authenticated Control Plane mappings, technical sources, existing successful JSON fields, and genuine failure behavior; add focused text/JSON tests for missing and removed credentials plus authenticated failure tests; update auth command surface, taxonomy, and architecture context wording. Out — changing login renewal/device flow, adding a new user-error catalog entry, or creating an ADR. - Dependencies: none - Done when: missing-token logout and whoami return `Ok(...)` with their existing text/JSON contracts, token deletion still reports success, authenticated `/me` and storage failures retain their typed errors and sources, and context no longer claims that observing logged-out state is `NotAuthenticated`. - - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auth_command::`; `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml app_support::`. - - Context synchronization: pending + - Verify: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auth_command::` — passed (5 tests); `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml app_support::` — passed (5 tests). + - Completed: 2026-08-26 + - Files changed: `cli/src/services/auth_command/mod.rs`, `context/architecture.md`, `context/cli/cli-command-surface.md`, `context/sce/cli-error-code-taxonomy.md` + - Result: Logout now succeeds idempotently and reports whether credentials were removed; unauthenticated whoami now returns its documented text/JSON state report, while authenticated and storage failures retain typed mappings and technical sources. Focused regression tests cover both output formats and authenticated failure classification. + - Context impact: domain — auth command state-query behavior, CLI error taxonomy, and architecture documentation; these context files now distinguish successful unauthenticated observation from genuine authentication failures. + - Context synchronization: synced ## Open questions diff --git a/context/sce/cli-error-code-taxonomy.md b/context/sce/cli-error-code-taxonomy.md index da58558d..1fbc8c8d 100644 --- a/context/sce/cli-error-code-taxonomy.md +++ b/context/sce/cli-error-code-taxonomy.md @@ -33,7 +33,7 @@ It complements the numeric process exit-code classes documented in `context/sce/ - `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`, `AuthStorageUnavailable`, `NotGitRepository`, or `UnexpectedFailure`) for expected, deliberately-explained failures; `CliError::Internal` carries a live `anyhow::Error` source for every other failure. `CliError::User` may also carry an optional preserved technical `source`, kept for observability only and never rendered to the terminal. -- `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 is a fixed, reviewed sentence returned by `UserError::message()`, keyed for structured logging by `UserError::key()`. `NotAuthenticated` (`auth.not_authenticated`) covers missing credentials from `sce auth logout`/`whoami` and Control Plane authentication failures from `whoami`; `AuthStorageUnavailable` (`auth.storage_unavailable`) is used by `sce sync` and the `sce auth login`, `logout`, and `whoami` command boundary for token-storage plus `AuthError::Io`/`Storage` failures. Both auth-command mappings preserve technical sources for observability. `NotGitRepository` (`setup.not_git_repository`) is used by the setup command only when setup-owned repository-root resolution positively identifies a target as outside a Git repository; it renders fixed Git-init/rerun guidance and preserves the technical source for observability. Other repository-root resolution failures, including nonexistent, inaccessible, process, and malformed-output failures, map to `UnexpectedFailure` (`general.unexpected_failure`). `UnexpectedFailure` is also used by `sce sync`, the config-command boundary for config execution failures, the version and doctor command boundaries for service execution failures, the auth-command boundary, and remaining setup execution failures; it renders one fixed, user-safe diagnostic sentence and has no automatic `Try:` suffix or dynamic path input. +- `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 is a fixed, reviewed sentence returned by `UserError::message()`, keyed for structured logging by `UserError::key()`. `NotAuthenticated` (`auth.not_authenticated`) covers Control Plane authentication failures from authenticated `sce auth whoami`; missing credentials from `sce auth logout`/`whoami` are successful state queries and do not enter the error catalog. `AuthStorageUnavailable` (`auth.storage_unavailable`) is used by `sce sync` and the `sce auth login`, `logout`, and `whoami` command boundary for token-storage plus `AuthError::Io`/`Storage` failures. Both auth-command mappings preserve technical sources for observability. `NotGitRepository` (`setup.not_git_repository`) is used by the setup command only when setup-owned repository-root resolution positively identifies a target as outside a Git repository; it renders fixed Git-init/rerun guidance and preserves the technical source for observability. Other repository-root resolution failures, including nonexistent, inaccessible, process, and malformed-output failures, map to `UnexpectedFailure` (`general.unexpected_failure`). `UnexpectedFailure` is also used by `sce sync`, the config-command boundary for config execution failures, the version and doctor command boundaries for service execution failures, the auth-command boundary, and remaining setup execution failures; it renders one fixed, user-safe diagnostic sentence and has no automatic `Try:` suffix or dynamic path input. - Command and domain layers construct and return a `CliError`; they do not format terminal text or apply styling. Auth command orchestration classifies expected authentication and credential-storage failures into the existing `UserError` catalog by typed domain variants, never by string matching, and preserves the original technical chain as the optional user-error source. `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 stderr rendering: it redacts and writes the catalog variant's message without a wrapper or styling, while `CliError::Internal` retains code-bearing rendering and styles its 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. From 10a930e25651c1d13705441bf1b1ff60ef179152 Mon Sep 17 00:00:00 2001 From: David Abram Date: Wed, 26 Aug 2026 15:32:39 +0200 Subject: [PATCH 8/8] auth: Fix color policy for text state queries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prevent `auth logout` and unauthenticated `whoami` text output from acquiring ANSI styling in non-color contexts by passing the renderer's color decision explicitly. Preserve JSON payloads and existing logout semantics, and record the completed regression criteria and validation evidence. Plan: fix-pr-223-error-classification-regressions (AC1–AC7) Co-authored-by: SCE --- cli/src/services/auth_command/mod.rs | 42 ++++++++++++--- cli/src/services/style.rs | 6 ++- ...pr-223-error-classification-regressions.md | 52 ++++++++++++++++--- 3 files changed, 84 insertions(+), 16 deletions(-) diff --git a/cli/src/services/auth_command/mod.rs b/cli/src/services/auth_command/mod.rs index ec764715..7ac4d2b7 100644 --- a/cli/src/services/auth_command/mod.rs +++ b/cli/src/services/auth_command/mod.rs @@ -326,9 +326,21 @@ fn render_login_refresh_result(tokens: &StoredTokens, format: AuthFormat) -> Res } fn render_logout_result(deleted: bool, format: AuthFormat) -> Result { + render_logout_result_with_color_policy( + deleted, + format, + crate::services::style::supports_color(), + ) +} + +fn render_logout_result_with_color_policy( + deleted: bool, + format: AuthFormat, + color_enabled: bool, +) -> Result { match format { AuthFormat::Text => Ok(if deleted { - success("Logged out") + crate::services::style::success_with_color_policy("Logged out", color_enabled) } else { value("No user logged in") }), @@ -344,10 +356,20 @@ fn render_logout_result(deleted: bool, format: AuthFormat) -> Result { } fn render_unauthenticated_whoami(format: AuthFormat) -> Result { + render_unauthenticated_whoami_with_color_policy( + format, + crate::services::style::supports_color(), + ) +} + +fn render_unauthenticated_whoami_with_color_policy( + format: AuthFormat, + color_enabled: bool, +) -> Result { match format { AuthFormat::Text => Ok(format!( "You are not logged in. Please log in using the {} command.", - success("sce auth login") + crate::services::style::success_with_color_policy("sce auth login", color_enabled) )), AuthFormat::Json => serde_json::to_string_pretty(&json!({ "status": "ok", @@ -441,11 +463,13 @@ mod tests { #[test] fn logout_text_reports_whether_credentials_were_removed() { assert_eq!( - render_logout_result(false, AuthFormat::Text).expect("logout should render"), + render_logout_result_with_color_policy(false, AuthFormat::Text, false) + .expect("logout should render"), "No user logged in" ); assert_eq!( - render_logout_result(true, AuthFormat::Text).expect("logout should render"), + render_logout_result_with_color_policy(true, AuthFormat::Text, false) + .expect("logout should render"), "Logged out" ); } @@ -453,11 +477,13 @@ mod tests { #[test] fn logout_json_reports_whether_credentials_were_removed() { let absent: serde_json::Value = serde_json::from_str( - &render_logout_result(false, AuthFormat::Json).expect("logout should render"), + &render_logout_result_with_color_policy(false, AuthFormat::Json, false) + .expect("logout should render"), ) .expect("logout JSON should be valid"); let present: serde_json::Value = serde_json::from_str( - &render_logout_result(true, AuthFormat::Json).expect("logout should render"), + &render_logout_result_with_color_policy(true, AuthFormat::Json, false) + .expect("logout should render"), ) .expect("logout JSON should be valid"); @@ -470,7 +496,7 @@ mod tests { #[test] fn unauthenticated_whoami_renders_text_guidance() { assert_eq!( - render_unauthenticated_whoami(AuthFormat::Text) + render_unauthenticated_whoami_with_color_policy(AuthFormat::Text, false) .expect("unauthenticated whoami should render"), "You are not logged in. Please log in using the sce auth login command." ); @@ -479,7 +505,7 @@ mod tests { #[test] fn unauthenticated_whoami_json_reports_state() { let report: serde_json::Value = serde_json::from_str( - &render_unauthenticated_whoami(AuthFormat::Json) + &render_unauthenticated_whoami_with_color_policy(AuthFormat::Json, false) .expect("unauthenticated whoami should render"), ) .expect("whoami JSON should be valid"); diff --git a/cli/src/services/style.rs b/cli/src/services/style.rs index f448b4d1..7c1f2e8e 100644 --- a/cli/src/services/style.rs +++ b/cli/src/services/style.rs @@ -39,10 +39,14 @@ where style_if(text, supports_color(), f) } -pub(crate) fn success_with_stderr_color_policy(text: &str, color_enabled: bool) -> String { +pub(crate) fn success_with_color_policy(text: &str, color_enabled: bool) -> String { style_if(text, color_enabled, |s| s.green().bold().to_string()) } +pub(crate) fn success_with_stderr_color_policy(text: &str, color_enabled: bool) -> String { + success_with_color_policy(text, color_enabled) +} + #[must_use] pub fn heading(text: &str) -> String { heading_with_color_policy(text, supports_color()) diff --git a/context/plans/fix-pr-223-error-classification-regressions.md b/context/plans/fix-pr-223-error-classification-regressions.md index feec31d3..6e230107 100644 --- a/context/plans/fix-pr-223-error-classification-regressions.md +++ b/context/plans/fix-pr-223-error-classification-regressions.md @@ -8,19 +8,19 @@ The fixes are deliberately split into three independently testable atomic commit ## Acceptance criteria -- [ ] AC1: Initial control-plane, stream-terminal, and stream-refresh `ControlPlaneError::Storage` failures all classify as `auth.storage_unavailable`; stream authentication remains `auth.not_authenticated`; other control-plane/runtime failures remain `general.unexpected_failure`, with technical `TraceSyncError` sources attached. +- [x] AC1: Initial control-plane, stream-terminal, and stream-refresh `ControlPlaneError::Storage` failures all classify as `auth.storage_unavailable`; stream authentication remains `auth.not_authenticated`; other control-plane/runtime failures remain `general.unexpected_failure`, with technical `TraceSyncError` sources attached. - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml sync::command` and `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml sync::`; inspect the classifier to confirm it remains unchanged and uses typed predicates rather than human-readable strings. -- [ ] AC2: Setup emits `setup.not_git_repository` only when the setup domain positively identifies a target as outside a Git repository; nonexistent, inaccessible, process, malformed-output, and unrelated filesystem failures classify as `general.unexpected_failure`, and both typed paths preserve technical sources. +- [x] AC2: Setup emits `setup.not_git_repository` only when the setup domain positively identifies a target as outside a Git repository; nonexistent, inaccessible, process, malformed-output, and unrelated filesystem failures classify as `general.unexpected_failure`, and both typed paths preserve technical sources. - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml setup::`; inspect the setup classifier for typed `GitRepositoryResolutionError` matching with no CLI-layer string matching. -- [ ] AC3: `sce auth logout` with no stored credentials succeeds with the existing text and JSON state-query semantics, including `credentials_removed: false`; deleting stored credentials still succeeds with `credentials_removed: true`. +- [x] AC3: `sce auth logout` with no stored credentials succeeds with the existing text and JSON state-query semantics, including `credentials_removed: false`; deleting stored credentials still succeeds with `credentials_removed: true`. - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auth_command::` plus focused text/JSON assertions for absent and present credentials. -- [ ] AC4: `sce auth whoami` with no stored credentials succeeds with the existing unauthenticated text guidance and JSON payload (`authentication_state: unauthenticated`, `has_stored_credentials: false`), while authenticated `/me` failures retain typed `NotAuthenticated`, `AuthStorageUnavailable`, or `UnexpectedFailure` mappings and technical sources. +- [x] AC4: `sce auth whoami` with no stored credentials succeeds with the existing unauthenticated text guidance and JSON payload (`authentication_state: unauthenticated`, `has_stored_credentials: false`), while authenticated `/me` failures retain typed `NotAuthenticated`, `AuthStorageUnavailable`, or `UnexpectedFailure` mappings and technical sources. - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auth_command::` plus focused missing-credential and authenticated-failure assertions. -- [ ] AC5: Genuine auth storage failures retain `auth.storage_unavailable`, stored credentials rejected by the Control Plane retain `auth.not_authenticated`, and all genuine failures retain exit code `4`, stdout/stderr routing, and machine-readable JSON contracts. +- [x] AC5: Genuine auth storage failures retain `auth.storage_unavailable`, stored credentials rejected by the Control Plane retain `auth.not_authenticated`, and all genuine failures retain exit code `4`, stdout/stderr routing, and machine-readable JSON contracts. - Validate: `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auth_command::` and `./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml app_support::`. -- [ ] AC6: The closed `UserError` catalog and typed-error architecture remain intact: no arbitrary message variant, no CLI-boundary human-readable string classification, no rollback to the pre-PR architecture, and no new ADR for this regression repair. +- [x] AC6: The closed `UserError` catalog and typed-error architecture remain intact: no arbitrary message variant, no CLI-boundary human-readable string classification, no rollback to the pre-PR architecture, and no new ADR for this regression repair. - Validate: inspect `cli/src/services/error.rs`, `cli/src/services/sync/command.rs`, and `cli/src/services/setup/command.rs`; confirm no `UserError::Message`/`Custom` variant and no CLI-layer error-string matching. -- [ ] AC7: Durable context accurately documents sync storage propagation, positive-only setup repository classification, and successful unauthenticated auth state queries, with no stale claim that missing logout/whoami credentials are `NotAuthenticated` failures. +- [x] AC7: Durable context accurately documents sync storage propagation, positive-only setup repository classification, and successful unauthenticated auth state queries, with no stale claim that missing logout/whoami credentials are `NotAuthenticated` failures. - Validate: `nix run .#pkl-check-generated` and targeted inspection of the context files listed under Context sync. ### Full validation @@ -103,3 +103,41 @@ Persist this field in every plan; this is durable plan state, not chat state: ## Open questions None. The request specifies the three regressions, the required typed boundaries, preserved contracts, tests, context updates, atomic commit messages, and final validation commands. The code inspection confirms the regressions are present at the stated PR head; no smaller change covers all three independent user-visible failures. + +## Validation Report + +**Status:** validated +**Date:** 2026-08-26 + +### Commands run + +- `nix develop -c ./scripts/run-cli-cargo.sh fmt --manifest-path cli/Cargo.toml -- --check` -> exit 0 (format check passed) +- `nix develop -c ./scripts/run-cli-cargo.sh clippy --manifest-path cli/Cargo.toml -- -D warnings` -> exit 0 (clippy passed with warnings denied) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml` -> exit 0 (622 tests passed) +- `nix run .#pkl-check-generated` -> exit 0 (ephemeral generation parity passed) +- `nix flake check` -> exit 0 (all checks passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml sync::command` -> exit 0 (9 tests passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml sync::` -> exit 0 (66 tests passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml setup::` -> exit 0 (67 tests passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml auth_command::` -> exit 0 (5 tests passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml app_support::` -> exit 0 (5 tests passed) +- Authorized inspection of `cli/src/services/error.rs`, `cli/src/services/sync/command.rs`, and `cli/src/services/setup/command.rs` -> passed (closed catalog and typed classifier boundaries confirmed; no arbitrary variants or CLI string matching) +- Authorized inspection of the four Context sync files -> passed (sync storage propagation, positive-only setup classification, and successful unauthenticated auth state queries are documented) + +### Success-criteria verification + +- [x] AC1: Initial, terminal-stream, and refresh-stream storage failures classify as `auth.storage_unavailable`; authentication and other failures retain their classifications and sources -> focused sync suites passed. +- [x] AC2: Setup uses positive-only non-Git classification and preserves sources for non-Git and unexpected resolution failures -> focused setup suite passed. +- [x] AC3: Logout is idempotent and preserves text/JSON credential-removal semantics -> focused auth suite passed. +- [x] AC4: Unauthenticated whoami succeeds with text/JSON state reports and authenticated failures retain typed mappings and sources -> focused auth suite passed. +- [x] AC5: Auth storage/authentication failures and output contracts retain their mappings and runtime behavior -> focused auth and app-support suites passed. +- [x] AC6: Closed `UserError` catalog and typed, non-string CLI boundaries remain intact -> authorized source inspection passed. +- [x] AC7: Durable context documents the corrected behavior -> `pkl-check-generated` and authorized context inspection passed. + +### Failed checks and follow-ups + +- None. + +### Residual risks + +- None identified.