From 73506b59c69125b6a7b48b9eb55196093e15776c Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 4 Sep 2026 17:55:07 +0000 Subject: [PATCH] fix(setup,relink): ownership proof has two strengths; weak proof never deletes a directory or discards a differing file The first #2119 gate treated a byte-identical or banner-bearing real-file SKILL.md as full ownership, so a prefix flip could rm -rf a user's directory (their own qa skill started from a gstack SKILL.md, plus my-templates/) and the link pass could replace their customized file with a symlink. Two strengths now: - STRONG: the .gstack-owned marker (we created the directory), or a directory holding nothing but symlinks and the marker (deleting it loses no data). Only strong proof removes a directory whole. - WEAK: byte-identity with our source or the two-line gen-skill-docs banner on a real file. Weak proof covers that SKILL.md and our runtime-asset links only; a differing file is moved to ${GSTACK_HOME:-~/.gstack}/backups/skills/// before we link over it, and setup/relink print one summary line naming what moved. The marker is written on every platform now (path-independent proof for Windows copies and for checkouts whose path carries no gstack segment), but only for a directory gstack creates: a directory we merely link into (unclaimed, or a legacy install) never becomes deletable whole. A directory with no SKILL.md at all is unclaimed: the link pass may add our file, the cleanup pass has nothing to remove. Also from the review passes: the banner check reads 8192 bytes, not 40 lines (investigate, office-hours, plan-ceo-review and design-consultation carry the banner past line 40 and were left "foreign" on pre-marker Windows installs); a link into a checkout named without a gstack segment (git worktree add ../gstack-) is ours when that tree carries setup + VERSION + bin/; relink's fast path is gone so both files canonicalize before judging; relink's root alias (_gstack-command) is gated and stamped like every other entry; relink reports the bare entry name with setup's wording and setup dedupes when forwarding (_run_relink_quiet); the summary names the browser skills as examples. Co-Authored-By: Claude Fable 5.1 --- bin/gstack-relink | 151 ++++++++++++++---- setup | 154 +++++++++++++++--- test/relink.test.ts | 181 ++++++++++++++++++++- test/setup-alias-name-uniqueness.test.ts | 19 +++ test/setup-cleanup-orphans.test.ts | 4 +- test/setup-link-ownership.test.ts | 191 ++++++++++++++++++++++- 6 files changed, 642 insertions(+), 58 deletions(-) diff --git a/bin/gstack-relink b/bin/gstack-relink index 6ce308715..ad8cb481e 100755 --- a/bin/gstack-relink +++ b/bin/gstack-relink @@ -49,8 +49,18 @@ RENDER_DIR="${GSTACK_USER_RENDER_DIR:-${GSTACK_HOME:-$HOME/.gstack}/render/claud # path) was deleted or had its SKILL.md replaced by a symlink into gstack # (#2119; Linux replaces a real file with `ln -snf`, macOS refuses by accident). # setup (link_claude_skill_dirs, cleanup_old_claude_symlinks, -# cleanup_prefixed_claude_symlinks) and gstack-uninstall apply the same rule; -# keep the four in sync until the shared helper TODOS.md files lands. +# cleanup_prefixed_claude_symlinks) applies the same rule; keep the two files +# in sync until the shared helper filed in TODOS.md lands. gstack-uninstall +# has its own stricter inventory+banner gate. +# +# Proof comes in two strengths. STRONG (a symlink resolving into gstack, or the +# .gstack-owned marker) means we created the entry: it may be deleted whole or +# refreshed in place. WEAK (byte-identity with our source, or gen-skill-docs' +# two-line banner on a real file) proves only that the SKILL.md came from us: +# it authorizes touching that one file, never deleting the directory, and a +# differing file is moved to ${GSTACK_HOME:-~/.gstack}/backups/skills// +# before we link over it (a user who started their own skill from a gstack +# SKILL.md looks exactly like a pre-marker legacy copy). # # An entry is OURS when: # - it is a symlink resolving into $INSTALL_DIR or $RENDER_DIR (as written or @@ -75,11 +85,17 @@ _target_is_ours() { # $1 = an ABSOLUTE path a symlink resolves to; ours when it lives under one # of our roots. "$ROOT"/* requires the separator, so /home/u/gstack2/x never # matches a /home/u/gstack root. + local root case "$1" in "$INSTALL_DIR"/*|"$RENDER_DIR"/*|"$_INSTALL_REAL"/*|"$_RENDER_REAL"/*) return 0 ;; gstack/*|*/gstack/*|*/.gstack/render/claude/*) return 0 ;; - *) return 1 ;; 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. + root="${1%/*/SKILL.md}" + if [ "$root" != "$1" ] && [ -f "$root/VERSION" ] && [ -f "$root/setup" ] && [ -d "$root/bin" ]; then return 0; fi + return 1 } # readlink of a RELATIVE symlink (older installs wrote `gstack/qa/SKILL.md`) @@ -92,12 +108,6 @@ _link_target_abs() { local link="$1" dest d b d_real dest="$(readlink "$link" 2>/dev/null || true)" [ -n "$dest" ] || return 1 - # Fast path: the common absolute link setup/relink wrote is decided without - # any further fork (relink runs on every ./setup and gstack-config set). - case "$dest" in - */../*|*/./*|*/..|*/.) ;; # dot segments: canonicalize before judging (/x/gstack/../foreign) - /*) if _target_is_ours "$dest"; then printf '%s\n' "$dest"; return 0; fi ;; - esac case "$dest" in /*) ;; *) dest="${link%/*}/$dest" ;; @@ -110,22 +120,38 @@ _link_target_abs() { fi } -# _entry_is_ours ENTRY SKILL — SKILL names the gstack skill this entry would -# serve, so a real-file copy can be compared against our own source. -_entry_is_ours() { - local entry="$1" skill="${2:-}" dest src +# _entry_owned_strongly ENTRY — we created this entry: a symlink resolving +# into gstack, or a real dir carrying the .gstack-owned marker or a SKILL.md +# symlink into gstack. +_entry_owned_strongly() { + local entry="$1" dest if [ -L "$entry" ]; then dest="$(_link_target_abs "$entry")" || return 1 _target_is_ours "$dest" return $? fi - if [ -d "$entry" ]; then - [ -f "$entry/.gstack-owned" ] && return 0 - if [ -L "$entry/SKILL.md" ]; then - dest="$(_link_target_abs "$entry/SKILL.md")" || return 1 - _target_is_ours "$dest" - return $? - fi + [ -d "$entry" ] || return 1 + [ -f "$entry/.gstack-owned" ] && return 0 + if [ -L "$entry/SKILL.md" ]; then + dest="$(_link_target_abs "$entry/SKILL.md")" || return 1 + _target_is_ours "$dest" + return $? + fi + return 1 +} + +# _entry_is_ours ENTRY SKILL — strong proof, or WEAK proof on a real-file +# SKILL.md (SKILL names the gstack skill this entry would serve, so the copy +# can be compared against our own source). +_entry_is_ours() { + local entry="$1" skill="${2:-}" src + _entry_owned_strongly "$entry" && return 0 + 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 + # SKILL.md overwrites nothing, so the link pass may proceed; the cleanup + # pass has nothing of ours to remove (see _cleanup_skill_entry). + [ -e "$entry/SKILL.md" ] || return 0 if [ -f "$entry/SKILL.md" ]; then for src in "$RENDER_DIR/$skill/SKILL.md" "$INSTALL_DIR/$skill/SKILL.md"; do [ -n "$skill" ] && [ -f "$src" ] && cmp -s "$entry/SKILL.md" "$src" && return 0 @@ -134,7 +160,7 @@ _entry_is_ours() { # top (same rule as setup's _gstack_generated_header), not a one-line # substring another generator could emit. A gstack fork rendering the # same banner is the accepted, filed residual. - case "$(head -n 40 "$entry/SKILL.md" 2>/dev/null)" in + case "$(head -c 8192 "$entry/SKILL.md" 2>/dev/null)" in *''*) return 0 ;; esac fi @@ -145,15 +171,32 @@ _entry_is_ours() { FOREIGN_SKIPPED=() _report_foreign() { - echo " skipped $1: not a gstack-managed entry (foreign skill with the same name) — left untouched" >&2 - FOREIGN_SKIPPED+=("$1") + # Same wording and bare name as setup's own line, so setup can dedupe when it + # forwards relink's output. + echo " skipped ${1##*/}: existing entry is not gstack-managed (foreign skill with the same name) — left untouched" >&2 + FOREIGN_SKIPPED+=("${1##*/}") +} + +# Weakly-proven real files we would otherwise overwrite go here, one summary +# line at the end. mv, not cp: the link that follows needs the path free. +BACKUP_ROOT="${GSTACK_HOME:-$HOME/.gstack}/backups/skills/$(date +%Y%m%dT%H%M%S)" +BACKED_UP=() +_backup_skill_md() { + 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 + 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:-}" + local entry="$1" skill="${2:-}" e dest [ -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 + return 0 + fi if ! _entry_is_ours "$entry" "$skill"; then _report_foreign "$entry" return 0 @@ -161,14 +204,47 @@ _cleanup_skill_entry() { if [ -L "$entry" ]; then rm -f "$entry" elif [ -d "$entry" ]; then - rm -rf "$entry" + # Whole-directory removal needs proof that nothing of the user's is inside: + # the marker (we created the dir) or a directory holding nothing but links. + if [ -f "$entry/.gstack-owned" ] || { [ -L "$entry/SKILL.md" ] && _dir_only_links "$entry"; }; then + rm -rf "$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. + rm -f "$entry/SKILL.md" "$entry/.gstack-owned" + for e in "$entry"/* "$entry"/.[!.]* "$entry"/..?*; do + [ -L "$e" ] || continue + dest="$(_link_target_abs "$e")" || continue + if _target_is_ours "$dest"; then rm -f "$e"; fi + done + rmdir "$entry" 2>/dev/null || echo " cleaned ${entry##*/}/SKILL.md (other files in that directory were left in place)" + fi fi } +# _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 + for e in "$d"/* "$d"/.[!.]* "$d"/..?*; do + { [ -e "$e" ] || [ -L "$e" ]; } || continue + [ -L "$e" ] && continue + [ "${e##*/}" = ".gstack-owned" ] && continue + return 1 + done + return 0 +} _link_root_skill_alias() { local target="$SKILLS_DIR/_gstack-command" [ -f "$INSTALL_DIR/SKILL.md" ] || return 0 + # Same ownership gate as every other entry (#2119): a user's own + # `_gstack-command` skill is reported and left alone, never overwritten. + if { [ -e "$target" ] || [ -L "$target" ]; } && ! _entry_is_ours "$target" ""; then + _report_foreign "$target" + return 0 + fi [ -L "$target" ] && rm -f "$target" mkdir -p "$target" # Copy-then-rewrite, never a symlink (#2511): a symlinked alias re-serves @@ -178,6 +254,9 @@ _link_root_skill_alias() { # write through it into the generated source. 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 } _link_root_skill_alias @@ -221,16 +300,29 @@ for skill_dir in "$INSTALL_DIR"/*/; do _report_foreign "$target" continue fi + # Remember whether WE are creating this directory: only then may the 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 # Upgrade old directory symlinks to real directories [ -L "$target" ] && rm -f "$target" # Create real directory with symlinked SKILL.md (absolute path) mkdir -p "$target" skill_md_src="$INSTALL_DIR/$skill/SKILL.md" [ -f "$RENDER_DIR/$skill/SKILL.md" ] && skill_md_src="$RENDER_DIR/$skill/SKILL.md" + # A real-file SKILL.md we can only WEAKLY prove ours and whose content + # 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" + fi ln -snf "$skill_md_src" "$target/SKILL.md" - # On Windows without Developer Mode `ln -snf` degrades to a copy; leave the - # same provenance marker setup writes so the next flip can prove ownership. - if [ ! -L "$target/SKILL.md" ]; then + # Provenance marker on every platform (path-independent proof; on Windows + # without Developer Mode `ln -snf` degrades to a copy and this is the only + # proof), but only for a directory we created or already owned. + if [ "$_pre_exists" -eq 0 ] || [ -f "$target/.gstack-owned" ]; then printf '%s\n' "$_INSTALL_REAL" > "$target/.gstack-owned" 2>/dev/null || true fi SKILL_COUNT=$((SKILL_COUNT + 1)) @@ -250,6 +342,9 @@ if [ "$PREFIX" = "true" ]; then else echo "Relinked $SKILL_COUNT skills as flat names" fi +if [ ${#BACKED_UP[@]} -gt 0 ]; then + echo "Moved ${#BACKED_UP[@]} pre-existing SKILL.md file(s) to $BACKUP_ROOT before linking gstack's: ${BACKED_UP[*]}" +fi if [ ${#FOREIGN_SKIPPED[@]} -gt 0 ]; then echo "Skipped ${#FOREIGN_SKIPPED[@]} foreign entr$( [ ${#FOREIGN_SKIPPED[@]} -eq 1 ] && echo y || echo ies) (not gstack-managed, left untouched): ${FOREIGN_SKIPPED[*]}" fi diff --git a/setup b/setup index fa0ea2773..121782cc4 100755 --- a/setup +++ b/setup @@ -119,7 +119,13 @@ _link_or_copy() { # ─── Ownership gate for skill entries (#2119) ───────────────────────────────── # setup and gstack-relink must never delete or link over a skill they do not # own. This is the single rule both use (relink carries the same logic — keep -# them in sync until the shared helper TODOS.md files lands). An entry is OURS +# them in sync until the shared helper filed in TODOS.md lands). Proof has two +# strengths: STRONG (a symlink resolving into gstack, or the .gstack-owned +# marker) means we created the entry and may delete or refresh it whole; WEAK +# (byte-identity with our source, or the two-line generated banner on a real +# file) covers only that SKILL.md — never the directory — and a differing +# weakly-proven file is moved to ${GSTACK_HOME:-~/.gstack}/backups/skills// +# before we install over it. An entry is OURS # when: it is a symlink resolving into the gstack payload / render dir (or any # path with a `gstack` segment, the convention cleanup and gstack-uninstall # already use, so entries from a sibling worktree still count), a real dir @@ -147,12 +153,40 @@ _gstack_target_is_ours() { case "$t" in "$g"/*|"$g_real"/*|"$render"/*|"$render_real"/*|gstack/*|*/gstack/*|*/.gstack/render/claude/*) return 0 ;; 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/. + local root="${t%/*/SKILL.md}" + if [ "$root" != "$t" ] && [ -f "$root/VERSION" ] && [ -f "$root/setup" ] && [ -d "$root/bin" ]; then return 0; fi return 1 } _claude_entry_is_ours() { # $1 = existing entry (dir or symlink), $2 = the gstack source SKILL.md it # would be linked to, $3 = gstack payload dir - local entry="$1" src_md="$2" g="$3" dest + local entry="$1" src_md="$2" g="$3" render_md + _claude_entry_owned_strongly "$entry" "$g" && return 0 + # 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 + # require a SKILL.md and so never touch it. + if [ -d "$entry" ] && [ ! -e "$entry/SKILL.md" ] && [ ! -L "$entry/SKILL.md" ]; then return 0; fi + if [ -d "$entry" ] && [ -f "$entry/SKILL.md" ] && [ ! -L "$entry/SKILL.md" ]; then + [ -n "$src_md" ] && [ -f "$src_md" ] && cmp -s "$entry/SKILL.md" "$src_md" && return 0 + # A gbrain install serves the RENDERED file (link_claude_skill_dirs prefers + # it), so an exact copy of that render is ours too. + if [ -n "$src_md" ]; then + render_md="${GSTACK_USER_RENDER_DIR:-${GSTACK_HOME:-$HOME/.gstack}/render/claude}/$(basename "$(dirname "$src_md")")/SKILL.md" + [ -f "$render_md" ] && cmp -s "$entry/SKILL.md" "$render_md" && return 0 + fi + _gstack_generated_header "$entry/SKILL.md" && return 0 + fi + return 1 +} +# _claude_entry_owned_strongly ENTRY GSTACK_DIR — we created it: a symlink into +# gstack, or a real dir with the .gstack-owned marker or a SKILL.md symlink +# into gstack. Only strong proof authorizes deleting a directory whole. +_claude_entry_owned_strongly() { + local entry="$1" g="$2" dest if [ -L "$entry" ]; then dest="$(_gstack_link_target_abs "$entry")" || return 1 _gstack_target_is_ours "$dest" "$g"; return $? @@ -163,20 +197,62 @@ _claude_entry_is_ours() { dest="$(_gstack_link_target_abs "$entry/SKILL.md")" || return 1 _gstack_target_is_ours "$dest" "$g"; return $? fi - if [ -f "$entry/SKILL.md" ]; then - [ -n "$src_md" ] && [ -f "$src_md" ] && cmp -s "$entry/SKILL.md" "$src_md" && return 0 - _gstack_generated_header "$entry/SKILL.md" && return 0 - fi return 1 } +# Weakly-proven real files we would otherwise overwrite are moved here (mv, so +# the path is free for the link); the final summary prints one line. +_SKILL_BACKUP_ROOT="${GSTACK_HOME:-$HOME/.gstack}/backups/skills/$(date +%Y%m%dT%H%M%S)" +_BACKED_UP_SKILL_MDS=() +_backup_skill_md() { + 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 + 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() { + 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. + for e in "$d"/* "$d"/.[!.]* "$d"/..?*; do + [ -L "$e" ] || continue + dest="$(_gstack_link_target_abs "$e")" || continue + if _gstack_target_is_ours "$dest" "$g"; then rm -f "$e"; fi + 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() { + local d="$1" e + for e in "$d"/* "$d"/.[!.]* "$d"/..?*; do + { [ -e "$e" ] || [ -L "$e" ]; } || continue + [ -L "$e" ] && continue + [ "${e##*/}" = ".gstack-owned" ] && continue + 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. +_cleanup_linked_dir() { + if [ -f "$1/.gstack-owned" ] || _gstack_dir_only_links "$1"; 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 # near the top, not by a one-line substring another generator could plausibly # emit. Still forgeable by a gstack fork that renders the same banner — that # residual is accepted and filed; the marker is the load-bearing signal. _gstack_generated_header() { + # Bytes, not lines: a long frontmatter pushes the banner past line 40 in + # four real skills (investigate: line 57), and a line-count check left them + # "foreign" on every pre-marker Windows install. local f="$1" head40 - head40="$(head -n 40 "$f" 2>/dev/null)" || return 1 + head40="$(head -c 8192 "$f" 2>/dev/null)" || return 1 case "$head40" in *''*) return 0 ;; esac @@ -1037,6 +1113,27 @@ _link_skill_runtime_assets() { # gstack/ (which would auto-prefix them as gstack-*). # When SKILL_PREFIX=1, directories are prefixed with "gstack-". # Use --no-prefix to restore flat names. +# Run gstack-relink and surface only what the user must see: foreign entries it +# skipped (deduped against the ones this setup already reported, same wording) +# and any pre-existing SKILL.md it moved to the backup root. +_run_relink_quiet() { + local out line name + out="$(GSTACK_SKILLS_DIR="$INSTALL_SKILLS_DIR" GSTACK_INSTALL_DIR="$SOURCE_GSTACK_DIR" "$GSTACK_RELINK" 2>&1 || true)" + while IFS= read -r line; do + case "$line" in + ' skipped '*) + name="${line# skipped }"; name="${name%%:*}" + case " ${_FOREIGN_SKIPPED_ENTRIES[*]:-} " in + *" $name "*) ;; + *) echo "$line" >&2; _FOREIGN_SKIPPED_ENTRIES+=("$name") ;; + esac ;; + 'Moved '*|' cleaned '*) echo " ${line# }" >&2 ;; + esac + done < "$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" } # Claude Code skips the repo-shaped ~/.claude/skills/gstack directory when @@ -1194,7 +1308,7 @@ cleanup_old_claude_symlinks() { # render prefix (~/.gstack/render/claude/...), which is not `/gstack/`. case "$link_dest" in gstack/*|*/gstack/*|*/.gstack/render/claude/*) - rm -rf "$old_target" + _cleanup_linked_dir "$old_target" "$gstack_dir" removed+=("$skill_name") ;; esac @@ -1220,7 +1334,9 @@ cleanup_old_claude_symlinks() { && { [ -f "$old_target/.gstack-owned" ] \ || cmp -s "$old_target/SKILL.md" "$skill_dir/SKILL.md" \ || _gstack_generated_header "$old_target/SKILL.md"; }; then - rm -rf "$old_target" + # 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 removed+=("$skill_name") fi fi @@ -1262,7 +1378,7 @@ cleanup_prefixed_claude_symlinks() { link_dest="$(readlink "$prefixed_target/SKILL.md" 2>/dev/null || true)" case "$link_dest" in gstack/*|*/gstack/*|*/.gstack/render/claude/*) - rm -rf "$prefixed_target" + _cleanup_linked_dir "$prefixed_target" "$gstack_dir" removed+=("gstack-$skill_name") ;; esac @@ -1274,7 +1390,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 - rm -rf "$prefixed_target" + if [ -f "$prefixed_target/.gstack-owned" ]; then rm -rf "$prefixed_target"; else _cleanup_weak_dir "$prefixed_target" "$gstack_dir"; fi removed+=("gstack-$skill_name") fi fi @@ -1832,10 +1948,7 @@ if [ "$INSTALL_CLAUDE" -eq 1 ]; then # setup, stale git state, or gen:skill-docs left name: fields out of sync. GSTACK_RELINK="$SOURCE_GSTACK_DIR/bin/gstack-relink" if [ -x "$GSTACK_RELINK" ]; then - # relink's own "skipped" lines (foreign entries) must reach the user; - # everything else it prints is noise here. - _RELINK_OUT="$(GSTACK_SKILLS_DIR="$INSTALL_SKILLS_DIR" GSTACK_INSTALL_DIR="$SOURCE_GSTACK_DIR" "$GSTACK_RELINK" 2>&1 || true)" - printf '%s\n' "$_RELINK_OUT" | grep '^ skipped ' >&2 || true + _run_relink_quiet fi # Backwards-compat alias: /connect-chrome → /open-gstack-browser # Rewritten copy, not a symlink: a symlinked alias re-serves the canonical @@ -1908,8 +2021,7 @@ if [ "$INSTALL_CLAUDE" -eq 1 ]; then _CLAUDE_SKILLS_LINKED=1 GSTACK_RELINK="$SOURCE_GSTACK_DIR/bin/gstack-relink" if [ -x "$GSTACK_RELINK" ]; then - _RELINK_OUT="$(GSTACK_SKILLS_DIR="$INSTALL_SKILLS_DIR" GSTACK_INSTALL_DIR="$SOURCE_GSTACK_DIR" "$GSTACK_RELINK" 2>&1 || true)" - printf '%s\n' "$_RELINK_OUT" | grep '^ skipped ' >&2 || true + _run_relink_quiet fi # Rewritten copy, not a symlink: a symlinked alias re-serves the # canonical name: open-gstack-browser, so one of the two silently diff --git a/test/relink.test.ts b/test/relink.test.ts index f6d69a246..9a1f1070b 100644 --- a/test/relink.test.ts +++ b/test/relink.test.ts @@ -686,12 +686,13 @@ describe('gstack-relink ownership gate (#2119)', () => { return run(`${path.join(installDir, 'bin', 'gstack-relink')} 2>&1`, { GSTACK_INSTALL_DIR: installDir, GSTACK_SKILLS_DIR: skillsDir, + GSTACK_HOME: path.join(tmpDir, 'home'), GSTACK_USER_RENDER_DIR: path.join(tmpDir, 'no-render'), ...env, }); } function setPrefix(v: 'true' | 'false') { run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix ${v}`, { - GSTACK_INSTALL_DIR: installDir, GSTACK_SKILLS_DIR: skillsDir, + GSTACK_INSTALL_DIR: installDir, GSTACK_SKILLS_DIR: skillsDir, GSTACK_HOME: path.join(tmpDir, 'home'), GSTACK_USER_RENDER_DIR: path.join(tmpDir, 'no-render'), }); } @@ -855,7 +856,7 @@ describe('gstack-relink ownership gate (#2119)', () => { expect(out).toContain('skipped'); }); - test('several foreign entries: pluralized summary lists every path and the relinked count excludes them', () => { + test('several foreign entries: pluralized summary lists every name and the relinked count excludes them', () => { setupMockInstall(['qa', 'ship', 'review']); for (const name of ['qa', 'ship']) { fs.mkdirSync(path.join(skillsDir, name)); @@ -865,8 +866,10 @@ describe('gstack-relink ownership gate (#2119)', () => { const out = relink(); expect(out).toContain('Relinked 1 skills as flat names'); expect(out).toContain('Skipped 2 foreign entries'); - expect(out).toContain(path.join(skillsDir, 'qa')); - expect(out).toContain(path.join(skillsDir, 'ship')); + // Bare names, same wording as setup's own line, so setup can dedupe when it forwards relink's output. + expect(out).toContain('skipped qa: existing entry is not gstack-managed'); + expect(out).toContain('skipped ship: existing entry is not gstack-managed'); + expect(out).toContain('left untouched): qa ship'); expect(out).not.toContain('foreign entry '); expect(fs.lstatSync(path.join(skillsDir, 'review', 'SKILL.md')).isSymbolicLink()).toBe(true); }); @@ -887,12 +890,13 @@ describe('gstack-relink ownership gate parity with setup (#2119 review fixes)', const FOREIGN = '---\nname: qa\ndescription: my own qa skill\n---\n# not gstack'; function relink(env: Record = {}): string { return run(`${path.join(installDir, 'bin', 'gstack-relink')} 2>&1`, { - GSTACK_INSTALL_DIR: installDir, GSTACK_SKILLS_DIR: skillsDir, ...env, + GSTACK_INSTALL_DIR: installDir, GSTACK_SKILLS_DIR: skillsDir, GSTACK_HOME: path.join(tmpDir, 'home'), GSTACK_USER_RENDER_DIR: path.join(tmpDir, 'no-render'), + ...env, }); } function setPrefix(v: 'true' | 'false') { run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix ${v}`, { - GSTACK_INSTALL_DIR: installDir, GSTACK_SKILLS_DIR: skillsDir, + GSTACK_INSTALL_DIR: installDir, GSTACK_SKILLS_DIR: skillsDir, GSTACK_HOME: path.join(tmpDir, 'home'), GSTACK_USER_RENDER_DIR: path.join(tmpDir, 'no-render'), }); } @@ -977,3 +981,168 @@ describe('gstack-relink ownership gate parity with setup (#2119 review fixes)', expect(fs.readlinkSync(path.join(skillsDir, 'qa', 'SKILL.md'))).toBe(path.join(installDir, 'qa', 'SKILL.md')); }); }); + +describe('gstack-relink root alias (_gstack-command) ownership gate', () => { + const ROOT_SKILL = '---\nname: gstack\ndescription: root\n---\n\n\n# gstack root\n'; + function relink(): string { + return run(`${path.join(installDir, 'bin', 'gstack-relink')} 2>&1`, { + GSTACK_INSTALL_DIR: installDir, GSTACK_SKILLS_DIR: skillsDir, GSTACK_HOME: path.join(tmpDir, 'home'), GSTACK_USER_RENDER_DIR: path.join(tmpDir, 'no-render'), + }); + } + + test('a user-owned _gstack-command skill is reported and left byte-identical', () => { + setupMockInstall(['qa']); + fs.writeFileSync(path.join(installDir, 'SKILL.md'), ROOT_SKILL); + const mine = '---\nname: _gstack-command\ndescription: my own command runner\n---\n# mine\n'; + fs.mkdirSync(path.join(skillsDir, '_gstack-command')); + fs.writeFileSync(path.join(skillsDir, '_gstack-command', 'SKILL.md'), mine); + const out = relink(); + expect(fs.readFileSync(path.join(skillsDir, '_gstack-command', 'SKILL.md'), 'utf-8')).toBe(mine); + expect(fs.existsSync(path.join(skillsDir, '_gstack-command', '.gstack-owned'))).toBe(false); + expect(out).toContain('skipped'); + expect(out).toContain('_gstack-command'); + }); + + test('our own rewritten alias copy is refreshed and stamped with the ownership marker', () => { + setupMockInstall(['qa']); + fs.writeFileSync(path.join(installDir, 'SKILL.md'), ROOT_SKILL); + const first = relink(); + expect(first).not.toContain('skipped'); + const alias = path.join(skillsDir, '_gstack-command'); + expect(fs.readFileSync(path.join(alias, 'SKILL.md'), 'utf-8')).toContain('name: _gstack-command'); + expect(fs.existsSync(path.join(alias, '.gstack-owned'))).toBe(true); + // Stale copy (older render) with the marker: refreshed, not reported. + fs.writeFileSync(path.join(alias, 'SKILL.md'), '---\nname: _gstack-command\n---\n# stale\n'); + const second = relink(); + expect(second).not.toContain('skipped'); + expect(fs.readFileSync(path.join(alias, 'SKILL.md'), 'utf-8')).toContain('# gstack root'); + }); +}); + +describe('gstack-relink weak proof is file-scoped; differing files are moved aside (#2119 review)', () => { + const BANNER = '\n'; + const env = () => ({ + GSTACK_INSTALL_DIR: installDir, GSTACK_SKILLS_DIR: skillsDir, GSTACK_HOME: path.join(tmpDir, 'home'), GSTACK_USER_RENDER_DIR: path.join(tmpDir, 'no-render'), + GSTACK_HOME: path.join(tmpDir, 'home'), GSTACK_USER_RENDER_DIR: path.join(tmpDir, 'no-render'), + }); + const relink = () => run(`${path.join(installDir, 'bin', 'gstack-relink')} 2>&1`, env()); + const setPrefix = (v: 'true' | 'false') => run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix ${v}`, env()); + const backups = () => { + const root = path.join(tmpDir, '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('flip cleanup on a banner-only copy removes SKILL.md and keeps the user\'s other files and the directory', () => { + setupMockInstall(['qa']); + fs.mkdirSync(path.join(skillsDir, 'gstack-qa', 'my-templates'), { recursive: true }); + fs.writeFileSync(path.join(skillsDir, 'gstack-qa', 'SKILL.md'), `---\nname: gstack-qa\n---\n${BANNER}\n# started from gstack\n`); + fs.writeFileSync(path.join(skillsDir, 'gstack-qa', 'my-templates', 'checklist.md'), '- mine\n'); + setPrefix('false'); + const out = relink(); + expect(fs.existsSync(path.join(skillsDir, 'gstack-qa', 'SKILL.md'))).toBe(false); + expect(fs.readFileSync(path.join(skillsDir, 'gstack-qa', 'my-templates', 'checklist.md'), 'utf-8')).toBe('- mine\n'); + expect(out).not.toContain('skipped'); + }); + + test('a marker-proven directory (we created it) is still removed whole on a flip', () => { + setupMockInstall(['qa']); + fs.mkdirSync(path.join(skillsDir, 'gstack-qa', 'sections'), { recursive: true }); + fs.writeFileSync(path.join(skillsDir, 'gstack-qa', 'SKILL.md'), '# stale copy\n'); + fs.writeFileSync(path.join(skillsDir, 'gstack-qa', 'sections', 'a.md'), 'a\n'); + fs.writeFileSync(path.join(skillsDir, 'gstack-qa', '.gstack-owned'), installDir + '\n'); + setPrefix('false'); + relink(); + expect(fs.existsSync(path.join(skillsDir, 'gstack-qa'))).toBe(false); + }); + + test('linking over a CUSTOMIZED banner copy moves it to the backup root first; the link then lands', () => { + setupMockInstall(['qa']); + const custom = `---\nname: qa\n---\n${BANNER}\n# my qa, started from gstack\n`; + fs.mkdirSync(path.join(skillsDir, 'qa')); + fs.writeFileSync(path.join(skillsDir, 'qa', 'SKILL.md'), custom); + setPrefix('false'); + const out = relink(); + expect(fs.lstatSync(path.join(skillsDir, 'qa', 'SKILL.md')).isSymbolicLink()).toBe(true); + const saved = backups(); + expect(saved.length).toBe(1); + expect(fs.readFileSync(saved[0], 'utf-8')).toBe(custom); + expect(out).not.toContain('skipped'); + }); + + test('linking over a byte-identical copy makes no backup', () => { + setupMockInstall(['qa']); + fs.mkdirSync(path.join(skillsDir, 'qa')); + fs.copyFileSync(path.join(installDir, 'qa', 'SKILL.md'), path.join(skillsDir, 'qa', 'SKILL.md')); + setPrefix('false'); + relink(); + expect(fs.lstatSync(path.join(skillsDir, 'qa', 'SKILL.md')).isSymbolicLink()).toBe(true); + expect(backups()).toEqual([]); + }); +}); + +describe('gstack-relink: checkout naming, legacy linked dirs, markers (#2119 review)', () => { + const env = () => ({ + GSTACK_INSTALL_DIR: installDir, GSTACK_SKILLS_DIR: skillsDir, + GSTACK_HOME: path.join(tmpDir, 'home'), GSTACK_USER_RENDER_DIR: path.join(tmpDir, 'no-render'), + }); + const relink = () => run(`${path.join(installDir, 'bin', 'gstack-relink')} 2>&1`, env()); + const setPrefix = (v: 'true' | 'false') => run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix ${v}`, env()); + + test('an entry linked into a checkout whose path has no `gstack` segment (git worktree add ../gs-feature) is ours when that tree carries setup + VERSION + bin/', () => { + setupMockInstall(['qa']); + const other = path.join(tmpDir, 'gs-feature'); + fs.mkdirSync(path.join(other, 'qa'), { recursive: true }); + 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, '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')); + setPrefix('false'); + 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. + const plain = path.join(tmpDir, 'plain-tools'); + fs.mkdirSync(path.join(plain, 'ship'), { recursive: true }); + 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'); + fs.mkdirSync(path.join(skillsDir, 'ship')); + fs.symlinkSync(path.join(plain, 'ship', 'SKILL.md'), path.join(skillsDir, 'ship', 'SKILL.md')); + const out2 = relink(); + expect(out2).toContain('skipped ship'); + }); + + test('a directory relink creates gets the marker on Linux; a pre-existing unclaimed directory does not', () => { + setupMockInstall(['qa', 'ship']); + fs.mkdirSync(path.join(skillsDir, 'ship')); + fs.writeFileSync(path.join(skillsDir, 'ship', 'notes.md'), 'mine\n'); + setPrefix('false'); + relink(); + expect(fs.existsSync(path.join(skillsDir, 'qa', '.gstack-owned'))).toBe(true); + expect(fs.lstatSync(path.join(skillsDir, 'ship', 'SKILL.md')).isSymbolicLink()).toBe(true); + expect(fs.existsSync(path.join(skillsDir, 'ship', '.gstack-owned'))).toBe(false); + expect(fs.readFileSync(path.join(skillsDir, 'ship', 'notes.md'), 'utf-8')).toBe('mine\n'); + }); + + test('flip on a legacy linked dir (no marker): all-links dir is removed whole; a dir with a user file keeps the file and drops our links', () => { + setupMockInstall(['qa', 'ship']); + fs.mkdirSync(path.join(installDir, 'qa', 'sections')); + for (const name of ['gstack-qa', 'gstack-ship']) { + fs.mkdirSync(path.join(skillsDir, name)); + fs.symlinkSync(path.join(installDir, 'qa', 'SKILL.md'), path.join(skillsDir, name, 'SKILL.md')); + fs.symlinkSync(path.join(installDir, 'qa', 'sections'), path.join(skillsDir, name, 'sections')); + } + fs.writeFileSync(path.join(skillsDir, 'gstack-ship', 'my-notes.md'), 'keep\n'); + // gstack-config `set` auto-relinks, so the flip cleanup runs there; capture both outputs. + const out = setPrefix('false') + relink(); + expect(fs.existsSync(path.join(skillsDir, 'gstack-qa'))).toBe(false); + expect(fs.existsSync(path.join(skillsDir, 'gstack-ship', 'SKILL.md'))).toBe(false); + expect(fs.existsSync(path.join(skillsDir, 'gstack-ship', 'sections'))).toBe(false); + expect(fs.readFileSync(path.join(skillsDir, 'gstack-ship', 'my-notes.md'), 'utf-8')).toBe('keep\n'); + expect(out).toContain('cleaned gstack-ship/SKILL.md'); + expect(out).not.toContain('skipped'); + }); +}); diff --git a/test/setup-alias-name-uniqueness.test.ts b/test/setup-alias-name-uniqueness.test.ts index c05d1d6d6..589d0aa3c 100644 --- a/test/setup-alias-name-uniqueness.test.ts +++ b/test/setup-alias-name-uniqueness.test.ts @@ -52,7 +52,18 @@ beforeAll(() => { 'SKILL_PREFIX=0', 'QUIET=1', '_WINDOWS_COPY_NOTE_PRINTED=1', + '_FOREIGN_SKIPPED_ENTRIES=()', + `SOURCE_GSTACK_DIR="${ROOT}"`, extractFn('_link_or_copy'), + extractFn('_gstack_link_target_abs'), + extractFn('_gstack_target_is_ours'), + extractFn('_gstack_generated_header'), + extractFn('_claude_entry_is_ours'), + extractFn('_claude_entry_owned_strongly'), + extractFn('_backup_skill_md'), + '_BACKED_UP_SKILL_MDS=()', + `_SKILL_BACKUP_ROOT="${os.tmpdir()}/gstack-alias-test-backups"`, + extractFn('_write_owned_marker'), extractFn('_print_windows_copy_note_once'), extractFn('_link_skill_runtime_assets'), extractFn('link_claude_skill_dirs'), @@ -144,7 +155,15 @@ describe('alias installs are rewritten copies (#2511, #2201)', () => { const script = [ 'set -e', 'IS_WINDOWS=0', + '_FOREIGN_SKIPPED_ENTRIES=()', + `SOURCE_GSTACK_DIR="${ROOT}"`, extractFn('_link_or_copy'), + extractFn('_gstack_link_target_abs'), + extractFn('_gstack_target_is_ours'), + extractFn('_gstack_generated_header'), + extractFn('_claude_entry_is_ours'), + extractFn('_claude_entry_owned_strongly'), + extractFn('_write_owned_marker'), extractFn('_install_alias_skill_md'), extractFn('link_claude_root_skill_alias'), `link_claude_root_skill_alias "${ROOT}" "${legacyDir}"`, diff --git a/test/setup-cleanup-orphans.test.ts b/test/setup-cleanup-orphans.test.ts index a3f4d25fc..50ac75412 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_generated_header') + extractFn('cleanup_old_claude_symlinks'); + 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'); } 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_generated_header'), + 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'), `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 c574ebf22..110cf29d8 100644 --- a/test/setup-link-ownership.test.ts +++ b/test/setup-link-ownership.test.ts @@ -33,6 +33,13 @@ const HELPERS = [ extractFn('_claude_entry_is_ours'), extractFn('_write_owned_marker'), extractFn('_gstack_generated_header'), + extractFn('_claude_entry_owned_strongly'), + extractFn('_backup_skill_md'), + extractFn('_cleanup_weak_dir'), + extractFn('_gstack_dir_only_links'), + extractFn('_cleanup_linked_dir'), + '_BACKED_UP_SKILL_MDS=()', + '_SKILL_BACKUP_ROOT="$HOME/.gstack/backups/skills/test"', ].join('\n'); const FOREIGN = '---\nname: qa\ndescription: mine\n---\n# not gstack\n'; @@ -50,6 +57,9 @@ function bash(lines: string[], tmp: string) { encoding: 'utf-8', timeout: 10_000, env: { PATH: process.env.PATH ?? '', HOME: tmp, GSTACK_USER_RENDER_DIR: path.join(tmp, 'no-render') }, }); + // An extracted function calling a helper this harness forgot to extract must + // fail loudly, not degrade into "foreign, skipped". + if (/command not found/.test(r.stderr ?? '')) throw new Error(`harness drift (missing extracted helper):\n${r.stderr}`); return { status: r.status ?? -1, stdout: r.stdout ?? '', stderr: r.stderr ?? '' }; } @@ -140,6 +150,9 @@ 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)); + expect(fs.existsSync(path.join(t.skills, 'connect-chrome', '.gstack-owned'))).toBe(false); } finally { fs.rmSync(t.tmp, { recursive: true, force: true }); } @@ -152,7 +165,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_generated_header'), 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('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 }); @@ -188,6 +201,25 @@ describe.skipIf(process.platform === 'win32')('setup: cleanup_prefixed_claude_sy expect(h.names).toEqual(['gstack']); }); + test('Windows: weak proof (banner, no marker) removes only SKILL.md; the user\'s other files and the directory survive', () => { + const r = runFlip('1', (skills) => { + fs.mkdirSync(path.join(skills, 'gstack-qa', 'my-templates'), { recursive: true }); + fs.writeFileSync(path.join(skills, 'gstack-qa', 'SKILL.md'), GENERATED('gstack-qa').replace('# gstack-qa', '# started from gstack, then customized')); + fs.writeFileSync(path.join(skills, 'gstack-qa', 'my-templates', 'checklist.md'), '- mine\n'); + }); + expect(r.status).toBe(0); + expect(r.names).toEqual(['gstack', 'gstack-qa']); + expect(r.stdout).toContain('cleaned gstack-qa/SKILL.md'); + // Strong proof (marker) still removes the directory we created. + const m = runFlip('1', (skills) => { + fs.mkdirSync(path.join(skills, 'gstack-qa', 'sections'), { recursive: true }); + fs.writeFileSync(path.join(skills, 'gstack-qa', 'SKILL.md'), '# stale\n'); + fs.writeFileSync(path.join(skills, 'gstack-qa', 'sections', 'x.md'), 'x\n'); + fs.writeFileSync(path.join(skills, 'gstack-qa', '.gstack-owned'), '/payload\n'); + }); + expect(m.names).toEqual(['gstack']); + }); + test('Windows: a ONE-line AUTO-GENERATED substring from another generator is not provenance — the entry survives', () => { const r = runFlip('1', (skills) => { fs.mkdirSync(path.join(skills, 'gstack-qa')); @@ -211,3 +243,160 @@ describe.skipIf(process.platform === 'win32')('setup: cleanup_prefixed_claude_sy expect(reap.stdout).toContain('cleaned up prefixed entries: gstack-qa'); }); }); + +describe.skipIf(process.platform === 'win32')('setup: weakly-proven files are moved aside, never overwritten (#2119 review)', () => { + test('Linux linker: a customized banner copy is moved to the backup root before the symlink lands; an identical copy is not', () => { + 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('qa').replace('# qa', '# my qa, started from gstack'); + fs.mkdirSync(path.join(t.skills, 'qa')); + fs.writeFileSync(path.join(t.skills, 'qa', 'SKILL.md'), custom); + fs.mkdirSync(path.join(t.skills, 'ship')); + fs.copyFileSync(path.join(t.payload, '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 "BACKED=${_BACKED_UP_SKILL_MDS[*]:-}"'], t.tmp); + expect(r.status).toBe(0); + expect(fs.lstatSync(path.join(t.skills, 'qa', 'SKILL.md')).isSymbolicLink()).toBe(true); + expect(fs.lstatSync(path.join(t.skills, 'ship', 'SKILL.md')).isSymbolicLink()).toBe(true); + expect(fs.readFileSync(path.join(t.tmp, '.gstack', 'backups', 'skills', 'test', 'qa', 'SKILL.md'), 'utf-8')).toBe(custom); + // Weakly-proven, pre-existing: no marker (it must never become deletable whole). Created dirs do get one. + expect(fs.existsSync(path.join(t.skills, 'qa', '.gstack-owned'))).toBe(false); + expect(fs.existsSync(path.join(t.tmp, '.gstack', 'backups', 'skills', 'test', 'ship'))).toBe(false); + expect(r.stdout).toContain('BACKED=qa\n'); + } finally { + fs.rmSync(t.tmp, { recursive: true, force: true }); + } + }); + + test('Windows flip: weak proof leaves the user\'s files in place', () => { + 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.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'), + `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.readFileSync(path.join(t.skills, 'gstack-qa', 'my-templates', 'checklist.md'), 'utf-8')).toBe('- mine\n'); + } finally { + fs.rmSync(t.tmp, { recursive: true, force: true }); + } + }); + + test('_run_relink_quiet forwards relink\'s skipped and moved lines once, deduped against setup\'s own report', () => { + const t = mkTree(); + try { + const fake = path.join(t.tmp, 'fake-relink'); + fs.writeFileSync(fake, '#!/usr/bin/env bash\necho "linked 3 skills"\necho " skipped qa: existing entry is not gstack-managed (foreign skill with the same name) — left untouched" >&2\necho " skipped ship: existing entry is not gstack-managed (foreign skill with the same name) — left untouched" >&2\necho "Moved 1 pre-existing SKILL.md file(s) to /x before linking gstack\'s: review"\n'); + fs.chmodSync(fake, 0o755); + const r = bash(['set -e', '_FOREIGN_SKIPPED_ENTRIES=(qa)', `GSTACK_RELINK="${fake}"`, 'INSTALL_SKILLS_DIR=/dev/null', 'SOURCE_GSTACK_DIR=/dev/null', + extractFn('_run_relink_quiet'), '_run_relink_quiet', 'echo "FOREIGN=${_FOREIGN_SKIPPED_ENTRIES[*]:-}"'], t.tmp); + expect(r.status).toBe(0); + expect(r.stderr.match(/skipped ship/g)?.length).toBe(1); + expect(r.stderr).not.toContain('skipped qa'); + expect(r.stderr).toContain('Moved 1 pre-existing'); + expect(r.stderr).not.toContain('linked 3 skills'); + expect(r.stdout).toContain('FOREIGN=qa ship\n'); + } finally { + fs.rmSync(t.tmp, { recursive: true, force: true }); + } + }); +}); + +describe.skipIf(process.platform === 'win32')('setup: banner census, checkout naming, legacy linked dirs, markers (#2119 review)', () => { + test('every generated SKILL.md in this tree passes _gstack_generated_header (four carry the banner past line 40)', () => { + const files = fs.readdirSync(ROOT, { withFileTypes: true }) + .filter((d) => d.isDirectory()) + .map((d) => path.join(ROOT, d.name, 'SKILL.md')) + .concat([path.join(ROOT, 'SKILL.md')]) + .filter((f) => fs.existsSync(f) && fs.readFileSync(f, 'utf-8').includes('