From fc02c4b9682af6205735bdfd221dfb668f1821ae Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Wed, 18 Mar 2026 07:55:09 -0400 Subject: [PATCH] fix(management): harden marketplace state transitions - detach verified shared symlinks when a profile is reopened as bare - prune stale marketplace entries whose payload directories no longer exist - add regressions for bare/non-bare transitions and stale registry recovery --- src/management/instance-manager.ts | 1 + src/management/shared-manager.ts | 147 ++++++++++++++++++- tests/unit/instance-manager-mcp-sync.test.ts | 93 ++++++++++++ tests/unit/shared-manager.test.ts | 37 +++++ 4 files changed, 277 insertions(+), 1 deletion(-) diff --git a/src/management/instance-manager.ts b/src/management/instance-manager.ts index d6e2f141..54302ada 100644 --- a/src/management/instance-manager.ts +++ b/src/management/instance-manager.ts @@ -65,6 +65,7 @@ class InstanceManager { return; } + this.sharedManager.detachSharedDirectories(instancePath); this.sharedManager.normalizeSharedPluginMetadataPaths(); }); }); diff --git a/src/management/shared-manager.ts b/src/management/shared-manager.ts index 7e87c198..a5318dcc 100644 --- a/src/management/shared-manager.ts +++ b/src/management/shared-manager.ts @@ -307,6 +307,31 @@ class SharedManager { this.normalizeSharedPluginMetadataPaths(instancePath); } + detachSharedDirectories(instancePath: string): void { + this.ensureSharedDirectories(); + + for (const item of this.sharedItems) { + const managedPath = path.join(instancePath, item.name); + if (!fs.existsSync(managedPath)) { + continue; + } + + if (item.name === 'plugins') { + this.detachManagedPluginLayout(instancePath); + continue; + } + + const stats = fs.lstatSync(managedPath); + if (!stats.isSymbolicLink()) { + continue; + } + + if (this.symlinkPointsTo(managedPath, path.join(this.sharedDir, item.name))) { + this.removeExistingPath(managedPath, item.type); + } + } + } + private ensureSharedPluginLayoutDefaults(): void { const pluginsDir = path.join(this.claudeDir, 'plugins'); fs.mkdirSync(pluginsDir, { recursive: true, mode: 0o700 }); @@ -906,7 +931,9 @@ class SharedManager { } } - for (const [name, value] of Object.entries(this.discoverMarketplaceEntries(targetConfigDir))) { + const discoveredEntries = this.discoverMarketplaceEntries(targetConfigDir); + + for (const [name, value] of Object.entries(discoveredEntries)) { const existing = merged[name]; if (existing && typeof existing === 'object' && !Array.isArray(existing)) { merged[name] = { @@ -919,6 +946,12 @@ class SharedManager { merged[name] = value; } + for (const name of Object.keys(merged)) { + if (!(name in discoveredEntries)) { + delete merged[name]; + } + } + return JSON.stringify(merged, null, 2); } @@ -1442,6 +1475,118 @@ class SharedManager { return candidate; } + private symlinkPointsTo(linkPath: string, expectedTarget: string): boolean { + try { + const currentTarget = fs.readlinkSync(linkPath); + const resolvedCurrentTarget = path.resolve(path.dirname(linkPath), currentTarget); + return ( + this.resolveCanonicalPath(resolvedCurrentTarget) === + this.resolveCanonicalPath(expectedTarget) + ); + } catch { + return false; + } + } + + private detachManagedPluginLayout(instancePath: string): void { + const pluginsPath = path.join(instancePath, 'plugins'); + if (!fs.existsSync(pluginsPath)) { + return; + } + + const stats = fs.lstatSync(pluginsPath); + const sharedPluginsPath = path.join(this.sharedDir, 'plugins'); + + if (stats.isSymbolicLink()) { + if (this.symlinkPointsTo(pluginsPath, sharedPluginsPath)) { + this.removeExistingPath(pluginsPath, 'directory'); + } + return; + } + + if (!stats.isDirectory()) { + return; + } + + let removedManagedEntries = false; + + for (const item of this.getSharedPluginLinkItems()) { + const pluginEntryPath = path.join(pluginsPath, item.name); + if (!fs.existsSync(pluginEntryPath)) { + continue; + } + + const entryStats = fs.lstatSync(pluginEntryPath); + if (!entryStats.isSymbolicLink()) { + continue; + } + + if (this.symlinkPointsTo(pluginEntryPath, path.join(sharedPluginsPath, item.name))) { + this.removeExistingPath(pluginEntryPath, item.type); + removedManagedEntries = true; + } + } + + if (!removedManagedEntries) { + return; + } + + this.reconcileLocalMarketplaceRegistry(instancePath); + + if (fs.readdirSync(pluginsPath).length === 0) { + fs.rmSync(pluginsPath, { recursive: true, force: true }); + } + } + + private reconcileLocalMarketplaceRegistry(configDir: string): void { + const registryPath = path.join(configDir, 'plugins', 'known_marketplaces.json'); + if (!fs.existsSync(registryPath)) { + return; + } + + const discoveredEntries = this.discoverMarketplaceEntries(configDir); + if (Object.keys(discoveredEntries).length === 0) { + this.removeExistingPath(registryPath, 'file'); + return; + } + + let parsed: Record = {}; + try { + const raw = JSON.parse(fs.readFileSync(registryPath, 'utf8')) as unknown; + if (raw && typeof raw === 'object' && !Array.isArray(raw)) { + parsed = raw as Record; + } + } catch { + parsed = {}; + } + + const reconciled = Object.fromEntries( + Object.entries(discoveredEntries).map(([name, value]) => { + const existing = parsed[name]; + if (existing && typeof existing === 'object' && !Array.isArray(existing)) { + return [ + name, + { + ...(normalizePluginMetadataValue(existing, configDir).normalized as Record< + string, + unknown + >), + installLocation: value.installLocation, + }, + ]; + } + + return [name, value]; + }) + ); + + this.writePluginMetadataFile( + registryPath, + JSON.stringify(reconciled, null, 2), + 'Synchronized marketplace registry paths' + ); + } + private resolveCanonicalPath(targetPath: string): string { try { return fs.realpathSync.native(targetPath); diff --git a/tests/unit/instance-manager-mcp-sync.test.ts b/tests/unit/instance-manager-mcp-sync.test.ts index fea00791..748eea2a 100644 --- a/tests/unit/instance-manager-mcp-sync.test.ts +++ b/tests/unit/instance-manager-mcp-sync.test.ts @@ -17,6 +17,10 @@ describe('InstanceManager MCP sync', () => { const readJson = (filePath: string) => JSON.parse(fs.readFileSync(filePath, 'utf8')) as Record; + function ensureMarketplacePayload(configDir: string, name = 'claude-code-plugins'): void { + fs.mkdirSync(marketplacePath(configDir, name), { recursive: true }); + } + function writeMarketplaceRegistry(registryPath: string, installLocation: string): void { fs.mkdirSync(path.dirname(registryPath), { recursive: true }); fs.writeFileSync( @@ -169,6 +173,7 @@ describe('InstanceManager MCP sync', () => { const manager = new InstanceManager(); const instancePath = manager.getInstancePath('sandbox'); const globalRegistryPath = path.join(claudeDir(), 'plugins', 'known_marketplaces.json'); + ensureMarketplacePayload(claudeDir()); writeMarketplaceRegistry( globalRegistryPath, path.join( @@ -193,6 +198,90 @@ describe('InstanceManager MCP sync', () => { expect(syncMcpSpy).not.toHaveBeenCalled(); }); + it('detaches existing shared layout when an instance is reopened as bare', async () => { + spyOn(SharedManager.prototype, 'syncProjectContext').mockResolvedValue(undefined); + spyOn(SharedManager.prototype, 'syncAdvancedContinuityArtifacts').mockResolvedValue(undefined); + const syncMcpSpy = spyOn(InstanceManager.prototype, 'syncMcpServers').mockImplementation( + () => false + ); + + const manager = new InstanceManager(); + const instancePath = await manager.ensureInstance('work', { mode: 'isolated' }); + syncMcpSpy.mockClear(); + + await manager.ensureInstance('work', { mode: 'isolated' }, { bare: true }); + + expect(fs.existsSync(path.join(instancePath, 'settings.json'))).toBe(false); + expect(fs.existsSync(path.join(instancePath, 'commands'))).toBe(false); + expect(fs.existsSync(path.join(instancePath, 'skills'))).toBe(false); + expect(fs.existsSync(path.join(instancePath, 'agents'))).toBe(false); + expect(fs.existsSync(path.join(instancePath, 'plugins'))).toBe(false); + expect(syncMcpSpy).not.toHaveBeenCalled(); + }); + + it('restores the shared layout when a bare-reopened instance is switched back to non-bare', async () => { + spyOn(SharedManager.prototype, 'syncProjectContext').mockResolvedValue(undefined); + spyOn(SharedManager.prototype, 'syncAdvancedContinuityArtifacts').mockResolvedValue(undefined); + const syncMcpSpy = spyOn(InstanceManager.prototype, 'syncMcpServers').mockImplementation( + () => false + ); + + const globalRegistryPath = path.join(claudeDir(), 'plugins', 'known_marketplaces.json'); + ensureMarketplacePayload(claudeDir()); + writeMarketplaceRegistry(globalRegistryPath, marketplacePath(claudeDir())); + + const manager = new InstanceManager(); + const instancePath = await manager.ensureInstance('work', { mode: 'isolated' }); + await manager.ensureInstance('work', { mode: 'isolated' }, { bare: true }); + syncMcpSpy.mockClear(); + + await manager.ensureInstance('work', { mode: 'isolated' }); + + expect(fs.lstatSync(path.join(instancePath, 'settings.json')).isSymbolicLink()).toBe(true); + expectMarketplaceLocation( + path.join(instancePath, 'plugins', 'known_marketplaces.json'), + marketplacePath(instancePath) + ); + expect(syncMcpSpy).toHaveBeenCalledWith(instancePath); + }); + + it('preserves genuine bare-local content when re-ensuring a bare instance', async () => { + spyOn(SharedManager.prototype, 'syncProjectContext').mockResolvedValue(undefined); + spyOn(SharedManager.prototype, 'syncAdvancedContinuityArtifacts').mockResolvedValue(undefined); + const syncMcpSpy = spyOn(InstanceManager.prototype, 'syncMcpServers').mockImplementation( + () => false + ); + + const manager = new InstanceManager(); + const instancePath = manager.getInstancePath('sandbox'); + fs.mkdirSync(path.join(instancePath, 'plugins', 'marketplaces', 'custom-market'), { + recursive: true, + }); + fs.mkdirSync(path.join(instancePath, 'commands'), { recursive: true }); + fs.writeFileSync( + path.join(instancePath, 'settings.json'), + JSON.stringify({ local: true }, null, 2), + 'utf8' + ); + fs.writeFileSync(path.join(instancePath, 'commands', 'local.md'), '# local', 'utf8'); + writeMarketplaceRegistry( + path.join(instancePath, 'plugins', 'known_marketplaces.json'), + marketplacePath(instancePath, 'custom-market') + ); + + await manager.ensureInstance('sandbox', { mode: 'isolated' }, { bare: true }); + + expect(readJson(path.join(instancePath, 'settings.json'))).toEqual({ local: true }); + expect(fs.readFileSync(path.join(instancePath, 'commands', 'local.md'), 'utf8')).toBe( + '# local' + ); + expectMarketplaceLocation( + path.join(instancePath, 'plugins', 'known_marketplaces.json'), + marketplacePath(instancePath, 'custom-market') + ); + expect(syncMcpSpy).not.toHaveBeenCalled(); + }); + it('rewrites existing non-bare instance marketplace metadata to the instance-local plugin dir', async () => { spyOn(SharedManager.prototype, 'syncProjectContext').mockResolvedValue(undefined); spyOn(SharedManager.prototype, 'syncAdvancedContinuityArtifacts').mockResolvedValue(undefined); @@ -202,6 +291,7 @@ describe('InstanceManager MCP sync', () => { const manager = new InstanceManager(); const instancePath = manager.getInstancePath('work'); + ensureMarketplacePayload(claudeDir()); writeMarketplaceRegistry( path.join(instancePath, 'plugins', 'known_marketplaces.json'), path.join(tempRoot, '.claude', 'plugins', 'marketplaces', 'claude-code-plugins') @@ -224,6 +314,7 @@ describe('InstanceManager MCP sync', () => { ); const globalRegistryPath = path.join(claudeDir(), 'plugins', 'known_marketplaces.json'); + ensureMarketplacePayload(claudeDir()); writeMarketplaceRegistry( globalRegistryPath, path.join( @@ -253,6 +344,7 @@ describe('InstanceManager MCP sync', () => { spyOn(SharedManager.prototype, 'syncAdvancedContinuityArtifacts').mockResolvedValue(undefined); const manager = new InstanceManager(); + ensureMarketplacePayload(claudeDir()); const workPath = await manager.ensureInstance('work', { mode: 'isolated' }); const workRegistryPath = path.join(workPath, 'plugins', 'known_marketplaces.json'); writeMarketplaceRegistryWithMetadata(workRegistryPath, marketplacePath(workPath), { @@ -315,6 +407,7 @@ describe('InstanceManager MCP sync', () => { fs.mkdirSync(legacyPath, { recursive: true }); fs.symlinkSync(sharedPluginsPath, path.join(legacyPath, 'plugins'), 'dir'); + ensureMarketplacePayload(claudeDir()); writeMarketplaceRegistryWithMetadata( path.join(claudeDir(), 'plugins', 'known_marketplaces.json'), path.join(tempRoot, '.ccs', 'shared', 'plugins', 'marketplaces', 'claude-code-plugins'), diff --git a/tests/unit/shared-manager.test.ts b/tests/unit/shared-manager.test.ts index 88caf1f2..3cd142f8 100644 --- a/tests/unit/shared-manager.test.ts +++ b/tests/unit/shared-manager.test.ts @@ -22,6 +22,10 @@ describe('SharedManager', () => { const readJson = (filePath: string) => JSON.parse(fs.readFileSync(filePath, 'utf8')) as Record; + function ensureMarketplacePayload(configDir: string, name = 'claude-code-plugins'): void { + fs.mkdirSync(marketplacePath(configDir, name), { recursive: true }); + } + function writeJson(filePath: string, value: unknown): void { fs.mkdirSync(path.dirname(filePath), { recursive: true }); fs.writeFileSync(filePath, JSON.stringify(value, null, 2), 'utf8'); @@ -153,6 +157,7 @@ describe('SharedManager', () => { describe('marketplace registry ownership', () => { it('writes global and instance registries with different authoritative install locations', () => { const globalRegistryPath = path.join(claudeDir(), 'plugins', 'known_marketplaces.json'); + ensureMarketplacePayload(claudeDir()); writeJson(globalRegistryPath, { 'claude-code-plugins': { installLocation: path.join( @@ -204,6 +209,36 @@ describe('SharedManager', () => { }); }); + it('prunes stale marketplace entries whose payload directories no longer exist', () => { + const manager = new SharedManager(); + const instancePath = instanceDir('work'); + fs.mkdirSync(instancePath, { recursive: true }); + manager.linkSharedDirectories(instancePath); + + fs.mkdirSync(marketplacePath(claudeDir(), 'claude-code-plugins'), { recursive: true }); + writeJson(path.join(instancePath, 'plugins', 'known_marketplaces.json'), { + 'claude-code-plugins': { + installLocation: marketplacePath(instancePath, 'claude-code-plugins'), + label: 'Official marketplace', + }, + stale: { + installLocation: marketplacePath(instancePath, 'stale'), + label: 'Stale marketplace', + }, + }); + + manager.normalizeMarketplaceRegistryPaths(instancePath); + + const reconciled = readJson( + path.join(instancePath, 'plugins', 'known_marketplaces.json') + ) as Record; + expect(reconciled['claude-code-plugins']).toEqual({ + installLocation: marketplacePath(instancePath, 'claude-code-plugins'), + label: 'Official marketplace', + }); + expect(reconciled.stale).toBeUndefined(); + }); + it('warns and skips malformed marketplace registries while keeping valid sources', () => { const manager = new SharedManager(); const instancePath = instanceDir('work'); @@ -211,6 +246,7 @@ describe('SharedManager', () => { manager.linkSharedDirectories(instancePath); const globalRegistryPath = path.join(claudeDir(), 'plugins', 'known_marketplaces.json'); + ensureMarketplacePayload(claudeDir()); writeJson(globalRegistryPath, { 'claude-code-plugins': { installLocation: path.join( @@ -249,6 +285,7 @@ describe('SharedManager', () => { }); const globalRegistryPath = path.join(claudeDir(), 'plugins', 'known_marketplaces.json'); + ensureMarketplacePayload(claudeDir()); writeJson(globalRegistryPath, { 'claude-code-plugins': { installLocation: path.join(