feat: add review to codex exec

This commit is contained in:
jif-oai
2025-11-12 13:09:13 +00:00
parent 530db0ad73
commit 50b8e856ab
10 changed files with 348 additions and 87 deletions

View File

@@ -52,6 +52,23 @@ You can enable notifications by configuring a script that is run whenever the ag
To run Codex non-interactively, run `codex exec PROMPT` (you can also pass the prompt via `stdin`) and Codex will work on your task until it decides that it is done and exits. Output is printed to the terminal directly. You can set the `RUST_LOG` environment variable to see more about what's going on.
You can also run the review flow headlessly:
```
# Review uncommitted changes
codex exec review --uncommitted
# Review changes against a base branch
codex exec review --branch main
# Review a specific commit
codex exec review --commit abcd123
# Custom review instructions (from arg or stdin)
codex exec review "Review changes in src/ with focus on error handling"
echo "Review all TODOs left in code" | codex exec review -
```
### Experimenting with the Codex Sandbox
To test to see what happens when a command is run under the sandbox provided by Codex, we provide the following subcommands in Codex CLI:

View File

@@ -48,6 +48,7 @@ pub use model_provider_info::create_oss_provider_with_base_url;
mod conversation_manager;
mod event_mapping;
pub mod review_format;
pub mod review_prompts;
pub use codex_protocol::protocol::InitialHistory;
pub use conversation_manager::ConversationManager;
pub use conversation_manager::NewConversation;

View File

@@ -0,0 +1,52 @@
use std::cmp::min;
/// Build the review prompt and user-facing hint for uncommitted changes.
pub fn review_uncommitted_prompt() -> (String, String) {
let prompt = "Review the current code changes (staged, unstaged, and untracked files) and provide prioritized findings.".to_string();
let hint = "current changes".to_string();
(prompt, hint)
}
/// Build the review prompt and hint for reviewing against a base branch.
pub fn review_branch_prompt(branch: &str) -> (String, String) {
let prompt = format!(
"Review the code changes against the base branch '{branch}'. Start by finding the merge diff between the current branch and {branch}'s upstream e.g. (`git merge-base HEAD \"$(git rev-parse --abbrev-ref \"{branch}@{{upstream}}\")\"`), then run `git diff` against that SHA to see what changes we would merge into the {branch} branch. Provide prioritized, actionable findings."
);
let hint = format!("changes against '{branch}'");
(prompt, hint)
}
/// Build the review prompt and hint for a specific commit.
/// If `subject_opt` is provided, it will be included in the prompt; otherwise it is omitted.
pub fn review_commit_prompt(sha: &str, subject_opt: Option<&str>) -> (String, String) {
let short = &sha[..min(7, sha.len())];
let prompt = if let Some(subject) = subject_opt {
format!(
"Review the code changes introduced by commit {sha} (\"{subject}\"). Provide prioritized, actionable findings."
)
} else {
format!(
"Review the code changes introduced by commit {sha}. Provide prioritized, actionable findings."
)
};
let hint = format!("commit {short}");
(prompt, hint)
}
/// Build the review prompt and hint for custom free-form instructions.
pub fn review_custom_prompt(custom: &str) -> (String, String) {
let prompt = custom.trim().to_string();
let trimmed = prompt.clone();
let hint = if trimmed.is_empty() {
"custom review".to_string()
} else {
let s = trimmed.replace('\n', " ");
let max = 80usize;
if s.len() > max {
format!("{}", &s[..max])
} else {
s
}
};
(prompt, hint)
}

View File

