Skip to content

Commit a54eb6d

Browse files
committed
fix: serialize mcp batches
1 parent ef27c41 commit a54eb6d

7 files changed

Lines changed: 57 additions & 10 deletions

File tree

src/__tests__/cli-batch.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,16 @@ test('batch accepts legacy positionals/flags steps with deprecation warning', as
8282
});
8383
});
8484

85+
test('batch rejects hybrid structured and legacy step shapes', async () => {
86+
const result = await runCliCapture([
87+
'batch',
88+
'--steps',
89+
'[{"command":"open","input":{},"positionals":["settings"]}]',
90+
]);
91+
assert.equal(result.code, 1);
92+
assert.match(result.stderr, /unknown legacy field\(s\): input/);
93+
});
94+
8595
test('batch --steps-file returns clear error for missing file', async () => {
8696
const result = await runCliCapture([
8797
'batch',

src/cli/batch-steps.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,14 @@ function legacyStepToStructuredStep(legacyStep: LegacyCliBatchStep): BatchStep {
5353
}
5454

5555
function isStructuredBatchStep(step: unknown): step is BatchStep {
56-
return step !== null && typeof step === 'object' && !Array.isArray(step) && 'input' in step;
56+
return (
57+
step !== null &&
58+
typeof step === 'object' &&
59+
!Array.isArray(step) &&
60+
'input' in step &&
61+
!('positionals' in step) &&
62+
!('flags' in step)
63+
);
5764
}
5865

5966
function readLegacyCliBatchStep(step: unknown, stepNumber: number): LegacyCliBatchStep {

src/commands/command-projection.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,15 +116,15 @@ function readBatchStepInput(record: Record<string, unknown>, stepNumber: number)
116116
function readBatchStepRuntime(
117117
record: Record<string, unknown>,
118118
stepNumber: number,
119-
): unknown | undefined {
119+
): Record<string, unknown> | undefined {
120120
const runtime = record.runtime;
121121
if (
122122
runtime !== undefined &&
123123
(!runtime || typeof runtime !== 'object' || Array.isArray(runtime))
124124
) {
125125
throw new AppError('INVALID_ARGS', `Batch step ${stepNumber} runtime must be an object.`);
126126
}
127-
return runtime;
127+
return runtime as Record<string, unknown> | undefined;
128128
}
129129

130130
export function prepareDaemonCommandRequest(

src/mcp/__tests__/command-tools.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ test('MCP command tool executor hides client creation behind an execution adapte
2525
{
2626
client,
2727
name: 'devices',
28-
input: { stateDir: '/tmp/agent-device-mcp' },
28+
input: {},
2929
},
3030
]);
3131
assert.deepEqual(result.structuredContent, { name: 'devices', ok: true });

src/mcp/__tests__/router.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import assert from 'node:assert/strict';
22
import { test } from 'vitest';
33
import { listMcpExposedCommandNames } from '../../command-catalog.ts';
44
import { handleMcpMessage } from '../router.ts';
5+
import { handleMcpPayload } from '../server.ts';
56

67
test('MCP exposes every automatable CLI command as a structured direct tool', async () => {
78
const response = await handleMcpMessage({
@@ -44,3 +45,16 @@ test('MCP exposes every automatable CLI command as a structured direct tool', as
4445
assert.equal((invalidFillResponse.result as { isError: boolean }).isError, true);
4546
assert.match(JSON.stringify(invalidFillResponse.result), /Expected target to be set/);
4647
});
48+
49+
test('MCP JSON-RPC batches return responses in request order and skip notifications', async () => {
50+
const response = await handleMcpPayload([
51+
{ jsonrpc: '2.0', id: 'first', method: 'ping' },
52+
{ jsonrpc: '2.0', method: 'notifications/initialized' },
53+
{ jsonrpc: '2.0', id: 'second', method: 'ping' },
54+
]);
55+
56+
assert.deepEqual(
57+
(response as Array<{ id: string }>).map((entry) => entry.id),
58+
['first', 'second'],
59+
);
60+
});

src/mcp/command-tools.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export function createCommandToolExecutor(
4747
throw new Error(`Unknown command tool: ${name}`);
4848
}
4949
const client = deps.createClient(readClientConfig(input));
50-
const result = await deps.runCommand(client, name, input);
50+
const result = await deps.runCommand(client, name, stripClientConfigFields(input));
5151
return {
5252
isError: false,
5353
structuredContent: result,
@@ -69,6 +69,12 @@ function readClientConfig(input: unknown): AgentDeviceClientConfig {
6969
return { stateDir };
7070
}
7171

72+
function stripClientConfigFields(input: unknown): unknown {
73+
if (!input || typeof input !== 'object' || Array.isArray(input)) return input;
74+
const { stateDir: _stateDir, ...commandInput } = input as Record<string, unknown>;
75+
return commandInput;
76+
}
77+
7278
function renderToolText(value: unknown): string {
7379
return typeof value === 'string' ? value : JSON.stringify(value, null, 2);
7480
}

src/mcp/server.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,20 @@
11
import { handleMcpMessage, type JsonRpcMessage } from './router.ts';
22

33
type JsonRpcResponse = Awaited<NonNullable<ReturnType<typeof handleMcpMessage>>>;
4+
type JsonRpcId = string | number | null;
45
type MessageSink = (message: JsonRpcMessage | JsonRpcMessage[]) => void;
56

67
export async function runAgentDeviceMcpServer(): Promise<void> {
78
const decoder = new McpMessageDecoder((messageOrBatch) => {
9+
const fallbackId = fallbackErrorId(messageOrBatch);
810
void handleMcpPayload(messageOrBatch)
911
.then((response) => {
1012
if (response) writeMessage(response);
1113
})
1214
.catch((error: unknown) => {
1315
writeMessage({
1416
jsonrpc: '2.0',
15-
id: null,
17+
id: fallbackId,
1618
error: {
1719
code: -32603,
1820
message: error instanceof Error ? error.message : String(error),
@@ -44,7 +46,7 @@ export async function runAgentDeviceMcpServer(): Promise<void> {
4446
});
4547
}
4648

47-
function handleMcpPayload(
49+
export function handleMcpPayload(
4850
messageOrBatch: JsonRpcMessage | JsonRpcMessage[],
4951
): Promise<unknown | null> {
5052
if (Array.isArray(messageOrBatch)) {
@@ -54,12 +56,20 @@ function handleMcpPayload(
5456
}
5557

5658
async function handleMcpBatch(messages: JsonRpcMessage[]): Promise<JsonRpcResponse[] | null> {
57-
const responses = (
58-
await Promise.all(messages.map(async (message) => await handleMcpMessage(message)))
59-
).flatMap(responseArray);
59+
const responses: JsonRpcResponse[] = [];
60+
for (const message of messages) {
61+
responses.push(...responseArray(await handleMcpMessage(message)));
62+
}
6063
return responses.length > 0 ? responses : null;
6164
}
6265

66+
function fallbackErrorId(messageOrBatch: JsonRpcMessage | JsonRpcMessage[]): JsonRpcId {
67+
if (Array.isArray(messageOrBatch)) {
68+
return messageOrBatch.length === 1 ? (messageOrBatch[0]?.id ?? null) : null;
69+
}
70+
return messageOrBatch.id ?? null;
71+
}
72+
6373
class McpMessageDecoder {
6474
private buffer = '';
6575
private readonly sink: MessageSink;

0 commit comments

Comments
 (0)