refactor(doctor/status): unify daemon health checks into getDaemonHealth() (#873)

- Add getDaemonHealth() returning 'stopped' | 'no-extension' | 'ready'
- Delete discover.ts (thin wrapper with no value)
- Bridge uses getDaemonHealth() + _pollUntilReady() (eliminates duplicate polling)
- Doctor simplified: live check auto-starts daemon; no-live mode does minimal
  auto-start only when stopped (avoids misreporting idle-exit as failure)
- CommanderAdapter preserves error message/hint detail (not just generic title)
- All callers use single unified status entry point

Author: jackwener

src/browser.test.ts

@@ -135,8 +135,7 @@ describe('BrowserBridge state', () => {
   });
 
   it('fails fast when daemon is running but extension is disconnected', async () => {
-    vi.spyOn(daemonClient, 'isExtensionConnected').mockResolvedValue(false);
-    vi.spyOn(daemonClient, 'fetchDaemonStatus').mockResolvedValue({ extensionConnected: false } as any);
+    vi.spyOn(daemonClient, 'getDaemonHealth').mockResolvedValue({ state: 'no-extension', status: { extensionConnected: false } as any });
 
     const bridge = new BrowserBridge();
 

src/browser/bridge.ts

@@ -9,7 +9,7 @@ import * as fs from 'node:fs';
 import type { IPage } from '../types.js';
 import type { IBrowserFactory } from '../runtime.js';
 import { Page } from './page.js';
-import { fetchDaemonStatus, isExtensionConnected } from './daemon-client.js';
+import { getDaemonHealth } from './daemon-client.js';
 import { DEFAULT_DAEMON_PORT } from '../constants.js';
 import { BrowserConnectError } from '../errors.js';
 
@@ -61,23 +61,18 @@ export class BrowserBridge implements IBrowserFactory {
     const effectiveSeconds = (timeoutSeconds && timeoutSeconds > 0) ? timeoutSeconds : Math.ceil(DAEMON_SPAWN_TIMEOUT / 1000);
     const timeoutMs = effectiveSeconds * 1000;
 
-    // Single status check instead of two separate fetchDaemonStatus() calls
-    const status = await fetchDaemonStatus();
+    const health = await getDaemonHealth();
 
-    // Fast path: extension already connected
-    if (status?.extensionConnected) return;
+    // Fast path: everything ready
+    if (health.state === 'ready') return;
 
     // Daemon running but no extension — wait for extension with progress
-    if (status !== null) {
+    if (health.state === 'no-extension') {
       if (process.env.OPENCLI_VERBOSE || process.stderr.isTTY) {
         process.stderr.write('⏳ Waiting for Chrome/Chromium extension to connect...\n');
         process.stderr.write('   Make sure Chrome or Chromium is open and the OpenCLI extension is enabled.\n');
       }
-      const deadline = Date.now() + timeoutMs;
-      while (Date.now() < deadline) {
-        await new Promise(resolve => setTimeout(resolve, 200));
-        if (await isExtensionConnected()) return;
-      }
+      if (await this._pollUntilReady(timeoutMs)) return;
       throw new BrowserConnectError(
         'Browser Bridge extension not connected',
         'Install the Browser Bridge:\n' +
@@ -111,14 +106,11 @@ export class BrowserBridge implements IBrowserFactory {
     });
     this._daemonProc.unref();
 
-    // Wait for daemon + extension with faster polling
-    const deadline = Date.now() + timeoutMs;
-    while (Date.now() < deadline) {
-      await new Promise(resolve => setTimeout(resolve, 200));
-      if (await isExtensionConnected()) return;
-    }
+    // Wait for daemon + extension
+    if (await this._pollUntilReady(timeoutMs)) return;
 
-    if ((await fetchDaemonStatus()) !== null) {
+    const finalHealth = await getDaemonHealth();
+    if (finalHealth.state === 'no-extension') {
       throw new BrowserConnectError(
         'Browser Bridge extension not connected',
         'Install the Browser Bridge:\n' +
@@ -135,4 +127,15 @@ export class BrowserBridge implements IBrowserFactory {
       'daemon-not-running',
     );
   }
+
+  /** Poll getDaemonHealth() until state is 'ready' or deadline is reached. */
+  private async _pollUntilReady(timeoutMs: number): Promise<boolean> {
+    const deadline = Date.now() + timeoutMs;
+    while (Date.now() < deadline) {
+      await new Promise(resolve => setTimeout(resolve, 200));
+      const h = await getDaemonHealth();
+      if (h.state === 'ready') return true;
+    }
+    return false;
+  }
 }

src/browser/daemon-client.test.ts

@@ -2,8 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
 import {
   fetchDaemonStatus,
-  isDaemonRunning,
-  isExtensionConnected,
+  getDaemonHealth,
   requestDaemonShutdown,
 } from './daemon-client.js';
 
@@ -63,41 +62,48 @@ describe('daemon-client', () => {
     );
   });
 
-  it('isDaemonRunning reflects shared status availability', async () => {
+  it('getDaemonHealth returns stopped when daemon is not reachable', async () => {
+    vi.mocked(fetch).mockRejectedValue(new Error('ECONNREFUSED'));
+
+    await expect(getDaemonHealth()).resolves.toEqual({ state: 'stopped', status: null });
+  });
+
+  it('getDaemonHealth returns no-extension when daemon is running but extension disconnected', async () => {
+    const status = {
+      ok: true,
+      pid: 123,
+      uptime: 10,
+      extensionConnected: false,
+      pending: 0,
+      lastCliRequestTime: Date.now(),
+      memoryMB: 16,
+      port: 19825,
+    };
     vi.mocked(fetch).mockResolvedValue({
       ok: true,
-      json: () =>
-        Promise.resolve({
-          ok: true,
-          pid: 123,
-          uptime: 10,
-          extensionConnected: false,
-          pending: 0,
-          lastCliRequestTime: Date.now(),
-          memoryMB: 16,
-          port: 19825,
-        }),
+      json: () => Promise.resolve(status),
     } as Response);
 
-    await expect(isDaemonRunning()).resolves.toBe(true);
+    await expect(getDaemonHealth()).resolves.toEqual({ state: 'no-extension', status });
   });
 
-  it('isExtensionConnected reflects shared status payload', async () => {
+  it('getDaemonHealth returns ready when daemon and extension are both connected', async () => {
+    const status = {
+      ok: true,
+      pid: 123,
+      uptime: 10,
+      extensionConnected: true,
+      extensionVersion: '1.2.3',
+      pending: 0,
+      lastCliRequestTime: Date.now(),
+      memoryMB: 32,
+      port: 19825,
+    };
     vi.mocked(fetch).mockResolvedValue({
       ok: true,
-      json: () =>
-        Promise.resolve({
-          ok: true,
-          pid: 123,
-          uptime: 10,
-          extensionConnected: false,
-          pending: 0,
-          lastCliRequestTime: Date.now(),
-          memoryMB: 16,
-          port: 19825,
-        }),
+      json: () => Promise.resolve(status),
     } as Response);
 
-    await expect(isExtensionConnected()).resolves.toBe(false);
+    await expect(getDaemonHealth()).resolves.toEqual({ state: 'ready', status });
   });
 });

src/browser/daemon-client.ts

@@ -91,6 +91,22 @@ export async function fetchDaemonStatus(opts?: { timeout?: number }): Promise<Da
   }
 }
 
+export type DaemonHealth =
+  | { state: 'stopped'; status: null }
+  | { state: 'no-extension'; status: DaemonStatus }
+  | { state: 'ready'; status: DaemonStatus };
+
+/**
+ * Unified daemon health check — single entry point for all status queries.
+ * Replaces isDaemonRunning(), isExtensionConnected(), and checkDaemonStatus().
+ */
+export async function getDaemonHealth(opts?: { timeout?: number }): Promise<DaemonHealth> {
+  const status = await fetchDaemonStatus(opts);
+  if (!status) return { state: 'stopped', status: null };
+  if (!status.extensionConnected) return { state: 'no-extension', status };
+  return { state: 'ready', status };
+}
+
 export async function requestDaemonShutdown(opts?: { timeout?: number }): Promise<boolean> {
   try {
     const res = await requestDaemon('/shutdown', { method: 'POST', timeout: opts?.timeout ?? 5000 });
@@ -100,21 +116,6 @@ export async function requestDaemonShutdown(opts?: { timeout?: number }): Promis
   }
 }
 
-/**
- * Check if daemon is running.
- */
-export async function isDaemonRunning(): Promise<boolean> {
-  return (await fetchDaemonStatus()) !== null;
-}
-
-/**
- * Check if daemon is running AND the extension is connected.
- */
-export async function isExtensionConnected(): Promise<boolean> {
-  const status = await fetchDaemonStatus();
-  return !!status?.extensionConnected;
-}
-
 /**
  * Send a command to the daemon and wait for a result.
  * Retries up to 4 times: network errors retry at 500ms,

src/browser/discover.ts

@@ -8,7 +8,8 @@
 export { Page } from './page.js';
 export { BrowserBridge } from './bridge.js';
 export { CDPBridge } from './cdp.js';
-export { isDaemonRunning } from './daemon-client.js';
+export { getDaemonHealth } from './daemon-client.js';
+export type { DaemonHealth } from './daemon-client.js';
 export { generateSnapshotJs, scrollToRefJs, getFormStateJs } from './dom-snapshot.js';
 export { generateStealthJs } from './stealth.js';
 export type { DomSnapshotOptions } from './dom-snapshot.js';

src/browser/index.ts

@@ -30,7 +30,7 @@ import {
   AdapterLoadError,
   CommandExecutionError,
 } from './errors.js';
-import { checkDaemonStatus } from './browser/discover.js';
+import { getDaemonHealth } from './browser/daemon-client.js';
 import { isDiagnosticEnabled } from './diagnostic.js';
 
 export function normalizeArgValue(argType: string | undefined, value: unknown, name: string): unknown {
@@ -204,14 +204,14 @@ function emitAutoFixHint(cmdName: string): void {
 async function renderError(err: unknown, cmdName: string, verbose: boolean): Promise<void> {
   // ── BrowserConnectError: real-time diagnosis, kind as fallback ────────
   if (err instanceof BrowserConnectError) {
-    console.error(chalk.red('🔌 Browser Bridge not connected'));
+    console.error(chalk.red(`🔌 ${err.message}`));
+    if (err.hint) console.error(chalk.yellow(`→ ${err.hint}`));
     console.error();
     try {
-      // 300ms matches execution.ts — localhost responds in <50ms when running.
-      const status = await checkDaemonStatus({ timeout: 300 });
-      renderBridgeStatus(status.running, status.extensionConnected);
+      const health = await getDaemonHealth({ timeout: 300 });
+      renderBridgeStatus(health.state !== 'stopped', health.state === 'ready');
     } catch (_statusErr) {
-      // checkDaemonStatus itself failed — derive best-guess state from kind.
+      // getDaemonHealth itself failed — derive best-guess state from kind.
       const running = err.kind !== 'daemon-not-running';
       const extensionConnected = err.kind === 'command-failed';
       renderBridgeStatus(running, extensionConnected);

src/commanderAdapter.ts

@@ -1,17 +1,14 @@
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 
-const { mockCheckDaemonStatus, mockListSessions, mockConnect, mockClose } = vi.hoisted(() => ({
-  mockCheckDaemonStatus: vi.fn(),
+const { mockGetDaemonHealth, mockListSessions, mockConnect, mockClose } = vi.hoisted(() => ({
+  mockGetDaemonHealth: vi.fn(),
   mockListSessions: vi.fn(),
   mockConnect: vi.fn(),
   mockClose: vi.fn(),
 }));
 
-vi.mock('./browser/discover.js', () => ({
-  checkDaemonStatus: mockCheckDaemonStatus,
-}));
-
 vi.mock('./browser/daemon-client.js', () => ({
+  getDaemonHealth: mockGetDaemonHealth,
   listSessions: mockListSessions,
 }));
 
@@ -113,35 +110,33 @@ describe('doctor report rendering', () => {
     expect(text).toContain('Daemon connectivity is unstable.');
   });
 
-  it('reports consistent status when live check auto-starts the daemon', async () => {
-    // checkDaemonStatus is called twice: once for auto-start check, once for final status.
-    // First call: daemon not running (triggers auto-start attempt)
-    mockCheckDaemonStatus.mockResolvedValueOnce({ running: false, extensionConnected: false });
-    // Auto-start attempt via BrowserBridge.connect fails
+  it('reports daemon not running when no-live and auto-start fails', async () => {
+    // no-live mode: getDaemonHealth called twice (initial check + final status)
+    // Initial: stopped → triggers auto-start attempt
+    mockGetDaemonHealth.mockResolvedValueOnce({ state: 'stopped', status: null });
+    // Auto-start fails
     mockConnect.mockRejectedValueOnce(new Error('Could not start daemon'));
-    // Second call: daemon still not running after failed auto-start
-    mockCheckDaemonStatus.mockResolvedValueOnce({ running: false, extensionConnected: false });
+    // Final: still stopped
+    mockGetDaemonHealth.mockResolvedValueOnce({ state: 'stopped', status: null });
 
     const report = await runBrowserDoctor({ live: false });
 
-    // Status reflects daemon not running
     expect(report.daemonRunning).toBe(false);
     expect(report.extensionConnected).toBe(false);
-    // checkDaemonStatus called twice (initial + final)
-    expect(mockCheckDaemonStatus).toHaveBeenCalledTimes(2);
-    // Should report daemon not running
+    expect(mockGetDaemonHealth).toHaveBeenCalledTimes(2);
     expect(report.issues).toEqual(expect.arrayContaining([
       expect.stringContaining('Daemon is not running'),
     ]));
   });
 
-  it('reports flapping when live check succeeds but final status flips disconnected', async () => {
-    mockCheckDaemonStatus.mockResolvedValueOnce({ running: true, extensionConnected: false });
+  it('reports flapping when live check succeeds but final status shows extension disconnected', async () => {
+    // Live check succeeds
     mockConnect.mockResolvedValueOnce({
       evaluate: vi.fn().mockResolvedValue(2),
     });
     mockClose.mockResolvedValueOnce(undefined);
-    mockCheckDaemonStatus.mockResolvedValueOnce({ running: true, extensionConnected: false });
+    // After live check, getDaemonHealth shows no-extension
+    mockGetDaemonHealth.mockResolvedValueOnce({ state: 'no-extension', status: { extensionConnected: false } });
 
     const report = await runBrowserDoctor({ live: true });
 
@@ -154,12 +149,13 @@ describe('doctor report rendering', () => {
   });
 
   it('reports daemon flapping when live check succeeds but daemon disappears afterward', async () => {
-    mockCheckDaemonStatus.mockResolvedValueOnce({ running: true, extensionConnected: true });
+    // Live check succeeds
     mockConnect.mockResolvedValueOnce({
       evaluate: vi.fn().mockResolvedValue(2),
     });
     mockClose.mockResolvedValueOnce(undefined);
-    mockCheckDaemonStatus.mockResolvedValueOnce({ running: false, extensionConnected: false });
+    // After live check, getDaemonHealth shows stopped
+    mockGetDaemonHealth.mockResolvedValueOnce({ state: 'stopped', status: null });
 
     const report = await runBrowserDoctor({ live: true });
 
@@ -173,18 +169,31 @@ describe('doctor report rendering', () => {
 
   it('uses the fast default timeout for live connectivity checks', async () => {
     let timeoutSeen: number | undefined;
-    mockCheckDaemonStatus.mockResolvedValueOnce({ running: true, extensionConnected: true });
     mockConnect.mockImplementationOnce(async (opts?: { timeout?: number }) => {
       timeoutSeen = opts?.timeout;
       return {
         evaluate: vi.fn().mockResolvedValue(2),
       };
     });
     mockClose.mockResolvedValueOnce(undefined);
-    mockCheckDaemonStatus.mockResolvedValueOnce({ running: true, extensionConnected: true });
+    mockGetDaemonHealth.mockResolvedValueOnce({ state: 'ready', status: { extensionConnected: true } });
 
     await runBrowserDoctor({ live: true });
 
     expect(timeoutSeen).toBe(8);
   });
+
+  it('skips auto-start in no-live mode when daemon is already running', async () => {
+    // no-live mode but daemon already running (no-extension)
+    mockGetDaemonHealth.mockResolvedValueOnce({ state: 'no-extension', status: { extensionConnected: false } });
+    // Final status: same
+    mockGetDaemonHealth.mockResolvedValueOnce({ state: 'no-extension', status: { extensionConnected: false } });
+
+    const report = await runBrowserDoctor({ live: false });
+
+    // Should NOT have tried auto-start since daemon was already running
+    expect(mockConnect).not.toHaveBeenCalled();
+    expect(report.daemonRunning).toBe(true);
+    expect(report.extensionConnected).toBe(false);
+  });
 });