From 95895fc5e3efe45e94eb62173d29b9a081f7d87f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Kova=C4=BE?= Date: Wed, 10 Jun 2026 20:21:29 +0200 Subject: [PATCH] fix: unify not-found errors to exit 3 (NotFoundError) (#18) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Source-not-found, missing-`extends`-parent, and in-compose bundle-not-found all raised UsageError (exit 2), contradicting the README "Exit codes" table which documents `3 = Source / bundle / parent not found`. Make the code match the contract: every "a referenced name/artifact doesn't exist" condition now raises NotFoundError (exit 3). Genuine validation/syntax errors (bare ref missing source qualifier, extends cycle) stay UsageError (exit 2). Sites changed to NotFoundError: - resolve.ts source(s) not found - compose.ts extends missing parent - compose.ts bundle not found (in-compose) — also resolves the latent inconsistency with the same condition at the top level (exec.ts), which already returned exit 3. Tests assert the exit code for all five sites (3 not-found -> 3, 2 validation -> 2). README table row 3 already describes this behavior, so no doc change is needed. BREAKING (pre-1.0): source-not-found and missing-parent now exit 3 instead of 2; scripts/CI wrappers branching on exit 2 for these will see the new code. --- src/bundle/compose.ts | 6 +++--- src/bundle/resolve.ts | 4 ++-- test/unit/bundle/compose.test.ts | 36 ++++++++++++++++++++++++++------ test/unit/bundle/resolve.test.ts | 29 ++++++++++++++++++------- 4 files changed, 56 insertions(+), 19 deletions(-) diff --git a/src/bundle/compose.ts b/src/bundle/compose.ts index 841525a..9f954a5 100644 --- a/src/bundle/compose.ts +++ b/src/bundle/compose.ts @@ -1,4 +1,4 @@ -import { UsageError } from "../errors.ts"; +import { NotFoundError, UsageError } from "../errors.ts"; import { ARTIFACT_KINDS } from "./kinds.ts"; import type { BundleManifest, BundleSettings } from "./manifest.ts"; @@ -6,7 +6,7 @@ export type ResolvedBundle = Omit; export function compose(name: string, index: Map): ResolvedBundle { if (!index.has(name)) { - throw new UsageError(`bundle '${name}' not found`); + throw new NotFoundError(`bundle '${name}' not found`); } const order = linearize(name, index); // order is oldest-first: ancestors before descendants. Merge left-to-right @@ -43,7 +43,7 @@ function linearize(start: string, index: Map): string[] } const cur = index.get(n); if (!cur) { - throw new UsageError( + throw new NotFoundError( `bundle '${start}' extends missing parent '${n}' (chain: ${[...stack, n].join(" → ")})`, ); } diff --git a/src/bundle/resolve.ts b/src/bundle/resolve.ts index a3086e5..c8b596b 100644 --- a/src/bundle/resolve.ts +++ b/src/bundle/resolve.ts @@ -1,6 +1,6 @@ import { existsSync } from "node:fs"; import { join } from "node:path"; -import { UsageError } from "../errors.ts"; +import { NotFoundError, UsageError } from "../errors.ts"; import { isDir } from "../target/walk.ts"; import type { ResolvedBundle } from "./compose.ts"; import { ARTIFACT_KINDS, type ArtifactKind } from "./kinds.ts"; @@ -50,7 +50,7 @@ export function resolveSources(bundle: ResolvedBundle, opts: ResolveOpts): Resol ); } if (missing.length > 0) { - throw new UsageError(`bundle '${bundle.name}': source(s) not found: ${missing.join(", ")}`); + throw new NotFoundError(`bundle '${bundle.name}': source(s) not found: ${missing.join(", ")}`); } if (opts.projectSkillsDir && isDir(opts.projectSkillsDir)) { diff --git a/test/unit/bundle/compose.test.ts b/test/unit/bundle/compose.test.ts index cad753a..8535d75 100644 --- a/test/unit/bundle/compose.test.ts +++ b/test/unit/bundle/compose.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { compose } from "../../../src/bundle/compose.ts"; import type { BundleManifest } from "../../../src/bundle/manifest.ts"; +import { CliError } from "../../../src/errors.ts"; function m(name: string, partial: Partial = {}): BundleManifest { return { name, sourcePath: `/x/${name}.md`, body: "", ...partial }; @@ -10,6 +11,16 @@ function index(...mans: BundleManifest[]): Map { return new Map(mans.map((x) => [x.name, x])); } +function thrown(fn: () => unknown): CliError { + try { + fn(); + } catch (e) { + if (e instanceof CliError) return e; + throw e; + } + throw new Error("expected function to throw"); +} + describe("compose", () => { it("returns the bundle as-is when extends is absent", () => { const ix = index(m("base", { skills: ["a", "b"] })); @@ -113,14 +124,27 @@ describe("compose", () => { expect(r.skills).toEqual(["g1", "p11", "p21"]); }); - it("missing parent → error names the missing bundle and chain", () => { - const ix = index(m("child", { extends: ["ghost"] })); - expect(() => compose("child", ix)).toThrow(/ghost/); + it("unknown bundle name → not found (exit 3)", () => { + const err = thrown(() => compose("ghost", index(m("base")))); + expect(err.name).toBe("NotFoundError"); + expect(err.exitCode).toBe(3); + expect(err.message).toMatch(/ghost.*not found/); }); - it("cycle in extends → error", () => { - const ix = index(m("a", { extends: ["b"] }), m("b", { extends: ["a"] })); - expect(() => compose("a", ix)).toThrow(/cycle/i); + it("missing parent → not found (exit 3), names the missing bundle and chain", () => { + const err = thrown(() => compose("child", index(m("child", { extends: ["ghost"] })))); + expect(err.name).toBe("NotFoundError"); + expect(err.exitCode).toBe(3); + expect(err.message).toMatch(/ghost/); + }); + + it("cycle in extends → usage error (exit 2), not not-found", () => { + const err = thrown(() => + compose("a", index(m("a", { extends: ["b"] }), m("b", { extends: ["a"] }))), + ); + expect(err.name).toBe("UsageError"); + expect(err.exitCode).toBe(2); + expect(err.message).toMatch(/cycle/i); }); }); diff --git a/test/unit/bundle/resolve.test.ts b/test/unit/bundle/resolve.test.ts index eacb514..a5e364f 100644 --- a/test/unit/bundle/resolve.test.ts +++ b/test/unit/bundle/resolve.test.ts @@ -3,8 +3,19 @@ import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import type { ResolvedBundle } from "../../../src/bundle/compose.ts"; import { resolveSources } from "../../../src/bundle/resolve.ts"; +import { CliError } from "../../../src/errors.ts"; import { cleanup, makeTmpDir } from "../../helpers/tmp.ts"; +function thrown(fn: () => unknown): CliError { + try { + fn(); + } catch (e) { + if (e instanceof CliError) return e; + throw e; + } + throw new Error("expected function to throw"); +} + describe("resolveSources", () => { let root: string; let roots: { @@ -60,18 +71,20 @@ describe("resolveSources", () => { expect(out.warnings).toEqual([]); }); - it("bare ref (no '/') in manifest → hints at missing source qualifier", () => { + it("bare ref (no '/') in manifest → usage error (exit 2), hints at missing source qualifier", () => { mkSubArtifact(roots.skills, "local", "tdd"); - expect(() => resolveSources(bundle({ skills: ["tdd"] }), { roots })).toThrow( - /missing source qualifier; use '\/'/, - ); + const err = thrown(() => resolveSources(bundle({ skills: ["tdd"] }), { roots })); + expect(err.name).toBe("UsageError"); + expect(err.exitCode).toBe(2); + expect(err.message).toMatch(/missing source qualifier; use '\/'/); }); - it("qualified-but-missing ref → original 'not found' message (no hint)", () => { + it("qualified-but-missing ref → not found (exit 3), original 'not found' message (no hint)", () => { mkSubArtifact(roots.skills, "local", "tdd"); - expect(() => resolveSources(bundle({ skills: ["local/ghost"] }), { roots })).toThrow( - /source\(s\) not found: skills\/local\/ghost/, - ); + const err = thrown(() => resolveSources(bundle({ skills: ["local/ghost"] }), { roots })); + expect(err.name).toBe("NotFoundError"); + expect(err.exitCode).toBe(3); + expect(err.message).toMatch(/source\(s\) not found: skills\/local\/ghost/); }); it("resolves all artifact kinds independently", () => {