From cfde08d90c0c6cc66a4549283ea60b4b61a93111 Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 14:16:47 +0000 Subject: [PATCH] fix: GST-987 fix-check: items under a FIXABLE heading need Blocks: no; Blocks: no-op is not Blocks: no; wrong-tree collect with an unreadable manifest Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-review-dispatch | 2 +- lib/review-ledger.ts | 9 +++++---- test/review-fixable-class.test.ts | 4 +++- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/bin/gstack-review-dispatch b/bin/gstack-review-dispatch index 9771e6b75..3bdc0d919 100755 --- a/bin/gstack-review-dispatch +++ b/bin/gstack-review-dispatch @@ -334,7 +334,7 @@ function main(): number { 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); } + if (mf && plan) { markReader(plan, reader, 'failed', { output: outFile, error: `reviewed another tree: ${tree.reported}` }); writeManifest(mf, plan); } process.stdout.write(wrongTree(reader, tree)); return EXIT.fail; } diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index 7c1bc19bd..0123bbf46 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -58,7 +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 }; + type Finding = { label: string; raw: string; fields: Set; list: boolean; lead?: string; fixable?: boolean }; const out: Finding[] = []; let section: string | null = null; let current: Finding | null = null; @@ -169,6 +169,7 @@ export function parseFindings(reader: string, output: string, fixableRaises = tr const found = label && inherited ? stronger(inherited, label) : label ?? inherited; if (found) { current = push(blocks ? 'CRITICAL' : found, t, [], strongestOf([leadOf(t), inherited, fieldOf(t)])); + current.fixable = inherited?.toUpperCase() === 'FIXABLE'; if (!indented) head = t; continue; } @@ -179,8 +180,8 @@ export function parseFindings(reader: string, output: string, fixableRaises = tr } if (!fresh && !current && (!LIST_ITEM.test(head) || locus)) head = t; } - const unblocked = (label: string, raw: string) => fixableRaises || role !== 'adversarial-native' || !(label.toUpperCase() === 'FIXABLE' || /(? ({ label: unblocked(label, raw) ? label : 'CRITICAL', raw })); + const unblocked = (f: Finding) => fixableRaises || role !== 'adversarial-native' || !(f.fixable || f.label.toUpperCase() === 'FIXABLE' || /(? ({ label: unblocked(f) ? f.label : 'CRITICAL', raw: f.raw })); } /** diff --git a/test/review-fixable-class.test.ts b/test/review-fixable-class.test.ts index 6140e630f..ce1075e00 100644 --- a/test/review-fixable-class.test.ts +++ b/test/review-fixable-class.test.ts @@ -33,7 +33,9 @@ describe('native FIXABLE by review class (GST-987)', () => { 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']); + expect(parseFindings('adversarial-native', '## FIXABLE\n1. Leak in a.ts:3. Severity: Low. Blocks: no\n2. Race in b.ts:4\n3. Drift in c.ts:5. Severity: Low', false).map(f => f.label)).toEqual(['Low', 'CRITICAL', 'CRITICAL']); + expect(parseFindings('adversarial-native', '**FIXABLE**\n- Leak in a.ts:3\n Severity: Medium', false).map(f => f.label)).toEqual(['CRITICAL']); + expect(parseFindings('adversarial-native', 'FIXABLE: race in a.ts:3. Severity: Low. Blocks: no-op path only', 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-'));