fix(review): Step 11 on its own --out renders only the voices (GST-1368) (#4003)

* fix(review): Step 11 on its own --out renders only the voices, so verify never sees an unsent specialist brief (GST-1368)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs(changelog): GST-1368 fragment

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs(review-policy): describe pass --voices (GST-1368)

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>

bin/gstack-review-dispatch

@@ -70,10 +70,12 @@ const HELP = `gstack-review-dispatch — verbatim reviewer briefs and loss-free
       review_exclude_paths prints REVIEW_EXCLUDE_RECORD: digest=<d> (the selection every reader's newest brief names: the
       value gstack-ship-receipt write --review-exclude takes), and a mixed set of digests makes it dirty
   pass --base <ref> --out <dir> --pass <id> [--ledger <f>] [--diff-base <sha>] [--learnings-file <f>] [--extras <json>] [--audit-only]
-       [--checklist-root <dir>] [--fix-list <f> --hunks <f> --tests <f> [--callers <f>]] [-- <gstack-review-plan flags>]
+       [--voices] [--checklist-root <dir>] [--fix-list <f> --hunks <f> --tests <f> [--callers <f>]] [-- <gstack-review-plan flags>]
       runs gstack-review-plan and writes <out>/pass-manifest.json: stage first (audit readers marked first in --extras),
       specialists (with the other --extras readers), red-team, voices (Step 11). Renders first and specialists now; prints
       the plan, MANIFEST: and one READER: line per reader. A plan whose NEXT is not full, scoped or fix-check prints PASS: none.
+      --voices (Step 11 on its own --out) keeps only the plan's voices, so it renders nothing its program never sends; it
+      takes no --extras, and a reviewer pass refuses to resume its manifest.
       <id> is letters, digits and dashes, read as lowercase (P1 is p1): the manifest, reader titles and a resume use that form.
       A fix-check reader (stage specialists) renders from decision 3.5's --fix-list, --hunks, --tests and --callers files.
       --diff-base sets every reader's base, a scoped pass's Red Team included (the targeted reader keeps the merge base).
@@ -255,6 +257,7 @@ function pass(): number {
   if (fs.existsSync(file)) {
     const m = readManifest(file);
     if (m.pass !== id) throw new BriefError(`${file} belongs to pass ${m.pass}, not ${id}`);
+    if (m.voices_only && !args.includes('--voices')) throw new BriefError(`${file} holds only Step 11's voices (pass --voices); a reviewer pass needs its own --out`);
     process.stdout.write([...m.plan_lines, `MANIFEST: file=${file} pass=${m.pass} readers=${m.readers.length} resumed`, ...m.readers.map(r => readerLine(m, r))].join('\n') + '\n');
     return EXIT.ok;
   }
@@ -264,7 +267,7 @@ function pass(): number {
     if (!extrasFile) { process.stderr.write('gstack-review-dispatch pass --audit-only: needs --extras (the audit readers, e.g. Step 14.5\'s docs reader)\n'); return EXIT.usage; }
     const extras = JSON.parse(readFile(extrasFile, 'extras')!) as Extra[];
     const m = buildManifest({ pass: id, out, ledger: flag('--ledger') ?? path.join(out, 'review-ledger.jsonl'), base, diffBase: mergeBaseWith(base) ?? 'HEAD', root: git(['rev-parse', '--show-toplevel'])?.trim() ?? process.cwd(),
-      checklistRoot: flag('--checklist-root') ?? ROOT, dispatches: [], next: 'audit', planLines: ['PASS: audit-only (no reviewer plan; its extras run as one batch)'], extras });
+      checklistRoot: flag('--checklist-root') ?? ROOT, dispatches: [], next: 'audit', planLines: ['PASS: audit-only (no reviewer plan; its extras run as one batch)'], extras, voices: args.includes('--voices') });
     fs.mkdirSync(m.out, { recursive: true, mode: 0o700 });
     const t = sectionTexts(ROOT);
     renderStage(m, 'first', t, branchDiff);
@@ -295,7 +298,7 @@ function pass(): number {
     dispatches: step === 'fix-check' ? p.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: flag('--diff-base') ? undefined : rangeBase,
     fixCheck: step === 'fix-check' && (flag('--fix-list') || flag('--hunks') || flag('--tests')) ? { fix_list: flag('--fix-list')!, hunks: flag('--hunks')!, tests: flag('--tests')!, callers: flag('--callers') } : undefined,
-    ...planExclude(p.review_exclude), ...(p.readers ? { readers: p.readers } : {}) });
+    ...planExclude(p.review_exclude), ...(p.readers ? { readers: p.readers } : {}), voices: args.includes('--voices') });
   const over = ceiling('gstack-review-dispatch pass', base, m.out, m.readers, m.diff_base, (m.review_exclude as PlanExclude) ?? null);
   if (over) { process.stdout.write(planLines.join('\n') + '\n'); process.stderr.write(over.line + '\n'); return over.exit; }
   fs.mkdirSync(m.out, { recursive: true, mode: 0o700 });

