Skip to content

[REA-3711] Review pull requests with Codex - #93

Open
aviksethia99 wants to merge 2 commits into
mainfrom
avik/REA-3711_codex-review
Open

[REA-3711] Review pull requests with Codex#93
aviksethia99 wants to merge 2 commits into
mainfrom
avik/REA-3711_codex-review

Conversation

@aviksethia99

@aviksethia99 aviksethia99 commented Jul 30, 2026

Copy link
Copy Markdown

Why

Automated review on this repository ran on Claude Code. Standardising on a
single automated reviewer means moving that job to Codex, and this repository
is public, so the review needs to hold a pull request to the standards in
AGENTS.md that ruff and ty cannot check — above all the rule against
assuming infrastructure this package does not ship.

Review also costs money to run, so it stays gated to maintainers rather than
firing for anyone who can open a pull request or leave a comment.

What Changed

openai/codex-action now performs the review, driven by one workflow that
keeps the trigger gate, the model, and the prompt in the same file. Review
runs when an owner, member, or collaborator opens a non-draft pull request,
and on demand when one of them comments /review; workflow_dispatch takes a
pull request number for a manual run. An outside contributor's pull request
never reaches a runner, so untrusted code is never checked out and no review
budget is spent on it. The action independently verifies that the actor has
write access, which leaves the trigger gate as the cheap first barrier rather
than the only one.

The prompt is the one this repository already used, carried over intact: the
package overview, the instruction to read AGENTS.md and hold the pull
request to it, and the focus areas — including references to infrastructure
this package does not ship, tests missing for changed behaviour, and typing
and docstring contract violations. The only substantive change is mechanical:
inline comments are posted with gh api rather than through a
Claude-specific tool. Findings land on the offending lines, and a review that
finds nothing posts a single lgtm comment.

[skip-codex] in a pull request title skips the review, and Dependabot
pull requests are excluded.

Verification

actionlint is clean. The job needs an OPENAI_API_KEY Actions secret
available to this repository; until it is, the review fails and reports
nothing, which leaves merges unaffected because it is not a required check.
Once the secret is in place, comment /review on an open pull request and
confirm the inline comments and the summary.

@aviksethia99
aviksethia99 requested a review from a team as a code owner July 30, 2026 00:36
@linear

linear Bot commented Jul 30, 2026

Copy link
Copy Markdown
REA-3711 Remove Claude code review; standardize on Codex review for all PRs

Goal

Stop running Claude Code review on PRs and use Codex review as the single PR code-review tool across all repos.

Approach

Centralize one reusable Codex review workflow in reactor-team/actions (codex-review.yml, pinned to the best available model) and have every repo call it via a ~10-line caller stub. One place to bump model/prompt/skip semantics.

Scope (verified against remotes 2026-07-29)

1. actions — add reusable codex-review.yml + its own caller stub (1 PR, merge first)

2. reactor — delete claude-code-review.yml, replace inline codex-review.yml with caller stub. Fixes current double-review on every PR. (1 PR)

3. Swap Claude → Codex (9 PRs): infrastructure, fluxcd, reactor-runtime, reactor-webrtc, reactor-models, reactor-proto, reactor-sdks, reactor-webapp, cpp_sdk — delete Claude workflow, add caller stub, update CONTRIBUTING/CLAUDE.md/AGENTS.md references in same PR.

4. Remove review entirely (2 PRs): conference-lingbot-demo, internal-tools — delete Claude workflow, add nothing.

5. Add Codex fresh (5 PRs): grafana-manifests, aws-deploy, ssosync, diagnostics, org-tooling.

6. Org settings (needs org admin): create org-level OPENAI_API_KEY Actions secret scoped to the 16 Codex repos; reactor drops its repo-level copy.

7. Secret cleanup: remove ANTHROPIC_API_KEY from repos that no longer use it. KEEP in reactor, js-sdk, js-sdk-internal, py-sdk — Claude docs-automation workflows (claude-docs-check, claude-public-docs-check, claude-docs-pr) are out of scope and keep running.

8. Per-repo check: confirm no branch protection/ruleset requires the Claude review check name (reactor verified clean — only buildkite/reactor required).

Out of scope

Claude docs automation (reactor docs checks, js-sdk/js-sdk-internal docs-pr, py-sdk public-docs-check). admin-webapp has no review workflow on remote (original list was stale). cvpr-lingbot-demo appears archived.

Why

Consolidate on one automated review tool (Codex) to avoid duplicate/competing review comments and standardize the PR workflow.

