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
29 changes: 9 additions & 20 deletions .archgate/adrs/ARCH-004-no-barrel-files.rules.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
/// <reference path="../rules.d.ts" />

/**
* A Program body is barrel-shaped when every top-level statement is purely
* import/re-export plumbing:
* - ImportDeclaration — `import { x } from "./y"` / `import "./y"`
* - ExportAllDeclaration — `export * from "./y"`
* - ExportNamedDeclaration with — `export { x } from "./y"` / `export { x }`
* declaration === null
*
* Anything else — `export const x = ...`, `export default ...`, function or
* class declarations, expression statements — is executable logic, so the
* file is not a barrel.
* A Program body is barrel-shaped when every top-level statement is pure
* import/re-export plumbing: ImportDeclaration, ExportAllDeclaration, or
* ExportNamedDeclaration with a null declaration. Anything else (`export
* const`, `export default`, function/class declarations, expression
* statements) is executable logic, so the file is not a barrel.
*/
function isReExportOnlyBody(body: EsTreeNode[]): boolean {
return body.every((node) => {
Expand All @@ -25,16 +20,10 @@ function isReExportOnlyBody(body: EsTreeNode[]): boolean {

/**
* Fallback for files whose transpiled Program body is EMPTY: ctx.ast()
* transpiles TypeScript before parsing, which erases type-only syntax
* (`export type { X } from "./y"`, `import type ...`), so a pure
* type-re-export barrel parses to an empty Program. Conservatively inspect
* the source: strip comments and blank space, then require every remaining
* statement to start with `import` or `export`. A comment-only/empty file
* is not a barrel.
*
* Handles multi-line statements (e.g. `export type {\n A,\n} from ...`)
* by tracking brace depth — continuation lines inside `{ }` are part of
* the enclosing import/export, not new statements.
* transpiles first, erasing type-only syntax, so a pure type-re-export
* barrel parses to an empty Program. Conservatively require every source
* statement (comments stripped, brace depth tracked across multi-line
* statements) to start with `import` or `export`; empty files are not barrels.
*/
function isTypeOnlyBarrel(source: string): boolean {
const stripped = source
Expand Down
1 change: 0 additions & 1 deletion .archgate/adrs/ARCH-005-testing-standards.rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ export default {
description: "Test directory structure should mirror src/ structure",
severity: "error",
async check(ctx) {
// Get all src modules (non-index, non-cli.ts)
const srcFiles = await ctx.glob("src/**/*.ts");
const testFiles = await ctx.glob("tests/**/*.test.ts");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ export default {
(f) => !f.includes("tests/") && !f.includes(".archgate/")
);

// Check for Bun.$ template literal usage
const bunShellMatches = await Promise.all(
files.map((file) => ctx.grep(file, /Bun\.\$`/u))
);
Expand All @@ -26,7 +25,6 @@ export default {
}
}

// Check for $ import from "bun" (the shell API)
const dollarImportMatches = await Promise.all(
files.map((file) =>
ctx.grep(file, /import\s*\{[^}]*\$[^}]*\}\s*from\s*["']bun["']/u)
Expand All @@ -44,7 +42,6 @@ export default {
}
}

// Check for await $` pattern (destructured $ usage)
const destructuredMatches = await Promise.all(
files.map((file) => ctx.grep(file, /await\s+\$`/u))
);
Expand Down
27 changes: 10 additions & 17 deletions .archgate/adrs/ARCH-008-typed-command-options.rules.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,17 @@
/// <reference path="../rules.d.ts" />

/**
* ARCH-008 enforcement, rewritten on top of ctx.ast() (ARCH-022).
*
* The previous implementation grepped single lines for `.option(...)` and
* missed any call formatted across multiple lines — exactly the fragility
* ARCH-022 was introduced to fix. These rules now walk the ESTree produced
* by ctx.ast(file, "typescript") and inspect real CallExpression arguments,
* so formatting, whitespace, and string escaping no longer matter.
* ARCH-008 enforcement on top of ctx.ast() (ARCH-022): rules walk the ESTree
* produced by ctx.ast(file, "typescript") and inspect real CallExpression
* arguments, so formatting, whitespace, and string escaping do not matter —
* multi-line `.option(...)` calls match the same as single-line ones.
*/

/**
* Description strings that enumerate a fixed set of values, e.g.
* "editor integration to configure (claude, cursor, vscode, copilot)" or
* "ADR domain: backend, frontend, data, architecture, general".
* Same heuristic as the previous regex rule, but applied to the parsed
* Literal VALUE rather than raw source text.
* Matched against the parsed Literal VALUE, not raw source text.
*/
const CHOICE_ENUMERATION = /(?:claude.*cursor|backend.*frontend)/u;

Expand Down Expand Up @@ -60,14 +56,11 @@ function isStringLiteral(
}

/**
* Locate the 1-based line of an option's flag string in the ORIGINAL source.
*
* ctx.ast(file, "typescript") parses Bun-transpiled output, which reprints
* the module and collapses multi-line calls onto single lines — node.loc
* therefore refers to transpiled lines and is unusable for reporting.
* Searching the untranspiled source for the quoted flag literal gives an
* exact line instead; when the flag can't be found (e.g. built dynamically),
* the violation is reported file-only rather than with a wrong line.
* Locate the 1-based line of an option's flag string in the ORIGINAL source:
* ctx.ast(file, "typescript") parses Bun-transpiled output whose node.loc
* refers to transpiled lines, so the untranspiled source is searched for the
* quoted flag literal instead. A flag that can't be found (e.g. built
* dynamically) is reported file-only rather than with a wrong line.
*/
function findFlagLine(source: string, flag: string): number | undefined {
const needles = [`"${flag}"`, `'${flag}'`, `\`${flag}\``];
Expand Down
2 changes: 2 additions & 0 deletions .archgate/adrs/ARCH-012-command-error-boundaries.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ Async command actions that lack try-catch error boundaries produce poor user exp

This was discovered during a repository-wide review where `review-context`, `session-context claude-code`, and `session-context cursor` all lacked error boundaries.

The failure mode is also documented by incident CLI-5: `src/commands/check.ts` once wrapped only `loadRuleAdrs()` in try-catch, so a `UserError` thrown later by `runChecks()` escaped to `main().catch()`, where it was miscaptured to Sentry and exited with code 2 instead of 1. A boundary that covers only part of the action body fails exactly like no boundary at all — the try-catch MUST span the entire action.

ARCH-002 defines the exit code convention and logging patterns, but does not require error boundaries in command actions. This ADR complements ARCH-002 by making error boundaries mandatory.

**Why not a global Commander.js error handler?** Commander provides `.exitOverride()` and `.configureOutput()` for parsing errors (unknown options, missing arguments), but these do **not** cover errors thrown inside async `.action()` callbacks. Commander's `preAction`/`postAction` hooks could theoretically wrap actions, but they don't catch async errors from the action body. The `main().catch()` in `cli.ts` catches unhandled rejections as a safety net (exit 2), but per-command try-catch is needed to produce contextual error messages and exit with code 1 instead of 2.
Expand Down
19 changes: 5 additions & 14 deletions .archgate/adrs/ARCH-012-command-error-boundaries.rules.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,11 @@
/// <reference path="../rules.d.ts" />

/**
* ARCH-012 enforcement, rewritten on top of ctx.ast() (ARCH-022).
*
* The previous implementation only regex-detected the PRESENCE of a try-catch
* inside an async action. That let partial boundaries pass: src/commands/
* check.ts once wrapped only loadRuleAdrs() in try/catch, and a UserError
* thrown later by runChecks() escaped to main().catch(), where it was
* miscaptured to Sentry with exit 2 (incident CLI-5). These rules now walk
* the ESTree produced by ctx.ast(file, "typescript"): the boundary rule
* additionally flags top-level awaited statements that sit OUTSIDE the
* action's try block — the exact statements whose rejections escape the
* boundary.
* ARCH-012 enforcement on top of ctx.ast() (ARCH-022). A try-catch that
* covers only part of an action is as leaky as none, so the boundary rule
* walks the ESTree and flags top-level awaited statements sitting OUTSIDE
* the action's try block, whose rejections escape to main().catch() and
* miscapture to Sentry. See ARCH-012 for the failure it prevents.
*/

/** Node types whose bodies run in their own context — awaits inside them are
Expand Down Expand Up @@ -153,7 +147,6 @@ export default {
"Async command actions must include try-catch error boundaries",
severity: "warning",
async check(ctx) {
// Only check non-index command files
const files = ctx.scopedFiles.filter(
(f) => f.includes("commands/") && !f.endsWith("index.ts")
);
Expand Down Expand Up @@ -235,15 +228,13 @@ export default {
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*\{/u.test(content);
if (!hasAsyncActionWithTryCatch) return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,10 @@ export default {
"Every top-level CLI command (src/commands/<name>.ts or src/commands/<name>/index.ts) must have a corresponding reference page at docs/src/content/docs/reference/cli/<name>.mdx, and vice versa",
severity: "error",
async check(ctx) {
// Discover top-level command names from src/commands/.
// Per ARCH-001, top-level commands live at either
// src/commands/<name>.ts — single-file command
// src/commands/<name>/index.ts — command group
// Nested files like src/commands/<name>/create.ts or
// src/commands/<name>/<sub>/index.ts are subcommands and NOT
// independent top-level commands.
// Discover top-level command names from src/commands/. Per ARCH-001
// they live at src/commands/<name>.ts (single-file) or
// src/commands/<name>/index.ts (command group); nested files are
// subcommands, not independent top-level commands.
const commandNames = new Set<string>();

const topLevelFiles = await ctx.glob(`${COMMANDS_DIR}/*.ts`);
Expand All @@ -38,7 +35,6 @@ export default {
commandNames.add(name);
}

// Collect docs stems.
const docFiles = await ctx.glob(`${DOCS_DIR}/*.mdx`);
const docStems = new Set<string>();
for (const file of docFiles) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,18 @@ export default {
severity: "error",
async check(ctx) {
// ── 1. Discover subcommand names from src/commands/ ──────────────
//
// Top-level command groups live at src/commands/<parent>/index.ts.
// Direct subcommands are either:
// src/commands/<parent>/<sub>.ts (single-file subcommand)
// src/commands/<parent>/<sub>/index.ts (nested command group)
//
// We only look one level deep: <parent>/<sub>. Files like
// src/commands/adr/domain/add.ts are sub-subcommands of "adr domain"
// and are NOT checked by this rule (they are documented in the
// "adr domain" section as a table, not as separate headings).

// Find all parent command groups (dirs with an index.ts).
// Direct subcommands of a group (src/commands/<parent>/index.ts) are
// <parent>/<sub>.ts or <parent>/<sub>/index.ts. Only one level deep
// is checked: deeper files like adr/domain/add.ts are sub-subcommands
// documented as a table in the parent's section, not as headings.

const groupIndexFiles = await ctx.glob(`${COMMANDS_DIR}/*/index.ts`);

// Extract parent names from index files.
const parentNames = groupIndexFiles.map((indexFile) => {
const rel = indexFile.slice(COMMANDS_DIR.length + 1);
return rel.split("/")[0];
});

// Discover subcommands for all parents in parallel.
const subResults = await Promise.all(
parentNames.map(async (parentName) => {
const [subFiles, nestedGroupFiles] = await Promise.all([
Expand All @@ -41,7 +32,6 @@ export default {

const subs = new Set<string>();

// Single-file subcommands
for (const sf of subFiles) {
const fileName = sf.slice(
`${COMMANDS_DIR}/${parentName}/`.length
Expand All @@ -50,7 +40,6 @@ export default {
subs.add(fileName.slice(0, -".ts".length));
}

// Nested command groups
for (const ngf of nestedGroupFiles) {
const nestedRel = ngf.slice(
`${COMMANDS_DIR}/${parentName}/`.length
Expand All @@ -74,7 +63,6 @@ export default {
// ### archgate adr domain
const headingPattern = /^#{1,4}\s+.*archgate\s+(\S+)\s+(\S+)/giu;

// Read all docs files in parallel.
const docsResults = await Promise.all(
[...subcommandsByParent.entries()].map(
async ([parentName, subNames]) => {
Expand All @@ -91,7 +79,6 @@ export default {
)
);

// Report violations.
for (const {
parentName,
subNames,
Expand All @@ -100,7 +87,6 @@ export default {
} of docsResults) {
if (docsContent === null) continue;

// Extract documented subcommand names from headings
const documentedSubs = new Set<string>();
let match;
headingPattern.lastIndex = 0;
Expand Down
1 change: 1 addition & 0 deletions .archgate/adrs/ARCH-019-inquirer-prompt-fix.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,4 @@ Code reviewers MUST verify any new interactive flow wraps its prompts in `withPr

- [ARCH-018: Lazy-Load Heavy Dependencies](./ARCH-018-lazy-load-heavy-dependencies.md) — `inquirer` is loaded lazily; this ADR governs how its prompts are invoked
- [`src/helpers/prompt.ts`](../../src/helpers/prompt.ts) — defines `withPromptFix()` and the stdout patch
- [Inquirer.js issue #2123](https://github.com/SBoudrias/Inquirer.js/issues/2123) — upstream report of the unrestored Windows console-mode flag
Loading