mirror of
https://github.com/affaan-m/ECC.git
synced 2026-08-17 21:15:40 +02:00
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
This commit is contained in:
@@ -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/<name>.md` shadowed
|
||||
// by an OpenCode `.opencode/commands/<name>.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
|
||||
|
||||
@@ -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/<name>.md source
|
||||
// (preserve-relative-path) and an override .opencode/commands/<name>.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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user