fix(cli): reject extra positional arguments passed to add (#3891)

* fix(cli): reject extra positional arguments passed to add

`add <a> <b> <c>` silently installed only `<a>` and exited 0, discarding
`<b>` and `<c>` with no warning. citty binds only the first positional
token to `name`; every token after it still lands in `args._`, but
nothing read it. `add` now rejects the extra arguments up front, before
touching the registry, naming exactly what was dropped.

Co-Authored-By: Miguel Angel <miguel.sierra@heygen.com>

* refactor(cli): inline the add extra-argument message and test it through citty parsing

---------

Co-authored-by: Miguel Angel <miguel.sierra@heygen.com>
This commit is contained in:
miga-heygen
2026-10-02 06:34:21 -07:00
committed by GitHub
co-authored by Miguel Angel
parent e8659cb653
commit ded5fa8661
2 changed files with 59 additions and 2 deletions
+46 -1
View File
@@ -1,11 +1,12 @@
import { describe, expect, it, vi, beforeEach, afterEach } from "vitest";
import { parseArgs as parseCittyArgs, type ArgsDef } from "citty";
import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import type { RegistryItem, RegistryManifest } from "@hyperframes/core";
import { lintHyperframeHtml } from "@hyperframes/lint";
import type { RunAddResult } from "./add.js";
import {
import addCommand, {
AddError,
buildSnippet,
compositionRootId,
@@ -16,6 +17,7 @@ import {
tagAddJson,
} from "./add.js";
import { trackRegistryItemAdded } from "../telemetry/events.js";
import { CliUsageError } from "../utils/commandResult.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
@@ -522,6 +524,49 @@ describe("variable values in the snippet", () => {
});
});
describe("add command run() — extra positional arguments", () => {
let dir: string;
let errorSpy: ReturnType<typeof vi.spyOn>;
beforeEach(() => {
dir = tmp();
errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
});
afterEach(() => {
errorSpy.mockRestore();
vi.unstubAllGlobals();
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 });
}
it("rejects `add a --dir <dir> b c` under real citty parsing, 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,
);
expect(fetchSpy).not.toHaveBeenCalled();
expect(errorSpy.mock.calls.flat().join(" ")).toContain("extra arguments: b, c");
});
it("does not fire on a normal single-item invocation", async () => {
mockFetch();
writeRegistryConfig(dir);
await expect(
runCommand({ name: "my-block", _: ["my-block"], dir, clipboard: false, json: true }),
).resolves.toBeUndefined();
});
});
describe("describeInstallFailure", () => {
it("explains a bare transport failure instead of echoing it", () => {
// What the user actually sees after copying a command off the catalog page.
+13 -1
View File
@@ -1,4 +1,4 @@
import { failCommand } from "../utils/commandResult.js";
import { failCommand, failUsage } from "../utils/commandResult.js";
import { defineCommand } from "citty";
import type { Example } from "./_examples.js";
@@ -490,6 +490,18 @@ export default defineCommand({
const projectDir = resolve(args.dir ?? process.cwd());
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.