@sigitcodereview / sigit / commits / 4a390b6

Take permission mode changes without waiting for the running turn

session/set_mode and the Permissions config option took the session lock and the workspace lock, and a turn holds the session lock from start to end. So picking Auto while a permission prompt was open waited until the prompt had been answered another way. Both now run without either lock. They touch only state keyed by session, so they check that the session is known, build the config options from that session's own model instead of the live one, and answer at once. Model and Inference changes swap the live backend and still wait for the turn. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Co-Authored-By: siGit Code v1.6.6-acp <noreply@sigit.si>

Seto Elkahfi committed Oct 10, 2026 at 17:07 UTC 4a390b66b03d004820f046b14c355431d0be0dc4

4 files changed +242 -38

.agents/AGENTS.md

+6 -1

@@ -340,7 +340,12 @@ feeds results back. Neither the loop nor ACP/TUI surfaces depend on a concrete b

340 `run_command` poll loop and `client_terminal::Route::run` watch, so a running

341 foreground command is killed instead of finishing first. The signal is reset at the

342 top of each turn, not when a tool starts, so a cancel that lands during a permission

343 - prompt still stops the command that follows. Tool

343 + prompt still stops the command that follows. Permission-mode changes

344 + (`session/set_mode`, and `session/set_config_option` for `sigit-permission-mode`) are

345 + the exception to the lock order: the dispatch loop runs them with neither lock, because

346 + they touch only state keyed by session (`permissions`) and have to land mid-turn. They

347 + check the id against `sessions` (`-32002` when unknown) and build the options from

348 + that session's own model (`config_options_for_session`), never `activate_session`. Tool

349 execution, subagents included, still runs one session at a time.

350 - **`src/instructions.rs`** — project instruction files, the always-on counterpart to skills.

