garrytan / garrytan/gstack

Installed `<skill>/sections` is linked from the checkout, not the render dir — gbrain section blocks never reach the agent (v1.71.0.0)

Open
#2,706 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
133k
Forks
19.9k
Avg merge
18h 46m
Merged PRs (30d)
26

Description

## Summary

`link_claude_skill_dirs` prefers the gbrain-rendered `SKILL.md` (#2569), but installs every *other* runtime asset from the source checkout:

```sh
_skill_md_src="$gstack_dir/$dir_name/SKILL.md"
if [ -f "$_render_dir/$dir_name/SKILL.md" ]; then
_skill_md_src="$_render_dir/$dir_name/SKILL.md" # render wins for SKILL.md
fi
_link_or_copy "$_skill_md_src" "$target/SKILL.md"
...
_link_skill_runtime_assets "$gstack_dir/$dir_name" "$target" # ← source only
```

So `~/.claude/skills//sections` points into the checkout while `SKILL.md` points into `~/.gstack/render/claude`. The rendered `sections/*.md` — the copies carrying the injected **Save Results to Brain** blocks — are generated and then never served.

The comment directly above the call asserts the opposite:

> `# The rendered file's section-base paths point into the render dir, so section reads resolve there too.`

That holds only for the absolute `Read ` directives. The skills also route via **relative** refs (`sections/.md`) in their Section index and carved-skill footer, and those resolve through the installed dir → checkout → block absent.

**Version:** v1.71.0.0 · gbrain installed · global-git install · macOS

## Impact

6 skills, one diverging section file each. Every one loses its brain-write step:

| skill | rendered file that never gets served |
|---|---|
| `ship` | `sections/adversarial.md` |
| `office-hours` | `sections/design-and-handoff.md` |
| `plan-ceo-review` | `sections/review-sections.md` |
| `plan-design-review` | `sections/review-sections.md` |
| `plan-devex-review` | `sections/review-sections.md` |
| `plan-eng-review` | `sections/review-sections.md` |

`gbrain-refresh` does not heal it — it re-renders into the render dir and never repoints the install, so exit 0 plus "Rendered brain-aware blocks" is not evidence the blocks are reachable.

Present since 1.67; still present on 1.71 after the carve wave took the roster 9 → 20.

## Relationship to #2692

Distinct, and #2692's fix does not close this one. #2692 is the *absolute* refs pointing at the deleted `claude.tmp.$$` dir. This is the *relative* refs resolving to the checkout. On this install both routes to `ship/sections/adversarial.md` fail — 9 dead tmp refs and 9 relative refs into un-rendered content in the same file. Fixing #2692 repairs the absolute route only.

## Reproduction (real install)

```bash
for d in ~/.gstack/render/claude/*/; do s=$(basename "$d"); [ -d "$d/sections" ] || continue
for f in "$d/sections"/*.md; do b=$(basename "$f")
src=~/.claude/skills/gstack/$s/sections/$b
[ -f "$src" ] && ! cmp -s "$f" "$src" && {
served=~/.claude/skills/$s/sections/$b
cmp -s "$served" "$f" && echo "$s/$b serves RENDER" || echo "$s/$b serves SOURCE (block lost)"; }
done; done
```

```
ship/adversarial.md serves SOURCE (block lost)
office-hours/design-and-handoff.md serves SOURCE (block lost)
plan-{ceo,design,devex,eng}-review/review-sections.md serves SOURCE (block lost)
```

Minimal diff proving the content is real and orphaned:

```bash
$ diff ~/.claude/skills/gstack/ship/sections/adversarial.md \
~/.gstack/render/claude/ship/sections/adversarial.md
> ## Save Results to Brain
> **Skip this entire section if `gbrain` is not on PATH.**
> ... gbrain put "releases/" ...
```

Driving the real helper out of `setup` (fixture: source `sections/{a.md,manifest.json,a.md.tmpl}`, render `sections/a.md` carrying the block):

```
=== current behavior (real _link_skill_runtime_assets) ===
sections/a.md -> SOURCE (block LOST)
manifest.json preserved
```

## Why the existing tests miss it

Generator side is pinned and passing — `test/gen-skill-docs-out-dir.test.ts` asserts (c) section refs repoint to the out-dir and (e) the out-dir section file gained the Save block. Both true here.

Installer side is where it slips. `test/setup-sections-linking.test.ts:34` pins the *source-only* argument as the correct shape:

```js
expect(body).toMatch(/_link_skill_runtime_assets\s+"\$gstack_dir\/\$dir_name"\s+"\$target"/);
```

so the current wiring is locked in by assertion. Nothing anywhere asserts that an installed skill dir *serves* the rendered section — the one property that matters. `test/user-render-out-dir-install.test.ts` defers section coverage to the generator test ("section repointing is pinned by gen-skill-docs-out-dir"), which covers the generator's output, not the install.

## Suggested fix

Pass the render dir through and overlay per file. A blanket repoint of the whole `sections` symlink would drop `manifest.json` / `*.tmpl`, which exist only in the checkout.

```diff
-_link_skill_runtime_assets "$gstack_dir/$dir_name" "$target"
+_link_skill_runtime_assets "$gstack_dir/$dir_name" "$target" "$_render_dir/$dir_name"
```

```diff
_link_skill_runtime_assets() {
- local src_dir="$1"
- local dst_dir="$2"
- local asset asset_name
+ local src_dir="$1" dst_dir="$2" render_dir="${3:-}"
+ local asset asset_name rsub child cn
for asset in "$src_dir"/*; do
...
+ # #2569 follow-up: when the gbrain render produced a counterpart, serve it.
+ # Render dirs carry only the generated *.md, so overlay per file rather
+ # than repointing the directory (that would drop manifest.json / *.tmpl).
+ rsub="$render_dir/$asset_name"
+ if [ -n "$render_dir" ] && [ -d "$asset" ] && [ -d "$rsub" ]; then
+ mkdir -p "$dst_dir/$asset_name"
+ for child in "$asset"/*; do
+ [ -e "$child" ] || continue
+ cn="$(basename "$child")"
+ if [ -e "$rsub/$cn" ]; then _link_or_copy "$rsub/$cn" "$dst_dir/$asset_name/$cn"
+ else _link_or_copy "$child" "$dst_dir/$asset_name/$cn"; fi
+ done
+ continue
+ elif [ -n "$render_dir" ] && [ -f "$asset" ] && [ -f "$rsub" ]; then
+ _link_or_copy "$rsub" "$dst_dir/$asset_name"; continue
+ fi
_link_or_copy "$asset" "$dst_dir/$asset_name"
done
}
```

Same fixture against the patched helper:

```
=== proposed fix ===
sections/a.md -> RENDER (block present)
manifest.json preserved
```

`bin/gstack-relink` has the same split — it repoints `SKILL.md` to the render copy (line ~108) and never touches runtime assets — so a relink between setups re-opens the gap even if setup is fixed.

The assertion at `setup-sections-linking.test.ts:34` needs updating with the fix, and the regression test worth adding is the content-provenance one: install a skill whose render and source `sections/x.md` differ, then assert the served file matches the render copy.

Opening a PR with this plus that regression test. Alternative, if you'd rather not overlay: have `gen-skill-docs --out-dir` emit the full `sections/` payload (passing `manifest.json`/`*.tmpl` through) so the whole directory can be linked from the render dir — bigger change, but it removes the two-tree split that produced both this and #2692.

Contributor guide

Open the contributing guide

Research direction

Start with the real _link_skill_runtime_assets call in setup and inspect bin/gstack-relink for the same split between SKILL.md and runtime assets. Read test/setup-sections-linking.test.ts and test/user-render-out-dir-install.test.ts, then add coverage using differing source and rendered sections. Done means installed section files serve the rendered content while source-only manifest.json and templates remain available.

Written by the indexing model from the issue text.

Assessment

Tech stack
shell, typescript
Domain
cli, testing-qa, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.