mirror of
https://github.com/affaan-m/ECC.git
synced 2026-09-20 16:47:59 +02:00
fix: reconcile pre-exclusion .agents installs on claude and codex home upgrades
This commit is contained in:
@@ -132,6 +132,13 @@ function printHumanPlan(plan, dryRun) {
|
||||
}
|
||||
}
|
||||
|
||||
if (Array.isArray(plan.reconciledExcludedPaths) && plan.reconciledExcludedPaths.length > 0) {
|
||||
console.log('\nReconciled excluded paths:');
|
||||
for (const removedPath of plan.reconciledExcludedPaths) {
|
||||
console.log(`- removed ${removedPath}`);
|
||||
}
|
||||
}
|
||||
|
||||
if (!dryRun) {
|
||||
console.log(`\nDone. Install-state written to ${plan.installStatePath}`);
|
||||
}
|
||||
|
||||
@@ -34,6 +34,10 @@ const {
|
||||
preserveUnwrittenFiles,
|
||||
} = require('./ownership-guard');
|
||||
const { cleanupLegacyOpencodeInstall } = require('./opencode-legacy-migration');
|
||||
const {
|
||||
completeExcludedPathsReconciliation,
|
||||
prepareExcludedPathsReconciliation,
|
||||
} = require('./excluded-paths-reconciliation');
|
||||
const { buildInstallIndex, rewriteRelativeLinks } = require('./link-rewrite');
|
||||
const { adaptAntigravityAgent } = require('./antigravity-agent');
|
||||
|
||||
@@ -449,9 +453,12 @@ function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = fals
|
||||
if (typeof beforeInstallStateRead === 'function') {
|
||||
beforeInstallStateRead({ plan });
|
||||
}
|
||||
const migration = prepareHookConsentMigration(
|
||||
const migration = prepareExcludedPathsReconciliation(
|
||||
plan,
|
||||
prepareUserOwnedFileGuard(plan, prepareClaudeSkillMigration(plan))
|
||||
prepareHookConsentMigration(
|
||||
plan,
|
||||
prepareUserOwnedFileGuard(plan, prepareClaudeSkillMigration(plan))
|
||||
)
|
||||
);
|
||||
const appliedPlan = {
|
||||
...plan,
|
||||
@@ -666,17 +673,31 @@ function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = fals
|
||||
];
|
||||
}
|
||||
|
||||
let excludedPathsRemoved = [];
|
||||
let excludedPathsWarnings = [];
|
||||
try {
|
||||
const excludedReconciliation = completeExcludedPathsReconciliation(migration, appliedPlan);
|
||||
excludedPathsRemoved = excludedReconciliation.removedPaths;
|
||||
excludedPathsWarnings = excludedReconciliation.warnings;
|
||||
} catch (error) {
|
||||
excludedPathsWarnings = [
|
||||
`Excluded-paths reconciliation did not finish: ${error.message}. Previously managed files under excluded source paths were preserved; remove them manually or rerun the install.`,
|
||||
];
|
||||
}
|
||||
|
||||
return {
|
||||
...plan,
|
||||
statePreview: finalState,
|
||||
plannedOperations: [...plan.operations],
|
||||
operations: migration.appliedOperations,
|
||||
skippedOperations: migration.skippedOperations,
|
||||
reconciledExcludedPaths: excludedPathsRemoved,
|
||||
warnings: [
|
||||
...(Array.isArray(plan.warnings) ? plan.warnings : []),
|
||||
...migration.warnings,
|
||||
...antigravityMigrationWarnings,
|
||||
...opencodeMigrationWarnings,
|
||||
...excludedPathsWarnings,
|
||||
],
|
||||
applied: true,
|
||||
};
|
||||
|
||||
@@ -0,0 +1,230 @@
|
||||
'use strict';
|
||||
|
||||
const crypto = require('crypto');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const { readInstallState } = require('../install-state');
|
||||
const { assertWithinTrustedRoot } = require('../path-safety');
|
||||
const { getInstallTargetAdapter } = require('../install-targets/registry');
|
||||
|
||||
/**
|
||||
* Upgrade reconciliation for excluded source paths (issue #3116).
|
||||
*
|
||||
* Adapters can declare `excludedSourcePaths` (today: `.agents` for the Claude
|
||||
* and Codex home targets). The exclusion stops new copy operations from being
|
||||
* planned, but a home install created before the exclusion still has the
|
||||
* copied files on disk and the copy operations recorded in install-state, so
|
||||
* doctor keeps reporting drift and repair keeps restoring files the target
|
||||
* never reads.
|
||||
*
|
||||
* prepareExcludedPathsReconciliation runs before the new state is written: it
|
||||
* reads the previous install-state and drops the recorded managed operations
|
||||
* whose source path is now excluded. completeExcludedPathsReconciliation runs
|
||||
* after a successful apply: it removes the files those operations recorded,
|
||||
* but only when the recorded content digest still matches, and prunes the
|
||||
* emptied directories. Files the state does not own, modified files,
|
||||
* symlinks, and anything outside the target root are preserved with a
|
||||
* warning.
|
||||
*/
|
||||
|
||||
function comparablePath(filePath) {
|
||||
const resolvedPath = path.resolve(filePath);
|
||||
return process.platform === 'win32' ? resolvedPath.toLowerCase() : resolvedPath;
|
||||
}
|
||||
|
||||
function getReconcilingAdapter(plan) {
|
||||
if (!plan || typeof plan.target !== 'string') {
|
||||
return null;
|
||||
}
|
||||
let adapter;
|
||||
try {
|
||||
adapter = getInstallTargetAdapter(plan.target);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
return adapter && typeof adapter.excludesSourcePath === 'function' ? adapter : null;
|
||||
}
|
||||
|
||||
function isRecordedExcludedManagedOperation(adapter, operation) {
|
||||
return Boolean(
|
||||
operation
|
||||
&& operation.ownership === 'managed'
|
||||
&& typeof operation.destinationPath === 'string'
|
||||
&& typeof operation.sourceRelativePath === 'string'
|
||||
&& adapter.excludesSourcePath(operation.sourceRelativePath)
|
||||
);
|
||||
}
|
||||
|
||||
function filterStateOperations(state, shouldDrop) {
|
||||
if (!state || !Array.isArray(state.operations)) {
|
||||
return state;
|
||||
}
|
||||
return {
|
||||
...state,
|
||||
operations: state.operations.filter(operation => !shouldDrop(operation)),
|
||||
};
|
||||
}
|
||||
|
||||
function prepareExcludedPathsReconciliation(plan, migration) {
|
||||
const adapter = getReconcilingAdapter(plan);
|
||||
if (!adapter || !fs.existsSync(plan.installStatePath)) {
|
||||
return { ...migration, excludedPathCandidates: [] };
|
||||
}
|
||||
|
||||
const previousState = readInstallState(plan.installStatePath);
|
||||
const candidates = ((previousState && previousState.operations) || [])
|
||||
.filter(operation => isRecordedExcludedManagedOperation(adapter, operation));
|
||||
|
||||
if (candidates.length === 0) {
|
||||
return { ...migration, excludedPathCandidates: [] };
|
||||
}
|
||||
|
||||
const droppedDestinations = new Set(
|
||||
candidates.map(operation => comparablePath(operation.destinationPath))
|
||||
);
|
||||
const shouldDrop = operation => Boolean(
|
||||
operation
|
||||
&& typeof operation.destinationPath === 'string'
|
||||
&& droppedDestinations.has(comparablePath(operation.destinationPath))
|
||||
&& typeof operation.sourceRelativePath === 'string'
|
||||
&& adapter.excludesSourcePath(operation.sourceRelativePath)
|
||||
);
|
||||
|
||||
return {
|
||||
...migration,
|
||||
bridgeState: filterStateOperations(migration.bridgeState, shouldDrop),
|
||||
finalState: filterStateOperations(migration.finalState, shouldDrop),
|
||||
excludedPathCandidates: candidates,
|
||||
};
|
||||
}
|
||||
|
||||
function pathExists(filePath) {
|
||||
try {
|
||||
fs.lstatSync(filePath);
|
||||
return true;
|
||||
} catch (error) {
|
||||
if (error && error.code === 'ENOENT') {
|
||||
return false;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
function hashFileNoFollow(filePath) {
|
||||
const flags = fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0);
|
||||
const descriptor = fs.openSync(filePath, flags);
|
||||
try {
|
||||
const before = fs.fstatSync(descriptor, { bigint: true });
|
||||
if (!before.isFile()) {
|
||||
throw new Error(`Refusing to read a non-file at ${filePath}`);
|
||||
}
|
||||
const content = fs.readFileSync(descriptor);
|
||||
const after = fs.fstatSync(descriptor, { bigint: true });
|
||||
const finalPathStat = fs.lstatSync(filePath, { bigint: true });
|
||||
const unchanged = before.dev === after.dev
|
||||
&& before.ino === after.ino
|
||||
&& before.size === after.size
|
||||
&& after.dev === finalPathStat.dev
|
||||
&& after.ino === finalPathStat.ino
|
||||
&& after.size === finalPathStat.size;
|
||||
if (finalPathStat.isSymbolicLink() || !finalPathStat.isFile() || !unchanged) {
|
||||
throw new Error(`Refusing to read a file that changed during validation: ${filePath}`);
|
||||
}
|
||||
return crypto.createHash('sha256').update(content).digest('hex');
|
||||
} finally {
|
||||
fs.closeSync(descriptor);
|
||||
}
|
||||
}
|
||||
|
||||
function removeEmptyParents(startPath, targetRoot) {
|
||||
let currentPath = path.dirname(startPath);
|
||||
while (comparablePath(currentPath) !== comparablePath(targetRoot)) {
|
||||
const safePath = assertWithinTrustedRoot(
|
||||
currentPath,
|
||||
targetRoot,
|
||||
'reconcile excluded install paths'
|
||||
);
|
||||
if (!pathExists(safePath)) {
|
||||
currentPath = path.dirname(safePath);
|
||||
continue;
|
||||
}
|
||||
const stat = fs.lstatSync(safePath);
|
||||
if (!stat.isDirectory() || stat.isSymbolicLink() || fs.readdirSync(safePath).length > 0) {
|
||||
return;
|
||||
}
|
||||
fs.rmdirSync(safePath);
|
||||
currentPath = path.dirname(safePath);
|
||||
}
|
||||
}
|
||||
|
||||
function completeExcludedPathsReconciliation(migration, plan) {
|
||||
const candidates = (migration && migration.excludedPathCandidates) || [];
|
||||
const removedPaths = [];
|
||||
const warnings = [];
|
||||
|
||||
for (const candidate of candidates) {
|
||||
if (candidate.kind !== 'copy-file') {
|
||||
continue;
|
||||
}
|
||||
|
||||
let safePath;
|
||||
try {
|
||||
safePath = assertWithinTrustedRoot(
|
||||
candidate.destinationPath,
|
||||
plan.targetRoot,
|
||||
'reconcile excluded install paths'
|
||||
);
|
||||
} catch (error) {
|
||||
warnings.push(
|
||||
`Preserved previously managed file ${candidate.destinationPath}: ${error.message}`
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (!pathExists(safePath)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const stat = fs.lstatSync(safePath);
|
||||
if (stat.isSymbolicLink() || !stat.isFile()) {
|
||||
warnings.push(
|
||||
`Preserved previously managed file ${safePath}: it is not a regular file; remove it manually if unwanted.`
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (typeof candidate.contentSha256 !== 'string') {
|
||||
warnings.push(
|
||||
`Preserved previously managed file ${safePath}: the recorded operation has no content digest, so the file cannot be verified unchanged; remove it manually if unwanted.`
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
let currentDigest;
|
||||
try {
|
||||
currentDigest = hashFileNoFollow(safePath);
|
||||
} catch (error) {
|
||||
warnings.push(`Preserved previously managed file ${safePath}: ${error.message}`);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (currentDigest !== candidate.contentSha256.toLowerCase()) {
|
||||
warnings.push(
|
||||
`Preserved previously managed file ${safePath}: content changed after install; remove it manually if unwanted.`
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
fs.unlinkSync(safePath);
|
||||
removedPaths.push(safePath);
|
||||
removeEmptyParents(safePath, plan.targetRoot);
|
||||
}
|
||||
|
||||
return { removedPaths, warnings };
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
completeExcludedPathsReconciliation,
|
||||
prepareExcludedPathsReconciliation,
|
||||
};
|
||||
@@ -7,6 +7,7 @@ const fs = require('fs');
|
||||
const os = require('os');
|
||||
const path = require('path');
|
||||
const { execFileSync, spawnSync } = require('child_process');
|
||||
const crypto = require('crypto');
|
||||
const yaml = require('js-yaml');
|
||||
const { applyInstallPlan } = require('../../scripts/lib/install/apply');
|
||||
|
||||
@@ -641,6 +642,134 @@ function runTests() {
|
||||
}
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('reconciles legacy .agents files and state operations on Claude and Codex home upgrades', () => {
|
||||
const homeDir = createTempDir('install-apply-home-');
|
||||
const projectDir = createTempDir('install-apply-project-');
|
||||
const digest = content => crypto.createHash('sha256').update(content).digest('hex');
|
||||
const legacyOperation = (destinationPath, sourceRelativePath, installedContent) => ({
|
||||
kind: 'copy-file',
|
||||
moduleId: 'agents-core',
|
||||
sourceRelativePath,
|
||||
destinationPath,
|
||||
strategy: 'preserve-relative-path',
|
||||
ownership: 'managed',
|
||||
scaffoldOnly: false,
|
||||
contentSha256: digest(installedContent),
|
||||
});
|
||||
const writeFile = (filePath, content) => {
|
||||
fs.mkdirSync(path.dirname(filePath), { recursive: true });
|
||||
fs.writeFileSync(filePath, content);
|
||||
};
|
||||
const writeLegacyState = (statePath, target, operations) => {
|
||||
writeFile(statePath, `${JSON.stringify({
|
||||
schemaVersion: 'ecc.install.v1',
|
||||
installedAt: '2026-09-01T00:00:00.000Z',
|
||||
target,
|
||||
request: {
|
||||
profile: 'core',
|
||||
modules: [],
|
||||
includeComponents: [],
|
||||
excludeComponents: [],
|
||||
legacyLanguages: [],
|
||||
legacyMode: false,
|
||||
hookConsent: target.target === 'claude' ? 'enabled' : null,
|
||||
},
|
||||
resolution: { selectedModules: ['agents-core'], skippedModules: [] },
|
||||
source: { repoVersion: '2.2.1', repoCommit: null, manifestVersion: 1 },
|
||||
operations,
|
||||
}, null, 2)}\n`);
|
||||
};
|
||||
|
||||
try {
|
||||
// Claude home seeded as installed before the .agents exclusion.
|
||||
const claudeRoot = path.join(homeDir, '.claude');
|
||||
const claudeStatePath = path.join(claudeRoot, 'ecc', 'install-state.json');
|
||||
const claudeSkillCopy = path.join(claudeRoot, '.agents', 'skills', 'legacy-skill', 'SKILL.md');
|
||||
const claudeModifiedCopy = path.join(claudeRoot, '.agents', 'plugins', 'marketplace.json');
|
||||
const claudeUserFile = path.join(claudeRoot, '.agents', 'user-note.txt');
|
||||
writeFile(claudeSkillCopy, '# legacy skill\n');
|
||||
writeFile(claudeModifiedCopy, '{"edited": true}\n');
|
||||
writeFile(claudeUserFile, 'user notes\n');
|
||||
writeLegacyState(claudeStatePath, {
|
||||
id: 'claude-home', target: 'claude', kind: 'home',
|
||||
root: claudeRoot, installStatePath: claudeStatePath,
|
||||
}, [
|
||||
legacyOperation(claudeSkillCopy, '.agents/skills/legacy-skill/SKILL.md', '# legacy skill\n'),
|
||||
legacyOperation(claudeModifiedCopy, '.agents/plugins/marketplace.json', '{"original": true}\n'),
|
||||
]);
|
||||
|
||||
const claudeResult = run(['--profile', 'core', '--enable-hooks'], { cwd: projectDir, homeDir });
|
||||
assert.strictEqual(claudeResult.code, 0, claudeResult.stderr);
|
||||
|
||||
assert.ok(!fs.existsSync(claudeSkillCopy), 'Unchanged managed .agents file should be removed');
|
||||
assert.ok(
|
||||
claudeResult.stdout.includes(
|
||||
`- removed ${path.join(fs.realpathSync(claudeRoot), '.agents', 'skills', 'legacy-skill', 'SKILL.md')}`
|
||||
),
|
||||
'Install output should log one line per removed path'
|
||||
);
|
||||
assert.strictEqual(
|
||||
fs.readFileSync(claudeModifiedCopy, 'utf8'),
|
||||
'{"edited": true}\n',
|
||||
'Modified managed file must be preserved'
|
||||
);
|
||||
assert.strictEqual(
|
||||
fs.readFileSync(claudeUserFile, 'utf8'),
|
||||
'user notes\n',
|
||||
'Files the state does not own must not be touched'
|
||||
);
|
||||
assert.ok(
|
||||
!fs.existsSync(path.join(claudeRoot, '.agents', 'skills')),
|
||||
'Emptied .agents subdirectories should be pruned'
|
||||
);
|
||||
|
||||
const claudeState = readJson(claudeStatePath);
|
||||
assert.ok(
|
||||
!claudeState.operations.some(operation => (
|
||||
String(operation.sourceRelativePath || '').replace(/\\/g, '/').split('/')[0] === '.agents'
|
||||
)),
|
||||
'Claude install-state must drop the excluded .agents operations'
|
||||
);
|
||||
assert.ok(fs.existsSync(path.join(claudeRoot, 'agents', 'architect.md')));
|
||||
assert.ok(fs.existsSync(path.join(claudeRoot, 'skills', 'tdd-workflow', 'SKILL.md')));
|
||||
|
||||
// Codex home seeded the same way; both recorded files are unchanged.
|
||||
const codexRoot = path.join(homeDir, '.codex');
|
||||
const codexStatePath = path.join(codexRoot, 'ecc-install-state.json');
|
||||
const codexSkillCopy = path.join(codexRoot, '.agents', 'skills', 'legacy-skill', 'SKILL.md');
|
||||
const codexMarketplaceCopy = path.join(codexRoot, '.agents', 'plugins', 'marketplace.json');
|
||||
writeFile(codexSkillCopy, '# legacy skill\n');
|
||||
writeFile(codexMarketplaceCopy, '{"original": true}\n');
|
||||
writeLegacyState(codexStatePath, {
|
||||
id: 'codex-home', target: 'codex', kind: 'home',
|
||||
root: codexRoot, installStatePath: codexStatePath,
|
||||
}, [
|
||||
legacyOperation(codexSkillCopy, '.agents/skills/legacy-skill/SKILL.md', '# legacy skill\n'),
|
||||
legacyOperation(codexMarketplaceCopy, '.agents/plugins/marketplace.json', '{"original": true}\n'),
|
||||
]);
|
||||
|
||||
const codexResult = run(['--target', 'codex', '--profile', 'core'], { cwd: projectDir, homeDir });
|
||||
assert.strictEqual(codexResult.code, 0, codexResult.stderr);
|
||||
|
||||
assert.ok(
|
||||
!fs.existsSync(path.join(codexRoot, '.agents')),
|
||||
'Fully reconciled .agents directory should be pruned from the Codex home'
|
||||
);
|
||||
const codexState = readJson(codexStatePath);
|
||||
assert.ok(
|
||||
!codexState.operations.some(operation => (
|
||||
String(operation.sourceRelativePath || '').replace(/\\/g, '/').split('/')[0] === '.agents'
|
||||
)),
|
||||
'Codex install-state must drop the excluded .agents operations'
|
||||
);
|
||||
assert.ok(fs.existsSync(path.join(codexRoot, 'agents', 'architect.md')));
|
||||
assert.ok(fs.existsSync(path.join(codexRoot, 'skills', 'tdd-workflow', 'SKILL.md')));
|
||||
} finally {
|
||||
cleanup(homeDir);
|
||||
cleanup(projectDir);
|
||||
}
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('preserves existing top-level Claude rules and skills during managed install', () => {
|
||||
const homeDir = createTempDir('install-apply-home-');
|
||||
const projectDir = createTempDir('install-apply-project-');
|
||||
|
||||
Reference in New Issue
Block a user