fix(uninstall): remove per-skill directories with nested SKILL.md symlinks

The uninstall script only removed entries that were themselves top-level
symlinks under ~/.claude/skills. But setup (lines 399-404) creates real
directories there, with each SKILL.md linked to gstack/<skill>/SKILL.md.
After uninstall, ~30 per-skill directories (autoplan, canary, ship, ...)
were left behind holding broken SKILL.md symlinks.

Adds a second pass that inspects the nested SKILL.md in each top-level
directory and removes the directory when that link points into gstack/.
The legacy top-level-symlink pass is preserved for older installs.

Scoped to ~/.claude/skills; the codex/factory/kiro paths use a different
install topology and are untouched.

Fixes #1132
This commit is contained in:
Matt Van Horn 2026-04-22 00:25:22 -07:00
parent 54d4cde773
commit 2293128fff
No known key found for this signature in database
2 changed files with 59 additions and 1 deletions

View File

@ -131,7 +131,7 @@ fi
# ─── Remove global Claude skills ────────────────────────────
CLAUDE_SKILLS="$HOME/.claude/skills"
if [ -d "$CLAUDE_SKILLS/gstack" ] || [ -L "$CLAUDE_SKILLS/gstack" ]; then
# Remove per-skill symlinks that point into gstack/
# Remove legacy per-skill symlinks that point into gstack/
for _LINK in "$CLAUDE_SKILLS"/*; do
[ -L "$_LINK" ] || continue
_NAME="$(basename "$_LINK")"
@ -142,6 +142,22 @@ if [ -d "$CLAUDE_SKILLS/gstack" ] || [ -L "$CLAUDE_SKILLS/gstack" ]; then
esac
done
# Remove per-skill directories whose nested SKILL.md symlinks into gstack/.
# setup creates real directories here (not top-level symlinks), with each
# SKILL.md linked to gstack/<skill>/SKILL.md. The legacy loop above never
# matches these, so they would otherwise be left behind once gstack/ is
# removed. See #1132.
while IFS= read -r _DIR; do
_NAME="$(basename "$_DIR")"
[ "$_NAME" = "gstack" ] && continue
[ "$_NAME" = "node_modules" ] && continue
[ -L "$_DIR/SKILL.md" ] || continue
_TARGET="$(readlink "$_DIR/SKILL.md" 2>/dev/null || true)"
case "$_TARGET" in
gstack/*|*/gstack/*) rm -rf "$_DIR"; REMOVED+=("claude/$_NAME") ;;
esac
done < <(find "$CLAUDE_SKILLS" -mindepth 1 -maxdepth 1 -type d 2>/dev/null || true)
rm -rf "$CLAUDE_SKILLS/gstack"
REMOVED+=("~/.claude/skills/gstack")
fi

View File

@ -161,5 +161,47 @@ describe('gstack-uninstall', () => {
// Non-gstack should survive
expect(fs.existsSync(path.join(mockHome, '.claude', 'skills', 'other-tool'))).toBe(true);
});
test('current setup layout: real per-skill dirs with nested SKILL.md symlinks are removed (#1132)', () => {
// The current setup script (see setup:399-404) creates real directories at the
// top level of ~/.claude/skills/ with SKILL.md as a nested symlink into gstack/.
// The legacy loop only matched top-level symlinks, so these survived uninstall.
fs.mkdirSync(path.join(mockHome, '.claude', 'skills', 'gstack', 'autoplan'), { recursive: true });
fs.mkdirSync(path.join(mockHome, '.claude', 'skills', 'gstack', 'canary'), { recursive: true });
fs.writeFileSync(path.join(mockHome, '.claude', 'skills', 'gstack', 'autoplan', 'SKILL.md'), 'real');
fs.writeFileSync(path.join(mockHome, '.claude', 'skills', 'gstack', 'canary', 'SKILL.md'), 'real');
// Per-skill directories with nested SKILL.md symlinks
fs.mkdirSync(path.join(mockHome, '.claude', 'skills', 'autoplan'), { recursive: true });
fs.symlinkSync(
path.join(mockHome, '.claude', 'skills', 'gstack', 'autoplan', 'SKILL.md'),
path.join(mockHome, '.claude', 'skills', 'autoplan', 'SKILL.md')
);
fs.mkdirSync(path.join(mockHome, '.claude', 'skills', 'canary'), { recursive: true });
fs.symlinkSync(
path.join(mockHome, '.claude', 'skills', 'gstack', 'canary', 'SKILL.md'),
path.join(mockHome, '.claude', 'skills', 'canary', 'SKILL.md')
);
const result = spawnSync('bash', [UNINSTALL, '--force'], {
stdio: 'pipe',
env: {
...process.env,
HOME: mockHome,
GSTACK_DIR: path.join(mockHome, '.claude', 'skills', 'gstack'),
GSTACK_STATE_DIR: path.join(mockHome, '.gstack'),
},
cwd: mockGitRoot,
});
expect(result.status).toBe(0);
// Per-skill directories with nested SKILL.md symlinks into gstack/ should be gone
expect(fs.existsSync(path.join(mockHome, '.claude', 'skills', 'autoplan'))).toBe(false);
expect(fs.existsSync(path.join(mockHome, '.claude', 'skills', 'canary'))).toBe(false);
// Non-gstack tool (no SKILL.md symlink) should survive
expect(fs.existsSync(path.join(mockHome, '.claude', 'skills', 'other-tool'))).toBe(true);
});
});
});