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
39 changes: 31 additions & 8 deletions .archgate/adrs/ARCH-001-command-structure.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Commands live in src/commands/ and export a register\*Command(program) function.
- **DO** use src/commands/<name>.ts for top-level commands
- **DO** use src/commands/<name>/index.ts for command groups with subcommands
- **DO** import the register function explicitly in src/cli.ts
- **DO** wrap all async logic in src/cli.ts in an async function main() and call it as main().catch((err) => { logError(String(err)); process.exit(2); }) — this is required for bun build --compile --bytecode compatibility
- **DO** wrap all async logic in src/cli.ts in an async function main() called as main().catch(...) — required for bun build --compile --bytecode compatibility. The catch handler branches per ARCH-002: ExitPromptError exits 130 silently, UserError logs and exits 1 without Sentry, anything else is captured to Sentry and exits 2 — always via exitWith(), never bare process.exit()

### Don't

Expand All @@ -59,18 +59,25 @@ import type { Command } from "@commander-js/extra-typings";
import { loadRuleAdrs } from "../engine/loader";
import { runChecks } from "../engine/runner";
import { reportConsole, reportJSON, getExitCode } from "../engine/reporter";
import { exitWith, handleCommandError } from "../helpers/exit";

export function registerCheckCommand(program: Command) {
program
.command("check")
.description("Run automated ADR compliance checks")
.option("--json", "Output results as JSON")
.action(async (opts) => {
const adrs = await loadRuleAdrs();
const results = await runChecks(adrs);
if (opts.json) reportJSON(results);
else reportConsole(results);
process.exit(getExitCode(results));
try {
const adrs = await loadRuleAdrs();
const results = await runChecks(adrs);
if (opts.json) reportJSON(results);
else reportConsole(results);
await exitWith(getExitCode(results));
} catch (err) {
// Re-throws ExitPromptError (Ctrl+C → exit 130 in main().catch());
// UserError exits 1, anything else exits 2 + Sentry (ARCH-012).
await handleCommandError(err);
}
});
}
```
Expand Down Expand Up @@ -107,6 +114,9 @@ bun build --compile --bytecode — the command used to produce standalone binari
```typescript
// src/cli.ts — GOOD: all async logic wrapped in main()
import { logError } from "./helpers/log";
import { exitWith } from "./helpers/exit";
import { captureException } from "./helpers/sentry";
import { UserError } from "./helpers/user-error";
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Synchronous bootstrap checks can remain at top level
createPathIfNotExists(paths.cacheFolder);
Expand All @@ -121,9 +131,22 @@ async function main() {
await program.parseAsync(process.argv);
}

