Skip to content

Commit 74febfc

Browse files
committed
fix(setup): route gen:skill-docs through bun_cmd; drop tail -3 pipe that masks bun failures
Two surgical fixes called out by @jbetala7 on #1883, carved out as the focused fast-merge PR they asked for. 1. link_codex_skill_dirs, link_factory_skill_dirs, and link_opencode_skill_dirs called literal `bun run gen:skill-docs ...` instead of routing through `bun_cmd`, the wrapper installed by prepare_bun_for_windows_compile. On a non-ASCII Windows username the literal call would exit non-zero; the next line checks if the output directory exists and prints a soft warning, masking the failure. 2. The gbrain-detected SKILL.md regen ran `bun_cmd run gen:skill-docs:user --host claude 2>&1 | tail -3`. The pipe makes the subshell's exit status `tail`'s (always 0), swallowing bun's failure so the `|| log "warning..."` clause right after never fires regardless of whether bun succeeded. Tests exercise the exit-code path, not source-grep: stub bun_cmd to fail, run the actual shell pattern, assert the `|| log` clause fires only on the fixed shape and does NOT fire on the buggy shape with the pipe.
1 parent 476b0ec commit 74febfc

4 files changed

Lines changed: 206 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,84 @@
11
# Changelog
22

3+
## [1.56.2.0] - 2026-06-07
4+
5+
## **Two `setup` bugs you couldn't see. One swallowed `bun` failures, one silently broke Codex/Factory/OpenCode skill regen on a non-ASCII Windows username.**
6+
7+
Both were originally found during the phase-reorder review on #1883 and asked
8+
to be carved out for a focused, fast-merge PR. This is that PR. Just the
9+
fixes, no restructuring.
10+
11+
The `bun_cmd` wrapper exists for one reason: on Windows with a non-ASCII
12+
username, `prepare_bun_for_windows_compile` copies bun to an ASCII path and
13+
points `BUN_CMD` at it, because the system `bun` can't always handle a path
14+
through `C:\Users\<unicode>\`. Three skill-doc generation calls in
15+
`link_codex_skill_dirs`, `link_factory_skill_dirs`, and
16+
`link_opencode_skill_dirs` bypassed the wrapper and called literal `bun run`,
17+
so on a non-ASCII Windows username the regen exited non-zero. The next line
18+
checks if the output directory was created, prints a "warning" with a
19+
manual-recovery command, and continues — masking the failure as a soft
20+
warning when it was actually the wrapper being skipped.
21+
22+
The gbrain-detected regen call had a subtler bug. `bun_cmd run
23+
gen:skill-docs:user --host claude 2>&1 | tail -3` pipes through `tail` so
24+
the subshell's exit status is `tail`'s, not bun's. `tail` always succeeds,
25+
so the `|| log "warning..."` clause right after never fired regardless of
26+
whether bun succeeded. Removing the pipe restores the warning path.
27+
28+
### The numbers that matter
29+
30+
Source: `bun test test/setup-bun-cmd-and-pipe-bugs.test.ts` on this branch.
31+
32+
| Metric | Before | After | Δ |
33+
|--------|--------|-------|---|
34+
| `link_*_skill_dirs` honor `BUN_CMD` override | no | yes | 3 call sites fixed |
35+
| `gen:skill-docs:user` warning fires on bun failure | no | yes | exit-code path restored |
36+
| Functional tests exercising the exit-code path | 0 | 5 | reviewer-requested over source-grep |
37+
38+
The tests don't grep source. They stub `bun_cmd` to fail, run the exact
39+
shell pattern from `setup`, and assert the `|| log` clause does fire on the
40+
fixed shape and does NOT fire on the buggy shape. The reviewer on #1883
41+
specifically asked for this over a source-string check.
42+
43+
### What this means for you
44+
45+
If you install gstack with a non-ASCII Windows username, your Codex,
46+
Factory Droid, and OpenCode skill docs will now actually regenerate
47+
instead of silently failing. If you have gbrain installed and the
48+
post-detect SKILL.md regen ever fails, you'll see the warning instead
49+
of a silent skip. Nothing to configure.
50+
51+
### Itemized changes
52+
53+
#### Fixed
54+
55+
- `link_codex_skill_dirs`, `link_factory_skill_dirs`, and
56+
`link_opencode_skill_dirs` now route `gen:skill-docs` through `bun_cmd`
57+
(the wrapper that honors `BUN_CMD`) instead of literal `bun run`. The
58+
literal form bypassed the Windows non-ASCII path workaround, so on a
59+
Windows username with non-ASCII characters the regen exited non-zero
60+
silently. (setup:714, 919, 951)
61+
- The gbrain-detected SKILL.md regen no longer pipes `gen:skill-docs:user`
62+
through `tail -3`. The pipe made `tail`'s exit status the subshell's,
63+
swallowing bun's failure so the trailing `|| log "warning..."` guard
64+
never fired. (setup:1297)
65+
66+
#### For contributors
67+
68+
- `test/setup-bun-cmd-and-pipe-bugs.test.ts` — 5 functional tests:
69+
- 2 prove the exit-code semantics of the `| tail -3` removal by running
70+
the actual shell pattern with a stubbed-to-fail `bun_cmd` and asserting
71+
the `||` clause fires only without the pipe.
72+
- 2 prove the `bun_cmd` wrapper honors `BUN_CMD` via a sentinel script,
73+
and that all three `link_*_skill_dirs` helpers route through it.
74+
- 1 anchors on the live `gbrain detected` block in `setup` (source-anchored,
75+
not line-number) to confirm no pipe before the `||` guard.
76+
77+
These are real exit-code-path tests, not source-string grep — addressing
78+
the specific concern raised on #1883.
79+
80+
Credit: bugs identified by @jbetala7 during review on #1883.
81+
382
## [1.56.1.0] - 2026-06-03
483

584
## **`/sync-gbrain` can no longer delete your repo. Cleanup now refuses any directory it cannot prove it created.**

VERSION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
1.56.1.0
1+
1.56.2.0

setup

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -711,7 +711,7 @@ link_codex_skill_dirs() {
711711

712712
if [ ! -d "$agents_dir" ]; then
713713
echo " Generating .agents/ skill docs..."
714-
( cd "$gstack_dir" && bun run gen:skill-docs --host codex )
714+
( cd "$gstack_dir" && bun_cmd run gen:skill-docs --host codex )
715715
fi
716716

717717
if [ ! -d "$agents_dir" ]; then
@@ -916,7 +916,7 @@ link_factory_skill_dirs() {
916916

917917
if [ ! -d "$factory_dir" ]; then
918918
echo " Generating .factory/ skill docs..."
919-
( cd "$gstack_dir" && bun run gen:skill-docs --host factory )
919+
( cd "$gstack_dir" && bun_cmd run gen:skill-docs --host factory )
920920
fi
921921

922922
if [ ! -d "$factory_dir" ]; then
@@ -948,7 +948,7 @@ link_opencode_skill_dirs() {
948948

949949
if [ ! -d "$opencode_dir" ]; then
950950
echo " Generating .opencode/ skill docs..."
951-
( cd "$gstack_dir" && bun run gen:skill-docs --host opencode )
951+
( cd "$gstack_dir" && bun_cmd run gen:skill-docs --host opencode )
952952
fi
953953

954954
if [ ! -d "$opencode_dir" ]; then
@@ -1294,7 +1294,7 @@ if [ -x "$DETECT_BIN" ]; then
12941294
log "gbrain detected — regenerating Claude SKILL.md with brain-aware blocks (~250 token overhead per planning skill)..."
12951295
(
12961296
cd "$SOURCE_GSTACK_DIR"
1297-
bun_cmd run gen:skill-docs:user --host claude 2>&1 | tail -3
1297+
bun_cmd run gen:skill-docs:user --host claude
12981298
) || log " warning: gen:skill-docs:user failed — run 'bun run gen:skill-docs:user' manually if you want brain-aware blocks"
12991299
else
13001300
log "gbrain not detected — brain-aware blocks suppressed in planning-skill SKILL.md files (zero token overhead)."
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
import { describe, test, expect } from 'bun:test';
2+
import { spawnSync } from 'child_process';
3+
import * as fs from 'fs';
4+
import * as path from 'path';
5+
import * as os from 'os';
6+
7+
const ROOT = path.resolve(import.meta.dir, '..');
8+
const SETUP_SRC = fs.readFileSync(path.join(ROOT, 'setup'), 'utf-8');
9+
10+
// Run a bash snippet, return {stdout, stderr, status}.
11+
function runBash(script: string): { stdout: string; stderr: string; status: number } {
12+
const r = spawnSync('bash', ['-c', script], { encoding: 'utf-8' });
13+
return { stdout: r.stdout || '', stderr: r.stderr || '', status: r.status ?? -1 };
14+
}
15+
16+
describe('setup: gen:skill-docs:user exit-code propagation (pipe-masking fix)', () => {
17+
// The bug: `cmd 2>&1 | tail -3` makes the subshell exit status `tail`'s,
18+
// so `(...) || log "warning"` never fires when `cmd` fails. The fix removes
19+
// the pipe. These tests RUN the pattern (not grep source) to prove the
20+
// exit-code semantics actually change.
21+
22+
test('without pipe: failing bun_cmd triggers the || warning clause', () => {
23+
const r = runBash(`
24+
set +e
25+
bun_cmd() { return 1; } # stub: gen:skill-docs:user failed
26+
log() { echo "LOG:$*"; }
27+
(
28+
cd /tmp
29+
bun_cmd run gen:skill-docs:user --host claude
30+
) || log " warning: gen:skill-docs:user failed"
31+
`);
32+
expect(r.stdout).toContain('LOG: warning: gen:skill-docs:user failed');
33+
});
34+
35+
test('with pipe (the bug shape): failing bun_cmd does NOT trigger the warning', () => {
36+
const r = runBash(`
37+
set +e
38+
bun_cmd() { return 1; } # stub: gen:skill-docs:user failed
39+
log() { echo "LOG:$*"; }
40+
(
41+
cd /tmp
42+
bun_cmd run gen:skill-docs:user --host claude 2>&1 | tail -3
43+
) || log " warning: gen:skill-docs:user failed"
44+
`);
45+
expect(r.stdout).not.toContain('LOG: warning');
46+
});
47+
48+
test('setup: the live gbrain regen block has no pipe before the || guard', () => {
49+
// Slice the exact block from setup and confirm the fix is in place
50+
// without resorting to a fragile line-number check.
51+
const start = SETUP_SRC.indexOf('gbrain detected — regenerating');
52+
expect(start).toBeGreaterThan(-1);
53+
const end = SETUP_SRC.indexOf('|| log', start);
54+
expect(end).toBeGreaterThan(start);
55+
const block = SETUP_SRC.slice(start, end);
56+
expect(block).toContain('bun_cmd run gen:skill-docs:user --host claude');
57+
// The bug shape: `... | tail -N` between the call and the `|| log` guard.
58+
expect(block).not.toMatch(/gen:skill-docs:user[^\n]*\|\s*tail/);
59+
});
60+
});
61+
62+
describe('setup: bun_cmd routing in link_*_skill_dirs (Windows non-ASCII path fix)', () => {
63+
// The bug: `bun run ...` bypasses the BUN_CMD wrapper installed by
64+
// prepare_bun_for_windows_compile. On a non-ASCII Windows username,
65+
// BUN_CMD points to an ASCII-path copy of bun and the literal `bun`
66+
// on PATH may not work. Test by stubbing BUN_CMD to a sentinel that
67+
// writes a marker file, and proving the wrapper path actually invokes it.
68+
69+
test('bun_cmd wrapper invokes $BUN_CMD (not literal bun on PATH)', () => {
70+
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-buncmd-'));
71+
const marker = path.join(tmp, 'invoked');
72+
const sentinel = path.join(tmp, 'fake-bun');
73+
fs.writeFileSync(sentinel, `#!/usr/bin/env bash\necho "ARGS:$*" > "${marker}"\n`);
74+
fs.chmodSync(sentinel, 0o755);
75+
76+
const r = runBash(`
77+
set -e
78+
BUN_CMD="${sentinel}"
79+
bun_cmd() { "$BUN_CMD" "$@"; }
80+
( cd /tmp && bun_cmd run gen:skill-docs --host codex )
81+
`);
82+
83+
expect(r.status).toBe(0);
84+
expect(fs.existsSync(marker)).toBe(true);
85+
expect(fs.readFileSync(marker, 'utf-8').trim()).toBe('ARGS:run gen:skill-docs --host codex');
86+
87+
fs.rmSync(tmp, { recursive: true, force: true });
88+
});
89+
90+
test('setup: the three link_*_skill_dirs helpers all use bun_cmd, not literal bun', () => {
91+
// Extract each helper body and check the gen:skill-docs invocation
92+
// inside it. Source-anchored (not line-number) and not a global grep,
93+
// so we only assert about the actual code path the bug fix touches.
94+
for (const fn of [
95+
'link_codex_skill_dirs',
96+
'link_factory_skill_dirs',
97+
'link_opencode_skill_dirs',
98+
]) {
99+
const start = SETUP_SRC.indexOf(`${fn}() {`);
100+
expect(start).toBeGreaterThan(-1);
101+
const end = SETUP_SRC.indexOf('\n}\n', start);
102+
expect(end).toBeGreaterThan(start);
103+
const body = SETUP_SRC.slice(start, end);
104+
// Must call through the wrapper.
105+
expect(body).toMatch(/bun_cmd run gen:skill-docs/);
106+
// Bug shape: a literal `bun run gen:skill-docs` in executable position
107+
// (skipping any comment / warning-string mentions that aren't being run).
108+
const lines = body.split('\n').filter((l) => {
109+
const t = l.trim();
110+
if (!t || t.startsWith('#')) return false;
111+
// Strings inside echo/warning messages don't execute bun.
112+
if (/echo\s+['"]/.test(t)) return false;
113+
return true;
114+
});
115+
for (const l of lines) {
116+
// `bun_cmd run ...` is fine; a bare `bun run ...` is the bug.
117+
const stripped = l.replace(/bun_cmd run/g, '');
118+
expect(stripped).not.toMatch(/\bbun run gen:skill-docs/);
119+
}
120+
}
121+
});
122+
});

0 commit comments

Comments
 (0)