Skip to content

fix(review): scope decisions to exact head#97

Merged
Pigbibi merged 1 commit into
mainfrom
codex/g1d-simplified-exact-head-20260718
Jul 17, 2026
Merged

fix(review): scope decisions to exact head#97
Pigbibi merged 1 commit into
mainfrom
codex/g1d-simplified-exact-head-20260718

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • remove cross-head finding history, streak, fingerprint, and arbitration state
  • publish only the current exact-head result and immutable implementation identity
  • treat backend and parse failures as external retryable failures without marking the head reviewed

Tests

  • RED: 4 focused exact-head/retry regressions failed on the base implementation
  • python3 -m unittest -q tests.test_run_codex_pr_review
  • python3 -m pytest tests ops/quant-monitor/tests -q (636 passed, 1 skipped, 50 subtests)
  • python3 -m ruff check .
  • python3 -m compileall -q client scripts service tests ops/quant-monitor
  • actionlint -config-file .github/actionlint.yaml
  • Node worker/dashboard tests (28 passed)
  • local static gate and git diff --check
  • size: 1992 changed lines (limit 2000)

Co-Authored-By: Codex <noreply@openai.com>
@github-actions

Copy link
Copy Markdown

🤖 Codex PR Review

🚫 Merge blocked: 1 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

publish_retryable_review_failure constructs the decision payload with blocked=False and blocking_findings=[], then calls publish_review_decision with exit_code=1. Any downstream workflow step or artifact consumer that reads decision.json or the blocked GitHub step output will see blocked=false and zero blocking findings, and may proceed to merge. The review_completed=False flag is intended to signal this state, but existing workflow consumers that key on blocked (the primary gate output, written by write_decision_outputs) will not observe the failure. The call path is directly reachable from main() for every ReviewError raised by the backend or parser. (line 1100)

Suggestion: Set blocked=True in the retryable-failure decision payload so the primary blocked step output correctly prevents merge when the review did not complete. Alternatively, document and enforce that all workflow gates must check review_completed before inspecting blocked, and add a test asserting decision['blocked'] is True on retryable failure.

ℹ️ Other Findings

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

After this PR removes all use of the prior review comment (streak, fingerprint, history), find_existing_review_comment is still called unconditionally in main() (the call is retained; only the parsing of its return value was removed). This performs an authenticated GitHub API request whose result previous_comment is immediately discarded. The call can raise ReviewError, which under the old code was caught and handled — but with the new code the exception would propagate uncaught out of main(), producing an unhandled exception and a non-zero exit without posting a PR comment or writing decision outputs. (line 1166)

Suggestion: Remove the find_existing_review_comment call entirely from main() since its return value is no longer consumed. The upsert_pr_comment path already handles comment creation vs. update internally.


Review by Codex PR Review bot • PR

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1ead7c9472

ℹ️ 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".

Comment thread scripts/run_codex_pr_review.py
@Pigbibi
Pigbibi merged commit 86458c4 into main Jul 17, 2026
6 of 7 checks passed
@Pigbibi
Pigbibi deleted the codex/g1d-simplified-exact-head-20260718 branch July 17, 2026 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant