mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
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
This commit is contained in:
parent
6a51a39d42
commit
f4cca2dc22
6 changed files with 34 additions and 58 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -101,8 +101,8 @@ interface ServerReport {
|
|||
tools: string[];
|
||||
/** Tools whose exposure differs from the server's, from `toolExposure`. */
|
||||
toolExposure?: Record<string, string>;
|
||||
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}`);
|
||||
|
|
|
|||
|
|
@ -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…";
|
||||
|
|
|
|||
|
|
@ -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 ?? "");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<void> {
|
||||
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<void> {
|
||||
this.shutdown.abort();
|
||||
|
|
|
|||
|
|
@ -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", () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue