fix: serialize mcp stdin requests

thymikee · thymikee · commit c030d013fcd8 · 2026-05-27T20:21:59.000+02:00
diff --git a/src/__tests__/client.test.ts b/src/__tests__/client.test.ts
@@ -1,6 +1,7 @@
 import { test } from 'vitest';
 import assert from 'node:assert/strict';
 import { createAgentDeviceClient, type AgentDeviceClientConfig } from '../client.ts';
+import { runCommand } from '../commands/command-surface.ts';
 import type { DaemonRequest, DaemonResponse } from '../contracts.ts';
 import { AppError } from '../utils/errors.ts';
 
@@ -118,6 +119,121 @@ test('apps.open forwards explicit runtime hints through the daemon request', asy
   });
 });
 
+test('structured command input accepts target as deviceTarget alias when no UI target exists', async () => {
+  const setup = createTransport(async (req) => {
+    if (req.command === 'open') {
+      return {
+        ok: true,
+        data: {
+          session: 'qa',
+          appName: 'Settings',
+          appBundleId: 'com.apple.Preferences',
+          platform: 'ios',
+          target: 'tv',
+          device: 'Apple TV',
+          id: 'TV-001',
+          kind: 'simulator',
+        },
+      };
+    }
+    throw new Error(`Unexpected command: ${req.command}`);
+  });
+  const client = createAgentDeviceClient(setup.config, { transport: setup.transport });
+
+  await runCommand(client, 'open', { app: 'Settings', target: 'tv' });
+
+  assert.equal(setup.calls.length, 1);
+  assert.equal(setup.calls[0]?.command, 'open');
+  assert.deepEqual(setup.calls[0]?.positionals, ['Settings']);
+  assert.equal(setup.calls[0]?.flags?.target, 'tv');
+});
+
+test('structured interaction input keeps UI target separate from deviceTarget', async () => {
+  const setup = createTransport(async (req) => {
+    if (req.command === 'get' || req.command === 'longpress') {
+      return {
+        ok: true,
+        data: { ok: true },
+      };
+    }
+    throw new Error(`Unexpected command: ${req.command}`);
+  });
+  const client = createAgentDeviceClient(setup.config, { transport: setup.transport });
+
+  await runCommand(client, 'get', {
+    deviceTarget: 'mobile',
+    format: 'text',
+    target: { kind: 'ref', ref: '@e1' },
+  });
+  await runCommand(client, 'longpress', {
+    deviceTarget: 'mobile',
+    durationMs: 800,
+    target: { kind: 'ref', ref: '@e2' },
+  });
+
+  assert.equal(setup.calls.length, 2);
+  assert.equal(setup.calls[0]?.command, 'get');
+  assert.deepEqual(setup.calls[0]?.positionals, ['text', '@e1']);
+  assert.equal(setup.calls[0]?.flags?.target, 'mobile');
+  assert.equal(setup.calls[1]?.command, 'longpress');
+  assert.deepEqual(setup.calls[1]?.positionals, ['@e2', '800']);
+  assert.equal(setup.calls[1]?.flags?.target, 'mobile');
+});
+
+test('apps.installFromSource forwards source payload and normalizes launch identity', async () => {
+  const setup = createTransport(async () => ({
+    ok: true,
+    data: {
+      packageName: 'com.example.demo',
+      appName: 'Demo',
+      launchTarget: 'com.example.demo',
+      installablePath: '/tmp/materialized/installable/demo.apk',
+      archivePath: '/tmp/materialized/archive/demo.zip',
+      materializationId: 'materialized-123',
+      materializationExpiresAt: '2026-03-13T12:00:00.000Z',
+    },
+  }));
+  const client = createAgentDeviceClient(setup.config, { transport: setup.transport });
+
+  const result = await client.apps.installFromSource({
+    platform: 'android',
+    retainPaths: true,
+    retentionMs: 60_000,
+    source: {
+      kind: 'url',
+      url: 'https://example.com/demo.apk',
+      headers: { authorization: 'Bearer token' },
+    },
+  });
+
+  assert.equal(setup.calls.length, 1);
+  assert.equal(setup.calls[0]?.command, 'install_source');
+  assert.deepEqual(setup.calls[0]?.meta?.installSource, {
+    kind: 'url',
+    url: 'https://example.com/demo.apk',
+    headers: { authorization: 'Bearer token' },
+  });
+  assert.equal(setup.calls[0]?.meta?.retainMaterializedPaths, true);
+  assert.equal(setup.calls[0]?.meta?.materializedPathRetentionMs, 60_000);
+  assert.deepEqual(result, {
+    appName: 'Demo',
+    appId: 'com.example.demo',
+    bundleId: undefined,
+    packageName: 'com.example.demo',
+    launchTarget: 'com.example.demo',
+    installablePath: '/tmp/materialized/installable/demo.apk',
+    archivePath: '/tmp/materialized/archive/demo.zip',
+    materializationId: 'materialized-123',
+    materializationExpiresAt: '2026-03-13T12:00:00.000Z',
+    identifiers: {
+      session: 'qa',
+      appId: 'com.example.demo',
+      appBundleId: undefined,
+      package: 'com.example.demo',
+    },
+  });
+});
+
 test('apps.installFromSource derives Android launchTarget from packageName when daemon omits it', async () => {
   const setup = createTransport(async () => ({
     ok: true,
diff --git a/src/commands/command-input.ts b/src/commands/command-input.ts
@@ -48,6 +48,7 @@ export type SelectorSnapshotInput = {
 };
 
 export type PointInput = { x: number; y: number };
+type CommonInputOptions = { readTargetAlias?: boolean };
 
 function commandInputSchema(
   properties: Record<string, JsonSchema>,
@@ -249,7 +250,9 @@ export function readFieldInput<TFields extends CommandFieldMap>(
       return value === undefined ? [] : [[key, value]];
     }),
   );
-  const commonInput = readCommonInput(record);
+  const commonInput = readCommonInput(record, {
+    readTargetAlias: !Object.hasOwn(fields, 'target'),
+  });
   return compactRecord({
     ...commonInput,
     ...commonToClientOptions(commonInput),
@@ -265,11 +268,14 @@ export function readInputRecord(input: unknown): Record<string, unknown> {
   return input as Record<string, unknown>;
 }
 
-export function readCommonInput(record: Record<string, unknown>): CommonCommandInput {
+export function readCommonInput(
+  record: Record<string, unknown>,
+  options: CommonInputOptions = {},
+): CommonCommandInput {
   return {
     session: optionalString(record, 'session'),
     platform: optionalEnum(record, 'platform', PLATFORM_VALUES),
-    deviceTarget: optionalEnum(record, 'deviceTarget', DEVICE_TARGET_VALUES),
+    deviceTarget: readDeviceTarget(record, options),
     device: optionalString(record, 'device'),
     udid: optionalString(record, 'udid'),
     serial: optionalString(record, 'serial'),
@@ -285,6 +291,19 @@ export function readCommonInput(record: Record<string, unknown>): CommonCommandI
   };
 }
 
+function readDeviceTarget(
+  record: Record<string, unknown>,
+  options: CommonInputOptions,
+): DeviceTarget | undefined {
+  const deviceTarget = optionalEnum(record, 'deviceTarget', DEVICE_TARGET_VALUES);
+  if (options.readTargetAlias === false || record.target === undefined) return deviceTarget;
+  const targetAlias = optionalEnum(record, 'target', DEVICE_TARGET_VALUES);
+  if (deviceTarget !== undefined && targetAlias !== deviceTarget) {
+    throw new Error('Expected target alias to match deviceTarget when both are set.');
+  }
+  return deviceTarget ?? targetAlias;
+}
+
 function readInteractionTarget(
   record: Record<string, unknown>,
   key: string,
@@ -548,6 +567,12 @@ function commonProperties(): Record<string, JsonSchema> {
       enum: DEVICE_TARGET_VALUES,
       description: 'Device target form. Maps to the CLI --target flag.',
     },
+    target: {
+      type: 'string',
+      enum: DEVICE_TARGET_VALUES,
+      description:
+        'Alias for deviceTarget on commands without a UI target field. Interaction commands reserve target for the UI element.',
+    },
     device: { type: 'string', description: 'Device name selector.' },
     udid: { type: 'string', description: 'iOS device UDID selector.' },
     serial: { type: 'string', description: 'Android serial selector.' },
diff --git a/src/mcp/__tests__/router.test.ts b/src/mcp/__tests__/router.test.ts
@@ -1,8 +1,9 @@
 import assert from 'node:assert/strict';
+import { setImmediate } from 'node:timers/promises';
 import { test } from 'vitest';
 import { listMcpExposedCommandNames } from '../../command-catalog.ts';
 import { handleMcpMessage } from '../router.ts';
-import { handleMcpPayload } from '../server.ts';
+import { createMcpPayloadQueue, handleMcpPayload } from '../server.ts';
 
 test('MCP exposes every automatable CLI command as a structured direct tool', async () => {
   const response = await handleMcpMessage({
@@ -58,3 +59,38 @@ test('MCP JSON-RPC batches return responses in request order and skip notificati
     ['first', 'second'],
   );
 });
+
+test('MCP stdio payload queue serializes separate messages', async () => {
+  const started: JsonRpcId[] = [];
+  const writes: unknown[] = [];
+  const completions = new Map<JsonRpcId, (response: unknown) => void>();
+  const queue = createMcpPayloadQueue({
+    handlePayload: async (message) => {
+      const id = Array.isArray(message) ? null : (message.id ?? null);
+      started.push(id);
+      return await new Promise((resolve) => completions.set(id, resolve));
+    },
+    write: (message) => {
+      writes.push(message);
+    },
+  });
+
+  queue.push({ jsonrpc: '2.0', id: 'first', method: 'tools/call' });
+  queue.push({ jsonrpc: '2.0', id: 'second', method: 'tools/call' });
+  await Promise.resolve();
+
+  assert.deepEqual(started, ['first']);
+  completions.get('first')?.({ jsonrpc: '2.0', id: 'first', result: {} });
+  await setImmediate();
+
+  assert.deepEqual(started, ['first', 'second']);
+  completions.get('second')?.({ jsonrpc: '2.0', id: 'second', result: {} });
+  await queue.idle();
+
+  assert.deepEqual(
+    writes.map((message) => (message as { id: JsonRpcId }).id),
+    ['first', 'second'],
+  );
+});
+
+type JsonRpcId = string | number | null;
diff --git a/src/mcp/server.ts b/src/mcp/server.ts
@@ -3,24 +3,15 @@ import { handleMcpMessage, type JsonRpcMessage } from './router.ts';
 type JsonRpcResponse = Awaited<NonNullable<ReturnType<typeof handleMcpMessage>>>;
 type JsonRpcId = string | number | null;
 type MessageSink = (message: JsonRpcMessage | JsonRpcMessage[]) => void;
+type PayloadHandler = (
+  messageOrBatch: JsonRpcMessage | JsonRpcMessage[],
+) => Promise<unknown | null>;
+type MessageWriter = (message: unknown) => void;
 
 export async function runAgentDeviceMcpServer(): Promise<void> {
+  const payloadQueue = createMcpPayloadQueue();
   const decoder = new McpMessageDecoder((messageOrBatch) => {
-    const fallbackId = fallbackErrorId(messageOrBatch);
-    void handleMcpPayload(messageOrBatch)
-      .then((response) => {
-        if (response) writeMessage(response);
-      })
-      .catch((error: unknown) => {
-        writeMessage({
-          jsonrpc: '2.0',
-          id: fallbackId,
-          error: {
-            code: -32603,
-            message: error instanceof Error ? error.message : String(error),
-          },
-        });
-      });
+    payloadQueue.push(messageOrBatch);
   });
 
   process.stdin.setEncoding('utf8');
@@ -44,6 +35,44 @@ export async function runAgentDeviceMcpServer(): Promise<void> {
     process.stdin.on('close', resolve);
     process.stdin.resume();
   });
+  await payloadQueue.idle();
+}
+
+export function createMcpPayloadQueue(
+  options: {
+    handlePayload?: PayloadHandler;
+    write?: MessageWriter;
+  } = {},
+): {
+  push: (messageOrBatch: JsonRpcMessage | JsonRpcMessage[]) => void;
+  idle: () => Promise<void>;
+} {
+  const handlePayload = options.handlePayload ?? handleMcpPayload;
+  const write = options.write ?? writeMessage;
+  let pending = Promise.resolve();
+  return {
+    push: (messageOrBatch) => {
+      const fallbackId = fallbackErrorId(messageOrBatch);
+      pending = pending
+        .then(async () => {
+          const response = await handlePayload(messageOrBatch);
+          if (response) write(response);
+        })
+        .catch((error: unknown) => {
+          write({
+            jsonrpc: '2.0',
+            id: fallbackId,
+            error: {
+              code: -32603,
+              message: error instanceof Error ? error.message : String(error),
+            },
+          });
+        });
+    },
+    idle: async () => {
+      await pending;
+    },
+  };
 }
 
 export function handleMcpPayload(
diff --git a/website/docs/docs/agent-setup.md b/website/docs/docs/agent-setup.md
@@ -56,6 +56,8 @@ Use MCP tools or the CLI in the integrated terminal. If `agent-device` is not on
 
 `agent-device mcp` starts the official stdio MCP server. It exposes direct structured tools for installed CLI commands. Tools run through command contracts and `AgentDeviceClient`; local-only workflows stay CLI-only rather than subprocess fallbacks.
 
+Tool execution failures are returned as MCP tool results with `isError: true`; clients and agents should inspect the tool result, not only the successful JSON-RPC envelope.
+
 MCP clients must not use this server as a generic shell runner. If the CLI is missing, agents should ask a human before installing or updating packages, then verify with `agent-device --version` and start with `agent-device help workflow`.
 
 Global install configuration:
