fix: a claimed clean review row with any unresolved non-gate CRITICAL finding reads UNVERIFIED (pass 2 red-team)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-10-31 13:05:31 +00:00
1 parent 9df9c6c9b3
commit 04a0ceea9a
3 files changed
+7 -3

No files matched your search

+2 -2
View File
@@ -9,11 +9,11 @@
- **`gstack-tree-receipt`** fails closed (`EVIDENCE_IDENTITY_UNKNOWN`) when git cannot produce the diff between the gated and current heads, and names git's reason. Before, it read a failed diff as no changes.
- **`gstack-review-log`** records a completed `review` or `review-fix-check` row claimed as `issues_found` as `clean` with zero counts when every finding's logger-stamped action is `fixed` or `auto-fixed`, or `deferred` or `noted` on a finding marked `blocking: false` that is not CRITICAL, and the findings account for the claimed issue and critical counts. A skipped finding, a gate finding left open, a CRITICAL or blocking note, a finding with no action, or an incomplete pass keeps the claimed status.
- **Findings files** (`--findings`) count `auto-fixed` as resolved, the same as `fixed`, and `deferred` or `noted` only below the floor. Their gate rows are stamped like inline ones, so an unattended run's gate stays open whatever its status or disposition says.
- **`review_freshness`** reads a claimed clean row as UNVERIFIED when a finding is a gate left `open` or a CRITICAL or blocking deferral or note.
- **`review_freshness`** reads a claimed clean row as UNVERIFIED when a finding is a gate left `open`, a CRITICAL or blocking deferral or note, or any other unresolved CRITICAL finding.
- **`review_freshness`** for a pass whose tree moved now reads `content changed during review: the tree moved from <start> to <end> while <skill> ran …`, followed by the repair: keep the checkout frozen until every reviewer returns, then rerun on the frozen tree (docs/troubleshooting.md#review-content-changed).
### Tests
- `test/ship-rereview-triggers.test.ts`: preflight on a founderstar-style adjacent edit prints the scoped-review `NEXT: carry route`, and its dry run writes no packet. `test/ship-control-flow.test.ts`: Step 1.4 and Step 16 stages 2 and 3 name the routes.
- `test/ship-receipt-carry-large-diff.test.ts`: a PR that adds 6,000 receipts under a long directory name writes a receipt with its product patch-id and carries across a base merge. Negative control: one rescored receipt still refuses with `RECEIPT_CARRY_PRODUCT_CHANGED`.
- `test/review-notes-only-pass.test.ts`: a pass with three notes is recorded clean and reads CURRENT. Negative controls: a skipped finding, an open gate finding (also under a caller's resolved status), a CRITICAL or blocking note, a note with no `blocking` field, counts the findings do not cover, a finding with no action, and an incomplete pass keep `issues_found`. A claimed clean row with an open gate or a CRITICAL deferral is not CURRENT, while kept and restored gates, skipped advisories and an INFORMATIONAL deferral stay CURRENT. A `--findings` gate row stays open in an unattended run. Also checks the reason a pass gets when its tree moved.
- `test/review-notes-only-pass.test.ts`: a pass with three notes is recorded clean and reads CURRENT. Negative controls: a skipped finding, an open gate finding (also under a caller's resolved status), a CRITICAL or blocking note, a note with no `blocking` field, counts the findings do not cover, a finding with no action, and an incomplete pass keep `issues_found`. A claimed clean row with an open gate, a CRITICAL deferral, or a skipped or action-less CRITICAL finding is not CURRENT, while kept and restored gates, skipped advisories and an INFORMATIONAL deferral stay CURRENT. A `--findings` gate row stays open in an unattended run. Also checks the reason a pass gets when its tree moved.
- `test/ship-receipt-carry-large-diff.test.ts` also: the patch-id file lives under the git directory (an unwritable TMPDIR still carries; an unwritable spot refuses by name), and a stale directory is swept. `test/train-gate.test.ts`: a diff git cannot produce refuses the tree receipt with git's reason. `test/ship-rereview-triggers.test.ts`: a dry run the receipt would refuse anyway names that refusal.
+2 -1
View File
@@ -355,7 +355,8 @@ export function reviewFreshness(rec: Record<string, any>, currentWtree: string):
if (rec.status !== 'clean' || open > 0 || rec.critical > 0 ||
(rec.skill === 'codex-review' && rec.findings > (rec.findings_fixed ?? 0)) ||
(Array.isArray(rec.findings) && rec.findings.some(f => record(f) && (f.action === 'open' ||
((f.action === 'deferred' || f.action === 'noted') && (f.blocking === true || isCriticalFinding(f))))))) {
((f.action === 'deferred' || f.action === 'noted') && (f.blocking === true || isCriticalFinding(f))) ||
(!f.gate && isCriticalFinding(f) && isOpenByAction(f)))))) {
return { status: 'UNVERIFIED', reason: 'review has unresolved findings or did not finish clean' };
}
return { status: 'CURRENT', reason: 'completed clean pass on unchanged content' };
+3
View File
@@ -91,6 +91,9 @@ describe('a notes-only /ship review pass (GST-430)', () => {
expect(reviewFreshness(row([note(1, 'deferred', { severity: 'CRITICAL', issue: null })]), w)?.status).toBe('UNVERIFIED');
expect(reviewFreshness(row([note(1, 'deferred', { blocking: true })]), w)?.status).toBe('UNVERIFIED');
expect(reviewFreshness(row([note(1, 'open', { gate: 'RH1' })]), w)?.status).toBe('UNVERIFIED');
expect([note(1, 'skipped', { severity: 'CRITICAL' }), { fingerprint: 'lib/a.ts:2:x', severity: 'CRITICAL' }, note(1, 'mystery', { severity: 'CRITICAL' })]
.map(f => reviewFreshness(row([f]), w)?.status)).toEqual(['UNVERIFIED', 'UNVERIFIED', 'UNVERIFIED']);
expect(reviewFreshness(row([note(1, 'kept', { gate: 'RH1', reason: 'r', actor: 'interactive', severity: 'CRITICAL' })]), w)?.status).toBe('CURRENT');
const gate = (action: string) => note(1, action, { gate: 'RH1', reason: 'r', actor: 'interactive' });
expect([gate('kept'), gate('restored'), note(1, 'skipped'), note(1, 'deferred', { blocking: undefined }), note(1, 'noted')]
.map(f => reviewFreshness(row([f]), w)?.status)).toEqual(['CURRENT', 'CURRENT', 'CURRENT', 'CURRENT', 'CURRENT']);