fix(mcp): admit a batch step's input against the nested command's own schema

src/commands/batch/metadata.ts

@@ -89,6 +89,12 @@ function batchStepSchema(nestedCommands: readonly string[]): JsonSchema {
       input: {
         type: 'object',
         additionalProperties: true,
+        // The accepted keys depend on `command`, so this object cannot be typed
+        // here. `commandInputFor` names the sibling that resolves them, which is
+        // what lets the model-facing admission boundary check a step's input
+        // against that command's own advertised schema instead of waving through
+        // an opaque object (`mcp/command-tools.ts`).
+        commandInputFor: 'command',
         description:
           'Structured command input for the nested command. Use the matching MCP tool schema for this object.',
       },

src/commands/command-contract.ts

@@ -14,6 +14,12 @@ export type JsonSchema = {
   const?: unknown;
   minimum?: number;
   maximum?: number;
+  /**
+   * Marks this object as ANOTHER command's input, naming the sibling property
+   * that holds that command's name. Model-facing admission recurses into it
+   * with that command's own advertised schema (`mcp/command-tools.ts`).
+   */
+  commandInputFor?: string;
 };
 
 export type CommandMetadata<Name extends string, Input> = {

src/core/batch-policy.ts

@@ -59,12 +59,30 @@ export function normalizeBatchCommandName(command: unknown): string {
   return typeof command === 'string' ? command.trim().toLowerCase() : '';
 }
 
+/**
+ * The command a raw step value will RUN as, or undefined.
+ *
+ * Every caller that decides something about a step — the readers below, and the
+ * model-facing admission boundary in `mcp/command-tools.ts` — must resolve the
+ * name through THIS function, because a step is normalized before it runs:
+ * a caller matching the raw value exactly would see ` SNAPSHOT ` as no command
+ * at all, while the reader resolves it to `snapshot` and runs it. Admission
+ * checking one command while the daemon runs another is the whole failure, so
+ * the read below is this function plus an error rather than a second copy.
+ */
+export function resolveStructuredBatchCommandName(
+  command: unknown,
+): StructuredBatchCommandName | undefined {
+  const normalized = normalizeBatchCommandName(command);
+  return isStructuredBatchCommandName(normalized) ? normalized : undefined;
+}
+
 export function readStructuredBatchCommandName(
   command: unknown,
   stepNumber: number,
 ): StructuredBatchCommandName {
-  const normalized = normalizeBatchCommandName(command);
-  if (isStructuredBatchCommandName(normalized)) return normalized;
+  const resolved = resolveStructuredBatchCommandName(command);
+  if (resolved !== undefined) return resolved;
   throw new AppError(
     'INVALID_ARGS',
     `Batch step ${stepNumber} command is not available through command batch: ${String(command)}`,

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

@@ -0,0 +1,184 @@
+import assert from 'node:assert/strict';
+import { test } from 'vitest';
+import type { AgentDeviceClient } from '../../client/client-types.ts';
+import { STRUCTURED_BATCH_COMMAND_NAMES } from '../../core/batch-policy.ts';
+import { findCommandMetadata } from '../../commands/command-metadata.ts';
+import { createCommandToolExecutor } from '../command-tools.ts';
+
+// `batch` is the one tool whose input nests another command's input, and its
+// step object is free-form by necessity (the accepted keys depend on the sibling
+// `command`, which JSON Schema cannot express). The flat admission scan looked
+// straight past it, so every key refused on a flat call was admitted one level
+// in — and `readBatchDaemonStep` projects a step's input into per-step daemon
+// request FLAGS, where `iosSimulatorDeviceSet` selects the simulator device set
+// `resolveTargetDevice` searches and `iosXctestrunFile` selects the `.xctestrun`
+// the Apple runner launches.
+//
+// The invariant these tests hold is a PARITY, not a key list: a batch step's
+// input admits exactly what the nested command's own tool admits. Stated that
+// way, a key added to `OPERATOR_INPUT_GUIDANCE` — or a whole new operator
+// classification — is covered the day it lands, with nothing here to update.
+const OPERATOR_PROBES: Readonly<Record<string, string>> = {
+  daemonAuthToken: 'stolen-token',
+  daemonBaseUrl: 'http://attacker.example:9000',
+  stateDir: '/attacker/state-dir',
+  cwd: '/attacker/cwd',
+  iosSimulatorDeviceSet: '/attacker/device-set',
+  iosXctestrunFile: '/attacker/run.xctestrun',
+  iosXctestDerivedDataPath: '/attacker/derived',
+  iosXctestEnvDir: '/attacker/env',
+  config: '/attacker/config.json',
+  remoteConfig: '/attacker/remote.json',
+  totallyUnknownKey: 'x',
+};
+
+function createProbeExecutor() {
+  const calls: Array<{ name: string; input: Record<string, unknown> }> = [];
+  const executor = createCommandToolExecutor({
+    createClient: () => ({}) as AgentDeviceClient,
+    runCommand: async (_client, name, input) => {
+      calls.push({ name, input: input as Record<string, unknown> });
+      return { total: 0, executed: 0, totalDurationMs: 0, results: [] };
+    },
+  });
+  return { calls, executor };
+}
+
+test('a batch step admits exactly what the nested command tool admits', async () => {
+  const { calls, executor } = createProbeExecutor();
+
+  for (const command of STRUCTURED_BATCH_COMMAND_NAMES) {
+    for (const [key, probe] of Object.entries(OPERATOR_PROBES)) {
+      const flat = await executor.execute(command, { [key]: probe });
+      calls.length = 0;
+      const nested = await executor.execute('batch', {
+        steps: [{ command, input: { [key]: probe } }],
+      });
+      assert.equal(
+        nested.isError,
+        flat.isError,
+        `${command}: nested ${key} must be admitted iff flat ${key} is`,
+      );
+      if (!flat.isError) continue;
+      assert.deepEqual(calls, [], `${command}: a refused nested ${key} must not be dispatched`);
+      assert.match(
+        nested.content[0]?.text ?? '',
+        new RegExp(`batch\\.steps\\[0\\]\\.input: ${key} is not`),
+        `${command}: nested ${key} must be refused with the flat guidance, located`,
+      );
+    }
+  }
+});
+
+test('a refused step is refused wherever it sits in the batch', async () => {
+  const { calls, executor } = createProbeExecutor();
+
+  const result = await executor.execute('batch', {
+    steps: [
+      { command: 'snapshot', input: {} },
+      { command: 'tap', input: { target: { kind: 'selector', selector: 'text=OK' } } },
+      { command: 'snapshot', input: { iosXctestrunFile: '/attacker/run.xctestrun' } },
+    ],
+  });
+
+  assert.equal(result.isError, true);
+  assert.match(result.content[0]?.text ?? '', /batch\.steps\[2\]\.input: iosXctestrunFile is not/);
+  assert.match(result.content[0]?.text ?? '', /AGENT_DEVICE_IOS_XCTESTRUN_FILE/);
+  assert.deepEqual(calls, [], 'no step runs when one is refused');
+});
+
+test('nested admission leaves legitimate batch input untouched', async () => {
+  const { calls, executor } = createProbeExecutor();
+
+  const steps = [
+    { command: 'open', input: { app: 'settings' } },
+    { command: 'snapshot', input: { interactiveOnly: true, noRecord: true } },
+    { command: 'tap', input: { target: { kind: 'selector', selector: 'text=OK' } } },
+    { command: 'type', input: { text: 'hello' }, runtime: { platform: 'ios' } },
+    { command: 'wait', input: { kind: 'duration', durationMs: 100, session: 'sim' } },
+  ];
+  const result = await executor.execute('batch', { steps });
+
+  assert.equal(result.isError, false, result.content[0]?.text);
+  assert.deepEqual(calls[0]?.input.steps, steps);
+});
+
+// A step is NORMALIZED before it runs (`resolveStructuredBatchCommandName`
+// trims and lowercases), so an admission boundary that matched the raw value
+// exactly would see ` SNAPSHOT ` as no command at all, check nothing, and let
+// the daemon run `snapshot` with the operator paths still aboard. Admission and
+// the reader must resolve a name identically or the boundary guards a different
+// command than the one that runs.
+const COMMAND_SPELLINGS = [
+  'snapshot',
+  ' snapshot',
+  'snapshot ',
+  ' SNAPSHOT ',
+  'SnApShOt',
+  '\tsnapshot\n',
+] as const;
+
+test('a step is admitted as the command it will run as, however it is spelled', async () => {
+  const { calls, executor } = createProbeExecutor();
+
+  for (const command of COMMAND_SPELLINGS) {
+    const steps = [{ command, input: { iosXctestrunFile: '/attacker/run.xctestrun' } }];
+    calls.length = 0;
+    const result = await executor.execute('batch', { steps });
+
+    assert.equal(result.isError, true, `${JSON.stringify(command)} must be refused`);
+    assert.match(
+      result.content[0]?.text ?? '',
+      /batch\.steps\[0\]\.input: iosXctestrunFile is not/,
+      `${JSON.stringify(command)} must be refused as the command it resolves to`,
+    );
+    assert.deepEqual(calls, [], `${JSON.stringify(command)} must not be dispatched`);
+  }
+});
+
+// The drift guard behind the case above: whatever spelling the reader accepts,
+// admission must have checked. Stated against the reader itself, so a change to
+// how a step name is normalized cannot quietly reopen the gap.
+test('admission refuses every spelling the batch reader resolves', async () => {
+  const { executor } = createProbeExecutor();
+  const readBatch = findCommandMetadata('batch').readInput;
+
+  for (const command of [...COMMAND_SPELLINGS, 'not-a-command', 'batch', '', '  ']) {
+    const steps = [{ command, input: { iosXctestrunFile: '/attacker/run.xctestrun' } }];
+    let resolves = true;
+    try {
+      readBatch({ steps });
+    } catch {
+      resolves = false;
+    }
+    const result = await executor.execute('batch', { steps });
+    assert.equal(
+      result.isError,
+      resolves,
+      `${JSON.stringify(command)}: admission must refuse it iff the reader runs it`,
+    );
+  }
+});
+
+// A step whose command cannot be resolved has no schema to check its input
+// against — and needs none, because that step cannot run. Admission must fall
+// through to the reader that owns the error instead of answering with a key
+// complaint that would bury it.
+test('an unresolvable step command is left to the batch reader', async () => {
+  const { calls, executor } = createProbeExecutor();
+  const steps = [
+    { command: 'not-a-command', input: { iosXctestrunFile: '/attacker/run.xctestrun' } },
+  ];
+
+  const result = await executor.execute('batch', { steps });
+
+  assert.equal(result.isError, false, 'admission has no verdict to give here');
+  assert.deepEqual(calls[0]?.input.steps, steps);
+
+  // And the reader that does own it still refuses the step, so the unchecked
+  // input never reaches a daemon request flag.
+  assert.throws(
+    () => findCommandMetadata('batch').readInput({ steps }),
+    /not available through command batch/,
+  );
+});

src/mcp/command-tools.ts

@@ -11,12 +11,14 @@ import {
   type CommandName,
 } from '../commands/command-metadata.ts';
 import { mcpBody } from '../commands/command-text.ts';
+import { resolveStructuredBatchCommandName } from '../core/batch-policy.ts';
 import {
   resolveCommandRecordsSessionAction,
   resolveCommandTimeoutPolicy,
 } from '../core/command-descriptor/registry.ts';
 import { MCP_COMMAND_OUTPUT_SCHEMAS } from './mcp-output-schemas.ts';
 import { AppError } from '@agent-device/kernel/errors';
+import { isRecord } from '../utils/parsing.ts';
 import { formatToolErrorText, normalizeToolError } from './tool-error.ts';
 import { resolveMcpConfigDefaults } from './tool-input-config.ts';
 import { projectStructuredContent } from './tool-result.ts';
@@ -134,8 +136,16 @@ export function createCommandToolExecutor(deps: CommandToolExecutorDeps = {}): C
       // with the operator's token. Reject every raw key the advertised schema
       // does not list, BEFORE config/env defaults merge (operator env/config
       // values still resolve below — they never arrive as tool input).
+      //
+      // "Every tool schema is additionalProperties:false" holds for the keys a
+      // schema can NAME. A nested command's input keys depend on a sibling
+      // value, so `batch` cannot name them and declares that object free-form;
+      // the flat scan below then looked straight past it. The second check
+      // recurses there — see `findInadmissibleNestedCommandInput`.
       const metadata = findCommandMetadata(name);
-      const rejection = findInadmissibleInput(name, metadata, input);
+      const rejection =
+        findInadmissibleInput(name, metadata, input) ??
+        findInadmissibleNestedCommandInput(metadata.inputSchema, input, name);
       if (rejection) {
         return buildErrorToolResult(
           new AppError('INVALID_ARGS', rejection),
@@ -343,6 +353,91 @@ function findInadmissibleInput(
   return undefined;
 }
 
+/**
+ * The first inadmissible key inside a NESTED command input, with guidance, or
+ * undefined.
+ *
+ * {@link findInadmissibleInput} reads the FLAT keys of a tool call, which is the
+ * whole boundary for a tool whose schema is `additionalProperties:false` all the
+ * way down. `batch` is not: a step's accepted keys depend on its sibling
+ * `command`, which JSON Schema cannot express, so `steps[].input` is declared
+ * free-form and the flat scan looked straight past it. Everything the flat scan
+ * refuses was therefore admitted one level in — and `readBatchDaemonStep`
+ * projects a step's input into per-step daemon request FLAGS, where
+ * `iosSimulatorDeviceSet` picks the simulator device set `resolveTargetDevice`
+ * searches and `iosXctestrunFile` picks the `.xctestrun` the Apple runner
+ * launches. That is the operator-infrastructure write
+ * {@link OPERATOR_INPUT_GUIDANCE} exists to refuse, reached through the one
+ * input the model is invited to nest.
+ *
+ * So the schema declares where those objects are (`commandInputFor` names the
+ * sibling holding the command name) and this walks the tool's own schema against
+ * the raw arguments, checking each one with {@link findInadmissibleInput}
+ * against that command's advertised schema. Nested admission is the same
+ * function, and returns the same answer, as flat admission: a step's input
+ * accepts exactly what the nested command's own tool accepts, no more.
+ */
+function findInadmissibleNestedCommandInput(
+  schema: JsonSchema | undefined,
+  value: unknown,
+  path: string,
+): string | undefined {
+  if (!schema || value === undefined) return undefined;
+  if (Array.isArray(value)) {
+    return firstRejection(value, (item, index) =>
+      findInadmissibleNestedCommandInput(schema.items, item, `${path}[${index}]`),
+    );
+  }
+  if (!isRecord(value)) return undefined;
+  return (
+    firstRejection(schema.oneOf ?? [], (branch) =>
+      findInadmissibleNestedCommandInput(branch, value, path),
+    ) ??
+    firstRejection(Object.entries(schema.properties ?? {}), ([key, property]) =>
+      Object.hasOwn(value, key)
+        ? findInadmissibleNestedProperty(property, value, key, `${path}.${key}`)
+        : undefined,
+    )
+  );
+}
+
+function findInadmissibleNestedProperty(
+  property: JsonSchema,
+  parent: Record<string, unknown>,
+  key: string,
+  path: string,
+): string | undefined {
+  const commandKey = property.commandInputFor;
+  if (commandKey === undefined) {
+    return findInadmissibleNestedCommandInput(property, parent[key], path);
+  }
+  const nested = parent[key];
+  // Resolve exactly as the nested reader does, never by matching the raw value:
+  // a step is normalized before it runs, so ` SNAPSHOT ` is no command here and
+  // `snapshot` there — admission would check nothing and the daemon would run it
+  // with the operator paths still aboard. `resolveStructuredBatchCommandName` is
+  // the function the readers themselves call, so the two cannot drift apart.
+  const command = resolveStructuredBatchCommandName(parent[commandKey]);
+  // A name that resolves to nothing leaves nothing to check against, and nothing
+  // to protect: the reader refuses that step, so nothing it carries reaches a
+  // daemon request flag. Reporting it is the reader's job — answering here would
+  // bury the real error under a key complaint.
+  if (command === undefined || !isRecord(nested)) return undefined;
+  const rejection = findInadmissibleInput(command, findCommandMetadata(command), nested);
+  return rejection === undefined ? undefined : `${path}: ${rejection}`;
+}
+
+function firstRejection<TItem>(
+  items: readonly TItem[],
+  check: (item: TItem, index: number) => string | undefined,
+): string | undefined {
+  for (const [index, item] of items.entries()) {
+    const rejection = check(item, index);
+    if (rejection) return rejection;
+  }
+  return undefined;
+}
+
 /**
  * AS-011 answer: the client request envelope from the descriptor registry
  * (timeout-policy, ADR 0008), declared on the tool description so the surface