diff --git a/src-tauri/src/acp/connection.rs b/src-tauri/src/acp/connection.rs index 581e190642..b4748ddd5d 100644 --- a/src-tauri/src/acp/connection.rs +++ b/src-tauri/src/acp/connection.rs @@ -6557,16 +6557,9 @@ async fn apply_preferred_session_options( preferred_config_values: &BTreeMap, initial_config_options: Vec, ) -> Vec { - if let Some(pref_mode) = preferred_mode_id { - let needs_apply = session - .modes() - .as_ref() - .map(|m| m.current_mode_id.to_string() != pref_mode) - .unwrap_or(false); - if needs_apply { - if let Err(e) = set_session_mode(session, state, emitter, pref_mode.to_string()).await { - tracing::error!("[ACP] failed to apply preferred mode '{pref_mode}' on connect: {e}"); - } + if let Some(pref_mode) = preferred_mode_to_apply(session.modes().as_ref(), preferred_mode_id) { + if let Err(e) = set_session_mode(session, state, emitter, pref_mode.to_string()).await { + tracing::error!("[ACP] failed to apply preferred mode '{pref_mode}' on connect: {e}"); } } @@ -6613,6 +6606,68 @@ async fn apply_preferred_session_options( options } +/// Return the preferred mode only when the freshly attached session advertises +/// modes and currently differs. Keeping this decision separate makes the fork +/// inheritance contract explicit: a differing parent mode produces exactly one +/// `session/set_mode`, while equal or absent parent modes produce none. +fn preferred_mode_to_apply<'a>( + session_modes: Option<&SessionModeState>, + preferred_mode_id: Option<&'a str>, +) -> Option<&'a str> { + preferred_mode_id.filter(|preferred| { + session_modes.is_some_and(|modes| modes.current_mode_id.to_string() != *preferred) + }) +} + +/// Prepare a freshly attached fork child before its command loop is allowed to +/// consume buffered prompts. Unlike an ordinary saved preference, an inherited +/// mode is part of the fork's permission contract: if the agent rejects the +/// restore, continuing on its default mode could execute the queued prompt with +/// different permissions. Therefore this path is strict and returns before +/// config emission or `SelectorsReady` on failure. +#[allow(clippy::too_many_arguments)] +async fn prepare_fork_child_session( + cx: &ConnectionTo, + session: &mut sacp::ActiveSession<'_, Agent>, + state: &Arc>, + emitter: &EventEmitter, + agent_type: AgentType, + grok_meta: Option<&serde_json::Map>, + grok_model_specs: Option<&HashMap>, + inherited_mode_id: Option<&str>, + initial_config_options: Vec, +) -> Result<(), sacp::Error> { + if let Some(mode_id) = + preferred_mode_to_apply(session.modes().as_ref(), inherited_mode_id) + { + set_session_mode(session, state, emitter, mode_id.to_string()) + .await + .map_err(|error| { + sacp::util::internal_error(format!( + "Failed to restore inherited mode '{mode_id}' after fork; queued prompts were not sent: {error}" + )) + })?; + } + + apply_and_emit_session_config_options( + cx, + session, + state, + emitter, + agent_type, + grok_meta, + grok_model_specs, + // The inherited mode was handled strictly above. Passing it through + // the ordinary preference path would turn a retry failure into a log. + None, + &BTreeMap::new(), + initial_config_options, + ) + .await; + emit_selectors_ready(state, emitter).await; + Ok(()) +} + const TERMINAL_POLL_INTERVAL_MS: u64 = 200; const TERMINAL_POLL_MISSING_LIMIT: u8 = 10; @@ -7356,12 +7411,28 @@ fn prepare_agent_bound_prompt( map_prompt_blocks(blocks) } +/// Read the event-backed mode selected on the live parent session, but only when +/// that parent advertises modes. `SessionState` spans nested fork transitions, +/// so its value alone may belong to an ancestor when the current session has no +/// modes. The `ActiveSession` snapshot is used only as this capability gate; +/// Codeg's `set_session_mode` path does not update its current-mode value. +fn live_mode_for_fork( + state: &SessionState, + parent_modes: Option<&SessionModeState>, +) -> Option { + parent_modes.and(state.current_mode.clone()) +} + /// Result when the conversation loop exits due to a fork request. struct ForkExitInfo { fork_response: sacp::schema::ForkSessionResponse, /// Raw top-level `models` from the fork response (Grok per-model effort data), /// captured before the typed deserialize drops it. `None` when absent. fork_models_raw: Option, + /// The parent's live mode at the instant the fork was requested. A fork + /// response may reset to the agent default, so the child must re-apply this + /// before selectors become ready and queued prompts can run. + inherited_mode_id: Option, original_session_id: String, reply: tokio::sync::oneshot::Sender>, connection: ConnectionTo, @@ -7407,6 +7478,7 @@ async fn handle_fork_or_exit( let cx = fork_info.connection; let fork_resp = fork_info.fork_response; let fork_models_raw = fork_info.fork_models_raw; + let inherited_mode_id = fork_info.inherited_mode_id; let new_sid = fork_resp.session_id.0.to_string(); tracing::info!( @@ -7453,7 +7525,7 @@ async fn handle_fork_or_exit( ) .await; emit_session_modes(state, emitter, session.modes()).await; - apply_and_emit_session_config_options( + prepare_fork_child_session( &cx, &mut session, state, @@ -7461,12 +7533,10 @@ async fn handle_fork_or_exit( agent_type, grok_meta.as_ref(), grok_model_specs.as_ref(), - None, - &BTreeMap::new(), + inherited_mode_id.as_deref(), initial_config_options.unwrap_or_default(), ) - .await; - emit_selectors_ready(state, emitter).await; + .await?; let loop_result = run_conversation_loop( &mut session, @@ -9022,6 +9092,10 @@ async fn run_conversation_loop<'a>( } let cx = session.connection(); let sid = session.session_id().clone(); + let inherited_mode_id = { + let state = state.read().await; + live_mode_for_fork(&state, session.modes().as_ref()) + }; tracing::info!( "[ACP] Sending session/fork for session_id={} cwd={}", sid.0, cwd @@ -9036,6 +9110,7 @@ async fn run_conversation_loop<'a>( return Ok(Some(ForkExitInfo { fork_response, fork_models_raw, + inherited_mode_id, original_session_id: sid.0.to_string(), reply, connection: cx, @@ -12404,7 +12479,7 @@ async fn emit_conversation_update( #[cfg(test)] mod tests { use super::*; - use sacp::schema::{Diff, SessionConfigId}; + use sacp::schema::{Diff, SessionConfigId, SessionMode}; /// Unwrap a select selector. The Grok synthesizers below only ever build /// selects, so any other kind is a test failure rather than a branch to @@ -12418,6 +12493,258 @@ mod tests { } } + fn fork_test_modes(current: &str) -> SessionModeState { + SessionModeState::new( + current.to_string(), + vec![ + SessionMode::new("manual", "Manual"), + SessionMode::new("auto", "Auto"), + ], + ) + } + + #[test] + fn fork_mode_inheritance_applies_a_different_parent_mode_once() { + let fork_modes = fork_test_modes("manual"); + + assert_eq!( + preferred_mode_to_apply(Some(&fork_modes), Some("auto")), + Some("auto"), + "a fork reset to Manual must schedule one set_mode back to the parent's Auto" + ); + } + + #[test] + fn fork_mode_inheritance_skips_an_already_matching_mode() { + let fork_modes = fork_test_modes("auto"); + + assert_eq!( + preferred_mode_to_apply(Some(&fork_modes), Some("auto")), + None, + "an already inherited mode must not produce a duplicate set_mode" + ); + } + + #[test] + fn fork_mode_inheritance_without_a_parent_mode_preserves_the_fork_default() { + let fork_modes = fork_test_modes("manual"); + + assert_eq!(preferred_mode_to_apply(Some(&fork_modes), None), None); + assert_eq!( + preferred_mode_to_apply(None, Some("auto")), + None, + "a session without advertised modes cannot accept an inherited mode" + ); + } + + #[test] + fn fork_mode_inheritance_reads_the_latest_live_session_state() { + let establishment_snapshot = fork_test_modes("manual"); + let mut state = SessionState::new( + "conn-fork-mode".to_string(), + AgentType::ClaudeCode, + None, + "window".to_string(), + None, + ); + + assert_eq!( + live_mode_for_fork(&state, Some(&establishment_snapshot)), + None + ); + state.apply_event(&AcpEvent::SessionModes { + modes: map_session_modes(&establishment_snapshot), + }); + state.apply_event(&AcpEvent::ModeChanged { + mode_id: "auto".to_string(), + }); + assert_eq!( + establishment_snapshot.current_mode_id.to_string(), + "manual", + "the ActiveSession-style establishment snapshot stays stale" + ); + assert_eq!( + live_mode_for_fork(&state, Some(&establishment_snapshot)).as_deref(), + Some("auto") + ); + + // A nested fork must inherit the latest selection, not the mode captured + // for the previous fork or the original establishment response. + state.apply_event(&AcpEvent::ModeChanged { + mode_id: "manual".to_string(), + }); + assert_eq!( + live_mode_for_fork(&state, Some(&establishment_snapshot)).as_deref(), + Some("manual") + ); + } + + #[test] + fn fork_mode_inheritance_ignores_stale_state_when_parent_has_no_modes() { + let mut state = SessionState::new( + "conn-fork-mode".to_string(), + AgentType::ClaudeCode, + None, + "window".to_string(), + None, + ); + state.apply_event(&AcpEvent::ModeChanged { + mode_id: "auto".to_string(), + }); + + assert_eq!(state.current_mode.as_deref(), Some("auto")); + assert_eq!( + live_mode_for_fork(&state, None), + None, + "a modes-less current parent must not inherit an ancestor's stale mode" + ); + } + + struct ForkModeGateProbe { + result: Result<(), sacp::Error>, + rpc_calls: Vec, + events: Vec, + queued_prompt_released: bool, + selectors_ready: bool, + } + + /// Drive the production fork-child preparation against an in-memory ACP + /// agent. The prompt sentinel is queued before preparation, exactly like a + /// frontend prompt buffered while the fork transition is in flight, and is + /// released only after the same function that emits `SelectorsReady` + /// succeeds. + async fn probe_fork_mode_gate( + child_mode: &str, + inherited_mode: Option<&str>, + reject_set_mode: bool, + ) -> ForkModeGateProbe { + let rpc_calls = Arc::new(std::sync::Mutex::new(Vec::new())); + let agent_rpc_calls = Arc::clone(&rpc_calls); + let agent = Agent.builder().on_receive_request( + async move |request: SetSessionModeRequest, + responder: Responder, + _cx| { + agent_rpc_calls + .lock() + .expect("rpc log lock") + .push(format!("set_mode:{}", request.mode_id)); + if reject_set_mode { + responder.respond_with_error(sacp::util::internal_error("mode rejected")) + } else { + responder.respond(sacp::schema::SetSessionModeResponse::new()) + } + }, + sacp::on_receive_request!(), + ); + let (agent_channel, client_channel) = sacp::Channel::duplex(); + let agent_task = tokio::spawn(async move { agent.connect_to(agent_channel).await }); + + let state = Arc::new(RwLock::new(SessionState::new( + "conn-fork-mode-wire".to_string(), + AgentType::ClaudeCode, + None, + "window".to_string(), + None, + ))); + let mut event_rx = { + let snapshot = state.read().await; + snapshot.event_stream().subscribe() + }; + let (prompt_tx, mut prompt_rx) = mpsc::channel(1); + prompt_tx.send(()).await.expect("queue prompt sentinel"); + let queued_prompt_released = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let released_in_connection = Arc::clone(&queued_prompt_released); + let state_in_connection = Arc::clone(&state); + let child_mode = child_mode.to_string(); + let inherited_mode = inherited_mode.map(str::to_string); + + let result = Client + .builder() + .connect_with(client_channel, async move |cx| { + let response = NewSessionResponse::new(SessionId::new("fork-child")) + .modes(fork_test_modes(&child_mode)); + let mut session = cx.attach_session(response, Default::default())?; + prepare_fork_child_session( + &cx, + &mut session, + &state_in_connection, + &EventEmitter::Noop, + AgentType::ClaudeCode, + None, + None, + inherited_mode.as_deref(), + Vec::new(), + ) + .await?; + + prompt_rx.recv().await.expect("queued prompt still present"); + released_in_connection.store(true, std::sync::atomic::Ordering::SeqCst); + Ok(()) + }) + .await; + // The in-memory transport intentionally has no process EOF to close the + // agent driver after the client continuation returns. + agent_task.abort(); + let _ = agent_task.await; + + let mut events = Vec::new(); + while let Ok(event) = event_rx.try_recv() { + events.push(event.payload.clone()); + } + let selectors_ready = state.read().await.selectors_ready; + let rpc_calls = rpc_calls.lock().expect("rpc log lock").clone(); + ForkModeGateProbe { + result, + rpc_calls, + events, + queued_prompt_released: queued_prompt_released.load(std::sync::atomic::Ordering::SeqCst), + selectors_ready, + } + } + + #[tokio::test] + async fn fork_mode_inheritance_restores_before_ready_and_releasing_queued_prompt() { + let probe = probe_fork_mode_gate("manual", Some("auto"), false).await; + + assert!(probe.result.is_ok(), "preparation failed: {:?}", probe.result); + assert_eq!(probe.rpc_calls, vec!["set_mode:auto"]); + assert!(matches!(probe.events.first(), Some(AcpEvent::ModeChanged { mode_id }) if mode_id == "auto")); + assert!(matches!(probe.events.last(), Some(AcpEvent::SelectorsReady))); + assert!(probe.selectors_ready); + assert!( + probe.queued_prompt_released, + "the queued prompt is released only after preparation returns" + ); + } + + #[tokio::test] + async fn fork_mode_inheritance_failure_blocks_ready_and_queued_prompt() { + let probe = probe_fork_mode_gate("manual", Some("auto"), true).await; + + let error = probe.result.expect_err("mode restore must fail closed"); + assert!( + error.to_string().contains("Failed to restore inherited mode 'auto' after fork") + && error.to_string().contains("queued prompts were not sent"), + "client-facing terminal error should explain the safety block: {error}" + ); + assert_eq!(probe.rpc_calls, vec!["set_mode:auto"]); + assert!(probe.events.is_empty(), "no readiness event may escape"); + assert!(!probe.selectors_ready); + assert!(!probe.queued_prompt_released); + } + + #[tokio::test] + async fn fork_mode_inheritance_noop_paths_reach_ready_without_set_mode() { + for inherited_mode in [None, Some("manual")] { + let probe = probe_fork_mode_gate("manual", inherited_mode, false).await; + assert!(probe.result.is_ok(), "preparation failed: {:?}", probe.result); + assert!(probe.rpc_calls.is_empty()); + assert!(matches!(probe.events.last(), Some(AcpEvent::SelectorsReady))); + assert!(probe.selectors_ready); + assert!(probe.queued_prompt_released); + } + } + // ── PermissionQueue (#442) ────────────────────────────────────────────── // // The queue is what stops N concurrent `session/request_permission`s from