fix(mcp): restore result and config parity (#1343)

* fix(mcp): restore result and config parity

* refactor(mcp): narrow parsed input types

* refactor(mcp): simplify parity boundaries

* refactor: narrow validated MCP tool inputs

* refactor: preserve command result types in MCP

* refactor: remove impossible MCP result fallbacks

Author: thymikee

src/commands/command-metadata.ts

@@ -37,6 +37,8 @@ export function isCommandName(name: string): name is CommandName {
   return commandMetadataMap.has(name as CommandName);
 }
 
+export function findCommandMetadata(name: CommandName): AnyCommandMetadata;
+export function findCommandMetadata(name: string): AnyCommandMetadata | undefined;
 export function findCommandMetadata(name: string): AnyCommandMetadata | undefined {
   return commandMetadataMap.get(name as CommandName);
 }

src/commands/command-surface.ts

@@ -7,16 +7,28 @@ const commandSurface = listCommandFamilyDefinitions();
 
 export type { BatchCommandName, CommandName };
 
+type CommandDefinitionFor<Name extends CommandName> = Extract<
+  CommandFamilyDefinition,
+  { name: Name }
+>;
+
+export type CommandExecutionResult<Name extends CommandName = CommandName> = Awaited<
+  ReturnType<CommandDefinitionFor<Name>['invoke']>
+>;
+
 const commandMap: ReadonlyMap<CommandName, CommandFamilyDefinition> = new Map(
   commandSurface.map((definition) => [definition.name, definition]),
 );
 
-export async function runCommand(
+export async function runCommand<Name extends CommandName>(
   client: AgentDeviceClient,
-  name: CommandName,
+  name: Name,
   input: unknown,
-): Promise<unknown> {
-  return await getCommandDefinition(name).invoke(client, input);
+): Promise<CommandExecutionResult<Name>> {
+  // The map is total over CommandName, but Map#get cannot retain the correlation
+  // between a runtime key and that definition's return type. Re-establish it at
+  // this single lookup seam; callers keep the per-command result type.
+  return (await getCommandDefinition(name).invoke(client, input)) as CommandExecutionResult<Name>;
 }
 
 /**

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

@@ -0,0 +1,95 @@
+import assert from 'node:assert/strict';
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+import { afterEach, test, vi } from 'vitest';
+import { createAgentDeviceClient } from '../../agent-device-client.ts';
+import type { AgentDeviceClient, AgentDeviceDaemonTransport } from '../../client/client-types.ts';
+import type { CommandExecutionResult } from '../../commands/command-surface.ts';
+import { createCommandToolExecutor, listCommandTools } from '../command-tools.ts';
+import { validateAgainstSchema } from './output-schema-validator.ts';
+
+afterEach(() => {
+  vi.unstubAllEnvs();
+  if (temporaryDirectory) {
+    fs.rmSync(temporaryDirectory, { recursive: true, force: true });
+    temporaryDirectory = undefined;
+  }
+});
+
+let temporaryDirectory: string | undefined;
+
+test('MCP collection results use object envelopes without changing object results or text', async () => {
+  const results = {
+    devices: [
+      {
+        id: 'device-1',
+        name: 'iPhone',
+        platform: 'ios',
+        target: 'mobile',
+        kind: 'device',
+        identifiers: {},
+      },
+    ],
+    apps: ['com.example.app'],
+    wait: { waitedMs: 10 },
+  } satisfies Record<'devices' | 'apps' | 'wait', CommandExecutionResult>;
+  const executor = createCommandToolExecutor({
+    createClient: () => ({}) as AgentDeviceClient,
+    runCommand: async (_client, name) => results[name as keyof typeof results],
+  });
+
+  const devices = await executor.execute('devices', {});
+  const apps = await executor.execute('apps', {});
+  const wait = await executor.execute('wait', {});
+
+  assert.deepEqual(devices.structuredContent, {
+    devices: [
+      { id: 'device-1', name: 'iPhone', platform: 'ios', target: 'mobile', kind: 'device' },
+    ],
+  });
+  assert.deepEqual(apps.structuredContent, { apps: results.apps });
+  assert.deepEqual(wait.structuredContent, results.wait);
+  assert.equal(devices.content[0]?.text, 'iPhone (ios device target=mobile)');
+  assert.equal(apps.content[0]?.text, 'com.example.app');
+
+  for (const [name, result] of [
+    ['devices', devices],
+    ['apps', apps],
+  ] as const) {
+    const schema = listCommandTools().find((tool) => tool.name === name)?.outputSchema;
+    assert.ok(schema, `${name} advertises an output schema`);
+    assert.deepEqual(validateAgainstSchema(result.structuredContent, schema), []);
+  }
+});
+
+test('MCP applies config-backed command defaults with explicit-input precedence and applicability', async () => {
+  const home = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-mcp-config-'));
+  temporaryDirectory = home;
+  const configuredXctestrun = path.join(home, 'configured.xctestrun');
+  fs.mkdirSync(path.join(home, '.agent-device'));
+  fs.writeFileSync(
+    path.join(home, '.agent-device', 'config.json'),
+    JSON.stringify({ iosXctestrunFile: configuredXctestrun, appsFilter: 'all' }),
+  );
+  vi.stubEnv('HOME', home);
+
+  const calls: Array<Parameters<AgentDeviceDaemonTransport>[0]> = [];
+  const transport: AgentDeviceDaemonTransport = async (request) => {
+    calls.push(request);
+    return { ok: true, data: { nodes: [], truncated: false } };
+  };
+  const executor = createCommandToolExecutor({
+    createClient: (config) => createAgentDeviceClient(config, { transport }),
+  });
+
+  await executor.execute('snapshot', {});
+  await executor.execute('snapshot', { iosXctestrunFile: '/explicit/runner.xctestrun' });
+
+  assert.deepEqual(
+    calls.map((request) => request.flags?.iosXctestrunFile),
+    [configuredXctestrun, '/explicit/runner.xctestrun'],
+  );
+  assert.ok(calls.every((request) => request.command === 'snapshot'));
+  assert.ok(calls.every((request) => request.flags?.appsFilter === undefined));
+});

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

@@ -209,7 +209,7 @@ test('MCP includeCost:true opts into agent-cost: sets client.cost, strips the ar
     },
     runCommand: async (_client, name, input) => {
       calls.push({ name, input });
-      return { message: `Ran ${name}`, cost: { wallClockMs: 42 } };
+      return { waitedMs: 42, cost: { wallClockMs: 42, runnerRoundTrips: 0 } };
     },
   });
 
@@ -220,7 +220,10 @@ test('MCP includeCost:true opts into agent-cost: sets client.cost, strips the ar
   // includeCost is an MCP-boundary field and must not leak into the command input.
   assert.deepEqual(calls, [{ name: 'wait', input: {} }]);
   // The daemon-provided cost rides through structuredContent unchanged.
-  assert.deepEqual(result.structuredContent, { message: 'Ran wait', cost: { wallClockMs: 42 } });
+  assert.deepEqual(result.structuredContent, {
+    waitedMs: 42,
+    cost: { wallClockMs: 42, runnerRoundTrips: 0 },
+  });
 });
 
 test('MCP includeCost absent/false leaves the request shape untouched (no cost config)', async () => {
@@ -409,18 +412,13 @@ test('MCP prepare outputSchema stays complete for the typed non-exposed command'
   assert.ok(prepareSchema.required?.includes('timing'));
 });
 
-test('MCP untyped tools stay byte-identical: no outputSchema key', () => {
+test('MCP untyped object tools stay byte-identical: no outputSchema key', () => {
   const tools = listCommandTools();
 
   // snapshot is intentionally absent from the typed registry (dynamic shape).
   const snapshot = tools.find((tool) => tool.name === 'snapshot');
   assert.ok(snapshot);
   assert.equal('outputSchema' in snapshot, false);
-
-  // devices is likewise untyped.
-  const devices = tools.find((tool) => tool.name === 'devices');
-  assert.ok(devices);
-  assert.equal('outputSchema' in devices, false);
 });
 
 test('MCP boot structuredContent is consistent with its advertised outputSchema', async () => {

src/mcp/__tests__/router.test.ts

@@ -45,6 +45,16 @@ test('MCP exposes every automatable CLI command as a structured direct tool', as
   assert.ok(invalidFillResponse && 'result' in invalidFillResponse);
   assert.equal((invalidFillResponse.result as { isError: boolean }).isError, true);
   assert.match(JSON.stringify(invalidFillResponse.result), /Expected target to be set/);
+
+  const malformedArgumentsResponse = await handleMcpMessage({
+    jsonrpc: '2.0',
+    id: 3,
+    method: 'tools/call',
+    params: { name: 'devices', arguments: [] },
+  });
+  assert.ok(malformedArgumentsResponse && 'result' in malformedArgumentsResponse);
+  assert.equal((malformedArgumentsResponse.result as { isError: boolean }).isError, true);
+  assert.match(JSON.stringify(malformedArgumentsResponse.result), /Expected object parameters/);
 });
 
 test('MCP JSON-RPC batches return responses in request order and skip notifications', async () => {

src/mcp/command-output-schemas.ts

@@ -27,7 +27,7 @@ import { DEVICE_TARGETS, PUBLIC_PLATFORMS } from '../kernel/device.ts';
  *    and discriminated-union branches mirror the source contract types.
  */
 
-const DEVICE_KINDS = ['simulator', 'emulator', 'device'] as const;
+export const DEVICE_KINDS = ['simulator', 'emulator', 'device'] as const;
 
 function numberSchema(description?: string): JsonSchema {
   return { type: 'number', ...(description ? { description } : {}) };