From 640a5d9647521551d6005fb0b0021445040604e1 Mon Sep 17 00:00:00 2001 From: oritwoen <18102267+oritwoen@users.noreply.github.com> Date: Fri, 27 Mar 2026 14:22:07 +0100 Subject: [PATCH 1/2] fix: handle dangling symlinks in restorePkgSymlink existsSync follows symlinks and returns false for dangling ones, so symlinkSync throws EEXIST when trying to create over a stale link. Use lstatSync to detect and remove dangling symlinks before re-creating. --- src/core/prepare.ts | 18 ++++- test/unit/prepare-restore.test.ts | 116 ++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 2 deletions(-) create mode 100644 test/unit/prepare-restore.test.ts diff --git a/src/core/prepare.ts b/src/core/prepare.ts index 8844ffb4..b607497d 100644 --- a/src/core/prepare.ts +++ b/src/core/prepare.ts @@ -34,8 +34,22 @@ export function restorePkgSymlink(skillsDir: string, name: string, info: SkillIn if (!existsSync(join(skillsDir, name))) return - if (existsSync(pkgLink)) - return + // Use lstatSync to detect dangling symlinks — existsSync follows symlinks + // and returns false for dangling ones, causing symlinkSync to throw EEXIST + try { + const stat = lstatSync(pkgLink) + if (stat.isSymbolicLink()) { + if (existsSync(pkgLink)) + return // symlink exists and target is valid + unlinkSync(pkgLink) // dangling symlink — remove before re-creating + } + else { + return // real file/dir exists at this path + } + } + catch { + // path doesn't exist — continue to create symlink + } const pkgName = info.packageName || name const pkgDir = resolvePkgDir(pkgName, cwd, info.version) diff --git a/test/unit/prepare-restore.test.ts b/test/unit/prepare-restore.test.ts new file mode 100644 index 00000000..81f717cd --- /dev/null +++ b/test/unit/prepare-restore.test.ts @@ -0,0 +1,116 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('node:fs', async () => { + const actual = await vi.importActual('node:fs') + return { + ...actual, + existsSync: vi.fn(), + lstatSync: vi.fn(), + mkdirSync: vi.fn(), + symlinkSync: vi.fn(), + unlinkSync: vi.fn(), + } +}) + +vi.mock('../../src/cache/version', () => ({ + getCacheDir: (name: string, version: string) => `/home/.skilld/references/${name}@${version}`, +})) + +describe('restorePkgSymlink', () => { + beforeEach(() => vi.resetAllMocks()) + afterEach(() => vi.restoreAllMocks()) + + it('skips when skill directory does not exist', async () => { + const fs = await import('node:fs') + const { restorePkgSymlink } = await import('../../src/core/prepare') + + vi.mocked(fs.existsSync).mockImplementation((p) => { + // skill dir doesn't exist — triggers early return + if (String(p).endsWith('/vue')) + return false + return true + }) + + restorePkgSymlink('/project/.skills', 'vue', { version: '3.4.0' }, '/project') + + expect(fs.symlinkSync).not.toHaveBeenCalled() + }) + + it('skips when pkg symlink target is valid', async () => { + const fs = await import('node:fs') + const { restorePkgSymlink } = await import('../../src/core/prepare') + + vi.mocked(fs.existsSync).mockReturnValue(true) + vi.mocked(fs.lstatSync).mockReturnValue({ isSymbolicLink: () => true } as any) + + restorePkgSymlink('/project/.skills', 'vue', { version: '3.4.0' }, '/project') + + expect(fs.symlinkSync).not.toHaveBeenCalled() + }) + + it('removes dangling symlink before re-creating', async () => { + const fs = await import('node:fs') + const { restorePkgSymlink } = await import('../../src/core/prepare') + + vi.mocked(fs.existsSync).mockImplementation((p) => { + const path = String(p) + // skill dir exists + if (path === '/project/.skills/vue') + return true + // dangling symlink: existsSync returns false (follows symlink, target gone) + if (path.endsWith('/pkg')) + return false + // node_modules/vue exists (freshly installed) + if (path.includes('node_modules/vue')) + return true + return false + }) + // lstatSync succeeds — the symlink itself exists on disk + vi.mocked(fs.lstatSync).mockReturnValue({ isSymbolicLink: () => true } as any) + + restorePkgSymlink('/project/.skills', 'vue', { version: '3.4.0' }, '/project') + + // Should remove the dangling symlink first + expect(fs.unlinkSync).toHaveBeenCalledWith( + expect.stringContaining('pkg'), + ) + // Then create a fresh symlink + expect(fs.symlinkSync).toHaveBeenCalledOnce() + }) + + it('creates symlink when no prior link exists', async () => { + const fs = await import('node:fs') + const { restorePkgSymlink } = await import('../../src/core/prepare') + + vi.mocked(fs.existsSync).mockImplementation((p) => { + const path = String(p) + if (path === '/project/.skills/vue') + return true + if (path.includes('node_modules/vue')) + return true + return false + }) + // lstatSync throws — no file at all + vi.mocked(fs.lstatSync).mockImplementation(() => { + throw new Error('ENOENT') + }) + + restorePkgSymlink('/project/.skills', 'vue', { version: '3.4.0' }, '/project') + + expect(fs.unlinkSync).not.toHaveBeenCalled() + expect(fs.symlinkSync).toHaveBeenCalledOnce() + }) + + it('skips when real file exists at pkg path', async () => { + const fs = await import('node:fs') + const { restorePkgSymlink } = await import('../../src/core/prepare') + + vi.mocked(fs.existsSync).mockReturnValue(true) + // lstatSync returns a regular file, not a symlink + vi.mocked(fs.lstatSync).mockReturnValue({ isSymbolicLink: () => false } as any) + + restorePkgSymlink('/project/.skills', 'vue', { version: '3.4.0' }, '/project') + + expect(fs.symlinkSync).not.toHaveBeenCalled() + }) +}) From fc54e96504c84793508c89f0b0c3eadb9b6eb332 Mon Sep 17 00:00:00 2001 From: oritwoen <18102267+oritwoen@users.noreply.github.com> Date: Fri, 27 Mar 2026 14:54:20 +0100 Subject: [PATCH 2/2] fix: narrow catch to ENOENT in restorePkgSymlink Return early on permission/IO errors instead of swallowing them. --- src/core/prepare.ts | 5 +++-- test/unit/prepare-restore.test.ts | 6 ++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/core/prepare.ts b/src/core/prepare.ts index b607497d..3f7f94f8 100644 --- a/src/core/prepare.ts +++ b/src/core/prepare.ts @@ -47,8 +47,9 @@ export function restorePkgSymlink(skillsDir: string, name: string, info: SkillIn return // real file/dir exists at this path } } - catch { - // path doesn't exist — continue to create symlink + catch (err) { + if ((err as NodeJS.ErrnoException).code !== 'ENOENT') + return // permission/IO error — bail instead of masking } const pkgName = info.packageName || name diff --git a/test/unit/prepare-restore.test.ts b/test/unit/prepare-restore.test.ts index 81f717cd..9cdd9900 100644 --- a/test/unit/prepare-restore.test.ts +++ b/test/unit/prepare-restore.test.ts @@ -90,9 +90,11 @@ describe('restorePkgSymlink', () => { return true return false }) - // lstatSync throws — no file at all + // lstatSync throws ENOENT — no file at all vi.mocked(fs.lstatSync).mockImplementation(() => { - throw new Error('ENOENT') + const err = new Error('ENOENT') as NodeJS.ErrnoException + err.code = 'ENOENT' + throw err }) restorePkgSymlink('/project/.skills', 'vue', { version: '3.4.0' }, '/project')