fix: flatten Claude skill installs (#2582)

Flatten managed Claude skill destinations, preserve user-owned conflicts, and migrate legacy nested installs through the lifecycle tooling.
This commit is contained in:
Affaan Mustafa 2026-07-26 03:20:06 -07:00 committed by GitHub
parent 71438391e8
commit f3afd59045
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
18 changed files with 1620 additions and 119 deletions

View file

@ -5,7 +5,12 @@ const path = require('path');
const { writeInstallState } = require('../install-state');
const { filterMcpConfig, parseDisabledMcpServers } = require('../mcp-config');
const { buildInstallIndex, isNamespacedSource, rewriteRelativeLinks } = require('./link-rewrite');
const {
assertSafeClaudeSkillOperation,
prepareClaudeSkillMigration,
removeLegacyClaudeSkillFiles,
} = require('./claude-skill-migration');
const { buildInstallIndex, rewriteRelativeLinks } = require('./link-rewrite');
function isMarkdownPath(filePath) {
return /\.(md|mdx|markdown)$/i.test(String(filePath || ''));
@ -139,13 +144,49 @@ function buildResolvedClaudeHooks(plan) {
};
}
function applyInstallPlan(plan) {
const resolvedClaudeHooksPlan = buildResolvedClaudeHooks(plan);
const disabledServers = parseDisabledMcpServers(process.env.ECC_DISABLED_MCPS);
const linkIndex = buildLinkIndexForPlan(plan);
function previewInstallPlan(plan) {
const migration = prepareClaudeSkillMigration(plan);
return {
...plan,
statePreview: migration.finalState,
plannedOperations: [...plan.operations],
operations: migration.appliedOperations,
skippedOperations: migration.skippedOperations,
warnings: [
...(Array.isArray(plan.warnings) ? plan.warnings : []),
...migration.warnings,
],
applied: false,
};
}
for (const operation of plan.operations) {
function applyInstallPlan(plan, dependencies = {}) {
const persistInstallState = dependencies.writeInstallState || writeInstallState;
const migration = prepareClaudeSkillMigration(plan);
const appliedPlan = {
...plan,
operations: migration.appliedOperations,
};
const resolvedClaudeHooksPlan = buildResolvedClaudeHooks(appliedPlan);
const disabledServers = parseDisabledMcpServers(process.env.ECC_DISABLED_MCPS);
const linkIndex = buildLinkIndexForPlan(appliedPlan);
const hasLegacyMigration = migration.legacyOperationsToRemove.length > 0;
if (migration.requiresBridgeState) {
// Own every operation that may be written during a flat-skill migration
// before the first copy. A later failure is retryable and uninstall can
// clean the entire partial install, including non-skill files. During
// legacy migration the bridge also retains the prior managed operations.
persistInstallState(plan.installStatePath, migration.bridgeState);
}
for (const operation of appliedPlan.operations) {
assertSafeClaudeSkillOperation(appliedPlan, operation);
fs.mkdirSync(path.dirname(operation.destinationPath), { recursive: true });
// Recheck directories that were absent during the first validation. This
// narrows the symlink-swap window around mkdirSync, but path checks cannot
// eliminate a later TOCTOU race before the file write.
assertSafeClaudeSkillOperation(appliedPlan, operation);
if (operation.kind === 'merge-json') {
const payload = cloneJsonValue(operation.mergePayload);
@ -174,16 +215,14 @@ function applyInstallPlan(plan) {
continue;
}
// Namespaced markdown (e.g. skills/<id> -> skills/ecc/<id>) needs its
// relative cross-directory links rewritten so they resolve after install
// (issue #2340). Files whose install path is unchanged (no namespace
// injected) and all non-markdown files stay on the byte-for-byte copy path.
// Markdown may reference files whose installed paths move, such as rules
// copied under rules/ecc. Rewrite only links that point at installed targets;
// untouched links and non-markdown files stay on the byte-for-byte path.
if (
linkIndex
&& operation.kind === 'copy-file'
&& operation.sourceRelativePath
&& isMarkdownPath(operation.destinationPath)
&& isNamespacedSource(operation.sourceRelativePath, linkIndex)
) {
const rewritten = rewriteRelativeLinks(
fs.readFileSync(operation.sourcePath, 'utf8'),
@ -205,14 +244,26 @@ function applyInstallPlan(plan) {
);
}
writeInstallState(plan.installStatePath, plan.statePreview);
if (hasLegacyMigration) {
removeLegacyClaudeSkillFiles(migration, plan.targetRoot);
}
persistInstallState(plan.installStatePath, migration.finalState);
return {
...plan,
statePreview: migration.finalState,
plannedOperations: [...plan.operations],
operations: migration.appliedOperations,
skippedOperations: migration.skippedOperations,
warnings: [
...(Array.isArray(plan.warnings) ? plan.warnings : []),
...migration.warnings,
],
applied: true,
};
}
module.exports = {
applyInstallPlan,
previewInstallPlan,
};

View file

@ -0,0 +1,415 @@
'use strict';
const fs = require('fs');
const path = require('path');
const { readInstallState } = require('../install-state');
const { assertWithinTrustedRoot } = require('../path-safety');
const CLAUDE_TARGETS = new Set(['claude', 'claude-project']);
function pathExists(filePath) {
try {
fs.lstatSync(filePath);
return true;
} catch (error) {
if (error && error.code === 'ENOENT') {
return false;
}
throw error;
}
}
function normalizeSourceRelativePath(sourceRelativePath) {
const slashNormalized = String(sourceRelativePath || '').replace(/\\/g, '/');
const normalized = path.posix.normalize(slashNormalized).replace(/^\.\//, '');
if (
!normalized
|| normalized === '.'
|| normalized === '..'
|| normalized.startsWith('../')
|| path.posix.isAbsolute(normalized)
) {
return null;
}
return normalized;
}
function comparablePath(filePath) {
const resolvedPath = path.resolve(filePath);
return process.platform === 'win32' ? resolvedPath.toLowerCase() : resolvedPath;
}
function samePath(leftPath, rightPath) {
return comparablePath(leftPath) === comparablePath(rightPath);
}
function assertSafeSkillPath(targetPath, targetRoot, action) {
const resolvedRoot = path.resolve(targetRoot);
const resolvedTarget = path.resolve(targetPath);
const relativePath = path.relative(resolvedRoot, resolvedTarget);
if (
relativePath === ''
|| relativePath.startsWith('..')
|| path.isAbsolute(relativePath)
) {
throw new Error(
`Refusing to ${action} outside the install root: '${targetPath}' is not within '${targetRoot}'.`
);
}
let currentPath = resolvedRoot;
for (const segment of relativePath.split(path.sep)) {
currentPath = path.join(currentPath, segment);
let stats;
try {
stats = fs.lstatSync(currentPath);
} catch (error) {
if (error && error.code === 'ENOENT') {
break;
}
throw error;
}
if (stats.isSymbolicLink()) {
throw new Error(
`Refusing to ${action} through symlinked Claude skill path: '${currentPath}'.`
);
}
}
if (pathExists(targetRoot)) {
assertWithinTrustedRoot(targetPath, targetRoot, action);
}
}
function describeClaudeSkillOperation(targetRoot, operation) {
if (!operation || operation.kind !== 'copy-file') {
return null;
}
const sourceRelativePath = normalizeSourceRelativePath(operation.sourceRelativePath);
if (!sourceRelativePath) {
return null;
}
const sourceParts = sourceRelativePath.split('/');
if (sourceParts[0] !== 'skills' || sourceParts.length < 3 || !sourceParts[1]) {
return null;
}
const skillName = sourceParts[1];
const relativeParts = sourceParts.slice(2);
const flatSkillRoot = path.join(targetRoot, 'skills', skillName);
const legacySkillRoot = path.join(targetRoot, 'skills', 'ecc', skillName);
return {
sourceKey: sourceRelativePath,
skillName,
flatSkillRoot,
flatDestinationPath: path.join(flatSkillRoot, ...relativeParts),
legacySkillRoot,
legacyDestinationPath: path.join(legacySkillRoot, ...relativeParts),
};
}
function assertSafeClaudeSkillOperation(plan, operation) {
const target = plan && plan.adapter && plan.adapter.target;
if (!CLAUDE_TARGETS.has(target)) {
return;
}
const descriptor = describeClaudeSkillOperation(plan.targetRoot, operation);
if (!descriptor || !samePath(operation.destinationPath, descriptor.flatDestinationPath)) {
return;
}
assertSafeSkillPath(
operation.destinationPath,
plan.targetRoot,
'install Claude skill'
);
}
function isManagedOperation(operation) {
return operation && operation.ownership === 'managed';
}
function uniqueOperations(operations) {
const seen = new Set();
return operations.filter(operation => {
const key = [
operation.kind,
normalizeSourceRelativePath(operation.sourceRelativePath) || operation.sourceRelativePath,
comparablePath(operation.destinationPath),
].join('\0');
if (seen.has(key)) {
return false;
}
seen.add(key);
return true;
});
}
function buildState(statePreview, operations) {
return {
...statePreview,
operations: uniqueOperations(operations).map(operation => ({ ...operation })),
};
}
function groupCurrentSkillOperations(plan) {
const groups = new Map();
for (const operation of plan.operations) {
const descriptor = describeClaudeSkillOperation(plan.targetRoot, operation);
if (!descriptor || !samePath(operation.destinationPath, descriptor.flatDestinationPath)) {
continue;
}
assertSafeSkillPath(
operation.destinationPath,
plan.targetRoot,
'install Claude skill'
);
const current = groups.get(descriptor.flatSkillRoot) || [];
current.push({ operation, descriptor });
groups.set(descriptor.flatSkillRoot, current);
}
return groups;
}
function classifyPreviousOperations(plan, previousState) {
const flatByDestination = new Map();
const legacyBySource = new Map();
const legacyBySkillRoot = new Map();
for (const operation of (previousState && previousState.operations) || []) {
if (!isManagedOperation(operation)) {
continue;
}
const descriptor = describeClaudeSkillOperation(plan.targetRoot, operation);
if (!descriptor) {
continue;
}
if (samePath(operation.destinationPath, descriptor.flatDestinationPath)) {
assertSafeSkillPath(
operation.destinationPath,
plan.targetRoot,
'inspect managed Claude skill'
);
flatByDestination.set(comparablePath(operation.destinationPath), operation);
continue;
}
if (!samePath(operation.destinationPath, descriptor.legacyDestinationPath)) {
continue;
}
assertSafeSkillPath(
operation.destinationPath,
plan.targetRoot,
'migrate managed Claude skill'
);
legacyBySource.set(descriptor.sourceKey, operation);
const current = legacyBySkillRoot.get(descriptor.legacySkillRoot) || [];
current.push({ operation, descriptor });
legacyBySkillRoot.set(descriptor.legacySkillRoot, current);
}
return {
flatByDestination,
legacyBySource,
legacyBySkillRoot,
};
}
function createConflictWarning(skillName, flatSkillRoot, retainsLegacy) {
const legacySuffix = retainsLegacy
? ' The existing ECC-managed nested copy was retained and remains tracked for uninstall.'
: '';
return `Skipped Claude skill '${skillName}' at ${flatSkillRoot}: the flat skill directory is user-owned because it is not recorded in ECC install-state.${legacySuffix}`;
}
function createFileConflictWarning(destinationPath, retainsLegacy) {
const legacySuffix = retainsLegacy
? ' The matching ECC-managed nested file was retained and remains tracked for uninstall.'
: '';
return `Skipped user-owned Claude skill file ${destinationPath}: the existing file is not recorded in ECC install-state.${legacySuffix}`;
}
function createDisabledMigration(plan) {
return {
enabled: false,
appliedOperations: [...plan.operations],
skippedOperations: [],
warnings: [],
bridgeState: plan.statePreview,
finalState: plan.statePreview,
legacyOperationsToRemove: [],
requiresBridgeState: false,
};
}
function collectRetainedLegacyOperations(currentGroups, previous) {
const currentSourceKeys = new Set(
[...currentGroups.values()]
.flat()
.map(({ descriptor }) => descriptor.sourceKey)
);
return (
[...previous.legacyBySource.entries()]
.filter(([sourceKey]) => !currentSourceKeys.has(sourceKey))
.map(([_sourceKey, operation]) => operation)
);
}
function classifySkillGroup(flatSkillRoot, entries, previous) {
const hasManagedFlatFile = entries.some(({ operation }) => (
previous.flatByDestination.has(comparablePath(operation.destinationPath))
));
const legacyEntries = previous.legacyBySkillRoot.get(
entries[0].descriptor.legacySkillRoot
) || [];
if (pathExists(flatSkillRoot) && !hasManagedFlatFile) {
return {
skippedOperations: entries.map(({ operation }) => operation),
warnings: [createConflictWarning(
entries[0].descriptor.skillName,
flatSkillRoot,
legacyEntries.length > 0
)],
retainedLegacyOperations: legacyEntries.map(({ operation }) => operation),
};
}
const conflicts = entries.filter(({ operation }) => (
pathExists(operation.destinationPath)
&& !previous.flatByDestination.has(comparablePath(operation.destinationPath))
));
return {
skippedOperations: conflicts.map(({ operation }) => operation),
warnings: conflicts.map(({ operation, descriptor }) => createFileConflictWarning(
operation.destinationPath,
previous.legacyBySource.has(descriptor.sourceKey)
)),
retainedLegacyOperations: conflicts
.map(({ descriptor }) => previous.legacyBySource.get(descriptor.sourceKey))
.filter(Boolean),
};
}
function classifySkillConflicts(currentGroups, previous) {
const groupClassifications = [...currentGroups.entries()]
.map(([flatSkillRoot, entries]) => classifySkillGroup(
flatSkillRoot,
entries,
previous
));
const skippedOperations = groupClassifications
.flatMap(classification => classification.skippedOperations);
return {
skippedOperations,
skippedDestinations: new Set(
skippedOperations.map(operation => comparablePath(operation.destinationPath))
),
warnings: groupClassifications.flatMap(classification => classification.warnings),
retainedLegacyOperations: new Set([
...collectRetainedLegacyOperations(currentGroups, previous),
...groupClassifications.flatMap(
classification => classification.retainedLegacyOperations
),
]),
};
}
function buildMigrationStates(plan, previousState, previous, classification) {
const { skippedDestinations, retainedLegacyOperations } = classification;
const appliedOperations = plan.operations.filter(operation => (
!skippedDestinations.has(comparablePath(operation.destinationPath))
));
const legacyOperations = [...previous.legacyBySource.values()];
const legacyOperationsToRemove = legacyOperations.filter(operation => (
!retainedLegacyOperations.has(operation)
));
const finalOperations = [
...plan.statePreview.operations.filter(operation => (
!skippedDestinations.has(comparablePath(operation.destinationPath))
)),
...retainedLegacyOperations,
];
const bridgeOperations = [
...((previousState && previousState.operations) || []),
...appliedOperations,
];
return {
appliedOperations,
bridgeState: buildState(plan.statePreview, bridgeOperations),
finalState: buildState(plan.statePreview, finalOperations),
legacyOperationsToRemove,
requiresBridgeState: appliedOperations.length > 0,
};
}
function prepareClaudeSkillMigration(plan) {
const target = plan && plan.adapter && plan.adapter.target;
if (!CLAUDE_TARGETS.has(target)) {
return createDisabledMigration(plan);
}
const previousState = pathExists(plan.installStatePath)
? readInstallState(plan.installStatePath)
: null;
const currentGroups = groupCurrentSkillOperations(plan);
const previous = classifyPreviousOperations(plan, previousState);
const classification = classifySkillConflicts(currentGroups, previous);
const states = buildMigrationStates(
plan,
previousState,
previous,
classification
);
return {
enabled: true,
appliedOperations: states.appliedOperations,
skippedOperations: classification.skippedOperations,
warnings: classification.warnings,
bridgeState: states.bridgeState,
finalState: states.finalState,
legacyOperationsToRemove: states.legacyOperationsToRemove,
requiresBridgeState: states.requiresBridgeState,
};
}
function cleanupEmptyLegacyParents(filePath, targetRoot) {
const skillsRoot = path.join(targetRoot, 'skills');
let currentPath = path.dirname(filePath);
while (!samePath(currentPath, skillsRoot)) {
assertSafeSkillPath(currentPath, targetRoot, 'clean Claude skill migration');
if (!pathExists(currentPath) || fs.readdirSync(currentPath).length > 0) {
return;
}
fs.rmdirSync(currentPath);
currentPath = path.dirname(currentPath);
}
}
function removeLegacyClaudeSkillFiles(migration, targetRoot) {
for (const operation of migration.legacyOperationsToRemove) {
assertSafeSkillPath(
operation.destinationPath,
targetRoot,
'migrate managed Claude skill'
);
fs.rmSync(operation.destinationPath, { force: true });
cleanupEmptyLegacyParents(operation.destinationPath, targetRoot);
}
}
module.exports = {
assertSafeClaudeSkillOperation,
prepareClaudeSkillMigration,
removeLegacyClaudeSkillFiles,
};

View file

@ -22,7 +22,7 @@ function stripTrailingSlash(value) {
// `fileMappings` is a list of { sourceRel, destRel } where both are paths
// relative to the repo root and the install root respectively. The directory
// map is derived by walking shared ancestors of each source/dest pair, which is
// exact for prefix-insertion namespacing (e.g. `skills/x` -> `skills/ecc/x`):
// exact for prefix-insertion namespacing (e.g. `rules/x` -> `rules/ecc/x`):
// the path suffix below the inserted segment is preserved, so ancestor `k`
// of the source maps to the dest with the matching number of trailing
// segments removed.
@ -94,27 +94,16 @@ function resolveInstalledTarget(target, sourceDir, index) {
return null;
}
// True when the plan installs `sourceRel` at a different relative path than the
// source (i.e. a namespace segment was injected, e.g. skills/x -> skills/ecc/x).
// Callers use this to keep non-namespaced files on the byte-for-byte copy path.
function isNamespacedSource(sourceRel, index) {
const normalizedSource = toPosix(sourceRel);
const installedSource = index && index.byFile.get(normalizedSource);
return Boolean(installedSource) && installedSource !== normalizedSource;
}
// Rewrite relative links in a single namespaced markdown file so they resolve
// to the file's installed location. Returns the content unchanged when the
// file itself was not namespaced or when no link needs adjustment. Pure: no IO.
// Rewrite relative links in a markdown file so they resolve to installed target
// locations. The source file may itself install at the same relative path; links
// can still need changes when their targets move, such as rules -> rules/ecc.
// Pure: no IO.
function rewriteRelativeLinks(content, options) {
const { sourceRel, index } = options || {};
const normalizedSource = toPosix(sourceRel);
const installedSource = index && index.byFile.get(normalizedSource);
// Only rewrite when the file's own install path gained/changed a namespace
// segment. If it lands at the same relative path, every link recomputes to
// itself, so there is nothing to do.
if (!installedSource || installedSource === normalizedSource) {
if (!installedSource) {
return content;
}
@ -174,6 +163,5 @@ function rewriteRelativeLinks(content, options) {
module.exports = {
buildInstallIndex,
isNamespacedSource,
rewriteRelativeLinks,
};