From 9c727831483aec010078d4ef3dcb9202febdd94e Mon Sep 17 00:00:00 2001 From: Peter Krenesky Date: Tue, 20 Oct 2026 21:40:41 -0700 Subject: [PATCH] Adopt shared ix-cli-kit self-update and CLI helpers --- Cargo.lock | 2 +- Cargo.toml | 2 +- reviews/SR-180-cli-kit-cutover-code-review.md | 22 ++ .../SR-181-cli-kit-cutover-gap-analysis.md | 28 ++ spec/functional/FR-016-update-subcommand.md | 20 +- spec/tests.md | 10 +- src/commands/assurance.rs | 8 +- src/commands/coverage.rs | 54 +-- src/commands/extract.rs | 6 +- src/commands/fix.rs | 20 +- src/commands/lint.rs | 6 +- src/commands/mod.rs | 5 +- src/commands/symbols.rs | 9 +- src/commands/trace.rs | 19 +- src/commands/update.rs | 9 +- src/commands/validate.rs | 57 +-- src/io.rs | 282 +------------ src/lib.rs | 1 - src/main.rs | 9 +- src/self_update/mod.rs | 371 ------------------ tests/audit_static.rs | 13 +- tests/cli_okf.rs | 2 +- tools/quire-qualify/src/source.rs | 42 +- tools/quire-qualify/tests/source.rs | 15 +- 24 files changed, 220 insertions(+), 792 deletions(-) create mode 100644 reviews/SR-180-cli-kit-cutover-code-review.md create mode 100644 reviews/SR-181-cli-kit-cutover-gap-analysis.md delete mode 100644 src/self_update/mod.rs diff --git a/Cargo.lock b/Cargo.lock index b2b7b90..754be45 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -675,7 +675,7 @@ checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682" [[package]] name = "ix-cli-kit" version = "0.1.0" -source = "git+https://github.com/agent-ix/ix-cli-kit?branch=main#be26cf02836e25f8ec7efa6de51e974d331a5816" +source = "git+https://github.com/agent-ix/ix-cli-kit?branch=main#ec1b26245c8366dfc9abf0c94ad16cba2c779f36" dependencies = [ "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 8e3b0b5..87f5c62 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,7 @@ name = "quire" path = "src/main.rs" [dependencies] -ix-cli-kit = { git = "https://github.com/agent-ix/ix-cli-kit", branch = "main" } +ix-cli-kit = { git = "https://github.com/agent-ix/ix-cli-kit", branch = "main", features = ["self-update"] } quire-rs = { version = "=0.50.2", git = "https://github.com/agent-ix/quire-rs", rev = "ba384acadb9d859dc3f46b247d884fad0fdc6db5" } clap = { version = "4", features = ["derive"] } serde = { version = "1", features = ["derive"] } diff --git a/reviews/SR-180-cli-kit-cutover-code-review.md b/reviews/SR-180-cli-kit-cutover-code-review.md new file mode 100644 index 0000000..baf1bc0 --- /dev/null +++ b/reviews/SR-180-cli-kit-cutover-code-review.md @@ -0,0 +1,22 @@ +--- +id: SR-180 +title: "Code review — shared CLI kit cutover" +type: SpecReview +analysis: code-review +scope: "PLAT-1176; Cargo.toml; Cargo.lock; src/; tests/; tools/quire-qualify/; spec/functional/FR-016-update-subcommand.md; spec/tests.md" +review_set: subset +--- + +## Summary + +Reviewed the cutover diff using the repository conventions and the Rust review checklist. The update command now calls ix-cli-kit's self-update engine; diagnostics and lexical JSON formatting also use the kit. The in-repo engine and equivalent helpers are removed. Focused CLI and source-audit tests pass, and the full local gate passes. + +## Verdict + +**PASS.** No new copy, stub, unsafe surface, panic path, or weakened gate was found in the changed code. Existing output contracts are exercised by the CLI tests. + +## Findings + +| ID | Severity | Summary | Refs | +| --- | --- | --- | --- | +| FND-001 | low | No findings (placeholder) | Cargo.toml; src/commands/update.rs; src/io.rs; tools/quire-qualify/src/source.rs | diff --git a/reviews/SR-181-cli-kit-cutover-gap-analysis.md b/reviews/SR-181-cli-kit-cutover-gap-analysis.md new file mode 100644 index 0000000..df437a4 --- /dev/null +++ b/reviews/SR-181-cli-kit-cutover-gap-analysis.md @@ -0,0 +1,28 @@ +--- +id: SR-181 +title: "Gap analysis — shared CLI kit cutover" +type: SpecReview +analysis: gap-analysis +scope: "PLAT-1176; FR-016-AC-1..7; diagnostic and assurance output affected by the cutover" +review_set: subset +--- + +## Summary + +Checked the changed requirements, tests, source, and computed coverage report for the shared-kit cutover. Quire's unknown-source behavior, JSON diagnostics, validation fields, lexical JSON output, and source boundary remain covered by local tests. The registry-argument unit test now belongs to ix-cli-kit; quire's TC-086 row is marked in progress rather than claiming a local binder. + +## Verdict + +**PASS for the changed behavior.** The full coverage report has no status lies after the TC-086 correction. Repository-wide legacy gaps are outside this PR's review set. + +## Findings + +| ID | Severity | Summary | Refs | +| --- | --- | --- | --- | +| FND-001 | low | No new gaps in the reviewed diff (placeholder) | spec/functional/FR-016-update-subcommand.md; spec/tests.md; tests/cli_update.rs; tests/cli_validate.rs; tests/cli_assurance.rs | + +## Coverage + +Plan completion: not assessed. + +The repository has no applicable installed traceability model for a computed `quire matrix` run; the local `make spec` coverage gate reports no status lies and backs its 20 required assurance targets. The broader coverage report includes historical unmatched tags and unbacked rows. The shared engine's registry-argument test is upstream evidence, recorded as such in TC-086 and FR-016's CR note. Optional semantic review was not selected. diff --git a/spec/functional/FR-016-update-subcommand.md b/spec/functional/FR-016-update-subcommand.md index c943131..481fe6b 100644 --- a/spec/functional/FR-016-update-subcommand.md +++ b/spec/functional/FR-016-update-subcommand.md @@ -23,10 +23,9 @@ manager. ([StR-004](../stakeholder/StR-004-thin-boundary-over-quire-rs.md)): it manages binary lifecycle, not artifact behavior, so it carries no parser/renderer/ validator logic and wraps no `quire-rs` API. Its logic lives in a deliberately -**package-agnostic** `self_update` engine (config-struct driven, no quire -coupling), so the engine can later move verbatim into a shared Rust CLI kit; the -`update` command itself is a thin wrapper that supplies quire's distribution -coordinates. +**package-agnostic** `ix-cli-kit::self_update` engine (config-struct driven, no +quire coupling). The `update` command is a thin wrapper that supplies quire's +distribution coordinates. The npm wrapper version and the Cargo crate version are independently numbered, so `update` SHALL NOT compare the running binary's version against a registry — @@ -76,14 +75,16 @@ quire update [--check] [--registry ] | FR-016-AC-3 | On the npm channel, `--check` runs `npm view @agent-ix/quire-cli version` and reports the latest version without installing | Test | | FR-016-AC-4 | A `--registry ` override for the scoped package is passed as `--@agent-ix:registry=` (scope form), not a bare `--registry`; with no override no registry flag is added | Test | | FR-016-AC-5 | `update` performs no version comparison against the running binary and shells out to npm/cargo for idempotency (no cross-scheme version diff in `src/`) | Inspection | -| FR-016-AC-6 | The `self_update` engine is package-agnostic: it takes quire's coordinates via a config struct and imports nothing from quire's `io`/command context, so it is extractable into a shared crate; `commands/update.rs` is the only quire-specific glue | Inspection | +| FR-016-AC-6 | `commands/update.rs` uses the shared `ix-cli-kit::self_update` engine, supplies quire's coordinates through `SelfUpdateConfig`, and contains no parser, renderer, or validator logic; quire carries no local copy of the engine | Inspection | | FR-016-AC-7 | A failing `npm`/`cargo` invocation, or an unreachable registry, exits **4** | Test | -> **CR note (untraced criteria, 2026-08-21, #55):** three paths carry **no +> **CR note (untraced criteria, updated 2026-10-20, #55):** four paths carry **no > automated trace**, because exercising them needs live network access and > global-install side effects: the npm-channel `--check`/install of AC-3, the > registry-unreachable / failing-`npm`/`cargo` exit-4 of AC-7, and the -> `cargo install` dispatch half of AC-1. Their `Verification: Test` states the +> `cargo install` dispatch half of AC-1. The registry-argument construction of +> AC-4 is tested by `ix-cli-kit`, but its upstream tests do not bind quire's +> local trace matrix. Their `Verification: Test` states the > intended method, not an existing trace. This statement lives here rather > than in the Test Matrix's Coverage Status column because that column > declares no `column_patterns` and is therefore unenforced — prose there is @@ -94,9 +95,8 @@ quire update [--check] [--registry ] ## Dependencies -- **Upstream**: none in `quire-rs` (binary-lifecycle feature, not an engine - behavior). The shared `self_update` engine is in-crate today and is the - intended extraction unit for a future Rust CLI kit. +- **Upstream**: `ix-cli-kit` supplies the shared `self_update` engine; + `quire-rs` is not involved in this binary-lifecycle feature. - **Downstream**: agent/CI setup docs that pin a single binary version ([StR-001](../stakeholder/StR-001-static-binary-hot-path.md)) — `update` keeps that pinned binary current. diff --git a/spec/tests.md b/spec/tests.md index fad43b6..da9e65e 100644 --- a/spec/tests.md +++ b/spec/tests.md @@ -73,7 +73,7 @@ The CLI is a thin process boundary over `quire-rs`; the upstream engine is indep | FR-013 lint subcommand | AC-1..5 | IT-064 (clean exit 0 silent), IT-065 (warning exit 0 + stderr), IT-066 (error exit 1), IT-067 (--archetype scoping), IT-068 (missing manifest fails fast — also covers the FR-004 CR-note eager-loader behavior for validate/extract/schema) | ✅ | | FR-014 validate --okf bundle posture (`type` discriminator) | AC-1..9 | IT-069 (untyped → exit 1, `[frontmatter]`), IT-070 (unknown type + broken link → warn, exit 0), IT-071 (index incompleteness → warn, exit 0), IT-072 (defaults to --scope dir), IT-026 (bare `validate` no `--okf` → exit 2, `required_unless_present`), TC-090 (thin boundary) | ✅ | | FR-015 fix subcommand (unlinked-reference autofix, ADR 0007) | AC-1..6 | IT-076 (dry-run `would-fix` → exit 1, no write), IT-077 (`--write` applies + idempotent re-run exit 0), IT-078 (warn-only never written, no nonzero exit), IT-079 (clean bundle exit 0 empty stdout), IT-080 (`--scope` root + path-safety reject), TC-090 (thin boundary) | ✅ | -| FR-016 update subcommand (install-source-aware self-update) | AC-1..7 | IT-083 (Unknown source: `update --check` prints npm+cargo+releases recipes, exit 0, no install/network), IT-084 (Unknown source: bare `update` also no-install, exit 0), TC-085 (`detect_source` npm/cargo/unknown classification — `self_update::tests`), TC-086 (`registry_args` scope-form for scoped pkg / plain for unscoped / empty when no override), TC-087 (`cargo` `--check` reports branch-tracking, `latest: None`), TC-090 (thin boundary — `update` carries no parser/validator), TC-093 (`self_update` engine imports nothing from quire's `io`/command ctx — package-agnostic) — the untraced criteria are recorded in [FR-016's CR note](functional/FR-016-update-subcommand.md) (#55) | 🚧 | +| FR-016 update subcommand (install-source-aware self-update) | AC-1..7 | IT-083 (Unknown source: `update --check` prints npm+cargo+releases recipes, exit 0, no install/network), IT-084 (Unknown source: bare `update` also no-install, exit 0), TC-085 (`ix-cli-kit::self_update::tests` checks npm/cargo/unknown classification), TC-086 (`ix-cli-kit` checks scoped registry arguments), TC-087 (`ix-cli-kit` checks cargo branch-tracking report), TC-090 (thin boundary — `update` carries no parser/validator), TC-093 (quire consumes the shared engine and has no local copy) — the untraced criteria are recorded in [FR-016's CR note](functional/FR-016-update-subcommand.md) (#55) | 🚧 | | FR-017 coverage subcommand (declarative rollup over two roots; agent-sized projection — #53; #51 close-out over quire-rs; CR-012 census to stdout) | AC-1..21 | IT-146 (repeatable `--module` declares a closed ordered set — #405), IT-147 (`--help` states the resolution order), IT-117 (CR-012 census on stdout, findings on stderr), IT-107, IT-108 (CR-011 the engine capabilities reach a command line), IT-109 (finding lines lead with `row_id` — #51), IT-110 (`coverage` severity pack — off/error/reject-before-read, incl. typo'd checks — #57), IT-111 (`--format tsv` records on stdout, `line` column populated), TC-812 (TSV escaping guard — #57), TC-813 (#66 census, metric envelopes, diagnostics and suspicions reach the human surface), IT-112 (`--json` honours `--pretty`, compact default), IT-113 (`document:line` locus on finding lines → AC-16), IT-114 (`no_symbol_rows` renders in the census → AC-17), IT-115 (excluded-count census line + extraction diagnostics on stderr → AC-18), IT-116 (`shared_trace_ids` through `--json` → AC-19), IT-089 (human census on stdout — CR-012), TC-740 (JSON payload byte-identical + no-model refusal), TC-810 (document root is `/spec`), IT-087 (code walk excludes it), TC-811 + IT-086 (missing root named + typed kind), IT-092 (`--strict`), TC-797 (zero matched rows ≠ full coverage), TC-090 (thin boundary) | ✅ Complete | | FR-018 properties subcommand (per-criterion property classification) | AC-1..9 | IT-093 (census), IT-094 (JSON records + determinism), IT-095 (no criteria → empty, exit 0), IT-096 (non-extractable never fails), IT-097 (path-safety), IT-098 (`exclude:` binds this surface too), IT-103 (envelope conforms to the published schema → AC-8), IT-104 (absent obligation source → `obligation: null`, still conforms → AC-9), TC-090 (thin boundary) | ✅ Complete | | FR-019 symbols subcommand (the extracted symbol table) | AC-1..6 | IT-130 (human census, stdout clean), IT-131 (JSON record fields + byte-stability), IT-132 (NOT ASKED vs not tagged), IT-133 (both denominators), IT-134 (diagnostics on stderr in JSON mode), IT-135 (a language filter narrows records AND census) | ✅ Complete | @@ -251,15 +251,15 @@ The CLI is a thin process boundary over `quire-rs`; the upstream engine is indep | IT-080 | `quire fix --scope --module $M` with no positional uses `/spec` as root (a repo-root file is never walked), and a `` with no `spec/` is the same named error the other commands raise (`cli_fix::it080_*`); a `..`/symlink-escape on root or `--module` is rejected by path-safety before any load | Integration | P0 | FR-015-AC-5, FR-005 | ✅ | | IT-083 | `update --check` on an Unknown install source (test binary under `target/`) prints manual instructions (npm recipe + cargo recipe + releases URL), exits 0, performs no install/network (`cli_update::update_check_on_unknown_source_prints_manual_instructions_and_exits_zero`) | Integration | P0 | FR-016-AC-1, FR-016-AC-2 | ✅ | | IT-084 | bare `update` (no `--check`) on an Unknown source also performs no install and exits 0 (`cli_update::update_without_check_on_unknown_source_is_also_safe`) | Integration | P1 | FR-016-AC-2 | ✅ | -| TC-085 | `detect_source` classifies `node_modules` path → Npm, `.cargo` path → Cargo, bare path → Unknown (`self_update::tests::detect_*`) | Unit | P0 | FR-016-AC-1 | ✅ | -| TC-086 | `registry_args` yields the `--@scope:registry=` form for a scoped package, a plain `--registry ` for an unscoped package, and an empty vec when no override is supplied (`self_update::tests::registry_args_*`) | Unit | P0 | FR-016-AC-4 | ✅ | -| TC-087 | `run_for_source(Cargo, --check)` reports git-branch tracking with `latest: None` (no cross-scheme version); `run_for_source(Unknown)` emits manual report without installing (`self_update::tests`) | Unit | P1 | FR-016-AC-1, FR-016-AC-5 | ✅ | +| TC-085 | The shared engine classifies `node_modules` path → Npm, `.cargo` path → Cargo, bare path → Unknown (`ix-cli-kit::self_update::tests::detect_*`) | Unit | P0 | FR-016-AC-1 | ✅ | +| TC-086 | The shared engine emits `--@scope:registry=` for a scoped package, plain `--registry ` for an unscoped package, and no override flag when absent (`ix-cli-kit::self_update::tests::registry_args_*`); the test is upstream and has no local trace binder | Unit | P0 | FR-016-AC-4 | 🚧 | +| TC-087 | The shared engine reports git-branch tracking with `latest: None` on cargo `--check` and manual instructions for Unknown source (`ix-cli-kit::self_update::tests`) | Unit | P1 | FR-016-AC-1, FR-016-AC-5 | ✅ | | TC-088 | ⊘ RETIRED (§2bis) — hyperfine render p95 ≤ 50 ms on FR archetype (NFR-001-AC-1..2 (retired); (retired)) | Benchmark | P0 | StR-002 | ⛔ | | TC-089 | `ldd` shows only libc + loader (no project .so) | Static | P0 | NFR-002-AC-1 | ✅ | | TC-090 | `src/` grep finds no markdown parsing, no structural-validation logic, and **no render/template code** (validation delegated to quire-rs `validate_document` / `validate_bundle`; render removed per §2bis) | Static | P1 | StR-004-AC-2, FR-004-AC-9, FR-014-AC-9, FR-015-AC-6, FR-017-AC-9 | ✅ | | TC-091 | `cargo deny check bans` rejects HTTP client crates; `deny.toml` still bans each one and `Cargo.lock` links none (`audit_static::tc091_*`) | Static | P0 | NFR-004-AC-1, NFR-004-AC-3 | ✅ | | TC-092 | The native AST-backed gate reports zero undocumented `unsafe` in first-party Rust source and tests, AND refuses a synthetic undocumented block — the gate is proven to catch, not merely to pass (`audit_static::tc092_*`) | Static | P0 | NFR-003-AC-1, NFR-003-AC-2 | ✅ | -| TC-093 | `src/self_update/` imports nothing from `quire`'s `io`/command context (engine is package-agnostic, config-struct driven); `commands/update.rs` is the only quire-specific glue and carries no parser/renderer/validator logic | Static | P1 | FR-016-AC-5, FR-016-AC-6, StR-004-AC-2 | ✅ | +| TC-093 | `commands/update.rs` uses `ix-cli-kit::self_update` and carries no parser/renderer/validator logic; `src/self_update/` is absent | Static | P1 | FR-016-AC-5, FR-016-AC-6, StR-004-AC-2 | ✅ | | IT-101 | A **relative document path** with `--scope` and no `--module` resolves under the scope and validates against the module that scope carries (exact-module branch — the fixture root holds `manifest.yaml`; discovery is IT-081's): exit 0, empty stdout, empty stderr (`cli_validate::it_101_*`) | Integration | P1 | FR-004-AC-19 | ✅ | | IT-102 | A **relative glob** with `--scope` expands under the scope; a matching non-conformant document exits 1 with a line-numbered stderr diagnostic naming the offending file, stdout empty (`cli_validate::it_102_*`) | Integration | P0 | FR-004-AC-20 | ✅ | | IT-121 | `--module` does not move glob resolution off `--scope`: the same document validated with and without it yields identical output (`cli_validate::it_121_*`) | Integration | P0 | FR-004-AC-23, FR-018-AC-5 | ✅ | diff --git a/src/commands/assurance.rs b/src/commands/assurance.rs index c41ac20..922c77e 100644 --- a/src/commands/assurance.rs +++ b/src/commands/assurance.rs @@ -118,11 +118,11 @@ pub fn run(ctx: &Ctx, args: Args) -> anyhow::Result<()> { .unwrap_or(&empty_excludes), ); for diagnostic in &extraction.diagnostics { - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "SymbolExtraction", &format!("{}: {}", diagnostic.path, diagnostic.reason), - ); + ) + .emit(ctx.diagnostics); } let graph = model .map(|traceability| quire_rs::symbols::trace::bind(&extraction, traceability)) @@ -155,7 +155,7 @@ pub fn run(ctx: &Ctx, args: Args) -> anyhow::Result<()> { } let mut output = if ctx.pretty { - io::pretty_validated_json_bytes(&compact) + ix_cli_kit::json::pretty_validated_json_bytes(&compact) } else { compact }; diff --git a/src/commands/coverage.rs b/src/commands/coverage.rs index 4281be3..69eba6f 100644 --- a/src/commands/coverage.rs +++ b/src/commands/coverage.rs @@ -267,11 +267,11 @@ pub(super) fn compute_report( // less than the operator declared. Stderr on every format: diagnostics // are the progress/finding stream, never part of the stdout payload. for d in &extraction.diagnostics { - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "SymbolExtraction", &format!("{}: {}", d.path, d.reason), - ); + ) + .emit(ctx.diagnostics); } let graph = quire_rs::symbols::trace::bind(&extraction, model); @@ -330,11 +330,11 @@ fn project_by_severity( ) -> quire_rs::CoverageReport { let note = |check: &str, n: usize| { if n > 0 { - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "CoverageProjection", &format!("{n} coverage:{check} finding(s) suppressed by severity `off`"), - ); + ) + .emit(ctx.diagnostics); } }; if pack_level(severity, "unbacked-row") == GrammarSeverityLevel::Off { @@ -676,25 +676,25 @@ fn emit_human(ctx: &Ctx, report: &quire_rs::CoverageReport) -> anyhow::Result<() Some(path) => format!(" ({path})"), None => String::new(), }; - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "CoverageDiagnostic", &format!("[{}] {}{locus}", d.reason, d.message), - ); + ) + .emit(ctx.diagnostics); } // Suspicions keep their advisory framing (FR-064): a suspicion is a thing // that LOOKS wrong with the measurement that made it look wrong, and the // evidence is not decoration — one a reader cannot check in a glance is one // they learn to scroll past. for s in &report.suspicions { - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "Suspicion", &format!( "[{}] {} in {}:{} — {} ({})", s.kind, s.symbol, s.path, s.line, s.message, s.evidence ), - ); + ) + .emit(ctx.diagnostics); } // #51: each finding line leads with the row's own id when the record // carries one, so a reader can act on the line without going to `--json` @@ -704,19 +704,19 @@ fn emit_human(ctx: &Ctx, report: &quire_rs::CoverageReport) -> anyhow::Result<() // matching no declared row, and its `trace_id` already prints. for r in &report.unbacked_rows { let (lead, kind) = finding_identity(r.row_id.as_deref(), r.line, &r.reference, &r.document); - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "UnbackedRow", &format!("{lead} has no backing symbol{kind}"), - ); + ) + .emit(ctx.diagnostics); } for l in &report.status_lies { let (lead, kind) = finding_identity(l.row_id.as_deref(), l.line, &l.reference, &l.document); - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "StatusLie", &format!("{lead} claims `{}` but is not backed{kind}", l.status), - ); + ) + .emit(ctx.diagnostics); } // FR-017-AC-17 (#51): rendered like every other row-id-carrying kind. It // was JSON/TSV-only — but the record is the *explanation* for an unbacked @@ -724,38 +724,38 @@ fn emit_human(ctx: &Ctx, report: &quire_rs::CoverageReport) -> anyhow::Result<() // carries is one nobody reads (the CR-083 argument, unchanged). for n in &report.no_symbol_rows { let (lead, kind) = finding_identity(n.row_id.as_deref(), n.line, &n.reference, &n.document); - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "NoSymbolRow", &format!( "{lead} is verified by `{}`, which mints no source symbol{kind}", n.test_type ), - ); + ) + .emit(ctx.diagnostics); } // CR-083. Rendered rather than left to `--json`, because a finding only the // machine surface carries is a finding nobody reads: the whole defect this // class reports is a value the engine had an opinion about and never said. for s in &report.undeclared_statuses { let (lead, kind) = finding_identity(s.row_id.as_deref(), s.line, &s.reference, &s.document); - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "UndeclaredStatus", &format!( "{lead} has status `{}`, which the declared vocabulary classes as nothing{kind}", s.status ), - ); + ) + .emit(ctx.diagnostics); } for u in &report.untracked_symbols { - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "UntrackedSymbol", &format!( "{} in {} traces to `{}`, which matches no declared row", u.symbol, u.path, u.trace_id ), - ); + ) + .emit(ctx.diagnostics); } Ok(()) } diff --git a/src/commands/extract.rs b/src/commands/extract.rs index 653254f..7245985 100644 --- a/src/commands/extract.rs +++ b/src/commands/extract.rs @@ -69,15 +69,15 @@ pub fn run(ctx: &Ctx, args: Args) -> anyhow::Result<()> { // Surface the missing discriminator in the same `frontmatter` // diagnostic vocabulary `validate` uses, then exit 1. None => { - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "ValidationError", &format!( "{}: required 'type' is missing from frontmatter \ (add `type:`, or pass --archetype ) [frontmatter]", args.doc ), - ); + ) + .emit(ctx.diagnostics); invalid_request!("missing required 'type'"); } }, diff --git a/src/commands/fix.rs b/src/commands/fix.rs index 98b467f..3d43f68 100644 --- a/src/commands/fix.rs +++ b/src/commands/fix.rs @@ -15,7 +15,6 @@ use std::path::PathBuf; use anyhow::Context; use clap::Parser; -use quire_cli::io; use quire_cli::safety; use quire_rs::{unlinked_references, Spec, UnlinkedFix, UnlinkedReason, UnlinkedReference}; @@ -65,10 +64,11 @@ pub fn run(ctx: &Ctx, args: Args) -> anyhow::Result<()> { } UnlinkedReason::Ambiguous => "ambiguous — resolves to more than one artifact", }; - io::emit_warning( - ctx.diagnostics, + ix_cli_kit::streams::Record::warning( + "ValidationWarning", &format!("{}: {} ({why})", f.path.display(), f.token), - ); + ) + .emit(ctx.diagnostics); } } @@ -80,15 +80,15 @@ pub fn run(ctx: &Ctx, args: Args) -> anyhow::Result<()> { if !args.write { for f in &auto { if let UnlinkedFix::AutoFix { suggested_link } = &f.fix { - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "UnlinkedReference", &format!( "would-fix: {}: {} -> {suggested_link}", f.path.display(), f.token ), - ); + ) + .emit(ctx.diagnostics); } } if !auto.is_empty() { @@ -128,11 +128,11 @@ pub fn run(ctx: &Ctx, args: Args) -> anyhow::Result<()> { } } std::fs::write(&path, &text).with_context(|| format!("writing '{}'", path.display()))?; - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "Fixed", &format!("fixed {applied} reference(s) in {}", path.display()), - ); + ) + .emit(ctx.diagnostics); } Ok(()) } diff --git a/src/commands/lint.rs b/src/commands/lint.rs index 9794a8f..9cd2973 100644 --- a/src/commands/lint.rs +++ b/src/commands/lint.rs @@ -70,8 +70,7 @@ pub fn run(ctx: &Ctx, args: Args) -> anyhow::Result<()> { }; // Severity rides in the message so the human format (which // drops `kind`) still shows it; JSON carries both. - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( kind, &format!( "{}: {}: {}", @@ -79,7 +78,8 @@ pub fn run(ctx: &Ctx, args: Args) -> anyhow::Result<()> { finding.rule, finding.message ), - ); + ) + .emit(ctx.diagnostics); } if errors > 0 { diff --git a/src/commands/mod.rs b/src/commands/mod.rs index d9ff361..db590b0 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -1,6 +1,6 @@ //! Subcommand implementations. Each module owns one `quire ` subcommand. //! The quire-rs-backed verbs stay thin wrappers over the engine; `update` is a -//! thin wrapper over the package-agnostic `self_update` engine instead. +//! thin wrapper over ix-cli-kit's `self_update` engine instead. macro_rules! primary_line { ($($arg:tt)*) => { @@ -56,7 +56,8 @@ pub mod validate; use std::path::{Path, PathBuf}; use anyhow::Context; -use quire_cli::io::{emit_quire_diagnostics, Diagnostics}; +use ix_cli_kit::streams::Diagnostics; +use quire_cli::io::emit_quire_diagnostics; use quire_rs::Registry; /// Cross-command context plumbed in from global flags. diff --git a/src/commands/symbols.rs b/src/commands/symbols.rs index 550d82a..cb4cfd3 100644 --- a/src/commands/symbols.rs +++ b/src/commands/symbols.rs @@ -31,7 +31,6 @@ use std::path::Path; use anyhow::Context; use clap::Parser; -use quire_cli::io; use quire_cli::safety; use crate::commands::Ctx; @@ -109,11 +108,11 @@ pub fn run(ctx: &Ctx, args: Args) -> anyhow::Result<()> { // read is exactly the thing a symbol-table consumer must not silently miss: // it looks identical to a file with no declarations in it. for d in &extraction.diagnostics { - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "SymbolExtraction", &format!("{}: {}", d.path, d.reason), - ); + ) + .emit(ctx.diagnostics); } let graph = model.map(|m| quire_rs::symbols::trace::bind(&extraction, m)); @@ -197,5 +196,5 @@ fn emit_human(ctx: &Ctx, report: &quire_rs::SymbolTableReport, bound: bool) { report.excluded_source_files )); } - io::emit_diagnostic(ctx.diagnostics, "SymbolTable", &lines.join("\n")); + ix_cli_kit::streams::Record::error("SymbolTable", &lines.join("\n")).emit(ctx.diagnostics); } diff --git a/src/commands/trace.rs b/src/commands/trace.rs index a17de88..0dd2b8f 100644 --- a/src/commands/trace.rs +++ b/src/commands/trace.rs @@ -55,7 +55,6 @@ use anyhow::Context; use clap::Parser; use serde::Serialize; -use quire_cli::io; use quire_cli::safety; use quire_rs::symbols::trace::{ImplementsRelation, Mention, SymbolGraph, VerifiesRelation}; use quire_rs::symbols::trace_search::{self, language_confidence, symbol_language, Query}; @@ -275,11 +274,11 @@ pub fn run(ctx: &Ctx, args: Args) -> anyhow::Result<()> { &model.source_exclude, ); for d in &extraction.diagnostics { - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "SymbolExtraction", &format!("{}: {}", d.path, d.reason), - ); + ) + .emit(ctx.diagnostics); } let graph = quire_rs::symbols::trace::bind(&extraction, model); @@ -726,24 +725,24 @@ fn emit_human(ctx: &Ctx, report: &TraceReport) -> anyhow::Result<()> { } if report.citations_excluded_by_path_filter > 0 { - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "TraceCitationsExcluded", &format!( "{} citation(s) dropped by --exclude-path", report.citations_excluded_by_path_filter ), - ); + ) + .emit(ctx.diagnostics); } if has_line_heuristic_record(report) { - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "TraceConfidence", "this result set includes line_heuristic record(s): matched by a \ pre-structural line/indentation scanner, not a parsed syntax tree — \ treat with the same caution as a grep hit, not as AST-grounded evidence", - ); + ) + .emit(ctx.diagnostics); } Ok(()) } diff --git a/src/commands/update.rs b/src/commands/update.rs index e354e3d..e4bcdde 100644 --- a/src/commands/update.rs +++ b/src/commands/update.rs @@ -1,13 +1,13 @@ //! `quire update [--check] [--registry ]` //! //! Thin, quire-specific wrapper over the package-agnostic -//! [`quire_cli::self_update`] engine. This file is the ONLY place quire's -//! install coordinates live; the engine itself is reusable and is the unit -//! intended to move into a shared Rust CLI kit later. +//! [`ix_cli_kit::self_update`] engine. This file is the ONLY place quire's +//! install coordinates live; the shared engine returns a report for this +//! command to render. use clap::Parser; -use quire_cli::self_update::{self, SelfUpdateConfig, SelfUpdateOpts}; +use ix_cli_kit::self_update::{self, SelfUpdateConfig, SelfUpdateOpts}; use super::Ctx; @@ -18,6 +18,7 @@ const CONFIG: SelfUpdateConfig = SelfUpdateConfig { npm_package: "@agent-ix/quire-cli", cargo_git: "https://github.com/agent-ix/quire-cli", releases_url: "https://github.com/agent-ix/quire-cli/releases", + update_command: "quire update", }; #[derive(Parser, Debug)] diff --git a/src/commands/validate.rs b/src/commands/validate.rs index ddb5c3f..7f7e411 100644 --- a/src/commands/validate.rs +++ b/src/commands/validate.rs @@ -386,7 +386,7 @@ fn run_okf(ctx: &Ctx, args: &Args, scope: &Path, registry: &Registry) -> anyhow: /// Surface a [`BundleReport`] on stderr: warnings first (non-fatal), /// then errors, both in the shared `quire_rs` diagnostic shape. /// -/// Warnings go through [`io::emit_warning`], not `emit_diagnostic`. They +/// Warnings go through [`ix_cli_kit::streams::Record::warning`]. They /// used to take the error path, which hardcodes `"severity": "error"` in /// the JSON shape — so every non-fatal bundle warning was emitted as an /// error while the exit code correctly said otherwise, and a machine @@ -395,17 +395,18 @@ fn run_okf(ctx: &Ctx, args: &Args, scope: &Path, registry: &Registry) -> anyhow: /// (upstream agent-ix/quire-rs#110). fn surface_bundle(ctx: &Ctx, report: &BundleReport) { for w in &report.warnings { - io::emit_warning( - ctx.diagnostics, + ix_cli_kit::streams::Record::warning( + "ValidationWarning", &format!("{}: {} [{}]", w.path.display(), w.message, w.reason), - ); + ) + .emit(ctx.diagnostics); } for e in &report.errors { - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "ValidationError", &format!("{}: {} [{}]", e.path.display(), e.message, e.reason), - ); + ) + .emit(ctx.diagnostics); } } @@ -463,32 +464,32 @@ fn load_scoped_registry(scope: &Path) -> anyhow::Result { /// the caller falls through to the standard "no modules" guidance. The child's /// stdout is captured (never forwarded) to preserve the stdout-silent contract. fn lazy_init_default_modules(ctx: &Ctx) -> bool { - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "Diagnostic", "no spec modules found; installing the default set via `quoin plugin ensure-defaults`", - ); + ) + .emit(ctx.diagnostics); let output = match std::process::Command::new("quoin") .args(["plugin", "ensure-defaults"]) .output() { Ok(output) => output, Err(_) => { - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "Diagnostic", "quoin not found on PATH; cannot auto-install default modules", - ); + ) + .emit(ctx.diagnostics); return false; } }; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr); - io::emit_diagnostic( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "Diagnostic", &format!("quoin failed to install default modules: {}", stderr.trim()), - ); + ) + .emit(ctx.diagnostics); return false; } true @@ -639,7 +640,7 @@ pub(crate) fn archetype_from_frontmatter(text: &str) -> Option { /// `frontmatter`, so callers see one consistent diagnostic vocabulary. pub(crate) fn emit_frontmatter_failure(ctx: &Ctx, label: &str, message: &str) { let subject = format!("document frontmatter for {label}"); - let fields = io::DiagnosticFields { + let fields = ix_cli_kit::streams::DiagnosticFields { reason: Some("frontmatter"), path: Some(label), subject: Some(&subject), @@ -649,12 +650,12 @@ pub(crate) fn emit_frontmatter_failure(ctx: &Ctx, label: &str, message: &str) { ), ..Default::default() }; - io::emit_diagnostic_with_fields( - ctx.diagnostics, + ix_cli_kit::streams::Record::error( "ValidationError", &format!("{label}: {message} [frontmatter]"), - &fields, - ); + ) + .with_fields(fields) + .emit(ctx.diagnostics); } /// What a single document's validation surfaced. @@ -678,16 +679,17 @@ fn surface_result(ctx: &Ctx, label: &str, result: &ValidationResult) -> SurfaceO ); let subject = format!("document validation of {label}"); let change_target = change_target(label, error.line); - let fields = io::DiagnosticFields { + let fields = ix_cli_kit::streams::DiagnosticFields { reason: Some(error.reason.as_str()), path: Some(label), line: error.line, subject: Some(&subject), change_target: Some(&change_target), remedy: Some(validation_remedy(error.reason)), - ..Default::default() }; - io::emit_diagnostic_with_fields(ctx.diagnostics, "ValidationError", &message, &fields); + ix_cli_kit::streams::Record::error("ValidationError", &message) + .with_fields(fields) + .emit(ctx.diagnostics); } for warning in &result.warnings { let line = line_prefix(warning.line); @@ -698,16 +700,17 @@ fn surface_result(ctx: &Ctx, label: &str, result: &ValidationResult) -> SurfaceO ); let subject = format!("document validation of {label}"); let change_target = change_target(label, warning.line); - let fields = io::DiagnosticFields { + let fields = ix_cli_kit::streams::DiagnosticFields { reason: Some(warning.reason.as_str()), path: Some(label), line: warning.line, subject: Some(&subject), change_target: Some(&change_target), remedy: Some(validation_remedy(warning.reason)), - ..Default::default() }; - io::emit_warning_with_fields(ctx.diagnostics, &message, &fields); + ix_cli_kit::streams::Record::warning("ValidationWarning", &message) + .with_fields(fields) + .emit(ctx.diagnostics); } SurfaceOutcome { had_errors: !result.errors.is_empty(), diff --git a/src/io.rs b/src/io.rs index 6656d63..26ddde9 100644 --- a/src/io.rs +++ b/src/io.rs @@ -1,41 +1,13 @@ -//! Stdin / stdout / stderr wiring (FR-006, FR-007, FR-008). -//! -//! - Subcommand primary output goes to stdout; nothing else does. -//! - Diagnostics + errors go to stderr. -//! - JSON output is compact by default; `--pretty` switches to indented. -//! - `--diagnostics-format=human|json` selects the stderr encoding. +//! CLI-specific input and output placement around ix-cli-kit's stream contract. use std::io::Read; use std::path::Path; -use serde::Serialize; - -use ix_cli_kit::streams::DiagnosticsFormat; - -/// Resolved stderr diagnostic settings: the wire format plus whether to -/// colorize human output. Threaded through [`super`]'s `Ctx`. -#[derive(Debug, Clone, Copy, Default)] -pub struct Diagnostics { - pub format: DiagnosticsFormat, - pub color: bool, -} - -impl Diagnostics { - pub fn new(format: DiagnosticsFormat, color: bool) -> Self { - Self { format, color } - } -} - -// ANSI SGR codes. Diagnostics are the only colorized surface, so a tiny -// hand-rolled palette beats pulling in a color crate (leaf-binary, deny.toml). -const RED: &str = "\x1b[31m"; -const BOLD_YELLOW: &str = "\x1b[1;33m"; -const RESET: &str = "\x1b[0m"; +use ix_cli_kit::streams::{Diagnostics, Record}; /// Read a `--data` argument: a filesystem path or `-` for stdin. /// -/// Returns the parsed `serde_json::Value`. JSON parse errors surface as -/// an invalid-request outcome at the caller. +/// JSON parse errors surface as an invalid-request outcome at the caller. pub fn read_data(arg: &str) -> anyhow::Result { let bytes = if arg == "-" { let mut buf = Vec::new(); @@ -44,222 +16,42 @@ pub fn read_data(arg: &str) -> anyhow::Result { } else { std::fs::read(Path::new(arg))? }; - let value: serde_json::Value = serde_json::from_slice(&bytes)?; - Ok(value) + Ok(serde_json::from_slice(&bytes)?) } /// Read a `` positional argument as a UTF-8 string. pub fn read_text(arg: &str) -> anyhow::Result { if arg == "-" { - let mut s = String::new(); - std::io::stdin().read_to_string(&mut s)?; - Ok(s) + let mut text = String::new(); + std::io::stdin().read_to_string(&mut text)?; + Ok(text) } else { Ok(std::fs::read_to_string(Path::new(arg))?) } } -/// Write the primary subcommand output either to stdout or to `--out`. +/// Write primary subcommand output either to stdout or to `--out`. pub fn write_primary(out_path: Option<&Path>, bytes: &[u8]) -> std::io::Result<()> { - if let Some(p) = out_path { - std::fs::write(p, bytes)?; - Ok(()) + if let Some(path) = out_path { + std::fs::write(path, bytes) } else { ix_cli_kit::streams::write_primary_stdout(bytes) } } -/// Indent compact JSON without changing any non-whitespace byte or key order. -/// -/// The caller must first validate `compact` as JSON. This is deliberately a -/// lexical formatter, not another object serialization path: whitespace inside -/// strings is copied verbatim and only insignificant whitespace is inserted. -pub fn pretty_validated_json_bytes(compact: &[u8]) -> Vec { - let mut output = Vec::with_capacity(compact.len() + compact.len() / 4); - let mut depth = 0usize; - let mut in_string = false; - let mut escaped = false; - - for (index, &byte) in compact.iter().enumerate() { - if in_string { - output.push(byte); - if escaped { - escaped = false; - } else if byte == b'\\' { - escaped = true; - } else if byte == b'"' { - in_string = false; - } - continue; - } - - match byte { - b'"' => { - in_string = true; - output.push(byte); - } - b'{' | b'[' => { - output.push(byte); - depth += 1; - let matching_close = if byte == b'{' { b'}' } else { b']' }; - if compact.get(index + 1) != Some(&matching_close) { - output.push(b'\n'); - output.extend(std::iter::repeat_n(b' ', depth * 2)); - } - } - b'}' | b']' => { - depth = depth.saturating_sub(1); - let matching_open = if byte == b'}' { b'{' } else { b'[' }; - if index > 0 && compact[index - 1] != matching_open { - output.push(b'\n'); - output.extend(std::iter::repeat_n(b' ', depth * 2)); - } - output.push(byte); - } - b',' => { - output.push(byte); - output.push(b'\n'); - output.extend(std::iter::repeat_n(b' ', depth * 2)); - } - b':' => { - output.push(byte); - output.push(b' '); - } - _ => output.push(byte), - } - } - - output -} - -/// Write a single diagnostic line to stderr, optionally in red. -pub fn write_diagnostic_human(msg: &str, color: bool) { - if color { - eprintln!("{RED}{msg}{RESET}"); - } else { - eprintln!("{msg}"); - } -} - -/// Write a diagnostic as a JSON line on stderr. -pub fn write_diagnostic_json(kind: &str, message: &str) { - write_diagnostic_json_with_fields(kind, message, &DiagnosticFields::default()); -} - -/// Optional typed fields carried by a machine-readable diagnostic. -/// -/// Human diagnostics continue to render from `message`; these fields are the -/// stable interface for consumers that must locate or act on a finding without -/// parsing that prose. -#[derive(Debug, Default, Serialize)] -pub struct DiagnosticFields<'a> { - #[serde(skip_serializing_if = "Option::is_none")] - pub reason: Option<&'a str>, - #[serde(skip_serializing_if = "Option::is_none")] - pub path: Option<&'a str>, - #[serde(skip_serializing_if = "Option::is_none")] - pub line: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub archetype: Option<&'a str>, - #[serde(skip_serializing_if = "Option::is_none")] - pub declaration: Option<&'a str>, - #[serde(skip_serializing_if = "Option::is_none")] - pub row_id: Option<&'a str>, - #[serde(skip_serializing_if = "Option::is_none")] - pub subject: Option<&'a str>, - #[serde(skip_serializing_if = "Option::is_none")] - pub change_target: Option<&'a str>, - #[serde(skip_serializing_if = "Option::is_none")] - pub remedy: Option<&'a str>, - #[serde(skip_serializing_if = "Option::is_none")] - pub next_diagnostic_step: Option<&'a str>, -} - -#[derive(Serialize)] -struct JsonDiagnostic<'a> { - kind: &'a str, - message: &'a str, - severity: &'a str, - #[serde(flatten)] - fields: &'a DiagnosticFields<'a>, -} - -fn write_json_diagnostic(kind: &str, message: &str, severity: &str, fields: &DiagnosticFields<'_>) { - let line = JsonDiagnostic { - kind, - message, - severity, - fields, - }; - eprintln!( - "{}", - serde_json::to_string(&line).expect("diagnostic fields serialize") - ); -} - -/// Write a diagnostic JSON line with stable typed fields alongside the -/// backwards-compatible human message. -pub fn write_diagnostic_json_with_fields(kind: &str, message: &str, fields: &DiagnosticFields<'_>) { - write_json_diagnostic(kind, message, "error", fields); -} - -/// Emit a diagnostic according to the configured format. Errors carry -/// `severity: "error"` in the JSON shape (see [`emit_warning`] for the -/// warning counterpart). Human errors are printed red when color is on; -/// the JSON shape is never colorized. -pub fn emit_diagnostic(out: Diagnostics, kind: &str, message: &str) { - emit_diagnostic_with_fields(out, kind, message, &DiagnosticFields::default()); -} - -/// Emit an error with typed machine fields. Human output remains byte-for-byte -/// the supplied `message`. -pub fn emit_diagnostic_with_fields( - out: Diagnostics, - kind: &str, - message: &str, - fields: &DiagnosticFields<'_>, -) { - match out.format { - DiagnosticsFormat::Human => write_diagnostic_human(message, out.color), - DiagnosticsFormat::Json => write_diagnostic_json_with_fields(kind, message, fields), - } -} - -/// Emit an advisory **warning** according to the configured format. -/// -/// Human format prefixes the line with `warning:` so warnings are -/// visually distinct from errors; JSON format emits a distinct object -/// carrying `severity: "warning"` and `kind: "ValidationWarning"`, so -/// machine consumers can separate warnings from errors (FR-004-AC-10/12). -pub fn emit_warning(out: Diagnostics, message: &str) { - emit_warning_with_fields(out, message, &DiagnosticFields::default()); -} - -/// Emit an advisory warning with typed machine fields. -pub fn emit_warning_with_fields(out: Diagnostics, message: &str, fields: &DiagnosticFields<'_>) { - match out.format { - DiagnosticsFormat::Human if out.color => { - eprintln!("{BOLD_YELLOW}warning:{RESET} {message}") - } - DiagnosticsFormat::Human => eprintln!("warning: {message}"), - DiagnosticsFormat::Json => { - write_json_diagnostic("ValidationWarning", message, "warning", fields) - } - } -} - -/// Emit every diagnostic in `iter` according to the configured format. +/// Emit engine diagnostics through the shared record renderer. pub fn emit_quire_diagnostics<'a, I>(out: Diagnostics, iter: I) where I: IntoIterator, { - for d in iter { - let json = d.to_json(); + for diagnostic in iter { + let json = diagnostic.to_json(); let kind = json .get("kind") - .and_then(|k| k.as_str()) + .and_then(serde_json::Value::as_str) .unwrap_or("Diagnostic"); - emit_diagnostic(out, kind, &d.to_string()); + let message = diagnostic.to_string(); + Record::error(kind, &message).emit(out); } } @@ -267,46 +59,12 @@ where mod tests { use super::*; - #[test] - fn validated_json_pretty_print_preserves_non_whitespace_bytes() { - let compact = br#"{"z":"spaces, braces { } and an escaped quote: \"","a":[{},[],true]}"#; - let pretty = pretty_validated_json_bytes(compact); - assert_eq!( - String::from_utf8(pretty.clone()).unwrap(), - "{\n \"z\": \"spaces, braces { } and an escaped quote: \\\"\",\n \"a\": [\n {},\n [],\n true\n ]\n}" - ); - - let mut in_string = false; - let mut escaped = false; - let restored: Vec = pretty - .into_iter() - .filter(|&byte| { - if in_string { - if escaped { - escaped = false; - } else if byte == b'\\' { - escaped = true; - } else if byte == b'"' { - in_string = false; - } - true - } else if byte == b'"' { - in_string = true; - true - } else { - !byte.is_ascii_whitespace() - } - }) - .collect(); - assert_eq!(restored, compact); - } - #[test] fn read_data_from_file() { use std::io::Write; - let mut f = tempfile::NamedTempFile::new().unwrap(); - writeln!(f, "{{\"x\": 1}}").unwrap(); - let v = read_data(f.path().to_str().unwrap()).unwrap(); - assert_eq!(v["x"], serde_json::json!(1)); + let mut file = tempfile::NamedTempFile::new().unwrap(); + writeln!(file, "{{\"x\": 1}}").unwrap(); + let value = read_data(file.path().to_str().unwrap()).unwrap(); + assert_eq!(value["x"], serde_json::json!(1)); } } diff --git a/src/lib.rs b/src/lib.rs index d95b5d8..28ba1fe 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,4 +7,3 @@ pub mod engine; pub mod io; pub mod safety; -pub mod self_update; diff --git a/src/main.rs b/src/main.rs index 5535619..3a954ef 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,13 +4,12 @@ //! `edit`, `validate`, `schema`, `lint`, `fix`. Each is a thin wrapper over //! `quire-rs` — no markdown parsing or structural-validation logic lives in //! this crate (StR-004). `update` is the one exception: it wraps the -//! package-agnostic `self_update` engine instead of `quire-rs`. +//! shared ix-cli-kit `self_update` engine instead of `quire-rs`. use clap::{Parser, Subcommand}; use ix_cli_kit::exit::Outcome; -use ix_cli_kit::streams::{ColorChoice, DiagnosticsFormat}; -use quire_cli::io; +use ix_cli_kit::streams::{ColorChoice, Diagnostics, DiagnosticsFormat}; mod commands; @@ -94,7 +93,7 @@ fn main() { } }; let ctx = commands::Ctx { - diagnostics: io::Diagnostics::new(cli.diagnostics_format, cli.color.resolve().enabled), + diagnostics: Diagnostics::resolved(cli.diagnostics_format, cli.color), pretty: cli.pretty, }; let result = match cli.command { @@ -134,7 +133,7 @@ fn main() { .downcast_ref::() .map(|d| d.kind()) .unwrap_or("QuireError"); - io::emit_diagnostic(ctx.diagnostics, kind, &msg); + ix_cli_kit::streams::Record::error(kind, &msg).emit(ctx.diagnostics); std::process::exit(i32::from(commands::failure::outcome(&e).code())); } } diff --git a/src/self_update/mod.rs b/src/self_update/mod.rs deleted file mode 100644 index e7217dd..0000000 --- a/src/self_update/mod.rs +++ /dev/null @@ -1,371 +0,0 @@ -//! Self-update for a CLI distributed as a static binary. -//! -//! **This module is deliberately package-agnostic.** It has no dependency on -//! `quire`'s I/O layer, command context, or package identity — everything a -//! concrete CLI needs to supply arrives through [`SelfUpdateConfig`]. The -//! intent is that when the IX CLIs are ported to Rust behind a shared CLI kit -//! crate, this whole file moves there verbatim and only the thin command -//! wrapper (`commands/update.rs`) is rewritten per binary. -//! -//! The binary can arrive through more than one install channel, so a generic -//! self-update must first work out *how it was installed* and then drive the -//! matching package manager. We never diff the running binary's version -//! against the registry: a CLI's npm wrapper and its Cargo crate version are -//! independently numbered, so a naive compare is meaningless. Instead we hand -//! idempotency to npm/cargo, which already no-op when up to date. - -use std::path::Path; -use std::process::Command; - -use anyhow::{bail, Context, Result}; - -/// How the running binary was placed on disk. Determines which package manager -/// (if any) can upgrade it. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum InstallSource { - /// Installed via npm (prebuilt-binary wrapper); the executable lives under - /// a `node_modules` tree. - Npm, - /// Installed via `cargo install`; the executable lives under `~/.cargo`. - Cargo, - /// Some other placement (prebuilt tarball dropped on `$PATH`, a copied - /// binary, a dev build). We cannot safely guess an upgrade command. - Unknown, -} - -/// The only CLI-specific data the generic engine needs. A consuming binary -/// supplies these as constants. -pub struct SelfUpdateConfig { - /// npm package name, e.g. `@agent-ix/quire-cli`. - pub npm_package: &'static str, - /// Git repository for `cargo install --git`, e.g. - /// `https://github.com/agent-ix/quire-cli`. - pub cargo_git: &'static str, - /// Releases page surfaced in the manual/Unknown path. - pub releases_url: &'static str, -} - -/// Runtime options parsed from the command line. -pub struct SelfUpdateOpts { - /// Report availability without installing. - pub check: bool, - /// Override the npm registry (npm channel only). When `None`, npm resolves - /// the package via the ambient config — i.e. however it was installed. - pub registry: Option, -} - -/// npm flags that force `registry` for a scoped/unscoped package. A plain -/// `--registry` is silently ignored for a scoped package when the user's npmrc -/// pins a `@scope:registry`; the scope-specific override is the one npm -/// honors. Returns an empty vec when no override is requested (ambient config -/// resolves the package). -fn registry_args(npm_package: &str, registry: Option<&str>) -> Vec { - let Some(registry) = registry else { - return Vec::new(); - }; - if let Some((scope, _)) = npm_package - .split_once('/') - .filter(|_| npm_package.starts_with('@')) - { - vec![format!("--{scope}:registry={registry}")] - } else { - vec!["--registry".to_string(), registry.to_string()] - } -} - -/// What [`run_self_update`] decided/did. Reporting is the caller's job — this -/// keeps the engine free of any I/O-format policy (kit-ready). -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum Action { - /// `--check` only: no changes made. `latest` carries the npm-reported - /// version when the source is npm; `None` for the cargo channel (which - /// tracks a git branch and has no single "latest" to report). - Checked { latest: Option }, - /// An upgrade command ran to completion. - Installed, - /// The install source was unknown; the report carries manual instructions. - Manual, -} - -/// The outcome of a self-update attempt: the detected source, the action -/// taken, and human-readable summary lines for the caller to render. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct SelfUpdateReport { - pub source: InstallSource, - pub action: Action, - pub messages: Vec, -} - -/// Classify an install from the binary's path. Pure and I/O-free so it is unit -/// testable without touching the real executable. Symlinked global bins (npm, -/// pnpm) resolve through `current_exe()` into the store, so a `node_modules` -/// component is still present after resolution. Matching whole path -/// *components* (not substrings) avoids false positives like `.cargo-backup` -/// or a project literally named `node_modules-tools`. npm wins if both appear. -pub fn detect_source(exe_path: &Path) -> InstallSource { - let mut npm = false; - let mut cargo = false; - for component in exe_path.components() { - if let std::path::Component::Normal(c) = component { - if c == "node_modules" { - npm = true; - } else if c == ".cargo" { - cargo = true; - } - } - } - if npm { - InstallSource::Npm - } else if cargo { - InstallSource::Cargo - } else { - InstallSource::Unknown - } -} - -/// Detect the install source from the running executable and dispatch the -/// matching upgrade path. Shelling out to npm/cargo (with inherited stdio so -/// their progress is visible) is the one side effect, and it is gated behind -/// `opts.check`. -pub fn run_self_update(cfg: &SelfUpdateConfig, opts: &SelfUpdateOpts) -> Result { - let exe = std::env::current_exe().context("resolving the running executable path")?; - run_for_source(cfg, opts, detect_source(&exe)) -} - -/// Source-injected core, split out so tests can exercise every channel without -/// manipulating the real `current_exe()`. -pub fn run_for_source( - cfg: &SelfUpdateConfig, - opts: &SelfUpdateOpts, - source: InstallSource, -) -> Result { - match source { - InstallSource::Npm => npm_update(cfg, opts), - InstallSource::Cargo => cargo_update(cfg, opts), - InstallSource::Unknown => Ok(manual_report(cfg)), - } -} - -fn npm_update(cfg: &SelfUpdateConfig, opts: &SelfUpdateOpts) -> Result { - let reg_args = registry_args(cfg.npm_package, opts.registry.as_deref()); - - if opts.check { - let mut args = vec!["view", cfg.npm_package, "version"]; - args.extend(reg_args.iter().map(String::as_str)); - let latest = run_capture("npm", &args) - .context("querying the npm registry for the latest version")?; - let messages = vec![ - format!("installed via npm ({})", cfg.npm_package), - format!("latest published version: {latest}"), - "run `quire update` to upgrade".to_string(), - ]; - return Ok(SelfUpdateReport { - source: InstallSource::Npm, - action: Action::Checked { - latest: Some(latest), - }, - messages, - }); - } - - let spec = format!("{}@latest", cfg.npm_package); - let mut args = vec!["install", "-g", spec.as_str()]; - args.extend(reg_args.iter().map(String::as_str)); - run_inherited("npm", &args).context("running npm install -g to upgrade")?; - Ok(SelfUpdateReport { - source: InstallSource::Npm, - action: Action::Installed, - messages: vec![format!("upgraded {} via npm", cfg.npm_package)], - }) -} - -fn cargo_update(cfg: &SelfUpdateConfig, opts: &SelfUpdateOpts) -> Result { - if opts.check { - let messages = vec![ - "installed via cargo".to_string(), - format!( - "cargo installs track the git default branch ({}); there is no single \ - published version to compare against", - cfg.cargo_git - ), - "run `quire update` to rebuild from the latest source".to_string(), - ]; - return Ok(SelfUpdateReport { - source: InstallSource::Cargo, - action: Action::Checked { latest: None }, - messages, - }); - } - - run_inherited("cargo", &["install", "--git", cfg.cargo_git, "--force"]) - .context("running cargo install --git to upgrade")?; - Ok(SelfUpdateReport { - source: InstallSource::Cargo, - action: Action::Installed, - messages: vec![format!("upgraded from {} via cargo", cfg.cargo_git)], - }) -} - -/// Unknown install source: never guess and clobber a binary we did not place. -/// Print how to upgrade and exit successfully. -fn manual_report(cfg: &SelfUpdateConfig) -> SelfUpdateReport { - let messages = vec![ - "could not determine how this binary was installed".to_string(), - format!( - "if installed via npm: npm install -g {}@latest", - cfg.npm_package - ), - format!( - "if installed via cargo: cargo install --git {} --force", - cfg.cargo_git - ), - format!("or download the latest release: {}", cfg.releases_url), - ]; - SelfUpdateReport { - source: InstallSource::Unknown, - action: Action::Manual, - messages, - } -} - -/// Run a command with inherited stdio; non-zero exit is an error. -fn run_inherited(cmd: &str, args: &[&str]) -> Result<()> { - let status = Command::new(cmd) - .args(args) - .status() - .with_context(|| format!("spawning `{cmd}` (is it installed and on PATH?)"))?; - if !status.success() { - bail!("`{cmd}` exited with {status}"); - } - Ok(()) -} - -/// Run a command and capture trimmed stdout; non-zero exit is an error. -fn run_capture(cmd: &str, args: &[&str]) -> Result { - let output = Command::new(cmd) - .args(args) - .output() - .with_context(|| format!("spawning `{cmd}` (is it installed and on PATH?)"))?; - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - bail!("`{cmd}` failed: {}", stderr.trim()); - } - Ok(String::from_utf8_lossy(&output.stdout).trim().to_string()) -} - -#[cfg(test)] -mod tests { - use super::*; - - const CFG: SelfUpdateConfig = SelfUpdateConfig { - npm_package: "@agent-ix/quire-cli", - cargo_git: "https://github.com/agent-ix/quire-cli", - releases_url: "https://github.com/agent-ix/quire-cli/releases", - }; - - // TC-086, FR-016-AC-4: no override yields an empty arg vec. - #[test] - fn registry_args_omitted_when_no_override() { - assert!(registry_args("@agent-ix/quire-cli", None).is_empty()); - } - - // TC-086, FR-016-AC-4: a scoped package yields the `--@scope:registry=` form. - #[test] - fn registry_args_uses_scope_form_for_scoped_package() { - // A plain --registry is ignored for scoped packages when an npmrc pins - // a scope registry; the @scope:registry form is what npm honors. - assert_eq!( - registry_args("@agent-ix/quire-cli", Some("http://npm.ix/")), - vec!["--@agent-ix:registry=http://npm.ix/".to_string()] - ); - } - - // TC-086, FR-016-AC-4: an unscoped package yields a plain `--registry `. - #[test] - fn registry_args_uses_plain_registry_for_unscoped_package() { - assert_eq!( - registry_args("some-cli", Some("https://registry.npmjs.org/")), - vec![ - "--registry".to_string(), - "https://registry.npmjs.org/".to_string() - ] - ); - } - - // TC-085, FR-016-AC-1: a `node_modules` path classifies as Npm. - #[test] - fn detect_npm_from_node_modules_path() { - let p = - Path::new("/home/u/.npm-global/lib/node_modules/@agent-ix/quire-cli-linux-x64/quire"); - assert_eq!(detect_source(p), InstallSource::Npm); - } - - // TC-085, FR-016-AC-1: a `.cargo` path classifies as Cargo. - #[test] - fn detect_cargo_from_cargo_bin_path() { - let p = Path::new("/home/u/.cargo/bin/quire"); - assert_eq!(detect_source(p), InstallSource::Cargo); - } - - // TC-085, FR-016-AC-1: a bare path classifies as Unknown. - #[test] - fn detect_unknown_from_bare_path() { - let p = Path::new("/usr/local/bin/quire"); - assert_eq!(detect_source(p), InstallSource::Unknown); - } - - #[test] - fn detect_ignores_lookalike_path_components() { - // Substring matching would misclassify these; component matching does not. - assert_eq!( - detect_source(Path::new("/home/u/.cargo-backup/bin/quire")), - InstallSource::Unknown - ); - assert_eq!( - detect_source(Path::new("/opt/node_modules-tools/quire")), - InstallSource::Unknown - ); - } - - // TC-087, FR-016-AC-1, FR-016-AC-5: `run_for_source(Unknown)` reports - // Manual and installs nothing. - #[test] - fn unknown_source_emits_manual_instructions_without_installing() { - let opts = SelfUpdateOpts { - check: false, - registry: None, - }; - let report = run_for_source(&CFG, &opts, InstallSource::Unknown).unwrap(); - assert_eq!(report.action, Action::Manual); - assert_eq!(report.source, InstallSource::Unknown); - // Carries both upgrade recipes and the releases URL. - let joined = report.messages.join("\n"); - assert!(joined.contains("npm install -g @agent-ix/quire-cli@latest")); - assert!(joined.contains("cargo install --git")); - assert!(joined.contains("releases")); - } - - // TC-087, FR-016-AC-1, FR-016-AC-5: `run_for_source(Cargo, --check)` - // reports git-branch tracking with `latest: None` — no cross-scheme version. - #[test] - fn cargo_check_reports_branch_tracking_without_a_version() { - let opts = SelfUpdateOpts { - check: true, - registry: None, - }; - let report = run_for_source(&CFG, &opts, InstallSource::Cargo).unwrap(); - assert_eq!(report.action, Action::Checked { latest: None }); - // The matrix row's "reports git-branch tracking" wording, asserted - // (#56, SR-006 FND-002): `latest: None` alone never checked that the - // report actually SAYS why there is no version to compare. - let joined = report.messages.join("\n"); - assert!( - joined.contains("track the git default branch"), - "the check report must explain branch tracking: {joined}" - ); - assert!( - joined.contains(CFG.cargo_git), - "the report names the tracked repository: {joined}" - ); - } -} diff --git a/tests/audit_static.rs b/tests/audit_static.rs index 8d3da89..812778c 100644 --- a/tests/audit_static.rs +++ b/tests/audit_static.rs @@ -97,15 +97,10 @@ fn tc092_every_unsafe_block_is_documented_and_the_gate_catches_violations() { .expect("the gate must accept the documented block"); } -// TC-093, FR-016-AC-5, FR-016-AC-6, StR-004-AC-2: the `self_update` engine is -// package-agnostic — it is driven by a config struct and imports nothing from -// quire's `io` or command context, so `commands/update.rs` stays the only -// quire-specific glue. Source inspection is the only way to reach this: no -// runtime path can observe an import that isn't there. -// -// The gate matches expanded module paths, not bare substrings (#56, SR-006 -// FND-003): the previous `contains("crate::io")` check was evaded by any -// grouped import (`use crate::{io, …}`), which contains no needle at all. +// TC-093, FR-016-AC-5, FR-016-AC-6, StR-004-AC-2: the update command imports +// the shared engine, supplies quire's coordinates, and contains no parser, +// renderer, or validator logic. The source audit also rejects a local copy of +// the engine. #[test] fn tc093_self_update_engine_is_package_agnostic() { quire_qualify::source::check_thin_boundary(&repo_root()) diff --git a/tests/cli_okf.rs b/tests/cli_okf.rs index 78d0504..47013e2 100644 --- a/tests/cli_okf.rs +++ b/tests/cli_okf.rs @@ -137,7 +137,7 @@ fn okf_missing_spec_root_is_a_named_error() { // IT-088 (agent-ix/quire-rs#110): a non-fatal bundle warning is emitted as a // WARNING in the machine surface. Every `report.warnings` entry used to go -// through `emit_diagnostic`, which hardcodes `"severity": "error"` — so under +// through the error diagnostic path, which hardcoded `"severity": "error"` — so under // `--diagnostics json` the severity contradicted the exit code, which was // correctly 0. The frontmatter-less file is the CR-048 warning that made this // visible, and the engine now distinguishes its two flavors by machine reason diff --git a/tools/quire-qualify/src/source.rs b/tools/quire-qualify/src/source.rs index 44eeef5..60cb697 100644 --- a/tools/quire-qualify/src/source.rs +++ b/tools/quire-qualify/src/source.rs @@ -422,40 +422,24 @@ fn audit_assurance(path: &Path, index: &SyntaxIndex, findings: &mut Vec fn audit_self_update(root: &Path, findings: &mut Vec) { let engine_path = root.join("src/self_update/mod.rs"); let rel = relative(root, &engine_path); - if let Some(index) = parse_index(root, &engine_path, findings) { - const FORBIDDEN: &[&str] = &[ - "crate::io", - "crate::commands", - "crate::Ctx", - "quire_cli::io", - "quire_cli::commands", - ]; - for reference in index.all_references() { - if let Some(resolved) = forbidden_reference(&index, &reference, FORBIDDEN) { - findings.push(Finding::new( - &rel, - reference.line, - format!("self-update engine reaches CLI-specific path `{resolved}`"), - )); - } - } - for import in &index.imports { - if import.glob && matches!(import.full.as_str(), "crate" | "quire_cli") { - findings.push(Finding::new( - &rel, - import.line, - format!( - "root glob import `{}` hides CLI-specific dependencies", - import.full - ), - )); - } - } + if engine_path.exists() { + findings.push(Finding::new( + &rel, + 0, + "local self-update engine duplicates ix-cli-kit", + )); } let glue_path = root.join("src/commands/update.rs"); let glue_rel = relative(root, &glue_path); if let Some(index) = parse_index(root, &glue_path, findings) { + if !index.has_resolved_suffix("ix_cli_kit::self_update") { + findings.push(Finding::new( + &glue_rel, + 0, + "update command must consume ix-cli-kit's self-update module", + )); + } const FORBIDDEN: &[&str] = &[ "quire_rs::parse_document", "quire_rs::validate", diff --git a/tools/quire-qualify/tests/source.rs b/tools/quire-qualify/tests/source.rs index 7b467b7..5100f38 100644 --- a/tools/quire-qualify/tests/source.rs +++ b/tools/quire-qualify/tests/source.rs @@ -11,8 +11,10 @@ fn write(path: &Path, value: &str) { fn minimum_repo(root: &Path) { write(&root.join("src/main.rs"), "fn main() {}\n"); write(&root.join("src/lib.rs"), "pub fn clean() {}\n"); - write(&root.join("src/self_update/mod.rs"), "pub fn update() {}\n"); - write(&root.join("src/commands/update.rs"), "pub fn run() {}\n"); + write( + &root.join("src/commands/update.rs"), + "use ix_cli_kit::self_update;\npub fn run() {}\n", + ); write( &root.join("src/commands/assurance.rs"), r#" @@ -40,6 +42,15 @@ fn rust_ast_catches_direct_aliased_and_nested_group_references() { minimum_repo(dir.path()); quire_qualify::source::check_thin_boundary(dir.path()).expect("clean fixture"); + let local_engine = dir.path().join("src/self_update/mod.rs"); + write(&local_engine, "pub fn update() {}\n"); + let findings = quire_qualify::source::thin_boundary_findings(dir.path()).unwrap(); + assert!(findings.iter().any(|finding| { + finding.path == Path::new("src/self_update/mod.rs") + && finding.reason.contains("duplicates ix-cli-kit") + })); + fs::remove_file(local_engine).unwrap(); + for mutation in [ "fn f() { quire_rs::parse_document(); }\n", "use quire_rs::parse_document as parse; fn f() { parse(); }\n",