refactor(cli): centralize process lifecycle

Author: jrusso1020

package.json

@@ -26,12 +26,13 @@
     "sync-schemas:check": "tsx scripts/sync-schemas.ts --check",
     "sync:package-subpaths": "node scripts/package-subpaths.mjs --write",
     "check:package-subpaths": "node scripts/package-subpaths.mjs",
-    "lint": "bun run check:tracked-artifacts && bun run check:workspace-contracts && bun run check:package-cycles && bun run check:package-subpaths && oxlint . && tsx scripts/lint-skills.ts",
+    "lint": "bun run check:tracked-artifacts && bun run check:workspace-contracts && bun run check:package-cycles && bun run check:package-subpaths && bun run check:cli-process-ownership && oxlint . && tsx scripts/lint-skills.ts",
     "lint:skills": "tsx scripts/lint-skills.ts",
     "lint:fix": "oxlint --fix .",
     "check:tracked-artifacts": "node scripts/check-tracked-artifacts.mjs",
     "check:workspace-contracts": "node scripts/check-workspace-contracts.mjs",
     "check:package-cycles": "node scripts/check-package-cycles.mjs",
+    "check:cli-process-ownership": "node scripts/check-cli-process-ownership.mjs",
     "format": "oxfmt .",
     "test": "bun run test:unit",
     "test:unit": "bun run --filter '*' test",
@@ -44,7 +45,7 @@
     "player:perf": "bun run --filter @hyperframes/player perf",
     "format:check": "oxfmt --check .",
     "knip": "knip",
-    "test:scripts": "node --import tsx --test scripts/check-tracked-artifacts.test.mjs scripts/check-workspace-contracts.test.mjs scripts/check-package-cycles.test.mjs scripts/package-subpaths.test.mjs scripts/validate-release-channel.test.mjs scripts/draft-changelog.test.ts scripts/set-version.test.ts scripts/release-prepare.test.ts scripts/cli-options.test.ts scripts/changelog-weekly.test.ts scripts/claude-plugin-compression.test.ts scripts/studio-runtime-smoke.test.mjs scripts/verify-packed-manifests.test.mjs",
+    "test:scripts": "node --import tsx --test scripts/check-tracked-artifacts.test.mjs scripts/check-workspace-contracts.test.mjs scripts/check-package-cycles.test.mjs scripts/check-cli-process-ownership.test.mjs scripts/package-subpaths.test.mjs scripts/validate-release-channel.test.mjs scripts/draft-changelog.test.ts scripts/set-version.test.ts scripts/release-prepare.test.ts scripts/cli-options.test.ts scripts/changelog-weekly.test.ts scripts/claude-plugin-compression.test.ts scripts/studio-runtime-smoke.test.mjs scripts/verify-packed-manifests.test.mjs",
     "test:skills": "node --test 'skills/**/*.test.mjs'",
     "generate:previews": "tsx scripts/generate-template-previews.ts",
     "generate:catalog-previews": "tsx scripts/generate-catalog-previews.ts",

packages/cli/src/auth/oauth.ts

