mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-03 23:37:53 +01:00
fix(ship): a missing recorded output file is dirty even with no findings (GST-541 fix-check)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
5391c13ee0
commit
29ba2981d0
3 files changed
+11
-2
No files matched your search
@@ -1,4 +1,4 @@
|
||||
**`gstack-review-dispatch verify` now catches a merged findings file edited after collect.** Verify used to compare only output hashes and line counts. So a `merged.jsonl` row whose label was lowered, or whose text was replaced with another line's summary, passed with `VERIFY: clean` once the judge agreed with the edited row. Verify now re-parses each reader's unchanged output and checks every merged row against the finding at its index, label and raw text byte for byte. A mismatch prints `merged_line_differs` on the OUTPUT and JUDGE rows, and verify is dirty. A missing output file, a judgment run on a copy of the merged file, and merged rows that collect never wrote are dirty too.
|
||||
**`gstack-review-dispatch verify` now catches a merged findings file edited after collect.** Verify used to compare only output hashes and line counts. So a `merged.jsonl` row whose label was lowered, or whose text was replaced with another line's summary, passed with `VERIFY: clean` once the judge agreed with the edited row. Verify now re-parses each reader's unchanged output and checks every merged row against the finding at its index, label and raw text byte for byte. A mismatch prints `merged_line_differs` on the OUTPUT and JUDGE rows, and verify is dirty. A missing output file (even for a reply with no findings), a judgment run on a copy of the merged file, and merged rows that collect never wrote are dirty too.
|
||||
|
||||
### Known
|
||||
- Deleting a whole output event together with its merged rows still verifies clean, because the ledger keeps no anchor for it. It is listed for the structured-findings plan.
|
||||
|
||||
@@ -289,7 +289,7 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri
|
||||
if (lines.length !== ids.length) { status = 'MISMATCH'; notes.push(`merged_lines=${lines.length}/${ids.length}`); }
|
||||
if (lines.some(l => l.output_sha256 !== o.sha256)) { status = 'MISMATCH'; notes.push('merged_line_from_other_output'); }
|
||||
// Each merged row must be the finding collect parses at that id's index from the unchanged output, label and raw text byte for byte.
|
||||
if (text === undefined && ids.length) {
|
||||
if (text === undefined && (ids.length || typeof o.file === 'string')) {
|
||||
status = 'MISMATCH';
|
||||
notes.push(typeof o.file === 'string' ? 'output_file_missing' : 'output_file_unrecorded');
|
||||
differs.push(...ids);
|
||||
|
||||
@@ -516,6 +516,15 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change
|
||||
expect(run(['judge', '--ledger', copy.ledger, '--merged', forgedCopy, '--judged', copy.w('judged.jsonl', readJsonl<MergedLine>(copy.merged).map(m => JSON.stringify({ id: m.id, severity: 'INFORMATIONAL' })).join('\n') + '\n')]).status).toBe(0);
|
||||
const viaCopy = run(['verify', '--ledger', copy.ledger]);
|
||||
expect([viaCopy.status, viaCopy.stdout]).toEqual([1, expect.stringMatching(/^JUDGE - MISMATCH .*judge_merged_not_collected/m)]);
|
||||
const quiet = pass();
|
||||
run(['sent', '--ledger', quiet.ledger, '--role', 'specialist', '--name', 'testing', '--sent', quiet.file]);
|
||||
const none = quiet.w('none.out', 'NO FINDINGS\n');
|
||||
expect(run(['collect', '--ledger', quiet.ledger, '--role', 'specialist', '--name', 'testing', '--output', none, '--merged', quiet.merged]).status).toBe(0);
|
||||
fs.writeFileSync(quiet.merged, '');
|
||||
expect(run(['judge', '--ledger', quiet.ledger, '--merged', quiet.merged, '--judged', quiet.w('judged.jsonl', '')]).status).toBe(0);
|
||||
fs.rmSync(none);
|
||||
const noneGone = run(['verify', '--ledger', quiet.ledger]);
|
||||
expect([noneGone.status, noneGone.stdout.includes('output_file_missing')]).toEqual([1, true]);
|
||||
const extra = forge(setup(), rows => { rows[0]!.label = 'INFORMATIONAL'; rows.push({ ...rows[1]!, id: 'specialist/testing#9' }); });
|
||||
expect([extra.status, extra.stdout.includes('merged_rows_not_collected=["specialist/testing#9"]')]).toEqual([1, true]);
|
||||
});
|
||||
|
||||
Reference in new issue
Block a user