mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-03 07:18:03 +01:00
fix: GST-987 review fixes: history/floor reasons and FIXABLE-only fields keep v2; manifest must name this ledger
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
48e9f2c5c0
commit
9d910f9625
4 files changed
+23
-13
No files matched your search
@@ -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));
|
||||
|
||||
@@ -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=<n>` 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
|
||||
|
||||
@@ -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<string | undefined>) => labels.reduce<string | undefined>((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' && /(?<![A-Za-z0-9])FIXABLE(?![A-Za-z0-9])/.test(t));
|
||||
const leadOf = (t: string) => { 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=<n>` 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));
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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 });
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user