fix(cli): keep extra arguments and the repo root value out of telemetry

Author: miguel-heygen

packages/cli/src/commands/lambda/repoRoot.test.ts

@@ -28,6 +28,7 @@ describe("repoRoot", () => {
   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=${root} is not a hyperframes checkout`);
+    expect(() => repoRoot()).toThrow("HYPERFRAMES_REPO_ROOT is not a hyperframes checkout");
+    expect(() => repoRoot()).not.toThrow(root);
   });
 });

packages/cli/src/commands/lambda/repoRoot.ts

@@ -17,7 +17,7 @@ export function repoRoot(): string {
   if (override) {
     if (existsSync(resolve(override, "packages", "aws-lambda", "package.json"))) return override;
     throw new Error(
-      `[hyperframes lambda] HYPERFRAMES_REPO_ROOT=${override} is not a hyperframes checkout ` +
+      "[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.",
     );
   }

packages/cli/src/telemetry/events.test.ts

@@ -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();
   });

packages/cli/src/utils/command-failure-tracking.test.ts

@@ -1,9 +1,10 @@
-import { mkdtempSync, writeFileSync } from "node:fs";
+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", () => ({
@@ -140,6 +141,12 @@ describe("trackCommandFailures: extra positionals", () => {
   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(() => {});
@@ -149,6 +156,45 @@ describe("trackCommandFailures: extra positionals", () => {
   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!.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 () => {
@@ -178,17 +224,18 @@ describe("trackCommandFailures: extra positionals", () => {
 
   it("rejects instead of joining when the first word names a file", async () => {
     const run = vi.fn();
-    const file = join(mkdtempSync(join(tmpdir(), "hf-join-")), "script.txt");
+    const file = join(tempDir(), "script.txt");
     writeFileSync(file, "hello");
     await expect(
       runCommand(await wrap(leaf("tts", run)), { rawArgs: [file, "extra"] }),
-    ).rejects.toThrow(/extra argument for hyperframes tts: 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 = mkdtempSync(join(tmpdir(), "hf-join-"));
+    const folder = tempDir();
     await runCommand(await wrap(leaf("catalog", run)), { rawArgs: [folder, "player"] });
     expect(run.mock.calls[0]![0].args.dir).toBe(`${folder} player`);
   });
@@ -227,7 +274,10 @@ describe("trackCommandFailures: extra positionals", () => {
       run: vi.fn(),
     } as CommandDef;
     await expect(runCommand(await wrap(group), { rawArgs: ["get", "id1", "id2"] })).rejects.toThrow(
-      /hyperframes cloud get: id2\nUsage: hyperframes cloud get \[DIR\]/,
+      CliUsageError,
+    );
+    expect(errorSpy.mock.calls.flat().join("\n")).toContain(
+      "hyperframes cloud get: id2\nUsage: hyperframes cloud get [DIR]",
     );
   });
 

packages/cli/src/utils/reject-extra-positionals.ts

@@ -65,5 +65,11 @@ export function resolveExtraPositionals(
   if (parsed?.json === true || rawArgs.some((tok) => tok === "--json" || tok === "--json=true"))
     console.log(JSON.stringify({ ok: false, error: message }));
   else console.error(message);
-  throw new CliUsageError(message, { presented: true });
+  // 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,
+    },
+  );
 }