diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ebfab1db..0a7645a98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,35 @@ ## [1.87.3.0] - 2026-09-15 -### Fixed +**Changed code needs another pass.** +**Review freshness now checks both ends.** + +A green review now stays attached to the code that was there when the pass began. `/review` and `/ship` capture that content before reading it, then compare again when the pass finishes. If fixes changed the files, another pass has to review those fixes before the dashboard can call the result CURRENT. Older log-only records remain visible, but cannot stand in for a completed code review. + +A clean result requires a completed pass on unchanged content with no unresolved findings. A stopped or nonconverged run cannot clear the code-review row just because its commit has not moved. + +### The three numbers that matter + +Source: scenarios in `test/review-start-evidence.test.ts`, checked against released v1.87.0.0 and this version. Run `bun test test/review-start-evidence.test.ts` to verify current behavior. These are false CURRENT grades across five deterministic cases, not production incident estimates. + +| Cases incorrectly graded CURRENT | Before | After | Δ | +|---|---:|---:|---:| +| Mid-review edits: tracked and untracked source | 2 | 0 | -2 | +| Log-only result without a captured review start | 1 | 0 | -1 | +| Codex advisory findings: none or only some resolved | 2 | 0 | -2 | + +The old stamp could certify a review after its own fixes changed the tree. Those cases now stay STALE or UNVERIFIED, including an advisory-only Codex pass that leaves findings unresolved. + +### What this means for developers + +You can distinguish a completed, unchanged pass from a run that still needs attention before merging. Plan reviews keep their existing rules; they assess the plan rather than the checked-out source. Completion is still reviewer-reported, not proof that an LLM read every file. Run `/review` again after fixes and use the new result when preparing to ship. + +### Itemized changes + +#### Fixed - **Review freshness no longer certifies unreviewed fixes.** `/review` and `/ship` bind each diff pass to the content captured before it starts. Edits during review, incomplete passes, and older log-only records stay stale or unverified in the readiness dashboard and `/land-and-deploy`, even when HEAD has not moved. Plan-review evidence keeps its existing freshness rules. +- **Codex readiness keeps unresolved findings visible.** Passing the critical-findings gate does not make a review CURRENT while advisory findings remain open. The gate's severity policy is unchanged. ## [1.87.0.0] - 2026-09-11 diff --git a/lib/review-evidence.ts b/lib/review-evidence.ts index 5a7405846..1f4d8a34e 100644 --- a/lib/review-evidence.ts +++ b/lib/review-evidence.ts @@ -61,7 +61,8 @@ export function reviewFreshness(rec: Record, currentWtree: string): if (!currentWtree || currentWtree === 'unknown' || rec.wtree !== currentWtree) { return { status: 'STALE', reason: 'working-tree content differs from reviewed content' }; } - if (rec.status !== 'clean' || rec.issues_found > 0 || rec.critical > 0) { + if (rec.status !== 'clean' || rec.issues_found > 0 || rec.critical > 0 || + (rec.skill === 'codex-review' && rec.findings > (rec.findings_fixed ?? 0))) { return { status: 'UNVERIFIED', reason: 'review has unresolved findings or did not finish clean' }; } return { status: 'CURRENT', reason: 'completed clean pass on unchanged content' }; diff --git a/test/review-start-evidence.test.ts b/test/review-start-evidence.test.ts index 5da5f0b61..9018fbc69 100644 --- a/test/review-start-evidence.test.ts +++ b/test/review-start-evidence.test.ts @@ -138,6 +138,21 @@ describe('review start/end binding (#2803)', () => { expect(log(rerun, { skill: 'codex-review' }).review_freshness.status).toBe('CURRENT'); }); + for (const [findings, findings_fixed, freshness] of [ + [2, 0, 'UNVERIFIED'], + [2, 1, 'UNVERIFIED'], + [2, 2, 'CURRENT'], + [0, 0, 'CURRENT'], + ] as const) { + test(`Codex gate pass with ${findings_fixed}/${findings} findings resolved grades ${freshness}`, () => { + const token = cli('gstack-review-log', ['--start', 'codex-review']); + const row = log(token, { skill: 'codex-review', status: 'clean', gate: 'pass', findings, findings_fixed }); + expect(row.review_binding.state).toBe('verified'); + expect(row.gate).toBe('pass'); + expect(row.review_freshness.status).toBe(freshness); + }); + } + test('legacy diff rows cannot use log-time wtree or HEAD; plan evidence is unchanged', () => { log(undefined, { skill: 'plan-eng-review', completed: undefined, converged: undefined }); const file = findFilesBySuffix(home, '-reviews.jsonl')[0];