@@ -81,6 +81,9 @@ pub struct Cli {
pub enum Command {
/// Resume a previous session by id or pick the most recent with --last.
Resume(ResumeArgs),
/// Run a code review.
Review(ReviewArgs),
}
#[derive(Parser, Debug)]
@@ -99,6 +102,33 @@ pub struct ResumeArgs {
pub prompt: Option<String>,
}
#[derive(Parser, Debug)]
pub struct ReviewArgs {
/// Select uncommitted (staged, unstaged, untracked) changes.
#[arg(long = "uncommitted", default_value_t = false, conflicts_with_all = ["branch", "commit", "prompt"])]
pub uncommitted: bool,
/// Review against the given base branch (PR-style).
#[arg(long = "branch", value_name = "BRANCH", conflicts_with_all = ["commit", "uncommitted", "prompt"])]
pub branch: Option<String>,
/// Review a specific commit by SHA.
#[arg(long = "commit", value_name = "SHA", conflicts_with_all = ["branch", "uncommitted", "prompt"])]
pub commit: Option<String>,
/// Optional override for the user-facing hint recorded in history.
#[arg(long = "hint", value_name = "TEXT")]
pub hint: Option<String>,
/// Optional override for the review model (defaults to config.review_model).
#[arg(long = "review-model", value_name = "MODEL")]
pub review_model: Option<String>,
/// Custom review instructions. If `-` is used, read from stdin.
#[arg(value_name = "PROMPT", value_hint = clap::ValueHint::Other)]
pub prompt: Option<String>,
}
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, ValueEnum)]
#[value(rename_all = "kebab-case")]
pub enum Color {

View File

@@ -10,12 +10,14 @@ use codex_core::protocol::Event;
use codex_core::protocol::EventMsg;
use codex_core::protocol::ExecCommandBeginEvent;
use codex_core::protocol::ExecCommandEndEvent;
use codex_core::protocol::ExitedReviewModeEvent;
use codex_core::protocol::FileChange;
use codex_core::protocol::McpInvocation;
use codex_core::protocol::McpToolCallBeginEvent;
use codex_core::protocol::McpToolCallEndEvent;
use codex_core::protocol::PatchApplyBeginEvent;
use codex_core::protocol::PatchApplyEndEvent;
use codex_core::protocol::ReviewRequest;
use codex_core::protocol::SessionConfiguredEvent;
use codex_core::protocol::StreamErrorEvent;
use codex_core::protocol::TaskCompleteEvent;
@@ -513,6 +515,31 @@ impl EventProcessor for EventProcessorWithHumanOutput {
ts_msg!(self, "task aborted: review ended");
}
},
EventMsg::EnteredReviewMode(ReviewRequest {
user_facing_hint, ..
}) => {
let banner = format!(">> Code review started: {user_facing_hint} <<");
ts_msg!(self, "{banner}");
}
EventMsg::ExitedReviewMode(ExitedReviewModeEvent { review_output }) => {
if let Some(output) = review_output {
if output.findings.is_empty() {
let explanation = output.overall_explanation.trim();
if !explanation.is_empty() {
ts_msg!(self, "{explanation}");
} else {
ts_msg!(self, "Reviewer failed to output a response.");
}
} else {
let block = codex_core::review_format::format_review_findings_block(
&output.findings,
None,
);
ts_msg!(self, "{block}");
}
}
ts_msg!(self, "{}", "<< Code review finished >>".style(self.dimmed));
}
EventMsg::ShutdownComplete => return CodexStatus::Shutdown,
EventMsg::WebSearchBegin(_)
| EventMsg::ExecApprovalRequest(_)
@@ -523,8 +550,6 @@ impl EventProcessor for EventProcessorWithHumanOutput {
| EventMsg::ListCustomPromptsResponse(_)
| EventMsg::RawResponseItem(_)
| EventMsg::UserMessage(_)
| EventMsg::EnteredReviewMode(_)
| EventMsg::ExitedReviewMode(_)
| EventMsg::AgentMessageDelta(_)
| EventMsg::AgentReasoningDelta(_)
| EventMsg::AgentReasoningRawContentDelta(_)

View File

@@ -143,6 +143,7 @@ impl EventProcessorWithJsonOutput {
message: ev.message.clone(),
})],
EventMsg::PlanUpdate(ev) => self.handle_plan_update(ev),
EventMsg::ExitedReviewMode(ev) => self.handle_exited_review_mode(ev),
_ => Vec::new(),
}
}
@@ -184,6 +185,31 @@ impl EventProcessorWithJsonOutput {
vec![ThreadEvent::ItemCompleted(ItemCompletedEvent { item })]
}
fn handle_exited_review_mode(
&self,
ev: &codex_core::protocol::ExitedReviewModeEvent,
) -> Vec<ThreadEvent> {
// Convert review output into a synthetic agent message so JSONL clients
// can observe review results without bespoke item types for now.
if let Some(output) = &ev.review_output {
let text = if output.findings.is_empty() {
output.overall_explanation.clone()
} else {
codex_core::review_format::format_review_findings_block(&output.findings, None)
};
if text.is_empty() {
return Vec::new();
}
let item = ThreadItem {
id: self.get_next_item_id(),
details: ThreadItemDetails::AgentMessage(AgentMessageItem { text }),
};
vec![ThreadEvent::ItemCompleted(ItemCompletedEvent { item })]
} else {
Vec::new()
}
}
fn handle_reasoning_event(&self, ev: &AgentReasoningEvent) -> Vec<ThreadEvent> {
let item = ThreadItem {
id: self.get_next_item_id(),

View File

@@ -42,6 +42,7 @@ use tracing_subscriber::EnvFilter;
use tracing_subscriber::prelude::*;
use crate::cli::Command as ExecCommand;
use crate::cli::ReviewArgs;
use crate::event_processor::CodexStatus;
use crate::event_processor::EventProcessor;
use codex_core::default_client::set_default_originator;
@@ -71,48 +72,43 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option<PathBuf>) -> any
config_overrides,
} = cli;
// Determine the prompt source (parent or subcommand) and read from stdin if needed.
let prompt_arg = match &command {
// Allow prompt before the subcommand by falling back to the parent-level prompt
// when the Resume subcommand did not provide its own prompt.
Some(ExecCommand::Resume(args)) => args.prompt.clone().or(prompt),
None => prompt,
};
let prompt = match prompt_arg {
Some(p) if p != "-" => p,
// Either `-` was passed or no positional arg.
maybe_dash => {
// When no arg (None) **and** stdin is a TTY, bail out early unless the
// user explicitly forced reading via `-`.
let force_stdin = matches!(maybe_dash.as_deref(), Some("-"));
if std::io::stdin().is_terminal() && !force_stdin {
eprintln!(
"No prompt provided. Either specify one as an argument or pipe the prompt into stdin."
);
std::process::exit(1);
}
// Ensure the user knows we are waiting on stdin, as they may
// have gotten into this state by mistake. If so, and they are not
// writing to stdin, Codex will hang indefinitely, so this should
// help them debug in that case.
if !force_stdin {
eprintln!("Reading prompt from stdin...");
}
let mut buffer = String::new();
if let Err(e) = std::io::stdin().read_to_string(&mut buffer) {
eprintln!("Failed to read prompt from stdin: {e}");
std::process::exit(1);
} else if buffer.trim().is_empty() {
eprintln!("No prompt provided via stdin.");
std::process::exit(1);
}
buffer
// Helper: read a prompt from stdin when '-' was passed or when required.
fn read_prompt_from_stdin_or_exit(force: bool) -> String {
if !force && std::io::stdin().is_terminal() {
eprintln!(
"No prompt provided. Either specify one as an argument or pipe the prompt into stdin."
);
std::process::exit(1);
}
if !force {
eprintln!("Reading prompt from stdin...");
}
let mut buffer = String::new();
if let Err(e) = std::io::stdin().read_to_string(&mut buffer) {
eprintln!("Failed to read prompt from stdin: {e}");
std::process::exit(1);
} else if buffer.trim().is_empty() {
eprintln!("No prompt provided via stdin.");
std::process::exit(1);
}
buffer
}
// Determine prompt or review args input.
let parent_or_resume_prompt_arg = match &command {
Some(ExecCommand::Resume(args)) => args.prompt.clone().or(prompt.clone()),
Some(ExecCommand::Review(_)) => None, // handled separately below
None => prompt.clone(),
};
let regular_prompt_opt = parent_or_resume_prompt_arg.map(|p| {
if p == "-" {
read_prompt_from_stdin_or_exit(true)
} else {
p
}
});
let output_schema = load_output_schema(output_schema_path);
let (stdout_with_ansi, stderr_with_ansi) = match color {
@@ -163,9 +159,15 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option<PathBuf>) -> any
};
// Load configuration and determine approval policy
// Allow --review-model override only when review subcommand is present.
let review_model_override: Option<String> = match &command {
Some(ExecCommand::Review(ReviewArgs { review_model, .. })) => review_model.clone(),
_ => None,
};
let overrides = ConfigOverrides {
model,
review_model: None,
review_model: review_model_override,
config_profile,
// Default to never ask for approvals in headless mode. Feature flags can override.
approval_policy: Some(AskForApproval::Never),
@@ -261,8 +263,8 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option<PathBuf>) -> any
conversation_id: _,
conversation,
session_configured,
} = if let Some(ExecCommand::Resume(args)) = command {
let resume_path = resolve_resume_path(&config, &args).await?;
} = if let Some(ExecCommand::Resume(args)) = &command {
let resume_path = resolve_resume_path(&config, args).await?;
if let Some(path) = resume_path {
conversation_manager
@@ -278,9 +280,12 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option<PathBuf>) -> any
.new_conversation(config.clone())
.await?
};
// Print the effective configuration and prompt so users can see what Codex
// is using.
event_processor.print_config_summary(&config, &prompt, &session_configured);
// Echo the effective configuration and the text that will be sent first.
let mut echo_prompt = String::new();
if let Some(p) = &regular_prompt_opt {
echo_prompt = p.clone();
}
event_processor.print_config_summary(&config, &echo_prompt, &session_configured);
info!("Codex initialized with event: {session_configured:?}");
@@ -323,25 +328,74 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option<PathBuf>) -> any
});
}
// Package images and prompt into a single user input turn.
let mut items: Vec<UserInput> = images
.into_iter()
.map(|path| UserInput::LocalImage { path })
.collect();
items.push(UserInput::Text { text: prompt });
let initial_prompt_task_id = conversation
.submit(Op::UserTurn {
items,
cwd: default_cwd,
approval_policy: default_approval_policy,
sandbox_policy: default_sandbox_policy,
model: default_model,
effort: default_effort,
summary: default_summary,
final_output_json_schema: output_schema,
})
.await?;
info!("Sent prompt with event ID: {initial_prompt_task_id}");
let _initial_prompt_task_id = match &command {
Some(ExecCommand::Review(args)) => {
use codex_core::protocol::ReviewRequest;
// Compute prompt + hint.
let (review_prompt, mut review_hint) = if let Some(custom) = args.prompt.as_deref() {
let text = if custom == "-" {
read_prompt_from_stdin_or_exit(true)
} else {
custom.to_string()
};
codex_core::review_prompts::review_custom_prompt(&text)
} else if let Some(branch) = &args.branch {
codex_core::review_prompts::review_branch_prompt(branch)
} else if let Some(sha) = &args.commit {
// Try to enrich with subject; fall back gracefully.
let mut subject: Option<String> = None;
let entries = codex_core::git_info::recent_commits(&default_cwd, 200).await;
for e in entries {
if e.sha.starts_with(sha) {
if !e.subject.trim().is_empty() {
subject = Some(e.subject);
}
break;
}
}
codex_core::review_prompts::review_commit_prompt(sha, subject.as_deref())
} else {
// Default to reviewing uncommitted changes if nothing else specified.
codex_core::review_prompts::review_uncommitted_prompt()
};
if let Some(hint_override) = &args.hint {
review_hint = hint_override.clone();
}
let review_request = ReviewRequest {
prompt: review_prompt,
user_facing_hint: review_hint,
};
let id = conversation.submit(Op::Review { review_request }).await?;
info!("Sent review request with event ID: {id}");
id
}
_ => {
// Package images and prompt into a single user input turn.
let mut items: Vec<UserInput> = images
.into_iter()
.map(|path| UserInput::LocalImage { path })
.collect();
let text = regular_prompt_opt.unwrap_or_default();
items.push(UserInput::Text { text });
let id = conversation
.submit(Op::UserTurn {
items,
cwd: default_cwd,
approval_policy: default_approval_policy,
sandbox_policy: default_sandbox_policy,
model: default_model,
effort: default_effort,
summary: default_summary,
final_output_json_schema: output_schema,
})
.await?;
info!("Sent prompt with event ID: {id}");
id
}
};
// Run the loop until the task is complete.
// Track whether a fatal error was reported by the server so we can

