test: close phase 3 coverage gaps across telemetry, platform, and the long tail

.archgate/adrs/ARCH-005-testing-standards.md

@@ -20,15 +20,15 @@ Bun's built-in test runner (`bun test`) provides a Jest-compatible API, native T
 
 ## Decision
 
-Use Bun's built-in test runner (`bun test`) for all tests. Test files live in `tests/`, mirroring `src/`; fixtures live in `tests/fixtures/`. Target 95% code coverage, enforced in CI.
+Use Bun's built-in test runner (`bun test`) for all tests. Test files live in `tests/`, mirroring `src/`; fixtures live in `tests/fixtures/`. Target 99.5% code coverage, enforced in CI.
 
 **Key conventions:**
 
 1. **Directory structure mirrors `src/`** — `src/engine/runner.ts` is tested by `tests/engine/runner.test.ts`, so tests are discoverable by convention.
 2. **Fixtures in `tests/fixtures/`** — sample ADR files and mock codebases are shared across suites.
 3. **Temp directories for filesystem tests** — tests that write files use `mkdtemp` for isolation and clean up in `afterEach` or `afterAll`.
 4. **Test file naming** — `<module-name>.test.ts`.
-5. **Coverage target: 95%** — enforced in CI. PRs that drop total line coverage below 95% are blocked by the `Validate Code` gate check.
+5. **Coverage target: 99.5%** — enforced in CI. PRs that drop total line coverage below 99.5% are blocked by the `Validate Code` gate check. A residue is unreachable: Bun emits never-incrementing lcov records for some structural tokens (`} catch {`; on Linux also blank lines, comments, braces).
 6. **Isolation is the test author's job** — Bun runs every test file in one process, so environment writes, `mock.module()` calls, and un-restored spies escape into later files and produce order-dependent flakes. Restore env vars with `restoreEnv()`, mock first-party modules with `spyOn` over an `import * as mod` namespace, and keep every write inside a `mkdtemp` directory.
 7. **Mock `os.homedir()`, never `HOME`** — Bun caches `os.homedir()` on Linux, so a runtime `HOME` override is silently ignored and the code under test resolves the REAL home directory. Env-var overrides remain valid ONLY for code that reads `Bun.env.*` at call time (`vscode-settings.ts`'s `APPDATA` branch, the `paths.ts` helpers documented as "resolved at call time"). Production code MUST NOT be rewritten to read `Bun.env.HOME` just to make an env override work.
 8. **Per-test timeouts only ever raise the global** — `bun run test` applies `--timeout 60000`, so a shorter override such as `}, 30_000` makes that test _more_ likely to time out, not less.
@@ -144,6 +144,50 @@ afterEach(() => {
 });
 ```
 
+### Simulating the platform instead of adding a seam
+
+`process.platform` and `process.execPath` are writable, configurable data
+properties in Bun. Overriding them covers platform-gated branches without
+adding a testability seam to production code.
+
+This is the only approach that moves the coverage aggregate: CI merges Linux
+and Windows runs only, so a `test.skipIf(platform !== "darwin")` test
+contributes to neither and covers nothing. Parametrize over the matrix
+(ARCH-025) so every case runs on every runner.
+
+```typescript
+// tests/helpers/vscode-settings.test.ts
+import { describe, it, expect, afterEach } from "bun:test";
+
+import { _resetAllCaches } from "../../src/helpers/platform";
+import { getVscodeUserSettingsPath } from "../../src/helpers/vscode-settings";
+
+const original = Object.getOwnPropertyDescriptor(process, "platform");
+
+afterEach(() => {
+  // GOOD: restore the captured descriptor — an override left in place leaks
+  // into every later file, since Bun runs the whole suite in one process.
+  if (original) Object.defineProperty(process, "platform", original);
+  _resetAllCaches();
+});
+
+describe.each([
+  ["win32", "AppData"],
+  ["darwin", "Library"],
+  ["linux", ".config"],
+])("on %s", (platform, expected) => {
+  it("resolves the user settings path", async () => {
+    Object.defineProperty(process, "platform", {
+      ...original,
+      value: platform,
+    });
+    // Clear whatever the module cached from the previous value.
+    _resetAllCaches();
+    expect(await getVscodeUserSettingsPath()).toContain(expected);
+  });
+});
+```
+
 ## Consequences
 
 ### Positive
@@ -166,7 +210,7 @@ afterEach(() => {
 - **Bun test runner API changes** — newer APIs may still evolve between minor versions.
   - **Mitigation:** the project pins a Bun version via `.prototools`; API changes surface during controlled upgrades with full suite validation.
 - **Coverage reporting gaps** — `bun test --coverage` may misreport code paths, especially dynamically imported modules.
-  - **Mitigation:** the 95% threshold is enforced on total line coverage, not per-file, and critical modules (engine, formats) are tested thoroughly regardless of the aggregate.
+  - **Mitigation:** the 99.5% threshold is enforced on total line coverage, not per-file, and critical modules (engine, formats) are tested thoroughly regardless of the aggregate.
 - **Cross-file pollution from shared process state** — leaked env vars, leaked spies, and writes to real user-scope paths (`~/.config/Code/User/settings.json`, `%APPDATA%`, `~/.cursor/`, `~/.config/opencode/`) produce order-dependent flakes that pass on a PR run and fail after merge with identical code. `Bun.env.NODE_ENV` left unset instead of set to `"test"` before Sentry initializes is the same class of leak — the SDK sets `enabled: Bun.env.NODE_ENV !== "test"`.
   - **Mitigation:** `restoreEnv()` for every env capture, `try/finally` around inline spies, and an `os.homedir()` spy that keeps writes inside a `mkdtemp` directory; `test-isolation/no-bare-env-restore` blocks the env variant at lint time.
 - **Platform-specific hangs and timeouts** — an external SDK instance left open keeps Bun's event loop alive on Linux and hangs `bun test` after every test passes, while slow Windows CI filesystems let large fixtures blow the per-test timeout and kill the staging subprocess (`git add . failed (exit 143)`, where 143 = 128 + SIGTERM). Neither reproduces on macOS or locally.
@@ -182,7 +226,7 @@ afterEach(() => {
 - **oxlint plugin** `test-mocking/no-first-party-module-mock` (`lint/no-first-party-module-mock.ts`): enabled for `tests/**/*.test.ts`, it fails the build for any `mock.module()` whose specifier is relative and carries a `src` path segment, while leaving third-party specifiers (`inquirer`, `node:readline`) alone. oxlint is the right layer for this Don't: the call is syntax-detectable from its specifier, and because `mock.module` is process-global and retroactive, an instance in one file corrupts files that never mention it — a defect no file-by-file review can see. Each plugin file MUST declare a `meta.name` no other plugin uses; a duplicate name silently drops the later file's rules, and oxlint then rejects the config with "Rule not found in plugin".
 - All three plugins are registered via `jsPlugins` in `.oxlintrc.json` and run as part of `bun run lint` (and therefore `bun run validate` and CI).
 - **CI pipeline**: every pull request runs `bun run validate:coverage`, which reaches the suite through the `test:coverage` script (`bun test --timeout 60000 --coverage`). Invoke the suite by script name (GEN-003) — a bare `bun test` applies Bun's 5-second default instead of the 60-second global and reports timeouts that the gate never sees. Test failures and per-test timeouts block merge, and all workflow jobs set `timeout-minutes` to prevent indefinite hangs.
-- **Coverage threshold**: the `Coverage Report` job enforces a 95% minimum line coverage; below that it fails and the `Validate Code` gate blocks the PR.
+- **Coverage threshold**: the `Coverage Report` job enforces a 99.5% minimum line coverage; below that it fails and the `Validate Code` gate blocks the PR.
 
 ### Manual Enforcement
 

.archgate/adrs/ARCH-009-platform-detection-helper.md

@@ -11,12 +11,12 @@ files:
 
 The Archgate CLI runs on macOS, Linux, and Windows (including WSL). Platform-specific behavior appears throughout the codebase: shell syntax in user-facing messages, path separators, subprocess resolution, and feature availability checks.
 
-The `src/helpers/platform.ts` module already provides a centralized, cached API for platform detection (`isWindows()`, `isMacOS()`, `isLinux()`, `isWSL()`, `getPlatformInfo()`). It also exposes a `_resetPlatformCache()` function that allows tests to simulate different platforms without mocking `process.platform` directly.
+The `src/helpers/platform.ts` module already provides a centralized, cached API for platform detection (`isWindows()`, `isMacOS()`, `isLinux()`, `isWSL()`, `getPlatformInfo()`). It also exposes a `_resetAllCaches()` function that allows tests to simulate different platforms without mocking `process.platform` directly.
 
 The problem is that nothing prevents code from bypassing this module and reading `process.platform` directly. Direct reads:
 
 - **Scatter platform logic** — Platform checks end up duplicated across modules with inconsistent patterns (`process.platform === "win32"` vs `process.platform !== "linux"`).
-- **Cannot be tested** — `process.platform` is read-only in Bun. Code that reads it directly cannot be tested under a different platform without modifying global state. The platform helper's `_resetPlatformCache()` makes cross-platform testing straightforward.
+- **Multiply the test surface** — `process.platform` is a writable, configurable data property in Bun, so simulating a platform means overriding it AND clearing whatever the reading module cached from it. Routed through the helper, a single seam (`_resetAllCaches()`) covers every consumer at once; read directly, each call site needs its own override and its own cache reset, and a site that captures the value at module load cannot be re-simulated at all.
 - **Miss WSL** — `process.platform` returns `"linux"` inside WSL. Code that checks for `"win32"` to decide Windows-specific behavior will miss WSL scenarios where Windows paths or tools are relevant. The platform helper accounts for WSL.
 
 ## Decision
@@ -40,7 +40,7 @@ This does NOT cover:
 ### Do
 
 - **DO** import from `src/helpers/platform.ts` for any platform check: `isWindows()`, `isMacOS()`, `isLinux()`, `isWSL()`, `getPlatformInfo()`
-- **DO** use `_resetPlatformCache()` in tests to simulate different platforms
+- **DO** use `_resetAllCaches()` in tests to simulate different platforms
 - **DO** consider WSL when implementing Windows-specific behavior — `isWSL()` returns true when running Linux inside WSL, where Windows tools may still be relevant
 
 ### Don't
@@ -54,7 +54,7 @@ This does NOT cover:
 ### Positive
 
 - **Single source of truth** — All platform detection flows through one module with consistent caching and WSL awareness.
-- **Testable** — Cross-platform behavior can be tested on any OS via `_resetPlatformCache()`.
+- **Testable** — Cross-platform behavior can be tested on any OS via `_resetAllCaches()`.
 - **WSL-safe** — The helper correctly distinguishes native Linux from WSL, preventing subtle bugs.
 
 ### Negative

.claude/agent-memory/archgate-developer/MEMORY.md

@@ -20,7 +20,7 @@ Exceptions: minor follow-up tweaks after validation already passed, and non-code
 - **`archgate check` emits non-blocking diagnostics** alongside rule failures: `[suppression]`, `[briefing]`, and `[adr]` lines are advisories that never affect `pass` by default — but `--strict` (or `.archgate/config.json`'s `strict: true`) elevates all three into failures, per ARCH-026. Check which kind you have, and whether strict mode is active, before treating a finding as a blocker.
 - **Splitting a test file for `oxlint`'s 500-line `max-lines` cap: add a sibling `<name>-<suffix>.test.ts`, don't trim coverage.** Precedent already established by `check-max-warnings.test.ts` as a sibling of `check.test.ts`; followed again for `reporter-strict.test.ts`, `sync-strict.test.ts`, and the `*-strict.test.ts` integration files when ARCH-026's tests pushed their parent files over the cap.
 - **A cache-busting dynamic import hides coverage: `await import(`../mod?t=${Date.now()}`)` makes Bun load a second module instance whose execution is attributed to nothing.** The source file then reports its paths uncovered while they are in fact tested, so the reporter understates and the "gap" is an illusion — check for this specifier before writing tests for any file that looks mysteriously uncovered. Removing it took `update-check.ts` from 76.47% to 100% with zero new tests. A static import is safe only when the module holds no mutable module-level state; verify that first (file-backed caches and `Bun.env` reads at call time are fine).
-- **Reproduce CI's coverage number yourself — bun's `All files` summary line is not it.** CI filters the merged lcov to `src/*` and computes `sum(LH)/sum(LF)`; bun's own table includes `tests/` and averages differently, so the two disagree by several points. Locally: `awk -F: '/^SF:/{p=($2~/^src[\\\/]/)} /^LF:/{if(p)f+=$2} /^LH:/{if(p)h+=$2} END{printf "%.2f\n",h/f*100}' coverage/lcov.info`. A single-platform local run is a floor, not the CI figure — CI unions Linux and Windows and counts a line covered if either platform hit it, and the gap is large enough to send you chasing phantom work (`platform.ts` reads as 64 missed on Windows alone, 12 merged). To get the real per-file picture, `gh run download <run-id> -n coverage-linux -D a -n coverage-windows -D b`, then union the `DA:<line>,<hits>` records across both files keyed by the path from `src/` onward and count a line covered when the summed hits exceed zero; that reproduces the number the PR comment reports, digit for digit.
+- **Reproduce CI's coverage number yourself — bun's `All files` summary line is not it.** CI filters the merged lcov to `src/*` and computes `sum(LH)/sum(LF)`; bun's own table includes `tests/` and averages differently, so the two disagree by several points. Locally: `awk -F: '/^SF:/{p=($2~/^src[\\\/]/)} /^LF:/{if(p)f+=$2} /^LH:/{if(p)h+=$2} END{printf "%.2f\n",h/f*100}' coverage/lcov.info`. A single-platform local run is a floor, not the CI figure — CI unions Linux and Windows and counts a line covered if either platform hit it, and the gap is large enough to send you chasing phantom work (`platform.ts` reads as 64 missed on Windows alone, 12 merged). To get the real per-file picture, `gh run download <run-id> -R archgate/cli -n coverage-linux -n coverage-windows -D cov` (`-n` repeats, `-D` does not — a second `-D` silently overrides the first, so both artifacts land under one root as `cov/coverage-linux/lcov.info` and `cov/coverage-windows/lcov.info`), then union the `DA:<line>,<hits>` records across both files keyed by the path from `src/` onward and count a line covered when the summed hits exceed zero; that reproduces the number the PR comment reports, digit for digit. Note `bun test --coverage-dir` is silently ignored — `bunfig.toml`'s `[test] coverageDir` wins — so every concurrent run clobbers the same `coverage/lcov.info`; isolate a run with a private config (`bun --config=<file>`) or read the per-file numbers off the text reporter's stdout instead. The text reporter is not equivalent to the lcov: it omits closing-brace lines from its "Uncovered Line #s" column while still counting them against the percentage, so a file can show a blank column at under 100%.
 - **`--update-snapshots` is never on its own the fix for a failing snapshot.** The repo's snapshot (`tests/helpers/__snapshots__/rules-shim.test.ts.snap`) is the entire `rules.d.ts` a governed project receives, and its diff is the review artifact — read every hunk and confirm it follows from an intended `src/formats/rules.ts` edit before regenerating. Deleting the file routes around nothing: Bun fails a missing snapshot whenever `CI` is set, and passes it locally.
 - **Commit with `--signoff`** — the DCO check rejects commits without `Signed-off-by`.
 - **This repo is PUBLIC** — no private sibling-repo internals, no Claude session links in PRs or commits.

.github/workflows/code-pull-request.yml

@@ -274,7 +274,7 @@ jobs:
         id: coverage-report
         uses: ./.github/actions/coverage-report
         with:
-          min-coverage: "95"
+          min-coverage: "99.5"
           github-token: ${{ github.token }}
           event-name: ${{ github.event_name }}
           pr-number: ${{ github.event.pull_request.number }}
@@ -294,7 +294,7 @@ jobs:
           # (zizmor: code injection via template expansion).
           COVERAGE: ${{ steps.coverage-report.outputs.coverage }}
         run: |
-          MIN_COVERAGE=95
+          MIN_COVERAGE=99.5
           # Pass values as awk data (-v), never interpolated into the awk
           # program source — a crafted step output could inject awk code.
           BELOW=$(awk -v coverage="$COVERAGE" -v minimum="$MIN_COVERAGE" \

tests/commands/adr/domain/add.test.ts

@@ -18,6 +18,12 @@ import { z } from "zod";
 
 import { registerDomainCommand } from "../../../../src/commands/adr/domain/index";
 
+const AddResultSchema = z.object({
+  domain: z.string(),
+  prefix: z.string(),
+  added: z.boolean(),
+});
+
 const DomainEntrySchema = z.object({
   domain: z.string(),
   prefix: z.string(),
@@ -82,6 +88,25 @@ describe("adr domain add", () => {
     expect(sec?.source).toBe("custom");
   });
 
+  test("--json prints the registered domain as a machine-readable payload", async () => {
+    const program = makeProgram();
+    await program.parseAsync([
+      "node",
+      "adr",
+      "domain",
+      "add",
+      "security",
+      "SEC",
+      "--json",
+    ]);
+
+    const raw = logSpy.mock.calls.map((c) => String(c[0])).join("");
+    const parsed = AddResultSchema.parse(JSON.parse(raw));
+    expect(parsed).toEqual({ domain: "security", prefix: "SEC", added: true });
+    // `--json` requests pretty-printed output regardless of agent context.
+    expect(raw).toContain("\n");
+  });
+
   test("rejects built-in names", async () => {
     const program = makeProgram();
     expect(

tests/commands/adr/domain/list.test.ts

@@ -16,6 +16,7 @@ import { join } from "node:path";
 import { Command } from "@commander-js/extra-typings";
 
 import { registerDomainCommand } from "../../../../src/commands/adr/domain/index";
+import * as projectConfig from "../../../../src/helpers/project-config";
 
 function makeProgram(): Command {
   const adr = new Command("adr").exitOverride();
@@ -57,4 +58,28 @@ describe("adr domain list", () => {
     expect(out).toContain("backend");
     expect(out).toContain("default");
   });
+
+  test("routes a config-read failure through the command error boundary", async () => {
+    const entriesSpy = spyOn(
+      projectConfig,
+      "listDomainEntries"
+    ).mockImplementation(() => {
+      throw new Error("config.json is unreadable");
+    });
+    try {
+      const program = makeProgram();
+      expect(
+        program.parseAsync(["node", "adr", "domain", "list"])
+      ).rejects.toThrow("process.exit");
+
+      // An unexpected failure is a bug, not user error → exit 2.
+      expect(exitSpy.mock.calls.at(-1)?.[0]).toBe(2);
+      const errOut = errorSpy.mock.calls
+        .map((c: unknown[]) => c.map(String).join(" "))
+        .join("\n");
+      expect(errOut).toContain("config.json is unreadable");
+    } finally {
+      entriesSpy.mockRestore();
+    }
+  });
 });

tests/commands/adr/list.test.ts

@@ -247,6 +247,21 @@ describe("adr list action handler", () => {
     });
   });
 
+  test("prints 'No ADRs found.' when only .archgate/lint/ marks the project root", async () => {
+    // `.archgate/lint/` alone identifies a project root, so the command runs
+    // with an adrs directory that was never created.
+    mkdirSync(join(tempDir, ".archgate", "lint"), { recursive: true });
+
+    const parent = makeProgram();
+    await parent.parseAsync(["node", "adr", "list"]);
+
+    const allOutput = logSpy.mock.calls
+      .map((c: unknown[]) => String(c[0]))
+      .join("\n");
+    expect(allOutput).toContain("No ADRs found.");
+    expect(exitSpy).not.toHaveBeenCalled();
+  });
+
   test("exits with error when .archgate/ directory is missing", async () => {
     const parent = makeProgram();