From f4bb341bc182f96a922775879a4dea9c6c9a8e1a Mon Sep 17 00:00:00 2001 From: Nahiyan Khan Date: Mon, 29 Jun 2026 10:48:51 -0400 Subject: [PATCH] refactor(ghost): consistent CLI exit-code contract Normalize exit codes so an agent can branch on them. Unexpected errors exit 1; caller mistakes exit 2. The wrinkle was that several genuine usage errors are surfaced by throwing from deep helpers (path validation, byte budgets, overwrite guards, bad --agent/--template), so a flat catch code mislabels them. Introduce a typed UsageError (ghost-core) carrying exitCode 2 and a shared failFromError() that exits with a thrown error's exitCode or 1 otherwise, then route every command catch through it. A missing package now throws UsageError with a 'ghost init' hint instead of leaking a raw ENOENT, and the exit-code contract is documented in the CLI reference. --- .changeset/exit-code-contract.md | 8 +++++ apps/docs/src/content/docs/cli-reference.mdx | 15 +++++++++ packages/ghost/src/cli.ts | 13 ++++---- packages/ghost/src/commands/checks-command.ts | 6 ++-- packages/ghost/src/commands/errors.ts | 22 +++++++++++++ .../src/commands/fingerprint-commands.ts | 16 +++------ packages/ghost/src/commands/gather-command.ts | 6 ++-- packages/ghost/src/commands/init-command.ts | 6 ++-- .../ghost/src/commands/migrate-command.ts | 9 +++-- packages/ghost/src/commands/review-packet.ts | 3 +- packages/ghost/src/commands/skill-command.ts | 12 +++---- packages/ghost/src/ghost-core/errors.ts | 33 +++++++++++++++++++ packages/ghost/src/ghost-core/index.ts | 2 ++ .../src/scan/fingerprint-package-layers.ts | 14 +++++++- .../ghost/src/scan/fingerprint-package.ts | 7 ++-- packages/ghost/src/scan/package-paths.ts | 7 ++-- packages/ghost/test/cli.test.ts | 19 +++++++++++ 17 files changed, 148 insertions(+), 50 deletions(-) create mode 100644 .changeset/exit-code-contract.md create mode 100644 packages/ghost/src/commands/errors.ts create mode 100644 packages/ghost/src/ghost-core/errors.ts diff --git a/.changeset/exit-code-contract.md b/.changeset/exit-code-contract.md new file mode 100644 index 00000000..0b92a47f --- /dev/null +++ b/.changeset/exit-code-contract.md @@ -0,0 +1,8 @@ +--- +"@anarchitecture/ghost": patch +--- + +Make CLI exit codes consistent so an agent can branch on them: unexpected +errors exit `1`, caller mistakes (bad flags, invalid environment, refused +overwrites) exit `2` via a typed `UsageError`, and a missing package now exits +`2` with a `ghost init` hint instead of leaking a raw filesystem error. diff --git a/apps/docs/src/content/docs/cli-reference.mdx b/apps/docs/src/content/docs/cli-reference.mdx index 0ae8df0b..6cf22357 100644 --- a/apps/docs/src/content/docs/cli-reference.mdx +++ b/apps/docs/src/content/docs/cli-reference.mdx @@ -31,6 +31,21 @@ The canonical fingerprint is a `.ghost/` directory tree of prose nodes: The command tables below are generated from the CLI source. Run `pnpm dump:cli-help` after command or flag changes. +#### Exit codes + +Every command follows one contract, so an agent can branch on the exit code: + +| Code | Meaning | +| --- | --- | +| `0` | Success. | +| `1` | The command ran but the result is unhappy: `validate` found issues, or an unexpected error was thrown. | +| `2` | The command was called wrong: a bad flag or argument, a node or surface that is not in the package (`ERR_UNKNOWN_SURFACE`), or a missing package for a command that needs one to do its work (`gather`, `checks`, `review`). | +| `3` | A command-specific refusal — currently only `skill install` when a skill is already present (pass `--force`). | + +Reporting commands treat a missing package as a state to report, not a usage +error: `validate` records it as a finding (exit `1`), and `scan`/`signals` +report what they can (exit `0`). + diff --git a/packages/ghost/src/cli.ts b/packages/ghost/src/cli.ts index eeced85b..87df9e08 100644 --- a/packages/ghost/src/cli.ts +++ b/packages/ghost/src/cli.ts @@ -5,8 +5,10 @@ import { dirname, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import { promisify } from "node:util"; import { cac } from "cac"; +import { UsageError } from "#ghost-core"; import { registerChecksCommand } from "./commands/checks-command.js"; import { formatGhostHelp } from "./commands/command-discovery.js"; +import { failFromError } from "./commands/errors.js"; import { registerFingerprintCommands } from "./commands/fingerprint-commands.js"; import { registerGatherCommand } from "./commands/gather-command.js"; import { registerMigrateCommand } from "./commands/migrate-command.js"; @@ -99,10 +101,7 @@ export function buildCli(): ReturnType { process.exit(2); return; } - console.error( - `Error: ${err instanceof Error ? err.message : String(err)}`, - ); - process.exit(2); + failFromError(err); } }); @@ -135,14 +134,14 @@ function parsePositiveIntegerOption( ): number | undefined { if (value === undefined) return undefined; if (typeof value !== "string" && typeof value !== "number") { - throw new Error(`${flagName} must be a positive integer`); + throw new UsageError(`${flagName} must be a positive integer`); } if (typeof value === "string" && value.trim() === "") { - throw new Error(`${flagName} must be a positive integer`); + throw new UsageError(`${flagName} must be a positive integer`); } const parsed = Number(value); if (!Number.isSafeInteger(parsed) || parsed < 1) { - throw new Error(`${flagName} must be a positive integer`); + throw new UsageError(`${flagName} must be a positive integer`); } return parsed; } diff --git a/packages/ghost/src/commands/checks-command.ts b/packages/ghost/src/commands/checks-command.ts index d344c664..7f7d7ce4 100644 --- a/packages/ghost/src/commands/checks-command.ts +++ b/packages/ghost/src/commands/checks-command.ts @@ -8,6 +8,7 @@ import { import { resolveFingerprintPackage } from "../fingerprint.js"; import { loadChecksDir } from "../scan/checks-dir.js"; import { loadFingerprintPackage } from "../scan/fingerprint-package.js"; +import { failFromError } from "./errors.js"; import { guardSurfaces } from "./surface-guard.js"; function parseSurfaceIds(value: unknown): string[] { @@ -108,10 +109,7 @@ export function registerChecksCommand(cli: CAC): void { } process.exit(0); } catch (err) { - console.error( - `Error: ${err instanceof Error ? err.message : String(err)}`, - ); - process.exit(1); + failFromError(err); } }); } diff --git a/packages/ghost/src/commands/errors.ts b/packages/ghost/src/commands/errors.ts new file mode 100644 index 00000000..6a6e5612 --- /dev/null +++ b/packages/ghost/src/commands/errors.ts @@ -0,0 +1,22 @@ +import { EXIT } from "#ghost-core"; + +/** + * Report a thrown error and exit. A `UsageError` (or anything carrying a numeric + * `exitCode`) exits with that code; everything else is an unexpected crash and + * exits `1`. Pass `stream` to match a command's existing output channel. + */ +export function failFromError( + err: unknown, + stream: "stderr" | "stdout" = "stderr", +): never { + const message = err instanceof Error ? err.message : String(err); + const line = `Error: ${message}\n`; + if (stream === "stdout") process.stdout.write(line); + else process.stderr.write(line); + + const exitCode = + typeof (err as { exitCode?: unknown })?.exitCode === "number" + ? (err as { exitCode: number }).exitCode + : EXIT.failure; + process.exit(exitCode); +} diff --git a/packages/ghost/src/commands/fingerprint-commands.ts b/packages/ghost/src/commands/fingerprint-commands.ts index c9b9e54d..80358b2c 100644 --- a/packages/ghost/src/commands/fingerprint-commands.ts +++ b/packages/ghost/src/commands/fingerprint-commands.ts @@ -8,6 +8,7 @@ import { } from "../fingerprint.js"; import { detectFileKind, lintDetectedFileKind } from "../scan/file-kind.js"; import { resolveGhostDirDefault, scanStatus, signals } from "../scan/index.js"; +import { failFromError } from "./errors.js"; import { registerInitCommand } from "./init-command.js"; /** @@ -50,10 +51,7 @@ export function registerFingerprintCommands(cli: CAC): void { process.exit(report.errors > 0 ? 1 : 0); } catch (err) { - console.error( - `Error: ${err instanceof Error ? err.message : String(err)}`, - ); - process.exit(2); + failFromError(err); } }); @@ -114,10 +112,7 @@ export function registerFingerprintCommands(cli: CAC): void { } process.exit(0); } catch (err) { - console.error( - `Error: ${err instanceof Error ? err.message : String(err)}`, - ); - process.exit(2); + failFromError(err); } }); @@ -134,10 +129,7 @@ export function registerFingerprintCommands(cli: CAC): void { process.stdout.write(`${JSON.stringify(out, null, 2)}\n`); process.exit(0); } catch (err) { - process.stderr.write( - `Error: ${err instanceof Error ? err.message : String(err)}\n`, - ); - process.exit(2); + failFromError(err); } }); } diff --git a/packages/ghost/src/commands/gather-command.ts b/packages/ghost/src/commands/gather-command.ts index 522daa52..4d846a9b 100644 --- a/packages/ghost/src/commands/gather-command.ts +++ b/packages/ghost/src/commands/gather-command.ts @@ -11,6 +11,7 @@ import { } from "#ghost-core"; import { resolveFingerprintPackage } from "../fingerprint.js"; import { loadFingerprintPackage } from "../scan/fingerprint-package.js"; +import { failFromError } from "./errors.js"; export function registerGatherCommand(cli: CAC): void { cli @@ -100,10 +101,7 @@ export function registerGatherCommand(cli: CAC): void { } process.exit(0); } catch (err) { - console.error( - `Error: ${err instanceof Error ? err.message : String(err)}`, - ); - process.exit(1); + failFromError(err); } }); } diff --git a/packages/ghost/src/commands/init-command.ts b/packages/ghost/src/commands/init-command.ts index 7b942483..4883c8c7 100644 --- a/packages/ghost/src/commands/init-command.ts +++ b/packages/ghost/src/commands/init-command.ts @@ -1,6 +1,7 @@ import type { CAC } from "cac"; import { initFingerprintPackage } from "../fingerprint.js"; import { resolveGhostDirDefault } from "../scan/index.js"; +import { failFromError } from "./errors.js"; export function registerInitCommand(cli: CAC): void { cli @@ -53,10 +54,7 @@ export function registerInitCommand(cli: CAC): void { } process.exit(0); } catch (err) { - console.error( - `Error: ${err instanceof Error ? err.message : String(err)}`, - ); - process.exit(2); + failFromError(err); } }); } diff --git a/packages/ghost/src/commands/migrate-command.ts b/packages/ghost/src/commands/migrate-command.ts index 23f0673e..f228ad28 100644 --- a/packages/ghost/src/commands/migrate-command.ts +++ b/packages/ghost/src/commands/migrate-command.ts @@ -2,6 +2,7 @@ import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; import { dirname, join } from "node:path"; import type { CAC } from "cac"; import { parse as parseYaml } from "yaml"; +import { UsageError } from "#ghost-core"; import { resolveFingerprintPackage } from "../fingerprint.js"; import { looksLegacy, @@ -10,6 +11,7 @@ import { migratedNodeFiles, migrateLegacyPackage, } from "../scan/index.js"; +import { failFromError } from "./errors.js"; export function registerMigrateCommand(cli: CAC): void { cli @@ -68,10 +70,7 @@ export function registerMigrateCommand(cli: CAC): void { ); process.exit(0); } catch (err) { - console.error( - `Error: ${err instanceof Error ? err.message : String(err)}`, - ); - process.exit(1); + failFromError(err); } }); } @@ -121,7 +120,7 @@ async function writeMigrated( flag: force ? "w" : "wx", }).catch((err: unknown) => { if (!force && isExisting(err)) { - throw new Error( + throw new UsageError( `Refusing to overwrite ${path}. Pass --force to rewrite the package in place.`, ); } diff --git a/packages/ghost/src/commands/review-packet.ts b/packages/ghost/src/commands/review-packet.ts index 49449e8f..27bb241d 100644 --- a/packages/ghost/src/commands/review-packet.ts +++ b/packages/ghost/src/commands/review-packet.ts @@ -3,6 +3,7 @@ import { type RoutedCheck, resolveGraphSlice, selectChecksForSurfaces, + UsageError, } from "#ghost-core"; import { loadChecksDir } from "../scan/checks-dir.js"; import { @@ -90,7 +91,7 @@ function budgetDiff( maxDiffBytes = DEFAULT_REVIEW_MAX_DIFF_BYTES, ): { diff: string; budgets: ReviewPacketBudgets; truncated: boolean } { if (!Number.isSafeInteger(maxDiffBytes) || maxDiffBytes < 1) { - throw new Error("--max-diff-bytes must be a positive integer"); + throw new UsageError("--max-diff-bytes must be a positive integer"); } const bytes = Buffer.byteLength(diffText, "utf-8"); if (bytes <= maxDiffBytes) { diff --git a/packages/ghost/src/commands/skill-command.ts b/packages/ghost/src/commands/skill-command.ts index 9ac9c30f..33fd7055 100644 --- a/packages/ghost/src/commands/skill-command.ts +++ b/packages/ghost/src/commands/skill-command.ts @@ -4,7 +4,8 @@ import { homedir } from "node:os"; import { dirname, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import type { CAC } from "cac"; -import { loadSkillBundle } from "#ghost-core"; +import { loadSkillBundle, UsageError } from "#ghost-core"; +import { failFromError } from "./errors.js"; // The bundle assets are copied to `dist/skill-bundle` (sibling of `commands/`). const SKILL_BUNDLE_ROOT = fileURLToPath( @@ -65,10 +66,7 @@ export function registerSkillCommand(cli: CAC): void { for (const file of written) process.stdout.write(` ${file}\n`); process.exit(0); } catch (err) { - console.error( - `Error: ${err instanceof Error ? err.message : String(err)}`, - ); - process.exit(2); + failFromError(err); } }); } @@ -81,7 +79,9 @@ function parseAgent(raw: unknown): SupportedAgent | undefined { ) { return raw as SupportedAgent; } - throw new Error(`--agent must be one of: ${SUPPORTED_AGENTS.join(", ")}`); + throw new UsageError( + `--agent must be one of: ${SUPPORTED_AGENTS.join(", ")}`, + ); } function detectAgent(): SupportedAgent { diff --git a/packages/ghost/src/ghost-core/errors.ts b/packages/ghost/src/ghost-core/errors.ts new file mode 100644 index 00000000..ca592c11 --- /dev/null +++ b/packages/ghost/src/ghost-core/errors.ts @@ -0,0 +1,33 @@ +/** + * The CLI exit-code contract, in one place so every command agrees and an agent + * can branch on the code: + * + * - `0` success + * - `1` ran, but unhappy: a lint/validate finding, or an unexpected error + * - `2` called wrong: a bad flag or argument, a missing package, an unknown + * node or surface + * - `3` a command-specific refusal (e.g. `skill install` without `--force`) + * + * Usage errors (`2`) are often surfaced by throwing from deep in a helper + * (path validation, byte budgets, overwrite guards). Throwing `UsageError` + * instead of a plain `Error` lets the shared catch tell those apart from a + * genuine crash, so the contract holds without each call site picking a code. + */ +export const EXIT = { + ok: 0, + failure: 1, + usage: 2, +} as const; + +/** + * A caller-facing "you used it wrong" error: bad flag or argument, missing + * package, invalid environment, or a refused overwrite. Reported at exit code + * 2, distinct from an unexpected crash (1). + */ +export class UsageError extends Error { + readonly exitCode = EXIT.usage; + constructor(message: string) { + super(message); + this.name = "UsageError"; + } +} diff --git a/packages/ghost/src/ghost-core/index.ts b/packages/ghost/src/ghost-core/index.ts index 772955ef..8abbcedb 100644 --- a/packages/ghost/src/ghost-core/index.ts +++ b/packages/ghost/src/ghost-core/index.ts @@ -18,6 +18,8 @@ export { type RoutedCheck, selectChecksForSurfaces, } from "./check/index.js"; +// --- CLI exit-code contract --- +export { EXIT, UsageError } from "./errors.js"; // --- Fingerprint package filenames --- // --- Graph (in-memory fingerprint node graph) --- export { diff --git a/packages/ghost/src/scan/fingerprint-package-layers.ts b/packages/ghost/src/scan/fingerprint-package-layers.ts index 16baf9f5..97cbe98c 100644 --- a/packages/ghost/src/scan/fingerprint-package-layers.ts +++ b/packages/ghost/src/scan/fingerprint-package-layers.ts @@ -7,6 +7,7 @@ import { GhostFingerprintPackageManifestSchema, type GhostGraphNode, lintGraph, + UsageError, } from "#ghost-core"; import { isMissingPathError } from "../internal/fs.js"; import { @@ -22,7 +23,18 @@ const LEGACY_FACET_FILES = ["intent.yml", "inventory.yml", "composition.yml"]; export async function loadFingerprintPackage( paths: FingerprintPackagePaths, ): Promise { - const manifestRaw = await readFile(paths.manifest, "utf-8"); + let manifestRaw: string; + try { + manifestRaw = await readFile(paths.manifest, "utf-8"); + } catch (err) { + // A missing package is a usage error (run `ghost init`), not a crash. + if (isMissingPathError(err)) { + throw new UsageError( + `No Ghost fingerprint package found at ${paths.packageDir} (expected manifest.yml). Run \`ghost init\` or pass --package .`, + ); + } + throw err; + } const manifest = parseManifest(manifestRaw, "manifest.yml"); // Legacy facet packages no longer load directly — guide to `ghost migrate`. diff --git a/packages/ghost/src/scan/fingerprint-package.ts b/packages/ghost/src/scan/fingerprint-package.ts index 006eb6b2..f2a7653d 100644 --- a/packages/ghost/src/scan/fingerprint-package.ts +++ b/packages/ghost/src/scan/fingerprint-package.ts @@ -4,6 +4,7 @@ import { type GhostFingerprintPackageManifest, type GhostGraph, lintGraph, + UsageError, } from "#ghost-core"; import { isExistingPathError, isMissingPathError } from "../internal/fs.js"; import { @@ -79,7 +80,7 @@ export async function initFingerprintPackage( const templateName = options.template ?? DEFAULT_TEMPLATE_NAME; const template = getInitTemplate(templateName); if (!template) { - throw new Error( + throw new UsageError( `Unknown init template '${templateName}'. Available: ${listInitTemplates().join(", ")}.`, ); } @@ -120,7 +121,7 @@ async function writeInitFile( }); } catch (err) { if (!force && isExistingPathError(err)) { - throw new Error( + throw new UsageError( `Refusing to overwrite existing Ghost fingerprint file:\n ${path}\nPass --force to overwrite.`, ); } @@ -141,7 +142,7 @@ async function assertInitDoesNotOverwrite(paths: string[]): Promise { } if (existing.length > 0) { const formatted = existing.map((path) => ` ${path}`).join("\n"); - throw new Error( + throw new UsageError( `Refusing to overwrite existing Ghost fingerprint file(s):\n${formatted}\nPass --force to overwrite.`, ); } diff --git a/packages/ghost/src/scan/package-paths.ts b/packages/ghost/src/scan/package-paths.ts index 6467d2ed..456d7265 100644 --- a/packages/ghost/src/scan/package-paths.ts +++ b/packages/ghost/src/scan/package-paths.ts @@ -1,6 +1,7 @@ import { execFile } from "node:child_process"; import { isAbsolute, resolve } from "node:path"; import { promisify } from "node:util"; +import { UsageError } from "#ghost-core"; import { FINGERPRINT_PACKAGE_DIR } from "./constants.js"; const execFileAsync = promisify(execFile); @@ -35,14 +36,14 @@ export function normalizeGhostDir(ghostDir = FINGERPRINT_PACKAGE_DIR): string { .replace(/\/+/g, "/") .replace(/\/$/g, ""); if (!normalized) { - throw new Error("GHOST_PACKAGE_DIR must not be empty"); + throw new UsageError("GHOST_PACKAGE_DIR must not be empty"); } if ( isAbsolute(ghostDir) || normalized.startsWith("/") || /^[A-Za-z]:/.test(normalized) ) { - throw new Error("GHOST_PACKAGE_DIR must be a relative directory path"); + throw new UsageError("GHOST_PACKAGE_DIR must be a relative directory path"); } const segments = normalized.split("/"); if ( @@ -50,7 +51,7 @@ export function normalizeGhostDir(ghostDir = FINGERPRINT_PACKAGE_DIR): string { (segment) => segment === "." || segment === ".." || segment === "", ) ) { - throw new Error( + throw new UsageError( "GHOST_PACKAGE_DIR must not contain '.', '..', or empty path segments", ); } diff --git a/packages/ghost/test/cli.test.ts b/packages/ghost/test/cli.test.ts index f29c38fc..1f85e061 100644 --- a/packages/ghost/test/cli.test.ts +++ b/packages/ghost/test/cli.test.ts @@ -303,6 +303,25 @@ describe("ghost CLI", () => { expect(init.stderr).toContain("GHOST_PACKAGE_DIR must not contain"); }); + it("exits 2 for a usage error surfaced by a thrown UsageError", async () => { + // A bad flag value is a usage error even when it throws from deep in a + // helper, not an unexpected crash: it must exit 2, not 1. + const bad = await runCli(["skill", "install", "--agent", "nope"], dir, { + allowNoExit: true, + }); + expect(bad.code).toBe(2); + expect(bad.stderr).toContain("--agent must be one of"); + }); + + it("exits 2 with guidance when no fingerprint package is present", async () => { + // A missing package is a usage error (run `ghost init`), not a raw crash. + const result = await runCli(["gather", "core"], dir, { + allowNoExit: true, + }); + expect(result.code).toBe(2); + expect(result.stderr).toContain("No Ghost fingerprint package found"); + }); + it("uses GHOST_PACKAGE_DIR as the default package lookup for scan", async () => { await runCli(["init", "--package", ".agents/ghost"], dir);