View File

@@ -4,5 +4,6 @@ mod auth_env;
mod originator;
mod output_schema;
mod resume;
mod review;
mod sandbox;
mod server_error_exit;

View File

@@ -0,0 +1,59 @@
#![cfg(not(target_os = "windows"))]
#![allow(clippy::expect_used, clippy::unwrap_used)]
use core_test_support::responses;
use core_test_support::test_codex_exec::test_codex_exec;
use predicates::prelude::*;
/// Verify that `codex exec review` triggers the review flow and renders
/// a formatted finding block when the reviewer returns a structured JSON result.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn exec_review_uncommitted_renders_findings() -> anyhow::Result<()> {
let test = test_codex_exec();
// Structured review output returned by the reviewer model (as a JSON string).
let review_json = serde_json::json!({
"findings": [
{
"title": "Prefer Stylize helpers",
"body": "Use .dim()/.bold() chaining instead of manual Style where possible.",
"confidence_score": 0.9,
"priority": 1,
"code_location": {
"absolute_file_path": "/tmp/file.rs",
"line_range": {"start": 10, "end": 20}
}
}
],
"overall_correctness": "good",
"overall_explanation": "All good with some improvements suggested.",
"overall_confidence_score": 0.8
})
.to_string();
let server = responses::start_mock_server().await;
let body = responses::sse(vec![
responses::ev_response_created("resp-1"),
responses::ev_assistant_message("m-1", &review_json),
responses::ev_completed("resp-1"),
]);
responses::mount_sse_once(&server, body).await;
test.cmd_with_server(&server)
.arg("--skip-git-repo-check")
.arg("-C")
.arg(test.cwd_path())
.arg("review")
.arg("--uncommitted")
.assert()
.success()
.stderr(predicate::str::contains(
">> Code review started: current changes <<",
))
.stderr(predicate::str::contains(
"- Prefer Stylize helpers — /tmp/file.rs:10-20",
))
.stderr(predicate::str::contains("<< Code review finished >>"));
Ok(())
}

