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: 2 additions & 2 deletions nori-rs/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions nori-rs/acp-host/docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ nori-acp-host <---> ACP Agent subprocess (JSON-RPC over stdio)
- `nori-harness` (`@/nori-rs/harness/`) is the primary consumer and re-exports every module (`pub use nori_acp_host::connection;` and friends in `@/nori-rs/harness/src/lib.rs`), so downstream consumers such as `@/nori-rs/tui/` import through `nori_harness` paths.
- Wire/schema types come from the official `agent-client-protocol` SDK; the schema's own `unstable` feature is enabled unconditionally for the Model-category config option.
- Depends on `codex-protocol` (`@/nori-rs/protocol/`) for the internal event vocabulary, `nori-config` (`@/nori-rs/nori-config/`) for agent/MCP/wire-proxy configuration types, and `codex-rmcp-client` (`@/nori-rs/rmcp-client/`) for OAuth token loading in `connection/mcp.rs`.
- Error classification lives here (`AcpErrorCategory`, `categorize_acp_error`); the harness-side user-facing message composition (`enhanced_error_message`) stays in `@/nori-rs/harness/src/backend/`.
- Error classification lives here (`AcpErrorCategory`, `AcpErrorDetails`, `categorize_acp_error`, `categorize_acp_error_chain`); the harness-side user-facing message composition (`enhanced_error_message`) stays in `@/nori-rs/harness/src/backend/`.

### Core Implementation

- `connection/` β€” `AcpConnection::spawn()` launches the agent as a child subprocess, owns its full lifecycle (exit watching, stderr tail, graceful stdin-EOF shutdown), forwards configured MCP servers (`mcp.rs`), optionally wraps the transport in an append-only wire logger (`wire_log.rs`), and delivers everything through one ordered `ConnectionEvent` inbox. See `@/nori-rs/acp-host/src/connection/docs.md`.
- `registry.rs` β€” data-driven agent registry merging built-in agents (Claude Code, Codex, Gemini) with custom `[[agents]]` config entries; resolves an agent slug to a spawnable `AcpAgentConfig` across npx/bunx/pipx/uvx/local distributions. The registry is process-global state (`AGENT_REGISTRY`, a `RwLock`) initialized once via `initialize_registry()` at startup, with a built-in-defaults fallback when uninitialized.
- `translator.rs` β€” converts user input into ACP `ContentBlock`s (text plus base64 image blocks) and provides local parsing/display helpers.
- `patch.rs` β€” diff/patch construction (`create_patch_with_context`) used to normalize file mutations for rendering and transcripts.
- `error_category.rs` β€” priority-chained substring matching (Auth > Quota > ExecutableNotFound > Initialization > PromptTooLong > ApiServerError > Unknown) over the Debug-formatted error chain; `is_retryable()` marks only server errors and quota limits as transient.
- `error_category.rs` β€” two-tier categorization. `categorize_acp_error_chain()` is the preferred entry point: it walks the anyhow chain for a structured `acp::Error`, maps it by JSON-RPC code (`-32000` auth, `-32002` session not found, `-32010` agent backing service unreachable, `-32012` not resumable, `-32014` no active session, `-32015` session already active β€” the code table mirrors the `nori-handroll acp` agent side), and extracts the agent-supplied `error.data.detail` into `AcpErrorDetails`. Structured codes always win over message text. Without a structured error it falls back to the legacy `categorize_acp_error()` priority-chained substring matching (Auth > Quota > ExecutableNotFound > Initialization > PromptTooLong > ApiServerError > Unknown) over the Debug-formatted error chain. `is_retryable()` marks server errors, quota limits, and `AgentUnreachable` as transient; the session-lifecycle states are persistent and never retryable.

### Things to Know

Expand Down
41 changes: 41 additions & 0 deletions nori-rs/acp-host/src/connection/acp_connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,47 @@ impl AcpConnection {
Ok(acp::SessionId::from(session_id.to_string()))
}

/// Resume (reattach to) an existing session via ACP `session/resume`.
///
/// Unlike `session/load`, resume is a live reattach with no history
/// replay β€” the path for agents that advertise
/// `sessionCapabilities.resume` with `loadSession: false` (nori cloud).
/// The returned `SessionId` is the input id (the response carries none).
pub async fn resume_session(
&self,
session_id: &str,
cwd: &Path,
mcp_servers: Vec<acp::McpServer>,
) -> Result<acp::SessionId> {
let response = self
.cx
.send_request(
acp::ResumeSessionRequest::new(session_id.to_string(), cwd)
.mcp_servers(mcp_servers),
)
.block_task()
.await
.context("Failed to resume ACP session")?;

if let Some(config_options) = response.config_options
&& let Ok(mut state) = self.session_config_state.write()
{
state.config_options = config_options;
}

Ok(acp::SessionId::from(session_id.to_string()))
}

/// Close (release) a session via ACP `session/close`.
pub async fn close_session(&self, session_id: &str) -> Result<()> {
self.cx
.send_request(acp::CloseSessionRequest::new(session_id.to_string()))
.block_task()
.await
.context("Failed to close ACP session")?;
Ok(())
}

/// List the agent's known sessions via ACP `session/list`.
///
/// `cwd` is forwarded as the spec's working-directory filter. Cursor-based
Expand Down
145 changes: 145 additions & 0 deletions nori-rs/acp-host/src/connection/acp_connection_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1212,3 +1212,148 @@ async fn test_list_sessions_maps_agent_session_info() {
];
assert_eq!(summaries, expected);
}

