fix(relink): don't clobber foreign skills that share a gstack name#2119
Open
smblight wants to merge 1 commit into
Open
fix(relink): don't clobber foreign skills that share a gstack name#2119smblight wants to merge 1 commit into
smblight wants to merge 1 commit into
Conversation
gstack-relink's _cleanup_skill_entry deleted ANY flat-named skill entry matching a gstack skill name when switching prefix modes — including symlinks that point outside the gstack install. A user with a personal /qa or /review skill (e.g. symlinked into ~/.agents/skills/) would have it silently removed on every ./setup, because setup runs gstack-relink as a self-healing step. In prefix mode the deletion isn't even needed: gstack installs under gstack-* names, so the flat entry it removed was pure collateral. Scope the cleanup to gstack-owned entries only: delete a flat entry just when its symlink (or its dir's SKILL.md symlink) resolves into the gstack install dir. Foreign skills sharing a gstack name are now preserved, while genuinely stale flat-mode gstack entries are still cleaned. This mirrors the provenance check setup's cleanup_old_claude_symlinks() already uses. Adds two regression tests to test/relink.test.ts: one asserting a foreign flat skill (both the dir-symlink and real-dir-with-foreign-SKILL.md shapes) survives a prefix-mode relink, and a companion asserting a gstack-owned stale flat entry is still cleaned. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Installing gstack with
--prefix(or any later./setuprun) silently deletes a user's own flat-named skills when they share a name with a gstack skill. Concretely: a personal/qaor/reviewskill symlinked into~/.agents/skills/gets removed.It's not a one-time hit —
setupcallsgstack-relinkas a self-healing step on every run, so each upgrade re-deletes them.Root cause
bin/gstack-relink→_cleanup_skill_entry()removed any flat-named entry matching a gstack skill name, with no check on where the symlink points:In prefix mode the deletion isn't even necessary — gstack installs under
gstack-*names — so the removed flat entry was pure collateral.Fix
Scope the cleanup to gstack-owned entries only: remove a flat entry just when its symlink (or its dir's
SKILL.mdsymlink) resolves into the gstack install dir. This mirrors the provenance checksetup'scleanup_old_claude_symlinks()already uses, and also handles installs not literally namedgstack/via an$INSTALL_DIR-prefix match.Result: foreign skills sharing a gstack name are preserved; genuinely stale flat-mode gstack entries are still cleaned.
Tests
Two regression tests added to
test/relink.test.ts:prefix mode preserves foreign flat skills sharing a gstack name— covers both the directory-symlink shape and the real-dir-with-foreign-SKILL.md-symlink shape.prefix mode still removes gstack-owned stale flat entries— guards against over-correcting.Verified the first test fails on
main(foreign/qadeleted →ENOENT) and passes with the fix. Full suite:26 pass, 0 fail.