diff --git a/ROADMAP.md b/ROADMAP.md index 2abb7a1..6c6ddfd 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -358,8 +358,22 @@ geometry) with drag-resize split out to #55 (blocked-by #54; feature-torture round-trip) and single-sourcing the G1 dump (#108) onto this richer model (#216) are follow-ups. Depends on #193; unblocks a verifiable act→observe loop for #194 - - [ ] `F-mcp-orchestration` (#194) — open/split/focus/rename/run-in-session; - depends on #193 + - [x] `F-mcp-orchestration` (#194) — **the action rung.** Six mutating tools — + `open_session`, `split_pane`, `focus_pane`, `rename_tab`, `close_pane`, + `run_in_session` — each a thin wrapper over an existing `core::App` event + (never a new state path, the #90 constraint). `core` is untouched: the + bridge grows an `Act(Action)` request + `ActionOutcome` reply, and the shell + adapter (which owns `core::App` **and** the one effect executor) resolves the + stable handle, applies the event(s) through `App::apply`, performs the + effects, and reports the **resulting focused handle** — so an agent gets + act→observe in one round trip. A handle no open pane hosts / an out-of-range + tab is rejected before any state is touched (surfaced as `invalid_params`); + every call stays `tokio::timeout`-bounded and apply-and-read (Q7). Handles + are strings, matching `list_sessions`/`snapshot`, and address a pane **in + any tab** — `Event::RevealPane` activates the owning tab first, since + click-to-focus (`FocusPane`) only reaches the active one and a silent no-op + there would let a close destroy the wrong terminal. Depends on #193; with #212 + (perception) this closes the act→observe loop - [ ] `F-mcp-terminal-sync` (#195) — `wait_for_status` (OSC) + `read_terminal`; depends on #193 - [ ] `F-mcp-agent-loop` (#196) — `type_into_terminal` + prompt→wait→read, diff --git a/crates/app/src/mcp/handler.rs b/crates/app/src/mcp/handler.rs index c5503b0..1440b98 100644 --- a/crates/app/src/mcp/handler.rs +++ b/crates/app/src/mcp/handler.rs @@ -19,9 +19,10 @@ use rmcp::{ tool, tool_handler, tool_router, }; use serde::{Deserialize, Serialize}; +use termherd_core::workspace::SplitDir; use termherd_core::{Section, SessionStatus, SnapshotFilter, TerminalScope, WorkspaceSnapshot}; -use crate::shell::bridge::{BridgeHandle, Reply, Request, SessionInfo, SessionKind}; +use crate::shell::bridge::{Action, BridgeHandle, Reply, Request, SessionInfo, SessionKind}; /// How long a tool waits for the shell to answer before failing the caller. /// Bounds the whole round-trip (enqueue + reply) via [`BridgeHandle::call`], so @@ -108,6 +109,155 @@ impl TermherdMcp { .map_err(|error| ErrorData::internal_error(error.to_string(), None))?; Ok(CallToolResult::structured(value)) } + + /// Open a new terminal session and focus it. → `open_session`. + #[tool( + name = "open_session", + description = "Open a new terminal session and focus it. Args: `project` \ + (working directory; omit for the home dir), `kind` \ + (\"shell\" or \"claude\", default \"shell\"). Returns the \ + new session's `focused_handle`." + )] + async fn open_session( + &self, + Parameters(args): Parameters, + ) -> Result { + let action = Action::Open { + project: args.project, + kind: match args.kind.as_deref() { + None | Some("shell") => SessionKind::Shell, + Some("claude") => SessionKind::Claude, + Some(other) => { + return Err(ErrorData::invalid_params( + format!("unknown kind {other:?}; expected \"shell\" or \"claude\""), + None, + )); + } + }, + }; + self.act(action).await + } + + /// Split a pane, opening a fresh session beside it. → `split_pane`. + #[tool( + name = "split_pane", + description = "Split a pane, opening a fresh shell beside it and focusing \ + it. Args: `direction` (\"vertical\" = side by side, \ + \"horizontal\" = stacked; default \"vertical\"), `pane` \ + (session handle to split; omit for the focused pane). \ + Returns the new pane's `focused_handle`." + )] + async fn split_pane( + &self, + Parameters(args): Parameters, + ) -> Result { + let dir = match args.direction.as_deref() { + None | Some("vertical") | Some("v") => SplitDir::Vertical, + Some("horizontal") | Some("h") => SplitDir::Horizontal, + Some(other) => { + return Err(ErrorData::invalid_params( + format!("unknown direction {other:?}; expected \"vertical\" or \"horizontal\""), + None, + )); + } + }; + let pane = parse_optional_handle(args.pane)?; + self.act(Action::Split { pane, dir }).await + } + + /// Move focus to the pane hosting a session. → `focus_pane`. + #[tool( + name = "focus_pane", + description = "Move focus to the pane hosting a session, and hand it the \ + keyboard. Args: `session` (its stable handle). Returns the \ + resulting `focused_handle`." + )] + async fn focus_pane( + &self, + Parameters(args): Parameters, + ) -> Result { + let session = parse_handle(&args.session)?; + self.act(Action::Focus { session }).await + } + + /// Give a tab a manual name. → `rename_tab`. + #[tool( + name = "rename_tab", + description = "Give the tab at `tab` (0-based index, as `snapshot` reports \ + tab order) a manual name. A blank `title` reverts to the \ + derived title. Returns the current `focused_handle`." + )] + async fn rename_tab( + &self, + Parameters(args): Parameters, + ) -> Result { + self.act(Action::Rename { + tab: args.tab, + title: args.title, + }) + .await + } + + /// Close a pane; a lone pane closes its whole tab. → `close_pane`. + #[tool( + name = "close_pane", + description = "Close a pane and kill its terminal. Args: `pane` (session \ + handle to close; omit for the focused pane). A lone pane is \ + its whole tab, which closes. Returns the resulting \ + `focused_handle` (null when the workspace is now empty)." + )] + async fn close_pane( + &self, + Parameters(args): Parameters, + ) -> Result { + let pane = parse_optional_handle(args.pane)?; + self.act(Action::Close { pane }).await + } + + /// Type text into a session's terminal without waiting. → `run_in_session`. + #[tool( + name = "run_in_session", + description = "Type text into a session's terminal, as if typed at the \ + keyboard — does not wait for it to finish (read the result \ + with `snapshot`'s scoped terminal text). Include a trailing \ + newline in `text` to submit a command. Args: `session` (its \ + stable handle), `text`. Returns the `focused_handle`." + )] + async fn run_in_session( + &self, + Parameters(args): Parameters, + ) -> Result { + let session = parse_handle(&args.session)?; + self.act(Action::Run { + session, + bytes: args.text.into_bytes(), + }) + .await + } +} + +impl TermherdMcp { + /// Send an [`Action`] over the bridge and shape its [`ActionOutcome`] into a + /// tool result: a rejection (unknown handle / out-of-range tab) becomes an + /// `invalid_params` error, an applied action a `{ focused_handle }` object. + async fn act(&self, action: Action) -> Result { + let reply = self + .bridge + .call(Request::Act(action), CALL_TIMEOUT) + .await + .map_err(|error| ErrorData::internal_error(error.to_string(), None))?; + let Reply::Acted(outcome) = reply else { + return Err(ErrorData::internal_error( + "bridge answered the wrong reply kind", + None, + )); + }; + if let Some(reason) = outcome.error { + return Err(ErrorData::invalid_params(reason, None)); + } + let value = serde_json::json!({ "focused_handle": outcome.focused }); + Ok(CallToolResult::structured(value)) + } } #[tool_handler(router = self.tool_router)] @@ -221,6 +371,80 @@ impl SnapshotArgs { } } +/// Arguments for `open_session`. +#[derive(Debug, Default, Deserialize, JsonSchema)] +#[schemars(crate = "rmcp::schemars")] +struct OpenArgs { + /// Working directory for the new session. Omit for the home directory. + #[serde(default)] + project: Option, + /// `"shell"` (default) or `"claude"`. + #[serde(default)] + kind: Option, +} + +/// Arguments for `split_pane`. +#[derive(Debug, Default, Deserialize, JsonSchema)] +#[schemars(crate = "rmcp::schemars")] +struct SplitArgs { + /// `"vertical"` (default, side by side) or `"horizontal"` (stacked). + #[serde(default)] + direction: Option, + /// Session handle to split. Omit for the focused pane. + #[serde(default)] + pane: Option, +} + +/// Arguments for `focus_pane`. +#[derive(Debug, Default, Deserialize, JsonSchema)] +#[schemars(crate = "rmcp::schemars")] +struct FocusArgs { + /// The stable handle of the session to focus. + session: String, +} + +/// Arguments for `rename_tab`. +#[derive(Debug, Default, Deserialize, JsonSchema)] +#[schemars(crate = "rmcp::schemars")] +struct RenameArgs { + /// 0-based tab index, as `snapshot` reports tab order. + tab: usize, + /// The new manual title; blank reverts to the derived title. + title: String, +} + +/// Arguments for `close_pane`. +#[derive(Debug, Default, Deserialize, JsonSchema)] +#[schemars(crate = "rmcp::schemars")] +struct CloseArgs { + /// Session handle to close. Omit for the focused pane. + #[serde(default)] + pane: Option, +} + +/// Arguments for `run_in_session`. +#[derive(Debug, Default, Deserialize, JsonSchema)] +#[schemars(crate = "rmcp::schemars")] +struct RunArgs { + /// The stable handle of the target session. + session: String, + /// Text to type; include a trailing newline to submit a command. + text: String, +} + +/// Parse a stable handle string (as `list_sessions` / `snapshot` report it) to +/// its numeric id, surfacing a non-numeric handle as an `invalid_params` error. +fn parse_handle(handle: &str) -> Result { + handle + .parse() + .map_err(|_| ErrorData::invalid_params(format!("handle {handle:?} is not a number"), None)) +} + +/// Parse an optional handle: absent stays absent, present is validated. +fn parse_optional_handle(handle: Option) -> Result, ErrorData> { + handle.map(|h| parse_handle(&h)).transpose() +} + /// The on-the-wire snapshot: `core`'s model flattened to string-typed enums an /// MCP client reads without knowing termherd's internals. Absent sections and /// empty terminal text are omitted, keeping a light call light. @@ -527,4 +751,234 @@ mod tests { .expect_err("a closed bridge is a tool error"); assert!(error.message.contains("closed"), "got: {}", error.message); } + + // ---- Orchestration tools (F-mcp-orchestration) ---------------------- + + use crate::shell::bridge::{Action, ActionOutcome}; + + /// Run `call` against a shell that records the request and answers it with an + /// applied outcome; return the `Action` the tool built. + async fn action_of(call: F) -> Action + where + F: FnOnce(TermherdMcp) -> Fut, + Fut: std::future::Future>, + { + let (handle, requests) = channel(); + let shell = spawn_test_shell( + requests, + Reply::Acted(ActionOutcome::applied(Some("2".into()))), + ); + call(TermherdMcp::new(handle)) + .await + .expect("the tool returns a result"); + match shell.await.expect("shell task") { + Request::Act(action) => action, + other => panic!("expected an action request, got {other:?}"), + } + } + + #[tokio::test] + async fn open_session_tool_builds_an_open_action() { + let action = action_of(|mcp| async move { + mcp.open_session(Parameters(OpenArgs { + project: Some("/proj".into()), + kind: Some("claude".into()), + })) + .await + }) + .await; + assert_eq!( + action, + Action::Open { + project: Some("/proj".into()), + kind: SessionKind::Claude, + } + ); + } + + #[tokio::test] + async fn open_session_tool_defaults_kind_to_shell() { + let action = + action_of(|mcp| async move { mcp.open_session(Parameters(OpenArgs::default())).await }) + .await; + assert_eq!( + action, + Action::Open { + project: None, + kind: SessionKind::Shell, + } + ); + } + + #[tokio::test] + async fn open_session_tool_rejects_an_unknown_kind() { + let (handle, requests) = channel(); + drop(requests); // never reached: the arg is rejected before the bridge. + let error = TermherdMcp::new(handle) + .open_session(Parameters(OpenArgs { + project: None, + kind: Some("wizard".into()), + })) + .await + .expect_err("an unknown kind is an invalid_params error"); + assert!(error.message.contains("wizard"), "got: {}", error.message); + } + + #[tokio::test] + async fn split_pane_tool_maps_direction_and_target() { + let action = action_of(|mcp| async move { + mcp.split_pane(Parameters(SplitArgs { + direction: Some("horizontal".into()), + pane: Some("5".into()), + })) + .await + }) + .await; + assert_eq!( + action, + Action::Split { + pane: Some(5), + dir: SplitDir::Horizontal, + } + ); + } + + #[tokio::test] + async fn split_pane_tool_defaults_to_a_vertical_split_of_the_focused_pane() { + let action = + action_of(|mcp| async move { mcp.split_pane(Parameters(SplitArgs::default())).await }) + .await; + assert_eq!( + action, + Action::Split { + pane: None, + dir: SplitDir::Vertical, + } + ); + } + + #[tokio::test] + async fn focus_and_run_tools_carry_the_parsed_handle() { + let focus = action_of(|mcp| async move { + mcp.focus_pane(Parameters(FocusArgs { + session: "3".into(), + })) + .await + }) + .await; + assert_eq!(focus, Action::Focus { session: 3 }); + + let run = action_of(|mcp| async move { + mcp.run_in_session(Parameters(RunArgs { + session: "3".into(), + text: "ls\n".into(), + })) + .await + }) + .await; + assert_eq!( + run, + Action::Run { + session: 3, + bytes: b"ls\n".to_vec(), + } + ); + } + + #[tokio::test] + async fn run_in_session_tool_rejects_a_non_numeric_handle() { + let (handle, requests) = channel(); + drop(requests); + let error = TermherdMcp::new(handle) + .run_in_session(Parameters(RunArgs { + session: "not-a-number".into(), + text: "x".into(), + })) + .await + .expect_err("a non-numeric handle is rejected before the bridge"); + assert!( + error.message.contains("not-a-number"), + "got: {}", + error.message + ); + } + + #[tokio::test] + async fn rename_and_close_tools_build_their_actions() { + let rename = action_of(|mcp| async move { + mcp.rename_tab(Parameters(RenameArgs { + tab: 1, + title: "build".into(), + })) + .await + }) + .await; + assert_eq!( + rename, + Action::Rename { + tab: 1, + title: "build".into(), + } + ); + + let close = action_of(|mcp| async move { + mcp.close_pane(Parameters(CloseArgs { + pane: Some("4".into()), + })) + .await + }) + .await; + assert_eq!(close, Action::Close { pane: Some(4) }); + } + + #[tokio::test] + async fn a_tool_surfaces_a_rejected_action_as_invalid_params() { + // The shell answers with a rejection (unknown handle); the tool must + // relay it as an error, not a success with a null handle. + let (handle, requests) = channel(); + let _shell = spawn_test_shell( + requests, + Reply::Acted(ActionOutcome::rejected("no live session with handle 9")), + ); + let error = TermherdMcp::new(handle) + .focus_pane(Parameters(FocusArgs { + session: "9".into(), + })) + .await + .expect_err("a rejected action is a tool error"); + assert!( + error.message.contains("handle 9"), + "the rejection reason is relayed, got: {}", + error.message + ); + } + + #[tokio::test] + async fn a_tool_reports_the_resulting_focus_on_success() { + let (handle, requests) = channel(); + let _shell = spawn_test_shell( + requests, + Reply::Acted(ActionOutcome::applied(Some("2".into()))), + ); + let result = TermherdMcp::new(handle) + .split_pane(Parameters(SplitArgs::default())) + .await + .expect("the tool returns a result"); + let value = result.structured_content.expect("structured json content"); + assert_eq!( + value["focused_handle"], "2", + "the new pane's handle is returned" + ); + } + + #[tokio::test] + async fn a_tool_surfaces_a_bridge_failure_as_an_error() { + let (handle, requests) = channel(); + drop(requests); + let error = TermherdMcp::new(handle) + .close_pane(Parameters(CloseArgs::default())) + .await + .expect_err("a closed bridge is a tool error"); + assert!(error.message.contains("closed"), "got: {}", error.message); + } } diff --git a/crates/app/src/shell.rs b/crates/app/src/shell.rs index 327c335..1430913 100644 --- a/crates/app/src/shell.rs +++ b/crates/app/src/shell.rs @@ -40,6 +40,7 @@ mod geometry; mod ime; mod input; mod launch; +mod orchestrate; mod record; mod routing; mod session_ops; @@ -1157,11 +1158,21 @@ impl Shell { } Message::RecordFrameTick(now) => self.on_record_frame_tick(now), Message::RecordFrame(screenshot) => self.record.on_frame(screenshot), - Message::Bridge { request, reply } => { - let inputs = self.snapshot_inputs(&request); - reply.answer(bridge::respond(&self.core, &request, &inputs)); - Task::none() - } + Message::Bridge { request, reply } => match request { + // Actions mutate, so they can't answer off a `&App`: apply them + // and perform the effects here, where the shell owns both. + bridge::Request::Act(action) => { + let (outcome, task) = self.perform_action(action); + reply.answer(bridge::Reply::Acted(outcome)); + task + } + // The two read requests answer straight from owned state. + read => { + let inputs = self.snapshot_inputs(&read); + reply.answer(bridge::respond(&self.core, &read, &inputs)); + Task::none() + } + }, } } @@ -1436,6 +1447,305 @@ mod key_routing { assert!(bare.terminals.is_empty(), "no terminal scope → no text"); } + // ---- Orchestration actions (F-mcp-orchestration) -------------------- + // + // Each MCP action is a thin wrapper over an existing core event. These + // pin the shell-side seam: handle resolution, the applied mutation, and + // the reported resulting focus. + + // `Action`/`SessionKind` already name the `termherd_core` types in this + // module, so the bridge's carry a `Bridge` prefix here. + use super::bridge::{Action as BridgeAction, SessionKind as BridgeKind}; + use termherd_core::workspace::SplitDir; + + /// The focused session's handle string, or `None` when nothing is focused. + fn focused(shell: &Shell) -> Option { + shell + .core + .workspace + .focused_session() + .map(|id| id.0.get().to_string()) + } + + #[test] + fn open_action_launches_and_focuses_a_new_pane() { + let pty = Arc::new(RecordingPty::default()); + let mut shell = shell_over(pty.clone()); + let (outcome, _task) = shell.perform_action(BridgeAction::Open { + project: Some("/tmp/x".into()), + kind: BridgeKind::Shell, + }); + assert_eq!(outcome.error, None, "opening a session never rejects"); + assert_eq!( + outcome.focused, + focused(&shell), + "the outcome reports the newly focused pane" + ); + assert!(outcome.focused.is_some(), "the new pane holds focus"); + assert_eq!(pty.spawn_count(), 1, "one PTY was spawned"); + assert_eq!(pty.launches(), vec![Launch::Shell]); + } + + #[test] + fn open_action_defaults_to_the_home_dir_and_kind_claude() { + let pty = Arc::new(RecordingPty::default()); + let mut shell = shell_over(pty.clone()); + let (outcome, _task) = shell.perform_action(BridgeAction::Open { + project: None, + kind: BridgeKind::Claude, + }); + assert_eq!(outcome.error, None); + assert_eq!( + pty.launches(), + vec![Launch::Claude { resume: None }], + "a fresh Claude session, no project → home dir" + ); + } + + #[test] + fn run_action_writes_the_bytes_to_the_target_pty() { + let (mut shell, pty) = shell_with_terminal(); + let handle = focused(&shell).expect("a focused session"); + let (outcome, _task) = shell.perform_action(BridgeAction::Run { + session: handle.parse().expect("numeric handle"), + bytes: b"ls\n".to_vec(), + }); + assert_eq!(outcome.error, None); + assert_eq!(pty.writes(), vec![b"ls\n".to_vec()], "the bytes were typed"); + } + + #[test] + fn run_action_rejects_an_unknown_handle_without_writing() { + let (mut shell, pty) = shell_with_terminal(); + let (outcome, _task) = shell.perform_action(BridgeAction::Run { + session: 999, + bytes: b"rm -rf /\n".to_vec(), + }); + assert!( + outcome.error.is_some_and(|e| e.contains("999")), + "an unknown handle is rejected, naming it" + ); + assert!(pty.writes().is_empty(), "nothing was typed into any PTY"); + } + + #[test] + fn focus_action_rejects_an_unknown_handle() { + let (mut shell, _pty) = shell_with_terminal(); + let before = focused(&shell); + let (outcome, _task) = shell.perform_action(BridgeAction::Focus { session: 999 }); + assert!(outcome.error.is_some(), "an unknown handle is rejected"); + assert_eq!(focused(&shell), before, "focus is untouched"); + } + + #[test] + fn focus_action_moves_focus_to_the_target_pane_in_a_split() { + let (mut shell, _pty) = shell_with_terminal(); + let first = focused(&shell).expect("first pane"); + // Split so the tab holds two panes; the new one takes focus. + let (split, _task) = shell.perform_action(BridgeAction::Split { + pane: None, + dir: SplitDir::Vertical, + }); + assert_eq!(split.error, None); + assert_ne!( + focused(&shell), + Some(first.clone()), + "the new pane is focused" + ); + // Focus back to the first pane by its handle. + let (outcome, _task) = shell.perform_action(BridgeAction::Focus { + session: first.parse().expect("numeric handle"), + }); + assert_eq!(outcome.error, None); + assert_eq!(focused(&shell), Some(first), "focus returned to the target"); + } + + #[test] + fn split_action_opens_and_focuses_a_pane_beside_the_focused_one() { + let (mut shell, pty) = shell_with_terminal(); + let first = focused(&shell).expect("first pane"); + let spawns_before = pty.spawn_count(); + let (outcome, _task) = shell.perform_action(BridgeAction::Split { + pane: None, + dir: SplitDir::Horizontal, + }); + assert_eq!(outcome.error, None); + let active = shell.core.workspace.active; + assert_eq!( + shell.core.workspace.tabs[active].sessions().len(), + 2, + "the tab now hosts two panes" + ); + assert_eq!( + outcome.focused, + focused(&shell), + "the outcome reports the new focus" + ); + assert_ne!(outcome.focused, Some(first), "the fresh pane holds focus"); + assert_eq!(pty.spawn_count(), spawns_before + 1, "one more PTY spawned"); + } + + #[test] + fn split_action_rejects_an_unknown_target_pane() { + let (mut shell, _pty) = shell_with_terminal(); + let (outcome, _task) = shell.perform_action(BridgeAction::Split { + pane: Some(999), + dir: SplitDir::Vertical, + }); + assert!( + outcome.error.is_some(), + "an unknown target pane is rejected" + ); + assert_eq!( + shell.core.workspace.tabs[shell.core.workspace.active] + .sessions() + .len(), + 1, + "no split happened" + ); + } + + #[test] + fn rename_action_relabels_the_tab() { + let (mut shell, _pty) = shell_with_terminal(); + let (outcome, _task) = shell.perform_action(BridgeAction::Rename { + tab: 0, + title: "build".into(), + }); + assert_eq!(outcome.error, None); + assert_eq!(shell.core.workspace.tabs[0].display_title(), "build"); + } + + #[test] + fn rename_action_rejects_an_out_of_range_tab() { + let (mut shell, _pty) = shell_with_terminal(); + let (outcome, _task) = shell.perform_action(BridgeAction::Rename { + tab: 9, + title: "nope".into(), + }); + assert!(outcome.error.is_some(), "a missing tab is rejected"); + } + + #[test] + fn close_action_closes_a_lone_pane_tab_and_kills_its_pty() { + let (mut shell, pty) = shell_with_terminal(); + assert_eq!(shell.core.workspace.tabs.len(), 1); + let (outcome, _task) = shell.perform_action(BridgeAction::Close { pane: None }); + assert_eq!(outcome.error, None); + assert!( + shell.core.workspace.tabs.is_empty(), + "the lone tab is closed" + ); + assert_eq!(pty.kill_count(), 1, "its PTY was killed"); + } + + /// A shell with two tabs; returns it plus the session handle of the *first* + /// tab, which is no longer the active one — the setup for the cross-tab + /// targeting tests below. + fn shell_with_two_tabs() -> (Shell, Arc, u64) { + let (mut shell, pty) = shell_with_terminal(); + let first = shell + .core + .workspace + .focused_session() + .expect("focused") + .0 + .get(); + let _ = shell.launch("/tmp/other".to_string(), Launch::Shell); + assert_eq!(shell.core.workspace.active, 1, "the new tab is active"); + (shell, pty, first) + } + + #[test] + fn close_action_reaches_a_pane_in_another_tab() { + // The registry is workspace-global, so a handle from an inactive tab + // resolves; the close must land on *that* pane, not on the active tab's + // focused one. + let (mut shell, _pty, first) = shell_with_two_tabs(); + let (outcome, _task) = shell.perform_action(BridgeAction::Close { pane: Some(first) }); + assert_eq!(outcome.error, None); + let live: Vec = shell + .core + .workspace + .tabs + .iter() + .flat_map(|tab| tab.sessions()) + .map(|id| id.0.get()) + .collect(); + assert!( + !live.contains(&first), + "the targeted pane is gone; surviving panes: {live:?}" + ); + assert_eq!(live.len(), 1, "the other tab must not have been closed"); + } + + #[test] + fn split_action_reaches_a_pane_in_another_tab() { + let (mut shell, _pty, first) = shell_with_two_tabs(); + let (outcome, _task) = shell.perform_action(BridgeAction::Split { + pane: Some(first), + dir: SplitDir::Vertical, + }); + assert_eq!(outcome.error, None); + assert_eq!( + shell.core.workspace.active, 0, + "the split happened in the target's tab" + ); + assert_eq!( + shell.core.workspace.tabs[0].sessions().len(), + 2, + "the target tab now hosts two panes" + ); + assert_eq!(shell.core.workspace.tabs[1].sessions().len(), 1); + } + + #[test] + fn focus_action_reaches_a_pane_in_another_tab() { + let (mut shell, _pty, first) = shell_with_two_tabs(); + let (outcome, _task) = shell.perform_action(BridgeAction::Focus { session: first }); + assert_eq!(outcome.error, None); + assert_eq!(shell.core.workspace.active, 0, "its tab was activated"); + assert_eq!( + outcome.focused, + Some(first.to_string()), + "the reported focus is the pane that was asked for" + ); + assert_eq!(focused(&shell), Some(first.to_string())); + } + + #[test] + fn focus_relative_actions_reject_a_handle_whose_pane_is_gone() { + // A stale handle — the agent read it, then the pane closed. Rejecting + // beats silently acting on whatever holds focus now, which is how a + // close request would destroy the wrong terminal. + let (mut shell, pty, first) = shell_with_two_tabs(); + let (outcome, _task) = shell.perform_action(BridgeAction::Close { pane: Some(first) }); + assert_eq!(outcome.error, None, "the first close lands"); + let handle = first; + + for action in [ + BridgeAction::Close { pane: Some(handle) }, + BridgeAction::Split { + pane: Some(handle), + dir: SplitDir::Vertical, + }, + BridgeAction::Focus { session: handle }, + ] { + let (outcome, _task) = shell.perform_action(action.clone()); + assert_eq!( + outcome.error, + Some(format!("no open pane hosts handle {handle}")), + "{action:?} should be rejected" + ); + } + assert_eq!( + shell.core.workspace.tabs.len(), + 1, + "the surviving tab is untouched" + ); + assert_eq!(pty.kill_count(), 1, "only the first, targeted close killed"); + } + #[test] fn a_drag_selection_reaches_the_pty_as_grid_anchored_ops() { // The canvas turns a press-then-drag into Select ops; the shell must diff --git a/crates/app/src/shell/bridge.rs b/crates/app/src/shell/bridge.rs index eb05329..1668982 100644 --- a/crates/app/src/shell/bridge.rs +++ b/crates/app/src/shell/bridge.rs @@ -21,6 +21,7 @@ use std::time::Duration; use iced::futures::{SinkExt, Stream}; use termherd_core::{ App, Launch, LiveSession, SessionStatus, SnapshotFilter, SnapshotInputs, WorkspaceSnapshot, + workspace::SplitDir, }; use tokio::sync::{mpsc, oneshot}; @@ -51,6 +52,75 @@ pub enum Request { Snapshot(SnapshotFilter), /// Every live session, for the `list_sessions` MCP tool. ListSessions, + /// A workspace mutation, for the orchestration MCP tools. Unlike the two + /// read requests it is answered off `respond` — the shell applies it through + /// `App::apply` and performs the effects, since it needs `&mut` state. + Act(Action), +} + +/// A workspace mutation an MCP client asks termherd to perform. Each variant +/// maps onto one or more existing core [`Event`](termherd_core::Event)s applied +/// through `App::apply` — the orchestration surface never bypasses the state +/// machine, it drives termherd exactly as a keystroke does. A `session`/`pane` +/// field is the stable handle as [`SessionInfo::handle`] reports it, resolved to +/// a live `SessionId` before anything is applied. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Action { + /// Open a new session in `project` (or the home dir when `None`), running + /// `kind`. → `Event::LaunchSession`, via the shell's own launch path. + Open { + project: Option, + kind: SessionKind, + }, + /// Split a pane, opening a fresh session beside it. Splits the focused pane, + /// or `pane` when given (revealed first, so a pane in another tab is + /// reachable). → `[RevealPane +] SplitFocused`. + Split { pane: Option, dir: SplitDir }, + /// Bring the pane hosting `session` into view, activating its tab when it + /// lives in another one. → `Event::RevealPane`. + Focus { session: u64 }, + /// Give the tab at `tab` a manual name (blank reverts to the derived title, + /// core's rule). → `Event::RenameTab`. + Rename { tab: usize, title: String }, + /// Close a pane — the focused one, or `pane` when given (revealed first). A + /// lone pane closes its whole tab (core collapses to `close_tab`, killing the + /// PTY). → `[RevealPane +] CloseFocusedPane`. + Close { pane: Option }, + /// Type `bytes` into a session's PTY without waiting (waiting is a later + /// rung). → `Event::TerminalInput`. + Run { session: u64, bytes: Vec }, +} + +/// The result of an [`Action`]. `error` is `Some` only when the action was +/// rejected before touching state — an unknown handle or an out-of-range tab — +/// in which case nothing was applied. Otherwise `focused` reports the session +/// handle that holds focus once the action settled (the new pane after +/// open/split, the target after focus), so an agent gets act→observe in one +/// round trip. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ActionOutcome { + /// The resulting focused session handle, or `None` when nothing is focused. + pub focused: Option, + /// Why the action was rejected, or `None` when it applied. + pub error: Option, +} + +impl ActionOutcome { + /// A rejection that touched no state — an unresolved handle / index. + pub fn rejected(reason: impl Into) -> Self { + Self { + focused: None, + error: Some(reason.into()), + } + } + + /// An applied action, reporting the resulting focused handle. + pub fn applied(focused: Option) -> Self { + Self { + focused, + error: None, + } + } } /// The app's answer to a [`Request`]. @@ -58,6 +128,7 @@ pub enum Request { pub enum Reply { Snapshot(WorkspaceSnapshot), Sessions(Vec), + Acted(ActionOutcome), } /// The kind of program a session runs, as an MCP client sees it. Distinct from @@ -231,6 +302,12 @@ pub fn respond(core: &App, request: &Request, inputs: &SnapshotInputs) -> Reply match request { Request::Snapshot(filter) => Reply::Snapshot(core.snapshot(filter, inputs)), Request::ListSessions => Reply::Sessions(list_sessions(core)), + // Actions mutate, so they can't answer off a `&App`; the shell branches + // them to `perform_action` before reaching here. This arm is the + // defensive default should that routing ever be bypassed. + Request::Act(_) => Reply::Acted(ActionOutcome::rejected( + "an action reached the read-only responder; the shell must apply it", + )), } } diff --git a/crates/app/src/shell/orchestrate.rs b/crates/app/src/shell/orchestrate.rs new file mode 100644 index 0000000..931e739 --- /dev/null +++ b/crates/app/src/shell/orchestrate.rs @@ -0,0 +1,179 @@ +//! The orchestration seam: carrying an MCP [`Action`] out against the running +//! workspace. Split from the shell's state machine so the "resolve a handle → +//! apply the existing event(s) → report the resulting focus" flow lives in one +//! place, mirroring how [`launch`](super::launch) owns the spawn-and-focus flow. +//! +//! Every action is a thin wrapper over an existing core +//! [`Event`](termherd_core::Event): the shell owns `core::App` *and* the one +//! effect executor, so it can resolve the stable handle, apply the event, and +//! perform the effects — the read-only `respond` cannot, since it holds only a +//! `&App`. + +use std::num::NonZeroU64; + +use iced::Task; +use termherd_core::workspace::{SessionId, SplitDir}; +use termherd_core::{Event, Launch}; + +use super::bridge::{Action, ActionOutcome, SessionKind}; +use super::{Focus, Message, Shell, home_dir}; + +impl Shell { + /// Carry out one MCP [`Action`], returning the outcome to answer the caller + /// with plus any async follow-up the applied effects need (a PTY spawn's + /// resize). A handle that resolves to no live session — or an out-of-range + /// tab — is rejected before any state is touched. + pub(super) fn perform_action(&mut self, action: Action) -> (ActionOutcome, Task) { + match action { + Action::Open { project, kind } => self.act_open(project, kind), + Action::Split { pane, dir } => self.act_split(pane, dir), + Action::Focus { session } => self.act_focus(session), + Action::Rename { tab, title } => self.act_rename(tab, title), + Action::Close { pane } => self.act_close(pane), + Action::Run { session, bytes } => self.act_run(session, bytes), + } + } + + /// Open a new session, reusing the shell's own launch path (the same one a + /// click drives), so the spawn, focus and resize all match. No project falls + /// back to the home directory, so the tool works from an empty workspace. + fn act_open( + &mut self, + project: Option, + kind: SessionKind, + ) -> (ActionOutcome, Task) { + let launch = match kind { + SessionKind::Shell => Launch::Shell, + SessionKind::Claude => Launch::Claude { resume: None }, + }; + let task = self.launch(project.unwrap_or_else(home_dir), launch); + (self.applied(), task) + } + + /// Split a pane, opening a fresh session beside it. With `pane` given, focus + /// it first so the focus-relative `SplitFocused` acts on it; the new pane + /// then takes focus. An unknown target is rejected before anything applies. + fn act_split(&mut self, pane: Option, dir: SplitDir) -> (ActionOutcome, Task) { + let mut effects = match self.retarget(pane) { + Ok(effects) => effects, + Err(outcome) => return (outcome, Task::none()), + }; + effects.extend(self.core.apply(Event::SplitFocused(dir))); + // A split halves the original pane's area and spawns the new one at a + // default grid, so both need a resize to their real cells. + let task = Task::batch([self.perform(effects), self.resize_panes()]); + (self.applied(), task) + } + + /// Bring the pane hosting `session` into view — activating its tab when it + /// lives in another one — and hand the keyboard to the terminal. Rejects a + /// handle no open pane hosts. + fn act_focus(&mut self, session: u64) -> (ActionOutcome, Task) { + let id = match self.resolve_pane(session) { + Ok(id) => id, + Err(outcome) => return (outcome, Task::none()), + }; + self.focus = Focus::Terminal; + let effects = self.core.apply(Event::RevealPane(id)); + // A reveal may activate another tab, whose panes were last sized for a + // different layout — resize like `activate_tab` does. + let task = Task::batch([self.perform(effects), self.resize_panes()]); + (self.applied(), task) + } + + /// Rename the tab at `tab`. A blank title reverts to the derived name + /// (core's rule). Rejects an index past the open tabs. + fn act_rename(&mut self, tab: usize, title: String) -> (ActionOutcome, Task) { + if self.core.workspace.tabs.get(tab).is_none() { + return ( + ActionOutcome::rejected(format!("no tab at index {tab}")), + Task::none(), + ); + } + let effects = self.core.apply(Event::RenameTab { index: tab, title }); + (self.applied(), self.perform(effects)) + } + + /// Close a pane — the focused one, or `pane` when given (focused first). A + /// lone pane is the whole tab, so core collapses to `close_tab`, killing the + /// PTY. Rejects an unknown target. + fn act_close(&mut self, pane: Option) -> (ActionOutcome, Task) { + let mut effects = match self.retarget(pane) { + Ok(effects) => effects, + Err(outcome) => return (outcome, Task::none()), + }; + effects.extend(self.core.apply(Event::CloseFocusedPane)); + let task = Task::batch([self.perform(effects), self.resize_panes()]); + (self.applied(), task) + } + + /// Type bytes into a session's PTY without waiting (waiting is a later rung). + /// Rejects an unknown handle, so a stale target can't misfire into a live + /// terminal. + fn act_run(&mut self, session: u64, bytes: Vec) -> (ActionOutcome, Task) { + let Some(id) = self.resolve(session) else { + return (unknown_handle(session), Task::none()); + }; + let effects = self.core.apply(Event::TerminalInput { session: id, bytes }); + (self.applied(), self.perform(effects)) + } + + /// The shared prelude of the focus-relative actions (split, close): reveal + /// `pane` first when one is named, returning the reveal's effects to fold + /// in, or a rejection when no open pane hosts the handle. `None` leaves the + /// current focus and yields no effects. + /// + /// The reveal must actually land, because what follows — `SplitFocused`, + /// `CloseFocusedPane` — reads the focus rather than the handle. A silent + /// no-op here would split or **kill the wrong pane** while reporting + /// success, so the target is checked before anything applies. + fn retarget(&mut self, pane: Option) -> Result, ActionOutcome> { + let Some(handle) = pane else { + return Ok(Vec::new()); + }; + let id = self.resolve_pane(handle)?; + self.focus = Focus::Terminal; + Ok(self.core.apply(Event::RevealPane(id))) + } + + /// Resolve a stable handle to the [`SessionId`] of a **pane that is + /// actually open**. The session registry is workspace-global and carries no + /// tab dimension, so it cannot answer this: `tab_of` is the check that + /// names a pane a focus-relative action can aim at. + fn resolve_pane(&self, handle: u64) -> Result { + NonZeroU64::new(handle) + .map(SessionId) + .filter(|id| self.core.workspace.tab_of(*id).is_some()) + .ok_or_else(|| unhosted_handle(handle)) + } + + /// Resolve a stable handle to a live [`SessionId`], or `None` when no session + /// carries it (already closed, or never existed). + fn resolve(&self, handle: u64) -> Option { + let id = NonZeroU64::new(handle).map(SessionId)?; + self.core.sessions.contains_key(&id).then_some(id) + } + + /// An applied outcome reporting the session that holds focus now. + fn applied(&self) -> ActionOutcome { + ActionOutcome::applied( + self.core + .workspace + .focused_session() + .map(|id| id.0.get().to_string()), + ) + } +} + +/// The rejection for a handle that resolves to no live session — named so an +/// agent sees which id it got wrong. +fn unknown_handle(handle: u64) -> ActionOutcome { + ActionOutcome::rejected(format!("no live session with handle {handle}")) +} + +/// The rejection for a handle no open pane hosts — closed, or never opened. A +/// focus-relative action has nothing to aim at, and must not fall back to +/// whatever happens to hold focus. +fn unhosted_handle(handle: u64) -> ActionOutcome { + ActionOutcome::rejected(format!("no open pane hosts handle {handle}")) +} diff --git a/crates/core/src/app.rs b/crates/core/src/app.rs index ebc5521..28948b1 100644 --- a/crates/core/src/app.rs +++ b/crates/core/src/app.rs @@ -221,6 +221,10 @@ impl App { self.workspace.focus_pane_of(session); Vec::new() } + Event::RevealPane(session) => { + self.workspace.reveal_pane_of(session); + Vec::new() + } Event::FocusDir(dir) => { self.workspace.focus_dir(dir); Vec::new() diff --git a/crates/core/src/app/events.rs b/crates/core/src/app/events.rs index e5fe256..cb8735b 100644 --- a/crates/core/src/app/events.rs +++ b/crates/core/src/app/events.rs @@ -95,8 +95,14 @@ pub enum Event { /// Move focus to the next / previous pane in the active tab (FR6). FocusNextPane, FocusPrevPane, - /// Move focus to the pane hosting a session (click-to-focus, FR6). + /// Move focus to the pane hosting a session *in the active tab* + /// (click-to-focus, FR6). A pane in another tab is out of a click's reach — + /// address it with [`Event::RevealPane`]. FocusPane(SessionId), + /// Bring the pane hosting a session into view wherever it lives, activating + /// its tab first (FR6). What a caller holding only a session handle — the + /// MCP control surface — needs, since it is not bound to the active tab. + RevealPane(SessionId), /// Move pane focus one step in a spatial direction, cycling within its axis /// (FR6). FocusDir(Direction), diff --git a/crates/core/src/workspace.rs b/crates/core/src/workspace.rs index e85f014..c7b801d 100644 --- a/crates/core/src/workspace.rs +++ b/crates/core/src/workspace.rs @@ -355,6 +355,24 @@ impl Workspace { Some(()) } + /// Bring the pane hosting `session` into view wherever it lives — activating + /// its tab first, then focusing the leaf. The counterpart of + /// [`Workspace::focus_pane_of`], which only reaches the *active* tab because + /// a click cannot land anywhere else; a caller addressing a pane by handle + /// can. Returns `None` — leaving both the active tab and the focus untouched + /// — when no tab hosts it. + pub fn reveal_pane_of(&mut self, session: SessionId) -> Option<()> { + let index = self.tab_of(session)?; + let previous = self.active; + self.active = index; + // Restore the previous tab if the leaf turns out to be unreachable, so a + // failed reveal changes nothing at all. + self.focus_pane_of(session).or_else(|| { + self.active = previous; + None + }) + } + /// Move pane focus one step in a spatial direction (FR6), cycling within /// the move's axis. It crosses the nearest ancestor split of that /// orientation into the sibling subtree, landing on the adjacent leaf; from @@ -638,6 +656,43 @@ mod tests { assert_eq!(ws.focused_session(), Some(sid(1))); } + #[test] + fn focus_pane_of_does_not_reach_into_another_tab() { + let mut ws = Workspace::new(); + ws.open(sid(1), "a"); + ws.open(sid(2), "b"); + // A click cannot land on an inactive tab, so this stays a no-op — the + // guarantee `reveal_pane_of` exists to lift. + assert_eq!(ws.focus_pane_of(sid(1)), None); + assert_eq!(ws.active, 1); + assert_eq!(ws.focused_session(), Some(sid(2))); + } + + #[test] + fn reveal_pane_of_activates_the_tab_hosting_the_session() { + let mut ws = Workspace::new(); + ws.open(sid(1), "a"); + ws.split(SplitDir::Vertical, sid(2)); + ws.open(sid(3), "b"); + assert_eq!(ws.active, 1); + + // Reaching back into the first tab activates it *and* lands focus on the + // named leaf — not merely on whatever that tab last focused. + assert_eq!(ws.reveal_pane_of(sid(1)), Some(())); + assert_eq!(ws.active, 0); + assert_eq!(ws.focused_session(), Some(sid(1))); + } + + #[test] + fn reveal_pane_of_leaves_everything_untouched_for_an_unhosted_session() { + let mut ws = Workspace::new(); + ws.open(sid(1), "a"); + ws.open(sid(2), "b"); + assert_eq!(ws.reveal_pane_of(sid(99)), None); + assert_eq!(ws.active, 1, "the active tab must not move"); + assert_eq!(ws.focused_session(), Some(sid(2))); + } + #[test] fn rename_tab_overrides_the_derived_title_while_keeping_it() { let mut ws = Workspace::new();