fix(acp): fail session/load for an id that does not exist
session/load with an id that was never saved fell through the restore step and opened an empty session under that id. The editor then showed a blank thread and had no way to tell it from a restored one. The handler now looks for the session before touching any state: the saved copy first, then a session this process still holds in memory, which covers a thread that was opened but not spoken in (nothing is written to disk until a turn completes). If neither exists it returns -32002 and registers nothing. Fixes #137 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Seto Elkahfi committed
Oct 5, 2026 at 00:01 UTC
7e761b054e7f5dbbf8acc92ca840147a3627f303
3 files changed
+128
-20
CHANGELOG.md
+6
@@ -17,6 +17,12 @@
17
one and resolve it against wherever the editor spawned the process. They now
18
answer with an invalid-params error (#136).
19
20
+- **Loading a session that does not exist is an error.** `session/load` with
21
+ an id that was never saved used to succeed and open an empty session under
22
+ that id, so the editor showed a blank thread that looked restored. It now
23
+ answers "not found" (`-32002`). A thread opened in the running process but
24
+ not spoken in yet still loads (#137).
25
+
26
## 1.5.13
27
28
### Added
src/main.rs
+39
-20
@@ -1889,6 +1889,28 @@ impl SiGitAgent {
1889
);
1890
1891
let session_key = args.session_id.to_string();
1892
+
1893
+ // The saved copy is the thread; failing that, this process may still
1894
+ // hold one nobody has spoken in yet (nothing is written until a turn
1895
+ // completes). An id found in neither place names no session, and
1896
+ // answering with an empty one would hand the client a blank thread it
1897
+ // cannot tell from a restored one. Parking first makes the in-memory
1898
+ // copy current when the id being loaded is the live session.
1899
+ self.park_active_session().await;
1900
+ let in_memory = self
1901
+ .sessions
1902
+ .lock()
1903
+ .unwrap()
1904
+ .get(&session_key)
1905
+ .map(|state| state.conversation.clone());
1906
+ let Some(history) = session_store::load(&session_key).or(in_memory) else {
1907
+ log::warn!("load_session: no session {session_key}");
1908
+ return Err(agent_client_protocol::Error::new(
1909
+ -32002,
1910
+ format!("session {session_key} not found"),
1911
+ ));
1912
+ };
1913
+
1914
let mut state = self.session_state(&args.cwd, &args.additional_directories);
1915
if let Some(model_id) = session_store::list()
1916
.into_iter()
@@ -1903,29 +1925,26 @@ impl SiGitAgent {
1925
// editor may still have them open.
1926
permissions::reset_session(&session_key);
1927
1906
- // Durable sessions: when this session id was saved before, restore its
1907
- // conversation. The saved system messages are dropped; they are rebuilt
1908
- // from the roots the editor sent now, which may differ from last time.
1909
- if let Some(history) = session_store::load(&session_key) {
1910
- let restored = history.len();
1911
-
1912
- // The client draws the reopened thread from these notifications
1913
- // alone.
1914
- let updates = history_replay_updates(&history);
1915
- let replayed = updates.len();
1916
- for update in updates {
1917
- cx.send_notification(SessionNotification::new(args.session_id.clone(), update))
1918
- .ok();
1919
- }
1928
+ // Restore the conversation. The saved system messages are dropped;
1929
+ // they are rebuilt from the roots the editor sent now, which may differ
1930
+ // from last time.
1931
+ let restored = history.len();
1932
1921
- state.conversation = backend::carryover_history(history);
1922
- log::info!(
1923
- "load_session: restored {restored} message(s), replayed {replayed} update(s) for {}",
1924
- args.session_id
1925
- );
1933
+ // The client draws the reopened thread from these notifications alone.
1934
+ let updates = history_replay_updates(&history);
1935
+ let replayed = updates.len();
1936
+ for update in updates {
1937
+ cx.send_notification(SessionNotification::new(args.session_id.clone(), update))
1938
+ .ok();
1939
}
1940
1928
- self.open_session(&args.session_id, state).await;
1941
+ state.conversation = backend::carryover_history(history);
1942
+ log::info!(
1943
+ "load_session: restored {restored} message(s), replayed {replayed} update(s) for {}",
1944
+ args.session_id
1945
+ );
1946
+
1947
+ self.open_parked_session(&args.session_id, state).await;
1948
1949
let config_options = {
1950
let guard = self.current_model.lock().unwrap();
tests/acp_session_load.rs
+83
@@ -186,6 +186,22 @@ impl AgentUnderTest {
186
}
187
}
188
189
+impl AgentUnderTest {
190
+ /// The raw response to one of our requests, error or not.
191
+ fn wait_for_raw_response(&mut self, id: u64) -> Value {
192
+ let deadline = Instant::now() + TIMEOUT;
193
+ loop {
194
+ let remaining = deadline.saturating_duration_since(Instant::now());
195
+ let Ok(message) = self.incoming.recv_timeout(remaining) else {
196
+ panic!("timed out waiting for the response to request {id}");
197
+ };
198
+ if message["id"] == id && message.get("method").is_none() {
199
+ return message;
200
+ }
201
+ }
202
+ }
203
+}
204
+
205
impl Drop for AgentUnderTest {
206
fn drop(&mut self) {
207
let _ = self.child.kill();
@@ -318,3 +334,70 @@ fn loading_a_saved_session_replays_it_to_the_client() {
334
drop(agent);
335
let _ = std::fs::remove_dir_all(&scratch);
336
}
337
+
338
+/// A session id that was never saved and that this process has never opened
339
+/// names nothing. Answering with an empty session would leave the client
340
+/// showing a blank thread under the old title (issue #137).
341
+#[test]
342
+fn loading_an_unknown_session_is_an_error() {
343
+ let scratch =
344
+ std::env::temp_dir().join(format!("sigit_acp_load_unknown_{}", std::process::id()));
345
+ let config_dir = scratch.join("config");
346
+ let project = scratch.join("project");
347
+ std::fs::create_dir_all(&config_dir).unwrap();
348
+ std::fs::create_dir_all(&project).unwrap();
349
+
350
+ let port = start_fake_endpoint(Vec::new());
351
+ let mut agent = spawn_agent(port, &config_dir);
352
+
353
+ let id = agent.request(
354
+ "initialize",
355
+ json!({"protocolVersion": 1, "clientCapabilities": {}}),
356
+ );
357
+ agent.wait_for_response_with_updates(id);
358
+
359
+ let id = agent.request(
360
+ "session/load",
361
+ json!({"sessionId": "does-not-exist", "cwd": project, "mcpServers": []}),
362
+ );
363
+ let response = agent.wait_for_raw_response(id);
364
+ assert_eq!(
365
+ response["error"]["code"], -32002,
366
+ "an unknown id must be reported as not found: {response}"
367
+ );
368
+
369
+ // The failed load must not have registered the id as a side effect.
370
+ let id = agent.request(
371
+ "session/prompt",
372
+ json!({
373
+ "sessionId": "does-not-exist",
374
+ "prompt": [{"type": "text", "text": "/help"}],
375
+ }),
376
+ );
377
+ let response = agent.wait_for_raw_response(id);
378
+ assert!(
379
+ response.get("error").is_some(),
380
+ "the id must still be unknown after a failed load: {response}"
381
+ );
382
+
383
+ // A thread opened in this process but never spoken in has nothing on disk
384
+ // yet. It is still a session, so loading it keeps working.
385
+ let id = agent.request("session/new", json!({"cwd": project, "mcpServers": []}));
386
+ let (new_session, _) = agent.wait_for_response_with_updates(id);
387
+ let session_id = new_session["result"]["sessionId"]
388
+ .as_str()
389
+ .expect("session id")
390
+ .to_string();
391
+ let id = agent.request(
392
+ "session/load",
393
+ json!({"sessionId": session_id, "cwd": project, "mcpServers": []}),
394
+ );
395
+ let response = agent.wait_for_raw_response(id);
396
+ assert!(
397
+ response.get("error").is_none(),
398
+ "a session this process holds must load without a saved copy: {response}"
399
+ );
400
+
401
+ drop(agent);
402
+ let _ = std::fs::remove_dir_all(&scratch);
403
+}