fix: handle dangling symlinks in restorePkgSymlink

[oritwoen]

Description

restorePkgSymlink crashes with EEXIST when the .skilld/pkg symlink exists but points to a deleted target (dangling). existsSync follows symlinks and returns false for dangling ones, so the guard passes through but symlinkSync sees the path already exists on disk.

This happens during skilld prepare after cleaning node_modules and reinstalling - the old symlink is still on disk, target is gone, existsSync says "nothing here", symlinkSync disagrees.

Fixed by using lstatSync (doesn't follow symlinks) to detect and remove dangling symlinks before re-creating. Same pattern linkPkg in cache/storage.ts already uses.

Linked Issues

Additional context

5 unit tests added covering: dangling symlink, valid symlink skip, no prior link, missing skill dir, real file at path.

Summary by CodeRabbit

• Bug Fixes

  • Improved package symlink restoration to detect and remove invalid/dangling symbolic links before recreating them, preventing stale links and ensuring consistent package linking across environments.

• Tests

  • Added comprehensive unit tests covering missing directories, valid links, dangling links, non-existent paths, and cases where a regular file exists to ensure robust behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: df467d77-cf3b-41d8-9492-a4590b5e81fd

📥 Commits

Reviewing files that changed from the base of the PR and between 640a5d9 and fc54e96.

📒 Files selected for processing (2)

• src/core/prepare.ts
• test/unit/prepare-restore.test.ts

✅ Files skipped from review due to trivial changes (1)

• test/unit/prepare-restore.test.ts

---

📝 Walkthrough

Walkthrough

restorePkgSymlink now uses lstatSync to detect symlinks and distinguishes dangling symlinks from valid ones; dangling symlinks are unlinked before recreation, valid symlinks and non-symlink entries are left untouched. A unit test suite exercising five scenarios was added.

Changes

Cohort / File(s)	Summary
Core logic src/core/prepare.ts	Replaced an early existsSync check with lstatSync-based logic to detect symlinks. If the path is a dangling symlink it calls unlinkSync before symlinkSync; if it's a valid symlink or a real file/dir, it returns without recreating. Errors from lstatSync are caught and only ENOENT allows continuation.
Tests test/unit/prepare-restore.test.ts	Added new Vitest unit tests mocking node:fs and getCacheDir, covering: missing skill dir early return; preservation of valid symlinks; removal+recreation of dangling symlinks; creation when no prior entry; and skipping when a real file exists.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• harlan-zw

Poem

🐰 I sniff the links both old and new,

I tug the loose ones, leave the true,
A little hop, an unlink done,
Then weave a thread to link the one,
Five tests hum—our path is run.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: handle dangling symlinks in restorePkgSymlink' directly and accurately describes the main change: fixing how dangling symlinks are handled in the restorePkgSymlink function, which is the core issue addressed in the PR.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/restore-pkg-dangling-symlink

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/core/prepare.ts`:
- Around line 39-52: The catch block in restorePkgSymlink is currently
swallowing all errors from lstatSync/unlinkSync and proceeding to create a
symlink; change the empty catch to catch the error (e.g., catch (err)) and only
treat ENOENT as “path missing” (allowing execution to continue to symlinkSync);
for any other err.code (permissions, EIO, etc.) rethrow the error so the real
failure is not masked. Ensure you reference the same functions (lstatSync,
unlinkSync, symlinkSync) when updating the error handling.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7a282428-a483-419c-9d7d-b51b82f0e82b

📥 Commits

Reviewing files that changed from the base of the PR and between 3c242ee and 640a5d9.

📒 Files selected for processing (2)

• src/core/prepare.ts
• test/unit/prepare-restore.test.ts