From e6850ceec3fc1db1ac4f99c20db2f98305290ba1 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 16:04:55 +0000 Subject: [PATCH] fix(install): keep each workflow's Bun on --check, refuse after a failed setup, launch only the admitting record's Bun (GST-1233 review) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-capy-install | 6 ++++-- bin/gstack-install-verdict.sh | 4 +++- changelog.d/gst-1233-verdict-union.md | 2 +- docs/capy-install.md | 7 ++++++- lib/install-verdict.ts | 5 +++-- lib/pregate/runtime.ts | 4 +++- test/bun-resolve.test.ts | 29 +++++++++++++++++++++++++++ test/gstack-capy-install.test.ts | 8 ++++++++ test/install-verdict.test.ts | 6 ++++++ 9 files changed, 63 insertions(+), 8 deletions(-) diff --git a/bin/gstack-capy-install b/bin/gstack-capy-install index d5343f2e8..26b9e6668 100755 --- a/bin/gstack-capy-install +++ b/bin/gstack-capy-install @@ -163,8 +163,10 @@ PROJECT_REAL=""; [ -n "$PROJECT" ] && PROJECT_REAL=$(cd "$PROJECT" && pwd -P) && VERDICT=$(gstack_install_verdict_path "$GSTACK_STATE_ROOT" "$ROOT_REAL" "$PROJECT_REAL") REC_BUN="" REC_VER="" REC_RULE="" REC_BUN_INSTALL="" REC_FOR="" if [ -f "$VERDICT" ]; then - REC_BUN=$(gstack_install_verdict_field "$VERDICT" bun_path) REC_VER=$(gstack_install_verdict_field "$VERDICT" bun_version) - REC_RULE=$(gstack_install_verdict_field "$VERDICT" bun_rule) REC_BUN_INSTALL=$(gstack_install_verdict_field "$VERDICT" bun_install) + _rec=$VERDICT + case "$FOR" in ''|*[!A-Za-z0-9._-]*) ;; *) [ "$PROFILE" -eq 0 ] && [ "$(gstack_install_verdict_field "${VERDICT%.json}.for/$FOR.json" ok 2>/dev/null)" = true ] && _rec="${VERDICT%.json}.for/$FOR.json" ;; esac + REC_BUN=$(gstack_install_verdict_field "$_rec" bun_path) REC_VER=$(gstack_install_verdict_field "$_rec" bun_version) + REC_RULE=$(gstack_install_verdict_field "$_rec" bun_rule) REC_BUN_INSTALL=$(gstack_install_verdict_field "$_rec" bun_install) REC_FOR=$(gstack_install_verdict_field "$VERDICT" for) fi # A reviewer's machine (--for review-voice) asked to run a workflow upgrades diff --git a/bin/gstack-install-verdict.sh b/bin/gstack-install-verdict.sh index 8cfa6854d..5714bf60a 100644 --- a/bin/gstack-install-verdict.sh +++ b/bin/gstack-install-verdict.sh @@ -192,9 +192,11 @@ gstack_install_admission() { _gstack_ia_check # GST-1233: each install also keeps its own verdict per workflow, so when a # later install for another workflow replaced the shared one, this - # workflow's own record still admits it. + # workflow's own record still admits it; never after a failed ./setup, which + # changed the checkout every record certified. _ia_own="${file%.json}.for/$skill.json" case "$skill" in ''|*[!A-Za-z0-9._-]*) _ia_own="" ;; esac + case "$trailer" in "gstack: fail setup"*) _ia_own="" ;; esac if [ -n "$detail" ] && [ -n "$_ia_own" ] && [ -f "$_ia_own" ]; then _ia_m_file=$file _ia_m_detail=$detail _ia_m_fix=$fix _ia_m_code=$code _ia_m_status=$_ia_status _ia_m_trailer=$trailer file=$_ia_own _ia_status=ok diff --git a/changelog.d/gst-1233-verdict-union.md b/changelog.d/gst-1233-verdict-union.md index fd40a6b03..d40746af5 100644 --- a/changelog.d/gst-1233-verdict-union.md +++ b/changelog.d/gst-1233-verdict-union.md @@ -1,3 +1,3 @@ ### Fixed -- **A child's install for another workflow no longer drops /ship's install verdict on a shared machine (GST-1233).** When a /ship docs child ran `install.sh --for document-release` with a different Bun on PATH, or its install failed, it replaced the shared install verdict, and the parent's finish pregate refused with `INSTALL_UNVERIFIED` until someone reinstalled `--for ship`. Each workflow install now also writes that workflow's own verdict beside the shared one (`.for/.json`), and workflow admission and the pregate fall back to it with the same checks. A new HEAD still invalidates both. +- **A child's install for another workflow no longer drops /ship's install verdict on a shared machine (GST-1233).** When a /ship docs child ran `install.sh --for document-release` with a different Bun on PATH, or its install check failed, it replaced the shared install verdict, and the parent's finish pregate refused with `INSTALL_UNVERIFIED` until someone reinstalled `--for ship`. Each workflow install now also writes that workflow's own verdict beside the shared one (`.for/.json`), and workflow admission and the pregate fall back to it with the same checks. A new HEAD or a failed `./setup` still invalidates every record, `--check --for ship` keeps ship's own recorded Bun, and the pregate never launches a Bun its admitting record didn't certify. The shared launch file is a separate follow-up (GST-1239). diff --git a/docs/capy-install.md b/docs/capy-install.md index d499a78c7..a1da5fd13 100644 --- a/docs/capy-install.md +++ b/docs/capy-install.md @@ -337,7 +337,12 @@ keeps the lease. `bin/gstack-lease list` shows `alive|idle|dead|stale`, and workflow's own record with the same checks, so a later install for another workflow on a shared machine (a /ship docs child resolving another Bun, or a failed child install) never drops /ship's admission (GST-1233). A new HEAD - still invalidates both. Admission refuses a workflow whose + still invalidates both, and a failed `./setup` (`gstack: fail setup`) + invalidates every workflow's record. `--check --for ` keeps the + Bun that workflow's own record holds. The launch file and launch directory + stay shared per checkout and project (GST-1239), so the pregate launches + from the recorded Bun's own directory when the launch directory's `bun` + resolves to another one. Admission refuses a workflow whose running Bun is not the one the verdict recorded ([BUN_RESOLUTION_DRIFT](troubleshooting.md#bun-resolution-drift)), and so do the pregate's lanes and guards, which also refuse an invalid verdict diff --git a/lib/install-verdict.ts b/lib/install-verdict.ts index 1e05e3df3..60f787cd2 100644 --- a/lib/install-verdict.ts +++ b/lib/install-verdict.ts @@ -148,12 +148,13 @@ export function verdictProblem(skill: string, gstackRoot: string, env: Env, bunP /** * The verdict file that admits `skill`: `file` itself, else the workflow's own record * .for/.json that every install also writes, so a later install for another - * workflow never drops this one (GST-1233). When neither admits, `file` with its problem. + * workflow never drops this one (GST-1233), except after a failed ./setup, which changed the + * checkout every record certified. When neither admits, `file` with its problem. */ export function admittedVerdict(skill: string, src: string, file: string, project: string | null, bunPath: string, bunVersion: string): { file: string; problem: VerdictProblem | null } { const problem = verdictFileProblem(file, skill, src, project, bunPath, bunVersion); const own = file.replace(/\.json$/, `.for/${skill}.json`); - if (problem && /^[A-Za-z0-9._-]+$/.test(skill) && fs.existsSync(own) && !verdictFileProblem(own, skill, src, project, bunPath, bunVersion)) return { file: own, problem: null }; + if (problem && /^[A-Za-z0-9._-]+$/.test(skill) && !readInstallVerdict(file)?.trailer?.startsWith('gstack: fail setup') && fs.existsSync(own) && !verdictFileProblem(own, skill, src, project, bunPath, bunVersion)) return { file: own, problem: null }; return { file, problem }; } diff --git a/lib/pregate/runtime.ts b/lib/pregate/runtime.ts index 82386438a..9985bafc8 100644 --- a/lib/pregate/runtime.ts +++ b/lib/pregate/runtime.ts @@ -181,7 +181,9 @@ export function launchRuntime(repoRoot: string, env: NodeJS.ProcessEnv): LaunchR const now = fs.existsSync(recorded!.bun_path) ? bunVersionOf(recorded!.bun_path, env) : ''; const why = !now ? 'is gone' : now !== recorded!.bun_version ? `now reports ${now}` : !accept(now) ? 'no longer satisfies the project pins' : ''; if (why) return refuse('BUN_RESOLUTION_DRIFT', `recorded bun ${recorded!.bun_version} at ${recorded!.bun_path} ${why}`, 're-run the Capy skill installer with --check (scripts/install.sh --check --for ship) and follow its fix'); - const launch = recorded!.launch_path && fs.existsSync(recorded!.launch_path) ? recorded!.launch_path : path.dirname(recorded!.bun_path); + let launchBun = ''; + try { launchBun = fs.realpathSync(path.join(recorded!.launch_path, 'bun')); } catch { /* no Bun in the launch dir */ } + const launch = recorded!.launch_path && fs.existsSync(recorded!.launch_path) && (!launchBun || launchBun === recorded!.bun_path) ? recorded!.launch_path : path.dirname(recorded!.bun_path); return result(launch, now, `bun ${now} at ${recorded!.bun_path} (recorded by the install verdict)`, 'ok', ''); } } diff --git a/test/bun-resolve.test.ts b/test/bun-resolve.test.ts index 9af139cd9..ade1fe074 100644 --- a/test/bun-resolve.test.ts +++ b/test/bun-resolve.test.ts @@ -138,4 +138,33 @@ describe('one Bun resolution contract, two twins', () => { expect(viaLaunch.dir).toBe(launch); expect(viaLaunch.env.PATH?.split(path.delimiter)[0]).toBe(launch); }); + + // GST-1233: a docs child's install replaced the shared verdict and republished the shared launch + // directory with its own Bun; the finish pregate admits from ship's own record and launches ship's Bun. + test("launchRuntime admits /ship from its own record when another workflow's install replaced the shared verdict, and never launches that install's Bun", () => { + const l = layout({ storeVersions: ['1.4.4'] }); + const { installVerdictPath } = require('../lib/install-verdict') as typeof import('../lib/install-verdict'); + const checkout = fs.realpathSync(ROOT); + const file = installVerdictPath(checkout, l.state, null); + const head = spawnSync('git', ['-C', ROOT, 'rev-parse', 'HEAD'], { encoding: 'utf8', timeout: 10_000 }).stdout.trim(); + const childBun = path.join(bunStore(l.state), '1.4.4/bun'); + const launch = file.replace(/\.json$/, '.launch'); + fs.mkdirSync(launch, { recursive: true }); + fs.symlinkSync(childBun, path.join(launch, 'bun')); + const record = { schema_version: 2, checkout, project: '', head, pins_planning_only: false, ok: true, min_revision: '', launch_path: launch, time: '' }; + fs.writeFileSync(file, JSON.stringify({ ...record, for: 'document-release', required: ['claude'], trailer: 'gstack: ok for=document-release', bun_path: fs.realpathSync(childBun), bun_version: '1.4.4' })); + const refused = launchRuntime(ROOT, l.env); + expect(refused.refusal).toMatchObject({ code: 'INSTALL_UNVERIFIED' }); + expect(refused.refusal?.detail).toContain('certified for=document-release'); + const own = file.replace(/\.json$/, '.for/ship.json'); + fs.mkdirSync(path.dirname(own), { recursive: true }); + fs.writeFileSync(own, JSON.stringify({ ...record, for: 'ship', required: ['claude', 'codex', 'patch'], trailer: 'gstack: ok for=ship', bun_path: REAL_BUN, bun_version: Bun.version })); + const r = launchRuntime(ROOT, l.env); + expect(r.refusal).toBeNull(); + expect(r.verdict).toBe('ok'); + expect(r.label).toBe(`bun ${Bun.version} at ${REAL_BUN} (recorded by the install verdict)`); + expect(r.dir).toBe(path.dirname(REAL_BUN)); + fs.writeFileSync(file, JSON.stringify({ ...JSON.parse(fs.readFileSync(file, 'utf8')), ok: false, trailer: 'gstack: fail setup — ./setup failed' })); + expect(launchRuntime(ROOT, l.env).refusal?.detail).toBe('install verdict: the install verdict says: gstack: fail setup — ./setup failed'); + }); }); diff --git a/test/gstack-capy-install.test.ts b/test/gstack-capy-install.test.ts index de314b8b0..9809cae64 100644 --- a/test/gstack-capy-install.test.ts +++ b/test/gstack-capy-install.test.ts @@ -423,6 +423,14 @@ describe('gstack-capy-install', () => { expect(JSON.parse(fs.readFileSync(file, 'utf8'))).toMatchObject({ for: 'document-release', bun_path: path.join(childBin, 'bun') }); expect(admits('ship')).toBeNull(); expect(admits('document-release')).toBeNull(); + const shipBun = JSON.parse(fs.readFileSync(file.replace(/\.json$/, '.for/ship.json'), 'utf8')).bun_path; + expect(shipBun).not.toBe(path.join(childBin, 'bun')); + const check = run(f, ['--check', '--for', 'ship'], { STUB_REQUIRED: 'claude codex patch runtime pins' }); + expect(check.status, check.stdout + check.stderr).toBe(0); + expect(JSON.parse(fs.readFileSync(file.replace(/\.json$/, '.for/ship.json'), 'utf8')).bun_path).toBe(shipBun); + expect(JSON.parse(fs.readFileSync(file, 'utf8'))).toMatchObject({ for: 'ship', bun_path: shipBun }); + expect(admits('ship')).toBeNull(); + expect(admits('document-release')).toBeNull(); const failed = run(f, ['--for', 'document-release'], { STUB_REQUIRED: 'claude runtime', STUB_DOCTOR_FAIL: '1' }); expect(failed.status).not.toBe(0); expect(JSON.parse(fs.readFileSync(file, 'utf8'))).toMatchObject({ for: 'document-release', ok: false }); diff --git a/test/install-verdict.test.ts b/test/install-verdict.test.ts index 85b55c39a..3c4bb0050 100644 --- a/test/install-verdict.test.ts +++ b/test/install-verdict.test.ts @@ -164,6 +164,12 @@ describe('install verdict keyed by (checkout, project): both twins', () => { writeVerdict(sr, { project: proj, bun_path: BUN, bun_version: '1.4.2', schema_version: 2, for: 'ship', trailer: 'gstack: ok 9.9.9.9 for=ship' }, own); expect(both('ship', proj, env)).toMatchObject({ refuse: false, status: 'ok' }); expect(bashAdmit('ship', proj, { _GH: sr, ...env }).line).toBe('INSTALL_VERDICT: ok gstack: ok 9.9.9.9 for=ship'); + const setupFailed = { project: proj, bun_path: '/opt/child/bun', bun_version: '1.4.2', schema_version: 2, for: 'document-release', ok: false, trailer: 'gstack: fail setup' }; + writeVerdict(sr, setupFailed); + expect(both('ship', proj, env)).toMatchObject({ refuse: true, status: 'verdict', detail: 'the install verdict says: gstack: fail setup' }); + writeVerdict(sr, { ...setupFailed, trailer: 'gstack: fail codex' }); + expect(both('ship', proj, env)).toMatchObject({ refuse: false, status: 'ok' }); + writeVerdict(sr, { project: proj, bun_path: '/opt/child/bun', bun_version: '1.4.2', schema_version: 2, for: 'document-release', required: ['claude', 'runtime'], trailer: 'gstack: ok 9.9.9.9 for=document-release' }); writeVerdict(sr, { project: proj, bun_path: BUN, bun_version: '1.4.2', schema_version: 2, for: 'ship', head: 'f'.repeat(40) }, own); const stale = both('ship', proj, env); expect(stale).toMatchObject({ refuse: true, status: 'verdict', code: 'INSTALL_UNVERIFIED', detail: without.detail });