mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-02 23:14:33 +01:00
fix(ship): an output with no readable recorded file is dirty even with no ids (GST-541 fix-check 3)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
50dc73cd5a
commit
b30df5c9d6
3 files changed
+11
-7
No files matched your search
@@ -1,11 +1,11 @@
|
||||
**`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, merged rows that collect never wrote, and an output whose finding count no longer matches its recorded ids are dirty too. A reply refused for naming the wrong checkout is still allowed to hold findings collect never merged, but only when verify re-checks its tree line and finds it wrong.
|
||||
**`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 or an output event with no file recorded (even for a reply with no findings), a judgment run on a copy of the merged file, merged rows that collect never wrote, and an output whose finding count no longer matches its recorded ids are dirty too. A reply refused for naming the wrong checkout is still allowed to hold findings collect never merged, but only when verify re-checks its tree line and finds it wrong.
|
||||
|
||||
### 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.
|
||||
|
||||
### Tests
|
||||
- `test/review-dispatch.test.ts`: a forged label, a summary copied onto a blocker's id, a deleted output file, a judgment on a forged copy and an extra merged row all verify dirty. Each verified clean at main c830c9b.
|
||||
- `test/review-dispatch.test.ts`: an output event edited to look like a wrong-tree refusal (no ids, `review: false`, `tree_ok: false`) verifies dirty, even after a re-sent brief is answered. A real wrong-tree reply stays `NOT_REVIEWED` until a re-sent brief gets a review, then verifies clean.
|
||||
- `test/review-dispatch.test.ts`: an output event edited to look like a wrong-tree refusal (no ids, `review: false`, `tree_ok: false`) verifies dirty, even after a re-sent brief is answered, and so does one edited to drop its file and ids. A real wrong-tree reply stays `NOT_REVIEWED` until a re-sent brief gets a review, then verifies clean.
|
||||
|
||||
### Docs
|
||||
- `docs/review-policy.md` (verify) describes the per-row check.
|
||||
@@ -307,11 +307,11 @@ 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 || typeof o.file === 'string')) {
|
||||
if (text === undefined) {
|
||||
status = 'MISMATCH';
|
||||
notes.push(typeof o.file === 'string' ? 'output_file_missing' : 'output_file_unrecorded');
|
||||
differs.push(...ids);
|
||||
} else if (text !== undefined && promptHash(text) === o.sha256) {
|
||||
} else if (promptHash(text) === o.sha256) {
|
||||
const parsed = parseFindings(reader, text);
|
||||
const off = ids.filter((id, k) => { const m = lines.find(l => l.id === id); return !m || parsed[k]?.label !== m.label || parsed[k]?.raw !== m.raw; });
|
||||
if (unavailableOutput(o) && parsed.length) { status = 'MISMATCH'; notes.push('unavailable_output_has_findings'); }
|
||||
|
||||
@@ -540,10 +540,10 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change
|
||||
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]);
|
||||
});
|
||||
// Value: protects=an output event edited to read like a refused reply (no ids, review false, tree_ok false) cannot drop its findings
|
||||
// and still verify clean, while a reply collect really refused for its tree stays NOT_REVIEWED until a re-sent brief is answered;
|
||||
// Value: protects=an output event edited to read like a refused reply (no ids, review false, tree_ok false) or to drop its file
|
||||
// cannot drop its findings and still verify clean, while a reply collect really refused for its tree stays NOT_REVIEWED until a re-sent brief is answered;
|
||||
// fails_when=verify skips the count check on the editable review or tree_ok field, so a clean marker keeps the output MATCH or a
|
||||
// later review voids it (fix-checks at 721539c and 56ed6e3: VERIFY: clean); why_new=the GST-482 count exemption trusted collect's
|
||||
// later review voids it, or an output with no recorded file passes on its review field (fix-checks at 721539c, 56ed6e3 and 50dc73c: VERIFY: clean); why_new=the GST-482 count exemption trusted collect's
|
||||
// fields; seam=verify; no_claim=that a deleted output event is caught (#3719)
|
||||
test('an output event edited to look refused is dirty; a real wrong-tree reply is NOT_REVIEWED, then voided by a review', () => {
|
||||
const head = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: ROOT, encoding: 'utf8', timeout: 30_000 }).stdout.trim();
|
||||
@@ -574,6 +574,10 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change
|
||||
noIds.forge({ ids: [], review: false });
|
||||
const v = noIds.verify();
|
||||
expect([v.status, v.stdout.includes('merged_count_differs=1/0'), v.stdout.includes('VERIFY: dirty')]).toEqual([1, true, true]);
|
||||
const unfiled = voice(findings);
|
||||
unfiled.forge({ file: null, ids: [] });
|
||||
const u = unfiled.verify();
|
||||
expect([u.status, u.stdout.includes('output_file_unrecorded'), u.stdout.includes('VERIFY: dirty')]).toEqual([1, true, true]);
|
||||
const flagged = voice(findings);
|
||||
flagged.forge({ ids: [], review: false, tree_ok: false });
|
||||
flagged.send();
|
||||
|
||||
Reference in new issue
Block a user