mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-07 17:28:23 +01:00
review collect: a 'Finding N' lead keeps its own Severity and Blocks lines (GST-1657) (#4089)
* review collect: a 'Finding N' lead keeps its own Severity and Blocks lines (GST-1657) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes A located line right under a 'Finding N' lead is another finding, not that lead's fields: the inline-field join now refuses a file:line, as the blank-line branch does, so judge() cannot lower the lead with a second finding's Blocks: no (red-team#1, GST-1657). Review-Finding: p1/red-team#1 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes A heading that names CRITICAL or P0 itself keeps its blocker: the no-gap inline-field join no longer attaches another paragraph's Blocks: no to it, so ledgerBlockers() still counts it, as on main (outside-voice#3, GST-1657). Review-Finding: v1/outside-voice#3 reader=outside-voice Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: GST-1657 release note names the located-line and CRITICAL-heading guards Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
b493d4e31a
commit
be173d52bf
7 files changed
+198
-6
No files matched your search
@@ -0,0 +1 @@
|
||||
- **A native finding led by `Finding 1 (FIXABLE)` keeps its own Severity and Blocks lines.** The review collector treated only `**1. …**` as a numbered finding heading, so when the native adversarial reader opened a finding with `**Finding 1 (FIXABLE): …**` and put numbered steps or more paragraphs before its `Severity: Medium` and `Blocks: no` lines, those lines split off onto a phantom second record and the finding itself collected CRITICAL from its FIXABLE tag. The judge then refused the downgrade, and gstack-evals #248's /ship blocked until a review-loop exception was granted by hand. A "Finding N" lead is now a numbered heading, so the fields land on their finding and it collects at the Severity the reader wrote (GST-1031 ruling A). A line right under a numbered heading that carries its Severity and Blocks fields, with no other label and no file:line, now joins the heading too, at the stronger label, so the ledger stops counting a `Blocks: no` finding as a blocker and lead judgment may lower it with a reason. A located line there stays a finding of its own, and a heading that names CRITICAL or P0 itself keeps its blocker. Checked against 586 captured reader outputs: only four outputs of this shape changed, and no CRITICAL a reader rated as blocking was lost. (GST-1657)
|
||||
@@ -863,9 +863,11 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends:
|
||||
finding that states `Blocks: no` and one `Severity` field collects at that field (GST-1031 ruling A); a native FIXABLE
|
||||
finding without `Blocks: no`, with no `Severity` field, or whose field names only FIXABLE or INVESTIGATE still collects
|
||||
as CRITICAL. On a code or docs plan with only `runtime=<n>` depth reasons (collect `--manifest` naming this ledger's
|
||||
pass; GST-987) FIXABLE also raises nothing through a label heading or a leading tag. A numbered finding heading keeps
|
||||
every unlabeled line under it (located or numbered bullets, a bold subheading, a located paragraph) until its
|
||||
`Severity` or `Blocks` line comes, so those fields never split off as a second record (GST-1031). An unnumbered, unlabeled bullet with no file:line
|
||||
pass; GST-987) FIXABLE also raises nothing through a label heading or a leading tag. A numbered finding heading (`**1. …**`,
|
||||
or a lead that names its number, `**Finding 1 (FIXABLE): …**`, GST-1657) keeps every unlabeled line under it (located or
|
||||
numbered bullets, a bold subheading, a located paragraph) until its `Severity` or `Blocks` line comes, so those fields
|
||||
never split off as a second record (GST-1031); the line right under the heading that carries those fields, no other
|
||||
label and no file:line joins it at the stronger label (not under a heading that names CRITICAL or P0 itself), the way an unlocated paragraph after a blank line already does. 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 reader's own no-defect closing sentence is no finding at the label it denies (GST-435): a prose line with no
|
||||
|
||||
+17
-3
@@ -56,7 +56,8 @@ const ANY_LABEL = new RegExp(`(?<![A-Za-z0-9])(${LABELS})(?![A-Za-z0-9])`);
|
||||
const LABEL_HEADING = new RegExp(`^(?:#{1,6}\\s*|\\*\\*)(${LABELS})(?:\\s+(?:findings?|severity|issues?))?\\s*:?\\**\\s*$`);
|
||||
const HEADING = /^(?:#{1,6}\s|\*\*[^*]+\*\*:?$)/;
|
||||
const LIST_ITEM = /^(?:[-*+]|\d+[.)])\s+\S/;
|
||||
const NUMBERED_HEAD = /^(?:#{1,6}\s+|\*\*\s*)\d+[.)]\s/;
|
||||
/** A numbered finding heading ("**1. …**", "### 2) …", GST-1031) or a finding lead that names its number ("**Finding 1 (FIXABLE): …**", GST-1657). */
|
||||
const NUMBERED_HEAD = /^(?:#{1,6}\s+|\*\*\s*)(?:\d+[.)]\s|Finding\s+\d+\b)/;
|
||||
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 = /(?<![A-Za-z0-9])Blocks:?\**:?\s*\**\s*yes(?![A-Za-z0-9])/i;
|
||||
@@ -174,8 +175,11 @@ export interface MergedLine { id: string; reader: string; label: string; stated?
|
||||
* starts the next finding, except an unlabeled, unnumbered bullet with no file:line under a paragraph finding whose fields have not
|
||||
* come yet, which is its detail and joins its text (GSTA-196: 4 findings with detail bullets collected as 8). An inline
|
||||
* `Blocks: yes` is CRITICAL too. The native voice's FIXABLE collects as CRITICAL; other labels stay as written for lead judgment. When in doubt the parser merges rather than drops: a lost blocker costs the gate.
|
||||
* A numbered finding heading keeps every unlabeled line under it, located or listed or bold, until its fields come (GST-1031), so its
|
||||
* Severity and Blocks never split off as a second record; a prose line that only names a severity in passing is a summary (summaryLine).
|
||||
* A numbered finding heading ("**1. …**", or a lead naming its number, "**Finding 1 (FIXABLE): …**", GST-1657) keeps every unlabeled line
|
||||
* under it, located or listed or bold, until its fields come (GST-1031), so its Severity and Blocks never split off as a second record;
|
||||
* the line right under it that carries those fields, no other label and no file:line joins it at the stronger label, as a paragraph after
|
||||
* a blank line does (a located line there is another finding), unless the heading itself names CRITICAL or P0, whose blocker another
|
||||
* subject's Blocks: no must not lift. A prose line that only names a severity in passing is a summary (summaryLine).
|
||||
* A native FIXABLE finding that states `Blocks: no` and one Severity field collects at that field on every class (GST-1031 ruling A);
|
||||
* one with no Blocks: no, no Severity field or only a FIXABLE field stays CRITICAL. fixableRaises false (a code or docs review with no
|
||||
* risk, error, history or floor reason, GST-987) also drops FIXABLE's raise from label headings and leads.
|
||||
@@ -234,6 +238,16 @@ export function parseFindings(reader: string, output: string, fixableRaises = tr
|
||||
}
|
||||
const heading = LABEL_HEADING.exec(t);
|
||||
if (heading) { section = fixableRaises ? named(heading[1]!) : heading[1]!; current = null; head = ''; continue; }
|
||||
const field = fieldOf(t);
|
||||
if (!afterGap && current?.heading && !current.fields.size && (field !== undefined || t.match(BLOCKS_FIELD)) && !ANY_LABEL.test(t.replace(SEVERITY_FIELD, ' '))
|
||||
&& !LIST_ITEM.test(t) && !LOCUS.test(t) && !HEADING.test(t) && !NUMBERED_HEAD.test(t) && !t.startsWith('{')
|
||||
&& ![...current.raw.split(' | ')[0]!.matchAll(ALL_LABELS)].some(m => rank(m[1]) === 4)) {
|
||||
current.raw += ` | ${t}`;
|
||||
current.label = blocks ? 'CRITICAL' : stronger(current.label, label ?? current.label);
|
||||
if (field !== undefined) current.fields.add('severity');
|
||||
if (t.match(BLOCKS_FIELD)) current.fields.add('blocks');
|
||||
continue;
|
||||
}
|
||||
if (current?.heading && !current.fields.size && !label && !blocks && !/^#{1,6}\s/.test(t) && !NUMBERED_HEAD.test(t) && !t.startsWith('{')) { current.raw += ` | ${t}`; continue; }
|
||||
if (!indented && HEADING.test(t)) { section = null; current = null; }
|
||||
if (t.startsWith('{')) {
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
REVIEWED_TREE: 7eb34e9e9d9aaa17ee0bad89281392cd611f15f2 /workspace/gstack-evals
|
||||
|
||||
**Scope.** I read the full diff against 97e4796 for every source file: `src/waste/crewlog.ts`, `src/waste/README.md`, `src/cli.ts`, `src/errors/codes.ts` and `docs/errors.md`. The test files were reviewed in summary mode only, so the coverage reduction is visible: `tests/waste/crewlog.test.ts` (+378, 48 tests on shell parsing, bash traits, thread exports, receipts, per-PR data and CLI errors) and `tests/cli.test.ts` (+1/-1, adds `waste` to PLANNED). At this tree, `bun test tests/waste tests/cli.test.ts` passes (48/48), and so do `bun run typecheck` and `bun run errors:check`. I checked findings against real inputs where I could: this crew's thread export shape, 11 gstack-evals PR bodies, the 7 FOUN-64 per-PR data files, and gstack 1.93.39.0's receipt code. I left no changes in the tree.
|
||||
|
||||
**1. R5 drops every PR with zero fix-checks from the rounds count, which inflates the past-cap share. FIXABLE.**
|
||||
crewlog.ts:618-619 and 625-626 (`receiptLoop`) and 730-731 (`scanPrs`) only compute rounds when `fix_checks` is present. gstack leaves it out when it is 0 (`if (fixChecks) out.fix_checks = fixChecks;`, review-scope.ts:458). `REVIEW_LOOP:` lines never print `rounds=` and also leave out a zero `fix_checks`. gstack's own rule is `rounds ?? passes + (fix_checks ?? 0)` (ship-receipt-depth.ts:259).
|
||||
- **Real receipts:** PR #110's receipt (`passes:1`, no `fix_checks` or `rounds`) comes back with `rounds: null`.
|
||||
- **FOUN-64 data:** `waste prs` measures rounds for 12 of 31 receipt PRs in gbrain, 13 of 20 in gstack-evals and 20 of 37 in truther. Overall it reports 27 of 45 PRs past the cap (60%, mean 3.71 rounds). gstack's rule over the same data gives 30 of 88 (34%, mean 2.72).
|
||||
|
||||
The dropped PRs are the low-round ones, so this is selection bias in the in-the-wild R5 baseline, not random noise. Fix: use gstack's fallback in all three places. If an older receipt's missing field really means "unknown", count it in a visible lower-bound bucket instead of dropping it. Severity: High. Blocks: yes (the next step publishes exactly this number from exactly this data).
|
||||
|
||||
**2. R1 cannot see re-runs of test runners that print timings. INVESTIGATE.**
|
||||
Two back-to-back `bun test tests/waste | tail -4` runs with no change between them differ only in `[32.00ms]` vs `[25.00ms]`. Scanning them gives `r1_checks: 0, informative_reruns: 1`. README:42 labels that bucket "a flake or a real change", which is wrong for timing noise. jest, vitest, pytest, go test and cargo test all print elapsed times, so `r1_checks` sits near zero for anything but quiet tools like tsc. The GEVA-220 plan's R1 already names a sensitivity reading that runs a frozen normalizer (durations, ISO timestamps, PIDs, temp-path suffixes) before comparing; the reader doesn't implement it. The exact-match primary rule follows the plan, so whether the crew baseline needs the normalized reading is a call for whoever owns the plan. Severity: Medium. Blocks: no.
|
||||
|
||||
**3. Substitutions hide tree changes from the lenient R1 boundary. FIXABLE.**
|
||||
`parseShell` (line 102, and the `$(` handling inside double quotes) keeps `$(...)` and backtick bodies as plain words and never parses them. A run of `bun run typecheck`, then `R=$(python3 scripts/fix_imports.py 2>&1); echo "$R"`, then `bun run typecheck` scores `r1: 1, r1_checks: 1`, even though the script edited the tree. Only `r1_strict` catches it. A related case: inside a `$(cat <<'EOF' ...)` commit message, an apostrophe in the here-doc body throws off `closeParen`'s single-quote scan, so the rest of the command (for example `&& git push`) is swallowed and `pushed_head_checks` undercounts. Smaller gaps: `env -u X git push` and `nice -n 10 cmd` take the flag's value as the program name, and `FIX_FLAGS` treats `python -u -m pytest` (unbuffered output) as an update flag. Fix: parse substitution bodies recursively and merge their traits into the segment. Severity: Low. Blocks: no.
|
||||
|
||||
**4. Some bad exports are accepted or misreported. FIXABLE.**
|
||||
An empty export file exits 0 and adds a session with 0 items to the totals, so a failed export step is counted silently. An item with `calls: [null]` (line 490), or a non-string `input.text` reaching `.trim()`, exits through E_INTERNAL saying "a defect in src/waste/crewlog.ts, not in your input". The README promises E_WASTE_INPUT for input problems. Fix: refuse an export with zero items, and check the call shape in `readExport`. Severity: Low. Blocks: no.
|
||||
|
||||
**5. The output prints model ids taken from the input. FIXABLE.**
|
||||
Line 486 copies each assistant item's `model` string into the output as an object key. That breaks the README's "never prints any other text from what it reads" claim. A model id that happens to be an inherited property name such as `constructor` also turns that count into a string. It is harmless for real Capy exports, but the privacy promise should either say this or the code should map the ids to a fixed set. Severity: Low. Blocks: no.
|
||||
|
||||
Recommendation: Fix the R5 rounds fallback at crewlog.ts:619/626/731 because it drops 43 of 88 receipt-bearing PRs in the real FOUN-64 data and reports 60% of PRs past the cap where gstack's own rule gives 34%.
|
||||
+34
@@ -0,0 +1,34 @@
|
||||
REVIEWED_TREE: 2e7994fd11ddd4e94557478583bfcdf7c54ee626 /workspace/gstack-evals
|
||||
|
||||
**Scope.** The range fc880c3..HEAD changes two files. `src/trial/review/run.ts` (source) adds each ceo session's `trial.json` and `host/skill-gates/` to the receipts list that `copyToSink` copies, and updates the header comment to match. `tests/trial/review/ceo-followup.test.ts` adds one test, "the receipts sink holds what ceo rows and the gate report read". It checks that `trial.json` and `host/skill-gates` land byte-identical under the sink. I reviewed the test and fixture files in summary mode only (diff stat and test names), so their bodies were not part of the adversarial pass.
|
||||
|
||||
**Checks.** That test file passes (45 pass, 0 fail) and `bun run typecheck` is clean.
|
||||
|
||||
**What holds up.**
|
||||
- Missing files are fine. `copyToSink` keeps only files that exist, so non-enforced arms with no `skill-gates/`, and sessions with no `trial.json`, copy cleanly.
|
||||
- Links are fine. `gateRecord`'s `copyTree` copies only regular files and directories, so `host/skill-gates/` can't bring a symlink into the sink. The hard-link check still covers every new target.
|
||||
- The paths match what the rows read. `trial.json` is written at `spec.dir`, which equals `sessionDir(...)`. That is exactly where `ceo-cli.ts` `gateReport` and `ceoDisk` look.
|
||||
- No new secret path. `trial.json` holds the record, the delta (path lists only), logs (stderr tails, copy failures) and the gate report. None of these carry the proxy token. Sealed suites are still refused a sink inside the repo.
|
||||
|
||||
**Finding 1 (FIXABLE): a re-run session's sink copy mixes the new trial.json with the earlier attempt's files.** `copyToSink` uses `cpSync(..., { recursive: true, force: true })`. That merges into an existing sink directory and never removes what is already there. It also skips sources that are missing this time, so a file the new attempt didn't write keeps the earlier attempt's copy.
|
||||
|
||||
I reproduced it with the real `copyToSink`:
|
||||
1. Attempt 1 wrote `trial.json`, `host/ceo-review/a/ceo-review.md`, `host/skill-gates/OLD/` and `host/transcript.ndjson`, and the run copied them to the sink.
|
||||
2. Attempt 2 ran in the same session dir (`rmSync(dir)` at run.ts:494) and wrote only a new `trial.json`.
|
||||
3. After the second copy, the sink held the new `trial.json` plus attempt 1's review copy, gate record and transcript.
|
||||
|
||||
Before this diff the sink had no `trial.json`, so `ceoDisk` and `gateReport` reading the sink copy got `copy_failed: true`, and `findings_written` and `gate_armed` read false. Now a fresh `trial.json` with empty `copy_out` vouches for stale `host/ceo-review/` copies and a stale transcript. So `ceo rows` run against the sink can score an attempt-2 journal entry with attempt-1 findings and exposure, and it gives no error.
|
||||
|
||||
It's reachable when a run completes, its copy lands, and a later run with the same out dir and sink re-runs sessions (`--repair` on stale cells, or a resume that re-runs non-reused cells). Rows read from the out dir are unaffected.
|
||||
|
||||
The fix: in `copyToSink`, or in run.ts before the copy, `rmSync` each session-scoped sink target for every session that ran in this invocation, whether or not its source exists now. Then copy. Add a test that re-runs one session and checks the sink holds no stale `ceo-review/` copy.
|
||||
|
||||
Severity: Medium
|
||||
Blocks: no
|
||||
|
||||
**Finding 2 (INVESTIGATE): the sink copy runs only once, at the end of `runReviewUnits` (run.ts:677).** This predates the diff, but the diff raises its stakes. A run that throws mid-way, or a machine that dies mid-way, copies nothing: not the new `trial.json` and gate records, and not even sessions that finished. On an ephemeral VM, every finished session's gate evidence is lost. Copying per session after `runTrial` would close it, but that changes the copy contract, so it needs a call from you.
|
||||
|
||||
Severity: Low
|
||||
Blocks: no
|
||||
|
||||
Recommendation: Fix the merge-not-replace sink copy at durable.ts copyToSink (exposed by run.ts:370-373 now copying trial.json) because a re-run session's fresh trial.json makes sink-based ceo rows trust the previous attempt's ceo-review copies and transcript, which silently corrupts findings_written and exposure.
|
||||
@@ -0,0 +1,27 @@
|
||||
REVIEWED_TREE: 45c8748f0c330a4959d4cbdda7ecce5b3fa0d79f /workspace/gstack
|
||||
|
||||
**Scope.** The range 4eff47c5..HEAD changes three files across commits e0220c0, eb75309 and 0854fea. I read `lib/ship-preflight.ts` (one added line at :71) and `changelog.d/gst-691-preflight-merge-noop.md` in full, plus the `git()` helper in `lib/ship-bin-common.ts:18`. I reviewed `test/ship-preflight-base-ancestor.test.ts` (+59 lines) in summary mode only, so this coverage is reduced and that is deliberate. It has three cases: base already in HEAD runs no `git merge` and prints `MERGE: up to date`; a moved base still runs `git merge`; an unfinished merge with conflicts still stops on those conflicts.
|
||||
|
||||
**What I checked and found clean.** All of these fall through to the old `git merge` path, so they behave exactly as before:
|
||||
- `merge-base --is-ancestor` exits 1 (not an ancestor) or 128 (missing ref, broken repo).
|
||||
- The spawn times out, which makes the status null.
|
||||
- A shallow clone, where the check can only give a false "not an ancestor", never a false "ancestor".
|
||||
- `ls-files -u` output that overflows the buffer, which leaves the output non-empty and so takes the merge path.
|
||||
|
||||
The `MERGE: up to date` string is the same one the old path printed when HEAD did not move, so nothing downstream sees a new format. Fetch staleness, a narrowed refspec and argument injection through `base` were already present at the base revision; this range does not change them.
|
||||
|
||||
**Finding 1: an in-progress merge whose conflicts are resolved and staged gets past the guard (FIXABLE).** In `lib/ship-preflight.ts:71`, the `ls-files -u` guard only sees conflicts that are still unresolved. If someone resolves and `git add`s the files but has not committed, the unmerged list is empty while MERGE_HEAD still exists. If origin/main is already in HEAD, the new branch then prints `MERGE: up to date` and continues into pregate and the test lanes.
|
||||
|
||||
Before this change, `git merge origin/main` refused here and preflight stopped. I reproduced it in a scratch repo:
|
||||
- unmerged entries: 3 before `git add`, 0 after
|
||||
- `MERGE_HEAD present`
|
||||
- `is-ancestor exit=0`
|
||||
- the old path fails with `fatal: You have not concluded your merge (MERGE_HEAD exists).`
|
||||
|
||||
The same gap applies to CHERRY_PICK_HEAD and REVERT_HEAD after resolution. The consequence is that `/ship` goes on carrying a merge nobody concluded. A later path-limited commit (the version or CHANGELOG stamp) would then fail with "cannot do a partial commit during a merge", far from the cause; otherwise the unrelated merge is swept into the ship commit. Commit 0854fea set out to leave unfinished merges to Step 3's stop, and it only covers the conflicted case. Its test has no resolved-but-uncommitted case.
|
||||
|
||||
The fix is to also require that no operation is in progress before taking the shortcut, for example `!git(cwd, ['rev-parse', '-q', '--verify', 'MERGE_HEAD']).stdout.trim()`, with CHERRY_PICK_HEAD and REVERT_HEAD checked the same way. Then add a test case that resolves and stages the conflict without committing. No test fails today, nothing is lost and the failure is loud, so this sits below the blocking floor. Severity: Low. Blocks: no
|
||||
|
||||
No other findings.
|
||||
|
||||
Recommendation: Ship as-is because the strongest finding, a staged but uncommitted merge slipping past the shortcut at ship-preflight.ts:71, needs a half-finished manual merge and fails loudly later instead of losing data. It's a cheap MERGE_HEAD check to add in a follow-up.
|
||||
@@ -0,0 +1,90 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { collect, ledgerBlockers, parseFindings, readJsonl, recordJudgment, statesNoBlocks, type JudgedLine } from '../lib/review-ledger';
|
||||
|
||||
const read = (f: string) => fs.readFileSync(path.resolve(import.meta.dir, 'fixtures/review-ledger/gst-1657', f), 'utf8');
|
||||
const CAPTURED = read('geva-358-pr248-p2_adversarial-native.txt');
|
||||
const native = (output: string, raises: boolean) => parseFindings('adversarial-native', output, raises);
|
||||
const finding = (head: string, fields: string[]) => [head, 'It merges and never deletes.', '',
|
||||
'I reproduced it:', '1. Attempt 1 wrote trial.json.', '2. Attempt 2 ran in the same dir (run.ts:494).', '',
|
||||
'The fix: rmSync each sink target, then copy.', '', ...fields, '', 'Recommendation: Fix copyToSink because it mixes attempts.'].join('\n');
|
||||
const BOLD = "**Finding 1 (FIXABLE): a re-run session's sink copy mixes attempts.**";
|
||||
/** Collects one native output, records a judgment that keeps every label except the lowerings given, and returns the verdict and blockers. */
|
||||
const judged = (output: string, lower: Record<string, [string, string]> = {}) => {
|
||||
const d = fs.mkdtempSync(path.join(os.tmpdir(), 'gst1657-'));
|
||||
const [ledger, merged, file, out] = ['l.jsonl', 'm.jsonl', 'j.jsonl', 'out.txt'].map(f => path.join(d, f)) as [string, string, string, string];
|
||||
fs.writeFileSync(out, output);
|
||||
const c = collect(ledger, 'adversarial-native', output, merged, out);
|
||||
const set: JudgedLine[] = c.lines.map(l => (lower[l.id] ? { id: l.id, severity: lower[l.id]![0], reason: lower[l.id]![1] } : { id: l.id, severity: l.label }));
|
||||
fs.writeFileSync(file, set.map(j => JSON.stringify(j)).join('\n') + '\n');
|
||||
const verdict = recordJudgment(ledger, c.lines, readJsonl(file), file, merged);
|
||||
const result = { labels: c.lines.map(l => l.label), ok: verdict.ok, dismissed: verdict.dismissed, blockers: ledgerBlockers(ledger).length };
|
||||
fs.rmSync(d, { recursive: true, force: true });
|
||||
return result;
|
||||
};
|
||||
|
||||
describe("a native finding's own Severity and Blocks lines stay on that finding (GST-1657)", () => {
|
||||
// Value: protects=a native adversarial finding led by "**Finding N (FIXABLE): ...**" keeps its body (numbered steps, located lines,
|
||||
// later paragraphs) until its Severity and Blocks lines come, so "Severity: Medium / Blocks: no" collects at Medium under GST-1031
|
||||
// ruling A instead of the FIXABLE raise; fails_when=the finding collects CRITICAL, its fields split off as a second record carrying
|
||||
// another paragraph as its subject, or the ledger counts a blocker; why_new=gstack-evals #248 (GEVA-358 p2/adversarial-native#1)
|
||||
// collected CRITICAL beside a phantom Medium record, the judge refused the downgrade and /ship blocked until a hand-granted
|
||||
// review-loop exception; seam=parseFindings, collect, recordJudgment and ledgerBlockers; no_claim=other finding-lead spellings
|
||||
test('the captured #248 native output collects the Severity its reader wrote, with no blocker', () => {
|
||||
for (const raises of [true, false]) {
|
||||
const found = native(CAPTURED, raises);
|
||||
expect([raises, found.map(f => f.label)]).toEqual([raises, ['Medium', 'Low']]);
|
||||
expect(found[0]!.raw.startsWith('**Finding 1 (FIXABLE)')).toBe(true);
|
||||
expect(found[0]!.raw.endsWith(' | Severity: Medium | Blocks: no')).toBe(true);
|
||||
}
|
||||
expect(judged(CAPTURED)).toEqual({ labels: ['Medium', 'Low'], ok: true, dismissed: [], blockers: 0 });
|
||||
});
|
||||
|
||||
// Value: protects=a line right under a numbered heading that carries only that finding's Severity and Blocks fields joins the heading
|
||||
// instead of starting a phantom record, keeping the stronger label (GST-1406: collect cannot tell a heading's own inline fields from
|
||||
// another subject's), so a Blocks: no finding counts no blocker and lead judgment may lower it to its stated Severity, while
|
||||
// Blocks: yes still blocks; fails_when=a phantom Low or Medium record reappears beside its heading, the Blocks: yes finding stops
|
||||
// counting, or the lowering is refused; why_new=the GST-1657 replay found gstack-evals GEVA-228 (three FIXABLE findings, each
|
||||
// "Severity: Low. Blocks: no.") and GSTA-234 collecting CRITICAL headings the judge would not lower; seam=parseFindings, collect,
|
||||
// recordJudgment and ledgerBlockers; no_claim=that a located paragraph after a blank line joins (it stays its own record)
|
||||
test('inline fields under a numbered heading land on it, at the stronger label', () => {
|
||||
const crewlog = read('geva-228-s1.3_adversarial-native.txt');
|
||||
const preflight = read('gsta-234-s6_adversarial-native.txt');
|
||||
for (const raises of [true, false]) {
|
||||
expect([raises, native(crewlog, raises).map(f => f.label)]).toEqual([raises, ['CRITICAL', 'Medium', 'CRITICAL', 'CRITICAL', 'CRITICAL']]);
|
||||
expect([raises, native(crewlog, raises).map(f => statesNoBlocks(f.raw))]).toEqual([raises, [false, true, true, true, true]]);
|
||||
expect([raises, native(preflight, raises).map(f => [f.label, statesNoBlocks(f.raw)])]).toEqual([raises, [['CRITICAL', true]]]);
|
||||
}
|
||||
const why = 'the reader wrote Severity: Low. Blocks: no.';
|
||||
expect(judged(crewlog, { 'adversarial-native#3': ['Low', why], 'adversarial-native#4': ['Low', why], 'adversarial-native#5': ['Low', why] }))
|
||||
.toEqual({ labels: ['CRITICAL', 'Medium', 'CRITICAL', 'CRITICAL', 'CRITICAL'], ok: true, dismissed: ['adversarial-native#3', 'adversarial-native#4', 'adversarial-native#5'], blockers: 1 });
|
||||
expect(judged(preflight)).toEqual({ labels: ['CRITICAL'], ok: true, dismissed: [], blockers: 0 });
|
||||
});
|
||||
|
||||
// Value: protects=FIXABLE still raises whenever the finding's own fields do not lower it: Blocks: yes, a Severity with no Blocks: no
|
||||
// on a raising class (GST-987), and no fields at all collect CRITICAL; a "### Finding N" lead and the inline quote of the issue
|
||||
// ("Severity: Medium, Blocks: no") collect at Medium; an unrelated paragraph's inline fields under a heading never lower its label
|
||||
// (GST-1406's probe); fails_when=any control collects below CRITICAL or a lowered shape stays CRITICAL; why_new=GST-1657;
|
||||
// seam=parseFindings; no_claim=that each control is a real finding
|
||||
test('the FIXABLE raise holds unless the finding states Blocks: no and one Severity', () => {
|
||||
for (const [fields, raises, want] of [[['Severity: Medium', 'Blocks: yes'], false, 'CRITICAL'], [['Severity: Medium'], true, 'CRITICAL'],
|
||||
[[], true, 'CRITICAL'], [[], false, 'CRITICAL'], [['Severity: Medium', 'Blocks: no'], true, 'Medium'], [['Severity: Low. Blocks: no.'], true, 'Low']] as const)
|
||||
expect([fields, raises, native(finding(BOLD, [...fields]), raises).map(f => f.label)]).toEqual([fields, raises, [want]]);
|
||||
expect(native(finding('### Finding 1 (FIXABLE): sink copy mixes attempts', ['Severity: Medium', 'Blocks: no']), true).map(f => f.label)).toEqual(['Medium']);
|
||||
expect(native(`${BOLD} It merges and never deletes. Severity: Medium, Blocks: no`, true).map(f => f.label)).toEqual(['Medium']);
|
||||
const two = native([finding(BOLD, ['Severity: Medium', 'Blocks: no']).replace(/\n\nRecommendation:.*/, ''), '',
|
||||
'**Finding 2 (FIXABLE): the copy runs once (run.ts:677).**', 'A crash copies nothing.', '', 'Severity: High', 'Blocks: yes'].join('\n'), true);
|
||||
expect(two.map(f => f.label)).toEqual(['Medium', 'CRITICAL']);
|
||||
for (const raises of [true, false])
|
||||
expect(native(['### 1. FIXABLE: webhook auth bypass in lib/auth.ts:42', 'Any caller can post to the webhook.', '',
|
||||
'Unrelated, the help text names an old flag. Severity: Low. Blocks: no.'].join('\n'), raises).map(f => f.label)).toEqual(['CRITICAL']);
|
||||
const located = ['**Finding 1 (FIXABLE): A in x.ts:3**', 'Second, y.ts:9 drops writes. Severity: High; Blocks: no'].join('\n');
|
||||
expect(native(located, true).map(f => f.label)).toEqual(['CRITICAL', 'High']);
|
||||
expect(judged(located, { 'adversarial-native#1': ['High', 'the next line says Blocks: no'] }).ok).toBe(false);
|
||||
const stated = parseFindings('outside-voice', ['### 1. CRITICAL: webhook accepts requests without authentication',
|
||||
'Separately, the help text names an obsolete flag. Severity: Low. Blocks: no.'].join('\n'));
|
||||
expect(stated.map(f => [f.label, statesNoBlocks(f.raw)])).toEqual([['CRITICAL', false], ['Low', true]]);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user