diff --git a/bin/gstack-relink b/bin/gstack-relink index dd2a681fa..748456d7f 100755 --- a/bin/gstack-relink +++ b/bin/gstack-relink @@ -37,12 +37,24 @@ SKILLS_DIR="${GSTACK_SKILLS_DIR:-$(dirname "$INSTALL_DIR")}" PREFIX=$("$GSTACK_CONFIG" get skill_prefix 2>/dev/null || echo "false") # Helper: remove old skill entry (symlink or real directory with symlinked SKILL.md) +# ONLY removes gstack-owned entries — i.e. ones whose symlink (or whose dir's +# SKILL.md symlink) resolves into the gstack install dir. A flat entry of the +# same name that points elsewhere (e.g. a personal /qa or /review symlink into +# ~/.agents/skills/) is left untouched, so switching prefix modes never clobbers +# unrelated user skills that happen to share a gstack skill name. _cleanup_skill_entry() { local entry="$1" + local dest if [ -L "$entry" ]; then - rm -f "$entry" + dest="$(readlink "$entry" 2>/dev/null || true)" + case "$dest" in + "$INSTALL_DIR"/*|gstack/*|*/gstack/*) rm -f "$entry" ;; + esac elif [ -d "$entry" ] && [ -L "$entry/SKILL.md" ]; then - rm -rf "$entry" + dest="$(readlink "$entry/SKILL.md" 2>/dev/null || true)" + case "$dest" in + "$INSTALL_DIR"/*|gstack/*|*/gstack/*) rm -rf "$entry" ;; + esac fi } diff --git a/test/relink.test.ts b/test/relink.test.ts index d83c4cd37..96e241b66 100644 --- a/test/relink.test.ts +++ b/test/relink.test.ts @@ -367,6 +367,72 @@ describe('gstack-relink (#578)', () => { expect(fs.existsSync(path.join(skillsDir, 'gstack-qa'))).toBe(false); }); + // REGRESSION: prefix-mode relink must NOT clobber a *foreign* flat skill that + // happens to share a gstack skill name. Only gstack-owned flat entries (whose + // symlink resolves into the install dir) may be cleaned — a personal `/qa` or + // `/review` symlink pointing into e.g. ~/.agents/skills/ must survive. + test('prefix mode preserves foreign flat skills sharing a gstack name', () => { + setupMockInstall(['qa', 'ship']); + + // A user's personal skills living OUTSIDE the gstack install. + const foreignDir = path.join(tmpDir, 'foreign-skills'); + fs.mkdirSync(path.join(foreignDir, 'qa'), { recursive: true }); + fs.writeFileSync( + path.join(foreignDir, 'qa', 'SKILL.md'), + '---\nname: qa\ndescription: my own qa skill\n---\n# my qa', + ); + + // Case A: foreign entry is a directory symlink (e.g. `qa -> ~/.agents/skills/qa`). + fs.symlinkSync(path.join(foreignDir, 'qa'), path.join(skillsDir, 'qa')); + // Case B: foreign entry is a real dir whose SKILL.md symlinks OUTSIDE the install. + fs.mkdirSync(path.join(skillsDir, 'ship'), { recursive: true }); + fs.symlinkSync( + path.join(foreignDir, 'qa', 'SKILL.md'), + path.join(skillsDir, 'ship', 'SKILL.md'), + ); + + run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`, { + GSTACK_INSTALL_DIR: installDir, + GSTACK_SKILLS_DIR: skillsDir, + }); + run(`${path.join(installDir, 'bin', 'gstack-relink')}`, { + GSTACK_INSTALL_DIR: installDir, + GSTACK_SKILLS_DIR: skillsDir, + }); + + // Foreign /qa symlink survives, still pointing outside the install. + expect(fs.lstatSync(path.join(skillsDir, 'qa')).isSymbolicLink()).toBe(true); + expect(fs.readlinkSync(path.join(skillsDir, 'qa'))).toBe(path.join(foreignDir, 'qa')); + // Foreign /ship (real dir, foreign SKILL.md) survives too. + expect(fs.readlinkSync(path.join(skillsDir, 'ship', 'SKILL.md'))) + .toBe(path.join(foreignDir, 'qa', 'SKILL.md')); + // gstack still installs under its prefixed names alongside them. + expect(fs.existsSync(path.join(skillsDir, 'gstack-qa'))).toBe(true); + expect(fs.existsSync(path.join(skillsDir, 'gstack-ship'))).toBe(true); + }); + + // REGRESSION companion: a genuinely gstack-owned stale flat entry IS still removed. + test('prefix mode still removes gstack-owned stale flat entries', () => { + setupMockInstall(['qa']); + // Leftover flat-mode gstack entry: real dir whose SKILL.md → into the install. + fs.mkdirSync(path.join(skillsDir, 'qa'), { recursive: true }); + fs.symlinkSync( + path.join(installDir, 'qa', 'SKILL.md'), + path.join(skillsDir, 'qa', 'SKILL.md'), + ); + run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`, { + GSTACK_INSTALL_DIR: installDir, + GSTACK_SKILLS_DIR: skillsDir, + }); + run(`${path.join(installDir, 'bin', 'gstack-relink')}`, { + GSTACK_INSTALL_DIR: installDir, + GSTACK_SKILLS_DIR: skillsDir, + }); + // The stale flat /qa (gstack-owned) is gone; only gstack-qa remains. + expect(fs.existsSync(path.join(skillsDir, 'gstack-qa'))).toBe(true); + expect(fs.readdirSync(skillsDir)).not.toContain('qa'); + }); + // Test 14: error when install dir missing test('prints error when install dir missing', () => { const output = run(`${BIN}/gstack-relink`, {