[ci] code review: add Codex review job triggered by codex-review label

[tiankongdeguiji]

Summary

Adds an OpenAI Codex PR-review path to code_review.yml, alongside the existing Claude flow: a collaborator adds the codex-review label and the same self-hosted runner runs a headless codex exec review, posting inline comments through the existing bun MCP server and a summary via gh pr comment.

Single source of truth for review content. No second set of review markdowns: scripts/ci/gen_codex_review.py converts the trusted base-branch .claude/agents/*.md into Codex subagent TOMLs (.codex/agents/<name>.toml, sandbox_mode = "read-only" mirroring the Claude agents' read-only tools) and turns .claude/commands/review-pr.md into the exec prompt at runtime. The one shared wording change makes the inline-comment instruction tool-neutral so both CLIs resolve their own MCP tool naming.

Design notes

• Same trusted-file pattern as the Claude job (GHSA-f9x3-9rgg-92p7): .claude/ + converter are checked out from base.sha; the PR's .claude/, .codex/, and AGENTS.md are dropped, and trusted CLAUDE.md is installed as AGENTS.md (Codex auto-reads it — supplies project context and blocks a PR-authored injection file).
• Sandbox is workspace-write with sandbox_workspace_write.network_access=true (gh needs network), never danger-full-access, so runner home / stored credentials stay unreadable to the model.
• MCP server config is passed per-run via -c overrides; env_vars forwards GITHUB_TOKEN/REPO_OWNER/REPO_NAME/PR_NUMBER to the server by name only — no secret values in argv or on-disk config.
• Auth is stored ChatGPT OAuth from a one-time codex login --device-auth on the runner (mirrors claude /login); no OPENAI_API_KEY secret.
• Concurrency groups now include the label name, so codex and claude reviews of the same PR no longer cancel each other (previously any label event shared one group).

Known limitations / follow-ups

• pull_request_target runs the base-branch workflow definition, so this job can only be exercised after merge: do the one-time runner setup in the yml header (install Codex CLI, codex login --device-auth), then add the codex-review label to a test PR. Requires a recent Codex CLI (subagents + mcp_servers.*.env_vars).
• The codex-review label must exist in the repo before first use.

Verification

• Converter run against the repo's real .claude/: 5 agent TOMLs generated, all parse with tomllib, prompt assembles with REPO/PR header.
• Workflow YAML parses; both jobs and label-scoped concurrency verified.
• pre-commit passes on all changed files.

🤖 Generated with Claude Code

https://claude.ai/code/session_012Lbq87BJ7AiDFqs7srcP6v

[github-actions]

Blocker: CLAUDE.md does not exist in this repo, so this job fails on every run.

The base branch root (origin/master) contains only .claude and README.md — there is no CLAUDE.md anywhere in the tree, and this PR doesn't add one. Since GitHub Actions run: steps execute under bash -eo pipefail, this cp exits 1 ("No such file or directory") and aborts the Use trusted agent files step before RunCodexReview is ever reached. The manual verification (converter run against real .claude/) can't catch this because the cp is a separate shell step.

Options: add a real CLAUDE.md at repo root (cone-mode sparse-checkout already pulls top-level files), or have gen_codex_review.py emit AGENTS.md too, or cp ... || true if it's optional.

Note this also contradicts the header comment (lines 12-13) "Review instructions live only in .claude/" — that's true today, but this line installs an extra top-level instruction file from a nonexistent CLAUDE.md.

[github-actions]

Security: the trusted-files defense only strips root-level agent files. This removes pr-code/.claude, pr-code/.codex, and the root pr-code/AGENTS.md, but Codex discovers AGENTS.md hierarchically — a nested file like tzrec/.../AGENTS.md in the PR tree is still read as trusted developer instructions when the reviewer touches that subtree, bypassing the GHSA-f9x3-9rgg-92p7 regeneration this step is meant to enforce. Since the main codex exec runs --sandbox workspace-write with network_access=true (and workspace-write still permits filesystem-wide reads), an injected instruction could read the runner's persistent OAuth store (~/.codex/auth.json, ~/.claude/.credentials.json, per the setup notes above) and exfiltrate it.

Cheap hardening: strip tree-wide before regenerating, e.g.

find pr-code -name AGENTS.md -delete
find pr-code -type d \( -name .codex -o -name .claude \) -prune -exec rm -rf {} +

(delete-then-recreate to keep the existing nesting protection).

[github-actions]

Minor: the agent bodies contain non-ASCII (e.g. the em-dash — in security-code-reviewer.md / performance-reviewer.md), so read_text()/write_text() here and at lines 63/83/85 should pass encoding="utf-8" — otherwise they inherit the platform-default locale and can raise UnicodeDecodeError on a non-UTF-8 runner. Also agents_dir.mkdir(parents=True) at line 78 will raise FileExistsError on a local rerun; exist_ok=True makes it idempotent (the CI rm -rf currently masks this).

[github-actions]

Code review summary

Nice, well-documented change — the trusted-file design is sound: converter + .claude/ are checked out from base.sha (not the PR), GITHUB_TOKEN is forwarded to the MCP server by name via env_vars rather than placed in argv/on-disk config, danger-full-access is correctly avoided, and keeping .claude/ as the single source of truth avoids a second set of review docs. Keeping the reworded create_inline_comment instruction tool-neutral works for both CLIs.

Must fix before this can run

• cp trusted-claude/CLAUDE.md → job fails every run: CLAUDE.md does not exist anywhere in the repo (base root has only .claude and README.md), so this cp aborts the job under bash -eo pipefail. See inline on line 192.

Worth addressing

• Injection defense is root-only: nested AGENTS.md in the PR tree survives the cleanup and Codex reads it hierarchically; with workspace-write + network_access=true that chains to reading the runner’s persistent OAuth credentials. Cheap tree-wide find-delete fix inline on line 185.
• encoding="utf-8" + mkdir(exist_ok=True) in gen_codex_review.py (inline).

Lower priority (not blocking)

• The review and codex-review jobs now duplicate label-removal, dual checkout, env block, and bun-path resolution; a composite action would prevent drift. The codex exec -c MCP overrides (nested-key flags with "\"${BUN_PATH}\"" quoting) are hard to read — emitting a config.toml would be cleaner.
• No test for the pure split_front_matter/gen_agent_toml (repo has no precedent for testing scripts/ci/, so low priority), but a tomllib round-trip assertion on the generated TOML would cheaply guard the "json.dumps escaping is valid for TOML" assumption and the after-merge-only path.
• split_front_matter text.split("---", 2) raises ValueError on a --- file with no closing fence, and sandbox_mode is hardcoded read-only without reading the agents’ tools field (correct today; would silently mismap a future write-capable agent).

The pull_request_target + untrusted-code exposure is gated by the write-access-only label (same as the Claude job) — acceptable, but it makes the label load-bearing, so prioritize the injection/credential-read hardening.

[tiankongdeguiji]

Follow-up bd6def9 after a full end-to-end run of the codex-review job against a real fork PR (the first live run failed at the trusted-file step): the setup step no longer references untracked CLAUDE.md (AGENTS.md injection is instead blocked wholesale via project_doc_max_bytes=0), and codex exec is hardened for headless CI — MCP tools pre-approved (exec mode otherwise auto-cancels them), GITHUB_TOKEN visible to gh (codex strips *TOKEN* env vars from shell commands by default), runner user's personal config/skills isolated via --ignore-user-config + HOME shim, reasoning effort pinned to medium, and the trusted-file swap committed in the workspace so the review sees only the PR's own diff.

Verified end-to-end on the review runner: 5 subagents spawned from the generated .codex/agents/, inline comment posted via the MCP server, and a summary comment via gh pr comment.

[tiankongdeguiji]

Review triage (65a4a19):

Fixed

• cp CLAUDE.md blocker — already removed in bd6def9 (it was the live CI failure); AGENTS.md injection is instead disabled wholesale via project_doc_max_bytes=0.
• encoding="utf-8" on all converter file I/O and mkdir(exist_ok=True) — both applied; converter re-verified under LC_ALL=C with a double-run and tomllib round-trip of all generated agent TOMLs.

Addressed differently

• Nested-AGENTS.md sweep: project_doc_max_bytes=0 (bd6def9) turns off the AGENTS.md mechanism entirely — root, nested, and user-global — so the tree-wide find-delete would be a redundant second layer. The remaining kernel (workspace-write confines writes, not reads, while gh needs network) can't be closed by config today; it is now documented in the header as part of the label-gated trust model, with guidance to keep the runner single-purpose and its credentials minimal.

Not taken

• Composite action / emitting config.toml: per-run -c overrides were chosen deliberately (no on-disk runner config to drift, secrets forwarded by name only); the two jobs share shape but differ enough that the indirection outweighs the dedup.
• Converter unit test: tzrec/tests/run.py only discovers tests under tzrec/, so a scripts/ci test would never execute in CI; the tomllib round-trip is performed as part of change verification instead.
• Unclosed front-matter fence / sandbox_mode hardcoding: converter inputs are trusted repo files, and read-only is the intended policy for review agents regardless of their Claude-side tools — a loud failure at CI setup is acceptable for malformed input.

[github-actions]

Bug: this aborts the claude-review job for any PR that doesn't modify instruction files. When the trusted-file swap is a no-op — the PR touches none of .claude/.codex/.agents/CLAUDE.md/AGENTS.md, and base==head for those — git add -A stages nothing and git commit exits 1 (nothing to commit, working tree clean). Under set -euo pipefail that fails the step and the whole job. The codex-review job is masked because its overlay always adds new .codex/agents/*.toml; and this PR itself is masked because it edits .claude/commands/review-pr.md, so the regression won't surface when testing on this PR. Guard the commit:

Suggested change

	git -C "$pr" add -A
	git -C "$pr" -c user.name=ci -c user.email=ci@localhost \
	commit -q -m "ci: trusted review setup"
	git -C "$pr" add -A
	if ! git -C "$pr" diff --cached --quiet; then
	git -C "$pr" -c user.name=ci -c user.email=ci@localhost \
	commit -q -m "ci: trusted review setup"
	fi

[github-actions]

ignore_default_excludes=true disables Codex's built-in stripping of *TOKEN*/*KEY*/*SECRET* env vars from model-run commands, so GITHUB_TOKEN/GH_TOKEN become visible to every sandboxed command, not just the allowlisted gh calls. Because the gh pr comment rule in codex_review.rules constrains only the first three argv tokens and leaves --body free, a reviewer prompt-injected via the PR diff it must read can printenv GH_TOKEN in-sandbox and then gh pr comment … --body <token> to exfiltrate it.

This is at parity with the already-accepted claude-job exfil model, but it turns off a protection Codex gives for free. If you want Codex tighter than that baseline: authenticate gh out-of-band (echo "$GH_TOKEN" | gh auth login --with-token in the trusted step, then unset GH_TOKEN GITHUB_TOKEN) so the default exclusion can stay on — that removes the trivial printenv path (the token stays file-readable via hosts.yml, a smaller surface). Not a blocker; flagging since exfil-resistance is the whole point of this workflow.

[github-actions]

Minor: this slightly overstates the guarantee. With default ensure_ascii=True, json.dumps encodes non-BMP characters as UTF-16 surrogate-pair escapes (e.g. 😀 → "😀"), and lone surrogates aren't valid Unicode scalar values in TOML basic strings — tomllib/Codex would reject the file. Harmless today since these agent files are ASCII-only per repo policy, but it would break silently if a name/description/body ever gained an emoji or astral char. Worth a "(ASCII inputs)" caveat, or a multi-line TOML literal.

[github-actions]

Review summary

Solid, carefully-reasoned PR — the trusted-file swap, sandbox + gh-only network escape, ephemeral CODEX_HOME/HOME shim, and label-scoped concurrency are all well thought through, and the inline docs are accurate (I verified the sandbox/AGENTS.md-trust claims against the actual flags). Reviewed statically. One thing to fix before merge, plus two notes.

Should fix

• scripts/ci/use_trusted_agent_files.sh — the unconditional git commit aborts the claude-review job for any PR that doesn't modify instruction files: the swap is a no-op, nothing is staged, and git commit exits 1 under set -e. The codex job is masked (its overlay always adds .codex/agents/*.toml) and so is this PR (it edits review-pr.md), so it won't show up when testing here. Guard suggested inline.

Consider (non-blocking)

• code_review.yml:234 — ignore_default_excludes=true re-exposes GITHUB_TOKEN/GH_TOKEN to every model-run command, so a prompt-injected reviewer could exfiltrate the token via the free-form --body of the allowlisted gh pr comment. At parity with the accepted claude baseline, but it disables a protection Codex gives for free — out-of-band gh auth login --with-token would let you keep it.
• gen_codex_review.py:55 — the "json.dumps escaping is valid for TOML basic strings" comment holds only for ASCII/BMP input; non-BMP chars become surrogate escapes that TOML rejects. Latent given the repo's ASCII policy.

[tiankongdeguiji]

Review triage, round 2 (cbcb275..c6c2fda) — all three findings were true issues:

No-op git commit aborts claude-review (should-fix) — confirmed and fixed in cbcb275. Reproduced exactly as described (clean clone → swap is a no-op → git commit exit 1 under set -e); the staged-diff guard now skips the commit, and a real instruction-file diff still produces the setup commit.

Token exposure via ignore_default_excludes=true (consider) — the objective was right but the mechanism claim doesn't hold at runtime: shell_environment_policy is inert in codex 0.142.4. Verified live that model-run commands see the token with the flag absent, with an explicit exclude=["GITHUB_TOKEN","GH_TOKEN"], and with inherit="core" — so there was no built-in stripping to preserve, and the flag being removed was already a no-op. c6c2fda fixes it at the source instead: codex runs under env -u GITHUB_TOKEN -u GH_TOKEN, gh authenticates from a stored login in the shim HOME (out-of-band gh auth login --with-token, as suggested), and the MCP server gets its env from a generated config.toml in the per-run CODEX_HOME — which also replaces the -c override forest and --ignore-user-config. Result is tighter than proposed: printenv in model commands returns unset (verified live, along with gh pr view via stored login and an inline comment through the config-file MCP wiring). Remaining surface is file-reads (hosts.yml, per-run config.toml) feeding the accepted gh pr comment --body sink.

json.dumps non-BMP surrogate escapes (consider) — true; fixed properly in 1bad992 with ensure_ascii=False (raw UTF-8 is valid in TOML basic strings), verified by round-tripping an emoji/CJK agent file through tomllib, rather than documenting the ASCII caveat.

One CI-only residual to watch on first post-merge run: gh auth login --with-token with the Actions installation token is exercised only in CI (local verification used a user token); if it rejects the token the job fails loudly at the trusted step.