From dc6252d1df7f1f650ea6e9b2bba7d08fab5de902 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 11 May 2026 20:35:58 -0700 Subject: [PATCH] v1.33.2.0 fix: setup guards against Conductor worktree pollution of global install (#1446) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(setup): skip Claude skill registration when run from a worktree of the global install Add a guard before `ln -snf "$SOURCE_GSTACK_DIR" "$HOME/.claude/skills/gstack"` that detects whether the target already exists as a separate real directory. On macOS/BSD, `ln -snf SRC DST` does not replace a real DST — it creates DST/$(basename SRC) → SRC inside it. Running ./setup from each Conductor worktree of the gstack repo was leaking per-worktree child symlinks into the global install, which Claude Code then picked up as separate top-level skills. The guard uses `cd ... && pwd -P` to resolve the existing real dir and compare against the source (mirroring setup's own `SOURCE_GSTACK_DIR` resolution). When they differ, prints a four-line remediation hint naming both paths and exits the Claude registration branch cleanly. Binaries still build locally. The four other code paths through this branch are unchanged: fresh install, retarget an existing symlink, self-rerun where the existing dir resolves to the same source, and --local installs. Includes 8 tests covering static guard placement, `pwd -P` resolution, the remediation message, a behavioral reproduction of the BSD `ln -snf` child- symlink bug, and every branch of the guard (skip on real-dir-elsewhere, allow on fresh, allow on existing symlink, allow on self-rerun). * chore: bump version and changelog (v1.33.2.0) Co-Authored-By: Claude Opus 4.7 --------- Co-authored-by: Claude Opus 4.7 --- CHANGELOG.md | 42 ++++++ VERSION | 2 +- package.json | 2 +- setup | 83 +++++++---- test/setup-conductor-worktree.test.ts | 200 ++++++++++++++++++++++++++ 5 files changed, 302 insertions(+), 27 deletions(-) create mode 100644 test/setup-conductor-worktree.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 86c066df9..937e67e37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,47 @@ # Changelog +## [1.33.2.0] - 2026-05-11 + +## **`./setup` no longer pollutes the global install when run from a Conductor worktree.** +## **Six-line bash guard catches the BSD `ln -snf` footgun that was leaking per-worktree symlinks into `~/.claude/skills/gstack/`.** + +When you ran `./setup` from a Conductor worktree of the gstack repo itself (e.g. `~/conductor/workspaces/gstack/dublin-v1`), it would silently corrupt your global install. The "register this checkout as the active gstack" branch did `ln -snf "$SOURCE_GSTACK_DIR" "$HOME/.claude/skills/gstack"`. On macOS and BSD, when the destination is an existing real directory (your global git clone), `ln -snf` does NOT replace it. It creates a child symlink INSIDE: `~/.claude/skills/gstack/dublin-v1 → ~/conductor/workspaces/gstack/dublin-v1`. Claude Code reads every directory in `~/.claude/skills/` that contains a `SKILL.md`, so each leaked worktree showed up as its own top-level skill: `/dublin-v1`, `/wellington`, `/santiago-v1`, etc. The skill picker filled with noise. + +The fix in `setup` checks whether `~/.claude/skills/gstack` is already a real (non-symlink) directory whose resolved `pwd -P` differs from `$SOURCE_GSTACK_DIR`. If so, refuse the `ln -snf`, print a four-line remediation hint, and exit the Claude registration branch cleanly. Binaries (`browse`, `design`, `make-pdf`, `find-browse`) still build locally for dev. The four other code paths through the same branch (fresh install, retarget existing symlink, self-rerun pointing to the same dir, `--local`) are unchanged. + +### The numbers that matter + +Source: `bun test test/setup-conductor-worktree.test.ts` — 8 tests covering every branch of the new guard plus a behavioral reproduction of the BSD `ln -snf` bug itself. + +| Scenario | Before | After | +|---|---|---| +| `./setup` from worktree A with global install present | Leaks `~/.claude/skills/gstack/A → workspaces/gstack/A` | Skipped with remediation hint | +| `./setup` from N sibling worktrees over a week | N child symlinks accumulate inside global install | 0 leaks | +| Claude Code skill picker shows extra entries | Yes: `dublin-v1`, `wellington`, `santiago-v1`, etc. | No | +| Fresh install (no existing global) | Worked | Worked (unchanged path) | +| Re-running `./setup` from inside the global install | Worked | Worked (unchanged path) | +| Test coverage of the guard | 0 tests | 8 tests, all branches | + +The behavioral test in `test/setup-conductor-worktree.test.ts` actually invokes `ln -snf SRC DST` against a real tmpdir to prove the macOS/BSD child-symlink behavior happens, then re-runs with the new guard to prove the leak doesn't. The bug is now documented in the test suite, not just the patch. + +### What this means for builders + +If you've been seeing extra top-level skills (`/dublin-v1`, `/wellington`, etc.) in Claude Code, that's the leak. Run `/gstack-upgrade` to pick up this fix, then manually remove the existing child symlinks: `cd ~/.claude/skills/gstack && find . -maxdepth 1 -type l -delete`. The guard prevents new leaks from `./setup` runs in any Conductor worktree of the gstack repo. If you actually want to register a worktree as the active gstack (rare, usually only when dogfooding a big in-progress change), remove the global install first: `rm -rf ~/.claude/skills/gstack && cd && ./setup`. + +### Itemized changes + +#### Fixed + +- **`setup`** — added Conductor worktree guard before `ln -snf "$SOURCE_GSTACK_DIR" "$CLAUDE_GSTACK_LINK"`. Checks `[ -d "$CLAUDE_GSTACK_LINK" ] && [ ! -L "$CLAUDE_GSTACK_LINK" ]` for a real directory, then `cd ... && pwd -P` to compare against the source. If they differ, sets `_SKIP_CLAUDE_REGISTER=1`, prints a remediation message naming both paths, and exits the Claude registration branch without touching the global install. + +#### Added + +- **`test/setup-conductor-worktree.test.ts`** — 8 tests (27 expect calls) covering: guard placement in `setup` before `ln -snf`, `pwd -P` resolution against `$SOURCE_GSTACK_DIR`, the skip-branch's remediation message, BSD `ln -snf` reproducer (proves the bug shape exists), guard skips when dest is real-dir-elsewhere, guard allows ln when dest doesn't exist, guard allows ln when dest is an existing symlink (upgrade-in-place), guard allows ln when dest already resolves to source (self-rerun). + +#### For contributors + +- The guard intentionally does NOT clean up pre-existing pollution inside `~/.claude/skills/gstack/`. Users must remove leaked symlinks manually (see "What this means for builders" above). Retroactive cleanup would require a separate migration script, filed for a future release if the manual remediation friction becomes noticeable. + ## [1.33.1.0] - 2026-05-11 ## **Long skills stop drifting away from their starting context.** diff --git a/VERSION b/VERSION index bb92f340d..0df2c524d 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.33.1.0 +1.33.2.0 diff --git a/package.json b/package.json index 87c4c7992..d4512f5e7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.33.1.0", + "version": "1.33.2.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 4c1763f9f..f812511e4 100755 --- a/setup +++ b/setup @@ -806,35 +806,68 @@ if [ "$INSTALL_CLAUDE" -eq 1 ]; then fi log " browse: $BROWSE_BIN" else - # Not inside a skills/ directory — symlink into ~/.claude/skills/ and retry + # Not inside a skills/ directory — would symlink the source into + # ~/.claude/skills/gstack/ and register from there. CLAUDE_SKILLS_DIR="$HOME/.claude/skills" CLAUDE_GSTACK_LINK="$CLAUDE_SKILLS_DIR/gstack" - mkdir -p "$CLAUDE_SKILLS_DIR" - ln -snf "$SOURCE_GSTACK_DIR" "$CLAUDE_GSTACK_LINK" - log " symlinked $CLAUDE_GSTACK_LINK -> $SOURCE_GSTACK_DIR" - INSTALL_SKILLS_DIR="$CLAUDE_SKILLS_DIR" - INSTALL_GSTACK_DIR="$CLAUDE_GSTACK_LINK" - # Clean up stale symlinks from the opposite prefix mode - if [ "$SKILL_PREFIX" -eq 1 ]; then - cleanup_old_claude_symlinks "$SOURCE_GSTACK_DIR" "$INSTALL_SKILLS_DIR" + + # Conductor worktree guard: if ~/.claude/skills/gstack is already a real + # (non-symlink) directory pointing to a *different* install, refuse to plant + # a symlink there. On macOS/BSD, `ln -snf SRC DST` won't replace a real DST; + # it creates DST/$(basename SRC) → SRC inside it. The result is per-worktree + # symlinks leaking into the global install that Claude Code picks up as + # separate top-level skills (dublin-v1, lincoln-v2, ...). Typical trigger: + # running ./setup from a Conductor worktree of the gstack repo itself. + _SKIP_CLAUDE_REGISTER=0 + if [ -d "$CLAUDE_GSTACK_LINK" ] && [ ! -L "$CLAUDE_GSTACK_LINK" ]; then + _EXISTING_REAL=$(cd "$CLAUDE_GSTACK_LINK" 2>/dev/null && pwd -P || echo "") + if [ -n "$_EXISTING_REAL" ] && [ "$_EXISTING_REAL" != "$SOURCE_GSTACK_DIR" ]; then + _SKIP_CLAUDE_REGISTER=1 + fi + fi + + if [ "$_SKIP_CLAUDE_REGISTER" -eq 1 ]; then + log "" + log " $CLAUDE_GSTACK_LINK already exists as a separate global install." + log " Skipping Claude skill registration to avoid polluting it with" + log " per-worktree symlinks. (Binaries still built locally for dev.)" + log "" + log " Global install: $CLAUDE_GSTACK_LINK" + log " This worktree: $SOURCE_GSTACK_DIR" + log "" + log " To register this worktree as the active gstack, remove the global" + log " install first: rm -rf $CLAUDE_GSTACK_LINK" + log "" + log "gstack built (claude registration skipped)." + log " browse: $BROWSE_BIN" else - cleanup_prefixed_claude_symlinks "$SOURCE_GSTACK_DIR" "$INSTALL_SKILLS_DIR" + mkdir -p "$CLAUDE_SKILLS_DIR" + ln -snf "$SOURCE_GSTACK_DIR" "$CLAUDE_GSTACK_LINK" + log " symlinked $CLAUDE_GSTACK_LINK -> $SOURCE_GSTACK_DIR" + INSTALL_SKILLS_DIR="$CLAUDE_SKILLS_DIR" + INSTALL_GSTACK_DIR="$CLAUDE_GSTACK_LINK" + # Clean up stale symlinks from the opposite prefix mode + if [ "$SKILL_PREFIX" -eq 1 ]; then + cleanup_old_claude_symlinks "$SOURCE_GSTACK_DIR" "$INSTALL_SKILLS_DIR" + else + cleanup_prefixed_claude_symlinks "$SOURCE_GSTACK_DIR" "$INSTALL_SKILLS_DIR" + fi + "$SOURCE_GSTACK_DIR/bin/gstack-patch-names" "$SOURCE_GSTACK_DIR" "$SKILL_PREFIX" + link_claude_skill_dirs "$SOURCE_GSTACK_DIR" "$INSTALL_SKILLS_DIR" + GSTACK_RELINK="$SOURCE_GSTACK_DIR/bin/gstack-relink" + if [ -x "$GSTACK_RELINK" ]; then + GSTACK_SKILLS_DIR="$INSTALL_SKILLS_DIR" GSTACK_INSTALL_DIR="$SOURCE_GSTACK_DIR" "$GSTACK_RELINK" >/dev/null 2>&1 || true + fi + _OGB_LINK="$INSTALL_SKILLS_DIR/connect-chrome" + if [ "$SKILL_PREFIX" -eq 1 ]; then + _OGB_LINK="$INSTALL_SKILLS_DIR/gstack-connect-chrome" + fi + if [ -L "$_OGB_LINK" ] || [ ! -e "$_OGB_LINK" ]; then + ln -snf "gstack/open-gstack-browser" "$_OGB_LINK" + fi + log "gstack ready (claude)." + log " browse: $BROWSE_BIN" fi - "$SOURCE_GSTACK_DIR/bin/gstack-patch-names" "$SOURCE_GSTACK_DIR" "$SKILL_PREFIX" - link_claude_skill_dirs "$SOURCE_GSTACK_DIR" "$INSTALL_SKILLS_DIR" - GSTACK_RELINK="$SOURCE_GSTACK_DIR/bin/gstack-relink" - if [ -x "$GSTACK_RELINK" ]; then - GSTACK_SKILLS_DIR="$INSTALL_SKILLS_DIR" GSTACK_INSTALL_DIR="$SOURCE_GSTACK_DIR" "$GSTACK_RELINK" >/dev/null 2>&1 || true - fi - _OGB_LINK="$INSTALL_SKILLS_DIR/connect-chrome" - if [ "$SKILL_PREFIX" -eq 1 ]; then - _OGB_LINK="$INSTALL_SKILLS_DIR/gstack-connect-chrome" - fi - if [ -L "$_OGB_LINK" ] || [ ! -e "$_OGB_LINK" ]; then - ln -snf "gstack/open-gstack-browser" "$_OGB_LINK" - fi - log "gstack ready (claude)." - log " browse: $BROWSE_BIN" fi fi diff --git a/test/setup-conductor-worktree.test.ts b/test/setup-conductor-worktree.test.ts new file mode 100644 index 000000000..6fb675afc --- /dev/null +++ b/test/setup-conductor-worktree.test.ts @@ -0,0 +1,200 @@ +import { describe, test, expect } from 'bun:test'; +import { spawnSync } from 'child_process'; +import * as path from 'path'; +import * as fs from 'fs'; +import * as os from 'os'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const SETUP_SCRIPT = path.join(ROOT, 'setup'); + +describe('setup: Conductor worktree guard', () => { + test('setup contains the real-dir guard before the ln -snf into ~/.claude/skills/', () => { + const content = fs.readFileSync(SETUP_SCRIPT, 'utf-8'); + const guardIdx = content.indexOf('_SKIP_CLAUDE_REGISTER=0'); + const lnIdx = content.indexOf('ln -snf "$SOURCE_GSTACK_DIR" "$CLAUDE_GSTACK_LINK"'); + expect(guardIdx).toBeGreaterThan(-1); + expect(lnIdx).toBeGreaterThan(-1); + expect(guardIdx).toBeLessThan(lnIdx); + }); + + test('guard resolves the existing real dir with `pwd -P` and compares against source', () => { + const content = fs.readFileSync(SETUP_SCRIPT, 'utf-8'); + expect(content).toContain('[ -d "$CLAUDE_GSTACK_LINK" ] && [ ! -L "$CLAUDE_GSTACK_LINK" ]'); + expect(content).toContain('cd "$CLAUDE_GSTACK_LINK" 2>/dev/null && pwd -P'); + expect(content).toContain('"$_EXISTING_REAL" != "$SOURCE_GSTACK_DIR"'); + }); + + test('skip branch prints "registration skipped" + remediation hint', () => { + const content = fs.readFileSync(SETUP_SCRIPT, 'utf-8'); + expect(content).toContain('Skipping Claude skill registration'); + expect(content).toContain('claude registration skipped'); + expect(content).toContain('rm -rf $CLAUDE_GSTACK_LINK'); + }); + + // Reproduce the BSD/macOS `ln -snf` behavior that caused the bug, then + // confirm the guard avoids it. This is a behavioral test of the guard logic + // running in an isolated tmpdir — not the full setup script. + test('BSD ln -snf into an existing real dir creates a child symlink (bug reproduces)', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-setup-guard-')); + try { + const source = path.join(tmp, 'source-worktree'); + const dest = path.join(tmp, 'dest-real-dir'); + fs.mkdirSync(source); + fs.mkdirSync(dest); + // The buggy invocation: target dest is an existing real dir. + const result = spawnSync('ln', ['-snf', source, dest], { encoding: 'utf-8' }); + expect(result.status).toBe(0); + // Child symlink leaked inside dest. + const leaked = path.join(dest, path.basename(source)); + expect(fs.existsSync(leaked)).toBe(true); + expect(fs.lstatSync(leaked).isSymbolicLink()).toBe(true); + expect(fs.readlinkSync(leaked)).toBe(source); + // dest itself stayed a real directory (not replaced). + expect(fs.lstatSync(dest).isSymbolicLink()).toBe(false); + expect(fs.lstatSync(dest).isDirectory()).toBe(true); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + test('guard logic refuses to ln when dest is a real dir pointing elsewhere', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-setup-guard-')); + try { + const source = path.join(tmp, 'source-worktree'); + const dest = path.join(tmp, 'dest-real-dir'); + fs.mkdirSync(source); + fs.mkdirSync(dest); + // Inline the guard logic from setup. If it triggers, $_SKIP=1 is echoed + // and no ln is performed; otherwise ln runs and we'd see the leak. + const script = ` + set -e + SOURCE_GSTACK_DIR='${source}' + CLAUDE_GSTACK_LINK='${dest}' + _SKIP_CLAUDE_REGISTER=0 + if [ -d "$CLAUDE_GSTACK_LINK" ] && [ ! -L "$CLAUDE_GSTACK_LINK" ]; then + _EXISTING_REAL=$(cd "$CLAUDE_GSTACK_LINK" 2>/dev/null && pwd -P || echo "") + if [ -n "$_EXISTING_REAL" ] && [ "$_EXISTING_REAL" != "$SOURCE_GSTACK_DIR" ]; then + _SKIP_CLAUDE_REGISTER=1 + fi + fi + if [ "$_SKIP_CLAUDE_REGISTER" -eq 1 ]; then + echo "SKIP" + else + ln -snf "$SOURCE_GSTACK_DIR" "$CLAUDE_GSTACK_LINK" + echo "LINKED" + fi + `; + const result = spawnSync('bash', ['-c', script], { encoding: 'utf-8' }); + expect(result.status).toBe(0); + expect(result.stdout.trim()).toBe('SKIP'); + // No child symlink leaked. + const leaked = path.join(dest, path.basename(source)); + expect(fs.existsSync(leaked)).toBe(false); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + test('guard allows ln when dest does not exist (fresh install path)', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-setup-guard-')); + try { + const source = path.join(tmp, 'source-worktree'); + const dest = path.join(tmp, 'fresh-dest'); + fs.mkdirSync(source); + const script = ` + set -e + SOURCE_GSTACK_DIR='${source}' + CLAUDE_GSTACK_LINK='${dest}' + _SKIP_CLAUDE_REGISTER=0 + if [ -d "$CLAUDE_GSTACK_LINK" ] && [ ! -L "$CLAUDE_GSTACK_LINK" ]; then + _EXISTING_REAL=$(cd "$CLAUDE_GSTACK_LINK" 2>/dev/null && pwd -P || echo "") + if [ -n "$_EXISTING_REAL" ] && [ "$_EXISTING_REAL" != "$SOURCE_GSTACK_DIR" ]; then + _SKIP_CLAUDE_REGISTER=1 + fi + fi + if [ "$_SKIP_CLAUDE_REGISTER" -eq 1 ]; then + echo "SKIP" + else + ln -snf "$SOURCE_GSTACK_DIR" "$CLAUDE_GSTACK_LINK" + echo "LINKED" + fi + `; + const result = spawnSync('bash', ['-c', script], { encoding: 'utf-8' }); + expect(result.status).toBe(0); + expect(result.stdout.trim()).toBe('LINKED'); + expect(fs.lstatSync(dest).isSymbolicLink()).toBe(true); + expect(fs.readlinkSync(dest)).toBe(source); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + test('guard allows ln when dest is an existing symlink (upgrade-in-place path)', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-setup-guard-')); + try { + const source = path.join(tmp, 'new-source'); + const oldSource = path.join(tmp, 'old-source'); + const dest = path.join(tmp, 'dest-symlink'); + fs.mkdirSync(source); + fs.mkdirSync(oldSource); + fs.symlinkSync(oldSource, dest); + // Existing symlink: -L is true, so the guard does NOT trigger. ln -snf + // should atomically retarget the symlink to the new source. + const script = ` + set -e + SOURCE_GSTACK_DIR='${source}' + CLAUDE_GSTACK_LINK='${dest}' + _SKIP_CLAUDE_REGISTER=0 + if [ -d "$CLAUDE_GSTACK_LINK" ] && [ ! -L "$CLAUDE_GSTACK_LINK" ]; then + _EXISTING_REAL=$(cd "$CLAUDE_GSTACK_LINK" 2>/dev/null && pwd -P || echo "") + if [ -n "$_EXISTING_REAL" ] && [ "$_EXISTING_REAL" != "$SOURCE_GSTACK_DIR" ]; then + _SKIP_CLAUDE_REGISTER=1 + fi + fi + if [ "$_SKIP_CLAUDE_REGISTER" -eq 1 ]; then + echo "SKIP" + else + ln -snf "$SOURCE_GSTACK_DIR" "$CLAUDE_GSTACK_LINK" + echo "LINKED" + fi + `; + const result = spawnSync('bash', ['-c', script], { encoding: 'utf-8' }); + expect(result.status).toBe(0); + expect(result.stdout.trim()).toBe('LINKED'); + expect(fs.readlinkSync(dest)).toBe(source); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + test('guard allows ln when dest is a real dir already pointing to source (self-rerun)', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-setup-guard-')); + try { + const source = path.join(tmp, 'source-worktree'); + fs.mkdirSync(source); + // Mirror setup's SOURCE_GSTACK_DIR resolution (`pwd -P`) so the comparison + // is fair on macOS where /tmp itself is a symlink to /private/tmp. + const resolvedSource = fs.realpathSync(source); + // Degenerate case: existing real dir IS the source. + const dest = source; + const script = ` + set -e + SOURCE_GSTACK_DIR='${resolvedSource}' + CLAUDE_GSTACK_LINK='${dest}' + _SKIP_CLAUDE_REGISTER=0 + if [ -d "$CLAUDE_GSTACK_LINK" ] && [ ! -L "$CLAUDE_GSTACK_LINK" ]; then + _EXISTING_REAL=$(cd "$CLAUDE_GSTACK_LINK" 2>/dev/null && pwd -P || echo "") + if [ -n "$_EXISTING_REAL" ] && [ "$_EXISTING_REAL" != "$SOURCE_GSTACK_DIR" ]; then + _SKIP_CLAUDE_REGISTER=1 + fi + fi + echo "skip=$_SKIP_CLAUDE_REGISTER" + `; + const result = spawnSync('bash', ['-c', script], { encoding: 'utf-8' }); + expect(result.status).toBe(0); + expect(result.stdout.trim()).toBe('skip=0'); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); +});