diff --git a/codex-rs/README.md b/codex-rs/README.md index 385b4c62e5..29f5477195 100644 --- a/codex-rs/README.md +++ b/codex-rs/README.md @@ -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: diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 5229d00606..287dff1919 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -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; diff --git a/codex-rs/core/src/review_prompts.rs b/codex-rs/core/src/review_prompts.rs new file mode 100644 index 0000000000..9c033dfd70 --- /dev/null +++ b/codex-rs/core/src/review_prompts.rs @@ -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) +} diff --git a/codex-rs/exec/src/cli.rs b/codex-rs/exec/src/cli.rs index 065f4826a0..3d5268037a 100644 --- a/codex-rs/exec/src/cli.rs +++ b/codex-rs/exec/src/cli.rs @@ -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, } +#[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, + + /// Review a specific commit by SHA. + #[arg(long = "commit", value_name = "SHA", conflicts_with_all = ["branch", "uncommitted", "prompt"])] + pub commit: Option, + + /// Optional override for the user-facing hint recorded in history. + #[arg(long = "hint", value_name = "TEXT")] + pub hint: Option, + + /// Optional override for the review model (defaults to config.review_model). + #[arg(long = "review-model", value_name = "MODEL")] + pub review_model: Option, + + /// Custom review instructions. If `-` is used, read from stdin. + #[arg(value_name = "PROMPT", value_hint = clap::ValueHint::Other)] + pub prompt: Option, +} + #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, ValueEnum)] #[value(rename_all = "kebab-case")] pub enum Color { diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index 93e0e493bb..cef66959b7 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -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(_) diff --git a/codex-rs/exec/src/event_processor_with_jsonl_output.rs b/codex-rs/exec/src/event_processor_with_jsonl_output.rs index 23dff015eb..b8cff7a4b2 100644 --- a/codex-rs/exec/src/event_processor_with_jsonl_output.rs +++ b/codex-rs/exec/src/event_processor_with_jsonl_output.rs @@ -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 { + // 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 { let item = ThreadItem { id: self.get_next_item_id(), diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 13d43f61e2..3949cf5857 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -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) -> 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) -> any }; // Load configuration and determine approval policy + // Allow --review-model override only when review subcommand is present. + let review_model_override: Option = 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) -> 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) -> 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) = ®ular_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) -> any }); } - // Package images and prompt into a single user input turn. - let mut items: Vec = 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 = 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 = 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 diff --git a/codex-rs/exec/tests/suite/mod.rs b/codex-rs/exec/tests/suite/mod.rs index 052c43bf38..90ad138001 100644 --- a/codex-rs/exec/tests/suite/mod.rs +++ b/codex-rs/exec/tests/suite/mod.rs @@ -4,5 +4,6 @@ mod auth_env; mod originator; mod output_schema; mod resume; +mod review; mod sandbox; mod server_error_exit; diff --git a/codex-rs/exec/tests/suite/review.rs b/codex-rs/exec/tests/suite/review.rs new file mode 100644 index 0000000000..12dd281461 --- /dev/null +++ b/codex-rs/exec/tests/suite/review.rs @@ -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(()) +} diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 7055822274..53a8d9b734 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -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::(); 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, }, })); }),