main().catch((err) => {
main().catch(async (err: unknown) => {
// Ctrl+C during an interactive prompt — exit silently (ARCH-002)
if (err instanceof Error && err.name === "ExitPromptError") {
await exitWith(130, { outcome: "cancelled" });
}

// Expected failure that escaped a command boundary — log, exit 1, no Sentry
if (err instanceof UserError) {
logError(err.message);
await exitWith(1, { outcome: "user_error" });
}

// Internal bug — capture to Sentry, then exit 2
captureException(err, { command: "main" });
logError(String(err));
process.exit(2);
await exitWith(2, { outcome: "internal_error" });
});
```

Expand Down
2 changes: 1 addition & 1 deletion .archgate/adrs/ARCH-002-error-handling.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ Use four exit codes with clear semantics:

- **DO** use `logError()` from `src/helpers/log.ts` for user-facing errors — it writes to stderr, never stdout
- **DO** exit with code 1 for expected failures (missing config, invalid input, violations found)
- **DO** let unexpected errors crash naturally (exit code 2)
- **DO** let unexpected errors propagate to the command boundary (exit 2)
- **DO** provide actionable suggestions in error messages
- **DO** fall back to `process.cwd()` when `findProjectRoot()` returns null in commands that don't require `.archgate/` — e.g., `session-context` reads `~/.claude/projects/` and uses `process.cwd()` as its path key
- **DO** handle Inquirer's `ExitPromptError` 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
Expand Down
12 changes: 5 additions & 7 deletions .archgate/adrs/ARCH-004-no-barrel-files.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,17 @@ files: ["src/**/*.ts"]

## Context

Barrel files are `index.ts` files whose sole purpose is re-exporting symbols from sibling modules. They introduce five concrete problems:
Barrel files are `index.ts` files whose sole purpose is re-exporting symbols from sibling modules. They introduce four concrete problems:

1. **Circular dependency risk** — A barrel pulls all siblings into one module surface, so cycles (module A imports the barrel that re-exports module B, which imports A) hide behind the indirection layer.
2. **Tree-shaking degradation** — Bun's module cache treats the barrel as a single unit, pulling in all symbols even when only one is needed, increasing memory footprint and startup time.
3. **Hidden coupling** — Consumers cannot tell which concrete module provides a symbol. This obscures the real dependency graph and masks architectural drift: moving a function between source modules requires no import change when the barrel re-exports both.
4. **IDE confusion** — The same symbol is reachable from both the barrel (`../formats`) and the source module (`../formats/adr`), producing inconsistent import paths across the codebase.
5. **Grep-unfriendly navigation** — Symbol searches land on the barrel first, costing an extra hop to reach the real implementation.
2. **Hidden coupling** — Consumers cannot tell which concrete module provides a symbol. This obscures the real dependency graph and masks architectural drift: moving a function between source modules requires no import change when the barrel re-exports both.
3. **IDE confusion** — The same symbol is reachable from both the barrel (`../formats`) and the source module (`../formats/adr`), producing inconsistent import paths across the codebase.
4. **Grep-unfriendly navigation** — Symbol searches land on the barrel first, costing an extra hop to reach the real implementation.

**Alternatives considered:**

- **Barrels as "public API" facades** — Appropriate for npm packages with external consumers; Archgate CLI has none, so the facade adds indirection without value.
- **Barrels only at package boundaries** (e.g., `src/engine/index.ts`) — Still carries the circular dependency and tree-shaking costs, plus the overhead of deciding which directories "deserve" one.
- **Barrels only at package boundaries** (e.g., `src/engine/index.ts`) — Still carries the circular dependency cost, plus the overhead of deciding which directories "deserve" one.
- **Path aliases** (e.g., `@engine/loader`) — Archgate uses Bun's native module resolution without `paths` ([ARCH-006 — Dependency Policy](./ARCH-006-dependency-policy.md)), and `paths` configuration carries its own maintenance burden.

Every module here is internal and consumed only within this repository, so direct imports keep the dependency graph explicit and auditable; the extra path verbosity is a worthwhile trade. This refines [ARCH-001 — Command Structure](./ARCH-001-command-structure.md), which permits `index.ts` for command groups containing real logic: `index.ts` with logic is permitted, `index.ts` that only re-exports is forbidden.
Expand Down Expand Up @@ -111,7 +110,6 @@ export function registerAdrCommand(program: Command) {
- **Faster IDE navigation** — Go-to-definition jumps straight to the source module
- **Simpler grep results** — Symbol searches find the real implementation without hops through barrels
- **Consistent import style** — One direct pattern everywhere; no ambiguity between barrel and source
- **Better tree-shaking** — Bun resolves only the module needed, not unrelated siblings behind a shared barrel

### Negative

Expand Down
21 changes: 17 additions & 4 deletions .archgate/adrs/ARCH-008-typed-command-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,21 +57,34 @@ Options that require type narrowing beyond plain strings MUST use `new Option()`
```typescript
import type { Command } from "@commander-js/extra-typings";
import { Option } from "@commander-js/extra-typings";
import { EDITOR_TARGETS } from "../helpers/init-project";

// EDITOR_TARGETS is the shared editor list — reference it instead of
// re-hardcoding a literal that drifts when an editor is added.
const editorOption = new Option("--editor <editor>", "target editor")
.choices(["claude", "cursor", "vscode", "copilot"] as const)
.choices(EDITOR_TARGETS) // ["claude", "cursor", "vscode", "copilot", "opencode"] as const
.default("claude" as const);

export function registerExampleCommand(program: Command) {
program
.command("example")
.addOption(editorOption)
.action(async (opts) => {
// opts.editor is typed as "claude" | "cursor" | "vscode" | "copilot"
// TypeScript enforces exhaustive matching
// opts.editor is typed as the EDITOR_TARGETS union. The never-typed
// default is what makes the switch compile-time exhaustive: adding an
// editor to EDITOR_TARGETS without a case fails to compile here.
switch (opts.editor) {
case "claude":
break;
case "cursor":
case "vscode":
case "copilot":
case "opencode":
break;
default: {
const _exhaustive: never = opts.editor;
throw new Error(`Unhandled editor: ${String(_exhaustive)}`);
}
}
});
}
Expand Down Expand Up @@ -123,7 +136,7 @@ program
- **Compile-time safety** — Invalid option values are caught by TypeScript, not just at runtime
- **Consistent error messages** — Commander produces standard error output for invalid choices
- **No boilerplate validation** — Eliminates repeated `if (!VALID.includes(...))` patterns
- **Exhaustive switch/case** — TypeScript ensures all choices are handled when switching on the option value
- **Exhaustive switch/case** — With a `never`-typed `default` branch, TypeScript flags any unhandled choice when switching on the option value

### Negative

Expand Down
22 changes: 10 additions & 12 deletions .archgate/adrs/ARCH-012-command-error-boundaries.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ files: ["src/commands/**/*.ts"]

Async command actions that lack try-catch error boundaries produce poor user experiences when they fail. Without explicit error handling:

1. Errors propagate to the top-level `main().catch()` in `cli.ts`, which exits with code 2 (internal error) and shows only the raw error message
1. Errors propagate to the top-level `main().catch()` in `cli.ts`, which shows only the raw error message and exits with code 2 (internal error) for anything that is not a `UserError` (exit 1) or `ExitPromptError` (exit 130)
2. Users cannot distinguish between a command failure (code 1) and a CLI bug (code 2)
3. Error messages lack context about what the command was trying to do

Expand All @@ -27,10 +27,9 @@ ARCH-002 defines the exit code convention and logging patterns, but does not req
Every async command action MUST wrap its body in a try-catch block that:

1. Catches errors from async operations
2. Formats them with `logError()` from `src/helpers/log.ts`
3. Exits with code 1 (expected failure) for user-facing errors
2. Routes them through `handleCommandError()` from `src/helpers/exit.ts`, which re-throws `ExitPromptError` (so Ctrl+C reaches `main().catch()` for exit 130), logs with `logError()`, and exits 1 for `UserError` (expected failure) or 2 plus Sentry capture for anything else

The top-level `main().catch()` in `cli.ts` remains as a safety net for truly unexpected errors (code 2), but it should never be the primary error handler for commands.
The top-level `main().catch()` in `cli.ts` remains as a safety net for errors that escape a boundary, but it should never be the primary error handler for commands.
Comment thread
coderabbitai[bot] marked this conversation as resolved.

**Pattern:**

Expand All @@ -39,10 +38,9 @@ The top-level `main().catch()` in `cli.ts` remains as a safety net for truly une
try {
// command logic
} catch (err) {
// Re-throw ExitPromptError so main().catch() handles Ctrl+C (exit 130)
if (err instanceof Error && err.name === "ExitPromptError") throw err;
logError(err instanceof Error ? err.message : String(err));
process.exit(1);
// Re-throws ExitPromptError (Ctrl+C → exit 130 in main().catch());
// UserError exits 1, anything else exits 2 + Sentry.
await handleCommandError(err);
}
});
```
Expand All @@ -58,16 +56,16 @@ The top-level `main().catch()` in `cli.ts` remains as a safety net for truly une

- Wrap every async command action body in a try-catch
- **Cover the ENTIRE action body** — the try block MUST start at the first statement of the action and end at the last. A boundary that wraps only part of the body (e.g., a single risky call) still lets errors from the uncovered statements escape to `main().catch()`, converting expected failures (exit 1) into internal crashes (exit 2 + Sentry). Incident: `check.ts` once wrapped only `loadRuleAdrs()` — a `UserError` thrown later by `runChecks()` escaped and was reported to Sentry (issue CLI-5)
- Use `logError()` for error messages in the catch block
- Exit with code 1 for expected failures
- **Re-throw `ExitPromptError` in command error boundaries** — Commands that use Inquirer prompts (directly or via helpers like `promptEditorSelection`) MUST re-throw `ExitPromptError` from the catch block so `main().catch()` handles Ctrl+C with exit code 130. Pattern: `if (err instanceof Error && err.name === "ExitPromptError") throw err;`
- Route the catch block through `handleCommandError()` — it logs with `logError()` and selects the exit code
- Throw `UserError` for expected failures so the boundary exits with code 1
- **Re-throw `ExitPromptError` in command error boundaries** — Commands that use Inquirer prompts (directly or via helpers like `promptEditorSelection`) MUST re-throw `ExitPromptError` from the catch block so `main().catch()` handles Ctrl+C with exit code 130. `handleCommandError()` does this automatically; a hand-written catch needs `if (err instanceof Error && err.name === "ExitPromptError") throw err;`

### Don't

- Don't rely on `main().catch()` as the only error handler for commands
- Don't scope the try-catch to a subset of the action body — partial boundaries pass the automated presence check while still leaking errors from uncovered statements
- Don't catch and silently swallow errors — always log them
- Don't exit with code 2 in command catch blocks — that code is reserved for unexpected crashes
- Don't hardcode exit code 2 in command catch blocks — `handleCommandError()` decides between 1 (expected) and 2 (unexpected crash)
- Don't catch `ExitPromptError` as a command failure — it represents user cancellation (Ctrl+C), not an error. Let it propagate to `main().catch()` for exit code 130 handling (see [ARCH-002](./ARCH-002-error-handling.md))

## Consequences
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ Command names are derived from the [src/commands/](../../src/commands/) director

Nested subcommand files (`src/commands/adr/create.ts`, `src/commands/adr/domain/index.ts`, etc.) are NOT treated as top-level commands and contribute nothing to the expected docs set.

The module layout and `src/cli.ts` registration MUST agree in both directions: every command module has a `register*Command(program)` call in `src/cli.ts` (derived by kebab-casing the register name: `registerReviewContextCommand` → `review-context`), and every register call has a module at a conventional path.

## Do's and Don'ts

### Do
Expand All @@ -71,14 +73,14 @@ Nested subcommand files (`src/commands/adr/create.ts`, `src/commands/adr/domain/

