From 1d1a880fc9c76c394888eec40764bdccc76fd016 Mon Sep 17 00:00:00 2001 From: Baptiste LAFOURCADE Date: Wed, 29 Jul 2026 07:23:00 +0200 Subject: [PATCH 1/2] fix(cli): report restore outcomes it previously hid Two defects found while refactoring nearby code during the cartography backlog, recorded then but left alone to keep those PRs behaviour-preserving. Both change user-visible output, because the output was wrong. Restore silently dropped drifted files. collectDrift ended both its deleted and modified branches with `if (distFile) drift.push(...)` and no else, so a tracked file that genuinely drifted was skipped whenever the current distribution no longer contained it: not restored, not kept, not reported. The user was told restore succeeded while the file stayed broken. Reachable whenever the manifest outlives the distribution. The merge path had the same hole, confirmed rather than assumed: its `if (!distFile || mergeStrategy === "none") continue` dropped genuinely drifted files under both conditions. Drift collection now separates "did it drift" from "can it be restored". The third outcome is `unrestorable`, threaded through RestoreToolFilesResult to RestoreResult to RestoreAllResult, never counted as restored, and surfaced by all three restore entry points as a warning naming the files. The "nothing to restore" success message now also requires no unrestorable entries, so it can no longer claim success while hiding them. Restore also reported two different numbers depending on its path. restoreViaBuiltTree returned the plugin's total file count while restoreViaTranslate counted only files actually written, so a no-op restore on cursor or opencode claimed to have restored everything. Both now mean the same thing. Making that count honest required making the write conditional: the built-tree path previously rewrote every file unconditionally. It now skips files already matching the built content, so the count reflects real writes. The bytes on disk are unchanged either way; unchanged files simply are not rewritten. This also applies to addPlugin, where a fresh install matches nothing and a re-install skips identical writes. Two existing tests were named "silently drops a file" and pinned the bug. They are renamed and now assert the file is reported. A third, covering strategy "none", tested a genuinely drifted file; it is renamed and a sibling test keeps the true no-drift case returning null. 2146/2146 pass (2144 plus 2), tsc clean, golden baseline unchanged since its restore scenario has no unrestorable entries. Each fix was reverted individually to confirm the new tests catch it. Not fixed here: restoreViaBuiltTree ignores fileFilter where restoreViaTranslate honours it. The built-tree translator reads and writes the subtree as one unit with no per-file hook, so whether partial restore even applies to build-mirroring tools is a product decision. --no-verify: biome OOMs here; each changed file was checked individually and reports no fixes. --- .../plan.md | 113 ++++++++++++++++++ cli/src/application/commands/ai.ts | 5 + cli/src/application/commands/ide.ts | 5 + cli/src/application/commands/restore.ts | 11 +- .../use-cases/global/restore-all-use-case.ts | 15 ++- .../use-cases/plugin/plugin-helpers.ts | 26 +++- .../built-tree-materialization-translator.ts | 23 +++- .../restore/restore-tool-files-use-case.ts | 14 ++- .../use-cases/restore/restore-use-case.ts | 2 + .../shared/apply-plugin-files-use-case.ts | 24 ++-- .../shared/restore-drift-entries-use-case.ts | 27 ++++- .../shared/restore-merge-files-use-case.ts | 43 ++++--- .../shared/restore-regular-files-use-case.ts | 45 ++++--- ...apply-plugin-files-built-tree.unit.test.ts | 33 +++++ .../restore-merge-files-use-case.unit.test.ts | 39 +++++- ...estore-regular-files-use-case.unit.test.ts | 12 +- 16 files changed, 356 insertions(+), 81 deletions(-) create mode 100644 cli/aidd_docs/tasks/2026_07/2026_07_29_restore-reporting-findings/plan.md diff --git a/cli/aidd_docs/tasks/2026_07/2026_07_29_restore-reporting-findings/plan.md b/cli/aidd_docs/tasks/2026_07/2026_07_29_restore-reporting-findings/plan.md new file mode 100644 index 000000000..4701f1f85 --- /dev/null +++ b/cli/aidd_docs/tasks/2026_07/2026_07_29_restore-reporting-findings/plan.md @@ -0,0 +1,113 @@ +--- +objective: "Restore tells the truth about what it did, and marketplace-mode plugins are handled the same way on every command." +status: findings-1-2-done +--- + +# Plan: three findings surfaced during the cartography backlog + +None of these came from a ticket. Each was found while refactoring nearby code, recorded in a PR, and deliberately not fixed at the time to keep those PRs behaviour-preserving. + +## Finding 1 — restore silently drops a drifted file (confirmed) + +`restore-regular-files-use-case.ts:83-103`. Both drift branches end in the same guard: + +```ts +const distFile = distMap.get(manifestFile.relativePath); +if (distFile) drift.push({ ... }); +// no else +``` + +A file the manifest tracks, which is genuinely deleted or modified on disk, is skipped when the current distribution no longer contains it. It is not restored, not counted as kept, and not reported. The user is told restore succeeded while a tracked file stays broken. + +This is reachable whenever the manifest outlives the distribution: a file dropped in a newer framework version, or a tool whose content set changed. + +**Fix.** Collect these into a distinct outcome rather than dropping them. The skeleton already partitions `restored` / `kept`, so a third category is a natural extension: the entry is drifted, but nothing exists to restore it from. Surface it in the command output as a warning, since a silent success is the actual defect. + +## Finding 2 — restore reports two different numbers (confirmed, worse than recorded) + +`apply-plugin-files-use-case.ts:70-100`. The same `execute()` returns a count whose meaning depends on which path ran: + +| Path | Returns | Honours `fileFilter` | +| ---- | ------- | -------------------- | +| `restoreViaBuiltTree` (line 77) | `plugin.files.size`, every file the plugin has | no | +| `restoreViaTranslate` (line 86-99) | only files actually written | yes | + +So on a materializing tool (cursor, opencode), a restore where nothing drifted still reports the full file count as if it had restored everything. + +The `fileFilter` half was not in the original note and is the more serious of the two: a scoped restore may write outside its scope on the built-tree path. Whether that is wrong depends on whether a built subtree can be partially copied at all, so it needs checking before changing. + +**Fix.** Make `restoreViaBuiltTree` count what it actually wrote, so both paths mean the same thing. Treat the `fileFilter` asymmetry as a separate question and confirm intent before touching it. + +## Finding 3 — marketplace-mode plugins take a different path on update/restore (needs verification first) + +`resolveTranslator` returns `ModeAMarketplaceTranslator` when `translationMode === "marketplace"` (factory line 44-45). But `plugin-update-use-case.ts` and `apply-plugin-files-use-case.ts` both keep the translator only if it is a `BuiltTreeMaterializationTranslator` and otherwise fall through to `PluginContentTranslator`, which materializes files. + +If a tool installs a plugin by registering a reference and writing nothing, then update and restore writing files for it would create content install never produced. + +**Not yet actionable.** Two things decide it, and neither is established: + +1. which tools actually set `translationMode: "marketplace"`; +2. whether such a plugin has any `files` in the manifest, since if it has none the fall-through iterates nothing and the concern is empty. + +A spike answers both before any fix. Consistent with the rest of this backlog, this may end up infirmed. + +## Order + +1. Spike finding 3 (cheap, decides whether it is real). +2. Fix finding 1, with a test proving the dropped entry is now reported. +3. Fix finding 2's count, with a test proving a no-op restore on the built-tree path reports zero. +4. Fix finding 3 only if the spike confirms it. + +Findings 1 and 2 both change user-visible output. That is the point in both cases: the current output is wrong. + +## What actually happened (2026-07-29) + +Findings 1 and 2 fixed. Finding 3 untouched — still needs its spike before any code change, and wasn't in this pass's scope. + +### Finding 1 + +Added a third outcome, named `unrestorable`, to the shared drift skeleton (`restore-drift-entries-use-case.ts`): `DriftCollection` is now `{ drift, unrestorable }`, and `RestoreDriftLeaf.buildResult` takes `(restored, kept, unrestorable)`. The null short-circuit (`collectDrift` found nothing at all) now checks both arrays, so a tool with only unrestorable entries no longer collapses to a silent no-op. + +Both leaves (`restore-regular-files-use-case.ts`, `restore-merge-files-use-case.ts`) were rewritten to separate "did this drift" (independent of the dist map) from "can we restore it" (dist map lookup) — previously those were fused, which is exactly how the entry vanished with no `else`. + +The merge path had the same hole, confirmed by tracing it, not assumed: `collectMergeDrift`'s `if (!distFile || distFile.mergeStrategy === "none") continue;` silently dropped genuinely-drifted files under both conditions. Both are now routed to `unrestorable` — a file the current distribution no longer merge-tracks (dropped entirely, or its strategy became `"none"`) has nothing left to restore drift from, same as the regular-files case. An existing test asserting `mergeStrategy: "none"` silently returns `null` was renamed and its assertion changed (see below); a second test was added confirming `"none"` with *no* actual drift still correctly returns `null` (the "not drifted at all" case wasn't part of the bug and stays untouched). + +The count flows up unchanged in shape: `RestoreToolFilesResult`, `RestoreResult` (restore-use-case.ts), and `RestoreAllResult` (restore-all-use-case.ts) each gained an `unrestorable: string[]` field, aggregated by concatenation/flatMap at each layer — never counted as `restored`. + +Surfaced to the user as a warning line in all three command entry points that report restore results: `aidd restore` (restore.ts), `aidd ide restore` (ide.ts), `aidd ai restore` (ai.ts). Each now emits, when `unrestorable.length > 0`: + +``` +Could not restore N file(s) no longer part of the current distribution: +``` + +The "nothing to restore" success gate in all three now also checks `unrestorable.length === 0`, so a tool with only unrestorable entries no longer reports false success. + +### Finding 2 + +`restoreViaBuiltTree` no longer reads `manifest.getPlugins(toolId).find(...)?.files.size` after the fact. `materializeViaBuiltTree` (plugin-helpers.ts) now returns the actual number of files (re)written, and `BuiltTreeMaterializationTranslator.addPlugin` computes that count itself: it now skips writing (and doesn't count) any file whose disk content already matches the built-tree content, mirroring `restoreViaTranslate`'s `isFileAtDesiredState` check exactly — both paths now call the same extracted helper, `isPluginFileAtDesiredState` in plugin-helpers.ts. This was necessary, not just a reporting fix: the built-tree write step previously overwrote every file unconditionally every restore, so "how many files were written" and "the plugin's total file count" were the same number by construction. Making the count honest required making the write conditional. + +`BuiltTreeMaterializationTranslator.addPlugin`'s own return type is widened to `{ skipped; written?: number }` (not the shared `PluginTranslator` interface, which stays `{ skipped }` — this class is always consumed through its concrete type at the two call sites that need the count). `written` is `undefined` only on the rare fallback branch (marketplace lookup fails at restore time and materialization delegates to `ModeBFlatMaterializationTranslator`); `materializeViaBuiltTree` reports that as `0` rather than guessing, since undercounting is safer than falsely claiming files changed. + +**fileFilter read-only finding:** `restoreViaBuiltTree` still ignores `fileFilter` — confirmed, not changed. `BuiltTreeMaterializationTranslator` reads the entire built subtree for a plugin from disk (`readBuiltFiles`/`readFlatFiles`, driven by `listFilesRecursive` over the built dir) and writes it as one unit; there is no per-file hook into that read step that a `fileFilter` predicate could gate without restructuring how the built tree is enumerated. Recommendation: treat verbatim-subtree-copy as intentional for this translator (it exists specifically so restored bytes match `framework build` output 1:1) and either (a) accept that `--files` scoping doesn't apply to built-tree-materialized tools (cursor/opencode plugins) and say so in `--help`/docs, or (b) if partial restore of a built subtree is actually wanted, filter the file list returned by `readBuiltFiles`/`readFlatFiles` before `writeChangedFiles` — a small, isolated change, but a product decision (does "restore only these files" mean anything for a tool that's supposed to mirror the build verbatim?) rather than a bug fix. + +### Existing test assertions changed + +1. `restore-regular-files-use-case.unit.test.ts`: the two tests titled *"silently drops a \[deleted/modified\] file that has no corresponding entry in the dist map"* asserted `result === null` and the file left untouched. Renamed to *"reports a \[deleted/modified\] file with no corresponding dist entry as unrestorable, without touching disk"*; assertions changed to `result?.unrestorable === ["a.md"]`, `restored === []`, `kept === []`, disk still untouched. This is the literal behavior the finding says was wrong — the old assertion encoded the bug. +2. `restore-merge-files-use-case.unit.test.ts`: *"skips merge files whose distribution strategy is 'none'"* asserted `result === null` for a file that was genuinely drifted on disk. Renamed to *"reports a drifted merge file as unrestorable when the distribution strategy is 'none'"*; assertions changed to `unrestorable === ["a.json"]`, disk untouched. Added a new sibling test, *"returns null when a merge file's distribution strategy is 'none' and nothing drifted"*, to keep the genuinely-inert case (no drift at all) covered as returning `null`, since that part of the old test's premise was correct and shouldn't regress. + +### New tests + +- Two regression tests above (deleted/modified file dropped from dist map) in `restore-regular-files-use-case.unit.test.ts`. +- One regression test in `restore-merge-files-use-case.unit.test.ts` (mergeStrategy `"none"` with real drift). +- One in `apply-plugin-files-built-tree.unit.test.ts`: *"reports zero files restored when a built-tree restore finds nothing drifted"* — seeds the user-scope plugin dir with content identical to the built tree, then asserts `result.totalFiles === 0`. (Note: had to seed disk directly rather than rely on the preceding install, because the test helper's `installMarketplacePlugin` uses `PluginAddUseCase`'s default `os.homedir()` while `makeRestoreUseCase` injects a fake `homedir: () => "/home/u"` — those two write to different base directories in this test file already, unrelated to this fix, left as-is.) + +### Verification + +- `npx tsc --noEmit`: clean. +- `pnpm test` (full build + vitest): 198 files, **2146 tests**, 0 failures (baseline was 2144; net +2 from the new merge-drift and built-tree no-op tests — the two regular-files tests were renamed in place, not added). +- Biome: every changed file individually reports "No fixes applied" after one `--write` pass (two files needed one auto-format pass for a line-wrap, then were clean). +- Golden baseline e2e (`tests/golden/golden-baseline.e2e.test.ts`): passed unchanged, no regeneration needed — its only restore scenario (`restore --force` with nothing modified) has zero unrestorable entries, so output is byte-identical. +- Mutation testing: reverted each fix to its buggy form one at a time, confirmed exactly the new/changed tests failed (and only those), then restored the fix and re-ran the full suite green. + - Finding 1: reverted `collectDrift`'s dist-map-miss branch back to a silent `continue`. Both new regular-files tests failed with `result === null` where `unrestorable: ["a.md"]` was expected — 2/2146 failed. + - Finding 1 (merge): reverted the merge leaf's dist-map/`"none"` branch back to a silent `continue`. The new merge test failed the same way — 1/2146 failed. + - Finding 2: reverted `restoreViaBuiltTree` back to `manifest.getPlugins(...).files.size`. The new built-tree no-op test failed (`1` instead of `0`) — 1/2146 failed. diff --git a/cli/src/application/commands/ai.ts b/cli/src/application/commands/ai.ts index 291419576..1dbb73314 100644 --- a/cli/src/application/commands/ai.ts +++ b/cli/src/application/commands/ai.ts @@ -227,6 +227,11 @@ export function registerAiCommand(program: Command): void { output.success( `Restored ${restored} ${restored === 1 ? "file" : "files"}, kept ${kept} ${kept === 1 ? "file" : "files"}` ); + if (result.unrestorable.length > 0) { + output.warn( + `Could not restore ${result.unrestorable.length} file(s) no longer part of the current distribution: ${result.unrestorable.join(", ")}` + ); + } } catch (error) { errorHandler.handle(error); } diff --git a/cli/src/application/commands/ide.ts b/cli/src/application/commands/ide.ts index d85e0cc73..61e0183f2 100644 --- a/cli/src/application/commands/ide.ts +++ b/cli/src/application/commands/ide.ts @@ -214,6 +214,11 @@ export function registerIdeCommand(program: Command): void { output.success( `Restored ${result.totalRestored} ${result.totalRestored === 1 ? "file" : "files"}, kept ${result.totalKept} ${result.totalKept === 1 ? "file" : "files"}` ); + if (result.unrestorable.length > 0) { + output.warn( + `Could not restore ${result.unrestorable.length} file(s) no longer part of the current distribution: ${result.unrestorable.join(", ")}` + ); + } } catch (error) { errorHandler.handle(error); } diff --git a/cli/src/application/commands/restore.ts b/cli/src/application/commands/restore.ts index 733d63cc4..b38cad6e6 100644 --- a/cli/src/application/commands/restore.ts +++ b/cli/src/application/commands/restore.ts @@ -19,7 +19,11 @@ export function registerRestoreCommand(program: Command): void { for (const e of result.errors) output.warn(`[${e.scope}] ${e.message}`); - if (result.totalRestored === 0 && result.pluginNamesRestored.length === 0) { + if ( + result.totalRestored === 0 && + result.pluginNamesRestored.length === 0 && + result.unrestorable.length === 0 + ) { output.success("Nothing to restore — all files are unmodified."); return; } @@ -31,6 +35,11 @@ export function registerRestoreCommand(program: Command): void { if (result.pluginNamesRestored.length > 0) { output.success(`Restored plugins: ${result.pluginNamesRestored.join(", ")}`); } + if (result.unrestorable.length > 0) { + output.warn( + `Could not restore ${result.unrestorable.length} file(s) no longer part of the current distribution: ${result.unrestorable.join(", ")}` + ); + } } catch (error) { errorHandler.handle(error); } diff --git a/cli/src/application/use-cases/global/restore-all-use-case.ts b/cli/src/application/use-cases/global/restore-all-use-case.ts index 58f4107c2..690c073c1 100644 --- a/cli/src/application/use-cases/global/restore-all-use-case.ts +++ b/cli/src/application/use-cases/global/restore-all-use-case.ts @@ -11,6 +11,7 @@ export interface RestoreAllResult { totalKept: number; pluginNamesRestored: string[]; errors: GlobalExecutionError[]; + unrestorable: string[]; } export class RestoreAllUseCase { @@ -42,6 +43,7 @@ export class RestoreAllUseCase { totalKept: restoreResult.totalKept, pluginNamesRestored: restoreResult.restoredPluginNames, errors, + unrestorable: restoreResult.unrestorable, }; } @@ -76,9 +78,15 @@ export class RestoreAllUseCase { interactive: boolean, manifest: Awaited>, errors: GlobalExecutionError[] - ): Promise<{ totalRestored: number; totalKept: number; restoredPluginNames: string[] }> { + ): Promise<{ + totalRestored: number; + totalKept: number; + restoredPluginNames: string[]; + unrestorable: string[]; + }> { + const empty = { totalRestored: 0, totalKept: 0, restoredPluginNames: [], unrestorable: [] }; try { - if (manifest === null) return { totalRestored: 0, totalKept: 0, restoredPluginNames: [] }; + if (manifest === null) return empty; const result = await this.restoreUseCase.execute({ version, docsDir: DOCS_DIR, @@ -92,13 +100,14 @@ export class RestoreAllUseCase { totalRestored: result.totalRestored, totalKept: result.totalKept, restoredPluginNames: result.restoredPluginNames, + unrestorable: result.unrestorable, }; } catch (err) { errors.push({ scope: "config-restore", message: err instanceof Error ? err.message : String(err), }); - return { totalRestored: 0, totalKept: 0, restoredPluginNames: [] }; + return empty; } } } diff --git a/cli/src/application/use-cases/plugin/plugin-helpers.ts b/cli/src/application/use-cases/plugin/plugin-helpers.ts index eff0e160d..f23aaeec5 100644 --- a/cli/src/application/use-cases/plugin/plugin-helpers.ts +++ b/cli/src/application/use-cases/plugin/plugin-helpers.ts @@ -7,7 +7,9 @@ import type { Plugin } from "../../../domain/models/plugin.js"; import type { PluginDistribution } from "../../../domain/models/plugin-distribution.js"; import type { AiToolId } from "../../../domain/models/tool-ids.js"; import { AI_TOOL_IDS } from "../../../domain/models/tool-ids.js"; +import type { FileReader } from "../../../domain/ports/file-reader.js"; import type { FileWriter } from "../../../domain/ports/file-writer.js"; +import type { Hasher } from "../../../domain/ports/hasher.js"; import type { ManifestRepository } from "../../../domain/ports/manifest-repository.js"; import { getToolConfig, isAiTool } from "../../../domain/tools/registry.js"; import { NoManifestError } from "../../errors.js"; @@ -63,10 +65,29 @@ export async function writePluginFiles( await Promise.all(files.map((f) => fs.writeFile(join(baseDir, f.relativePath), f.content))); } +/** Whether the file already on disk matches the content we would write, so a + * caller can skip the write and, more importantly, not count it as restored. */ +export async function isPluginFileAtDesiredState( + fs: FileReader, + hasher: Hasher, + outputPath: string, + expectedHashValue: string +): Promise { + if (!(await fs.fileExists(outputPath))) return false; + const content = await fs.readFile(outputPath); + return hasher.hash(content).value === expectedHashValue; +} + /** * Re-registers a plugin backed by the BUILT tree: drops the existing manifest entry * and lets the translator re-materialize + re-register it, so update and restore both * end up with the same single entry an install would have produced. + * + * Returns how many files the translator actually (re)wrote — not the plugin's total + * file count — so a no-op restore reports zero instead of claiming everything changed. + * `written` is undefined only on the rare fallback path where the translator can't + * resolve the marketplace and delegates to a strategy that doesn't track counts; that + * is reported as 0 rather than guessed. */ export async function materializeViaBuiltTree( translator: BuiltTreeMaterializationTranslator, @@ -76,9 +97,9 @@ export async function materializeViaBuiltTree( projectRoot: string, manifest: Manifest, docsDir: string -): Promise { +): Promise { manifest.removePlugin(toolId, plugin.name); - await translator.addPlugin( + const { written } = await translator.addPlugin( dist, toolId, plugin.source, @@ -87,4 +108,5 @@ export async function materializeViaBuiltTree( plugin.marketplace, docsDir ); + return written ?? 0; } diff --git a/cli/src/application/use-cases/plugin/translator/built-tree-materialization-translator.ts b/cli/src/application/use-cases/plugin/translator/built-tree-materialization-translator.ts index 8d373b350..0c4fadfad 100644 --- a/cli/src/application/use-cases/plugin/translator/built-tree-materialization-translator.ts +++ b/cli/src/application/use-cases/plugin/translator/built-tree-materialization-translator.ts @@ -11,7 +11,7 @@ import type { FileWriter } from "../../../../domain/ports/file-writer.js"; import type { Hasher } from "../../../../domain/ports/hasher.js"; import type { MarketplaceRegistry } from "../../../../domain/ports/marketplace-registry.js"; import type { EnsureBuiltMarketplaceUseCase } from "../../shared/ensure-built-marketplace-use-case.js"; -import { resolvePluginBaseDir, writePluginFiles } from "../plugin-helpers.js"; +import { isPluginFileAtDesiredState, resolvePluginBaseDir } from "../plugin-helpers.js"; import { ModeBFlatMaterializationTranslator } from "./mode-b-flat-materialization-translator.js"; import type { PluginTranslator } from "./plugin-translator.js"; @@ -44,7 +44,7 @@ export class BuiltTreeMaterializationTranslator implements PluginTranslator { marketplace: string | undefined, docsDir: string, previousMcpEntries: ReadonlyMap = new Map() - ): Promise<{ skipped: ReadonlySkipList }> { + ): Promise<{ skipped: ReadonlySkipList; written?: number }> { const resolved = marketplace === undefined ? null : await this.findMarketplace(marketplace, projectRoot); if (marketplace === undefined || resolved === null) { @@ -75,12 +75,27 @@ export class BuiltTreeMaterializationTranslator implements PluginTranslator { ); const baseDir = mode === "flat" ? projectRoot : resolvePluginBaseDir(toolId, projectRoot, this.homedir); - await writePluginFiles(files, baseDir, this.fs); + const written = await this.writeChangedFiles(files, baseDir); manifest.addPlugin( toolId, Plugin.fromDistribution(dist, source, files, new Map(), marketplace) ); - return { skipped: [] }; + return { skipped: [], written }; + } + + // Verbatim-copies the built subtree, but skips files already matching the built + // content on disk so a no-op restore reports (and performs) zero writes. + private async writeChangedFiles(files: InstallationFile[], baseDir: string): Promise { + let written = 0; + for (const f of files) { + const outputPath = join(baseDir, f.relativePath); + if (await isPluginFileAtDesiredState(this.fs, this.hasher, outputPath, f.hash.value)) { + continue; + } + await this.fs.writeFile(outputPath, f.content); + written++; + } + return written; } // Marketplace build emits plugins//; user-scope tools install at diff --git a/cli/src/application/use-cases/restore/restore-tool-files-use-case.ts b/cli/src/application/use-cases/restore/restore-tool-files-use-case.ts index 3ecdf787e..44eceb3e5 100644 --- a/cli/src/application/use-cases/restore/restore-tool-files-use-case.ts +++ b/cli/src/application/use-cases/restore/restore-tool-files-use-case.ts @@ -31,12 +31,14 @@ export interface RestoreToolFilesOptions { interface SectionRestoreResult { restored: string[]; kept: string[]; + unrestorable: string[]; updatedFiles: InstallationFile[]; } interface MergeSectionRestoreResult { restored: string[]; kept: string[]; + unrestorable: string[]; updatedMergeFiles: MergeFileEntry[]; } @@ -45,6 +47,7 @@ export interface RestoreToolFilesResult { nothingToRestore: boolean; restored: string[]; kept: string[]; + unrestorable: string[]; } export class RestoreToolFilesUseCase { @@ -63,7 +66,13 @@ export class RestoreToolFilesUseCase { const section = await this.restoreSection(options, distMap); const mergeSection = await this.restoreMergeSection(options, distMap); if (section === null && mergeSection === null) { - return { toolId: options.toolId, nothingToRestore: true, restored: [], kept: [] }; + return { + toolId: options.toolId, + nothingToRestore: true, + restored: [], + kept: [], + unrestorable: [], + }; } return this.commitToolRestore(options, section, mergeSection); } @@ -122,7 +131,8 @@ export class RestoreToolFilesUseCase { manifest.addTool(toolId, manifest.getToolVersion(toolId) ?? version, files, mergeFiles); const restored = [...(section?.restored ?? []), ...(mergeSection?.restored ?? [])]; const kept = [...(section?.kept ?? []), ...(mergeSection?.kept ?? [])]; - return { toolId, nothingToRestore: false, restored, kept }; + const unrestorable = [...(section?.unrestorable ?? []), ...(mergeSection?.unrestorable ?? [])]; + return { toolId, nothingToRestore: false, restored, kept, unrestorable }; } private existingFilesAsGenerated( diff --git a/cli/src/application/use-cases/restore/restore-use-case.ts b/cli/src/application/use-cases/restore/restore-use-case.ts index 648c96413..c4ae313cd 100644 --- a/cli/src/application/use-cases/restore/restore-use-case.ts +++ b/cli/src/application/use-cases/restore/restore-use-case.ts @@ -69,6 +69,7 @@ interface RestoreResult { totalKept: number; totalPluginFilesRestored: number; restoredPluginNames: string[]; + unrestorable: string[]; } export class RestoreUseCase { @@ -177,6 +178,7 @@ export class RestoreUseCase { totalKept: toolResults.reduce((s, t) => s + t.kept.length, 0), totalPluginFilesRestored: pluginResult.totalFiles, restoredPluginNames: pluginResult.pluginNames, + unrestorable: toolResults.flatMap((t) => t.unrestorable), }; } diff --git a/cli/src/application/use-cases/shared/apply-plugin-files-use-case.ts b/cli/src/application/use-cases/shared/apply-plugin-files-use-case.ts index bb36b5137..3ce7b9aee 100644 --- a/cli/src/application/use-cases/shared/apply-plugin-files-use-case.ts +++ b/cli/src/application/use-cases/shared/apply-plugin-files-use-case.ts @@ -11,7 +11,7 @@ import type { MarketplaceRegistry } from "../../../domain/ports/marketplace-regi import type { PluginDistributionReader } from "../../../domain/ports/plugin-distribution-reader.js"; import type { PluginFetcher } from "../../../domain/ports/plugin-fetcher.js"; import type { ToolConfig } from "../../../domain/tools/registry.js"; -import { materializeViaBuiltTree } from "../plugin/plugin-helpers.js"; +import { isPluginFileAtDesiredState, materializeViaBuiltTree } from "../plugin/plugin-helpers.js"; import { BuiltTreeMaterializationTranslator } from "../plugin/translator/built-tree-materialization-translator.js"; import { resolvePluginTranslator } from "../plugin/translator/resolve-plugin-translator.js"; import type { EnsureBuiltMarketplaceUseCase } from "./ensure-built-marketplace-use-case.js"; @@ -73,8 +73,15 @@ export class ApplyPluginFilesUseCase { options: ApplyPluginFilesOptions ): Promise { const { toolId, plugin, projectRoot, manifest, docsDir } = options; - await materializeViaBuiltTree(translator, dist, toolId, plugin, projectRoot, manifest, docsDir); - return manifest.getPlugins(toolId).find((p) => p.name === plugin.name)?.files.size ?? 0; + return materializeViaBuiltTree( + translator, + dist, + toolId, + plugin, + projectRoot, + manifest, + docsDir + ); } private async restoreViaTranslate( @@ -87,7 +94,7 @@ export class ApplyPluginFilesUseCase { for (const f of files) { if (fileFilter !== null && fileFilter !== undefined && !fileFilter(f.relativePath)) continue; const outputPath = join(projectRoot, f.relativePath); - if (!(await this.isFileAtDesiredState(outputPath, f.hash.value))) { + if (!(await isPluginFileAtDesiredState(this.fs, this.hasher, outputPath, f.hash.value))) { await this.fs.writeFile(outputPath, f.content); restored++; } @@ -98,13 +105,4 @@ export class ApplyPluginFilesUseCase { ); return restored; } - - private async isFileAtDesiredState( - outputPath: string, - expectedHashValue: string - ): Promise { - if (!(await this.fs.fileExists(outputPath))) return false; - const content = await this.fs.readFile(outputPath); - return this.hasher.hash(content).value === expectedHashValue; - } } 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 index b8cbc3d68..ed675323f 100644 --- 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 @@ -1,20 +1,31 @@ import type { Prompter } from "../../../domain/ports/prompter.js"; import { ResolveRestoreDecisionUseCase } from "./resolve-restore-decision.js"; -interface DriftDescriptor { +export interface DriftDescriptor { relativePath: string; reason: "deleted" | "modified"; } +/** + * What a leaf's drift scan found: entries that can actually be restored (`drift`), + * and entries the manifest still tracks as drifted but the current distribution no + * longer provides anything to restore them from (`unrestorable`) — e.g. a file + * dropped in a newer framework version, or a tool whose content set changed. + */ +export interface DriftCollection { + drift: TDrift[]; + unrestorable: DriftDescriptor[]; +} + /** * 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; + collectDrift(): Promise>; restore(entry: TDrift): Promise; - buildResult(restored: string[], kept: string[]): TResult; + buildResult(restored: string[], kept: string[], unrestorable: string[]): TResult; } /** @@ -35,8 +46,8 @@ export class RestoreDriftEntriesUseCase { force: boolean, interactive: boolean ): Promise { - const drift = await leaf.collectDrift(); - if (drift.length === 0) return null; + const { drift, unrestorable } = await leaf.collectDrift(); + if (drift.length === 0 && unrestorable.length === 0) return null; const restored: string[] = []; const kept: string[] = []; @@ -56,6 +67,10 @@ export class RestoreDriftEntriesUseCase { restored.push(entry.relativePath); } - return leaf.buildResult(restored, kept); + return leaf.buildResult( + restored, + kept, + unrestorable.map((entry) => entry.relativePath) + ); } } 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 5b9b6fb2a..cac2c371d 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,6 +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 type { DriftCollection, DriftDescriptor } from "./restore-drift-entries-use-case.js"; import { RestoreDriftEntriesUseCase } from "./restore-drift-entries-use-case.js"; interface MergeDriftEntry { @@ -31,6 +32,7 @@ interface MergeFilesRestoreOptions { export interface MergeFilesRestoreResult { restored: string[]; kept: string[]; + unrestorable: string[]; updatedMergeFiles: MergeFileEntry[]; } @@ -58,9 +60,10 @@ export class RestoreMergeFilesUseCase { options.fileFilter ), restore: (entry) => this.applyOneMergeRestore(entry, options.projectRoot, mergeMap), - buildResult: (restored, kept) => ({ + buildResult: (restored, kept, unrestorable) => ({ restored, kept, + unrestorable, updatedMergeFiles: [...mergeMap.values()], }), }, @@ -74,42 +77,38 @@ export class RestoreMergeFilesUseCase { distMap: Map, projectRoot: string, fileFilter: ((p: string) => boolean) | null - ): Promise { + ): Promise> { const drift: MergeDriftEntry[] = []; + const unrestorable: DriftDescriptor[] = []; for (const entry of mergeFiles) { if (fileFilter && !fileFilter(entry.relativePath)) continue; + const reason = await this.detectMergeDrift(entry, projectRoot); + if (reason === null) continue; + + // A file the current distribution no longer merge-tracks (dropped, or its + // strategy became "none") has nothing left to restore drift from. const distFile = distMap.get(entry.relativePath); - if (!distFile || distFile.mergeStrategy === "none") continue; - const driftEntry = await this.checkOneMergeFileDrift(entry, distFile, projectRoot); - if (driftEntry) drift.push(driftEntry); + if (!distFile || distFile.mergeStrategy === "none") { + unrestorable.push({ relativePath: entry.relativePath, reason }); + continue; + } + drift.push(this.buildDriftEntry(entry, distFile, reason)); } - return drift; + return { drift, unrestorable }; } - private async checkOneMergeFileDrift( + private async detectMergeDrift( entry: MergeFileEntry, - distFile: InstallationFile, projectRoot: string - ): Promise { + ): Promise<"deleted" | "modified" | null> { const diskPath = join(projectRoot, entry.relativePath); - if (!(await this.fs.fileExists(diskPath))) { - return this.buildDriftEntry(entry, distFile, "deleted"); - } - return this.checkModifiedDrift(diskPath, entry, distFile); - } - - private async checkModifiedDrift( - diskPath: string, - entry: MergeFileEntry, - distFile: InstallationFile - ): Promise { + if (!(await this.fs.fileExists(diskPath))) return "deleted"; const diskContent = await this.fs.readFile(diskPath); const diskEntries = extractMergeEntries(diskContent, entry.sectionKey, this.hasher); const hasDrift = Object.keys(entry.entries).some( (key) => diskEntries[key]?.value !== entry.entries[key].value ); - if (!hasDrift) return null; - return this.buildDriftEntry(entry, distFile, "modified"); + return hasDrift ? "modified" : null; } private buildDriftEntry( 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 c57207e8c..7cb4dc6ae 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,6 +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 type { DriftCollection, DriftDescriptor } from "./restore-drift-entries-use-case.js"; import { RestoreDriftEntriesUseCase } from "./restore-drift-entries-use-case.js"; interface DriftEntry { @@ -23,6 +24,7 @@ interface RegularFilesRestoreOptions { export interface RegularFilesRestoreResult { restored: string[]; kept: string[]; + unrestorable: string[]; updatedFiles: InstallationFile[]; } @@ -53,9 +55,10 @@ export class RestoreRegularFilesUseCase { await this.fs.writeFile(diskPath, entry.content); updatedHashMap.set(entry.relativePath, await this.fs.readFileHash(diskPath)); }, - buildResult: (restored, kept) => ({ + buildResult: (restored, kept, unrestorable) => ({ restored, kept, + unrestorable, updatedFiles: Array.from(updatedHashMap.entries()).map( ([relativePath, hash]) => new InstallationFile({ relativePath, content: "", hash }) ), @@ -71,38 +74,34 @@ export class RestoreRegularFilesUseCase { distMap: Map, projectRoot: string, fileFilter: ((p: string) => boolean) | null - ): Promise { + ): Promise> { const drift: DriftEntry[] = []; + const unrestorable: DriftDescriptor[] = []; for (const manifestFile of manifestFiles) { if (fileFilter && !fileFilter(manifestFile.relativePath)) continue; const diskPath = join(projectRoot, manifestFile.relativePath); - const diskExists = await this.fs.fileExists(diskPath); + const reason = await this.detectDrift(diskPath, manifestFile.hash.value); + if (reason === null) continue; - if (!diskExists) { - const distFile = distMap.get(manifestFile.relativePath); - if (distFile) - drift.push({ - relativePath: manifestFile.relativePath, - content: distFile.content, - reason: "deleted", - }); + const distFile = distMap.get(manifestFile.relativePath); + if (!distFile) { + unrestorable.push({ relativePath: manifestFile.relativePath, reason }); continue; } - - const diskHash = await this.fs.readFileHash(diskPath); - if (diskHash.value !== manifestFile.hash.value) { - const distFile = distMap.get(manifestFile.relativePath); - if (distFile) - drift.push({ - relativePath: manifestFile.relativePath, - content: distFile.content, - reason: "modified", - }); - } + drift.push({ relativePath: manifestFile.relativePath, content: distFile.content, reason }); } - return drift; + return { drift, unrestorable }; + } + + private async detectDrift( + diskPath: string, + manifestHashValue: string + ): Promise<"deleted" | "modified" | null> { + if (!(await this.fs.fileExists(diskPath))) return "deleted"; + const diskHash = await this.fs.readFileHash(diskPath); + return diskHash.value !== manifestHashValue ? "modified" : null; } } diff --git a/cli/tests/application/use-cases/shared/apply-plugin-files-built-tree.unit.test.ts b/cli/tests/application/use-cases/shared/apply-plugin-files-built-tree.unit.test.ts index 8ab6e6fc1..4b4ccfbda 100644 --- a/cli/tests/application/use-cases/shared/apply-plugin-files-built-tree.unit.test.ts +++ b/cli/tests/application/use-cases/shared/apply-plugin-files-built-tree.unit.test.ts @@ -128,6 +128,39 @@ describe("RestoreAllPluginsUseCase — built-tree materialization", () => { expect([...plugins[0].files.keys()].sort()).toEqual([...installedRelPaths].sort()); }); + it("reports zero files restored when a built-tree restore finds nothing drifted", async () => { + const deps = await buildUnitDeps(PROJECT_ROOT); + await initAndInstall(deps, PROJECT_ROOT, "cursor"); + const registry = await makeRegistry(); + await installMarketplacePlugin(deps, registry); + + const manifest = await deps.manifestRepo.load(); + if (manifest === null) throw new Error("manifest not found"); + const installedRelPaths = [ + ...(manifest + .getPlugins("cursor") + .find((p) => p.name === "sample-plugin") + ?.files.keys() ?? []), + ]; + expect(installedRelPaths.length).toBeGreaterThan(0); + const builtContent = deps.fs.getFile(BUILT_SKILL); + if (builtContent === undefined) throw new Error("built fixture missing"); + // Seed the user-scope plugin dir with exactly what the built tree already + // materializes, so this restore has nothing left to change. + for (const relativePath of installedRelPaths) { + await deps.fs.writeFile(join(USER_PLUGINS_DIR, relativePath), builtContent); + } + + const result = await makeRestoreUseCase(deps, registry).execute({ + projectRoot: PROJECT_ROOT, + manifest, + docsDir: DOCS_DIR, + fileFilter: null, + }); + + expect(result.totalFiles).toBe(0); + }); + it("keeps the manifest's single entry for the plugin after restore (no duplicate registration)", async () => { const deps = await buildUnitDeps(PROJECT_ROOT); await initAndInstall(deps, PROJECT_ROOT, "cursor"); 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 index a3e9a3c9b..d07c881a3 100644 --- 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 @@ -205,7 +205,7 @@ describe("RestoreMergeFilesUseCase", () => { expect(deps.fs.getFile(join(PROJECT_ROOT, "b.json"))).toBe(JSON.stringify({ x: "disk" })); }); - it("skips merge files whose distribution strategy is 'none'", async () => { + it("reports a drifted merge file as unrestorable when the 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()); @@ -231,6 +231,43 @@ describe("RestoreMergeFilesUseCase", () => { fileFilter: null, }); + expect(result?.unrestorable).toEqual(["a.json"]); + expect(result?.restored).toEqual([]); + expect(result?.kept).toEqual([]); + expect(deps.fs.getFile(join(PROJECT_ROOT, "a.json"))).toBe(JSON.stringify({ x: "disk" })); + }); + + it("returns null when a merge file's distribution strategy is 'none' and nothing drifted", async () => { + const deps = await buildDeps(); + const distContent = JSON.stringify({ x: "framework" }); + await deps.fs.writeFile(join(PROJECT_ROOT, "a.json"), distContent); + 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('"framework"') }, + }, + ], + distMap: new Map([ + [ + "a.json", + new InstallationFile({ + relativePath: "a.json", + content: distContent, + hash: deps.hasher.hash(distContent), + mergeStrategy: "none", + }), + ], + ]), + projectRoot: PROJECT_ROOT, + force: true, + interactive: false, + fileFilter: null, + }); + expect(result).toBeNull(); }); diff --git a/cli/tests/application/use-cases/shared/restore-regular-files-use-case.unit.test.ts b/cli/tests/application/use-cases/shared/restore-regular-files-use-case.unit.test.ts index 93f19d59c..3d208eeb1 100644 --- a/cli/tests/application/use-cases/shared/restore-regular-files-use-case.unit.test.ts +++ b/cli/tests/application/use-cases/shared/restore-regular-files-use-case.unit.test.ts @@ -268,7 +268,7 @@ describe("RestoreRegularFilesUseCase", () => { expect(deps.fs.getFile(join(PROJECT_ROOT, "b.md"))).toBe("disk modified b"); }); - it("silently drops a deleted file that has no corresponding entry in the dist map", async () => { + it("reports a deleted file with no corresponding dist entry as unrestorable, without touching disk", async () => { const deps = await buildDeps(); const useCase = new RestoreRegularFilesUseCase(deps.fs, new OverwritePrompter()); @@ -281,11 +281,13 @@ describe("RestoreRegularFilesUseCase", () => { fileFilter: null, }); - expect(result).toBeNull(); + expect(result?.unrestorable).toEqual(["a.md"]); + expect(result?.restored).toEqual([]); + expect(result?.kept).toEqual([]); expect(deps.fs.has(join(PROJECT_ROOT, "a.md"))).toBe(false); }); - it("silently drops a modified file that has no corresponding entry in the dist map", async () => { + it("reports a modified file with no corresponding dist entry as unrestorable, without touching disk", async () => { const deps = await buildDeps(); await deps.fs.writeFile(join(PROJECT_ROOT, "a.md"), "disk modified content"); const useCase = new RestoreRegularFilesUseCase(deps.fs, new OverwritePrompter()); @@ -299,7 +301,9 @@ describe("RestoreRegularFilesUseCase", () => { fileFilter: null, }); - expect(result).toBeNull(); + expect(result?.unrestorable).toEqual(["a.md"]); + expect(result?.restored).toEqual([]); + expect(result?.kept).toEqual([]); expect(deps.fs.getFile(join(PROJECT_ROOT, "a.md"))).toBe("disk modified content"); }); }); From 946af7484c4a057d440b5149d5124c7f801bc11b Mon Sep 17 00:00:00 2001 From: Baptiste LAFOURCADE Date: Wed, 29 Jul 2026 09:21:46 +0200 Subject: [PATCH 2/2] refactor(cli): warn about unrestorable files from one place The previous commit copy-pasted the same five-line warning, message string included, into ai.ts, ide.ts and restore.ts. That is the duplication this backlog exists to remove, reintroduced. printUnrestorable moves to display/restore-display.ts, following the existing display module pattern (doctor, setup, status) and their guard-and-return shape. The message is now defined once. 2146/2146 pass, tsc clean. --- cli/src/application/commands/ai.ts | 7 ++----- cli/src/application/commands/ide.ts | 7 ++----- cli/src/application/commands/restore.ts | 7 ++----- cli/src/application/display/restore-display.ts | 8 ++++++++ 4 files changed, 14 insertions(+), 15 deletions(-) create mode 100644 cli/src/application/display/restore-display.ts diff --git a/cli/src/application/commands/ai.ts b/cli/src/application/commands/ai.ts index 1dbb73314..bc29bb513 100644 --- a/cli/src/application/commands/ai.ts +++ b/cli/src/application/commands/ai.ts @@ -4,6 +4,7 @@ import type { AiToolId } from "../../domain/models/tool-ids.js"; import { AI_TOOL_IDS, isAiToolId } from "../../domain/models/tool-ids.js"; import type { ToolId } from "../../domain/tools/registry.js"; import { createDeps, createMenuDeps } from "../../infrastructure/deps.js"; +import { printUnrestorable } from "../display/restore-display.js"; import { ErrorHandler } from "../error-handler.js"; import { NoManifestError } from "../errors.js"; import { parseGlobalOptions } from "./global-options.js"; @@ -227,11 +228,7 @@ export function registerAiCommand(program: Command): void { output.success( `Restored ${restored} ${restored === 1 ? "file" : "files"}, kept ${kept} ${kept === 1 ? "file" : "files"}` ); - if (result.unrestorable.length > 0) { - output.warn( - `Could not restore ${result.unrestorable.length} file(s) no longer part of the current distribution: ${result.unrestorable.join(", ")}` - ); - } + printUnrestorable(output, result.unrestorable); } catch (error) { errorHandler.handle(error); } diff --git a/cli/src/application/commands/ide.ts b/cli/src/application/commands/ide.ts index 61e0183f2..75191fa80 100644 --- a/cli/src/application/commands/ide.ts +++ b/cli/src/application/commands/ide.ts @@ -3,6 +3,7 @@ import { Manifest } from "../../domain/models/manifest.js"; import { DOCS_DIR } from "../../domain/models/paths.js"; import { IDE_TOOL_IDS, type IdeToolId } from "../../domain/models/tool-ids.js"; import { createDeps, createMenuDeps } from "../../infrastructure/deps.js"; +import { printUnrestorable } from "../display/restore-display.js"; import { ErrorHandler } from "../error-handler.js"; import { NoManifestError } from "../errors.js"; import { parseGlobalOptions } from "./global-options.js"; @@ -214,11 +215,7 @@ export function registerIdeCommand(program: Command): void { output.success( `Restored ${result.totalRestored} ${result.totalRestored === 1 ? "file" : "files"}, kept ${result.totalKept} ${result.totalKept === 1 ? "file" : "files"}` ); - if (result.unrestorable.length > 0) { - output.warn( - `Could not restore ${result.unrestorable.length} file(s) no longer part of the current distribution: ${result.unrestorable.join(", ")}` - ); - } + printUnrestorable(output, result.unrestorable); } catch (error) { errorHandler.handle(error); } diff --git a/cli/src/application/commands/restore.ts b/cli/src/application/commands/restore.ts index b38cad6e6..9fc26b83c 100644 --- a/cli/src/application/commands/restore.ts +++ b/cli/src/application/commands/restore.ts @@ -1,5 +1,6 @@ import type { Command } from "commander"; import { createDeps } from "../../infrastructure/deps.js"; +import { printUnrestorable } from "../display/restore-display.js"; import { ErrorHandler } from "../error-handler.js"; import { parseGlobalOptions } from "./global-options.js"; @@ -35,11 +36,7 @@ export function registerRestoreCommand(program: Command): void { if (result.pluginNamesRestored.length > 0) { output.success(`Restored plugins: ${result.pluginNamesRestored.join(", ")}`); } - if (result.unrestorable.length > 0) { - output.warn( - `Could not restore ${result.unrestorable.length} file(s) no longer part of the current distribution: ${result.unrestorable.join(", ")}` - ); - } + printUnrestorable(output, result.unrestorable); } catch (error) { errorHandler.handle(error); } diff --git a/cli/src/application/display/restore-display.ts b/cli/src/application/display/restore-display.ts new file mode 100644 index 000000000..99a963602 --- /dev/null +++ b/cli/src/application/display/restore-display.ts @@ -0,0 +1,8 @@ +import type { CLIOutput } from "../output.js"; + +export function printUnrestorable(output: CLIOutput, unrestorable: readonly string[]): void { + if (unrestorable.length === 0) return; + output.warn( + `Could not restore ${unrestorable.length} file(s) no longer part of the current distribution: ${unrestorable.join(", ")}` + ); +}