ci: review pull requests with Codex - #599
Conversation
Runs `codex exec` on every same-repo pull request and turns its findings into inline review comments plus one sticky summary comment. The agent holds the API key and reads untrusted PR content, so it gets no write permission and no network. The sandbox is workspace-write with networking off, the checkout keeps no credentials, and the PR metadata, diff, linked issues, previous summary and previous review threads are written to `.codex-review/` before it starts. It returns JSON against `.github/codex-review-schema.json`, and a second job with `pull-requests: write` validates that and posts it. Each finding carries a stable `key` that the bot embeds in the comment it posts. The next run reads those keys back, so an issue is commented on once instead of on every push, a thread closes when its issue is gone, and a thread the author resolved stays resolved. Review criteria come from `.claude/skills/review/SKILL.md`, so the bot and a local `/review` judge a pull request by the same rules. Needs the `OPENAI_API_KEY` repository secret. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first run of this workflow showed the step running, and reporting success, on a failure that happened before the context directory existed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces an automated Codex-based pull request review workflow that (1) collects PR metadata/diff/previous bot threads into a local context directory, (2) runs Codex to produce JSON findings, and (3) posts those findings back to the PR as inline comments plus an upserted sticky summary comment.
Changes:
- Add a two-job GitHub Actions workflow to run Codex review and then post results with constrained permissions.
- Add a Python implementation for context collection, diff line anchoring, deduplication, and comment/thread management, plus offline tests.
- Add the Codex prompt and JSON schema, and ignore the generated context directory in git.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| .gitignore | Ignore the local .codex-review/ context directory produced by the workflow. |
| .github/workflows/codex-review.yml | New CI workflow to run Codex review and post results (two-job security split). |
| .github/scripts/test_codex_review.py | Offline tests for diff parsing, anchoring, validation, and posting decisions. |
| .github/scripts/codex_review.py | Core logic to build context, parse/post findings, manage threads, and upsert summary. |
| .github/codex-review-schema.json | JSON schema defining the expected agent output contract. |
| .github/codex-review-prompt.md | Prompt instructing Codex how to review and how to emit schema-compliant JSON. |
Suppressed comments (1)
.github/scripts/codex_review.py:109
review_threads()fetches onlycomments(first: 50). If a thread grows beyond 50 comments, later author replies won’t be included inprior_threads.json, which undermines the “read every reply and accept dismissals” rule and can cause the bot to repeat findings that were already addressed/dismissed. Bump this to 100 (GraphQL max) or add pagination for comments if you expect long threads.
comments(first: 50) {
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| reviewThreads(first: 100) { | ||
| nodes { |
There was a problem hiding this comment.
Fixed in fc31c2b. reviewThreads now pages through pageInfo { hasNextPage endCursor } until it is done, bounded by MAX_THREAD_PAGES as a loop backstop. This one mattered: a missed thread means a comment posted twice, or a thread left open on an issue that is already gone. Covered by a test that stubs graphql with two pages and asserts both are collected and that the second request carries the first page cursor.
Left comments(first: 50) unpaged. It only feeds the author replies back to the agent, and 50 comments on a single review thread is not a case worth code.
| try: | ||
| cleaned = { | ||
| "key": str(finding["key"]).strip(), | ||
| "severity": str(finding["severity"]).strip().lower(), | ||
| "path": str(finding["path"]).strip().lstrip("/"), | ||
| "line": int(finding["line"]), | ||
| "title": str(finding["title"]).strip(), | ||
| "body": str(finding["body"]), | ||
| "dismissed_but_real": bool(finding.get("dismissed_but_real")), | ||
| } |
There was a problem hiding this comment.
Fixed in fc31c2b. A non-boolean now warns and falls back to false, so the finding still gets its inline comment. Falling back to false rather than dropping the finding is deliberate: a comment that should have been suppressed is visible and can be replied to, while a silently swallowed finding is not. Tested against "false", "true", 0, 1, null and "yes".
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fb0370e. Configure here.
Restrict both trigger paths to same-repo pull requests. Both jobs run scripts out of the PR head, one with the API key in scope and one with `pull-requests: write`, so the branch has to come from someone who already has push access. `workflow_dispatch` accepts a PR number, which can name a fork PR, so the check sits in the first step as well as in the job's `if`. Page through `reviewThreads`. One page holds 100, and a thread that is missed is a comment posted twice or a thread left open on an issue that is gone. Drop a `suggestion` block when the comment is not on the line the finding named. GitHub applies a suggestion to the line the comment sits on, so on a snapped anchor a one-click apply would replace the wrong code. The replacement code stays, as a plain block, and the comment says which line the finding named. Require `dismissed_but_real` to be a real boolean. Truthiness would read the string "false" as true and silently swallow the inline comment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
Runs
codex execon every same-repo pull request and turns its findings into inline reviewcomments plus one sticky summary comment, modelled on the
Code Reviewjob inClickHouse/ClickHouse(ci/jobs/copilot_review_job.py).Review criteria come from
.claude/skills/review/SKILL.md, so the bot and a local/reviewjudgea pull request by the same rules. The prompt overrides that file's
LOCAL VALIDATIONandREQUESTED OUTPUT FORMATsections, which do not apply to a sandboxed run that returns JSON.How it is put together
Two jobs, which is the security model:
reviewwrites the PR metadata, diff, linked issue bodies, previous summary and previousreview threads into
.codex-review/, then runs the agent. It holds the API key and readsuntrusted PR content, so it gets
contents: readandpull-requests: readand nothing else:the sandbox is workspace-write with networking off, and the checkout keeps no credentials. An
injected instruction in a PR body has nothing to reach. The agent returns JSON against
.github/codex-review-schema.json.postholdspull-requests: writebut runs no model. It validates the JSON, anchors eachfinding to a line that is really in the diff, and posts.
Findings are deduplicated across pushes. Each one carries a stable
keythat the bot embeds inthe thread it opens; the next run reads those keys back, so an issue is commented on once instead
of on every push, a thread resolves when its issue is gone, and a thread the author resolved stays
resolved. Only threads the bot resolved itself are ever re-opened.
A finding whose line is a few lines off the diff is snapped onto the nearest commentable line. One
that cannot be anchored at all, or that GitHub rejects, is listed in the summary instead of being
dropped. Inline comments are capped at 20 per run, and anything over the cap is named in the
summary.
Scope limits
pull_requestgives neither anysecrets. 39 of the last 40 pull requests here were same-repo.
workflow_dispatchtakes a PRnumber for the exceptions.
The jobs that own those report them with full output, so a review comment is noise.
github-actions[bot].Before it works
This needs the
OPENAI_API_KEYrepository secret. The workflow fails with an explicit message ifit is missing, rather than a confusing one from the CLI.
Testing
.github/scripts/test_codex_review.pycovers diff parsing, line anchoring, agent-outputvalidation, and the full posting decision table with every mutation stubbed. The review job runs
it before the model call, so a broken script costs a second instead of a review.
Checked against live data as well: the diff parser's line arithmetic was verified on #584 by
confirming that all 220 line positions it reports as commentable match the file contents at that
PR's head; context collection ran end to end against #584; and the review-thread query was checked
against #581's threads.
The model call itself has not run yet, so this pull request is its first end-to-end test.
Checklist