How to review Codex pull requests: evidence, scope and the merge decision

Why does a Codex pull request need its own review routine?

Because the volume changes. Once Codex runs features in slices, pull requests arrive faster than a team used to reading them, and the review that worked for a colleague's two pull requests a week does not stretch to ten. OpenAI's Codex best practices describe their own end state plainly: "At OpenAI, Codex reviews 100% of PRs." That figure describes OpenAI's internal use, where Codex is the reviewer, not the author, and it says nothing about how often Codex reviews are right. What it does show is that the vendor treats automated review as routine, which makes the human part of review the scarce resource worth spending well.

The routine below spends it in three passes: evidence, scope, then code. Most bad Codex pull requests fail the first two, which take minutes, before anyone has to read a line of code.

How do you set Codex up to open pull requests instead of merging?

Two layers, one that asks and one that enforces. The asking layer is a line in AGENTS.md, from the AGENTS.md guide: "Work on a branch and open a draft pull request. Never merge." Codex reads that before every task, and in the ChatGPT desktop app the built-in Git controls let you commit, push a branch and create a pull request without leaving the app, so the pull request is the natural end of a run.

A line in AGENTS.md is followed, not enforced. The enforcing layer lives on GitHub. The GitHub documentation on protected branches states that with required reviews on, "collaborators can only push changes to a protected branch via a pull request that is approved by the required number of reviewers with write permissions." Turn that on for main, and turn on the option to dismiss stale approvals, which GitHub describes as clearing an approval when new commits change the diff. That second setting matters with Codex, because a Codex fix pushed after your approval would otherwise ride in on it.

OpenAI's own GitHub review page draws the same line from the other side: "Code review rules guide Codex; they don't replace tests, branch protections, or required approvals."

What must the pull request description carry?

Everything a reviewer needs to check the work without rerunning it. For the invoice CSV export built in the Codex feature build loop, the description Codex was asked to produce looked like this:

## What and why
Finance users can export the invoices they are viewing as CSV, with the same
filters and sort. Spec: docs/specs/invoice-csv-export.md

## Files changed
- apps/api/src/routes/invoices.ts: new GET /invoices/export.csv route
- apps/api/src/lib/csv.ts: CSV writer, formats integer cents as 1234.50
- apps/api/test/invoices-export.test.ts: filter parity, empty result, 413 limit
- apps/web/app/invoices/page.tsx: renders the export button
- apps/web/app/invoices/ExportButton.tsx: calls the endpoint with current filters
- apps/web/app/invoices/ExportButton.test.tsx: passes filters, shows 413 message

## Not touched
packages/db, packages/ui, billing, auth. No new dependencies.

## Evidence
pnpm lint: 0 problems
pnpm typecheck: 0 errors
pnpm test --filter api: 48 passed
pnpm test --filter web: 112 passed

## Verify by hand
On /invoices, filter to Overdue, sort by due date, click Export CSV.
The file's rows match the screen. Filter to a customer with no invoices:
the file has a header row only.

## Risk tier
Medium: user-facing, reversible, reads invoice data, writes nothing.

The two sections that do the most work are "Files changed" and "Not touched". Together they make a claim about scope that the diff can prove or disprove in under a minute. "Evidence" holds command output with numbers in it rather than a sentence saying checks pass, and "Risk tier" is the author's own estimate of blast radius, which the reviewer can disagree with out loud.

How deep should the review go?

Set depth by what breaks if the change is wrong, not by how long the diff is. A one-line change to a tax rule deserves more scrutiny than three hundred lines of a new internal page. Map the tiers to paths in your repository so the tier is a lookup, not a judgment made after you have already been persuaded by a tidy description.

Tier	Paths in the example repo	Minimum review before merge
Low	docs/, copy strings, internal-only pages	Evidence check, scope check, skim the diff
Medium	apps/web, apps/api routes that read data	Evidence and scope check, run the change on a preview, read every test that was added or changed
High	packages/db, packages/ui, billing, auth, permissions	All of the above, an engineer reads every line, a named approver, a written rollback step

A pull request takes the tier of its riskiest file. The CSV export claims medium, which is right for the six files it lists. Whether it stays medium depends on whether those six files are all it touches.

A worked review: the file nobody listed

Pass one, evidence: the description has real command output with pass counts, a spec link and manual steps. Pass two, scope: compare the file list with the diff before opening any file.

