fix(mcp): make the HTTP fallback work and stop it masking bridge errors

CHANGELOG.md

@@ -29,6 +29,13 @@ evals.
 
 ### Fixed
 
+- The MCP server's HTTP fallback, which never worked: it sent the extension's
+  request types (`browser.snapshot`) to an API that takes tool names
+  (`browser_snapshot`), so with the socket unavailable every tool came back
+  "Unknown tool". It now uses the routes the bridge has. The fallback is also
+  taken only when the socket cannot be opened; an error the bridge returned is
+  reported as it is, where it used to be replaced by the failed retry's, and a
+  socket timeout no longer sends the same click or keystroke a second time.
 - Page content could break out of the block that marks it as untrusted. The
   observation fence used fixed, public markers, so a page that printed the
   closing marker in its own text had everything after it read as instruction

scripts/check.sh

@@ -5,6 +5,7 @@ ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
 
 node "$ROOT/scripts/eval-fixtures.mjs"
 node "$ROOT/scripts/bridge-smoke.mjs"
+node "$ROOT/scripts/mcp-smoke.mjs"
 
 cd "$ROOT/core"
 npm run typecheck

scripts/mcp-smoke.mjs

@@ -0,0 +1,186 @@
+#!/usr/bin/env node
+// The MCP server reaches the bridge over a Unix socket and falls back to the
+// bridge's HTTP API when the socket cannot be opened. This drives the real
+// mcp-server.js over stdio and checks both halves of that arrangement.
+import assert from 'node:assert/strict';
+import { once } from 'node:events';
+import { spawn } from 'node:child_process';
+import http from 'node:http';
+import readline from 'node:readline';
+
+const root = new URL('..', import.meta.url);
+const port = 21000 + Math.floor(Math.random() * 1000);
+const token = 'test-token';
+const socketPath = `/tmp/weboperator-mcp-smoke-${process.pid}.sock`;
+const noSocket = `/tmp/weboperator-mcp-smoke-${process.pid}-absent.sock`;
+
+const TOOL_CALLS = [
+  ['browser_snapshot', {}],
+  ['browser_navigate', { url: 'https://example.com' }],
+  ['browser_click', { index: 1 }],
+  ['browser_type', { index: 1, text: 'hello' }],
+  ['browser_press', { key: 'Enter' }],
+  ['browser_scroll', { direction: 'down' }],
+  ['browser_screenshot', {}],
+  ['browser_extract', { instruction: 'the title' }],
+  ['browser_solve_captcha', { type: 'auto' }],
+  ['weboperator_execute_goal', { goal: 'read the page' }],
+];
+
+// A real bridge with no extension attached: every request it understands
+// fails the same way, and one it does not understand fails differently.
+const bridge = spawn(process.execPath, ['weboperator-bridge/bridge.js'], {
+  cwd: root,
+  env: {
+    ...process.env,
+    WEBOPERATOR_BRIDGE_PORT: String(port),
+    WEBOPERATOR_AGENT_SOCKET: socketPath,
+    WEBOPERATOR_BRIDGE_LOG: '/tmp/weboperator-mcp-smoke.log',
+    WEBOPERATOR_API_TOKEN: token,
+  },
+  stdio: ['ignore', 'ignore', 'inherit'],
+});
+
+// Stands in for the bridge's HTTP API where the routes themselves are the
+// thing under test, and records what it was asked.
+const seen = [];
+const fake = http.createServer((req, res) => {
+  let raw = '';
+  req.on('data', (chunk) => { raw += chunk; });
+  req.on('end', () => {
+    const body = raw ? JSON.parse(raw) : {};
+    seen.push({ method: req.method, path: req.url, body });
+    res.setHeader('content-type', 'application/json');
+    if (req.url === '/v1/tasks') return res.end(JSON.stringify({ id: 'task 1' }));
+    if (req.url === '/v1/tasks/task%201/wait') {
+      return res.end(JSON.stringify({ id: 'task 1', status: 'completed', steps: [] }));
+    }
+    if (req.url === '/v1/tools/call') return res.end(JSON.stringify({ ok: true, tool: body.tool }));
+    res.statusCode = 404;
+    return res.end(JSON.stringify({ error: `Unknown endpoint: ${req.method} ${req.url}` }));
+  });
+});
+fake.listen(0, '127.0.0.1');
+await once(fake, 'listening');
+const fakePort = fake.address().port;
+
+const servers = [];
+
+try {
+  await waitForServer(`http://127.0.0.1:${port}/health`);
+
+  // 1. No socket, real bridge: every tool has to arrive as a request the
+  //    bridge recognises. The fallback used to send `browser.snapshot` where
+  //    the HTTP API takes `browser_snapshot`, so all ten came back
+  //    "Unknown tool" and the fallback could never work.
+  const overHttp = startMcp({ socket: noSocket, httpPort: port });
+  await overHttp.request('initialize', {});
+  const listed = await overHttp.request('tools/list', {});
+  assert.deepEqual(listed.tools.map((tool) => tool.name), TOOL_CALLS.map(([name]) => name));
+  for (const [name, args] of TOOL_CALLS) {
+    const result = await overHttp.request('tools/call', { name, arguments: args });
+    const text = result.content[0].text;
+    assert.equal(result.isError, true, `${name} cannot succeed without an extension`);
+    assert.match(text, /extension is not connected/i, `${name} over HTTP: ${text}`);
+  }
+
+  // 2. No socket, recording server: a goal is two requests, and neither is
+  //    a tool call.
+  const recorded = startMcp({ socket: noSocket, httpPort: fakePort });
+  await recorded.request('initialize', {});
+  const goal = await recorded.request('tools/call', {
+    name: 'weboperator_execute_goal', arguments: { goal: 'read the page', timeoutMs: 5000 },
+  });
+  assert.notEqual(goal.isError, true, goal.content[0].text);
+  assert.deepEqual(seen.map((entry) => `${entry.method} ${entry.path}`), [
+    'POST /v1/tasks',
+    'POST /v1/tasks/task%201/wait',
+  ]);
+  assert.deepEqual(seen[0].body, { goal: 'read the page', timeoutMs: 5000 });
+  assert.deepEqual(seen[1].body, { timeoutMs: 5000 });
+
+  seen.length = 0;
+  const click = await recorded.request('tools/call', {
+    name: 'browser_click', arguments: { index: 3 },
+  });
+  assert.notEqual(click.isError, true, click.content[0].text);
+  assert.equal(seen.length, 1);
+  assert.equal(seen[0].path, '/v1/tools/call');
+  assert.equal(seen[0].body.tool, 'browser_click');
+  assert.equal(seen[0].body.arguments.index, 3);
+
+  // 3. Socket open, bridge answers with an error: that error is the result.
+  //    Falling back here replaced it with the second attempt's, and after a
+  //    timeout would have run the action twice.
+  seen.length = 0;
+  const overSocket = startMcp({ socket: socketPath, httpPort: fakePort });
+  await overSocket.request('initialize', {});
+  const refused = await overSocket.request('tools/call', { name: 'browser_click', arguments: { index: 1 } });
+  assert.equal(refused.isError, true);
+  assert.match(refused.content[0].text, /extension is not connected/i);
+  assert.equal(seen.length, 0, 'an error from the bridge must not be retried over HTTP');
+
+  console.log('MCP smoke tests passed');
+} finally {
+  for (const server of servers) await stopChild(server.child);
+  await stopChild(bridge);
+  fake.close();
+}
+
+function startMcp({ socket, httpPort }) {
+  const child = spawn(process.execPath, ['weboperator-bridge/mcp-server.js'], {
+    cwd: root,
+    env: {
+      ...process.env,
+      WEBOPERATOR_AGENT_SOCKET: socket,
+      WEBOPERATOR_BRIDGE_PORT: String(httpPort),
+      WEBOPERATOR_API_TOKEN: token,
+    },
+    stdio: ['pipe', 'pipe', 'inherit'],
+  });
+  const waiting = new Map();
+  let nextId = 1;
+  readline.createInterface({ input: child.stdout }).on('line', (line) => {
+    const message = JSON.parse(line);
+    const settle = waiting.get(message.id);
+    if (!settle) return;
+    waiting.delete(message.id);
+    if (message.error) settle.reject(new Error(message.error.message));
+    else settle.resolve(message.result);
+  });
+  const server = {
+    child,
+    request(method, params) {
+      const id = nextId++;
+      return new Promise((resolve, reject) => {
+        const timer = setTimeout(() => reject(new Error(`No answer to ${method}`)), 15_000);
+        waiting.set(id, {
+          resolve: (value) => { clearTimeout(timer); resolve(value); },
+          reject: (err) => { clearTimeout(timer); reject(err); },
+        });
+        child.stdin.write(`${JSON.stringify({ jsonrpc: '2.0', id, method, params })}\n`);
+      });
+    },
+  };
+  servers.push(server);
+  return server;
+}
+
+async function waitForServer(url) {
+  const started = Date.now();
+  while (Date.now() - started < 5000) {
+    try {
+      const res = await fetch(url);
+      if (res.ok) return;
+    } catch {}
+    await new Promise((resolve) => setTimeout(resolve, 100));
+  }
+  throw new Error(`Timed out waiting for ${url}`);
+}
+
+async function stopChild(childProcess) {
+  if (childProcess.exitCode !== null || childProcess.signalCode !== null) return;
+  const exited = once(childProcess, 'exit').catch(() => {});
+  childProcess.kill('SIGTERM');
+  await exited;
+}

