From 6e9cbda281c8d4b172fb5d16b8766414dd0afcba Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 5 Oct 2026 15:53:35 +1300 Subject: [PATCH 1/3] fix(tools): sanitize tool output with abnegate-secret zone_core kept its own copy of the terminal-control stripper that abnegate-secret 0.1.3 already publishes as sanitize and sanitize_owned, with the same redaction pass behind it. Two copies drift, and the published one has since closed gaps the local one still had: invisible formatting characters (zero-width, bidirectional overrides, tag characters) let a credential split by a zero-width space reach the transcript unredacted, two-byte escapes outside 0x40-0x5F and escapes with intermediate bytes such as ESC ( B left their tails behind, and the ESC X start-of-string leaked its payload. ToolResult and zone_notify now call the crate directly. zone_notify used zone_core for nothing else, so that dependency goes too. The new ToolResult tests pin each output change, and a read_file test pins the cost: file content reaches the model through ToolResult, so variation selectors and zero-width joiners and non-joiners are dropped from what it reads, though not from the file. Co-Authored-By: Claude Opus 5.5 --- runner/Cargo.lock | 1 - runner/zone_core/src/tools/file.rs | 28 +++ runner/zone_core/src/tools/mod.rs | 56 ++++- runner/zone_core/src/tools/sanitize.rs | 248 ---------------------- runner/zone_notify/Cargo.toml | 1 - runner/zone_notify/src/backend/email.rs | 2 +- runner/zone_notify/src/backend/webhook.rs | 2 +- runner/zone_notify/src/lib.rs | 5 +- runner/zone_notify/src/notification.rs | 13 +- 9 files changed, 86 insertions(+), 270 deletions(-) delete mode 100644 runner/zone_core/src/tools/sanitize.rs diff --git a/runner/Cargo.lock b/runner/Cargo.lock index d51b8848..0937adfb 100644 --- a/runner/Cargo.lock +++ b/runner/Cargo.lock @@ -9934,7 +9934,6 @@ dependencies = [ "tracing", "url", "wiremock", - "zone_core", ] [[package]] diff --git a/runner/zone_core/src/tools/file.rs b/runner/zone_core/src/tools/file.rs index acaadd35..20f5a83e 100644 --- a/runner/zone_core/src/tools/file.rs +++ b/runner/zone_core/src/tools/file.rs @@ -1178,6 +1178,34 @@ mod tests { assert!(!output.contains("Line 4")); } + /// Tool output is sanitized, so the model reads a file without its + /// invisible formatting characters: an emoji's variation selector and a + /// Persian word's zero-width non-joiner are gone from what it sees, and a + /// patch or rewrite built from that text does not carry them. + #[tokio::test] + async fn read_file_returns_text_without_its_invisible_formatting_characters() { + let dir = tempdir().unwrap(); + let content = + "\u{2764}\u{FE0F} \u{645}\u{6CC}\u{200C}\u{62E}\u{648}\u{627}\u{647}\u{645}\n"; + fs::write(dir.path().join("note.txt"), content).unwrap(); + let context = create_test_context(dir.path()); + + let result = ReadFileTool + .execute(serde_json::json!({"path": "note.txt"}), &context) + .await + .unwrap(); + + assert_eq!( + result.output.as_deref(), + Some("\u{2764} \u{645}\u{6CC}\u{62E}\u{648}\u{627}\u{647}\u{645}\n") + ); + assert_eq!( + fs::read_to_string(dir.path().join("note.txt")).unwrap(), + content, + "reading leaves the file itself untouched" + ); + } + #[test] fn page_text_caps_and_continues_by_character() { let content = "α".repeat(10); diff --git a/runner/zone_core/src/tools/mod.rs b/runner/zone_core/src/tools/mod.rs index 38b8542e..f2fc43e0 100644 --- a/runner/zone_core/src/tools/mod.rs +++ b/runner/zone_core/src/tools/mod.rs @@ -10,18 +10,16 @@ mod identity; pub mod job; mod reason; pub mod routing; -mod sanitize; pub mod tail; mod tier; pub use command::*; pub use file::*; pub use reason::{REASON_DESCRIPTION, REASON_PARAM, reason_property}; -pub use sanitize::sanitize; pub use tier::{CONFIRMED_FROM, Tier}; +use abnegate_secret::sanitize_owned; use async_trait::async_trait; -use sanitize::sanitize_owned; use serde::{Deserialize, Serialize}; use serde_json::Value; use std::collections::{HashMap, HashSet}; @@ -179,7 +177,7 @@ pub struct ToolResult { } impl ToolResult { - /// Wrap successful tool output, [`sanitize`]d on the way in. + /// Wrap successful tool output, [`sanitize`](abnegate_secret::sanitize)d on the way in. pub fn success(output: impl Into) -> Self { Self { success: true, @@ -189,7 +187,7 @@ impl ToolResult { } } - /// Wrap a tool failure, [`sanitize`]d on the way in. + /// Wrap a tool failure, [`sanitize`](abnegate_secret::sanitize)d on the way in. pub fn error(error: impl Into) -> Self { Self { success: false, @@ -751,6 +749,7 @@ pub(crate) mod test_support { #[cfg(test)] mod tests { use super::*; + use abnegate_secret::REDACTED; /// A preview a reader approves has to say what will run. `sh -c` runs one /// command per line, so two lines joined by a space showed them a single @@ -805,11 +804,9 @@ mod tests { #[test] fn success_redacts_a_credential_in_the_output() { let result = ToolResult::success("printenv\nGITHUB_TOKEN=ghp_0123456789abcdefghij\n"); - assert_eq!( - result.output.as_deref(), - Some("printenv\nGITHUB_TOKEN=[REDACTED]\n") - ); - assert_eq!(result.to_message(), "printenv\nGITHUB_TOKEN=[REDACTED]\n"); + let redacted = format!("printenv\nGITHUB_TOKEN={REDACTED}\n"); + assert_eq!(result.output.as_deref(), Some(redacted.as_str())); + assert_eq!(result.to_message(), redacted); } #[test] @@ -819,6 +816,45 @@ mod tests { assert_eq!(result.to_message(), "Error: command not found\n"); } + #[test] + fn success_redacts_a_credential_split_by_an_invisible_character() { + let result = ToolResult::success("fatal: ghp_0123\u{200B}456789abcdefghij rejected"); + assert_eq!(result.output, Some(format!("fatal: {REDACTED} rejected"))); + } + + #[test] + fn success_strips_bidirectional_overrides() { + let result = ToolResult::success("access = \u{202E}nimda\u{202C} check"); + assert_eq!(result.output.as_deref(), Some("access = nimda check")); + } + + #[test] + fn error_strips_a_whole_two_byte_escape() { + let result = ToolResult::error("a\u{1b}7b\u{1b}cc"); + assert_eq!(result.error.as_deref(), Some("abc")); + } + + #[test] + fn error_strips_an_escape_with_intermediates() { + let result = ToolResult::error("a\u{1b}(Bb"); + assert_eq!(result.error.as_deref(), Some("ab")); + } + + #[test] + fn error_strips_a_start_of_string_with_its_payload() { + let result = ToolResult::error("a\u{1b}Xhidden\u{1b}\\b"); + assert_eq!(result.error.as_deref(), Some("ab")); + } + + #[test] + fn success_drops_emoji_joiners_and_variation_selectors() { + let result = ToolResult::success("\u{2764}\u{FE0F} \u{1F468}\u{200D}\u{1F469}"); + assert_eq!( + result.output.as_deref(), + Some("\u{2764} \u{1F468}\u{1F469}") + ); + } + #[test] fn to_message_keeps_the_head_and_tail_of_huge_success_output() { let body = format!("HEAD_MARKER{}TAIL_MARKER", "x".repeat(20_000)); diff --git a/runner/zone_core/src/tools/sanitize.rs b/runner/zone_core/src/tools/sanitize.rs deleted file mode 100644 index 46c3918b..00000000 --- a/runner/zone_core/src/tools/sanitize.rs +++ /dev/null @@ -1,248 +0,0 @@ -//! Hygiene for text a tool hands back. -//! -//! Tool output carries fetched pages, command output and build logs, so it is -//! attacker-influenced text on its way into a terminal, a browser and a stored -//! message. Terminal control sequences are stripped first so a credential -//! cannot hide behind one, then credentials are redacted. - -use std::borrow::Cow; - -use abnegate_secret::redact; - -const ESCAPE: u8 = 0x1B; -const BELL: u8 = 0x07; -const BACKSLASH: u8 = b'\\'; -const TAB: u8 = b'\t'; -const LINE_FEED: u8 = b'\n'; -const CARRIAGE_RETURN: u8 = b'\r'; -const DELETE: u8 = 0x7F; -const CONTROL_LEAD: u8 = 0xC2; -const CONTROL_SEQUENCE_INTRODUCER: u8 = 0x9B; - -/// Strip terminal control sequences from `text` and redact any credential. -/// -/// Text with nothing to remove is returned untouched and unallocated. -pub fn sanitize(text: &str) -> Cow<'_, str> { - let stripped = strip_control_sequences(text); - let redacted = match redact(&stripped) { - Cow::Borrowed(_) => None, - Cow::Owned(redacted) => Some(redacted), - }; - match redacted { - Some(redacted) => Cow::Owned(redacted), - None => stripped, - } -} - -pub(crate) fn sanitize_owned(text: String) -> String { - let sanitized = match sanitize(&text) { - Cow::Borrowed(_) => None, - Cow::Owned(sanitized) => Some(sanitized), - }; - sanitized.unwrap_or(text) -} - -fn strip_control_sequences(text: &str) -> Cow<'_, str> { - if !text.bytes().any(needs_inspection) { - return Cow::Borrowed(text); - } - - let bytes = text.as_bytes(); - let mut output = String::with_capacity(text.len()); - let mut index = 0; - - while index < bytes.len() { - match bytes[index] { - ESCAPE => index = escape_sequence(bytes, index), - CARRIAGE_RETURN => { - output.push('\n'); - index += if bytes.get(index + 1) == Some(&LINE_FEED) { - 2 - } else { - 1 - }; - } - TAB | LINE_FEED => { - output.push(char::from(bytes[index])); - index += 1; - } - 0x00..=0x1F | DELETE => index += 1, - CONTROL_LEAD => match bytes.get(index + 1) { - Some(&CONTROL_SEQUENCE_INTRODUCER) => { - index = control_sequence(bytes, index + 2).unwrap_or(index + 2); - } - Some(0x80..=0x9F) => index += 2, - _ => { - output.push_str(&text[index..index + 2]); - index += 2; - } - }, - _ => { - let start = index; - while index < bytes.len() && !needs_inspection(bytes[index]) { - index += 1; - } - output.push_str(&text[start..index]); - } - } - } - - Cow::Owned(output) -} - -fn needs_inspection(byte: u8) -> bool { - matches!(byte, 0x00..=0x08 | 0x0B..=0x1F | DELETE | CONTROL_LEAD) -} - -/// The end of the sequence introduced by the escape at `index`. -/// -/// An unterminated sequence loses only its introducer, leaving the payload as -/// ordinary text, which is what a terminal would show. -fn escape_sequence(bytes: &[u8], index: usize) -> usize { - match bytes.get(index + 1) { - Some(b']') => operating_system_command(bytes, index + 2).unwrap_or(index + 2), - Some(b'P' | b'^' | b'_') => device_control(bytes, index + 2).unwrap_or(index + 2), - Some(b'[') => control_sequence(bytes, index + 2).unwrap_or(index + 2), - Some(0x40..=0x5F) => index + 2, - _ => index + 1, - } -} - -fn operating_system_command(bytes: &[u8], from: usize) -> Option { - let mut index = from; - while index < bytes.len() { - if bytes[index] == BELL { - return Some(index + 1); - } - if bytes[index] == ESCAPE && bytes.get(index + 1) == Some(&BACKSLASH) { - return Some(index + 2); - } - index += 1; - } - None -} - -fn device_control(bytes: &[u8], from: usize) -> Option { - let mut index = from; - while index < bytes.len() { - if bytes[index] == ESCAPE && bytes.get(index + 1) == Some(&BACKSLASH) { - return Some(index + 2); - } - index += 1; - } - None -} - -fn control_sequence(bytes: &[u8], from: usize) -> Option { - let mut index = from; - while matches!(bytes.get(index), Some(0x30..=0x3F)) { - index += 1; - } - while matches!(bytes.get(index), Some(0x20..=0x2F)) { - index += 1; - } - match bytes.get(index) { - Some(0x40..=0x7E) => Some(index + 1), - _ => None, - } -} - -#[cfg(test)] -mod tests { - use super::*; - use abnegate_secret::REDACTED; - - #[test] - fn leaves_plain_text_alone() { - let text = "Compiling zone_core v0.1.0\n Finished in 4.21s\n"; - assert!(matches!(sanitize(text), Cow::Borrowed(_))); - assert_eq!(sanitize(text), text); - } - - #[test] - fn strips_a_colour_sequence() { - assert_eq!( - sanitize("\u{1b}[31merror\u{1b}[0m: failed"), - "error: failed" - ); - } - - #[test] - fn strips_a_cursor_sequence_with_intermediates() { - assert_eq!( - sanitize("before\u{1b}[?25l\u{1b}[1;2 qafter"), - "beforeafter" - ); - } - - #[test] - fn strips_a_control_sequence_introduced_by_c1() { - assert_eq!(sanitize("before\u{9b}31mafter"), "beforeafter"); - } - - #[test] - fn strips_an_operating_system_command() { - assert_eq!(sanitize("\u{1b}]0;take over the title\u{7}ok"), "ok"); - assert_eq!(sanitize("\u{1b}]8;;https://evil.test\u{1b}\\ok"), "ok"); - } - - #[test] - fn strips_device_control_privacy_and_application_strings() { - assert_eq!(sanitize("a\u{1b}Pq#0;2;0;0;0\u{1b}\\b"), "ab"); - assert_eq!(sanitize("a\u{1b}^private\u{1b}\\b"), "ab"); - assert_eq!(sanitize("a\u{1b}_application\u{1b}\\b"), "ab"); - } - - #[test] - fn strips_a_lone_escape() { - assert_eq!(sanitize("a\u{1b}Mb"), "ab"); - assert_eq!(sanitize("a\u{1b}7b"), "a7b"); - } - - #[test] - fn drops_the_introducer_of_an_unterminated_sequence() { - assert_eq!(sanitize("\u{1b}]0;never terminated"), "0;never terminated"); - assert_eq!(sanitize("\u{1b}[31"), "31"); - } - - #[test] - fn normalises_line_endings() { - assert_eq!(sanitize("one\r\ntwo\rthree\nfour"), "one\ntwo\nthree\nfour"); - } - - #[test] - fn drops_c0_and_c1_controls_but_keeps_tab_and_newline() { - assert_eq!( - sanitize("a\u{0}b\u{8}c\u{b}d\u{c}e\u{1f}f\u{7f}g\u{85}h\ti\nj"), - "abcdefgh\ti\nj" - ); - } - - #[test] - fn keeps_multibyte_text_that_is_not_a_control() { - let text = "résumé £20 中文 🔐"; - assert_eq!(sanitize(text), text); - } - - #[test] - fn redacts_a_credential_hidden_behind_a_control_sequence() { - assert_eq!( - sanitize("token=ghp_0123\u{1b}[0m456789abcdefghij"), - format!("token={REDACTED}") - ); - } - - #[test] - fn redacts_and_strips_together() { - assert_eq!( - sanitize("\u{1b}[31mfatal\u{1b}[0m: sk-0123456789abcdefghij rejected\r\n"), - format!("fatal: {REDACTED} rejected\n") - ); - } - - #[test] - fn sanitize_owned_keeps_the_original_buffer_when_clean() { - let text = String::from("nothing to clean here"); - assert_eq!(sanitize_owned(text.clone()), text); - } -} diff --git a/runner/zone_notify/Cargo.toml b/runner/zone_notify/Cargo.toml index a5bc5dfb..0b5c680c 100644 --- a/runner/zone_notify/Cargo.toml +++ b/runner/zone_notify/Cargo.toml @@ -27,7 +27,6 @@ lettre = { version = "=0.11.23", default-features = false, features = [ ] } abnegate-secret = { workspace = true } -zone_core = { path = "../zone_core" } [dev-dependencies] tokio = { workspace = true, features = ["test-util", "macros"] } diff --git a/runner/zone_notify/src/backend/email.rs b/runner/zone_notify/src/backend/email.rs index f38d8261..cdcb9681 100644 --- a/runner/zone_notify/src/backend/email.rs +++ b/runner/zone_notify/src/backend/email.rs @@ -2,12 +2,12 @@ use std::fmt; +use abnegate_secret::sanitize; use async_trait::async_trait; use lettre::message::Mailbox; use lettre::message::header::ContentType; use lettre::transport::smtp::authentication::Credentials; use lettre::{Address, AsyncSmtpTransport, AsyncTransport, Message, Tokio1Executor}; -use zone_core::tools::sanitize; use crate::backend::smtp::SmtpConfig; use crate::channel::Channel; diff --git a/runner/zone_notify/src/backend/webhook.rs b/runner/zone_notify/src/backend/webhook.rs index f17ded7d..99891b8c 100644 --- a/runner/zone_notify/src/backend/webhook.rs +++ b/runner/zone_notify/src/backend/webhook.rs @@ -2,11 +2,11 @@ use std::time::Duration; +use abnegate_secret::sanitize; use futures::StreamExt; use reqwest::redirect::Policy; use reqwest::{Client, Response, StatusCode}; use serde_json::Value; -use zone_core::tools::sanitize; use crate::endpoint::Endpoint; use crate::error::NotifyError; diff --git a/runner/zone_notify/src/lib.rs b/runner/zone_notify/src/lib.rs index d797ac9f..02026117 100644 --- a/runner/zone_notify/src/lib.rs +++ b/runner/zone_notify/src/lib.rs @@ -38,8 +38,9 @@ //! //! # Content //! -//! A notification's text passes through [`zone_core::tools::sanitize`] as it -//! is built, which strips terminal control sequences and redacts credentials. +//! A notification's text passes through [`abnegate_secret::sanitize`] as it +//! is built, which strips terminal control sequences and invisible formatting +//! characters and redacts credentials. //! Bodies carry tool output and user data, a chat channel's history is much //! harder to scrub than a log file, and control sequences in a message can //! forge output in a terminal-based reader. diff --git a/runner/zone_notify/src/notification.rs b/runner/zone_notify/src/notification.rs index 4900bb26..cac9f482 100644 --- a/runner/zone_notify/src/notification.rs +++ b/runner/zone_notify/src/notification.rs @@ -1,8 +1,8 @@ //! What gets delivered. +use abnegate_secret::sanitize; use chrono::{DateTime, Utc}; use url::Url; -use zone_core::tools::sanitize; use crate::field::Field; use crate::severity::Severity; @@ -12,11 +12,12 @@ use crate::severity::Severity; /// Fields are private because the text is sanitized on the way in and that /// invariant has to hold for every backend. A notification body carries tool /// output and user data, so it passes through -/// [`zone_core::tools::sanitize`], which strips terminal control sequences -/// and redacts credentials. A chat channel's history is far harder to scrub -/// than a log file, and control sequences in a message can forge output in -/// any terminal-based reader, so sanitizing happens here, once, rather than -/// in each backend where it could be forgotten. +/// [`abnegate_secret::sanitize`], which strips terminal control sequences +/// and invisible formatting characters and redacts credentials. A chat +/// channel's history is far harder to scrub than a log file, and control +/// sequences in a message can forge output in any terminal-based reader, so +/// sanitizing happens here, once, rather than in each backend where it could +/// be forgotten. #[derive(Clone, Debug, PartialEq, Eq)] pub struct Notification { title: String, From 8b3bb0ad9ac330643db5cd98b59e28c3fb454a55 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 5 Oct 2026 15:53:35 +1300 Subject: [PATCH 2/3] refactor(chat): box the route a chat turn carries Clippy 1.98 flags Routing as a large enum: the Chat variant's Route makes every Routing at least 856 bytes, 216 more than the next largest variant, and clippy -D warnings fails zone_server on main. Boxing the route keeps the enum near the media variants' size; it is built once per turn, so the allocation costs nothing that matters. Co-Authored-By: Claude Opus 5.5 --- runner/zone_server/src/ws/chat.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/runner/zone_server/src/ws/chat.rs b/runner/zone_server/src/ws/chat.rs index 70cc1d5d..df188670 100644 --- a/runner/zone_server/src/ws/chat.rs +++ b/runner/zone_server/src/ws/chat.rs @@ -388,7 +388,7 @@ enum Routing { Video(crate::config::ComfyUiConfig), Audio(crate::config::ComfyUiConfig), Upscale(crate::config::ComfyUiConfig), - Chat(chats::ChatRow, Route), + Chat(chats::ChatRow, Box), } /// What a provider reported when a turn failed, remedied for a coding agent's @@ -2495,7 +2495,7 @@ async fn handle_send_message( return Ok(()); } _ = session.guard.lost() => { return Err(OWNERSHIP_LOST.into()); } - result = prepare_chat(state, stream, chat_id, workspace_id, user_id, content, metadata.as_ref(), chat, route, web_search_requested) => result?, + result = prepare_chat(state, stream, chat_id, workspace_id, user_id, content, metadata.as_ref(), chat, *route, web_search_requested) => result?, }; handle_chat_generation(state, stream, chat_id, workspace_id, user_id, preparation, &mut request, &mut session, &mut jobs).await } @@ -2666,7 +2666,9 @@ async fn prepare_message( } crate::services::image_intent::GenerationIntent::Audio => Routing::Audio(image_config), crate::services::image_intent::GenerationIntent::Upscale => Routing::Upscale(image_config), - crate::services::image_intent::GenerationIntent::Chat => Routing::Chat(chat, route), + crate::services::image_intent::GenerationIntent::Chat => { + Routing::Chat(chat, Box::new(route)) + } }) } @@ -4569,7 +4571,7 @@ mod tests { QUESTION, None, chat, - route, + *route, false, ) .await From a3a861d1d91601a234a0e4761cc4783b506e6043 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 5 Oct 2026 16:20:54 +1300 Subject: [PATCH 3/3] chore(deps): bump rmcp to 3.4.0 and dirs to 7.0.0 abnegate-agent 0.1.x requires rmcp ^3.4, so moving to it later would otherwise drag an MCP SDK bump into an already large migration. Bumping now keeps that PR to the crate swap itself. rmcp 3.3 and 3.4 add ServerConfig/ClientConfig and negotiate_initialize, route peer cancellation by lifecycle and tighten HTTP transport validation; none of zone's call sites change and the negotiated protocol versions are the same. Routing cancellation by lifecycle means a stdio server's notifications/cancelled no longer fails zone's in-flight tools/call at once. The call is then bounded only by ToolContext::command_timeout, so a test now pins that a server which cancels and never answers ends in the timeout error rather than hanging. It fails against rmcp 3.2.0, where the call ended at once with the server's cancellation. dirs 7.0.0 only changes preference_dir on Windows, which zone never calls; the abnegate crates already pull 7.0.0, so zone_core now takes the workspace pin instead of repeating the version. Co-Authored-By: Claude Opus 5.5 --- runner/Cargo.lock | 18 +++---- runner/Cargo.toml | 4 +- runner/zone_core/Cargo.toml | 2 +- runner/zone_core/src/mcp/client.rs | 86 +++++++++++++++++++++++++++++- 4 files changed, 97 insertions(+), 13 deletions(-) diff --git a/runner/Cargo.lock b/runner/Cargo.lock index 0937adfb..7d0577dd 100644 --- a/runner/Cargo.lock +++ b/runner/Cargo.lock @@ -5420,9 +5420,9 @@ dependencies = [ [[package]] name = "process-wrap" -version = "9.1.0" +version = "10.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2e842efad9119158434d193c6682e2ebee4b44d6ad801d7b349623b3f57cdf55" +checksum = "1f21b97672d2dc848e7b25701ab4535618b92f4861c13cc3f7f7bed52ad3c8da" dependencies = [ "futures", "indexmap 2.14.1", @@ -6028,9 +6028,9 @@ dependencies = [ [[package]] name = "rmcp" -version = "3.2.0" +version = "3.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "42b6914fac0be956fe704a38239c3f44a9f841d1b06a5713d2f638065593f5b5" +checksum = "b23c62fe489ac1d401ab32688cfacac3737a8978dc3343e5361464c7724fd3cb" dependencies = [ "chrono", "futures", @@ -6052,9 +6052,9 @@ dependencies = [ [[package]] name = "rmcp-macros" -version = "3.2.0" +version = "3.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cdf1c49bd4d52014b94db0877410db273c2008f01628b0252a2e9460ad9b7fda" +checksum = "cd740c45d66ceb87e5579082abc27bd771665e464e9660a17a048c721b2a6025" dependencies = [ "darling 0.24.1", "proc-macro2", @@ -9798,7 +9798,7 @@ dependencies = [ "async-trait", "base64 0.23.1", "chrono", - "dirs 6.0.0", + "dirs 7.0.0", "fastembed", "futures", "glob", @@ -9850,7 +9850,7 @@ dependencies = [ "async-trait", "chrono", "dashmap", - "dirs 6.0.0", + "dirs 7.0.0", "futures", "nix", "once_cell", @@ -9898,7 +9898,7 @@ dependencies = [ "axum", "bytes", "crc", - "dirs 6.0.0", + "dirs 7.0.0", "futures", "rand 0.10.2", "reqwest 0.13.5", diff --git a/runner/Cargo.toml b/runner/Cargo.toml index 5b4444a8..acaf088e 100644 --- a/runner/Cargo.toml +++ b/runner/Cargo.toml @@ -31,7 +31,7 @@ chrono = { version = "=0.4.45", features = ["serde"] } uuid = { version = "=1.26.0", features = ["v4", "serde"] } reqwest = { version = "=0.13.5", features = ["json", "stream"] } async-trait = "=0.1.92" -rmcp = { version = "=3.2.0", default-features = false } +rmcp = { version = "=3.4.0", default-features = false } # Shared crates, published from abnegate/crates. Caret ranges, unlike every # other dependency here, so a patch release of a crate reaches zone on update. @@ -55,7 +55,7 @@ metrics = "=0.24.6" metrics-exporter-prometheus = "=0.18.3" # CLI dependencies -dirs = "=6.0.0" +dirs = "=7.0.0" toml = "=1.1.5" dialoguer = "=0.12.0" indicatif = "=0.18.6" diff --git a/runner/zone_core/Cargo.toml b/runner/zone_core/Cargo.toml index a63dc3b3..dea02060 100644 --- a/runner/zone_core/Cargo.toml +++ b/runner/zone_core/Cargo.toml @@ -27,7 +27,7 @@ futures = { workspace = true } nix = { workspace = true, features = ["fs"] } # Session storage -dirs = "=6.0.0" +dirs = { workspace = true } abnegate-exec = { workspace = true } abnegate-secret = { workspace = true } diff --git a/runner/zone_core/src/mcp/client.rs b/runner/zone_core/src/mcp/client.rs index 12378680..002c71fc 100644 --- a/runner/zone_core/src/mcp/client.rs +++ b/runner/zone_core/src/mcp/client.rs @@ -230,7 +230,12 @@ mod tests { use crate::mcp::McpServerSpec; use crate::tools::{Tier, ToolContext, ToolRegistry}; use rmcp::handler::server::wrapper::Parameters; - use rmcp::{ServerHandler, ServiceExt, schemars, tool, tool_handler, tool_router}; + use rmcp::model::{ + CancelledNotification, CancelledNotificationParam, ClientJsonRpcMessage, ClientRequest, + InitializeResult, JsonObject, ListToolsResult, ServerJsonRpcMessage, + }; + use rmcp::transport::{IntoTransport, Transport}; + use rmcp::{RoleServer, ServerHandler, ServiceExt, schemars, tool, tool_handler, tool_router}; use serde::Deserialize; use std::collections::HashMap; use std::path::PathBuf; @@ -384,6 +389,85 @@ mod tests { server_task.abort(); } + #[tokio::test] + async fn a_call_the_server_cancels_and_never_answers_times_out() { + let (server_stream, client_stream) = tokio::io::duplex(64 * 1024); + + let server_task = tokio::spawn(async move { + let mut server = IntoTransport::::into_transport(server_stream); + while let Some(message) = server.receive().await { + let ClientJsonRpcMessage::Request(request) = message else { + continue; + }; + let reply = match request.request { + ClientRequest::InitializeRequest(_) => ServerJsonRpcMessage::response( + InitializeResult::default().into(), + request.id, + ), + ClientRequest::ListToolsRequest(_) => ServerJsonRpcMessage::response( + ListToolsResult::with_all_items(vec![Tool::new( + "hang", + "Cancels every call and never answers", + JsonObject::new(), + )]) + .into(), + request.id, + ), + ClientRequest::CallToolRequest(_) => ServerJsonRpcMessage::notification( + CancelledNotification::new(CancelledNotificationParam::new( + Some(request.id), + Some("abandoned".to_string()), + )) + .into(), + ), + _ => continue, + }; + server.send(reply).await.expect("server send"); + } + }); + + let client = ().serve(client_stream).await.expect("client serve"); + let remote_tools = client.list_all_tools().await.expect("list tools"); + let hub = McpHub { + sessions: vec![Arc::new(McpSession { + name: "silent".to_string(), + remote_tools, + client: Mutex::new(client), + })], + }; + let mut registry = ToolRegistry::new(); + assert_eq!(registry.register_mcp(&hub), 1); + + let result = tokio::time::timeout( + Duration::from_secs(30), + registry.execute( + "silent_hang", + serde_json::json!({}), + &ToolContext { + command_timeout: 1, + ..ToolContext::default() + }, + ), + ) + .await + .expect("the command timeout must end a call the server cancelled and never answered") + .expect("execute silent_hang"); + + assert!(!result.success, "{result:?}"); + assert!( + result + .error + .as_deref() + .unwrap_or("") + .contains("timed out after 1 seconds"), + "a server's cancellation no longer ends the call, so the command timeout must: {result:?}" + ); + + drop(registry); + drop(hub); + server_task.abort(); + } + /// A remote method can write files, spend money or message a stranger, and /// nothing Zone can trust says which one it is. It is therefore gated like /// the calls that cannot be taken back, and the card carries the call