fix: address bot review feedback

- Replace process.exit() in OutputError handler with throw + Stricli
  intercept in app.ts (BugBot: kills host process in library mode)
- Wrap withTelemetry in try/catch in sentry() to convert all internal
  errors (AuthError, OutputError, etc.) to SentryError (Seer + BugBot)
- Set process.exitCode=1 in bin wrapper catch handler (BugBot)

Author: BYK

script/bundle.ts

@@ -207,7 +207,7 @@ const result = await build({
 // Write the CLI bin wrapper (tiny — shebang + version check + dispatch)
 const BIN_WRAPPER = `#!/usr/bin/env node
 if(parseInt(process.versions.node)<22){console.error("Error: sentry requires Node.js 22 or later (found "+process.version+").\\n\\nEither upgrade Node.js, or install the standalone binary instead:\\n  curl -fsSL https://cli.sentry.dev/install | bash\\n");process.exit(1)}
-require('./index.cjs')._cli().catch(()=>{});
+require('./index.cjs')._cli().catch(()=>{process.exitCode=1});
 `;
 await Bun.write("./dist/bin.cjs", BIN_WRAPPER);
 

src/app.ts

@@ -41,6 +41,7 @@ import {
   AuthError,
   CliError,
   getExitCode,
+  OutputError,
   stringifyUnknown,
 } from "./lib/errors.js";
 import { error as errorColor, warning } from "./lib/formatters/colors.js";
@@ -151,6 +152,12 @@ const customText: ApplicationText = {
     return text_en.exceptionWhileParsingArguments(exc, ansiColor);
   },
   exceptionWhileRunningCommand: (exc: unknown, ansiColor: boolean): string => {
+    // OutputError: data was already rendered to stdout — just re-throw
+    // so the exit code propagates without Stricli printing an error message.
+    if (exc instanceof OutputError) {
+      throw exc;
+    }
+
     // Re-throw AuthError for auto-login flow in bin.ts
     // Don't capture to Sentry - it's an expected state (user not logged in or token expired), not an error
     // Note: skipAutoAuth is checked in bin.ts, not here — all auth errors must escape Sentry capture

src/cli.ts

@@ -112,7 +112,7 @@ export async function runCli(cliArgs: string[]): Promise<void> {
   const { ExitCode, run } = await import("@stricli/core");
   const { app } = await import("./app.js");
   const { buildContext } = await import("./context.js");
-  const { AuthError, formatError, getExitCode } = await import(
+  const { AuthError, OutputError, formatError, getExitCode } = await import(
     "./lib/errors.js"
   );
   const { error, warning } = await import("./lib/formatters/colors.js");
@@ -372,6 +372,12 @@ export async function runCli(cliArgs: string[]): Promise<void> {
       await executor(helpArgs);
     }
   } catch (err) {
+    // OutputError: data was already rendered to stdout by the command wrapper.
+    // Just set exitCode silently — no stderr message needed.
+    if (err instanceof OutputError) {
+      process.exitCode = err.exitCode;
+      return;
+    }
     process.stderr.write(`${error("Error:")} ${formatError(err)}\n`);
     process.exitCode = getExitCode(err);
     return;

src/index.ts

@@ -98,6 +98,7 @@ export class SentryError extends Error {
  */
 function sentry(...input: string[]): Promise<unknown>;
 function sentry(...input: [...string[], SentryOptions]): Promise<unknown>;
+// biome-ignore lint/complexity/noExcessiveCognitiveComplexity: library entry orchestrates env isolation, output capture, telemetry, and error conversion in one function
 async function sentry(...input: (string | SentryOptions)[]): Promise<unknown> {
   // Detect trailing options object
   let args: string[];
@@ -157,11 +158,25 @@ async function sentry(...input: (string | SentryOptions)[]): Promise<unknown> {
     const { buildContext } = await import("./context.js");
     const { withTelemetry } = await import("./lib/telemetry.js");
 
-    await withTelemetry(
-      // biome-ignore lint/suspicious/noExplicitAny: fakeProcess duck-types the process interface
-      async (span) => run(app, args, buildContext(fakeProcess as any, span)),
-      { libraryMode: true }
-    );
+    try {
+      await withTelemetry(
+        // biome-ignore lint/suspicious/noExplicitAny: fakeProcess duck-types the process interface
+        async (span) => run(app, args, buildContext(fakeProcess as any, span)),
+        { libraryMode: true }
+      );
+    } catch (thrown) {
+      // Stricli catches command errors and writes them to stderr + sets exitCode.
+      // But some errors (AuthError, OutputError) are re-thrown through Stricli.
+      // Convert any that escape into SentryError for a consistent library API.
+      const stderrStr = stderrChunks.join("");
+      const exitCode = fakeProcess.exitCode !== 0 ? fakeProcess.exitCode : 1;
+      // biome-ignore lint/suspicious/noControlCharactersInRegex: ANSI escape sequences use ESC (0x1b)
+      const ANSI_RE = /\x1b\[[0-9;]*m/g;
+      const message =
+        stderrStr.replace(ANSI_RE, "").trim() ||
+        (thrown instanceof Error ? thrown.message : String(thrown));
+      throw new SentryError(message, exitCode, stderrStr);
+    }
 
     // Flush telemetry (no beforeExit handler in library mode)
     try {

src/lib/command.ts

@@ -483,10 +483,10 @@ export function buildCommand<
       : undefined;
 
     // OutputError handler: render data through the output system, then
-    // exit with the error's code. Stricli overwrites process.exitCode = 0
-    // after successful returns, so process.exit() is the only way to
-    // preserve a non-zero code. This lives in the framework — commands
-    // simply `throw new OutputError(data)`.
+    // re-throw so the exit code propagates. Stricli's
+    // exceptionWhileRunningCommand intercepts OutputError and re-throws
+    // it without formatting, so both bin.ts and index.ts can set
+    // exitCode from the caught error.
     const handleOutputError = (err: unknown): never => {
       if (err instanceof OutputError && outputConfig) {
         // Only render if there's actual data to show
@@ -498,7 +498,7 @@ export function buildCommand<
             renderer
           );
         }
-        process.exit(err.exitCode);
+        throw err;
       }
       throw err;
     };

test/commands/cli/fix.test.ts

@@ -3,23 +3,15 @@
  *
  * Output goes through the structured CommandOutput/OutputError path, which
  * renders via the `output` config (stdout). For failure paths, `OutputError`
- * triggers `process.exit()` — tests mock it to capture the exit code.
+ * is thrown — tests catch it and check the exitCode property.
  *
  * Tests run non-TTY so plain mode applies: markdown is parsed and ANSI
  * stripped. Headings render without `###`, underscores are unescaped,
  * and code spans have backticks stripped.
  */
 
 import { Database } from "bun:sqlite";
-import {
-  afterEach,
-  beforeEach,
-  describe,
-  expect,
-  mock,
-  spyOn,
-  test,
-} from "bun:test";
+import { afterEach, describe, expect, mock, spyOn, test } from "bun:test";
 import { chmodSync, statSync } from "node:fs";
 import { join } from "node:path";
 import { fixCommand } from "../../../src/commands/cli/fix.js";
@@ -29,6 +21,7 @@ import {
   generatePreMigrationTableDDL,
   initSchema,
 } from "../../../src/lib/db/schema.js";
+import { OutputError } from "../../../src/lib/errors.js";
 import { useTestConfigDir } from "../../helpers.js";
 
 /**
@@ -77,24 +70,11 @@ function createDatabaseWithMissingTables(
   ).run();
 }
 
-/**
- * Thrown by the mock `process.exit()` to halt execution without actually
- * exiting the process. The `code` field captures the requested exit code.
- */
-class MockExitError extends Error {
-  code: number;
-  constructor(code: number) {
-    super(`process.exit(${code})`);
-    this.code = code;
-  }
-}
-
 /**
  * Create a mock Stricli context that captures stdout output.
  *
  * The `buildCommand` wrapper renders structured output to `context.stdout`.
- * For failure paths (OutputError), it calls `process.exit()` — the mock
- * intercepts this and throws MockExitError to halt execution.
+ * For failure paths, `OutputError` is thrown after data is rendered.
  */
 function createContext() {
   const stdoutChunks: string[] = [];
@@ -120,30 +100,24 @@ const getTestDir = useTestConfigDir("fix-test-");
 
 /**
  * Run the fix command with the given flags and return captured output.
- * Mocks `process.exit()` so OutputError paths don't terminate the test.
+ * Catches `OutputError` to extract the exit code (data is already rendered
+ * to stdout before the throw).
  */
 async function runFix(dryRun: boolean) {
   const { context, getOutput } = createContext();
 
   let exitCode = 0;
-  const originalExit = process.exit;
-  process.exit = ((code?: number) => {
-    exitCode = code ?? 0;
-    throw new MockExitError(code ?? 0);
-  }) as typeof process.exit;
 
   try {
     const func = await fixCommand.loader();
     await func.call(context, { "dry-run": dryRun, json: false });
     // Successful return — exitCode stays 0
   } catch (err) {
-    if (err instanceof MockExitError) {
-      exitCode = err.code;
+    if (err instanceof OutputError) {
+      exitCode = err.exitCode;
     } else {
       throw err;
     }
-  } finally {
-    process.exit = originalExit;
   }
 
   return {
@@ -491,25 +465,7 @@ describe("sentry cli fix", () => {
 describe("sentry cli fix — ownership detection", () => {
   const getOwnershipTestDir = useTestConfigDir("fix-ownership-test-");
 
-  let exitMock: { restore: () => void; exitCode: number };
-
-  beforeEach(() => {
-    const originalExit = process.exit;
-    const state = {
-      exitCode: 0,
-      restore: () => {
-        process.exit = originalExit;
-      },
-    };
-    process.exit = ((code?: number) => {
-      state.exitCode = code ?? 0;
-      throw new MockExitError(code ?? 0);
-    }) as typeof process.exit;
-    exitMock = state;
-  });
-
   afterEach(() => {
-    exitMock.restore();
     closeDatabase();
   });
 
@@ -521,13 +477,16 @@ describe("sentry cli fix — ownership detection", () => {
   async function runFixWithUid(dryRun: boolean, getuid: () => number) {
     const { context, getOutput } = createContext();
     const getuidSpy = spyOn(process, "getuid").mockImplementation(getuid);
-    exitMock.exitCode = 0;
+
+    let exitCode = 0;
 
     try {
       const func = await fixCommand.loader();
       await func.call(context, { "dry-run": dryRun, json: false });
     } catch (err) {
-      if (!(err instanceof MockExitError)) {
+      if (err instanceof OutputError) {
+        exitCode = err.exitCode;
+      } else {
         throw err;
       }
     } finally {
@@ -536,7 +495,7 @@ describe("sentry cli fix — ownership detection", () => {
 
     return {
       output: getOutput(),
-      exitCode: exitMock.exitCode,
+      exitCode,
     };
   }
 

test/lib/command.test.ts

@@ -14,6 +14,7 @@ import {
   buildRouteMap,
   type CommandContext,
   run,
+  text_en,
 } from "@stricli/core";
 import {
   applyLoggingFlags,
@@ -1278,9 +1279,6 @@ describe("buildCommand return-based output", () => {
   });
 
   test("OutputError renders data and exits with error code", async () => {
-    let exitCalledWith: number | undefined;
-    const originalExit = process.exit;
-
     const command = buildCommand<
       { json: boolean; fields?: string[] },
       [],
@@ -1301,37 +1299,34 @@ describe("buildCommand return-based output", () => {
       routes: { test: command },
       docs: { brief: "Test app" },
     });
-    const app = buildApplication(routeMap, { name: "test" });
+    // Re-throw OutputError from Stricli's error handler (matches production app.ts)
+    const app = buildApplication(routeMap, {
+      name: "test",
+      localization: {
+        loadText: () => ({
+          ...text_en,
+          exceptionWhileRunningCommand: (exc: unknown) => {
+            if (exc instanceof OutputError) throw exc;
+            return text_en.exceptionWhileRunningCommand(exc, false);
+          },
+        }),
+      },
+    });
     const ctx = createTestContext();
 
-    // Mock process.exit — must throw to prevent fall-through since
-    // the real process.exit() is typed as `never`
-    class MockExit extends Error {
-      code: number;
-      constructor(code: number) {
-        super(`process.exit(${code})`);
-        this.code = code;
-      }
-    }
-    process.exit = ((code?: number) => {
-      exitCalledWith = code;
-      throw new MockExit(code ?? 0);
-    }) as typeof process.exit;
-
+    // OutputError is now thrown (not process.exit) — catch it
     try {
       await run(app, ["test"], ctx as TestContext);
-    } finally {
-      process.exit = originalExit;
+      expect.unreachable("Expected OutputError to be thrown");
+    } catch (err) {
+      expect(err).toBeInstanceOf(OutputError);
+      expect((err as OutputError).exitCode).toBe(1);
     }
-    expect(exitCalledWith).toBe(1);
-    // Output was rendered BEFORE exit
+    // Output was rendered BEFORE the throw
     expect(ctx.output.join("")).toContain("Error: not found");
   });
 
   test("OutputError renders JSON in --json mode", async () => {
-    let exitCalledWith: number | undefined;
-    const originalExit = process.exit;
-
     const command = buildCommand<
       { json: boolean; fields?: string[] },
       [],
@@ -1352,27 +1347,29 @@ describe("buildCommand return-based output", () => {
       routes: { test: command },
       docs: { brief: "Test app" },
     });
-    const app = buildApplication(routeMap, { name: "test" });
+    // Re-throw OutputError from Stricli's error handler (matches production app.ts)
+    const app = buildApplication(routeMap, {
+      name: "test",
+      localization: {
+        loadText: () => ({
+          ...text_en,
+          exceptionWhileRunningCommand: (exc: unknown) => {
+            if (exc instanceof OutputError) throw exc;
+            return text_en.exceptionWhileRunningCommand(exc, false);
+          },
+        }),
+      },
+    });
     const ctx = createTestContext();
 
-    class MockExit extends Error {
-      code: number;
-      constructor(code: number) {
-        super(`process.exit(${code})`);
-        this.code = code;
-      }
-    }
-    process.exit = ((code?: number) => {
-      exitCalledWith = code;
-      throw new MockExit(code ?? 0);
-    }) as typeof process.exit;
-
+    // OutputError is now thrown (not process.exit) — catch it
     try {
       await run(app, ["test", "--json"], ctx as TestContext);
-    } finally {
-      process.exit = originalExit;
+      expect.unreachable("Expected OutputError to be thrown");
+    } catch (err) {
+      expect(err).toBeInstanceOf(OutputError);
+      expect((err as OutputError).exitCode).toBe(1);
     }
-    expect(exitCalledWith).toBe(1);
     const jsonOutput = JSON.parse(ctx.output.join(""));
     expect(jsonOutput).toEqual({ error: "not found" });
   });