/// `resume_session` reattaches to an agent-side session over ACP
/// `session/resume` β€” the reattach path for agents (like nori cloud) that
/// advertise `sessionCapabilities.resume` with `loadSession: false`.
/// `MOCK_AGENT_FAIL_NEW_SESSION_FROM=0` makes any silent `session/new`
/// fallback fail loudly.
#[tokio::test]
async fn test_resume_session_reattaches_over_session_resume() {
let Some(mut config) = mock_agent_config() else {
return;
};
config.env.insert(
"MOCK_AGENT_SUPPORT_SESSION_RESUME".to_string(),
"1".to_string(),
);
config.env.insert(
"MOCK_AGENT_FAIL_NEW_SESSION_FROM".to_string(),
"0".to_string(),
);

let temp_dir = tempdir().expect("temp dir");
let conn = AcpConnection::spawn(
&config,
temp_dir.path(),
nori_config::AcpProxyConfig::disabled(),
)
.await
.expect("Failed to spawn AcpConnection");

assert!(
conn.capabilities().session_capabilities.resume.is_some(),
"mock should advertise session/resume when env-gated on"
);

let session_id = conn
.resume_session("mock-session-1", temp_dir.path(), Vec::new())
.await
.expect("resume_session should succeed against a live agent");
assert_eq!(session_id.to_string(), "mock-session-1");
}

/// A structured `session/resume` failure must stay categorizable: the agent's
/// error code (-32002 resource_not_found) and `data.detail` survive the real
/// stdio transport and the anyhow chain so callers can show a precise message.
#[tokio::test]
async fn test_resume_session_failure_categorizes_as_session_not_found() {
let Some(mut config) = mock_agent_config() else {
return;
};
config.env.insert(
"MOCK_AGENT_SUPPORT_SESSION_RESUME".to_string(),
"1".to_string(),
);
config.env.insert(
"MOCK_AGENT_RESUME_SESSION_FAIL".to_string(),
"1".to_string(),
);

let temp_dir = tempdir().expect("temp dir");
let conn = AcpConnection::spawn(
&config,
temp_dir.path(),
nori_config::AcpProxyConfig::disabled(),
)
.await
.expect("Failed to spawn AcpConnection");

let err = conn
.resume_session("gone-session", temp_dir.path(), Vec::new())
.await
.expect_err("resume_session should fail when the agent rejects the id");
let details = crate::categorize_acp_error_chain(&err);
assert_eq!(
(details.category, details.detail.as_deref()),
(
crate::AcpErrorCategory::SessionNotFound,
Some("the session is no longer claimed")
)
);
}

/// `close_session` releases an agent-side session over ACP `session/close`.
#[tokio::test]
async fn test_close_session_round_trips() {
let Some(mut config) = mock_agent_config() else {
return;
};
config.env.insert(
"MOCK_AGENT_SUPPORT_SESSION_CLOSE".to_string(),
"1".to_string(),
);

let temp_dir = tempdir().expect("temp dir");
let conn = AcpConnection::spawn(
&config,
temp_dir.path(),
nori_config::AcpProxyConfig::disabled(),
)
.await
.expect("Failed to spawn AcpConnection");

assert!(
conn.capabilities().session_capabilities.close.is_some(),
"mock should advertise session/close when env-gated on"
);

conn.close_session("mock-session-1")
.await
.expect("close_session should succeed against a live agent");
}

/// A structured `session/close` failure propagates β€” proving the request
/// actually crosses the process boundary (a no-op `close_session` could not
/// observe the agent-side error).
#[tokio::test]
async fn test_close_session_failure_categorizes_as_session_not_found() {
let Some(mut config) = mock_agent_config() else {
return;
};
config.env.insert(
"MOCK_AGENT_SUPPORT_SESSION_CLOSE".to_string(),
"1".to_string(),
);
config
.env
.insert("MOCK_AGENT_CLOSE_SESSION_FAIL".to_string(), "1".to_string());

let temp_dir = tempdir().expect("temp dir");
let conn = AcpConnection::spawn(
&config,
temp_dir.path(),
nori_config::AcpProxyConfig::disabled(),
)
.await
.expect("Failed to spawn AcpConnection");

let err = conn
.close_session("gone-session")
.await
.expect_err("close_session should fail when the agent rejects the id");
assert_eq!(
crate::categorize_acp_error_chain(&err).category,
crate::AcpErrorCategory::SessionNotFound
);
}
3 changes: 2 additions & 1 deletion nori-rs/acp-host/src/connection/docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,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()`, `list_sessions()`, `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()`, `resume_session()`, `close_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 @@ -50,6 +50,7 @@ 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
- `resume_session()` sends ACP `session/resume` β€” a live reattach with **no** history replay, unlike `session/load`. It is the reattach path for agents that advertise `sessionCapabilities.resume` with `loadSession: false` (the nori cloud contract). The response carries no session id (the input id is returned) but may carry `config_options`, which replace the live session-config snapshot. `close_session()` sends ACP `session/close` to release the agent-side session. Failures on both preserve the structured `acp::Error` (JSON-RPC code plus `data.detail`) in the anyhow chain so `categorize_acp_error_chain()` in `@/nori-rs/acp-host/src/error_category.rs` can classify them precisely
- `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_harness`), 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/harness/docs.md`)

Created and maintained by Nori.
Loading
Loading