Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
---
objective: "Build the coverage safety net that US-E8-02, US-E8-03 and US-E8-04 need before their refactors touch install and restore code."
status: done
---

# SPIKE-E8-01: install/restore coverage

Baseline confirmed before any change: 2118 passing tests (`pnpm test`, 196 files). After this spike: 2136 passing tests (197 files, 18 new), zero failures.

## Coverage, before / after

| File | Branch before | Branch after | Stmt before | Stmt after | Needed by |
| --- | ---: | ---: | ---: | ---: | --- |
| `shared/restore-regular-files-use-case.ts` | 72.7% | **100%** | 42.7% | **100%** | US-E8-04 (blocking) |
| `install/install-agents-use-case.ts` | 88.2% | 94.7% | 95.7% | 100% | US-E8-02 |
| `install/install-commands-use-case.ts` | 88.9% | 94.4% | 100% | 100% | US-E8-02 |
| `install/install-rules-use-case.ts` | 87.5% | 94.4% | 95.0% | 100% | US-E8-02 |
| `install/install-skills-use-case.ts` | 88.9% | 94.4% | 100% | 100% | US-E8-02 |
| `shared/restore-merge-files-use-case.ts` | 96.6% | 96.6% | 100% | 100% | US-E8-04 (already fine, untouched) |
| `global/update-ai-tools-use-case.ts` | 100% | 100% | 100% | 100% | US-E8-03, nothing to do |
| `global/update-ide-tools-use-case.ts` | 100% | 100% | 100% | 100% | US-E8-03, nothing to do |

Measured with `vitest --coverage` over `unit` + `integration`, `--coverage.reporter=json-summary`, scoped per file via `--coverage.include`. Numbers match the task brief's baseline exactly, confirmed before writing any test.

US-E8-03 needs no new coverage. Both `update-ai-tools-use-case.ts` and `update-ide-tools-use-case.ts` were already at 100%/100% before this spike and remain there. No test file was touched for either.

## Priority 1: RestoreRegularFilesUseCase

No dedicated test file existed anywhere in the repo for this class. Its 42.7% statement coverage was incidental, leaking in from `restore-use-case.unit.test.ts`, which only exercises it indirectly through the full `RestoreUseCase` (and the fixture files that test uses never hit several paths directly).

New file: `tests/application/use-cases/shared/restore-regular-files-use-case.unit.test.ts`, 10 tests, instantiating `RestoreRegularFilesUseCase` directly against `buildUnitDeps`'s in-memory `fs` and `DeterministicHasher`, with `KeepPrompter` / `OverwritePrompter` / `ScriptedPrompter` from the existing prompter fakes.

Covered:
- drift kind "deleted" (file absent from disk) restoring from the dist map, without prompting and without `--force`.
- drift kind "modified" (disk content differs from the manifest hash), across all three decision modes: `force=true` (no prompt), `interactive=true` with the prompter choosing keep, `interactive=true` with the prompter choosing overwrite, and neither force nor interactive (throws `InputRequiredError`, and the file is left untouched by the throw).
- the restored/kept partition within a single call, using a scripted prompter with two files that get different answers in the same `execute()`.
- `fileFilter` honoured (excludes a file from drift collection entirely, so it is never even considered restored or kept) versus `fileFilter: null` (all manifest files considered).
- `execute()` returning `null` when nothing has drifted.
- two edge cases not named in the task brief but present in the uncovered line ranges: a manifest-tracked file that drifts (deleted or modified) but has no corresponding entry in the dist map is silently dropped, not restored, not kept, no error. See "Suspicious behaviour" below.

Result: 100% branches, 100% statements (up from 72.7% / 42.7%), exceeding the story's ask.

### Mutation evidence

Two mutations applied to production code, one test run each, then reverted (confirmed via `git diff src/` returning empty afterward):

1. `restore-regular-files-use-case.ts`, `applyRestorations`: inverted `if (skip)` to `if (!skip)`, which swaps which files land in `restored` versus `kept`. Result: 6 of 10 tests failed (all four decision-mode tests, the fileFilter test, and the multi-file partition test).
2. `resolve-restore-decision.ts`: inverted `if (!force && !interactive)` to `if (force && !interactive)`, which flips when the non-interactive/non-force gate throws `InputRequiredError`. Result: 3 of 10 tests failed (the throw test itself, the force=true test, and the fileFilter test, which also runs in force mode).

Both mutations were caught, confirmed failing, then reverted and confirmed green again (10/10 pass, `git diff src/` empty).

## Priority 2: the four install content use-cases

