fix(ship): verify fails closed on a missing output, a forged merged copy and uncollected rows (GST-541 review)

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

changelog.d/gst-541-verify-binding.md

@@ -1,7 +1,10 @@
-**`gstack-review-dispatch verify` now catches a merged findings file edited after collect.** Verify used to compare only output hashes and line counts. So a `merged.jsonl` row whose label was lowered, or whose text was replaced with another line's summary, passed with `VERIFY: clean` once the judge agreed with the edited row. Verify now re-parses each reader's unchanged output and checks every merged row against the finding at its index, label and raw text byte for byte. A mismatch prints `merged_line_differs` on the OUTPUT and JUDGE rows, and verify is dirty.
+**`gstack-review-dispatch verify` now catches a merged findings file edited after collect.** Verify used to compare only output hashes and line counts. So a `merged.jsonl` row whose label was lowered, or whose text was replaced with another line's summary, passed with `VERIFY: clean` once the judge agreed with the edited row. Verify now re-parses each reader's unchanged output and checks every merged row against the finding at its index, label and raw text byte for byte. A mismatch prints `merged_line_differs` on the OUTPUT and JUDGE rows, and verify is dirty. A missing output file, a judgment run on a copy of the merged file, and merged rows that collect never wrote are dirty too.
+
+### Known
+- Deleting a whole output event together with its merged rows still verifies clean, because the ledger keeps no anchor for it. It is listed for the structured-findings plan.
 
 ### Tests
-- `test/review-dispatch.test.ts`: a forged label and a summary copied onto a blocker's id both verify dirty. Both verified clean at main c830c9b.
+- `test/review-dispatch.test.ts`: a forged label, a summary copied onto a blocker's id, a deleted output file, a judgment on a forged copy and an extra merged row all verify dirty. Each verified clean at main c830c9b.
 
 ### Docs
 - `docs/review-policy.md` (verify) describes the per-row check.

docs/review-policy.md

@@ -501,7 +501,11 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends:
    against the merged lines they produced, and each merged line against the finding collect
    parses at that id's index from the unchanged output file, label and raw text byte for byte
    (`merged_line_differs` on the OUTPUT and JUDGE rows otherwise, GST-541), so an edited
-   merged.jsonl cannot lower a finding or swap its text, and the judgment rerun against the
+   merged.jsonl cannot lower a finding or swap its text. A recorded output file that is missing
+   (`output_file_missing`), a judgment run on a merged file collect did not write
+   (`judge_merged_not_collected`) and merged rows collect never recorded
+   (`merged_rows_not_collected`) are dirty too. Deleting a whole output event with its rows is
+   not caught yet, because the ledger has no anchor for it, and the judgment rerun against the
    recorded judged set, covering every collected id. A ledger from a run that kept no
    prompts prints `VERIFY: no brief records` (exit 3), never clean.
 6. `pass --base <b> --out <dir> --pass <id> -- <plan flags>` runs one pass from a

lib/review-ledger.ts

@@ -280,7 +280,8 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri
     const lastSent = p ? events.slice(p.at + 1, i).filter(e => e.event === 'sent' && e.reader === reader).at(-1) : undefined;
     if (!p) { status = 'MISMATCH'; notes.push('brief_not_rendered'); }
     else if (!lastSent || lastSent.sha256 !== events[p.at]!.sha256) { status = 'MISMATCH'; notes.push('brief_not_sent_verbatim'); }
-    const text = typeof o.file === 'string' && fs.existsSync(o.file) ? fs.readFileSync(o.file, 'utf8') : undefined;
+    let text: string | undefined;
+    try { text = typeof o.file === 'string' ? fs.readFileSync(o.file, 'utf8') : undefined; } catch { /* missing or unreadable */ }
     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[] = [];
@@ -288,10 +289,18 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri
     if (lines.length !== ids.length) { status = 'MISMATCH'; notes.push(`merged_lines=${lines.length}/${ids.length}`); }
     if (lines.some(l => l.output_sha256 !== o.sha256)) { status = 'MISMATCH'; notes.push('merged_line_from_other_output'); }
     // Each merged row must be the finding collect parses at that id's index from the unchanged output, label and raw text byte for byte.
