From 9044eebddef34e8b17943a68c63e3d66060bc990 Mon Sep 17 00:00:00 2001 From: Seto Elkahfi <1797197+setoelkahfi@users.noreply.github.com> Date: Mon, 27 Jul 2026 18:45:43 +0200 Subject: [PATCH] Fix loading siGit Code on Xcode custom ACP agent --- README.md | 21 ++++++ src/main.rs | 191 ++++++++++++++++++++++++++++++++++++---------------- src/mcp.rs | 28 ++++++-- 3 files changed, 176 insertions(+), 64 deletions(-) diff --git a/README.md b/README.md index e3838f2..8a0b8cd 100644 --- a/README.md +++ b/README.md @@ -61,6 +61,27 @@ Add this to `~/.config/zed/settings.json`: Use the full absolute path. `~` does not expand here. +## Xcode custom agent + +In Xcode, open **Settings > Intelligence > Agents**, add a custom agent, and set: + +- **Executable:** the absolute path to `sigit` +- **Arguments:** `--acp` + +The explicit `--acp` mode is designed for Xcode: it loads the selected on-device +model on the first prompt, so you do not need to send `/load` from the Xcode chat. + +To let siGit use Xcode's build, test, and project tools, enable **Allow external +agents to use Xcode tools** in Xcode's Intelligence settings, keep the project +open, and add this to `~/.config/sigit/mcp.toml`: + +```toml +[[server]] +name = "xcode" +command = "xcrun" +args = ["mcpbridge"] +``` + ## VS Code ### With siGit Code Extension diff --git a/src/main.rs b/src/main.rs index 2c7dba5..0f62f6d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -433,6 +433,9 @@ struct SiGitAgent { model_load_error: Arc>>, /// true when the startup model isn't cached yet startup_needs_download: bool, + /// Xcode launches custom agents with `--acp`; that mode should be ready to + /// answer the first prompt without requiring an editor slash command. + auto_load_local_model: bool, /// for progress UI startup_model_name: String, /// for download-progress polling @@ -453,6 +456,7 @@ impl SiGitAgent { startup_model_load_started: Arc, model_load_error: Arc>>, startup_needs_download: bool, + auto_load_local_model: bool, ) -> Self { let startup_model_name = initial_model.display_name.clone(); let startup_model_id = initial_model.model_id.clone(); @@ -466,6 +470,7 @@ impl SiGitAgent { startup_model_load_started, model_load_error, startup_needs_download, + auto_load_local_model, startup_model_name, startup_model_id, turn_lock: Arc::new(tokio::sync::Mutex::new(())), @@ -1390,21 +1395,25 @@ impl SiGitAgent { let backend = self.backend.lock().await.clone(); // Only on-device inference needs a local model in memory. Cloud tiers run - // over the network, so they never need a local model. We never load the - // on-device model implicitly: the user loads it explicitly with `/load` - // (or by picking one in `/models`). If a prompt arrives before that, guide - // them rather than blocking on a multi-minute download/load. + // over the network, so they never need a local model. Xcode's explicit + // `--acp` mode loads lazily on the first prompt because its custom-agent + // UI has no reliable equivalent of siGit's `/load` command. if !backend.is_remote() && self.engine.info().await.status == onde::inference::EngineStatus::Unloaded { - self.send_assistant_message( - cx, - session_id, - "No on-device model is loaded. Run `/load` to load the selected model, \ - or `/models` to choose one.", - ) - .ok(); - return Ok(PromptResponse::new(StopReason::EndTurn)); + if self.auto_load_local_model { + self.start_startup_model_load_if_needed(); + self.await_model_ready(cx, &session_id).await?; + } else { + self.send_assistant_message( + cx, + session_id, + "No on-device model is loaded. Run `/load` to load the selected model, \ + or `/models` to choose one.", + ) + .ok(); + return Ok(PromptResponse::new(StopReason::EndTurn)); + } } // ── tool-calling loop ──────────────────────────────────────────── @@ -1422,6 +1431,7 @@ impl SiGitAgent { let mut assembled = String::new(); let mut sent = String::new(); let mut streamed_any = false; + let mut repeated_tool_calls = std::collections::HashMap::::new(); let mut result = self .drain_turn( @@ -1470,6 +1480,7 @@ impl SiGitAgent { } let mut tool_results = Vec::new(); + let mut force_text = false; for (call_index, tc) in result.tool_calls.iter().enumerate() { log::info!( @@ -1478,52 +1489,79 @@ impl SiGitAgent { tc.arguments.chars().take(120).collect::() ); + let signature = format!("{}\n{}", tc.name, tc.arguments); + let repeat_count = repeated_tool_calls + .entry(signature) + .and_modify(|count| *count += 1) + .or_insert(1); + let repeated = *repeat_count >= 3; + if repeated { + force_text = true; + log::warn!( + "prompt({}) stopping repeated tool call `{}` after {} attempts", + session_id, + tc.name, + repeat_count + ); + } + // Permission gate: read-only tools pass straight through; a // mutating tool consults policy and may ask the client. - let output = match permissions::decision_for( - &session_id.to_string(), - &tc.name, - &tc.arguments, - ) { - permissions::Decision::Allow => { - tools::execute_tool(&tc.name, &tc.arguments).await - } - permissions::Decision::Deny(reason) => { - log::info!(" ✗ {} denied by policy", tc.name); - reason - } - permissions::Decision::Ask => { - match self - .request_tool_permission(cx, &session_id, &tc.name, &tc.arguments) - .await - { - PermissionVerdict::Approved => { - tools::execute_tool(&tc.name, &tc.arguments).await - } - PermissionVerdict::Denied(reason) => { - log::info!(" ✗ {} denied by user", tc.name); - reason - } - PermissionVerdict::TurnCancelled => { - log::info!("prompt({}) cancelled at permission gate", session_id); - // The assistant message carrying these tool - // calls is already in the backend history; - // leaving any of them unanswered makes strict - // OpenAI-compatible endpoints reject every - // later request in the session. Close out this - // call and the ones this round never reached. - for pending in &result.tool_calls[call_index..] { - tool_results.push(BackendToolResult { - tool_call_id: pending.id.clone(), - content: format!( - "`{}` was not executed: the user cancelled the turn \ - at the permission prompt.", - pending.name - ), - }); + let output = if repeated { + format!( + "The tool `{}` was not executed again because the model repeated \ + the same call three times. Continue without this tool.", + tc.name + ) + } else { + match permissions::decision_for( + &session_id.to_string(), + &tc.name, + &tc.arguments, + ) { + permissions::Decision::Allow => { + tools::execute_tool(&tc.name, &tc.arguments).await + } + permissions::Decision::Deny(reason) => { + log::info!(" ✗ {} denied by policy", tc.name); + reason + } + permissions::Decision::Ask => { + match self + .request_tool_permission(cx, &session_id, &tc.name, &tc.arguments) + .await + { + PermissionVerdict::Approved => { + tools::execute_tool(&tc.name, &tc.arguments).await + } + PermissionVerdict::Denied(reason) => { + log::info!(" ✗ {} denied by user", tc.name); + reason + } + PermissionVerdict::TurnCancelled => { + log::info!( + "prompt({}) cancelled at permission gate", + session_id + ); + // The assistant message carrying these tool + // calls is already in the backend history; + // leaving any of them unanswered makes strict + // OpenAI-compatible endpoints reject every + // later request in the session. Close out this + // call and the ones this round never reached. + for pending in &result.tool_calls[call_index..] { + tool_results.push(BackendToolResult { + tool_call_id: pending.id.clone(), + content: format!( + "`{}` was not executed: the user cancelled the turn \ + at the permission prompt.", + pending.name + ), + }); + } + backend.record_cancelled_tool_results(tool_results).await; + return Ok(PromptResponse::new(StopReason::Cancelled)); } - backend.record_cancelled_tool_results(tool_results).await; - return Ok(PromptResponse::new(StopReason::Cancelled)); } } } @@ -1537,7 +1575,7 @@ impl SiGitAgent { }); } - let next_tools = if round < MAX_TOOL_ROUNDS { + let next_tools = if round < MAX_TOOL_ROUNDS && !force_text { Some(tools.as_slice()) } else { None // last round: force text @@ -3064,7 +3102,17 @@ fn default_local_model_config() -> GgufModelConfig { .unwrap_or_else(GgufModelConfig::qwen25_3b) } -async fn run_acp_server() -> anyhow::Result<()> { +fn parse_explicit_acp(args: &[String]) -> Result { + if !args.iter().any(|arg| arg == "--acp") { + return Ok(false); + } + if args.iter().any(|arg| arg != "--acp") { + return Err("--acp does not accept additional arguments".to_string()); + } + Ok(true) +} + +async fn run_acp_server(auto_load_local_model: bool) -> anyhow::Result<()> { log::info!("ACP mode — starting agent server"); let config = default_local_model_config(); @@ -3102,6 +3150,7 @@ async fn run_acp_server() -> anyhow::Result<()> { startup_model_load_started, model_load_error, needs_download, + auto_load_local_model, )); // Honor the explicit provider override (OPENAI_BASE_URL/OPENAI_API_KEY or @@ -3294,6 +3343,25 @@ async fn main() -> anyhow::Result<()> { // is dispatched here, before the TTY/ACP split, like the account // subcommands above. let cli_args: Vec = std::env::args().skip(1).collect(); + // Xcode's custom-agent UI launches ACP agents with an explicit `--acp` + // argument. Honor it even when Xcode gives the child a terminal-like stdin. + match parse_explicit_acp(&cli_args) { + Err(error) => { + eprintln!("sigit: {error}"); + std::process::exit(2); + } + Ok(true) => { + init_logging(false); + setup::setup_shared_model_cache(); + mcp::init().await; + log::info!( + "siGit v{} starting (explicit ACP mode)", + env!("CARGO_PKG_VERSION") + ); + return run_acp_server(true).await; + } + Ok(false) => {} + } match headless::parse_args(&cli_args) { Ok(None) => {} Ok(Some(config)) => { @@ -3350,7 +3418,7 @@ async fn main() -> anyhow::Result<()> { // Best-effort MCP discovery (incl. the official server) before serving. mcp::init().await; log::info!("siGit v{} starting (ACP mode)", env!("CARGO_PKG_VERSION")); - run_acp_server().await + run_acp_server(false).await } } @@ -3358,6 +3426,13 @@ async fn main() -> anyhow::Result<()> { mod tests { use super::*; + #[test] + fn explicit_acp_flag_is_accepted_without_other_arguments() { + assert_eq!(parse_explicit_acp(&["--acp".to_string()]), Ok(true)); + assert_eq!(parse_explicit_acp(&[]), Ok(false)); + assert!(parse_explicit_acp(&["--acp".to_string(), "--quiet".to_string()]).is_err()); + } + #[test] fn system_prompt_advertises_the_commit_co_author_trailer() { // The prompt instructs the model with the exact trailer that diff --git a/src/mcp.rs b/src/mcp.rs index 06d63dc..beb79ca 100644 --- a/src/mcp.rs +++ b/src/mcp.rs @@ -127,9 +127,12 @@ const PROTOCOL_VERSION: &str = "2025-06-18"; /// sum. const HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(8); -/// Overall request timeout for an individual `tools/call`. Generous, since an -/// MCP tool may do real work server-side. +/// Overall request timeout for an individual `tools/call`. Generous for build +/// and test tools, while the Xcode bridge gets a shorter bound below because +/// it otherwise leaves an ACP prompt looking permanently busy when Xcode +/// cannot service the request. const CALL_TIMEOUT: Duration = Duration::from_secs(120); +const XCODE_CALL_TIMEOUT: Duration = Duration::from_secs(30); /// Cap on the characters returned from a single tool call, so a chatty server /// can't blow up the model's context. Matches the spirit of the file-read cap. @@ -818,7 +821,12 @@ impl Mcp { let result = match &server.transport { None => return Err(format!("server '{}' is not connected", server.name)), Some(Transport::Stdio(stdio)) => { - stdio.request("tools/call", params, CALL_TIMEOUT).await? + let timeout = if server.name == "xcode" { + XCODE_CALL_TIMEOUT + } else { + CALL_TIMEOUT + }; + stdio.request("tools/call", params, timeout).await? } Some(Transport::Http(http_conn)) => { let body = json!({ @@ -827,14 +835,19 @@ impl Mcp { "method": "tools/call", "params": params }); - match post_rpc(&self.http, &server.name, http_conn, &body, CALL_TIMEOUT).await { + let timeout = if server.name == "xcode" { + XCODE_CALL_TIMEOUT + } else { + CALL_TIMEOUT + }; + match post_rpc(&self.http, &server.name, http_conn, &body, timeout).await { Ok(result) => result, Err(error) if error.contains("returned 404") => { // Session expired — drop it, re-handshake, and retry once. *http_conn.session_id.lock().await = None; initialize(&self.http, server).await?; notify_initialized(&self.http, server).await?; - post_rpc(&self.http, &server.name, http_conn, &body, CALL_TIMEOUT).await? + post_rpc(&self.http, &server.name, http_conn, &body, timeout).await? } Err(error) => return Err(error), } @@ -1027,7 +1040,10 @@ async fn stdio_reader(mut stdout: BufReader, shared: Arc