diff --git a/src-tauri/src/acp/connection.rs b/src-tauri/src/acp/connection.rs index eedbdc4e4d..45eff65ebf 100644 --- a/src-tauri/src/acp/connection.rs +++ b/src-tauri/src/acp/connection.rs @@ -236,7 +236,10 @@ pub(crate) fn cursor_force_enabled(value: Option<&str>) -> bool { /// Gated on the explicit `CURSOR_AUTH_MODE` knob (written by the Cursor panel), /// so legacy rows and operator-provided container env are left untouched. In /// custom mode the credentials are present and non-empty, so nothing is cleared. -fn apply_cursor_env_policy(merged: &mut Vec<(String, String)>, runtime_env: &BTreeMap) { +fn apply_cursor_env_policy( + merged: &mut Vec<(String, String)>, + runtime_env: &BTreeMap, +) { if runtime_env.get("CURSOR_AUTH_MODE").map(String::as_str) != Some("subscription") { return; } @@ -260,7 +263,10 @@ fn apply_cursor_env_policy(merged: &mut Vec<(String, String)>, runtime_env: &BTr /// sacp-tokio) to `env_remove` the inherited var. In api_key mode the key is /// present and non-empty, so nothing is cleared; legacy/no-mode rows are left /// untouched. -fn apply_grok_env_policy(merged: &mut Vec<(String, String)>, runtime_env: &BTreeMap) { +fn apply_grok_env_policy( + merged: &mut Vec<(String, String)>, + runtime_env: &BTreeMap, +) { if runtime_env.get("GROK_AUTH_MODE").map(String::as_str) != Some("subscription") { return; } @@ -786,9 +792,9 @@ pub fn antigravity_effective_auth_type( .filter(|value| !value.is_empty()) // The server resolves the legacy spelling before it tests membership, // so a caller matching on canonical ids would otherwise miss it. - .map(|value| AntigravityAuthType::Declared( - canonical_antigravity_auth_method(value).to_string(), - )) + .map(|value| { + AntigravityAuthType::Declared(canonical_antigravity_auth_method(value).to_string()) + }) .unwrap_or(AntigravityAuthType::Absent) } @@ -1235,22 +1241,97 @@ fn tag_mcp_suspect( struct ConnectionCleanupGuard { connections: Arc>>, connection_id: String, + runtime: tokio::runtime::Handle, + delegation_release: Option>, } impl Drop for ConnectionCleanupGuard { fn drop(&mut self) { + if let Some(release) = self.delegation_release.take() { + release.mark_driver_done(); + return; + } if let Ok(mut guard) = self.connections.try_lock() { guard.remove(&self.connection_id); return; } let connections = self.connections.clone(); let connection_id = std::mem::take(&mut self.connection_id); - tokio::spawn(async move { + self.runtime.spawn(async move { connections.lock().await.remove(&connection_id); }); } } +/// Two-signal release barrier for a broker-owned process. A connection stops +/// occupying its external session only after the driver has finished and the +/// child has actually been reaped. Missing exit acknowledgement intentionally +/// leaves the entry busy. +struct DelegationReleaseBarrier { + driver_done: std::sync::atomic::AtomicBool, + reaped: std::sync::atomic::AtomicBool, + spawned: std::sync::atomic::AtomicBool, + released: std::sync::atomic::AtomicBool, + runtime: tokio::runtime::Handle, + connections: Arc>>, + connection_id: String, + task_id: Option, + broker: Option>, +} + +impl DelegationReleaseBarrier { + fn mark_spawned(&self) { + self.spawned + .store(true, std::sync::atomic::Ordering::SeqCst); + } + + fn mark_reaped(&self) { + self.reaped.store(true, std::sync::atomic::Ordering::SeqCst); + self.try_release(); + } + + fn mark_driver_done(&self) { + self.driver_done + .store(true, std::sync::atomic::Ordering::SeqCst); + // If the process never reached `on_spawn`, there is nothing to reap. + if !self.spawned.load(std::sync::atomic::Ordering::SeqCst) { + self.reaped.store(true, std::sync::atomic::Ordering::SeqCst); + } + self.try_release(); + } + + fn try_release(&self) { + if !self.driver_done.load(std::sync::atomic::Ordering::SeqCst) + || !self.reaped.load(std::sync::atomic::Ordering::SeqCst) + || self + .released + .swap(true, std::sync::atomic::Ordering::SeqCst) + { + return; + } + let connections = Arc::clone(&self.connections); + let connection_id = self.connection_id.clone(); + let task_id = self.task_id.clone(); + let broker = self.broker.clone(); + self.runtime.spawn(async move { + let removed = { + let mut map = connections.lock().await; + let owns_slot = task_id.as_deref().is_some_and(|task_id| { + map.get(&connection_id) + .is_some_and(|conn| conn.delegation_task_id.as_deref() == Some(task_id)) + }); + owns_slot + .then(|| map.remove(&connection_id)) + .flatten() + .is_some() + }; + if let (true, Some(broker), Some(task_id)) = (removed, broker, task_id) { + broker.connection_released(&task_id).await; + } + }); + } +} + /// Represents a single active ACP agent connection. pub struct AgentConnection { pub id: String, @@ -1301,6 +1382,17 @@ pub struct AgentConnection { /// the tree without waiting, so the agent may still be alive and still /// needs the backstop. pub child_pid: Arc, + /// Session id this process was asked to restore. Unlike + /// `SessionState::external_id`, this exists throughout the handshake and + /// therefore participates in admission before the agent replies. + pub requested_session_id: Option, + /// Immutable execution identity for a broker-owned connection. Lifecycle + /// routing and teardown compare this value instead of trusting the child + /// conversation's mutable `delegation_call_id`. + pub delegation_task_id: Option, + /// Cancels the whole driver future, including initialize/load handshakes + /// which have not started consuming `ConnectionCommand` yet. + pub driver_cancel: tokio_util::sync::CancellationToken, } impl AgentConnection { @@ -1520,9 +1612,7 @@ async fn record_turn_end( /// boolean — see `config_option_already_holds`). /// /// Used to carry a session's selectors across a fork. -fn current_config_option_values( - opts: &[SessionConfigOptionInfo], -) -> BTreeMap { +fn current_config_option_values(opts: &[SessionConfigOptionInfo]) -> BTreeMap { opts.iter() .map(|opt| { let value = match &opt.kind { @@ -1994,11 +2084,8 @@ async fn build_agent( .unwrap_or(false); let agent_name = meta.name.to_string(); let tail = Arc::clone(stderr_tail); - Ok( - AcpAgent::new(sacp::schema::McpServer::Stdio(server)).with_debug( - agent_debug_callback(agent_name, tail, stdio_debug_enabled), - ), - ) + Ok(AcpAgent::new(sacp::schema::McpServer::Stdio(server)) + .with_debug(agent_debug_callback(agent_name, tail, stdio_debug_enabled))) } AgentDistribution::Uvx { package, @@ -2100,6 +2187,17 @@ async fn build_agent( /// into boxed sub-futures rather than raising it further. const ACP_CONNECTION_STACK_SIZE: usize = 8 * 1024 * 1024; +/// How a connection may recover a requested external session. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SessionRecoveryPolicy { + /// Existing interactive behaviour: resume, then load, with the established + /// per-agent fallback to a fresh session where allowed. + BestEffort, + /// Broker continuation behaviour: the requested session must be restored; + /// no failure or missing capability may cross into `session/new`. + Strict, +} + /// Spawn an ACP agent process and run the connection loop in a background task. /// /// On success, the newly created `AgentConnection` is inserted into @@ -2121,6 +2219,8 @@ pub async fn spawn_agent_connection( preferred_config_values: BTreeMap, delegation_injection: Option, terminal_shell_config: TerminalShellRuntimeConfig, + recovery_policy: SessionRecoveryPolicy, + delegation_task_id: Option, ) -> Result, AcpError> { // Create the authoritative session state up front. Subsequent emit_with_state // calls write through this state and increment its seq counter so the first @@ -2169,6 +2269,22 @@ pub async fn spawn_agent_connection( // backstop when the connection driver thread is torn down by process exit // before `ChildGuard::drop` can run. 0 = not spawned yet / unknown. let child_pid = Arc::new(std::sync::atomic::AtomicU32::new(0)); + let connection_runtime = tokio::runtime::Handle::current(); + let delegation_release = delegation_task_id.is_some().then(|| { + Arc::new(DelegationReleaseBarrier { + driver_done: std::sync::atomic::AtomicBool::new(false), + reaped: std::sync::atomic::AtomicBool::new(false), + spawned: std::sync::atomic::AtomicBool::new(false), + released: std::sync::atomic::AtomicBool::new(false), + runtime: connection_runtime.clone(), + connections: Arc::clone(&connections), + connection_id: connection_id.clone(), + task_id: delegation_task_id.clone(), + broker: delegation_injection + .as_ref() + .map(|injection| Arc::clone(&injection.broker)), + }) + }); // Connection-scoped ring buffer of the agent's stderr, populated by the // `with_debug` callback `build_agent` installs and read at turn end when a // turn is diagnosed as silently empty. Created here so both the spawn side @@ -2178,7 +2294,13 @@ pub async fn spawn_agent_connection( .await? .on_spawn({ let child_pid = Arc::clone(&child_pid); - move |pid| child_pid.store(pid, std::sync::atomic::Ordering::SeqCst) + let delegation_release = delegation_release.clone(); + move |pid| { + child_pid.store(pid, std::sync::atomic::Ordering::SeqCst); + if let Some(release) = delegation_release.as_ref() { + release.mark_spawned(); + } + } }) // Paired with `on_spawn`: publish 0 again once the process has been // reaped, so the shutdown backstop can never `kill_tree` a pid the OS @@ -2188,7 +2310,13 @@ pub async fn spawn_agent_connection( // may still be running. .on_exit({ let child_pid = Arc::clone(&child_pid); - move || child_pid.store(0, std::sync::atomic::Ordering::SeqCst) + let delegation_release = delegation_release.clone(); + move || { + child_pid.store(0, std::sync::atomic::Ordering::SeqCst); + if let Some(release) = delegation_release.as_ref() { + release.mark_reaped(); + } + } }); // Path policy for the ACP `fs/*` channel. Built HERE rather than inside @@ -2222,6 +2350,7 @@ pub async fn spawn_agent_connection( prepend_officecli_path(&mut terminal_base_env); let (cmd_tx, cmd_rx) = mpsc::channel::(32); + let driver_cancel = tokio_util::sync::CancellationToken::new(); let conn_id = connection_id.clone(); let emitter_clone = emitter.clone(); let cleanup_connections = connections.clone(); @@ -2238,6 +2367,7 @@ pub async fn spawn_agent_connection( // Insert the entry BEFORE spawning the background task so that a // fast-failing `run_connection` can never remove it before it was // inserted (would otherwise leak the entry). + let requested_session_id = session_id.clone(); connections.lock().await.insert( connection_id.clone(), AgentConnection { @@ -2252,6 +2382,9 @@ pub async fn spawn_agent_connection( last_observed_fingerprint: config_fingerprint.clone(), config_fingerprint, child_pid, + requested_session_id, + delegation_task_id, + driver_cancel: driver_cancel.clone(), }, ); @@ -2264,7 +2397,7 @@ pub async fn spawn_agent_connection( // The connection is fire-and-forget (torn down from within via `cmd_rx` / // process exit; no JoinHandle is awaited), so a thread is behaviorally // equivalent to the previous task. - let connection_rt = tokio::runtime::Handle::current(); + let connection_rt = connection_runtime.clone(); // RAII guard built OUTSIDE the thread body and moved in: on a normal exit // or panic unwind its Drop removes the manager map entry, AND if the thread // fails to spawn the dropped closure runs the same Drop — so the entry is @@ -2272,6 +2405,8 @@ pub async fn spawn_agent_connection( let cleanup_guard = ConnectionCleanupGuard { connections: cleanup_connections, connection_id: cleanup_connection_id, + runtime: connection_runtime, + delegation_release, }; let connection_thread = std::thread::Builder::new() .name(format!("acp-conn-{conn_id}")) @@ -2279,92 +2414,99 @@ pub async fn spawn_agent_connection( .spawn(move || { let _cleanup = cleanup_guard; connection_rt.block_on(async move { - let delegation_for_cleanup = delegation_injection.clone(); - let result = run_connection( - agent, - conn_id.clone(), - agent_type, - working_dir, - session_id, - cmd_rx, - emitter_clone.clone(), - Arc::clone(&state_clone), - terminal_base_env, - terminal_shell_config, - preferred_mode_id, - preferred_config_values, - delegation_injection, - fs_policy, - host_tools, - stderr_tail, - ) - .await; + let delegation_for_cleanup = delegation_injection.clone(); + let connection = run_connection( + agent, + conn_id.clone(), + agent_type, + working_dir, + session_id, + cmd_rx, + emitter_clone.clone(), + Arc::clone(&state_clone), + terminal_base_env, + terminal_shell_config, + preferred_mode_id, + preferred_config_values, + delegation_injection, + fs_policy, + host_tools, + stderr_tail, + recovery_policy, + ); + tokio::pin!(connection); + let result = tokio::select! { + result = &mut connection => result, + _ = driver_cancel.cancelled() => { + Err(AcpError::protocol("connection driver canceled")) + } + }; - // Revoke the per-launch token + cascade cancel any still-pending - // delegations AND questions owned by this parent connection. All are - // best-effort: a missing token entry is a no-op, and both - // `cancel_by_parent` calls are safe on an empty pending map. - if let Some(inj) = delegation_for_cleanup { - let token = { - let snap = state_clone.read().await; - snap.delegation_token.clone() - }; - if let Some(tok) = token { - inj.tokens.revoke(&tok).await; - } - inj.broker.cancel_by_parent(&conn_id).await; - // Reclaim a parked `ask_user_question` instead of waiting for the - // companion's ask socket to close (which a reparented/hard-killed - // agent may never do); the dropped sender declines the tool cleanly. - inj.questions.cancel_questions_by_parent(&conn_id).await; - // Likewise reclaim a parked Grok `exit_plan_mode` approval; the - // dropped sender replies disconnect so grok keeps plan mode active. - inj.plan_approvals - .cancel_plan_approvals_by_parent(&conn_id) - .await; - } + // Revoke the per-launch token + cascade cancel any still-pending + // delegations AND questions owned by this parent connection. All are + // best-effort: a missing token entry is a no-op, and both + // `cancel_by_parent` calls are safe on an empty pending map. + if let Some(inj) = delegation_for_cleanup { + let token = { + let snap = state_clone.read().await; + snap.delegation_token.clone() + }; + if let Some(tok) = token { + inj.tokens.revoke(&tok).await; + } + inj.broker.cancel_by_parent(&conn_id).await; + // Reclaim a parked `ask_user_question` instead of waiting for the + // companion's ask socket to close (which a reparented/hard-killed + // agent may never do); the dropped sender declines the tool cleanly. + inj.questions.cancel_questions_by_parent(&conn_id).await; + // Likewise reclaim a parked Grok `exit_plan_mode` approval; the + // dropped sender replies disconnect so grok keeps plan mode active. + inj.plan_approvals + .cancel_plan_approvals_by_parent(&conn_id) + .await; + } - if let Err(e) = result { - let code = e.code().map(String::from); - emit_with_state( - &state_clone, - &emitter_clone, - AcpEvent::Error { - message: e.to_string(), - agent_type: agent_type.to_string(), - code, - details: None, - // The only genuinely terminal emit site: `run_connection` - // is unwinding and the next event is `Disconnected`. - // The lifecycle worker uses this flag to decide whether - // to flip the conversation row to Cancelled and to - // buffer the detail for the broker's cancel reason. - terminal: true, - }, - ) - .await; - // Drive the state machine through `Error` before `Disconnected` - // so the frontend's error-handling effect (cancelled-on-error) - // engages — without this hop the connection would jump straight - // to Disconnected and look like a clean shutdown. - emit_with_state( - &state_clone, - &emitter_clone, - AcpEvent::StatusChanged { - status: ConnectionStatus::Error, - }, - ) - .await; - } + if let Err(e) = result { + let code = e.code().map(String::from); + emit_with_state( + &state_clone, + &emitter_clone, + AcpEvent::Error { + message: e.to_string(), + agent_type: agent_type.to_string(), + code, + details: None, + // The only genuinely terminal emit site: `run_connection` + // is unwinding and the next event is `Disconnected`. + // The lifecycle worker uses this flag to decide whether + // to flip the conversation row to Cancelled and to + // buffer the detail for the broker's cancel reason. + terminal: true, + }, + ) + .await; + // Drive the state machine through `Error` before `Disconnected` + // so the frontend's error-handling effect (cancelled-on-error) + // engages — without this hop the connection would jump straight + // to Disconnected and look like a clean shutdown. + emit_with_state( + &state_clone, + &emitter_clone, + AcpEvent::StatusChanged { + status: ConnectionStatus::Error, + }, + ) + .await; + } - emit_with_state( - &state_clone, - &emitter_clone, - AcpEvent::StatusChanged { - status: ConnectionStatus::Disconnected, - }, - ) - .await; + emit_with_state( + &state_clone, + &emitter_clone, + AcpEvent::StatusChanged { + status: ConnectionStatus::Disconnected, + }, + ) + .await; // Connection loop ended; `block_on` returns and `_cleanup` // (bound at the top of the thread body) drops next, removing // the manager map entry — same as on a panic unwind. @@ -3403,9 +3545,8 @@ async fn send_steer_request( blocks: &[PromptInputBlock], ) -> Result { let params = build_steer_params(session_id.0.as_ref(), blocks); - let untyped_req = UntypedMessage::new("_session/steering", params).map_err(|e| { - AcpError::protocol(format!("Failed to build steering request: {e}")) - })?; + let untyped_req = UntypedMessage::new("_session/steering", params) + .map_err(|e| AcpError::protocol(format!("Failed to build steering request: {e}")))?; let raw = cx .send_request_to(Agent, untyped_req) .block_task() @@ -3448,9 +3589,8 @@ async fn send_stop_async_task_request( "sessionId": session_id.0.as_ref(), "asyncTaskId": task_id, }); - let untyped_req = UntypedMessage::new("_session/async_task/stop", params).map_err(|e| { - AcpError::protocol(format!("Failed to build async task stop request: {e}")) - })?; + let untyped_req = UntypedMessage::new("_session/async_task/stop", params) + .map_err(|e| AcpError::protocol(format!("Failed to build async task stop request: {e}")))?; let raw = cx .send_request_to(Agent, untyped_req) .block_task() @@ -3978,8 +4118,7 @@ fn build_client_capabilities( client_capabilities = client_capabilities.terminal(true).fs( FileSystemCapabilities::new() .read_text_file(true) - .write_text_file(true), - ); + .write_text_file(true)); } // Form elicitation is advertised only to agents that are KNOWN to send // spec-conformant `elicitation/create` forms `classify_elicitation` can @@ -4005,7 +4144,10 @@ fn build_client_capabilities( // convention is to advertise nothing an agent hasn't implemented. let mut meta = serde_json::Map::new(); if agent_type == AgentType::ClaudeCode { - meta.insert("subagent-transcript".to_string(), serde_json::Value::Bool(true)); + meta.insert( + "subagent-transcript".to_string(), + serde_json::Value::Bool(true), + ); } // claude-agent-acp 0.73.0 added "asyncTasks", and codex-acp 1.10.0 joined // it, so BOTH are advertised. It publishes the lifecycle of an agent's @@ -4205,9 +4347,8 @@ async fn send_resume_session( cx: &ConnectionTo, req: ResumeSessionRequest, ) -> Result<(ResumeSessionResponse, Option), sacp::Error> { - let untyped_req = UntypedMessage::new("session/resume", req).map_err(|e| { - sacp::util::internal_error(format!("Failed to build resume request: {e}")) - })?; + let untyped_req = UntypedMessage::new("session/resume", req) + .map_err(|e| sacp::util::internal_error(format!("Failed to build resume request: {e}")))?; let mut raw_response = cx.send_request_to(Agent, untyped_req).block_task().await?; // Capture the raw top-level `models` (per-model reasoning-effort data) BEFORE @@ -4215,9 +4356,8 @@ async fn send_resume_session( // field survives serde as an ignored unknown for other agents). let models = raw_response.get("models").cloned(); strip_unknown_config_options(&mut raw_response, "session/resume"); - let resp = serde_json::from_value(raw_response).map_err(|e| { - sacp::util::internal_error(format!("Failed to parse resume response: {e}")) - })?; + let resp = serde_json::from_value(raw_response) + .map_err(|e| sacp::util::internal_error(format!("Failed to parse resume response: {e}")))?; Ok((resp, models)) } @@ -4892,6 +5032,7 @@ async fn run_connection( // callback installed by `build_agent`. Read only when a turn ends without // agent output, to attach evidence to the synthesized error. stderr_tail: Arc, + recovery_policy: SessionRecoveryPolicy, ) -> Result<(), AcpError> { let pending_perms: PendingPermissions = Arc::new(tokio::sync::Mutex::new(PermissionQueue::default())); @@ -5846,6 +5987,11 @@ async fn run_connection( .await } Err(e) => { + if recovery_policy == SessionRecoveryPolicy::Strict { + return Err(sacp::util::internal_error(format!( + "strict session recovery failed for {sid}: {e}" + ))); + } // session/load failed. Classify it: an unrecoverable // historical session — the agent has no record of it // (ResourceNotFound, -32002) or the agent process/session @@ -6037,6 +6183,11 @@ async fn run_connection( } } } else { + if recovery_policy == SessionRecoveryPolicy::Strict { + return Err(sacp::util::internal_error( + "strict session recovery requires an external session id", + )); + } // Create new session let (new_resp, grok_models_raw) = send_new_session_capturing_models( &cx, @@ -6421,9 +6572,9 @@ async fn try_bridge_pi_select_ask( ) .await; let outcome = match option_id { - Some(option_id) => { - RequestPermissionOutcome::Selected(SelectedPermissionOutcome::new(option_id)) - } + Some(option_id) => RequestPermissionOutcome::Selected( + SelectedPermissionOutcome::new(option_id), + ), None => RequestPermissionOutcome::Cancelled, }; let _ = responder.respond(RequestPermissionResponse::new(outcome)); @@ -6668,11 +6819,11 @@ async fn handle_elicitation_request( let reaper_conn = connection_id.to_string(); let reaper_qid = registered.question_id.clone(); tokio::spawn(async move { - tokio::time::sleep(std::time::Duration::from_millis( - ms.saturating_add(2_000), - )) - .await; - reaper_access.cancel_question(&reaper_conn, &reaper_qid).await; + tokio::time::sleep(std::time::Duration::from_millis(ms.saturating_add(2_000))) + .await; + reaper_access + .cancel_question(&reaper_conn, &reaper_qid) + .await; }); } // The user answers out-of-band (the `answer_question` endpoint @@ -8660,9 +8811,9 @@ impl EmptyTurnCause { "{agent_type} produced output that codeg could not parse — \ the agent version may not match the protocol." ), - EmptyTurnCause::MetadataOnly => format!( - "{agent_type} sent only status updates this turn and no reply." - ), + EmptyTurnCause::MetadataOnly => { + format!("{agent_type} sent only status updates this turn and no reply.") + } } } } @@ -8963,9 +9114,15 @@ async fn run_conversation_loop<'a>( ); let cx = session.connection(); let sid = session.session_id().clone(); - if let Err(e) = - set_session_config_option(&cx, &sid, state, emitter, config_id.clone(), value_id) - .await + if let Err(e) = set_session_config_option( + &cx, + &sid, + state, + emitter, + config_id.clone(), + value_id, + ) + .await { // Advisory: the agent is running what it pushed and has already // told the frontend so. Failing the connection over a selector @@ -9167,6 +9324,12 @@ async fn run_conversation_loop<'a>( // to avoid deadlocking when the agent awaits a permission response. loop { tokio::select! { + // sacp routes wire notifications into `read_update` before + // routing a following prompt response, but both futures + // can be ready by the time this task is polled. Preserve + // that wire order so TurnComplete snapshots every queued + // assistant chunk instead of racing past the final text. + biased; update = session.read_update() => { let update = match update { Ok(u) => u, @@ -10257,7 +10420,9 @@ fn build_new_file_diff(path: &str, new_text: &str) -> String { /// on `AcpEvent::ToolCall(Update)` stays absent for non-image tool calls /// (preserves replace-on-update semantics: an absent field means "keep /// prior", a `Some(vec)` replaces). -pub(crate) fn extract_tool_call_images(content: &[ToolCallContent]) -> Option> { +pub(crate) fn extract_tool_call_images( + content: &[ToolCallContent], +) -> Option> { let mut imgs: Vec = Vec::new(); for item in content { if let ToolCallContent::Content(c) = item { @@ -10949,7 +11114,10 @@ fn is_subagent_invocation(agent_type: AgentType, raw_input: &Option) -> /// historical unwrap in `parsers/codebuddy.rs`. `raw_input` is left untouched /// (the cards peel `params` themselves, and that keeps `inferFromInput` from /// misclassifying `cancel_delegation`'s `{task_id}` as a generic task). -fn codebuddy_deferred_tool_name(agent_type: AgentType, raw_input: &Option) -> Option { +fn codebuddy_deferred_tool_name( + agent_type: AgentType, + raw_input: &Option, +) -> Option { if agent_type != AgentType::CodeBuddy { return None; } @@ -12634,7 +12802,12 @@ fn map_grok_subagent_notification_inner( .get("output") .and_then(|v| v.as_str()) .filter(|s| !s.is_empty()) - .map(|s| crate::parsers::truncate_str(s, crate::parsers::claude::BACKGROUND_RESULT_MAX_CHARS)); + .map(|s| { + crate::parsers::truncate_str( + s, + crate::parsers::claude::BACKGROUND_RESULT_MAX_CHARS, + ) + }); Some(vec![AcpEvent::BackgroundActivity { session_id: session_id.to_string(), turns: Vec::new(), @@ -13408,9 +13581,7 @@ async fn emit_conversation_update( Some((_, inner)) => { json_value_to_text(&Some(inner.clone())).filter(|t| !t.trim().is_empty()) } - None => { - json_value_to_text(&tcu.fields.raw_input).filter(|t| !t.trim().is_empty()) - } + None => json_value_to_text(&tcu.fields.raw_input).filter(|t| !t.trim().is_empty()), }; let synthesized_edit = if own_raw_input.is_none() { content_blocks.and_then(synthesize_edit_input_from_diffs) @@ -13517,7 +13688,13 @@ async fn emit_conversation_update( } // Symmetric with the ToolCall arm: an update may carry the terminal // status (and, on grok, usually re-carries the `x.ai/tool` meta). - track_grok_spawn_call(cb_state, grok_spawn, status.as_deref(), &tool_call_id, &raw_input); + track_grok_spawn_call( + cb_state, + grok_spawn, + status.as_deref(), + &tool_call_id, + &raw_input, + ); // Ordering variant: `subagent_spawned` can pair BEFORE the launch // call's terminal frame arrives. The pairing site skipped its // outstanding emission then (call not yet settled), so surface the @@ -13839,11 +14016,101 @@ async fn emit_conversation_update( } } +#[cfg(test)] +mod continuation_protocol_tests; + #[cfg(test)] mod tests { use super::*; use sacp::schema::{Diff, SessionConfigId}; + async fn release_barrier_fixture( + connection_id: &str, + ) -> ( + Arc>>, + Arc, + ) { + let connections = Arc::new(tokio::sync::Mutex::new(HashMap::new())); + let (cmd_tx, _cmd_rx) = mpsc::channel(1); + let task_id = format!("task-{connection_id}"); + connections.lock().await.insert( + connection_id.to_string(), + AgentConnection { + id: connection_id.to_string(), + agent_type: AgentType::Codex, + status: ConnectionStatus::Connected, + owner_window_label: "test-window".into(), + cmd_tx, + state: Arc::new(RwLock::new(SessionState::new( + connection_id.to_string(), + AgentType::Codex, + None, + "test-window".into(), + None, + ))), + emitter: EventEmitter::Noop, + prompt_lock: Arc::new(tokio::sync::Mutex::new(())), + config_fingerprint: String::new(), + last_observed_fingerprint: String::new(), + child_pid: Arc::new(std::sync::atomic::AtomicU32::new(0)), + requested_session_id: None, + delegation_task_id: Some(task_id.clone()), + driver_cancel: tokio_util::sync::CancellationToken::new(), + }, + ); + let barrier = Arc::new(DelegationReleaseBarrier { + driver_done: std::sync::atomic::AtomicBool::new(false), + reaped: std::sync::atomic::AtomicBool::new(false), + spawned: std::sync::atomic::AtomicBool::new(false), + released: std::sync::atomic::AtomicBool::new(false), + runtime: tokio::runtime::Handle::current(), + connections: Arc::clone(&connections), + connection_id: connection_id.to_string(), + task_id: Some(task_id), + broker: None, + }); + (connections, barrier) + } + + async fn wait_until_released( + connections: &Arc>>, + connection_id: &str, + ) { + tokio::time::timeout(std::time::Duration::from_secs(1), async { + loop { + if !connections.lock().await.contains_key(connection_id) { + break; + } + tokio::task::yield_now().await; + } + }) + .await + .expect("release barrier should remove the owned slot"); + } + + #[tokio::test] + async fn delegation_release_waits_for_driver_and_reap_in_either_order() { + let (connections, release) = release_barrier_fixture("driver-first").await; + release.mark_spawned(); + release.mark_driver_done(); + tokio::task::yield_now().await; + assert!(connections.lock().await.contains_key("driver-first")); + release.mark_reaped(); + wait_until_released(&connections, "driver-first").await; + + let (connections, release) = release_barrier_fixture("reap-first").await; + release.mark_spawned(); + release.mark_reaped(); + tokio::task::yield_now().await; + assert!(connections.lock().await.contains_key("reap-first")); + release.mark_driver_done(); + wait_until_released(&connections, "reap-first").await; + + let (connections, release) = release_barrier_fixture("never-spawned").await; + release.mark_driver_done(); + wait_until_released(&connections, "never-spawned").await; + } + /// 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 /// handle — this keeps the assertions as terse as the irrefutable `let` @@ -14458,9 +14725,7 @@ mod tests { assert!(!init_advertises_steering(Some(&off))); // Wrong nesting (e.g. another convention's namespace) must not count. - let nested = meta_map( - serde_json::json!({"symposium": {"steering": {"supported": true}}}), - ); + let nested = meta_map(serde_json::json!({"symposium": {"steering": {"supported": true}}})); assert!(!init_advertises_steering(Some(&nested))); // Non-bool / absent → false. @@ -14518,9 +14783,8 @@ mod tests { serde_json::json!({"codex": {"goal": {"objective": "legacy", "status": "active"}}}), ); assert!(session_info_goal_value(true, Some(&legacy_only)).is_none()); - let neutral_only = meta_map( - serde_json::json!({"goal": {"objective": "neutral", "status": "active"}}), - ); + let neutral_only = + meta_map(serde_json::json!({"goal": {"objective": "neutral", "status": "active"}})); assert!(session_info_goal_value(false, Some(&neutral_only)).is_none()); // `goal: null` IS a value (the clear signal), not an absent key. @@ -15652,10 +15916,7 @@ mod tests { } fn antigravity_runtime(method: &str) -> BTreeMap { - BTreeMap::from([( - ANTIGRAVITY_AUTH_METHOD_ENV.to_string(), - method.to_string(), - )]) + BTreeMap::from([(ANTIGRAVITY_AUTH_METHOD_ENV.to_string(), method.to_string())]) } #[test] @@ -15855,8 +16116,11 @@ mod tests { // whole enum exists for. A caller that treated it as "nothing there" // would sign out of a `gemini-api-key` connection, clear nothing, and // be told `{}`. - std::fs::write(&path, "{\n // mine\n \"auth\": {\"type\": \"oauth-personal\"},\n}\n") - .unwrap(); + std::fs::write( + &path, + "{\n // mine\n \"auth\": {\"type\": \"oauth-personal\"},\n}\n", + ) + .unwrap(); assert_eq!( antigravity_effective_auth_type(&home()), AntigravityAuthType::Unreadable @@ -15995,9 +16259,14 @@ mod tests { // No file at all: created from scratch. (A non-object ROOT never gets // here — the read side already refused it.) - let created = merge_antigravity_settings(None, "oauth-personal", GcpField::Set("p"), GcpField::Set("global")) - .expect("editable") - .expect("created"); + let created = merge_antigravity_settings( + None, + "oauth-personal", + GcpField::Set("p"), + GcpField::Set("global"), + ) + .expect("editable") + .expect("created"); assert_eq!(created["auth"]["type"], "oauth-personal"); assert_eq!(created["gcp"]["project"], "p"); assert_eq!(created["gcp"]["location"], "global"); @@ -16015,18 +16284,21 @@ mod tests { // into it. With no project or location supplied, a strange `gcp` is // none of codeg's business and must not block the `auth.type` update. let odd_gcp = serde_json::json!({ "gcp": ["not", "an", "object"] }); - assert!( - merge_antigravity_settings( - Some(odd_gcp.clone()), - "oauth-personal", - GcpField::Set("p"), - GcpField::Keep, - ) - .is_err() - ); - let untouched = merge_antigravity_settings(Some(odd_gcp), "oauth-personal", GcpField::Keep, GcpField::Keep) - .expect("editable") - .expect("auth.type still written"); + assert!(merge_antigravity_settings( + Some(odd_gcp.clone()), + "oauth-personal", + GcpField::Set("p"), + GcpField::Keep, + ) + .is_err()); + let untouched = merge_antigravity_settings( + Some(odd_gcp), + "oauth-personal", + GcpField::Keep, + GcpField::Keep, + ) + .expect("editable") + .expect("auth.type still written"); assert_eq!(untouched["auth"]["type"], "oauth-personal"); assert_eq!(untouched["gcp"], serde_json::json!(["not", "an", "object"])); @@ -16063,10 +16335,14 @@ mod tests { }; // The panel owns both fields for this method and both are now empty. - let cleared = - merge_antigravity_settings(Some(existing()), "oauth-business", GcpField::Clear, GcpField::Clear) - .expect("editable") - .expect("the gcp block changed, so this is a real write"); + let cleared = merge_antigravity_settings( + Some(existing()), + "oauth-business", + GcpField::Clear, + GcpField::Clear, + ) + .expect("editable") + .expect("the gcp block changed, so this is a real write"); assert!( cleared.get("gcp").is_none(), "an emptied block should go rather than linger as {{}}: {cleared}" @@ -16075,10 +16351,14 @@ mod tests { assert_eq!(cleared["keep"], 1, "foreign keys still survive a clear"); // One cleared, one set. - let partial = - merge_antigravity_settings(Some(existing()), "oauth-business", GcpField::Set("new"), GcpField::Clear) - .expect("editable") - .expect("changed"); + let partial = merge_antigravity_settings( + Some(existing()), + "oauth-business", + GcpField::Set("new"), + GcpField::Clear, + ) + .expect("editable") + .expect("changed"); assert_eq!(partial["gcp"]["project"], "new"); assert!(partial["gcp"].get("location").is_none()); @@ -16099,10 +16379,14 @@ mod tests { // take the `auth.type` update down with it — the one part of this file // the agent cannot start without. let odd = serde_json::json!({ "gcp": ["not", "an", "object"] }); - let still_written = - merge_antigravity_settings(Some(odd), "oauth-business", GcpField::Clear, GcpField::Clear) - .expect("a clear must not refuse a block it cannot edit") - .expect("auth.type still written"); + let still_written = merge_antigravity_settings( + Some(odd), + "oauth-business", + GcpField::Clear, + GcpField::Clear, + ) + .expect("a clear must not refuse a block it cannot edit") + .expect("auth.type still written"); assert_eq!(still_written["auth"]["type"], "oauth-business"); assert_eq!( still_written["gcp"], @@ -16549,7 +16833,12 @@ mod tests { #[test] fn prepend_path_windows_seeds_from_fallback_with_semicolon() { let mut env = BTreeMap::new(); - prepend_dir_to_path_env(&mut env, r"C:\OfficeCLI", r"C:\Windows;C:\Windows\System32", true); + prepend_dir_to_path_env( + &mut env, + r"C:\OfficeCLI", + r"C:\Windows;C:\Windows\System32", + true, + ); // No prior key → default `Path` casing on Windows. assert_eq!(env.get("Path").unwrap(), r"C:\OfficeCLI;C:\Windows;C:\Windows\System32"); } @@ -18018,7 +18307,10 @@ mod tests { #[test] fn note_dropped_counts_each_site_separately_and_keeps_the_first() { let mut probe = TurnOutputProbe::new(0); - probe.note_dropped(DropSite::Dispatch, &drop_err("missing field `sessionUpdate`")); + probe.note_dropped( + DropSite::Dispatch, + &drop_err("missing field `sessionUpdate`"), + ); probe.note_dropped(DropSite::Decode, &drop_err("missing field `update`")); probe.note_dropped(DropSite::Decode, &drop_err("missing field `content`")); @@ -18337,9 +18629,9 @@ mod tests { sacp::schema::SessionConfigSelectOptions::Ungrouped(Vec::new()), )) } - SessionConfigKindInfo::Boolean(b) => { - SessionConfigKind::Boolean(sacp::schema::SessionConfigBoolean::new(b.current_value)) - } + SessionConfigKindInfo::Boolean(b) => SessionConfigKind::Boolean( + sacp::schema::SessionConfigBoolean::new(b.current_value), + ), }, ); let extracted = values.get(&opt.id).expect("every option is extracted"); @@ -19465,12 +19757,7 @@ mod tests { cache: &mut ToolCallOutputCache, cb: &mut CodeBuddyLiveState, wire: serde_json::Value, - ) -> ( - Option, - Option, - Option, - Option, - ) { + ) -> (Option, Option, Option, Option) { let st = SessionState::new( "conn-pi".to_string(), agent_type, @@ -19495,12 +19782,7 @@ mod tests { raw_input, raw_output, .. - } => Some(( - content.clone(), - raw_input.clone(), - raw_output.clone(), - None, - )), + } => Some((content.clone(), raw_input.clone(), raw_output.clone(), None)), AcpEvent::ToolCallUpdate { content, raw_input, @@ -20356,8 +20638,11 @@ mod tests { /// own counters so the banner can render its localized line. #[tokio::test] async fn pi_retry_chunk_becomes_the_retry_banner_with_counters() { - let events = - pi_emit_chunk(AgentType::Pi, pi_chunk("Retrying (attempt 2/3, waiting 4s)...")).await; + let events = pi_emit_chunk( + AgentType::Pi, + pi_chunk("Retrying (attempt 2/3, waiting 4s)..."), + ) + .await; assert!( !events .iter() diff --git a/src-tauri/src/acp/connection/continuation_protocol_tests.rs b/src-tauri/src/acp/connection/continuation_protocol_tests.rs new file mode 100644 index 0000000000..876d4908a7 --- /dev/null +++ b/src-tauri/src/acp/connection/continuation_protocol_tests.rs @@ -0,0 +1,671 @@ +use super::*; +use async_trait::async_trait; +use sea_orm::Database; +use serde_json::{json, Value}; +use std::{future::Future, str::FromStr, time::Duration}; + +use crate::acp::delegation::broker::{DbDepthLookup, DelegationBroker, DelegationConfig}; +use crate::acp::delegation::spawner::{ + ConnectionSpawner, DelegationDispatch, DelegationLink, ResumedSpawn, SpawnerError, +}; +use crate::acp::delegation::types::{ + DelegationOutcome, DelegationRequest, DelegationSuccess, DelegationTaskReport, TaskStatus, +}; +use crate::db::service::delegation_task_service as ledger; +use crate::db::service::{conversation_service, folder_service}; +use crate::db::test_helpers::fresh_disk_db; + +// Process creation and strict session recovery can approach five seconds on a +// saturated Windows CI runner. Keep protocol-response assertions at five +// seconds, but give process-bound phases a separate, still-bounded budget. +const FIXTURE_PROCESS_TIMEOUT: Duration = Duration::from_secs(30); + +fn run_on_large_stack(future: impl Future + Send + 'static) -> T { + std::thread::Builder::new() + .name("continuation-protocol-test".into()) + .stack_size(64 * 1024 * 1024) + .spawn(move || { + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap() + .block_on(future) + }) + .unwrap() + .join() + .unwrap() +} + +fn fixture_agent(mode: &str, log: &Path) -> AcpAgent { + let script = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/bounded_continuation_agent.py"); + let command = json!({ + "type": "stdio", + "name": "bounded-continuation", + "command": "python3", + "args": [script, mode, log], + "env": [] + }); + AcpAgent::from_str(&command.to_string()).unwrap() +} + +async fn run_driver( + mode: &'static str, +) -> ( + tempfile::TempDir, + PathBuf, + Arc>, + mpsc::Sender, + tokio::task::JoinHandle>, + tokio::sync::oneshot::Receiver<()>, +) { + let dir = tempfile::tempdir().unwrap(); + let log = dir.path().join("wire.jsonl"); + let agent = fixture_agent(mode, &log).with_current_dir(dir.path()); + let mut initial = SessionState::new( + "continuation-test".into(), + AgentType::ClaudeCode, + Some(dir.path().to_path_buf()), + "test".into(), + None, + ); + let started = initial.install_session_started_signal(); + let state = Arc::new(RwLock::new(initial)); + let (tx, rx) = mpsc::channel(8); + let worker_state = Arc::clone(&state); + let cwd = dir.path().to_path_buf(); + let driver = tokio::spawn(async move { + run_connection( + agent, + "continuation-test".into(), + AgentType::ClaudeCode, + Some(cwd.to_string_lossy().into_owned()), + Some("source-session".into()), + rx, + EventEmitter::Noop, + worker_state, + BTreeMap::new(), + TerminalShellRuntimeConfig::default(), + Some("plan".into()), + BTreeMap::from([("model".into(), "source-model".into())]), + None, + FsAccessPolicy::from_env(&cwd, AgentType::ClaudeCode, &BTreeMap::new()), + HostToolsPolicy::Default, + Arc::new(StderrTail::new()), + SessionRecoveryPolicy::Strict, + ) + .await + }); + (dir, log, state, tx, driver, started) +} + +fn wire_methods(log: &Path) -> Vec { + std::fs::read_to_string(log) + .unwrap_or_default() + .lines() + .map(|line| serde_json::from_str(line).unwrap()) + .collect() +} + +struct FixtureConnection { + tx: mpsc::Sender, + state: Arc>, + driver: tokio::task::JoinHandle>, +} + +/// Production-shaped continuation spawner backed by the deterministic ACP +/// fixture. It performs the same durable admission immediately before the +/// prompt, while the connection itself exercises the real strict-resume +/// protocol in `run_connection`. +struct FixtureContinuationSpawner { + db: Arc, + working_dir: PathBuf, + log: PathBuf, + child_conversation_id: i32, + connection: tokio::sync::Mutex>, +} + +#[async_trait] +impl ConnectionSpawner for FixtureContinuationSpawner { + async fn spawn( + &self, + _parent_connection_id: &str, + _agent_type: AgentType, + _working_dir: Option, + _preferred_mode_id: Option, + _preferred_config_values: BTreeMap, + ) -> Result { + Err(SpawnerError::Spawn( + "fixture only supports durable continuation".into(), + )) + } + + async fn spawn_for_delegation( + &self, + _parent_connection_id: &str, + agent_type: AgentType, + working_dir: Option, + preferred_mode_id: Option, + preferred_config_values: BTreeMap, + _task_id: String, + resume_binding: Option, + ) -> Result { + let binding = resume_binding + .ok_or_else(|| SpawnerError::Spawn("expected continuation binding".into()))?; + if binding.agent_type != agent_type { + return Err(SpawnerError::Spawn("agent binding changed".into())); + } + let cwd = working_dir + .map(PathBuf::from) + .unwrap_or_else(|| self.working_dir.clone()); + let connection_id = "child-after-reopen".to_string(); + let agent = fixture_agent("resume_ok", &self.log).with_current_dir(&cwd); + let mut initial = SessionState::new( + connection_id.clone(), + agent_type, + Some(cwd.clone()), + "reopened-parent-window".into(), + None, + ); + let started = initial.install_session_started_signal(); + let state = Arc::new(RwLock::new(initial)); + let (tx, rx) = mpsc::channel(8); + let worker_state = Arc::clone(&state); + let driver_connection_id = connection_id.clone(); + let driver = tokio::spawn(async move { + run_connection( + agent, + driver_connection_id, + agent_type, + Some(cwd.to_string_lossy().into_owned()), + Some(binding.external_session_id), + rx, + EventEmitter::Noop, + worker_state, + BTreeMap::new(), + TerminalShellRuntimeConfig::default(), + preferred_mode_id, + preferred_config_values, + None, + FsAccessPolicy::from_env(&cwd, agent_type, &BTreeMap::new()), + HostToolsPolicy::Default, + Arc::new(StderrTail::new()), + SessionRecoveryPolicy::Strict, + ) + .await + }); + match tokio::time::timeout(FIXTURE_PROCESS_TIMEOUT, started).await { + Ok(Ok(())) => {} + Ok(Err(_)) => { + driver.abort(); + return Err(SpawnerError::Spawn( + "fixture resume stopped before announcing its session".into(), + )); + } + Err(_) => { + driver.abort(); + return Err(SpawnerError::Spawn(format!( + "fixture resume timed out after {FIXTURE_PROCESS_TIMEOUT:?}; wire={:?}", + wire_methods(&self.log) + ))); + } + } + *self.connection.lock().await = Some(FixtureConnection { tx, state, driver }); + Ok(connection_id) + } + + async fn send_prompt_linked_for_delegation( + &self, + _conn_id: &str, + task: String, + link: DelegationLink, + ) -> Result { + let admission = link + .admission + .ok_or_else(|| SpawnerError::Send("missing durable admission".into()))?; + let resume_binding = admission + .resume_binding + .ok_or_else(|| SpawnerError::Send("missing continuation binding".into()))?; + let result = ledger::admit_continuation( + &self.db.conn, + ledger::AdmissionInput { + task_id: link.delegation_call_id, + parent_conversation_id: link.parent_conversation_id, + child_conversation_id: self.child_conversation_id, + source_task_id: admission.source_task_id, + task: admission.task, + requested_working_dir: admission.requested_working_dir, + resume_binding, + }, + ) + .await + .map_err(|error| SpawnerError::Send(error.to_string()))?; + if let ledger::AdmissionResult::Existing { entry } = result { + return Ok(DelegationDispatch::Existing(entry.report)); + } + if let ledger::AdmissionResult::Conflict { + next_task_id, + reason, + } = result + { + return Ok(DelegationDispatch::Conflict { + next_task_id, + reason, + }); + } + + let (tx, state) = { + let connection = self.connection.lock().await; + let connection = connection + .as_ref() + .ok_or_else(|| SpawnerError::Send("fixture connection missing".into()))?; + (connection.tx.clone(), Arc::clone(&connection.state)) + }; + tx.send(ConnectionCommand::Prompt { + blocks: vec![PromptInputBlock::Text { text: task }], + user_message: None, + }) + .await + .map_err(|error| SpawnerError::Send(error.to_string()))?; + tokio::time::timeout(Duration::from_secs(5), async { + loop { + let snapshot = state.read().await; + if snapshot.status == ConnectionStatus::Connected + && snapshot.last_assistant_text.as_deref() == Some("immediate reply 1") + { + break; + } + drop(snapshot); + tokio::task::yield_now().await; + } + }) + .await + .map_err(|_| SpawnerError::Send("fixture continuation prompt timed out".into()))?; + Ok(DelegationDispatch::Started(self.child_conversation_id)) + } + + async fn spawn_for_resume( + &self, + _parent_connection_id: &str, + _task_id: &str, + _agent_type: AgentType, + _working_dir: Option, + _external_session_id: &str, + _preferred_mode_id: Option, + _preferred_config_values: BTreeMap, + ) -> Result { + Err(SpawnerError::Spawn( + "legacy resume is outside this fixture".into(), + )) + } + + async fn send_resume_prompt( + &self, + _conn_id: &str, + _prompt: String, + _folder_id: i32, + _child_conversation_id: i32, + _link: DelegationLink, + ) -> Result<(), SpawnerError> { + Err(SpawnerError::Send( + "legacy resume is outside this fixture".into(), + )) + } + + async fn has_live_connection_for_conversation(&self, conversation_id: i32) -> bool { + conversation_id == self.child_conversation_id && self.connection.lock().await.is_some() + } + + async fn cancel(&self, _conn_id: &str) -> Result<(), SpawnerError> { + if let Some(connection) = self.connection.lock().await.as_ref() { + connection + .tx + .send(ConnectionCommand::Cancel) + .await + .map_err(|error| SpawnerError::Cancel(error.to_string()))?; + } + Ok(()) + } + + async fn disconnect(&self, _conn_id: &str) -> Result<(), SpawnerError> { + let Some(connection) = self.connection.lock().await.take() else { + return Ok(()); + }; + connection + .tx + .send(ConnectionCommand::Disconnect) + .await + .map_err(|error| SpawnerError::Disconnect(error.to_string()))?; + let mut driver = connection.driver; + let joined = match tokio::time::timeout(FIXTURE_PROCESS_TIMEOUT, &mut driver).await { + Ok(joined) => joined, + Err(_) => { + driver.abort(); + return Err(SpawnerError::Disconnect(format!( + "fixture disconnect timed out after {FIXTURE_PROCESS_TIMEOUT:?}; wire={:?}", + wire_methods(&self.log) + ))); + } + }; + joined + .map_err(|error| SpawnerError::Disconnect(error.to_string()))? + .map_err(|error| SpawnerError::Disconnect(error.to_string()))?; + Ok(()) + } +} + +#[test] +fn strict_resume_never_falls_back_to_new() { + for mode in ["unsupported", "load_fail"] { + run_on_large_stack(async move { + let (_dir, log, state, _tx, mut driver, _started) = run_driver(mode).await; + let error = match tokio::time::timeout(FIXTURE_PROCESS_TIMEOUT, &mut driver).await { + Ok(joined) => joined + .expect("strict-recovery fixture driver panicked") + .expect_err("strict recovery unexpectedly succeeded"), + Err(_) => { + driver.abort(); + panic!( + "strict-recovery fixture timed out after {FIXTURE_PROCESS_TIMEOUT:?}; mode={mode}; wire={:?}", + wire_methods(&log) + ); + } + }; + assert!(error.to_string().contains("strict session recovery failed")); + assert!( + state.read().await.external_id.is_none(), + "a failed strict recovery must not announce a session" + ); + let wire = wire_methods(&log); + assert!(!wire + .iter() + .any(|message| message["method"] == "session/new")); + assert!(!wire + .iter() + .any(|message| message["method"] == "session/prompt")); + }); + } +} + +#[test] +fn immediate_text_is_reduced_before_prompt_response_every_time() { + run_on_large_stack(async { + let (_dir, log, state, tx, mut driver, started) = run_driver("resume_ok").await; + match tokio::time::timeout(FIXTURE_PROCESS_TIMEOUT, started).await { + Ok(Ok(())) => {} + Ok(Err(_)) => panic!("fixture stopped before announcing its session"), + Err(_) => { + driver.abort(); + panic!( + "fixture session startup timed out after {FIXTURE_PROCESS_TIMEOUT:?}; wire={:?}", + wire_methods(&log) + ); + } + } + + for turn in 1..=50 { + tx.send(ConnectionCommand::Prompt { + blocks: vec![PromptInputBlock::Text { + text: format!("turn {turn}"), + }], + user_message: None, + }) + .await + .unwrap(); + let expected = format!("immediate reply {turn}"); + tokio::time::timeout(Duration::from_secs(5), async { + loop { + let snapshot = state.read().await; + if snapshot.status == ConnectionStatus::Connected + && snapshot.last_assistant_text.as_deref() == Some(expected.as_str()) + { + break; + } + drop(snapshot); + tokio::task::yield_now().await; + } + }) + .await + .unwrap_or_else(|_| panic!("turn {turn} completed without its queued text")); + } + + tx.send(ConnectionCommand::Disconnect).await.unwrap(); + match tokio::time::timeout(FIXTURE_PROCESS_TIMEOUT, &mut driver).await { + Ok(joined) => joined + .expect("fixture driver panicked during disconnect") + .expect("fixture driver failed during disconnect"), + Err(_) => { + driver.abort(); + panic!( + "fixture disconnect timed out after {FIXTURE_PROCESS_TIMEOUT:?}; wire={:?}", + wire_methods(&log) + ); + } + } + let wire = wire_methods(&log); + let prompt = wire + .iter() + .position(|message| message["method"] == "session/prompt") + .unwrap(); + for method in ["session/set_mode", "session/set_config_option"] { + assert!( + wire.iter() + .position(|message| message["method"] == method) + .unwrap() + < prompt + ); + } + assert_eq!(wire[prompt]["params"]["sessionId"], "source-session"); + assert!(!wire + .iter() + .any(|message| message["method"] == "session/new")); + }); +} + +#[test] +fn durable_continuation_reopens_the_source_session_and_admits_one_successor() { + run_on_large_stack(async { + let dir = tempfile::tempdir().expect("tempdir"); + let working_dir = std::fs::canonicalize(dir.path()) + .expect("canonical working directory") + .to_string_lossy() + .into_owned(); + let db = fresh_disk_db(dir.path()).await; + let folder = folder_service::add_folder(&db.conn, &working_dir) + .await + .expect("folder"); + let parent = conversation_service::create( + &db.conn, + folder.id, + AgentType::ClaudeCode, + Some("parent".into()), + None, + ) + .await + .expect("parent conversation"); + let source_task_id = "source-task"; + let child = conversation_service::create_with_delegation( + &db.conn, + folder.id, + AgentType::ClaudeCode, + Some("source child".into()), + None, + Some(DelegationLink { + parent_conversation_id: parent.id, + parent_tool_use_id: "source-tool".into(), + delegation_call_id: source_task_id.into(), + admission: None, + }), + ) + .await + .expect("child conversation"); + conversation_service::bind_external_id(&db.conn, child.id, "source-session", &[]) + .await + .expect("bind source session"); + + let source_binding = ledger::ResumeBinding { + agent_type: AgentType::ClaudeCode, + external_session_id: "source-session".into(), + child_conversation_id: child.id, + working_dir: working_dir.clone(), + preferred_mode_id: Some("plan".into()), + preferred_config_values: BTreeMap::from([("model".into(), "source-model".into())]), + config_fingerprint: "source-config".into(), + }; + let admitted = ledger::admit( + &db.conn, + ledger::AdmissionInput { + task_id: source_task_id.into(), + parent_conversation_id: parent.id, + child_conversation_id: child.id, + source_task_id: None, + task: "first round".into(), + requested_working_dir: Some(working_dir.clone()), + resume_binding: source_binding, + }, + ) + .await + .expect("admit source task"); + assert!(matches!(admitted, ledger::AdmissionResult::New { .. })); + let source_report = DelegationTaskReport { + task_id: Some(source_task_id.into()), + status: TaskStatus::Completed, + child_conversation_id: Some(child.id), + agent_type: Some(AgentType::ClaudeCode), + text: Some("first round complete".into()), + error_code: None, + message: None, + duration_ms: Some(1), + blocked_on: None, + }; + assert!( + ledger::finish(&db.conn, parent.id, source_task_id, &source_report) + .await + .expect("finish source task") + ); + assert!(ledger::mark_released(&db.conn, parent.id, source_task_id) + .await + .expect("release source task")); + db.conn.close().await.expect("close source database"); + + let reopened_conn = Database::connect(format!( + "sqlite:{}?mode=rwc", + dir.path().join("source.db").to_string_lossy() + )) + .await + .expect("reopen database"); + let reopened = Arc::new(crate::db::AppDatabase { + conn: reopened_conn, + }); + let log = dir.path().join("reopened-wire.jsonl"); + let spawner = Arc::new(FixtureContinuationSpawner { + db: Arc::clone(&reopened), + working_dir: dir.path().to_path_buf(), + log: log.clone(), + child_conversation_id: child.id, + connection: tokio::sync::Mutex::new(None), + }); + let broker = DelegationBroker::new( + spawner.clone() as Arc, + Arc::new(DbDepthLookup { + db: Arc::clone(&reopened), + }), + ) + .with_ledger(Arc::clone(&reopened)); + broker + .set_config(DelegationConfig { + enabled: true, + ..DelegationConfig::default() + }) + .await; + + let ack = broker + .start_delegation(DelegationRequest { + parent_connection_id: "parent-after-reopen".into(), + parent_conversation_id: parent.id, + parent_tool_use_id: "follow-up-tool".into(), + agent_type: AgentType::ClaudeCode, + task: "second round".into(), + working_dir: Some(working_dir.clone()), + requested_working_dir: Some(working_dir), + continue_from_task_id: Some(source_task_id.into()), + external_handle: None, + }) + .await; + assert_eq!(ack.status, TaskStatus::Running, "{ack:?}"); + assert_eq!(ack.child_conversation_id, Some(child.id)); + let successor_task_id = ack.task_id.clone().expect("successor task id"); + assert_ne!(successor_task_id, source_task_id); + + let wire = wire_methods(&log); + let resumed = wire + .iter() + .position(|message| message["method"] == "session/resume") + .expect("strict resume request"); + let prompted = wire + .iter() + .position(|message| message["method"] == "session/prompt") + .expect("continuation prompt"); + assert_eq!( + wire.iter() + .filter(|message| message["method"] == "session/prompt") + .count(), + 1 + ); + assert!(resumed < prompted); + assert_eq!(wire[resumed]["params"]["sessionId"], "source-session"); + assert_eq!(wire[prompted]["params"]["sessionId"], "source-session"); + assert_eq!( + wire[prompted]["params"]["prompt"][0]["text"], + "second round" + ); + assert!(!wire + .iter() + .any(|message| message["method"] == "session/new")); + + let successor = ledger::successor(&reopened.conn, parent.id, source_task_id) + .await + .expect("successor lookup") + .expect("successor ledger row"); + assert_eq!(successor.task_id, successor_task_id); + assert_eq!(successor.source_task_id.as_deref(), Some(source_task_id)); + assert_eq!(successor.status, TaskStatus::Running); + assert_eq!( + successor.resume_binding.external_session_id, + "source-session" + ); + + broker + .complete_call( + &successor_task_id, + DelegationOutcome::Ok(DelegationSuccess { + text: "second round complete".into(), + child_conversation_id: child.id, + child_agent_type: AgentType::ClaudeCode, + turn_count: 1, + duration_ms: 1, + token_usage: None, + }), + ) + .await; + let terminal = ledger::lookup(&reopened.conn, parent.id, &successor_task_id) + .await + .expect("terminal lookup") + .expect("terminal ledger row"); + assert_eq!(terminal.status, TaskStatus::Completed); + assert_eq!( + terminal.report.text.as_deref(), + Some("second round complete") + ); + let original = ledger::lookup(&reopened.conn, parent.id, source_task_id) + .await + .expect("source lookup after successor completion") + .expect("source ledger row"); + assert_eq!(original.status, TaskStatus::Completed); + assert_eq!( + original.report.text.as_deref(), + Some("first round complete") + ); + assert!(spawner.connection.lock().await.is_none()); + }); +} diff --git a/src-tauri/src/acp/delegation/broker.rs b/src-tauri/src/acp/delegation/broker.rs index f62029a986..a7266a31fa 100644 --- a/src-tauri/src/acp/delegation/broker.rs +++ b/src-tauri/src/acp/delegation/broker.rs @@ -64,7 +64,9 @@ use crate::acp::delegation::live_reply::{ChildLiveReplyLookup, NoopChildLiveRepl use crate::acp::delegation::meta_writer::{ build_delegation_meta, is_synthetic_parent_tool_use_id, DelegationMetaWriter, NoopMetaWriter, }; -use crate::acp::delegation::spawner::{ConnectionSpawner, DelegationLink}; +use crate::acp::delegation::spawner::{ + ConnectionSpawner, DelegationAdmission, DelegationDispatch, DelegationLink, +}; use crate::acp::delegation::types::{ AgentDelegationDefaults, BlockedKind, BlockedOn, DelegationError, DelegationOutcome, DelegationRequest, DelegationTaskReport, ResumeDelegationRequest, TaskStatus, @@ -90,13 +92,6 @@ const DEFAULT_COMPLETED_CACHE_CAP_BYTES: usize = 512 * 1024 * 1024; /// never the eviction victim in `insert_completed`. const COMPLETED_TEXT_CAP: usize = 256 * 1024; -/// Cap on the `task_preview` carried by the `DelegationStarted` event and the -/// parent-card meta writes. The full task text lives in the MCP call (and, on -/// most hosts, in the parent tool call's own `raw_input`); the preview only -/// has to label the delegation card, so it shares the status-preview budget -/// rather than the multi-KiB result cap. -const TASK_PREVIEW_CAP: usize = 2 * 1024; - /// Cap on the inline `text_preview` carried by the `DelegationCompleted` event /// and the terminal meta, so the parent card can render the result inline /// without re-fetching the child session. @@ -228,10 +223,12 @@ struct RunningTask { child_connection_id: String, child_conversation_id: i32, parent_connection_id: String, + parent_conversation_id: i32, parent_tool_use_id: String, /// Target agent — surfaced in status reports. agent_type: AgentType, - /// Bounded preview of the delegated task text ([`TASK_PREVIEW_CAP`]). + /// Bounded preview of the delegated task text + /// ([`super::TASK_PREVIEW_CAP`]). /// Carried so TERMINAL meta writes can keep labeling the parent card — /// meta is replace-wholesale on the ToolCallState, so a terminal write /// that dropped the task text would erase what the running write supplied. @@ -864,7 +861,7 @@ fn running_ack( /// Cap on the free-form `reason` a `resume_delegation` call may carry into the /// child's continuation prompt. The reason is interruption CONTEXT, not new -/// instructions — the cap (shared with `TASK_PREVIEW_CAP`'s budget) keeps it +/// instructions — the cap (shared with the task-preview budget) keeps it /// from smuggling a task-sized payload past the "no new iterations" contract. const RESUME_REASON_CAP: usize = 2 * 1024; @@ -1075,6 +1072,25 @@ fn unknown_report(task_id: &str) -> DelegationTaskReport { } } +fn interrupted_ledger_report( + entry: &crate::db::service::delegation_task_service::TaskLedgerEntry, +) -> DelegationTaskReport { + DelegationTaskReport { + task_id: Some(entry.task_id.clone()), + status: TaskStatus::Unknown, + child_conversation_id: Some(entry.child_conversation_id), + agent_type: Some(entry.resume_binding.agent_type), + text: None, + error_code: Some("interrupted".into()), + message: Some( + "The application stopped while this delegation was running; its outcome is unknown." + .into(), + ), + duration_ms: None, + blocked_on: None, + } +} + /// Status report recovered from the DB after the in-memory result was evicted. /// Carries status only — the full output lives in the child session. fn db_report(task_id: &str, rec: &ChildStatusRecord) -> DelegationTaskReport { @@ -1150,6 +1166,41 @@ fn classify_locked(inner: &PendingInner, parent_connection_id: &str, task_id: &s } } +/// The durable parent/conversation check has already authorized this id, so a +/// reconnect may read the live cache even though the owning connection id has +/// changed. The ledger remains the security boundary. +fn classify_authorized_locked( + inner: &PendingInner, + task_id: &str, + ledger_report: &DelegationTaskReport, +) -> StatusClass { + if let Some(c) = inner.completed.get(task_id) { + return StatusClass::Settled(completed_report(task_id, c)); + } + match inner.running.get(task_id) { + Some(r) => StatusClass::Running { + report: running_report(task_id, r), + child_connection_id: r.child_connection_id.clone(), + }, + None => match inner.setups.get(task_id) { + Some(child_connection_id) => StatusClass::Running { + report: ledger_report.clone(), + child_connection_id: child_connection_id.clone(), + }, + None => StatusClass::NotInMemory, + }, + } +} + +fn same_effective_working_dir(requested: Option<&str>, stored: &str) -> bool { + requested.is_none_or(|dir| { + std::fs::canonicalize(dir) + .map(|path| path.to_string_lossy().to_string()) + .unwrap_or_else(|_| dir.to_string()) + == stored + }) +} + /// Map a terminal [`DelegationTaskReport`] back to a [`DelegationOutcome`] for /// the test-only `handle_request` shim (so pre-async tests keep asserting on /// the old outcome shape). @@ -1287,6 +1338,7 @@ pub struct DelegationMatchKey { pub agent_type: AgentType, pub task: String, pub working_dir: Option, + pub continue_from_task_id: Option, } /// One captured parent-side `delegate_to_agent` tool_call awaiting its @@ -1418,6 +1470,11 @@ pub struct DelegationBroker { /// no-op ("no hint"); production wires `ConnectionManagerLiveReplyLookup` via /// `with_live_reply_lookup`. live_reply_lookup: Arc, + /// Installed in production to make admission/results survive broker cache + /// eviction and process restarts. Tests which do not exercise persistence + /// keep the legacy in-memory path. + ledger_db: Option>, + release_state: Arc>, pending: Arc, tool_calls: Arc, pre_canceled_handles: Arc, @@ -1431,7 +1488,109 @@ pub struct DelegationBroker { block_resurface: Duration, } +#[derive(Default)] +struct ReleaseState { + parents: HashMap, + released: HashSet, + admitted: HashSet, +} + impl DelegationBroker { + /// Project one durable ledger row through the broker's live task tables. + /// + /// A `running` row can mean either a task this broker still owns or a task + /// whose process disappeared before a terminal snapshot reached SQLite. + /// Historical readers must make the same distinction as status queries: + /// preserve genuinely active work, prefer a terminal in-memory result if + /// it won a narrow persistence race, and describe an orphan as interrupted + /// without rewriting the durable row. + pub(crate) async fn project_ledger_report( + &self, + entry: &crate::db::service::delegation_task_service::TaskLedgerEntry, + ) -> DelegationTaskReport { + if entry.status != TaskStatus::Running { + return entry.report.clone(); + } + let inner = self.pending.inner.lock().await; + if let Some(completed) = inner.completed.get(&entry.task_id) { + return completed_report(&entry.task_id, completed); + } + if let Some(running) = inner.running.get(&entry.task_id) { + return running_report(&entry.task_id, running); + } + if inner.setups.contains_key(&entry.task_id) { + return entry.report.clone(); + } + interrupted_ledger_report(entry) + } + + /// Called by the connection's driver/reap barrier. Persistence is wired by + /// the continuation ledger; keeping the notification on the broker avoids + /// making the ACP process layer depend on database services. + pub(crate) async fn connection_released(&self, task_id: &str) { + let parent = { + let mut state = self.release_state.lock().await; + let Some(parent) = state.parents.get(task_id).copied() else { + return; + }; + state.released.insert(task_id.to_string()); + if state.admitted.contains(task_id) { + Some(parent) + } else { + None + } + }; + if let (Some(db), Some(parent)) = (self.ledger_db.as_ref(), parent) { + if let Err(error) = crate::db::service::delegation_task_service::mark_released( + &db.conn, parent, task_id, + ) + .await + { + tracing::error!(task_id, %error, "[delegation] failed to persist process release"); + } else { + let mut state = self.release_state.lock().await; + state.parents.remove(task_id); + state.released.remove(task_id); + state.admitted.remove(task_id); + } + } + } + + async fn register_release_slot(&self, task_id: &str, parent_conversation_id: i32) { + self.release_state + .lock() + .await + .parents + .insert(task_id.to_string(), parent_conversation_id); + } + + async fn abandon_release_slot(&self, task_id: &str) { + let mut state = self.release_state.lock().await; + state.parents.remove(task_id); + state.released.remove(task_id); + state.admitted.remove(task_id); + } + + async fn mark_admission_ready(&self, task_id: &str) { + let parent = { + let mut state = self.release_state.lock().await; + state.admitted.insert(task_id.to_string()); + state + .released + .contains(task_id) + .then(|| state.parents.get(task_id).copied()) + .flatten() + }; + if let (Some(db), Some(parent)) = (self.ledger_db.as_ref(), parent) { + if crate::db::service::delegation_task_service::mark_released(&db.conn, parent, task_id) + .await + .is_ok() + { + self.abandon_release_slot(task_id).await; + } + } + } + pub fn new( spawner: Arc, depth_lookup: Arc, @@ -1479,6 +1638,8 @@ impl DelegationBroker { event_emitter, status_lookup: Arc::new(NoopChildStatusLookup), live_reply_lookup: Arc::new(NoopChildLiveReplyLookup), + ledger_db: None, + release_state: Arc::new(Mutex::new(ReleaseState::default())), pending: Arc::new(PendingCalls::default()), tool_calls: Arc::new(ToolCallTracker::default()), pre_canceled_handles: Arc::new(PreCanceledHandles::default()), @@ -1509,6 +1670,11 @@ impl DelegationBroker { self } + pub fn with_ledger(mut self, db: Arc) -> Self { + self.ledger_db = Some(db); + self + } + /// Shrink [`BLOCK_RESURFACE_INTERVAL`] so a test can observe the /// delivery-failure valve without waiting minutes. Test-only by /// construction — production always keeps the constant. @@ -2333,6 +2499,7 @@ impl DelegationBroker { agent_type: req.agent_type, task: req.task.clone(), working_dir: req.requested_working_dir.clone(), + continue_from_task_id: req.continue_from_task_id.clone(), }; let _ = self .take_matching_tool_call(&req.parent_connection_id, &key) @@ -2365,6 +2532,7 @@ impl DelegationBroker { agent_type: req.agent_type, task: req.task.clone(), working_dir: req.requested_working_dir.clone(), + continue_from_task_id: req.continue_from_task_id.clone(), }; let claimed = self .claim_pending_tool_call_with_brief_wait(&req.parent_connection_id, &match_key) @@ -2393,6 +2561,12 @@ impl DelegationBroker { if let Some(dir) = req.requested_working_dir.as_deref() { raw_input.insert("working_dir".into(), serde_json::Value::String(dir.into())); } + if let Some(source) = req.continue_from_task_id.as_deref() { + raw_input.insert( + "continue_from_task_id".into(), + serde_json::Value::String(source.into()), + ); + } self.meta_writer .write_tool_call_identity( &req.parent_connection_id, @@ -2463,11 +2637,107 @@ impl DelegationBroker { // Pull per-agent overrides from the broker config (defaults to empty). // Cloning is cheap — `AgentDelegationDefaults` is at most one Option // and a small BTreeMap, and the spawner consumes both fields by value. - let (preferred_mode_id, preferred_config_values) = cfg + let (mut preferred_mode_id, mut preferred_config_values) = cfg .agent_defaults .get(&req.agent_type) .map(|d: &AgentDelegationDefaults| (d.mode_id.clone(), d.config_values.clone())) .unwrap_or((None, BTreeMap::new())); + let call_id = uuid::Uuid::new_v4().to_string(); + let resume_binding = if let Some(source_task_id) = req.continue_from_task_id.as_deref() { + let Some(db) = self.ledger_db.as_ref() else { + self.drop_inflight(inflight_id).await; + return report_err( + req.agent_type, + DelegationError::ContinuationInvalid( + "durable continuation storage is unavailable".into(), + ), + None, + ); + }; + use crate::db::service::delegation_task_service as ledger; + match ledger::successor(&db.conn, req.parent_conversation_id, source_task_id).await { + Ok(Some(existing)) => { + self.drop_inflight(inflight_id).await; + let same = existing.task == req.task + && existing.resume_binding.agent_type == req.agent_type + && same_effective_working_dir( + req.working_dir.as_deref(), + &existing.resume_binding.working_dir, + ); + if same { + return self.project_ledger_report(&existing).await; + } + return report_err( + req.agent_type, + DelegationError::ContinuationConflict(format!( + "source {source_task_id} already continues as {}; use that task id", + existing.task_id + )), + None, + ); + } + Ok(None) => {} + Err(error) => { + self.drop_inflight(inflight_id).await; + return report_err( + req.agent_type, + DelegationError::ContinuationInvalid(error.to_string()), + None, + ); + } + } + let source = + match ledger::lookup(&db.conn, req.parent_conversation_id, source_task_id).await { + Ok(Some(source)) => source, + Ok(None) => { + self.drop_inflight(inflight_id).await; + return report_err( + req.agent_type, + DelegationError::ContinuationInvalid(format!( + "source task {source_task_id} was not found for this parent" + )), + None, + ); + } + Err(error) => { + self.drop_inflight(inflight_id).await; + return report_err( + req.agent_type, + DelegationError::ContinuationInvalid(error.to_string()), + None, + ); + } + }; + if source.status == TaskStatus::Running || !source.released { + self.drop_inflight(inflight_id).await; + return report_err( + req.agent_type, + DelegationError::ContinuationBusy(format!( + "source task {source_task_id} has not finished process cleanup" + )), + None, + ); + } + let binding = source.resume_binding; + if binding.agent_type != req.agent_type + || !same_effective_working_dir(req.working_dir.as_deref(), &binding.working_dir) + { + self.drop_inflight(inflight_id).await; + return report_err( + req.agent_type, + DelegationError::ContinuationConflict( + "agent or working directory differs from the source task".into(), + ), + None, + ); + } + req.working_dir = Some(binding.working_dir.clone()); + preferred_mode_id = binding.preferred_mode_id.clone(); + preferred_config_values = binding.preferred_config_values.clone(); + Some(binding) + } else { + None + }; // Checkpoint #1 (opportunistic): if a parent cancel already landed // during the claim/depth phase, bail before spawning a child the parent // has abandoned. No child exists yet, so there's nothing to tear down. @@ -2480,19 +2750,33 @@ impl DelegationBroker { None, ); } + self.register_release_slot(&call_id, req.parent_conversation_id) + .await; let child_connection_id = match self .spawner - .spawn( + .spawn_for_delegation( &req.parent_connection_id, req.agent_type, req.working_dir.clone(), - preferred_mode_id, - preferred_config_values, + preferred_mode_id.clone(), + preferred_config_values.clone(), + call_id.clone(), + resume_binding.clone(), ) .await { Ok(id) => id, + Err(crate::acp::delegation::spawner::SpawnerError::Busy(message)) => { + self.abandon_release_slot(&call_id).await; + self.drop_inflight(inflight_id).await; + return report_err( + req.agent_type, + DelegationError::ContinuationBusy(message), + None, + ); + } Err(e) => { + self.abandon_release_slot(&call_id).await; self.drop_inflight(inflight_id).await; return report_err( req.agent_type, @@ -2508,6 +2792,7 @@ impl DelegationBroker { // the primary guard for the spawn window, which can block while the // agent process starts up. if self.take_inflight_cancel(inflight_id).await { + self.abandon_release_slot(&call_id).await; let _ = self.spawner.disconnect(&child_connection_id).await; return report_err( req.agent_type, @@ -2519,11 +2804,10 @@ impl DelegationBroker { } // --- Send linked prompt ------------------------------------------------ - let call_id = uuid::Uuid::new_v4().to_string(); // Bounded task label used by the started event and every meta write — // the frontend card's fallback when the parent tool call's `raw_input` // never carried the arguments (Cursor's identity-less announcements). - let task_preview = truncate_on_char_boundary(&req.task, TASK_PREVIEW_CAP); + let task_preview = super::task_preview(&req.task); // Now that the child connection and task id exist, fill the span's empty // fields so every subsequent log line in this delegation carries the // parent→child linkage (see the `delegation_task` span on this fn). @@ -2533,6 +2817,14 @@ impl DelegationBroker { parent_conversation_id: req.parent_conversation_id, parent_tool_use_id: req.parent_tool_use_id.clone(), delegation_call_id: call_id.clone(), + admission: self.ledger_db.as_ref().map(|_| DelegationAdmission { + source_task_id: req.continue_from_task_id.clone(), + task: req.task.clone(), + requested_working_dir: req.requested_working_dir.clone(), + preferred_mode_id, + preferred_config_values, + resume_binding, + }), }; // Reserve this delegation (both ids) BEFORE sending its first prompt. @@ -2560,7 +2852,43 @@ impl DelegationBroker { .send_prompt_linked_for_delegation(&child_connection_id, req.task.clone(), link) .await { - Ok(cid) => cid, + Ok(DelegationDispatch::Started(cid)) => cid, + Ok(DelegationDispatch::Existing(report)) => { + let mut inner = self.pending.inner.lock().await; + inner.unreserve(&call_id, &child_connection_id); + inner.deregister_inflight(inflight_id); + drop(inner); + let _ = self.spawner.disconnect(&child_connection_id).await; + self.abandon_release_slot(&call_id).await; + return report; + } + Ok(DelegationDispatch::Conflict { + next_task_id, + reason, + }) => { + let mut inner = self.pending.inner.lock().await; + inner.unreserve(&call_id, &child_connection_id); + inner.deregister_inflight(inflight_id); + drop(inner); + let _ = self.spawner.disconnect(&child_connection_id).await; + self.abandon_release_slot(&call_id).await; + return report_err( + req.agent_type, + DelegationError::ContinuationConflict(format!( + "{reason}; existing successor is {next_task_id}" + )), + None, + ); + } + Ok(DelegationDispatch::Failed(report)) => { + let mut inner = self.pending.inner.lock().await; + inner.unreserve(&call_id, &child_connection_id); + inner.deregister_inflight(inflight_id); + drop(inner); + self.mark_admission_ready(&call_id).await; + let _ = self.spawner.disconnect(&child_connection_id).await; + return report; + } Err(e) => { // Setup failed before parking — release the reservation (and // discard any terminal that buffered against this delegation in @@ -2572,6 +2900,7 @@ impl DelegationBroker { inner.deregister_inflight(inflight_id); } let _ = self.spawner.disconnect(&child_connection_id).await; + self.abandon_release_slot(&call_id).await; return report_err( req.agent_type, DelegationError::SpawnFailed(e.to_string()), @@ -2579,6 +2908,7 @@ impl DelegationBroker { ); } }; + self.mark_admission_ready(&call_id).await; // The child is now running. Stamp the start so terminal paths can // report a real `duration_ms`. @@ -2735,6 +3065,7 @@ impl DelegationBroker { child_connection_id: child_connection_id.clone(), child_conversation_id, parent_connection_id: req.parent_connection_id.clone(), + parent_conversation_id: req.parent_conversation_id, parent_tool_use_id: req.parent_tool_use_id.clone(), agent_type: req.agent_type, task_preview: task_preview.clone(), @@ -2758,6 +3089,7 @@ impl DelegationBroker { Disposition::ChildTerminal(outcome) => { self.finalize_delegation( &req.parent_connection_id, + req.parent_conversation_id, &req.parent_tool_use_id, &child_connection_id, child_conversation_id, @@ -2781,6 +3113,15 @@ impl DelegationBroker { // ourselves (cancel + disconnect, since a turn is in flight) and // return a canceled report. The canceled result was recorded above. Disposition::ParentCanceled => { + self.freeze_ledger_outcome( + req.parent_conversation_id, + &call_id, + req.agent_type, + child_conversation_id, + setup_duration_ms, + &canceled_outcome(child_conversation_id, "parent canceled"), + ) + .await; self.write_meta_if_real( &req.parent_connection_id, &req.parent_tool_use_id, @@ -2855,6 +3196,15 @@ impl DelegationBroker { } }; if let Some(duration_ms) = canceled_duration_ms { + self.freeze_ledger_outcome( + req.parent_conversation_id, + &call_id, + req.agent_type, + child_conversation_id, + duration_ms, + &canceled_outcome(child_conversation_id, "canceled before await"), + ) + .await; self.write_meta_if_real( &req.parent_connection_id, &req.parent_tool_use_id, @@ -2947,6 +3297,7 @@ impl DelegationBroker { if let Some((task, duration_ms)) = task { self.finalize_delegation( &task.parent_connection_id, + task.parent_conversation_id, &task.parent_tool_use_id, &task.child_connection_id, task.child_conversation_id, @@ -2976,6 +3327,7 @@ impl DelegationBroker { async fn finalize_delegation( &self, parent_connection_id: &str, + parent_conversation_id: i32, parent_tool_use_id: &str, child_connection_id: &str, child_conversation_id: i32, @@ -2985,6 +3337,15 @@ impl DelegationBroker { task_preview: &str, task_id: &str, ) { + self.freeze_ledger_outcome( + parent_conversation_id, + task_id, + agent_type, + child_conversation_id, + duration_ms, + outcome, + ) + .await; let meta = match outcome { DelegationOutcome::Ok(ok) => build_delegation_meta( "completed", @@ -3022,6 +3383,36 @@ impl DelegationBroker { let _ = self.spawner.disconnect(child_connection_id).await; } + async fn freeze_ledger_outcome( + &self, + parent_conversation_id: i32, + task_id: &str, + agent_type: AgentType, + _child_conversation_id: i32, + duration_ms: u64, + outcome: &DelegationOutcome, + ) { + let Some(db) = self.ledger_db.as_ref() else { + return; + }; + let report = report_from_outcome( + Some(task_id.to_string()), + Some(agent_type), + outcome, + Some(duration_ms), + ); + if let Err(error) = crate::db::service::delegation_task_service::finish( + &db.conn, + parent_conversation_id, + task_id, + &report, + ) + .await + { + tracing::error!(task_id, %error, "[delegation] failed to freeze terminal report"); + } + } + /// Internal helper — apply the meta write iff the parent's /// `tool_use_id` refers to a real ACP `tool_call_id`. The /// broker-synthesized `"delegation-"` placeholder targets no @@ -3323,6 +3714,15 @@ impl DelegationBroker { duration_ms: u64, cancel_turn: bool, ) { + self.freeze_ledger_outcome( + task.parent_conversation_id, + &task.task_id, + task.agent_type, + task.child_conversation_id, + duration_ms, + &canceled_outcome(task.child_conversation_id, "delegation canceled"), + ) + .await; self.write_meta_if_real( &task.parent_connection_id, &task.parent_tool_use_id, @@ -3436,13 +3836,49 @@ impl DelegationBroker { // One lock acquisition classifies every requested id. The async // resolution of running (live reply) / not-in-memory (DB) ids is // deferred to `assemble_reports`, OUTSIDE this lock. - let classes: Vec = { - let inner = self.pending.inner.lock().await; - task_ids - .iter() - .map(|id| classify_locked(&inner, parent_connection_id, id)) - .collect() + let ledger_scopes = if let (Some(db), Some(parent)) = + (self.ledger_db.as_ref(), parent_conversation_id) + { + let mut scopes = Vec::with_capacity(task_ids.len()); + for id in task_ids { + let scope = crate::db::service::delegation_task_service::lookup_scoped( + &db.conn, parent, id, + ) + .await + .unwrap_or(crate::db::service::delegation_task_service::ScopedLookup::Hidden); + scopes.push(scope); + } + Some(scopes) + } else { + None }; + let classes: Vec = + { + let inner = self.pending.inner.lock().await; + task_ids + .iter() + .enumerate() + .map( + |(index, id)| { + match ledger_scopes.as_ref().map(|v| &v[index]) { + Some(crate::db::service::delegation_task_service::ScopedLookup::Visible( + entry, + )) if entry.status != TaskStatus::Running => { + StatusClass::Settled(entry.report.clone()) + } + Some(crate::db::service::delegation_task_service::ScopedLookup::Visible( + entry, + )) => classify_authorized_locked(&inner, id, &entry.report), + Some(crate::db::service::delegation_task_service::ScopedLookup::Hidden) => { + StatusClass::Settled(unknown_report(id)) + } + Some(crate::db::service::delegation_task_service::ScopedLookup::Absent) + | None => classify_locked(&inner, parent_connection_id, id), + } + }, + ) + .collect() + }; let running_count = classes .iter() .filter(|c| matches!(c, StatusClass::Running { .. })) @@ -3677,16 +4113,40 @@ impl DelegationBroker { parent_conversation_id: Option, task_id: &str, ) -> DelegationTaskReport { + let durable_authorized = if let (Some(db), Some(parent)) = + (self.ledger_db.as_ref(), parent_conversation_id) + { + match crate::db::service::delegation_task_service::lookup_scoped( + &db.conn, parent, task_id, + ) + .await + { + Ok(crate::db::service::delegation_task_service::ScopedLookup::Visible(entry)) => { + if entry.status != TaskStatus::Running { + return entry.report; + } + true + } + Ok(crate::db::service::delegation_task_service::ScopedLookup::Hidden) | Err(_) => { + return unknown_report(task_id); + } + Ok(crate::db::service::delegation_task_service::ScopedLookup::Absent) => false, + } + } else { + false + }; let drained = { let mut inner = self.pending.inner.lock().await; if let Some(c) = inner.completed.get(task_id) { - if c.parent_connection_id == parent_connection_id { + if durable_authorized || c.parent_connection_id == parent_connection_id { return completed_report(task_id, c); } return unknown_report(task_id); } match inner.running.get(task_id) { - Some(r) if r.parent_connection_id == parent_connection_id => { + Some(r) + if durable_authorized || r.parent_connection_id == parent_connection_id => + { drain_and_record_canceled( &mut inner, vec![task_id.to_string()], @@ -3724,6 +4184,21 @@ impl DelegationBroker { parent_conversation_id: Option, task_id: &str, ) -> DelegationTaskReport { + if let (Some(db), Some(parent)) = (self.ledger_db.as_ref(), parent_conversation_id) { + match crate::db::service::delegation_task_service::lookup_scoped( + &db.conn, parent, task_id, + ) + .await + { + Ok(crate::db::service::delegation_task_service::ScopedLookup::Visible(entry)) => { + return self.project_ledger_report(&entry).await; + } + Ok(crate::db::service::delegation_task_service::ScopedLookup::Hidden) | Err(_) => { + return unknown_report(task_id); + } + Ok(crate::db::service::delegation_task_service::ScopedLookup::Absent) => {} + } + } match self.status_lookup.find_by_call_id(task_id).await { Some(rec) if parent_conversation_id.is_some() && rec.parent_id == parent_conversation_id => @@ -3820,6 +4295,37 @@ impl DelegationBroker { ); } + // Tasks admitted by the durable continuation protocol must advance to + // a new task id. Keep the legacy same-id resume path only for rows that + // predate the ledger. + if let Some(db) = self.ledger_db.as_ref() { + use crate::db::service::delegation_task_service::ScopedLookup; + match crate::db::service::delegation_task_service::lookup_scoped( + &db.conn, + req.parent_conversation_id, + &req.task_id, + ) + .await + { + Ok(ScopedLookup::Visible(entry)) => { + self.drop_inflight(inflight_id).await; + return not_resumable_report( + &req.task_id, + entry.status, + Some(entry.child_conversation_id), + Some(entry.resume_binding.agent_type), + "this task uses durable continuation. Start the next round with \ + delegate_to_agent and continue_from_task_id set to this task id.", + ); + } + Ok(ScopedLookup::Absent) => {} + Ok(ScopedLookup::Hidden) | Err(_) => { + self.drop_inflight(inflight_id).await; + return unknown_report(&req.task_id); + } + } + } + // --- In-memory gate --------------------------------------------------- // One lock pass over the live maps. Only a CANCELED, caller-owned cached // entry may proceed (its removal is deferred to the registration lock so @@ -4053,6 +4559,7 @@ impl DelegationBroker { .spawner .spawn_for_resume( &req.parent_connection_id, + &req.task_id, ctx.agent_type, Some(working_dir), &external_id, @@ -4101,7 +4608,7 @@ impl DelegationBroker { let task_preview = ctx .title .as_deref() - .map(|t| truncate_on_char_boundary(t, TASK_PREVIEW_CAP)) + .map(super::task_preview) .unwrap_or_else(|| "(resumed delegation)".to_string()); // The resumed run reuses the ORIGINAL parent-side tool_use_id persisted // on the row so the parent's original delegation card re-binds; a row @@ -4132,6 +4639,12 @@ impl DelegationBroker { build_resume_prompt(req.reason.as_deref()), ctx.folder_id, ctx.child_conversation_id, + DelegationLink { + parent_conversation_id: req.parent_conversation_id, + parent_tool_use_id: parent_tool_use_id.clone(), + delegation_call_id: call_id.clone(), + admission: None, + }, ) .await { @@ -4275,6 +4788,7 @@ impl DelegationBroker { child_connection_id: child_connection_id.clone(), child_conversation_id: ctx.child_conversation_id, parent_connection_id: req.parent_connection_id.clone(), + parent_conversation_id: req.parent_conversation_id, parent_tool_use_id: parent_tool_use_id.clone(), agent_type: ctx.agent_type, task_preview: task_preview.clone(), @@ -4294,6 +4808,7 @@ impl DelegationBroker { Disposition::ChildTerminal(outcome) => { self.finalize_delegation( &req.parent_connection_id, + req.parent_conversation_id, &parent_tool_use_id, &child_connection_id, ctx.child_conversation_id, @@ -4313,6 +4828,15 @@ impl DelegationBroker { ); } Disposition::ParentCanceled => { + self.freeze_ledger_outcome( + req.parent_conversation_id, + &call_id, + ctx.agent_type, + ctx.child_conversation_id, + setup_duration_ms, + &canceled_outcome(ctx.child_conversation_id, "parent canceled"), + ) + .await; self.write_meta_if_real( &req.parent_connection_id, &parent_tool_use_id, @@ -4360,10 +4884,8 @@ impl DelegationBroker { let canceled_duration_ms = { let mut inner = self.pending.inner.lock().await; if inner.running.remove(&call_id).is_some() { - let outcome = canceled_outcome( - ctx.child_conversation_id, - "canceled before await", - ); + let outcome = + canceled_outcome(ctx.child_conversation_id, "canceled before await"); let duration_ms = started_at.elapsed().as_millis() as u64; inner.insert_completed( &call_id, @@ -4381,6 +4903,15 @@ impl DelegationBroker { } }; if let Some(duration_ms) = canceled_duration_ms { + self.freeze_ledger_outcome( + req.parent_conversation_id, + &call_id, + ctx.agent_type, + ctx.child_conversation_id, + duration_ms, + &canceled_outcome(ctx.child_conversation_id, "canceled before await"), + ) + .await; self.write_meta_if_real( &req.parent_connection_id, &parent_tool_use_id, @@ -4656,6 +5187,7 @@ mod tests { task: "do x".into(), working_dir: None, requested_working_dir: None, + continue_from_task_id: None, external_handle: None, } } @@ -5140,6 +5672,212 @@ mod tests { assert_eq!(batch[0].task_id, single.task_id); } + #[tokio::test] + async fn ledger_authorizes_cache_and_marks_orphaned_running_as_interrupted() { + use crate::db::service::{ + conversation_service, delegation_task_service as ledger, folder_service, + }; + use std::collections::BTreeMap; + + let db = Arc::new(crate::db::test_helpers::fresh_in_memory_db().await); + let folder = folder_service::add_folder(&db.conn, "/tmp/broker-ledger") + .await + .unwrap(); + let parent = + conversation_service::create(&db.conn, folder.id, AgentType::ClaudeCode, None, None) + .await + .unwrap(); + let child = conversation_service::create(&db.conn, folder.id, AgentType::Codex, None, None) + .await + .unwrap(); + let binding = ledger::ResumeBinding { + agent_type: AgentType::Codex, + external_session_id: "session-1".into(), + child_conversation_id: child.id, + working_dir: "/tmp/broker-ledger".into(), + preferred_mode_id: None, + preferred_config_values: BTreeMap::new(), + config_fingerprint: "cfg".into(), + }; + ledger::admit( + &db.conn, + ledger::AdmissionInput { + task_id: "durable-running".into(), + parent_conversation_id: parent.id, + child_conversation_id: child.id, + source_task_id: None, + task: "work".into(), + requested_working_dir: None, + resume_binding: binding.clone(), + }, + ) + .await + .unwrap(); + let broker = DelegationBroker::new( + Arc::new(MockSpawner::new()) as Arc, + shallow_lookup(), + ) + .with_ledger(db.clone()); + + let interrupted = broker + .get_task_status( + "reconnected-parent", + Some(parent.id), + "durable-running", + StatusWait::Immediate, + ) + .await; + assert_eq!(interrupted.task_id.as_deref(), Some("durable-running")); + assert_eq!(interrupted.status, TaskStatus::Unknown); + assert_eq!(interrupted.error_code.as_deref(), Some("interrupted")); + let running_entry = ledger::lookup(&db.conn, parent.id, "durable-running") + .await + .unwrap() + .unwrap(); + let history = broker.project_ledger_report(&running_entry).await; + assert_eq!(history.status, TaskStatus::Unknown); + assert_eq!(history.error_code.as_deref(), Some("interrupted")); + + let terminal = DelegationTaskReport { + task_id: Some("durable-running".into()), + status: TaskStatus::Completed, + child_conversation_id: Some(child.id), + agent_type: Some(AgentType::Codex), + text: Some("durable result".into()), + error_code: None, + message: None, + duration_ms: Some(1), + blocked_on: None, + }; + ledger::finish(&db.conn, parent.id, "durable-running", &terminal) + .await + .unwrap(); + let restored = broker + .get_task_status( + "new-parent-connection", + Some(parent.id), + "durable-running", + StatusWait::Immediate, + ) + .await; + assert_eq!(restored.text.as_deref(), Some("durable result")); + + let foreign = + conversation_service::create(&db.conn, folder.id, AgentType::ClaudeCode, None, None) + .await + .unwrap(); + assert_eq!( + broker + .get_task_status( + "new-parent-connection", + Some(foreign.id), + "durable-running", + StatusWait::Immediate, + ) + .await + .status, + TaskStatus::Unknown + ); + conversation_service::soft_delete(&db.conn, child.id) + .await + .unwrap(); + assert_eq!( + broker + .get_task_status( + "new-parent-connection", + Some(parent.id), + "durable-running", + StatusWait::Immediate, + ) + .await + .status, + TaskStatus::Unknown + ); + } + + #[tokio::test] + async fn ledger_authorizes_cancel_after_parent_reconnect_and_hides_foreign_parent() { + use crate::db::service::{ + conversation_service, delegation_task_service as ledger, folder_service, + }; + + let db = Arc::new(crate::db::test_helpers::fresh_in_memory_db().await); + let folder = folder_service::add_folder(&db.conn, "/tmp/broker-ledger-cancel") + .await + .unwrap(); + let parent = + conversation_service::create(&db.conn, folder.id, AgentType::ClaudeCode, None, None) + .await + .unwrap(); + let foreign = + conversation_service::create(&db.conn, folder.id, AgentType::ClaudeCode, None, None) + .await + .unwrap(); + let child = conversation_service::create(&db.conn, folder.id, AgentType::Codex, None, None) + .await + .unwrap(); + let mock = Arc::new(MockSpawner::new()); + let broker = DelegationBroker::new( + mock.clone() as Arc, + shallow_lookup(), + ) + .with_ledger(db.clone()); + enable_delegation(&broker).await; + let task_id = start_running(&broker, &mock, "child-ledger-cancel", child.id, "tu").await; + ledger::admit( + &db.conn, + ledger::AdmissionInput { + task_id: task_id.clone(), + parent_conversation_id: parent.id, + child_conversation_id: child.id, + source_task_id: None, + task: "work".into(), + requested_working_dir: None, + resume_binding: ledger::ResumeBinding { + agent_type: AgentType::Codex, + external_session_id: "session-cancel".into(), + child_conversation_id: child.id, + working_dir: "/tmp/broker-ledger-cancel".into(), + preferred_mode_id: None, + preferred_config_values: BTreeMap::new(), + config_fingerprint: "cfg".into(), + }, + }, + ) + .await + .unwrap(); + + let running_entry = ledger::lookup(&db.conn, parent.id, &task_id) + .await + .unwrap() + .unwrap(); + let history = broker.project_ledger_report(&running_entry).await; + assert_eq!( + history.status, + TaskStatus::Running, + "history must not mislabel a task still owned by this broker" + ); + assert_eq!(history.error_code, None); + + assert_eq!( + broker + .cancel_task_by_id("guessed-connection", Some(foreign.id), &task_id) + .await + .status, + TaskStatus::Unknown + ); + assert!(mock.cancels.lock().await.is_empty()); + + let report = broker + .cancel_task_by_id("reconnected-parent", Some(parent.id), &task_id) + .await; + assert_eq!(report.status, TaskStatus::Canceled); + assert_eq!( + mock.cancels.lock().await.as_slice(), + &["child-ledger-cancel".to_string()] + ); + } + /// An immediate batch poll resolves a mix of completed / running / unknown /// tasks in ONE pass, preserving request order. #[tokio::test] @@ -5336,6 +6074,32 @@ mod tests { } } + #[tokio::test] + async fn admission_race_preserves_continuation_conflict() { + let mock = Arc::new(MockSpawner::new()); + mock.queue_spawn(Ok("losing-child".into())).await; + mock.queue_dispatch(DelegationDispatch::Conflict { + next_task_id: "winning-task".into(), + reason: "source already has a successor".into(), + }) + .await; + let broker = + DelegationBroker::new(mock.clone() as Arc, shallow_lookup()); + enable_delegation(&broker).await; + + let report = broker.start_delegation(request(1, "pt-race")).await; + + assert_eq!(report.error_code.as_deref(), Some("continuation_conflict")); + assert!(report.message.as_deref().unwrap().contains("winning-task")); + assert_eq!( + mock.disconnects.lock().await.as_slice(), + &["losing-child".to_string()] + ); + assert_eq!(broker.inflight_count().await, 0); + assert_eq!(broker.reserved_call_count().await, 0); + assert_eq!(broker.pending_count().await, 0); + } + #[tokio::test] async fn agent_defaults_are_forwarded_to_spawner() { // Configure broker with per-agent defaults for ClaudeCode and verify @@ -5884,6 +6648,7 @@ mod tests { agent_type, task: task.to_string(), working_dir: None, + continue_from_task_id: None, } } @@ -5892,6 +6657,7 @@ mod tests { agent_type: AgentType::Codex, task: task.to_string(), working_dir: Some(working_dir.to_string()), + continue_from_task_id: None, } } @@ -5934,6 +6700,40 @@ mod tests { .is_none()); } + #[tokio::test] + async fn parallel_continuations_bind_by_source_regardless_of_order() { + let broker = DelegationBroker::new( + Arc::new(MockSpawner::new()) as Arc, + shallow_lookup(), + ); + let key = |source: &str| DelegationMatchKey { + agent_type: AgentType::Codex, + task: "same follow-up".into(), + working_dir: None, + continue_from_task_id: Some(source.into()), + }; + broker + .register_pending_tool_call_with_key("p1", "tc-A".into(), Some(key("source-A"))) + .await; + broker + .register_pending_tool_call_with_key("p1", "tc-B".into(), Some(key("source-B"))) + .await; + assert_eq!( + broker + .take_matching_tool_call("p1", &key("source-B")) + .await + .as_deref(), + Some("tc-B") + ); + assert_eq!( + broker + .take_matching_tool_call("p1", &key("source-A")) + .await + .as_deref(), + Some("tc-A") + ); + } + #[tokio::test] async fn parallel_same_task_different_agent_do_not_swap() { // Regression for Codex review: two parallel calls with the SAME task @@ -7196,6 +7996,7 @@ mod tests { agent_type: AgentType::ClaudeCode, task: "do x".into(), working_dir: None, + continue_from_task_id: None, }), ) .await; @@ -9175,6 +9976,7 @@ mod tests { agent_type: AgentType::ClaudeCode, task: "do x".into(), working_dir: None, + continue_from_task_id: None, }; // The lifecycle registered the keyed tool_call for this delegation. broker @@ -9437,11 +10239,9 @@ mod tests { let mock = Arc::new(MockSpawner::new()); let lookup = Arc::new(MockResumeLookup::default()); *lookup.ctx.lock().await = ctx; - let broker = DelegationBroker::new( - mock.clone() as Arc, - shallow_lookup(), - ) - .with_status_lookup(lookup.clone() as Arc); + let broker = + DelegationBroker::new(mock.clone() as Arc, shallow_lookup()) + .with_status_lookup(lookup.clone() as Arc); enable_delegation(&broker).await; (mock, lookup, broker) } @@ -9484,6 +10284,7 @@ mod tests { // The spawn resumed the recorded agent session in the recorded dir. let spawns = mock.resume_spawn_args.lock().await; assert_eq!(spawns.len(), 1); + assert_eq!(spawns[0].task_id, "task-1"); assert_eq!(spawns[0].external_session_id, "ext-session-1"); assert_eq!(spawns[0].working_dir.as_deref(), Some("/work")); drop(spawns); @@ -9493,6 +10294,8 @@ mod tests { assert_eq!(sends.len(), 1); assert_eq!(sends[0].child_conversation_id, 42); assert_eq!(sends[0].folder_id, 7); + assert_eq!(sends[0].link.delegation_call_id, "task-1"); + assert_eq!(sends[0].link.parent_conversation_id, 1); assert!(sends[0].prompt.contains("machine rebooted")); drop(sends); diff --git a/src-tauri/src/acp/delegation/listener.rs b/src-tauri/src/acp/delegation/listener.rs index cd60d31d47..e0a4bb4fd7 100644 --- a/src-tauri/src/acp/delegation/listener.rs +++ b/src-tauri/src/acp/delegation/listener.rs @@ -15,20 +15,20 @@ use async_trait::async_trait; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::sync::RwLock; +use crate::acp::chat_authoring::{AuthoringContext, AuthoringOutcome, ChatAuthoringAccess}; use crate::acp::delegation::broker::{DelegationBroker, StatusWait}; use crate::acp::delegation::transport::{ read_frame, write_frame, BrokerAskRequest, BrokerCancelRequest, BrokerCancelTaskRequest, - BrokerCommitFeedbackRequest, BrokerFeedbackRequest, BrokerMessage, BrokerRequest, - BrokerCreateAutomationRequest, BrokerCreateWorkTaskRequest, BrokerResponse, - BrokerResumeTaskRequest, BrokerSessionRequest, BrokerStatusRequest, - BrokerTaskCompleteRequest, BrokerTaskProgressRequest, + BrokerCommitFeedbackRequest, BrokerCreateAutomationRequest, BrokerCreateWorkTaskRequest, + BrokerFeedbackRequest, BrokerMessage, BrokerRequest, BrokerResponse, BrokerResumeTaskRequest, + BrokerSessionRequest, BrokerStatusRequest, BrokerTaskCompleteRequest, + BrokerTaskProgressRequest, }; use crate::acp::delegation::types::{ DelegationRequest, DelegationTaskReport, ResumeDelegationRequest, TaskStatus, }; use crate::acp::feedback::{PendingFeedback, SessionFeedbackAccess}; use crate::acp::question::{QuestionOutcome, SessionQuestionAccess}; -use crate::acp::chat_authoring::{AuthoringContext, AuthoringOutcome, ChatAuthoringAccess}; use crate::acp::session_info::{SessionInfo, SessionInfoAccess}; use crate::acp::work_task_tools::{TaskReportAck, WorkTaskToolAccess}; use crate::models::AgentType; @@ -40,7 +40,6 @@ use serde_json::Value; /// `wait_ms = 0` opts out of the ceiling and blocks until the task is terminal. const STATUS_WAIT_MAX_MS: u64 = 60_000; - /// The bound-but-not-yet-served socket handed from [`DelegationListener::bind`] /// to [`DelegationListener::accept_loop`]. A UDS listener on unix; on Windows, /// the first named-pipe server instance (the loop creates each subsequent one @@ -242,11 +241,7 @@ impl DelegationListener { #[cfg(unix)] fn staging_socket_path(socket_path: &Path) -> PathBuf { let salt = uuid::Uuid::new_v4().simple().to_string(); - socket_path.with_file_name(format!( - ".stg-{}-{}", - std::process::id(), - &salt[..8] - )) + socket_path.with_file_name(format!(".stg-{}-{}", std::process::id(), &salt[..8])) } #[cfg(windows)] @@ -373,10 +368,7 @@ impl DelegationListener { write_frame(conn, &feedback_response(&[])?).await?; } Some(parent_conn_id) => { - let pending = self - .feedback - .read_pending_feedback(&parent_conn_id) - .await; + let pending = self.feedback.read_pending_feedback(&parent_conn_id).await; // Read-only: the response carries the note ids // (`_commit_ids`); delivery is committed LATER, by the // companion's `CommitFeedback` once it actually returns @@ -813,6 +805,19 @@ impl DelegationListener { let working_dir = requested_working_dir .clone() .or_else(|| Some(entry.working_dir.to_string_lossy().to_string())); + let continue_from_task_id = match req.input.get("continue_from_task_id") { + None | Some(serde_json::Value::Null) => None, + Some(serde_json::Value::String(value)) if !value.trim().is_empty() => { + Some(value.trim().to_string()) + } + Some(serde_json::Value::String(_)) => None, + Some(_) => { + return report_failed( + "continuation_invalid", + "continue_from_task_id must be a string or null", + ); + } + }; let delegation_req = DelegationRequest { parent_connection_id: req.parent_connection_id, @@ -822,6 +827,7 @@ impl DelegationListener { task, working_dir, requested_working_dir, + continue_from_task_id, external_handle: req.external_handle, }; self.broker.start_delegation(delegation_req).await @@ -1047,10 +1053,7 @@ mod tests { } #[async_trait] impl SessionFeedbackAccess for StubFeedback { - async fn read_pending_feedback( - &self, - parent_connection_id: &str, - ) -> Vec { + async fn read_pending_feedback(&self, parent_connection_id: &str) -> Vec { *self.read_conn.lock().await = Some(parent_connection_id.to_string()); self.items.lock().await.clone() } @@ -1070,9 +1073,7 @@ mod tests { #[derive(Default)] struct StubQuestion { pending: tokio::sync::Mutex>>, - registered: tokio::sync::Mutex< - Vec<(String, Vec)>, - >, + registered: tokio::sync::Mutex)>>, canceled: tokio::sync::Mutex>, } #[async_trait] @@ -1522,6 +1523,7 @@ mod tests { task: "do x".into(), working_dir: None, requested_working_dir: None, + continue_from_task_id: None, external_handle: None, }) .await; @@ -1675,6 +1677,7 @@ mod tests { task: "do x".into(), working_dir: None, requested_working_dir: None, + continue_from_task_id: None, external_handle: None, }) .await @@ -1777,6 +1780,7 @@ mod tests { task: "do x".into(), working_dir: None, requested_working_dir: None, + continue_from_task_id: None, external_handle: None, }) .await; @@ -1828,6 +1832,7 @@ mod tests { task: "do x".into(), working_dir: None, requested_working_dir: None, + continue_from_task_id: None, external_handle: Some("h-1".into()), }; broker.handle_request(req).await @@ -1900,7 +1905,8 @@ mod tests { } let mock = Arc::new(MockSpawner::new()); - mock.queue_resume_spawn(Ok(ResumedSpawn::fresh("child-conn-2"))).await; + mock.queue_resume_spawn(Ok(ResumedSpawn::fresh("child-conn-2"))) + .await; mock.queue_resume_send(Ok(())).await; let broker = Arc::new( DelegationBroker::new( @@ -2057,6 +2063,57 @@ mod tests { assert_eq!(report.error_code.as_deref(), Some("spawn_failed")); } + #[tokio::test] + async fn continuation_id_validation_rejects_types_and_defaults_empty_values() { + let mock = Arc::new(MockSpawner::new()); + let broker = make_broker(mock.clone()).await; + let tokens = Arc::new(TokenRegistry::default()); + tokens + .register( + "tok".into(), + TokenEntry { + parent_connection_id: "parent-conn".into(), + working_dir: PathBuf::from("/tmp"), + }, + ) + .await; + let listener = make_listener(broker, tokens, Some(1)); + for invalid in [json!(7), json!([])] { + let report = listener + .process( + make_request(json!({ + "agent_type": "codex", + "task": "next", + "continue_from_task_id": invalid, + })) + .await, + ) + .await; + assert_eq!(report.error_code.as_deref(), Some("continuation_invalid")); + } + assert!(mock.spawn_args.lock().await.is_empty()); + + mock.queue_spawn(Err(SpawnerError::Spawn("stop-null".into()))) + .await; + mock.queue_spawn(Err(SpawnerError::Spawn("stop-blank".into()))) + .await; + for value in [serde_json::Value::Null, json!(" ")] { + let report = listener + .process( + make_request(json!({ + "agent_type": "codex", + "task": "fresh", + "continue_from_task_id": value, + })) + .await, + ) + .await; + assert_eq!(report.error_code.as_deref(), Some("spawn_failed")); + } + + assert_eq!(mock.spawn_args.lock().await.len(), 2); + } + // --- check_user_feedback over the listener ----------------------------- use crate::acp::feedback::PendingFeedback; @@ -2149,7 +2206,10 @@ mod tests { let commit_ids = resp.outcome["_commit_ids"].as_array().unwrap(); assert_eq!(commit_ids, &vec!["f1", "f2"]); // Read was scoped to the token's parent connection id. - assert_eq!(feedback.read_conn.lock().await.as_deref(), Some("parent-conn")); + assert_eq!( + feedback.read_conn.lock().await.as_deref(), + Some("parent-conn") + ); // The Feedback arm is READ-ONLY — it does NOT commit (delivery is // committed later, by the companion's CommitFeedback). assert!(feedback.committed.lock().await.is_empty()); @@ -2593,7 +2653,10 @@ mod tests { .await .expect("serve_one must return after peer close"); result.unwrap().unwrap(); - assert_eq!(questions.canceled.lock().await.as_slice(), &["q-1".to_string()]); + assert_eq!( + questions.canceled.lock().await.as_slice(), + &["q-1".to_string()] + ); } /// An invalid token never registers a question and returns a `declined` @@ -2601,7 +2664,8 @@ mod tests { #[tokio::test] async fn ask_invalid_token_declined() { let questions = Arc::new(StubQuestion::default()); - let listener = make_question_listener(Arc::new(TokenRegistry::default()), questions.clone()); + let listener = + make_question_listener(Arc::new(TokenRegistry::default()), questions.clone()); let (mut client, mut server) = duplex(8 * 1024); let server_task = tokio::spawn(async move { listener.serve_one(&mut server).await.unwrap(); @@ -2614,5 +2678,4 @@ mod tests { assert_eq!(resp.outcome["declined"], true); assert!(questions.registered.lock().await.is_empty()); } - } diff --git a/src-tauri/src/acp/delegation/mod.rs b/src-tauri/src/acp/delegation/mod.rs index 36ba18a8bd..7a64f165d6 100644 --- a/src-tauri/src/acp/delegation/mod.rs +++ b/src-tauri/src/acp/delegation/mod.rs @@ -63,3 +63,32 @@ pub const DELEGATE_TOOL_REWRITE_TITLE: &str = "codeg-mcp__delegate_to_agent"; pub const STATUS_TOOL_REWRITE_TITLE: &str = "codeg-mcp__get_delegation_status"; pub const CANCEL_TOOL_REWRITE_TITLE: &str = "codeg-mcp__cancel_delegation"; pub const RESUME_TOOL_REWRITE_TITLE: &str = "codeg-mcp__resume_delegation"; + +/// Byte cap shared by every fallback task label written to delegation-card +/// metadata. The full task remains available in the child session. +pub(crate) const TASK_PREVIEW_CAP: usize = 2 * 1024; + +/// Return a bounded task label without splitting a UTF-8 code point. +pub(crate) fn task_preview(task: &str) -> String { + if task.len() <= TASK_PREVIEW_CAP { + return task.to_string(); + } + const ELLIPSIS: &str = "…"; + let mut end = TASK_PREVIEW_CAP.saturating_sub(ELLIPSIS.len()); + while end > 0 && !task.is_char_boundary(end) { + end -= 1; + } + format!("{}{}", &task[..end], ELLIPSIS) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn task_preview_is_utf8_safe_and_within_cap() { + let preview = task_preview(&"界".repeat(TASK_PREVIEW_CAP)); + assert!(preview.len() <= TASK_PREVIEW_CAP); + assert!(preview.ends_with('…')); + } +} diff --git a/src-tauri/src/acp/delegation/spawner.rs b/src-tauri/src/acp/delegation/spawner.rs index 8c650fbaa8..a7add499a4 100644 --- a/src-tauri/src/acp/delegation/spawner.rs +++ b/src-tauri/src/acp/delegation/spawner.rs @@ -14,6 +14,8 @@ use std::collections::BTreeMap; use async_trait::async_trait; +use super::types::DelegationTaskReport; +use crate::db::service::delegation_task_service::ResumeBinding; use crate::models::agent::AgentType; /// Identifies a delegation call across the broker, the ACP layer, and the DB. @@ -28,12 +30,38 @@ pub struct DelegationLink { pub parent_conversation_id: i32, pub parent_tool_use_id: String, pub delegation_call_id: String, + /// Present only on the durable production path. Legacy broker tests leave + /// this empty and keep exercising the original one-shot mock contract. + pub admission: Option, +} + +#[derive(Debug, Clone)] +pub struct DelegationAdmission { + pub source_task_id: Option, + pub task: String, + pub requested_working_dir: Option, + pub preferred_mode_id: Option, + pub preferred_config_values: BTreeMap, + pub resume_binding: Option, +} + +#[derive(Debug, Clone)] +pub enum DelegationDispatch { + Started(i32), + Existing(DelegationTaskReport), + Conflict { + next_task_id: String, + reason: String, + }, + Failed(DelegationTaskReport), } #[derive(Debug, thiserror::Error)] pub enum SpawnerError { #[error("spawn failed: {0}")] Spawn(String), + #[error("session busy: {0}")] + Busy(String), #[error("send prompt failed: {0}")] Send(String), #[error("disconnect failed: {0}")] @@ -82,6 +110,7 @@ impl ResumedSpawn { /// All methods are `async` because the production impl drives a Tokio runtime /// and DB; the mock returns immediately. #[async_trait] +#[allow(clippy::too_many_arguments)] pub trait ConnectionSpawner: Send + Sync { /// Spawn a fresh child ACP connection of `agent_type` in `working_dir`. /// Delegation children are always brand-new sessions (no resume), but the @@ -111,6 +140,26 @@ pub trait ConnectionSpawner: Send + Sync { preferred_config_values: BTreeMap, ) -> Result; + async fn spawn_for_delegation( + &self, + parent_connection_id: &str, + agent_type: AgentType, + working_dir: Option, + preferred_mode_id: Option, + preferred_config_values: BTreeMap, + _task_id: String, + _resume_binding: Option, + ) -> Result { + self.spawn( + parent_connection_id, + agent_type, + working_dir, + preferred_mode_id, + preferred_config_values, + ) + .await + } + /// Send the delegation task as the child's first prompt. The /// `DelegationLink` is persisted onto the new conversation row so the /// lifecycle subscriber can later notify the broker on `TurnComplete`. @@ -121,7 +170,7 @@ pub trait ConnectionSpawner: Send + Sync { conn_id: &str, task: String, link: DelegationLink, - ) -> Result; + ) -> Result; /// Re-spawn a connection for an INTERRUPTED delegation child, resuming the /// agent session identified by `external_session_id` (the child row's @@ -134,6 +183,7 @@ pub trait ConnectionSpawner: Send + Sync { async fn spawn_for_resume( &self, parent_connection_id: &str, + task_id: &str, agent_type: AgentType, working_dir: Option, external_session_id: &str, @@ -154,6 +204,7 @@ pub trait ConnectionSpawner: Send + Sync { prompt: String, folder_id: i32, child_conversation_id: i32, + link: DelegationLink, ) -> Result<(), SpawnerError>; /// Whether any live connection is currently bound to `conversation_id`. @@ -188,7 +239,7 @@ pub mod mock { #[derive(Default)] pub struct MockSpawner { pub spawn_results: Mutex>>, - pub send_results: Mutex>>, + pub send_results: Mutex>>, pub cancels: Mutex>, pub disconnects: Mutex>, pub spawn_args: Mutex>, @@ -229,6 +280,7 @@ pub mod mock { #[derive(Debug, Clone, PartialEq, Eq)] pub struct ResumeSpawnCallArgs { pub parent_connection_id: String, + pub task_id: String, pub agent_type: AgentType, pub working_dir: Option, pub external_session_id: String, @@ -236,12 +288,13 @@ pub mod mock { pub preferred_config_values: BTreeMap, } - #[derive(Debug, Clone, PartialEq, Eq)] + #[derive(Debug, Clone)] pub struct ResumeSendCallArgs { pub conn_id: String, pub prompt: String, pub folder_id: i32, pub child_conversation_id: i32, + pub link: DelegationLink, } impl MockSpawner { @@ -254,7 +307,14 @@ pub mod mock { } pub async fn queue_send(&self, r: Result) { - self.send_results.lock().await.push_back(r); + self.send_results + .lock() + .await + .push_back(r.map(DelegationDispatch::Started)); + } + + pub async fn queue_dispatch(&self, dispatch: DelegationDispatch) { + self.send_results.lock().await.push_back(Ok(dispatch)); } /// Install a one-shot gate that holds the next @@ -328,7 +388,7 @@ pub mod mock { _conn_id: &str, _task: String, _link: DelegationLink, - ) -> Result { + ) -> Result { // Honor a test-installed gate: block here (after the broker has // reserved the child, before it parks the pending entry) until the // test releases it. @@ -346,27 +406,30 @@ pub mod mock { async fn spawn_for_resume( &self, parent_connection_id: &str, + task_id: &str, agent_type: AgentType, working_dir: Option, external_session_id: &str, preferred_mode_id: Option, preferred_config_values: BTreeMap, ) -> Result { - self.resume_spawn_args.lock().await.push(ResumeSpawnCallArgs { - parent_connection_id: parent_connection_id.to_string(), - agent_type, - working_dir, - external_session_id: external_session_id.to_string(), - preferred_mode_id, - preferred_config_values, - }); + self.resume_spawn_args + .lock() + .await + .push(ResumeSpawnCallArgs { + parent_connection_id: parent_connection_id.to_string(), + task_id: task_id.to_string(), + agent_type, + working_dir, + external_session_id: external_session_id.to_string(), + preferred_mode_id, + preferred_config_values, + }); self.resume_spawn_results .lock() .await .pop_front() - .unwrap_or_else(|| { - Err(SpawnerError::Spawn("no queued resume spawn result".into())) - }) + .unwrap_or_else(|| Err(SpawnerError::Spawn("no queued resume spawn result".into()))) } async fn send_resume_prompt( @@ -375,12 +438,14 @@ pub mod mock { prompt: String, folder_id: i32, child_conversation_id: i32, + link: DelegationLink, ) -> Result<(), SpawnerError> { self.resume_send_args.lock().await.push(ResumeSendCallArgs { conn_id: conn_id.to_string(), prompt, folder_id, child_conversation_id, + link, }); self.resume_send_results .lock() diff --git a/src-tauri/src/acp/delegation/tool_schema.json b/src-tauri/src/acp/delegation/tool_schema.json index b15bb368f3..46a141acb8 100644 --- a/src-tauri/src/acp/delegation/tool_schema.json +++ b/src-tauri/src/acp/delegation/tool_schema.json @@ -1,7 +1,7 @@ [ { "name": "delegate_to_agent", - "description": "Hand off a self-contained sub-task to a separate local AI agent that runs in its own session. ASYNCHRONOUS: returns a task_id right away and the sub-agent keeps working in the background — this call never blocks. So you can fan out several delegations at once and keep working, then collect the results with get_delegation_status by passing the task_ids array (one id to poll a single task, or many at once to poll the whole fan-out in one call) — or stop one early with cancel_delegation(task_id). The sub-agent CANNOT see this conversation, your open files, or earlier turns — it starts cold, so `task` must carry everything it needs. Best for independent, parallelizable work you can describe up front; not for steps that need your ongoing back-and-forth. RECOGNIZING AN EXPLICIT DELEGATION REQUEST: the user can name a sub-agent directly in their message — it appears as `@AgentName` or as a Markdown link `[@AgentName](codeg://agent/)`. Such a mention IS an explicit instruction to delegate the associated work to that agent, even when the user never names this tool. If the message names several agents, make one call per agent, each carrying that agent's slice of the work.", + "description": "Hand off work to a separate local AI agent that runs in its own session. ASYNCHRONOUS: returns a task_id right away and the sub-agent keeps working in the background — this call never blocks. So you can fan out several delegations at once and keep working, then collect the results with get_delegation_status by passing the task_ids array (one id to poll a single task, or many at once to poll the whole fan-out in one call) — or stop one early with cancel_delegation(task_id). Best for work you can hand off and collect asynchronously — either self-contained in one round, or as successive rounds on the same child via continue_from_task_id. Without continue_from_task_id the sub-agent starts cold and cannot see this conversation, your open files, or earlier turns, so task must be self-contained. With continue_from_task_id it strictly restores that task's child agent session and can use the child session's earlier context; use the newest returned task_id for each later business round. RECOGNIZING AN EXPLICIT DELEGATION REQUEST: the user can name a sub-agent directly in their message — it appears as `@AgentName` or as a Markdown link `[@AgentName](codeg://agent/)`. Such a mention IS an explicit instruction to delegate the associated work to that agent, even when the user never names this tool. If the message names several agents, make one call per agent, each carrying that agent's slice of the work.", "inputSchema": { "type": "object", "required": ["agent_type", "task"], @@ -25,15 +25,19 @@ "qoder", "antigravity" ], - "description": "Which local agent runs the sub-task. Pick the one best suited to the work — or, when the user's message names an agent via `@AgentName` / `codeg://agent/`, use that exact `` slug (e.g. `codeg://agent/claude_code` means `claude_code`)." + "description": "Which local agent runs the task. Pick the one best suited to the work — or, when the user's message names an agent via `@AgentName` / `codeg://agent/`, use that exact `` slug (e.g. `codeg://agent/claude_code` means `claude_code`)." }, "task": { "type": "string", - "description": "The complete, self-contained prompt for the sub-agent. It has no access to this conversation or your context, so spell out everything: the goal, the relevant background and absolute file paths, any constraints, and exactly what to return." + "description": "The task for the sub-agent. For a fresh delegation, make it self-contained because the child has no earlier context. For a continuation, describe the new business round; the same child agent session retains its earlier context." }, "working_dir": { "type": "string", "description": "Absolute path the sub-agent runs in. Defaults to this session's working directory." + }, + "continue_from_task_id": { + "type": "string", + "description": "A terminal task_id from this parent whose exact child agent session should be resumed after its process is fully released. Each source permits one successor: an identical retry returns that successor, while a different retry reports a conflict. The continuation receives a new task_id; use the newer id for another round." } } } @@ -75,14 +79,14 @@ }, { "name": "resume_delegation", - "description": "Resume a delegated task that was INTERRUPTED — canceled, or cut off when a session/app terminated unexpectedly — so its sub-agent picks the ORIGINAL task back up in its own session with all prior context and work intact, under the SAME task_id (keep collecting it with get_delegation_status, or stop it again with cancel_delegation). STRICT SCOPE: this tool ONLY revives interrupted work; it takes no task text and must NOT be used to iterate on, extend, retry, or redo a task. A task that is still running, or that already finished on its own (completed or failed), is refused with error_code \"not_resumable\" and its actual status — for follow-up or retry work, start a fresh delegate_to_agent call instead. The optional `reason` is context about WHY the run is being resumed (e.g. \"the app was killed mid-run\"), passed to the sub-agent as interruption context — never new instructions.", + "description": "Resume a legacy delegated task that predates the durable task ledger and was INTERRUPTED — canceled, or cut off when a session/app terminated unexpectedly — so its sub-agent picks the ORIGINAL task back up in its own session with all prior context and work intact, under the SAME task_id (keep collecting it with get_delegation_status, or stop it again with cancel_delegation). STRICT SCOPE: this tool ONLY revives interrupted work; it takes no task text and must NOT be used to iterate on, extend, retry, or redo a task. New ledger tasks must continue through delegate_to_agent with continue_from_task_id, which creates a new task id for the next round. A legacy task that is still running, or that already finished on its own (completed or failed), is refused with error_code \"not_resumable\" and its actual status — for follow-up or retry work, call delegate_to_agent and use continue_from_task_id when continuing the same child session. The optional `reason` is context about WHY the run is being resumed (e.g. \"the app was killed mid-run\"), passed to the sub-agent as interruption context — never new instructions.", "inputSchema": { "type": "object", "required": ["task_id"], "properties": { "task_id": { "type": "string", - "description": "The task_id of the interrupted task — the id delegate_to_agent originally returned (also reported by get_delegation_status as canceled). The resumed run keeps this exact id." + "description": "The task_id of an interrupted legacy task that predates the durable ledger. The resumed run keeps this exact id; ledger-backed tasks continue with delegate_to_agent and continue_from_task_id instead." }, "reason": { "type": "string", diff --git a/src-tauri/src/acp/delegation/types.rs b/src-tauri/src/acp/delegation/types.rs index bb597b9720..662ced899c 100644 --- a/src-tauri/src/acp/delegation/types.rs +++ b/src-tauri/src/acp/delegation/types.rs @@ -70,6 +70,8 @@ pub struct DelegationRequest { #[serde(default, skip_serializing_if = "Option::is_none")] pub requested_working_dir: Option, #[serde(default, skip_serializing_if = "Option::is_none")] + pub continue_from_task_id: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] pub external_handle: Option, } @@ -136,6 +138,12 @@ pub enum DelegationError { InvalidWorkingDir(String), #[error("spawn failed: {0}")] SpawnFailed(String), + #[error("continuation is busy: {0}")] + ContinuationBusy(String), + #[error("continuation conflict: {0}")] + ContinuationConflict(String), + #[error("continuation is not eligible: {0}")] + ContinuationInvalid(String), #[error("subagent runtime error: {0}")] SubagentRuntimeError(String), /// Child agent ended its turn via `refusal`. Often a backend / gateway @@ -294,6 +302,9 @@ impl DelegationOutcome { DelegationError::InvalidAgentType => "invalid_agent_type", DelegationError::InvalidWorkingDir(_) => "invalid_working_dir", DelegationError::SpawnFailed(_) => "spawn_failed", + DelegationError::ContinuationBusy(_) => "continuation_busy", + DelegationError::ContinuationConflict(_) => "continuation_conflict", + DelegationError::ContinuationInvalid(_) => "continuation_invalid", DelegationError::SubagentRuntimeError(_) => "subagent_error", DelegationError::ChildRefusal => "child_refusal", DelegationError::ChildMaxTokens => "child_max_tokens", diff --git a/src-tauri/src/acp/error.rs b/src-tauri/src/acp/error.rs index 60ba0cc7a1..e2928e90df 100644 --- a/src-tauri/src/acp/error.rs +++ b/src-tauri/src/acp/error.rs @@ -10,6 +10,8 @@ pub enum AcpError { Protocol(String), #[error("agent process exited unexpectedly")] ProcessExited, + #[error("agent session is busy: {0}")] + SessionBusy(String), /// A prompt arrived while this connection already had a turn in flight. /// The connection loop processes one turn at a time; a second concurrent /// prompt (e.g. two co-controlling clients sending near-simultaneously) @@ -103,6 +105,7 @@ impl AcpError { Self::InitializeTimeout => Some("initialize_timeout"), Self::ProbeTimedOut => Some("probe_timed_out"), Self::ProcessExited => Some("process_exited"), + Self::SessionBusy(_) => Some("session_busy"), Self::TurnInProgress => Some("turn_in_progress"), Self::NoActiveTurn => Some("no_active_turn"), Self::FeedbackDisabled => Some("feedback_disabled"), diff --git a/src-tauri/src/acp/lifecycle.rs b/src-tauri/src/acp/lifecycle.rs index 2779992305..d86a8b0a69 100644 --- a/src-tauri/src/acp/lifecycle.rs +++ b/src-tauri/src/acp/lifecycle.rs @@ -128,6 +128,7 @@ fn is_dispatcher_terminal(event: &AcpEvent) -> bool { /// can wake up after the entry is already gone. struct CachedConn { conversation_id: i32, + delegation_task_id: Option, state: Arc>, emitter: EventEmitter, } @@ -267,8 +268,8 @@ pub(crate) async fn handle_event( // `cancelled` and any future reason: don't write here. _ => None, }; - let Some((state_arc, emitter)) = - manager.get_state_and_emitter(&envelope.connection_id).await + let Some((state_arc, emitter, delegation_task_id)) = + manager.get_lifecycle_context(&envelope.connection_id).await else { return Ok(()); }; @@ -286,16 +287,31 @@ pub(crate) async fn handle_event( // DB write before emit so any downstream subscriber that observes // the ConversationStatusChanged event can assume the row is // already at the target status. - conversation_service::update_status(db_conn, cid, ts.clone()).await?; - emit_with_state( - &state_arc, - &emitter, - AcpEvent::ConversationStatusChanged { - conversation_id: cid, - status: ts, - }, + let changed = conversation_service::update_status_for_execution_if( + db_conn, + cid, + delegation_task_id.as_deref(), + ConversationStatus::InProgress, + ts.clone(), ) - .await; + .await?; + if !changed { + tracing::info!( + conversation_id = cid, + task_id = delegation_task_id.as_deref(), + "[delegation] terminal status CAS did not change the conversation" + ); + } else { + emit_with_state( + &state_arc, + &emitter, + AcpEvent::ConversationStatusChanged { + conversation_id: cid, + status: ts, + }, + ) + .await; + } } // If this conversation was spawned by a delegation, resolve the @@ -306,6 +322,7 @@ pub(crate) async fn handle_event( db_conn, b.as_ref(), cid, + delegation_task_id.as_deref(), stop_reason.as_str(), last_text, ) @@ -368,6 +385,7 @@ async fn forward_turn_complete_to_broker( db_conn: &DatabaseConnection, broker: &DelegationBroker, conversation_id: i32, + execution_task_id: Option<&str>, stop_reason: &str, last_text: Option, ) { @@ -381,9 +399,10 @@ async fn forward_turn_complete_to_broker( return; } }; - let call_id = match row.delegation_call_id.clone() { - Some(id) => id, + let call_id = match execution_task_id { + Some(id) if row.delegation_call_id.as_deref() == Some(id) => id.to_string(), None => return, // not a delegation child; nothing to do. + Some(_) => return, }; if row.parent_tool_use_id.is_none() { tracing::info!( @@ -449,7 +468,9 @@ async fn try_cache_link( // The connection is necessarily still in the manager at this point — // `ConversationLinked` is emitted by `send_prompt_linked` from the // connection's own send path, well before any disconnect. - let Some((state, emitter)) = manager.get_state_and_emitter(connection_id).await else { + let Some((state, emitter, delegation_task_id)) = + manager.get_lifecycle_context(connection_id).await + else { tracing::warn!( "[lifecycle][WARN] ConversationLinked for unknown connection {connection_id}; \ skipping cache (terminal-status hand-off will no-op)" @@ -460,6 +481,7 @@ async fn try_cache_link( connection_id.to_string(), CachedConn { conversation_id, + delegation_task_id, state, emitter, }, @@ -484,9 +506,10 @@ async fn handle_terminal_event( return Ok(()); }; let cid = entry.conversation_id; - let changed = conversation_service::update_status_if( + let changed = conversation_service::update_status_for_execution_if( db_conn, cid, + entry.delegation_task_id.as_deref(), ConversationStatus::InProgress, ConversationStatus::Cancelled, ) @@ -665,10 +688,17 @@ fn extract_delegation_match_key(raw_input: Option<&str>) -> Option { + assert_eq!(success.text, "finished despite cancel race"); + assert_eq!(success.child_conversation_id, child.id); + } + other => panic!("expected successful broker completion, got {other:?}"), + } + assert_eq!( + read_row_status(&db, child.id).await, + ConversationStatus::Cancelled, + "the losing status CAS must not rewrite the independently-cancelled row" + ); + } + /// `Error` alone must NOT drain the broker. The pending entry stays /// in-flight so an upcoming `TurnComplete` can resolve it via /// `complete_call` with the correct child-side error mapping. diff --git a/src-tauri/src/acp/manager.rs b/src-tauri/src/acp/manager.rs index c8db43c9b6..bfb5c12590 100644 --- a/src-tauri/src/acp/manager.rs +++ b/src-tauri/src/acp/manager.rs @@ -10,10 +10,11 @@ use sea_orm::{ TransactionTrait, }; +use crate::acp::agent_mentions::strip_route_separator_from_prompt; use crate::acp::connection::{ - spawn_agent_connection, AgentConnection, ConnectionCommand, GoalControlAction, SteerOutcome, + spawn_agent_connection, AgentConnection, ConnectionCommand, GoalControlAction, + SessionRecoveryPolicy, SteerOutcome, }; -use crate::acp::agent_mentions::strip_route_separator_from_prompt; use crate::acp::error::AcpError; use crate::acp::feedback::{ bounded_feedback_batch, FeedbackItem, FeedbackStatus, PendingFeedback, @@ -29,7 +30,7 @@ use crate::acp::question::{ use crate::acp::terminal_runtime::TerminalShellRuntimeConfig; use crate::acp::types::{ AcpEvent, AgentOptionsSnapshot, ConfigStaleKind, ConnectionInfo, ConnectionStatus, - ForkResultInfo, PromptCapabilitiesInfo, PromptInputBlock, + ForkResultInfo, PromptCapabilitiesInfo, PromptInputBlock, SessionConfigKindInfo, }; use crate::db::entities::conversation::{self, ConversationKind, ConversationStatus}; use crate::db::service::conversation_service; @@ -275,13 +276,14 @@ fn delegation_child_title_seed(blocks: &[PromptInputBlock]) -> Option { } /// Composite key identifying a logical agent session for spawn-time dedup. -/// Two `acp_connect` calls with the same triple race for the same `Mutex`, +/// The cwd is deliberately not part of the key: it is a binding to validate, +/// not a namespace that may open two processes for one external session. +/// Two `acp_connect` calls with the same pair race for the same `Mutex`, /// so the second one observes the first's freshly-spawned connection in /// `find_connection_for_reuse` instead of starting a duplicate process. #[derive(Clone, Debug, Eq, Hash, PartialEq)] struct SpawnDedupKey { agent_type: AgentType, - working_dir: Option, session_id: String, } @@ -368,6 +370,8 @@ async fn wait_for_session_started( /// A connection that has left the map while its process may still be running. struct DrainingChild { agent: AgentType, + requested_session_id: Option, + pid_was_published: bool, /// The connection's live pid cell. `on_spawn` publishes the pid and /// `on_exit` zeroes it on a real reap. pid: Arc, @@ -540,16 +544,11 @@ impl ConnectionManager { /// Install the chat-channel manager exactly once during bootstrap so /// live title writes can sync bound forum topics. Calling twice is a /// no-op. Tests leave this unset and skip the remote sync. - pub fn install_chat_channel( - &self, - manager: crate::chat_channel::manager::ChatChannelManager, - ) { + pub fn install_chat_channel(&self, manager: crate::chat_channel::manager::ChatChannelManager) { let _ = self.chat_channel.set(manager); } - pub(crate) fn chat_channel( - &self, - ) -> Option { + pub(crate) fn chat_channel(&self) -> Option { self.chat_channel.get().map(|c| c.clone_ref()) } @@ -623,6 +622,9 @@ impl ConnectionManager { config_fingerprint: String::new(), last_observed_fingerprint: String::new(), child_pid: Arc::new(std::sync::atomic::AtomicU32::new(0)), + requested_session_id: None, + delegation_task_id: None, + driver_cancel: tokio_util::sync::CancellationToken::new(), }; let mut map = self.connections.lock().await; map.insert(id.to_string(), conn); @@ -666,6 +668,9 @@ impl ConnectionManager { config_fingerprint: String::new(), last_observed_fingerprint: String::new(), child_pid: Arc::new(std::sync::atomic::AtomicU32::new(0)), + requested_session_id: None, + delegation_task_id: None, + driver_cancel: tokio_util::sync::CancellationToken::new(), }; self.connections.lock().await.insert(id.to_string(), conn); rx @@ -683,12 +688,109 @@ impl ConnectionManager { preferred_mode_id: Option, preferred_config_values: BTreeMap, ) -> Result { - // Held for the whole establishment. A restore writing back to the - // agents' own directories takes the write side, so it can never see an - // empty connection list and then have one appear underneath it. Not - // re-entrant: nothing reachable from here calls `spawn_agent` again. - let _restore_guard = self.external_restore_lock.read().await; + self.spawn_agent_with_policy( + agent_type, + working_dir, + session_id, + runtime_env, + owner_window_label, + emitter, + preferred_mode_id, + preferred_config_values, + SessionRecoveryPolicy::BestEffort, + None, + ) + .await + } + + /// Spawn a broker-owned connection with an immutable task identity. A + /// continuation passes `Strict`; an initial delegation passes + /// `BestEffort` with no requested session. + #[allow(clippy::too_many_arguments)] + pub(crate) async fn spawn_delegation_agent( + &self, + agent_type: AgentType, + working_dir: Option, + session_id: Option, + runtime_env: BTreeMap, + owner_window_label: String, + emitter: EventEmitter, + preferred_mode_id: Option, + preferred_config_values: BTreeMap, + recovery_policy: SessionRecoveryPolicy, + task_id: String, + ) -> Result { + let expected_working_dir = working_dir.as_ref().map(PathBuf::from); + let expected_session_id = session_id.clone(); + let expected_mode_id = preferred_mode_id.clone(); + let expected_config_values = preferred_config_values.clone(); + let expected_task_id = task_id.clone(); + let conn_id = self + .spawn_agent_with_policy( + agent_type, + working_dir, + session_id, + runtime_env, + owner_window_label, + emitter, + preferred_mode_id, + preferred_config_values, + recovery_policy, + Some(task_id), + ) + .await?; + if recovery_policy == SessionRecoveryPolicy::Strict { + let session_id = expected_session_id.as_deref().ok_or_else(|| { + AcpError::protocol("strict session recovery requires an external session id") + })?; + if let Err(error) = self + .wait_for_strict_resume_ready( + &conn_id, + expected_task_id.as_str(), + agent_type, + expected_working_dir.as_ref(), + session_id, + expected_mode_id.as_deref(), + &expected_config_values, + ) + .await + { + let _ = self + .request_delegation_disconnect(&conn_id, &expected_task_id) + .await; + return Err(error); + } + } else if let Err(error) = self + .wait_for_delegation_ready(&conn_id, &expected_task_id) + .await + { + let _ = self + .request_delegation_disconnect(&conn_id, &expected_task_id) + .await; + return Err(error); + } + + Ok(conn_id) + } + + #[allow(clippy::too_many_arguments)] + async fn spawn_agent_with_policy( + &self, + agent_type: AgentType, + working_dir: Option, + session_id: Option, + runtime_env: BTreeMap, + owner_window_label: String, + emitter: EventEmitter, + preferred_mode_id: Option, + preferred_config_values: BTreeMap, + recovery_policy: SessionRecoveryPolicy, + delegation_task_id: Option, + ) -> Result { + // External state restore takes the write side, so no connection can + // appear while it is replacing agent-owned directories. + let _restore_guard = self.external_restore_lock.read().await; // Connection dedup: when resuming an agent session (session_id is // Some), look for a live AgentConnection that already represents // the same external session in the same working_dir for the same @@ -710,7 +812,6 @@ impl ConnectionManager { let dedup_lock = if let Some(sid) = session_id.as_deref() { let key = SpawnDedupKey { agent_type, - working_dir: working_dir_path.clone(), session_id: sid.to_string(), }; let mu = { @@ -725,16 +826,36 @@ impl ConnectionManager { None }; - if let Some(existing) = self - .find_connection_for_reuse(agent_type, working_dir_path.as_ref(), session_id.as_deref()) - .await - { - tracing::info!( - "[ACP] reusing connection id={} for session_id={}", - existing, - session_id.as_deref().unwrap_or("") - ); - return Ok(existing); + if let Some(sid) = session_id.as_deref() { + if let Some((existing, existing_cwd, status)) = + self.find_connection_for_session(agent_type, sid).await + { + let same_cwd = existing_cwd.as_ref() == working_dir_path.as_ref(); + if recovery_policy == SessionRecoveryPolicy::BestEffort + && same_cwd + && !matches!( + status, + ConnectionStatus::Disconnected | ConnectionStatus::Error + ) + { + tracing::info!( + "[ACP] reusing connection id={} for session_id={}", + existing, + sid + ); + return Ok(existing); + } + return Err(AcpError::SessionBusy(format!( + "session {sid} already has connection {existing} ({status:?}); wait for it to be released" + ))); + } + if recovery_policy == SessionRecoveryPolicy::Strict + && !self.settle_draining_session(agent_type, sid).await + { + return Err(AcpError::SessionBusy(format!( + "session {sid} still has a process being released" + ))); + } } let connection_id = uuid::Uuid::new_v4().to_string(); @@ -760,6 +881,8 @@ impl ConnectionManager { preferred_config_values, self.delegation_snapshot(), self.terminal_shell_config.clone(), + recovery_policy, + delegation_task_id, ) .await?; @@ -909,7 +1032,12 @@ impl ConnectionManager { } } for (state, emitter, stale) in targets { - emit_with_state(&state, &emitter, AcpEvent::SessionConfigStale { stale, kind }).await; + emit_with_state( + &state, + &emitter, + AcpEvent::SessionConfigStale { stale, kind }, + ) + .await; } stale_count } @@ -931,6 +1059,7 @@ impl ConnectionManager { /// for an imperceptible latency win. The connections-map mutex is held /// across the awaits — fine because no path takes `state.write()` while /// holding the connections mutex (no lock-cycle). + #[allow(dead_code)] pub(crate) async fn find_connection_for_reuse( &self, agent_type: AgentType, @@ -939,25 +1068,36 @@ impl ConnectionManager { ) -> Option { // No session_id → caller is opening a fresh session; never dedup. let session_id = session_id?; + let (id, cwd, status) = self + .find_connection_for_session(agent_type, session_id) + .await?; + (cwd.as_ref() == working_dir + && !matches!( + status, + ConnectionStatus::Disconnected | ConnectionStatus::Error + )) + .then_some(id) + } + + /// Find any map occupant claiming a requested or established external + /// session. Requested identity closes the handshake window where + /// `SessionState::external_id` is not populated yet. + async fn find_connection_for_session( + &self, + agent_type: AgentType, + session_id: &str, + ) -> Option<(String, Option, ConnectionStatus)> { let connections = self.connections.lock().await; for (id, conn) in connections.iter() { if conn.agent_type != agent_type { continue; } let state = conn.state.read().await; - if state.external_id.as_deref() != Some(session_id) { - continue; - } - if state.working_dir.as_ref() != working_dir { - continue; - } - if matches!( - state.status, - ConnectionStatus::Disconnected | ConnectionStatus::Error - ) { - continue; + if conn.requested_session_id.as_deref() == Some(session_id) + || state.external_id.as_deref() == Some(session_id) + { + return Some((id.clone(), state.working_dir.clone(), state.status.clone())); } - return Some(id.clone()); } None } @@ -1055,6 +1195,17 @@ impl ConnectionManager { conn_id: &str, blocks: Vec, ) -> Result<(), AcpError> { + { + let connections = self.connections.lock().await; + let conn = connections + .get(conn_id) + .ok_or_else(|| AcpError::ConnectionNotFound(conn_id.into()))?; + if conn.delegation_task_id.is_some() { + return Err(AcpError::protocol( + "delegation connection is reserved until process cleanup completes", + )); + } + } let prompt_lock = self.clone_prompt_lock(conn_id).await?; let _guard = prompt_lock.lock_owned().await; self.send_prompt_inner(conn_id, blocks, None).await @@ -1145,15 +1296,9 @@ impl ConnectionManager { "conversation_id provided without folder_id".to_string(), )); } - // Delegation is only meaningful on the create-new-row branch — adopting - // an existing caller-supplied row already has its own (or no) parent - // linkage. Reject the combination loudly so a misuse from the broker - // doesn't silently drop the linkage. - if delegation.is_some() && conversation_id.is_some() { - return Err(AcpError::protocol( - "delegation link is incompatible with caller-supplied conversation_id".to_string(), - )); - } + // A continuation deliberately adopts its source child row while + // carrying a new delegation execution id. The durable ledger owns + // history; the row's delegation_call_id is only the current pointer. // Acquire the per-connection prompt lock for the entire link-check // + DB write + emit + cmd_tx.send sequence. Two concurrent prompts @@ -1172,6 +1317,16 @@ impl ConnectionManager { let conn = connections .get(conn_id) .ok_or_else(|| AcpError::ConnectionNotFound(conn_id.into()))?; + if let Some(task_id) = conn.delegation_task_id.as_deref() { + let supplied = delegation + .as_ref() + .map(|link| link.delegation_call_id.as_str()); + if supplied != Some(task_id) { + return Err(AcpError::protocol( + "delegation connection is reserved by another task", + )); + } + } let (already, in_flight) = { let s = conn.state.read().await; (s.conversation_id.is_some(), s.turn_in_flight) @@ -1484,6 +1639,11 @@ impl ConnectionManager { // on the row (touches `updated_at` only). let conversation_id_for_status = state_arc.read().await.conversation_id; if let Some(cid) = conversation_id_for_status { + if delegation.is_none() { + conversation_service::clear_delegation_call_id(&db.conn, cid) + .await + .map_err(|e| AcpError::protocol(e.to_string()))?; + } conversation_service::update_status(&db.conn, cid, ConversationStatus::InProgress) .await .map_err(|e| AcpError::protocol(e.to_string()))?; @@ -1649,6 +1809,11 @@ impl ConnectionManager { let conn = connections .get(conn_id) .ok_or_else(|| AcpError::ConnectionNotFound(conn_id.into()))?; + if conn.delegation_task_id.is_some() { + return Err(AcpError::protocol( + "delegation connection configuration is immutable", + )); + } conn.cmd_tx.clone() }; cmd_tx @@ -1668,6 +1833,11 @@ impl ConnectionManager { let conn = connections .get(conn_id) .ok_or_else(|| AcpError::ConnectionNotFound(conn_id.into()))?; + if conn.delegation_task_id.is_some() { + return Err(AcpError::protocol( + "delegation connection configuration is immutable", + )); + } conn.cmd_tx.clone() }; cmd_tx @@ -1807,7 +1977,7 @@ impl ConnectionManager { } pub async fn cancel(&self, db: &DatabaseConnection, conn_id: &str) -> Result<(), AcpError> { - let (cmd_tx, state_arc, emitter) = { + let (cmd_tx, state_arc, emitter, delegation_task_id) = { let connections = self.connections.lock().await; let conn = connections .get(conn_id) @@ -1816,6 +1986,7 @@ impl ConnectionManager { conn.cmd_tx.clone(), conn.state.clone(), conn.emitter.clone(), + conn.delegation_task_id.clone(), ) }; cmd_tx @@ -1832,9 +2003,10 @@ impl ConnectionManager { // status if the turn happened to end just before the user clicked. let conversation_id = state_arc.read().await.conversation_id; if let Some(cid) = conversation_id { - match conversation_service::update_status_if( + match conversation_service::update_status_for_execution_if( db, cid, + delegation_task_id.as_deref(), ConversationStatus::InProgress, ConversationStatus::Cancelled, ) @@ -2332,20 +2504,37 @@ impl ConnectionManager { } pub async fn disconnect(&self, conn_id: &str) -> Result<(), AcpError> { - let removed = { - // The map lock is held ACROSS the handoff into `draining`, and - // readers take it in the same order, so an observer can never see - // the connection in neither place. + let (cmd_tx, driver_cancel, retained) = { let mut connections = self.connections.lock().await; - let removed = connections.remove(conn_id); - if let Some(conn) = &removed { - self.park_draining(conn).await; + let retained = connections + .get(conn_id) + .is_some_and(|conn| conn.delegation_task_id.is_some()); + let cmd_tx = if retained { + connections + .get(conn_id) + .map(|conn| (conn.cmd_tx.clone(), conn.driver_cancel.clone())) + } else { + let removed = connections.remove(conn_id); + if let Some(conn) = &removed { + self.park_draining(conn).await; + } + removed.map(|conn| (conn.cmd_tx, conn.driver_cancel)) + }; + match cmd_tx { + Some((cmd_tx, cancel)) => (Some(cmd_tx), retained.then_some(cancel), retained), + None => (None, None, retained), } - removed }; - if let Some(conn) = removed { - tracing::info!("[ACP] disconnect connection={}", conn_id); - let _ = conn.cmd_tx.send(ConnectionCommand::Disconnect).await; + if let Some(cmd_tx) = cmd_tx { + tracing::info!( + "[ACP] disconnect connection={} retain_until_reaped={}", + conn_id, + retained + ); + if let Some(cancel) = driver_cancel { + cancel.cancel(); + } + let _ = cmd_tx.send(ConnectionCommand::Disconnect).await; Ok(()) } else { Err(AcpError::ConnectionNotFound(conn_id.into())) @@ -2359,11 +2548,51 @@ impl ConnectionManager { prune_reaped(&mut draining); draining.push(DrainingChild { agent: conn.agent_type, + requested_session_id: conn.requested_session_id.clone(), + pid_was_published: conn.child_pid.load(std::sync::atomic::Ordering::SeqCst) != 0, pid: conn.child_pid.clone(), parked_at: std::time::Instant::now(), }); } + /// A strict continuation must not start a second writer for a session that + /// a normal UI teardown has only just removed from the live map. + async fn settle_draining_session(&self, agent: AgentType, session_id: &str) -> bool { + let children: Vec<(Arc, bool)> = { + let mut draining = self.draining.lock().await; + prune_reaped(&mut draining); + draining + .iter() + .filter(|child| { + child.agent == agent + && child.requested_session_id.as_deref() == Some(session_id) + }) + .map(|child| (Arc::clone(&child.pid), child.pid_was_published)) + .collect() + }; + if children.is_empty() { + return true; + } + + tokio::time::sleep(DISCONNECT_ALL_GRACE).await; + tokio::task::spawn_blocking(move || { + let mut settled = true; + for (cell, was_published) in children { + if !was_published && cell.load(std::sync::atomic::Ordering::SeqCst) == 0 { + // Still ambiguous: the old connection may publish its pid + // after this check. Refuse this attempt; a retry can settle + // it once the pid appears or the drain grace expires. + settled = false; + } else if !kill_tree_and_wait(cell.as_ref()) { + settled = false; + } + } + settled + }) + .await + .unwrap_or(false) + } + /// Every agent that could still be writing to its own files: connected or /// prompting, plus any whose connection is gone but whose process is not. /// @@ -2526,13 +2755,7 @@ impl ConnectionManager { let grace_period = Duration::from_millis(500); let mut selectors_ready_at: Option = None; loop { - let ( - config_options, - modes, - available_commands, - prompt_capabilities, - selectors_ready, - ) = { + let (config_options, modes, available_commands, prompt_capabilities, selectors_ready) = { let conns = self.connections.lock().await; let conn = conns .get(conn_id) @@ -2566,6 +2789,182 @@ impl ConnectionManager { } } + /// Wait until recovery has applied its selector preferences, then verify + /// the immutable session identity before a continuation prompt is sent. + #[allow(clippy::too_many_arguments)] + async fn wait_for_strict_resume_ready( + &self, + conn_id: &str, + task_id: &str, + agent_type: AgentType, + working_dir: Option<&PathBuf>, + session_id: &str, + mode_id: Option<&str>, + config_values: &BTreeMap, + ) -> Result<(), AcpError> { + let started = std::time::Instant::now(); + loop { + let (ready, status, actual_session, actual_mode, options) = { + let connections = self.connections.lock().await; + let conn = connections + .get(conn_id) + .ok_or_else(|| AcpError::ConnectionNotFound(conn_id.into()))?; + if conn.agent_type != agent_type + || conn.delegation_task_id.as_deref() != Some(task_id) + { + return Err(AcpError::protocol( + "strict resume execution identity changed during startup", + )); + } + let state = conn.state.read().await; + if state.working_dir.as_ref() != working_dir { + return Err(AcpError::protocol( + "strict resume working directory does not match the source task", + )); + } + ( + state.selectors_ready, + state.status.clone(), + state.external_id.clone(), + state.current_mode.clone(), + state.config_options.clone(), + ) + }; + + if matches!( + status, + ConnectionStatus::Error | ConnectionStatus::Disconnected + ) { + return Err(AcpError::protocol(format!( + "strict session recovery ended before it became ready ({status:?})" + ))); + } + if ready { + if actual_session.as_deref() != Some(session_id) { + return Err(AcpError::protocol( + "strict resume returned a different external session id", + )); + } + if mode_id.is_some_and(|expected| actual_mode.as_deref() != Some(expected)) { + tracing::warn!( + task_id, + expected_mode = ?mode_id, + actual_mode = ?actual_mode.as_deref(), + "[delegation] continuation resumed with a different mode" + ); + } + let options = options.unwrap_or_default(); + for (id, expected) in config_values { + let Some(option) = options.iter().find(|option| option.id == *id) else { + tracing::warn!( + task_id, + config_id = id, + "[delegation] continuation did not expose a preferred config option" + ); + continue; + }; + let matches = match &option.kind { + SessionConfigKindInfo::Select(select) => select.current_value == *expected, + SessionConfigKindInfo::Boolean(boolean) => { + boolean.current_value == (expected == "true") + } + }; + if !matches { + tracing::warn!( + task_id, + config_id = id, + "[delegation] continuation resumed with a different config value" + ); + } + } + return Ok(()); + } + if started.elapsed() >= self.spawn_handshake_timeout { + return Err(AcpError::protocol( + "strict session recovery timed out before configuration was ready", + )); + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + } + + /// Initial delegation also needs a durable external session binding before + /// the broker may admit and send its first prompt. + async fn wait_for_delegation_ready( + &self, + conn_id: &str, + task_id: &str, + ) -> Result<(), AcpError> { + let started = std::time::Instant::now(); + loop { + let (ready, external_id, status) = { + let connections = self.connections.lock().await; + let conn = connections + .get(conn_id) + .ok_or_else(|| AcpError::ConnectionNotFound(conn_id.into()))?; + if conn.delegation_task_id.as_deref() != Some(task_id) { + return Err(AcpError::protocol( + "delegation execution identity changed during startup", + )); + } + let state = conn.state.read().await; + ( + state.selectors_ready, + state.external_id.clone(), + state.status.clone(), + ) + }; + if matches!( + status, + ConnectionStatus::Error | ConnectionStatus::Disconnected + ) { + return Err(AcpError::protocol( + "delegation connection ended before its session binding was ready", + )); + } + if ready && external_id.is_some() { + return Ok(()); + } + if started.elapsed() >= self.spawn_handshake_timeout { + return Err(AcpError::protocol( + "delegation session binding timed out before prompt admission", + )); + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + } + + /// Ask a broker-owned connection to stop without removing its occupancy. + /// The fixed task check prevents a failed setup from disconnecting a newer + /// execution, and the entry remains busy until the driver/reap barrier + /// confirms release. + pub(crate) async fn request_delegation_disconnect( + &self, + conn_id: &str, + task_id: &str, + ) -> Result<(), AcpError> { + let (cmd_tx, driver_cancel) = { + let connections = self.connections.lock().await; + let conn = connections + .get(conn_id) + .ok_or_else(|| AcpError::ConnectionNotFound(conn_id.into()))?; + if conn.delegation_task_id.as_deref() != Some(task_id) { + return Err(AcpError::protocol( + "delegation connection belongs to a different task", + )); + } + (conn.cmd_tx.clone(), conn.driver_cancel.clone()) + }; + // A strict resume can still be blocked inside session/load and unable + // to consume the command queue. Cancel the driver so its process-reap + // guard runs even in that handshake window. + driver_cancel.cancel(); + cmd_tx + .send(ConnectionCommand::Disconnect) + .await + .map_err(|_| AcpError::ProcessExited) + } + pub async fn disconnect_by_owner_window(&self, owner_window_label: &str) -> usize { let cmd_txs = { let mut connections = self.connections.lock().await; @@ -2582,18 +2981,26 @@ impl ConnectionManager { let mut txs = Vec::with_capacity(ids.len()); for id in ids { - if let Some(conn) = connections.remove(&id) { - // Same handoff as `disconnect`: closing a window leaves - // the agents exiting, not exited. + let retained = connections + .get(&id) + .is_some_and(|conn| conn.delegation_task_id.is_some()); + if retained { + if let Some(conn) = connections.get(&id) { + txs.push((conn.cmd_tx.clone(), Some(conn.driver_cancel.clone()))); + } + } else if let Some(conn) = connections.remove(&id) { self.park_draining(&conn).await; - txs.push(conn.cmd_tx); + txs.push((conn.cmd_tx, None)); } } txs }; let disconnected = cmd_txs.len(); - for cmd_tx in cmd_txs { + for (cmd_tx, cancel) in cmd_txs { + if let Some(cancel) = cancel { + cancel.cancel(); + } let _ = cmd_tx.send(ConnectionCommand::Disconnect).await; } tracing::info!( @@ -2835,9 +3242,7 @@ impl ConnectionManager { /// Block new connections from being established until the returned guard /// is dropped. The caller must enumerate live connections only AFTER /// holding this, never before. - pub async fn lock_out_new_connections( - &self, - ) -> tokio::sync::OwnedRwLockWriteGuard<()> { + pub async fn lock_out_new_connections(&self) -> tokio::sync::OwnedRwLockWriteGuard<()> { self.external_restore_lock.clone().write_owned().await } @@ -2927,6 +3332,27 @@ impl ConnectionManager { .map(|conn| (conn.state.clone(), conn.emitter.clone())) } + /// Snapshot lifecycle routing data under one connections-map lock. The + /// task id is immutable for the lifetime of a connection, so a late event + /// cannot become human-owned merely because cleanup reaped the entry. + pub(crate) async fn get_lifecycle_context( + &self, + conn_id: &str, + ) -> Option<( + std::sync::Arc>, + EventEmitter, + Option, + )> { + let connections = self.connections.lock().await; + connections.get(conn_id).map(|conn| { + ( + conn.state.clone(), + conn.emitter.clone(), + conn.delegation_task_id.clone(), + ) + }) + } + /// Wait (bounded) for the connected agent to say what a prompt may carry. /// /// `spawn_agent` returns as soon as the process is up and registered: it @@ -3052,11 +3478,8 @@ impl ConnectionManager { return Err(AcpError::NoActiveTurn); } - let item = FeedbackItem::new_pending( - uuid::Uuid::new_v4().to_string(), - text, - chrono::Utc::now(), - ); + let item = + FeedbackItem::new_pending(uuid::Uuid::new_v4().to_string(), text, chrono::Utc::now()); // Gate on `turn_in_flight` and append in ONE critical section (via the // gated emit): a `TurnComplete` (flips the flag) or `UserMessage` // (clears `feedback`) can't slip between the gate and the append+seq, so @@ -3214,9 +3637,9 @@ impl ConnectionManager { }) .await .map_err(|_| AcpError::ProcessExited)?; - let steer = reply_rx.await.map_err(|_| { - AcpError::protocol("Steer reply channel closed".to_string()) - })??; + let steer = reply_rx + .await + .map_err(|_| AcpError::protocol("Steer reply channel closed".to_string()))??; match steer { // Honored opt-in: the content was NOT consumed and is // still host-owned. Surface the frontend's existing @@ -3699,8 +4122,12 @@ impl ConnectionManager { // (disconnect removes it before this sweep), so tolerate `None`. if let Some((state, emitter)) = self.get_state_and_emitter(conn_id).await { for approval_id in drained { - emit_with_state(&state, &emitter, AcpEvent::PlanApprovalResolved { approval_id }) - .await; + emit_with_state( + &state, + &emitter, + AcpEvent::PlanApprovalResolved { approval_id }, + ) + .await; } } } @@ -3870,13 +4297,97 @@ impl crate::acp::delegation::spawner::ConnectionSpawner for ConnectionManagerSpa .map_err(|e| SpawnerError::Spawn(e.to_string())) } + async fn spawn_for_delegation( + &self, + parent_connection_id: &str, + agent_type: AgentType, + working_dir: Option, + preferred_mode_id: Option, + preferred_config_values: BTreeMap, + task_id: String, + resume_binding: Option, + ) -> Result { + use crate::acp::delegation::spawner::SpawnerError; + let (emitter, owner_window, parent_working_dir) = { + let conns = self.manager.connections.lock().await; + let parent = conns.get(parent_connection_id).ok_or_else(|| { + SpawnerError::Spawn(format!( + "parent connection {parent_connection_id} not found" + )) + })?; + let pwd = parent + .state + .read() + .await + .working_dir + .as_ref() + .map(|p| p.to_string_lossy().to_string()); + ( + parent.emitter.clone(), + parent.owner_window_label.clone(), + pwd, + ) + }; + let effective_working_dir = working_dir + .or(parent_working_dir) + .map(|dir| { + std::fs::canonicalize(&dir) + .map(|path| path.to_string_lossy().to_string()) + .map_err(|error| { + SpawnerError::Spawn(format!( + "cannot resolve working directory {dir}: {error}" + )) + }) + }) + .transpose()?; + let runtime_env = crate::commands::acp::build_session_runtime_env( + &self.db, + agent_type, + None, + self.data_dir.as_path(), + ) + .await + .map_err(|e| SpawnerError::Spawn(e.to_string()))?; + let (session_id, policy) = match resume_binding.as_ref() { + Some(binding) => ( + Some(binding.external_session_id.clone()), + crate::acp::connection::SessionRecoveryPolicy::Strict, + ), + None => ( + None, + crate::acp::connection::SessionRecoveryPolicy::BestEffort, + ), + }; + self.manager + .spawn_delegation_agent( + agent_type, + effective_working_dir, + session_id, + runtime_env, + owner_window, + emitter, + preferred_mode_id, + preferred_config_values, + policy, + task_id, + ) + .await + .map_err(|e| match e { + AcpError::SessionBusy(message) => SpawnerError::Busy(message), + other => SpawnerError::Spawn(other.to_string()), + }) + } + async fn send_prompt_linked_for_delegation( &self, conn_id: &str, task: String, link: crate::acp::delegation::spawner::DelegationLink, - ) -> Result { - use crate::acp::delegation::spawner::SpawnerError; + ) -> Result< + crate::acp::delegation::spawner::DelegationDispatch, + crate::acp::delegation::spawner::SpawnerError, + > { + use crate::acp::delegation::spawner::{DelegationDispatch, SpawnerError}; // The child has no caller-supplied conversation_id (it's brand new). // folder_id must be None too — the manager's create-new-row branch // requires folder_id, which we resolve from the child's working_dir @@ -3908,35 +4419,172 @@ impl crate::acp::delegation::spawner::ConnectionSpawner for ConnectionManagerSpa .await .map_err(|e| SpawnerError::Send(format!("ensure_folder_for_path: {e}")))?; - let result = self + let Some(admission) = link.admission.clone() else { + let result = self + .manager + .send_prompt_linked( + &self.db, + conn_id, + vec![PromptInputBlock::Text { text: task }], + Some(folder.id), + None, + Some(link), + ) + .await + .map_err(|e| SpawnerError::Send(e.to_string()))?; + return result.map(DelegationDispatch::Started).ok_or_else(|| { + SpawnerError::Send("delegation did not bind a conversation".into()) + }); + }; + + let (agent_type, state, fingerprint) = { + let conns = self.manager.connections.lock().await; + let conn = conns + .get(conn_id) + .ok_or_else(|| SpawnerError::Send(format!("child {conn_id} not found")))?; + ( + conn.agent_type, + conn.state.clone(), + conn.config_fingerprint.clone(), + ) + }; + let child_conversation_id = if let Some(binding) = admission.resume_binding.as_ref() { + binding.child_conversation_id + } else { + let title = task + .lines() + .map(str::trim) + .find(|line| !line.is_empty()) + .map(|line| line.chars().take(80).collect()); + crate::db::service::conversation_service::create_with_delegation( + &self.db.conn, + folder.id, + agent_type, + title, + None, + Some(link.clone()), + ) + .await + .map_err(|e| SpawnerError::Send(e.to_string()))? + .id + }; + let resume_binding = match admission.resume_binding.clone() { + Some(binding) => binding, + None => { + let snapshot = state.read().await; + let effective_config_values = snapshot + .config_options + .as_deref() + .unwrap_or_default() + .iter() + .map(|option| { + let value = match &option.kind { + SessionConfigKindInfo::Select(select) => select.current_value.clone(), + SessionConfigKindInfo::Boolean(boolean) => { + boolean.current_value.to_string() + } + }; + (option.id.clone(), value) + }) + .collect(); + crate::db::service::delegation_task_service::ResumeBinding { + agent_type, + external_session_id: snapshot.external_id.clone().ok_or_else(|| { + SpawnerError::Send("child has no external session id".into()) + })?, + child_conversation_id, + working_dir: snapshot + .working_dir + .as_ref() + .ok_or_else(|| SpawnerError::Send("child has no working directory".into()))? + .to_string_lossy() + .to_string(), + preferred_mode_id: snapshot.current_mode.clone(), + preferred_config_values: effective_config_values, + config_fingerprint: fingerprint, + } + } + }; + let input = crate::db::service::delegation_task_service::AdmissionInput { + task_id: link.delegation_call_id.clone(), + parent_conversation_id: link.parent_conversation_id, + child_conversation_id, + source_task_id: admission.source_task_id, + task: admission.task, + requested_working_dir: admission.requested_working_dir, + resume_binding, + }; + let admission_result = if input.source_task_id.is_some() { + crate::db::service::delegation_task_service::admit_continuation(&self.db.conn, input) + .await + } else { + crate::db::service::delegation_task_service::admit(&self.db.conn, input).await + } + .map_err(|e| SpawnerError::Send(e.to_string()))?; + match admission_result { + crate::db::service::delegation_task_service::AdmissionResult::Existing { entry } => { + return Ok(DelegationDispatch::Existing(entry.report)); + } + crate::db::service::delegation_task_service::AdmissionResult::Conflict { + next_task_id, + reason, + } => { + return Ok(DelegationDispatch::Conflict { + next_task_id, + reason, + }); + } + crate::db::service::delegation_task_service::AdmissionResult::New { .. } => {} + } + let send_result = self .manager .send_prompt_linked( &self.db, conn_id, vec![PromptInputBlock::Text { text: task }], Some(folder.id), - None, - Some(link), + Some(child_conversation_id), + Some(link.clone()), + ) + .await; + if let Err(error) = send_result { + let report = crate::acp::delegation::types::DelegationTaskReport { + task_id: Some(link.delegation_call_id.clone()), + status: crate::acp::delegation::types::TaskStatus::Failed, + child_conversation_id: Some(child_conversation_id), + agent_type: Some(agent_type), + text: None, + error_code: Some("spawn_failed".into()), + message: Some(error.to_string()), + duration_ms: Some(0), + blocked_on: None, + }; + crate::db::service::delegation_task_service::finish( + &self.db.conn, + link.parent_conversation_id, + &link.delegation_call_id, + &report, ) .await .map_err(|e| SpawnerError::Send(e.to_string()))?; - result.ok_or_else(|| { - SpawnerError::Send( - "send_prompt_linked succeeded but no conversation_id was bound".into(), - ) - }) + return Ok(DelegationDispatch::Failed(report)); + } + Ok(DelegationDispatch::Started(child_conversation_id)) } async fn spawn_for_resume( &self, parent_connection_id: &str, + task_id: &str, agent_type: AgentType, working_dir: Option, external_session_id: &str, preferred_mode_id: Option, preferred_config_values: BTreeMap, - ) -> Result - { + ) -> Result< + crate::acp::delegation::spawner::ResumedSpawn, + crate::acp::delegation::spawner::SpawnerError, + > { use crate::acp::delegation::spawner::{ResumedSpawn, SpawnerError}; // Same parent inheritance as `spawn` — a resumed child whose emitter is // wired to a different broadcaster would stream to nobody. @@ -3970,31 +4618,12 @@ impl crate::acp::delegation::spawner::ConnectionSpawner for ConnectionManagerSpa .await .map_err(|e| SpawnerError::Spawn(e.to_string()))?; - // Detect dedup reuse BEFORE spawning, with the SAME lookup - // `spawn_agent` runs at its own entry: a live connection for this - // (agent, working_dir, session_id) — e.g. the user has the canceled - // child session open in a tab — makes `spawn_agent` return that - // connection instead of creating one. The broker must know, because - // its failure teardown may only disconnect a connection this call - // actually created. A connection appearing in the pre-check→spawn - // window is missed, but that window is milliseconds and a misfire - // additionally requires the send itself to fail. - let working_dir_path = effective_working_dir.as_ref().map(std::path::PathBuf::from); - let pre_existing = self - .manager - .find_connection_for_reuse( - agent_type, - working_dir_path.as_ref(), - Some(external_session_id), - ) - .await; - - // `session_id = Some(..)` is the whole difference vs `spawn`: the - // connection loads the child's prior agent session (and its context) - // instead of minting a fresh one. + // A legacy same-id resume is still broker-owned. Carry the immutable + // task id through the connection so late lifecycle events cannot infer + // execution identity from the mutable child row. let connection_id = self .manager - .spawn_agent( + .spawn_delegation_agent( agent_type, effective_working_dir, Some(external_session_id.to_string()), @@ -4003,13 +4632,14 @@ impl crate::acp::delegation::spawner::ConnectionSpawner for ConnectionManagerSpa emitter, preferred_mode_id, preferred_config_values, + crate::acp::connection::SessionRecoveryPolicy::Strict, + task_id.to_string(), ) .await .map_err(|e| SpawnerError::Spawn(e.to_string()))?; - let reused = pre_existing.as_deref() == Some(connection_id.as_str()); Ok(ResumedSpawn { connection_id, - reused, + reused: false, }) } @@ -4019,13 +4649,11 @@ impl crate::acp::delegation::spawner::ConnectionSpawner for ConnectionManagerSpa prompt: String, folder_id: i32, child_conversation_id: i32, + link: crate::acp::delegation::spawner::DelegationLink, ) -> Result<(), crate::acp::delegation::spawner::SpawnerError> { use crate::acp::delegation::spawner::SpawnerError; - // Adopt the child's EXISTING row (caller-supplied path) — no delegation - // link: the row already carries parent_id / parent_tool_use_id / - // delegation_call_id from the original delegation, and - // `send_prompt_linked` rejects a link combined with an explicit - // conversation_id precisely because adopted rows own their linkage. + // Adopt the existing row while explicitly presenting the immutable + // execution identity required by the broker-owned connection guard. self.manager .send_prompt_linked( &self.db, @@ -4033,7 +4661,7 @@ impl crate::acp::delegation::spawner::ConnectionSpawner for ConnectionManagerSpa vec![PromptInputBlock::Text { text: prompt }], Some(folder_id), Some(child_conversation_id), - None, + Some(link), ) .await .map(|_| ()) @@ -4099,10 +4727,7 @@ pub struct ConnectionManagerFeedbackLookup { #[async_trait::async_trait] impl SessionFeedbackAccess for ConnectionManagerFeedbackLookup { - async fn read_pending_feedback( - &self, - parent_connection_id: &str, - ) -> Vec { + async fn read_pending_feedback(&self, parent_connection_id: &str) -> Vec { self.manager .read_pending_feedback(parent_connection_id) .await @@ -4367,7 +4992,116 @@ mod tests { config_fingerprint: String::new(), last_observed_fingerprint: String::new(), child_pid: Arc::new(std::sync::atomic::AtomicU32::new(0)), + requested_session_id: None, + delegation_task_id: None, + driver_cancel: tokio_util::sync::CancellationToken::new(), + } + } + + #[tokio::test] + async fn delegation_disconnect_aborts_driver_and_keeps_slot_until_reap() { + let mgr = ConnectionManager::new(); + let mut conn = fake_connection("delegated", None); + conn.delegation_task_id = Some("task-1".into()); + let cancel = conn.driver_cancel.clone(); + mgr.connections + .lock() + .await + .insert("delegated".into(), conn); + + let result = mgr + .request_delegation_disconnect("delegated", "task-1") + .await; + assert!(matches!(result, Ok(()) | Err(AcpError::ProcessExited))); + assert!(cancel.is_cancelled()); + assert!(mgr.connections.lock().await.contains_key("delegated")); + } + + #[tokio::test] + async fn resumed_human_teardown_is_not_retained_as_a_delegation() { + let mgr = ConnectionManager::new(); + + let mut direct = fake_connection("human-direct", None); + direct.requested_session_id = Some("session-direct".into()); + let direct_cancel = direct.driver_cancel.clone(); + mgr.connections + .lock() + .await + .insert("human-direct".into(), direct); + + mgr.disconnect("human-direct").await.unwrap(); + assert!(!direct_cancel.is_cancelled()); + assert!(!mgr.connections.lock().await.contains_key("human-direct")); + + let mut window = fake_connection("human-window", None); + window.requested_session_id = Some("session-window".into()); + let window_cancel = window.driver_cancel.clone(); + mgr.connections + .lock() + .await + .insert("human-window".into(), window); + + assert_eq!(mgr.disconnect_by_owner_window("test-window").await, 1); + assert!(!window_cancel.is_cancelled()); + assert!(!mgr.connections.lock().await.contains_key("human-window")); + assert_eq!(mgr.draining.lock().await.len(), 2); + } + + #[tokio::test] + async fn strict_delegation_resume_reports_live_human_session_as_busy() { + let mgr = ConnectionManager::new(); + let mut conn = fake_connection("human", None); + conn.requested_session_id = Some("session-1".into()); + mgr.connections.lock().await.insert("human".into(), conn); + + let error = mgr + .spawn_delegation_agent( + AgentType::ClaudeCode, + None, + Some("session-1".into()), + BTreeMap::new(), + "test-window".into(), + EventEmitter::Noop, + None, + BTreeMap::new(), + SessionRecoveryPolicy::Strict, + "task-1".into(), + ) + .await + .unwrap_err(); + assert!(matches!(error, AcpError::SessionBusy(_))); + assert!(mgr.connections.lock().await.contains_key("human")); + } + + #[tokio::test] + async fn strict_resume_allows_mode_and_config_drift_after_identity_matches() { + let mgr = ConnectionManager::new(); + let mut conn = fake_connection("continued", None); + conn.delegation_task_id = Some("task-2".into()); + { + let mut state = conn.state.write().await; + state.working_dir = Some(PathBuf::from("/workspace/project")); + state.external_id = Some("session-2".into()); + state.current_mode = Some("new-mode".into()); + state.config_options = None; + state.selectors_ready = true; } + mgr.connections + .lock() + .await + .insert("continued".into(), conn); + + mgr.wait_for_strict_resume_ready( + "continued", + "task-2", + AgentType::ClaudeCode, + Some(&PathBuf::from("/workspace/project")), + "session-2", + Some("old-mode"), + &BTreeMap::from([("model".into(), "old-model".into())]), + ) + .await + .expect("mode and config are preferences, not session identity"); } /// Spawn a two-level process tree: `sh` (the stand-in for the agent CLI) @@ -4417,6 +5151,58 @@ mod tests { false } + /// A normal tab teardown removes a resumed session immediately, but its + /// process can still be exiting. Strict delegation must settle that exact + /// session before it is allowed to start a second writer for it. + #[cfg(unix)] + #[tokio::test] + async fn strict_resume_settles_a_matching_draining_session() { + let dir = tempfile::tempdir().expect("tempdir"); + let (mut child, gpid) = spawn_process_tree(&dir.path().join("g.pid")).await; + + let mgr = ConnectionManager::new(); + let mut conn = fake_connection("human", None); + conn.requested_session_id = Some("session-1".into()); + conn.child_pid + .store(child.id(), std::sync::atomic::Ordering::SeqCst); + let pid_cell = Arc::clone(&conn.child_pid); + mgr.connections.lock().await.insert("human".into(), conn); + mgr.disconnect("human").await.unwrap(); + + let reaper = tokio::task::spawn_blocking(move || { + let _ = child.wait(); + pid_cell.store(0, std::sync::atomic::Ordering::SeqCst); + }); + + assert!( + mgr.settle_draining_session(AgentType::ClaudeCode, "session-1") + .await + ); + assert!( + wait_until_dead(gpid).await, + "strict resume left the previous session process alive" + ); + reaper.await.unwrap(); + } + + #[tokio::test] + async fn strict_resume_refuses_a_drainer_that_has_not_published_its_pid() { + let mgr = ConnectionManager::new(); + let mut conn = fake_connection("human-connecting", None); + conn.requested_session_id = Some("session-connecting".into()); + mgr.connections + .lock() + .await + .insert("human-connecting".into(), conn); + mgr.disconnect("human-connecting").await.unwrap(); + + assert!( + !mgr.settle_draining_session(AgentType::ClaudeCode, "session-connecting") + .await, + "an unpublished pid is not proof that the previous process is gone" + ); + } + #[cfg(unix)] fn is_alive(pid: i32) -> bool { // SAFETY: signal 0 only probes for existence; it sends no signal. @@ -4822,6 +5608,9 @@ mod tests { config_fingerprint: String::new(), last_observed_fingerprint: String::new(), child_pid: Arc::new(std::sync::atomic::AtomicU32::new(0)), + requested_session_id: None, + delegation_task_id: None, + driver_cancel: tokio_util::sync::CancellationToken::new(), }; mgr.connections .lock() @@ -4851,8 +5640,9 @@ mod tests { mut rx: tokio::sync::mpsc::Receiver, expected: GoalControlAction, landed: bool, - ) -> tokio::task::JoinHandle> - { + ) -> tokio::task::JoinHandle< + tokio::sync::mpsc::Receiver, + > { tokio::spawn(async move { match rx.recv().await.expect("goal control enqueued") { ConnectionCommand::GoalControl { action, reply } => { @@ -5715,6 +6505,9 @@ mod tests { config_fingerprint: String::new(), last_observed_fingerprint: String::new(), child_pid: Arc::new(std::sync::atomic::AtomicU32::new(0)), + requested_session_id: None, + delegation_task_id: None, + driver_cancel: tokio_util::sync::CancellationToken::new(), }; let mgr = ConnectionManager::new(); mgr.connections @@ -5967,6 +6760,7 @@ mod tests { parent_conversation_id: parent.id, parent_tool_use_id: "tu-1".into(), delegation_call_id: "call-1".into(), + admission: None, }), ) .await @@ -7079,7 +7873,6 @@ mod tests { // observe via the other. let key = SpawnDedupKey { agent_type: AgentType::ClaudeCode, - working_dir: Some(PathBuf::from("/tmp/dedup-test")), session_id: "ext-shared".into(), }; { @@ -7356,6 +8149,9 @@ mod tests { config_fingerprint: String::new(), last_observed_fingerprint: String::new(), child_pid: Arc::new(std::sync::atomic::AtomicU32::new(0)), + requested_session_id: None, + delegation_task_id: None, + driver_cancel: tokio_util::sync::CancellationToken::new(), }; let mgr = Arc::new(ConnectionManager::new()); { @@ -8027,6 +8823,9 @@ mod tests { config_fingerprint: String::new(), last_observed_fingerprint: String::new(), child_pid: Arc::new(std::sync::atomic::AtomicU32::new(0)), + requested_session_id: None, + delegation_task_id: None, + driver_cancel: tokio_util::sync::CancellationToken::new(), }; let mgr = ConnectionManager::new(); { @@ -9342,5 +10141,4 @@ mod tests { // Commit on a missing connection is a safe no-op. mgr.commit_feedback_delivered("nope", vec!["x".into()]).await; } - } diff --git a/src-tauri/src/app_state.rs b/src-tauri/src/app_state.rs index 7777e9c0e2..eab66f70fa 100644 --- a/src-tauri/src/app_state.rs +++ b/src-tauri/src/app_state.rs @@ -97,6 +97,18 @@ pub fn default_chat_channel_manager() -> ChatChannelManager { ChatChannelManager::new() } +/// A restarted process owns none of the previous process's delegation children. +/// Reconcile their durable rows before the listener can accept new work. +pub async fn reconcile_interrupted_delegations(conn: &sea_orm::DatabaseConnection) { + match crate::db::service::delegation_task_service::boot_reconcile_interrupted(conn).await { + Ok(n) if n > 0 => { + tracing::info!("[delegation] boot reconcile settled {n} interrupted task(s)") + } + Ok(_) => {} + Err(error) => tracing::error!(%error, "[delegation] boot reconcile failed"), + } +} + /// Build the delegation broker + token registry + per-process UDS socket /// path. Shared between codeg-server bootstrap and the Tauri `setup` block /// so both modes apply identical depth limit + timeout defaults. @@ -147,7 +159,8 @@ pub fn build_delegation_stack( db: db_arc.clone(), }) as Arc; - let status_lookup = Arc::new(DbChildStatusLookup { db: db_arc }) as Arc; + let status_lookup = + Arc::new(DbChildStatusLookup { db: db_arc.clone() }) as Arc; let meta_writer = Arc::new(ConnectionManagerMetaWriter { manager: cm_arc.clone(), }) as Arc; @@ -159,6 +172,7 @@ pub fn build_delegation_stack( let broker = Arc::new( DelegationBroker::with_writers(spawner, depth_lookup, meta_writer, event_emitter) .with_status_lookup(status_lookup) + .with_ledger(db_arc) .with_live_reply_lookup(live_reply_lookup), ); let tokens = Arc::new(TokenRegistry::default()); diff --git a/src-tauri/src/bin/codeg_server.rs b/src-tauri/src/bin/codeg_server.rs index 71c5052b5e..a2897161bd 100644 --- a/src-tauri/src/bin/codeg_server.rs +++ b/src-tauri/src/bin/codeg_server.rs @@ -192,6 +192,7 @@ async fn async_main() -> ExitCode { let db = codeg_lib::db::init_database(&data_dir, app_version) .await .expect("Failed to initialize database"); + codeg_lib::app_state::reconcile_interrupted_delegations(&db.conn).await; // Logging phase 2: override the default level from the persisted // `logging.level` now that the DB is open. Phase 3 (wiring the emitter) diff --git a/src-tauri/src/commands/conversations.rs b/src-tauri/src/commands/conversations.rs index 1ebf890d62..d9dde10484 100644 --- a/src-tauri/src/commands/conversations.rs +++ b/src-tauri/src/commands/conversations.rs @@ -1155,12 +1155,38 @@ fn build_historical_delegation_meta(child: &DbConversationSummary) -> serde_json if let Some(title) = child.title.as_deref().map(str::trim).filter(|t| !t.is_empty()) { obj.insert( "task_preview".into(), - serde_json::Value::String(title.into()), + serde_json::Value::String(crate::acp::delegation::task_preview(title)), ); } serde_json::Value::Object(obj) } +fn build_ledger_delegation_meta( + entry: &crate::db::service::delegation_task_service::TaskLedgerEntry, +) -> serde_json::Value { + let mut value = serde_json::to_value(&entry.report).unwrap_or_default(); + if let Some(obj) = value.as_object_mut() { + let interrupted = entry.report.status + == crate::acp::delegation::types::TaskStatus::Unknown + && entry.report.error_code.as_deref() == Some("interrupted"); + if entry.report.status == crate::acp::delegation::types::TaskStatus::Canceled + || interrupted + { + // Delegation-card metadata has a closed status vocabulary; its + // failure detail still distinguishes cancellation/interruption by + // error_code. The broker uses `unknown` for an orphaned durable + // run, but the card parser deliberately has no `unknown` lifecycle + // state, so render that known interruption as a closed failure. + obj.insert("status".into(), serde_json::Value::String("failed".into())); + } + obj.insert( + "task_preview".into(), + serde_json::Value::String(crate::acp::delegation::task_preview(&entry.task)), + ); + } + value +} + /// The broker-minted task id a `delegate_to_agent` result announces. Codex /// persists the ack as prose (`Delegation successful. task_id=. Call /// get_delegation_status …`); other hosts return `{"task_id":""}` — both @@ -1269,8 +1295,12 @@ fn parse_resume_task_id(input: &str) -> Option { /// an earlier turn). Without this the resumed card would be frozen at the /// `running` its ack reported, forever — the child's real outcome landed on the /// DB row, not on the resume result. -fn inject_delegation_meta(turns: &mut [MessageTurn], children: &[DbConversationSummary]) { - if children.is_empty() { +fn inject_delegation_meta( + turns: &mut [MessageTurn], + children: &[DbConversationSummary], + ledger: &[crate::db::service::delegation_task_service::TaskLedgerEntry], +) { + if children.is_empty() && ledger.is_empty() { return; } let by_parent_tool_use_id: HashMap<&str, &DbConversationSummary> = children @@ -1281,11 +1311,15 @@ fn inject_delegation_meta(turns: &mut [MessageTurn], children: &[DbConversationS .iter() .filter_map(|c| c.delegation_call_id.as_deref().map(|id| (id, c))) .collect(); + let ledger_by_task_id: HashMap<&str, _> = ledger + .iter() + .map(|entry| (entry.task_id.as_str(), entry)) + .collect(); // The task id lives on the call's RESULT, which the parsers emit as a // separate block (usually a later turn), so collect it up front. let mut task_id_by_call: HashMap = HashMap::new(); - if !by_task_id.is_empty() { + if !by_task_id.is_empty() || !ledger_by_task_id.is_empty() { for turn in turns.iter() { for block in turn.blocks.iter() { if let ContentBlock::ToolResult { @@ -1317,8 +1351,23 @@ fn inject_delegation_meta(turns: &mut [MessageTurn], children: &[DbConversationS if meta.is_some() { continue; } + let is_delegate = tool_name.contains("delegate_to_agent"); + let is_resume = tool_name.contains("resume_delegation"); + if !is_delegate && !is_resume { + continue; + } + if let Some(entry) = tool_use_id + .as_deref() + .and_then(|tu| task_id_by_call.get(tu)) + .and_then(|task_id| ledger_by_task_id.get(task_id.as_str())) + { + *meta = Some(serde_json::json!({ + "codeg.delegation": build_ledger_delegation_meta(entry), + })); + continue; + } let child: Option<&DbConversationSummary> = - if tool_name.contains("delegate_to_agent") { + if is_delegate { tool_use_id.as_deref().and_then(|tu| { by_parent_tool_use_id .get(tu) @@ -1329,13 +1378,13 @@ fn inject_delegation_meta(turns: &mut [MessageTurn], children: &[DbConversationS }) .copied() }) - } else if tool_name.contains("resume_delegation") { + } else if is_resume { input_preview .as_deref() .and_then(parse_resume_task_id) .and_then(|task_id| by_task_id.get(task_id.as_str()).copied()) } else { - continue; + unreachable!("delegation tool was checked above") }; if let Some(child) = child { *meta = Some(serde_json::json!({ @@ -1356,6 +1405,14 @@ fn inject_delegation_meta(turns: &mut [MessageTurn], children: &[DbConversationS pub async fn get_folder_conversation_core( conn: &sea_orm::DatabaseConnection, conversation_id: i32, +) -> Result<(DbConversationDetail, Option), AppCommandError> { + get_folder_conversation_core_with_broker(conn, conversation_id, None).await +} + +async fn get_folder_conversation_core_with_broker( + conn: &sea_orm::DatabaseConnection, + conversation_id: i32, + broker: Option<&crate::acp::delegation::broker::DelegationBroker>, ) -> Result<(DbConversationDetail, Option), AppCommandError> { let summary = conversation_service::get_by_id(conn, conversation_id) .await @@ -1495,7 +1552,19 @@ pub async fn get_folder_conversation_core( let children = conversation_service::list_children(conn, conversation_id) .await .unwrap_or_default(); - inject_delegation_meta(&mut turns, &children); + let mut ledger = crate::db::service::delegation_task_service::list_for_parent( + conn, + conversation_id, + ) + .await + .unwrap_or_default(); + if let Some(broker) = broker { + for entry in &mut ledger { + let projected = broker.project_ledger_report(entry).await; + entry.report = projected; + } + } + inject_delegation_meta(&mut turns, &children, &ledger); Ok(( DbConversationDetail { @@ -1716,12 +1785,14 @@ fn apply_turn_window( pub async fn get_folder_conversation_with_live_core( conn: &sea_orm::DatabaseConnection, manager: &crate::acp::manager::ConnectionManager, + broker: &crate::acp::delegation::broker::DelegationBroker, chat_channel_manager: &crate::chat_channel::manager::ChatChannelManager, emitter: &EventEmitter, conversation_id: i32, window: Option, ) -> Result { - let (mut detail, parsed_title) = get_folder_conversation_core(conn, conversation_id).await?; + let (mut detail, parsed_title) = + get_folder_conversation_core_with_broker(conn, conversation_id, Some(broker)).await?; // Per-turn auto-title backfill. The parse `get_folder_conversation_core` // just did already produced the session-file title; adopt it (and broadcast @@ -1798,12 +1869,14 @@ pub async fn get_folder_conversation_with_live_core( /// auto-title refresh, no live correlation, no sidebar events. pub async fn get_folder_conversation_turns_core( conn: &sea_orm::DatabaseConnection, + broker: Option<&crate::acp::delegation::broker::DelegationBroker>, conversation_id: i32, before_index: usize, limit: usize, ) -> Result { use crate::commands::turn_window; - let (detail, _parsed_title) = get_folder_conversation_core(conn, conversation_id).await?; + let (detail, _parsed_title) = + get_folder_conversation_core_with_broker(conn, conversation_id, broker).await?; let turns = detail.turns; let (start, end) = turn_window::resolve_page_bounds(&turns, before_index, limit); let meta = turn_window::window_meta(&turns, start); @@ -1831,9 +1904,14 @@ pub async fn get_folder_conversation( from_index: Option, ) -> Result { let window = resolve_turn_window_req(tail_turns, from_index)?; + let broker = app + .state::>() + .inner() + .clone(); get_folder_conversation_with_live_core( &db.conn, &manager, + &broker, &chat_channel_manager, &EventEmitter::Tauri(app), conversation_id, @@ -1846,11 +1924,19 @@ pub async fn get_folder_conversation( #[cfg_attr(feature = "tauri-runtime", tauri::command)] pub async fn get_folder_conversation_turns( db: tauri::State<'_, AppDatabase>, + broker: tauri::State<'_, std::sync::Arc>, conversation_id: i32, before_index: usize, limit: usize, ) -> Result { - get_folder_conversation_turns_core(&db.conn, conversation_id, before_index, limit).await + get_folder_conversation_turns_core( + &db.conn, + Some(&broker), + conversation_id, + before_index, + limit, + ) + .await } /// Emit a `conversation://changed` Upsert for `conversation_id` so every @@ -3025,7 +3111,7 @@ mod tests { "mcp__codeg-mcp__delegate_to_agent", )]; let children = vec![summary_child(42, "tu-1", "completed")]; - inject_delegation_meta(&mut turns, &children); + inject_delegation_meta(&mut turns, &children, &[]); let meta = first_block_meta(&turns[0]).expect("meta should be set"); let inner = meta.get("codeg.delegation").expect("codeg.delegation key"); assert_eq!(inner["status"], "completed"); @@ -3074,7 +3160,7 @@ mod tests { let mut child = summary_child(2890, "exec-0fb6db94-3042-4cc4-b492-2edd1804c1fa", "completed"); child.delegation_call_id = Some("8ff4c14c-740c-4482-b758-8f2091f97063".into()); - inject_delegation_meta(&mut turns, &[child]); + inject_delegation_meta(&mut turns, &[child], &[]); let inner = first_block_meta(&turns[0]) .and_then(|m| m.get("codeg.delegation").cloned()) @@ -3082,6 +3168,140 @@ mod tests { assert_eq!(inner["child_conversation_id"], 2890); } + #[test] + fn ledger_restores_an_old_round_without_marking_a_status_call() { + use crate::acp::delegation::types::{DelegationTaskReport, TaskStatus}; + use crate::db::service::delegation_task_service::{ResumeBinding, TaskLedgerEntry}; + + let mut turns = vec![ + tool_use_turn(Some("exec-r1"), "mcp__codeg_mcp__delegate_to_agent"), + tool_result_turn("exec-r1", "Delegation successful. task_id=task-r1."), + tool_use_turn(Some("exec-status"), "mcp__codeg_mcp__get_delegation_status"), + tool_result_turn("exec-status", r#"{"task_id":"task-r1"}"#), + ]; + let mut child = summary_child(42, "exec-r3", "in_progress"); + child.delegation_call_id = Some("task-r3".into()); + let now = chrono::Utc::now(); + let ledger = TaskLedgerEntry { + id: 1, + task_id: "task-r1".into(), + parent_conversation_id: 1, + child_conversation_id: 42, + source_task_id: None, + task: "round one".into(), + requested_working_dir: None, + resume_binding: ResumeBinding { + agent_type: AgentType::Codex, + external_session_id: "session-1".into(), + child_conversation_id: 42, + working_dir: "/tmp".into(), + preferred_mode_id: None, + preferred_config_values: Default::default(), + config_fingerprint: "fp".into(), + }, + status: TaskStatus::Canceled, + released: true, + report: DelegationTaskReport { + task_id: Some("task-r1".into()), + status: TaskStatus::Canceled, + child_conversation_id: Some(42), + agent_type: Some(AgentType::Codex), + text: None, + error_code: Some("canceled".into()), + message: Some("Canceled by user".into()), + duration_ms: Some(12), + blocked_on: None, + }, + created_at: now, + updated_at: now, + }; + + inject_delegation_meta(&mut turns, &[child], &[ledger]); + let meta = first_block_meta(&turns[0]) + .and_then(|value| value.get("codeg.delegation")) + .expect("old ledger round is restored by task id"); + assert_eq!(meta["task_id"], "task-r1"); + assert_eq!(meta["child_conversation_id"], 42); + assert_eq!(meta["status"], "failed"); + assert_eq!(meta["error_code"], "canceled"); + assert!( + first_block_meta(&turns[2]).is_none(), + "status polling remains a status card" + ); + } + + #[test] + fn interrupted_ledger_report_closes_the_history_card_truthfully() { + use crate::acp::delegation::types::{DelegationTaskReport, TaskStatus}; + use crate::db::service::delegation_task_service::{ResumeBinding, TaskLedgerEntry}; + + let mut turns = vec![ + tool_use_turn(Some("call-orphan"), "mcp__codeg_mcp__delegate_to_agent"), + tool_result_turn( + "call-orphan", + "Delegation successful. task_id=task-orphan.", + ), + ]; + let now = chrono::Utc::now(); + let ledger = TaskLedgerEntry { + id: 1, + task_id: "task-orphan".into(), + parent_conversation_id: 1, + child_conversation_id: 42, + source_task_id: None, + task: "work interrupted by restart".into(), + requested_working_dir: None, + resume_binding: ResumeBinding { + agent_type: AgentType::Codex, + external_session_id: "session-1".into(), + child_conversation_id: 42, + working_dir: "/tmp".into(), + preferred_mode_id: None, + preferred_config_values: Default::default(), + config_fingerprint: "fp".into(), + }, + // The durable row remains running; only the broker-projected report + // says the process disappeared and its outcome is unknown. + status: TaskStatus::Running, + released: false, + report: DelegationTaskReport { + task_id: Some("task-orphan".into()), + status: TaskStatus::Unknown, + child_conversation_id: Some(42), + agent_type: Some(AgentType::Codex), + text: None, + error_code: Some("interrupted".into()), + message: Some( + "The application stopped while this delegation was running; its outcome is unknown." + .into(), + ), + duration_ms: None, + blocked_on: None, + }, + created_at: now, + updated_at: now, + }; + + inject_delegation_meta(&mut turns, &[], std::slice::from_ref(&ledger)); + + let meta = first_block_meta(&turns[0]) + .and_then(|value| value.get("codeg.delegation")) + .expect("orphaned ledger row should still restore its card"); + assert_eq!(meta["status"], "failed"); + assert_eq!(meta["error_code"], "interrupted"); + assert_eq!(meta["task_preview"], "work interrupted by restart"); + assert_eq!(meta["child_conversation_id"], 42); + + let mut oversized = ledger; + oversized.task = "界".repeat(crate::acp::delegation::TASK_PREVIEW_CAP); + let preview = build_ledger_delegation_meta(&oversized)["task_preview"] + .as_str() + .unwrap() + .to_string(); + assert!(preview.len() <= crate::acp::delegation::TASK_PREVIEW_CAP); + assert!(preview.ends_with('…')); + } + #[test] fn inject_delegation_meta_does_not_bind_a_foreign_task_id() { let mut turns = vec![ @@ -3091,7 +3311,7 @@ mod tests { let mut child = summary_child(1, "exec-zzz", "completed"); child.delegation_call_id = Some("bbbb".into()); - inject_delegation_meta(&mut turns, &[child]); + inject_delegation_meta(&mut turns, &[child], &[]); assert!( first_block_meta(&turns[0]).is_none(), @@ -3115,7 +3335,7 @@ mod tests { child.delegation_call_id = Some("b0858712-9257".into()); child.title = Some("Build the /test4 sandbox page".into()); - inject_delegation_meta(&mut turns, &[child]); + inject_delegation_meta(&mut turns, &[child], &[]); let inner = first_block_meta(&turns[0]) .and_then(|m| m.get("codeg.delegation").cloned()) @@ -3138,7 +3358,7 @@ mod tests { let mut child = summary_child(9, "tu-x", "completed"); child.delegation_call_id = Some("bbbb".into()); - inject_delegation_meta(&mut turns, &[child]); + inject_delegation_meta(&mut turns, &[child], &[]); assert!( first_block_meta(&turns[0]).is_none(), @@ -3223,7 +3443,7 @@ mod tests { fn inject_delegation_meta_maps_in_progress_to_running() { let mut turns = vec![tool_use_turn(Some("tu-1"), "delegate_to_agent")]; let children = vec![summary_child(7, "tu-1", "in_progress")]; - inject_delegation_meta(&mut turns, &children); + inject_delegation_meta(&mut turns, &children, &[]); let inner = first_block_meta(&turns[0]) .unwrap() .get("codeg.delegation") @@ -3242,7 +3462,7 @@ mod tests { // "running" badge until the user reloads again. let mut turns = vec![tool_use_turn(Some("tu-1"), "delegate_to_agent")]; let children = vec![summary_child(11, "tu-1", "pending_review")]; - inject_delegation_meta(&mut turns, &children); + inject_delegation_meta(&mut turns, &children, &[]); let inner = first_block_meta(&turns[0]) .unwrap() .get("codeg.delegation") @@ -3261,7 +3481,7 @@ mod tests { // as user-cancel. Emit `failed` without a code instead. let mut turns = vec![tool_use_turn(Some("tu-1"), "delegate_to_agent")]; let children = vec![summary_child(9, "tu-1", "cancelled")]; - inject_delegation_meta(&mut turns, &children); + inject_delegation_meta(&mut turns, &children, &[]); let inner = first_block_meta(&turns[0]) .unwrap() .get("codeg.delegation") @@ -3277,7 +3497,7 @@ mod tests { fn inject_delegation_meta_skips_non_delegation_tool_calls() { let mut turns = vec![tool_use_turn(Some("tu-1"), "bash")]; let children = vec![summary_child(42, "tu-1", "completed")]; - inject_delegation_meta(&mut turns, &children); + inject_delegation_meta(&mut turns, &children, &[]); assert!( first_block_meta(&turns[0]).is_none(), "non-delegation tool_name must not get meta even on tool_use_id match" @@ -3288,7 +3508,7 @@ mod tests { fn inject_delegation_meta_skips_blocks_without_tool_use_id() { let mut turns = vec![tool_use_turn(None, "delegate_to_agent")]; let children = vec![summary_child(42, "tu-1", "completed")]; - inject_delegation_meta(&mut turns, &children); + inject_delegation_meta(&mut turns, &children, &[]); assert!(first_block_meta(&turns[0]).is_none()); } @@ -3315,7 +3535,7 @@ mod tests { agent_message_id: None, }]; let children = vec![summary_child(42, "tu-1", "completed")]; - inject_delegation_meta(&mut turns, &children); + inject_delegation_meta(&mut turns, &children, &[]); // The 999 (broker-written) survives — DB-derived 42 is not used here. let inner = first_block_meta(&turns[0]) .unwrap() @@ -3328,7 +3548,7 @@ mod tests { #[test] fn inject_delegation_meta_no_op_when_children_empty() { let mut turns = vec![tool_use_turn(Some("tu-1"), "delegate_to_agent")]; - inject_delegation_meta(&mut turns, &[]); + inject_delegation_meta(&mut turns, &[], &[]); assert!(first_block_meta(&turns[0]).is_none()); } @@ -3336,7 +3556,7 @@ mod tests { fn inject_delegation_meta_unmatched_tool_use_id_left_alone() { let mut turns = vec![tool_use_turn(Some("tu-other"), "delegate_to_agent")]; let children = vec![summary_child(42, "tu-1", "completed")]; - inject_delegation_meta(&mut turns, &children); + inject_delegation_meta(&mut turns, &children, &[]); assert!(first_block_meta(&turns[0]).is_none()); } @@ -3363,6 +3583,7 @@ mod tests { parent_conversation_id: parent_id, parent_tool_use_id: "tu-historical".into(), delegation_call_id: "call-historical".into(), + admission: None, }; conversation_service::create_with_delegation( &db.conn, @@ -4812,6 +5033,7 @@ mod tests { parent_conversation_id: parent_id, parent_tool_use_id: (*tool_use).into(), delegation_call_id: format!("call-{i}"), + admission: None, }; let child = conversation_service::create_with_delegation( &db.conn, @@ -5170,6 +5392,7 @@ mod tests { parent_conversation_id: parent_id, parent_tool_use_id: "tu-1".into(), delegation_call_id: "call-1".into(), + admission: None, }), ) .await @@ -5214,6 +5437,7 @@ mod tests { parent_conversation_id: parent_id, parent_tool_use_id: "tu-1".into(), delegation_call_id: "call-1".into(), + admission: None, }), ) .await @@ -6455,7 +6679,7 @@ mod tests { let conv_id = create_conversation_core(&db.conn, folder_id, AgentType::ClaudeCode, None) .await .expect("create conversation"); - let page = get_folder_conversation_turns_core(&db.conn, conv_id, 10, 5) + let page = get_folder_conversation_turns_core(&db.conn, None, conv_id, 10, 5) .await .expect("page fetch"); assert_eq!(page.turns_total, 0); diff --git a/src-tauri/src/commands/pet.rs b/src-tauri/src/commands/pet.rs index a7542cff62..9b792488ec 100644 --- a/src-tauri/src/commands/pet.rs +++ b/src-tauri/src/commands/pet.rs @@ -662,6 +662,7 @@ mod tests { parent_conversation_id: parent.id, parent_tool_use_id: "tu-1".into(), delegation_call_id: "call-1".into(), + admission: None, }), ) .await diff --git a/src-tauri/src/db/entities/delegation_task.rs b/src-tauri/src/db/entities/delegation_task.rs new file mode 100644 index 0000000000..c4dba13f75 --- /dev/null +++ b/src-tauri/src/db/entities/delegation_task.rs @@ -0,0 +1,34 @@ +use sea_orm::entity::prelude::*; + +/// Durable admission record for one ordinary delegation execution. +/// +/// Conversation rows are foreign-key parents. Soft deletion still hides +/// history from lookup, while physical deletion cascades the ledger row. +#[derive(Clone, Debug, PartialEq, DeriveEntityModel)] +#[sea_orm(table_name = "delegation_task")] +pub struct Model { + #[sea_orm(primary_key)] + pub id: i32, + #[sea_orm(unique)] + pub task_id: String, + pub parent_conversation_id: i32, + pub child_conversation_id: i32, + pub source_task_id: Option, + pub task: String, + pub requested_working_dir: Option, + /// Current execution status. The terminal report is immutable and lives in + /// `terminal_report`; this column is not derived from the child row. + pub status: String, + /// JSON snapshot of the exact terminal `DelegationTaskReport`. + pub terminal_report: Option, + /// JSON [`ResumeBinding`](crate::db::service::delegation_task_service::ResumeBinding). + pub resume_binding: String, + pub released: bool, + pub created_at: DateTimeUtc, + pub updated_at: DateTimeUtc, +} + +#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] +pub enum Relation {} + +impl ActiveModelBehavior for ActiveModel {} diff --git a/src-tauri/src/db/entities/mod.rs b/src-tauri/src/db/entities/mod.rs index 6d8bbe07ad..016d68f19b 100644 --- a/src-tauri/src/db/entities/mod.rs +++ b/src-tauri/src/db/entities/mod.rs @@ -9,6 +9,7 @@ pub mod chat_channel_sender_context; pub mod chat_channel_thread_binding; pub mod conversation; pub mod custom_agent; +pub mod delegation_task; pub mod folder; pub mod folder_command; pub mod folder_group; diff --git a/src-tauri/src/db/entities/prelude.rs b/src-tauri/src/db/entities/prelude.rs index b40272cba9..8538bf6931 100644 --- a/src-tauri/src/db/entities/prelude.rs +++ b/src-tauri/src/db/entities/prelude.rs @@ -11,6 +11,7 @@ pub use super::chat_channel_sender_context::Entity as ChatChannelSenderContext; pub use super::chat_channel_thread_binding::Entity as ChatChannelThreadBinding; pub use super::conversation::Entity as Conversation; pub use super::custom_agent::Entity as CustomAgent; +pub use super::delegation_task::Entity as DelegationTask; pub use super::folder::Entity as Folder; pub use super::folder_command::Entity as FolderCommand; pub use super::folder_group::Entity as FolderGroup; diff --git a/src-tauri/src/db/migration/m20260908_000001_delegation_task_ledger.rs b/src-tauri/src/db/migration/m20260908_000001_delegation_task_ledger.rs new file mode 100644 index 0000000000..68ed0a3ce9 --- /dev/null +++ b/src-tauri/src/db/migration/m20260908_000001_delegation_task_ledger.rs @@ -0,0 +1,394 @@ +use sea_orm_migration::prelude::*; + +#[derive(DeriveMigrationName)] +pub struct Migration; + +#[async_trait::async_trait] +impl MigrationTrait for Migration { + async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> { + manager + .create_table( + Table::create() + .table(DelegationTask::Table) + .if_not_exists() + .col( + ColumnDef::new(DelegationTask::Id) + .integer() + .not_null() + .auto_increment() + .primary_key(), + ) + .col( + ColumnDef::new(DelegationTask::TaskId) + .string() + .not_null() + .unique_key(), + ) + .col( + ColumnDef::new(DelegationTask::ParentConversationId) + .integer() + .not_null(), + ) + .col( + ColumnDef::new(DelegationTask::ChildConversationId) + .integer() + .not_null(), + ) + .col(ColumnDef::new(DelegationTask::SourceTaskId).string().null()) + .col(ColumnDef::new(DelegationTask::Task).text().not_null()) + .col( + ColumnDef::new(DelegationTask::RequestedWorkingDir) + .text() + .null(), + ) + .col(ColumnDef::new(DelegationTask::Status).string().not_null()) + .col(ColumnDef::new(DelegationTask::TerminalReport).text().null()) + .col( + ColumnDef::new(DelegationTask::ResumeBinding) + .text() + .not_null(), + ) + .col( + ColumnDef::new(DelegationTask::Released) + .boolean() + .not_null() + .default(false), + ) + .col( + ColumnDef::new(DelegationTask::CreatedAt) + .timestamp_with_time_zone() + .not_null(), + ) + .col( + ColumnDef::new(DelegationTask::UpdatedAt) + .timestamp_with_time_zone() + .not_null(), + ) + .foreign_key( + ForeignKey::create() + .name("fk_delegation_task_parent_conversation") + .from(DelegationTask::Table, DelegationTask::ParentConversationId) + .to(Conversation::Table, Conversation::Id) + .on_delete(ForeignKeyAction::Cascade) + .on_update(ForeignKeyAction::Cascade), + ) + .foreign_key( + ForeignKey::create() + .name("fk_delegation_task_child_conversation") + .from(DelegationTask::Table, DelegationTask::ChildConversationId) + .to(Conversation::Table, Conversation::Id) + .on_delete(ForeignKeyAction::Cascade) + .on_update(ForeignKeyAction::Cascade), + ) + .to_owned(), + ) + .await?; + + manager + .create_index( + Index::create() + .if_not_exists() + .name("idx_delegation_task_parent") + .table(DelegationTask::Table) + .col(DelegationTask::ParentConversationId) + .col(DelegationTask::UpdatedAt) + .to_owned(), + ) + .await?; + + manager + .create_index( + Index::create() + .if_not_exists() + .name("idx_delegation_task_source_unique") + .table(DelegationTask::Table) + .col(DelegationTask::SourceTaskId) + .unique() + .to_owned(), + ) + .await?; + + manager + .create_index( + Index::create() + .if_not_exists() + .name("idx_delegation_task_child") + .table(DelegationTask::Table) + .col(DelegationTask::ChildConversationId) + .to_owned(), + ) + .await + } + + async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> { + manager + .drop_table( + Table::drop() + .table(DelegationTask::Table) + .if_exists() + .to_owned(), + ) + .await + } +} + +#[derive(DeriveIden)] +enum DelegationTask { + Table, + Id, + TaskId, + ParentConversationId, + ChildConversationId, + SourceTaskId, + Task, + RequestedWorkingDir, + Status, + TerminalReport, + ResumeBinding, + Released, + CreatedAt, + UpdatedAt, +} + +#[derive(DeriveIden)] +enum Conversation { + Table, + Id, +} + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use sea_orm::{ConnectionTrait, Database, DbBackend, Statement}; + use sea_orm_migration::{MigratorTrait, SchemaManager}; + + use crate::acp::delegation::spawner::DelegationLink; + use crate::db::migration::Migrator; + use crate::db::service::{conversation_service, delegation_task_service, folder_service}; + use crate::models::AgentType; + + fn sql(statement: &str) -> Statement { + Statement::from_string(DbBackend::Sqlite, statement.to_owned()) + } + + async fn legacy_projection( + conn: &sea_orm::DatabaseConnection, + parent_id: i32, + completed_task_id: &str, + interrupted_task_id: &str, + ) -> serde_json::Value { + serde_json::json!({ + "parent": conversation_service::get_by_id(conn, parent_id) + .await + .expect("legacy parent"), + "children": conversation_service::list_children(conn, parent_id) + .await + .expect("legacy children"), + "completed_task_child": conversation_service::get_by_delegation_call_id( + conn, + completed_task_id, + ) + .await + .expect("legacy completed task child"), + "interrupted_task_child": conversation_service::get_by_delegation_call_id( + conn, + interrupted_task_id, + ) + .await + .expect("legacy interrupted task child"), + }) + } + + /// Reproduce an on-disk database at upstream/main's last schema before the + /// durable ledger existed. The migration must leave all legacy delegation + /// history readable exactly as before and make the new ledger usable. + #[tokio::test] + async fn upgrades_pre_ledger_database_without_rewriting_legacy_history() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("pre-ledger.db"); + let url = format!("sqlite:{}?mode=rwc", path.to_string_lossy()); + let conn = Database::connect(url.clone()) + .await + .expect("open pre-ledger db"); + conn.execute(sql("PRAGMA foreign_keys=ON;")) + .await + .expect("foreign keys"); + + let migrations = ::migrations(); + let ledger_idx = migrations + .iter() + .position(|migration| migration.name() == "m20260908_000001_delegation_task_ledger") + .expect("ledger migration is registered"); + Migrator::up(&conn, Some(ledger_idx as u32)) + .await + .expect("apply upstream pre-ledger schema"); + assert!( + !SchemaManager::new(&conn) + .has_table("delegation_task") + .await + .expect("inspect pre-ledger schema"), + "the fixture must start before the ledger table exists" + ); + + let folder = folder_service::add_folder(&conn, "/workspace/legacy-project") + .await + .expect("legacy folder") + .id; + let parent = conversation_service::create( + &conn, + folder, + AgentType::ClaudeCode, + Some("Legacy parent".into()), + Some("legacy-branch".into()), + ) + .await + .expect("legacy parent"); + let legacy_task_id = "legacy-delegation-call"; + let child = conversation_service::create_with_delegation( + &conn, + folder, + AgentType::Codex, + Some("Legacy completed child".into()), + Some("legacy-branch".into()), + Some(DelegationLink { + parent_conversation_id: parent.id, + parent_tool_use_id: "legacy-tool-use".into(), + delegation_call_id: legacy_task_id.into(), + admission: None, + }), + ) + .await + .expect("legacy child"); + conversation_service::bind_external_id(&conn, child.id, "legacy-session", &[]) + .await + .expect("legacy external session"); + conversation_service::update_status( + &conn, + child.id, + crate::db::entities::conversation::ConversationStatus::Completed, + ) + .await + .expect("legacy completed status"); + conn.execute(sql(&format!( + "UPDATE conversation SET message_count = 7 WHERE id = {}", + child.id + ))) + .await + .expect("legacy message history"); + + let interrupted_task_id = "legacy-interrupted-call"; + let interrupted_child = conversation_service::create_with_delegation( + &conn, + folder, + AgentType::Codex, + Some("Legacy interrupted child".into()), + Some("legacy-branch".into()), + Some(DelegationLink { + parent_conversation_id: parent.id, + parent_tool_use_id: "legacy-interrupted-tool-use".into(), + delegation_call_id: interrupted_task_id.into(), + admission: None, + }), + ) + .await + .expect("legacy interrupted child"); + conversation_service::bind_external_id( + &conn, + interrupted_child.id, + "legacy-interrupted-session", + &[], + ) + .await + .expect("legacy interrupted external session"); + + let before = legacy_projection(&conn, parent.id, legacy_task_id, interrupted_task_id).await; + conn.close().await.expect("close pre-ledger db"); + + // Production applies pending migrations on a fresh, single connection + // at startup. Reopen here so no connection-local schema cache from the + // fixture setup can make this easier than a real application upgrade. + let conn = Database::connect(url).await.expect("reopen for upgrade"); + conn.execute(sql("PRAGMA foreign_keys=ON;")) + .await + .expect("foreign keys after reopen"); + Migrator::up(&conn, Some(1)) + .await + .expect("apply ledger migration"); + assert!( + SchemaManager::new(&conn) + .has_table("delegation_task") + .await + .expect("inspect upgraded schema"), + "the upgrade must create the ledger table" + ); + assert_eq!( + legacy_projection(&conn, parent.id, legacy_task_id, interrupted_task_id).await, + before, + "adding the ledger must not rewrite legacy conversation history" + ); + for task_id in [legacy_task_id, interrupted_task_id] { + assert!( + delegation_task_service::lookup(&conn, parent.id, task_id) + .await + .expect("legacy ledger lookup") + .is_none(), + "the migration must not invent ledger facts for pre-ledger children" + ); + } + + let new_task_id = "post-upgrade-task"; + let new_child = conversation_service::create_with_delegation( + &conn, + folder, + AgentType::Codex, + Some("Post-upgrade child".into()), + None, + Some(DelegationLink { + parent_conversation_id: parent.id, + parent_tool_use_id: "post-upgrade-tool-use".into(), + delegation_call_id: new_task_id.into(), + admission: None, + }), + ) + .await + .expect("post-upgrade child"); + let binding = delegation_task_service::ResumeBinding { + agent_type: AgentType::Codex, + external_session_id: "post-upgrade-session".into(), + child_conversation_id: new_child.id, + working_dir: "/workspace/legacy-project".into(), + preferred_mode_id: Some("default".into()), + preferred_config_values: BTreeMap::new(), + config_fingerprint: "post-upgrade-fingerprint".into(), + }; + let admitted = delegation_task_service::admit( + &conn, + delegation_task_service::AdmissionInput { + task_id: new_task_id.into(), + parent_conversation_id: parent.id, + child_conversation_id: new_child.id, + source_task_id: None, + task: "Verify the upgraded ledger".into(), + requested_working_dir: None, + resume_binding: binding.clone(), + }, + ) + .await + .expect("admit post-upgrade task"); + assert!(matches!( + admitted, + delegation_task_service::AdmissionResult::New { .. } + )); + let entry = delegation_task_service::lookup(&conn, parent.id, new_task_id) + .await + .expect("post-upgrade lookup") + .expect("post-upgrade ledger entry"); + assert_eq!(entry.child_conversation_id, new_child.id); + assert_eq!(entry.resume_binding, binding); + assert_eq!( + entry.status, + crate::acp::delegation::types::TaskStatus::Running + ); + } +} diff --git a/src-tauri/src/db/migration/mod.rs b/src-tauri/src/db/migration/mod.rs index 54e642be74..817130c423 100644 --- a/src-tauri/src/db/migration/mod.rs +++ b/src-tauri/src/db/migration/mod.rs @@ -45,6 +45,7 @@ mod m20260829_000001_folder_group; mod m20260830_000001_canvas_node; mod m20260831_000001_canvas_node_group_grid; mod m20260907_000001_canvas_node_path; +mod m20260908_000001_delegation_task_ledger; pub struct Migrator; #[async_trait::async_trait] @@ -96,6 +97,7 @@ impl MigratorTrait for Migrator { Box::new(m20260830_000001_canvas_node::Migration), Box::new(m20260831_000001_canvas_node_group_grid::Migration), Box::new(m20260907_000001_canvas_node_path::Migration), + Box::new(m20260908_000001_delegation_task_ledger::Migration), ] } } diff --git a/src-tauri/src/db/service/conversation_service.rs b/src-tauri/src/db/service/conversation_service.rs index 08ec608838..4584486aef 100644 --- a/src-tauri/src/db/service/conversation_service.rs +++ b/src-tauri/src/db/service/conversation_service.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use chrono::Utc; use sea_orm::{ - ActiveModelTrait, ActiveValue::NotSet, ColumnTrait, DatabaseConnection, EntityTrait, + ActiveModelTrait, ActiveValue::NotSet, ColumnTrait, Condition, DatabaseConnection, EntityTrait, QueryFilter, QueryOrder, QuerySelect, Set, }; @@ -140,6 +140,46 @@ pub async fn update_status( Ok(()) } +/// Move the child row's mutable "currently active delegation" pointer only +/// when it still names the expected predecessor. Historical linkage lives in +/// the delegation ledger; this field is solely a live routing guard. +pub async fn advance_delegation_call_id( + conn: &DatabaseConnection, + conversation_id: i32, + expected: &str, + next: &str, +) -> Result { + let result = conversation::Entity::update_many() + .col_expr( + conversation::Column::DelegationCallId, + sea_orm::sea_query::Expr::value(next.to_string()), + ) + .filter(conversation::Column::Id.eq(conversation_id)) + .filter( + Condition::any() + .add(conversation::Column::DelegationCallId.eq(expected)) + .add(conversation::Column::DelegationCallId.is_null()), + ) + .exec(conn) + .await?; + Ok(result.rows_affected == 1) +} + +pub async fn clear_delegation_call_id( + conn: &DatabaseConnection, + conversation_id: i32, +) -> Result<(), DbError> { + conversation::Entity::update_many() + .col_expr( + conversation::Column::DelegationCallId, + sea_orm::sea_query::Expr::value(Option::::None), + ) + .filter(conversation::Column::Id.eq(conversation_id)) + .exec(conn) + .await?; + Ok(()) +} + /// Conditional status transition (CAS): write `new_status` only if the row's /// current `status` equals `expected`. Returns `true` when the row was /// updated. Used by the lifecycle subscriber on disconnect/error so a @@ -162,6 +202,29 @@ pub async fn update_status_if( Ok(result.rows_affected > 0) } +/// Conditional status transition scoped to the immutable identity of the +/// connection that produced the event. `None` is the human-owned path and may +/// only update a row whose live delegation pointer is also NULL. +pub async fn update_status_for_execution_if( + conn: &DatabaseConnection, + conversation_id: i32, + execution_task_id: Option<&str>, + expected: conversation::ConversationStatus, + new_status: conversation::ConversationStatus, +) -> Result { + use sea_orm::sea_query::Expr; + let update = conversation::Entity::update_many() + .col_expr(conversation::Column::Status, Expr::value(new_status)) + .col_expr(conversation::Column::UpdatedAt, Expr::value(Utc::now())) + .filter(conversation::Column::Id.eq(conversation_id)) + .filter(conversation::Column::Status.eq(expected)); + let update = match execution_task_id { + Some(task_id) => update.filter(conversation::Column::DelegationCallId.eq(task_id)), + None => update.filter(conversation::Column::DelegationCallId.is_null()), + }; + Ok(update.exec(conn).await?.rows_affected == 1) +} + /// Manual rename: set the title AND lock it. Once locked, the per-turn /// auto-title backfill ([`refresh_auto_title`]) leaves this row alone, so the /// user's hand-picked name survives every subsequent session-file parse. @@ -821,7 +884,10 @@ pub async fn bind_external_id( } let agent_type = carried.agent_type.clone(); - let preserved = carried.into_active_model(previous.clone()).insert(txn).await?; + let preserved = carried + .into_active_model(previous.clone()) + .insert(txn) + .await?; // The one signal that this happened at all. Deliberately WARN: // every occurrence means a connection bound to a row while // holding a session unrelated to that row's history, which is @@ -1395,6 +1461,7 @@ mod tests { parent_conversation_id: parent.id, parent_tool_use_id: "tu-1".into(), delegation_call_id: "call-1".into(), + admission: None, }; let child = create_with_delegation( conn, @@ -1486,6 +1553,7 @@ mod tests { parent_conversation_id: parent.id, parent_tool_use_id: "tu-1".into(), delegation_call_id: "call-1".into(), + admission: None, }), ) .await @@ -1500,6 +1568,7 @@ mod tests { parent_conversation_id: parent.id, parent_tool_use_id: "tu-2".into(), delegation_call_id: "call-2".into(), + admission: None, }), ) .await @@ -1548,6 +1617,7 @@ mod tests { parent_conversation_id: child, parent_tool_use_id: "tu-2".into(), delegation_call_id: "call-2".into(), + admission: None, }; create_with_delegation( &db.conn, @@ -1593,15 +1663,9 @@ mod tests { async fn seed_model_fills_an_empty_column_once_without_bumping_updated_at() { let db = fresh_in_memory_db().await; let folder = seed_folder(&db, "/tmp/codeg-seed-model").await; - let conv = create( - &db.conn, - folder, - AgentType::Codex, - Some("c".into()), - None, - ) - .await - .expect("create"); + let conv = create(&db.conn, folder, AgentType::Codex, Some("c".into()), None) + .await + .expect("create"); // The gap this closes: a row created in-app carries no model at all, // which is why the sidebar could only ever show one for imported @@ -1644,26 +1708,18 @@ mod tests { ); // A transcript that names no model asks for no write at all. - assert!( - !seed_model_if_empty(&db.conn, conv.id, " ") - .await - .expect("blank seed") - ); + assert!(!seed_model_if_empty(&db.conn, conv.id, " ") + .await + .expect("blank seed")); } #[tokio::test] async fn seed_model_skips_a_soft_deleted_row() { let db = fresh_in_memory_db().await; let folder = seed_folder(&db, "/tmp/codeg-seed-model-deleted").await; - let conv = create( - &db.conn, - folder, - AgentType::Codex, - Some("c".into()), - None, - ) - .await - .expect("create"); + let conv = create(&db.conn, folder, AgentType::Codex, Some("c".into()), None) + .await + .expect("create"); soft_delete(&db.conn, conv.id).await.expect("delete"); assert!( @@ -1785,7 +1841,10 @@ mod tests { } /// The single live row (if any) holding `external_id`, whatever its id. - async fn rows_holding(conn: &DatabaseConnection, external_id: &str) -> Vec { + async fn rows_holding( + conn: &DatabaseConnection, + external_id: &str, + ) -> Vec { conversation::Entity::find() .filter(conversation::Column::ExternalId.eq(external_id)) .filter(conversation::Column::DeletedAt.is_null()) @@ -2095,7 +2154,10 @@ mod tests { preserved, None, "a row with no session yet has nothing to preserve" ); - assert_eq!(raw_row(&db.conn, row.id).await.external_id.as_deref(), Some("S1")); + assert_eq!( + raw_row(&db.conn, row.id).await.external_id.as_deref(), + Some("S1") + ); } #[tokio::test] @@ -2107,7 +2169,9 @@ mod tests { let row = create(&db.conn, folder, AgentType::ClaudeCode, None, None) .await .expect("create"); - bind_external_id(&db.conn, row.id, "S1", &[]).await.expect("bind"); + bind_external_id(&db.conn, row.id, "S1", &[]) + .await + .expect("bind"); let preserved = bind_external_id(&db.conn, row.id, "S1", &[]) .await @@ -2128,7 +2192,9 @@ mod tests { let row = create(&db.conn, folder, AgentType::Codex, None, None) .await .expect("create"); - bind_external_id(&db.conn, row.id, "S1", &[]).await.expect("bind"); + bind_external_id(&db.conn, row.id, "S1", &[]) + .await + .expect("bind"); // Stand in for fork's sibling insert. let sibling = create(&db.conn, folder, AgentType::Codex, None, None) .await @@ -2142,7 +2208,10 @@ mod tests { let mut original: conversation::ActiveModel = raw_row(&db.conn, row.id).await.into(); original.external_id = Set(Some("S2".into())); original.update(&db.conn).await.expect("release"); - active.update(&db.conn).await.expect("hand S1 to the sibling"); + active + .update(&db.conn) + .await + .expect("hand S1 to the sibling"); // Now the late SessionStarted{S2} arrives for the original row. let preserved = bind_external_id(&db.conn, row.id, "S2", &[]) @@ -2183,7 +2252,9 @@ mod tests { .await .expect("create"); let seed = format!("S1-{original:?}"); - bind_external_id(&db.conn, row.id, &seed, &[]).await.expect("bind"); + bind_external_id(&db.conn, row.id, &seed, &[]) + .await + .expect("bind"); update_status(&db.conn, row.id, original.clone()) .await .expect("status"); @@ -2520,11 +2591,9 @@ mod tests { .expect("seed-locked"), "a locked title must not be seeded over" ); - assert!( - !seed_auto_title_if_empty(&db.conn, row.id, String::new()) - .await - .expect("seed-empty") - ); + assert!(!seed_auto_title_if_empty(&db.conn, row.id, String::new()) + .await + .expect("seed-empty")); let summary = get_by_id(&db.conn, row.id).await.expect("get"); assert_eq!(summary.title.as_deref(), Some("User pick")); } @@ -2561,7 +2630,9 @@ mod tests { ) .await .expect("create"); - soft_delete(&db.conn, refreshed.id).await.expect("soft delete"); + soft_delete(&db.conn, refreshed.id) + .await + .expect("soft delete"); assert!( !refresh_auto_title(&db.conn, refreshed.id, "Agent title".into()) .await @@ -3233,6 +3304,7 @@ mod tests { parent_conversation_id: parent.id, parent_tool_use_id: "tu-activity".into(), delegation_call_id: "call-activity".into(), + admission: None, }), ) .await @@ -3293,6 +3365,7 @@ mod tests { parent_conversation_id: regular.id, parent_tool_use_id: "tu-kind".into(), delegation_call_id: "call-kind".into(), + admission: None, }), ) .await diff --git a/src-tauri/src/db/service/delegation_task_service.rs b/src-tauri/src/db/service/delegation_task_service.rs new file mode 100644 index 0000000000..e3027a129a --- /dev/null +++ b/src-tauri/src/db/service/delegation_task_service.rs @@ -0,0 +1,1265 @@ +//! Durable ordinary-delegation admission and terminal-result ledger. +//! +//! This module intentionally owns only the durable facts needed by the +//! delegation runtime. It does not own connections, turns, retries, or +//! process cleanup. The runtime performs its busy/strict-resume checks first, +//! then calls [`admit`] immediately before sending the child prompt. + +use std::collections::BTreeMap; + +use chrono::{DateTime, Utc}; +use sea_orm::{ + ActiveModelTrait, ActiveValue::NotSet, ColumnTrait, Condition, ConnectionTrait, + DatabaseConnection, EntityTrait, IntoActiveModel, QueryFilter, QueryOrder, Set, + TransactionTrait, +}; + +use crate::acp::delegation::types::{DelegationTaskReport, TaskStatus}; +use crate::db::entities::{conversation, delegation_task, folder}; +use crate::db::error::DbError; +use crate::models::AgentType; + +/// The session identity and selector preferences captured for a child execution. +/// +/// This is serialized into the ledger as one JSON value so a future resume can +/// validate agent/session/cwd identity and best-effort restore its selectors. +/// `working_dir` is the canonical directory actually used by the child, while +/// `requested_working_dir` on [`AdmissionInput`] preserves the caller's raw +/// request for retry correlation. +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +pub struct ResumeBinding { + pub agent_type: AgentType, + pub external_session_id: String, + pub child_conversation_id: i32, + pub working_dir: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + /// Effective value after the runtime applies its requested/default mode. + /// The `preferred_` name matches the manager/spawner hand-off vocabulary. + pub preferred_mode_id: Option, + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub preferred_config_values: BTreeMap, + /// Historical launch snapshot for diagnostics; configuration is not + /// session identity and may legitimately change between continuations. + pub config_fingerprint: String, +} + +/// Prepared admission input. The runtime must supply the already-created +/// child row and the strict binding it intends to use; no prompt is sent by +/// this module. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AdmissionInput { + pub task_id: String, + pub parent_conversation_id: i32, + pub child_conversation_id: i32, + pub source_task_id: Option, + pub task: String, + pub requested_working_dir: Option, + pub resume_binding: ResumeBinding, +} + +/// Metadata plus the report visible to the broker. A terminal report is read +/// from the immutable JSON snapshot; a running row gets a synthesized running +/// report and never reads the child's mutable conversation status. +#[derive(Debug, Clone)] +pub struct TaskLedgerEntry { + pub id: i32, + pub task_id: String, + pub parent_conversation_id: i32, + pub child_conversation_id: i32, + pub source_task_id: Option, + pub task: String, + pub requested_working_dir: Option, + pub resume_binding: ResumeBinding, + pub status: TaskStatus, + pub released: bool, + pub report: DelegationTaskReport, + pub created_at: DateTime, + pub updated_at: DateTime, +} + +/// Result of the atomic source-slot admission. +#[derive(Debug, Clone)] +pub enum AdmissionResult { + New { + entry: TaskLedgerEntry, + }, + Existing { + entry: TaskLedgerEntry, + }, + Conflict { + next_task_id: String, + reason: String, + }, +} + +/// Distinguishes a pre-ledger task from a ledger task hidden by parent or +/// soft-delete authorization. Callers may use legacy storage only for +/// `Absent`; `Hidden` must remain opaque. +#[derive(Debug, Clone)] +// The visible branch intentionally returns the complete immutable ledger +// snapshot; boxing every scoped read would add allocation to the hot status +// path solely to shrink the two marker variants. +#[allow(clippy::large_enum_variant)] +pub enum ScopedLookup { + Visible(TaskLedgerEntry), + Hidden, + Absent, +} + +/// Insert one durable execution record. A source slot is reserved by the +/// unique index on `source_task_id`; a loser of that race is reconciled with +/// the winner and never sends a second prompt for the same source. +pub async fn admit( + conn: &DatabaseConnection, + input: AdmissionInput, +) -> Result { + admit_on(conn, input).await +} + +/// Reserve a continuation source slot and advance the child's live routing +/// pointer in one SQLite transaction. A crash can therefore expose neither a +/// phantom pointer nor an admitted task whose prompt cannot be routed. +pub async fn admit_continuation( + conn: &DatabaseConnection, + input: AdmissionInput, +) -> Result { + let source_task_id = input.source_task_id.clone().ok_or_else(|| { + DbError::Validation("continuation admission requires a source task".into()) + })?; + let txn = conn.begin().await?; + let result = admit_on(&txn, input.clone()).await?; + let next_task_id = match &result { + AdmissionResult::New { entry } => &entry.task_id, + AdmissionResult::Existing { .. } | AdmissionResult::Conflict { .. } => { + txn.commit().await?; + return Ok(result); + } + }; + let updated = conversation::Entity::update_many() + .col_expr( + conversation::Column::DelegationCallId, + sea_orm::sea_query::Expr::value(next_task_id.clone()), + ) + .filter(conversation::Column::Id.eq(input.child_conversation_id)) + .filter( + Condition::any() + .add(conversation::Column::DelegationCallId.eq(&source_task_id)) + .add(conversation::Column::DelegationCallId.is_null()), + ) + .exec(&txn) + .await?; + if updated.rows_affected != 1 { + return Err(DbError::Conflict( + "child session was taken over during continuation admission".into(), + )); + } + txn.commit().await?; + Ok(result) +} + +async fn admit_on( + conn: &C, + input: AdmissionInput, +) -> Result { + validate_input(&input)?; + ensure_live_conversation(conn, input.parent_conversation_id, "parent").await?; + ensure_live_conversation(conn, input.child_conversation_id, "child").await?; + + if let Some(source_task_id) = input.source_task_id.as_deref() { + // Retries must resolve an already-admitted successor before checking + // whether the source can be started again. This is important after a + // process restart: the successor may still be running while the + // caller repeats the same request. + if let Some(winner) = delegation_task::Entity::find() + .filter(delegation_task::Column::ParentConversationId.eq(input.parent_conversation_id)) + .filter(delegation_task::Column::SourceTaskId.eq(source_task_id)) + .one(conn) + .await? + { + let Some(entry) = + load_authorized(conn, input.parent_conversation_id, &winner.task_id).await? + else { + return Err(DbError::Conflict(format!( + "source task {source_task_id} is already reserved but is no longer queryable" + ))); + }; + if same_admission_key(&winner, &input)? { + return Ok(AdmissionResult::Existing { entry }); + } + return Ok(AdmissionResult::Conflict { + next_task_id: winner.task_id, + reason: format!( + "source task {source_task_id} already has a successor with a different task, agent, or working directory" + ), + }); + } + + let source = find_raw_by_task_id(conn, source_task_id) + .await? + .ok_or_else(|| DbError::NotFound(format!("source task {source_task_id}")))?; + if source.parent_conversation_id != input.parent_conversation_id { + return Err(DbError::NotFound(format!("source task {source_task_id}"))); + } + if !is_terminal_status(&source.status) || !source.released { + return Err(DbError::Validation(format!( + "source task {source_task_id} is not eligible: it must be terminal and released" + ))); + } + let source_binding: ResumeBinding = + serde_json::from_str(&source.resume_binding).map_err(|e| { + DbError::Migration(format!( + "invalid resume binding for source {source_task_id}: {e}" + )) + })?; + if source_binding != input.resume_binding { + return Err(DbError::Conflict(format!( + "source task {source_task_id} binding does not match the requested child session" + ))); + } + // This also checks the source child and its folder. A retained source + // is not a usable resume anchor after either conversation is deleted. + if load_authorized(conn, input.parent_conversation_id, source_task_id) + .await? + .is_none() + { + return Err(DbError::NotFound(format!("source task {source_task_id}"))); + } + } + + let binding_json = serde_json::to_string(&input.resume_binding) + .map_err(|e| DbError::Validation(format!("invalid resume binding: {e}")))?; + let now = Utc::now(); + let active = delegation_task::ActiveModel { + id: NotSet, + task_id: Set(input.task_id.clone()), + parent_conversation_id: Set(input.parent_conversation_id), + child_conversation_id: Set(input.child_conversation_id), + source_task_id: Set(input.source_task_id.clone()), + task: Set(input.task.clone()), + requested_working_dir: Set(input.requested_working_dir.clone()), + status: Set(status_string(TaskStatus::Running)), + terminal_report: Set(None), + resume_binding: Set(binding_json), + released: Set(false), + created_at: Set(now), + updated_at: Set(now), + }; + + match active.insert(conn).await { + Ok(model) => Ok(AdmissionResult::New { + entry: entry_from_model(model)?, + }), + Err(insert_error) => reconcile_insert_race(conn, &input, insert_error).await, + } +} + +/// Lookup a task under parent authorization. Deleted parent/child/folder rows +/// are intentionally indistinguishable from an unknown task. +pub async fn lookup( + conn: &DatabaseConnection, + parent_conversation_id: i32, + task_id: &str, +) -> Result, DbError> { + load_authorized(conn, parent_conversation_id, task_id).await +} + +pub async fn lookup_scoped( + conn: &DatabaseConnection, + parent_conversation_id: i32, + task_id: &str, +) -> Result { + let Some(row) = find_raw_by_task_id(conn, task_id).await? else { + return Ok(ScopedLookup::Absent); + }; + if row.parent_conversation_id != parent_conversation_id + || !conversations_are_live(conn, parent_conversation_id, row.child_conversation_id).await? + { + return Ok(ScopedLookup::Hidden); + } + Ok(ScopedLookup::Visible(entry_from_model(row)?)) +} + +/// Return every durable task owned by a parent whose parent/child rows and +/// folders are still visible. A child can execute several continuation rounds, +/// so this deliberately returns task history rather than its current pointer. +pub async fn list_for_parent( + conn: &DatabaseConnection, + parent_conversation_id: i32, +) -> Result, DbError> { + let rows = delegation_task::Entity::find() + .filter(delegation_task::Column::ParentConversationId.eq(parent_conversation_id)) + .order_by_asc(delegation_task::Column::CreatedAt) + .order_by_asc(delegation_task::Column::Id) + .all(conn) + .await?; + let mut entries = Vec::with_capacity(rows.len()); + for row in rows { + if conversations_are_live(conn, parent_conversation_id, row.child_conversation_id).await? { + entries.push(entry_from_model(row)?); + } + } + Ok(entries) +} + +/// Return the one successor reserved by `source_task_id`, if it is visible to +/// the authorized parent. +pub async fn successor( + conn: &DatabaseConnection, + parent_conversation_id: i32, + source_task_id: &str, +) -> Result, DbError> { + let row = delegation_task::Entity::find() + .filter(delegation_task::Column::ParentConversationId.eq(parent_conversation_id)) + .filter(delegation_task::Column::SourceTaskId.eq(source_task_id)) + .one(conn) + .await?; + let Some(row) = row else { + return Ok(None); + }; + if !conversations_are_live(conn, parent_conversation_id, row.child_conversation_id).await? { + return Ok(None); + } + Ok(Some(entry_from_model(row)?)) +} + +/// Return the source id of an authorized task. The source row itself is not +/// required to be live here; only the task being inspected is authorized. +pub async fn source( + conn: &DatabaseConnection, + parent_conversation_id: i32, + task_id: &str, +) -> Result, DbError> { + Ok(lookup(conn, parent_conversation_id, task_id) + .await? + .and_then(|entry| entry.source_task_id)) +} + +/// Freeze one terminal report. The conditional UPDATE makes finish idempotent +/// and prevents a late old connection from changing a newer terminal result. +/// Returns `true` only when this call won the terminal write. +pub async fn finish( + conn: &DatabaseConnection, + parent_conversation_id: i32, + task_id: &str, + report: &DelegationTaskReport, +) -> Result { + let Some(row) = load_for_write(conn, parent_conversation_id, task_id).await? else { + return Err(DbError::NotFound(format!("delegation task {task_id}"))); + }; + validate_terminal_report(task_id, report, &row)?; + let terminal_report = serde_json::to_string(report) + .map_err(|e| DbError::Validation(format!("cannot serialize terminal report: {e}")))?; + let result = delegation_task::Entity::update_many() + .col_expr( + delegation_task::Column::Status, + sea_orm::sea_query::Expr::value(status_string(report.status)), + ) + .col_expr( + delegation_task::Column::TerminalReport, + sea_orm::sea_query::Expr::value(terminal_report), + ) + .col_expr( + delegation_task::Column::UpdatedAt, + sea_orm::sea_query::Expr::value(Utc::now()), + ) + .filter(delegation_task::Column::Id.eq(row.id)) + .filter(delegation_task::Column::Status.eq(status_string(TaskStatus::Running))) + .filter(delegation_task::Column::TerminalReport.is_null()) + .exec(conn) + .await?; + Ok(result.rows_affected == 1) +} + +/// Mark process release independently from finish. This is deliberately a +/// one-way CAS: finishing a task never resets a release acknowledgement. +pub async fn mark_released( + conn: &DatabaseConnection, + parent_conversation_id: i32, + task_id: &str, +) -> Result { + let Some(row) = load_for_write(conn, parent_conversation_id, task_id).await? else { + return Err(DbError::NotFound(format!("delegation task {task_id}"))); + }; + let result = delegation_task::Entity::update_many() + .col_expr( + delegation_task::Column::Released, + sea_orm::sea_query::Expr::value(true), + ) + .col_expr( + delegation_task::Column::UpdatedAt, + sea_orm::sea_query::Expr::value(Utc::now()), + ) + .filter(delegation_task::Column::Id.eq(row.id)) + .filter(delegation_task::Column::Released.eq(false)) + .exec(conn) + .await?; + Ok(result.rows_affected == 1) +} + +/// Reconcile execution ownership from a previous process. No ACP child or +/// release barrier survives a restart, so every unreleased row is now released; +/// a row still marked running is frozen with an unknown interrupted outcome. +pub async fn boot_reconcile_interrupted(conn: &DatabaseConnection) -> Result { + let txn = conn.begin().await?; + let rows = delegation_task::Entity::find() + .filter(delegation_task::Column::Released.eq(false)) + .all(&txn) + .await?; + let count = rows.len() as u64; + + for row in rows { + let mut active = row.clone().into_active_model(); + if row.status == status_string(TaskStatus::Running) { + let binding: ResumeBinding = serde_json::from_str(&row.resume_binding).map_err(|e| { + DbError::Migration(format!( + "invalid resume binding for {}: {e}", + row.task_id + )) + })?; + let report = DelegationTaskReport { + task_id: Some(row.task_id.clone()), + status: TaskStatus::Unknown, + child_conversation_id: Some(row.child_conversation_id), + agent_type: Some(binding.agent_type), + text: None, + error_code: Some("interrupted".into()), + message: Some( + "The application stopped while this delegation was running; its outcome is unknown." + .into(), + ), + duration_ms: None, + blocked_on: None, + }; + active.status = Set(status_string(TaskStatus::Unknown)); + active.terminal_report = Set(Some(serde_json::to_string(&report).map_err(|e| { + DbError::Validation(format!("cannot serialize interrupted report: {e}")) + })?)); + } + active.released = Set(true); + active.updated_at = Set(Utc::now()); + active.update(&txn).await?; + } + + txn.commit().await?; + Ok(count) +} + +async fn reconcile_insert_race( + conn: &C, + input: &AdmissionInput, + insert_error: sea_orm::DbErr, +) -> Result { + let Some(source_task_id) = input.source_task_id.as_deref() else { + return Err(insert_error.into()); + }; + let Some(winner) = delegation_task::Entity::find() + .filter(delegation_task::Column::ParentConversationId.eq(input.parent_conversation_id)) + .filter(delegation_task::Column::SourceTaskId.eq(source_task_id)) + .one(conn) + .await? + else { + return Err(insert_error.into()); + }; + let Some(entry) = load_authorized(conn, input.parent_conversation_id, &winner.task_id).await? + else { + return Err(DbError::Conflict(format!( + "source task {source_task_id} is already reserved but is no longer queryable" + ))); + }; + if same_admission_key(&winner, input)? { + return Ok(AdmissionResult::Existing { entry }); + } + Ok(AdmissionResult::Conflict { + next_task_id: winner.task_id, + reason: format!( + "source task {source_task_id} already has a successor with a different task, agent, or working directory" + ), + }) +} + +async fn load_authorized( + conn: &C, + parent_conversation_id: i32, + task_id: &str, +) -> Result, DbError> { + let Some(row) = delegation_task::Entity::find() + .filter(delegation_task::Column::TaskId.eq(task_id)) + .filter(delegation_task::Column::ParentConversationId.eq(parent_conversation_id)) + .one(conn) + .await? + else { + return Ok(None); + }; + if !conversations_are_live(conn, parent_conversation_id, row.child_conversation_id).await? { + return Ok(None); + } + Ok(Some(entry_from_model(row)?)) +} + +async fn load_for_write( + conn: &C, + parent_conversation_id: i32, + task_id: &str, +) -> Result, DbError> { + let row = delegation_task::Entity::find() + .filter(delegation_task::Column::TaskId.eq(task_id)) + .filter(delegation_task::Column::ParentConversationId.eq(parent_conversation_id)) + .one(conn) + .await?; + row.map(entry_from_model).transpose() +} + +async fn find_raw_by_task_id( + conn: &C, + task_id: &str, +) -> Result, DbError> { + Ok(delegation_task::Entity::find() + .filter(delegation_task::Column::TaskId.eq(task_id)) + .one(conn) + .await?) +} + +async fn ensure_live_conversation( + conn: &C, + conversation_id: i32, + label: &str, +) -> Result<(), DbError> { + if conversations_are_live(conn, conversation_id, conversation_id).await? { + return Ok(()); + } + Err(DbError::NotFound(format!( + "{label} conversation {conversation_id}" + ))) +} + +async fn conversations_are_live( + conn: &C, + parent_conversation_id: i32, + child_conversation_id: i32, +) -> Result { + let Some(parent) = conversation::Entity::find_by_id(parent_conversation_id) + .one(conn) + .await? + else { + return Ok(false); + }; + let Some(child) = conversation::Entity::find_by_id(child_conversation_id) + .one(conn) + .await? + else { + return Ok(false); + }; + if parent.deleted_at.is_some() || child.deleted_at.is_some() { + return Ok(false); + } + let Some(parent_folder) = folder::Entity::find_by_id(parent.folder_id) + .one(conn) + .await? + else { + return Ok(false); + }; + let Some(child_folder) = folder::Entity::find_by_id(child.folder_id) + .one(conn) + .await? + else { + return Ok(false); + }; + Ok(parent_folder.deleted_at.is_none() && child_folder.deleted_at.is_none()) +} + +fn entry_from_model(model: delegation_task::Model) -> Result { + let status = parse_status(&model.status)?; + let binding: ResumeBinding = serde_json::from_str(&model.resume_binding).map_err(|e| { + DbError::Migration(format!("invalid resume binding for {}: {e}", model.task_id)) + })?; + let report = match model.terminal_report.as_deref() { + Some(raw) => serde_json::from_str(raw).map_err(|e| { + DbError::Migration(format!( + "invalid terminal report for {}: {e}", + model.task_id + )) + })?, + None => running_report(&model, status, binding.agent_type), + }; + Ok(TaskLedgerEntry { + id: model.id, + task_id: model.task_id, + parent_conversation_id: model.parent_conversation_id, + child_conversation_id: model.child_conversation_id, + source_task_id: model.source_task_id, + task: model.task, + requested_working_dir: model.requested_working_dir, + resume_binding: binding, + status, + released: model.released, + report, + created_at: model.created_at, + updated_at: model.updated_at, + }) +} + +fn running_report( + model: &delegation_task::Model, + status: TaskStatus, + agent_type: AgentType, +) -> DelegationTaskReport { + DelegationTaskReport { + task_id: Some(model.task_id.clone()), + status, + child_conversation_id: Some(model.child_conversation_id), + agent_type: Some(agent_type), + text: None, + error_code: None, + message: None, + duration_ms: None, + blocked_on: None, + } +} + +fn validate_input(input: &AdmissionInput) -> Result<(), DbError> { + if input.task_id.trim().is_empty() { + return Err(DbError::Validation("task id must not be empty".into())); + } + if input.task.trim().is_empty() { + return Err(DbError::Validation("task must not be empty".into())); + } + if input.resume_binding.child_conversation_id != input.child_conversation_id { + return Err(DbError::Validation( + "resume binding child conversation does not match admission".into(), + )); + } + if input.resume_binding.external_session_id.trim().is_empty() + || input.resume_binding.working_dir.trim().is_empty() + || input.resume_binding.config_fingerprint.trim().is_empty() + { + return Err(DbError::Validation( + "resume binding requires session id, working directory, and config fingerprint".into(), + )); + } + Ok(()) +} + +fn validate_terminal_report( + task_id: &str, + report: &DelegationTaskReport, + entry: &TaskLedgerEntry, +) -> Result<(), DbError> { + if !is_terminal(report.status) { + return Err(DbError::Validation( + "only completed, failed, or canceled reports can finish a task".into(), + )); + } + if report.task_id.as_deref() != Some(task_id) { + return Err(DbError::Validation( + "terminal report must carry the admitted task id".into(), + )); + } + if report.child_conversation_id != Some(entry.child_conversation_id) { + return Err(DbError::Validation( + "terminal report child conversation does not match admission".into(), + )); + } + if report.agent_type != Some(entry.resume_binding.agent_type) { + return Err(DbError::Validation( + "terminal report agent does not match admission".into(), + )); + } + Ok(()) +} + +fn same_admission_key( + row: &delegation_task::Model, + input: &AdmissionInput, +) -> Result { + let binding: ResumeBinding = serde_json::from_str(&row.resume_binding).map_err(|e| { + DbError::Migration(format!("invalid resume binding for {}: {e}", row.task_id)) + })?; + Ok(row.task == input.task && binding == input.resume_binding) +} + +fn is_terminal(status: TaskStatus) -> bool { + matches!( + status, + TaskStatus::Completed | TaskStatus::Failed | TaskStatus::Canceled + ) +} + +fn is_terminal_status(status: &str) -> bool { + matches!(status, "completed" | "failed" | "canceled" | "unknown") +} + +fn status_string(status: TaskStatus) -> String { + serde_json::to_value(status) + .ok() + .and_then(|v| v.as_str().map(str::to_owned)) + .unwrap_or_else(|| "unknown".to_owned()) +} + +fn parse_status(status: &str) -> Result { + serde_json::from_value(serde_json::Value::String(status.to_owned())) + .map_err(|e| DbError::Migration(format!("invalid delegation task status {status:?}: {e}"))) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::db::service::{conversation_service, folder_service}; + use crate::db::test_helpers::{fresh_disk_db, fresh_in_memory_db}; + use sea_orm::Database; + + fn binding(child: i32) -> ResumeBinding { + ResumeBinding { + agent_type: AgentType::Codex, + external_session_id: format!("session-{child}"), + child_conversation_id: child, + working_dir: "/workspace/project".into(), + preferred_mode_id: Some("default".into()), + preferred_config_values: BTreeMap::from([(String::from("model"), String::from("o3"))]), + config_fingerprint: "fingerprint-1".into(), + } + } + + fn input( + task_id: &str, + parent: i32, + child: i32, + source: Option<&str>, + task: &str, + ) -> AdmissionInput { + AdmissionInput { + task_id: task_id.into(), + parent_conversation_id: parent, + child_conversation_id: child, + source_task_id: source.map(str::to_owned), + task: task.into(), + requested_working_dir: Some("/workspace/project".into()), + resume_binding: binding(child), + } + } + + async fn conversations(db: &crate::db::AppDatabase) -> (i32, i32) { + let folder = folder_service::add_folder(&db.conn, "/workspace/project") + .await + .expect("folder") + .id; + let parent = + conversation_service::create(&db.conn, folder, AgentType::ClaudeCode, None, None) + .await + .expect("parent") + .id; + let child = conversation_service::create(&db.conn, folder, AgentType::Codex, None, None) + .await + .expect("child") + .id; + (parent, child) + } + + fn report(task_id: &str, child: i32, text: &str, status: TaskStatus) -> DelegationTaskReport { + DelegationTaskReport { + task_id: Some(task_id.into()), + status, + child_conversation_id: Some(child), + agent_type: Some(AgentType::Codex), + text: Some(text.into()), + error_code: None, + message: None, + duration_ms: Some(12), + blocked_on: None, + } + } + + #[tokio::test] + async fn terminal_reports_survive_disk_reopen_and_child_status_drift() { + let dir = tempfile::tempdir().expect("tempdir"); + let db = fresh_disk_db(dir.path()).await; + let (parent, child) = conversations(&db).await; + let result = admit(&db.conn, input("t0", parent, child, None, "first")) + .await + .expect("admit"); + assert!(matches!(result, AdmissionResult::New { .. })); + + let first = report("t0", child, "original", TaskStatus::Completed); + assert!(finish(&db.conn, parent, "t0", &first) + .await + .expect("finish")); + mark_released(&db.conn, parent, "t0") + .await + .expect("release t0"); + admit( + &db.conn, + input("t1", parent, child, Some("t0"), "follow-up"), + ) + .await + .expect("admit t1"); + assert!(finish( + &db.conn, + parent, + "t1", + &report("t1", child, "follow-up", TaskStatus::Completed), + ) + .await + .expect("finish t1")); + assert!(!finish( + &db.conn, + parent, + "t0", + &report("t0", child, "late overwrite", TaskStatus::Failed), + ) + .await + .expect("idempotent finish")); + conversation_service::update_status( + &db.conn, + child, + conversation::ConversationStatus::Cancelled, + ) + .await + .expect("child status"); + db.conn.close().await.expect("close disk db"); + + let path = dir.path().join("source.db"); + let reopened = Database::connect(format!("sqlite:{}?mode=rwc", path.to_string_lossy())) + .await + .expect("reopen disk db"); + + let entry = lookup(&reopened, parent, "t0") + .await + .expect("lookup") + .expect("entry"); + assert_eq!( + serde_json::to_value(&entry.report).expect("stored report"), + serde_json::to_value(&first).expect("expected report"), + ); + assert_eq!(entry.status, TaskStatus::Completed); + let successor_entry = lookup(&reopened, parent, "t1") + .await + .expect("lookup t1") + .expect("t1 entry"); + assert_eq!(successor_entry.report.text.as_deref(), Some("follow-up")); + reopened.close().await.expect("close reopened db"); + } + + #[tokio::test] + async fn boot_reconcile_repairs_unreleased_rows_after_disk_reopen() { + let dir = tempfile::tempdir().expect("tempdir"); + let db = fresh_disk_db(dir.path()).await; + let (parent, child) = conversations(&db).await; + admit(&db.conn, input("running", parent, child, None, "running task")) + .await + .expect("admit running"); + admit(&db.conn, input("done", parent, child, None, "done task")) + .await + .expect("admit done"); + let done_report = report("done", child, "finished", TaskStatus::Completed); + assert!(finish(&db.conn, parent, "done", &done_report) + .await + .expect("finish done")); + db.conn.close().await.expect("close disk db"); + + let path = dir.path().join("source.db"); + let reopened = Database::connect(format!("sqlite:{}?mode=rwc", path.to_string_lossy())) + .await + .expect("reopen disk db"); + assert_eq!(boot_reconcile_interrupted(&reopened).await.unwrap(), 2); + assert_eq!(boot_reconcile_interrupted(&reopened).await.unwrap(), 0); + + let interrupted = lookup(&reopened, parent, "running") + .await + .unwrap() + .unwrap(); + assert_eq!(interrupted.status, TaskStatus::Unknown); + assert!(interrupted.released); + assert_eq!(interrupted.report.error_code.as_deref(), Some("interrupted")); + + let done = lookup(&reopened, parent, "done").await.unwrap().unwrap(); + assert_eq!(done.status, TaskStatus::Completed); + assert!(done.released); + assert_eq!(done.report.text.as_deref(), Some("finished")); + + assert!(matches!( + admit_continuation( + &reopened, + input("continued", parent, child, Some("running"), "continue"), + ) + .await + .unwrap(), + AdmissionResult::New { .. } + )); + reopened.close().await.expect("close reopened db"); + } + + #[tokio::test] + async fn lookup_requires_parent_and_live_rows() { + let db = fresh_in_memory_db().await; + let (parent, child) = conversations(&db).await; + admit(&db.conn, input("t0", parent, child, None, "first")) + .await + .expect("admit"); + let other_parent = { + let folder = folder_service::add_folder(&db.conn, "/workspace/other") + .await + .expect("folder") + .id; + conversation_service::create(&db.conn, folder, AgentType::ClaudeCode, None, None) + .await + .expect("other parent") + .id + }; + assert!(lookup(&db.conn, other_parent, "t0") + .await + .expect("auth lookup") + .is_none()); + conversation_service::soft_delete(&db.conn, child) + .await + .expect("delete child"); + assert!(lookup(&db.conn, parent, "t0") + .await + .expect("deleted lookup") + .is_none()); + + let db = fresh_in_memory_db().await; + let (parent, child) = conversations(&db).await; + admit(&db.conn, input("folder-task", parent, child, None, "task")) + .await + .expect("admit folder task"); + let folder_id = conversation::Entity::find_by_id(parent) + .one(&db.conn) + .await + .expect("parent row") + .expect("parent") + .folder_id; + folder_service::soft_delete_folder(&db.conn, folder_id) + .await + .expect("delete folder"); + assert!(lookup(&db.conn, parent, "folder-task") + .await + .expect("folder-deleted lookup") + .is_none()); + + let db = fresh_in_memory_db().await; + let (parent, child) = conversations(&db).await; + admit(&db.conn, input("parent-task", parent, child, None, "task")) + .await + .expect("admit parent task"); + conversation_service::soft_delete(&db.conn, parent) + .await + .expect("delete parent"); + assert!(lookup(&db.conn, parent, "parent-task") + .await + .expect("parent-deleted lookup") + .is_none()); + } + + #[tokio::test] + async fn deleted_parent_still_allows_finish_release_before_restore_lookup() { + let db = fresh_in_memory_db().await; + let (parent, child) = conversations(&db).await; + admit(&db.conn, input("t0", parent, child, None, "first")) + .await + .expect("admit"); + let folder_id = conversation::Entity::find_by_id(parent) + .one(&db.conn) + .await + .expect("parent row") + .expect("parent") + .folder_id; + + conversation_service::soft_delete(&db.conn, parent) + .await + .expect("delete parent"); + let done = report("t0", child, "done", TaskStatus::Completed); + assert!(finish(&db.conn, parent, "t0", &done) + .await + .expect("finish after parent delete")); + assert!(mark_released(&db.conn, parent, "t0") + .await + .expect("release after parent delete")); + assert!( + conversation_service::restore_soft_deleted(&db.conn, parent, folder_id) + .await + .expect("restore parent") + ); + + let entry = lookup(&db.conn, parent, "t0") + .await + .expect("cold lookup") + .expect("restored entry"); + assert_eq!(entry.status, TaskStatus::Completed); + assert!(entry.released); + assert_eq!( + serde_json::to_value(&entry.report).expect("stored report"), + serde_json::to_value(&done).expect("expected report"), + ); + } + + #[tokio::test] + async fn incomplete_or_unreleased_source_rejection_does_not_reserve_slot() { + let db = fresh_in_memory_db().await; + let (parent, child) = conversations(&db).await; + admit(&db.conn, input("t0", parent, child, None, "first")) + .await + .expect("admit t0"); + finish( + &db.conn, + parent, + "t0", + &report("t0", child, "done", TaskStatus::Completed), + ) + .await + .expect("finish t0"); + assert!( + admit(&db.conn, input("t1", parent, child, Some("t0"), "next")) + .await + .is_err() + ); + mark_released(&db.conn, parent, "t0") + .await + .expect("release t0"); + assert!(matches!( + admit(&db.conn, input("t1", parent, child, Some("t0"), "next")) + .await + .expect("admit after release"), + AdmissionResult::New { .. } + )); + + mark_released(&db.conn, parent, "t1") + .await + .expect("release running t1"); + assert!( + admit(&db.conn, input("t2", parent, child, Some("t1"), "next")) + .await + .is_err() + ); + finish( + &db.conn, + parent, + "t1", + &report("t1", child, "done", TaskStatus::Completed), + ) + .await + .expect("finish t1"); + assert!(matches!( + admit(&db.conn, input("t2", parent, child, Some("t1"), "next")) + .await + .expect("admit after finish"), + AdmissionResult::New { .. } + )); + } + + #[tokio::test] + async fn continuation_admission_and_child_pointer_commit_or_rollback_together() { + let db = fresh_in_memory_db().await; + let (parent, child) = conversations(&db).await; + admit(&db.conn, input("t0", parent, child, None, "first")) + .await + .unwrap(); + finish( + &db.conn, + parent, + "t0", + &report("t0", child, "done", TaskStatus::Completed), + ) + .await + .unwrap(); + mark_released(&db.conn, parent, "t0").await.unwrap(); + conversation_service::advance_delegation_call_id(&db.conn, child, "t0", "taken") + .await + .unwrap(); + + assert!( + admit_continuation(&db.conn, input("t1", parent, child, Some("t0"), "next")) + .await + .is_err() + ); + assert!(successor(&db.conn, parent, "t0").await.unwrap().is_none()); + let row = conversation::Entity::find_by_id(child) + .one(&db.conn) + .await + .unwrap() + .unwrap(); + assert_eq!(row.delegation_call_id.as_deref(), Some("taken")); + + conversation_service::advance_delegation_call_id(&db.conn, child, "taken", "t0") + .await + .unwrap(); + assert!(matches!( + admit_continuation(&db.conn, input("t1", parent, child, Some("t0"), "next")) + .await + .unwrap(), + AdmissionResult::New { .. } + )); + let row = conversation::Entity::find_by_id(child) + .one(&db.conn) + .await + .unwrap() + .unwrap(); + assert_eq!(row.delegation_call_id.as_deref(), Some("t1")); + } + + #[tokio::test] + async fn source_slot_race_returns_one_existing_successor() { + let db = fresh_in_memory_db().await; + let (parent, child) = conversations(&db).await; + admit(&db.conn, input("t0", parent, child, None, "first")) + .await + .expect("admit source"); + let done = report("t0", child, "done", TaskStatus::Completed); + finish(&db.conn, parent, "t0", &done).await.expect("finish"); + mark_released(&db.conn, parent, "t0") + .await + .expect("release"); + let left = input("t1-left", parent, child, Some("t0"), "next"); + let mut right = left.clone(); + right.task_id = "t1-right".into(); + let (left, right) = tokio::join!(admit(&db.conn, left), admit(&db.conn, right)); + let mut new_count = 0; + let mut existing_count = 0; + let ids = [left, right] + .into_iter() + .map(|result| match result.expect("admission") { + AdmissionResult::New { entry } => { + new_count += 1; + entry.task_id + } + AdmissionResult::Existing { entry } => { + existing_count += 1; + entry.task_id + } + AdmissionResult::Conflict { .. } => panic!("same key must be idempotent"), + }) + .collect::>(); + assert_eq!(new_count, 1); + assert_eq!(existing_count, 1); + assert_eq!(ids[0], ids[1]); + assert!(successor(&db.conn, parent, "t0") + .await + .expect("successor") + .is_some()); + } + + #[tokio::test] + async fn different_successor_task_is_a_conflict_with_next_id() { + let db = fresh_in_memory_db().await; + let (parent, child) = conversations(&db).await; + admit(&db.conn, input("t0", parent, child, None, "first")) + .await + .expect("admit source"); + finish( + &db.conn, + parent, + "t0", + &report("t0", child, "done", TaskStatus::Completed), + ) + .await + .expect("finish"); + mark_released(&db.conn, parent, "t0") + .await + .expect("release"); + admit(&db.conn, input("t1", parent, child, Some("t0"), "next")) + .await + .expect("first successor"); + let different = input("t2", parent, child, Some("t0"), "different"); + let result = admit(&db.conn, different).await.expect("conflict"); + assert!(matches!( + result, + AdmissionResult::Conflict { next_task_id, .. } if next_task_id == "t1" + )); + } + + #[tokio::test] + async fn physical_child_delete_cascades_ledger_row() { + let db = fresh_in_memory_db().await; + let (parent, child) = conversations(&db).await; + admit(&db.conn, input("t0", parent, child, None, "first")) + .await + .expect("admit"); + assert!(delegation_task::Entity::find() + .filter(delegation_task::Column::TaskId.eq("t0")) + .one(&db.conn) + .await + .expect("ledger lookup") + .is_some()); + + conversation::Entity::delete_by_id(child) + .exec(&db.conn) + .await + .expect("physical child delete"); + assert!(delegation_task::Entity::find() + .filter(delegation_task::Column::TaskId.eq("t0")) + .one(&db.conn) + .await + .expect("ledger lookup after cascade") + .is_none()); + } + + #[tokio::test] + async fn continuation_must_keep_the_source_session_binding() { + let db = fresh_in_memory_db().await; + let (parent, child) = conversations(&db).await; + admit(&db.conn, input("t0", parent, child, None, "first")) + .await + .expect("admit source"); + finish( + &db.conn, + parent, + "t0", + &report("t0", child, "done", TaskStatus::Completed), + ) + .await + .expect("finish"); + mark_released(&db.conn, parent, "t0") + .await + .expect("release"); + let mut different_session = input("t1", parent, child, Some("t0"), "next"); + different_session.resume_binding.external_session_id = "other".into(); + let error = admit(&db.conn, different_session) + .await + .expect_err("different session must be refused"); + assert!(error.to_string().contains("binding")); + assert!(successor(&db.conn, parent, "t0") + .await + .expect("successor lookup") + .is_none()); + } + + #[tokio::test] + async fn release_and_finish_are_independent_in_both_orders() { + let db = fresh_in_memory_db().await; + let (parent, child) = conversations(&db).await; + for (task_id, finish_first) in [("t0", true), ("t1", false)] { + admit(&db.conn, input(task_id, parent, child, None, "task")) + .await + .expect("admit"); + if finish_first { + finish( + &db.conn, + parent, + task_id, + &report(task_id, child, "done", TaskStatus::Completed), + ) + .await + .expect("finish"); + mark_released(&db.conn, parent, task_id) + .await + .expect("release"); + } else { + mark_released(&db.conn, parent, task_id) + .await + .expect("release"); + finish( + &db.conn, + parent, + task_id, + &report(task_id, child, "done", TaskStatus::Completed), + ) + .await + .expect("finish"); + } + let entry = lookup(&db.conn, parent, task_id) + .await + .expect("lookup") + .expect("entry"); + assert!(entry.released); + assert_eq!(entry.status, TaskStatus::Completed); + } + } +} diff --git a/src-tauri/src/db/service/mod.rs b/src-tauri/src/db/service/mod.rs index 49cbe4e877..9ea81424fb 100644 --- a/src-tauri/src/db/service/mod.rs +++ b/src-tauri/src/db/service/mod.rs @@ -6,6 +6,7 @@ pub mod chat_channel_message_log_service; pub mod chat_channel_service; pub mod conversation_service; pub mod custom_agent_service; +pub mod delegation_task_service; pub mod folder_command_service; pub mod folder_group_service; pub mod folder_link_service; diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 1307db5cb5..311e6d1390 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -790,6 +790,7 @@ mod tauri_app { let session_info_for_init = session_info_config.clone(); let chat_authoring_for_init = chat_authoring_config.clone(); tauri::async_runtime::block_on(async move { + crate::app_state::reconcile_interrupted_delegations(&db_for_init).await; delegation_commands::apply_persisted_config( &db_for_init, &broker_for_init, diff --git a/src-tauri/src/web/handlers/conversations.rs b/src-tauri/src/web/handlers/conversations.rs index a46a20fc1e..b59933a4c6 100644 --- a/src-tauri/src/web/handlers/conversations.rs +++ b/src-tauri/src/web/handlers/conversations.rs @@ -146,6 +146,7 @@ pub async fn get_folder_conversation( let result = conv_commands::get_folder_conversation_with_live_core( &db.conn, &state.connection_manager, + &state.delegation_broker, &state.chat_channel_manager, &state.emitter, params.conversation_id, @@ -169,6 +170,7 @@ pub async fn get_folder_conversation_turns( ) -> Result, AppCommandError> { let result = conv_commands::get_folder_conversation_turns_core( &state.db.conn, + Some(&state.delegation_broker), params.conversation_id, params.before_index, params.limit, diff --git a/src-tauri/tests/fixtures/bounded_continuation_agent.py b/src-tauri/tests/fixtures/bounded_continuation_agent.py new file mode 100644 index 0000000000..1726a19920 --- /dev/null +++ b/src-tauri/tests/fixtures/bounded_continuation_agent.py @@ -0,0 +1,122 @@ +"""Deterministic ACP peer for strict continuation and ordering tests.""" + +import json +import sys + + +mode, log_path = sys.argv[1:] +turn = 0 + + +def send(message): + sys.stdout.write(json.dumps(message) + "\n") + + +for line in sys.stdin: + message = json.loads(line) + method = message.get("method") + if not method: + continue + params = message.get("params", {}) + with open(log_path, "a", encoding="utf-8") as log: + log.write( + json.dumps( + { + "method": method, + "params": { + key: params[key] + for key in ( + "sessionId", + "modeId", + "configId", + "value", + "prompt", + ) + if key in params + }, + } + ) + + "\n" + ) + + result = {} + error = None + if method == "initialize": + result = { + "protocolVersion": 1, + "agentCapabilities": { + "loadSession": mode == "load_fail", + "sessionCapabilities": {} if mode == "unsupported" else {"resume": {}}, + }, + "authMethods": [], + } + elif method == "session/resume": + if mode == "load_fail": + error = {"code": -32601, "message": "injected resume failure"} + else: + result = { + "modes": { + "currentModeId": "default", + "availableModes": [ + {"id": "default", "name": "Default"}, + {"id": "plan", "name": "Plan"}, + ], + }, + "configOptions": [ + { + "type": "select", + "id": "model", + "name": "Model", + "currentValue": "default-model", + "options": [ + {"value": "default-model", "name": "Default"}, + {"value": "source-model", "name": "Source"}, + ], + } + ], + } + elif method == "session/load": + error = {"code": -32603, "message": "injected load failure"} + elif method == "session/new": + result = {"sessionId": "unexpected-new-session"} + elif method == "session/set_config_option": + result = { + "configOptions": [ + { + "type": "select", + "id": "model", + "name": "Model", + "currentValue": "source-model", + "options": [{"value": "source-model", "name": "Source"}], + } + ] + } + elif method == "session/prompt": + turn += 1 + session_id = params["sessionId"] + notification = { + "jsonrpc": "2.0", + "method": "session/update", + "params": { + "sessionId": session_id, + "update": { + "sessionUpdate": "agent_message_chunk", + "content": {"type": "text", "text": f"immediate reply {turn}"}, + }, + }, + } + response = { + "jsonrpc": "2.0", + "id": message["id"], + "result": {"stopReason": "end_turn"}, + } + # One write and one flush makes both items ready together at the host. + sys.stdout.write(json.dumps(notification) + "\n" + json.dumps(response) + "\n") + sys.stdout.flush() + continue + + if "id" in message: + response = {"jsonrpc": "2.0", "id": message["id"]} + response["error" if error else "result"] = error if error else result + send(response) + sys.stdout.flush() diff --git a/src/components/message/delegation-status-badge.tsx b/src/components/message/delegation-status-badge.tsx index 046e303448..165edaf74d 100644 --- a/src/components/message/delegation-status-badge.tsx +++ b/src/components/message/delegation-status-badge.tsx @@ -101,6 +101,8 @@ function ErrorLabel({ code }: { code?: string }) { return <>{t("child_empty")} case "child_auth_required": return <>{t("child_auth_required")} + case "interrupted": + return <>{t("interrupted")} case "child_unknown": return <>{t("child_unknown")} case "unknown": diff --git a/src/components/message/delegation-status-group-card.test.tsx b/src/components/message/delegation-status-group-card.test.tsx index 9754a8a23c..f3bf8fff1a 100644 --- a/src/components/message/delegation-status-group-card.test.tsx +++ b/src/components/message/delegation-status-group-card.test.tsx @@ -623,4 +623,35 @@ describe("DelegationStatusGroupCard", () => { expect(screen.getByText("Final result")).toBeInTheDocument() expect(screen.getByText("3 / 3")).toBeInTheDocument() }) + + it("keeps results separate for different tasks sharing one child", () => { + renderWithIntl( + + ) + expect(screen.getAllByText("done")).toHaveLength(2) + const buttons = screen.getAllByRole("button") + expect(buttons).toHaveLength(2) + for (const button of buttons) fireEvent.click(button) + expect(screen.getByText("Original review result")).toBeInTheDocument() + expect(screen.getByText("Follow-up review result")).toBeInTheDocument() + }) }) diff --git a/src/i18n/messages/ar.json b/src/i18n/messages/ar.json index a9dc1d5b86..b0cd3497c4 100644 --- a/src/i18n/messages/ar.json +++ b/src/i18n/messages/ar.json @@ -3379,6 +3379,7 @@ "child_max_turn_requests": "الوكيل الفرعي: حد الطلبات", "child_empty": "الوكيل الفرعي بدون استجابة", "child_auth_required": "الوكيل الفرعي يحتاج تسجيل دخول", + "interrupted": "تمت المقاطعة؛ النتيجة غير معروفة", "child_unknown": "الوكيل الفرعي: خطأ", "unknown": "مهمة غير معروفة" } diff --git a/src/i18n/messages/de.json b/src/i18n/messages/de.json index 02e9ff7bc8..761f32d399 100644 --- a/src/i18n/messages/de.json +++ b/src/i18n/messages/de.json @@ -3379,6 +3379,7 @@ "child_max_turn_requests": "Subagent: Anfragelimit", "child_empty": "Subagent ohne Ausgabe", "child_auth_required": "Subagent braucht Anmeldung", + "interrupted": "unterbrochen; Ergebnis unbekannt", "child_unknown": "Subagent: Fehler", "unknown": "unbekannte Aufgabe" } diff --git a/src/i18n/messages/en.json b/src/i18n/messages/en.json index a335deff38..0f2478aae2 100644 --- a/src/i18n/messages/en.json +++ b/src/i18n/messages/en.json @@ -3379,6 +3379,7 @@ "child_max_turn_requests": "subagent request limit", "child_empty": "subagent no output", "child_auth_required": "subagent needs sign-in", + "interrupted": "Interrupted; outcome unknown", "child_unknown": "subagent unknown error", "unknown": "unknown task" } diff --git a/src/i18n/messages/es.json b/src/i18n/messages/es.json index 06211ca06a..2031ba3dd8 100644 --- a/src/i18n/messages/es.json +++ b/src/i18n/messages/es.json @@ -3379,6 +3379,7 @@ "child_max_turn_requests": "subagente: límite de solicitudes", "child_empty": "subagente sin salida", "child_auth_required": "subagente requiere sesión", + "interrupted": "interrumpido; resultado desconocido", "child_unknown": "subagente: error", "unknown": "tarea desconocida" } diff --git a/src/i18n/messages/fr.json b/src/i18n/messages/fr.json index 5fe613634c..d5b8364765 100644 --- a/src/i18n/messages/fr.json +++ b/src/i18n/messages/fr.json @@ -3379,6 +3379,7 @@ "child_max_turn_requests": "sous-agent : limite de requêtes", "child_empty": "sous-agent sans sortie", "child_auth_required": "sous-agent : connexion requise", + "interrupted": "interrompu ; résultat inconnu", "child_unknown": "sous-agent : erreur", "unknown": "tâche inconnue" } diff --git a/src/i18n/messages/ja.json b/src/i18n/messages/ja.json index 6619102325..534e9e8132 100644 --- a/src/i18n/messages/ja.json +++ b/src/i18n/messages/ja.json @@ -3379,6 +3379,7 @@ "child_max_turn_requests": "サブエージェント: 要求上限", "child_empty": "サブエージェント応答なし", "child_auth_required": "サブエージェント要サインイン", + "interrupted": "中断済み（結果不明）", "child_unknown": "サブエージェント: エラー", "unknown": "不明なタスク" } diff --git a/src/i18n/messages/ko.json b/src/i18n/messages/ko.json index c9cee2d063..3d40fabab6 100644 --- a/src/i18n/messages/ko.json +++ b/src/i18n/messages/ko.json @@ -3379,6 +3379,7 @@ "child_max_turn_requests": "서브에이전트: 요청 한도", "child_empty": "서브에이전트 응답 없음", "child_auth_required": "서브에이전트 로그인 필요", + "interrupted": "중단됨, 결과 알 수 없음", "child_unknown": "서브에이전트: 오류", "unknown": "알 수 없는 작업" } diff --git a/src/i18n/messages/pt.json b/src/i18n/messages/pt.json index 3de179b348..5d081adf98 100644 --- a/src/i18n/messages/pt.json +++ b/src/i18n/messages/pt.json @@ -3379,6 +3379,7 @@ "child_max_turn_requests": "subagente: limite de solicitações", "child_empty": "subagente sem saída", "child_auth_required": "subagente exige login", + "interrupted": "interrompido; resultado desconhecido", "child_unknown": "subagente: erro", "unknown": "tarefa desconhecida" } diff --git a/src/i18n/messages/zh-CN.json b/src/i18n/messages/zh-CN.json index ad4a6b31bb..be9e617273 100644 --- a/src/i18n/messages/zh-CN.json +++ b/src/i18n/messages/zh-CN.json @@ -3379,6 +3379,7 @@ "child_max_turn_requests": "子代理超出请求上限", "child_empty": "子代理无响应", "child_auth_required": "子智能体需登录", + "interrupted": "已中断，结果未知", "child_unknown": "子代理未知错误", "unknown": "未知任务" } diff --git a/src/i18n/messages/zh-TW.json b/src/i18n/messages/zh-TW.json index 8d5324d66d..cdca32032d 100644 --- a/src/i18n/messages/zh-TW.json +++ b/src/i18n/messages/zh-TW.json @@ -3379,6 +3379,7 @@ "child_max_turn_requests": "子代理超出請求上限", "child_empty": "子代理無回應", "child_auth_required": "子智慧體需登入", + "interrupted": "已中斷，結果未知", "child_unknown": "子代理未知錯誤", "unknown": "未知任務" } diff --git a/src/lib/delegation-status.test.ts b/src/lib/delegation-status.test.ts index d8fb998783..b558fcafa5 100644 --- a/src/lib/delegation-status.test.ts +++ b/src/lib/delegation-status.test.ts @@ -575,6 +575,17 @@ describe("deriveBadge", () => { }) }) + it("preserves the interrupted code from an unknown status report", () => { + const report = parseStatusReport( + envelope({ task_id: "x", status: "unknown", error_code: "interrupted" }), + null + ) + expect(deriveBadge("status", report, "output-available", false)).toEqual({ + status: "err", + errorCode: "interrupted", + }) + }) + it("treats canceled as success for cancel, terminal error for a status query", () => { const report = parseStatusReport( envelope({ task_id: "x", status: "canceled", error_code: "canceled" }), diff --git a/src/lib/delegation-status.ts b/src/lib/delegation-status.ts index d9936e7557..260a3fddd4 100644 --- a/src/lib/delegation-status.ts +++ b/src/lib/delegation-status.ts @@ -581,7 +581,7 @@ export function deriveBadge( return { status: report.blockedOn ? "waiting" : "checked" } case "unknown": // Terminal "task id not known" — surface as error, not an endless spinner. - return { status: "err", errorCode: "unknown" } + return { status: "err", errorCode: report.errorCode ?? "unknown" } case "failed": return { status: "err", errorCode: report.errorCode ?? undefined } case "canceled":