351 Reads `AGENTS.md` (the cross-tool [agents.md](https://agents.md) standard) and `CLAUDE.md`,

CHANGELOG.md

@@ -14,6 +14,12 @@

14 noticed the cancel. A cancel or `session/close` now kills the process tree, or

15 sends `terminal/kill` when the command runs in the editor's terminal, and the

16 turn ends `cancelled` within a moment.

17 +- **Switching permission mode works while a turn is running.** `session/set_mode`

18 + and the Permissions config option queued behind the turn's session lock, so

19 + choosing Auto while a permission prompt was open did nothing until the turn

20 + ended. They now take no lock, are answered at once, and the pending call's

21 + successors follow the new mode. Model and Inference changes still wait for

22 + the turn.

23 - **A prompt that is only audio is answered.** A clip with no text or image used

24 to end the turn at once with nothing sent to the model, even though the agent

25 advertises audio.

src/main.rs

+94 -37

@@ -4445,6 +4445,69 @@ impl SiGitAgent {

4445 /// modes are the Permissions dropdown under another name, so this changes

4446 /// the same state and then refreshes the dropdown for a client that shows

4447 /// both.

4448 + /// `-32002` unless `session_id` is a session this process knows.

4449 + fn require_known_session(&self, session_id: &SessionId) -> agent_client_protocol::Result<()> {

4450 + let key = session_id.to_string();

4451 + if self.sessions.lock().unwrap().contains_key(&key) {

4452 + return Ok(());

4453 + }

4454 + Err(agent_client_protocol::Error::new(

4455 + -32002,

4456 + format!(

4457 + "unknown session {key}; create it with session/new or restore it with session/load"

4458 + ),

4459 + ))

4460 + }

4461 +

4462 + /// The config options for `key`, built from that session's own model and

4463 + /// not from whichever session is live, so it can be read without the

4464 + /// workspace lock.

4465 + fn config_options_for_session(&self, key: &str) -> Vec<SessionConfigOption> {

4466 + let live = self.active_session.lock().unwrap().as_deref() == Some(key);

4467 + let model_id = if live {

4468 + self.current_model.lock().unwrap().model_id.clone()

4469 + } else {

4470 + self.sessions

4471 + .lock()

4472 + .unwrap()

4473 + .get(key)

4474 + .map(|state| {

4475 + state

4476 + .remote

4477 + .as_ref()

4478 + .map_or_else(|| state.model_id.clone(), |r| r.model.model_id.clone())

4479 + })

4480 + .unwrap_or_default()

4481 + };

4482 + build_config_options_for(&model_id, key)

4483 + }

4484 +

4485 + /// Apply a Permissions change for `session_id`, which can be mid-turn.

4486 + ///

4487 + /// Takes neither the session lock nor the workspace lock: the turn holds

4488 + /// the first for its whole length, and a user flipping to Auto while a

4489 + /// permission prompt is open has to be heard at once. It touches only

4490 + /// state keyed by session (`permissions`), so the live session is left

4491 + /// alone. Returns the new config options and the status line to show.

4492 + fn change_permission_mode(

4493 + &self,

4494 + cx: &ConnectionTo<Client>,

4495 + session_id: &SessionId,

4496 + mode_id: Option<&str>,

4497 + ) -> agent_client_protocol::Result<Vec<SessionConfigOption>> {

4498 + self.require_known_session(session_id)?;

4499 + let key = session_id.to_string();

4500 + let Some(message) = mode_id.and_then(|id| apply_permission_mode(&key, id)) else {

4501 + return Err(agent_client_protocol::Error::new(

4502 + -32602,

4503 + format!("unknown permission mode: {mode_id:?}"),

4504 + ));

4505 + };

4506 + self.send_system_status(cx, session_id.clone(), message)

4507 + .ok();

4508 + Ok(self.config_options_for_session(&key))

4509 + }

4510 +

4511 async fn handle_set_session_mode(

4512 &self,

4513 cx: &ConnectionTo<Client>,

@@ -4456,21 +4519,8 @@ impl SiGitAgent {

4519 args.mode_id

4520 );

4521

4459 - self.activate_session(&args.session_id).await?;

4460 -

4461 - let session_key = args.session_id.to_string();

4462 - let Some(message) = apply_permission_mode(&session_key, args.mode_id.0.as_ref()) else {

4463 - return Err(agent_client_protocol::Error::new(

4464 - -32602,

4465 - format!("unknown session mode: {}", args.mode_id),

4466 - ));

4467 - };

4468 - self.send_system_status(cx, args.session_id.clone(), message)

4469 - .ok();

4470 - let config_options = {

4471 - let current = self.current_model.lock().unwrap();

4472 - build_config_options(&current, &session_key)

4473 - };

4522 + let config_options =

4523 + self.change_permission_mode(cx, &args.session_id, Some(args.mode_id.0.as_ref()))?;

4524 self.send_tool_call_update(

4525 cx,

4526 args.session_id,

@@ -4491,6 +4541,16 @@ impl SiGitAgent {

4541 args.value

4542 );

4543

4544 + // ── Permissions dropdown (issue #76) ────────────────────────────────

4545 + // Handled before the session is activated: the dispatch loop runs this

4546 + // one without the session or workspace lock, so it can land mid-turn.

4547 + if args.config_id.0.as_ref() == PERMISSION_MODE_CONFIG_ID {

4548 + let picked = args.value.as_value_id().map(|v| v.0.as_ref());

4549 + let config_options = self.change_permission_mode(cx, &args.session_id, picked)?;

4550 + self.send_current_mode(cx, args.session_id);

4551 + return Ok(SetSessionConfigOptionResponse::new(config_options));

4552 + }

4553 +

4554 // A model switch carries the live conversation onto the new backend, so

4555 // the live conversation has to be the one whose picker changed.

4556 self.activate_session(&args.session_id).await?;

@@ -4531,25 +4591,6 @@ impl SiGitAgent {

4591 return Ok(SetSessionConfigOptionResponse::new(config_options));

4592 }

4593

4534 - // ── Permissions dropdown (issue #76) ────────────────────────────────

4535 - if args.config_id.0.as_ref() == PERMISSION_MODE_CONFIG_ID {

4536 - let session_key = args.session_id.to_string();

4537 - let picked = args.value.as_value_id().map(|v| v.0.as_ref());

4538 - let Some(message) = picked.and_then(|id| apply_permission_mode(&session_key, id))

4539 - else {

4540 - return Err(agent_client_protocol::Error::new(

4541 - -32602,

4542 - format!("unknown Permissions value: {picked:?}"),

4543 - ));

4544 - };

4545 - self.send_current_mode(cx, args.session_id.clone());

4546 - self.send_system_status(cx, args.session_id.clone(), message)

4547 - .ok();

4548 - let current = self.current_model.lock().unwrap().clone();

4549 - let config_options = build_config_options(&current, &session_key);

4550 - return Ok(SetSessionConfigOptionResponse::new(config_options));

4551 - }

4552 -

4594 if args.config_id.0.as_ref() != MODEL_CONFIG_ID {

4595 return Err(agent_client_protocol::Error::new(

4596 -32602,

@@ -5009,6 +5050,13 @@ fn build_config_options(

5050 current_model: &GgufModelConfig,

5051 session_key: &str,

5052 ) -> Vec<SessionConfigOption> {

5053 + build_config_options_for(&current_model.model_id, session_key)

5054 +}

5055 +

5056 +/// The config options for a session whose selected model is `current_model_id`.

5057 +/// Only the id matters, which is what lets a session that is not the live one

5058 +/// have its options built without installing it.

5059 +fn build_config_options_for(current_model_id: &str, session_key: &str) -> Vec<SessionConfigOption> {

5060 // The full list, including the siGit Code Cloud tiers, so the panel picker

5061 // mirrors the TUI `/models`. Cloud entries are sign-in gated at selection.

5062 let items = models::build_model_picker_items();

@@ -5135,7 +5183,7 @@ fn build_config_options(

5183 return vec![local_option, permission_option];

5184 }

5185

5138 - let current_value = SessionConfigValueId::new(current_model.model_id.as_str());

5186 + let current_value = SessionConfigValueId::new(current_model_id);

5187

5188 vec![

5189 SessionConfigOption::select(MODEL_CONFIG_ID, "Model", current_value, options)

@@ -6255,6 +6303,15 @@ async fn run_acp_server(auto_load_local_model: bool) -> anyhow::Result<()> {

6303 let state = Arc::clone(&state);

6304 let task_cx = cx.clone();

6305 cx.spawn(async move {

6306 + // A Permissions change can arrive while a turn is

6307 + // running, so it takes neither lock. Model and

6308 + // Inference swap the live backend and wait for the turn.

6309 + if req.config_id.0.as_ref() == PERMISSION_MODE_CONFIG_ID {

6310 + return handle_response(

6311 + responder,

6312 + state.handle_set_session_config_option(&task_cx, req).await,

6313 + );

6314 + }

6315 let _session = state.lock_session(&req.session_id).await;

6316 let _workspace = state.workspace_lock.lock().await;

6317 handle_response(

@@ -6272,9 +6329,9 @@ async fn run_acp_server(auto_load_local_model: bool) -> anyhow::Result<()> {

6329 async move |req: SetSessionModeRequest, responder, cx: ConnectionTo<Client>| {

6330 let state = Arc::clone(&state);

6331 let task_cx = cx.clone();

6332 + // No locks: the mode can change while a turn is running,

6333 + // and the change touches only state keyed by session.

6334 cx.spawn(async move {

6276 - let _session = state.lock_session(&req.session_id).await;

6277 - let _workspace = state.workspace_lock.lock().await;

6335 handle_response(

6336 responder,

6337 state.handle_set_session_mode(&task_cx, req).await,

tests/acp_permissions.rs

+136

@@ -2268,3 +2268,139 @@ fn session_modes_mirror_the_permissions_config_option() {

2268 drop(agent);

2269 let _ = std::fs::remove_dir_all(&scratch);

2270 }

2271 +

2272 +/// A mode change is heard while a turn waits on a permission prompt, and the

2273 +/// call that follows obeys it. The turn holds its session's lock for its whole

2274 +/// length, so a change that queued behind it would sit unanswered until the

2275 +/// prompt had been dealt with.

2276 +#[test]

2277 +fn a_mode_switch_during_a_permission_prompt_is_answered_at_once() {

2278 + let endpoint = start_fake_endpoint(vec![

2279 + sse_tool_call("call_1", "run_command", r#"{"command":"echo sigit-first"}"#),

2280 + sse_tool_call(

2281 + "call_2",

2282 + "run_command",

2283 + r#"{"command":"echo sigit-second"}"#,

2284 + ),

2285 + sse_text("done"),

2286 + ]);

2287 +

2288 + let scratch = std::env::temp_dir().join(format!("sigit_acp_mode_mid_{}", std::process::id()));

2289 + let config_dir = scratch.join("config");

2290 + let cwd = scratch.join("cwd");

2291 + std::fs::create_dir_all(&config_dir).unwrap();

2292 + std::fs::create_dir_all(&cwd).unwrap();

2293 +

2294 + let mut agent = spawn_agent(endpoint.port, &config_dir);

2295 +

2296 + let id = agent.request(

2297 + "initialize",

2298 + json!({"protocolVersion": 1, "clientCapabilities": {}}),

2299 + );

2300 + agent.wait_for_response(id);

2301 + let id = agent.request("session/new", json!({"cwd": cwd, "mcpServers": []}));

2302 + let session_id = agent.wait_for_response(id)["result"]["sessionId"]

2303 + .as_str()

2304 + .expect("session id")

2305 + .to_string();

2306 +

2307 + let prompt_id = agent.request(

2308 + "session/prompt",

2309 + json!({

2310 + "sessionId": session_id,

2311 + "prompt": [{"type": "text", "text": "run both commands"}],

2312 + }),

2313 + );

2314 + let permission = agent.wait_for_agent_request("session/request_permission");

2315 +

2316 + // The turn is parked on the question. Switch to Auto, and expect the

2317 + // answer now, not after the turn.

2318 + let mode_id = agent.request(

2319 + "session/set_mode",

2320 + json!({"sessionId": session_id, "modeId": "permission-mode-auto"}),

2321 + );

2322 + let response = agent.wait_for(

2323 + "the set_mode response while the permission prompt is open",

2324 + |message| {

2325 + message["id"] == mode_id && message.get("method").is_none()

2326 + || message["id"] == prompt_id && message.get("method").is_none()

2327 + },

2328 + );

2329 + assert_eq!(

2330 + response["id"], mode_id,

2331 + "the turn ended before set_mode was answered: {response}"

2332 + );

2333 + assert!(response.get("error").is_none(), "{response}");

2334 +

2335 + // Same for the config option, which is the path most editors use.

2336 + let option_id = agent.request(

2337 + "session/set_config_option",

2338 + json!({

2339 + "sessionId": session_id,

2340 + "configId": "sigit-permission-mode",

2341 + "value": "permission-mode-auto",

2342 + }),

2343 + );

2344 + let response = agent.wait_for(

2345 + "the set_config_option response while the permission prompt is open",

2346 + |message| {

2347 + message["id"] == option_id && message.get("method").is_none()

2348 + || message["id"] == prompt_id && message.get("method").is_none()

2349 + },

2350 + );

2351 + assert_eq!(response["id"], option_id, "{response}");

2352 + assert!(response.get("error").is_none(), "{response}");

2353 +

2354 + // Allow the pending call. The next one is decided under Auto: no prompt.

2355 + agent.respond(

2356 + permission["id"].clone(),

2357 + json!({"outcome": {"outcome": "selected", "optionId": "allow_once"}}),

2358 + );

2359 + let (response, _updates) = agent.wait_for_response_with_updates(prompt_id);

2360 + assert_eq!(response["result"]["stopReason"], "end_turn");

2361 +

2362 + let requests = endpoint.requests.lock().unwrap();

2363 + let second_result = requests[2]["messages"]

2364 + .as_array()

2365 + .expect("messages")

2366 + .iter()

2367 + .find(|message| message["role"] == "tool" && message["tool_call_id"] == "call_2")

2368 + .expect("tool result for the second call");

2369 + assert!(

2370 + second_result["content"]

2371 + .as_str()

2372 + .unwrap_or_default()

2373 + .contains("sigit-second"),

2374 + "the second call should have run under Auto: {second_result}"

2375 + );

2376 +

2377 + drop(requests);

2378 + drop(agent);

2379 + let _ = std::fs::remove_dir_all(&scratch);

2380 +}

2381 +

2382 +#[test]

2383 +fn a_mode_change_for_an_unknown_session_is_resource_not_found() {

2384 + let endpoint = start_fake_endpoint(vec![]);

2385 + let scratch =

2386 + std::env::temp_dir().join(format!("sigit_acp_mode_unknown_{}", std::process::id()));

2387 + let config_dir = scratch.join("config");

2388 + std::fs::create_dir_all(&config_dir).unwrap();

2389 +

2390 + let mut agent = spawn_agent(endpoint.port, &config_dir);

2391 + let id = agent.request(

2392 + "initialize",

2393 + json!({"protocolVersion": 1, "clientCapabilities": {}}),

2394 + );

2395 + agent.wait_for_response(id);

2396 +

2397 + let id = agent.request(

2398 + "session/set_mode",

2399 + json!({"sessionId": "nope", "modeId": "permission-mode-auto"}),

2400 + );

2401 + let response = agent.wait_for("set_mode answer", |m| m["id"] == id);

2402 + assert_eq!(response["error"]["code"], -32002, "{response}");

2403 +

2404 + drop(agent);

2405 + let _ = std::fs::remove_dir_all(&scratch);

2406 +}