Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion nori-rs/acp/docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:<port>/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:<port>/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.

Expand Down Expand Up @@ -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)
|
Expand Down
11 changes: 11 additions & 0 deletions nori-rs/acp/src/backend/nori_client_mcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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,
))
Expand Down Expand Up @@ -541,6 +551,7 @@ mod tests {
nori_protocol::AgentCapabilitiesView {
http_mcp: true,
load_session: false,
session_list: false,
}
}

Expand Down
47 changes: 47 additions & 0 deletions nori-rs/acp/src/connection/acp_connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Vec<crate::connection::AcpSessionSummary>> {
// 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<String> = 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.
Expand Down
57 changes: 57 additions & 0 deletions nori-rs/acp/src/connection/acp_connection_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
3 changes: 2 additions & 1 deletion nori-rs/acp/src/connection/docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
16 changes: 16 additions & 0 deletions nori-rs/acp/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,22 @@ pub struct ApprovalRequest {
pub response_tx: oneshot::Sender<ReviewDecision>,
}

/// 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<String>,
/// ISO 8601 timestamp of last activity, when the agent provides one.
pub updated_at: Option<String>,
}

/// Session config state captured from ACP session setup and updates.
///
/// This stores the complete current `configOptions` snapshot for the active
Expand Down
1 change: 1 addition & 0 deletions nori-rs/acp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion nori-rs/mock-acp-agent/docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<Client>`; the closures share state through an `Arc<MockState>`. 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<Client>`; the closures share state through an `Arc<MockState>`. 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
Expand All @@ -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
Expand Down
Loading
Loading