From a8bc93eb2c7e0bc5c9bc29ed259a6590f862ec6d Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 4 Sep 2026 18:14:31 +0000 Subject: [PATCH] =?UTF-8?q?fix(setup,relink):=20weak=20proof=20never=20cos?= =?UTF-8?q?ts=20the=20user=20a=20file=20=E2=80=94=20assets,=20flips,=20fai?= =?UTF-8?q?led=20backups,=20foreign=20dir=20links,=20alias=20markers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third review cycle on the ownership model, every item reproduced against a fixture before the fix: - Runtime assets (sections/, templates/, checklist.md, ...) were refreshed with rm -rf regardless of who owned the directory, so an unclaimed or weakly-owned directory lost the user's same-named real files. Real assets are now replaced only in a directory gstack created or strongly owns (marker, or SKILL.md symlink into gstack), plus the legacy Windows real-copy shape; elsewhere they are kept and reported. Symlinks are never content and are always refreshed. - The prefix-flip cleanup deleted a customized banner-bearing SKILL.md that the link pass would have backed up. Both cleanups now compare the file against the source (raw, or with its name: line rewritten to the entry name, which is how alias and prefixed copies legitimately differ) and move a differing file to the backup root. - A failed backup (unwritable root) returned success and the caller linked over the file anyway. It now fails, and the entry is left untouched and reported. - A foreign DIRECTORY symlink whose target had no SKILL.md fell through to the "unclaimed directory" rule and was replaced by a real directory. A symlink that does not resolve into gstack is foreign, full stop. - The alias installers stamped .gstack-owned into pre-existing directories; they now follow the same created-or-already-marked rule. - A directory counts as "only links" only when every link resolves into gstack: a user's own symlink makes it mixed, so their link survives. - The gstack-tree heuristic requires bin/gstack-relink, not just a VERSION file, a setup script and a bin/ directory. Co-Authored-By: Claude Fable 5.1 --- bin/gstack-relink | 54 +++++++--- setup | 102 ++++++++++++++----- test/relink.test.ts | 88 +++++++++++++++- test/setup-cleanup-orphans.test.ts | 4 +- test/setup-link-ownership.test.ts | 158 +++++++++++++++++++++++++++-- 5 files changed, 360 insertions(+), 46 deletions(-) diff --git a/bin/gstack-relink b/bin/gstack-relink index ad8cb481e..ecc609cae 100755 --- a/bin/gstack-relink +++ b/bin/gstack-relink @@ -92,9 +92,10 @@ _target_is_ours() { esac # A checkout named without a `gstack` segment (git worktree add # ../gstack-): the target's skill root is a gstack tree if it - # carries setup + VERSION + bin/. Same rule as setup's _gstack_target_is_ours. + # carries setup + VERSION + bin/gstack-relink (a hand-written skill repo with + # a VERSION file does not). Same rule as setup's _gstack_target_is_ours. root="${1%/*/SKILL.md}" - if [ "$root" != "$1" ] && [ -f "$root/VERSION" ] && [ -f "$root/setup" ] && [ -d "$root/bin" ]; then return 0; fi + if [ "$root" != "$1" ] && [ -f "$root/VERSION" ] && [ -f "$root/setup" ] && [ -f "$root/bin/gstack-relink" ]; then return 0; fi return 1 } @@ -146,6 +147,9 @@ _entry_owned_strongly() { _entry_is_ours() { local entry="$1" skill="${2:-}" src _entry_owned_strongly "$entry" && return 0 + # A symlink that did not resolve into gstack is someone else's; never follow + # it into the "unclaimed directory" rule below. + [ -L "$entry" ] && return 1 if [ -d "$entry" ] && [ ! -L "$entry/SKILL.md" ]; then # No SKILL.md at all: an UNCLAIMED directory (a weak cleanup left the # user's other files behind, or the dir was never a skill). Adding our @@ -182,16 +186,18 @@ _report_foreign() { BACKUP_ROOT="${GSTACK_HOME:-$HOME/.gstack}/backups/skills/$(date +%Y%m%dT%H%M%S)" BACKED_UP=() _backup_skill_md() { + # Non-zero when the file could NOT be moved: the caller leaves the entry alone. local file="$1" name="$2" - mkdir -p "$BACKUP_ROOT/$name" 2>/dev/null || return 0 - if mv -f "$file" "$BACKUP_ROOT/$name/SKILL.md" 2>/dev/null; then BACKED_UP+=("$name"); fi + mkdir -p "$BACKUP_ROOT/$name" 2>/dev/null || return 1 + mv -f "$file" "$BACKUP_ROOT/$name/SKILL.md" 2>/dev/null || return 1 + BACKED_UP+=("$name") return 0 } # Helper: remove an OLD skill entry from the opposite prefix mode. Only entries # we can prove are ours are removed; anything else is reported and kept. _cleanup_skill_entry() { - local entry="$1" skill="${2:-}" e dest + local entry="$1" skill="${2:-}" e dest src [ -e "$entry" ] || [ -L "$entry" ] || return 0 # Unclaimed dir (no SKILL.md, no marker): nothing of ours to clean. if [ -d "$entry" ] && [ ! -L "$entry" ] && [ ! -e "$entry/SKILL.md" ] && [ ! -L "$entry/SKILL.md" ] && [ ! -f "$entry/.gstack-owned" ]; then @@ -211,7 +217,19 @@ _cleanup_skill_entry() { else # Otherwise only what is ours goes: the SKILL.md, the marker, and our # runtime-asset links. The user's files stay, and so does the directory - # if it is not empty afterwards. + # if it is not empty afterwards. A real SKILL.md that differs from our + # source (raw, or with its name: rewritten to the entry name) is a + # customized file: moved to the backup root, never deleted. + if [ -f "$entry/SKILL.md" ] && [ ! -L "$entry/SKILL.md" ] && [ -n "$skill" ]; then + src="$INSTALL_DIR/$skill/SKILL.md"; [ -f "$RENDER_DIR/$skill/SKILL.md" ] && src="$RENDER_DIR/$skill/SKILL.md" + if [ -f "$src" ] && ! cmp -s "$entry/SKILL.md" "$src" \ + && ! sed "1,/^---\$/ s/^name:[[:space:]].*/name: ${entry##*/}/" "$src" | cmp -s - "$entry/SKILL.md"; then + if ! _backup_skill_md "$entry/SKILL.md" "${entry##*/}"; then + echo " kept ${entry##*/}/SKILL.md: could not back up the customized file — left untouched" >&2 + return 0 + fi + fi + fi rm -f "$entry/SKILL.md" "$entry/.gstack-owned" for e in "$entry"/* "$entry"/.[!.]* "$entry"/..?*; do [ -L "$e" ] || continue @@ -225,12 +243,15 @@ _cleanup_skill_entry() { # _dir_only_links DIR — deleting DIR whole loses no real data: every entry is # a symlink or our marker. _dir_only_links() { - local d="$1" e + # Every entry must be a symlink resolving into gstack, or our marker: a + # user's own link (notes.md -> ~/notes) makes the directory mixed. + local d="$1" e dest for e in "$d"/* "$d"/.[!.]* "$d"/..?*; do { [ -e "$e" ] || [ -L "$e" ]; } || continue - [ -L "$e" ] && continue [ "${e##*/}" = ".gstack-owned" ] && continue - return 1 + [ -L "$e" ] || return 1 + dest="$(_link_target_abs "$e")" || return 1 + _target_is_ours "$dest" || return 1 done return 0 } @@ -245,6 +266,8 @@ _link_root_skill_alias() { _report_foreign "$target" return 0 fi + local pre=0 + if [ -e "$target" ] || [ -L "$target" ]; then pre=1; fi [ -L "$target" ] && rm -f "$target" mkdir -p "$target" # Copy-then-rewrite, never a symlink (#2511): a symlinked alias re-serves @@ -255,8 +278,11 @@ _link_root_skill_alias() { rm -f "$target/SKILL.md" sed "1,/^---\$/ s/^name:[[:space:]].*/name: _gstack-command/" "$INSTALL_DIR/SKILL.md" > "$target/SKILL.md" # The rewritten copy is a real file on every platform: the marker, not the - # banner, is what proves it ours on the next run. - printf '%s\n' "$_INSTALL_REAL" > "$target/.gstack-owned" 2>/dev/null || true + # banner, is what proves it ours on the next run — written only for a + # directory we created (or already marked), never one we merely wrote into. + if [ "$pre" -eq 0 ] || [ -f "$target/.gstack-owned" ]; then + printf '%s\n' "$_INSTALL_REAL" > "$target/.gstack-owned" 2>/dev/null || true + fi } _link_root_skill_alias @@ -316,7 +342,11 @@ for skill_dir in "$INSTALL_DIR"/*/; do # differs from what we are about to serve is moved aside, not overwritten. if [ -f "$target/SKILL.md" ] && [ ! -L "$target/SKILL.md" ] && ! _entry_owned_strongly "$target" \ && ! cmp -s "$target/SKILL.md" "$skill_md_src"; then - _backup_skill_md "$target/SKILL.md" "$link_name" + if ! _backup_skill_md "$target/SKILL.md" "$link_name"; then + echo " skipped $link_name: could not back up its customized SKILL.md — left untouched" >&2 + FOREIGN_SKIPPED+=("$link_name") + continue + fi fi ln -snf "$skill_md_src" "$target/SKILL.md" # Provenance marker on every platform (path-independent proof; on Windows diff --git a/setup b/setup index d08e9164f..4d8e0448b 100755 --- a/setup +++ b/setup @@ -155,9 +155,10 @@ _gstack_target_is_ours() { esac # A checkout named without a `gstack` segment (git worktree add # ../gstack-, a ZIP unpacked as gstack-main): the target's skill - # root is a gstack tree if it carries setup + VERSION + bin/. + # root is a gstack tree if it carries setup + VERSION + bin/gstack-relink + # (a hand-written skill repo with a VERSION file and a setup script does not). local root="${t%/*/SKILL.md}" - if [ "$root" != "$t" ] && [ -f "$root/VERSION" ] && [ -f "$root/setup" ] && [ -d "$root/bin" ]; then return 0; fi + if [ "$root" != "$t" ] && [ -f "$root/VERSION" ] && [ -f "$root/setup" ] && [ -f "$root/bin/gstack-relink" ]; then return 0; fi return 1 } _claude_entry_is_ours() { @@ -165,6 +166,9 @@ _claude_entry_is_ours() { # would be linked to, $3 = gstack payload dir local entry="$1" src_md="$2" g="$3" render_md _claude_entry_owned_strongly "$entry" "$g" && return 0 + # A symlink that did not resolve into gstack is someone else's; never follow + # it into the "unclaimed directory" rule below. + [ -L "$entry" ] && return 1 # No SKILL.md at all: an UNCLAIMED directory (a weak cleanup left the user's # other files behind, or it was never a skill). Adding our SKILL.md # overwrites nothing, so installing into it is allowed; the cleanup arms @@ -204,19 +208,35 @@ _claude_entry_owned_strongly() { _SKILL_BACKUP_ROOT="${GSTACK_HOME:-$HOME/.gstack}/backups/skills/$(date +%Y%m%dT%H%M%S)" _BACKED_UP_SKILL_MDS=() _backup_skill_md() { + # Returns non-zero when the file could NOT be moved: the caller must then + # leave the entry untouched (a failed backup is never a license to overwrite). local file="$1" name="$2" - mkdir -p "$_SKILL_BACKUP_ROOT/$name" 2>/dev/null || return 0 - if mv -f "$file" "$_SKILL_BACKUP_ROOT/$name/SKILL.md" 2>/dev/null; then _BACKED_UP_SKILL_MDS+=("$name"); fi + mkdir -p "$_SKILL_BACKUP_ROOT/$name" 2>/dev/null || return 1 + mv -f "$file" "$_SKILL_BACKUP_ROOT/$name/SKILL.md" 2>/dev/null || return 1 + _BACKED_UP_SKILL_MDS+=("$name") return 0 } # _cleanup_weak_dir DIR — remove only what weak proof covers: the SKILL.md and # our marker. User files in the directory stay, and so does the directory # when it is not empty afterwards. +# _cleanup_weak_dir DIR GSTACK_DIR [SRC_SKILL_MD NAME] — remove only what weak +# proof covers. A real SKILL.md that differs from our source (raw, or with its +# name: line rewritten to NAME, which is how alias and prefixed copies differ) +# is a customized file: it is moved to the backup root, never deleted, and if +# the backup fails it stays. Our runtime-asset links go; the user's files stay. _cleanup_weak_dir() { - local d="$1" g="$2" e dest - rm -f "$d/SKILL.md" "$d/.gstack-owned" - # Our runtime-asset links (sections/, templates, checklist.md, ...) go too; - # anything the user put there stays. + local d="$1" g="$2" src="${3:-}" name="${4:-${1##*/}}" e dest + if [ -f "$d/SKILL.md" ] && [ ! -L "$d/SKILL.md" ] && [ -n "$src" ] && [ -f "$src" ] \ + && ! cmp -s "$d/SKILL.md" "$src" \ + && ! sed "1,/^---\$/ s/^name:[[:space:]].*/name: $name/" "$src" | cmp -s - "$d/SKILL.md"; then + if ! _backup_skill_md "$d/SKILL.md" "$name"; then + echo " kept $name/SKILL.md: could not back up the customized file — left untouched" >&2 + return 0 + fi + else + rm -f "$d/SKILL.md" + fi + rm -f "$d/.gstack-owned" for e in "$d"/* "$d"/.[!.]* "$d"/..?*; do [ -L "$e" ] || continue dest="$(_gstack_link_target_abs "$e")" || continue @@ -224,23 +244,25 @@ _cleanup_weak_dir() { done rmdir "$d" 2>/dev/null || echo " cleaned ${d##*/}/SKILL.md (other files in that directory were left in place)" } -# _gstack_dir_only_links DIR — true when deleting DIR whole loses no real data: -# every entry is a symlink (ours or not — a link is not content) or our marker. +# _gstack_dir_only_links DIR GSTACK_DIR — true when deleting DIR whole loses +# nothing of the user's: every entry is a symlink resolving into gstack, or +# our marker. A user's own link (notes.md -> ~/notes) makes the dir mixed. _gstack_dir_only_links() { - local d="$1" e + local d="$1" g="$2" e dest for e in "$d"/* "$d"/.[!.]* "$d"/..?*; do { [ -e "$e" ] || [ -L "$e" ]; } || continue - [ -L "$e" ] && continue [ "${e##*/}" = ".gstack-owned" ] && continue - return 1 + [ -L "$e" ] || return 1 + dest="$(_gstack_link_target_abs "$e")" || return 1 + _gstack_target_is_ours "$dest" "$g" || return 1 done return 0 } # _cleanup_linked_dir DIR GSTACK_DIR — a real dir whose SKILL.md is a symlink # into gstack. Whole-directory removal needs the marker (we created it) or a -# directory holding nothing but links; otherwise only our files go. +# directory holding nothing but our links; otherwise only our files go. _cleanup_linked_dir() { - if [ -f "$1/.gstack-owned" ] || _gstack_dir_only_links "$1"; then rm -rf "$1"; else _cleanup_weak_dir "$1" "$2"; fi + if [ -f "$1/.gstack-owned" ] || _gstack_dir_only_links "$1" "$2"; then rm -rf "$1"; else _cleanup_weak_dir "$1" "$2"; fi } # _gstack_generated_header FILE — a pre-marker legacy COPY (Windows, before # .gstack-owned existed) is recognized by gen-skill-docs' full two-line banner @@ -1099,6 +1121,12 @@ mkdir -p "$HOME/.gstack/projects" _link_skill_runtime_assets() { local src_dir="$1" local dst_dir="$2" + # $3 = 0 when the destination directory is not provably ours (pre-existed + # unclaimed, or only weakly proven where gstack never wrote real assets): its + # real files are the user's, so a same-named real asset is kept and reported + # instead of replaced (#2119). Symlinks are never content and are always + # refreshed. Default 1 = the directory is ours. + local replace_real="${3:-1}" local asset asset_name for asset in "$src_dir"/*; do [ -e "$asset" ] || continue # empty-glob guard @@ -1106,8 +1134,12 @@ _link_skill_runtime_assets() { case "$asset_name" in SKILL.md|node_modules|dist|test|*.tmpl) continue ;; esac - # Refresh unconditionally: rm the old entry (symlink OR real copy — the - # Windows install pattern) so re-runs after `git pull` pick up changes. + if [ -e "$dst_dir/$asset_name" ] && [ ! -L "$dst_dir/$asset_name" ] && [ "$replace_real" != "1" ]; then + echo " kept ${dst_dir##*/}/$asset_name: a file you own already uses that name — left untouched" >&2 + continue + fi + # Refresh: rm the old entry (symlink OR real copy — the Windows install + # pattern) so re-runs after `git pull` pick up changes. if [ -e "$dst_dir/$asset_name" ] || [ -L "$dst_dir/$asset_name" ]; then rm -rf "$dst_dir/$asset_name" fi @@ -1185,8 +1217,21 @@ link_claude_skill_dirs() { # provenance marker below make it deletable whole. A directory we merely # link into (unclaimed, or a legacy install) never gets one — legacy # all-links dirs are removed by the only-links rule instead. - _pre_exists=0 - if [ -e "$target" ] || [ -L "$target" ]; then _pre_exists=1; fi + # _assets_replace: may _link_skill_runtime_assets replace a REAL file or + # dir already present under the target? Yes for a directory we create or + # strongly own (marker, or SKILL.md symlink into gstack). On Windows also + # for a weakly-proven real-file copy install (the legacy pre-marker shape, + # whose asset copies are ours). Otherwise (unclaimed, or a weak copy on a + # platform where gstack never wrote real assets) real files are the + # user's and are kept. + _pre_exists=0; _assets_replace=1 + if [ -e "$target" ] || [ -L "$target" ]; then + _pre_exists=1 + if _claude_entry_owned_strongly "$target" "$gstack_dir"; then _assets_replace=1 + elif [ "$IS_WINDOWS" -eq 1 ] && [ -f "$target/SKILL.md" ] && [ ! -L "$target/SKILL.md" ]; then _assets_replace=1 + else _assets_replace=0 + fi + fi # Upgrade old directory symlinks to real directories if [ -L "$target" ]; then rm -f "$target" @@ -1211,7 +1256,11 @@ link_claude_skill_dirs() { # differs from what we are about to serve is moved aside, not overwritten. if [ -f "$target/SKILL.md" ] && [ ! -L "$target/SKILL.md" ] && ! _claude_entry_owned_strongly "$target" "$gstack_dir" \ && ! cmp -s "$target/SKILL.md" "$_skill_md_src"; then - _backup_skill_md "$target/SKILL.md" "$link_name" + if ! _backup_skill_md "$target/SKILL.md" "$link_name"; then + echo " skipped $link_name: could not back up its customized SKILL.md — left untouched" >&2 + _FOREIGN_SKIPPED_ENTRIES+=("$link_name") + continue + fi fi _link_or_copy "$_skill_md_src" "$target/SKILL.md" # Provenance marker (#2119) on every platform — path-independent proof @@ -1227,7 +1276,7 @@ link_claude_skill_dirs() { # landed and /review 404'd at "Read .claude/skills/review/checklist.md" # on every fresh Claude install. Routes through _link_or_copy so Windows # gets real copies refreshed on every ./setup. - _link_skill_runtime_assets "$gstack_dir/$dir_name" "$target" + _link_skill_runtime_assets "$gstack_dir/$dir_name" "$target" "$_assets_replace" linked+=("$link_name") fi done @@ -1262,6 +1311,8 @@ _install_alias_skill_md() { return 0 fi # Old installs left the alias as a whole-dir symlink — replace it. + _alias_pre=0 + if [ -e "$dst_dir" ] || [ -L "$dst_dir" ]; then _alias_pre=1; fi if [ -L "$dst_dir" ]; then rm -f "$dst_dir"; fi mkdir -p "$dst_dir" # Remove any prior symlinked SKILL.md so the redirect below cannot write @@ -1269,8 +1320,9 @@ _install_alias_skill_md() { rm -f "$dst_dir/SKILL.md" sed "1,/^---\$/ s/^name:[[:space:]].*/name: $alias_name/" "$src_skill_md" > "$dst_dir/SKILL.md" # A rewritten copy is a real file on every platform; the marker proves it - # ours on the next run without leaning on the banner. - _write_owned_marker "$dst_dir" "$SOURCE_GSTACK_DIR" + # ours on the next run without leaning on the banner — but only for a + # directory we created (or already marked), never one we merely wrote into. + if [ "$_alias_pre" -eq 0 ] || [ -f "$dst_dir/.gstack-owned" ]; then _write_owned_marker "$dst_dir" "$SOURCE_GSTACK_DIR"; fi } # Claude Code skips the repo-shaped ~/.claude/skills/gstack directory when @@ -1354,7 +1406,7 @@ cleanup_old_claude_symlinks() { || _gstack_generated_header "$old_target/SKILL.md"; }; then # Only the marker proves we created the directory; weak proof covers # the SKILL.md alone (a user's files next to it survive). - if [ -f "$old_target/.gstack-owned" ]; then rm -rf "$old_target"; else _cleanup_weak_dir "$old_target" "$gstack_dir"; fi + if [ -f "$old_target/.gstack-owned" ]; then rm -rf "$old_target"; else _cleanup_weak_dir "$old_target" "$gstack_dir" "$skill_dir/SKILL.md" "$skill_name"; fi removed+=("$skill_name") fi fi @@ -1408,7 +1460,7 @@ cleanup_prefixed_claude_symlinks() { && { [ -f "$prefixed_target/.gstack-owned" ] \ || cmp -s "$prefixed_target/SKILL.md" "$skill_dir/SKILL.md" \ || _gstack_generated_header "$prefixed_target/SKILL.md"; }; then - if [ -f "$prefixed_target/.gstack-owned" ]; then rm -rf "$prefixed_target"; else _cleanup_weak_dir "$prefixed_target" "$gstack_dir"; fi + if [ -f "$prefixed_target/.gstack-owned" ]; then rm -rf "$prefixed_target"; else _cleanup_weak_dir "$prefixed_target" "$gstack_dir" "$skill_dir/SKILL.md" "gstack-$skill_name"; fi removed+=("gstack-$skill_name") fi fi diff --git a/test/relink.test.ts b/test/relink.test.ts index 9a1f1070b..65424f9f1 100644 --- a/test/relink.test.ts +++ b/test/relink.test.ts @@ -1096,6 +1096,7 @@ describe('gstack-relink: checkout naming, legacy linked dirs, markers (#2119 rev fs.mkdirSync(path.join(other, 'bin')); fs.writeFileSync(path.join(other, 'VERSION'), '1.0.0.0\n'); fs.writeFileSync(path.join(other, 'setup'), '#!/bin/bash\n'); + fs.writeFileSync(path.join(other, 'bin', 'gstack-relink'), '#!/bin/bash\n'); fs.writeFileSync(path.join(other, 'qa', 'SKILL.md'), '---\nname: qa\n---\n'); fs.mkdirSync(path.join(skillsDir, 'qa')); fs.symlinkSync(path.join(other, 'qa', 'SKILL.md'), path.join(skillsDir, 'qa', 'SKILL.md')); @@ -1103,9 +1104,12 @@ describe('gstack-relink: checkout naming, legacy linked dirs, markers (#2119 rev const out = relink(); expect(out).not.toContain('skipped'); expect(fs.readlinkSync(path.join(skillsDir, 'qa', 'SKILL.md'))).toBe(path.join(installDir, 'qa', 'SKILL.md')); - // ...but a plain directory that merely holds a SKILL.md is not a gstack tree. + // ...but a hand-written skill repo with VERSION + setup + bin/ (no gstack-relink) is not a gstack tree. const plain = path.join(tmpDir, 'plain-tools'); fs.mkdirSync(path.join(plain, 'ship'), { recursive: true }); + fs.mkdirSync(path.join(plain, 'bin')); + fs.writeFileSync(path.join(plain, 'VERSION'), '0.1\n'); + fs.writeFileSync(path.join(plain, 'setup'), '#!/bin/bash\n'); fs.writeFileSync(path.join(plain, 'ship', 'SKILL.md'), '---\nname: ship\n---\n'); fs.mkdirSync(path.join(installDir, 'ship')); fs.writeFileSync(path.join(installDir, 'ship', 'SKILL.md'), '---\nname: ship\n---\n'); @@ -1146,3 +1150,85 @@ describe('gstack-relink: checkout naming, legacy linked dirs, markers (#2119 rev expect(out).not.toContain('skipped'); }); }); + +describe('gstack-relink cycle-3 hardening: foreign dir links, failed backups, flip backups, mixed dirs (#2119 review)', () => { + const BANNER = '\n'; + const env = (extra: Record = {}) => ({ + GSTACK_INSTALL_DIR: installDir, GSTACK_SKILLS_DIR: skillsDir, + GSTACK_HOME: path.join(tmpDir, 'home'), GSTACK_USER_RENDER_DIR: path.join(tmpDir, 'no-render'), ...extra, + }); + const relink = (extra: Record = {}) => run(`${path.join(installDir, 'bin', 'gstack-relink')} 2>&1`, env(extra)); + const setPrefix = (v: 'true' | 'false') => run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix ${v}`, env()); + const backups = (home = path.join(tmpDir, 'home')) => { + const root = path.join(home, 'backups', 'skills'); + if (!fs.existsSync(root)) return [] as string[]; + return fs.readdirSync(root).flatMap((ts) => fs.readdirSync(path.join(root, ts)).map((n) => path.join(root, ts, n, 'SKILL.md'))); + }; + + test('a foreign DIRECTORY symlink whose target has no SKILL.md is foreign, not unclaimed', () => { + setupMockInstall(['qa']); + const userdir = path.join(tmpDir, 'userdir'); + fs.mkdirSync(userdir); + fs.writeFileSync(path.join(userdir, 'notes.md'), 'mine\n'); + fs.symlinkSync(userdir, path.join(skillsDir, 'qa')); + setPrefix('false'); + const out = relink(); + expect(out).toContain('skipped qa'); + expect(fs.readlinkSync(path.join(skillsDir, 'qa'))).toBe(userdir); + expect(fs.existsSync(path.join(userdir, 'SKILL.md'))).toBe(false); + }); + + test('flip cleanup moves a CUSTOMIZED banner copy to the backup root instead of deleting it', () => { + setupMockInstall(['qa']); + const custom = `---\nname: gstack-qa\n---\n${BANNER}\n# customized\n`; + fs.mkdirSync(path.join(skillsDir, 'gstack-qa')); + fs.writeFileSync(path.join(skillsDir, 'gstack-qa', 'SKILL.md'), custom); + setPrefix('false'); + relink(); + expect(fs.existsSync(path.join(skillsDir, 'gstack-qa'))).toBe(false); + const saved = backups(); + expect(saved.length).toBe(1); + expect(fs.readFileSync(saved[0], 'utf-8')).toBe(custom); + }); + + test('when the backup root cannot be created, the customized file stays and the entry is reported', () => { + setupMockInstall(['qa']); + const custom = `---\nname: qa\n---\n${BANNER}\n# customized\n`; + fs.mkdirSync(path.join(skillsDir, 'qa')); + fs.writeFileSync(path.join(skillsDir, 'qa', 'SKILL.md'), custom); + fs.mkdirSync(path.join(tmpDir, 'home')); + fs.writeFileSync(path.join(tmpDir, 'home', 'backups'), 'not a dir'); + setPrefix('false'); + const out = relink(); + expect(out).toContain('could not back up'); + expect(fs.lstatSync(path.join(skillsDir, 'qa', 'SKILL.md')).isSymbolicLink()).toBe(false); + expect(fs.readFileSync(path.join(skillsDir, 'qa', 'SKILL.md'), 'utf-8')).toBe(custom); + }); + + test('a legacy linked dir holding the user\'s OWN symlink is mixed: our links go, theirs stays', () => { + setupMockInstall(['qa']); + fs.mkdirSync(path.join(installDir, 'qa', 'sections')); + fs.mkdirSync(path.join(skillsDir, 'gstack-qa')); + fs.symlinkSync(path.join(installDir, 'qa', 'SKILL.md'), path.join(skillsDir, 'gstack-qa', 'SKILL.md')); + fs.symlinkSync(path.join(installDir, 'qa', 'sections'), path.join(skillsDir, 'gstack-qa', 'sections')); + fs.writeFileSync(path.join(tmpDir, 'my-notes.md'), 'mine\n'); + fs.symlinkSync(path.join(tmpDir, 'my-notes.md'), path.join(skillsDir, 'gstack-qa', 'notes.md')); + setPrefix('false'); + relink(); + expect(fs.existsSync(path.join(skillsDir, 'gstack-qa', 'SKILL.md'))).toBe(false); + expect(fs.existsSync(path.join(skillsDir, 'gstack-qa', 'sections'))).toBe(false); + expect(fs.readlinkSync(path.join(skillsDir, 'gstack-qa', 'notes.md'))).toBe(path.join(tmpDir, 'my-notes.md')); + }); + + test('the root alias marker is written only for a directory relink creates', () => { + setupMockInstall(['qa']); + fs.writeFileSync(path.join(installDir, 'SKILL.md'), `---\nname: gstack\n---\n${BANNER}\n# root\n`); + fs.mkdirSync(path.join(skillsDir, '_gstack-command')); + fs.writeFileSync(path.join(skillsDir, '_gstack-command', 'notes.md'), 'mine\n'); + setPrefix('false'); + relink(); + expect(fs.readFileSync(path.join(skillsDir, '_gstack-command', 'SKILL.md'), 'utf-8')).toContain('name: _gstack-command'); + expect(fs.existsSync(path.join(skillsDir, '_gstack-command', '.gstack-owned'))).toBe(false); + expect(fs.readFileSync(path.join(skillsDir, '_gstack-command', 'notes.md'), 'utf-8')).toBe('mine\n'); + }); +}); diff --git a/test/setup-cleanup-orphans.test.ts b/test/setup-cleanup-orphans.test.ts index 50ac75412..1874caba2 100644 --- a/test/setup-cleanup-orphans.test.ts +++ b/test/setup-cleanup-orphans.test.ts @@ -24,7 +24,7 @@ function extractFn(name: string): string { } function cleanupBody(): string { - return [extractFn('_gstack_link_target_abs'), extractFn('_gstack_target_is_ours'), extractFn('_gstack_dir_only_links'), extractFn('_cleanup_linked_dir'), extractFn('_gstack_generated_header'), extractFn('_cleanup_weak_dir'), extractFn('cleanup_old_claude_symlinks')].join('\n'); + return [extractFn('_gstack_link_target_abs'), extractFn('_gstack_target_is_ours'), extractFn('_gstack_dir_only_links'), extractFn('_cleanup_linked_dir'), extractFn('_gstack_generated_header'), extractFn('_cleanup_weak_dir'), extractFn('_backup_skill_md'), '_BACKED_UP_SKILL_MDS=()', `_SKILL_BACKUP_ROOT="${os.tmpdir()}/cleanup-orphans-backups-${process.pid}"`, extractFn('cleanup_old_claude_symlinks')].join('\n'); } describe('setup: cleanup_old_claude_symlinks — static (#2204)', () => { @@ -68,7 +68,7 @@ describe.skipIf(process.platform === 'win32')('setup: cleanup_old_claude_symlink const script = [ 'set -e', `IS_WINDOWS=${opts.isWindows ?? '0'}`, - extractFn('_gstack_link_target_abs'), extractFn('_gstack_target_is_ours'), extractFn('_gstack_dir_only_links'), extractFn('_cleanup_linked_dir'), extractFn('_gstack_generated_header'), extractFn('_cleanup_weak_dir'), + extractFn('_gstack_link_target_abs'), extractFn('_gstack_target_is_ours'), extractFn('_gstack_dir_only_links'), extractFn('_cleanup_linked_dir'), extractFn('_gstack_generated_header'), extractFn('_cleanup_weak_dir'), extractFn('_backup_skill_md'), '_BACKED_UP_SKILL_MDS=()', `_SKILL_BACKUP_ROOT="${tmp}/backups"`, extractFn('cleanup_old_claude_symlinks'), `cleanup_old_claude_symlinks "${gstackArg}" "${skills}"`, ].join('\n'); diff --git a/test/setup-link-ownership.test.ts b/test/setup-link-ownership.test.ts index 110cf29d8..01b57310b 100644 --- a/test/setup-link-ownership.test.ts +++ b/test/setup-link-ownership.test.ts @@ -150,9 +150,13 @@ describe.skipIf(process.platform === 'win32')('setup: _install_alias_skill_md ne expect(r.stdout).toContain('FOREIGN=connect-chrome'); expect(fs.readFileSync(path.join(t.skills, 'gstack-connect-chrome', 'SKILL.md'), 'utf-8')).toContain('name: gstack-connect-chrome'); expect(fs.readFileSync(path.join(t.skills, 'gstack-connect-chrome', 'SKILL.md'), 'utf-8')).not.toContain('old alias copy'); - // The refreshed copy is stamped, so next run's provenance is the marker, not the banner. - expect(fs.readFileSync(path.join(t.skills, 'gstack-connect-chrome', '.gstack-owned'), 'utf-8').trim()).toBe(fs.realpathSync(t.payload)); + // A pre-existing alias directory is never stamped (we did not create it); a created one is. + expect(fs.existsSync(path.join(t.skills, 'gstack-connect-chrome', '.gstack-owned'))).toBe(false); expect(fs.existsSync(path.join(t.skills, 'connect-chrome', '.gstack-owned'))).toBe(false); + const created = bash(['set -e', 'IS_WINDOWS=0', `SOURCE_GSTACK_DIR="${t.payload}"`, HELPERS, extractFn('_install_alias_skill_md'), + `_install_alias_skill_md "${t.payload}/open-gstack-browser/SKILL.md" "${t.skills}/fresh-alias" fresh-alias`], t.tmp); + expect(created.status).toBe(0); + expect(fs.readFileSync(path.join(t.skills, 'fresh-alias', '.gstack-owned'), 'utf-8').trim()).toBe(fs.realpathSync(t.payload)); } finally { fs.rmSync(t.tmp, { recursive: true, force: true }); } @@ -165,7 +169,7 @@ describe.skipIf(process.platform === 'win32')('setup: cleanup_prefixed_claude_sy fs.mkdirSync(path.join(t.payload, 'qa')); fs.writeFileSync(path.join(t.payload, 'qa', 'SKILL.md'), GENERATED('qa')); plant(t.skills, t.payload); - const r = bash(['set -e', `IS_WINDOWS=${isWindows}`, extractFn('_gstack_link_target_abs'), extractFn('_gstack_target_is_ours'), extractFn('_gstack_dir_only_links'), extractFn('_cleanup_linked_dir'), extractFn('_gstack_generated_header'), extractFn('_cleanup_weak_dir'), extractFn('cleanup_prefixed_claude_symlinks'), + const r = bash(['set -e', `IS_WINDOWS=${isWindows}`, extractFn('_gstack_link_target_abs'), extractFn('_gstack_target_is_ours'), extractFn('_gstack_dir_only_links'), extractFn('_cleanup_linked_dir'), extractFn('_gstack_generated_header'), extractFn('_cleanup_weak_dir'), extractFn('_backup_skill_md'), '_BACKED_UP_SKILL_MDS=()', '_SKILL_BACKUP_ROOT="$HOME/.gstack/backups/skills/test"', extractFn('cleanup_prefixed_claude_symlinks'), `cleanup_prefixed_claude_symlinks "${t.payload}" "${t.skills}"`], t.tmp); const names = fs.readdirSync(t.skills).sort(); fs.rmSync(t.tmp, { recursive: true, force: true }); @@ -281,7 +285,7 @@ describe.skipIf(process.platform === 'win32')('setup: weakly-proven files are mo fs.mkdirSync(path.join(t.skills, 'gstack-qa', 'my-templates'), { recursive: true }); fs.writeFileSync(path.join(t.skills, 'gstack-qa', 'SKILL.md'), GENERATED('gstack-qa')); fs.writeFileSync(path.join(t.skills, 'gstack-qa', 'my-templates', 'checklist.md'), '- mine\n'); - const r = bash(['set -e', 'IS_WINDOWS=1', extractFn('_gstack_link_target_abs'), extractFn('_gstack_target_is_ours'), extractFn('_gstack_dir_only_links'), extractFn('_cleanup_linked_dir'), extractFn('_gstack_generated_header'), extractFn('_cleanup_weak_dir'), extractFn('cleanup_prefixed_claude_symlinks'), + const r = bash(['set -e', 'IS_WINDOWS=1', extractFn('_gstack_link_target_abs'), extractFn('_gstack_target_is_ours'), extractFn('_gstack_dir_only_links'), extractFn('_cleanup_linked_dir'), extractFn('_gstack_generated_header'), extractFn('_cleanup_weak_dir'), extractFn('_backup_skill_md'), '_BACKED_UP_SKILL_MDS=()', '_SKILL_BACKUP_ROOT="$HOME/.gstack/backups/skills/test"', extractFn('cleanup_prefixed_claude_symlinks'), `cleanup_prefixed_claude_symlinks "${t.payload}" "${t.skills}"`], t.tmp); expect(r.status).toBe(0); expect(fs.existsSync(path.join(t.skills, 'gstack-qa', 'SKILL.md'))).toBe(false); @@ -340,13 +344,26 @@ describe.skipIf(process.platform === 'win32')('setup: banner census, checkout na fs.mkdirSync(path.join(other, 'bin')); fs.writeFileSync(path.join(other, 'VERSION'), '1.0.0.0\n'); fs.writeFileSync(path.join(other, 'setup'), '#!/bin/bash\n'); + fs.writeFileSync(path.join(other, 'bin', 'gstack-relink'), '#!/bin/bash\n'); fs.writeFileSync(path.join(other, 'qa', 'SKILL.md'), GENERATED('qa')); fs.mkdirSync(path.join(t.skills, 'qa')); fs.symlinkSync(path.join(other, 'qa', 'SKILL.md'), path.join(t.skills, 'qa', 'SKILL.md')); + // A hand-written skill repo that happens to carry VERSION + setup + bin/ is NOT a gstack tree. + const mine = path.join(t.tmp, 'myskills'); + fs.mkdirSync(path.join(mine, 'ship'), { recursive: true }); + fs.mkdirSync(path.join(mine, 'bin')); + fs.writeFileSync(path.join(mine, 'VERSION'), '0.1\n'); + fs.writeFileSync(path.join(mine, 'setup'), '#!/bin/bash\n'); + fs.writeFileSync(path.join(mine, 'ship', 'SKILL.md'), FOREIGN); + fs.mkdirSync(path.join(t.payload, 'ship')); + fs.writeFileSync(path.join(t.payload, 'ship', 'SKILL.md'), GENERATED('ship')); + fs.mkdirSync(path.join(t.skills, 'ship')); + fs.symlinkSync(path.join(mine, 'ship', 'SKILL.md'), path.join(t.skills, 'ship', 'SKILL.md')); const r = bash(['set -e', 'IS_WINDOWS=0', 'SKILL_PREFIX=0', HELPERS, extractFn('link_claude_skill_dirs'), `link_claude_skill_dirs "${t.payload}" "${t.skills}"`, 'echo "FOREIGN=${_FOREIGN_SKIPPED_ENTRIES[*]:-}"'], t.tmp); expect(r.status).toBe(0); - expect(r.stdout).toContain('FOREIGN=\n'); + expect(r.stdout).toContain('FOREIGN=ship\n'); + expect(fs.readlinkSync(path.join(t.skills, 'ship', 'SKILL.md'))).toBe(path.join(mine, 'ship', 'SKILL.md')); expect(fs.readlinkSync(path.join(t.skills, 'qa', 'SKILL.md'))).toBe(path.join(t.payload, 'qa', 'SKILL.md')); // Re-pointed, but the directory pre-existed: no marker (only directories we create get one). expect(fs.existsSync(path.join(t.skills, 'qa', '.gstack-owned'))).toBe(false); @@ -387,7 +404,7 @@ describe.skipIf(process.platform === 'win32')('setup: banner census, checkout na fs.symlinkSync(path.join(t.payload, 'qa', 'sections'), path.join(t.skills, name, 'sections')); } fs.writeFileSync(path.join(t.skills, 'gstack-ship', 'my-notes.md'), 'keep\n'); - const r = bash(['set -e', 'IS_WINDOWS=0', extractFn('_gstack_link_target_abs'), extractFn('_gstack_target_is_ours'), extractFn('_gstack_dir_only_links'), extractFn('_cleanup_linked_dir'), extractFn('_gstack_generated_header'), extractFn('_cleanup_weak_dir'), extractFn('cleanup_prefixed_claude_symlinks'), + const r = bash(['set -e', 'IS_WINDOWS=0', extractFn('_gstack_link_target_abs'), extractFn('_gstack_target_is_ours'), extractFn('_gstack_dir_only_links'), extractFn('_cleanup_linked_dir'), extractFn('_gstack_generated_header'), extractFn('_cleanup_weak_dir'), extractFn('_backup_skill_md'), '_BACKED_UP_SKILL_MDS=()', '_SKILL_BACKUP_ROOT="$HOME/.gstack/backups/skills/test"', extractFn('cleanup_prefixed_claude_symlinks'), `cleanup_prefixed_claude_symlinks "${t.payload}" "${t.skills}"`], t.tmp); expect(r.status).toBe(0); expect(fs.existsSync(path.join(t.skills, 'gstack-qa'))).toBe(false); @@ -400,3 +417,132 @@ describe.skipIf(process.platform === 'win32')('setup: banner census, checkout na } }); }); + +describe.skipIf(process.platform === 'win32')('setup: cycle-3 hardening — assets, foreign dir links, failed backups, flip backups (#2119 review)', () => { + const REAL_ASSETS = extractFn('_link_skill_runtime_assets'); + + test('an unclaimed or weakly-owned directory keeps the user\'s same-named real assets; a created directory gets ours', () => { + const t = mkTree(); + try { + for (const n of ['qa', 'ship', 'review']) { + fs.mkdirSync(path.join(t.payload, n, 'templates'), { recursive: true }); + fs.writeFileSync(path.join(t.payload, n, 'SKILL.md'), GENERATED(n)); + fs.writeFileSync(path.join(t.payload, n, 'templates', 'ours.md'), 'ours\n'); + fs.writeFileSync(path.join(t.payload, n, 'checklist.md'), 'ours\n'); + } + // qa: unclaimed (no SKILL.md) with the user's templates/ and checklist.md + fs.mkdirSync(path.join(t.skills, 'qa', 'templates'), { recursive: true }); + fs.writeFileSync(path.join(t.skills, 'qa', 'templates', 'mine.md'), 'mine\n'); + fs.writeFileSync(path.join(t.skills, 'qa', 'checklist.md'), 'my checklist\n'); + // ship: weakly owned (customized banner copy) with the user's templates/ + fs.mkdirSync(path.join(t.skills, 'ship', 'templates'), { recursive: true }); + fs.writeFileSync(path.join(t.skills, 'ship', 'SKILL.md'), GENERATED('ship').replace('# ship', '# customized')); + fs.writeFileSync(path.join(t.skills, 'ship', 'templates', 'mine.md'), 'mine\n'); + const r = bash(['set -e', 'IS_WINDOWS=0', 'SKILL_PREFIX=0', HELPERS, REAL_ASSETS, extractFn('link_claude_skill_dirs'), + `link_claude_skill_dirs "${t.payload}" "${t.skills}"`], t.tmp); + expect(r.status).toBe(0); + expect(fs.readFileSync(path.join(t.skills, 'qa', 'templates', 'mine.md'), 'utf-8')).toBe('mine\n'); + expect(fs.readFileSync(path.join(t.skills, 'qa', 'checklist.md'), 'utf-8')).toBe('my checklist\n'); + expect(fs.lstatSync(path.join(t.skills, 'qa', 'checklist.md')).isSymbolicLink()).toBe(false); + expect(fs.readFileSync(path.join(t.skills, 'ship', 'templates', 'mine.md'), 'utf-8')).toBe('mine\n'); + expect(r.stderr).toContain('kept qa/templates'); + expect(r.stderr).toContain('kept qa/checklist.md'); + expect(r.stderr).toContain('kept ship/templates'); + // review: created by us → assets linked + expect(fs.lstatSync(path.join(t.skills, 'review', 'templates')).isSymbolicLink()).toBe(true); + expect(fs.lstatSync(path.join(t.skills, 'review', 'checklist.md')).isSymbolicLink()).toBe(true); + // ship's customized SKILL.md was backed up, its assets kept, its checklist (absent before) linked + expect(fs.existsSync(path.join(t.tmp, '.gstack', 'backups', 'skills', 'test', 'ship', 'SKILL.md'))).toBe(true); + expect(fs.lstatSync(path.join(t.skills, 'ship', 'checklist.md')).isSymbolicLink()).toBe(true); + } finally { + fs.rmSync(t.tmp, { recursive: true, force: true }); + } + }); + + test('a foreign DIRECTORY symlink (target has no SKILL.md) is foreign, not unclaimed: the user\'s link survives', () => { + const t = mkTree(); + try { + fs.mkdirSync(path.join(t.payload, 'qa')); + fs.writeFileSync(path.join(t.payload, 'qa', 'SKILL.md'), GENERATED('qa')); + const userdir = path.join(t.tmp, 'userdir'); + fs.mkdirSync(userdir); + fs.writeFileSync(path.join(userdir, 'notes.md'), 'mine\n'); + fs.symlinkSync(userdir, path.join(t.skills, 'qa')); + const r = bash(['set -e', 'IS_WINDOWS=0', 'SKILL_PREFIX=0', HELPERS, extractFn('link_claude_skill_dirs'), + `link_claude_skill_dirs "${t.payload}" "${t.skills}"`, 'echo "FOREIGN=${_FOREIGN_SKIPPED_ENTRIES[*]:-}"'], t.tmp); + expect(r.status).toBe(0); + expect(r.stdout).toContain('FOREIGN=qa\n'); + expect(fs.lstatSync(path.join(t.skills, 'qa')).isSymbolicLink()).toBe(true); + expect(fs.readlinkSync(path.join(t.skills, 'qa'))).toBe(userdir); + expect(fs.existsSync(path.join(userdir, 'SKILL.md'))).toBe(false); + } finally { + fs.rmSync(t.tmp, { recursive: true, force: true }); + } + }); + + test('when the backup cannot be written, the customized file is left untouched and the entry is reported, never overwritten', () => { + const t = mkTree(); + try { + fs.mkdirSync(path.join(t.payload, 'qa')); + fs.writeFileSync(path.join(t.payload, 'qa', 'SKILL.md'), GENERATED('qa')); + const custom = GENERATED('qa').replace('# qa', '# customized'); + fs.mkdirSync(path.join(t.skills, 'qa')); + fs.writeFileSync(path.join(t.skills, 'qa', 'SKILL.md'), custom); + fs.writeFileSync(path.join(t.tmp, 'not-a-dir'), 'x'); + const r = bash(['set -e', 'IS_WINDOWS=0', 'SKILL_PREFIX=0', HELPERS, `_SKILL_BACKUP_ROOT="${t.tmp}/not-a-dir/backups"`, extractFn('link_claude_skill_dirs'), + `link_claude_skill_dirs "${t.payload}" "${t.skills}"`, 'echo "FOREIGN=${_FOREIGN_SKIPPED_ENTRIES[*]:-}"'], t.tmp); + expect(r.status).toBe(0); + expect(fs.lstatSync(path.join(t.skills, 'qa', 'SKILL.md')).isSymbolicLink()).toBe(false); + expect(fs.readFileSync(path.join(t.skills, 'qa', 'SKILL.md'), 'utf-8')).toBe(custom); + expect(r.stderr).toContain('could not back up'); + expect(r.stdout).toContain('FOREIGN=qa\n'); + } finally { + fs.rmSync(t.tmp, { recursive: true, force: true }); + } + }); + + test('Windows flip: a customized banner copy is moved to the backup root, not deleted; an alias-shaped copy (only name: differs) is just removed', () => { + const t = mkTree(); + try { + fs.mkdirSync(path.join(t.payload, 'qa')); + fs.writeFileSync(path.join(t.payload, 'qa', 'SKILL.md'), GENERATED('qa')); + fs.mkdirSync(path.join(t.payload, 'ship')); + fs.writeFileSync(path.join(t.payload, 'ship', 'SKILL.md'), GENERATED('ship')); + const custom = GENERATED('gstack-qa').replace('# gstack-qa', '# customized on windows'); + fs.mkdirSync(path.join(t.skills, 'gstack-qa')); + fs.writeFileSync(path.join(t.skills, 'gstack-qa', 'SKILL.md'), custom); + fs.mkdirSync(path.join(t.skills, 'gstack-ship')); + fs.writeFileSync(path.join(t.skills, 'gstack-ship', 'SKILL.md'), GENERATED('ship').replace('name: ship', 'name: gstack-ship')); + const r = bash(['set -e', 'IS_WINDOWS=1', HELPERS, extractFn('cleanup_prefixed_claude_symlinks'), + `cleanup_prefixed_claude_symlinks "${t.payload}" "${t.skills}"`], t.tmp); + expect(r.status).toBe(0); + expect(fs.existsSync(path.join(t.skills, 'gstack-qa'))).toBe(false); + expect(fs.readFileSync(path.join(t.tmp, '.gstack', 'backups', 'skills', 'test', 'gstack-qa', 'SKILL.md'), 'utf-8')).toBe(custom); + expect(fs.existsSync(path.join(t.skills, 'gstack-ship'))).toBe(false); + expect(fs.existsSync(path.join(t.tmp, '.gstack', 'backups', 'skills', 'test', 'gstack-ship'))).toBe(false); + } finally { + fs.rmSync(t.tmp, { recursive: true, force: true }); + } + }); + + test('a legacy linked dir holding the user\'s OWN symlink is mixed: our links go, theirs stays, the dir stays', () => { + const t = mkTree(); + try { + fs.mkdirSync(path.join(t.payload, 'qa', 'sections'), { recursive: true }); + fs.writeFileSync(path.join(t.payload, 'qa', 'SKILL.md'), GENERATED('qa')); + fs.mkdirSync(path.join(t.skills, 'gstack-qa')); + fs.symlinkSync(path.join(t.payload, 'qa', 'SKILL.md'), path.join(t.skills, 'gstack-qa', 'SKILL.md')); + fs.symlinkSync(path.join(t.payload, 'qa', 'sections'), path.join(t.skills, 'gstack-qa', 'sections')); + fs.writeFileSync(path.join(t.tmp, 'my-notes.md'), 'mine\n'); + fs.symlinkSync(path.join(t.tmp, 'my-notes.md'), path.join(t.skills, 'gstack-qa', 'notes.md')); + const r = bash(['set -e', 'IS_WINDOWS=0', HELPERS, extractFn('cleanup_prefixed_claude_symlinks'), + `cleanup_prefixed_claude_symlinks "${t.payload}" "${t.skills}"`], t.tmp); + expect(r.status).toBe(0); + expect(fs.existsSync(path.join(t.skills, 'gstack-qa', 'SKILL.md'))).toBe(false); + expect(fs.existsSync(path.join(t.skills, 'gstack-qa', 'sections'))).toBe(false); + expect(fs.readlinkSync(path.join(t.skills, 'gstack-qa', 'notes.md'))).toBe(path.join(t.tmp, 'my-notes.md')); + } finally { + fs.rmSync(t.tmp, { recursive: true, force: true }); + } + }); +});