mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-20 11:52:20 +02:00
fix(uninstall): provenance-gate the shape-2 and cursor sweeps; document the alias-name coupling
Three ways gstack-uninstall could touch a user's own skills: - Shape 2 (real dir + symlinked SKILL.md) matched the link target against a bare *gstack* substring, so a skill symlinked from ~/tools/gstack-fork/ was wiped on uninstall. The gate now requires "gstack" as an anchored path segment (gstack/*|*/gstack/*, same pattern as shape 1) AND the dir name in gstack's skill inventory (parity with shape 3); anything else is listed to stderr, never deleted. - The new Cursor removals (~/.cursor/skills/gstack* and repo-local .cursor/skills/gstack*) rm -rf'd any glob match with no provenance check, so a hand-written ~/.cursor/skills/gstack-fork-notes was swept. Real dirs now require the AUTO-GENERATED banner in SKILL.md; non-matching dirs are kept and listed. Legacy codex/factory/kiro globs are untouched (tracked in TODOS as a follow-up). - The _INVENTORY seed list hardcodes alias names created by setup's _install_alias_skill_md; both sites now carry mirrored keep-in-sync comments so a renamed alias can't silently strand its dir. The skipped-entry report moves to the end of the run so cursor skips are listed alongside the Claude ones. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
166ac2ceee
commit
4a95ce61a0
+52
-17
@@ -140,6 +140,9 @@ CLAUDE_SKILLS="$HOME/.claude/skills"
|
|||||||
# prefixed variants, and the alias dirs. Built BEFORE the install root is
|
# prefixed variants, and the alias dirs. Built BEFORE the install root is
|
||||||
# removed. A real directory in ~/.claude/skills is only deletable when its
|
# removed. A real directory in ~/.claude/skills is only deletable when its
|
||||||
# name is in this inventory AND its SKILL.md carries the generated banner.
|
# name is in this inventory AND its SKILL.md carries the generated banner.
|
||||||
|
# The seed names below are the alias dirs setup's _install_alias_skill_md
|
||||||
|
# creates (setup: link_claude_root_skill_alias + the connect-chrome call
|
||||||
|
# sites) — keep in sync with setup if an alias is added or renamed there.
|
||||||
_INVENTORY=" _gstack-command connect-chrome gstack-connect-chrome "
|
_INVENTORY=" _gstack-command connect-chrome gstack-connect-chrome "
|
||||||
if [ -d "$GSTACK_DIR" ]; then
|
if [ -d "$GSTACK_DIR" ]; then
|
||||||
for _SRC in "$GSTACK_DIR"/*/; do
|
for _SRC in "$GSTACK_DIR"/*/; do
|
||||||
@@ -172,13 +175,21 @@ if [ -d "$CLAUDE_SKILLS/gstack" ] || [ -L "$CLAUDE_SKILLS/gstack" ]; then
|
|||||||
esac
|
esac
|
||||||
elif [ -d "$_ENTRY" ] && { [ -f "$_ENTRY/SKILL.md" ] || [ -L "$_ENTRY/SKILL.md" ]; }; then
|
elif [ -d "$_ENTRY" ] && { [ -f "$_ENTRY/SKILL.md" ] || [ -L "$_ENTRY/SKILL.md" ]; }; then
|
||||||
if [ -L "$_ENTRY/SKILL.md" ]; then
|
if [ -L "$_ENTRY/SKILL.md" ]; then
|
||||||
# Shape 2: provenance readable from the symlink target itself
|
# Shape 2: provenance readable from the symlink target itself.
|
||||||
# (mirrors setup's cleanup_old_claude_symlinks semantics).
|
# Gate 1: the name must be in gstack's skill inventory (parity with
|
||||||
|
# shape 3). Gate 2: the target must contain "gstack" as an ANCHORED
|
||||||
|
# path segment (gstack/*|*/gstack/*, same pattern as shape 1) — a
|
||||||
|
# bare *gstack* substring match would wipe a user's own skill whose
|
||||||
|
# SKILL.md merely lives under e.g. ~/tools/gstack-fork/.
|
||||||
_TARGET="$(readlink "$_ENTRY/SKILL.md" 2>/dev/null || true)"
|
_TARGET="$(readlink "$_ENTRY/SKILL.md" 2>/dev/null || true)"
|
||||||
case "$_TARGET" in
|
if _in_skill_inventory "$_NAME"; then
|
||||||
*gstack*) rm -rf "$_ENTRY"; REMOVED+=("claude/$_NAME") ;;
|
case "$_TARGET" in
|
||||||
*) _SKIPPED_DIRS+=("$_ENTRY") ;;
|
gstack/*|*/gstack/*) rm -rf "$_ENTRY"; REMOVED+=("claude/$_NAME") ;;
|
||||||
esac
|
*) _SKIPPED_DIRS+=("$_ENTRY") ;;
|
||||||
|
esac
|
||||||
|
else
|
||||||
|
_SKIPPED_DIRS+=("$_ENTRY")
|
||||||
|
fi
|
||||||
elif _in_skill_inventory "$_NAME" && grep -q '<!-- AUTO-GENERATED from' "$_ENTRY/SKILL.md" 2>/dev/null; then
|
elif _in_skill_inventory "$_NAME" && grep -q '<!-- AUTO-GENERATED from' "$_ENTRY/SKILL.md" 2>/dev/null; then
|
||||||
# Shape 3: delete ONLY when BOTH gates pass (F8) — the name is in
|
# Shape 3: delete ONLY when BOTH gates pass (F8) — the name is in
|
||||||
# gstack's skill inventory AND the SKILL.md carries the existing
|
# gstack's skill inventory AND the SKILL.md carries the existing
|
||||||
@@ -199,13 +210,6 @@ if [ -d "$CLAUDE_SKILLS/gstack" ] || [ -L "$CLAUDE_SKILLS/gstack" ]; then
|
|||||||
REMOVED+=("~/.claude/skills/gstack")
|
REMOVED+=("~/.claude/skills/gstack")
|
||||||
fi
|
fi
|
||||||
|
|
||||||
if [ ${#_SKIPPED_DIRS[@]} -gt 0 ]; then
|
|
||||||
echo "left in place (not provably gstack-managed — remove by hand if they are yours):" >&2
|
|
||||||
for _D in "${_SKIPPED_DIRS[@]}"; do
|
|
||||||
echo " $_D" >&2
|
|
||||||
done
|
|
||||||
fi
|
|
||||||
|
|
||||||
# ─── Remove project-local Claude skills (--local installs) ──
|
# ─── Remove project-local Claude skills (--local installs) ──
|
||||||
if [ -n "$_GIT_ROOT" ] && [ -d "$_GIT_ROOT/.claude/skills" ]; then
|
if [ -n "$_GIT_ROOT" ] && [ -d "$_GIT_ROOT/.claude/skills" ]; then
|
||||||
for _LINK in "$_GIT_ROOT/.claude/skills"/*; do
|
for _LINK in "$_GIT_ROOT/.claude/skills"/*; do
|
||||||
@@ -252,12 +256,28 @@ if [ -d "$KIRO_SKILLS" ]; then
|
|||||||
fi
|
fi
|
||||||
|
|
||||||
# ─── Remove Cursor skills ───────────────────────────────────
|
# ─── Remove Cursor skills ───────────────────────────────────
|
||||||
|
# Cursor installs are rendered REAL directories, so a bare gstack* glob could
|
||||||
|
# sweep a user's own dir that merely starts with "gstack" (e.g.
|
||||||
|
# ~/.cursor/skills/gstack-fork-notes). Provenance gate: a real dir is only
|
||||||
|
# deleted when its SKILL.md carries the generated banner; anything else is
|
||||||
|
# listed, never deleted. Symlinks stay ungated — removing a link never
|
||||||
|
# destroys user content.
|
||||||
|
_cursor_item_is_gstack_managed() {
|
||||||
|
# Symlinks and plain files are safe to remove; real dirs need the banner.
|
||||||
|
if [ -L "$1" ] || [ ! -d "$1" ]; then return 0; fi
|
||||||
|
grep -q '<!-- AUTO-GENERATED from' "$1/SKILL.md" 2>/dev/null
|
||||||
|
}
|
||||||
|
|
||||||
CURSOR_SKILLS="$HOME/.cursor/skills"
|
CURSOR_SKILLS="$HOME/.cursor/skills"
|
||||||
if [ -d "$CURSOR_SKILLS" ]; then
|
if [ -d "$CURSOR_SKILLS" ]; then
|
||||||
for _ITEM in "$CURSOR_SKILLS"/gstack*; do
|
for _ITEM in "$CURSOR_SKILLS"/gstack*; do
|
||||||
[ -e "$_ITEM" ] || [ -L "$_ITEM" ] || continue
|
[ -e "$_ITEM" ] || [ -L "$_ITEM" ] || continue
|
||||||
rm -rf "$_ITEM"
|
if _cursor_item_is_gstack_managed "$_ITEM"; then
|
||||||
REMOVED+=("cursor/$(basename "$_ITEM")")
|
rm -rf "$_ITEM"
|
||||||
|
REMOVED+=("cursor/$(basename "$_ITEM")")
|
||||||
|
else
|
||||||
|
_SKIPPED_DIRS+=("$_ITEM")
|
||||||
|
fi
|
||||||
done
|
done
|
||||||
fi
|
fi
|
||||||
|
|
||||||
@@ -287,11 +307,16 @@ fi
|
|||||||
|
|
||||||
# ─── Remove per-project .cursor/skills/gstack* ──────────────
|
# ─── Remove per-project .cursor/skills/gstack* ──────────────
|
||||||
# Never rmdir .cursor itself — Cursor IDE stores rules and other user config there.
|
# Never rmdir .cursor itself — Cursor IDE stores rules and other user config there.
|
||||||
|
# Same provenance gate as the global cursor block above.
|
||||||
if [ -n "$_GIT_ROOT" ] && [ -d "$_GIT_ROOT/.cursor/skills" ]; then
|
if [ -n "$_GIT_ROOT" ] && [ -d "$_GIT_ROOT/.cursor/skills" ]; then
|
||||||
for _ITEM in "$_GIT_ROOT/.cursor/skills"/gstack*; do
|
for _ITEM in "$_GIT_ROOT/.cursor/skills"/gstack*; do
|
||||||
[ -e "$_ITEM" ] || [ -L "$_ITEM" ] || continue
|
[ -e "$_ITEM" ] || [ -L "$_ITEM" ] || continue
|
||||||
rm -rf "$_ITEM"
|
if _cursor_item_is_gstack_managed "$_ITEM"; then
|
||||||
REMOVED+=("cursor/$(basename "$_ITEM")")
|
rm -rf "$_ITEM"
|
||||||
|
REMOVED+=("cursor/$(basename "$_ITEM")")
|
||||||
|
else
|
||||||
|
_SKIPPED_DIRS+=("$_ITEM")
|
||||||
|
fi
|
||||||
done
|
done
|
||||||
|
|
||||||
rmdir "$_GIT_ROOT/.cursor/skills" 2>/dev/null || true
|
rmdir "$_GIT_ROOT/.cursor/skills" 2>/dev/null || true
|
||||||
@@ -338,6 +363,16 @@ for _TMP in /tmp/gstack-latest-version /tmp/gstack-sketch-*.html /tmp/gstack-ske
|
|||||||
fi
|
fi
|
||||||
done
|
done
|
||||||
|
|
||||||
|
# ─── Skipped-entry report ───────────────────────────────────
|
||||||
|
# Everything any provenance gate refused to delete (Claude shapes 2/3,
|
||||||
|
# cursor real dirs) — listed once, at the end, so nothing is silent.
|
||||||
|
if [ ${#_SKIPPED_DIRS[@]} -gt 0 ]; then
|
||||||
|
echo "left in place (not provably gstack-managed — remove by hand if they are yours):" >&2
|
||||||
|
for _D in "${_SKIPPED_DIRS[@]}"; do
|
||||||
|
echo " $_D" >&2
|
||||||
|
done
|
||||||
|
fi
|
||||||
|
|
||||||
# ─── Summary ────────────────────────────────────────────────
|
# ─── Summary ────────────────────────────────────────────────
|
||||||
if [ ${#REMOVED[@]} -gt 0 ]; then
|
if [ ${#REMOVED[@]} -gt 0 ]; then
|
||||||
echo "Removed: ${REMOVED[*]}"
|
echo "Removed: ${REMOVED[*]}"
|
||||||
|
|||||||
@@ -843,6 +843,10 @@ link_claude_skill_dirs() {
|
|||||||
# (#2511, #2201). Copy-then-rewrite instead: sed reads the SOURCE and writes a
|
# (#2511, #2201). Copy-then-rewrite instead: sed reads the SOURCE and writes a
|
||||||
# fresh copy with name: set to the alias. It must never edit through an
|
# fresh copy with name: set to the alias. It must never edit through an
|
||||||
# existing symlink — that would rewrite the generated source file itself.
|
# existing symlink — that would rewrite the generated source file itself.
|
||||||
|
# NOTE: every alias name passed to this helper (_gstack-command,
|
||||||
|
# connect-chrome, gstack-connect-chrome) is hardcoded in the _INVENTORY seed
|
||||||
|
# list in bin/gstack-uninstall — keep the two sites in sync when adding or
|
||||||
|
# renaming an alias, or uninstall will refuse to delete the new alias dir.
|
||||||
_install_alias_skill_md() {
|
_install_alias_skill_md() {
|
||||||
local src_skill_md="$1"
|
local src_skill_md="$1"
|
||||||
local dst_dir="$2"
|
local dst_dir="$2"
|
||||||
|
|||||||
@@ -148,9 +148,56 @@ describe.skipIf(process.platform === 'win32')(
|
|||||||
expect(fs.existsSync(dir)).toBe(false);
|
expect(fs.existsSync(dir)).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('SKILL.md symlink into a gstack-SUBSTRING path (gstack-fork) → kept and listed', () => {
|
||||||
|
// DM5: the shape-2 gate must match "gstack" as an anchored path
|
||||||
|
// segment, not a substring — a user's own skill whose SKILL.md links
|
||||||
|
// into ~/tools/gstack-fork/ is NOT ours, even when the dir name
|
||||||
|
// collides with a real gstack skill (here: review, in the inventory).
|
||||||
|
// The anchored gate only matches a literal /gstack/ path segment, so
|
||||||
|
// the tmpdir must not carry one (shared-process shard runs can leave
|
||||||
|
// $TMPDIR pointing into a gstack worktree — same hazard as the
|
||||||
|
// "pointing elsewhere" test below). Fall back to a fixed neutral root
|
||||||
|
// and ASSERT the precondition.
|
||||||
|
let neutralRoot = os.tmpdir();
|
||||||
|
if (neutralRoot.split(path.sep).includes('gstack')) neutralRoot = '/private' + path.sep + 'tmp';
|
||||||
|
const forkRoot = fs.mkdtempSync(path.join(neutralRoot, 'tools-'));
|
||||||
|
expect(forkRoot.split(path.sep).includes('gstack')).toBe(false);
|
||||||
|
const forkSrc = path.join(forkRoot, 'gstack-fork', 'review');
|
||||||
|
fs.mkdirSync(forkSrc, { recursive: true });
|
||||||
|
fs.writeFileSync(path.join(forkSrc, 'SKILL.md'), skillMd('review'));
|
||||||
|
const dir = path.join(skillsDir, 'review');
|
||||||
|
fs.mkdirSync(dir, { recursive: true });
|
||||||
|
fs.symlinkSync(path.join(forkSrc, 'SKILL.md'), path.join(dir, 'SKILL.md'));
|
||||||
|
|
||||||
|
try {
|
||||||
|
const r = runUninstall();
|
||||||
|
expect(r.status).toBe(0);
|
||||||
|
expect(fs.existsSync(dir)).toBe(true);
|
||||||
|
expect(r.stderr).toContain('left in place');
|
||||||
|
expect(r.stderr).toContain(path.join('skills', 'review'));
|
||||||
|
} finally {
|
||||||
|
fs.rmSync(forkRoot, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test('SKILL.md symlink into gstack but name NOT in inventory → kept and listed', () => {
|
||||||
|
// Shape 2 now carries the same inventory gate as shape 3: a dir whose
|
||||||
|
// name setup could never have created is skipped even when its
|
||||||
|
// SKILL.md target resolves into the install root.
|
||||||
|
const dir = path.join(skillsDir, 'my-custom-wrapper');
|
||||||
|
fs.mkdirSync(dir, { recursive: true });
|
||||||
|
fs.symlinkSync(path.join(installRoot, 'qa', 'SKILL.md'), path.join(dir, 'SKILL.md'));
|
||||||
|
|
||||||
|
const r = runUninstall();
|
||||||
|
expect(r.status).toBe(0);
|
||||||
|
expect(fs.existsSync(dir)).toBe(true);
|
||||||
|
expect(r.stderr).toContain('my-custom-wrapper');
|
||||||
|
});
|
||||||
|
|
||||||
test('SKILL.md symlink pointing elsewhere → kept and listed', () => {
|
test('SKILL.md symlink pointing elsewhere → kept and listed', () => {
|
||||||
// Target path must not contain "gstack" anywhere (the provenance match
|
// Target path must not contain a gstack path segment (the provenance
|
||||||
// is a substring check, mirroring setup's cleanup helpers) — the suite
|
// match is anchored: gstack/*|*/gstack/*; keeping the stricter
|
||||||
|
// no-substring precondition costs nothing) — the suite
|
||||||
// tmpdir prefix does, so use a separate neutral tmpdir. os.tmpdir()
|
// tmpdir prefix does, so use a separate neutral tmpdir. os.tmpdir()
|
||||||
// reads $TMPDIR at CALL time, and in shared-process shard runs a
|
// reads $TMPDIR at CALL time, and in shared-process shard runs a
|
||||||
// neighboring test can leave it pointing at a gstack-containing path —
|
// neighboring test can leave it pointing at a gstack-containing path —
|
||||||
|
|||||||
+38
-3
@@ -163,15 +163,18 @@ describe('gstack-uninstall', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
test('--force removes Cursor gstack skills and leaves other Cursor skills', () => {
|
test('--force removes Cursor gstack skills and leaves other Cursor skills', () => {
|
||||||
|
// Cursor installs are rendered real dirs, so removal is gated on the
|
||||||
|
// generated banner in SKILL.md (S5) — the managed fixtures carry it.
|
||||||
|
const banner = '<!-- AUTO-GENERATED from SKILL.md.tmpl - DO NOT EDIT DIRECTLY -->\n# x\n';
|
||||||
fs.mkdirSync(path.join(mockHome, '.cursor', 'skills', 'gstack'), { recursive: true });
|
fs.mkdirSync(path.join(mockHome, '.cursor', 'skills', 'gstack'), { recursive: true });
|
||||||
fs.writeFileSync(path.join(mockHome, '.cursor', 'skills', 'gstack', 'SKILL.md'), 'test');
|
fs.writeFileSync(path.join(mockHome, '.cursor', 'skills', 'gstack', 'SKILL.md'), banner);
|
||||||
fs.mkdirSync(path.join(mockHome, '.cursor', 'skills', 'gstack-review'), { recursive: true });
|
fs.mkdirSync(path.join(mockHome, '.cursor', 'skills', 'gstack-review'), { recursive: true });
|
||||||
fs.writeFileSync(path.join(mockHome, '.cursor', 'skills', 'gstack-review', 'SKILL.md'), 'test');
|
fs.writeFileSync(path.join(mockHome, '.cursor', 'skills', 'gstack-review', 'SKILL.md'), banner);
|
||||||
fs.mkdirSync(path.join(mockHome, '.cursor', 'skills', 'frontend-design'), { recursive: true });
|
fs.mkdirSync(path.join(mockHome, '.cursor', 'skills', 'frontend-design'), { recursive: true });
|
||||||
fs.writeFileSync(path.join(mockHome, '.cursor', 'skills', 'frontend-design', 'SKILL.md'), 'keep');
|
fs.writeFileSync(path.join(mockHome, '.cursor', 'skills', 'frontend-design', 'SKILL.md'), 'keep');
|
||||||
|
|
||||||
fs.mkdirSync(path.join(mockGitRoot, '.cursor', 'skills', 'gstack-ship'), { recursive: true });
|
fs.mkdirSync(path.join(mockGitRoot, '.cursor', 'skills', 'gstack-ship'), { recursive: true });
|
||||||
fs.writeFileSync(path.join(mockGitRoot, '.cursor', 'skills', 'gstack-ship', 'SKILL.md'), 'test');
|
fs.writeFileSync(path.join(mockGitRoot, '.cursor', 'skills', 'gstack-ship', 'SKILL.md'), banner);
|
||||||
fs.mkdirSync(path.join(mockGitRoot, '.cursor', 'rules'), { recursive: true });
|
fs.mkdirSync(path.join(mockGitRoot, '.cursor', 'rules'), { recursive: true });
|
||||||
fs.writeFileSync(path.join(mockGitRoot, '.cursor', 'rules', 'keep.md'), 'keep');
|
fs.writeFileSync(path.join(mockGitRoot, '.cursor', 'rules', 'keep.md'), 'keep');
|
||||||
|
|
||||||
@@ -194,5 +197,37 @@ describe('gstack-uninstall', () => {
|
|||||||
expect(fs.existsSync(path.join(mockGitRoot, '.cursor', 'skills', 'gstack-ship'))).toBe(false);
|
expect(fs.existsSync(path.join(mockGitRoot, '.cursor', 'skills', 'gstack-ship'))).toBe(false);
|
||||||
expect(fs.existsSync(path.join(mockGitRoot, '.cursor', 'rules', 'keep.md'))).toBe(true);
|
expect(fs.existsSync(path.join(mockGitRoot, '.cursor', 'rules', 'keep.md'))).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("a user's own gstack-prefixed Cursor dir (no banner) survives and is listed", () => {
|
||||||
|
// S5: the bare gstack* glob must not sweep a dir that merely starts
|
||||||
|
// with "gstack" — provenance comes from the generated banner, and a
|
||||||
|
// hand-written SKILL.md never carries it.
|
||||||
|
const foreign = path.join(mockHome, '.cursor', 'skills', 'gstack-fork-notes');
|
||||||
|
fs.mkdirSync(foreign, { recursive: true });
|
||||||
|
fs.writeFileSync(path.join(foreign, 'SKILL.md'), '# my own notes\n');
|
||||||
|
|
||||||
|
const foreignLocal = path.join(mockGitRoot, '.cursor', 'skills', 'gstack-my-rules');
|
||||||
|
fs.mkdirSync(foreignLocal, { recursive: true });
|
||||||
|
fs.writeFileSync(path.join(foreignLocal, 'SKILL.md'), '# hand-written\n');
|
||||||
|
|
||||||
|
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);
|
||||||
|
expect(fs.existsSync(foreign)).toBe(true);
|
||||||
|
expect(fs.existsSync(foreignLocal)).toBe(true);
|
||||||
|
const stderr = result.stderr.toString();
|
||||||
|
expect(stderr).toContain('left in place');
|
||||||
|
expect(stderr).toContain('gstack-fork-notes');
|
||||||
|
expect(stderr).toContain('gstack-my-rules');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user