fix(mcp): dry runs honour tags, single flow files bring their subflows, failed starts say what to check

src/mcp/bridge.ts

@@ -16,6 +16,7 @@
  * stdout is the JSON-RPC channel: everything else goes to stderr.
  */
 import { readFile, stat } from 'node:fs/promises';
+import * as path from 'node:path';
 
 import { Client } from '@modelcontextprotocol/sdk/client/index.js';
 import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js';
@@ -28,7 +29,11 @@ import {
   type Tool,
 } from '@modelcontextprotocol/sdk/types.js';
 
+import { DeviceGateway } from '../gateways/device-gateway.js';
+import { processDependencies, readTestYamlFileAsJson } from '../services/execution-plan.utils.js';
+import { computeCommonRoot } from '../services/flow-paths.js';
 import type { AuthContext } from '../types/domain/auth.types.js';
+import type { DeviceSession } from '../types/domain/device-session.types.js';
 import { uploadLocalApp } from '../utils/app-upload.js';
 import { getCliVersion } from '../utils/cli.js';
 import { uploadFlowBundle } from '../utils/device-bundle.js';
@@ -63,7 +68,7 @@ const LOCAL_INPUTS: Record<string, { description: string; properties: Record<str
   },
   run: {
     description:
-      ' Or pass path: a flow file on this machine (sent as yaml), or a directory of flows (flow picks the entry file inside it). Only the YAML files and the files the flows use (scripts, media) are uploaded, not builds or anything else in the directory.',
+      ' Or pass path: a flow file on this machine, or a directory of flows (flow picks the entry file inside it). A file that uses other files (runFlow, runScript, addMedia and the like) is uploaded with its folder; one that uses none is sent as yaml. Only the YAML files and the files the flows use (scripts, media) are uploaded, not builds or anything else in the directory.',
     properties: {
       path: {
         type: 'string',
@@ -87,6 +92,30 @@ export function withLocalInputs(tool: Tool): Tool {
   };
 }
 
+/**
+ * For a single flow file that uses other files (runFlow, runScript, addMedia
+ * and the like): the folder to upload as a bundle, and the file's path
+ * inside it. Sent as yaml on its own, the flow would fail on the device with
+ * an invalid file path, because the files it uses would not be there. Null
+ * when the flow uses no other files, or can't be read (the device reports
+ * that itself).
+ */
+export function flowFileBundle(file: string): { dir: string; entry: string } | null {
+  const absolute = path.resolve(file);
+  let deps: ReturnType<typeof processDependencies>;
+  try {
+    const { config, testSteps } = readTestYamlFileAsJson(absolute);
+    deps = processDependencies({ config, input: absolute, testSteps });
+  } catch {
+    return null;
+  }
+
+  // A missing file goes this way too, so the upload names it.
+  if (deps.allFiles.length === 0 && deps.allErrors.length === 0) return null;
+  const dir = computeCommonRoot([absolute], deps.allFiles) || path.dirname(absolute);
+  return { dir, entry: path.relative(dir, absolute) };
+}
+
 /**
  * Do the local part of a call (uploads, reading a file) and return the
  * arguments the hosted tool takes.
@@ -123,12 +152,56 @@ export async function resolveLocalInputs(
       return { ...rest, flow_upload_id: bundle.id };
     }
 
+    const bundled = flowFileBundle(flowPath as string);
+    if (bundled) {
+      logStderr(`${flowPath} uses other files; uploading it with ${bundled.dir}…`);
+      const bundle = await uploadFlowBundle({ ...ctx, ...bundled, log: logStderr });
+      return { ...rest, flow: bundled.entry, flow_upload_id: bundle.id };
+    }
+
     return { ...rest, yaml: await readFile(flowPath as string, 'utf8') };
   }
 
   return args;
 }
 
+/** How recent an open session must be to count as one a failed start may have made. */
+export const START_RECOVERY_WINDOW_MS = 2 * 60 * 1000;
+
+/**
+ * What to tell the agent when a start_device call failed without an answer
+ * (the connection dropped, or the server never replied). The request may
+ * still have started a session, which is billed while it runs, and an agent
+ * that just retries would end up with two. So list the open sessions started
+ * in the last couple of minutes, and say to check before starting again.
+ */
+export async function explainFailedStart(
+  message: string,
+  ctx: { apiUrl: string; auth: AuthContext },
+  now = Date.now(),
+): Promise<string> {
+  const head = `start_device failed: ${message}. The request may still have started a session, and a session is billed while it runs.`;
+  let recent: DeviceSession[];
+  try {
+    const { sessions } = await DeviceGateway.list(ctx.apiUrl, ctx.auth);
+    recent = sessions.filter((s) => now - Date.parse(s.created_at) <= START_RECOVERY_WINDOW_MS);
+  } catch {
+    return `${head} Call list_devices to check for one before you call start_device again.`;
+  }
+
+  if (recent.length === 0) {
+    return `${head} No session was started in the last ${START_RECOVERY_WINDOW_MS / 60_000} minutes, so it is safe to call start_device again.`;
+  }
+
+  const listed = recent.map((s) => ({
+    client_label: s.client_label,
+    created_at: s.created_at,
+    id: s.id,
+    state: s.state,
+  }));
+  return `${head} Open sessions started in the last ${START_RECOVERY_WINDOW_MS / 60_000} minutes: ${JSON.stringify(listed)}. If one is yours, use it (get_device_status with wait: true until it is ready) instead of starting another. Otherwise stop it with stop_device. Check list_devices before you call start_device again.`;
+}
+
 /**
  * The hosted device tools, over one connection made on first use and shared
  * by every call.
@@ -247,8 +320,18 @@ export async function createBridgeServer(
         return errorResult(`${name} is not available in read-only mode.`);
       }
       const client = await remote.connect();
-      const remoteArgs = await resolveLocalInputs(name, args, await getContext());
-      return (await client.callTool({ name, arguments: remoteArgs }, undefined, options)) as CallToolResult;
+      const ctx = await getContext();
+      const remoteArgs = await resolveLocalInputs(name, args, ctx);
+      try {
+        return (await client.callTool({ name, arguments: remoteArgs }, undefined, options)) as CallToolResult;
+      } catch (error) {
+        // A tool's own refusal comes back as a result. A throw means no answer
+        // arrived, so for a start the session's fate is unknown.
+        if (name !== 'start_device' || extra.signal.aborted) throw error;
+        return errorResult(
+          await explainFailedStart(error instanceof Error ? error.message : String(error), ctx),
+        );
+      }
     });
   });
 

src/mcp/tools/run-cloud-test.ts

@@ -4,12 +4,16 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
 import { z } from 'zod';
 
 import { ApiGateway } from '../../gateways/api-gateway.js';
-import { plan } from '../../services/execution-plan.service.js';
+import { type IExecutionPlan, plan } from '../../services/execution-plan.service.js';
 import {
   isSubmitEndpointMissing,
   submitWithRetry,
 } from '../../services/flow-submit.service.js';
-import { computeCommonRoot, buildTestMetadataMap } from '../../services/flow-paths.js';
+import {
+  computeCommonRoot,
+  buildTestMetadataMap,
+  type FlowMetadataEntry,
+} from '../../services/flow-paths.js';
 import { DeviceValidationService } from '../../services/device-validation.service.js';
 import { platformFromAppFile } from '../../services/notices.service.js';
 import type { EncryptionTelemetry } from '../../services/telemetry.service.js';
@@ -20,6 +24,7 @@ import { refreshAuth } from '../../utils/auth.js';
 import { getCliVersion } from '../../utils/cli.js';
 import { fetchCompatibilityData } from '../../utils/compatibility.js';
 import { checkKekForEncryption, resolveEncryption } from '../../utils/envelope.js';
+import { toPortableRelativePath } from '../../utils/paths.js';
 import { getConsoleUrl } from '../../utils/styling.js';
 import { getContext, logStderr } from '../context.js';
 import { jsonResult, runTool } from '../helpers.js';
@@ -31,6 +36,24 @@ const TERMINAL_STATUSES = new Set(['PASSED', 'FAILED', 'CANCELLED', 'ERROR']);
 const sleep = (ms: number) =>
   new Promise<void>((resolve) => setTimeout(resolve, ms));
 
+/**
+ * The flows a dry run lists: the ones a real run would submit, after the
+ * include/exclude tags and excludeFlows, as `dcd cloud --dry-run` lists
+ * them. `flowMetadata` holds every flow in the folder, filtered out or not,
+ * so it can't be the list.
+ */
+export function dryRunFlows(
+  executionPlan: Pick<IExecutionPlan, 'flowsToRun' | 'sequence'>,
+  testMetadataMap: Record<string, FlowMetadataEntry>,
+  commonRoot: string,
+): { file: string; flowName: string; tags: string[] }[] {
+  return [...executionPlan.flowsToRun, ...(executionPlan.sequence?.flows ?? [])].map((absolute) => {
+    const file = toPortableRelativePath(absolute, commonRoot);
+    const meta = testMetadataMap[file];
+    return { file, flowName: meta?.flowName ?? path.parse(absolute).name, tags: meta?.tags ?? [] };
+  });
+}
+
 /**
  * Submit a Maestro flow (or directory of flows) to devicecloud.dev.
  *
@@ -198,11 +221,7 @@ export function registerRunCloudTest(server: McpServer): void {
         if (args.dryRun) {
           return jsonResult({
             dryRun: true,
-            flows: Object.entries(testMetadataMap).map(([file, meta]) => ({
-              file,
-              flowName: meta.flowName,
-              tags: meta.tags,
-            })),
+            flows: dryRunFlows(executionPlan, testMetadataMap, commonRoot),
             sequentialFlowCount: executionPlan.sequence?.flows.length ?? 0,
           });
         }

test/unit/mcp-bridge.test.ts

@@ -11,7 +11,13 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
 import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js';
 import { z } from 'zod';
 
-import { createBridgeServer, resolveLocalInputs, withLocalInputs } from '../../src/mcp/bridge.js';
+import {
+  createBridgeServer,
+  explainFailedStart,
+  flowFileBundle,
+  resolveLocalInputs,
+  withLocalInputs,
+} from '../../src/mcp/bridge.js';
 import { clearContextCache } from '../../src/mcp/context.js';
 
 /**
@@ -20,13 +26,21 @@ import { clearContextCache } from '../../src/mcp/context.js';
  */
 function startHosted(): Promise<{
   calls: { args: unknown; auth?: string; name: string }[];
+  failStart: boolean;
   http: HttpServer;
   inits: number;
+  sessions: Record<string, unknown>[];
   url: string;
 }> {
   const calls: { args: unknown; auth?: string; name: string }[] = [];
-  const hosted = { calls, inits: 0 };
+  const hosted = { calls, failStart: false, inits: 0, sessions: [] as Record<string, unknown>[] };
   const http = createHttpServer(async (req, res) => {
+    // The open sessions, as GET /device/sessions answers.
+    if (req.method === 'GET' && req.url === '/device/sessions') {
+      res.writeHead(200, { 'content-type': 'application/json' }).end(JSON.stringify({ sessions: hosted.sessions }));
+      return;
+    }
+
     // Stateless, as the API: no GET stream, no session to delete.
     if (req.method !== 'POST') {
       res.writeHead(405).end();
@@ -37,6 +51,11 @@ function startHosted(): Promise<{
     for await (const c of req) chunks.push(c as Buffer);
     const body = JSON.parse(Buffer.concat(chunks).toString() || '{}');
     if (body.method === 'initialize') hosted.inits++;
+    // A start whose answer never arrives, as when a proxy gives up on it.
+    if (hosted.failStart && body.method === 'tools/call' && body.params?.name === 'start_device') {
+      res.writeHead(502, { 'content-type': 'application/json' }).end('{"message":"Application failed to respond"}');
+      return;
+    }
     const auth = req.headers['x-app-api-key'] as string | undefined;
     const server = new McpServer({ name: 'hosted', version: '1' });
     const record = (name: string) => async (args: unknown) => {
@@ -64,6 +83,11 @@ function startHosted(): Promise<{
         return { content: [{ type: 'text' as const, text: 'late' }] };
       },
     );
+    server.registerTool(
+      'start_device',
+      { description: 'Start a device.', inputSchema: { platform: z.string() } },
+      record('start_device'),
+    );
     server.registerTool(
       'inspect_screen',
       { description: 'Inspect.', inputSchema: {}, annotations: { readOnlyHint: true } },
@@ -181,6 +205,34 @@ describe('dcd mcp bridge', () => {
     await client.close();
   });
 
+  it('says a failed start may have made a session, and lists the recent ones', async () => {
+    hosted.failStart = true;
+    const now = Date.now();
+    hosted.sessions = [
+      { client_label: 'claude-code', created_at: new Date(now - 30_000).toISOString(), id: 'new-one', state: 'provisioning' },
+      { client_label: 'other', created_at: new Date(now - 30 * 60_000).toISOString(), id: 'old-one', state: 'active' },
+    ];
+    const client = await connect();
+    await client.listTools();
+    const result = await client.callTool({ name: 'start_device', arguments: { platform: 'ios' } });
+    expect(result.isError).to.equal(true);
+    const text = JSON.stringify(result.content);
+    expect(text).to.include('may still have started a session');
+    expect(text).to.include('new-one');
+    expect(text).to.not.include('old-one');
+    expect(text).to.include('list_devices');
+    await client.close();
+  });
+
+  it('leaves a start that answered alone', async () => {
+    const client = await connect();
+    await client.listTools();
+    const result = await client.callTool({ name: 'start_device', arguments: { platform: 'ios' } });
+    expect(result.isError).to.not.equal(true);
+    expect(hosted.calls.map((c) => c.name)).to.deep.equal(['start_device']);
+    await client.close();
+  });
+
   it('read-only mode keeps only device tools that change nothing', async () => {
     process.env.DCD_MCP_READONLY = '1';
     const client = await connect();
@@ -211,6 +263,49 @@ describe('dcd mcp bridge', () => {
   });
 
   describe('helpers', () => {
+    it('explainFailedStart says it is safe to retry when no session started', async () => {
+      hosted.sessions = [];
+      const ctx = { apiUrl: hosted.url, auth: { headers: {}, mode: 'apiKey' as const } };
+      const text = await explainFailedStart('fetch failed', ctx);
+      expect(text).to.include('safe to call start_device again');
+    });
+
+    it('explainFailedStart still warns when the sessions cannot be read', async () => {
+      const ctx = { apiUrl: 'http://127.0.0.1:9', auth: { headers: {}, mode: 'apiKey' as const } };
+      const text = await explainFailedStart('fetch failed', ctx);
+      expect(text).to.include('Call list_devices to check');
+    });
+
+    it('flowFileBundle sends a flow that uses no other files as yaml', () => {
+      const flow = path.join(tempDir, 'plain.yaml');
+      fs.writeFileSync(flow, 'appId: x\n---\n- launchApp\n- tapOn: OK\n');
+      expect(flowFileBundle(flow)).to.equal(null);
+    });
+
+    it('flowFileBundle uploads the folder of a flow that runs a subflow or a script', () => {
+      fs.mkdirSync(path.join(tempDir, 'flows', 'subflows'), { recursive: true });
+      fs.writeFileSync(path.join(tempDir, 'flows', 'subflows', 'login.yaml'), 'appId: x\n---\n- tapOn: Log in\n');
+      fs.writeFileSync(path.join(tempDir, 'flows', 'setup.js'), 'output.ok = true\n');
+      const flow = path.join(tempDir, 'flows', 'main.yaml');
+      fs.writeFileSync(flow, 'appId: x\n---\n- runFlow: subflows/login.yaml\n- runScript: setup.js\n');
+      expect(flowFileBundle(flow)).to.deep.equal({ dir: path.join(tempDir, 'flows'), entry: 'main.yaml' });
+    });
+
+    it('flowFileBundle reaches up to a shared folder the flow uses', () => {
+      fs.mkdirSync(path.join(tempDir, 'shared'), { recursive: true });
+      fs.mkdirSync(path.join(tempDir, 'ios'), { recursive: true });
+      fs.writeFileSync(path.join(tempDir, 'shared', 'login.yaml'), 'appId: x\n---\n- tapOn: Log in\n');
+      const flow = path.join(tempDir, 'ios', 'main.yaml');
+      fs.writeFileSync(flow, 'appId: x\n---\n- runFlow: ../shared/login.yaml\n');
+      expect(flowFileBundle(flow)).to.deep.equal({ dir: tempDir, entry: path.join('ios', 'main.yaml') });
+    });
+
+    it('flowFileBundle still bundles a flow whose subflow is missing, so the upload names it', () => {
+      const flow = path.join(tempDir, 'broken.yaml');
+      fs.writeFileSync(flow, 'appId: x\n---\n- runFlow: subflows/missing.yaml\n');
+      expect(flowFileBundle(flow)).to.deep.equal({ dir: tempDir, entry: 'broken.yaml' });
+    });
+
     it('withLocalInputs adds the local inputs and leaves other tools alone', () => {
       const tool = { name: 'run', description: 'Run.', inputSchema: { type: 'object' as const, properties: { yaml: {} } } };
       const patched = withLocalInputs(tool);