diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/session.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/session.rs index 8c41056e88..389ba8edaf 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/session.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/session.rs @@ -126,6 +126,31 @@ pub fn end_session(session_id: &str) { fire_session_end(session_id); } +/// Revive a session that a prior `end_session` / idle-TTL sweep marked ended, +/// so an explicit `start_session` can resume a run after an idle gap. +/// +/// Without this, an idle-reaped session is permanently dead: `is_session_ended` +/// stays true forever (the tombstone is never cleared), `touch_session` no-ops +/// on an ended id, and the daemon's resurrection guard rejects every subsequent +/// `call` with "session ended; tool call ignored" — including `start_session` +/// itself. Clearing the tombstone and re-arming the idle-TTL clock makes the id +/// usable again. No-op for the anonymous fallback. +pub fn revive_session(session_id: &str) { + if !is_trackable(session_id) { + return; + } + // Clear the permanent tombstone so `is_session_ended` reads false again and + // the daemon's resurrection guard stops rejecting calls for this id. A + // later `end_session` re-fires the cleanup hooks (fire_session_end is keyed + // on this set), which is correct — it's a fresh lifecycle for the id. + ended_sessions().lock().unwrap().remove(session_id); + // Re-arm the idle-TTL clock so the revived session is tracked again. + activity() + .lock() + .unwrap() + .insert(session_id.to_owned(), Instant::now()); +} + /// End every session whose last activity is older than `ttl`, returning the ids /// ended. This is the idle-TTL sweep the daemon runs periodically: a /// caller-declared session is no longer tied to a connection's lifetime, so a @@ -158,6 +183,14 @@ mod tests { use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; + /// Serialize tests that invoke the process-global `evict_idle(ZERO)` sweep, + /// so one test's zero-TTL reap can't re-end a session another test just + /// (re)touched — they share `SESSION_ACTIVITY` / `ENDED_SESSIONS`. + static SWEEP_TEST_LOCK: Mutex<()> = Mutex::new(()); + fn sweep_lock() -> std::sync::MutexGuard<'static, ()> { + SWEEP_TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()) + } + #[test] fn fire_session_end_is_idempotent_per_id() { // Distinct, test-local ids so we don't collide with other tests that @@ -187,6 +220,7 @@ mod tests { #[test] fn touch_then_evict_by_ttl() { + let _sweep = sweep_lock(); let sid = "test-ttl-session-DDEEFF"; touch_session(sid); // A huge TTL leaves it alone (just touched). @@ -199,6 +233,7 @@ mod tests { #[test] fn anonymous_ids_are_never_tracked() { + let _sweep = sweep_lock(); touch_session("default"); touch_session(""); // Neither shows up under a zero-TTL sweep (they were never inserted). @@ -208,6 +243,7 @@ mod tests { #[test] fn end_session_is_explicit_teardown() { + let _sweep = sweep_lock(); let sid = "test-end-session-112233"; touch_session(sid); end_session(sid); @@ -215,4 +251,24 @@ mod tests { // Its TTL entry is gone, so a later sweep doesn't re-fire for it. assert!(!evict_idle(Duration::ZERO).iter().any(|s| s == sid)); } + + #[test] + fn revive_resumes_an_ended_session() { + let _sweep = sweep_lock(); + let sid = "test-revive-session-778899"; + touch_session(sid); + end_session(sid); + assert!(is_session_ended(sid), "precondition: session is ended"); + // Revive must clear the tombstone so the id is usable again... + revive_session(sid); + assert!(!is_session_ended(sid), "revive must un-end the session"); + // ...and re-arm the idle-TTL so the session is tracked again: a fresh + // zero-TTL sweep should be able to reclaim it (proving it's live, not + // stuck in limbo). + assert!( + evict_idle(Duration::ZERO).iter().any(|s| s == sid), + "a revived session must be tracked again (re-armed TTL)" + ); + assert!(is_session_ended(sid), "and can be ended again after revival"); + } } diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/session_tools.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/session_tools.rs index dda88f0d39..65e7d5c5ea 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/session_tools.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/session_tools.rs @@ -45,9 +45,12 @@ impl Tool for StartSessionTool { The cursor's color is derived from the id, so distinct runs are visually \ distinct. A cursor is shown only for a declared session — call this (or \ pass `session` on your first action) to opt in. Idempotent: re-calling \ - with the same id just refreshes its idle-TTL. End it with `end_session` \ - (or let the idle-TTL reclaim it). Concurrent runs/subagents each pass \ - their own `session` to get their own cursor." + with the same id refreshes its idle-TTL, and RESUMES the session if a \ + prior idle-TTL reclaim or `end_session` had ended it — so calling this \ + is also how you recover from a \"session ended; tool call ignored\" \ + error. End it with `end_session` (or let the idle-TTL reclaim it). \ + Concurrent runs/subagents each pass their own `session` to get their \ + own cursor." .into(), input_schema: json!({ "type": "object", @@ -73,9 +76,13 @@ impl Tool for StartSessionTool { "start_session requires a non-empty `session` id.", ); }; - // Refresh (or begin) the session's idle-TTL clock. The cursor appears on - // the first action carrying this `session`. - crate::session::touch_session(&id); + // Begin / refresh / RESUME the session. `revive_session` is a superset + // of `touch_session`: for a live or brand-new id it just (re)arms the + // idle-TTL clock; for an id a prior idle-TTL sweep or `end_session` + // marked ended it also clears the tombstone so the run can continue. + // Without this, an idle-reaped session was permanently dead — even + // `start_session` got rejected by the daemon's resurrection guard. + crate::session::revive_session(&id); ToolResult::text(format!("✅ Session '{id}' is active.")) .with_structured(json!({ "session": id, "active": true })) } diff --git a/libs/cua-driver/rust/crates/cua-driver/src/serve.rs b/libs/cua-driver/rust/crates/cua-driver/src/serve.rs index 7efa76ec38..808991ec74 100644 --- a/libs/cua-driver/rust/crates/cua-driver/src/serve.rs +++ b/libs/cua-driver/rust/crates/cua-driver/src/serve.rs @@ -664,20 +664,27 @@ pub async fn run_serve( // config override, recording) the reaper already // passed. Skip + benign ok. Live and anonymous // calls pass through unchanged. - if let Some(sid) = &effective_session { - if cua_driver_core::session::is_session_ended(sid) { - let resp = DaemonResponse::ok(serde_json::json!({ - "content": [{ - "type": "text", - "text": "session ended; tool call ignored" - }], - "isError": false, - "sessionEnded": true - })); - let _ = writer.write_all( - (serde_json::to_string(&resp).unwrap() + "\n").as_bytes() - ).await; - continue; + // `start_session` is the explicit recovery path + // that REVIVES an idle-reaped session, so it must + // be exempt from the resurrection guard. Every + // other tool stays guarded so a late in-flight + // call can't rebuild reaped session state. + if tool_name != "start_session" { + if let Some(sid) = &effective_session { + if cua_driver_core::session::is_session_ended(sid) { + let resp = DaemonResponse::ok(serde_json::json!({ + "content": [{ + "type": "text", + "text": "session ended; tool call ignored. Call start_session with this `session` id to resume the run." + }], + "isError": false, + "sessionEnded": true + })); + let _ = writer.write_all( + (serde_json::to_string(&resp).unwrap() + "\n").as_bytes() + ).await; + continue; + } } } if reg.get_def(&tool_name).is_none() { @@ -1180,20 +1187,27 @@ pub async fn run_serve( let effective_session = apply_session_identity(&mut args, &req.session_id); // Resurrection guard on the effective session. - if let Some(sid) = &effective_session { - if cua_driver_core::session::is_session_ended(sid) { - let resp = DaemonResponse::ok(serde_json::json!({ - "content": [{ - "type": "text", - "text": "session ended; tool call ignored" - }], - "isError": false, - "sessionEnded": true - })); - let _ = writer.write_all( - (serde_json::to_string(&resp).unwrap() + "\n").as_bytes() - ).await; - continue; + // `start_session` is the explicit recovery path + // that REVIVES an idle-reaped session, so it must + // be exempt from the resurrection guard. Every + // other tool stays guarded so a late in-flight + // call can't rebuild reaped session state. + if tool_name != "start_session" { + if let Some(sid) = &effective_session { + if cua_driver_core::session::is_session_ended(sid) { + let resp = DaemonResponse::ok(serde_json::json!({ + "content": [{ + "type": "text", + "text": "session ended; tool call ignored. Call start_session with this `session` id to resume the run." + }], + "isError": false, + "sessionEnded": true + })); + let _ = writer.write_all( + (serde_json::to_string(&resp).unwrap() + "\n").as_bytes() + ).await; + continue; + } } } if reg.get_def(&tool_name).is_none() {