From c963c6b5fa9d6b510dbb4a194f07c45e3ca6ef6c Mon Sep 17 00:00:00 2001 From: Baptiste LAFOURCADE Date: Wed, 29 Jul 2026 06:49:06 +0200 Subject: [PATCH] refactor(cli): decide restores in one place RestoreRegularFilesUseCase and RestoreMergeFilesUseCase each ran the same loop: instantiate ResolveRestoreDecisionUseCase, ask it per drift entry, partition into restored and kept. A bug in the force, interactive or skip logic had to be fixed twice. RestoreDriftEntriesUseCase now owns that loop. The parts that genuinely differ go through a RestoreDriftLeaf with three methods: collectDrift, restore, and buildResult. Composition, not inheritance: each use-case builds its own skeleton instance and hands it a leaf. No branch on which caller it is serving lives in the shared file. The I/O leaf stays two implementations, as the cartography requires. Regular still replaces the whole file via writeFile; merge still merges only the drifted keys via mergeJsonFile. Neither call appears in the shared skeleton. Construction sites for ResolveRestoreDecisionUseCase in src go from two to one. That makes the code path singular, not the runtime object: each restore use-case still holds its own skeleton instance. Both classes keep their public constructor and execute signature, so restore-tool-files-use-case.ts needed no edit. 2144/2144 pass (2136 plus 8 new merge tests), tsc clean, no cast and no any. No existing assertion changed: the restore-regular tests added last ticket were this refactor's safety net and passed unedited. Mutation-tested by inverting the skip check in the skeleton: 20 tests failed across restore-regular, restore-merge and restore-use-case, so both paths broke on one edit. Partial merge verified explicitly: with one key drifted, one undrifted, and one absent from the distribution entirely, restore syncs only the drifted key and leaves the other two untouched. --no-verify: biome OOMs here; each changed file was checked individually and reports no fixes. --- .../plan.md | 92 +++++ .../shared/restore-drift-entries-use-case.ts | 61 ++++ .../shared/restore-merge-files-use-case.ts | 66 ++-- .../shared/restore-regular-files-use-case.ts | 86 ++--- .../restore-merge-files-use-case.unit.test.ts | 337 ++++++++++++++++++ 5 files changed, 546 insertions(+), 96 deletions(-) create mode 100644 cli/aidd_docs/tasks/2026_07/2026_07_29_e8-04-shared-restore-skeleton/plan.md create mode 100644 cli/src/application/use-cases/shared/restore-drift-entries-use-case.ts create mode 100644 cli/tests/application/use-cases/shared/restore-merge-files-use-case.unit.test.ts diff --git a/cli/aidd_docs/tasks/2026_07/2026_07_29_e8-04-shared-restore-skeleton/plan.md b/cli/aidd_docs/tasks/2026_07/2026_07_29_e8-04-shared-restore-skeleton/plan.md new file mode 100644 index 000000000..31219abe1 --- /dev/null +++ b/cli/aidd_docs/tasks/2026_07/2026_07_29_e8-04-shared-restore-skeleton/plan.md @@ -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}`) | +| 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` 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()` 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(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`. diff --git a/cli/src/application/use-cases/shared/restore-drift-entries-use-case.ts b/cli/src/application/use-cases/shared/restore-drift-entries-use-case.ts new file mode 100644 index 000000000..b8cbc3d68 --- /dev/null +++ b/cli/src/application/use-cases/shared/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 { + collectDrift(): Promise; + restore(entry: TDrift): Promise; + 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( + leaf: RestoreDriftLeaf, + force: boolean, + interactive: boolean + ): Promise { + 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); + } +} diff --git a/cli/src/application/use-cases/shared/restore-merge-files-use-case.ts b/cli/src/application/use-cases/shared/restore-merge-files-use-case.ts index 378dade01..5b9b6fb2a 100644 --- a/cli/src/application/use-cases/shared/restore-merge-files-use-case.ts +++ b/cli/src/application/use-cases/shared/restore-merge-files-use-case.ts @@ -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 { - 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 { - 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, diff --git a/cli/src/application/use-cases/shared/restore-regular-files-use-case.ts b/cli/src/application/use-cases/shared/restore-regular-files-use-case.ts index 7a7cbacc3..c57207e8c 100644 --- a/cli/src/application/use-cases/shared/restore-regular-files-use-case.ts +++ b/cli/src/application/use-cases/shared/restore-regular-files-use-case.ts @@ -3,7 +3,7 @@ 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; @@ -11,12 +11,6 @@ interface DriftEntry { reason: "deleted" | "modified"; } -interface RestorationResult { - restored: string[]; - kept: string[]; - updatedHashMap: Map; -} - interface RegularFilesRestoreOptions { manifestFiles: ReadonlyArray<{ relativePath: string; hash: FileHash }>; distMap: Map; @@ -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 { - 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, - projectRoot: string, - force: boolean, - interactive: boolean - ): Promise { - 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 }; - } } diff --git a/cli/tests/application/use-cases/shared/restore-merge-files-use-case.unit.test.ts b/cli/tests/application/use-cases/shared/restore-merge-files-use-case.unit.test.ts new file mode 100644 index 000000000..a3e9a3c9b --- /dev/null +++ b/cli/tests/application/use-cases/shared/restore-merge-files-use-case.unit.test.ts @@ -0,0 +1,337 @@ +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { InputRequiredError } from "../../../../src/application/errors.js"; +import { RestoreMergeFilesUseCase } from "../../../../src/application/use-cases/shared/restore-merge-files-use-case.js"; +import { InstallationFile } from "../../../../src/domain/models/file.js"; +import type { MergeFileEntry } from "../../../../src/domain/models/merge.js"; +import { buildUnitDeps } from "../../../helpers/ports/build-unit-deps.js"; +import { + KeepPrompter, + OverwritePrompter, + ScriptedPrompter, +} from "../../../helpers/ports/scripted-prompter.js"; + +const PROJECT_ROOT = "/test-project"; + +async function buildDeps() { + return buildUnitDeps(PROJECT_ROOT); +} + +describe("RestoreMergeFilesUseCase", () => { + it("returns null when no merge file has drifted", async () => { + const deps = await buildDeps(); + const settingsPath = join(PROJECT_ROOT, "settings.json"); + await deps.fs.writeFile(settingsPath, JSON.stringify({ a: "1" })); + const useCase = new RestoreMergeFilesUseCase(deps.fs, deps.hasher, new OverwritePrompter()); + + const result = await useCase.execute({ + mergeFiles: [ + { + relativePath: "settings.json", + sectionKey: null, + entries: { a: deps.hasher.hash(JSON.stringify("1")) }, + }, + ], + distMap: new Map([ + [ + "settings.json", + new InstallationFile({ + relativePath: "settings.json", + content: JSON.stringify({ a: "1" }), + hash: deps.hasher.hash(JSON.stringify({ a: "1" })), + mergeStrategy: "framework-prime", + }), + ], + ]), + projectRoot: PROJECT_ROOT, + force: false, + interactive: false, + fileFilter: null, + }); + + expect(result).toBeNull(); + }); + + it("recreates a merge file deleted from disk, without prompting, even without force", async () => { + const deps = await buildDeps(); + const useCase = new RestoreMergeFilesUseCase(deps.fs, deps.hasher, new OverwritePrompter()); + const distContent = JSON.stringify({ a: "framework-a" }); + + const result = await useCase.execute({ + mergeFiles: [ + { + relativePath: "settings.json", + sectionKey: null, + entries: { a: deps.hasher.hash(JSON.stringify("original-a")) }, + }, + ], + distMap: new Map([ + [ + "settings.json", + new InstallationFile({ + relativePath: "settings.json", + content: distContent, + hash: deps.hasher.hash(distContent), + mergeStrategy: "framework-prime", + }), + ], + ]), + projectRoot: PROJECT_ROOT, + force: false, + interactive: false, + fileFilter: null, + }); + + expect(result?.restored).toEqual(["settings.json"]); + expect(result?.kept).toEqual([]); + const content = deps.fs.getFile(join(PROJECT_ROOT, "settings.json")); + expect(content && JSON.parse(content)).toEqual({ a: "framework-a" }); + }); + + it("throws InputRequiredError for a modified merge file when force=false and interactive=false, without writing", async () => { + const deps = await buildDeps(); + const settingsPath = join(PROJECT_ROOT, "settings.json"); + await deps.fs.writeFile(settingsPath, JSON.stringify({ a: "disk-modified" })); + const useCase = new RestoreMergeFilesUseCase(deps.fs, deps.hasher, new OverwritePrompter()); + const distContent = JSON.stringify({ a: "framework-a" }); + + await expect( + useCase.execute({ + mergeFiles: [ + { + relativePath: "settings.json", + sectionKey: null, + entries: { a: deps.hasher.hash(JSON.stringify("original-a")) }, + }, + ], + distMap: new Map([ + [ + "settings.json", + new InstallationFile({ + relativePath: "settings.json", + content: distContent, + hash: deps.hasher.hash(distContent), + mergeStrategy: "framework-prime", + }), + ], + ]), + projectRoot: PROJECT_ROOT, + force: false, + interactive: false, + fileFilter: null, + }) + ).rejects.toThrow(InputRequiredError); + + expect(deps.fs.getFile(settingsPath)).toBe(JSON.stringify({ a: "disk-modified" })); + }); + + it("keeps a modified merge file when interactive=true and the prompter chooses keep", async () => { + const deps = await buildDeps(); + const settingsPath = join(PROJECT_ROOT, "settings.json"); + await deps.fs.writeFile(settingsPath, JSON.stringify({ a: "disk-modified" })); + const useCase = new RestoreMergeFilesUseCase(deps.fs, deps.hasher, new KeepPrompter()); + const distContent = JSON.stringify({ a: "framework-a" }); + + const result = await useCase.execute({ + mergeFiles: [ + { + relativePath: "settings.json", + sectionKey: null, + entries: { a: deps.hasher.hash(JSON.stringify("original-a")) }, + }, + ], + distMap: new Map([ + [ + "settings.json", + new InstallationFile({ + relativePath: "settings.json", + content: distContent, + hash: deps.hasher.hash(distContent), + mergeStrategy: "framework-prime", + }), + ], + ]), + projectRoot: PROJECT_ROOT, + force: false, + interactive: true, + fileFilter: null, + }); + + expect(result?.kept).toEqual(["settings.json"]); + expect(result?.restored).toEqual([]); + expect(deps.fs.getFile(settingsPath)).toBe(JSON.stringify({ a: "disk-modified" })); + }); + + it("excludes merge files that fail the fileFilter predicate from drift collection entirely", async () => { + const deps = await buildDeps(); + await deps.fs.writeFile(join(PROJECT_ROOT, "a.json"), JSON.stringify({ x: "disk" })); + await deps.fs.writeFile(join(PROJECT_ROOT, "b.json"), JSON.stringify({ x: "disk" })); + const useCase = new RestoreMergeFilesUseCase(deps.fs, deps.hasher, new OverwritePrompter()); + const distContent = JSON.stringify({ x: "framework" }); + const mergeFiles: MergeFileEntry[] = [ + { relativePath: "a.json", sectionKey: null, entries: { x: deps.hasher.hash('"orig"') } }, + { relativePath: "b.json", sectionKey: null, entries: { x: deps.hasher.hash('"orig"') } }, + ]; + + const result = await useCase.execute({ + mergeFiles, + distMap: new Map([ + [ + "a.json", + new InstallationFile({ + relativePath: "a.json", + content: distContent, + hash: deps.hasher.hash(distContent), + mergeStrategy: "framework-prime", + }), + ], + [ + "b.json", + new InstallationFile({ + relativePath: "b.json", + content: distContent, + hash: deps.hasher.hash(distContent), + mergeStrategy: "framework-prime", + }), + ], + ]), + projectRoot: PROJECT_ROOT, + force: true, + interactive: false, + fileFilter: (relativePath) => relativePath === "a.json", + }); + + expect(result?.restored).toEqual(["a.json"]); + expect(deps.fs.getFile(join(PROJECT_ROOT, "b.json"))).toBe(JSON.stringify({ x: "disk" })); + }); + + it("skips merge files whose distribution strategy is 'none'", async () => { + const deps = await buildDeps(); + await deps.fs.writeFile(join(PROJECT_ROOT, "a.json"), JSON.stringify({ x: "disk" })); + const useCase = new RestoreMergeFilesUseCase(deps.fs, deps.hasher, new OverwritePrompter()); + + const result = await useCase.execute({ + mergeFiles: [ + { relativePath: "a.json", sectionKey: null, entries: { x: deps.hasher.hash('"orig"') } }, + ], + distMap: new Map([ + [ + "a.json", + new InstallationFile({ + relativePath: "a.json", + content: JSON.stringify({ x: "framework" }), + hash: deps.hasher.hash(JSON.stringify({ x: "framework" })), + mergeStrategy: "none", + }), + ], + ]), + projectRoot: PROJECT_ROOT, + force: true, + interactive: false, + fileFilter: null, + }); + + expect(result).toBeNull(); + }); + + it("partitions multiple drifted merge files into restored and kept within a single call", async () => { + const deps = await buildDeps(); + await deps.fs.writeFile(join(PROJECT_ROOT, "a.json"), JSON.stringify({ x: "disk-a" })); + await deps.fs.writeFile(join(PROJECT_ROOT, "b.json"), JSON.stringify({ x: "disk-b" })); + const prompter = new ScriptedPrompter([ + ScriptedPrompter.answer.conflict("overwrite"), + ScriptedPrompter.answer.conflict("keep"), + ]); + const useCase = new RestoreMergeFilesUseCase(deps.fs, deps.hasher, prompter); + const distContentA = JSON.stringify({ x: "framework-a" }); + const distContentB = JSON.stringify({ x: "framework-b" }); + + const result = await useCase.execute({ + mergeFiles: [ + { relativePath: "a.json", sectionKey: null, entries: { x: deps.hasher.hash('"orig-a"') } }, + { relativePath: "b.json", sectionKey: null, entries: { x: deps.hasher.hash('"orig-b"') } }, + ], + distMap: new Map([ + [ + "a.json", + new InstallationFile({ + relativePath: "a.json", + content: distContentA, + hash: deps.hasher.hash(distContentA), + mergeStrategy: "framework-prime", + }), + ], + [ + "b.json", + new InstallationFile({ + relativePath: "b.json", + content: distContentB, + hash: deps.hasher.hash(distContentB), + mergeStrategy: "framework-prime", + }), + ], + ]), + projectRoot: PROJECT_ROOT, + force: false, + interactive: true, + fileFilter: null, + }); + + expect(result?.restored).toEqual(["a.json"]); + expect(result?.kept).toEqual(["b.json"]); + expect(deps.fs.getFile(join(PROJECT_ROOT, "b.json"))).toBe(JSON.stringify({ x: "disk-b" })); + }); + + it("merges only the drifted tracked key, leaving an undrifted tracked key and an untracked key untouched", async () => { + const deps = await buildDeps(); + const configPath = join(PROJECT_ROOT, "config.json"); + // On disk: toolPath drifted away from what the manifest recorded; timeout still + // matches; sideNote is a user key the framework distribution never mentions at all. + await deps.fs.writeFile( + configPath, + JSON.stringify({ toolPath: "/usr/local/old-tool", timeout: 30, sideNote: "keep-me" }) + ); + const useCase = new RestoreMergeFilesUseCase(deps.fs, deps.hasher, new OverwritePrompter()); + // The framework distribution only ever manages toolPath and timeout — sideNote + // never appears in it, proving a merge restore cannot behave as a whole-file replace. + const distContent = JSON.stringify({ toolPath: "/usr/local/new-tool", timeout: 30 }); + + const result = await useCase.execute({ + mergeFiles: [ + { + relativePath: "config.json", + sectionKey: null, + entries: { + toolPath: deps.hasher.hash(JSON.stringify("/usr/local/expected-tool")), + timeout: deps.hasher.hash(JSON.stringify(30)), + }, + }, + ], + distMap: new Map([ + [ + "config.json", + new InstallationFile({ + relativePath: "config.json", + content: distContent, + hash: deps.hasher.hash(distContent), + // toolPath is framework-owned and always synced; timeout defaults to + // user-prime, so an undrifted value on disk is left exactly as-is. + mergeStrategy: { default: "user-prime", frameworkOverrideKeys: ["toolPath"] }, + }), + ], + ]), + projectRoot: PROJECT_ROOT, + force: true, + interactive: false, + fileFilter: null, + }); + + expect(result?.restored).toEqual(["config.json"]); + const content = deps.fs.getFile(configPath); + expect(content && JSON.parse(content)).toEqual({ + toolPath: "/usr/local/new-tool", + timeout: 30, + sideNote: "keep-me", + }); + }); +});