fix(ship): verify accepts an output as a review only when its reply names the render's tree (GST-541 red team)

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

lib/review-ledger.ts

@@ -236,18 +236,6 @@ export function recordJudgment(ledger: string, merged: MergedLine[], judged: Jud
   return res;
 }
 
-/**
- * Whether a recorded output was a review: it has findings, or its file (unchanged since collect) carries the reader's clean marker, so
- * a ledger written before collect recorded the field is judged by the same rule; with no readable file, collect's own review field.
- */
-function reviewed(o: LedgerEvent): boolean {
-  if (o.tree_ok === false || unavailableOutput(o)) return false;
-  if (Array.isArray(o.ids) && o.ids.length) return true;
-  let text: string | undefined;
-  try { text = typeof o.file === 'string' ? fs.readFileSync(o.file, 'utf8') : undefined; } catch { /* unreadable */ }
-  return text !== undefined && promptHash(text) === o.sha256 ? hasCleanMarker(String(o.reader), text) : o.review === true;
-}
-
 /** A recorded output's file text while it still hashes to what collect recorded. */
 function unchangedOutput(o: LedgerEvent): string | undefined {
   try {
@@ -256,6 +244,23 @@ function unchangedOutput(o: LedgerEvent): string | undefined {
   } catch { return undefined; }
 }
 
+/** Whether a reply names the tree its render recorded (GST-482), checked on the reply itself, never on an output event's editable fields. */
+const treeHolds = (render: LedgerEvent | undefined, reply: string | undefined) => !render || !('tree' in render)
+  || (reply !== undefined && checkReviewedTree(reply, typeof render.tree === 'string' ? render.tree : null).ok);
+
+/**
+ * Whether a recorded output was a review: its unchanged reply names its render's tree, and it has findings or carries the reader's
+ * clean marker, so a ledger written before collect recorded the field is judged by the same rule; with no readable file, collect's own
+ * review field.
+ */
+function reviewed(o: LedgerEvent, render?: LedgerEvent): boolean {
+  if (o.tree_ok === false || unavailableOutput(o)) return false;
+  const text = unchangedOutput(o);
+  if (!treeHolds(render, text)) return false;
+  if (Array.isArray(o.ids) && o.ids.length) return true;
+  return text !== undefined ? hasCleanMarker(String(o.reader), text) : o.review === true;
+}
+
 /** Every id collect recorded, in order. */
 const collectedIds = (events: LedgerEvent[]) => events.filter(e => e.event === 'output').flatMap(e => (Array.isArray(e.ids) ? (e.ids as string[]) : []));
 
@@ -293,7 +298,7 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri
     voided += epVoided;
     const lastMatch = [...sent].reverse().find(({ e }) => e.sha256 === r.sha256);
     const outputs = lastMatch ? inside.filter(({ e, i }) => e.event === 'output' && i > lastMatch.i) : [];
-    const collected = outputs.some(({ e }) => reviewed(e));
+    const collected = outputs.some(({ e }) => reviewed(e, r));
     const unavailable = !collected && outputs.some(({ e }) => unavailableOutput(e));
     const status = !sent.length ? 'NOT_SENT' : mismatched ? 'MISMATCH' : collected ? 'MATCH' : unavailable ? 'UNAVAILABLE' : 'NOT_COLLECTED';
     const tampered = fileNow && fileNow !== r.sha256 ? ' file_changed_since_render' : '';
@@ -310,10 +315,7 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri
     else if (!lastSent || lastSent.sha256 !== events[p.at]!.sha256) { status = 'MISMATCH'; notes.push('brief_not_sent_verbatim'); }
     let text: string | undefined;
     try { text = typeof o.file === 'string' ? fs.readFileSync(o.file, 'utf8') : undefined; } catch { /* missing or unreadable */ }
-    // Whether a reply names the tree its render recorded (GST-482), checked on the reply itself, never on an output event's editable fields.
     const render = p ? events[p.at]! : undefined;
-    const treeHolds = (reply: string | undefined) => !render || !('tree' in render)
-      || (reply !== undefined && checkReviewedTree(reply, typeof render.tree === 'string' ? render.tree : null).ok);
     if (text !== undefined && promptHash(text) !== o.sha256) { status = 'MISMATCH'; notes.push('output_file_changed'); }
     const ids = Array.isArray(o.ids) ? (o.ids as string[]) : [];
     let lines: MergedLine[] = [];
@@ -332,17 +334,17 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri
       else if (!unavailableOutput(o)) {
         if (off.length) { status = 'MISMATCH'; notes.push(`merged_line_differs=${JSON.stringify(off)}`); differs.push(...off); }
         // A reply collect refused for its tree (GST-482) records no ids whatever it holds; the refusal is re-derived from the reply.
-        if (parsed.length !== ids.length && !(o.tree_ok === false && !ids.length && !treeHolds(text))) { status = 'MISMATCH'; notes.push(`merged_count_differs=${parsed.length}/${ids.length}`); }
+        if (parsed.length !== ids.length && !(o.tree_ok === false && !ids.length && !treeHolds(render, text))) { status = 'MISMATCH'; notes.push(`merged_count_differs=${parsed.length}/${ids.length}`); }
       }
     }
