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
1 change: 1 addition & 0 deletions .archgate/adrs/ARCH-012-command-error-boundaries.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ The top-level `main().catch()` in `cli.ts` remains as a safety net for truly une
### Automated Enforcement

- **Archgate rule** `ARCH-012/async-action-error-boundary`: Scans async command actions for try-catch blocks. Severity: `warning` (some commands may have valid reasons for alternative patterns).
- **Archgate rule** `ARCH-012/exit-prompt-error-rethrow`: Verifies that async command actions with try-catch blocks include the `ExitPromptError` re-throw pattern. Severity: `error` — missing re-throws silently convert user cancellation (Ctrl+C, exit 130) into command failure (exit 1).

### Manual Enforcement

Expand Down
34 changes: 34 additions & 0 deletions .archgate/adrs/ARCH-012-command-error-boundaries.rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,39 @@ export default {
await Promise.all(checks);
},
},
"exit-prompt-error-rethrow": {
description:
"Catch blocks in async command actions must re-throw ExitPromptError for proper Ctrl+C handling (exit 130)",
async check(ctx) {
// Only check non-index command files that have async actions
const files = ctx.scopedFiles.filter(
(f) => f.includes("commands/") && !f.endsWith("index.ts")
);

const checks = files.map(async (file) => {
const content = await ctx.readFile(file);

// Only check files with async actions that have try-catch
const hasAsyncActionWithTryCatch =
/\.action\(\s*async\s[\s\S]*?\btry\s*\{/.test(content);
if (!hasAsyncActionWithTryCatch) return;

// Check for the ExitPromptError re-throw pattern anywhere in the file.
// The canonical pattern is:
// if (err instanceof Error && err.name === "ExitPromptError") throw err;
const hasExitPromptRethrow = /ExitPromptError/.test(content);

if (!hasExitPromptRethrow) {
ctx.report.violation({
message:
"Catch block in async command action must re-throw ExitPromptError so Ctrl+C exits with code 130 instead of code 1",
file,
fix: 'Add `if (err instanceof Error && err.name === "ExitPromptError") throw err;` as the first line in the catch block',
});
}
});
await Promise.all(checks);
},
},
},
} satisfies RuleSet;
2 changes: 2 additions & 0 deletions .claude/agent-memory/archgate-developer/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ Skipping steps 2 or 3 is a workflow violation. The user should NEVER have to inv
- **Windows binary upgrade: never use detached child processes for `.old` cleanup** — On Windows, `replaceBinary()` renames the running exe to `.old` because the OS file-locks it. Cleaning up the `.old` via a detached `cmd /c ping -n 2 ... & del` process is unreliable (process may not spawn, `del` may fail silently, timing races). Instead, `cleanupStaleBinary()` runs at the next CLI startup as a fire-and-forget `unlink()` — the file is guaranteed unlocked by then. The cleanup is platform-agnostic (uses `getArtifactInfo()` to resolve the binary name), so it works on any supported platform even though only Windows currently creates `.old` files. The sync `unlinkSync` in `replaceBinary()` is kept as defense-in-depth for leftover `.old` files from previous upgrades. Do NOT reintroduce detached cleanup processes.
- **`bun:sqlite` file handles persist after `db.close()` on Windows — wrap test cleanup in try/catch** — Tests that create temp SQLite databases via `new Database(path)` will fail with `EBUSY: resource busy or locked` when `rmSync` tries to remove the temp directory in `afterEach`, even after calling `db.close()`. Windows holds the file handle briefly. Fix: (1) set `PRAGMA journal_mode = DELETE` in test DBs to avoid creating WAL/SHM files, and (2) wrap `rmSync` in `afterEach` with `try { rmSync(...) } catch { /* SQLite handles may persist */ }`. Each test must use a unique temp dir name so leftover files don't collide.
- **Inquirer prompts leave cursor at wrong column on Windows — always reset with `cursorTo`** — After an `inquirer.prompt()` call finishes (especially checkbox with long wrapped answer lines), the cursor is left at the end of the rendered answer text. On Windows terminals, `\n` moves down but does NOT reset to column 0, so all subsequent output starts at the wrong horizontal offset. Fix: call `cursorTo(process.stdout, 0)` from `node:readline` after each prompt returns, guarded by `if (process.stdout.isTTY)`. Applied in `src/helpers/editor-detect.ts` for `promptEditorSelection()` and `promptSingleEditorSelection()`. The `upgrade.ts` command already uses the same `clearLine`/`cursorTo` pattern for download progress cleanup.
- **`inquirer` must be lazy-loaded via dynamic `import()` — never statically imported at module level** — `inquirer` costs ~200ms to parse. Static `import inquirer from "inquirer"` at module level forces every CLI invocation to pay this cost — even `--help`, `--version`, and non-interactive commands that never prompt. Always use `const { default: inquirer } = await import("inquirer")` at the call site, inside the action handler or function that actually needs it. Applied in `src/commands/adr/create.ts`, `src/commands/init.ts`, `src/helpers/editor-detect.ts`, `src/helpers/login-flow.ts`. Same principle applies to any heavy dependency used only in specific code paths (e.g., Sentry and PostHog SDKs are already lazy-loaded in `sentry.ts` and `telemetry.ts`).
- **Telemetry/Sentry init should be started eagerly but awaited lazily** — In `src/cli.ts`, `initSentry()` + `initTelemetry()` are started before command registration (so they run concurrently with setup) but only awaited in the `preAction` hook (right before the first telemetry event). This defers ~150ms of SDK parsing + git subprocess cost for `--help`/`--version` which never trigger `preAction`. The `preAction` hook is `async` to support this `await`.
- **`GITHUB_TOKEN`-authored pushes do NOT trigger downstream workflows — release.yml MUST use the GH App token** — When an Actions workflow pushes commits or opens PRs using `${{ github.token }}` / `secrets.GITHUB_TOKEN`, GitHub intentionally suppresses the resulting `push` / `pull_request` events to prevent recursion. Symptom on release PRs: the head SHA has no `pull_request`-event check runs, so `Validate Code` / `Lint, Test & Check` / `DCO Sign-off Check` are missing from the PR rollup and branch protection treats the PR as missing required checks. PR [#131](https://github.com/archgate/cli/pull/131) papered over this by manually `gh workflow run` + posting commit statuses, but `workflow_dispatch` runs land on `head_branch: release` with `pull_requests: []` — they are not associated with the PR ref, so `Lint, Test & Check` stayed orphaned and the bug recurred on PR [#251](https://github.com/archgate/cli/pull/251). Root-cause fix: in `release.yml` the `pull-request` job MUST generate a GitHub App installation token via `actions/create-github-app-token` (using `secrets.GH_APP_APP_ID` / `secrets.GH_APP_PRIVATE_KEY`) and pass it to BOTH `actions/checkout` and `simple-release-action`. App-token-authored pushes DO trigger `pull_request` events naturally. Apply the same pattern to any future workflow that pushes to a branch whose downstream CI must run.

## Validation Pipeline
Expand Down
26 changes: 16 additions & 10 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,16 @@ cleanupStaleBinary();
async function main() {
await installGit();

// Initialize error tracking and telemetry (no-ops if opted out).
// Start error tracking and telemetry initialization concurrently but do NOT
// await them here. Both SDKs are lazy-loaded via dynamic `import()` inside
// these functions, so the `ARCHGATE_TELEMETRY=0` path is a cheap no-op.
//
// Both SDKs are lazy-loaded via dynamic `import()` inside these functions,
// so the `ARCHGATE_TELEMETRY=0` path skips the parse/init cost entirely.
//
// Await telemetry so the repo context is resolved before the preAction
// hook fires `command_executed` — otherwise that event always lands
// without `repo_id` (see PR #211). The two init calls run concurrently,
// so the wall-clock cost is bounded by whichever is slowest.
await Promise.all([initSentry(), initTelemetry()]);
// The promise is awaited in the preAction hook — right before the first
// telemetry event fires — so `repo_id` is always present on `command_executed`
// events (see PR #211). This defers ~150ms of SDK parse + git subprocess cost
// off the critical path for --help, --version, and fast-exit commands that
// never trigger preAction.
const telemetryReady = Promise.all([initSentry(), initTelemetry()]);

const logLevelOption = new Option("--log-level <level>", "Set log verbosity")
.choices(["error", "warn", "info", "debug"] as const)
Expand All @@ -93,7 +93,13 @@ async function main() {
// Commander invokes the hook callback with (hookedCommand, actionCommand);
// the second arg is the actual subcommand being executed. We use the action
// command so `adr create` etc. resolves correctly instead of always "root".
program.hook("preAction", (_hookedCommand, actionCommand) => {
program.hook("preAction", async (_hookedCommand, actionCommand) => {
// Ensure telemetry SDKs are initialized before emitting any events.
// By the time a real command's preAction fires, the init promises have
// usually already resolved (they started before command registration),
// so this await is effectively free in practice.
await telemetryReady;

// Apply log level from global option before any command runs
const rootOpts = program.opts();
setLogLevel(rootOpts.logLevel as LogLevel);
Expand Down
7 changes: 6 additions & 1 deletion src/commands/adr/create.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { cursorTo } from "node:readline";

import type { Command } from "@commander-js/extra-typings";
import inquirer from "inquirer";

import type { AdrDomain } from "../../formats/adr";
import { createAdrFile } from "../../helpers/adr-writer";
Expand Down Expand Up @@ -56,6 +55,10 @@ export function registerAdrCreateCommand(adr: Command) {
body = opts.body;
} else {
const choices = getAllDomainNames(projectRoot);
// Lazy-load inquirer — it costs ~200ms to parse and is only
// needed for interactive prompts, not for scripted --title/--domain
// invocations or --help/--version.
const { default: inquirer } = await import("inquirer");
// Interactive mode
const answers = await inquirer.prompt([
{
Expand Down Expand Up @@ -108,6 +111,8 @@ export function registerAdrCreateCommand(adr: Command) {
console.log(`Created ADR: ${result.filePath}`);
}
} 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));
await exitWith(1);
}
Expand Down
1 change: 1 addition & 0 deletions src/commands/adr/domain/add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export function registerDomainAddCommand(domain: Command) {
console.log(`Registered custom domain: ${name} → ${prefix}`);
}
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
logError(err instanceof Error ? err.message : String(err));
await exitWith(1);
}
Expand Down
1 change: 1 addition & 0 deletions src/commands/adr/domain/remove.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export function registerDomainRemoveCommand(domain: Command) {
console.log(`Removed custom domain: ${name}`);
}
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
logError(err instanceof Error ? err.message : String(err));
await exitWith(1);
}
Expand Down
1 change: 1 addition & 0 deletions src/commands/adr/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export function registerAdrListCommand(adr: Command) {
);
}
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
logError(err instanceof Error ? err.message : String(err));
await exitWith(1);
}
Expand Down
1 change: 1 addition & 0 deletions src/commands/adr/show.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export function registerAdrShowCommand(adr: Command) {
const content = await Bun.file(adr.filePath).text();
console.log(content);
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
logError(err instanceof Error ? err.message : String(err));
await exitWith(1);
}
Expand Down
1 change: 1 addition & 0 deletions src/commands/adr/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export function registerAdrUpdateCommand(adr: Command) {
console.log(`Updated ADR: ${result.filePath}`);
}
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
const message = err instanceof Error ? err.message : String(err);
logError(message);
await exitWith(1);
Expand Down
1 change: 1 addition & 0 deletions src/commands/check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export function registerCheckCommand(program: Command) {
try {
loadResults = await loadRuleAdrs(projectRoot, opts.adr);
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
logError(
err instanceof Error ? err.message : `Failed to load rules: ${err}`
);
Expand Down
2 changes: 2 additions & 0 deletions src/commands/clean.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ export function registerCleanCommand(program: Command) {
console.log(`${destinationPath} cleaned up`);
}
} catch (error) {
if (error instanceof Error && error.name === "ExitPromptError")
throw error;
logError(
`Failed to clean ${destinationPath}.`,
error instanceof Error ? error.message : String(error)
Expand Down
1 change: 1 addition & 0 deletions src/commands/doctor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ export function registerDoctorCommand(program: Command) {
printConsole(report);
}
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
logError(err instanceof Error ? err.message : String(err));
await exitWith(1);
}
Expand Down
6 changes: 5 additions & 1 deletion src/commands/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { styleText } from "node:util";

import type { Command } from "@commander-js/extra-typings";
import { Option } from "@commander-js/extra-typings";
import inquirer from "inquirer";

import { loadCredentials } from "../helpers/credential-store";
import { detectEditors, promptEditorSelection } from "../helpers/editor-detect";
Expand Down Expand Up @@ -83,6 +82,9 @@ export function registerInitCommand(program: Command) {
opts.installPlugin === undefined &&
process.stdin.isTTY
) {
// Lazy-load inquirer — it costs ~200ms to parse and is only needed
// for interactive prompts, not for scripted or --help invocations.
const { default: inquirer } = await import("inquirer");
const { wantPlugin } = await inquirer.prompt([
{
type: "confirm",
Expand Down Expand Up @@ -179,6 +181,8 @@ export function registerInitCommand(program: Command) {
: {}),
});
} catch (err) {
// Re-throw ExitPromptError so main().catch() handles Ctrl+C (exit 130)
if (err instanceof Error && err.name === "ExitPromptError") throw err;
if (isTlsError(err)) {
logError(tlsHintMessage());
await exitWith(1);
Expand Down
4 changes: 4 additions & 0 deletions src/commands/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export function registerLoginCommand(program: Command) {
await exitWith(1);
}
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
const failureReason = isTlsError(err) ? "tls" : "other";
trackLoginResult({
subcommand: "login",
Expand Down Expand Up @@ -63,6 +64,7 @@ export function registerLoginCommand(program: Command) {
console.log("Not logged in. Run `archgate login` to authenticate.");
}
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
logError(err instanceof Error ? err.message : String(err));
await exitWith(1);
}
Expand All @@ -77,6 +79,7 @@ export function registerLoginCommand(program: Command) {
trackLoginResult({ subcommand: "logout", success: true });
console.log("Logged out successfully.");
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
logError(err instanceof Error ? err.message : String(err));
await exitWith(1);
}
Expand All @@ -96,6 +99,7 @@ export function registerLoginCommand(program: Command) {
await exitWith(1);
}
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
const failureReason = isTlsError(err) ? "tls" : "other";
trackLoginResult({
subcommand: "refresh",
Expand Down
1 change: 1 addition & 0 deletions src/commands/plugin/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export function registerPluginUrlCommand(plugin: Command) {

console.log(url);
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
logError(err instanceof Error ? err.message : String(err));
await exitWith(1);
}
Expand Down
1 change: 1 addition & 0 deletions src/commands/review-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export function registerReviewContextCommand(program: Command) {

console.log(formatJSON(context));
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
logError(err instanceof Error ? err.message : String(err));
await exitWith(1);
}
Expand Down
1 change: 1 addition & 0 deletions src/commands/session-context/claude-code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export function registerClaudeCodeSessionContextCommand(parent: Command) {

console.log(formatJSON(result.data));
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
logError(err instanceof Error ? err.message : String(err));
await exitWith(1);
}
Expand Down
1 change: 1 addition & 0 deletions src/commands/session-context/copilot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export function registerCopilotSessionContextCommand(parent: Command) {

console.log(formatJSON(result.data));
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
logError(err instanceof Error ? err.message : String(err));
await exitWith(1);
}
Expand Down
1 change: 1 addition & 0 deletions src/commands/session-context/cursor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export function registerCursorSessionContextCommand(parent: Command) {

console.log(formatJSON(result.data));
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
logError(err instanceof Error ? err.message : String(err));
await exitWith(1);
}
Expand Down
1 change: 1 addition & 0 deletions src/commands/session-context/opencode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export function registerOpencodeSessionContextCommand(parent: Command) {

console.log(formatJSON(result.data));
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
logError(err instanceof Error ? err.message : String(err));
await exitWith(1);
}
Expand Down
2 changes: 2 additions & 0 deletions src/commands/telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export function registerTelemetryCommand(program: Command) {
"Telemetry enabled. Thank you for helping improve Archgate."
);
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
logError(err instanceof Error ? err.message : String(err));
await exitWith(1);
}
Expand All @@ -86,6 +87,7 @@ export function registerTelemetryCommand(program: Command) {
await setTelemetryEnabled(false);
console.log("Telemetry disabled. No usage data will be collected.");
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
logError(err instanceof Error ? err.message : String(err));
await exitWith(1);
}
Expand Down
2 changes: 2 additions & 0 deletions src/commands/upgrade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ async function upgradeBinary(tag: string): Promise<void> {
logDebug("Replacing binary:", process.execPath);
replaceBinary(process.execPath, newBinaryPath);
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
finishDownloadProgress();
logError(
"Failed to upgrade binary.",
Expand Down Expand Up @@ -369,6 +370,7 @@ export function registerUpgradeCommand(program: Command) {

console.log(`Archgate upgraded to ${latestVersion} successfully.`);
} catch (err) {
if (err instanceof Error && err.name === "ExitPromptError") throw err;
trackUpgradeResult({
from_version: "unknown",
to_version: "unknown",
Expand Down
Loading
Loading