Skip to content

Commit 78f33ec

Browse files
chenhan1218Copilot
andcommitted
fix(acp/pool): preserve session ID on session/load timeout
When session/load times out transiently, return an error to the user instead of falling through to session/new with no history context. The original session ID is already in state.persisted (never modified on this code path), so the next message automatically retries session/load. Only actual timeouts trigger this path; permanent rejections (e.g. session/load rejected) still fall through to session/new as before. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 4195802 commit 78f33ec

2 files changed

Lines changed: 29 additions & 5 deletions

File tree

src/acp/pool.rs

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ impl SessionPool {
243243
new_conn.initialize().await?;
244244

245245
let mut resumed = false;
246+
let mut load_failed = false;
246247
if let Some(ref sid) = saved_session_id {
247248
if new_conn.supports_load_session {
248249
match new_conn.session_load(sid, &effective_workdir).await {
@@ -251,19 +252,32 @@ impl SessionPool {
251252
resumed = true;
252253
}
253254
Err(e) => {
254-
warn!(thread_id, session_id = %sid, error = %e, "session/load failed, creating new session");
255+
let is_timeout = e.to_string().contains("timeout waiting for");
256+
if is_timeout {
257+
warn!(thread_id, session_id = %sid, error = %e,
258+
"session/load timed out, preserving session ID for retry");
259+
load_failed = true;
260+
} else {
261+
warn!(thread_id, session_id = %sid, error = %e,
262+
"session/load failed, creating new session");
263+
}
255264
}
256265
}
257266
}
258267
}
259268

269+
if load_failed {
270+
// session/load timed out transiently. The original session ID is already
271+
// in state.persisted (we haven't touched it), so the next message will
272+
// retry session/load automatically. Return an error so the current message
273+
// is not processed against a context-free session.
274+
return Err(anyhow!("session load timeout: could not restore previous session"));
275+
}
276+
260277
if !resumed {
261278
new_conn.session_new(&effective_workdir).await?;
262-
// Surface the reset banner both for restored sessions and for stale
263-
// live entries that died before we could recover a resumable
264-
// session id. In both cases the caller is continuing after an
265-
// unexpected session loss.
266279
if had_existing || saved_session_id.is_some() {
280+
// Genuine session loss (agent died, session file gone, etc.).
267281
new_conn.session_reset = true;
268282
}
269283
}

src/error_display.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ pub fn format_user_error(message: &str) -> String {
1010
let msg_lower = message.to_lowercase();
1111

1212
// Startup / connection errors (code == 0 from anyhow)
13+
if msg_lower.contains("session load timeout") {
14+
return "**Session Load Timeout**\nCould not restore your previous session. Send any message to retry, or use /reset to start fresh.".to_string();
15+
}
1316
if msg_lower.contains("timeout waiting for") {
1417
// Use msg_lower for extraction to stay case-insistent with the match above.
1518
// msg_lower and message are the same length, so byte offsets are valid.
@@ -97,6 +100,13 @@ mod tests {
97100

98101
// ─── format_user_error tests ─────────────────────────────────────────────
99102

103+
#[test]
104+
fn format_user_error_session_load_timeout() {
105+
let result = format_user_error("session load timeout: could not restore previous session");
106+
assert!(result.contains("Session Load Timeout"));
107+
assert!(result.contains("/reset"));
108+
}
109+
100110
#[test]
101111
fn format_user_error_timeout() {
102112
let result = format_user_error("timeout waiting for session/new response");

0 commit comments

Comments
 (0)