mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 14:38:59 +02:00
fix(render): a failed brain-aware render can no longer vanish the installed skill set
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>
This commit is contained in:
co-authored by
Claude Fable 5
parent
77374d1f0e
commit
594ecf818d
+29
-3
@@ -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
|
||||
;;
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
Reference in New Issue
Block a user