fix(ship): negation stops at a conjunction, a summary line with a blocker stays, detail bullets need no gap and no locus; a re-sent review voids a refusal and the outside voice can be collected unavailable (pre-landing review)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-10-31 10:15:39 +00:00
1 parent 574d6e51e2
commit 27a937a141
5 files changed
+129 -46

No files matched your search

+8 -3
View File
@@ -16,7 +16,7 @@ import * as fs from 'node:fs';
import * as path from 'node:path';
import { EXIT, exitTableHelp } from '../lib/headless-artifacts';
import { BRIEF_ROLES, BriefError, briefFile, promptHash, renderBrief, sectionTexts, type BriefRole, type Slots } from '../lib/review-brief';
import { cleanMarkers, collect, readJsonl, readLedger, readerKey, recordJudgment, recordSent, appendEvent, verify, type JudgedLine, type MergedLine, type VerifyRow } from '../lib/review-ledger';
import { cleanMarkers, collect, mayBeUnavailable, readJsonl, readLedger, readerKey, recordJudgment, recordSent, appendEvent, verify, type JudgedLine, type MergedLine, type VerifyRow } from '../lib/review-ledger';
import { TARGETED_RULES, parseClassList, targetedBriefSha, targetedPatchId } from '../lib/review-targeted';
import { resolveBaseBranch } from '../lib/ship-policy';
@@ -38,6 +38,8 @@ const HELP = `gstack-review-dispatch — verbatim reviewer briefs and loss-free
the reader's raw output: hashes it and appends each finding to merged.jsonl with its label as written. A reply with
no finding must carry its brief's clean marker (NO FINDINGS; the Recommendation: line for the voices); without one it is
COLLECT_NOT_REVIEWED (exit 1): a refusal, an empty reply or a reader that never saw the diff, and verify reports NOT_COLLECTED
until the brief is re-sent and answered with a review, which voids it. --unavailable '<reason>' (the outside voice only) records
its failure text as missing coverage: COLLECT_UNAVAILABLE, and verify prints UNAVAILABLE and unavailable=N, never a review
judge --ledger <f> --merged <merged.jsonl> --judged <judged.jsonl>
judged lines {"id","severity","reason"?,"duplicate_of"?}: every merged id kept at its severity or above (a
duplicate folds into one at least as severe); added and promoted findings need a reason. JUDGMENT: ok …, or
@@ -135,7 +137,10 @@ function main(): number {
const output = readFile(outFile, 'reader output');
const merged = flag('--merged');
if (!reader || output === undefined || !merged) { process.stderr.write('gstack-review-dispatch collect: needs --role, --output <file> and --merged <file>\n'); return EXIT.usage; }
const r = collect(ledger, reader, output, merged, path.resolve(outFile!));
const unavailable = flag('--unavailable');
if (unavailable !== undefined && !mayBeUnavailable(reader)) { process.stderr.write(`gstack-review-dispatch collect: --unavailable is for the outside voice only; ${reader}'s missing coverage leaves the review incomplete, never clean\n`); return EXIT.usage; }
const r = collect(ledger, reader, output, merged, path.resolve(outFile!), unavailable);
if (unavailable !== undefined) { process.stdout.write(`COLLECT_UNAVAILABLE: reader=${reader} sha256=${r.sha} reason=${JSON.stringify(unavailable)} (missing outside coverage: unavailable, never clean)\n`); return EXIT.ok; }
if (!r.review) {
process.stdout.write(`COLLECT_NOT_REVIEWED: reader=${reader} findings=0 sha256=${r.sha} (no finding and no ${cleanMarkers(reader).map(([m]) => m).join(' or ')} line: a refusal, an empty reply or a reader that never saw the diff is not a review; verify reports this brief NOT_COLLECTED)\n`);
return EXIT.fail;
@@ -161,7 +166,7 @@ function main(): number {
process.stdout.write(`TARGETED_RECORD: classes=${record.classes.join(',')} status=${record.status} findings=${record.findings} brief=${record.brief_sha.slice(0, 12)} rules=${record.rules}\n`);
if (flag('--targeted-out')) fs.writeFileSync(flag('--targeted-out')!, JSON.stringify(record) + '\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`);
process.stdout.write(`VERIFY: ${v.clean ? 'clean' : 'dirty'} briefs=${v.briefs} outputs=${v.rows.filter(r => r.kind === 'OUTPUT').length} voided=${v.voided}${v.unavailable ? ` unavailable=${v.unavailable}` : ''}\n`);
return v.clean ? EXIT.ok : EXIT.fail;
}
process.stderr.write(`gstack-review-dispatch: unknown command ${cmd}\n${HELP}\n`);
+3 -3
View File
@@ -1,8 +1,8 @@
**/ship's review collection reads reviewer replies the way they were meant: a reviewer that says it could not review no longer counts as clean, and a closing "nothing here is CRITICAL" no longer counts as a CRITICAL finding.** In the GSTA-196 replay a specialist replied in prose that the diff base did not exist and it was not returning NO FINDINGS. `collect` parsed that reply to zero findings, and `verify` marked the brief collected, so the review would have read clean from a reader that never saw the diff. What reviewers see is unchanged: every brief renders byte for byte as before.
### Fixed
- **`gstack-review-dispatch collect` needs the clean marker a reader's own brief asks for before it counts a reply with no findings.** That is `NO FINDINGS` alone on a line for the specialists, the batched specialist, the targeted reader, the Red Team and fix-check; the `Recommendation:` line for the outside voice; and either one for the native adversarial voice. A reply with no finding and no marker (a refusal, an empty reply, a reader that never got the diff) is recorded with `review: false`, prints `COLLECT_NOT_REVIEWED` and exits 1.
- **`gstack-review-dispatch collect` needs the clean marker a reader's own brief asks for before it counts a reply with no findings.** That is `NO FINDINGS` alone on a line for the specialists, the batched specialist, the targeted reader, the Red Team and fix-check; the `Recommendation:` line for the outside voice; and either one for the native adversarial voice. A reply with no finding and no marker (a refusal, an empty reply, a reader that never got the diff) is recorded with `review: false`, prints `COLLECT_NOT_REVIEWED` and exits 1. Re-sending the brief and collecting a real review voids it, and the outside voice, whose failure never blocks, is collected with `--unavailable '<reason>'` and verifies as `UNAVAILABLE`, never as a review.
- **`gstack-review-dispatch verify` reports that reader's brief `NOT_COLLECTED` and its output `NOT_REVIEWED`, so VERIFY reads dirty.** When the output file is still there and unchanged, verify checks the marker in it again, so a ledger written before this change is judged by the same rule. The GSTA-189 replay's combined ledger still verifies clean, with five `NO FINDINGS` specialists and one outside voice ending in `Recommendation:`.
- **A severity word used as a word is no longer a finding.** A fix-check reader on gstack-evals#94 closed with "Neither finding is CRITICAL.", `collect` stored that line as a CRITICAL finding, and `judge` would not let the lead drop it. Now a label that is negated ("not CRITICAL", "no High findings", "Neither finding is CRITICAL"), used as a yardstick ("below CRITICAL"), quoted or listed as vocabulary ("Critical|High") is skipped, and so is a closing "Overall:" or "Summary:" line after the findings. A label the line opens with, a `Severity:` field, a table cell, and a label whose negation describes the defect ("Without auth this is CRITICAL") still count.
- **A paragraph finding keeps its detail bullets.** The GSTA-196 native adversarial voice wrote four findings, each with unlabeled bullets and then a `Severity:` line. Every bullet closed the open finding, so each `Severity:` line opened a new one: 4 findings collected as 8. They now collect as 4, each at the stronger of its two labels.
- **A severity word used as a word is no longer a finding.** A fix-check reader on gstack-evals#94 closed with "Neither finding is CRITICAL.", `collect` stored that line as a CRITICAL finding, and `judge` would not let the lead drop it. Now a label that is negated ("not CRITICAL", "no High findings", "Neither finding is CRITICAL"), used as a yardstick ("below CRITICAL"), quoted or listed as vocabulary ("Critical|High") is skipped, and so is a closing "Overall:" or "Summary:" line after the findings unless it says `Blocks: yes`, names a file and line, or carries a label stronger than every finding before it. A label the line opens with, a `Severity:` field, a table cell, a backticked label, and a label whose negation describes the defect ("Without auth this is CRITICAL", "None of the inputs are sanitized so this is CRITICAL") still count.
- **A paragraph finding keeps its detail bullets.** The GSTA-196 native adversarial voice wrote four findings, each with unlabeled bullets and then a `Severity:` line. Every bullet closed the open finding, so each `Severity:` line opened a new one: 4 findings collected as 8. They now collect as 4, each at the stronger of its two labels. Only an unnumbered bullet right under the finding, with no file and line of its own, counts as detail; a numbered item or one after a blank line is still its own finding.
- **No blocker is lost.** Main's parser and this one were run over every recorded reader output: 25 raw outputs from the GSTA-189 and GSTA-196 replays and 2,172 finding records from arms C6, L6, A3668 (both suites), G1 and L. The 8 GSTA-196 lines merge into 4 findings, none at a weaker label. One INVESTIGATE finding, recorded in both arm G1 and arm L, loses a phantom CRITICAL ("no CRITICAL" inside its sentence), and another loses a phantom High ("resolves below High"). None of the 260 records matched to a judged blocker changes.
+14 -8
View File
@@ -474,11 +474,15 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends:
`Blocks: yes` makes it CRITICAL and `Blocks: no` never lowers it; an inline
`Blocks: yes` is CRITICAL too, and with no finding above it is one. The native
adversarial voice's FIXABLE collects as CRITICAL (normalizer v2, GSTA-167). A label word used as a word is not a label: negated
by the word right before it ("not CRITICAL", "no High findings"), said of a negated subject ("Neither finding is CRITICAL",
"None of these is High"), a yardstick ("below CRITICAL", "worse than High"), quoted, or listed as vocabulary ("Critical|High").
A negation that describes the defect ("Without auth this is CRITICAL") leaves the label standing; a label the line opens with always counts, and a
closing "Overall:", "Summary:" or "Verdict:" line after the findings starts none (GST-435). An unlabeled bullet under a paragraph
finding whose Severity or Blocks line has not come yet is that finding's detail, so the field line after it lands on the finding.
by the word right before it ("not CRITICAL", "no High findings"), said of a negated subject in the same clause ("Neither finding is CRITICAL",
"None of these is High"; a "so", "but" or "and" ends the subject, so "None of the inputs are sanitized so this is CRITICAL" keeps its label),
a yardstick ("below CRITICAL", "worse than High"; "up to" and "at most" are not yardsticks), in straight or curly quotes (a backticked label
counts), or listed as vocabulary ("Critical|High"). A negation that describes the defect ("Without auth this is CRITICAL") leaves the label
standing; a label the line opens with, a label on a `Severity:` or `Blocks:` field line and a table cell always count. A closing "Overall:",
"Summary:" or "Verdict:" line after the findings starts none unless it carries `Blocks: yes`, a file:line, or a label stronger than every
finding before it (GST-435). An unnumbered, unlabeled bullet with no file:line, right under a paragraph finding whose Severity or Blocks
line has not come yet, is that finding's detail, so the field line after it lands on the finding; a numbered item or one after a blank line
is its own finding.
A phantom finding costs a fold; a lost one costs the gate.
A reply with no finding counts as a review only with the clean marker its own brief asks for: `NO FINDINGS` alone on a
line for the specialists, the batched specialist, the targeted reader, the Red Team and fix-check, the `Recommendation:`
@@ -493,9 +497,11 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends:
two-pass ledger checks pass-1 sends against pass-1 briefs; a later render prints as
`<reader>@2`), a collected output that is a review for every sent reader (`NOT_COLLECTED`
otherwise, and the output row `NOT_REVIEWED`; the marker is checked again in the output file when it is unchanged, so an
older ledger meets the same rule); a reader that failed is collected from its failure text, where any
label word becomes a finding the judge folds, and its coverage is logged
`unavailable` in the review-log row). A mismatched send re-sent
older ledger meets the same rule; a non-review the same dispatch later replaced with a review, by re-sending the brief, is `VOIDED`
and counted in `voided=N`). The outside voice may fail without blocking: `collect --unavailable '<reason>'` records its failure text
as missing coverage with no findings (`COLLECT_UNAVAILABLE`), verify prints its brief and output `UNAVAILABLE` and `unavailable=N` on the
verdict line, and its coverage is logged `unavailable` in the review-log row; any other reader is refused (exit 2), because its
missing coverage leaves the review incomplete. 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
+54 -30
View File
@@ -62,30 +62,38 @@ const ALL_LABELS = new RegExp(ANY_LABEL.source, 'g');
/** The label the line opens with, after a list marker, heading mark, table bar, bracket, bold or quote: always a label. */
const OPENS = /^(?:[-*+>]\s+|\d+[.)]\s+|#{1,6}\s*)?[\s[(*_`"'“‘|]*$/;
/**
* A label word used as a word, not as a label (GST-435): negated by the word right before it ("not CRITICAL", "no High findings",
* "isn't a CRITICAL", "no CRITICAL or High"), stated of a negated subject through a copula ("Neither finding is CRITICAL", "None of these is High", "No
* finding here is CRITICAL"), used as a yardstick ("below CRITICAL", "worse than High"), quoted ("'High'", `CRITICAL`) or listed as
* vocabulary ("Critical|High"). A negation that describes the defect ("Without auth this is CRITICAL") leaves the label standing. The
* clause runs back to the last sentence, colon, comma, dash or bracket.
* A label word used as a word, not as a label (GST-435): negated by the words right before it ("not CRITICAL", "no High findings",
* "isn't a CRITICAL", "doesn't rise to CRITICAL", "no CRITICAL or High"), said of a negated subject through a copula with no
* conjunction between ("Neither finding is CRITICAL", "None of these is High", "Nothing here is CRITICAL"), used as a yardstick
* ("below CRITICAL", "worse than High"), in quotation marks ("rated it 'High'") or listed as vocabulary ("Critical|High"). A negation
* that describes the defect ("Without auth this is CRITICAL", "None of the inputs are sanitized so this is CRITICAL") leaves the label
* standing, and so does every label in a table row; a Severity or Blocks field line keeps its quoted labels. The clause runs back to the last sentence,
* colon, comma, dash or bracket.
*/
const QUOTE_MARKS = '*_`"\'“‘';
const NEGATED = new RegExp(`\\b(?:not|no|never|nothing|without|zero|nor|isn't|aren't|wasn't|weren't)\\s+(?:(?:a|an|any|rated|at)\\s+)?(?:(?:${LABELS})\\s*(?:or|nor|and|/)\\s*)*[${QUOTE_MARKS}]*$`, 'i');
const NEGATED_SUBJECT = new RegExp(`^\\s*(?:neither|none|no\\s+(?:[\\w-]+\\s+)?(?:findings?|issues?|items?|defects?|problems?))\\b.*\\b(?:is|are|was|were|rises?\\s+to|reach(?:es)?|counts?\\s+as|rated)\\s+(?:(?:a|an)\\s+)?[${QUOTE_MARKS}]*$`, 'i');
const YARDSTICK = new RegExp(`\\b(?:below|under|than|short of|instead of|up to|at most)\\s+[${QUOTE_MARKS}]*$`, 'i');
const QUOTES: Record<string, string> = { '"': '"', "'": "'", '`': '`', '“': '”', '‘': '’' };
const NEGATOR = "not|no|never|nothing|without|zero|nor|isn't|aren't|wasn't|weren't|doesn't|don't|didn't|cannot|can't|won't";
const FILLER = '(?:a|an|any|rated|at|be|really|quite|even|rises?\\s+to|reach(?:es)?|counts?\\s+as)';
const NEGATED = new RegExp(`\\b(?:${NEGATOR})\\s+(?:${FILLER}\\s+)*(?:(?:${LABELS})\\s*(?:or|nor|and|/)\\s*)*["'“‘]?$`, 'i');
const CONJUNCTION = 'so|but|and|or|because|while|which|that|yet|though|although|since|whereas|if|when';
const NEGATED_SUBJECT = new RegExp(`^\\s*(?:neither|none|nothing|no\\s+(?:[\\w-]+\\s+)?(?:findings?|issues?|items?|defects?|problems?))\\b(?:(?!\\b(?:${CONJUNCTION})\\b).)*\\b(?:is|are|was|were|rises?\\s+to|reach(?:es)?|counts?\\s+as|rated)\\s+(?:(?:a|an)\\s+)?["'“‘]?$`, 'i');
const YARDSTICK = /\b(?:below|under|than|short of|instead of)\s+["'“‘]?$/i;
const QUOTES: Record<string, string> = { '"': '"', "'": "'", '“': '”', '‘': '’' };
const VOCAB_AFTER = new RegExp(`^\\|(?:${LABELS})(?![A-Za-z0-9])`);
const VOCAB_BEFORE = new RegExp(`(?<![A-Za-z0-9])(?:${LABELS})\\|$`);
function aWord(t: string, at: number, len: number): boolean {
const before = t.slice(0, at);
if (OPENS.test(before)) return false;
if (OPENS.test(before) || t.startsWith('|')) return false;
const after = t.slice(at + len);
if (QUOTES[before.slice(-1)] !== undefined && after.startsWith(QUOTES[before.slice(-1)]!)) return true;
if (VOCAB_AFTER.test(after) || VOCAB_BEFORE.test(before)) return true;
const field = META.test(t);
if (!field && QUOTES[before.slice(-1)] !== undefined && after.startsWith(QUOTES[before.slice(-1)]!)) return true;
if (!field && (VOCAB_AFTER.test(after) || VOCAB_BEFORE.test(before))) return true;
const clause = ' ' + before.slice(Math.max(...['.', ';', ':', '!', '?', ',', '—', '–', '(', ')'].map(c => before.lastIndexOf(c))) + 1);
return NEGATED.test(clause) || NEGATED_SUBJECT.test(clause) || YARDSTICK.test(clause);
}
/** A closing summary line ("Overall: …", "In summary, …", "Verdict: …") after the findings restates them; it starts no finding. */
const SUMMARY = /^(?:\*\*)?(?:in\s+)?(?:summary|overall|verdict|conclusion|bottom line)\b/i;
/**
* A closing summary line ("Overall: …", "In summary, …", "Verdict: …") after the findings restates them: it starts no finding unless
* it names a file:line, says `Blocks: yes` or carries a label stronger than every finding before it.
*/
const SUMMARY = /^(?:\*\*)?(?:in\s+summary|summary|overall|verdict|conclusion|bottom\s+line)\**\s*[:,]/i;
/** A file:line locus ("auth.ts:9"): prose naming one after a finding's fields starts the next finding. */
const LOCUS = /[\w./-]+\.\w+:\d+/;
/** A line that is only a finding's `Severity` or `Blocks` field ("Severity: High", "- Blocks: yes", "Severity High"). */
@@ -99,7 +107,8 @@ export interface MergedLine { id: string; reader: string; label: string; raw: st
* of the two labels). A `Severity` or `Blocks` field line continues the open finding, once per field: `Severity` raises its label,
* `Blocks: yes` makes it CRITICAL, `Blocks: no` never lowers it; with no finding open it is one, carrying the paragraph's subject.
* A list item, a heading, a bold or post-blank paragraph at the margin, or a line naming a file:line after the open finding's fields
* starts the next finding, except an unlabeled bullet under a paragraph finding whose fields have not come yet, which is its detail.
* starts the next finding, except an unlabeled, unnumbered bullet right under a paragraph finding whose fields have not come yet,
* which is its detail and joins its text.
* An inline `Blocks: yes` is CRITICAL too. A label word used as a word (negated, a yardstick, quoted or listed as vocabulary) is
* not a label, and a closing summary line after the findings starts none (GST-435). The native voice's FIXABLE collects as CRITICAL;
* other labels stay as written for lead judgment. When in doubt the parser merges rather than drops: a lost blocker costs the gate.
@@ -120,7 +129,8 @@ export function parseFindings(reader: string, output: string): Array<{ label: st
if (!t) { gap = true; continue; }
const afterGap = gap;
gap = false;
if (/^NO FINDINGS\.?$/.test(t) || /^Recommendation:/i.test(t) || (out.length && SUMMARY.test(t))) { current = null; continue; }
if (/^NO FINDINGS\.?$/.test(t) || /^Recommendation:/i.test(t)) { current = null; continue; }
if (out.length && SUMMARY.test(t) && !BLOCKS_YES.test(t) && !LOCUS.test(t) && (rank(strongest(t)) ?? -1) <= Math.max(...out.map(o => rank(o.label) ?? 0))) { current = null; continue; }
const indented = line !== line.trimStart();
const label = strongest(t);
const blocks = BLOCKS_YES.test(t);
@@ -147,9 +157,10 @@ export function parseFindings(reader: string, output: string): Array<{ label: st
if (typeof o.severity === 'string') { current = push(o.severity, t); continue; }
} catch { /* prose that starts with a brace */ }
}
// An unlabeled bullet under a paragraph finding whose fields have not come yet is its detail, not the next finding (GSTA-196 native voice).
const detail = listItem && !label && !section && !!current && !current.list && !current.fields.size;
// An unlabeled, unnumbered bullet right under a paragraph finding whose fields have not come yet is its detail (GSTA-196 native voice).
const detail = listItem && /^[-*+]\s/.test(t) && !afterGap && !locus && !label && !section && !!current && !current.list && !current.fields.size;
const fresh = (listItem && !detail) || (!indented && !detail && (t.startsWith('**') || afterGap)) || (!!current?.fields.size && locus);
if (detail && !blocks) { current!.raw += ` | ${t}`; continue; }
if (fresh) { current = null; head = t; }
const inherited = section && listItem ? section : undefined;
const found = label && inherited ? stronger(inherited, label) : label ?? inherited;
@@ -181,16 +192,22 @@ const CLEAN_MARKERS: Record<string, Array<[string, RegExp]>> = {
export const cleanMarkers = (reader: string) => CLEAN_MARKERS[reader.split('/')[0]!] ?? [NO_FINDINGS, RECOMMENDATION];
const hasCleanMarker = (reader: string, output: string) => output.replace(/\r\n/g, '\n').split('\n').some(l => cleanMarkers(reader).some(([, re]) => re.test(l.trim())));
/** collect: hash the raw output, append its findings to merged.jsonl and record both; a reply with no finding and no clean marker records review: false. */
export function collect(ledger: string, reader: string, output: string, mergedFile: string, outputFile?: string): { sha: string; lines: MergedLine[]; review: boolean } {
/**
* collect: hash the raw output, append its findings to merged.jsonl and record both; a reply with no finding and no clean marker records
* review: false. With unavailable (the outside voice only) the output is its failure text, recorded as missing coverage.
*/
export function collect(ledger: string, reader: string, output: string, mergedFile: string, outputFile?: string, unavailable?: string): { sha: string; lines: MergedLine[]; review: boolean } {
const sha = promptHash(output);
const prior = readLedger(ledger).filter(e => e.event === 'output' && e.reader === reader).length;
const lines = parseFindings(reader, output).map((f, i) => ({ id: `${reader}#${prior ? `${prior + 1}.` : ''}${i + 1}`, reader, label: f.label, raw: f.raw, output_sha256: sha }));
const review = lines.length > 0 || hasCleanMarker(reader, output);
const lines = unavailable ? [] : parseFindings(reader, output).map((f, i) => ({ id: `${reader}#${prior ? `${prior + 1}.` : ''}${i + 1}`, reader, label: f.label, raw: f.raw, output_sha256: sha }));
const review = !unavailable && (lines.length > 0 || hasCleanMarker(reader, output));
fs.appendFileSync(mergedFile, lines.map(l => JSON.stringify(l) + '\n').join(''), { mode: 0o600 });
appendEvent(ledger, { event: 'output', reader, sha256: sha, file: outputFile ? path.resolve(outputFile) : null, merged: path.resolve(mergedFile), ids: lines.map(l => l.id), review });
appendEvent(ledger, { event: 'output', reader, sha256: sha, file: outputFile ? path.resolve(outputFile) : null, merged: path.resolve(mergedFile), ids: lines.map(l => l.id), review, ...(unavailable ? { unavailable } : {}) });
return { sha, lines, review };
}
/** The outside voice may fail without blocking (ship/sections/adversarial.md): its failure is recorded as unavailable coverage, never as a review. */
export const mayBeUnavailable = (reader: string) => reader.split('/')[0] === 'outside-voice';
const unavailableOutput = (o: LedgerEvent) => typeof o.unavailable === 'string' && mayBeUnavailable(String(o.reader));
export function readJsonl<T>(file: string): T[] {
return fs.readFileSync(file, 'utf8').split('\n').filter(l => l.trim()).map(l => JSON.parse(l) as T);
@@ -256,7 +273,7 @@ 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; voided: number } {
export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; briefs: number; voided: number; unavailable: number } {
const events = readLedger(ledger);
const rows: VerifyRow[] = [];
const short = (s: unknown) => (typeof s === 'string' ? s.slice(0, 12) : '-');
@@ -287,9 +304,10 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri
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 status = !sent.length ? 'NOT_SENT' : mismatched ? 'MISMATCH' : collected ? 'MATCH' : 'NOT_COLLECTED';
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' : '';
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}` : ''}${outputs.length && !collected ? ' not_reviewed' : ''}${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}` : ''}${outputs.length && !collected && !unavailable ? ' not_reviewed' : ''}${tampered}` });
}
for (const [i, o] of events.entries()) {
if (o.event !== 'output') continue;
@@ -306,7 +324,13 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri
try { lines = typeof o.merged === 'string' ? readJsonl<MergedLine>(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'); }
if (status === 'MATCH' && !reviewed(o)) { status = 'NOT_REVIEWED'; notes.push('no_finding_no_clean_marker'); }
if (status === 'MATCH' && !reviewed(o)) {
// A non-review the same dispatch later replaced with a review (the brief re-sent and answered) is void, counted like a voided send.
const replaced = !!p && events.slice(i + 1, p.next).some(e => e.event === 'output' && e.reader === reader && reviewed(e));
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 { 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(' ') : ''}` });
}
const lastJudge = events.map(e => e.event).lastIndexOf('judge');
@@ -328,6 +352,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 = episodes.length > 0 && rows.every(r => r.status === 'MATCH' || r.status === 'OK');
return { rows, clean, briefs: episodes.length, voided };
const clean = episodes.length > 0 && rows.every(r => ['MATCH', 'OK', 'VOIDED', 'UNAVAILABLE'].includes(r.status));
return { rows, clean, briefs: episodes.length, voided, unavailable: rows.filter(r => r.kind === 'BRIEF' && r.status === 'UNAVAILABLE').length };
}
+50 -2
View File
@@ -418,7 +418,8 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change
fs.writeFileSync(judged, r.lines.map(l => JSON.stringify({ id: l.id, severity: 'INFORMATIONAL' })).join('\n') + '\n');
expect(judge(readJsonl<MergedLine>(merged), readJsonl(judged)).ok).toBe(true);
for (const word of ['Neither finding is CRITICAL.', 'This is not CRITICAL.', 'There are no CRITICAL findings.', 'None of these is High.', 'The race sits below CRITICAL on the floor.',
'It ranks lower than High.', "Pass 1 rated it 'High'; the fix closes it.", 'I used the `CRITICAL` label for none of them.', 'Severity vocabulary: Critical|High|Medium|Low'])
'It ranks lower than High.', "Pass 1 rated it 'High'; the fix closes it.", 'The brief\'s word "CRITICAL" fits none of them.', 'Severity vocabulary: Critical|High|Medium|Low',
"This doesn't rise to CRITICAL.", 'It cannot be CRITICAL.', 'Nothing here is CRITICAL.', 'This is not really CRITICAL.'])
expect([word, parseFindings('outside-voice', word)]).toEqual([word, []]);
expect(parseFindings('outside-voice', '1. **High**: retry loop never backs off (q.ts:78)\n\nOverall: one High finding, nothing CRITICAL.').map(f => f.label)).toEqual(['High']);
expect(parseFindings('outside-voice', 'Severity: Low (not High)\n').map(f => f.label)).toEqual(['Low']);
@@ -426,8 +427,16 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change
['| High | race in queue.ts:4 |', 'High'], ['| `High` | race in queue.ts:4 |', 'High'], ['The retry drops writes, a CRITICAL defect (q.ts:78).', 'CRITICAL'], ['[P1] token not expired', 'P1'],
['Not checking expiry at auth.ts:9 lets stale tokens through. Severity: High', 'High'], ['Summary: CRITICAL auth bypass at a.ts:3', 'CRITICAL'], ["The lock isn't released on error (pool.ts:12): High", 'High'],
['1. Without auth this is CRITICAL (a.ts:3)', 'CRITICAL'], ['- Input is never sanitized CRITICAL', 'CRITICAL'], ['1. Requests with no token pass High (a.ts:4)', 'High'],
['No bounds check lets CRITICAL data leak (a.ts:3)', 'CRITICAL'], ['|auth.ts:9|token never expires|CRITICAL|', 'CRITICAL']];
['No bounds check lets CRITICAL data leak (a.ts:3)', 'CRITICAL'], ['|auth.ts:9|token never expires|CRITICAL|', 'CRITICAL'],
['None of the inputs are sanitized so this is CRITICAL (a.ts:3)', 'CRITICAL'], ['Neither path checks auth so this is CRITICAL (a.ts:3)', 'CRITICAL'],
['No defects in parsing but the upload handler is CRITICAL.', 'CRITICAL'], ['A retry storm could escalate up to CRITICAL.', 'CRITICAL'],
['|1|FIXABLE|High|race in queue.ts:78|', 'High'], ['**Token leak in log.ts:3**\nSeverity: `Critical`', 'Critical']];
for (const [line, label] of kept) expect([line, parseFindings('outside-voice', line).map(f => f.label)]).toEqual([line, [label]]);
// Pre-landing review (#3677 Step 11): a summary-led line with a locus or Blocks: yes, or a stronger label than any finding so far,
// still collects; a backticked native classification still collects as CRITICAL.
expect(parseFindings('outside-voice', '1. **High**: race in a.ts:1\nSummary endpoint in summary.ts:9 leaks tokens. Severity: High. Blocks: yes').map(f => f.label)).toEqual(['High', 'CRITICAL']);
expect(parseFindings('outside-voice', '1. **Low**: noisy log in a.ts:1\n\nOverall: the upload handler is CRITICAL.').map(f => f.label)).toEqual(['Low', 'CRITICAL']);
expect(parseFindings('adversarial-native', '**1. Race in a.ts:3 (`FIXABLE`).**\nSeverity: Low. Blocks: no').map(f => f.label)).toEqual(['CRITICAL']);
});
// Value: protects=the mirror of GST-435: a refusal whose prose names severity words ("no CRITICAL or High findings to report") is
@@ -452,6 +461,45 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change
.toEqual([['High', '**1. A PR in', true], ['CRITICAL', '**2. A case ', true], ['CRITICAL', '**3. Local `', true], ['Low', '**4. No trai', true]]);
expect(parseFindings('outside-voice', '1. FIXABLE: race in a.ts:3\n- Unlabeled sibling at b.ts:9\nSeverity: Medium').map(f => f.label)).toEqual(['FIXABLE', 'Medium']);
expect(parseFindings('outside-voice', '**Retry storm in r.ts:2 (INVESTIGATE).**\n- detail\n- High: a second defect in s.ts:4\nSeverity: Medium').map(f => f.label)).toEqual(['INVESTIGATE', 'High']);
expect(parseFindings('outside-voice', 'Low: cache TTL mismatch (cache.ts:4)\n\n2. Token never expires (auth.ts:9)\nSeverity: High\nBlocks: yes').map(f => [f.label, f.raw]))
.toEqual([['Low', 'Low: cache TTL mismatch (cache.ts:4)'], ['CRITICAL', '2. Token never expires (auth.ts:9) | Severity: High | Blocks: yes']]);
expect(parseFindings('outside-voice', '**Race in a.ts:3 (High).**\n- the writer holds no lock\nSeverity: Medium').map(f => f.raw)).toEqual(['**Race in a.ts:3 (High).** | - the writer holds no lock | Severity: Medium']);
});
// Value: protects=a non-review the same dispatch later answers with a review is voided (counted, VERIFY clean), and a failed outside
// voice recorded with --unavailable reads UNAVAILABLE (missing coverage, never a review) without blocking, while --unavailable is
// refused for any other reader; fails_when=one outside-voice timeout or one refusal makes VERIFY dirty for good, or --unavailable
// lets a specialist skip coverage; why_new=#3677 Step 11 adversarial voice; seam=collect and verify; no_claim=the review-log row
test('a refusal replaced by a review is voided; an outside voice failure is UNAVAILABLE; --unavailable is the outside voice only', () => {
const p = pass();
const send = () => run(['sent', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--sent', p.file]);
const take = (f: string) => run(['collect', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--output', f, '--merged', p.merged]);
send();
expect(take(p.w('refusal.out', 'I could not access the diff.\n')).status).toBe(1);
expect(run(['verify', '--ledger', p.ledger]).stdout).toContain('VERIFY: dirty');
send();
expect(take(p.w('clean.out', 'NO FINDINGS\n')).status).toBe(0);
fs.writeFileSync(path.join(p.out, 'judged.jsonl'), '');
run(['judge', '--ledger', p.ledger, '--merged', p.merged, '--judged', path.join(p.out, 'judged.jsonl')]);
const v = run(['verify', '--ledger', p.ledger]);
expect([v.status, v.stdout.includes('replaced_by_a_review'), v.stdout.includes('VERIFY: clean briefs=1 outputs=2 voided=1')]).toEqual([0, true, true]);
const head = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: ROOT, encoding: 'utf8', timeout: 10_000 }).stdout.trim();
const o = tmp();
const ov = field(run(['render', '--role', 'outside-voice', '--out', o, '--diff-base', head]).stdout, 'file');
const ledger = path.join(o, 'review-ledger.jsonl'), merged = path.join(o, 'merged.jsonl');
run(['sent', '--ledger', ledger, '--role', 'outside-voice', '--sent', ov]);
const fail = path.join(o, 'timeout.out');
fs.writeFileSync(fail, 'Codex timed out after 9 minutes and was terminated; this pass produced NO findings.\n');
const c = run(['collect', '--ledger', ledger, '--role', 'outside-voice', '--output', fail, '--merged', merged, '--unavailable', 'timeout']);
expect([c.status, c.stdout.startsWith('COLLECT_UNAVAILABLE: reader=outside-voice ')]).toEqual([0, true]);
fs.writeFileSync(path.join(o, 'judged.jsonl'), '');
run(['judge', '--ledger', ledger, '--merged', merged, '--judged', path.join(o, 'judged.jsonl')]);
const w = run(['verify', '--ledger', ledger]);
expect([w.status, w.stdout.includes('BRIEF outside-voice UNAVAILABLE'), w.stdout.includes('OUTPUT outside-voice UNAVAILABLE'), w.stdout.includes(' unavailable=1\n')]).toEqual([0, true, true, true]);
const q = pass();
run(['sent', '--ledger', q.ledger, '--role', 'specialist', '--name', 'testing', '--sent', q.file]);
const refused = run(['collect', '--ledger', q.ledger, '--role', 'specialist', '--name', 'testing', '--output', q.w('t.out', 'timed out\n'), '--merged', q.merged, '--unavailable', 'timeout']);
expect([refused.status, refused.stderr.includes('--unavailable is for the outside voice only')]).toEqual([2, true]);
});
test('verify on a ledger that kept no prompts says so and exits 3', () => {