Compare commits

...

1 Commits

Author SHA1 Message Date
jackwener a1546d5d69 refactor(doctor/status): unify daemon health checks into getDaemonHealth()
- 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
2026-04-08 14:53:24 +08:00
9 changed files with 143 additions and 149 deletions
+1 -2
View File
@@ -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();
+21 -18
View File
@@ -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;
}
}
+37 -31
View File
@@ -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 () => {
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,
}),
} as Response);
it('getDaemonHealth returns stopped when daemon is not reachable', async () => {
vi.mocked(fetch).mockRejectedValue(new Error('ECONNREFUSED'));
await expect(isDaemonRunning()).resolves.toBe(true);
await expect(getDaemonHealth()).resolves.toEqual({ state: 'stopped', status: null });
});
it('isExtensionConnected reflects shared status payload', async () => {
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(isExtensionConnected()).resolves.toBe(false);
await expect(getDaemonHealth()).resolves.toEqual({ state: 'no-extension', status });
});
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(status),
} as Response);
await expect(getDaemonHealth()).resolves.toEqual({ state: 'ready', status });
});
});
+16 -15
View File
@@ -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,
-26
View File
@@ -1,26 +0,0 @@
/**
* Daemon discovery — checks if the daemon is running.
*/
import { fetchDaemonStatus, isDaemonRunning } from './daemon-client.js';
export { isDaemonRunning };
/**
* Check daemon status and return connection info.
*/
export async function checkDaemonStatus(opts?: { timeout?: number }): Promise<{
running: boolean;
extensionConnected: boolean;
extensionVersion?: string;
}> {
const status = await fetchDaemonStatus({ timeout: opts?.timeout ?? 2000 });
if (!status) {
return { running: false, extensionConnected: false };
}
return {
running: true,
extensionConnected: status.extensionConnected,
extensionVersion: status.extensionVersion,
};
}
+2 -1
View File
@@ -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';
+6 -6
View File
@@ -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);
+33 -24
View File
@@ -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,7 +169,6 @@ 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 {
@@ -181,10 +176,24 @@ describe('doctor report rendering', () => {
};
});
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);
});
});
+27 -26
View File
@@ -6,9 +6,8 @@
import chalk from 'chalk';
import { DEFAULT_DAEMON_PORT } from './constants.js';
import { checkDaemonStatus } from './browser/discover.js';
import { BrowserBridge } from './browser/index.js';
import { listSessions } from './browser/daemon-client.js';
import { getDaemonHealth, listSessions } from './browser/daemon-client.js';
import { getErrorMessage } from './errors.js';
import { getRuntimeLabel } from './runtime-detect.js';
@@ -58,31 +57,32 @@ export async function checkConnectivity(opts?: { timeout?: number }): Promise<Co
}
export async function runBrowserDoctor(opts: DoctorOptions = {}): Promise<DoctorReport> {
// Try to auto-start daemon if it's not running, so we show accurate status.
let initialStatus = await checkDaemonStatus();
if (!initialStatus.running) {
try {
const bridge = new BrowserBridge();
await bridge.connect({ timeout: 5 });
await bridge.close();
} catch {
// Auto-start failed; we'll report it below.
}
}
// Run the live connectivity check — it may also auto-start the daemon as a
// side-effect, so we read daemon status only *after* all side-effects settle.
// Live connectivity check doubles as auto-start (bridge.connect spawns daemon).
let connectivity: ConnectivityResult | undefined;
if (opts.live) {
connectivity = await checkConnectivity();
} else {
// No live probe — daemon may have idle-exited. Do a minimal auto-start
// so we don't misreport a lazy-lifecycle stop as a real failure.
const initialHealth = await getDaemonHealth();
if (initialHealth.state === 'stopped') {
try {
const bridge = new BrowserBridge();
await bridge.connect({ timeout: 5 });
await bridge.close();
} catch {
// Auto-start failed; we'll report it below.
}
}
}
const status = await checkDaemonStatus();
const daemonFlaky = !!(connectivity?.ok && !status.running);
const extensionFlaky = !!(connectivity?.ok && status.running && !status.extensionConnected);
const daemonRunning = status.running;
const extensionConnected = status.extensionConnected;
const sessions = opts.sessions && status.running && status.extensionConnected
// Single status read *after* all side-effects (live check / auto-start) settle.
const health = await getDaemonHealth();
const daemonRunning = health.state !== 'stopped';
const extensionConnected = health.state === 'ready';
const daemonFlaky = !!(connectivity?.ok && !daemonRunning);
const extensionFlaky = !!(connectivity?.ok && daemonRunning && !extensionConnected);
const sessions = opts.sessions && health.state === 'ready'
? await listSessions() as Array<{ workspace: string; windowId: number; tabCount: number; idleMsRemaining: number }>
: undefined;
@@ -112,12 +112,13 @@ export async function runBrowserDoctor(opts: DoctorOptions = {}): Promise<Doctor
if (connectivity && !connectivity.ok) {
issues.push(`Browser connectivity test failed: ${connectivity.error ?? 'unknown'}`);
}
if (status.extensionVersion && opts.cliVersion) {
const extMajor = status.extensionVersion.split('.')[0];
const extensionVersion = health.status?.extensionVersion;
if (extensionVersion && opts.cliVersion) {
const extMajor = extensionVersion.split('.')[0];
const cliMajor = opts.cliVersion.split('.')[0];
if (extMajor !== cliMajor) {
issues.push(
`Extension major version mismatch: extension v${status.extensionVersion} ≠ CLI v${opts.cliVersion}\n` +
`Extension major version mismatch: extension v${extensionVersion} ≠ CLI v${opts.cliVersion}\n` +
' Download the latest extension from: https://github.com/jackwener/opencli/releases',
);
}
@@ -129,7 +130,7 @@ export async function runBrowserDoctor(opts: DoctorOptions = {}): Promise<Doctor
daemonFlaky,
extensionConnected,
extensionFlaky,
extensionVersion: status.extensionVersion,
extensionVersion,
connectivity,
sessions,
issues,