From cbca1dc2cc835ff27cd1cf7adf6759a1651cad0f Mon Sep 17 00:00:00 2001 From: akushonkamen Date: Sun, 11 Oct 2026 19:30:35 +0800 Subject: [PATCH] fix(claude-plugin): target explicitly disabled destinations in scope migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- scripts/lib/claude-scope-migration.js | 86 ++++++++++++- tests/lib/claude-scope-migration.test.js | 151 +++++++++++++++++++++++ tests/scripts/setup.test.js | 1 + 3 files changed, 233 insertions(+), 5 deletions(-) diff --git a/scripts/lib/claude-scope-migration.js b/scripts/lib/claude-scope-migration.js index acb789f3b..389c3de76 100644 --- a/scripts/lib/claude-scope-migration.js +++ b/scripts/lib/claude-scope-migration.js @@ -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) diff --git a/tests/lib/claude-scope-migration.test.js b/tests/lib/claude-scope-migration.test.js index c3ab6606f..bace9db1c 100644 --- a/tests/lib/claude-scope-migration.test.js +++ b/tests/lib/claude-scope-migration.test.js @@ -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); diff --git a/tests/scripts/setup.test.js b/tests/scripts/setup.test.js index 715d383c2..806e31e24 100644 --- a/tests/scripts/setup.test.js +++ b/tests/scripts/setup.test.js @@ -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', ]);