diff --git a/changelog.d/gst-1031-severity-collect.md b/changelog.d/gst-1031-severity-collect.md index 6b110bca3..9078501ef 100644 --- a/changelog.d/gst-1031-severity-collect.md +++ b/changelog.d/gst-1031-severity-collect.md @@ -1 +1 @@ -- **Review collector and blocker count follow what the reader said (GST-1031).** A native adversarial finding that states `Blocks: no` and one Severity now collects once, at that severity, on every review class; a numbered finding heading keeps its body (located or numbered bullets, a bold subheading, a located paragraph) so its Severity line no longer splits off as a second CRITICAL record. A closing summary such as "so nothing is CRITICAL" is marked `summary_candidate`, and lead judgment may lower it with a reason that `judge` records as `dismissed`. A CRITICAL that says `Blocks: no` no longer counts as a blocker in `REVIEW_LOOP` or Step 9.4's classify. Decision 3.5's fix-check pass takes a `review-fix-check` start token even when given `logStart: ["review"]`, so it is never counted as a pass. `gstack-ship-release finish` routes a `scripts/harness-version.json` edit that only records non-behavioral pins at the same version as release metadata. +- **Review collector and blocker count follow what the reader said (GST-1031).** A native adversarial finding that states `Blocks: no` and one Severity now collects once, at that severity, on every review class; a numbered finding heading keeps its body (located or numbered bullets, a bold subheading, a located paragraph) so its Severity line no longer splits off as a second CRITICAL record. A closing summary such as "so nothing is CRITICAL" is marked `summary_candidate`, and lead judgment may lower it with a reason that `judge` records as `dismissed`. A CRITICAL whose reader writes the field `Blocks: no` no longer counts as a blocker in `REVIEW_LOOP`'s ledger count (classify does the same for a finding that carries `stated`; Step 9.4 does not copy it yet). Decision 3.5's fix-check pass takes a `review-fix-check` start token even when given `logStart: ["review"]`, so it is never counted as a pass. `gstack-ship-release finish` routes a `scripts/harness-version.json` edit that only records non-behavioral pins at the same version as release metadata. diff --git a/docs/review-policy.md b/docs/review-policy.md index 3c0f8bec8..96725709d 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -596,9 +596,10 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends: records the Severity and Blocks its own text states as `stated`, beside the collected label and never in its place; a classified finding that carries `stated` shows it in its deferred-review issue next to the label ("CRITICAL (the reviewer wrote Severity: Low, Blocks: no)"). /ship's Step 9.4 does not copy `stated` into its findings file yet: that - instruction lives in a reviewer brief that changes only after a no-loss replay. A CRITICAL whose own text states - `Blocks: no` (and no `Blocks: yes`) is not counted as a blocker by the ledger count, the review-log count or classify - (GST-1031); lead judgment that raises such a line above its label counts it again. + instruction lives in a reviewer brief that changes only after a no-loss replay. A CRITICAL whose own text writes the + field `Blocks: no` (colon required, and no `Blocks: yes`) is not counted as a blocker by the ledger count, nor by the + review-log count or classify when the finding carries `stated` (GST-1031); lead judgment that raises such a line above + its label counts it again. 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:` diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index cd576587d..9667de4a2 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -84,22 +84,26 @@ const SEVERITY_FIELD = new RegExp(`(?:^(?:[-*+>]\\s+|\\d+[.)]\\s+)?\\**\\s*Sever const FIELD_LABELS = new RegExp(ANY_LABEL.source, 'gi'); const LEAD_LABEL = new RegExp(`^(?:[^\\p{L}\\p{N}\\s]\\s*|\\d+[.)]\\s*)*(?:\\w[\\w./-]*:\\d+(?:[-–:,]\\s*\\d+)*[^\\p{L}\\p{N}\\s]*\\s+(?:[^\\p{L}\\p{N}\\s]\\s*)*)?(${LABELS})(?=[*_\`\\])]|\\s*[:|—–]|\\s+-\\s|\\s*$|(?<=[A-Z]{3})[\\s,(.])`, 'u'); const STATED_BLOCKS = /(? { const said = [...raw.matchAll(BLOCKS_FIELD)].map(m => m[1]!.toLowerCase()); return said.length > 0 && !said.includes('yes'); }; /** - * A summary candidate (GST-1031): a prose finding that names a severity in passing ("so nothing is CRITICAL", "the test covers the - * CRITICAL fix") with no lead label, no Severity or Blocks field, no FIXABLE or INVESTIGATE tag, and no file:line or exception clause in - * any sentence naming a label. It still collects at its label (a blocker named in another sentence stays on record, GST-435); lead + * A summary candidate (GST-1031): a prose finding whose every severity word is denied ("so nothing is CRITICAL", "No CRITICAL issues") + * or names a fix ("the test covers the CRITICAL fix"), with no lead label, no Severity or Blocks field, no FIXABLE or INVESTIGATE tag, + * and no file:line or exception clause in any sentence naming a label. A defect called CRITICAL ("this is CRITICAL") is none. It still collects at its label (a blocker named in another sentence stays on record, GST-435); lead * judgment may lower it with a reason, which judge records as dismissed. "No CRITICAL issues except the token leak in log.ts:3" is no candidate. */ const SENTENCE = /(?<=[.!?])\s+/; const EXCEPTION = /(? ANY_LABEL.test(t) && !t.startsWith('{') && !t.startsWith('**') && !LIST_ITEM.test(t) && !HEADING.test(t) && !LEAD_LABEL.test(t) +const DENIED_OR_FIXED = new RegExp(`(?:(? ANY_LABEL.test(t) && !ANY_LABEL.test(t.replace(DENIED_OR_FIXED, ' ')) && !t.startsWith('{') && !t.startsWith('**') && !LIST_ITEM.test(t) && !HEADING.test(t) && !LEAD_LABEL.test(t) && !t.match(SEVERITY_FIELD) && !t.match(STATED_BLOCKS) && [...t.matchAll(ALL_LABELS)].every(m => !!rank(m[1])) && t.split(SENTENCE).filter(s => ANY_LABEL.test(s)).every(s => !PATH_LOCUS.test(s) && !EXCEPTION.test(s)); export function stated(raw: string): Stated | undefined { if (raw.startsWith('{')) return undefined; const severity = [...raw.matchAll(STATED_SEVERITY)].at(-1)?.[1]; - const said = [...raw.matchAll(STATED_BLOCKS)].map(m => m[1]!.toLowerCase()); + const said = [...raw.matchAll(BLOCKS_FIELD)].map(m => m[1]!.toLowerCase()); const blocks = said.includes('yes') ? 'yes' : said.at(-1); return severity || blocks ? { ...(severity ? { severity } : {}), ...(blocks ? { blocks } : {}) } : undefined; } @@ -209,7 +213,7 @@ export function parseFindings(reader: string, output: string, fixableRaises = tr const relaxed = fixableRaises && role === 'adversarial-native' ? parseFindings(reader, output, false) : undefined; const statesSeverity = (raw: string) => { const ls = [...raw.matchAll(SEVERITY_FIELD)].flatMap(m => (m[1] ?? m[2]!).match(FIELD_LABELS)!); return ls.length === 1 && !!rank(ls[0]); }; return out.map((f, i) => { - const own = relaxed?.length === out.length && relaxed[i]!.raw === f.raw && BLOCKS_NO.test(f.raw) && statesSeverity(f.raw) ? relaxed[i]!.label : undefined; + const own = relaxed?.length === out.length && relaxed[i]!.raw === f.raw && statesNoBlocks(f.raw) && statesSeverity(f.raw) ? relaxed[i]!.label : undefined; return { label: own ?? (unblocked(f) ? f.label : 'CRITICAL'), raw: f.raw }; }); } @@ -363,7 +367,7 @@ export function ledgerBlockers(ledger: string): LedgerBlocker[] { for (const id of o.ids as string[]) { const line = lines.find(l => l.id === id) ?? fail(`collected id ${id} is missing from ${String(o.merged)}`); ranks.set(id, rank(line.label) ?? 0); - if (stated(line.raw)?.blocks === 'no') unblocking.set(id, rank(line.label) ?? 0); + if (statesNoBlocks(line.raw)) unblocking.set(id, rank(line.label) ?? 0); raws.set(id, line.raw); loci.set(id, lineLocus(line.raw)); } diff --git a/test/review-gst1031.test.ts b/test/review-gst1031.test.ts index 705a2528e..5b53fab23 100644 --- a/test/review-gst1031.test.ts +++ b/test/review-gst1031.test.ts @@ -72,7 +72,9 @@ describe('a closing summary is a candidate lead judgment may dismiss on record ( }); test('negative controls: a locus, an exception clause, a lead label, a field or a JSON finding is no candidate and keeps its label', () => { for (const line of ['No CRITICAL issues except the token leak in log.ts:3.', 'Nothing else is CRITICAL, but the retry loop never ends.', 'The token leak in log.ts:3 is CRITICAL.', - 'The leak in bin/gstack-log:3 is CRITICAL.', 'CRITICAL: the retry loop never ends.', 'The retry loop never ends. Severity: High', 'Not checked. Blocks: no. CRITICAL', '{"severity":"CRITICAL","summary":"x"}']) { + 'The leak in bin/gstack-log:3 is CRITICAL.', 'CRITICAL: the retry loop never ends.', 'The retry loop never ends. Severity: High', 'Not checked. Blocks: no. CRITICAL', '{"severity":"CRITICAL","summary":"x"}', + 'The retry loop in the upload handler never ends, which is CRITICAL.', 'Path traversal in the download route lets any user read arbitrary files; this is CRITICAL.', + 'The session token is written to the debug log on every request, a CRITICAL leak.', 'Nothing guards the write path the failure of which is CRITICAL']) { expect([line, summaryCandidate(line)]).toEqual([line, false]); expect(parseFindings('fix-check', line).length).toBe(1); } @@ -106,6 +108,15 @@ describe('the blocker count keys on Blocks (GST-1031 case 3)', () => { const promote = ['outside-voice#1', 'outside-voice#2', 'outside-voice#3'].map(id => JSON.stringify({ id, severity: 'CRITICAL' })).join('\n') + '\n' + JSON.stringify({ id: 'outside-voice#4', severity: 'CRITICAL', reason: 'it loses rows' }) + '\n'; expect(run(promote)).toEqual(['outside-voice#2', 'outside-voice#3', 'outside-voice#4']); }); + test('a verb "blocks no" is no Blocks field: the located CRITICAL still counts, on the ledger and in a native finding', () => { + for (const out of ['CRITICAL: The lock in queue.ts:40 blocks no other writer, so two passes lose findings.\n', '1. **CRITICAL** — cache.ts:12 evicts on read; it blocks no request with a stale token.\n', '- CRITICAL src/auth.ts:9 token check is skipped; it blocks no attacker.\n']) { + const d = fs.mkdtempSync(path.join(os.tmpdir(), 'gst1031v-')); + const c = collect(path.join(d, 'l.jsonl'), 'outside-voice', out, path.join(d, 'm.jsonl')); + expect([out, c.lines[0]!.stated?.blocks, ledgerBlockers(path.join(d, 'l.jsonl')).map(b => b.id)]).toEqual([out, undefined, ['outside-voice#1']]); + fs.rmSync(d, { recursive: true, force: true }); + } + expect(labels('adversarial-native', '**1. Race in a.ts:3. FIXABLE.**\nIt blocks no attacker.\nSeverity: Low.', true)).toEqual(['CRITICAL']); + }); test('classify and the row count key on Blocks, and a raw holding both Blocks: yes and no blocks', () => { expect(isBlocking({ severity: 'CRITICAL', stated: { severity: 'Critical', blocks: 'no' } })).toBe(false); expect(isBlocking({ severity: 'CRITICAL', stated: { severity: 'Critical', blocks: 'yes' } })).toBe(true);