diff --git a/bin/gstack-review-dispatch b/bin/gstack-review-dispatch index 4ac279ea6..e91ae05cc 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 r = collect(ledger, reader, output, merged, path.resolve(outFile!), unavailable, tree, fixableRaises(mf ? readManifest(mf).plan_lines : undefined)); + const plan = mf ? readManifest(mf) : undefined; + 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); } process.stdout.write(wrongTree(reader, tree)); diff --git a/docs/review-policy.md b/docs/review-policy.md index ce92a4775..6a55e9b3e 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -530,10 +530,11 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends: second line for the same field starts another: `Severity` sets its label (a label leading the finding stays its floor), `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) on the gate class, on any `risk:` or `error:` - depth reason, and whenever collect has no pass manifest to read the plan from. On any other class (collect - `--manifest` reads `REVIEW_CLASS:` and `REVIEW_DEPTH: … reasons=`), a native finding's own `Severity` field decides and - FIXABLE beside it raises nothing; a native finding with no `Severity` field still collects as CRITICAL (GST-987). An unnumbered, unlabeled bullet with no file:line + 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 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 32a1466fa..8200117d5 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -117,7 +117,7 @@ export function parseFindings(reader: string, output: string, fixableRaises = tr const named = (l: string) => (role === 'adversarial-native' && l.toUpperCase() === 'FIXABLE' ? 'CRITICAL' : l); const strongestOf = (labels: Array) => labels.reduce((a, b) => (a === undefined ? b : b === undefined ? a : stronger(a, b)), undefined); const strongest = (t: string) => strongestOf([...t.matchAll(ALL_LABELS)].map(m => named(m[1]!))); - const fieldOf = (t: string) => { const ls = [...t.matchAll(SEVERITY_FIELD)].flatMap(m => (m[1] ?? m[2]!).match(FIELD_LABELS)!); return ls.length === 1 ? ls[0] : undefined; }; + const fieldOf = (t: string) => { const ls = [...t.matchAll(SEVERITY_FIELD)].flatMap(m => (m[1] ?? m[2]!).match(FIELD_LABELS)!); return ls.length === 1 && (fixableRaises || role !== 'adversarial-native' || rank(ls[0]!)) ? ls[0] : undefined; }; const critical = (t: string) => BLOCKS_YES.test(t) || (fixableRaises && role === 'adversarial-native' && /(? { const l = LEAD_LABEL.exec(t)?.[1]; return l === undefined || !fixableRaises ? l : named(l); }; const labelOf = (t: string) => { @@ -183,14 +183,14 @@ export function parseFindings(reader: string, output: string, fixableRaises = tr /** * Whether the native voice's FIXABLE raises a finding with its own Severity field to CRITICAL (GST-987): true on the gate class, on any - * risk reason (security, secret, auth, money, user data), on a depth error, and whenever the plan's REVIEW_CLASS or REVIEW_DEPTH - * reasons cannot be read. + * risk reason (security, secret, auth, money, user data), a depth error, a history or floor escalation, and whenever the plan's + * REVIEW_CLASS or REVIEW_DEPTH reasons cannot be read: only a code or docs plan whose every reason is `runtime=` relaxes it. */ export function fixableRaises(planLines?: readonly string[]): boolean { const cls = planLines?.map(l => /^REVIEW_CLASS: (\S+)/.exec(l)?.[1]).find(Boolean); const reasons = planLines?.map(l => /^REVIEW_DEPTH: .*?\breasons=(\S+)/.exec(l)?.[1]).find(Boolean); if (!cls || !reasons) return true; - return cls === 'gate' || reasons.split(',').some(r => /^(?:risk|error):/.test(r)); + return !['code', 'docs'].includes(cls) || !reasons.split(',').every(r => /^runtime=\d+$/.test(r)); } /** diff --git a/test/review-fixable-class.test.ts b/test/review-fixable-class.test.ts index 1008af6b5..a12d2249e 100644 --- a/test/review-fixable-class.test.ts +++ b/test/review-fixable-class.test.ts @@ -21,7 +21,7 @@ describe('native FIXABLE by review class (GST-987)', () => { expect(parseFindings('adversarial-native', NATIVE).map(f => f.label)).toEqual(['CRITICAL', 'CRITICAL']); }); test('gate class, any risk reason, a depth error or an unreadable plan keeps normalizer v2', () => { - for (const lines of [plan('gate', 'gate-class,runtime=9'), plan('code', 'risk:money,runtime=9'), plan('code', 'risk:data,runtime=9'), plan('code', 'error:DIFF,runtime=9'), plan('code', ''), [CODE[0]!], [], undefined]) + for (const lines of [plan('gate', 'gate-class,runtime=9'), plan('code', 'risk:money,runtime=9'), plan('code', 'risk:data,runtime=9'), plan('code', 'error:DIFF,runtime=9'), plan('code', 'runtime=9,history:fixed-blocking'), plan('code', 'runtime=9,floor:full'), plan('code', ''), plan('unknown', 'runtime=9'), [CODE[0]!], [], undefined]) expect(fixableRaises(lines)).toBe(true); expect(fixableRaises(CODE)).toBe(false); expect(fixableRaises(plan('docs', 'runtime=0'))).toBe(false); @@ -30,22 +30,30 @@ describe('native FIXABLE by review class (GST-987)', () => { expect(parseFindings('adversarial-native', '1. Race in a.ts:3. FIXABLE', false).map(f => f.label)).toEqual(['CRITICAL']); 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']); }); test('collect reads the class from --manifest and defaults to the gate class without one', () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gst987-')); const out = path.join(dir, 'native.txt'); fs.writeFileSync(out, NATIVE + '\n'); const manifest = path.join(dir, 'pass-manifest.json'); - fs.writeFileSync(manifest, JSON.stringify({ version: 1, plan_lines: CODE, readers: [] })); + const ledger = path.join(dir, 'ledger.jsonl'); + fs.writeFileSync(manifest, JSON.stringify({ version: 1, ledger, plan_lines: CODE, readers: [] })); + const foreign = path.join(dir, 'foreign.json'); + fs.writeFileSync(foreign, JSON.stringify({ version: 1, ledger: path.join(dir, 'other.jsonl'), plan_lines: CODE, readers: [] })); + let n = 0; const labels = (extra: string[]) => { - const ledger = path.join(dir, `${extra.length}.jsonl`); - const merged = path.join(dir, `${extra.length}.merged.jsonl`); + fs.rmSync(ledger, { force: true }); + const merged = path.join(dir, `${n++}.merged.jsonl`); const r = spawnSync(path.join(ROOT, 'bin/gstack-review-dispatch'), ['collect', '--ledger', ledger, '--role', 'adversarial-native', '--output', out, '--merged', merged, ...extra], { cwd: ROOT, encoding: 'utf8', timeout: 60_000 }); expect(r.stderr).toBe(''); return fs.readFileSync(merged, 'utf8').trim().split('\n').map(l => JSON.parse(l).label); }; expect(labels(['--manifest', manifest])).toEqual(['Medium', 'Low']); + expect(fs.readFileSync(ledger, 'utf8')).toContain('"fixable_raises":false'); expect(labels([])).toEqual(['CRITICAL', 'CRITICAL']); + expect(fs.readFileSync(ledger, 'utf8')).not.toContain('fixable_raises'); + expect(labels(['--manifest', foreign])).toEqual(['CRITICAL', 'CRITICAL']); fs.rmSync(dir, { recursive: true, force: true }); }); });