From ab2e69575668bf22e0a8165d520d00b5628e540a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Byjo=C5=9B?= Date: Tue, 10 Nov 2026 22:11:31 +0100 Subject: [PATCH] feat(acp): let codeg-server own agent lifecycle instead of the browser tab On a server deployment used from several devices, agents died whenever the user looked away: a phone freezes the page when the screen locks, the 30 s keepalive stops, and the 180 s idle sweep ends every idle agent. Closing a tab on any device also ended agents another device was using. - Idle policy per runtime: desktop unchanged (180 s, no cap); the server keeps idle agents for 2 h, bounded by a cap of 8 idle agents (longest idle and unwatched first). Headless owners (automation, codeg-cli, one-shot) keep the short timeout. New CODEG_ACP_MAX_IDLE_AGENTS. - In web mode closing a view, replacing a preview tab, the frontend idle sweep and the provider unmount detach the view instead of calling acp_disconnect. - A conversation whose agent went away while it was open resumes its session on its own once the page is visible (at most once a minute). - A pending ScheduleWakeup counts as outstanding background work until it fires, is stopped, or passes its deadline plus 5 minutes. Co-Authored-By: Claude Opus 5.5 --- src-tauri/src/acp/background_watch.rs | 205 +++++++++++++++- src-tauri/src/acp/idle_sweep.rs | 227 ++++++++++++++---- src-tauri/src/acp/manager.rs | 196 +++++++++++++-- src-tauri/src/acp/mod.rs | 2 +- src-tauri/src/bin/codeg_server.rs | 16 +- src-tauri/src/lib.rs | 14 +- .../tab-first-show-auto-connect.test.tsx | 5 + src/contexts/acp-connections-context.test.tsx | 63 +++++ src/contexts/acp-connections-context.tsx | 61 ++++- src/contexts/workspace-context.test.tsx | 2 + ...-connection-lifecycle.agent-switch.test.ts | 7 + ...connection-lifecycle.connect-error.test.ts | 7 + .../use-connection-lifecycle.opening.test.ts | 7 + ...-connection-lifecycle.send-failure.test.ts | 7 + src/hooks/use-connection-lifecycle.ts | 66 ++++- .../use-connection-lifecycle.unmount.test.ts | 166 ++++++++++++- src/hooks/use-connection.ts | 10 + src/lib/constants.ts | 4 + src/lib/platform.ts | 16 ++ 19 files changed, 985 insertions(+), 96 deletions(-) diff --git a/src-tauri/src/acp/background_watch.rs b/src-tauri/src/acp/background_watch.rs index d064da776b6..9c0decfebad 100644 --- a/src-tauri/src/acp/background_watch.rs +++ b/src-tauri/src/acp/background_watch.rs @@ -101,6 +101,14 @@ const RECENT_ACTIVITY_WINDOW: Duration = Duration::from_secs(30); /// never-persisted prompt must not linger and swallow a later cron re-fire of /// the same text. const LEDGER_TTL: Duration = Duration::from_secs(600); +/// `ScheduleWakeup` holds one wakeup per session: a later call replaces the +/// pending one, so it is counted under this fixed id rather than a task id +/// (its ack carries none). +const SCHEDULED_WAKEUP_TASK_ID: &str = "scheduled-wakeup"; +/// How long past its scheduled time a wakeup stays counted. The CLI rounds +/// the time to the minute and fires on its own timer tick (observed up to +/// ~20 s late); a wakeup that never fires must not pin the connection. +const WAKEUP_GRACE: Duration = Duration::from_secs(5 * 60); /// Max fingerprints kept (oldest evicted first). Far above any realistic /// number of prompts in flight between transcript flushes. const LEDGER_CAP: usize = 32; @@ -757,10 +765,14 @@ async fn run_watch( } } -/// One launched-but-unresolved background task. +/// One launched-but-unresolved background task — or work the CLI holds for +/// later, like a pending `ScheduleWakeup`. struct TaskEntry { kind: &'static str, started_at: Instant, + /// How long after `started_at` the entry expires. `None` = the background + /// keep-alive max age; a scheduled wakeup knows its own deadline. + ttl: Option, } /// The current out-of-turn episode: a contiguous run of transcript records @@ -888,6 +900,12 @@ pub(crate) struct WatchState { turn_origin_task_ids: HashMap>, last_disk_activity: Option, last_emitted_outstanding: Option, + /// When the last `BackgroundActivity` went out. While work is pending the + /// count is re-sent at least every half max age, so the connection's + /// sweep exemption — which lapses one max age after the last event — holds + /// for as long as a live watcher still counts the work (a wakeup can be + /// scheduled close to an hour out, past that window). + last_emitted_at: Option, armed_logged: bool, /// Base of the most recently created episode's id namespace. Episode /// bases must be STRICTLY increasing: two episodes created while @@ -957,6 +975,7 @@ impl WatchState { // so the first tick must not emit an accounting-only event for a // connection with no background work. last_emitted_outstanding: Some(0), + last_emitted_at: None, armed_logged: false, last_episode_base: 0, epoch: None, @@ -1069,7 +1088,9 @@ impl WatchState { let recently_active = self .last_disk_activity .is_some_and(|at| at.elapsed() < RECENT_ACTIVITY_WINDOW); - if !self.tasks.is_empty() || recently_active { + // A pending wakeup alone waits on a known clock; only running work + // needs the tight cadence to surface its completion promptly. + if self.tasks.values().any(|t| t.kind != "wakeup") || recently_active { POLL_ACTIVE } else { POLL_IDLE @@ -1144,7 +1165,7 @@ impl WatchState { .unwrap_or(Duration::from_secs(3600)); let before = self.tasks.len(); self.tasks.retain(|id, t| { - let keep = t.started_at.elapsed() < max_age; + let keep = t.started_at.elapsed() < t.ttl.unwrap_or(max_age); if !keep { tracing::info!( "[bg-watch] expiring {} task={id} after max-age (completion never observed)", @@ -1309,12 +1330,18 @@ impl WatchState { }); let outstanding = self.tasks.len() as u32; + let heartbeat_due = outstanding > 0 + && !max_age.is_zero() + && self + .last_emitted_at + .is_some_and(|at| at.elapsed() >= max_age / 2); let accounting_changed = - expired_any || self.last_emitted_outstanding != Some(outstanding); + expired_any || heartbeat_due || self.last_emitted_outstanding != Some(outstanding); if changed_turns.is_empty() && settled.is_empty() && !accounting_changed { return None; } self.last_emitted_outstanding = Some(outstanding); + self.last_emitted_at = Some(Instant::now()); Some(AcpEvent::BackgroundActivity { session_id, turns: changed_turns, @@ -1455,10 +1482,37 @@ impl WatchState { .or_insert_with(|| TaskEntry { kind: "agent", started_at: Instant::now(), + ttl: None, }); tracing::info!("[bg-watch] interim notification task={id} known={known}, kept outstanding"); } + /// Count a pending `ScheduleWakeup` firing at `scheduled_for_ms` (epoch + /// ms) until it fires, is stopped, or is [`WAKEUP_GRACE`] overdue. A later + /// call replaces the earlier one, as the CLI does. One already that far + /// overdue (a record read long after it was written) is not counted. + fn register_wakeup(&mut self, scheduled_for_ms: u64) { + let now_ms = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |d| d.as_millis() as u64); + let ttl = Duration::from_millis(scheduled_for_ms.saturating_sub(now_ms)) + WAKEUP_GRACE; + if scheduled_for_ms.saturating_add(WAKEUP_GRACE.as_millis() as u64) <= now_ms { + return; + } + tracing::info!( + "[bg-watch] scheduled wakeup registered, fires in {}s", + scheduled_for_ms.saturating_sub(now_ms) / 1000 + ); + self.tasks.insert( + SCHEDULED_WAKEUP_TASK_ID.to_string(), + TaskEntry { + kind: "wakeup", + started_at: Instant::now(), + ttl: Some(ttl), + }, + ); + } + /// Task accounting for one record: launch acks; settlements via a /// `` record or one folded into a running turn, a /// `TaskOutput` result reaching a terminal `task.status`, or a @@ -1491,7 +1545,23 @@ impl WatchState { } } "user" => { + // A scheduled wakeup fired: the CLI re-sent its prompt as a + // turn of its own. Any later `ScheduleWakeup` re-arms it. + if value.get("turnOrigin").and_then(|o| o.as_str()) == Some("scheduled") + && self.tasks.remove(SCHEDULED_WAKEUP_TASK_ID).is_some() + { + tracing::info!("[bg-watch] scheduled wakeup fired"); + } if let Some(tur) = value.get("toolUseResult") { + // A `ScheduleWakeup` ack: the CLI will re-send a prompt at + // `scheduledFor` (epoch ms). Ending the CLI before then + // loses it, so it counts as pending work. + if let (Some(at_ms), Some(_)) = ( + tur.get("scheduledFor").and_then(|v| v.as_u64()), + tur.get("clampedDelaySeconds"), + ) { + self.register_wakeup(at_ms); + } if tur.get("status").and_then(|s| s.as_str()) == Some("async_launched") { if let Some(id) = tur .get("agentId") @@ -1511,6 +1581,7 @@ impl WatchState { TaskEntry { kind: "agent", started_at: Instant::now(), + ttl: None, } }); // This turn is still `Prompting` at launch time — a @@ -1537,6 +1608,7 @@ impl WatchState { TaskEntry { kind: "shell", started_at: Instant::now(), + ttl: None, } }); // Deliberately NOT inserted into `current_turn_launched_ids`: @@ -1666,6 +1738,7 @@ impl WatchState { TaskEntry { kind: "agent", started_at: Instant::now(), + ttl: None, }, ); // Mirrors the launch-time insert in the @@ -1681,6 +1754,14 @@ impl WatchState { } } } + // `stop: true` ends the dynamic loop: no wakeup follows. + "ScheduleWakeup" => { + let stop = input.and_then(|i| i.get("stop")).and_then(|v| v.as_bool()) + == Some(true); + if stop && self.tasks.remove(SCHEDULED_WAKEUP_TASK_ID).is_some() { + tracing::info!("[bg-watch] scheduled wakeup stopped"); + } + } // Explicit kill: the background task's process is gone, // so it must leave the outstanding count now — no // completion notification will follow. `TaskStop` names @@ -2243,6 +2324,33 @@ mod tests { ) } + fn epoch_ms_in(offset_secs: i64) -> u64 { + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_millis() as i64; + (now + offset_secs * 1000) as u64 + } + + /// Real-shape `ScheduleWakeup` ack: the tool result plus a structured + /// `toolUseResult` naming when the CLI will re-send the prompt. + fn wakeup_ack(scheduled_for_ms: u64) -> String { + format!( + r#"{{"type":"user","timestamp":"2026-07-07T03:46:20.000Z","uuid":"u-wake-{scheduled_for_ms}","message":{{"role":"user","content":[{{"tool_use_id":"toolu_wake","type":"tool_result","content":"Next wakeup scheduled for 19:11:00 (in 1214s)."}}]}},"toolUseResult":{{"scheduledFor":{scheduled_for_ms},"clampedDelaySeconds":1200,"wasClamped":false}}}}"# + ) + } + + /// Real-shape fired wakeup: the prompt re-sent as a meta user record. + fn wakeup_fired(text: &str) -> String { + format!( + r#"{{"type":"user","timestamp":"2026-07-07T04:06:00.000Z","uuid":"u-fired","isMeta":true,"promptSource":"system","turnOrigin":"scheduled","wakeupSource":"loop_wakeup","message":{{"role":"user","content":"{text}"}}}}"# + ) + } + + fn wakeup_stop() -> String { + r#"{"type":"assistant","timestamp":"2026-07-07T03:47:40.000Z","uuid":"a-wake-stop","message":{"role":"assistant","content":[{"type":"tool_use","id":"toolu_wake_stop","name":"ScheduleWakeup","input":{"stop":true}}]}}"#.to_string() + } + /// Real-shape cron-fired prompt: `isMeta:true` with bare STRING content. fn cron_prompt(text: &str) -> String { format!( @@ -4433,6 +4541,95 @@ mod tests { ); } + /// A pending `ScheduleWakeup` lives only in the CLI process: ending the + /// connection before it fires loses it, so it counts as outstanding work. + #[test] + fn a_pending_wakeup_counts_as_outstanding_until_it_fires() { + let dir = tempfile::tempdir().unwrap(); + let path = temp_session(&dir); + write_lines(&path, &[&wakeup_ack(epoch_ms_in(20 * 60))]); + let ledger = PromptLedger::shared(); + let mut ws = WatchState::with_file_for_test("s1", path.clone()); + let (_, outstanding, ..) = unpack(tick_now(&mut ws, &ledger).expect("ack event")); + assert_eq!(outstanding, 1, "ending the CLI now would lose the wakeup"); + + write_lines(&path, &[&wakeup_fired("Check CI again")]); + let (_, outstanding, ..) = unpack(tick_now(&mut ws, &ledger).expect("fire event")); + assert_eq!(outstanding, 0); + } + + #[test] + fn a_later_wakeup_replaces_the_pending_one() { + let dir = tempfile::tempdir().unwrap(); + let path = temp_session(&dir); + write_lines( + &path, + &[ + &wakeup_ack(epoch_ms_in(10 * 60)), + &wakeup_ack(epoch_ms_in(30 * 60)), + ], + ); + let ledger = PromptLedger::shared(); + let mut ws = WatchState::with_file_for_test("s1", path.clone()); + let (_, outstanding, ..) = unpack(tick_now(&mut ws, &ledger).expect("ack event")); + assert_eq!(outstanding, 1, "one wakeup slot per session"); + } + + #[test] + fn stopping_the_loop_clears_the_pending_wakeup() { + let dir = tempfile::tempdir().unwrap(); + let path = temp_session(&dir); + write_lines(&path, &[&wakeup_ack(epoch_ms_in(20 * 60))]); + let ledger = PromptLedger::shared(); + let mut ws = WatchState::with_file_for_test("s1", path.clone()); + unpack(tick_now(&mut ws, &ledger).expect("ack event")); + + write_lines(&path, &[&wakeup_stop()]); + let (_, outstanding, ..) = unpack(tick_now(&mut ws, &ledger).expect("stop event")); + assert_eq!(outstanding, 0); + } + + #[test] + fn a_long_overdue_wakeup_is_not_counted() { + let dir = tempfile::tempdir().unwrap(); + let path = temp_session(&dir); + write_lines(&path, &[&wakeup_ack(epoch_ms_in(-60 * 60))]); + let ledger = PromptLedger::shared(); + let mut ws = WatchState::with_file_for_test("s1", path.clone()); + let outstanding = tick_now(&mut ws, &ledger).map_or(0, |e| unpack(e).1); + assert_eq!( + outstanding, 0, + "a record read long after the fact pins nothing" + ); + } + + #[test] + fn a_wakeup_that_never_fires_expires_on_its_own_deadline() { + let dir = tempfile::tempdir().unwrap(); + let path = temp_session(&dir); + write_lines(&path, &[&wakeup_ack(epoch_ms_in(20 * 60))]); + let ledger = PromptLedger::shared(); + let mut ws = WatchState::with_file_for_test("s1", path.clone()); + unpack(tick_now(&mut ws, &ledger).expect("ack event")); + + // Its deadline (scheduled time + grace) passes, well before max age. + ws.tasks.get_mut(SCHEDULED_WAKEUP_TASK_ID).unwrap().ttl = Some(Duration::ZERO); + let (_, outstanding, ..) = unpack(tick_now(&mut ws, &ledger).expect("expiry event")); + assert_eq!(outstanding, 0); + } + + #[test] + fn a_pending_wakeup_alone_keeps_the_idle_poll_cadence() { + let dir = tempfile::tempdir().unwrap(); + let path = temp_session(&dir); + write_lines(&path, &[&wakeup_ack(epoch_ms_in(20 * 60))]); + let ledger = PromptLedger::shared(); + let mut ws = WatchState::with_file_for_test("s1", path.clone()); + unpack(tick_now(&mut ws, &ledger).expect("ack event")); + ws.last_disk_activity = None; + assert_eq!(ws.poll_delay(), POLL_IDLE); + } + /// An explicit `TaskStop` settles the task immediately — the process is /// gone and no completion notification will follow. #[test] diff --git a/src-tauri/src/acp/idle_sweep.rs b/src-tauri/src/acp/idle_sweep.rs index 0d326421216..5435e563ff8 100644 --- a/src-tauri/src/acp/idle_sweep.rs +++ b/src-tauri/src/acp/idle_sweep.rs @@ -1,42 +1,142 @@ -//! Periodic sweeper that disconnects ACP connections idle past a deadline. +//! Periodic sweeper that puts idle ACP connections to sleep. //! -//! Connections accumulate when frontends close their window/tab without -//! triggering an explicit disconnect — common in web mode (browser tab -//! close has no server-side hook), and possible on desktop after panics. -//! The sweep prevents long-lived processes from leaking ACP child -//! processes, file handles, and memory. +//! "Sleep" ends the agent CLI process but keeps its session: the +//! conversation stays where it was, and the next message resumes it with +//! `session/load`. What the sweep must never do is end work the user is +//! waiting for, so a connection is only ever a candidate while nothing is in +//! flight (see [`ConnectionManager::sweep_idle_with`] for the exemptions). +//! +//! Two runtimes, two policies: +//! +//! * **Desktop** — the agent lives as long as a window shows it. Every open +//! tab touches its connection every 30 s, so a connection nobody touched +//! for a few minutes belongs to a window that is gone, and is reclaimed. +//! * **Server** (`codeg-server` on a VM, used from several devices) — the +//! agent belongs to the server. A phone that locks its screen freezes the +//! page and stops touching; a laptop lid does the same. Neither means "I +//! am done with this agent", so a short timeout reads as agents dying +//! whenever the user looks away. The server keeps idle agents for hours +//! and bounds the cost with a cap on how many idle agents nobody is +//! watching stay warm, instead of with a clock tied to a browser tab. +//! +//! Connections no person composes prompts for — automation runs, `codeg-cli` +//! runs, one-shot generations — get the short timeout in both runtimes: no +//! tab will ever come back to them, and they must not hold cap slots that an +//! interactive conversation could use. use std::time::Duration; use crate::acp::manager::ConnectionManager; -/// Default idle threshold (3 minutes). Override at startup via -/// `CODEG_ACP_IDLE_TIMEOUT_SECS`. The sweep only runs against -/// connections in `Connected` state with no `pending_permission`, and -/// `last_activity_at` is bumped on every emit and on every frontend -/// keepalive touch (~30s cadence for open tabs), so an actively-used -/// or visible connection never qualifies. +/// Desktop idle threshold (3 minutes). Override via +/// `CODEG_ACP_IDLE_TIMEOUT_SECS`. `last_activity_at` is bumped on every emit +/// and on every frontend keepalive touch (~30 s cadence for open tabs), so a +/// visible connection never qualifies. pub const DEFAULT_IDLE_TIMEOUT_SECS: u64 = 180; +/// Server idle threshold (2 hours). Override via `CODEG_ACP_IDLE_TIMEOUT_SECS`. +pub const DEFAULT_SERVER_IDLE_TIMEOUT_SECS: u64 = 2 * 60 * 60; +/// Server cap on idle interactive agents kept warm. Override via +/// `CODEG_ACP_MAX_IDLE_AGENTS` (`0` = no cap). Each agent CLI is a process +/// tree (often with MCP servers of its own) worth hundreds of megabytes, so +/// an unbounded pool on a small VM is the thing this replaces the short +/// timeout with. +pub const DEFAULT_SERVER_MAX_IDLE_AGENTS: usize = 8; +/// Upper bound of the idle threshold for connections no person composes +/// prompts for (automation, `codeg-cli`, one-shot). +pub const HEADLESS_IDLE_TIMEOUT_SECS: u64 = DEFAULT_IDLE_TIMEOUT_SECS; +/// A connection touched within this window is being watched by an open tab +/// (tabs touch every 30 s; a desktop browser throttles a background tab's +/// timers to about once a minute). The cap never puts a watched agent to +/// sleep. +pub const WATCHED_WINDOW_SECS: u64 = 180; /// Sweep cadence — runs once per minute. Each tick is a brief lock on the /// connections map plus per-state `try_read`s, so a 1-minute interval is /// trivially cheap relative to the wall-clock idle threshold. pub const SWEEP_INTERVAL_SECS: u64 = 60; -/// Read the idle timeout from `CODEG_ACP_IDLE_TIMEOUT_SECS`, falling back -/// to `DEFAULT_IDLE_TIMEOUT_SECS`. A `0` value disables the sweep -/// (returns `None`); any unparseable value is treated as "use default". -pub fn idle_timeout_from_env() -> Option { - let secs = match std::env::var("CODEG_ACP_IDLE_TIMEOUT_SECS") { - Ok(raw) => raw.parse::().unwrap_or(DEFAULT_IDLE_TIMEOUT_SECS), - Err(_) => DEFAULT_IDLE_TIMEOUT_SECS, - }; - if secs == 0 { - return None; +/// Which runtime the sweep is running in; picks the defaults. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SweepRuntime { + Desktop, + Server, +} + +/// When the sweep puts an idle connection to sleep. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct IdleSweepPolicy { + /// Idle time after which a connection a person works in is put to sleep. + pub idle_timeout: Duration, + /// Idle time after which a connection nobody composes prompts for + /// (automation, `codeg-cli`, one-shot) is reclaimed. + pub headless_idle_timeout: Duration, + /// Most idle interactive connections kept warm. Past it, the ones idle + /// the longest are put to sleep early — but only those no open tab is + /// watching. `None` = no cap. + pub max_idle: Option, + /// See [`WATCHED_WINDOW_SECS`]. + pub watched_window: Duration, +} + +impl IdleSweepPolicy { + /// One timeout for every connection and no cap — the behavior before + /// runtimes had separate policies. Kept for tests and callers that only + /// care about the clock. + pub fn uniform(idle_timeout: Duration) -> Self { + Self { + idle_timeout, + headless_idle_timeout: idle_timeout, + max_idle: None, + watched_window: Duration::from_secs(WATCHED_WINDOW_SECS), + } + } + + /// Build the policy for `runtime` from the process environment. `None` + /// when `CODEG_ACP_IDLE_TIMEOUT_SECS=0` disables the sweep. + pub fn from_env(runtime: SweepRuntime) -> Option { + Self::from_values( + runtime, + std::env::var("CODEG_ACP_IDLE_TIMEOUT_SECS").ok().as_deref(), + std::env::var("CODEG_ACP_MAX_IDLE_AGENTS").ok().as_deref(), + ) + } + + /// Pure form of [`Self::from_env`]: unparseable values fall back to the + /// runtime's default, the same way a missing one does. + pub fn from_values( + runtime: SweepRuntime, + idle_timeout_secs: Option<&str>, + max_idle_agents: Option<&str>, + ) -> Option { + let (default_timeout, default_cap) = match runtime { + SweepRuntime::Desktop => (DEFAULT_IDLE_TIMEOUT_SECS, None), + SweepRuntime::Server => ( + DEFAULT_SERVER_IDLE_TIMEOUT_SECS, + Some(DEFAULT_SERVER_MAX_IDLE_AGENTS), + ), + }; + let secs = idle_timeout_secs + .and_then(|raw| raw.trim().parse::().ok()) + .unwrap_or(default_timeout); + if secs == 0 { + return None; + } + let max_idle = match max_idle_agents.and_then(|raw| raw.trim().parse::().ok()) { + Some(0) => None, + Some(n) => Some(n), + None => default_cap, + }; + let idle_timeout = Duration::from_secs(secs); + Some(Self { + idle_timeout, + headless_idle_timeout: idle_timeout + .min(Duration::from_secs(HEADLESS_IDLE_TIMEOUT_SECS)), + max_idle, + watched_window: Duration::from_secs(WATCHED_WINDOW_SECS), + }) } - Some(Duration::from_secs(secs)) } -/// Long-running task that calls `ConnectionManager::sweep_idle` on a +/// Long-running task that calls `ConnectionManager::sweep_idle_with` on a /// fixed interval. The caller spawns the returned future onto whichever /// runtime they manage (`tokio::spawn` from inside an async context, /// `tauri::async_runtime::spawn` from a Tauri `setup` callback that runs @@ -46,9 +146,17 @@ pub fn idle_timeout_from_env() -> Option { /// shutting down (process exit cleans up everything). pub async fn idle_sweep_task( manager: ConnectionManager, - idle_timeout: Duration, + policy: IdleSweepPolicy, interval: Duration, ) { + tracing::info!( + "[ACP] idle sweep: sleep after {}s idle (headless {}s), max idle agents {}", + policy.idle_timeout.as_secs(), + policy.headless_idle_timeout.as_secs(), + policy + .max_idle + .map_or_else(|| "unlimited".to_string(), |n| n.to_string()), + ); let mut ticker = tokio::time::interval(interval); ticker.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Delay); // First `tick().await` returns immediately. Skip it so we don't @@ -56,9 +164,9 @@ pub async fn idle_sweep_task( ticker.tick().await; loop { ticker.tick().await; - let n = manager.sweep_idle(idle_timeout).await; + let n = manager.sweep_idle_with(&policy).await; if n > 0 { - tracing::info!("[ACP] idle sweep disconnected {n} connection(s)"); + tracing::info!("[ACP] idle sweep put {n} connection(s) to sleep"); } } } @@ -67,32 +175,55 @@ pub async fn idle_sweep_task( mod tests { use super::*; - /// Single test sequences all env-var assertions to avoid the - /// notorious parallel-test race on shared environment state. Cargo - /// runs tests in parallel by default; setting `CODEG_ACP_IDLE_TIMEOUT_SECS` - /// in concurrent tests would interleave with each other. #[test] - fn idle_timeout_env_parsing() { - // Disabled when zero. - std::env::set_var("CODEG_ACP_IDLE_TIMEOUT_SECS", "0"); - assert!(idle_timeout_from_env().is_none()); + fn desktop_defaults_keep_the_three_minute_timeout_and_no_cap() { + let p = IdleSweepPolicy::from_values(SweepRuntime::Desktop, None, None).unwrap(); + assert_eq!(p.idle_timeout.as_secs(), DEFAULT_IDLE_TIMEOUT_SECS); + assert_eq!(p.headless_idle_timeout.as_secs(), DEFAULT_IDLE_TIMEOUT_SECS); + assert_eq!(p.max_idle, None); + } - // Falls back to default when unparseable. - std::env::set_var("CODEG_ACP_IDLE_TIMEOUT_SECS", "not-a-number"); + #[test] + fn server_defaults_keep_agents_for_hours_under_a_cap() { + let p = IdleSweepPolicy::from_values(SweepRuntime::Server, None, None).unwrap(); + assert_eq!(p.idle_timeout.as_secs(), DEFAULT_SERVER_IDLE_TIMEOUT_SECS); assert_eq!( - idle_timeout_from_env().unwrap().as_secs(), - DEFAULT_IDLE_TIMEOUT_SECS + p.headless_idle_timeout.as_secs(), + HEADLESS_IDLE_TIMEOUT_SECS ); + assert_eq!(p.max_idle, Some(DEFAULT_SERVER_MAX_IDLE_AGENTS)); + } - // Uses provided value when it parses. - std::env::set_var("CODEG_ACP_IDLE_TIMEOUT_SECS", "120"); - assert_eq!(idle_timeout_from_env().unwrap().as_secs(), 120); + #[test] + fn zero_timeout_disables_the_sweep_in_both_runtimes() { + for runtime in [SweepRuntime::Desktop, SweepRuntime::Server] { + assert!(IdleSweepPolicy::from_values(runtime, Some("0"), None).is_none()); + } + } - // Falls back to default when unset. - std::env::remove_var("CODEG_ACP_IDLE_TIMEOUT_SECS"); - assert_eq!( - idle_timeout_from_env().unwrap().as_secs(), - DEFAULT_IDLE_TIMEOUT_SECS - ); + #[test] + fn unparseable_values_fall_back_to_the_runtime_default() { + let p = + IdleSweepPolicy::from_values(SweepRuntime::Server, Some("soon"), Some("lots")).unwrap(); + assert_eq!(p.idle_timeout.as_secs(), DEFAULT_SERVER_IDLE_TIMEOUT_SECS); + assert_eq!(p.max_idle, Some(DEFAULT_SERVER_MAX_IDLE_AGENTS)); + } + + #[test] + fn explicit_values_override_and_zero_cap_means_unlimited() { + let p = IdleSweepPolicy::from_values(SweepRuntime::Server, Some("600"), Some("0")).unwrap(); + assert_eq!(p.idle_timeout.as_secs(), 600); + assert_eq!(p.max_idle, None); + + let p = + IdleSweepPolicy::from_values(SweepRuntime::Desktop, Some(" 120 "), Some("3")).unwrap(); + assert_eq!(p.idle_timeout.as_secs(), 120); + assert_eq!(p.max_idle, Some(3)); + } + + #[test] + fn headless_timeout_never_exceeds_the_interactive_one() { + let p = IdleSweepPolicy::from_values(SweepRuntime::Server, Some("60"), None).unwrap(); + assert_eq!(p.headless_idle_timeout.as_secs(), 60); } } diff --git a/src-tauri/src/acp/manager.rs b/src-tauri/src/acp/manager.rs index ce49c224589..13b5cfe3b87 100644 --- a/src-tauri/src/acp/manager.rs +++ b/src-tauri/src/acp/manager.rs @@ -20,6 +20,7 @@ use crate::acp::feedback::{ bounded_feedback_batch, FeedbackItem, FeedbackStatus, PendingFeedback, SessionFeedbackAccess, MAX_FEEDBACK_CHARS, MAX_FEEDBACK_RESPONSE_BYTES, }; +use crate::acp::idle_sweep::IdleSweepPolicy; use crate::acp::plan_approval::{ PlanApprovalAnswer, RegisteredPlanApproval, SessionPlanApprovalAccess, }; @@ -38,6 +39,16 @@ use crate::db::AppDatabase; use crate::models::agent::AgentType; use crate::web::event_bridge::{emit_with_state, emit_with_state_gated, EventEmitter}; +/// Owners no person composes prompts for: an automation run, a `codeg-cli` +/// run, a one-shot generation. No tab comes back to their connections, so +/// the idle sweep reclaims them on the short timeout and keeps them out of +/// the idle-agent cap (see `acp::idle_sweep`). +fn owner_is_headless(owner_window_label: &str) -> bool { + owner_window_label == "automation" + || owner_window_label == crate::cli_run::service::CLI_OWNER_LABEL + || crate::acp::one_shot::is_one_shot_owner(owner_window_label) +} + /// Cap on the number of prompt-text chars kept in the `user_prompt_sent` /// preview. Past this, `truncate_str` keeps this many chars and appends a short /// `...` marker (so the rendered string can be a few chars longer). Bounds the @@ -1042,24 +1053,47 @@ impl ConnectionManager { true } - /// Disconnect connections that have been idle longer than `idle_timeout`. - /// "Idle" means: status is `Connected`, no `pending_permission`, no - /// launched-but-unresolved background work (async sub-agent / background - /// shell — disconnecting kills the agent CLI and the background work with - /// it), no Claude Remote Control (it runs in the same CLI), no codeg - /// delegation whose child is still running (ending the parent cancels it), - /// and no activity (no events, no commands) for at least - /// `idle_timeout`. `Prompting` connections are always preserved (a turn is - /// in flight). Returns the number of connections that were disconnected. + /// [`Self::sweep_idle_with`] with one timeout for every connection and no + /// cap ([`IdleSweepPolicy::uniform`]). pub async fn sweep_idle(&self, idle_timeout: Duration) -> usize { + self.sweep_idle_with(&IdleSweepPolicy::uniform(idle_timeout)) + .await + } + + /// Put idle connections to sleep (end the agent CLI; the session stays + /// resumable) according to `policy`. Returns how many were put to sleep. + /// + /// A connection is only ever a candidate while it is idle: status + /// `Connected`, no `pending_permission`, no launched-but-unresolved + /// background work or scheduled work held by the CLI (disconnecting kills + /// the agent CLI and that work with it), no Claude Remote Control (it runs + /// in the same CLI), and no codeg delegation whose child is still running + /// (ending the parent cancels it). `Prompting` connections are always + /// preserved (a turn is in flight). + /// + /// Among candidates, one is put to sleep when: + /// - it has been idle (no events, no commands, no keepalive touch) for + /// the policy's timeout — the short `headless_idle_timeout` when no + /// person composes prompts for it (automation, `codeg-cli`, one-shot); + /// - or, with a cap, there are more idle interactive connections than + /// `max_idle`: the ones idle the longest go first, but only those no + /// open tab is watching (untouched for `watched_window`). A watched + /// connection is never put to sleep by the cap. + pub async fn sweep_idle_with(&self, policy: &IdleSweepPolicy) -> usize { let now = chrono::Utc::now(); - let timeout = match chrono::Duration::from_std(idle_timeout) { - Ok(d) => d, - Err(_) => return 0, + let (Ok(timeout), Ok(headless_timeout), Ok(watched_window)) = ( + chrono::Duration::from_std(policy.idle_timeout), + chrono::Duration::from_std(policy.headless_idle_timeout), + chrono::Duration::from_std(policy.watched_window), + ) else { + return 0; }; - let to_disconnect: Vec = { + let to_disconnect: Vec<(String, &'static str)> = { let connections = self.connections.lock().await; let mut victims = Vec::new(); + // Idle interactive connections still under the timeout, with how + // long each has been idle — the pool the cap trims. + let mut warm: Vec<(String, chrono::Duration)> = Vec::new(); for (id, conn) in connections.iter() { let Ok(state) = conn.state.try_read() else { // Per-state writer holds the lock; a future tick will @@ -1098,15 +1132,37 @@ impl ConnectionManager { continue; } let elapsed = now.signed_duration_since(state.last_activity_at); - if elapsed >= timeout { - victims.push(id.clone()); + let headless = owner_is_headless(&conn.owner_window_label); + let limit = if headless { headless_timeout } else { timeout }; + if elapsed >= limit { + victims.push((id.clone(), "idle timeout")); + } else if !headless { + warm.push((id.clone(), elapsed)); + } + } + if let Some(max_idle) = policy.max_idle { + if warm.len() > max_idle { + let excess = warm.len() - max_idle; + // Longest idle first; ties broken by id so a tick is + // deterministic. + warm.sort_by(|a, b| b.1.cmp(&a.1).then_with(|| a.0.cmp(&b.0))); + victims.extend( + warm.into_iter() + .filter(|(_, elapsed)| *elapsed >= watched_window) + .take(excess) + .map(|(id, _)| (id, "idle agent cap")), + ); } } victims }; let mut disconnected = 0; - for id in to_disconnect { - tracing::info!("[ACP] idle sweep disconnecting connection={}", id); + for (id, reason) in to_disconnect { + tracing::info!( + "[ACP] idle sweep disconnecting connection={} reason={}", + id, + reason + ); if self.disconnect(&id).await.is_ok() { disconnected += 1; } @@ -8579,6 +8635,112 @@ mod tests { assert!(map.contains_key("c")); } + fn server_policy(idle_secs: u64, max_idle: Option) -> IdleSweepPolicy { + IdleSweepPolicy { + idle_timeout: Duration::from_secs(idle_secs), + headless_idle_timeout: Duration::from_secs(180), + max_idle, + watched_window: Duration::from_secs(180), + } + } + + async fn set_owner(mgr: &ConnectionManager, conn_id: &str, owner: &str) { + mgr.connections + .lock() + .await + .get_mut(conn_id) + .expect("connection exists") + .owner_window_label = owner.to_string(); + } + + /// The server keeps an interactive agent whose tab went quiet — a phone + /// that locked its screen stops touching, it is not done with the agent. + #[tokio::test] + async fn sweep_with_server_policy_keeps_an_unwatched_agent_until_its_long_timeout() { + let mgr = ConnectionManager::new(); + insert_fake_connection( + &mgr, + "phone", + AgentType::ClaudeCode, + None, + EventEmitter::Noop, + ) + .await; + // Twenty minutes without a touch: long past the desktop timeout. + backdate_last_activity(&mgr, "phone", 20 * 60).await; + assert_eq!(mgr.sweep_idle_with(&server_policy(7200, Some(8))).await, 0); + assert!(mgr.connections.lock().await.contains_key("phone")); + + backdate_last_activity(&mgr, "phone", 7200).await; + assert_eq!(mgr.sweep_idle_with(&server_policy(7200, Some(8))).await, 1); + assert!(!mgr.connections.lock().await.contains_key("phone")); + } + + /// Nobody comes back to an automation run's connection, so it keeps the + /// short timeout even under the server's long one. + #[tokio::test] + async fn sweep_with_server_policy_reclaims_headless_owners_on_the_short_timeout() { + let mgr = ConnectionManager::new(); + for (id, owner) in [ + ("automation", "automation"), + ("cli", crate::cli_run::service::CLI_OWNER_LABEL), + ("one-shot", crate::acp::one_shot::OWNER_LABEL), + ("person", "web"), + ] { + insert_fake_connection(&mgr, id, AgentType::ClaudeCode, None, EventEmitter::Noop).await; + set_owner(&mgr, id, owner).await; + backdate_last_activity(&mgr, id, 600).await; + } + assert_eq!(mgr.sweep_idle_with(&server_policy(7200, None)).await, 3); + let map = mgr.connections.lock().await; + assert!(map.contains_key("person"), "a person's agent stays"); + assert_eq!(map.len(), 1); + } + + /// Past the cap, the agents idle the longest go to sleep first — and only + /// as many as it takes to get back under the cap. + #[tokio::test] + async fn sweep_cap_puts_the_longest_idle_unwatched_agents_to_sleep() { + let mgr = ConnectionManager::new(); + for (id, idle_mins) in [("a", 10), ("b", 40), ("c", 20), ("d", 30)] { + insert_fake_connection(&mgr, id, AgentType::ClaudeCode, None, EventEmitter::Noop).await; + backdate_last_activity(&mgr, id, idle_mins * 60).await; + } + assert_eq!(mgr.sweep_idle_with(&server_policy(7200, Some(2))).await, 2); + let map = mgr.connections.lock().await; + assert!(map.contains_key("a") && map.contains_key("c")); + assert!(!map.contains_key("b") && !map.contains_key("d")); + } + + /// An agent an open tab is touching is never put to sleep by the cap, even + /// when that leaves the pool over it. + #[tokio::test] + async fn sweep_cap_never_touches_a_watched_agent() { + let mgr = ConnectionManager::new(); + for (id, idle_secs) in [("watched-1", 20), ("watched-2", 50), ("unwatched", 3600)] { + insert_fake_connection(&mgr, id, AgentType::ClaudeCode, None, EventEmitter::Noop).await; + backdate_last_activity(&mgr, id, idle_secs).await; + } + assert_eq!(mgr.sweep_idle_with(&server_policy(7200, Some(1))).await, 1); + let map = mgr.connections.lock().await; + assert!(map.contains_key("watched-1") && map.contains_key("watched-2")); + assert!(!map.contains_key("unwatched")); + } + + /// The cap counts only idle agents: one mid-turn neither counts toward it + /// nor is ever chosen. + #[tokio::test] + async fn sweep_cap_ignores_busy_agents() { + let mgr = ConnectionManager::new(); + for id in ["busy", "idle"] { + insert_fake_connection(&mgr, id, AgentType::ClaudeCode, None, EventEmitter::Noop).await; + backdate_last_activity(&mgr, id, 3600).await; + } + mgr.get_state("busy").await.unwrap().write().await.status = ConnectionStatus::Prompting; + assert_eq!(mgr.sweep_idle_with(&server_policy(7200, Some(1))).await, 0); + assert_eq!(mgr.connections.lock().await.len(), 2); + } + /// When two `spawn_agent` calls race for the same logical session id, /// the per-key dedup mutex makes the second one observe the first's /// freshly-spawned connection and reuse it. Without the mutex, both diff --git a/src-tauri/src/acp/mod.rs b/src-tauri/src/acp/mod.rs index 89de586e1e5..f1ff247b321 100644 --- a/src-tauri/src/acp/mod.rs +++ b/src-tauri/src/acp/mod.rs @@ -67,7 +67,7 @@ pub mod work_task_tools; pub mod zcode; pub(crate) mod zcode_turn; -pub use idle_sweep::{idle_sweep_task, idle_timeout_from_env, SWEEP_INTERVAL_SECS}; +pub use idle_sweep::{idle_sweep_task, IdleSweepPolicy, SweepRuntime, SWEEP_INTERVAL_SECS}; pub use internal_bus::{EventBusMetrics, EventBusMetricsSnapshot, InternalEventBus}; pub use lifecycle::lifecycle_subscriber_task; pub use session_state::{LiveSessionSnapshot, SessionState}; diff --git a/src-tauri/src/bin/codeg_server.rs b/src-tauri/src/bin/codeg_server.rs index 7179953157f..1ad2ff588af 100644 --- a/src-tauri/src/bin/codeg_server.rs +++ b/src-tauri/src/bin/codeg_server.rs @@ -650,20 +650,22 @@ async fn async_main( pet_state_handle, )); - // Spawn the idle sweep so connections abandoned without an explicit - // disconnect (e.g. browser tab closed, panic survivors) are reaped. - // Override the 60-second default via `CODEG_ACP_IDLE_TIMEOUT_SECS` - // (set to `0` to disable). - if let Some(idle_timeout) = codeg_lib::idle_timeout_from_env() { + // Spawn the idle sweep. Server policy: the agent belongs to the server, + // not to a browser tab — a phone that locks its screen stops touching its + // connections without being done with them. Idle agents are kept for + // hours (`CODEG_ACP_IDLE_TIMEOUT_SECS`, `0` disables the sweep) and the + // cost is bounded by a cap on idle agents nobody is watching + // (`CODEG_ACP_MAX_IDLE_AGENTS`, `0` = no cap). See `acp::idle_sweep`. + if let Some(policy) = codeg_lib::IdleSweepPolicy::from_env(codeg_lib::SweepRuntime::Server) { tokio::spawn(codeg_lib::idle_sweep_task( state.connection_manager.clone_ref(), - idle_timeout, + policy, std::time::Duration::from_secs(codeg_lib::SWEEP_INTERVAL_SECS), )); } // Reclaim scratch directories lost track of mid-session. Deliberately NOT - // gated on `idle_timeout_from_env` like the sweep above: setting + // gated on the idle sweep policy above: setting // `CODEG_ACP_IDLE_TIMEOUT_SECS=0` disables idle disconnects, not disk // reclamation. tokio::spawn(codeg_lib::scratch_sweep_task()); diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index e5900e5aef0..13718ee0931 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -17,7 +17,7 @@ pub mod acp; pub mod acp_transcript; pub mod agent_turn_files; pub use acp::{ - idle_sweep_task, idle_timeout_from_env, lifecycle_subscriber_task, SWEEP_INTERVAL_SECS, + idle_sweep_task, lifecycle_subscriber_task, IdleSweepPolicy, SweepRuntime, SWEEP_INTERVAL_SECS, }; pub use acp::scratch_dir::scratch_sweep_task; pub use network::proxy::init_proxy_from_db; @@ -1431,14 +1431,16 @@ mod tauri_app { // Spawn the idle sweep so connections abandoned without an // explicit disconnect (e.g. window/tab closed without - // teardown, panic survivors) are reaped. Override the - // 60-second default via `CODEG_ACP_IDLE_TIMEOUT_SECS` - // (set to `0` to disable). - if let Some(idle_timeout) = crate::acp::idle_timeout_from_env() { + // teardown, panic survivors) are reaped. Desktop policy: the + // 3-minute default, overridable via + // `CODEG_ACP_IDLE_TIMEOUT_SECS` (set to `0` to disable). + if let Some(policy) = + crate::acp::IdleSweepPolicy::from_env(crate::acp::SweepRuntime::Desktop) + { let cm = app.state::().clone_ref(); tauri::async_runtime::spawn(crate::acp::idle_sweep_task( cm, - idle_timeout, + policy, std::time::Duration::from_secs(crate::acp::SWEEP_INTERVAL_SECS), )); } diff --git a/src/components/conversations/tab-first-show-auto-connect.test.tsx b/src/components/conversations/tab-first-show-auto-connect.test.tsx index 7f1d45365aa..04759a2c5c9 100644 --- a/src/components/conversations/tab-first-show-auto-connect.test.tsx +++ b/src/components/conversations/tab-first-show-auto-connect.test.tsx @@ -76,6 +76,11 @@ const stubs = vi.hoisted(() => { } }) vi.mock("@/hooks/use-connection", () => ({ useConnection: () => stubs.conn })) +// Local desktop semantics: the window owns the agents it opened. +vi.mock("@/lib/platform", async (importOriginal) => ({ + ...(await importOriginal()), + serverOwnsAgentLifecycle: () => false, +})) vi.mock("@/contexts/acp-connections-context", () => ({ useAcpActions: () => stubs.acp, })) diff --git a/src/contexts/acp-connections-context.test.tsx b/src/contexts/acp-connections-context.test.tsx index ad6bdcedb2c..0c403810d79 100644 --- a/src/contexts/acp-connections-context.test.tsx +++ b/src/contexts/acp-connections-context.test.tsx @@ -103,7 +103,10 @@ vi.mock("next-intl", () => ({ vi.mock("@/lib/platform", () => ({ subscribe: vi.fn(async () => () => {}), getEventStream: () => h.eventStreamValue, + // Local desktop semantics unless a test opts into a server-owned lifecycle. + serverOwnsAgentLifecycle: () => serverLifecycle.owned, })) +const serverLifecycle = vi.hoisted(() => ({ owned: false })) vi.mock("@/lib/delegation-seed", () => ({ buildDelegationSeedEnvelopes: h.buildDelegationSeedEnvelopes, @@ -198,6 +201,7 @@ async function mountProvider() { const TAB = "conv-1-claude_code-42" beforeEach(() => { + serverLifecycle.owned = false h.attach.mockClear() h.store = null h.eventStreamValue = h.stream @@ -860,6 +864,42 @@ describe("AcpConnectionsProvider preview-tab release (disconnectIfIdle)", () => expect(h.acpDisconnect).not.toHaveBeenCalled() expect(h.store!.getConnection(TAB)).toBeUndefined() }) + + it("only detaches an idle owner when the server owns the agent", async () => { + serverLifecycle.owned = true + const handlers = await connectOwner() + emitAcpEvent(handlers, { + seq: 1, + connection_id: "spawned-conn", + type: "status_changed", + status: "connected", + }) + + await act(async () => { + await h.actions!.disconnectIfIdle(TAB) + }) + + // The surface lets go; the agent stays on the server for the next view. + expect(h.acpDisconnect).not.toHaveBeenCalled() + expect(h.store!.getConnection(TAB)).toBeUndefined() + }) + + it("release() detaches an owner without ending its agent", async () => { + const handlers = await connectOwner() + emitAcpEvent(handlers, { + seq: 1, + connection_id: "spawned-conn", + type: "status_changed", + status: "connected", + }) + + await act(async () => { + await h.actions!.release(TAB) + }) + + expect(h.acpDisconnect).not.toHaveBeenCalled() + expect(h.store!.getConnection(TAB)).toBeUndefined() + }) }) // The provider unmounting under a live page (the whole workspace tree swapped @@ -945,6 +985,15 @@ describe("AcpConnectionsProvider unmount teardown (desktop)", () => { expect(h.acpDisconnect).toHaveBeenCalledWith("spawned-conn") }) + + it("leaves even an idle owner running when the server owns the agent", async () => { + serverLifecycle.owned = true + const { unmount } = await connectOwner() + + unmount() + + expect(h.acpDisconnect).not.toHaveBeenCalled() + }) }) // A view claims the active key while it is the one on screen, and has to give @@ -7732,6 +7781,20 @@ describe("live surfaces that are not tabs", () => { } }) + it("drops the entry but leaves the agent to the server when it owns it", async () => { + vi.useFakeTimers() + try { + serverLifecycle.owned = true + await connectOwner() + h.actions!.setActiveKey("some-other-surface") + await sweepPastIdleTimeout() + expect(h.acpDisconnect).not.toHaveBeenCalled() + expect(h.store!.getConnection(TAB)).toBeUndefined() + } finally { + vi.useRealTimers() + } + }) + it("keeps each registrar's claims separate", async () => { vi.useFakeTimers() try { diff --git a/src/contexts/acp-connections-context.tsx b/src/contexts/acp-connections-context.tsx index ebe1aa6b3a6..bc43a615bfd 100644 --- a/src/contexts/acp-connections-context.tsx +++ b/src/contexts/acp-connections-context.tsx @@ -10,7 +10,11 @@ import { type ReactNode, } from "react" import { useTranslations } from "next-intl" -import { subscribe, getEventStream } from "@/lib/platform" +import { + subscribe, + getEventStream, + serverOwnsAgentLifecycle, +} from "@/lib/platform" import type { AttachHandlers, EventStreamSubscription, @@ -3511,6 +3515,16 @@ export interface AcpActionsValue { * stop. */ disconnectIfIdle(contextKey: string): Promise + /** + * Detach this surface from its connection WITHOUT ending the agent: the + * local entry, route and attach subscription go, the backend connection + * stays. For a server-owned lifecycle (`serverOwnsAgentLifecycle`), where a + * view going away — a closed tab, a phone that froze the page — says + * nothing about whether the user is done with the agent; the server's idle + * sweep decides when an idle agent goes to sleep. Reopening the + * conversation attaches to the same agent. + */ + release(contextKey: string): Promise disconnectAll(): Promise sendPrompt( contextKey: string, @@ -6964,8 +6978,11 @@ export function AcpConnectionsProvider({ children }: { children: ReactNode }) { } } + // With a server-owned lifecycle the sweep only drops this client's + // entry; the server's own idle sweep decides when the agent sleeps. + const keepAgents = serverOwnsAgentLifecycle() for (const { contextKey, connectionId } of toDisconnect) { - acpDisconnect(connectionId).catch(() => {}) + if (!keepAgents) acpDisconnect(connectionId).catch(() => {}) releaseConnectionRoute(connectionId, contextKey) teardownAttachSubscription(contextKey) lastActivityRef.current.delete(contextKey) @@ -7023,7 +7040,11 @@ export function AcpConnectionsProvider({ children }: { children: ReactNode }) { // its work settles — the same rule as a closed tab. if (conn && isConnectionBusy(conn)) busy = true } - if (owned && !busy) acpDisconnect(connectionId).catch(() => {}) + // A server-owned agent outlives this client (see + // `serverOwnsAgentLifecycle`): the tree going away only detaches. + if (owned && !busy && !serverOwnsAgentLifecycle()) { + acpDisconnect(connectionId).catch(() => {}) + } } for (const [, sub] of attachSubs) { try { @@ -7784,8 +7805,10 @@ export function AcpConnectionsProvider({ children }: { children: ReactNode }) { ) connectRef.current = connect - const disconnect = useCallback( - async (contextKey: string): Promise => { + // Shared by `disconnect` (ends an owned agent) and `release` (`keepAgent`: + // detaches this surface and leaves the agent to the server's lifecycle). + const teardownSurface = useCallback( + async (contextKey: string, keepAgent: boolean): Promise => { pendingConnectRequestsRef.current.delete(contextKey) // Whatever the surface does next — close, switch agent, reconnect — the // last attempt's failure no longer describes it. @@ -7813,7 +7836,7 @@ export function AcpConnectionsProvider({ children }: { children: ReactNode }) { ([key, other]) => key !== contextKey && other.connectionId === conn.connectionId ) - if (!conn.isViewer || !connectionStillShown) { + if ((!conn.isViewer && !keepAgent) || !connectionStillShown) { retireTurnFailures(conn.connectionId) } // Before either branch drops the entry: an explicit teardown is also how @@ -7821,12 +7844,15 @@ export function AcpConnectionsProvider({ children }: { children: ReactNode }) { // been known to the entry (cold attach hydrates it from the snapshot, // never from a replayed `session_started`). captureIdentityBeforeRemoval(contextKey) - if (conn.isViewer) { + if (conn.isViewer || keepAgent) { // Viewer teardown: drop our read-only attachment WITHOUT // `acpDisconnect` — the backend connection belongs to another client, // and disconnecting it would kill the owner's agent mid-turn. Mirrors // detachDelegationChild. The owner's own disconnect / the idle sweep // governs the connection's real lifetime. + // A `release` takes the same path for an owner: the agent belongs to + // the server, which puts it to sleep once it has been idle long + // enough. teardownAttachSubscription(contextKey) releaseConnectionRoute(conn.connectionId, contextKey) pendingUnmappedEventsRef.current.delete(conn.connectionId) @@ -7870,6 +7896,18 @@ export function AcpConnectionsProvider({ children }: { children: ReactNode }) { ] ) + const disconnect = useCallback( + (contextKey: string) => teardownSurface(contextKey, false), + [teardownSurface] + ) + + const release = useCallback( + async (contextKey: string) => { + await teardownSurface(contextKey, true) + }, + [teardownSurface] + ) + // Lifecycle release for a surface that vanished on its own — currently the // preview tab replaced by the next single-click in the sidebar. `disconnect` // stays unconditional because its other callers express user INTENT (agent @@ -7880,6 +7918,11 @@ export function AcpConnectionsProvider({ children }: { children: ReactNode }) { // longer in `openTabKeys`, so nothing else keeps it alive. const disconnectIfIdle = useCallback( async (contextKey: string) => { + // The server owns the agent: the surface lets go, the agent stays. + if (serverOwnsAgentLifecycle()) { + await release(contextKey) + return + } const conn = storeRef.current.connections.get(contextKey) // Owners only: a viewer's disconnect just detaches (it never // acpDisconnects), and leaving one attached would leak its subscription @@ -7887,7 +7930,7 @@ export function AcpConnectionsProvider({ children }: { children: ReactNode }) { if (conn && !conn.isViewer && isConnectionBusy(conn)) return await disconnect(contextKey) }, - [disconnect] + [disconnect, release] ) const reapplyConfig = useCallback( @@ -8435,6 +8478,7 @@ export function AcpConnectionsProvider({ children }: { children: ReactNode }) { connect, disconnect, disconnectIfIdle, + release, disconnectAll, sendPrompt, setMode, @@ -8464,6 +8508,7 @@ export function AcpConnectionsProvider({ children }: { children: ReactNode }) { connect, disconnect, disconnectIfIdle, + release, disconnectAll, sendPrompt, setMode, diff --git a/src/contexts/workspace-context.test.tsx b/src/contexts/workspace-context.test.tsx index 34831970fc4..a8a507939b3 100644 --- a/src/contexts/workspace-context.test.tsx +++ b/src/contexts/workspace-context.test.tsx @@ -206,6 +206,8 @@ vi.mock(import("@/lib/platform"), async (importOriginal) => ({ getEventStream: () => ({ attach: () => ({ subscriptionId: "mobile-stream", detach: () => {} }), }), + // Local desktop semantics: closing the retained view ends its idle agent. + serverOwnsAgentLifecycle: () => false, })) // Controllable workspace-state store: the tab watcher acquires per-root diff --git a/src/hooks/use-connection-lifecycle.agent-switch.test.ts b/src/hooks/use-connection-lifecycle.agent-switch.test.ts index bf507c7e590..42d17465243 100644 --- a/src/hooks/use-connection-lifecycle.agent-switch.test.ts +++ b/src/hooks/use-connection-lifecycle.agent-switch.test.ts @@ -11,6 +11,13 @@ import { renderHook } from "@testing-library/react" import { beforeEach, describe, expect, it, vi } from "vitest" +// Local desktop semantics: the window owns the agents it opened. The +// server-owned lifecycle is covered in use-connection-lifecycle.unmount.test.ts. +vi.mock("@/lib/platform", async (importOriginal) => ({ + ...(await importOriginal()), + serverOwnsAgentLifecycle: () => false, +})) + vi.mock("next-intl", () => ({ useTranslations: () => (key: string) => key, })) diff --git a/src/hooks/use-connection-lifecycle.connect-error.test.ts b/src/hooks/use-connection-lifecycle.connect-error.test.ts index 36e805a933b..87167d68a8a 100644 --- a/src/hooks/use-connection-lifecycle.connect-error.test.ts +++ b/src/hooks/use-connection-lifecycle.connect-error.test.ts @@ -13,6 +13,13 @@ import { renderHook } from "@testing-library/react" import { beforeEach, describe, expect, it, vi } from "vitest" import type { ConnectErrorInfo } from "@/contexts/acp-connections-context" +// Local desktop semantics: the window owns the agents it opened. The +// server-owned lifecycle is covered in use-connection-lifecycle.unmount.test.ts. +vi.mock("@/lib/platform", async (importOriginal) => ({ + ...(await importOriginal()), + serverOwnsAgentLifecycle: () => false, +})) + vi.mock("next-intl", () => ({ useTranslations: () => (key: string) => key, })) diff --git a/src/hooks/use-connection-lifecycle.opening.test.ts b/src/hooks/use-connection-lifecycle.opening.test.ts index 00945bdd60c..33fa36465c6 100644 --- a/src/hooks/use-connection-lifecycle.opening.test.ts +++ b/src/hooks/use-connection-lifecycle.opening.test.ts @@ -18,6 +18,13 @@ import { act, renderHook } from "@testing-library/react" import { beforeEach, describe, expect, it, vi } from "vitest" +// Local desktop semantics: the window owns the agents it opened. The +// server-owned lifecycle is covered in use-connection-lifecycle.unmount.test.ts. +vi.mock("@/lib/platform", async (importOriginal) => ({ + ...(await importOriginal()), + serverOwnsAgentLifecycle: () => false, +})) + vi.mock("next-intl", () => ({ useTranslations: () => (key: string, params?: Record) => params ? `${key}:${JSON.stringify(params)}` : key, diff --git a/src/hooks/use-connection-lifecycle.send-failure.test.ts b/src/hooks/use-connection-lifecycle.send-failure.test.ts index 1675fea52e2..3d24d4a09af 100644 --- a/src/hooks/use-connection-lifecycle.send-failure.test.ts +++ b/src/hooks/use-connection-lifecycle.send-failure.test.ts @@ -21,6 +21,13 @@ import { TurnBusyError } from "@/lib/turn-busy" const toastError = vi.fn() +// Local desktop semantics: the window owns the agents it opened. The +// server-owned lifecycle is covered in use-connection-lifecycle.unmount.test.ts. +vi.mock("@/lib/platform", async (importOriginal) => ({ + ...(await importOriginal()), + serverOwnsAgentLifecycle: () => false, +})) + vi.mock("sonner", () => ({ toast: { error: (...args: unknown[]) => toastError(...args), diff --git a/src/hooks/use-connection-lifecycle.ts b/src/hooks/use-connection-lifecycle.ts index 81281e26add..a21a8f13009 100644 --- a/src/hooks/use-connection-lifecycle.ts +++ b/src/hooks/use-connection-lifecycle.ts @@ -7,6 +7,8 @@ import { useTaskContext } from "@/contexts/task-context" import { useConnection, type UseConnectionReturn } from "@/hooks/use-connection" import { extractAppCommandError } from "@/lib/app-error" import { notify } from "@/lib/notify" +import { serverOwnsAgentLifecycle } from "@/lib/platform" +import { AUTO_RESUME_MIN_INTERVAL_MS } from "@/lib/constants" import { isConnectionBusy, type ConnectionWork, @@ -153,6 +155,7 @@ export function useConnectionLifecycle({ selectorsReady, connect: connConnect, disconnect: connDisconnect, + release: connRelease, sendPrompt, setMode: connSetMode, setConfigOption: connSetConfigOption, @@ -291,6 +294,58 @@ export function useConnectionLifecycle({ } }, [isActive, workingDir, agentType]) + // Resume a server-owned agent that went away under the open conversation — + // the server put it to sleep after a long idle stretch, or restarted. The + // conversation on screen should simply work again when the user comes back + // to it, without a dimmed composer and a Reconnect button to find first. + // `connect()` resumes the same session (`session/load`), so nothing is lost. + // + // Only for a connection that went away WHILE this view was open and active: + // a view that mounts on, or switches to, a disconnected entry is the + // auto-connect's job above. Only while the page is visible — a hidden page + // resumes when it is shown again. And at most once per + // AUTO_RESUME_MIN_INTERVAL_MS: an agent that dies again right after starting + // must not turn this into a respawn loop (the composer's Reconnect is still + // there for that case). + const lastAutoResumeAtRef = useRef(0) + const resumePendingRef = useRef(false) + const resumePrevStatusRef = useRef(status) + useEffect(() => { + const previous = resumePrevStatusRef.current + resumePrevStatusRef.current = status + if (status !== "disconnected" || !isActive) { + resumePendingRef.current = false + return + } + if (previous !== "disconnected") resumePendingRef.current = true + if (!serverOwnsAgentLifecycle() || !workingDir) return + const resume = () => { + if (!resumePendingRef.current) return + if (document.visibilityState !== "visible") return + const now = Date.now() + if (now - lastAutoResumeAtRef.current < AUTO_RESUME_MIN_INTERVAL_MS) { + return + } + resumePendingRef.current = false + lastAutoResumeAtRef.current = now + connConnectRef + .current( + agentType, + workingDir, + sessionIdRef.current, + conversationIdRef.current + ) + .catch((e: unknown) => { + if (!isExpectedConnectError(e)) { + console.error("[ConnLifecycle] auto-resume:", e) + } + }) + } + resume() + document.addEventListener("visibilitychange", resume) + return () => document.removeEventListener("visibilitychange", resume) + }, [status, isActive, workingDir, agentType]) + // Status-bar task for the two legs that precede a usable session: // "preparing" — the caller is resolving what to connect to (see `preparing`) // "connecting" — `connect()` is in flight: agent spawn, ACP `initialize`, @@ -403,6 +458,10 @@ export function useConnectionLifecycle({ useEffect(() => { connDisconnectRef.current = connDisconnect }, [connDisconnect]) + const connReleaseRef = useRef(connRelease) + useEffect(() => { + connReleaseRef.current = connRelease + }, [connRelease]) const releaseActiveKeyRef = useRef(releaseActiveKey) useEffect(() => { releaseActiveKeyRef.current = releaseActiveKey @@ -433,7 +492,12 @@ export function useConnectionLifecycle({ // mid-turn is safe and leaves the owner's agent untouched. And it's // necessary: the idle sweep skips viewers, so a viewer left attached // here would leak its WS subscription until the whole provider unmounts. - if ( + // With a server-owned lifecycle the view only lets go: the agent keeps + // running (or idles) on the server, which alone decides when it sleeps, + // and reopening the conversation attaches to it again. + if (serverOwnsAgentLifecycle() && !transientUnmount) { + connReleaseRef.current().catch(() => {}) + } else if ( shouldDisconnectOnUnmount({ ...liveWorkRef.current, status: statusRef.current, diff --git a/src/hooks/use-connection-lifecycle.unmount.test.ts b/src/hooks/use-connection-lifecycle.unmount.test.ts index 54cae977c0d..ef0124546ad 100644 --- a/src/hooks/use-connection-lifecycle.unmount.test.ts +++ b/src/hooks/use-connection-lifecycle.unmount.test.ts @@ -8,16 +8,27 @@ * * It must also give back the active key the view claimed, or the busy owner it * leaves running stays out of both sweeps after its work settles. + * + * With a server-owned lifecycle (web, or a desktop window on a remote server) + * the view never ends the agent: it only lets go, and the server decides when + * an idle agent sleeps. A conversation whose agent went away while it stayed + * open resumes on its own. */ -import { renderHook } from "@testing-library/react" -import { beforeEach, describe, expect, it, vi } from "vitest" +import { act, renderHook } from "@testing-library/react" +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" import type { AsyncTaskRecord, RemoteControlInfo } from "@/lib/types" vi.mock("next-intl", () => ({ useTranslations: () => (key: string) => key, })) +const lifecycle = vi.hoisted(() => ({ serverOwned: false })) +vi.mock("@/lib/platform", async (importOriginal) => ({ + ...(await importOriginal()), + serverOwnsAgentLifecycle: () => lifecycle.serverOwned, +})) + const acp = vi.hoisted(() => ({ setActiveKey: (() => {}) as (key: string | null) => void, releaseActiveKey: (() => {}) as (key: string) => void, @@ -38,20 +49,24 @@ const conn = vi.hoisted(() => ({ asyncTasks: [] as AsyncTaskRecord[], remoteControl: null as RemoteControlInfo | null, delegatedChildren: [] as string[], + status: "connected" as string, disconnect: (() => Promise.resolve()) as () => Promise, + release: (() => Promise.resolve()) as () => Promise, + connect: (() => Promise.resolve()) as (...args: unknown[]) => Promise, })) vi.mock("@/hooks/use-connection", () => { const noop = vi.fn().mockResolvedValue(undefined) return { useConnection: () => ({ - status: "connected", + status: conn.status, agentType: "codex", connectError: null, selectorsReady: true, hasCachedSelectors: true, - connect: noop, + connect: conn.connect, disconnect: conn.disconnect, + release: conn.release, sendPrompt: noop, setMode: noop, setConfigOption: noop, @@ -101,6 +116,10 @@ describe("useConnectionLifecycle unmount teardown", () => { let releaseActiveKey: ReturnType beforeEach(() => { + lifecycle.serverOwned = false + conn.status = "connected" + conn.release = vi.fn().mockResolvedValue(undefined) + conn.connect = vi.fn().mockResolvedValue(undefined) disconnect = vi.fn().mockResolvedValue(undefined) conn.disconnect = disconnect as unknown as () => Promise conn.isViewer = false @@ -174,3 +193,142 @@ describe("useConnectionLifecycle unmount teardown", () => { expect(releaseActiveKey).not.toHaveBeenCalled() }) }) + +describe("useConnectionLifecycle with a server-owned agent lifecycle", () => { + let release: ReturnType + let connect: ReturnType + let disconnect: ReturnType + + beforeEach(() => { + lifecycle.serverOwned = true + conn.isViewer = false + conn.asyncTasks = [] + conn.remoteControl = null + conn.delegatedChildren = [] + conn.status = "connected" + release = vi.fn().mockResolvedValue(undefined) + connect = vi.fn().mockResolvedValue(undefined) + disconnect = vi.fn().mockResolvedValue(undefined) + conn.release = release as unknown as () => Promise + conn.connect = connect as unknown as (...args: unknown[]) => Promise + conn.disconnect = disconnect as unknown as () => Promise + acp.setActiveKey = vi.fn() + acp.releaseActiveKey = vi.fn() + acp.touchActivity = vi.fn() + vi.spyOn(Date, "now").mockReturnValue(1_000_000) + }) + + afterEach(() => { + vi.restoreAllMocks() + setVisibility("visible") + }) + + function setVisibility(state: DocumentVisibilityState) { + Object.defineProperty(document, "visibilityState", { + configurable: true, + get: () => state, + }) + } + + function openView() { + return renderHook( + ({ status }: { status: string }) => { + conn.status = status + return useConnectionLifecycle({ + contextKey: KEY, + agentType: "codex", + workingDir: "/repo", + sessionId: "session-1", + conversationId: 42, + isActive: true, + }) + }, + { initialProps: { status: "connected" } } + ) + } + + it("lets an idle owner go without ending its agent when the view closes", () => { + closeView() + + expect(release).toHaveBeenCalledTimes(1) + expect(disconnect).not.toHaveBeenCalled() + }) + + it("lets a busy owner go the same way", () => { + conn.asyncTasks = [backgroundTerminal("running")] + + closeView() + + expect(release).toHaveBeenCalledTimes(1) + expect(disconnect).not.toHaveBeenCalled() + }) + + it("keeps the view attached across a transient (reparent) unmount", () => { + closeView({ isActive: true, transient: true }) + + expect(release).not.toHaveBeenCalled() + expect(disconnect).not.toHaveBeenCalled() + }) + + it("resumes the session when the agent goes away under the open view", () => { + const view = openView() + connect.mockClear() // the mount's own auto-connect + + view.rerender({ status: "disconnected" }) + + expect(connect).toHaveBeenCalledTimes(1) + expect(connect).toHaveBeenCalledWith("codex", "/repo", "session-1", 42) + }) + + it("waits for a hidden page to be shown before resuming", () => { + setVisibility("hidden") + const view = openView() + connect.mockClear() + + view.rerender({ status: "disconnected" }) + expect(connect).not.toHaveBeenCalled() + + setVisibility("visible") + act(() => { + document.dispatchEvent(new Event("visibilitychange")) + }) + expect(connect).toHaveBeenCalledTimes(1) + }) + + it("does not respawn an agent that dies again right after resuming", () => { + const view = openView() + connect.mockClear() + + view.rerender({ status: "disconnected" }) + view.rerender({ status: "connecting" }) + view.rerender({ status: "disconnected" }) + + expect(connect).toHaveBeenCalledTimes(1) + }) + + it("leaves a view that opens on a disconnected entry to the auto-connect", () => { + const view = renderHook(() => { + conn.status = "disconnected" + return useConnectionLifecycle({ + contextKey: KEY, + agentType: "codex", + workingDir: "/repo", + isActive: true, + }) + }) + + // The mount's auto-connect only — no second, auto-resume connect. + expect(connect).toHaveBeenCalledTimes(1) + view.unmount() + }) + + it("never resumes on its own in the local desktop app", () => { + lifecycle.serverOwned = false + const view = openView() + connect.mockClear() + + view.rerender({ status: "disconnected" }) + + expect(connect).not.toHaveBeenCalled() + }) +}) diff --git a/src/hooks/use-connection.ts b/src/hooks/use-connection.ts index 90dba66f46a..fad4bc0bace 100644 --- a/src/hooks/use-connection.ts +++ b/src/hooks/use-connection.ts @@ -142,6 +142,9 @@ export interface UseConnectionReturn { conversationId?: number ) => Promise disconnect: () => Promise + /** Detach this surface and leave the agent running on the server (see + * `AcpActionsValue.release`). */ + release: () => Promise /** Restart the session (disconnect + resume same sessionId) so it picks up * current agent/model settings. Returns `true` if it actually restarted, * `false` on a no-op (viewer / delegation child / no connection). */ @@ -346,6 +349,11 @@ export function useConnection(contextKey: string): UseConnectionReturn { await actions.disconnect(contextKey) }, [actions, contextKey]) + const release = useCallback( + () => actions.release(contextKey), + [actions, contextKey] + ) + const sendPrompt = useCallback( ( blocks: PromptInputBlock[], @@ -435,6 +443,7 @@ export function useConnection(contextKey: string): UseConnectionReturn { delegatedChildren, connect, disconnect, + release, reapplyConfig, dismissConfigStale, sendPrompt, @@ -482,6 +491,7 @@ export function useConnection(contextKey: string): UseConnectionReturn { delegatedChildren, connect, disconnect, + release, reapplyConfig, dismissConfigStale, sendPrompt, diff --git a/src/lib/constants.ts b/src/lib/constants.ts index 36555cded0c..97a07aba277 100644 --- a/src/lib/constants.ts +++ b/src/lib/constants.ts @@ -5,3 +5,7 @@ export const IDLE_SWEEP_INTERVAL_MS = 60 * 1000 // 1 minute // open tab gets at least one touch per backend timeout window — 30s // gives ample safety margin under network jitter. export const CONNECTION_KEEPALIVE_INTERVAL_MS = 30 * 1000 // 30 seconds +// Least time between two automatic resumes of a server-owned agent that went +// away under the open conversation (see `useConnectionLifecycle`). Keeps an +// agent that dies right after starting from becoming a respawn loop. +export const AUTO_RESUME_MIN_INTERVAL_MS = 60 * 1000 // 1 minute diff --git a/src/lib/platform.ts b/src/lib/platform.ts index efe1dcfdbb9..2f3ad494959 100644 --- a/src/lib/platform.ts +++ b/src/lib/platform.ts @@ -23,6 +23,22 @@ export function isLocalDesktop(): boolean { return isDesktop() && getActiveRemoteConnectionId() === null } +/** + * True when agents run on a codeg server any number of devices can use at + * once — a browser, or a desktop window bound to a remote server — rather + * than in the local desktop app. + * + * Then the agent belongs to the server, not to the view showing it: a closed + * tab, a phone that froze the page when its screen locked, or a laptop lid + * says nothing about whether the user is done with the agent. Views only + * detach (`release`), and the server's idle sweep alone decides when an idle + * agent goes to sleep. In the local desktop app the window owns the agents it + * opened, and closing a view ends an idle one. + */ +export function serverOwnsAgentLifecycle(): boolean { + return !isLocalDesktop() +} + /** * True for a Tauri window pointed at a REMOTE workspace — the one case where * we know for certain the host that owns the workspace paths is not the machine