fix(daemon): report unknown browser command results (#1558)

Author: jackwener

src/browser/daemon-client.test.ts

@@ -1,6 +1,7 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
 import {
+  BrowserCommandError,
   fetchDaemonStatus,
   getDaemonHealth,
   requestDaemonShutdown,
@@ -220,4 +221,26 @@ describe('daemon-client', () => {
     });
     expect(ids[0]).not.toBe(ids[1]);
   });
+
+  it('sendCommand does not retry command_result_unknown even when the message looks transient', async () => {
+    const fetchMock = vi.mocked(fetch);
+    fetchMock.mockResolvedValue({
+      ok: false,
+      status: 503,
+      json: () => Promise.resolve({
+        id: 'server',
+        ok: false,
+        errorCode: 'command_result_unknown',
+        error: 'Extension disconnected after command timeout',
+        errorHint: 'Inspect state before retrying.',
+      }),
+    } as Response);
+
+    await expect(sendCommand('exec', { code: 'window.__mutate = true' })).rejects.toMatchObject({
+      name: 'BrowserCommandError',
+      code: 'command_result_unknown',
+      hint: 'Inspect state before retrying.',
+    } satisfies Partial<BrowserCommandError>);
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
 });

src/browser/daemon-client.ts

@@ -94,6 +94,7 @@ export interface DaemonStatus {
   profileDisconnected?: boolean;
   profiles?: BrowserProfileStatus[];
   pending: number;
+  commandResultUnknown?: number;
   memoryMB: number;
   port: number;
 }
