From 3f79d816730e8cd3633f31dc20351c0501ad958e Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 31 Oct 2026 19:21:49 -0700 Subject: [PATCH] v1.92.10.0 fix: /ship's scoped review pass runs again and reads the fix range (GST-619) (#3766) * fix: review-dispatch pass accepts the scoped plan; the fix-check runs on its own (GST-619) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: a scoped pass stages its readers on the fix range base (GST-619) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: keep the pinned /ship reviewer prose; the scoped fix-check already runs inside the pass (GST-619) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: a scoped pass gives the fix range to the Red Team; pending Step 11 voices keep the merge base (GST-619) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: the scoped pass's adversarial reader reads the whole branch until Step 11 runs (GST-619) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 11 +++++++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- bin/gstack-review-dispatch | 14 ++++++--- docs/review-policy.md | 5 +-- lib/review-pass.ts | 7 +++-- package.json | 2 +- test/review-pass.test.ts | 57 ++++++++++++++++++++++++++++++++++ 8 files changed, 87 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2b079389..0fe555e1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## [1.92.10.0] - 2026-11-01 + +**/ship's scoped second pass runs again, and its Red Team reviews the fix range while Step 11's voices still read the whole branch.** After a blocking fix on a small range, `gstack-review-plan` plans a scoped pass: one fix-check, the Red Team and the native adversarial pass on the range. `gstack-review-dispatch pass` refused that plan with `BRIEF_RENDER_FAILED: a pass dispatches no fix-check reader`, because a fix-check is not a pass reader. Every scoped pass failed this way, and GEVA-82 rendered its briefs by hand to get through (GST-619). Even past that refusal, the pass would have staged its Red Team on the merge base, because the Capy pass program never passes `--diff-base`. + +### Fixed +- **`gstack-review-dispatch pass` keeps the fix-check out of the pass manifest and prints it as `FIX_CHECK: planned effort=full (Step 3.5 runs it on its own; not in this manifest)`.** A resumed `pass` command prints the line again. /ship's Step 9 text already runs that fix-check inside the scoped pass with `git diff $RANGE_BASE` as its hunks, so the pinned reviewer prose is unchanged. No reader is added or removed, so the pass runs the readers the plan already lists. +- **A scoped pass's Red Team reviews the plan's fix range.** `pass` resolves the range base from the plan's `fix_range` and gives it to the Red Team alone, so it reviews `RANGE_BASE..HEAD` as docs/review-policy.md describes. Until Step 11 has run, the native adversarial pass and the outside voice in a scoped pass are Step 11's own voices, so they keep the merge base and read the whole branch. Once the plan flags carry `--adversarial-ran`, the whole scoped pass reads the range. An explicit `--diff-base` still wins, and a scoped plan without a readable range base is refused. + +### Tests +- `test/review-pass.test.ts`: GEVA-2's invocation (`--extras`, `--pass1-depth standard --fixed-blocking --range-base `) plans `NEXT: scoped`. `pass` exits 0, prints the `FIX_CHECK:` line (and again on resume), and stages the Red Team, the adversarial pass, the outside voice and the audit reader. The Red Team's brief names the full range-base sha, and the outside voice's diff carries the pass-1 lines. With `--adversarial-ran` added, the manifest's diff base is the range base. Both fail on main with `BRIEF_RENDER_FAILED`, and on the previous head of this PR because every reader there read the range. + ## [1.92.9.0] - 2026-11-01 **/freeze can no longer be walked around with paths through folders that don't exist yet (#3623).** /freeze checks every Edit, Write and NotebookEdit path against its folder. The check only resolved a parent folder that already existed and compared the raw text otherwise, so `/../outside/new/x`, `/link-to-outside/new/x`, `/..`, `~/.zshrc` with the working folder inside the boundary, and `C:\proj\src\..\..\other\new\x` were all allowed. Reported by Quinn O'Donnell (#3623, #3624). diff --git a/VERSION b/VERSION index 7c57a3b73..3c7c970e5 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.92.9.0 +1.92.10.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 448bb63e9..b9921251e 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.92.9.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.92.10.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/bin/gstack-review-dispatch b/bin/gstack-review-dispatch index af3d4ac8d..827ba0c5b 100755 --- a/bin/gstack-review-dispatch +++ b/bin/gstack-review-dispatch @@ -161,18 +161,22 @@ function pass(): number { if (text.status !== 0) { process.stdout.write(text.stdout ?? ''); process.stderr.write(text.stderr ?? ''); return EXIT.fail; } const j = plan(true); if (j.status !== 0) { process.stderr.write(j.stderr ?? ''); return EXIT.fail; } - const p = JSON.parse(j.stdout) as { next: { step: string }; dispatches: Dispatch[]; targeted?: { classes: string[] } }; - const planLines = text.stdout.trimEnd().split('\n'); + const p = JSON.parse(j.stdout) as { next: { step: string }; dispatches: Dispatch[]; targeted?: { classes: string[] }; fix_range?: { base: string } | null }; + const fixChecks = p.dispatches.filter(d => d.role === 'fix-check'); + const planLines = [...text.stdout.trimEnd().split('\n'), ...fixChecks.map(d => `FIX_CHECK: planned effort=${d.effort} (Step 3.5 runs it on its own; not in this manifest)`)]; const step = p.next.step; if (step !== 'full' && step !== 'scoped') { process.stdout.write([...planLines, `PASS: none next=${step} (no reviewer pass; follow NEXT)`].join('\n') + '\n'); return EXIT.ok; } - const diffBase = flag('--diff-base') ?? git(['merge-base', base, 'HEAD'])?.trim(); + const rangeBase = step === 'scoped' ? git(['rev-parse', '--verify', '--quiet', `${p.fix_range?.base ?? ''}^{commit}`])?.trim() : undefined; + if (step === 'scoped' && !rangeBase) throw new BriefError('a scoped plan names no readable fix range base'); + const mergeBase = git(['merge-base', base, 'HEAD'])?.trim(); + const diffBase = flag('--diff-base') ?? (planArgs.includes('--adversarial-ran') ? rangeBase : undefined) ?? mergeBase; if (!diffBase) throw new BriefError(`no merge base between ${base} and HEAD`); const root = git(['rev-parse', '--show-toplevel'])?.trim() ?? process.cwd(); const extrasFile = flag('--extras'); const extras = extrasFile ? JSON.parse(readFile(extrasFile, 'extras')!) as Extra[] : []; const m = buildManifest({ pass: id, out, ledger: flag('--ledger') ?? path.join(out, 'review-ledger.jsonl'), base, diffBase, root, checklistRoot: flag('--checklist-root') ?? ROOT, - dispatches: p.dispatches, next: step, planLines, learnings: readFile(flag('--learnings-file'), 'learnings') ?? null, extras, - targetedClasses: p.targeted?.classes, mergeBase: git(['merge-base', base, 'HEAD'])?.trim() }); + dispatches: p.dispatches.filter(d => d.role !== 'fix-check'), next: step, planLines, learnings: readFile(flag('--learnings-file'), 'learnings') ?? null, extras, + targetedClasses: p.targeted?.classes, mergeBase, rangeBase }); fs.mkdirSync(m.out, { recursive: true, mode: 0o700 }); const t = sectionTexts(ROOT); renderStage(m, 'first', t, branchDiff); diff --git a/docs/review-policy.md b/docs/review-policy.md index 0986de1de..283671003 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -271,8 +271,9 @@ the specialists, audits and watchers at the host default too ### Behavior changes - Pass 2 after a blocking fix on a diff below full depth, with a fix range of at - most 150 runtime lines, is a [scoped pass](#scoped-pass): a fix-check, the Red - Team and the native adversarial pass on the range instead of the specialist army. + most 150 runtime lines, is a [scoped pass](#scoped-pass): a fix-check and the Red + Team on the range, plus the native adversarial pass (on the range once Step 11 has + run, otherwise on the whole branch), instead of the specialist army. - A tests-only diff (runtime 0) no longer dispatches the Testing specialist: the core checklist and the gate scan review it. - Every depth dispatches one batched specialist instead of one reader per diff --git a/lib/review-pass.ts b/lib/review-pass.ts index e4187eb0d..52de74ecd 100644 --- a/lib/review-pass.ts +++ b/lib/review-pass.ts @@ -74,7 +74,7 @@ const STAGE_OF: Record = { specialist: 'specialists', 'red-team': export function buildManifest(o: { pass: string; out: string; ledger: string; base: string; diffBase: string; root: string; checklistRoot: string; dispatches: Dispatch[]; next: string; planLines: string[]; learnings?: string | null; extras?: Extra[]; - targetedClasses?: readonly string[]; mergeBase?: string; + targetedClasses?: readonly string[]; mergeBase?: string; rangeBase?: string; }): Manifest { const readers: PassReader[] = []; for (const d of o.dispatches) { @@ -85,7 +85,8 @@ export function buildManifest(o: { const reader = readerKey(role, name); const exclude = d.exclude.length ? diffCommand(d).slice('git diff '.length) : undefined; readers.push({ reader, stage, title: `ship ${o.pass} ${reader}`, state: 'planned', role, name, effort: d.effort, exclude, checklists: d.checklists, - ...(role === 'targeted' ? { classes: [...(o.targetedClasses ?? [])], diff_base: o.mergeBase ?? o.diffBase } : {}) }); + ...(role === 'targeted' ? { classes: [...(o.targetedClasses ?? [])], diff_base: o.mergeBase ?? o.diffBase } : {}), + ...(role === 'red-team' && o.rangeBase ? { diff_base: o.rangeBase } : {}) }); } for (const e of o.extras ?? []) { if (!/^[a-z0-9-]+$/.test(e.name)) throw new BriefError(`an audit reader's name is lowercase letters, digits and dashes: ${e.name}`); @@ -142,7 +143,7 @@ export function renderStage(m: Manifest, stage: Stage, t: SectionTexts, diffFor: continue; } const role = r.role!; - const slots: Slots = { name: r.name, diff_base: m.diff_base, exclude: r.exclude, stack: m.stack, test_fw: m.test_fw, learnings: m.learnings ?? undefined, + const slots: Slots = { name: r.name, diff_base: r.diff_base ?? m.diff_base, exclude: r.exclude, stack: m.stack, test_fw: m.test_fw, learnings: m.learnings ?? undefined, checklists: r.checklists?.map(n => path.join(m.checklist_root, 'review/specialists', `${n}.md`)).join(',') }; if (role === 'specialist') slots.checklist = path.join(m.checklist_root, 'review/specialists', `${r.name}.md`); let findingsSha: string | undefined; diff --git a/package.json b/package.json index 8af4553a5..f3351cf57 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.92.9", + "version": "1.92.10", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/test/review-pass.test.ts b/test/review-pass.test.ts index bedd1df37..e3458effc 100644 --- a/test/review-pass.test.ts +++ b/test/review-pass.test.ts @@ -489,6 +489,63 @@ describe('the Capy skill loader', () => { }); +describe('a scoped pass after a blocker fix (GST-619)', () => { + // Value: protects=the pass command accepts the scoped plan gstack-review-plan writes after a blocking fix, invoked as GEVA-2 ran it: the Red Team reviews the fix range, Step 11's voices still pending keep the whole branch, the fix-check is named outside the manifest for Step 3.5, and a resumed pass names it again; fails_when=pass refuses the scoped plan, stages the Red Team on the merge base, stages pending Step 11 voices on the range, drops the fix-check silently, or puts it in the manifest; + // why_new=GEVA-82 on 1.91.379.0 hit BRIEF_RENDER_FAILED on every scoped pass and rendered briefs by hand; seam=none; no_claim=the fix-check's own brief + function scoped(flags: string[]) { + const { dir, env } = repo(); + const sh = (c: string) => { const r = spawnSync('bash', ['-c', c], { cwd: dir, env, encoding: 'utf8', timeout: 60_000 }); if (r.status !== 0) throw new Error(r.stderr); return r.stdout.trim(); }; + const mergeBase = sh('git merge-base main HEAD'); + const rangeBase = sh('git rev-parse HEAD'); + sh('for i in $(seq 1 24); do echo "export const f$i = $i;" >> src/fix.ts; done && git add -A && git commit -qm "fix: pre-landing review fixes"'); + const out = path.join(dir, '.state/run/p2'); + const extras = path.join(dir, '.state/extras.json'); + fs.mkdirSync(path.join(dir, '.state'), { recursive: true }); + fs.writeFileSync(path.join(dir, '.state/qa.md'), 'QA the fix.\n'); + fs.writeFileSync(extras, JSON.stringify([{ name: 'qa', prompt_file: path.join(dir, '.state/qa.md') }])); + const args = ['pass', '--base', 'main', '--out', out, '--pass', 'p2', '--extras', extras, '--', '--pass', '1', '--last', 'full', '--pass1-depth', 'standard', '--fixed-blocking', ...flags, '--range-base', rangeBase.slice(0, 7)]; + const p = bin(dir, env, args); + expect(p.status, p.stdout + p.stderr).toBe(0); + expect(p.stdout).toMatch(/^NEXT: scoped /m); + expect(p.stdout).toMatch(/^FIX_CHECK: planned effort=full \(Step 3\.5 runs it on its own; not in this manifest\)$/m); + const mf = path.join(out, 'pass-manifest.json'); + return { dir, env, args, mf, mergeBase, rangeBase, m: JSON.parse(fs.readFileSync(mf, 'utf8')) as Manifest }; + } + const settle = (s: ReturnType, stage: string) => { + if (stage !== 'specialists') expect(bin(s.dir, s.env, ['stage', '--manifest', s.mf, '--stage', stage]).status).toBe(0); + const m = JSON.parse(fs.readFileSync(s.mf, 'utf8')) as Manifest; + const reply = path.join(s.dir, '.state/reply.txt'); + fs.writeFileSync(reply, treeLine(s.dir) + 'NO FINDINGS\n'); + for (const r of m.readers.filter(x => x.stage === stage)) for (const [st, extra] of [['intent', []], ['created', ['--path', '1']], ['collected', r.reader.startsWith('audit/') ? ['--output', reply] : []]] as const) expect(bin(s.dir, s.env, ['mark', '--manifest', s.mf, '--reader', r.reader, '--state', st, ...extra]).status).toBe(0); + return JSON.parse(fs.readFileSync(s.mf, 'utf8')) as Manifest; + }; + + test('GEVA-2\'s invocation: the Red Team reads the fix range, the pending Step 11 voices read the whole branch, and the fix-check is named outside the manifest', () => { + const s = scoped([]); + expect(s.m.next).toBe('scoped'); + expect(s.m.diff_base).toBe(s.mergeBase); + expect(s.m.readers.map(r => [r.reader, r.diff_base ?? null])).toEqual([['red-team', s.rangeBase], ['adversarial-native', null], ['outside-voice', null], ['audit/qa', null]]); + settle(s, 'specialists'); + const m = settle(s, 'red-team'); + const redTeam = fs.readFileSync(m.readers.find(r => r.reader === 'red-team')!.file!, 'utf8'); + expect(redTeam).toContain(s.rangeBase); + expect(redTeam).not.toContain(s.mergeBase); + expect(bin(s.dir, s.env, ['stage', '--manifest', s.mf, '--stage', 'voices']).status).toBe(0); + const voices = JSON.parse(fs.readFileSync(s.mf, 'utf8')) as Manifest; + expect(fs.readFileSync(voices.readers.find(r => r.reader === 'outside-voice')!.file!, 'utf8')).toContain('+export const v1 = 1;'); + const again = bin(s.dir, s.env, s.args); + expect(again.status, again.stdout + again.stderr).toBe(0); + expect(again.stdout).toMatch(/^FIX_CHECK: planned effort=full /m); + expect(again.stdout).toContain('readers=4 resumed'); + }, 120_000); + + test('after Step 11 ran (--adversarial-ran), the whole scoped pass reads the fix range', () => { + const s = scoped(['--adversarial-ran']); + expect(s.m.diff_base).toBe(s.rangeBase); + expect(s.m.readers.map(r => [r.reader, r.diff_base ?? null])).toEqual([['red-team', s.rangeBase], ['adversarial-native', null], ['audit/qa', null]]); + }, 120_000); +}); + describe('readers on a shared machine review the checkout the pass ships (GST-482)', () => { /** The primary checkout on feat, and a worktree of it on another branch with one more commit: the GBRA-218 layout. */ function worktree() {