diff --git a/bin/gstack-review-dispatch b/bin/gstack-review-dispatch index e91ae05cc..9771e6b75 100755 --- a/bin/gstack-review-dispatch +++ b/bin/gstack-review-dispatch @@ -330,7 +330,8 @@ function main(): number { return EXIT.ok; } const mf = flag('--manifest'); - const plan = mf ? readManifest(mf) : undefined; + let plan: ReturnType | undefined; + try { plan = mf ? readManifest(mf) : undefined; } catch (e) { process.stderr.write(`gstack-review-dispatch collect: ${(e as Error).message}; native FIXABLE collects as CRITICAL\n`); } const r = collect(ledger, reader, output, merged, path.resolve(outFile!), unavailable, tree, fixableRaises(plan && path.resolve(plan.ledger) === path.resolve(ledger) ? plan.plan_lines : undefined)); if (tree && !tree.ok) { if (mf) { const m = readManifest(mf); markReader(m, reader, 'failed', { output: outFile, error: `reviewed another tree: ${tree.reported}` }); writeManifest(mf, m); } diff --git a/changelog.d/gst-987-fixable-by-class.md b/changelog.d/gst-987-fixable-by-class.md index c17998555..ab6c6880f 100644 --- a/changelog.d/gst-987-fixable-by-class.md +++ b/changelog.d/gst-987-fixable-by-class.md @@ -2,4 +2,4 @@ ### Fixed -- On a review whose class is not gate and that carries no risk reason (security, secrets, auth, money, user data), a native adversarial finding marked FIXABLE with its own `Severity: Medium` or `Severity: Low` now collects at that severity instead of CRITICAL. Gate and risk reviews, depth errors and any collect without a pass manifest keep collecting FIXABLE as CRITICAL, and a FIXABLE finding with no Severity field still collects as CRITICAL. (GST-987) +- On a code or docs review with no risk reason (security, secrets, auth, money, user data), no depth error and no history or floor escalation, a native adversarial finding marked FIXABLE that states its own `Severity: Medium` or `Severity: Low` and `Blocks: no` now collects at that severity instead of CRITICAL. Gate and risk reviews, and any collect without this pass's manifest, keep collecting FIXABLE as CRITICAL, and so does a FIXABLE finding without `Blocks: no` or without a Severity field. (GST-987) diff --git a/docs/review-policy.md b/docs/review-policy.md index 6a55e9b3e..e81577eb4 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -533,8 +533,8 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends: adversarial voice's FIXABLE collects as CRITICAL (normalizer v2, GSTA-167) unless collect `--manifest` names this ledger's pass and its plan is the code or docs class with only `runtime=` depth reasons (no `risk:`, `error:`, `history:` or `floor:` reason). On such a plan a native finding's own `Severity` field decides and FIXABLE beside it - raises nothing; a native finding with no `Severity` field, or one whose field names only FIXABLE or INVESTIGATE, still - collects as CRITICAL (GST-987). An unnumbered, unlabeled bullet with no file:line + raises nothing once it also states `Blocks: no`; a native FIXABLE finding without `Blocks: no`, with no `Severity` field, + or whose field names only FIXABLE or INVESTIGATE still collects as CRITICAL (GST-987). 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 negated severity word still collects at its label ("found nothing CRITICAL", "No P0/P1 issues"), so a reader's diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index 8200117d5..7c1bc19bd 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -58,6 +58,7 @@ const LIST_ITEM = /^(?:[-*+]|\d+[.)])\s+\S/; const stronger = (a: string, b: string) => ((rank(a) ?? 0) >= (rank(b) ?? 0) ? a : b); /** A prose voice's `Blocks: yes` (GSTA-167 normalizer v2): the finding is CRITICAL whatever its label; `Blocks: no` never lowers one. */ const BLOCKS_YES = /(? { type Finding = { label: string; raw: string; fields: Set; list: boolean; lead?: string }; @@ -152,7 +153,7 @@ export function parseFindings(reader: string, output: string, fixableRaises = tr continue; } const heading = LABEL_HEADING.exec(t); - if (heading) { section = named(heading[1]!); current = null; head = ''; continue; } + if (heading) { section = fixableRaises ? named(heading[1]!) : heading[1]!; current = null; head = ''; continue; } if (!indented && HEADING.test(t)) { section = null; current = null; } if (t.startsWith('{')) { try { @@ -178,7 +179,8 @@ export function parseFindings(reader: string, output: string, fixableRaises = tr } if (!fresh && !current && (!LIST_ITEM.test(head) || locus)) head = t; } - return out.map(({ label, raw }) => ({ label, raw })); + const unblocked = (label: string, raw: string) => fixableRaises || role !== 'adversarial-native' || !(label.toUpperCase() === 'FIXABLE' || /(? ({ label: unblocked(label, raw) ? label : 'CRITICAL', raw })); } /** diff --git a/test/review-fixable-class.test.ts b/test/review-fixable-class.test.ts index a12d2249e..6140e630f 100644 --- a/test/review-fixable-class.test.ts +++ b/test/review-fixable-class.test.ts @@ -31,6 +31,9 @@ describe('native FIXABLE by review class (GST-987)', () => { expect(parseFindings('adversarial-native', 'FIXABLE: race in a.ts:3\nSeverity: Medium\nBlocks: no', false).map(f => f.label)).toEqual(['Medium']); expect(parseFindings('adversarial-native', 'FIXABLE: race in a.ts:3\nSeverity: Medium\nBlocks: yes', false).map(f => f.label)).toEqual(['CRITICAL']); expect(parseFindings('adversarial-native', '1. Race in a.ts:3. Severity: FIXABLE', false).map(f => f.label)).toEqual(['CRITICAL']); + expect(parseFindings('adversarial-native', '1. FIXABLE: race in store.ts:40. Severity: High', false).map(f => f.label)).toEqual(['CRITICAL']); + expect(parseFindings('adversarial-native', '1. FIXABLE: race in store.ts:40\nSeverity: High\nBlocks: no', false).map(f => f.label)).toEqual(['High']); + expect(parseFindings('adversarial-native', '## FIXABLE\n1. Leak in a.ts:3. Severity: Low. Blocks: no\n2. Race in b.ts:4', false).map(f => f.label)).toEqual(['Low', 'CRITICAL']); }); test('collect reads the class from --manifest and defaults to the gate class without one', () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gst987-')); diff --git a/test/review-ledger-invariant.test.ts b/test/review-ledger-invariant.test.ts index 467e3805c..18b2f6744 100644 --- a/test/review-ledger-invariant.test.ts +++ b/test/review-ledger-invariant.test.ts @@ -20,8 +20,8 @@ describe('parseFindings is never weaker than main (GST-976 invariant)', () => { // its weaker label (b3ddd6c) or a field whose clause runs on stops counting and a body P0 raises the finding (f3810d4); // why_new=three GST-976 review rounds each found a shape the row tables missed; seam=parseFindings; no_claim=that main's label was // right (it is the floor), or coverage of stored private reader outputs (checked outside the repo). GST-987: the gate class (and every - // risk class) keeps this rule; outside it the native voice's FIXABLE no longer counts as a raise, so a fielded finding collects at its - // field unless a lead, a label heading or Blocks: yes raises it + // risk class) keeps this rule; outside it the native voice's FIXABLE beside Blocks: no no longer counts as a raise, so such a fielded + // finding collects at its field unless a lead, a label heading or Blocks: yes raises it test.each([['gate', true], ['code', false]] as const)('every pinned input, reader fixture and generated field shape (%s class)', (_cls, raises) => { const { entries, shapes } = JSON.parse(fs.readFileSync(path.join(ROOT, 'test/fixtures/review-ledger/main-labels.json'), 'utf8')); const { bodies, fields: tails, joins, readers, generated } = shapes; @@ -37,7 +37,7 @@ describe('parseFindings is never weaker than main (GST-976 invariant)', () => { findings++; const fields = [...f.raw.matchAll(FIELD)].map(m => (m[1] ?? m[2])!); const single = fields.length === 1 && fields[0]!.match(LABEL)!.length === 1 ? fields[0]!.match(LABEL)![0] : undefined; - const raised = /Blocks:?\**:?\s*\**\s*yes/i.test(f.raw) || (raises && e.reader.startsWith('adversarial-native') && /FIXABLE/.test(f.raw)) || (raises ? LEAD : LEAD_NO_FIXABLE).test(f.raw) || HEADING.test(output); + const raised = /Blocks:?\**:?\s*\**\s*yes/i.test(f.raw) || (e.reader.startsWith('adversarial-native') && /FIXABLE/.test(f.raw) && (raises || !/Blocks:?\**:?\s*\**\s*no\b/i.test(f.raw))) || (raises ? LEAD : LEAD_NO_FIXABLE).test(f.raw) || HEADING.test(output); const ok = single === undefined ? (rank(f.label) ?? 0) >= (rank(e.main[i]) ?? 0) : (rank(f.label) ?? 0) >= (rank(single) ?? 0) && (raised || (rank(f.label) ?? 0) === (rank(single) ?? 0)); if (!ok) bad.push(`${e.reader} main=${e.main[i]} got=${f.label} field=${single ?? '-'} ${JSON.stringify(f.raw.slice(0, 100))}`);