From 7ed5e87bba3bb33114dab0548f9899d4565e4e5b Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 31 Aug 2026 21:23:53 +0000 Subject: [PATCH] fix(render): section refs point at the FINAL render dir, never the tmp swap dir (#2692) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gen-skill-docs bakes its --out-dir into rendered CONTENT (rewriteSectionBase), and both swap-in callers (setup, gstack-config gbrain-refresh) render into claude.tmp. before the #2569 atomic rename — so every rendered skill carried ~9 dead section Read paths that pointed at a directory the swap had just deleted. New --link-root flag names the final serving dir (defaults to --out-dir for direct-render callers: bin/dev-setup, dev-skill.ts, mkdtemp tests — full caller audit in the wave notes); the rewrite now uses a replacement callback so a $-bearing configured path can't expand as $& in a replacement string. The swap logic itself stays byte-identical. Tests pin the generator contract (tmp out-dir files reference the final dir, $-bearing path included) and both callers' wiring. Fixes #2692 Co-Authored-By: Claude Fable 5 --- bin/gstack-config | 2 +- scripts/gen-skill-docs.ts | 23 ++++++++++++++-- setup | 2 +- test/gen-skill-docs-out-dir.test.ts | 42 +++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 4 deletions(-) diff --git a/bin/gstack-config b/bin/gstack-config index 46f82bfd0..cc1c20e6a 100755 --- a/bin/gstack-config +++ b/bin/gstack-config @@ -542,7 +542,7 @@ case "${1:-}" in # 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 + if ( cd "$INSTALL_DIR" && bun run gen:skill-docs:user --host claude --out-dir "$RENDER_TMP" --link-root "$RENDER_DIR" >/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. diff --git a/scripts/gen-skill-docs.ts b/scripts/gen-skill-docs.ts index c015f3be2..8ad828824 100644 --- a/scripts/gen-skill-docs.ts +++ b/scripts/gen-skill-docs.ts @@ -168,6 +168,23 @@ const OUT_DIR: string | null = (() => { return path.resolve(val); })(); +// #2692: callers that render into a TMP dir and atomically swap it into place +// (bin/gstack-config gbrain-refresh, setup — the #2569 pattern) must pass the +// FINAL directory here, or rewriteSectionBase bakes the tmp path +// (…/render/claude.tmp./…) into the rendered CONTENT and every section +// Read dies after the swap. Defaults to OUT_DIR for direct-render callers +// (bin/dev-setup, scripts/dev-skill.ts, mkdtemp tests), where out-dir IS the +// serving path. +const LINK_ROOT_ARG = process.argv.find(a => a.startsWith('--link-root')); +const LINK_ROOT: string | null = (() => { + if (!LINK_ROOT_ARG) return OUT_DIR; + const val = LINK_ROOT_ARG.includes('=') + ? LINK_ROOT_ARG.split('=')[1] + : process.argv[process.argv.indexOf(LINK_ROOT_ARG) + 1]; + if (!val) throw new Error('--link-root requires a directory path'); + return path.resolve(val); +})(); + /** * When rendering to an out-dir, repoint the literal section-base path at the * out-dir so section Reads resolve to the rendered copy, not the global install. @@ -176,10 +193,12 @@ const OUT_DIR: string | null = (() => { * install, which still works). No-op when --out-dir is unset. */ function rewriteSectionBase(content: string): string { - if (!OUT_DIR) return content; + if (!LINK_ROOT) return content; + // Replacement CALLBACK, not a template string: `$` sequences in a + // configured path are special in JS replacement strings ($&, $', $1…). return content.replace( /~\/\.claude\/skills\/gstack\/([^\s)`"'*]+\/sections\/)/g, - `${OUT_DIR}/$1`, + (_m, p1: string) => `${LINK_ROOT}/${p1}`, ); } diff --git a/setup b/setup index 0ba25180d..4e4cce4e3 100755 --- a/setup +++ b/setup @@ -2162,7 +2162,7 @@ if [ -x "$DETECT_BIN" ]; then # 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_TMP" 2>&1) + _GEN_USER_OUT=$(bun_cmd run gen:skill-docs:user --host claude --out-dir "$_GSTACK_RENDER_TMP" --link-root "$_GSTACK_RENDER_DIR" 2>&1) _GEN_USER_RC=$? printf '%s\n' "$_GEN_USER_OUT" | tail -3 exit "$_GEN_USER_RC" diff --git a/test/gen-skill-docs-out-dir.test.ts b/test/gen-skill-docs-out-dir.test.ts index f957d89be..7801c11c6 100644 --- a/test/gen-skill-docs-out-dir.test.ts +++ b/test/gen-skill-docs-out-dir.test.ts @@ -77,6 +77,48 @@ describe('gen-skill-docs --out-dir (B2 render isolation)', () => { } }); + // #2692: the swap-in callers (setup, gstack-config gbrain-refresh) render + // into claude.tmp. then RENAME it into place — so section refs must be + // rewritten to the FINAL serving dir (--link-root), never the tmp out-dir, + // or every rendered Read dies the moment the swap completes. + test('--link-root repoints section refs at the FINAL dir, not the tmp out-dir (#2692)', () => { + const tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-home-')); + const base = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-swap-')); + // Mirror the real caller shape, including a `$`-bearing path segment so a + // replacement-string regression ($& expansion) fails loudly. + const finalDir = path.join(base, 'render$live', 'claude'); + const outDir = `${finalDir}.tmp.12345`; + fs.mkdirSync(path.dirname(finalDir), { recursive: true }); + try { + fs.writeFileSync( + path.join(tmpHome, 'gbrain-detection.json'), + JSON.stringify({ gbrain_local_status: 'ok', gbrain_version: '9.9.9' }), + ); + const res = spawnSync( + 'bun', + ['run', 'scripts/gen-skill-docs.ts', '--respect-detection', '--host', 'claude', + '--out-dir', outDir, '--link-root', finalDir], + { cwd: ROOT, encoding: 'utf-8', timeout: 120_000, env: { ...process.env, GSTACK_HOME: tmpHome } }, + ); + expect(res.status).toBe(0); + const skillContent = fs.readFileSync(path.join(outDir, 'ship', 'SKILL.md'), 'utf-8'); + // Files land in the tmp out-dir; their CONTENT references the final dir. + expect(skillContent).toContain(`${finalDir}/ship/sections/`); + expect(skillContent).not.toContain(`${outDir}/ship/sections/`); + expect(skillContent).not.toContain('~/.claude/skills/gstack/ship/sections/'); + } finally { + fs.rmSync(tmpHome, { recursive: true, force: true }); + fs.rmSync(base, { recursive: true, force: true }); + } + }); + + test('both swap-in callers pass --link-root with the final render dir (#2692 wiring)', () => { + const setupSrc = fs.readFileSync(path.join(ROOT, 'setup'), 'utf-8'); + const configSrc = fs.readFileSync(path.join(ROOT, 'bin', 'gstack-config'), 'utf-8'); + expect(setupSrc).toContain('--out-dir "$_GSTACK_RENDER_TMP" --link-root "$_GSTACK_RENDER_DIR"'); + expect(configSrc).toContain('--out-dir "$RENDER_TMP" --link-root "$RENDER_DIR"'); + }); + test('retired global extras (proactive-suggestions.json) are not written anywhere', () => { const outDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-out-')); try {