Make Codex review dispositions deterministic

[Pigbibi]

Summary

• compute review_finding_id.v1 only after exact head, right-side patch line, repository source, category/severity, evidence, description, and impact validation
• preserve exact active finding IDs until a fully anchored review_finding_disposition.v1 record is read from authenticated repository review metadata
• isolate bounded v2 machine state from model prose and remove model-issued identity, provenance, and reachability authority
• fail closed on malformed history, line maps, evidence, identity, authority metadata, or dispositions

Tests

• seven locked characterization regressions: RED on base, then 7 passed
• python3 -m unittest -q tests.test_run_codex_pr_review — 70 passed
• python3 -m pytest tests ops/quant-monitor/tests -q — 668 passed, 1 skipped, 54 subtests passed
• python3 -m ruff check .
• python3 -m compileall -q service scripts
• actionlint
• both Cloudflare Node test suites — 28 passed
• git diff --check

Compatibility / risk

• legacy active history without exact bridge-issued identity is intentionally fail-closed and requires contract arbitration
• deterministic caller/boundary validation is currently Python-aware; unverifiable evidence for other source types remains blocked without automatic remediation

[github-actions]

🤖 Codex PR Review

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

🚫 Blocking Issues

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

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

_canonical_finding_evidence() treats any non-def source line containing symbol( as a valid current_caller. In the current main() -> evaluate_findings() -> _validated_blocking_finding() path, model-supplied evidence can therefore point at a comment or string literal such as # changed( or logger.info("changed(") and still pass deterministic validation, which lets a non-reachable blocker acquire a stable review_finding_id and keep the merge blocked until someone files an authenticated disposition. (line 1143)

Suggestion: Validate current_caller evidence from parsed syntax instead of a regex on one line: require an actual call expression to the reported symbol inside executable production code, and reject comments/strings entirely.

2. 🔴 [CRITICAL] Reliability in scripts/run_codex_pr_review.py

build_review_state_marker() raises ReviewError when there are more than 32 active findings or when the encoded state exceeds 16 KB, but the main() -> publish_review_decision() path does not catch that failure. Because active findings are built from untrusted model output plus preserved history, a PR with many high/critical findings or long descriptions can crash the production review job after the decision has already been computed, leaving no comment/state update behind. (line 1608)

Suggestion: Make state serialization non-throwing in the publish path: either cap/truncate persisted findings before encoding, or catch ReviewError around marker construction and emit a fail-closed comment/output instead of aborting the process.

ℹ️ Other Findings

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

When fetch_pr_diff(..., current_head_sha) detects that the PR head changed, main() writes blocked step outputs and returns immediately without calling publish_review_decision(). This is reachable whenever a push lands between the event snapshot and the diff fetch, and it leaves the previously trusted PR comment untouched, so the visible review comment can still show stale allow/block state for the wrong head while automation is already failing closed. (line 2753)

Suggestion: On head-SHA validation failure, publish an explicit invalid/blocked review-state comment (or remove the stale trusted comment) before returning so the PR comment, artifact, and step outputs stay consistent.

---

Review by Codex PR Review bot • PR

[chatgpt-codex-connector]

💡 Codex Review

AIAuditBridge/scripts/run_codex_pr_review.py

Lines 2192 to 2196 in 5f3f01b

	lines = [
	f"#### {index}. {emoji} [{severity.upper()}] {category.title()} in `{file_path}`",
	"",
	f"> {description}",
	]

Publish the exact disposition handle for active findings

When next_action becomes authenticated_disposition, the only trusted clear path is an anchored record containing the exact review_finding_id and head SHA, but the PR comment formats active findings without either those values or any marker/instructions. In a false-positive or stale-active-finding case, maintainers cannot produce the required disposition from the normal review output, so the gate can remain blocked unless someone manually decodes the hidden machine state.

ℹ️ 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]

Surface invalid blocking findings in comments

When a high/critical finding fails deterministic validation (for example because the model reports a removed line, missing evidence, or a non-Python boundary), this path appends the sanitized finding only to invalid_findings. The decision still blocks the merge, but build_pr_comment renders only blocking_findings and non_blocking_findings, so authors see a generic metadata-validation failure with no reported finding or validation_error to act on. Include these invalid findings in a rendered blocking section or otherwise surface their sanitized details.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Allow valid short callable names in evidence

This pattern requires at least three identifier characters, so legitimate Python callables named f, go, or up are rejected before an active finding can be computed. For high/critical findings inside such functions, otherwise valid evidence becomes invalid metadata and forces a contract-arbitration block instead of a normal dispositionable finding; the regex should allow Python's valid one-character identifiers.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Support blocker evidence outside Python files

For a high/critical finding in a workflow, shell script, JS/TS file, Dockerfile, or other non-Python file, _python_enclosing_callable() returns None here, so _canonical_finding_evidence() rejects the finding before it can receive a deterministic ID. The gate still fails closed, but the finding is treated as invalid metadata and cannot use the new authenticated-disposition flow even though this review workflow is used on non-Python paths such as .github/workflows/* and scripts.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Provide a recovery path for invalid review state

Once the bot publishes a v2 state with status: invalid (for example after malformed legacy history or a state serialization failure), every later run hits this check and blocks before fetching the diff or running review. Because invalid states contain no active finding IDs to disposition, a normal follow-up commit or contract/documentation fix cannot clear the gate; it requires out-of-band comment deletion/editing despite next_action=contract_arbitration.

Useful? React with 👍 / 👎.