View File

@@ -2561,16 +2561,15 @@ impl ChatWidget {
items.push(SelectionItem {
name: "Review uncommitted changes".to_string(),
actions: vec![Box::new(
move |tx: &AppEventSender| {
tx.send(AppEvent::CodexOp(Op::Review {
review_request: ReviewRequest {
prompt: "Review the current code changes (staged, unstaged, and untracked files) and provide prioritized findings.".to_string(),
user_facing_hint: "current changes".to_string(),
},
}));
},
)],
actions: vec![Box::new(move |tx: &AppEventSender| {
let (prompt, hint) = codex_core::review_prompts::review_uncommitted_prompt();
tx.send(AppEvent::CodexOp(Op::Review {
review_request: ReviewRequest {
prompt,
user_facing_hint: hint,
},
}));
})],
dismiss_on_select: true,
..Default::default()
});
@@ -2617,12 +2616,11 @@ impl ChatWidget {
items.push(SelectionItem {
name: format!("{current_branch} -> {branch}"),
actions: vec![Box::new(move |tx3: &AppEventSender| {
let (prompt, hint) = codex_core::review_prompts::review_branch_prompt(&branch);
tx3.send(AppEvent::CodexOp(Op::Review {
review_request: ReviewRequest {
prompt: format!(
"Review the code changes against the base branch '{branch}'. Start by finding the merge diff between the current branch and {branch}'s upstream e.g. (`git merge-base HEAD \"$(git rev-parse --abbrev-ref \"{branch}@{{upstream}}\")\"`), then run `git diff` against that SHA to see what changes we would merge into the {branch} branch. Provide prioritized, actionable findings."
),
user_facing_hint: format!("changes against '{branch}'"),
prompt,
user_facing_hint: hint,
},
}));
})],
@@ -2649,16 +2647,13 @@ impl ChatWidget {
for entry in commits {
let subject = entry.subject.clone();
let sha = entry.sha.clone();
let short = sha.chars().take(7).collect::<String>();
let search_val = format!("{subject} {sha}");
items.push(SelectionItem {
name: subject.clone(),
actions: vec![Box::new(move |tx3: &AppEventSender| {
let hint = format!("commit {short}");
let prompt = format!(
"Review the code changes introduced by commit {sha} (\"{subject}\"). Provide prioritized, actionable findings."
);
let (prompt, hint) =
codex_core::review_prompts::review_commit_prompt(&sha, Some(&subject));
tx3.send(AppEvent::CodexOp(Op::Review {
review_request: ReviewRequest {
prompt,
@@ -2693,10 +2688,11 @@ impl ChatWidget {
if trimmed.is_empty() {
return;
}
let (prompt, hint) = codex_core::review_prompts::review_custom_prompt(&trimmed);
tx.send(AppEvent::CodexOp(Op::Review {
review_request: ReviewRequest {
prompt: trimmed.clone(),
user_facing_hint: trimmed,
prompt,
user_facing_hint: hint,
},
}));
}),