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
74 changes: 74 additions & 0 deletions .archgate/adrs/ARCH-011-consistent-project-root-resolution.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
---
id: ARCH-011
title: Consistent Project Root Resolution
domain: architecture
rules: true
files: ["src/commands/**/*.ts"]
---

# Consistent Project Root Resolution

## Context

Commands that operate on project-level resources (ADRs, rules, `.archgate/` config) need to locate the project root directory. Inconsistent strategies for finding the project root cause different behavior depending on the user's working directory:

- Commands using `findProjectRoot()` (walks up from cwd to find `.archgate/adrs/`) work correctly from any subdirectory
- Commands using `process.cwd()` directly fail when the user is in a subdirectory because they assume cwd IS the project root

This inconsistency was discovered during a repository-wide consistency review where `archgate check` worked from subdirectories but `archgate adr list` did not.

**Alternatives considered:**

- **Always use `process.cwd()`** — Simplest, but breaks when the user is in a subdirectory of the project. This is a common workflow (e.g., running `archgate adr list` while editing files in `src/`).
- **Require users to run from the project root** — Adds friction and goes against CLI conventions (git, npm, etc. all resolve upward).
- **Walk up from cwd to find `.archgate/adrs/`** — Standard convention used by git, npm, and other project-aware CLIs. Already implemented as `findProjectRoot()` in `src/helpers/paths.ts`.

## Decision

All commands that operate on `.archgate/` project resources MUST use `findProjectRoot()` from `src/helpers/paths.ts` to locate the project root. Direct use of `process.cwd()` for project root resolution in command files is prohibited.

**Exceptions:**

- `archgate init` — Creates the `.archgate/` directory; uses `process.cwd()` because no project root exists yet
- `archgate upgrade` — Operates on the binary, not on a project; its `findPackageRoot()` walks up from the binary path to find `package.json` for local install detection (a different concern than project root)
- Commands that don't require a project (e.g., `clean`, `login`) are not affected

## Do's and Don'ts

### Do

- Use `findProjectRoot()` from `src/helpers/paths.ts` in all commands that read from `.archgate/`
- Check the return value for `null` and exit with a helpful error message
- Pass the resolved `projectRoot` to `projectPaths()` for derived paths

### Don't

- Don't use `process.cwd()` to locate `.archgate/` in command files (except `init`)
- Don't define local `findProjectRoot()` variants — use the shared implementation
- Don't assume the user is running from the project root

## Consequences

### Positive

- All commands work consistently regardless of the user's working directory
- Matches user expectations from git, npm, and other project-aware CLIs

### Negative

- Slightly more verbose command setup (null check on `findProjectRoot()`)

## Compliance and Enforcement

### Automated Enforcement

- **Archgate rule** `ARCH-011/no-process-cwd-for-project-root`: Scans command files for `process.cwd()` usage and flags violations. The `init` command is exempt. Severity: `error`.

### Manual Enforcement

Code reviewers MUST verify that new commands use `findProjectRoot()` for project-aware operations.

## References

- [ARCH-001 — Command Structure](./ARCH-001-command-structure.md) — Commands handle I/O only
- [ARCH-002 — Error Handling](./ARCH-002-error-handling.md) — Exit with code 1 when project not found
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/// <reference path="../rules.d.ts" />

export default {
rules: {
"no-process-cwd-for-project-root": {
description:
"Command files must use findProjectRoot() instead of process.cwd() for project root resolution",
severity: "error",
async check(ctx) {
// init.ts is exempt — it creates the project, so no root exists yet
const files = ctx.scopedFiles.filter(
(f) =>
f.includes("commands/") &&
!f.endsWith("init.ts") &&
!f.endsWith("index.ts")
);

const checks = files.map(async (file) => {
const matches = await ctx.grep(file, /process\.cwd\(\)/);
for (const m of matches) {
// Allow process.cwd() as a fallback after findProjectRoot()
// e.g. findProjectRoot() ?? process.cwd()
if (m.content.includes("findProjectRoot")) continue;

ctx.report.violation({
message:
"Use findProjectRoot() from helpers/paths.ts instead of process.cwd() for project root resolution",
file: m.file,
line: m.line,
fix: "Import { findProjectRoot } from '../helpers/paths' and use findProjectRoot()",
});
}
});
await Promise.all(checks);
},
},
},
} satisfies RuleSet;
92 changes: 92 additions & 0 deletions .archgate/adrs/ARCH-012-command-error-boundaries.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
---
id: ARCH-012
title: Command Error Boundaries
domain: architecture
rules: true
files: ["src/commands/**/*.ts"]
---

# Command Error Boundaries

## Context

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
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

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

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.

## Decision

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

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.

**Pattern:**

```typescript
.action(async (opts) => {
try {
// command logic
} catch (err) {
logError(err instanceof Error ? err.message : String(err));
process.exit(1);
}
});
```

**Exceptions:**

- Synchronous command actions (e.g., `clean`) that cannot throw async errors
- Command group index files that only register subcommands

## Do's and Don'ts

### Do

- Wrap every async command action body in a try-catch
- Use `logError()` for error messages in the catch block
- Exit with code 1 for expected failures

### Don't

- Don't rely on `main().catch()` as the only error handler for commands
- 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

## Consequences

### Positive

- Users see contextual error messages instead of raw exception text
- Exit code 1 vs 2 distinction is preserved — scripts and CI can differentiate
- Every command handles its own failures gracefully

### Negative

- Minor boilerplate in every async command action

## Compliance and Enforcement

### 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).

### Manual Enforcement

Code reviewers MUST verify that new async commands include error boundaries.

## References

