fix(review): harden bounded review-state transport

[Pigbibi]

Summary

• add a bounded, canonical v1 review-state block with fully anchored machine records
• sanitize model prose and fail closed on malformed, oversized, or excessive findings while still publishing comment/artifact/outputs
• preserve trusted history/fingerprint/arbitration behavior, with invalid-state recovery only on a verified new PR head
• keep legacy trusted comments bounded and fail closed without assigning identity to invalid findings

Scope

G1c-A review-state transport safety only. No finding IDs, dispositions, semantic matching, policy-limit changes, or new production dependencies.

Verification

• five focused review-state regressions: 5 passed
• python3 -m unittest tests.test_run_codex_pr_review: 62 passed
• python3 -m pytest tests ops/quant-monitor/tests -q: 660 passed, 1 skipped, 52 subtests passed
• python3 -m ruff check --no-cache .
• python3 -m compileall -q service scripts tests ops/quant-monitor
• actionlint .github/workflows/*.yml
• Cloudflare Node tests: 13 passed + 15 passed
• git diff --check
• static size: 1100 additions + 80 deletions = 1180 (<1700)

[github-actions]

🤖 Codex PR Review

🚫 Merge blocked: 1 serious issue(s) found in high-risk files

⚖️ Codex Review Arbitration

🚫 block: The finding remains valid. In the backend-failure path the code now calls publish_review_decision() with blocking_streak=previous_streak, finding_fingerprint=previous_fingerprint, finding_fingerprints=previous_fingerprints, and preserved finding_history, but also with reviewed_head_sha=current_head_sha (scripts/run_codex_pr_review.py around lines 2669-2677). The invalid-output path repeats the same pattern around lines 2708-2716, and the validated-findings failure path does so again just below. Because review_state_valid is left true, publish_review_decision() serializes a normal v1 review-state record, so parse_review_state() on the next run will trust that stale blocking identity as belonging to the new head. Then main() derives previous_head_sha from that stored state and computes new_head by comparing it to the current PR head; after the first failed attempt on a new commit, the retry sees previous_head_sha == current_head_sha and therefore new_head=False, even though no successful review ever completed for that head. That lets old blocking fingerprints/streak/history participate in repeated-finding and arbitration logic as if they had already been reconfirmed on the new commit. This does not conflict with the prior finding: preserving prior blocking history on failure is still required, but advancing reviewed_head_sha on failure is a separate regression.

🚫 Blocking Issues

These issues must be fixed before this PR can be merged:

1. 🟠 [HIGH] Logic in scripts/run_codex_pr_review.py

The backend-failure path now republishes prior blocking fingerprints/history with reviewed_head_sha=current_head_sha even though no successful review completed for that head. When a PR gets a new commit and the first attempt on that new head fails (same bug is repeated in the invalid-output branch below), the stale identity from the previous head is treated as if it already belongs to the new SHA; the next retry on the same head will run with new_head=False, so repeated-finding/arbitration logic can escalate old blockers as though they were reconfirmed on the new commit. (line 2678)

Suggestion: On review/parse failure paths, keep reviewed_head_sha pinned to the last successfully reviewed head (or empty if none) and only advance it to current_head_sha after a successful review result has been parsed and published.

ℹ️ Other Findings

1. 🟡 [MEDIUM] Logic in scripts/run_codex_pr_review.py

When build_review_state_marker() fails and returns an invalid sentinel, publish_review_decision() flips the machine state to blocked but still renders the original published_decision summary/findings into the PR comment. This is reachable during legacy migration because _parse_legacy_review_state() accepts histories that can exceed the new REVIEW_STATE_MAX_FINDINGS cap, causing canonical-state serialization to fail; the workflow output then says the review is blocked while the visible PR comment can still say the review is clear or show stale findings. (line 2276)

Suggestion: If actual_state_valid is false, replace the rendered decision with an explicit review-state failure decision before calling build_pr_comment(), instead of only mutating flags in decision_payload.

---

Review by Codex PR Review bot • PR

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 512b4bde1f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ccefc9e566

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".