From 0e57cf34403c90e87665fc81e1994981df65161b Mon Sep 17 00:00:00 2001 From: Solomon Date: Mon, 15 Dec 2025 12:54:08 +0000 Subject: [PATCH] Approvals for ACP-based executors (#1511) * Approvals for ACP-based executors Gemini, Qwen, and Opencode * set all permissions to "ask" * use `normalize_unified_diff` in other log normalizers --- crates/executors/default_profiles.json | 22 ++- crates/executors/src/env.rs | 4 + crates/executors/src/executors/acp/client.rs | 161 +++++++++++++++--- crates/executors/src/executors/acp/harness.rs | 107 +++++++++--- crates/executors/src/executors/acp/mod.rs | 8 + .../src/executors/acp/normalize_logs.rs | 86 +++++++++- crates/executors/src/executors/acp/session.rs | 1 + .../src/executors/codex/normalize_logs.rs | 7 +- crates/executors/src/executors/cursor.rs | 8 +- .../src/executors/droid/normalize_logs.rs | 7 +- crates/executors/src/executors/gemini.rs | 33 +++- crates/executors/src/executors/opencode.rs | 48 +++++- crates/executors/src/executors/qwen.rs | 33 +++- crates/local-deployment/src/container.rs | 20 ++- crates/utils/src/diff.rs | 6 + shared/schemas/opencode.json | 5 + shared/types.ts | 6 +- 17 files changed, 467 insertions(+), 95 deletions(-) diff --git a/crates/executors/default_profiles.json b/crates/executors/default_profiles.json index 66146857..7a01330b 100644 --- a/crates/executors/default_profiles.json +++ b/crates/executors/default_profiles.json @@ -46,6 +46,11 @@ "model": "gemini-3-pro-preview", "yolo": true } + }, + "APPROVALS": { + "GEMINI": { + "yolo": false + } } }, "CODEX": { @@ -78,11 +83,19 @@ }, "OPENCODE": { "DEFAULT": { - "OPENCODE": {} + "OPENCODE": { + "auto_approve": true + } }, "PLAN": { "OPENCODE": { - "mode": "plan" + "mode": "plan", + "auto_approve": true + } + }, + "APPROVALS": { + "OPENCODE": { + "auto_approve": false } } }, @@ -91,6 +104,11 @@ "QWEN_CODE": { "yolo": true } + }, + "APPROVALS": { + "QWEN_CODE": { + "yolo": false + } } }, "CURSOR_AGENT": { diff --git a/crates/executors/src/env.rs b/crates/executors/src/env.rs index cdc2b4f7..a3e3ab4c 100644 --- a/crates/executors/src/env.rs +++ b/crates/executors/src/env.rs @@ -49,6 +49,10 @@ impl ExecutionEnv { command.env(key, value); } } + + pub fn contains_key(&self, key: &str) -> bool { + self.vars.contains_key(key) + } } #[cfg(test)] diff --git a/crates/executors/src/executors/acp/client.rs b/crates/executors/src/executors/acp/client.rs index fcacb42f..7d1216c1 100644 --- a/crates/executors/src/executors/acp/client.rs +++ b/crates/executors/src/executors/acp/client.rs @@ -1,19 +1,35 @@ -use agent_client_protocol as acp; -use async_trait::async_trait; -use tokio::sync::mpsc; -use tracing::{debug, warn}; +use std::sync::Arc; -use crate::executors::acp::AcpEvent; +use agent_client_protocol::{self as acp, ErrorCode}; +use async_trait::async_trait; +use tokio::sync::{Mutex, mpsc}; +use tracing::{debug, warn}; +use workspace_utils::approvals::ApprovalStatus; + +use crate::{ + approvals::{ExecutorApprovalError, ExecutorApprovalService}, + executors::acp::{AcpEvent, ApprovalResponse}, +}; /// ACP client that handles agent-client protocol communication +#[derive(Clone)] pub struct AcpClient { event_tx: mpsc::UnboundedSender, + approvals: Option>, + feedback_queue: Arc>>, } impl AcpClient { /// Create a new ACP client - pub fn new(event_tx: mpsc::UnboundedSender) -> Self { - Self { event_tx } + pub fn new( + event_tx: mpsc::UnboundedSender, + approvals: Option>, + ) -> Self { + Self { + event_tx, + approvals, + feedback_queue: Arc::new(Mutex::new(Vec::new())), + } } pub fn record_user_prompt_event(&self, prompt: &str) { @@ -26,6 +42,21 @@ impl AcpClient { warn!("Failed to send ACP event: {}", e); } } + + /// Queue a user feedback message to be sent after a denial. + pub async fn enqueue_feedback(&self, message: String) { + let trimmed = message.trim().to_string(); + if !trimmed.is_empty() { + let mut q = self.feedback_queue.lock().await; + q.push(trimmed); + } + } + + /// Drain and return queued feedback messages. + pub async fn drain_feedback(&self) -> Vec { + let mut q = self.feedback_queue.lock().await; + q.drain(..).collect() + } } #[async_trait(?Send)] @@ -34,31 +65,107 @@ impl acp::Client for AcpClient { &self, args: acp::RequestPermissionRequest, ) -> Result { - // Forward the request as an event self.send_event(AcpEvent::RequestPermission(args.clone())); - // Auto-approve with best available option - let chosen_option = args - .options - .iter() - .find(|o| matches!(o.kind, acp::PermissionOptionKind::AllowAlways)) - .or_else(|| { - args.options - .iter() - .find(|o| matches!(o.kind, acp::PermissionOptionKind::AllowOnce)) - }) - .or_else(|| args.options.first()); + if self.approvals.is_none() { + // Auto-approve with best available option when no approval service is configured + let chosen_option = args + .options + .iter() + .find(|o| matches!(o.kind, acp::PermissionOptionKind::AllowAlways)) + .or_else(|| { + args.options + .iter() + .find(|o| matches!(o.kind, acp::PermissionOptionKind::AllowOnce)) + }) + .or_else(|| args.options.first()); - let outcome = if let Some(opt) = chosen_option { - debug!("Auto-approving permission with option: {}", opt.option_id); - acp::RequestPermissionOutcome::Selected(acp::SelectedPermissionOutcome::new( - opt.option_id.clone(), - )) - } else { - warn!("No permission options available, cancelling"); - acp::RequestPermissionOutcome::Cancelled + let outcome = if let Some(opt) = chosen_option { + debug!("Auto-approving permission with option: {}", opt.option_id); + acp::RequestPermissionOutcome::Selected(acp::SelectedPermissionOutcome::new( + opt.option_id.clone(), + )) + } else { + warn!("No permission options available, cancelling"); + acp::RequestPermissionOutcome::Cancelled + }; + + return Ok(acp::RequestPermissionResponse::new(outcome)); + } + + let tool_call_id = args.tool_call.tool_call_id.0.to_string(); + let status = match self + .approvals + .as_ref() + .ok_or(ExecutorApprovalError::ServiceUnavailable) + .map_err(|_| acp::Error::invalid_request())? + .request_tool_approval( + args.tool_call.fields.title.as_deref().unwrap_or("tool"), + serde_json::json!({ "tool_call": args.tool_call }), + &tool_call_id, + ) + .await + { + Ok(s) => s, + Err(err) => { + warn!("Failed to request tool approval: {}", err); + return Err(acp::Error::new( + ErrorCode::INTERNAL_ERROR.code, + format!("Approval request failed: {}", err), + )); + } }; + // Map our ApprovalStatus to ACP outcome + let outcome = match &status { + ApprovalStatus::Approved => { + let chosen = args + .options + .iter() + .find(|o| matches!(o.kind, acp::PermissionOptionKind::AllowOnce)); + if let Some(opt) = chosen { + acp::RequestPermissionOutcome::Selected(acp::SelectedPermissionOutcome::new( + opt.option_id.clone(), + )) + } else { + tracing::error!("No suitable approval option found, cancelling"); + return Err(acp::Error::invalid_request()); + } + } + ApprovalStatus::Denied { reason } => { + // If user provided a reason, queue it to send after denial + if let Some(feedback) = reason.as_ref() { + self.enqueue_feedback(feedback.clone()).await; + } + let chosen = args + .options + .iter() + .find(|o| matches!(o.kind, acp::PermissionOptionKind::RejectOnce)); + if let Some(opt) = chosen { + acp::RequestPermissionOutcome::Selected(acp::SelectedPermissionOutcome::new( + opt.option_id.clone(), + )) + } else { + warn!("No permission options for denial, cancelling"); + acp::RequestPermissionOutcome::Cancelled + } + } + ApprovalStatus::TimedOut => { + warn!("Approval timed out"); + acp::RequestPermissionOutcome::Cancelled + } + ApprovalStatus::Pending => { + // This should not occur after waiter resolves + warn!("Approval resolved to Pending"); + acp::RequestPermissionOutcome::Cancelled + } + }; + + self.send_event(AcpEvent::ApprovalResponse(ApprovalResponse { + tool_call_id: tool_call_id.clone(), + status: status.clone(), + })); + Ok(acp::RequestPermissionResponse::new(outcome)) } diff --git a/crates/executors/src/executors/acp/harness.rs b/crates/executors/src/executors/acp/harness.rs index f987d832..e42fd4d4 100644 --- a/crates/executors/src/executors/acp/harness.rs +++ b/crates/executors/src/executors/acp/harness.rs @@ -1,6 +1,7 @@ use std::{ path::{Path, PathBuf}, process::Stdio, + rc::Rc, sync::Arc, }; @@ -14,10 +15,11 @@ use tokio_util::{ io::ReaderStream, }; use tracing::error; -use workspace_utils::stream_lines::LinesStreamExt; +use workspace_utils::{approvals::ApprovalStatus, stream_lines::LinesStreamExt}; use super::{AcpClient, SessionManager}; use crate::{ + approvals::ExecutorApprovalService, command::{CmdOverrides, CommandParts}, env::ExecutionEnv, executors::{ExecutorError, ExecutorExitResult, SpawnedChild, acp::AcpEvent}, @@ -73,6 +75,7 @@ impl AcpAgentHarness { command_parts: CommandParts, env: &ExecutionEnv, cmd_overrides: &CmdOverrides, + approvals: Option>, ) -> Result { let (program_path, args) = command_parts.into_resolved().await?; let mut command = Command::new(program_path); @@ -101,6 +104,7 @@ impl AcpAgentHarness { self.session_namespace.clone(), self.model.clone(), self.mode.clone(), + approvals, ) .await?; @@ -111,6 +115,7 @@ impl AcpAgentHarness { }) } + #[allow(clippy::too_many_arguments)] pub async fn spawn_follow_up_with_command( &self, current_dir: &Path, @@ -119,6 +124,7 @@ impl AcpAgentHarness { command_parts: CommandParts, env: &ExecutionEnv, cmd_overrides: &CmdOverrides, + approvals: Option>, ) -> Result { let (program_path, args) = command_parts.into_resolved().await?; let mut command = Command::new(program_path); @@ -147,6 +153,7 @@ impl AcpAgentHarness { self.session_namespace.clone(), self.model.clone(), self.mode.clone(), + approvals, ) .await?; @@ -167,6 +174,7 @@ impl AcpAgentHarness { session_namespace: String, model: Option, mode: Option, + approvals: Option>, ) -> Result<(), ExecutorError> { // Take child's stdio for ACP wiring let orig_stdout = child.inner().stdout.take().ok_or_else(|| { @@ -281,8 +289,9 @@ impl AcpAgentHarness { }; let session_manager = std::sync::Arc::new(session_manager); - // Create ACP client - let client = AcpClient::new(event_tx.clone()); + // Create ACP client with approvals support + let client = AcpClient::new(event_tx.clone(), approvals.clone()); + let client_feedback_handle = client.clone(); client.record_user_prompt_event(&prompt); @@ -291,6 +300,7 @@ impl AcpAgentHarness { proto::ClientSideConnection::new(client, outgoing, incoming, |fut| { tokio::task::spawn_local(fut); }); + let conn = Rc::new(conn); // Drive I/O let io_handle = tokio::task::spawn_local(async move { @@ -382,13 +392,30 @@ impl AcpAgentHarness { let app_tx_clone = log_tx.clone(); let sess_id_for_writer = display_session_id.clone(); let sm_for_writer = session_manager.clone(); - tokio::spawn(async move { + let conn_for_cancel = conn.clone(); + let acp_session_id_for_cancel = acp_session_id.clone(); + tokio::task::spawn_local(async move { while let Some(event) = event_rx.recv().await { + if let AcpEvent::ApprovalResponse(resp) = &event + && let ApprovalStatus::Denied { + reason: Some(reason), + } = &resp.status + && !reason.trim().is_empty() + { + let _ = conn_for_cancel + .cancel(proto::CancelNotification::new( + proto::SessionId::new( + acp_session_id_for_cancel.clone(), + ), + )) + .await; + } + + let line = event.to_string(); // Forward to stdout - let _ = app_tx_clone.send(event.to_string()); + let _ = app_tx_clone.send(line.clone()); // Persist to session file - let _ = sm_for_writer - .append_raw_line(&sess_id_for_writer, &event.to_string()); + let _ = sm_for_writer.append_raw_line(&sess_id_for_writer, &line); } }); @@ -400,35 +427,61 @@ impl AcpAgentHarness { ); // Build prompt request - let req = proto::PromptRequest::new( + let initial_req = proto::PromptRequest::new( proto::SessionId::new(acp_session_id.clone()), vec![proto::ContentBlock::Text(proto::TextContent::new( prompt_to_send, ))], ); - // Send the prompt and await completion to obtain stop_reason - match conn.prompt(req).await { - Ok(resp) => { - // Emit done with stop_reason - let stop_reason = - serde_json::to_string(&resp.stop_reason).unwrap_or_default(); - let _ = log_tx.send(AcpEvent::Done(stop_reason).to_string()); - } - Err(e) => { - tracing::debug!("error {} {e} {:?}", e.code, e.data); - if e.code == agent_client_protocol::ErrorCode::INTERNAL_ERROR.code - && e.data - .as_ref() - .is_some_and(|d| d == "server shut down unexpectedly") - { - tracing::debug!("ACP server killed"); - } else { - let _ = - log_tx.send(AcpEvent::Error(format!("{e}")).to_string()); + let mut current_req = Some(initial_req); + + while let Some(req) = current_req.take() { + tracing::trace!(?req, "sending ACP prompt request"); + // Send the prompt and await completion to obtain stop_reason + match conn.prompt(req).await { + Ok(resp) => { + // Emit done with stop_reason + let stop_reason = serde_json::to_string(&resp.stop_reason) + .unwrap_or_default(); + let _ = log_tx.send(AcpEvent::Done(stop_reason).to_string()); + } + Err(e) => { + tracing::debug!("error {} {e} {:?}", e.code, e.data); + if e.code + == agent_client_protocol::ErrorCode::INTERNAL_ERROR.code + && e.data + .as_ref() + .is_some_and(|d| d == "server shut down unexpectedly") + { + tracing::debug!("ACP server killed"); + } else { + let _ = log_tx + .send(AcpEvent::Error(format!("{e}")).to_string()); + } } } + + // Flush any pending user feedback after finish + let feedback = client_feedback_handle + .drain_feedback() + .await + .join("\n") + .trim() + .to_string(); + if !feedback.is_empty() { + tracing::trace!(?feedback, "sending ACP follow-up feedback"); + let session_id = proto::SessionId::new(acp_session_id.clone()); + let feedback_req = proto::PromptRequest::new( + session_id.clone(), + vec![proto::ContentBlock::Text(proto::TextContent::new( + feedback, + ))], + ); + current_req = Some(feedback_req); + } } + // Notify container of completion if let Some(tx) = exit_signal_tx.take() { let _ = tx.send(ExecutorExitResult::Success); diff --git a/crates/executors/src/executors/acp/mod.rs b/crates/executors/src/executors/acp/mod.rs index a1ccb4b6..7eb9cb8a 100644 --- a/crates/executors/src/executors/acp/mod.rs +++ b/crates/executors/src/executors/acp/mod.rs @@ -10,6 +10,7 @@ pub use harness::AcpAgentHarness; pub use normalize_logs::*; use serde::{Deserialize, Serialize}; pub use session::SessionManager; +use workspace_utils::approvals::ApprovalStatus; /// Parsed event types for internal processing #[derive(Debug, Clone, Serialize, Deserialize)] @@ -24,6 +25,7 @@ pub enum AcpEvent { AvailableCommands(Vec), CurrentMode(agent_client_protocol::SessionModeId), RequestPermission(agent_client_protocol::RequestPermissionRequest), + ApprovalResponse(ApprovalResponse), Error(String), Done(String), Other(agent_client_protocol::SessionNotification), @@ -42,3 +44,9 @@ impl FromStr for AcpEvent { serde_json::from_str(s) } } + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ApprovalResponse { + pub tool_call_id: String, + pub status: ApprovalStatus, +} diff --git a/crates/executors/src/executors/acp/normalize_logs.rs b/crates/executors/src/executors/acp/normalize_logs.rs index 64da1369..446af9b1 100644 --- a/crates/executors/src/executors/acp/normalize_logs.rs +++ b/crates/executors/src/executors/acp/normalize_logs.rs @@ -8,15 +8,18 @@ use agent_client_protocol::{self as acp, SessionNotification}; use futures::StreamExt; use regex::Regex; use serde::Deserialize; -use workspace_utils::msg_store::MsgStore; +use workspace_utils::{approvals::ApprovalStatus, msg_store::MsgStore}; pub use super::AcpAgentHarness; use super::AcpEvent; -use crate::logs::{ - ActionType, FileChange, NormalizedEntry, NormalizedEntryError, NormalizedEntryType, TodoItem, - ToolResult, ToolResultValueType, ToolStatus as LogToolStatus, - stderr_processor::normalize_stderr_logs, - utils::{ConversationPatch, EntryIndexProvider}, +use crate::{ + approvals::ToolCallMetadata, + logs::{ + ActionType, FileChange, NormalizedEntry, NormalizedEntryError, NormalizedEntryType, + TodoItem, ToolResult, ToolResultValueType, ToolStatus as LogToolStatus, + stderr_processor::normalize_stderr_logs, + utils::{ConversationPatch, EntryIndexProvider}, + }, }; pub fn normalize_logs(msg_store: Arc, worktree_path: &Path) { @@ -220,6 +223,35 @@ pub fn normalize_logs(msg_store: Arc, worktree_path: &Path) { tracing::debug!("Failed to convert tool call update to ToolCall"); } } + AcpEvent::ApprovalResponse(resp) => { + tracing::trace!("Received approval response: {:?}", resp); + if let ApprovalStatus::Denied { reason } = resp.status { + let tool_name = tool_states + .get(&resp.tool_call_id) + .map(|t| { + extract_tool_name_from_id(t.id.0.as_ref()) + .unwrap_or_else(|| t.title.clone()) + }) + .unwrap_or_default(); + let idx = entry_index.next(); + let entry = NormalizedEntry { + timestamp: None, + entry_type: NormalizedEntryType::UserFeedback { + denied_tool: tool_name, + }, + content: reason + .clone() + .unwrap_or_else(|| { + "User denied this tool use request".to_string() + }) + .trim() + .to_string(), + metadata: None, + }; + msg_store + .push_patch(ConversationPatch::add_normalized_entry(idx, entry)); + } + } AcpEvent::User(_) | AcpEvent::Other(_) => (), } } @@ -251,7 +283,10 @@ pub fn normalize_logs(msg_store: Arc, worktree_path: &Path) { status: convert_tool_status(&tool_data.status), }, content: get_tool_content(tool_data), - metadata: None, + metadata: serde_json::to_value(ToolCallMetadata { + tool_call_id: tool_data.id.0.to_string(), + }) + .ok(), }; let patch = if is_new { ConversationPatch::add_normalized_entry(tool_data.index, entry) @@ -461,6 +496,31 @@ pub fn normalize_logs(msg_store: Arc, worktree_path: &Path) { } } } + if changes.is_empty() + && let Some(raw) = &tc.raw_input + && let Ok(edit_input) = serde_json::from_value::(raw.clone()) + { + if let Some(diff) = edit_input.diff { + changes.push(FileChange::Edit { + unified_diff: workspace_utils::diff::normalize_unified_diff( + &edit_input.file_path, + &diff, + ), + has_line_numbers: true, + }); + } else if let Some(old) = edit_input.old_string + && let Some(new) = edit_input.new_string + { + changes.push(FileChange::Edit { + unified_diff: workspace_utils::diff::create_unified_diff( + &edit_input.file_path, + &old, + &new, + ), + has_line_numbers: false, + }); + } + } changes } @@ -687,3 +747,15 @@ struct StreamingText { index: usize, content: String, } + +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +struct EditInput { + file_path: String, + #[serde(default)] + diff: Option, + #[serde(default)] + old_string: Option, + #[serde(default)] + new_string: Option, +} diff --git a/crates/executors/src/executors/acp/session.rs b/crates/executors/src/executors/acp/session.rs index 6fc15acc..62640c92 100644 --- a/crates/executors/src/executors/acp/session.rs +++ b/crates/executors/src/executors/acp/session.rs @@ -83,6 +83,7 @@ impl SessionManager { | AcpEvent::ToolUpdate(..) | AcpEvent::Plan(..) | AcpEvent::AvailableCommands(..) + | AcpEvent::ApprovalResponse(..) | AcpEvent::CurrentMode(..) => {} AcpEvent::RequestPermission(req) => event = AcpEvent::ToolUpdate(req.tool_call), diff --git a/crates/executors/src/executors/codex/normalize_logs.rs b/crates/executors/src/executors/codex/normalize_logs.rs index 39b3618a..37329d7c 100644 --- a/crates/executors/src/executors/codex/normalize_logs.rs +++ b/crates/executors/src/executors/codex/normalize_logs.rs @@ -26,9 +26,7 @@ use regex::Regex; use serde::{Deserialize, Serialize}; use serde_json::Value; use workspace_utils::{ - approvals::ApprovalStatus, - diff::{concatenate_diff_hunks, extract_unified_diff_hunks}, - msg_store::MsgStore, + approvals::ApprovalStatus, diff::normalize_unified_diff, msg_store::MsgStore, path::make_path_relative, }; @@ -337,8 +335,7 @@ fn normalize_file_changes( make_path_relative(dest.to_string_lossy().as_ref(), worktree_path); edits.push(FileChange::Rename { new_path: dest_rel }); } - let hunks = extract_unified_diff_hunks(unified_diff); - let diff = concatenate_diff_hunks(&relative, &hunks); + let diff = normalize_unified_diff(&relative, unified_diff); edits.push(FileChange::Edit { unified_diff: diff, has_line_numbers: true, diff --git a/crates/executors/src/executors/cursor.rs b/crates/executors/src/executors/cursor.rs index cde3ae0c..e8a2ef5b 100644 --- a/crates/executors/src/executors/cursor.rs +++ b/crates/executors/src/executors/cursor.rs @@ -9,7 +9,7 @@ use serde::{Deserialize, Serialize}; use tokio::{io::AsyncWriteExt, process::Command}; use ts_rs::TS; use workspace_utils::{ - diff::{concatenate_diff_hunks, create_unified_diff, extract_unified_diff_hunks}, + diff::{create_unified_diff, normalize_unified_diff}, msg_store::MsgStore, path::make_path_relative, shell::resolve_executable_path_blocking, @@ -734,9 +734,8 @@ impl CursorToolCall { let mut changes = vec![]; if let Some(apply_patch) = &args.apply_patch { - let hunks = extract_unified_diff_hunks(&apply_patch.patch_content); changes.push(FileChange::Edit { - unified_diff: concatenate_diff_hunks(&path, &hunks), + unified_diff: normalize_unified_diff(&path, &apply_patch.patch_content), has_line_numbers: false, }); } @@ -774,9 +773,8 @@ impl CursorToolCall { .. })) = &result { - let hunks = extract_unified_diff_hunks(diff_string); changes.push(FileChange::Edit { - unified_diff: concatenate_diff_hunks(&path, &hunks), + unified_diff: normalize_unified_diff(&path, diff_string), has_line_numbers: false, }); } diff --git a/crates/executors/src/executors/droid/normalize_logs.rs b/crates/executors/src/executors/droid/normalize_logs.rs index fa241d1e..0fdd4ec8 100644 --- a/crates/executors/src/executors/droid/normalize_logs.rs +++ b/crates/executors/src/executors/droid/normalize_logs.rs @@ -8,9 +8,7 @@ use futures::{StreamExt, future::ready}; use serde::{Deserialize, Serialize}; use serde_json::Value; use workspace_utils::{ - diff::{concatenate_diff_hunks, extract_unified_diff_hunks}, - msg_store::MsgStore, - path::make_path_relative, + diff::normalize_unified_diff, msg_store::MsgStore, path::make_path_relative, }; use crate::logs::{ @@ -767,9 +765,8 @@ fn parse_apply_patch_result(value: &Value, worktree_path: &str) -> Option, #[serde(flatten)] pub cmd: CmdOverrides, + #[serde(skip)] + #[ts(skip)] + #[derivative(Debug = "ignore", PartialEq = "ignore")] + pub approvals: Option>, } impl Gemini { @@ -49,6 +56,10 @@ impl Gemini { #[async_trait] impl StandardCodingAgentExecutor for Gemini { + fn use_approvals(&mut self, approvals: Arc) { + self.approvals = Some(approvals); + } + async fn spawn( &self, current_dir: &Path, @@ -58,8 +69,20 @@ impl StandardCodingAgentExecutor for Gemini { let harness = AcpAgentHarness::new(); let combined_prompt = self.append_prompt.combine_prompt(prompt); let gemini_command = self.build_command_builder().build_initial()?; + let approvals = if self.yolo.unwrap_or(false) { + None + } else { + self.approvals.clone() + }; harness - .spawn_with_command(current_dir, combined_prompt, gemini_command, env, &self.cmd) + .spawn_with_command( + current_dir, + combined_prompt, + gemini_command, + env, + &self.cmd, + approvals, + ) .await } @@ -73,6 +96,11 @@ impl StandardCodingAgentExecutor for Gemini { let harness = AcpAgentHarness::new(); let combined_prompt = self.append_prompt.combine_prompt(prompt); let gemini_command = self.build_command_builder().build_follow_up(&[])?; + let approvals = if self.yolo.unwrap_or(false) { + None + } else { + self.approvals.clone() + }; harness .spawn_follow_up_with_command( current_dir, @@ -81,6 +109,7 @@ impl StandardCodingAgentExecutor for Gemini { gemini_command, env, &self.cmd, + approvals, ) .await } diff --git a/crates/executors/src/executors/opencode.rs b/crates/executors/src/executors/opencode.rs index 2c2c402b..2f8e59ae 100644 --- a/crates/executors/src/executors/opencode.rs +++ b/crates/executors/src/executors/opencode.rs @@ -1,12 +1,14 @@ use std::{path::Path, sync::Arc}; use async_trait::async_trait; +use derivative::Derivative; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use ts_rs::TS; use workspace_utils::msg_store::MsgStore; use crate::{ + approvals::ExecutorApprovalService, command::{CmdOverrides, CommandBuilder, apply_overrides}, env::ExecutionEnv, executors::{ @@ -15,7 +17,8 @@ use crate::{ }, }; -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, TS, JsonSchema)] +#[derive(Derivative, Clone, Serialize, Deserialize, TS, JsonSchema)] +#[derivative(Debug, PartialEq)] pub struct Opencode { #[serde(default)] pub append_prompt: AppendPrompt, @@ -23,13 +26,20 @@ pub struct Opencode { pub model: Option, #[serde(default, skip_serializing_if = "Option::is_none", alias = "agent")] pub mode: Option, + /// Auto-approve agent actions + #[serde(default = "default_to_true")] + pub auto_approve: bool, #[serde(flatten)] pub cmd: CmdOverrides, + #[serde(skip)] + #[ts(skip)] + #[derivative(Debug = "ignore", PartialEq = "ignore")] + pub approvals: Option>, } impl Opencode { fn build_command_builder(&self) -> CommandBuilder { - let builder = CommandBuilder::new("npx -y opencode-ai@1.0.134 acp"); + let builder = CommandBuilder::new("npx -y opencode-ai@1.0.134").extend_params(["acp"]); apply_overrides(builder, &self.cmd) } @@ -40,6 +50,10 @@ impl Opencode { #[async_trait] impl StandardCodingAgentExecutor for Opencode { + fn use_approvals(&mut self, approvals: Arc) { + self.approvals = Some(approvals); + } + async fn spawn( &self, current_dir: &Path, @@ -56,13 +70,20 @@ impl StandardCodingAgentExecutor for Opencode { harness = harness.with_mode(agent); } let opencode_command = self.build_command_builder().build_initial()?; + let approvals = if self.auto_approve { + None + } else { + self.approvals.clone() + }; + let env = setup_approvals_env(self.auto_approve, env); harness .spawn_with_command( current_dir, combined_prompt, opencode_command, - env, + &env, &self.cmd, + approvals, ) .await } @@ -83,14 +104,21 @@ impl StandardCodingAgentExecutor for Opencode { harness = harness.with_mode(agent); } let opencode_command = self.build_command_builder().build_follow_up(&[])?; + let approvals = if self.auto_approve { + None + } else { + self.approvals.clone() + }; + let env = setup_approvals_env(self.auto_approve, env); harness .spawn_follow_up_with_command( current_dir, combined_prompt, session_id, opencode_command, - env, + &env, &self.cmd, + approvals, ) .await } @@ -127,3 +155,15 @@ impl StandardCodingAgentExecutor for Opencode { } } } + +fn default_to_true() -> bool { + true +} + +fn setup_approvals_env(auto_approve: bool, env: &ExecutionEnv) -> ExecutionEnv { + let mut env = env.clone(); + if !auto_approve && !env.contains_key("OPENCODE_PERMISSION") { + env.insert("OPENCODE_PERMISSION", r#"{"edit": "ask", "bash": "ask", "webfetch": "ask", "doom_loop": "ask", "external_directory": "ask"}"#); + } + env +} diff --git a/crates/executors/src/executors/qwen.rs b/crates/executors/src/executors/qwen.rs index e157cc24..fad59a97 100644 --- a/crates/executors/src/executors/qwen.rs +++ b/crates/executors/src/executors/qwen.rs @@ -1,12 +1,14 @@ use std::{path::Path, sync::Arc}; use async_trait::async_trait; +use derivative::Derivative; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use ts_rs::TS; use workspace_utils::msg_store::MsgStore; use crate::{ + approvals::ExecutorApprovalService, command::{CmdOverrides, CommandBuilder, apply_overrides}, env::ExecutionEnv, executors::{ @@ -15,7 +17,8 @@ use crate::{ }, }; -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, TS, JsonSchema)] +#[derive(Derivative, Clone, Serialize, Deserialize, TS, JsonSchema)] +#[derivative(Debug, PartialEq)] pub struct QwenCode { #[serde(default)] pub append_prompt: AppendPrompt, @@ -23,6 +26,10 @@ pub struct QwenCode { pub yolo: Option, #[serde(flatten)] pub cmd: CmdOverrides, + #[serde(skip)] + #[ts(skip)] + #[derivative(Debug = "ignore", PartialEq = "ignore")] + pub approvals: Option>, } impl QwenCode { @@ -39,6 +46,10 @@ impl QwenCode { #[async_trait] impl StandardCodingAgentExecutor for QwenCode { + fn use_approvals(&mut self, approvals: Arc) { + self.approvals = Some(approvals); + } + async fn spawn( &self, current_dir: &Path, @@ -48,8 +59,20 @@ impl StandardCodingAgentExecutor for QwenCode { let qwen_command = self.build_command_builder().build_initial()?; let combined_prompt = self.append_prompt.combine_prompt(prompt); let harness = AcpAgentHarness::with_session_namespace("qwen_sessions"); + let approvals = if self.yolo.unwrap_or(false) { + None + } else { + self.approvals.clone() + }; harness - .spawn_with_command(current_dir, combined_prompt, qwen_command, env, &self.cmd) + .spawn_with_command( + current_dir, + combined_prompt, + qwen_command, + env, + &self.cmd, + approvals, + ) .await } @@ -63,6 +86,11 @@ impl StandardCodingAgentExecutor for QwenCode { let qwen_command = self.build_command_builder().build_follow_up(&[])?; let combined_prompt = self.append_prompt.combine_prompt(prompt); let harness = AcpAgentHarness::with_session_namespace("qwen_sessions"); + let approvals = if self.yolo.unwrap_or(false) { + None + } else { + self.approvals.clone() + }; harness .spawn_follow_up_with_command( current_dir, @@ -71,6 +99,7 @@ impl StandardCodingAgentExecutor for QwenCode { qwen_command, env, &self.cmd, + approvals, ) .await } diff --git a/crates/local-deployment/src/container.rs b/crates/local-deployment/src/container.rs index 48f36afc..422f25fe 100644 --- a/crates/local-deployment/src/container.rs +++ b/crates/local-deployment/src/container.rs @@ -972,14 +972,18 @@ impl ContainerService for LocalContainerService { let approvals_service: Arc = match executor_action.base_executor() { - Some(BaseCodingAgent::Codex) | Some(BaseCodingAgent::ClaudeCode) => { - ExecutorApprovalBridge::new( - self.approvals.clone(), - self.db.clone(), - self.notification_service.clone(), - execution_process.id, - ) - } + Some( + BaseCodingAgent::Codex + | BaseCodingAgent::ClaudeCode + | BaseCodingAgent::Gemini + | BaseCodingAgent::QwenCode + | BaseCodingAgent::Opencode, + ) => ExecutorApprovalBridge::new( + self.approvals.clone(), + self.db.clone(), + self.notification_service.clone(), + execution_process.id, + ), _ => Arc::new(NoopExecutorApprovalService {}), }; diff --git a/crates/utils/src/diff.rs b/crates/utils/src/diff.rs index a197c05e..66947007 100644 --- a/crates/utils/src/diff.rs +++ b/crates/utils/src/diff.rs @@ -238,3 +238,9 @@ pub fn concatenate_diff_hunks(file_path: &str, hunks: &[String]) -> String { unified_diff } + +/// Normalizes a unified diff the format supported by the diff viewer, +pub fn normalize_unified_diff(file_path: &str, unified_diff: &str) -> String { + let hunks = extract_unified_diff_hunks(unified_diff); + concatenate_diff_hunks(file_path, &hunks) +} diff --git a/shared/schemas/opencode.json b/shared/schemas/opencode.json index 0e43cd5c..964e0845 100644 --- a/shared/schemas/opencode.json +++ b/shared/schemas/opencode.json @@ -23,6 +23,11 @@ "null" ] }, + "auto_approve": { + "description": "Auto-approve agent actions", + "type": "boolean", + "default": true + }, "base_command_override": { "title": "Base Command Override", "description": "Override the base command with a custom command", diff --git a/shared/types.ts b/shared/types.ts index cca85d48..961c77e8 100644 --- a/shared/types.ts +++ b/shared/types.ts @@ -378,7 +378,11 @@ export type CursorAgent = { append_prompt: AppendPrompt, force?: boolean | null, export type Copilot = { append_prompt: AppendPrompt, model?: string | null, allow_all_tools?: boolean | null, allow_tool?: string | null, deny_tool?: string | null, add_dir?: Array | null, disable_mcp_server?: Array | null, base_command_override?: string | null, additional_params?: Array | null, env?: { [key in string]?: string } | null, }; -export type Opencode = { append_prompt: AppendPrompt, model?: string | null, mode?: string | null, base_command_override?: string | null, additional_params?: Array | null, env?: { [key in string]?: string } | null, }; +export type Opencode = { append_prompt: AppendPrompt, model?: string | null, mode?: string | null, +/** + * Auto-approve agent actions + */ +auto_approve: boolean, base_command_override?: string | null, additional_params?: Array | null, env?: { [key in string]?: string } | null, }; export type QwenCode = { append_prompt: AppendPrompt, yolo?: boolean | null, base_command_override?: string | null, additional_params?: Array | null, env?: { [key in string]?: string } | null, };