All four (`install-agents`, `install-rules`, `install-commands`, `install-skills`) share the same `processFile` skeleton: `startsWith` directory check, `acceptsFileName`, an `entryFile` gate, `buildInstallPath`, then a `.gitkeep` special case. Confirmed identical structure by diffing the four files directly, not by assumption.

Pulled the exact missing branch line numbers from `coverage-final.json` per file (not just the summary) before writing anything:

| File | Missing branch lines | What they are |
| --- | --- | --- |
| `install-agents-use-case.ts` | 43, 48 | `entryFile !== null` gate never entered; `buildInstallPath` returning `null` never hit |
| `install-rules-use-case.ts` | 40, 45 | same two, rules had no `entryFile` test at all |
| `install-commands-use-case.ts` | 41, 45 | `entryFile` gate was already covered by an existing test; `buildInstallPath === null` was not |
| `install-skills-use-case.ts` | 41, 45 | `entryFile` gate already covered (both directions); `buildInstallPath === null` was not |

What was added:
- `install-agents-use-case.unit.test.ts` and `install-rules-use-case.unit.test.ts`: a new `entryFile`-set `ContentSection` plus two tests each (accepts the matching basename, filters out a mismatching one), matching the pattern the `install-skills` test file already used for its own `entryFile` coverage. `install-commands-use-case.unit.test.ts` already had an equivalent test (`"respects entryFile filter when section has an entryFile"`); left it untouched.
- All four files: one new test using the `copilot` tool config (imported alongside `claude`) installing a `.gitkeep` file. Claude's `buildInstallPath` never returns `null`, so claude's existing `.gitkeep` test always takes the "empty-content `InstallationFile`" branch. Copilot's `agentsHandler` / `rulesHandler` / `commandsHandler` / `skillsHandler.buildFilePath` all explicitly `return null` when the basename is `.gitkeep` (checked in `src/domain/tools/ai/copilot.ts`), which is a real, reachable path to the `outputPath === null` branch and a genuine behavioural difference between tools: with `claude`, a tracked `.gitkeep` produces an empty `InstallationFile`; with `copilot`, it is filtered out of the result entirely. Pinned as-is; not a bug, just an asymmetry the US-E8-02 generalisation needs to preserve.
- Confirmed the documented asymmetry: `InstallAgentsUseCase.execute` calls `cap.acceptsFileName(relativeFileName, ALL_TOOL_SUFFIXES)` with the extra `ALL_TOOL_SUFFIXES` argument the other three don't pass. This was already exercised by the existing "filters out agent files for other tools" test (unchanged); the new `entryFile` and `.gitkeep` tests for agents go through the same call site, so the asymmetric signature stays exercised under the new cases too.

Result per file: statements 100% (up from 95.0-100%), branches 94.4-94.7% (up from 87.5-88.9%).

### Remaining uncovered branch (dead code, not pursued)

Every one of the four files has this line inside `processFile`:

```ts
const basename = relativeFileName.split("/").at(-1) ?? relativeFileName;
```

The `?? relativeFileName` fallback is unreachable: `String.prototype.split` always returns an array with at least one element, so `.at(-1)` on its result is only ever `undefined` if the array were empty, which cannot happen. No test input can trigger this branch. It is the one remaining branch miss in all four coverage-after numbers above (1 out of 18-19 branches per file). Reported here rather than forced with a fake test, per the characterization-only rule: pinning a branch that cannot execute would mean asserting nothing real.

## Suspicious behaviour found, deliberately pinned rather than fixed

