fix(cli): report command failures once

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

@@ -47,4 +47,10 @@ describe("CLI command registration", () => {
     expect(condition).toContain('command !== "events"');
     expect(condition).toContain('command !== "skills"');
   });
+
+  it("reports each command failure only at the executable boundary", () => {
+    expect(cliSource).toContain("trackCommandFailures(load)");
+    expect(cliSource).not.toContain("trackCommandFailures(load,");
+    expect(cliSource.match(/reportCommandFailure\(command, error\)/g)).toHaveLength(1);
+  });
 });

packages/cli/src/cli.ts

@@ -163,10 +163,7 @@ const commandLoaders = {
 };
 
 const subCommands = Object.fromEntries(
-  Object.entries(commandLoaders).map(([name, load]) => [
-    name,
-    trackCommandFailures(load, (error) => reportCommandFailure(command, error)),
-  ]),
+  Object.entries(commandLoaders).map(([name, load]) => [name, trackCommandFailures(load)]),
 );
 
 const main = defineCommand({

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

@@ -14,40 +14,32 @@ function defineRun(run: CommandDef["run"]): CommandDef {
 }
 
 describe("trackCommandFailures", () => {
-  it("reports the error and re-throws when run() rejects", async () => {
-    const onFailure = vi.fn();
+  it("re-throws when run() rejects so the executable boundary can report it", async () => {
     const boom = new Error("ffmpeg not found");
-    const wrapped = trackCommandFailures(
-      () => Promise.resolve(defineRun(() => Promise.reject(boom))),
-      onFailure,
+    const wrapped = trackCommandFailures(() =>
+      Promise.resolve(defineRun(() => Promise.reject(boom))),
     );
 
     const cmd = await wrapped();
     await expect((cmd.run as () => Promise<unknown>)()).rejects.toBe(boom);
-    expect(onFailure).toHaveBeenCalledWith(boom);
   });
 
-  it("does not report when run() succeeds, and returns its value", async () => {
-    const onFailure = vi.fn();
-    const wrapped = trackCommandFailures(
-      () => Promise.resolve(defineRun(() => Promise.resolve("ok" as unknown as void))),
-      onFailure,
+  it("returns the command value when run() succeeds", async () => {
+    const wrapped = trackCommandFailures(() =>
+      Promise.resolve(defineRun(() => Promise.resolve("ok" as unknown as void))),
     );
 
     const cmd = await wrapped();
     await expect((cmd.run as () => Promise<unknown>)()).resolves.toBe("ok");
-    expect(onFailure).not.toHaveBeenCalled();
   });
 
   it("rejects an unknown flag on a leaf command", async () => {
-    const wrapped = trackCommandFailures(
-      () =>
-        Promise.resolve({
-          meta: { name: "leaf" },
-          args: { out: { type: "string" } },
-          run: () => Promise.resolve(),
-        } as CommandDef),
-      vi.fn(),
+    const wrapped = trackCommandFailures(() =>
+      Promise.resolve({
+        meta: { name: "leaf" },
+        args: { out: { type: "string" } },
+        run: () => Promise.resolve(),
+      } as CommandDef),
     );
     const cmd = await wrapped();
     await expect(
@@ -59,14 +51,12 @@ describe("trackCommandFailures", () => {
     // `figma component <ref> --name x`: --name belongs to the subcommand's
     // table; the group (subCommands + fallback-help run) must not reject it.
     const run = vi.fn(() => Promise.resolve());
-    const wrapped = trackCommandFailures(
-      () =>
-        Promise.resolve({
-          meta: { name: "figma" },
-          subCommands: { component: () => Promise.resolve({ meta: { name: "component" } }) },
-          run,
-        } as unknown as CommandDef),
-      vi.fn(),
+    const wrapped = trackCommandFailures(() =>
+      Promise.resolve({
+        meta: { name: "figma" },
+        subCommands: { component: () => Promise.resolve({ meta: { name: "component" } }) },
+        run,
+      } as unknown as CommandDef),
     );
     const cmd = await wrapped();
     await expect(
@@ -78,14 +68,12 @@ describe("trackCommandFailures", () => {
   });
 
   it("still rejects an unknown flag when the group is NOT delegating", async () => {
-    const wrapped = trackCommandFailures(
-      () =>
-        Promise.resolve({
-          meta: { name: "figma" },
-          subCommands: { component: () => Promise.resolve({ meta: { name: "component" } }) },
-          run: () => Promise.resolve(),
-        } as unknown as CommandDef),
-      vi.fn(),
+    const wrapped = trackCommandFailures(() =>
+      Promise.resolve({
+        meta: { name: "figma" },
+        subCommands: { component: () => Promise.resolve({ meta: { name: "component" } }) },
+        run: () => Promise.resolve(),
+      } as unknown as CommandDef),
     );
     const cmd = await wrapped();
     await expect(
@@ -94,49 +82,29 @@ describe("trackCommandFailures", () => {
   });
 
   it("passes through a command with no run() untouched", async () => {
-    const onFailure = vi.fn();
     const parent: CommandDef = { meta: { name: "parent" } };
-    const wrapped = trackCommandFailures(() => Promise.resolve(parent), onFailure);
+    const wrapped = trackCommandFailures(() => Promise.resolve(parent));
 
     const cmd = await wrapped();
     expect(cmd).toBe(parent);
-    expect(onFailure).not.toHaveBeenCalled();
   });
 
-  it("awaits onFailure and re-throws the ORIGINAL error even if onFailure rejects", async () => {
-    const boom = new Error("original failure");
-    const wrapped = trackCommandFailures(
-      () => Promise.resolve(defineRun(() => Promise.reject(boom))),
-      () => Promise.reject(new Error("telemetry is down")),
-    );
-
-    const cmd = await wrapped();
-    await expect((cmd.run as () => Promise<unknown>)()).rejects.toBe(boom);
-  });
-
-  it("REPORTS an unknown-flag rejection to onFailure (HF#2033: assertion inside the try)", async () => {
-    // The flag assertion used to run before the try/catch, so an unknown-flag
-    // throw skipped telemetry. It must now be reported like any other failure.
-    const onFailure = vi.fn();
+  it("re-throws an unknown-flag rejection to the executable boundary", async () => {
     const cmd = {
       meta: { name: "render" },
       args: { output: { type: "string", alias: "o" } },
       run: vi.fn(() => Promise.resolve()),
     } as unknown as CommandDef;
-    const wrapped = trackCommandFailures(() => Promise.resolve(cmd), onFailure);
+    const wrapped = trackCommandFailures(() => Promise.resolve(cmd));
 
     const resolved = await wrapped();
     await expect(
       (resolved.run as (ctx: unknown) => Promise<unknown>)({ rawArgs: ["--nope", "x"] }),
     ).rejects.toThrow(/unknown flag/i);
-    expect(onFailure).toHaveBeenCalledTimes(1);
     expect(cmd.run).not.toHaveBeenCalled(); // body never ran — flag rejected first
   });
 
-  it("recursively wraps nested subcommands so their failures report too (HF#2033)", async () => {
-    // cli.ts wraps only top-level loaders; a group's leaves (cloud/*, auth/*, …)
-    // were never wrapped, silently dropping unknown-flag + failure telemetry.
-    const onFailure = vi.fn();
+  it("recursively wraps nested subcommands so their failures reach the boundary", async () => {
     const boom = new Error("nested boom");
     const group: CommandDef = {
       meta: { name: "cloud" },
@@ -144,7 +112,7 @@ describe("trackCommandFailures", () => {
         render: defineRun(() => Promise.reject(boom)),
       },
     };
-    const wrapped = trackCommandFailures(() => Promise.resolve(group), onFailure);
+    const wrapped = trackCommandFailures(() => Promise.resolve(group));
 
     const resolvedGroup = await wrapped();
     const subLoader = (resolvedGroup.subCommands as Record<string, () => Promise<CommandDef>>)
@@ -152,7 +120,6 @@ describe("trackCommandFailures", () => {
     if (!subLoader) throw new Error("expected a wrapped 'render' subcommand loader");
     const leaf = await subLoader();
     await expect((leaf.run as () => Promise<unknown>)()).rejects.toBe(boom);
-    expect(onFailure).toHaveBeenCalledWith(boom);
   });
 });
 

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

@@ -6,43 +6,27 @@ import { assertKnownFlags } from "./reject-unknown-flags.js";
 type AnyCommandDef = CommandDef<any>;
 
 /**
- * Wrap a lazy command loader so a thrown failure is reported via `onFailure`
- * before it propagates. citty's `runMain` catches command errors and exits 1
- * without re-throwing, so this is the only place to capture the reason. The
- * error is re-thrown unchanged, preserving citty's print + exit-1 behavior.
- *
- * `onFailure` is awaited so it can resolve the (lazily-loaded) telemetry module
- * before the error propagates — otherwise a command that throws before the
- * telemetry import settles would lose its event. A throw from `onFailure` is
- * swallowed so telemetry can never mask the real command failure.
- *
- * Commands that call `process.exit()` themselves bypass this (the process is
- * already gone) and must report their failure inline.
+ * Wrap a lazy command loader so leaf commands and nested subcommands share the
+ * unknown-flag guard. Errors propagate unchanged to the executable boundary,
+ * which is the sole command-failure telemetry reporter.
  */
 export function trackCommandFailures(
   load: () => Promise<AnyCommandDef>,
-  onFailure: (err: unknown) => void | Promise<void>,
 ): () => Promise<AnyCommandDef> {
-  return () => load().then((cmd) => wrapCommand(cmd, onFailure));
+  return () => load().then((cmd) => wrapCommand(cmd));
 }
 
 /**
- * Wrap a resolved command's `run` (assert-flags + report-failure) AND
+ * Wrap a resolved command's `run` (assert-flags) AND
  * recursively wrap every entry in its `subCommands`. Two HF#2033 fixes live
  * here:
- *   1. `assertKnownFlags` runs INSIDE the try, so an unknown-flag throw is
- *      routed through `onFailure` (telemetry) like any other failure — it
- *      used to throw before the try and lose the event entirely.
+ *   1. `assertKnownFlags` runs in the wrapped command, so an unknown-flag
+ *      throw reaches the executable boundary like every other failure.
  *   2. Recursion covers nested command groups (`cloud/*`, `auth/*`, `figma/*`,
- *      `lambda/*`, `capture/*`, `skills`). cli.ts only wraps the top-level
- *      loaders, so before this a `hyperframes cloud render --badflag` silently
- *      ignored the flag and reported nothing — citty dispatches to the leaf,
- *      whose `run` was never wrapped.
+ *      `lambda/*`, `capture/*`, `skills`). Without it, a nested command's
+ *      unknown flags would bypass the leaf's guard.
  */
-function wrapCommand(
-  cmd: AnyCommandDef,
-  onFailure: (err: unknown) => void | Promise<void>,
-): AnyCommandDef {
+function wrapCommand(cmd: AnyCommandDef): AnyCommandDef {
   const run = cmd.run;
   // Nothing to wrap (no run, no nested subcommands) — preserve identity.
   if (typeof run !== "function" && !cmd.subCommands) return cmd;
@@ -50,36 +34,27 @@ function wrapCommand(
   const wrapped: AnyCommandDef = { ...cmd };
   if (typeof run === "function") {
     wrapped.run = async (ctx: Parameters<typeof run>[0]) => {
-      try {
-        // Reject unknown flags before the command body: citty silently ignores
-        // them otherwise, dropping the value (e.g. `render --out x` fell back to
-        // the default output path). Inside the try so the rejection is reported.
-        //
-        // A command group (subCommands + fallback-help run) delegating to a
-        // subcommand must NOT assert here: the flags belong to the subcommand's
-        // table, not the group's, and would be falsely rejected (e.g.
-        // `figma component <ref> --name x`). The wrapped subcommand loaders
-        // below assert the leaf's own table, so typo protection is preserved.
-        // Heuristic caveat: "first non-dash token names a subcommand" is sound
-        // only while command groups declare no flags of their own — if a group
-        // grows a flag whose value could match a subcommand name, replace this
-        // with a real argv parse.
-        const rawArgs = ctx?.rawArgs ?? [];
-        const firstPositional = rawArgs.find((tok) => tok && !tok.startsWith("-"));
-        const delegatesToSub =
-          cmd.subCommands != null &&
-          firstPositional != null &&
-          Object.prototype.hasOwnProperty.call(cmd.subCommands, firstPositional);
-        if (!delegatesToSub) assertKnownFlags(cmd, rawArgs);
-        return await run(ctx);
-      } catch (err) {
-        try {
-          await onFailure(err);
-        } catch {
-          // Telemetry must never mask the real command failure.
-        }
-        throw err;
-      }
+      // Reject unknown flags before the command body: citty silently ignores
+      // them otherwise, dropping the value (e.g. `render --out x` fell back to
+      // the default output path).
+      //
+      // A command group (subCommands + fallback-help run) delegating to a
+      // subcommand must NOT assert here: the flags belong to the subcommand's
+      // table, not the group's, and would be falsely rejected (e.g.
+      // `figma component <ref> --name x`). The wrapped subcommand loaders
+      // below assert the leaf's own table, so typo protection is preserved.
+      // Heuristic caveat: "first non-dash token names a subcommand" is sound
+      // only while command groups declare no flags of their own — if a group
+      // grows a flag whose value could match a subcommand name, replace this
+      // with a real argv parse.
+      const rawArgs = ctx?.rawArgs ?? [];
+      const firstPositional = rawArgs.find((tok) => tok && !tok.startsWith("-"));
+      const delegatesToSub =
+        cmd.subCommands != null &&
+        firstPositional != null &&
+        Object.prototype.hasOwnProperty.call(cmd.subCommands, firstPositional);
+      if (!delegatesToSub) assertKnownFlags(cmd, rawArgs);
+      return await run(ctx);
     };
   }
   if (cmd.subCommands) {
@@ -89,7 +64,7 @@ function wrapCommand(
       // (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, onFailure),
+          wrapCommand(c as AnyCommandDef),
         );
     }
     wrapped.subCommands = wrappedSubs;