- [ARCH-002 — Error Handling](./ARCH-002-error-handling.md) — Exit code convention and logError() requirement
- [ARCH-001 — Command Structure](./ARCH-001-command-structure.md) — Command file conventions
41 changes: 41 additions & 0 deletions .archgate/adrs/ARCH-012-command-error-boundaries.rules.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/// <reference path="../rules.d.ts" />

export default {
rules: {
"async-action-error-boundary": {
description:
"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")
);

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

// Find async action callbacks
const hasAsyncAction = /\.action\(\s*async\s/.test(content);
if (!hasAsyncAction) return;

// Check if the async action body contains a try block
// Match: .action(async (...) => { ... try { ... } ... })
const hasTryCatch = /\.action\(\s*async\s[\s\S]*?\btry\s*\{/.test(
content
);

if (!hasTryCatch) {
ctx.report.warning({
message:
"Async command action should include a try-catch error boundary",
file,
fix: "Wrap the action body in try { ... } catch (err) { logError(...); process.exit(1); }",
});
}
});
await Promise.all(checks);
},
},
},
} satisfies RuleSet;
64 changes: 64 additions & 0 deletions .archgate/adrs/ARCH-013-version-synchronization.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
---
id: ARCH-013
title: Version Synchronization
domain: architecture
rules: true
files: ["package.json", "docs/**"]
---

# Version Synchronization

## Context

The CLI version appears in multiple locations that must stay in sync:

1. `package.json` `version` — canonical source of truth
2. `package.json` `optionalDependencies` — platform-specific npm packages (`archgate-darwin-arm64`, `archgate-linux-x64`, `archgate-win32-x64`) must match the CLI version
3. `docs/astro.config.mjs` — `softwareVersion` in the JSON-LD structured data

When versions diverge, npm installs pull mismatched platform binaries and search engines display outdated version info. This was discovered during a consistency review where `package.json` was at `0.16.0` but `docs/astro.config.mjs` was still at `0.11.0`.

## Decision

`package.json` `version` is the single source of truth. All other version references MUST match it.

**Automated via release process:** The `.simple-release.js` bump hook already syncs `optionalDependencies` versions during the release workflow — it reads `package.json` after the version bump and updates all `optionalDependencies` entries to match. This is fully automated and requires no manual intervention.

**Automated via release process:** The `.simple-release.js` bump hook also updates `softwareVersion` in `docs/astro.config.mjs` to match `package.json`. Both syncs are fully automated.

## Do's and Don'ts

### Do

- Rely on `.simple-release.js` for both `optionalDependencies` and `softwareVersion` sync (do not update manually)
- Use the companion rules to catch version drift in CI as a safety net

### Don't

- Don't manually edit `optionalDependencies` versions — the release hook handles this
- Don't manually edit `softwareVersion` in `docs/astro.config.mjs` — the release hook handles this

## Consequences

### Positive

- Consistent version information across npm packages and user-facing surfaces
- CI catches version drift before it reaches production
- Platform-specific npm packages always match the CLI version

### Negative

- None — all version sync is automated via the release hook

## Compliance and Enforcement

### Automated Enforcement

- **Release hook** `.simple-release.js`: Syncs `optionalDependencies` versions and `docs/astro.config.mjs` `softwareVersion` during `bump()`. Fully automated.
- **Archgate rule** `ARCH-013/docs-version-sync`: Checks that `softwareVersion` in `docs/astro.config.mjs` matches `package.json` version. Severity: `error`.
- **Archgate rule** `ARCH-013/optional-deps-version-sync`: Checks that all `optionalDependencies` versions match `package.json` version. Severity: `error`.

## References

- [GEN-001 — Documentation Site](./GEN-001-documentation-site.md) — Docs site structure and configuration
- [`.simple-release.js`](../../.simple-release.js) — Release bump hook that syncs optionalDependencies
61 changes: 61 additions & 0 deletions .archgate/adrs/ARCH-013-version-synchronization.rules.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/// <reference path="../rules.d.ts" />

export default {
rules: {
"docs-version-sync": {
description:
"softwareVersion in docs/astro.config.mjs must match package.json version",
severity: "error",
async check(ctx) {
const pkgJson = (await ctx.readJSON("package.json")) as {
version?: string;
};
if (!pkgJson.version) return;

let astroConfig: string;
try {
astroConfig = await ctx.readFile("docs/astro.config.mjs");
} catch {
// docs/astro.config.mjs may not exist in all contexts
return;
}

const match = astroConfig.match(/softwareVersion:\s*"([^"]+)"/);
if (!match) return;

const docsVersion = match[1];
if (docsVersion !== pkgJson.version) {
ctx.report.violation({
message: `docs/astro.config.mjs softwareVersion "${docsVersion}" does not match package.json version "${pkgJson.version}"`,
file: "docs/astro.config.mjs",
fix: `Update softwareVersion to "${pkgJson.version}" in docs/astro.config.mjs`,
});
}
},
},
"optional-deps-version-sync": {
description:
"optionalDependencies versions must match package.json version",
severity: "error",
async check(ctx) {
const pkgJson = (await ctx.readJSON("package.json")) as {
version?: string;
optionalDependencies?: Record<string, string>;
};
if (!pkgJson.version || !pkgJson.optionalDependencies) return;

for (const [dep, depVersion] of Object.entries(
pkgJson.optionalDependencies
)) {
if (depVersion !== pkgJson.version) {
ctx.report.violation({
message: `optionalDependencies "${dep}" version "${depVersion}" does not match package.json version "${pkgJson.version}"`,
file: "package.json",
fix: `Update ${dep} to "${pkgJson.version}" in optionalDependencies (normally handled by .simple-release.js during release)`,
});
}
}
},
},
},
} satisfies RuleSet;
Loading
Loading