diff --git a/nori-rs/acp/docs.md b/nori-rs/acp/docs.md index 599fe276b..c744f9f73 100644 --- a/nori-rs/acp/docs.md +++ b/nori-rs/acp/docs.md @@ -119,7 +119,7 @@ The ACP backend owns the `/goal` feature as per-session state instead of delegat `nori_client_mcp.rs` is a bridge, not a second store. The goal tools are typed rmcp `#[tool]` handlers on `NoriClientService`; they lock the same `ThreadGoalState` used by TUI `/goal` operations, return JSON snapshots shaped for model consumption, and emit the same `ThreadGoalUpdated` client event after mutations. The bridge records those emitted events through `@/nori-rs/acp/src/backend/transcript.rs` when a transcript recorder is available; `NoriClientShared` stores the recorder behind a shared cell because the service is built before the session id is known. -The local `nori-client` server is only advertised when `register_for_session` in `@/nori-rs/acp/src/backend/nori_client_mcp.rs` sees HTTP MCP support from `@/nori-rs/acp/src/connection/mod.rs`. Nori advertises a real `http://127.0.0.1:/mcp` endpoint rather than an ACP pseudo-URL, because Codex ACP and Claude ACP both forward ACP `mcpServers` to their underlying clients as ordinary HTTP MCP server config. Each eligible session registration gets a fresh loopback server with a generated bearer token advertised as an `Authorization` header; an `axum` middleware rejects unauthenticated requests before they reach rmcp's `StreamableHttpService` (stateless mode). The server is owned by the ACP backend (abort-on-drop) and talks directly to the same in-memory goal state as `/goal`. The server is named `nori-client` rather than `nori-goal` because it is Nori's general harness-side channel to the external ACP agent -- the single point of contact for harness-specific tooling the ACP protocol does not yet provide -- and goal tools are only its live-state surface. The backend emits a `SessionCapabilitiesChanged` projection after session setup so clients can derive built-in command availability from the same capability state. That projection separates raw agent capabilities (`agent.http_mcp`, `agent.load_session`), `nori_client.advertised`, `nori_client.initialized`, and derived `builtin_commands` availability. Initial and post-replacement snapshots read the same connected flag flipped by MCP `initialize`, so agents that eagerly initialize the advertised server during session setup do not observe initialized state regress from true back to false. +The local `nori-client` server is only advertised when `register_for_session` in `@/nori-rs/acp/src/backend/nori_client_mcp.rs` sees HTTP MCP support from `@/nori-rs/acp/src/connection/mod.rs`. Nori advertises a real `http://127.0.0.1:/mcp` endpoint rather than an ACP pseudo-URL, because Codex ACP and Claude ACP both forward ACP `mcpServers` to their underlying clients as ordinary HTTP MCP server config. Each eligible session registration gets a fresh loopback server with a generated bearer token advertised as an `Authorization` header; an `axum` middleware rejects unauthenticated requests before they reach rmcp's `StreamableHttpService` (stateless mode). The server is owned by the ACP backend (abort-on-drop) and talks directly to the same in-memory goal state as `/goal`. The server is named `nori-client` rather than `nori-goal` because it is Nori's general harness-side channel to the external ACP agent -- the single point of contact for harness-specific tooling the ACP protocol does not yet provide -- and goal tools are only its live-state surface. The backend emits a `SessionCapabilitiesChanged` projection after session setup so clients can derive built-in command availability from the same capability state. That projection separates raw agent capabilities (`agent.http_mcp`, `agent.load_session`, `agent.session_list`), `nori_client.advertised`, `nori_client.initialized`, and derived `builtin_commands` availability. `agent.session_list` is the raw projection of the agent's ACP `session/list` capability (`session_capabilities.list.is_some()`), set at both the `capabilities_update_for_nori_client` and `register_for_session` build sites in `@/nori-rs/acp/src/backend/nori_client_mcp.rs`; the TUI consumes it to decide whether the in-session `/resume` picker is sourced from the live agent (see `@/nori-rs/tui/docs.md`). Initial and post-replacement snapshots read the same connected flag flipped by MCP `initialize`, so agents that eagerly initialize the advertised server during session setup do not observe initialized state regress from true back to false. `@/nori-rs/acp/src/backend/nori_client_context.rs` owns the fixed read-only catalog exposed through the same server. `nori_client_mcp.rs` advertises tools, resources, and prompts in `ServerInfo`, but delegates list/read/get operations to that sibling module so transport, initialization, connected-gate behavior, and capability registration stay separate from Nori's curated operating context. The catalog intentionally serves Nori-owned harness facts, minimal ACP debugging and custom-agent help, and a compact source map for answering Nori CLI questions; it is not an arbitrary filesystem, repo-read, or skill-workflow API. Unknown resource URIs and prompt names are rejected as MCP errors, keeping the context surface closed and predictable. @@ -1034,6 +1034,8 @@ The `ContextCompactedEvent.summary` field is the coupling point between the ACP `resume_session()` always spawns the agent via `AcpConnection::spawn()` -- like every other code path in this crate, it only knows about local subprocess agents. +The session *list* that feeds the in-session `/resume` picker can be sourced two ways. When the live agent advertises `session/list` (surfaced as `agent.session_list`), `@/nori-rs/tui` calls `AcpConnection::list_sessions()` on the already-running connection to enumerate the agent's own resumable sessions, then resumes the selected one over ACP via `session/load` against that same session (no local transcript involved). Otherwise the TUI falls back to enumerating the local on-disk transcript store. Either way, the chosen session is resumed through the `resume_session()` strategy selection above; the agent-sourced path passes `transcript: None` and relies on server-side `session/load` replay rather than transcript-derived replay. The standalone `nori resume` startup picker is unaffected by `session/list` and remains transcript-backed. See `@/nori-rs/acp/src/connection/docs.md` for `list_sessions()` and the `AcpSessionSummary` boundary type. + ``` AcpBackend::resume_session(config, acp_session_id, transcript, backend_event_tx) | diff --git a/nori-rs/acp/src/backend/nori_client_mcp.rs b/nori-rs/acp/src/backend/nori_client_mcp.rs index b56d4bbb3..37c707597 100644 --- a/nori-rs/acp/src/backend/nori_client_mcp.rs +++ b/nori-rs/acp/src/backend/nori_client_mcp.rs @@ -466,6 +466,11 @@ pub(super) fn capabilities_update_for_nori_client( let agent = nori_protocol::AgentCapabilitiesView { http_mcp: connection.capabilities().mcp_capabilities.http, load_session: connection.capabilities().load_session, + session_list: connection + .capabilities() + .session_capabilities + .list + .is_some(), }; capabilities_update(agent, nori_client_advertised, nori_client_initialized) } @@ -514,6 +519,11 @@ pub(super) async fn register_for_session( nori_protocol::AgentCapabilitiesView { http_mcp: connection.capabilities().mcp_capabilities.http, load_session: connection.capabilities().load_session, + session_list: connection + .capabilities() + .session_capabilities + .list + .is_some(), }, true, )) @@ -541,6 +551,7 @@ mod tests { nori_protocol::AgentCapabilitiesView { http_mcp: true, load_session: false, + session_list: false, } } diff --git a/nori-rs/acp/src/connection/acp_connection.rs b/nori-rs/acp/src/connection/acp_connection.rs index 8fff086c1..f440d4edc 100644 --- a/nori-rs/acp/src/connection/acp_connection.rs +++ b/nori-rs/acp/src/connection/acp_connection.rs @@ -699,6 +699,53 @@ impl AcpConnection { Ok(acp::SessionId::from(session_id.to_string())) } + /// List the agent's known sessions via ACP `session/list`. + /// + /// `cwd` is forwarded as the spec's working-directory filter. Cursor-based + /// pagination is drained internally, so the returned vector contains every + /// page concatenated in agent order. + pub async fn list_sessions( + &self, + cwd: &Path, + ) -> Result> { + // Bound pagination so a misbehaving agent that keeps returning a + // `next_cursor` cannot pin the picker in an unbounded loop. + const MAX_SESSION_LIST_PAGES: usize = 1000; + + let mut summaries = Vec::new(); + let mut cursor: Option = None; + + for _ in 0..MAX_SESSION_LIST_PAGES { + let mut request = acp::ListSessionsRequest::new().cwd(cwd.to_path_buf()); + if let Some(cursor) = cursor.take() { + request = request.cursor(cursor); + } + + let response = self + .cx + .send_request(request) + .block_task() + .await + .context("Failed to list ACP sessions")?; + + summaries.extend(response.sessions.into_iter().map(|session| { + crate::connection::AcpSessionSummary { + session_id: session.session_id.to_string(), + cwd: session.cwd, + title: session.title, + updated_at: session.updated_at, + } + })); + + match response.next_cursor { + Some(next) => cursor = Some(next), + None => break, + } + } + + Ok(summaries) + } + /// Send a prompt to an existing session and receive streaming updates. /// /// Updates flow through the ordered event inbox. diff --git a/nori-rs/acp/src/connection/acp_connection_tests.rs b/nori-rs/acp/src/connection/acp_connection_tests.rs index fe96f0ec2..9e99e6967 100644 --- a/nori-rs/acp/src/connection/acp_connection_tests.rs +++ b/nori-rs/acp/src/connection/acp_connection_tests.rs @@ -1155,3 +1155,60 @@ async fn test_spawn_failure_surfaces_child_stderr() { "the surfaced stderr must drive categorization (auth, not init failure)" ); } + +/// `list_sessions` drains the agent's `session/list` pages and maps each +/// `SessionInfo` into an owned `AcpSessionSummary`, preserving the optional +/// `title`/`updated_at` fields (including their absence). +#[tokio::test] +#[serial] +async fn test_list_sessions_maps_agent_session_info() { + let Some(config) = mock_agent_config() else { + return; + }; + let temp_dir = tempdir().expect("temp dir"); + + // SAFETY: serialized test; the variable is removed before any assertion + // that could unwind, and `#[serial]` keeps env mutation isolated. + unsafe { + std::env::set_var("MOCK_AGENT_SUPPORT_SESSION_LIST", "1"); + } + + let conn = AcpConnection::spawn( + &config, + temp_dir.path(), + crate::config::AcpProxyConfig::disabled(), + ) + .await + .expect("Failed to spawn AcpConnection"); + + assert!( + conn.capabilities().session_capabilities.list.is_some(), + "mock should advertise session/list when env-gated on" + ); + + let summaries = conn + .list_sessions(temp_dir.path()) + .await + .expect("list_sessions should succeed against a live agent"); + + // SAFETY: cleaning up the env var we set above. + unsafe { + std::env::remove_var("MOCK_AGENT_SUPPORT_SESSION_LIST"); + } + + let expected = vec![ + super::AcpSessionSummary { + session_id: "mock-session-1".to_string(), + cwd: temp_dir.path().to_path_buf(), + title: Some("First mock session".to_string()), + updated_at: Some("2026-01-02T03:04:05Z".to_string()), + }, + super::AcpSessionSummary { + session_id: "mock-session-2".to_string(), + cwd: temp_dir.path().to_path_buf(), + title: None, + updated_at: None, + }, + ]; + assert_eq!(summaries, expected); +} diff --git a/nori-rs/acp/src/connection/docs.md b/nori-rs/acp/src/connection/docs.md index ec92ec994..5c3413941 100644 --- a/nori-rs/acp/src/connection/docs.md +++ b/nori-rs/acp/src/connection/docs.md @@ -31,7 +31,7 @@ Local ACP Agent (subprocess, own process group) ### Core Implementation -- **`AcpConnection`** (`acp_connection.rs`): The central `Send + Sync` type that owns the SDK connection task, the child's teardown handles, and the event receiver. Key methods: `create_session()`, `load_session()`, `prompt()`, `cancel()`, `set_config_option()` +- **`AcpConnection`** (`acp_connection.rs`): The central `Send + Sync` type that owns the SDK connection task, the child's teardown handles, and the event receiver. Key methods: `create_session()`, `load_session()`, `list_sessions()`, `prompt()`, `cancel()`, `set_config_option()` - **`establish_connection()`**: A free function that registers all SDK handlers (notification, permission request, file read/write) and performs the initialization handshake. The caller (`spawn()`) supplies the `ConnectionEvent` channel so extra producers -- notably the child exit watcher -- can report through the same ordered stream - **Child exit watcher**: A background task OWNS the `Child`: it `wait()`s and reaps the process, publishes the exit status on a `watch` channel, and emits `ConnectionEvent::ChildExited { status, stderr_tail }` (defined in `mod.rs`) when the child dies. The connection holds only a `ChildHandle` (pid, exit-status receiver, kill `Notify`) -- never the `Child` itself - **stderr capture**: A background task logs each stderr line via tracing and keeps a bounded tail of recent lines. The tail is attached to startup failures and `ChildExited` events so the real cause (e.g. an auth hint) is user-visible @@ -49,5 +49,6 @@ Local ACP Agent (subprocess, own process group) - Without the `ChildExited` event, a dead child is a silent EOF that the ACP SDK layer treats as non-terminal -- pending requests would hang forever. The backend relay turns it into a visible error and fails any in-flight prompt - File write handlers restrict writes to the workspace directory or `/tmp` (canonicalized path check). File read handlers are unrestricted. Both emit synthetic `ToolCall` updates for TUI rendering - `AcpConnection::prompt()` absorbs stale cancel-tail responses (empty `end_turn` responses left over from a previous cancellation) by retrying until streamed updates arrive, keeping the reducer contract unchanged +- `list_sessions()` sends the ACP `session/list` request and drains cursor pagination internally (following `next_cursor` until exhausted, bounded by a page cap so a misbehaving agent that keeps returning a cursor cannot loop unbounded), concatenating every page in agent order. Each ACP `SessionInfo` is mapped into `AcpSessionSummary` (defined in `mod.rs`, re-exported from `nori_acp`), an owned boundary type carrying `session_id`, `cwd`, optional `title`, and optional `updated_at`. The boundary type decouples consumers from the raw ACP schema, letting `@/nori-rs/tui` -- which has no ACP-schema dependency -- render the agent-sourced `/resume` picker. It mirrors `load_session()`'s structure; the agent only honors `session/list` when it advertises the capability (projected as `agent.session_list`, see `@/nori-rs/acp/docs.md`) Created and maintained by Nori. diff --git a/nori-rs/acp/src/connection/mod.rs b/nori-rs/acp/src/connection/mod.rs index bc4d63908..708b2da63 100644 --- a/nori-rs/acp/src/connection/mod.rs +++ b/nori-rs/acp/src/connection/mod.rs @@ -90,6 +90,22 @@ pub struct ApprovalRequest { pub response_tx: oneshot::Sender, } +/// Owned summary of one session returned by ACP `session/list`. +/// +/// This decouples consumers (e.g. the TUI resume picker) from the raw ACP +/// schema types, exposing only the fields the picker renders. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AcpSessionSummary { + /// The agent's session identifier, used to resume via `session/load`. + pub session_id: String, + /// The working directory the session was created in (absolute path). + pub cwd: std::path::PathBuf, + /// Human-readable session title, when the agent provides one. + pub title: Option, + /// ISO 8601 timestamp of last activity, when the agent provides one. + pub updated_at: Option, +} + /// Session config state captured from ACP session setup and updates. /// /// This stores the complete current `configOptions` snapshot for the active diff --git a/nori-rs/acp/src/lib.rs b/nori-rs/acp/src/lib.rs index 00ddb2f57..8180de203 100644 --- a/nori-rs/acp/src/lib.rs +++ b/nori-rs/acp/src/lib.rs @@ -41,6 +41,7 @@ pub use backend::AcpBackend; pub use backend::AcpBackendConfig; pub use backend::BackendEvent; pub use connection::AcpSessionConfigState; +pub use connection::AcpSessionSummary; pub use connection::ApprovalRequest; pub use connection::acp_connection::AcpConnection; pub use registry::AcpAgentConfig; diff --git a/nori-rs/mock-acp-agent/docs.md b/nori-rs/mock-acp-agent/docs.md index 7b16938e1..f39913576 100644 --- a/nori-rs/mock-acp-agent/docs.md +++ b/nori-rs/mock-acp-agent/docs.md @@ -12,7 +12,7 @@ Used by `@/nori-rs/tui-pty-e2e/` for end-to-end integration testing. The mock ag ### Core Implementation -**Agent wiring**: The mock is assembled in `main()` from the SDK's `Agent.builder()` with typed `on_receive_request`/`on_receive_notification` handler closures -- one per method (`initialize`, `authenticate`, `session/new`, `session/load`, `session/prompt`, `session/set_mode`, `session/set_config_option`, plus the `session/cancel` notification) -- then connected over `ByteStreams` to the wrapped stdin/stdout. Each closure receives the typed request/notification, a `Responder`, and a `ConnectionTo`; the closures share state through an `Arc`. Handled concerns include: +**Agent wiring**: The mock is assembled in `main()` from the SDK's `Agent.builder()` with typed `on_receive_request`/`on_receive_notification` handler closures -- one per method (`initialize`, `authenticate`, `session/new`, `session/load`, `session/list`, `session/prompt`, `session/set_mode`, `session/set_config_option`, plus the `session/cancel` notification) -- then connected over `ByteStreams` to the wrapped stdin/stdout. Each closure receives the typed request/notification, a `Responder`, and a `ConnectionTo`; the closures share state through an `Arc`. Handled concerns include: - Session creation, load (history replay), and config-option/mode mutation - Prompt processing with simulated responses, streaming, and tool calls - Permission request/response flow @@ -22,6 +22,7 @@ Used by `@/nori-rs/tui-pty-e2e/` for end-to-end integration testing. The mock ag **Session Lifecycle Testing**: Several env vars control `session/load` behavior for testing the resume path in `@/nori-rs/acp/src/backend/session.rs`: - `MOCK_AGENT_SUPPORT_LOAD_SESSION` -- when set, the agent advertises `load_session: true` in its capabilities during `initialize()` +- `MOCK_AGENT_SUPPORT_SESSION_LIST` -- when set, the agent advertises the ACP `session/list` capability during `initialize()` and its `session/list` handler returns two canned `SessionInfo` rows; exercises the agent-sourced `/resume` picker wire path (`AcpConnection::list_sessions()` in `@/nori-rs/acp/src/connection/`, surfaced as `agent.session_list`) - `MOCK_AGENT_MCP_HTTP` -- when set, the agent advertises HTTP MCP capability so the backend-owned `nori-client` MCP server in `@/nori-rs/acp/src/backend/nori_client_mcp.rs` can be tested through the normal `session/new` MCP server advertisement path - `MOCK_AGENT_INITIALIZE_NORI_CLIENT_DURING_NEW_SESSION` -- when set, `new_session()` eagerly sends an MCP `initialize` request to the advertised `nori-client` server before returning, mirroring agents that initialize advertised MCP servers during session setup - `MOCK_AGENT_FAIL_NEW_SESSION_FROM` -- when set to an integer N, `new_session()` returns an error once the generated session id is at least N, allowing tests to exercise replacement-session failures without breaking the initial backend startup diff --git a/nori-rs/mock-acp-agent/src/main.rs b/nori-rs/mock-acp-agent/src/main.rs index c8de799f9..880765f55 100644 --- a/nori-rs/mock-acp-agent/src/main.rs +++ b/nori-rs/mock-acp-agent/src/main.rs @@ -1341,6 +1341,14 @@ async fn main() -> acp::Result<()> { has_capabilities = true; } + if std::env::var("MOCK_AGENT_SUPPORT_SESSION_LIST").is_ok() { + eprintln!("Mock agent: advertising session/list capability"); + capabilities = capabilities.session_capabilities( + acp::SessionCapabilities::new().list(acp::SessionListCapabilities::new()), + ); + has_capabilities = true; + } + if has_capabilities { response = response.agent_capabilities(capabilities); } @@ -1449,6 +1457,24 @@ async fn main() -> acp::Result<()> { }, agent_client_protocol::on_receive_request!(), ) + .on_receive_request( + async move |arguments: acp::ListSessionsRequest, + responder: Responder, + _cx: ConnectionTo| { + eprintln!("Mock agent: session/list"); + let cwd = arguments + .cwd + .unwrap_or_else(|| PathBuf::from("/mock/session/cwd")); + let sessions = vec![ + acp::SessionInfo::new("mock-session-1", cwd.clone()) + .title("First mock session") + .updated_at("2026-01-02T03:04:05Z"), + acp::SessionInfo::new("mock-session-2", cwd), + ]; + responder.respond(acp::ListSessionsResponse::new(sessions)) + }, + agent_client_protocol::on_receive_request!(), + ) .on_receive_request( { let state = state.clone(); diff --git a/nori-rs/nori-protocol/src/lib.rs b/nori-rs/nori-protocol/src/lib.rs index 19628adcb..48d7600b2 100644 --- a/nori-rs/nori-protocol/src/lib.rs +++ b/nori-rs/nori-protocol/src/lib.rs @@ -108,6 +108,9 @@ pub struct SessionCapabilitiesView { pub struct AgentCapabilitiesView { pub http_mcp: bool, pub load_session: bool, + /// Whether the agent advertises the ACP `session/list` capability. + #[serde(default)] + pub session_list: bool, } #[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize)] @@ -1856,6 +1859,7 @@ mod tests { agent: AgentCapabilitiesView { http_mcp: false, load_session: true, + session_list: false, }, nori_client: NoriClientCapabilitiesView { advertised: false, diff --git a/nori-rs/tui-pty-e2e/tests/acp_resume_session_list.rs b/nori-rs/tui-pty-e2e/tests/acp_resume_session_list.rs new file mode 100644 index 000000000..62ded192c --- /dev/null +++ b/nori-rs/tui-pty-e2e/tests/acp_resume_session_list.rs @@ -0,0 +1,57 @@ +//! E2E test for the agent-sourced `/resume` session picker. +//! +//! When the live ACP agent advertises both the `session/list` and +//! `load_session` capabilities, the in-session `/resume` command sources its +//! picker rows from the agent (via `session/list`) instead of the local +//! transcript store, and resuming a row loads it over ACP via `session/load`. +//! This drives the real `nori` binary against the mock agent and verifies the +//! agent's sessions show up in the picker and can be resumed. + +use std::time::Duration; +use tui_pty_e2e::Key; +use tui_pty_e2e::SessionConfig; +use tui_pty_e2e::TIMEOUT; +use tui_pty_e2e::TIMEOUT_INPUT; +use tui_pty_e2e::TuiSession; + +#[test] +#[cfg(target_os = "linux")] +fn resume_picker_lists_agent_sessions_when_session_list_supported() { + let config = SessionConfig::new() + .with_model("mock-model".to_owned()) + .with_agent_env("MOCK_AGENT_SUPPORT_SESSION_LIST", "1") + .with_agent_env("MOCK_AGENT_SUPPORT_LOAD_SESSION", "1"); + + let mut session = + TuiSession::spawn_with_config(30, 100, config).expect("Failed to spawn session"); + + session + .wait_for_text("›", TIMEOUT) + .expect("TUI should start"); + // Let the session-capability projection arrive before opening the picker. + std::thread::sleep(TIMEOUT_INPUT); + + // Dispatch the in-session `/resume` slash command. + session.send_str("/resume").unwrap(); + std::thread::sleep(TIMEOUT_INPUT); + session.send_key(Key::Enter).unwrap(); + + session + .wait_for_text("Resume previous session", Duration::from_secs(5)) + .expect("Should show the agent-sourced resume picker"); + + // The mock agent returns two sessions: the first carries a title, the second + // falls back to its session id because it has none. + session + .wait_for_text("First mock session", Duration::from_secs(5)) + .expect("Picker should list the agent's titled session"); + session + .wait_for_text("mock-session-2", Duration::from_secs(2)) + .expect("Picker should list the untitled session by its id"); + + // Selecting the first (default-highlighted) row resumes it over ACP. + session.send_key(Key::Enter).unwrap(); + session + .wait_for_text("Resuming session with", Duration::from_secs(5)) + .expect("Selecting a listed session should resume it over ACP"); +} diff --git a/nori-rs/tui/docs.md b/nori-rs/tui/docs.md index 4178eb290..d7b572a33 100644 --- a/nori-rs/tui/docs.md +++ b/nori-rs/tui/docs.md @@ -859,7 +859,9 @@ Resume hints use the shared `RESUME_HINT_LEAD` and `resume_command_for_conversat The `/resume` command allows reconnecting to a previous ACP session. It uses the ACP agent's `session/load` RPC when available, and otherwise falls back to a fresh ACP session plus normalized replay derived from the saved transcript (see `@/nori-rs/acp/docs.md`). -The flow involves three layers: +The picker list itself comes from one of two sources. `ChatWidget::open_resume_session_picker()` has a capability-gated branch: when the agent advertises *both* `session/list` and `load_session` (`self.session_agent_capabilities.session_list && self.session_agent_capabilities.load_session`) and an `acp_handle` exists, it spawns an async task that calls `handle.list_sessions(cwd)` on the live agent (via the `ListSessions { cwd, response_tx }` `AcpAgentCommand`) and emits `AppEvent::ShowAcpResumeSessionPicker { sessions }` with the agent's own `AcpSessionSummary` rows. An empty result inserts a "no resumable sessions" error cell and a list failure inserts an error cell instead of opening a picker. Otherwise it falls back to the existing local-transcript picker (`AppEvent::ShowResumeSessionPicker`) described below. `load_session` is required in addition to `session_list` because resuming an agent-sourced row passes `transcript: None` and depends entirely on server-side `session/load` replay; without `load_session` an agent would silently start a blank session, so such agents fall through to the transcript-backed picker. The `session_list` capability is the raw agent-capability projection sourced from `@/nori-rs/acp`; this is generic to any agent that advertises ACP `session/list`, not Nori/Codex-specific. Selecting an agent-sourced row emits `AppEvent::ResumeAcpSession { acp_session_id }`, whose handler shuts down the current conversation and starts a new resumed ACP chat widget via `new_resumed_acp(init, Some(acp_session_id), None)` -- there is no local transcript, so server-side `session/load` replay rehydrates history. `show_acp_resume_session_picker()` builds the modal via `acp_resume_session_picker_params()` in `@/nori-rs/tui/src/nori/resume_session_picker.rs`, mapping each summary to a row (name = title falling back to session_id, description = relative time plus cwd, action = `ResumeAcpSession`). + +The transcript-backed flow involves three layers: ``` SlashCommand::Resume @@ -894,7 +896,7 @@ Lazy picker summaries: after `ShowResumeSessionPicker` is sent, `ChatWidget::ope The resume session picker reuses the `SessionPickerInfo` type and `format_relative_time()` utility from `@/nori-rs/tui/src/nori/viewonly_session_picker.rs`. The `format_relative_time` function was made `pub(crate)` for this reuse. -`spawn_acp_agent_resume()` in `@/nori-rs/tui/src/chatwidget/agent.rs` mirrors `spawn_acp_agent()` but calls `AcpBackend::resume_session()` instead of `AcpBackend::spawn()`, passing the optional `acp_session_id` and the full `Transcript`. Both spawn paths receive a single `BackendEvent` stream from `nori-acp`: normalized `ClientEvent` items drive ACP session rendering, while `Control` events still carry shared app-level concerns such as `SessionConfigured`, warnings, and shutdown. +`spawn_acp_agent_resume()` in `@/nori-rs/tui/src/chatwidget/agent.rs` mirrors `spawn_acp_agent()` but calls `AcpBackend::resume_session()` instead of `AcpBackend::spawn()`, passing the optional `acp_session_id` and an `Option` (the transcript-backed `/resume` and `nori resume` paths supply `Some`; the agent-sourced `session/list` path supplies `None` and relies on server-side replay). Both spawn paths receive a single `BackendEvent` stream from `nori-acp`: normalized `ClientEvent` items drive ACP session rendering, while `Control` events still carry shared app-level concerns such as `SessionConfigured`, warnings, and shutdown. **Agent Connection Lifecycle & Failure Recovery:** diff --git a/nori-rs/tui/src/app/event_handling.rs b/nori-rs/tui/src/app/event_handling.rs index 749cf3f5e..c418a87cf 100644 --- a/nori-rs/tui/src/app/event_handling.rs +++ b/nori-rs/tui/src/app/event_handling.rs @@ -1054,6 +1054,9 @@ impl App { self.chat_widget .show_resume_session_picker(params, generation); } + AppEvent::ShowAcpResumeSessionPicker { sessions } => { + self.chat_widget.show_acp_resume_session_picker(sessions); + } AppEvent::ResumeSessionSummaryReady { generation, session_id, @@ -1094,7 +1097,7 @@ impl App { None, ); self.chat_widget = - ChatWidget::new_resumed_acp(init, acp_session_id, transcript); + ChatWidget::new_resumed_acp(init, acp_session_id, Some(transcript)); self.configure_new_chat_widget(); self.chat_widget.add_info_message( @@ -1109,6 +1112,28 @@ impl App { } } } + AppEvent::ResumeAcpSession { acp_session_id } => { + let display_name = crate::nori::agent_picker::get_agent_info(&self.config.model) + .map(|info| info.display_name) + .unwrap_or_else(|| self.config.model.clone()); + + self.shutdown_current_conversation(); + + let init = self.chat_widget_init( + tui.frame_requester(), + None, + Vec::new(), + None, + false, + None, + ); + self.chat_widget = ChatWidget::new_resumed_acp(init, Some(acp_session_id), None); + self.configure_new_chat_widget(); + + self.chat_widget + .add_info_message(format!("Resuming session with {display_name}..."), None); + tui.frame_requester().schedule_frame(); + } #[cfg(unix)] AppEvent::BrowserLaunched { ws_url, cdp_port } => { let prompt = diff --git a/nori-rs/tui/src/app/mod.rs b/nori-rs/tui/src/app/mod.rs index fd807f2cf..7bb931cc8 100644 --- a/nori-rs/tui/src/app/mod.rs +++ b/nori-rs/tui/src/app/mod.rs @@ -353,7 +353,7 @@ impl App { .load_transcript(&target.project_id, &target.session_id) .await?; let acp_session_id = transcript.meta.acp_session_id.clone(); - ChatWidget::new_resumed_acp(init, acp_session_id, transcript) + ChatWidget::new_resumed_acp(init, acp_session_id, Some(transcript)) } ResumeSelection::StartFresh | ResumeSelection::Exit => ChatWidget::new(init), } diff --git a/nori-rs/tui/src/app_event.rs b/nori-rs/tui/src/app_event.rs index 235d175b5..1b4423e8b 100644 --- a/nori-rs/tui/src/app_event.rs +++ b/nori-rs/tui/src/app_event.rs @@ -481,6 +481,20 @@ pub(crate) enum AppEvent { session_id: String, }, + /// Show the resume session picker sourced from the live agent's ACP + /// `session/list` rather than the local transcript store. + ShowAcpResumeSessionPicker { + /// Session summaries reported by the agent. + sessions: Vec, + }, + + /// Resume a session reported by the agent's `session/list`, via + /// `session/load` with no local transcript (the agent replays history). + ResumeAcpSession { + /// The agent's session identifier to load. + acp_session_id: String, + }, + /// Launch a terminal file manager to browse and optionally edit files. #[cfg(feature = "nori-config")] BrowseFiles(nori_acp::config::FileManager), diff --git a/nori-rs/tui/src/chatwidget/agent.rs b/nori-rs/tui/src/chatwidget/agent.rs index c76d4560a..e2ae96aea 100644 --- a/nori-rs/tui/src/chatwidget/agent.rs +++ b/nori-rs/tui/src/chatwidget/agent.rs @@ -5,6 +5,7 @@ use codex_core::config::Config; use codex_core::protocol::Op; use nori_acp::AcpBackend; use nori_acp::AcpBackendConfig; +use nori_acp::AcpSessionSummary; use nori_acp::HistoryPersistence; use nori_acp::SessionConfigOption; use nori_acp::find_nori_home; @@ -63,6 +64,11 @@ pub(crate) enum AcpAgentCommand { value: String, response_tx: oneshot::Sender>>, }, + /// List the agent's known sessions via ACP `session/list`. + ListSessions { + cwd: std::path::PathBuf, + response_tx: oneshot::Sender>>, + }, } /// Handle for communicating with an ACP agent. @@ -111,6 +117,20 @@ impl AcpAgentHandle { .await .map_err(|_| anyhow::anyhow!("ACP agent did not respond"))? } + + /// List the agent's known sessions via ACP `session/list`. + pub async fn list_sessions( + &self, + cwd: std::path::PathBuf, + ) -> anyhow::Result> { + let (response_tx, response_rx) = oneshot::channel(); + self.command_tx + .send(AcpAgentCommand::ListSessions { cwd, response_tx }) + .map_err(|_| anyhow::anyhow!("ACP agent command channel closed"))?; + response_rx + .await + .map_err(|_| anyhow::anyhow!("ACP agent did not respond"))? + } } /// Result of spawning an agent, which may include an ACP handle for model control. @@ -328,6 +348,10 @@ fn spawn_acp_agent( .map(|()| backend_for_agent.config_options()); let _ = response_tx.send(result); } + AcpAgentCommand::ListSessions { cwd, response_tx } => { + let result = backend_for_agent.connection().list_sessions(&cwd).await; + let _ = response_tx.send(result); + } } } }); @@ -364,7 +388,7 @@ fn spawn_acp_agent( pub(crate) fn spawn_acp_agent_resume( config: Config, acp_session_id: Option, - transcript: nori_acp::transcript::Transcript, + transcript: Option, app_event_tx: AppEventSender, ) -> SpawnAgentResult { let (codex_op_tx, mut codex_op_rx) = unbounded_channel::(); @@ -444,7 +468,7 @@ pub(crate) fn spawn_acp_agent_resume( result = AcpBackend::resume_session( &acp_config, acp_session_id.as_deref(), - Some(&transcript), + transcript.as_ref(), backend_event_tx, ) => { match result { @@ -505,6 +529,10 @@ pub(crate) fn spawn_acp_agent_resume( .map(|()| backend_for_agent.config_options()); let _ = response_tx.send(result); } + AcpAgentCommand::ListSessions { cwd, response_tx } => { + let result = backend_for_agent.connection().list_sessions(&cwd).await; + let _ = response_tx.send(result); + } } } }); diff --git a/nori-rs/tui/src/chatwidget/constructors.rs b/nori-rs/tui/src/chatwidget/constructors.rs index 7c18d8dce..921f54c30 100644 --- a/nori-rs/tui/src/chatwidget/constructors.rs +++ b/nori-rs/tui/src/chatwidget/constructors.rs @@ -132,7 +132,7 @@ impl ChatWidget { pub(crate) fn new_resumed_acp( common: ChatWidgetInit, acp_session_id: Option, - transcript: nori_acp::transcript::Transcript, + transcript: Option, ) -> Self { let ChatWidgetInit { config, diff --git a/nori-rs/tui/src/chatwidget/pickers.rs b/nori-rs/tui/src/chatwidget/pickers.rs index 607dfe6a0..5761333d4 100644 --- a/nori-rs/tui/src/chatwidget/pickers.rs +++ b/nori-rs/tui/src/chatwidget/pickers.rs @@ -88,6 +88,43 @@ impl ChatWidget { } pub(crate) fn open_resume_session_picker(&mut self) { + // When the live agent advertises ACP `session/list`, source the picker + // from the agent itself (and resume over ACP) instead of the local + // transcript store. Resuming a listed session loads it over ACP via + // `session/load`, so the agent must also advertise `load_session`; + // without it, fall back to the local-transcript picker. + if self.session_agent_capabilities.session_list + && self.session_agent_capabilities.load_session + && let Some(handle) = self.acp_handle.clone() + { + let cwd = self.config.cwd.clone(); + let tx = self.app_event_tx.clone(); + tokio::spawn(async move { + match handle.list_sessions(cwd).await { + Ok(sessions) if sessions.is_empty() => { + tx.send(crate::app_event::AppEvent::InsertHistoryCell(Box::new( + crate::history_cell::new_error_event( + "The agent reported no resumable sessions.".to_string(), + ), + ))); + } + Ok(sessions) => { + tx.send(crate::app_event::AppEvent::ShowAcpResumeSessionPicker { + sessions, + }); + } + Err(e) => { + tx.send(crate::app_event::AppEvent::InsertHistoryCell(Box::new( + crate::history_cell::new_error_event(format!( + "Failed to list sessions from the agent: {e}" + )), + ))); + } + } + }); + return; + } + let started = std::time::Instant::now(); let cwd = self.config.cwd.clone(); let tx = self.app_event_tx.clone(); @@ -199,6 +236,15 @@ impl ChatWidget { self.bottom_pane.show_selection_view(params); } + /// Show the resume picker populated from the agent's ACP `session/list`. + pub(crate) fn show_acp_resume_session_picker( + &mut self, + sessions: Vec, + ) { + let params = crate::nori::resume_session_picker::acp_resume_session_picker_params(sessions); + self.bottom_pane.show_selection_view(params); + } + pub(crate) fn update_resume_session_picker_item( &mut self, generation: u64, diff --git a/nori-rs/tui/src/chatwidget/tests/part2.rs b/nori-rs/tui/src/chatwidget/tests/part2.rs index 938e44f92..0ea87abe8 100644 --- a/nori-rs/tui/src/chatwidget/tests/part2.rs +++ b/nori-rs/tui/src/chatwidget/tests/part2.rs @@ -92,6 +92,7 @@ fn goal_capabilities(goal_enabled: bool) -> nori_protocol::SessionCapabilitiesVi agent: nori_protocol::AgentCapabilitiesView { http_mcp: goal_enabled, load_session: true, + session_list: false, }, nori_client: nori_protocol::NoriClientCapabilitiesView { advertised: goal_enabled, @@ -648,6 +649,30 @@ fn approvals_selection_popup_snapshot() { assert_snapshot!("approvals_selection_popup", popup); } +#[test] +fn acp_resume_session_picker_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); + + let sessions = vec![ + nori_acp::AcpSessionSummary { + session_id: "session-abc".to_string(), + cwd: PathBuf::from("/home/user/project"), + title: Some("Refactor the parser".to_string()), + updated_at: Some("2020-01-15T10:30:00Z".to_string()), + }, + nori_acp::AcpSessionSummary { + session_id: "session-def".to_string(), + cwd: PathBuf::from("/home/user/other"), + title: None, + updated_at: None, + }, + ]; + chat.show_acp_resume_session_picker(sessions); + + let popup = render_bottom_popup(&chat, 80); + assert_snapshot!("acp_resume_session_picker", popup); +} + #[test] fn approval_preset_actions_emit_a_single_atomic_app_event() { let (_chat, app_event_tx, mut rx, _op_rx) = make_chatwidget_manual_with_sender(); diff --git a/nori-rs/tui/src/chatwidget/tests/snapshots/nori_tui__chatwidget__tests__part2__acp_resume_session_picker.snap b/nori-rs/tui/src/chatwidget/tests/snapshots/nori_tui__chatwidget__tests__part2__acp_resume_session_picker.snap new file mode 100644 index 000000000..8475633fd --- /dev/null +++ b/nori-rs/tui/src/chatwidget/tests/snapshots/nori_tui__chatwidget__tests__part2__acp_resume_session_picker.snap @@ -0,0 +1,12 @@ +--- +source: tui/src/chatwidget/tests/part2.rs +expression: popup +--- + Resume previous session + Select a session to resume + + Type to search sessions +› Refactor the parser 2020-01-15 10:30 · /home/user/project + session-def /home/user/other + + ↑/k ↓/j to navigate, enter to confirm, esc to go back diff --git a/nori-rs/tui/src/nori/resume_session_picker.rs b/nori-rs/tui/src/nori/resume_session_picker.rs index 56a65265f..7e1b83e1f 100644 --- a/nori-rs/tui/src/nori/resume_session_picker.rs +++ b/nori-rs/tui/src/nori/resume_session_picker.rs @@ -8,6 +8,7 @@ use std::path::Path; use std::path::PathBuf; use std::time::Instant; +use nori_acp::AcpSessionSummary; use nori_acp::transcript::TranscriptLoader; use crate::app_event::AppEvent; @@ -89,6 +90,74 @@ pub fn resume_session_picker_params( } } +/// Build the resume picker from sessions reported by the live agent's ACP +/// `session/list`. +/// +/// Unlike [`resume_session_picker_params`], rows come from the agent rather +/// than the local transcript store, and selecting one resumes via +/// `session/load` using the agent's own session id (no local transcript). +/// Columns map as: `title` → row name (falling back to the session id when the +/// agent omits a title), `updated_at` → relative time, and `cwd` → the row +/// description. +pub fn acp_resume_session_picker_params(sessions: Vec) -> SelectionViewParams { + if sessions.is_empty() { + return SelectionViewParams { + title: Some("Resume previous session".to_string()), + subtitle: Some("The agent reported no resumable sessions".to_string()), + footer_hint: Some(standard_popup_hint_line()), + items: vec![], + ..Default::default() + }; + } + + let items: Vec = sessions + .into_iter() + .map(|session| { + let cwd_display = session.cwd.display().to_string(); + let name = session + .title + .clone() + .filter(|title| !title.is_empty()) + .unwrap_or_else(|| session.session_id.clone()); + let description = match session.updated_at.as_deref() { + Some(updated_at) => Some(format!( + "{} · {cwd_display}", + format_relative_time(updated_at) + )), + None => Some(cwd_display.clone()), + }; + let search_value = format!("{} {name} {cwd_display}", session.session_id); + + let acp_session_id = session.session_id; + let actions: Vec = vec![Box::new(move |tx: &AppEventSender| { + tx.send(AppEvent::ResumeAcpSession { + acp_session_id: acp_session_id.clone(), + }); + })]; + + SelectionItem { + name, + description, + search_value: Some(search_value), + is_current: false, + actions, + dismiss_on_select: true, + ..Default::default() + } + }) + .collect(); + + SelectionViewParams { + title: Some("Resume previous session".to_string()), + subtitle: Some("Select a session to resume".to_string()), + footer_hint: Some(standard_popup_hint_line()), + items, + is_searchable: true, + search_placeholder: Some("Type to search sessions".to_string()), + ..Default::default() + } +} + pub(crate) fn resume_session_item_update( session_id: &str, started_at: &str, @@ -265,6 +334,52 @@ mod tests { assert!(params.is_searchable); } + #[test] + fn acp_resume_picker_maps_agent_session_summaries() { + let sessions = vec![ + AcpSessionSummary { + session_id: "agent-sess-1".to_string(), + cwd: PathBuf::from("/repo/one"), + title: Some("Fix the parser".to_string()), + updated_at: Some("2020-01-15T10:30:00Z".to_string()), + }, + AcpSessionSummary { + session_id: "agent-sess-2".to_string(), + cwd: PathBuf::from("/repo/two"), + title: None, + updated_at: None, + }, + ]; + + let params = acp_resume_session_picker_params(sessions); + + assert_eq!(params.items.len(), 2); + // Title becomes the row name; missing title falls back to session id. + assert_eq!(params.items[0].name, "Fix the parser"); + assert_eq!(params.items[1].name, "agent-sess-2"); + // cwd shows up in the description for both rows. + assert!( + params.items[0] + .description + .as_deref() + .unwrap() + .contains("/repo/one") + ); + assert_eq!(params.items[1].description.as_deref(), Some("/repo/two")); + assert!(params.is_searchable); + } + + #[test] + fn acp_resume_picker_handles_empty_session_list() { + let params = acp_resume_session_picker_params(vec![]); + + assert!(params.items.is_empty()); + assert_eq!( + params.subtitle.as_deref(), + Some("The agent reported no resumable sessions") + ); + } + #[test] fn resume_picker_omits_turn_count_until_known() { let (tx, _rx) = tokio::sync::mpsc::unbounded_channel();