From 5fafafc33ff3bcaaff4d921cc65271c1575e278a Mon Sep 17 00:00:00 2001 From: Rhuan Barreto Date: Wed, 5 Aug 2026 23:14:52 +0200 Subject: [PATCH 1/4] test: close phase 3 coverage gaps across telemetry, platform, and the long tail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 3 of #522. Covers 333 of the 334 lines the merged Linux+Windows baseline reported uncovered, entirely from `tests/` — no production file is modified. ## Scope - `telemetry.ts` (106) — `posthog-node` is faked via `mock.module`, which makes it safe to lift the `NODE_ENV === "test"` short-circuit in `trackEvent`. That guard was what left the property builders and env detectors dark; with the SDK faked, no event can leave the process. The CI-provider and shell detectors are driven as `test.each` tables through the captured payload, and the constructor's `options.fetch` wrapper is invoked directly to reach the network-failure fallback. - Platform branches (51) — `binary-upgrade.ts`, `vscode-settings.ts`, and `platform.ts`. `getArtifactInfo` is parameterized over the full platform/arch matrix so every combination runs on every runner. - Commands (60), engine and session-context (56), helpers (61) — error paths, `--json` variants, and malformed-input branches. ## `process.platform` is writable in Bun The planned testability seam in `vscode-settings.ts` proved unnecessary. `process.platform` is a writable, configurable data property, so `Object.defineProperty` plus a cache reset reaches every branch of `platform.ts` and every caller of it from any runner, with the descriptor restored per test. This keeps the macOS and WSL paths covered by tests that run everywhere rather than by `skipIf` tests that contribute to neither CI platform. ## One line is not coverable `runner.ts:348` is the `} catch {` token of a branch whose body is already exercised — the pre-existing tests emit `DA:350,1` beside `DA:348,0`. Bun emits a never-incrementing record for a `catch` clause line when the body has its own statements. A test was written to confirm this moved nothing, then removed rather than left behind. The same accounting explains part of the residual elsewhere: on Linux, Bun emits zero-hit records for blank lines, comments, and closing braces that Windows omits entirely, so the union keeps them at zero permanently. All seven lines the baseline attributed to `prompt.ts` are of this kind, which is why that file is untouched. ## Fixed in passing Two `describe` blocks in `binary-upgrade.test.ts` assigned `globalThis.fetch` directly but relied on `mock.restore()`, which does not undo a direct assignment (ARCH-005). Both now save and restore it. Refs #522 Signed-off-by: Rhuan Barreto Signed-off-by: Rhuan Barreto --- tests/commands/adr/domain/add.test.ts | 25 + tests/commands/adr/domain/list.test.ts | 25 + tests/commands/adr/list.test.ts | 15 + tests/commands/adr/update.test.ts | 77 +++ tests/commands/check-action-strict.test.ts | 331 +++++++++++ tests/commands/clean.test.ts | 29 + tests/commands/login.test.ts | 46 ++ tests/commands/plugin/url.test.ts | 36 ++ tests/commands/review-context-strict.test.ts | 120 ++++ .../session-context/claude-code.test.ts | 39 +- tests/commands/upgrade.test.ts | 15 + tests/engine/ast-support-errors.test.ts | 93 ++++ tests/engine/git-files.test.ts | 21 + tests/engine/rule-scanner.test.ts | 22 + tests/engine/runner-error-paths.test.ts | 124 +++++ tests/engine/safe-path.test.ts | 45 +- tests/engine/yaml-utils.test.ts | 20 +- tests/formats/pack.test.ts | 20 + tests/helpers/adr-import-failures.test.ts | 198 +++++++ tests/helpers/adr-writer.test.ts | 25 + tests/helpers/auth.test.ts | 16 +- tests/helpers/binary-upgrade-archive.test.ts | 222 ++++++++ tests/helpers/binary-upgrade-artifact.test.ts | 83 +++ tests/helpers/binary-upgrade.test.ts | 74 +-- tests/helpers/editor-detect.test.ts | 102 +++- tests/helpers/exit.test.ts | 104 +++- tests/helpers/init-project-editors.test.ts | 77 +++ tests/helpers/install-info.test.ts | 83 +++ tests/helpers/log.test.ts | 70 ++- tests/helpers/login-flow.test.ts | 53 +- tests/helpers/pack-recommend.test.ts | 58 ++ tests/helpers/paths.test.ts | 61 +- tests/helpers/platform-simulated.test.ts | 299 ++++++++++ tests/helpers/project-config.test.ts | 55 ++ tests/helpers/repo.test.ts | 41 ++ tests/helpers/sentry.test.ts | 130 ++++- tests/helpers/session-context-copilot.test.ts | 65 ++- tests/helpers/session-context-cursor.test.ts | 34 +- tests/helpers/session-context.test.ts | 77 +++ tests/helpers/stack-detect-errors.test.ts | 157 ++++++ tests/helpers/stream-guards.test.ts | 23 +- tests/helpers/telemetry-config.test.ts | 21 + tests/helpers/telemetry-events.test.ts | 522 ++++++++++++++++++ tests/helpers/vscode-settings.test.ts | 133 +++++ 44 files changed, 3831 insertions(+), 55 deletions(-) create mode 100644 tests/commands/check-action-strict.test.ts create mode 100644 tests/commands/review-context-strict.test.ts create mode 100644 tests/engine/ast-support-errors.test.ts create mode 100644 tests/engine/runner-error-paths.test.ts create mode 100644 tests/helpers/adr-import-failures.test.ts create mode 100644 tests/helpers/binary-upgrade-archive.test.ts create mode 100644 tests/helpers/binary-upgrade-artifact.test.ts create mode 100644 tests/helpers/init-project-editors.test.ts create mode 100644 tests/helpers/platform-simulated.test.ts create mode 100644 tests/helpers/stack-detect-errors.test.ts create mode 100644 tests/helpers/telemetry-events.test.ts diff --git a/tests/commands/adr/domain/add.test.ts b/tests/commands/adr/domain/add.test.ts index ac44b361..2bf33b86 100644 --- a/tests/commands/adr/domain/add.test.ts +++ b/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( diff --git a/tests/commands/adr/domain/list.test.ts b/tests/commands/adr/domain/list.test.ts index d5e4178c..470fac39 100644 --- a/tests/commands/adr/domain/list.test.ts +++ b/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(); + } + }); }); diff --git a/tests/commands/adr/list.test.ts b/tests/commands/adr/list.test.ts index 59a1a866..e1b97449 100644 --- a/tests/commands/adr/list.test.ts +++ b/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(); diff --git a/tests/commands/adr/update.test.ts b/tests/commands/adr/update.test.ts index a98aa859..05f755bc 100644 --- a/tests/commands/adr/update.test.ts +++ b/tests/commands/adr/update.test.ts @@ -201,6 +201,83 @@ describe("adr update action handler", () => { expect(updatedContent).toContain("domain: architecture"); }); + test("--files replaces the frontmatter globs, trimming and dropping blanks", async () => { + const adrsDir = join(tempDir, ".archgate", "adrs"); + mkdirSync(adrsDir, { recursive: true }); + writeFileSync(join(adrsDir, "ARCH-001-use-typescript.md"), ADR_CONTENT); + + process.chdir(tempDir); + const parent = makeProgram(); + await parent.parseAsync([ + "node", + "adr", + "update", + "--id", + "ARCH-001", + "--body", + "## Context\nScoped now.", + "--files", + " src/api/** , ,src/routes/** ", + ]); + + const updatedContent = await Bun.file( + join(adrsDir, "ARCH-001-use-typescript.md") + ).text(); + expect(updatedContent).toContain('"src/api/**"'); + expect(updatedContent).toContain('"src/routes/**"'); + expect(updatedContent).not.toContain('""'); + }); + + test("--domain rewrites the domain after validating it against the config", async () => { + const adrsDir = join(tempDir, ".archgate", "adrs"); + mkdirSync(adrsDir, { recursive: true }); + writeFileSync(join(adrsDir, "ARCH-001-use-typescript.md"), ADR_CONTENT); + + process.chdir(tempDir); + const parent = makeProgram(); + await parent.parseAsync([ + "node", + "adr", + "update", + "--id", + "ARCH-001", + "--body", + "## Context\nMoved.", + "--domain", + "backend", + ]); + + const updatedContent = await Bun.file( + join(adrsDir, "ARCH-001-use-typescript.md") + ).text(); + expect(updatedContent).toContain("domain: backend"); + }); + + test("--domain rejects a domain that is neither built-in nor registered", async () => { + const adrsDir = join(tempDir, ".archgate", "adrs"); + mkdirSync(adrsDir, { recursive: true }); + writeFileSync(join(adrsDir, "ARCH-001-use-typescript.md"), ADR_CONTENT); + + process.chdir(tempDir); + const parent = makeProgram(); + + expect( + parent.parseAsync([ + "node", + "adr", + "update", + "--id", + "ARCH-001", + "--body", + "## Context\nSomething.", + "--domain", + "not-a-domain", + ]) + ).rejects.toThrow("process.exit"); + + expect(exitSpy).toHaveBeenCalledWith(1); + }); + test("exits with error when ADR ID is not found", async () => { const adrsDir = join(tempDir, ".archgate", "adrs"); mkdirSync(adrsDir, { recursive: true }); diff --git a/tests/commands/check-action-strict.test.ts b/tests/commands/check-action-strict.test.ts new file mode 100644 index 00000000..a5138344 --- /dev/null +++ b/tests/commands/check-action-strict.test.ts @@ -0,0 +1,331 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 Archgate + +// --------------------------------------------------------------------------- +// Action handler tests for two edges of the check command: the zero-rule-ADR +// path (its non-console reporters and its advisory diagnostics), and the +// ARCH-026 --strict stderr explanation that precedes a strict-driven exit. +// --------------------------------------------------------------------------- + +import { + afterEach, + beforeEach, + describe, + expect, + spyOn, + test, + type Mock, +} from "bun:test"; + +import { Command } from "@commander-js/extra-typings"; + +import { registerCheckCommand } from "../../src/commands/check"; +import * as adrSectionsModule from "../../src/engine/adr-sections"; +import * as loaderModule from "../../src/engine/loader"; +import type { ReportSummary } from "../../src/engine/reporter"; +import * as reporterModule from "../../src/engine/reporter"; +import type { CheckResult } from "../../src/engine/runner"; +import * as runnerModule from "../../src/engine/runner"; +import * as exitModule from "../../src/helpers/exit"; +import * as logModule from "../../src/helpers/log"; +import * as pathsModule from "../../src/helpers/paths"; +import * as stackDetectModule from "../../src/helpers/stack-detect"; +import * as telemetryModule from "../../src/helpers/telemetry"; + +const MOCK_CHECK_RESULT: CheckResult = { results: [], totalDurationMs: 5 }; + +const BASE_SUMMARY: ReportSummary = { + pass: true, + total: 0, + passed: 0, + failed: 0, + warnings: 0, + errors: 0, + infos: 0, + ruleErrors: 0, + warningsExceeded: false, + strictAdvisoryExceeded: false, + truncated: false, + suppressed: 0, + suppressionWarnings: [], + unparsedAdrs: [], + briefingWarnings: [], + results: [], + durationMs: 5, +}; + +const BRIEFING_WARNING = { + adrId: "LONG-001", + file: ".archgate/adrs/LONG-001-verbose.md", + section: "Decision", + length: 4000, + cap: 2000, +}; + +describe("check action handler — zero-rule and strict branches", () => { + let logSpy: Mock; + let errorSpy: Mock; + let warnSpy: Mock; + let exitSpy: Mock; + let findProjectRootSpy: Mock; + let loadRuleAdrsSpy: Mock; + let runChecksSpy: Mock; + let buildSummarySpy: Mock; + let getExitCodeSpy: Mock; + let reportConsoleSpy: Mock; + let reportJSONSpy: Mock; + let reportCISpy: Mock; + let reportSarifSpy: Mock; + let diagnosticsSpy: Mock; + let detectStackSpy: Mock; + let trackCheckResultSpy: Mock; + let originalIsTTY: boolean | undefined; + + beforeEach(() => { + logSpy = spyOn(console, "log").mockImplementation(() => {}); + errorSpy = spyOn(console, "error").mockImplementation(() => {}); + warnSpy = spyOn(logModule, "logWarn").mockImplementation(() => {}); + exitSpy = spyOn(exitModule, "exitWith").mockImplementation(() => { + throw new Error("process.exit"); + }); + + findProjectRootSpy = spyOn(pathsModule, "findProjectRoot").mockReturnValue( + "/fake/project" + ); + loadRuleAdrsSpy = spyOn(loaderModule, "loadRuleAdrs").mockResolvedValue([]); + runChecksSpy = spyOn(runnerModule, "runChecks").mockResolvedValue( + MOCK_CHECK_RESULT + ); + buildSummarySpy = spyOn(reporterModule, "buildSummary").mockReturnValue( + BASE_SUMMARY + ); + getExitCodeSpy = spyOn(reporterModule, "getExitCode").mockReturnValue(0); + reportConsoleSpy = spyOn( + reporterModule, + "reportConsole" + ).mockImplementation(() => {}); + reportJSONSpy = spyOn(reporterModule, "reportJSON").mockImplementation( + () => {} + ); + reportCISpy = spyOn(reporterModule, "reportCI").mockImplementation( + () => {} + ); + reportSarifSpy = spyOn(reporterModule, "reportSarif").mockImplementation( + () => {} + ); + diagnosticsSpy = spyOn( + adrSectionsModule, + "collectBriefingDiagnostics" + ).mockResolvedValue({ briefingWarnings: [], unparsedAdrs: [] }); + detectStackSpy = spyOn(stackDetectModule, "detectStack").mockResolvedValue({ + languages: [], + runtimes: [], + frameworks: [], + }); + trackCheckResultSpy = spyOn( + telemetryModule, + "trackCheckResult" + ).mockImplementation(() => {}); + + originalIsTTY = process.stdout.isTTY; + Object.defineProperty(process.stdout, "isTTY", { + value: true, + configurable: true, + }); + }); + + afterEach(() => { + logSpy.mockRestore(); + errorSpy.mockRestore(); + warnSpy.mockRestore(); + exitSpy.mockRestore(); + findProjectRootSpy.mockRestore(); + loadRuleAdrsSpy.mockRestore(); + runChecksSpy.mockRestore(); + buildSummarySpy.mockRestore(); + getExitCodeSpy.mockRestore(); + reportConsoleSpy.mockRestore(); + reportJSONSpy.mockRestore(); + reportCISpy.mockRestore(); + reportSarifSpy.mockRestore(); + diagnosticsSpy.mockRestore(); + detectStackSpy.mockRestore(); + trackCheckResultSpy.mockRestore(); + Object.defineProperty(process.stdout, "isTTY", { + value: originalIsTTY, + configurable: true, + }); + }); + + function makeProgram(): Command { + const program = new Command().exitOverride(); + registerCheckCommand(program); + return program; + } + + function warnings(): string { + return warnSpy.mock.calls.flat().map(String).join(" "); + } + + // -- Zero rule ADRs: non-console reporters -- + + test("no rules with --output sarif reports the empty result through reportSarif", async () => { + expect( + makeProgram().parseAsync(["node", "test", "check", "--output", "sarif"]) + ).rejects.toThrow("process.exit"); + + expect(reportSarifSpy).toHaveBeenCalledTimes(1); + expect(reportSarifSpy.mock.calls[0][0].results).toEqual([]); + expect(reportCISpy).not.toHaveBeenCalled(); + expect(reportJSONSpy).not.toHaveBeenCalled(); + expect(reportConsoleSpy).not.toHaveBeenCalled(); + }); + + test("no rules with --output github reports the empty result through reportCI", async () => { + expect( + makeProgram().parseAsync(["node", "test", "check", "--output", "github"]) + ).rejects.toThrow("process.exit"); + + expect(reportCISpy).toHaveBeenCalledTimes(1); + expect(reportCISpy.mock.calls[0][0].results).toEqual([]); + expect(reportSarifSpy).not.toHaveBeenCalled(); + expect(reportConsoleSpy).not.toHaveBeenCalled(); + }); + + // -- Zero rule ADRs: advisory diagnostics still surface on the console path -- + + test("no rules still renders advisory findings on the console path", async () => { + diagnosticsSpy.mockResolvedValue({ + briefingWarnings: [BRIEFING_WARNING], + unparsedAdrs: [], + }); + + expect(makeProgram().parseAsync(["node", "test", "check"])).rejects.toThrow( + "process.exit" + ); + + const output = logSpy.mock.calls + .map((c: unknown[]) => String(c[0])) + .join("\n"); + expect(output).toContain("No rules to check"); + // The corpus-wide diagnostics are rendered by the standard console + // reporter, so a prose-only corpus still shows its briefing overruns. + expect(reportConsoleSpy).toHaveBeenCalledTimes(1); + expect(reportConsoleSpy.mock.calls[0][0].briefingWarnings).toEqual([ + BRIEFING_WARNING, + ]); + }); + + test("no rules skips the console reporter when there is nothing advisory to show", async () => { + expect(makeProgram().parseAsync(["node", "test", "check"])).rejects.toThrow( + "process.exit" + ); + + expect(reportConsoleSpy).not.toHaveBeenCalled(); + }); + + test("no rules with --strict explains an advisory-only failure", async () => { + diagnosticsSpy.mockResolvedValue({ + briefingWarnings: [BRIEFING_WARNING], + unparsedAdrs: ["broken.md"], + }); + buildSummarySpy.mockReturnValue({ + ...BASE_SUMMARY, + pass: false, + strictAdvisoryExceeded: true, + }); + getExitCodeSpy.mockReturnValue(1); + + expect( + makeProgram().parseAsync(["node", "test", "check", "--strict"]) + ).rejects.toThrow("process.exit"); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(warnings()).toContain( + "failing because advisory findings (briefing budget or unparsed ADRs) exist even though no rules ran" + ); + }); + + test("no rules without --strict stays silent about advisory findings", async () => { + diagnosticsSpy.mockResolvedValue({ + briefingWarnings: [BRIEFING_WARNING], + unparsedAdrs: [], + }); + buildSummarySpy.mockReturnValue({ + ...BASE_SUMMARY, + strictAdvisoryExceeded: true, + }); + + expect(makeProgram().parseAsync(["node", "test", "check"])).rejects.toThrow( + "process.exit" + ); + + expect(warnSpy).not.toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(0); + }); + + // -- ARCH-026 strict explanation after a normal run -- + + test("--strict explains a warning-driven failure", async () => { + loadRuleAdrsSpy.mockResolvedValue([ + // Only the array length is read before runChecks is reached, which is + // itself stubbed — the ADR/ruleSet contents are never touched. + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + { type: "loaded", value: {} } as unknown as loaderModule.LoadResult, + ]); + buildSummarySpy.mockReturnValue({ + ...BASE_SUMMARY, + pass: false, + warnings: 3, + warningsExceeded: true, + }); + getExitCodeSpy.mockReturnValue(1); + + expect( + makeProgram().parseAsync(["node", "test", "check", "--strict"]) + ).rejects.toThrow("process.exit"); + + expect(warnings()).toContain( + "3 rule-severity warning(s) are treated as failures under --strict" + ); + expect(warnings()).not.toContain("advisory findings"); + expect(exitSpy).toHaveBeenCalledWith(1); + }); + + test("--strict joins both reasons when warnings and advisory findings coexist", async () => { + loadRuleAdrsSpy.mockResolvedValue([ + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + { type: "loaded", value: {} } as unknown as loaderModule.LoadResult, + ]); + buildSummarySpy.mockReturnValue({ + ...BASE_SUMMARY, + pass: false, + warnings: 1, + warningsExceeded: true, + strictAdvisoryExceeded: true, + }); + getExitCodeSpy.mockReturnValue(1); + + expect( + makeProgram().parseAsync(["node", "test", "check", "--strict"]) + ).rejects.toThrow("process.exit"); + + expect(warnings()).toContain( + "1 rule-severity warning(s) are treated as failures under --strict; advisory findings (briefing budget, suppression, or unparsed ADRs) failed under --strict" + ); + }); + + test("--strict with nothing strict-relevant emits no explanation", async () => { + loadRuleAdrsSpy.mockResolvedValue([ + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + { type: "loaded", value: {} } as unknown as loaderModule.LoadResult, + ]); + + expect( + makeProgram().parseAsync(["node", "test", "check", "--strict"]) + ).rejects.toThrow("process.exit"); + + expect(warnSpy).not.toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(0); + }); +}); diff --git a/tests/commands/clean.test.ts b/tests/commands/clean.test.ts index b8a8109a..deb9bedc 100644 --- a/tests/commands/clean.test.ts +++ b/tests/commands/clean.test.ts @@ -16,6 +16,7 @@ import { writeFileSync, existsSync, } from "node:fs"; +import * as nodeFs from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -142,4 +143,32 @@ describe("clean action handler", () => { }); } }); + + test("routes a removal failure through the command error boundary", async () => { + const archgateDir = join(fakeHome, ".archgate"); + mkdirSync(archgateDir, { recursive: true }); + writeFileSync(join(archgateDir, "cache.json"), "{}"); + + const errorSpy = spyOn(console, "error").mockImplementation(() => {}); + const rmSpy = spyOn(nodeFs, "rmSync").mockImplementation(() => { + throw new Error("EPERM: operation not permitted"); + }); + + try { + const program = makeProgram(); + expect(program.parseAsync(["node", "test", "clean"])).rejects.toThrow( + "process.exit" + ); + + // An unexpected filesystem 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("EPERM: operation not permitted"); + } finally { + rmSpy.mockRestore(); + errorSpy.mockRestore(); + } + }); }); diff --git a/tests/commands/login.test.ts b/tests/commands/login.test.ts index 7eb28fe5..8bc2d81b 100644 --- a/tests/commands/login.test.ts +++ b/tests/commands/login.test.ts @@ -23,6 +23,7 @@ import { registerLoginCommand } from "../../src/commands/login"; import * as credentialStore from "../../src/helpers/credential-store"; import * as exitMod from "../../src/helpers/exit"; import * as loginFlow from "../../src/helpers/login-flow"; +import * as paths from "../../src/helpers/paths"; import * as telemetry from "../../src/helpers/telemetry"; // --------------------------------------------------------------------------- @@ -254,6 +255,51 @@ describe("login action handlers", () => { expect(allOutput).toMatch(/archgate (check|init)/u); }); + test("next step is `archgate check` when a project root is present", async () => { + loadCredentialsSpy.mockResolvedValueOnce(null); + runLoginFlowSpy.mockResolvedValueOnce({ + ok: true, + githubUser: "octocat", + }); + const rootSpy = spyOn(paths, "findProjectRoot").mockReturnValue( + "/fake/project" + ); + + try { + const program = makeProgram(); + await program.parseAsync(["node", "test", "login"]); + + const allOutput = logSpy.mock.calls + .map((c: unknown[]) => c.map(String).join(" ")) + .join("\n"); + expect(allOutput).toContain("archgate check"); + } finally { + rootSpy.mockRestore(); + } + }); + + test("next step is `archgate init` when no project root is found", async () => { + loadCredentialsSpy.mockResolvedValueOnce(null); + runLoginFlowSpy.mockResolvedValueOnce({ + ok: true, + githubUser: "octocat", + }); + const rootSpy = spyOn(paths, "findProjectRoot").mockReturnValue(null); + + try { + const program = makeProgram(); + await program.parseAsync(["node", "test", "login"]); + + const allOutput = logSpy.mock.calls + .map((c: unknown[]) => c.map(String).join(" ")) + .join("\n"); + expect(allOutput).toContain("archgate init"); + expect(allOutput).not.toContain("archgate check"); + } finally { + rootSpy.mockRestore(); + } + }); + test("exits with code 1 and prints TLS hint on TLS error", async () => { loadCredentialsSpy.mockResolvedValueOnce(null); runLoginFlowSpy.mockRejectedValueOnce( diff --git a/tests/commands/plugin/url.test.ts b/tests/commands/plugin/url.test.ts index 20147608..00bcba9f 100644 --- a/tests/commands/plugin/url.test.ts +++ b/tests/commands/plugin/url.test.ts @@ -156,6 +156,42 @@ describe("plugin url action handler", () => { } }); + test("routes a detection failure through the command error boundary", async () => { + const originalIsTTY = process.stdin.isTTY; + const errorSpy = spyOn(console, "error").mockImplementation(() => {}); + const exitSpy = spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit"); + }); + const detectSpy = spyOn(editorDetect, "detectEditors").mockRejectedValue( + new Error("editor probe failed") + ); + try { + Object.defineProperty(process.stdin, "isTTY", { + value: true, + configurable: true, + }); + + expect(makeProgram().parseAsync(["node", "test", "url"])).rejects.toThrow( + "process.exit" + ); + + // An unexpected probe 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("editor probe failed"); + } finally { + Object.defineProperty(process.stdin, "isTTY", { + value: originalIsTTY, + configurable: true, + }); + detectSpy.mockRestore(); + exitSpy.mockRestore(); + errorSpy.mockRestore(); + } + }); + test("TTY without --editor detects editors and prints the selected one", async () => { const originalIsTTY = process.stdin.isTTY; const detectSpy = spyOn(editorDetect, "detectEditors").mockResolvedValue( diff --git a/tests/commands/review-context-strict.test.ts b/tests/commands/review-context-strict.test.ts new file mode 100644 index 00000000..2a2a0162 --- /dev/null +++ b/tests/commands/review-context-strict.test.ts @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 Archgate + +// --------------------------------------------------------------------------- +// The ordering contract review-context.ts documents around its --strict exit: +// the full context payload is written to stdout BEFORE the exit, so a piped +// consumer still receives it on a strict failure. +// --------------------------------------------------------------------------- + +import { + afterEach, + beforeEach, + describe, + expect, + spyOn, + test, + type Mock, +} from "bun:test"; +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { Command } from "@commander-js/extra-typings"; + +import { registerReviewContextCommand } from "../../src/commands/review-context"; +import * as exitModule from "../../src/helpers/exit"; +import * as logModule from "../../src/helpers/log"; +import { expectKeys, safeRmSync } from "../test-utils"; + +/** An ADR whose Decision section overruns the per-section briefing budget. */ +function overBudgetAdr(id: string): string { + return `--- +id: ${id} +title: Very Long ADR +domain: architecture +rules: false +--- + +## Context +Short context. + +## Decision +${"This decision sentence is repeated to blow past the briefing budget. ".repeat(50)} + +## Do's and Don'ts +### Do +- Do something. +`; +} + +describe("review-context --strict output ordering", () => { + let tempDir: string; + let adrsDir: string; + let originalCwd: string; + let logSpy: Mock; + let warnSpy: Mock; + let exitSpy: Mock; + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), "archgate-review-strict-test-")); + adrsDir = join(tempDir, ".archgate", "adrs"); + mkdirSync(adrsDir, { recursive: true }); + originalCwd = process.cwd(); + Bun.env.ARCHGATE_PROJECT_CEILING = tempDir; + logSpy = spyOn(console, "log").mockImplementation(() => {}); + warnSpy = spyOn(logModule, "logWarn").mockImplementation(() => {}); + exitSpy = spyOn(exitModule, "exitWith").mockImplementation( + async (): Promise => + // exitWith is `Promise` because it terminates the process. A + // resolving stub hands control back to the action instead, which is + // what lets the assertions below check that stdout is complete and + // final at the moment the exit is requested. + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + undefined as unknown as never + ); + }); + + afterEach(() => { + process.chdir(originalCwd); + delete Bun.env.ARCHGATE_PROJECT_CEILING; + safeRmSync(tempDir); + exitSpy.mockRestore(); + warnSpy.mockRestore(); + logSpy.mockRestore(); + }); + + test("prints the whole context payload before requesting exit 1", async () => { + writeFileSync( + join(adrsDir, "LONG-001-verbose.md"), + overBudgetAdr("LONG-001") + ); + process.chdir(tempDir); + + const program = new Command().exitOverride(); + registerReviewContextCommand(program); + await program.parseAsync([ + "node", + "test", + "review-context", + "--run-checks", + "--strict", + ]); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(warnSpy.mock.calls.flat().map(String).join(" ")).toContain( + "--strict: failing because" + ); + + // Exactly one stdout write, and it is the complete JSON context — the + // strict failure adds nothing to stdout and truncates nothing from it. + expect(logSpy).toHaveBeenCalledTimes(1); + const payload = expectKeys( + JSON.parse(String(logSpy.mock.calls[0][0])), + "checkSummary", + "domains", + "allChangedFiles" + ); + expect(payload.checkSummary).not.toBeNull(); + }); +}); diff --git a/tests/commands/session-context/claude-code.test.ts b/tests/commands/session-context/claude-code.test.ts index 7362540a..d3453132 100644 --- a/tests/commands/session-context/claude-code.test.ts +++ b/tests/commands/session-context/claude-code.test.ts @@ -15,7 +15,10 @@ import { join } from "node:path"; import { Command } from "@commander-js/extra-typings"; -import { registerClaudeCodeSessionContextCommand } from "../../../src/commands/session-context/claude-code"; +import { + makeMaxEntriesOption, + registerClaudeCodeSessionContextCommand, +} from "../../../src/commands/session-context/claude-code"; import * as sessionContextHelpers from "../../../src/helpers/session-context"; import { runCli } from "../../integration/cli-harness"; import { safeRmSync } from "../../test-utils"; @@ -51,6 +54,40 @@ describe("registerClaudeCodeSessionContextCommand", () => { }); }); +describe("makeMaxEntriesOption", () => { + /** Parse `--max-entries ` in isolation, with commander's exit and + * stderr writes neutralized so a rejection surfaces as a thrown error. */ + function parseMaxEntries(value: string): number | undefined { + const program = new Command("probe") + .addOption(makeMaxEntriesOption()) + .exitOverride() + .configureOutput({ + writeErr: () => { + // Commander writes the rejection to stderr; the throw is the signal. + }, + }); + program.parse(["--max-entries", value], { from: "user" }); + return program.opts().maxEntries; + } + + test.each(["0", "-1", "abc", "", "Infinity", "0.5"])( + "rejects %p as a limit", + (value) => { + expect(() => parseMaxEntries(value)).toThrow( + /must be a positive integer/u + ); + } + ); + + test.each<[string, number]>([ + ["1", 1], + ["200", 200], + ["3.9", 3], + ])("accepts %p as %p", (value, expected) => { + expect(parseMaxEntries(value)).toBe(expected); + }); +}); + describe("claude-code action handler", () => { let tempDir: string; let originalCwd: string; diff --git a/tests/commands/upgrade.test.ts b/tests/commands/upgrade.test.ts index bb27622d..fe9d48f1 100644 --- a/tests/commands/upgrade.test.ts +++ b/tests/commands/upgrade.test.ts @@ -195,6 +195,21 @@ describe("install method detection", () => { expect(method.type).toBe("package-manager"); }); + test("stops the package-root walk at the ancestor-depth cap", async () => { + // 1200 synthetic ancestors, none of which exist: the walk for a + // package.json exhausts its depth budget before reaching a filesystem + // root, so the local-install branch gives up rather than looping on a + // pathological path. No directories are created — `dirname` is pure + // string work and `existsSync` simply reports false throughout. + const segments = Array.from({ length: 1200 }, (_, i) => `d${String(i)}`); + setExecPath( + join(tempDir, "node_modules", ...segments, ".bin", "archgate") + ); + + const method = await _detectInstallMethod(); + expect(method.type).toBe("package-manager"); + }); + test("binary detection takes priority over other methods", async () => { setExecPath(join(tempDir, ".archgate", "bin", "archgate")); const method = await _detectInstallMethod(); diff --git a/tests/engine/ast-support-errors.test.ts b/tests/engine/ast-support-errors.test.ts new file mode 100644 index 00000000..b9e04bcf --- /dev/null +++ b/tests/engine/ast-support-errors.test.ts @@ -0,0 +1,93 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 Archgate +import { describe, expect, spyOn, test } from "bun:test"; +import { mkdtempSync, readdirSync, rmSync } from "node:fs"; +import * as os from "node:os"; +import { join } from "node:path"; + +import { + interpreterNotFoundError, + probeInterpreter, + writeTempSourceFile, +} from "../../src/engine/ast-support"; + +// Failure paths of the ctx.ast() support helpers; the happy paths and the +// interpreter end-to-end programs live in ast-support.test.ts. + +/** Placeholder callback for a timer whose only purpose is a handle to clear. */ +function noop(): void { + // Intentionally empty. +} + +describe("interpreterNotFoundError", () => { + test.each<["python" | "ruby", string, string]>([ + ["python", "Python", "src/app.py"], + ["ruby", "Ruby", "src/app.rb"], + ])( + "names the %s interpreter, candidates and path", + (language, label, path) => { + const err = interpreterNotFoundError(language, ["a", "b"], path); + + expect(err).toBeInstanceOf(Error); + expect(err.message).toContain(`${label} interpreter not found on PATH`); + expect(err.message).toContain("(tried: a, b)"); + expect(err.message).toContain(`ctx.ast("${path}", "${language}")`); + } + ); +}); + +describe("probeInterpreter timeout", () => { + /** + * Fire the probe's own 5s timer synchronously so the race resolves to + * "timeout" deterministically, without holding the suite for five seconds. + * Every other delay passes through to the real timer. + */ + function installImmediateProbeTimer(): () => void { + const realSetTimeout = globalThis.setTimeout; + const spy = spyOn(globalThis, "setTimeout"); + // The replacement deliberately does not match setTimeout's overloads — + // it forwards everything it does not intercept to the real one. + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + spy.mockImplementation(((fn: () => void, ms?: number) => { + if (ms !== 5_000) return realSetTimeout(fn, ms); + fn(); + return realSetTimeout(noop, 0); + }) as unknown as typeof setTimeout); + return () => { + spy.mockRestore(); + }; + } + + test("kills a candidate that outlives the probe timeout and moves on", async () => { + // process.execPath is the running bun binary — it exists and would + // normally answer `--version`, so only the timeout can reject it. + const restoreTimer = installImmediateProbeTimer(); + + try { + expect(await probeInterpreter([process.execPath])).toBeNull(); + } finally { + restoreTimer(); + } + }); +}); + +describe("writeTempSourceFile", () => { + test("removes the private temp dir when the file cannot be created", () => { + const realTmpdir = os.tmpdir(); + const sandbox = mkdtempSync(join(realTmpdir, "archgate-ast-sandbox-")); + const tmpdirSpy = spyOn(os, "tmpdir").mockReturnValue(sandbox); + + try { + // An extension carrying a directory component aims the exclusive + // create at a path whose parent was never made, so the write fails + // after mkdtempSync has already created the private directory. + expect(() => + writeTempSourceFile("print(1)", "/missing/source.py") + ).toThrow(); + expect(readdirSync(sandbox)).toEqual([]); + } finally { + tmpdirSpy.mockRestore(); + rmSync(sandbox, { recursive: true, force: true }); + } + }); +}); diff --git a/tests/engine/git-files.test.ts b/tests/engine/git-files.test.ts index 55564fe8..911d5693 100644 --- a/tests/engine/git-files.test.ts +++ b/tests/engine/git-files.test.ts @@ -107,6 +107,27 @@ describe("git-files", () => { }); } ); + + test("prefers the remote HEAD symref over local branches", async () => { + await git(["init", "--initial-branch=main"], tempDir); + await git(["config", "user.email", "test@test.com"], tempDir); + await git(["config", "user.name", "Test"], tempDir); + writeFileSync(join(tempDir, "file.ts"), "export const x = 1;"); + await git(["add", "file.ts"], tempDir); + await git(["commit", "-m", "init"], tempDir); + // What `git remote set-head origin` writes; no network needed. + await git( + [ + "symbolic-ref", + "refs/remotes/origin/HEAD", + "refs/remotes/origin/trunk", + ], + tempDir + ); + + const ref = await detectBaseRef(tempDir); + expect(ref).toBe("origin/trunk"); + }); }); describe("resolveBaseRef", () => { diff --git a/tests/engine/rule-scanner.test.ts b/tests/engine/rule-scanner.test.ts index 7263b0c3..7ab3d620 100644 --- a/tests/engine/rule-scanner.test.ts +++ b/tests/engine/rule-scanner.test.ts @@ -238,6 +238,28 @@ describe("scanRuleSource", () => { }); }); + describe("caller-supplied transpiled JS", () => { + test("walks the supplied JS rather than transpiling the source again", () => { + // The TypeScript source is clean; only the supplied JS names a banned + // module, so a violation proves the supplied JS is what was walked. + const violations = scanRuleSource( + `export const ok = 1;`, + `import fs from "node:fs";` + ); + expect(violations).toHaveLength(1); + expect(violations[0].message).toContain(`"node:fs"`); + expect(violations[0].message).toContain("blocked"); + }); + + test("returns a parse-error violation when the supplied JS is broken", () => { + const violations = scanRuleSource(`export const ok = 1;`, `const = ;`); + expect(violations).toHaveLength(1); + expect(violations[0].message).toContain("Parse error"); + expect(violations[0].line).toBe(1); + expect(violations[0].column).toBe(0); + }); + }); + // Position remapping tests are in rule-scanner-positions.test.ts }); diff --git a/tests/engine/runner-error-paths.test.ts b/tests/engine/runner-error-paths.test.ts new file mode 100644 index 00000000..90b7af9d --- /dev/null +++ b/tests/engine/runner-error-paths.test.ts @@ -0,0 +1,124 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 Archgate +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import * as astSupport from "../../src/engine/ast-support"; +import type { LoadResult } from "../../src/engine/loader"; +import { runChecks } from "../../src/engine/runner"; +import type { AdrDocument } from "../../src/formats/adr"; +import type { RuleSet } from "../../src/formats/rules"; +import { safeRmSync } from "../test-utils"; + +// Failure paths of the rule runner: a missing interpreter and the per-rule +// timeout. Happy paths live in runner.test.ts. + +/** Placeholder callback for a promise/timer that must never do anything. */ +function noop(): void { + // Intentionally empty. +} + +function makeLoadedAdr( + ruleSet: RuleSet, + overrides: Partial = {} +): LoadResult { + return { + type: "loaded", + value: { + adr: { + frontmatter: { + id: "ERR-001", + title: "Runner Error Paths", + domain: "general", + rules: true, + ...overrides, + }, + body: "", + filePath: "/test.md", + }, + ruleSet, + }, + }; +} + +describe("runChecks failure paths", () => { + let tempDir: string; + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), "archgate-runner-errors-")); + mkdirSync(join(tempDir, "src"), { recursive: true }); + }); + + afterEach(() => { + safeRmSync(tempDir); + }); + + test("ctx.ast reports a missing interpreter as a rule error", async () => { + writeFileSync(join(tempDir, "src", "app.py"), "x = 1\n"); + const probeSpy = spyOn(astSupport, "probeInterpreter").mockResolvedValue( + null + ); + + try { + const loaded = makeLoadedAdr({ + rules: { + "parse-python": { + description: "Parse a Python file", + async check(ctx) { + await ctx.ast("src/app.py", "python"); + }, + }, + }, + }); + + const result = await runChecks(tempDir, [loaded]); + + expect(result.results[0].error).toContain( + "Python interpreter not found on PATH" + ); + expect(result.results[0].error).toContain("src/app.py"); + } finally { + probeSpy.mockRestore(); + } + }); + + test("a rule that never settles is failed by the per-rule timeout", async () => { + // Fire the runner's own 30s timer synchronously so the race resolves to + // the timeout deterministically, without holding the suite for 30 + // seconds. Every other delay passes through to the real timer. + const realSetTimeout = globalThis.setTimeout; + const timerSpy = spyOn(globalThis, "setTimeout"); + // The replacement deliberately does not match setTimeout's overloads — + // it forwards everything it does not intercept to the real one. + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + timerSpy.mockImplementation(((fn: () => void, ms?: number) => { + if (ms !== 30_000) return realSetTimeout(fn, ms); + fn(); + return realSetTimeout(noop, 0); + }) as unknown as typeof setTimeout); + + try { + const loaded = makeLoadedAdr({ + rules: { + "never-settles": { + description: "A rule whose check never resolves", + async check() { + await new Promise(noop); + }, + }, + }, + }); + + const result = await runChecks(tempDir, [loaded]); + + expect(result.results[0].error).toBe( + "Rule never-settles timed out after 30000ms" + ); + expect(result.results[0].violations).toHaveLength(0); + } finally { + timerSpy.mockRestore(); + } + }); +}); diff --git a/tests/engine/safe-path.test.ts b/tests/engine/safe-path.test.ts index 894de1e3..34adaf6e 100644 --- a/tests/engine/safe-path.test.ts +++ b/tests/engine/safe-path.test.ts @@ -1,6 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright 2026 Archgate -import { describe, expect, test, beforeEach, afterEach } from "bun:test"; +import { describe, expect, test, beforeEach, afterEach, spyOn } from "bun:test"; +import * as fs from "node:fs"; import { mkdtempSync, rmSync, @@ -225,5 +226,47 @@ describe("safe-path", () => { ); } ); + + test.skipIf(!DIR_LINKS)( + "ALLOWS a dangling link, which resolves to nothing", + () => { + // The link's target is gone, so it can reach neither inside nor + // outside the project; the eventual read fails on its own merits. + const outsideDir = makeOutsideDir(); + symlinkSync(outsideDir, join(tempDir, "gone"), "junction"); + rmSync(outsideDir, { recursive: true, force: true }); + + expect(() => safePath(tempDir, "gone/secret.txt")).not.toThrow(); + } + ); + + test.skipIf(!DIR_LINKS)( + "stays fail-closed when the root itself cannot be realpath'd", + () => { + const outsideDir = makeOutsideDir(); + writeFileSync(join(outsideDir, "secret.txt"), "sensitive"); + symlinkSync(outsideDir, join(tempDir, "linkdir"), "junction"); + + const realRealpathSync = fs.realpathSync; + const realpathSpy = spyOn(fs, "realpathSync"); + // The replacement narrows to the single-argument form the module + // under test uses; everything else forwards to the real one. + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + realpathSpy.mockImplementation(((p: fs.PathLike) => { + if (p === tempDir) throw new Error("EACCES: permission denied"); + return realRealpathSync(p); + }) as unknown as typeof fs.realpathSync); + + try { + // The comparison falls back to the lexical root — an escaping link + // must still be refused rather than waved through. + expect(() => safePath(tempDir, "linkdir/secret.txt")).toThrow( + /resolves outside the project through symbolic link "linkdir"/u + ); + } finally { + realpathSpy.mockRestore(); + } + } + ); }); }); diff --git a/tests/engine/yaml-utils.test.ts b/tests/engine/yaml-utils.test.ts index 7ad90480..aeacf6c4 100644 --- a/tests/engine/yaml-utils.test.ts +++ b/tests/engine/yaml-utils.test.ts @@ -1,6 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright 2026 Archgate -import { describe, expect, test } from "bun:test"; +import { describe, expect, spyOn, test } from "bun:test"; import { parseYamlDocument } from "../../src/engine/yaml-utils"; @@ -42,6 +42,24 @@ describe("parseYamlDocument — YAML files (.yml/.yaml)", () => { /Failed to parse "conf\/bad\.yml" as YAML/u ); }); + + test("throws when the parse result is not expressible as YAML data", () => { + // Bun.YAML.parse is typed `unknown` and the shape guard is what lets the + // return type be YamlValue without an assertion. No YAML document + // produces such a value today, so the parser is stubbed to prove the + // guard reports rather than leaking the value to the caller. + const parseSpy = spyOn(Bun.YAML, "parse").mockReturnValue( + Symbol("not-yaml-data") + ); + + try { + expect(() => parseYamlDocument("a: 1\n", "conf/x.yml")).toThrow( + /Failed to parse "conf\/x\.yml" as YAML: Parsed YAML value has an unsupported shape/u + ); + } finally { + parseSpy.mockRestore(); + } + }); }); describe("parseYamlDocument — frontmatter files (everything else)", () => { diff --git a/tests/formats/pack.test.ts b/tests/formats/pack.test.ts index 7e668008..2b54cf7d 100644 --- a/tests/formats/pack.test.ts +++ b/tests/formats/pack.test.ts @@ -10,6 +10,7 @@ import { ImportsManifestSchema, parsePackMetadata, } from "../../src/formats/pack"; +import { UserError } from "../../src/helpers/user-error"; describe("PackMetadataSchema", () => { test("parses valid pack metadata", () => { @@ -143,6 +144,25 @@ tags: expect(result.version).toBe("0.2.0"); expect(result.tags).toEqual(["language:go"]); }); + + test("throws a UserError listing every schema failure", () => { + const yaml = ` +name: Test_Pack +version: "1.0" +description: A test pack +maintainers: [] +`; + expect(() => parsePackMetadata(yaml)).toThrow(UserError); + expect(() => parsePackMetadata(yaml)).toThrow(/Invalid pack metadata:/u); + expect(() => parsePackMetadata(yaml)).toThrow(/lowercase kebab-case/u); + expect(() => parsePackMetadata(yaml)).toThrow(/version must be semver/u); + }); + + test("throws when the document is not a mapping at all", () => { + expect(() => parsePackMetadata("just a scalar")).toThrow( + /Invalid pack metadata:/u + ); + }); }); describe("CommunityLinkSchema", () => { diff --git a/tests/helpers/adr-import-failures.test.ts b/tests/helpers/adr-import-failures.test.ts new file mode 100644 index 00000000..a378ee39 --- /dev/null +++ b/tests/helpers/adr-import-failures.test.ts @@ -0,0 +1,198 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 Archgate + +// --------------------------------------------------------------------------- +// Failure paths of the ADR import pipeline: a malformed manifest, a clone that +// aborts partway through a multi-source run, and the rollback that unwinds a +// partially written ADR set. +// --------------------------------------------------------------------------- + +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import { + existsSync, + mkdirSync, + mkdtempSync, + rmSync, + writeFileSync, +} from "node:fs"; +import * as nodeFs from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { + cleanupTempDirs, + loadImportsManifest, + resolveAndCloneSources, + writeImportedAdrs, + type AdrToImport, + type IdMapping, +} from "../../src/helpers/adr-import"; +import * as logModule from "../../src/helpers/log"; +import * as registry from "../../src/helpers/registry"; +import { rejectionMessage } from "../test-utils"; + +const ADR_MARKDOWN = + "---\nid: PACK-001\ntitle: Thing\ndomain: general\nrules: false\n---\n\n## Context\n"; + +describe("loadImportsManifest — malformed manifest", () => { + let tempDir: string; + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), "archgate-manifest-bad-")); + mkdirSync(join(tempDir, ".archgate"), { recursive: true }); + }); + + afterEach(() => { + rmSync(tempDir, { recursive: true, force: true }); + }); + + test("rejects a manifest whose entries fail schema validation", async () => { + writeFileSync( + join(tempDir, ".archgate", "imports.json"), + JSON.stringify({ imports: [{ source: 42 }] }) + ); + + const message = await rejectionMessage(loadImportsManifest(tempDir)); + expect(message).toContain("Invalid imports manifest at"); + expect(message).toContain("imports.json"); + }); +}); + +describe("resolveAndCloneSources — abort partway through", () => { + const cloneDirs: string[] = []; + + afterEach(() => { + for (const dir of cloneDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test("removes every clone already made when a later source fails", async () => { + const cloneDir = mkdtempSync(join(tmpdir(), "archgate-clone-")); + cloneDirs.push(cloneDir); + + const cloneSpy = spyOn(registry, "shallowClone").mockResolvedValue( + cloneDir + ); + const targetSpy = spyOn(registry, "detectTarget").mockRejectedValue( + new Error("no ADRs found at subpath") + ); + + try { + const message = await rejectionMessage( + resolveAndCloneSources(["packs/example"]) + ); + expect(message).toBe("no ADRs found at subpath"); + + // The clone is transient scratch space — a failed run must not leave it + // behind, since nobody downstream still holds a handle to it. + expect(existsSync(cloneDir)).toBe(false); + expect(cloneSpy).toHaveBeenCalledTimes(1); + } finally { + targetSpy.mockRestore(); + cloneSpy.mockRestore(); + } + }); +}); + +describe("writeImportedAdrs — rollback", () => { + const tempDirs: string[] = []; + let srcDir: string; + let adrsDir: string; + + beforeEach(() => { + srcDir = mkdtempSync(join(tmpdir(), "archgate-rollback-src-")); + adrsDir = mkdtempSync(join(tmpdir(), "archgate-rollback-dest-")); + tempDirs.push(srcDir, adrsDir); + }); + + afterEach(() => { + for (const dir of tempDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } + }); + + /** + * Two ADRs whose destination filenames are `GEN-001-first.md` and + * `GEN-002-second.md`, so a test can sabotage the second write and observe + * what happens to the first. + */ + function twoAdrs(): { adrs: AdrToImport[]; idMap: IdMapping[] } { + const adrs: AdrToImport[] = []; + const idMap: IdMapping[] = []; + const titles = ["First", "Second"]; + for (const [i, title] of titles.entries()) { + const sourcePath = join(srcDir, `PACK-00${String(i + 1)}.md`); + writeFileSync(sourcePath, ADR_MARKDOWN); + adrs.push({ + sourcePath, + rulesPath: null, + originalId: `PACK-00${String(i + 1)}`, + title, + source: "github:example/pack", + }); + idMap.push({ + original: `PACK-00${String(i + 1)}`, + newId: `GEN-00${String(i + 1)}`, + title, + }); + } + return { adrs, idMap }; + } + + test("unlinks already-written files and rethrows when a later write fails", async () => { + const { adrs, idMap } = twoAdrs(); + // A directory occupying the second ADR's destination path makes its + // writeFileSync fail after the first ADR has already landed. + mkdirSync(join(adrsDir, "GEN-002-second.md")); + + const message = await rejectionMessage( + writeImportedAdrs(adrs, idMap, adrsDir) + ); + expect(message).toBeTruthy(); + expect(existsSync(join(adrsDir, "GEN-001-first.md"))).toBe(false); + }); + + test("still rethrows when the rollback unlink itself fails", async () => { + const { adrs, idMap } = twoAdrs(); + mkdirSync(join(adrsDir, "GEN-002-second.md")); + + const unlinkSpy = spyOn(nodeFs, "unlinkSync").mockImplementation(() => { + throw new Error("EBUSY: file is locked"); + }); + + try { + const message = await rejectionMessage( + writeImportedAdrs(adrs, idMap, adrsDir) + ); + // The original write failure is what the caller sees — a best-effort + // rollback must not mask it with its own error. + expect(message).not.toContain("EBUSY"); + expect(unlinkSpy).toHaveBeenCalledTimes(1); + } finally { + unlinkSpy.mockRestore(); + } + }); +}); + +describe("cleanupTempDirs — unremovable directory", () => { + test("logs and continues when a directory cannot be removed", () => { + const debugSpy = spyOn(logModule, "logDebug").mockImplementation(() => {}); + const rmSpy = spyOn(nodeFs, "rmSync").mockImplementation(() => { + throw new Error("EPERM: operation not permitted"); + }); + + try { + expect(() => { + cleanupTempDirs(["/tmp/archgate-locked-clone"]); + }).not.toThrow(); + expect(debugSpy).toHaveBeenCalledWith( + "Failed to clean up temp dir:", + "/tmp/archgate-locked-clone" + ); + } finally { + rmSpy.mockRestore(); + debugSpy.mockRestore(); + } + }); +}); diff --git a/tests/helpers/adr-writer.test.ts b/tests/helpers/adr-writer.test.ts index f0d80ac9..ce4c3e22 100644 --- a/tests/helpers/adr-writer.test.ts +++ b/tests/helpers/adr-writer.test.ts @@ -20,6 +20,7 @@ import { findAdrFileById, updateAdrFile, } from "../../src/helpers/adr-writer"; +import { rejectionMessage } from "../test-utils"; describe("slugify", () => { test("converts to lowercase kebab-case and handles edge cases", () => { @@ -166,6 +167,30 @@ describe("createAdrFile", () => { }); expect(existsSync(def.filePath.replace(".md", ".rules.ts"))).toBe(false); }); + + test("an explicit prefix overrides the built-in domain lookup", async () => { + const result = await createAdrFile(tempDir, { + title: "Custom Domain ADR", + domain: "general", + prefix: "SEC", + }); + expect(result.id).toBe("SEC-001"); + expect(result.fileName).toBe("SEC-001-custom-domain-adr.md"); + }); + + test("rejects a prefix that resolves to nothing", async () => { + // `resolveDomainPrefix` hands the caller a prefix for custom domains; an + // empty one means the domain was never registered, so no ID can be built. + expect( + await rejectionMessage( + createAdrFile(tempDir, { + title: "No Prefix", + domain: "general", + prefix: "", + }) + ) + ).toContain("No prefix registered for domain 'general'"); + }); }); describe("findAdrFileById", () => { diff --git a/tests/helpers/auth.test.ts b/tests/helpers/auth.test.ts index b2d4166c..aa89f52d 100644 --- a/tests/helpers/auth.test.ts +++ b/tests/helpers/auth.test.ts @@ -5,7 +5,7 @@ import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { restoreEnv } from "../test-utils"; +import { rejectionMessage, restoreEnv } from "../test-utils"; /** Type-safe fetch mock — Bun's fetch type includes `preconnect` which mock() doesn't provide. */ function mockFetch(handler: () => Promise) { @@ -337,5 +337,19 @@ describe("auth", () => { "Device code expired. Please try again." ); }); + + test("throws on a non-OK HTTP status without parsing the body", async () => { + const { pollForAccessToken } = await import("../../src/helpers/auth"); + + Bun.sleep = mock(async () => {}); + + // A gateway error carries no OAuth error code, so the status is all the + // message can report. + mockFetch(async () => new Response("upstream failure", { status: 502 })); + + expect(await rejectionMessage(pollForAccessToken("dc_abc", 0, 60))).toBe( + "GitHub token poll failed (HTTP 502)" + ); + }); }); }); diff --git a/tests/helpers/binary-upgrade-archive.test.ts b/tests/helpers/binary-upgrade-archive.test.ts new file mode 100644 index 00000000..85b137b7 --- /dev/null +++ b/tests/helpers/binary-upgrade-archive.test.ts @@ -0,0 +1,222 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 Archgate +import { + describe, + expect, + test, + mock, + spyOn, + beforeEach, + afterEach, +} from "bun:test"; +import { rmSync } from "node:fs"; +import { dirname } from "node:path"; + +import { + type ArtifactInfo, + downloadReleaseBinary, +} from "../../src/helpers/binary-upgrade"; +import { rejectionMessage } from "../test-utils"; + +const TAR_ARTIFACT: ArtifactInfo = { + name: "archgate-linux-x64", + ext: ".tar.gz", + binaryName: "archgate", +}; + +const ZIP_ARTIFACT: ArtifactInfo = { + name: "archgate-win32-x64", + ext: ".zip", + binaryName: "archgate.exe", +}; + +const ENCODER = new TextEncoder(); + +function writeHeaderField( + header: Uint8Array, + offset: number, + value: string +): void { + header.set(ENCODER.encode(value), offset); +} + +/** + * Build a 512-byte ustar header for a zero-length regular file. + * + * `tar` refuses to *create* an archive whose member escapes the extraction + * root, so an archive carrying such a member has to be assembled byte by byte. + * That is the only shape that reaches the path-traversal guard. + */ +function tarHeader(name: string): Uint8Array { + const header = new Uint8Array(512); + writeHeaderField(header, 0, name); + writeHeaderField(header, 100, "0000644\0"); + writeHeaderField(header, 108, "0000000\0"); + writeHeaderField(header, 116, "0000000\0"); + writeHeaderField(header, 124, "00000000000\0"); + writeHeaderField(header, 136, "00000000000\0"); + // The checksum is computed with its own field filled with spaces. + writeHeaderField(header, 148, " "); + writeHeaderField(header, 156, "0"); + writeHeaderField(header, 257, "ustar\0"); + writeHeaderField(header, 263, "00"); + + let checksum = 0; + for (const byte of header) checksum += byte; + writeHeaderField(header, 148, checksum.toString(8).padStart(6, "0") + "\0 "); + + return header; +} + +/** A gzipped tar holding the named entries, each an empty regular file. */ +function buildTarGz(names: string[]): Uint8Array { + // Two trailing zero blocks mark end-of-archive. + const tar = new Uint8Array(names.length * 512 + 1024); + names.forEach((name, index) => { + tar.set(tarHeader(name), index * 512); + }); + return Bun.gzipSync(tar); +} + +/** + * Serve `archive` as the release download and 404 the checksum request, so the + * archive reaches extraction without a checksum to satisfy. + */ +function mockArchiveDownload(archive: Uint8Array): void { + let callCount = 0; + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + globalThis.fetch = mock(async () => { + callCount++; + if (callCount === 1) { + const body = archive.buffer.slice( + archive.byteOffset, + archive.byteOffset + archive.byteLength + ); + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + return { + ok: true, + arrayBuffer: async () => body, + headers: new Headers(), + body: null, + } as Response; + } + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + return { ok: false, status: 404 } as Response; + }) as unknown as typeof fetch; +} + +describe("downloadReleaseBinary archive handling", () => { + let originalFetch: typeof fetch; + + beforeEach(() => { + originalFetch = globalThis.fetch; + }); + + afterEach(() => { + // mock.restore() does not undo a direct assignment to globalThis.fetch. + globalThis.fetch = originalFetch; + mock.restore(); + }); + + // A backslash-separated escape (`..\evil`) has no row: GNU tar lists it with + // the backslash escaped, so the guard's backslash normalization never sees + // the shape it is written for. `test.skipIf(...).each()` also only accepts a + // mutable row array, hence no `as const` here. + const unsafeEntries: string[] = [ + "../evil", + "pkg/../../evil", + "/etc/cron.d/evil", + "..", + ]; + + // The guard reads `tar -tzf` output, and GNU tar on Windows treats the + // `C:\...` archive path as a remote host, so the listing never happens + // there. Windows ships `.zip` releases and takes the PowerShell branch below. + test.skipIf(process.platform === "win32").each(unsafeEntries)( + "aborts extraction for the unsafe archive entry %s", + async (entry) => { + mockArchiveDownload(buildTarGz([entry])); + + const message = await rejectionMessage( + downloadReleaseBinary("v1.0.0", TAR_ARTIFACT) + ); + + expect(message).toContain("Unsafe path in release archive"); + expect(message).toContain(entry); + } + ); + + test.skipIf(process.platform === "win32")( + "extracts an archive whose entries all stay inside the root", + async () => { + mockArchiveDownload(buildTarGz(["archgate", "nested/dir/file"])); + + const binaryPath = await downloadReleaseBinary("v1.0.0", TAR_ARTIFACT); + try { + expect(binaryPath).toEndWith("archgate"); + } finally { + // downloadReleaseBinary extracts into its own mkdtemp directory. + rmSync(dirname(binaryPath), { recursive: true, force: true }); + } + } + ); + + test("reports the tar exit code when extraction fails", async () => { + // Not a gzip stream at all: `tar -tzf` lists nothing, so the guard passes + // and `tar -xzf` is what rejects it. + mockArchiveDownload(new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8])); + + const message = await rejectionMessage( + downloadReleaseBinary("v1.0.0", TAR_ARTIFACT) + ); + + expect(message).toContain("Failed to extract archive (tar exit code"); + }); + + test("reports the PowerShell exit code when zip extraction fails", async () => { + mockArchiveDownload(new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8])); + // A corrupt archive does not produce a failing exit code here: + // `Expand-Archive`'s error is non-terminating, so `powershell -Command` + // still exits 0. Stubbing the spawn is what reaches the failure branch, + // and it reaches it on every runner rather than only on Windows. + const spawnSpy = spyOn(Bun, "spawn"); + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + spawnSpy.mockImplementation((() => ({ + stdout: "", + stderr: "Expand-Archive : Central Directory corrupt.", + exited: Promise.resolve(3), + })) as unknown as typeof Bun.spawn); + + const message = await rejectionMessage( + downloadReleaseBinary("v1.0.0", ZIP_ARTIFACT) + ); + + expect(message).toBe("Failed to extract archive (PowerShell exit code 3)"); + }); + + test("continues past checksum verification when the request fails", async () => { + let callCount = 0; + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + globalThis.fetch = mock(async () => { + callCount++; + if (callCount === 1) { + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + return { + ok: true, + arrayBuffer: async () => new ArrayBuffer(8), + headers: new Headers(), + body: null, + } as Response; + } + throw new Error("checksum host unreachable"); + }) as unknown as typeof fetch; + + const message = await rejectionMessage( + downloadReleaseBinary("v1.0.0", TAR_ARTIFACT) + ); + + // The transport failure is swallowed; the run proceeds to extraction. + expect(message).not.toContain("checksum host unreachable"); + expect(message).toContain("Failed to extract archive"); + }); +}); diff --git a/tests/helpers/binary-upgrade-artifact.test.ts b/tests/helpers/binary-upgrade-artifact.test.ts new file mode 100644 index 00000000..316e6a50 --- /dev/null +++ b/tests/helpers/binary-upgrade-artifact.test.ts @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 Archgate +import { describe, expect, test } from "bun:test"; + +import { getArtifactInfo } from "../../src/helpers/binary-upgrade"; + +/** + * Run `fn` while `process.platform` and `process.arch` report the given values, + * restoring the original property descriptors afterwards. Bun defines both as + * writable data properties, which is what makes a release-target matrix + * runnable from any single OS. + */ +function withSimulatedTarget( + platform: string, + arch: string, + fn: () => T +): T { + const platformDesc = Object.getOwnPropertyDescriptor(process, "platform")!; + const archDesc = Object.getOwnPropertyDescriptor(process, "arch")!; + Object.defineProperty(process, "platform", { + ...platformDesc, + value: platform, + }); + Object.defineProperty(process, "arch", { ...archDesc, value: arch }); + try { + return fn(); + } finally { + Object.defineProperty(process, "platform", platformDesc); + Object.defineProperty(process, "arch", archDesc); + } +} + +describe("getArtifactInfo", () => { + test.skipIf(getArtifactInfo() === null)( + "returns artifact info for the current platform", + () => { + const info = getArtifactInfo(); + + expect(info).not.toBeNull(); + expect(info!.name).toMatch( + /^archgate-(darwin-arm64|linux-x64|win32-x64)$/u + ); + expect(info!.ext).toMatch(/^\.(tar\.gz|zip)$/u); + expect(info!.binaryName).toMatch(/^archgate(\.exe)?$/u); + } + ); + + // Every published release target, resolved from a single runner. Gating each + // one on the real OS would leave two of the three uncovered on both CI + // platforms, since a skipped test counts for neither. + const targets = [ + ["darwin", "arm64", "archgate-darwin-arm64", ".tar.gz", "archgate"], + ["linux", "x64", "archgate-linux-x64", ".tar.gz", "archgate"], + ["win32", "x64", "archgate-win32-x64", ".zip", "archgate.exe"], + ] as const; + + test.each(targets)( + "resolves %s/%s to %s", + (platform, arch, name, ext, binaryName) => { + const info = withSimulatedTarget(platform, arch, getArtifactInfo); + + expect(info).toEqual({ name, ext, binaryName }); + } + ); + + // A supported OS on an unpublished architecture, and an OS with no release + // at all — both fall through to the "no artifact" result that `upgrade` + // turns into a manual-install hint. + const unsupportedTargets = [ + ["darwin", "x64"], + ["linux", "arm64"], + ["win32", "arm64"], + ["freebsd", "x64"], + ["sunos", "s390x"], + ] as const; + + test.each(unsupportedTargets)( + "returns null for the unsupported target %s/%s", + (platform, arch) => { + expect(withSimulatedTarget(platform, arch, getArtifactInfo)).toBeNull(); + } + ); +}); diff --git a/tests/helpers/binary-upgrade.test.ts b/tests/helpers/binary-upgrade.test.ts index f7c9792f..e7b11968 100644 --- a/tests/helpers/binary-upgrade.test.ts +++ b/tests/helpers/binary-upgrade.test.ts @@ -32,43 +32,6 @@ function mockFetch(handler: () => Promise) { globalThis.fetch = mock(handler) as unknown as typeof fetch; } -describe("getArtifactInfo", () => { - test.skipIf(getArtifactInfo() === null)( - "returns artifact info for the current platform", - () => { - const info = getArtifactInfo(); - - expect(info).not.toBeNull(); - expect(info!.name).toMatch( - /^archgate-(darwin-arm64|linux-x64|win32-x64)$/u - ); - expect(info!.ext).toMatch(/^\.(tar\.gz|zip)$/u); - expect(info!.binaryName).toMatch(/^archgate(\.exe)?$/u); - } - ); - - test.skipIf(process.platform !== "win32")( - "returns .zip extension for win32", - () => { - const info = getArtifactInfo(); - expect(info).not.toBeNull(); - expect(info!.ext).toBe(".zip"); - expect(info!.binaryName).toBe("archgate.exe"); - expect(info!.name).toBe("archgate-win32-x64"); - } - ); - - test.skipIf(process.platform === "win32")( - "returns .tar.gz extension for non-win32", - () => { - const info = getArtifactInfo(); - expect(info).not.toBeNull(); - expect(info!.ext).toBe(".tar.gz"); - expect(info!.binaryName).toBe("archgate"); - } - ); -}); - describe("getManualInstallHint", () => { test.skipIf(process.platform !== "win32")( "returns Windows install command", @@ -90,7 +53,15 @@ describe("getManualInstallHint", () => { }); describe("fetchLatestGitHubVersion", () => { + let originalFetch: typeof fetch; + + beforeEach(() => { + originalFetch = globalThis.fetch; + }); + afterEach(() => { + // mock.restore() does not undo a direct assignment to globalThis.fetch. + globalThis.fetch = originalFetch; mock.restore(); }); @@ -128,10 +99,39 @@ describe("fetchLatestGitHubVersion", () => { const result = await fetchLatestGitHubVersion(); expect(result).toBeNull(); }); + + // A payload that parses as JSON but fails the release schema — the GitHub + // API returning a non-string tag, or an error object in place of a release. + const malformedPayloads = [ + ["a non-string tag_name", { tag_name: 42 }], + ["a null tag_name", { tag_name: null }], + ["an array payload", []], + ] as const; + + test.each(malformedPayloads)( + "returns null for %s", + async (_label, payload) => { + mockFetch(async () => { + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + return { ok: true, json: async () => payload } as Response; + }); + + const result = await fetchLatestGitHubVersion(); + expect(result).toBeNull(); + } + ); }); describe("downloadReleaseBinary", () => { + let originalFetch: typeof fetch; + + beforeEach(() => { + originalFetch = globalThis.fetch; + }); + afterEach(() => { + // mock.restore() does not undo a direct assignment to globalThis.fetch. + globalThis.fetch = originalFetch; mock.restore(); }); diff --git a/tests/helpers/editor-detect.test.ts b/tests/helpers/editor-detect.test.ts index 5ef93d54..95bd91e4 100644 --- a/tests/helpers/editor-detect.test.ts +++ b/tests/helpers/editor-detect.test.ts @@ -10,10 +10,33 @@ import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; const mockCursorTo = mock(() => true); void mock.module("node:readline", () => ({ cursorTo: mockCursorTo })); +/** + * Shape of the question objects both prompt helpers build. Declaring it here + * (rather than asserting on `unknown`) lets tests read `choices`/`validate` + * off the recorded call without a type assertion. + */ +interface EditorQuestion { + choices?: { name: string; value: string; checked?: boolean }[]; + default?: string; + validate?: (input: string[]) => boolean | string; +} + /** Mock inquirer so prompts resolve immediately without user interaction. */ -void mock.module("inquirer", () => ({ - default: { prompt: mock(async () => ({ selected: ["claude"] })) }, -})); +const mockPrompt = mock( + async (_questions: EditorQuestion[]): Promise> => ({ + selected: ["claude"], + }) +); +void mock.module("inquirer", () => ({ default: { prompt: mockPrompt } })); + +/** The single question each helper passes to inquirer. */ +function questionFrom(call: [EditorQuestion[]]): EditorQuestion { + const questions = call[0]; + if (questions.length === 0) { + throw new TypeError("inquirer was called without a question"); + } + return questions[0]; +} // --------------------------------------------------------------------------- // Imports under test — loaded AFTER mocks are registered. @@ -56,6 +79,79 @@ describe("editor-detect", () => { }); }); + describe("promptEditorSelection", () => { + beforeEach(() => { + mockPrompt.mockClear(); + mockPrompt.mockImplementation(async () => ({ + selected: ["claude", "vscode"], + })); + }); + + test("returns the editors the user checked", async () => { + expect(await promptEditorSelection(MOCK_DETECTED)).toEqual([ + "claude", + "vscode", + ]); + }); + + test("pre-checks detected editors and marks them in the label", async () => { + await promptEditorSelection(MOCK_DETECTED); + + expect(questionFrom(mockPrompt.mock.calls[0]).choices).toEqual([ + { name: "Claude Code (detected)", value: "claude", checked: true }, + { name: "Cursor", value: "cursor", checked: false }, + { name: "VS Code (detected)", value: "vscode", checked: true }, + { name: "GitHub Copilot", value: "copilot", checked: false }, + { name: "opencode", value: "opencode", checked: false }, + ]); + }); + + test("its validator rejects an empty selection", async () => { + await promptEditorSelection(MOCK_DETECTED); + const { validate } = questionFrom(mockPrompt.mock.calls[0]); + + expect(validate?.([])).toBe("Select at least one editor."); + expect(validate?.(["claude"])).toBe(true); + }); + }); + + describe("promptSingleEditorSelection", () => { + beforeEach(() => { + mockPrompt.mockClear(); + mockPrompt.mockImplementation(async () => ({ selected: "cursor" })); + }); + + test("returns the editor the user picked", async () => { + expect(await promptSingleEditorSelection(MOCK_DETECTED)).toBe("cursor"); + }); + + test("defaults to the first detected editor", async () => { + await promptSingleEditorSelection(MOCK_DETECTED); + + expect(questionFrom(mockPrompt.mock.calls[0]).default).toBe("claude"); + }); + + test("defaults to claude when nothing is detected", async () => { + await promptSingleEditorSelection( + MOCK_DETECTED.map((e) => ({ ...e, available: false })) + ); + + expect(questionFrom(mockPrompt.mock.calls[0]).default).toBe("claude"); + }); + + test("offers every editor, detected or not", async () => { + await promptSingleEditorSelection(MOCK_DETECTED); + + expect(questionFrom(mockPrompt.mock.calls[0]).choices).toEqual([ + { name: "Claude Code (detected)", value: "claude" }, + { name: "Cursor", value: "cursor" }, + { name: "VS Code (detected)", value: "vscode" }, + { name: "GitHub Copilot", value: "copilot" }, + { name: "opencode", value: "opencode" }, + ]); + }); + }); + // ------------------------------------------------------------------------- // Cursor reset after inquirer prompts (Windows spacing fix) // diff --git a/tests/helpers/exit.test.ts b/tests/helpers/exit.test.ts index d7b1a9c2..b1aacff4 100644 --- a/tests/helpers/exit.test.ts +++ b/tests/helpers/exit.test.ts @@ -1,17 +1,28 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright 2026 Archgate -import { describe, expect, test, beforeEach, afterEach } from "bun:test"; +import { + describe, + expect, + test, + beforeEach, + afterEach, + spyOn, + type Mock, +} from "bun:test"; import { beginCommand, classifyErrorKind, + exitForBrokenPipe, + exitWith, finalizeCommand, isEpipeError, _getExitState, _resetExitState, } from "../../src/helpers/exit"; +import * as telemetryMod from "../../src/helpers/telemetry"; import { UserError } from "../../src/helpers/user-error"; -import { restoreEnv } from "../test-utils"; +import { rejectionMessage, restoreEnv } from "../test-utils"; describe("exit helper", () => { let originalNodeEnv: string | undefined; @@ -144,4 +155,93 @@ describe("exit helper", () => { ); }); }); + + describe("exitWith", () => { + let exitSpy: Mock; + let trackSpy: Mock; + + beforeEach(() => { + // Throwing instead of exiting lets the test observe the requested code + // without tearing down the test runner. + exitSpy = spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit"); + }); + trackSpy = spyOn(telemetryMod, "trackCommandResult").mockImplementation( + () => {} + ); + }); + + afterEach(() => { + exitSpy.mockRestore(); + trackSpy.mockRestore(); + }); + + test.each([ + [0, "success"], + [1, "user_error"], + [2, "internal_error"], + [130, "cancelled"], + ] as const)("code %d maps to the %s outcome", async (code, outcome) => { + beginCommand("check"); + + expect(await rejectionMessage(exitWith(code))).toBe("process.exit"); + expect(exitSpy).toHaveBeenCalledWith(code); + expect(trackSpy).toHaveBeenCalledTimes(1); + expect(trackSpy.mock.calls[0][0]).toBe("check"); + expect(trackSpy.mock.calls[0][1]).toBe(code); + expect(trackSpy.mock.calls[0][3]).toMatchObject({ + outcome, + error_kind: null, + }); + }); + + test("an explicit outcome overrides the code-derived default", async () => { + beginCommand("check"); + + expect( + await rejectionMessage( + exitWith(1, { outcome: "cancelled", errorKind: "user_abort" }) + ) + ).toBe("process.exit"); + expect(trackSpy.mock.calls[0][3]).toMatchObject({ + outcome: "cancelled", + error_kind: "user_abort", + }); + }); + + test("falls back to the 'root' command name when none was begun", async () => { + expect(await rejectionMessage(exitWith(0))).toBe("process.exit"); + // beginCommand was never called, so exitWith names the invocation + // "root" and finalizeCommand resolves it to "unknown". + expect(trackSpy.mock.calls[0][0]).toBe("root"); + }); + }); + + describe("exitForBrokenPipe", () => { + test("exits 0 and tags the completion as a cancelled broken pipe", async () => { + const exitSpy = spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit"); + }); + const trackSpy = spyOn( + telemetryMod, + "trackCommandResult" + ).mockImplementation(() => {}); + try { + beginCommand("adr list"); + + expect(await rejectionMessage(exitForBrokenPipe())).toBe( + "process.exit" + ); + // Pipeline convention: a closed reader is success, not an error. + expect(exitSpy).toHaveBeenCalledWith(0); + expect(trackSpy.mock.calls[0][3]).toMatchObject({ + outcome: "cancelled", + error_kind: "broken_pipe", + }); + } finally { + exitSpy.mockRestore(); + trackSpy.mockRestore(); + } + }); + }); }); diff --git a/tests/helpers/init-project-editors.test.ts b/tests/helpers/init-project-editors.test.ts new file mode 100644 index 00000000..2c65ab73 --- /dev/null +++ b/tests/helpers/init-project-editors.test.ts @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 Archgate + +// --------------------------------------------------------------------------- +// Editor-dispatch edges of initProject: the exhaustiveness guard that fires +// when an unknown editor reaches configureEditorSettings, and the Cursor +// plugin install whose failure must not abort init. +// --------------------------------------------------------------------------- + +import { + afterEach, + beforeEach, + describe, + expect, + spyOn, + test, + type Mock, +} from "bun:test"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import * as credentialStore from "../../src/helpers/credential-store"; +import { initProject, type EditorTarget } from "../../src/helpers/init-project"; +import * as pluginInstall from "../../src/helpers/plugin-install"; +import { rejectionMessage, safeRmSync } from "../test-utils"; + +describe("initProject editor dispatch", () => { + let tempDir: string; + let credSpy: Mock; + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), "archgate-initproj-editor-")); + credSpy = spyOn(credentialStore, "loadCredentials").mockResolvedValue(null); + }); + + afterEach(() => { + credSpy.mockRestore(); + safeRmSync(tempDir); + }); + + test("rejects an editor outside the EditorTarget union", async () => { + // The union makes this unreachable through the CLI (`--editor` restricts + // its choices), so the exhaustiveness guard only fires for a caller that + // bypasses the type — which is exactly what this asserts still throws. + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + const editor = "emacs" as EditorTarget; + + const message = await rejectionMessage(initProject(tempDir, { editor })); + expect(message).toBe("Unhandled editor target: emacs"); + }); + + test("a failed Cursor plugin install is reported, not thrown", async () => { + credSpy.mockResolvedValue({ token: "tok", github_user: "user" }); + const installSpy = spyOn( + pluginInstall, + "installCursorPlugin" + ).mockRejectedValue(new Error("tarball download returned 502")); + + try { + const result = await initProject(tempDir, { + installPlugin: true, + editor: "cursor", + }); + + // Cursor's project-scope settings were still written, so init succeeded; + // only the component download is reported as incomplete. + expect(result.plugin).toEqual({ + installed: true, + detail: "tarball download returned 502", + }); + expect(installSpy).toHaveBeenCalledWith("tok"); + } finally { + installSpy.mockRestore(); + } + }); +}); diff --git a/tests/helpers/install-info.test.ts b/tests/helpers/install-info.test.ts index 2b75918b..467047e0 100644 --- a/tests/helpers/install-info.test.ts +++ b/tests/helpers/install-info.test.ts @@ -10,6 +10,7 @@ import { getProjectContext, _resetInstallInfoCaches, } from "../../src/helpers/install-info"; +import { restoreEnv } from "../test-utils"; describe("install-info", () => { afterEach(() => { @@ -38,6 +39,88 @@ describe("install-info", () => { // the process paths changed (they don't), so they should be equal. expect(first).toBe(second); }); + + describe("with a synthetic executable path", () => { + // `process.execPath` is the only input to the classification when it + // does not name the bun runtime, so pointing it at a fabricated + // location exercises each branch without installing anything. + let tempHome: string; + let originalExecPath: string; + let originalHome: string | undefined; + let originalProtoHome: string | undefined; + + beforeEach(() => { + tempHome = mkdtempSync(join(tmpdir(), "archgate-installmethod-")); + originalExecPath = process.execPath; + originalHome = Bun.env.HOME; + originalProtoHome = Bun.env.PROTO_HOME; + Bun.env.HOME = tempHome; + delete Bun.env.PROTO_HOME; + _resetInstallInfoCaches(); + }); + + afterEach(() => { + process.execPath = originalExecPath; + restoreEnv("HOME", originalHome); + restoreEnv("PROTO_HOME", originalProtoHome); + _resetInstallInfoCaches(); + rmSync(tempHome, { recursive: true, force: true }); + }); + + test("an executable under ~/.archgate/bin is a binary install", () => { + process.execPath = join(tempHome, ".archgate", "bin", "archgate"); + expect(detectInstallMethod()).toBe("binary"); + }); + + test("an executable under PROTO_HOME/tools/archgate is a proto install", () => { + const protoHome = join(tempHome, "custom-proto"); + Bun.env.PROTO_HOME = protoHome; + process.execPath = join( + protoHome, + "tools", + "archgate", + "1.2.3", + "archgate" + ); + expect(detectInstallMethod()).toBe("proto"); + }); + + test("proto detection falls back to ~/.proto when PROTO_HOME is unset", () => { + process.execPath = join( + tempHome, + ".proto", + "tools", + "archgate", + "1.2.3", + "archgate" + ); + expect(detectInstallMethod()).toBe("proto"); + }); + + test("an executable inside node_modules is a local install", () => { + process.execPath = join( + tempHome, + "project", + "node_modules", + ".bin", + "archgate" + ); + expect(detectInstallMethod()).toBe("local"); + }); + + test("anything else is a global package-manager install", () => { + process.execPath = join(tempHome, "usr", "local", "archgate"); + expect(detectInstallMethod()).toBe("global-pm"); + }); + + test("the first classification is cached, later path changes ignored", () => { + process.execPath = join(tempHome, ".archgate", "bin", "archgate"); + expect(detectInstallMethod()).toBe("binary"); + + process.execPath = join(tempHome, "usr", "local", "archgate"); + expect(detectInstallMethod()).toBe("binary"); + }); + }); }); describe("getProjectContext", () => { diff --git a/tests/helpers/log.test.ts b/tests/helpers/log.test.ts index 493b9098..5719c094 100644 --- a/tests/helpers/log.test.ts +++ b/tests/helpers/log.test.ts @@ -10,7 +10,14 @@ import { type Mock, } from "bun:test"; -import { logDebug, logInfo, logError, logWarn } from "../../src/helpers/log"; +import { + logDebug, + logInfo, + logError, + logWarn, + setLogLevel, +} from "../../src/helpers/log"; +import { restoreEnv } from "../test-utils"; describe("log helpers", () => { let logSpy: Mock; @@ -99,3 +106,64 @@ describe("log helpers", () => { }); }); }); + +describe("setLogLevel", () => { + let originalDebug: string | undefined; + let logSpy: Mock; + let warnSpy: Mock; + + beforeEach(() => { + originalDebug = Bun.env.DEBUG; + delete Bun.env.DEBUG; + logSpy = spyOn(console, "log").mockImplementation(() => {}); + warnSpy = spyOn(console, "warn").mockImplementation(() => {}); + }); + + afterEach(() => { + // The active level is module state shared with every later test file, so + // put it back to the "info" default before restoring DEBUG. + setLogLevel("info"); + restoreEnv("DEBUG", originalDebug); + logSpy.mockRestore(); + warnSpy.mockRestore(); + }); + + test("raising the level to debug makes logDebug write", () => { + setLogLevel("debug"); + logDebug("now visible"); + expect(warnSpy).toHaveBeenCalledTimes(1); + }); + + test("the debug level exports DEBUG so subprocesses inherit it", () => { + setLogLevel("debug"); + expect(Bun.env.DEBUG).toBe("1"); + }); + + test.each([ + ["error", 0, 0], + ["warn", 0, 1], + ["info", 1, 1], + ] as const)( + "level %s allows %d info and %d warn writes", + (level, infoWrites, warnWrites) => { + setLogLevel(level); + + logInfo("info line"); + logWarn("warn line"); + + expect(logSpy).toHaveBeenCalledTimes(infoWrites); + expect(warnSpy).toHaveBeenCalledTimes(warnWrites); + } + ); + + test("errors are written regardless of the level", () => { + const errorSpy = spyOn(console, "error").mockImplementation(() => {}); + try { + setLogLevel("error"); + logError("always shown"); + expect(errorSpy).toHaveBeenCalledTimes(1); + } finally { + errorSpy.mockRestore(); + } + }); +}); diff --git a/tests/helpers/login-flow.test.ts b/tests/helpers/login-flow.test.ts index 76f50a5f..3dbda1e2 100644 --- a/tests/helpers/login-flow.test.ts +++ b/tests/helpers/login-flow.test.ts @@ -31,8 +31,19 @@ let mockSaveCredentials: Mock; // Mock inquirer for the signup flow prompts (lazy-loaded via dynamic import). // Use Record as return type so mockImplementation can return // different shapes for different prompts (email, editor, useCase, confirmed). +/** + * The subset of each signup question the tests read back. Declaring it lets + * the validator assertions below index the recorded call without a cast. + */ +interface SignupQuestion { + name?: string; + validate?: (value: string) => boolean | string; +} + const mockInquirerPrompt = mock( - async (): Promise> => ({ email: "test@example.com" }) + async (_question: SignupQuestion): Promise> => ({ + email: "test@example.com", + }) ); void mock.module("inquirer", () => ({ default: { prompt: mockInquirerPrompt }, @@ -418,6 +429,46 @@ describe("login-flow", () => { expect(promptCallCount).toBe(3); }); + test("signup prompts validate the email and use-case answers", async () => { + mockClaimArchgateToken.mockImplementation(async () => { + throw new SignupRequiredError(); + }); + + // Deliberately incomplete fake: only the call signature fetch invokes + // matters for this test. + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + globalThis.fetch = (async () => + Response.json( + { token: "validated-token" }, + { status: 201 } + )) as unknown as typeof fetch; + + // A pre-selected editor drops the editor prompt, so the recorded calls + // are email, useCase, confirm — in that order. + let promptCallCount = 0; + mockInquirerPrompt.mockImplementation(async () => { + promptCallCount++; + switch (promptCallCount) { + case 1: + return { email: "test@example.com" }; + case 2: + return { useCase: "governance" }; + default: + return { confirmed: true }; + } + }); + + await runLoginFlow({ editor: "claude-code" }); + + const emailValidate = mockInquirerPrompt.mock.calls[0][0].validate; + expect(emailValidate?.("not-an-email")).toBe("Enter a valid email address"); + expect(emailValidate?.("dev@example.com")).toBe(true); + + const useCaseValidate = mockInquirerPrompt.mock.calls[1][0].validate; + expect(useCaseValidate?.(" ")).toBe("Please describe your use case"); + expect(useCaseValidate?.("governance")).toBe(true); + }); + test("claimArchgateToken throws non-signup error -> propagates", async () => { mockClaimArchgateToken.mockImplementation(async () => { throw new Error("Token claim failed (HTTP 500)"); diff --git a/tests/helpers/pack-recommend.test.ts b/tests/helpers/pack-recommend.test.ts index 433aa4c2..2c358e9e 100644 --- a/tests/helpers/pack-recommend.test.ts +++ b/tests/helpers/pack-recommend.test.ts @@ -199,6 +199,46 @@ describe("recommendPacksFromDir", () => { expect(recs[0].matchedTags).toContain("runtime:bun"); }); + test.each([ + ["runtime", "runtime:deno"], + ["framework", "framework:remix"], + ["unknown-namespace", "planet:mars"], + ])("drops a non-matching %s tag but keeps the pack", (_kind, tag) => { + createPack(tempDir, "mixed", { + tags: [tag, "concern:testing"], + adrCount: 1, + }); + + const stack: DetectedStack = { + languages: ["typescript"], + runtimes: ["node"], + frameworks: ["nextjs"], + }; + + const recs = recommendPacksFromDir(stack, tempDir); + expect(recs).toHaveLength(1); + // Only the concern tag survives, so relevance stays medium. + expect(recs[0].matchedTags).toEqual(["concern:testing"]); + expect(recs[0].relevance).toBe("medium"); + }); + + test("skips a pack whose metadata cannot be parsed", () => { + createPack(tempDir, "healthy", { tags: ["concern:testing"], adrCount: 1 }); + const brokenDir = join(tempDir, "packs", "broken"); + mkdirSync(brokenDir, { recursive: true }); + // Missing every required field — parsePackMetadata throws a UserError. + writeFileSync(join(brokenDir, "archgate-pack.yaml"), "name: 'Not Valid'\n"); + + const stack: DetectedStack = { + languages: ["typescript"], + runtimes: [], + frameworks: [], + }; + + const recs = recommendPacksFromDir(stack, tempDir); + expect(recs.map((r) => r.packName)).toEqual(["healthy"]); + }); + test("alphabetical sort within same relevance", () => { createPack(tempDir, "zebra", { tags: ["concern:zebra"], adrCount: 1 }); createPack(tempDir, "alpha", { tags: ["concern:alpha"], adrCount: 1 }); @@ -329,6 +369,24 @@ describe("recommendPacks", () => { expect(existsSync(fakeCloneDir)).toBe(false); }); + test("a failed clone cleanup does not replace the return value", async () => { + // A NUL byte makes rmSync throw on argument validation (every platform), + // while existsSync just reports the path as absent. + mockShallowClone(async () => + join(tmpdir(), `archgate-rec-${String.fromCodePoint(0)}bad`) + ); + + const stack: DetectedStack = { + languages: ["typescript"], + runtimes: [], + frameworks: [], + }; + + // Without the try/catch in the `finally` block, the cleanup error would + // propagate and this would reject instead. + expect(await recommendPacks(stack)).toEqual([]); + }); + test("returns empty array when cloned registry has no matching packs", async () => { const fakeCloneDir = scaffoldRegistry({ tags: ["language:rust"], diff --git a/tests/helpers/paths.test.ts b/tests/helpers/paths.test.ts index 75412aeb..9a75890e 100644 --- a/tests/helpers/paths.test.ts +++ b/tests/helpers/paths.test.ts @@ -1,11 +1,13 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright 2026 Archgate -import { describe, expect, test, beforeEach, afterEach } from "bun:test"; +import { describe, expect, test, beforeEach, afterEach, spyOn } from "bun:test"; import { mkdtempSync, rmSync, mkdirSync } from "node:fs"; +import * as os from "node:os"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { findProjectRoot } from "../../src/helpers/paths"; +import { findProjectRoot, internalPath } from "../../src/helpers/paths"; +import { restoreEnv } from "../test-utils"; describe("findProjectRoot", () => { let tempDir: string; @@ -73,4 +75,59 @@ describe("findProjectRoot", () => { const result = findProjectRoot(nested); expect(result).toBeNull(); }); + + test("gives up at the ancestor-depth bound instead of walking on", () => { + mkdirSync(join(tempDir, ".archgate", "adrs"), { recursive: true }); + // Deeper than the 1000-ancestor bound. The directories need not exist — + // the walk only stats each candidate. + let deep = tempDir; + for (let i = 0; i < 1005; i++) deep = join(deep, "d"); + + // A real project root sits above, but further than the bound allows. + expect(findProjectRoot(deep)).toBeNull(); + // Fire-test the other direction: within the bound the same root is found. + expect(findProjectRoot(join(tempDir, "d"))).toBe(tempDir); + }); +}); + +describe("internalPath", () => { + let originalHome: string | undefined; + let originalUserProfile: string | undefined; + + beforeEach(() => { + originalHome = Bun.env.HOME; + originalUserProfile = Bun.env.USERPROFILE; + }); + + afterEach(() => { + restoreEnv("HOME", originalHome); + restoreEnv("USERPROFILE", originalUserProfile); + }); + + test("prefers HOME when it holds a usable value", () => { + Bun.env.HOME = join("/srv", "someone"); + expect(internalPath("cache")).toBe( + join("/srv", "someone", ".archgate", "cache") + ); + }); + + test.each(["", "undefined"])( + "falls back to os.homedir() when HOME is %p", + (badHome) => { + const fakeHome = join(tmpdir(), "archgate-fake-home"); + const homeSpy = spyOn(os, "homedir").mockReturnValue(fakeHome); + try { + // Shells and tooling surface an unset variable both ways; neither may + // reach path.join, or an ./undefined/.archgate tree appears under cwd. + Bun.env.HOME = badHome; + delete Bun.env.USERPROFILE; + + expect(internalPath("cache")).toBe( + join(fakeHome, ".archgate", "cache") + ); + } finally { + homeSpy.mockRestore(); + } + } + ); }); diff --git a/tests/helpers/platform-simulated.test.ts b/tests/helpers/platform-simulated.test.ts new file mode 100644 index 00000000..6ba9e2fc --- /dev/null +++ b/tests/helpers/platform-simulated.test.ts @@ -0,0 +1,299 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 Archgate +import { + describe, + expect, + test, + beforeEach, + afterEach, + type Mock, + spyOn, +} from "bun:test"; +import * as fs from "node:fs"; + +import { + getPlatformInfo, + isLinux, + isMacOS, + isSupportedPlatform, + isWSL, + isWindows, + resolveCommand, + getWindowsHomeDirFromWSL, + _resetAllCaches, +} from "../../src/helpers/platform"; +import { restoreEnv } from "../test-utils"; + +// `getPlatformInfo()` reads `process.platform` at call time and caches the +// answer, and Bun exposes that property as writable. Simulating it and clearing +// the cache reaches the macOS, WSL and native-Windows branches from any runner, +// which a `skipIf` gate on the real OS cannot do — a skipped test contributes +// to neither platform's coverage. + +/** The `process.platform` descriptor, captured before any simulation. */ +function platformDescriptor(): PropertyDescriptor { + return Object.getOwnPropertyDescriptor(process, "platform")!; +} + +function simulatePlatform(desc: PropertyDescriptor, runtime: string): void { + Object.defineProperty(process, "platform", { ...desc, value: runtime }); +} + +describe("platform predicates across simulated platforms", () => { + let platformDesc: PropertyDescriptor; + let savedEnv: Record; + + beforeEach(() => { + platformDesc = platformDescriptor(); + savedEnv = { + WSL_DISTRO_NAME: process.env.WSL_DISTRO_NAME, + WSL_INTEROP: process.env.WSL_INTEROP, + }; + }); + + afterEach(() => { + Object.defineProperty(process, "platform", platformDesc); + restoreEnv("WSL_DISTRO_NAME", savedEnv.WSL_DISTRO_NAME); + restoreEnv("WSL_INTEROP", savedEnv.WSL_INTEROP); + _resetAllCaches(); + }); + + // [runtime, isWindows, isMacOS, isLinux, isSupportedPlatform] + const predicateCases = [ + ["win32", true, false, false, true], + ["darwin", false, true, false, true], + ["linux", false, false, true, true], + ["freebsd", false, false, false, false], + ["aix", false, false, false, false], + ] as const; + + test.each(predicateCases)( + "reports %s as windows=%p macos=%p linux=%p supported=%p", + (runtime, windows, macos, linux, supported) => { + simulatePlatform(platformDesc, runtime); + delete process.env.WSL_DISTRO_NAME; + delete process.env.WSL_INTEROP; + _resetAllCaches(); + + expect(getPlatformInfo().runtime).toBe(runtime); + expect(isWindows()).toBe(windows); + expect(isMacOS()).toBe(macos); + expect(isLinux()).toBe(linux); + expect(isSupportedPlatform()).toBe(supported); + } + ); +}); + +describe("WSL detection via /proc/version", () => { + let platformDesc: PropertyDescriptor; + let savedEnv: Record; + let readFileSyncSpy: Mock; + + beforeEach(() => { + platformDesc = platformDescriptor(); + savedEnv = { + WSL_DISTRO_NAME: process.env.WSL_DISTRO_NAME, + WSL_INTEROP: process.env.WSL_INTEROP, + }; + readFileSyncSpy = spyOn(fs, "readFileSync"); + // Linux with no WSL environment variables: detection has to fall back to + // reading /proc/version, which is what each case below controls. + simulatePlatform(platformDesc, "linux"); + delete process.env.WSL_DISTRO_NAME; + delete process.env.WSL_INTEROP; + }); + + afterEach(() => { + readFileSyncSpy.mockRestore(); + Object.defineProperty(process, "platform", platformDesc); + restoreEnv("WSL_DISTRO_NAME", savedEnv.WSL_DISTRO_NAME); + restoreEnv("WSL_INTEROP", savedEnv.WSL_INTEROP); + _resetAllCaches(); + }); + + // The fallback exists for WSL1, which sets neither WSL_DISTRO_NAME nor + // WSL_INTEROP; the kernel banner is the only marker left, and the casing of + // "microsoft" in it has varied across releases. + const procVersions = [ + [ + "a Microsoft-branded WSL1 kernel", + "Linux version 4.4.0-19041-Microsoft", + true, + ], + [ + "a lowercase microsoft banner", + "Linux version 5.15.0-microsoft-standard", + true, + ], + [ + "a stock upstream kernel", + "Linux version 6.8.0-45-generic (buildd)", + false, + ], + ] as const; + + test.each(procVersions)("detects %s", (_label, procVersion, expected) => { + readFileSyncSpy.mockReturnValue(procVersion); + _resetAllCaches(); + + const info = getPlatformInfo(); + + expect(info.isWSL).toBe(expected); + expect(info.wslDistro).toBeNull(); + expect(isWSL()).toBe(expected); + }); + + test("treats an unreadable /proc/version as not WSL", () => { + readFileSyncSpy.mockImplementation(() => { + throw new Error( + "ENOENT: no such file or directory, open '/proc/version'" + ); + }); + _resetAllCaches(); + + expect(getPlatformInfo().isWSL).toBe(false); + }); +}); + +describe("getWindowsHomeDirFromWSL in simulated WSL", () => { + let platformDesc: PropertyDescriptor; + let savedEnv: Record; + let spawnSpy: Mock; + + beforeEach(() => { + platformDesc = platformDescriptor(); + savedEnv = { + WSL_DISTRO_NAME: process.env.WSL_DISTRO_NAME, + WSL_INTEROP: process.env.WSL_INTEROP, + }; + spawnSpy = spyOn(Bun, "spawn"); + simulatePlatform(platformDesc, "linux"); + process.env.WSL_DISTRO_NAME = "Ubuntu-22.04"; + _resetAllCaches(); + }); + + afterEach(() => { + spawnSpy.mockRestore(); + Object.defineProperty(process, "platform", platformDesc); + restoreEnv("WSL_DISTRO_NAME", savedEnv.WSL_DISTRO_NAME); + restoreEnv("WSL_INTEROP", savedEnv.WSL_INTEROP); + _resetAllCaches(); + }); + + /** Stub the `cmd.exe` probe; `wslpath -u` always converts successfully. */ + function stubCmdExe(stdout: string, exitCode: number): void { + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + spawnSpy.mockImplementation(((cmd: string[]) => { + if (cmd[0] === "wslpath") { + return { + stdout: "/mnt/c/Users/simulated\n", + stderr: "", + exited: Promise.resolve(0), + }; + } + return { stdout, stderr: "", exited: Promise.resolve(exitCode) }; + }) as unknown as typeof Bun.spawn); + } + + // [label, cmd.exe stdout, cmd.exe exit code, resolved Windows home] + const probeCases = [ + [ + "converts the USERPROFILE it prints", + "C:\\Users\\simulated\r\n", + 0, + "/mnt/c/Users/simulated", + ], + ["gives up when cmd.exe exits non-zero", "C:\\Users\\simulated", 1, null], + ["gives up when the output is blank", " \n", 0, null], + ["gives up when USERPROFILE stays unexpanded", "%USERPROFILE%\n", 0, null], + ] as const; + + test.each(probeCases)("%s", async (_label, stdout, exitCode, expected) => { + stubCmdExe(stdout, exitCode); + + expect(await getWindowsHomeDirFromWSL()).toBe(expected); + }); + + test("caches the resolved home across calls", async () => { + stubCmdExe("C:\\Users\\simulated\r\n", 0); + + const first = await getWindowsHomeDirFromWSL(); + const callsAfterFirst = spawnSpy.mock.calls.length; + const second = await getWindowsHomeDirFromWSL(); + + expect(second).toBe(first); + expect(spawnSpy.mock.calls).toHaveLength(callsAfterFirst); + }); +}); + +describe("resolveCommand on simulated native Windows", () => { + let platformDesc: PropertyDescriptor; + let spawnSpy: Mock; + let whichSpy: Mock; + + beforeEach(() => { + platformDesc = platformDescriptor(); + simulatePlatform(platformDesc, "win32"); + spawnSpy = spyOn(Bun, "spawn"); + // Nothing resolves on the native PATH, so every case reaches the WSL probe. + whichSpy = spyOn(Bun, "which").mockReturnValue(null); + _resetAllCaches(); + }); + + afterEach(() => { + whichSpy.mockRestore(); + spawnSpy.mockRestore(); + Object.defineProperty(process, "platform", platformDesc); + _resetAllCaches(); + }); + + // [label, `wsl which ` exit code, resolved command] + const wslProbeCases = [ + ["resolves the command when the wsl probe succeeds", 0, "ripgrep"], + ["returns null when the wsl probe reports failure", 1, null], + ] as const; + + test.each(wslProbeCases)("%s", async (_label, exitCode, expected) => { + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + spawnSpy.mockImplementation((() => ({ + stdout: "", + stderr: "", + exited: Promise.resolve(exitCode), + })) as unknown as typeof Bun.spawn); + + expect(await resolveCommand("ripgrep")).toBe(expected); + }); + + test("kills the wsl probe and returns null when it hangs", async () => { + let settle: ((exitCode: number) => void) | undefined; + const neverExits = new Promise((resolve) => { + settle = resolve; + }); + let killed = false; + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + spawnSpy.mockImplementation((() => ({ + stdout: "", + stderr: "", + exited: neverExits, + kill: () => { + killed = true; + }, + })) as unknown as typeof Bun.spawn); + + try { + // The production race gives the probe 3s before killing it. + expect(await resolveCommand("ripgrep")).toBeNull(); + expect(killed).toBe(true); + } finally { + settle?.(0); + } + }); + + test("returns null when spawning wsl throws", async () => { + spawnSpy.mockImplementation(() => { + throw new Error("wsl is not recognized as an internal command"); + }); + + expect(await resolveCommand("ripgrep")).toBeNull(); + }); +}); diff --git a/tests/helpers/project-config.test.ts b/tests/helpers/project-config.test.ts index 744d75a0..8197934a 100644 --- a/tests/helpers/project-config.test.ts +++ b/tests/helpers/project-config.test.ts @@ -7,6 +7,7 @@ import { join } from "node:path"; import { addCustomDomain, + ensureBaseBranch, getAllDomainNames, getConfiguredStrict, getMergedDomainPrefixes, @@ -138,6 +139,60 @@ describe("project-config", () => { expect(loadProjectConfig(projectRoot).domains.infra).toBe("INFRA"); }); + test("loadProjectConfig returns empty when valid JSON fails the schema", async () => { + // Parses fine, so the JSON.parse guard never fires — the schema check is + // the only thing standing between a hand-edited config and a bad shape. + await Bun.write( + join(projectRoot, ".archgate", "config.json"), + JSON.stringify({ domains: { infra: 42 } }) + ); + expect(loadProjectConfig(projectRoot)).toEqual({ domains: {} }); + }); + + describe("ensureBaseBranch", () => { + test("returns the configured branch without invoking the detector", async () => { + await saveProjectConfig(projectRoot, { + domains: {}, + baseBranch: "develop", + }); + let detectorCalls = 0; + const detect = async (): Promise => { + detectorCalls++; + return "main"; + }; + + expect(await ensureBaseBranch(projectRoot, detect)).toBe("develop"); + expect(detectorCalls).toBe(0); + }); + + test("persists the detected branch when unconfigured", async () => { + const result = await ensureBaseBranch(projectRoot, async () => "trunk"); + + expect(result).toBe("trunk"); + expect(loadProjectConfig(projectRoot).baseBranch).toBe("trunk"); + }); + + test("saves nothing when detection yields no branch", async () => { + const result = await ensureBaseBranch(projectRoot, async () => null); + + expect(result).toBeNull(); + expect(existsSync(join(projectRoot, ".archgate", "config.json"))).toBe( + false + ); + }); + + test("returns null when the detector throws (not a git repo)", async () => { + const result = await ensureBaseBranch(projectRoot, async () => { + throw new Error("not a git repository"); + }); + + expect(result).toBeNull(); + expect(existsSync(join(projectRoot, ".archgate", "config.json"))).toBe( + false + ); + }); + }); + describe("getConfiguredStrict", () => { test("returns null when config file is absent", () => { expect(getConfiguredStrict(projectRoot)).toBeNull(); diff --git a/tests/helpers/repo.test.ts b/tests/helpers/repo.test.ts index 9c15379e..713a7319 100644 --- a/tests/helpers/repo.test.ts +++ b/tests/helpers/repo.test.ts @@ -240,6 +240,47 @@ describe("repo helper", () => { } }); + test("prefers the remote HEAD symref over the checked-out branch", async () => { + const tempDir = mkdtempSync(join(tmpdir(), "archgate-repo-symref-")); + try { + await git(["init", "--initial-branch=feature"], tempDir); + await git(["config", "user.email", "test@test.com"], tempDir); + await git(["config", "user.name", "Test"], tempDir); + await git(["config", "commit.gpgsign", "false"], tempDir); + await git(["commit", "--allow-empty", "-m", "init"], tempDir); + await git( + ["remote", "add", "origin", "https://github.com/foo/bar.git"], + tempDir + ); + // Stand in for a `git clone`, which is what normally leaves both the + // remote-tracking ref and origin/HEAD behind. + await git(["update-ref", "refs/remotes/origin/main", "HEAD"], tempDir); + await git( + [ + "symbolic-ref", + "refs/remotes/origin/HEAD", + "refs/remotes/origin/main", + ], + tempDir + ); + + process.chdir(tempDir); + _resetRepoContextCache(); + const ctx = await getRepoContext(); + + // "origin/main" → "main", winning over the checked-out "feature". + expect(ctx.defaultBranch).toBe("main"); + expect(ctx.host).toBe("github"); + expect(ctx.owner).toBe("foo"); + expect(ctx.name).toBe("bar"); + expect(ctx.remoteUrl).toBe("https://github.com/foo/bar.git"); + expect(ctx.repoId).toBe(hashRepoId("github.com/foo/bar")); + } finally { + process.chdir(originalCwd); + safeRmSync(tempDir); + } + }); + test("returns an empty context when git cannot be spawned", async () => { const spawnSpy = spyOn(Bun, "spawn").mockImplementation(() => { throw new Error("spawn unavailable"); diff --git a/tests/helpers/sentry.test.ts b/tests/helpers/sentry.test.ts index 298cd645..ca69a46f 100644 --- a/tests/helpers/sentry.test.ts +++ b/tests/helpers/sentry.test.ts @@ -1,10 +1,21 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright 2026 Archgate -import { describe, expect, test, beforeEach, afterEach, mock } from "bun:test"; +import { + describe, + expect, + test, + beforeEach, + afterEach, + mock, + spyOn, +} from "bun:test"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { getClient } from "@sentry/node-core/light"; + +import * as telemetryConfigMod from "../../src/helpers/telemetry-config"; import { restoreEnv } from "../test-utils"; describe("sentry", () => { @@ -48,6 +59,31 @@ describe("sentry", () => { expect(initSentry()).resolves.toBeUndefined(); }); + test("an init failure leaves the SDK unarmed instead of propagating", async () => { + // The install ID is read while building the init options, inside the + // try block — a failure there must be swallowed like any other. + const installIdSpy = spyOn( + telemetryConfigMod, + "getInstallId" + ).mockImplementation(() => { + throw new Error("telemetry config unreadable"); + }); + try { + const { initSentry, captureException } = + await import("../../src/helpers/sentry"); + + await initSentry(); + + expect(installIdSpy).toHaveBeenCalled(); + // Nothing was armed, so later calls stay no-ops. + expect(() => { + captureException(new Error("after a failed init")); + }).not.toThrow(); + } finally { + installIdSpy.mockRestore(); + } + }); + test("does not initialize when telemetry is disabled", async () => { Bun.env.ARCHGATE_TELEMETRY = "0"; @@ -102,6 +138,98 @@ describe("sentry", () => { }); }); + describe("SDK-level failures", () => { + /** + * The live client after a successful init. Reading it back is the only + * way to reach the options archgate handed the SDK, and its methods are + * own-property assignable, so a failing transport can be simulated + * without mocking the module (which would leak process-wide). + */ + async function initAndGetClient() { + const { initSentry } = await import("../../src/helpers/sentry"); + await initSentry(); + const client = getClient(); + expect(client).toBeDefined(); + if (!client) throw new TypeError("Sentry client was not created"); + return client; + } + + test.each([ + ["the inquirer ExitPromptError type", { type: "ExitPromptError" }], + [ + "a SIGINT prompt-cancellation message", + { value: "User force closed the prompt with SIGINT" }, + ], + ])("beforeSend drops %s", async (_label, exceptionValue) => { + const client = await initAndGetClient(); + const { beforeSend } = client.getOptions(); + + expect( + beforeSend?.( + // `type: undefined` is ErrorEvent's discriminant against + // transaction events. + { type: undefined, exception: { values: [exceptionValue] } }, + {} + ) + ).toBeNull(); + }); + + test("beforeSend passes a genuine crash through untouched", async () => { + const client = await initAndGetClient(); + const { beforeSend } = client.getOptions(); + const event = { + type: undefined, + exception: { values: [{ type: "TypeError", value: "boom" }] }, + }; + + expect(beforeSend?.(event, {})).toBe(event); + // An event with no exception at all also survives. + expect( + beforeSend?.({ type: undefined, message: "just a log" }, {}) + ).toEqual({ type: undefined, message: "just a log" }); + }); + + test("captureException swallows a failing client", async () => { + const client = await initAndGetClient(); + const original = client.captureException.bind(client); + const { captureException } = await import("../../src/helpers/sentry"); + let captureCalls = 0; + try { + client.captureException = () => { + captureCalls++; + throw new Error("transport exploded"); + }; + + expect(() => { + captureException(new Error("boom"), { command: "check" }); + }).not.toThrow(); + expect(captureCalls).toBe(1); + } finally { + client.captureException = original; + } + }); + + test("flushSentry swallows a failing client", async () => { + const client = await initAndGetClient(); + const original = client.flush.bind(client); + const { flushSentry } = await import("../../src/helpers/sentry"); + let flushCalls = 0; + try { + client.flush = () => { + flushCalls++; + throw new Error("flush exploded"); + }; + + // A propagated failure would reject here and fail the test. + await flushSentry(50); + + expect(flushCalls).toBe(1); + } finally { + client.flush = original; + } + }); + }); + describe("flushSentry", () => { test("is a no-op when not initialized", async () => { const { flushSentry } = await import("../../src/helpers/sentry"); diff --git a/tests/helpers/session-context-copilot.test.ts b/tests/helpers/session-context-copilot.test.ts index 5376d0be..5b1e1134 100644 --- a/tests/helpers/session-context-copilot.test.ts +++ b/tests/helpers/session-context-copilot.test.ts @@ -1,7 +1,13 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright 2026 Archgate import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { + mkdirSync, + mkdtempSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; @@ -343,6 +349,61 @@ describe("readCopilotSession", () => { const preview = result.data.transcript[0]?.contentPreview ?? ""; expect(preview).toHaveLength(503); // 500 chars + "..." - expect(preview.endsWith("...")).toBe(true); + expect(preview).toEndWith("..."); + }); + + test("returns error when the state directory holds no sessions", async () => { + const result = await readCopilotSession(projectRoot); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toBe("No session directories found"); + expect(result.path).toBe(stateDir); + } + }); + + test("list surfaces the discovery error for an empty state directory", async () => { + const result = await listCopilotSessions(projectRoot); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toBe("No session directories found"); + expect(result.path).toBe(stateDir); + } + }); + + test("skips a session directory with no workspace.yaml", async () => { + mkdirSync(join(stateDir, `copilot-${uniqueId}-no-meta`), { + recursive: true, + }); + const sessionId = `copilot-${uniqueId}-with-meta`; + makeSession(sessionId, projectRoot, [ + JSON.stringify({ type: "user.message", data: { content: "kept" } }), + ]); + + const result = await listCopilotSessions(projectRoot); + expect(result.ok).toBe(true); + if (!result.ok) throw new Error("expected ok"); + expect(result.data.sessions.map((s) => s.id)).toEqual([sessionId]); + }); + + test("ignores an entry whose stat fails (dangling link)", async () => { + // A link left behind by a deleted session: readdir still lists it, but + // stat follows the link and raises ENOENT. "junction" so the link can be + // created unprivileged on Windows; the type is ignored on POSIX. + const danglingTarget = mkdtempSync(join(tmpdir(), "archgate-dangling-")); + symlinkSync( + danglingTarget, + join(stateDir, `copilot-${uniqueId}-gone`), + "junction" + ); + rmSync(danglingTarget, { recursive: true, force: true }); + const sessionId = `copilot-${uniqueId}-live`; + makeSession(sessionId, projectRoot, [ + JSON.stringify({ type: "user.message", data: { content: "still here" } }), + ]); + + const result = await listCopilotSessions(projectRoot); + expect(result.ok).toBe(true); + if (!result.ok) throw new Error("expected ok"); + expect(result.data.sessions.map((s) => s.id)).toEqual([sessionId]); }); }); diff --git a/tests/helpers/session-context-cursor.test.ts b/tests/helpers/session-context-cursor.test.ts index 7ef98ddd..2093b4b6 100644 --- a/tests/helpers/session-context-cursor.test.ts +++ b/tests/helpers/session-context-cursor.test.ts @@ -9,7 +9,13 @@ import { test, type Mock, } from "bun:test"; -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { + mkdirSync, + mkdtempSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; import * as os from "node:os"; import { join } from "node:path"; @@ -312,4 +318,30 @@ describe("readCursorSession", () => { expect(result.data.sessionId).toBe("session-good"); }); + + test("ignores an entry whose stat fails (dangling link)", async () => { + // A link left behind by a deleted session directory: readdir still lists + // it, but stat follows the link and raises ENOENT. + const danglingTarget = mkdtempSync(join(os.tmpdir(), "archgate-dangling-")); + // "junction" so the link can be created unprivileged on Windows too; + // the type argument is ignored on POSIX. + symlinkSync( + danglingTarget, + join(transcriptsDir, "session-gone"), + "junction" + ); + rmSync(danglingTarget, { recursive: true, force: true }); + makeSession("session-live", [ + JSON.stringify({ + role: "user", + message: { role: "user", content: "still here" }, + }), + ]); + + const result = await listCursorSessions(projectRoot); + expect(result.ok).toBe(true); + if (!result.ok) throw new Error("expected ok"); + + expect(result.data.sessions.map((s) => s.id)).toEqual(["session-live"]); + }); }); diff --git a/tests/helpers/session-context.test.ts b/tests/helpers/session-context.test.ts index a1c7ba21..ebf6bcb8 100644 --- a/tests/helpers/session-context.test.ts +++ b/tests/helpers/session-context.test.ts @@ -13,9 +13,12 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import * as os from "node:os"; import { join } from "node:path"; +import * as platform from "../../src/helpers/platform"; import { encodeProjectPath, + getContentPreview, listClaudeCodeSessions, + listCursorSessions, readClaudeCodeSession, readCursorSession, } from "../../src/helpers/session-context"; @@ -55,6 +58,58 @@ describe("encodeProjectPath", () => { }); }); +describe("encodeProjectPath under WSL", () => { + // The Windows-side editor writes its session directory under the Windows + // spelling of the project path, so the encoder must translate first. + let isWSLSpy: Mock; + let toWindowsPathSpy: Mock; + + beforeEach(() => { + isWSLSpy = spyOn(platform, "isWSL").mockReturnValue(true); + toWindowsPathSpy = spyOn(platform, "toWindowsPath"); + }); + + afterEach(() => { + isWSLSpy.mockRestore(); + toWindowsPathSpy.mockRestore(); + }); + + test("encodes the translated Windows path", async () => { + toWindowsPathSpy.mockResolvedValue("C:\\Users\\me\\proj"); + + expect(await encodeProjectPath("/home/me/proj")).toBe("C--Users-me-proj"); + expect(toWindowsPathSpy).toHaveBeenCalledWith("/home/me/proj"); + }); + + test.each<[string, string | null]>([ + ["null", null], + ["an empty string", ""], + ])("keeps the Linux path when wslpath yields %s", async (_label, value) => { + toWindowsPathSpy.mockResolvedValue(value); + + expect(await encodeProjectPath("/home/me/proj")).toBe("-home-me-proj"); + }); +}); + +describe("getContentPreview", () => { + test("returns an empty string when the entry carries no content", () => { + expect(getContentPreview({ type: "user", role: "user" })).toBe(""); + }); + + test("skips blocks with no recognized preview shape", () => { + const preview = getContentPreview({ + type: "assistant", + role: "assistant", + message: { + role: "assistant", + content: [{ type: "thinking", thinking: "internal" }], + }, + }); + + expect(preview).toBe(""); + }); +}); + describe("readClaudeCodeSession", () => { test("returns error when no session files found", async () => { const result = await readClaudeCodeSession("/nonexistent/path"); @@ -334,6 +389,17 @@ describe("readClaudeCodeSession", () => { }); }); +describe("listClaudeCodeSessions", () => { + test("returns error when the projects directory cannot be read", async () => { + const result = await listClaudeCodeSessions("/nonexistent/archgate/path"); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toBe("No session files found"); + expect(result.path).toContain("nonexistent"); + } + }); +}); + describe("readCursorSession", () => { test("returns error when no transcripts directory found", async () => { const result = await readCursorSession("/nonexistent/path"); @@ -347,3 +413,14 @@ describe("readCursorSession", () => { // Happy-path tests with temp home dir are in session-context-cursor.test.ts. }); + +describe("listCursorSessions", () => { + test("returns error when the transcripts directory cannot be read", async () => { + const result = await listCursorSessions("/nonexistent/archgate/path"); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toBe("No Cursor agent-transcripts directory found"); + expect(result.path).toContain("nonexistent"); + } + }); +}); diff --git a/tests/helpers/stack-detect-errors.test.ts b/tests/helpers/stack-detect-errors.test.ts new file mode 100644 index 00000000..beb74bc7 --- /dev/null +++ b/tests/helpers/stack-detect-errors.test.ts @@ -0,0 +1,157 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 Archgate +/** + * Failure paths of stack detection: unreadable or unparseable project files, + * and a disk cache that cannot be read or written. Detection is best-effort + * everywhere — a broken input must degrade the result, never throw. Kept + * separate from stack-detect.test.ts, which covers the happy paths. + */ +import { describe, expect, test, beforeEach, afterEach } from "bun:test"; +import { + existsSync, + mkdirSync, + mkdtempSync, + readdirSync, + statSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { + detectStack, + detectStackUncached, +} from "../../src/helpers/stack-detect"; +import { restoreEnv, safeRmSync } from "../test-utils"; + +describe("detectStackUncached with unreadable project files", () => { + let tempDir: string; + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), "archgate-stack-err-")); + }); + + afterEach(() => { + if (tempDir) safeRmSync(tempDir); + }); + + test("keeps the file-based signals when package.json is not valid JSON", async () => { + writeFileSync(join(tempDir, "package.json"), "{ not json"); + + const stack = await detectStackUncached(tempDir); + + // The file's presence still implies node + javascript; only the + // dependency-derived frameworks are lost. + expect(stack.runtimes).toContain("node"); + expect(stack.languages).toEqual(["javascript"]); + expect(stack.frameworks).toEqual([]); + }); + + test("detects dart but not flutter when pubspec.yaml cannot be read", async () => { + // A directory in the file's place: `existsSync` says yes, reading throws. + mkdirSync(join(tempDir, "pubspec.yaml")); + + const stack = await detectStackUncached(tempDir); + + expect(stack.languages).toContain("dart"); + expect(stack.frameworks).not.toContain("flutter"); + }); + + test("detects elixir but not phoenix when mix.exs cannot be read", async () => { + mkdirSync(join(tempDir, "mix.exs")); + + const stack = await detectStackUncached(tempDir); + + expect(stack.languages).toContain("elixir"); + expect(stack.frameworks).not.toContain("phoenix"); + }); + + test("detects python when requirements.txt cannot be read", async () => { + mkdirSync(join(tempDir, "requirements.txt")); + + const stack = await detectStackUncached(tempDir); + + expect(stack.languages).toContain("python"); + expect(stack.frameworks).toEqual([]); + }); + + test("detects python when pyproject.toml is not valid TOML", async () => { + writeFileSync(join(tempDir, "pyproject.toml"), "[project\nname = "); + + const stack = await detectStackUncached(tempDir); + + expect(stack.languages).toContain("python"); + expect(stack.frameworks).toEqual([]); + }); +}); + +describe("detectStack disk cache failures", () => { + let tempDir: string; + let homeDir: string; + let originalHome: string | undefined; + let originalUserProfile: string | undefined; + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), "archgate-stack-cache-")); + // The cache lives under ~/.archgate/cache — redirect it so no test ever + // reads or writes the real user-scope cache. + homeDir = mkdtempSync(join(tmpdir(), "archgate-stack-home-")); + originalHome = Bun.env.HOME; + originalUserProfile = Bun.env.USERPROFILE; + Bun.env.HOME = homeDir; + Bun.env.USERPROFILE = homeDir; + }); + + afterEach(() => { + restoreEnv("HOME", originalHome); + restoreEnv("USERPROFILE", originalUserProfile); + if (tempDir) safeRmSync(tempDir); + if (homeDir) safeRmSync(homeDir); + }); + + /** Wait for the fire-and-forget cache write kicked off by detectStack. */ + async function waitForCacheFile(cacheDir: string): Promise { + /* oxlint-disable no-await-in-loop -- polling is inherently sequential */ + for (let attempt = 0; attempt < 50; attempt++) { + if (existsSync(cacheDir)) { + const hit = readdirSync(cacheDir).find( + (f) => f.startsWith("stack-") && f.endsWith(".json") + ); + if (hit !== undefined) return join(cacheDir, hit); + } + await Bun.sleep(20); + } + /* oxlint-enable no-await-in-loop */ + throw new Error(`no stack cache file appeared in ${cacheDir}`); + } + + test("re-runs full detection when the cached file is corrupt", async () => { + writeFileSync(join(tempDir, "go.mod"), "module example.com/test"); + + const first = await detectStack(tempDir); + const cachePath = await waitForCacheFile( + join(homeDir, ".archgate", "cache") + ); + writeFileSync(cachePath, "{ not json"); + + // A corrupt cache is treated as a miss, not an error. + expect(await detectStack(tempDir)).toEqual(first); + expect(first.languages).toContain("go"); + }); + + test("returns a stack even when the cache cannot be written", async () => { + writeFileSync(join(tempDir, "Cargo.toml"), '[package]\nname = "t"'); + // A regular file where the cache directory belongs, so Bun.write cannot + // create the parent and the write rejects. + mkdirSync(join(homeDir, ".archgate"), { recursive: true }); + writeFileSync(join(homeDir, ".archgate", "cache"), "not a directory"); + + const stack = await detectStack(tempDir); + expect(stack.languages).toContain("rust"); + + // Let the background write settle so its rejection is observed here + // rather than escaping into a later test file. + await Bun.sleep(50); + expect(statSync(join(homeDir, ".archgate", "cache")).isFile()).toBe(true); + }); +}); diff --git a/tests/helpers/stream-guards.test.ts b/tests/helpers/stream-guards.test.ts index 27983d42..43e5e985 100644 --- a/tests/helpers/stream-guards.test.ts +++ b/tests/helpers/stream-guards.test.ts @@ -1,7 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright 2026 Archgate -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import * as exitMod from "../../src/helpers/exit"; import { handleStderrError, handleStdoutError, @@ -80,6 +81,26 @@ describe("stream-guards", () => { }); }); + describe("the default exit action", () => { + test("routes an unreplaced stdout EPIPE through exitForBrokenPipe", () => { + // Never settles: the production action fires and forgets, and a + // resolving stub would let the real exit path run under the test. + const exitSpy = spyOn(exitMod, "exitForBrokenPipe").mockImplementation( + async () => new Promise(() => {}) + ); + try { + // Drop the per-test stub so the shipped action is what runs. + _setBrokenPipeExit(null); + + handleStdoutError(makeEpipeError()); + + expect(exitSpy).toHaveBeenCalledTimes(1); + } finally { + exitSpy.mockRestore(); + } + }); + }); + describe("installStreamErrorGuards", () => { test("attaches the handlers as error listeners on both streams", () => { installStreamErrorGuards(); diff --git a/tests/helpers/telemetry-config.test.ts b/tests/helpers/telemetry-config.test.ts index b43a76aa..f9cb3ea2 100644 --- a/tests/helpers/telemetry-config.test.ts +++ b/tests/helpers/telemetry-config.test.ts @@ -98,6 +98,27 @@ describe("telemetry-config", () => { expect(config.noticeShown).toBe(true); }); + test("creates new config when the file is valid JSON of the wrong shape", async () => { + // JSON.parse succeeds, so only the schema check rejects this — a + // distinct path from the malformed-JSON case below. + const { mkdirSync } = await import("node:fs"); + const configDir = join(tempDir, ".archgate"); + mkdirSync(configDir, { recursive: true }); + await Bun.write( + join(configDir, "config.json"), + JSON.stringify({ telemetry: "yes", installId: 42 }) + ); + + const { loadTelemetryConfig } = + await import("../../src/helpers/telemetry-config"); + + const config = loadTelemetryConfig(); + expect(config.telemetry).toBe(true); + expect(config.installId).toMatch( + /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/u + ); + }); + test("creates new config when file is malformed", async () => { const { mkdirSync } = await import("node:fs"); const configDir = join(tempDir, ".archgate"); diff --git a/tests/helpers/telemetry-events.test.ts b/tests/helpers/telemetry-events.test.ts new file mode 100644 index 00000000..4396da08 --- /dev/null +++ b/tests/helpers/telemetry-events.test.ts @@ -0,0 +1,522 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 Archgate + +// Covers the telemetry paths that only run behind a live PostHog client: the +// environment detectors, common event properties, the SDK fetch wrapper, and +// flush logging. `mock.module` is process-global and retroactive, so this fake +// PostHog also serves every later in-process `initTelemetry()` — safer than the +// real SDK, which could emit into the production project. + +import { + afterEach, + beforeEach, + describe, + expect, + mock, + spyOn, + test, +} from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import * as repoModule from "../../src/helpers/repo"; +import type { RepoContext } from "../../src/helpers/repo"; +import * as sentryModule from "../../src/helpers/sentry"; +import * as telemetryModule from "../../src/helpers/telemetry"; +import { _resetConfigCache } from "../../src/helpers/telemetry-config"; +import { restoreEnv } from "../test-utils"; + +// --------------------------------------------------------------------------- +// Fake PostHog SDK +// --------------------------------------------------------------------------- + +interface CapturePayload { + distinctId: string; + event: string; + properties: Record; +} + +/** Shape the telemetry fetch wrapper resolves to on both of its branches. */ +interface WrappedResponse { + status: number; + text: () => Promise; + json: () => Promise; +} + +type PostHogFetch = (url: string, init?: unknown) => Promise; + +interface FakePostHogOptions { + host?: string; + flushAt?: number; + flushInterval?: number; + disableGeoip?: boolean; + fetch?: PostHogFetch; +} + +let constructorThrows = false; +let captureThrows = false; +let shutdownHangs = false; +const fakeClients: FakePostHog[] = []; + +class FakePostHog { + readonly options: FakePostHogOptions; + readonly captures: CapturePayload[] = []; + shutdownCount = 0; + + constructor(_apiKey: string, options: FakePostHogOptions) { + if (constructorThrows) throw new Error("PostHog constructor failed"); + this.options = options; + fakeClients.push(this); + } + + capture(payload: CapturePayload): void { + if (captureThrows) throw new Error("PostHog capture failed"); + this.captures.push(payload); + } + + async shutdown(): Promise { + this.shutdownCount++; + if (shutdownHangs) { + await new Promise(() => { + // Never settles — the flush timeout is what resolves the race. + }); + } + } +} + +void mock.module("posthog-node", () => ({ PostHog: FakePostHog })); + +// --------------------------------------------------------------------------- +// Environment managed by this file +// --------------------------------------------------------------------------- + +const MANAGED_ENV_KEYS = [ + "CI", + "GITHUB_ACTIONS", + "GITLAB_CI", + "CIRCLECI", + "TRAVIS", + "BUILDKITE", + "JENKINS_URL", + "JENKINS_HOME", + "BITBUCKET_BUILD_NUMBER", + "TF_BUILD", + "TEAMCITY_VERSION", + "CODEBUILD_BUILD_ID", + "SHELL", + "PSModulePath", + "ComSpec", + "LANG", + "HOME", + "ARCHGATE_TELEMETRY", + "NODE_ENV", +]; + +const FAKE_REPO: RepoContext = { + isGit: true, + host: "github", + owner: "archgate", + name: "cli", + repoId: "0f1e2d3c4b5a6978", + remoteUrl: "https://github.com/archgate/cli.git", + defaultBranch: "main", +}; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function fakeClient(): FakePostHog { + const client = fakeClients.at(-1); + if (client === undefined) throw new Error("PostHog was never constructed"); + return client; +} + +function fetchWrapper(): PostHogFetch { + const wrapper = fakeClient().options.fetch; + if (wrapper === undefined) + throw new Error("PostHog options carried no fetch wrapper"); + return wrapper; +} + +function lastProperties(): Record { + const last = fakeClient().captures.at(-1); + if (last === undefined) throw new Error("no PostHog capture was recorded"); + return last.properties; +} + +/** Initialize against the fake SDK, emit one event, read its properties. */ +async function captureProperties(): Promise> { + await telemetryModule.initTelemetry(); + telemetryModule.trackEvent("telemetry_events_probe"); + return lastProperties(); +} + +/** A `globalThis.fetch` stand-in carrying the `preconnect` member its type requires. */ +function stubFetch(impl: () => Promise): typeof globalThis.fetch { + return Object.assign(impl, { + preconnect: () => { + // Present only to satisfy the fetch type; no test calls it. + }, + }); +} + +/** Redefine a member of a global namespace, keeping it configurable for restore. */ +function overrideGlobal(target: object, key: string, value: unknown): void { + Object.defineProperty(target, key, { + configurable: true, + writable: true, + value, + }); +} + +describe("telemetry events", () => { + let tempDir: string; + const savedEnv = new Map(); + let sentrySpy: ReturnType< + typeof spyOn + >; + let repoSpy: ReturnType>; + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), "archgate-telemetry-events-")); + for (const key of MANAGED_ENV_KEYS) { + savedEnv.set(key, Bun.env[key]); + delete Bun.env[key]; + } + Bun.env.HOME = tempDir; + // NODE_ENV is deliberately not "test": `isTestEnvironment()` otherwise + // short-circuits `trackEvent` before any of these paths run. Safe only + // because the PostHog SDK is faked — see the file header. + Bun.env.NODE_ENV = "telemetry-events"; + + constructorThrows = false; + captureThrows = false; + shutdownHangs = false; + fakeClients.length = 0; + + repoSpy = spyOn(repoModule, "getRepoContext").mockResolvedValue(FAKE_REPO); + sentrySpy = spyOn(sentryModule, "captureException").mockImplementation( + () => { + // Keep the real Sentry transport out of the test process. + } + ); + + telemetryModule._resetTelemetry(); + _resetConfigCache(); + }); + + afterEach(() => { + telemetryModule._resetTelemetry(); + _resetConfigCache(); + mock.restore(); + for (const [key, value] of savedEnv) restoreEnv(key, value); + savedEnv.clear(); + rmSync(tempDir, { recursive: true, force: true }); + }); + + describe("client construction", () => { + test("configures the archgate proxy host and explicit flush settings", async () => { + await telemetryModule.initTelemetry(); + + const { options } = fakeClient(); + expect(options.host).toBe("https://n.archgate.dev"); + expect(options.disableGeoip).toBe(false); + expect(options.flushAt).toBe(20); + expect(options.flushInterval).toBe(0); + }); + + test("leaves the client null when the SDK constructor throws", async () => { + constructorThrows = true; + + await telemetryModule.initTelemetry(); + + expect(telemetryModule._getClient()).toBeNull(); + expect(() => { + telemetryModule.trackEvent("never_captured"); + }).not.toThrow(); + }); + }); + + describe("fetch wrapper", () => { + const url = "https://n.archgate.dev/batch/"; + let originalFetch: typeof globalThis.fetch; + + beforeEach(() => { + originalFetch = globalThis.fetch; + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + }); + + test("passes the real response through when the network is reachable", async () => { + globalThis.fetch = stubFetch( + async () => new Response("delegated", { status: 201 }) + ); + await telemetryModule.initTelemetry(); + + const response = await fetchWrapper()(url, { method: "POST" }); + + expect(response.status).toBe(201); + expect(await response.text()).toBe("delegated"); + expect(sentrySpy).not.toHaveBeenCalled(); + }); + + test("returns a synthetic 200 and reports to Sentry when fetch throws", async () => { + const networkError = new Error("ECONNREFUSED"); + globalThis.fetch = stubFetch(() => { + throw networkError; + }); + await telemetryModule.initTelemetry(); + + const response = await fetchWrapper()(url, { method: "POST" }); + + expect(response.status).toBe(200); + expect(await response.text()).toBe("ok"); + expect(await response.json()).toEqual({}); + expect(sentrySpy).toHaveBeenCalledWith(networkError, { + source: "posthog-fetch", + url, + }); + }); + }); + + describe("common event properties", () => { + test("sends CLI and runtime identity with a null $ip", async () => { + const props = await captureProperties(); + + expect(props).toContainKeys([ + "cli_version", + "os", + "arch", + "bun_version", + "install_method", + "is_tty", + "is_wsl", + ]); + expect(props.bun_version).toBe(Bun.version); + expect(props.arch).toBe(process.arch); + expect(props.$ip).toBeNull(); + }); + + test("counts the ADRs of the surrounding archgate project", async () => { + const props = await captureProperties(); + + expect(props.has_project).toBe(true); + expect(props.adr_count).toBeGreaterThan(0); + expect(props.adr_with_rules_count).toBeGreaterThan(0); + expect(props.adr_domains_count).toBeGreaterThan(0); + }); + + test("carries the repo context resolved during init", async () => { + const props = await captureProperties(); + + expect(props.repo_is_git).toBe(true); + expect(props.repo_host).toBe("github"); + expect(props.repo_id).toBe(FAKE_REPO.repoId); + expect(props.git_default_branch).toBe("main"); + // Raw identity ships only on `project_initialized`, never per-event. + expect(props).not.toContainKey("remote_url"); + expect(props).not.toContainKey("repo_owner"); + expect(props).not.toContainKey("repo_name"); + }); + + test("nulls the repo fields when repo-context resolution rejects", async () => { + repoSpy.mockRejectedValue(new Error("git is not installed")); + + const props = await captureProperties(); + + expect(props.repo_is_git).toBe(false); + expect(props.repo_host).toBeNull(); + expect(props.repo_id).toBeNull(); + expect(props.git_default_branch).toBeNull(); + }); + + test("memoizes the static snapshot across events", async () => { + const first = await captureProperties(); + Bun.env.GITLAB_CI = "true"; + telemetryModule.trackEvent("telemetry_events_probe_2"); + const second = lastProperties(); + + expect(second.ci_provider).toBe(first.ci_provider); + expect(fakeClient().captures).toHaveLength(2); + }); + }); + + describe("ci_provider detection", () => { + test.each([ + { + envKey: "GITHUB_ACTIONS", + envValue: "true", + expected: "github-actions", + }, + { envKey: "GITLAB_CI", envValue: "true", expected: "gitlab-ci" }, + { envKey: "CIRCLECI", envValue: "true", expected: "circleci" }, + { envKey: "TRAVIS", envValue: "true", expected: "travis" }, + { envKey: "BUILDKITE", envValue: "true", expected: "buildkite" }, + { envKey: "JENKINS_URL", envValue: "http://ci", expected: "jenkins" }, + { envKey: "JENKINS_HOME", envValue: "/jenkins", expected: "jenkins" }, + { + envKey: "BITBUCKET_BUILD_NUMBER", + envValue: "42", + expected: "bitbucket-pipelines", + }, + { envKey: "TF_BUILD", envValue: "True", expected: "azure-pipelines" }, + { envKey: "TEAMCITY_VERSION", envValue: "2024.03", expected: "teamcity" }, + { + envKey: "CODEBUILD_BUILD_ID", + envValue: "build:1", + expected: "aws-codebuild", + }, + { envKey: "CI", envValue: "1", expected: "other" }, + ])( + "classifies $envKey as $expected", + async ({ envKey, envValue, expected }) => { + Bun.env[envKey] = envValue; + + const props = await captureProperties(); + + expect(props.ci_provider).toBe(expected); + } + ); + + test("reports a null provider outside CI", async () => { + const props = await captureProperties(); + + expect(props.ci_provider).toBeNull(); + expect(props.is_ci).toBe(false); + }); + + test("reports is_ci true when CI is set", async () => { + Bun.env.CI = "true"; + + const props = await captureProperties(); + + expect(props.is_ci).toBe(true); + }); + + test("ignores an empty CI variable", async () => { + Bun.env.GITHUB_ACTIONS = ""; + + const props = await captureProperties(); + + expect(props.ci_provider).toBeNull(); + }); + }); + + describe("shell detection", () => { + test.each([ + { envKey: "SHELL", envValue: "/usr/bin/zsh", expected: "zsh" }, + { + envKey: "PSModulePath", + envValue: "/ps/Modules", + expected: "powershell", + }, + { + envKey: "ComSpec", + envValue: "/windows/system32/CMD.EXE", + expected: "cmd.exe", + }, + ])( + "derives the shell from $envKey", + async ({ envKey, envValue, expected }) => { + Bun.env[envKey] = envValue; + + const props = await captureProperties(); + + expect(props.shell).toBe(expected); + } + ); + + test("reports a null shell when no shell variable is set", async () => { + const props = await captureProperties(); + + expect(props.shell).toBeNull(); + }); + }); + + describe("locale detection", () => { + let originalDateTimeFormat: typeof Intl.DateTimeFormat; + + beforeEach(() => { + originalDateTimeFormat = Intl.DateTimeFormat; + }); + + afterEach(() => { + overrideGlobal(Intl, "DateTimeFormat", originalDateTimeFormat); + }); + + function breakIntl(): void { + overrideGlobal(Intl, "DateTimeFormat", () => { + throw new Error("Intl unavailable"); + }); + } + + test("reports the resolved Intl locale", async () => { + const props = await captureProperties(); + + expect(props.locale).toBe(Intl.DateTimeFormat().resolvedOptions().locale); + }); + + test("falls back to LANG when Intl is unavailable", async () => { + breakIntl(); + Bun.env.LANG = "pt_BR.UTF-8"; + + const props = await captureProperties(); + + expect(props.locale).toBe("pt_BR.UTF-8"); + }); + + test("falls back to null when Intl is unavailable and LANG is unset", async () => { + breakIntl(); + + const props = await captureProperties(); + + expect(props.locale).toBeNull(); + }); + }); + + describe("trackEvent", () => { + test("merges caller properties over the common ones", async () => { + await telemetryModule.initTelemetry(); + telemetryModule.trackCommand("check", { json: true }); + + const captured = fakeClient().captures.at(-1); + expect(captured?.event).toBe("command_executed"); + expect(captured?.properties.command).toBe("check"); + expect(captured?.properties.json).toBe(true); + expect(captured?.distinctId).toBeString(); + }); + + test("swallows a capture failure", async () => { + await telemetryModule.initTelemetry(); + captureThrows = true; + + expect(() => { + telemetryModule.trackEvent("capture_explodes"); + }).not.toThrow(); + }); + }); + + describe("flushTelemetry", () => { + test("shuts the client down once", async () => { + await telemetryModule.initTelemetry(); + const client = fakeClient(); + + await telemetryModule.flushTelemetry(); + + expect(client.shutdownCount).toBe(1); + }); + + test("resolves via the timeout when shutdown hangs", async () => { + await telemetryModule.initTelemetry(); + shutdownHangs = true; + + await telemetryModule.flushTelemetry(10); + + expect(fakeClient().shutdownCount).toBe(1); + }); + }); +}); diff --git a/tests/helpers/vscode-settings.test.ts b/tests/helpers/vscode-settings.test.ts index a7e980aa..ea641a12 100644 --- a/tests/helpers/vscode-settings.test.ts +++ b/tests/helpers/vscode-settings.test.ts @@ -364,3 +364,136 @@ describe("getVscodeUserSettingsPath", () => { } ); }); + +/** + * `getVscodeUserSettingsPath()` branches on `src/helpers/platform.ts`, which + * resolves `process.platform` at call time. Bun exposes that as a writable + * property, so simulating it and clearing the platform cache reaches the macOS + * and WSL branches from any runner. + */ +describe("getVscodeUserSettingsPath branch matrix", () => { + const HOME = "/simulated-home"; + const APP_DATA = "/simulated-appdata"; + const WIN_HOME = "/mnt/c/Users/simulated"; + + let platformDesc: PropertyDescriptor; + let savedEnv: Record; + let homedirSpy: Mock; + let spawnSpy: Mock; + + beforeEach(() => { + platformDesc = Object.getOwnPropertyDescriptor(process, "platform")!; + savedEnv = { + APPDATA: process.env.APPDATA, + WSL_DISTRO_NAME: process.env.WSL_DISTRO_NAME, + WSL_INTEROP: process.env.WSL_INTEROP, + }; + homedirSpy = spyOn(os, "homedir").mockReturnValue(HOME); + spawnSpy = spyOn(Bun, "spawn"); + }); + + afterEach(() => { + Object.defineProperty(process, "platform", platformDesc); + spawnSpy.mockRestore(); + homedirSpy.mockRestore(); + restoreEnvAll(savedEnv); + _resetAllCaches(); + }); + + /** + * Stub `cmd.exe` and `wslpath`, the two subprocesses + * `getWindowsHomeDirFromWSL()` shells out to. `resolvable: false` makes the + * first one fail, which is the "Windows home not resolvable" fall-through. + */ + function stubWslSubprocesses(resolvable: boolean): void { + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + spawnSpy.mockImplementation(((cmd: string[]) => { + if (cmd[0] === "wslpath") { + return { + stdout: WIN_HOME + "\n", + stderr: "", + exited: Promise.resolve(0), + }; + } + return { + stdout: resolvable ? "C:\\Users\\simulated\r\n" : "", + stderr: "", + exited: Promise.resolve(resolvable ? 0 : 1), + }; + }) as unknown as typeof Bun.spawn); + } + + // [label, simulated platform, APPDATA, WSL_DISTRO_NAME, Windows home + // resolvable, expected path with "/" separators] + const cases = [ + [ + "win32 with APPDATA set", + "win32", + APP_DATA, + null, + false, + `${APP_DATA}/Code/User/settings.json`, + ], + [ + "win32 with APPDATA unset", + "win32", + null, + null, + false, + `${HOME}/AppData/Roaming/Code/User/settings.json`, + ], + [ + "macOS", + "darwin", + null, + null, + false, + `${HOME}/Library/Application Support/Code/User/settings.json`, + ], + [ + "WSL with a resolvable Windows home", + "linux", + null, + "Ubuntu-22.04", + true, + `${WIN_HOME}/AppData/Roaming/Code/User/settings.json`, + ], + [ + "WSL without a resolvable Windows home", + "linux", + null, + "Ubuntu-22.04", + false, + `${HOME}/.config/Code/User/settings.json`, + ], + [ + "plain Linux", + "linux", + null, + null, + false, + `${HOME}/.config/Code/User/settings.json`, + ], + ] as const; + + test.each(cases)( + "resolves the settings path on %s", + async (_label, platform, appData, wslDistro, resolvable, expected) => { + Object.defineProperty(process, "platform", { + ...platformDesc, + value: platform, + }); + delete process.env.WSL_INTEROP; + if (appData === null) delete process.env.APPDATA; + else process.env.APPDATA = appData; + if (wslDistro === null) delete process.env.WSL_DISTRO_NAME; + else process.env.WSL_DISTRO_NAME = wslDistro; + stubWslSubprocesses(resolvable); + _resetAllCaches(); + + const path = await getVscodeUserSettingsPath(); + + expect(path.replaceAll("\\", "/")).toBe(expected); + } + ); +}); From 6140bc17223295b9863fe58bd25ee49e557823ba Mon Sep 17 00:00:00 2001 From: Rhuan Barreto Date: Wed, 5 Aug 2026 23:28:29 +0200 Subject: [PATCH 2/4] ci: raise the coverage floor to 99.5% MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merged Linux+Windows line coverage is 99.9% (9719/9729) as of the phase 3 test additions, up from 96.6%. The gate and ARCH-005's stated target move together so the ADR and the workflow agree. The floor sits below the measured figure by ~39 lines of slack rather than tracking it exactly, because a residue of lines cannot be covered by any test. Bun emits a never-incrementing lcov record for some structural tokens: a `} catch {` whose body has its own statements, and — on Linux only — blank lines, comments, and closing braces that the Windows run omits from its records entirely, so the union pins them at zero. Ten such lines remain across `src/`. Both the action input and the enforcing step compare with awk, so the decimal threshold is handled as a float in each. Refs #522 Signed-off-by: Rhuan Barreto --- .archgate/adrs/ARCH-005-testing-standards.md | 8 ++++---- .github/workflows/code-pull-request.yml | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.archgate/adrs/ARCH-005-testing-standards.md b/.archgate/adrs/ARCH-005-testing-standards.md index b85a4c45..68705481 100644 --- a/.archgate/adrs/ARCH-005-testing-standards.md +++ b/.archgate/adrs/ARCH-005-testing-standards.md @@ -20,7 +20,7 @@ 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:** @@ -28,7 +28,7 @@ Use Bun's built-in test runner (`bun test`) for all tests. Test files live in `t 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** — `.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. @@ -166,7 +166,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 +182,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 diff --git a/.github/workflows/code-pull-request.yml b/.github/workflows/code-pull-request.yml index 2d945bce..b0ff52fa 100644 --- a/.github/workflows/code-pull-request.yml +++ b/.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" \ From 4bd16c0c2e0c493a34b35dc91708fe4b8d4c644c Mon Sep 17 00:00:00 2001 From: Rhuan Barreto Date: Wed, 5 Aug 2026 23:34:16 +0200 Subject: [PATCH 3/4] docs(adrs): correct ARCH-009's platform-testing rationale and record the simulation idiom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## ARCH-009 The Context listed "cannot be tested" among the costs of reading `process.platform` directly, on the grounds that the property is read-only in Bun. It is not: the descriptor reports `writable: true, configurable: true`, and tests override it with `Object.defineProperty` to reach every platform branch from any runner. The bullet now states the cost that is real. Simulating a platform means overriding the property AND clearing whatever the reading module cached from it, so routing through the helper lets one seam cover every consumer, while direct reads need an override and a cache reset per call site — and a site that captures the value at module load cannot be re-simulated at all. The Decision is unchanged; it rests on this plus the scattered-logic and missed-WSL rationales already recorded. ## ARCH-005 Implementation Pattern gains the simulation idiom, which removes the need to add testability seams to production code: capture the descriptor, override it, reset the dependent cache, and restore the descriptor in a hook so it cannot leak into later files in the shared process. It also records why the idiom is the only one that moves the number. CI merges Linux and Windows runs only, so a `skipIf`-gated darwin test contributes to neither; parametrizing over the matrix runs every case on every runner. Both edits land outside the sections `review-context` briefs, so the briefing budget is unaffected. Refs #522 Signed-off-by: Rhuan Barreto --- .archgate/adrs/ARCH-005-testing-standards.md | 43 +++++++++++++++++++ .../ARCH-009-platform-detection-helper.md | 2 +- .../agent-memory/archgate-developer/MEMORY.md | 2 +- 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/.archgate/adrs/ARCH-005-testing-standards.md b/.archgate/adrs/ARCH-005-testing-standards.md index 68705481..c56015f1 100644 --- a/.archgate/adrs/ARCH-005-testing-standards.md +++ b/.archgate/adrs/ARCH-005-testing-standards.md @@ -144,6 +144,49 @@ 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, expect, afterEach } from "bun:test"; + +import { _resetAllCaches } from "../../src/helpers/platform"; + +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 diff --git a/.archgate/adrs/ARCH-009-platform-detection-helper.md b/.archgate/adrs/ARCH-009-platform-detection-helper.md index 839b0fa2..b8ba2a6b 100644 --- a/.archgate/adrs/ARCH-009-platform-detection-helper.md +++ b/.archgate/adrs/ARCH-009-platform-detection-helper.md @@ -16,7 +16,7 @@ The `src/helpers/platform.ts` module already provides a centralized, cached API 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 (`_resetPlatformCache()`) 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 diff --git a/.claude/agent-memory/archgate-developer/MEMORY.md b/.claude/agent-memory/archgate-developer/MEMORY.md index 87dd3488..44a4fef5 100644 --- a/.claude/agent-memory/archgate-developer/MEMORY.md +++ b/.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 `-.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 -n coverage-linux -D a -n coverage-windows -D b`, then union the `DA:,` 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 -n coverage-linux -D a -n coverage-windows -D b`, then union the `DA:,` 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=`) 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. From 64abea11fe572b0c5383d436c2c4006f33ff9faf Mon Sep 17 00:00:00 2001 From: Rhuan Barreto Date: Wed, 5 Aug 2026 23:52:26 +0200 Subject: [PATCH 4/4] test: address review findings and correct two stale references in ARCH-009 ## Tests - `review-context-strict.test.ts` samples the stdout call count inside the `exitWith` stub, so the ordering contract is pinned at the moment the exit is requested rather than inferred from final mock state. - `ast-support-errors.test.ts` counts `proc.kill()` calls while the real subprocess still spawns, so the probe's cleanup is observed. - `pack.test.ts` asserts the empty-`maintainers` error too; the test claims to list every schema failure and asserted two of three. - `adr-import-failures.test.ts` uses two sources, resolving the first target and rejecting the second, so cleanup after an earlier success is covered. - `editor-detect.test.ts` marks an editor available that is neither first in the fixture nor the fallback; both assertions previously expected the same value, so the test passed whether or not detection worked. - `install-info.test.ts` collapses four identical-shape classification cases into one `test.each` table (ARCH-025), keeping the `PROTO_HOME` and caching cases separate for their extra setup and assertions. - Comments in `adr-writer`, `exit`, `session-context-copilot` and `session-context-cursor` tests state current behavior (GEN-004). ## ADRs ARCH-009 named `_resetPlatformCache()` in four places. `platform.ts` exports `_resetAllCaches()`, and that is its only `_reset*` export; every reference now uses the real name. The ARCH-005 example imports `it` and `getVscodeUserSettingsPath`, which its body uses. ## Memory The artifact-download recipe passed `-D` twice. `-n` repeats but `-D` does not, so the second silently overrides the first; a single `-D` now takes both artifacts, which land under distinct subdirectories. Refs #522 Signed-off-by: Rhuan Barreto --- .archgate/adrs/ARCH-005-testing-standards.md | 3 +- .../ARCH-009-platform-detection-helper.md | 8 +-- .../agent-memory/archgate-developer/MEMORY.md | 2 +- tests/commands/review-context-strict.test.ts | 20 +++++-- tests/engine/ast-support-errors.test.ts | 35 +++++++++++ tests/formats/pack.test.ts | 3 + tests/helpers/adr-import-failures.test.ts | 39 +++++++----- tests/helpers/adr-writer.test.ts | 3 +- tests/helpers/editor-detect.test.ts | 8 ++- tests/helpers/exit.test.ts | 3 +- tests/helpers/install-info.test.ts | 60 +++++++++---------- tests/helpers/session-context-copilot.test.ts | 6 +- tests/helpers/session-context-cursor.test.ts | 4 +- 13 files changed, 125 insertions(+), 69 deletions(-) diff --git a/.archgate/adrs/ARCH-005-testing-standards.md b/.archgate/adrs/ARCH-005-testing-standards.md index c56015f1..b181afce 100644 --- a/.archgate/adrs/ARCH-005-testing-standards.md +++ b/.archgate/adrs/ARCH-005-testing-standards.md @@ -157,9 +157,10 @@ contributes to neither and covers nothing. Parametrize over the matrix ```typescript // tests/helpers/vscode-settings.test.ts -import { describe, expect, afterEach } from "bun:test"; +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"); diff --git a/.archgate/adrs/ARCH-009-platform-detection-helper.md b/.archgate/adrs/ARCH-009-platform-detection-helper.md index b8ba2a6b..267222ee 100644 --- a/.archgate/adrs/ARCH-009-platform-detection-helper.md +++ b/.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"`). -- **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 (`_resetPlatformCache()`) 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. +- **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 diff --git a/.claude/agent-memory/archgate-developer/MEMORY.md b/.claude/agent-memory/archgate-developer/MEMORY.md index 44a4fef5..1c53d7e3 100644 --- a/.claude/agent-memory/archgate-developer/MEMORY.md +++ b/.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 `-.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 -n coverage-linux -D a -n coverage-windows -D b`, then union the `DA:,` 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=`) 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%. +- **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 -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:,` 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=`) 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. diff --git a/tests/commands/review-context-strict.test.ts b/tests/commands/review-context-strict.test.ts index 2a2a0162..c389f4b9 100644 --- a/tests/commands/review-context-strict.test.ts +++ b/tests/commands/review-context-strict.test.ts @@ -55,6 +55,8 @@ describe("review-context --strict output ordering", () => { let logSpy: Mock; let warnSpy: Mock; let exitSpy: Mock; + /** How many stdout writes had happened when the exit was requested. */ + let logCallsAtExit: number; beforeEach(() => { tempDir = mkdtempSync(join(tmpdir(), "archgate-review-strict-test-")); @@ -64,14 +66,19 @@ describe("review-context --strict output ordering", () => { Bun.env.ARCHGATE_PROJECT_CEILING = tempDir; logSpy = spyOn(console, "log").mockImplementation(() => {}); warnSpy = spyOn(logModule, "logWarn").mockImplementation(() => {}); + logCallsAtExit = -1; exitSpy = spyOn(exitModule, "exitWith").mockImplementation( - async (): Promise => + async (): Promise => { + // Sampling the stdout spy here is what makes the ordering observable: + // the count is taken at the moment of the exit request, not after + // parseAsync has unwound. + logCallsAtExit = logSpy.mock.calls.length; // exitWith is `Promise` because it terminates the process. A // resolving stub hands control back to the action instead, which is - // what lets the assertions below check that stdout is complete and - // final at the moment the exit is requested. + // what lets the assertions below run at all. // oxlint-disable-next-line typescript/no-unsafe-type-assertion - undefined as unknown as never + return undefined as unknown as never; + } ); }); @@ -106,8 +113,9 @@ describe("review-context --strict output ordering", () => { "--strict: failing because" ); - // Exactly one stdout write, and it is the complete JSON context — the - // strict failure adds nothing to stdout and truncates nothing from it. + // The payload is already on stdout when the exit is requested, and it is + // the only write — the strict failure adds nothing to stdout afterwards. + expect(logCallsAtExit).toBe(1); expect(logSpy).toHaveBeenCalledTimes(1); const payload = expectKeys( JSON.parse(String(logSpy.mock.calls[0][0])), diff --git a/tests/engine/ast-support-errors.test.ts b/tests/engine/ast-support-errors.test.ts index b9e04bcf..905d7110 100644 --- a/tests/engine/ast-support-errors.test.ts +++ b/tests/engine/ast-support-errors.test.ts @@ -58,14 +58,49 @@ describe("probeInterpreter timeout", () => { }; } + /** + * Wrap `Bun.spawn` so the probe still spawns its real subprocess while every + * `kill()` made against that subprocess is counted. Counting is the only way + * to observe the cleanup — the timed-out candidate is otherwise indistinguishable + * from one that simply exited non-zero. + */ + function countProbeKills(): { kills: () => number; restore: () => void } { + const realSpawn = Bun.spawn; + let killCount = 0; + const spy = spyOn(Bun, "spawn"); + // The wrapper forwards to the real spawn; Bun.spawn's overload set is not + // expressible in a single mock implementation signature. + // oxlint-disable-next-line typescript/no-unsafe-type-assertion + spy.mockImplementation(((...args: Parameters) => { + const proc = realSpawn(...args); + const realKill = proc.kill.bind(proc); + proc.kill = (...killArgs: Parameters): void => { + killCount++; + realKill(...killArgs); + }; + return proc; + }) as unknown as typeof Bun.spawn); + return { + kills: () => killCount, + restore: () => { + spy.mockRestore(); + }, + }; + } + test("kills a candidate that outlives the probe timeout and moves on", async () => { // process.execPath is the running bun binary — it exists and would // normally answer `--version`, so only the timeout can reject it. const restoreTimer = installImmediateProbeTimer(); + const spawns = countProbeKills(); try { expect(await probeInterpreter([process.execPath])).toBeNull(); + // The subprocess is terminated rather than left running for the rest of + // the check. + expect(spawns.kills()).toBe(1); } finally { + spawns.restore(); restoreTimer(); } }); diff --git a/tests/formats/pack.test.ts b/tests/formats/pack.test.ts index 2b54cf7d..82971879 100644 --- a/tests/formats/pack.test.ts +++ b/tests/formats/pack.test.ts @@ -156,6 +156,9 @@ maintainers: [] expect(() => parsePackMetadata(yaml)).toThrow(/Invalid pack metadata:/u); expect(() => parsePackMetadata(yaml)).toThrow(/lowercase kebab-case/u); expect(() => parsePackMetadata(yaml)).toThrow(/version must be semver/u); + expect(() => parsePackMetadata(yaml)).toThrow( + /maintainers: Too small: expected array to have >=1 items/u + ); }); test("throws when the document is not a mapping at all", () => { diff --git a/tests/helpers/adr-import-failures.test.ts b/tests/helpers/adr-import-failures.test.ts index a378ee39..82e68d1b 100644 --- a/tests/helpers/adr-import-failures.test.ts +++ b/tests/helpers/adr-import-failures.test.ts @@ -68,26 +68,37 @@ describe("resolveAndCloneSources — abort partway through", () => { }); test("removes every clone already made when a later source fails", async () => { - const cloneDir = mkdtempSync(join(tmpdir(), "archgate-clone-")); - cloneDirs.push(cloneDir); - - const cloneSpy = spyOn(registry, "shallowClone").mockResolvedValue( - cloneDir - ); - const targetSpy = spyOn(registry, "detectTarget").mockRejectedValue( - new Error("no ADRs found at subpath") - ); + // Two sources resolving to different repos, so each gets its own clone + // rather than a cache hit. The first completes; the second aborts, which + // is what puts an already-successful clone in the cleanup's path. + const firstClone = mkdtempSync(join(tmpdir(), "archgate-clone-a-")); + const secondClone = mkdtempSync(join(tmpdir(), "archgate-clone-b-")); + cloneDirs.push(firstClone, secondClone); + + const cloneSpy = spyOn(registry, "shallowClone") + .mockResolvedValueOnce(firstClone) + .mockResolvedValueOnce(secondClone); + const targetSpy = spyOn(registry, "detectTarget") + .mockResolvedValueOnce({ + kind: "single-adr", + adrFile: join(firstClone, "ADR-001.md"), + rulesFile: null, + baseDir: firstClone, + }) + .mockRejectedValueOnce(new Error("no ADRs found at subpath")); try { const message = await rejectionMessage( - resolveAndCloneSources(["packs/example"]) + resolveAndCloneSources(["packs/example", "acme/adrs/backend"]) ); expect(message).toBe("no ADRs found at subpath"); - // The clone is transient scratch space — a failed run must not leave it - // behind, since nobody downstream still holds a handle to it. - expect(existsSync(cloneDir)).toBe(false); - expect(cloneSpy).toHaveBeenCalledTimes(1); + // Clones are transient scratch space — a failed run must not leave any + // behind, including the ones made before the failing source. + expect(existsSync(firstClone)).toBe(false); + expect(existsSync(secondClone)).toBe(false); + expect(cloneSpy).toHaveBeenCalledTimes(2); + expect(targetSpy).toHaveBeenCalledTimes(2); } finally { targetSpy.mockRestore(); cloneSpy.mockRestore(); diff --git a/tests/helpers/adr-writer.test.ts b/tests/helpers/adr-writer.test.ts index ce4c3e22..515c557a 100644 --- a/tests/helpers/adr-writer.test.ts +++ b/tests/helpers/adr-writer.test.ts @@ -179,8 +179,7 @@ describe("createAdrFile", () => { }); test("rejects a prefix that resolves to nothing", async () => { - // `resolveDomainPrefix` hands the caller a prefix for custom domains; an - // empty one means the domain was never registered, so no ID can be built. + // An empty explicit prefix carries no ID stem, so no ADR ID can be built. expect( await rejectionMessage( createAdrFile(tempDir, { diff --git a/tests/helpers/editor-detect.test.ts b/tests/helpers/editor-detect.test.ts index 95bd91e4..1fa0779b 100644 --- a/tests/helpers/editor-detect.test.ts +++ b/tests/helpers/editor-detect.test.ts @@ -126,9 +126,13 @@ describe("editor-detect", () => { }); test("defaults to the first detected editor", async () => { - await promptSingleEditorSelection(MOCK_DETECTED); + // vscode is neither first in the list nor the no-detection fallback, so + // only "first available wins" produces it. + await promptSingleEditorSelection( + MOCK_DETECTED.map((e) => ({ ...e, available: e.id === "vscode" })) + ); - expect(questionFrom(mockPrompt.mock.calls[0]).default).toBe("claude"); + expect(questionFrom(mockPrompt.mock.calls[0]).default).toBe("vscode"); }); test("defaults to claude when nothing is detected", async () => { diff --git a/tests/helpers/exit.test.ts b/tests/helpers/exit.test.ts index b1aacff4..db11aa9e 100644 --- a/tests/helpers/exit.test.ts +++ b/tests/helpers/exit.test.ts @@ -211,8 +211,7 @@ describe("exit helper", () => { test("falls back to the 'root' command name when none was begun", async () => { expect(await rejectionMessage(exitWith(0))).toBe("process.exit"); - // beginCommand was never called, so exitWith names the invocation - // "root" and finalizeCommand resolves it to "unknown". + // beginCommand was never called, so exitWith names the invocation "root". expect(trackSpy.mock.calls[0][0]).toBe("root"); }); }); diff --git a/tests/helpers/install-info.test.ts b/tests/helpers/install-info.test.ts index 467047e0..54c5f6d3 100644 --- a/tests/helpers/install-info.test.ts +++ b/tests/helpers/install-info.test.ts @@ -67,10 +67,34 @@ describe("install-info", () => { rmSync(tempHome, { recursive: true, force: true }); }); - test("an executable under ~/.archgate/bin is a binary install", () => { - process.execPath = join(tempHome, ".archgate", "bin", "archgate"); - expect(detectInstallMethod()).toBe("binary"); - }); + test.each([ + { + label: "~/.archgate/bin", + segments: [".archgate", "bin", "archgate"], + method: "binary", + }, + { + label: "the ~/.proto fallback when PROTO_HOME is unset", + segments: [".proto", "tools", "archgate", "1.2.3", "archgate"], + method: "proto", + }, + { + label: "node_modules/.bin", + segments: ["project", "node_modules", ".bin", "archgate"], + method: "local", + }, + { + label: "an unrecognized prefix", + segments: ["usr", "local", "archgate"], + method: "global-pm", + }, + ] as const)( + "classifies an executable under $label as $method", + ({ segments, method }) => { + process.execPath = join(tempHome, ...segments); + expect(detectInstallMethod()).toBe(method); + } + ); test("an executable under PROTO_HOME/tools/archgate is a proto install", () => { const protoHome = join(tempHome, "custom-proto"); @@ -85,34 +109,6 @@ describe("install-info", () => { expect(detectInstallMethod()).toBe("proto"); }); - test("proto detection falls back to ~/.proto when PROTO_HOME is unset", () => { - process.execPath = join( - tempHome, - ".proto", - "tools", - "archgate", - "1.2.3", - "archgate" - ); - expect(detectInstallMethod()).toBe("proto"); - }); - - test("an executable inside node_modules is a local install", () => { - process.execPath = join( - tempHome, - "project", - "node_modules", - ".bin", - "archgate" - ); - expect(detectInstallMethod()).toBe("local"); - }); - - test("anything else is a global package-manager install", () => { - process.execPath = join(tempHome, "usr", "local", "archgate"); - expect(detectInstallMethod()).toBe("global-pm"); - }); - test("the first classification is cached, later path changes ignored", () => { process.execPath = join(tempHome, ".archgate", "bin", "archgate"); expect(detectInstallMethod()).toBe("binary"); diff --git a/tests/helpers/session-context-copilot.test.ts b/tests/helpers/session-context-copilot.test.ts index 5b1e1134..8fc771c2 100644 --- a/tests/helpers/session-context-copilot.test.ts +++ b/tests/helpers/session-context-copilot.test.ts @@ -386,9 +386,9 @@ describe("readCopilotSession", () => { }); test("ignores an entry whose stat fails (dangling link)", async () => { - // A link left behind by a deleted session: readdir still lists it, but - // stat follows the link and raises ENOENT. "junction" so the link can be - // created unprivileged on Windows; the type is ignored on POSIX. + // A dangling link: readdir lists the entry, while stat follows it and + // raises ENOENT. "junction" so the link can be created unprivileged on + // Windows; the type is ignored on POSIX. const danglingTarget = mkdtempSync(join(tmpdir(), "archgate-dangling-")); symlinkSync( danglingTarget, diff --git a/tests/helpers/session-context-cursor.test.ts b/tests/helpers/session-context-cursor.test.ts index 2093b4b6..2d67478a 100644 --- a/tests/helpers/session-context-cursor.test.ts +++ b/tests/helpers/session-context-cursor.test.ts @@ -320,8 +320,8 @@ describe("readCursorSession", () => { }); test("ignores an entry whose stat fails (dangling link)", async () => { - // A link left behind by a deleted session directory: readdir still lists - // it, but stat follows the link and raises ENOENT. + // A dangling link: readdir lists the entry, while stat follows it and + // raises ENOENT. const danglingTarget = mkdtempSync(join(os.tmpdir(), "archgate-dangling-")); // "junction" so the link can be created unprivileged on Windows too; // the type argument is ignored on POSIX.