From 8d1b5f6c23d52e95184388c939796fb101dddedd Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp <19+brynary@users.noreply.github.com> Date: Sun, 17 May 2026 09:12:32 -0700 Subject: [PATCH] feat(mcp): add interrupt action to fabro_run_interact (#296) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Adds a standalone `interrupt` action to the `fabro_run_interact` MCP tool, closing a parity gap with the HTTP API (`POST /api/v1/runs/{id}/interrupt`). - Lets MCP callers cancel an API-mode agent's current LLM round and park it in `SteeringHub`'s `waiting_for_steer` state without committing to follow-up text in the same call. - Dispatches through the existing `Client::interrupt_run`; no client or server-side changes. ## Why not just use `message` with `interrupt: true`? Combined interrupt+steer remains the right choice when you want to redirect the agent. Bare interrupt is for "pause the agent while I decide what to say next." The variant's doc comment steers callers toward `message` or `cancel` as the usual options, since a bare interrupt with no follow-up leaves the run idle indefinitely. ## Test plan - [x] `cargo nextest run -p fabro-mcp-server` — 13/13 pass, including new `interrupt_action_requires_only_run_id` unit test - [x] `cargo nextest run -p fabro-cli -E 'test(/mcp_/)'` — 27/27 pass, including extended `mcp_interact_actions_resolve_selector_and_call_expected_endpoints` E2E (mocks `POST /runs/{id}/interrupt`, asserts the tool hits it) - [x] `cargo +nightly-2026-04-14 clippy -p fabro-mcp-server -p fabro-cli --all-targets -- -D warnings` — clean - [x] `cargo +nightly-2026-04-14 fmt --check` on touched crates — clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) --- ...26-05-11-add-fabro-mcp-server-test-plan.md | 2 +- lib/crates/fabro-cli/tests/it/cmd/mcp.rs | 15 +++++++- .../src/run_tools/interact.rs | 34 +++++++++++++++++++ lib/crates/fabro-mcp-server/src/server.rs | 2 +- 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/docs/plans/2026-05-11-add-fabro-mcp-server-test-plan.md b/docs/plans/2026-05-11-add-fabro-mcp-server-test-plan.md index d90a41459..ee9224c99 100644 --- a/docs/plans/2026-05-11-add-fabro-mcp-server-test-plan.md +++ b/docs/plans/2026-05-11-add-fabro-mcp-server-test-plan.md @@ -273,7 +273,7 @@ Covered action space: - CLI executable commands: `fabro mcp --help`, `fabro mcp start --help`, `fabro mcp config --help`, `fabro mcp init --help`, `fabro mcp config`, `fabro mcp config --server --storage-dir`, and `fabro mcp init claude|cursor|windsurf`. - MCP protocol actions: stdio process startup, `initialize`, `tools/list`, and `tools/call`. -- MCP tool actions: `fabro_run_create`; `fabro_run_search`; `fabro_run_interact` actions `get`, `start`, `message`, `cancel`, `archive`, `unarchive`, `get_questions`, `answer`; `fabro_run_gather`; `fabro_run_events` actions `list`, `details`, and `search`. +- MCP tool actions: `fabro_run_create`; `fabro_run_search`; `fabro_run_interact` actions `get`, `start`, `message`, `interrupt`, `cancel`, `archive`, `unarchive`, `get_questions`, `answer`; `fabro_run_gather`; `fabro_run_events` actions `list`, `details`, and `search`. - Error and boundary behavior: invalid local parameters, too many run ids, null input conversion, missing action fields, unsupported answer shapes, invalid agent config JSON, missing auth, unreachable server after local validation, timeout expiry, and service liveness after tool errors. - Integration boundaries: CLI auth store reuse, Fabro API client, real local Fabro server, run manifest construction/validation, event store, generated API answer types, and existing `fabro-mcp` client crate. - Performance smoke: initialize plus `tools/list` without auth/server. diff --git a/lib/crates/fabro-cli/tests/it/cmd/mcp.rs b/lib/crates/fabro-cli/tests/it/cmd/mcp.rs index 358372879..332af03e8 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/mcp.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/mcp.rs @@ -1369,6 +1369,11 @@ async fn mcp_interact_actions_resolve_selector_and_call_expected_endpoints() { .json_body(serde_json::json!({ "text": "continue", "interrupt": true })); then.status(202); }); + let interrupt = server.mock(|when, then| { + when.method(POST) + .path(format!("/api/v1/runs/{run_id}/interrupt")); + then.status(202); + }); let cancel = server.mock(|when, then| { when.method(POST) .path(format!("/api/v1/runs/{run_id}/cancel")); @@ -1408,6 +1413,12 @@ async fn mcp_interact_actions_resolve_selector_and_call_expected_endpoints() { }), ) .await; + let interrupt_result = call_tool_json( + &client, + "fabro_run_interact", + serde_json::json!({ "run_id": selector, "action": "interrupt" }), + ) + .await; let cancel_result = call_tool_json( &client, "fabro_run_interact", @@ -1419,12 +1430,14 @@ async fn mcp_interact_actions_resolve_selector_and_call_expected_endpoints() { assert_eq!(start_result["result"]["summary"]["run_id"], run_id); assert_eq!(message_result["result"]["message"], "continue"); assert_eq!(message_result["result"]["interrupt"], true); + assert_eq!(interrupt_result["result"]["interrupted"], true); assert_eq!(cancel_result["result"]["summary"]["run_id"], run_id); - resolve.assert_calls(4); + resolve.assert_calls(5); retrieve.assert_calls(1); projection.assert(); start.assert(); message.assert(); + interrupt.assert(); cancel.assert(); client .shutdown() diff --git a/lib/crates/fabro-mcp-server/src/run_tools/interact.rs b/lib/crates/fabro-mcp-server/src/run_tools/interact.rs index abd6b6cf7..28542abd2 100644 --- a/lib/crates/fabro-mcp-server/src/run_tools/interact.rs +++ b/lib/crates/fabro-mcp-server/src/run_tools/interact.rs @@ -17,6 +17,11 @@ pub(crate) enum RunInteractAction { Get, Start, Message, + /// Cancel the active API-mode agent's current LLM round and park it + /// waiting for a later `message`. The run sits idle until you follow up + /// with `message` or `cancel`. To redirect the agent, prefer `message` + /// (optionally with `interrupt: true`). + Interrupt, Cancel, Archive, Unarchive, @@ -111,6 +116,7 @@ pub(crate) enum ValidatedInteractAction { message: String, interrupt: bool, }, + Interrupt, Cancel, Archive, Unarchive, @@ -127,6 +133,7 @@ impl ValidatedInteractAction { Self::Get => RunInteractAction::Get, Self::Start => RunInteractAction::Start, Self::Message { .. } => RunInteractAction::Message, + Self::Interrupt => RunInteractAction::Interrupt, Self::Cancel => RunInteractAction::Cancel, Self::Archive => RunInteractAction::Archive, Self::Unarchive => RunInteractAction::Unarchive, @@ -160,6 +167,7 @@ impl TryFrom for ValidatedInteractRun { interrupt: params.interrupt.unwrap_or(false), } } + RunInteractAction::Interrupt => ValidatedInteractAction::Interrupt, RunInteractAction::Cancel => ValidatedInteractAction::Cancel, RunInteractAction::Archive => ValidatedInteractAction::Archive, RunInteractAction::Unarchive => ValidatedInteractAction::Unarchive, @@ -224,6 +232,13 @@ pub(crate) async fn interact_run( .map_err(|err| ToolError::from_anyhow(&err))?; json!({ "message": message, "interrupt": interrupt }) } + ValidatedInteractAction::Interrupt => { + client + .interrupt_run(&run_id) + .await + .map_err(|err| ToolError::from_anyhow(&err))?; + json!({ "interrupted": true }) + } ValidatedInteractAction::Cancel => { let summary = client .cancel_run(&run_id) @@ -396,4 +411,23 @@ mod tests { assert!(err.as_str().contains("option, options, text")); } + + #[test] + fn interrupt_action_requires_only_run_id() { + let validated = ValidatedInteractRun::try_from(FabroRunInteractParams { + action: RunInteractAction::Interrupt, + run_id: "run_123".to_string(), + message: None, + interrupt: None, + question_id: None, + answer: None, + }) + .expect("interrupt should validate with only run_id"); + + assert_eq!(validated.run_id, "run_123"); + assert!(matches!( + validated.action, + ValidatedInteractAction::Interrupt + )); + } } diff --git a/lib/crates/fabro-mcp-server/src/server.rs b/lib/crates/fabro-mcp-server/src/server.rs index e82b2e404..b44fb4803 100644 --- a/lib/crates/fabro-mcp-server/src/server.rs +++ b/lib/crates/fabro-mcp-server/src/server.rs @@ -93,7 +93,7 @@ impl FabroMcpServer { #[tool( name = "fabro_run_interact", - description = "Get, start, message, cancel, archive, unarchive, inspect questions, or answer a Fabro run." + description = "Get, start, message, interrupt, cancel, archive, unarchive, inspect questions, or answer a Fabro run." )] async fn fabro_run_interact( &self,