From 4befa436a49ef275260615529ab8e3c44f69f254 Mon Sep 17 00:00:00 2001 From: Nori Date: Sun, 15 Nov 2026 17:18:09 +0000 Subject: [PATCH 1/6] feat(delegation): coordinate child sessions and automatic results --- docs/remote-control.md | 63 +- nori-rs/Cargo.lock | 1 + nori-rs/acp-host/tests/managed_host.rs | 2 + nori-rs/cli/src/host/docs.md | 10 +- nori-rs/cli/src/host/table.rs | 40 + nori-rs/harness/docs.md | 82 +- nori-rs/harness/src/backend/mod.rs | 5 + .../harness/src/backend/nori_client_mcp.rs | 48 +- nori-rs/harness/src/backend/session.rs | 2 + nori-rs/harness/src/backend/session_swap.rs | 1 + .../harness/src/backend/spawn_and_relay.rs | 2 + nori-rs/harness/src/managed_remote.rs | 53 + nori-rs/harness/src/remote_agent.rs | 3 +- nori-rs/harness/src/runtime.rs | 2 + .../harness/src/session_manager/delegation.rs | 355 +++++++ .../src/session_manager/delegation_driver.rs | 184 ++++ .../src/session_manager/delegation_state.rs | 255 +++++ nori-rs/harness/src/session_manager/mod.rs | 118 ++- .../harness/src/session_manager/registry.rs | 39 +- nori-rs/harness/tests/delegation.rs | 994 ++++++++++++++++++ nori-rs/harness/tests/managed_remote.rs | 113 ++ nori-rs/mock-acp-agent/Cargo.toml | 2 + nori-rs/mock-acp-agent/docs.md | 11 + nori-rs/mock-acp-agent/src/delegation.rs | 58 + nori-rs/mock-acp-agent/src/main.rs | 32 + nori-rs/nori-protocol/src/session_event.rs | 26 + nori-rs/tui-pty-e2e/tests/delegation.rs | 177 ++++ nori-rs/tui-pty-e2e/tests/host_attach.rs | 80 ++ nori-rs/tui-pty-e2e/tests/session_list.rs | 12 + ..._list__session_list_fork_under_parent.snap | 2 +- nori-rs/tui/docs.md | 47 +- nori-rs/tui/src/app/delegation.rs | 77 ++ nori-rs/tui/src/app/event_handling.rs | 28 + nori-rs/tui/src/app/mod.rs | 3 + nori-rs/tui/src/app/session_overview.rs | 25 + nori-rs/tui/src/app/session_setup.rs | 5 +- nori-rs/tui/src/app_event.rs | 6 + nori-rs/tui/src/attach/docs.md | 17 + nori-rs/tui/src/attach/input.rs | 80 ++ nori-rs/tui/src/attach/mod.rs | 7 +- nori-rs/tui/src/attach/render.rs | 41 +- nori-rs/tui/src/attach/runtime.rs | 24 + ...h__tests__delegation_approval_watcher.snap | 21 + ..._attach__tests__delegation_child_list.snap | 21 + ...tui__attach__tests__delegation_result.snap | 21 + nori-rs/tui/src/attach/state.rs | 42 + nori-rs/tui/src/attach/tests.rs | 211 ++++ .../tui/src/bottom_pane/bottom_pane_view.rs | 5 + .../src/bottom_pane/list_selection_view.rs | 6 + nori-rs/tui/src/bottom_pane/mod.rs | 6 + nori-rs/tui/src/chatwidget/agent.rs | 104 ++ nori-rs/tui/src/chatwidget/constructors.rs | 33 + nori-rs/tui/src/chatwidget/delegation.rs | 135 +++ nori-rs/tui/src/chatwidget/event_handlers.rs | 10 +- nori-rs/tui/src/chatwidget/key_handling.rs | 3 + nori-rs/tui/src/chatwidget/mod.rs | 4 + .../tui/src/chatwidget/tests/delegation.rs | 143 +++ nori-rs/tui/src/chatwidget/tests/mod.rs | 1 + ...delegation__local_delegation_approval.snap | 13 + nori-rs/tui/src/chatwidget/user_input.rs | 19 + nori-rs/tui/src/delegation.rs | 39 + nori-rs/tui/src/lib.rs | 1 + nori-rs/tui/src/slash_command.rs | 8 +- 63 files changed, 3944 insertions(+), 34 deletions(-) create mode 100644 nori-rs/harness/src/session_manager/delegation.rs create mode 100644 nori-rs/harness/src/session_manager/delegation_driver.rs create mode 100644 nori-rs/harness/src/session_manager/delegation_state.rs create mode 100644 nori-rs/harness/tests/delegation.rs create mode 100644 nori-rs/mock-acp-agent/src/delegation.rs create mode 100644 nori-rs/tui-pty-e2e/tests/delegation.rs create mode 100644 nori-rs/tui/src/app/delegation.rs create mode 100644 nori-rs/tui/src/attach/snapshots/nori_tui__attach__tests__delegation_approval_watcher.snap create mode 100644 nori-rs/tui/src/attach/snapshots/nori_tui__attach__tests__delegation_child_list.snap create mode 100644 nori-rs/tui/src/attach/snapshots/nori_tui__attach__tests__delegation_result.snap create mode 100644 nori-rs/tui/src/chatwidget/delegation.rs create mode 100644 nori-rs/tui/src/chatwidget/tests/delegation.rs create mode 100644 nori-rs/tui/src/chatwidget/tests/snapshots/nori_tui__chatwidget__tests__delegation__local_delegation_approval.snap create mode 100644 nori-rs/tui/src/delegation.rs diff --git a/docs/remote-control.md b/docs/remote-control.md index 6a792ecdb..8e5388d93 100644 --- a/docs/remote-control.md +++ b/docs/remote-control.md @@ -333,9 +333,11 @@ prompts; the WebSocket extension controls sessions already owned by the TUI. | `_nori/session/subscribe` | Optional `session_id`, optional `cursors` map | `{ "subscribed": true }`, then envelope notifications | | `_nori/session/prompt` | Optional `session_id`, ACP `prompt` content blocks, optional `meta` | `session_id`, admitted harness `request_id` | | `_nori/session/respond` | Optional `session_id`, permission `request_id`, ACP permission `response` | `session_id`, `answered: true` | +| `_nori/session/delegate` | Optional parent `session_id`, `agent`, `task` | Delegation record | +| `_nori/session/delegation/resolve` | Optional parent `session_id`, delegation `request_id`, `approved` boolean | Updated delegation record | A list row reports identity, agent, status, cwd, lineage, pending request IDs, -sequence bounds (`latest_seq`, `first_available_seq`), age in seconds, and a +`source`, delegation `depth`, the parent's `delegations`, sequence bounds (`latest_seq`, `first_available_seq`), age in seconds, and a title derived from observed session activity. `replay_cursors` includes every conversation known to the journal. Restarting a failed session with Ctrl+R keeps its Nori conversation ID, list row, and sequence counter. Existing subscriptions @@ -370,6 +372,59 @@ method rather than a generic deserialize/serialize round trip, which can choose an empty response variant and discard fields. The transport serializes the original manager event without that round trip. +### Delegation + +Delegation is a Nori-owned manager operation behind `multi_session`, shared by +remote controllers, the local `/delegate ` command, and the +session-bound `delegate` tool on Nori's authenticated HTTP `nori-client` MCP +server. This puts approval, session capacity, and child lifetime under the +same authority even when no terminal is attached. The MCP tool accepts +`agent` and `task`; its parent identity is bound by Nori to the calling session. +Agents without HTTP MCP support retain user-driven `/delegate`, but have no +stdio agent-tool fallback. + +A delegation record contains `request_id`, `parent_id`, `child_id`, +`parent_agent`, `agent`, `task`, `depth`, `status`, and optional `result`. +The child conversation ID is reserved before approval. The statuses are +`awaiting_approval`, `running`, `declined`, `completed`, `failed`, and +`cancelled`; these describe delegation state, separately from session status. +`DelegationChanged` Nori events carry the record through ordered manager +envelopes. Lists expose current records for recovering approval presentation. +All-session subscribers also receive the child's normal envelopes; a +parent-only subscription does not become a subscription to every descendant. + +Same-kind children start immediately. A different-kind request returns +`awaiting_approval`; resolve it for the requesting parent and request ID with +`approved: true` (Start) or `approved: false` (Decline). Only the controller may +request or resolve delegation. Unknown, resolved, or wrongly attributed +requests fail rather than starting another child. Approval does not reserve a +process indefinitely: actual startup must pass the shared session cap. Depth +is measured in delegation edges, separately from forks. Children may delegate +one level further; a request deeper than two levels below the top session +returns an error to its requester. Branch-at-head transfers parent or child +ownership to the process's new conversation without increasing delegation +depth. The same delegation request ID remains valid, while `DelegationChanged` +publishes updated attribution; pending approvals and result delivery follow +the new parent identity. + +A child's ACP permission request belongs to that child's conversation and +request ID. Answer it after selecting the child, with Alt+Down in the terminal +for next waiting session. Parent notices name the child and parent rather than +opening the child's permission popup in the parent. A completed child's result +is automatically queued as attributed input to the parent, including when the +parent is busy. The manager retains delivery until that parent's correlated +ACP prompt response, rather than treating queue admission as acknowledgment. +If the parent process crashes before acknowledgment, stable-ID resume submits +the result again. A delegated child with descendants waits for them and its +result-driven continuation before reporting its final result. The child process +then ends and frees its cap slot; its record and transcript remain inspectable. + +Explicit parent close cancels descendants and outstanding approvals, even if +the parent's process has already failed. Parent crash preserves running +children and holds results until stable-ID resume (Ctrl+R). Child failure +returns a failure result without failing the parent. This state belongs to the +current manager lifetime; host restart does not restore it from transcripts. + ### Reconnect and retention The manager journals at most 4096 envelopes **total across sessions** in memory. @@ -518,6 +573,12 @@ highlighted failed session with the same conversation ID. The status line shows controlling, watching/read-only, reconnecting, or disconnected state. The current session marker appears inside TITLE, leaving its short ID visible. Session commands address the host; they do not launch local agents. +`/delegate ` is the delegation entry point; the list does not +start delegations. The configured `hotkeys.next_waiting_session` binding +(Alt+Down by default) selects the next waiting child or other session. +Delegated child rows prefix TITLE with the parent identity and hierarchy marker, +so long task titles cannot displace that identity; metadata columns remain fixed +in the attached list and `nori host ls`. Socket loss or a receive-liveness deadline starts recovery. Periodic session list requests keep idle connections observable; receiving JSON renews the diff --git a/nori-rs/Cargo.lock b/nori-rs/Cargo.lock index b30cf1bfb..2e56a6300 100644 --- a/nori-rs/Cargo.lock +++ b/nori-rs/Cargo.lock @@ -3617,6 +3617,7 @@ dependencies = [ "env_logger", "futures", "nori-protocol", + "rmcp", "serde_json", "tokio", "tokio-util", diff --git a/nori-rs/acp-host/tests/managed_host.rs b/nori-rs/acp-host/tests/managed_host.rs index e2b1ac333..a511a6e4d 100644 --- a/nori-rs/acp-host/tests/managed_host.rs +++ b/nori-rs/acp-host/tests/managed_host.rs @@ -130,6 +130,8 @@ async fn watcher_cannot_mutate_and_takeover_notifies_both_without_disconnecting( "_nori/session/close", "_nori/session/cancel", "_nori/session/resume", + "_nori/session/delegate", + "_nori/session/delegation/resolve", ] { send(&mut second, 2, method, json!({})).await; let response = receive(&mut second).await; diff --git a/nori-rs/cli/src/host/docs.md b/nori-rs/cli/src/host/docs.md index 1887bee45..6ed38d7f5 100644 --- a/nori-rs/cli/src/host/docs.md +++ b/nori-rs/cli/src/host/docs.md @@ -50,7 +50,10 @@ host ls / host stop ---------------+ SessionManager control characters and colors status only on a terminal. Terminal rows fit the available display cells by truncating titles with an ellipsis at grapheme boundaries; if metadata alone exceeds the width, that prefix is clipped. - Piped output retains full titles without color or a width limit. + Piped output retains full titles without color or a width limit. Delegated + child rows retain their own ID, agent, status, and age columns; the hierarchy + marker and parent identity prefix TITLE, so long task titles do not displace + parent attribution and indentation never shifts the metadata columns. - Lifecycle requests prepare agents using the host's configuration. Resume uses the stored agent and working directory while retaining the conversation identity. Stop closes the server, shuts down the manager, removes discovery @@ -66,6 +69,11 @@ host ls / host stop ---------------+ SessionManager - The authenticated listener exposes only `/sessions`; it does not change the legacy interactive remote-control listener. See the [wire contract](../../../../docs/remote-control.md#11-background-host-and-attached-terminals). +- Delegation requests pass through `ManagedRemoteHost` to the same manager + used by local sessions. Child sessions consume the ordinary session cap; + completing a child releases its process slot but preserves its listed record + and transcript. Closing a parent cancels descendants, while a parent crash + preserves children and holds their results for stable-ID resume. - The host has no persistent manager snapshot. Transcripts survive normally, but in-memory registry state and reconnect replay belong to the current host process. The token rotates when that process starts again. diff --git a/nori-rs/cli/src/host/table.rs b/nori-rs/cli/src/host/table.rs index e8391ce93..d3da6427f 100644 --- a/nori-rs/cli/src/host/table.rs +++ b/nori-rs/cli/src/host/table.rs @@ -27,6 +27,18 @@ pub(super) fn format_table(listing: &Value, width: Option, color: bool) - let agent = clean("agent"); let status = clean("status").replace('_', " "); let title = clean("title"); + let title = match row["parent"].as_str() { + Some(_) if row["source"] == "session" => { + let parent = clean("parent"); + let parent = if width.is_some() { + parent.chars().take(8).collect::() + } else { + parent + }; + format!("↳ (parent {parent}) {title}") + } + _ => title, + }; let seconds = row["age_seconds"].as_i64().unwrap_or(0).max(0); let age = if seconds < 60 { format!("{seconds}s") @@ -123,4 +135,32 @@ mod tests { assert!(rendered.lines().nth(1).unwrap().ends_with(title)); assert!(!rendered.contains('\u{1b}')); } + + #[test] + fn delegated_child_identity_stays_inside_title_column() { + let parent = "11111111-1111-1111-1111-111111111111"; + let child = "22222222-2222-2222-2222-222222222222"; + let listing = json!({"sessions":[ + {"session_id":parent,"agent":"mock-model","status":"ready","title":"Parent"}, + {"session_id":child,"agent":"mock-model-alt","status":"waiting_on_you","title":"Child task","parent":parent,"source":"session","depth":1} + ]}); + let piped = format_table(&listing, None, false); + let rows = piped.lines().collect::>(); + assert_eq!(&rows[2][..36], child); + assert_eq!(&rows[2][38..54], "mock-model-alt "); + assert_eq!(&rows[2][56..72], "waiting on you "); + let title = &rows[2][82..]; + assert!(title.contains("Child task")); + assert!( + title.contains(parent), + "child row must identify its parent: {title}" + ); + let tty = format_table(&listing, Some(110), false); + assert!(tty.lines().all(|line| line.width() <= 110)); + assert!( + tty.lines().nth(2).unwrap()[82..].contains("parent 11111111"), + "normal terminal width must retain parent identity" + ); + assert_eq!(&tty.lines().nth(2).unwrap()[..82], &rows[2][..82]); + } } diff --git a/nori-rs/harness/docs.md b/nori-rs/harness/docs.md index af6b26b29..925e6c834 100644 --- a/nori-rs/harness/docs.md +++ b/nori-rs/harness/docs.md @@ -664,7 +664,7 @@ processes. | --- | --- | | `Starting` | Before `SessionStarted` | | `Working` | Phase is `Prompting`, `Loading`, or `Cancelling` | - | `WaitingOnYou` | An ACP permission request is pending | + | `WaitingOnYou` | An ACP permission request or child-start approval is pending | | `Ready` | Idle | | `Failed` | `SessionEnded` with `ConnectionLost`, `SpawnFailed`, or `TimedOut`, or the stream closed with no end event | | `Closed` | `SessionEnded` with `Shutdown` or `Closed`, or superseded by a fork | @@ -712,8 +712,10 @@ processes. nor managed remote list responses expose it. Resume validates failed status, agent identity, and a readable transcript, then reserves capacity before launching the replacement. -- **Close and shutdown.** `close(id, child_grace)` shuts one session down, - bounded by `shutdown_timeout`; closing an ended session does nothing. +- **Close and shutdown.** `close(id, child_grace)` cancels delegated descendants and shuts the + target session down, bounded by `shutdown_timeout`. An ended target has no + process to stop, but its running descendants and pending approvals still + require cancellation. Successful close waits for the manager's event pump to observe process end and release the capacity slot, so a caller can immediately start a replacement. Backend shutdown and that observation share one timeout. The @@ -742,6 +744,80 @@ processes. `SessionManagerConfig` defaults to eight sessions and a five-second shutdown timeout. +#### Delegation between managed sessions + +The [session manager](src/session_manager/mod.rs) owns delegation admission, +approval, parent/child links, result delivery, and descendant lifetime. Both terminal `/delegate` commands and the session-bound `delegate` +tool on the existing authenticated +[`nori-client` HTTP MCP server](src/backend/nori_client_mcp.rs) enter this same +manager path. The tool takes an agent and task; its authority comes from the +calling session, not a caller-supplied parent ID. Delegation requires the +parent's resolved `multi_session` flag. Direct single-session launch behavior +is unchanged. + +```text +parent agent -- HTTP MCP delegate --+ +local /delegate -------------------+--> SessionManager +remote /session/delegate ----------+ | + different agent? -> Start / Decline + | + child ACP process + | + correlated prompt outcome + | + attributed parent prompt queue +``` + +- Delegation records reserve a child conversation identity before approval. + Same-kind children start immediately; different-kind children stay + `awaiting_approval` until Start or Decline. Approval is scoped to the + requesting parent and request ID. Spawn admission uses the manager's shared + session cap, including children, and is checked when a child actually starts. +- Depth counts delegation edges, separately from fork ancestry. A top-level + session may delegate to a child, which may delegate to a grandchild; + deeper requests fail with an error returned to the requester. Branch-at-head + transfers delegation ownership to the conversation now carried by that + process without adding depth. Parent links, pending start approvals, and + result correlation follow the fork; the delegation request identity remains + stable. [State reduction](src/session_manager/delegation_state.rs) republishes + the changed attribution for observers. +- `DelegationChanged` carries request, parent, and child identities, agent + names, task, depth, status, and optional result. Manager envelopes make those + changes observable through ordinary subscriptions and reconnect replay, + including events published while a failed parent has no process. Child-start + approval contributes to the parent's `WaitingOnYou` status separately from + its ACP permission request IDs. + ACP permission requests remain owned by the child conversation; permission + responses must target that child, not its parent. +- A child's final output or failure becomes an attributed prompt queued into + its parent automatically, without a review step. A busy parent consumes it + through its normal prompt queue. The [result driver](src/session_manager/delegation_driver.rs) + retains that delivery until the parent's correlated ACP `PromptResponse`, + not merely queue admission. If the process crashes first, stable-ID resume + resubmits the unacknowledged result; the parent may therefore see it again. + A child that delegates waits for its descendants and resulting continuation + turns before returning its own final result. Terminal completion stops the + child process and releases its capacity slot while retaining the registry + record and transcript. +- Explicit parent close cancels descendants and invalidates pending delegation + approvals, including when the parent has already failed. A parent process + crash leaves children running; results wait for the parent's stable-ID + resume. These held results and delegation state belong to the live manager, + not a persistent host restart snapshot. A child crash produces a failure + result without failing its parent. +- Admission and explicit close share a mutex so a start cannot escape a parent + close. Result work is claimed under manager state, then prompts and process + shutdown run outside that lock. Prompt request IDs, outstanding descendants, + and in-flight result submissions determine completion; an idle phase alone + does not prove that a delegated task has finished. The MCP authority follows + its registered process through a weak manager reference, avoiding an + ownership cycle and rejecting calls from an ended process. + +Agents without HTTP MCP support can still receive children started by the +user's `/delegate`; an agent-side stdio delegation tool is not provided. +The [managed remote contract](../../docs/remote-control.md#delegation) defines +controller authorization and request payloads. + #### Browser sessions and profile tiers `backend/browser_session.rs` (`#[cfg(unix)]`) launches a headed Chrome browser diff --git a/nori-rs/harness/src/backend/mod.rs b/nori-rs/harness/src/backend/mod.rs index 2a10d2727..438e2fae8 100644 --- a/nori-rs/harness/src/backend/mod.rs +++ b/nori-rs/harness/src/backend/mod.rs @@ -147,6 +147,10 @@ pub fn enhance_agent_error( pub struct AcpBackendConfig { /// Agent name used to look up agent in registry pub agent: String, + /// Enable manager-owned delegation for this session. + pub multi_session: bool, + /// Session/process-bound authority for the internal delegation tool. + pub delegation_context: Option, /// Working directory for the session pub cwd: PathBuf, /// Approval policy for command execution @@ -282,6 +286,7 @@ pub struct AcpBackend { goal_mcp_connected: Arc, /// Loopback HTTP server exposing the backend-owned `nori-client` MCP tools. goal_mcp_http_server: Arc>>, + delegation_context: Option, /// Goal extension capability the agent advertised at initialize, parsed /// once. `Some` enables driving goals via `_session/goal`. goal_ext_capability: Option, diff --git a/nori-rs/harness/src/backend/nori_client_mcp.rs b/nori-rs/harness/src/backend/nori_client_mcp.rs index 00132de6c..4523018d9 100644 --- a/nori-rs/harness/src/backend/nori_client_mcp.rs +++ b/nori-rs/harness/src/backend/nori_client_mcp.rs @@ -64,6 +64,16 @@ use super::*; const DUPLICATE_CREATE_GOAL_ERROR: &str = "cannot create a new goal because this thread already has a goal; use update_goal only when the existing goal is complete"; +#[derive(Deserialize, schemars::JsonSchema)] +#[schemars(crate = "rmcp::schemars")] +#[serde(deny_unknown_fields)] +struct DelegateRequest { + /// Agent kind registered with Nori, for example claude-code or codex. + agent: String, + /// Complete task instructions for the child session. + task: String, +} + #[derive(Deserialize, schemars::JsonSchema)] #[schemars(crate = "rmcp::schemars")] #[serde(deny_unknown_fields)] @@ -115,6 +125,7 @@ pub(crate) struct NoriClientShared { agent_capabilities: crate::normalized::AgentCapabilitiesView, nori_client_advertised: bool, goal_ext_advertised: bool, + delegation_context: Option, } impl NoriClientShared { @@ -133,6 +144,7 @@ impl NoriClientShared { agent_capabilities, nori_client_advertised, goal_ext_advertised, + delegation_context: None, } } } @@ -149,9 +161,31 @@ pub(crate) struct NoriClientService { #[tool_router] impl NoriClientService { fn new(shared: NoriClientShared) -> Self { + let mut tool_router = Self::tool_router(); + if shared.delegation_context.is_none() { + tool_router.remove_route("delegate"); + } Self { shared, - tool_router: Self::tool_router(), + tool_router, + } + } + + #[tool( + description = "Delegate a task to a child Nori session. Same-kind children start immediately; a different agent kind waits for user approval. Results return automatically. At most two delegation levels below the top session are allowed." + )] + async fn delegate( + &self, + Parameters(DelegateRequest { agent, task }): Parameters, + ) -> Result { + let Some(context) = &self.shared.delegation_context else { + return Ok(tool_error("delegation requires multi_session")); + }; + match context.delegate(agent, task).await { + Ok(delegation) => Ok(tool_success( + serde_json::json!({ "delegation": delegation }), + )), + Err(error) => Ok(tool_error(error.to_string())), } } @@ -526,20 +560,22 @@ pub(super) async fn register_for_session( thread_goal_state: Arc>, backend_event_tx: mpsc::Sender, connected: Arc, + delegation_context: Option, ) -> Result> { if !connection.capabilities().mcp_capabilities.http { return Ok(None); } - let next_server = NoriClientServer::spawn(NoriClientShared::new( + let mut shared = NoriClientShared::new( thread_goal_state, backend_event_tx, Arc::clone(&connected), agent_capabilities_view(connection), true, goal_ext::GoalExtCapability::from_initialize_meta(connection.initialize_meta()).is_some(), - )) - .await?; + ); + shared.delegation_context = delegation_context; + let next_server = NoriClientServer::spawn(shared).await?; let advertised_server = next_server.as_mcp_server(); let previous_connected = connected.swap(false, Ordering::Relaxed); mcp_servers.push(advertised_server); @@ -819,6 +855,10 @@ mod tests { assert!(tool_names.contains(&"get_goal")); assert!(tool_names.contains(&"create_goal")); assert!(tool_names.contains(&"update_goal")); + assert!( + !tool_names.contains(&"delegate"), + "unmanaged sessions do not advertise delegation" + ); let create = client .call_tool( diff --git a/nori-rs/harness/src/backend/session.rs b/nori-rs/harness/src/backend/session.rs index 2c04ec893..a3ed1176a 100644 --- a/nori-rs/harness/src/backend/session.rs +++ b/nori-rs/harness/src/backend/session.rs @@ -21,6 +21,7 @@ async fn session_mcp_servers( Arc::clone(thread_goal_state), backend_event_tx.clone(), Arc::clone(goal_mcp_connected), + config.delegation_context.clone(), ) .await?; if let Some(server) = nori_client_server { @@ -688,6 +689,7 @@ impl AcpBackend { thread_goal_state, goal_mcp_connected, goal_mcp_http_server, + delegation_context: config.delegation_context.clone(), goal_ext_capability, pending_hook_context: Arc::new(Mutex::new(pending_hook_context)), transcript_recorder: Arc::new(RwLock::new(transcript_recorder)), diff --git a/nori-rs/harness/src/backend/session_swap.rs b/nori-rs/harness/src/backend/session_swap.rs index ce29aa1cf..29b323f2c 100644 --- a/nori-rs/harness/src/backend/session_swap.rs +++ b/nori-rs/harness/src/backend/session_swap.rs @@ -52,6 +52,7 @@ impl AcpBackend { Arc::clone(&self.thread_goal_state), self.backend_event_tx.clone(), Arc::clone(&self.goal_mcp_connected), + self.delegation_context.clone(), ) .await { diff --git a/nori-rs/harness/src/backend/spawn_and_relay.rs b/nori-rs/harness/src/backend/spawn_and_relay.rs index b8049880d..d36948e3f 100644 --- a/nori-rs/harness/src/backend/spawn_and_relay.rs +++ b/nori-rs/harness/src/backend/spawn_and_relay.rs @@ -168,6 +168,7 @@ impl AcpBackend { Arc::clone(&thread_goal_state), backend_event_tx.clone(), Arc::clone(&goal_mcp_connected), + config.delegation_context.clone(), ) .await?; if let Some(server) = nori_client_server { @@ -270,6 +271,7 @@ impl AcpBackend { thread_goal_state, goal_mcp_connected, goal_mcp_http_server, + delegation_context: config.delegation_context.clone(), goal_ext_capability, pending_hook_context: Arc::new(Mutex::new(pending_hook_context)), transcript_recorder: Arc::new(RwLock::new(transcript_recorder)), diff --git a/nori-rs/harness/src/managed_remote.rs b/nori-rs/harness/src/managed_remote.rs index 7f7a6e2a7..63c575fba 100644 --- a/nori-rs/harness/src/managed_remote.rs +++ b/nori-rs/harness/src/managed_remote.rs @@ -46,6 +46,22 @@ struct RespondParams { response: acp::RequestPermissionResponse, } +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct DelegateParams { + session_id: Option, + agent: String, + task: String, +} + +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct ResolveDelegationParams { + session_id: Option, + request_id: String, + approved: bool, +} + struct RemoteError { code: i32, message: String, @@ -110,6 +126,14 @@ impl ManagedRemoteHost { let selected = *self.selected.lock().unwrap_or_else(PoisonError::into_inner); let sessions = self.manager.list().into_iter().map(|record| { let replay = self.manager.replay_info(&record.conversation_id); + let mut depth = 0; + let mut ancestor = Some(record.clone()); + while let Some(current) = ancestor { + if current.source == crate::session_manager::SessionSource::Session { + depth += 1; + } + ancestor = current.parent.and_then(|parent| self.manager.get(&parent)); + } json!({ "session_id":record.conversation_id.to_string(), "agent":record.agent, @@ -117,6 +141,13 @@ impl ManagedRemoteHost { "cwd":record.cwd, "parent":record.parent.map(|id| id.to_string()), "pending_requests":record.pending_requests, + "source": match record.source { + crate::session_manager::SessionSource::User => "user", + crate::session_manager::SessionSource::Fork => "fork", + crate::session_manager::SessionSource::Session => "session", + }, + "depth": depth, + "delegations": self.manager.delegations(&record.conversation_id), "latest_seq":replay.as_ref().map_or(0, |info| info.latest_seq), "first_available_seq":replay.as_ref().map_or(1, |info| info.first_available_seq), "age_seconds":replay.as_ref().map_or(0, |info| info.age_seconds), @@ -166,6 +197,28 @@ impl ManagedRemoteHost { None, )) } + "_nori/session/delegate" => { + let params: DelegateParams = serde_json::from_value(params) + .map_err(|error| RemoteError::new(-32602, error))?; + let parent = self.target(params.session_id.as_deref())?; + let delegation = self + .manager + .delegate(&parent, params.agent, params.task) + .await + .map_err(|error| RemoteError::new(-32000, error))?; + Ok((json!(delegation), None)) + } + "_nori/session/delegation/resolve" => { + let params: ResolveDelegationParams = serde_json::from_value(params) + .map_err(|error| RemoteError::new(-32602, error))?; + let parent = self.target(params.session_id.as_deref())?; + let delegation = self + .manager + .resolve_delegation(&parent, ¶ms.request_id, params.approved) + .await + .map_err(|error| RemoteError::new(-32000, error))?; + Ok((json!(delegation), None)) + } "_nori/session/respond" => { let params: RespondParams = serde_json::from_value(params) .map_err(|error| RemoteError::new(-32602, error))?; diff --git a/nori-rs/harness/src/remote_agent.rs b/nori-rs/harness/src/remote_agent.rs index 636efc340..e5c29045f 100644 --- a/nori-rs/harness/src/remote_agent.rs +++ b/nori-rs/harness/src/remote_agent.rs @@ -475,7 +475,8 @@ async fn run_event_loop(state: Arc>, mut events: mpsc::Receive | NoriEvent::PromptSummaryUpdated(_) | NoriEvent::Notice(_) // Emitted by the session manager, never by a harness session. - | NoriEvent::RequestResolved(_), + | NoriEvent::RequestResolved(_) + | NoriEvent::DelegationChanged(_), ) => {} } } diff --git a/nori-rs/harness/src/runtime.rs b/nori-rs/harness/src/runtime.rs index 7eb784582..f8ba39fe6 100644 --- a/nori-rs/harness/src/runtime.rs +++ b/nori-rs/harness/src/runtime.rs @@ -701,6 +701,8 @@ pub(crate) fn build_backend_config(spec: AgentPrepareSpec) -> AcpBackendConfig { AcpBackendConfig { agent: agent.clone(), + multi_session: config.multi_session, + delegation_context: None, cwd: config.cwd.clone(), approval_policy: config.approval_policy, sandbox_policy: config.sandbox_policy.clone(), diff --git a/nori-rs/harness/src/session_manager/delegation.rs b/nori-rs/harness/src/session_manager/delegation.rs new file mode 100644 index 000000000..9f67e6466 --- /dev/null +++ b/nori-rs/harness/src/session_manager/delegation.rs @@ -0,0 +1,355 @@ +//! Manager-owned delegation admission, process authority and result delivery. +use std::collections::HashSet; +use std::sync::Weak; + +use nori_protocol::Delegation; +use nori_protocol::DelegationStatus; +use nori_protocol::acp; + +use super::delegation_driver::prompt; +pub(super) use super::delegation_driver::wake; +pub(super) use super::delegation_state::DelegationState; +use super::delegation_state::Entry; +use super::*; + +/// Authority for one agent process to delegate through its private MCP server. +/// A weak reference prevents the server and manager from keeping each other alive. +#[derive(Clone)] +pub struct DelegationContext { + shared: Weak, + process: AgentProcessId, +} + +impl std::fmt::Debug for DelegationContext { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("DelegationContext") + .field("process", &self.process) + .finish_non_exhaustive() + } +} + +impl DelegationContext { + pub(super) fn new(shared: &Arc, process: AgentProcessId) -> Self { + Self { + shared: Arc::downgrade(shared), + process, + } + } + + pub(crate) async fn delegate(&self, agent: String, task: String) -> anyhow::Result { + let shared = self + .shared + .upgrade() + .ok_or_else(|| anyhow::anyhow!("session manager has closed"))?; + let parent = { + let state = shared.state(); + let id = state + .registry + .conversation_of(self.process) + .ok_or_else(|| anyhow::anyhow!("requesting agent process has ended"))?; + anyhow::ensure!( + state.registry.live_process(&id) == Some(self.process), + "requesting agent process has ended" + ); + id + }; + SessionManager { shared } + .delegate_from(&parent, agent, task, Some(self.process)) + .await + } +} + +impl SessionManager { + /// Request a child session. A different agent kind waits for user approval. + pub async fn delegate( + &self, + parent: &ConversationId, + agent: String, + task: String, + ) -> anyhow::Result { + self.delegate_from(parent, agent, task, None).await + } + + async fn delegate_from( + &self, + parent: &ConversationId, + agent: String, + task: String, + expected_process: Option, + ) -> anyhow::Result { + let _admission = self.shared.admission.lock().await; + let task = task.trim().to_string(); + anyhow::ensure!(!task.is_empty(), "delegation task must not be empty"); + crate::get_agent_config(&agent)?; + let value = { + let mut state = self.shared.state(); + anyhow::ensure!( + !state.delegation.closed.contains(parent), + "parent session {parent} is closed" + ); + let process = state + .registry + .live_process(parent) + .ok_or_else(|| anyhow::anyhow!("parent session {parent} is not running"))?; + anyhow::ensure!( + expected_process.is_none_or(|expected| expected == process), + "requesting agent process has ended" + ); + let config = state + .configs + .get(&process) + .ok_or_else(|| anyhow::anyhow!("parent session configuration is unavailable"))?; + anyhow::ensure!( + config.multi_session, + "delegation requires [features] multi_session = true" + ); + let parent_agent = state + .registry + .record(parent) + .ok_or(StartError::UnknownParent(*parent))? + .agent; + let mut depth = 1; + let mut ancestor = Some(*parent); + while let Some(id) = ancestor { + let Some(record) = state.registry.record(&id) else { + break; + }; + if record.source == SessionSource::Session { + depth += 1; + } + ancestor = record.parent; + } + anyhow::ensure!( + depth <= 2, + "delegation depth limit is 2 levels below the top session" + ); + anyhow::ensure!( + state.registry.free_slots() > 0, + "{} sessions are already running; close one before delegating", + state.registry.limit() + ); + let status = if parent_agent == agent { + DelegationStatus::Running + } else { + DelegationStatus::AwaitingApproval + }; + let value = Delegation { + request_id: ConversationId::default().to_string(), + parent_id: parent.to_string(), + child_id: ConversationId::default().to_string(), + parent_agent, + agent, + task, + status, + depth, + result: None, + }; + state.delegation.entries.push(Entry { + value: value.clone(), + submitted: false, + submitting: 0, + pending_turns: HashSet::new(), + completed_turns: HashSet::new(), + output: String::new(), + last_result: String::new(), + failure: None, + cancelled: false, + }); + if status == DelegationStatus::AwaitingApproval { + state.publish_delegation(&value); + } + value + }; + if value.status == DelegationStatus::Running { + self.launch_delegation(&value)?; + } + Ok(value) + } + + /// Resolve exactly one pending request belonging to `parent`. + pub async fn resolve_delegation( + &self, + parent: &ConversationId, + request: &str, + approve: bool, + ) -> anyhow::Result { + let _admission = self.shared.admission.lock().await; + let value = { + let mut state = self.shared.state(); + anyhow::ensure!( + !state.delegation.closed.contains(parent), + "parent session {parent} is closed" + ); + anyhow::ensure!( + state.registry.live_process(parent).is_some(), + "parent session {parent} is not running" + ); + let entry = state + .delegation + .entries + .iter_mut() + .find(|e| e.value.request_id == request && e.value.parent_id == parent.to_string()) + .ok_or_else(|| { + anyhow::anyhow!("delegation request does not belong to parent session {parent}") + })?; + anyhow::ensure!( + entry.value.status == DelegationStatus::AwaitingApproval, + "delegation request has already been resolved" + ); + entry.value.status = if approve { + DelegationStatus::Running + } else { + DelegationStatus::Declined + }; + if !approve { + entry.value.result = Some("The user declined this delegation.".into()); + } + let value = entry.value.clone(); + if !approve { + state.publish_delegation(&value); + state.queue_result(&value); + } + value + }; + if approve { + self.launch_delegation(&value)?; + } + wake(&self.shared); + Ok(value) + } + + /// Requests made by this parent, including completed and declined requests. + pub fn delegations(&self, parent: &ConversationId) -> Vec { + self.shared + .state() + .delegation + .entries + .iter() + .filter(|entry| entry.value.parent_id == parent.to_string()) + .map(|entry| entry.value.clone()) + .collect() + } + + fn launch_delegation(&self, value: &Delegation) -> anyhow::Result<()> { + let request_id = value.request_id.clone(); + let launched = self.prepare_delegation(value); + let launched = match launched { + Ok(launched) => launched, + Err(error) => { + let mut state = self.shared.state(); + if let Some(entry) = state + .delegation + .entries + .iter_mut() + .find(|entry| entry.value.request_id == value.request_id) + { + entry.value.status = DelegationStatus::Failed; + entry.value.result = Some(error.to_string()); + let failed = entry.value.clone(); + state.publish_delegation(&failed); + state.queue_result(&failed); + } + drop(state); + wake(&self.shared); + return Err(error); + } + }; + { + let mut state = self.shared.state(); + if let Some(entry) = state + .delegation + .entries + .iter_mut() + .find(|entry| entry.value.request_id == value.request_id) + { + entry.submitting += 1; + } + state.publish_delegation(value); + } + let manager = self.clone(); + let task = value.task.clone(); + tokio::spawn(async move { + let result = launched.handle.prompt(prompt(&task)).await; + manager.record_submission(Some(request_id), result, true); + }); + Ok(()) + } + + fn prepare_delegation(&self, value: &Delegation) -> anyhow::Result { + let parent = ConversationId::from_string(&value.parent_id)?; + let child = ConversationId::from_string(&value.child_id)?; + let config = { + let state = self.shared.state(); + let process = state + .registry + .live_process(&parent) + .ok_or_else(|| anyhow::anyhow!("parent session {parent} is not running"))?; + let mut config = state + .configs + .get(&process) + .cloned() + .ok_or_else(|| anyhow::anyhow!("parent configuration unavailable"))?; + if config.agent != value.agent { + config.default_model = None; + } + config.agent.clone_from(&value.agent); + config.conversation_id = Some(child); + config.initial_context = None; + config.delegation_context = None; + config + }; + let settings = SessionSettings { + conversation_id: child, + agent: value.agent.clone(), + parent: Some(parent), + source: SessionSource::Session, + resumed_from: None, + cwd: config.cwd.clone(), + model: config.default_model.clone(), + approval_policy: config.approval_policy, + }; + self.launch_registered( + SessionAgentSource::Prepare(Box::new(config.clone())), + settings, + config, + SessionStart::New, + ) + .map_err(anyhow::Error::from) + } + + pub(super) fn record_submission( + &self, + request: Option, + result: anyhow::Result, + initial: bool, + ) { + let accepted = result.is_ok(); + { + let mut state = self.shared.state(); + if let Some(entry) = state + .delegation + .entries + .iter_mut() + .find(|e| request.as_ref() == Some(&e.value.request_id)) + { + entry.submitting -= 1; + entry.submitted |= initial; + match result { + Ok(id) => { + if !entry.completed_turns.remove(&id) { + entry.pending_turns.insert(id); + } + } + Err(error) => { + if initial { + entry.failure = Some(error.to_string()); + } + } + } + } + } + if accepted || initial { + wake(&self.shared); + } + } +} diff --git a/nori-rs/harness/src/session_manager/delegation_driver.rs b/nori-rs/harness/src/session_manager/delegation_driver.rs new file mode 100644 index 000000000..711a72796 --- /dev/null +++ b/nori-rs/harness/src/session_manager/delegation_driver.rs @@ -0,0 +1,184 @@ +//! Claim completion and result work atomically, then perform I/O outside the lock. +use super::delegation_state::active; +use super::*; +use nori_protocol::DelegationStatus; +use nori_protocol::acp; + +pub(super) fn prompt(text: &str) -> Vec { + vec![acp::v1::ContentBlock::Text(acp::v1::TextContent::new(text))] +} + +/// Claim work under the state lock, then submit prompts and stop processes outside it. +pub(super) fn wake(shared: &Arc) { + let manager = SessionManager { + shared: Arc::clone(shared), + }; + let (finished, deliveries) = { + let mut state = shared.state(); + let mut finished = Vec::new(); + let mut changed = Vec::new(); + for index in 0..state.delegation.entries.len() { + let entry = &state.delegation.entries[index]; + if entry.value.status != DelegationStatus::Running { + continue; + } + let Ok(child) = ConversationId::from_string(&entry.value.child_id) else { + continue; + }; + let failed = state + .registry + .record(&child) + .is_some_and(|record| record.status == SessionStatus::Failed) + || entry.failure.is_some(); + let pending_child = state + .delegation + .entries + .iter() + .any(|e| e.value.parent_id == entry.value.child_id && active(e.value.status)); + let pending_result = state + .delegation + .deliveries + .iter() + .any(|d| d.parent == child); + if !failed + && (!entry.submitted + || entry.submitting > 0 + || !entry.pending_turns.is_empty() + || pending_child + || pending_result) + { + continue; + } + // Registration precedes submission. A just-created child cannot complete before its task is accepted. + if !entry.submitted && !failed { + continue; + } + let entry = &mut state.delegation.entries[index]; + entry.value.status = if failed { + DelegationStatus::Failed + } else if entry.cancelled { + DelegationStatus::Cancelled + } else { + DelegationStatus::Completed + }; + entry.value.result = Some(if failed { + entry + .failure + .clone() + .unwrap_or_else(|| "Child agent process failed".into()) + } else { + entry.last_result.clone() + }); + changed.push(entry.value.clone()); + finished.push(child); + } + for value in changed { + state.publish_delegation(&value); + state.queue_result(&value); + } + let mut deliveries = Vec::new(); + for index in 0..state.delegation.deliveries.len() { + let delivery = &state.delegation.deliveries[index]; + if let Some(previous) = delivery.process + && state.registry.live_process(&delivery.parent) != Some(previous) + { + let delivery = &mut state.delegation.deliveries[index]; + delivery.in_flight = false; + delivery.submitted = None; + delivery.process = None; + } + let delivery = &state.delegation.deliveries[index]; + if delivery.in_flight || state.delegation.closed.contains(&delivery.parent) { + continue; + } + let Some(process) = state.registry.live_process(&delivery.parent) else { + continue; + }; + let Some(handle) = state.handles.get(&process).cloned() else { + continue; + }; + let parent = delivery.parent; + let owner_request = state + .delegation + .entries + .iter() + .find(|e| e.value.child_id == parent.to_string()) + .map(|e| e.value.request_id.clone()); + let delivery = &mut state.delegation.deliveries[index]; + delivery.in_flight = true; + delivery.process = Some(process); + deliveries.push(( + delivery.request.clone(), + parent, + delivery.text.clone(), + handle, + process, + owner_request, + )); + if let Some(entry) = state + .delegation + .entries + .iter_mut() + .find(|e| e.value.child_id == parent.to_string()) + { + entry.submitting += 1; + } + } + (finished, deliveries) + }; + for child in finished { + let manager = manager.clone(); + tokio::spawn(async move { + if let Err(error) = manager.close_one(&child, Duration::ZERO).await { + tracing::warn!(%child, %error, "failed to release completed delegation process"); + } + }); + } + for (request, _parent, text, handle, process, owner_request) in deliveries { + let manager = manager.clone(); + tokio::spawn(async move { + let result = tokio::select! { + result = handle.prompt(prompt(&text)) => result, + () = wait_process_end(&manager.shared, process) => Err(anyhow::anyhow!("parent process ended before accepting the child result")), + }; + { + let mut state = manager.shared.state(); + if let Ok(id) = &result { + if state.delegation.completed.remove(&(process, id.clone())) { + state + .delegation + .deliveries + .retain(|d| d.request != request || d.process != Some(process)); + } else if let Some(delivery) = state + .delegation + .deliveries + .iter_mut() + .find(|d| d.request == request && d.process == Some(process)) + { + delivery.submitted = Some((process, id.clone())); + } + } else if let Some(delivery) = state + .delegation + .deliveries + .iter_mut() + .find(|d| d.request == request && d.process == Some(process)) + { + delivery.in_flight = false; + } + } + manager.record_submission(owner_request, result, false); + }); + } +} + +async fn wait_process_end(shared: &Arc, process: AgentProcessId) { + loop { + let ended = shared.process_ended.notified(); + tokio::pin!(ended); + ended.as_mut().enable(); + if !shared.state().handles.contains_key(&process) { + return; + } + ended.await; + } +} diff --git a/nori-rs/harness/src/session_manager/delegation_state.rs b/nori-rs/harness/src/session_manager/delegation_state.rs new file mode 100644 index 000000000..83e86e4cc --- /dev/null +++ b/nori-rs/harness/src/session_manager/delegation_state.rs @@ -0,0 +1,255 @@ +//! Delegation state, event attribution and explicit-close traversal. +use super::*; +use nori_protocol::AcpEvent; +use nori_protocol::Delegation; +use nori_protocol::DelegationStatus; +use nori_protocol::SessionPhase; +use nori_protocol::acp; +use std::collections::HashSet; +use std::collections::VecDeque; + +pub(super) struct Entry { + pub(super) value: Delegation, + pub(super) submitted: bool, + pub(super) submitting: i32, + pub(super) pending_turns: HashSet, + pub(super) completed_turns: HashSet, + pub(super) output: String, + pub(super) last_result: String, + pub(super) failure: Option, + pub(super) cancelled: bool, +} + +pub(super) struct Delivery { + pub(super) request: String, + pub(super) parent: ConversationId, + pub(super) text: String, + pub(super) in_flight: bool, + pub(super) submitted: Option<(AgentProcessId, acp::v1::RequestId)>, + pub(super) process: Option, +} + +#[derive(Default)] +pub(super) struct DelegationState { + pub(super) entries: Vec, + pub(super) closed: HashSet, + pub(super) deliveries: VecDeque, + pub(super) completed: HashSet<(AgentProcessId, acp::v1::RequestId)>, +} + +pub(super) fn active(status: DelegationStatus) -> bool { + matches!( + status, + DelegationStatus::Running | DelegationStatus::AwaitingApproval + ) +} + +impl DelegationState { + pub(super) fn observe(&mut self, process: AgentProcessId, envelope: &SessionEnvelope) { + if let SessionEvent::Acp(AcpEvent::Response { + request_id, + response: Ok(acp::v1::AgentResponse::PromptResponse(_)), + }) = &envelope.event + { + if self.deliveries.iter().any(|delivery| { + delivery.parent.to_string() == envelope.conversation_id + && delivery.in_flight + && delivery.submitted.is_none() + }) { + self.completed.insert((process, request_id.clone())); + } + self.deliveries.retain(|delivery| { + delivery.submitted.as_ref() != Some(&(process, request_id.clone())) + }); + } + if let SessionEvent::Nori(NoriEvent::SessionForked(forked)) = &envelope.event { + for entry in &mut self.entries { + if entry.value.child_id == forked.previous_conversation_id { + entry.value.child_id.clone_from(&forked.new_conversation_id); + } + if entry.value.parent_id == forked.previous_conversation_id { + entry + .value + .parent_id + .clone_from(&forked.new_conversation_id); + } + } + if let Ok(child) = ConversationId::from_string(&forked.new_conversation_id) { + for delivery in &mut self.deliveries { + if delivery.parent.to_string() == forked.previous_conversation_id { + delivery.parent = child; + } + } + } + } + let Some(entry) = self.entries.iter_mut().find(|e| { + e.value.child_id == envelope.conversation_id + && e.value.status == DelegationStatus::Running + }) else { + return; + }; + match &envelope.event { + SessionEvent::Nori(NoriEvent::SessionPhaseChanged(SessionPhase::Prompting { + .. + })) => entry.output.clear(), + SessionEvent::Acp(AcpEvent::Notification( + acp::v1::AgentNotification::SessionNotification(n), + )) => { + if let acp::v1::SessionUpdate::AgentMessageChunk(chunk) = &n.update + && let acp::v1::ContentBlock::Text(text) = &chunk.content + { + entry.output.push_str(&text.text); + } + } + SessionEvent::Acp(AcpEvent::Response { + request_id, + response, + }) => { + if matches!(response, Ok(acp::v1::AgentResponse::PromptResponse(_))) + || entry.pending_turns.contains(request_id) + { + if !entry.pending_turns.remove(request_id) { + entry.completed_turns.insert(request_id.clone()); + } + entry.last_result.clone_from(&entry.output); + if let Ok(acp::v1::AgentResponse::PromptResponse(response)) = response { + entry.cancelled = response.stop_reason == acp::v1::StopReason::Cancelled; + } + if let Err(error) = response { + entry.failure = Some(error.to_string()); + } + } + } + SessionEvent::Nori(NoriEvent::RequestFailed(failure)) => { + entry.failure = Some(format!("{failure:?}")) + } + _ => {} + } + } +} + +impl State { + pub(super) fn observe_delegation( + &mut self, + process: AgentProcessId, + envelope: &SessionEnvelope, + ) { + self.delegation.observe(process, envelope); + if let SessionEvent::Nori(NoriEvent::SessionForked(forked)) = &envelope.event { + let changed: Vec<_> = self + .delegation + .entries + .iter() + .filter(|entry| { + entry.value.parent_id == forked.new_conversation_id + || entry.value.child_id == forked.new_conversation_id + }) + .map(|entry| entry.value.clone()) + .collect(); + for value in changed { + self.publish_delegation(&value); + } + } + } + pub(super) fn notify_child_request(&mut self, envelope: &SessionEnvelope) { + if !matches!( + &envelope.event, + SessionEvent::Acp(AcpEvent::Request { + request: acp::v1::AgentRequest::RequestPermissionRequest(_), + .. + }) + ) { + return; + } + let Some(entry) = self + .delegation + .entries + .iter() + .find(|entry| entry.value.child_id == envelope.conversation_id) + else { + return; + }; + let value = entry.value.clone(); + let Ok(parent) = ConversationId::from_string(&value.parent_id) else { + return; + }; + let event = SessionEvent::Nori(NoriEvent::Notice(nori_protocol::Notice { + message: format!( + "Child session {} ({}) of parent session {} ({}) needs your answer. Switch to the child with Alt+Down.", + value.child_id, value.agent, value.parent_id, value.parent_agent + ), + })); + if let Some(envelope) = self.registry.emit(&parent, event) { + self.journal.publish(&envelope); + if let Some(primary) = self.primary.get(&parent) { + let _ = primary.send(envelope); + } + } + } + pub(super) fn publish_delegation(&mut self, value: &Delegation) { + for owner in [&value.parent_id, &value.child_id] { + if let Ok(id) = ConversationId::from_string(owner) + && let Some(envelope) = self.registry.emit( + &id, + SessionEvent::Nori(NoriEvent::DelegationChanged(value.clone())), + ) + { + self.journal.publish(&envelope); + if let Some(primary) = self.primary.get(&id) { + let _ = primary.send(envelope); + } + } + } + } + + pub(super) fn queue_result(&mut self, value: &Delegation) { + let Ok(parent) = ConversationId::from_string(&value.parent_id) else { + return; + }; + if self.delegation.closed.contains(&parent) { + return; + } + let result = value.result.as_deref().unwrap_or(""); + self.delegation.deliveries.push_back(Delivery { + request: value.request_id.clone(), parent, in_flight: false, submitted: None, process: None, + text: format!("[Nori delegation result]\nChild session {} ({}) for parent session {} ({}): {:?}\n{}", value.child_id, value.agent, value.parent_id, value.parent_agent, value.status, result), + }); + } + + pub(super) fn cancel_tree(&mut self, parent: &ConversationId) -> Vec { + let mut tree = vec![*parent]; + let mut next = 0; + while next < tree.len() { + let member = tree[next]; + for record in self.registry.records() { + if record.parent == Some(member) && !tree.contains(&record.conversation_id) { + tree.push(record.conversation_id); + } + } + next += 1; + } + self.delegation.closed.extend(tree.iter().copied()); + self.delegation + .deliveries + .retain(|d| !tree.contains(&d.parent)); + let mut changed = Vec::new(); + for entry in &mut self.delegation.entries { + if active(entry.value.status) + && tree.iter().any(|id| { + entry.value.parent_id == id.to_string() + || entry.value.child_id == id.to_string() + }) + { + entry.value.status = DelegationStatus::Cancelled; + entry.value.result = Some("Session closed by the user".into()); + changed.push(entry.value.clone()); + } + } + for value in changed { + self.publish_delegation(&value); + self.queue_result(&value); + } + tree.reverse(); + tree + } +} diff --git a/nori-rs/harness/src/session_manager/mod.rs b/nori-rs/harness/src/session_manager/mod.rs index f5b112b42..762e71bbd 100644 --- a/nori-rs/harness/src/session_manager/mod.rs +++ b/nori-rs/harness/src/session_manager/mod.rs @@ -1,5 +1,9 @@ //! Session manager: one registry of live sessions per Nori process. +mod delegation; +mod delegation_driver; +mod delegation_state; +pub use delegation::DelegationContext; mod registry; mod replay; @@ -76,6 +80,8 @@ pub enum StartError { ShuttingDown, #[error("parent session {0} is not registered")] UnknownParent(ConversationId), + #[error("parent session {0} is not running")] + ParentEnded(ConversationId), #[error("the prepared agent already carries a session")] AgentAlreadyActivated, #[error("session {0} has not failed; only failed sessions can be resumed")] @@ -174,12 +180,16 @@ struct Shared { config: SessionManagerConfig, state: Mutex, process_ended: Notify, + admission: tokio::sync::Mutex<()>, } struct State { registry: Registry, handles: HashMap, journal: replay::Journal, + delegation: delegation::DelegationState, + configs: HashMap, + primary: HashMap>, } impl Shared { @@ -200,10 +210,14 @@ impl SessionManager { shared: Arc::new(Shared { config, process_ended: Notify::new(), + admission: tokio::sync::Mutex::new(()), state: Mutex::new(State { registry: Registry::new(config.max_sessions), handles: HashMap::new(), journal: replay::Journal::new(capacity), + delegation: delegation::DelegationState::default(), + configs: HashMap::new(), + primary: HashMap::new(), }), }), } @@ -263,30 +277,69 @@ impl SessionManager { model: config.default_model.clone(), approval_policy: config.approval_policy, }; - let (agent_source, settings) = match agent { + let (agent_source, settings, config) = match agent { SessionAgent::Prepared(mut agent) => { let config = agent .activation_config_mut() .ok_or(StartError::AgentAlreadyActivated)?; config.conversation_id = Some(conversation_id); let settings = settings_from(config); - (SessionAgentSource::Prepared(Box::new(agent)), settings) + let config = config.clone(); + ( + SessionAgentSource::Prepared(Box::new(agent)), + settings, + config, + ) } SessionAgent::Prepare(spec) => { let mut config = build_backend_config(spec); config.conversation_id = Some(conversation_id); let settings = settings_from(&config); - (SessionAgentSource::Prepare(Box::new(config)), settings) + ( + SessionAgentSource::Prepare(Box::new(config.clone())), + settings, + config, + ) } }; + self.launch_registered(agent_source, settings, config, start) + } + + fn launch_registered( + &self, + mut agent_source: SessionAgentSource, + settings: SessionSettings, + mut config: AcpBackendConfig, + start: SessionStart, + ) -> Result { + let conversation_id = settings.conversation_id; let process = { let mut state = self.shared.state(); + if settings.source == SessionSource::Session + && let Some(parent) = settings.parent + && (state.registry.live_process(&parent).is_none() + || state.delegation.closed.contains(&parent)) + { + return Err(StartError::ParentEnded(parent)); + } let reservation = state.registry.reserve()?; let process = state.registry.register(settings, reservation)?; state.journal.register(conversation_id.to_string()); process }; + if config.multi_session { + config.delegation_context = Some(DelegationContext::new(&self.shared, process)); + } + match &mut agent_source { + SessionAgentSource::Prepared(agent) => { + if let Some(activation) = agent.activation_config_mut() { + activation.delegation_context = config.delegation_context.clone(); + } + } + SessionAgentSource::Prepare(activation) => **activation = config.clone(), + } + self.shared.state().configs.insert(process, config); let launched = launch_session_from(agent_source, start); // Answers are queued before they reach the agent, and the pump reads // this queue first, so an answer is numbered ahead of anything the @@ -320,6 +373,10 @@ impl SessionManager { } let (primary_tx, primary_rx) = mpsc::unbounded_channel(); + self.shared + .state() + .primary + .insert(conversation_id, primary_tx.clone()); let shared = Arc::clone(&self.shared); let mut events = launched.events; tokio::spawn(async move { @@ -332,11 +389,20 @@ impl SessionManager { None => break, }, }; - let envelopes = { + { let mut state = shared.state(); let envelopes = state.registry.observe_all(process, event); for envelope in &envelopes { state.journal.publish(envelope); + if let Ok(id) = ConversationId::from_string(&envelope.conversation_id) { + state + .primary + .entry(id) + .or_insert_with(|| primary_tx.clone()); + let _ = primary_tx.send(envelope.clone()); + } + state.observe_delegation(process, envelope); + state.notify_child_request(envelope); } let ended = state .registry @@ -347,20 +413,25 @@ impl SessionManager { // Holding a handle to an ended session would keep its // runtime, and so this stream, alive. state.handles.remove(&process); + state.configs.remove(&process); + if let Some(id) = state.registry.conversation_of(process) { + state.primary.remove(&id); + } shared.process_ended.notify_waiters(); } - envelopes }; - for envelope in envelopes { - // The client may stop reading; status and subscribers - // still follow the session. - let _ = primary_tx.send(envelope); - } + delegation::wake(&shared); } let mut state = shared.state(); + if let Some(id) = state.registry.conversation_of(process) { + state.primary.remove(&id); + } state.registry.process_exited(process); + state.configs.remove(&process); state.handles.remove(&process); shared.process_ended.notify_waiters(); + drop(state); + delegation::wake(&shared); }); Ok(ManagedSession { @@ -469,6 +540,22 @@ impl SessionManager { &self, conversation_id: &ConversationId, child_grace: Duration, + ) -> anyhow::Result<()> { + let ids = { + let _admission = self.shared.admission.lock().await; + let mut state = self.shared.state(); + state.cancel_tree(conversation_id) + }; + for id in ids { + self.close_one(&id, child_grace).await?; + } + Ok(()) + } + + async fn close_one( + &self, + conversation_id: &ConversationId, + child_grace: Duration, ) -> anyhow::Result<()> { let process_and_handle = { let state = self.shared.state(); @@ -512,8 +599,17 @@ impl SessionManager { /// handle stdin EOF before it is stopped. pub async fn shutdown(&self, child_grace: Duration) -> ShutdownReport { let live: Vec<(ConversationId, HarnessHandle)> = { - let state = self.shared.state(); + let mut state = self.shared.state(); state.registry.close_admission(); + let ids: Vec<_> = state + .registry + .records() + .into_iter() + .map(|r| r.conversation_id) + .collect(); + for id in ids { + state.cancel_tree(&id); + } state .handles .iter() diff --git a/nori-rs/harness/src/session_manager/registry.rs b/nori-rs/harness/src/session_manager/registry.rs index c1c9e1707..229cdb626 100644 --- a/nori-rs/harness/src/session_manager/registry.rs +++ b/nori-rs/harness/src/session_manager/registry.rs @@ -65,6 +65,7 @@ struct Entry { busy: bool, /// Agent requests waiting on a client, in arrival order. pending_requests: Vec, + delegation_approvals: std::collections::HashSet, /// Answers that arrived before the request they answer. A client can /// read a request from the harness directly, ahead of this registry. early_answers: Vec, @@ -86,6 +87,7 @@ impl Entry { started: false, busy: false, pending_requests: Vec::new(), + delegation_approvals: std::collections::HashSet::new(), early_answers: Vec::new(), ended: None, forked_into: None, @@ -99,7 +101,7 @@ impl Entry { ended } else if !self.started { SessionStatus::Starting - } else if !self.pending_requests.is_empty() { + } else if !self.pending_requests.is_empty() || !self.delegation_approvals.is_empty() { SessionStatus::WaitingOnYou } else if self.busy { SessionStatus::Working @@ -242,6 +244,34 @@ impl Registry { Ok(process) } + /// Number manager-owned events even when the session has no live process. + pub(crate) fn emit( + &mut self, + conversation: &ConversationId, + event: SessionEvent, + ) -> Option { + let entry = self.sessions.get_mut(conversation)?; + if let SessionEvent::Nori(NoriEvent::DelegationChanged(delegation)) = &event + && delegation.parent_id == conversation.to_string() + { + if delegation.status == nori_protocol::DelegationStatus::AwaitingApproval { + entry + .delegation_approvals + .insert(delegation.request_id.clone()); + } else { + entry.delegation_approvals.remove(&delegation.request_id); + } + } + let seq = entry.next_seq; + entry.next_seq += 1; + Some(SessionEnvelope { + conversation_id: conversation.to_string(), + agent: entry.settings.agent.clone(), + seq, + event, + }) + } + /// Apply one event from `process` and return it wrapped in its envelope. pub(crate) fn observe( &mut self, @@ -412,6 +442,13 @@ impl Registry { entry.acp_session_id = Some(acp_session_id); entry.transcript_path = transcript_dir.map(|dir| dir.join(format!("{child}.jsonl"))); self.sessions.insert(child, entry); + for entry in self.sessions.values_mut() { + if entry.settings.parent == Some(parent) + && entry.settings.source == SessionSource::Session + { + entry.settings.parent = Some(child); + } + } self.processes.insert(process, child); } diff --git a/nori-rs/harness/tests/delegation.rs b/nori-rs/harness/tests/delegation.rs new file mode 100644 index 000000000..769eb7e47 --- /dev/null +++ b/nori-rs/harness/tests/delegation.rs @@ -0,0 +1,994 @@ +//! Delegation boundaries exercised with real ACP subprocesses and their MCP client. +#![allow( + clippy::unwrap_used, + clippy::expect_used, + reason = "integration assertions identify failed behavior" +)] +use nori_config::NoriConfig; +use nori_harness::ConversationId; +use nori_harness::remote_agent::ManagedAgent; +use nori_harness::remote_agent::ManagedRemoteHost; +use nori_harness::runtime::AgentPrepareSpec; +use nori_harness::runtime::SessionStart; +use nori_harness::session_manager::ManagedSession; +use nori_harness::session_manager::SessionAgent; +use nori_harness::session_manager::SessionManager; +use nori_harness::session_manager::SessionManagerConfig; +use nori_harness::session_manager::SessionSource; +use nori_harness::session_manager::StartSession; +use nori_protocol::AcpEvent; +use nori_protocol::Delegation; +use nori_protocol::DelegationStatus; +use nori_protocol::NoriEvent; +use nori_protocol::SessionEvent; +use nori_protocol::SessionStatus; +use nori_protocol::acp; +use serial_test::serial; +use std::path::Path; +use std::sync::Arc; +use std::time::Duration; + +const WAIT: Duration = Duration::from_secs(20); +struct Env(&'static str, Option); +impl Env { + fn set(key: &'static str, value: &str) -> Self { + let old = std::env::var(key).ok(); + unsafe { + std::env::set_var(key, value); + } + Self(key, old) + } +} +impl Drop for Env { + fn drop(&mut self) { + unsafe { + match &self.1 { + Some(v) => std::env::set_var(self.0, v), + None => std::env::remove_var(self.0), + } + } + } +} +fn spec(home: &Path, enabled: bool) -> AgentPrepareSpec { + AgentPrepareSpec { + config: Arc::new(NoriConfig { + active_agent: "mock-model".into(), + cwd: home.into(), + nori_home: home.into(), + multi_session: enabled, + ..Default::default() + }), + cli_version: "delegation-test".into(), + session_context: None, + initial_context: None, + } +} +fn start(manager: &SessionManager, home: &Path, enabled: bool) -> ManagedSession { + manager + .start(StartSession { + agent: SessionAgent::Prepare(spec(home, enabled)), + start: SessionStart::New, + parent: None, + source: SessionSource::User, + }) + .expect("parent starts") +} +fn text(s: &str) -> Vec { + vec![acp::v1::ContentBlock::Text(acp::v1::TextContent::new(s))] +} +fn id(d: &Delegation) -> ConversationId { + ConversationId::from_string(&d.child_id).expect("child identity") +} +async fn status(m: &SessionManager, id: &ConversationId, wanted: SessionStatus) { + tokio::time::timeout(WAIT, async { + loop { + if m.get(id).is_some_and(|r| r.status == wanted) { + break; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("session reaches status"); +} +async fn terminal(m: &SessionManager, parent: &ConversationId, request: &str) -> Delegation { + tokio::time::timeout(WAIT, async { + loop { + if let Some(d) = m.delegations(parent).into_iter().find(|d| { + d.request_id == request + && matches!( + d.status, + DelegationStatus::Completed + | DelegationStatus::Failed + | DelegationStatus::Cancelled + | DelegationStatus::Declined + ) + }) { + return d; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("delegation completes") +} +fn manager(cap: i32) -> SessionManager { + SessionManager::new(SessionManagerConfig { + max_sessions: cap, + ..Default::default() + }) +} + +async fn wait_parent_reply(parent: &mut ManagedSession, child: &str) -> String { + tokio::time::timeout(WAIT, async { + loop { + if let SessionEvent::Acp(AcpEvent::Notification( + acp::v1::AgentNotification::SessionNotification(n), + )) = parent.events.recv().await.unwrap().event + && let acp::v1::SessionUpdate::AgentMessageChunk(chunk) = n.update + && let acp::v1::ContentBlock::Text(text) = chunk.content + && text.text.contains(child) + { + return text.text; + } + } + }) + .await + .expect("parent agent consumes attributed child result") +} + +#[tokio::test] +#[serial] +async fn same_kind_starts_immediately_and_retains_child_history_after_releasing_slot() { + let home = tempfile::tempdir().unwrap(); + let m = manager(2); + let p = start(&m, home.path(), true); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + let d = m + .delegate( + &p.conversation_id, + "mock-model".into(), + "summarize E1".into(), + ) + .await + .unwrap(); + assert_eq!(d.status, DelegationStatus::Running); + let done = terminal(&m, &p.conversation_id, &d.request_id).await; + assert_eq!(done.status, DelegationStatus::Completed); + assert!(done.result.is_some()); + status(&m, &id(&d), SessionStatus::Closed).await; + assert_eq!(m.free_slots(), 1); + let child = m.get(&id(&d)).unwrap(); + assert_eq!(child.parent, Some(p.conversation_id)); + assert_eq!(child.source, SessionSource::Session); + assert!(child.transcript_path.is_some()); + m.shutdown(Duration::ZERO).await; +} +#[tokio::test] +#[serial] +async fn different_kind_waits_for_approval_and_decline_never_launches() { + let home = tempfile::tempdir().unwrap(); + let m = manager(3); + let p = start(&m, home.path(), true); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + let d = m + .delegate( + &p.conversation_id, + "mock-model-alt".into(), + "child task".into(), + ) + .await + .unwrap(); + assert_eq!(d.status, DelegationStatus::AwaitingApproval); + assert_eq!(m.list().len(), 1); + let no = m + .resolve_delegation(&p.conversation_id, &d.request_id, false) + .await + .unwrap(); + assert_eq!(no.status, DelegationStatus::Declined); + assert_eq!(m.list().len(), 1); + assert!( + m.resolve_delegation(&p.conversation_id, &d.request_id, true) + .await + .is_err() + ); + let d = m + .delegate( + &p.conversation_id, + "mock-model-alt".into(), + "approved task".into(), + ) + .await + .unwrap(); + let yes = m + .resolve_delegation(&p.conversation_id, &d.request_id, true) + .await + .unwrap(); + assert_eq!(yes.status, DelegationStatus::Running); + assert_eq!( + terminal(&m, &p.conversation_id, &d.request_id).await.status, + DelegationStatus::Completed + ); + m.shutdown(Duration::ZERO).await; +} +#[tokio::test] +#[serial] +async fn depth_two_is_allowed_and_depth_three_is_rejected() { + let home = tempfile::tempdir().unwrap(); + let m = manager(5); + let p = start(&m, home.path(), true); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + let a = m + .delegate( + &p.conversation_id, + "mock-model".into(), + "mock:stream-until-cancel".into(), + ) + .await + .unwrap(); + status(&m, &id(&a), SessionStatus::Working).await; + let b = m + .delegate( + &id(&a), + "mock-model".into(), + "mock:stream-until-cancel".into(), + ) + .await + .unwrap(); + status(&m, &id(&b), SessionStatus::Working).await; + assert_eq!(b.depth, 2); + let err = m + .delegate(&id(&b), "mock-model".into(), "too deep".into()) + .await + .unwrap_err(); + assert!(err.to_string().contains("depth")); + m.shutdown(Duration::ZERO).await; +} +#[tokio::test] +#[serial] +async fn child_permission_belongs_to_child_and_family_reports_waiting() { + let home = tempfile::tempdir().unwrap(); + let m = manager(3); + let p = start(&m, home.path(), true); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + let mut events = m.subscribe(4096); + let d = m + .delegate( + &p.conversation_id, + "mock-model".into(), + "mock:request-permission:echo child".into(), + ) + .await + .unwrap(); + let request = tokio::time::timeout(WAIT, async { + loop { + let e = events.recv().await.unwrap(); + if let SessionEvent::Acp(AcpEvent::Request { + request_id, + request: acp::v1::AgentRequest::RequestPermissionRequest(_), + }) = e.event + { + assert_eq!(e.conversation_id, d.child_id); + return request_id; + } + } + }) + .await + .unwrap(); + assert_eq!(m.get(&id(&d)).unwrap().parent, Some(p.conversation_id)); + assert!( + m.get(&p.conversation_id) + .unwrap() + .pending_requests + .is_empty() + ); + assert_eq!( + m.family_status(&p.conversation_id), + Some(SessionStatus::WaitingOnYou) + ); + m.handle(&id(&d)) + .unwrap() + .respond_to_agent( + request, + Ok(acp::v1::ClientResponse::RequestPermissionResponse( + acp::v1::RequestPermissionResponse::new( + acp::v1::RequestPermissionOutcome::Cancelled, + ), + )), + ) + .await + .unwrap(); + terminal(&m, &p.conversation_id, &d.request_id).await; + m.shutdown(Duration::ZERO).await; +} +#[tokio::test] +#[serial] +async fn result_is_automatically_submitted_as_attributed_parent_input() { + let _echo = Env::set("MOCK_AGENT_ECHO_PROMPT", "1"); + let home = tempfile::tempdir().unwrap(); + let m = manager(3); + let mut p = start(&m, home.path(), true); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + let d = m + .delegate( + &p.conversation_id, + "mock-model".into(), + "unique child payload".into(), + ) + .await + .unwrap(); + tokio::time::timeout(WAIT, async { + loop { + let e = p.events.recv().await.unwrap(); + if let SessionEvent::Acp(AcpEvent::Notification( + acp::v1::AgentNotification::SessionNotification(n), + )) = e.event + && let acp::v1::SessionUpdate::AgentMessageChunk(c) = n.update + && let acp::v1::ContentBlock::Text(t) = c.content + && t.text.contains("unique child payload") + { + assert!(t.text.contains(&d.child_id)); + assert!(t.text.contains(&d.parent_id)); + break; + } + } + }) + .await + .expect("automatic result reaches parent agent"); + m.shutdown(Duration::ZERO).await; +} +#[tokio::test] +#[serial] +async fn closing_parent_cancels_running_descendants_and_pending_approvals() { + let home = tempfile::tempdir().unwrap(); + let m = manager(5); + let p = start(&m, home.path(), true); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + let a = m + .delegate( + &p.conversation_id, + "mock-model".into(), + "mock:stream-until-cancel".into(), + ) + .await + .unwrap(); + status(&m, &id(&a), SessionStatus::Working).await; + let grandchild = m + .delegate( + &id(&a), + "mock-model".into(), + "mock:stream-until-cancel".into(), + ) + .await + .unwrap(); + status(&m, &id(&grandchild), SessionStatus::Working).await; + let pending = m + .delegate( + &p.conversation_id, + "mock-model-alt".into(), + "not started".into(), + ) + .await + .unwrap(); + m.close(&p.conversation_id, Duration::ZERO).await.unwrap(); + assert_eq!( + terminal(&m, &p.conversation_id, &a.request_id).await.status, + DelegationStatus::Cancelled + ); + assert_eq!( + terminal(&m, &p.conversation_id, &pending.request_id) + .await + .status, + DelegationStatus::Cancelled + ); + assert!( + m.resolve_delegation(&p.conversation_id, &pending.request_id, true) + .await + .is_err() + ); + assert_eq!( + terminal(&m, &id(&a), &grandchild.request_id).await.status, + DelegationStatus::Cancelled + ); + assert_eq!(m.free_slots(), 5); +} +#[tokio::test] +#[serial] +async fn child_crash_returns_failure_without_failing_parent() { + let home = tempfile::tempdir().unwrap(); + let m = manager(3); + let mut p = start(&m, home.path(), true); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + let d = m + .delegate( + &p.conversation_id, + "mock-model".into(), + "mock:exit-during-prompt".into(), + ) + .await + .unwrap(); + let done = terminal(&m, &p.conversation_id, &d.request_id).await; + assert_eq!(done.status, DelegationStatus::Failed); + assert!(m.handle(&p.conversation_id).is_some()); + let reply = wait_parent_reply(&mut p, &d.child_id).await; + assert!(reply.contains(&d.parent_id)); + assert!(reply.contains("Failed")); + m.shutdown(Duration::ZERO).await; +} +#[tokio::test] +#[serial] +async fn competing_children_share_the_same_session_cap() { + let home = tempfile::tempdir().unwrap(); + let m = manager(2); + let p = start(&m, home.path(), true); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + let (a, b) = tokio::join!( + m.delegate( + &p.conversation_id, + "mock-model".into(), + "mock:stream-until-cancel".into() + ), + m.delegate( + &p.conversation_id, + "mock-model".into(), + "mock:stream-until-cancel".into() + ) + ); + assert_ne!(a.is_ok(), b.is_ok()); + assert_eq!(m.free_slots(), 0); + m.shutdown(Duration::ZERO).await; +} +#[tokio::test] +#[serial] +async fn flag_off_rejects_delegation_without_starting_child() { + let home = tempfile::tempdir().unwrap(); + let m = manager(3); + let p = start(&m, home.path(), false); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + assert!( + m.delegate(&p.conversation_id, "mock-model".into(), "task".into()) + .await + .unwrap_err() + .to_string() + .contains("multi_session") + ); + assert_eq!(m.list().len(), 1); + m.shutdown(Duration::ZERO).await; +} +#[tokio::test] +#[serial] +async fn actual_agent_mcp_request_starts_child_and_nested_results_finish_once() { + let _http = Env::set("MOCK_AGENT_MCP_HTTP", "1"); + let _echo = Env::set("MOCK_AGENT_ECHO_PROMPT", "1"); + let home = tempfile::tempdir().unwrap(); + let m = manager(4); + let p = start(&m, home.path(), true); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + let mut events = m.subscribe(4096); + p.handle + .prompt(text( + "mock:delegate mock-model mock:delegate mock-model nested leaf", + )) + .await + .unwrap(); + tokio::time::timeout(WAIT, async { + loop { + let ds = m.delegations(&p.conversation_id); + if ds.iter().any(|d| d.status == DelegationStatus::Completed) { + break; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("nested delegation completes"); + let top = m.delegations(&p.conversation_id).pop().unwrap(); + let nested = m.delegations(&id(&top)); + assert_eq!(nested.len(), 1); + assert_eq!(nested[0].status, DelegationStatus::Completed); + assert!(top.result.as_ref().unwrap().contains("nested leaf")); + assert!(top.result.as_ref().unwrap().contains(&nested[0].child_id)); + let mut completions = 0; + let mut grandchild_finished = false; + let mut continuation_finished = false; + while let Ok(e) = events.try_recv() { + match e.event { + SessionEvent::Nori(NoriEvent::DelegationChanged(d)) + if d.status == DelegationStatus::Completed => + { + if d.request_id == nested[0].request_id { + grandchild_finished = true; + } + if d.request_id == top.request_id + && e.conversation_id == p.conversation_id.to_string() + { + assert!( + continuation_finished, + "child continuation finished before reporting final result" + ); + completions += 1; + } + } + SessionEvent::Acp(AcpEvent::Response { + response: Ok(acp::v1::AgentResponse::PromptResponse(_)), + .. + }) if e.conversation_id == top.child_id && grandchild_finished => { + continuation_finished = true + } + _ => {} + } + } + assert_eq!(completions, 1); + m.shutdown(Duration::ZERO).await; +} +#[tokio::test] +#[serial] +async fn crashed_parent_preserves_child_and_result_reaches_stable_resume() { + let _delay = Env::set("MOCK_AGENT_DELAY_MS", "500"); + let _response = Env::set("MOCK_AGENT_RESPONSE", "held unique result"); + let home = tempfile::tempdir().unwrap(); + let m = manager(3); + let p = start(&m, home.path(), true); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + let d = m + .delegate( + &p.conversation_id, + "mock-model".into(), + "surviving child".into(), + ) + .await + .unwrap(); + p.handle + .prompt(text("mock:exit-during-prompt")) + .await + .unwrap(); + status(&m, &p.conversation_id, SessionStatus::Failed).await; + assert_eq!( + terminal(&m, &p.conversation_id, &d.request_id).await.status, + DelegationStatus::Completed + ); + let _echo = Env::set("MOCK_AGENT_ECHO_PROMPT", "1"); + let mut resumed = m + .resume(&p.conversation_id, spec(home.path(), true)) + .await + .unwrap(); + assert_eq!(resumed.conversation_id, p.conversation_id); + tokio::time::timeout(WAIT, async { + loop { + if let SessionEvent::Acp(AcpEvent::Notification( + acp::v1::AgentNotification::SessionNotification(n), + )) = resumed.events.recv().await.unwrap().event + && let acp::v1::SessionUpdate::AgentMessageChunk(c) = n.update + && let acp::v1::ContentBlock::Text(t) = c.content + && t.text.contains(&d.child_id) + && t.text.contains("held unique result") + { + break; + } + } + }) + .await + .expect("held child result starts resumed parent turn"); + m.shutdown(Duration::ZERO).await; +} +#[tokio::test] +#[serial] +async fn closing_crashed_parent_still_cancels_child() { + let home = tempfile::tempdir().unwrap(); + let m = manager(3); + let p = start(&m, home.path(), true); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + let d = m + .delegate( + &p.conversation_id, + "mock-model".into(), + "mock:stream-until-cancel".into(), + ) + .await + .unwrap(); + status(&m, &id(&d), SessionStatus::Working).await; + p.handle + .prompt(text("mock:exit-during-prompt")) + .await + .unwrap(); + status(&m, &p.conversation_id, SessionStatus::Failed).await; + m.close(&p.conversation_id, Duration::ZERO).await.unwrap(); + assert_eq!( + terminal(&m, &p.conversation_id, &d.request_id).await.status, + DelegationStatus::Cancelled + ); + assert!(m.handle(&id(&d)).is_none()); +} +#[tokio::test] +#[serial] +async fn approval_racing_parent_close_cannot_leave_a_running_child() { + let home = tempfile::tempdir().unwrap(); + let m = manager(3); + let p = start(&m, home.path(), true); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + let d = m + .delegate( + &p.conversation_id, + "mock-model-alt".into(), + "mock:stream-until-cancel".into(), + ) + .await + .unwrap(); + let (_approved, closed) = tokio::join!( + m.resolve_delegation(&p.conversation_id, &d.request_id, true), + m.close(&p.conversation_id, Duration::ZERO) + ); + closed.unwrap(); + assert_eq!( + terminal(&m, &p.conversation_id, &d.request_id).await.status, + DelegationStatus::Cancelled + ); + assert_eq!(m.free_slots(), 3); +} +#[tokio::test] +#[serial] +async fn child_result_waits_in_busy_parent_queue() { + let delay = Env::set("MOCK_AGENT_DELAY_MS", "1000"); + let home = tempfile::tempdir().unwrap(); + let m = manager(3); + let mut p = start(&m, home.path(), true); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + drop(delay); + p.handle.prompt(text("finish parent work")).await.unwrap(); + status(&m, &p.conversation_id, SessionStatus::Working).await; + let mut events = m.subscribe(4096); + let d = m + .delegate( + &p.conversation_id, + "mock-model".into(), + "queued child result".into(), + ) + .await + .unwrap(); + tokio::time::timeout(WAIT, async { + loop { + let e = events.recv().await.unwrap(); + if e.conversation_id == p.conversation_id.to_string() + && let SessionEvent::Nori(NoriEvent::QueueChanged(q)) = e.event + && q.prompts.iter().any(|prompt| prompt.contains(&d.child_id)) + { + break; + } + } + }) + .await + .expect("result queued in busy parent"); + assert_eq!( + terminal(&m, &p.conversation_id, &d.request_id).await.status, + DelegationStatus::Completed + ); + let reply = wait_parent_reply(&mut p, &d.child_id).await; + assert!(reply.contains(&d.parent_id)); + m.shutdown(Duration::ZERO).await; +} + +#[tokio::test] +#[serial] +async fn only_requesting_parent_can_resolve_and_concurrent_approval_starts_one_child() { + let home = tempfile::tempdir().unwrap(); + let m = manager(5); + let p = start(&m, home.path(), true); + let other = start(&m, home.path(), true); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + status(&m, &other.conversation_id, SessionStatus::Ready).await; + let d = m + .delegate( + &p.conversation_id, + "mock-model-alt".into(), + "mock:stream-until-cancel".into(), + ) + .await + .unwrap(); + assert!( + m.resolve_delegation(&other.conversation_id, &d.request_id, true) + .await + .is_err() + ); + let (a, b) = tokio::join!( + m.resolve_delegation(&p.conversation_id, &d.request_id, true), + m.resolve_delegation(&p.conversation_id, &d.request_id, true) + ); + assert_ne!(a.is_ok(), b.is_ok()); + assert_eq!(m.list().len(), 3); + m.shutdown(Duration::ZERO).await; +} +#[tokio::test] +#[serial] +async fn flag_off_agent_cannot_invoke_delegate_over_mcp() { + let _http = Env::set("MOCK_AGENT_MCP_HTTP", "1"); + let home = tempfile::tempdir().unwrap(); + let m = manager(3); + let mut p = start(&m, home.path(), false); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + let request = p + .handle + .prompt(text("mock:delegate mock-model forbidden child")) + .await + .unwrap(); + tokio::time::timeout(WAIT, async { + loop { + if let SessionEvent::Acp(AcpEvent::Response { request_id, .. }) = + p.events.recv().await.unwrap().event + && request_id == request + { + break; + } + } + }) + .await + .unwrap(); + assert_eq!(m.list().len(), 1); + assert!(m.delegations(&p.conversation_id).is_empty()); + m.shutdown(Duration::ZERO).await; +} + +#[tokio::test] +#[serial] +async fn cancelling_child_reports_cancelled_to_parent() { + let home = tempfile::tempdir().unwrap(); + let m = manager(3); + let p = start(&m, home.path(), true); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + let d = m + .delegate( + &p.conversation_id, + "mock-model".into(), + "mock:stream-until-cancel".into(), + ) + .await + .unwrap(); + status(&m, &id(&d), SessionStatus::Working).await; + m.handle(&id(&d)).unwrap().cancel().await.unwrap(); + assert_eq!( + terminal(&m, &p.conversation_id, &d.request_id).await.status, + DelegationStatus::Cancelled + ); + m.shutdown(Duration::ZERO).await; +} + +#[tokio::test] +#[serial] +async fn forking_a_child_preserves_delegation_depth() { + let _http = Env::set("MOCK_AGENT_MCP_HTTP", "1"); + let _fork = Env::set("MOCK_AGENT_SUPPORT_SESSION_FORK", "1"); + let home = tempfile::tempdir().unwrap(); + let m = manager(6); + let p = start(&m, home.path(), true); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + let mut events = m.subscribe(4096); + let child = m + .delegate( + &p.conversation_id, + "mock-model".into(), + "mock:delegate mock-model-alt pending leaf".into(), + ) + .await + .unwrap(); + tokio::time::timeout(WAIT, async { + loop { + if !m.delegations(&id(&child)).is_empty() { + break; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .unwrap(); + tokio::time::timeout(WAIT, async { + loop { + let e = events.recv().await.unwrap(); + if e.conversation_id == child.child_id + && matches!( + e.event, + SessionEvent::Acp(AcpEvent::Response { + response: Ok(acp::v1::AgentResponse::PromptResponse(_)), + .. + }) + ) + { + break; + } + } + }) + .await + .unwrap(); + m.handle(&id(&child)).unwrap().branch().await.unwrap(); + let fork = tokio::time::timeout(WAIT, async { + loop { + if let Some(fork) = m + .list() + .into_iter() + .find(|r| r.parent == Some(id(&child)) && r.source == SessionSource::Fork) + { + return fork.conversation_id; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .unwrap(); + let leaf = m + .delegate( + &fork, + "mock-model".into(), + "mock:stream-until-cancel".into(), + ) + .await + .unwrap(); + assert_eq!(leaf.depth, 2); + status(&m, &id(&leaf), SessionStatus::Working).await; + assert!( + m.delegate(&id(&leaf), "mock-model".into(), "too deep".into()) + .await + .unwrap_err() + .to_string() + .contains("depth") + ); + let listed = ManagedRemoteHost::new(m.clone()) + .request(serde_json::json!({"jsonrpc":"2.0","id":1,"method":"_nori/session/list"})) + .await; + let rows = listed.response["result"]["sessions"].as_array().unwrap(); + assert_eq!( + rows.iter() + .find(|row| row["session_id"] == fork.to_string()) + .unwrap()["depth"], + 1 + ); + assert_eq!( + rows.iter() + .find(|row| row["session_id"] == leaf.child_id) + .unwrap()["depth"], + 2 + ); + m.shutdown(Duration::ZERO).await; +} + +#[tokio::test] +#[serial] +async fn fork_migrates_running_children_and_pending_approval_to_new_parent() { + let _fork = Env::set("MOCK_AGENT_SUPPORT_SESSION_FORK", "1"); + let home = tempfile::tempdir().unwrap(); + let m = manager(5); + let mut p = start(&m, home.path(), true); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + let running = m + .delegate( + &p.conversation_id, + "mock-model".into(), + "mock:stream-until-cancel".into(), + ) + .await + .unwrap(); + let pending = m + .delegate( + &p.conversation_id, + "mock-model-alt".into(), + "mock:stream-until-cancel".into(), + ) + .await + .unwrap(); + p.handle.branch().await.unwrap(); + let fork = tokio::time::timeout(WAIT, async { + loop { + let e = p.events.recv().await.unwrap(); + if let SessionEvent::Nori(NoriEvent::SessionForked(f)) = e.event { + return ConversationId::from_string(&f.new_conversation_id).unwrap(); + } + } + }) + .await + .unwrap(); + status(&m, &fork, SessionStatus::WaitingOnYou).await; + assert_eq!(m.get(&id(&running)).unwrap().parent, Some(fork)); + let approved = m + .resolve_delegation(&fork, &pending.request_id, true) + .await + .unwrap(); + assert_eq!(approved.parent_id, fork.to_string()); + m.close(&fork, Duration::ZERO).await.unwrap(); + assert_eq!(m.free_slots(), 5); + assert_eq!( + terminal(&m, &fork, &running.request_id).await.status, + DelegationStatus::Cancelled + ); + assert_eq!( + terminal(&m, &fork, &pending.request_id).await.status, + DelegationStatus::Cancelled + ); +} + +#[cfg(unix)] +#[tokio::test] +#[serial] +async fn result_queued_before_parent_crash_is_redelivered_after_resume() { + let _reply = Env::set("MOCK_AGENT_RESPONSE", "durable child payload"); + let delay = Env::set("MOCK_AGENT_DELAY_MS", "5000"); + let home = tempfile::tempdir().unwrap(); + let m = manager(3); + let mut parent_spec = spec(home.path(), true); + Arc::make_mut(&mut parent_spec.config).acp_proxy = nori_config::AcpProxyConfig { + enabled: true, + log_dir: home.path().join("parent-wire"), + }; + let p = m + .start(StartSession { + agent: SessionAgent::Prepare(parent_spec), + start: SessionStart::New, + parent: None, + source: SessionSource::User, + }) + .unwrap(); + status(&m, &p.conversation_id, SessionStatus::Ready).await; + drop(delay); + p.handle + .prompt(text("parent interrupted work")) + .await + .unwrap(); + status(&m, &p.conversation_id, SessionStatus::Working).await; + let mut events = m.subscribe(4096); + let child = m + .delegate( + &p.conversation_id, + "mock-model".into(), + "durable task".into(), + ) + .await + .unwrap(); + tokio::time::timeout(WAIT, async { + loop { + let e = events.recv().await.unwrap(); + if e.conversation_id == p.conversation_id.to_string() + && let SessionEvent::Nori(NoriEvent::QueueChanged(q)) = e.event + && q.prompts.iter().any(|text| text.contains(&child.child_id)) + { + break; + } + } + }) + .await + .unwrap(); + let pid = std::fs::read_dir(home.path().join("parent-wire")) + .unwrap() + .map(|entry| entry.unwrap().path()) + .filter_map(|path| { + let name = path.file_name()?.to_str()?; + let pid = name.split('-').nth(1)?.parse::().ok()?; + Some((path, pid)) + }) + .find(|(path, _)| { + std::fs::read_to_string(path) + .unwrap_or_default() + .contains("parent interrupted work") + }) + .unwrap() + .1; + assert_eq!(unsafe { libc::kill(pid, libc::SIGKILL) }, 0); + status(&m, &p.conversation_id, SessionStatus::Failed).await; + let _echo = Env::set("MOCK_AGENT_ECHO_PROMPT", "1"); + let mut resumed = m + .resume(&p.conversation_id, spec(home.path(), true)) + .await + .unwrap(); + tokio::time::timeout(WAIT, async { + loop { + if let SessionEvent::Acp(AcpEvent::Notification( + acp::v1::AgentNotification::SessionNotification(n), + )) = resumed.events.recv().await.unwrap().event + && let acp::v1::SessionUpdate::AgentMessageChunk(c) = n.update + && let acp::v1::ContentBlock::Text(t) = c.content + && t.text.contains(&child.child_id) + && t.text.contains("durable child payload") + { + break; + } + } + }) + .await + .expect("queued result survives parent crash"); + m.shutdown(Duration::ZERO).await; +} diff --git a/nori-rs/harness/tests/managed_remote.rs b/nori-rs/harness/tests/managed_remote.rs index 5ad1f6ca6..8b40909c5 100644 --- a/nori-rs/harness/tests/managed_remote.rs +++ b/nori-rs/harness/tests/managed_remote.rs @@ -52,6 +52,7 @@ impl Fixture { .start(StartSession { agent: SessionAgent::Prepare(AgentPrepareSpec { config: Arc::new(NoriConfig { + multi_session: true, active_agent: if index == 0 { "mock-model" } else { @@ -722,3 +723,115 @@ async fn websocket_subscription_and_precrash_cursor_follow_the_same_id_after_res ); fixture.stop().await; } + +#[tokio::test] +async fn delegation_remote_admission_approval_and_child_events_keep_their_identity() { + let fixture = Fixture::new(4096).await; + let mut controller = fixture.connect().await; + let parent = fixture.ids[0].to_string(); + send( + &mut controller, + 2, + "_nori/session/delegate", + json!({ + "session_id":parent, "agent":"mock-model-alt", "task":"mock:request-permission" + }), + ) + .await; + let requested = receive(&mut controller).await; + assert!(requested["error"].is_null(), "{requested}"); + let delegation = &requested["result"]; + assert_eq!(delegation["status"], "awaiting_approval"); + assert_eq!( + fixture.manager.list().len(), + 2, + "approval cannot spawn a process" + ); + send( + &mut controller, + 3, + "_nori/session/delegation/resolve", + json!({ + "session_id":parent, "request_id":delegation["request_id"], "approved":true + }), + ) + .await; + let approved = receive(&mut controller).await; + assert!(approved["error"].is_null(), "{approved}"); + let child = approved["result"]["child_id"].as_str().unwrap().to_owned(); + let child_id = ConversationId::from_string(&child).unwrap(); + wait_status(&fixture.manager, child_id, SessionStatus::WaitingOnYou).await; + send(&mut controller, 4, "_nori/session/list", json!({})).await; + let listed = receive(&mut controller).await; + let rows = listed["result"]["sessions"].as_array().unwrap(); + let row = rows.iter().find(|row| row["session_id"] == child).unwrap(); + assert_eq!(row["parent"], parent); + assert_eq!(row["source"], "session"); + assert_eq!(row["depth"], 1); + assert_eq!(row["status"], "waiting_on_you"); + send( + &mut controller, + 5, + "_nori/session/subscribe", + json!({"session_id":"*"}), + ) + .await; + assert!(receive(&mut controller).await["error"].is_null()); + let child_permission = tokio::time::timeout(WAIT, async { + loop { + let value = receive(&mut controller).await; + if let Some(envelope) = event(&value) + && envelope.conversation_id == child + && matches!( + envelope.event, + nori_protocol::SessionEvent::Acp(nori_protocol::AcpEvent::Request { .. }) + ) + { + return envelope; + } + } + }) + .await + .unwrap(); + assert_eq!(child_permission.agent, "mock-model-alt"); + assert!( + fixture + .manager + .get(&fixture.ids[0]) + .unwrap() + .pending_requests + .is_empty() + ); + fixture.stop().await; +} + +#[tokio::test] +async fn delegation_remote_decline_and_duplicate_approval_never_spawn_a_child() { + let fixture = Fixture::new(4096).await; + let mut socket = fixture.connect().await; + let parent = fixture.ids[0].to_string(); + send( + &mut socket, + 1, + "_nori/session/delegate", + json!({ + "session_id":parent, "agent":"mock-model-alt", "task":"do not start" + }), + ) + .await; + let requested = receive(&mut socket).await; + assert!(requested["error"].is_null(), "{requested}"); + let params = json!({"session_id":parent, "request_id":requested["result"]["request_id"], "approved":false}); + send( + &mut socket, + 2, + "_nori/session/delegation/resolve", + params.clone(), + ) + .await; + assert_eq!(receive(&mut socket).await["result"]["status"], "declined"); + send(&mut socket, 3, "_nori/session/delegation/resolve", json!({"session_id":parent, "request_id":requested["result"]["request_id"], "approved":true})).await; + assert!(receive(&mut socket).await["error"].is_object()); + assert_eq!(fixture.manager.list().len(), 2); + fixture.stop().await; +} diff --git a/nori-rs/mock-acp-agent/Cargo.toml b/nori-rs/mock-acp-agent/Cargo.toml index 7f8d2d2fb..f1f772310 100644 --- a/nori-rs/mock-acp-agent/Cargo.toml +++ b/nori-rs/mock-acp-agent/Cargo.toml @@ -27,3 +27,5 @@ serde_json = "1.0" env_logger = "0.11" tungstenite = "0.26" ureq = "3" + +rmcp = { workspace = true, features = ["client", "transport-streamable-http-client-reqwest"] } diff --git a/nori-rs/mock-acp-agent/docs.md b/nori-rs/mock-acp-agent/docs.md index bad47a38f..7a9e0f260 100644 --- a/nori-rs/mock-acp-agent/docs.md +++ b/nori-rs/mock-acp-agent/docs.md @@ -75,6 +75,17 @@ apart. `MOCK_AGENT_PERMISSION_DELAY_MS` waits before the request is sent, so a test can move to another session before the request arrives in the background. +[`delegation.rs`](src/delegation.rs) exercises the real authenticated HTTP +MCP boundary. A prompt beginning `mock:delegate ` calls `delegate` +on the `nori-client` server advertised to that ACP session, using its advertised +bearer header. `MOCK_AGENT_MCP_HTTP` enables the fixture's HTTP MCP capability. +Nested task markers let a child issue its own delegation. Prompts carrying the +manager's delegation-result prefix follow the result path instead of repeating +a marker embedded in the returned text, so nested results do not recursively +launch work. The [harness tests](../harness/tests/delegation.rs) and +[PTY tests](../tui-pty-e2e/tests/delegation.rs) use this path to cover process +lifetime, approvals, result continuation, and terminal navigation. + The mock deliberately issues outbound client requests through the SDK rather than calling host internals. This makes the public `AcpEvent::Request` and `HarnessHandle::respond_to_agent` path observable end to end. diff --git a/nori-rs/mock-acp-agent/src/delegation.rs b/nori-rs/mock-acp-agent/src/delegation.rs new file mode 100644 index 000000000..5a1984b3a --- /dev/null +++ b/nori-rs/mock-acp-agent/src/delegation.rs @@ -0,0 +1,58 @@ +//! Real MCP calls used by delegation integration scenarios. + +use nori_protocol::acp::v1 as acp; +use rmcp::ServiceExt; +use rmcp::model::CallToolRequestParams; +use rmcp::transport::StreamableHttpClientTransport; +use rmcp::transport::streamable_http_client::StreamableHttpClientTransportConfig; + +pub(crate) fn advertised(servers: &[acp::McpServer]) -> Option { + servers.iter().find_map(|server| match server { + acp::McpServer::Http(http) if http.name == "nori-client" => Some(http.clone()), + _ => None, + }) +} + +pub(crate) async fn call(server: Option, request: &str) -> String { + match call_inner(server, request).await { + Ok(text) => text, + Err(error) => format!("Delegation tool error: {error}"), + } +} + +async fn call_inner( + server: Option, + request: &str, +) -> Result> { + let server = server.ok_or("nori-client MCP unavailable")?; + let (agent, task) = request.split_once(' ').ok_or("expected agent and task")?; + let bearer = server + .headers + .iter() + .find(|header| header.name.eq_ignore_ascii_case("authorization")) + .and_then(|header| header.value.strip_prefix("Bearer ")) + .ok_or("missing MCP authorization")?; + let transport = StreamableHttpClientTransport::from_config( + StreamableHttpClientTransportConfig::with_uri(server.url).auth_header(bearer.to_owned()), + ); + let client = ().serve(transport).await?; + let result = client + .call_tool( + CallToolRequestParams::new("delegate").with_arguments( + serde_json::json!({"agent":agent,"task":task}) + .as_object() + .unwrap() + .clone(), + ), + ) + .await?; + let text = result + .content + .iter() + .filter_map(|content| content.as_text()) + .map(|text| text.text.clone()) + .collect::>() + .join("\n"); + client.cancel().await?; + Ok(text) +} diff --git a/nori-rs/mock-acp-agent/src/main.rs b/nori-rs/mock-acp-agent/src/main.rs index ae4d62fc8..17b054c78 100644 --- a/nori-rs/mock-acp-agent/src/main.rs +++ b/nori-rs/mock-acp-agent/src/main.rs @@ -2,6 +2,7 @@ mod browser_modify; mod context_usage; +mod delegation; mod runaway_search; use std::collections::HashMap; @@ -107,6 +108,7 @@ struct MockState { context_prompt_index: AtomicI64, cancel_requested: AtomicBool, session_configs: Mutex>, + delegation_servers: Mutex>, } #[derive(Clone)] @@ -299,6 +301,29 @@ impl MockAgent { arguments: acp::PromptRequest, ) -> Result { eprintln!("Mock agent: prompt"); + if let Some(prompt) = last_prompt_text_block(&arguments.prompt) { + if prompt.starts_with("[Nori delegation result]") { + self.send_text_chunk( + arguments.session_id.clone(), + &format!("Delegation continuation received: {prompt}"), + ) + .await?; + return Ok(acp::PromptResponse::new(acp::StopReason::EndTurn)); + } + if let Some(request) = prompt.strip_prefix("mock:delegate ") { + let server = self + .state + .delegation_servers + .lock() + .unwrap() + .get(&arguments.session_id.to_string()) + .cloned(); + let output = delegation::call(server, request).await; + self.send_text_chunk(arguments.session_id.clone(), &output) + .await?; + return Ok(acp::PromptResponse::new(acp::StopReason::EndTurn)); + } + } if let Ok(expected) = std::env::var("MOCK_AGENT_EXPECT_LAST_PROMPT_TEXT_BLOCK") { let actual = last_prompt_text_block(&arguments.prompt); if actual != Some(expected.as_str()) { @@ -1734,6 +1759,10 @@ async fn main() -> acp::Result<()> { } } let session_key = session_id.to_string(); + if let Some(server) = delegation::advertised(&arguments.mcp_servers) { + state.delegation_servers.lock().unwrap().insert(session_key.clone(), server); + } + let session_config = default_session_config(); state .session_configs @@ -1787,6 +1816,9 @@ async fn main() -> acp::Result<()> { async move |arguments: acp::LoadSessionRequest, responder: Responder, cx: ConnectionTo| { + if let Some(server) = delegation::advertised(&arguments.mcp_servers) { + state.delegation_servers.lock().unwrap().insert(arguments.session_id.to_string(), server); + } let session_config = state .session_configs .lock() diff --git a/nori-rs/nori-protocol/src/session_event.rs b/nori-rs/nori-protocol/src/session_event.rs index 0c89d0d9d..a07e771e7 100644 --- a/nori-rs/nori-protocol/src/session_event.rs +++ b/nori-rs/nori-protocol/src/session_event.rs @@ -57,6 +57,7 @@ pub enum NoriEvent { Notice(Notice), RequestFailed(RequestFailure), RequestResolved(RequestResolved), + DelegationChanged(Delegation), } /// Persistent and agent-side identities established for a harness session. @@ -260,3 +261,28 @@ pub enum RequestFailureKind { Retryable, Fatal, } + +/// Manager-owned delegation state, attributed to both participating sessions. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct Delegation { + pub request_id: String, + pub parent_id: String, + pub child_id: String, + pub parent_agent: String, + pub agent: String, + pub task: String, + pub status: DelegationStatus, + pub depth: i32, + pub result: Option, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum DelegationStatus { + AwaitingApproval, + Running, + Declined, + Completed, + Failed, + Cancelled, +} diff --git a/nori-rs/tui-pty-e2e/tests/delegation.rs b/nori-rs/tui-pty-e2e/tests/delegation.rs new file mode 100644 index 000000000..55a68b336 --- /dev/null +++ b/nori-rs/tui-pty-e2e/tests/delegation.rs @@ -0,0 +1,177 @@ +//! Delegation traverses the real manager, child process, and terminal. +use std::time::Duration; +use tui_pty_e2e::Key; +use tui_pty_e2e::SessionConfig; +use tui_pty_e2e::TIMEOUT; +use tui_pty_e2e::TuiSession; + +fn spawn(enabled: bool) -> TuiSession { + let config = SessionConfig::new() + .with_extra_config_toml(format!("[features]\nmulti_session = {enabled}\n")) + .with_agent_env("MOCK_AGENT_RESPONSE_MOCK_MODEL", "parent final answer") + .with_agent_env("MOCK_AGENT_RESPONSE_MOCK_MODEL_ALT", "child final answer"); + let mut session = TuiSession::spawn_with_config(40, 130, config).unwrap(); + session + .wait_for_text("Mock Default Model", TIMEOUT) + .unwrap(); + session.submit_input("parent task identity").unwrap(); + session + .wait_for_text("parent final answer", TIMEOUT) + .unwrap(); + session +} + +#[test] +fn same_kind_child_starts_and_returns_without_approval_or_exiting_terminal() { + let mut session = spawn(true); + session + .submit_input("/delegate mock-model inspect the parser") + .unwrap(); + session + .wait_for( + |screen| screen.contains("completed") || screen.contains("Completed"), + TIMEOUT, + ) + .unwrap(); + assert!( + !session + .screen_contents() + .contains("Start delegated session") + ); + session.submit_input("/sessions").unwrap(); + session + .wait_for_text("inspect the parser", TIMEOUT) + .unwrap(); + session.send_key(Key::Down).unwrap(); + session.send_key(Key::Enter).unwrap(); + session + .wait_for( + |screen| { + screen.contains("inspect the parser") && !screen.contains("parent task identity") + }, + TIMEOUT, + ) + .unwrap(); + session + .wait_for_text("parent final answer", TIMEOUT) + .unwrap(); + session.send_key(Key::Alt(',')).unwrap(); + session + .wait_for_text("parent task identity", TIMEOUT) + .unwrap(); + let before = session + .history_contents() + .matches("parent final answer") + .count(); + session.submit_input("still usable").unwrap(); + let deadline = std::time::Instant::now() + TIMEOUT; + while session + .history_contents() + .matches("parent final answer") + .count() + <= before + { + assert!( + std::time::Instant::now() < deadline, + "parent must answer after child completion" + ); + session.settle(Duration::from_millis(50)); + } +} + +#[test] +fn different_kind_child_needs_start_and_its_permission_stays_in_child() { + let mut session = spawn(true); + session + .submit_input("/delegate mock-model-alt mock:request-permission:write-child-file") + .unwrap(); + session + .wait_for_text("Start delegated session", TIMEOUT) + .unwrap(); + assert!( + !session + .screen_contents() + .contains("Would you like to allow") + ); + session.send_key(Key::Char('1')).unwrap(); + session + .wait_for( + |screen| screen.contains("waiting on you") && screen.contains("parent"), + TIMEOUT, + ) + .unwrap(); + assert!( + !session + .screen_contents() + .contains("Would you like to allow"), + "child permission must not open in parent" + ); + session.send_key(Key::AltDown).unwrap(); + session + .wait_for_text("Would you like to allow", TIMEOUT) + .unwrap(); + session.wait_for_text("write-child-file", TIMEOUT).unwrap(); + session.send_key(Key::Char('1')).unwrap(); + session + .wait_for_text("Permission granted with option: allow", TIMEOUT) + .unwrap(); + session.send_key(Key::Alt(',')).unwrap(); + session + .wait_for( + |screen| screen.contains("completed") || screen.contains("Completed"), + TIMEOUT, + ) + .unwrap(); + session + .wait_for( + |screen| { + screen.contains("parent task identity") + && screen.contains("Permission granted with option: allow") + }, + TIMEOUT, + ) + .expect("attributed result is visible in parent after child redraw"); +} + +#[test] +fn declining_different_kind_does_not_start_child() { + let mut session = spawn(true); + session + .submit_input("/delegate mock-model-alt inspect parser without starting a child") + .unwrap(); + session + .wait_for_text("Start delegated session", TIMEOUT) + .unwrap(); + session.send_key(Key::Char('2')).unwrap(); + session + .wait_for_text("The user declined this delegation.", TIMEOUT) + .unwrap(); + session + .wait_for_text("Delegation continuation received:", TIMEOUT) + .unwrap(); + session + .wait_for( + |screen| !screen.contains("Working") && !screen.contains("Start delegated session"), + TIMEOUT, + ) + .unwrap(); + session.submit_input("/sessions").unwrap(); + assert!(!session.screen_contents().contains("child final answer")); + session.wait_for_text("Sessions 1", TIMEOUT).unwrap(); +} + +#[test] +fn delegation_is_unavailable_without_multi_session() { + let mut session = spawn(false); + session + .submit_input("/delegate mock-model-alt should not run") + .unwrap(); + session + .wait_for_text("Unrecognized command '/delegate'", TIMEOUT) + .unwrap(); + assert!( + !session + .screen_contents() + .contains("Start delegated session") + ); +} diff --git a/nori-rs/tui-pty-e2e/tests/host_attach.rs b/nori-rs/tui-pty-e2e/tests/host_attach.rs index d933b1669..4fa40fa4f 100644 --- a/nori-rs/tui-pty-e2e/tests/host_attach.rs +++ b/nori-rs/tui-pty-e2e/tests/host_attach.rs @@ -792,3 +792,83 @@ fn host_list_truncates_tty_title_without_losing_piped_title() { assert!(row.trim_end().ends_with('…'), "{screen}"); assert!(!screen.contains("continuation text"), "{screen}"); } + +#[test] +fn delegated_child_in_host_list_preserves_metadata_columns_and_can_be_opened() { + let host = Host::new(); + host.start(); + let mut client = host.create_session(); + ask(&mut client, "parent host task"); + client + .submit_input("/delegate mock-model mock:request-permission:child-column-check") + .unwrap(); + let deadline = Instant::now() + TIMEOUT; + let listing = loop { + let listing = host.list(); + if listing.lines().count() >= 3 && listing.contains("waiting on you") { + break listing; + } + assert!( + Instant::now() < deadline, + "child must start: {listing}\n{}", + client.screen_contents() + ); + client.settle(Duration::from_millis(100)); + }; + let rows: Vec<_> = listing.lines().skip(1).collect(); + assert_eq!(rows.len(), 2, "{listing}"); + for row in &rows { + let id = row.get(..36).unwrap(); + assert_eq!(id.len(), 36, "{listing}"); + assert!( + !id.starts_with(' '), + "child hierarchy must stay in TITLE: {listing}" + ); + assert_eq!(row.get(38..54).unwrap().trim(), "mock-model", "{listing}"); + assert!( + ["ready", "waiting on you", "working"].contains(&row.get(56..72).unwrap().trim()), + "STATUS must keep its column: {listing}" + ); + } + assert!( + rows.iter() + .any(|row| row.get(82..).unwrap().contains("child-column-check")), + "{listing}" + ); + client.wait_for_text("needs your answer", TIMEOUT).unwrap(); + let parent_screen = client.screen_contents(); + for row in &rows { + assert!( + parent_screen.contains(row.get(..36).unwrap()), + "notice names parent and child: {parent_screen}" + ); + } + client.send_key(Key::AltDown).unwrap(); + client.wait_for_text("child-column-check", TIMEOUT).unwrap(); + client.wait_for_text("Allow", TIMEOUT).unwrap(); + client.send_key(Key::Char('1')).unwrap(); + client + .wait_for_text("Permission granted with option: allow", TIMEOUT) + .unwrap(); +} + +#[test] +fn watcher_cannot_delegate_a_child() { + let host = Host::new(); + host.start(); + let mut controller = host.create_session(); + ask(&mut controller, "delegation ownership"); + let id = host.session_id(); + let mut watcher = host.attach(Some(&id)); + wait_watching(&mut watcher); + let before = refusal_count(&mut watcher); + watcher + .submit_input("/delegate mock-model watcher forbidden child") + .unwrap(); + wait_refusal(&mut watcher, before); + assert_eq!( + host.list().lines().count(), + 2, + "watcher cannot create a child" + ); +} diff --git a/nori-rs/tui-pty-e2e/tests/session_list.rs b/nori-rs/tui-pty-e2e/tests/session_list.rs index 1bdbd1453..733e43321 100644 --- a/nori-rs/tui-pty-e2e/tests/session_list.rs +++ b/nori-rs/tui-pty-e2e/tests/session_list.rs @@ -352,6 +352,18 @@ fn a_fork_is_listed_under_its_parent_and_its_request_marks_the_parent() { parent_row.contains("waiting on you"), "the child's request marks the parent row:\n{screen}" ); + let request_line = screen + .lines() + .find(|line| line.contains("is waiting on you")) + .unwrap(); + assert!( + request_line.contains("to jump"), + "the attributed request stays on one row: {screen}" + ); + let screen = regex::Regex::new(r"\(parent [0-9a-f]{8}\)") + .unwrap() + .replace_all(&screen, "(parent PARENTID)") + .into_owned(); insta::assert_snapshot!( "session_list_fork_under_parent", normalize_for_input_snapshot(screen) diff --git a/nori-rs/tui-pty-e2e/tests/snapshots/session_list__session_list_fork_under_parent.snap b/nori-rs/tui-pty-e2e/tests/snapshots/session_list__session_list_fork_under_parent.snap index e3b255fa1..cd1ded8b7 100644 --- a/nori-rs/tui-pty-e2e/tests/snapshots/session_list__session_list_fork_under_parent.snap +++ b/nori-rs/tui-pty-e2e/tests/snapshots/session_list__session_list_fork_under_parent.snap @@ -8,7 +8,7 @@ expression: normalize_for_input_snapshot(screen) • alpha agent answer - ● mock:request-permission… is waiting on you · approve: rm -rf target/debug · alt + down to jump + ● (parent PARENTID) mock:… is waiting on you · approve: rm -rf target/debug · alt + down to jump 1 waiting on you · 1 ready Sessions 2 · 1 waiting on you · 1 ready tab: waiting only · type to search diff --git a/nori-rs/tui/docs.md b/nori-rs/tui/docs.md index 31a170023..9bddf45cf 100644 --- a/nori-rs/tui/docs.md +++ b/nori-rs/tui/docs.md @@ -952,7 +952,14 @@ in the prompt still moves the cursor. tone, and its glyph, follow its **session family** status, so a parent reads "waiting on you" when a child does. Manager sessions that descend from a listed session but have no TUI view are listed under their parent with negative keys that - cannot collide with a generation; they are not navigation targets. + cannot collide with a generation; they are not navigation targets. Delegated + children are adopted as real parked widgets by + [`app/delegation.rs`](src/app/delegation.rs), using atomic replay/live + subscription to populate their history; those children are navigation + targets, including when their task completed before the first draw. The + view follows the child process across branch-at-head conversation changes + while filtering unrelated conversations, so a fork does not strand its + permission UI or recreate an already-adopted child. - `App::refresh_session_overview` runs before every draw. While the list is open it replaces the rows in place, keeping the selection on the same session. In cloud mode every session's agent reads "Nori Cloud". @@ -967,9 +974,9 @@ while a view is open: - a background-request line, naming the first off-screen session waiting on you by title (` is waiting on you`), its request, `+N more` when - others wait, and the `next_waiting_session` key. The title and request text - are cut to short fixed lengths so the line fits one row of a 100-column - terminal; + others wait, and the `next_waiting_session` key. The title, including any + parent attribution prefix, shares one bounded label budget; the request text + is separately shortened so the line fits one row of a 100-column terminal; - a status line counting sessions by their own status, most urgent first (waiting on you, failed, working, starting, ready; closed is omitted), shown only with two or more sessions. @@ -1039,6 +1046,38 @@ The ignored `jump_timing_on_a_long_history` PTY test measures a jump to a long history; the reprint is bounded by the same cell and row caps as resize reflow. +#### Delegation + +`/delegate <agent> <task>` is the user entry point in the local and attached +frontends. It is enabled only with `multi_session`; the session list does not +start delegations. The shared [parser and labels](src/delegation.rs) keep +syntax and parent/child attribution consistent. The +[chat-widget adapter](src/chatwidget/delegation.rs) calls the harness manager +and renders its `DelegationChanged` events. Different-kind requests expose +Start and Decline; same-kind requests start immediately. These approval choices +control child creation, separately from a child's later ACP permission request. + +Children receive navigable parked widgets without replacing the parent on +screen. Their stream, history, pending permissions, and session-scoped events +follow the normal session-deck path. Alt+Down (`next_waiting_session`) opens the +next session needing the user, where the ordinary permission UI answers the +child. Parent notices identify both child and parent and do not open the +child's permission popup in the parent. Child list labels put the parent +identity before the task title so title truncation preserves attribution. Closed completed children keep +inspectable history; ending their process must not be interpreted as exiting +Nori. + +Result delivery is manager-owned and automatically prompts the parent. The +terminal renders attributed status/result text; it neither approves the result +nor drives completion. Explicit parent close cancels descendants; a crashed +parent can be resumed with Ctrl+R under the same conversation ID to receive +held results, including redelivery when the previous parent process admitted +but never acknowledged a result turn. See the [manager lifecycle](../harness/docs.md#delegation-between-managed-sessions). + +The initial rendering is intentionally text and existing selection components. +Dedicated delegation/result cards and further hierarchy styling belong to the +later presentation workstream. + #### Host attachment [`attach`](src/attach/docs.md) is the dedicated frontend for `nori attach`, diff --git a/nori-rs/tui/src/app/delegation.rs b/nori-rs/tui/src/app/delegation.rs new file mode 100644 index 000000000..1ca01178f --- /dev/null +++ b/nori-rs/tui/src/app/delegation.rs @@ -0,0 +1,77 @@ +use super::session_deck::ParkedSession; +use super::*; + +impl App { + /// Adopt manager-started children without replacing the parent's widget. + pub(super) fn adopt_delegated_sessions(&mut self, tui: &crate::tui::Tui) { + if !self.config.multi_session { + return; + } + let known: std::collections::HashSet<_> = self + .session_facts() + .into_iter() + .filter_map(|fact| fact.conversation_id) + .collect(); + for record in self.session_manager.list() { + if record.source != nori_harness::session_manager::SessionSource::Session + || known.contains(&record.conversation_id) + || self.adopted_delegations.contains(&record.conversation_id) + { + continue; + } + // Follow forks of this process while skipping unrelated histories. + let mut cursors = self.session_manager.replay_cursors(); + let records = self.session_manager.list(); + let mut lineage = std::collections::HashSet::from([record.conversation_id]); + loop { + let before = lineage.len(); + for descendant in &records { + if descendant.source == nori_harness::session_manager::SessionSource::Fork + && descendant + .parent + .is_some_and(|parent| lineage.contains(&parent)) + { + lineage.insert(descendant.conversation_id); + } + } + if lineage.len() == before { + break; + } + } + for id in lineage { + cursors.insert(id.to_string(), 0); + } + let subscription = match self.session_manager.subscribe_from(None, &cursors, 4096) { + Ok(subscription) => subscription, + Err(error) => { + tracing::warn!(%error, "cannot replay delegated session"); + continue; + } + }; + let mut init = self.chat_widget_init( + tui.frame_requester(), + None, + Vec::new(), + Some(record.agent.clone()), + true, + None, + ); + init.config = self.config_for_agent(&record.agent); + init.config.cwd = record.cwd; + let widget = ChatWidget::from_managed_view( + init, + record.conversation_id, + self.session_manager.handle(&record.conversation_id), + subscription, + ); + self.adopted_delegations.insert(record.conversation_id); + self.session_deck.park( + widget.session_generation(), + ParkedSession { + widget: Box::new(widget), + transcript: Vec::new(), + }, + ); + } + } +} diff --git a/nori-rs/tui/src/app/event_handling.rs b/nori-rs/tui/src/app/event_handling.rs index 136fcb395..a8dee735c 100644 --- a/nori-rs/tui/src/app/event_handling.rs +++ b/nori-rs/tui/src/app/event_handling.rs @@ -109,6 +109,7 @@ impl App { self.chat_widget.handle_paste(pasted); } TuiEvent::Draw => { + self.adopt_delegated_sessions(tui); self.refresh_session_overview(); self.chat_widget.pre_draw_tick(); self.chat_widget.maybe_post_pending_notification(tui); @@ -665,6 +666,33 @@ impl App { AppEvent::ConsolidateAgentMessage { source, cwd } => { self.consolidate_active_agent_message(tui, source, cwd); } + AppEvent::ResolveDelegation { + parent, + request_id, + approved, + } => { + let manager = self.session_manager.clone(); + let tx = self.app_event_tx.clone(); + let generation = self + .session_facts() + .into_iter() + .find(|fact| fact.conversation_id == Some(parent)) + .map(|fact| fact.generation); + tokio::spawn(async move { + if let Err(error) = manager + .resolve_delegation(&parent, &request_id, approved) + .await + { + let tx = generation + .map_or_else(|| tx.clone(), |generation| tx.for_session(generation)); + tx.send(AppEvent::InsertHistoryCell(Box::new( + crate::history_cell::new_error_event(format!( + "Delegation failed: {error}" + )), + ))); + } + }); + } AppEvent::OpenSessionList => { self.open_session_list(); tui.frame_requester().schedule_frame(); diff --git a/nori-rs/tui/src/app/mod.rs b/nori-rs/tui/src/app/mod.rs index 6c6732dbf..9a6ff795d 100644 --- a/nori-rs/tui/src/app/mod.rs +++ b/nori-rs/tui/src/app/mod.rs @@ -104,6 +104,7 @@ pub(crate) struct App { /// Every session this TUI holds, in navigation order. Sessions that are /// not on screen are parked here with their own widget and transcript. session_deck: session_deck::SessionDeck<Box<ChatWidget>>, + adopted_delegations: std::collections::HashSet<ConversationId>, /// The active session's transcript, taken when that session is about to be /// parked behind a new one. Set only with `[features] multi_session`. retiring_transcript: Option<Vec<Arc<dyn HistoryCell>>>, @@ -242,6 +243,7 @@ struct SystemInfoRefreshRequest { } mod config_persistence; +mod delegation; mod event_handling; mod resize_reflow; mod session_deck; @@ -360,6 +362,7 @@ impl App { enhanced_keys_supported, transcript_cells: Vec::new(), session_deck: Default::default(), + adopted_delegations: Default::default(), retiring_transcript: None, session_reprint_pending: false, unseen_finishes: Default::default(), diff --git a/nori-rs/tui/src/app/session_overview.rs b/nori-rs/tui/src/app/session_overview.rs index fa4dcd690..278db22f4 100644 --- a/nori-rs/tui/src/app/session_overview.rs +++ b/nori-rs/tui/src/app/session_overview.rs @@ -162,6 +162,7 @@ impl App { let found: Vec<_> = records .iter() .filter(|record| !known.contains(&record.conversation_id)) + .filter(|record| !self.adopted_delegations.contains(&record.conversation_id)) .filter(|record| record.parent.is_some_and(|parent| known.contains(&parent))) .collect(); if found.is_empty() { @@ -188,6 +189,20 @@ impl App { let title = self .session_rows_title(fact.generation) .unwrap_or_else(|| "New session".to_string()); + let title = fact + .conversation_id + .and_then(|id| self.session_manager.get(&id)) + .filter(|record| record.source == nori_harness::session_manager::SessionSource::Session) + .and_then(|record| record.parent) + .map_or_else( + || title.clone(), + |parent| { + format!( + "(parent {}) {title}", + parent.to_string().chars().take(8).collect::<String>() + ) + }, + ); // A parent speaks for its family: if a child needs the user, so does // the parent row. let shown_status = if depth == 0 { @@ -340,6 +355,16 @@ impl App { let title = self .session_title(first.generation, REQUEST_LINE_TITLE_CHARS) .unwrap_or_else(|| "A session".to_string()); + let title = first.parent.map_or_else( + || title.clone(), + |parent| { + format!( + "(parent {}) {title}", + parent.to_string().chars().take(8).collect::<String>() + ) + }, + ); + let title = cut(&title, REQUEST_LINE_TITLE_CHARS); let mut spans = vec![ "● ".cyan(), title.bold(), diff --git a/nori-rs/tui/src/app/session_setup.rs b/nori-rs/tui/src/app/session_setup.rs index 32e4bf5b7..9efd00a2f 100644 --- a/nori-rs/tui/src/app/session_setup.rs +++ b/nori-rs/tui/src/app/session_setup.rs @@ -251,7 +251,10 @@ impl App { /// With `[features] multi_session` a live session keeps running and is /// parked when the next widget is installed; otherwise it shuts down. pub(crate) fn shutdown_current_conversation(&mut self) { - if self.config.multi_session && self.chat_widget.has_harness_session() { + if self.config.multi_session + && (self.chat_widget.has_harness_session() + || self.chat_widget.managed_conversation_id().is_some()) + { self.retiring_transcript = Some(std::mem::take(&mut self.transcript_cells)); } else { self.chat_widget.shutdown_harness_session(); diff --git a/nori-rs/tui/src/app_event.rs b/nori-rs/tui/src/app_event.rs index faf713022..2b53ea112 100644 --- a/nori-rs/tui/src/app_event.rs +++ b/nori-rs/tui/src/app_event.rs @@ -163,6 +163,12 @@ pub(crate) enum AppEvent { /// Open the list of sessions (`[features] multi_session`). OpenSessionList, + ResolveDelegation { + parent: nori_harness::ConversationId, + request_id: String, + approved: bool, + }, + /// Start a failed session again from its transcript, in its place in the /// session list, and bring it on screen. ResumeFailedSession { diff --git a/nori-rs/tui/src/attach/docs.md b/nori-rs/tui/src/attach/docs.md index 62003eba9..2cf427e4b 100644 --- a/nori-rs/tui/src/attach/docs.md +++ b/nori-rs/tui/src/attach/docs.md @@ -54,6 +54,23 @@ Path: @/nori-rs/tui/src/attach permissions but must take control before sending mutations. The current session marker appears in TITLE, preserving the short ID in its own column. +- Delegation uses the shared [command parser](../delegation.rs): + `/delegate <agent> <task>` sends the selected parent to the manager's remote + delegate method. Different-kind Start/Decline decisions resolve the manager + request; watchers must take control before either operation. Session lists + carry pending delegation state so reconnecting terminals can recover the + approval view without resending a mutation. +- Child permissions stay in the child's conversation. The configured + `next_waiting_session` binding (Alt+Down by default) navigates there. If the + cached list has no waiting target, a connected terminal retains the navigation + action while refreshing the list, then selects a newly discovered waiting + session from the response. List errors or disconnection clear that pending + action. Parent notices and child list rows identify the child and its parent. + Hierarchy labels prefix TITLE so parent attribution survives long task titles while + metadata columns keep their positions. + Results return through the parent's ordinary event stream without a terminal + review step. + ### Things to Know - [Input routing](input.rs) honors `hotkeys.open_session_list`, also opens the diff --git a/nori-rs/tui/src/attach/input.rs b/nori-rs/tui/src/attach/input.rs index 98a9b6b42..cde9b58c8 100644 --- a/nori-rs/tui/src/attach/input.rs +++ b/nori-rs/tui/src/attach/input.rs @@ -21,6 +21,12 @@ impl Runtime { if key.code == KeyCode::Char('d') && key.modifiers.contains(KeyModifiers::CONTROL) { return false; } + if self.composer.is_empty() + && crate::nori::hotkey_match::matches_binding(&self.next_waiting_binding, &key) + { + self.next_waiting_session(true); + return true; + } if crate::nori::hotkey_match::matches_binding(&self.session_list_binding, &key) { self.state.list_open = !self.state.list_open; self.refresh_list(); @@ -76,6 +82,9 @@ impl Runtime { if self.state.list_open && self.picker_key(key) { return true; } + if self.composer.is_empty() && !self.state.list_open && self.delegation_key(key) { + return true; + } if self.composer.is_empty() && !self.state.list_open && self.permission_key(key) { return true; } @@ -104,6 +113,66 @@ impl Runtime { true } + pub(super) fn next_waiting_session(&mut self, refresh_if_missing: bool) { + let rows = &self.state.sessions; + let start = rows + .iter() + .position(|row| Some(&row.session_id) == self.state.selected.as_ref()) + .unwrap_or(rows.len().saturating_sub(1)); + let target = (1..=rows.len()) + .map(|offset| &rows[(start + offset) % rows.len()]) + .find(|row| { + row.status == nori_protocol::SessionStatus::WaitingOnYou + && Some(&row.session_id) != self.state.selected.as_ref() + }) + .map(|row| row.session_id.clone()); + if let Some(id) = target { + self.state.selected = Some(id); + self.state.list_open = false; + self.state.scroll = 0; + self.state.notice = None; + self.state.replay_error = None; + self.subscribe_selected(); + } else if refresh_if_missing && self.state.connected { + self.next_waiting_after_refresh = true; + self.refresh_list(); + } + } + + fn delegation_key(&mut self, key: KeyEvent) -> bool { + let Some(id) = self.state.selected.clone() else { + return false; + }; + let Some(delegation) = self + .state + .conversations + .get(&id) + .and_then(|conversation| { + conversation.delegations.iter().find(|delegation| { + delegation.status == nori_protocol::DelegationStatus::AwaitingApproval + }) + }) + .cloned() + else { + return false; + }; + let approved = match (key.code, key.modifiers) { + (KeyCode::Char('1'), KeyModifiers::NONE) | (KeyCode::Enter, KeyModifiers::NONE) => true, + (KeyCode::Char('2'), KeyModifiers::NONE) => false, + _ => return false, + }; + if let Err(error) = self.require_mutation() { + self.state.show_notice(error.to_string()); + return true; + } + self.send( + "_nori/session/delegation/resolve", + json!({"session_id":id,"request_id":delegation.request_id,"approved":approved}), + Pending::Mutation { draft: None }, + ); + true + } + fn picker_key(&mut self, key: KeyEvent) -> bool { let action = match (key.code, key.modifiers) { (KeyCode::Char('f'), KeyModifiers::CONTROL) => PickerAction::ActivateSearch, @@ -230,6 +299,17 @@ impl Runtime { .show_notice("Choose a session with /sessions or start one with /new."); return true; }; + if let Some(command) = crate::delegation::parse_command(command) { + match command { + Ok((agent, task)) => self.send( + "_nori/session/delegate", + json!({"session_id":id,"agent":agent,"task":task}), + Pending::Mutation { draft: Some(text) }, + ), + Err(message) => self.state.show_notice(message), + } + return true; + } let method = match command { "/close" => Some("_nori/session/close"), "/cancel" => Some("_nori/session/cancel"), diff --git a/nori-rs/tui/src/attach/mod.rs b/nori-rs/tui/src/attach/mod.rs index 51f160ece..ce0f41287 100644 --- a/nori-rs/tui/src/attach/mod.rs +++ b/nori-rs/tui/src/attach/mod.rs @@ -36,7 +36,7 @@ pub async fn run_attach( let mut tui = crate::tui::Tui::new(crate::tui::init()?); let result = async { tui.enter_alt_screen()?; - run(&mut tui, options, initial, config.hotkeys.open_session_list).await + run(&mut tui, options, initial, config.hotkeys).await } .await; let leave = tui.leave_alt_screen(); @@ -51,12 +51,13 @@ async fn run( tui: &mut crate::tui::Tui, options: AttachOptions, initial: ManagedClient, - session_list_binding: nori_config::HotkeyBinding, + hotkeys: nori_config::HotkeyConfig, ) -> anyhow::Result<()> { let (request_tx, request_rx) = tokio::sync::mpsc::unbounded_channel(); let (event_tx, mut event_rx) = tokio::sync::mpsc::unbounded_channel(); let mut runtime = runtime::Runtime::new(options.session_id.clone(), request_tx); - runtime.set_session_list_binding(session_list_binding); + runtime.set_session_list_binding(hotkeys.open_session_list); + runtime.set_next_waiting_binding(hotkeys.next_waiting_session); let connection = tokio::spawn(connection::drive(options, initial, request_rx, event_tx)); let mut events = tui.event_stream(); let mut refresh = tokio::time::interval(std::time::Duration::from_secs(1)); diff --git a/nori-rs/tui/src/attach/render.rs b/nori-rs/tui/src/attach/render.rs index f626a443f..1b78ae36c 100644 --- a/nori-rs/tui/src/attach/render.rs +++ b/nori-rs/tui/src/attach/render.rs @@ -144,6 +144,32 @@ fn render_conversation(state: &AttachState, id: &str, area: Rect, buf: &mut Buff Paragraph::new(" Waiting for session history…").render(area, buf); return; }; + let delegation = conversation + .delegations + .iter() + .find(|delegation| delegation.status == nori_protocol::DelegationStatus::AwaitingApproval); + let delegation_lines = delegation + .map(|delegation| { + crate::wrapping::word_wrap_lines( + [ + Line::from("Start delegated session?".bold()), + Line::from(format!( + "Child {} ({}) · parent {} ({})", + delegation.child_id, + delegation.agent, + delegation.parent_id, + delegation.parent_agent + )), + Line::from(delegation.task.clone()), + Line::from("1 Start · 2 Decline".green()), + ], + usize::from(area.width.saturating_sub(2)), + ) + }) + .unwrap_or_default(); + let delegation_height = u16::try_from(delegation_lines.len()) + .unwrap_or(u16::MAX) + .min(area.height); let permission = conversation.permissions.first(); let permission_height = permission.map_or(0, |request| { u16::try_from(request.options.len() + 4) @@ -154,7 +180,8 @@ fn render_conversation(state: &AttachState, id: &str, area: Rect, buf: &mut Buff area.x + 1, area.y, area.width.saturating_sub(2), - area.height.saturating_sub(permission_height), + area.height + .saturating_sub(permission_height + delegation_height), ); let mut lines = Vec::new(); for part in &conversation.parts { @@ -215,6 +242,18 @@ fn render_conversation(state: &AttachState, id: &str, area: Rect, buf: &mut Buff .collect::<Vec<_>>(), ) .render(history_area, buf); + if delegation_height > 0 { + Paragraph::new(delegation_lines).render( + Rect::new( + area.x + 1, + area.bottom() + .saturating_sub(permission_height + delegation_height), + area.width.saturating_sub(2), + delegation_height, + ), + buf, + ); + } if let Some(permission) = permission { let mut rows = vec![Line::from( format!("Permission · {}", permission.title).bold(), diff --git a/nori-rs/tui/src/attach/runtime.rs b/nori-rs/tui/src/attach/runtime.rs index 5dfd29cfa..f201de805 100644 --- a/nori-rs/tui/src/attach/runtime.rs +++ b/nori-rs/tui/src/attach/runtime.rs @@ -26,9 +26,22 @@ pub(super) struct Runtime { next_id: i64, uncertain_delivery: bool, pub session_list_binding: nori_config::HotkeyBinding, + pub next_waiting_binding: nori_config::HotkeyBinding, + pub(super) next_waiting_after_refresh: bool, } impl Runtime { + pub fn set_next_waiting_binding(&mut self, binding: nori_config::HotkeyBinding) { + self.next_waiting_binding = binding; + self.composer.set_hint_items(vec![ + (self.session_list_binding.as_str(), "sessions"), + (self.next_waiting_binding.as_str(), "next waiting"), + ("alt+t", "take control"), + ("ctrl+d", "detach"), + ("/delegate", "delegate task"), + ]); + } + pub fn set_session_list_binding(&mut self, binding: nori_config::HotkeyBinding) { self.session_list_binding = binding; self.composer.set_hint_items(vec![ @@ -59,6 +72,8 @@ impl Runtime { next_id: 0, uncertain_delivery: false, session_list_binding: nori_config::HotkeyConfig::default().open_session_list, + next_waiting_binding: nori_config::HotkeyConfig::default().next_waiting_session, + next_waiting_after_refresh: false, } } @@ -105,6 +120,7 @@ impl Runtime { match event { ConnectionEvent::Stopped(reason) => { self.state.connected = false; + self.next_waiting_after_refresh = false; self.state.stopped = Some(reason); self.pending.clear(); } @@ -113,6 +129,7 @@ impl Runtime { } ConnectionEvent::Disconnected(reason) => { self.state.connected = false; + self.next_waiting_after_refresh = false; let uncertain = self .pending .values() @@ -161,12 +178,16 @@ impl Runtime { return Ok(()); }; if let Some(error) = message.get("error") { + if matches!(pending, Pending::List) { + self.next_waiting_after_refresh = false; + } let text = error["message"] .as_str() .unwrap_or("Host request failed") .to_string(); if matches!(pending, Pending::Initialize) { self.state.connected = false; + self.next_waiting_after_refresh = false; self.state.stopped = Some(format!( "Host initialization failed: {text}. Re-run nori attach." )); @@ -200,6 +221,9 @@ impl Runtime { } Pending::List => { self.state.replace_sessions(result.clone())?; + if std::mem::take(&mut self.next_waiting_after_refresh) { + self.next_waiting_session(false); + } if let Some(id) = &self.state.selected && !self.state.sessions.iter().any(|row| &row.session_id == id) { diff --git a/nori-rs/tui/src/attach/snapshots/nori_tui__attach__tests__delegation_approval_watcher.snap b/nori-rs/tui/src/attach/snapshots/nori_tui__attach__tests__delegation_approval_watcher.snap new file mode 100644 index 000000000..4514a9fdb --- /dev/null +++ b/nori-rs/tui/src/attach/snapshots/nori_tui__attach__tests__delegation_approval_watcher.snap @@ -0,0 +1,21 @@ +--- +source: tui/src/attach/tests.rs +expression: rendered +--- + Nori host WATCHING · read-only · alt+t take control + session · parent-session · + + Child child-session (mock-model-alt) · parent parent-session (mock-model) · awaiting + approval + + + + + + + Start delegated session? + Child child-session (mock-model-alt) · parent parent-session (mock-model) + Review the parser + 1 Start · 2 Decline + +› Send a prompt diff --git a/nori-rs/tui/src/attach/snapshots/nori_tui__attach__tests__delegation_child_list.snap b/nori-rs/tui/src/attach/snapshots/nori_tui__attach__tests__delegation_child_list.snap new file mode 100644 index 000000000..c5b54ab19 --- /dev/null +++ b/nori-rs/tui/src/attach/snapshots/nori_tui__attach__tests__delegation_child_list.snap @@ -0,0 +1,21 @@ +--- +source: tui/src/attach/tests.rs +expression: rendered +--- + Nori host WATCHING · read-only · alt+t take control + mock-model · parent-session · ready + + + Sessions 2 + + ID AGENT STATUS AGE TITLE + ▏ parent-s mock-model ready 0s (current) Parent task ▕ + child-se mock-model-alt waiting on you 0s ↳ (parent parent-s) Child task + + + + + ↑↓ move enter open ctrl+r resume ctrl+f search esc back + + +› Send a prompt diff --git a/nori-rs/tui/src/attach/snapshots/nori_tui__attach__tests__delegation_result.snap b/nori-rs/tui/src/attach/snapshots/nori_tui__attach__tests__delegation_result.snap new file mode 100644 index 000000000..c32e1e4ab --- /dev/null +++ b/nori-rs/tui/src/attach/snapshots/nori_tui__attach__tests__delegation_result.snap @@ -0,0 +1,21 @@ +--- +source: tui/src/attach/tests.rs +expression: rendered +--- + Nori host WATCHING · read-only · alt+t take control + session · parent-session · + + Child child-session (mock-model-alt) · parent parent-session (mock-model) · awaiting + approval + + Child child-session (mock-model-alt) · parent parent-session (mock-model) · completed + Parser reviewed + + + + + + + + +› Send a prompt diff --git a/nori-rs/tui/src/attach/state.rs b/nori-rs/tui/src/attach/state.rs index a8af1320b..a5f8cc9cc 100644 --- a/nori-rs/tui/src/attach/state.rs +++ b/nori-rs/tui/src/attach/state.rs @@ -32,6 +32,12 @@ pub(super) struct SessionRow { pub title: Option<String>, #[serde(default)] pub age_seconds: i64, + #[serde(default)] + pub parent: Option<String>, + #[serde(default)] + pub depth: i32, + #[serde(default)] + pub delegations: Vec<nori_protocol::Delegation>, } pub(super) enum Part { @@ -56,6 +62,7 @@ pub(super) struct Permission { pub(super) struct Conversation { pub parts: Vec<Part>, pub permissions: Vec<Permission>, + pub delegations: Vec<nori_protocol::Delegation>, pub cwd: std::path::PathBuf, normalizer: ClientEventNormalizer, message_boundary: bool, @@ -109,6 +116,17 @@ impl Default for AttachState { impl AttachState { pub fn replace_sessions(&mut self, value: Value) -> anyhow::Result<()> { self.sessions = serde_json::from_value(value["sessions"].clone())?; + for row in &self.sessions { + if let Some(conversation) = self.conversations.get_mut(&row.session_id) { + conversation.delegations.clone_from(&row.delegations); + } else if !row.delegations.is_empty() { + self.conversations + .entry(row.session_id.clone()) + .or_default() + .delegations + .clone_from(&row.delegations); + } + } self.picker.title = format!("Sessions {}", self.sessions.len()); self.picker.replace_items(self.sessions.iter().map(|row| { let seconds = row.age_seconds.max(0); @@ -120,6 +138,18 @@ impl AttachState { format!("{}h", seconds / 3600) }; let title = row.title.clone().unwrap_or_else(|| "New session".into()); + let title = row.parent.as_ref().map_or_else( + || title.clone(), + |parent| { + format!( + "{}↳ (parent {}) {title}", + " ".repeat( + usize::try_from(row.depth.saturating_sub(1).clamp(0, 2)).unwrap_or(0) + ), + parent.chars().take(8).collect::<String>() + ) + }, + ); let title = if self.selected.as_ref() == Some(&row.session_id) { format!("(current) {title}") } else { @@ -270,6 +300,18 @@ impl AttachState { conversation.parts.push(Part::Notice(error.message)); conversation.message_boundary = true; } + SessionEvent::Nori(NoriEvent::DelegationChanged(delegation)) => { + if delegation.parent_id == envelope.conversation_id { + conversation + .delegations + .retain(|current| current.request_id != delegation.request_id); + conversation + .parts + .push(Part::Notice(crate::delegation::summary(&delegation))); + conversation.delegations.push(delegation); + conversation.message_boundary = true; + } + } SessionEvent::Nori(NoriEvent::SessionStarted(started)) => { conversation.cwd = started.cwd } diff --git a/nori-rs/tui/src/attach/tests.rs b/nori-rs/tui/src/attach/tests.rs index d7dd71a0f..68bd159b9 100644 --- a/nori-rs/tui/src/attach/tests.rs +++ b/nori-rs/tui/src/attach/tests.rs @@ -429,3 +429,214 @@ fn stopped_explanation_remains_visible_above_a_tall_draft() { assert!(!composer.is_empty()); insta::assert_snapshot!("attached_host_restarted_with_draft", rendered); } + +fn delegation_event(seq: i64, status: &str) -> SessionEnvelope { + serde_json::from_value(json!({ + "conversation_id":"parent-session", "agent":"mock-model", "seq":seq, + "event":{"source":"nori", "event":{"event_type":"delegation_changed", "event":{ + "request_id":"delegation-one", "parent_id":"parent-session", "child_id":"child-session", + "parent_agent":"mock-model", "agent":"mock-model-alt", "task":"Review the parser", + "status":status, "depth":1, "result":if status == "completed" { Some("Parser reviewed") } else { None } + }}} + })).unwrap() +} + +#[test] +fn delegation_command_targets_selected_parent_and_preserves_task() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); + let mut runtime = super::runtime::Runtime::new(Some("parent-session".into()), tx); + runtime.state.connected = true; + runtime.state.role_changed("controller"); + runtime + .composer + .handle_paste("/delegate mock-model-alt Review the parser carefully".into()); + runtime.key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + let request = rx.try_recv().unwrap(); + assert_eq!(request["method"], "_nori/session/delegate"); + assert_eq!( + request["params"], + json!({"session_id":"parent-session","agent":"mock-model-alt","task":"Review the parser carefully"}) + ); +} + +#[test] +fn delegation_approval_names_both_sessions_and_cannot_be_answered_by_watcher() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + for (key, approved) in [('1', true), ('2', false)] { + let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); + let mut runtime = super::runtime::Runtime::new(Some("parent-session".into()), tx); + runtime.state.connected = true; + runtime + .state + .apply(delegation_event(1, "awaiting_approval")) + .unwrap(); + let rendered = screen(&runtime.state); + assert!( + rendered.contains("child-session") && rendered.contains("parent-session"), + "{rendered}" + ); + assert!( + rendered.contains("Start") && rendered.contains("Decline"), + "{rendered}" + ); + if approved { + insta::assert_snapshot!("delegation_approval_watcher", rendered); + } + runtime.key(KeyEvent::new(KeyCode::Char(key), KeyModifiers::NONE)); + assert!( + rx.try_recv().is_err(), + "watcher must not resolve delegation" + ); + runtime.state.role_changed("controller"); + runtime.key(KeyEvent::new(KeyCode::Char(key), KeyModifiers::NONE)); + let request = rx.try_recv().unwrap(); + assert_eq!(request["method"], "_nori/session/delegation/resolve"); + assert_eq!( + request["params"], + json!({"session_id":"parent-session","request_id":"delegation-one","approved":approved}) + ); + } +} + +#[test] +fn delegation_result_replaces_approval_and_replay_is_not_duplicated() { + let mut state = AttachState { + connected: true, + selected: Some("parent-session".into()), + list_open: false, + ..Default::default() + }; + state + .apply(delegation_event(1, "awaiting_approval")) + .unwrap(); + state.apply(delegation_event(2, "completed")).unwrap(); + state.apply(delegation_event(2, "completed")).unwrap(); + let rendered = screen(&state); + assert!(rendered.contains("Parser reviewed"), "{rendered}"); + assert!( + !rendered.contains("1 Start"), + "completed delegation must not retain approval" + ); + assert_eq!( + state + .transcript_text("parent-session") + .matches("Parser reviewed") + .count(), + 1 + ); + insta::assert_snapshot!("delegation_result", rendered); +} + +#[test] +fn next_waiting_opens_actual_child_and_keeps_parent_identity_in_list() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); + let mut runtime = super::runtime::Runtime::new(Some("parent-session".into()), tx); + runtime.state.connected = true; + runtime.state.replace_sessions(json!({"sessions":[ + {"session_id":"parent-session","agent":"mock-model","status":"ready","title":"Parent task","parent":null,"depth":0}, + {"session_id":"child-session","agent":"mock-model-alt","status":"waiting_on_you","title":"Child task","parent":"parent-session","depth":1} + ]})).unwrap(); + runtime.state.list_open = true; + let rendered = screen(&runtime.state); + assert!( + rendered.contains("Parent task") && rendered.contains("Child task"), + "{rendered}" + ); + assert!( + rendered.contains("(parent parent-s) Child task"), + "{rendered}" + ); + insta::assert_snapshot!("delegation_child_list", rendered); + runtime.key(KeyEvent::new(KeyCode::Down, KeyModifiers::ALT)); + assert_eq!(runtime.state.selected.as_deref(), Some("child-session")); + assert!(!runtime.state.list_open); + let request = rx.try_recv().unwrap(); + assert_eq!(request["method"], "_nori/session/subscribe"); + assert_eq!(request["params"]["session_id"], "child-session"); +} + +#[test] +fn next_waiting_uses_the_configured_binding() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + let (tx, _rx) = tokio::sync::mpsc::unbounded_channel(); + let mut runtime = super::runtime::Runtime::new(Some("parent-session".into()), tx); + runtime.set_next_waiting_binding(nori_config::HotkeyBinding::from_str("alt+w")); + runtime.state.connected = true; + runtime.state.replace_sessions(json!({"sessions":[ + {"session_id":"parent-session","agent":"mock-model","status":"ready","title":"Parent task"}, + {"session_id":"child-session","agent":"mock-model-alt","status":"waiting_on_you","title":"Child task","parent":"parent-session"} + ]})).unwrap(); + runtime.key(KeyEvent::new(KeyCode::Down, KeyModifiers::ALT)); + assert_eq!(runtime.state.selected.as_deref(), Some("parent-session")); + runtime.key(KeyEvent::new(KeyCode::Char('w'), KeyModifiers::ALT)); + assert_eq!(runtime.state.selected.as_deref(), Some("child-session")); +} + +#[test] +fn refreshed_delegation_completion_removes_an_unseen_approval() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); + let mut runtime = super::runtime::Runtime::new(Some("parent-session".into()), tx); + runtime.state.connected = true; + runtime.state.role_changed("controller"); + runtime + .state + .apply(delegation_event(1, "awaiting_approval")) + .unwrap(); + let nori_protocol::SessionEvent::Nori(nori_protocol::NoriEvent::DelegationChanged(completed)) = + delegation_event(2, "completed").event + else { + panic!("delegation fixture"); + }; + runtime.state.replace_sessions(json!({"sessions":[{"session_id":"parent-session","agent":"mock-model","status":"ready","title":"Parent task","delegations":[completed]}]})).unwrap(); + runtime.key(KeyEvent::new(KeyCode::Char('1'), KeyModifiers::NONE)); + assert!( + rx.try_recv().is_err(), + "list refresh must remove stale approval actions before parent replay arrives" + ); + assert!(!screen(&runtime.state).contains("1 Start")); +} + +#[test] +fn next_waiting_refreshes_stale_rows_and_opens_new_child_without_second_key() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); + let mut runtime = super::runtime::Runtime::new(Some("parent-session".into()), tx); + runtime.state.connected = true; + runtime.state.replace_sessions(json!({"sessions":[ + {"session_id":"parent-session","agent":"mock-model","status":"ready","title":"Parent task"} + ]})).unwrap(); + runtime.key(KeyEvent::new(KeyCode::Down, KeyModifiers::ALT)); + let refresh = rx + .try_recv() + .expect("navigation refreshes stale session rows"); + assert_eq!(refresh["method"], "_nori/session/list"); + runtime.connection_event(super::connection::ConnectionEvent::Message(json!({ + "id":refresh["id"],"result":{"sessions":[ + {"session_id":"parent-session","agent":"mock-model","status":"ready"}, + {"session_id":"child-session","agent":"mock-model","status":"waiting_on_you","parent":"parent-session","depth":1} + ]} + }))).unwrap(); + assert_eq!(runtime.state.selected.as_deref(), Some("child-session")); + let subscribe = rx.try_recv().unwrap(); + assert_eq!(subscribe["method"], "_nori/session/subscribe"); + assert_eq!(subscribe["params"]["session_id"], "child-session"); + assert!( + rx.try_recv().is_err(), + "one navigation sends one subscription" + ); +} diff --git a/nori-rs/tui/src/bottom_pane/bottom_pane_view.rs b/nori-rs/tui/src/bottom_pane/bottom_pane_view.rs index 6de1a2088..ed4ce2660 100644 --- a/nori-rs/tui/src/bottom_pane/bottom_pane_view.rs +++ b/nori-rs/tui/src/bottom_pane/bottom_pane_view.rs @@ -85,6 +85,11 @@ pub(crate) trait BottomPaneView: Renderable { false } + /// Whether a selection view owns this stable action identifier. + fn has_selection_item(&self, _stable_id: &str) -> bool { + false + } + /// Remove an item in a generic selection view by stable id. fn remove_selection_item(&mut self, _stable_id: &str) -> bool { false diff --git a/nori-rs/tui/src/bottom_pane/list_selection_view.rs b/nori-rs/tui/src/bottom_pane/list_selection_view.rs index d96dd2617..f953ff1a5 100644 --- a/nori-rs/tui/src/bottom_pane/list_selection_view.rs +++ b/nori-rs/tui/src/bottom_pane/list_selection_view.rs @@ -629,6 +629,12 @@ impl BottomPaneView for ListSelectionView { self.update_item(stable_id, name, description, search_value) } + fn has_selection_item(&self, stable_id: &str) -> bool { + self.items + .iter() + .any(|item| item.search_value.as_deref().map(search_value_id) == Some(stable_id)) + } + fn remove_selection_item(&mut self, stable_id: &str) -> bool { self.remove_item(stable_id) } diff --git a/nori-rs/tui/src/bottom_pane/mod.rs b/nori-rs/tui/src/bottom_pane/mod.rs index 26f249b4a..da212d581 100644 --- a/nori-rs/tui/src/bottom_pane/mod.rs +++ b/nori-rs/tui/src/bottom_pane/mod.rs @@ -601,6 +601,12 @@ impl BottomPane { } } + pub(crate) fn dismiss_selection(&mut self, stable_id: &str) { + self.view_stack + .retain(|view| !view.has_selection_item(stable_id)); + self.request_redraw(); + } + pub(crate) fn remove_selection_item(&mut self, stable_id: &str) { if let Some(view) = self.view_stack.last_mut() && view.remove_selection_item(stable_id) diff --git a/nori-rs/tui/src/chatwidget/agent.rs b/nori-rs/tui/src/chatwidget/agent.rs index 206f87316..6033ba76f 100644 --- a/nori-rs/tui/src/chatwidget/agent.rs +++ b/nori-rs/tui/src/chatwidget/agent.rs @@ -269,3 +269,107 @@ pub(super) fn forward_managed_session( conversation_id: Some(session.conversation_id), } } + +/// Connect a child history and live subscription to its widget. +pub(super) fn forward_managed_view( + id: nori_harness::ConversationId, + mut subscription: nori_harness::remote_agent::ManagedSubscription, + tx: AppEventSender, + generation: crate::app_event::SessionGeneration, +) { + tokio::spawn(async move { + let mut current = id.to_string(); + let mut forward = |envelope: nori_protocol::SessionEnvelope| { + if let SessionEvent::Nori(NoriEvent::SessionForked(fork)) = &envelope.event + && fork.previous_conversation_id == current + { + current.clone_from(&fork.new_conversation_id); + } + if envelope.conversation_id == current { + tx.send(AppEvent::SessionEvent { + generation, + event: envelope.event, + }); + } + }; + for envelope in subscription.replay { + forward(envelope); + } + while let Some(envelope) = subscription.events.recv().await { + forward(envelope); + } + }); +} + +#[cfg(test)] +mod delegated_view_tests { + use super::*; + + #[tokio::test] + async fn delegated_view_follows_fork_and_excludes_other_conversations() { + let old = nori_harness::ConversationId::from_string("00000000-0000-0000-0000-000000000001") + .unwrap(); + let new = "00000000-0000-0000-0000-000000000002"; + let envelope = |id: &str, seq: i64, event| nori_protocol::SessionEnvelope { + conversation_id: id.into(), + agent: "mock-model".into(), + seq, + event, + }; + let notice = |text: &str| { + SessionEvent::Nori(NoriEvent::Notice(nori_protocol::Notice { + message: text.into(), + })) + }; + let (events, receiver) = tokio::sync::mpsc::channel(4); + let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); + forward_managed_view( + old, + nori_harness::remote_agent::ManagedSubscription { + replay: vec![ + envelope(&old.to_string(), 1, notice("before fork")), + envelope( + new, + 1, + SessionEvent::Nori(NoriEvent::SessionForked( + nori_protocol::SessionForked { + previous_conversation_id: old.to_string(), + new_conversation_id: new.into(), + new_acp_session_id: "fork-agent".into(), + }, + )), + ), + ], + events: receiver, + }, + AppEventSender::new(tx), + 7, + ); + events + .send(envelope("unrelated", 1, notice("unrelated activity"))) + .await + .unwrap(); + events + .send(envelope(new, 2, notice("child needs permission"))) + .await + .unwrap(); + drop(events); + let mut received = Vec::new(); + while let Some(event) = tokio::time::timeout(std::time::Duration::from_secs(2), rx.recv()) + .await + .unwrap() + { + if let AppEvent::SessionEvent { event, .. } = event { + received.push(event); + } + } + assert_eq!( + received.len(), + 3, + "only the child process's events reach its view" + ); + assert!( + matches!(received.last(), Some(SessionEvent::Nori(NoriEvent::Notice(notice))) if notice.message == "child needs permission") + ); + } +} diff --git a/nori-rs/tui/src/chatwidget/constructors.rs b/nori-rs/tui/src/chatwidget/constructors.rs index 2339f095f..016456e9b 100644 --- a/nori-rs/tui/src/chatwidget/constructors.rs +++ b/nori-rs/tui/src/chatwidget/constructors.rs @@ -14,7 +14,14 @@ impl ChatWidget { ) -> Self { common.deferred_spawn = true; let analytics = common.analytics.clone(); + let delegated_child = common + .session_manager + .get(&session.conversation_id) + .is_some_and(|record| { + record.source == nori_harness::session_manager::SessionSource::Session + }); let mut widget = Self::new(common); + widget.delegated_child = delegated_child; let spawn = super::agent::forward_managed_session( session, widget.app_event_tx.clone(), @@ -26,6 +33,26 @@ impl ChatWidget { widget } + /// Observe a child using the manager's atomic replay and live subscription. + pub(crate) fn from_managed_view( + mut common: ChatWidgetInit, + id: ConversationId, + handle: Option<HarnessHandle>, + subscription: nori_harness::remote_agent::ManagedSubscription, + ) -> Self { + common.deferred_spawn = true; + let mut widget = Self::new(common); + widget.delegated_child = true; + widget.managed_conversation_id = Some(id); + widget.harness_handle = handle; + widget.set_foreground(false); + widget.suppress_welcome_card(); + let tx = widget.app_event_tx.clone(); + let generation = widget.session_generation; + super::agent::forward_managed_view(id, subscription, tx, generation); + widget + } + /// Build a hidden switch candidate. The app commits it only after its /// `SessionStarted` event. pub(crate) fn new_candidate(common: ChatWidgetInit) -> Self { @@ -149,6 +176,9 @@ impl ChatWidget { managed_conversation_id: spawn_result.conversation_id, session_close_in_flight: false, closed_by_user: false, + delegated_child: false, + delegations: HashMap::new(), + delegation_approval: None, exiting: false, agent_config: crate::nori::agent_config_state::AgentConfigState::default(), agent_status: crate::nori::session_header::AgentStatusHandle::new( @@ -338,6 +368,9 @@ impl ChatWidget { managed_conversation_id: spawn_result.conversation_id, session_close_in_flight: false, closed_by_user: false, + delegated_child: false, + delegations: HashMap::new(), + delegation_approval: None, exiting: false, agent_config: crate::nori::agent_config_state::AgentConfigState::default(), agent_status: crate::nori::session_header::AgentStatusHandle::new( diff --git a/nori-rs/tui/src/chatwidget/delegation.rs b/nori-rs/tui/src/chatwidget/delegation.rs new file mode 100644 index 000000000..0ed2dae33 --- /dev/null +++ b/nori-rs/tui/src/chatwidget/delegation.rs @@ -0,0 +1,135 @@ +use super::*; +use nori_protocol::Delegation; +use nori_protocol::DelegationStatus; + +impl ChatWidget { + pub(super) fn handle_delegate_command(&mut self, text: &str) -> bool { + let Some(command) = crate::delegation::parse_command(text) else { + return false; + }; + if !self.config.multi_session { + self.add_error_message("Delegation requires [features] multi_session = true.".into()); + return true; + } + if text.trim() == "/delegate" && self.show_delegation_approval() { + return true; + } + let (agent, task) = match command { + Ok(command) => command, + Err(message) => { + self.add_error_message(message); + return true; + } + }; + let Some(parent) = self.managed_conversation_id else { + self.add_error_message("Start a session before delegating a task.".into()); + return true; + }; + let manager = self.session_manager.clone(); + let tx = self.app_event_tx.clone(); + tokio::spawn(async move { + if let Err(error) = manager.delegate(&parent, agent, task).await { + tx.send(AppEvent::InsertHistoryCell(Box::new( + history_cell::new_error_event(format!("Delegation failed: {error}")), + ))); + } + }); + true + } + + pub(super) fn handle_delegation_changed(&mut self, delegation: Delegation) { + if self + .managed_conversation_id + .is_some_and(|id| id.to_string() != delegation.parent_id) + { + return; + } + if self.delegations.get(&delegation.request_id) == Some(&delegation) { + return; + } + let parent_changed = self + .delegations + .get(&delegation.request_id) + .is_some_and(|previous| previous.parent_id != delegation.parent_id); + if (delegation.status != DelegationStatus::AwaitingApproval || parent_changed) + && self.delegation_approval.as_ref() == Some(&delegation.request_id) + { + self.bottom_pane.dismiss_selection(&delegation.request_id); + self.delegation_approval = None; + } + self.add_info_message(crate::delegation::summary(&delegation), None); + self.delegations + .insert(delegation.request_id.clone(), delegation); + self.show_delegation_approval(); + } + + fn show_delegation_approval(&mut self) -> bool { + if self.delegation_approval.is_some() { + return true; + } + let Some(delegation) = self + .delegations + .values() + .find(|delegation| delegation.status == DelegationStatus::AwaitingApproval) + .cloned() + else { + return false; + }; + let Ok(parent) = ConversationId::from_string(&delegation.parent_id) else { + return false; + }; + let items = [("Start", true), ("Decline", false)] + .into_iter() + .map(|(name, approved)| { + let request_id = delegation.request_id.clone(); + let stable_id = request_id.clone(); + SelectionItem { + name: name.into(), + actions: vec![Box::new(move |tx| { + tx.send(AppEvent::ResolveDelegation { + parent, + request_id: request_id.clone(), + approved, + }) + })], + search_value: Some(stable_id), + dismiss_on_select: false, + ..Default::default() + } + }) + .collect(); + self.delegation_approval = Some(delegation.request_id.clone()); + let request_id = delegation.request_id.clone(); + self.bottom_pane.show_selection_view(SelectionViewParams { + on_dismiss: Some(Box::new(move |tx| { + tx.send(AppEvent::ResolveDelegation { + parent, + request_id: request_id.clone(), + approved: false, + }) + })), + header: Box::new(ColumnRenderable::with(vec![ + Box::new(Line::from("Start delegated session?").bold()) as Box<dyn Renderable>, + Box::new( + Paragraph::new(format!( + "Child {} ({})", + delegation.child_id, delegation.agent + )) + .wrap(Wrap { trim: false }), + ), + Box::new( + Paragraph::new(format!( + "parent {} ({})", + delegation.parent_id, delegation.parent_agent + )) + .wrap(Wrap { trim: false }), + ), + Box::new(Paragraph::new(delegation.task).wrap(Wrap { trim: false })), + ])), + items, + footer_hint: Some(standard_popup_hint_line()), + ..Default::default() + }); + true + } +} diff --git a/nori-rs/tui/src/chatwidget/event_handlers.rs b/nori-rs/tui/src/chatwidget/event_handlers.rs index aa45cdd52..20647c92f 100644 --- a/nori-rs/tui/src/chatwidget/event_handlers.rs +++ b/nori-rs/tui/src/chatwidget/event_handlers.rs @@ -235,13 +235,16 @@ impl ChatWidget { }; self.handle_client_phase_changed(phase); } + nori_protocol::NoriEvent::DelegationChanged(delegation) => { + self.handle_delegation_changed(delegation); + } nori_protocol::NoriEvent::SessionEnded(ended) => { if !self.session_configured_received { self.bottom_pane.hide_status_indicator(); } match ended.reason { nori_protocol::SessionEndReason::Shutdown => { - if !self.closed_by_user { + if !self.closed_by_user && !self.delegated_child { self.app_event_tx.send(AppEvent::ExitRequest); } } @@ -348,6 +351,11 @@ impl ChatWidget { nori_harness::ConversationId::from_string(&forked.new_conversation_id).ok(); if self.conversation_id.is_some() { self.managed_conversation_id = self.conversation_id; + if self.delegated_child { + self.harness_handle = self + .conversation_id + .and_then(|id| self.session_manager.handle(&id)); + } } self.forked_from = nori_harness::ConversationId::from_string(&forked.previous_conversation_id).ok(); diff --git a/nori-rs/tui/src/chatwidget/key_handling.rs b/nori-rs/tui/src/chatwidget/key_handling.rs index 273bf4749..8734cff73 100644 --- a/nori-rs/tui/src/chatwidget/key_handling.rs +++ b/nori-rs/tui/src/chatwidget/key_handling.rs @@ -147,6 +147,9 @@ impl ChatWidget { SlashCommand::New => { self.app_event_tx.send(AppEvent::NewSession); } + SlashCommand::Delegate => { + self.handle_delegate_command("/delegate"); + } SlashCommand::Sessions => { self.app_event_tx.send(AppEvent::OpenSessionList); } diff --git a/nori-rs/tui/src/chatwidget/mod.rs b/nori-rs/tui/src/chatwidget/mod.rs index 8d26df55c..62acb66e6 100644 --- a/nori-rs/tui/src/chatwidget/mod.rs +++ b/nori-rs/tui/src/chatwidget/mod.rs @@ -62,6 +62,7 @@ use self::agent::spawn_agent; mod approvals; mod constructors; mod context; +mod delegation; pub(crate) use context::ContextSnapshot; mod event_handlers; mod goal; @@ -240,6 +241,9 @@ pub(crate) struct ChatWidget { /// True once the user closed this session beside others, so its agent /// shutting down ends only this session, not Nori. closed_by_user: bool, + delegated_child: bool, + delegations: HashMap<String, nori_protocol::Delegation>, + delegation_approval: Option<String>, /// True once the user asked to quit: input is refused, and a hard-exit /// watchdog guarantees the process leaves within ~a second even if the /// backend teardown stalls. On cloud agents this exit is a detach — the diff --git a/nori-rs/tui/src/chatwidget/tests/delegation.rs b/nori-rs/tui/src/chatwidget/tests/delegation.rs new file mode 100644 index 000000000..2adfcab6f --- /dev/null +++ b/nori-rs/tui/src/chatwidget/tests/delegation.rs @@ -0,0 +1,143 @@ +use super::*; + +fn delegation(status: &str) -> nori_protocol::SessionEvent { + serde_json::from_value(serde_json::json!({"source":"nori","event":{"event_type":"delegation_changed","event":{ + "request_id":"delegation-one","parent_id":"00000000-0000-0000-0000-000000000001", + "child_id":"00000000-0000-0000-0000-000000000002","parent_agent":"mock-model","agent":"mock-model-alt", + "task":"Review the parser","depth":1,"status":status,"result":null + }}})).unwrap() +} + +#[test] +fn local_delegation_approval_identifies_parent_child_and_start_decline() { + use crate::render::renderable::Renderable; + let (mut chat, _rx, _) = make_chatwidget_manual(); + chat.config.multi_session = true; + let generation = chat.session_generation(); + chat.handle_session_event(generation, delegation("awaiting_approval")); + let area = ratatui::layout::Rect::new(0, 0, 100, chat.bottom_pane.desired_height(100)); + let mut buffer = ratatui::buffer::Buffer::empty(area); + chat.bottom_pane.render(area, &mut buffer); + let rendered = (0..area.height) + .map(|y| { + (0..area.width) + .map(|x| buffer[(x, y)].symbol()) + .collect::<String>() + .trim_end() + .to_owned() + }) + .collect::<Vec<_>>() + .join("\n"); + assert!( + rendered.contains("Start") && rendered.contains("Decline"), + "{rendered}" + ); + assert!( + rendered.contains("mock-model-alt") && rendered.contains("parent"), + "{rendered}" + ); + assert!( + rendered.contains("00000000-0000-0000-0000-000000000001") + && rendered.contains("00000000-0000-0000-0000-000000000002"), + "approval identifies both sessions: {rendered}" + ); + insta::assert_snapshot!("local_delegation_approval", rendered); +} + +#[test] +fn child_shutdown_preserves_terminal_and_history() { + let (mut chat, mut rx, _) = make_chatwidget_manual(); + chat.config.multi_session = true; + let id = + nori_harness::ConversationId::from_string("00000000-0000-0000-0000-000000000002").unwrap(); + chat.managed_conversation_id = Some(id); + chat.delegated_child = true; + while rx.try_recv().is_ok() {} + chat.handle_session_event( + chat.session_generation(), + nori_protocol::SessionEvent::Nori(nori_protocol::NoriEvent::SessionEnded( + nori_protocol::SessionEnded { + reason: nori_protocol::SessionEndReason::Shutdown, + message: None, + }, + )), + ); + while let Ok(event) = rx.try_recv() { + assert!( + !matches!( + event, + AppEvent::ExitRequest | AppEvent::SessionClosed { .. } + ), + "a completed child must not exit or replace the visible session" + ); + } +} + +#[test] +fn local_delegate_rejects_flag_off_without_prompting_agent() { + let (mut chat, mut rx, _) = make_chatwidget_manual(); + chat.submit_user_message_text("/delegate mock-model-alt inspect parser".into()); + let mut output = String::new(); + while let Ok(event) = rx.try_recv() { + if let AppEvent::InsertHistoryCell(cell) = event { + output.push_str(&lines_to_single_string(&cell.display_lines(120))); + } else { + assert!( + !matches!(event, AppEvent::HarnessAction(_) | AppEvent::NewSession), + "flag-off command cannot submit work" + ); + } + } + assert!(output.contains("multi_session"), "{output}"); +} + +#[test] +fn forked_parent_refreshes_pending_approval_target() { + let (mut chat, mut rx, _) = make_chatwidget_manual(); + chat.config.multi_session = true; + let generation = chat.session_generation(); + chat.handle_session_event(generation, delegation("awaiting_approval")); + let nori_protocol::SessionEvent::Nori(nori_protocol::NoriEvent::DelegationChanged(mut changed)) = + delegation("awaiting_approval") + else { + panic!("delegation fixture"); + }; + changed.parent_id = "00000000-0000-0000-0000-000000000003".into(); + chat.handle_session_event( + generation, + nori_protocol::SessionEvent::Nori(nori_protocol::NoriEvent::DelegationChanged(changed)), + ); + chat.handle_key_event(crossterm::event::KeyEvent::new( + crossterm::event::KeyCode::Char('1'), + crossterm::event::KeyModifiers::NONE, + )); + let mut approved = None; + while let Ok(event) = rx.try_recv() { + if let AppEvent::ResolveDelegation { parent, .. } = event { + approved = Some(parent.to_string()); + } + } + assert_eq!( + approved.as_deref(), + Some("00000000-0000-0000-0000-000000000003") + ); +} + +#[test] +fn delegation_result_and_parent_continuation_preserve_composer_draft() { + let (mut chat, _rx, _) = make_chatwidget_manual(); + chat.config.multi_session = true; + chat.set_composer_text("/sessions".into()); + let generation = chat.session_generation(); + chat.handle_session_event(generation, delegation("completed")); + chat.on_task_started(); + chat.handle_client_event(crate::presentation::ClientEvent::MessageDelta( + crate::presentation::MessageDelta { + stream: crate::presentation::MessageStream::User, + delta: "[Nori delegation result] Child completed".into(), + message_id: Some("delegation-result".into()), + }, + )); + chat.on_task_complete(Some("Result received".into())); + assert_eq!(chat.composer_text(), "/sessions"); +} diff --git a/nori-rs/tui/src/chatwidget/tests/mod.rs b/nori-rs/tui/src/chatwidget/tests/mod.rs index 06ffa4b71..596a21757 100644 --- a/nori-rs/tui/src/chatwidget/tests/mod.rs +++ b/nori-rs/tui/src/chatwidget/tests/mod.rs @@ -127,5 +127,6 @@ fn make_chatwidget_manual_for_mode( mod agent_status; mod context; +mod delegation; mod part10; mod part8; diff --git a/nori-rs/tui/src/chatwidget/tests/snapshots/nori_tui__chatwidget__tests__delegation__local_delegation_approval.snap b/nori-rs/tui/src/chatwidget/tests/snapshots/nori_tui__chatwidget__tests__delegation__local_delegation_approval.snap new file mode 100644 index 000000000..e9671c2e7 --- /dev/null +++ b/nori-rs/tui/src/chatwidget/tests/snapshots/nori_tui__chatwidget__tests__delegation__local_delegation_approval.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/chatwidget/tests/delegation.rs +expression: rendered +--- + Start delegated session? + Child 00000000-0000-0000-0000-000000000002 (mock-model-alt) + parent 00000000-0000-0000-0000-000000000001 (mock-model) + Review the parser + +› 1. Start + 2. Decline + + ↑/k ↓/j to navigate, enter to confirm, esc to go back diff --git a/nori-rs/tui/src/chatwidget/user_input.rs b/nori-rs/tui/src/chatwidget/user_input.rs index 502125afb..e00cd071a 100644 --- a/nori-rs/tui/src/chatwidget/user_input.rs +++ b/nori-rs/tui/src/chatwidget/user_input.rs @@ -91,6 +91,10 @@ impl ChatWidget { return; } + if image_paths.is_empty() && self.handle_delegate_command(&text) { + return; + } + if image_paths.is_empty() && self.handle_goal_user_message(&text) { self.persist_prompt_history(&text); return; @@ -132,6 +136,21 @@ impl ChatWidget { return; } + if self.delegated_child + && self + .managed_conversation_id + .and_then(|id| self.session_manager.get(&id)) + .is_some_and(|record| { + matches!( + record.status, + nori_protocol::SessionStatus::Closed | nori_protocol::SessionStatus::Failed + ) + }) + { + self.add_error_message("This child session has ended. Its history remains available; return to its parent or start /new.".into()); + return; + } + // Followers may use local commands, but must never submit a prompt to // the Slack-owned ACP session, including through programmatic callers. if self.follow_only_attachment { diff --git a/nori-rs/tui/src/delegation.rs b/nori-rs/tui/src/delegation.rs new file mode 100644 index 000000000..084b526ea --- /dev/null +++ b/nori-rs/tui/src/delegation.rs @@ -0,0 +1,39 @@ +//! Shared command syntax and minimal delegation labels for both terminal frontends. +use nori_protocol::Delegation; +use nori_protocol::DelegationStatus; + +pub(crate) fn parse_command(text: &str) -> Option<Result<(String, String), String>> { + let rest = text.strip_prefix("/delegate")?; + if !rest.is_empty() && !rest.starts_with(char::is_whitespace) { + return None; + } + let Some((agent, task)) = rest.trim().split_once(char::is_whitespace) else { + return Some(Err("Usage: /delegate <agent> <task>".into())); + }; + let task = task.trim(); + Some(if task.is_empty() { + Err("Usage: /delegate <agent> <task>".into()) + } else { + Ok((agent.to_owned(), task.to_owned())) + }) +} + +pub(crate) fn summary(delegation: &Delegation) -> String { + let status = match delegation.status { + DelegationStatus::AwaitingApproval => "awaiting approval", + DelegationStatus::Running => "running", + DelegationStatus::Declined => "declined", + DelegationStatus::Completed => "completed", + DelegationStatus::Failed => "failed", + DelegationStatus::Cancelled => "cancelled", + }; + let mut message = format!( + "Child {} ({}) · parent {} ({}) · {status}", + delegation.child_id, delegation.agent, delegation.parent_id, delegation.parent_agent, + ); + if let Some(result) = &delegation.result { + message.push('\n'); + message.push_str(result); + } + message +} diff --git a/nori-rs/tui/src/lib.rs b/nori-rs/tui/src/lib.rs index 6a1d64800..592c2060d 100644 --- a/nori-rs/tui/src/lib.rs +++ b/nori-rs/tui/src/lib.rs @@ -44,6 +44,7 @@ mod clipboard_paste; mod color; mod context_view; pub mod custom_terminal; +mod delegation; mod diff_render; mod editor; mod effective_cwd_tracker; diff --git a/nori-rs/tui/src/slash_command.rs b/nori-rs/tui/src/slash_command.rs index 03de1fc4d..0fb6543b3 100644 --- a/nori-rs/tui/src/slash_command.rs +++ b/nori-rs/tui/src/slash_command.rs @@ -36,6 +36,7 @@ pub enum SlashCommand { Goal, New, Sessions, + Delegate, Resume, ResumeViewonly, Close, @@ -67,6 +68,7 @@ impl SlashCommand { SlashCommand::Agent => "switch between available ACP agents", SlashCommand::New => "start a new chat during a conversation", SlashCommand::Sessions => "list sessions and move between them", + SlashCommand::Delegate => "delegate a task: /delegate <agent> <task>", SlashCommand::Resume => "resume a previous session", SlashCommand::ResumeViewonly => "view a previous session transcript (read-only)", SlashCommand::Close => "close (release) the current session and start a fresh chat", @@ -127,6 +129,7 @@ impl SlashCommand { | SlashCommand::Browser => false, SlashCommand::Browse | SlashCommand::Sessions + | SlashCommand::Delegate | SlashCommand::Diff | SlashCommand::Mention | SlashCommand::Status @@ -159,6 +162,7 @@ impl SlashCommand { | SlashCommand::Goal | SlashCommand::New | SlashCommand::Sessions + | SlashCommand::Delegate | SlashCommand::Resume | SlashCommand::ResumeViewonly | SlashCommand::Init @@ -183,7 +187,9 @@ impl SlashCommand { match self { #[cfg(not(feature = "login"))] SlashCommand::Logout => false, - SlashCommand::Sessions => MULTI_SESSION_COMMANDS.load(Ordering::Relaxed), + SlashCommand::Sessions | SlashCommand::Delegate => { + MULTI_SESSION_COMMANDS.load(Ordering::Relaxed) + } _ => true, } } From 3ee695d2c4df067f4e27a7811974b6ec04c5b68f Mon Sep 17 00:00:00 2001 From: Nori <contact@tilework.tech> Date: Sun, 15 Nov 2026 17:21:08 +0000 Subject: [PATCH 2/6] fix(attach): preserve draft ownership during waiting-session refresh --- nori-rs/tui/src/attach/docs.md | 10 +++-- nori-rs/tui/src/attach/input.rs | 2 + nori-rs/tui/src/attach/runtime.rs | 3 +- nori-rs/tui/src/attach/tests.rs | 62 +++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 5 deletions(-) diff --git a/nori-rs/tui/src/attach/docs.md b/nori-rs/tui/src/attach/docs.md index 2cf427e4b..a4a1294e9 100644 --- a/nori-rs/tui/src/attach/docs.md +++ b/nori-rs/tui/src/attach/docs.md @@ -64,10 +64,12 @@ Path: @/nori-rs/tui/src/attach `next_waiting_session` binding (Alt+Down by default) navigates there. If the cached list has no waiting target, a connected terminal retains the navigation action while refreshing the list, then selects a newly discovered waiting - session from the response. List errors or disconnection clear that pending - action. Parent notices and child list rows identify the child and its parent. - Hierarchy labels prefix TITLE so parent attribution survives long task titles while - metadata columns keep their positions. + session from the response. Later input or navigation, list errors, and + disconnection cancel that pending action. The response also requires an empty + composer, including after paste, so a draft begun in the parent stays there. + Parent notices and child list rows identify the child and its parent. + Hierarchy labels prefix TITLE so parent attribution survives long task titles + while metadata columns keep their positions. Results return through the parent's ordinary event stream without a terminal review step. diff --git a/nori-rs/tui/src/attach/input.rs b/nori-rs/tui/src/attach/input.rs index cde9b58c8..e2d8aa023 100644 --- a/nori-rs/tui/src/attach/input.rs +++ b/nori-rs/tui/src/attach/input.rs @@ -18,6 +18,8 @@ impl Runtime { if key.kind == KeyEventKind::Release { return true; } + // A later user action supersedes navigation waiting on a list refresh. + self.next_waiting_after_refresh = false; if key.code == KeyCode::Char('d') && key.modifiers.contains(KeyModifiers::CONTROL) { return false; } diff --git a/nori-rs/tui/src/attach/runtime.rs b/nori-rs/tui/src/attach/runtime.rs index f201de805..84065dfd7 100644 --- a/nori-rs/tui/src/attach/runtime.rs +++ b/nori-rs/tui/src/attach/runtime.rs @@ -221,7 +221,8 @@ impl Runtime { } Pending::List => { self.state.replace_sessions(result.clone())?; - if std::mem::take(&mut self.next_waiting_after_refresh) { + if std::mem::take(&mut self.next_waiting_after_refresh) && self.composer.is_empty() + { self.next_waiting_session(false); } if let Some(id) = &self.state.selected diff --git a/nori-rs/tui/src/attach/tests.rs b/nori-rs/tui/src/attach/tests.rs index 68bd159b9..72dda707d 100644 --- a/nori-rs/tui/src/attach/tests.rs +++ b/nori-rs/tui/src/attach/tests.rs @@ -640,3 +640,65 @@ fn next_waiting_refreshes_stale_rows_and_opens_new_child_without_second_key() { "one navigation sends one subscription" ); } + +#[test] +fn deferred_next_waiting_keeps_a_new_draft_in_its_parent_session() { + use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; + let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); + let mut runtime = super::runtime::Runtime::new(Some("parent-session".into()), tx); + runtime.state.connected = true; + runtime.state.role_changed("controller"); + runtime + .state + .replace_sessions(json!({"sessions":[ + {"session_id":"parent-session","agent":"mock-model","status":"ready"} + ]})) + .unwrap(); + runtime.key(KeyEvent::new(KeyCode::Down, KeyModifiers::ALT)); + let refresh = rx.try_recv().unwrap(); + runtime.composer.handle_paste("Parent-only draft".into()); + runtime.connection_event(super::connection::ConnectionEvent::Message(json!({ + "id":refresh["id"],"result":{"sessions":[ + {"session_id":"parent-session","agent":"mock-model","status":"ready"}, + {"session_id":"child-session","agent":"mock-model","status":"waiting_on_you","parent":"parent-session","depth":1} + ]} + }))).unwrap(); + assert_eq!(runtime.state.selected.as_deref(), Some("parent-session")); + assert!(rx.try_recv().is_err(), "draft prevents deferred navigation"); + runtime.key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + let prompt = rx.try_recv().unwrap(); + assert_eq!(prompt["method"], "_nori/session/prompt"); + assert_eq!(prompt["params"]["session_id"], "parent-session"); +} + +#[test] +fn explicit_session_list_navigation_cancels_deferred_next_waiting() { + use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; + let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); + let mut runtime = super::runtime::Runtime::new(Some("parent-session".into()), tx); + runtime.state.connected = true; + runtime + .state + .replace_sessions(json!({"sessions":[ + {"session_id":"parent-session","agent":"mock-model","status":"ready"} + ]})) + .unwrap(); + runtime.key(KeyEvent::new(KeyCode::Down, KeyModifiers::ALT)); + let refresh = rx.try_recv().unwrap(); + runtime.key(KeyEvent::new(KeyCode::Char('s'), KeyModifiers::ALT)); + runtime.connection_event(super::connection::ConnectionEvent::Message(json!({ + "id":refresh["id"],"result":{"sessions":[ + {"session_id":"parent-session","agent":"mock-model","status":"ready"}, + {"session_id":"child-session","agent":"mock-model","status":"waiting_on_you","parent":"parent-session","depth":1} + ]} + }))).unwrap(); + assert_eq!(runtime.state.selected.as_deref(), Some("parent-session")); + assert!( + runtime.state.list_open, + "explicit list navigation remains open" + ); + assert!( + rx.try_recv().is_err(), + "refresh cannot override explicit navigation" + ); +} From e9ff6232c3269111f720128dcde16389827b1abb Mon Sep 17 00:00:00 2001 From: Nori <contact@tilework.tech> Date: Sun, 15 Nov 2026 17:22:56 +0000 Subject: [PATCH 3/6] style(attach): format navigation regression imports --- nori-rs/tui/src/attach/tests.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/nori-rs/tui/src/attach/tests.rs b/nori-rs/tui/src/attach/tests.rs index 72dda707d..7f0dddc9c 100644 --- a/nori-rs/tui/src/attach/tests.rs +++ b/nori-rs/tui/src/attach/tests.rs @@ -643,7 +643,9 @@ fn next_waiting_refreshes_stale_rows_and_opens_new_child_without_second_key() { #[test] fn deferred_next_waiting_keeps_a_new_draft_in_its_parent_session() { - use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); let mut runtime = super::runtime::Runtime::new(Some("parent-session".into()), tx); runtime.state.connected = true; @@ -673,7 +675,9 @@ fn deferred_next_waiting_keeps_a_new_draft_in_its_parent_session() { #[test] fn explicit_session_list_navigation_cancels_deferred_next_waiting() { - use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); let mut runtime = super::runtime::Runtime::new(Some("parent-session".into()), tx); runtime.state.connected = true; From e93deefdfdaddccfd9bf06ba92c9d9fb6f0498a4 Mon Sep 17 00:00:00 2001 From: Nori <contact@tilework.tech> Date: Sun, 15 Nov 2026 18:25:00 +0000 Subject: [PATCH 4/6] fix: scope delegation action failures to their session --- nori-rs/harness/docs.md | 10 + nori-rs/harness/src/runtime.rs | 120 ++++++--- nori-rs/tui/docs.md | 26 +- nori-rs/tui/src/app/mod.rs | 2 + nori-rs/tui/src/app/session_view.rs | 91 +++++-- nori-rs/tui/src/app/session_view_tests.rs | 205 ++++++++++++++++ nori-rs/tui/src/app_event_sender.rs | 3 +- nori-rs/tui/src/chatwidget/event_handlers.rs | 11 +- nori-rs/tui/src/chatwidget/helpers.rs | 8 +- .../src/chatwidget/tests/action_failures.rs | 228 ++++++++++++++++++ nori-rs/tui/src/chatwidget/tests/mod.rs | 11 + ...lures__visible_session_action_failure.snap | 5 + 12 files changed, 657 insertions(+), 63 deletions(-) create mode 100644 nori-rs/tui/src/app/session_view_tests.rs create mode 100644 nori-rs/tui/src/chatwidget/tests/action_failures.rs create mode 100644 nori-rs/tui/src/chatwidget/tests/snapshots/nori_tui__chatwidget__tests__action_failures__visible_session_action_failure.snap diff --git a/nori-rs/harness/docs.md b/nori-rs/harness/docs.md index 925e6c834..c5453a25d 100644 --- a/nori-rs/harness/docs.md +++ b/nori-rs/harness/docs.md @@ -501,6 +501,16 @@ task. A frontend may remain open to show the terminal state. Explicit `shutdown()` aborts the retained reducer and relay tasks during teardown so those session-owned tasks cannot outlive the harness. +[`HarnessHandle`](src/runtime.rs) clones share a normal-teardown marker for +their process. It is set while handling shutdown or close, retained on success, +reset on failure, and also set when the relay observes `Shutdown` or `Closed`. +Command-channel and response-channel closure use a private typed error; +`is_expected_shutdown_error` requires both that type and the normal-teardown +marker. Frontends can therefore omit late command failures caused by ordinary +child cleanup without hiding an actual backend error, even one returned before +shutdown began. Unexpected process loss does not establish normal teardown, +and a replacement process receives a fresh marker. + #### Compaction, branching, and session swap `compact()` has two paths, chosen in `harness/src/backend/submit_and_ops.rs`. If diff --git a/nori-rs/harness/src/runtime.rs b/nori-rs/harness/src/runtime.rs index f8ba39fe6..258b6c784 100644 --- a/nori-rs/harness/src/runtime.rs +++ b/nori-rs/harness/src/runtime.rs @@ -11,6 +11,8 @@ use std::collections::VecDeque; use std::path::PathBuf; use std::sync::Arc; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering; use std::time::Duration; use codex_rmcp_client::OAuthCredentialsStoreMode; @@ -183,10 +185,17 @@ enum HarnessCommand { }, } +/// The command transport disappeared before accepting or answering an action. +/// Backend errors retain their own types so shutdown never masks an ACP failure. +#[derive(Debug, thiserror::Error)] +#[error("{0}")] +struct HarnessTransportClosed(&'static str); + /// Typed handle for controlling one Nori harness session. #[derive(Clone)] pub struct HarnessHandle { command_tx: mpsc::UnboundedSender<HarnessCommand>, + normal_shutdown: Arc<AtomicBool>, first_prompt_started: Option<Arc<FirstPromptStarted>>, agent_response_observer: Option<AgentResponseObserver>, responder: Arc<str>, @@ -220,6 +229,14 @@ impl std::fmt::Debug for HarnessHandle { } impl HarnessHandle { + /// Whether an action lost its command transport during this process's + /// normal teardown. Actual backend failures remain reportable, including + /// failures returned before a subsequent shutdown. + pub fn is_expected_shutdown_error(&self, error: &anyhow::Error) -> bool { + self.normal_shutdown.load(Ordering::Acquire) + && error.downcast_ref::<HarnessTransportClosed>().is_some() + } + /// Return the context observations captured at prompt boundaries. pub async fn context_timeline( &self, @@ -309,10 +326,14 @@ impl HarnessHandle { admission, response_tx, }) - .map_err(|_| anyhow::anyhow!("ACP agent command channel closed"))?; - let request_id = response_rx - .await - .map_err(|_| anyhow::anyhow!("ACP agent did not accept the prompt"))??; + .map_err(|_| { + anyhow::Error::new(HarnessTransportClosed("ACP agent command channel closed")) + })?; + let request_id = response_rx.await.map_err(|_| { + anyhow::Error::new(HarnessTransportClosed( + "ACP agent did not accept the prompt", + )) + })??; if let Some(first_prompt_started) = &self.first_prompt_started && !first_prompt_started .fired @@ -340,10 +361,12 @@ impl HarnessHandle { child_grace, response_tx, }) - .map_err(|_| anyhow::anyhow!("ACP agent command channel closed"))?; - response_rx - .await - .map_err(|_| anyhow::anyhow!("ACP agent did not shut down"))? + .map_err(|_| { + anyhow::Error::new(HarnessTransportClosed("ACP agent command channel closed")) + })?; + response_rx.await.map_err(|_| { + anyhow::Error::new(HarnessTransportClosed("ACP agent did not shut down")) + })? } /// Complete a delegated ACP request using its original request ID. @@ -362,10 +385,14 @@ impl HarnessHandle { response, response_tx, }) - .map_err(|_| anyhow::anyhow!("ACP agent command channel closed"))?; - response_rx - .await - .map_err(|_| anyhow::anyhow!("ACP request response was not accepted"))? + .map_err(|_| { + anyhow::Error::new(HarnessTransportClosed("ACP agent command channel closed")) + })?; + response_rx.await.map_err(|_| { + anyhow::Error::new(HarnessTransportClosed( + "ACP request response was not accepted", + )) + })? } pub async fn cancel(&self) -> anyhow::Result<()> { @@ -417,10 +444,12 @@ impl HarnessHandle { let (response_tx, response_rx) = oneshot::channel(); self.command_tx .send(HarnessCommand::CustomPrompts { response_tx }) - .map_err(|_| anyhow::anyhow!("ACP agent command channel closed"))?; + .map_err(|_| { + anyhow::Error::new(HarnessTransportClosed("ACP agent command channel closed")) + })?; response_rx .await - .map_err(|_| anyhow::anyhow!("ACP agent did not respond")) + .map_err(|_| anyhow::Error::new(HarnessTransportClosed("ACP agent did not respond"))) } pub async fn compact(&self) -> anyhow::Result<()> { @@ -443,10 +472,12 @@ impl HarnessHandle { let (response_tx, response_rx) = oneshot::channel(); self.command_tx .send(HarnessCommand::UndoSnapshots { response_tx }) - .map_err(|_| anyhow::anyhow!("ACP agent command channel closed"))?; + .map_err(|_| { + anyhow::Error::new(HarnessTransportClosed("ACP agent command channel closed")) + })?; response_rx .await - .map_err(|_| anyhow::anyhow!("ACP agent did not respond")) + .map_err(|_| anyhow::Error::new(HarnessTransportClosed("ACP agent did not respond"))) } pub async fn undo_to(&self, index: i64) -> anyhow::Result<()> { @@ -477,10 +508,12 @@ impl HarnessHandle { let (response_tx, response_rx) = oneshot::channel(); self.command_tx .send(HarnessCommand::Goal { response_tx }) - .map_err(|_| anyhow::anyhow!("ACP agent command channel closed"))?; + .map_err(|_| { + anyhow::Error::new(HarnessTransportClosed("ACP agent command channel closed")) + })?; response_rx .await - .map_err(|_| anyhow::anyhow!("ACP agent did not respond")) + .map_err(|_| anyhow::Error::new(HarnessTransportClosed("ACP agent did not respond"))) } pub async fn set_goal( @@ -517,12 +550,12 @@ impl HarnessHandle { command: impl FnOnce(oneshot::Sender<anyhow::Result<T>>) -> HarnessCommand, ) -> anyhow::Result<T> { let (response_tx, response_rx) = oneshot::channel(); - self.command_tx - .send(command(response_tx)) - .map_err(|_| anyhow::anyhow!("ACP agent command channel closed"))?; + self.command_tx.send(command(response_tx)).map_err(|_| { + anyhow::Error::new(HarnessTransportClosed("ACP agent command channel closed")) + })?; response_rx .await - .map_err(|_| anyhow::anyhow!("ACP agent did not respond"))? + .map_err(|_| anyhow::Error::new(HarnessTransportClosed("ACP agent did not respond")))? } /// Get the current ACP session config snapshot from the agent. @@ -551,10 +584,12 @@ impl HarnessHandle { value, response_tx, }) - .map_err(|_| anyhow::anyhow!("ACP agent command channel closed"))?; + .map_err(|_| { + anyhow::Error::new(HarnessTransportClosed("ACP agent command channel closed")) + })?; response_rx .await - .map_err(|_| anyhow::anyhow!("ACP agent did not respond"))? + .map_err(|_| anyhow::Error::new(HarnessTransportClosed("ACP agent did not respond")))? } /// List the agent's known sessions via ACP `session/list`. @@ -562,10 +597,12 @@ impl HarnessHandle { let (response_tx, response_rx) = oneshot::channel(); self.command_tx .send(HarnessCommand::ListSessions { cwd, response_tx }) - .map_err(|_| anyhow::anyhow!("ACP agent command channel closed"))?; + .map_err(|_| { + anyhow::Error::new(HarnessTransportClosed("ACP agent command channel closed")) + })?; response_rx .await - .map_err(|_| anyhow::anyhow!("ACP agent did not respond"))? + .map_err(|_| anyhow::Error::new(HarnessTransportClosed("ACP agent did not respond")))? } /// Close (release) the active session via ACP `session/close`. @@ -573,10 +610,12 @@ impl HarnessHandle { let (response_tx, response_rx) = oneshot::channel(); self.command_tx .send(HarnessCommand::CloseSession { response_tx }) - .map_err(|_| anyhow::anyhow!("ACP agent command channel closed"))?; + .map_err(|_| { + anyhow::Error::new(HarnessTransportClosed("ACP agent command channel closed")) + })?; response_rx .await - .map_err(|_| anyhow::anyhow!("ACP agent did not respond"))? + .map_err(|_| anyhow::Error::new(HarnessTransportClosed("ACP agent did not respond")))? } /// Subscribe to the ordered public event stream as an additional bounded @@ -587,10 +626,12 @@ impl HarnessHandle { let (response_tx, response_rx) = oneshot::channel(); self.command_tx .send(HarnessCommand::SubscribeEvents { response_tx }) - .map_err(|_| anyhow::anyhow!("ACP agent command channel closed"))?; + .map_err(|_| { + anyhow::Error::new(HarnessTransportClosed("ACP agent command channel closed")) + })?; response_rx .await - .map_err(|_| anyhow::anyhow!("ACP agent did not respond")) + .map_err(|_| anyhow::Error::new(HarnessTransportClosed("ACP agent did not respond"))) } /// Flush the session transcript to disk and wait for the write barrier. @@ -798,8 +839,10 @@ pub(crate) fn launch_session_from( SessionStart::New => crate::context_timeline::ContextTimelineState::default(), }; let context_timeline = Arc::new(tokio::sync::RwLock::new(initial_timeline)); + let normal_shutdown = Arc::new(AtomicBool::new(false)); let handle = HarnessHandle { command_tx: agent_cmd_tx, + normal_shutdown: Arc::clone(&normal_shutdown), first_prompt_started: None, agent_response_observer: None, responder: Arc::from(DEFAULT_RESPONDER), @@ -902,6 +945,7 @@ pub(crate) fn launch_session_from( command = agent_cmd_rx.recv() => { match command { Some(HarnessCommand::Shutdown { response_tx, .. }) => { + normal_shutdown.store(true, Ordering::Release); let _ = response_tx.send(Ok(())); let _ = event_tx.send(SessionEvent::Nori(NoriEvent::SessionEnded( SessionEnded { @@ -982,6 +1026,7 @@ pub(crate) fn launch_session_from( } let backend_for_agent = Arc::clone(&backend); let event_fanout = event_tx.clone(); + let command_normal_shutdown = Arc::clone(&normal_shutdown); tokio::spawn(async move { loop { let cmd = match pending_commands.pop_front() { @@ -1006,8 +1051,10 @@ pub(crate) fn launch_session_from( child_grace, response_tx, } => { + command_normal_shutdown.store(true, Ordering::Release); let result = backend_for_agent.shutdown(child_grace).await; let shutdown_complete = result.is_ok(); + command_normal_shutdown.store(shutdown_complete, Ordering::Release); let _ = response_tx.send(result); if shutdown_complete { break; @@ -1122,8 +1169,10 @@ pub(crate) fn launch_session_from( let _ = response_tx.send(result); } HarnessCommand::CloseSession { response_tx } => { + command_normal_shutdown.store(true, Ordering::Release); let result = backend_for_agent.close_active_session().await; let close_complete = result.is_ok(); + command_normal_shutdown.store(close_complete, Ordering::Release); let _ = response_tx.send(result); if close_complete { break; @@ -1145,6 +1194,15 @@ pub(crate) fn launch_session_from( let mut recorder_stopped = false; while let Some(BackendEvent::Public(event)) = backend_event_rx.recv().await { let session_ended = matches!(event, SessionEvent::Nori(NoriEvent::SessionEnded(_))); + if matches!( + &event, + SessionEvent::Nori(NoriEvent::SessionEnded(SessionEnded { + reason: SessionEndReason::Shutdown | SessionEndReason::Closed, + .. + })) + ) { + normal_shutdown.store(true, Ordering::Release); + } if matches!(event, SessionEvent::Nori(NoriEvent::ReplayStarted(_))) { replaying = true; } @@ -1238,6 +1296,7 @@ mod tests { }); let handle = HarnessHandle { command_tx, + normal_shutdown: Arc::default(), first_prompt_started: None, agent_response_observer: None, responder: Arc::from(DEFAULT_RESPONDER), @@ -1268,6 +1327,7 @@ mod tests { let callback_count_for_handle = Arc::clone(&callback_count); let handle = HarnessHandle { command_tx, + normal_shutdown: Arc::default(), first_prompt_started: None, agent_response_observer: None, responder: Arc::from(DEFAULT_RESPONDER), diff --git a/nori-rs/tui/docs.md b/nori-rs/tui/docs.md index 9bddf45cf..862ce4399 100644 --- a/nori-rs/tui/docs.md +++ b/nori-rs/tui/docs.md @@ -732,11 +732,11 @@ exit requests immediate owned ACP process-group cleanup; cloud exit allows a short detach grace. The TUI exits when cleanup publishes `SessionEnded`, rather than using an independent timer that can abandon reaping. -Events entering the application are tagged with their session generation. -Candidate events are routed to the candidate widget until activation commits; -events for a parked session (see the multi-session view below) go to its -widget; after a session is replaced, events from other older generations are -discarded. Every path that replaces the active widget calls +Harness session-stream events entering the application carry their session +generation. Candidate stream events go to the candidate widget until +activation commits; events for a parked session (see the multi-session view +below) go to its widget. Stream events from replaced generations are discarded. +Every path that replaces the active widget calls `shutdown_current_conversation` and then `install_active_widget` (in [`session_setup.rs`](src/app/session_setup.rs) and [`session_view.rs`](src/app/session_view.rs)); a loop iteration is the @@ -885,6 +885,15 @@ App `InsertHistoryCell` and `ConsolidateAgentMessage` in `AppEvent::SessionHistory`. `App` files history from a parked widget in that session's own transcript and prints only the active session's history. +- **Action failures.** The same sender tags `HarnessActionFailed` with its + originating widget generation. [Routing](src/app/session_view.rs) uses the + owning active or parked widget's [error path](src/chatwidget/helpers.rs), + which flushes buffered cells before the error and preserves the separator + before the next agent message. An activating candidate's error is retained + for startup failure handling. Queued history events are checked again against + active, parked, or known candidate generations; removed or replaced + generations are dropped, including when removal happens after the error + queued its history cell. - **Background sessions.** Parked widgets keep receiving their `SessionEvent`s and commit ticks, so streams finish into their own transcript. A parked widget never writes the terminal title @@ -1065,7 +1074,12 @@ child. Parent notices identify both child and parent and do not open the child's permission popup in the parent. Child list labels put the parent identity before the task title so title truncation preserves attribution. Closed completed children keep inspectable history; ending their process must not be interpreted as exiting -Nori. +Nori. When [startup replay](src/chatwidget/event_handlers.rs) reaches a closed +child widget with no live handle, it applies the history metadata without +requesting custom prompts. If a retained handle instead races normal teardown, +[command submission](src/chatwidget/helpers.rs) suppresses only the harness's +classified expected transport closure. Real backend errors and unexpected +transport loss still follow the originating session's failure route. Result delivery is manager-owned and automatically prompts the parent. The terminal renders attributed status/result text; it neither approves the result diff --git a/nori-rs/tui/src/app/mod.rs b/nori-rs/tui/src/app/mod.rs index 9a6ff795d..aa1033417 100644 --- a/nori-rs/tui/src/app/mod.rs +++ b/nori-rs/tui/src/app/mod.rs @@ -253,6 +253,8 @@ mod session_overview; mod session_resume; mod session_setup; mod session_view; +#[cfg(test)] +pub(crate) use session_view::route_session_history_event; impl App { #[allow(clippy::too_many_arguments)] diff --git a/nori-rs/tui/src/app/session_view.rs b/nori-rs/tui/src/app/session_view.rs index 5ba7c97eb..419492b72 100644 --- a/nori-rs/tui/src/app/session_view.rs +++ b/nori-rs/tui/src/app/session_view.rs @@ -160,30 +160,21 @@ impl App { generation: SessionGeneration, event: AppEvent, ) -> Option<AppEvent> { - let Some(parked) = self.session_deck.parked_mut(generation) else { - return Some(event); + let candidate = match self.candidate_agent.as_mut() { + Some(super::CandidateAgent::Activating { + widget, + activation_error, + .. + }) => Some((widget.session_generation(), activation_error)), + _ => None, }; - match event { - AppEvent::InsertHistoryCell(cell) => { - let cell: Arc<dyn HistoryCell> = cell.into(); - parked.transcript.push(cell); - } - AppEvent::ConsolidateAgentMessage { source, cwd } => { - crate::transcript_reflow::consolidate_agent_message_cells( - &mut parked.transcript, - source, - &cwd, - ); - } - AppEvent::AgentConnecting { display_name } => { - parked.widget.show_connecting_status(&display_name); - } - AppEvent::AgentSpawnFailed { agent_name, error } => { - parked.widget.on_agent_spawn_failed(&agent_name, &error); - } - other => return Some(other), - } - None + route_session_history_event( + &mut self.chat_widget, + &mut self.session_deck, + candidate, + generation, + event, + ) } /// Hand a session event to the parked widget it belongs to. Returns the @@ -309,3 +300,57 @@ impl App { || self.primary_agent_preparation.is_some() } } + +pub(crate) fn route_session_history_event( + active: &mut ChatWidget, + deck: &mut super::session_deck::SessionDeck<Box<ChatWidget>>, + candidate: Option<(SessionGeneration, &mut Option<String>)>, + generation: SessionGeneration, + event: AppEvent, +) -> Option<AppEvent> { + if let AppEvent::HarnessActionFailed(message) = event { + // The widget flushes buffered user/tool cells before its error, retaining + // chronological order and the separator before the next agent message. + if active.session_generation() == generation { + active.add_error_message(message); + } else if let Some(parked) = deck.parked_mut(generation) { + parked.widget.add_error_message(message); + } else if let Some((candidate_generation, activation_error)) = candidate + && candidate_generation == generation + { + *activation_error = Some(message); + } + // Removed or replaced generations no longer own a transcript here. + return None; + } + let Some(parked) = deck.parked_mut(generation) else { + let known_candidate = + candidate.is_some_and(|(candidate_generation, _)| candidate_generation == generation); + return (active.session_generation() == generation || known_candidate).then_some(event); + }; + match event { + AppEvent::InsertHistoryCell(cell) => { + let cell: Arc<dyn HistoryCell> = cell.into(); + parked.transcript.push(cell); + } + AppEvent::ConsolidateAgentMessage { source, cwd } => { + crate::transcript_reflow::consolidate_agent_message_cells( + &mut parked.transcript, + source, + &cwd, + ); + } + AppEvent::AgentConnecting { display_name } => { + parked.widget.show_connecting_status(&display_name); + } + AppEvent::AgentSpawnFailed { agent_name, error } => { + parked.widget.on_agent_spawn_failed(&agent_name, &error); + } + other => return Some(other), + } + None +} + +#[cfg(test)] +#[path = "session_view_tests.rs"] +mod action_failure_tests; diff --git a/nori-rs/tui/src/app/session_view_tests.rs b/nori-rs/tui/src/app/session_view_tests.rs new file mode 100644 index 000000000..472e8cc38 --- /dev/null +++ b/nori-rs/tui/src/app/session_view_tests.rs @@ -0,0 +1,205 @@ +use super::super::session_deck::SessionDeck; +use super::*; + +fn rendered(event: AppEvent) -> String { + let AppEvent::InsertHistoryCell(cell) = event else { + panic!("expected history cell"); + }; + cell.display_lines(80) + .iter() + .map(std::string::ToString::to_string) + .collect::<Vec<_>>() + .join("\n") +} + +#[test] +fn action_failures_route_to_active_parked_candidate_and_discard_removed_generations() { + let (mut active, mut active_rx, _) = crate::chatwidget::tests::make_scoped_chatwidget_manual(); + let (parked, mut parked_rx, _) = crate::chatwidget::tests::make_scoped_chatwidget_manual(); + let (candidate, _, _) = crate::chatwidget::tests::make_scoped_chatwidget_manual(); + let active_generation = active.session_generation(); + let parked_generation = parked.session_generation(); + let candidate_generation = candidate.session_generation(); + let mut candidate_error = None; + let mut deck = SessionDeck::default(); + deck.park( + parked_generation, + ParkedSession { + widget: Box::new(parked), + transcript: Vec::new(), + }, + ); + route_session_history_event( + &mut active, + &mut deck, + None, + active_generation, + AppEvent::HarnessActionFailed("active failure".into()), + ); + for (generation, message) in [ + (parked_generation, "parked failure"), + (candidate_generation, "candidate failure"), + ] { + assert!( + route_session_history_event( + &mut active, + &mut deck, + Some((candidate_generation, &mut candidate_error)), + generation, + AppEvent::HarnessActionFailed(message.into()) + ) + .is_none(), + "another session's failure cannot render on screen" + ); + } + let mut visible = Vec::new(); + for rx in [&mut active_rx, &mut parked_rx] { + while let Ok(event) = rx.try_recv() { + if let AppEvent::SessionHistory { generation, event } = event + && let Some(event) = + route_session_history_event(&mut active, &mut deck, None, generation, *event) + { + visible.push(rendered(event)); + } + } + } + assert_eq!(visible, vec!["■ active failure"]); + let parked = deck.parked(parked_generation).unwrap(); + let text = parked + .transcript + .iter() + .flat_map(|cell| cell.display_lines(80)) + .map(|line| line.to_string()) + .collect::<Vec<_>>() + .join("\n"); + assert_eq!(text, "■ parked failure"); + assert_eq!(candidate_error.as_deref(), Some("candidate failure")); + deck.remove(parked_generation); + assert!( + route_session_history_event( + &mut active, + &mut deck, + None, + parked_generation, + AppEvent::HarnessActionFailed("removed failure".into()) + ) + .is_none() + ); +} +fn assert_failure_follows_buffered_message(parked_target: bool) { + let (mut active, mut active_rx, _) = crate::chatwidget::tests::make_scoped_chatwidget_manual(); + let (mut child, mut child_rx, _) = crate::chatwidget::tests::make_scoped_chatwidget_manual(); + let target = if parked_target { + &mut child + } else { + &mut active + }; + let generation = target.session_generation(); + target.handle_client_event(crate::presentation::ClientEvent::MessageDelta( + crate::presentation::MessageDelta { + stream: crate::presentation::MessageStream::User, + delta: "question before failure".into(), + message_id: Some("question-one".into()), + }, + )); + let mut deck = SessionDeck::default(); + if parked_target { + deck.park( + generation, + ParkedSession { + widget: Box::new(child), + transcript: Vec::new(), + }, + ); + } + let mut visible = Vec::new(); + if let Some(event) = route_session_history_event( + &mut active, + &mut deck, + None, + generation, + AppEvent::HarnessActionFailed("operation failed".into()), + ) { + visible.push(rendered(event)); + } + for rx in [&mut active_rx, &mut child_rx] { + while let Ok(event) = rx.try_recv() { + if let AppEvent::SessionHistory { generation, event } = event + && let Some(event) = + route_session_history_event(&mut active, &mut deck, None, generation, *event) + { + visible.push(rendered(event)); + } + } + } + let text = if parked_target { + assert!( + visible.is_empty(), + "parked history cannot appear in the active session" + ); + deck.parked(generation) + .unwrap() + .transcript + .iter() + .flat_map(|cell| cell.display_lines(80)) + .map(|line| line.to_string()) + .collect::<Vec<_>>() + .join("\n") + } else { + visible.join("\n") + }; + let question = text + .find("question before failure") + .expect("buffered user message is flushed"); + let error = text.find("operation failed").expect("real error renders"); + assert!(question < error, "chronological history: {text}"); +} + +#[test] +fn active_action_failure_preserves_buffered_message_order() { + assert_failure_follows_buffered_message(false); +} + +#[test] +fn parked_action_failure_preserves_buffered_message_order() { + assert_failure_follows_buffered_message(true); +} + +#[test] +fn history_queued_by_removed_generation_is_discarded() { + let (mut active, _, _) = crate::chatwidget::tests::make_chatwidget_manual(); + let (removed, _, _) = crate::chatwidget::tests::make_chatwidget_manual(); + let event = AppEvent::InsertHistoryCell(Box::new(crate::history_cell::new_error_event( + "late failure".into(), + ))); + assert!( + route_session_history_event( + &mut active, + &mut Default::default(), + None, + removed.session_generation(), + event + ) + .is_none() + ); +} + +#[test] +fn candidate_startup_history_retains_its_routing() { + let (mut active, _, _) = crate::chatwidget::tests::make_chatwidget_manual(); + let (candidate, _, _) = crate::chatwidget::tests::make_chatwidget_manual(); + let generation = candidate.session_generation(); + let mut error = None; + assert!(matches!( + route_session_history_event( + &mut active, + &mut Default::default(), + Some((generation, &mut error)), + generation, + AppEvent::AgentConnecting { + display_name: "candidate".into() + } + ), + Some(AppEvent::AgentConnecting { .. }) + )); +} diff --git a/nori-rs/tui/src/app_event_sender.rs b/nori-rs/tui/src/app_event_sender.rs index 91b8bbd16..92501e4c9 100644 --- a/nori-rs/tui/src/app_event_sender.rs +++ b/nori-rs/tui/src/app_event_sender.rs @@ -43,7 +43,8 @@ impl AppEventSender { event @ (AppEvent::InsertHistoryCell(_) | AppEvent::ConsolidateAgentMessage { .. } | AppEvent::AgentConnecting { .. } - | AppEvent::AgentSpawnFailed { .. }), + | AppEvent::AgentSpawnFailed { .. } + | AppEvent::HarnessActionFailed(_)), ) => AppEvent::SessionHistory { generation, event: Box::new(event), diff --git a/nori-rs/tui/src/chatwidget/event_handlers.rs b/nori-rs/tui/src/chatwidget/event_handlers.rs index 20647c92f..184b77c48 100644 --- a/nori-rs/tui/src/chatwidget/event_handlers.rs +++ b/nori-rs/tui/src/chatwidget/event_handlers.rs @@ -373,7 +373,16 @@ impl ChatWidget { fn on_session_started(&mut self, event: nori_protocol::SessionStarted) { self.session_configured_received = true; - self.submit_harness_action(crate::app_event::HarnessAction::LoadCustomPrompts); + // A completed child's replay has no live process to load prompts from. + let historical_child = self.delegated_child + && self.harness_handle.is_none() + && self + .managed_conversation_id + .and_then(|id| self.session_manager.get(&id)) + .is_some_and(|record| record.status == nori_protocol::SessionStatus::Closed); + if !historical_child { + self.submit_harness_action(crate::app_event::HarnessAction::LoadCustomPrompts); + } self.bottom_pane.hide_status_indicator(); self.update_approval_mode_label(); self.bottom_pane.set_history_metadata( diff --git a/nori-rs/tui/src/chatwidget/helpers.rs b/nori-rs/tui/src/chatwidget/helpers.rs index e0f5e5b7e..293d06d91 100644 --- a/nori-rs/tui/src/chatwidget/helpers.rs +++ b/nori-rs/tui/src/chatwidget/helpers.rs @@ -400,7 +400,9 @@ impl ChatWidget { response, } => handle.respond_to_agent(request_id, *response).await, }; - if let Err(error) = result { + if let Err(error) = result + && !handle.is_expected_shutdown_error(&error) + { app_event_tx.send(AppEvent::HarnessActionFailed(error.to_string())); } }); @@ -415,7 +417,9 @@ impl ChatWidget { }; let app_event_tx = self.app_event_tx.clone(); tokio::spawn(async move { - if let Err(error) = handle.prompt(content).await { + if let Err(error) = handle.prompt(content).await + && !handle.is_expected_shutdown_error(&error) + { app_event_tx.send(AppEvent::HarnessActionFailed(error.to_string())); } }); diff --git a/nori-rs/tui/src/chatwidget/tests/action_failures.rs b/nori-rs/tui/src/chatwidget/tests/action_failures.rs new file mode 100644 index 000000000..6378a9157 --- /dev/null +++ b/nori-rs/tui/src/chatwidget/tests/action_failures.rs @@ -0,0 +1,228 @@ +use super::*; +use crate::app_event::HarnessAction; +use nori_harness::session_manager::ManagedSession; +use nori_harness::session_manager::SessionAgent; +use nori_harness::session_manager::SessionSource; +use nori_harness::session_manager::StartSession; +use nori_protocol::NoriEvent; +use nori_protocol::SessionEvent; +use std::sync::Arc; +use std::time::Duration; + +async fn started_session( + chat: &mut ChatWidget, + home: &std::path::Path, +) -> (ManagedSession, SessionEvent) { + let mut session = chat + .session_manager + .start(StartSession { + agent: SessionAgent::Prepare(nori_harness::runtime::AgentPrepareSpec { + config: Arc::new(nori_config::NoriConfig { + active_agent: "mock-model".into(), + cwd: home.into(), + nori_home: home.into(), + multi_session: true, + ..Default::default() + }), + cli_version: "action-failure-test".into(), + session_context: None, + initial_context: None, + }), + start: nori_harness::runtime::SessionStart::New, + parent: None, + source: SessionSource::User, + }) + .expect("start real mock process"); + let started = tokio::time::timeout(Duration::from_secs(15), async { + loop { + let event = session.events.recv().await.expect("session stream").event; + if matches!(event, SessionEvent::Nori(NoriEvent::SessionStarted(_))) { + break event; + } + } + }) + .await + .expect("mock starts"); + chat.managed_conversation_id = Some(session.conversation_id); + chat.harness_handle = Some(session.handle.clone()); + (session, started) +} + +async fn close_session(session: &mut ManagedSession) { + session.handle.shutdown().await.expect("normal shutdown"); + tokio::time::timeout(Duration::from_secs(15), async { + while session.events.recv().await.is_some() {} + }) + .await + .expect("process and event stream close"); +} + +fn action_error(event: &AppEvent) -> bool { + match event { + AppEvent::HarnessActionFailed(_) => true, + AppEvent::SessionHistory { event, .. } => action_error(event), + _ => false, + } +} + +// Replay can arrive after the manager releases the child process. Both adoption +// schedules (retained old handle and no live handle) must remain quiet. +async fn completed_child_replay(keep_handle: bool) { + let home = tempfile::tempdir().unwrap(); + let (mut chat, mut rx, _) = make_chatwidget_manual(); + let (mut session, started) = started_session(&mut chat, home.path()).await; + close_session(&mut session).await; + assert!( + chat.session_manager + .handle(&session.conversation_id) + .is_none() + ); + assert!( + session.handle.custom_prompts().await.is_err(), + "closed transport reproduces the race" + ); + chat.delegated_child = true; + if !keep_handle { + chat.harness_handle = None; + } + while rx.try_recv().is_ok() {} + chat.handle_session_event(chat.session_generation(), started); + // Closing the widget's sender leaves only actual action tasks owning + // senders. EOF is the deterministic task-completion barrier. + drop(chat); + tokio::time::timeout(Duration::from_secs(15), async { + while let Some(event) = rx.recv().await { + assert!( + !action_error(&event), + "successful child teardown emitted {event:?}; retained handle={keep_handle}" + ); + } + }) + .await + .expect("all action tasks finish"); +} + +#[tokio::test] +async fn completed_child_retained_handle_replay_does_not_report_action_failure() { + completed_child_replay(true).await; +} + +#[tokio::test] +async fn completed_child_without_handle_replay_does_not_report_action_failure() { + completed_child_replay(false).await; +} + +#[tokio::test] +async fn visible_session_real_action_failure_is_rendered() { + let home = tempfile::tempdir().unwrap(); + let (mut chat, mut rx, _) = make_chatwidget_manual(); + let (mut session, _) = started_session(&mut chat, home.path()).await; + while rx.try_recv().is_ok() {} + let session_generation = chat.session_generation(); + chat.app_event_tx = chat.app_event_tx.for_session(session_generation); + chat.submit_harness_action(HarnessAction::Branch); + let message = tokio::time::timeout(Duration::from_secs(15), async { + loop { + match rx.recv().await.expect("action event") { + AppEvent::SessionHistory { generation, event } + if generation == session_generation => + { + if let AppEvent::HarnessActionFailed(message) = *event { + break message; + } + } + _ => {} + } + } + }) + .await + .expect("real failed operation remains visible"); + let mut deck = Default::default(); + crate::app::route_session_history_event( + &mut chat, + &mut deck, + None, + session_generation, + AppEvent::HarnessActionFailed(message), + ); + let mut text = String::new(); + while let Ok(event) = rx.try_recv() { + if let AppEvent::SessionHistory { generation, event } = event + && let Some(AppEvent::InsertHistoryCell(cell)) = crate::app::route_session_history_event( + &mut chat, &mut deck, None, generation, *event, + ) + { + text.push_str(&lines_to_single_string(&cell.display_lines(80))); + } + } + assert!( + !text.trim().is_empty(), + "visible failure must produce history" + ); + insta::assert_snapshot!("visible_session_action_failure", text); + close_session(&mut session).await; +} + +#[tokio::test] +async fn shutdown_classification_preserves_real_backend_failures() { + let home = tempfile::tempdir().unwrap(); + let (mut chat, _, _) = make_chatwidget_manual(); + let (mut session, _) = started_session(&mut chat, home.path()).await; + let backend_error = session + .handle + .branch() + .await + .expect_err("unsupported branch is a real failure"); + assert!(!session.handle.is_expected_shutdown_error(&backend_error)); + close_session(&mut session).await; + assert!( + !session.handle.is_expected_shutdown_error(&backend_error), + "shutdown cannot erase an already returned real error" + ); + let transport_error = session + .handle + .custom_prompts() + .await + .expect_err("closed transport"); + assert!(session.handle.is_expected_shutdown_error(&transport_error)); +} + +#[tokio::test] +async fn crashed_session_action_failure_is_not_normal_shutdown() { + let home = tempfile::tempdir().unwrap(); + let (mut chat, _, _) = make_chatwidget_manual(); + let (mut session, _) = started_session(&mut chat, home.path()).await; + session + .handle + .prompt(vec![nori_protocol::acp::v1::ContentBlock::Text( + nori_protocol::acp::v1::TextContent::new("mock:exit-during-prompt"), + )]) + .await + .expect("submit crash marker"); + tokio::time::timeout(Duration::from_secs(15), async { + loop { + let event = session.events.recv().await.expect("crash event").event; + if matches!(event, SessionEvent::Nori(NoriEvent::SessionEnded(_))) { + break; + } + } + }) + .await + .expect("process reports its crash"); + let error = tokio::time::timeout( + Duration::from_secs(15), + session.handle.prompt_with_meta_if_idle( + vec![nori_protocol::acp::v1::ContentBlock::Text( + nori_protocol::acp::v1::TextContent::new("after crash"), + )], + None, + ), + ) + .await + .expect("failed process answers action") + .expect_err("crashed transport"); + assert!( + !session.handle.is_expected_shutdown_error(&error), + "a crash must remain a user-facing failure" + ); +} diff --git a/nori-rs/tui/src/chatwidget/tests/mod.rs b/nori-rs/tui/src/chatwidget/tests/mod.rs index 596a21757..1ddf1b15b 100644 --- a/nori-rs/tui/src/chatwidget/tests/mod.rs +++ b/nori-rs/tui/src/chatwidget/tests/mod.rs @@ -125,8 +125,19 @@ fn make_chatwidget_manual_for_mode( (widget, event_rx, unused_rx) } +mod action_failures; mod agent_status; mod context; mod delegation; mod part10; mod part8; + +pub(crate) fn make_scoped_chatwidget_manual() -> ( + ChatWidget, + tokio::sync::mpsc::UnboundedReceiver<AppEvent>, + tokio::sync::mpsc::UnboundedReceiver<()>, +) { + let (mut widget, rx, op_rx) = make_chatwidget_manual(); + widget.app_event_tx = widget.app_event_tx.for_session(widget.session_generation()); + (widget, rx, op_rx) +} diff --git a/nori-rs/tui/src/chatwidget/tests/snapshots/nori_tui__chatwidget__tests__action_failures__visible_session_action_failure.snap b/nori-rs/tui/src/chatwidget/tests/snapshots/nori_tui__chatwidget__tests__action_failures__visible_session_action_failure.snap new file mode 100644 index 000000000..8caa9d7a4 --- /dev/null +++ b/nori-rs/tui/src/chatwidget/tests/snapshots/nori_tui__chatwidget__tests__action_failures__visible_session_action_failure.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/chatwidget/tests/action_failures.rs +expression: text +--- +■ This agent does not support branching From 290fdfcd6a9368861076e62632128c49dc692b98 Mon Sep 17 00:00:00 2001 From: Nori <contact@tilework.tech> Date: Sun, 15 Nov 2026 20:59:40 +0000 Subject: [PATCH 5/6] fix(delegation): report a crashed child in one plain line A child that crashed during a prompt sent its parent the Debug dump of the RequestFailure, with escaped newlines and the agent's stderr tail. Keep only the failure's first line, such as "Agent process exited unexpectedly (exit code 17)."; the stderr tail stays in the child's own history. --- nori-rs/harness/src/session_manager/delegation_state.rs | 9 ++++++++- nori-rs/harness/tests/delegation.rs | 6 ++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/nori-rs/harness/src/session_manager/delegation_state.rs b/nori-rs/harness/src/session_manager/delegation_state.rs index 83e86e4cc..235cc6cbe 100644 --- a/nori-rs/harness/src/session_manager/delegation_state.rs +++ b/nori-rs/harness/src/session_manager/delegation_state.rs @@ -121,7 +121,14 @@ impl DelegationState { } } SessionEvent::Nori(NoriEvent::RequestFailed(failure)) => { - entry.failure = Some(format!("{failure:?}")) + // Only the headline reaches the parent; the stderr tail stays + // in the child's own history. + entry.failure = failure + .message + .lines() + .map(str::trim) + .find(|line| !line.is_empty()) + .map(str::to_owned) } _ => {} } diff --git a/nori-rs/harness/tests/delegation.rs b/nori-rs/harness/tests/delegation.rs index 769eb7e47..248e242a4 100644 --- a/nori-rs/harness/tests/delegation.rs +++ b/nori-rs/harness/tests/delegation.rs @@ -409,10 +409,16 @@ async fn child_crash_returns_failure_without_failing_parent() { .unwrap(); let done = terminal(&m, &p.conversation_id, &d.request_id).await; assert_eq!(done.status, DelegationStatus::Failed); + assert_eq!( + done.result.as_deref(), + Some("Agent process exited unexpectedly (exit code 17).") + ); assert!(m.handle(&p.conversation_id).is_some()); let reply = wait_parent_reply(&mut p, &d.child_id).await; assert!(reply.contains(&d.parent_id)); assert!(reply.contains("Failed")); + assert!(reply.contains("Agent process exited unexpectedly (exit code 17).")); + assert!(!reply.contains("RequestFailure"), "{reply}"); m.shutdown(Duration::ZERO).await; } #[tokio::test] From e61d10ecf85a577da4513adedfd800180156c748 Mon Sep 17 00:00:00 2001 From: Nori <contact@tilework.tech> Date: Sun, 15 Nov 2026 21:40:58 +0000 Subject: [PATCH 6/6] fix(delegation): keep a failed child failed when its error has no text --- .../src/session_manager/delegation_state.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/nori-rs/harness/src/session_manager/delegation_state.rs b/nori-rs/harness/src/session_manager/delegation_state.rs index 235cc6cbe..c5cef2aa3 100644 --- a/nori-rs/harness/src/session_manager/delegation_state.rs +++ b/nori-rs/harness/src/session_manager/delegation_state.rs @@ -123,12 +123,15 @@ impl DelegationState { SessionEvent::Nori(NoriEvent::RequestFailed(failure)) => { // Only the headline reaches the parent; the stderr tail stays // in the child's own history. - entry.failure = failure - .message - .lines() - .map(str::trim) - .find(|line| !line.is_empty()) - .map(str::to_owned) + entry.failure = Some( + failure + .message + .lines() + .map(str::trim) + .find(|line| !line.is_empty()) + .unwrap_or("Child agent request failed") + .to_owned(), + ) } _ => {} }