mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-01 22:45:02 +01:00
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>
This commit is contained in:
1 parent
510dd07d6b
commit
3f79d81673
8 files changed
+87
-13
No files matched your search
@@ -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 <short sha>`) 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 `<boundary>/../outside/new/x`, `<boundary>/link-to-outside/new/x`, `<boundary>/..`, `~/.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).
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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
|
||||
|
||||
+4
-3
@@ -74,7 +74,7 @@ const STAGE_OF: Record<string, Stage> = { 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 <diff-base> '.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;
|
||||
|
||||
+1
-1
@@ -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",
|
||||
|
||||
@@ -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<typeof scoped>, 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() {
|
||||
|
||||
Reference in new issue
Block a user