-    if (text !== undefined && promptHash(text) === o.sha256 && !unavailableOutput(o)) {
+    if (text === undefined && ids.length) {
+      status = 'MISMATCH';
+      notes.push(typeof o.file === 'string' ? 'output_file_missing' : 'output_file_unrecorded');
+      differs.push(...ids);
+    } else if (text !== undefined && promptHash(text) === o.sha256) {
       const parsed = parseFindings(reader, text);
       const off = ids.filter((id, k) => { const m = lines.find(l => l.id === id); return !m || parsed[k]?.label !== m.label || parsed[k]?.raw !== m.raw; });
-      if (off.length || parsed.length !== ids.length) { status = 'MISMATCH'; notes.push(`merged_line_differs=${JSON.stringify(off)}`); differs.push(...off); }
+      if (unavailableOutput(o) && parsed.length) { status = 'MISMATCH'; notes.push('unavailable_output_has_findings'); }
+      else if (!unavailableOutput(o)) {
+        if (off.length) { status = 'MISMATCH'; notes.push(`merged_line_differs=${JSON.stringify(off)}`); differs.push(...off); }
+        if (parsed.length !== ids.length) { status = 'MISMATCH'; notes.push(`merged_count_differs=${parsed.length}/${ids.length}`); }
+      }
     }
     if (status === 'MATCH' && !reviewed(o)) {
       // 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.
@@ -313,11 +322,18 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri
     const judgedIds = new Set(Array.isArray(v.merged_ids) ? (v.merged_ids as string[]) : []);
     const missing = collectedIds(events.slice(0, lastJudge)).filter(id => !judgedIds.has(id));
     if (missing.length) notes.push(`merged_ids_missing=${missing.join(',')}`);
+    // The judged set must come from the merged file collect wrote, holding only rows collect recorded.
+    const outputs = events.slice(0, lastJudge).filter(e => e.event === 'output' && Array.isArray(e.ids) && e.ids.length);
+    if (outputs.some(e => typeof e.merged !== 'string' || path.resolve(e.merged) !== path.resolve(String(v.merged)))) notes.push('judge_merged_not_collected');
     let rerun: Judgment | null = null;
     try {
       const judgedText = fs.readFileSync(String(v.judged), 'utf8');
       if (promptHash(judgedText) !== v.judged_sha256) notes.push('judged_changed_since_judge');
-      rerun = judge(readJsonl<MergedLine>(String(v.merged)), readJsonl<JudgedLine>(String(v.judged)));
+      const merged = readJsonl<MergedLine>(String(v.merged));
+      const collected = new Set(collectedIds(events.slice(0, lastJudge)));
+      const extra = merged.filter(m => !collected.has(m.id)).map(m => m.id);
+      if (extra.length) notes.push(`merged_rows_not_collected=${JSON.stringify(extra)}`);
+      rerun = judge(merged, readJsonl<JudgedLine>(String(v.judged)));
     } catch { notes.push('judged_or_merged_unreadable'); }
     if (rerun && !rerun.ok) notes.push(`rerun_rejects dropped=${JSON.stringify(rerun.dropped)} downgraded=${JSON.stringify(rerun.downgraded)}`);
     const forged = differs.filter(id => judgedIds.has(id));

test/review-dispatch.test.ts

@@ -507,5 +507,16 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change
     const copied = forge(setup(), rows => { rows[0]!.label = rows[1]!.label; rows[0]!.raw = rows[1]!.raw; });
     expect([copied.status, copied.stdout]).toEqual([1, expect.stringContaining('merged_line_differs=["specialist/testing#1"]')]);
     expect(copied.stdout).toMatch(/^JUDGE - MISMATCH .*merged_line_differs/m);
+    const gone = setup();
+    fs.rmSync(path.join(gone.out, 'spec.out'));
+    const missing = forge(gone, rows => { rows[0]!.label = 'INFORMATIONAL'; });
+    expect([missing.status, missing.stdout.includes('output_file_missing'), missing.stdout.includes('merged_line_differs=["specialist/testing#1","specialist/testing#2"]')]).toEqual([1, true, true]);
+    const copy = setup();
+    const forgedCopy = copy.w('merged2.jsonl', readJsonl<MergedLine>(copy.merged).map((r, k) => JSON.stringify(k ? r : { ...r, label: 'INFORMATIONAL' })).join('\n') + '\n');
+    expect(run(['judge', '--ledger', copy.ledger, '--merged', forgedCopy, '--judged', copy.w('judged.jsonl', readJsonl<MergedLine>(copy.merged).map(m => JSON.stringify({ id: m.id, severity: 'INFORMATIONAL' })).join('\n') + '\n')]).status).toBe(0);
+    const viaCopy = run(['verify', '--ledger', copy.ledger]);
+    expect([viaCopy.status, viaCopy.stdout]).toEqual([1, expect.stringMatching(/^JUDGE - MISMATCH .*judge_merged_not_collected/m)]);
+    const extra = forge(setup(), rows => { rows[0]!.label = 'INFORMATIONAL'; rows.push({ ...rows[1]!, id: 'specialist/testing#9' }); });
+    expect([extra.status, extra.stdout.includes('merged_rows_not_collected=["specialist/testing#9"]')]).toEqual([1, true]);
   });
 });