From f4cca2dc22ee4ce05ad75e185a025f6c664829c8 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Mon, 12 Oct 2026 14:15:00 +0200 Subject: [PATCH] fix(coding-agent): stop listing MCP resources when connecting Servers with resources registered no tools until resources/list and resources/templates/list finished, so a slow listing (a large drive) kept their tools out of the first prompt. The lists only fed counts in /mcp and pi mcp list; both now show that a server offers resources. The resource tools still list and read resources on demand. fixes #10526 --- packages/coding-agent/CHANGELOG.md | 2 + .../coding-agent/src/extensions/mcp/cli.ts | 13 ++--- .../coding-agent/src/extensions/mcp/index.ts | 6 +-- .../src/extensions/mcp/resources.ts | 2 +- .../src/extensions/mcp/runtime.ts | 47 ++----------------- .../coding-agent/test/mcp-extension.test.ts | 22 ++++++++- 6 files changed, 34 insertions(+), 58 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 3621d3150..8117a9466 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -7,6 +7,7 @@ - Published generated configuration schemas for models, settings, keybindings, and themes at `https://pi.dev/schemas/*.schema.json`. The theme schema moved to `https://pi.dev/schemas/theme.schema.json`; themes loaded during startup now receive the same schema validation as later reloads. Update theme `$schema` references, define reusable colors under `vars`, and remove unsupported metadata. - Changed `before_agent_start` to also fire for runs started by `pi.sendMessage(..., { triggerTurn: true })` while idle. `BeforeAgentStartEvent` is now a union on `messageType`: `"user"` events carry `prompt` and `images`, and `"custom"` events carry the starting `message` and no `prompt`. Messages returned by handlers are added to both kinds of run, and pending `deliverAs: "nextTurn"` messages are delivered with the custom-message run. Handlers that should act only on user prompts must check `event.messageType === "user"` ([#10267](https://github.com/earendil-works/pi/issues/10267), [#5581](https://github.com/earendil-works/pi/issues/5581)) - Changed the session to count as busy while a run starts, including during `before_agent_start` handlers: `ctx.isIdle()` is false, `pi.sendMessage()` queues into the starting run instead of starting a second run that failed with "Agent is already processing", `pi.sendUserMessage()` requires `deliverAs`, and an abort requested meanwhile aborts the run once it starts +- Changed `pi mcp list --json` to report `resources: true` for servers that offer resources instead of `resources` and `resourceTemplates` counts; Pi no longer lists resources when connecting ([#10526](https://github.com/earendil-works/pi/issues/10526)) ### Added @@ -30,6 +31,7 @@ - Fixed codemode blocking the process for seconds to minutes after scripts with many `text()` calls; joining the output items was quadratic ([#10678](https://github.com/earendil-works/pi/issues/10678)) - Fixed `pi mcp login --help` and the CLI docs describing `--timeout` as only the wait for the browser; it limits the whole sign-in ([#10636](https://github.com/earendil-works/pi/issues/10636)) - Fixed `~` not being expanded in `!pattern`, `+path`, and `-path` entries of the `extensions`, `skills`, `prompts`, and `themes` settings, so entries like `!~/skills/pdf` matched nothing ([#10765](https://github.com/earendil-works/pi/issues/10765)) +- Fixed MCP servers with resources registering no tools until every resource was listed; Pi no longer lists resources when connecting, so `/mcp` and `pi mcp list` show that a server offers resources instead of counting them ([#10526](https://github.com/earendil-works/pi/issues/10526)) ### Removed diff --git a/packages/coding-agent/src/extensions/mcp/cli.ts b/packages/coding-agent/src/extensions/mcp/cli.ts index 4d9dd44b7..75cdf1f08 100644 --- a/packages/coding-agent/src/extensions/mcp/cli.ts +++ b/packages/coding-agent/src/extensions/mcp/cli.ts @@ -101,8 +101,8 @@ interface ServerReport { tools: string[]; /** Tools whose exposure differs from the server's, from `toolExposure`. */ toolExposure?: Record; - resources?: number; - resourceTemplates?: number; + /** Whether the server offers resources. They are not listed: listing every one can take long. */ + resources?: true; error?: string; } @@ -466,10 +466,7 @@ async function list( return exposure === report.exposure ? [] : [[tool.name, exposure] as const]; }); if (overrides.length > 0) report.toolExposure = Object.fromEntries(overrides); - if (connection.hasResources) { - report.resources = connection.resources.length; - report.resourceTemplates = connection.resourceTemplates.length; - } + if (connection.hasResources) report.resources = true; if (connection.state !== "connected" && connection.error) report.error = connection.error; await connection.close(); return report; @@ -508,9 +505,7 @@ async function list( }); log(` tools: ${tools.join(", ")}`); } - if (report.resources !== undefined) { - log(` resources: ${report.resources}, URI templates: ${report.resourceTemplates ?? 0}`); - } + if (report.resources) log(" offers resources"); if (report.error) log(` ${report.error.split("\n").join("\n ")}`); } for (const configError of loaded.errors) log(`config error: ${configError}`); diff --git a/packages/coding-agent/src/extensions/mcp/index.ts b/packages/coding-agent/src/extensions/mcp/index.ts index 048ec89b2..8a97d7fb2 100644 --- a/packages/coding-agent/src/extensions/mcp/index.ts +++ b/packages/coding-agent/src/extensions/mcp/index.ts @@ -241,10 +241,8 @@ function describeState(server: McpServer, withError = true): string { case "failed": return withError ? `failed: ${firstLine(connection.error ?? "unknown error")}` : "failed"; case "connected": { - const { tools, resources } = connection; - const count = resources.length; - const resourceCount = count > 0 ? ` · ${count} resource${count === 1 ? "" : "s"}` : ""; - return `connected · ${tools.length} tool${tools.length === 1 ? "" : "s"}${resourceCount}`; + const { tools, hasResources } = connection; + return `connected · ${tools.length} tool${tools.length === 1 ? "" : "s"}${hasResources ? " · resources" : ""}`; } case "connecting": return "connecting…"; diff --git a/packages/coding-agent/src/extensions/mcp/resources.ts b/packages/coding-agent/src/extensions/mcp/resources.ts index e0e21efeb..c93ae4c80 100644 --- a/packages/coding-agent/src/extensions/mcp/resources.ts +++ b/packages/coding-agent/src/extensions/mcp/resources.ts @@ -44,7 +44,7 @@ export interface McpResourceServer { } /** MCP App user interfaces, which only hosts that render them can use. */ -export function isMcpAppResource(item: { uri?: string; uriTemplate?: string; mimeType?: string }): boolean { +function isMcpAppResource(item: { uri?: string; uriTemplate?: string; mimeType?: string }): boolean { const uri = item.uri ?? item.uriTemplate ?? ""; return uri.startsWith("ui://") || /;\s*profile\s*=\s*"?mcp-app"?/i.test(item.mimeType ?? ""); } diff --git a/packages/coding-agent/src/extensions/mcp/runtime.ts b/packages/coding-agent/src/extensions/mcp/runtime.ts index 0c42f7983..f13d21b12 100644 --- a/packages/coding-agent/src/extensions/mcp/runtime.ts +++ b/packages/coding-agent/src/extensions/mcp/runtime.ts @@ -39,7 +39,7 @@ import { type McpOAuthCredentialStore, type McpOAuthSettings, } from "./oauth.ts"; -import { isMcpAppResource, type McpResourceServer } from "./resources.ts"; +import type { McpResourceServer } from "./resources.ts"; import type { McpToolCaller } from "./tools.ts"; export { McpServerLog } from "./log.ts"; @@ -135,23 +135,6 @@ function listTemplates(client: McpClient, options: McpRequestOptions = {}): Prom return withoutTemplates(() => client.listResourceTemplates(options), []); } -/** - * Resources and templates at connect time, for the counts in `/mcp` and `pi mcp list`. A server - * whose lists fail still connects: the resource tools list and read its resources on demand. - */ -async function fetchResources( - client: McpClient, -): Promise<{ resources: Resource[]; resourceTemplates: ResourceTemplate[] }> { - const [resources, resourceTemplates] = await Promise.all([ - client.listResources().catch(() => []), - listTemplates(client).catch(() => []), - ]); - return { - resources: resources.filter((resource) => !isMcpAppResource(resource)), - resourceTemplates: resourceTemplates.filter((template) => !isMcpAppResource(template)), - }; -} - /** One configured server. Reconnects lazily when a call finds the connection gone. */ export class McpServerConnection implements McpToolCaller, McpResourceServer { readonly entry: McpServerEntry; @@ -159,12 +142,10 @@ export class McpServerConnection implements McpToolCaller, McpResourceServer { error: string | undefined; tools: McpTool[] = []; /** - * Whether the server offers resources. The lists below are what it listed at the last connect or - * change, without MCP App resources. + * Whether the server offers resources, which the resource tools list and read on demand. They are + * not listed at connect: listing every resource can take long (a large drive, say). */ hasResources = false; - resources: Resource[] = []; - resourceTemplates: ResourceTemplate[] = []; /** Server instructions from `initialize`, describing its tools as a group. */ instructions: string | undefined; /** Last OAuth challenge from the server; sign-in uses its resource metadata URL and scope. */ @@ -399,24 +380,15 @@ export class McpServerConnection implements McpToolCaller, McpResourceServer { client.onNotification("notifications/tools/list_changed", () => { void this.refreshTools(client); }); - client.onNotification("notifications/resources/list_changed", () => { - void this.refreshResources(client); - }); const stdio = transport instanceof StdioTransport ? transport : undefined; client.onClose(() => this.handleClientClose(client, stdio)); // Servers without the tools capability (prompts or resources only) do not answer tools/list. - const hasResources = client.serverCapabilities?.resources !== undefined; - const [tools, resources] = await Promise.all([ - client.serverCapabilities?.tools ? client.listTools() : [], - hasResources ? fetchResources(client) : { resources: [], resourceTemplates: [] }, - ]); + const tools = client.serverCapabilities?.tools ? await client.listTools() : []; if (this.closed) throw new Error("shut down while connecting"); if (client.connectionState !== "connected") throw new Error("connection closed during setup"); this.client = client; this.tools = tools; - this.hasResources = hasResources; - this.resources = resources.resources; - this.resourceTemplates = resources.resourceTemplates; + this.hasResources = client.serverCapabilities?.resources !== undefined; this.instructions = client.instructions?.trim() || undefined; this.state = "connected"; this.error = undefined; @@ -467,15 +439,6 @@ export class McpServerConnection implements McpToolCaller, McpResourceServer { this.changed(); } - private async refreshResources(client: McpClient): Promise { - const { resources, resourceTemplates } = await fetchResources(client); - if (this.client !== client || this.closed) return; - this.resources = resources; - this.resourceTemplates = resourceTemplates; - this.onTools(this); - this.changed(); - } - /** Resolves once no transport of this server is open, including one that was still connecting. */ async close(): Promise { this.shutdown.abort(); diff --git a/packages/coding-agent/test/mcp-extension.test.ts b/packages/coding-agent/test/mcp-extension.test.ts index d0370aaba..1a99e723e 100644 --- a/packages/coding-agent/test/mcp-extension.test.ts +++ b/packages/coding-agent/test/mcp-extension.test.ts @@ -435,7 +435,9 @@ describe("MCP connections", () => { const servers: InMemoryTransport[] = []; /** In-memory server that answers initialize, tools/list, and tools/call with "ok". */ - function createTransport(options: { expireFirstCall?: boolean; methods?: string[]; noTools?: boolean } = {}) { + function createTransport( + options: { expireFirstCall?: boolean; methods?: string[]; noTools?: boolean; resources?: boolean } = {}, + ) { const pair = createInMemoryTransportPair(); servers.push(pair.server); pair.server.onMessage((message) => { @@ -448,7 +450,10 @@ describe("MCP connections", () => { id: message.id, result: { protocolVersion: LATEST_PROTOCOL_VERSION, - capabilities: options.noTools ? { prompts: {} } : { tools: {} }, + capabilities: { + ...(options.noTools ? { prompts: {} } : { tools: {} }), + ...(options.resources ? { resources: {} } : {}), + }, serverInfo: { name: "fake", version: "1.0.0" }, }, } @@ -774,6 +779,19 @@ for await (const line of createInterface({ input: process.stdin })) { expect(() => connection.oauthSettings()).toThrow("oauth.clientSecret"); await connection.close(); }); + + // #10526: listing every resource can take long (a large drive), so connecting must not wait for it. + it("connects to servers with resources without listing them", async () => { + const methods: string[] = []; + const { connection } = connect({ name: "fake", config: { command: "unused" }, source: "test" }, [ + () => createTransport({ resources: true, methods }), + ]); + await connection.getClient(); + expect(connection.state).toBe("connected"); + expect(connection.hasResources).toBe(true); + expect(methods).toEqual(["initialize", "tools/list"]); + await connection.close(); + }); }); describe("MCP servers section", () => {