Skip to content

Fix loading siGit Code on Xcode custom ACP agent #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
setoelkahfi merged 1 commit into from
Jul 27, 2026
Merged

Fix loading siGit Code on Xcode custom ACP agent #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
191 changes: 133 additions & 58 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,9 @@ struct SiGitAgent {
model_load_error: Arc<std::sync::Mutex<Option<String>>>,
/// 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
Expand All @@ -453,6 +456,7 @@ impl SiGitAgent {
startup_model_load_started: Arc<AtomicBool>,
model_load_error: Arc<std::sync::Mutex<Option<String>>>,
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();
Expand All @@ -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(())),
Expand Down Expand Up @@ -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 ────────────────────────────────────────────
Expand All @@ -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::<String, usize>::new();

let mut result = self
.drain_turn(
Expand Down Expand Up @@ -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!(
Expand All @@ -1478,52 +1489,79 @@ impl SiGitAgent {
tc.arguments.chars().take(120).collect::<String>()
);

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));
}
}
}
Expand All @@ -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
Expand Down Expand Up @@ -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<bool, String> {
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();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<String> = 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)) => {
Expand Down Expand Up @@ -3350,14 +3418,21 @@ 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
}
}

#[cfg(test)]
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
Expand Down
28 changes: 22 additions & 6 deletions src/mcp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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!({
Expand All @@ -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),
}
Expand Down Expand Up @@ -1027,7 +1040,10 @@ async fn stdio_reader(mut stdout: BufReader<ChildStdout>, shared: Arc<StdioShare
);
continue;
}
let Some(id) = message.get("id").and_then(Value::as_i64) else {
let Some(id) = message
.get("id")
.and_then(|value| value.as_i64().or_else(|| value.as_str()?.parse().ok()))
else {
log::warn!(
"mcp: '{}' sent a response without a usable id; ignoring",
shared.name
Expand Down