Review in Linear

@aviksethia99
aviksethia99 marked this pull request as draft July 30, 2026 00:40
@aviksethia99
aviksethia99 marked this pull request as ready for review July 30, 2026 05:34
@github-actions

Copy link
Copy Markdown

[codex-review] No issues found — this PR looks good.

@Dere-Wah
Dere-Wah force-pushed the avik/REA-3711_codex-review branch 2 times, most recently from b592fa8 to 344908d Compare July 30, 2026 09:15
@Dere-Wah Dere-Wah changed the title REA-3711 Swap Claude review for shared Codex review [REA-3711] Review pull requests with Codex Jul 30, 2026
@Dere-Wah
Dere-Wah force-pushed the avik/REA-3711_codex-review branch from 344908d to 8972918 Compare July 30, 2026 09:24
@aviksethia99
aviksethia99 force-pushed the avik/REA-3711_codex-review branch from 30e9177 to 28fc538 Compare July 30, 2026 21:33
@aviksethia99 aviksethia99 reopened this Jul 30, 2026
@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown

[codex-review] - [P2] .github/workflows/codex-review.yml: Public workflow exposes and depends on an internal repository reference.

Scope: full (70cfaae..28fc538).

View workflow run.

Comment thread .github/workflows/codex-review.yml Outdated
# open-source track), and GitHub does not allow public repos to call reusable
# workflows in internal repositories — so the full review lifecycle lives
# here instead of a thin caller. Mirrors
# reactor-team/actions/.github/workflows/codex-review.yml; sync manually when

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] [P2] This public workflow names an internal repository and tells maintainers to synchronize from it, which directly violates AGENTS.md's non-negotiable rule that private systems must not be named, linked, or assumed by public-repo content. Remove the internal repository reference and describe this workflow solely in terms of its public, self-contained contract.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed — header rewritten to describe only the public, self-contained contract, and the dead reactor-team/actions special-case in the concurrency guard is removed. No internal systems referenced anywhere in the file now.

@aviksethia99
aviksethia99 force-pushed the avik/REA-3711_codex-review branch from 28fc538 to 0a676a1 Compare July 30, 2026 21:54
Removes the Claude Code review workflow and adds the full Codex review
lifecycle in-repo (mirrors reactor-team/actions REA-4724 version).
Public/OSS repos cannot call reusable workflows in internal
repositories, so the logic is inlined rather than a thin caller. Only
external dependency: the org-level OPENAI_API_KEY secret, which fork
PRs never receive.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Avik Sethia <aviksethia99@gmail.com>
@aviksethia99
aviksethia99 force-pushed the avik/REA-3711_codex-review branch from 0a676a1 to ee34f08 Compare July 30, 2026 21:55

@tempusfrangit tempusfrangit left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am requesting changes here to hold. We need to be extra careful that this is only run on PRs we explicitly know are not going to try and maliciously hand things off to codex. This means we need to ensure that we only run from the workflow on main and likely only for trusted contributors.

This same comment applies to all public repositories. We do not want to leak our OpenAI key.

The review job checks out the pull request head and runs the agent over
it with the API key in scope. For a fork head that puts untrusted code
in a privileged context — reachable even without write access, since a
maintainer running /review on someone else's pull request is enough.

Guard on isCrossRepository from the API in the acknowledge job, so every
trigger (pull_request, issue_comment, workflow_dispatch) is covered
rather than just the event payloads that carry head repo information.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Avik Sethia <aviksethia99@gmail.com>
@aviksethia99

Copy link
Copy Markdown
Author

Added a fork guard (second commit): the acknowledge job now refuses when the PR head lives in a fork.

Why: the review job checks out the PR head and runs the agent over it with OPENAI_API_KEY and GH_TOKEN in scope. Fork PRs never get secrets on pull_request events, but the issue_comment path runs from the base repo with secrets — so a maintainer typing /review on an outside contributor's fork PR would execute untrusted content in a privileged context. That's reachable without any write access, which made it a real escalation path on a public repo.

Implementation: guards on isCrossRepository from the API inside acknowledge, so all three triggers (pull_request, issue_comment, workflow_dispatch) are covered rather than only the events that carry head-repo info in their payload. The error message tells the maintainer to review the fork PR manually.

Deliberately unchanged: sandbox: danger-full-access and allow-bots: true stay as configured upstream — the agent needs network for the API and for posting comments, so sandboxing would restrict writes without preventing exfiltration; the fork guard is what actually closes the hole.

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.

3 participants