From dce43eeb207352c8a33e88cd35f26ee157372826 Mon Sep 17 00:00:00 2001 From: Ytallo Layon Date: Fri, 14 Aug 2026 20:44:53 -0300 Subject: [PATCH] (MOT-4443) fix(console): recover router presence after reconnect --- .../src/hooks/use-llm-router-status.test.ts | 28 +- .../web/src/hooks/use-model-picker-source.ts | 101 ++++--- .../web/src/hooks/use-worker-presence.test.ts | 229 ++++++++++++++++ console/web/src/hooks/use-worker-presence.ts | 246 +++++++++++++++--- .../web/src/hooks/use-worktree-status.test.ts | 28 +- console/web/src/lib/conversations-context.tsx | 23 +- harness/tests/quickstart/README.md | 6 + .../console-router-recovery.spec.ts | 230 ++++++++++++++++ harness/tests/quickstart/playwright.config.ts | 8 +- harness/tests/quickstart/run-ci.sh | 15 +- 10 files changed, 815 insertions(+), 99 deletions(-) create mode 100644 console/web/src/hooks/use-worker-presence.test.ts create mode 100644 harness/tests/quickstart/console-router-recovery.spec.ts diff --git a/console/web/src/hooks/use-llm-router-status.test.ts b/console/web/src/hooks/use-llm-router-status.test.ts index f004222cc..a6aa935b1 100644 --- a/console/web/src/hooks/use-llm-router-status.test.ts +++ b/console/web/src/hooks/use-llm-router-status.test.ts @@ -16,8 +16,30 @@ describe('llm-router presence probe wiring', () => { }) it('gates on both presence and the initial probe settling', () => { - expect(isLlmRouterAvailable({ present: true, loading: false })).toBe(true) - expect(isLlmRouterAvailable({ present: true, loading: true })).toBe(false) - expect(isLlmRouterAvailable({ present: false, loading: false })).toBe(false) + const refresh = async () => true + expect( + isLlmRouterAvailable({ + present: true, + loading: false, + revision: 1, + refresh, + }), + ).toBe(true) + expect( + isLlmRouterAvailable({ + present: true, + loading: true, + revision: 1, + refresh, + }), + ).toBe(false) + expect( + isLlmRouterAvailable({ + present: false, + loading: false, + revision: 1, + refresh, + }), + ).toBe(false) }) }) diff --git a/console/web/src/hooks/use-model-picker-source.ts b/console/web/src/hooks/use-model-picker-source.ts index acb83fb9c..334a8b799 100644 --- a/console/web/src/hooks/use-model-picker-source.ts +++ b/console/web/src/hooks/use-model-picker-source.ts @@ -1,6 +1,5 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { onHarnessConfigSaved } from '@/lib/harness-config-events' -import { getIiiClient } from '@/lib/iii-client' import { catalogRowsToModelOptions, fetchModelsCatalog, @@ -33,18 +32,23 @@ import type { ModelOption } from '@/types/chat' export function useModelPickerSource( backendId: string, routerAvailable = true, + routerRevision = 0, ): { modelOptions: ModelOption[] catalogKeys: string[] catalogLoading: boolean presentProviders: ProviderListEntry[] - refresh: () => Promise + refresh: (force?: boolean) => Promise } { const [modelOptions, setModelOptions] = useState([]) const [presentProviders, setPresentProviders] = useState( [], ) const providerEventVersion = useRef(0) + const catalogRequestVersion = useRef(0) + const providerRequestVersion = useRef(0) + const routerRevisionRef = useRef(routerRevision) + routerRevisionRef.current = routerRevision // Mirror of `presentProviders` for event handlers: React state updaters may // run deferred, so membership checks must not live inside them. const providersRef = useRef([]) @@ -55,27 +59,41 @@ export function useModelPickerSource( backendId === 'real' && routerAvailable, ) - const refresh = useCallback(async () => { - if (backendId !== 'real') { - setModelOptions([]) - setCatalogLoading(false) - return - } - if (!routerAvailable) { - setModelOptions([]) - setCatalogLoading(false) - return - } - setCatalogLoading(true) - try { - const rows = await fetchModelsCatalog() - setModelOptions(catalogRowsToModelOptions(rows)) - } catch { - setModelOptions([]) - } finally { - setCatalogLoading(false) - } - }, [backendId, routerAvailable]) + const refresh = useCallback( + async (force = false) => { + const requestVersion = ++catalogRequestVersion.current + const revisionAtStart = routerRevision + if (backendId !== 'real') { + setModelOptions([]) + setCatalogLoading(false) + return + } + if (!routerAvailable && !force) { + setModelOptions([]) + setCatalogLoading(false) + return + } + setCatalogLoading(true) + try { + const rows = await fetchModelsCatalog() + if ( + catalogRequestVersion.current === requestVersion && + routerRevisionRef.current === revisionAtStart + ) { + setModelOptions(catalogRowsToModelOptions(rows)) + } + } catch { + // A timeout or a reconnect race is not evidence that the configured + // catalogue became empty. Preserve the last good snapshot; a successful + // empty response above still clears it authoritatively. + } finally { + if (catalogRequestVersion.current === requestVersion) { + setCatalogLoading(false) + } + } + }, + [backendId, routerAvailable, routerRevision], + ) useEffect(() => { void refresh() @@ -84,6 +102,8 @@ export function useModelPickerSource( // Re-read `router::provider::list`, dropping the result if a newer provider // event (or a newer snapshot) has advanced the version since we started. const refreshProviders = useCallback(async () => { + const requestVersion = ++providerRequestVersion.current + const revisionAtStart = routerRevision if (backendId !== 'real' || !routerAvailable) { setPresentProviders([]) return @@ -91,15 +111,17 @@ export function useModelPickerSource( const snapshotVersion = providerEventVersion.current try { const providers = await fetchProviderList() - if (providerEventVersion.current === snapshotVersion) { + if ( + providerEventVersion.current === snapshotVersion && + providerRequestVersion.current === requestVersion && + routerRevisionRef.current === revisionAtStart + ) { setPresentProviders(providers) } } catch { - if (providerEventVersion.current === snapshotVersion) { - setPresentProviders([]) - } + // Preserve the last authoritative provider snapshot on transport errors. } - }, [backendId, routerAvailable]) + }, [backendId, routerAvailable, routerRevision]) // Initial snapshot (re-run when the router (re)appears). Availability flips // are applied from `router::provider::changed`; an event for a provider the @@ -173,29 +195,6 @@ export function useModelPickerSource( } }, [backendId, routerAvailable, refresh, refreshProviders]) - // A WebSocket drop loses any change events fired while disconnected; - // re-pull both reads when the connection comes back. - useEffect(() => { - if (backendId !== 'real' || !routerAvailable) return - let disposed = false - let offConn: (() => void) | null = null - getIiiClient() - .then((client) => { - if (disposed) return - offConn = client.addConnectionStateListener((state) => { - if (state === 'connected') { - void refresh() - void refreshProviders() - } - }) - }) - .catch(() => {}) - return () => { - disposed = true - offConn?.() - } - }, [backendId, routerAvailable, refresh, refreshProviders]) - useEffect(() => { if (backendId !== 'real' || !routerAvailable) return return onHarnessConfigSaved(() => { diff --git a/console/web/src/hooks/use-worker-presence.test.ts b/console/web/src/hooks/use-worker-presence.test.ts new file mode 100644 index 000000000..0298b411d --- /dev/null +++ b/console/web/src/hooks/use-worker-presence.test.ts @@ -0,0 +1,229 @@ +import { describe, expect, it, vi } from 'vitest' +import type { + IIIConnectionState, + IiiClient, + RegisterTriggerInput, +} from '@/lib/iii-client' +import { + createWorkerPresenceWatcher, + type WorkerPresenceWatcher, +} from './use-worker-presence' + +interface WorkerRow { + id: string + name: string +} + +function fakeClient(initialWorkers: WorkerRow[] = []) { + let workers = initialWorkers + const handlers = new Map void | Promise>() + const connectionHandlers = new Set<(state: IIIConnectionState) => void>() + const triggerRegistrations: RegisterTriggerInput[] = [] + const offHandler = vi.fn() + const offTrigger = vi.fn() + const offConnection = vi.fn() + + const client: IiiClient = { + browserId: 'presence-browser', + trigger: vi.fn(async (functionId: string) => { + if (functionId !== 'engine::workers::list') { + throw new Error(`unexpected function: ${functionId}`) + } + return { workers } + }) as IiiClient['trigger'], + on: vi.fn((functionId, handler) => { + handlers.set(functionId, handler) + return () => { + handlers.delete(functionId) + offHandler() + } + }), + registerTrigger: vi.fn((input) => { + triggerRegistrations.push(input) + return offTrigger + }), + addConnectionStateListener: vi.fn((handler) => { + connectionHandlers.add(handler) + return () => { + connectionHandlers.delete(handler) + offConnection() + } + }), + dispose: vi.fn(async () => {}), + } + + return { + client, + handlers, + triggerRegistrations, + offHandler, + offTrigger, + offConnection, + setWorkers(next: WorkerRow[]) { + workers = next + }, + connect() { + for (const handler of connectionHandlers) { + handler('connected') + } + }, + } +} + +function watcher( + client: IiiClient, + onChange: (state: { + present: boolean + loading: boolean + revision: number + }) => void, +): WorkerPresenceWatcher { + return createWorkerPresenceWatcher({ + client, + workerName: 'llm-router', + localFnId: 'console::llm-router-watch::presence::test', + initial: { present: false, loading: true, revision: 0 }, + onChange, + }) +} + +async function settle(): Promise { + await Promise.resolve() + await Promise.resolve() +} + +describe('worker presence watcher', () => { + it('re-probes while absent when the browser reconnects', async () => { + const fake = fakeClient() + const states: Array<{ + present: boolean + loading: boolean + revision: number + }> = [] + const presence = watcher(fake.client, (state) => states.push(state)) + + await expect(presence.refresh()).resolves.toBe(false) + expect(states.at(-1)).toEqual({ + present: false, + loading: false, + revision: 0, + }) + + fake.setWorkers([{ id: 'router-b', name: 'llm-router' }]) + fake.connect() + await settle() + + expect(states.at(-1)).toEqual({ + present: true, + loading: false, + revision: 1, + }) + }) + + it('uses engine worker catalogue ticks to detect a present-to-present restart', async () => { + const fake = fakeClient([{ id: 'router-a', name: 'llm-router' }]) + const states: Array<{ + present: boolean + loading: boolean + revision: number + }> = [] + const presence = watcher(fake.client, (state) => states.push(state)) + + await presence.refresh() + expect(states.at(-1)?.revision).toBe(1) + expect(fake.triggerRegistrations).toEqual([ + { + type: 'engine::workers-available', + function_id: + 'console::llm-router-watch::presence::test::presence-browser', + config: {}, + }, + ]) + + fake.setWorkers([{ id: 'router-b', name: 'llm-router' }]) + await fake.handlers.get('console::llm-router-watch::presence::test')?.({}) + await settle() + + expect(states.at(-1)).toEqual({ + present: true, + loading: false, + revision: 2, + }) + }) + + it('drops an older presence response after a newer probe wins', async () => { + let resolveFirst: ((value: { workers: WorkerRow[] }) => void) | undefined + const firstResponse = new Promise<{ workers: WorkerRow[] }>((resolve) => { + resolveFirst = resolve + }) + const fake = fakeClient() + vi.mocked(fake.client.trigger) + .mockImplementationOnce(() => firstResponse) + .mockResolvedValueOnce({ + workers: [{ id: 'router-new', name: 'llm-router' }], + }) + const states: Array<{ + present: boolean + loading: boolean + revision: number + }> = [] + const presence = watcher(fake.client, (state) => states.push(state)) + + const oldProbe = presence.refresh() + await expect(presence.refresh()).resolves.toBe(true) + resolveFirst?.({ workers: [] }) + await expect(oldProbe).resolves.toBe(false) + + expect(states.at(-1)).toEqual({ + present: true, + loading: false, + revision: 1, + }) + }) + + it('does not let an in-flight probe undo a lifecycle removal', async () => { + let resolveProbe: ((value: { workers: WorkerRow[] }) => void) | undefined + const probeResponse = new Promise<{ workers: WorkerRow[] }>((resolve) => { + resolveProbe = resolve + }) + const fake = fakeClient() + vi.mocked(fake.client.trigger).mockImplementationOnce(() => probeResponse) + const states: Array<{ + present: boolean + loading: boolean + revision: number + }> = [] + const presence = watcher(fake.client, (state) => states.push(state)) + + const probe = presence.refresh() + presence.markAbsent() + resolveProbe?.({ workers: [{ id: 'router-old', name: 'llm-router' }] }) + await expect(probe).resolves.toBe(true) + + expect(states.at(-1)).toEqual({ + present: false, + loading: false, + revision: 0, + }) + }) + + it('forces a consumer revision on reconnect and disposes every binding', async () => { + const fake = fakeClient([{ id: 'router-a', name: 'llm-router' }]) + const states: Array<{ + present: boolean + loading: boolean + revision: number + }> = [] + const presence = watcher(fake.client, (state) => states.push(state)) + + await presence.refresh() + fake.connect() + await settle() + expect(states.at(-1)?.revision).toBe(2) + + presence.dispose() + expect(fake.offConnection).toHaveBeenCalledOnce() + expect(fake.offTrigger).toHaveBeenCalledOnce() + expect(fake.offHandler).toHaveBeenCalledOnce() + }) +}) diff --git a/console/web/src/hooks/use-worker-presence.ts b/console/web/src/hooks/use-worker-presence.ts index 3c8243fb7..427f9e0bb 100644 --- a/console/web/src/hooks/use-worker-presence.ts +++ b/console/web/src/hooks/use-worker-presence.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useState } from 'react' +import { useCallback, useEffect, useId, useMemo, useRef, useState } from 'react' import { z } from 'zod' import { getIiiClient, type IiiClient } from '@/lib/iii-client' import { useWorkerLifecycle } from './use-worker-lifecycle' @@ -9,11 +9,15 @@ import { useWorkerLifecycle } from './use-worker-lifecycle' * can gate worker-specific UI + RPC on it and never trigger "function not * found". * - * Presence is fed by two signals, mirroring the harness probe: + * Presence is reconciled from four signals: * 1. An initial `engine::workers::list` read on mount. - * 2. A real-time `worker` add/remove lifecycle trigger, so the UI reacts the + * 2. `engine::workers-available`, which covers raw process connections and + * disconnects (including crashes/restarts outside the worker manager). + * 3. A real-time `worker` add/remove lifecycle trigger, so the UI reacts the * instant the worker is added or removed — whether from the CLI * (`iii worker add `) or another surface. + * 4. Browser WebSocket reconnect, which re-reads the authoritative snapshot + * because events fired during the outage are not replayed. * * Each consumer MUST pass a unique `watchFnId` so its browser-local handler for * the `worker` trigger does not collide with another presence probe's. @@ -24,6 +28,26 @@ export interface WorkerPresence { present: boolean /** Initial presence probe in flight. */ loading: boolean + /** + * Advances whenever a present worker instance changes or the browser + * reconnects. Consumers that own worker-scoped trigger bindings include it + * in their effect dependencies so those bindings are recreated even across + * a present→present restart. + */ + revision: number + /** Authoritatively re-read presence; used by manual recovery controls. */ + refresh: () => Promise +} + +interface WorkerPresenceState { + present: boolean + loading: boolean + revision: number +} + +interface WorkerSnapshot { + present: boolean + workerId: string | null } const workerEventSchema = z.object({ @@ -61,13 +85,149 @@ function eventMatchesWorker(evt: WorkerEvent, workerName: string): boolean { async function checkWorkerPresent( client: IiiClient, name: string, -): Promise { - const res = await client.trigger<{ workers?: Array<{ name?: unknown }> }>( - 'engine::workers::list', - {}, - ) +): Promise { + const res = await client.trigger<{ + workers?: Array<{ id?: unknown; name?: unknown }> + }>('engine::workers::list', {}) const workers = Array.isArray(res?.workers) ? res.workers : [] - return workers.some((w) => w?.name === name) + const worker = workers.find((w) => w?.name === name) + if (!worker) return { present: false, workerId: null } + return { + present: true, + // Current engines expose an instance id. The name fallback preserves + // compatibility with older engines, which can still detect absent→present. + workerId: typeof worker.id === 'string' ? worker.id : name, + } +} + +export interface WorkerPresenceWatcher { + refresh(options?: { forceRevision?: boolean }): Promise + markAbsent(): void + dispose(): void +} + +interface CreateWorkerPresenceWatcherOptions { + client: IiiClient + workerName: string + localFnId: string + initial: WorkerPresenceState + onChange: (state: WorkerPresenceState) => void +} + +/** + * Imperative presence state machine kept outside React so reconnect, worker + * catalogue ticks, stale async reads, and cleanup can be tested together. + */ +export function createWorkerPresenceWatcher({ + client, + workerName, + localFnId, + initial, + onChange, +}: CreateWorkerPresenceWatcherOptions): WorkerPresenceWatcher { + let state = { ...initial, workerId: null as string | null } + let generation = 0 + let disposed = false + let offHandler: (() => void) | undefined + let offTrigger: (() => void) | undefined + let offConnection: (() => void) | undefined + + const publish = (snapshot: WorkerSnapshot, forceRevision: boolean) => { + if (disposed) return + const identityChanged = + snapshot.present && + (!state.present || state.workerId !== snapshot.workerId) + const revision = + state.revision + + (snapshot.present && (forceRevision || identityChanged) ? 1 : 0) + const next = { + present: snapshot.present, + loading: false, + revision, + workerId: snapshot.workerId, + } + const changed = + next.present !== state.present || + next.loading !== state.loading || + next.revision !== state.revision || + next.workerId !== state.workerId + state = next + if (changed) { + onChange({ + present: state.present, + loading: state.loading, + revision: state.revision, + }) + } + } + + const refresh: WorkerPresenceWatcher['refresh'] = async (options = {}) => { + const requestGeneration = ++generation + try { + const snapshot = await checkWorkerPresent(client, workerName) + if (!disposed && requestGeneration === generation) { + publish(snapshot, options.forceRevision === true) + } + return snapshot.present + } catch { + if (!disposed && requestGeneration === generation) { + publish({ present: false, workerId: null }, false) + } + return false + } + } + + const markAbsent = () => { + generation += 1 + publish({ present: false, workerId: null }, false) + } + + // Engine-owned connection signal: unlike the worker-manager `worker` + // lifecycle, this fires for raw process crashes/restarts as well as CLI + // add/remove operations. Re-read the authoritative list instead of trusting + // an event payload shape. + try { + offHandler = client.on(localFnId, () => { + void refresh() + }) + offTrigger = client.registerTrigger({ + type: 'engine::workers-available', + function_id: `${localFnId}::${client.browserId}`, + config: {}, + }) + } catch { + offTrigger?.() + offHandler?.() + offTrigger = undefined + offHandler = undefined + } + + // This listener is deliberately active even while the target worker is + // absent. Events fired during the WebSocket outage are not replayed, so a + // successful reconnect must force both a presence read and consumer + // re-subscription. + try { + offConnection = client.addConnectionStateListener((connectionState) => { + if (connectionState === 'connected') { + void refresh({ forceRevision: true }) + } + }) + } catch { + offConnection = undefined + } + + return { + refresh, + markAbsent, + dispose: () => { + if (disposed) return + disposed = true + generation += 1 + offConnection?.() + offTrigger?.() + offHandler?.() + }, + } } export interface UseWorkerPresenceOptions { @@ -87,8 +247,16 @@ export function useWorkerPresence({ watchFnId, enabled, }: UseWorkerPresenceOptions): WorkerPresence { - const [present, setPresent] = useState(!enabled) - const [loading, setLoading] = useState(enabled) + const [status, setStatus] = useState({ + present: !enabled, + loading: enabled, + revision: 0, + }) + const watcherRef = useRef(null) + const refreshRef = useRef<() => Promise>(async () => !enabled) + const instanceId = useId().replace(/[^a-zA-Z0-9]/g, '') + + const refresh = useCallback(() => refreshRef.current(), []) // Live add/remove so installing or dropping the worker mid-session flips the // UI without a reload. `workerName` is a stable literal per consumer, so this @@ -103,8 +271,11 @@ export function useWorkerPresence({ ) { return } - if (evt.operation === 'add') setPresent(true) - else if (evt.operation === 'remove') setPresent(false) + if (evt.operation === 'add') { + void watcherRef.current?.refresh({ forceRevision: true }) + } else if (evt.operation === 'remove') { + watcherRef.current?.markAbsent() + } }, [workerName], ) @@ -116,32 +287,47 @@ export function useWorkerPresence({ onEvent: handleEvent, }) - // One-time presence probe on mount. Live changes arrive through the `worker` - // trigger above; no polling. + // Initial snapshot plus engine-owned worker-catalogue and browser reconnect + // signals. No polling. useEffect(() => { if (!enabled) { - setLoading(false) - setPresent(true) + watcherRef.current = null + refreshRef.current = async () => true + setStatus({ present: true, loading: false, revision: 0 }) return } + let cancelled = false - void (async () => { - const client = await getIiiClient() - try { - const found = await checkWorkerPresent(client, workerName) - if (!cancelled) setPresent(found) - } catch { - if (!cancelled) setPresent(false) - } finally { - if (!cancelled) setLoading(false) - } - })() + setStatus({ present: false, loading: true, revision: 0 }) + void getIiiClient() + .then((client) => { + if (cancelled) return + const watcher = createWorkerPresenceWatcher({ + client, + workerName, + localFnId: `${watchFnId}::presence::${instanceId}`, + initial: { present: false, loading: true, revision: 0 }, + onChange: setStatus, + }) + watcherRef.current = watcher + refreshRef.current = () => watcher.refresh({ forceRevision: true }) + void watcher.refresh() + }) + .catch(() => { + if (!cancelled) { + setStatus({ present: false, loading: false, revision: 0 }) + } + }) + return () => { cancelled = true + watcherRef.current?.dispose() + watcherRef.current = null + refreshRef.current = async () => false } - }, [enabled, workerName]) + }, [enabled, instanceId, watchFnId, workerName]) - return useMemo(() => ({ present, loading }), [present, loading]) + return useMemo(() => ({ ...status, refresh }), [status, refresh]) } /** diff --git a/console/web/src/hooks/use-worktree-status.test.ts b/console/web/src/hooks/use-worktree-status.test.ts index d8d045b65..a2504745b 100644 --- a/console/web/src/hooks/use-worktree-status.test.ts +++ b/console/web/src/hooks/use-worktree-status.test.ts @@ -14,8 +14,30 @@ describe('worktree presence probe wiring', () => { }) it('gates on both presence and the initial probe settling', () => { - expect(isWorktreeAvailable({ present: true, loading: false })).toBe(true) - expect(isWorktreeAvailable({ present: true, loading: true })).toBe(false) - expect(isWorktreeAvailable({ present: false, loading: false })).toBe(false) + const refresh = async () => true + expect( + isWorktreeAvailable({ + present: true, + loading: false, + revision: 1, + refresh, + }), + ).toBe(true) + expect( + isWorktreeAvailable({ + present: true, + loading: true, + revision: 1, + refresh, + }), + ).toBe(false) + expect( + isWorktreeAvailable({ + present: false, + loading: false, + revision: 1, + refresh, + }), + ).toBe(false) }) }) diff --git a/console/web/src/lib/conversations-context.tsx b/console/web/src/lib/conversations-context.tsx index e9d814ddc..5872b496f 100644 --- a/console/web/src/lib/conversations-context.tsx +++ b/console/web/src/lib/conversations-context.tsx @@ -31,6 +31,7 @@ import { import type { ChatBackend } from '@/lib/backend' import { getDefaultBackend } from '@/lib/backend' import { + fetchProviderList, type ProviderListEntry, refreshProviderModels, } from '@/lib/models-catalog' @@ -107,9 +108,8 @@ export function ConversationsProvider({ const harnessStatus = useHarnessStatus(backend.id === 'real') // The model picker reads router-owned RPCs; gate them on llm-router, not // the harness — the harness being slow or absent must not blank the picker. - const routerAvailable = isLlmRouterAvailable( - useLlmRouterStatus(backend.id === 'real'), - ) + const routerStatus = useLlmRouterStatus(backend.id === 'real') + const routerAvailable = isLlmRouterAvailable(routerStatus) const approvalGateAvailable = isApprovalGateAvailable( useApprovalGateStatus(backend.id === 'real'), ) @@ -126,7 +126,7 @@ export function ConversationsProvider({ catalogLoading, presentProviders, refresh, - } = useModelPickerSource(backend.id, routerAvailable) + } = useModelPickerSource(backend.id, routerAvailable, routerStatus.revision) // Conversations are backed by the session-manager worker on the real // backend; mocks stay in-memory. const api = useConversations( @@ -137,23 +137,30 @@ export function ConversationsProvider({ const [refreshingModels, setRefreshingModels] = useState(false) const refreshModels = useCallback(async () => { - if (!routerAvailable) return + // A manual refresh is also an escape hatch for stale presence. The hook's + // state update is asynchronous, so force the catalogue read below after + // the authoritative probe succeeds instead of waiting for another render. + const available = (await routerStatus.refresh()) || routerAvailable + if (!available) return setRefreshingModels(true) try { if (backend.id === 'real') { // Refresh the present providers that can list models. With no present // providers this is a no-op for discovery; the catalog re-read below // still runs. - const ids = presentProviders + const providers = await fetchProviderList().catch( + () => presentProviders, + ) + const ids = providers .filter((p) => p.supports_model_listing) .map((p) => p.id) await refreshProviderModels(ids) } - await refresh() + await refresh(true) } finally { setRefreshingModels(false) } - }, [routerAvailable, refresh, presentProviders]) + }, [routerStatus, routerAvailable, refresh, presentProviders]) const value: ConversationsContextValue = { ...api, diff --git a/harness/tests/quickstart/README.md b/harness/tests/quickstart/README.md index 5247f173f..f69fbb9e2 100644 --- a/harness/tests/quickstart/README.md +++ b/harness/tests/quickstart/README.md @@ -25,6 +25,12 @@ It verifies that: - in a separate chat, a user can invoke one real `shell::exec` capability, observe its successful Console card and exact output, and recover the same conversation after a reload; +- restarting only the `llm-router` worker changes its engine worker id while + the browser document stays mounted, and the model picker recovers without a + page reload; +- when `llm-router` starts while only the browser WebSocket is disconnected, + reconnecting the same document re-probes presence and repopulates the model + picker without a page reload; - the capability call and its exact output are present in the durable session transcript; and - `config.yaml` and `iii.lock` contain the installed workers. diff --git a/harness/tests/quickstart/console-router-recovery.spec.ts b/harness/tests/quickstart/console-router-recovery.spec.ts new file mode 100644 index 000000000..41fdae46a --- /dev/null +++ b/harness/tests/quickstart/console-router-recovery.spec.ts @@ -0,0 +1,230 @@ +import { execFile } from 'node:child_process' +import { mkdir, writeFile } from 'node:fs/promises' +import path from 'node:path' +import { promisify } from 'node:util' +import { expect, type Page, test, type WebSocketRoute } from '@playwright/test' + +const execFileAsync = promisify(execFile) +const SONNET_LABEL = /claude[\s-]+sonnet[\s-]+5/i + +function required(name: string): string { + const value = process.env[name] + if (!value) throw new Error(`${name} is required`) + return value +} + +async function iiiJson(args: string[]): Promise> { + const iiiBin = required('HARNESS_QUICKSTART_III_BIN') + const projectDir = required('HARNESS_QUICKSTART_PROJECT_DIR') + const { stdout } = await execFileAsync(iiiBin, args, { + cwd: projectDir, + env: process.env, + maxBuffer: 4 * 1024 * 1024, + }) + return JSON.parse(stdout) as Record +} + +async function routerId(): Promise { + const port = required('HARNESS_QUICKSTART_ENGINE_PORT') + const result = await iiiJson([ + 'trigger', + 'engine::workers::list', + '--port', + port, + '--json', + '{}', + ]) + const workers = Array.isArray(result.workers) ? result.workers : [] + const router = workers.find( + (worker) => + worker && + typeof worker === 'object' && + (worker as Record).name === 'llm-router', + ) as Record | undefined + return typeof router?.id === 'string' ? router.id : null +} + +async function modelCount(): Promise { + const port = required('HARNESS_QUICKSTART_ENGINE_PORT') + const result = await iiiJson([ + 'trigger', + 'router::models::list', + '--port', + port, + '--json', + '{}', + ]) + return Array.isArray(result.models) ? result.models.length : 0 +} + +async function expectSonnetInPicker( + page: Page, + timeout = 30_000, +): Promise { + const picker = page.getByRole('button', { name: /^model(?::|\s|$)/i }) + await expect(picker).toBeEnabled({ timeout }) + await picker.click() + + const anthropic = page.getByRole('menuitem', { name: /^anthropic/i }) + await expect(anthropic).toBeVisible() + if ((await anthropic.getAttribute('aria-expanded')) !== 'true') { + await anthropic.click() + } + await expect( + page.getByRole('menuitemradio', { name: SONNET_LABEL }), + ).toHaveCount(1, { timeout }) + await page.keyboard.press('Escape') +} + +async function runWorkerCommand(command: 'restart' | 'start' | 'stop') { + const iiiBin = required('HARNESS_QUICKSTART_III_BIN') + const projectDir = required('HARNESS_QUICKSTART_PROJECT_DIR') + await execFileAsync(iiiBin, ['worker', command, 'llm-router'], { + cwd: projectDir, + env: process.env, + timeout: 120_000, + maxBuffer: 4 * 1024 * 1024, + }) +} + +test('recovers the model catalogue after the router is replaced without reloading the page', async ({ + page, +}) => { + const consoleUrl = required('HARNESS_QUICKSTART_CONSOLE_URL') + const artifactsRoot = required('HARNESS_QUICKSTART_ARTIFACTS_DIR') + + await page.goto(consoleUrl) + const chatTab = page.getByRole('tab', { name: /^chat \+ traces/i }) + await chatTab.click() + await expect(chatTab).toHaveAttribute('aria-selected', 'true') + await expectSonnetInPicker(page) + + const documentMarker = 'quickstart-router-recovery-document' + await page.evaluate((marker) => { + Reflect.set(window, '__quickstartRouterRecovery', marker) + }, documentMarker) + + const beforeId = await routerId() + expect(beforeId).toBeTruthy() + + // Restart only llm-router. The engine, Console worker, browser + // document, and browser-to-engine WebSocket all remain alive. + await runWorkerCommand('restart') + + await expect + .poll(routerId, { timeout: 60_000, intervals: [250, 500, 1_000] }) + .not.toBe(beforeId) + const afterId = await routerId() + expect(afterId).toBeTruthy() + + await expect + .poll(modelCount, { timeout: 60_000, intervals: [250, 500, 1_000] }) + .toBeGreaterThan(0) + await expectSonnetInPicker(page) + + await expect( + page.evaluate(() => Reflect.get(window, '__quickstartRouterRecovery')), + ).resolves.toBe(documentMarker) + + await mkdir(artifactsRoot, { recursive: true }) + await writeFile( + path.join(artifactsRoot, 'router-recovery-browser-evidence.json'), + `${JSON.stringify( + { + schema_version: 1, + router_worker_id_before: beforeId, + router_worker_id_after: afterId, + backend_model_count: await modelCount(), + same_browser_document: true, + model_picker_recovered_without_reload: true, + }, + null, + 2, + )}\n`, + ) +}) + +test('recovers presence when the router starts during a browser WebSocket outage', async ({ + page, +}) => { + const consoleUrl = required('HARNESS_QUICKSTART_CONSOLE_URL') + const artifactsRoot = required('HARNESS_QUICKSTART_ARTIFACTS_DIR') + let allowBrowserConnection = true + let browserSocket: WebSocketRoute | null = null + + await page.routeWebSocket('**/ws', (socket) => { + browserSocket = socket + if (allowBrowserConnection) { + socket.connectToServer() + } else { + void socket.close({ code: 1001, reason: 'quickstart controlled outage' }) + } + }) + + await page.goto(consoleUrl) + const chatTab = page.getByRole('tab', { name: /^chat \+ traces/i }) + await chatTab.click() + await expect(chatTab).toHaveAttribute('aria-selected', 'true') + await expectSonnetInPicker(page) + + const documentMarker = 'quickstart-presence-reconnect-document' + await page.evaluate((marker) => { + Reflect.set(window, '__quickstartPresenceReconnect', marker) + }, documentMarker) + + await runWorkerCommand('stop') + await expect + .poll(routerId, { timeout: 30_000, intervals: [250, 500, 1_000] }) + .toBeNull() + await expect( + page.getByRole('button', { name: /^model(?::|\s|$)/i }), + ).toBeDisabled() + + allowBrowserConnection = false + const activeSocket = browserSocket as WebSocketRoute | null + if (!activeSocket) + throw new Error('Console did not open its engine WebSocket') + await activeSocket.close({ + code: 1001, + reason: 'quickstart controlled outage', + }) + + try { + // The router becomes healthy while the browser cannot receive its worker + // arrival event. Reconnect must therefore re-read workers::list. + await runWorkerCommand('start') + await expect + .poll(routerId, { timeout: 60_000, intervals: [250, 500, 1_000] }) + .not.toBeNull() + await expect + .poll(modelCount, { timeout: 60_000, intervals: [250, 500, 1_000] }) + .toBeGreaterThan(0) + + allowBrowserConnection = true + await expectSonnetInPicker(page, 60_000) + await expect( + page.evaluate(() => Reflect.get(window, '__quickstartPresenceReconnect')), + ).resolves.toBe(documentMarker) + + await mkdir(artifactsRoot, { recursive: true }) + await writeFile( + path.join(artifactsRoot, 'presence-reconnect-browser-evidence.json'), + `${JSON.stringify( + { + schema_version: 1, + router_started_while_browser_disconnected: true, + backend_model_count: await modelCount(), + same_browser_document: true, + model_picker_recovered_without_reload: true, + }, + null, + 2, + )}\n`, + ) + } finally { + allowBrowserConnection = true + if ((await routerId().catch(() => null)) === null) { + await runWorkerCommand('start').catch(() => undefined) + } + } +}) diff --git a/harness/tests/quickstart/playwright.config.ts b/harness/tests/quickstart/playwright.config.ts index 3ca8e204a..06e630c11 100644 --- a/harness/tests/quickstart/playwright.config.ts +++ b/harness/tests/quickstart/playwright.config.ts @@ -8,11 +8,15 @@ if (!artifactsRoot) { export default defineConfig({ testDir: __dirname, - testMatch: ['console-first-message.spec.ts', 'console-first-capability.spec.ts'], + testMatch: [ + 'console-first-message.spec.ts', + 'console-first-capability.spec.ts', + 'console-router-recovery.spec.ts', + ], fullyParallel: false, workers: 1, retries: 0, - timeout: 180_000, + timeout: 300_000, expect: { timeout: 30_000 }, reporter: [['list']], outputDir: path.join(artifactsRoot, 'playwright-output'), diff --git a/harness/tests/quickstart/run-ci.sh b/harness/tests/quickstart/run-ci.sh index 9c1915961..3341ad951 100755 --- a/harness/tests/quickstart/run-ci.sh +++ b/harness/tests/quickstart/run-ci.sh @@ -121,6 +121,8 @@ rm -f \ "$artifact_dir/terminal-status.json" \ "$artifact_dir/first-capability-browser-evidence.json" \ "$artifact_dir/first-capability-evidence.json" \ + "$artifact_dir/router-recovery-browser-evidence.json" \ + "$artifact_dir/presence-reconnect-browser-evidence.json" \ "$artifact_dir/config.yaml" \ "$artifact_dir/iii.lock" \ "$artifact_dir/commands.log" \ @@ -167,13 +169,15 @@ die() { write_result() { local status=$1 - local browser=null terminal=null first_capability_browser=null first_capability_durable=null + local browser=null terminal=null first_capability_browser=null first_capability_durable=null router_recovery=null presence_reconnect=null local video=null local video_path="$artifact_dir/slack-evidence/quickstart-provider-switch.mp4" [[ -f "$artifact_dir/browser-evidence.json" ]] && browser=$(jq -c . "$artifact_dir/browser-evidence.json") [[ -f "$artifact_dir/terminal-status.json" ]] && terminal=$(jq -c . "$artifact_dir/terminal-status.json") [[ -f "$artifact_dir/first-capability-browser-evidence.json" ]] && first_capability_browser=$(jq -c . "$artifact_dir/first-capability-browser-evidence.json") [[ -f "$artifact_dir/first-capability-evidence.json" ]] && first_capability_durable=$(jq -c . "$artifact_dir/first-capability-evidence.json") + [[ -f "$artifact_dir/router-recovery-browser-evidence.json" ]] && router_recovery=$(jq -c . "$artifact_dir/router-recovery-browser-evidence.json") + [[ -f "$artifact_dir/presence-reconnect-browser-evidence.json" ]] && presence_reconnect=$(jq -c . "$artifact_dir/presence-reconnect-browser-evidence.json") if [[ -f "$video_path" ]]; then video=$(jq -n \ --arg path "slack-evidence/quickstart-provider-switch.mp4" \ @@ -196,6 +200,8 @@ write_result() { --argjson terminal "$terminal" \ --argjson first_capability_browser "$first_capability_browser" \ --argjson first_capability_durable "$first_capability_durable" \ + --argjson router_recovery "$router_recovery" \ + --argjson presence_reconnect "$presence_reconnect" \ --argjson slack_evidence "$video" \ --argjson elapsed_ms "$(((SECONDS - started_at_seconds) * 1000))" \ --argjson engine_port "$engine_port" \ @@ -217,6 +223,8 @@ write_result() { browser: $first_capability_browser, durable: $first_capability_durable }, + router_recovery: $router_recovery, + presence_reconnect: $presence_reconnect, slack_evidence: $slack_evidence, elapsed_ms: $elapsed_ms, engine_port: $engine_port @@ -385,6 +393,9 @@ record_browser_video() { local playwright_status video_source local output_dir="$artifact_dir/slack-evidence" export HARNESS_QUICKSTART_CONSOLE_URL="http://127.0.0.1:$console_port/" + export HARNESS_QUICKSTART_III_BIN="$iii_bin" + export HARNESS_QUICKSTART_PROJECT_DIR="$project_dir" + export HARNESS_QUICKSTART_ENGINE_PORT="$engine_port" export NODE_PATH="$repo_root/console/web/node_modules${NODE_PATH:+:$NODE_PATH}" set +e "$playwright_bin" test \ @@ -406,7 +417,7 @@ record_browser_video() { fi ((playwright_status == 0)) || die "Console quickstart Playwright tests failed" - ok "Console provider switch and first capability completed with recorded evidence" + ok "Console provider switch, router recovery, and first capability completed with recorded evidence" } wait_for_terminal_turns() {