MCP agent instances should import remote MCP tools as first-class callable tools #261

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, the security one). Leaving open with what the code now looks like, so the next run does not re-derive it.

   What #286 changed for this ticket. Importing remote tools as first-class tools means each synthetic tool dispatches with a credential — and until today that credential was one account-wide bearer, so an imported tool from server A carried a token that also worked at server B. Importing tools on top of that would have multiplied the blast radius of the bug. resolveMcpCredential(env, userId, normalizedEndpoint) is now the single resolution point, so a synthetic tool needs only to carry its endpoint and the right credential follows.

   Two things to keep when building it:

   1. The synthetic tool name is not the consent key. Consent is (instance, endpoint, remote tool name) in instance_mcp_consent, and isDestructiveToolName runs on the name we put on the wire. If an imported tool is exposed as mcp:<id>:<tool> and the gate is checked on that synthetic string, the destructive-name test silently stops matching (mcp:7:delete_site still splits to include delete, but a hash-based id would not, and any renaming scheme breaks the grant lookup outright). Dispatch must resolve back to (endpoint, remoteToolName) and check those.
   2. Caching the catalog is caching remote, attacker-shaped data. parseToolCatalog already bounds it (500 entries, 300-char descriptions, non-empty string names). Anything persisted must keep those bounds, and the cached description ends up in a model prompt — so a refresh must be able to shrink the tool set, or a server that removes a tool leaves a callable ghost.

   Collision handling asked for by the ticket falls out of (1): the endpoint is part of the identity, so two servers publishing create_site are already distinct rows in consent.
2. serge-ivo

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

   feat(mcp): a granted remote tool is a real tool, not a string the mod…

   ...

   3ed7a3f
3. serge-ivo commented on Aug 6, 2026

   

   serge-ivo

   on Aug 6, 2026

   ContributorAuthor

   More actions

   Shipped in 3ed7a3f (main, CI green).

   The shape. A tool a server publishes AND the owner has granted is projected into an ordinary function tool carrying the server's own name, description and input schema. mcp_call_tool stays as the escape hatch, exactly as this ticket asked.

   Both warnings from the previous run are honoured, and they are what the tests are about.

   1. The synthetic name is not the consent key. The label (mcp_<endpoint-slug>_<tool>) is derived at projection time, never stored, and never consulted by a gate. Dispatch resolves it back to {endpoint, remote tool} and runs the ordinary mcp_call_tool path — same Outbound MCP consent should be per server and per remote tool, not one global write grant #262 consent on the remote name, same isDestructiveToolName on the name we put on the wire, same per-endpoint credential (MCP bearer/OAuth credentials should be scoped by endpoint, not one global mcp token #286), same redacted trace row. mcp-tool-catalog.test.ts asserts the pair it carries is exactly what grantsAllowTool accepts, and that a wildcard grant imports create_site but not delete_site. An imported tool is a nicer way to type a call, never a second way to authorize one.
   2. Caching the catalog is caching attacker-shaped data. Bounded on the way in — 500 tools, 300-char descriptions, object-only schemas capped at 8KB, with an oversized schema stored as absent rather than truncated (half a JSON Schema is not a schema). A refresh replaces the endpoint's rows wholesale, so a removed tool stops being offered instead of leaving a callable ghost — and only on a successful probe, since emptying the catalog because a server was briefly down would strip an agent of tools it is still granted. Capped at 32 projected tools with deterministic ordering, so which ones survive a wildcard grant on a large server is stable rather than a function of row order.

   Collision handling falls out of (1) as predicted: the endpoint is part of the label, so two servers publishing create_site are already distinct. Two remote names that sanitize to the same label get a deterministic suffix rather than one of them silently vanishing while still granted.

   One thing worth flagging that was not in the ticket: the remote description ends up in a tool definition the model reads on every turn, and unlike a resource body a tool definition has nowhere to put a fence. So it is quoted as the server's claim about itself, not an instruction to you, and the tool says plainly that it runs outside ProAgentStore.

   Acceptance criteria. Visible catalog in settings ✓ (the #266 connection panel already lists them with grant/callable state) · enable/disable per remote tool ✓ (the existing per-tool grants ARE the switch — no second, divergent toggle was added) · structured function tools in chat ✓ · cached with a refresh action and invalidation ✓ · collisions safe ✓ · mcp_call_tool retained ✓.

   Two operational notes, deliberate rather than missed:

   • The refresh action is the connection test — the one path that already asks a server what it publishes. So a grant made before this landed imports nothing until its server is tested once.
   • The connection report does not yet say "and these N are imported as typed tools". Left out on purpose: computing an importedAs per row outside the projection would re-derive the dedupe and could disagree with the real label, which is precisely the class of drift this design avoids. If the panel wants it, it should read the projection, not recompute it. A concurrent session held McpConnections.tsx this run in any case.

   Migration 0086_mcp_tool_catalog.sql; logic in lib/mcp-tool-catalog.ts (pure projection + fail-soft store), wired in agent-think.ts and refreshed from POST /v1/instances/:id/mcp/test.
4. serge-ivo

   closed this as completedon Aug 6, 2026