From 594ecf818de2a3a1c683b680abcaa5df9a3f62b5 Mon Sep 17 00:00:00 2001
From: Garry Tan <garrytan@gmail.com>
Date: Sun, 16 Aug 2026 13:57:39 -0700
Subject: [PATCH] fix(render): a failed brain-aware render can no longer vanish
 the installed skill set
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Both render sites (setup's gbrain step and gstack-config gbrain-refresh)
ran `rm -rf` on the LIVE render dir BEFORE invoking gen:skill-docs:user.
Installed skills symlink into that dir (relink prefers it), so one
transient render failure — bun error, disk full, broken template — left
every brain-aware skill's SKILL.md symlink dangling: the whole skill set
vanished from Claude Code until a successful re-render.

Both sites now render into "$RENDER_DIR.tmp.$$" and swap it in only on
SUCCESS via a shared-contract _swap_in_render helper (mv old away, mv tmp
in, drop old — links into the live path stay valid because the path never
changes). The failure branch removes only the tmp dir and says so: the
previous render, and every link into it, stays fully intact. The
deliberate wipe on the gbrain-GONE path (stale render shadowing canonical
files) is unchanged.

Pinned in test/user-render-out-dir-install.test.ts: static shape (render
targets the TMP dir, never the live dir), _swap_in_render driven
behaviorally from BOTH files, and an end-to-end failure-branch fixture
proving a pre-existing render plus an installed symlink survive a failed
render.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 bin/gstack-config                        | 32 ++++++++-
 setup                                    | 29 +++++++-
 test/user-render-out-dir-install.test.ts | 85 ++++++++++++++++++++++--
 3 files changed, 136 insertions(+), 10 deletions(-)

diff --git a/bin/gstack-config b/bin/gstack-config
index c92befed8..b8adf9c25 100755
--- a/bin/gstack-config
+++ b/bin/gstack-config
@@ -17,6 +17,21 @@ set -euo pipefail
 STATE_DIR="${GSTACK_STATE_ROOT:-${GSTACK_HOME:-${GSTACK_STATE_DIR:-$HOME/.gstack}}}"
 CONFIG_FILE="$STATE_DIR/config.yaml"
 
+# Swap a freshly-rendered tmp dir into the live render location (#2569
+# hardening). Installed skills SYMLINK into the live dir, so it is only ever
+# replaced AFTER a successful render — a failed render leaves the previous
+# render (and every link into it) fully intact. Keep in sync with setup's
+# _swap_in_render (same contract, both pinned by
+# test/user-render-out-dir-install.test.ts).
+_swap_in_render() {
+  local render_dir="$1" render_tmp="$2"
+  local render_old="$render_dir.old.$$"
+  rm -rf "$render_old"
+  if [ -e "$render_dir" ] || [ -L "$render_dir" ]; then mv "$render_dir" "$render_old"; fi
+  mv "$render_tmp" "$render_dir"
+  rm -rf "$render_old"
+}
+
 # Annotated header for new config files. Written once on first `set`.
 # Default semantics: DEFAULTS table below is the canonical source. Header text
 # is documentation that must stay in sync with DEFAULTS.
@@ -462,15 +477,26 @@ case "${1:-}" in
         elif ! command -v bun >/dev/null 2>&1; then
           echo "Skip: bun not on PATH — can't render. Install bun, then re-run 'gstack-config gbrain-refresh'."
         else
-          rm -rf "$RENDER_DIR"
-          if ( cd "$INSTALL_DIR" && bun run gen:skill-docs:user --host claude --out-dir "$RENDER_DIR" >/dev/null 2>&1 ); then
+          # Render into a tmp dir and swap it in only on SUCCESS. Installed
+          # skills SYMLINK into $RENDER_DIR (gstack-relink prefers it), so
+          # wiping it before the render meant one transient failure (bun
+          # error, disk full, broken template) left every brain-aware
+          # SKILL.md link dangling — the whole skill set vanished from
+          # Claude Code until a successful re-render. A failed render now
+          # leaves the previous render fully intact.
+          RENDER_TMP="$RENDER_DIR.tmp.$$"
+          rm -rf "$RENDER_TMP"
+          if ( cd "$INSTALL_DIR" && bun run gen:skill-docs:user --host claude --out-dir "$RENDER_TMP" >/dev/null 2>&1 ); then
+            _swap_in_render "$RENDER_DIR" "$RENDER_TMP"
             # Repoint installed skills at the render — gstack-relink prefers
             # the render dir when present.
             "$INSTALL_DIR/bin/gstack-relink" >/dev/null 2>&1 || true
             echo "Rendered brain-aware blocks into $RENDER_DIR — now live across all your projects' Claude sessions."
             echo "The install checkout stays clean: upgrades no longer stash generated render dirt (#2569)."
           else
-            echo "Warning: render failed. Run 'cd $INSTALL_DIR && bun run gen:skill-docs:user --host claude --out-dir $RENDER_DIR' manually to see the error."
+            rm -rf "$RENDER_TMP"
+            echo "Warning: render failed — previous render (if any) left in place, links stay valid."
+            echo "Run 'cd $INSTALL_DIR && bun run gen:skill-docs:user --host claude --out-dir $RENDER_DIR' manually to see the error."
           fi
         fi
         ;;
diff --git a/setup b/setup
index 4301ce80a..a17e5c22c 100755
--- a/setup
+++ b/setup
@@ -136,6 +136,21 @@ _sidecar_root_user_owned() {
   ! grep -q '<!-- AUTO-GENERATED from' "$root/SKILL.md" 2>/dev/null
 }
 
+# Swap a freshly-rendered tmp dir into the live render location (#2569
+# hardening). Installed skills SYMLINK into the live dir, so it is only ever
+# replaced AFTER a successful render — a failed render leaves the previous
+# render (and every link into it) fully intact. Keep in sync with
+# bin/gstack-config's _swap_in_render (same contract, both pinned by
+# test/user-render-out-dir-install.test.ts).
+_swap_in_render() {
+  local render_dir="$1" render_tmp="$2"
+  local render_old="$render_dir.old.$$"
+  rm -rf "$render_old"
+  if [ -e "$render_dir" ] || [ -L "$render_dir" ]; then mv "$render_dir" "$render_old"; fi
+  mv "$render_tmp" "$render_dir"
+  rm -rf "$render_old"
+}
+
 _WINDOWS_COPY_NOTE_PRINTED=0
 _print_windows_copy_note_once() {
   if [ "$IS_WINDOWS" -eq 1 ] && [ "$_WINDOWS_COPY_NOTE_PRINTED" -eq 0 ]; then
@@ -1917,24 +1932,32 @@ if [ -x "$DETECT_BIN" ]; then
         log "gbrain detected — GSTACK_SKIP_GBRAIN_REGEN set: leaving tracked SKILL.md canonical (dev/source tree)."
       else
         log "gbrain detected — rendering brain-aware Claude SKILL.md into $_GSTACK_RENDER_DIR (~250 token overhead per planning skill; source checkout stays clean)..."
-        rm -rf "$_GSTACK_RENDER_DIR"
+        # Render into a tmp dir and swap it in only on SUCCESS. Installed
+        # skills SYMLINK into the render dir (relink prefers it), so wiping
+        # it before the render meant one transient failure left every
+        # brain-aware SKILL.md link dangling — the whole skill set vanished
+        # from Claude Code until a successful re-render.
+        _GSTACK_RENDER_TMP="$_GSTACK_RENDER_DIR.tmp.$$"
+        rm -rf "$_GSTACK_RENDER_TMP"
         if (
           cd "$SOURCE_GSTACK_DIR"
           # No pipe before the || guard: `cmd | tail -3` reports TAIL's exit
           # status, so a generator crash read as success (same masking the
           # main gen:skill-docs site had). Capture, show the tail, propagate.
-          _GEN_USER_OUT=$(bun_cmd run gen:skill-docs:user --host claude --out-dir "$_GSTACK_RENDER_DIR" 2>&1)
+          _GEN_USER_OUT=$(bun_cmd run gen:skill-docs:user --host claude --out-dir "$_GSTACK_RENDER_TMP" 2>&1)
           _GEN_USER_RC=$?
           printf '%s\n' "$_GEN_USER_OUT" | tail -3
           exit "$_GEN_USER_RC"
         ); then
+          _swap_in_render "$_GSTACK_RENDER_DIR" "$_GSTACK_RENDER_TMP"
           # Repoint the installed skills at the fresh render — the installer
           # prefers rendered files when present (#2569).
           if [ "${_CLAUDE_SKILLS_LINKED:-0}" -eq 1 ]; then
             link_claude_skill_dirs "$SOURCE_GSTACK_DIR" "$INSTALL_SKILLS_DIR" >/dev/null
           fi
         else
-          log "  warning: gen:skill-docs:user failed — run 'bun run gen:skill-docs:user --host claude --out-dir $_GSTACK_RENDER_DIR' manually if you want brain-aware blocks"
+          rm -rf "$_GSTACK_RENDER_TMP"
+          log "  warning: gen:skill-docs:user failed — previous render (if any) left in place, links stay valid. Run 'bun run gen:skill-docs:user --host claude --out-dir $_GSTACK_RENDER_DIR' manually if you want fresh brain-aware blocks"
         fi
       fi
     else
diff --git a/test/user-render-out-dir-install.test.ts b/test/user-render-out-dir-install.test.ts
index eb7946fe2..4c0980d23 100644
--- a/test/user-render-out-dir-install.test.ts
+++ b/test/user-render-out-dir-install.test.ts
@@ -43,21 +43,98 @@ describe(':user render targets the out-dir, never the checkout (#2569)', () => {
     expect(sites).toBeGreaterThanOrEqual(outDirSites);
   });
 
-  test('setup wipes and repoints: rm -rf render dir + relink after a successful render', () => {
+  test('setup renders into a TMP dir, swaps on success, repoints after (never wipes first)', () => {
     const block = SETUP_SRC.slice(
       SETUP_SRC.indexOf('# ─── GBrain detection + conditional SKILL.md render'),
       SETUP_SRC.indexOf('# 11. Plan-tune cathedral hook install'),
     );
-    expect(block).toContain('rm -rf "$_GSTACK_RENDER_DIR"');
+    // The live render dir is symlinked into by installed skills — it may only
+    // be replaced AFTER a successful render (a pre-render wipe left every
+    // brain-aware SKILL.md link dangling on a transient render failure).
+    expect(block).toContain('--out-dir "$_GSTACK_RENDER_TMP"');
+    expect(block).not.toContain('--out-dir "$_GSTACK_RENDER_DIR"');
+    expect(block).toContain('_swap_in_render "$_GSTACK_RENDER_DIR" "$_GSTACK_RENDER_TMP"');
     expect(block).toContain('link_claude_skill_dirs "$SOURCE_GSTACK_DIR" "$INSTALL_SKILLS_DIR"');
-    // Stale-render cleanup on the gbrain-gone path.
+    // Stale-render cleanup on the gbrain-gone path (a deliberate wipe).
     expect(block).toContain('gbrain not detected');
+    expect(block).toContain('rm -rf "$_GSTACK_RENDER_DIR"');
   });
 
-  test('gstack-config gbrain-refresh renders to the out-dir and dropped the dirty-tree caveat', () => {
+  test('gstack-config gbrain-refresh renders to a TMP out-dir, swaps on success', () => {
     expect(CONFIG_SRC).toContain('gen:skill-docs:user --host claude --out-dir');
     expect(CONFIG_SRC).not.toContain("this dirties the install's git tree");
     expect(CONFIG_SRC).toContain('gstack-relink');
+    expect(CONFIG_SRC).toContain('--out-dir "$RENDER_TMP"');
+    expect(CONFIG_SRC).not.toContain('--out-dir "$RENDER_DIR"');
+    expect(CONFIG_SRC).toContain('_swap_in_render "$RENDER_DIR" "$RENDER_TMP"');
+  });
+
+  test('_swap_in_render behavior: success replaces, and the shape means failure never touches the live dir', () => {
+    // Both files carry the same-contract helper — drive each for real.
+    for (const src of [SETUP_SRC, CONFIG_SRC]) {
+      const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-render-swap-'));
+      try {
+        const live = path.join(tmp, 'claude');
+        const fresh = path.join(tmp, 'claude.tmp.123');
+        fs.mkdirSync(path.join(live, 'ship'), { recursive: true });
+        fs.writeFileSync(path.join(live, 'ship', 'SKILL.md'), 'old-render\n');
+        fs.mkdirSync(path.join(fresh, 'ship'), { recursive: true });
+        fs.writeFileSync(path.join(fresh, 'ship', 'SKILL.md'), 'new-render\n');
+
+        const script = [
+          'set -e',
+          extractFn(src, '_swap_in_render'),
+          `_swap_in_render "${live}" "${fresh}"`,
+        ].join('\n');
+        const r = spawnSync('bash', ['-c', script], { encoding: 'utf-8', timeout: 15_000 });
+        expect(r.status).toBe(0);
+        // Live dir now serves the fresh render at the SAME path (links into
+        // it stay valid), tmp and .old are gone.
+        expect(fs.readFileSync(path.join(live, 'ship', 'SKILL.md'), 'utf-8')).toBe('new-render\n');
+        expect(fs.existsSync(fresh)).toBe(false);
+        expect(fs.readdirSync(tmp)).toEqual(['claude']);
+      } finally {
+        fs.rmSync(tmp, { recursive: true, force: true });
+      }
+    }
+  });
+
+  test('a FAILED render leaves the previous render dir fully intact (gstack-config path, end-to-end shape)', () => {
+    // Reconstruct the exact failure branch: render into tmp fails → tmp is
+    // removed, the live dir (and the symlinks into it) are untouched, and
+    // _swap_in_render is never called.
+    const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-render-fail-'));
+    try {
+      const live = path.join(tmp, 'claude');
+      fs.mkdirSync(path.join(live, 'ship'), { recursive: true });
+      fs.writeFileSync(path.join(live, 'ship', 'SKILL.md'), 'previous-render\n');
+      // An installed skill symlinks into the live render dir.
+      const installed = path.join(tmp, 'installed-ship-SKILL.md');
+      fs.symlinkSync(path.join(live, 'ship', 'SKILL.md'), installed);
+
+      const script = [
+        'set -u',
+        extractFn(CONFIG_SRC, '_swap_in_render'),
+        `RENDER_DIR="${live}"`,
+        'RENDER_TMP="$RENDER_DIR.tmp.$$"',
+        'rm -rf "$RENDER_TMP"',
+        // The render fails (broken template, bun error, disk full).
+        'if ( mkdir -p "$RENDER_TMP" && false ); then',
+        '  _swap_in_render "$RENDER_DIR" "$RENDER_TMP"',
+        'else',
+        '  rm -rf "$RENDER_TMP"',
+        '  echo "render failed — previous render left in place" >&2',
+        'fi',
+      ].join('\n');
+      const r = spawnSync('bash', ['-c', script], { encoding: 'utf-8', timeout: 15_000 });
+      expect(r.status).toBe(0);
+      expect(fs.readFileSync(path.join(live, 'ship', 'SKILL.md'), 'utf-8')).toBe('previous-render\n');
+      // The installed symlink still resolves — the skill set did not vanish.
+      expect(fs.readFileSync(installed, 'utf-8')).toBe('previous-render\n');
+      expect(fs.readdirSync(tmp).sort()).toEqual(['claude', 'installed-ship-SKILL.md']);
+    } finally {
+      fs.rmSync(tmp, { recursive: true, force: true });
+    }
   });
 
   test('gstack-relink prefers the render dir when a rendered SKILL.md exists', () => {