Skip to content

fix: separate advisory review from required gate#92

Merged
Pigbibi merged 2 commits into
mainfrom
codex/review-gate-authority-split-20260717
Jul 16, 2026
Merged

fix: separate advisory review from required gate#92
Pigbibi merged 2 commits into
mainfrom
codex/review-gate-authority-split-20260717

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • keep Codex Review Gate / gate deterministic and blocking for static secret/path/metadata checks only
  • run the required gate from trusted base code on pull_request_target; GitHub API failures fail closed
  • report official connector reviews through a separate current-head, non-blocking, event-driven Codex Review Advisory / advisory check
  • remove connector polling and prevent advisory events from overwriting the required gate context

Scope

  • workflow authority split only
  • no prompt/evidence parser, event persistence, automation, deployment, permissions widening, or trading changes

Verification

  • python3 -m pytest -q tests/test_review_authority_split.py tests/test_gate_codex_app_review.py → 9 passed
  • python3 -m pytest -q → 653 passed, 1 skipped, 50 subtests
  • python3 -m compileall -q scripts tests
  • actionlint .github/workflows/codex_review_gate.yml .github/workflows/codex_review_advisory.yml
  • git diff --check
  • ruff unavailable locally

Review contract

One concentrated closure maximum. Do not widen into prompt semantics or a machine evidence parser.

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

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown

🤖 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/gate_codex_app_review.py

The workflow is supposed to preserve the required Codex Review Gate / gate context, but the new explicit head check is created with the name Codex Review Gate. Required checks are matched by exact context name, so this change will stop updating the existing required status on PR heads and can leave merges permanently blocked waiting for Codex Review Gate / gate (or make the new check non-required if rules are not migrated perfectly). (line 34)

Suggestion: Publish the head check under the exact existing required context name (Codex Review Gate / gate), or otherwise keep reporting that exact context until branch protection has been migrated in lockstep.

2. 🟠 [HIGH] Reliability in scripts/gate_codex_app_review.py

run_static_guard() still downloads pull_request.diff_url with a plain urlopen() request and no authentication. On private repositories and many GitHub Enterprise setups that URL is not anonymously readable, so this PR's new fail-closed behavior will turn every gate run into Static guard unavailable and block all merges. (line 154)

Suggestion: Fetch the diff through an authenticated GitHub API request instead of the public diff_url page, or at minimum attach the repository token to the diff request and avoid endpoints that rely on anonymous access.


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: 94f7260a2d

ℹ️ 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 .github/workflows/codex_review_gate.yml
Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi
Pigbibi merged commit a59727b into main Jul 16, 2026
3 of 4 checks passed
@Pigbibi
Pigbibi deleted the codex/review-gate-authority-split-20260717 branch July 16, 2026 19:20
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