diff --git a/.changeset/acp-stdio-mcp-regression.md b/.changeset/acp-stdio-mcp-regression.md new file mode 100644 index 0000000000..90354b7375 --- /dev/null +++ b/.changeset/acp-stdio-mcp-regression.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix ACP session methods rejecting stdio MCP servers, and fix session/load failing when reloading a closed session. diff --git a/packages/acp-server/src/convert.ts b/packages/acp-server/src/convert.ts index 308d400a0d..3e405039a5 100644 --- a/packages/acp-server/src/convert.ts +++ b/packages/acp-server/src/convert.ts @@ -176,7 +176,13 @@ export function acpMcpServersToConfigRecord( const out: Record = {}; for (const server of servers) { if (!('type' in server)) { - throw new Error(`ACP stdio MCP server ${server.name} does not declare a runtime identity`); + out[server.name] = { + transport: 'stdio', + command: server.command, + args: server.args, + env: namedPairsToRecord(server.env), + }; + continue; } if (server.type === 'http' || server.type === 'sse') { out[server.name] = { diff --git a/packages/acp-server/test/convert.test.ts b/packages/acp-server/test/convert.test.ts index c5a1c490ca..fe34f16627 100644 --- a/packages/acp-server/test/convert.test.ts +++ b/packages/acp-server/test/convert.test.ts @@ -19,7 +19,7 @@ describe('acpMcpServersToConfigRecord', () => { expect(acpMcpServersToConfigRecord([])).toBeUndefined(); }); - it('rejects stdio servers that cannot declare a runtime identity', () => { + it('maps stdio servers (no `type` discriminator) with env pairs as a record', () => { const servers: McpServer[] = [ { name: 'fs', @@ -31,9 +31,14 @@ describe('acpMcpServersToConfigRecord', () => { ], }, ]; - expect(() => acpMcpServersToConfigRecord(servers)).toThrow( - 'ACP stdio MCP server fs does not declare a runtime identity', - ); + expect(acpMcpServersToConfigRecord(servers)).toEqual({ + fs: { + transport: 'stdio', + command: '/usr/local/bin/mcp-fs', + args: ['--root', '/tmp'], + env: { API_KEY: 'secret', DEBUG: '1' }, + }, + }); }); it('maps http and sse servers with header pairs as a record', () => { diff --git a/packages/acp-server/test/lifecycle.test.ts b/packages/acp-server/test/lifecycle.test.ts index fe124ca8d8..5b505203f3 100644 --- a/packages/acp-server/test/lifecycle.test.ts +++ b/packages/acp-server/test/lifecycle.test.ts @@ -291,10 +291,10 @@ describe('acp-server session lifecycle', () => { ); it( - 'session/new rejects stdio MCP servers without runtime identity', + 'session/new connects ACP mcpServers as ephemeral session servers', async () => { const c = await boot(); - await expect(c.send('session/new', { + const created = (await c.send('session/new', { cwd: homeDir, mcpServers: [ { @@ -304,16 +304,19 @@ describe('acp-server session lifecycle', () => { env: [{ name: 'KIMI_TEST_MCP_START_DELAY_MS', value: '0' }], }, ], - })).rejects.toThrow('ACP stdio MCP server mock does not declare a runtime identity'); + })) as { sessionId: string }; + expect(created.sessionId).toMatch(/^session_/); // Engine-side assertion: the session scope's MCP handle is the overlay // view and the converted server ended up connected under its ACP name. + const entries = await sessionMcpEntries(c, created.sessionId); + expect(entries.find((e) => e.name === 'mock')?.status).toBe('connected'); }, 30_000, ); it( - 'session/load rejects stdio MCP servers without runtime identity', + 'session/load forwards mcpServers to the re-materialized session', async () => { const c = await boot(); const created = (await c.send('session/new', { cwd: homeDir, mcpServers: [] })) as { @@ -321,13 +324,16 @@ describe('acp-server session lifecycle', () => { }; await c.send('session/close', { sessionId: created.sessionId }); - await expect(c.send('session/load', { + await c.send('session/load', { sessionId: created.sessionId, cwd: homeDir, mcpServers: [ { name: 'mock', command: process.execPath, args: [STDIO_MCP_FIXTURE], env: [] }, ], - })).rejects.toThrow('ACP stdio MCP server mock does not declare a runtime identity'); + }); + + const entries = await sessionMcpEntries(c, created.sessionId); + expect(entries.find((e) => e.name === 'mock')?.status).toBe('connected'); }, 30_000, ); diff --git a/packages/agent-core-v2/src/runtime/runtimeUnitHost.ts b/packages/agent-core-v2/src/runtime/runtimeUnitHost.ts index 9554fbff4c..585b5e0707 100644 --- a/packages/agent-core-v2/src/runtime/runtimeUnitHost.ts +++ b/packages/agent-core-v2/src/runtime/runtimeUnitHost.ts @@ -295,7 +295,7 @@ class SharedRuntimeUnitHost implements RuntimeUnitHost { }, registerRuntime: (runtime) => { if (!active) throw new Error('runtime unit transaction is disposed'); - if (runtimes.some((entry) => entry.runtime.identity.runtimeId === runtime.identity.runtimeId)) { + if (runtimes.some((entry) => entry.active && entry.runtime.identity.runtimeId === runtime.identity.runtimeId)) { throw new Error(`runtime ${runtime.identity.runtimeId} is registered twice in one transaction`); } const staged: StagedRuntime = { runtime, active: true }; diff --git a/packages/agent-core-v2/src/workspace/workspaceMcp/workspaceMcpService.ts b/packages/agent-core-v2/src/workspace/workspaceMcp/workspaceMcpService.ts index 951088adbf..e8c21a4ed9 100644 --- a/packages/agent-core-v2/src/workspace/workspaceMcp/workspaceMcpService.ts +++ b/packages/agent-core-v2/src/workspace/workspaceMcp/workspaceMcpService.ts @@ -125,7 +125,6 @@ export class WorkspaceMcpService extends Disposable implements IWorkspaceMcpServ runtimeResolver: this.runtimeResolver, workspaceId: this.workspaceId, runtimeId: 'local', - requireStdioRuntimeId: true, resolveDefaultTimeouts: () => this.mcpConfig.tunables(), resolveClientName: this.resolveClientName, }); diff --git a/packages/agent-core-v2/test/runtime/runtimeUnitHost.test.ts b/packages/agent-core-v2/test/runtime/runtimeUnitHost.test.ts index 9b9eec0e4a..f29e3ab117 100644 --- a/packages/agent-core-v2/test/runtime/runtimeUnitHost.test.ts +++ b/packages/agent-core-v2/test/runtime/runtimeUnitHost.test.ts @@ -225,6 +225,30 @@ describe('RuntimeUnitHost', () => { disposables.dispose(); }); + it('allows re-registering a runtime id whose earlier registration was removed', async () => { + const { disposables, host, registry } = setup(); + let providerHost!: RuntimeProviderHost; + const handle = await host.provide(emptyImports(), async (provider) => { + providerHost = provider; + return { dispose: () => {} }; + }); + + const first = runtime('one'); + const registration = providerHost.registerRuntime(first); + await registration.remove(); + expect(registry.current('local')).toBeUndefined(); + + const second = runtime('two'); + providerHost.registerRuntime(second); + expect(registry.current('local')).toBe(second); + + await handle.remove(); + expect(registry.current('local')).toBeUndefined(); + expect(second.disposed).toBe(true); + await host.dispose(); + disposables.dispose(); + }); + it('waits for in-flight prepare, rejects new transactions, and tears down in reverse order', async () => { const { disposables, host } = setup(); const order: string[] = [];