mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
fix(review): keep unresolved Codex findings unverified
This commit is contained in:
1 parent
87dfa748a7
commit
69cd6849cf
3 files changed
+44
-2
No files matched your search
+27
-1
@@ -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
|
||||
|
||||
|
||||
@@ -61,7 +61,8 @@ export function reviewFreshness(rec: Record<string, any>, 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' };
|
||||
|
||||
@@ -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];
|
||||
|
||||
Reference in new issue
Block a user