revert: rollback exact-review admission and tuple fixes (#840)#841
revert: rollback exact-review admission and tuple fixes (#840)#841brokemac79 wants to merge 1 commit into
Conversation
|
Regarding #840 Proof of what was fixed before, openclaw/openclaw#111368This PR is exactly what shows the fixes to the previous bugs. 111368, there's a small issue with Clawsweeper right now - PRs that are in the backlog period can't be accurately reviewed or have their review conclusions issued a reflection of that No impact on newly submitted issues/PRs |
|
Codex review: needs real behavior proof before merge. Reviewed July 24, 2026, 8:37 PM ET / July 25, 2026, 00:37 UTC. ClawSweeper reviewWhat this changesThis draft mechanically reverts the exact-review queue’s terminal-item admission safeguards and the publisher’s structured batch-outcome reporting added by the prior merged PR. Merge readiness⛔ Blocked until stronger real behavior proof is added - 5 items remain Keep this draft PR open as a maintainer-controlled containment option, not a merge candidate: it intentionally restores the failure modes fixed by #840, and no material post-merge regression evidence is present here to justify that trade-off. Likely related people: brokemac, with high confidence from the merged implementation and this rollback proposal. Priority: P2 Review scores
Verification
How this fits togetherGitHub events enter the exact-review queue, which admits work, dispatches bounded review batches, and records durable completion state. The publisher reports each batch item’s outcome back to the coordinator so terminal failures are not retried as generic transient failures. flowchart LR
A[GitHub review event] --> B[Exact-review queue]
B --> C[Admission status check]
C --> D[Bounded batch dispatch]
D --> E[Publisher result]
E --> F[Durable queue and dashboard state]
C --> G[Terminal completion without dispatch]
Decision needed
Why: The branch is deliberately a faithful rollback rather than a corrective patch, so source review cannot determine whether the operational containment trade-off is justified. Before merge
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the protections from #840 on main; use this branch only if a documented monitor signal establishes a material regression and verifies that the rollback is the safest containment action. Do we have a high-confidence way to reproduce the issue? Not applicable as a reported bug reproduction: this is a contingency rollback, and the supplied context does not establish a current material regression from the merged behavior. Is this the best way to solve the issue? No for immediate merge: the rollback is mechanically coherent, but it is only the best solution after monitor evidence proves that its known regressions are preferable to the live incident it would contain. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8006cf0bcf7f. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
Summary
Draft emergency rollback for #840, merged as
8006cf0bcf7ff810761c11a3cf0609b33b03475b.Purpose and use
This is deliberately a draft. Do not merge it unless the post-merge monitor establishes that #840 caused a material live regression and a rollback is the right containment action. If the monitor remains healthy, close this draft without merging it.
The commit reverts #840 only and restores the exact pre-#840 tree. Consequently it also restores the prior behaviour for:
Those are intentional rollback trade-offs, not new design changes in this PR.
Validation
node --test test/dashboard-worker.test.ts— 188 passednode --test test/exact-review-publication-batches.test.ts— 44 passedpnpm run build:dashboard— passedgit diff --cached --check— passed before commitcodex review --uncommitted— reviewed the two intentional restored behaviours above; no unrelated change acceptedgit fetch origin main --prune && codex review --base origin/main— clean: exact rollback to the pre-fix(exact-review): handle terminal admissions and tuple failures #840 tree, no unintended deviations; whitespace-cleanThe current formatter would reformat the restored pre-#840 files. That churn is intentionally excluded so this remains a mechanical, reversible rollback.
Risk / rollout
Merging this draft would remove the safeguards introduced by #840. Keep it ready as a rapid containment option only; use the live monitor and maintainer judgment before taking that action.