diff --git a/src/mcp/bridge.ts b/src/mcp/bridge.ts index a716a7b..cd34889 100644 --- a/src/mcp/bridge.ts +++ b/src/mcp/bridge.ts @@ -124,31 +124,48 @@ export async function resolveLocalInputs( return args; } -/** The hosted device tools, connected lazily and remade when auth changes. */ +/** + * The hosted device tools, over one connection made on first use and shared + * by every call. + * + * Auth goes on each request, not on the connection. Remaking the connection + * when a session is renewed would close it under calls still running (a `run` + * can take minutes), and they would fail with "Connection closed" while the + * flow carried on. The hosted /mcp is stateless, so nothing ties the + * connection to the auth it started with. + */ class RemoteDevices { - private client: Client | null = null; - private auth: AuthContext | null = null; + private connecting: Promise | null = null; private tools: Tool[] | null = null; - async connect(): Promise<{ client: Client; ctx: { apiUrl: string; auth: AuthContext } }> { - const ctx = await getContext(); - if (this.client && this.auth === ctx.auth) return { client: this.client, ctx }; + /** Calls that arrive together share one connect. A failed one is retried by the next call. */ + connect(): Promise { + this.connecting ??= this.open().catch((error: unknown) => { + this.connecting = null; + throw error; + }); + return this.connecting; + } - await this.client?.close().catch(() => {}); + private async open(): Promise { + const { apiUrl } = await getContext(); const client = new Client({ name: 'dcd-mcp-bridge', version: getCliVersion() }); await client.connect( - new StreamableHTTPClientTransport(new URL(`${ctx.apiUrl}/mcp`), { - requestInit: { headers: { ...ctx.auth.headers } }, + new StreamableHTTPClientTransport(new URL(`${apiUrl}/mcp`), { + fetch: async (url, init) => { + const { auth } = await getContext(); + const headers = new Headers(init?.headers); + for (const [key, value] of Object.entries(auth.headers)) headers.set(key, value); + return fetch(url, { ...init, headers }); + }, }), ); - this.client = client; - this.auth = ctx.auth; - return { client, ctx }; + return client; } async list(): Promise { if (this.tools) return this.tools; - const { client } = await this.connect(); + const client = await this.connect(); const { tools } = await client.listTools(); this.tools = tools; return tools; @@ -224,8 +241,8 @@ export async function createBridgeServer( if (readOnly && !remote.isReadOnly(name)) { return errorResult(`${name} is not available in read-only mode.`); } - const { client, ctx } = await remote.connect(); - const remoteArgs = await resolveLocalInputs(name, args, ctx); + const client = await remote.connect(); + const remoteArgs = await resolveLocalInputs(name, args, await getContext()); return (await client.callTool({ name, arguments: remoteArgs }, undefined, options)) as CallToolResult; }); }); diff --git a/src/utils/browser.ts b/src/utils/browser.ts index e196034..627cae9 100644 --- a/src/utils/browser.ts +++ b/src/utils/browser.ts @@ -1,13 +1,30 @@ import { spawn } from 'node:child_process'; -/** Open `url` in the default browser. False if no opener could be started. */ +/** + * Open `url` in the default browser. False if it isn't an http(s) URL or no + * opener could be started. + * + * Only http(s) is opened, so a URL can't name a local file or start with `-` + * and be read as an option. No opener goes through a shell: Windows uses + * rundll32, because `cmd /c start` parses the URL again and treats its `&` + * as a command separator. + */ export function openBrowser(url: string): boolean { + let href: string; + try { + const parsed = new URL(url); + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') return false; + href = parsed.href; + } catch { + return false; + } + const cmd = process.platform === 'darwin' - ? { bin: 'open', args: [url] } + ? { bin: 'open', args: [href] } : process.platform === 'win32' - ? { bin: 'cmd', args: ['/c', 'start', '""', url] } - : { bin: 'xdg-open', args: [url] }; + ? { bin: 'rundll32', args: ['url.dll,FileProtocolHandler', href] } + : { bin: 'xdg-open', args: [href] }; try { const child = spawn(cmd.bin, cmd.args, { detached: true, stdio: 'ignore' }); child.unref(); diff --git a/test/unit/browser.test.ts b/test/unit/browser.test.ts new file mode 100644 index 0000000..0934735 --- /dev/null +++ b/test/unit/browser.test.ts @@ -0,0 +1,11 @@ +import { expect } from 'chai'; + +import { openBrowser } from '../../src/utils/browser.js'; + +describe('openBrowser', () => { + it('opens nothing that is not an http(s) URL', () => { + expect(openBrowser('file:///etc/passwd')).to.equal(false); + expect(openBrowser('--help')).to.equal(false); + expect(openBrowser('not a url')).to.equal(false); + }); +}); diff --git a/test/unit/mcp-bridge.test.ts b/test/unit/mcp-bridge.test.ts index 32e2d2a..622cdbd 100644 --- a/test/unit/mcp-bridge.test.ts +++ b/test/unit/mcp-bridge.test.ts @@ -18,8 +18,14 @@ import { clearContextCache } from '../../src/mcp/context.js'; * A stand-in for the hosted /mcp: stateless Streamable HTTP, one server per * POST, as the API runs it. Records each call and the auth header it saw. */ -function startHosted(): Promise<{ calls: { args: unknown; auth?: string; name: string }[]; http: HttpServer; url: string }> { +function startHosted(): Promise<{ + calls: { args: unknown; auth?: string; name: string }[]; + http: HttpServer; + inits: number; + url: string; +}> { const calls: { args: unknown; auth?: string; name: string }[] = []; + const hosted = { calls, inits: 0 }; const http = createHttpServer(async (req, res) => { // Stateless, as the API: no GET stream, no session to delete. if (req.method !== 'POST') { @@ -30,6 +36,7 @@ function startHosted(): Promise<{ calls: { args: unknown; auth?: string; name: s const chunks: Buffer[] = []; 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++; 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) => { @@ -69,7 +76,7 @@ function startHosted(): Promise<{ calls: { args: unknown; auth?: string; name: s return new Promise((resolve) => { http.listen(0, '127.0.0.1', () => { const { port } = http.address() as AddressInfo; - resolve({ calls, http, url: `http://127.0.0.1:${port}` }); + resolve(Object.assign(hosted, { http, url: `http://127.0.0.1:${port}` })); }); }); } @@ -127,6 +134,30 @@ describe('dcd mcp bridge', () => { await client.close(); }); + it('keeps a running call when the session is renewed, and sends the new auth', async () => { + const client = await connect(); + await client.listTools(); + const slow = client.callTool({ name: 'get_logs', arguments: { ms: 300 } }); + await new Promise((r) => setTimeout(r, 50)); + // As if a `dcd login` session were renewed: getContext resolves new auth. + process.env.DEVICE_CLOUD_API_KEY = 'renewed-key'; + clearContextCache(); + await client.callTool({ name: 'inspect_screen', arguments: {} }); + const result = await slow; + expect(result.isError).to.not.equal(true); + expect(JSON.stringify(result.content)).to.include('late'); + expect(hosted.calls).to.deep.equal([{ name: 'inspect_screen', args: {}, auth: 'renewed-key' }]); + expect(hosted.inits).to.equal(1); + await client.close(); + }); + + it('shares one connection between calls that arrive together', async () => { + const client = await connect(); + await Promise.all([client.listTools(), client.listTools(), client.callTool({ name: 'inspect_screen', arguments: {} })]); + expect(hosted.inits).to.equal(1); + await client.close(); + }); + it("applies the bridge's call timeout, not the SDK's 60s default", async () => { // The agent's own client allows long calls; only the bridge's limit applies. const client = await connect(300);