mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 22:14:25 +01:00
v1.91.367.0 fix(ship): collect never counts a non-review as clean, and a severity word used as a word is no finding (GST-434, GST-435) (#3677)
* fix(ship): collect counts a reply with no finding and no clean marker as a non-review, never clean (GSTA-167) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship): a label word used as a word is no finding, and a paragraph finding keeps its detail bullets (GST-435) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship): a negation counts only when it applies to the label; compact table rows keep their label; register the review-ledger fixtures (pre-landing review) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * 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> * fix(ship): bold and code marks keep a negated label a word, a subject phrase with and/or stays negated, table negation counts, --unavailable refuses an output with findings, VOIDED needs a re-send (fix-check) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship): a quoted label counts at the end of its line and in table cells, which ends a negated subject, and a negated subject never cancels a label on a file:line line (fix-check) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship): a line naming a file:line or a table row always keeps a finding; "of which" stays inside a negated subject (fix-check) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test(ship): the review-pass fake outside voice ends with its Recommendation: line, the marker its brief prescribes (#3665 integration) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * revert(ship): parseFindings labels and negation back to main; GST-435 moves to its own PR (seed branch gsta-435-seed). Keeps the non-review rule and the detail-bullet merge (split) 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>
This commit is contained in:
1 parent
12dc6782c2
commit
41a67b996d
12 files changed
+307
-32
No files matched your search
@@ -1,5 +1,15 @@
|
||||
# Changelog
|
||||
|
||||
## [1.91.367.0] - 2026-10-31
|
||||
|
||||
**/ship's review collection no longer counts a reviewer that says it could not review as clean.** 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. 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. A failure text that still holds findings is refused there and collected normally, so nothing is lost.
|
||||
- **`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 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 with no file and line of its own counts as detail; a numbered item is still its own finding.
|
||||
- **Every other finding collects exactly as before.** 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. 2,196 of the 2,197 collect the same findings with the same labels. The one difference is the GSTA-196 native sample, where 8 fragments join into 4 findings, each at its strongest fragment's label.
|
||||
|
||||
## [1.91.366.0] - 2026-10-31
|
||||
|
||||
**A partial `gstack-post pr-body` post keeps the PR's intro.** Since GST-412, any body file with a `## ` section replaced the text above the PR's first heading, so a lane that posted only `## Screenshots` deleted a human intro or a `Review-loop exception:` line under the receipt (GStack Minder: "a partial pr-body post must keep the human intro").
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
# gstack digest v1.91.366.0 — regenerate/re-copy after upgrading gstack
|
||||
# gstack digest v1.91.367.0 — regenerate/re-copy after upgrading gstack
|
||||
|
||||
Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed
|
||||
for agent hosts without a full skill install. The full skills add workflows,
|
||||
|
||||
@@ -20,7 +20,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 { collect, readJsonl, readLedger, readerKey, recordJudgment, recordSent, appendEvent, verify, type JudgedLine, type MergedLine, type VerifyRow } from '../lib/review-ledger';
|
||||
import { cleanMarkers, collect, mayBeUnavailable, parseFindings, readJsonl, readLedger, readerKey, recordJudgment, recordSent, appendEvent, verify, type JudgedLine, type MergedLine, type VerifyRow } from '../lib/review-ledger';
|
||||
import { buildManifest, chunkCount, chunkOf, manifestPath, markReader, readManifest, renderStage, writeManifest, STAGES, type Extra, type Manifest, type PassReader, type ReaderState, type Stage } from '../lib/review-pass';
|
||||
import type { Dispatch } from '../lib/review-depth';
|
||||
import { runScriptSync } from '../lib/script-invocation';
|
||||
@@ -42,7 +42,11 @@ const HELP = `gstack-review-dispatch — verbatim reviewer briefs and loss-free
|
||||
sent --ledger <f> --role <role> [--name <n>] --sent <file>
|
||||
the exact prompt a dispatch passed: BRIEF_SENT: … match, or BRIEF_MISMATCH: … (exit 1; the pass is not reviewed)
|
||||
collect --ledger <f> --role <role> [--name <n>] --output <file> --merged <merged.jsonl>
|
||||
the reader's raw output: hashes it and appends each finding to merged.jsonl with its label as written
|
||||
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; refused when the
|
||||
output holds findings) 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
|
||||
@@ -268,7 +272,16 @@ 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; }
|
||||
if (unavailable !== undefined && !unavailable.trim()) { process.stderr.write('gstack-review-dispatch collect: --unavailable needs a reason\n'); return EXIT.usage; }
|
||||
if (unavailable !== undefined && parseFindings(reader, output).length) { process.stderr.write(`gstack-review-dispatch collect: ${reader}'s output holds findings; collect it without --unavailable so none is lost\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;
|
||||
}
|
||||
process.stdout.write(`COLLECT: reader=${reader} findings=${r.lines.length} sha256=${r.sha} merged=${merged}\n`);
|
||||
return EXIT.ok;
|
||||
}
|
||||
@@ -290,7 +303,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`);
|
||||
|
||||
+16
-5
@@ -473,7 +473,14 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends:
|
||||
second line for the same field starts another: `Severity` raises its label,
|
||||
`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 phantom finding costs a fold; a lost one costs the gate.
|
||||
adversarial voice's FIXABLE collects as CRITICAL (normalizer v2, GSTA-167). An unnumbered, unlabeled bullet with no file:line
|
||||
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 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:`
|
||||
line for the outside voice, either for the native adversarial voice. Without one (a refusal, an empty reply, a reader
|
||||
that never saw the diff) collect records `review: false`, prints `COLLECT_NOT_REVIEWED` and exits 1.
|
||||
4. Lead judgment writes `judged.jsonl`; `judge` refuses a set that drops a merged
|
||||
finding, lowers its severity, folds it into a less severe duplicate, or adds or
|
||||
promotes one without a reason (`JUDGMENT_REJECTED`). A merged file missing any id
|
||||
@@ -481,10 +488,14 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends:
|
||||
5. `verify --ledger <f>` replays all of it from the run's ledger: rendered against
|
||||
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
|
||||
`<reader>@2`), a collected output for every sent reader (`NOT_COLLECTED`
|
||||
otherwise; 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
|
||||
`<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 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 (`COLLECT_UNAVAILABLE`); an empty reason or an output that holds findings is refused (exit 2), so a partial
|
||||
answer is collected normally and loses nothing. 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
|
||||
|
||||
+66
-17
@@ -4,7 +4,7 @@
|
||||
*
|
||||
* rendered a brief gstack-review-dispatch render wrote (role, name, file, sha256)
|
||||
* sent the prompt a dispatch program actually passed, by hash; a mismatch fails the pass
|
||||
* output a reader's raw output (sha256) and the merged.jsonl ids collect wrote from it
|
||||
* output a reader's raw output (sha256), the merged.jsonl ids collect wrote from it, and whether it was a review at all
|
||||
* judgment each lead-judgment change: an added or promoted finding, with its reason
|
||||
* judge the verdict on the parent's judged set (ok, or the dropped and downgraded ids)
|
||||
*
|
||||
@@ -72,11 +72,12 @@ 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. An inline `Blocks: yes` is CRITICAL too. 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.
|
||||
* starts the next finding, except an unlabeled, unnumbered bullet with no file:line under a paragraph finding whose fields have not
|
||||
* come yet, which is its detail and joins its text (GSTA-196: 4 findings with detail bullets collected as 8). An inline
|
||||
* `Blocks: yes` is CRITICAL too. 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.
|
||||
*/
|
||||
export function parseFindings(reader: string, output: string): Array<{ label: string; raw: string }> {
|
||||
type Finding = { label: string; raw: string; fields: Set<string> };
|
||||
type Finding = { label: string; raw: string; fields: Set<string>; list: boolean };
|
||||
const out: Finding[] = [];
|
||||
let section: string | null = null;
|
||||
let current: Finding | null = null;
|
||||
@@ -85,7 +86,7 @@ export function parseFindings(reader: string, output: string): Array<{ label: st
|
||||
const role = reader.split('/')[0];
|
||||
const named = (l: string) => (role === 'adversarial-native' && l.toUpperCase() === 'FIXABLE' ? 'CRITICAL' : l);
|
||||
const strongest = (t: string) => [...t.matchAll(ALL_LABELS)].map(m => named(m[1]!)).reduce<string | undefined>((a, b) => (a === undefined ? b : stronger(a, b)), undefined);
|
||||
const push = (label: string, raw: string, fields: string[] = []): Finding => { const f = { label, raw, fields: new Set(fields) }; out.push(f); return f; };
|
||||
const push = (label: string, raw: string, fields: string[] = []): Finding => { const f = { label, raw, fields: new Set(fields), list: LIST_ITEM.test(raw) }; out.push(f); return f; };
|
||||
for (const line of output.replace(/\r\n/g, '\n').split('\n')) {
|
||||
const t = line.trim();
|
||||
if (!t) { gap = true; continue; }
|
||||
@@ -118,7 +119,9 @@ 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 */ }
|
||||
}
|
||||
const fresh = listItem || (!indented && (t.startsWith('**') || afterGap)) || (!!current?.fields.size && locus);
|
||||
const detail = listItem && /^[-*+]\s/.test(t) && !locus && !label && !section && !!current && !current.list && !current.fields.size;
|
||||
if (detail && !blocks) { current!.raw += ` | ${t}`; continue; }
|
||||
const fresh = (listItem && !detail) || (!indented && !detail && (t.startsWith('**') || afterGap)) || (!!current?.fields.size && locus);
|
||||
if (fresh) { current = null; head = t; }
|
||||
const inherited = section && listItem ? section : undefined;
|
||||
const found = label && inherited ? stronger(inherited, label) : label ?? inherited;
|
||||
@@ -133,15 +136,39 @@ export function parseFindings(reader: string, output: string): Array<{ label: st
|
||||
return out.map(({ label, raw }) => ({ label, raw }));
|
||||
}
|
||||
|
||||
/** collect: hash the raw output, append its findings to merged.jsonl and record both. */
|
||||
export function collect(ledger: string, reader: string, output: string, mergedFile: string, outputFile?: string): { sha: string; lines: MergedLine[] } {
|
||||
/**
|
||||
* The clean marker each reader's own brief prescribes for a review that found nothing: `NO FINDINGS` alone on a line for the
|
||||
* specialists, the batched specialist and the targeted reader (ship/sections/review-army.md, the specialist brief; review/specialists/*.md),
|
||||
* the Red Team (its brief in ship/sections/review-army.md) and fix-check (scripts/resolvers/fix-check.ts); the `Recommendation:` line
|
||||
* for the outside voice (ship/sections/adversarial.md, the outside prompt); either for the native adversarial voice, whose brief asks
|
||||
* for both. A reply with no finding and no marker (a refusal, an empty reply, a reader that never saw the diff) is not a review.
|
||||
*/
|
||||
const NO_FINDINGS: [string, RegExp] = ['NO FINDINGS', /^[`*]*NO FINDINGS\.?[`*]*$/];
|
||||
const RECOMMENDATION: [string, RegExp] = ['Recommendation:', /^[`*]*Recommendation:/i];
|
||||
const CLEAN_MARKERS: Record<string, Array<[string, RegExp]>> = {
|
||||
specialist: [NO_FINDINGS], 'specialist-batched': [NO_FINDINGS], targeted: [NO_FINDINGS], 'red-team': [NO_FINDINGS], 'fix-check': [NO_FINDINGS],
|
||||
'adversarial-native': [NO_FINDINGS, RECOMMENDATION], 'outside-voice': [RECOMMENDATION],
|
||||
};
|
||||
/** The markers a reader's zero-finding reply may carry; a role outside the brief roles takes either. */
|
||||
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. 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 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) });
|
||||
return { sha, lines };
|
||||
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);
|
||||
@@ -190,13 +217,24 @@ 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 (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;
|
||||
}
|
||||
|
||||
/** 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[]) : []));
|
||||
|
||||
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) : '-');
|
||||
@@ -225,10 +263,12 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri
|
||||
}
|
||||
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 outputs = lastMatch ? inside.filter(({ e, i }) => e.event === 'output' && i > lastMatch.i) : [];
|
||||
const collected = outputs.some(({ e }) => reviewed(e));
|
||||
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}` : ''}${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;
|
||||
@@ -245,6 +285,15 @@ 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)) {
|
||||
// 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));
|
||||
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 { 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');
|
||||
@@ -266,6 +315,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 };
|
||||
}
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "gstack",
|
||||
"version": "1.91.366",
|
||||
"version": "1.91.367",
|
||||
"description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.",
|
||||
"license": "MIT",
|
||||
"type": "module",
|
||||
|
||||
+43
@@ -0,0 +1,43 @@
|
||||
Adversarial review (native) of `1b68e637c07c..0a8efed`. HEAD is still `0a8efeda8f79`, the working tree is clean and the same 10 files changed, so the findings below are the ones I reported earlier, rechecked against this range.
|
||||
|
||||
**Coverage:** I read the full diffs of the non-fixture source: `.github/workflows/{ci,sealed,train-gate}.yml`, `scripts/gate.ts`, `gate/manifest.json`, `package.json` and `README.md`. **I reviewed the test files in summary mode only**, so their coverage of the review is reduced:
|
||||
- `tests/ci/gate.test.ts` is new (+299). It covers the train chain walk, `routeDispatch` refusals, `publishable` binding and the gate-surface rule, and `runCases`/junit judging.
|
||||
- `tests/ci/sealed-workflow.test.ts` (+19/−6) adds the push trigger.
|
||||
- `tests/ci/workflow-hygiene.test.ts` is new (+85). It covers SHA pins, read-only tokens, no secrets, the checksummed Bun install, the gstack pin and parity between the manifest and ci.yml.
|
||||
|
||||
**1. A PR in the train controls the workflow that judges the train (INVESTIGATE).** `workflow_dispatch` runs the copy of `train-gate.yml` at the dispatched ref, `train/<nonce>`, and that copy contains every train PR's edits. So a PR that edits `train-gate.yml` changes the `route`, `test` and `publish` jobs themselves.
|
||||
- It can drop the call to the base's `gate.ts publish`. It can also post the `full_gate_check` check run unconditionally with the publish job's `checks: write`, or give itself more `permissions:` and use repository secrets.
|
||||
- The header says "a pull request in the train cannot change how the train is judged". In practice only the case list and `gate.ts` come from the base.
|
||||
- The "a gate surface PR rides alone" rule (`publishable`, `slots > 1`) is enforced by the publish job, which is code the PR can rewrite. So the rule holds only against PRs that don't break it.
|
||||
- `ci.yml`'s new `workflow_dispatch` has the same property.
|
||||
- Real mitigations would run the judging workflow from the default branch, for example `repository_dispatch` or a dispatch on `main` that takes the train head as an input, or have gstack verify which workflow file produced the check. Either one changes the protocol with gstack.
|
||||
- Today an attacker needs a malicious workflow edit to pass review and READY, and no workflow references a secret.
|
||||
|
||||
Severity: High. Blocks: no.
|
||||
|
||||
**2. A case timeout leaks processes and can lose the record (FIXABLE).** `runCases` calls `spawnSync(..., { timeout })` (`scripts/gate.ts:252`), which kills only the `bash` wrapper. Grandchildren such as `bun test` survive with inherited stdio and keep running into later cases.
|
||||
- A 20-minute case timeout (`CASE_TIMEOUT_MS`) plus the other cases can go past the job's 30-minute `timeout-minutes`.
|
||||
- GitHub then kills `gate.ts` before it writes `train-record.json`. That run has no record and fails as infrastructure, not as a red verdict.
|
||||
- Fix: spawn each case detached and kill its whole process group on timeout, and keep the per-case cap well under the job limit (or size the job to cases × cap).
|
||||
|
||||
Severity: Low. Blocks: no.
|
||||
|
||||
**3. Local `run` hides a broken train chain (FIXABLE).** In `gate.ts run` (`:386`), when `trainBase` returns `{ error }`, the code falls back to `merge-base HEAD origin/<default>` without saying so. A malformed train run locally is judged against the wrong base.
|
||||
- CI always passes `--base`, so this affects only local `bun run gate`.
|
||||
- Fix: fail on an error result, the way `route` and `publish` already do.
|
||||
|
||||
Severity: Low. Blocks: no.
|
||||
|
||||
**4. No train policy is on the base, so trains cannot run yet (INVESTIGATE).** `.gstack/ship-policy.json` does not exist at HEAD. Until a commit with a `train` block lands on main, `route` refuses every train and bisect dispatch with "needs a train block". The workflow cannot gate a train at merge, and this PR does not add the file.
|
||||
- Related: `bun.lock` is not in `gate_surface`. A multi-PR train can change resolved dependency versions within `package.json` ranges, for example `@types/bun ^1.4.4`, which changes what `typecheck` and `unit` run against.
|
||||
|
||||
Severity: Low. Blocks: no.
|
||||
|
||||
**Ruled out:**
|
||||
- `grep -q` after `tee` in the sealed case is not a SIGPIPE risk. `SEALED:` is the last and only line on gstack-pregate's stdout.
|
||||
- The empty wiki `--base` on dispatch is safe: `resolveBase` falls back to `origin/HEAD`.
|
||||
- `Bun.Glob("**/tsconfig*.json")` matches `tsconfig.json` at the repository root.
|
||||
- The bisect artifact name matches gstack's `bisectArtifact`.
|
||||
- `route` refuses when the bash T1 lookup and `trainBase` disagree.
|
||||
|
||||
Recommendation: Ship as-is and send finding 1 to the train-protocol owner because the strongest issue, a train PR rewriting `train-gate.yml` to post `full_gate_check` itself, needs a malicious workflow edit to pass review, and fixing it means changing the dispatch protocol with gstack, not this diff.
|
||||
@@ -0,0 +1,5 @@
|
||||
I couldn't run the review because the diff base doesn't exist. `git diff 1b68e637c07cea38f01c01120e02d184575da82c` fails with "bad object", and fetching that sha from origin is refused with "not our ref", so the commit is neither in this checkout nor on the remote.
|
||||
|
||||
This checkout is on `main` at 047dad7 (v1.91.357.0), tracking origin/main with no uncommitted changes, so there's nothing local to review against either. I'm not returning NO FINDINGS, because that would claim a clean diff I never saw.
|
||||
|
||||
To continue I need one of these: a diff base that exists, or the branch or sha holding the c3-36-p1 change. It may only exist on another machine and was never pushed. I've read all three checklists (testing, security, state-machines) and will apply them as soon as I have a diff.
|
||||
@@ -195,6 +195,7 @@ export const FREE_FIXTURES: Readonly<Record<string, readonly string[]>> = {
|
||||
'test/fixtures/redact-diagram/**': ['test/redact-context-exemptions.test.ts'],
|
||||
'test/fixtures/remote-identity-vectors.json': ['test/bin-context-windows-slug.test.ts', 'test/gstack-slug-parity.test.ts', 'test/remote-identity-vectors.test.ts'],
|
||||
'test/fixtures/review-depth/**': ['test/review-dispatch-compare.test.ts'],
|
||||
'test/fixtures/review-ledger/**': ['test/review-dispatch.test.ts'],
|
||||
'test/fixtures/review-design-lite-reports-ci-36633323521.json': ['test/review-finalization-budget.test.ts'],
|
||||
'test/fixtures/review-gate-integrity-opus-reports.json': ['test/gate-integrity-templates.test.ts'],
|
||||
'test/fixtures/session-update-heartbeat.sh': ['test/session-update-heartbeat.test.ts'],
|
||||
|
||||
@@ -4,7 +4,7 @@ import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { BriefError, promptHash, renderBrief, sectionTexts } from '../lib/review-brief';
|
||||
import { judge, parseFindings, type MergedLine } from '../lib/review-ledger';
|
||||
import { collect, judge, parseFindings, type MergedLine } from '../lib/review-ledger';
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
const BIN = path.join(ROOT, 'bin', 'gstack-review-dispatch');
|
||||
@@ -326,6 +326,148 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change
|
||||
expect(parseFindings('outside-voice', '1. Retry loses writes (queue.ts:78). Severity: Medium\n Fix: reject the batch. Blocks: yes')).toEqual([{ label: 'CRITICAL', raw: '1. Retry loses writes (queue.ts:78). Severity: Medium | Fix: reject the batch. Blocks: yes' }]);
|
||||
});
|
||||
|
||||
// Value: protects=a reader reply with no finding and no clean marker (a refusal, an empty reply, a reader that never saw the diff) is
|
||||
// recorded as a non-review: collect exits 1 and verify reports the brief NOT_COLLECTED, never clean; fails_when=collect hashes the
|
||||
// GSTA-196 refusal ("I'm not returning NO FINDINGS"), an empty file or an "I could not access the diff" reply as findings=0 and verify
|
||||
// reads MATCH (replay C3-36: the batched specialist found no diff base and its brief read MATCH);
|
||||
// why_new=collect recorded whatever parseFindings returned and verify counted any output as collected; seam=collect and verify;
|
||||
// no_claim=that a reply carrying a marker actually reviewed the diff
|
||||
test('a reply with no finding and no clean marker is a non-review: collect exits 1 and verify is NOT_COLLECTED, never clean', () => {
|
||||
const refusal = fs.readFileSync(path.join(ROOT, 'test/fixtures/review-ledger/gsta-196-refusal.txt'), 'utf8');
|
||||
expect(promptHash(refusal)).toBe('7150c3b9ffe2698f77a988fa422b66d7004e1939d12e81185e8d556a47dff784');
|
||||
const b = tmp();
|
||||
const batched = field(run(['render', '--role', 'specialist-batched', '--checklists', 'testing,security,state-machines', '--out', b, '--diff-base', SHA]).stdout, 'file');
|
||||
const cases = [
|
||||
{ reader: 'specialist-batched', dir: b, brief: batched, output: refusal, args: ['--role', 'specialist-batched'] },
|
||||
...['', 'I could not access the diff: git diff failed with "bad object", so there is nothing here to review.\n'].map(output => {
|
||||
const p = pass();
|
||||
return { reader: 'specialist/testing', dir: p.out, brief: p.file, output, args: ['--role', 'specialist', '--name', 'testing'] };
|
||||
}),
|
||||
];
|
||||
for (const c of cases) {
|
||||
const ledger = path.join(c.dir, 'review-ledger.jsonl');
|
||||
const merged = path.join(c.dir, 'merged.jsonl');
|
||||
const out = path.join(c.dir, 'reply.out');
|
||||
fs.writeFileSync(out, c.output);
|
||||
expect(run(['sent', '--ledger', ledger, ...c.args, '--sent', c.brief]).status).toBe(0);
|
||||
const col = run(['collect', '--ledger', ledger, ...c.args, '--output', out, '--merged', merged]);
|
||||
expect([col.status, col.stdout.startsWith(`COLLECT_NOT_REVIEWED: reader=${c.reader} findings=0 sha256=${promptHash(c.output)} `)]).toEqual([1, true]);
|
||||
const event = JSON.parse(fs.readFileSync(ledger, 'utf8').trim().split('\n').at(-1)!);
|
||||
expect([event.event, event.ids, event.review]).toEqual(['output', [], false]);
|
||||
fs.writeFileSync(path.join(c.dir, 'judged.jsonl'), '');
|
||||
expect(run(['judge', '--ledger', ledger, '--merged', merged, '--judged', path.join(c.dir, 'judged.jsonl')]).status).toBe(0);
|
||||
const v = run(['verify', '--ledger', ledger]);
|
||||
expect([v.status, v.stdout.includes(`BRIEF ${c.reader} NOT_COLLECTED rendered=`), v.stdout.includes(`OUTPUT ${c.reader} NOT_REVIEWED`), v.stdout.includes(' not_reviewed\n'), v.stdout.includes('VERIFY: dirty')]).toEqual([1, true, true, true, true]);
|
||||
}
|
||||
});
|
||||
|
||||
// Value: protects=a clean reply carrying its section's marker and a reply with findings still collect with exit 0 and verify clean under
|
||||
// the non-review rule; fails_when=the rule makes a real NO FINDINGS specialist or a findings reply dirty or exit 1;
|
||||
// why_new=the non-review rule above could over-reach; seam=collect and verify; no_claim=other roles (the matrix below covers them)
|
||||
test('a NO FINDINGS reply and a findings reply still collect with exit 0 and verify clean', () => {
|
||||
for (const [output, findings] of [['NO FINDINGS\n', 0], ['{"severity":"CRITICAL","path":"a.ts","line":3,"category":"logic","summary":"off by one","specialist":"testing"}\n', 1]] as const) {
|
||||
const p = pass();
|
||||
run(['sent', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--sent', p.file]);
|
||||
const col = run(['collect', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--output', p.w('reply.out', output), '--merged', p.merged]);
|
||||
expect([col.status, col.stdout.startsWith(`COLLECT: reader=specialist/testing findings=${findings} `)]).toEqual([0, true]);
|
||||
expect(JSON.parse(fs.readFileSync(p.ledger, 'utf8').trim().split('\n').at(-1)!).review).toBe(true);
|
||||
const judged = p.w('judged.jsonl', findings ? JSON.stringify({ id: 'specialist/testing#1', severity: 'CRITICAL' }) + '\n' : '');
|
||||
expect(run(['judge', '--ledger', p.ledger, '--merged', p.merged, '--judged', judged]).status).toBe(0);
|
||||
const v = run(['verify', '--ledger', p.ledger]);
|
||||
expect([v.status, v.stdout.includes(`OUTPUT specialist/testing MATCH sha=${promptHash(output).slice(0, 12)} findings=${findings}`), v.stdout.includes('VERIFY: clean briefs=1 outputs=1')]).toEqual([0, true, true]);
|
||||
}
|
||||
});
|
||||
|
||||
// Value: protects=each role's zero-finding reply counts as a review only with the marker its own section prescribes (NO FINDINGS for
|
||||
// specialists, the batched specialist, Red Team, fix-check and targeted; the Recommendation: line for the outside voice; either for the
|
||||
// native adversarial voice); fails_when=a marker named inside a sentence, another role's marker or an empty reply reads as clean, or a
|
||||
// prescribed marker reads as a non-review; why_new=no marker check existed; seam=collect; no_claim=that the marker's reason is true
|
||||
test('each role takes only the clean marker its section prescribes', () => {
|
||||
const d = tmp();
|
||||
const outsideClean = 'No actionable findings in the supplied diff. Static review only.\n\nRecommendation: approve because the diff introduces no demonstrated bypass.\n';
|
||||
const cases: Array<[string, string, boolean]> = [
|
||||
['specialist/testing', 'NO FINDINGS\n', true], ['specialist-batched', '`NO FINDINGS`\n', true], ['targeted', 'NO FINDINGS\n', true], ['fix-check', 'NO FINDINGS\n', true],
|
||||
['red-team', 'Fixtures were reviewed in summary mode only.\n\nNO FINDINGS\n', true],
|
||||
['adversarial-native', 'Fixtures were reviewed in summary mode.\n\nRecommendation: Ship as-is because no finding survived tracing\n', true], ['adversarial-native', 'NO FINDINGS\n', true],
|
||||
['outside-voice', outsideClean, true], ['outside-voice', 'NO FINDINGS\n', false],
|
||||
['specialist/testing', 'Recommendation: ship because it is clean\n', false], ['specialist/testing', "I'm not returning NO FINDINGS, because I never saw the diff.\n", false],
|
||||
['red-team', '', false], ['fix-check', 'The fixes look fine to me.\n', false], ['adversarial-native', 'I could not read the diff.\n', false],
|
||||
];
|
||||
for (const [i, [reader, output, review]] of cases.entries()) {
|
||||
const ledger = path.join(d, `l${i}.jsonl`);
|
||||
const r = collect(ledger, reader, output, path.join(d, `m${i}.jsonl`));
|
||||
expect([reader, output, r.lines.length, r.review]).toEqual([reader, output, 0, review]);
|
||||
expect(JSON.parse(fs.readFileSync(ledger, 'utf8').trim()).review).toBe(review);
|
||||
}
|
||||
});
|
||||
|
||||
// Value: protects=a paragraph finding's unlabeled detail bullets and the Severity/Blocks line after them stay one finding (GSTA-196
|
||||
// native voice: 4 findings collected as 8, the closing Severity lines opening phantoms judge had to fold); a list item's sibling and
|
||||
// a labeled bullet still start their own findings; fails_when=the sample collects more or fewer than 4, or a labeled bullet merges
|
||||
// away; why_new=every margin list item closed the open finding; seam=parseFindings; no_claim=indented detail lists (already attached)
|
||||
test('unlabeled detail bullets and the Severity line after them stay on the open paragraph finding', () => {
|
||||
const native = fs.readFileSync(path.join(ROOT, 'test/fixtures/review-ledger/gsta-196-native.txt'), 'utf8');
|
||||
expect(parseFindings('adversarial-native', native).map(f => [f.label, f.raw.slice(0, 12), f.raw.endsWith('| Severity: ' + (f.label === 'High' ? 'High' : 'Low') + '. Blocks: no.')]))
|
||||
.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']);
|
||||
expect(parseFindings('outside-voice', '**Race in a.ts:3 (Medium).**\n\n- the writer holds no lock\n\nSeverity: Medium. Blocks: yes').map(f => [f.label, f.raw]))
|
||||
.toEqual([['CRITICAL', '**Race in a.ts:3 (Medium).** | - the writer holds no lock | Severity: Medium. Blocks: yes']]);
|
||||
});
|
||||
|
||||
// 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]);
|
||||
const partial = path.join(o, 'partial.out');
|
||||
fs.writeFileSync(partial, '1. **CRITICAL**: token leak in a.ts:3\nCodex then timed out.\n');
|
||||
const kept = run(['collect', '--ledger', ledger, '--role', 'outside-voice', '--output', partial, '--merged', merged, '--unavailable', 'timeout']);
|
||||
expect([kept.status, kept.stderr.includes('holds findings; collect it without --unavailable')]).toEqual([2, true]);
|
||||
expect(run(['collect', '--ledger', ledger, '--role', 'outside-voice', '--output', fail, '--merged', merged, '--unavailable', ' ']).status).toBe(2);
|
||||
const n = pass();
|
||||
const takeN = (f: string) => run(['collect', '--ledger', n.ledger, '--role', 'specialist', '--name', 'testing', '--output', f, '--merged', n.merged]);
|
||||
run(['sent', '--ledger', n.ledger, '--role', 'specialist', '--name', 'testing', '--sent', n.file]);
|
||||
takeN(n.w('refusal.out', 'I could not access the diff.\n'));
|
||||
takeN(n.w('clean.out', 'NO FINDINGS\n'));
|
||||
fs.writeFileSync(path.join(n.out, 'judged.jsonl'), '');
|
||||
run(['judge', '--ledger', n.ledger, '--merged', n.merged, '--judged', path.join(n.out, 'judged.jsonl')]);
|
||||
const unsent = run(['verify', '--ledger', n.ledger]);
|
||||
expect([unsent.status, unsent.stdout.includes(' NOT_REVIEWED '), unsent.stdout.includes('VERIFY: dirty')]).toEqual([1, true, true]);
|
||||
});
|
||||
|
||||
test('verify on a ledger that kept no prompts says so and exits 3', () => {
|
||||
const d = tmp();
|
||||
const ledger = path.join(d, 'ledger.jsonl');
|
||||
|
||||
@@ -128,6 +128,7 @@ function repo(): { dir: string; env: NodeJS.ProcessEnv } {
|
||||
|
||||
interface Sub { title: string; prompt: string; status: string; polls: number; model?: unknown; summary?: string; late?: boolean }
|
||||
/** A fake Capy host: the shell is real (the repository's bins), subagents are scripted. */
|
||||
const cleanReply = (title: string) => (title.endsWith('outside-voice') ? 'Recommendation: ship as-is because no finding survived' : 'NO FINDINGS');
|
||||
function host(dir: string, env: NodeJS.ProcessEnv, o: { output?: (title: string) => string; fail?: (title: string) => boolean; reject?: (title: string) => boolean; createThrows?: (title: string) => 'before' | 'after' | undefined; pollsToFinish?: number; lateSummary?: boolean } = {}) {
|
||||
let clock = 0;
|
||||
const subs = new Map<string, Sub>();
|
||||
@@ -135,12 +136,12 @@ function host(dir: string, env: NodeJS.ProcessEnv, o: { output?: (title: string)
|
||||
const starts: number[][] = [];
|
||||
const advance = () => {
|
||||
for (const s of subs.values()) {
|
||||
if (s.late && !s.summary) { s.summary = o.output?.(s.title) ?? 'NO FINDINGS'; continue; }
|
||||
if (s.late && !s.summary) { s.summary = o.output?.(s.title) ?? cleanReply(s.title); continue; }
|
||||
if (s.status !== 'working') continue;
|
||||
if (++s.polls < (o.pollsToFinish ?? 1)) continue;
|
||||
if (o.fail?.(s.title)) s.status = 'failed';
|
||||
else if (o.lateSummary && !s.late) { s.status = 'done'; s.late = true; }
|
||||
else { s.status = 'done'; s.summary = o.output?.(s.title) ?? 'NO FINDINGS'; }
|
||||
else { s.status = 'done'; s.summary = o.output?.(s.title) ?? cleanReply(s.title); }
|
||||
}
|
||||
};
|
||||
const tools = {
|
||||
|
||||
Reference in new issue
Block a user