changelog.d/garrytan-gst-1368-voices-pass-renders-only-voices.md

@@ -0,0 +1 @@
+- **Step 11's review voices no longer leave an unsent brief behind (GST-1368).** When /ship ran Step 11 on its own output folder, which happens after a fix round moves the head, `gstack-review-dispatch pass` built the manifest from the whole pass-1 plan and wrote the specialist brief into the ledger, but the voices program never sends it. `gstack-review-dispatch verify` then failed the run with `BRIEF specialist-batched NOT_SENT`. Step 11 now asks for `pass --voices`, which keeps only the plan's voices, so nothing is rendered that no reader gets. `--voices` refuses `--extras`, and a reviewer pass refuses to resume a voices-only manifest. A ledger written before this fix still verifies dirty; rerun Step 11 with a fresh `--out` and `--pass` to get a clean one.

docs/review-policy.md

@@ -825,7 +825,9 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends:
    checked-in program `lib/capy-review-pass.js`, served by `program` and loaded
    unchanged by the gstack Capy skill. A full repeat pass takes `--diff-base` (RANGE_BASE; a scoped pass, none), a
    fix-check plan renders its reader from `--fix-list`, `--hunks`, `--tests` and `--callers`,
-   `--audit-only` runs just the `--extras` readers (Step 14.5's docs audit), and the fork
+   `--audit-only` runs just the `--extras` readers (Step 14.5's docs audit), `--voices`
+   keeps only the plan's voices for Step 11 on its own `--out` (it takes no `--extras`, and a
+   reviewer pass refuses to resume that manifest, GST-1368), and the fork
    point is found against `origin/<base>` first, as preflight does.
 7. Every brief, audit readers' included, opens with a fingerprint step and ends with a
    workspace paragraph naming the checkout the bins run in (GST-482): a reader on a shared

lib/capy-review-pass.js

@@ -56,6 +56,7 @@ if (input.extras && typeof input.extras !== 'string') {
 if (input.learnings) passCmd.push('--learnings-file', q(input.learnings));
 if (input.diffBase) passCmd.push('--diff-base', q(input.diffBase));
 if (input.step === '14.5') passCmd.push('--audit-only');
+if (input.stage === 'voices') passCmd.push('--voices');
 for (const [k, v] of Object.entries(input.fixCheck || {})) passCmd.push('--' + k.replace('_', '-'), q(v));
 passCmd.push('--', ...(input.planFlags || []).map(q));
 const pr = await sh(passCmd.join(' '));

lib/review-pass.ts

@@ -56,6 +56,8 @@ export interface Manifest {
   review_exclude?: Record<string, unknown>; disclosure?: string;
   /** Decision 3.5's files for a fix-check reader: the fix list, touched hunks, test results and callers. */
   fix_check?: FixCheckFiles;
+  /** Step 11's own pass (`pass --voices`): the plan's voices only, so nothing is rendered that the voices program never sends. */
+  voices_only?: true;
   /** The plan's readers policy (lib/review-fetch.ts): what every brief says readers may not fetch or open; absent means the default rule. */
   fetch_policy?: EffectiveReaders;
 }
@@ -93,16 +95,18 @@ const shellQuote = (s: string) => `'${s.replace(/'/g, "'\\''")}'`;
 const safe = (reader: string) => reader.replace(/[^a-zA-Z0-9._-]/g, '_');
 const STAGE_OF: Record<string, Stage> = { specialist: 'specialists', 'fix-check': 'specialists', 'red-team': 'red-team', 'adversarial-native': 'voices', 'outside-voice': 'voices', targeted: 'voices' };
 
-/** The manifest for one pass from the plan's dispatches. A fix-check reader needs decision 3.5's files. */
+/** The manifest for one pass from the plan's dispatches. A fix-check reader needs decision 3.5's files. `voices` keeps only Step 11's readers. */
 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; rangeBase?: string; fixCheck?: FixCheckFiles; reviewExclude?: Record<string, unknown>; disclosure?: string; readers?: EffectiveReaders;
+  targetedClasses?: readonly string[]; mergeBase?: string; rangeBase?: string; fixCheck?: FixCheckFiles; reviewExclude?: Record<string, unknown>; disclosure?: string; readers?: EffectiveReaders; voices?: boolean;
 }): Manifest {
+  if (o.voices && o.extras?.length) throw new BriefError(`Step 11's voices pass sends no audit reader, so it takes no --extras: ${o.extras.map(e => e.name).join(', ')}`);
   const readers: PassReader[] = [];
   for (const d of o.dispatches) {
     const stage = STAGE_OF[d.role];
     if (!stage) throw new BriefError(`a pass dispatches no ${d.role} reader`);
+    if (o.voices && stage !== 'voices') continue;
     if (d.role === 'fix-check' && !o.fixCheck) throw new BriefError('the plan dispatches a fix-check: pass needs --fix-list, --hunks and --tests (decision 3.5\'s files; --callers optional)');
     const role: BriefRole = d.role === 'specialist' && d.checklists ? 'specialist-batched' : d.role as BriefRole;
     const name = role === 'specialist' ? d.name : undefined;
@@ -128,7 +132,7 @@ export function buildManifest(o: {
   return {
     version: 1, pass: o.pass, out, ledger: path.resolve(o.ledger), merged: path.join(out, 'merged.jsonl'), base: o.base, diff_base: o.diffBase,
     stack: detectStack(o.root), test_fw: detectTestFw(o.root), learnings: o.learnings ?? null, checklist_root: o.checklistRoot,
-    next: o.next, plan_lines: o.planLines, readers, ...(o.reviewExclude ? { review_exclude: o.reviewExclude } : {}), ...(o.disclosure ? { disclosure: o.disclosure } : {}), ...(o.readers ? { fetch_policy: o.readers } : {}),
+    next: o.next, plan_lines: o.planLines, readers, ...(o.voices ? { voices_only: true as const } : {}), ...(o.reviewExclude ? { review_exclude: o.reviewExclude } : {}), ...(o.disclosure ? { disclosure: o.disclosure } : {}), ...(o.readers ? { fetch_policy: o.readers } : {}),
     ...(o.fixCheck ? { fix_check: Object.fromEntries(Object.entries(o.fixCheck).filter(([, v]) => v).map(([k, v]) => [k, path.resolve(v as string)])) as unknown as FixCheckFiles } : {}),
   };
 }

test/review-pass.test.ts

@@ -359,6 +359,34 @@ describe('Capy pass program (fake host, real bins)', () => {
     expect(Object.keys(m.log_starts ?? {}).sort()).toEqual(['pass:review', 'voices:adversarial-review']);
   }, 240_000);
 
+  // Value: protects=Step 11 on its own --out (the head moved after a fix round) renders and sends only the voices, so its ledger verifies clean; fails_when=pass renders the plan's first and specialists stages for a voices-only program, leaving BRIEF specialist-batched NOT_SENT in the ledger;
+  //   why_new=GST-1368 (gstack-evals#208 at fb407fb: VERIFY: dirty briefs=4 outputs=3); seam=none; no_claim=which plan flags Step 11 takes
+  test('Step 11 on a fresh --out renders only the voices, and its ledger verifies clean', async () => {
+    const { dir, env } = repo();
+    const out = path.join(dir, '.state/run/s11');
+    const h = host(dir, env);
+    const models = { 'outside-voice': { model_id: 'openai/gpt-6.1-sol' } };
+    const res = await runPass(h, { base: 'main', out, pass: 'x7s11', planFlags: ['--depth-floor', 'full'], stage: 'voices', models });
+    expect(res).toContain('NEXT: judge the voices');
+    const m = JSON.parse(fs.readFileSync(path.join(out, 'pass-manifest.json'), 'utf8')) as Manifest;
+    fs.writeFileSync(path.join(out, 'judged.jsonl'), '');
+    expect(bin(dir, env, ['judge', '--ledger', m.ledger, '--merged', m.merged, '--judged', path.join(out, 'judged.jsonl')]).status).toBe(0);
+    const v = bin(dir, env, ['verify', '--ledger', m.ledger]);
+    expect(v.stdout).not.toContain('NOT_SENT');
+    expect(v.stdout).toContain('VERIFY: clean briefs=' + m.readers.length);
+    expect(m.readers.length).toBeGreaterThan(0);
+    expect(m.readers.every(r => r.stage === 'voices' && r.state === 'collected')).toBe(true);
+    expect(h.creates.map(c => c.title).sort()).toEqual(m.readers.map(r => r.title).sort());
+    const rendered = fs.readFileSync(m.ledger, 'utf8').trim().split('\n').map(l => JSON.parse(l)).filter(e => e.event === 'rendered').map(e => e.reader);
+    expect(rendered.sort()).toEqual(m.readers.map(r => r.reader).sort());
+    const reviewer = bin(dir, env, ['pass', '--base', 'main', '--out', out, '--pass', 'x7s11', '--', '--depth-floor', 'full']);
+    expect([reviewer.status === 0, reviewer.stderr]).toEqual([false, expect.stringContaining("holds only Step 11's voices")]);
+    const extras = path.join(dir, '.state/extras.json');
+    fs.writeFileSync(extras, JSON.stringify([{ name: 'plan', prompt_file: path.join(dir, 'package.json') }]));
+    const audit = bin(dir, env, ['pass', '--base', 'main', '--out', path.join(dir, '.state/run/s11x'), '--pass', 'x7s11x', '--voices', '--extras', extras, '--', '--depth-floor', 'full']);
+    expect([audit.status === 0, audit.stderr]).toEqual([false, expect.stringContaining("Step 11's voices pass sends no audit reader")]);
+  }, 240_000);
+
   test('Step 11 voices run from the same manifest; the outside voice needs a model of another family', async () => {
     const { dir, env } = repo();
     const out = path.join(dir, '.state/run/p1');