@@ -1,3 +1,4 @@
+import { failCommand } from "../utils/commandResult.js";
 /**
  * OAuth 2.0 + PKCE driver for the HeyGen public OAuth flow.
  *
@@ -115,7 +116,7 @@ export function assertOAuthConfiguredOrExit(): void {
     if (isAuthError(err) && err.code === "OAUTH_NOT_CONFIGURED") {
       console.error(`Error: ${err.message}`);
       if (err.hint) console.error(err.hint);
-      process.exit(1);
+      failCommand();
     }
     throw err;
   }

packages/cli/src/cli.ts

@@ -99,10 +99,19 @@ try {
 // Telemetry, update checks, and heavy modules are imported only when needed.
 // For --help we skip telemetry entirely.
 
-import { defineCommand, runMain } from "citty";
+import { defineCommand, runCommand } from "citty";
 import type { ArgsDef, CommandDef } from "citty";
 import { getRunId } from "./telemetry/runId.js";
 import { reportCommandFailure, trackCommandFailures } from "./utils/command-failure-tracking.js";
+import { resolveCommandUsage } from "./utils/commandUsageResolution.js";
+import {
+  CliResultSignal,
+  CliRuntimeError,
+  CliUsageError,
+  consumeCommandResult,
+  registerRootExitRequester,
+  type CommandResult,
+} from "./utils/commandResult.js";
 
 const isHelp = process.argv.includes("--help") || process.argv.includes("-h");
 
@@ -151,14 +160,10 @@ const commandLoaders = {
   figma: () => import("./commands/figma.js").then((m) => m.default),
 };
 
-// Wrap each command's run() so a thrown failure reports its reason to telemetry
-// before citty catches the error and exits 1. The error is re-thrown unchanged,
-// preserving citty's print + exit-1 behavior. Commands that call process.exit()
-// themselves (e.g. `browser path`) bypass this and report inline.
 const subCommands = Object.fromEntries(
   Object.entries(commandLoaders).map(([name, load]) => [
     name,
-    trackCommandFailures(load, (err) => reportCommandFailure(command, err)),
+    trackCommandFailures(load, (error) => reportCommandFailure(command, error)),
   ]),
 );
 
@@ -207,12 +212,13 @@ let _trackCommandResult:
   | undefined;
 let _printUpdateNotice: (() => void) | undefined;
 let _printSkillsUpdateNotice: (() => void) | undefined;
+let telemetryReady: Promise<void> = Promise.resolve();
 
 // `events` is a telemetry-internal beacon: it self-tracks + self-flushes, so it
 // skips the per-command wrapper (no duplicate cli_command, no first-run notice
 // printed into a skill's captured output).
 if (!isHelp && command !== "telemetry" && command !== "events" && command !== "unknown") {
-  import("./telemetry/index.js").then((mod) => {
+  telemetryReady = import("./telemetry/index.js").then((mod) => {
     _flush = mod.flush;
     _flushSync = mod.flushSync;
     _trackCliError = mod.trackCliError;
@@ -262,33 +268,55 @@ if (
 
 const commandStart = Date.now();
 const runId = getRunId();
+let finalized = false;
 
-// Async flush for normal exit. `beforeExit` re-fires every time the
-// event loop drains, and the async `_flush()` itself schedules new
-// work — so a plain `on` listener would print the update notice (and
-// re-flush) once per drain (the user-reported double-print). `once`
-// detaches after first invocation, which is what we want for both.
-process.once("beforeExit", () => {
-  _flush?.().catch(() => {});
+// fallow-ignore-next-line complexity
+async function finalizeCli(result: CommandResult): Promise<void> {
+  if (finalized) return;
+  finalized = true;
+  commandFailed ||= result.exitCode !== 0;
+  await telemetryReady.catch(() => {});
+  _trackCommandResult?.({
+    command,
+    success: result.exitCode === 0 && !commandFailed,
+    exitCode: result.exitCode,
+    durationMs: Date.now() - commandStart,
+    runId,
+  });
+  await _flush?.().catch(() => {});
   if (!hasJsonFlag) {
     _printUpdateNotice?.();
     _printSkillsUpdateNotice?.();
   }
+  process.exitCode = result.exitCode;
+}
+
+registerRootExitRequester((exitCode) => {
+  void finalizeCli({
+    exitCode,
+    kind: exitCode === 0 ? "success" : "runtime_error",
+    presented: true,
+  }).finally(() => process.exit(exitCode));
 });
 
 // Sync-only: exit handlers cannot await promises or drain microtasks.
 // _trackCommandResult / _trackCliError are captured references resolved
 // at init time, so they're callable synchronously here.
-process.on("exit", (code) => {
-  _trackCommandResult?.({
-    command,
-    success: code === 0 && !commandFailed,
-    exitCode: code,
-    durationMs: Date.now() - commandStart,
-    runId,
-  });
-  _flushSync?.();
-});
+process.on(
+  "exit",
+  // fallow-ignore-next-line complexity
+  (code) => {
+    if (finalized) return;
+    _trackCommandResult?.({
+      command,
+      success: code === 0 && !commandFailed,
+      exitCode: code,
+      durationMs: Date.now() - commandStart,
+      runId,
+    });
+    _flushSync?.();
+  },
+);
 
 process.on("uncaughtException", (error) => {
   if ((error as NodeJS.ErrnoException).code === "EPIPE") {
@@ -312,6 +340,7 @@ process.on("uncaughtException", (error) => {
 // The exit handler above will still fire with the real exit code.
 process.on("unhandledRejection", (reason) => {
   commandFailed = true;
+  process.exitCode = 1;
   const error = reason instanceof Error ? reason : new Error(String(reason));
   _trackCliError?.({
     error_name: error.name,
@@ -331,4 +360,39 @@ async function showUsage<T extends ArgsDef>(
   return impl(cmd as CommandDef, parent as CommandDef | undefined);
 }
 
-runMain(main, { showUsage });
+async function showRequestedUsage(): Promise<void> {
+  const requested = await resolveCommandUsage(main as CommandDef, argv);
+  return showUsage(requested.command, requested.parent);
+}
+
+function commandResultForError(error: unknown): CommandResult {
+  if (error instanceof CliResultSignal) return error.result;
+  if (error instanceof CliUsageError || error instanceof CliRuntimeError) return error.result;
+  return { exitCode: 1, kind: "runtime_error" };
+}
+
+// fallow-ignore-next-line complexity
+async function executeCli(): Promise<void> {
+  let result: CommandResult = { exitCode: 0, kind: "success" };
+  try {
+    if (isHelp) await showRequestedUsage();
+    else await runCommand(main, { rawArgs: argv });
+  } catch (error) {
+    result = commandResultForError(error);
+    if (!(error instanceof CliResultSignal)) {
+      commandFailed = true;
+      reportCommandFailure(command, error);
+      const typed = error instanceof CliUsageError || error instanceof CliRuntimeError;
+      if (error instanceof CliUsageError && !error.result.presented) await showRequestedUsage();
+      if (!typed || !error.result.presented) {
+        console.error(error instanceof Error ? error.message : String(error));
+      }
+    }
+  } finally {
+    const pending = consumeCommandResult();
+    if (pending.exitCode !== 0 || result.exitCode === 0) result = pending;
+    await finalizeCli(result);
+  }
+}
+
+await executeCli();

packages/cli/src/cloud/errors.test.ts

@@ -1,4 +1,4 @@
-import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { beforeEach, describe, expect, it, vi } from "vitest";
 
 // errorBox writes to the console; mock it so we can assert which title /
 // message / third line the cascade picked without matching ANSI output.
@@ -10,30 +10,19 @@ vi.mock("../ui/format.js", async (importOriginal) => ({
 import { errorBox } from "../ui/format.js";
 import { HyperframesApiError } from "./_gen/client.js";
 import { reportApiError } from "./errors.js";
+import { CliRuntimeError } from "../utils/commandResult.js";
 
 describe("cloud/errors reportApiError", () => {
-  // process.exit has signature `(code?) => never` which doesn't unify with
-  // vi.spyOn's inference; cast through `unknown` like cloud/parsing.test.ts.
-  let exitSpy: { mockRestore: () => void } & { mock: { calls: unknown[][] } };
-
   beforeEach(() => {
-    exitSpy = vi.spyOn(process, "exit").mockImplementation((() => {
-      throw new Error("process.exit called");
-    }) as unknown as (code?: string | number | null) => never) as unknown as typeof exitSpy;
     vi.mocked(errorBox).mockClear();
   });
 
-  afterEach(() => {
-    exitSpy.mockRestore();
-  });
-
   it("short-circuits a 404 to a Not found box when notFound is provided", () => {
     const err = new HyperframesApiError({ status: 404, message: "missing" });
     expect(() =>
       reportApiError("Get failed", err, { notFound: "render hfr_x no longer exists" }),
-    ).toThrow("process.exit called");
+    ).toThrow(CliRuntimeError);
     expect(errorBox).toHaveBeenCalledWith("Not found", "render hfr_x no longer exists");
-    expect(exitSpy).toHaveBeenCalledWith(1);
   });
 
   it("uses the code-specific hint when the error code is known", () => {
@@ -42,7 +31,7 @@ describe("cloud/errors reportApiError", () => {
       message: "slow down",
       code: "rate_limit_exceeded",
     });
-    expect(() => reportApiError("Submit failed", err)).toThrow("process.exit called");
+    expect(() => reportApiError("Submit failed", err)).toThrow(CliRuntimeError);
     expect(errorBox).toHaveBeenCalledWith(
       "Submit failed (HTTP 429)",
       "slow down",
@@ -57,7 +46,7 @@ describe("cloud/errors reportApiError", () => {
       code: "invalid_parameter",
     });
     expect(() => reportApiError("Submit failed", err, { suggestion: "see --help" })).toThrow(
-      "process.exit called",
+      CliRuntimeError,
     );
     expect(errorBox).toHaveBeenCalledWith(
       "Submit failed (HTTP 400)",
@@ -73,7 +62,7 @@ describe("cloud/errors reportApiError", () => {
       code: "some_unmapped_code",
     });
     expect(() => reportApiError("List failed", err, { suggestion: "retry shortly" })).toThrow(
-      "process.exit called",
+      CliRuntimeError,
     );
     expect(errorBox).toHaveBeenCalledWith("List failed (HTTP 500)", "boom", "retry shortly");
   });
@@ -84,7 +73,7 @@ describe("cloud/errors reportApiError", () => {
       message: "boom",
       code: "some_unmapped_code",
     });
-    expect(() => reportApiError("List failed", err)).toThrow("process.exit called");
+    expect(() => reportApiError("List failed", err)).toThrow(CliRuntimeError);
     expect(errorBox).toHaveBeenCalledWith(
       "List failed (HTTP 500)",
       "boom",
@@ -94,15 +83,15 @@ describe("cloud/errors reportApiError", () => {
 
   it("omits the third line when there is no code, hint, or suggestion", () => {
     const err = new HyperframesApiError({ status: 503, message: "unavailable" });
-    expect(() => reportApiError("Get failed", err)).toThrow("process.exit called");
+    expect(() => reportApiError("Get failed", err)).toThrow(CliRuntimeError);
     expect(errorBox).toHaveBeenCalledWith("Get failed (HTTP 503)", "unavailable");
   });
 
   it("merges extraHints on top of the built-in table", () => {
     const err = new HyperframesApiError({ status: 418, message: "teapot", code: "custom_code" });
     expect(() =>
       reportApiError("Brew failed", err, { extraHints: { custom_code: "use coffee instead" } }),
-    ).toThrow("process.exit called");
+    ).toThrow(CliRuntimeError);
     expect(errorBox).toHaveBeenCalledWith("Brew failed (HTTP 418)", "teapot", "use coffee instead");
   });
 
@@ -116,7 +105,7 @@ describe("cloud/errors reportApiError", () => {
       reportApiError("Submit failed", err, {
         extraHints: { rate_limit_exceeded: "custom backoff guidance" },
       }),
-    ).toThrow("process.exit called");
+    ).toThrow(CliRuntimeError);
     expect(errorBox).toHaveBeenCalledWith(
       "Submit failed (HTTP 429)",
       "slow down",
@@ -127,12 +116,12 @@ describe("cloud/errors reportApiError", () => {
   it("reports a plain Error with the stage title and suggestion", () => {
     expect(() =>
       reportApiError("Upload failed", new Error("disk full"), { suggestion: "free up space" }),
-    ).toThrow("process.exit called");
+    ).toThrow(CliRuntimeError);
     expect(errorBox).toHaveBeenCalledWith("Upload failed", "disk full", "free up space");
   });
 
   it("stringifies a non-Error thrown value", () => {
-    expect(() => reportApiError("Weird failure", "just a string")).toThrow("process.exit called");
+    expect(() => reportApiError("Weird failure", "just a string")).toThrow(CliRuntimeError);
     expect(errorBox).toHaveBeenCalledWith("Weird failure", "just a string", undefined);
   });
 });

packages/cli/src/cloud/errors.ts

@@ -1,3 +1,4 @@
+import { failCommand } from "../utils/commandResult.js";
 /**
  * Shared API-error reporter for the cloud subverbs.
  *
@@ -61,7 +62,7 @@ export function reportApiError(
   if (err instanceof HyperframesApiError) {
     if (err.status === 404 && options.notFound) {
       errorBox("Not found", options.notFound);
-      process.exit(1);
+      failCommand();
     }
     const hint = err.code ? hints[err.code] : undefined;
     const title = `${stage} (HTTP ${err.status})`;
@@ -78,12 +79,12 @@ export function reportApiError(
     } else {
       errorBox(title, err.message);
     }
-    process.exit(1);
+    failCommand();
   }
   if (err instanceof Error) {
     errorBox(stage, err.message, options.suggestion);
-    process.exit(1);
+    failCommand();
   }
   errorBox(stage, String(err), options.suggestion);
-  process.exit(1);
+  failCommand();
 }