diff --git a/packages/cli/src/cli.agentMode.test.ts b/packages/cli/src/cli.agentMode.test.ts index 03cbbd586d..4624d3a8ab 100644 --- a/packages/cli/src/cli.agentMode.test.ts +++ b/packages/cli/src/cli.agentMode.test.ts @@ -190,6 +190,79 @@ describe("agent-mode contract", () => { expect(run.code, run.stderr).toBe(2); }); + // A bad flag value or flag combination a command checks itself is a usage error like any other: + // exit 2, and under --json a usage_error envelope that names the problem. + describe.concurrent("flag checks a command makes itself", () => { + it.each([ + ["lambda render proj", "--width and --height are required"], + ["lambda render proj --width 1920 --height 1080 --fps 7", "--fps must be one of"], + ["lambda render proj --width 0 --height 1080", "--width must be a positive integer"], + ["lambda render proj --width 1920 --height 1080 --chunk-size 0", "--chunk-size must be"], + ["lambda render proj --width 1920 --height 1080 --format avi", "--format must be"], + ["lambda render proj --width 1920 --height 1080 --output-resolution 8k", "8k"], + ["lambda render-batch proj --batch b.jsonl", "--width and --height are required"], + ["lambda render-batch proj --batch b.jsonl --width 1920 --height 1080 --fps 7", "--fps"], + ["lambda deploy --concurrency 0", "--concurrency must be"], + ["lambda sites bogus", 'unknown verb "bogus"'], + ["lambda bogus", "Unknown subcommand"], + ["cloudrun render proj", "--width and --height are required"], + ["cloudrun render proj --width 1920 --height 1080 --fps 7", "--fps must be one of"], + ["cloudrun render proj --width 1920 --height 1080 --chunk-size 0", "--chunk-size must be"], + ["cloudrun render proj --width 1920 --height 1080 --wait-interval-ms 0", "--wait-interval"], + ["cloudrun render proj --width 1920 --height 1080 --variables {bad", "--variables"], + ["cloudrun render-batch proj --batch b.jsonl", "--width and --height are required"], + ["cloudrun render-batch proj --batch b.jsonl --width 1920 --height 1080 --fps 7", "--fps"], + ["cloudrun render-batch proj --batch b.jsonl --width 640 --height 360 --format avi", "avi"], + ["cloudrun sites bogus", 'unknown verb "bogus"'], + ["cloudrun bogus", "Unknown subcommand"], + ["capture https://example.com --max-screenshots 0", "--max-screenshots must be"], + ["capture https://example.com --capture-budget 0", "--capture-budget must be"], + ["capture https://example.com --local-file page.html", "not both"], + ["capture not-a-url", "Invalid URL"], + ["benchmark --runs 0", "Invalid runs"], + ["models bogus", "Unknown: models bogus"], + ["tts", "Provide text to speak"], + ["tts hello --speed 9", "Speed must be"], + ["cloud render --asset-id a --url https://example.com/p.zip", "Conflicting inputs"], + ["cloud delete render_1", "Confirmation required"], + ["snapshot --at 1 --range 0,1", "Use only one of"], + ["snapshot --at x", "--at must be"], + ["snapshot --frames abc", "--frames must be a positive integer"], + ["snapshot --frames -3", "--frames must be a positive integer"], + ["compare a.html", "need 2+ paths"], + ["compare a.html b.html --cols 0", "--cols must be"], + ["grade-compare --for frame.png", "Exactly one of --grades or --luts"], + ["check --range 5,1", "Invalid --range"], + ["normalize-audio --target music --tolerance=-1", "--tolerance must be"], + ["media-treatment --capabilities --all --capability x", "--all or --capability"], + ])("`%s` exits 2 with a usage_error envelope", async (args, message) => { + const run = await runAsAgent([...args.split(" "), "--json"]); + expectQuiet(run); + expect(run.code, run.stderr).toBe(2); + expectJsonFailure(run, "usage_error"); + expect(JSON.parse(run.stdout).error.message).toContain(message); + }); + + it.each([ + ["feedback --rating 99", "Rating must be an integer between 0 and 10"], + ["feedback", "Give a --rating"], + ["feedback --comment x --file-issue", "--file-issue needs a --rating"], + ["feedback --rating 5 --source bot", "Source must be"], + ["browser bogus", "Unknown subcommand"], + ["telemetry bogus", "Unknown subcommand"], + ["docs bogus", "Unknown topic"], + ["init demo --template x --non-interactive", "renamed to --example"], + ["init demo --video a.mp4 --audio b.mp3 --non-interactive", "--video and --audio"], + ["play --port 0", "--port must be"], + ["present --port 0", "--port must be"], + ])("`%s` has no JSON output and exits 2 naming the problem", async (args, message) => { + const run = await runAsAgent(args.split(" ")); + expect(run.signal, "finished before the timeout").toBeNull(); + expect(run.code, run.stderr).toBe(2); + expect(run.stdout + run.stderr).toContain(message); + }); + }); + it.each([ ["help", "Usage:"], ["help render", "USAGE hyperframes render"], @@ -256,9 +329,9 @@ describe("agent-mode contract", () => { writeFileSync(join(project, "index.html"), ""); const run = await runAsAgent(["render", project, "--variables", "{bad", "--json"]); expectQuiet(run); - expect(run.code, run.stderr).toBe(1); + expect(run.code, run.stderr).toBe(2); expect(run.stdout.trim().split("\n")).toHaveLength(1); - expectJsonFailure(run, "runtime_error"); + expectJsonFailure(run, "usage_error"); expect(JSON.parse(run.stdout).error.message).toBe("Invalid JSON in --variables"); }); diff --git a/packages/cli/src/commands/benchmark.ts b/packages/cli/src/commands/benchmark.ts index 3cf2047f9f..882e365348 100644 --- a/packages/cli/src/commands/benchmark.ts +++ b/packages/cli/src/commands/benchmark.ts @@ -1,4 +1,4 @@ -import { failCommand, setCommandExitCode } from "../utils/commandResult.js"; +import { failCommand, failUsage, setCommandExitCode } from "../utils/commandResult.js"; import { defineCommand } from "../utils/defineCommand.js"; import type { Example } from "./_examples.js"; import { existsSync, mkdtempSync, rmSync, statSync } from "node:fs"; @@ -59,16 +59,16 @@ export default defineCommand({ json: { type: "boolean", description: "Output results as JSON", default: false }, }, async run({ args }) { - // ── Resolve project ────────────────────────────────────────────────── - const project = resolveProject(args.dir); - // ── Parse runs ─────────────────────────────────────────────────────── const runsPerConfig = parseInt(args.runs ?? "3", 10); if (isNaN(runsPerConfig) || runsPerConfig < 1 || runsPerConfig > 20) { errorBox("Invalid runs", `Got "${args.runs ?? "3"}". Must be between 1 and 20.`); - failCommand(); + failUsage(); } + // ── Resolve project ────────────────────────────────────────────────── + const project = resolveProject(args.dir); + const jsonOutput = args.json ?? false; // ── Temp output for benchmark renders ──────────────────────────────── diff --git a/packages/cli/src/commands/browser.ts b/packages/cli/src/commands/browser.ts index f4ce74076c..fa6ceddcd4 100644 --- a/packages/cli/src/commands/browser.ts +++ b/packages/cli/src/commands/browser.ts @@ -1,4 +1,4 @@ -import { failCommand } from "../utils/commandResult.js"; +import { failCommand, failUsage } from "../utils/commandResult.js"; import { defineCommand } from "../utils/defineCommand.js"; import type { Example } from "./_examples.js"; import * as clack from "../ui/prompts.js"; @@ -182,7 +182,7 @@ ${c.bold("EXAMPLES:")} console.error( `${c.error("Unknown subcommand:")} ${subcommand}\n\nRun ${c.accent("hyperframes browser --help")} for usage.`, ); - failCommand(1, `Unknown subcommand: ${subcommand}`); + failUsage(); } }, }); diff --git a/packages/cli/src/commands/capture.test.ts b/packages/cli/src/commands/capture.test.ts index 3f8be2346b..16ee6307cf 100644 --- a/packages/cli/src/commands/capture.test.ts +++ b/packages/cli/src/commands/capture.test.ts @@ -169,7 +169,7 @@ describe("capture command — vision control", () => { json: true, }, } as never), - ).rejects.toBeInstanceOf(CliRuntimeError); + ).rejects.toBeInstanceOf(CliUsageError); expect(captureWebsiteMock).not.toHaveBeenCalled(); }, @@ -189,7 +189,7 @@ describe("capture command — vision control", () => { json: true, }, } as never), - ).rejects.toBeInstanceOf(CliRuntimeError); + ).rejects.toBeInstanceOf(CliUsageError); expect(error).toHaveBeenCalledWith("--max-screenshots must be a positive integer."); expect(captureWebsiteMock).not.toHaveBeenCalled(); }, @@ -232,7 +232,7 @@ describe("capture command — vision control", () => { captureCommand.run!({ args: { ...target, output: "/tmp/hf-local-refused", json: true }, } as never), - ).rejects.toThrow("local-root" in target ? CliUsageError : CliRuntimeError); + ).rejects.toThrow("url" in target ? CliUsageError : CliRuntimeError); expect(error.mock.calls.flat().join("\n")).toContain(message); expect(captureWebsiteMock).not.toHaveBeenCalled(); }); diff --git a/packages/cli/src/commands/capture.ts b/packages/cli/src/commands/capture.ts index 1e56474ddc..9657746c8d 100644 --- a/packages/cli/src/commands/capture.ts +++ b/packages/cli/src/commands/capture.ts @@ -24,7 +24,7 @@ function parsePositiveInteger( const parsed = Number(raw); if (!Number.isInteger(parsed) || parsed <= 0) { console.error(`${flag} must be a positive integer${unit}.`); - failCommand(); + failUsage(); } return parsed; } @@ -152,7 +152,7 @@ export default defineCommand({ if (localFile !== undefined) { if (url) { console.error("Pass either a URL or --local-file, not both."); - failCommand(); + failUsage(); } const { resolveLocalCaptureTarget } = await import("../capture/localPage.js"); try { @@ -176,7 +176,7 @@ export default defineCommand({ new URL(url); } catch { console.error(`Invalid URL: ${url}`); - failCommand(); + failUsage(); } } diff --git a/packages/cli/src/commands/check.test.ts b/packages/cli/src/commands/check.test.ts index 37f30f28b8..a53f1c67e6 100644 --- a/packages/cli/src/commands/check.test.ts +++ b/packages/cli/src/commands/check.test.ts @@ -48,7 +48,11 @@ import { type MotionSpecResolution, } from "../utils/checkPipeline.js"; import { resolveCompositionViewportFromHtml } from "../utils/compositionViewport.js"; -import { consumeCommandResult, registerRootExitRequester } from "../utils/commandResult.js"; +import { + CliUsageError, + consumeCommandResult, + registerRootExitRequester, +} from "../utils/commandResult.js"; import { ambiguousIssue } from "../utils/motionAudit.js"; import type { ProjectLintResult } from "../utils/lintProject.js"; import type { @@ -654,7 +658,9 @@ it("prints a dash-prefixed --frame-check value's parse failure exactly once, not withMeta: (value) => value, }); - await runViaCli(command, { rawArgs: ["--frame-check=--json"] }); + await expect(runViaCli(command, { rawArgs: ["--frame-check=--json"] })).rejects.toBeInstanceOf( + CliUsageError, + ); expect(runPipeline).not.toHaveBeenCalled(); const matching = errorLog.mock.calls.filter( @@ -841,16 +847,20 @@ it("rejects malformed caption-zone specs instead of silently disabling the gate" withMeta: (value) => ({ ...value, _meta: { version: "test" } }), }); - await runCommand(command, { - rawArgs: ["--json", "--caption-zone", "x0=0;y0=.8;x1=1;y1=1.2"], - }); + await expect( + runCommand(command, { + rawArgs: ["--json", "--caption-zone", "x0=0;y0=.8;x1=1;y1=1.2"], + }), + ).rejects.toMatchObject({ result: { exitCode: 2, kind: "usage_error" } }); expect(runPipeline).not.toHaveBeenCalled(); - expect(consumeCommandResult().exitCode).toBe(1); expect(log).toHaveBeenCalledTimes(1); expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual({ ok: false, - error: expect.objectContaining({ message: expect.stringContaining("Invalid --caption-zone") }), + error: expect.objectContaining({ + code: "usage_error", + message: expect.stringContaining("Invalid --caption-zone"), + }), _meta: { version: "test" }, }); }); diff --git a/packages/cli/src/commands/check.ts b/packages/cli/src/commands/check.ts index c3bb2391f4..20cf1b98f9 100644 --- a/packages/cli/src/commands/check.ts +++ b/packages/cli/src/commands/check.ts @@ -20,7 +20,7 @@ import { parseAt } from "./layout.js"; import { c } from "../ui/colors.js"; import { registerBrowserSessionCleanup } from "../browser/launch.js"; import { normalizeErrorMessage } from "../utils/errorMessage.js"; -import { setCommandExitCode } from "../utils/commandResult.js"; +import { CliUsageError, failUsage, setCommandExitCode } from "../utils/commandResult.js"; import { swallowedFlagUsageError } from "../utils/reject-unknown-flags.js"; import { formatLayoutIssue } from "../utils/layoutAudit.js"; import { resolveProject, type ProjectDir } from "../utils/project.js"; @@ -278,8 +278,8 @@ export function createCheckCommand( const reported = untilReported(); try { - const project = dependencies.resolveProject(args.dir); const options = parseCheckOptions(args); + const project = dependencies.resolveProject(args.dir); if (!asJson) { console.log(`${c.accent("◆")} Checking ${c.accent(project.name)}`); } @@ -303,15 +303,7 @@ export function createCheckCommand( } setCommandExitCode(checkExitCode(report)); } catch (error) { - const message = failureMessage(error); - if (asJson) { - console.log( - JSON.stringify(dependencies.withMeta(jsonFailure("runtime_error", message)), null, 2), - ); - } else { - console.error(`${c.error("✗")} Check failed: ${message}`); - } - setCommandExitCode(1); + reportCheckFailure(error, asJson, (value) => dependencies.withMeta(value)); } finally { reported.done(); } @@ -319,6 +311,23 @@ export function createCheckCommand( }); } +function reportCheckFailure( + error: unknown, + asJson: boolean, + addMeta: (value: object) => object, +): void { + const message = failureMessage(error); + const usage = error instanceof CliUsageError; + if (asJson) { + const code = usage ? error.code : "runtime_error"; + console.log(JSON.stringify(addMeta(jsonFailure(code, message)), null, 2)); + } else { + console.error(`${c.error("✗")} Check failed: ${message}`); + } + if (usage) failUsage(); + setCommandExitCode(1); +} + const REPORT_GRACE_MS = 5_000; function untilReported(): { done: () => void } { @@ -427,7 +436,7 @@ export function parseCheckRange(value: unknown): [number, number] | undefined { start < 0 || end <= start ) - throw new Error("Invalid --range: use non-negative start,end with end > start"); + throw new CliUsageError("Invalid --range: use non-negative start,end with end > start"); return [start, end]; } @@ -474,7 +483,7 @@ function parseFrameCheckTolerance(raw: string | undefined): number | undefined { } function frameCheckError(): Error { - return new Error( + return new CliUsageError( 'Invalid --frame-check: use bare --frame-check or "severity=warning|error;seek=.25,.75;tol=4" (all fields optional)', ); } @@ -513,7 +522,7 @@ function parseProseCoverageFloor(raw: string | undefined): number | undefined { } function layoutError(): Error { - return new Error( + return new CliUsageError( 'Invalid --layout: use "proseCoverageFloor=0.05" with a fraction from 0 to 1 (inclusive)', ); } @@ -615,7 +624,7 @@ function captionSeeks( } function captionZoneError(): Error { - return new Error( + return new CliUsageError( 'Invalid --caption-zone; use "x0=0;y0=.82;x1=1;y1=1[;severity=warning|error][;seek=.5,1]" with fractions from 0 to 1.', ); } diff --git a/packages/cli/src/commands/cloud/delete.ts b/packages/cli/src/commands/cloud/delete.ts index 0cf8ba1245..c9cde36014 100644 --- a/packages/cli/src/commands/cloud/delete.ts +++ b/packages/cli/src/commands/cloud/delete.ts @@ -1,4 +1,4 @@ -import { failCommand } from "../../utils/commandResult.js"; +import { failCommand, failUsage } from "../../utils/commandResult.js"; /** * `hyperframes cloud delete ` — soft-delete a cloud render. * @@ -55,7 +55,7 @@ export default defineCommand({ "delete cannot prompt for confirmation here: this run cannot answer a prompt, or --json was passed.", "Re-run with --no-confirm to acknowledge the irreversible delete.", ); - failCommand(); + failUsage(); } const ok = await confirmDelete(args.id); if (!ok) { diff --git a/packages/cli/src/commands/cloud/render.ts b/packages/cli/src/commands/cloud/render.ts index 1b2a8a9f23..576f499860 100644 --- a/packages/cli/src/commands/cloud/render.ts +++ b/packages/cli/src/commands/cloud/render.ts @@ -425,7 +425,7 @@ function resolveProjectInput(opts: { const count = Number(explicit.dir) + Number(explicit.assetId) + Number(explicit.url); if (count > 1) { errorBox("Conflicting inputs", "Pass only one of: project dir, --asset-id, --url."); - failCommand(); + failUsage(); } if (explicit.assetId) return { kind: "asset_id", assetId: opts.assetId }; if (explicit.url) return { kind: "url", url: opts.url }; diff --git a/packages/cli/src/commands/cloudrun.ts b/packages/cli/src/commands/cloudrun.ts index 6527c87428..ef585538d3 100644 --- a/packages/cli/src/commands/cloudrun.ts +++ b/packages/cli/src/commands/cloudrun.ts @@ -1,5 +1,5 @@ // fallow-ignore-file code-duplication -import { failCommand, failUsage } from "../utils/commandResult.js"; +import { CliUsageError, failCommand, failUsage } from "../utils/commandResult.js"; /** * `hyperframes cloudrun` — deploy + drive distributed renders on Google * Cloud Run + Cloud Workflows. @@ -259,7 +259,7 @@ export default defineCommand({ return runDestroy(args); default: console.error(`${c.error("Unknown subcommand:")} ${subcommand}\n${HELP}`); - failCommand(); + failUsage(); } }, }); @@ -492,7 +492,7 @@ async function runSites(args: Record): Promise { console.error( `[cloudrun sites] unknown verb "${String(args.target)}". Only "create" is supported.`, ); - failCommand(); + failUsage(); } const projectDir = args.extra as string | undefined; if (!projectDir) { @@ -528,7 +528,7 @@ function resolveCloudRunFps( console.error( `[cloudrun ${command}] --fps must be one of ${DISTRIBUTED_FPS.join(", ")}; got ${fps}.`, ); - failCommand(); + failUsage(); } // fallow-ignore-next-line complexity @@ -544,12 +544,13 @@ async function runRender(args: Record): Promise { const height = parsePositiveInt(args.height, "--height"); if (width === undefined || height === undefined) { console.error("[cloudrun render] --width and --height are required."); - failCommand(); + failUsage(); } const fps = resolveCloudRunFps(args, projectDir, "render"); - const state = readState(args); const variables = resolveAndValidateVariables(args, resolve(projectDir)); const config = buildRenderConfig(args, fps, width, height, variables); + const intervalMs = parsePositiveInt(args["wait-interval-ms"], "--wait-interval-ms") ?? 5000; + const state = readState(args); const { renderToCloudRun, getRenderProgress } = await loadCloudRunAdapter(); const handle = await renderToCloudRun({ @@ -576,7 +577,6 @@ async function runRender(args: Record): Promise { return; } - const intervalMs = parsePositiveInt(args["wait-interval-ms"], "--wait-interval-ms") ?? 5000; let progress = await getRenderProgress({ executionName: handle.executionName }); while (progress.status === "running") { await new Promise((r) => setTimeout(r, intervalMs)); @@ -650,9 +650,10 @@ async function runRenderBatch(args: Record): Promise { const height = parsePositiveInt(args.height, "--height"); if (width === undefined || height === undefined) { console.error("[cloudrun render-batch] --width and --height are required."); - failCommand(); + failUsage(); } const fps = resolveCloudRunFps(args, projectDir, "render-batch"); + buildRenderConfig(args, fps, width, height, undefined); if (!existsSync(resolve(batchPath))) { console.error(`[cloudrun render-batch] batch file not found: ${batchPath}`); failCommand(); @@ -674,9 +675,9 @@ async function runRenderBatch(args: Record): Promise { return; } - const state = readState(args); const maxConcurrent = parsePositiveInt(args["max-concurrent"], "--max-concurrent") ?? DEFAULT_BATCH_MAX_CONCURRENT; + const state = readState(args); const { deploySite, renderToCloudRun } = await loadCloudRunAdapter(); // Upload the project once; every entry reuses the same content-addressed @@ -898,7 +899,7 @@ function parsePositiveInt(raw: unknown, flagName: string): number | undefined { const n = parseIntFlag(raw); if (n === undefined) return undefined; if (!Number.isInteger(n) || n < 1) { - throw new Error(`[cloudrun] ${flagName} must be a positive integer; got ${n}`); + throw new CliUsageError(`[cloudrun] ${flagName} must be a positive integer; got ${n}`); } return n; } @@ -912,7 +913,7 @@ function parseEnum( if (raw === undefined || raw === null || raw === "") return defaultValue; const s = String(raw); if ((allowed as readonly string[]).includes(s)) return s as T; - throw new Error(`${errorPrefix} must be ${allowed.join("|")}; got ${s}`); + throw new CliUsageError(`${errorPrefix} must be ${allowed.join("|")}; got ${s}`); } const FORMATS = ["mp4", "mov", "png-sequence", "webm"] as const; const CODECS = ["h264", "h265"] as const; diff --git a/packages/cli/src/commands/compare.ts b/packages/cli/src/commands/compare.ts index a96db89456..111a27a5d5 100644 --- a/packages/cli/src/commands/compare.ts +++ b/packages/cli/src/commands/compare.ts @@ -27,6 +27,7 @@ import { withMeta } from "../utils/updateCheck.js"; import type { Example } from "./_examples.js"; import { isPathInside } from "@hyperframes/parsers/asset-paths"; import { failCaught } from "../utils/jsonError.js"; +import { CliUsageError } from "../utils/commandResult.js"; const MAX_COMPARE_VARIANTS = 16; const MAX_COLUMNS = 4; @@ -95,10 +96,12 @@ function parseLabels(value: unknown, pathCount: number): string[] | undefined { if (!raw) return undefined; const labels = raw.split(",").map((part) => part.trim()); if (labels.some((label) => label.length === 0)) { - throw new Error("--labels entries must be non-empty"); + throw new CliUsageError("--labels entries must be non-empty"); } if (labels.length !== pathCount) { - throw new Error(`--labels count (${labels.length}) must match path count (${pathCount})`); + throw new CliUsageError( + `--labels count (${labels.length}) must match path count (${pathCount})`, + ); } return labels; } @@ -108,7 +111,7 @@ function parseAtSeconds(value: unknown): number { if (!raw) return 0; const parsed = Number(raw); if (!Number.isFinite(parsed) || parsed < 0) { - throw new Error("--at must be a non-negative number of seconds"); + throw new CliUsageError("--at must be a non-negative number of seconds"); } return parsed; } @@ -118,7 +121,7 @@ function parseColumns(value: unknown): number | undefined { if (!raw) return undefined; const parsed = Number(raw); if (!Number.isInteger(parsed) || parsed < 1) { - throw new Error("--cols must be a positive integer"); + throw new CliUsageError("--cols must be a positive integer"); } return parsed; } @@ -141,7 +144,7 @@ export function parseCompareArgs( ): ParsedCompareArgs { const paths = parsePathArgs(args); if (paths.length < 2) { - throw new Error("need 2+ paths to compare"); + throw new CliUsageError("need 2+ paths to compare"); } const labels = parseLabels(args.labels, paths.length); diff --git a/packages/cli/src/commands/docs.ts b/packages/cli/src/commands/docs.ts index 64a473b269..a3c7566d4c 100644 --- a/packages/cli/src/commands/docs.ts +++ b/packages/cli/src/commands/docs.ts @@ -1,4 +1,4 @@ -import { failCommand } from "../utils/commandResult.js"; +import { failCommand, failUsage } from "../utils/commandResult.js"; import { defineCommand } from "../utils/defineCommand.js"; import type { Example } from "./_examples.js"; import { readFileSync, existsSync } from "node:fs"; @@ -128,7 +128,7 @@ export default defineCommand({ for (const name of Object.keys(TOPICS)) { console.error(` ${c.accent(name)}`); } - failCommand(); + failUsage(); } const filePath = join(docsDir(), entry.file); diff --git a/packages/cli/src/commands/feedback.test.ts b/packages/cli/src/commands/feedback.test.ts index 4ef4814272..b274f4f790 100644 --- a/packages/cli/src/commands/feedback.test.ts +++ b/packages/cli/src/commands/feedback.test.ts @@ -1,4 +1,5 @@ import { afterEach, expect, it, vi } from "vitest"; +import { CliUsageError } from "../utils/commandResult.js"; const mocks = vi.hoisted(() => ({ // Never settles: waiting on it would hang the command. @@ -57,7 +58,9 @@ it("refuses a source that is neither person nor agent, and sends nothing", async vi.spyOn(console, "error").mockImplementation(() => {}); mocks.submitFeedback.mockClear(); const { default: feedback } = await import("./feedback.js"); - await expect(feedback.run?.({ args: { rating: "6", source: "bot" } } as never)).rejects.toThrow(); + await expect(feedback.run?.({ args: { rating: "6", source: "bot" } } as never)).rejects.toThrow( + CliUsageError, + ); expect(mocks.submitFeedback).not.toHaveBeenCalled(); }); @@ -82,16 +85,16 @@ it.each([ vi.spyOn(console, "error").mockImplementation(() => {}); mocks.submitFeedback.mockClear(); const { default: feedback } = await import("./feedback.js"); - await expect(feedback.run?.({ args } as never)).rejects.toThrow(); + await expect(feedback.run?.({ args } as never)).rejects.toThrow(CliUsageError); expect(mocks.submitFeedback).not.toHaveBeenCalled(); }); it("keeps the rating error for an unreadable rating, so a retry keeps the rating", async () => { const errors = vi.spyOn(console, "error").mockImplementation(() => {}); const { default: feedback } = await import("./feedback.js"); - await expect( - feedback.run?.({ args: { rating: "8/10", comment: "x" } } as never), - ).rejects.toThrow(); + await expect(feedback.run?.({ args: { rating: "8/10", comment: "x" } } as never)).rejects.toThrow( + CliUsageError, + ); expect(String(errors.mock.calls.at(-1)?.[0])).toContain( "Rating must be an integer between 0 and 10", ); diff --git a/packages/cli/src/commands/feedback.ts b/packages/cli/src/commands/feedback.ts index 84fbc5e8c1..ac4f166000 100644 --- a/packages/cli/src/commands/feedback.ts +++ b/packages/cli/src/commands/feedback.ts @@ -1,4 +1,4 @@ -import { failCommand } from "../utils/commandResult.js"; +import { failUsage } from "../utils/commandResult.js"; import { randomUUID } from "node:crypto"; import { resolve } from "node:path"; import { defineCommand } from "../utils/defineCommand.js"; @@ -174,15 +174,15 @@ function reportRating( const rating = raw === undefined ? undefined : parseFeedbackRating(raw); if (rating === null) { console.error(c.error("Rating must be an integer between 0 and 10")); - failCommand(); + failUsage(); } if (rating === undefined && !comment?.trim()) { console.error(c.error("Give a --rating (an integer from 0 to 10), a --comment, or both")); - failCommand(); + failUsage(); } if (rating === undefined && fileIssue) { console.error(c.error("--file-issue needs a --rating")); - failCommand(); + failUsage(); } return rating; } @@ -279,7 +279,7 @@ export default defineCommand({ const source = parseFeedbackSource(args.source); if (source === null) { console.error(c.error('Source must be "person" or "agent"')); - failCommand(); + failUsage(); } if (!shouldTrack()) { diff --git a/packages/cli/src/commands/grade-compare.ts b/packages/cli/src/commands/grade-compare.ts index 1a8a28ebfe..f67956f83f 100644 --- a/packages/cli/src/commands/grade-compare.ts +++ b/packages/cli/src/commands/grade-compare.ts @@ -29,6 +29,7 @@ import { trackCompareSheet } from "../telemetry/events.js"; import { serveStaticProjectHtml } from "../utils/staticProjectServer.js"; import { withMeta } from "../utils/updateCheck.js"; import { failCaught } from "../utils/jsonError.js"; +import { CliUsageError } from "../utils/commandResult.js"; const COMPOSITION_ID = "grade-compare"; const COMPOSITION_DURATION = "1"; @@ -243,7 +244,7 @@ export function resolveLutCells(luts: string): GradeCompareCell[] { .map((part) => part.trim()) .filter(Boolean); if (paths.length === 0) { - throw new Error("--luts must include at least one LUT path"); + throw new CliUsageError("--luts must include at least one LUT path"); } return paths.map((lutPath) => validateCell(basename(lutPath, extname(lutPath)), { lut: { src: lutPath } }), @@ -267,12 +268,12 @@ export function parseGradeCompareArgs(args: { timeout?: unknown; }): ParsedGradeCompareArgs { const frameArg = readOptionalString(args.for); - if (!frameArg) throw new Error("--for is required"); + if (!frameArg) throw new CliUsageError("--for is required"); const gradesArg = readOptionalString(args.grades); const lutsArg = readOptionalString(args.luts); if (Boolean(gradesArg) === Boolean(lutsArg)) { - throw new Error("Exactly one of --grades or --luts is required"); + throw new CliUsageError("Exactly one of --grades or --luts is required"); } const projectDir = resolve(readOptionalString(args.project) ?? process.cwd()); diff --git a/packages/cli/src/commands/init.test.ts b/packages/cli/src/commands/init.test.ts index 0248495ae3..b297d79770 100644 --- a/packages/cli/src/commands/init.test.ts +++ b/packages/cli/src/commands/init.test.ts @@ -445,7 +445,7 @@ describe("hyperframes init flag rename", () => { "--audio", "track.mp3", ]); - expect(res.status).toBe(1); + expect(res.status).toBe(2); expect(res.stderr).toContain("Cannot use --video and --audio together"); expect(existsSync(target)).toBe(false); } finally { @@ -529,19 +529,19 @@ describe("hyperframes init flag rename", () => { "-V", "missing.mp4", ]); - expect(res.status).toBe(1); + expect(res.status).toBe(2); expect(res.stderr).toContain("The -V short flag no longer maps to --video"); } finally { rmSync(dir, { recursive: true, force: true }); } }); - it("--template prints a rename hint and exits non-zero", () => { + it("--template prints a rename hint and exits 2", () => { const dir = mkdtempSync(join(tmpdir(), "hf-init-test-")); const target = join(dir, "proj"); try { const res = runInit([target, "--template", "blank", "--non-interactive", "--skip-skills"]); - expect(res.status).toBe(1); + expect(res.status).toBe(2); expect(res.stderr).toContain("--template flag was renamed to --example"); expect(res.stderr).toContain(`--example "blank"`); expect(existsSync(target)).toBe(false); diff --git a/packages/cli/src/commands/init.ts b/packages/cli/src/commands/init.ts index 06b6cfef7d..243d6466da 100644 --- a/packages/cli/src/commands/init.ts +++ b/packages/cli/src/commands/init.ts @@ -802,7 +802,7 @@ export default defineCommand({ `The --template flag was renamed to --example. Example:\n npx hyperframes init ${args.name ?? "my-video"} --example "${args.template}"`, ), ); - failCommand(); + failUsage(); } if (args["video-legacy"] !== undefined) { console.error( @@ -810,7 +810,7 @@ export default defineCommand({ `The -V short flag no longer maps to --video. Use --video (or -v). Example:\n npx hyperframes init ${args.name ?? "my-video"} --video "${args["video-legacy"]}"`, ), ); - failCommand(); + failUsage(); } const exampleFlag = args.example; const videoFlag = args.video; @@ -864,7 +864,7 @@ export default defineCommand({ if (videoFlag && audioFlag) { console.error(c.error("Cannot use --video and --audio together")); - failCommand(); + failUsage(); } // Validate source files before creating destDir so a failed run does diff --git a/packages/cli/src/commands/lambda.ts b/packages/cli/src/commands/lambda.ts index 7cfed00481..d90c755acc 100644 --- a/packages/cli/src/commands/lambda.ts +++ b/packages/cli/src/commands/lambda.ts @@ -1,4 +1,4 @@ -import { failCommand, failUsage } from "../utils/commandResult.js"; +import { CliUsageError, failCommand, failUsage } from "../utils/commandResult.js"; /** * `hyperframes lambda` — top-level dispatcher for AWS Lambda subcommands. * @@ -289,7 +289,7 @@ export default defineCommand({ console.error( `[lambda sites] unknown verb "${String(args.target)}". Only "create" is supported.`, ); - failCommand(); + failUsage(); } const projectDir = args.extra as string | undefined; if (!projectDir) { @@ -319,14 +319,14 @@ export default defineCommand({ const height = parsePositiveInt(args.height, "--height"); if (width === undefined || height === undefined) { console.error("[lambda render] --width and --height are required."); - failCommand(); + failUsage(); } const fpsRaw = parseIntFlag(args.fps) ?? defaultCloudFps(projectDir); if (!isDistributedFps(fpsRaw)) { console.error( `[lambda render] --fps must be one of ${DISTRIBUTED_FPS.join(", ")}; got ${fpsRaw}.`, ); - failCommand(); + failUsage(); } const { runRender } = await import("./lambda/render.js"); const renderResolution = parseOutputResolution(args["output-resolution"]); @@ -376,14 +376,14 @@ export default defineCommand({ const height = parsePositiveInt(args.height, "--height"); if (width === undefined || height === undefined) { console.error("[lambda render-batch] --width and --height are required."); - failCommand(); + failUsage(); } const fpsRaw = parseIntFlag(args.fps) ?? defaultCloudFps(projectDir); if (!isDistributedFps(fpsRaw)) { console.error( `[lambda render-batch] --fps must be one of ${DISTRIBUTED_FPS.join(", ")}; got ${fpsRaw}.`, ); - failCommand(); + failUsage(); } const { runRenderBatch } = await import("./lambda/render-batch.js"); const batchResolution = parseOutputResolution(args["output-resolution"]); @@ -446,7 +446,7 @@ export default defineCommand({ } default: console.error(`${c.error("Unknown subcommand:")} ${subcommand}\n${HELP}`); - failCommand(); + failUsage(); } }, }); @@ -466,7 +466,7 @@ function parsePositiveInt(raw: unknown, flagName: string): number | undefined { const n = parseIntFlag(raw); if (n === undefined) return undefined; if (!Number.isInteger(n) || n < 1) { - throw new Error(`[lambda] ${flagName} must be a positive integer; got ${n}`); + throw new CliUsageError(`[lambda] ${flagName} must be a positive integer; got ${n}`); } return n; } @@ -486,7 +486,7 @@ function parseEnum( if (raw === undefined || raw === null || raw === "") return defaultValue; const s = String(raw); if ((allowed as readonly string[]).includes(s)) return s as T; - throw new Error(`${errorPrefix} must be ${allowed.join("|")}; got ${s}`); + throw new CliUsageError(`${errorPrefix} must be ${allowed.join("|")}; got ${s}`); } const FORMATS = [ diff --git a/packages/cli/src/commands/lambda/sam.ts b/packages/cli/src/commands/lambda/sam.ts index e9bf49feca..c1944099fa 100644 --- a/packages/cli/src/commands/lambda/sam.ts +++ b/packages/cli/src/commands/lambda/sam.ts @@ -21,6 +21,7 @@ import { execFileSync, spawnSync } from "node:child_process"; import { existsSync } from "node:fs"; import { join } from "node:path"; +import { CliUsageError } from "../../utils/commandResult.js"; /** Throws with a clear hint when the SAM CLI is not on PATH. */ function assertSamAvailable(): void { @@ -83,7 +84,7 @@ export function parseReservedConcurrency(raw: unknown): number | undefined { if (raw === "none") return -1; const n = Number(raw); if (!Number.isInteger(n) || n < 1) { - throw new Error( + throw new CliUsageError( `[lambda] --concurrency must be a positive integer or "none"; got ${String(raw)}`, ); } diff --git a/packages/cli/src/commands/media-treatment.test.ts b/packages/cli/src/commands/media-treatment.test.ts index 4a5377c3f7..a335416e17 100644 --- a/packages/cli/src/commands/media-treatment.test.ts +++ b/packages/cli/src/commands/media-treatment.test.ts @@ -15,7 +15,7 @@ import { mediaTreatmentCommand, resolveMediaTreatmentSource, } from "./media-treatment.js"; -import { CliRuntimeError } from "../utils/commandResult.js"; +import { CliRuntimeError, CliUsageError } from "../utils/commandResult.js"; import { COLOR_GRADING_TOP_LEVEL_KEYS, validateColorGradingContract, @@ -812,7 +812,7 @@ describe("applyMediaTreatmentToHtml", () => { runCommand(mediaTreatmentCommand, { rawArgs: ["--project", project, "--selector", "#hero", "--grading", grading], }), - ).rejects.toThrow(CliRuntimeError); + ).rejects.toThrow(CliUsageError); expect(error).toHaveBeenLastCalledWith(expect.stringContaining("--grading requires --apply")); expect(readFileSync(file, "utf8")).toBe(VIDEO); diff --git a/packages/cli/src/commands/media-treatment.ts b/packages/cli/src/commands/media-treatment.ts index e2546d8d3a..42d29627b9 100644 --- a/packages/cli/src/commands/media-treatment.ts +++ b/packages/cli/src/commands/media-treatment.ts @@ -29,7 +29,7 @@ import { lintHyperframeHtml } from "@hyperframes/lint"; import { parseHTML } from "linkedom"; import type { Example } from "./_examples.js"; import { c } from "../ui/colors.js"; -import { failCommand } from "../utils/commandResult.js"; +import { CliUsageError, failCommand, failUsage } from "../utils/commandResult.js"; import { readOptionalString } from "../utils/pathArgs.js"; import { projectCompositionFiles } from "../utils/mountedCompositions.js"; import { readJsonFlag } from "../utils/jsonFlag.js"; @@ -528,7 +528,7 @@ function parseSelectorIndex(raw: string | undefined): number | undefined { if (raw === undefined) return undefined; const value = Number(raw); if (!Number.isInteger(value) || value < 0) { - throw new Error("--selector-index must be a non-negative integer"); + throw new CliUsageError("--selector-index must be a non-negative integer"); } return value; } @@ -569,15 +569,15 @@ function parseGrading( ): unknown { if (clear) { if (raw !== undefined || apply) { - throw new Error("Use either --apply with --grading or --clear, not both"); + throw new CliUsageError("Use either --apply with --grading or --clear, not both"); } return undefined; } if (!apply && !dryRun) { - if (raw !== undefined) throw new Error("--grading requires --apply or --dry-run"); - throw new Error("Use --apply with --grading or --clear"); + if (raw !== undefined) throw new CliUsageError("--grading requires --apply or --dry-run"); + throw new CliUsageError("Use --apply with --grading or --clear"); } - if (raw === undefined) throw new Error("--apply requires --grading "); + if (raw === undefined) throw new CliUsageError("--apply requires --grading "); return readJsonFlag("--grading", raw); } @@ -613,9 +613,10 @@ function runCapabilityQuery(args: MediaTreatmentCommandArgs): void { args.apply, args.analyze, ].some(Boolean); - if (hasMutationOption) throw new Error("--capabilities cannot be combined with mutation options"); + if (hasMutationOption) + throw new CliUsageError("--capabilities cannot be combined with mutation options"); if (args.all === true && capability) - throw new Error("Use either --all or --capability, not both"); + throw new CliUsageError("Use either --all or --capability, not both"); let capabilities: unknown = getMediaTreatmentCapabilityOverview(); if (args.all === true) capabilities = getHfColorGradingCapabilities(); @@ -647,7 +648,7 @@ function resolveMutationFile(args: MediaTreatmentCommandArgs, selector: string | (selector ? fileDeclaringSelector(project.dir, selector) : "index.html"); const filePath = resolve(project.dir, fileArg); if (!isPathInside(filePath, project.dir) || !filePath.toLowerCase().endsWith(".html")) { - throw new Error("--file must be an HTML file inside the project"); + throw new CliUsageError("--file must be an HTML file inside the project"); } if (!existsSync(filePath)) throw new Error(`Composition file not found: ${fileArg}`); return { project, filePath }; @@ -656,14 +657,14 @@ function resolveMutationFile(args: MediaTreatmentCommandArgs, selector: string | function analyzeTarget(args: MediaTreatmentCommandArgs) { const selector = readOptionalString(args.selector); const { project, filePath } = resolveMutationFile(args, selector); - if (!selector) throw new Error("--selector is required"); + if (!selector) throw new CliUsageError("--selector is required"); if ( readOptionalString(args.grading) || args.clear === true || args.apply === true || args["dry-run"] === true ) { - throw new Error("--analyze cannot be combined with mutation options"); + throw new CliUsageError("--analyze cannot be combined with mutation options"); } const selectorIndex = parseSelectorIndex(readOptionalString(args["selector-index"])); const { element, tag } = selectMediaElement( @@ -689,7 +690,7 @@ function analyzeTarget(args: MediaTreatmentCommandArgs) { async function prepareMutation(args: MediaTreatmentCommandArgs) { const selector = readOptionalString(args.selector); const { project, filePath } = resolveMutationFile(args, selector); - if (!selector) throw new Error("--selector is required"); + if (!selector) throw new CliUsageError("--selector is required"); const clear = args.clear === true; const apply = args.apply === true; const dryRun = args["dry-run"] === true; @@ -770,8 +771,13 @@ async function printMutation(args: MediaTreatmentCommandArgs): Promise { function printFailure(error: unknown, json: boolean): void { const message = failureMessage(error); - if (json) console.log(JSON.stringify(withMeta(jsonFailure("runtime_error", message)))); - else console.error(`${c.error("✗")} ${message}`); + const usage = error instanceof CliUsageError; + if (json) { + console.log( + JSON.stringify(withMeta(jsonFailure(usage ? error.code : "runtime_error", message))), + ); + } else console.error(`${c.error("✗")} ${message}`); + if (usage) failUsage(); failCommand(1, error); } diff --git a/packages/cli/src/commands/models.ts b/packages/cli/src/commands/models.ts index aec5a0269f..2510509c41 100644 --- a/packages/cli/src/commands/models.ts +++ b/packages/cli/src/commands/models.ts @@ -3,7 +3,7 @@ import * as clack from "../ui/prompts.js"; import type { Example } from "./_examples.js"; import { c } from "../ui/colors.js"; import { formatBytes } from "../ui/format.js"; -import { failCommand, setCommandExitCode } from "../utils/commandResult.js"; +import { failCommand, failUsage, setCommandExitCode } from "../utils/commandResult.js"; import { createRenderCancellationScope } from "../utils/renderCancellation.js"; import { PARAKEET_INSTALL_COMMAND, PARAKEET_MODEL_LABEL } from "../whisper/parakeet.js"; import type { ParakeetRunner } from "../whisper/parakeetRunner.js"; @@ -14,9 +14,11 @@ export const examples: Example[] = [ ["Download the Parakeet speech model that transcribe uses", PARAKEET_INSTALL_COMMAND], ]; -function fail(message: string, json: boolean): never { - if (json) console.log(JSON.stringify(jsonFailure("runtime_error", message))); +function fail(message: string, json: boolean, usage = false): never { + if (json) + console.log(JSON.stringify(jsonFailure(usage ? "usage_error" : "runtime_error", message))); else console.error(c.error(message)); + if (usage) failUsage(); failCommand(); } @@ -163,6 +165,7 @@ export default defineCommand({ fail( `Unknown: models ${[args.action, args.name].filter(Boolean).join(" ")}. Try: hyperframes models list, or ${PARAKEET_INSTALL_COMMAND}`, args.json, + true, ); } return installParakeet(args.json); diff --git a/packages/cli/src/commands/normalize-audio.ts b/packages/cli/src/commands/normalize-audio.ts index f9fa146458..02bb1f024c 100644 --- a/packages/cli/src/commands/normalize-audio.ts +++ b/packages/cli/src/commands/normalize-audio.ts @@ -19,7 +19,7 @@ import { defineCommand } from "../utils/defineCommand.js"; import { c } from "../ui/colors.js"; import { projectCompositionFiles } from "../utils/mountedCompositions.js"; import { resolveProject } from "../utils/project.js"; -import { failCommand } from "../utils/commandResult.js"; +import { CliUsageError, failCommand, failUsage } from "../utils/commandResult.js"; import type { Example } from "./_examples.js"; import { jsonFailure } from "../utils/jsonError.js"; @@ -35,8 +35,13 @@ export { const DEFAULT_TOLERANCE_LU = 0.5; function fail(message: string, json: boolean, cause?: unknown): never { - if (json) console.log(JSON.stringify(jsonFailure("runtime_error", message), null, 2)); - else console.error(c.error(message)); + const usage = cause instanceof CliUsageError; + if (json) { + console.log( + JSON.stringify(jsonFailure(usage ? cause.code : "runtime_error", message), null, 2), + ); + } else console.error(c.error(message)); + if (usage) failUsage(); failCommand(1, cause); } @@ -81,7 +86,7 @@ function selectedTags(tags: readonly ProjectAudioTag[], referenceId: string, tar const referenceTag = byId(tags, referenceId, "Reference"); const targetTag = byId(tags, targetId, "Target"); if (referenceTag.id === targetTag.id) { - throw new Error("Reference and target must be different audio elements"); + throw new CliUsageError("Reference and target must be different audio elements"); } return { referenceTag, targetTag }; } @@ -123,14 +128,15 @@ function writeAtomically(path: string, contents: string): void { function parsedTolerance(raw: string): number { const tolerance = Number(raw); if (!Number.isFinite(tolerance) || tolerance < 0) { - throw new Error("--tolerance must be a non-negative number"); + throw new CliUsageError("--tolerance must be a non-negative number"); } return tolerance; } function parsedLufs(raw: string): number { const lufs = Number(raw); - if (!Number.isFinite(lufs) || lufs >= 0) throw new Error("--lufs must be a negative number"); + if (!Number.isFinite(lufs) || lufs >= 0) + throw new CliUsageError("--lufs must be a negative number"); return lufs; } @@ -173,9 +179,9 @@ async function plannedNormalization( } async function normalizeAudio(options: NormalizeAudioOptions): Promise { + const tolerance = parsedTolerance(options.tolerance); const project = resolveProject(options.dir); const tags = projectAudioTags(project.dir); - const tolerance = parsedTolerance(options.tolerance); const { targetTag, withinTolerance, plan } = await plannedNormalization( project.dir, tags, diff --git a/packages/cli/src/commands/play.ts b/packages/cli/src/commands/play.ts index 5df9fbe23f..19045347a5 100644 --- a/packages/cli/src/commands/play.ts +++ b/packages/cli/src/commands/play.ts @@ -1,4 +1,4 @@ -import { setCommandExitCode } from "../utils/commandResult.js"; +import { failUsage, setCommandExitCode } from "../utils/commandResult.js"; import { defineCommand } from "../utils/defineCommand.js"; import type { Example } from "./_examples.js"; import { existsSync, readFileSync } from "node:fs"; @@ -91,16 +91,14 @@ export default defineCommand({ startPort = parsePort(args.port ?? "3003"); } catch (error) { clack.log.error(normalizeErrorMessage(error)); - setCommandExitCode(1); - return; + failUsage(); } const project = resolveProject(args.dir); // Validation: --user-data-dir requires --browser-path if (args["user-data-dir"] && !args["browser-path"]) { clack.log.error("--user-data-dir requires --browser-path"); - setCommandExitCode(1); - return; + failUsage(); } // Validation: --remote-debugging-port deps const depsError = validateRemoteDebuggingPortDeps({ @@ -110,8 +108,7 @@ export default defineCommand({ }); if (depsError) { clack.log.error(depsError); - setCommandExitCode(1); - return; + failUsage(); } // Parse --remote-debugging-port before any server setup so an invalid value // exits cleanly instead of leaving an orphan listening socket behind. @@ -122,8 +119,7 @@ export default defineCommand({ ); } catch (err) { clack.log.error((err as Error).message); - setCommandExitCode(1); - return; + failUsage(); } // Resolve runtime path — same logic as studioServer.ts diff --git a/packages/cli/src/commands/present.ts b/packages/cli/src/commands/present.ts index f52e1ab860..853b945f1e 100644 --- a/packages/cli/src/commands/present.ts +++ b/packages/cli/src/commands/present.ts @@ -1,4 +1,4 @@ -import { setCommandExitCode } from "../utils/commandResult.js"; +import { failUsage, setCommandExitCode } from "../utils/commandResult.js"; import { defineCommand } from "../utils/defineCommand.js"; import type { Example } from "./_examples.js"; import { existsSync, readFileSync } from "node:fs"; @@ -55,15 +55,13 @@ export default defineCommand({ startPort = parsePort(args.port ?? "3004"); } catch (error) { clack.log.error(normalizeErrorMessage(error)); - setCommandExitCode(1); - return; + failUsage(); } const project = resolveProject(args.dir); if (args["user-data-dir"] && !args["browser-path"]) { clack.log.error("--user-data-dir requires --browser-path"); - setCommandExitCode(1); - return; + failUsage(); } const depsError = validateRemoteDebuggingPortDeps({ browserPath: args["browser-path"] as string | undefined, @@ -72,8 +70,7 @@ export default defineCommand({ }); if (depsError) { clack.log.error(depsError); - setCommandExitCode(1); - return; + failUsage(); } let remoteDebuggingPort: number | undefined; try { @@ -82,8 +79,7 @@ export default defineCommand({ ); } catch (err) { clack.log.error((err as Error).message); - setCommandExitCode(1); - return; + failUsage(); } const playerPath = resolvePlayerPath(); diff --git a/packages/cli/src/commands/serverPort.test.ts b/packages/cli/src/commands/serverPort.test.ts index 2ac7818f12..e70408f477 100644 --- a/packages/cli/src/commands/serverPort.test.ts +++ b/packages/cli/src/commands/serverPort.test.ts @@ -24,9 +24,9 @@ describe.each(commands)("%s --port", (_name, run) => { it.each(["0", "65536", "3003oops", "22.5"])( "rejects invalid value %j before resolving a project", async (value) => { - await run(["--port", value, "--no-open"]); - - expect(consumeCommandResult()).toMatchObject({ exitCode: 1 }); + await expect(run(["--port", value, "--no-open"])).rejects.toMatchObject({ + result: { exitCode: 2, kind: "usage_error" }, + }); expect(clack.log.error).toHaveBeenCalledWith("--port must be an integer between 1 and 65535"); }, ); diff --git a/packages/cli/src/commands/snapshot.ts b/packages/cli/src/commands/snapshot.ts index e721c461ad..39cbabcdeb 100644 --- a/packages/cli/src/commands/snapshot.ts +++ b/packages/cli/src/commands/snapshot.ts @@ -12,6 +12,7 @@ import { reportArtifacts } from "../utils/toolContext.js"; import { withProjectLock } from "../utils/projectLock.js"; import { parseSnapshotAt, + parseSnapshotFrames, parseSnapshotRange, sampleSnapshotRange, resolveSnapshotCompositionRange, @@ -29,7 +30,7 @@ import { } from "@hyperframes/studio-server/history"; import { pngDimensions } from "../utils/pngDimensions.js"; import { runWithConcurrencyLimit } from "../utils/concurrency.js"; -import { CliRuntimeError, failCommand } from "../utils/commandResult.js"; +import { CliRuntimeError, CliUsageError, failCommand } from "../utils/commandResult.js"; // fallow-ignore-file complexity import { defineCommand } from "../utils/defineCommand.js"; import { @@ -207,7 +208,7 @@ function assertDistinctKeepNames(times: number[]): void { const label = formatSnapshotTimestamp(time); const other = timeByLabel.get(label); if (other !== undefined && other !== time) { - throw new Error( + throw new CliUsageError( `--keep would name both ${other} and ${time} at-${label}.png; pass times at least 1 ms apart`, ); } @@ -1480,21 +1481,21 @@ export default defineCommand({ failCommand(); }; if ([args.at, args.range, args.composition].filter((value) => value !== undefined).length > 1) - throw new Error("Use only one of --at, --range, or --composition"); + throw new CliUsageError("Use only one of --at, --range, or --composition"); const reviewMode = args.review === true; if (reviewMode) { const clash = (["at", "range", "zoom", "keep", "against"] as const).find( (flag) => args[flag] !== undefined && args[flag] !== false, ); - if (clash) throw new Error(`--review picks its own frames; drop --${clash}`); + if (clash) throw new CliUsageError(`--review picks its own frames; drop --${clash}`); } - const frames = parseInt(args.frames as string, 10) || 5; + const frames = args.frames === undefined ? 5 : parseSnapshotFrames(String(args.frames)); const range = args.range === undefined ? undefined : parseSnapshotRange(String(args.range)); const atTimestamps = args.at === undefined ? undefined : parseSnapshotAt(String(args.at)); if (args.keep === true && atTimestamps) assertDistinctKeepNames(atTimestamps); const composition = args.composition === undefined ? undefined : String(args.composition).trim(); - if (composition === "") throw new Error("--composition requires a composition ID"); + if (composition === "") throw new CliUsageError("--composition requires a composition ID"); const project = resolveProject(args.dir); const lintResult = await lintProject(project.dir); if (hasDefinitiveEntryMismatch(lintResult)) { diff --git a/packages/cli/src/commands/snapshotSampling.test.ts b/packages/cli/src/commands/snapshotSampling.test.ts index 7cb6bbeb9a..23b1e79eaa 100644 --- a/packages/cli/src/commands/snapshotSampling.test.ts +++ b/packages/cli/src/commands/snapshotSampling.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from "vitest"; import { + parseSnapshotFrames, parseSnapshotRange, sampleSnapshotRange, resolveSnapshotCompositionRange, @@ -18,6 +19,9 @@ describe("snapshot scoped sampling", () => { expect(() => parseSnapshotRange(v)).toThrow(); expect(parseSnapshotRange("1.8:2.5")).toEqual([1.8, 2.5]); expect(parseSnapshotRange("1.8,2.5")).toEqual([1.8, 2.5]); + for (const v of ["abc", "-3", "0", "2.5", ""]) + expect(() => parseSnapshotFrames(v)).toThrow("--frames must be a positive integer"); + expect(parseSnapshotFrames("10")).toBe(10); for (const n of [0, -1, 1.5, NaN]) expect(() => sampleSnapshotRange([1, 2], n, 5)).toThrow(); expect(() => sampleSnapshotRange([1, 6], 5, 5)).toThrow(); }); diff --git a/packages/cli/src/commands/snapshotSampling.ts b/packages/cli/src/commands/snapshotSampling.ts index 9cac6bc4be..2618cfc1c3 100644 --- a/packages/cli/src/commands/snapshotSampling.ts +++ b/packages/cli/src/commands/snapshotSampling.ts @@ -1,5 +1,6 @@ import { parseHTML } from "linkedom"; import { createRuntimeStartTimeResolver } from "@hyperframes/core"; +import { CliUsageError } from "../utils/commandResult.js"; export type SnapshotRange = [number, number]; export function parseSnapshotRange(value: string): SnapshotRange { const parts = value.split(/[,:]/); @@ -10,13 +11,19 @@ export function parseSnapshotRange(value: string): SnapshotRange { range[0]! < 0 || range[1]! <= range[0]! ) - throw new Error("--range must be START,END in seconds, with 0 <= START < END"); + throw new CliUsageError("--range must be START,END in seconds, with 0 <= START < END"); return [range[0]!, range[1]!]; } +export function parseSnapshotFrames(value: string): number { + const frames = Number(value); + if (value.trim() === "" || !Number.isInteger(frames) || frames < 1) + throw new CliUsageError("--frames must be a positive integer"); + return frames; +} export function parseSnapshotAt(value: string): number[] { const times = value.split(",").map((part) => (part.trim() === "" ? NaN : Number(part))); if (!times.every((time) => Number.isFinite(time) && time >= 0)) - throw new Error("--at must be comma-separated times in seconds, each 0 or more"); + throw new CliUsageError("--at must be comma-separated times in seconds, each 0 or more"); return times; } export function sampleSnapshotRange( diff --git a/packages/cli/src/commands/telemetry.ts b/packages/cli/src/commands/telemetry.ts index 6b9e17e8cc..0a67d52044 100644 --- a/packages/cli/src/commands/telemetry.ts +++ b/packages/cli/src/commands/telemetry.ts @@ -7,7 +7,7 @@ import { } from "../telemetry/config.js"; import { effectiveTelemetryStatus, type TelemetryStatusSource } from "../telemetry/policy.js"; import { c } from "../ui/colors.js"; -import { failCommand } from "../utils/commandResult.js"; +import { failCommand, failUsage } from "../utils/commandResult.js"; import type { Example } from "./_examples.js"; export const examples: Example[] = [ @@ -123,7 +123,7 @@ ${c.dim("You can also set")} ${c.accent("HYPERFRAMES_NO_TELEMETRY=1")} ${c.dim(" console.error( `${c.error("Unknown subcommand:")} ${subcommand}\n\nRun ${c.accent("hyperframes telemetry --help")} for usage.`, ); - failCommand(); + failUsage(); } }, }); diff --git a/packages/cli/src/commands/tts.ts b/packages/cli/src/commands/tts.ts index 6073de1ced..ccbdb77a08 100644 --- a/packages/cli/src/commands/tts.ts +++ b/packages/cli/src/commands/tts.ts @@ -100,7 +100,7 @@ export default defineCommand({ const input = args["text-file"] ?? args.input; if (!input) { console.error(c.error("Provide text to speak, or use --list to see available voices.")); - failCommand(); + failUsage(); } let text: string; @@ -126,7 +126,7 @@ export default defineCommand({ if (!Number.isFinite(speed) || speed < 0.1 || speed > 3) { console.error(c.error("Speed must be a number between 0.1 and 3.0")); - failCommand(); + failUsage(); } const inferredLang = inferLangFromVoiceId(voice); diff --git a/packages/cli/src/ui/prompts.test.ts b/packages/cli/src/ui/prompts.test.ts index 9713437f92..9141e440a5 100644 --- a/packages/cli/src/ui/prompts.test.ts +++ b/packages/cli/src/ui/prompts.test.ts @@ -46,14 +46,14 @@ describe("prompt wrapper", () => { } }); - it("an unattended spinner writes only its outcome, as a plain line on stderr", () => { + it("an unattended spinner writes its start and outcome, not its progress, on stderr", () => { const write = vi.spyOn(process.stderr, "write").mockImplementation(() => true); try { const spin = prompts.spinner(); - spin.start("Checking..."); - spin.message("Still checking..."); + spin.start("\u001b[36mTranscoding\u001b[39m clip.mov"); + spin.message("Still transcoding..."); spin.stop("\u001b[32mDone\u001b[39m"); - expect(write.mock.calls).toEqual([["Done\n"]]); + expect(write.mock.calls).toEqual([["Transcoding clip.mov\n"], ["Done\n"]]); expect(clack.spinner).not.toHaveBeenCalled(); } finally { write.mockRestore(); diff --git a/packages/cli/src/ui/prompts.ts b/packages/cli/src/ui/prompts.ts index f3bd1aca04..9f94ac9dd6 100644 --- a/packages/cli/src/ui/prompts.ts +++ b/packages/cli/src/ui/prompts.ts @@ -57,7 +57,7 @@ function lineSpinner(): SpinnerResult { if (message) process.stderr.write(`${stripVTControlCharacters(message)}\n`); }; return { - start: () => {}, + start: say, message: () => {}, stop: say, cancel: say, @@ -67,7 +67,7 @@ function lineSpinner(): SpinnerResult { }; } -/** Unattended, only the outcome is written, as one plain line on stderr. */ +/** Unattended, the start and outcome messages are each written as one plain line on stderr. */ export function spinner(opts?: SpinnerOptions): SpinnerResult { return isUnattended() ? lineSpinner() : clack.spinner(opts); } diff --git a/packages/cli/src/utils/jsonError.ts b/packages/cli/src/utils/jsonError.ts index 9b1f447fda..616143daab 100644 --- a/packages/cli/src/utils/jsonError.ts +++ b/packages/cli/src/utils/jsonError.ts @@ -3,6 +3,7 @@ import { CliRuntimeError, CliUsageError, failCommand, + failUsage, type CommandResult, } from "./commandResult.js"; @@ -22,7 +23,7 @@ export function jsonFailure(code: string, message: string, hint?: string): JsonF return { ok: false, error: hint ? { code, message, hint } : { code, message } }; } -/** Reports a caught failure as the --json envelope or one line on stderr, then fails the command. */ +/** Reports a caught failure as the --json envelope or one stderr line; exit 2 if usage, else 1. */ export function failCaught( error: unknown, json: boolean, @@ -30,8 +31,10 @@ export function failCaught( print: (payload: object) => void = (payload) => console.log(JSON.stringify(payload)), ): never { const message = failureMessage(error); - if (json) print(jsonFailure("runtime_error", message)); + const usage = error instanceof CliUsageError; + if (json) print(jsonFailure(usage ? error.code : "runtime_error", message)); else console.error(line(message)); + if (usage) failUsage(); failCommand(); } diff --git a/packages/cli/src/utils/parseOutputResolution.ts b/packages/cli/src/utils/parseOutputResolution.ts index 3b0a00de36..69fe91995d 100644 --- a/packages/cli/src/utils/parseOutputResolution.ts +++ b/packages/cli/src/utils/parseOutputResolution.ts @@ -21,6 +21,7 @@ import { type CanvasResolution, resolveResolutionFlagPair } from "@hyperframes/core"; import { VALID_CANVAS_RESOLUTIONS } from "@hyperframes/core"; +import { CliUsageError } from "./commandResult.js"; /** * Free-text prefix the thrown error is scoped to (e.g. `"[cloudrun render]"`, @@ -60,7 +61,7 @@ export function parseOutputResolutionFlag( const { outputResolution, outputResolutionAspectAgnostic } = resolveResolutionFlagPair(asString); if (outputResolution) return { outputResolution, outputResolutionAspectAgnostic }; const aliasHint = options.aliasHint ?? "1080p, 4k, uhd, hd, …"; - throw new Error( + throw new CliUsageError( `${options.surfaceLabel} --output-resolution must be one of ${VALID_CANVAS_RESOLUTIONS.join("|")} ` + `(or an alias: ${aliasHint}); got ${asString}`, ); diff --git a/packages/cli/src/utils/variables.ts b/packages/cli/src/utils/variables.ts index 7eddaeba34..a0e1974fc9 100644 --- a/packages/cli/src/utils/variables.ts +++ b/packages/cli/src/utils/variables.ts @@ -1,4 +1,4 @@ -import { failCommand } from "./commandResult.js"; +import { failCommand, failUsage } from "./commandResult.js"; /** * Shared `--variables` / `--variables-file` / `--strict-variables` parsing * and validation helpers used by both `hyperframes render` (in-process) and @@ -135,6 +135,9 @@ export function resolveVariablesArg( if (!result.ok) { const { title, message } = variablesErrorMessage(result.error); errorBox(title, message); + const { kind } = result.error; + const flagsAreWrong = kind === "conflict" || (inline != null && kind !== "read-error"); + if (flagsAreWrong) failUsage(); failCommand(); } return result.value;