From ac79d7c8aeec1ed392d302394bfd79b6d988d417 Mon Sep 17 00:00:00 2001 From: UnbreakableMJ Date: Sun, 9 Aug 2026 23:13:50 +0000 Subject: [PATCH] =?UTF-8?q?feat(construct-cli):=20severity-tagged=20diagno?= =?UTF-8?q?stics=20=E2=80=94=20CLI=20Standard=20v1.1.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the diagnostics spec (references/diagnostics.md, PR #35): - New output/diagnostic.rs: Severity ladder (Info < Ok < Warn < Error, §18.2.1 tags), Diagnostic type emitting single-line {"diagnostic":{...}} envelopes in machine mode and [TAG]-first human rendering, gated by the severity floor on Context (--quiet → errors only, agent env → warn+, default → ok+, --verbose → info+). Errors (AppError) never consult the floor. - error.rs human render: error:/hint: → [ERROR] tag + indented hint:, tag carried in the colorless branch too (color is never the sole carrier of meaning). - mode.rs: warn_tui_fallback deleted — resolve() now returns the fallback reason and main emits a TUI_FALLBACK warn diagnostic after Context exists (no more raw JSON on a human TTY; drops the non-spec fell_back_to field). FORCE_COLOR now checked before NO_COLOR so it overrides it (was inverted vs its own comment and the spec). - theme.rs migrated from the grandfathered v1.33 six-token palette to the eleven Steelbore 2 §11.1 role tokens (this is the "next minor release" the grandfathering clause named); render.rs and tui/mod.rs remapped per tui-explore.md §3. - sync.rs nix progress passthrough gated by the info floor; ship.rs oversized-description hint and tui/mod.rs fallback hint made paste-test-clean runnable commands. - Tests: floor table, envelope shape/single-line/hint-omission, tag presence with and without color, FORCE_COLOR-over-NO_COLOR, agent-env explore fallback parses as a warn diagnostic, --quiet suppresses the fallback but never errors. Co-Authored-By: Claude Fable 5 --- construct-cli/AGENTS.md | 20 +- construct-cli/src/commands/ship.rs | 2 +- construct-cli/src/commands/sync.rs | 10 +- construct-cli/src/context.rs | 56 ++++- construct-cli/src/main.rs | 6 + construct-cli/src/output/diagnostic.rs | 270 +++++++++++++++++++++++++ construct-cli/src/output/error.rs | 92 ++++++--- construct-cli/src/output/mod.rs | 1 + construct-cli/src/output/mode.rs | 126 ++++++++---- construct-cli/src/output/render.rs | 18 +- construct-cli/src/output/theme.rs | 67 +++--- construct-cli/src/tui/mod.rs | 28 +-- construct-cli/tests/cli.rs | 64 ++++++ 13 files changed, 625 insertions(+), 135 deletions(-) create mode 100644 construct-cli/src/output/diagnostic.rs diff --git a/construct-cli/AGENTS.md b/construct-cli/AGENTS.md index 112e2ed..eddcee6 100644 --- a/construct-cli/AGENTS.md +++ b/construct-cli/AGENTS.md @@ -3,7 +3,7 @@ `construct` is the Spacecraft Software **Construct** skills package manager (Rust CLI + TUI) — the first executable in the Construct catalogue repository. It conforms to the Spacecraft Software Dual-Mode Self-Documenting CLI Standard -(v1.0.0). This file and `CLAUDE.md` are peers; keep them identical. +(v1.1.0). This file and `CLAUDE.md` are peers; keep them identical. ## Build / test / lint @@ -32,12 +32,19 @@ so run it locally before adding a dependency (Standard §3.3). - `cli.rs` — the clap derive tree and the §3 global flags (`global = true`). - `context.rs` — per-invocation resolved state (output mode, color, flags). - `src/output/` is the **only** place that writes to stdout: - - `mode.rs` — the §5 detection cascade + §6 color precedence. + - `mode.rs` — the §5 detection cascade + §6 color precedence + (FORCE_COLOR overrides NO_COLOR). - `envelope.rs` — the `{ metadata, data }` JSON envelope. - - `error.rs` — the structured `AppError` (machine: single-line `{"error":…}`). + - `error.rs` — the structured `AppError` (machine: single-line `{"error":…}`; + human: `[ERROR]`-tagged line + indented `hint:`). Never suppressible. + - `diagnostic.rs` — non-error diagnostics (`Severity` ladder `[OK]`/`[WARN]`/ + `[INFO]`, machine: single-line `{"diagnostic":…}`, human: `[TAG]` line), + gated by `Context::severity_floor` (`--quiet` → errors only, agent env → + `warn`+, default → `ok`+, `--verbose` → `info`+). See the CLI Standard's + `references/diagnostics.md`. - `render.rs` — json / jsonl / yaml / csv / human renderers; `--fields`. - - `theme.rs` — the Steelbore palette (v1.33 tokens, grandfathered per - Standard §11.1 until the next minor release; no inline hex). + - `theme.rs` — the `steelbore` theme: the eleven Steelbore 2 role tokens of + Standard §11.1 (no inline hex). - `src/commands/` — one handler per command. - `manifest.rs` — the single source of truth for `schema` and `describe`; the `tests::manifest_in_sync_with_cli` test fails if it drifts from the clap tree. @@ -50,6 +57,9 @@ so run it locally before adding a dependency (Standard §3.3). - All timestamps go through `time::now_iso8601()` → ISO 8601 UTC with `Z`. Never local time, never `chrono::Local` / `NaiveDateTime`. - Errors are `AppError` whose `hint` is a RUNNABLE command, not prose. +- Every non-error stderr message goes through `output::diagnostic::Diagnostic` + (or `emit_passthrough` for raw subprocess output) so the severity floor and + `[TAG]` rendering apply — no bare `eprintln!` diagnostics. - Exit codes follow the canonical map (0,1,2,3,4,5,127,…). - Every `.rs` / `.toml` starts with the two-line SPDX header; license is `GPL-3.0-or-later`. diff --git a/construct-cli/src/commands/ship.rs b/construct-cli/src/commands/ship.rs index a52ce57..2cf1536 100644 --- a/construct-cli/src/commands/ship.rs +++ b/construct-cli/src/commands/ship.rs @@ -146,7 +146,7 @@ pub(crate) fn run(ctx: &Context, args: &ShipArgs) -> Result Result 0 { - let progress = String::from_utf8_lossy(&output.stderr); - if !progress.trim().is_empty() { - eprint!("{progress}"); - } - } + // nix logs progress to stderr; it is info-level passthrough, visible only + // when the severity floor admits it (`--verbose`, diagnostics.md §4). + crate::output::diagnostic::emit_passthrough(ctx, &String::from_utf8_lossy(&output.stderr)); if !output.status.success() { return Err(AppError::general( diff --git a/construct-cli/src/context.rs b/construct-cli/src/context.rs index 411b6c1..2e18cb5 100644 --- a/construct-cli/src/context.rs +++ b/construct-cli/src/context.rs @@ -7,6 +7,7 @@ //! stays consistent across the whole surface. use crate::cli::Cli; +use crate::output::diagnostic::Severity; use crate::output::mode::{self, OutputMode}; /// Resolved runtime settings for a single invocation. @@ -37,6 +38,12 @@ pub(crate) struct Context { pub(crate) yes: bool, /// `--absolute-time`: render absolute timestamps in human mode. pub(crate) absolute_time: bool, + /// The minimum severity emitted to stderr (diagnostics.md §4), resolved + /// once per invocation from `--quiet` / `--verbose` / the agent env. + pub(crate) severity_floor: Severity, + /// Why `--format explore` fell back to JSON, when it did. Emitted as a + /// `TUI_FALLBACK` warn diagnostic by `main` once the context exists. + pub(crate) tui_fallback: Option<&'static str>, } impl Context { @@ -44,7 +51,7 @@ impl Context { /// detection cascade and color precedence chain. pub(crate) fn from_cli(cli: &Cli) -> Self { let g = &cli.global; - let mode = mode::resolve(g); + let (mode, tui_fallback) = mode::resolve(g); Self { command: invocation_string(), mode, @@ -56,8 +63,31 @@ impl Context { print0: g.print0, yes: g.yes, absolute_time: g.absolute_time, + severity_floor: resolve_floor(g.quiet, g.verbose, mode::is_agent_env()), + tui_fallback, } } + + /// Whether a diagnostic of `severity` clears the floor and is emitted. + /// Errors always do — `AppError` never consults the floor. + pub(crate) fn allows(&self, severity: Severity) -> bool { + severity >= self.severity_floor + } +} + +/// Resolve the severity floor (diagnostics.md §4). Explicit flags beat the +/// environment: `--quiet` → errors only; `--verbose` → everything; a detected +/// agent env → failures and degradations (`warn`+); default → `ok`+. +fn resolve_floor(quiet: bool, verbose: u8, agent_env: bool) -> Severity { + if quiet { + Severity::Error + } else if verbose > 0 { + Severity::Info + } else if agent_env { + Severity::Warn + } else { + Severity::Ok + } } /// The full command line with `argv[0]` normalized to the canonical binary name @@ -69,3 +99,27 @@ fn invocation_string() -> String { } args.join(" ") } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn floor_table_matches_diagnostics_spec() { + // --quiet → errors only; beats the agent env. + assert_eq!(resolve_floor(true, 0, true), Severity::Error); + // --verbose → everything; beats the agent env. + assert_eq!(resolve_floor(false, 1, true), Severity::Info); + // agent env → failures and degradations. + assert_eq!(resolve_floor(false, 0, true), Severity::Warn); + // default → ok and up. + assert_eq!(resolve_floor(false, 0, false), Severity::Ok); + } + + #[test] + fn severity_ordering_backs_the_floor_comparison() { + assert!(Severity::Info < Severity::Ok); + assert!(Severity::Ok < Severity::Warn); + assert!(Severity::Warn < Severity::Error); + } +} diff --git a/construct-cli/src/main.rs b/construct-cli/src/main.rs index f27806a..c0809f4 100644 --- a/construct-cli/src/main.rs +++ b/construct-cli/src/main.rs @@ -61,6 +61,12 @@ fn real_main() -> i32 { let ctx = Context::from_cli(&cli); + // `--format explore` fell back to JSON: surface why as a warn diagnostic, + // now that the context can render it per mode and severity floor. + if let Some(reason) = ctx.tui_fallback { + output::diagnostic::emit_tui_fallback(&ctx, reason); + } + let dispatched = std::panic::catch_unwind(AssertUnwindSafe(|| commands::dispatch(&cli, &ctx))); match dispatched { diff --git a/construct-cli/src/output/diagnostic.rs b/construct-cli/src/output/diagnostic.rs new file mode 100644 index 0000000..d535de2 --- /dev/null +++ b/construct-cli/src/output/diagnostic.rs @@ -0,0 +1,270 @@ +// SPDX-FileCopyrightText: 2026 Mohamed Hammad +// SPDX-License-Identifier: GPL-3.0-or-later + +//! Non-error diagnostics (CLI Standard `references/diagnostics.md`). +//! +//! Warnings, informational notes, and success confirmations share one type: +//! machine mode emits a single-line `{"diagnostic":{severity, code, message, +//! hint?, timestamp, command, …}}` object on stderr; human mode renders the +//! severity-tagged `[WARN] message` layout with an indented `hint:` line. The +//! severity floor (`--quiet` → errors only, agent env → `warn`+, default → +//! `ok`+, `--verbose` → `info`+) gates emission. Errors are not diagnostics — +//! they are [`crate::output::error::AppError`], which is never suppressible. + +use std::io::Write as _; + +use owo_colors::OwoColorize as _; +use serde::Serialize; +use serde_json::{Map, Value}; + +use crate::context::Context; +use crate::output::theme; + +/// The severity ladder, ordered `Info < Ok < Warn < Error` so that +/// `severity >= floor` is the emission test. The tags are the Steelbore +/// Standard §18.2.1 vocabulary; color is never the sole carrier of meaning. +#[derive(Debug, Serialize, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] +#[serde(rename_all = "lowercase")] +pub(crate) enum Severity { + /// Diagnostic narration; hidden unless `--verbose`. + Info, + /// Side-effect confirmation. + Ok, + /// Degradation or fallback the caller should know about. + Warn, + /// Exists for floor comparisons only — error output goes through + /// [`crate::output::error::AppError`], never a `Diagnostic`. + Error, +} + +impl Severity { + /// The §18.2.1 text tag. Present in every human-mode line, colored or not. + pub(crate) fn tag(self) -> &'static str { + match self { + Self::Info => "[INFO]", + Self::Ok => "[OK]", + Self::Warn => "[WARN]", + Self::Error => "[ERROR]", + } + } + + /// The theme role token for the tag (diagnostics.md §5). + fn color(self) -> (u8, u8, u8) { + match self { + Self::Info => theme::STRUCTURE, + Self::Ok => theme::SUCCESS, + Self::Warn => theme::WARNING, + Self::Error => theme::ERROR, + } + } +} + +/// A non-error diagnostic bound to the invocation that produced it. +#[derive(Debug, Serialize)] +pub(crate) struct Diagnostic { + /// `"ok"`, `"warn"`, or `"info"` — never `"error"` (that is the `error` + /// envelope's job). + pub(crate) severity: Severity, + /// Stable upper-snake-case code (e.g. `TUI_FALLBACK`). + pub(crate) code: &'static str, + /// One-sentence, lowercase, period-free description. + pub(crate) message: String, + /// Optional runnable command — same contract as `error.hint`. + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) hint: Option, + /// ISO 8601 UTC time the diagnostic was produced. + pub(crate) timestamp: String, + /// The invocation that produced it. + pub(crate) command: String, + /// Extended structured fields (e.g. `reason`), flattened as siblings. + #[serde(flatten)] + pub(crate) extensions: Map, +} + +impl Diagnostic { + /// Construct a diagnostic bound to the current invocation context. + /// + /// # Panics + /// + /// Debug-asserts that `severity` is not [`Severity::Error`] — an + /// error-severity diagnostic is a programming error; use `AppError`. + pub(crate) fn new( + ctx: &Context, + severity: Severity, + code: &'static str, + message: impl Into, + ) -> Self { + debug_assert!( + severity != Severity::Error, + "error-severity output goes through AppError, not Diagnostic" + ); + Self { + severity, + code, + message: message.into(), + hint: None, + timestamp: crate::time::now_iso8601(), + command: ctx.command.clone(), + extensions: Map::new(), + } + } + + /// Attach the runnable recovery/next-step command. + pub(crate) fn with_hint(mut self, hint: impl Into) -> Self { + self.hint = Some(hint.into()); + self + } + + /// Attach an extended structured field (documented in `schema`). + pub(crate) fn with_extension(mut self, key: &str, value: Value) -> Self { + self.extensions.insert(key.to_owned(), value); + self + } + + /// The single-line machine form: `{"diagnostic":{…}}`. Compact, because + /// PowerShell fragments multi-line stderr into separate records. + pub(crate) fn machine_line(&self) -> String { + #[derive(Serialize)] + struct Wrapper<'a> { + diagnostic: &'a Diagnostic, + } + serde_json::to_string(&Wrapper { diagnostic: self }) + .unwrap_or_else(|_| String::from("{\"diagnostic\":{\"severity\":\"warn\"}}")) + } + + /// The human rendering: `[TAG] message` plus an indented `hint:` line. + /// The tag is present with and without color (§18.2.1 — color is never + /// the sole carrier of meaning). + pub(crate) fn render_human(&self, color: bool) -> String { + use std::fmt::Write as _; + + let mut out = String::new(); + if color { + let (r, g, b) = self.severity.color(); + let tag = self.severity.tag().truecolor(r, g, b).bold().to_string(); + let msg = self.message.truecolor( + theme::FOREGROUND.0, + theme::FOREGROUND.1, + theme::FOREGROUND.2, + ); + let _ = writeln!(out, "{tag} {msg}"); + if let Some(hint) = &self.hint { + let label = "hint:".truecolor(theme::ACCENT.0, theme::ACCENT.1, theme::ACCENT.2); + let text = hint.truecolor(theme::ACCENT.0, theme::ACCENT.1, theme::ACCENT.2); + let _ = writeln!(out, " {label} {text}"); + } + } else { + let _ = writeln!(out, "{} {}", self.severity.tag(), self.message); + if let Some(hint) = &self.hint { + let _ = writeln!(out, " hint: {hint}"); + } + } + out + } + + /// Emit to stderr in the form the output mode requires, gated by the + /// severity floor. A suppressed diagnostic is simply not written — never + /// downgraded or merged into stdout. + pub(crate) fn emit(&self, ctx: &Context) { + if !ctx.allows(self.severity) { + return; + } + let mut stderr = std::io::stderr(); + if ctx.mode.is_machine() { + let _ = writeln!(stderr, "{}", self.machine_line()); + } else { + let _ = write!(stderr, "{}", self.render_human(ctx.color)); + } + let _ = stderr.flush(); + } +} + +/// Emit the `--format explore` fallback as a warn diagnostic +/// (`references/tui-explore.md` §1). Replaces the deprecated pre-v1.1.0 +/// `{"warning":{…}}` shape; the hint is the working non-interactive +/// invocation the caller should use instead. +pub(crate) fn emit_tui_fallback(ctx: &Context, reason: &'static str) { + Diagnostic::new( + ctx, + Severity::Warn, + "TUI_FALLBACK", + "interactive explore mode unavailable; falling back to `--format json`", + ) + .with_hint("construct skill find --json") + .with_extension("reason", Value::String(reason.to_owned())) + .emit(ctx); +} + +/// Raw passthrough of subprocess progress output to stderr. This is +/// `info`-level output (diagnostics.md §4): visible under `--verbose`, +/// suppressed otherwise. The text is forwarded verbatim — it is opaque +/// third-party output, not a diagnostic envelope. +pub(crate) fn emit_passthrough(ctx: &Context, text: &str) { + if !ctx.allows(Severity::Info) || text.trim().is_empty() { + return; + } + let mut stderr = std::io::stderr(); + let _ = write!(stderr, "{text}"); + let _ = stderr.flush(); +} + +#[cfg(test)] +mod tests { + use super::*; + + fn sample(hint: Option<&str>) -> Diagnostic { + Diagnostic { + severity: Severity::Warn, + code: "TUI_FALLBACK", + message: "interactive explore mode unavailable; falling back to `--format json`" + .to_owned(), + hint: hint.map(str::to_owned), + timestamp: "2026-08-10T00:00:00Z".to_owned(), + command: "construct skill find --format explore".to_owned(), + extensions: Map::new(), + } + } + + #[test] + fn machine_line_is_single_line_parseable_envelope() { + let line = sample(Some("construct skill find --json")).machine_line(); + assert!(!line.contains('\n'), "must be single-line: {line}"); + let value: serde_json::Value = serde_json::from_str(&line).expect("valid JSON"); + assert_eq!(value["diagnostic"]["severity"], "warn"); + assert_eq!(value["diagnostic"]["code"], "TUI_FALLBACK"); + assert_eq!(value["diagnostic"]["hint"], "construct skill find --json"); + assert!(value["diagnostic"]["timestamp"] + .as_str() + .is_some_and(|t| t.ends_with('Z'))); + } + + #[test] + fn machine_line_omits_absent_hint() { + let value: serde_json::Value = + serde_json::from_str(&sample(None).machine_line()).expect("valid JSON"); + assert!(value["diagnostic"].get("hint").is_none()); + } + + #[test] + fn human_render_carries_tag_without_color() { + let text = sample(Some("construct skill find --json")).render_human(false); + assert!(text.starts_with("[WARN] "), "tag missing: {text}"); + assert!(text.contains("\n hint: construct skill find --json")); + assert!(!text.contains('\u{1b}'), "colorless render leaked ANSI"); + } + + #[test] + fn human_render_keeps_tag_with_color() { + let text = sample(None).render_human(true); + assert!(text.contains("[WARN]"), "tag must survive coloring: {text}"); + assert!(text.contains('\u{1b}'), "colored render carries ANSI"); + } + + #[test] + fn tags_match_the_standard_vocabulary() { + assert_eq!(Severity::Info.tag(), "[INFO]"); + assert_eq!(Severity::Ok.tag(), "[OK]"); + assert_eq!(Severity::Warn.tag(), "[WARN]"); + assert_eq!(Severity::Error.tag(), "[ERROR]"); + } +} diff --git a/construct-cli/src/output/error.rs b/construct-cli/src/output/error.rs index 00cdf3a..1ba708f 100644 --- a/construct-cli/src/output/error.rs +++ b/construct-cli/src/output/error.rs @@ -169,45 +169,43 @@ impl AppError { let _ = stderr.flush(); } - /// Render the error for a human terminal. Content matches the structured - /// form; color is applied only when enabled. - fn emit_human(&self, color: bool) { - let mut stderr = std::io::stderr(); + /// Render the error for a human terminal in the unified diagnostic layout + /// (diagnostics.md §5): the `[ERROR]` tag first — carried with and without + /// color, since color is never the sole carrier of meaning (§18.2.1) — + /// then the message in the default foreground, then the indented `hint:`. + /// Content matches the structured form. + pub(crate) fn render_human(&self, color: bool) -> String { if color { - let _ = writeln!( - stderr, - "{}: {}", - "error" - .truecolor(theme::RED_OXIDE.0, theme::RED_OXIDE.1, theme::RED_OXIDE.2) - .bold(), - self.message - .truecolor(theme::RED_OXIDE.0, theme::RED_OXIDE.1, theme::RED_OXIDE.2) - ); - let _ = writeln!( - stderr, - " {}: {}", - "hint".truecolor( - theme::MOLTEN_AMBER.0, - theme::MOLTEN_AMBER.1, - theme::MOLTEN_AMBER.2 - ), - self.hint.truecolor( - theme::MOLTEN_AMBER.0, - theme::MOLTEN_AMBER.1, - theme::MOLTEN_AMBER.2 - ) + let tag = "[ERROR]" + .truecolor(theme::ERROR.0, theme::ERROR.1, theme::ERROR.2) + .bold() + .to_string(); + let msg = self.message.truecolor( + theme::FOREGROUND.0, + theme::FOREGROUND.1, + theme::FOREGROUND.2, ); + let label = "hint:".truecolor(theme::ACCENT.0, theme::ACCENT.1, theme::ACCENT.2); + let hint = self + .hint + .truecolor(theme::ACCENT.0, theme::ACCENT.1, theme::ACCENT.2); + format!("{tag} {msg}\n {label} {hint}\n") } else { - let _ = writeln!(stderr, "error: {}", self.message); - let _ = writeln!(stderr, " hint: {}", self.hint); + format!("[ERROR] {}\n hint: {}\n", self.message, self.hint) } + } + + /// Write the human rendering to stderr. + fn emit_human(&self, color: bool) { + let mut stderr = std::io::stderr(); + let _ = write!(stderr, "{}", self.render_human(color)); let _ = stderr.flush(); } } /// Emit an error in the form appropriate to the output mode and return its exit /// code. Machine modes get the structured JSON object; human modes get the -/// colored rendering. +/// tagged rendering. Errors never consult the severity floor. pub(crate) fn report(err: &AppError, mode: OutputMode) -> i32 { if mode.is_machine() { err.emit_to_stderr(); @@ -216,3 +214,39 @@ pub(crate) fn report(err: &AppError, mode: OutputMode) -> i32 { } err.exit_code } + +#[cfg(test)] +mod tests { + use super::*; + + fn sample() -> AppError { + AppError { + code: ErrorCode::NotFound, + exit_code: 3, + message: "skill `foo` does not exist".to_owned(), + hint: "construct skill find --json".to_owned(), + timestamp: "2026-08-10T00:00:00Z".to_owned(), + command: "construct skill use foo".to_owned(), + docs_url: None, + extensions: Map::new(), + } + } + + #[test] + fn human_render_carries_error_tag_without_color() { + let text = sample().render_human(false); + assert!(text.starts_with("[ERROR] "), "tag missing: {text}"); + assert!(text.contains("\n hint: construct skill find --json")); + assert!(!text.contains('\u{1b}'), "colorless render leaked ANSI"); + } + + #[test] + fn human_render_keeps_tag_with_color() { + let text = sample().render_human(true); + assert!( + text.contains("[ERROR]"), + "tag must survive coloring: {text}" + ); + assert!(text.contains('\u{1b}'), "colored render carries ANSI"); + } +} diff --git a/construct-cli/src/output/mod.rs b/construct-cli/src/output/mod.rs index 3862321..63c506a 100644 --- a/construct-cli/src/output/mod.rs +++ b/construct-cli/src/output/mod.rs @@ -6,6 +6,7 @@ //! [`theme`], and the [`render`]er that turns a [`CommandOutput`] into bytes on //! stdout. Printing happens **only** in this module (CLI Standard §7). +pub(crate) mod diagnostic; pub(crate) mod envelope; pub(crate) mod error; pub(crate) mod mode; diff --git a/construct-cli/src/output/mode.rs b/construct-cli/src/output/mode.rs index 15c4459..a2dac4a 100644 --- a/construct-cli/src/output/mode.rs +++ b/construct-cli/src/output/mode.rs @@ -43,8 +43,12 @@ impl OutputMode { } } -/// Resolve the output mode from the global flags and environment. -pub(crate) fn resolve(g: &GlobalArgs) -> OutputMode { +/// Resolve the output mode from the global flags and environment. The second +/// element is the `--format explore` fallback reason, when the TUI could not +/// run — the caller emits it as a `TUI_FALLBACK` warn diagnostic once a full +/// `Context` exists, so it honors the output mode and severity floor instead +/// of being raw JSON on a human terminal. +pub(crate) fn resolve(g: &GlobalArgs) -> (OutputMode, Option<&'static str>) { // 1. Explicit flag (`--json` is sugar for `--format json`). let explicit = if g.json { Some(FormatArg::Json) @@ -53,21 +57,21 @@ pub(crate) fn resolve(g: &GlobalArgs) -> OutputMode { }; if let Some(fmt) = explicit { return match fmt { - FormatArg::Json => OutputMode::Json, - FormatArg::Jsonl => OutputMode::Jsonl, - FormatArg::Yaml => OutputMode::Yaml, - FormatArg::Csv => OutputMode::Csv, + FormatArg::Json => (OutputMode::Json, None), + FormatArg::Jsonl => (OutputMode::Jsonl, None), + FormatArg::Yaml => (OutputMode::Yaml, None), + FormatArg::Csv => (OutputMode::Csv, None), FormatArg::Explore => resolve_explore(), }; } // 2. Agent / CI environment. if is_agent_env() || is_ci() { - return OutputMode::Json; + return (OutputMode::Json, None); } // 3 / 4. TTY detection. - if std::io::stdout().is_terminal() { + let mode = if std::io::stdout().is_terminal() { if should_use_color(g) { OutputMode::HumanWithColor } else { @@ -75,24 +79,31 @@ pub(crate) fn resolve(g: &GlobalArgs) -> OutputMode { } } else { OutputMode::Json - } + }; + (mode, None) } /// Resolve `--format explore`. The interactive TUI runs only on a real /// terminal (both stdout and stdin must be TTYs); for agents, CI, dumb -/// terminals, or pipes it falls back to JSON and warns — never trapping an -/// agent in a render loop (Standard §5). -fn resolve_explore() -> OutputMode { - if is_agent_env() - || is_ci() - || is_dumb_term() - || !std::io::stdout().is_terminal() - || !std::io::stdin().is_terminal() - { - warn_tui_fallback("no interactive terminal available", "json"); - OutputMode::Json +/// terminals, or pipes it falls back to JSON with the reason — never trapping +/// an agent in a render loop (Standard §5). +fn resolve_explore() -> (OutputMode, Option<&'static str>) { + let reason = if is_agent_env() { + Some("agent environment (AI_AGENT/AGENT) is set") + } else if is_ci() { + Some("running under CI") + } else if is_dumb_term() { + Some("TERM=dumb") + } else if !std::io::stdout().is_terminal() { + Some("stdout is not a TTY") + } else if !std::io::stdin().is_terminal() { + Some("stdin is not a TTY") } else { - OutputMode::Explore + None + }; + match reason { + Some(r) => (OutputMode::Json, Some(r)), + None => (OutputMode::Explore, None), } } @@ -130,32 +141,61 @@ fn should_use_color(g: &GlobalArgs) -> bool { Some(ColorArg::Always) => return true, Some(ColorArg::Auto) | None => {} } - // 3. NO_COLOR (set + non-empty) disables. - if std::env::var_os("NO_COLOR").is_some_and(|v| !v.is_empty()) { - return false; - } - // 4. FORCE_COLOR (set + non-empty) enables, overriding NO_COLOR. - if std::env::var_os("FORCE_COLOR").is_some_and(|v| !v.is_empty()) { + // 3–7. Environment + TTY, as a pure decision for testability. + color_env_decision( + std::env::var_os("FORCE_COLOR").is_some_and(|v| !v.is_empty()), + std::env::var_os("NO_COLOR").is_some_and(|v| !v.is_empty()), + std::env::var("CLICOLOR").as_deref() == Ok("0"), + is_dumb_term(), + std::io::stdout().is_terminal(), + ) +} + +/// Steps 3–7 of the §6 color-precedence chain, decoupled from the live +/// environment. First match decides; `FORCE_COLOR` is checked before +/// `NO_COLOR` so it overrides it (per force-color.org). +fn color_env_decision( + force_color: bool, + no_color: bool, + clicolor_zero: bool, + dumb_term: bool, + tty: bool, +) -> bool { + if force_color { return true; } - // 5/6. CLICOLOR=0 and TERM=dumb disable. - if std::env::var("CLICOLOR").as_deref() == Ok("0") || is_dumb_term() { + if no_color { + return false; + } + if clicolor_zero || dumb_term { return false; } - // 7. TTY. - std::io::stdout().is_terminal() + tty } -/// Emit a single-line JSON warning to stderr when the TUI cannot run. -fn warn_tui_fallback(reason: &str, fell_back_to: &str) { - let warning = serde_json::json!({ - "warning": { - "code": "TUI_FALLBACK", - "message": "interactive explore mode unavailable; falling back", - "reason": reason, - "fell_back_to": fell_back_to, - "timestamp": crate::time::now_iso8601(), - } - }); - eprintln!("{warning}"); +#[cfg(test)] +mod tests { + use super::color_env_decision; + + #[test] + fn force_color_overrides_no_color() { + assert!(color_env_decision(true, true, false, false, false)); + } + + #[test] + fn no_color_disables_on_a_tty() { + assert!(!color_env_decision(false, true, false, false, true)); + } + + #[test] + fn clicolor_zero_and_dumb_term_disable() { + assert!(!color_env_decision(false, false, true, false, true)); + assert!(!color_env_decision(false, false, false, true, true)); + } + + #[test] + fn tty_decides_when_no_env_signal() { + assert!(color_env_decision(false, false, false, false, true)); + assert!(!color_env_decision(false, false, false, false, false)); + } } diff --git a/construct-cli/src/output/render.rs b/construct-cli/src/output/render.rs index 172faa9..1d81a89 100644 --- a/construct-cli/src/output/render.rs +++ b/construct-cli/src/output/render.rs @@ -160,15 +160,11 @@ fn emit_human(human: &HumanRender, color: bool) { let _ = writeln!( out, "{} {}", - label.truecolor( - theme::STEEL_BLUE.0, - theme::STEEL_BLUE.1, - theme::STEEL_BLUE.2 - ), + label.truecolor(theme::STRUCTURE.0, theme::STRUCTURE.1, theme::STRUCTURE.2), value.truecolor( - theme::LIQUID_COOLANT.0, - theme::LIQUID_COOLANT.1, - theme::LIQUID_COOLANT.2 + theme::FOREGROUND.0, + theme::FOREGROUND.1, + theme::FOREGROUND.2 ) ); } else { @@ -184,9 +180,9 @@ fn emit_human(human: &HumanRender, color: bool) { out, "{}", header_line.truecolor( - theme::MOLTEN_AMBER.0, - theme::MOLTEN_AMBER.1, - theme::MOLTEN_AMBER.2 + theme::STRUCTURE.0, + theme::STRUCTURE.1, + theme::STRUCTURE.2 ) ); } else { diff --git a/construct-cli/src/output/theme.rs b/construct-cli/src/output/theme.rs index 03c1ee3..34b1893 100644 --- a/construct-cli/src/output/theme.rs +++ b/construct-cli/src/output/theme.rs @@ -1,38 +1,55 @@ // SPDX-FileCopyrightText: 2026 Mohamed Hammad // SPDX-License-Identifier: GPL-3.0-or-later -//! The Steelbore v1.33 six-token color palette, grandfathered per Steelbore -//! Standard §11.1/§11.2 (v1.34) until construct's next minor release, when it -//! migrates to the nine-token Steelbore 2 palette. +//! The `steelbore` theme: the Steelbore 2 (Steelbore Modern) role tokens of +//! Standard §11.1. This release is construct's "next minor release" that the +//! v1.33 grandfathering clause pointed at, so the legacy six-token palette is +//! replaced by the full eleven-role contract. //! -//! Colors are referenced through these named tokens, never as inline hex +//! Colors are referenced through these named role tokens, never as inline hex //! literals, so a future theme can be substituted in one place. Each token is -//! an RGB triple consumed by `owo-colors`' `truecolor`. +//! an RGB triple consumed by `owo-colors`' `truecolor`. Values mirror the +//! canonical `steelbore-color-palette` skill's `steelbore.toml`. -/// Mandatory background / neutral chrome. -pub(crate) const VOID_NAVY: (u8, u8, u8) = (0x00, 0x00, 0x27); -/// Primary text, warnings, hints. -pub(crate) const MOLTEN_AMBER: (u8, u8, u8) = (0xD9, 0x8E, 0x32); -/// Accent / informational / structural. -pub(crate) const STEEL_BLUE: (u8, u8, u8) = (0x4B, 0x7E, 0xB0); -/// Success / safe status. -pub(crate) const RADIUM_GREEN: (u8, u8, u8) = (0x50, 0xFA, 0x7B); -/// Errors / warning status. -pub(crate) const RED_OXIDE: (u8, u8, u8) = (0xFF, 0x5C, 0x5C); -/// Data values / links / info. -pub(crate) const LIQUID_COOLANT: (u8, u8, u8) = (0x8B, 0xE9, 0xFD); +/// `background` — Void Navy. Mandatory canvas / neutral chrome. +pub(crate) const BACKGROUND: (u8, u8, u8) = (0x00, 0x00, 0x27); +/// `surface` — Quantum Blue. Elevated panel/card fill; never a text color. +pub(crate) const SURFACE: (u8, u8, u8) = (0x0E, 0x2A, 0x47); +/// `surface-alt` — Deep Matrix. Code/terminal well fill; never a text color. +pub(crate) const SURFACE_ALT: (u8, u8, u8) = (0x0B, 0x1A, 0x12); +/// `foreground` — Platinum Mist. Body text / default readout. +pub(crate) const FOREGROUND: (u8, u8, u8) = (0xD9, 0xDE, 0xE5); +/// `accent` — Plasma Orange. Primary accent; the `hint:` color. +pub(crate) const ACCENT: (u8, u8, u8) = (0xFF, 0x5E, 0x00); +/// `structure` — Pulse Violet. Structure / links / `[INFO]` diagnostics. +pub(crate) const STRUCTURE: (u8, u8, u8) = (0x8A, 0x6C, 0xFF); +/// `success` — Acid Lime. `[OK]` diagnostics / safe status. +pub(crate) const SUCCESS: (u8, u8, u8) = (0xB4, 0xFF, 0x00); +/// `error` — Mars Red. `[ERROR]` diagnostics. +pub(crate) const ERROR: (u8, u8, u8) = (0xFF, 0x3B, 0x3B); +/// `warning` — Plasma Magenta. `[WARN]` diagnostics. +pub(crate) const WARNING: (u8, u8, u8) = (0xE4, 0x45, 0xFF); +/// `focus` — Acid Lime. Visible focus indicator (alias of `success`). +pub(crate) const FOCUS: (u8, u8, u8) = SUCCESS; +/// `border` — Pulse Violet. Boundaries and table chrome (alias of `structure`). +pub(crate) const BORDER: (u8, u8, u8) = STRUCTURE; /// Silence "unused" while later phases (TUI, richer rendering) adopt the rest of /// the palette. Referencing every token keeps them live and documents intent. #[allow( dead_code, - reason = "full palette is part of the public theme; consumed incrementally across phases" + reason = "full eleven-role theme is part of the public contract; consumed incrementally across phases" )] -pub(crate) const PALETTE: [(u8, u8, u8); 6] = [ - VOID_NAVY, - MOLTEN_AMBER, - STEEL_BLUE, - RADIUM_GREEN, - RED_OXIDE, - LIQUID_COOLANT, +pub(crate) const PALETTE: [(u8, u8, u8); 11] = [ + BACKGROUND, + SURFACE, + SURFACE_ALT, + FOREGROUND, + ACCENT, + STRUCTURE, + SUCCESS, + ERROR, + WARNING, + FOCUS, + BORDER, ]; diff --git a/construct-cli/src/tui/mod.rs b/construct-cli/src/tui/mod.rs index 86c2d9f..517cc30 100644 --- a/construct-cli/src/tui/mod.rs +++ b/construct-cli/src/tui/mod.rs @@ -194,7 +194,7 @@ pub(crate) fn run(ctx: &Context) -> Result { ctx, ErrorCode::InternalError, format!("explore TUI error: {e}"), - "construct skill find # use the non-interactive browser instead", + "construct skill find --json", ) }) } @@ -294,25 +294,27 @@ fn render_tabs(frame: &mut Frame, app: &App, area: ratatui::layout::Rect) { Block::default() .borders(Borders::ALL) .title(" construct explore ") - .border_style(Style::default().fg(rgb(theme::STEEL_BLUE))), + .border_style(Style::default().fg(rgb(theme::STRUCTURE))), ) .select(selected) .highlight_style( Style::default() - .fg(rgb(theme::MOLTEN_AMBER)) + .fg(rgb(theme::ACCENT)) .add_modifier(Modifier::BOLD), ); frame.render_widget(tabs, area); } fn render_list(frame: &mut Frame, app: &App, area: ratatui::layout::Rect) { + // Selected/active rows are Acid Lime per tui-explore §3; inverted text on + // the fill is the verified 16.75:1 pairing (§11.0.2). let highlight = Style::default() - .bg(rgb(theme::STEEL_BLUE)) - .fg(rgb(theme::VOID_NAVY)) + .bg(rgb(theme::SUCCESS)) + .fg(rgb(theme::BACKGROUND)) .add_modifier(Modifier::BOLD); let border = Block::default() .borders(Borders::ALL) - .border_style(Style::default().fg(rgb(theme::STEEL_BLUE))); + .border_style(Style::default().fg(rgb(theme::STRUCTURE))); match app.tab { Tab::Skills => { @@ -327,7 +329,7 @@ fn render_list(frame: &mut Frame, app: &App, area: ratatui::layout::Rect) { "[ ] " }; ListItem::new(Line::from(vec![ - Span::styled(mark, Style::default().fg(rgb(theme::RADIUM_GREEN))), + Span::styled(mark, Style::default().fg(rgb(theme::SUCCESS))), Span::raw(s.name.clone()), ])) }) @@ -345,9 +347,9 @@ fn render_list(frame: &mut Frame, app: &App, area: ratatui::layout::Rect) { .map(|i| { let a = &app.agents[i]; let flag = if a.hm_managed { - Span::styled(" hm", Style::default().fg(rgb(theme::MOLTEN_AMBER))) + Span::styled(" hm", Style::default().fg(rgb(theme::ACCENT))) } else if a.installed { - Span::styled(" ✓", Style::default().fg(rgb(theme::RADIUM_GREEN))) + Span::styled(" ✓", Style::default().fg(rgb(theme::SUCCESS))) } else { Span::raw("") }; @@ -367,9 +369,9 @@ fn render_detail(frame: &mut Frame, app: &App, area: ratatui::layout::Rect) { let border = Block::default() .borders(Borders::ALL) .title(" Detail ") - .border_style(Style::default().fg(rgb(theme::STEEL_BLUE))); - let value = Style::default().fg(rgb(theme::LIQUID_COOLANT)); - let label = Style::default().fg(rgb(theme::MOLTEN_AMBER)); + .border_style(Style::default().fg(rgb(theme::STRUCTURE))); + let value = Style::default().fg(rgb(theme::FOREGROUND)); + let label = Style::default().fg(rgb(theme::STRUCTURE)); let lines: Vec = match app.tab { Tab::Skills => match app.current_skill() { @@ -441,7 +443,7 @@ fn render_help(frame: &mut Frame, app: &App, area: ratatui::layout::Rect) { ) }; frame.render_widget( - Paragraph::new(help).style(Style::default().fg(rgb(theme::STEEL_BLUE))), + Paragraph::new(help).style(Style::default().fg(rgb(theme::STRUCTURE))), area, ); } diff --git a/construct-cli/tests/cli.rs b/construct-cli/tests/cli.rs index 70bcd46..2a96b31 100644 --- a/construct-cli/tests/cli.rs +++ b/construct-cli/tests/cli.rs @@ -170,6 +170,70 @@ fn explore_falls_back_to_json_when_not_a_tty() { ); } +#[test] +fn explore_fallback_under_agent_env_is_a_diagnostic_envelope() { + // Under a real agent env value, the fallback warning must be the + // single-line `{"diagnostic":{...}}` envelope (diagnostics.md §3) — + // the agent floor is `warn`, so a warn-severity diagnostic still emits. + let assertion = bin() + .env("AI_AGENT", "claude-code_2-1-218_agent") + .args(["--format", "explore"]) + .assert() + .success(); + let out = assertion.get_output(); + serde_json::from_slice::(&out.stdout).expect("JSON fallback on stdout"); + let stderr = String::from_utf8(out.stderr.clone()).expect("valid UTF-8"); + let line = stderr.lines().next().expect("one stderr line"); + let value: Value = serde_json::from_str(line).expect("single-line diagnostic JSON"); + assert_eq!(value["diagnostic"]["severity"], "warn"); + assert_eq!(value["diagnostic"]["code"], "TUI_FALLBACK"); + assert!( + value["diagnostic"]["hint"] + .as_str() + .is_some_and(|h| h.starts_with("construct ")), + "hint must be a runnable construct command" + ); + assert!(value["diagnostic"]["reason"].is_string()); + assert!(!stderr.contains('\u{1b}'), "no ANSI in machine mode"); +} + +#[test] +fn quiet_suppresses_the_tui_fallback_warning() { + // `--quiet` raises the severity floor to errors-only: the warn-severity + // TUI fallback is not written, and stdout still carries the JSON payload. + let assertion = bin() + .args(["--format", "explore", "--quiet"]) + .assert() + .success(); + let out = assertion.get_output(); + serde_json::from_slice::(&out.stdout).expect("JSON fallback on stdout"); + assert!( + out.stderr.is_empty(), + "quiet must suppress the fallback warning, got: {}", + String::from_utf8_lossy(&out.stderr) + ); +} + +#[test] +fn quiet_never_suppresses_errors() { + // Errors bypass the severity floor: the structured error object still + // lands on stderr under --quiet. + let assertion = bin() + .args([ + "skill", + "sync", + "--flake-dir", + "/no/such/construct/dir", + "--json", + "--quiet", + ]) + .assert() + .code(3); + let err = assertion.get_output().stderr.clone(); + let value: Value = serde_json::from_slice(&err).expect("structured error on stderr"); + assert_eq!(value["error"]["code"], "NOT_FOUND"); +} + #[test] fn version_names_maintainer_and_site() { let out = bin()