diff --git a/bin/gstack-review-dispatch b/bin/gstack-review-dispatch index e7c17de6d..66a47d9c4 100755 --- a/bin/gstack-review-dispatch +++ b/bin/gstack-review-dispatch @@ -117,7 +117,7 @@ function main(): number { const v = verify(ledger); for (const r of v.rows) process.stdout.write(`${r.kind} ${r.reader} ${r.status} ${r.detail}\n`); if (!v.briefs) { process.stdout.write('VERIFY: no brief records (this run\'s ledger kept no prompts; dispatched-vs-rendered cannot be checked)\n'); return EXIT.refused; } - process.stdout.write(`VERIFY: ${v.clean ? 'clean' : 'dirty'} briefs=${v.briefs} outputs=${v.rows.filter(r => r.kind === 'OUTPUT').length}\n`); + process.stdout.write(`VERIFY: ${v.clean ? 'clean' : 'dirty'} briefs=${v.briefs} outputs=${v.rows.filter(r => r.kind === 'OUTPUT').length} voided=${v.voided}\n`); return v.clean ? EXIT.ok : EXIT.fail; } process.stderr.write(`gstack-review-dispatch: unknown command ${cmd}\n${HELP}\n`); diff --git a/docs/review-policy.md b/docs/review-policy.md index 0077a56e7..ca0eee31a 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -384,10 +384,16 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends: promotes one without a reason (`JUDGMENT_REJECTED`). A merged file missing any id the ledger collected is a drop. Each addition and promotion is logged with its reason. 5. `verify --ledger ` replays all of it from the run's ledger: rendered against - dispatched hashes, a collected output for every sent reader (`NOT_COLLECTED` - otherwise), output hashes against the merged lines they produced, 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. + dispatched hashes per render (each render is one dispatch of its reader, so a + two-pass ledger checks pass-1 sends against pass-1 briefs; a later render prints as + `@2`), a collected output for every sent reader (`NOT_COLLECTED` + otherwise; a reader that failed is collected from its failure text, with zero + findings, and its coverage is logged `unavailable`). A mismatched send re-sent + byte-exact before anything was collected is void and counted (`voided=N` on the + verdict line); one that produced an output stays `MISMATCH`. Then output hashes + against the merged lines they produced, 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. ## Where this is enforced diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index 672be6b57..050522ef5 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -64,7 +64,7 @@ export function parseFindings(reader: string, output: string): Array<{ label: st if (!t || /^NO FINDINGS\.?$/.test(t) || /^Recommendation:/i.test(t)) continue; const heading = LABEL_HEADING.exec(t); if (heading) { section = heading[1]!; continue; } - if (HEADING.test(t)) section = null; + if (line === line.trimStart() && HEADING.test(t)) section = null; if (t.startsWith('{')) { try { const o = JSON.parse(t) as Record; @@ -142,36 +142,54 @@ const collectedIds = (events: LedgerEvent[]) => events.filter(e => e.event === ' export interface VerifyRow { kind: 'BRIEF' | 'OUTPUT' | 'JUDGE'; reader: string; status: string; detail: string } /** Replay every check from a finished run's ledger: briefs dispatched as rendered, outputs merged as returned, judgment without loss. */ -export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; briefs: number } { +export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; briefs: number; voided: number } { const events = readLedger(ledger); const rows: VerifyRow[] = []; const short = (s: unknown) => (typeof s === 'string' ? s.slice(0, 12) : '-'); - const readers = [...new Set(events.filter(e => e.event === 'rendered').map(e => String(e.reader)))]; - for (const reader of readers) { - const r = lastRendered(events, reader)!; - const sent = events.filter(e => e.event === 'sent' && e.reader === reader); + // Each render opens one dispatch of its reader (a pass); its sends and outputs run until that reader's next render. + const renders = events.flatMap((e, i) => (e.event === 'rendered' ? [i] : [])); + const episodes = renders.map((at, n) => { + const reader = String(events[at]!.reader); + const next = renders.slice(n + 1).find(j => events[j]!.reader === reader) ?? events.length; + const nth = renders.slice(0, n + 1).filter(j => events[j]!.reader === reader).length; + return { at, next, reader, label: nth > 1 ? `${reader}@${nth}` : reader }; + }); + let voided = 0; + for (const p of episodes) { + const r = events[p.at]!; + const inside = events.map((e, i) => ({ e, i })).filter(({ e, i }) => i > p.at && i < p.next && e.reader === p.reader); + const sent = inside.filter(({ e }) => e.event === 'sent'); let fileNow = ''; try { fileNow = typeof r.file === 'string' && fs.existsSync(r.file) ? promptHash(fs.readFileSync(r.file, 'utf8')) : ''; } catch { /* unreadable */ } - const collected = events.some(e => e.event === 'output' && e.reader === reader); - const status = !sent.length ? 'NOT_SENT' : !sent.every(s => s.sha256 === r.sha256) ? 'MISMATCH' : collected ? 'MATCH' : 'NOT_COLLECTED'; + // A mismatched send is void when the reader was re-sent byte-exact before anything was collected from it. + let mismatched = 0, epVoided = 0; + for (const s of sent.filter(({ e }) => e.sha256 !== r.sha256)) { + const after = inside.find(({ e, i }) => i > s.i && (e.event === 'output' || (e.event === 'sent' && e.sha256 === r.sha256))); + if (after?.e.event === 'sent') epVoided++; else mismatched++; + } + voided += epVoided; + const lastMatch = [...sent].reverse().find(({ e }) => e.sha256 === r.sha256); + const collected = !!lastMatch && inside.some(({ e, i }) => e.event === 'output' && i > lastMatch.i); + const status = !sent.length ? 'NOT_SENT' : mismatched ? 'MISMATCH' : collected ? 'MATCH' : 'NOT_COLLECTED'; const tampered = fileNow && fileNow !== r.sha256 ? ' file_changed_since_render' : ''; - rows.push({ kind: 'BRIEF', reader, status: tampered ? 'MISMATCH' : status, detail: `rendered=${short(r.sha256)} sent=${sent.map(s => short(s.sha256)).join(',') || '-'}${tampered}` }); + rows.push({ kind: 'BRIEF', reader: p.label, status: tampered ? 'MISMATCH' : status, detail: `rendered=${short(r.sha256)} sent=${sent.map(({ e }) => short(e.sha256)).join(',') || '-'}${epVoided ? ` voided=${epVoided}` : ''}${tampered}` }); } - for (const o of events.filter(e => e.event === 'output')) { + for (const [i, o] of events.entries()) { + if (o.event !== 'output') continue; const reader = String(o.reader); + const p = episodes.find(q => q.reader === reader && q.at < i && i < q.next); let status = 'MATCH'; const notes: string[] = []; - const rendered = lastRendered(events, reader); - const sent = events.filter(e => e.event === 'sent' && e.reader === reader); - if (!rendered) { status = 'MISMATCH'; notes.push('brief_not_rendered'); } - else if (!sent.length || sent.some(s => s.sha256 !== rendered.sha256)) { status = 'MISMATCH'; notes.push('brief_not_sent_verbatim'); } + 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'); } if (typeof o.file === 'string' && fs.existsSync(o.file) && promptHash(fs.readFileSync(o.file, 'utf8')) !== o.sha256) { status = 'MISMATCH'; notes.push('output_file_changed'); } const ids = Array.isArray(o.ids) ? (o.ids as string[]) : []; let lines: MergedLine[] = []; try { lines = typeof o.merged === 'string' ? readJsonl(o.merged).filter(l => ids.includes(l.id)) : []; } catch { notes.push('merged_unreadable'); status = 'MISMATCH'; } 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'); } - rows.push({ kind: 'OUTPUT', reader, status, detail: `sha=${short(o.sha256)} findings=${ids.length}${notes.length ? ' ' + notes.join(' ') : ''}` }); + rows.push({ kind: 'OUTPUT', reader: p?.label ?? reader, status, detail: `sha=${short(o.sha256)} findings=${ids.length}${notes.length ? ' ' + notes.join(' ') : ''}` }); } const lastJudge = events.map(e => e.event).lastIndexOf('judge'); if (lastJudge >= 0) { @@ -192,6 +210,6 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri const status = !v.ok ? 'REJECTED' : notes.length ? 'MISMATCH' : 'OK'; rows.push({ kind: 'JUDGE', reader: '-', status, detail: `dropped=${JSON.stringify(v.dropped)} downgraded=${JSON.stringify(v.downgraded)} added=${v.added} promoted=${v.promoted} folded=${v.folded}${notes.length ? ' ' + notes.join(' ') : ''}` }); } else if (events.some(e => e.event === 'output')) rows.push({ kind: 'JUDGE', reader: '-', status: 'MISSING', detail: 'outputs were collected but no judged set was checked' }); - const clean = readers.length > 0 && rows.every(r => r.status === 'MATCH' || r.status === 'OK'); - return { rows, clean, briefs: readers.length }; + const clean = episodes.length > 0 && rows.every(r => r.status === 'MATCH' || r.status === 'OK'); + return { rows, clean, briefs: episodes.length, voided }; } diff --git a/test/review-dispatch.test.ts b/test/review-dispatch.test.ts index f0c0305cd..42719905e 100644 --- a/test/review-dispatch.test.ts +++ b/test/review-dispatch.test.ts @@ -99,6 +99,30 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change expect([v.status, v.stdout.includes('BRIEF specialist/testing MISMATCH'), v.stdout.includes('VERIFY: dirty')]).toEqual([1, true, true]); }); + // Value: protects=verify checks each render as its own dispatch, so a two-pass ledger compares pass-1 sends with pass-1 briefs, and a mismatched send re-sent byte-exact before any collect is reported as voided, never hidden; a mismatched send that produced an output stays dirty; fails_when=a run-wide verify prints dirty for a clean two-pass run, a superseded mismatch blocks, voided is not printed, or an output collected under a mismatched send reads clean (GSTA-189 #36 replay: VERIFY dirty briefs=9 outputs=16, every output MATCH); + // why_new=verify compared every send with the reader's last render; seam=sent, collect and verify; no_claim=which pass a render belongs to beyond ledger order + test('two passes verify per render; a superseded mismatch is voided=1; a mismatched send with an output stays dirty', () => { + const p = pass(); + const out = p.w('spec.out', 'NO FINDINGS\n'); + const send = (file: string) => run(['sent', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--sent', file]); + const take = () => run(['collect', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--output', out, '--merged', p.merged]); + send(p.file); take(); + const second = field(run(['render', '--role', 'specialist', '--name', 'testing', '--out', tmp(), '--ledger', p.ledger, '--diff-base', 'b'.repeat(40)]).stdout, 'file'); + const truncated = p.w('truncated.txt', fs.readFileSync(second, 'utf8').slice(0, 200)); + expect(send(truncated).status).toBe(1); + send(second); take(); + expect(run(['judge', '--ledger', p.ledger, '--merged', p.merged, '--judged', p.w('judged.jsonl', '')]).status).toBe(0); + const v = run(['verify', '--ledger', p.ledger]); + expect([v.status, v.stdout.includes('BRIEF specialist/testing MATCH'), v.stdout.includes('BRIEF specialist/testing@2 MATCH'), v.stdout.includes(' voided=1\n'), v.stdout.includes('VERIFY: clean briefs=2 outputs=2 voided=1')]).toEqual([0, true, true, true, true]); + const q = pass(); + const qt = q.w('t.txt', fs.readFileSync(q.file, 'utf8').slice(0, 200)); + run(['sent', '--ledger', q.ledger, '--role', 'specialist', '--name', 'testing', '--sent', qt]); + run(['collect', '--ledger', q.ledger, '--role', 'specialist', '--name', 'testing', '--output', q.w('o.out', 'NO FINDINGS\n'), '--merged', q.merged]); + run(['sent', '--ledger', q.ledger, '--role', 'specialist', '--name', 'testing', '--sent', q.file]); + const w = run(['verify', '--ledger', q.ledger]); + expect([w.status, w.stdout.includes('BRIEF specialist/testing MISMATCH'), w.stdout.includes('brief_not_sent_verbatim'), w.stdout.includes('voided=0')]).toEqual([1, true, true, true]); + }); + // Value: protects=findings reach merged.jsonl only through collect with each voice's label as written, and lead judgment can add or promote but never drop or downgrade; fails_when=the parent drops a reader's finding, lowers its severity, folds it into a weaker duplicate or adds one silently, or merged lines no longer trace to the output hash; // why_new=merging was the model's own prose step; seam=collect, judge and verify; no_claim=that the judged set is used downstream by Step 9.4 test('collect keeps raw labels; judge rejects a drop and a downgrade, accepts a reasoned promotion and a fold into an equal duplicate', () => { @@ -180,7 +204,7 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change test('judge and parseFindings as functions: an unknown severity is invalid, NO FINDINGS and the Recommendation line are not findings', () => { expect(parseFindings('r', 'NO FINDINGS\n')).toEqual([]); - expect(parseFindings('r', '## High\n1. The auth check skips webhooks (a.ts:4)\n - Impact: nested evidence\n- second one\n## Notes\n- not a finding\n**Critical findings**\n- real one\n**Recommendations**\n- consider renaming foo\n').map(f => [f.label, f.raw])).toEqual([['High', '1. The auth check skips webhooks (a.ts:4)'], ['High', '- second one'], ['Critical', '- real one']]); + expect(parseFindings('r', '## High\n1. The auth check skips webhooks (a.ts:4)\n - Impact: nested evidence\n **Fix:**\n- second one\n## Notes\n- not a finding\n**Critical findings**\n- real one\n**Recommendations**\n- consider renaming foo\n').map(f => [f.label, f.raw])).toEqual([['High', '1. The auth check skips webhooks (a.ts:4)'], ['High', '- second one'], ['Critical', '- real one']]); const shapes = ['1. **Unbounded retry in queue.ts:78** (FIXABLE): loops on 429', '2. `auth.ts:42` skips token expiry check — INVESTIGATE', '### CRITICAL: missing auth check in api.ts:10', '#### [P1] token not expired', '### High: authorization bypass (auth.ts:14)', '- **HIGH**: leak', '| High | race in queue.ts:4 |', 'Classification: INVESTIGATE', '7. **Informational: reads every file**']; expect(parseFindings('r', shapes.join('\n')).map(f => f.label)).toEqual(['FIXABLE', 'INVESTIGATE', 'CRITICAL', 'P1', 'High', 'HIGH', 'High', 'INVESTIGATE', 'Informational']); expect(parseFindings('r', 'FIXABLE: race in a.ts:3\nINVESTIGATE: odd lock\nRecommendation: Fix the race because x\n').map(f => f.label)).toEqual(['FIXABLE', 'INVESTIGATE']);