In `RestoreRegularFilesUseCase.collectDrift`, when a manifest-tracked file has drifted (deleted from disk, or modified on disk) but has no corresponding entry in `distMap`, the drift is silently dropped: nothing is pushed to the `drift` array, so the file appears in neither `restored` nor `kept`. If it was the only file being restored, `execute()` returns `null`, i.e. "nothing to restore", indistinguishable from the file never having drifted at all. No error, no log, no indication that a tracked file is out of sync with no way to reconcile it. Two tests pin this exact behaviour (`"silently drops a deleted file..."` / `"silently drops a modified file..."`). Whether this is intended (dist map legitimately doesn't have every manifest entry in some scenario) or a gap worth surfacing to the user is a product question, not something this spike should decide by asserting different behaviour.

## Verification

- `npx tsc --noEmit`: no errors.
- `pnpm test`: 197 files, 2136 tests, 0 failures (2118 baseline + 18 new).
- Biome (`./node_modules/.bin/biome check --write <file>`, one file at a time): all 5 changed/new files report "No fixes applied" (one auto-fix applied to the new restore-regular-files test file's import wrapping, confirmed clean on the following run).
- Mutation testing on the blocking file: see "Mutation evidence" above.
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
// Register the claude tool so its capabilities are accessible
// Register the claude and copilot tools so their capabilities are accessible
import "../../../../src/domain/tools/ai/claude.js";
import "../../../../src/domain/tools/ai/copilot.js";
import { describe, expect, it } from "vitest";
import { InstallAgentsUseCase } from "../../../../src/application/use-cases/install/install-agents-use-case.js";
import type { ContentSection } from "../../../../src/domain/models/framework.js";
import { GITKEEP_FILE } from "../../../../src/domain/models/framework.js";
import { claude } from "../../../../src/domain/tools/ai/claude.js";
import { copilot } from "../../../../src/domain/tools/ai/copilot.js";
import { DeterministicHasher } from "../../../helpers/ports/deterministic-hasher.js";

const DOCS_DIR = "aidd_docs";
Expand All @@ -15,6 +17,12 @@ const agentsSection: ContentSection = {
entryFile: null,
};

const agentsSectionWithEntry: ContentSection = {
name: "agents",
directory: "agents",
entryFile: "AGENT.md",
};

function buildUseCase() {
const hasher = new DeterministicHasher();
const useCase = new InstallAgentsUseCase(hasher);
Expand Down Expand Up @@ -146,4 +154,57 @@ describe("InstallAgentsUseCase", () => {
expect(paths).toContain(".claude/agents/agent-b.md");
});
});

describe("execute β€” entryFile section", () => {
it("accepts only the entryFile-named file and installs it", () => {
const { useCase } = buildUseCase();
const contentFiles = new Map([
["agents/my-agent/AGENT.md", "---\nname: my-agent\ndescription: My agent\n---\n# Agent\n"],
["agents/my-agent/other.claude.md", "---\nname: other\ndescription: Other\n---\n# Other\n"],
]);

const files = useCase.execute({
toolConfig: claude,
section: agentsSectionWithEntry,
contentFiles,
docsDir: DOCS_DIR,
});

expect(files).toHaveLength(1);
expect(files[0].frameworkPath).toBe("agents/my-agent/AGENT.md");
});

it("filters out files whose basename does not match entryFile", () => {
const { useCase } = buildUseCase();
const contentFiles = new Map([
["agents/reviewer/helper.claude.md", "# helper β€” not AGENT.md basename"],
]);

const files = useCase.execute({
toolConfig: claude,
section: agentsSectionWithEntry,
contentFiles,
docsDir: DOCS_DIR,
});

expect(files).toHaveLength(0);
});
});

describe("execute β€” tool with a null install path for .gitkeep", () => {
it("filters out the .gitkeep file entirely instead of producing an empty InstallationFile", () => {
const { useCase } = buildUseCase();
const gitkeepPath = `agents/${GITKEEP_FILE}`;
const contentFiles = new Map([[gitkeepPath, ""]]);

const files = useCase.execute({
toolConfig: copilot,
section: agentsSection,
contentFiles,
docsDir: DOCS_DIR,
});

expect(files).toHaveLength(0);
});
});
});
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
// Register the claude tool so its capabilities are accessible
// Register the claude and copilot tools so their capabilities are accessible
import "../../../../src/domain/tools/ai/claude.js";
import "../../../../src/domain/tools/ai/copilot.js";
import { describe, expect, it } from "vitest";
import { InstallCommandsUseCase } from "../../../../src/application/use-cases/install/install-commands-use-case.js";
import type { ContentSection } from "../../../../src/domain/models/framework.js";
import { GITKEEP_FILE } from "../../../../src/domain/models/framework.js";
import { claude } from "../../../../src/domain/tools/ai/claude.js";
import { copilot } from "../../../../src/domain/tools/ai/copilot.js";
import { DeterministicHasher } from "../../../helpers/ports/deterministic-hasher.js";

const DOCS_DIR = "aidd_docs";
Expand Down Expand Up @@ -151,5 +153,20 @@ describe("InstallCommandsUseCase", () => {
const paths = files.map((f) => f.frameworkPath);
expect(paths).not.toContain("commands/other.claude.md");
});

