diff --git a/src/browser.test.ts b/src/browser.test.ts index 4617a8f57..0b8c43d85 100644 --- a/src/browser.test.ts +++ b/src/browser.test.ts @@ -170,6 +170,31 @@ describe('BrowserBridge state', () => { await expect(bridge.connect({ timeout: 0.1 })).rejects.toThrow('Browser Bridge extension not connected'); }); + it('threads preferredContextId into every readiness health read', async () => { + const { PKG_VERSION } = await import('./version.js'); + const spy = vi.spyOn(daemonTransport, 'getDaemonHealth').mockResolvedValue({ + state: 'no-extension', + status: { + ok: true, + pid: 999999, + uptime: 0, + daemonVersion: PKG_VERSION, + extensionConnected: false, + pending: 0, + memoryMB: 0, + port: 0, + }, + }); + + const bridge = new BrowserBridge(); + + await expect(bridge.connect({ timeout: 0.1, preferredContextId: 'zvypsyje' })).rejects.toThrow('Browser Bridge extension not connected'); + expect(spy.mock.calls.length).toBeGreaterThan(1); + for (const call of spy.mock.calls) { + expect(call[0]).toMatchObject({ preferredContextId: 'zvypsyje' }); + } + }); + it('attempts stale daemon replacement when daemonVersion is missing', async () => { vi.spyOn(daemonTransport, 'getDaemonHealth').mockResolvedValue({ state: 'no-extension', diff --git a/src/browser/bridge-readiness.test.ts b/src/browser/bridge-readiness.test.ts index 9d64db209..0b319fd49 100644 --- a/src/browser/bridge-readiness.test.ts +++ b/src/browser/bridge-readiness.test.ts @@ -54,6 +54,19 @@ describe('waitForBridgeReady', () => { expect(fetchHealth).toHaveBeenCalledTimes(3); }); + it('forwards preferredContextId to every health poll', async () => { + const sequence: DaemonHealth[] = [notReadyHealth('profile-required'), readyHealth()]; + let i = 0; + const fetchHealth: HealthFetcher = vi.fn(async () => sequence[i++] ?? readyHealth()); + + await waitForBridgeReady(fetchHealth, { timeoutMs: 10_000, intervalMs: 1, preferredContextId: 'zvypsyje' }); + + expect(vi.mocked(fetchHealth).mock.calls.length).toBeGreaterThan(1); + for (const call of vi.mocked(fetchHealth).mock.calls) { + expect(call[0]).toMatchObject({ preferredContextId: 'zvypsyje' }); + } + }); + it('returns the last observed non-ready health when the deadline expires', async () => { const fetchHealth: HealthFetcher = vi.fn(async () => notReadyHealth('profile-disconnected')); diff --git a/src/browser/bridge-readiness.ts b/src/browser/bridge-readiness.ts index f570648c5..2c1637a32 100644 --- a/src/browser/bridge-readiness.ts +++ b/src/browser/bridge-readiness.ts @@ -2,22 +2,22 @@ import type { DaemonHealth } from './daemon-transport.js'; export type { DaemonHealth }; -export type HealthFetcher = (opts?: { timeout?: number; contextId?: string }) => Promise; +export type HealthFetcher = (opts?: { timeout?: number; contextId?: string; preferredContextId?: string }) => Promise; const DEFAULT_POLL_INTERVAL_MS = 200; export async function waitForBridgeReady( fetchHealth: HealthFetcher, - opts: { timeoutMs: number; contextId?: string; intervalMs?: number }, + opts: { timeoutMs: number; contextId?: string; preferredContextId?: string; intervalMs?: number }, ): Promise { const interval = opts.intervalMs ?? DEFAULT_POLL_INTERVAL_MS; - let health = await fetchHealth({ contextId: opts.contextId }); + let health = await fetchHealth({ contextId: opts.contextId, preferredContextId: opts.preferredContextId }); if (health.state === 'ready') return health; const deadline = Date.now() + opts.timeoutMs; while (Date.now() < deadline) { await new Promise((resolve) => setTimeout(resolve, interval)); - health = await fetchHealth({ contextId: opts.contextId }); + health = await fetchHealth({ contextId: opts.contextId, preferredContextId: opts.preferredContextId }); if (health.state === 'ready') return health; } return health; diff --git a/src/browser/bridge.ts b/src/browser/bridge.ts index 5eeca0502..354842312 100644 --- a/src/browser/bridge.ts +++ b/src/browser/bridge.ts @@ -41,7 +41,7 @@ export class BrowserBridge implements IBrowserFactory { const routing = opts.contextId || opts.preferredContextId ? { contextId: opts.contextId, preferredContextId: opts.preferredContextId } : profileRouteParams(resolveProfileSelection()); - await this._ensureDaemon(opts.timeout, routing.contextId); + await this._ensureDaemon(opts.timeout, routing.contextId, routing.preferredContextId); if (!opts.session?.trim()) throw new Error('Browser session is required'); this._page = new Page(opts.session.trim(), opts.idleTimeout, routing.contextId, opts.windowMode, opts.surface, opts.siteSession, routing.preferredContextId); this._state = 'connected'; @@ -61,10 +61,11 @@ export class BrowserBridge implements IBrowserFactory { this._state = 'closed'; } - private async _ensureDaemon(timeoutSeconds?: number, contextId?: string): Promise { + private async _ensureDaemon(timeoutSeconds?: number, contextId?: string, preferredContextId?: string): Promise { const result = await ensureBrowserBridgeReady({ timeoutSeconds: timeoutSeconds ?? Math.ceil(DAEMON_SPAWN_TIMEOUT / 1000), contextId, + preferredContextId, }); this._daemonProc = result.spawnedProcess; } diff --git a/src/browser/daemon-client.test.ts b/src/browser/daemon-client.test.ts index 71b6f9660..8058c0811 100644 --- a/src/browser/daemon-client.test.ts +++ b/src/browser/daemon-client.test.ts @@ -156,6 +156,27 @@ describe('daemon-client', () => { expect(vi.mocked(fetch).mock.calls[0][0]).toMatch(/\/status\?contextId=work$/); }); + it('fetchDaemonStatus joins contextId and preferredContextId in the status query', async () => { + vi.mocked(fetch).mockResolvedValue({ + ok: true, + json: () => Promise.resolve({ + ok: true, + pid: 1, + uptime: 0, + extensionConnected: true, + pending: 0, + memoryMB: 1, + port: 19825, + }), + } as Response); + + await fetchDaemonStatus({ preferredContextId: 'zvypsyje' }); + await fetchDaemonStatus({ contextId: 'work', preferredContextId: 'zvypsyje' }); + + expect(vi.mocked(fetch).mock.calls[0][0]).toMatch(/\/status\?preferredContextId=zvypsyje$/); + expect(vi.mocked(fetch).mock.calls[1][0]).toMatch(/\/status\?contextId=work&preferredContextId=zvypsyje$/); + }); + it('rejects OPENCLI_DAEMON_PORT so CLI and extension cannot split bridge ports', async () => { vi.resetModules(); vi.stubEnv('OPENCLI_DAEMON_PORT', '19999'); @@ -444,9 +465,9 @@ describe('daemon-client', () => { json: () => Promise.resolve({ id: 'server', ok: true, data: 7 }), } as Response); - await expect(sendCommand('exec', { code: '1 + 6', contextId: 'work' })).resolves.toBe(7); + await expect(sendCommand('exec', { code: '1 + 6', contextId: 'work', preferredContextId: 'zvypsyje' })).resolves.toBe(7); - expect(ensureSpy).toHaveBeenCalledWith(expect.objectContaining({ contextId: 'work', verbose: false })); + expect(ensureSpy).toHaveBeenCalledWith(expect.objectContaining({ contextId: 'work', preferredContextId: 'zvypsyje', verbose: false })); const ids = fetchMock.mock.calls.map(([, init]) => (JSON.parse(String(init?.body)) as { id: string }).id); expect(ids).toHaveLength(2); // Transport retries keep the id stable so the executor's journal can dedupe. diff --git a/src/browser/daemon-client.ts b/src/browser/daemon-client.ts index 9d7bffc0c..fe9b93a9a 100644 --- a/src/browser/daemon-client.ts +++ b/src/browser/daemon-client.ts @@ -352,10 +352,12 @@ async function sendCommandRaw( const remainingSeconds = Math.ceil((deadlineAt - Date.now()) / 1000); const ready = await ensureBrowserBridgeReady({ timeoutSeconds: Math.max(1, Math.min(DEFAULT_BROWSER_CONNECT_TIMEOUT, remainingSeconds)), - // Only an explicit requirement pins readiness to a specific profile — - // waiting for a stale preferred profile to come back would hang the - // ensure path even though the daemon can already serve the command. + // Only an explicit requirement pins readiness to a specific profile; a + // preferred one is arbitrated against live connections on every poll, + // so a stale default still cannot hang the ensure path while a valid + // one avoids the multi-profile ambiguity error (#2259). contextId, + preferredContextId, verbose: false, }); executorJournaled = versionAtLeast(ready.health.status?.extensionVersion, MIN_JOURNAL_EXTENSION_VERSION); diff --git a/src/browser/daemon-lifecycle.ts b/src/browser/daemon-lifecycle.ts index 6123199b7..f4f7b75a4 100644 --- a/src/browser/daemon-lifecycle.ts +++ b/src/browser/daemon-lifecycle.ts @@ -94,14 +94,15 @@ export async function restartDaemon(opts: { stopTimeoutMs?: number; startTimeout } export async function ensureBrowserBridgeReady( - opts: { timeoutSeconds?: number; contextId?: string; verbose?: boolean } = {}, + opts: { timeoutSeconds?: number; contextId?: string; preferredContextId?: string; verbose?: boolean } = {}, ): Promise { const timeoutSeconds = opts.timeoutSeconds && opts.timeoutSeconds > 0 ? opts.timeoutSeconds : 10; const timeoutMs = timeoutSeconds * 1000; const verbose = opts.verbose ?? true; const contextId = opts.contextId; + const preferredContextId = opts.preferredContextId; - const health = await getDaemonHealth({ contextId }); + const health = await getDaemonHealth({ contextId, preferredContextId }); const daemonVersion = health.status?.daemonVersion; const isStale = !!health.status && (!daemonVersion || daemonVersion !== PKG_VERSION); let staleDaemonReplaced = false; @@ -158,7 +159,7 @@ export async function ensureBrowserBridgeReady( process.stderr.write(' Make sure Chrome or Chromium is open and the OpenCLI extension is enabled.\n'); } - const finalHealth = await waitForBridgeReady(getDaemonHealth, { timeoutMs, contextId }); + const finalHealth = await waitForBridgeReady(getDaemonHealth, { timeoutMs, contextId, preferredContextId }); if (finalHealth.state === 'ready') return { health: finalHealth, spawnedProcess }; throw browserConnectErrorFromHealth(finalHealth, contextId); } diff --git a/src/browser/daemon-transport.ts b/src/browser/daemon-transport.ts index 2fabcb8a1..3a9bafa25 100644 --- a/src/browser/daemon-transport.ts +++ b/src/browser/daemon-transport.ts @@ -66,10 +66,13 @@ export async function requestDaemon(pathname: string, init?: RequestInit & { tim } } -export async function fetchDaemonStatus(opts?: { timeout?: number; contextId?: string }): Promise { +export async function fetchDaemonStatus(opts?: { timeout?: number; contextId?: string; preferredContextId?: string }): Promise { try { - const params = opts?.contextId ? `?contextId=${encodeURIComponent(opts.contextId)}` : ''; - const res = await requestDaemon(`/status${params}`, { timeout: opts?.timeout ?? 2000 }); + const query = new URLSearchParams({ + ...(opts?.contextId ? { contextId: opts.contextId } : {}), + ...(opts?.preferredContextId ? { preferredContextId: opts.preferredContextId } : {}), + }).toString(); + const res = await requestDaemon(`/status${query ? `?${query}` : ''}`, { timeout: opts?.timeout ?? 2000 }); if (!res.ok) return null; return await res.json() as DaemonStatus; } catch (err) { @@ -78,7 +81,7 @@ export async function fetchDaemonStatus(opts?: { timeout?: number; contextId?: s } } -export async function getDaemonHealth(opts?: { timeout?: number; contextId?: string }): Promise { +export async function getDaemonHealth(opts?: { timeout?: number; contextId?: string; preferredContextId?: string }): Promise { const status = await fetchDaemonStatus(opts); if (!status) return { state: 'stopped', status: null }; if (status.profileRequired) return { state: 'profile-required', status }; diff --git a/src/daemon.ts b/src/daemon.ts index d68bdd0c7..83793493a 100644 --- a/src/daemon.ts +++ b/src/daemon.ts @@ -294,7 +294,8 @@ async function handleRequest(req: IncomingMessage, res: ServerResponse): Promise const mem = process.memoryUsage(); const params = new URL(url, `http://localhost:${PORT}`).searchParams; const requestedContextId = params.get('contextId')?.trim() || undefined; - const route = resolveExtensionConnection(requestedContextId); + const preferredContextId = params.get('preferredContextId')?.trim() || undefined; + const route = resolveExtensionConnection(requestedContextId, preferredContextId); const profiles = activeProfiles().map((profile) => ({ contextId: profile.contextId, extensionConnected: true, diff --git a/src/doctor.test.ts b/src/doctor.test.ts index 5ff160a5e..95acf1d83 100644 --- a/src/doctor.test.ts +++ b/src/doctor.test.ts @@ -213,6 +213,32 @@ describe('doctor report rendering', () => { } }); + it('threads the configured default profile into the health read', async () => { + const fs = await import('node:fs'); + const os = await import('node:os'); + const path = await import('node:path'); + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'opencli-doctor-profile-')); + fs.writeFileSync( + path.join(configDir, 'browser-profiles.json'), + JSON.stringify({ version: 1, aliases: {}, defaultContextId: 'zvypsyje' }), + ); + vi.stubEnv('OPENCLI_CONFIG_DIR', configDir); + vi.stubEnv('OPENCLI_PROFILE', ''); + try { + mockGetDaemonHealth.mockResolvedValueOnce({ + state: 'ready', + status: { extensionConnected: true }, + }); + + await runBrowserDoctor(); + + expect(mockGetDaemonHealth).toHaveBeenCalledWith({ preferredContextId: 'zvypsyje' }); + } finally { + vi.unstubAllEnvs(); + fs.rmSync(configDir, { recursive: true, force: true }); + } + }); + it('reports flapping when live check succeeds but final status shows extension disconnected', async () => { mockGetDaemonHealth.mockResolvedValueOnce({ state: 'no-extension', status: { extensionConnected: false } }); diff --git a/src/doctor.ts b/src/doctor.ts index 53c6198a0..e71711680 100644 --- a/src/doctor.ts +++ b/src/doctor.ts @@ -12,7 +12,7 @@ import { getErrorMessage } from './errors.js'; import { getRuntimeLabel } from './runtime-detect.js'; import { getCachedLatestExtensionVersion } from './update-check.js'; import type { BrowserProfileStatus } from './browser/daemon-transport.js'; -import { aliasForContextId, loadProfileConfig } from './browser/profile.js'; +import { aliasForContextId, loadProfileConfig, profileRouteParams, resolveProfileSelection } from './browser/profile.js'; import { formatDaemonVersion, isDaemonStale, staleDaemonIssue } from './browser/daemon-version.js'; import { findShadowedUserAdapters, formatAdapterShadowIssue, type AdapterShadow } from './adapter-shadow.js'; @@ -111,8 +111,10 @@ export async function runBrowserDoctor(opts: DoctorOptions = {}): Promise { +async function getStatus(query = ''): Promise { try { - const res = await fetch(`${BASE}/status`, { headers: HEADERS, signal: AbortSignal.timeout(2_000) }); + const res = await fetch(`${BASE}/status${query}`, { headers: HEADERS, signal: AbortSignal.timeout(2_000) }); if (!res.ok) return null; return await res.json(); } catch { @@ -294,6 +295,29 @@ describe('daemon transport contracts (real daemon)', () => { } }); + it('resolves /status multi-profile ambiguity through preferredContextId', async () => { + if (guard()) return; + const first = new FakeExtension(); + const second = new FakeExtension(); + await first.connect('ctx-status-a'); + await second.connect('ctx-status-b'); + try { + // Two live profiles and no hint → ambiguous, the caller must pick. + const ambiguous = await getStatus(); + expect(ambiguous?.profileRequired).toBe(true); + expect(ambiguous?.extensionConnected).toBe(false); + + // The forwarded preference resolves the ambiguity (#2259). + const preferred = await getStatus('?preferredContextId=ctx-status-b'); + expect(preferred?.profileRequired).toBe(false); + expect(preferred?.contextId).toBe('ctx-status-b'); + expect(preferred?.extensionConnected).toBe(true); + } finally { + first.close(); + second.close(); + } + }); + it('flushes a structured daemon_shutting_down 503 to in-flight dispatched commands on shutdown', async () => { if (guard()) return; const ext = new FakeExtension();