chore: review everywhere

codex-rs/app-server/src/bespoke_event_handling.rs

@@ -61,6 +61,7 @@ use codex_core::protocol::ReviewDecision;
 use codex_core::protocol::TokenCountEvent;
 use codex_core::protocol::TurnDiffEvent;
 use codex_core::review_format::format_review_findings_block;
+use codex_core::review_prompts;
 use codex_protocol::ConversationId;
 use codex_protocol::plan_tool::UpdatePlanArgs;
 use codex_protocol::protocol::ReviewOutputEvent;
@@ -343,7 +344,9 @@ pub(crate) async fn apply_bespoke_event_handling(
                 .await;
         }
         EventMsg::EnteredReviewMode(review_request) => {
-            let review = review_request.user_facing_hint;
+            let review = review_request
+                .user_facing_hint
+                .unwrap_or_else(|| review_prompts::user_facing_hint(&review_request.target));
             let item = ThreadItem::EnteredReviewMode {
                 id: event_turn_id.clone(),
                 review,

codex-rs/app-server/src/codex_message_processor.rs

@@ -64,7 +64,7 @@ use codex_app_server_protocol::ResumeConversationResponse;
 use codex_app_server_protocol::ReviewDelivery as ApiReviewDelivery;
 use codex_app_server_protocol::ReviewStartParams;
 use codex_app_server_protocol::ReviewStartResponse;
-use codex_app_server_protocol::ReviewTarget;
+use codex_app_server_protocol::ReviewTarget as ApiReviewTarget;
 use codex_app_server_protocol::SandboxMode;
 use codex_app_server_protocol::SendUserMessageParams;
 use codex_app_server_protocol::SendUserMessageResponse;
@@ -124,6 +124,7 @@ use codex_core::protocol::EventMsg;
 use codex_core::protocol::Op;
 use codex_core::protocol::ReviewDelivery as CoreReviewDelivery;
 use codex_core::protocol::ReviewRequest;
+use codex_core::protocol::ReviewTarget as CoreReviewTarget;
 use codex_core::protocol::SessionConfiguredEvent;
 use codex_core::read_head_for_summary;
 use codex_feedback::CodexFeedback;
@@ -255,7 +256,7 @@ impl CodexMessageProcessor {
     }
 
     fn review_request_from_target(
-        target: ReviewTarget,
+        target: ApiReviewTarget,
     ) -> Result<(ReviewRequest, String), JSONRPCErrorError> {
         fn invalid_request(message: String) -> JSONRPCErrorError {
             JSONRPCErrorError {
@@ -265,73 +266,52 @@ impl CodexMessageProcessor {
             }
         }
 
-        match target {
-            // TODO(jif) those messages will be extracted in a follow-up PR.
-            ReviewTarget::UncommittedChanges => Ok((
-                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(),
-                },
-                "Review uncommitted changes".to_string(),
-            )),
-            ReviewTarget::BaseBranch { branch } => {
+        let cleaned_target = match target {
+            ApiReviewTarget::UncommittedChanges => ApiReviewTarget::UncommittedChanges,
+            ApiReviewTarget::BaseBranch { branch } => {
                 let branch = branch.trim().to_string();
                 if branch.is_empty() {
                     return Err(invalid_request("branch must not be empty".to_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}'");
-                let display = format!("Review changes against base branch '{branch}'");
-                Ok((
-                    ReviewRequest {
-                        prompt,
-                        user_facing_hint: hint,
-                    },
-                    display,
-                ))
-            }
-            ReviewTarget::Commit { sha, title } => {
+                ApiReviewTarget::BaseBranch { branch }
+            }
+            ApiReviewTarget::Commit { sha, title } => {
                 let sha = sha.trim().to_string();
                 if sha.is_empty() {
                     return Err(invalid_request("sha must not be empty".to_string()));
                 }
-                let brief_title = title
+                let title = title
                     .map(|t| t.trim().to_string())
                     .filter(|t| !t.is_empty());
-                let prompt = if let Some(title) = brief_title.clone() {
-                    format!("Review the code changes introduced by commit {sha} (\"{title}\"). Provide prioritized, actionable findings.")
-                } else {
-                    format!("Review the code changes introduced by commit {sha}. Provide prioritized, actionable findings.")
-                };
-                let short_sha = sha.chars().take(7).collect::<String>();
-                let hint = format!("commit {short_sha}");
-                let display = if let Some(title) = brief_title {
-                    format!("Review commit {short_sha}: {title}")
-                } else {
-                    format!("Review commit {short_sha}")
-                };
-                Ok((
-                    ReviewRequest {
-                        prompt,
-                        user_facing_hint: hint,
-                    },
-                    display,
-                ))
-            }
-            ReviewTarget::Custom { instructions } => {
+                ApiReviewTarget::Commit { sha, title }
+            }
+            ApiReviewTarget::Custom { instructions } => {
                 let trimmed = instructions.trim().to_string();
                 if trimmed.is_empty() {
-                    return Err(invalid_request("instructions must not be empty".to_string()));
+                    return Err(invalid_request(
+                        "instructions must not be empty".to_string(),
+                    ));
+                }
+                ApiReviewTarget::Custom {
+                    instructions: trimmed,
                 }
-                Ok((
-                    ReviewRequest {
-                        prompt: trimmed.clone(),
-                        user_facing_hint: trimmed.clone(),
-                    },
-                    trimmed,
-                ))
             }
-        }
+        };
+
+        let core_target = match cleaned_target {
+            ApiReviewTarget::UncommittedChanges => CoreReviewTarget::UncommittedChanges,
+            ApiReviewTarget::BaseBranch { branch } => CoreReviewTarget::BaseBranch { branch },
+            ApiReviewTarget::Commit { sha, title } => CoreReviewTarget::Commit { sha, title },
+            ApiReviewTarget::Custom { instructions } => CoreReviewTarget::Custom { instructions },
+        };
+
+        let hint = codex_core::review_prompts::user_facing_hint(&core_target);
+        let review_request = ReviewRequest {
+            target: core_target,
+            user_facing_hint: Some(hint.clone()),
+        };
+
+        Ok((review_request, hint))
     }
 
     pub async fn process_request(&mut self, request: ClientRequest) {

codex-rs/app-server/tests/suite/v2/review.rs

@@ -93,7 +93,7 @@ async fn review_start_runs_review_turn_and_emits_code_review_item() -> Result<()
         match started.item {
             ThreadItem::EnteredReviewMode { id, review } => {
                 assert_eq!(id, turn_id);
-                assert_eq!(review, "commit 1234567");
+                assert_eq!(review, "commit 1234567: Tidy UI colors");
                 saw_entered_review_mode = true;
                 break;
             }

codex-rs/cli/src/main.rs

@@ -18,6 +18,8 @@ use codex_cli::login::run_logout;
 use codex_cloud_tasks::Cli as CloudTasksCli;
 use codex_common::CliConfigOverrides;
 use codex_exec::Cli as ExecCli;
+use codex_exec::Command as ExecCommand;
+use codex_exec::ReviewArgs;
 use codex_execpolicy::ExecPolicyCheckCommand;
 use codex_responses_api_proxy::Args as ResponsesApiProxyArgs;
 use codex_tui::AppExitInfo;
@@ -72,6 +74,9 @@ enum Subcommand {
     #[clap(visible_alias = "e")]
     Exec(ExecCli),
 
+    /// Run a code review non-interactively.
+    Review(ReviewArgs),
+
     /// Manage login.
     Login(LoginCommand),
 
@@ -449,6 +454,15 @@ async fn cli_main(codex_linux_sandbox_exe: Option<PathBuf>) -> anyhow::Result<()
             );
             codex_exec::run_main(exec_cli, codex_linux_sandbox_exe).await?;
         }
+        Some(Subcommand::Review(review_args)) => {
+            let mut exec_cli = ExecCli::try_parse_from(["codex", "exec"])?;
+            exec_cli.command = Some(ExecCommand::Review(review_args));
+            prepend_config_flags(
+                &mut exec_cli.config_overrides,
+                root_config_overrides.clone(),
+            );
+            codex_exec::run_main(exec_cli, codex_linux_sandbox_exe).await?;
+        }
         Some(Subcommand::McpServer) => {
             codex_mcp_server::run_main(codex_linux_sandbox_exe, root_config_overrides).await?;
         }

codex-rs/core/src/codex.rs

@@ -1476,6 +1476,7 @@ mod handlers {
     use crate::codex::spawn_review_thread;
     use crate::config::Config;
     use crate::mcp::auth::compute_auth_statuses;
+    use crate::review_prompts::resolve_review_request;
     use crate::tasks::CompactTask;
     use crate::tasks::RegularTask;
     use crate::tasks::UndoTask;
@@ -1784,14 +1785,28 @@ mod handlers {
         let turn_context = sess
             .new_turn_with_sub_id(sub_id.clone(), SessionSettingsUpdate::default())
             .await;
-        spawn_review_thread(
-            Arc::clone(sess),
-            Arc::clone(config),
-            turn_context.clone(),
-            sub_id,
-            review_request,
-        )
-        .await;
+        match resolve_review_request(review_request, config.cwd.as_path()) {
+            Ok(resolved) => {
+                spawn_review_thread(
+                    Arc::clone(sess),
+                    Arc::clone(config),
+                    turn_context.clone(),
+                    sub_id,
+                    resolved,
+                )
+                .await;
+            }
+            Err(err) => {
+                let event = Event {
+                    id: sub_id,
+                    msg: EventMsg::Error(ErrorEvent {
+                        message: err.to_string(),
+                        codex_error_info: Some(CodexErrorInfo::Other),
+                    }),
+                };
+                sess.send_event(&turn_context, event.msg).await;
+            }
+        }
     }
 }
 
@@ -1801,7 +1816,7 @@ async fn spawn_review_thread(
     config: Arc<Config>,
     parent_turn_context: Arc<TurnContext>,
     sub_id: String,
-    review_request: ReviewRequest,
+    resolved: crate::review_prompts::ResolvedReviewRequest,
 ) {
     let model = config.review_model.clone();
     let review_model_family = find_family_for_model(&model)
@@ -1817,7 +1832,7 @@ async fn spawn_review_thread(
     });
 
     let base_instructions = REVIEW_PROMPT.to_string();
-    let review_prompt = review_request.prompt.clone();
+    let review_prompt = resolved.prompt.clone();
     let provider = parent_turn_context.client.get_provider();
     let auth_manager = parent_turn_context.client.get_auth_manager();
     let model_family = review_model_family.clone();
@@ -1879,6 +1894,10 @@ async fn spawn_review_thread(
     sess.spawn_task(tc.clone(), input, ReviewTask::new()).await;
 
     // Announce entering review mode so UIs can switch modes.
+    let review_request = ReviewRequest {
+        target: resolved.target,
+        user_facing_hint: Some(resolved.user_facing_hint),
+    };
     sess.send_event(&tc, EventMsg::EnteredReviewMode(review_request))
         .await;
 }

codex-rs/core/src/lib.rs

@@ -58,6 +58,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;

codex-rs/core/src/review_prompts.rs

@@ -0,0 +1,93 @@
+use codex_git::merge_base_with_head;
+use codex_protocol::protocol::ReviewRequest;
+use codex_protocol::protocol::ReviewTarget;
+use std::path::Path;
+
+#[derive(Clone, Debug, PartialEq)]
+pub struct ResolvedReviewRequest {
+    pub target: ReviewTarget,
+    pub prompt: String,
+    pub user_facing_hint: String,
+}
+
+const UNCOMMITTED_PROMPT: &str = "Review the current code changes (staged, unstaged, and untracked files) and provide prioritized findings.";
+
+const BASE_BRANCH_PROMPT_BACKUP: &str = "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.";
+const BASE_BRANCH_PROMPT: &str = "Review the code changes against the base branch '{baseBranch}'. The merge base commit for this comparison is {mergeBaseSha}. Run `git diff {mergeBaseSha}` to inspect the changes relative to {baseBranch}. Provide prioritized, actionable findings.";
+
+const COMMIT_PROMPT_WITH_TITLE: &str = "Review the code changes introduced by commit {sha} (\"{title}\"). Provide prioritized, actionable findings.";
+const COMMIT_PROMPT: &str =
+    "Review the code changes introduced by commit {sha}. Provide prioritized, actionable findings.";
+
+pub fn resolve_review_request(
+    request: ReviewRequest,
+    cwd: &Path,
+) -> anyhow::Result<ResolvedReviewRequest> {
+    let target = request.target;
+    let prompt = review_prompt(&target, cwd)?;
+    let user_facing_hint = request
+        .user_facing_hint
+        .unwrap_or_else(|| user_facing_hint(&target));
+
+    Ok(ResolvedReviewRequest {
+        target,
+        prompt,
+        user_facing_hint,
+    })
+}
+
+pub fn review_prompt(target: &ReviewTarget, cwd: &Path) -> anyhow::Result<String> {
+    match target {
+        ReviewTarget::UncommittedChanges => Ok(UNCOMMITTED_PROMPT.to_string()),
+        ReviewTarget::BaseBranch { branch } => {
+            if let Some(commit) = merge_base_with_head(cwd, branch)? {
+                Ok(BASE_BRANCH_PROMPT
+                    .replace("{baseBranch}", branch)
+                    .replace("{mergeBaseSha}", &commit))
+            } else {
+                Ok(BASE_BRANCH_PROMPT_BACKUP.replace("{branch}", branch))
+            }
+        }
+        ReviewTarget::Commit { sha, title } => {
+            if let Some(title) = title {
+                Ok(COMMIT_PROMPT_WITH_TITLE
+                    .replace("{sha}", sha)
+                    .replace("{title}", title))
+            } else {
+                Ok(COMMIT_PROMPT.replace("{sha}", sha))
+            }
+        }
+        ReviewTarget::Custom { instructions } => {
+            let prompt = instructions.trim();
+            if prompt.is_empty() {
+                anyhow::bail!("Review prompt cannot be empty");
+            }
+            Ok(prompt.to_string())
+        }
+    }
+}
+
+pub fn user_facing_hint(target: &ReviewTarget) -> String {
+    match target {
+        ReviewTarget::UncommittedChanges => "current changes".to_string(),
+        ReviewTarget::BaseBranch { branch } => format!("changes against '{branch}'"),
+        ReviewTarget::Commit { sha, title } => {
+            let short_sha: String = sha.chars().take(7).collect();
+            if let Some(title) = title {
+                format!("commit {short_sha}: {title}")
+            } else {
+                format!("commit {short_sha}")
+            }
+        }
+        ReviewTarget::Custom { instructions } => instructions.trim().to_string(),
+    }
+}
+
+impl From<ResolvedReviewRequest> for ReviewRequest {
+    fn from(resolved: ResolvedReviewRequest) -> Self {
+        ReviewRequest {
+            target: resolved.target,
+            user_facing_hint: Some(resolved.user_facing_hint),
+        }
+    }
+}