Skip to content

fix(cua-driver): let start_session revive an idle-reaped session (recover from "session ended") #2035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
LaZzyMan wants to merge 1 commit into from
Closed

fix(cua-driver): let start_session revive an idle-reaped session (recover from "session ended") #2035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions libs/cua-driver/rust/crates/cua-driver-core/src/session.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Comment on lines +138 to +152

@coderabbitai coderabbitai Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 1. Find revive_session and any hooks it fires.
rg -nP -C3 '\bfn revive_session\b' libs/cua-driver/rust/crates

# 2. Show how the overlay `ended` tombstone is mutated — is it ever removed from?
fd -t f overlay.rs libs/cua-driver/rust/crates/platform-macos \
  --exec rg -nP -C3 '\bended\b'

# 3. Look for any revive/resume hook registration that could clear overlay state.
rg -nP -C2 'revive|resume_session|register_session_(begin|revive)_hook' \
  libs/cua-driver/rust/crates

Repository: trycua/cua

Length of output: 10236

revive_session clears the session tombstone but not the overlay-layer cursor tombstone — revived sessions will fail to render cursors.

The cursor overlay (platform-macos/src/overlay.rs) maintains a separate ended: HashSet<CursorKey> that permanently blocks command processing for tombstoned keys. As seen in lines 143–145 and 255–257 of overlay.rs, any Cmd or seed_start for a key in this set is silently dropped to prevent "ghost-cursor resurrection."

revive_session in session.rs (lines 138–152) correctly clears the session-layer tombstone (ended_sessions) but fails to clear the overlay-layer tombstone. Consequently:

  1. The daemon accepts tool calls for the revived session.
  2. The overlay guard continues to drop all cursor commands for that session_id.
  3. The cursor never reappears, violating the "RESUMES the session" contract in session_tools.rs (line 79).

Add a call to clear the overlay tombstone (e.g., via a fire_session_revive hook that removes the session_id from RenderState::ended) to ensure the render layer respects the session lifecycle reset.

Relevant code sections

session.rs:

pub fn revive_session(session_id: &str) {
    if !is_trackable(session_id) {
        return;
    }
    // ...
    ended_sessions().lock().unwrap().remove(session_id); // Clears session tombstone
    // MISSING: Clear overlay tombstone here
    activity()
        .lock()
        .unwrap()
        .insert(session_id.to_owned(), Instant::now());
}

overlay.rs (guard logic):

// Line 143-145
if map.ended.contains(&key) {
    return None; // Drops command silently
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@libs/cua-driver/rust/crates/cua-driver-core/src/session.rs` around lines 138
- 152, revive_session currently clears the session-layer tombstone but leaves
the overlay-layer tombstone in place, so revived sessions still get blocked by
the cursor renderer. Update revive_session in session.rs to also clear the
overlay state for the same session_id, ideally by introducing or invoking a
dedicated revive hook that removes the key from RenderState::ended in
platform-macos/src/overlay.rs. Make sure the fix aligns with the existing
end-session cleanup path so revived sessions can start rendering cursors again
without changing the trackable-session guard behavior.


/// 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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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).
Expand All @@ -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).
Expand All @@ -208,11 +243,32 @@ 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);
assert!(is_session_ended(sid));
// 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");
}
}
19 changes: 13 additions & 6 deletions libs/cua-driver/rust/crates/cua-driver-core/src/session_tools.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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 }))
}
Expand Down
70 changes: 42 additions & 28 deletions libs/cua-driver/rust/crates/cua-driver/src/serve.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down