it("filters out the .gitkeep file entirely when the tool's install path is null for it", () => {
const { useCase } = buildUseCase();
const gitkeepPath = `commands/04_code/${GITKEEP_FILE}`;
const contentFiles = new Map([[gitkeepPath, ""]]);

const files = useCase.execute({
toolConfig: copilot,
section: commandsSection,
contentFiles,
docsDir: DOCS_DIR,
});

expect(files).toHaveLength(0);
});
});
});
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
// Register the claude tool so its capabilities are accessible
// Register the claude and copilot tools so their capabilities are accessible
import "../../../../src/domain/tools/ai/claude.js";
import "../../../../src/domain/tools/ai/copilot.js";
import { describe, expect, it } from "vitest";
import { InstallRulesUseCase } from "../../../../src/application/use-cases/install/install-rules-use-case.js";
import type { ContentSection } from "../../../../src/domain/models/framework.js";
import { GITKEEP_FILE } from "../../../../src/domain/models/framework.js";
import { claude } from "../../../../src/domain/tools/ai/claude.js";
import { copilot } from "../../../../src/domain/tools/ai/copilot.js";
import { DeterministicHasher } from "../../../helpers/ports/deterministic-hasher.js";

const DOCS_DIR = "aidd_docs";
Expand All @@ -15,6 +17,12 @@ const rulesSection: ContentSection = {
entryFile: null,
};

const rulesSectionWithEntry: ContentSection = {
name: "rules",
directory: "rules",
entryFile: "RULE.md",
};

function buildUseCase() {
const hasher = new DeterministicHasher();
const useCase = new InstallRulesUseCase(hasher);
Expand Down Expand Up @@ -162,4 +170,57 @@ describe("InstallRulesUseCase", () => {
expect(paths).toContain(".claude/rules/02-patterns/rule-b.md");
});
});

describe("execute β€” entryFile section", () => {
it("accepts only the entryFile-named file and installs it", () => {
const { useCase } = buildUseCase();
const contentFiles = new Map([
["rules/my-rule/RULE.md", "---\npaths:\n - src/**\n---\n# Rule\n"],
["rules/my-rule/other.claude.md", "---\npaths:\n - src/**\n---\n# Other\n"],
]);

const files = useCase.execute({
toolConfig: claude,
section: rulesSectionWithEntry,
contentFiles,
docsDir: DOCS_DIR,
});

expect(files).toHaveLength(1);
expect(files[0].frameworkPath).toBe("rules/my-rule/RULE.md");
});

it("filters out files whose basename does not match entryFile", () => {
const { useCase } = buildUseCase();
const contentFiles = new Map([
["rules/standards/helper.claude.md", "# helper β€” not RULE.md basename"],
]);

const files = useCase.execute({
toolConfig: claude,
section: rulesSectionWithEntry,
contentFiles,
docsDir: DOCS_DIR,
});

expect(files).toHaveLength(0);
});
});

describe("execute β€” tool with a null install path for .gitkeep", () => {
it("filters out the .gitkeep file entirely instead of producing an empty InstallationFile", () => {
const { useCase } = buildUseCase();
const gitkeepPath = `rules/${GITKEEP_FILE}`;
const contentFiles = new Map([[gitkeepPath, ""]]);

const files = useCase.execute({
toolConfig: copilot,
section: rulesSection,
contentFiles,
docsDir: DOCS_DIR,
});

expect(files).toHaveLength(0);
});
});
});
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
// Register the claude tool so its capabilities are accessible
// Register the claude and copilot tools so their capabilities are accessible
import "../../../../src/domain/tools/ai/claude.js";
import "../../../../src/domain/tools/ai/copilot.js";
import { describe, expect, it } from "vitest";
import { InstallSkillsUseCase } from "../../../../src/application/use-cases/install/install-skills-use-case.js";
import type { ContentSection } from "../../../../src/domain/models/framework.js";
import { GITKEEP_FILE } from "../../../../src/domain/models/framework.js";
import { claude } from "../../../../src/domain/tools/ai/claude.js";
import { copilot } from "../../../../src/domain/tools/ai/copilot.js";
import { DeterministicHasher } from "../../../helpers/ports/deterministic-hasher.js";

const DOCS_DIR = "aidd_docs";
Expand Down Expand Up @@ -172,4 +174,21 @@ describe("InstallSkillsUseCase", () => {
expect(files).toHaveLength(0);
});
});

describe("execute β€” tool with a null install path for .gitkeep", () => {
it("filters out the .gitkeep file entirely instead of producing an empty InstallationFile", () => {
const { useCase } = buildUseCase();
const gitkeepPath = `skills/${GITKEEP_FILE}`;
const contentFiles = new Map([[gitkeepPath, ""]]);

const files = useCase.execute({
toolConfig: copilot,
section: skillsSectionFlat,
contentFiles,
docsDir: DOCS_DIR,
});

expect(files).toHaveLength(0);
});
});
});
Loading