Fix broken github codex review (#14923)

[joshkang97]

Summary:

Fix the GitHub Actions Codex review job by running generated review prompts through generic codex exec instead of the specialized codex exec review subcommand. The workflow already embeds PR metadata, reviewer instructions, and the diff in /tmp/prompt.txt, while codex exec review --base rejects a positional/stdin prompt. This removes the invalid --base/stdin combination from both automatic and manual Codex review paths while leaving /codex-query behavior unchanged.

Differential Revision: D111135454

[meta-codesync]

@joshkang97 has exported this pull request. If you are a Meta employee, you can view the originating Diff in D111135454.

[github-actions]

✅ clang-tidy: No findings on changed lines

Completed in 0.0s.

[github-actions]

Codex Code Review - OBSOLETE

Superseded by a newer AI review. Expand to see the original review.

🟡 Codex Code Review

Auto-triggered after CI passed — reviewing commit 3b5d976

---

❌ Codex review failed before producing findings.

WARNING: proceeding, even though we could not create PATH aliases: Refusing to create helper binaries under temporary dir "/tmp" (codex_home: AbsolutePathBuf("/tmp/codex-home"))
error: the argument '--base <BRANCH>' cannot be used with '[PROMPT]'

Usage: codex exec review --commit <SHA> --base <BRANCH> --title <TITLE> --model <MODEL> --config <key=value> --dangerously-bypass-approvals-and-sandbox --output-last-message <FILE> [PROMPT]

For more information, try '--help'.

---

ℹ️ About this response

Generated by Codex CLI.
Review methodology: claude_md/code_review.md

Limitations:

• Codex may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /codex-review [context] — Request a code review
• /codex-query <question> — Ask about the PR or codebase

[github-actions]

Claude Code Review - OBSOLETE

Superseded by a newer AI review. Expand to see the original review.

✅ Claude Code Review

Auto-triggered after CI passed — reviewing commit 3b5d976

---

Summary

Clean, correct fix. The PR replaces the broken codex exec review --base --title invocations with codex exec in both the automatic and manual Codex review paths. The PR metadata (title, base SHA, diff) is already embedded in /tmp/prompt.txt by the upstream "Generate diff and prompt" steps, so passing them again via CLI flags was both redundant and broken. The change is consistent with existing patterns in the same file (complexity classification, recovery steps, and the query branch all already use codex exec). The unused BASE_SHA/PR_TITLE env vars are correctly removed from both steps.

No high-severity findings.

Full review (click to expand)

Findings

🔴 HIGH

None.

🟡 MEDIUM

None.

🟢 LOW / NIT

L1. Residual BASE_SHA/PR_TITLE env vars in other steps — not a bug

The BASE_SHA and PR_TITLE env vars remain in the "Generate diff and prompt" steps (lines 446-447, 1015-1016), where they are legitimately used to generate the diff and build /tmp/prompt.txt. No cleanup needed there — just noting for completeness that the removal was correctly scoped to only the Codex invocation steps.

Cross-Component Analysis

Path	Before	After	Correct?
Auto review (line ~726)	codex exec review --base --title	codex exec	YES — prompt.txt has all metadata
Manual review (line ~1357)	codex exec review --base --title	codex exec	YES — prompt.txt has all metadata
Manual query (line ~1350)	codex exec (unchanged)	codex exec (unchanged)	YES — already correct
Recovery (line ~754)	codex exec (unchanged)	codex exec (unchanged)	YES — already correct
Complexity (line ~561)	codex exec (unchanged)	codex exec (unchanged)	YES — already correct

Both changed paths now match the pattern used by the other codex exec invocations in the same file, ensuring consistency.

Positive Observations

• The change correctly removes env vars that are no longer referenced in the run block, keeping the workflow clean.
• The manual path's if/else branching (query vs review) now uses the same codex exec command for both branches, which is a simplification — though the if/else still exists for the output file naming and is still correct.

---

ℹ️ About this response

Generated by Claude Code.
Review methodology: claude_md/code_review.md

Limitations:

• Claude may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /claude-review [context] — Request a code review
• /claude-query <question> — Ask about the PR or codebase

[github-actions]

🟡 Codex Code Review

Auto-triggered after CI passed — reviewing commit 1edfcaf

---

❌ Codex review failed before producing findings.

WARNING: proceeding, even though we could not create PATH aliases: Refusing to create helper binaries under temporary dir "/tmp" (codex_home: AbsolutePathBuf("/tmp/codex-home"))
error: the argument '--base <BRANCH>' cannot be used with '[PROMPT]'

Usage: codex exec review --commit <SHA> --base <BRANCH> --title <TITLE> --model <MODEL> --config <key=value> --dangerously-bypass-approvals-and-sandbox --output-last-message <FILE> [PROMPT]

For more information, try '--help'.

---

ℹ️ About this response

Generated by Codex CLI.
Review methodology: claude_md/code_review.md

Limitations:

• Codex may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /codex-review [context] — Request a code review
• /codex-query <question> — Ask about the PR or codebase

[github-actions]

✅ Claude Code Review

Auto-triggered after CI passed — reviewing commit 1edfcaf

---

Summary

Clean, minimal CI fix that replaces the invalid codex exec review --base --title invocation with generic codex exec in two workflow steps, and removes the now-unused BASE_SHA/PR_TITLE environment variables from those steps. The PR metadata is already embedded in /tmp/prompt.txt by earlier steps, so no information is lost.

No high-severity findings.

Full review (click to expand)

Findings

🔴 HIGH

None.

🟡 MEDIUM

None.

🟢 LOW / NIT

L1. Consistency already achieved — codex exec matches existing usage — .github/workflows/ai-review-analysis.yml

• Observation: After this change, both the automatic review path (line ~724) and the manual review else branch (line ~1350) now use codex exec — matching the existing query path and recovery steps that already use codex exec. This is correct and consistent.

Cross-Component Analysis

Aspect	Assessment
Prompt content	/tmp/prompt.txt is still constructed with PR title, base SHA, diff, and full review instructions in earlier steps (lines 446-468, 1015-1053). No information loss.
BASE_SHA / PR_TITLE in other steps	Still defined and used in the prompt-construction steps — only removed from the two Codex invocation steps where they were passed as CLI args. No dangling references.
/codex-query path	Unchanged — the if [ "${COMMAND_TYPE}" = "query" ] branch already used codex exec and is not modified.
Recovery steps	Already used codex exec (lines 754, 1386). Unaffected.

Positive Observations

• The change is correctly scoped — only the two broken invocations are modified.
• Environment variable cleanup is thorough: BASE_SHA and PR_TITLE are removed only from steps that no longer use them.
• The fix is consistent with the working codex exec pattern used elsewhere in the same workflow.

---

ℹ️ About this response

Generated by Claude Code.
Review methodology: claude_md/code_review.md

Limitations:

• Claude may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /claude-review [context] — Request a code review
• /claude-query <question> — Ask about the PR or codebase