fix(review): summary candidates need a denied or fixed label; only a Blocks: field lowers the count (GST-1031 review pass 1)

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

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.

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:`

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 = /(?<![A-Za-z0-9])Blocks\**\s*:?\s*\**\s*(yes|no)(?![A-Za-z0-9])/gi;
+/** A reader's `Blocks` field as it can lower a count (GST-1031): its colon required, so prose ("blocks no other writer") never does. */
+const BLOCKS_FIELD = /(?<![A-Za-z0-9])Blocks\**\s*:\s*\**\s*(yes|no)(?![A-Za-z0-9-])/gi;
+export const statesNoBlocks = (raw: string) => { 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 = /(?<![A-Za-z])(?:except|but|other than|apart from|aside from|besides|unless|save for)(?![A-Za-z])/i;
 const PATH_LOCUS = /(?:[\w.-]+\/)+[\w.-]+:\d+|[\w.-]+\.\w+:\d+/;
-export const summaryCandidate = (t: string) => 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(`(?:(?<![A-Za-z])(?:nothing|none|no|neither|zero)(?:\\s+(?:of\\s+)?(?:these|them|this|it|other|else|further|new|remaining|the\\s+\\w+|\\w+\\s+findings?|findings?|issues?))?\\s+(?:else\\s+)?(?:(?:is|are|was|were|would\\s+be|at)\\s+)?|(?<![A-Za-z])(?:cover(?:s|ed)?|fix(?:es|ed)?|address(?:es|ed)?|resolve[sd]?|close[sd]?)\\s+(?:the|that|this)\\s+)[*\`]*(?:${LABELS})(?![A-Za-z0-9])`, 'gi');
+export const summaryCandidate = (t: string) => 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));
     }

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);