Outbound MCP client should support resources and prompts, not only tools #263

Activity

1. serge-ivo commented on Aug 6, 2026

   

   serge-ivo

   on Aug 6, 2026

   ContributorAuthor

   More actions

   Not reached this run (capacity went to #286). Leaving open — but this is the smallest of the remaining four and the plumbing is already in place.

   It is mostly wiring. mcpCall(ctx, input, method, params, log) is method-agnostic: era detection, version negotiation, safeFetch, the redacted agent_events row, and (since #286) per-endpoint credential resolution all happen regardless of the JSON-RPC method. resources/list, resources/read, prompts/list, prompts/get need four new ToolDefs and no new transport. mcpNameFor already handles both new name sources — params.name for prompts/get, params.uri for resources/read — so the modern-era Mcp-Name header is already correct for them.

   Three things that are not wiring:

   1. Scope. resources/* and prompts/* are reads, so by the connector's own rule they take scope:"read" and need no write consent. But Outbound MCP consent should be per server and per remote tool, not one global write grant #262 consent is keyed on a tool name, and a resource is a URI, not a tool — so either resource reads are ungated like tools/list (defensible: read-scoped, and it is what makes per-tool consent approachable in the first place), or the consent key has to grow a kind. Pick deliberately; do not let it fall out of an implementation detail.
   2. resources/read returns content into the model's context. That is untrusted remote text on an instruction path. It must go through the same data-not-instructions fencing agent-think.ts applies to RAG, or a resource read becomes prompt injection with a nicer name.
   3. Size limits, which the ticket asks for. extractToolResult flattens text parts with no cap today — fine for a tool result, not for a resource that may be a whole file. Cap and truncate visibly; a silent truncation produces a model confidently reasoning about the half it got.

   Tests should cover JSON and SSE framing for the new methods (parseRpcBody handles both, but nothing currently exercises them on a non-tools/* method), plus -32601 from a server that publishes no resources — that must read as "this server has none", not as a transport failure.
2. serge-ivo

   added a commit that references this issue on Aug 6, 2026

   feat(mcp): read the server's resources and prompts — fenced, capped, …

   ...

   4062a07
3. serge-ivo commented on Aug 6, 2026

   

   serge-ivo

   on Aug 6, 2026

   ContributorAuthor

   More actions

   Shipped in 4062a07 (main, CI green). Leaving open for one criterion only — the Settings UI. Everything else is done.

   Four tools: mcp_list_resources, mcp_read_resource, mcp_list_prompts, mcp_get_prompt. The previous run was right that the transport is pure wiring — mcpCall is method-agnostic, and mcpNameFor already sourced the modern Mcp-Name header correctly for both new name sources. The three things that were not wiring were each decided rather than defaulted:

   1. Consent — read-scoped and ungated, chosen deliberately. They match tools/list, which is the property that makes per-tool consent approachable at all: you cannot approve tools you cannot enumerate. Gating them was rejected for the reason you flagged — #262's key names a tool and a resource is a URI, so reusing that column would make one row mean two things and leave * permanently ambiguous ("all tools" — does that include every resource?). If read consent is wanted later, the honest shape is a kind column on instance_mcp_consent, not a URI stuffed into the tool column. What bounds them today: the endpoint's own credential (#286 — a server the account never connected returns nothing), the agent's declared capabilities.tools, and the owner's per-tool off-switch. A test asserts all four are scope:"read" so a later tidy-up cannot flip them to write and make enumeration impossible before consent.

   2. Fencing — and it moved. You were right that this is the whole reason #263 is riskier than it looks. The RAG fence was an inline string literal in agent-think.ts; it is now lib/untrusted-fence.ts, and resources/read / prompts/get wrap their payload with it. Two properties worth naming:

   • Fenced in the connector, not at the chat surface. These handlers also answer a pipeline step, POST /v1/instances/:id/tools/:name, and MCP. Fencing in agent-think.ts would have covered exactly one of four surfaces.
   • The fence neutralizes its own closing marker inside the body. Without that it is decorative: a resource containing </untrusted_reference_material> closes the block early and everything after it reads as trusted system text. Replaced with a visible placeholder rather than stripped, so the text is neutralized rather than silently altered. There is a test for exactly that payload.

   A prompt template is the sharper case — it is literally instruction-shaped text authored by the remote server — so prompts/get says in its own description that the result is the server's suggested wording and not an instruction.

   3. Size. RESOURCE_MAX_CHARS = 20_000, truncated with both numbers ("showing the first N of M characters"): a silent cut produces a model reasoning confidently about the half it got. Binary parts are described by type and size, not inlined — base64 in a prompt is pure spend with nothing the model can use. nextCursor is carried through both list methods, so continuation is actually reachable.

   -32601 reads as "this server has none", not as a transport failure — reported as a failure, a model retries and then blames the server. A 401 stays an auth failure, tested alongside, so "no resources" and "you may not see them" never collapse into one answer. -32002 on a read is named as a missing resource. Tests cover JSON and SSE framing on the new methods, plus the legacy handshake fallback (nothing had exercised a non-tools/* method on either before).

   Bonus fix found on the way: a parsed JSON-RPC error was evicting the cached protocol era. A -32601 — an entirely normal answer from a server with no resources — therefore bought a repeat modern probe on every subsequent call for no information. Any parsed JSON-RPC response proves the era (a server of the other era answers a wrongly-shaped POST with an HTML error page), so rpc_error and input_required are now exempt.

   Remaining: "Settings UI shows resources/prompts availability for the connection." Not done — a concurrent session held store/console/src/components/McpConnections.tsx this run. The API side needs one addition when someone picks it up: POST /v1/instances/:id/mcp/test currently probes tools/list only, so the report has nothing to say about resources or prompts. The cheap version is two extra read-scoped calls on that route (both already ungated, both already -32601-tolerant) surfacing resourceCount / promptCount on McpConnectionReport.
4. serge-ivo

   added 3 commits that reference this issue on Aug 6, 2026

   test(security): make the invariants fail the build when the next call…

   ...

   ff372a1

   fix(security): gate the mutating http call, fence the remote text it …

   ...

   54b2228

   feat(mcp): say what else a server publishes, and whether the agent ca…

   ...

   ae6ef00
5. serge-ivo commented on Aug 6, 2026

   

   serge-ivo

   on Aug 6, 2026

   ContributorAuthor

   More actions

   Last criterion done. Closing. ae6ef00 on main.

   "Settings UI shows resources/prompts availability for the connection" — with the API-side prerequisite the previous run flagged: /mcp/test probed tools/list only, so the report had nothing to say about either surface.

   The probe now asks all three. probeMcpSurface runs resources/list and prompts/list through the same mcpCall as everything else — therefore safeFetch, https-only and SSRF-guarded, therefore the era cache and the redacted agent_events row. The endpoint is user-supplied config, which is exactly why a "test this" button gets no fast path of its own: a probe is as auditable as a real call rather than a side channel around the log. A test asserts the trace row.

   Three states, and merging any two of them would be a lie.

   • unsupported — the server answered -32601. It publishes none. That is a correct answer, not a fault; reported as a failure it sends an owner to debug a connection that works perfectly.
   • unreadable — we could not ask (rejected credential, transport failure), keeping the transport's own sentence. Folding this into unsupported would claim zero resources for a server that may hold thousands, undetectably. Both are tested against each other.
   • available — a count. Distinct from unsupported even at zero: "it offers none right now" and "it does not do resources" are different facts, and an owner who conflates them goes hunting for a permission that was never involved.

   Absent entirely when the connection itself failed. An unreachable server taught us nothing about what it publishes, and a confident "no resources" for a host that was briefly down is worse than silence — the same reasoning that already stops a failed probe from emptying the imported tool catalog.

   #266's rule carried onto the new surface, because it is the same lie in a new place. The probe enumerates on the owner's authority, so a bare "3 resources" is true about the server and false about what the agent will manage to do. Each surface reports reach beside availability and names the gate to fix first, so nobody is told to enable mcp_read_resource while mcp_list_resources still hides the catalog. The console line takes its tone from reach, not availability: 40 resources this agent cannot list is amber, and a server that simply has none is neutral.

   Reported apart from gates — the honest shape, not the convenient one. gates describes what blocks mcp_call_tool. These surfaces are read-scoped, so the connector write kill switch and the per-(endpoint, tool) grant do not apply to them at all. Printing a write switch beside a resource count would send an owner to flip something that changes nothing here, so the panel says outright that reads need no per-item approval instead of leaving them looking for one. A route test pins it: no write consent, no grants, resources still fully reachable.

   Pagination is surfaced rather than hidden — a paged count renders 200+, because a flat "200" for a server holding thousands reads as complete.

   Verification. Full workspace typecheck, 4176 unit tests (16 new), docs:drift, openapi:coverage, the file-size ratchet, and a console build — all green locally. No CI run has verified this: Actions fired nothing for the two commits preceding mine, and none for this one. store/openapi.yaml carries a new McpSurfaceReport schema. Ratchet pins raised on purpose in the same commit (mcp.ts 1191→1236, tools.ts 811→833) — the network belongs with the transport, and the pure reasoning it feeds already lives in mcp-connection.ts, which is where a split would have put it.
6. serge-ivo

   closed this as completedon Aug 6, 2026
7. serge-ivo

   added 3 commits that reference this issue on Aug 7, 2026

   fix(api,runner,console): a terminal-driven CLI is unmetered, not free (…

   ...

   a9f4cd7

   fix(console): an unconfigured connector is a deployment gap, not the …

   ...

   8ab3481

   feat(tools): a tool the platform provides is reachable by someone, or…

   ...

   a21accf
8. serge-ivo

   added 2 commits that reference this issue on Aug 23, 2026

   fix(security): the apply and browse loops read the page as fenced dat…

   ...

   9f8c154

   fix(security): the knowledge tools return the corpus fenced, the same…

   ...

   4264497