GH-164: plugin mcp remove envelope

crates/plugin-mcp/src/dynamic.rs

@@ -7,7 +7,8 @@ use std::collections::BTreeMap;
 use rmcp::model::JsonObject;
 use rmcp::service::{Peer, RoleClient};
 use runvil_plugin::{
-    Pack, PackFuture, PackId, ToolContext, ToolError, ToolErrorKind, ToolId, ToolInfo, ToolOutput,
+    Pack, PackFuture, PackId, ToolCaller, ToolContext, ToolError, ToolErrorKind, ToolId, ToolInfo,
+    ToolOutput,
 };
 use serde_json::{Map, Value, json};
 use tokio::runtime::Handle;
@@ -70,6 +71,16 @@ pub(crate) struct Mcp {
     runtime: Handle,
 }
 
+/// Marks `text` by who called: the section's own script receives it
+/// trusted, and every other caller, a model round included, untrusted,
+/// because the text comes from the servers.
+fn marked(caller: ToolCaller, text: String) -> ToolOutput {
+    match caller {
+        ToolCaller::Script => ToolOutput::trusted(text),
+        _ => ToolOutput::untrusted(text),
+    }
+}
+
 fn invalid(text: impl Into<String>) -> ToolError {
     ToolError::message(text).with_kind(ToolErrorKind::InvalidArguments)
 }
@@ -265,9 +276,9 @@ impl Mcp {
         })
     }
 
