From 890da82aab63dba32a4eb643f7be43b674f35c60 Mon Sep 17 00:00:00 2001 From: Ivan Ivic Date: Mon, 24 Aug 2026 13:56:08 +0200 Subject: [PATCH 1/5] setup: Enforce Git remote preflight before setup Require every setup mode to validate its configured Agent Trace remote before prompts or writes. Add typed actionable errors while preserving technical sources without exposing remote URLs, with focused regression coverage and updated contracts. Co-authored-by: SCE --- cli/src/services/error.rs | 15 +- cli/src/services/setup/command.rs | 19 +- cli/src/services/setup/mod.rs | 18 ++ context/cli/cli-command-surface.md | 4 +- context/glossary.md | 2 +- context/overview.md | 36 ++-- context/plans/setup-git-remote-preflight.md | 165 ++++++++++++++++++ context/sce/cli-error-code-taxonomy.md | 5 +- context/sce/setup-githooks-cli-ux.md | 5 +- .../sce/setup-repo-local-config-bootstrap.md | 10 +- 10 files changed, 245 insertions(+), 34 deletions(-) create mode 100644 context/plans/setup-git-remote-preflight.md diff --git a/cli/src/services/error.rs b/cli/src/services/error.rs index 5e2b487c..b9238d7c 100644 --- a/cli/src/services/error.rs +++ b/cli/src/services/error.rs @@ -51,15 +51,20 @@ impl FailureClass { /// Catalog of expected, deliberately-explained failures presented to the user /// as a friendly diagnostic instead of a technical error chain. #[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[allow(clippy::enum_variant_names)] pub enum UserError { #[allow(dead_code)] NotAuthenticated, + NotGitRepository, + NotGitRemote, } impl UserError { pub fn class(self) -> FailureClass { match self { - Self::NotAuthenticated => FailureClass::Runtime, + Self::NotAuthenticated | Self::NotGitRepository | Self::NotGitRemote => { + FailureClass::Runtime + } } } @@ -67,6 +72,8 @@ impl UserError { pub fn key(self) -> &'static str { match self { Self::NotAuthenticated => "auth.not_authenticated", + Self::NotGitRepository => "setup.not_git_repository", + Self::NotGitRemote => "setup.not_git_remote", } } @@ -75,6 +82,12 @@ impl UserError { Self::NotAuthenticated => { "You are not logged in. Please log in using the `sce auth login` command." } + Self::NotGitRepository => { + "The target directory is not a Git repository. Please run `git init`, then retry." + } + Self::NotGitRemote => { + "The Git repository has no configured remote URL. Please run `git remote add `, then retry." + } } } } diff --git a/cli/src/services/setup/command.rs b/cli/src/services/setup/command.rs index 86bb6fe3..b0d08b63 100644 --- a/cli/src/services/setup/command.rs +++ b/cli/src/services/setup/command.rs @@ -1,11 +1,11 @@ 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, }; -use crate::services::setup; +use crate::services::{config, setup}; pub struct SetupCommand { pub request: setup::SetupRequest, @@ -22,8 +22,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(CliError::runtime)?; + let repository_root = resolve_setup_repository(&setup_start_path)?; let setup_dispatch = if self.request.context_only { None @@ -102,6 +101,18 @@ impl SetupCommand { } } +fn resolve_setup_repository(start_path: &std::path::Path) -> Result { + let repository_root = setup::ensure_git_repository(start_path) + .map_err(|source| CliError::user_with_source(UserError::NotGitRepository, source))?; + let storage_config = config::resolve_agent_trace_storage_runtime_config(&repository_root) + .map_err(CliError::runtime)?; + + setup::ensure_git_remote(&repository_root, &storage_config.repository_remote) + .map_err(|source| CliError::user_with_source(UserError::NotGitRemote, source))?; + + Ok(repository_root) +} + fn setup_required_hooks_outcome_from_lifecycle( outcome: &RequiredHooksInstallOutcome, ) -> setup::RequiredHooksInstallOutcome { diff --git a/cli/src/services/setup/mod.rs b/cli/src/services/setup/mod.rs index 76e0d625..4705ad9e 100644 --- a/cli/src/services/setup/mod.rs +++ b/cli/src/services/setup/mod.rs @@ -424,6 +424,24 @@ pub fn ensure_git_repository(directory: &Path) -> Result { install::ensure_git_repository(directory) } +/// Preflight check that verifies the named Git remote has a configured URL. +/// The URL itself is intentionally discarded so callers can preserve a +/// technical diagnostic without echoing credential-bearing remote values. +pub fn ensure_git_remote(repository_root: &Path, remote_name: &str) -> Result<()> { + if crate::services::repository_identity::resolve::lookup_remote_url( + repository_root, + remote_name, + ) + .is_some() + { + return Ok(()); + } + + bail!( + "Git remote '{remote_name}' has no configured URL. Try: run 'git remote add {remote_name} ', then rerun 'sce setup'." + ) +} + /// Bootstraps the repo-local `.sce/config.json` file if it does not already exist. /// /// Creates the `.sce/` parent directory as needed, then writes the canonical diff --git a/context/cli/cli-command-surface.md b/context/cli/cli-command-surface.md index 63891f8c..1108fb94 100644 --- a/context/cli/cli-command-surface.md +++ b/context/cli/cli-command-surface.md @@ -56,7 +56,7 @@ Deferred or gated command surfaces currently avoid claiming unimplemented behavi `config` exposes deterministic inspect/validate entrypoints (`sce config show`, `sce config validate`) with explicit precedence (`flags > env > config file > defaults`), a shared auth-runtime resolver for supported keys that declare env/config/optional baked-default inputs starting with `workos_client_id`, first-class `policies.bash` reporting for preset/custom blocked-command rules, and deterministic text/JSON output modes where `show` reports resolved values with provenance while `validate` reports pass/fail plus validation issues and warnings only. `version` exposes deterministic runtime identification output in text mode by default and JSON mode via `--format json`. `completion` exposes deterministic shell completion generation via `sce completion --shell `. -`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` 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. Every setup mode first validates the Git repository and effective `agent_trace.repository_remote` (default `origin`) before prompts or writes. `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 ensures that baseline only after both preflights. `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`. @@ -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; `cli/src/services/setup/command.rs` owns the setup runtime command handler and runs the Git-root plus effective named-remote preflights before prompts or writes, mapping failures to typed `UserError` values with preserved technical sources. After both gates, 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 e43633ce..8af1c5cf 100644 --- a/context/glossary.md +++ b/context/glossary.md @@ -37,7 +37,7 @@ - `split commit references`: The generated `sce-commit/references/atomic-commit.md`, `references/commit-message-style.md`, and `references/output.md` documents. The atomic reference owns staged-diff procedure, internal result branching, and commit boundaries; the style reference owns commit-message wording; and the output reference owns human-visible layouts. No `commit-contract.yaml` artifact or YAML result-contract section is generated. - `canonical OpenCode plugin registration source`: Shared Pkl-authored plugin-registration definition in `config/pkl/base/opencode.pkl`, re-exported from `config/pkl/renderers/common.pkl` as the canonical plugin list/path JSON consumed by OpenCode renderers before they emit generated `opencode.json` manifests; the current entries are `sce-bash-policy` and `sce-agent-trace`. - `checkout identity`: Stable UUIDv7 identifier assigned to a cloned repository or linked Git worktree, stored in `/sce/checkout-id` (never committed) and resolved via `git rev-parse --git-dir`. The identity is created or reused by `sce setup` through `AgentTraceDbLifecycle::setup()` and also auto-created by hook runtime when `sce setup` has not been run. Checkout identity is now diagnostic metadata for repository-scoped Agent Trace storage; it does not select the active DB and is not stored on Agent Trace rows. Any pre-migration per-checkout DB files at `/sce/agent-trace-{checkout_id}.db` are never touched by SCE and are no longer inspectable via the CLI (the checkout-scoped DB surface was removed by the `retire-legacy-agent-trace-db` plan). See `context/cli/checkout-identity.md`. -- `repository identity`: Stable identity of a logical Git repository used to select the active repository-scoped Agent Trace DB path `/sce/repos//agent-trace.db` through the `agent_trace_storage` resolver. Resolved by `cli/src/services/repository_identity/` with precedence: explicit `agent_trace.repository_id` config value, then the URL of the configured Git remote (`agent_trace.repository_remote`, default `origin`), otherwise an actionable `.sce/config.json` error. Remote URLs canonicalize to a scheme-neutral, credential-free `host[:port]/path` form so equivalent SSH/SCP/HTTPS remotes converge, and the repository ID is `sha256("sce-repository-id-v1\0" + canonical_identity)` hex. Distinct from `checkout identity`, which stays per clone/worktree for diagnostics. See `context/cli/repository-identity.md`. +- `repository identity`: Stable identity of a logical Git repository used to select the active repository-scoped Agent Trace DB path `/sce/repos//agent-trace.db` through the `agent_trace_storage` resolver. Resolved by `cli/src/services/repository_identity/` with precedence: explicit `agent_trace.repository_id` config value, then the URL of the configured Git remote (`agent_trace.repository_remote`, default `origin`), otherwise an actionable `.sce/config.json` error. Remote URLs canonicalize to a scheme-neutral, credential-free `host[:port]/path` form so equivalent SSH/SCP/HTTPS remotes converge, and the repository ID is `sha256("sce-repository-id-v1\0" + canonical_identity)` hex. Distinct from `checkout identity`, which stays per clone/worktree for diagnostics. Setup's named-remote preflight delegates to the same lookup and discards the URL, retaining only the remote name in technical failure sources. See `context/cli/repository-identity.md`. - `refresh single-flight guard`: Client-owned async coordination for Agent Trace control-plane authentication. Concurrent callers whose stored access token is expired, or whose request rejected the same token, serialize only refresh-and-save work, re-check credentials after acquiring the guard, and reuse the token saved by the first refresher; valid-token requests do not acquire the guard. - `repository-scoped Agent Trace DB`: Active Agent Trace storage shape where one logical Git repository maps to `/sce/repos//agent-trace.db`. The current seam is `RepositoryAgentTraceDb = TursoDb` in `cli/src/services/agent_trace_db/repository.rs`, backed by the fresh multi-statement `001_repository_schema` baseline plus the additive `002_repository_source_instance_id` migration, with `repository_metadata` (`repository_id` plus `source_instance_id`) plus repository-level trace tables, no `checkout_id` columns, and typed repository-level insert helpers for diff traces, post-commit intersections, Agent Trace rows, messages, and parts. Hook runtime, Agent Trace setup/lifecycle, and `sce sync` resolve repository-scoped storage through `agent_trace_storage`. This is the sole Agent Trace DB adapter; the checkout-scoped adapter and former trace inspection surface were removed by the `retire-legacy-agent-trace-db` plan. - `source_instance_id`: Physical-database identity column on `repository_metadata`, independent of the logical `repository_id`. Added by the additive `002_repository_source_instance_id` migration (existing/placeholder rows default to an empty string); generated once per physical `agent-trace.db` by application code (`generate_source_instance_id()`, UUID v4 today) and validated with `is_valid_source_instance_id()` (non-empty once trimmed) — never generated in SQL and never derived from `repository_id`, remote URL, checkout ID, filesystem path, hostname, or user/workspace identity. `RepositoryAgentTraceDb::verify_or_initialize_repository_metadata` claims it with a concurrency-safe `UPDATE ... WHERE source_instance_id = ''`, so concurrent first opens of the same physical database converge on one winner and an already-valid value is never overwritten; the value stays stable across reopen and repeated `sce setup` runs. Two independently created databases for the same logical repository (for example two clones) get different `source_instance_id` values. See `context/sce/agent-trace-db.md`. diff --git a/context/overview.md b/context/overview.md index 637ad8b8..951d0d27 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 user-facing stderr error classes (`SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, `SCE-ERR-DEPENDENCY`) using deterministic `Error []: ...` diagnostics with class-default `Try:` remediation appended when missing. The command boundary's former flat, string-only `ClassifiedError` has been replaced by typed `CliError` in `cli/src/services/error.rs`: `CliError::User` carries a closed `UserError` catalog (`NotAuthenticated`, `NotGitRepository`, and `NotGitRemote`) for expected, deliberately-explained failures rendered as fixed friendly sentences 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`. Setup preflight errors preserve technical sources for observability while keeping raw remote URLs out of user-facing diagnostics. See `context/sce/cli-error-code-taxonomy.md` for the full contract. The app runtime now also includes a structured observability baseline in `cli/src/services/observability.rs`: deterministic env-controlled log threshold/format (`SCE_LOG_LEVEL` defaults to `error`; `SCE_LOG_FORMAT` defaults to `text`), default-backed log-directory routing (`SCE_LOG_DIR` / config-file `log_dir` / `/sce/logs`) with per-operation machine-local dated file selection, optional session filename partitioning, and creation-triggered retention of direct regular `*.log` files to 10 entries, stable lifecycle event IDs, 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. @@ -36,6 +36,8 @@ Invalid default-discovered config files now also degrade gracefully at startup: The shared default path service in`cli/src/services/default_paths.rs`is now the canonical owner for production CLI path definitions. It resolves per-user config/state/cache roots through a dedicated internal`roots`seam, exposes the current persisted-artifact inventory (global config and auth tokens), and also defines named DB paths (auth DB, local DB, Agent Trace DB) plus the repo-relative, install, hook, and context-path accessors consumed across current CLI production code. Non-test production modules should consume this shared catalog instead of hardcoding owned path literals. No default cache-backed persisted artifact currently exists, so cache-root resolution remains available without speculative cache-path features and no legacy default-path fallback is supported. The Rust CLI also centralizes SCE-owned web URI construction in`cli/src/services/agent_trace.rs`, with `SCE_WEB_BASE_URL`as the single Rust owner for`https://sce.crocoder.dev` and helpers consumed by Agent Trace conversation URLs, Agent Trace persisted trace URLs, Agent Trace session URLs, and setup-created repo-local config schema URLs. The config resolver separately owns `control_plane_base_url` and its `https://sce.crocoderlab.dev` baked sync default; the two URL owners must not be conflated. The current user-facing synchronization entrypoint is `sce sync`; references to the former nested spelling in historical records do not describe an available command. + +Setup repository preflight: every `sce setup` mode, including `--bootstrap-context`, validates an initialized Git repository and the configured `agent_trace.repository_remote` URL (default `origin`) before prompts, context/bootstrap, lifecycle setup, or integration writes. `services/setup/command.rs` maps missing prerequisites to typed `NotGitRepository` and `NotGitRemote` diagnostics while preserving technical sources without rendering remote URLs. Sync owns the complete progress boundary in `cli/src/services/sync/progress.rs`: the consumer-typed `ProgressReporter` contract, no-op reporter, focused contract tests, and fixed `indicatif` terminal adapter. `SyncProgressEvent` remains owned by `cli/src/services/sync/sync.rs`; `sync/command.rs` selects the adapter or no-op implementation by output format, there is no top-level `cli/src/services/progress/` module, and JSON callers use the sync-owned no-op reporter. The same config resolver now also owns the attribution-hooks gate used by local hook runtime: opt-out env `SCE_ATTRIBUTION_HOOKS_DISABLED` overrides `policies.attribution_hooks.enabled` with inverted semantics, and the gate defaults to enabled unless explicitly disabled. The config service split now includes `cli/src/services/config/resolver.rs` as the focused owner for config-file discovery, file-layer merging, env/flag/default precedence, auth-key resolution, observability resolution, attribution-hooks resolution, and default-discovered invalid-file degradation; `cli/src/services/config/mod.rs` remains the facade/rendering orchestration surface while preserving existing `services::config` imports. @@ -57,22 +59,22 @@ The checked-in Flatpak packaging surface lives under `packaging/flatpak/`with Ni The current supported automated release target matrix is `x86_64-unknown-linux-musl`, `aarch64-unknown-linux-musl`, and `aarch64-apple-darwin`; npm launcher platform support remains a separate current-state surface documented in the npm distribution contract and launcher code. - Native release binary portability auditing is exposed as `nix run .#native-portability-audit -- --binary [--platform auto|linux|macos]` plus the `native-portability-audit` flake check; it reports forbidden `/nix/store/` runtime references found by Linux ELF/string inspection or macOS `otool -L` install-name inspection. `release-artifacts` runs that audit against the staged `bin/sce` before tarball creation and, on macOS, rewrites Nix-store `libiconv.*.dylib` install names to `/usr/lib/...` with ad-hoc re-signing before the audit. The three native reusable release workflows also extract the generated archive, smoke-run `bin/sce version --format json`, and rerun the native portability audit before uploading native artifacts. -The downstream publish-stage implementation is now complete for both registries: `.github/workflows/publish-crates.yml` publishes the checked-in crate version after `.version`/tag/Cargo parity checks, and `.github/workflows/publish-npm.yml` publishes the checked-in npm package after `.version`/tag/npm parity checks plus verification of the canonical `sce-v-npm.tgz` GitHub release asset. -The repository root now also owns the canonical Biome contract for the current JavaScript tooling slice: `biome.json` scopes formatting/linting to `npm/` and the shared `config/lib/` plugin package root while excluding package-local `node_modules/`, and the root Nix dev shell provides the `biome` binary so contributors do not need a host-installed formatter/linter for those areas. -Flatpak validation/build orchestration is reduced to a minimal app surface: Linux flake apps expose the umbrella `sce-flatpak` (`nix run .#sce-flatpak -- ` for `validate`, `prepare-local-manifest`, etc.) plus `release-flatpak-package`, `release-flatpak-bundle`, and the `regenerate-flatpak-manifest` / `regenerate-cargo-sources` helpers; the previously separate `flatpak-validate`, `flatpak-local-manifest`, and `flatpak-build` wrapper apps are removed. Default `nix flake check` keeps the lightweight Nix-built static/AppStream validator plus the parity checks (`flatpak-manifest-parity`, `cargo-sources-parity`) and does not run a network-heavy Flatpak build. The former standalone install-channel integration runner and `install-channel-integration-tests` flake app are not active current-state surfaces. -Shared Context Plan and Shared Context Code remain separate OpenCode routing roles: the generated Plan agent routes only to `/change-to-plan`, while the generated Code agent routes to `/next-task`, `/validate`, `/commit`, `/handover`, and `/brownfield`. Workflow behavior lives in the six workflow entrypoints and their six skill packages rather than in agent bodies. `config/pkl/base/workflow-catalog.pkl` assigns each workflow to its role, and OpenCode command routing plus each agent's ordered `skill:` permissions derive from those records: ordinary non-SCE skills are allowed by the wildcard, arbitrary `sce-*` skills are denied, and only the role's owned workflows are allowed after that deny — `sce-change-to-plan` for Plan; `sce-next-task`, `sce-validate`, `sce-commit`, `sce-handover`, and `sce-brownfield` for Code. The Code agent additionally allows `sce-decision` for task synchronization. -The canonical workflow definitions remain phase-decomposed as authoring source: `/change-to-plan` sequences `sce-context-load` then `sce-plan-authoring`; `/next-task` sequences `sce-plan-review`, `sce-task-execution`, and `sce-task-context-sync`; `/validate` runs `sce-validation` only and reports its Validation Report; `/commit` delegates staged-diff analysis and message generation to `sce-atomic-commit`; `/handover` has no phases, since writer and loader mode has no SCE sibling handoff or wait mid-run; `/brownfield` likewise has none, since its single skill owns investigation, the blocking clarification gate, writing, and reporting itself. Relevant non-SCE skills may help inside an active workflow step, but they return control to that step without changing its canonical invariants. No target generates those phase modules as packages. All four consume them as inputs to the shared `workflow-composite.pkl` renderer, which composes each workflow into one skill package. Every workflow supplies typed package/composite render values for frontmatter, bodies, semantic references, phases, persisted-document formats where applicable, and output references; the composite renderer performs no prose-wide internalization or frontmatter stripping. -Every target preserves the same gates and lifecycle semantics through six renderer-composed workflow packages: `sce-change-to-plan`, `sce-next-task`, `sce-validate`, `sce-commit`, `sce-handover`, and `sce-brownfield`. Each thin command or Pi prompt invokes exactly one corresponding skill, and OpenCode command frontmatter names that single skill as both `entry-skill` and the whole `skills` chain. Each phase-based package keeps control flow, internal status branching, waits, and same-session resume in `SKILL.md`, while package-local Markdown references own phase instructions and persisted-document formats; `references/output.md` remains the sole definition of human-visible gates and terminal Markdown. Phase-free `/handover` retains `SKILL.md`, `references/handover-template.md`, and `references/output.md`, while `/brownfield` retains `SKILL.md` plus `references/output.md`. No target emits phase-skill packages or inter-skill machine contracts; phase statuses stay internal to one skill invocation. -Context sync uses an important-change gate: cross-cutting/policy/architecture/terminology changes require root shared-file edits, while localized tasks run verify-only root checks without default churn. -OpenCode and Claude no longer generate legacy bootstrap or context-sync skills; `/commit` and `/handover` are generated only as catalog-registered composite workflow packages. OpenCode retains only thin routing agents, while Claude emits no agents. The superseded grouped Markdown catalog and automated OpenCode profile have been removed from Pkl ownership and generated outputs. -The prior no-git-wrapper Agent Trace design artifacts under `context/sce/agent-trace-*.md` are retained only as historical reference; the current CLI runtime no longer wires the removed Agent Trace schema adaptation, payload building, retry replay, or rewrite handling paths into local hook execution. -The hooks service now uses a minimal attribution-only runtime: `commit-msg` is the only hook that mutates behavior, conditionally injecting exactly one canonical SCE trailer when the attribution-hooks gate is enabled, `SCE_DISABLED` is false, and the staged-diff AI-overlap preflight confirms AI/editor evidence (`StagedDiffAiOverlapResult::Overlap`); when the preflight returns `NoOverlap` or `Error` (including DB open failure, schema not ready, query error, staged diff read failure, or zero overlap), the trailer is not appended and errors are logged via `sce.hooks.commit_msg.ai_overlap_error`; `pre-commit` and `post-rewrite` remain deterministic no-op entrypoints; `post-commit` requires validated `--remote-url`, threads that URL through the Agent Trace flow, prints it to stderr, captures current commit patch, queries recent `diff_traces` from past 7 days (dispatching `patch` rows through existing unified-diff parsing and `structured` rows through `structured_patch::derive_claude_structured_patch` at read time, then assigning each structured hunk the persisted row model and every structured touched line the persisted canonical `cc_...` row session), combines/intersects patches, persists intersection metadata to `post_commit_patch_intersections`, and persists the schema-validated built Agent Trace payload, including optional top-level `tool` metadata from recent diff-trace rows, top-level `metadata.sce.version` from the compiled `sce` CLI package version, always-emitted `metadata.sce.line_changes` touched-line attribution counts, and range-level `content_hash` values, to AgentTraceDb `agent_traces` (DB-only, no post-commit Agent Trace file artifact); `diff-trace` currently validates/persists required non-empty `sessionID`/`diff`/`tool_name`, optional `model_id` (absent or `null` → `None`, present+non-empty → `Some`, present+empty → error), required nullable/non-empty `tool_version`, plus required `u64` millisecond `time`, uses direct-first/event-transcript-second Claude `model_id` resolution plus direct `tool_version` without any `session_models` runtime, and continues with `None` when event-local lookup cannot resolve attribution, with same-tool-idempotent stored `session_id` prefixing (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi), non-lossy AgentTraceDb `time_ms` conversion, and no parsed-payload artifact fallback. Claude structured `PostToolUse` diff-trace intake prefers direct top-level or nested model metadata, then matches the event's `tool_use_id` in its JSONL `transcript_path`, normalizes either source once with the `claude/` prefix, and fails open to `None`. -The CLI now also includes an approved operator-environment doctor contract documented in `context/sce/agent-trace-hook-doctor.md`; the runtime includes `sce doctor --fix` parsing/help, stable problem/fix-result reporting, canonical hook-repair reuse, bounded doctor-owned local-DB directory bootstrap for the missing SCE-owned DB parent path, and target-scoped integration inventory: Claude reports `Plugins`, `Commands`, and `Skills`; OpenCode reports `Plugins`, `Agents`, `Commands`, and `Skills`; Pi reports `Extensions`, `Prompts`, and `Skills`; and Codex reports `Skills` and `Hooks` (see `context/sce/doctor-human-text-contract.md`). Its non-launching post-commit Agent Trace auto-sync fact reports enabled/current, explicit disabled, not-ready, and not-applicable states using canonical managed-block currency and resolved configuration; existing hook remediation and readiness semantics remain unchanged. -The local DB service now provides `LocalDb` as a thin `TursoDb` alias in `cli/src/services/local_db/mod.rs`; `LocalDbSpec` resolves the canonical local DB path from the shared default-path catalog and currently declares zero migrations. Shared Turso infrastructure lives in `cli/src/services/db/mod.rs`, where `DbSpec` and generic `TursoDb` support local or remote sync-mode opens, parent-directory creation, connection setup, synchronous query helpers, embedded migration execution, and shared DB lifecycle helpers. Auth DB persistence uses encrypted `AuthDb = EncryptedTursoDb` and token storage persists credentials through the `auth_credentials` table. Agent Trace persistence uses the sole `RepositoryAgentTraceDb = TursoDb` adapter at `/sce/repos//agent-trace.db`, with a one-file repository schema for `repository_metadata`, `diff_traces`, `post_commit_patch_intersections`, `agent_traces`, `messages`, `parts`, indexes/triggers, and no `checkout_id` columns on trace rows. The checkout-scoped `AgentTraceDb = TursoDb` adapter, its `agent_trace_db_path()`/`agent_trace_db_path_for_checkout()` helpers, and the 15-file `cli/migrations/agent-trace/` chain were removed by the `retire-legacy-agent-trace-db` plan; active hook runtime writes nullable event-local diff-trace attribution without a `session_models` API/table dependency. -The hooks command surface now also supports concrete runtime subcommand routing (`pre-commit`, `commit-msg`, `post-commit`, `post-rewrite`, `diff-trace`, and `conversation-trace`) with deterministic argument/STDIN validation; `session-model` is no longer supported. Current runtime behavior keeps commit-msg attribution enabled by default unless explicitly opted out: the attribution gate enables canonical trailer insertion in `commit-msg` only when the staged-diff AI-overlap preflight confirms AI/editor evidence (no trailer is appended when the preflight finds no overlap or encounters any error); `pre-commit`/`post-rewrite` remain deterministic no-ops, `post-commit` requires validated `--remote-url`, threads that URL into the Agent Trace flow, prints it to stderr, remains the active bounded recent-diff-trace intersection path, and after successful Agent Trace persistence optionally launches the detached sync-owned `sync --format json` child when config-file-only `agent_trace.auto_sync` is true; `diff-trace` is the active intake path for parsed STDIN `{ sessionID, diff, time, model_id?, tool_name, tool_version }` payload persistence with optional `model_id`, required non-empty `tool_name`, required nullable/non-empty `tool_version`, direct-first/event-transcript-second Claude `model_id` plus direct `tool_version` values (no session-model fallback or cache), required `u64` millisecond `time`, same-tool-idempotent stored `session_id` prefixing (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi), non-lossy AgentTraceDb `time_ms` conversion, and no parsed-payload artifact fallback. This behavior is documented in `context/sce/agent-trace-hooks-command-routing.md`. The removed `sce hooks claude-capture` raw capture route is documented in `context/sce/claude-raw-hook-capture.md` as a removed feature. -The setup service now also exposes deterministic required-hook embedded asset accessors (`iter_required_hook_assets`, `get_required_hook_asset`) backed by canonical templates in `cli/assets/hooks/` for `pre-commit`, `commit-msg`, and `post-commit`; this behavior is documented in `context/sce/setup-githooks-hook-asset-packaging.md`. -The setup service now also includes required-hook install orchestration (`install_required_git_hooks`) that resolves repository root and effective hooks path from git truth, computes the bytes to stage by merging the canonical hook template with any existing hook (preserving a foreign hook's content as an exact prefix with the SCE managed block appended, or bringing an SCE-owned block current in place) rather than writing canonical bytes verbatim, enforces deterministic per-hook outcomes (`Installed`/`Updated`/`Skipped`) against that merged content, surfaces a deterministic advisory when an appended block would be unreachable, and uses a unified atomic-swap policy that renames staged content directly over existing hooks without unlinking them first, with deterministic recovery guidance on swap failures; this behavior is documented in `context/sce/setup-githooks-install-flow.md`. -The setup command parser/dispatch now also supports composable setup+hooks runs (`sce setup --opencode|--claude|--pi|--codex|--all --hooks`) plus hooks-only mode (`sce setup --hooks` with optional `--repo `), enforces deterministic compatibility validation (`--repo` requires `--hooks`; target flags remain mutually exclusive), and emits deterministic setup/hook outcome messaging (`installed`/`updated`/`skipped`); this behavior is documented in `context/sce/setup-githooks-cli-ux.md`. + The downstream publish-stage implementation is now complete for both registries: `.github/workflows/publish-crates.yml` publishes the checked-in crate version after `.version`/tag/Cargo parity checks, and `.github/workflows/publish-npm.yml` publishes the checked-in npm package after `.version`/tag/npm parity checks plus verification of the canonical `sce-v-npm.tgz` GitHub release asset. + The repository root now also owns the canonical Biome contract for the current JavaScript tooling slice: `biome.json` scopes formatting/linting to `npm/` and the shared `config/lib/` plugin package root while excluding package-local `node_modules/`, and the root Nix dev shell provides the `biome` binary so contributors do not need a host-installed formatter/linter for those areas. + Flatpak validation/build orchestration is reduced to a minimal app surface: Linux flake apps expose the umbrella `sce-flatpak` (`nix run .#sce-flatpak -- ` for `validate`, `prepare-local-manifest`, etc.) plus `release-flatpak-package`, `release-flatpak-bundle`, and the `regenerate-flatpak-manifest` / `regenerate-cargo-sources` helpers; the previously separate `flatpak-validate`, `flatpak-local-manifest`, and `flatpak-build` wrapper apps are removed. Default `nix flake check` keeps the lightweight Nix-built static/AppStream validator plus the parity checks (`flatpak-manifest-parity`, `cargo-sources-parity`) and does not run a network-heavy Flatpak build. The former standalone install-channel integration runner and `install-channel-integration-tests` flake app are not active current-state surfaces. + Shared Context Plan and Shared Context Code remain separate OpenCode routing roles: the generated Plan agent routes only to `/change-to-plan`, while the generated Code agent routes to `/next-task`, `/validate`, `/commit`, `/handover`, and `/brownfield`. Workflow behavior lives in the six workflow entrypoints and their six skill packages rather than in agent bodies. `config/pkl/base/workflow-catalog.pkl` assigns each workflow to its role, and OpenCode command routing plus each agent's ordered `skill:` permissions derive from those records: ordinary non-SCE skills are allowed by the wildcard, arbitrary `sce-*` skills are denied, and only the role's owned workflows are allowed after that deny — `sce-change-to-plan` for Plan; `sce-next-task`, `sce-validate`, `sce-commit`, `sce-handover`, and `sce-brownfield` for Code. The Code agent additionally allows `sce-decision` for task synchronization. + The canonical workflow definitions remain phase-decomposed as authoring source: `/change-to-plan` sequences `sce-context-load` then `sce-plan-authoring`; `/next-task` sequences `sce-plan-review`, `sce-task-execution`, and `sce-task-context-sync`; `/validate` runs `sce-validation` only and reports its Validation Report; `/commit` delegates staged-diff analysis and message generation to `sce-atomic-commit`; `/handover` has no phases, since writer and loader mode has no SCE sibling handoff or wait mid-run; `/brownfield` likewise has none, since its single skill owns investigation, the blocking clarification gate, writing, and reporting itself. Relevant non-SCE skills may help inside an active workflow step, but they return control to that step without changing its canonical invariants. No target generates those phase modules as packages. All four consume them as inputs to the shared `workflow-composite.pkl` renderer, which composes each workflow into one skill package. Every workflow supplies typed package/composite render values for frontmatter, bodies, semantic references, phases, persisted-document formats where applicable, and output references; the composite renderer performs no prose-wide internalization or frontmatter stripping. + Every target preserves the same gates and lifecycle semantics through six renderer-composed workflow packages: `sce-change-to-plan`, `sce-next-task`, `sce-validate`, `sce-commit`, `sce-handover`, and `sce-brownfield`. Each thin command or Pi prompt invokes exactly one corresponding skill, and OpenCode command frontmatter names that single skill as both `entry-skill` and the whole `skills` chain. Each phase-based package keeps control flow, internal status branching, waits, and same-session resume in `SKILL.md`, while package-local Markdown references own phase instructions and persisted-document formats; `references/output.md` remains the sole definition of human-visible gates and terminal Markdown. Phase-free `/handover` retains `SKILL.md`, `references/handover-template.md`, and `references/output.md`, while `/brownfield` retains `SKILL.md` plus `references/output.md`. No target emits phase-skill packages or inter-skill machine contracts; phase statuses stay internal to one skill invocation. + Context sync uses an important-change gate: cross-cutting/policy/architecture/terminology changes require root shared-file edits, while localized tasks run verify-only root checks without default churn. + OpenCode and Claude no longer generate legacy bootstrap or context-sync skills; `/commit` and `/handover` are generated only as catalog-registered composite workflow packages. OpenCode retains only thin routing agents, while Claude emits no agents. The superseded grouped Markdown catalog and automated OpenCode profile have been removed from Pkl ownership and generated outputs. + The prior no-git-wrapper Agent Trace design artifacts under `context/sce/agent-trace-*.md` are retained only as historical reference; the current CLI runtime no longer wires the removed Agent Trace schema adaptation, payload building, retry replay, or rewrite handling paths into local hook execution. + The hooks service now uses a minimal attribution-only runtime: `commit-msg` is the only hook that mutates behavior, conditionally injecting exactly one canonical SCE trailer when the attribution-hooks gate is enabled, `SCE_DISABLED` is false, and the staged-diff AI-overlap preflight confirms AI/editor evidence (`StagedDiffAiOverlapResult::Overlap`); when the preflight returns `NoOverlap` or `Error` (including DB open failure, schema not ready, query error, staged diff read failure, or zero overlap), the trailer is not appended and errors are logged via `sce.hooks.commit_msg.ai_overlap_error`; `pre-commit` and `post-rewrite` remain deterministic no-op entrypoints; `post-commit` requires validated `--remote-url`, threads that URL through the Agent Trace flow, prints it to stderr, captures current commit patch, queries recent `diff_traces` from past 7 days (dispatching `patch` rows through existing unified-diff parsing and `structured` rows through `structured_patch::derive_claude_structured_patch` at read time, then assigning each structured hunk the persisted row model and every structured touched line the persisted canonical `cc_...` row session), combines/intersects patches, persists intersection metadata to `post_commit_patch_intersections`, and persists the schema-validated built Agent Trace payload, including optional top-level `tool` metadata from recent diff-trace rows, top-level `metadata.sce.version` from the compiled `sce` CLI package version, always-emitted `metadata.sce.line_changes` touched-line attribution counts, and range-level `content_hash` values, to AgentTraceDb `agent_traces` (DB-only, no post-commit Agent Trace file artifact); `diff-trace` currently validates/persists required non-empty `sessionID`/`diff`/`tool_name`, optional `model_id` (absent or `null` → `None`, present+non-empty → `Some`, present+empty → error), required nullable/non-empty `tool_version`, plus required `u64` millisecond `time`, uses direct-first/event-transcript-second Claude `model_id` resolution plus direct `tool_version` without any `session_models` runtime, and continues with `None` when event-local lookup cannot resolve attribution, with same-tool-idempotent stored `session_id` prefixing (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi), non-lossy AgentTraceDb `time_ms` conversion, and no parsed-payload artifact fallback. Claude structured `PostToolUse` diff-trace intake prefers direct top-level or nested model metadata, then matches the event's `tool_use_id` in its JSONL `transcript_path`, normalizes either source once with the `claude/` prefix, and fails open to `None`. + The CLI now also includes an approved operator-environment doctor contract documented in `context/sce/agent-trace-hook-doctor.md`; the runtime includes `sce doctor --fix` parsing/help, stable problem/fix-result reporting, canonical hook-repair reuse, bounded doctor-owned local-DB directory bootstrap for the missing SCE-owned DB parent path, and target-scoped integration inventory: Claude reports `Plugins`, `Commands`, and `Skills`; OpenCode reports `Plugins`, `Agents`, `Commands`, and `Skills`; Pi reports `Extensions`, `Prompts`, and `Skills`; and Codex reports `Skills` and `Hooks` (see `context/sce/doctor-human-text-contract.md`). Its non-launching post-commit Agent Trace auto-sync fact reports enabled/current, explicit disabled, not-ready, and not-applicable states using canonical managed-block currency and resolved configuration; existing hook remediation and readiness semantics remain unchanged. + The local DB service now provides `LocalDb` as a thin `TursoDb` alias in `cli/src/services/local_db/mod.rs`; `LocalDbSpec` resolves the canonical local DB path from the shared default-path catalog and currently declares zero migrations. Shared Turso infrastructure lives in `cli/src/services/db/mod.rs`, where `DbSpec` and generic `TursoDb` support local or remote sync-mode opens, parent-directory creation, connection setup, synchronous query helpers, embedded migration execution, and shared DB lifecycle helpers. Auth DB persistence uses encrypted `AuthDb = EncryptedTursoDb` and token storage persists credentials through the `auth_credentials` table. Agent Trace persistence uses the sole `RepositoryAgentTraceDb = TursoDb` adapter at `/sce/repos//agent-trace.db`, with a one-file repository schema for `repository_metadata`, `diff_traces`, `post_commit_patch_intersections`, `agent_traces`, `messages`, `parts`, indexes/triggers, and no `checkout_id` columns on trace rows. The checkout-scoped `AgentTraceDb = TursoDb` adapter, its `agent_trace_db_path()`/`agent_trace_db_path_for_checkout()` helpers, and the 15-file `cli/migrations/agent-trace/` chain were removed by the `retire-legacy-agent-trace-db` plan; active hook runtime writes nullable event-local diff-trace attribution without a `session_models` API/table dependency. + The hooks command surface now also supports concrete runtime subcommand routing (`pre-commit`, `commit-msg`, `post-commit`, `post-rewrite`, `diff-trace`, and `conversation-trace`) with deterministic argument/STDIN validation; `session-model` is no longer supported. Current runtime behavior keeps commit-msg attribution enabled by default unless explicitly opted out: the attribution gate enables canonical trailer insertion in `commit-msg` only when the staged-diff AI-overlap preflight confirms AI/editor evidence (no trailer is appended when the preflight finds no overlap or encounters any error); `pre-commit`/`post-rewrite` remain deterministic no-ops, `post-commit` requires validated `--remote-url`, threads that URL into the Agent Trace flow, prints it to stderr, remains the active bounded recent-diff-trace intersection path, and after successful Agent Trace persistence optionally launches the detached sync-owned `sync --format json` child when config-file-only `agent_trace.auto_sync` is true; `diff-trace` is the active intake path for parsed STDIN `{ sessionID, diff, time, model_id?, tool_name, tool_version }` payload persistence with optional `model_id`, required non-empty `tool_name`, required nullable/non-empty `tool_version`, direct-first/event-transcript-second Claude `model_id` plus direct `tool_version` values (no session-model fallback or cache), required `u64` millisecond `time`, same-tool-idempotent stored `session_id` prefixing (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi), non-lossy AgentTraceDb `time_ms` conversion, and no parsed-payload artifact fallback. This behavior is documented in `context/sce/agent-trace-hooks-command-routing.md`. The removed `sce hooks claude-capture` raw capture route is documented in `context/sce/claude-raw-hook-capture.md` as a removed feature. + The setup service now also exposes deterministic required-hook embedded asset accessors (`iter_required_hook_assets`, `get_required_hook_asset`) backed by canonical templates in `cli/assets/hooks/` for `pre-commit`, `commit-msg`, and `post-commit`; this behavior is documented in `context/sce/setup-githooks-hook-asset-packaging.md`. + The setup service now also includes required-hook install orchestration (`install_required_git_hooks`) that resolves repository root and effective hooks path from git truth, computes the bytes to stage by merging the canonical hook template with any existing hook (preserving a foreign hook's content as an exact prefix with the SCE managed block appended, or bringing an SCE-owned block current in place) rather than writing canonical bytes verbatim, enforces deterministic per-hook outcomes (`Installed`/`Updated`/`Skipped`) against that merged content, surfaces a deterministic advisory when an appended block would be unreachable, and uses a unified atomic-swap policy that renames staged content directly over existing hooks without unlinking them first, with deterministic recovery guidance on swap failures; this behavior is documented in `context/sce/setup-githooks-install-flow.md`. + The setup command parser/dispatch now also supports composable setup+hooks runs (`sce setup --opencode|--claude|--pi|--codex|--all --hooks`) plus hooks-only mode (`sce setup --hooks` with optional `--repo `), enforces deterministic compatibility validation (`--repo` requires `--hooks`; target flags remain mutually exclusive), and emits deterministic setup/hook outcome messaging (`installed`/`updated`/`skipped`); this behavior is documented in `context/sce/setup-githooks-cli-ux.md`. ## Repository model diff --git a/context/plans/setup-git-remote-preflight.md b/context/plans/setup-git-remote-preflight.md new file mode 100644 index 00000000..fefa24af --- /dev/null +++ b/context/plans/setup-git-remote-preflight.md @@ -0,0 +1,165 @@ +# Plan: setup-git-remote-preflight + +## Change summary + +Extend `sce setup`'s existing repository preflight so every setup mode stops +before prompts or writes unless the target is both an initialized Git +repository and has a configured Git remote URL. The remote check will use the +same resolved `agent_trace.repository_remote` name that Agent Trace identity +resolution uses, defaulting to `origin`, rather than hard-coding `origin` or +accepting an unrelated remote. + +Add `UserError::NotGitRepository` and `UserError::NotGitRemote` to the typed +CLI error catalog. Preserve technical sources for observability while giving +operators stable, actionable messages explaining `git init` or +`git remote add ` remediation. Reuse the existing Git root +resolution and `lookup_remote_url` implementations; this change does not test +remote network reachability or redesign repository identity resolution. + +## Acceptance criteria + +How this plan is proven complete. Each criterion is observable and names the +check that proves it. `/validate` runs these checks; no task in the stack +performs final validation. + +- [x] AC1: `sce setup` reports a typed, actionable `NotGitRepository` failure + when the target path is not inside an initialized Git repository, and does + not prompt or perform setup writes. + - Validate: Setup preflight tests exercise a non-repository directory and + assert the `UserError::NotGitRepository` variant and `git init` guidance. +- [x] AC2: `sce setup` reports a typed, actionable `NotGitRemote` failure when + the resolved SCE remote has no configured URL, and does not prompt or perform + setup writes. + - Validate: Setup preflight tests exercise an initialized repository without + the selected remote URL and assert the `UserError::NotGitRemote` variant + and `git remote add ` guidance. +- [x] AC3: Remote validation uses the resolved `agent_trace.repository_remote` + name, including the default `origin`, and setup proceeds past the preflight + when that named remote has a URL regardless of whether another remote is + present. + - Validate: Tests cover both the default `origin` and a configured alternate + remote such as `upstream`, including rejection when only an unrelated + remote exists. +- [x] AC4: Existing successful setup behavior and user-error rendering remain + compatible, while technical error sources remain available to observability + and remote URLs are not echoed in diagnostics. + - Validate: Existing CLI error tests plus the new setup error tests pass, and + the full repository check suite succeeds. + +### Full validation + +Repository-wide checks `/validate` runs after the last task, regardless of +which criterion they map to. + +- `nix flake check` + +### Context sync + +- `context/overview.md` — update the current setup and typed user-error + summaries. +- `context/cli/cli-command-surface.md` — document the Git-plus-remote preflight + and its ownership in `services/setup/command.rs`. +- `context/sce/setup-githooks-cli-ux.md` — document the remote prerequisite for + all setup modes and actionable failure behavior. +- `context/sce/setup-repo-local-config-bootstrap.md` — record that both + repository preflights precede context/config/database/bootstrap side effects. +- `context/sce/cli-error-code-taxonomy.md` — add the two setup-specific + `UserError` catalog entries and preserve-source rendering contract. + +## 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/error.rs`, `cli/src/services/setup/mod.rs`, + `cli/src/services/setup/command.rs`, focused Rust tests, and the listed + durable setup/error context files. +- **Out of scope:** remote network connectivity checks, remote URL + canonicalization changes, changes to Agent Trace identity precedence, + changes to `doctor`, and changes to setup flags or successful output. +- **Constraints:** Reuse `setup::ensure_git_repository`, the existing config + resolver for `agent_trace.repository_remote`, and + `repository_identity::resolve::lookup_remote_url`; preserve stdout/stderr + and stable `SCE-ERR-RUNTIME` behavior for `UserError` failures; do not expose + raw credential-bearing remote URLs. +- **Non-goal:** Accepting any arbitrary Git remote when the configured SCE + remote is missing; setup must validate the remote Agent Trace will use. + +## Assumptions + +- The remote preflight applies to `sce setup --bootstrap-context` as well as + normal, hooks-only, combined, and interactive setup because the requirement + is that `sce setup` validates both prerequisites before any setup path. +- An explicit `agent_trace.repository_id` does not waive the remote preflight; + the requested setup contract requires a Git remote independently of identity + fallback behavior. +- `UserError` messages remain fixed catalog sentences; the configured remote + name is retained in the technical source for diagnostics/tests rather than + being added as a payload field to the enum. + +## Task stack + +- [x] T01: `Add typed setup preflight errors and remote validation primitive` (status:done) + - Task ID: T01 + - Scope: In — add `NotGitRepository` and `NotGitRemote` to the `UserError` catalog with keys, runtime classification, and actionable fixed messages; add a setup-owned remote preflight helper that delegates to `lookup_remote_url`; add focused tests for catalog behavior, missing remotes, alternate remote names, and credential-safe source text. Out — command wiring and durable context edits. + - Dependencies: none + - Done when: The setup service exposes a reusable remote-URL preflight, both typed errors render their reviewed remediation messages, missing/configured remote cases are covered, and no raw remote URL is included in the generated error source. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml error setup repository_identity` + - Completed: 2026-08-24 + - Files changed: `cli/src/services/error.rs`, `cli/src/services/setup/mod.rs` + - Result: Added runtime-classified `setup.not_git_repository` and `setup.not_git_remote` catalog entries with fixed `git init` and `git remote add ` remediation messages. Added `setup::ensure_git_remote`, which delegates named-remote URL lookup to `repository_identity::resolve::lookup_remote_url` and retains only the configured remote name in its technical error. Added catalog, source-preservation, missing-origin, configured-origin, alternate-remote, and unrelated-remote tests. + - Verify: The combined filter form was rejected because Cargo accepts one test filter at a time; equivalent separate runs passed: `error` (35 passed), `setup` (63 passed), and `repository_identity` (24 passed). + - Context impact: Material CLI behavior change in the typed error catalog and setup service; durable context documentation is deferred to T02's context synchronization scope. + - Context synchronization: synced + +- [x] T02: `Enforce Git and configured-remote preflights at setup dispatch` (status:done) + - Task ID: T02 + - Scope: In — resolve the effective `agent_trace.repository_remote` in `setup/command.rs`, run the Git-root and named-remote preflights before prompts/context bootstrap/lifecycle setup, map failures through `CliError::user_with_source`, add command-level regression coverage for no-Git/no-remote/default/alternate-remote paths, and update the listed durable context files including the existing command-ownership drift. Out — network reachability, remote canonicalization, doctor behavior, and unrelated setup output changes. + - Dependencies: T01 + - Done when: Every setup mode fails early and actionably for either missing prerequisite, valid default and configured alternate remotes pass the gate, existing setup success/cancellation behavior remains intact, and durable context describes the implemented gate and error catalog accurately. + - Verify: `nix flake check` + - Completed: 2026-08-24 + - Files changed: `cli/src/services/error.rs`, `cli/src/services/setup/command.rs`, `context/overview.md`, `context/cli/cli-command-surface.md`, `context/sce/setup-githooks-cli-ux.md`, `context/sce/setup-repo-local-config-bootstrap.md`, `context/sce/cli-error-code-taxonomy.md`, `context/plans/setup-git-remote-preflight.md` + - Result: Wired setup dispatch through a pre-prompt Git-root and configured-remote gate. The effective `agent_trace.repository_remote` is resolved with the existing config resolver and defaults to `origin`; missing prerequisites map through `CliError::user_with_source` to typed `NotGitRepository` or `NotGitRemote` diagnostics while preserving technical sources without rendering remote URLs. Added command-level coverage for missing Git, missing origin, configured origin, configured alternate remotes, and unrelated remotes, and documented the ordering and ownership across durable setup/error context. + - Verify: Focused setup preflight tests passed (6 tests). `nix flake check` passed all repository checks; an earlier full-check failure was an unrelated flaky Agent Trace row-count test and passed on rerun. + - Context impact: Root — setup is now governed by a repository-wide Git-plus-configured-remote preflight and the typed CLI error catalog; updated root setup/error summaries and the authoritative setup/bootstrap/error domain contracts. + - Context synchronization: synced + +## Open questions + +None. The remote selection rule, mandatory scope, error classification, and +non-network validation boundary were resolved during discussion. + +## Validation Report + +**Status:** validated +**Date:** 2026-08-24 + +### Commands run + +- `nix flake check` -> exit 0 (all repository checks passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml error` -> exit 0 (35 tests passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml setup` -> exit 0 (67 tests passed) + +### Success-criteria verification + +- [x] AC1: `sce setup` reports a typed, actionable `NotGitRepository` failure and performs no setup writes -> command-level preflight test passed for a non-repository directory. +- [x] AC2: `sce setup` reports a typed, actionable `NotGitRemote` failure and performs no setup writes -> command-level missing-origin test passed; error tests confirmed remote-add guidance. +- [x] AC3: Remote validation uses the resolved `agent_trace.repository_remote` name -> setup tests passed for default `origin`, configured alternate remotes, and rejection of unrelated remotes. +- [x] AC4: Existing setup behavior and user-error rendering remain compatible without exposing remote URLs -> error and setup test filters plus the full repository check passed; source-preservation tests passed without rendered URL leakage. + +### Failed checks and follow-ups + +- None. + +### Residual risks + +- None identified. diff --git a/context/sce/cli-error-code-taxonomy.md b/context/sce/cli-error-code-taxonomy.md index b43c0da4..b9691fc6 100644 --- a/context/sce/cli-error-code-taxonomy.md +++ b/context/sce/cli-error-code-taxonomy.md @@ -25,13 +25,14 @@ It complements the numeric process exit-code classes documented in `context/sce/ - High-frequency parse/invocation failures use explicit `Try:` remediations instead of generic usage-only hints. - Top-level unknown command/option messages include targeted retry guidance (`sce --help` and command-local `sce --help`). - Setup invocation validation failures (`--repo` without `--hooks`, mutually exclusive target flags, unexpected args) include concrete valid alternatives. +- Setup repository preflight failures use fixed `UserError::NotGitRepository` and `UserError::NotGitRemote` sentences with `git init` and `git remote add ` remediation; their runtime technical sources may retain the configured remote name but never the remote URL. - Hooks invocation validation failures (missing hook subcommand, missing `commit-msg` message file, unknown subcommand) include command-form examples that are copyable for retry automation. -- This actionable-message normalization is owned by parser/validation paths in `cli/src/app.rs`, `cli/src/services/setup/mod.rs`, and `cli/src/services/hooks/mod.rs`. +- This actionable-message normalization is owned by parser/validation paths in `cli/src/app.rs`, `cli/src/services/setup/mod.rs`, `cli/src/services/setup/command.rs`, and `cli/src/services/hooks/mod.rs`. ## 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. +- `CliError::{User,Internal}` in `cli/src/services/error.rs` is the typed CLI-boundary error type; `CliError::code()`/`CliError::class()` delegate to the failure class. `CliError::User` carries a catalog `UserError` (`NotAuthenticated`, `NotGitRepository`, or `NotGitRemote`) 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()`. - Command and domain layers construct and return a `CliError`; they do not format terminal text, apply styling, or decide authentication/user-error semantics from string matching. `app_support` is the sole owner of turning a `CliError` into the final stderr sentence. - `Logger::log_cli_error` in `cli/src/services/observability.rs` owns structured error logging with `sce.error.{code}` event IDs. diff --git a/context/sce/setup-githooks-cli-ux.md b/context/sce/setup-githooks-cli-ux.md index 4350f1ff..3f890bf0 100644 --- a/context/sce/setup-githooks-cli-ux.md +++ b/context/sce/setup-githooks-cli-ux.md @@ -26,7 +26,7 @@ Validation is deterministic and enforced during setup option resolution: - `--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` -- 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 +- all `sce setup` modes (config-only, hooks-only, combined, interactive, and `--bootstrap-context`) require an initialized Git repository and a URL for the effective `agent_trace.repository_remote` before prompts or writes; `setup::command` uses the configured name, defaulting to `origin`, and reports typed `NotGitRepository` or `NotGitRemote` failures without echoing remote URLs Target-install mode contract: @@ -67,4 +67,5 @@ When config install and hook install run together, CLI output is deterministic: - `cli/src/app.rs` - `cli/src/services/setup/mod.rs` -- `cli/src/command_surface.rs` \ No newline at end of file +- `cli/src/services/setup/command.rs` +- `cli/src/command_surface.rs` diff --git a/context/sce/setup-repo-local-config-bootstrap.md b/context/sce/setup-repo-local-config-bootstrap.md index 177c0ac1..22bf57bf 100644 --- a/context/sce/setup-repo-local-config-bootstrap.md +++ b/context/sce/setup-repo-local-config-bootstrap.md @@ -11,13 +11,13 @@ Task `setup-repo-gate-and-local-config-bootstrap` T02, `turso-local-db-sync` T04 - If `.sce/config.json` already exists, the bootstrap step returns `Ok(())` immediately and leaves the file untouched — no merge, no reformat, no overwrite. - The parent `.sce/` directory is created via `fs::create_dir_all` if missing. - The setup flow also bootstraps the canonical local DB through `LocalDbLifecycle::setup` and the Agent Trace DB through `AgentTraceDbLifecycle::setup`; both use the shared `TursoDb` adapter. -- Config/DB bootstrap runs after the git-repo gate (`ensure_git_repository`) and after context baseline bootstrap, and before config/hooks dispatch, so it applies to all normal setup modes: config-only, hooks-only, combined, and interactive. +- Config/DB bootstrap runs after both repository preflights (`ensure_git_repository` and the effective named-remote URL check) and after context baseline bootstrap, and before config/hooks dispatch, so it applies to all normal setup modes: config-only, hooks-only, combined, and interactive. ## Context baseline bootstrap - `sce setup --bootstrap-context` is a non-interactive context-only mode and must be used alone (no target, hooks, non-interactive, or `--repo` flags). -- Context-only setup ensures the Git-repository gate, then creates the baseline durable-context tree and exits without lifecycle providers, integration installs, or prompts. -- Every normal successful setup path also calls the same additive context bootstrap after the Git gate and before lifecycle/config install work. +- Context-only setup ensures both repository preflights, then creates the baseline durable-context tree and exits without lifecycle providers, integration installs, or prompts. +- Every normal successful setup path also calls the same additive context bootstrap after both preflights and before lifecycle/config install work. - Baseline paths: `context/overview.md`, `context/architecture.md`, `context/patterns.md`, `context/glossary.md`, `context/context-map.md`, `context/plans/`, `context/handovers/`, `context/decisions/`, `context/tmp/`, and `context/tmp/.gitignore`. - Create-if-missing only: existing files and directory contents are left untouched; missing individual paths are restored even when `context/` already exists. - New Markdown files use neutral headings/placeholders; `context-map.md` links baseline entry points without inventing repository details; `context/tmp/.gitignore` ignores scratch content while retaining itself (`*\n!.gitignore\n`). @@ -53,11 +53,11 @@ The same write also records the run's resolved optional-workflow selection under - `cli/src/services/agent_trace_db/lifecycle.rs` implements `AgentTraceDbLifecycle::setup()` for Agent Trace DB initialization. - Repo-local config bootstrap uses `RepoPaths::sce_config_file()` and `RepoPaths::sce_dir()`; context baseline bootstrap uses the shared context accessors including `RepoPaths::context_tmp_gitignore_file()`. - The canonical payload constant is `REPO_LOCAL_CONFIG_BOOTSTRAP_PAYLOAD`. -- `cli/src/services/setup/command.rs` runs `bootstrap_context_baseline` immediately after `ensure_git_repository`. Context-only requests return there. Normal modes then derive a repo-root-scoped `AppContext` and aggregate lifecycle providers in config → local_db → auth_db → agent_trace_db → hooks order; `ConfigLifecycle::setup()` calls `bootstrap_repo_local_config(...)`, `LocalDbLifecycle::setup()` initializes the local DB, `AuthDbLifecycle::setup()` initializes the auth DB, and `AgentTraceDbLifecycle::setup()` initializes the Agent Trace DB. +- `cli/src/services/setup/command.rs` resolves the effective `agent_trace.repository_remote` and runs both repository preflights before `bootstrap_context_baseline`. Context-only requests return after the baseline. Normal modes then derive a repo-root-scoped `AppContext` and aggregate lifecycle providers in config → local_db → auth_db → agent_trace_db → hooks order; `ConfigLifecycle::setup()` calls `bootstrap_repo_local_config(...)`, `LocalDbLifecycle::setup()` initializes the local DB, `AuthDbLifecycle::setup()` initializes the auth DB, and `AgentTraceDbLifecycle::setup()` initializes the Agent Trace DB. ## Relationship to other setup contracts -- The git-repo gate (`ensure_git_repository`) remains the precondition for every setup write path, including context-only bootstrap. +- The Git-repo gate (`ensure_git_repository`) and effective named-remote URL preflight remain the preconditions for every setup write path, including context-only bootstrap. - Context baseline bootstrap is independent of config/DB/hooks install and runs before those steps on normal setup paths. - Local bootstrap (repo config + local DB init) is independent of config install and hook install; it runs before both after context baseline bootstrap. - The bootstrap payload matches the `$schema` declaration accepted by startup config loading and the Pkl-authored JSON Schema embedded from Cargo `OUT_DIR`. From cedb307f5fa85b76cbf4be3c1ff33dcb12e55388 Mon Sep 17 00:00:00 2001 From: Ivan Ivic Date: Tue, 25 Aug 2026 16:24:53 +0200 Subject: [PATCH 2/5] setup: Narrow preflight error classification Preserve runtime diagnostics for Git and remote lookup failures while retaining actionable typed errors for missing prerequisites. Add strict remote URL lookup and classify only explicit missing-repository and missing-remote cases as user errors. Preserve technical sources and credential-safe diagnostics for other failures. Co-authored-by: SCE --- .../services/repository_identity/resolve.rs | 48 +++++++++-- cli/src/services/setup/command.rs | 20 ++++- cli/src/services/setup/mod.rs | 83 +++++++++++++------ context/cli/repository-identity.md | 4 +- context/glossary.md | 2 +- context/overview.md | 2 +- context/plans/setup-git-remote-preflight.md | 56 ++++++++++--- context/sce/cli-error-code-taxonomy.md | 4 +- context/sce/setup-githooks-cli-ux.md | 2 +- .../sce/setup-repo-local-config-bootstrap.md | 2 +- 10 files changed, 167 insertions(+), 56 deletions(-) diff --git a/cli/src/services/repository_identity/resolve.rs b/cli/src/services/repository_identity/resolve.rs index e2bb0d0c..5d611741 100644 --- a/cli/src/services/repository_identity/resolve.rs +++ b/cli/src/services/repository_identity/resolve.rs @@ -13,6 +13,8 @@ use std::path::Path; use std::process::Command; +use anyhow::{bail, Context, Result}; + use super::{ repository_identity_from_explicit, repository_identity_from_remote_url, RepositoryIdentity, RepositoryIdentityError, @@ -124,21 +126,55 @@ pub fn resolve_repository_identity_with_lookup( /// Returns `None` when git is unavailable, the directory is not a /// repository, or the remote has no URL. pub fn lookup_remote_url(repository_root: &Path, remote_name: &str) -> Option { + lookup_remote_url_strict(repository_root, remote_name) + .ok() + .flatten() +} + +/// Reads a named Git remote URL while distinguishing a missing URL from a +/// failure to execute or interpret the lookup. +pub fn lookup_remote_url_strict( + repository_root: &Path, + remote_name: &str, +) -> Result> { let output = Command::new("git") .arg("-C") .arg(repository_root) .args(["config", "--get", &format!("remote.{remote_name}.url")]) .output() - .ok()?; + .with_context(|| { + format!( + "Failed to look up Git remote '{remote_name}' URL in '{}'", + repository_root.display() + ) + })?; + if !output.status.success() { - return None; + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + if output.status.code() == Some(1) && stderr.is_empty() { + return Ok(None); + } + + let diagnostic = if stderr.is_empty() { + String::from("git config exited with a non-zero status") + } else { + crate::services::security::redact_sensitive_text(&stderr) + }; + bail!( + "Failed to look up Git remote '{remote_name}' URL in '{}': {diagnostic}", + repository_root.display() + ); } - let url = String::from_utf8_lossy(&output.stdout).trim().to_string(); + + let url = String::from_utf8(output.stdout) + .context("Git remote lookup output contained invalid UTF-8")? + .trim() + .to_string(); if url.is_empty() { - None - } else { - Some(url) + return Ok(None); } + + Ok(Some(url)) } #[cfg(test)] diff --git a/cli/src/services/setup/command.rs b/cli/src/services/setup/command.rs index b0d08b63..dfa02ce0 100644 --- a/cli/src/services/setup/command.rs +++ b/cli/src/services/setup/command.rs @@ -102,13 +102,25 @@ impl SetupCommand { } fn resolve_setup_repository(start_path: &std::path::Path) -> Result { - let repository_root = setup::ensure_git_repository(start_path) - .map_err(|source| CliError::user_with_source(UserError::NotGitRepository, source))?; + let repository_root = setup::ensure_git_repository(start_path).map_err(|source| { + if setup::is_not_git_repository_error(&source) { + CliError::user_with_source(UserError::NotGitRepository, source) + } else { + CliError::runtime(source) + } + })?; let storage_config = config::resolve_agent_trace_storage_runtime_config(&repository_root) .map_err(CliError::runtime)?; - setup::ensure_git_remote(&repository_root, &storage_config.repository_remote) - .map_err(|source| CliError::user_with_source(UserError::NotGitRemote, source))?; + setup::ensure_git_remote(&repository_root, &storage_config.repository_remote).map_err( + |source| { + if setup::is_missing_git_remote_error(&source) { + CliError::user_with_source(UserError::NotGitRemote, source) + } else { + CliError::runtime(source) + } + }, + )?; Ok(repository_root) } diff --git a/cli/src/services/setup/mod.rs b/cli/src/services/setup/mod.rs index 4705ad9e..b7f0dbe2 100644 --- a/cli/src/services/setup/mod.rs +++ b/cli/src/services/setup/mod.rs @@ -12,6 +12,49 @@ pub mod command; pub(crate) mod config_merge; pub(crate) mod hook_merge; +#[derive(Debug)] +struct NotGitRepositoryError { + directory: PathBuf, +} + +impl std::fmt::Display for NotGitRepositoryError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "Directory '{}' is not a git repository. Try: run 'git init' in '{}', then rerun 'sce setup'.", + self.directory.display(), + self.directory.display() + ) + } +} + +impl std::error::Error for NotGitRepositoryError {} + +#[derive(Debug)] +struct MissingGitRemoteError { + remote_name: String, +} + +impl std::fmt::Display for MissingGitRemoteError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "Git remote '{}' has no configured URL. Try: run 'git remote add {} ', then rerun 'sce setup'.", + self.remote_name, self.remote_name + ) + } +} + +impl std::error::Error for MissingGitRemoteError {} + +pub(crate) fn is_not_git_repository_error(error: &anyhow::Error) -> bool { + error.downcast_ref::().is_some() +} + +pub(crate) fn is_missing_git_remote_error(error: &anyhow::Error) -> bool { + error.downcast_ref::().is_some() +} + /// Canonical JSON payload for a newly bootstrapped repo-local `.sce/config.json`. /// Contains only the `$schema` declaration pointing to the SCE config JSON Schema. fn repo_local_config_bootstrap_payload() -> String { @@ -428,18 +471,18 @@ pub fn ensure_git_repository(directory: &Path) -> Result { /// The URL itself is intentionally discarded so callers can preserve a /// technical diagnostic without echoing credential-bearing remote values. pub fn ensure_git_remote(repository_root: &Path, remote_name: &str) -> Result<()> { - if crate::services::repository_identity::resolve::lookup_remote_url( + let remote_url = crate::services::repository_identity::resolve::lookup_remote_url_strict( repository_root, remote_name, - ) - .is_some() - { + )?; + + if remote_url.is_some() { return Ok(()); } - bail!( - "Git remote '{remote_name}' has no configured URL. Try: run 'git remote add {remote_name} ', then rerun 'sce setup'." - ) + Err(anyhow::Error::new(MissingGitRemoteError { + remote_name: remote_name.to_string(), + })) } /// Bootstraps the repo-local `.sce/config.json` file if it does not already exist. @@ -1147,29 +1190,12 @@ mod install { } fn resolve_git_repository_root(repository_root: &Path) -> Result { - let repository_root_output = run_git_command_in_directory( + run_git_command_in_directory( repository_root, &["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))?; - Ok(PathBuf::from(repository_root_output)) - } - - fn map_setup_non_git_repository_error( - 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() - ) - } else { - error - } + .map(PathBuf::from) } fn resolve_git_hooks_directory(repository_root: &Path) -> Result { @@ -1206,6 +1232,11 @@ mod install { if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + if args == ["rev-parse", "--show-toplevel"] && stderr.contains("not a git repository") { + return Err(anyhow::Error::new(super::NotGitRepositoryError { + directory: repository_root.to_path_buf(), + })); + } let diagnostic = if stderr.is_empty() { String::from("git command exited with a non-zero status") } else { diff --git a/context/cli/repository-identity.md b/context/cli/repository-identity.md index 7e58907f..f948a728 100644 --- a/context/cli/repository-identity.md +++ b/context/cli/repository-identity.md @@ -38,7 +38,8 @@ Example: `git@GitHub.com:Acme/Widgets.git`, `ssh://git@github.com:22/Acme/Widget - `resolve_repository_identity(repository_root, explicit_identity, remote_name)` — process-spawning entrypoint. - `resolve_repository_identity_with_lookup(explicit, remote_name, lookup)` — precedence core with injectable remote lookup for tests/callers. -- `lookup_remote_url(repository_root, remote_name) -> Option` — returns `None` when git is unavailable, the directory is not a repository, or the remote has no URL. +- `lookup_remote_url_strict(repository_root, remote_name) -> Result>` — distinguishes a missing/empty remote URL (`Ok(None)`) from Git process, configuration, or output failures (`Err`); setup uses this strict seam. +- `lookup_remote_url(repository_root, remote_name) -> Option` — compatibility lookup used by repository-identity resolution; it preserves the existing fail-to-missing behavior by collapsing strict lookup failures to `None`. - `ResolvedRepositoryIdentity { identity, source }` with `RepositoryIdentitySource::{ExplicitConfig, RemoteUrl { remote_name }}` — source is retained for later diagnostics rendering (T10). - `RepositoryIdentityResolutionError::{InvalidExplicitIdentity, InvalidRemoteUrl, MissingIdentity}` — every `Display` message includes `.sce/config.json` guidance naming the `agent_trace.*` keys; variants carry only the configured remote name, never URLs or identity values. @@ -49,6 +50,7 @@ Local paths are never used implicitly: a local-path remote URL fails canonicaliz - The returned canonical identity and repository ID never contain userinfo/credentials. - `RepositoryIdentityError` variants (`EmptyExplicitIdentity`, `EmptyRemoteUrl`, `UnsupportedRemoteUrl`, `MissingHost`, `MissingPath`, `InvalidPort`) are fieldless and their `Display` messages never echo the raw input, so credential-bearing URLs cannot leak through diagnostics. - `RepositoryIdentityResolutionError` follows the same rule: it never echoes remote URLs or explicit identity values; only operator-chosen remote names appear in messages. +- Setup does not reuse the compatibility collapse for its preflight: only an explicit missing URL becomes `NotGitRemote`, while remote lookup execution failures remain `CliError::Internal` runtime errors with their technical sources. ## Status diff --git a/context/glossary.md b/context/glossary.md index 8af1c5cf..71b0eae0 100644 --- a/context/glossary.md +++ b/context/glossary.md @@ -37,7 +37,7 @@ - `split commit references`: The generated `sce-commit/references/atomic-commit.md`, `references/commit-message-style.md`, and `references/output.md` documents. The atomic reference owns staged-diff procedure, internal result branching, and commit boundaries; the style reference owns commit-message wording; and the output reference owns human-visible layouts. No `commit-contract.yaml` artifact or YAML result-contract section is generated. - `canonical OpenCode plugin registration source`: Shared Pkl-authored plugin-registration definition in `config/pkl/base/opencode.pkl`, re-exported from `config/pkl/renderers/common.pkl` as the canonical plugin list/path JSON consumed by OpenCode renderers before they emit generated `opencode.json` manifests; the current entries are `sce-bash-policy` and `sce-agent-trace`. - `checkout identity`: Stable UUIDv7 identifier assigned to a cloned repository or linked Git worktree, stored in `/sce/checkout-id` (never committed) and resolved via `git rev-parse --git-dir`. The identity is created or reused by `sce setup` through `AgentTraceDbLifecycle::setup()` and also auto-created by hook runtime when `sce setup` has not been run. Checkout identity is now diagnostic metadata for repository-scoped Agent Trace storage; it does not select the active DB and is not stored on Agent Trace rows. Any pre-migration per-checkout DB files at `/sce/agent-trace-{checkout_id}.db` are never touched by SCE and are no longer inspectable via the CLI (the checkout-scoped DB surface was removed by the `retire-legacy-agent-trace-db` plan). See `context/cli/checkout-identity.md`. -- `repository identity`: Stable identity of a logical Git repository used to select the active repository-scoped Agent Trace DB path `/sce/repos//agent-trace.db` through the `agent_trace_storage` resolver. Resolved by `cli/src/services/repository_identity/` with precedence: explicit `agent_trace.repository_id` config value, then the URL of the configured Git remote (`agent_trace.repository_remote`, default `origin`), otherwise an actionable `.sce/config.json` error. Remote URLs canonicalize to a scheme-neutral, credential-free `host[:port]/path` form so equivalent SSH/SCP/HTTPS remotes converge, and the repository ID is `sha256("sce-repository-id-v1\0" + canonical_identity)` hex. Distinct from `checkout identity`, which stays per clone/worktree for diagnostics. Setup's named-remote preflight delegates to the same lookup and discards the URL, retaining only the remote name in technical failure sources. See `context/cli/repository-identity.md`. +- `repository identity`: Stable identity of a logical Git repository used to select the active repository-scoped Agent Trace DB path `/sce/repos//agent-trace.db` through the `agent_trace_storage` resolver. Resolved by `cli/src/services/repository_identity/` with precedence: explicit `agent_trace.repository_id` config value, then the URL of the configured Git remote (`agent_trace.repository_remote`, default `origin`), otherwise an actionable `.sce/config.json` error. Remote URLs canonicalize to a scheme-neutral, credential-free `host[:port]/path` form so equivalent SSH/SCP/HTTPS remotes converge, and the repository ID is `sha256("sce-repository-id-v1\0" + canonical_identity)` hex. Distinct from `checkout identity`, which stays per clone/worktree for diagnostics. Setup's named-remote preflight uses a strict lookup that returns `Ok(None)` only for a missing/empty URL, preserves other failures as runtime errors, and discards the URL, retaining only the remote name in technical failure sources. See `context/cli/repository-identity.md`. - `refresh single-flight guard`: Client-owned async coordination for Agent Trace control-plane authentication. Concurrent callers whose stored access token is expired, or whose request rejected the same token, serialize only refresh-and-save work, re-check credentials after acquiring the guard, and reuse the token saved by the first refresher; valid-token requests do not acquire the guard. - `repository-scoped Agent Trace DB`: Active Agent Trace storage shape where one logical Git repository maps to `/sce/repos//agent-trace.db`. The current seam is `RepositoryAgentTraceDb = TursoDb` in `cli/src/services/agent_trace_db/repository.rs`, backed by the fresh multi-statement `001_repository_schema` baseline plus the additive `002_repository_source_instance_id` migration, with `repository_metadata` (`repository_id` plus `source_instance_id`) plus repository-level trace tables, no `checkout_id` columns, and typed repository-level insert helpers for diff traces, post-commit intersections, Agent Trace rows, messages, and parts. Hook runtime, Agent Trace setup/lifecycle, and `sce sync` resolve repository-scoped storage through `agent_trace_storage`. This is the sole Agent Trace DB adapter; the checkout-scoped adapter and former trace inspection surface were removed by the `retire-legacy-agent-trace-db` plan. - `source_instance_id`: Physical-database identity column on `repository_metadata`, independent of the logical `repository_id`. Added by the additive `002_repository_source_instance_id` migration (existing/placeholder rows default to an empty string); generated once per physical `agent-trace.db` by application code (`generate_source_instance_id()`, UUID v4 today) and validated with `is_valid_source_instance_id()` (non-empty once trimmed) — never generated in SQL and never derived from `repository_id`, remote URL, checkout ID, filesystem path, hostname, or user/workspace identity. `RepositoryAgentTraceDb::verify_or_initialize_repository_metadata` claims it with a concurrency-safe `UPDATE ... WHERE source_instance_id = ''`, so concurrent first opens of the same physical database converge on one winner and an already-valid value is never overwritten; the value stays stable across reopen and repeated `sce setup` runs. Two independently created databases for the same logical repository (for example two clones) get different `source_instance_id` values. See `context/sce/agent-trace-db.md`. diff --git a/context/overview.md b/context/overview.md index 951d0d27..b7a53cd2 100644 --- a/context/overview.md +++ b/context/overview.md @@ -37,7 +37,7 @@ The shared default path service in`cli/src/services/default_paths.rs`is now the The Rust CLI also centralizes SCE-owned web URI construction in`cli/src/services/agent_trace.rs`, with `SCE_WEB_BASE_URL`as the single Rust owner for`https://sce.crocoder.dev` and helpers consumed by Agent Trace conversation URLs, Agent Trace persisted trace URLs, Agent Trace session URLs, and setup-created repo-local config schema URLs. The config resolver separately owns `control_plane_base_url` and its `https://sce.crocoderlab.dev` baked sync default; the two URL owners must not be conflated. The current user-facing synchronization entrypoint is `sce sync`; references to the former nested spelling in historical records do not describe an available command. -Setup repository preflight: every `sce setup` mode, including `--bootstrap-context`, validates an initialized Git repository and the configured `agent_trace.repository_remote` URL (default `origin`) before prompts, context/bootstrap, lifecycle setup, or integration writes. `services/setup/command.rs` maps missing prerequisites to typed `NotGitRepository` and `NotGitRemote` diagnostics while preserving technical sources without rendering remote URLs. +Setup repository preflight: every `sce setup` mode, including `--bootstrap-context`, validates an initialized Git repository and the configured `agent_trace.repository_remote` URL (default `origin`) before prompts, context/bootstrap, lifecycle setup, or integration writes. `services/setup/command.rs` maps only Git's explicit missing-repository result and an actually missing configured remote URL to typed `NotGitRepository` and `NotGitRemote` diagnostics; Git launch, permission, bare/malformed-repository, and remote-lookup execution failures remain runtime errors with preserved technical sources, without rendering remote URLs. Sync owns the complete progress boundary in `cli/src/services/sync/progress.rs`: the consumer-typed `ProgressReporter` contract, no-op reporter, focused contract tests, and fixed `indicatif` terminal adapter. `SyncProgressEvent` remains owned by `cli/src/services/sync/sync.rs`; `sync/command.rs` selects the adapter or no-op implementation by output format, there is no top-level `cli/src/services/progress/` module, and JSON callers use the sync-owned no-op reporter. The same config resolver now also owns the attribution-hooks gate used by local hook runtime: opt-out env `SCE_ATTRIBUTION_HOOKS_DISABLED` overrides `policies.attribution_hooks.enabled` with inverted semantics, and the gate defaults to enabled unless explicitly disabled. The config service split now includes `cli/src/services/config/resolver.rs` as the focused owner for config-file discovery, file-layer merging, env/flag/default precedence, auth-key resolution, observability resolution, attribution-hooks resolution, and default-discovered invalid-file degradation; `cli/src/services/config/mod.rs` remains the facade/rendering orchestration surface while preserving existing `services::config` imports. diff --git a/context/plans/setup-git-remote-preflight.md b/context/plans/setup-git-remote-preflight.md index fefa24af..08097373 100644 --- a/context/plans/setup-git-remote-preflight.md +++ b/context/plans/setup-git-remote-preflight.md @@ -16,6 +16,13 @@ operators stable, actionable messages explaining `git init` or resolution and `lookup_remote_url` implementations; this change does not test remote network reachability or redesign repository identity resolution. +Correction to the completed preflight implementation: narrow typed error +classification so only Git's explicit `not a git repository` failure becomes +`NotGitRepository`, and only an actually missing configured remote URL becomes +`NotGitRemote`. Git process-launch, permission, bare-repository, malformed +repository, and remote-lookup execution failures must remain runtime errors +with their technical sources intact. + ## Acceptance criteria How this plan is proven complete. Each criterion is observable and names the @@ -45,6 +52,12 @@ performs final validation. and remote URLs are not echoed in diagnostics. - Validate: Existing CLI error tests plus the new setup error tests pass, and the full repository check suite succeeds. +- [x] AC5: Setup classifies only the explicit missing-repository and missing- + remote conditions as typed user errors; Git/process/configuration failures + remain runtime errors and retain their technical sources. + - Validate: Focused setup and repository-identity tests cover a missing Git + repository, missing remote URL, Git launch/non-repository edge failures, + and remote lookup execution failures with exact `CliError` classification. ### Full validation @@ -65,6 +78,8 @@ which criterion they map to. repository preflights precede context/config/database/bootstrap side effects. - `context/sce/cli-error-code-taxonomy.md` — add the two setup-specific `UserError` catalog entries and preserve-source rendering contract. +- `context/cli/repository-identity.md` — document the strict remote-lookup + distinction used by setup while preserving repository-identity behavior. ## Task context synchronization lifecycle @@ -86,10 +101,10 @@ Persist this field in every plan; this is durable plan state, not chat state: canonicalization changes, changes to Agent Trace identity precedence, changes to `doctor`, and changes to setup flags or successful output. - **Constraints:** Reuse `setup::ensure_git_repository`, the existing config - resolver for `agent_trace.repository_remote`, and - `repository_identity::resolve::lookup_remote_url`; preserve stdout/stderr - and stable `SCE-ERR-RUNTIME` behavior for `UserError` failures; do not expose - raw credential-bearing remote URLs. + resolver for `agent_trace.repository_remote`, and the repository-identity + remote lookup seam; preserve stdout/stderr and stable `SCE-ERR-RUNTIME` + behavior for non-user failures; do not expose raw credential-bearing remote + URLs. - **Non-goal:** Accepting any arbitrary Git remote when the configured SCE remote is missing; setup must validate the remote Agent Trace will use. @@ -130,8 +145,21 @@ Persist this field in every plan; this is durable plan state, not chat state: - Files changed: `cli/src/services/error.rs`, `cli/src/services/setup/command.rs`, `context/overview.md`, `context/cli/cli-command-surface.md`, `context/sce/setup-githooks-cli-ux.md`, `context/sce/setup-repo-local-config-bootstrap.md`, `context/sce/cli-error-code-taxonomy.md`, `context/plans/setup-git-remote-preflight.md` - Result: Wired setup dispatch through a pre-prompt Git-root and configured-remote gate. The effective `agent_trace.repository_remote` is resolved with the existing config resolver and defaults to `origin`; missing prerequisites map through `CliError::user_with_source` to typed `NotGitRepository` or `NotGitRemote` diagnostics while preserving technical sources without rendering remote URLs. Added command-level coverage for missing Git, missing origin, configured origin, configured alternate remotes, and unrelated remotes, and documented the ordering and ownership across durable setup/error context. - Verify: Focused setup preflight tests passed (6 tests). `nix flake check` passed all repository checks; an earlier full-check failure was an unrelated flaky Agent Trace row-count test and passed on rerun. - - Context impact: Root — setup is now governed by a repository-wide Git-plus-configured-remote preflight and the typed CLI error catalog; updated root setup/error summaries and the authoritative setup/bootstrap/error domain contracts. - - Context synchronization: synced + - Context impact: Root — setup is now governed by a repository-wide Git-plus-configured-remote preflight and the typed CLI error catalog; updated root setup/error summaries and the authoritative setup/bootstrap/error domain contracts. + - Context synchronization: synced + +- [x] T03: `Narrow setup preflight error classification` (status:done) + - Task ID: T03 + - Scope: In — distinguish the exact `not a git repository` Git failure from other `rev-parse` execution failures; distinguish a missing/empty configured remote URL from failures running the remote lookup; preserve technical `CliError::Internal` runtime classification for the latter cases; add focused regression coverage and update the setup/error/repository-identity context contracts. Out — remote network reachability, repository identity precedence, doctor behavior, and changes to successful setup output. + - Dependencies: T02 + - Done when: A missing Git repository still renders `NotGitRepository`, a missing configured remote URL still renders `NotGitRemote`, and Git launch/permission/bare/malformed-repository plus remote lookup execution failures render as runtime errors with their original sources; credential-bearing remote URLs remain absent from user-facing diagnostics. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml setup`; `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml repository_identity`; `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml error` + - Completed: 2026-08-25 + - Files changed: `cli/src/services/error.rs`, `cli/src/services/repository_identity/resolve.rs`, `cli/src/services/setup/command.rs`, `cli/src/services/setup/mod.rs`, `context/cli/repository-identity.md`, `context/overview.md`, `context/plans/setup-git-remote-preflight.md`, `context/sce/cli-error-code-taxonomy.md`, `context/sce/setup-githooks-cli-ux.md`, `context/sce/setup-repo-local-config-bootstrap.md` + - Result: Added strict remote URL lookup that preserves execution/configuration failures while retaining compatibility fail-to-missing behavior for repository identity resolution. Setup now emits typed errors only for Git's explicit missing-repository failure and missing configured remote URL, mapping all other preflight failures to runtime `CliError::Internal` errors with technical sources preserved. Added command, setup, repository-identity, and error regression coverage plus updated durable setup/error/identity contracts. + - Verify: `setup` passed with 69 tests; `repository_identity` passed with 25 tests; `error` passed with 46 tests. + - Context impact: Root — setup preflight error classification and the repository-identity remote lookup boundary now distinguish expected missing prerequisites from runtime execution failures; durable root setup/error/identity contracts were updated. + - Context synchronization: synced ## Open questions @@ -141,20 +169,22 @@ non-network validation boundary were resolved during discussion. ## Validation Report **Status:** validated -**Date:** 2026-08-24 +**Date:** 2026-08-25 ### Commands run - `nix flake check` -> exit 0 (all repository checks passed) -- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml error` -> exit 0 (35 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 setup` -> exit 0 (69 tests passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml repository_identity` -> exit 0 (25 tests passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml error` -> exit 0 (46 tests passed) ### Success-criteria verification -- [x] AC1: `sce setup` reports a typed, actionable `NotGitRepository` failure and performs no setup writes -> command-level preflight test passed for a non-repository directory. -- [x] AC2: `sce setup` reports a typed, actionable `NotGitRemote` failure and performs no setup writes -> command-level missing-origin test passed; error tests confirmed remote-add guidance. -- [x] AC3: Remote validation uses the resolved `agent_trace.repository_remote` name -> setup tests passed for default `origin`, configured alternate remotes, and rejection of unrelated remotes. -- [x] AC4: Existing setup behavior and user-error rendering remain compatible without exposing remote URLs -> error and setup test filters plus the full repository check passed; source-preservation tests passed without rendered URL leakage. +- [x] AC1: `sce setup` reports a typed, actionable `NotGitRepository` failure and performs no setup writes -> setup preflight tests passed for a non-repository directory and preserved the `git init` guidance. +- [x] AC2: `sce setup` reports a typed, actionable `NotGitRemote` failure and performs no setup writes -> setup and error tests passed for missing configured remotes and preserved `git remote add ` guidance. +- [x] AC3: Remote validation uses the resolved `agent_trace.repository_remote` name -> setup and repository-identity tests passed for default `origin`, configured alternate remotes, and rejection of unrelated remotes. +- [x] AC4: Existing setup behavior and user-error rendering remain compatible without exposing remote URLs -> full repository checks and focused setup/error tests passed, including source preservation and credential-safe diagnostics. +- [x] AC5: Setup classifies only explicit missing-repository and missing-remote conditions as typed user errors -> setup, repository-identity, and error tests passed for missing prerequisites, Git/runtime edge failures, strict remote lookup failures, preserved sources, and safe diagnostics. ### Failed checks and follow-ups diff --git a/context/sce/cli-error-code-taxonomy.md b/context/sce/cli-error-code-taxonomy.md index b9691fc6..02f47c72 100644 --- a/context/sce/cli-error-code-taxonomy.md +++ b/context/sce/cli-error-code-taxonomy.md @@ -25,14 +25,14 @@ It complements the numeric process exit-code classes documented in `context/sce/ - High-frequency parse/invocation failures use explicit `Try:` remediations instead of generic usage-only hints. - Top-level unknown command/option messages include targeted retry guidance (`sce --help` and command-local `sce --help`). - Setup invocation validation failures (`--repo` without `--hooks`, mutually exclusive target flags, unexpected args) include concrete valid alternatives. -- Setup repository preflight failures use fixed `UserError::NotGitRepository` and `UserError::NotGitRemote` sentences with `git init` and `git remote add ` remediation; their runtime technical sources may retain the configured remote name but never the remote URL. +- Setup repository preflight failures use fixed `UserError::NotGitRepository` and `UserError::NotGitRemote` sentences with `git init` and `git remote add ` remediation only for Git's explicit `not a git repository` result and an actually missing/empty configured remote URL. Git launch, permission, bare/malformed-repository, configuration, and remote-lookup execution failures remain `CliError::Internal` runtime errors with their technical sources; no user-facing diagnostic echoes a remote URL. - Hooks invocation validation failures (missing hook subcommand, missing `commit-msg` message file, unknown subcommand) include command-form examples that are copyable for retry automation. - This actionable-message normalization is owned by parser/validation paths in `cli/src/app.rs`, `cli/src/services/setup/mod.rs`, `cli/src/services/setup/command.rs`, and `cli/src/services/hooks/mod.rs`. ## Ownership - `FailureClass` in `cli/src/services/error.rs` owns class selection and stable code assignment (`FailureClass::code()`). -- `CliError::{User,Internal}` in `cli/src/services/error.rs` is the typed CLI-boundary error type; `CliError::code()`/`CliError::class()` delegate to the failure class. `CliError::User` carries a catalog `UserError` (`NotAuthenticated`, `NotGitRepository`, or `NotGitRemote`) 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. +- `CliError::{User,Internal}` in `cli/src/services/error.rs` is the typed CLI-boundary error type; `CliError::code()`/`CliError::class()` delegate to the failure class. `CliError::User` carries a catalog `UserError` (`NotAuthenticated`, `NotGitRepository`, or `NotGitRemote`) for expected, deliberately-explained failures; `CliError::Internal` carries a live `anyhow::Error` source for every other failure, including non-classifiable setup preflight failures. `CliError::User` may also carry an optional preserved technical `source`, kept for observability only and never rendered to the terminal. - `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()`. - Command and domain layers construct and return a `CliError`; they do not format terminal text, apply styling, or decide authentication/user-error semantics from string matching. `app_support` is the sole owner of turning a `CliError` into the final stderr sentence. - `Logger::log_cli_error` in `cli/src/services/observability.rs` owns structured error logging with `sce.error.{code}` event IDs. diff --git a/context/sce/setup-githooks-cli-ux.md b/context/sce/setup-githooks-cli-ux.md index 3f890bf0..a54b072f 100644 --- a/context/sce/setup-githooks-cli-ux.md +++ b/context/sce/setup-githooks-cli-ux.md @@ -26,7 +26,7 @@ Validation is deterministic and enforced during setup option resolution: - `--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` -- all `sce setup` modes (config-only, hooks-only, combined, interactive, and `--bootstrap-context`) require an initialized Git repository and a URL for the effective `agent_trace.repository_remote` before prompts or writes; `setup::command` uses the configured name, defaulting to `origin`, and reports typed `NotGitRepository` or `NotGitRemote` failures without echoing remote URLs +- all `sce setup` modes (config-only, hooks-only, combined, interactive, and `--bootstrap-context`) require an initialized Git repository and a URL for the effective `agent_trace.repository_remote` before prompts or writes; `setup::command` uses the configured name, defaulting to `origin`, and reports typed `NotGitRepository` or `NotGitRemote` failures without echoing remote URLs only for the explicit missing-repository and missing-URL cases. Git/process/configuration failures remain runtime diagnostics with their technical sources preserved. Target-install mode contract: diff --git a/context/sce/setup-repo-local-config-bootstrap.md b/context/sce/setup-repo-local-config-bootstrap.md index 22bf57bf..167cfe95 100644 --- a/context/sce/setup-repo-local-config-bootstrap.md +++ b/context/sce/setup-repo-local-config-bootstrap.md @@ -57,7 +57,7 @@ The same write also records the run's resolved optional-workflow selection under ## Relationship to other setup contracts -- The Git-repo gate (`ensure_git_repository`) and effective named-remote URL preflight remain the preconditions for every setup write path, including context-only bootstrap. +- The Git-repo gate (`ensure_git_repository`) and effective named-remote URL preflight remain the preconditions for every setup write path, including context-only bootstrap. The gate classifies only an explicit Git `not a git repository` result and an actually missing/empty named-remote URL as typed user errors; Git/process/configuration and remote-lookup execution failures remain runtime errors with technical sources preserved. - Context baseline bootstrap is independent of config/DB/hooks install and runs before those steps on normal setup paths. - Local bootstrap (repo config + local DB init) is independent of config install and hook install; it runs before both after context baseline bootstrap. - The bootstrap payload matches the `$schema` declaration accepted by startup config loading and the Pkl-authored JSON Schema embedded from Cargo `OUT_DIR`. From d31f4fd2e91dfba1ee8bdce4d394e23799fb8885 Mon Sep 17 00:00:00 2001 From: Ivan Ivic Date: Wed, 26 Aug 2026 13:38:30 +0200 Subject: [PATCH 3/5] setup+config: Fail closed on invalid config and pin Git locale Reject invalid discovered configuration before setup side effects or Agent Trace identity fallback. Pin setup and repository-identity Git commands to LC_ALL=C for stable parsing and diagnostics Co-authored-by: SCE --- cli/src/services/config/resolver.rs | 42 +++- .../services/repository_identity/resolve.rs | 1 + cli/src/services/setup/command.rs | 1 + cli/src/services/setup/mod.rs | 19 ++ context/architecture.md | 2 +- context/cli/agent-trace-storage.md | 4 +- context/cli/config-precedence-contract.md | 2 +- context/cli/repository-identity.md | 2 +- context/context-map.md | 5 +- ...p-storage-fail-closed-on-invalid-config.md | 76 +++++++ context/glossary.md | 2 +- context/overview.md | 2 +- context/patterns.md | 1 + .../setup-invalid-config-and-git-locale.md | 196 ++++++++++++++++++ context/sce/setup-githooks-install-flow.md | 5 +- .../sce/setup-repo-local-config-bootstrap.md | 14 +- 16 files changed, 352 insertions(+), 22 deletions(-) create mode 100644 context/decisions/2026-08-26-setup-storage-fail-closed-on-invalid-config.md create mode 100644 context/plans/setup-invalid-config-and-git-locale.md diff --git a/cli/src/services/config/resolver.rs b/cli/src/services/config/resolver.rs index fb5a2581..38b28e77 100644 --- a/cli/src/services/config/resolver.rs +++ b/cli/src/services/config/resolver.rs @@ -153,13 +153,7 @@ pub(crate) fn resolve_agent_trace_auto_sync_runtime_config( pub(crate) fn resolve_agent_trace_storage_runtime_config( cwd: &Path, ) -> Result { - let runtime = resolve_runtime_config_with( - &ConfigRequest { - report_format: ReportFormat::Text, - config_path: None, - log_level: None, - timeout_ms: None, - }, + resolve_agent_trace_storage_runtime_config_with( cwd, |key| std::env::var(key).ok(), |path| { @@ -168,8 +162,42 @@ pub(crate) fn resolve_agent_trace_storage_runtime_config( }, Path::exists, resolve_default_global_config_path, + ) +} + +fn resolve_agent_trace_storage_runtime_config_with( + cwd: &Path, + env_lookup: FEnv, + read_file: FRead, + path_exists: fn(&Path) -> bool, + resolve_global_config_path: FGlobalPath, +) -> Result +where + FEnv: Fn(&str) -> Option, + FRead: Fn(&Path) -> Result, + FGlobalPath: Fn() -> Result, +{ + let runtime = resolve_runtime_config_with( + &ConfigRequest { + report_format: ReportFormat::Text, + config_path: None, + log_level: None, + timeout_ms: None, + }, + cwd, + env_lookup, + read_file, + path_exists, + resolve_global_config_path, )?; + if !runtime.validation_errors.is_empty() { + bail!( + "Agent Trace storage config resolution failed because a discovered config file is invalid: {}", + runtime.validation_errors.join(" | ") + ); + } + Ok(ResolvedAgentTraceStorageRuntimeConfig { repository_id: runtime.agent_trace_repository_id.value, repository_remote: runtime.agent_trace_repository_remote.value, diff --git a/cli/src/services/repository_identity/resolve.rs b/cli/src/services/repository_identity/resolve.rs index 5d611741..6c2db6f1 100644 --- a/cli/src/services/repository_identity/resolve.rs +++ b/cli/src/services/repository_identity/resolve.rs @@ -141,6 +141,7 @@ pub fn lookup_remote_url_strict( .arg("-C") .arg(repository_root) .args(["config", "--get", &format!("remote.{remote_name}.url")]) + .env("LC_ALL", "C") .output() .with_context(|| { format!( diff --git a/cli/src/services/setup/command.rs b/cli/src/services/setup/command.rs index dfa02ce0..a37d7c29 100644 --- a/cli/src/services/setup/command.rs +++ b/cli/src/services/setup/command.rs @@ -23,6 +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 = resolve_setup_repository(&setup_start_path)?; + setup::validate_existing_repo_local_config(&repository_root).map_err(CliError::runtime)? let setup_dispatch = if self.request.context_only { None diff --git a/cli/src/services/setup/mod.rs b/cli/src/services/setup/mod.rs index b7f0dbe2..31696202 100644 --- a/cli/src/services/setup/mod.rs +++ b/cli/src/services/setup/mod.rs @@ -485,6 +485,24 @@ pub fn ensure_git_remote(repository_root: &Path, remote_name: &str) -> Result<() })) } +/// Validates an existing repo-local `.sce/config.json` before setup performs +/// any other repository or lifecycle work. An absent config remains eligible +/// for the normal bootstrap path. +pub fn validate_existing_repo_local_config(repository_root: &Path) -> Result<()> { + let config_file = RepoPaths::new(repository_root).sce_config_file(); + if !config_file.exists() { + return Ok(()); + } + + crate::services::config::validate_config_file(&config_file).with_context(|| { + format!( + "Setup preflight rejected invalid repo-local config file '{}'", + config_file.display() + ) + }) +} +} + /// Bootstraps the repo-local `.sce/config.json` file if it does not already exist. /// /// Creates the `.sce/` parent directory as needed, then writes the canonical @@ -1221,6 +1239,7 @@ mod install { let output = Command::new("git") .args(args) .current_dir(repository_root) + .env("LC_ALL", "C") .output() .with_context(|| { format!( diff --git a/context/architecture.md b/context/architecture.md index ccfdb91c..9c7ed7fa 100644 --- a/context/architecture.md +++ b/context/architecture.md @@ -115,7 +115,7 @@ The repository includes a new placeholder Rust binary crate at `cli/`. - `cli/src/cli_schema.rs` is now the canonical owner for top-level command metadata for the real clap-backed command set (`auth`, `config`, `setup`, `doctor`, `hooks`, `policy`, `sync`, `version`, `completion`), including the slim top-level help purpose text and per-command visibility on `sce`, `sce help`, and `sce --help`; `cli/src/command_surface.rs` remains the custom top-level help renderer and known-command classifier, adding the synthetic `help` row plus the ASCII banner while consuming that shared metadata instead of maintaining a parallel command catalog. - `cli/src/services/default_paths.rs` is the canonical production path catalog for the CLI: it resolves config/state/cache roots with platform-aware XDG or `dirs` fallbacks through an internal `roots` seam, exposes named default paths for current persisted artifacts and database/log files (global config, auth tokens, auth DB, local DB, default observability log directory, and the sole Agent Trace DB path helper `agent_trace_db_path_for_repository` under `repos//agent-trace.db`; the former global-sentinel and per-checkout Agent Trace path helpers were removed by the `retire-legacy-agent-trace-db` plan), and owns canonical repo-relative, embedded-asset, install, hook, and context-path accessors so non-test production path definitions have one shared owner. Compile-time generated payload paths are owned by `build.rs` under `OUT_DIR`, not by the default-path catalog. Current production consumers such as config discovery, observability config resolution, doctor reporting, setup/install flows, database adapters, checkout identity, Agent Trace storage resolution, and local hook runtime path resolution consume this shared catalog rather than defining owned path literals in their own modules. - `cli/src/services/agent_trace.rs` is the Rust CLI owner for the SCE web base URL (`SCE_WEB_BASE_URL`) and exposes helpers for SCE-owned URL construction: Agent Trace conversation lookup URLs, persisted Agent Trace trace URLs, Agent Trace session URLs, and setup-created config schema URLs. Production Rust code should consume those helpers instead of repeating `sce.crocoder.dev` literals. The config resolver separately owns the `control_plane_base_url` runtime seam, whose baked `sce sync` default is `https://sce.crocoderlab.dev`; this control-plane host is not a web URL or schema owner. -- `cli/src/services/config/mod.rs` is the config service facade and `sce config` orchestration surface (`show`, `validate`, `--help`), with bare `sce config` routed by `cli/src/app.rs` to the same help payload as `sce config --help`. Focused submodules own the implementation slices: `types.rs` owns shared config/runtime primitives, `schema.rs` owns generated schema embedding plus typed file parsing, `policy.rs` owns bash-policy semantic validation plus policy-specific formatting and runtime preset-catalog access for the Rust evaluator, `resolver.rs` owns deterministic config-file discovery, file-layer merging, explicit value precedence (`flags > env > config file > defaults` where flag-backed), shared auth-key resolution, observability-runtime resolution, attribution-hooks runtime gate and `agent_trace.auto_sync` resolution, database-retry config resolution and `DATABASE_RETRY_CONFIG` `OnceLock` initialization, default-discovered invalid-file degradation, and explicit-path fatal errors for `--config` / `SCE_CONFIG_FILE`, and private `render.rs` owns `sce config show` / `sce config validate` text and JSON output construction plus rendering-specific display-value helpers. The facade preserves existing `services::config` imports for startup/auth/hooks callers while delegating command execution to resolution plus rendering submodules. +- `cli/src/services/config/mod.rs` is the config service facade and `sce config` orchestration surface (`show`, `validate`, `--help`), with bare `sce config` routed by `cli/src/app.rs` to the same help payload as `sce config --help`. Focused submodules own the implementation slices: `types.rs` owns shared config/runtime primitives, `schema.rs` owns generated schema embedding plus typed file parsing, `policy.rs` owns bash-policy semantic validation plus policy-specific formatting and runtime preset-catalog access for the Rust evaluator, `resolver.rs` owns deterministic config-file discovery, file-layer merging, explicit value precedence (`flags > env > config file > defaults` where flag-backed), shared auth-key resolution, observability-runtime resolution, attribution-hooks runtime gate and `agent_trace.auto_sync` resolution, database-retry config resolution and `DATABASE_RETRY_CONFIG` `OnceLock` initialization, default-discovered invalid-file degradation, strict invalid-discovered-layer errors for Agent Trace storage, and explicit-path fatal errors for `--config` / `SCE_CONFIG_FILE`, and private `render.rs` owns `sce config show` / `sce config validate` text and JSON output construction plus rendering-specific display-value helpers. The facade preserves existing `services::config` imports for startup/auth/hooks callers while delegating command execution to resolution plus rendering submodules. - `cli/src/services/output_format.rs` defines the canonical shared CLI output-format contract (`OutputFormat`) for supporting commands, with deterministic `text|json` parsing and command-scoped actionable invalid-value guidance. - `cli/src/services/config/types.rs` is the canonical owner for the shared runtime/config primitive seam used by the CLI: `LogLevel`, `LogFormat`, `SCE_LOG_LEVEL`, `SCE_LOG_FORMAT`, `SCE_LOG_DIR`, `DEFAULT_LOG_FILE_RETENTION_LIMIT`, and the shared bool parsing helpers used by both config resolution and observability bootstrap; `cli/src/services/config/mod.rs` re-exports those primitives through the facade. - `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. diff --git a/context/cli/agent-trace-storage.md b/context/cli/agent-trace-storage.md index 5973a7fd..1b55f589 100644 --- a/context/cli/agent-trace-storage.md +++ b/context/cli/agent-trace-storage.md @@ -13,7 +13,7 @@ Module at `cli/src/services/agent_trace_storage/` (T04 of the `repository-scoped ## Resolution flow -1. Repository identity via `repository_identity::resolve` precedence (explicit config ID → configured remote URL, default `origin`); resolution errors carry `.sce/config.json` guidance and never echo URLs. A failed identity resolution creates no state directories. +1. Agent Trace storage runtime config is resolved through the config service; any invalid discovered config layer is an error at this boundary rather than a skipped layer with fallback values. For valid input, repository identity uses `repository_identity::resolve` precedence (explicit config ID → configured remote URL, default `origin`); resolution errors carry `.sce/config.json` guidance and never echo URLs. A failed config or identity resolution creates no state directories. 2. Checkout identity reuse via `checkout::resolve_git_dir` + `get_or_create_checkout_id` (`/sce/checkout-id`). 3. DB path from `default_paths::agent_trace_db_path_for_repository{,_at}`, which rejects empty or path-unsafe repository IDs (separators, `.`, `..`). 4. DB open splits by caller through `agent_trace_db::repository::RepositoryAgentTraceDb`, sharing steps 1–3 through an internal `open_storage_with` helper parameterized by the DB-opener: @@ -28,4 +28,4 @@ The resolver never selects, creates, or touches pre-migration checkout-scoped `< Registered in `cli/src/services/mod.rs` and consumed by hook runtime, Agent Trace lifecycle setup, and `sce sync`. T05 changed the resolved DB handle to the repository-scoped adapter and validates the stored `repository_metadata.repository_id` before returning storage; T08 wired hooks/lifecycle to pass resolved config values into this context; the former trace UX was later removed by the `retire-legacy-agent-trace-db` plan. The `agent-trace-source-instance-id` plan's T03 split hook-runtime resolution into its own no-migration entrypoint, switching `open_agent_trace_db_for_hook_runtime` in `cli/src/services/hooks/mod.rs` off the setup/lifecycle resolver so hook runtime never runs migration `002` (or any migration). Covered by in-module tests: repository separation, SSH/HTTPS clone consolidation, linked-worktree consolidation, explicit-ID override, idempotent re-resolution, missing-identity guidance, path-segment validation, pre-migration checkout DB byte preservation/non-selection, empty fresh repository DB state, repository-level row sharing across equivalent clone checkouts, credential-safe remote canonicalization, concurrent first-open convergence, and hook-runtime resolution (fails before setup on a missing DB, fails before setup on a baseline-only pre-`002` schema without recording migration `002`, and matches setup's `RepositoryMetadata` once setup has run) (`nix build .#checks..cli-tests`). -See also: [repository-identity.md](repository-identity.md), [checkout-identity.md](checkout-identity.md), [default-path-catalog.md](default-path-catalog.md), [../sce/agent-trace-db.md](../sce/agent-trace-db.md) +See also: [repository-identity.md](repository-identity.md), [checkout-identity.md](checkout-identity.md), [default-path-catalog.md](default-path-catalog.md), [../sce/agent-trace-db.md](../sce/agent-trace-db.md), and [the fail-closed boundary decision](../decisions/2026-08-26-setup-storage-fail-closed-on-invalid-config.md). diff --git a/context/cli/config-precedence-contract.md b/context/cli/config-precedence-contract.md index 82568794..1ac3e725 100644 --- a/context/cli/config-precedence-contract.md +++ b/context/cli/config-precedence-contract.md @@ -69,7 +69,7 @@ Config file selection follows this deterministic order: When both discovered defaults exist, they are merged in memory in deterministic order `global -> local`, and local values override global values per key. -When a default-discovered global or repo-local config file exists but fails JSON parsing, top-level-object validation, or schema validation, runtime resolution now skips that file, collects the failure text in `validation_errors`, and continues with remaining discovered layers plus defaults. Explicit `--config ` and `SCE_CONFIG_FILE` selections remain fatal on those errors. This means normal command startup still reaches dispatch for commands such as `sce version`, `sce doctor`, and `sce hooks commit-msg` even when discovered config is invalid. +When a default-discovered global or repo-local config file exists but fails JSON parsing, top-level-object validation, or schema validation, runtime resolution now skips that file, collects the failure text in `validation_errors`, and continues with remaining discovered layers plus defaults. Explicit `--config ` and `SCE_CONFIG_FILE` selections remain fatal on those errors. This means normal command startup still reaches dispatch for commands such as `sce version`, `sce doctor`, and `sce hooks commit-msg` even when discovered config is invalid. Setup and Agent Trace storage are deliberate stricter consumers: setup validates an existing repo-local file after Git-root resolution and before prompts, context bootstrap, lifecycle work, or asset installation, while storage resolution errors on any invalid discovered layer instead of using fallback identity values. See [the fail-closed boundary decision](../decisions/2026-08-26-setup-storage-fail-closed-on-invalid-config.md). ## Validation contract diff --git a/context/cli/repository-identity.md b/context/cli/repository-identity.md index f948a728..e14b35b7 100644 --- a/context/cli/repository-identity.md +++ b/context/cli/repository-identity.md @@ -33,7 +33,7 @@ Example: `git@GitHub.com:Acme/Widgets.git`, `ssh://git@github.com:22/Acme/Widget `repository_identity/resolve.rs` applies the repository identity precedence at runtime: 1. Explicit `agent_trace.repository_id` config value (trim-only canonicalization; invalid explicit values error, they do not fall back to remotes). -2. URL of the configured Git remote (`agent_trace.repository_remote`, default `origin`), read via `git config --get remote..url`. +2. URL of the configured Git remote (`agent_trace.repository_remote`, default `origin`), read via `git config --get remote..url` with `LC_ALL=C` for locale-stable output. 3. Otherwise an actionable error pointing at `.sce/config.json`. - `resolve_repository_identity(repository_root, explicit_identity, remote_name)` — process-spawning entrypoint. diff --git a/context/context-map.md b/context/context-map.md index 5aaa2a05..b1985622 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -12,7 +12,7 @@ Feature/domain context: - `context/cli/cli-command-surface.md` (CLI command surface including top-level help with ASCII art banner and gradient rendering, setup install flow with the repeatable `sce setup --workflow ` optional-workflow selection and its interactive post-target multi-select, WorkOS device authorization flow + token storage behavior including stored-credential renewal through `sce auth login`, attribution-only hook routing with validated post-commit `--remote-url` plumbing plus DB-backed `diff-trace` dual persistence and post-commit Agent Trace payload persistence including range `content_hash`, setup-owned local DB + repository-scoped Agent Trace DB bootstrap plus doctor DB health coverage with credential-safe repository identity diagnostics, centralized Rust SCE web URL helpers in `services::agent_trace`, nested flake release package/app installability, Cargo local install + crates.io readiness policy, hidden `sce policy bash` command adapter for bash-policy hook callers, and top-level `sce sync` command wiring for current-repository Agent Trace synchronization; static `RuntimeCommand` enum dispatch lives in `services/command_registry.rs`, command payload structs for help/version/completion/auth/config/setup/doctor/hooks/policy/sync are owned by their respective `services/{name}/command.rs` files, and clap-to-runtime conversion lives in `services/parse/command_runtime.rs`) - `context/cli/default-path-catalog.md` (canonical production CLI path-ownership contract centered on `cli/src/services/default_paths.rs`, including persisted auth/config files, named DB paths for auth/local/repository-scoped Agent Trace databases, the default observability log-dir accessor consumed by config resolution with Linux `${XDG_STATE_HOME:-~/.local/state}/sce/logs` fallback semantics, repo-relative, embedded-asset, install, hook, and context-path families plus the regression guard that keeps production path ownership centralized) - `context/cli/repository-identity.md` (repository identity module in `cli/src/services/repository_identity/`: pure scheme-neutral `host[:port]/path` canonicalization for SCP/`ssh://`/HTTPS/`git://` remote URLs with credential stripping, hostname lowercasing, default-port removal, and query/fragment/trailing-`.git` cleanup, trim-only explicit-identity handling, `sha256("sce-repository-id-v1\0" + canonical_identity)` repository IDs, credential-safe fieldless errors, plus the `resolve` runtime submodule applying explicit-config-then-configured-remote precedence with `git config --get remote..url` lookup, `RepositoryIdentitySource` provenance, and `.sce/config.json`-guidance resolution errors that never echo URLs; consumed by the T04 `agent_trace_storage` resolver) -- `context/cli/agent-trace-storage.md` (repository-scoped Agent Trace storage resolver in `cli/src/services/agent_trace_storage/`: `AgentTraceStorageContext` inputs mirroring the `agent_trace.*` config keys, `ResolvedAgentTraceStorage` carrying repository identity + checkout ID + `/sce/repos//agent-trace.db` path + open `RepositoryAgentTraceDb` + typed `RepositoryMetadata`, `resolve_agent_trace_storage{,_at_state_root}` setup/lifecycle entrypoints with idempotent concurrent-safe first open via bounded fast-path-then-migrate retry plus narrow one-file schema migration-metadata repair and repository metadata validation, a separate no-migration `resolve_agent_trace_storage_for_hook_runtime{,_at_state_root}` pair that high-frequency hook callers use exclusively and that never runs migration `002` or any migration, path-unsafe repository ID rejection in `default_paths::agent_trace_db_path_for_repository{,_at}`, strict never-touch boundary for any pre-migration checkout-scoped/global DB files, and active hook/runtime plus Agent Trace lifecycle setup call-site consumption after T08) +- `context/cli/agent-trace-storage.md` (repository-scoped Agent Trace storage resolver in `cli/src/services/agent_trace_storage/`: `AgentTraceStorageContext` inputs mirroring the `agent_trace.*` config keys, strict rejection of invalid discovered config before identity fallback, `ResolvedAgentTraceStorage` carrying repository identity + checkout ID + `/sce/repos//agent-trace.db` path + open `RepositoryAgentTraceDb` + typed `RepositoryMetadata`, `resolve_agent_trace_storage{,_at_state_root}` setup/lifecycle entrypoints with idempotent concurrent-safe first open via bounded fast-path-then-migrate retry plus narrow one-file schema migration-metadata repair and repository metadata validation, a separate no-migration `resolve_agent_trace_storage_for_hook_runtime{,_at_state_root}` pair that high-frequency hook callers use exclusively and that never runs migration `002` or any migration, path-unsafe repository ID rejection in `default_paths::agent_trace_db_path_for_repository{,_at}`, strict never-touch boundary for any pre-migration checkout-scoped/global DB files, and active hook/runtime plus Agent Trace lifecycle setup call-site consumption after T08) - `context/cli/checkout-identity.md` (current checkout identity infrastructure in `cli/src/services/checkout/`, including `/sce/checkout-id` UUIDv7 storage, setup/hook integration that creates/reuses checkout identity as repository-scoped Agent Trace diagnostic metadata, the removed per-checkout DB opener/path helper, `sce doctor` checkout identity display, and the never-touch on-disk handling of pre-migration checkout-scoped DB files that are no longer inspectable via the CLI) - `context/cli/patch-service.md` (standalone patch domain model, parser, JSON load helpers, and set operations in `cli/src/services/patch.rs` for in-memory parsed unified-diff representation, capturing only touched lines plus minimal per-file/per-hunk metadata, supporting both `Index:` SVN-style and `diff --git` git-style formats, with `ParseError` for actionable malformed-input diagnostics, `PatchLoadError`/`load_patch_from_json`/`load_patch_from_json_bytes` for storage-agnostic JSON reconstruction, `intersect_patches` for target-shaped overlap with exact-match-first and historical `kind`+`content` fallback semantics plus matched-constructed-line `session_id` and matched-constructed-hunk `model_id` provenance inheritance, and `combine_patches` for ordered patch combination with later-wins conflict resolution plus winning-hunk `model_id` provenance inheritance; repository structured-row reconstruction supplies persisted hunk-model and canonical touched-line-session provenance before these operations; `parse_patch`, `intersect_patches`, and `combine_patches` are consumed by the active post-commit hook runtime) - `context/cli/structured-patch-service.md` (Claude structured editor-hook derivation in `cli/src/services/structured_patch.rs`, including `Write` structured-update hunks, `Write` `tool_input.content` create fallback, `Edit` structured patches, deterministic skip reasons, `ParsedPatch` output semantics, Rust golden fixture coverage, and repository read-time enrichment that assigns persisted row `model_id` to each hunk and canonical row `session_id` to each touched line) @@ -56,7 +56,7 @@ Feature/domain context: - `context/sce/setup-githooks-hook-asset-packaging.md` (compile-time `sce setup --hooks` required-hook template packaging contract, including all-hook non-blocking missing-`sce` install guidance, available-CLI argument forwarding, post-commit-only origin remote lookup plus remote-URL forwarding/fallback behavior, setup-service accessor surface, and current validation posture) - `context/sce/setup-githooks-install-flow.md` (setup-service required-hook install orchestration with git-truth hooks-path resolution, managed-block merge content computation that preserves foreign hook content, per-hook installed/updated/skipped outcomes decided against the merged content, the unreachable-block advisory, and atomic-swap replacement with recovery guidance) - `context/sce/setup-githooks-cli-ux.md` (T04 composable `sce setup` target+`--hooks` / `--repo` command-surface contract, option compatibility validation, and deterministic setup/hook output semantics) -- `context/sce/setup-repo-local-config-bootstrap.md` (setup local bootstrap behavior: additive durable-context baseline via `sce setup --bootstrap-context` and every normal setup path, repo-local `.sce/config.json` create-if-missing via config lifecycle, additive `integrations.target` persistence after successful target installs, `integrations.optional_workflows` selection persistence with prompt-over-flag-over-persisted precedence, plus lifecycle-owned local DB initialization before hooks/config asset dispatch) +- `context/sce/setup-repo-local-config-bootstrap.md` (setup local bootstrap behavior: Git-root-gated validation of existing repo-local config before prompts, context, lifecycle, hooks, or assets, additive durable-context baseline via `sce setup --bootstrap-context` and every normal setup path, repo-local `.sce/config.json` create-if-missing via config lifecycle, additive `integrations.target` persistence after successful target installs, `integrations.optional_workflows` selection persistence with prompt-over-flag-over-persisted precedence, plus lifecycle-owned local DB initialization before hooks/config asset dispatch) - `context/sce/cli-security-hardening-contract.md` (T06 CLI redaction contract, setup `--repo` canonicalization/validation, and setup write-permission probe behavior) - `context/sce/agent-trace-post-rewrite-local-remap-ingestion.md` (current post-rewrite no-op baseline plus historical remap-ingestion reference) - `context/sce/agent-trace-rewrite-trace-transformation.md` (current post-rewrite no-op baseline plus historical rewrite-transformation reference) @@ -103,6 +103,7 @@ Recent decision records: - `context/decisions/2026-08-23-codex-event-scoped-apply-patch-evidence-identities.md` (uses bounded, deterministic `tool_use_id`-derived synthetic line identities for Codex apply_patch evidence; positions are evidence identities rather than source line numbers, with existing patch combination/intersection semantics unchanged) - `context/decisions/2026-08-23-codex-truthful-model-provenance.md` (preserves non-empty Codex model IDs unchanged, leaves blank/missing values nullable, and forbids inferred provider prefixes or a fabricated provider field) - `context/decisions/2026-08-23-codex-root-aware-hook-invocation.md` (requires generated Codex hook commands to resolve the Git root at invocation time, quote the helper path, preserve STDIN, and fail open when root resolution fails) +- `context/decisions/2026-08-26-setup-storage-fail-closed-on-invalid-config.md` (keeps general startup degradation for invalid discovered config while making setup and Agent Trace storage fail closed before side effects or fallback identity selection) - `context/decisions/2026-08-14-compact-task-record-supersedes-handoff.md` (the completed task record — `Completed`/`Files changed`/`Result`/`Verify`/`Context impact`/`Context synchronization`, identified only by plan path and task ID — is the sole durable input for immediate and cross-session task synchronization, with no separate persisted `Context synchronization handoff` structure; supersedes only the handoff-shape portion of `2026-08-12-persist-workflow-sync-lifecycle-in-plans.md`, whose `pending`/`synced`/`blocked` lifecycle-state invariant remains in force) - `context/decisions/2026-08-12-decision-gate-semantics.md` (nonqualifying/skipped decision gates are non-blocking; ADRs are immutable, active-only reuse is allowed, changed decisions create new dated records, and `Deprecated`/`Superseded` are creation-time-only statuses) - `context/decisions/2026-08-12-observational-final-validation.md` diff --git a/context/decisions/2026-08-26-setup-storage-fail-closed-on-invalid-config.md b/context/decisions/2026-08-26-setup-storage-fail-closed-on-invalid-config.md new file mode 100644 index 00000000..3643dc69 --- /dev/null +++ b/context/decisions/2026-08-26-setup-storage-fail-closed-on-invalid-config.md @@ -0,0 +1,76 @@ +# Decision: Fail Closed at Setup and Agent Trace Storage Boundaries for Invalid Discovered Config + +Date: 2026-08-26 +Status: Accepted +Plan: `context/plans/setup-invalid-config-and-git-locale.md` +Task: `T01` + +## Context + +Default-discovered configuration remains intentionally degradable for ordinary +startup so commands can continue with defaults and report validation issues. +That behavior is unsafe at two boundaries: setup can create databases, hooks, +context, and target assets, while Agent Trace storage resolution can select a +repository database from a fallback remote after discarding an invalid config +layer. The completed T01 implementation and focused Rust tests establish that +these boundaries must use the existing config validation seam before proceeding. + +## Decision + +`sce setup` and Agent Trace storage runtime configuration resolution fail closed +when a discovered config file is invalid, while general startup config +consumers retain their existing degraded-default behavior. + +## Rationale + +Setup must not perform side effects from an invalid repository configuration, +and storage identity must not silently select a potentially different database. +Keeping the stricter behavior at these two consumers preserves the established +startup compatibility contract without weakening repository safety. + +## Alternatives considered + +- **Continue with remaining layers and defaults everywhere** — preserves the + existing resolver behavior but permits setup side effects and wrong storage + identity selection. +- **Make all default-discovered config consumers fatal** — avoids degradation + but broadens the user-visible startup contract beyond the required boundary. + +## Compatibility and risks + +- Existing startup, inspection, and observability consumers continue to skip + invalid discovered layers and report validation errors; setup and Agent Trace + storage now return actionable validation failures instead. No schema or + precedence semantics change. + +## Guardrails + +- Validate only an existing repo-local config during setup, leaving absent-file + bootstrap behavior unchanged. +- Reuse the existing generated-schema and typed-config validation seam. +- Keep the strict storage check limited to invalid discovered config layers; + explicit identity, remote precedence, and repository canonicalization remain + unchanged. + +## Consequences + +- Invalid repo-local config cannot trigger setup prompts, context bootstrap, + lifecycle initialization, hooks, or target asset installation. +- Agent Trace storage resolution no longer returns fallback identity values when + a discovered config layer failed validation. +- Operators must repair invalid config before rerunning setup or storage-backed + Agent Trace operations. + +## Follow-up + +- `T02` continues the same plan's independent Git locale-stability change. + +## References + +- Plan: [`setup-invalid-config-and-git-locale`](../plans/setup-invalid-config-and-git-locale.md) +- Task: `T01` +- Current-state context: [`CLI config precedence contract`](../cli/config-precedence-contract.md) +- Current-state context: [`SCE setup local bootstrap`](../sce/setup-repo-local-config-bootstrap.md) +- Current-state context: [`Repository-scoped Agent Trace storage resolver`](../cli/agent-trace-storage.md) +- Evidence: [`config resolver`](../../cli/src/services/config/resolver.rs) +- Evidence: [`setup command`](../../cli/src/services/setup/command.rs) diff --git a/context/glossary.md b/context/glossary.md index 71b0eae0..0424a317 100644 --- a/context/glossary.md +++ b/context/glossary.md @@ -3,7 +3,7 @@ - `repo-level verification preference`: Current repository guidance that contributor-facing validation/check flows should prefer `nix flake check`; direct Cargo verification commands are secondary and used only when explicitly requested or for narrow targeted debugging, while `cargo fmt` remains the explicit autofix path. - lightweight post-task verification baseline: Required quick checks after each completed task in this repo: `nix run .#pkl-check-generated` and `nix flake check`. - disposable plan lifecycle: Policy where `context/plans/` holds active execution artifacts only; completed plans are disposable and durable outcomes must be reflected in current-state context files and/or `context/decisions/`. -- important change (context sync): A completed task change that affects cross-cutting behavior, repository-wide policy/contracts, architecture boundaries, or canonical terminology; these changes require root context edits in `context/overview.md`, `context/architecture.md`, and/or `context/glossary.md` instead of verify-only handling. +- important change (context sync): A completed task change that affects cross-cutting behavior, repository-wide policy/contracts, architecture boundaries, or canonical terminology; these changes require root context edits in `context/overview.md`, `context/architecture.md`, and/or `context/glossary.md` instead of verify-only handling. `setup config preflight` is the Git-root-gated check that validates an existing repo-local `.sce/config.json` before prompts, context bootstrap, lifecycle initialization, hooks, or target asset installation; invalid config fails setup closed, absent config remains eligible for create-if-missing bootstrap, Agent Trace storage has the parallel strict rule for invalid discovered config layers, and ordinary startup consumers retain degraded-default behavior. See [the fail-closed boundary decision](decisions/2026-08-26-setup-storage-fail-closed-on-invalid-config.md). - verify-only root context pass: Context-sync mode for localized tasks where root-level behavior, architecture, and terminology are unchanged; root shared files are checked against code truth but are not edited by default. - ephemeral generated payload: Files materialized by `config/pkl/generate.pkl` using payload-relative `config/.opencode/**`, `config/.claude/**`, `config/.pi/**`, `config/.agents/**`, `config/.codex/**`, and `config/schema/sce-config.schema.json` paths beneath Cargo `OUT_DIR`, temporary previews, or packaging fallbacks. These layouts are installed by `sce setup` but are never committed as repository target trees; `config/automated/.opencode/**` remains a forbidden generator surface. - `Codex root-aware hook invocation`: Generated `.codex/hooks.json` command contract that resolves the Git repository root at hook runtime, invokes the installed helper through quoted path expansion from root or nested event cwd, preserves JSON STDIN, and exits silently successfully when Git-root resolution fails. The existing helper remains responsible for missing-`sce` stderr guidance; the contract forbids install-time absolute paths and `eval`. diff --git a/context/overview.md b/context/overview.md index b7a53cd2..8452b848 100644 --- a/context/overview.md +++ b/context/overview.md @@ -12,7 +12,7 @@ The generated `/next-task` workflow persists task-level context-synchronization - **Stderr diagnostics:** stable `SCE-ERR-{PARSE,VALIDATION,RUNTIME,DEPENDENCY}` codes with class-default `Try:` remediation (see `context/sce/cli-error-code-taxonomy.md`). - **Stdout/stderr:** command payloads on stdout only; redacted diagnostics and text-mode `sce sync` progress on stderr, while JSON sync remains silent (see `context/sce/cli-stdout-stderr-contract.md`). - **Observability:** config-resolved logging with tracing, explicit config-file/default `log_to_file` control, error-specific stderr suppression when file logging is enabled, and optional dated/session-partitioned `log_dir` / `SCE_LOG_DIR` files with retention (see `context/sce/cli-observability-contract.md`). -- **Config precedence:** `flags > env > config file > defaults` (see `context/cli/config-precedence-contract.md`); the config-file-only `agent_trace.auto_sync` setting defaults to `true` and is resolved with source metadata for the post-commit trigger and doctor readiness boundaries. Its asynchronous post-commit behavior and doctor capability reporting are documented in `context/cli/agent-trace-auto-sync.md`. +- **Config precedence:** `flags > env > config file > defaults` (see `context/cli/config-precedence-contract.md`); invalid default-discovered config remains degradable for ordinary startup, while setup and Agent Trace storage fail closed before side effects or fallback identity selection. The config-file-only `agent_trace.auto_sync` setting defaults to `true` and is resolved with source metadata for the post-commit trigger and doctor readiness boundaries. Its asynchronous post-commit behavior and doctor capability reporting are documented in `context/cli/agent-trace-auto-sync.md`. - **Attribution hooks:** enabled by default, gated by staged-diff AI-overlap preflight; `SCE_ATTRIBUTION_HOOKS_DISABLED` opt-out (see `context/sce/agent-trace-commit-msg-coauthor-policy.md`). - **Install channels:** repo-flake Nix, Cargo, npm, and source-built Flatpak (`dev.crocoder.sce`); Homebrew deferred (see `context/sce/cli-first-install-channels-contract.md`). diff --git a/context/patterns.md b/context/patterns.md index 72ccff40..9ad112fd 100644 --- a/context/patterns.md +++ b/context/patterns.md @@ -131,6 +131,7 @@ - For default-enabled automatic Agent Trace synchronization, keep the post-commit launcher as a one-shot reuse of `sce sync`: start the current executable after local Agent Trace persistence, use the repository root and null child streams, do not wait, and fail open on launcher errors. Keep manual sync and the control-plane cursor authority as the retry path; do not add daemons, watchers, polling, queues, or high-frequency hook triggers. - For commands that support text/JSON dual output, centralize `--format ` parsing in one shared contract and pass command-specific `--help` guidance into invalid-value errors instead of duplicating parser logic per command. - For setup-style command contracts, keep interactive mode as the zero-flag default and enforce mutually-exclusive explicit target flags for non-interactive automation. +- For setup config safety, validate an existing repo-local config immediately after resolving the Git root and before prompts, context bootstrap, lifecycle providers, hooks, or target assets; preserve create-if-missing behavior for absent config and keep general startup degradation scoped away from setup and Agent Trace storage identity resolution. - For durable-context bootstrap, keep create-if-missing additive semantics: ensure baseline paths on every successful setup path, offer a dedicated standalone `--bootstrap-context` mode, and never overwrite existing context content. - For security-sensitive CLI UX, redact common secret-bearing token/value forms before emitting diagnostics/log lines, including app-level errors, setup git stderr diagnostics, and observability sink output. - For user-supplied setup repository paths (`sce setup --hooks --repo `), canonicalize/validate the path as an existing directory before git command execution, and run deterministic write-permission probes on setup write targets before staging/swap operations. diff --git a/context/plans/setup-invalid-config-and-git-locale.md b/context/plans/setup-invalid-config-and-git-locale.md new file mode 100644 index 00000000..609eab38 --- /dev/null +++ b/context/plans/setup-invalid-config-and-git-locale.md @@ -0,0 +1,196 @@ +# Plan: setup-invalid-config-and-git-locale + +## Change summary + +Make repository setup fail closed when the repo-local `.sce/config.json` is +invalid, before lifecycle setup can initialize databases, install hooks, or +install target assets. The Agent Trace storage runtime resolver must not silently +discard an invalid discovered config layer and substitute a default remote, +because that can select the wrong repository database. + +Also make Git subprocess behavior locale-stable by setting `LC_ALL=C` on the +Git commands used by setup and repository-identity resolution, preserving the +existing output/error handling while preventing localized Git diagnostics from +breaking parsing and classification. + +## Acceptance criteria + +How this plan is proven complete. Each criterion is observable and names the +check that proves it. `/validate` runs these checks; no task in the stack +performs final validation. + +- [x] AC1: An existing invalid repo-local `.sce/config.json` makes `sce setup` + fail before lifecycle database initialization, hook installation, or target + asset installation, and the diagnostic identifies the invalid config. + - Validate: Focused setup/config lifecycle tests assert the error and that no + setup-owned DB, hook, or target asset is created. +- [x] AC2: `resolve_agent_trace_storage_runtime_config()` returns an error when a + discovered config file is invalid instead of returning a fallback remote or + other storage identity values from the remaining layers/defaults. + - Validate: Resolver tests cover invalid local config with a configured + remote and assert the resolver fails without producing storage config. +- [x] AC3: Git subprocesses used by setup and repository-identity remote lookup + run with `LC_ALL=C`, while successful output and existing diagnostics remain + unchanged. + - Validate: Source-level inspection of the centralized Git command paths plus + the focused setup/repository-identity test suites. + +### Full validation + +Repository-wide checks `/validate` runs after the last task, regardless of +which criterion they map to. + +- `nix run .#pkl-check-generated` +- `nix flake check` + +### Context sync + +- `context/cli/config-precedence-contract.md` +- `context/sce/setup-repo-local-config-bootstrap.md` +- `context/cli/repository-identity.md` +- `context/cli/agent-trace-storage.md` + +## Task context synchronization lifecycle + +Persist this field in every plan; this is durable plan state, not chat state: + +- **Task context synchronization:** every task carries `pending | synced | blocked`. + A completed task must be `synced` before another task can start or the plan can + finish. +- For `blocked`, record **Blocker**, **Required action**, and **Retry condition** + beside the status. Never infer `synced` from conversation history; write every + lifecycle transition to the plan file. + +## Constraints and non-goals + +- **In scope:** Runtime config resolution for Agent Trace storage, setup's + preflight ordering, repository-identity Git remote lookup, setup/repository- + identity tests, and the four listed durable context files. +- **Out of scope:** Changing the general startup policy that lets `sce` continue + with degraded defaults for invalid discovered config, changing config schema + semantics, changing repository identity canonicalization, or changing Git + remote selection precedence. +- **Constraints:** Preserve credential-safe diagnostics, deterministic error + text, existing setup ordering after the new preflight, and the repository's + Nix-based verification workflow. Use the existing config validation seam and + avoid adding dependencies. +- **Non-goal:** Do not make unrelated commands or observability-only config + consumers fail hard merely because a default-discovered config is invalid; + the fail-closed boundary is setup and Agent Trace storage identity resolution. + +## Assumptions + +- The invalid-config preflight should run immediately after the Git repository + root is resolved, before prompts, context/lifecycle setup, or integration + writes, while an absent local config continues through the existing bootstrap + path. +- Applying `LC_ALL=C` to the shared setup Git runner and the repository-identity + remote lookup is sufficient; test-only Git setup helpers need no behavioral + contract change. + +## Task stack + +- [x] T01: `Fail closed on invalid repository config before setup and storage identity resolution` (status:complete) + - Task ID: T01 + - Scope: In — add a setup preflight using the existing config validation + service, make Agent Trace storage runtime resolution reject invalid + discovered config layers, and add focused tests proving no later setup work + or fallback remote occurs. Out — general startup degradation behavior and + config schema changes. + - Dependencies: none + - Done when: Invalid `.sce/config.json` stops setup before lifecycle or asset + side effects, and storage runtime config reports the validation failure + instead of returning a potentially wrong remote; valid and absent config + behavior remains unchanged. + - Verify: Focused Rust tests for config resolver, setup, and Agent Trace + storage/config lifecycle behavior. + - Completed: 2026-08-26 + - Files changed: + - `cli/src/services/config/resolver.rs` + - `cli/src/services/setup/command.rs` + - `cli/src/services/setup/mod.rs` + - Result: Added a repo-local config preflight immediately after Git-root + resolution, preventing setup prompts, context bootstrap, lifecycle work, + hooks, and target asset installation when the existing config is invalid. + Agent Trace storage runtime resolution now rejects invalid discovered config + layers instead of using fallback storage identity values. + - Verify: + - Passed: `services::config::resolver` Rust tests (19 passed), including + invalid discovered config storage resolution coverage. + - Passed: `services::setup` Rust tests (62 passed), including setup + preflight side-effect coverage. + - Passed: `services::agent_trace_storage` Rust tests (14 passed). + - Context impact: + - Classification: material + - Affected areas: setup lifecycle ordering, runtime config resolution, and + Agent Trace repository storage identity. + - Reason: Invalid repo-local configuration now changes the fail-closed + boundary for setup and Agent Trace storage consumers. + - Context synchronization: synced + +- [x] T02: `Pin setup and repository remote Git commands to the C locale` (status:complete) + - Task ID: T02 + - Scope: In — set `LC_ALL=C` on the shared setup Git command runner and + repository-identity remote lookup, and cover the affected command paths in + focused tests or inspection. Out — changing Git arguments, remote + precedence, canonicalization rules, or unrelated non-Git subprocesses. + - Dependencies: T01 + - Done when: Every production Git subprocess in the affected setup and remote + lookup paths explicitly sets `LC_ALL` to `C`, and existing success/error + behavior remains stable. + - Verify: Focused setup/repository-identity tests and source inspection of the + affected `Command::new("git")` call sites. + - Completed: 2026-08-26 + - Files changed: + - `cli/src/services/repository_identity/resolve.rs` + - `cli/src/services/setup/mod.rs` + - Result: Pinned the production setup Git runner and repository-identity remote + lookup to `LC_ALL=C`, preserving their existing arguments, output handling, + and diagnostics. + - Verify: + - Passed: setup Rust tests (65 passed). + - Passed: repository-identity Rust tests (24 passed). + - Passed: Source inspection confirmed the affected production Git runners + explicitly set `LC_ALL` to `C`; test-only Git initialization helpers were + left unchanged. + - Context impact: + - Classification: material + - Affected areas: setup Git repository/hooks resolution and repository-identity + remote lookup. + - Reason: Production Git parsing and diagnostics in these repository-wide + identity/setup paths are now explicitly locale-stable. + - Context synchronization: synced + +## Open questions + +None. The requested fail-closed boundary, fallback-remote risk, and locale +requirement are specific enough to implement using existing seams. + +## Validation Report + +**Status:** validated +**Date:** 2026-08-26 + +### Commands run + +- `nix run .#pkl-check-generated` -> exit 0 (ephemeral Pkl generation passed for 141 files) +- `nix flake check` -> exit 0 (all flake checks passed) +- `nix develop -c sh -c './scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::config::resolver && ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::setup && ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::agent_trace_storage && ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml services::repository_identity'` -> exit 0 (19 resolver, 62 setup, 14 Agent Trace storage, and 23 repository-identity tests passed) +- Manual `sce setup --claude --non-interactive --hooks --repo ` -> exit 4 (invalid repo-local config reported; no context, hook, Claude assets, or local DB were created) +- Manual `sce sync --format json` in a repo with invalid config and `agent_trace.repository_remote=upstream` -> exit 4 (storage resolution rejected the invalid discovered config; no remote fallback or storage directory was produced) +- Manual setup with absent repo-local config and a valid `origin` remote -> exit 0 (bootstrap, repository-scoped storage, hooks, and Claude assets completed) +- Manual setup with valid `agent_trace.repository_remote=upstream` and distinct `origin`/`upstream` remotes -> exit 0 (the configured `upstream` remote was selected) + +### Success-criteria verification + +- [x] AC1: An existing invalid repo-local `.sce/config.json` makes `sce setup` fail before lifecycle database initialization, hook installation, or target asset installation, and the diagnostic identifies the invalid config. -> Focused setup/config suites passed; manual setup exited 4 before creating context, hooks, Claude assets, or the local DB. Absent-config setup still completed successfully. +- [x] AC2: `resolve_agent_trace_storage_runtime_config()` returns an error when a discovered config file is invalid instead of returning a fallback remote or other storage identity values from the remaining layers/defaults. -> Resolver and Agent Trace storage suites passed; manual sync exited 4 for invalid config with `upstream`, without selecting a fallback remote or creating storage. +- [x] AC3: Git subprocesses used by setup and repository-identity remote lookup run with `LC_ALL=C`, while successful output and existing diagnostics remain unchanged. -> Source inspection confirmed `LC_ALL=C` on the production setup Git runner and repository-identity remote lookup; test-only Git helpers are excluded. Focused setup and repository-identity suites passed, and valid remote selection remained unchanged. + +### Failed checks and follow-ups + +- None. + +### Residual risks + +- None identified. diff --git a/context/sce/setup-githooks-install-flow.md b/context/sce/setup-githooks-install-flow.md index ff1cbcd9..5b1dee00 100644 --- a/context/sce/setup-githooks-install-flow.md +++ b/context/sce/setup-githooks-install-flow.md @@ -22,6 +22,9 @@ For the provided repository path, setup resolves git truth before any writes: 1. `git rev-parse --show-toplevel` 2. `git rev-parse --git-path hooks` +The production Git runner sets `LC_ALL=C` for both commands so repository-path +parsing and diagnostics remain locale-stable. + Before those git operations, setup canonicalizes/validates the user-provided repository path (`--repo`) as an existing directory. If the hooks path is relative, it is resolved against the git toplevel. @@ -64,4 +67,4 @@ On swap failure, setup removes the staging artifact and returns deterministic re - a legacy pre-marker SCE hook upgrading to the current canonical marker form - a foreign hook ending in a zero-indent `exec` installing the block and reporting `unreachable_block_advisory`, with a sibling foreign hook ending in an ordinary command not reporting it -`cli/src/services/setup/hook_merge.rs` unit-tests the pure merge computation itself (filesystem-free); the tests above verify the install-time wiring around it. \ No newline at end of file +`cli/src/services/setup/hook_merge.rs` unit-tests the pure merge computation itself (filesystem-free); the tests above verify the install-time wiring around it. diff --git a/context/sce/setup-repo-local-config-bootstrap.md b/context/sce/setup-repo-local-config-bootstrap.md index 167cfe95..e190fa4e 100644 --- a/context/sce/setup-repo-local-config-bootstrap.md +++ b/context/sce/setup-repo-local-config-bootstrap.md @@ -11,13 +11,14 @@ Task `setup-repo-gate-and-local-config-bootstrap` T02, `turso-local-db-sync` T04 - If `.sce/config.json` already exists, the bootstrap step returns `Ok(())` immediately and leaves the file untouched — no merge, no reformat, no overwrite. - The parent `.sce/` directory is created via `fs::create_dir_all` if missing. - The setup flow also bootstraps the canonical local DB through `LocalDbLifecycle::setup` and the Agent Trace DB through `AgentTraceDbLifecycle::setup`; both use the shared `TursoDb` adapter. -- Config/DB bootstrap runs after both repository preflights (`ensure_git_repository` and the effective named-remote URL check) and after context baseline bootstrap, and before config/hooks dispatch, so it applies to all normal setup modes: config-only, hooks-only, combined, and interactive. +- After both repository preflights (`ensure_git_repository` and the effective named-remote URL check), setup validates an existing repo-local `.sce/config.json` before prompts, context baseline bootstrap, lifecycle providers, hooks, or target assets. Invalid config stops the run without those side effects; an absent config continues through the normal bootstrap path. +- Config/DB bootstrap runs after those preflights and config validation, and after context baseline bootstrap, before config/hooks dispatch, so it applies to all normal setup modes: config-only, hooks-only, combined, and interactive. ## Context baseline bootstrap - `sce setup --bootstrap-context` is a non-interactive context-only mode and must be used alone (no target, hooks, non-interactive, or `--repo` flags). -- Context-only setup ensures both repository preflights, then creates the baseline durable-context tree and exits without lifecycle providers, integration installs, or prompts. -- Every normal successful setup path also calls the same additive context bootstrap after both preflights and before lifecycle/config install work. +- Context-only setup ensures both repository preflights, validates an existing repo-local config, then creates the baseline durable-context tree and exits without lifecycle providers, integration installs, or prompts. +- Every normal successful setup path also calls the same additive context bootstrap after both repository preflights and config validation, before lifecycle/config install work. - Baseline paths: `context/overview.md`, `context/architecture.md`, `context/patterns.md`, `context/glossary.md`, `context/context-map.md`, `context/plans/`, `context/handovers/`, `context/decisions/`, `context/tmp/`, and `context/tmp/.gitignore`. - Create-if-missing only: existing files and directory contents are left untouched; missing individual paths are restored even when `context/` already exists. - New Markdown files use neutral headings/placeholders; `context-map.md` links baseline entry points without inventing repository details; `context/tmp/.gitignore` ignores scratch content while retaining itself (`*\n!.gitignore\n`). @@ -53,11 +54,14 @@ The same write also records the run's resolved optional-workflow selection under - `cli/src/services/agent_trace_db/lifecycle.rs` implements `AgentTraceDbLifecycle::setup()` for Agent Trace DB initialization. - Repo-local config bootstrap uses `RepoPaths::sce_config_file()` and `RepoPaths::sce_dir()`; context baseline bootstrap uses the shared context accessors including `RepoPaths::context_tmp_gitignore_file()`. - The canonical payload constant is `REPO_LOCAL_CONFIG_BOOTSTRAP_PAYLOAD`. -- `cli/src/services/setup/command.rs` resolves the effective `agent_trace.repository_remote` and runs both repository preflights before `bootstrap_context_baseline`. Context-only requests return after the baseline. Normal modes then derive a repo-root-scoped `AppContext` and aggregate lifecycle providers in config → local_db → auth_db → agent_trace_db → hooks order; `ConfigLifecycle::setup()` calls `bootstrap_repo_local_config(...)`, `LocalDbLifecycle::setup()` initializes the local DB, `AuthDbLifecycle::setup()` initializes the auth DB, and `AgentTraceDbLifecycle::setup()` initializes the Agent Trace DB. +- `cli/src/services/setup/command.rs` resolves the effective `agent_trace.repository_remote`, runs both repository preflights, and validates an existing repo-local config before `bootstrap_context_baseline`. Context-only requests return after the baseline. Normal modes then derive a repo-root-scoped `AppContext` and aggregate lifecycle providers in config → local_db → auth_db → agent_trace_db → hooks order; `ConfigLifecycle::setup()` calls `bootstrap_repo_local_config(...)`, `LocalDbLifecycle::setup()` initializes the local DB, `AuthDbLifecycle::setup()` initializes the auth DB, and `AgentTraceDbLifecycle::setup()` initializes the Agent Trace DB. ## Relationship to other setup contracts -- The Git-repo gate (`ensure_git_repository`) and effective named-remote URL preflight remain the preconditions for every setup write path, including context-only bootstrap. The gate classifies only an explicit Git `not a git repository` result and an actually missing/empty named-remote URL as typed user errors; Git/process/configuration and remote-lookup execution failures remain runtime errors with technical sources preserved. +- The Git-repo gate (`ensure_git_repository`), effective named-remote URL preflight, and existing-config validation remain the preconditions for every setup write path, including context-only bootstrap. The gate classifies only an explicit Git `not a git repository` result and an actually missing/empty named-remote URL as typed user errors; Git/process/configuration and remote-lookup execution failures remain runtime errors with technical sources preserved. +- The repo-local config preflight is fail-closed only for an existing invalid config, while absent config remains create-if-missing. - Context baseline bootstrap is independent of config/DB/hooks install and runs before those steps on normal setup paths. - Local bootstrap (repo config + local DB init) is independent of config install and hook install; it runs before both after context baseline bootstrap. - The bootstrap payload matches the `$schema` declaration accepted by startup config loading and the Pkl-authored JSON Schema embedded from Cargo `OUT_DIR`. + +See also [the fail-closed boundary decision](../decisions/2026-08-26-setup-storage-fail-closed-on-invalid-config.md). From a32b9257dd4766c9e64579f92752f9fc3ddfa953 Mon Sep 17 00:00:00 2001 From: Ivan Ivic Date: Wed, 26 Aug 2026 15:11:49 +0200 Subject: [PATCH 4/5] setup: Preserve configured remote names in missing-remote diagnostics Identify the effective missing remote in user-facing setup guidance while keeping remote URLs out of diagnostics. Carry the remote name through the typed error and update the documented CLI contracts. Co-authored-by: SCE --- cli/src/services/app_support.rs | 4 +- cli/src/services/error.rs | 24 ++++++---- cli/src/services/setup/command.rs | 9 +++- cli/src/services/setup/mod.rs | 1 - context/cli/cli-command-surface.md | 4 +- context/overview.md | 4 +- context/plans/setup-git-remote-preflight.md | 51 ++++++++++++++++----- context/sce/cli-error-code-taxonomy.md | 6 +-- context/sce/setup-githooks-cli-ux.md | 2 +- 9 files changed, 71 insertions(+), 34 deletions(-) diff --git a/cli/src/services/app_support.rs b/cli/src/services/app_support.rs index 14671d7e..0f26d6ee 100644 --- a/cli/src/services/app_support.rs +++ b/cli/src/services/app_support.rs @@ -184,7 +184,7 @@ fn write_error_diagnostic_with_color_policy( } CliError::User { error: user_error, .. - } => user_error.message().to_string(), + } => user_error.message(), }; let styled_message = services::style::error_text_with_color_policy( &services::security::redact_sensitive_text(&rendered), @@ -328,7 +328,7 @@ mod tests { let rendered = String::from_utf8(stderr).expect("stderr is valid utf8"); let redacted_message = - services::security::redact_sensitive_text(UserError::NotAuthenticated.message()); + services::security::redact_sensitive_text(&UserError::NotAuthenticated.message()); assert!(rendered.contains(&redacted_message)); } diff --git a/cli/src/services/error.rs b/cli/src/services/error.rs index b9238d7c..cc46b3eb 100644 --- a/cli/src/services/error.rs +++ b/cli/src/services/error.rs @@ -50,44 +50,48 @@ impl FailureClass { /// Catalog of expected, deliberately-explained failures presented to the user /// as a friendly diagnostic instead of a technical error chain. -#[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Eq, PartialEq)] #[allow(clippy::enum_variant_names)] pub enum UserError { #[allow(dead_code)] NotAuthenticated, NotGitRepository, - NotGitRemote, + NotGitRemote { + remote_name: String, + }, } impl UserError { - pub fn class(self) -> FailureClass { + pub fn class(&self) -> FailureClass { match self { - Self::NotAuthenticated | Self::NotGitRepository | Self::NotGitRemote => { + Self::NotAuthenticated | Self::NotGitRepository | Self::NotGitRemote { .. } => { FailureClass::Runtime } } } #[allow(dead_code)] - pub fn key(self) -> &'static str { + pub fn key(&self) -> &'static str { match self { Self::NotAuthenticated => "auth.not_authenticated", Self::NotGitRepository => "setup.not_git_repository", - Self::NotGitRemote => "setup.not_git_remote", + Self::NotGitRemote { .. } => "setup.not_git_remote", } } - pub fn message(self) -> &'static str { + pub fn message(&self) -> String { match self { Self::NotAuthenticated => { "You are not logged in. Please log in using the `sce auth login` command." + .to_string() } Self::NotGitRepository => { "The target directory is not a Git repository. Please run `git init`, then retry." + .to_string() } - Self::NotGitRemote => { - "The Git repository has no configured remote URL. Please run `git remote add `, then retry." - } + Self::NotGitRemote { remote_name } => format!( + "The Git repository has no configured URL for remote '{remote_name}'. Please run `git remote add {remote_name} `, then retry." + ), } } } diff --git a/cli/src/services/setup/command.rs b/cli/src/services/setup/command.rs index a37d7c29..8fe6a616 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 = resolve_setup_repository(&setup_start_path)?; - setup::validate_existing_repo_local_config(&repository_root).map_err(CliError::runtime)? + setup::validate_existing_repo_local_config(&repository_root).map_err(CliError::runtime)?; let setup_dispatch = if self.request.context_only { None @@ -116,7 +116,12 @@ fn resolve_setup_repository(start_path: &std::path::Path) -> Result Result<()> ) }) } -} /// Bootstraps the repo-local `.sce/config.json` file if it does not already exist. /// diff --git a/context/cli/cli-command-surface.md b/context/cli/cli-command-surface.md index 1108fb94..cd560099 100644 --- a/context/cli/cli-command-surface.md +++ b/context/cli/cli-command-surface.md @@ -56,7 +56,7 @@ Deferred or gated command surfaces currently avoid claiming unimplemented behavi `config` exposes deterministic inspect/validate entrypoints (`sce config show`, `sce config validate`) with explicit precedence (`flags > env > config file > defaults`), a shared auth-runtime resolver for supported keys that declare env/config/optional baked-default inputs starting with `workos_client_id`, first-class `policies.bash` reporting for preset/custom blocked-command rules, and deterministic text/JSON output modes where `show` reports resolved values with provenance while `validate` reports pass/fail plus validation issues and warnings only. `version` exposes deterministic runtime identification output in text mode by default and JSON mode via `--format json`. `completion` exposes deterministic shell completion generation via `sce completion --shell `. -`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. Every setup mode first validates the Git repository and effective `agent_trace.repository_remote` (default `origin`) before prompts or writes. `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 ensures that baseline only after both preflights. +`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. Every setup mode first validates the Git repository and effective `agent_trace.repository_remote` (default `origin`) before prompts or writes. If that configured remote is missing, the typed `NotGitRemote { remote_name }` diagnostic identifies the effective name in its explanation and `git remote add` remediation without exposing the URL. `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 ensures that baseline only after both preflights. `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`. @@ -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 and runs the Git-root plus effective named-remote preflights before prompts or writes, mapping failures to typed `UserError` values with preserved technical sources. After both gates, 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; `cli/src/services/setup/command.rs` owns the setup runtime command handler and runs the Git-root plus effective named-remote preflights before prompts or writes, mapping failures to typed `UserError` values with preserved technical sources. Missing configured remotes become `UserError::NotGitRemote { remote_name }`, retaining the resolved remote name for safe operator guidance. After both gates, 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/overview.md b/context/overview.md index 8452b848..1c8fcb6c 100644 --- a/context/overview.md +++ b/context/overview.md @@ -20,7 +20,7 @@ The CLI crate currently depends on `anyhow`, `chrono`, `clap`, `clap_complete`, Its command loop is implemented with `clap` derive-based argument parsing and `anyhow` error handling. Top-level help displays an ASCII art "SCE" banner with a per-column right-to-left color gradient (cyan to magenta when color is enabled, plain ASCII when disabled) above a slim command list without implemented/placeholder labels; `auth` is visible while `hooks` and `policy` remain directly invocable but hidden. The real top-level command catalog/help-visibility contract is centralized in `cli/src/cli_schema.rs` and consumed by `cli/src/command_surface.rs` for custom banner/help rendering plus known-command classification. The runtime includes implemented auth flows (`auth login|logout|whoami`), with authenticated whoami reading the Control Plane `GET /me` profile and rendering flat email/name/role/permissions/organization labels, optional names and missing values handled deterministically, and exact login guidance returned when logged out, alongside config inspection/validation, setup orchestration, doctor diagnosis/repair, attribution-only hooks, shell completion, and the top-level `sync` command. Parse-time command conversion plus run-time command handling flow through the internal `RuntimeCommand` seam in `cli/src/app.rs`. The current doctor presentation contract supersedes the earlier output-shape scaffolding wording above: human text uses the compact Environment/Repository/Integrations hierarchy with healthy rows collapsed and unhealthy branches expanded, while JSON retains complete path, identity, problem, and fix-result detail. See `context/sce/doctor-human-text-contract.md`. The command loop now enforces a stable exit-code contract in `cli/src/app.rs`: `2` parse failures, `3` invocation validation failures, `4` runtime failures, and `5` dependency startup failures. -The same runtime also emits stable user-facing stderr error classes (`SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, `SCE-ERR-DEPENDENCY`) using deterministic `Error []: ...` diagnostics with class-default `Try:` remediation appended when missing. The command boundary's former flat, string-only `ClassifiedError` has been replaced by typed `CliError` in `cli/src/services/error.rs`: `CliError::User` carries a closed `UserError` catalog (`NotAuthenticated`, `NotGitRepository`, and `NotGitRemote`) for expected, deliberately-explained failures rendered as fixed friendly sentences 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`. Setup preflight errors preserve technical sources for observability while keeping raw remote URLs out of user-facing diagnostics. See `context/sce/cli-error-code-taxonomy.md` for the full contract. +The same runtime also emits stable user-facing stderr error classes (`SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, `SCE-ERR-DEPENDENCY`) using deterministic `Error []: ...` diagnostics with class-default `Try:` remediation appended when missing. The command boundary's former flat, string-only `ClassifiedError` has been replaced by typed `CliError` in `cli/src/services/error.rs`: `CliError::User` carries a closed `UserError` catalog (`NotAuthenticated`, `NotGitRepository`, and payload-bearing `NotGitRemote { remote_name }`) for expected, deliberately-explained failures rendered without a `Try:` suffix, while `CliError::Internal` carries a live `anyhow::Error` source rendered as the real error chain with class-default remediation; `app_support` is the sole owner of turning either into the final styled stderr diagnostic, and `sce sync` is the first command to classify a failure (authentication) into `CliError::User`. Setup preflight errors preserve technical sources for observability, identify the configured missing remote by name, and keep raw remote URLs out of user-facing diagnostics. See `context/sce/cli-error-code-taxonomy.md` for the full contract. The 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. @@ -37,7 +37,7 @@ The shared default path service in`cli/src/services/default_paths.rs`is now the The Rust CLI also centralizes SCE-owned web URI construction in`cli/src/services/agent_trace.rs`, with `SCE_WEB_BASE_URL`as the single Rust owner for`https://sce.crocoder.dev` and helpers consumed by Agent Trace conversation URLs, Agent Trace persisted trace URLs, Agent Trace session URLs, and setup-created repo-local config schema URLs. The config resolver separately owns `control_plane_base_url` and its `https://sce.crocoderlab.dev` baked sync default; the two URL owners must not be conflated. The current user-facing synchronization entrypoint is `sce sync`; references to the former nested spelling in historical records do not describe an available command. -Setup repository preflight: every `sce setup` mode, including `--bootstrap-context`, validates an initialized Git repository and the configured `agent_trace.repository_remote` URL (default `origin`) before prompts, context/bootstrap, lifecycle setup, or integration writes. `services/setup/command.rs` maps only Git's explicit missing-repository result and an actually missing configured remote URL to typed `NotGitRepository` and `NotGitRemote` diagnostics; Git launch, permission, bare/malformed-repository, and remote-lookup execution failures remain runtime errors with preserved technical sources, without rendering remote URLs. +Setup repository preflight: every `sce setup` mode, including `--bootstrap-context`, validates an initialized Git repository and the configured `agent_trace.repository_remote` URL (default `origin`) before prompts, context/bootstrap, lifecycle setup, or integration writes. `services/setup/command.rs` maps only Git's explicit missing-repository result and an actually missing configured remote URL to typed `NotGitRepository` and `NotGitRemote { remote_name }` diagnostics; the latter names the effective configured remote in both the explanation and `git remote add` remediation. Git launch, permission, bare/malformed-repository, and remote-lookup execution failures remain runtime errors with preserved technical sources, without rendering remote URLs. Sync owns the complete progress boundary in `cli/src/services/sync/progress.rs`: the consumer-typed `ProgressReporter` contract, no-op reporter, focused contract tests, and fixed `indicatif` terminal adapter. `SyncProgressEvent` remains owned by `cli/src/services/sync/sync.rs`; `sync/command.rs` selects the adapter or no-op implementation by output format, there is no top-level `cli/src/services/progress/` module, and JSON callers use the sync-owned no-op reporter. The same config resolver now also owns the attribution-hooks gate used by local hook runtime: opt-out env `SCE_ATTRIBUTION_HOOKS_DISABLED` overrides `policies.attribution_hooks.enabled` with inverted semantics, and the gate defaults to enabled unless explicitly disabled. The config service split now includes `cli/src/services/config/resolver.rs` as the focused owner for config-file discovery, file-layer merging, env/flag/default precedence, auth-key resolution, observability resolution, attribution-hooks resolution, and default-discovered invalid-file degradation; `cli/src/services/config/mod.rs` remains the facade/rendering orchestration surface while preserving existing `services::config` imports. diff --git a/context/plans/setup-git-remote-preflight.md b/context/plans/setup-git-remote-preflight.md index 08097373..6c63d22d 100644 --- a/context/plans/setup-git-remote-preflight.md +++ b/context/plans/setup-git-remote-preflight.md @@ -23,6 +23,12 @@ classification so only Git's explicit `not a git repository` failure becomes repository, and remote-lookup execution failures must remain runtime errors with their technical sources intact. +Revision requested after the completed preflight: make `NotGitRemote` retain the +configured remote name so the user-facing diagnostic identifies the actual +missing remote (for example, `upstream`) and gives matching `git remote add` +guidance. The existing source-preservation, URL-safety, and narrow +classification rules remain unchanged. + ## Acceptance criteria How this plan is proven complete. Each criterion is observable and names the @@ -58,6 +64,14 @@ performs final validation. - Validate: Focused setup and repository-identity tests cover a missing Git repository, missing remote URL, Git launch/non-repository edge failures, and remote lookup execution failures with exact `CliError` classification. +- [x] AC6: `UserError::NotGitRemote` retains the configured remote name and its + rendered diagnostic identifies that name in both the missing-URL explanation + and the `git remote add ` remediation, without exposing any remote + URL. + - Validate: Error and setup tests construct/render the `upstream` case, + assert the structured variant carries `remote_name: "upstream"`, assert the + message names `upstream` in both places, and assert a credential-bearing URL + is absent from the diagnostic. ### Full validation @@ -116,9 +130,10 @@ Persist this field in every plan; this is durable plan state, not chat state: - An explicit `agent_trace.repository_id` does not waive the remote preflight; the requested setup contract requires a Git remote independently of identity fallback behavior. -- `UserError` messages remain fixed catalog sentences; the configured remote - name is retained in the technical source for diagnostics/tests rather than - being added as a payload field to the enum. +- The earlier assumption that `UserError` messages remain remote-agnostic fixed + sentences is superseded by this revision: `NotGitRemote` carries the + configured remote name, while the URL itself remains excluded from the user + error payload and rendered diagnostic. ## Task stack @@ -161,30 +176,44 @@ Persist this field in every plan; this is durable plan state, not chat state: - Context impact: Root — setup preflight error classification and the repository-identity remote lookup boundary now distinguish expected missing prerequisites from runtime execution failures; durable root setup/error/identity contracts were updated. - Context synchronization: synced + - [x] T04: `Preserve the configured remote name in NotGitRemote` (status:done) + - Task ID: T04 + - Scope: In — change `UserError::NotGitRemote` into a payload-bearing variant, update its catalog methods and setup mapping to carry the resolved `agent_trace.repository_remote` name, assert the named diagnostic/remediation and credential-safe rendering in focused tests, and update `context/overview.md`, `context/cli/cli-command-surface.md`, `context/sce/cli-error-code-taxonomy.md`, and `context/sce/setup-githooks-cli-ux.md`. Out — remote lookup semantics, preflight ordering/classification, remote URL canonicalization, doctor behavior, and successful setup output. + - Dependencies: T03 + - Done when: A missing configured `upstream` remote renders a typed `NotGitRemote { remote_name: "upstream" }` diagnostic that names `upstream` in the no-URL explanation and in `git remote add upstream ` guidance; `NotGitRepository`, runtime classification, technical source preservation, and URL redaction remain unchanged. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml error`; `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml setup` + - Completed: 2026-08-26 + - Files changed: `cli/src/services/app_support.rs`, `cli/src/services/error.rs`, `cli/src/services/setup/command.rs`, `cli/src/services/setup/mod.rs`, `context/cli/cli-command-surface.md`, `context/overview.md`, `context/plans/setup-git-remote-preflight.md`, `context/sce/cli-error-code-taxonomy.md`, `context/sce/setup-githooks-cli-ux.md` + - Result: Made `UserError::NotGitRemote` carry the resolved remote name, rendered that name in the missing-URL explanation and matching `git remote add` remediation, and passed the configured name through setup classification. Added focused catalog and setup-source tests proving name retention, source preservation, and credential-safe rendering; updated the durable setup and error contracts. + - Verify: `error` passed with 45 tests; `setup` passed with 65 tests. + - Context impact: Root — the typed CLI error catalog and setup preflight diagnostic contract now carry the effective configured remote name; updated the root setup/error summaries and authoritative CLI setup/error UX contracts listed in the task scope. + - Context synchronization: synced + ## Open questions -None. The remote selection rule, mandatory scope, error classification, and -non-network validation boundary were resolved during discussion. +None. The remote selection rule, payload shape, mandatory scope, error +classification, and non-network validation boundary are resolved. ## Validation Report **Status:** validated -**Date:** 2026-08-25 +**Date:** 2026-08-26 ### Commands run -- `nix flake check` -> exit 0 (all repository checks passed) -- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml setup` -> exit 0 (69 tests passed) -- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml repository_identity` -> exit 0 (25 tests passed) -- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml error` -> exit 0 (46 tests passed) +- `nix flake check` -> exit 0 (all checks passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml setup` -> exit 0 (65 tests passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml repository_identity` -> exit 0 (24 tests passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml error` -> exit 0 (45 tests passed) ### Success-criteria verification - [x] AC1: `sce setup` reports a typed, actionable `NotGitRepository` failure and performs no setup writes -> setup preflight tests passed for a non-repository directory and preserved the `git init` guidance. - [x] AC2: `sce setup` reports a typed, actionable `NotGitRemote` failure and performs no setup writes -> setup and error tests passed for missing configured remotes and preserved `git remote add ` guidance. - [x] AC3: Remote validation uses the resolved `agent_trace.repository_remote` name -> setup and repository-identity tests passed for default `origin`, configured alternate remotes, and rejection of unrelated remotes. -- [x] AC4: Existing setup behavior and user-error rendering remain compatible without exposing remote URLs -> full repository checks and focused setup/error tests passed, including source preservation and credential-safe diagnostics. +- [x] AC4: Existing setup behavior and user-error rendering remain compatible, while technical error sources remain available to observability and remote URLs are not echoed in diagnostics -> focused setup and error tests passed, and the full repository check passed. - [x] AC5: Setup classifies only explicit missing-repository and missing-remote conditions as typed user errors -> setup, repository-identity, and error tests passed for missing prerequisites, Git/runtime edge failures, strict remote lookup failures, preserved sources, and safe diagnostics. +- [x] AC6: `UserError::NotGitRemote` retains the configured remote name and renders credential-safe named guidance -> error and setup tests passed for `upstream`, including structured name retention, both diagnostic mentions, and URL redaction. ### Failed checks and follow-ups diff --git a/context/sce/cli-error-code-taxonomy.md b/context/sce/cli-error-code-taxonomy.md index 02f47c72..33f2329a 100644 --- a/context/sce/cli-error-code-taxonomy.md +++ b/context/sce/cli-error-code-taxonomy.md @@ -25,15 +25,15 @@ It complements the numeric process exit-code classes documented in `context/sce/ - High-frequency parse/invocation failures use explicit `Try:` remediations instead of generic usage-only hints. - Top-level unknown command/option messages include targeted retry guidance (`sce --help` and command-local `sce --help`). - Setup invocation validation failures (`--repo` without `--hooks`, mutually exclusive target flags, unexpected args) include concrete valid alternatives. -- Setup repository preflight failures use fixed `UserError::NotGitRepository` and `UserError::NotGitRemote` sentences with `git init` and `git remote add ` remediation only for Git's explicit `not a git repository` result and an actually missing/empty configured remote URL. Git launch, permission, bare/malformed-repository, configuration, and remote-lookup execution failures remain `CliError::Internal` runtime errors with their technical sources; no user-facing diagnostic echoes a remote URL. +- Setup repository preflight failures use `UserError::NotGitRepository` and payload-bearing `UserError::NotGitRemote { remote_name }` messages with `git init` and `git remote add ` remediation only for Git's explicit `not a git repository` result and an actually missing/empty configured remote URL. The configured remote name appears in the missing-URL explanation and remediation, while the URL itself is never rendered. Git launch, permission, bare/malformed-repository, configuration, and remote-lookup execution failures remain `CliError::Internal` runtime errors with their technical sources. - Hooks invocation validation failures (missing hook subcommand, missing `commit-msg` message file, unknown subcommand) include command-form examples that are copyable for retry automation. - This actionable-message normalization is owned by parser/validation paths in `cli/src/app.rs`, `cli/src/services/setup/mod.rs`, `cli/src/services/setup/command.rs`, and `cli/src/services/hooks/mod.rs`. ## Ownership - `FailureClass` in `cli/src/services/error.rs` owns class selection and stable code assignment (`FailureClass::code()`). -- `CliError::{User,Internal}` in `cli/src/services/error.rs` is the typed CLI-boundary error type; `CliError::code()`/`CliError::class()` delegate to the failure class. `CliError::User` carries a catalog `UserError` (`NotAuthenticated`, `NotGitRepository`, or `NotGitRemote`) for expected, deliberately-explained failures; `CliError::Internal` carries a live `anyhow::Error` source for every other failure, including non-classifiable setup preflight failures. `CliError::User` may also carry an optional preserved technical `source`, kept for observability only and never rendered to the terminal. -- `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`, `NotGitRepository`, or `NotGitRemote { remote_name }`) for expected, deliberately-explained failures; `CliError::Internal` carries a live `anyhow::Error` source for every other failure, including non-classifiable setup preflight failures. `CliError::User` may also carry an optional preserved technical `source`, kept for observability only and never rendered to the terminal; the named remote payload contains no URL. +- `UserError` in `cli/src/services/error.rs` is the closed catalog of deliberately presented terminal failures. It has no arbitrary-message variant (no `Message(String)`/`Custom(...)` escape hatch): every entry returns a fixed reviewed sentence or reviewed payload-derived message from `UserError::message()`, keyed for structured logging by `UserError::key()`. - Command and domain layers construct and return a `CliError`; they do not format terminal text, apply styling, or decide authentication/user-error semantics from string matching. `app_support` is the sole owner of turning a `CliError` into the final stderr sentence. - `Logger::log_cli_error` in `cli/src/services/observability.rs` owns structured error logging with `sce.error.{code}` event IDs. - `write_error_diagnostic` in `cli/src/services/app_support.rs` owns final code-bearing stderr rendering, including styling `CliError::User`'s catalog message and `CliError::Internal`'s rendered chain through `services::style::error_text_with_color_policy` under the stderr TTY/`NO_COLOR` policy (`services::style::supports_color_stderr()`), independent of stdout's TTY state. diff --git a/context/sce/setup-githooks-cli-ux.md b/context/sce/setup-githooks-cli-ux.md index a54b072f..36e0c2b4 100644 --- a/context/sce/setup-githooks-cli-ux.md +++ b/context/sce/setup-githooks-cli-ux.md @@ -26,7 +26,7 @@ Validation is deterministic and enforced during setup option resolution: - `--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` -- all `sce setup` modes (config-only, hooks-only, combined, interactive, and `--bootstrap-context`) require an initialized Git repository and a URL for the effective `agent_trace.repository_remote` before prompts or writes; `setup::command` uses the configured name, defaulting to `origin`, and reports typed `NotGitRepository` or `NotGitRemote` failures without echoing remote URLs only for the explicit missing-repository and missing-URL cases. Git/process/configuration failures remain runtime diagnostics with their technical sources preserved. +- all `sce setup` modes (config-only, hooks-only, combined, interactive, and `--bootstrap-context`) require an initialized Git repository and a URL for the effective `agent_trace.repository_remote` before prompts or writes; `setup::command` uses the configured name, defaulting to `origin`, and reports typed `NotGitRepository` or `NotGitRemote { remote_name }` failures. A missing configured remote is named in both the no-URL explanation and matching `git remote add ` guidance, while remote URLs are never echoed. These typed failures apply only to the explicit missing-repository and missing-URL cases; Git/process/configuration failures remain runtime diagnostics with their technical sources preserved. Target-install mode contract: From a1bedcbf253fbffd841834513132c86e75cebe06 Mon Sep 17 00:00:00 2001 From: Ivan Ivic Date: Wed, 26 Aug 2026 16:41:04 +0200 Subject: [PATCH 5/5] services: Fix Git remote command placeholder Use consistently in the suggested git remote add command. --- cli/src/services/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/src/services/error.rs b/cli/src/services/error.rs index cc46b3eb..cfb4e109 100644 --- a/cli/src/services/error.rs +++ b/cli/src/services/error.rs @@ -90,7 +90,7 @@ impl UserError { .to_string() } Self::NotGitRemote { remote_name } => format!( - "The Git repository has no configured URL for remote '{remote_name}'. Please run `git remote add {remote_name} `, then retry." + "The Git repository has no configured URL for remote '{remote_name}'. Please run `git remote add `, then retry." ), } }