- **Discoverability guaranteed.** Every command shipped in the CLI has a dedicated, linkable reference page.
- **Orphan detection.** Docs pages that outlive their command are flagged automatically, keeping the reference section truthful.
- **Cheap to enforce.** The rule reads directory listings only — no AST parsing of `src/cli.ts`, no `--help` invocation, no cross-process work.
- **Cheap to enforce.** The rule reads directory listings and walks `src/cli.ts`'s AST for executable register calls (via the in-process parser, ARCH-022) — no `--help` invocation, no cross-process work.
- **Aligns with existing conventions.** Piggybacks on the `src/commands/<name>.ts` / `src/commands/<name>/index.ts` pattern from ARCH-001 without introducing new metadata.
- **Composes with GEN-002.** This rule handles command↔EN-doc parity; GEN-002 handles EN↔pt-br parity. Together they guarantee every command has docs in every supported locale.

### Negative

- **Prose overhead on new commands.** Adding a top-level command requires writing a reference page, not just code + tests. Mitigated by the short, templated structure of existing `.mdx` files.
- **False negative for exotic layouts.** A command-registration pattern that bypasses both `src/commands/<name>.ts` and `src/commands/<name>/index.ts` is invisible to the rule. ARCH-001 forbids this, so the drift is caught there first.
- **Naming-convention dependency.** The registration cross-check maps `register<Name>Command` to a module path by kebab-casing, so a register function whose name diverges from its module stem is reported as a mismatch even if it works at runtime. That is deliberate: ARCH-001's convention is what makes docs coverage derivable from the layout.

### Risks

Expand All @@ -89,7 +91,7 @@ Nested subcommand files (`src/commands/adr/create.ts`, `src/commands/adr/domain/

### Automated Enforcement

- **Archgate rule** `ARCH-015/cli-command-has-docs-page`: Enumerates top-level commands under `src/commands/` and `.mdx` pages under `docs/src/content/docs/reference/cli/`, then reports any mismatch in either direction (missing docs, orphan docs). Severity: `error`. Runs as part of `bun run validate` via `archgate check`.
- **Archgate rule** `ARCH-015/cli-command-has-docs-page`: Enumerates top-level commands under `src/commands/` and `.mdx` pages under `docs/src/content/docs/reference/cli/`, then reports any mismatch in either direction (missing docs, orphan docs). Also cross-checks the module layout against executable `register*Command(program)` call expressions in `src/cli.ts`'s AST, reporting unregistered modules and register calls without a conventional module path. Severity: `error`. Runs as part of `bun run validate` via `archgate check`.

### Manual Enforcement

Expand Down
Loading
Loading