weboperator-bridge/mcp-server.js

@@ -196,16 +196,42 @@ async function callBridge(type, payload = {}, timeoutMs = 60_000) {
   // Try Unix Domain Socket first
   try {
     return await callSocket(type, payload, timeoutMs);
-  } catch {
-    // Fallback to HTTP API
+  } catch (err) {
+    // Once the bridge has taken the request, its answer is the answer. A
+    // retry over HTTP would hide the real error behind whatever the second
+    // attempt says, and after a timeout it would run the action again — a
+    // second click, a second form submission.
+    if (err && err.reachedBridge) throw err;
+    // The socket could not be opened at all: fall back to the HTTP API.
     return await callHttp(type, payload, timeoutMs);
   }
 }
 
+// The socket takes the extension's own request types; the HTTP API has its
+// own routes and tool names. This is the translation between the two.
+function httpRequestFor(type, payload, timeoutMs) {
+  if (type === 'tasks.start') {
+    return { path: '/v1/tasks', body: payload };
+  }
+  if (type === 'tasks.wait') {
+    return {
+      path: `/v1/tasks/${encodeURIComponent(payload.id)}/wait`,
+      body: { timeoutMs: payload.timeoutMs },
+    };
+  }
+  // browser.snapshot -> browser_snapshot, and so on for every browser tool.
+  return {
+    path: '/v1/tools/call',
+    body: { tool: type.replace('.', '_'), arguments: payload, timeoutMs },
+  };
+}
+
 
 function callSocket(type, payload, timeoutMs) {
   return new Promise((resolve, reject) => {
     let resolved = false;
+    let connected = false;
+    const fail = (err) => reject(Object.assign(err, { reachedBridge: connected }));
     const socket = net.createConnection(SOCKET_PATH);
     const id = randomUUID();
     const message = { id, type, payload, timeoutMs, ...(API_TOKEN ? { token: API_TOKEN } : {}) };
@@ -214,14 +240,15 @@ function callSocket(type, payload, timeoutMs) {
       if (!resolved) {
         resolved = true;
         socket.destroy();
-        reject(new Error(`Bridge socket timeout for ${type}`));
+        fail(new Error(`Bridge socket timeout for ${type}`));
       }
     }, timeoutMs);
 
     let buffer = Buffer.alloc(0);
     let nextLength = null;
 
     socket.on('connect', () => {
+      connected = true;
       const body = Buffer.from(JSON.stringify(message), 'utf8');
       const header = Buffer.alloc(4);
       header.writeUInt32LE(body.length, 0);
@@ -248,15 +275,15 @@ function callSocket(type, payload, timeoutMs) {
             resolved = true;
             clearTimeout(timer);
             socket.end();
-            if (msg.error) reject(new Error(msg.error));
+            if (msg.error) fail(new Error(msg.error));
             else resolve(msg.result);
           }
         } catch (parseErr) {
           if (!resolved) {
             resolved = true;
             clearTimeout(timer);
             socket.destroy();
-            reject(parseErr);
+            fail(parseErr);
           }
         }
       }
@@ -266,17 +293,16 @@ function callSocket(type, payload, timeoutMs) {
       if (!resolved) {
         resolved = true;
         clearTimeout(timer);
-        reject(err);
+        fail(err);
       }
     });
   });
 }
 
 function callHttp(type, payload, timeoutMs) {
   return new Promise((resolve, reject) => {
-    let path = '/v1/tools/call';
+    const { path, body: bodyObj } = httpRequestFor(type, payload, timeoutMs);
     const method = 'POST';
-    const bodyObj = { tool: type, arguments: payload, timeoutMs };
 
     const headers = {
       'content-type': 'application/json',