diff --git a/crates/db/src/models/execution_process.rs b/crates/db/src/models/execution_process.rs index 0bf7cf6a..3be615c7 100644 --- a/crates/db/src/models/execution_process.rs +++ b/crates/db/src/models/execution_process.rs @@ -686,6 +686,9 @@ impl ExecutionProcess { ExecutorActionType::CodingAgentFollowUpRequest(request) => { Ok(Some(request.executor_profile_id.clone())) } + ExecutorActionType::ReviewRequest(request) => { + Ok(Some(request.executor_profile_id.clone())) + } _ => Err(ExecutionProcessError::ValidationError( "Couldn't find profile from initial request".to_string(), )), diff --git a/crates/executors/src/actions/mod.rs b/crates/executors/src/actions/mod.rs index 7d24c115..ae07e2f1 100644 --- a/crates/executors/src/actions/mod.rs +++ b/crates/executors/src/actions/mod.rs @@ -8,7 +8,8 @@ use ts_rs::TS; use crate::{ actions::{ coding_agent_follow_up::CodingAgentFollowUpRequest, - coding_agent_initial::CodingAgentInitialRequest, script::ScriptRequest, + coding_agent_initial::CodingAgentInitialRequest, review::ReviewRequest, + script::ScriptRequest, }, approvals::ExecutorApprovalService, env::ExecutionEnv, @@ -16,8 +17,11 @@ use crate::{ }; pub mod coding_agent_follow_up; pub mod coding_agent_initial; +pub mod review; pub mod script; +pub use review::RepoReviewContext; + #[enum_dispatch] #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, TS)] #[serde(tag = "type")] @@ -25,6 +29,7 @@ pub enum ExecutorActionType { CodingAgentInitialRequest, CodingAgentFollowUpRequest, ScriptRequest, + ReviewRequest, } #[derive(Debug, Clone, Serialize, Deserialize, TS)] @@ -60,6 +65,7 @@ impl ExecutorAction { ExecutorActionType::CodingAgentFollowUpRequest(request) => { Some(request.base_executor()) } + ExecutorActionType::ReviewRequest(request) => Some(request.base_executor()), ExecutorActionType::ScriptRequest(_) => None, } } diff --git a/crates/executors/src/actions/review.rs b/crates/executors/src/actions/review.rs new file mode 100644 index 00000000..fce7a131 --- /dev/null +++ b/crates/executors/src/actions/review.rs @@ -0,0 +1,81 @@ +use std::{path::Path, sync::Arc}; + +use async_trait::async_trait; +use serde::{Deserialize, Serialize}; +use ts_rs::TS; +use uuid::Uuid; + +use crate::{ + actions::Executable, + approvals::ExecutorApprovalService, + env::ExecutionEnv, + executors::{BaseCodingAgent, ExecutorError, SpawnedChild, StandardCodingAgentExecutor}, + profile::{ExecutorConfigs, ExecutorProfileId}, +}; + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, TS)] +pub struct RepoReviewContext { + pub repo_id: Uuid, + pub repo_name: String, + pub base_commit: String, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, TS)] +pub struct ReviewRequest { + pub executor_profile_id: ExecutorProfileId, + pub context: Option>, + pub prompt: String, + /// Optional session ID to resume an existing session + #[serde(default)] + pub session_id: Option, + /// Optional relative path to execute the agent in (relative to container_ref). + #[serde(default)] + pub working_dir: Option, +} + +impl ReviewRequest { + pub fn base_executor(&self) -> BaseCodingAgent { + self.executor_profile_id.executor + } + + pub fn effective_dir(&self, current_dir: &Path) -> std::path::PathBuf { + match &self.working_dir { + Some(rel_path) => current_dir.join(rel_path), + None => current_dir.to_path_buf(), + } + } +} + +#[async_trait] +impl Executable for ReviewRequest { + async fn spawn( + &self, + current_dir: &Path, + approvals: Arc, + env: &ExecutionEnv, + ) -> Result { + // Use working_dir if specified, otherwise use current_dir + let effective_dir = match &self.working_dir { + Some(rel_path) => current_dir.join(rel_path), + None => current_dir.to_path_buf(), + }; + + let executor_profile_id = self.executor_profile_id.clone(); + let mut agent = ExecutorConfigs::get_cached() + .get_coding_agent(&executor_profile_id) + .ok_or(ExecutorError::UnknownExecutorType( + executor_profile_id.to_string(), + ))?; + + agent.use_approvals(approvals.clone()); + + agent + .spawn_review( + &effective_dir, + &self.prompt, + self.session_id.as_deref(), + env, + ) + .await + } +} diff --git a/crates/executors/src/executors/codex.rs b/crates/executors/src/executors/codex.rs index 00f77018..0568a065 100644 --- a/crates/executors/src/executors/codex.rs +++ b/crates/executors/src/executors/codex.rs @@ -1,6 +1,7 @@ pub mod client; pub mod jsonrpc; pub mod normalize_logs; +pub mod review; pub mod session; use std::{ collections::HashMap, @@ -9,7 +10,7 @@ use std::{ }; use async_trait::async_trait; -use codex_app_server_protocol::NewConversationParams; +use codex_app_server_protocol::{NewConversationParams, ReviewTarget}; use codex_protocol::{ config_types::SandboxMode as CodexSandboxMode, protocol::AskForApproval as CodexAskForApproval, }; @@ -102,6 +103,11 @@ pub enum ReasoningSummaryFormat { Experimental, } +enum CodexSessionAction { + Chat { prompt: String }, + Review { target: ReviewTarget }, +} + #[derive(Derivative, Clone, Serialize, Deserialize, TS, JsonSchema)] #[derivative(Debug, PartialEq)] pub struct Codex { @@ -155,7 +161,11 @@ impl StandardCodingAgentExecutor for Codex { env: &ExecutionEnv, ) -> Result { let command_parts = self.build_command_builder().build_initial()?; - self.spawn_inner(current_dir, prompt, command_parts, None, env) + let combined_prompt = self.append_prompt.combine_prompt(prompt); + let action = CodexSessionAction::Chat { + prompt: combined_prompt, + }; + self.spawn_inner(current_dir, command_parts, action, None, env) .await } @@ -167,7 +177,11 @@ impl StandardCodingAgentExecutor for Codex { env: &ExecutionEnv, ) -> Result { let command_parts = self.build_command_builder().build_follow_up(&[])?; - self.spawn_inner(current_dir, prompt, command_parts, Some(session_id), env) + let combined_prompt = self.append_prompt.combine_prompt(prompt); + let action = CodexSessionAction::Chat { + prompt: combined_prompt, + }; + self.spawn_inner(current_dir, command_parts, action, Some(session_id), env) .await } @@ -206,6 +220,24 @@ impl StandardCodingAgentExecutor for Codex { AvailabilityInfo::NotFound } } + + async fn spawn_review( + &self, + current_dir: &Path, + prompt: &str, + session_id: Option<&str>, + env: &ExecutionEnv, + ) -> Result { + let command_parts = self.build_command_builder().build_initial()?; + let review_target = ReviewTarget::Custom { + instructions: prompt.to_string(), + }; + let action = CodexSessionAction::Review { + target: review_target, + }; + self.spawn_inner(current_dir, command_parts, action, session_id, env) + .await + } } impl Codex { @@ -294,12 +326,11 @@ impl Codex { async fn spawn_inner( &self, current_dir: &Path, - prompt: &str, command_parts: CommandParts, + action: CodexSessionAction, resume_session: Option<&str>, env: &ExecutionEnv, ) -> Result { - let combined_prompt = self.append_prompt.combine_prompt(prompt); let (program_path, args) = command_parts.into_resolved().await?; let mut process = Command::new(program_path); @@ -340,19 +371,37 @@ impl Codex { tokio::spawn(async move { let exit_signal_tx = ExitSignalSender::new(exit_signal_tx); let log_writer = LogWriter::new(new_stdout); - if let Err(err) = Self::launch_codex_app_server( - params, - resume_session, - combined_prompt, - child_stdout, - child_stdin, - log_writer.clone(), - exit_signal_tx.clone(), - approvals, - auto_approve, - ) - .await - { + let launch_result = match action { + CodexSessionAction::Chat { prompt } => { + Self::launch_codex_app_server( + params, + resume_session, + prompt, + child_stdout, + child_stdin, + log_writer.clone(), + exit_signal_tx.clone(), + approvals, + auto_approve, + ) + .await + } + CodexSessionAction::Review { target } => { + review::launch_codex_review( + params, + resume_session, + target, + child_stdout, + child_stdin, + log_writer.clone(), + exit_signal_tx.clone(), + approvals, + auto_approve, + ) + .await + } + }; + if let Err(err) = launch_result { match &err { ExecutorError::Io(io_err) if io_err.kind() == std::io::ErrorKind::BrokenPipe => diff --git a/crates/executors/src/executors/codex/client.rs b/crates/executors/src/executors/codex/client.rs index 535cc0ce..bb90ca2b 100644 --- a/crates/executors/src/executors/codex/client.rs +++ b/crates/executors/src/executors/codex/client.rs @@ -12,7 +12,8 @@ use codex_app_server_protocol::{ GetAuthStatusParams, GetAuthStatusResponse, InitializeParams, InitializeResponse, InputItem, JSONRPCError, JSONRPCNotification, JSONRPCRequest, JSONRPCResponse, NewConversationParams, NewConversationResponse, RequestId, ResumeConversationParams, ResumeConversationResponse, - SendUserMessageParams, SendUserMessageResponse, ServerNotification, ServerRequest, + ReviewStartParams, ReviewStartResponse, ReviewTarget, SendUserMessageParams, + SendUserMessageResponse, ServerNotification, ServerRequest, }; use codex_protocol::{ConversationId, protocol::ReviewDecision}; use serde::{Serialize, de::DeserializeOwned}; @@ -146,6 +147,23 @@ impl AppServerClient { }; self.send_request(request, "getAuthStatus").await } + + pub async fn start_review( + &self, + thread_id: String, + target: ReviewTarget, + ) -> Result { + let request = ClientRequest::ReviewStart { + request_id: self.next_request_id(), + params: ReviewStartParams { + thread_id, + target, + delivery: None, + }, + }; + self.send_request(request, "reviewStart").await + } + async fn handle_server_request( &self, peer: &JsonRpcPeer, @@ -482,7 +500,8 @@ fn request_id(request: &ClientRequest) -> RequestId { | ClientRequest::GetAuthStatus { request_id, .. } | ClientRequest::ResumeConversation { request_id, .. } | ClientRequest::AddConversationListener { request_id, .. } - | ClientRequest::SendUserMessage { request_id, .. } => request_id.clone(), + | ClientRequest::SendUserMessage { request_id, .. } + | ClientRequest::ReviewStart { request_id, .. } => request_id.clone(), _ => unreachable!("request_id called for unsupported request variant"), } } diff --git a/crates/executors/src/executors/codex/review.rs b/crates/executors/src/executors/codex/review.rs new file mode 100644 index 00000000..e61e20b8 --- /dev/null +++ b/crates/executors/src/executors/codex/review.rs @@ -0,0 +1,63 @@ +use std::sync::Arc; + +use codex_app_server_protocol::{NewConversationParams, ReviewTarget}; + +use super::{ + client::{AppServerClient, LogWriter}, + jsonrpc::{ExitSignalSender, JsonRpcPeer}, + session::SessionHandler, +}; +use crate::{approvals::ExecutorApprovalService, executors::ExecutorError}; + +#[allow(clippy::too_many_arguments)] +pub async fn launch_codex_review( + conversation_params: NewConversationParams, + resume_session: Option, + review_target: ReviewTarget, + child_stdout: tokio::process::ChildStdout, + child_stdin: tokio::process::ChildStdin, + log_writer: LogWriter, + exit_signal_tx: ExitSignalSender, + approvals: Option>, + auto_approve: bool, +) -> Result<(), ExecutorError> { + let client = AppServerClient::new(log_writer, approvals, auto_approve); + let rpc_peer = JsonRpcPeer::spawn(child_stdin, child_stdout, client.clone(), exit_signal_tx); + client.connect(rpc_peer); + client.initialize().await?; + let auth_status = client.get_auth_status().await?; + if auth_status.requires_openai_auth.unwrap_or(true) && auth_status.auth_method.is_none() { + return Err(ExecutorError::AuthRequired( + "Codex authentication required".to_string(), + )); + } + + let conversation_id = match resume_session { + Some(session_id) => { + let (rollout_path, _forked_session_id) = SessionHandler::fork_rollout_file(&session_id) + .map_err(|e| ExecutorError::FollowUpNotSupported(e.to_string()))?; + let response = client + .resume_conversation(rollout_path.clone(), conversation_params) + .await?; + tracing::debug!( + "resuming session for review using rollout file {}, response {:?}", + rollout_path.display(), + response + ); + response.conversation_id + } + None => { + let response = client.new_conversation(conversation_params).await?; + response.conversation_id + } + }; + + client.register_session(&conversation_id).await?; + client.add_conversation_listener(conversation_id).await?; + + client + .start_review(conversation_id.to_string(), review_target) + .await?; + + Ok(()) +} diff --git a/crates/executors/src/executors/mod.rs b/crates/executors/src/executors/mod.rs index 3b526d95..95a5e525 100644 --- a/crates/executors/src/executors/mod.rs +++ b/crates/executors/src/executors/mod.rs @@ -15,7 +15,7 @@ use workspace_utils::msg_store::MsgStore; #[cfg(feature = "qa-mode")] use crate::executors::qa_mock::QaMockExecutor; use crate::{ - actions::ExecutorAction, + actions::{ExecutorAction, review::RepoReviewContext}, approvals::ExecutorApprovalService, command::CommandBuildError, env::ExecutionEnv, @@ -215,6 +215,20 @@ pub trait StandardCodingAgentExecutor { session_id: &str, env: &ExecutionEnv, ) -> Result; + + async fn spawn_review( + &self, + current_dir: &Path, + prompt: &str, + session_id: Option<&str>, + env: &ExecutionEnv, + ) -> Result { + match session_id { + Some(id) => self.spawn_follow_up(current_dir, prompt, id, env).await, + None => self.spawn(current_dir, prompt, env).await, + } + } + fn normalize_logs(&self, _raw_logs_event_store: Arc, _worktree_path: &Path); // MCP configuration methods @@ -298,6 +312,34 @@ impl AppendPrompt { } } +pub fn build_review_prompt( + context: Option<&[RepoReviewContext]>, + additional_prompt: Option<&str>, +) -> String { + let mut prompt = String::from("Please review the code changes.\n\n"); + + if let Some(repos) = context { + for repo in repos { + prompt.push_str(&format!("Repository: {}\n", repo.repo_name)); + prompt.push_str(&format!( + "Review all changes from base commit {} to HEAD.\n", + repo.base_commit + )); + prompt.push_str(&format!( + "Use `git diff {}..HEAD` to see the changes.\n", + repo.base_commit + )); + prompt.push('\n'); + } + } + + if let Some(additional) = additional_prompt { + prompt.push_str(additional); + } + + prompt +} + #[cfg(test)] mod tests { use std::str::FromStr; diff --git a/crates/server/src/bin/generate_types.rs b/crates/server/src/bin/generate_types.rs index 50f00dac..8171fd9e 100644 --- a/crates/server/src/bin/generate_types.rs +++ b/crates/server/src/bin/generate_types.rs @@ -114,6 +114,8 @@ fn generate_types_content() -> String { server::routes::task_attempts::PushTaskAttemptRequest::decl(), server::routes::task_attempts::RenameBranchRequest::decl(), server::routes::task_attempts::RenameBranchResponse::decl(), + server::routes::sessions::review::StartReviewRequest::decl(), + server::routes::sessions::review::ReviewError::decl(), server::routes::task_attempts::OpenEditorRequest::decl(), server::routes::task_attempts::OpenEditorResponse::decl(), server::routes::shared_tasks::AssignSharedTaskRequest::decl(), @@ -196,6 +198,8 @@ fn generate_types_content() -> String { executors::executors::AppendPrompt::decl(), executors::actions::coding_agent_initial::CodingAgentInitialRequest::decl(), executors::actions::coding_agent_follow_up::CodingAgentFollowUpRequest::decl(), + executors::actions::review::ReviewRequest::decl(), + executors::actions::review::RepoReviewContext::decl(), executors::logs::CommandExitStatus::decl(), executors::logs::CommandRunResult::decl(), executors::logs::NormalizedEntry::decl(), diff --git a/crates/server/src/routes/sessions/mod.rs b/crates/server/src/routes/sessions/mod.rs index ccf689b4..7ab4de38 100644 --- a/crates/server/src/routes/sessions/mod.rs +++ b/crates/server/src/routes/sessions/mod.rs @@ -1,4 +1,5 @@ pub mod queue; +pub mod review; use std::str::FromStr; @@ -239,6 +240,7 @@ pub fn router(deployment: &DeploymentImpl) -> Router { let session_id_router = Router::new() .route("/", get(get_session)) .route("/follow-up", post(follow_up)) + .route("/review", post(review::start_review)) .layer(from_fn_with_state( deployment.clone(), load_session_middleware, diff --git a/crates/server/src/routes/sessions/review.rs b/crates/server/src/routes/sessions/review.rs new file mode 100644 index 00000000..3290c5d6 --- /dev/null +++ b/crates/server/src/routes/sessions/review.rs @@ -0,0 +1,138 @@ +use std::path::PathBuf; + +use axum::{Extension, Json, extract::State, response::Json as ResponseJson}; +use db::models::{ + execution_process::{ExecutionProcess, ExecutionProcessRunReason}, + session::Session, + workspace::{Workspace, WorkspaceError}, + workspace_repo::WorkspaceRepo, +}; +use deployment::Deployment; +use executors::{ + actions::{ + ExecutorAction, ExecutorActionType, + review::{RepoReviewContext as ExecutorRepoReviewContext, ReviewRequest as ReviewAction}, + }, + executors::build_review_prompt, + profile::ExecutorProfileId, +}; +use serde::{Deserialize, Serialize}; +use services::services::container::ContainerService; +use ts_rs::TS; +use utils::response::ApiResponse; + +use crate::{DeploymentImpl, error::ApiError}; + +#[derive(Debug, Deserialize, Serialize, TS)] +pub struct StartReviewRequest { + pub executor_profile_id: ExecutorProfileId, + pub additional_prompt: Option, + #[serde(default)] + pub use_all_workspace_commits: bool, +} + +#[derive(Debug, Serialize, Deserialize, TS)] +#[serde(tag = "type", rename_all = "snake_case")] +#[ts(tag = "type", rename_all = "snake_case")] +pub enum ReviewError { + ProcessAlreadyRunning, +} + +#[axum::debug_handler] +pub async fn start_review( + Extension(session): Extension, + State(deployment): State, + Json(payload): Json, +) -> Result>, ApiError> { + let pool = &deployment.db().pool; + + let workspace = Workspace::find_by_id(pool, session.workspace_id) + .await? + .ok_or(ApiError::Workspace(WorkspaceError::ValidationError( + "Workspace not found".to_string(), + )))?; + + if ExecutionProcess::has_running_non_dev_server_processes_for_workspace(pool, workspace.id) + .await? + { + return Ok(ResponseJson(ApiResponse::error_with_data( + ReviewError::ProcessAlreadyRunning, + ))); + } + + let container_ref = deployment + .container() + .ensure_container_exists(&workspace) + .await?; + + let agent_session_id = + ExecutionProcess::find_latest_coding_agent_turn_session_id(pool, session.id).await?; + + let context: Option> = if payload.use_all_workspace_commits { + let repos = + WorkspaceRepo::find_repos_with_target_branch_for_workspace(pool, workspace.id).await?; + let workspace_path = PathBuf::from(container_ref.as_str()); + + let mut contexts = Vec::new(); + for repo in repos { + let worktree_path = workspace_path.join(&repo.repo.name); + if let Ok(base_commit) = deployment.git().get_fork_point( + &worktree_path, + &repo.target_branch, + &workspace.branch, + ) { + contexts.push(ExecutorRepoReviewContext { + repo_id: repo.repo.id, + repo_name: repo.repo.display_name, + base_commit, + }); + } + } + if contexts.is_empty() { + None + } else { + Some(contexts) + } + } else { + None + }; + + let prompt = build_review_prompt(context.as_deref(), payload.additional_prompt.as_deref()); + let resumed_session = agent_session_id.is_some(); + + let action = ExecutorAction::new( + ExecutorActionType::ReviewRequest(ReviewAction { + executor_profile_id: payload.executor_profile_id.clone(), + context, + prompt, + session_id: agent_session_id, + working_dir: workspace.agent_working_dir.clone(), + }), + None, + ); + + let execution_process = deployment + .container() + .start_execution( + &workspace, + &session, + &action, + &ExecutionProcessRunReason::CodingAgent, + ) + .await?; + + deployment + .track_if_analytics_allowed( + "review_started", + serde_json::json!({ + "workspace_id": workspace.id.to_string(), + "session_id": session.id.to_string(), + "executor": payload.executor_profile_id.executor.to_string(), + "variant": payload.executor_profile_id.variant, + "resumed_session": resumed_session, + }), + ) + .await; + + Ok(ResponseJson(ApiResponse::success(execution_process))) +} diff --git a/crates/services/src/services/container.rs b/crates/services/src/services/container.rs index 094b2b5b..b0775cc4 100644 --- a/crates/services/src/services/container.rs +++ b/crates/services/src/services/container.rs @@ -766,6 +766,17 @@ pub trait ContainerService { ); } } + #[cfg(feature = "qa-mode")] + ExecutorActionType::ReviewRequest(_request) => { + let executor = QaMockExecutor; + executor.normalize_logs(temp_store.clone(), ¤t_dir); + } + #[cfg(not(feature = "qa-mode"))] + ExecutorActionType::ReviewRequest(request) => { + let executor = ExecutorConfigs::get_cached() + .get_coding_agent_or_default(&request.executor_profile_id); + executor.normalize_logs(temp_store.clone(), ¤t_dir); + } _ => { tracing::debug!( "Executor action doesn't support log normalization: {:?}", @@ -1029,7 +1040,10 @@ pub trait ContainerService { ExecutorActionType::CodingAgentFollowUpRequest(follow_up_request) => { Some(follow_up_request.prompt.clone()) } - _ => None, + ExecutorActionType::ReviewRequest(review_request) => { + Some(review_request.prompt.clone()) + } + ExecutorActionType::ScriptRequest(_) => None, } { let create_coding_agent_turn = CreateCodingAgentTurn { execution_process_id: execution_process.id, @@ -1117,6 +1131,10 @@ pub trait ContainerService { &request.executor_profile_id, request.effective_dir(&workspace_root), )), + ExecutorActionType::ReviewRequest(request) => Some(( + &request.executor_profile_id, + request.effective_dir(&workspace_root), + )), _ => None, } { @@ -1160,13 +1178,15 @@ pub trait ContainerService { } ( ExecutorActionType::CodingAgentInitialRequest(_) - | ExecutorActionType::CodingAgentFollowUpRequest(_), + | ExecutorActionType::CodingAgentFollowUpRequest(_) + | ExecutorActionType::ReviewRequest(_), ExecutorActionType::ScriptRequest(_), ) => ExecutionProcessRunReason::CleanupScript, ( _, ExecutorActionType::CodingAgentFollowUpRequest(_) - | ExecutorActionType::CodingAgentInitialRequest(_), + | ExecutorActionType::CodingAgentInitialRequest(_) + | ExecutorActionType::ReviewRequest(_), ) => ExecutionProcessRunReason::CodingAgent, }; diff --git a/crates/services/src/services/git.rs b/crates/services/src/services/git.rs index 5562ec0a..1b129126 100644 --- a/crates/services/src/services/git.rs +++ b/crates/services/src/services/git.rs @@ -1054,6 +1054,16 @@ impl GitService { Ok(oid) } + pub fn get_fork_point( + &self, + worktree_path: &Path, + target_branch: &str, + task_branch: &str, + ) -> Result { + let git = GitCli::new(); + Ok(git.merge_base(worktree_path, target_branch, task_branch)?) + } + /// Get the subject/summary line for a given commit OID pub fn get_commit_subject( &self, diff --git a/crates/services/src/services/git/cli.rs b/crates/services/src/services/git/cli.rs index d3b4d56c..23134dbc 100644 --- a/crates/services/src/services/git/cli.rs +++ b/crates/services/src/services/git/cli.rs @@ -478,7 +478,12 @@ impl GitCli { /// Return the merge base commit sha of two refs in the given worktree. /// If `git merge-base --fork-point` fails, falls back to regular `merge-base`. - fn merge_base(&self, worktree_path: &Path, a: &str, b: &str) -> Result { + pub fn merge_base( + &self, + worktree_path: &Path, + a: &str, + b: &str, + ) -> Result { let out = self .git(worktree_path, ["merge-base", "--fork-point", a, b]) .unwrap_or(self.git(worktree_path, ["merge-base", a, b])?); diff --git a/frontend/src/components/dialogs/index.ts b/frontend/src/components/dialogs/index.ts index 8932454d..94d53739 100644 --- a/frontend/src/components/dialogs/index.ts +++ b/frontend/src/components/dialogs/index.ts @@ -93,6 +93,10 @@ export { type EditBranchNameDialogResult, } from './tasks/EditBranchNameDialog'; export { CreateAttemptDialog } from './tasks/CreateAttemptDialog'; +export { + StartReviewDialog, + type StartReviewDialogProps, +} from './tasks/StartReviewDialog'; // Auth dialogs export { GhCliSetupDialog } from './auth/GhCliSetupDialog'; diff --git a/frontend/src/components/dialogs/tasks/StartReviewDialog.tsx b/frontend/src/components/dialogs/tasks/StartReviewDialog.tsx new file mode 100644 index 00000000..e35bf7f6 --- /dev/null +++ b/frontend/src/components/dialogs/tasks/StartReviewDialog.tsx @@ -0,0 +1,289 @@ +import { useState, useCallback, useMemo } from 'react'; +import { useTranslation } from 'react-i18next'; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '@/components/ui/dialog'; +import { Button } from '@/components/ui/button'; +import { Textarea } from '@/components/ui/textarea'; +import { Label } from '@/components/ui/label'; +import { Switch } from '@/components/ui/switch'; +import { Checkbox } from '@/components/ui/checkbox'; +import { AgentSelector } from '@/components/tasks/AgentSelector'; +import { ConfigSelector } from '@/components/tasks/ConfigSelector'; +import { useUserSystem } from '@/components/ConfigProvider'; +import { useWorkspaceContext } from '@/contexts/WorkspaceContext'; +import { sessionsApi } from '@/lib/api'; +import { useQueryClient } from '@tanstack/react-query'; +import NiceModal, { useModal } from '@ebay/nice-modal-react'; +import { defineModal } from '@/lib/modals'; +import type { BaseCodingAgent, ExecutorProfileId } from 'shared/types'; + +export interface StartReviewDialogProps { + sessionId?: string; + workspaceId: string; + reviewMarkdown?: string; + defaultProfile?: ExecutorProfileId | null; + onSuccess?: (newSessionId?: string) => void; +} + +const StartReviewDialogImpl = NiceModal.create( + ({ sessionId, workspaceId, reviewMarkdown, defaultProfile, onSuccess }) => { + const modal = useModal(); + const queryClient = useQueryClient(); + const { profiles, config } = useUserSystem(); + const { sessions, selectedSession, selectedSessionId, selectSession } = + useWorkspaceContext(); + const { t } = useTranslation(['tasks', 'common']); + + const resolvedSessionId = sessionId ?? selectedSessionId; + const resolvedSession = useMemo(() => { + if (!resolvedSessionId) return selectedSession ?? null; + return ( + sessions.find((session) => session.id === resolvedSessionId) ?? + selectedSession ?? + null + ); + }, [sessions, resolvedSessionId, selectedSession]); + const sessionExecutor = resolvedSession?.executor as BaseCodingAgent | null; + + const resolvedDefaultProfile = useMemo(() => { + if (defaultProfile) return defaultProfile; + if (sessionExecutor) { + const variant = + config?.executor_profile?.executor === sessionExecutor + ? config.executor_profile.variant + : null; + return { executor: sessionExecutor, variant }; + } + return config?.executor_profile ?? null; + }, [defaultProfile, sessionExecutor, config?.executor_profile]); + + const [userSelectedProfile, setUserSelectedProfile] = + useState(null); + const [additionalPrompt, setAdditionalPrompt] = useState(''); + const [createNewSession, setCreateNewSession] = useState( + () => !resolvedSessionId + ); + const [includeGitContext, setIncludeGitContext] = useState(true); + const [isSubmitting, setIsSubmitting] = useState(false); + const [error, setError] = useState(null); + + const effectiveProfile = userSelectedProfile ?? resolvedDefaultProfile; + + const canSubmit = Boolean(effectiveProfile && !isSubmitting); + + const handleSubmit = useCallback(async () => { + if (!effectiveProfile) return; + + setIsSubmitting(true); + setError(null); + + try { + let targetSessionId = resolvedSessionId; + + if (createNewSession || !resolvedSessionId) { + const session = await sessionsApi.create({ + workspace_id: workspaceId, + executor: effectiveProfile.executor, + }); + targetSessionId = session.id; + + queryClient.invalidateQueries({ + queryKey: ['workspaceSessions', workspaceId], + }); + } + + if (!targetSessionId) { + setError('Failed to create session'); + setIsSubmitting(false); + return; + } + + const promptParts = [reviewMarkdown, additionalPrompt].filter(Boolean); + const combinedPrompt = promptParts.join('\n\n'); + + await sessionsApi.startReview(targetSessionId, { + executor_profile_id: effectiveProfile, + additional_prompt: combinedPrompt || null, + use_all_workspace_commits: includeGitContext, + }); + + queryClient.invalidateQueries({ + queryKey: ['processes', workspaceId], + }); + queryClient.invalidateQueries({ + queryKey: ['branchStatus', workspaceId], + }); + + const createdNewSession = targetSessionId !== resolvedSessionId; + if (createdNewSession && targetSessionId) { + selectSession(targetSessionId); + } + onSuccess?.(createdNewSession ? targetSessionId : undefined); + modal.hide(); + } catch (err) { + console.error('Failed to start review:', err); + setError('Failed to start review. Please try again.'); + } finally { + setIsSubmitting(false); + } + }, [ + effectiveProfile, + resolvedSessionId, + workspaceId, + createNewSession, + includeGitContext, + reviewMarkdown, + additionalPrompt, + queryClient, + selectSession, + onSuccess, + modal, + ]); + + const handleOpenChange = (open: boolean) => { + if (!open) modal.hide(); + }; + + const handleNewSessionChange = (checked: boolean) => { + setCreateNewSession(checked); + if (!checked && resolvedDefaultProfile) { + setUserSelectedProfile(resolvedDefaultProfile); + } + }; + + const hasReviewComments = Boolean(reviewMarkdown); + + return ( + + + + {t('startReviewDialog.title')} + + {t('startReviewDialog.description')} + + + +
+
+ +