@@ -197,6 +198,9 @@ async function sendCommandRaw(
       const result = (await res.json()) as DaemonResult;
 
       if (!result.ok) {
+        if (result.errorCode === 'command_result_unknown') {
+          throw new BrowserCommandError(result.error ?? 'Browser command result is unknown', result.errorCode, result.errorHint);
+        }
         const isDuplicateCommandId = res.status === 409
           || (result.error ?? '').includes('Duplicate command id');
         if (isDuplicateCommandId && attempt < maxRetries) {

src/daemon-utils.ts

@@ -0,0 +1,55 @@
+export const COMMAND_RESULT_UNKNOWN_CODE = 'command_result_unknown';
+
+export const COMMAND_RESULT_UNKNOWN_HINT =
+  'Inspect the browser/session state before retrying. Do not blindly retry write commands such as navigate, click, type, or eval.';
+
+export const PROFILE_DISCONNECTED_HINT =
+  'Open that Chrome profile and make sure the OpenCLI extension is enabled, or choose another profile with opencli profile use <name>.';
+
+export type DaemonFailureContract = {
+  message: string;
+  errorCode: string;
+  errorHint: string;
+  status: number;
+  countAsCommandResultUnknown: boolean;
+};
+
+export function commandResultUnknownMessage(action: string): string {
+  return `Browser connection dropped after the ${action} command was dispatched; it may have completed.`;
+}
+
+export function buildExtensionDisconnectFailure(input: {
+  contextId: string;
+  action: string;
+  dispatched: boolean;
+}): DaemonFailureContract {
+  if (input.dispatched) {
+    return {
+      message: commandResultUnknownMessage(input.action),
+      errorCode: COMMAND_RESULT_UNKNOWN_CODE,
+      errorHint: COMMAND_RESULT_UNKNOWN_HINT,
+      status: 503,
+      countAsCommandResultUnknown: true,
+    };
+  }
+  return buildCommandDispatchFailure(input.contextId);
+}
+
+export function buildCommandDispatchFailure(contextId: string): DaemonFailureContract {
+  return {
+    message: `Browser profile "${contextId}" disconnected before command dispatch`,
+    errorCode: 'profile_disconnected',
+    errorHint: PROFILE_DISCONNECTED_HINT,
+    status: 503,
+    countAsCommandResultUnknown: false,
+  };
+}
+
+export function getResponseCorsHeaders(pathname: string, origin?: string): Record<string, string> | undefined {
+  if (pathname !== '/ping') return undefined;
+  if (!origin || !origin.startsWith('chrome-extension://')) return undefined;
+  return {
+    'Access-Control-Allow-Origin': origin,
+    Vary: 'Origin',
+  };
+}

src/daemon.test.ts

@@ -1,6 +1,13 @@
 import { describe, expect, it } from 'vitest';
 
-import { getResponseCorsHeaders } from './daemon.js';
+import {
+  COMMAND_RESULT_UNKNOWN_CODE,
+  COMMAND_RESULT_UNKNOWN_HINT,
+  buildCommandDispatchFailure,
+  buildExtensionDisconnectFailure,
+  commandResultUnknownMessage,
+  getResponseCorsHeaders,
+} from './daemon-utils.js';
 
 describe('getResponseCorsHeaders', () => {
   it('allows the Browser Bridge extension origin to read /ping', () => {
@@ -22,3 +29,48 @@ describe('getResponseCorsHeaders', () => {
     expect(getResponseCorsHeaders('/command', 'chrome-extension://abc123')).toBeUndefined();
   });
 });
+
+describe('daemon command dispatch', () => {
+  it('uses a distinct command_result_unknown contract for ambiguous dispatched commands', () => {
+    expect(COMMAND_RESULT_UNKNOWN_CODE).toBe('command_result_unknown');
+    expect(commandResultUnknownMessage('navigate')).toContain('navigate command was dispatched');
+    expect(COMMAND_RESULT_UNKNOWN_HINT).toContain('Inspect the browser/session state');
+    expect(COMMAND_RESULT_UNKNOWN_HINT).toContain('Do not blindly retry write commands');
+  });
+
+  it('classifies dispatched extension disconnects as command_result_unknown', () => {
+    expect(buildExtensionDisconnectFailure({
+      contextId: 'work',
+      action: 'navigate',
+      dispatched: true,
+    })).toEqual({
+      message: 'Browser connection dropped after the navigate command was dispatched; it may have completed.',
+      errorCode: 'command_result_unknown',
+      errorHint: COMMAND_RESULT_UNKNOWN_HINT,
+      status: 503,
+      countAsCommandResultUnknown: true,
+    });
+  });
+
+  it('classifies pre-dispatch extension disconnects as profile_disconnected', () => {
+    expect(buildExtensionDisconnectFailure({
+      contextId: 'work',
+      action: 'navigate',
+      dispatched: false,
+    })).toMatchObject({
+      message: 'Browser profile "work" disconnected before command dispatch',
+      errorCode: 'profile_disconnected',
+      status: 503,
+      countAsCommandResultUnknown: false,
+    });
+  });
+
+  it('classifies ws.send dispatch failures as profile_disconnected', () => {
+    expect(buildCommandDispatchFailure('work')).toMatchObject({
+      message: 'Browser profile "work" disconnected before command dispatch',
+      errorCode: 'profile_disconnected',
+      status: 503,
+      countAsCommandResultUnknown: false,
+    });
+  });
+});

src/daemon.ts

@@ -28,6 +28,11 @@ import { log } from './logger.js';
 import { PKG_VERSION } from './version.js';
 import { DEFAULT_CONTEXT_ID } from './browser/profile.js';
 import { recordExtensionVersion } from './update-check.js';
+import {
+  buildCommandDispatchFailure,
+  buildExtensionDisconnectFailure,
+  getResponseCorsHeaders,
+} from './daemon-utils.js';
 
 const PORT = parseInt(process.env.OPENCLI_DAEMON_PORT ?? String(DEFAULT_DAEMON_PORT), 10);
 
@@ -44,10 +49,13 @@ type ExtensionProfileConnection = {
 const extensionProfiles = new Map<string, ExtensionProfileConnection>();
 const pending = new Map<string, {
   contextId: string;
+  action: string;
+  dispatched: boolean;
   resolve: (data: unknown) => void;
   reject: (error: Error) => void;
   timer: ReturnType<typeof setTimeout>;
 }>();
+let commandResultUnknownCount = 0;
 // Extension log ring buffer
 interface LogEntry { level: string; msg: string; ts: number; }
 const LOG_BUFFER_SIZE = 200;
@@ -136,12 +144,16 @@ function unregisterExtensionConnection(ws: WebSocket): void {
     for (const [id, p] of pending) {
       if (p.contextId !== contextId) continue;
       clearTimeout(p.timer);
-      p.reject(new DaemonCommandFailure(
-        `Browser profile "${contextId}" disconnected`,
-        'profile_disconnected',
-        'Open that Chrome profile and make sure the OpenCLI extension is enabled, or choose another profile with opencli profile use <name>.',
-        503,
-      ));
+      const failure = buildExtensionDisconnectFailure({
+        contextId,
+        action: p.action,
+        dispatched: p.dispatched,
+      });
+      if (failure.countAsCommandResultUnknown) {
+        commandResultUnknownCount++;
+        log.warn(`[daemon] Command result unknown after extension disconnect (id=${id}, action=${p.action}, context=${contextId})`);
+      }
+      p.reject(new DaemonCommandFailure(failure.message, failure.errorCode, failure.errorHint, failure.status));
       pending.delete(id);
     }
   }
@@ -176,15 +188,6 @@ function jsonResponse(
   res.end(JSON.stringify(data));
 }
 
-export function getResponseCorsHeaders(pathname: string, origin?: string): Record<string, string> | undefined {
-  if (pathname !== '/ping') return undefined;
-  if (!origin || !origin.startsWith('chrome-extension://')) return undefined;
-  return {
-    'Access-Control-Allow-Origin': origin,
-    Vary: 'Origin',
-  };
-}
-
 async function handleRequest(req: IncomingMessage, res: ServerResponse): Promise<void> {
   // ─── Security: Origin & custom-header check ──────────────────────
   // Block browser-based CSRF: browsers always send an Origin header on
@@ -257,6 +260,7 @@ async function handleRequest(req: IncomingMessage, res: ServerResponse): Promise
       profileDisconnected: route.errorCode === 'profile_disconnected',
       profiles,
       pending: pending.size,
+      commandResultUnknown: commandResultUnknownCount,
       memoryMB: Math.round(mem.rss / 1024 / 1024 * 10) / 10,
       port: PORT,
     });
@@ -321,8 +325,34 @@ async function handleRequest(req: IncomingMessage, res: ServerResponse): Promise
           pending.delete(body.id);
           reject(new Error(`Command timeout (${timeoutMs / 1000}s)`));
         }, timeoutMs);
-        pending.set(body.id, { contextId: route.connection!.contextId, resolve, reject, timer });
-        route.connection!.ws.send(JSON.stringify(body));
+        const entry = {
+          contextId: route.connection!.contextId,
+          action: typeof body.action === 'string' ? body.action : 'unknown',
+          dispatched: false,
+          resolve,
+          reject,
+          timer,
+        };
+        pending.set(body.id, entry);
+        const failBeforeDispatch = (err: unknown) => {
+          if (pending.get(body.id) !== entry) return;
+          const failure = buildCommandDispatchFailure(entry.contextId);
+          clearTimeout(timer);
+          pending.delete(body.id);
+          reject(new DaemonCommandFailure(failure.message, failure.errorCode, failure.errorHint, failure.status));
+          log.warn(`[daemon] Failed to dispatch command ${body.id}: ${err instanceof Error ? err.message : String(err)}`);
+        };
+        try {
+          route.connection!.ws.send(JSON.stringify(body), (err?: Error) => {
+            if (err && !entry.dispatched) failBeforeDispatch(err);
+          });
+          // Once ws accepts the frame, the command may execute even if the
+          // result is later lost; do not downgrade later disconnects to a
+          // pre-dispatch failure just because no result/ack has arrived yet.
+          entry.dispatched = true;
+        } catch (err) {
+          failBeforeDispatch(err);
+        }
       });
 
       jsonResponse(res, 200, result);