From 850ac83997c7bf795e0c4e1a2d0445c27c45e33e Mon Sep 17 00:00:00 2001 From: garrytan Date: Sun, 4 Oct 2026 17:24:49 +0000 Subject: [PATCH 1/5] E1: one Bun version source; setup refuses below the 1.3.3 floor, updater checks before advancing bin/gstack-bun-version.sh holds the security floor (1.3.3) and the tested version (1.4.0, the CI pin and engines.bun). setup refuses Bun below the floor before writing anything, warns between floor and tested, warns and continues on an unparseable version, and prints a 1.4.0 install hint. Prereleases sort below their release (semver). gstack-session-update now fetches, reads the incoming release's floor and checks the bun setup will run before fast-forwarding to that exact revision; below it the update is held with reason bun-too-old and a resume command, leaving the revision and installs untouched. Fixture copies of setup (team-mode, claude-code migration) now include the sourced file; the slow-git lock test sleeps on fetch, the updater's network step. --- README.md | 2 +- bin/gstack-bun-version.sh | 58 ++++++++++ bin/gstack-session-update | 45 +++++++- setup | 14 ++- test/bun-version-drift.test.ts | 83 ++++++++++++++ test/session-update-autostash.test.ts | 2 +- test/session-update-stages.test.ts | 71 +++++++++++- test/setup-bun-floor.test.ts | 139 +++++++++++++++++++++++ test/setup-claude-code-migration.test.ts | 2 +- test/team-mode.test.ts | 2 +- 10 files changed, 408 insertions(+), 10 deletions(-) create mode 100644 bin/gstack-bun-version.sh create mode 100644 test/setup-bun-floor.test.ts diff --git a/README.md b/README.md index 0ad7765c5..75e18080d 100644 --- a/README.md +++ b/README.md @@ -44,7 +44,7 @@ Fork it. Improve it. Make it yours. And if you want to hate on free open source `./setup` took 31 seconds after the clone on a 4-vCPU Linux cloud machine (v1.91.13.0, clean HOME, including the binary build and the Chromium download). Slower networks and laptops take longer. -**Requirements:** [Claude Code](https://docs.anthropic.com/en/docs/claude-code), [Git](https://git-scm.com/), [Bun](https://bun.sh/) v1.0+, [Node.js](https://nodejs.org/) (Windows only). **Recommended on macOS:** the [Aside](https://aside.com) browser (macOS 15+) — browser skills, `/make-pdf`, and `/diagram` drive it first, with your real logged-in sessions. Without it, `./setup` builds gstack's own bundled browser and the same skills use that. `/cso` additionally needs a Bun release with all four `--no-compile-autoload-*` build flags plus a native toolchain: a static-capable C compiler on Linux, Xcode command-line tools on macOS, or Visual Studio 2022 Build Tools with Desktop development with C++ on Windows. If those are absent, setup installs everything else, removes stale CSO helpers, and `/cso` reports `not assessed` with the prerequisite. +**Requirements:** [Claude Code](https://docs.anthropic.com/en/docs/claude-code), [Git](https://git-scm.com/), [Bun](https://bun.sh/) v1.4.0+ (the tested version; setup refuses Bun older than 1.3.3, which would let compiled tools read a project's `.env`, and warns in between), [Node.js](https://nodejs.org/) (Windows only). **Recommended on macOS:** the [Aside](https://aside.com) browser (macOS 15+) — browser skills, `/make-pdf`, and `/diagram` drive it first, with your real logged-in sessions. Without it, `./setup` builds gstack's own bundled browser and the same skills use that. `/cso` additionally needs a Bun release with all four `--no-compile-autoload-*` build flags plus a native toolchain: a static-capable C compiler on Linux, Xcode command-line tools on macOS, or Visual Studio 2022 Build Tools with Desktop development with C++ on Windows. If those are absent, setup installs everything else, removes stale CSO helpers, and `/cso` reports `not assessed` with the prerequisite. When qualified CSO runtime images are published, setup gives each automatic preload a 30-second window plus a bounded setup allowance for the declared catalog. For slower registries, set an integer such as `GSTACK_CSO_IMAGE_PULL_TIMEOUT_SECONDS=120` (accepted range: 5–300 seconds). One image timing out does not consume the remaining images' windows; setup reports partial progress and a later run resumes from exact digests already present in local Docker. The complete preload is capped at one hour. diff --git a/bin/gstack-bun-version.sh b/bin/gstack-bun-version.sh new file mode 100644 index 000000000..eeb66e978 --- /dev/null +++ b/bin/gstack-bun-version.sh @@ -0,0 +1,58 @@ +# shellcheck shell=bash +# gstack-bun-version.sh — the one source for the Bun versions gstack needs. +# Sourced, never executed: ./setup refuses or warns from it before writing +# anything, and bin/gstack-session-update reads the INCOMING release's copy +# (git show) before advancing a live checkout. test/bun-version-drift.test.ts +# keeps it equal to package.json engines.bun, every CI pin, setup's install +# hint and README. Bash 3.2 builtins only. +# +# GSTACK_BUN_FLOOR is the security floor. Older Bun accepts +# --no-compile-autoload-dotenv/--no-compile-autoload-bunfig but ignores them, +# so gstack's compiled tools would read a project's .env. Verified by a +# compiled probe (a binary built with all four no-autoload flags, run beside a +# .env): Bun 1.3.2 loads it, 1.3.3 and 1.4.0 do not. +# GSTACK_BUN_TESTED is the supported minimum: the CI pin and engines.bun. +GSTACK_BUN_FLOOR="1.3.3" +GSTACK_BUN_TESTED="1.4.0" + +# _gstack_bun_parse — sets _gb_core ("MAJOR MINOR PATCH" as decimal +# integers) and _gb_pre (prerelease, may be empty); returns 1 when malformed. +# A leading "v" and "+build" metadata are ignored. +_gstack_bun_parse() { + _gb_v="${1#v}"; _gb_v="${_gb_v%%+*}"; _gb_pre="" + case "$_gb_v" in + *-*) _gb_pre="${_gb_v#*-}"; _gb_v="${_gb_v%%-*}"; [ -n "$_gb_pre" ] || return 1 ;; + esac + case "$_gb_v" in *.*.*) ;; *) return 1 ;; esac + _gb_maj="${_gb_v%%.*}"; _gb_rest="${_gb_v#*.}"; _gb_min="${_gb_rest%%.*}"; _gb_pat="${_gb_rest#*.}" + case "$_gb_maj" in ''|*[!0-9]*) return 1 ;; esac + case "$_gb_min" in ''|*[!0-9]*) return 1 ;; esac + case "$_gb_pat" in ''|*[!0-9]*) return 1 ;; esac + case "$_gb_pre" in *[!0-9A-Za-z.-]*) return 1 ;; esac + _gb_core="$((10#$_gb_maj)) $((10#$_gb_min)) $((10#$_gb_pat))" +} + +# _gstack_bun_below — true when version sorts below the +# release. Prerelease rule (semver): 1.3.3-canary.1 is below 1.3.3 and above +# 1.3.2, so a prerelease of the floor itself is refused. +_gstack_bun_below() { + _gstack_bun_parse "$2" || return 1 + set -- "$1" $_gb_core + _gstack_bun_parse "$1" || return 1 + set -- "$@" $_gb_core + [ "$5" -ne "$2" ] && { [ "$5" -lt "$2" ]; return; } + [ "$6" -ne "$3" ] && { [ "$6" -lt "$3" ]; return; } + [ "$7" -ne "$4" ] && { [ "$7" -lt "$4" ]; return; } + [ -n "$_gb_pre" ] +} + +# gstack_bun_status [floor] — prints too-old, untested, ok or +# malformed. The optional floor lets the auto-updater apply the incoming +# release's floor with this release's comparison rule. +gstack_bun_status() { + if ! _gstack_bun_parse "$1"; then echo malformed + elif _gstack_bun_below "$1" "${2:-$GSTACK_BUN_FLOOR}"; then echo too-old + elif _gstack_bun_below "$1" "$GSTACK_BUN_TESTED"; then echo untested + else echo ok + fi +} diff --git a/bin/gstack-session-update b/bin/gstack-session-update index 0fa33bb35..53d5d1fb2 100755 --- a/bin/gstack-session-update +++ b/bin/gstack-session-update @@ -5,6 +5,11 @@ # The entire update runs in background (forked). The hook itself exits # immediately so session startup is never delayed. # +# Before the live checkout moves, the updater fetches, reads the incoming +# release's Bun floor (bin/gstack-bun-version.sh) and checks the `bun` setup +# will run; below it the update is held with reason bun-too-old and nothing +# changes (E1). Updaters older than this check cannot protect their first pull. +# # Stages (B11): pull, setup and migrations are tracked separately. The # "just upgraded" marker is written only when every stage succeeded. A failed # stage is persisted in $STATE_DIR/session-update-pending and retried even @@ -25,6 +30,9 @@ STATE_DIR="$(gstack_state_root; printf x)"; STATE_DIR="${STATE_DIR%x}" # Egress receipt helpers (_receipted_git): fail-open — an update pull must # never block a session over a receipt hiccup. . "$(cd "$(dirname "$0")" && pwd)/gstack-egress-lib.sh" +# Bun floor comparison (E1): the incoming release's floor is applied before +# the live checkout moves. Missing helper fails open, like every error here. +. "$(cd "$(dirname "$0")" && pwd)/gstack-bun-version.sh" 2>/dev/null THROTTLE_FILE="$STATE_DIR/.last-session-update" LOCK_DIR="$STATE_DIR/.setup-lock" LOG_FILE="$STATE_DIR/analytics/session-update.log" @@ -63,7 +71,12 @@ if [ -f "$PENDING_FILE" ]; then case "$_P_REASON" in bun*) _P_FIX="install Bun (https://bun.sh), then $_P_FIX" ;; esac echo "gstack auto-update: ${_P_STAGES} did not finish (${_P_REASON:-unknown reason}); installed skills may be out of date. gstack retries automatically. Fix now: $_P_FIX" elif [ -n "$_P_PULL" ]; then - echo "gstack auto-update: git pull did not finish ($_P_PULL); gstack is not updating. Fix now: cd $GSTACK_DIR && git pull --ff-only && ./setup" + case "$_P_PULL" in + bun-too-old*) + echo "gstack auto-update: update held ($_P_PULL); nothing was installed or changed. Fix now: bun upgrade && cd $GSTACK_DIR && git pull --ff-only && ./setup" ;; + *) + echo "gstack auto-update: git pull did not finish ($_P_PULL); gstack is not updating. Fix now: cd $GSTACK_DIR && git pull --ff-only && ./setup" ;; + esac fi fi @@ -158,7 +171,7 @@ fi # heartbeat. trap 'kill "$HB_PID" 2>/dev/null; [ "$(cat "$LOCK_DIR/pid" 2>/dev/null)" = "$MYPID" ] && rm -rf "$LOCK_DIR" 2>/dev/null' EXIT - # ── Pull latest ── + # ── Fetch, check the incoming release's Bun floor, then fast-forward (E1) ── OLD_HEAD=$(git -C "$GSTACK_DIR" rev-parse HEAD 2>/dev/null) UPDATE_URL=$(git -C "$GSTACK_DIR" remote get-url origin 2>/dev/null || echo "") UPDATE_HOST="${UPDATE_URL#*://}"; UPDATE_HOST="${UPDATE_HOST#*@}"; UPDATE_HOST="${UPDATE_HOST%%[/:]*}" @@ -170,8 +183,27 @@ fi # Capture stderr: the log must carry WHY a pull failed, never just the code. PULL_ERR_FILE=$(mktemp "${TMPDIR:-/tmp}/gstack-session-pull-XXXXXX" 2>/dev/null || echo "") GSTACK_HOME="$STATE_DIR" _receipted_git open session-update "${UPDATE_HOST:-unknown}" gstack-self-update-pull "auto_upgrade=true" \ - bash -c 'git -C "$1" pull --ff-only --autostash -q 2>"${2:-/dev/null}"' _ "$GSTACK_DIR" "$PULL_ERR_FILE" + bash -c 'git -C "$1" fetch -q 2>"${2:-/dev/null}"' _ "$GSTACK_DIR" "$PULL_ERR_FILE" PULL_EXIT=$? + # The fast-forward targets the exact revision whose requirement was read. + # Setup runs `bun` from this PATH, so that is the binary checked; a + # malformed version passes here because setup only warns on it. + INCOMING="" BUN_HELD="" + if [ "$PULL_EXIT" -eq 0 ]; then + INCOMING=$(git -C "$GSTACK_DIR" rev-parse --verify '@{upstream}^{commit}' 2>"${PULL_ERR_FILE:-/dev/null}") || PULL_EXIT=1 + fi + if [ "$PULL_EXIT" -eq 0 ] && [ "$INCOMING" != "$OLD_HEAD" ] && command -v bun >/dev/null 2>&1; then + _IN_FLOOR=$(git -C "$GSTACK_DIR" show "$INCOMING:bin/gstack-bun-version.sh" 2>/dev/null | sed -n 's/^GSTACK_BUN_FLOOR="\([0-9A-Za-z.+-]*\)".*/\1/p' | head -1) + _BUN_FOUND=$(bun --version 2>/dev/null | head -1) + if [ -n "$_IN_FLOOR" ] && [ "$(gstack_bun_status "$_BUN_FOUND" "$_IN_FLOOR" 2>/dev/null)" = too-old ]; then + _IN_VERSION=$(git -C "$GSTACK_DIR" show "$INCOMING:VERSION" 2>/dev/null | head -1) + BUN_HELD="bun-too-old: found Bun $_BUN_FOUND at $(command -v bun); gstack ${_IN_VERSION:-update} needs $_IN_FLOOR or newer" + fi + fi + if [ "$PULL_EXIT" -eq 0 ] && [ -z "$BUN_HELD" ]; then + git -C "$GSTACK_DIR" merge --ff-only --autostash -q "$INCOMING" >/dev/null 2>"${PULL_ERR_FILE:-/dev/null}" + PULL_EXIT=$? + fi NEW_HEAD=$(git -C "$GSTACK_DIR" rev-parse HEAD 2>/dev/null) # Heartbeat: pull done — keep the TTL clock fresh for the setup step. @@ -185,6 +217,13 @@ fi case "$P_FAILURES" in ''|*[!0-9]*) P_FAILURES=0 ;; esac case "$P_NEXT" in ''|*[!0-9]*) P_NEXT=0 ;; esac + if [ -n "$BUN_HELD" ]; then + log_entry "HELD $BUN_HELD incoming=$INCOMING" + P_PULL="$BUN_HELD" + pending_save + rm -f "$PULL_ERR_FILE" 2>/dev/null + exit 0 + fi if [ "$PULL_EXIT" -ne 0 ]; then PULL_REASON=$(head -c 300 "$PULL_ERR_FILE" 2>/dev/null | tr '\n' ' ' | tr -s ' ') log_entry "PULL_FAILED exit=$PULL_EXIT reason=${PULL_REASON:-unknown}" diff --git a/setup b/setup index 70947d524..8eb4264b5 100755 --- a/setup +++ b/setup @@ -283,16 +283,28 @@ case "$HOST" in echo "Unknown --host value: $HOST — nothing was installed or changed. Expected one of: claude (default), codex, kiro, factory, opencode, cursor, copilot, slate, openclaw, hermes, gbrain, or auto. Tiers: ./setup --help" >&2; exit 1 ;; esac +# Bun versions come from one sourced file (floor and tested; E1). +. "$(dirname "$0")/bin/gstack-bun-version.sh" 2>/dev/null || { echo "$0: bin/gstack-bun-version.sh is missing; nothing was installed or changed. Fix: reinstall gstack (git clone https://github.com/garrytan/gstack)" >&2; exit 1; } if ! command -v bun >/dev/null 2>&1; then echo "Error: bun is required but not installed." >&2 echo "Install with checksum verification:" >&2 - echo ' BUN_VERSION="1.3.10"' >&2 + echo " BUN_VERSION=\"$GSTACK_BUN_TESTED\"" >&2 echo ' tmpfile=$(mktemp)' >&2 echo ' curl -fsSL "https://bun.sh/install" -o "$tmpfile"' >&2 echo ' echo "Verify checksum before running: sha256sum $tmpfile # or: shasum -a 256 $tmpfile"' >&2 echo ' BUN_VERSION="$BUN_VERSION" bash "$tmpfile" && rm "$tmpfile"' >&2 exit 1 fi +_BUN_FOUND="$(bun --version 2>/dev/null | head -1)" +case "$(gstack_bun_status "$_BUN_FOUND")" in + too-old) + echo "gstack needs Bun $GSTACK_BUN_FLOOR or newer ($GSTACK_BUN_TESTED recommended); found $_BUN_FOUND at $(command -v bun). Nothing was installed or changed." >&2 + echo "Why: older Bun ignores --no-compile-autoload-dotenv, so gstack's compiled tools could read a project's .env." >&2 + echo "Fix: bun upgrade, then re-run ./setup (docs/troubleshooting.md#bun-too-old)" >&2 + exit 1 ;; + untested) echo "warning: gstack is tested on Bun $GSTACK_BUN_TESTED (CI pin); found $_BUN_FOUND. Upgrade with: bun upgrade (docs/troubleshooting.md#bun-too-old)" >&2 ;; + malformed) echo "warning: could not read the Bun version (bun --version printed '$(printf '%s' "$_BUN_FOUND" | head -c 60)'); continuing. gstack is tested on Bun $GSTACK_BUN_TESTED (docs/troubleshooting.md#bun-too-old)" >&2 ;; +esac INSTALL_GSTACK_DIR="$(cd "$(dirname "$0")" && pwd)" SOURCE_GSTACK_DIR="$(cd "$(dirname "$0")" && pwd -P)" diff --git a/test/bun-version-drift.test.ts b/test/bun-version-drift.test.ts index 0a0d21578..c2f0a2e93 100644 --- a/test/bun-version-drift.test.ts +++ b/test/bun-version-drift.test.ts @@ -10,9 +10,16 @@ * parsing — a lane on a different Bun is testing a different product. * * Bumping Bun: change every surface in one commit; this test names each one. + * + * E1: bin/gstack-bun-version.sh is the one source setup and the auto-updater + * read. Its tested version is the CI pin, engines.bun's lower bound, setup's + * install hint and README's requirement; its floor (1.3.3) is the security + * boundary below which the no-autoload compile flags are silently ignored. */ import { describe, expect, test } from 'bun:test'; +import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; +import * as os from 'node:os'; import * as path from 'node:path'; const ROOT = path.resolve(__dirname, '..'); @@ -88,3 +95,79 @@ describe('bun version pins', () => { } }); }); + +function bunVersionsFile(): { floor: string; tested: string } { + const src = fs.readFileSync(path.join(ROOT, 'bin', 'gstack-bun-version.sh'), 'utf-8'); + return { + floor: src.match(/^GSTACK_BUN_FLOOR="([^"]+)"$/m)?.[1] ?? '', + tested: src.match(/^GSTACK_BUN_TESTED="([^"]+)"$/m)?.[1] ?? '', + }; +} + +describe('Bun requirement surfaces agree (E1)', () => { + const { floor, tested } = bunVersionsFile(); + + test('tested version == every CI pin == engines.bun lower bound; floor sits below it', () => { + const ciVersions = [...new Set(collectPins().map((p) => p.version))]; + expect(ciVersions).toEqual([tested]); + const pkg = JSON.parse(fs.readFileSync(path.join(ROOT, 'package.json'), 'utf-8')); + expect(pkg.engines?.bun).toBe(`>=${tested}`); + expect(floor).toBe('1.3.3'); + expect(Bun.semver.order(floor, tested)).toBe(-1); + }); + + test('README states the tested version and the floor', () => { + const readme = fs.readFileSync(path.join(ROOT, 'README.md'), 'utf-8'); + expect(readme).toContain(`[Bun](https://bun.sh/) v${tested}+`); + expect(readme).toContain(`refuses Bun older than ${floor}`); + }); + + test.skipIf(process.platform === 'win32')('setup install hint pins the tested version', () => { + const base = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-bun-hint-')); + try { + const nobun = path.join(base, 'bin'); + fs.mkdirSync(nobun); + for (const tool of ['dirname', 'cat', 'uname', 'mkdir']) { + const found = spawnSync('bash', ['-c', `command -v ${tool}`], { encoding: 'utf-8', timeout: 10_000 }).stdout.trim(); + if (found) fs.symlinkSync(found, path.join(nobun, tool)); + } + const r = spawnSync('/bin/bash', [path.join(ROOT, 'setup')], { + encoding: 'utf-8', env: { PATH: nobun, HOME: base }, timeout: 30_000, + }); + expect(r.status).toBe(1); + expect(r.stderr).toContain('bun is required but not installed'); + expect(r.stderr).toContain(`BUN_VERSION="${tested}"`); + } finally { + fs.rmSync(base, { recursive: true, force: true }); + } + }); + + // Compiled probe of the security floor: a binary built with the four + // no-autoload flags must not read a .env beside it. Run by hand against + // released binaries on 2026-10-04: Bun 1.3.2 accepted the flags and still + // loaded .env; 1.3.3 and 1.4.0 did not. This keeps the probe honest on the + // Bun under test (the CI pin); the control build proves the probe can see a leak. + test.skipIf(process.platform === 'win32')('compiled probe: the no-autoload flags hold on the Bun under test', () => { + const base = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-bun-probe-')); + try { + fs.writeFileSync(path.join(base, 'probe.ts'), 'console.log(process.env.GSTACK_BUN_PROBE ?? "absent");\n'); + const run = path.join(base, 'run'); + fs.mkdirSync(run); + fs.writeFileSync(path.join(run, '.env'), 'GSTACK_BUN_PROBE=leaked\n'); + const build = (out: string, flags: string[]) => { + const r = spawnSync(process.execPath, ['build', '--compile', ...flags, 'probe.ts', '--outfile', out], { + cwd: base, encoding: 'utf-8', timeout: 120_000, + }); + expect(r.status, r.stderr).toBe(0); + return spawnSync(path.join(base, out), [], { cwd: run, encoding: 'utf-8', timeout: 30_000 }).stdout.trim(); + }; + expect(build('control', [])).toBe('leaked'); + expect(build('guarded', [ + '--no-compile-autoload-dotenv', '--no-compile-autoload-bunfig', + '--no-compile-autoload-tsconfig', '--no-compile-autoload-package-json', + ])).toBe('absent'); + } finally { + fs.rmSync(base, { recursive: true, force: true }); + } + }, 240_000); +}); diff --git a/test/session-update-autostash.test.ts b/test/session-update-autostash.test.ts index b61b659a2..5bc0098c6 100644 --- a/test/session-update-autostash.test.ts +++ b/test/session-update-autostash.test.ts @@ -138,7 +138,7 @@ describe('gstack-session-update lock identity + TTL (#2613)', () => { const realGit = execFileSync('bash', ['-c', 'command -v git'], { encoding: 'utf8' }).trim(); fs.writeFileSync( path.join(shimDir, 'git'), - `#!/usr/bin/env bash\ncase "$*" in *pull*) sleep ${sleepSecs};; esac\nexec "${realGit}" "$@"\n`, + `#!/usr/bin/env bash\ncase "$*" in *fetch*) sleep ${sleepSecs};; esac\nexec "${realGit}" "$@"\n`, { mode: 0o755 }, ); return shimDir; diff --git a/test/session-update-stages.test.ts b/test/session-update-stages.test.ts index 4107fe656..4f68c4cbc 100644 --- a/test/session-update-stages.test.ts +++ b/test/session-update-stages.test.ts @@ -78,8 +78,27 @@ function upstreamRelease(fx: Fx, version: string) { git(fx.seed, 'push', '-q', 'origin', 'main'); } -function runHook(fx: Fx, opts: { noBun?: boolean } = {}) { - const PATH = opts.noBun ? pathWithoutBun(fx.base) : `${path.dirname(process.execPath)}:${process.env.PATH}`; +/** Upstream release that ships the Bun floor file (E1). */ +function upstreamReleaseWithFloor(fx: Fx, version: string, floor: string) { + fs.copyFileSync(path.join(ROOT, 'bin', 'gstack-bun-version.sh'), path.join(fx.seed, 'bin', 'gstack-bun-version.sh')); + const file = path.join(fx.seed, 'bin', 'gstack-bun-version.sh'); + fs.writeFileSync(file, fs.readFileSync(file, 'utf8').replace(/^GSTACK_BUN_FLOOR="[^"]*"/m, `GSTACK_BUN_FLOOR="${floor}"`)); + git(fx.seed, 'add', '-A'); + upstreamRelease(fx, version); +} + +/** A `bun` first on PATH that reports `version` (the binary setup would run). */ +function bunStub(fx: Fx, version: string): string { + const dir = path.join(fx.base, 'bun-stub'); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'bun'), `#!/usr/bin/env bash\nprintf '%s\\n' '${version}'\n`, { mode: 0o755 }); + return dir; +} + +function runHook(fx: Fx, opts: { noBun?: boolean; bun?: string } = {}) { + const PATH = opts.noBun ? pathWithoutBun(fx.base) + : opts.bun ? `${bunStub(fx, opts.bun)}:${pathWithoutBun(fx.base)}` + : `${path.dirname(process.execPath)}:${process.env.PATH}`; return spawnSync('bash', [SCRIPT], { encoding: 'utf8', env: { PATH, HOME: fx.home, GSTACK_DIR: fx.install, GSTACK_STATE_ROOT: fx.state, SETUP_CONTROL: fx.control, SETUP_CALLS: fx.calls, TMPDIR: fx.base }, @@ -198,3 +217,51 @@ describe.skipIf(process.platform === 'win32')('gstack-session-update stages (B11 expect(setupRuns(fx)).toBe(1); }); }); + +describe.skipIf(process.platform === 'win32')('gstack-session-update Bun floor (E1)', () => { + const head = (fx: Fx) => git(fx.install, 'rev-parse', 'HEAD'); + + test('below the incoming floor: the live checkout and installs stay untouched, with a stable bun-too-old reason', async () => { + const fx = makeFixture(); + const before = head(fx); + upstreamReleaseWithFloor(fx, '1.1.0', '1.3.3'); + const incoming = git(fx.seed, 'rev-parse', 'HEAD'); + + expect(runHook(fx, { bun: '1.3.2' }).stdout).toBe(''); + const stub = path.join(fx.base, 'bun-stub', 'bun'); + const log = await waitFor(fx, /HELD bun-too-old/); + expect(log).toContain(`HELD bun-too-old: found Bun 1.3.2 at ${stub}; gstack 1.1.0 needs 1.3.3 or newer incoming=${incoming}`); + expect(head(fx)).toBe(before); + expect(fs.readFileSync(path.join(fx.install, 'VERSION'), 'utf8')).toBe('1.0.0\n'); + expect(git(fx.install, 'status', '--porcelain')).toBe(''); + expect(setupRuns(fx)).toBe(0); + expect(fs.existsSync(marker(fx))).toBe(false); + expect(fs.readFileSync(pending(fx), 'utf8')).toContain(`pull=bun-too-old: found Bun 1.3.2 at ${stub}; gstack 1.1.0 needs 1.3.3 or newer\n`); + + expireThrottle(fx); + expect(runHook(fx, { bun: '1.3.2' }).stdout.trim()).toBe( + `gstack auto-update: update held (bun-too-old: found Bun 1.3.2 at ${stub}; gstack 1.1.0 needs 1.3.3 or newer); nothing was installed or changed. Fix now: bun upgrade && cd ${fx.install} && git pull --ff-only && ./setup`, + ); + await waitFor(fx, /HELD bun-too-old/, 2); + expect(head(fx)).toBe(before); + + // After `bun upgrade` the next session start resumes the update on its own. + expireThrottle(fx); + runHook(fx, { bun: '1.4.0' }); + await waitFor(fx, /UPDATED from=1\.0\.0 to=1\.1\.0/); + expect(head(fx)).toBe(incoming); + expect(setupRuns(fx)).toBe(1); + expect(fs.existsSync(pending(fx))).toBe(false); + }); + + for (const version of ['1.3.14', 'bun-dev (local build)']) { + test(`supported or unparseable Bun (${version}) proceeds: setup decides`, async () => { + const fx = makeFixture(); + upstreamReleaseWithFloor(fx, '1.1.0', '1.3.3'); + runHook(fx, { bun: version }); + const log = await waitFor(fx, /UPDATED from=1\.0\.0 to=1\.1\.0/); + expect(log).not.toContain('HELD'); + expect(head(fx)).toBe(git(fx.seed, 'rev-parse', 'HEAD')); + }); + } +}); diff --git a/test/setup-bun-floor.test.ts b/test/setup-bun-floor.test.ts new file mode 100644 index 000000000..f812aaaf7 --- /dev/null +++ b/test/setup-bun-floor.test.ts @@ -0,0 +1,139 @@ +import { afterEach, 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'; + +// E1: setup refuses Bun below the security floor (1.3.3) before writing +// anything, warns from the floor up to the tested version (1.4.0), warns and +// continues on a version it cannot parse, and keeps --help/--status working +// without Bun. The auto-updater half lives in session-update-stages.test.ts. + +const ROOT = path.resolve(import.meta.dir, '..'); +const SETUP = path.join(ROOT, 'setup'); +const VERSIONS = path.join(ROOT, 'bin', 'gstack-bun-version.sh'); +const bases: string[] = []; +afterEach(() => { for (const b of bases.splice(0)) fs.rmSync(b, { recursive: true, force: true }); }); + +function tempBase(): string { + const base = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-bun-floor-')); + bases.push(base); + return base; +} + +/** PATH without any bun: directories holding bun are replaced by shims of everything else. */ +function pathWithoutBun(base: string): string { + return (process.env.PATH ?? '/usr/bin:/bin').split(':').filter(Boolean).map((dir, i) => { + if (!fs.existsSync(path.join(dir, 'bun'))) return dir; + const shim = path.join(base, `nobun-${i}`); + if (fs.existsSync(shim)) return shim; + fs.mkdirSync(shim, { recursive: true }); + for (const name of fs.readdirSync(dir)) if (name !== 'bun') fs.symlinkSync(path.join(dir, name), path.join(shim, name)); + return shim; + }).join(':'); +} + +/** A `bun` that reports `version`; any other call is recorded and fails, so setup stops right after the gate. */ +function runSetupWithBun(version: string) { + const base = tempBase(); + const stub = path.join(base, 'stub'); + const home = path.join(base, 'home'); + const state = path.join(base, 'state'); + const calls = path.join(base, 'bun-calls'); + fs.mkdirSync(stub, { recursive: true }); + fs.mkdirSync(home); + fs.mkdirSync(state); + fs.writeFileSync(path.join(stub, 'bun'), [ + '#!/usr/bin/env bash', + `if [ "$1" = "--version" ]; then printf '%s\\n' '${version}'; exit 0; fi`, + `echo "$*" >> '${calls}'`, + 'echo stubbed-bun-stop >&2', + 'exit 1', + ].join('\n') + '\n', { mode: 0o755 }); + const r = spawnSync('bash', [SETUP, '--claude-model', 'claude-opus-4-8'], { + encoding: 'utf8', + env: { PATH: `${stub}:${pathWithoutBun(base)}`, HOME: home, GSTACK_STATE_ROOT: state, TMPDIR: base }, + timeout: 30_000, + }); + const bunCalls = fs.existsSync(calls) ? fs.readFileSync(calls, 'utf8') : ''; + const written = [...fs.readdirSync(home), ...fs.readdirSync(state)]; + return { ...r, stub: path.join(stub, 'bun'), bunCalls, written }; +} + +function status(version: string, floor?: string): string { + const r = spawnSync('bash', ['-c', `. "$1"; gstack_bun_status "$2" ${floor ? '"$3"' : ''}`, '_', VERSIONS, version, floor ?? ''], { + encoding: 'utf8', + timeout: 10_000, + }); + return r.stdout.trim(); +} + +describe.skipIf(process.platform === 'win32')('Bun floor (E1)', () => { + test('comparison rule: numeric fields, build metadata ignored, a prerelease sorts below its release', () => { + const table: Record = { + '1.3.2': 'too-old', + '1.3.3-canary.1': 'too-old', + '1.3.3': 'untested', + '1.3.14': 'untested', + '1.4.0-canary.20261001+abc123': 'untested', + '1.4.0': 'ok', + 'v1.4.1': 'ok', + '1.10.0': 'ok', + '2.0.0': 'ok', + '': 'malformed', + '1.4': 'malformed', + '1.4.0.1': 'malformed', + '1.4.0-': 'malformed', + 'bun-dev (local build)': 'malformed', + }; + const got = Object.fromEntries(Object.keys(table).map((v) => [v, status(v)])); + expect(got).toEqual(table); + // The updater passes the incoming release's floor explicitly. + expect(status('1.3.5', '1.3.6')).toBe('too-old'); + expect(status('1.3.6', '1.3.6')).toBe('untested'); + }); + + test('below the floor: refused with cause and fix, before anything is written', () => { + for (const version of ['1.3.2', '1.3.3-canary.1']) { + const r = runSetupWithBun(version); + expect(r.status).toBe(1); + expect(r.stderr).toContain(`gstack needs Bun 1.3.3 or newer (1.4.0 recommended); found ${version} at ${r.stub}. Nothing was installed or changed.`); + expect(r.stderr).toContain('older Bun ignores --no-compile-autoload-dotenv'); + expect(r.stderr).toContain('Fix: bun upgrade, then re-run ./setup (docs/troubleshooting.md#bun-too-old)'); + expect(r.bunCalls).toBe(''); + expect(r.written).toEqual([]); + } + }); + + test('between the floor and the tested version: one warning, then setup continues', () => { + const r = runSetupWithBun('1.3.14'); + expect(r.stderr).toContain('warning: gstack is tested on Bun 1.4.0 (CI pin); found 1.3.14. Upgrade with: bun upgrade'); + expect(r.stderr).not.toContain('gstack needs Bun'); + expect(r.bunCalls).toContain('run scripts/models.ts claude-overlay claude-opus-4-8'); + }); + + test('the tested version passes silently', () => { + const r = runSetupWithBun('1.4.0'); + expect(r.stderr).not.toMatch(/warning: gstack is tested on Bun|gstack needs Bun|could not read the Bun version/); + expect(r.bunCalls).toContain('run scripts/models.ts claude-overlay claude-opus-4-8'); + }); + + test('a malformed version warns and continues', () => { + const r = runSetupWithBun('bun-dev (local build)'); + expect(r.stderr).toContain("warning: could not read the Bun version (bun --version printed 'bun-dev (local build)'); continuing. gstack is tested on Bun 1.4.0"); + expect(r.stderr).not.toContain('gstack needs Bun'); + expect(r.bunCalls).toContain('run scripts/models.ts claude-overlay claude-opus-4-8'); + }); + + test('--help and --status keep working without Bun', () => { + const base = tempBase(); + const env = { PATH: pathWithoutBun(base), HOME: base, GSTACK_STATE_ROOT: path.join(base, 'state') }; + expect(spawnSync('bash', ['-c', 'command -v bun'], { env, timeout: 10_000 }).status).not.toBe(0); + const help = spawnSync('bash', [SETUP, '--help'], { encoding: 'utf8', env, timeout: 30_000 }); + expect(help.status).toBe(0); + expect(help.stdout).toContain('Usage:'); + const st = spawnSync('bash', [SETUP, '--status'], { encoding: 'utf8', env, timeout: 30_000 }); + expect(st.status).toBe(0); + expect(st.stderr).not.toContain('bun is required'); + }); +}); diff --git a/test/setup-claude-code-migration.test.ts b/test/setup-claude-code-migration.test.ts index fe33e9910..a4bb2344b 100644 --- a/test/setup-claude-code-migration.test.ts +++ b/test/setup-claude-code-migration.test.ts @@ -30,7 +30,7 @@ function fixture(copy: boolean) { put(path.join(root, 'VERSION'), '2.0.0.0\n'); put(path.join(root, 'ETHOS.md'), 'Fixture ethos\n'); put(path.join(root, 'package.json'), JSON.stringify({ type: 'module', scripts: { 'gen:skill-docs': 'bun run scripts/gen-skill-docs.ts' } })); - for (const bin of ['gstack-config', 'gstack-patch-names', 'gstack-relink', 'gstack-migrate-claude-code', 'gstack-claude-code', 'gstack-state-root.sh', 'gstack-install-registry.sh', 'gstack-render-claude.sh']) { + for (const bin of ['gstack-config', 'gstack-patch-names', 'gstack-relink', 'gstack-migrate-claude-code', 'gstack-claude-code', 'gstack-state-root.sh', 'gstack-bun-version.sh', 'gstack-install-registry.sh', 'gstack-render-claude.sh']) { const target = path.join(root, 'bin', bin); fs.mkdirSync(path.dirname(target), { recursive: true }); fs.copyFileSync(path.join(ROOT, 'bin', bin), target); diff --git a/test/team-mode.test.ts b/test/team-mode.test.ts index f2705165f..cee5b095a 100644 --- a/test/team-mode.test.ts +++ b/test/team-mode.test.ts @@ -366,7 +366,7 @@ describe('setup --team / --no-team / -q', () => { fs.writeFileSync(file, content, { mode: 0o755 }); }; try { - for (const rel of ['setup', 'VERSION', 'SKILL.md', 'qa/SKILL.md', 'bin/gstack-config', 'bin/gstack-patch-names', 'bin/gstack-state-root.sh', 'bin/gstack-install-registry.sh', 'bin/gstack-render-claude.sh', 'scripts/resolve-codex-generation-model.ts', 'scripts/models.ts', 'scripts/preflight-codex-overlap.ts', 'scripts/discover-skills.ts', 'scripts/external-skill-names.ts', 'scripts/host-config.ts']) { + for (const rel of ['setup', 'VERSION', 'SKILL.md', 'qa/SKILL.md', 'bin/gstack-config', 'bin/gstack-patch-names', 'bin/gstack-state-root.sh', 'bin/gstack-bun-version.sh', 'bin/gstack-install-registry.sh', 'bin/gstack-render-claude.sh', 'scripts/resolve-codex-generation-model.ts', 'scripts/models.ts', 'scripts/preflight-codex-overlap.ts', 'scripts/discover-skills.ts', 'scripts/external-skill-names.ts', 'scripts/host-config.ts']) { const dest = path.join(cwd, rel); fs.mkdirSync(path.dirname(dest), { recursive: true }); fs.copyFileSync(path.join(ROOT, rel), dest); From c58cd8031777b4501e1bd3817f3847dddbf31eb8 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sun, 4 Oct 2026 17:26:47 +0000 Subject: [PATCH 2/5] E3: record the owned Chromium's PID over CDP so the #2709 reap runs on Playwright 1.62 Playwright 1.62's Browser has no process(), so chromiumProcInfo was always null and browse stop never reaped a surviving headless Chromium. The owned headless launch now reads the browser PID from CDP SystemInfo.getProcessInfo (bounded at 2s; a hung or failed call logs once and records nothing). The reaper's start-time and cmdline identity checks are unchanged, so a recycled PID or an externally owned browser is never signalled. --- browse/src/browser-manager.ts | 43 +++++++++-- browse/test/headless-gpu-and-reap.test.ts | 88 ++++++++++++++++++++++- 2 files changed, 125 insertions(+), 6 deletions(-) diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index 2ae2221d8..c976511ed 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -182,6 +182,41 @@ function launchedProcess(browser: Browser | null | undefined): ChildProcess | nu return typeof withProcess?.process === 'function' ? withProcess.process() : null; } +/** + * #2709: PID of the Chromium browser process gstack launched, read over CDP + * (`SystemInfo.getProcessInfo`, type "browser") because Playwright 1.62's + * Browser has no `process()`. Call it only for a launch gstack owns; a + * connected browser belongs to someone else and must never be reaped. Bounded: + * a hung or failed CDP call logs once and returns null, which means no reap. + */ +export async function launchedChromiumPid(browser: Browser, timeoutMs = 2_000): Promise { + let timer: ReturnType | undefined; + const query = async (): Promise => { + const session = await browser.newBrowserCDPSession(); + try { + const { processInfo } = await session.send('SystemInfo.getProcessInfo'); + const pid = processInfo.find((p) => p.type === 'browser')?.id; + return Number.isInteger(pid) && pid! > 0 ? pid! : null; + } finally { + await session.detach().catch(() => {}); + } + }; + try { + return await Promise.race([ + query(), + new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(`timed out after ${timeoutMs}ms`)), timeoutMs); + }), + ]); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + console.warn(`[browse] Could not record the Chromium PID (CDP SystemInfo.getProcessInfo: ${msg}); browse stop cannot reap a surviving Chromium.`); + return null; + } finally { + clearTimeout(timer); + } +} + /** * Resolve why the underlying Chromium ChildProcess is going away. * @@ -669,12 +704,10 @@ export class BrowserManager { }); // #2709: record the child's identity so the CLI can reap a survivor after - // daemon shutdown. `.process()` exists here — we launched this browser. + // daemon shutdown. We launched this browser, so its PID is ours to record. { - const proc = launchedProcess(this.browser); - this.chromiumProcInfo = proc?.pid - ? { pid: proc.pid, startTime: readPidStartTime(proc.pid) } - : null; + const pid = await launchedChromiumPid(this.browser); + this.chromiumProcInfo = pid ? { pid, startTime: readPidStartTime(pid) } : null; } this.context = await this.browser.newContext(contextOptions); diff --git a/browse/test/headless-gpu-and-reap.test.ts b/browse/test/headless-gpu-and-reap.test.ts index cef5776e1..4b06423c1 100644 --- a/browse/test/headless-gpu-and-reap.test.ts +++ b/browse/test/headless-gpu-and-reap.test.ts @@ -14,10 +14,11 @@ */ import { describe, expect, test } from 'bun:test'; import { spawn } from 'node:child_process'; -import { headlessGpuArgs } from '../src/browser-manager'; +import { BrowserManager, headlessGpuArgs, launchedChromiumPid } from '../src/browser-manager'; import { reapRecordedChromium } from '../src/cli'; import { readPidStartTime } from '../src/xvfb'; import { isProcessAlive } from '../src/error-handling'; +import { chromium, type Browser } from 'playwright'; describe('headlessGpuArgs (#2709)', () => { test('darwin gets the validated four-flag set', () => { @@ -105,6 +106,83 @@ describe.skipIf(process.platform !== 'linux')('reapRecordedChromium identity gat }); }); +// E3: Playwright 1.62's Browser has no process(), so the recorded identity was +// always null and the reap above never ran. The PID now comes from CDP +// SystemInfo.getProcessInfo, only for a launch gstack owns. +describe.skipIf(process.platform !== 'linux')('Chromium PID via CDP on the pinned Playwright (#2709, E3)', () => { + const closeQuietly = (b: { close(): Promise } | null | undefined) => + Promise.race([b?.close().catch(() => {}), new Promise((r) => setTimeout(r, 3_000))]); + + test('an owned headless launch records the browser PID and start time, and the reap kills it', async () => { + const bm = new BrowserManager(); + await bm.launch(); + try { + const info = bm.getChromiumProcInfo(); + expect(info, 'headless launch recorded no Chromium identity').not.toBeNull(); + expect(isProcessAlive(info!.pid)).toBe(true); + expect(info!.startTime).toBe(readPidStartTime(info!.pid)); + expect(info!.startTime).not.toBe(''); + await reapRecordedChromium({ chromiumPid: info!.pid, chromiumStartTime: info!.startTime }); + expect(isProcessAlive(info!.pid)).toBe(false); + } finally { + await closeQuietly(bm); + } + }, 60_000); + + test('an externally owned Chromium is never reaped: its PID is not recorded and a stale identity never matches', async () => { + const external = await chromium.launch({ headless: true, chromiumSandbox: false }); + const bm = new BrowserManager(); + await bm.launch(); + try { + const externalPid = await launchedChromiumPid(external); + expect(externalPid).not.toBeNull(); + const ours = bm.getChromiumProcInfo(); + expect(ours?.pid).not.toBe(externalPid); + await reapRecordedChromium({ chromiumPid: ours!.pid, chromiumStartTime: ours!.startTime }); + expect(isProcessAlive(externalPid!)).toBe(true); + // A recycled PID: a live Chromium at the recorded PID with another start time. + await reapRecordedChromium({ chromiumPid: externalPid!, chromiumStartTime: 'Mon Jan 1 00:00:00 1990' }); + expect(isProcessAlive(externalPid!)).toBe(true); + } finally { + await closeQuietly(bm); + await closeQuietly(external); + } + }, 60_000); +}); + +describe('launchedChromiumPid bounded CDP failure (E3)', () => { + const stub = (newBrowserCDPSession: () => Promise) => ({ newBrowserCDPSession }) as unknown as Browser; + + test('a hung CDP session returns null within the bound and logs once', async () => { + const warn = console.warn; + const lines: string[] = []; + console.warn = (...args: unknown[]) => { lines.push(args.join(' ')); }; + try { + const t0 = Date.now(); + expect(await launchedChromiumPid(stub(() => new Promise(() => {})), 100)).toBeNull(); + expect(Date.now() - t0).toBeLessThan(2_000); + } finally { + console.warn = warn; + } + expect(lines).toEqual([ + '[browse] Could not record the Chromium PID (CDP SystemInfo.getProcessInfo: timed out after 100ms); browse stop cannot reap a surviving Chromium.', + ]); + }); + + test('a failing or browser-less answer returns null', async () => { + const warn = console.warn; + console.warn = () => {}; + try { + expect(await launchedChromiumPid(stub(async () => { throw new Error('Target closed'); }))).toBeNull(); + const session = (processInfo: unknown[]) => async () => ({ send: async () => ({ processInfo }), detach: async () => {} }); + expect(await launchedChromiumPid(stub(session([{ type: 'GPU', id: 4242 }])))).toBeNull(); + expect(await launchedChromiumPid(stub(session([{ type: 'browser', id: 4242 }])))).toBe(4242); + } finally { + console.warn = warn; + } + }); +}); + // ─── Source pins: the #2709 pieces stay WIRED ──────────────────────────── // The unit tests above prove headlessGpuArgs and reapRecordedChromium behave; // these pins prove the daemon persists the child's identity, the CLI reaps on @@ -195,4 +273,12 @@ describe('stop-reap wiring pins (#2709)', () => { const headless = src.slice(src.indexOf(' async launch() {'), src.indexOf(' async launchHeaded(')); expect(headless).toContain('launchArgs.push(...headlessGpuArgs(process.platform, process.env));'); }); + + test('launchedChromiumPid is called only by the owned headless launch()', () => { + const src = read('browser-manager.ts'); + const calls = src.match(/await launchedChromiumPid\(/g) ?? []; + expect(calls.length).toBe(1); + const headless = src.slice(src.indexOf(' async launch() {'), src.indexOf(' async launchHeaded(')); + expect(headless).toContain('const pid = await launchedChromiumPid(this.browser);'); + }); }); From 337d34fe16599a2adab28d1ae1c9fcc5655f5cf4 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sun, 4 Oct 2026 17:44:25 +0000 Subject: [PATCH 3/5] E4: fingerprint under a read-only .git via a private object dir owned by review-log Codex's sandbox mounts .git read-only, so gstack-wtree's git add -A could not write blobs and /review lost its start capture. gstack-review-log now creates a private 0700 object directory under TMPDIR, passes it to gstack-wtree (which writes new objects there with the common-dir object store, resolved before exporting, as an alternate, so linked worktrees work), exposes it to the bindReview/checkSharedLibsReuse consumers that read the tree and blobs, and removes it on EXIT/INT/TERM after they finish. gstack-wtree prints one stderr line and exits 1 on a failed write or unwritable TMPDIR instead of printing a partial hash; review-log no longer swallows that line. Standalone gstack-wtree keeps writing to the repository store so git diff between saved snapshots (/ship) still works. --- bin/gstack-review-log | 17 ++++- bin/gstack-wtree | 47 ++++++++++-- test/review-start-evidence.test.ts | 116 ++++++++++++++++++++++++++++- 3 files changed, 170 insertions(+), 10 deletions(-) diff --git a/bin/gstack-review-log b/bin/gstack-review-log index c1bdab264..b2b309523 100755 --- a/bin/gstack-review-log +++ b/bin/gstack-review-log @@ -40,7 +40,22 @@ WTREE="" DIRTY="" if [ -n "$COMMIT_FULL" ]; then TREE=$(git rev-parse 'HEAD^{tree}' 2>/dev/null || true) - WTREE=$("$SCRIPT_DIR/gstack-wtree" 2>/dev/null || true) + # E4: the fingerprint's new objects go to a private 0700 directory this + # operation owns (a read-only .git still fingerprints), and stay readable + # until the consumers below (bindReview / checkSharedLibsReuse read the tree + # and blobs) finish; removed on EXIT/INT/TERM. gstack-wtree's one-line + # diagnostic reaches stderr; stdout stays the hash. + if WTREE_OBJECTS=$(mktemp -d "${TMPDIR:-/tmp}/gstack-wtree-objects-XXXXXX" 2>/dev/null); then + case "$(uname -s)" in + MINGW*|MSYS*|CYGWIN*) command -v cygpath >/dev/null 2>&1 && WTREE_OBJECTS="$(cygpath -m "$WTREE_OBJECTS")" ;; + esac + trap 'rm -rf "$WTREE_OBJECTS"' EXIT + trap 'exit 130' INT TERM + WTREE=$(GSTACK_WTREE_OBJECT_DIR="$WTREE_OBJECTS" "$SCRIPT_DIR/gstack-wtree" || true) + export GIT_ALTERNATE_OBJECT_DIRECTORIES="$WTREE_OBJECTS${GIT_ALTERNATE_OBJECT_DIRECTORIES:+:$GIT_ALTERNATE_OBJECT_DIRECTORIES}" + else + echo "gstack-review-log: cannot create a private object directory under ${TMPDIR:-/tmp}; no working-tree fingerprint. Set TMPDIR to a writable directory." >&2 + fi if [ -n "$(git status --porcelain -uno 2>/dev/null | head -1)" ]; then DIRTY="true" else diff --git a/bin/gstack-wtree b/bin/gstack-wtree index 94265fa7d..b2c1c258c 100755 --- a/bin/gstack-wtree +++ b/bin/gstack-wtree @@ -21,11 +21,17 @@ # produce the identical write-tree hash. Fallback: `read-tree HEAD` when the # index copy is unavailable (fresh repo, exotic index). # -# The real repo index is never touched. Staged blobs land in the object store -# as unreachable objects and get gc'd like stash churn (note: this means the +# The real repo index is never touched. Run standalone, staged blobs land in +# the object store as unreachable objects and get gc'd like stash churn (the # CONTENT of untracked, non-ignored files enters .git/objects until gc — the -# same property `git stash -u` has). Exit 1 outside a git repo or in a repo -# with no commits — callers treat that as "no fingerprint". +# same property `git stash -u` has), so `git diff ` between saved +# snapshots works. With GSTACK_WTREE_OBJECT_DIR set (gstack-review-log, E4), +# new objects go to that private directory instead, with the repository's +# object store as an alternate: the fingerprint works under a read-only .git +# (Codex's sandbox), and the caller owns the directory's lifetime because its +# consumers read the tree afterwards. Exit 1 outside a git repo, in a repo with +# no commits, or when a write fails (one stderr line names the cause) — callers +# treat that as "no fingerprint"; a partial hash is never printed. set -euo pipefail TOP=$(git rev-parse --show-toplevel 2>/dev/null) || exit 1 @@ -33,14 +39,34 @@ TOP=$(git rev-parse --show-toplevel 2>/dev/null) || exit 1 # var set, `git rev-parse --git-path index` returns the temp index itself and # the stat-cache seed silently self-copies into a dead fast path. REAL_INDEX=$(git -C "$TOP" rev-parse --git-path index 2>/dev/null || true) -TMPIDX=$(mktemp "${TMPDIR:-/tmp}/gstack-wtree-XXXXXX") +# Same for the object store: the common dir's objects (linked worktrees share +# it), resolved to an absolute path before GIT_OBJECT_DIRECTORY is exported. +REAL_OBJECTS=$(git -C "$TOP" rev-parse --git-path objects 2>/dev/null || true) +OBJ_DIR="${GSTACK_WTREE_OBJECT_DIR:-}" +if [ -n "$OBJ_DIR" ] && [ ! -d "$OBJ_DIR" ]; then + echo "gstack-wtree: private object directory $OBJ_DIR does not exist" >&2 + exit 1 +fi +TMPIDX=$(mktemp "${TMPDIR:-/tmp}/gstack-wtree-XXXXXX" 2>/dev/null) || { + echo "gstack-wtree: cannot create a temp index under ${TMPDIR:-/tmp}; set TMPDIR to a writable directory" >&2 + exit 1 +} trap 'rm -f "$TMPIDX"' EXIT +trap 'exit 130' INT TERM export GIT_INDEX_FILE="$TMPIDX" # Resolve relative --git-path output against the repo root. case "$REAL_INDEX" in ""|/*) ;; *) REAL_INDEX="$TOP/$REAL_INDEX" ;; esac +case "$REAL_OBJECTS" in + ""|/*) ;; + *) REAL_OBJECTS="$TOP/$REAL_OBJECTS" ;; +esac +if [ -n "$OBJ_DIR" ]; then + export GIT_OBJECT_DIRECTORY="$OBJ_DIR" + export GIT_ALTERNATE_OBJECT_DIRECTORIES="$REAL_OBJECTS${GIT_ALTERNATE_OBJECT_DIRECTORIES:+:$GIT_ALTERNATE_OBJECT_DIRECTORIES}" +fi if [ -n "$REAL_INDEX" ] && [ -f "$REAL_INDEX" ] && cp "$REAL_INDEX" "$TMPIDX" 2>/dev/null; then # Carry the real index's mtime onto the copy. Git's racy-git protection # re-hashes any entry whose cached mtime is not older than the index file @@ -58,5 +84,12 @@ if [ -n "$REAL_INDEX" ] && [ -f "$REAL_INDEX" ] && cp "$REAL_INDEX" "$TMPIDX" 2> else git -C "$TOP" read-tree HEAD 2>/dev/null || exit 1 fi -git -C "$TOP" add -A 2>/dev/null || exit 1 -git -C "$TOP" write-tree 2>/dev/null +if ! ERR=$(git -C "$TOP" add -A 2>&1 >/dev/null); then + echo "gstack-wtree: cannot stage the working tree: $(printf '%s' "$ERR" | head -1)" >&2 + exit 1 +fi +if ! TREE=$(git -C "$TOP" write-tree 2>&1) || ! printf '%s' "$TREE" | grep -Eqx '[0-9a-f]{40}|[0-9a-f]{64}'; then + echo "gstack-wtree: cannot write the tree: $(printf '%s' "$TREE" | head -1)" >&2 + exit 1 +fi +printf '%s\n' "$TREE" diff --git a/test/review-start-evidence.test.ts b/test/review-start-evidence.test.ts index 3cd9f51b0..cfcf032a9 100644 --- a/test/review-start-evidence.test.ts +++ b/test/review-start-evidence.test.ts @@ -1,6 +1,6 @@ import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; -import { execFileSync } from 'node:child_process'; -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { execFileSync, spawnSync } from 'node:child_process'; +import { chmodSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { findFilesBySuffix, gitArgvIn } from './helpers/scratch-repo'; @@ -241,3 +241,115 @@ describe('review start/end binding (#2803)', () => { expect(row.review_freshness.reason).toContain('telemetry'); }); }); + +// E4: Codex's sandbox mounts .git read-only, so `git add -A` could not write +// the fingerprint's blobs and /review lost its start capture. gstack-wtree now +// writes new objects to a private directory owned by the review-log operation, +// which keeps it until bindReview/checkSharedLibsReuse have read the tree. +const asRoot = typeof process.getuid === 'function' && process.getuid() === 0; +describe.skipIf(process.platform === 'win32' || asRoot)('review fingerprint under a read-only .git (E4)', () => { + let scratch: string; + beforeEach(() => { scratch = mkdtempSync(join(tmpdir(), 'review-ro-tmp-')); }); + afterEach(() => { + spawnSync('chmod', ['-R', 'u+w', join(repo, '.git')], { timeout: 10_000 }); + rmSync(scratch, { recursive: true, force: true }); + }); + + const run = (name: string, args: string[], cwd: string, opts: { input?: string; env?: Record } = {}) => + spawnSync(join(ROOT, 'bin', name), args, { + cwd, input: opts.input, encoding: 'utf8', timeout: 20_000, + env: { ...process.env, GSTACK_HOME: home, TMPDIR: scratch, ...opts.env }, + }); + const finding = { + advisory: true, severity: 'INFORMATIONAL', action: 'skipped', + evidence_paths: ['source.ts', 'untracked.ts'], helper_target: { path: 'lib/shared.ts', symbol: 'readValue' }, + }; + const readOnlyGit = () => { + const r = spawnSync('chmod', ['-R', 'a-w', join(repo, '.git')], { timeout: 10_000 }); + expect(r.status).toBe(0); + }; + + for (const where of ['main checkout', 'linked worktree']) { + test(`--start -> --finish -> shared-libs reuse with dirty and untracked files (${where})`, () => { + let cwd = repo; + if (where === 'linked worktree') { + cwd = join(scratch, 'linked wt'); + git('worktree', 'add', '-q', '-b', 'linked', cwd); + } + writeFileSync(join(cwd, 'source.ts'), 'export const value = 42;\n'); + writeFileSync(join(cwd, 'untracked.ts'), 'export const fresh = true;\n'); + readOnlyGit(); + const before = spawnSync('find', [join(repo, '.git', 'objects'), '-type', 'f'], { encoding: 'utf8', timeout: 10_000 }).stdout; + + const start = run('gstack-review-log', ['--start', 'review'], cwd); + expect(start.stderr).toBe(''); + expect(start.status).toBe(0); + const token = start.stdout.trim(); + expect(token).toMatch(/^[0-9a-f-]{36}$/); + const record = { skill: 'review', status: 'clean', timestamp: new Date().toISOString(), completed: true, converged: true, findings: [finding] }; + const finish = run('gstack-review-log', [JSON.stringify(record), '--finish', token], cwd); + expect(finish.status).toBe(0); + const logged = JSON.parse(run('gstack-review-read', [], cwd).stdout.split('---CONFIG---')[0].trim().split('\n').at(-1)!); + expect(logged.review_binding.state).toBe('verified'); + expect(logged.wtree).toMatch(/^[0-9a-f]{40}$/); + // The consumer read the private tree and blobs before cleanup. + expect(logged.findings[0].snapshot_covered_paths).toEqual(['source.ts', 'untracked.ts']); + + const again = run('gstack-review-log', ['--start', 'review'], cwd).stdout.trim(); + const reuse = run('gstack-review-log', ['--check-shared-libs', again], cwd, { input: JSON.stringify(finding) }); + expect(reuse.status).toBe(0); + expect(JSON.parse(reuse.stdout).reusable).toBe(true); + + expect(spawnSync('find', [join(repo, '.git', 'objects'), '-type', 'f'], { encoding: 'utf8', timeout: 10_000 }).stdout).toBe(before); + expect(readdirSync(scratch).filter((name) => name.startsWith('gstack-wtree'))).toEqual([]); + + // Same fingerprint as a writable .git. + spawnSync('chmod', ['-R', 'u+w', join(repo, '.git')], { timeout: 10_000 }); + expect(run('gstack-wtree', [], cwd).stdout.trim()).toBe(logged.wtree); + }, 60_000); + } + + test('the private directory is 0700 and alive while the consumer runs, then removed', () => { + writeFileSync(join(repo, 'untracked.ts'), 'export const fresh = true;\n'); + readOnlyGit(); + const shim = join(scratch, 'shim'); + const probe = join(scratch, 'probe.log'); + mkdirSync(shim); + writeFileSync(join(shim, 'bun'), [ + '#!/usr/bin/env bash', + 'first="${GIT_ALTERNATE_OBJECT_DIRECTORIES%%:*}"', + `[ -n "$first" ] && ls -ld "$first" | cut -c1-10 >> '${probe}'`, + `exec '${process.execPath}' "$@"`, + ].join('\n') + '\n', { mode: 0o755 }); + const env = { PATH: `${shim}:${process.env.PATH}` }; + const token = run('gstack-review-log', ['--start', 'review'], repo, { env }).stdout.trim(); + const record = { skill: 'review', status: 'clean', timestamp: new Date().toISOString(), completed: true, converged: true }; + expect(run('gstack-review-log', [JSON.stringify(record), '--finish', token], repo, { env }).status).toBe(0); + expect(readFileSync(probe, 'utf8').trim().split('\n')).toEqual(['drwx------', 'drwx------']); + expect(readdirSync(scratch).filter((name) => name.startsWith('gstack-wtree'))).toEqual([]); + }, 60_000); + + test('a failed object write exits 1 with one stderr line and no partial hash', () => { + writeFileSync(join(repo, 'untracked.ts'), 'export const fresh = true;\n'); + const objects = join(scratch, 'objects'); + mkdirSync(objects, { mode: 0o500 }); + const r = run('gstack-wtree', [], repo, { env: { GSTACK_WTREE_OBJECT_DIR: objects } }); + chmodSync(objects, 0o700); + expect(r.status).toBe(1); + expect(r.stdout).toBe(''); + expect(r.stderr.trim().split('\n')).toHaveLength(1); + expect(r.stderr).toStartWith('gstack-wtree: cannot stage the working tree: '); + }); + + test('an unwritable TMPDIR is named, and review-log reports it on stderr while stdout stays clean', () => { + const missing = join(scratch, 'no such dir'); + const wtree = run('gstack-wtree', [], repo, { env: { TMPDIR: missing } }); + expect(wtree.status).toBe(1); + expect(wtree.stdout).toBe(''); + expect(wtree.stderr.trim()).toBe(`gstack-wtree: cannot create a temp index under ${missing}; set TMPDIR to a writable directory`); + const start = run('gstack-review-log', ['--start', 'review'], repo, { env: { TMPDIR: missing } }); + expect(start.status).not.toBe(0); + expect(start.stdout).toBe(''); + expect(start.stderr).toContain(`gstack-review-log: cannot create a private object directory under ${missing}; no working-tree fingerprint.`); + }, 30_000); +}); From d16a1687f965ceca641a32f779b37f4b36f065a5 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sun, 4 Oct 2026 17:50:11 +0000 Subject: [PATCH 4/5] E2: one make-pdf resolver for readiness and every $P block on env-var hosts make-pdf exported $P from its readiness block, which Codex and the other env-var hosts lose between blocks. binaryAssignment gains 'make-pdf': the MAKE-PDF SETUP readiness check and the runtime prelude both use it, so every block that runs $P derives the same make-pdf/dist/pdf binary, with MAKE_PDF_BIN (dev builds) winning when it is executable. Claude output keeps its repo-local-first probe; only the false 'subsequent blocks' comment changed. Root plus P=... fits the 400-byte prelude budget; the fixed MAKE_PDF_OVERRIDE segment is the one exemption, named in runtime-root.ts and the conformance test. --- scripts/resolvers/make-pdf.ts | 27 +++++----- scripts/resolvers/runtime-root.ts | 37 +++++++++---- test/env-host-fence-conformance.test.ts | 70 +++++++++++++++++++++---- 3 files changed, 100 insertions(+), 34 deletions(-) diff --git a/scripts/resolvers/make-pdf.ts b/scripts/resolvers/make-pdf.ts index 55729b0cc..52216d92e 100644 --- a/scripts/resolvers/make-pdf.ts +++ b/scripts/resolvers/make-pdf.ts @@ -1,29 +1,26 @@ -import { type TemplateContext, toShellPath } from './types'; +import { type TemplateContext } from './types'; +import { binaryAssignment } from './runtime-root'; /** - * {{MAKE_PDF_SETUP}} — emits the shell preamble that resolves $P to the - * make-pdf binary. Mirrors generateDesignSetup. + * {{MAKE_PDF_SETUP}} — emits the readiness check that resolves $P to the + * make-pdf binary (make-pdf/dist/pdf). Mirrors generateDesignSetup: the + * assignment comes from binaryAssignment(ctx, 'make-pdf'), the same resolver + * the runtime prelude puts into every later block that uses $P on env-var + * hosts, so readiness and use always name the same binary. * - * $P = make-pdf/dist/pdf. - * - * Resolution order: - * 1. Local skill root: $_ROOT/{localSkillRoot}/make-pdf/dist/pdf - * 2. Global: ~/{globalRoot}/make-pdf/dist/pdf - * 3. Env override (MAKE_PDF_BIN) — for contributor dev builds + * Resolution order: MAKE_PDF_BIN (contributor dev builds), then the + * repo-local install, then the global install (env-var hosts: $GSTACK_ROOT, + * which the prelude resolves repo-local first). */ export function generateMakePdfSetup(ctx: TemplateContext): string { return `## MAKE-PDF SETUP (run this check BEFORE any make-pdf command) \`\`\`bash -_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) -P="" -[ -n "$MAKE_PDF_BIN" ] && [ -x "$MAKE_PDF_BIN" ] && P="$MAKE_PDF_BIN" -[ -z "$P" ] && [ -n "$_ROOT" ] && [ -x "$_ROOT/${ctx.paths.localSkillRoot}/make-pdf/dist/pdf" ] && P="$_ROOT/${ctx.paths.localSkillRoot}/make-pdf/dist/pdf" -[ -z "$P" ] && P="${toShellPath(ctx.paths.skillRoot)}/make-pdf/dist/pdf" +${binaryAssignment(ctx, 'make-pdf')} if [ -x "$P" ]; then echo "MAKE_PDF_READY: $P" alias _p_="$P" # shellcheck alias helper (not exported) - export P # available as $P in subsequent blocks within the same skill invocation + export P # env-var hosts re-derive $P in every later block (runtime prelude) else echo "MAKE_PDF_NOT_AVAILABLE (run './setup' in the gstack repo to build it)" fi diff --git a/scripts/resolvers/runtime-root.ts b/scripts/resolvers/runtime-root.ts index e9e6a507e..31df6b168 100644 --- a/scripts/resolvers/runtime-root.ts +++ b/scripts/resolvers/runtime-root.ts @@ -2,8 +2,8 @@ * Runtime root for env-var hosts (C1, #1159): the one owner of how generated * bash finds gstack on Codex and every other `usesEnvVars` host. * - * Those hosts run each fenced block in a fresh shell, so `GSTACK_ROOT`, `B` - * and `D` set by an earlier block are gone. The preamble's root resolution and + * Those hosts run each fenced block in a fresh shell, so `GSTACK_ROOT`, `B`, + * `D` and `P` set by an earlier block are gone. The preamble's root resolution and * the per-fence prelude come from `runtimeRootPrelude()`; `insertRuntimePreludes()` * is the single post-render pass (gen-skill-docs, before rewriteInstallRoot) * that puts it into every fence that uses these variables without assigning @@ -19,6 +19,13 @@ import { toShellPath, type TemplateContext } from './types'; /** Per-fence byte budget for the prelude (T-ENG1), asserted by the INV-3 test. */ export const PRELUDE_BYTE_BUDGET = 400; +/** + * make-pdf's dev-build override (E2), appended to `P`'s assignment. It is the + * one fixed segment outside PRELUDE_BYTE_BUDGET: root plus `P=...` fits the + * budget like `B`/`D` do, and the override is needed in every block that runs + * `$P` so readiness and use name the same binary. + */ +export const MAKE_PDF_OVERRIDE = ';[ -x "${MAKE_PDF_BIN:-}" ]&&P=$MAKE_PDF_BIN||:'; const ROOT_VARS = /\$\{?GSTACK_(?:ROOT|BIN|BROWSE|DESIGN|MAKE_PDF)\b/; const assigns = (name: string) => new RegExp(`(?:^|[\\s;&|(])${name}=`, 'm'); @@ -48,13 +55,23 @@ export function runtimeRootPrelude(ctx: TemplateContext): string { } /** - * Lines that assign `B` (browse) or `D` (design). BROWSE SETUP, DESIGN SETUP and - * the prelude all use this, so a later fence runs the binary setup checked. - * Env-var hosts derive from `GSTACK_ROOT`, which the prelude already points at - * the repo-local install when one exists; other hosts keep their - * repo-local-first probe. + * Lines that assign `B` (browse), `D` (design) or `P` (make-pdf). BROWSE SETUP, + * DESIGN SETUP, MAKE-PDF SETUP and the prelude all use this, so a later fence + * runs the binary setup checked. Env-var hosts derive from `GSTACK_ROOT`, which + * the prelude already points at the repo-local install when one exists; other + * hosts keep their repo-local-first probe. make-pdf's binary is + * `make-pdf/dist/pdf`, and `MAKE_PDF_BIN` (a contributor dev build) wins on + * every host. */ -export function binaryAssignment(ctx: TemplateContext, tool: 'browse' | 'design'): string { +export function binaryAssignment(ctx: TemplateContext, tool: 'browse' | 'design' | 'make-pdf'): string { + if (tool === 'make-pdf') { + if (getHostConfig(ctx.host).usesEnvVars) return `P=$GSTACK_ROOT/make-pdf/dist/pdf${MAKE_PDF_OVERRIDE}`; + return `_ROOT=$(git rev-parse --show-toplevel 2>/dev/null) +P="" +[ -n "$MAKE_PDF_BIN" ] && [ -x "$MAKE_PDF_BIN" ] && P="$MAKE_PDF_BIN" +[ -z "$P" ] && [ -n "$_ROOT" ] && [ -x "$_ROOT/${ctx.paths.localSkillRoot}/make-pdf/dist/pdf" ] && P="$_ROOT/${ctx.paths.localSkillRoot}/make-pdf/dist/pdf" +[ -z "$P" ] && P="${toShellPath(ctx.paths.skillRoot)}/make-pdf/dist/pdf"`; + } const v = tool === 'browse' ? 'B' : 'D'; const dir = tool === 'browse' ? ctx.paths.browseDir : ctx.paths.designDir; if (getHostConfig(ctx.host).usesEnvVars) return `${v}=$GSTACK_ROOT/${tool}/dist/${tool}`; @@ -69,8 +86,9 @@ export function fencePrelude(ctx: TemplateContext, body: string): string { if (!getHostConfig(ctx.host).usesEnvVars) return ''; const needB = uses('B').test(body) && !assigns('B').test(body); const needD = uses('D').test(body) && !assigns('D').test(body); + const needP = uses('P').test(body) && !assigns('P').test(body); if (assigns('GSTACK_ROOT').test(body)) return ''; - const needRoot = needB || needD || ROOT_VARS.test(body); + const needRoot = needB || needD || needP || ROOT_VARS.test(body); const derived = Object.entries(DERIVED) .filter(([name]) => uses(`GSTACK_${name}`).test(body) && !assigns(`GSTACK_${name}`).test(body)) .map(([name, dir]) => `GSTACK_${name}=$GSTACK_ROOT/${dir}`); @@ -79,6 +97,7 @@ export function fencePrelude(ctx: TemplateContext, body: string): string { derived.join(' '), needB ? binaryAssignment(ctx, 'browse') : '', needD ? binaryAssignment(ctx, 'design') : '', + needP ? binaryAssignment(ctx, 'make-pdf') : '', ].filter(Boolean).join('\n'); } diff --git a/test/env-host-fence-conformance.test.ts b/test/env-host-fence-conformance.test.ts index 872f3d304..78b37d98a 100644 --- a/test/env-host-fence-conformance.test.ts +++ b/test/env-host-fence-conformance.test.ts @@ -16,13 +16,14 @@ import * as os from 'os'; import * as path from 'path'; import { ALL_HOST_CONFIGS } from '../hosts'; import { runGeneration } from '../scripts/gen-skill-docs'; -import { binaryAssignment, fencePrelude, insertRuntimePreludes, PRELUDE_BYTE_BUDGET, runtimeRootPrelude } from '../scripts/resolvers/runtime-root'; +import { generateMakePdfSetup } from '../scripts/resolvers/make-pdf'; +import { binaryAssignment, fencePrelude, insertRuntimePreludes, MAKE_PDF_OVERRIDE, PRELUDE_BYTE_BUDGET, runtimeRootPrelude } from '../scripts/resolvers/runtime-root'; import { HOST_PATHS, type TemplateContext } from '../scripts/resolvers/types'; import { lintFence, normalizePlaceholders } from './helpers/generated-bash-lint'; const ENV_HOSTS = ALL_HOST_CONFIGS.filter(h => h.usesEnvVars); const FORBIDDEN = /^(?:\/bin\/|\/browse|\/design|\/gstack-)/; -const PRELUDE_LINE = /^\[ -d "\$\{GSTACK_ROOT:-\/-\}\/bin" \]|^(?:GSTACK_(?:BIN|BROWSE|DESIGN|MAKE_PDF)=\$GSTACK_ROOT\/\S+ ?)+$|^[BD]=\$GSTACK_ROOT\//; +const PRELUDE_LINE = /^\[ -d "\$\{GSTACK_ROOT:-\/-\}\/bin" \]|^(?:GSTACK_(?:BIN|BROWSE|DESIGN|MAKE_PDF)=\$GSTACK_ROOT\/\S+ ?)+$|^[BDP]=\$GSTACK_ROOT\//; const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-env-fences-')); const renderDir = path.join(tmp, 'render'); @@ -70,8 +71,10 @@ function fenceProblems(fence: Fence): string[] { if (/\$\{?GSTACK_(?:ROOT|BIN|BROWSE|DESIGN|MAKE_PDF)\b/.test(fence.body) && !assigned('GSTACK_ROOT')) problems.push(`${where} uses GSTACK_* without resolving GSTACK_ROOT`); if (/\$\{?B\b/.test(fence.body) && !assigned('B')) problems.push(`${where} uses $B without deriving it`); if (/\$\{?D\b/.test(fence.body) && !assigned('D')) problems.push(`${where} uses $D without deriving it`); + if (/\$\{?P\b/.test(fence.body) && !/(?:^|[\s;&|(])P=/m.test(fence.body)) problems.push(`${where} uses $P without deriving it`); if (fence.body.includes('gstack: no install found')) { - const bytes = Buffer.byteLength(fence.body.split('\n').filter(l => PRELUDE_LINE.test(l)).join('\n') + '\n'); + // MAKE_PDF_OVERRIDE is the one fixed segment outside the budget (E2). + const bytes = Buffer.byteLength(fence.body.split('\n').filter(l => PRELUDE_LINE.test(l)).join('\n').replace(MAKE_PDF_OVERRIDE, '') + '\n'); if (bytes > PRELUDE_BYTE_BUDGET) problems.push(`${where} prelude is ${bytes} bytes (budget ${PRELUDE_BYTE_BUDGET})`); if (fence.body.split('gstack: no install found').length !== 2) problems.push(`${where} carries the prelude more than once`); } @@ -84,7 +87,7 @@ function sh(script: string, cwd: string, env: Record) { return spawnSync('bash', ['-uc', script], { cwd, encoding: 'utf8', timeout: 10_000, env: { PATH: process.env.PATH ?? '/usr/bin:/bin', ...env } }); } function mkroot(dir: string) { - for (const sub of ['bin', 'lib', 'browse/dist', 'design/dist']) fs.mkdirSync(path.join(dir, sub), { recursive: true }); + for (const sub of ['bin', 'lib', 'browse/dist', 'design/dist', 'make-pdf/dist']) fs.mkdirSync(path.join(dir, sub), { recursive: true }); return dir; } @@ -95,13 +98,13 @@ beforeAll(async () => { afterAll(() => fs.rmSync(tmp, { recursive: true, force: true })); describe('C1: every env-var host fence resolves its own runtime paths', () => { - test('each fence that uses $GSTACK_*, $B or $D assigns them in the same fence, within the byte budget', () => { + test('each fence that uses $GSTACK_*, $B, $D or $P assigns them in the same fence, within the byte budget', () => { const problems: string[] = []; let checked = 0; for (const host of ENV_HOSTS) { for (const file of renderedDocs(renderDir, host.hostSubdir)) { for (const fence of bashFences(fs.readFileSync(file, 'utf8'), path.relative(renderDir, file))) { - if (!/\$\{?(?:GSTACK_(?:ROOT|BIN|BROWSE|DESIGN|MAKE_PDF)|B|D)\b/.test(fence.body)) continue; + if (!/\$\{?(?:GSTACK_(?:ROOT|BIN|BROWSE|DESIGN|MAKE_PDF)|B|D|P)\b/.test(fence.body)) continue; checked++; problems.push(...fenceProblems(fence)); } @@ -234,6 +237,7 @@ describe('INV-3: every rendered fence parses, runs its prelude and passes the li for (const host of ENV_HOSTS) { const root = mkroot(path.join(home, host.name === 'codex' ? '.codex/skills/gstack' : host.globalRoot)); for (const tool of ['browse', 'design']) fs.writeFileSync(path.join(root, tool, 'dist', tool), `#!/bin/sh\necho "STUB_${tool}"\n`, { mode: 0o755 }); + fs.writeFileSync(path.join(root, 'make-pdf', 'dist', 'pdf'), '#!/bin/sh\necho "STUB_pdf"\n', { mode: 0o755 }); const preludes = new Set(); for (const file of renderedDocs(renderDir, host.hostSubdir)) { for (const fence of bashFences(fs.readFileSync(file, 'utf8'))) { @@ -244,8 +248,8 @@ describe('INV-3: every rendered fence parses, runs its prelude and passes the li expect(preludes.size).toBeGreaterThan(0); for (const prelude of preludes) { ran++; - const vars = [...new Set([...prelude.matchAll(/(?:^|\s)(GSTACK_[A-Z_]+|[BD])=/gm)].map(m => m[1]))]; - const consumers = vars.filter(v => v === 'B' || v === 'D').map(v => `"$${v}"`).join('\n'); + const vars = [...new Set([...prelude.matchAll(/(?:^|\s)(GSTACK_[A-Z_]+|[BDP])=/gm)].map(m => m[1]))]; + const consumers = vars.filter(v => v === 'B' || v === 'D' || v === 'P').map(v => `"$${v}"`).join('\n'); const script = `${prelude}\n${vars.map(v => `printf '%s=%s\\n' ${v} "$${v}"`).join('\n')}\n${consumers}`; const r = spawnSync('env', ['-i', `HOME=${home}`, 'PATH=/usr/bin:/bin', 'bash', '-uc', script], { cwd: w, encoding: 'utf8', timeout: 10_000 }); if (r.status !== 0) { problems.push(`${host.name}: exit ${r.status}: ${r.stderr.trim()}\n${prelude}`); continue; } @@ -253,8 +257,8 @@ describe('INV-3: every rendered fence parses, runs its prelude and passes the li const value = line.slice(line.indexOf('=') + 1); if (!value || FORBIDDEN.test(value)) problems.push(`${host.name}: ${line}`); } - for (const v of vars.filter(x => x === 'B' || x === 'D')) { - if (!r.stdout.includes(`STUB_${v === 'B' ? 'browse' : 'design'}`)) problems.push(`${host.name}: $${v} did not run the stub binary`); + for (const v of vars.filter(x => x === 'B' || x === 'D' || x === 'P')) { + if (!r.stdout.includes(`STUB_${{ B: 'browse', D: 'design', P: 'pdf' }[v]}`)) problems.push(`${host.name}: $${v} did not run the stub binary`); } } } @@ -302,3 +306,49 @@ describe('INV-3: negative controls (planted bad fences fail each check)', () => expect(fenceProblems({ file: 'x', line: 1, body: twice })).toContain('x:1 carries the prelude more than once'); }); }); + +// E2: make-pdf's $P comes from one resolver (binaryAssignment 'make-pdf') for +// the readiness check and every later block, honoring MAKE_PDF_BIN. +describe('E2: make-pdf $P on env-var hosts', () => { + const pdfStub = (file: string, label: string) => { + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, `#!/bin/sh\necho "${label} $*"\n`, { mode: 0o755 }); + }; + + test('readiness and a later block resolve the same binary, MAKE_PDF_BIN wins, paths with spaces work', () => { + const w = fs.mkdtempSync(path.join(tmp, 'pdf with spaces-')); + const home = path.join(w, 'home dir'); + const root = mkroot(path.join(w, 'gstack root')); + pdfStub(path.join(root, 'make-pdf', 'dist', 'pdf'), 'ROOT_PDF'); + const dev = path.join(w, 'dev build', 'pdf'); + pdfStub(dev, 'DEV_PDF'); + const c = { ...ctx('codex'), skillName: 'make-pdf' }; + const readiness = bashFences(insertRuntimePreludes(generateMakePdfSetup(c), c))[0].body; + const later = fencePrelude(c, '"$P" generate letter.md') + '\n"$P" generate letter.md'; + const env = (extra: Record = {}) => + ['-i', `HOME=${home}`, 'PATH=/usr/bin:/bin', `GSTACK_ROOT=${root}`, ...Object.entries(extra).map(([k, v]) => `${k}=${v}`)]; + const run = (script: string, extra?: Record) => + spawnSync('env', [...env(extra), 'bash', '-uc', script], { cwd: w, encoding: 'utf8', timeout: 10_000 }); + + let r = run(readiness); + expect(r.stdout, r.stderr).toContain(`MAKE_PDF_READY: ${root}/make-pdf/dist/pdf`); + r = run(later); + expect(r.stdout.trim(), r.stderr).toBe('ROOT_PDF generate letter.md'); + + r = run(readiness, { MAKE_PDF_BIN: dev }); + expect(r.stdout, r.stderr).toContain(`MAKE_PDF_READY: ${dev}`); + r = run(later, { MAKE_PDF_BIN: dev }); + expect(r.stdout.trim(), r.stderr).toBe('DEV_PDF generate letter.md'); + + // A MAKE_PDF_BIN that is not executable falls back to the install, in both places. + r = run(later, { MAKE_PDF_BIN: path.join(w, 'missing pdf') }); + expect(r.stdout.trim(), r.stderr).toBe('ROOT_PDF generate letter.md'); + }); + + test('Claude keeps its repo-local-first probe with MAKE_PDF_BIN first; only the comment changed', () => { + const block = bashFences(generateMakePdfSetup({ ...ctx('claude'), skillName: 'make-pdf' }))[0].body; + expect(block).toContain(binaryAssignment(ctx('claude'), 'make-pdf')); + expect(block.indexOf('MAKE_PDF_BIN')).toBeLessThan(block.indexOf('_ROOT/')); + expect(block).not.toContain('available as $P in subsequent blocks'); + }); +}); From ec35833218b4cde8879813f21082ed2109650885 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sun, 4 Oct 2026 17:53:29 +0000 Subject: [PATCH 5/5] E5: GSTACK_DESIGN_IMAGE_MODEL override; the live model check always tests the defaults The image_generation tool model gains an env override validated against a conservative gpt-image- pattern; a bad value fails before any request and the error names the variable. Pairing and rejection hints name the tool model actually requested. design/scripts/live-model-check.ts now ignores GSTACK_DESIGN_MODEL and GSTACK_DESIGN_IMAGE_MODEL (and says so), so an override cannot mask a retirement of a default, and its exit 2 (no key) says nothing was checked. --- design/scripts/live-model-check.ts | 29 ++++++---- design/src/models.ts | 50 ++++++++++++++---- design/test/image-gen-pairing.test.ts | 76 +++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 22 deletions(-) diff --git a/design/scripts/live-model-check.ts b/design/scripts/live-model-check.ts index e887afc06..111a18587 100644 --- a/design/scripts/live-model-check.ts +++ b/design/scripts/live-model-check.ts @@ -1,31 +1,39 @@ /** - * Opt-in live check of the design binary's OpenAI defaults (#2807). Not part - * of any test lane: it spends real tokens (one low-quality 1024x1024 image, - * one short vision answer). + * Live check of the design binary's OpenAI defaults (#2807, E5). It spends + * real tokens (one low-quality 1024x1024 image, one short vision answer). * * OPENAI_API_KEY=... bun run design/scripts/live-model-check.ts * * Sends exactly the request bodies the design commands send (built by - * design/src/models.ts), honoring GSTACK_DESIGN_MODEL when set. Prints the - * models requested and returned, HTTP status, token usage and PASS/FAIL per - * call — never the key, and any key-shaped text in an error body is redacted. - * Exit 0 both pass, 1 any fail, 2 OPENAI_API_KEY unset. + * design/src/models.ts) with the DEFAULT models: GSTACK_DESIGN_MODEL and + * GSTACK_DESIGN_IMAGE_MODEL are ignored (and named when set), so an override + * cannot mask a retirement of a default. Prints the models requested and + * returned, HTTP status, token usage and PASS/FAIL per call — never the key, + * and any key-shaped text in an error body is redacted. + * Exit 0 both pass, 1 any fail, 2 OPENAI_API_KEY unset: a skip that checked + * nothing, which a caller must record as skipped with no coverage credit. */ import { + DESIGN_IMAGE_MODEL_ENV, DESIGN_MODEL_ENV, - IMAGE_TOOL_MODEL, designModel, imageRequestBody, + imageToolModel, visionRequestBody, } from "../src/models"; import { receiptedFetch } from "../src/receipted-fetch"; const TINY_PNG = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkAAIAAAoAAv/lxKUAAAAASUVORK5CYII="; +const ignored = [DESIGN_MODEL_ENV, DESIGN_IMAGE_MODEL_ENV].filter((name) => process.env[name]?.trim()); +for (const name of ignored) delete process.env[name]; +console.log(`testing defaults: image=${designModel("image")} tool=${imageToolModel()} vision=${designModel("vision")}` + + (ignored.length ? ` (ignored: ${ignored.join(", ")})` : "")); + const key = process.env.OPENAI_API_KEY?.trim(); if (!key) { - console.log("SKIP: OPENAI_API_KEY is not set"); + console.log("SKIP: OPENAI_API_KEY is not set; the design model defaults were not checked (no coverage)"); process.exit(2); } @@ -56,7 +64,6 @@ async function post(payloadClass: string, url: string, body: string, timeoutMs: const report = (fields: Record) => console.log(Object.entries(fields).map(([k, v]) => `${k}=${typeof v === "string" ? v : JSON.stringify(v)}`).join(" ")); -console.log(`${DESIGN_MODEL_ENV}=${process.env[DESIGN_MODEL_ENV]?.trim() || "(unset, defaults)"}`); let failed = false; let image = TINY_PNG; @@ -78,7 +85,7 @@ let image = TINY_PNG; report({ call: "image", result: pass ? "PASS" : "FAIL", status: r.status, ms: r.ms, requested_model: designModel("image"), returned_model: r.json?.model ?? null, - tool_model: IMAGE_TOOL_MODEL, image_bytes: pass ? Buffer.from(item.result, "base64").length : 0, + tool_model: imageToolModel(), image_bytes: pass ? Buffer.from(item.result, "base64").length : 0, ...(pass ? {} : { error: redact(r.json?.error?.message ?? r.text) }), }); failed ||= !pass; diff --git a/design/src/models.ts b/design/src/models.ts index 081709eb8..5fed94daf 100644 --- a/design/src/models.ts +++ b/design/src/models.ts @@ -8,6 +8,9 @@ * analysis — check, diff, memory, design-to-code, evolve's analysis step. * * `GSTACK_DESIGN_MODEL` (named like `GSTACK_CODEX_MODEL`) replaces both. + * `GSTACK_DESIGN_IMAGE_MODEL` replaces the image_generation tool's model; it + * must look like a gpt-image model name (IMAGE_TOOL_MODEL_PATTERN) and a bad + * value fails before any request, naming the variable. * * Defaults (checked against OpenAI's model and deprecation pages, 2026-10): * gpt-5.5 lists Chat Completions, image input and the Responses @@ -24,7 +27,8 @@ * The limit is a cap, not a charge — only generated tokens are billed. * * `bun run design/scripts/live-model-check.ts` probes these defaults with a - * real key (opt-in, not in any test lane). + * real key. It always tests the defaults (overrides are ignored), so an + * override cannot mask a retirement. */ export const DESIGN_MODEL_ENV = "GSTACK_DESIGN_MODEL"; @@ -36,9 +40,29 @@ export const DEFAULT_DESIGN_MODELS: Readonly> = { vision: "gpt-5.5", }; -/** The image model the hosted image_generation tool runs; never omitted (the API default is gpt-image-1). */ +/** The image model the hosted image_generation tool runs by default; never omitted (the API default is gpt-image-1). */ export const IMAGE_TOOL_MODEL = "gpt-image-2"; +export const DESIGN_IMAGE_MODEL_ENV = "GSTACK_DESIGN_IMAGE_MODEL"; + +/** Conservative shape for an image tool model override: gpt-image-[-variant]. */ +export const IMAGE_TOOL_MODEL_PATTERN = /^gpt-image-[0-9][a-z0-9.-]{0,31}$/; + +/** + * The image_generation tool's model: `GSTACK_DESIGN_IMAGE_MODEL` when set, + * else IMAGE_TOOL_MODEL. Throws, naming the variable, when the override does + * not match IMAGE_TOOL_MODEL_PATTERN. + */ +export function imageToolModel(env: Record = process.env): string { + const override = env[DESIGN_IMAGE_MODEL_ENV]?.trim(); + if (!override) return IMAGE_TOOL_MODEL; + if (!IMAGE_TOOL_MODEL_PATTERN.test(override)) { + throw new Error(`${DESIGN_IMAGE_MODEL_ENV}=${JSON.stringify(override.slice(0, 80))} is not a gpt-image model name ` + + `(expected something like ${IMAGE_TOOL_MODEL}); fix it or unset it to use ${IMAGE_TOOL_MODEL}`); + } + return override; +} + /** Reasoning allowance added to every vision call's visible-answer budget. */ export const REASONING_HEADROOM_TOKENS = 8192; @@ -47,10 +71,10 @@ export function designModel(call: DesignCall, env: Record = {}, ): string { const model = designModel("image"); - const problem = imagePairingProblem(model); + const toolModel = imageToolModel(); + const problem = imagePairingProblem(model, toolModel); if (problem) throw new Error(problem); return JSON.stringify({ model, input, ...extra, - tools: [{ type: "image_generation", model: IMAGE_TOOL_MODEL, size: tool.size, quality: tool.quality }], + tools: [{ type: "image_generation", model: toolModel, size: tool.size, quality: tool.quality }], }); } @@ -100,8 +125,11 @@ export function modelRejectionHint(status: number, body: string, call: DesignCal if (!/\bmodel\b|unsupported (?:parameter|value)|not supported/i.test(body)) return ""; const model = designModel(call); const source = process.env[DESIGN_MODEL_ENV]?.trim() ? `${DESIGN_MODEL_ENV}=${model}` : `the default model ${model}`; - const what = call === "image" - ? `image-generation (Responses API, tool model ${IMAGE_TOOL_MODEL})` - : "vision (Chat Completions)"; - return ` — OpenAI rejected ${source} for this ${what} call; set ${DESIGN_MODEL_ENV} to a model your account can use for it`; + if (call !== "image") { + return ` — OpenAI rejected ${source} for this vision (Chat Completions) call; set ${DESIGN_MODEL_ENV} to a model your account can use for it`; + } + const toolOverride = process.env[DESIGN_IMAGE_MODEL_ENV]?.trim(); + const tool = toolOverride ? `${DESIGN_IMAGE_MODEL_ENV}=${toolOverride}` : `the default tool model ${IMAGE_TOOL_MODEL}`; + return ` — OpenAI rejected ${source} or ${tool} for this image-generation (Responses API) call; ` + + `set ${DESIGN_MODEL_ENV} or ${DESIGN_IMAGE_MODEL_ENV} to models your account can use for it`; } diff --git a/design/test/image-gen-pairing.test.ts b/design/test/image-gen-pairing.test.ts index 8e72ded15..dba74642a 100644 --- a/design/test/image-gen-pairing.test.ts +++ b/design/test/image-gen-pairing.test.ts @@ -1,13 +1,17 @@ import { afterEach, describe, expect, test } from "bun:test"; +import { spawnSync } from "child_process"; import fs from "fs"; +import os from "os"; import path from "path"; import { DEFAULT_DESIGN_MODELS, + DESIGN_IMAGE_MODEL_ENV, DESIGN_MODEL_ENV, IMAGE_TOOL_MODEL, imagePairingProblem, imageRequestBody, + imageToolModel, } from "../src/models"; // Tripwire for the image_generation orchestrator/tool pairing. @@ -33,9 +37,12 @@ const sources = fs .map((f) => ({ rel: `src/${f}`, body: fs.readFileSync(path.join(DESIGN_SRC, f), "utf-8") })); const savedOverride = process.env[DESIGN_MODEL_ENV]; +const savedImageOverride = process.env[DESIGN_IMAGE_MODEL_ENV]; afterEach(() => { if (savedOverride === undefined) delete process.env[DESIGN_MODEL_ENV]; else process.env[DESIGN_MODEL_ENV] = savedOverride; + if (savedImageOverride === undefined) delete process.env[DESIGN_IMAGE_MODEL_ENV]; + else process.env[DESIGN_IMAGE_MODEL_ENV] = savedImageOverride; }); describe("design image-generation tool/orchestrator pairing (#1771, #2807)", () => { @@ -71,3 +78,72 @@ describe("design image-generation tool/orchestrator pairing (#1771, #2807)", () }); } }); + +// E5: users switch the tool model without editing code; a bad value fails +// before any request, naming the variable; the live check always tests the +// defaults, and a missing key is a skip, never a pass. +describe("GSTACK_DESIGN_IMAGE_MODEL override (E5)", () => { + const body = () => JSON.parse(imageRequestBody("x", { size: "1024x1024", quality: "low" })); + + test("a gpt-image model name replaces the tool model; blank means the default", () => { + delete process.env[DESIGN_MODEL_ENV]; + for (const model of ["gpt-image-1.5", "gpt-image-2-mini", "gpt-image-3"]) { + process.env[DESIGN_IMAGE_MODEL_ENV] = ` ${model} `; + expect(body().tools[0].model).toBe(model); + } + process.env[DESIGN_IMAGE_MODEL_ENV] = " "; + expect(body().tools[0].model).toBe(IMAGE_TOOL_MODEL); + expect(imageToolModel({})).toBe(IMAGE_TOOL_MODEL); + }); + + test("anything else is refused before a request, naming the variable", () => { + for (const bad of ["dall-e-3", "gpt-5.5", "GPT-IMAGE-2", "gpt-image-2 --x", 'gpt-image-2"}', "gpt-image-", `gpt-image-2${"a".repeat(40)}`]) { + process.env[DESIGN_IMAGE_MODEL_ENV] = bad; + expect(() => body()).toThrow(`${DESIGN_IMAGE_MODEL_ENV}=`); + expect(() => imageToolModel()).toThrow(`unset it to use ${IMAGE_TOOL_MODEL}`); + } + }); + + test("the pairing refusal names the tool model actually requested", () => { + process.env[DESIGN_MODEL_ENV] = "gpt-4o"; + process.env[DESIGN_IMAGE_MODEL_ENV] = "gpt-image-1.5"; + expect(() => body()).toThrow("cannot drive the gpt-image-1.5 image_generation tool"); + }); +}); + +describe("live-model-check always tests the defaults (E5)", () => { + const script = path.join(import.meta.dir, "..", "scripts", "live-model-check.ts"); + const run = (env: Record) => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "design-live-check-")); + try { + return spawnSync(process.execPath, ["run", script], { + encoding: "utf8", timeout: 60_000, + env: { PATH: process.env.PATH ?? "/usr/bin:/bin", HOME: home, GSTACK_HOME: home, ...env }, + }); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }; + const overrides = { [DESIGN_MODEL_ENV]: "gpt-4.1", [DESIGN_IMAGE_MODEL_ENV]: "gpt-image-1" }; + + test("no key: exit 2 with a no-coverage SKIP line, after naming the ignored overrides", () => { + const r = run(overrides); + expect(r.status).toBe(2); + expect(r.stdout.trim().split("\n")).toEqual([ + `testing defaults: image=${DEFAULT_DESIGN_MODELS.image} tool=${IMAGE_TOOL_MODEL} vision=${DEFAULT_DESIGN_MODELS.vision} (ignored: ${DESIGN_MODEL_ENV}, ${DESIGN_IMAGE_MODEL_ENV})`, + "SKIP: OPENAI_API_KEY is not set; the design model defaults were not checked (no coverage)", + ]); + }); + + test("with a key, the requests carry the defaults even when overrides are set", () => { + // An unreachable proxy fails both calls locally: no request leaves the machine. + const r = run({ ...overrides, OPENAI_API_KEY: "sk-test-not-a-real-key", HTTPS_PROXY: "http://127.0.0.1:9", https_proxy: "http://127.0.0.1:9" }); + expect(r.status).toBe(1); + const image = r.stdout.split("\n").find((l) => l.startsWith("call=image")) ?? ""; + const vision = r.stdout.split("\n").find((l) => l.startsWith("call=vision")) ?? ""; + expect(image).toContain(`requested_model=${DEFAULT_DESIGN_MODELS.image}`); + expect(image).toContain(`tool_model=${IMAGE_TOOL_MODEL}`); + expect(vision).toContain(`requested_model=${DEFAULT_DESIGN_MODELS.vision}`); + expect(r.stdout).not.toContain("sk-test-not-a-real-key"); + }); +});