From 2293128fff5f7a7c386304e0973d129e3eebb24c Mon Sep 17 00:00:00 2001
From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Date: Wed, 22 Apr 2026 00:25:22 -0700
Subject: [PATCH] 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
---
 bin/gstack-uninstall   | 18 +++++++++++++++++-
 test/uninstall.test.ts | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/bin/gstack-uninstall b/bin/gstack-uninstall
index 4f7b0fc1e..438ad0a48 100755
--- a/bin/gstack-uninstall
+++ b/bin/gstack-uninstall
@@ -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
diff --git a/test/uninstall.test.ts b/test/uninstall.test.ts
index a7208e877..32d1ac059 100644
--- a/test/uninstall.test.ts
+++ b/test/uninstall.test.ts
@@ -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);
+    });
   });
 });