diff --git a/.archgate/adrs/ARCH-002-error-handling.md b/.archgate/adrs/ARCH-002-error-handling.md index d1db6e6..ff870cc 100644 --- a/.archgate/adrs/ARCH-002-error-handling.md +++ b/.archgate/adrs/ARCH-002-error-handling.md @@ -22,13 +22,14 @@ The three-tier exit code model provides a simple contract that covers all CLI us ## Decision -Use three exit codes with clear semantics: +Use four exit codes with clear semantics: -| Exit Code | Meaning | When to Use | -| --------- | ---------------- | ----------------------------------------------------------------------------- | -| `0` | Success | Operation completed successfully | -| `1` | Expected failure | Invalid input, missing config, ADR violations found, operation cannot proceed | -| `2` | Internal error | Bugs, unhandled exceptions, unexpected crashes | +| Exit Code | Meaning | When to Use | +| --------- | ----------------- | ----------------------------------------------------------------------------- | +| `0` | Success | Operation completed successfully | +| `1` | Expected failure | Invalid input, missing config, ADR violations found, operation cannot proceed | +| `2` | Internal error | Bugs, unhandled exceptions, unexpected crashes | +| `130` | User cancellation | User pressed Ctrl+C / SIGINT during an interactive prompt | **Error output conventions:** @@ -48,6 +49,7 @@ Use three exit codes with clear semantics: - Provide actionable suggestions in error messages - Write errors to stderr (via `logError()`), not stdout - **Commands that don't require `.archgate/` SHOULD fall back to `process.cwd()`** when `findProjectRoot()` returns null — e.g., `session-context` reads from `~/.claude/projects/` and uses `process.cwd()` as its path key when no project is found +- **Handle `ExitPromptError` from Inquirer as user cancellation** — catch it in the top-level error boundary and exit with code 130 (SIGINT convention) without logging an error or sending to Sentry ### Don't @@ -56,7 +58,8 @@ Use three exit codes with clear semantics: - Don't use `console.error()` directly — use `logError()` for consistent formatting - Don't use `console.log()` or `console.warn()` directly in helper or engine files — use `logInfo()` or `logWarn()` (command files are the I/O layer and may use console directly) - Don't exit with code 0 when an operation fails -- Don't use exit codes other than 0, 1, or 2 +- Don't use exit codes other than 0, 1, 2, or 130 +- Don't send user-cancellation errors (e.g., `ExitPromptError` from Inquirer) to Sentry — filter them in `beforeSend` ## Implementation Pattern @@ -133,7 +136,7 @@ try { - **Archgate rule** `ARCH-002/use-log-error`: Scans all source files (excluding `helpers/log.ts` and test files) for `console.error()` usage and flags violations. Severity: `error`. - **Archgate rule** `ARCH-002/use-log-helpers`: Scans helper and engine files for direct `console.log()`, `console.warn()`, or `console.info()` usage. Excludes `helpers/log.ts` (canonical implementation), `engine/reporter.ts` (check output system), `helpers/login-flow.ts` (interactive device flow UI), and test files. Command files are exempt since they are the I/O layer. Severity: `error`. -- **Archgate rule** `ARCH-002/exit-code-convention`: Scans all source files for `process.exit()` calls and verifies the exit code is 0, 1, or 2. Severity: `error`. +- **Archgate rule** `ARCH-002/exit-code-convention`: Scans all source files for `process.exit()` calls and verifies the exit code is 0, 1, 2, or 130. Severity: `error`. ### Manual Enforcement diff --git a/.archgate/adrs/ARCH-002-error-handling.rules.ts b/.archgate/adrs/ARCH-002-error-handling.rules.ts index 74c7deb..92b5a64 100644 --- a/.archgate/adrs/ARCH-002-error-handling.rules.ts +++ b/.archgate/adrs/ARCH-002-error-handling.rules.ts @@ -54,8 +54,9 @@ export default { }, }, "exit-code-convention": { - description: "Process.exit should use codes 0, 1, or 2 only", + description: "Process.exit should use codes 0, 1, 2, or 130 only", async check(ctx) { + const allowedCodes = new Set([0, 1, 2, 130]); const matches = await Promise.all( ctx.scopedFiles.map((file) => ctx.grep(file, /process\.exit\((\d+)\)/) @@ -66,9 +67,9 @@ export default { const codeMatch = m.content.match(/process\.exit\((\d+)\)/); if (codeMatch) { const code = Number(codeMatch[1]); - if (code !== 0 && code !== 1 && code !== 2) { + if (!allowedCodes.has(code)) { ctx.report.violation({ - message: `Exit code ${code} is not standard. Use 0 (success), 1 (failure), or 2 (internal error)`, + message: `Exit code ${code} is not standard. Use 0 (success), 1 (failure), 2 (internal error), or 130 (user cancellation/SIGINT)`, file: m.file, line: m.line, }); diff --git a/.archgate/adrs/ARCH-005-testing-standards.md b/.archgate/adrs/ARCH-005-testing-standards.md index e116ce1..c5695f0 100644 --- a/.archgate/adrs/ARCH-005-testing-standards.md +++ b/.archgate/adrs/ARCH-005-testing-standards.md @@ -53,6 +53,7 @@ Use Bun's built-in test runner (`bun test`) for all tests. Test files go in `tes - Don't leave temp files after test runs - **Don't leave external SDK instances open after tests** — instances from external libraries may hold internal references that keep Bun's event loop alive on Linux, causing `bun test` to hang indefinitely after all tests complete even though every test passes. Always call the cleanup method in `afterEach`. - **Don't rely on globally-configured git identity in temp git repos** — always set `user.email` and `user.name` locally in any repo that makes commits. Omitting this works locally (where developers have global git config) but fails silently in CI, producing a cryptic `ShellPromise` error with no indication that git identity is the cause. +- **Don't let tests send real events to Sentry** — set `Bun.env.NODE_ENV = "test"` in `beforeEach` (and restore in `afterEach`) for any test that initializes Sentry. The Sentry SDK is configured with `enabled: Bun.env.NODE_ENV !== "test"` to prevent test noise from polluting production error tracking. - Don't skip tests without a tracking issue - Don't import test utilities from `node:test` — use Bun's built-in `bun:test` module - **Don't use `mock.module("node:fetch", ...)` to intercept HTTP fetch calls** — in Bun, the runtime fetch is `globalThis.fetch` and `mock.module` targeting `node:fetch` does not intercept it. The mock silently has no effect: the real network is hit, making tests non-deterministic and dependent on external services. Assign `globalThis.fetch` directly instead (see Do's above). diff --git a/CLAUDE.md b/CLAUDE.md index f4324e7..2d6d034 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -54,7 +54,7 @@ The npm package is a **thin shim** — it contains only `bin/archgate.cjs` and ` - Commands export `register*Command(program)`, handle I/O only — no business logic - OS: macOS, Linux, and Windows - Output: `styleText()` from `node:util`; `--json` for machine-readable; auto-compact JSON in agent contexts (non-TTY, non-CI); no emoji -- Exit codes: 0 = success, 1 = violation, 2 = internal error +- Exit codes: 0 = success, 1 = violation, 2 = internal error, 130 = user cancellation (SIGINT) - Deps: minimal; prefer Bun built-ins (see ARCH-006) ## Toolchain diff --git a/src/cli.ts b/src/cli.ts index aa7b3d5..bc6b955 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -135,6 +135,11 @@ function getFullCommandName(command: Command): string { } main().catch(async (err: unknown) => { + // User pressed Ctrl+C during an Inquirer prompt — exit silently + if (err instanceof Error && err.name === "ExitPromptError") { + process.exit(130); + } + captureException(err, { command: "main" }); await Promise.all([flushTelemetry(), flushSentry()]); logError(String(err)); diff --git a/src/helpers/sentry.ts b/src/helpers/sentry.ts index 77e5477..50e92fa 100644 --- a/src/helpers/sentry.ts +++ b/src/helpers/sentry.ts @@ -74,8 +74,24 @@ export function initSentry(): void { dsn: SENTRY_DSN, release: cliVersion, environment: Bun.env.NODE_ENV ?? "production", + // Disable sending events in test environments + enabled: Bun.env.NODE_ENV !== "test", // Do not send default PII (hostnames, IPs, etc.) sendDefaultPii: false, + // Drop user-initiated prompt cancellations (Ctrl+C) + beforeSend(event) { + const values = event.exception?.values; + if ( + values?.some( + (v) => + v.type === "ExitPromptError" || + v.value?.includes("force closed the prompt with SIGINT") + ) + ) { + return null; + } + return event; + }, // Set the anonymous install ID as the user initialScope: { user: { id: getInstallId() }, diff --git a/tests/helpers/sentry.test.ts b/tests/helpers/sentry.test.ts index 7473010..78336e4 100644 --- a/tests/helpers/sentry.test.ts +++ b/tests/helpers/sentry.test.ts @@ -7,21 +7,29 @@ describe("sentry", () => { let tempDir: string; let originalHome: string | undefined; let originalTelemetryEnv: string | undefined; + let originalNodeEnv: string | undefined; beforeEach(() => { tempDir = mkdtempSync(join(tmpdir(), "archgate-sentry-test-")); - originalHome = process.env.HOME; - originalTelemetryEnv = process.env.ARCHGATE_TELEMETRY; - process.env.HOME = tempDir; - delete process.env.ARCHGATE_TELEMETRY; + originalHome = Bun.env.HOME; + originalTelemetryEnv = Bun.env.ARCHGATE_TELEMETRY; + originalNodeEnv = Bun.env.NODE_ENV; + Bun.env.HOME = tempDir; + Bun.env.NODE_ENV = "test"; + delete Bun.env.ARCHGATE_TELEMETRY; }); afterEach(async () => { - process.env.HOME = originalHome; + Bun.env.HOME = originalHome; if (originalTelemetryEnv === undefined) { - delete process.env.ARCHGATE_TELEMETRY; + delete Bun.env.ARCHGATE_TELEMETRY; } else { - process.env.ARCHGATE_TELEMETRY = originalTelemetryEnv; + Bun.env.ARCHGATE_TELEMETRY = originalTelemetryEnv; + } + if (originalNodeEnv === undefined) { + delete Bun.env.NODE_ENV; + } else { + Bun.env.NODE_ENV = originalNodeEnv; } rmSync(tempDir, { recursive: true, force: true }); @@ -42,7 +50,7 @@ describe("sentry", () => { }); test("does not initialize when telemetry is disabled", async () => { - process.env.ARCHGATE_TELEMETRY = "0"; + Bun.env.ARCHGATE_TELEMETRY = "0"; const { initSentry, captureException } = await import("../../src/helpers/sentry");