Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/acp-stdio-mcp-regression.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@moonshot-ai/kimi-code": patch
---

ACP: fix stdio MCP servers being rejected in session/new, session/load, and session/resume with "does not declare a runtime identity" since v0.37.0, which broke ACP clients (e.g. Zed) that forward stdio MCP servers. Also fix session/load failing with "registered twice in one transaction" when a session is closed and loaded again in the same process.
Comment thread
kimi-agent-bot marked this conversation as resolved.
Outdated
8 changes: 7 additions & 1 deletion packages/acp-server/src/convert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,13 @@ export function acpMcpServersToConfigRecord(
const out: Record<string, McpServerConfig> = {};
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] = {
Expand Down
13 changes: 9 additions & 4 deletions packages/acp-server/test/convert.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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', () => {
Expand Down
18 changes: 12 additions & 6 deletions packages/acp-server/test/lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
{
Expand All @@ -304,30 +304,36 @@ 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 {
sessionId: string;
};
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,
);
Expand Down
2 changes: 1 addition & 1 deletion packages/agent-core-v2/src/runtime/runtimeUnitHost.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
Expand Down
24 changes: 24 additions & 0 deletions packages/agent-core-v2/test/runtime/runtimeUnitHost.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [];
Expand Down
Loading