-    /// Performs `call_dynamic_tool`. Every argument is checked before
-    /// anything is sent.
-    async fn call_tool(&self, args: Value) -> Result<ToolOutput, ToolError> {
+    /// Performs `call_dynamic_tool` and returns the result's text. Every
+    /// argument is checked before anything is sent.
+    async fn call_tool(&self, args: Value) -> Result<String, ToolError> {
         let mut members = members(
             args,
             "call_dynamic_tool takes a JSON object with the strings `namespace` and `toolName` \
@@ -303,10 +314,11 @@ impl Pack for Mcp {
     ) -> PackFuture<'a, Result<ToolOutput, ToolError>> {
         Box::pin(async move {
             let tool = cx.tool();
+            let caller = cx.origin().caller;
             if *tool == self.get.id {
-                self.list(args).map(ToolOutput::untrusted)
+                self.list(args).map(|text| marked(caller, text))
             } else if *tool == self.call.id {
-                self.call_tool(args).await
+                self.call_tool(args).await.map(|text| marked(caller, text))
             } else {
                 Err(ToolError::message(format!("MCP has no tool {tool}")))
             }

crates/plugin-mcp/src/lib.rs

@@ -23,6 +23,10 @@
 //!   an optional object `arguments`, which defaults to `{}`, and calls that
 //!   server's tool.
 //!
+//! Both tools answer a model round with untrusted text, and the section's
+//! own script with trusted text, so the script receives the server's text
+//! without the envelope.
+//!
 //! A namespace is the server's name exactly as the configuration writes
 //! it, and a tool's name is the one its server lists. Both must match
 //! exactly. A server's status is `ready`, `loading` while it is still
@@ -79,8 +83,10 @@
 //! - May depend on: `runvil-plugin`, `shared-*` crates, and outside
 //!   libraries. `cargo test -p build-xtask` enforces the Plugin family's
 //!   allow-list. Only [`install`] is public.
-//! - Every output is untrusted, because descriptions, schemas, and results
-//!   all come from the servers.
+//! - Output to a model round is untrusted, because descriptions, schemas,
+//!   and results all come from the servers, and output to the section's
+//!   own script is trusted, so the script receives the server's text
+//!   without the envelope.
 //! - No reason, log line, `Debug` output, or error names an `env` or
 //!   `headers` value. They may name keys.
 //! - It runs on the Host's runtime, read through the contract's

crates/plugin-mcp/src/result-tests.rs

@@ -1,10 +1,10 @@
-//! Tests for how an MCP answer becomes tool output or a tool error.
+//! Tests for how an MCP answer becomes text or a tool error.
 
 use std::time::Duration;
 
 use rmcp::ServiceError;
 use rmcp::model::{CallToolResult, ServerResult};
-use runvil_plugin::{OutputTrust, ToolErrorKind};
+use runvil_plugin::ToolErrorKind;
 use serde_json::{Value, json};
 
 use super::{output, render, service_error};
@@ -14,9 +14,7 @@ fn result(value: Value) -> CallToolResult {
 }
 
 fn text_of(value: Value) -> String {
-    let output = render(&result(value)).expect("a success renders");
-    assert_eq!(output.trust(), OutputTrust::Untrusted);
-    output.text().to_owned()
+    render(&result(value)).expect("a success renders")
 }
 
 #[test]
@@ -83,7 +81,7 @@ fn content_blocks_win_over_structured_content() {
 }
 
 #[test]
-fn a_result_with_nothing_is_empty_untrusted_text() {
+fn a_result_with_nothing_is_empty_text() {
     assert_eq!(text_of(json!({ "content": [] })), "");
 }
 
@@ -165,8 +163,7 @@ fn a_failed_request_maps_through_output_and_a_good_one_renders() {
         json!({ "content": [{ "type": "text", "text": "fine" }] }),
     ))))
     .expect("renders");
-    assert_eq!(good.text(), "fine");
-    assert_eq!(good.trust(), OutputTrust::Untrusted);
+    assert_eq!(good, "fine");
 }
 
 #[test]

crates/plugin-mcp/src/result.rs

@@ -1,8 +1,8 @@
-//! Turns an MCP answer into tool output or a tool error.
+//! Turns an MCP answer into text or a tool error.
 
 use rmcp::ServiceError;
 use rmcp::model::{CallToolResult, ContentBlock, ResourceContents, ServerResult};
-use runvil_plugin::{ToolError, ToolErrorKind, ToolOutput};
+use runvil_plugin::{ToolError, ToolErrorKind};
 
 /// What a content block this change does not know becomes. rmcp's content
 /// types are `#[non_exhaustive]`, so a new kind reaches this text.
@@ -41,10 +41,8 @@ pub(crate) fn service_error(error: &ServiceError) -> ToolError {
     ToolError::message(text).with_kind(kind)
 }
 
-/// Maps the answer to a `tools/call` request.
-pub(crate) fn output(
-    response: Result<ServerResult, ServiceError>,
-) -> Result<ToolOutput, ToolError> {
+/// Maps the answer to a `tools/call` request to its text.
+pub(crate) fn output(response: Result<ServerResult, ServiceError>) -> Result<String, ToolError> {
     match response.map_err(|e| service_error(&e))? {
         ServerResult::CallToolResult(result) => render(&result),
         _ => Err(ToolError::message(
@@ -56,7 +54,7 @@ pub(crate) fn output(
 
 /// Joins a result's text. An `is_error` result becomes a backend error that
 /// carries the server's text.
-pub(crate) fn render(result: &CallToolResult) -> Result<ToolOutput, ToolError> {
+pub(crate) fn render(result: &CallToolResult) -> Result<String, ToolError> {
     let text = text_of(result);
     if result.is_error == Some(true) {
         let message = if text.is_empty() {
@@ -66,7 +64,7 @@ pub(crate) fn render(result: &CallToolResult) -> Result<ToolOutput, ToolError> {
         };
         return Err(ToolError::message(message).with_kind(ToolErrorKind::Backend));
     }
-    Ok(ToolOutput::untrusted(text))
+    Ok(text)
 }
 
 /// The text blocks, embedded text resources, and placeholders, joined with

crates/plugin-mcp/src/server.rs

@@ -8,7 +8,7 @@ use rmcp::model::{
     RequestId, Tool,
 };
 use rmcp::service::{Peer, PeerRequestOptions, RoleClient};
-use runvil_plugin::{ToolError, ToolErrorKind, ToolOutput};
+use runvil_plugin::{ToolError, ToolErrorKind};
 use serde_json::Value;
 use tokio::runtime::Handle;
 use tokio::sync::watch;
@@ -96,7 +96,7 @@ impl Drop for Server {
 }
 
 /// A spawned call, aborted when the call awaiting it is dropped.
-struct AbortOnDrop(JoinHandle<Result<ToolOutput, ToolError>>);
+struct AbortOnDrop(JoinHandle<Result<String, ToolError>>);
 
 impl Drop for AbortOnDrop {
     fn drop(&mut self) {
@@ -125,14 +125,14 @@ impl Drop for CancelOnDrop {
     }
 }
 
-/// Sends one `tools/call` and renders the answer. It runs on the Host's
+/// Sends one `tools/call` and renders the answer as text. It runs on the Host's
 /// runtime, so dropping the awaiting call aborts it and fires the guard.
 async fn send(
     peer: Peer<RoleClient>,
     name: String,
     arguments: JsonObject,
     runtime: Handle,
-) -> Result<ToolOutput, ToolError> {
+) -> Result<String, ToolError> {
     let request = ClientRequest::CallToolRequest(CallToolRequest::new(
         CallToolRequestParams::new(name).with_arguments(arguments),
     ));
@@ -157,7 +157,7 @@ pub(crate) async fn call(
     peer: Peer<RoleClient>,
     name: String,
     arguments: JsonObject,
-) -> Result<ToolOutput, ToolError> {
+) -> Result<String, ToolError> {
     let mut task = AbortOnDrop(runtime.spawn(send(peer, name, arguments, runtime.clone())));
     match (&mut task.0).await {
         Ok(outcome) => outcome,

crates/plugin-mcp/tests/it/calls.rs

@@ -5,11 +5,11 @@
 use std::time::Duration;
 
 use runvil_plugin::testing::TestCall;
-use runvil_plugin::{ToolErrorKind, ToolId};
+use runvil_plugin::{OutputTrust, ToolErrorKind, ToolId};
 use serde_json::json;
 
 use crate::fixture::Fixture;
-use crate::support::{Silent, call, invoke, pack_over, ready, remote, settled};
+use crate::support::{Silent, call, invoke, invoke_as_script, pack_over, ready, remote, settled};
 
 #[tokio::test(flavor = "multi_thread")]
 async fn a_call_sends_the_tools_own_name_and_returns_untrusted_text() {
@@ -23,6 +23,40 @@ async fn a_call_sends_the_tools_own_name_and_returns_untrusted_text() {
     assert_eq!(text, r#"echo: {"q":"hello"}"#);
 }
 
+#[tokio::test(flavor = "multi_thread")]
+async fn a_script_call_returns_the_servers_text_trusted_without_the_envelope() {
+    let fixture = Fixture::start().await;
+    let pack = pack_over(json!({ "GitHub": remote(&fixture, &json!({})) }));
+    ready(&pack, "GitHub").await;
+
+    let output = invoke_as_script(
+        &pack,
+        "call_dynamic_tool",
+        json!({ "namespace": "GitHub", "toolName": "Echo", "arguments": { "q": "hello" } }),
+    )
+    .await
+    .expect("succeeds");
+    assert_eq!(output.trust(), OutputTrust::Trusted);
+    assert_eq!(output.text(), r#"echo: {"q":"hello"}"#);
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn a_script_calls_is_error_result_is_the_same_backend_error_as_a_models() {
+    let fixture = Fixture::start().await;
+    let pack = pack_over(json!({ "GitHub": remote(&fixture, &json!({})) }));
+    ready(&pack, "GitHub").await;
+
+    let error = invoke_as_script(
+        &pack,
+        "call_dynamic_tool",
+        json!({ "namespace": "GitHub", "toolName": "fail", "arguments": {} }),
+    )
+    .await
+    .expect_err("isError fails");
+    assert_eq!(error.kind(), ToolErrorKind::Backend);
+    assert_eq!(error.to_string(), "it failed");
+}
+
 #[tokio::test(flavor = "multi_thread")]
 async fn a_call_without_arguments_sends_an_empty_object() {
     let fixture = Fixture::start().await;

crates/plugin-mcp/tests/it/listing.rs

@@ -3,11 +3,11 @@
 
 use std::time::Duration;
 
-use runvil_plugin::ToolErrorKind;
+use runvil_plugin::{OutputTrust, ToolErrorKind};
 use serde_json::{Value, json};
 
 use crate::fixture::Fixture;
-use crate::support::{SECRET, Silent, invoke, list, pack_over, ready, remote};
+use crate::support::{SECRET, Silent, invoke, invoke_as_script, list, pack_over, ready, remote};
 
 /// The fixture's tools as the listing with no arguments shows them.
 fn named_tools() -> Value {
@@ -72,6 +72,20 @@ async fn the_listing_shows_each_server_by_its_own_name_with_the_tools_it_lists()
     );
 }
 
+#[tokio::test(flavor = "multi_thread")]
+async fn a_script_listing_is_trusted_json_equal_to_the_models() {
+    let fixture = Fixture::start().await;
+    let pack = pack_over(json!({ "GitHub": remote(&fixture, &json!({})) }));
+    ready(&pack, "GitHub").await;
+
+    let output = invoke_as_script(&pack, "get_dynamic_tools", Value::Null)
+        .await
+        .expect("the listing answers");
+    assert_eq!(output.trust(), OutputTrust::Trusted);
+    let listing: Value = serde_json::from_str(output.text()).expect("the listing is JSON");
+    assert_eq!(listing, list(&pack, json!({})).await);
+}
+
 #[tokio::test(flavor = "multi_thread")]
 async fn a_namespace_lists_its_tools_with_their_schemas_and_a_tool_name_lists_one() {
     let fixture = Fixture::start().await;

crates/plugin-mcp/tests/it/support.rs

@@ -8,7 +8,7 @@ use std::time::Duration;
 use runvil_plugin::testing::TestCall;
 use runvil_plugin::{
     HostServices, OutputTrust, Pack, PackId, PackInfo, PackRegistry, ServiceKey, TOKIO_RUNTIME,
-    ToolError, ToolId,
+    ToolCaller, ToolError, ToolId, ToolOutput,
 };
 use serde_json::{Map, Value, json};
 use tokio::net::TcpListener;
@@ -64,23 +64,37 @@ pub(crate) fn remote(fixture: &Fixture, headers: &Value) -> Value {
     json!({ "url": fixture.url, "headers": headers })
 }
 
-/// Calls the Pack's tool `mcp/<tool>` with `args` and returns its text,
-/// checking that the output is untrusted.
+fn tool_call(tool: &str) -> TestCall {
+    TestCall::new(ToolId::parse(&format!("mcp/{tool}")).expect("a tool id parses"))
+}
+
+/// Calls the Pack's tool `mcp/<tool>` with `args` from a model round and
+/// returns its text, checking that the output is untrusted.
 pub(crate) async fn invoke(
     pack: &Arc<dyn Pack>,
     tool: &str,
     args: Value,
 ) -> Result<String, ToolError> {
-    let test = TestCall::new(ToolId::parse(&format!("mcp/{tool}")).expect("a tool id parses"));
+    let test = tool_call(tool).with_caller(ToolCaller::Model);
     let output = pack.call(test.context(), args).await?;
     assert_eq!(
         output.trust(),
         OutputTrust::Untrusted,
-        "every output is untrusted"
+        "output to a model round is untrusted"
     );
     Ok(output.text().to_owned())
 }
 
+/// Calls the Pack's tool `mcp/<tool>` with `args` from the section's own
+/// script and returns the whole output.
+pub(crate) async fn invoke_as_script(
+    pack: &Arc<dyn Pack>,
+    tool: &str,
+    args: Value,
+) -> Result<ToolOutput, ToolError> {
+    pack.call(tool_call(tool).context(), args).await
+}
+
 /// The JSON `get_dynamic_tools` answers for `args`.
 pub(crate) async fn list(pack: &Arc<dyn Pack>, args: Value) -> Value {
     let text = invoke(pack, "get_dynamic_tools", args)

crates/runvil-plugin/src/testing.rs

@@ -8,8 +8,10 @@ use crate::{HostServices, ToolContext};
 /// One tool call a test makes directly, without a Harness.
 ///
 /// It owns everything a [`ToolContext`] borrows: the called tool's id, an
-/// access over a fresh in-memory filesystem, an origin naming the script
-/// of the section `Test`, and the run's services, which are empty unless
+/// access over a fresh in-memory filesystem, an origin naming the section
+/// `Test` and a caller that is the script unless
+/// [`with_caller`](TestCall::with_caller) names another, and the run's
+/// services, which are empty unless
 /// [`with_services`](TestCall::with_services) supplies them.
 #[derive(Debug)]
 pub struct TestCall {
@@ -51,6 +53,14 @@ impl TestCall {
         self
     }
 
+    /// Returns the call with `caller` as the origin's caller, keeping its
+    /// execution and section.
+    #[must_use]
+    pub fn with_caller(mut self, caller: ToolCaller) -> TestCall {
+        self.origin = ToolCallOrigin::new("plugin-test", "Test", caller);
+        self
+    }
+
     /// Lends the context to one call.
     #[must_use]
     pub fn context(&self) -> ToolContext<'_> {

crates/runvil-plugin/tests/it/test_call.rs

@@ -33,6 +33,15 @@ fn a_test_call_lends_its_tool_a_script_origin_and_a_working_filesystem() {
     );
 }
 
+#[test]
+fn a_test_call_with_a_caller_lends_that_caller() {
+    let call = TestCall::new(inspect()).with_caller(ToolCaller::Model);
+    let cx = call.context();
+    assert_eq!(cx.tool(), &inspect());
+    assert_eq!(cx.origin().caller, ToolCaller::Model);
+    assert_eq!(cx.origin().section, "Test");
+}
+
 #[test]
 fn a_test_call_lends_the_services_it_was_given() {
     let mut services = HostServices::new();