Skip to content
Merged
4 changes: 4 additions & 0 deletions src/bundle/compose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ export function compose(name: string, index: Map<string, BundleManifest>): Resol
return merged!;
}

export function composeChain(name: string, index: Map<string, BundleManifest>): string[] {
return linearize(name, index);
}

function stripExtends(b: BundleManifest): ResolvedBundle {
const { extends: _e, ...rest } = b;
return rest;
Expand Down
13 changes: 11 additions & 2 deletions src/bundle/discover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ export interface BundleEntry {
path: string;
manifest?: BundleManifest;
malformed: boolean;
error?: string;
warnings?: string[];
shadowed: boolean;
}

Expand Down Expand Up @@ -43,17 +45,24 @@ function scanScope(dir: string, scope: BundleScope): BundleEntry[] {
const name = file.slice(0, -3);
let manifest: BundleManifest | undefined;
let malformed = false;
let error: string | undefined;
let warnings: string[] | undefined;
try {
manifest = loadManifest(path).manifest;
} catch {
const result = loadManifest(path);
manifest = result.manifest;
warnings = result.warnings;
} catch (e) {
malformed = true;
error = e instanceof Error ? e.message : String(e);
}
out.push({
name: manifest?.name ?? name,
scope,
path,
...(manifest !== undefined ? { manifest } : {}),
malformed,
...(error !== undefined ? { error } : {}),
...(warnings !== undefined && warnings.length > 0 ? { warnings } : {}),
shadowed: false,
});
}
Expand Down
24 changes: 19 additions & 5 deletions src/bundle/exec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { homedir } from "node:os";
import { dirname, join } from "node:path";
import { NotFoundError } from "../errors.ts";
import { NotFoundError, UsageError } from "../errors.ts";
import { computeClaudeArgs } from "./claude-args.ts";
import { compile } from "./compile.ts";
import { type ResolvedBundle, compose } from "./compose.ts";
import { type ResolvedBundle, compose, composeChain } from "./compose.ts";
import { discoverBundles } from "./discover.ts";
import {
artifactRoots,
Expand Down Expand Up @@ -76,21 +76,33 @@ export function resolveBundle(
name: string,
index: BundleIndex,
env: NodeJS.ProcessEnv,
): { resolved: ResolvedBundle; sources: ResolvedSources } {
): { resolved: ResolvedBundle; sources: ResolvedSources; warnings: string[] } {
const ix = new Map<string, BundleManifest>();
for (const e of index.entries) {
if (e.manifest && !ix.has(e.name)) ix.set(e.name, e.manifest);
}
if (!ix.has(name)) {
const malformed = index.entries.find((e) => e.malformed && e.name === name);
if (malformed) {
throw new UsageError(malformed.error ?? `bundle '${name}' is malformed`);
}
throw new NotFoundError(`bundle '${name}' not found`);
}
const resolved = compose(name, ix);
const chain = new Set(composeChain(name, ix));
const warnings = [
...new Set(
index.entries
.filter((e) => !e.shadowed && chain.has(e.name))
.flatMap((e) => e.warnings ?? []),
),
];
const projectSkillsDir = join(dirname(index.projectDir), "skills");
const sources = resolveSources(resolved, {
roots: artifactRoots(env),
projectSkillsDir,
});
return { resolved, sources };
return { resolved, sources, warnings };
}

export interface PrepareOpts {
Expand All @@ -109,12 +121,13 @@ export interface PreparedInvocation {
args: string[];
env: NodeJS.ProcessEnv;
cacheDir: string;
warnings: string[];
}

export function prepareBundleInvocation(opts: PrepareOpts): PreparedInvocation {
const { name, claudeArgs, env, cwd } = opts;
const index = opts.preloadedIndex ?? loadBundleIndex(env, cwd);
const { resolved, sources } = resolveBundle(name, index, env);
const { resolved, sources, warnings } = resolveBundle(name, index, env);
const { cacheDir, version } = compile(resolved, sources, {
cacheRoot: bundleCacheRoot(env),
...(opts.onBuild ? { onBuild: opts.onBuild } : {}),
Expand All @@ -133,5 +146,6 @@ export function prepareBundleInvocation(opts: PrepareOpts): PreparedInvocation {
args: [...computeClaudeArgs(resolved, cacheDir), ...claudeArgs],
env: spawnEnv,
cacheDir,
warnings,
};
}
9 changes: 7 additions & 2 deletions src/bundle/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,17 @@ function formatGroups(entries: BundleEntry[], dirs: ListScopeDirs, opts: RenderL
return `${out.join("\n")}\n`;
}

function malformedReason(error: string | undefined): string {
if (!error) return "malformed";
return error.split("\n", 1)[0]!.replace(/^bundle .*?: /, "");
}

function formatTable(rows: BundleEntry[], opts: RenderListOpts): string[] {
const headers = ["NAME", "DESCRIPTION", "EXTENDS", "PINNED"];
const pinned = new Set(opts.pinnedNames ?? []);
const data: string[][] = rows.map((r) => [
r.name,
r.manifest?.description ?? "—",
r.malformed ? `${r.name} ⚠` : r.name,
r.malformed ? malformedReason(r.error) : (r.manifest?.description ?? "—"),
(r.manifest?.extends ?? []).join(", ") || "—",
pinned.has(r.name) ? (r.name === opts.defaultName ? "yes*" : "yes") : "—",
]);
Expand Down
19 changes: 16 additions & 3 deletions src/bundle/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,24 @@ export interface ManifestResult {

const NAME_RE = /^[a-z][a-z0-9-]{1,40}$/;

function parseFrontmatter(
path: string,
raw: string,
): { data: Record<string, unknown>; body: string } {
try {
const parsed = matter(raw);
return { data: parsed.data as Record<string, unknown>, body: parsed.content };
} catch (e) {
const first = e instanceof Error ? e.message.split("\n", 1)[0] : String(e);
throw new UsageError(
`bundle ${path}: invalid YAML in frontmatter: ${first}\nHint: if a value contains \`{...}\`, \`[...]\`, or unquoted colons (e.g. code snippets in description), wrap it as a YAML block scalar:\n description: >-\n your text here`,
);
}
}

export function loadManifest(path: string): ManifestResult {
const raw = readFileSync(path, "utf8");
const parsed = matter(raw);
const data = parsed.data as Record<string, unknown>;
const body = parsed.content;
const { data, body } = parseFrontmatter(path, raw);

if (typeof data.name !== "string" || data.name.length === 0) {
throw new UsageError(`bundle ${path}: 'name' is required`);
Expand Down
6 changes: 4 additions & 2 deletions src/bundle/show.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { ResolvedSources } from "./resolve.ts";

export interface ShowOpts {
projectMcpPath?: string;
warnings?: string[];
}

export function renderShow(
Expand All @@ -25,9 +26,10 @@ export function renderShow(
sections.push("## mcp");
sections.push(formatMcpDiff(bundle, sources, opts));

if (sources.warnings.length > 0) {
const warnings = [...new Set([...sources.warnings, ...(opts.warnings ?? [])])];
if (warnings.length > 0) {
sections.push("## warnings");
sections.push(sources.warnings.map((w) => `- ${w}`).join("\n"));
sections.push(warnings.map((w) => `- ${w}`).join("\n"));
}

return `${sections.join("\n\n")}\n`;
Expand Down
29 changes: 20 additions & 9 deletions src/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ async function runBundleRun(rest: string[], env: NodeJS.ProcessEnv, cwd: string)
// otherwise look like a freeze before claude's first output appears.
onBuild: () => process.stderr.write(`building bundle '${resolvedName}'…\n`),
});
emitWarnings(prepared.warnings);
const result = spawnSync(prepared.command, prepared.args, {
env: prepared.env as NodeJS.ProcessEnv,
stdio: "inherit",
Expand Down Expand Up @@ -368,21 +369,30 @@ async function runBundleApply(
name = picked.name;
}

const built = buildBundle(name, index, env);
const { cacheDir, warnings } = buildBundle(name, index, env);
emitWarnings(warnings);
const path = writePin(cwd, homedir(), name);
process.stdout.write(`pinned bundle '${name}' at ${path}\n`);
process.stdout.write(`built ${built}\n`);
process.stdout.write(`built ${cacheDir}\n`);
return 0;
}

function emitWarnings(warnings: string[]): void {
for (const w of warnings) process.stderr.write(`${w}\n`);
}

function buildBundle(
name: string,
index: BundleIndex,
env: NodeJS.ProcessEnv,
forceRebuild = false,
): string {
const { resolved, sources } = resolveBundle(name, index, env);
return compile(resolved, sources, { cacheRoot: bundleCacheRoot(env), forceRebuild }).cacheDir;
): { cacheDir: string; warnings: string[] } {
const { resolved, sources, warnings } = resolveBundle(name, index, env);
const { cacheDir } = compile(resolved, sources, {
cacheRoot: bundleCacheRoot(env),
forceRebuild,
});
return { cacheDir, warnings };
}

function runBundleUnpin(cwd: string): number {
Expand Down Expand Up @@ -425,8 +435,9 @@ async function runBundleBuild(
if (typeof picked === "number") return picked;
name = picked;
}
const built = buildBundle(name, index, env, noCache);
process.stdout.write(`${built}\n`);
const { cacheDir, warnings } = buildBundle(name, index, env, noCache);
emitWarnings(warnings);
process.stdout.write(`${cacheDir}\n`);
return 0;
}

Expand All @@ -438,9 +449,9 @@ async function runBundleShow(rest: string[], env: NodeJS.ProcessEnv, cwd: string
if (typeof picked === "number") return picked;
name = picked;
}
const { resolved, sources } = resolveBundle(name, index, env);
const { resolved, sources, warnings } = resolveBundle(name, index, env);
const projectMcpPath = join(cwd, ".mcp.json");
process.stdout.write(renderShow(resolved, sources, { projectMcpPath }));
process.stdout.write(renderShow(resolved, sources, { projectMcpPath, warnings }));
return 0;
}

Expand Down
61 changes: 61 additions & 0 deletions test/integration/cli.bundle-malformed.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { spawnSync } from "node:child_process";
import { existsSync, mkdirSync, writeFileSync } from "node:fs";
import { dirname, join, resolve } from "node:path";
import { fileURLToPath } from "node:url";
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { cleanup, makeTmpDir } from "../helpers/tmp.ts";

const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), "..", "..");
const CLI = join(ROOT, "dist", "cli.js");

describe("cli bundle exit codes", () => {
let root: string;
let artifacts: string;
let bundles: string;

beforeEach(() => {
if (!existsSync(CLI)) {
throw new Error(`dist/cli.js not built — run \`npm run build\` first (looked at ${CLI})`);
}
root = makeTmpDir();
artifacts = join(root, "artifacts");
bundles = join(artifacts, "bundles");
mkdirSync(bundles, { recursive: true });
});
afterEach(() => cleanup(root));

function runCli(args: string[]): { status: number; stdout: string; stderr: string } {
const r = spawnSync("node", [CLI, ...args], {
encoding: "utf8",
cwd: root,
env: {
...process.env,
HOME: root,
UMBEL_ARTIFACTS_DIR: artifacts,
UMBEL_CACHE_DIR: join(root, "cache"),
},
});
return { status: r.status ?? -1, stdout: r.stdout, stderr: r.stderr };
}

it("exit 2 with the real validation message on a malformed bundle", () => {
writeFileSync(join(bundles, "bad.md"), "---\nname: bad\nsettings:\n notAllowed: 1\n---\n");
const r = runCli(["build", "bad"]);
expect(r.status).toBe(2);
expect(r.stderr).toMatch(/not in the whitelist/);
expect(r.stderr).not.toMatch(/not found/);
});

it("exit 3 on a genuinely nonexistent bundle name", () => {
const r = runCli(["build", "ghost"]);
expect(r.status).toBe(3);
expect(r.stderr).toMatch(/not found/);
});

it("exit 0 with a stderr warning on an unknown frontmatter field", () => {
writeFileSync(join(bundles, "typo.md"), "---\nname: typo\nbogusKey: 123\n---\nbody\n");
const r = runCli(["build", "typo"]);
expect(r.status).toBe(0);
expect(r.stderr).toMatch(/unknown field 'bogusKey'/);
});
});
12 changes: 12 additions & 0 deletions test/unit/bundle/compose.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,15 @@ describe("compose", () => {
});

import type { ResolvedBundle } from "../../../src/bundle/compose.ts";
import { composeChain } from "../../../src/bundle/compose.ts";

describe("composeChain", () => {
it("composeChain returns the linearized extends chain, ancestors first", () => {
const ix = new Map<string, BundleManifest>([
["base", { name: "base", body: "", sourcePath: "base.md" }],
["mid", { name: "mid", extends: ["base"], body: "", sourcePath: "mid.md" }],
["leaf", { name: "leaf", extends: ["mid"], body: "", sourcePath: "leaf.md" }],
]);
expect(composeChain("leaf", ix)).toEqual(["base", "mid", "leaf"]);
});
});
26 changes: 26 additions & 0 deletions test/unit/bundle/discover.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,30 @@ describe("discoverBundles", () => {
const out = discoverBundles({ userDir, projectDir });
expect(out.map((e) => e.name)).toEqual(["ok"]);
});

it("preserves the validation error on a malformed entry", () => {
mkdirSync(userDir, { recursive: true });
writeFileSync(join(userDir, "bad.md"), "---\nname: bad\nsettings:\n notAllowed: 1\n---\n");
const out = discoverBundles({ userDir, projectDir });
const bad = out.find((e) => e.name === "bad");
expect(bad?.malformed).toBe(true);
expect(bad?.error).toMatch(/not in the whitelist/);
expect(bad?.manifest).toBeUndefined();
});

it("preserves unknown-field warnings on a healthy entry", () => {
mkdirSync(userDir, { recursive: true });
writeFileSync(join(userDir, "warn.md"), "---\nname: warn\nbogusKey: 1\n---\nbody\n");
const out = discoverBundles({ userDir, projectDir });
const warn = out.find((e) => e.name === "warn");
expect(warn?.malformed).toBe(false);
expect(warn?.warnings?.some((w) => w.includes("bogusKey"))).toBe(true);
});

it("a clean entry has neither error nor warnings", () => {
writeBundleFile(userDir, "clean");
const clean = discoverBundles({ userDir, projectDir }).find((e) => e.name === "clean");
expect(clean?.error).toBeUndefined();
expect(clean?.warnings).toBeUndefined();
});
});
Loading
Loading