diff --git a/src/main/agent/knowledge-mcp-gateway.test.ts b/src/main/agent/knowledge-mcp-gateway.test.ts index 18f50ba9..652f46bc 100644 --- a/src/main/agent/knowledge-mcp-gateway.test.ts +++ b/src/main/agent/knowledge-mcp-gateway.test.ts @@ -1079,7 +1079,7 @@ describe('KnowledgeMcpGateway', () => { expect(listed.tools).toEqual([ expect.objectContaining({ name: expect.stringMatching( - /^mcp_[a-f0-9]{8}_[a-f0-9]{8}_echo_private$/u + /^[a-f0-9]{4}_echo_private$/u ), description: expect.stringContaining('Private MCP') }) diff --git a/src/main/agent/knowledge-mcp-gateway.ts b/src/main/agent/knowledge-mcp-gateway.ts index 12465760..418af3a2 100644 --- a/src/main/agent/knowledge-mcp-gateway.ts +++ b/src/main/agent/knowledge-mcp-gateway.ts @@ -77,7 +77,7 @@ import type { ResolvedMcpServer } from '../capabilities/capability-service' import { - createMcpToolName, + createGatewayMcpToolName, isValidMcpToolName, listAllMcpTools, normalizeMcpToolSchema @@ -848,7 +848,7 @@ export class KnowledgeMcpGateway { >(normalizeMcpToolSchema(tool.outputSchema)) : undefined, exposedTool: { - name: createMcpToolName(server.id, tool.name), + name: createGatewayMcpToolName(server.id, tool.name), title: `${server.name} / ${tool.name}`.slice(0, 200), description: [ `GoodBuddy 代理的自定义 MCP Server「${server.name}」工具。`, diff --git a/src/main/agent/mcp-tool-utils.test.ts b/src/main/agent/mcp-tool-utils.test.ts new file mode 100644 index 00000000..424e1d36 --- /dev/null +++ b/src/main/agent/mcp-tool-utils.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from 'vitest' +import { + createGatewayMcpToolName, + MAXIMUM_GATEWAY_MCP_TOOL_NAME_LENGTH +} from './mcp-tool-utils' + +describe('createGatewayMcpToolName', () => { + it('keeps provider-safe original names readable behind a short server hash', () => { + expect( + createGatewayMcpToolName('server-1', 'crmtools_search_tools') + ).toMatch(/^[a-f0-9]{4}_crmtools_search_tools$/u) + }) + + it('separates identical tool names from different servers', () => { + expect(createGatewayMcpToolName('server-1', 'search')).not.toBe( + createGatewayMcpToolName('server-2', 'search') + ) + }) + + it('sanitizes unsafe names and keeps them unique with a tool hash', () => { + const dotted = createGatewayMcpToolName('server-1', 'files.read') + const spaced = createGatewayMcpToolName('server-1', 'files read') + expect(dotted).toMatch(/^[a-f0-9]{4}_files_read_[a-f0-9]{4}$/u) + expect(spaced).toMatch(/^[a-f0-9]{4}_files_read_[a-f0-9]{4}$/u) + expect(dotted).not.toBe(spaced) + expect(createGatewayMcpToolName('server-1', '工具')).toMatch( + /^[a-f0-9]{4}_tool_[a-f0-9]{4}$/u + ) + }) + + it('keeps the runtime-prefixed name within 64 characters for long tool names', () => { + const longA = `${'a'.repeat(120)}_one` + const longB = `${'a'.repeat(120)}_two` + const nameA = createGatewayMcpToolName('server-1', longA) + const nameB = createGatewayMcpToolName('server-1', longB) + expect(nameA.length).toBeLessThanOrEqual( + MAXIMUM_GATEWAY_MCP_TOOL_NAME_LENGTH + ) + expect(nameA).not.toBe(nameB) + expect(`gbc-a1b2c3-9_${nameA}`.length).toBeLessThanOrEqual(64) + expect(nameA).toMatch(/^[a-zA-Z0-9_-]+$/u) + }) +}) diff --git a/src/main/agent/mcp-tool-utils.ts b/src/main/agent/mcp-tool-utils.ts index 4f27ea4e..7eb42504 100644 --- a/src/main/agent/mcp-tool-utils.ts +++ b/src/main/agent/mcp-tool-utils.ts @@ -105,6 +105,48 @@ export function createMcpToolName( return `mcp_${serverHash}_${toolHash}_${readable}`.slice(0, 64) } +// Budget for tools re-exposed through a GoodBuddy gateway MCP. Runtimes such as +// OpenCode prepend "_" (up to 13 characters, e.g. "gbc-a1b2c3-9_") +// and common provider APIs reject tool names longer than 64 characters. +export const MAXIMUM_GATEWAY_MCP_TOOL_NAME_LENGTH = 51 +const GATEWAY_SERVER_HASH_LENGTH = 4 +const GATEWAY_TOOL_HASH_LENGTH = 4 + +/** + * Compact name for a custom MCP tool exposed through the GoodBuddy gateway: + * "_". The original name is kept verbatim when it + * is already provider-safe and fits; otherwise the readable part is sanitized + * and truncated, and a short hash of the original name keeps it unique. + */ +export function createGatewayMcpToolName( + serverId: string, + originalName: string +): string { + const serverHash = createHash('sha256') + .update(serverId) + .digest('hex') + .slice(0, GATEWAY_SERVER_HASH_LENGTH) + const prefix = `${serverHash}_` + const available = MAXIMUM_GATEWAY_MCP_TOOL_NAME_LENGTH - prefix.length + if ( + /^[a-zA-Z0-9_-]+$/u.test(originalName) && + originalName.length <= available + ) { + return `${prefix}${originalName}` + } + const toolHash = createHash('sha256') + .update(originalName) + .digest('hex') + .slice(0, GATEWAY_TOOL_HASH_LENGTH) + const readable = + originalName + .replace(/[^a-zA-Z0-9_-]+/gu, '_') + .replace(/^_+|_+$/gu, '') + .slice(0, available - GATEWAY_TOOL_HASH_LENGTH - 1) + .replace(/_+$/u, '') || 'tool' + return `${prefix}${readable}_${toolHash}` +} + export function normalizeMcpToolSchema( value: unknown ): Record & { type: 'object' } { diff --git a/src/main/agent/opencode-runtime.test.ts b/src/main/agent/opencode-runtime.test.ts index e0881d34..2752788a 100644 --- a/src/main/agent/opencode-runtime.test.ts +++ b/src/main/agent/opencode-runtime.test.ts @@ -17,6 +17,7 @@ import { describe, expect, it, vi } from "vitest"; import type { KnowledgeMcpGateway } from "./knowledge-mcp-gateway"; import type { RuntimeEvent } from "./runtime"; import { + createTemporaryMcpName, OpenCodeRuntime, type OpenCodeRuntimeDependencies, } from "./opencode-runtime"; @@ -2265,7 +2266,7 @@ describe("OpenCodeRuntime embedded permission mediation", () => { expect(setup.client.mcp.add).toHaveBeenCalledWith( { directory: process.cwd(), - name: expect.stringMatching(/^goodbuddy-custom-[a-f0-9]{20}$/u), + name: expect.stringMatching(/^gbc-[a-f0-9]{6}$/u), config: { type: "remote", url: "http://127.0.0.1:4567/mcp", @@ -3164,7 +3165,7 @@ describe("OpenCodeRuntime embedded permission mediation", () => { expect(setup.client.mcp.add).toHaveBeenCalledWith( { directory: process.cwd(), - name: expect.stringMatching(/^goodbuddy-data-[a-f0-9]{20}$/u), + name: expect.stringMatching(/^gbd-[a-f0-9]{6}$/u), config: { type: "remote", url: "http://127.0.0.1:4567/mcp", @@ -3201,8 +3202,8 @@ describe("OpenCodeRuntime embedded permission mediation", () => { read: false, write: false, bash: false, - "goodbuddy-data-*": false, - "goodbuddy-custom-*": false, + "gbd-*": false, + "gbc-*": false, [knowledgeToolId]: true, }, }), @@ -3210,7 +3211,7 @@ describe("OpenCodeRuntime embedded permission mediation", () => { ); expect(setup.client.mcp.disconnect).toHaveBeenCalledWith( { - name: expect.stringMatching(/^goodbuddy-data-/u), + name: expect.stringMatching(/^gbd-/u), directory: process.cwd(), }, { signal: expect.any(AbortSignal) }, @@ -3219,6 +3220,123 @@ describe("OpenCodeRuntime embedded permission mediation", () => { await runtime.dispose(); }); + it("keeps temporary MCP names stable within a conversation and short enough for providers", async () => { + const setup = runClient([ + { id: "idle", type: "session.idle", properties: { sessionID: "session-1" } }, + ]); + const runtime = embeddedRuntime(setup.client, { + knowledgeGateway: { + getEndpoint: () => "http://127.0.0.1:4567/mcp", + getAvailableToolNames: () => ["goodbuddy_config_capabilities"], + } as unknown as KnowledgeMcpGateway, + }); + const runOnce = async (requestId: string, conversationId: string) => { + for await (const _event of runtime.run( + { + requestId, + conversationId, + prompt: "search", + workMode: "ask", + knowledgeCapabilityToken: requestId, + }, + new AbortController().signal, + )) { + void _event; + } + }; + try { + await runOnce("request-1", "conversation-1"); + await runOnce("request-2", "conversation-1"); + await runOnce("request-3", "conversation-2"); + const add = setup.client.mcp.add as unknown as ReturnType; + const names = add.mock.calls.map( + (call) => (call[0] as { name: string }).name, + ); + expect(names[0]).toMatch(/^gbd-[a-f0-9]{6}$/u); + expect(names[1]).toBe(names[0]); + expect(names[2]).toMatch(/^gbd-[a-f0-9]{6}$/u); + expect(names[2]).not.toBe(names[0]); + expect(`${names[0]}_goodbuddy_config_capabilities`.length).toBeLessThanOrEqual(64); + } finally { + await runtime.dispose(); + } + }); + + it("does not reuse a temporary MCP name whose disconnect failed", async () => { + const setup = runClient([ + { id: "idle", type: "session.idle", properties: { sessionID: "session-1" } }, + ]); + const disconnect = setup.client.mcp.disconnect as unknown as ReturnType< + typeof vi.fn + >; + disconnect + .mockRejectedValueOnce(new Error("disconnect timed out")) + .mockResolvedValueOnce({ data: undefined, error: { name: "BadRequest" } }); + const runtime = embeddedRuntime(setup.client, { + knowledgeGateway: { + getEndpoint: () => "http://127.0.0.1:4567/mcp", + getAvailableToolNames: () => ["knowledge_search"], + } as unknown as KnowledgeMcpGateway, + }); + const runOnce = async (requestId: string) => { + for await (const _event of runtime.run( + { + requestId, + conversationId: "conversation-1", + prompt: "search", + workMode: "ask", + knowledgeCapabilityToken: requestId, + }, + new AbortController().signal, + )) { + void _event; + } + }; + try { + await runOnce("request-1"); + await runOnce("request-2"); + await runOnce("request-3"); + await runOnce("request-4"); + const add = setup.client.mcp.add as unknown as ReturnType; + const names = add.mock.calls.map( + (call) => (call[0] as { name: string }).name, + ); + const base = names[0]!; + expect(base).toMatch(/^gbd-[a-f0-9]{6}$/u); + // request-1 failed to disconnect (rejected) -> slot 1 stays leased. + expect(names[1]).toBe(`${base}-2`); + // request-2 disconnect returned an error -> slot 2 stays leased. + expect(names[2]).toBe(`${base}-3`); + // request-3 disconnected cleanly -> slot 3 is reusable. + expect(names[3]).toBe(`${base}-3`); + } finally { + await runtime.dispose(); + } + }); + + it("leases distinct temporary MCP slots for concurrent holders and caps them", () => { + const setup = runClient([]); + const runtime = embeddedRuntime(setup.client) as unknown as { + acquireTemporaryMcpSlot: (client: unknown, key: string) => number; + releaseTemporaryMcpSlot: (client: unknown, key: string, slot: number) => void; + }; + const otherClient = {}; + const slots = Array.from({ length: 9 }, () => + runtime.acquireTemporaryMcpSlot(setup.client, "abc123"), + ); + expect(slots).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9]); + expect(() => runtime.acquireTemporaryMcpSlot(setup.client, "abc123")).toThrow( + /并行的 OpenCode 请求过多/u, + ); + // Other conversation keys and other OpenCode clients are independent. + expect(runtime.acquireTemporaryMcpSlot(setup.client, "def456")).toBe(1); + expect(runtime.acquireTemporaryMcpSlot(otherClient, "abc123")).toBe(1); + runtime.releaseTemporaryMcpSlot(setup.client, "abc123", 4); + expect(runtime.acquireTemporaryMcpSlot(setup.client, "abc123")).toBe(4); + expect(createTemporaryMcpName("gbd-", "abc123", 1)).toBe("gbd-abc123"); + expect(createTemporaryMcpName("gbc-", "abc123", 4)).toBe("gbc-abc123-4"); + }); + it("enables the deterministic MCP tool name when tool ids omit dynamic tools", async () => { const setup = runClient([ { @@ -3694,8 +3812,8 @@ describe("OpenCodeRuntime embedded permission mediation", () => { write: false, bash: false, task: false, - "goodbuddy-data-*": false, - "goodbuddy-custom-*": false, + "gbd-*": false, + "gbc-*": false, skill: true, }, }), @@ -3767,12 +3885,12 @@ describe("OpenCodeRuntime embedded permission mediation", () => { permission: [ { permission: "*", pattern: "*", action: "allow" }, { - permission: "goodbuddy-data-*", + permission: "gbd-*", pattern: "*", action: "deny", }, { - permission: "goodbuddy-custom-*", + permission: "gbc-*", pattern: "*", action: "deny", }, @@ -3783,8 +3901,8 @@ describe("OpenCodeRuntime embedded permission mediation", () => { expect(session.promptAsync).toHaveBeenCalledWith( expect.objectContaining({ tools: { - "goodbuddy-data-*": false, - "goodbuddy-custom-*": false, + "gbd-*": false, + "gbc-*": false, }, }), expect.anything(), @@ -4334,8 +4452,8 @@ describe("OpenCodeRuntime embedded permission mediation", () => { write: false, bash: false, task: false, - "goodbuddy-data-*": false, - "goodbuddy-custom-*": false, + "gbd-*": false, + "gbc-*": false, }, }), expect.anything(), @@ -4483,7 +4601,7 @@ describe("OpenCodeRuntime native customization", () => { "webfetch", "websearch", "write", - "goodbuddy-data-123_search", + "gbd-123_search", "extension_tool", ], error: undefined, @@ -4513,7 +4631,7 @@ describe("OpenCodeRuntime native customization", () => { hints: [], }, { - name: "goodbuddy-data-123", + name: "gbd-123", source: "mcp", template: "temporary prompt", hints: [], @@ -4548,7 +4666,7 @@ describe("OpenCodeRuntime native customization", () => { status: vi.fn().mockResolvedValue({ data: { public: { status: "failed", error: "private failure" }, - "goodbuddy-custom-123": { status: "connected" }, + "gbc-123": { status: "connected" }, }, }), }, @@ -4566,7 +4684,7 @@ describe("OpenCodeRuntime native customization", () => { temporary: { name: "Temporary resource", uri: "docs://temporary", - client: "goodbuddy-data-123", + client: "gbd-123", }, }, }), @@ -4697,8 +4815,8 @@ describe("OpenCodeRuntime native customization", () => { expect(serialized).not.toContain("must not be exposed"); expect(serialized).not.toContain("C:\\private"); expect(serialized).not.toContain("assigned-skill"); - expect(serialized).not.toContain("goodbuddy-data-"); - expect(serialized).not.toContain("goodbuddy-custom-"); + expect(serialized).not.toContain("gbd-"); + expect(serialized).not.toContain("gbc-"); expect(serialized).not.toContain('"invalid"'); vi.mocked(client.tool.ids).mockRejectedValueOnce( diff --git a/src/main/agent/opencode-runtime.ts b/src/main/agent/opencode-runtime.ts index 1172f816..f6be1642 100644 --- a/src/main/agent/opencode-runtime.ts +++ b/src/main/agent/opencode-runtime.ts @@ -95,10 +95,16 @@ const MAX_NATIVE_RESOURCES = runtimeNativeInventoryLimits.resources; const COMPACTION_USAGE_EVENT_GRACE_MS = 1_000; const EMBEDDED_SERVER_USERNAME = "goodbuddy"; const OPENCODE_SKILL_NAME_PATTERN = /^[a-z0-9]+(?:-[a-z0-9]+)*$/u; +// Keep temporary MCP names short: OpenCode exposes tools as +// "_" and common provider APIs cap tool names at 64. +const KNOWLEDGE_MCP_PREFIX = "gbd-"; +const CUSTOM_MCP_PREFIX = "gbc-"; const TEMPORARY_MCP_PREFIXES = [ - "goodbuddy-data-", - "goodbuddy-custom-", + KNOWLEDGE_MCP_PREFIX, + CUSTOM_MCP_PREFIX, ] as const; +const TEMPORARY_MCP_CONVERSATION_HASH_LENGTH = 6; +const MAX_TEMPORARY_MCP_SLOTS = 9; const temporaryMcpToolOverrides = Object.fromEntries( TEMPORARY_MCP_PREFIXES.map((prefix) => [`${prefix}*`, false]), ); @@ -534,6 +540,21 @@ function isTemporaryMcpName(value: string): boolean { return TEMPORARY_MCP_PREFIXES.some((prefix) => value.startsWith(prefix)); } +function temporaryMcpConversationKey(conversationId: string): string { + return createHash("sha256") + .update(conversationId) + .digest("hex") + .slice(0, TEMPORARY_MCP_CONVERSATION_HASH_LENGTH); +} + +export function createTemporaryMcpName( + prefix: (typeof TEMPORARY_MCP_PREFIXES)[number], + conversationKey: string, + slot: number, +): string { + return `${prefix}${conversationKey}${slot === 1 ? "" : `-${slot}`}`; +} + function isTemporaryMcpInventoryItem( ...values: Array ): boolean { @@ -753,6 +774,15 @@ export class OpenCodeRuntime implements AgentRuntime { >(); private readonly conversationRunTails = new Map>(); private mcpMutationTail: Promise = Promise.resolve(); + // Temporary MCP name slots per OpenCode client and conversation. A slot is + // held while a request owns its MCP connections and is only freed after + // every disconnect succeeds, so a request never reuses a name that may still + // be bound to another request's capability token. A new client (restarted + // server) starts with a clean MCP registry and therefore fresh slots. + private readonly temporaryMcpSlots = new WeakMap< + OpencodeClient, + Map> + >(); private readonly dependencies: OpenCodeRuntimeDependencies; constructor( @@ -829,6 +859,47 @@ export class OpenCodeRuntime implements AgentRuntime { } } + private acquireTemporaryMcpSlot( + client: OpencodeClient, + conversationKey: string, + ): number { + let byConversation = this.temporaryMcpSlots.get(client); + if (!byConversation) { + byConversation = new Map(); + this.temporaryMcpSlots.set(client, byConversation); + } + let occupied = byConversation.get(conversationKey); + if (!occupied) { + occupied = new Set(); + byConversation.set(conversationKey, occupied); + } + for (let slot = 1; slot <= MAX_TEMPORARY_MCP_SLOTS; slot += 1) { + if (!occupied.has(slot)) { + occupied.add(slot); + return slot; + } + } + throw new Error( + `同一会话中并行的 OpenCode 请求过多（最多 ${MAX_TEMPORARY_MCP_SLOTS} 个），请等待其他请求结束后重试`, + ); + } + + private releaseTemporaryMcpSlot( + client: OpencodeClient, + conversationKey: string, + slot: number, + ): void { + const byConversation = this.temporaryMcpSlots.get(client); + const occupied = byConversation?.get(conversationKey); + if (!byConversation || !occupied) { + return; + } + occupied.delete(slot); + if (occupied.size === 0) { + byConversation.delete(conversationKey); + } + } + private terminate(child: SpawnedProcess): void { requestProcessTreeTermination(child, { platform: this.dependencies.platform, @@ -2169,6 +2240,16 @@ export class OpenCodeRuntime implements AgentRuntime { let customMcpName: string | undefined; let customMcpToken: string | undefined; const attemptedMcpNames: string[] = []; + const mcpConversationKey = temporaryMcpConversationKey( + request.conversationId, + ); + let mcpSlot: number | undefined; + const temporaryMcpName = ( + prefix: (typeof TEMPORARY_MCP_PREFIXES)[number], + ): string => { + mcpSlot ??= this.acquireTemporaryMcpSlot(client, mcpConversationKey); + return createTemporaryMcpName(prefix, mcpConversationKey, mcpSlot); + }; const subscriptionController = new AbortController(); const imageCapabilityToken = request.imageToolBinding && request.workMode === 'execute' ? this.options.knowledgeGateway?.bindImageTool(request.imageToolBinding, signal, request.knowledgeCapabilityToken) @@ -2181,10 +2262,7 @@ export class OpenCodeRuntime implements AgentRuntime { this.options.knowledgeGateway?.getEndpoint() ) { const knowledgeEndpoint = this.options.knowledgeGateway.getEndpoint()!; - knowledgeMcpName = `goodbuddy-data-${createHash("sha256") - .update(`${request.conversationId}\0${request.requestId}`) - .digest("hex") - .slice(0, 20)}`; + knowledgeMcpName = temporaryMcpName(KNOWLEDGE_MCP_PREFIX); const added = await this.mutateMcp(signal, () => this.controlRequest( "连接内置只读工具", @@ -2246,10 +2324,7 @@ export class OpenCodeRuntime implements AgentRuntime { customMcpToken, signal, ); - customMcpName = `goodbuddy-custom-${createHash("sha256") - .update(`${request.conversationId}\0${request.requestId}`) - .digest("hex") - .slice(0, 20)}`; + customMcpName = temporaryMcpName(CUSTOM_MCP_PREFIX); const added = await this.mutateMcp(signal, () => this.controlRequest( "连接自定义 MCP 工具", @@ -3117,18 +3192,31 @@ export class OpenCodeRuntime implements AgentRuntime { } finally { subscriptionController.abort(); if (imageCapabilityToken && imageCapabilityToken !== request.knowledgeCapabilityToken) this.options.knowledgeGateway?.revoke(imageCapabilityToken); + let allMcpDisconnected = true; for (const name of attemptedMcpNames) { - await this.mutateMcp(new AbortController().signal, () => { - // Queue wait must not consume the disconnect timeout. - const cleanupSignal = AbortSignal.timeout(1_000); - return awaitWithAbort( - client.mcp.disconnect( - { name, directory }, - { signal: cleanupSignal }, - ), - cleanupSignal, - ); - }).catch(() => undefined); + const disconnected = await this.mutateMcp( + new AbortController().signal, + () => { + // Queue wait must not consume the disconnect timeout. + const cleanupSignal = AbortSignal.timeout(1_000); + return awaitWithAbort( + client.mcp.disconnect( + { name, directory }, + { signal: cleanupSignal }, + ), + cleanupSignal, + ); + }, + ).then( + (result) => !(result as { error?: unknown } | undefined)?.error, + () => false, + ); + allMcpDisconnected &&= disconnected; + } + // A slot whose connection may still be alive stays occupied so a later + // request cannot reuse a name bound to this request's token. + if (mcpSlot !== undefined && allMcpDisconnected) { + this.releaseTemporaryMcpSlot(client, mcpConversationKey, mcpSlot); } if (customMcpToken) { this.options.knowledgeGateway?.revoke(customMcpToken);