Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,13 @@
"sync-schemas:check": "tsx scripts/sync-schemas.ts --check",
"sync:package-subpaths": "node scripts/package-subpaths.mjs --write",
"check:package-subpaths": "node scripts/package-subpaths.mjs",
"lint": "bun run check:tracked-artifacts && bun run check:workspace-contracts && bun run check:package-cycles && bun run check:package-subpaths && oxlint . && tsx scripts/lint-skills.ts",
"lint": "bun run check:tracked-artifacts && bun run check:workspace-contracts && bun run check:package-cycles && bun run check:package-subpaths && bun run check:cli-process-ownership && oxlint . && tsx scripts/lint-skills.ts",
"lint:skills": "tsx scripts/lint-skills.ts",
"lint:fix": "oxlint --fix .",
"check:tracked-artifacts": "node scripts/check-tracked-artifacts.mjs",
"check:workspace-contracts": "node scripts/check-workspace-contracts.mjs",
"check:package-cycles": "node scripts/check-package-cycles.mjs",
"check:cli-process-ownership": "node scripts/check-cli-process-ownership.mjs",
"format": "oxfmt .",
"test": "bun run test:unit",
"test:unit": "bun run --filter '*' test",
Expand All @@ -44,7 +45,7 @@
"player:perf": "bun run --filter @hyperframes/player perf",
"format:check": "oxfmt --check .",
"knip": "knip",
"test:scripts": "node --import tsx --test scripts/check-tracked-artifacts.test.mjs scripts/check-workspace-contracts.test.mjs scripts/check-package-cycles.test.mjs scripts/package-subpaths.test.mjs scripts/validate-release-channel.test.mjs scripts/draft-changelog.test.ts scripts/set-version.test.ts scripts/release-prepare.test.ts scripts/cli-options.test.ts scripts/changelog-weekly.test.ts scripts/claude-plugin-compression.test.ts scripts/studio-runtime-smoke.test.mjs scripts/verify-packed-manifests.test.mjs",
"test:scripts": "node --import tsx --test scripts/check-tracked-artifacts.test.mjs scripts/check-workspace-contracts.test.mjs scripts/check-package-cycles.test.mjs scripts/check-cli-process-ownership.test.mjs scripts/package-subpaths.test.mjs scripts/validate-release-channel.test.mjs scripts/draft-changelog.test.ts scripts/set-version.test.ts scripts/release-prepare.test.ts scripts/cli-options.test.ts scripts/changelog-weekly.test.ts scripts/claude-plugin-compression.test.ts scripts/studio-runtime-smoke.test.mjs scripts/verify-packed-manifests.test.mjs",
"test:skills": "node --test 'skills/**/*.test.mjs'",
"generate:previews": "tsx scripts/generate-template-previews.ts",
"generate:catalog-previews": "tsx scripts/generate-catalog-previews.ts",
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/auth/oauth.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { failCommand } from "../utils/commandResult.js";
/**
* OAuth 2.0 + PKCE driver for the HeyGen public OAuth flow.
*
Expand Down Expand Up @@ -115,7 +116,7 @@ export function assertOAuthConfiguredOrExit(): void {
if (isAuthError(err) && err.code === "OAUTH_NOT_CONFIGURED") {
console.error(`Error: ${err.message}`);
if (err.hint) console.error(err.hint);
process.exit(1);
failCommand();
}
throw err;
}
Expand Down
114 changes: 89 additions & 25 deletions packages/cli/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,19 @@ try {
// Telemetry, update checks, and heavy modules are imported only when needed.
// For --help we skip telemetry entirely.

import { defineCommand, runMain } from "citty";
import { defineCommand, runCommand } from "citty";
import type { ArgsDef, CommandDef } from "citty";
import { getRunId } from "./telemetry/runId.js";
import { reportCommandFailure, trackCommandFailures } from "./utils/command-failure-tracking.js";
import { resolveCommandUsage } from "./utils/commandUsageResolution.js";
import {
CliResultSignal,
CliRuntimeError,
CliUsageError,
consumeCommandResult,
registerRootExitRequester,
type CommandResult,
} from "./utils/commandResult.js";

const isHelp = process.argv.includes("--help") || process.argv.includes("-h");

Expand Down Expand Up @@ -151,14 +160,10 @@ const commandLoaders = {
figma: () => import("./commands/figma.js").then((m) => m.default),
};

// Wrap each command's run() so a thrown failure reports its reason to telemetry
// before citty catches the error and exits 1. The error is re-thrown unchanged,
// preserving citty's print + exit-1 behavior. Commands that call process.exit()
// themselves (e.g. `browser path`) bypass this and report inline.
const subCommands = Object.fromEntries(
Object.entries(commandLoaders).map(([name, load]) => [
name,
trackCommandFailures(load, (err) => reportCommandFailure(command, err)),
trackCommandFailures(load, (error) => reportCommandFailure(command, error)),
]),
);

Expand Down Expand Up @@ -207,12 +212,13 @@ let _trackCommandResult:
| undefined;
let _printUpdateNotice: (() => void) | undefined;
let _printSkillsUpdateNotice: (() => void) | undefined;
let telemetryReady: Promise<void> = Promise.resolve();

// `events` is a telemetry-internal beacon: it self-tracks + self-flushes, so it
// skips the per-command wrapper (no duplicate cli_command, no first-run notice
// printed into a skill's captured output).
if (!isHelp && command !== "telemetry" && command !== "events" && command !== "unknown") {
import("./telemetry/index.js").then((mod) => {
telemetryReady = import("./telemetry/index.js").then((mod) => {
_flush = mod.flush;
_flushSync = mod.flushSync;
_trackCliError = mod.trackCliError;
Expand Down Expand Up @@ -262,33 +268,55 @@ if (

const commandStart = Date.now();
const runId = getRunId();
let finalized = false;

// Async flush for normal exit. `beforeExit` re-fires every time the
// event loop drains, and the async `_flush()` itself schedules new
// work — so a plain `on` listener would print the update notice (and
// re-flush) once per drain (the user-reported double-print). `once`
// detaches after first invocation, which is what we want for both.
process.once("beforeExit", () => {
_flush?.().catch(() => {});
// fallow-ignore-next-line complexity
async function finalizeCli(result: CommandResult): Promise<void> {
if (finalized) return;
finalized = true;
commandFailed ||= result.exitCode !== 0;
await telemetryReady.catch(() => {});
_trackCommandResult?.({
command,
success: result.exitCode === 0 && !commandFailed,
exitCode: result.exitCode,
durationMs: Date.now() - commandStart,
runId,
});
await _flush?.().catch(() => {});
if (!hasJsonFlag) {
_printUpdateNotice?.();
_printSkillsUpdateNotice?.();
}
process.exitCode = result.exitCode;
}

registerRootExitRequester((exitCode) => {
void finalizeCli({
exitCode,
kind: exitCode === 0 ? "success" : "runtime_error",
presented: true,
}).finally(() => process.exit(exitCode));
});

// Sync-only: exit handlers cannot await promises or drain microtasks.
// _trackCommandResult / _trackCliError are captured references resolved
// at init time, so they're callable synchronously here.
process.on("exit", (code) => {
_trackCommandResult?.({
command,
success: code === 0 && !commandFailed,
exitCode: code,
durationMs: Date.now() - commandStart,
runId,
});
_flushSync?.();
});
process.on(
"exit",
// fallow-ignore-next-line complexity
(code) => {
if (finalized) return;
_trackCommandResult?.({
command,
success: code === 0 && !commandFailed,
exitCode: code,
durationMs: Date.now() - commandStart,
runId,
});
_flushSync?.();
},
);

process.on("uncaughtException", (error) => {
if ((error as NodeJS.ErrnoException).code === "EPIPE") {
Expand All @@ -312,6 +340,7 @@ process.on("uncaughtException", (error) => {
// The exit handler above will still fire with the real exit code.
process.on("unhandledRejection", (reason) => {
commandFailed = true;
process.exitCode = 1;
const error = reason instanceof Error ? reason : new Error(String(reason));
_trackCliError?.({
error_name: error.name,
Expand All @@ -331,4 +360,39 @@ async function showUsage<T extends ArgsDef>(
return impl(cmd as CommandDef, parent as CommandDef | undefined);
}

runMain(main, { showUsage });
async function showRequestedUsage(): Promise<void> {
const requested = await resolveCommandUsage(main as CommandDef, argv);
return showUsage(requested.command, requested.parent);
}

function commandResultForError(error: unknown): CommandResult {
if (error instanceof CliResultSignal) return error.result;
if (error instanceof CliUsageError || error instanceof CliRuntimeError) return error.result;
return { exitCode: 1, kind: "runtime_error" };
}

// fallow-ignore-next-line complexity
async function executeCli(): Promise<void> {
let result: CommandResult = { exitCode: 0, kind: "success" };
try {
if (isHelp) await showRequestedUsage();
else await runCommand(main, { rawArgs: argv });
} catch (error) {
result = commandResultForError(error);
if (!(error instanceof CliResultSignal)) {
commandFailed = true;
reportCommandFailure(command, error);
const typed = error instanceof CliUsageError || error instanceof CliRuntimeError;
if (error instanceof CliUsageError && !error.result.presented) await showRequestedUsage();
if (!typed || !error.result.presented) {
console.error(error instanceof Error ? error.message : String(error));
}
}
} finally {
const pending = consumeCommandResult();
if (pending.exitCode !== 0 || result.exitCode === 0) result = pending;
await finalizeCli(result);
}
}

await executeCli();
35 changes: 12 additions & 23 deletions packages/cli/src/cloud/errors.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { beforeEach, describe, expect, it, vi } from "vitest";

// errorBox writes to the console; mock it so we can assert which title /
// message / third line the cascade picked without matching ANSI output.
Expand All @@ -10,30 +10,19 @@ vi.mock("../ui/format.js", async (importOriginal) => ({
import { errorBox } from "../ui/format.js";
import { HyperframesApiError } from "./_gen/client.js";
import { reportApiError } from "./errors.js";
import { CliRuntimeError } from "../utils/commandResult.js";

describe("cloud/errors reportApiError", () => {
// process.exit has signature `(code?) => never` which doesn't unify with
// vi.spyOn's inference; cast through `unknown` like cloud/parsing.test.ts.
let exitSpy: { mockRestore: () => void } & { mock: { calls: unknown[][] } };

beforeEach(() => {
exitSpy = vi.spyOn(process, "exit").mockImplementation((() => {
throw new Error("process.exit called");
}) as unknown as (code?: string | number | null) => never) as unknown as typeof exitSpy;
vi.mocked(errorBox).mockClear();
});

afterEach(() => {
exitSpy.mockRestore();
});

it("short-circuits a 404 to a Not found box when notFound is provided", () => {
const err = new HyperframesApiError({ status: 404, message: "missing" });
expect(() =>
reportApiError("Get failed", err, { notFound: "render hfr_x no longer exists" }),
).toThrow("process.exit called");
).toThrow(CliRuntimeError);
expect(errorBox).toHaveBeenCalledWith("Not found", "render hfr_x no longer exists");
expect(exitSpy).toHaveBeenCalledWith(1);
});

it("uses the code-specific hint when the error code is known", () => {
Expand All @@ -42,7 +31,7 @@ describe("cloud/errors reportApiError", () => {
message: "slow down",
code: "rate_limit_exceeded",
});
expect(() => reportApiError("Submit failed", err)).toThrow("process.exit called");
expect(() => reportApiError("Submit failed", err)).toThrow(CliRuntimeError);
expect(errorBox).toHaveBeenCalledWith(
"Submit failed (HTTP 429)",
"slow down",
Expand All @@ -57,7 +46,7 @@ describe("cloud/errors reportApiError", () => {
code: "invalid_parameter",
});
expect(() => reportApiError("Submit failed", err, { suggestion: "see --help" })).toThrow(
"process.exit called",
CliRuntimeError,
);
expect(errorBox).toHaveBeenCalledWith(
"Submit failed (HTTP 400)",
Expand All @@ -73,7 +62,7 @@ describe("cloud/errors reportApiError", () => {
code: "some_unmapped_code",
});
expect(() => reportApiError("List failed", err, { suggestion: "retry shortly" })).toThrow(
"process.exit called",
CliRuntimeError,
);
expect(errorBox).toHaveBeenCalledWith("List failed (HTTP 500)", "boom", "retry shortly");
});
Expand All @@ -84,7 +73,7 @@ describe("cloud/errors reportApiError", () => {
message: "boom",
code: "some_unmapped_code",
});
expect(() => reportApiError("List failed", err)).toThrow("process.exit called");
expect(() => reportApiError("List failed", err)).toThrow(CliRuntimeError);
expect(errorBox).toHaveBeenCalledWith(
"List failed (HTTP 500)",
"boom",
Expand All @@ -94,15 +83,15 @@ describe("cloud/errors reportApiError", () => {

it("omits the third line when there is no code, hint, or suggestion", () => {
const err = new HyperframesApiError({ status: 503, message: "unavailable" });
expect(() => reportApiError("Get failed", err)).toThrow("process.exit called");
expect(() => reportApiError("Get failed", err)).toThrow(CliRuntimeError);
expect(errorBox).toHaveBeenCalledWith("Get failed (HTTP 503)", "unavailable");
});

it("merges extraHints on top of the built-in table", () => {
const err = new HyperframesApiError({ status: 418, message: "teapot", code: "custom_code" });
expect(() =>
reportApiError("Brew failed", err, { extraHints: { custom_code: "use coffee instead" } }),
).toThrow("process.exit called");
).toThrow(CliRuntimeError);
expect(errorBox).toHaveBeenCalledWith("Brew failed (HTTP 418)", "teapot", "use coffee instead");
});

Expand All @@ -116,7 +105,7 @@ describe("cloud/errors reportApiError", () => {
reportApiError("Submit failed", err, {
extraHints: { rate_limit_exceeded: "custom backoff guidance" },
}),
).toThrow("process.exit called");
).toThrow(CliRuntimeError);
expect(errorBox).toHaveBeenCalledWith(
"Submit failed (HTTP 429)",
"slow down",
Expand All @@ -127,12 +116,12 @@ describe("cloud/errors reportApiError", () => {
it("reports a plain Error with the stage title and suggestion", () => {
expect(() =>
reportApiError("Upload failed", new Error("disk full"), { suggestion: "free up space" }),
).toThrow("process.exit called");
).toThrow(CliRuntimeError);
expect(errorBox).toHaveBeenCalledWith("Upload failed", "disk full", "free up space");
});

it("stringifies a non-Error thrown value", () => {
expect(() => reportApiError("Weird failure", "just a string")).toThrow("process.exit called");
expect(() => reportApiError("Weird failure", "just a string")).toThrow(CliRuntimeError);
expect(errorBox).toHaveBeenCalledWith("Weird failure", "just a string", undefined);
});
});
9 changes: 5 additions & 4 deletions packages/cli/src/cloud/errors.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { failCommand } from "../utils/commandResult.js";
/**
* Shared API-error reporter for the cloud subverbs.
*
Expand Down Expand Up @@ -61,7 +62,7 @@ export function reportApiError(
if (err instanceof HyperframesApiError) {
if (err.status === 404 && options.notFound) {
errorBox("Not found", options.notFound);
process.exit(1);
failCommand();
}
const hint = err.code ? hints[err.code] : undefined;
const title = `${stage} (HTTP ${err.status})`;
Expand All @@ -78,12 +79,12 @@ export function reportApiError(
} else {
errorBox(title, err.message);
}
process.exit(1);
failCommand();
}
if (err instanceof Error) {
errorBox(stage, err.message, options.suggestion);
process.exit(1);
failCommand();
}
errorBox(stage, String(err), options.suggestion);
process.exit(1);
failCommand();
}
Loading
Loading