From 0bbb9b752d5b644a8fd055b38d1dbd2d19bf2822 Mon Sep 17 00:00:00 2001 From: rob thijssen Date: Thu, 28 May 2026 14:34:41 +0300 Subject: [PATCH] feat(helexa-acp): session/list so Zed can discover sessions to resume MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stage 3b only implemented the trailing half of resume: write sessions to disk + handle session/load. But Zed (and any ACP client) needs `session/list` to discover *which* session belongs to the workspace it's reopening — without it, the client only knows how to mint new sessions and resume never fires even though the JSON sits ready on disk. Add the missing pieces: - store::list / list_in_dir — enumerate {id}.json under sessions_dir(), optionally filter by cwd, sort recent-first. Skips unparseable files with a warn rather than aborting. - store::unix_to_iso8601 — RFC 3339 formatter for SessionInfo.updated_at; pulls chrono in directly (already in the dep tree transitively). - agent::handle_list_sessions — wires the request to the store, builds SessionInfo entries with derived titles (first user turn, truncated to 60 chars). - agent::initialize_response — advertise session_capabilities.list = {} alongside the existing load_session: true. Verified end-to-end against the user's real hxa-1.json (60-turn beat conversation): `session/list` returns the entry with cwd, derived title, and ISO 8601 timestamp. 4 new store unit tests for list filtering, missing-dir handling, unparseable-file skipping, and ISO 8601 formatting. Co-Authored-By: Claude Opus 4.7 (1M context) --- Cargo.lock | 1 + crates/helexa-acp/Cargo.toml | 4 ++ crates/helexa-acp/src/agent.rs | 82 ++++++++++++++++++--- crates/helexa-acp/src/store.rs | 127 +++++++++++++++++++++++++++++++++ 4 files changed, 206 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9d0ef20..c3f30b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1819,6 +1819,7 @@ dependencies = [ "anyhow", "async-stream", "async-trait", + "chrono", "eventsource-stream", "futures", "reqwest", diff --git a/crates/helexa-acp/Cargo.toml b/crates/helexa-acp/Cargo.toml index 641da93..25e41f7 100644 --- a/crates/helexa-acp/Cargo.toml +++ b/crates/helexa-acp/Cargo.toml @@ -33,6 +33,10 @@ tokio-util = { version = "0.7", features = ["rt"] } eventsource-stream = "0.2" async-stream = "0.3" url = { version = "2", features = ["serde"] } +# Already transitively pulled via the ACP SDK; declared directly so we +# can format ISO 8601 timestamps for `SessionInfo.updated_at` in the +# session/list response. +chrono = { version = "0.4", default-features = false, features = ["std"] } [[bin]] name = "helexa-acp" diff --git a/crates/helexa-acp/src/agent.rs b/crates/helexa-acp/src/agent.rs index f588413..5c363c7 100644 --- a/crates/helexa-acp/src/agent.rs +++ b/crates/helexa-acp/src/agent.rs @@ -19,9 +19,10 @@ use std::sync::atomic::{AtomicU64, Ordering}; use agent_client_protocol::schema::{ AgentCapabilities, CancelNotification, ContentBlock, InitializeRequest, InitializeResponse, - LoadSessionRequest, LoadSessionResponse, NewSessionRequest, NewSessionResponse, - PromptCapabilities, PromptRequest, PromptResponse, SessionId, SessionMode, SessionModeId, - SessionModeState, SessionNotification, SessionUpdate, SetSessionModeRequest, + ListSessionsRequest, ListSessionsResponse, LoadSessionRequest, LoadSessionResponse, + NewSessionRequest, NewSessionResponse, PromptCapabilities, PromptRequest, PromptResponse, + SessionCapabilities, SessionId, SessionInfo, SessionListCapabilities, SessionMode, + SessionModeId, SessionModeState, SessionNotification, SessionUpdate, SetSessionModeRequest, SetSessionModeResponse, StopReason, TextContent, }; use agent_client_protocol::{Agent as AgentRole, Client, ConnectionTo, Dispatch, Stdio}; @@ -151,6 +152,15 @@ impl Agent { }, agent_client_protocol::on_receive_request!(), ) + .on_receive_request( + async move |req: ListSessionsRequest, responder, _cx| match handle_list_sessions( + req, + ) { + Ok(resp) => responder.respond(resp), + Err(e) => responder.respond_with_internal_error(format!("{e:#}")), + }, + agent_client_protocol::on_receive_request!(), + ) .on_receive_request( { let inner = inner.clone(); @@ -221,14 +231,18 @@ fn initialize_response(req: &InitializeRequest) -> InitializeResponse { // Stage 2: text-only prompts. Image / audio / embedded resources // flip on in later stages. let prompt_caps = PromptCapabilities::default(); + // Stage 3b: advertise both the top-level `load_session` flag and + // the `session/list` sub-capability. Zed (and other ACP clients) + // uses `session/list` to discover the session id that belongs to + // a workspace before sending `session/load` — without it, the + // client only knows how to mint new sessions and resume never + // fires regardless of what's on disk. + let session_caps = + SessionCapabilities::default().list(Some(SessionListCapabilities::default())); InitializeResponse::new(req.protocol_version).agent_capabilities( AgentCapabilities::new() .prompt_capabilities(prompt_caps) - // Stage 3b: `session/load` is implemented. Persisted - // sessions live on disk under - // `$XDG_DATA_HOME/helexa-acp/sessions/`; clients (Zed) - // can hand us back any session_id we previously - // minted to resume the conversation. + .session_capabilities(session_caps) .load_session(true), ) } @@ -315,6 +329,58 @@ async fn handle_load_session( Ok(LoadSessionResponse::new().modes(modes)) } +/// Enumerate persisted sessions for the `session/list` ACP method. +/// +/// Zed calls this on workspace open to find the session belonging +/// to the cwd it's reopening — without it, even though `session/load` +/// works, the client has no way to discover the session_id and +/// always falls back to `session/new`. That's exactly the +/// "history didn't survive the restart" symptom. +/// +/// Cursor pagination from the request is accepted but ignored: +/// helexa-acp's session counts are too small to need it. We always +/// return the whole filtered list with `next_cursor = None`. +fn handle_list_sessions(req: ListSessionsRequest) -> anyhow::Result { + let sessions = store::list(req.cwd.as_deref())?; + let infos: Vec = sessions + .into_iter() + .map(|s| { + let mut info = SessionInfo::new(SessionId::new(s.session_id), s.cwd); + info = info.title(derive_session_title(&s.history)); + info = info.updated_at(store::unix_to_iso8601(s.updated_at)); + info + }) + .collect(); + tracing::info!( + cwd = ?req.cwd, + count = infos.len(), + "session/list responded" + ); + Ok(ListSessionsResponse::new(infos)) +} + +/// Best-effort human-readable title for a session, derived from the +/// first user turn's text (truncated to ~60 chars). Empty string +/// becomes `None` so Zed can fall back to its own placeholder. +fn derive_session_title(history: &[Message]) -> Option { + history + .iter() + .find_map(|msg| match (msg.role, &msg.content) { + (Role::User, MessageContent::Text { text }) => Some(text.as_str()), + _ => None, + }) + .map(|s| { + let trimmed = s.trim(); + if trimmed.chars().count() > 60 { + let prefix: String = trimmed.chars().take(60).collect(); + format!("{prefix}…") + } else { + trimmed.to_string() + } + }) + .filter(|s| !s.is_empty()) +} + /// The two modes every Stage 3 session advertises. Stage 7 may grow /// this list (e.g. "plan" for plan-only output, "ask" for read-only), /// but Default + Bypass cover the two operationally distinct diff --git a/crates/helexa-acp/src/store.rs b/crates/helexa-acp/src/store.rs index 6674273..8c5acdd 100644 --- a/crates/helexa-acp/src/store.rs +++ b/crates/helexa-acp/src/store.rs @@ -116,6 +116,64 @@ pub fn load_from_dir( Ok(session) } +/// List all persisted sessions, optionally filtered by `cwd`. Used +/// by the `session/list` handler so a client (Zed) can find the +/// session that belongs to the workspace it's reopening. +/// +/// `filter_cwd = None` returns every session on disk. `Some(path)` +/// returns only sessions whose persisted `cwd` is exactly equal. +/// +/// Files that fail to parse are skipped with a warning rather than +/// aborting the whole list — one corrupt session shouldn't make +/// the resume picker unusable. +pub fn list(filter_cwd: Option<&std::path::Path>) -> anyhow::Result> { + let dir = sessions_dir() + .ok_or_else(|| anyhow::anyhow!("can't resolve XDG_DATA_HOME or HOME for session store"))?; + list_in_dir(&dir, filter_cwd) +} + +/// Explicit-dir variant for tests, mirroring [`save_to_dir`] / +/// [`load_from_dir`]. +pub fn list_in_dir( + dir: &std::path::Path, + filter_cwd: Option<&std::path::Path>, +) -> anyhow::Result> { + let read = match std::fs::read_dir(dir) { + Ok(r) => r, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(e) => return Err(anyhow::anyhow!("read_dir {}: {e}", dir.display())), + }; + let mut out = Vec::new(); + for entry in read.flatten() { + let path = entry.path(); + if path.extension().and_then(|s| s.to_str()) != Some("json") { + continue; + } + match std::fs::read(&path).and_then(|bytes| { + serde_json::from_slice::(&bytes).map_err(std::io::Error::other) + }) { + Ok(session) => { + if let Some(want) = filter_cwd + && session.cwd != want + { + continue; + } + out.push(session); + } + Err(e) => { + tracing::warn!( + path = %path.display(), + error = %e, + "store: skipping unparseable session file" + ); + } + } + } + // Most-recent first by updated_at. + out.sort_by_key(|s| std::cmp::Reverse(s.updated_at)); + Ok(out) +} + /// Seconds-since-epoch, saturating to 0 if the system clock is /// behind epoch (which shouldn't happen but the type system /// requires a fallible read). @@ -126,6 +184,16 @@ pub fn now_secs() -> u64 { .unwrap_or(0) } +/// Format seconds-since-epoch as an ISO 8601 / RFC 3339 string +/// (`YYYY-MM-DDTHH:MM:SSZ`) for `SessionInfo.updated_at`. Returns +/// `None` for values outside the representable range, in which +/// case the caller should omit the field. +pub fn unix_to_iso8601(secs: u64) -> Option { + use chrono::TimeZone; + let dt = chrono::Utc.timestamp_opt(secs as i64, 0).single()?; + Some(dt.to_rfc3339_opts(chrono::SecondsFormat::Secs, true)) +} + /// Strip anything that isn't a safe filename character so a /// mischievous (or just unconventional) session id can't escape /// the sessions directory. @@ -265,6 +333,65 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn list_filters_by_cwd_and_sorts_recent_first() { + let dir = unique_dir(); + let mut a = sample("a"); + a.cwd = PathBuf::from("/home/me/proj-x"); + a.updated_at = 1_700_000_010; + let mut b = sample("b"); + b.cwd = PathBuf::from("/home/me/proj-x"); + b.updated_at = 1_700_000_020; + let mut c = sample("c"); + c.cwd = PathBuf::from("/home/me/elsewhere"); + c.updated_at = 1_700_000_030; + save_to_dir(&dir, &a).unwrap(); + save_to_dir(&dir, &b).unwrap(); + save_to_dir(&dir, &c).unwrap(); + + let proj_x = PathBuf::from("/home/me/proj-x"); + let list = list_in_dir(&dir, Some(&proj_x)).unwrap(); + let ids: Vec<&str> = list.iter().map(|s| s.session_id.as_str()).collect(); + // Filtered to proj-x; b before a because b is more recent. + assert_eq!(ids, vec!["b", "a"]); + + let all = list_in_dir(&dir, None).unwrap(); + assert_eq!(all.len(), 3); + // Global list still sorted recent-first across all cwds. + assert_eq!(all[0].session_id, "c"); + + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn list_returns_empty_for_missing_dir() { + let dir = unique_dir().join("does-not-exist"); + let list = list_in_dir(&dir, None).unwrap(); + assert!(list.is_empty()); + } + + #[test] + fn list_skips_unparseable_files() { + let dir = unique_dir(); + save_to_dir(&dir, &sample("good")).unwrap(); + std::fs::write(dir.join("garbage.json"), b"{not valid json").unwrap(); + let list = list_in_dir(&dir, None).unwrap(); + // Garbage skipped; good survives. + assert_eq!(list.len(), 1); + assert_eq!(list[0].session_id, "good"); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn iso8601_formats_unix_seconds() { + // 2024-01-01T00:00:00Z is 1704067200 unix seconds. + assert_eq!( + unix_to_iso8601(1_704_067_200), + Some("2024-01-01T00:00:00Z".into()) + ); + assert_eq!(unix_to_iso8601(0), Some("1970-01-01T00:00:00Z".into())); + } + #[test] fn sanitize_id_rejects_path_traversal() { // `../../etc/passwd` — 6 non-alnum chars before "etc"