fix(pi): preserve user-managed symlinks and unmanaged paths on install

[anishekkamal]

Fixes #1048

Summary

Installing to Pi no longer destroys local skill overrides. Previously, compound-plugin install <plugin> --to pi replaced whatever sat at each target skill/agent path with a fresh upstream copy — including user-managed symlinks pointing at a personal fork or worktree, and manually authored directories. A user following the standard symlink-override pattern (the basis of downstream patch projects like agents-skills) had their patches silently deactivated on every install, with no warning and nothing in the output to indicate it happened.

Current behavior

From #1048, after creating the standard override symlink and reinstalling:

ln -sfn ~/agents-skills/skills/ce-session-inventory ~/.pi/agent/skills/ce-session-inventory
bunx @every-env/compound-plugin install compound-engineering --to pi

ls -la ~/.pi/agent/skills/ce-session-inventory
# drwxr-xr-x ... SKILL.md          <- real dir, NOT a symlink; local patch gone

The skill/agent cleanup helpers in src/targets/pi.ts run fs.rm(..., { recursive: true, force: true }) and then copy, with no check for what the existing path is.

Fix

The cleanup helpers now lstat the target before acting, implementing the contract from the issue:

Existing path state	Behavior
Doesn't exist	Create fresh (unchanged)
Real path recorded in the install manifest	Replace (unchanged)
Real path not in the manifest (manually authored)	Preserve, warn, skip the copy
Symlink to anything, including dangling	Preserve, warn, skip the copy

Three decisions worth reviewer attention:

• Preservation skips both the rm and the subsequent copy/write. copySkillDir merges without pre-clearing, so writing through a preserved symlink would push upstream content into the user's fork — worse than the original bug.
• Preserved names are excluded from the freshly written install manifest, so the tool never claims ownership of a path it didn't write. The bookkeeping is self-healing: if the user later removes the symlink, the next install writes fresh content and resumes tracking it.
• cleanupRemovedSkills / cleanupRemovedAgents apply the same symlink guard, covering the case where a skill is dropped from the bundle while a stale manifest entry still claims a path the user has since converted to a symlink.

The guard covers the skillDirs, generatedSkills, and agents loops alike.

Verification

• 9 new tests in tests/pi-writer.test.ts: symlinked skill dir / agent file preserved with fork content untouched and manifest exclusion; unmanaged real dir/file preserved; manifest-owned dir and agent file still replaced on reinstall; preserved symlink survives a later run where the skill/agent is dropped from the bundle; dangling symlink preserved (pins the lstat-not-stat behavior).
• Red then green: with src/targets/pi.ts restored to main, 7 of the new tests fail; with the fix, all 19 tests in the file pass.
• Symlink capability is probed at module load — directory junctions and file symlinks separately, since Windows grants them different privileges — and symlink-dependent tests skip where creation fails. On this Windows machine both succeeded, so all 19 ran.
• Full bun test baseline diff against main on this machine: zero new failures (the pre-existing failure set here is environment-related and identical on both).

Scope notes

• Pre-manifest installs freeze rather than update. Pi skills were written before the install manifest existed (manifest landed in refactor(install): prefer native plugin install across targets #609). An install from that era with no manifest now matches the "unmanaged" branch and is preserved with a warning instead of overwritten. "Not in the manifest" can't distinguish user-authored from pre-manifest tool-written, and erring toward preservation is the point of this fix; one reinstall after deleting the frozen dirs restores tracking.
• Same pattern in two other writers. src/targets/codex.ts (cleanupCurrentManagedSkillDir) and src/targets/managed-artifacts.ts (cleanupCurrentManagedDirectory, used by the OpenCode writer) share the unconditional rm-then-copy shape and would clobber symlinks the same way. Left untouched to keep this PR reviewable against the Pi repro in Installing to pi clobbers user-managed symlinks at skills/ and agents/ paths #1048; happy to file a follow-up or extend this fix if preferred.

---

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8b1afd4f9d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[tmchow]

Addressed the Codex P1 (legacy sweep followed symlinks).

Root cause: cleanupKnownLegacyPiArtifacts renamed CE-owned legacy skill/agent paths into legacy-backup even when the user had swapped the path for a symlink into a fork. Those names are dropped from the bundle, so they never reach the new preservation loop — the ce-session-inventory repro from #1048 was still broken despite the warning claiming the symlink was preserved.

Fix: guard the single destructive choke point, moveLegacyArtifactToBackup, so every legacy sweep (skills, agents, prompts, extensions, mcporter) refuses to rename through a user symlink. Checked before pathExists (which follows the link) so dangling links are preserved too:

if (await isPreservedSymlink(artifactPath)) return
if (!(await pathExists(artifactPath))) return

Guarding the choke point rather than the two call-site loops reuses the existing isPreservedSymlink helper and covers prompts/extensions/mcporter for free.

Test: added a regression mirroring the exact repro — a symlinked ce-session-inventory that only reaches the legacy sweep. It asserts isLegacySkillArtifactOwned(...) === true first as a false-green guard, so it fails loudly if the ownership fingerprint ever drifts instead of silently passing without exercising the guard.

Verified red-without-guard / green-with-guard; bun test full suite 1897 pass / 0 fail; tsc --noEmit clean.

The codex.ts / managed-artifacts.ts writers share the same unguarded shape and are still out of scope here — worth a follow-up issue.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e9a4f854c0

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".