From 05dea222382c06f6bc2d1ee229b0854e7a708c07 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Tue, 24 Mar 2026 13:34:34 -0400 Subject: [PATCH] fix(shared-manager): allow external claude symlink chains --- src/management/shared-manager.ts | 14 +++---- tests/unit/shared-manager.test.ts | 65 +++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 8 deletions(-) diff --git a/src/management/shared-manager.ts b/src/management/shared-manager.ts index b4054c22..bb8cc7ac 100644 --- a/src/management/shared-manager.ts +++ b/src/management/shared-manager.ts @@ -149,7 +149,7 @@ class SharedManager { /** * Detect circular symlink before creation */ - private detectCircularSymlink(target: string, linkPath: string): boolean { + private detectCircularSymlink(target: string): boolean { // Check if target exists and is symlink if (!fs.existsSync(target)) { return false; @@ -165,15 +165,13 @@ class SharedManager { const targetLink = fs.readlinkSync(target); const resolvedTarget = path.resolve(path.dirname(target), targetLink); - // Check if target points back to our shared dir or link path + // Only treat targets inside the managed shared root as circular. + // Existing shared symlinks may already resolve through ~/.claude/ to an + // external repo, which is a supported upgrade path rather than a loop. const sharedDir = this.resolveCanonicalPath(path.join(getCcsDir(), 'shared')); const canonicalResolvedTarget = this.resolveCanonicalPath(resolvedTarget); - const canonicalLinkPath = this.resolveCanonicalPath(linkPath); - if ( - this.isPathWithinDirectory(canonicalResolvedTarget, sharedDir) || - canonicalResolvedTarget === canonicalLinkPath - ) { + if (this.isPathWithinDirectory(canonicalResolvedTarget, sharedDir)) { console.log(warn(`Circular symlink detected: ${target} → ${resolvedTarget}`)); return true; } @@ -219,7 +217,7 @@ class SharedManager { } // Check for circular symlink - if (this.detectCircularSymlink(claudePath, sharedPath)) { + if (this.detectCircularSymlink(claudePath)) { console.log(warn(`Skipping ${item.name}: circular symlink detected`)); continue; } diff --git a/tests/unit/shared-manager.test.ts b/tests/unit/shared-manager.test.ts index 3cd142f8..dee54d4f 100644 --- a/tests/unit/shared-manager.test.ts +++ b/tests/unit/shared-manager.test.ts @@ -154,6 +154,71 @@ describe('SharedManager', () => { }); }); + describe('shared symlink lifecycle', () => { + it('preserves external ~/.claude symlinks during upgrade reconciliation', () => { + const manager = new SharedManager(); + const externalCommandsDir = path.join(tempRoot, 'Documents', 'claude-config', 'commands'); + const externalSettingsPath = path.join( + tempRoot, + 'Documents', + 'claude-config', + 'settings.json' + ); + const claudeCommandsPath = path.join(claudeDir(), 'commands'); + const claudeSettingsPath = path.join(claudeDir(), 'settings.json'); + const sharedCommandsPath = path.join(ccsDir(), 'shared', 'commands'); + const sharedSettingsPath = path.join(ccsDir(), 'shared', 'settings.json'); + const logSpy = spyOn(console, 'log').mockImplementation(() => {}); + + fs.mkdirSync(externalCommandsDir, { recursive: true }); + fs.mkdirSync(path.dirname(externalSettingsPath), { recursive: true }); + fs.mkdirSync(claudeDir(), { recursive: true }); + fs.mkdirSync(path.join(ccsDir(), 'shared'), { recursive: true }); + fs.writeFileSync(externalSettingsPath, JSON.stringify({ theme: 'dark' }), 'utf8'); + fs.symlinkSync(externalCommandsDir, claudeCommandsPath, 'dir'); + fs.symlinkSync(externalSettingsPath, claudeSettingsPath, 'file'); + fs.symlinkSync(claudeCommandsPath, sharedCommandsPath, 'dir'); + fs.symlinkSync(claudeSettingsPath, sharedSettingsPath, 'file'); + + manager.ensureSharedDirectories(); + + expect( + logSpy.mock.calls.some(([message]) => + String(message).includes('Skipping commands: circular symlink detected') || + String(message).includes('Skipping settings.json: circular symlink detected') + ) + ).toBe(false); + expect(fs.lstatSync(sharedCommandsPath).isSymbolicLink()).toBe(true); + expect( + path.resolve(path.dirname(sharedCommandsPath), fs.readlinkSync(sharedCommandsPath)) + ).toBe(claudeCommandsPath); + expect(fs.lstatSync(sharedSettingsPath).isSymbolicLink()).toBe(true); + expect( + path.resolve(path.dirname(sharedSettingsPath), fs.readlinkSync(sharedSettingsPath)) + ).toBe(claudeSettingsPath); + }); + + it('still blocks real circular links back into ~/.ccs/shared', () => { + const manager = new SharedManager(); + const claudeCommandsPath = path.join(claudeDir(), 'commands'); + const sharedCommandsPath = path.join(ccsDir(), 'shared', 'commands'); + const logSpy = spyOn(console, 'log').mockImplementation(() => {}); + + fs.mkdirSync(claudeDir(), { recursive: true }); + fs.mkdirSync(sharedCommandsPath, { recursive: true }); + fs.symlinkSync(sharedCommandsPath, claudeCommandsPath, 'dir'); + + manager.ensureSharedDirectories(); + + expect( + logSpy.mock.calls.some(([message]) => + String(message).includes('Skipping commands: circular symlink detected') + ) + ).toBe(true); + expect(fs.lstatSync(sharedCommandsPath).isDirectory()).toBe(true); + }); + }); + describe('marketplace registry ownership', () => { it('writes global and instance registries with different authoritative install locations', () => { const globalRegistryPath = path.join(claudeDir(), 'plugins', 'known_marketplaces.json');