$ git diff --stat main...feat/invoice-csv-export
 apps/api/src/lib/csv.ts                          |  31 +++
 apps/api/src/routes/invoices.ts                  |  42 ++++
 apps/api/test/invoices-export.test.ts            |  88 +++++++
 apps/web/app/invoices/ExportButton.test.tsx      |  40 +++
 apps/web/app/invoices/ExportButton.tsx           |  36 +++
 apps/web/app/invoices/page.tsx                   |   4 +-
 packages/ui/src/format/currency.ts               |   6 +-
 packages/ui/src/format/__snapshots__/currency.snap |  12 +-
 8 files changed, 251 insertions(+), 8 deletions(-)

Six files in the description, eight in the diff, and both extras sit in packages/ui, a path the description lists under "Not touched" and AGENTS.md puts on the stop-and-ask list. Opening currency.ts shows why: formatCurrency gained a plain option that drops the currency symbol, and its default was flipped to true, so the CSV would hold bare numbers. The snapshot file was regenerated to match, which is why every test passed. The change happened in the last step of the run, when the fix prompt after /review asked Codex to "make amounts in the export plain numbers" without naming which files it could touch.

The tests were green and the feature worked. The pull request still could not merge, because formatCurrency renders every amount on every screen and on the PDF invoices customers receive, and with the new default all of them would lose their currency symbol. The pull request jumps from medium to high the moment that file appears, and the author's own risk tier is now wrong.

The review comment states the finding, the impact and the safe path, so the fix needs no back and forth:

Request changes.
packages/ui/src/format/currency.ts and its snapshot are changed but not listed
in the description, and packages/ui is on the AGENTS.md stop list. Flipping the
plain default removes the currency symbol from every amount in the app and in
invoice PDFs. The snapshot was regenerated, so tests do not catch it.
Fix: revert both packages/ui files. Format plain amounts inside
apps/api/src/lib/csv.ts, which already owns cents-to-decimal conversion.
Update the description and rerun the evidence.

The fix prompt to Codex repeats the fence that was missing the first time: revert the two packages/ui files, change only apps/api/src/lib/csv.ts and its test, paste the checks. The second diff stat showed six files, matching the description, and the review continued to pass three, reading the code at medium depth.

If the repository's AGENTS.md carries the ## Code Review Rules section from the AGENTS.md guide, asking for @codex review on the pull request adds a second check aimed at the same failure. OpenAI notes that in GitHub, "Codex flags only P0 and P1 issues so review comments stay focused on high-priority risks", so it is a check for serious problems, and the diff stat comparison stays your job.

Can Codex help review its own pull request?

Yes, with the roles kept separate. In the ChatGPT desktop app, the Code Review view shows a pull request's description, changed files, comments and checks, and the code review documentation suggests asking for evidence rather than a verdict, for example: "Show me the code path and tests for the no-matches case. Explain what is covered and what still needs verification." That is a good prompt for pass three on a large diff. The same page is clear about what a chat review does not do: "Reviewing a pull request in chat doesn't post comments, approve it, or merge it."

The counterargument deserves a straight answer: if Codex wrote the change and Codex reviews it, is the review worth anything? For mechanical problems such as a missing null check or an unhandled error path, a fresh review chat that did not write the code does catch things. For scope and product judgment, such as whether a shared formatter should change at all, the reviewer has to be someone who knows what the change was supposed to be. Keep that decision with a person.

What does the merge decision come down to?

Five lines, checked in order, with the first failure stopping the review:

1. Evidence: real command output with counts, a spec link, manual steps.
2. Scope: the description's file list matches git diff --stat exactly.
3. Tier: the riskiest file sets the tier, and the review depth matches it.
4. Tests: every added or changed test asserts behaviour, and no snapshot
   was regenerated without a reason in the description.
5. Rollback: for high tier, a named approver and a written way back.

Line four is the one Codex runs make necessary. An agent asked to make tests pass will sometimes change the test, and a regenerated snapshot is the quietest form of that. The method behind all five lines is tool-agnostic and covered in more depth in how to verify AI-generated code before merge; to stop typing the description by hand, turn the template above into a Codex skill that refuses to write it without the diff stat and the check output.

See the Building with Codex bootcamp for the session plan and the next cohort dates.