-    if (status === 'MATCH' && !reviewed(o)) {
+    if (status === 'MATCH' && !reviewed(o, render)) {
       // A non-review the same dispatch later replaced with a review (the brief re-sent verbatim, then answered) is void, counted like a voided send.
       const later = p ? events.slice(i + 1, p.next) : [];
-      const review = later.findIndex(e => e.event === 'output' && e.reader === reader && reviewed(e) && treeHolds(unchangedOutput(e)));
+      const review = later.findIndex(e => e.event === 'output' && e.reader === reader && reviewed(e, render));
       const replaced = review > 0 && later.slice(0, review).some(e => e.event === 'sent' && e.reader === reader && e.sha256 === events[p!.at]!.sha256);
       if (unavailableOutput(o)) { status = 'UNAVAILABLE'; notes.push(`reason=${JSON.stringify(o.unavailable)}`); }
       else if (replaced) { status = 'VOIDED'; voided++; notes.push('replaced_by_a_review'); }
-      else if (o.tree_ok === false) { status = 'NOT_REVIEWED'; notes.push(`wrong_tree reported=${JSON.stringify(o.reported_tree)} expected=${JSON.stringify(o.tree)}`); }
+      else if (o.tree_ok === false || !treeHolds(render, text)) { status = 'NOT_REVIEWED'; notes.push(`wrong_tree reported=${JSON.stringify(o.reported_tree)} expected=${JSON.stringify(o.tree)}`); }
       else { status = 'NOT_REVIEWED'; notes.push('no_finding_no_clean_marker'); }
     }
     rows.push({ kind: 'OUTPUT', reader: p?.label ?? reader, status, detail: `sha=${short(o.sha256)} findings=${ids.length}${notes.length ? ' ' + notes.join(' ') : ''}` });

test/review-dispatch.test.ts

@@ -741,7 +741,7 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change
   //   fails_when=verify skips the count check on the editable review or tree_ok field, so a clean marker keeps the output MATCH or a
   //   later review voids it, or an output with no recorded file passes on its review field (fix-checks at 721539c, 56ed6e3 and 50dc73c: VERIFY: clean); why_new=the GST-482 count exemption trusted collect's
   //   fields; seam=verify; no_claim=that a deleted output event is caught (#3719)
-  test('an output event edited to look refused is dirty; a real wrong-tree reply is NOT_REVIEWED, then voided by a review', () => {
+  test('an output event edited to look refused, or a refused one edited to look accepted, is dirty; a real wrong-tree reply is NOT_REVIEWED, then voided by a review', () => {
     const head = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: ROOT, encoding: 'utf8', timeout: 30_000 }).stdout.trim();
     const findings = '1. **CRITICAL**: auth.ts:3 leaks a token\nBlocks: yes\n\nRecommendation: request changes\n';
     const voice = (reply: string) => {
@@ -788,6 +788,15 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change
     expect(wrong.take('Recommendation: approve\n').status).toBe(0);
     const r = wrong.verify();
     expect([r.status, r.stdout.includes('replaced_by_a_review'), r.stdout.includes('VERIFY: clean briefs=1 outputs=2 voided=1')]).toEqual([0, true, true]);
+    const elsewhere = pass();
+    run(['sent', '--ledger', elsewhere.ledger, '--role', 'specialist', '--name', 'testing', '--sent', elsewhere.file]);
+    const refused = run(['collect', '--ledger', elsewhere.ledger, '--role', 'specialist', '--name', 'testing', '--output', elsewhere.w('else.out', `REVIEWED_TREE: ${'b'.repeat(40)} /elsewhere\nNO FINDINGS\n`), '--merged', elsewhere.merged]);
+    expect(refused.stdout.startsWith('COLLECT_WRONG_TREE: reader=specialist/testing')).toBe(true);
+    fs.writeFileSync(elsewhere.ledger, readJsonl<Record<string, unknown>>(elsewhere.ledger).map(e => JSON.stringify(e.event === 'output' ? { ...e, tree_ok: true, review: true, reported_tree: e.tree } : e)).join('\n') + '\n');
+    fs.writeFileSync(elsewhere.merged, '');
+    expect(run(['judge', '--ledger', elsewhere.ledger, '--merged', elsewhere.merged, '--judged', elsewhere.w('judged.jsonl', '')]).status).toBe(0);
+    const e = run(['verify', '--ledger', elsewhere.ledger]);
+    expect([e.status, /^OUTPUT specialist\/testing NOT_REVIEWED .*wrong_tree/m.test(e.stdout), e.stdout.includes('VERIFY: dirty')]).toEqual([1, true, true]);
   });
 });