Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions .archgate/adrs/ARCH-002-error-handling.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:**

Expand All @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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

Expand Down
7 changes: 4 additions & 3 deletions .archgate/adrs/ARCH-002-error-handling.rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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+)\)/)
Expand All @@ -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,
});
Expand Down
1 change: 1 addition & 0 deletions .archgate/adrs/ARCH-005-testing-standards.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
16 changes: 16 additions & 0 deletions src/helpers/sentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() },
Expand Down
24 changes: 16 additions & 8 deletions tests/helpers/sentry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });

Expand All @@ -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");
Expand Down
Loading