diff --git a/CLAUDE.md b/CLAUDE.md index 4ea6ff6..3cef23c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -53,7 +53,7 @@ Top-level `defineCommand` in `src/index.ts` wires thirteen subcommands (`cloud`, **MCP server.** `src/mcp/` is a third front-end onto the same service layer (a sibling to `src/commands/`), shipped as the `dcd-mcp` bin over stdio transport (`@modelcontextprotocol/sdk`, `zod` schemas). `index.ts` boots the server; `server.ts` registers tools; `context.ts` resolves auth + API URL **lazily**, and again when a `dcd login` session nears expiry (so `tools/list` works unauthenticated, auth errors surface as tool errors rather than a boot crash, and a long-lived server outlasts the one-hour access token) via the same `resolveAuth`/`resolveApiUrl` as the CLI — `DEVICE_CLOUD_API_KEY` env or stored `dcd login` session, with `DCD_API_URL` to override. **Critical invariant: stdout is the JSON-RPC channel** — tools must never call the `src/commands/*` layer or `utils/cli` `logger` (both write to stdout / can `process.exit`); they call services/gateways directly with `logStderr` and return data via `helpers.ts` `jsonResult`/`errorResult`. The `runTool` wrapper records `mcp tool …` telemetry and converts thrown errors to `isError` results. Tools: `dcd_list_devices`, `dcd_list_runs`, `dcd_get_status`, `dcd_download_artifacts` (read-only), and `dcd_run_cloud_test` (billable — gated out by `--read-only` / `DCD_MCP_READONLY=1`, annotated destructive, async-by-default). `dcd_run_cloud_test` reuses `computeCommonRoot`/`buildTestMetadataMap` from `src/services/flow-paths.ts` (extracted from `cloud.ts` so both build identical server-side paths). Registry manifest: `server.json` at repo root. -**`dcd mcp` (the bridge).** A second stdio entry point, `src/mcp/bridge.ts`, run by the `dcd mcp` command. It is a low-level `Server` that routes between two MCP clients: the `dcd_*` tools from the same in-process `createServer()` (over an in-memory transport), and the device tools proxied from the hosted `/mcp`, so those are defined and contract-tested once, in the dcd API. It adds the inputs only a local process can serve: `install_app` takes `app_path` (uploaded with `uploadLocalApp`) and `run` takes `path` (a file is sent as `yaml`, a directory is uploaded as a flow bundle). Credentials are the CLI's (`getContext`): the hosted `/mcp` accepts an API key or a `dcd login` session with `x-dcd-org`, and the remote connection is remade when a session is renewed. If the hosted endpoint can't be reached, it serves the `dcd_*` tools only and says why on stderr. Read-only mode keeps only device tools annotated `readOnlyHint`. `dcd-mcp` itself is unchanged. +**`dcd mcp` (the bridge).** A second stdio entry point, `src/mcp/bridge.ts`, run by the `dcd mcp` command. It is a low-level `Server` that routes between two MCP clients: the `dcd_*` tools from the same in-process `createServer()` (over an in-memory transport), and the device tools proxied from the hosted `/mcp`, so those are defined and contract-tested once, in the dcd API. It adds the inputs only a local process can serve: `install_app` takes `app_path` (uploaded with `uploadLocalApp`) and `run` takes `path` (a file is sent as `yaml`, a directory is uploaded as a flow bundle). Credentials are the CLI's (`getContext`): the hosted `/mcp` accepts an API key or a `dcd login` session with `x-dcd-org`. One connection is shared, and its `fetch` re-reads the auth headers from `getContext()` on every request, so a renewed session is picked up without remaking the connection (which would close it under a `run` still going). If the hosted endpoint can't be reached, it serves the `dcd_*` tools only and says why on stderr. Read-only mode keeps only device tools annotated `readOnlyHint`. `dcd-mcp` itself is unchanged. ## Contributing diff --git a/src/commands/device.ts b/src/commands/device.ts index 1e7f265..43a371a 100644 --- a/src/commands/device.ts +++ b/src/commands/device.ts @@ -1041,7 +1041,7 @@ const openSub = defineCommand({ const id = await resolveSessionId(ctx.apiUrl, ctx.auth, args.id as string | undefined); const { session } = await DeviceGateway.status(ctx.apiUrl, ctx.auth, id); // Only a person at a terminal gets a browser; a script gets the URL. - const opened = interactive(json) && openBrowser(session.monitor_url); + const opened = interactive(json) && (await openBrowser(session.monitor_url)); return { session, data: { url: session.monitor_url, opened }, diff --git a/src/commands/login.ts b/src/commands/login.ts index 611eef6..553cf2b 100644 --- a/src/commands/login.ts +++ b/src/commands/login.ts @@ -171,7 +171,7 @@ export const loginCommand = defineCommand({ if (noBrowser) { rows.push(ui.note('Open this URL in a browser to finish login:'), colors.highlight(loginUrl)); } else { - const opened = openBrowser(loginUrl); + const opened = await openBrowser(loginUrl); rows.push( ui.note( opened diff --git a/src/mcp/bridge.ts b/src/mcp/bridge.ts index cd34889..636e475 100644 --- a/src/mcp/bridge.ts +++ b/src/mcp/bridge.ts @@ -10,8 +10,8 @@ * a bundle). * * Auth is the CLI's: an API key or a `dcd login` session (getContext). The - * hosted endpoint accepts both. A session is renewed before it expires, and - * the remote connection is remade with it. + * hosted endpoint accepts both. A session is renewed before it expires; the + * one shared connection re-reads the auth on every request (RemoteDevices). * * stdout is the JSON-RPC channel: everything else goes to stderr. */ diff --git a/src/utils/browser.ts b/src/utils/browser.ts index 627cae9..93f51c1 100644 --- a/src/utils/browser.ts +++ b/src/utils/browser.ts @@ -2,14 +2,14 @@ import { spawn } from 'node:child_process'; /** * Open `url` in the default browser. False if it isn't an http(s) URL or no - * opener could be started. + * opener could be started (none installed, as on a headless box). * * 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 { +export async function openBrowser(url: string): Promise { let href: string; try { const parsed = new URL(url); @@ -25,11 +25,25 @@ export function openBrowser(url: string): boolean { : process.platform === 'win32' ? { 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(); - return true; - } catch { - return false; - } + return startOpener(cmd.bin, cmd.args); +} + +/** + * Start `bin` detached. False if it couldn't be started. A missing program is + * reported as an 'error' event, not thrown, and with no listener it would + * crash the process, so this waits for 'spawn' or 'error' to know. + */ +export function startOpener(bin: string, args: string[]): Promise { + return new Promise((resolve) => { + try { + const child = spawn(bin, args, { detached: true, stdio: 'ignore' }); + child.once('spawn', () => { + child.unref(); + resolve(true); + }); + child.once('error', () => resolve(false)); + } catch { + resolve(false); + } + }); } diff --git a/test/unit/browser.test.ts b/test/unit/browser.test.ts index 0934735..e13e000 100644 --- a/test/unit/browser.test.ts +++ b/test/unit/browser.test.ts @@ -1,11 +1,16 @@ import { expect } from 'chai'; -import { openBrowser } from '../../src/utils/browser.js'; +import { openBrowser, startOpener } 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); + it('opens nothing that is not an http(s) URL', async () => { + expect(await openBrowser('file:///etc/passwd')).to.equal(false); + expect(await openBrowser('--help')).to.equal(false); + expect(await openBrowser('not a url')).to.equal(false); + }); + + it('returns false, rather than crashing, when the opener is not installed', async () => { + // As with xdg-open on a headless box: spawn emits ENOENT as an event. + expect(await startOpener('dcd-test-no-such-opener', ['https://example.com'])).to.equal(false); }); });