Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/browser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
39 changes: 21 additions & 18 deletions src/browser/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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' +
Expand Down Expand Up @@ -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' +
Expand All @@ -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;
}
}
62 changes: 34 additions & 28 deletions src/browser/daemon-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

import {
fetchDaemonStatus,
isDaemonRunning,
isExtensionConnected,
getDaemonHealth,
requestDaemonShutdown,
} from './daemon-client.js';

Expand Down Expand Up @@ -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 });
});
});
31 changes: 16 additions & 15 deletions src/browser/daemon-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand All @@ -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,
Expand Down
26 changes: 0 additions & 26 deletions src/browser/discover.ts

This file was deleted.

3 changes: 2 additions & 1 deletion src/browser/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
12 changes: 6 additions & 6 deletions src/commanderAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
57 changes: 33 additions & 24 deletions src/doctor.test.ts
Original file line number Diff line number Diff line change
@@ -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,
}));

Expand Down Expand Up @@ -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 });

Expand All @@ -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 });

Expand All @@ -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);
});
});
Loading
Loading