diff --git a/src/mcp/bridge.ts b/src/mcp/bridge.ts index d271feb..3dab41e 100644 --- a/src/mcp/bridge.ts +++ b/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; + 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 { + 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), + ); + } }); }); diff --git a/src/mcp/tools/run-cloud-test.ts b/src/mcp/tools/run-cloud-test.ts index d099fe3..38ae9c6 100644 --- a/src/mcp/tools/run-cloud-test.ts +++ b/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((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, + testMetadataMap: Record, + 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, }); } diff --git a/test/unit/mcp-bridge.test.ts b/test/unit/mcp-bridge.test.ts index 622cdbd..ead46a9 100644 --- a/test/unit/mcp-bridge.test.ts +++ b/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[]; 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[] }; 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); diff --git a/test/unit/mcp-dry-run.test.ts b/test/unit/mcp-dry-run.test.ts new file mode 100644 index 0000000..d927b2a --- /dev/null +++ b/test/unit/mcp-dry-run.test.ts @@ -0,0 +1,69 @@ +import { expect } from 'chai'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; + +import { dryRunFlows } from '../../src/mcp/tools/run-cloud-test.js'; +import { plan } from '../../src/services/execution-plan.service.js'; +import { buildTestMetadataMap, computeCommonRoot } from '../../src/services/flow-paths.js'; + +/** What dcd_run_cloud_test's dry run lists for a folder, given these filters. */ +async function preview(dir: string, filters: { excludeTags?: string[]; includeTags?: string[] }) { + const executionPlan = await plan({ + excludeTags: filters.excludeTags ?? [], + includeTags: filters.includeTags ?? [], + input: `${dir}/`, + warn() {}, + }); + const commonRoot = computeCommonRoot( + executionPlan.flowsToRun, + executionPlan.referencedFiles, + executionPlan.includedFiles, + ); + const map = buildTestMetadataMap(executionPlan.flowMetadata, commonRoot); + return { + // What `dcd cloud --dry-run` lists. + cli: executionPlan.flowsToRun.map((f) => path.basename(f)).sort(), + mcp: dryRunFlows(executionPlan, map, commonRoot) + .map((f) => path.basename(f.file)) + .sort(), + }; +} + +describe('dcd_run_cloud_test dry run', () => { + let dir: string; + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'dcd-dry-run-')); + const flow = (name: string, tag: string) => + fs.writeFileSync(path.join(dir, name), `appId: x\ntags:\n - ${tag}\n---\n- launchApp\n`); + flow('android-a.yaml', 'android'); + flow('android-b.yaml', 'android'); + flow('ios-a.yaml', 'ios'); + }); + + afterEach(() => fs.rmSync(dir, { force: true, recursive: true })); + + it('lists only the flows the include tags pick, as the CLI does', async () => { + const { cli, mcp } = await preview(dir, { includeTags: ['ios'] }); + expect(mcp).to.deep.equal(['ios-a.yaml']); + expect(mcp).to.deep.equal(cli); + }); + + it('leaves out the flows the exclude tags drop, as the CLI does', async () => { + const { cli, mcp } = await preview(dir, { excludeTags: ['android'] }); + expect(mcp).to.deep.equal(['ios-a.yaml']); + expect(mcp).to.deep.equal(cli); + }); + + it('lists every flow with no filters, with its tags', async () => { + const executionPlan = await plan({ excludeTags: [], includeTags: [], input: `${dir}/`, warn() {} }); + const root = computeCommonRoot(executionPlan.flowsToRun, [], []); + const flows = dryRunFlows(executionPlan, buildTestMetadataMap(executionPlan.flowMetadata, root), root); + expect(flows).to.deep.equal([ + { file: './android-a.yaml', flowName: 'android-a', tags: ['android'] }, + { file: './android-b.yaml', flowName: 'android-b', tags: ['android'] }, + { file: './ios-a.yaml', flowName: 'ios-a', tags: ['ios'] }, + ]); + }); +});