From 645e39eb4757f323b9b8689bb9aba345b063597b Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 21:11:18 +0000 Subject: [PATCH] F5: installed Claude skills serve the rendered sections, not the checkout copies Probe: a gbrain user render serves ship/SKILL.md from the render dir; its STOP pointers name absolute render paths, but its section index names `sections/.md` relative to the installed skill directory, whose sections/ was linked from the checkout. So the rendered 'Save Results to Brain' blocks (plan reviews, office-hours, ship adversarial) were generated and never read. setup's _link_skill_runtime_assets and gstack-relink (the gbrain-refresh path) now serve a rendered counterpart per file, keep checkout-only assets (manifest.json), return an asset to the checkout once its render is gone, and never touch an entry that is not gstack's links. Co-authored-by: Jonah Berg --- bin/gstack-relink | 42 ++++++++ setup | 35 +++++-- test/render-sections-provenance.test.ts | 124 ++++++++++++++++++++++++ 3 files changed, 195 insertions(+), 6 deletions(-) create mode 100644 test/render-sections-provenance.test.ts diff --git a/bin/gstack-relink b/bin/gstack-relink index 79a2e8ba9..71ddb5477 100755 --- a/bin/gstack-relink +++ b/bin/gstack-relink @@ -299,6 +299,47 @@ _dir_only_links() { return 0 } +# _serve_rendered_assets SKILL TARGET — F5 (#2706): when the gbrain render has +# a counterpart of a runtime asset setup linked from the checkout (sections/, +# ...), serve the rendered file instead, per file (render dirs carry only the +# generated *.md). An asset whose render counterpart is gone goes back to the +# checkout. Only entries that are our links, or directories of nothing but our +# links, are touched; anything else is the user's and stays. +_serve_rendered_assets() { + local skill="$1" target="$2" asset name t child + for asset in "$INSTALL_DIR/$skill"/*; do + [ -e "$asset" ] || continue + name="${asset##*/}" + case "$name" in SKILL.md|node_modules|dist|test|*.tmpl) continue ;; esac + t="$target/$name" + if [ -L "$t" ]; then + _target_is_ours "$(_link_target_abs "$t")" || continue + elif [ -d "$t" ]; then + _dir_only_links "$t" || continue + else + continue + fi + if [ -d "$asset" ] && [ -d "$RENDER_DIR/$skill/$name" ]; then + rm -rf "$t" + mkdir -p "$t" + for child in "$asset"/*; do + [ -e "$child" ] || continue + if [ -f "$RENDER_DIR/$skill/$name/${child##*/}" ]; then + ln -snf "$RENDER_DIR/$skill/$name/${child##*/}" "$t/${child##*/}" + else + ln -snf "$child" "$t/${child##*/}" + fi + done + elif [ -f "$asset" ] && [ -f "$RENDER_DIR/$skill/$name" ]; then + rm -rf "$t" + ln -snf "$RENDER_DIR/$skill/$name" "$t" + elif [ -d "$t" ] && [ ! -L "$t" ] || [ "$(_link_target_abs "$t")" != "$asset" ]; then + rm -rf "$t" + ln -snf "$asset" "$t" + fi + done +} + _link_root_skill_alias() { local target="$SKILLS_DIR/_gstack-command" @@ -399,6 +440,7 @@ for skill_dir in "$INSTALL_DIR"/*/; do fi fi ln -snf "$skill_md_src" "$target/SKILL.md" + _serve_rendered_assets "$skill" "$target" # Provenance marker on every platform (path-independent proof; on Windows # without Developer Mode `ln -snf` degrades to a copy and this is the only # proof), but only for a directory we created or already owned. diff --git a/setup b/setup index 9044503d4..0e50e67f6 100755 --- a/setup +++ b/setup @@ -2028,7 +2028,13 @@ _link_skill_runtime_assets() { # instead of replaced (#2119). Symlinks are never content and are always # refreshed. Default 1 = the directory is ours. local replace_real="${3:-1}" - local asset asset_name + # $4 = this skill's directory in the gbrain render, or empty (F5, #2706). + # The rendered SKILL.md is served from there, and its section index names + # sections relative to the installed directory, so a rendered counterpart of + # an asset is served per file over the checkout's (render dirs carry only + # generated *.md; manifest.json and other assets still come from the checkout). + local render_dir="${4:-}" + local asset asset_name child for asset in "$src_dir"/*; do [ -e "$asset" ] || continue # empty-glob guard asset_name="$(basename "$asset")" @@ -2044,7 +2050,21 @@ _link_skill_runtime_assets() { if [ -e "$dst_dir/$asset_name" ] || [ -L "$dst_dir/$asset_name" ]; then rm -rf "$dst_dir/$asset_name" fi - _link_or_copy "$asset" "$dst_dir/$asset_name" + if [ -n "$render_dir" ] && [ -d "$asset" ] && [ -d "$render_dir/$asset_name" ]; then + mkdir -p "$dst_dir/$asset_name" + for child in "$asset"/*; do + [ -e "$child" ] || continue + if [ -f "$render_dir/$asset_name/${child##*/}" ]; then + _link_or_copy "$render_dir/$asset_name/${child##*/}" "$dst_dir/$asset_name/${child##*/}" + else + _link_or_copy "$child" "$dst_dir/$asset_name/${child##*/}" + fi + done + elif [ -n "$render_dir" ] && [ -f "$asset" ] && [ -f "$render_dir/$asset_name" ]; then + _link_or_copy "$render_dir/$asset_name" "$dst_dir/$asset_name" + else + _link_or_copy "$asset" "$dst_dir/$asset_name" + fi # P5: the exclusion list above filters DIRECT children only, but the # Windows cp -R copy sweeps NESTED gitignored build output too (concrete: # ios-qa/scripts/gen-accessors-tool/.build is 252MB). Prune post-copy — @@ -2151,13 +2171,16 @@ link_claude_skill_dirs() { # #2569: prefer a rendered :user variant when present. gbrain installs # render brain-aware SKILL.md into ${GSTACK_HOME}/render/claude via # gen:skill-docs --out-dir instead of dirtying the tracked source - # checkout; when a render exists for this skill, serve it. The rendered - # file's section-base paths point into the render dir, so section reads - # resolve there too. + # checkout; when a render exists for this skill, serve it. Its STOP + # pointers are absolute render-dir paths, but its section index names + # `sections/.md` relative to this directory, so the runtime assets + # below prefer the render too (F5, #2706). _skill_md_src="$gstack_dir/$dir_name/SKILL.md" _render_dir="${GSTACK_USER_RENDER_DIR:-$GSTACK_STATE_ROOT/render/claude}" + _skill_render="" if [ -f "$_render_dir/$dir_name/SKILL.md" ]; then _skill_md_src="$_render_dir/$dir_name/SKILL.md" + _skill_render="$_render_dir/$dir_name" fi # A real-file SKILL.md we can only WEAKLY prove ours and whose content # differs from what we are about to serve is moved aside, not overwritten. @@ -2183,7 +2206,7 @@ link_claude_skill_dirs() { # landed and /review 404'd at "Read .claude/skills/review/checklist.md" # on every fresh Claude install. Routes through _link_or_copy so Windows # gets real copies refreshed on every ./setup. - _link_skill_runtime_assets "$gstack_dir/$dir_name" "$target" "$_assets_replace" + _link_skill_runtime_assets "$gstack_dir/$dir_name" "$target" "$_assets_replace" "$_skill_render" linked+=("$link_name") fi done diff --git a/test/render-sections-provenance.test.ts b/test/render-sections-provenance.test.ts new file mode 100644 index 000000000..2573c0bcd --- /dev/null +++ b/test/render-sections-provenance.test.ts @@ -0,0 +1,124 @@ +/** + * F5 (#2706): installed skills must SERVE the rendered section files. + * + * A gbrain install serves each brain-aware SKILL.md from the render dir + * (#2569). Its STOP pointers name absolute render-dir paths, but its section + * index names `sections/.md` relative to the installed skill directory, + * and runtime assets were linked from the checkout, so the rendered "Save + * Results to Brain" blocks, which live only in the rendered section files, + * were generated and never read. Both install paths are covered: setup's + * _link_skill_runtime_assets and bin/gstack-relink (which + * `gstack-config gbrain-refresh` calls after rendering). + */ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { runBashScript } from './helpers/bash-script'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const BIN = path.join(ROOT, 'bin'); +const SETUP_SRC = fs.readFileSync(path.join(ROOT, 'setup'), 'utf-8'); +const SAVE_BLOCK = '## Save Results to Brain'; + +function extractFn(name: string): string { + const start = SETUP_SRC.indexOf(`\n${name}() {`); + const end = SETUP_SRC.indexOf('\n}\n', start); + if (start < 0 || end < 0) throw new Error(`Could not locate ${name}()`); + return SETUP_SRC.slice(start + 1, end + 3); +} + +let tmp: string; +beforeEach(() => { tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-render-sections-')); }); +afterEach(() => { fs.rmSync(tmp, { recursive: true, force: true }); }); + +/** A checkout and a render for one carved skill; manifest.json exists only in the checkout. */ +function fixture() { + const install = path.join(tmp, 'gstack'); + const render = path.join(tmp, 'state', 'render', 'claude'); + const skills = path.join(tmp, 'skills'); + const put = (p: string, body: string) => { fs.mkdirSync(path.dirname(p), { recursive: true }); fs.writeFileSync(p, body); }; + put(path.join(install, 'ship/SKILL.md'), '---\nname: ship\ndescription: t\n---\n# ship'); + put(path.join(install, 'ship/sections/adversarial.md'), 'body\n'); + put(path.join(install, 'ship/sections/tests.md'), 'tests\n'); + put(path.join(install, 'ship/sections/manifest.json'), '{"x":1}\n'); + put(path.join(install, 'ship/checklist.md'), 'checkout checklist\n'); + put(path.join(render, 'ship/SKILL.md'), '---\nname: ship\ndescription: t\n---\n# ship rendered'); + put(path.join(render, 'ship/sections/adversarial.md'), `body\n${SAVE_BLOCK}\n`); + fs.mkdirSync(skills, { recursive: true }); + return { install, render, skills, dst: path.join(skills, 'ship') }; +} + +function runSetupHelper(src: string, dst: string, render?: string) { + fs.mkdirSync(dst, { recursive: true }); + const r = runBashScript(['set -eu', 'IS_WINDOWS=0', extractFn('_link_or_copy'), extractFn('_link_skill_runtime_assets'), + `_link_skill_runtime_assets "${src}" "${dst}" 1${render ? ` "${render}"` : ''}`].join('\n'), { timeout: 30_000 }); + expect(r.status, r.stderr).toBe(0); +} + +describe('installed skills serve rendered sections (F5, #2706)', () => { + test("setup's _link_skill_runtime_assets serves the rendered section per file and keeps checkout-only assets", () => { + const fx = fixture(); + runSetupHelper(path.join(fx.install, 'ship'), fx.dst, path.join(fx.render, 'ship')); + expect(fs.readFileSync(path.join(fx.dst, 'sections/adversarial.md'), 'utf-8')).toContain(SAVE_BLOCK); + expect(fs.readFileSync(path.join(fx.dst, 'sections/tests.md'), 'utf-8')).toBe('tests\n'); + expect(fs.existsSync(path.join(fx.dst, 'sections/manifest.json'))).toBe(true); + expect(fs.readFileSync(path.join(fx.dst, 'checklist.md'), 'utf-8')).toBe('checkout checklist\n'); + }); + + test('without a render the checkout assets are linked as before', () => { + const fx = fixture(); + runSetupHelper(path.join(fx.install, 'ship'), fx.dst); + expect(fs.lstatSync(path.join(fx.dst, 'sections')).isSymbolicLink()).toBe(true); + expect(fs.readFileSync(path.join(fx.dst, 'sections/adversarial.md'), 'utf-8')).not.toContain(SAVE_BLOCK); + }); + + test('gstack-relink (the gbrain-refresh path) serves the render, and the checkout again once the render is gone', () => { + const fx = fixture(); + fs.mkdirSync(path.join(fx.install, 'bin'), { recursive: true }); + for (const f of ['gstack-relink', 'gstack-config', 'gstack-state-root.sh', 'gstack-install-registry.sh', 'gstack-patch-names']) { + fs.copyFileSync(path.join(BIN, f), path.join(fx.install, 'bin', f)); + fs.chmodSync(path.join(fx.install, 'bin', f), 0o755); + } + // The layout setup leaves: SKILL.md and sections/ linked from the checkout. + runSetupHelper(path.join(fx.install, 'ship'), fx.dst); + fs.symlinkSync(path.join(fx.install, 'ship/SKILL.md'), path.join(fx.dst, 'SKILL.md')); + const relink = () => { + const r = spawnSync('bash', [path.join(fx.install, 'bin/gstack-relink')], { + encoding: 'utf-8', timeout: 30_000, + env: { PATH: process.env.PATH ?? '/usr/bin:/bin', HOME: tmp, GSTACK_STATE_ROOT: path.join(tmp, 'state'), + GSTACK_INSTALL_DIR: fx.install, GSTACK_SKILLS_DIR: fx.skills, GSTACK_USER_RENDER_DIR: fx.render }, + }); + expect(r.status, r.stderr).toBe(0); + }; + relink(); + expect(fs.readFileSync(path.join(fx.dst, 'SKILL.md'), 'utf-8')).toContain('ship rendered'); + expect(fs.readFileSync(path.join(fx.dst, 'sections/adversarial.md'), 'utf-8')).toContain(SAVE_BLOCK); + expect(fs.existsSync(path.join(fx.dst, 'sections/manifest.json'))).toBe(true); + expect(fs.readFileSync(path.join(fx.install, 'ship/sections/adversarial.md'), 'utf-8')).toBe('body\n'); + + fs.rmSync(path.join(fx.render, 'ship/sections'), { recursive: true }); + relink(); + expect(fs.realpathSync(path.join(fx.dst, 'sections'))).toBe(fs.realpathSync(path.join(fx.install, 'ship/sections'))); + }); + + test('a sections/ directory the user owns is left alone by relink', () => { + const fx = fixture(); + fs.mkdirSync(path.join(fx.install, 'bin'), { recursive: true }); + for (const f of ['gstack-relink', 'gstack-config', 'gstack-state-root.sh', 'gstack-install-registry.sh', 'gstack-patch-names']) { + fs.copyFileSync(path.join(BIN, f), path.join(fx.install, 'bin', f)); + fs.chmodSync(path.join(fx.install, 'bin', f), 0o755); + } + fs.mkdirSync(path.join(fx.dst, 'sections'), { recursive: true }); + fs.writeFileSync(path.join(fx.dst, 'sections/adversarial.md'), 'my notes\n'); + fs.writeFileSync(path.join(fx.dst, '.gstack-owned'), fx.install + '\n'); + const r = spawnSync('bash', [path.join(fx.install, 'bin/gstack-relink')], { + encoding: 'utf-8', timeout: 30_000, + env: { PATH: process.env.PATH ?? '/usr/bin:/bin', HOME: tmp, GSTACK_STATE_ROOT: path.join(tmp, 'state'), + GSTACK_INSTALL_DIR: fx.install, GSTACK_SKILLS_DIR: fx.skills, GSTACK_USER_RENDER_DIR: fx.render }, + }); + expect(r.status, r.stderr).toBe(0); + expect(fs.readFileSync(path.join(fx.dst, 'sections/adversarial.md'), 'utf-8')).toBe('my notes\n'); + }); +});