ai-driven-dev · blafourcade · Jul 29, 2026 · Jul 29, 2026
@@ -0,0 +1,92 @@
+---
+objective: Extract the shared collect-drift/decide/partition skeleton out of RestoreRegularFilesUseCase and RestoreMergeFilesUseCase, keeping the whole-file-write vs per-key-merge leaf separate.
+status: implemented
+---
+
+# Shared restore skeleton
+
+## Difference table (before the change)
+
+| Aspect | RestoreRegularFilesUseCase | RestoreMergeFilesUseCase |
+|---|---|---|
+| Constructor | `(fs: FileReader & FileWriter, prompter: Prompter)` | `(fs: FileReader & FileMerger, hasher: Hasher, prompter: Prompter)` |
+| Input shape | `manifestFiles: {relativePath, hash}[]` | `mergeFiles: MergeFileEntry[]` (`{relativePath, sectionKey, entries: Record<string, FileHash>}`) |
+| Drift detection | Compare disk file's whole-content hash to the manifest-recorded hash | Compare each tracked key's hash (via `extractMergeEntries`, JSON-parsed and per-key hashed) to the manifest-recorded per-key hash; drift if any key differs |
+| Drift source method | `collectDrift` (single loop over manifest files) | `collectMergeDrift` calling `checkOneMergeFileDrift` calling `checkModifiedDrift` (three methods) |
+| Decision call | `new ResolveRestoreDecisionUseCase(this.prompter).execute(...)`, instantiated inside the loop | Same call, same instantiate-inside-loop pattern |
+| Partition loop | `applyRestorations`: loop over drift, skip or apply, push to `restored`/`kept` | `applyMergeRestorations`: identical shape, same push pattern |
+| Restore action (I/O leaf) | `fs.writeFile(path, content)`, replaces the whole file, then re-hashes | `fs.mergeJsonFile(path, content, mergeStrategy)`, merges per key per `MergeStrategy`, then re-extracts per-key hashes into a fresh `MergeFileEntry` |
+| Result shape | `{restored, kept, updatedFiles: InstallationFile[]}` built from a hash map | `{restored, kept, updatedMergeFiles: MergeFileEntry[]}` built from a map of `MergeFileEntry` |
+| `fileFilter` option | Present, applied in `collectDrift` | Present, applied in `collectMergeDrift` |
+
+The literally duplicated code was the partition loop: instantiate `ResolveRestoreDecisionUseCase`,
+iterate drift entries, call `.execute()` with the same four fields, branch on the boolean result,
+push into `restored`/`kept`. `collectDrift` vs `collectMergeDrift` are conceptually parallel but
+mechanically different (single hash compare vs per-key JSON compare), so they were left as
+leaf-owned methods, not folded into the shared skeleton, because there is no line-for-line
+duplication between them beyond the `fileFilter` early-continue, which is too small to be worth
+extracting on its own.
+
+## Decisions
+
+| Decision | Why |
+|---|---|
+| New file `restore-drift-entries-use-case.ts` exports `RestoreDriftEntriesUseCase` and a `RestoreDriftLeaf<TDrift, TResult>` interface | Names the responsibility ("restore a batch of drift entries"), not the mechanism. The leaf interface has three methods: `collectDrift()`, `restore(entry)`, `buildResult(restored, kept)`. |
+| The skeleton is injected via **composition**, not a base class | The previous ticket (`refactor(cli): update ai and ide tools through one implementation`, PR #553) used inheritance and drew reviewer pushback for it. Each restore use-case builds its own `RestoreDriftEntriesUseCase` instance in its constructor and hands it a leaf object at `execute()` time. No shared base class, no `extends`. |
+| `RestoreDriftEntriesUseCase` owns exactly one `ResolveRestoreDecisionUseCase` construction site, built once in its own constructor | The original code built a new `ResolveRestoreDecisionUseCase` on every loop iteration, in two separate files, two separate call sites. Now there is exactly one place in the codebase that constructs it (`grep -rn "new ResolveRestoreDecisionUseCase" src/` returns one hit), and it happens once per skeleton instance rather than once per entry. At runtime there are still two instances, one per restore use-case, each holding its own `RestoreDriftEntriesUseCase`. The thing that became singular is the code path that decides skip/overwrite, not a process-wide singleton object. |
+| The leaf's `restore(entry)` method is the only place that writes anything | `RestoreRegularFilesUseCase.execute` builds a leaf whose `restore` calls `fs.writeFile` (whole-file replace); `RestoreMergeFilesUseCase.execute` builds a leaf whose `restore` calls the unmodified `applyOneMergeRestore` (`fs.mergeJsonFile`, per-key). The skeleton's `execute<TDrift, TResult>()` never inspects `entry` beyond `relativePath`/`reason` and never branches on which leaf it is running. There is no `if (kind === "merge")` anywhere in the shared file. |
+| `collectDrift`, `checkOneMergeFileDrift`, `checkModifiedDrift`, `buildDriftEntry`, `applyOneMergeRestore` stay as private methods on their original class | They differ mechanically (whole-file hash vs per-key JSON hash comparison) and were never literally duplicated between the two files, so extracting them would relocate complexity rather than remove duplication. |
+| No `as`/`as unknown as`/`any` anywhere | TypeScript's generic inference on `RestoreDriftEntriesUseCase.execute<TDrift, TResult>(leaf, force, interactive)` resolves both type parameters from the object literal passed at each call site. No cast was needed. Confirmed by `tsc --noEmit`. |
+| Both classes' public constructors and `execute()` signatures are byte-identical to before | `RestoreRegularFilesUseCase(fs, prompter)` and `RestoreMergeFilesUseCase(fs, hasher, prompter)` are unchanged, so `restore-tool-files-use-case.ts` (the only call site, found by grepping `RestoreRegularFilesUseCase`/`RestoreMergeFilesUseCase` across `src/` and `tests/`) needed no edits. `deps.ts` does not reference either class directly, confirmed by grep. |
+
+## Checked fact: partial-key merge still merges only the intended keys
+
+New test `merges only the drifted tracked key, leaving an undrifted tracked key and an untracked
+key untouched` in `tests/application/use-cases/shared/restore-merge-files-use-case.unit.test.ts`:
+
+- Disk holds `{toolPath: "/usr/local/old-tool", timeout: 30, sideNote: "keep-me"}`.
+- The manifest recorded `toolPath` as `"/usr/local/expected-tool"` (so it drifted) and `timeout` as
+  `30` (so it did not drift). `sideNote` is not part of the framework distribution at all.
+- The merge strategy is per-key: `{default: "user-prime", frameworkOverrideKeys: ["toolPath"]}`.
+- After restore: `toolPath` becomes `"/usr/local/new-tool"` (framework-owned key, synced),
+  `timeout` stays `30` (user-prime, untouched), and `sideNote` stays `"keep-me"`, which proves the
+  merge leaf never replaces the whole file the way the regular leaf does.
+
+Result verified by running this test: **PASS**. This pins unchanged production behavior
+(`mergeJsonFile` / `mergePerKey` were not touched); the test exists as this refactor's guardrail,
+so a future regression that collapses the merge leaf into a whole-file write would fail it.
+
+Note: the strategy shape used in this test, `{default: "user-prime", frameworkOverrideKeys: [...]}`,
+is not used by any current production tool config (`src/domain/tools/` only uses the plain
+`"user-prime"`, `"framework-prime"`, and `"none"` strings). It is the shape that makes "only the
+keys that drifted get merged" literally true; a plain `"framework-prime"` strategy would resync
+every framework-managed key on any drift, not just the one that drifted, which is also correct
+existing behavior but does not demonstrate the same guardrail as cleanly.
+
+## Verification
+
+1. `npx tsc --noEmit`: no errors.
+2. `pnpm test` from `cli/`: **2144 passed, 0 failed** (198 test files). Baseline was 2136. The 8 new
+   tests are in the new `restore-merge-files-use-case.unit.test.ts` file. No existing assertion was
+   modified.
+3. Biome, one file at a time, each reporting "No fixes applied":
+   - `src/application/use-cases/shared/restore-drift-entries-use-case.ts`
+   - `src/application/use-cases/shared/restore-regular-files-use-case.ts`
+   - `src/application/use-cases/shared/restore-merge-files-use-case.ts`
+   - `tests/application/use-cases/shared/restore-merge-files-use-case.unit.test.ts`
+4. Mutation test: inverted the `if (skip)` branch to `if (!skip)` inside
+   `RestoreDriftEntriesUseCase.execute`. Re-ran the full suite (through `rtk proxy npx vitest run`,
+   needed because the standard reporter wrapper failed to parse output for this particular run; the
+   proxy path bypasses the wrapper and prints the real vitest report). Result: **20 tests failed
+   across exactly 3 files**:
+   - `tests/application/use-cases/shared/restore-regular-files-use-case.unit.test.ts`, 6 failures, regular path
+   - `tests/application/use-cases/shared/restore-merge-files-use-case.unit.test.ts`, 5 failures, merge path
+   - `tests/application/use-cases/restore-use-case.unit.test.ts`, 9 failures, both paths exercised together through `RestoreUseCase`/`RestoreToolFilesUseCase`
+   Both the regular and the merge path failed, confirming the extraction is genuinely shared rather
+   than duplicated behind a common name. Reverted the mutation; full suite back to 2144/2144 passed
+   (re-confirmed with `pnpm test`, which runs `pnpm build && vitest run`).
+5. Grepped `RestoreRegularFilesUseCase`, `RestoreMergeFilesUseCase`, `ResolveRestoreDecisionUseCase`
+   across `src/` and `tests/`: the only construction site for both restore classes is
+   `src/application/use-cases/restore/restore-tool-files-use-case.ts`, unchanged; `deps.ts` does not
+   reference either class directly; `new ResolveRestoreDecisionUseCase` appears exactly once in
+   `src/`, inside `restore-drift-entries-use-case.ts`.
@@ -0,0 +1,61 @@
+import type { Prompter } from "../../../domain/ports/prompter.js";
+import { ResolveRestoreDecisionUseCase } from "./resolve-restore-decision.js";
+
+interface DriftDescriptor {
+  relativePath: string;
+  reason: "deleted" | "modified";
+}
+
+/**
+ * The I/O leaf: everything that differs between restoring a whole file and
+ * merging drifted keys back into one. The skeleton below never branches on
+ * which leaf it is running — it only calls these three methods.
+ */
+export interface RestoreDriftLeaf<TDrift extends DriftDescriptor, TResult> {
+  collectDrift(): Promise<TDrift[]>;
+  restore(entry: TDrift): Promise<void>;
+  buildResult(restored: string[], kept: string[]): TResult;
+}
+
+/**
+ * Shared skeleton for both restore flows: collect drift, delegate the
+ * keep/overwrite decision to ResolveRestoreDecisionUseCase, then partition
+ * into restored/kept. This is the single place that decision logic lives —
+ * both restore use-cases inject their own leaf instead of duplicating the loop.
+ */
+export class RestoreDriftEntriesUseCase {
+  private readonly resolveDecision: ResolveRestoreDecisionUseCase;
+
+  constructor(prompter: Prompter) {
+    this.resolveDecision = new ResolveRestoreDecisionUseCase(prompter);
+  }
+
+  async execute<TDrift extends DriftDescriptor, TResult>(
+    leaf: RestoreDriftLeaf<TDrift, TResult>,
+    force: boolean,
+    interactive: boolean
+  ): Promise<TResult | null> {
+    const drift = await leaf.collectDrift();
+    if (drift.length === 0) return null;
+
+    const restored: string[] = [];
+    const kept: string[] = [];
+
+    for (const entry of drift) {
+      const skip = await this.resolveDecision.execute({
+        relativePath: entry.relativePath,
+        reason: entry.reason,
+        force,
+        interactive,
+      });
+      if (skip) {
+        kept.push(entry.relativePath);
+        continue;
+      }
+      await leaf.restore(entry);
+      restored.push(entry.relativePath);
+    }
+
+    return leaf.buildResult(restored, kept);
+  }
+}
@@ -9,7 +9,7 @@ import type { FileMerger } from "../../../domain/ports/file-merger.js";
 import type { FileReader } from "../../../domain/ports/file-reader.js";
 import type { Hasher } from "../../../domain/ports/hasher.js";
 import type { Prompter } from "../../../domain/ports/prompter.js";
-import { ResolveRestoreDecisionUseCase } from "./resolve-restore-decision.js";
+import { RestoreDriftEntriesUseCase } from "./restore-drift-entries-use-case.js";
 
 interface MergeDriftEntry {
   relativePath: string;
@@ -35,24 +35,35 @@ export interface MergeFilesRestoreResult {
 }
 
 export class RestoreMergeFilesUseCase {
+  private readonly restoreDriftEntries: RestoreDriftEntriesUseCase;
+
   constructor(
     private readonly fs: FileReader & FileMerger,
     private readonly hasher: Hasher,
-    private readonly prompter: Prompter
-  ) {}
+    prompter: Prompter
+  ) {
+    this.restoreDriftEntries = new RestoreDriftEntriesUseCase(prompter);
+  }
 
   async execute(options: MergeFilesRestoreOptions): Promise<MergeFilesRestoreResult | null> {
-    const drift = await this.collectMergeDrift(
-      options.mergeFiles,
-      options.distMap,
-      options.projectRoot,
-      options.fileFilter
-    );
-    if (drift.length === 0) return null;
-    return this.applyMergeRestorations(
-      drift,
-      options.mergeFiles,
-      options.projectRoot,
+    const mergeMap = new Map(options.mergeFiles.map((m) => [m.relativePath, m]));
+
+    return this.restoreDriftEntries.execute(
+      {
+        collectDrift: () =>
+          this.collectMergeDrift(
+            options.mergeFiles,
+            options.distMap,
+            options.projectRoot,
+            options.fileFilter
+          ),
+        restore: (entry) => this.applyOneMergeRestore(entry, options.projectRoot, mergeMap),
+        buildResult: (restored, kept) => ({
+          restored,
+          kept,
+          updatedMergeFiles: [...mergeMap.values()],
+        }),
+      },
       options.force,
       options.interactive
     );
@@ -115,33 +126,6 @@ export class RestoreMergeFilesUseCase {
     };
   }
 
-  private async applyMergeRestorations(
-    drift: MergeDriftEntry[],
-    mergeFiles: readonly MergeFileEntry[],
-    projectRoot: string,
-    force: boolean,
-    interactive: boolean
-  ): Promise<MergeFilesRestoreResult> {
-    const restored: string[] = [];
-    const kept: string[] = [];
-    const mergeMap = new Map(mergeFiles.map((m) => [m.relativePath, m]));
-    for (const entry of drift) {
-      const skip = await new ResolveRestoreDecisionUseCase(this.prompter).execute({
-        relativePath: entry.relativePath,
-        reason: entry.reason,
-        force,
-        interactive,
-      });
-      if (skip) {
-        kept.push(entry.relativePath);
-        continue;
-      }
-      await this.applyOneMergeRestore(entry, projectRoot, mergeMap);
-      restored.push(entry.relativePath);
-    }
-    return { restored, kept, updatedMergeFiles: [...mergeMap.values()] };
-  }
-
   private async applyOneMergeRestore(
     entry: MergeDriftEntry,
     projectRoot: string,

@@ -3,20 +3,14 @@ import { type FileHash, InstallationFile } from "../../../domain/models/file.js"
 import type { FileReader } from "../../../domain/ports/file-reader.js";
 import type { FileWriter } from "../../../domain/ports/file-writer.js";
 import type { Prompter } from "../../../domain/ports/prompter.js";
-import { ResolveRestoreDecisionUseCase } from "./resolve-restore-decision.js";
+import { RestoreDriftEntriesUseCase } from "./restore-drift-entries-use-case.js";
 
 interface DriftEntry {
   relativePath: string;
   content: string;
   reason: "deleted" | "modified";
 }
 
-interface RestorationResult {
-  restored: string[];
-  kept: string[];
-  updatedHashMap: Map<string, FileHash>;
-}
-
 interface RegularFilesRestoreOptions {
   manifestFiles: ReadonlyArray<{ relativePath: string; hash: FileHash }>;
   distMap: Map<string, InstallationFile>;
@@ -33,30 +27,43 @@ export interface RegularFilesRestoreResult {
 }
 
 export class RestoreRegularFilesUseCase {
+  private readonly restoreDriftEntries: RestoreDriftEntriesUseCase;
+
   constructor(
     private readonly fs: FileReader & FileWriter,
-    private readonly prompter: Prompter
-  ) {}
+    prompter: Prompter
+  ) {
+    this.restoreDriftEntries = new RestoreDriftEntriesUseCase(prompter);
+  }
 
   async execute(options: RegularFilesRestoreOptions): Promise<RegularFilesRestoreResult | null> {
-    const drift = await this.collectDrift(
-      options.manifestFiles,
-      options.distMap,
-      options.projectRoot,
-      options.fileFilter
-    );
-    if (drift.length === 0) return null;
-    const { restored, kept, updatedHashMap } = await this.applyRestorations(
-      drift,
-      new Map(options.manifestFiles.map((f) => [f.relativePath, f.hash])),
-      options.projectRoot,
+    const updatedHashMap = new Map(options.manifestFiles.map((f) => [f.relativePath, f.hash]));
+
+    return this.restoreDriftEntries.execute(
+      {
+        collectDrift: () =>
+          this.collectDrift(
+            options.manifestFiles,
+            options.distMap,
+            options.projectRoot,
+            options.fileFilter
+          ),
+        restore: async (entry) => {
+          const diskPath = join(options.projectRoot, entry.relativePath);
+          await this.fs.writeFile(diskPath, entry.content);
+          updatedHashMap.set(entry.relativePath, await this.fs.readFileHash(diskPath));
+        },
+        buildResult: (restored, kept) => ({
+          restored,
+          kept,
+          updatedFiles: Array.from(updatedHashMap.entries()).map(
+            ([relativePath, hash]) => new InstallationFile({ relativePath, content: "", hash })
+          ),
+        }),
+      },
       options.force,
       options.interactive
     );
-    const updatedFiles = Array.from(updatedHashMap.entries()).map(
-      ([relativePath, hash]) => new InstallationFile({ relativePath, content: "", hash })
-    );
-    return { restored, kept, updatedFiles };
   }
 
   private async collectDrift(
@@ -98,35 +105,4 @@ export class RestoreRegularFilesUseCase {
 
     return drift;
   }
-
-  private async applyRestorations(
-    drift: DriftEntry[],
-    initialHashMap: Map<string, FileHash>,
-    projectRoot: string,
-    force: boolean,
-    interactive: boolean
-  ): Promise<RestorationResult> {
-    const restored: string[] = [];
-    const kept: string[] = [];
-    const updatedHashMap = new Map(initialHashMap);
-
-    for (const { relativePath, content, reason } of drift) {
-      const skip = await new ResolveRestoreDecisionUseCase(this.prompter).execute({
-        relativePath,
-        reason,
-        force,
-        interactive,
-      });
-      if (skip) {
-        kept.push(relativePath);
-        continue;
-      }
-      await this.fs.writeFile(join(projectRoot, relativePath), content);
-      const newHash = await this.fs.readFileHash(join(projectRoot, relativePath));
-      updatedHashMap.set(relativePath, newHash);
-      restored.push(relativePath);
-    }
-
-    return { restored, kept, updatedHashMap };
-  }
 }