From a36104729620a74927b947c021c5f3b0de6404b0 Mon Sep 17 00:00:00 2001 From: Jason Brazeal Date: Wed, 21 Oct 2026 12:27:16 -0700 Subject: [PATCH 1/3] Let TestCall name the calling side of a tool call Plugin tests can now build a direct tool call that appears to come from a model round instead of the section's script. A call still comes from the script by default, so existing tests keep their behavior. This lets a Pack test output that depends on who made the call. - `TestCall::with_caller` rebuilds the origin with `ToolCallOrigin::new`, keeping execution `plugin-test` and section `Test`. It is public within the `test-support` module and returns the call by value like `with_services`. - `a_test_call_with_a_caller_lends_that_caller` asserts that a call built with `ToolCaller::Model` lends that caller, section `Test`, and the given tool. Design: new surface-growth @ crates/runvil-plugin/src/testing.rs::TestCall::with_caller boundary: pub Plan: vibe/2026-10-21-1-mcp-script-raw-text.md --- crates/runvil-plugin/src/testing.rs | 14 +- crates/runvil-plugin/tests/it/test_call.rs | 9 ++ vibe/2026-10-21-1-mcp-script-raw-text.md | 162 +++++++++++++++++++++ vibe/ACTIVE | 1 + 4 files changed, 184 insertions(+), 2 deletions(-) create mode 100644 vibe/2026-10-21-1-mcp-script-raw-text.md create mode 100644 vibe/ACTIVE diff --git a/crates/runvil-plugin/src/testing.rs b/crates/runvil-plugin/src/testing.rs index daadc73db..ac7e20b6c 100644 --- a/crates/runvil-plugin/src/testing.rs +++ b/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<'_> { diff --git a/crates/runvil-plugin/tests/it/test_call.rs b/crates/runvil-plugin/tests/it/test_call.rs index c0916c272..752e26d91 100644 --- a/crates/runvil-plugin/tests/it/test_call.rs +++ b/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(); diff --git a/vibe/2026-10-21-1-mcp-script-raw-text.md b/vibe/2026-10-21-1-mcp-script-raw-text.md new file mode 100644 index 000000000..9324fb1b2 --- /dev/null +++ b/vibe/2026-10-21-1-mcp-script-raw-text.md @@ -0,0 +1,162 @@ +--- +name: MCP script calls return raw text +overview: "plugin-mcp marks its output by caller: a call the section's Lua makes through tools.call gets the raw text back, and a call a model round makes keeps the untrusted envelope. Both of the Pack's tools follow the rule. runvil-plugin's TestCall gains a way to name the caller so the Pack's tests can exercise both paths." +todos: + - id: s1-test-call-caller + content: "S1: add TestCall::with_caller to runvil-plugin's test support, with a test that a call built with ToolCaller::Model lends a model origin" + status: pending + - id: s2-mcp-trust-by-caller + content: "S2: plugin-mcp returns trusted output to ToolCaller::Script and untrusted output to every other caller, for both get_dynamic_tools and call_dynamic_tool; update the crate docs and tests" + status: pending +isProject: false +--- + +# plugin-mcp: No Envelope on Lua Calls + + + +## Product Requirements + +- Ticket: GH-164, "plugin-mcp: no envelope on Lua calls". The ticket's whole text: "Script-issued MCP calls return the raw text string, and the model path keeps its envelope." +- Repository: `runvil` (`/code/runvil`), branch `GH-164-plugin-mcp-remove-envelope`, at `31941a46`. +- Problem: `plugin-mcp` returns every output as `ToolOutput::untrusted`. The shared tool dispatch (`crates/runvil-internal/lua/src/dispatch.rs`, the `match output.trust()` after the call) nonce-wraps untrusted output before it reaches either the next model turn or the calling script. A section's Lua that calls `mcp/call_dynamic_tool` or `mcp/get_dynamic_tools` through `tools.call` therefore gets the envelope, not the server's text. The crate docs already promise that a script reads the listing with `json.decode`, which the envelope breaks. +- Goals + - A call whose origin's caller is `ToolCaller::Script` gets the raw text: the exact string the call renders today before wrapping, with no envelope. + - A call whose origin's caller is `ToolCaller::Model`, or any caller a future version adds, keeps the untrusted envelope exactly as today. + - The rule covers both of the Pack's tools, `get_dynamic_tools` and `call_dynamic_tool`. +- Non-goals: no change to the Engine, the Harness, the dispatch's wrapping, the envelope format, other Packs, error mapping, or the text a call renders. No change to `ToolCallOrigin` or `ToolCaller`. +- Success criteria + - Through the Pack's own tests: a script-origin call of either tool returns `OutputTrust::Trusted` with the same text as before; a model-origin call returns `OutputTrust::Untrusted` with the same text. + - Errors, including an `isError` result, stay `ToolError`s for both callers. + - Every touched test, lint, format, and docs gate passes. + +## Functional Specification + +- `mcp/call_dynamic_tool` called by a script: returns the joined text of the result (text blocks, embedded text resources, and placeholders joined with blank lines, or the `structuredContent` JSON), marked trusted, so the Lua caller receives that string verbatim. +- `mcp/get_dynamic_tools` called by a script: returns the listing JSON text, marked trusted, so `json.decode` on the returned string succeeds. +- Either tool called by a model round: returns the same text marked untrusted, so the dispatch nonce-wraps it as today. +- A caller that is neither `Script` nor `Model` (the enum is `#[non_exhaustive]`) is treated as a model call: untrusted. +- Failures are unchanged for both callers: argument refusals, unknown namespaces or tools, servers not ready, transport errors, and `isError` results are `ToolError`s with the same kinds and messages. + + + + +## Technical Design + +- `runvil-plugin` (test support only, behind the `test-support` feature) + - `crates/runvil-plugin/src/testing.rs`: add `TestCall::with_caller(self, caller: ToolCaller) -> TestCall`, a builder like `with_services`, which replaces the origin's caller and keeps its execution (`plugin-test`) and section (`Test`). `TestCall::new` keeps the `ToolCaller::Script` default. Update the struct doc, which says the origin names "the script of the section `Test`", to say the caller is the script unless `with_caller` names another. + - `ToolCallOrigin`'s fields are public and `#[non_exhaustive]` blocks struct literals outside its crate, so rebuild the origin with `ToolCallOrigin::new("plugin-test", "Test", caller)`. +- `plugin-mcp` + - Trust is decided once, in `Mcp::call` (`crates/plugin-mcp/src/dynamic.rs`), from `cx.origin().caller`. Add a private free function in `dynamic.rs`, `fn marked(caller: ToolCaller, text: String) -> ToolOutput`, that returns `ToolOutput::trusted(text)` for `ToolCaller::Script` and `ToolOutput::untrusted(text)` for every other caller (a `_` arm, because `ToolCaller` is `#[non_exhaustive]`). + - The text path below `Mcp::call` returns plain text instead of `ToolOutput`: `result::render` and `result::output` return `Result`; `server::send`, `server::call`, and `AbortOnDrop`'s join handle carry `Result`; `Mcp::call_tool` returns `Result`. `Mcp::list` already returns text. `Mcp::call` maps both tools' `Ok` text through `marked`. Error construction is untouched. Doc comments that say "tool output" for these functions say "text" instead. + - `crates/plugin-mcp/src/lib.rs` crate docs: replace the invariant "Every output is untrusted, because descriptions, schemas, and results all come from the servers." with one stating that 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. Add one sentence to the description of the two tools saying the same thing for a reader of the tool list. +- Boundaries: `plugin-mcp` keeps depending only on `runvil-plugin` among workspace crates (`ToolCaller` is re-exported there). No public item of `plugin-mcp` changes; only `install` is public. + + + + +## Testing Plan + +- Step 1 (`runvil-plugin`): in `crates/runvil-plugin/tests/it/test_call.rs`, add a test that `TestCall::new(tool).with_caller(ToolCaller::Model)` lends a context whose `origin().caller` is `ToolCaller::Model`, whose `origin().section` is `Test`, and whose `tool()` is the given tool. It fails to compile before `with_caller` exists. The existing test that `TestCall::new` lends a script origin stays and pins the default. +- Step 2 (`plugin-mcp`) + - `crates/plugin-mcp/tests/it/support.rs`: `invoke` builds its `TestCall` with `.with_caller(ToolCaller::Model)` and keeps asserting `OutputTrust::Untrusted`, with the assertion message changed to say model-round output is untrusted. Every existing test that goes through `invoke`, `list`, or `call` thereby keeps pinning the model path. Add a helper that calls a tool as the script (`TestCall::new`, whose default caller is `Script`) and returns the whole `ToolOutput`. + - New integration tests (in `tests/it/calls.rs` for the call tool, in `tests/it/listing.rs` for the listing), names stating the behavior: + - A script call of `call_dynamic_tool` to the fixture's `Echo` tool with `{ "q": "hello" }` returns `OutputTrust::Trusted` and the text `echo: {"q":"hello"}` exactly. Before the change it is `Untrusted`, so the test fails. + - A script call of `get_dynamic_tools` with no arguments returns `OutputTrust::Trusted`, and its text parses as JSON equal to the model-path listing for the same Pack. + - A script call of `call_dynamic_tool` to the fixture's `fail` tool is a `ToolErrorKind::Backend` error with the message `it failed`, the same as the model path. + - `crates/plugin-mcp/src/result-tests.rs`: `render` and `output` now return text, so drop the trust assertions there (trust is no longer `result.rs`'s concern) and assert on the returned strings. Keep every text-rendering case. + - The focused run must show the new script-path tests failing before the source change and passing after. +- Exit checks at the final step, full scope, from the project survey: format check, both clippy runs, the full test suites, the docs builds, `cargo test -p build-xtask`, and `node --test crates/workshop/ui/test/docs-claims.mjs` (the crate docs change touches an `## Invariants` section). + + + + +## Decision Record + +- User-resolved choices + - Trust boundary: output to the section's own script is trusted, output to a model round stays untrusted. This is the ticket's requirement, settled by the operator. Consequence: a script that forwards MCP text into a prompt does so without the envelope, which is the script author's decision, just as with any other value the script builds. + - Scope: both tools, `get_dynamic_tools` and `call_dynamic_tool`, follow the rule (the operator: "unwrap both"). Reason: a script that lists tools wants to decode the listing JSON, which the crate docs already promise and the envelope breaks. +- Reversible decisions and consequences + - Decide trust in the Pack from `ToolContext::origin`, not in the Engine's dispatch. The Plugin contract already hands each call its origin "to apply different policy to the same tool depending on who called it", and the dispatch honors the output's own trust marking, so no Engine change is needed. Rejected: making the dispatch skip wrapping for every script call, because that changes every Pack's contract, not this ticket's. + - An unknown future `ToolCaller` variant gets untrusted output, the safe default, matching the dispatch's own handling of an unknown `OutputTrust`. + - Trust is decided at one point, `Mcp::call`, and the layers below return plain text. Rejected: passing the caller down into `server` and `result`, because it would thread a policy value through transport code that does not need it. + - Tests reach the model path through a new `TestCall::with_caller` in `runvil-plugin`'s test support. Rejected: building a `ToolContext` by hand in `plugin-mcp`'s tests, because an `Access` needs `runvil-vfs` constructors that `runvil-plugin` does not re-export, and `plugin-mcp` may depend only on `runvil-plugin` among workspace crates. +- Assumptions and risks + - Assumption: a trusted script result is not otherwise treated as model input; the dispatch hands the content to the script and records the result's trust in the tool-result report. + - Risk: none to stored data or wire formats; the change affects only the trust marking of in-run output. + +### Deferred and Out of Scope + +- Any change to how other Packs (`plugin-web`) mark script-issued output - revisit with its own ticket if wanted. + + + + +## Project Survey + +- Status: complete +- Build command: `cargo build --locked -p ` (plain `cargo build` builds only `crates/gateway/app`, the sole default member; the desktop app is `cargo build -p workshop`). +- Focused test command pattern: `cargo nextest run --locked -p -E 'test()'` (nextest config in `.config/nextest.toml`). +- Component test command pattern: `cargo nextest run --locked -p --all-features` (e.g. `-p plugin-mcp`); workshop crates run without `--all-features`. +- Full-suite test command: `cargo nextest run --locked --workspace --exclude workshop --exclude workshop-server --exclude workshop-server-api --all-features`, then `cargo nextest run --locked -p workshop -p workshop-server -p workshop-server-api`; plus `cargo test -p build-xtask` (structural checks) and `node --test crates/workshop/ui/test/docs-claims.mjs` (wording scan). +- Linter command: `CARGO_BUILD_WARNINGS=deny cargo clippy --workspace --exclude workshop --exclude workshop-server --exclude workshop-server-api --all-targets --all-features` and `CARGO_BUILD_WARNINGS=deny cargo clippy -p workshop -p workshop-server -p workshop-server-api --all-targets`; extra gate `cargo check -p gateway --no-default-features`. +- Formatter check command: `cargo fmt --all --check`. +- Docs command: `RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps --all-features --exclude workshop --exclude workshop-server --exclude workshop-server-api` and `RUSTDOCFLAGS="-D warnings" cargo doc -p runvil --no-deps`; facade surface `cargo + xtask api --check` with `CARGO_BUILD_WARNINGS` unset (checks `crates/runvil/public-api.txt`). +- Test placement and naming conventions: unit tests live in a kebab sibling `foo-tests.rs` wired from `foo.rs` via `#[path = "foo-tests.rs"] mod tests;` (e.g. `crates/plugin-mcp/src/{connect,entry,result,server}-tests.rs`); integration tests are a single target per crate under `tests/it/main.rs` or `tests/suite/main.rs` with topic modules (`calls.rs`, `listing.rs`) plus `support.rs`/`fixture.rs`; prompt fixtures under `tests/prompts/`. Engine tests: no tokio, sleeps, wall clock, or OS entropy; fixed seeds; temp dirs named from test name, pid, counter. +- Directory map: `crates/` top level = public/shared layer (`runvil` Engine facade, `runvil-plugin` Plugin contract, `harness`, `harness-gateway-client`, `plugin-mcp`/`plugin-web`/`plugin-user-input` Plugins, `gateway-api-types`/`gateway-api-discovery`, `shared-*`, `build-*` tooling incl. `build-xtask`); containers of private crates: `crates/runvil-internal/` (types, engine, lua, parser, vfs, model-client), `crates/harness-internal/` (runner, plugin-loader, test-plugin), `crates/gateway/` (app, config, routing, local, stt/*, web-search, ...), `crates/workshop/` (desktop, server, server-api, ui SPA, look CSS, and support crates); `crates/shared-ui` is a TS/CSS package; `guide/` user guide books; `prompts/` sample prompts; `tools/` Node scripts; `vibe/` dated plan docs; `.config/nextest.toml`. +- Component boundaries: Engine (`runvil` → `runvil-internal/*`) depends on no tokio/reqwest/async-trait/getrandom/fastrand and never names the Host; `runvil-plugin` defines Pack/tool contracts; Plugins (e.g. `plugin-mcp` → `runvil-plugin`, `rmcp`, `reqwest`, `tokio`) depend on the contract, not the Harness; `harness` → `harness-runner`, `harness-plugin-loader`, `runvil`, `runvil-plugin`; Hosts (Workshop) → Harness; families reach each other only through top-level crates; boundaries enforced by `cargo test -p build-xtask`. +- Conventions summary: edition 2024, rust-version 1.95, workspace-inherited deps and lints with comments explaining each dependency's purpose; flat source dirs with `parent-label.rs` siblings below three files; capitalized defined terms (Engine, Harness, Host, Pack, Plugin) in docs/comments; crate docs carry `## Invariants`; model-oriented error messages naming required vs actual; recorded JSON round-trips exactly with sorted keys; behavior changes ship with tests; comments only for non-obvious constraints, citing upstream issue URLs for workarounds. + + + + +## Execution Instructions + + + +### Step 1: TestCall::with_caller in runvil-plugin test support [completed] + +- Component: none +- Tests: `cargo nextest run --locked -p runvil-plugin --all-features` + +- Scope: `runvil-plugin` test support only, behind the `test-support` feature. No change to `ToolCallOrigin` or `ToolCaller`. +- `crates/runvil-plugin/src/testing.rs`: + - Add `TestCall::with_caller(self, caller: ToolCaller) -> TestCall`, a builder shaped like `with_services`, which replaces the origin's caller and keeps its execution (`plugin-test`) and section (`Test`). Rebuild the origin with `ToolCallOrigin::new("plugin-test", "Test", caller)`, since `ToolCallOrigin` is `#[non_exhaustive]` and struct literals are blocked outside its crate. + - `TestCall::new` keeps the `ToolCaller::Script` default. + - Update the `TestCall` struct doc, which says the origin names "the script of the section `Test`", to say the caller is the script unless `with_caller` names another. +- `crates/runvil-plugin/tests/it/test_call.rs`: add a test (e.g. `a_test_call_with_a_caller_lends_that_caller`) that `TestCall::new(tool).with_caller(ToolCaller::Model)` lends a context whose `origin().caller` is `ToolCaller::Model`, whose `origin().section` is `Test`, and whose `tool()` is the given tool. It fails to compile before `with_caller` exists. Keep `a_test_call_lends_its_tool_a_script_origin_and_a_working_filesystem`, which pins the `Script` default. +- Commit: code and test together. + + + + + +### Step 2: plugin-mcp marks output by caller for both tools + +- Component: none +- Tests: `cargo nextest run --locked -p plugin-mcp --all-features`; exit checks: `cargo fmt --all --check`; `CARGO_BUILD_WARNINGS=deny cargo clippy --workspace --exclude workshop --exclude workshop-server --exclude workshop-server-api --all-targets --all-features`; `CARGO_BUILD_WARNINGS=deny cargo clippy -p workshop -p workshop-server -p workshop-server-api --all-targets`; `cargo nextest run --locked --workspace --exclude workshop --exclude workshop-server --exclude workshop-server-api --all-features`; `cargo nextest run --locked -p workshop -p workshop-server -p workshop-server-api`; `RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps --all-features --exclude workshop --exclude workshop-server --exclude workshop-server-api`; `RUSTDOCFLAGS="-D warnings" cargo doc -p runvil --no-deps`; `cargo test -p build-xtask`; `node --test crates/workshop/ui/test/docs-claims.mjs` + +- Behavior (ticket GH-164: "Script-issued MCP calls return the raw text string, and the model path keeps its envelope."): + - `mcp/call_dynamic_tool` called with origin caller `ToolCaller::Script` returns the joined text of the result (text blocks, embedded text resources, and placeholders joined with blank lines, or the `structuredContent` JSON), marked trusted, exactly the string rendered today before wrapping. + - `mcp/get_dynamic_tools` called by `ToolCaller::Script` returns the listing JSON text, marked trusted, so `json.decode` succeeds. + - Either tool called by `ToolCaller::Model`, or any other caller (`ToolCaller` is `#[non_exhaustive]`), returns the same text marked untrusted, so the dispatch nonce-wraps it as today. + - Failures unchanged for both callers: argument refusals, unknown namespaces or tools, servers not ready, transport errors, and `isError` results stay `ToolError`s with the same kinds and messages. + - No change to the Engine, the Harness, the dispatch's wrapping, the envelope format, other Packs (`plugin-web` is out of scope), error mapping, or rendered text. `plugin-mcp` keeps depending only on `runvil-plugin` among workspace crates (`ToolCaller` is re-exported there); no public item changes (only `install` is public). +- Source, `crates/plugin-mcp/src/`: + - `dynamic.rs`: add private `fn marked(caller: ToolCaller, text: String) -> ToolOutput` returning `ToolOutput::trusted(text)` for `ToolCaller::Script` and `ToolOutput::untrusted(text)` in a `_` arm. `Mcp::call` reads `cx.origin().caller` once and maps both tools' `Ok` text through `marked`. `Mcp::call_tool` returns `Result`; `Mcp::list` already returns text. + - `result.rs`: `result::render` and `result::output` return `Result`. + - `server.rs`: `server::send`, `server::call`, and `AbortOnDrop`'s join handle carry `Result`. + - Error construction untouched. Doc comments that say "tool output" for these functions say "text" instead. + - `lib.rs` crate docs: replace the `## Invariants` line "Every output is untrusted, because descriptions, schemas, and results all come from the servers." with one stating that 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. Add one sentence to the description of the two tools saying the same for a reader of the tool list. Use capitalized defined terms (Pack, Engine, Harness) per the wording scan. +- Tests: + - `crates/plugin-mcp/tests/it/support.rs`: `invoke` builds its `TestCall` with `.with_caller(ToolCaller::Model)` and keeps asserting `OutputTrust::Untrusted`, with the message changed to say model-round output is untrusted; existing tests through `invoke`, `list`, and `call` thereby keep pinning the model path. Add a helper that calls a tool as the script (`TestCall::new`, default caller `Script`) and returns the whole `ToolOutput`. + - `tests/it/calls.rs`: a script call of `call_dynamic_tool` to the fixture's `Echo` tool with `{ "q": "hello" }` returns `OutputTrust::Trusted` and the text `echo: {"q":"hello"}` exactly. A script call of `call_dynamic_tool` to the fixture's `fail` tool is a `ToolErrorKind::Backend` error with message `it failed`, same as the model path. + - `tests/it/listing.rs`: a script call of `get_dynamic_tools` with no arguments returns `OutputTrust::Trusted`, and its text parses as JSON equal to the model-path listing for the same Pack. + - Test names state the behavior. + - `src/result-tests.rs`: drop the trust assertions (trust is no longer `result.rs`'s concern), assert on returned strings, keep every text-rendering case. + - Run the new script-path tests focused (`cargo nextest run --locked -p plugin-mcp --all-features -E 'test()'`) before the source change to show them failing (the `Echo` and listing tests report `Untrusted`), then after to show them passing. +- Commit: code, docs, and tests together. + + + + diff --git a/vibe/ACTIVE b/vibe/ACTIVE new file mode 100644 index 000000000..66f0990ab --- /dev/null +++ b/vibe/ACTIVE @@ -0,0 +1 @@ +vibe/2026-10-21-1-mcp-script-raw-text.md From 0cf1ed25a34209d3ff5b74cd24fd6f09982cc047 Mon Sep 17 00:00:00 2001 From: Jason Brazeal Date: Wed, 21 Oct 2026 12:31:05 -0700 Subject: [PATCH 2/3] Mark MCP tool output trusted for the section's script Both MCP tools now decide output trust from who made the call. A call from the section's own script gets the server's text marked trusted, so the script receives it without the untrusted envelope, while a model round or any other caller still gets untrusted text. The rendering path below the dispatch now returns plain text, so trust is decided in one place. - `marked` maps `ToolCaller::Script` to `ToolOutput::trusted` and every other caller, through a `_` arm, to `ToolOutput::untrusted`; `Mcp::call` reads `cx.origin().caller` once and applies it to both tools. - `render`, `output`, `send`, `call`, `call_tool`, and the `AbortOnDrop` join handle now carry `Result` instead of `ToolOutput`, so the result module no longer sets trust. - `invoke` now calls as `ToolCaller::Model` and still asserts untrusted output, so the existing tests keep pinning the model path; `invoke_as_script` returns the whole output for the new script-path tests. - `a_script_calls_is_error_result_is_the_same_backend_error_as_a_models` pins that a script call's `isError` result stays the same `Backend` error with the message `it failed`. - `lib.rs` replaces the invariant that every output is untrusted with the per-caller rule and states the same rule beside the two tool descriptions. Design: new pure-function @ crates/plugin-mcp/src/dynamic.rs::marked deps: String,ToolCaller Design: new surface-growth @ crates/plugin-mcp/src/dynamic.rs::Mcp::call Design: new pure-function @ crates/plugin-mcp/tests/it/support.rs::tool_call deps: str Plan: vibe/2026-10-21-1-mcp-script-raw-text.md --- crates/plugin-mcp/src/dynamic.rs | 24 +++++++++++---- crates/plugin-mcp/src/lib.rs | 10 +++++-- crates/plugin-mcp/src/result-tests.rs | 13 ++++---- crates/plugin-mcp/src/result.rs | 14 ++++----- crates/plugin-mcp/src/server.rs | 10 +++---- crates/plugin-mcp/tests/it/calls.rs | 38 ++++++++++++++++++++++-- crates/plugin-mcp/tests/it/listing.rs | 18 +++++++++-- crates/plugin-mcp/tests/it/support.rs | 24 +++++++++++---- vibe/2026-10-21-1-mcp-script-raw-text.md | 2 +- 9 files changed, 114 insertions(+), 39 deletions(-) diff --git a/crates/plugin-mcp/src/dynamic.rs b/crates/plugin-mcp/src/dynamic.rs index 51d59525f..8a50191a0 100644 --- a/crates/plugin-mcp/src/dynamic.rs +++ b/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) -> 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 { + /// 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 { 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> { 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}"))) } diff --git a/crates/plugin-mcp/src/lib.rs b/crates/plugin-mcp/src/lib.rs index 8bf4ba4b6..46f18b47c 100644 --- a/crates/plugin-mcp/src/lib.rs +++ b/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 diff --git a/crates/plugin-mcp/src/result-tests.rs b/crates/plugin-mcp/src/result-tests.rs index 4972a758b..5787c31ea 100644 --- a/crates/plugin-mcp/src/result-tests.rs +++ b/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] diff --git a/crates/plugin-mcp/src/result.rs b/crates/plugin-mcp/src/result.rs index f1cc889a4..0eaae44c1 100644 --- a/crates/plugin-mcp/src/result.rs +++ b/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, -) -> Result { +/// Maps the answer to a `tools/call` request to its text. +pub(crate) fn output(response: Result) -> Result { 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 { +pub(crate) fn render(result: &CallToolResult) -> Result { 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 { }; 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 diff --git a/crates/plugin-mcp/src/server.rs b/crates/plugin-mcp/src/server.rs index f3b3a584b..9d3c0d9b2 100644 --- a/crates/plugin-mcp/src/server.rs +++ b/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>); +struct AbortOnDrop(JoinHandle>); 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, name: String, arguments: JsonObject, runtime: Handle, -) -> Result { +) -> Result { let request = ClientRequest::CallToolRequest(CallToolRequest::new( CallToolRequestParams::new(name).with_arguments(arguments), )); @@ -157,7 +157,7 @@ pub(crate) async fn call( peer: Peer, name: String, arguments: JsonObject, -) -> Result { +) -> Result { let mut task = AbortOnDrop(runtime.spawn(send(peer, name, arguments, runtime.clone()))); match (&mut task.0).await { Ok(outcome) => outcome, diff --git a/crates/plugin-mcp/tests/it/calls.rs b/crates/plugin-mcp/tests/it/calls.rs index 14e09bdd3..8e0c1a7bb 100644 --- a/crates/plugin-mcp/tests/it/calls.rs +++ b/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; diff --git a/crates/plugin-mcp/tests/it/listing.rs b/crates/plugin-mcp/tests/it/listing.rs index 3d04b01a5..a557a993a 100644 --- a/crates/plugin-mcp/tests/it/listing.rs +++ b/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; diff --git a/crates/plugin-mcp/tests/it/support.rs b/crates/plugin-mcp/tests/it/support.rs index 1ddca5b05..eab5c40ab 100644 --- a/crates/plugin-mcp/tests/it/support.rs +++ b/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/` 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/` with `args` from a model round and +/// returns its text, checking that the output is untrusted. pub(crate) async fn invoke( pack: &Arc, tool: &str, args: Value, ) -> Result { - 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/` with `args` from the section's own +/// script and returns the whole output. +pub(crate) async fn invoke_as_script( + pack: &Arc, + tool: &str, + args: Value, +) -> Result { + pack.call(tool_call(tool).context(), args).await +} + /// The JSON `get_dynamic_tools` answers for `args`. pub(crate) async fn list(pack: &Arc, args: Value) -> Value { let text = invoke(pack, "get_dynamic_tools", args) diff --git a/vibe/2026-10-21-1-mcp-script-raw-text.md b/vibe/2026-10-21-1-mcp-script-raw-text.md index 9324fb1b2..b08041ac0 100644 --- a/vibe/2026-10-21-1-mcp-script-raw-text.md +++ b/vibe/2026-10-21-1-mcp-script-raw-text.md @@ -131,7 +131,7 @@ isProject: false -### Step 2: plugin-mcp marks output by caller for both tools +### Step 2: plugin-mcp marks output by caller for both tools [completed] - Component: none - Tests: `cargo nextest run --locked -p plugin-mcp --all-features`; exit checks: `cargo fmt --all --check`; `CARGO_BUILD_WARNINGS=deny cargo clippy --workspace --exclude workshop --exclude workshop-server --exclude workshop-server-api --all-targets --all-features`; `CARGO_BUILD_WARNINGS=deny cargo clippy -p workshop -p workshop-server -p workshop-server-api --all-targets`; `cargo nextest run --locked --workspace --exclude workshop --exclude workshop-server --exclude workshop-server-api --all-features`; `cargo nextest run --locked -p workshop -p workshop-server -p workshop-server-api`; `RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps --all-features --exclude workshop --exclude workshop-server --exclude workshop-server-api`; `RUSTDOCFLAGS="-D warnings" cargo doc -p runvil --no-deps`; `cargo test -p build-xtask`; `node --test crates/workshop/ui/test/docs-claims.mjs` From a09e17de05a31672baaa5feb931f58c665943378 Mon Sep 17 00:00:00 2001 From: Jason Brazeal Date: Wed, 21 Oct 2026 12:38:47 -0700 Subject: [PATCH 3/3] Close plan: mcp-script-raw-text Plan: vibe/2026-10-21-1-mcp-script-raw-text.md --- vibe/2026-10-21-1-mcp-script-raw-text.md | 4 ++-- vibe/ACTIVE | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) delete mode 100644 vibe/ACTIVE diff --git a/vibe/2026-10-21-1-mcp-script-raw-text.md b/vibe/2026-10-21-1-mcp-script-raw-text.md index b08041ac0..6a0571aee 100644 --- a/vibe/2026-10-21-1-mcp-script-raw-text.md +++ b/vibe/2026-10-21-1-mcp-script-raw-text.md @@ -4,10 +4,10 @@ overview: "plugin-mcp marks its output by caller: a call the section's Lua makes todos: - id: s1-test-call-caller content: "S1: add TestCall::with_caller to runvil-plugin's test support, with a test that a call built with ToolCaller::Model lends a model origin" - status: pending + status: completed - id: s2-mcp-trust-by-caller content: "S2: plugin-mcp returns trusted output to ToolCaller::Script and untrusted output to every other caller, for both get_dynamic_tools and call_dynamic_tool; update the crate docs and tests" - status: pending + status: completed isProject: false --- diff --git a/vibe/ACTIVE b/vibe/ACTIVE deleted file mode 100644 index 66f0990ab..000000000 --- a/vibe/ACTIVE +++ /dev/null @@ -1 +0,0 @@ -vibe/2026-10-21-1-mcp-script-raw-text.md