From c8c83ef42829707803794dfa6d54e77d47dcd855 Mon Sep 17 00:00:00 2001 From: Gaurav Dubey Date: Sat, 4 Jul 2026 09:07:32 +0530 Subject: [PATCH] fix(install): dedupe copy-file operations sharing a destination (#2429) OpenCode ships override command files under .opencode/commands/ that shadow the generic commands/*.md sources. The manifest install plan recorded both writes against the same destination, so `ecc doctor` reported perpetual drift for the 29 shadowed command files and `ecc repair` "fixed" that phantom drift by copying the generic source over the correct override, corrupting the installed command while never clearing the warning. Dedupe copy-file operations by destination in createManifestInstallPlan, keeping the last writer to match the sequential apply order. install, repair, and doctor all consume this one builder, so a fresh install is clean and a single repair rewrites drifted state green. Fixes #2414 --- scripts/lib/install-executor.js | 31 ++++++++++++++++- tests/lib/install-executor.test.js | 54 ++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/scripts/lib/install-executor.js b/scripts/lib/install-executor.js index e5855eccc..5147f3c4d 100644 --- a/scripts/lib/install-executor.js +++ b/scripts/lib/install-executor.js @@ -688,6 +688,32 @@ function materializeScaffoldOperation(sourceRoot, operation) { }); } +function dedupeCopyFileOperations(operations) { + // A `copy-file` operation fully overwrites its destination, so when several + // of them target the same path (e.g. a generic `commands/.md` shadowed + // by an OpenCode `.opencode/commands/.md` override) only the last one + // actually determines the installed content. Recording the shadowed earlier + // writes in install-state makes `doctor` report perpetual drift and drives + // `repair` to clobber the override with the generic source (issue #2414). + // Keep only the last `copy-file` per destination — matching the sequential + // apply order in applyInstallPlan — and leave every other operation kind + // (e.g. accumulating `merge-json` writes into a shared config) untouched and + // in order. + const lastCopyIndexByDestination = new Map(); + operations.forEach((operation, index) => { + if (operation.kind === 'copy-file' && operation.destinationPath) { + lastCopyIndexByDestination.set(operation.destinationPath, index); + } + }); + + return operations.filter((operation, index) => { + if (operation.kind !== 'copy-file' || !operation.destinationPath) { + return true; + } + return lastCopyIndexByDestination.get(operation.destinationPath) === index; + }); +} + function createManifestInstallPlan(options = {}) { const sourceRoot = options.sourceRoot || getSourceRoot(); const projectRoot = options.projectRoot || process.cwd(); @@ -716,7 +742,9 @@ function createManifestInstallPlan(options = {}) { target }); const adapter = getInstallTargetAdapter(target); - const operations = plan.operations.flatMap(operation => materializeScaffoldOperation(sourceRoot, operation)); + const operations = dedupeCopyFileOperations( + plan.operations.flatMap(operation => materializeScaffoldOperation(sourceRoot, operation)) + ); const source = { repoVersion: getPackageVersion(sourceRoot), repoCommit: getRepoCommit(sourceRoot), @@ -776,6 +804,7 @@ module.exports = { createLegacyCompatInstallPlan, createManifestInstallPlan, createLegacyInstallPlan, + dedupeCopyFileOperations, getSourceRoot, listAvailableLanguages, parseInstallArgs diff --git a/tests/lib/install-executor.test.js b/tests/lib/install-executor.test.js index 8b3cdcd68..b26fea924 100644 --- a/tests/lib/install-executor.test.js +++ b/tests/lib/install-executor.test.js @@ -14,6 +14,7 @@ const { createLegacyCompatInstallPlan, createLegacyInstallPlan, createManifestInstallPlan, + dedupeCopyFileOperations, listAvailableLanguages, } = require('../../scripts/lib/install-executor'); @@ -428,6 +429,59 @@ function runTests() { } })) passed++; else failed++; + if (test('dedupeCopyFileOperations keeps the last writer per destination (issue #2414)', () => { + // Mirrors the OpenCode command scenario: a generic commands/.md source + // (preserve-relative-path) and an override .opencode/commands/.md source + // (sync-root-children) both write the same destination. Before the fix both + // ops were recorded, so `doctor` reported perpetual drift and `repair` + // clobbered the override. Only the last writer (the override) should survive. + const dest = '/home/.opencode/commands/build-fix.md'; + const operations = [ + { kind: 'copy-file', sourceRelativePath: 'commands/build-fix.md', destinationPath: dest, strategy: 'preserve-relative-path' }, + { kind: 'copy-file', sourceRelativePath: '.opencode/commands/build-fix.md', destinationPath: dest, strategy: 'sync-root-children' }, + { kind: 'copy-file', sourceRelativePath: 'commands/other.md', destinationPath: '/home/.opencode/commands/other.md', strategy: 'preserve-relative-path' }, + ]; + + const deduped = dedupeCopyFileOperations(operations); + + const destinations = deduped + .filter(operation => operation.kind === 'copy-file') + .map(operation => operation.destinationPath); + assert.deepStrictEqual( + destinations, + [dest, '/home/.opencode/commands/other.md'], + 'each copy-file destination must appear exactly once' + ); + const survivor = deduped.find(operation => operation.destinationPath === dest); + assert.strictEqual( + survivor.sourceRelativePath, + '.opencode/commands/build-fix.md', + 'the last writer (override) must win, not the generic source' + ); + })) passed++; else failed++; + + if (test('dedupeCopyFileOperations leaves non copy-file operations and order intact', () => { + // merge-json operations legitimately accumulate into a shared config file, so + // multiple writes to one destination must be preserved; only redundant + // copy-file writes are collapsed, and surviving ops keep their relative order. + const mergeDest = '/home/.opencode/opencode.json'; + const operations = [ + { kind: 'merge-json', sourceRelativePath: 'a.json', destinationPath: mergeDest }, + { kind: 'copy-file', sourceRelativePath: 'src/x.md', destinationPath: '/home/x.md' }, + { kind: 'merge-json', sourceRelativePath: 'b.json', destinationPath: mergeDest }, + { kind: 'copy-file', sourceRelativePath: 'other/x.md', destinationPath: '/home/x.md' }, + { kind: 'remove', destinationPath: '/home/legacy.md' }, + ]; + + const deduped = dedupeCopyFileOperations(operations); + + assert.deepStrictEqual( + deduped.map(operation => `${operation.kind}:${operation.sourceRelativePath || operation.destinationPath}`), + ['merge-json:a.json', 'merge-json:b.json', 'copy-file:other/x.md', 'remove:/home/legacy.md'], + 'both merge-json writes and the remove op survive; only the shadowed copy-file is dropped, order preserved' + ); + })) passed++; else failed++; + console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); process.exit(failed > 0 ? 1 : 0); }