From 4fc64f7f964a273f57167a193f9c88a9477cf79c Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Thu, 2 Apr 2026 18:34:00 -0700 Subject: [PATCH] fix: top-level skill dirs so Claude discovers unprefixed names (#761) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: top-level skill dirs so Claude discovers unprefixed names Replace directory symlinks (gstack/qa → qa) with real directories containing a SKILL.md symlink. Claude Code auto-prefixes skills nested under a parent dir symlink, so /plan-ceo-review became "Unknown skill" even with skill_prefix=false. Real dirs fix this. Also syncs package.json version to match VERSION file and updates test assertions to match the new mkdir + ln approach. Co-Authored-By: Claude Opus 4.6 * docs: update symlink references to new top-level directory pattern Co-Authored-By: Claude Opus 4.6 * test: regression tests for top-level skill directory structure Verifies the invariant that setup/relink creates real directories (not symlinks) at the top level, with SKILL.md symlinks inside. This prevents Claude Code from auto-prefixing skills with gstack- when using --no-prefix. Tests added: - unprefixed skills must be real dirs with SKILL.md symlinks - prefixed skills must also be real dirs with SKILL.md symlinks - old directory symlinks get upgraded to real directories - cleanup functions handle both old symlinks and new dir pattern - link function removes old directory symlinks before mkdir Co-Authored-By: Claude Opus 4.6 * test: namespace isolation tests for first install + mode switching Verifies the core invariant: when you pick a prefix mode, ONLY that mode's entries exist. Zero pollution from the other mode. - first install --no-prefix: only flat names, zero gstack-* leaks - first install --prefix: only gstack-* names, zero flat leaks - non-TTY defaults to flat names - switching prefix→no-prefix removes ALL gstack-* entries - switching no-prefix→prefix removes ALL flat entries Co-Authored-By: Claude Opus 4.6 * feat: upgrade migration system — versioned fix scripts for broken state Adds gstack-upgrade/migrations/ directory with version-keyed bash scripts that run automatically during /gstack-upgrade (Step 4.75, after ./setup). Each script is idempotent and handles state fixes that setup alone can't cover. First migration: v0.15.2.0.sh runs gstack-relink to fix stale directory symlinks from pre-v0.15.2.0 installs. Co-Authored-By: Claude Opus 4.6 * test: migration script validation + v0.15.2.0 end-to-end fix test Tests that migration scripts are executable, parse without syntax errors, follow the v{VERSION}.sh naming convention, and that v0.15.2.0 actually fixes stale directory symlinks by converting them to real directories. Co-Authored-By: Claude Opus 4.6 * docs: upgrade migration guide in CONTRIBUTING.md + CLAUDE.md pointer CONTRIBUTING.md: new "Upgrade migrations" section documenting when and how to add migration scripts for broken on-disk state. CLAUDE.md: added note under vendored symlink awareness pointing to CONTRIBUTING.md's migration section when worried about broken installs. Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- CHANGELOG.md | 4 + CLAUDE.md | 18 +- CONTRIBUTING.md | 72 +++++++- bin/gstack-relink | 26 ++- gstack-upgrade/SKILL.md | 26 +++ gstack-upgrade/SKILL.md.tmpl | 26 +++ gstack-upgrade/migrations/v0.15.2.0.sh | 20 +++ package.json | 2 +- setup | 55 ++++-- test/gen-skill-docs.test.ts | 38 ++++- test/relink.test.ts | 227 +++++++++++++++++++++++++ 11 files changed, 474 insertions(+), 40 deletions(-) create mode 100755 gstack-upgrade/migrations/v0.15.2.0.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 27d2c0c6c5e7019efd1e97b9c418e8d51654db82..387c6d419cf1c75e220e64b8576fe236fd0984f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ You can now run `/design-html` without having to run `/design-shotgun` first. Th - **`/design-html` works from any starting point.** Three routing modes: (A) approved mockup from /design-shotgun, (B) CEO plan and/or design variants without formal approval, (C) clean slate with just a description. Each mode asks the right questions and proceeds accordingly. - **AskUserQuestion for missing context.** Instead of blocking with "no approved design found," the skill now offers choices: run the planning skills first, provide a PNG, or just describe what you want and design live. +### Fixed + +- **Skills now discovered as top-level names.** Setup creates real directories with SKILL.md symlinks inside instead of directory symlinks. This fixes Claude auto-prefixing skill names with `gstack-` when using `--no-prefix` mode. `/qa` is now just `/qa`, not `/gstack-qa`. + ## [0.15.0.0] - 2026-04-01 — Session Intelligence Your AI sessions now remember what happened. Plans, reviews, checkpoints, and health scores survive context compaction and compound across sessions. Every skill writes a timeline event, and the preamble reads recent artifacts on startup so the agent knows where you left off. diff --git a/CLAUDE.md b/CLAUDE.md index 362b8f3274d96028571a2115797c2d1037fcd3d0..be06d22f1c71194f1343b0ccb9462d643f68aec2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -181,16 +181,24 @@ symlink or a real copy. If it's a symlink to your working directory, be aware th - During large refactors, remove the symlink (`rm .claude/skills/gstack`) so the global install at `~/.claude/skills/gstack/` is used instead -**Prefix setting:** Skill symlinks use either short names (`qa -> gstack/qa`) or -namespaced (`gstack-qa -> gstack/qa`), controlled by `skill_prefix` in -`~/.gstack/config.yaml`. When vendoring into a project, run `./setup` after -symlinking to create the per-skill symlinks with your preferred naming. Pass -`--no-prefix` or `--prefix` to skip the interactive prompt. +**Prefix setting:** Setup creates real directories (not symlinks) at the top level +with a SKILL.md symlink inside (e.g., `qa/SKILL.md -> gstack/qa/SKILL.md`). This +ensures Claude discovers them as top-level skills, not nested under `gstack/`. +Names are either short (`qa`) or namespaced (`gstack-qa`), controlled by +`skill_prefix` in `~/.gstack/config.yaml`. When vendoring into a project, run +`./setup` after symlinking to create the per-skill directories. Pass `--no-prefix` +or `--prefix` to skip the interactive prompt. **For plan reviews:** When reviewing plans that modify skill templates or the gen-skill-docs pipeline, consider whether the changes should be tested in isolation before going live (especially if the user is actively using gstack in other windows). +**Upgrade migrations:** When a change modifies on-disk state (directory structure, +config format, stale files) in ways that could break existing user installs, add a +migration script to `gstack-upgrade/migrations/`. Read CONTRIBUTING.md's "Upgrade +migrations" section for the format and testing requirements. The upgrade skill runs +these automatically after `./setup` during `/gstack-upgrade`. + ## Compiled binaries — NEVER commit browse/dist/ or design/dist/ The `browse/dist/` and `design/dist/` directories contain compiled Bun binaries diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fcb9c279aa7ad208c523099b1f1622cb9e177d6b..a7b639ee7dd369f008a3da4e5ee8a8201771a31e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -40,8 +40,8 @@ No setup needed. Learnings are logged automatically. View them with `/learn`. ln -sfn /path/to/your/gstack-fork .claude/skills/gstack cd .claude/skills/gstack && bun install && bun run build && ./setup ``` - Setup creates the per-skill symlinks (`qa -> gstack/qa`, etc.) and asks your - prefix preference. Pass `--no-prefix` to skip the prompt and use short names. + Setup creates per-skill directories with SKILL.md symlinks inside (`qa/SKILL.md -> gstack/qa/SKILL.md`) + and asks your prefix preference. Pass `--no-prefix` to skip the prompt and use short names. 5. **Fix the issue** — your changes are live immediately in this project 6. **Test by actually using gstack** — do the thing that annoyed you, verify it's fixed 7. **Open a PR from your fork** @@ -64,9 +64,11 @@ your local edits instead of the global install. gstack/ <- your working tree ├── .claude/skills/ <- created by dev-setup (gitignored) │ ├── gstack -> ../../ <- symlink back to repo root -│ ├── review -> gstack/review <- short names (default) -│ ├── ship -> gstack/ship <- or gstack-review, gstack-ship if --prefix -│ └── ... <- one symlink per skill +│ ├── review/ <- real directory (short name, default) +│ │ └── SKILL.md -> gstack/review/SKILL.md +│ ├── ship/ <- or gstack-review/, gstack-ship/ if --prefix +│ │ └── SKILL.md -> gstack/ship/SKILL.md +│ └── ... <- one directory per skill ├── review/ │ └── SKILL.md <- edit this, test with /review ├── ship/ @@ -77,7 +79,9 @@ gstack/ <- your working tree └── ... ``` -Skill symlink names depend on your prefix setting (`~/.gstack/config.yaml`). +Setup creates real directories (not symlinks) at the top level with a SKILL.md +symlink inside. This ensures Claude discovers them as top-level skills, not nested +under `gstack/`. Names depend on your prefix setting (`~/.gstack/config.yaml`). Short names (`/review`, `/ship`) are the default. Run `./setup --prefix` if you prefer namespaced names (`/gstack-review`, `/gstack-ship`). @@ -320,7 +324,7 @@ ln -sfn /path/to/your/gstack-checkout .claude/skills/gstack ### Step 2: Run setup to create per-skill symlinks The `gstack` symlink alone isn't enough. Claude Code discovers skills through -individual symlinks (`qa -> gstack/qa`, `ship -> gstack/ship`, etc.), not through +individual top-level directories (`qa/SKILL.md`, `ship/SKILL.md`, etc.), not through the `gstack/` directory itself. Run `./setup` to create them: ```bash @@ -344,8 +348,8 @@ Remove the project-local symlink. Claude Code falls back to `~/.claude/skills/gs rm .claude/skills/gstack ``` -The per-skill symlinks (`qa`, `ship`, etc.) still point to `gstack/...`, so they'll -resolve to the global install automatically. +The per-skill directories (`qa/`, `ship/`, etc.) contain SKILL.md symlinks that point +to `gstack/...`, so they'll resolve to the global install automatically. ### Switching prefix mode @@ -388,6 +392,56 @@ When community PRs accumulate, batch them into themed waves: See [PR #205](../../pull/205) (v0.8.3) for the first wave as an example. +## Upgrade migrations + +When a release changes on-disk state (directory structure, config format, stale +files) in ways that `./setup` alone can't fix, add a migration script so existing +users get a clean upgrade. + +### When to add a migration + +- Changed how skill directories are created (symlinks vs real dirs) +- Renamed or moved config keys in `~/.gstack/config.yaml` +- Need to delete orphaned files from a previous version +- Changed the format of `~/.gstack/` state files + +Don't add a migration for: new features (users get them automatically), new +skills (setup discovers them), or code-only changes (no on-disk state). + +### How to add one + +1. Create `gstack-upgrade/migrations/v{VERSION}.sh` where `{VERSION}` matches + the VERSION file for the release that needs the fix. +2. Make it executable: `chmod +x gstack-upgrade/migrations/v{VERSION}.sh` +3. The script must be **idempotent** (safe to run multiple times) and + **non-fatal** (failures are logged but don't block the upgrade). +4. Include a comment block at the top explaining what changed, why the + migration is needed, and which users are affected. + +Example: + +```bash +#!/usr/bin/env bash +# Migration: v0.15.2.0 — Fix skill directory structure +# Affected: users who installed with --no-prefix before v0.15.2.0 +set -euo pipefail +SCRIPT_DIR="$(cd "$(dirname "$0")/../.." && pwd)" +"$SCRIPT_DIR/bin/gstack-relink" 2>/dev/null || true +``` + +### How it runs + +During `/gstack-upgrade`, after `./setup` completes (Step 4.75), the upgrade +skill scans `gstack-upgrade/migrations/` and runs every `v*.sh` script whose +version is newer than the user's old version. Scripts run in version order. +Failures are logged but never block the upgrade. + +### Testing migrations + +Migrations are tested as part of `bun test` (tier 1, free). The test suite +verifies that all migration scripts in `gstack-upgrade/migrations/` are +executable and parse without syntax errors. + ## Shipping your changes When you're happy with your skill edits: diff --git a/bin/gstack-relink b/bin/gstack-relink index 4647f6df30f18298a3f00ed10ae6206a9ab6ea09..31e6b82f068bd16a2a1f36402e49efa5e78845e0 100755 --- a/bin/gstack-relink +++ b/bin/gstack-relink @@ -36,6 +36,16 @@ SKILLS_DIR="${GSTACK_SKILLS_DIR:-$(dirname "$INSTALL_DIR")}" # Read prefix setting PREFIX=$("$GSTACK_CONFIG" get skill_prefix 2>/dev/null || echo "false") +# Helper: remove old skill entry (symlink or real directory with symlinked SKILL.md) +_cleanup_skill_entry() { + local entry="$1" + if [ -L "$entry" ]; then + rm -f "$entry" + elif [ -d "$entry" ] && [ -L "$entry/SKILL.md" ]; then + rm -rf "$entry" + fi +} + # Discover skills (directories with SKILL.md, excluding meta dirs) SKILL_COUNT=0 for skill_dir in "$INSTALL_DIR"/*/; do @@ -51,18 +61,22 @@ for skill_dir in "$INSTALL_DIR"/*/; do gstack-*) link_name="$skill" ;; *) link_name="gstack-$skill" ;; esac - ln -sfn "$INSTALL_DIR/$skill" "$SKILLS_DIR/$link_name" - # Remove old flat symlink if it exists (and isn't the same as the new link) - [ "$link_name" != "$skill" ] && [ -L "$SKILLS_DIR/$skill" ] && rm -f "$SKILLS_DIR/$skill" + # Remove old flat entry if it exists (and isn't the same as the new link) + [ "$link_name" != "$skill" ] && _cleanup_skill_entry "$SKILLS_DIR/$skill" else - # Create flat symlink, remove gstack-* if exists - ln -sfn "$INSTALL_DIR/$skill" "$SKILLS_DIR/$skill" + link_name="$skill" # Don't remove gstack-* dirs that are their real name (e.g., gstack-upgrade) case "$skill" in gstack-*) ;; # Already the real name, no old prefixed link to clean - *) [ -L "$SKILLS_DIR/gstack-$skill" ] && rm -f "$SKILLS_DIR/gstack-$skill" ;; + *) _cleanup_skill_entry "$SKILLS_DIR/gstack-$skill" ;; esac fi + target="$SKILLS_DIR/$link_name" + # Upgrade old directory symlinks to real directories + [ -L "$target" ] && rm -f "$target" + # Create real directory with symlinked SKILL.md (absolute path) + mkdir -p "$target" + ln -snf "$INSTALL_DIR/$skill/SKILL.md" "$target/SKILL.md" SKILL_COUNT=$((SKILL_COUNT + 1)) done diff --git a/gstack-upgrade/SKILL.md b/gstack-upgrade/SKILL.md index f97f11fb743305edf9f4c9c95e28e78886990b57..d357e55212e04a3311c9f770207d64618f4f963b 100644 --- a/gstack-upgrade/SKILL.md +++ b/gstack-upgrade/SKILL.md @@ -170,6 +170,32 @@ mv "$LOCAL_GSTACK.bak" "$LOCAL_GSTACK" ``` Tell user: "Sync failed — restored previous version at `$LOCAL_GSTACK`. Run `/gstack-upgrade` manually to retry." +### Step 4.75: Run version migrations + +After `./setup` completes, run any migration scripts for versions between the old +and new version. Migrations handle state fixes that `./setup` alone can't cover +(stale config, orphaned files, directory structure changes). + +```bash +MIGRATIONS_DIR="$INSTALL_DIR/gstack-upgrade/migrations" +if [ -d "$MIGRATIONS_DIR" ]; then + for migration in $(find "$MIGRATIONS_DIR" -maxdepth 1 -name 'v*.sh' -type f 2>/dev/null | sort -V); do + # Extract version from filename: v0.15.2.0.sh → 0.15.2.0 + m_ver="$(basename "$migration" .sh | sed 's/^v//')" + # Run if this migration version is newer than old version + # (simple string compare works for dotted versions with same segment count) + if [ "$OLD_VERSION" != "unknown" ] && [ "$(printf '%s\n%s' "$OLD_VERSION" "$m_ver" | sort -V | head -1)" = "$OLD_VERSION" ] && [ "$OLD_VERSION" != "$m_ver" ]; then + echo "Running migration $m_ver..." + bash "$migration" || echo " Warning: migration $m_ver had errors (non-fatal)" + fi + done +fi +``` + +Migrations are idempotent bash scripts in `gstack-upgrade/migrations/`. Each is named +`v{VERSION}.sh` and runs only when upgrading from an older version. See CONTRIBUTING.md +for how to add new migrations. + ### Step 5: Write marker + clear cache ```bash diff --git a/gstack-upgrade/SKILL.md.tmpl b/gstack-upgrade/SKILL.md.tmpl index ac25894b3489c02d207b7a1b61b79067d4755c81..7204c75bfd645b2547ccc30838999e1860063de8 100644 --- a/gstack-upgrade/SKILL.md.tmpl +++ b/gstack-upgrade/SKILL.md.tmpl @@ -168,6 +168,32 @@ mv "$LOCAL_GSTACK.bak" "$LOCAL_GSTACK" ``` Tell user: "Sync failed — restored previous version at `$LOCAL_GSTACK`. Run `/gstack-upgrade` manually to retry." +### Step 4.75: Run version migrations + +After `./setup` completes, run any migration scripts for versions between the old +and new version. Migrations handle state fixes that `./setup` alone can't cover +(stale config, orphaned files, directory structure changes). + +```bash +MIGRATIONS_DIR="$INSTALL_DIR/gstack-upgrade/migrations" +if [ -d "$MIGRATIONS_DIR" ]; then + for migration in $(find "$MIGRATIONS_DIR" -maxdepth 1 -name 'v*.sh' -type f 2>/dev/null | sort -V); do + # Extract version from filename: v0.15.2.0.sh → 0.15.2.0 + m_ver="$(basename "$migration" .sh | sed 's/^v//')" + # Run if this migration version is newer than old version + # (simple string compare works for dotted versions with same segment count) + if [ "$OLD_VERSION" != "unknown" ] && [ "$(printf '%s\n%s' "$OLD_VERSION" "$m_ver" | sort -V | head -1)" = "$OLD_VERSION" ] && [ "$OLD_VERSION" != "$m_ver" ]; then + echo "Running migration $m_ver..." + bash "$migration" || echo " Warning: migration $m_ver had errors (non-fatal)" + fi + done +fi +``` + +Migrations are idempotent bash scripts in `gstack-upgrade/migrations/`. Each is named +`v{VERSION}.sh` and runs only when upgrading from an older version. See CONTRIBUTING.md +for how to add new migrations. + ### Step 5: Write marker + clear cache ```bash diff --git a/gstack-upgrade/migrations/v0.15.2.0.sh b/gstack-upgrade/migrations/v0.15.2.0.sh new file mode 100755 index 0000000000000000000000000000000000000000..ebee442e09ab01638dd8b83e9ad9e554d0f34df5 --- /dev/null +++ b/gstack-upgrade/migrations/v0.15.2.0.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash +# Migration: v0.15.2.0 — Fix skill directory structure for unprefixed discovery +# +# What changed: setup now creates real directories with SKILL.md symlinks +# inside instead of directory symlinks. The old pattern (qa -> gstack/qa) +# caused Claude Code to auto-prefix skills as "gstack-qa" even with +# --no-prefix, because Claude sees the symlink target's parent dir name. +# +# What this does: runs gstack-relink to recreate all skill entries using +# the new real-directory pattern. Idempotent — safe to run multiple times. +# +# Affected: users who installed gstack before v0.15.2.0 with --no-prefix +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")/../.." && pwd)" + +if [ -x "$SCRIPT_DIR/bin/gstack-relink" ]; then + echo " [v0.15.2.0] Fixing skill directory structure..." + "$SCRIPT_DIR/bin/gstack-relink" 2>/dev/null || true +fi diff --git a/package.json b/package.json index 50ec09145f25640c26951e6edd3d8d32f27cf029..af7d165a1f0d09c1cb731c2b5a33ca25129b4635 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "0.15.0.0", + "version": "0.15.1.0", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/setup b/setup index 91f0c9e73e500c3de7158caf9e6a6c8f52acbd5d..2fdd2892352f3bd4c448920e33f8e9ff6514d9dd 100755 --- a/setup +++ b/setup @@ -263,9 +263,11 @@ fi mkdir -p "$HOME/.gstack/projects" # ─── Helper: link Claude skill subdirectories into a skills parent directory ── -# When SKILL_PREFIX=1 (default), symlinks are prefixed with "gstack-" to avoid -# namespace pollution (e.g., gstack-review instead of review). -# Use --no-prefix to restore the old flat names. +# Creates real directories (not symlinks) at the top level with a SKILL.md symlink +# inside. This ensures Claude discovers them as top-level skills, not nested under +# gstack/ (which would auto-prefix them as gstack-*). +# When SKILL_PREFIX=1, directories are prefixed with "gstack-". +# Use --no-prefix to restore flat names. link_claude_skill_dirs() { local gstack_dir="$1" local skills_dir="$2" @@ -288,9 +290,14 @@ link_claude_skill_dirs() { link_name="$skill_name" fi target="$skills_dir/$link_name" - # Create or update symlink; skip if a real file/directory exists - if [ -L "$target" ] || [ ! -e "$target" ]; then - ln -snf "gstack/$dir_name" "$target" + # Upgrade old directory symlinks to real directories + if [ -L "$target" ]; then + rm -f "$target" + fi + # Create real directory with symlinked SKILL.md (absolute path) + if [ ! -e "$target" ] || [ -d "$target" ]; then + mkdir -p "$target" + ln -snf "$gstack_dir/$dir_name/SKILL.md" "$target/SKILL.md" linked+=("$link_name") fi fi @@ -300,9 +307,9 @@ link_claude_skill_dirs() { fi } -# ─── Helper: remove old unprefixed Claude skill symlinks ────────────────────── +# ─── Helper: remove old unprefixed Claude skill entries ─────────────────────── # Migration: when switching from flat names to gstack- prefixed names, -# clean up stale symlinks that point into the gstack directory. +# clean up stale symlinks or directories that point into the gstack directory. cleanup_old_claude_symlinks() { local gstack_dir="$1" local skills_dir="$2" @@ -314,7 +321,7 @@ cleanup_old_claude_symlinks() { # Skip already-prefixed dirs (gstack-upgrade) — no old symlink to clean case "$skill_name" in gstack-*) continue ;; esac old_target="$skills_dir/$skill_name" - # Only remove if it's a symlink pointing into gstack/ + # Remove directory symlinks pointing into gstack/ if [ -L "$old_target" ]; then link_dest="$(readlink "$old_target" 2>/dev/null || true)" case "$link_dest" in @@ -323,17 +330,26 @@ cleanup_old_claude_symlinks() { removed+=("$skill_name") ;; esac + # Remove real directories with symlinked SKILL.md pointing into gstack/ + elif [ -d "$old_target" ] && [ -L "$old_target/SKILL.md" ]; then + link_dest="$(readlink "$old_target/SKILL.md" 2>/dev/null || true)" + case "$link_dest" in + *gstack*) + rm -rf "$old_target" + removed+=("$skill_name") + ;; + esac fi fi done if [ ${#removed[@]} -gt 0 ]; then - echo " cleaned up old symlinks: ${removed[*]}" + echo " cleaned up old entries: ${removed[*]}" fi } -# ─── Helper: remove old prefixed Claude skill symlinks ──────────────────────── +# ─── Helper: remove old prefixed Claude skill entries ───────────────────────── # Reverse migration: when switching from gstack- prefixed names to flat names, -# clean up stale gstack-* symlinks that point into the gstack directory. +# clean up stale gstack-* symlinks or directories that point into the gstack directory. cleanup_prefixed_claude_symlinks() { local gstack_dir="$1" local skills_dir="$2" @@ -342,11 +358,11 @@ cleanup_prefixed_claude_symlinks() { if [ -f "$skill_dir/SKILL.md" ]; then skill_name="$(basename "$skill_dir")" [ "$skill_name" = "node_modules" ] && continue - # Only clean up prefixed symlinks for dirs that AREN'T already prefixed + # Only clean up prefixed entries for dirs that AREN'T already prefixed # (e.g., remove gstack-qa but NOT gstack-upgrade which is the real dir name) case "$skill_name" in gstack-*) continue ;; esac prefixed_target="$skills_dir/gstack-$skill_name" - # Only remove if it's a symlink pointing into gstack/ + # Remove directory symlinks pointing into gstack/ if [ -L "$prefixed_target" ]; then link_dest="$(readlink "$prefixed_target" 2>/dev/null || true)" case "$link_dest" in @@ -355,11 +371,20 @@ cleanup_prefixed_claude_symlinks() { removed+=("gstack-$skill_name") ;; esac + # Remove real directories with symlinked SKILL.md pointing into gstack/ + elif [ -d "$prefixed_target" ] && [ -L "$prefixed_target/SKILL.md" ]; then + link_dest="$(readlink "$prefixed_target/SKILL.md" 2>/dev/null || true)" + case "$link_dest" in + *gstack*) + rm -rf "$prefixed_target" + removed+=("gstack-$skill_name") + ;; + esac fi fi done if [ ${#removed[@]} -gt 0 ]; then - echo " cleaned up prefixed symlinks: ${removed[*]}" + echo " cleaned up prefixed entries: ${removed[*]}" fi } diff --git a/test/gen-skill-docs.test.ts b/test/gen-skill-docs.test.ts index 4a25195defd44f4cadc5e71b57390a4e02074d09..b0a7538f3aaa463f230275ad169785b8416b9b01 100644 --- a/test/gen-skill-docs.test.ts +++ b/test/gen-skill-docs.test.ts @@ -1969,13 +1969,43 @@ describe('setup script validation', () => { expect(fnBody).toContain('gstack*'); }); - test('link_claude_skill_dirs creates relative symlinks', () => { - // Claude links should be relative: ln -snf "gstack/$dir_name" - // Uses dir_name (not skill_name) because symlink target must point to the physical directory + test('link_claude_skill_dirs creates real directories with absolute SKILL.md symlinks', () => { + // Claude links should be real directories with absolute SKILL.md symlinks + // to ensure Claude Code discovers them as top-level skills (not nested under gstack/) const fnStart = setupContent.indexOf('link_claude_skill_dirs()'); const fnEnd = setupContent.indexOf('}', setupContent.indexOf('linked[@]}', fnStart)); const fnBody = setupContent.slice(fnStart, fnEnd); - expect(fnBody).toContain('ln -snf "gstack/$dir_name"'); + expect(fnBody).toContain('mkdir -p "$target"'); + expect(fnBody).toContain('ln -snf "$gstack_dir/$dir_name/SKILL.md" "$target/SKILL.md"'); + }); + + // REGRESSION: cleanup functions must handle both old symlinks AND new real-directory pattern + test('cleanup functions handle real directories with symlinked SKILL.md', () => { + // cleanup_old_claude_symlinks must detect and remove real dirs with SKILL.md symlinks + const cleanupOldStart = setupContent.indexOf('cleanup_old_claude_symlinks()'); + const cleanupOldEnd = setupContent.indexOf('}', setupContent.indexOf('cleaned up old', cleanupOldStart)); + const cleanupOldBody = setupContent.slice(cleanupOldStart, cleanupOldEnd); + expect(cleanupOldBody).toContain('-d "$old_target"'); + expect(cleanupOldBody).toContain('-L "$old_target/SKILL.md"'); + expect(cleanupOldBody).toContain('rm -rf "$old_target"'); + + // cleanup_prefixed_claude_symlinks must also handle the new pattern + const cleanupPrefixedStart = setupContent.indexOf('cleanup_prefixed_claude_symlinks()'); + const cleanupPrefixedEnd = setupContent.indexOf('}', setupContent.indexOf('cleaned up prefixed', cleanupPrefixedStart)); + const cleanupPrefixedBody = setupContent.slice(cleanupPrefixedStart, cleanupPrefixedEnd); + expect(cleanupPrefixedBody).toContain('-d "$prefixed_target"'); + expect(cleanupPrefixedBody).toContain('-L "$prefixed_target/SKILL.md"'); + expect(cleanupPrefixedBody).toContain('rm -rf "$prefixed_target"'); + }); + + // REGRESSION: link function must upgrade old directory symlinks + test('link_claude_skill_dirs removes old directory symlinks before creating real dirs', () => { + const fnStart = setupContent.indexOf('link_claude_skill_dirs()'); + const fnEnd = setupContent.indexOf('}', setupContent.indexOf('linked[@]}', fnStart)); + const fnBody = setupContent.slice(fnStart, fnEnd); + // Must check for and remove old symlinks before mkdir + expect(fnBody).toContain('if [ -L "$target" ]'); + expect(fnBody).toContain('rm -f "$target"'); }); test('setup supports --host auto|claude|codex|kiro', () => { diff --git a/test/relink.test.ts b/test/relink.test.ts index b368d2bf3856a6922010c0429a901df2380d8965..70c069dfbf7e9dcc05d9ee13cfb0ebb6955694b3 100644 --- a/test/relink.test.ts +++ b/test/relink.test.ts @@ -97,6 +97,173 @@ describe('gstack-relink (#578)', () => { expect(output).toContain('flat'); }); + // REGRESSION: unprefixed skills must be real directories, not symlinks (#761) + // Claude Code auto-prefixes skills nested under a parent dir symlink. + // e.g., `qa -> gstack/qa` gets discovered as "gstack-qa", not "qa". + // The fix: create real directories with SKILL.md symlinks inside. + test('unprefixed skills are real directories with SKILL.md symlinks, not dir symlinks', () => { + setupMockInstall(['qa', 'ship', 'review', 'plan-ceo-review']); + run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix false`); + run(`${path.join(installDir, 'bin', 'gstack-relink')}`, { + GSTACK_INSTALL_DIR: installDir, + GSTACK_SKILLS_DIR: skillsDir, + }); + for (const skill of ['qa', 'ship', 'review', 'plan-ceo-review']) { + const skillPath = path.join(skillsDir, skill); + const skillMdPath = path.join(skillPath, 'SKILL.md'); + // Must be a real directory, NOT a symlink + expect(fs.lstatSync(skillPath).isDirectory()).toBe(true); + expect(fs.lstatSync(skillPath).isSymbolicLink()).toBe(false); + // Must contain a SKILL.md that IS a symlink + expect(fs.existsSync(skillMdPath)).toBe(true); + expect(fs.lstatSync(skillMdPath).isSymbolicLink()).toBe(true); + // The SKILL.md symlink must point to the source skill's SKILL.md + const target = fs.readlinkSync(skillMdPath); + expect(target).toContain(skill); + expect(target).toEndWith('/SKILL.md'); + } + }); + + // Same invariant for prefixed mode + test('prefixed skills are real directories with SKILL.md symlinks, not dir symlinks', () => { + setupMockInstall(['qa', 'ship']); + run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`); + run(`${path.join(installDir, 'bin', 'gstack-relink')}`, { + GSTACK_INSTALL_DIR: installDir, + GSTACK_SKILLS_DIR: skillsDir, + }); + for (const skill of ['gstack-qa', 'gstack-ship']) { + const skillPath = path.join(skillsDir, skill); + const skillMdPath = path.join(skillPath, 'SKILL.md'); + expect(fs.lstatSync(skillPath).isDirectory()).toBe(true); + expect(fs.lstatSync(skillPath).isSymbolicLink()).toBe(false); + expect(fs.lstatSync(skillMdPath).isSymbolicLink()).toBe(true); + } + }); + + // Upgrade: old directory symlinks get replaced with real directories + test('upgrades old directory symlinks to real directories', () => { + setupMockInstall(['qa', 'ship']); + // Simulate old behavior: create directory symlinks (the old pattern) + fs.symlinkSync(path.join(installDir, 'qa'), path.join(skillsDir, 'qa')); + fs.symlinkSync(path.join(installDir, 'ship'), path.join(skillsDir, 'ship')); + // Verify they start as symlinks + expect(fs.lstatSync(path.join(skillsDir, 'qa')).isSymbolicLink()).toBe(true); + + run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix false`); + run(`${path.join(installDir, 'bin', 'gstack-relink')}`, { + GSTACK_INSTALL_DIR: installDir, + GSTACK_SKILLS_DIR: skillsDir, + }); + + // After relink: must be real directories, not symlinks + expect(fs.lstatSync(path.join(skillsDir, 'qa')).isSymbolicLink()).toBe(false); + expect(fs.lstatSync(path.join(skillsDir, 'qa')).isDirectory()).toBe(true); + expect(fs.lstatSync(path.join(skillsDir, 'qa', 'SKILL.md')).isSymbolicLink()).toBe(true); + }); + + // FIRST INSTALL: --no-prefix must create ONLY flat names, zero gstack-* pollution + test('first install --no-prefix: only flat names exist, zero gstack-* entries', () => { + setupMockInstall(['qa', 'ship', 'review', 'plan-ceo-review', 'gstack-upgrade']); + // Simulate first install: no saved config, pass --no-prefix equivalent + run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix false`); + run(`${path.join(installDir, 'bin', 'gstack-relink')}`, { + GSTACK_INSTALL_DIR: installDir, + GSTACK_SKILLS_DIR: skillsDir, + }); + // Enumerate everything in skills dir + const entries = fs.readdirSync(skillsDir); + // Expected: qa, ship, review, plan-ceo-review, gstack-upgrade (its real name) + expect(entries.sort()).toEqual(['gstack-upgrade', 'plan-ceo-review', 'qa', 'review', 'ship']); + // No gstack-qa, gstack-ship, gstack-review, gstack-plan-ceo-review + const leaked = entries.filter(e => e.startsWith('gstack-') && e !== 'gstack-upgrade'); + expect(leaked).toEqual([]); + }); + + // FIRST INSTALL: --prefix must create ONLY gstack-* names, zero flat-name pollution + test('first install --prefix: only gstack-* entries exist, zero flat names', () => { + setupMockInstall(['qa', 'ship', 'review', 'plan-ceo-review', 'gstack-upgrade']); + run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`); + run(`${path.join(installDir, 'bin', 'gstack-relink')}`, { + GSTACK_INSTALL_DIR: installDir, + GSTACK_SKILLS_DIR: skillsDir, + }); + const entries = fs.readdirSync(skillsDir); + // Expected: gstack-qa, gstack-ship, gstack-review, gstack-plan-ceo-review, gstack-upgrade + expect(entries.sort()).toEqual([ + 'gstack-plan-ceo-review', 'gstack-qa', 'gstack-review', 'gstack-ship', 'gstack-upgrade', + ]); + // No unprefixed qa, ship, review, plan-ceo-review + const leaked = entries.filter(e => !e.startsWith('gstack-')); + expect(leaked).toEqual([]); + }); + + // FIRST INSTALL: non-TTY (no saved config, piped stdin) defaults to flat names + test('non-TTY first install defaults to flat names via relink', () => { + setupMockInstall(['qa', 'ship']); + // Don't set any config — simulate fresh install + // gstack-relink reads config; on fresh install config returns empty → defaults to false + run(`${path.join(installDir, 'bin', 'gstack-relink')}`, { + GSTACK_INSTALL_DIR: installDir, + GSTACK_SKILLS_DIR: skillsDir, + }); + const entries = fs.readdirSync(skillsDir); + // Should be flat names (relink defaults to false when config returns empty) + expect(entries.sort()).toEqual(['qa', 'ship']); + }); + + // SWITCH: prefix → no-prefix must clean up ALL gstack-* entries + test('switching prefix to no-prefix removes all gstack-* entries completely', () => { + setupMockInstall(['qa', 'ship', 'review', 'plan-ceo-review', 'gstack-upgrade']); + // Start in prefix mode + run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`); + run(`${path.join(installDir, 'bin', 'gstack-relink')}`, { + GSTACK_INSTALL_DIR: installDir, + GSTACK_SKILLS_DIR: skillsDir, + }); + let entries = fs.readdirSync(skillsDir); + expect(entries.filter(e => !e.startsWith('gstack-'))).toEqual([]); + + // Switch to no-prefix + run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix false`); + run(`${path.join(installDir, 'bin', 'gstack-relink')}`, { + GSTACK_INSTALL_DIR: installDir, + GSTACK_SKILLS_DIR: skillsDir, + }); + entries = fs.readdirSync(skillsDir); + // Only flat names + gstack-upgrade (its real name) + expect(entries.sort()).toEqual(['gstack-upgrade', 'plan-ceo-review', 'qa', 'review', 'ship']); + const leaked = entries.filter(e => e.startsWith('gstack-') && e !== 'gstack-upgrade'); + expect(leaked).toEqual([]); + }); + + // SWITCH: no-prefix → prefix must clean up ALL flat entries + test('switching no-prefix to prefix removes all flat entries completely', () => { + setupMockInstall(['qa', 'ship', 'review', 'gstack-upgrade']); + // Start in no-prefix mode + run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix false`); + run(`${path.join(installDir, 'bin', 'gstack-relink')}`, { + GSTACK_INSTALL_DIR: installDir, + GSTACK_SKILLS_DIR: skillsDir, + }); + let entries = fs.readdirSync(skillsDir); + expect(entries.filter(e => e.startsWith('gstack-') && e !== 'gstack-upgrade')).toEqual([]); + + // Switch to prefix + run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`); + run(`${path.join(installDir, 'bin', 'gstack-relink')}`, { + GSTACK_INSTALL_DIR: installDir, + GSTACK_SKILLS_DIR: skillsDir, + }); + entries = fs.readdirSync(skillsDir); + // Only gstack-* names + expect(entries.sort()).toEqual([ + 'gstack-qa', 'gstack-review', 'gstack-ship', 'gstack-upgrade', + ]); + const leaked = entries.filter(e => !e.startsWith('gstack-')); + expect(leaked).toEqual([]); + }); + // Test 13: cleans stale symlinks from opposite mode test('cleans up stale symlinks from opposite mode', () => { setupMockInstall(['qa', 'ship']); @@ -158,6 +325,66 @@ describe('gstack-relink (#578)', () => { }); }); +describe('upgrade migrations', () => { + const MIGRATIONS_DIR = path.join(ROOT, 'gstack-upgrade', 'migrations'); + + test('migrations directory exists', () => { + expect(fs.existsSync(MIGRATIONS_DIR)).toBe(true); + }); + + test('all migration scripts are executable and parse without syntax errors', () => { + const scripts = fs.readdirSync(MIGRATIONS_DIR).filter(f => f.endsWith('.sh')); + expect(scripts.length).toBeGreaterThan(0); + for (const script of scripts) { + const fullPath = path.join(MIGRATIONS_DIR, script); + // Must be executable + const stat = fs.statSync(fullPath); + expect(stat.mode & 0o111).toBeGreaterThan(0); + // Must parse without syntax errors (bash -n is a syntax check, doesn't execute) + const result = execSync(`bash -n "${fullPath}" 2>&1`, { encoding: 'utf-8', timeout: 5000 }); + // bash -n outputs nothing on success + } + }); + + test('migration filenames follow v{VERSION}.sh pattern', () => { + const scripts = fs.readdirSync(MIGRATIONS_DIR).filter(f => f.endsWith('.sh')); + for (const script of scripts) { + expect(script).toMatch(/^v\d+\.\d+\.\d+\.\d+\.sh$/); + } + }); + + test('v0.15.2.0 migration runs gstack-relink', () => { + const content = fs.readFileSync(path.join(MIGRATIONS_DIR, 'v0.15.2.0.sh'), 'utf-8'); + expect(content).toContain('gstack-relink'); + }); + + test('v0.15.2.0 migration fixes stale directory symlinks', () => { + setupMockInstall(['qa', 'ship', 'review']); + // Simulate old state: directory symlinks (pre-v0.15.2.0 pattern) + fs.symlinkSync(path.join(installDir, 'qa'), path.join(skillsDir, 'qa')); + fs.symlinkSync(path.join(installDir, 'ship'), path.join(skillsDir, 'ship')); + fs.symlinkSync(path.join(installDir, 'review'), path.join(skillsDir, 'review')); + // Set no-prefix mode + run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix false`); + // Verify old state: symlinks + expect(fs.lstatSync(path.join(skillsDir, 'qa')).isSymbolicLink()).toBe(true); + + // Run the migration (it calls gstack-relink internally) + run(`bash ${path.join(MIGRATIONS_DIR, 'v0.15.2.0.sh')}`, { + GSTACK_INSTALL_DIR: installDir, + GSTACK_SKILLS_DIR: skillsDir, + }); + + // After migration: real directories with SKILL.md symlinks + for (const skill of ['qa', 'ship', 'review']) { + const skillPath = path.join(skillsDir, skill); + expect(fs.lstatSync(skillPath).isSymbolicLink()).toBe(false); + expect(fs.lstatSync(skillPath).isDirectory()).toBe(true); + expect(fs.lstatSync(path.join(skillPath, 'SKILL.md')).isSymbolicLink()).toBe(true); + } + }); +}); + describe('gstack-patch-names (#620/#578)', () => { // Helper to read name: from SKILL.md frontmatter function readSkillName(skillDir: string): string | null {