mirror of
https://github.com/affaan-m/ECC.git
synced 2026-10-19 14:31:50 +02:00
fix(claude-plugin): target explicitly disabled destinations in scope migration
Issue #3492: scope migration aborted with generic verification errors in
two real states. When the destination scope is installed but inventory
reports enabled:false because the user explicitly set
enabledPlugins["ecc@ecc"]: false, setup now throws a targeted
DESTINATION_EXPLICITLY_DISABLED error naming the setting and file and
offering the native claude plugin enable recovery, leaving the user's
choice untouched. The generic not-enabled error also gains the enable
command in its recovery list.
uninstallSource now accepts a destination-only state when the source
scope is gone from the fresh inventory even if the uninstall command
errored, and SOURCE_UNINSTALL_FAILED recovery no longer suggests
uninstalling a source scope the inventory no longer lists; it prepends
claude plugin list --json so the stuck state can be inspected.
Tests: node tests/lib/claude-scope-migration.test.js — five new cases
cover explicitly disabled resume and single-scope destinations across
user/project/local settings files, the generic fallback keeping the
enable path, an uninstall error with the source already absent, and
non-circular recovery. 16 -> 21 passing; red against unpatched scripts
is 15 passed / 6 failed. Full-suite failure set is byte-identical to
unpatched HEAD in a zero-dependency environment.
Also sync the pinned SOURCE_UNINSTALL_FAILED recovery in
tests/scripts/setup.test.js ('migration JSON failures retain phase,
scopes, and exact recovery') with the new claude plugin list --json
diagnostic: 29 passed / 1 failed on the patched tree before the sync,
30 passed / 0 failed after, identical to clean 2d515e4f.
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
'use strict';
|
||||
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const {
|
||||
@@ -45,6 +46,63 @@ function recoveryCommands(sourceScope, destinationScope) {
|
||||
return commands;
|
||||
}
|
||||
|
||||
function enableRecoveryCommands(destinationScope) {
|
||||
return [
|
||||
`claude plugin enable ${CURRENT_PLUGIN_ID} --scope ${destinationScope}`,
|
||||
...recoveryCommands(null, destinationScope),
|
||||
];
|
||||
}
|
||||
|
||||
function destinationSettingsPaths(paths, destinationScope) {
|
||||
const projectSettingsDir = path.join(paths.projectRoot, '.claude');
|
||||
if (destinationScope === 'user') {
|
||||
return [path.join(paths.configDir, 'settings.json')];
|
||||
}
|
||||
if (destinationScope === 'project') {
|
||||
return [path.join(projectSettingsDir, 'settings.json')];
|
||||
}
|
||||
return [
|
||||
path.join(projectSettingsDir, 'settings.local.json'),
|
||||
path.join(projectSettingsDir, 'settings.json'),
|
||||
];
|
||||
}
|
||||
|
||||
function readExplicitlyDisabledSettings(settingsPath) {
|
||||
try {
|
||||
const settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8'));
|
||||
return settings?.enabledPlugins?.[CURRENT_PLUGIN_ID] === false;
|
||||
} catch {
|
||||
// Missing or unreadable settings are not an explicit user choice.
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
function destinationExplicitlyDisabledError(paths, plugins, destinationScope) {
|
||||
const installed = currentEccPlugins(plugins);
|
||||
const destination = installed.find(plugin => plugin.scope === destinationScope);
|
||||
if (!destination || destination.enabled === true) {
|
||||
return null;
|
||||
}
|
||||
const disabledSettingsPaths = destinationSettingsPaths(paths, destinationScope)
|
||||
.filter(readExplicitlyDisabledSettings);
|
||||
if (disabledSettingsPaths.length === 0) {
|
||||
return null;
|
||||
}
|
||||
return migrationError(
|
||||
'DESTINATION_EXPLICITLY_DISABLED',
|
||||
`${CURRENT_PLUGIN_ID} is installed at ${destinationScope} scope but explicitly `
|
||||
+ `disabled by enabledPlugins["${CURRENT_PLUGIN_ID}"]: false in `
|
||||
+ `${disabledSettingsPaths.join(', ')}. ECC keeps that choice and will not `
|
||||
+ `rewrite the setting; enable the plugin to continue the migration.`,
|
||||
{
|
||||
phase: 'destination-verification',
|
||||
observedScopes: installed.map(plugin => plugin.scope),
|
||||
settingsPaths: disabledSettingsPaths,
|
||||
recovery: enableRecoveryCommands(destinationScope),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
function readPluginInventory(run, projectRoot, phase) {
|
||||
return parsePluginList(
|
||||
run(
|
||||
@@ -95,7 +153,7 @@ function assertMigrationInventory(plugins, destinationScope) {
|
||||
{
|
||||
phase: 'destination-verification',
|
||||
observedScopes,
|
||||
recovery: recoveryCommands(null, destinationScope),
|
||||
recovery: enableRecoveryCommands(destinationScope),
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -208,23 +266,33 @@ function uninstallSource(run, paths, migration, destinationScope) {
|
||||
assertNoConflictingEccPlugins(plugins);
|
||||
const installed = currentEccPlugins(plugins);
|
||||
observedScopes = installed.map(plugin => plugin.scope);
|
||||
const destination = installed.find(
|
||||
plugin => plugin.scope === destinationScope
|
||||
);
|
||||
if (
|
||||
installed.length === 1
|
||||
&& installed[0].scope === destinationScope
|
||||
&& installed[0].enabled === true
|
||||
destination
|
||||
&& destination.enabled !== false
|
||||
&& !installed.some(plugin => plugin.scope === migration.sourceScope)
|
||||
) {
|
||||
return ['Claude reported an uninstall error, but destination-only state was verified.'];
|
||||
}
|
||||
} catch {
|
||||
// Preserve the safest known two-scope state in the structured recovery.
|
||||
}
|
||||
const sourcePresent = observedScopes.includes(migration.sourceScope);
|
||||
throw migrationError(
|
||||
'SOURCE_UNINSTALL_FAILED',
|
||||
`The destination is installed, but Claude could not remove the ${migration.sourceScope} source scope.`,
|
||||
{
|
||||
phase: 'source-uninstall',
|
||||
observedScopes,
|
||||
recovery: recoveryCommands(migration.sourceScope, destinationScope),
|
||||
recovery: [
|
||||
'claude plugin list --json',
|
||||
...recoveryCommands(
|
||||
sourcePresent ? migration.sourceScope : null,
|
||||
destinationScope
|
||||
),
|
||||
],
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -265,6 +333,14 @@ function migrateClaudePluginScope(options = {}, dependencies = {}) {
|
||||
? createDryRunClaudeRunner(providerRun, paths, options)
|
||||
: providerRun;
|
||||
const plugins = readPluginInventory(run, paths.projectRoot, 'inventory');
|
||||
const disabledDestinationError = destinationExplicitlyDisabledError(
|
||||
paths,
|
||||
plugins,
|
||||
options.scope
|
||||
);
|
||||
if (disabledDestinationError) {
|
||||
throw disabledDestinationError;
|
||||
}
|
||||
const migration = assertMigrationInventory(plugins, options.scope);
|
||||
const hooks = options.hooks === undefined
|
||||
? deriveHookMode(settings)
|
||||
|
||||
@@ -514,6 +514,7 @@ test('source uninstall failure reports both scopes and exact forward recovery',
|
||||
assert.strictEqual(error.phase, 'source-uninstall');
|
||||
assert.deepStrictEqual([...error.observedScopes].sort(), ['project', 'user']);
|
||||
assert.deepStrictEqual(error.recovery, [
|
||||
'claude plugin list --json',
|
||||
'claude plugin uninstall ecc@ecc --scope user --keep-data',
|
||||
'ecc setup --mode claude-plugin --scope project --move-scope --yes',
|
||||
]);
|
||||
@@ -670,5 +671,155 @@ test('migration preserves an explicit includeCoAuthoredBy opt-in', () => {
|
||||
});
|
||||
});
|
||||
|
||||
function writeExplicitDisable(filePath, pluginId = 'ecc@ecc') {
|
||||
fs.mkdirSync(path.dirname(filePath), { recursive: true });
|
||||
fs.writeFileSync(filePath, `${JSON.stringify({
|
||||
enabledPlugins: { [pluginId]: false },
|
||||
}, null, 2)}\n`);
|
||||
}
|
||||
|
||||
test('an explicitly disabled resume destination names the setting and the enable recovery', () => {
|
||||
withFixture({
|
||||
plugins: [plugin('user'), plugin('local', { enabled: false })],
|
||||
marketplaces: [marketplace('user')],
|
||||
}, fixture => {
|
||||
const localSettings = path.join(
|
||||
fixture.projectRoot,
|
||||
'.claude',
|
||||
'settings.local.json'
|
||||
);
|
||||
writeExplicitDisable(localSettings);
|
||||
const error = captureError(() => (
|
||||
migrateClaudePluginScope(migrationOptions(fixture, 'local'))
|
||||
));
|
||||
assert.strictEqual(error.code, 'DESTINATION_EXPLICITLY_DISABLED');
|
||||
assert.strictEqual(error.phase, 'destination-verification');
|
||||
assert.match(error.message, /enabledPlugins/);
|
||||
assert.ok(error.message.includes(localSettings));
|
||||
assert.deepStrictEqual(error.observedScopes, ['user', 'local']);
|
||||
assert.deepStrictEqual(error.recovery, [
|
||||
'claude plugin enable ecc@ecc --scope local',
|
||||
'ecc setup --mode claude-plugin --scope local --move-scope --yes',
|
||||
]);
|
||||
assert.deepStrictEqual(readCalls(fixture), [
|
||||
['plugin', 'list', '--json'],
|
||||
]);
|
||||
assert.ok(!readCalls(fixture).some(argv => argv[1] === 'uninstall'));
|
||||
assert.deepStrictEqual(
|
||||
readState(fixture).plugins.map(entry => entry.scope).sort(),
|
||||
['local', 'user']
|
||||
);
|
||||
assert.strictEqual(JSON.parse(
|
||||
fs.readFileSync(localSettings, 'utf8')
|
||||
).enabledPlugins['ecc@ecc'], false);
|
||||
});
|
||||
});
|
||||
|
||||
test('a single-scope explicitly disabled destination is not a generic verification failure', () => {
|
||||
for (const { scope, settingsPath } of [
|
||||
{ scope: 'user', settingsPath: 'config' },
|
||||
{ scope: 'project', settingsPath: 'project-settings' },
|
||||
{ scope: 'local', settingsPath: 'project-settings' },
|
||||
{ scope: 'local', settingsPath: 'project-local-settings' },
|
||||
]) {
|
||||
withFixture({ plugins: [plugin(scope, { enabled: false })] }, fixture => {
|
||||
const explicitPaths = {
|
||||
config: fixture.settingsPath,
|
||||
'project-settings': path.join(
|
||||
fixture.projectRoot, '.claude', 'settings.json'
|
||||
),
|
||||
'project-local-settings': path.join(
|
||||
fixture.projectRoot, '.claude', 'settings.local.json'
|
||||
),
|
||||
};
|
||||
const disabledPath = explicitPaths[settingsPath];
|
||||
writeExplicitDisable(disabledPath);
|
||||
const error = captureError(() => (
|
||||
migrateClaudePluginScope(migrationOptions(fixture, scope))
|
||||
));
|
||||
assert.strictEqual(error.code, 'DESTINATION_EXPLICITLY_DISABLED');
|
||||
assert.ok(error.message.includes(disabledPath));
|
||||
assert.deepStrictEqual(error.recovery, [
|
||||
`claude plugin enable ecc@ecc --scope ${scope}`,
|
||||
`ecc setup --mode claude-plugin --scope ${scope} --move-scope --yes`,
|
||||
]);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
test('a disabled destination without an explicit setting stays generic and gains the enable path', () => {
|
||||
withFixture({ plugins: [plugin('local', { enabled: false })] }, fixture => {
|
||||
const error = captureError(() => (
|
||||
migrateClaudePluginScope(migrationOptions(fixture, 'local'))
|
||||
));
|
||||
assert.strictEqual(error.code, 'DESTINATION_VERIFICATION_FAILED');
|
||||
assert.match(error.message, /exists at local scope but is not enabled/);
|
||||
assert.deepStrictEqual(error.recovery, [
|
||||
'claude plugin enable ecc@ecc --scope local',
|
||||
'ecc setup --mode claude-plugin --scope local --move-scope --yes',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
test('a source uninstall that errors but removes the source accepts the verified destination', () => {
|
||||
withFixture({
|
||||
plugins: [plugin('user'), plugin('project')],
|
||||
marketplaces: [marketplace('user')],
|
||||
failures: [{
|
||||
argv: uninstallArgv('user'),
|
||||
status: 9,
|
||||
stderr: 'uninstall failed',
|
||||
times: 1,
|
||||
}],
|
||||
pluginListResponses: [
|
||||
[plugin('user'), plugin('project')],
|
||||
[plugin('user'), plugin('project')],
|
||||
[plugin('user'), plugin('project')],
|
||||
[plugin('project'), plugin('local')],
|
||||
[plugin('project')],
|
||||
],
|
||||
}, fixture => {
|
||||
const result = migrateClaudePluginScope(
|
||||
migrationOptions(fixture, 'project')
|
||||
);
|
||||
assert.strictEqual(result.action, 'resumed');
|
||||
assert.strictEqual(result.sourceScope, 'user');
|
||||
assert.deepStrictEqual(result.warnings, [
|
||||
'Claude reported an uninstall error, but destination-only state was verified.',
|
||||
]);
|
||||
assert.ok(!readCalls(fixture).slice(-2).some(argv => argv[1] === 'uninstall'));
|
||||
});
|
||||
});
|
||||
|
||||
test('source uninstall failure recovery stops suggesting an already-absent source', () => {
|
||||
withFixture({
|
||||
plugins: [plugin('user'), plugin('project')],
|
||||
marketplaces: [marketplace('user')],
|
||||
failures: [{
|
||||
argv: uninstallArgv('user'),
|
||||
status: 9,
|
||||
stderr: 'uninstall failed',
|
||||
times: 1,
|
||||
}],
|
||||
pluginListResponses: [
|
||||
[plugin('user'), plugin('project')],
|
||||
[plugin('user'), plugin('project')],
|
||||
[plugin('user'), plugin('project')],
|
||||
[],
|
||||
],
|
||||
}, fixture => {
|
||||
const error = captureError(() => (
|
||||
migrateClaudePluginScope(migrationOptions(fixture, 'project'))
|
||||
));
|
||||
assert.strictEqual(error.code, 'SOURCE_UNINSTALL_FAILED');
|
||||
assert.strictEqual(error.phase, 'source-uninstall');
|
||||
assert.deepStrictEqual(error.observedScopes, []);
|
||||
assert.deepStrictEqual(error.recovery, [
|
||||
'claude plugin list --json',
|
||||
'ecc setup --mode claude-plugin --scope project --move-scope --yes',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`);
|
||||
process.exit(failed > 0 ? 1 : 0);
|
||||
|
||||
@@ -646,6 +646,7 @@ test('migration JSON failures retain phase, scopes, and exact recovery', () => {
|
||||
assert.strictEqual(payload.error.phase, 'source-uninstall');
|
||||
assert.deepStrictEqual([...payload.error.observedScopes].sort(), ['project', 'user']);
|
||||
assert.deepStrictEqual(payload.error.recovery, [
|
||||
'claude plugin list --json',
|
||||
'claude plugin uninstall ecc@ecc --scope user --keep-data',
|
||||
'ecc setup --mode claude-plugin --scope project --move-scope --yes',
|
||||
]);
|
||||
|
||||
Reference in New Issue
Block a user