mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-10-30 09:16:55 +00:00
fix(cli): reject extra positional arguments on every command (#4905)
* fix(cli): reject extra positional arguments on every command * fix(cli): read the command name without assuming a resolved meta * fix(cli): join free-text words for catalog and tts, reject a bad repo root * test(cli): load the listed commands through static imports * fix(cli): keep a tts file path out of the word join and read the parsed json flag * fix(cli): apply the file exception only to a tts .txt path * fix(cli): keep usageLine private and test the json fallback for undeclared json * fix(cli): keep extra arguments and the repo root value out of telemetry * test(cli): type the skills update lookup * fix(cli): read json mode from the parsed flag, reject tts words with --text-file, restore add's hint * fix(cli): check tts words with --text-file before --list and test --json=false
This commit is contained in:
@@ -2,6 +2,11 @@ import { readFileSync } from "node:fs";
|
||||
import { dirname, join } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { CommandDef } from "citty";
|
||||
import {
|
||||
ACCEPTS_EXTRA_POSITIONALS,
|
||||
JOINS_EXTRA_POSITIONALS,
|
||||
} from "./utils/reject-extra-positionals.js";
|
||||
|
||||
const cliSource = readFileSync(join(dirname(fileURLToPath(import.meta.url)), "cli.ts"), "utf8");
|
||||
const helpSource = readFileSync(join(dirname(fileURLToPath(import.meta.url)), "help.ts"), "utf8");
|
||||
@@ -12,7 +17,32 @@ function commandLoaderBlock(): string {
|
||||
return match![1]!;
|
||||
}
|
||||
|
||||
// Vite cannot resolve a template-string import, so each listed top-level command is named here.
|
||||
const LIST_COMMANDS: Record<string, () => Promise<{ default: unknown }>> = {
|
||||
catalog: () => import("./commands/catalog.js"),
|
||||
compare: () => import("./commands/compare.js"),
|
||||
figma: () => import("./commands/figma.js"),
|
||||
skills: () => import("./commands/skills.js"),
|
||||
timeline: () => import("./commands/timeline.js"),
|
||||
tts: () => import("./commands/tts.js"),
|
||||
};
|
||||
|
||||
describe("CLI command registration", () => {
|
||||
it.each([...ACCEPTS_EXTRA_POSITIONALS, ...JOINS_EXTRA_POSITIONALS])(
|
||||
"names a real command in the extra-positional lists: %s",
|
||||
async (path) => {
|
||||
const [top, ...subs] = path.split(" ");
|
||||
expect(commandLoaderBlock()).toMatch(new RegExp(`\\b${top}:\\s*\\(\\)\\s*=>`));
|
||||
let cmd = (await LIST_COMMANDS[top!]!()).default as CommandDef;
|
||||
expect(cmd.meta).toMatchObject({ name: top });
|
||||
for (const sub of subs) {
|
||||
const entry = (cmd.subCommands as Record<string, unknown>)[sub!];
|
||||
cmd = (typeof entry === "function" ? await entry() : entry) as CommandDef;
|
||||
expect(cmd, `${path} resolves to a subcommand`).toBeTruthy();
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("registers keyframes as the only keyframe inspection command", () => {
|
||||
const loaders = commandLoaderBlock();
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it, vi, beforeEach, afterEach } from "vitest";
|
||||
import { parseArgs as parseCittyArgs, type ArgsDef } from "citty";
|
||||
import { runCommand as runCittyCommand, type CommandDef } from "citty";
|
||||
import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
@@ -18,6 +18,7 @@ import addCommand, {
|
||||
} from "./add.js";
|
||||
import { trackRegistryItemAdded } from "../telemetry/events.js";
|
||||
import { CliUsageError } from "../utils/commandResult.js";
|
||||
import { trackCommandFailures } from "../utils/command-failure-tracking.js";
|
||||
|
||||
// Assert the emitted payload rather than the transport: `shouldTrack()` is
|
||||
// already false under test (dev mode / no PostHog key), so a real call would
|
||||
@@ -539,22 +540,21 @@ describe("add command run() — extra positional arguments", () => {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
async function runCommand(args: Record<string, unknown>): Promise<void> {
|
||||
await (addCommand.run as (ctx: { args: Record<string, unknown> }) => Promise<void>)({ args });
|
||||
async function runWrapped(rawArgs: string[]): Promise<void> {
|
||||
const cmd = await trackCommandFailures(() => Promise.resolve(addCommand as CommandDef))();
|
||||
await runCittyCommand(cmd, { rawArgs });
|
||||
}
|
||||
|
||||
it("rejects `add a --dir <dir> b c` under real citty parsing, before any registry call", async () => {
|
||||
it("rejects `add a --dir <dir> b c` through the CLI wrapper, before any registry call", async () => {
|
||||
const fetchSpy = vi.fn();
|
||||
vi.stubGlobal("fetch", fetchSpy);
|
||||
|
||||
const parsed = parseCittyArgs(["a", "--dir", dir, "b", "c"], addCommand.args as ArgsDef);
|
||||
expect(parsed._).toEqual(["a", "b", "c"]);
|
||||
|
||||
await expect(runCommand(parsed as unknown as Record<string, unknown>)).rejects.toThrow(
|
||||
CliUsageError,
|
||||
);
|
||||
await expect(runWrapped(["a", "--dir", dir, "b", "c"])).rejects.toThrow(CliUsageError);
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
expect(errorSpy.mock.calls.flat().join(" ")).toContain("extra arguments: b, c");
|
||||
expect(errorSpy.mock.calls.flat().join(" ")).toContain(
|
||||
"Unexpected extra arguments for hyperframes add: b, c",
|
||||
);
|
||||
expect(errorSpy.mock.calls.flat().join(" ")).toContain("Run add once per item");
|
||||
});
|
||||
|
||||
it("does not fire on a normal single-item invocation", async () => {
|
||||
@@ -562,7 +562,7 @@ describe("add command run() — extra positional arguments", () => {
|
||||
writeRegistryConfig(dir);
|
||||
|
||||
await expect(
|
||||
runCommand({ name: "my-block", _: ["my-block"], dir, clipboard: false, json: true }),
|
||||
runWrapped(["my-block", "--dir", dir, "--no-clipboard", "--json"]),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { failCommand, failUsage } from "../utils/commandResult.js";
|
||||
import { failCommand } from "../utils/commandResult.js";
|
||||
import { defineCommand } from "citty";
|
||||
import type { Example } from "./_examples.js";
|
||||
|
||||
@@ -491,17 +491,6 @@ export default defineCommand({
|
||||
const json = args.json === true;
|
||||
const skipClipboard = args.clipboard === false;
|
||||
|
||||
// citty binds only the first positional to `name`; the rest land in `args._` and were silently dropped.
|
||||
const extra = args._.slice(1);
|
||||
if (extra.length > 0) {
|
||||
const msg =
|
||||
`add installs one item or tag per invocation. Got extra argument${extra.length === 1 ? "" : "s"}: ` +
|
||||
`${extra.join(", ")}. Run add once per item, or pass a single tag to install every item tagged with it.`;
|
||||
if (json) console.log(JSON.stringify({ ok: false, error: msg }));
|
||||
else console.error(c.error(msg));
|
||||
failUsage();
|
||||
}
|
||||
|
||||
const hasConfigBefore = existsSync(projectConfigPath(projectDir));
|
||||
|
||||
// Try single item first. If it fails, check if the name matches a tag.
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { repoRoot } from "./repoRoot.js";
|
||||
|
||||
describe("repoRoot", () => {
|
||||
const dirs: string[] = [];
|
||||
const tempDir = () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "hf-repo-root-"));
|
||||
dirs.push(dir);
|
||||
return dir;
|
||||
};
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("returns HYPERFRAMES_REPO_ROOT when it is a hyperframes checkout", () => {
|
||||
const root = tempDir();
|
||||
mkdirSync(join(root, "packages", "aws-lambda"), { recursive: true });
|
||||
writeFileSync(join(root, "packages", "aws-lambda", "package.json"), "{}");
|
||||
vi.stubEnv("HYPERFRAMES_REPO_ROOT", root);
|
||||
expect(repoRoot()).toBe(root);
|
||||
});
|
||||
|
||||
it("throws on a HYPERFRAMES_REPO_ROOT that is not a checkout instead of ignoring it", () => {
|
||||
const root = tempDir();
|
||||
vi.stubEnv("HYPERFRAMES_REPO_ROOT", root);
|
||||
expect(() => repoRoot()).toThrow("HYPERFRAMES_REPO_ROOT is not a hyperframes checkout");
|
||||
expect(() => repoRoot()).not.toThrow(root);
|
||||
});
|
||||
});
|
||||
@@ -14,8 +14,12 @@ import { fileURLToPath } from "node:url";
|
||||
|
||||
export function repoRoot(): string {
|
||||
const override = process.env.HYPERFRAMES_REPO_ROOT;
|
||||
if (override && existsSync(resolve(override, "packages", "aws-lambda", "package.json"))) {
|
||||
return override;
|
||||
if (override) {
|
||||
if (existsSync(resolve(override, "packages", "aws-lambda", "package.json"))) return override;
|
||||
throw new Error(
|
||||
"[hyperframes lambda] HYPERFRAMES_REPO_ROOT is not a hyperframes checkout " +
|
||||
"(no packages/aws-lambda/package.json there). Point it at the repo root, or unset it.",
|
||||
);
|
||||
}
|
||||
let dir = dirname(fileURLToPath(import.meta.url));
|
||||
for (let depth = 0; depth < 12; depth++) {
|
||||
|
||||
@@ -3,6 +3,7 @@ import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { assertKnownFlags } from "../utils/reject-unknown-flags.js";
|
||||
import { CliUsageError } from "../utils/commandResult.js";
|
||||
|
||||
const synthesizeMock = vi.fn().mockResolvedValue({
|
||||
durationSeconds: 1,
|
||||
@@ -42,4 +43,26 @@ describe("tts command", () => {
|
||||
expect.objectContaining({ lang: "en-us" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects text words with --text-file even under --list", async () => {
|
||||
const input = join(dir, "script.txt");
|
||||
writeFileSync(input, "From the file\n");
|
||||
vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
|
||||
await expect(
|
||||
ttsCommand.run!({ args: { "text-file": input, input: "hello", list: true } } as never),
|
||||
).rejects.toThrow(CliUsageError);
|
||||
});
|
||||
|
||||
it("rejects text words together with --text-file instead of dropping the words", async () => {
|
||||
const input = join(dir, "script.txt");
|
||||
writeFileSync(input, "From the file\n");
|
||||
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
|
||||
await expect(
|
||||
ttsCommand.run!({ args: { "text-file": input, input: "hello world" } } as never),
|
||||
).rejects.toThrow(CliUsageError);
|
||||
expect(synthesizeMock).not.toHaveBeenCalled();
|
||||
expect(errorSpy.mock.calls.flat().join(" ")).toContain("not both");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,8 +1,9 @@
|
||||
import { failCommand } from "../utils/commandResult.js";
|
||||
import { failCommand, failUsage } from "../utils/commandResult.js";
|
||||
import { isTextFile } from "../utils/textFile.js";
|
||||
// fallow-ignore-file code-duplication
|
||||
import { defineCommand } from "citty";
|
||||
import type { Example } from "./_examples.js";
|
||||
import { existsSync, readFileSync } from "node:fs";
|
||||
import { readFileSync } from "node:fs";
|
||||
|
||||
export const examples: Example[] = [
|
||||
["Generate speech from text", 'hyperframes tts "Welcome to HyperFrames"'],
|
||||
@@ -20,7 +21,7 @@ export const examples: Example[] = [
|
||||
["Read text from a file", "hyperframes tts script.txt"],
|
||||
["List available voices", "hyperframes tts --list"],
|
||||
];
|
||||
import { resolve, extname } from "node:path";
|
||||
import { resolve } from "node:path";
|
||||
import * as clack from "@clack/prompts";
|
||||
import { c } from "../ui/colors.js";
|
||||
import { errorBox } from "../ui/format.js";
|
||||
@@ -84,6 +85,11 @@ export default defineCommand({
|
||||
},
|
||||
// fallow-ignore-next-line complexity
|
||||
async run({ args }) {
|
||||
if (args["text-file"] && args.input) {
|
||||
console.error(c.error("Pass text to speak or --text-file, not both."));
|
||||
failUsage();
|
||||
}
|
||||
|
||||
// ── List voices mode ──────────────────────────────────────────────
|
||||
if (args.list) {
|
||||
return listVoices(args.json);
|
||||
@@ -97,10 +103,8 @@ export default defineCommand({
|
||||
}
|
||||
|
||||
let text: string;
|
||||
const maybeFile = resolve(input);
|
||||
|
||||
if (existsSync(maybeFile) && extname(maybeFile).toLowerCase() === ".txt") {
|
||||
text = readFileSync(maybeFile, "utf-8").trim();
|
||||
if (isTextFile(input)) {
|
||||
text = readFileSync(resolve(input), "utf-8").trim();
|
||||
if (!text) {
|
||||
console.error(c.error("File is empty."));
|
||||
failCommand();
|
||||
|
||||
@@ -1130,6 +1130,25 @@ describe("trackCliError", () => {
|
||||
});
|
||||
|
||||
describe("trackCommandFailure", () => {
|
||||
it("reports an extra-positional usage error by count, never the arguments", async () => {
|
||||
const { resolveExtraPositionals } = await import("../utils/reject-extra-positionals.js");
|
||||
const quiet = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
const cmd = { args: { dir: { type: "positional" } } } as Parameters<
|
||||
typeof resolveExtraPositionals
|
||||
>[0];
|
||||
let thrown: unknown;
|
||||
try {
|
||||
resolveExtraPositionals(cmd, "render", { _: ["./p", "Jane", "555-0100"] });
|
||||
} catch (error) {
|
||||
thrown = error;
|
||||
}
|
||||
quiet.mockRestore();
|
||||
trackCommandFailure("render", thrown);
|
||||
const payload = JSON.stringify(trackEvent.mock.calls.at(-1));
|
||||
expect(payload).toContain("2 unexpected extra arguments for hyperframes render");
|
||||
expect(payload).not.toMatch(/Jane|555-0100/);
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
trackEvent.mockClear();
|
||||
});
|
||||
|
||||
@@ -1,5 +1,10 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { CommandDef } from "citty";
|
||||
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { runCommand, type CommandDef } from "citty";
|
||||
import { CliUsageError } from "./commandResult.js";
|
||||
import { resolveExtraPositionals } from "./reject-extra-positionals.js";
|
||||
|
||||
const trackCommandFailure = vi.fn();
|
||||
vi.mock("../telemetry/events.js", () => ({
|
||||
@@ -123,6 +128,204 @@ describe("trackCommandFailures", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("trackCommandFailures: extra positionals", () => {
|
||||
const leaf = (name: string, run = vi.fn()): CommandDef =>
|
||||
({
|
||||
meta: { name },
|
||||
args: {
|
||||
dir: { type: "positional", required: false },
|
||||
json: { type: "boolean", default: false },
|
||||
},
|
||||
run,
|
||||
}) as CommandDef;
|
||||
const wrap = (cmd: CommandDef) => trackCommandFailures(() => Promise.resolve(cmd))();
|
||||
let errorSpy: ReturnType<typeof vi.spyOn>;
|
||||
let logSpy: ReturnType<typeof vi.spyOn>;
|
||||
const tempDirs: string[] = [];
|
||||
const tempDir = () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "hf-join-"));
|
||||
tempDirs.push(dir);
|
||||
return dir;
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
errorSpy.mockRestore();
|
||||
logSpy.mockRestore();
|
||||
for (const dir of tempDirs.splice(0)) rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("throws a presented error whose message carries a count, never the arguments", async () => {
|
||||
const error = await runCommand(await wrap(leaf("render")), {
|
||||
rawArgs: ["./proj", "secret-launch", "Jane"],
|
||||
}).catch((e: unknown) => e);
|
||||
expect(error).toBeInstanceOf(CliUsageError);
|
||||
expect((error as CliUsageError).result.presented).toBe(true);
|
||||
expect((error as Error).message).toBe("2 unexpected extra arguments for hyperframes render");
|
||||
});
|
||||
|
||||
it("labels a required positional <NAME> and an optional one [NAME]", async () => {
|
||||
const cmd = {
|
||||
meta: { name: "keyframes" },
|
||||
args: { file: { type: "positional" }, at: { type: "positional", required: false } },
|
||||
run: vi.fn(),
|
||||
};
|
||||
await expect(
|
||||
runCommand(await wrap(cmd as CommandDef), { rawArgs: ["a", "b", "c"] }),
|
||||
).rejects.toThrow(CliUsageError);
|
||||
expect(errorSpy.mock.calls.flat().join("\n")).toContain(
|
||||
"Usage: hyperframes keyframes <FILE> [AT]",
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["figma asset", () => import("../commands/figma/asset.js")],
|
||||
[
|
||||
"skills update",
|
||||
async () => ({
|
||||
default: (
|
||||
(await import("../commands/skills.js")).default.subCommands as Record<string, unknown>
|
||||
).update,
|
||||
}),
|
||||
],
|
||||
])("lets the real %s command read several positionals", async (path, load) => {
|
||||
const def = (await (load as () => Promise<{ default: unknown }>)()).default;
|
||||
const cmd = (typeof def === "function" ? await def() : def) as CommandDef;
|
||||
const parsed: Record<string, unknown> = { _: ["one", "two", "three"] };
|
||||
expect(() => resolveExtraPositionals(cmd, path as string, parsed)).not.toThrow();
|
||||
});
|
||||
|
||||
it("rejects a leaf's extra positional, naming it and the usage line, before run()", async () => {
|
||||
const run = vi.fn();
|
||||
await expect(
|
||||
runCommand(await wrap(leaf("render", run)), { rawArgs: ["./proj", "out.mp4"] }),
|
||||
).rejects.toThrow(CliUsageError);
|
||||
expect(run).not.toHaveBeenCalled();
|
||||
expect(errorSpy.mock.calls.flat().join("\n")).toBe(
|
||||
"Unexpected extra argument for hyperframes render: out.mp4\nUsage: hyperframes render [DIR] [OPTIONS]",
|
||||
);
|
||||
});
|
||||
|
||||
it.each(["--json", "--json=true", "--json=1"])(
|
||||
"reports the rejection as JSON on stdout under %s",
|
||||
async (flag) => {
|
||||
await expect(
|
||||
runCommand(await wrap(leaf("lint")), { rawArgs: ["a", "b", flag] }),
|
||||
).rejects.toThrow(CliUsageError);
|
||||
expect(JSON.parse(logSpy.mock.calls[0]![0] as string)).toEqual({
|
||||
ok: false,
|
||||
error:
|
||||
"Unexpected extra argument for hyperframes lint: b\nUsage: hyperframes lint [DIR] [OPTIONS]",
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it("rejects instead of joining when the first word names a file", async () => {
|
||||
const run = vi.fn();
|
||||
const file = join(tempDir(), "script.txt");
|
||||
writeFileSync(file, "hello");
|
||||
await expect(
|
||||
runCommand(await wrap(leaf("tts", run)), { rawArgs: [file, "extra"] }),
|
||||
).rejects.toThrow(CliUsageError);
|
||||
expect(errorSpy.mock.calls.flat().join("\n")).toContain("hyperframes tts: extra");
|
||||
expect(run).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("still joins catalog words when the first one names a folder", async () => {
|
||||
const run = vi.fn();
|
||||
const folder = tempDir();
|
||||
await runCommand(await wrap(leaf("catalog", run)), { rawArgs: [folder, "player"] });
|
||||
expect(run.mock.calls[0]![0].args.dir).toBe(`${folder} player`);
|
||||
});
|
||||
|
||||
it.each(["catalog", "tts"])(
|
||||
"joins %s's trailing words into its last positional",
|
||||
async (name) => {
|
||||
const run = vi.fn();
|
||||
await runCommand(await wrap(leaf(name, run)), { rawArgs: ["lower", "third", "--json"] });
|
||||
expect(run.mock.calls[0]![0].args.dir).toBe("lower third");
|
||||
},
|
||||
);
|
||||
|
||||
it("reads JSON mode from the parsed flag: --json=1 without a declared flag, not after --", async () => {
|
||||
const bare = { meta: { name: "docs" }, args: { topic: { type: "positional" } }, run: vi.fn() };
|
||||
await expect(
|
||||
runCommand(await wrap(bare as CommandDef), { rawArgs: ["a", "b", "--json=1"] }),
|
||||
).rejects.toThrow(CliUsageError);
|
||||
expect(JSON.parse(logSpy.mock.calls[0]![0] as string)).toMatchObject({ ok: false });
|
||||
logSpy.mockClear();
|
||||
await expect(
|
||||
runCommand(await wrap(bare as CommandDef), { rawArgs: ["a", "b", "--", "--json"] }),
|
||||
).rejects.toThrow(CliUsageError);
|
||||
expect(logSpy).not.toHaveBeenCalled();
|
||||
expect(errorSpy.mock.calls.flat().join("\n")).toContain("hyperframes docs: b, --json");
|
||||
});
|
||||
|
||||
it("reports JSON under --json=true for a command that declares no json flag", async () => {
|
||||
const bare = {
|
||||
meta: { name: "snapshot" },
|
||||
args: { dir: { type: "positional" } },
|
||||
run: vi.fn(),
|
||||
};
|
||||
await expect(
|
||||
runCommand(await wrap(bare as CommandDef), { rawArgs: ["a", "b", "--json=true"] }),
|
||||
).rejects.toThrow(CliUsageError);
|
||||
expect(JSON.parse(logSpy.mock.calls[0]![0] as string)).toMatchObject({ ok: false });
|
||||
});
|
||||
|
||||
it("reports text on stderr under --json=false for a command that declares no json flag", async () => {
|
||||
const bare = {
|
||||
meta: { name: "snapshot" },
|
||||
args: { dir: { type: "positional" } },
|
||||
run: vi.fn(),
|
||||
};
|
||||
await expect(
|
||||
runCommand(await wrap(bare as CommandDef), { rawArgs: ["a", "b", "--json=false"] }),
|
||||
).rejects.toThrow(CliUsageError);
|
||||
expect(logSpy).not.toHaveBeenCalled();
|
||||
expect(errorSpy.mock.calls.flat().join("\n")).toContain("hyperframes snapshot: b");
|
||||
});
|
||||
|
||||
it("runs a leaf given no more positionals than it declares", async () => {
|
||||
const run = vi.fn();
|
||||
await runCommand(await wrap(leaf("lint", run)), { rawArgs: ["a", "--json"] });
|
||||
expect(run).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("names a nested subcommand by its full path", async () => {
|
||||
const group = {
|
||||
meta: { name: "cloud" },
|
||||
subCommands: { get: leaf("get") },
|
||||
run: vi.fn(),
|
||||
} as CommandDef;
|
||||
await expect(runCommand(await wrap(group), { rawArgs: ["get", "id1", "id2"] })).rejects.toThrow(
|
||||
CliUsageError,
|
||||
);
|
||||
expect(errorSpy.mock.calls.flat().join("\n")).toContain(
|
||||
"hyperframes cloud get: id2\nUsage: hyperframes cloud get [DIR]",
|
||||
);
|
||||
});
|
||||
|
||||
it("lets an opted-out command read extra positionals", async () => {
|
||||
const run = vi.fn();
|
||||
await runCommand(await wrap(leaf("compare", run)), { rawArgs: ["a", "b", "c"] });
|
||||
expect(run).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("does not count the subcommand name against the group citty also runs", async () => {
|
||||
const run = vi.fn();
|
||||
const sub = vi.fn();
|
||||
const group = { meta: { name: "auth" }, subCommands: { status: leaf("status", sub) }, run };
|
||||
await runCommand(await wrap(group as CommandDef), { rawArgs: ["status", "--json"] });
|
||||
expect(sub).toHaveBeenCalledOnce();
|
||||
expect(run).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
|
||||
describe("reportCommandFailure", () => {
|
||||
beforeEach(() => {
|
||||
trackCommandFailure.mockReset();
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { CommandDef } from "citty";
|
||||
import { resolveExtraPositionals } from "./reject-extra-positionals.js";
|
||||
import { assertKnownFlags } from "./reject-unknown-flags.js";
|
||||
|
||||
// citty types subcommands as `CommandDef<any>` (SubCommandsDef); mirror that so
|
||||
@@ -13,11 +14,16 @@ type AnyCommandDef = CommandDef<any>;
|
||||
export function trackCommandFailures(
|
||||
load: () => Promise<AnyCommandDef>,
|
||||
): () => Promise<AnyCommandDef> {
|
||||
return () => load().then((cmd) => wrapCommand(cmd));
|
||||
return () => load().then((cmd) => wrapCommand(cmd, commandName(cmd)));
|
||||
}
|
||||
|
||||
function commandName(cmd: AnyCommandDef): string {
|
||||
const name = (cmd.meta as { name?: unknown } | undefined)?.name;
|
||||
return typeof name === "string" ? name : "";
|
||||
}
|
||||
|
||||
/**
|
||||
* Wrap a resolved command's `run` (assert-flags) AND
|
||||
* Wrap a resolved command's `run` (assert flags and positional count) AND
|
||||
* recursively wrap every entry in its `subCommands`. Two HF#2033 fixes live
|
||||
* here:
|
||||
* 1. `assertKnownFlags` runs in the wrapped command, so an unknown-flag
|
||||
@@ -26,7 +32,7 @@ export function trackCommandFailures(
|
||||
* `lambda/*`, `capture/*`, `skills`). Without it, a nested command's
|
||||
* unknown flags would bypass the leaf's guard.
|
||||
*/
|
||||
function wrapCommand(cmd: AnyCommandDef): AnyCommandDef {
|
||||
function wrapCommand(cmd: AnyCommandDef, path: string): AnyCommandDef {
|
||||
const run = cmd.run;
|
||||
// Nothing to wrap (no run, no nested subcommands) — preserve identity.
|
||||
if (typeof run !== "function" && !cmd.subCommands) return cmd;
|
||||
@@ -54,6 +60,8 @@ function wrapCommand(cmd: AnyCommandDef): AnyCommandDef {
|
||||
firstPositional != null &&
|
||||
Object.prototype.hasOwnProperty.call(cmd.subCommands, firstPositional);
|
||||
if (!delegatesToSub) assertKnownFlags(cmd, rawArgs);
|
||||
// Groups read `args._[0]` to pick fallback help, so only leaves get the count check.
|
||||
if (!cmd.subCommands) resolveExtraPositionals(cmd, path, ctx?.args);
|
||||
return await run(ctx);
|
||||
};
|
||||
}
|
||||
@@ -64,7 +72,7 @@ function wrapCommand(cmd: AnyCommandDef): AnyCommandDef {
|
||||
// (possibly async) loader. Normalize to a loader that resolves then wraps.
|
||||
wrappedSubs[name] = () =>
|
||||
Promise.resolve(typeof sub === "function" ? (sub as () => unknown)() : sub).then((c) =>
|
||||
wrapCommand(c as AnyCommandDef),
|
||||
wrapCommand(c as AnyCommandDef, `${path} ${name}`),
|
||||
);
|
||||
}
|
||||
wrapped.subCommands = wrappedSubs;
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
import type { ArgsDef, CommandDef } from "citty";
|
||||
import { CliUsageError } from "./commandResult.js";
|
||||
import { isTextFile } from "./textFile.js";
|
||||
|
||||
// citty binds each declared positional in order and leaves the rest in `args._`, where they were
|
||||
// silently dropped (`render ./proj out.mp4` rendered to the default path). Reject them up front.
|
||||
|
||||
// Leaf commands that read extra positionals from `args._` on purpose, by command path.
|
||||
export const ACCEPTS_EXTRA_POSITIONALS = new Set([
|
||||
"compare",
|
||||
"figma asset",
|
||||
"skills update",
|
||||
"timeline set",
|
||||
]);
|
||||
|
||||
// Free-text commands: their last positional takes every remaining word (`tts hello world`).
|
||||
export const JOINS_EXTRA_POSITIONALS = new Set(["catalog", "tts"]);
|
||||
|
||||
const EXTRA_POSITIONAL_HINTS: Record<string, string> = {
|
||||
add: "Run add once per item, or pass a single tag to install every item tagged with it.",
|
||||
};
|
||||
|
||||
// citty parses `--json` on every command (`--json=1` as "1"); after `--` it is a positional.
|
||||
function wantsJson(value: unknown): boolean {
|
||||
return value === true || (typeof value === "string" && value !== "false" && value !== "0");
|
||||
}
|
||||
|
||||
function declaredArgs(cmd: CommandDef<ArgsDef>): ArgsDef {
|
||||
const raw = cmd.args;
|
||||
return raw && typeof raw === "object" ? (raw as ArgsDef) : {};
|
||||
}
|
||||
|
||||
function usageLine(path: string, args: ArgsDef): string {
|
||||
const parts = ["hyperframes", path];
|
||||
let hasOptions = false;
|
||||
for (const [name, def] of Object.entries(args)) {
|
||||
if (def.type !== "positional") {
|
||||
hasOptions = true;
|
||||
continue;
|
||||
}
|
||||
const optional = def.required === false || def.default !== undefined;
|
||||
parts.push(optional ? `[${name.toUpperCase()}]` : `<${name.toUpperCase()}>`);
|
||||
}
|
||||
if (hasOptions) parts.push("[OPTIONS]");
|
||||
return parts.join(" ");
|
||||
}
|
||||
|
||||
/** Join a free-text command's trailing words, else throw a usage error naming the extras. */
|
||||
export function resolveExtraPositionals(
|
||||
cmd: CommandDef<ArgsDef>,
|
||||
path: string,
|
||||
parsed: Record<string, unknown> | undefined,
|
||||
): void {
|
||||
if (ACCEPTS_EXTRA_POSITIONALS.has(path)) return;
|
||||
const args = declaredArgs(cmd);
|
||||
const positionals = Object.entries(args).filter(([, def]) => def.type === "positional");
|
||||
const given = (Array.isArray(parsed?._) ? parsed._ : []).map(String);
|
||||
const extra = given.slice(positionals.length);
|
||||
if (extra.length === 0) return;
|
||||
const last = positionals.at(-1)?.[0];
|
||||
const words = given.slice(positionals.length - 1);
|
||||
// `tts script.txt extra` reads the file, so its first word is a path, not free text.
|
||||
const ttsFile = path === "tts" && isTextFile(words[0]!);
|
||||
if (parsed && last && JOINS_EXTRA_POSITIONALS.has(path) && !ttsFile) {
|
||||
parsed[last] = words.join(" ");
|
||||
return;
|
||||
}
|
||||
|
||||
const plural = extra.length === 1 ? "" : "s";
|
||||
const message =
|
||||
`Unexpected extra argument${plural} for hyperframes ${path}: ${extra.join(", ")}\n` +
|
||||
`Usage: ${usageLine(path, args)}` +
|
||||
(EXTRA_POSITIONAL_HINTS[path] ? `\n${EXTRA_POSITIONAL_HINTS[path]}` : "");
|
||||
if (wantsJson(parsed?.json)) console.log(JSON.stringify({ ok: false, error: message }));
|
||||
else console.error(message);
|
||||
// Telemetry reads the thrown message: the command path and a count, never the arguments.
|
||||
throw new CliUsageError(
|
||||
`${extra.length} unexpected extra argument${plural} for hyperframes ${path}`,
|
||||
{
|
||||
presented: true,
|
||||
},
|
||||
);
|
||||
}
|
||||
@@ -0,0 +1,8 @@
|
||||
import { existsSync } from "node:fs";
|
||||
import { extname, resolve } from "node:path";
|
||||
|
||||
/** True when `input` names an existing `.txt` file, which `tts` reads instead of speaking the path. */
|
||||
export function isTextFile(input: string): boolean {
|
||||
const path = resolve(input);
|
||||
return existsSync(path) && extname(path).toLowerCase() === ".txt";
|
||||
}
|
||||
Reference in New Issue
Block a user