From 382b26806e6fa060a38f16e04a84d3ea6c3beea5 Mon Sep 17 00:00:00 2001 From: Chris Scott <> Date: Tue, 20 Oct 2026 21:01:16 +0000 Subject: [PATCH 1/3] fix(events): clear stale pending questions from remote servers - Drop pending forms and permissions when a session is deleted or its page returns 404 - Reconcile remembered remote directories on browser reconnect, not only local repos - Purge pending items from remote servers that are disabled or removed - Prefer the viewed session's pending item over a pinned item from another session - Time out remote proxy GET/HEAD requests that never send headers (504) --- backend/src/routes/opencode-server-proxy.ts | 29 +++- .../test/routes/opencode-server-proxy.test.ts | 65 +++++++- frontend/src/contexts/EventContext.test.tsx | 139 +++++++++++++++++- frontend/src/contexts/EventContext.tsx | 69 +++++++-- frontend/src/pages/SessionDetail.tsx | 8 +- .../SessionDetail.form-prompt.test.tsx | 17 +++ .../__tests__/SessionDetail.remote.test.tsx | 1 + 7 files changed, 307 insertions(+), 21 deletions(-) diff --git a/backend/src/routes/opencode-server-proxy.ts b/backend/src/routes/opencode-server-proxy.ts index b86041b3d..38c68b310 100644 --- a/backend/src/routes/opencode-server-proxy.ts +++ b/backend/src/routes/opencode-server-proxy.ts @@ -12,6 +12,7 @@ import { logger } from '../utils/logger' const MOUNT_PREFIX = '/api/opencode/servers/' const REQUEST_EXCLUDED_HEADERS = new Set(['cookie', 'origin', 'referer']) const RESPONSE_EXCLUDED_HEADERS = new Set(['set-cookie']) +const UPSTREAM_HEADERS_TIMEOUT_MS = 30_000 type UpstreamPath = { ok: true; path: string } | { ok: false; status: 400 | 404 } @@ -82,6 +83,7 @@ export function createOpenCodeServerProxyRoutes( registry: OpenCodeServerRegistry, requireAuth: MiddlewareHandler, fetchFn: typeof fetch = fetch, + upstreamHeadersTimeoutMs: number = UPSTREAM_HEADERS_TIMEOUT_MS, ): Hono { const app = new Hono() @@ -117,16 +119,33 @@ export function createOpenCodeServerProxyRoutes( const hasBody = c.req.method !== 'GET' && c.req.method !== 'HEAD' + let headersTimeoutTimer: ReturnType | null = null + let headersTimedOut = false + let upstreamSignal = c.req.raw.signal + if (!hasBody) { + const timeoutController = new AbortController() + headersTimeoutTimer = setTimeout(() => { + headersTimedOut = true + timeoutController.abort() + }, upstreamHeadersTimeoutMs) + upstreamSignal = AbortSignal.any([c.req.raw.signal, timeoutController.signal]) + } + try { const upstreamResponse = await fetchFn(upstream.url.toString(), { method: c.req.method, headers, body: hasBody ? c.req.raw.body : undefined, redirect: 'manual', - signal: c.req.raw.signal, + signal: upstreamSignal, duplex: 'half', }) + if (headersTimeoutTimer !== null) { + clearTimeout(headersTimeoutTimer) + headersTimeoutTimer = null + } + const responseHeaders = new Headers( filterProxyHeaders(upstreamResponse.headers, RESPONSE_EXCLUDED_HEADERS), ) @@ -140,8 +159,16 @@ export function createOpenCodeServerProxyRoutes( headers: responseHeaders, }) } catch (error) { + if (headersTimedOut) { + logger.warn(`OpenCode server proxy timed out waiting for headers: ${c.req.method} ${upstream.url.host}${upstreamPath.path}`, error) + return c.json({ error: 'Remote OpenCode server timed out' }, 504) + } logger.warn(`OpenCode server proxy request failed: ${c.req.method} ${upstream.url.host}${upstreamPath.path}`, error) return c.json({ error: 'Remote OpenCode server unreachable' }, 502) + } finally { + if (headersTimeoutTimer !== null) { + clearTimeout(headersTimeoutTimer) + } } }) diff --git a/backend/test/routes/opencode-server-proxy.test.ts b/backend/test/routes/opencode-server-proxy.test.ts index fb7cea9ae..515bab26f 100644 --- a/backend/test/routes/opencode-server-proxy.test.ts +++ b/backend/test/routes/opencode-server-proxy.test.ts @@ -21,9 +21,13 @@ const passThroughAuth: MiddlewareHandler = async (_c, next) => { function buildApp( fetchFn: typeof fetch, requireAuth: MiddlewareHandler = passThroughAuth, + upstreamHeadersTimeoutMs?: number, ): Hono { const app = new Hono() - app.route('/api/opencode/servers', createOpenCodeServerProxyRoutes(registry, requireAuth, fetchFn)) + app.route( + '/api/opencode/servers', + createOpenCodeServerProxyRoutes(registry, requireAuth, fetchFn, upstreamHeadersTimeoutMs), + ) return app } @@ -200,6 +204,65 @@ describe('opencode server proxy routes', () => { expect(await res.json()).toEqual({ error: 'Remote OpenCode server unreachable' }) }) + it('returns 504 when a GET upstream does not send headers before the timeout', async () => { + const fetchMock = vi.fn((_url: string, init: RequestInit) => new Promise((_resolve, reject) => { + init.signal?.addEventListener('abort', () => { + reject(new DOMException('Aborted', 'AbortError')) + }) + })) + const app = buildApp(fetchMock as unknown as typeof fetch, passThroughAuth, 20) + + const res = await app.request('/api/opencode/servers/srv_1/api/session') + + expect(res.status).toBe(504) + expect(await res.json()).toEqual({ error: 'Remote OpenCode server timed out' }) + }) + + it('does not time out a POST that takes longer than the headers timeout', async () => { + let receivedSignal: AbortSignal | null | undefined + const fetchMock = vi.fn(async (_url: string, init: RequestInit) => { + receivedSignal = init.signal + await new Promise((resolve) => setTimeout(resolve, 40)) + return new Response('ok', { status: 200 }) + }) + const app = buildApp(fetchMock as unknown as typeof fetch, passThroughAuth, 10) + + const res = await app.request('/api/opencode/servers/srv_1/api/session/ses_1/message', { + method: 'POST', + body: '{}', + }) + + expect(res.status).toBe(200) + expect(receivedSignal?.aborted).toBe(false) + }) + + it('keeps streaming the upstream body after headers arrive', async () => { + const encoder = new TextEncoder() + let receivedSignal: AbortSignal | null | undefined + const fetchMock = vi.fn(async (_url: string, init: RequestInit) => { + receivedSignal = init.signal + const stream = new ReadableStream({ + async start(controller) { + controller.enqueue(encoder.encode('data: one\n\n')) + await new Promise((resolve) => setTimeout(resolve, 40)) + controller.enqueue(encoder.encode('data: two\n\n')) + controller.close() + }, + }) + return new Response(stream, { + status: 200, + headers: { 'content-type': 'text/event-stream' }, + }) + }) + const app = buildApp(fetchMock as unknown as typeof fetch, passThroughAuth, 10) + + const res = await app.request('/api/opencode/servers/srv_1/api/event') + + expect(res.status).toBe(200) + expect(await res.text()).toBe('data: one\n\ndata: two\n\n') + expect(receivedSignal?.aborted).toBe(false) + }) + it('streams the upstream response body and drops set-cookie', async () => { const encoder = new TextEncoder() const stream = new ReadableStream({ diff --git a/frontend/src/contexts/EventContext.test.tsx b/frontend/src/contexts/EventContext.test.tsx index c9a0191f2..157b50cd8 100644 --- a/frontend/src/contexts/EventContext.test.tsx +++ b/frontend/src/contexts/EventContext.test.tsx @@ -25,6 +25,7 @@ const mocks = vi.hoisted(() => ({ getHealth: vi.fn(), listProjects: vi.fn(), servers: [] as Array<{ id: string; enabled: boolean }>, + serversLoaded: false, })) vi.mock('@/api/repos', () => ({ @@ -48,7 +49,7 @@ vi.mock('@/hooks/useAuth', () => ({ })) vi.mock('@/hooks/useOpenCodeServers', () => ({ - useOpenCodeServers: () => ({ servers: mocks.servers }), + useOpenCodeServers: () => ({ servers: mocks.servers, isSuccess: mocks.serversLoaded }), })) vi.mock('@/lib/opencode-event-stream', () => ({ @@ -244,6 +245,7 @@ describe('EventProvider permissions and forms', () => { mocks.listRepos.mockResolvedValue([]) mocks.listProjects.mockResolvedValue([]) mocks.servers = [] + mocks.serversLoaded = false mocks.listPendingPermissions.mockResolvedValue([]) mocks.listPendingForms.mockResolvedValue([]) mocks.listActiveSessions.mockResolvedValue({}) @@ -522,6 +524,34 @@ describe('EventProvider permissions and forms', () => { }) }) + it('reconciles a remembered remote directory on reconnect', async () => { + render(, { wrapper: createWrapper() }) + + await waitFor(() => expect(mocks.subscribeGlobalMonitor).toHaveBeenCalled()) + + const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1] + const onEvent = lastSubscribeCall[0].onEvent as (data: unknown) => void + const handleStatusChange = lastSubscribeCall[0].onStatusChange as (connected: boolean) => void + + act(() => { + onEvent({ type: 'form.created', data: { form: pendingForm }, directory: '/srv/app', serverId: 'srv_remote' }) + }) + + await waitFor(() => { + expect(screen.getByTestId('count')).toHaveTextContent('1') + }) + + act(() => { + handleStatusChange(true) + }) + + await waitFor(() => { + expect(mocks.listPendingForms).toHaveBeenCalledWith('/srv/app', 'srv_remote') + expect(screen.getByTestId('count')).toHaveTextContent('0') + expect(screen.getByTestId('current')).toHaveTextContent('none') + }) + }) + it('reconciles a linked server enabled after the browser stream connects', async () => { const busySessionID = 'session-remote-busy' mocks.listActiveSessions.mockImplementation((serverId?: string) => @@ -989,6 +1019,36 @@ describe('EventProvider permissions and forms', () => { }) }) + it('clears pending forms and permissions for a deleted session', async () => { + render(, { wrapper: createWrapper() }) + + await waitFor(() => expect(mocks.subscribeGlobalMonitor).toHaveBeenCalled()) + + const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1] + const onEvent = lastSubscribeCall[0].onEvent as (data: unknown) => void + + act(() => { + onEvent({ type: 'form.created', data: { form: pendingForm }, directory: '/repo' }) + onEvent({ type: 'permission.asked', data: pendingPermission, directory: '/repo' }) + }) + + await waitFor(() => { + expect(screen.getByTestId('count')).toHaveTextContent('1') + expect(screen.getByTestId('permission-count')).toHaveTextContent('1') + }) + + act(() => { + onEvent({ type: 'session.deleted', data: { sessionID: 'session-1' }, directory: '/repo' }) + }) + + await waitFor(() => { + expect(screen.getByTestId('count')).toHaveTextContent('0') + expect(screen.getByTestId('permission-count')).toHaveTextContent('0') + expect(screen.getByTestId('current')).toHaveTextContent('none') + expect(screen.getByTestId('permission-current')).toHaveTextContent('none') + }) + }) + it('does not invalidate the transcript on form reply or cancel events', async () => { mocks.listRepos.mockResolvedValue([{ id: 123, fullPath: '/repo' }]) mocks.listPendingForms.mockResolvedValue([pendingForm, secondPendingForm]) @@ -1455,7 +1515,7 @@ describe('EventProvider permissions and forms', () => { }) }) - it('keeps the shown permission when a viewed-session permission arrives, then advances once it is answered', async () => { + it('replaces a pinned out-of-view permission with the viewed-session one, then advances to the remaining pinned one', async () => { render(, { wrapper: createWrapper(createTestQueryClient(), '/repos/123/sessions/session-view'), }) @@ -1479,21 +1539,53 @@ describe('EventProvider permissions and forms', () => { await waitFor(() => { expect(screen.getByTestId('permission-count')).toHaveTextContent('2') + expect(screen.getByTestId('permission-current')).toHaveTextContent('permission-view') }) - expect(screen.getByTestId('permission-current')).toHaveTextContent('permission-other') act(() => { onEvent({ type: 'permission.replied', - data: { sessionID: 'session-other', requestID: 'permission-other', reply: 'reject' }, + data: { sessionID: 'session-view', requestID: 'permission-view', reply: 'reject' }, directory: '/repo', }) }) await waitFor(() => { expect(screen.getByTestId('permission-count')).toHaveTextContent('1') + expect(screen.getByTestId('permission-current')).toHaveTextContent('permission-other') + }) + }) + + it('keeps a shown in-view permission when another in-view permission arrives', async () => { + act(() => { + useSessionStatus.getState().setSessionParent('child-1', 'session-view') + }) + + render(, { + wrapper: createWrapper(createTestQueryClient(), '/repos/123/sessions/session-view'), + }) + + await waitFor(() => expect(mocks.subscribeGlobalMonitor).toHaveBeenCalled()) + + const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1] + const onEvent = lastSubscribeCall[0].onEvent as (data: unknown) => void + + act(() => { + onEvent({ type: 'permission.asked', data: viewedSessionPermission, directory: '/repo' }) + }) + + await waitFor(() => { expect(screen.getByTestId('permission-current')).toHaveTextContent('permission-view') }) + + act(() => { + onEvent({ type: 'permission.asked', data: childSessionPermission, directory: '/repo' }) + }) + + await waitFor(() => { + expect(screen.getByTestId('permission-count')).toHaveTextContent('2') + }) + expect(screen.getByTestId('permission-current')).toHaveTextContent('permission-view') }) it('prioritizes the viewed session form over an earlier unrelated one', async () => { @@ -1562,7 +1654,7 @@ describe('EventProvider permissions and forms', () => { }) }) - it('keeps the shown form when a viewed-session form arrives, then advances once it is answered', async () => { + it('replaces a pinned out-of-view form with the viewed-session one, then advances to the remaining pinned one', async () => { render(, { wrapper: createWrapper(createTestQueryClient(), '/repos/123/sessions/session-view'), }) @@ -1586,20 +1678,20 @@ describe('EventProvider permissions and forms', () => { await waitFor(() => { expect(screen.getByTestId('count')).toHaveTextContent('2') + expect(screen.getByTestId('current')).toHaveTextContent('form-view') }) - expect(screen.getByTestId('current')).toHaveTextContent('form-other') act(() => { onEvent({ type: 'form.cancelled', - data: { id: 'form-other', sessionID: 'session-other' }, + data: { id: 'form-view', sessionID: 'session-view' }, directory: '/repo', }) }) await waitFor(() => { expect(screen.getByTestId('count')).toHaveTextContent('1') - expect(screen.getByTestId('current')).toHaveTextContent('form-view') + expect(screen.getByTestId('current')).toHaveTextContent('form-other') }) }) @@ -2931,6 +3023,37 @@ describe('EventProvider permissions and forms', () => { expect(mocks.listActiveSessions).not.toHaveBeenCalledWith('srv_disabled') }) + it('purges pending actions from a server that is not enabled once servers load', async () => { + const { rerender } = render(, { wrapper: createWrapper() }) + + await waitFor(() => expect(mocks.subscribeGlobalMonitor).toHaveBeenCalled()) + + const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1] + const onEvent = lastSubscribeCall[0].onEvent as (data: unknown) => void + + act(() => { + onEvent({ type: 'form.created', data: { form: pendingForm }, directory: '/srv/app', serverId: 'srv_removed' }) + onEvent({ type: 'form.created', data: { form: secondPendingForm }, directory: '/srv/app', serverId: 'srv_remote' }) + onEvent({ type: 'permission.asked', data: pendingPermission, directory: '/srv/app', serverId: 'srv_removed' }) + }) + + await waitFor(() => { + expect(screen.getByTestId('count')).toHaveTextContent('2') + expect(screen.getByTestId('permission-count')).toHaveTextContent('1') + }) + + mocks.servers = [{ id: 'srv_remote', enabled: true }] + mocks.serversLoaded = true + rerender() + + await waitFor(() => { + expect(screen.getByTestId('count')).toHaveTextContent('1') + expect(screen.getByTestId('permission-count')).toHaveTextContent('0') + }) + expect(screen.getByTestId('for-session-2')).toHaveTextContent('form-2') + expect(screen.getByTestId('for-session-1')).toHaveTextContent('none') + }) + it('subscribes the project directories of enabled remote servers', async () => { mocks.servers = [{ id: 'srv_remote', enabled: true }] mocks.listProjects.mockResolvedValue([{ id: 'proj-1', canonical: '/remote/repo' }]) diff --git a/frontend/src/contexts/EventContext.tsx b/frontend/src/contexts/EventContext.tsx index ac1368538..2173eeb09 100644 --- a/frontend/src/contexts/EventContext.tsx +++ b/frontend/src/contexts/EventContext.tsx @@ -104,9 +104,10 @@ function isSessionWithinView( } /** - * Pins the item currently on screen until it leaves `items`, then advances to the first item whose - * session is the viewed session or one of its descendants, falling back to the first item. - * The selection is derived during render so a newly arriving item never displaces the shown one. + * Selects the pinned item while it belongs to the viewed session or no session is viewed, + * otherwise the first item within the viewed session, otherwise the pinned item while it is still + * present, otherwise the first item. The selection is derived during render so a newly arriving + * item never displaces a shown item that is within the view. */ function usePinnedViewedItem( items: T[], @@ -116,15 +117,15 @@ function usePinnedViewedItem( const [pinnedId, setPinnedId] = useState(null) const selected = useMemo(() => { - if (pinnedId) { - const pinned = items.find((item) => item.id === pinnedId) - if (pinned) return pinned + const pinned = pinnedId ? items.find((item) => item.id === pinnedId) ?? null : null + if (pinned && (viewedSessionId === null || isSessionWithinView(pinned.sessionID, viewedSessionId, parents))) { + return pinned } const forViewedSession = viewedSessionId ? items.find((item) => isSessionWithinView(item.sessionID, viewedSessionId, parents)) ?? null : null - return forViewedSession ?? items[0] ?? null + return forViewedSession ?? pinned ?? items[0] ?? null }, [items, viewedSessionId, parents, pinnedId]) const selectedId = selected?.id ?? null @@ -158,6 +159,33 @@ function reconcileBySessionForDirectory(bySession: Record, sessionID: string): Record { + if (!(sessionID in bySession)) return bySession + const next = { ...bySession } + delete next[sessionID] + return next +} + +function withoutItemsFromUnavailableServers( + bySession: Record, + isServerEnabled: (serverId: string) => boolean, +): Record { + let changed = false + const next: Record = {} + + for (const [sessionID, items] of Object.entries(bySession)) { + const kept = items.filter((item) => isLocalOpenCodeServer(item.serverId) || isServerEnabled(item.serverId)) + if (kept.length === items.length) { + next[sessionID] = items + continue + } + changed = true + if (kept.length > 0) next[sessionID] = kept + } + + return changed ? next : bySession +} + interface SSHHostKeyState { request: SSHHostKeyRequest | null respond: (requestId: string, approved: boolean) => Promise @@ -226,6 +254,11 @@ interface EventContextValue { hasForSession: (sessionID: string) => boolean navigateToCurrent: () => void syncForSession: (directory: string, sessionID: string, serverId?: string) => Promise + /** + * Drops every pending form and permission recorded for the session, used when the session no + * longer exists on its server. + */ + forgetPendingActionsForSession: (sessionID: string) => void } sseHealth: SSEHealthState getRepoIdForSession: (sessionID: string) => number | null @@ -324,7 +357,7 @@ export function EventProvider({ children }: { children: React.ReactNode }) { }) const { isAuthenticated } = useAuth() - const { servers } = useOpenCodeServers({ enabled: isAuthenticated }) + const { servers, isSuccess: serversLoaded } = useOpenCodeServers({ enabled: isAuthenticated }) const enabledRemoteServers = useMemo(() => servers.filter((server) => server.enabled), [servers]) const enabledRemoteServersRef = useRef(enabledRemoteServers) useEffect(() => { @@ -355,6 +388,14 @@ export function EventProvider({ children }: { children: React.ReactNode }) { const pendingPermissionSessionIDs = usePendingSessionIDs(permissionsBySession) const pendingFormSessionIDs = usePendingSessionIDs(formsBySession) + useEffect(() => { + if (!serversLoaded) return + const enabledServerIds = new Set(enabledRemoteServers.map((server) => server.id)) + const isServerEnabled = (serverId: string) => enabledServerIds.has(serverId) + setPermissionsBySession((prev) => withoutItemsFromUnavailableServers(prev, isServerEnabled)) + setFormsBySession((prev) => withoutItemsFromUnavailableServers(prev, isServerEnabled)) + }, [enabledRemoteServers, serversLoaded]) + const viewedSessionId = useMemo( () => parseServerSessionRoute(location.pathname)?.sessionId ?? parseRepoRoute(location.pathname).sessionId, [location.pathname], @@ -468,6 +509,11 @@ export function EventProvider({ children }: { children: React.ReactNode }) { removeFromSessionKeyedState(setFormsBySession, formID, sessionID) }, []) + const forgetPendingActionsForSession = useCallback((sessionID: string) => { + setPermissionsBySession((prev) => withoutSessionKey(prev, sessionID)) + setFormsBySession((prev) => withoutSessionKey(prev, sessionID)) + }, []) + const reconcilePermissionsForDirectory = useCallback(( directory: string, permissions: PermissionRequest[], @@ -912,6 +958,7 @@ export function EventProvider({ children }: { children: React.ReactNode }) { } case 'session.deleted': { useSessionStatus.getState().forgetSession(event.data.sessionID) + forgetPendingActionsForSession(event.data.sessionID) invalidateSessionListCachesDebounced(queryClient, event.directory, serverId) break } @@ -1011,7 +1058,7 @@ export function EventProvider({ children }: { children: React.ReactNode }) { streamConnectedRef.current = true reconciledRemoteServerIdsRef.current.clear() initialFetchDoneRef.current = false - fetchInitialPendingData() + void reconcilePendingActionsForDirectories(collectTrackedDirectories()) fetchInitialSessionStatuses() reconcileRemoteServersOnce(enabledRemoteServersRef.current) void syncRunningShells(collectTrackedDirectories()) @@ -1073,7 +1120,7 @@ export function EventProvider({ children }: { children: React.ReactNode }) { subscription.dispose() subscriptionRef.current = null } - }, [addPermission, removePermission, addForm, removeForm, rememberSessionDirectory, fetchInitialPendingData, fetchInitialSessionStatuses, startStatusPoll, stopStatusPoll, queryClient, handleHealthChange, reconcilePendingActionsForDirectories, collectTrackedDirectories, syncRunningShells, resolveSessionAncestry, reconcileRemoteSessionStatuses, reconcileRemoteServersOnce, invalidateRemoteStatusSyncs]) + }, [addPermission, removePermission, addForm, removeForm, forgetPendingActionsForSession, rememberSessionDirectory, fetchInitialPendingData, fetchInitialSessionStatuses, startStatusPoll, stopStatusPoll, queryClient, handleHealthChange, reconcilePendingActionsForDirectories, collectTrackedDirectories, syncRunningShells, resolveSessionAncestry, reconcileRemoteSessionStatuses, reconcileRemoteServersOnce, invalidateRemoteStatusSyncs]) useEffect(() => { reposRef.current = repos @@ -1127,6 +1174,7 @@ export function EventProvider({ children }: { children: React.ReactNode }) { hasForSession: hasFormsForSession, navigateToCurrent: navigateToCurrentForm, syncForSession: syncFormsForSession, + forgetPendingActionsForSession, }, sseHealth, getRepoIdForSession, @@ -1156,6 +1204,7 @@ export function EventProvider({ children }: { children: React.ReactNode }) { hasFormsForSession, navigateToCurrentForm, syncFormsForSession, + forgetPendingActionsForSession, sseHealth, getRepoIdForSession, ]) diff --git a/frontend/src/pages/SessionDetail.tsx b/frontend/src/pages/SessionDetail.tsx index 3f51cb242..16c8f9b50 100644 --- a/frontend/src/pages/SessionDetail.tsx +++ b/frontend/src/pages/SessionDetail.tsx @@ -328,12 +328,18 @@ export function SessionDetail() { const isEditingMessage = useUIState((state) => state.isEditingMessage); const { isEnabled: ttsEnabled } = useTTS(); const { syncForSession: syncPermissionsForSession } = usePermissions(); - const { getForSession: getFormForSession, reply: replyToForm, cancel: cancelForm, syncForSession: syncFormsForSession } = useForms(); + const { getForSession: getFormForSession, reply: replyToForm, cancel: cancelForm, syncForSession: syncFormsForSession, forgetPendingActionsForSession } = useForms(); const currentForm = sessionId ? getFormForSession(sessionId) : null; const minimizedForm = currentForm && currentForm.id === minimizedFormId ? currentForm : null; const isFormOpen = !!currentForm && !minimizedForm; const isFormSheetOpen = isMobile && isFormOpen; + useEffect(() => { + if (!sessionId) return; + if (!(sessionQueryError instanceof FetchError) || sessionQueryError.statusCode !== 404) return; + forgetPendingActionsForSession(sessionId); + }, [sessionId, sessionQueryError, forgetPendingActionsForSession]); + const lastAssistantMessage = messages.filter(m => m.type === 'assistant').at(-1); const lastAssistantText = getAssistantText(lastAssistantMessage); const latestPlayableAssistant = useMemo(() => getLatestPlayableAssistantMessage(messages), [messages]); diff --git a/frontend/src/pages/__tests__/SessionDetail.form-prompt.test.tsx b/frontend/src/pages/__tests__/SessionDetail.form-prompt.test.tsx index 68efec2b3..08154cda7 100644 --- a/frontend/src/pages/__tests__/SessionDetail.form-prompt.test.tsx +++ b/frontend/src/pages/__tests__/SessionDetail.form-prompt.test.tsx @@ -4,6 +4,7 @@ import userEvent from '@testing-library/user-event' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { MemoryRouter, Route, Routes } from 'react-router-dom' import type { FormInfo } from '@opencode-manager/shared/opencode' +import { FetchError } from '@/api/fetchWrapper' import { SessionDetail } from '../SessionDetail' const mocks = vi.hoisted(() => ({ @@ -26,6 +27,7 @@ const formsState = { bySession: {} as Record, reply: vi.fn(), cancel: vi.fn(), + forgetPendingActionsForSession: vi.fn(), } vi.mock('@/config', () => ({ @@ -237,6 +239,7 @@ describe('SessionDetail form prompt session scoping', () => { formsState.bySession = {} formsState.reply = vi.fn().mockResolvedValue(undefined) formsState.cancel = vi.fn().mockResolvedValue(undefined) + formsState.forgetPendingActionsForSession = vi.fn() mocks.useSession.mockReturnValue({ data: undefined, isLoading: false }) mocks.useSessionTranscript.mockReturnValue({ @@ -259,6 +262,7 @@ describe('SessionDetail form prompt session scoping', () => { pendingCount: 0, reply: formsState.reply, cancel: formsState.cancel, + forgetPendingActionsForSession: formsState.forgetPendingActionsForSession, syncForSession: vi.fn(), })) mocks.useSSEHealth.mockReturnValue({ isHealthy: true }) @@ -398,4 +402,17 @@ describe('SessionDetail form prompt session scoping', () => { await waitFor(() => expect(screen.getByTestId('prompt-overlay')).not.toHaveClass('top-0')) }) + + it('forgets pending actions for a session that no longer exists', async () => { + mocks.useSession.mockReturnValue({ + data: undefined, + isLoading: false, + error: new FetchError('Session not found', 404), + }) + + render(renderTree()) + + await waitFor(() => expect(screen.getByText('Session not found')).toBeInTheDocument()) + expect(formsState.forgetPendingActionsForSession).toHaveBeenCalledWith(VIEWED_SESSION_ID) + }) }) diff --git a/frontend/src/pages/__tests__/SessionDetail.remote.test.tsx b/frontend/src/pages/__tests__/SessionDetail.remote.test.tsx index fcadbc868..4c2ef3712 100644 --- a/frontend/src/pages/__tests__/SessionDetail.remote.test.tsx +++ b/frontend/src/pages/__tests__/SessionDetail.remote.test.tsx @@ -245,6 +245,7 @@ describe('SessionDetail remote server mode', () => { hasFormsForSession: vi.fn(() => false), reply: vi.fn(), cancel: vi.fn(), + forgetPendingActionsForSession: vi.fn(), syncForSession: vi.fn(), }) mocks.useSettings.mockReturnValue({ From 9c28ab6873d897a27a13c6f90bafe50e6e633d7b Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Tue, 20 Oct 2026 18:16:02 -0400 Subject: [PATCH 2/3] fix(events): reconcile pending actions per server and drop unreachable ones - Reconcile pending forms and permissions per server on reconnect, remote resync, and when a server is enabled after connect - Resolve the directory of pending sessions with no known directory, dropping them when the session returns 404 - Drop a remote server's pending items when reconciling it fails as unreachable (status 0, 502, 504) - Generalize the pending-item pruning helper --- frontend/src/contexts/EventContext.test.tsx | 118 ++++++++++++++++++++ frontend/src/contexts/EventContext.tsx | 100 ++++++++++++++--- 2 files changed, 205 insertions(+), 13 deletions(-) diff --git a/frontend/src/contexts/EventContext.test.tsx b/frontend/src/contexts/EventContext.test.tsx index 157b50cd8..bd5deea81 100644 --- a/frontend/src/contexts/EventContext.test.tsx +++ b/frontend/src/contexts/EventContext.test.tsx @@ -525,6 +525,9 @@ describe('EventProvider permissions and forms', () => { }) it('reconciles a remembered remote directory on reconnect', async () => { + mocks.servers = [{ id: 'srv_remote', enabled: true }] + mocks.serversLoaded = true + render(, { wrapper: createWrapper() }) await waitFor(() => expect(mocks.subscribeGlobalMonitor).toHaveBeenCalled()) @@ -3081,4 +3084,119 @@ describe('EventProvider permissions and forms', () => { await waitFor(() => expect(mocks.subscribeGlobalMonitor).toHaveBeenCalled()) expect(mocks.listProjects).not.toHaveBeenCalled() }) + + it('drops pending forms of a remote server that becomes unreachable', async () => { + render(, { wrapper: createWrapper() }) + + await waitFor(() => expect(mocks.subscribeGlobalMonitor).toHaveBeenCalled()) + + const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1] + const onEvent = lastSubscribeCall[0].onEvent as (data: unknown) => void + const onDirectoriesSubscribed = lastSubscribeCall[0].onDirectoriesSubscribed as (directories: string[]) => void + + act(() => { + onEvent({ type: 'form.created', data: { form: pendingForm }, directory: '/srv/app', serverId: 'srv_remote' }) + }) + + await waitFor(() => expect(screen.getByTestId('count')).toHaveTextContent('1')) + + mocks.listPendingForms.mockRejectedValue(new FetchError('Bad gateway', 502)) + + await act(async () => { + onDirectoriesSubscribed(['srv_remote|/srv/app']) + }) + + await waitFor(() => { + expect(screen.getByTestId('count')).toHaveTextContent('0') + expect(screen.getByTestId('current')).toHaveTextContent('none') + }) + }) + + it('drops a pending remote form whose session no longer exists on reconnect', async () => { + mocks.servers = [{ id: 'srv_remote', enabled: true }] + mocks.getSession.mockRejectedValue(new FetchError('Session not found', 404)) + + render(, { wrapper: createWrapper() }) + + await waitFor(() => expect(mocks.subscribeGlobalMonitor).toHaveBeenCalled()) + + const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1] + const onEvent = lastSubscribeCall[0].onEvent as (data: unknown) => void + const handleStatusChange = lastSubscribeCall[0].onStatusChange as (connected: boolean) => void + + act(() => { + useSessionStatus.getState().setSessionParent('session-1', null) + onEvent({ type: 'form.created', data: { form: pendingForm }, serverId: 'srv_remote' }) + }) + + await waitFor(() => expect(screen.getByTestId('count')).toHaveTextContent('1')) + + act(() => { + handleStatusChange(true) + }) + + await waitFor(() => { + expect(mocks.getSession).toHaveBeenCalledWith('session-1', 'srv_remote') + expect(screen.getByTestId('count')).toHaveTextContent('0') + expect(screen.getByTestId('current')).toHaveTextContent('none') + }) + }) + + it('resolves an unknown remote session directory on reconnect', async () => { + mocks.servers = [{ id: 'srv_remote', enabled: true }] + mocks.getSession.mockImplementation((sessionID: string) => + sessionID === 'session-1' + ? Promise.resolve({ id: 'session-1', location: { directory: '/srv/resolved' } }) + : Promise.reject(new FetchError('Session not found', 404)), + ) + + render(, { wrapper: createWrapper() }) + + await waitFor(() => expect(mocks.subscribeGlobalMonitor).toHaveBeenCalled()) + + const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1] + const onEvent = lastSubscribeCall[0].onEvent as (data: unknown) => void + const handleStatusChange = lastSubscribeCall[0].onStatusChange as (connected: boolean) => void + + act(() => { + useSessionStatus.getState().setSessionParent('session-1', null) + onEvent({ type: 'form.created', data: { form: pendingForm }, serverId: 'srv_remote' }) + }) + + await waitFor(() => expect(screen.getByTestId('count')).toHaveTextContent('1')) + expect(useSessionStatus.getState().directories.has('session-1')).toBe(false) + + act(() => { + handleStatusChange(true) + }) + + await waitFor(() => { + expect(mocks.listPendingForms).toHaveBeenCalledWith('/srv/resolved', 'srv_remote') + expect(useSessionStatus.getState().directories.get('session-1')).toBe('srv_remote|/srv/resolved') + }) + }) + + it('reconciles pending actions for a remote server enabled after the browser stream connects', async () => { + const { rerender } = render(, { wrapper: createWrapper() }) + + await waitFor(() => expect(mocks.subscribeGlobalMonitor).toHaveBeenCalled()) + + const lastSubscribeCall = mocks.subscribeGlobalMonitor.mock.calls[mocks.subscribeGlobalMonitor.mock.calls.length - 1] + const handleStatusChange = lastSubscribeCall[0].onStatusChange as (connected: boolean) => void + + act(() => { + handleStatusChange(true) + }) + + act(() => { + useSessionStatus.getState().setSessionDirectory('remote-session', 'srv_remote|/remote/repo') + }) + + mocks.servers = [{ id: 'srv_remote', enabled: true }] + rerender() + + await waitFor(() => { + expect(mocks.listPendingForms).toHaveBeenCalledWith('/remote/repo', 'srv_remote') + }) + }) }) diff --git a/frontend/src/contexts/EventContext.tsx b/frontend/src/contexts/EventContext.tsx index 2173eeb09..023db05ac 100644 --- a/frontend/src/contexts/EventContext.tsx +++ b/frontend/src/contexts/EventContext.tsx @@ -166,15 +166,19 @@ function withoutSessionKey(bySession: Record, sessionID: string) return next } -function withoutItemsFromUnavailableServers( +/** + * Drops every item matched by `shouldRemove` and deletes a session key left empty. Returns the + * input record itself when no item matched. + */ +function withoutMatchingItems( bySession: Record, - isServerEnabled: (serverId: string) => boolean, + shouldRemove: (item: T) => boolean, ): Record { let changed = false const next: Record = {} for (const [sessionID, items] of Object.entries(bySession)) { - const kept = items.filter((item) => isLocalOpenCodeServer(item.serverId) || isServerEnabled(item.serverId)) + const kept = items.filter((item) => !shouldRemove(item)) if (kept.length === items.length) { next[sessionID] = items continue @@ -186,6 +190,15 @@ function withoutItemsFromUnavailableServers Promise @@ -340,7 +353,17 @@ export function EventProvider({ children }: { children: React.ReactNode }) { const [formsBySession, setFormsBySession] = useState({}) const [showPermissionDialog, setShowPermissionDialog] = useState(true) + const prunePendingActions = useCallback( + (shouldRemove: (item: { sessionID: string; serverId: string }) => boolean) => { + setPermissionsBySession((prev) => withoutMatchingItems(prev, shouldRemove)) + setFormsBySession((prev) => withoutMatchingItems(prev, shouldRemove)) + }, + [], + ) + const prevPermissionCountRef = useRef(0) + const permissionsBySessionRef = useRef({}) + const formsBySessionRef = useRef({}) const initialFetchDoneRef = useRef(false) const statusSyncVersionRef = useRef(0) const remoteStatusSyncVersionsRef = useRef(new Map()) @@ -391,10 +414,16 @@ export function EventProvider({ children }: { children: React.ReactNode }) { useEffect(() => { if (!serversLoaded) return const enabledServerIds = new Set(enabledRemoteServers.map((server) => server.id)) - const isServerEnabled = (serverId: string) => enabledServerIds.has(serverId) - setPermissionsBySession((prev) => withoutItemsFromUnavailableServers(prev, isServerEnabled)) - setFormsBySession((prev) => withoutItemsFromUnavailableServers(prev, isServerEnabled)) - }, [enabledRemoteServers, serversLoaded]) + prunePendingActions((item) => !isLocalOpenCodeServer(item.serverId) && !enabledServerIds.has(item.serverId)) + }, [enabledRemoteServers, serversLoaded, prunePendingActions]) + + useEffect(() => { + permissionsBySessionRef.current = permissionsBySession + }, [permissionsBySession]) + + useEffect(() => { + formsBySessionRef.current = formsBySession + }, [formsBySession]) const viewedSessionId = useMemo( () => parseServerSessionRoute(location.pathname)?.sessionId ?? parseRepoRoute(location.pathname).sessionId, @@ -665,12 +694,15 @@ export function EventProvider({ children }: { children: React.ReactNode }) { reconcilePermissionsForDirectory(directory, pendingPermissions, serverId) reconcileFormsForDirectory(directory, pendingForms, serverId) } catch (error) { + if (!isLocalOpenCodeServer(serverId) && isUnreachableServerError(error)) { + prunePendingActions((item) => item.serverId === serverId) + } if (import.meta.env.DEV) { console.warn(`Failed to fetch pending actions for ${directory}:`, error) } } })) - }, [reconcilePermissionsForDirectory, reconcileFormsForDirectory]) + }, [reconcilePermissionsForDirectory, reconcileFormsForDirectory, prunePendingActions]) const collectTrackedDirectories = useCallback((): string[] => { const directories = new Set() @@ -683,6 +715,47 @@ export function EventProvider({ children }: { children: React.ReactNode }) { return [...directories] }, []) + const reconcilePendingActionsForServer = useCallback(async ( + serverId: string, + extraDirectories: string[] = [], + ) => { + const directoryKeys = new Set( + [...collectTrackedDirectories(), ...extraDirectories] + .filter((key) => parseServerDirectoryKey(key).serverId === serverId), + ) + + const unresolvedSessionIDs = new Set() + for (const bySession of [permissionsBySessionRef.current, formsBySessionRef.current]) { + for (const [sessionID, items] of Object.entries(bySession)) { + if (!items.some((item) => item.serverId === serverId)) continue + if (findSessionDirectoryKey(sessionID) !== null) continue + unresolvedSessionIDs.add(sessionID) + } + } + + if (unresolvedSessionIDs.size > 0) { + await Promise.all([...unresolvedSessionIDs].map(async (sessionID) => { + try { + const session = await getSession(sessionID, serverId) + rememberSessionDirectory(sessionID, session.location.directory, serverId) + directoryKeys.add(buildServerDirectoryKey(serverId, session.location.directory)) + } catch (error) { + if (error instanceof FetchError && error.statusCode === 404) { + prunePendingActions((item) => item.sessionID === sessionID && item.serverId === serverId) + } + } + })) + } + + await reconcilePendingActionsForDirectories([...directoryKeys]) + }, [ + collectTrackedDirectories, + findSessionDirectoryKey, + rememberSessionDirectory, + prunePendingActions, + reconcilePendingActionsForDirectories, + ]) + const resolveSessionAncestry = useCallback(async ( sessionIDs: string[], serverId: string = LOCAL_OPENCODE_SERVER_ID, @@ -830,8 +903,9 @@ export function EventProvider({ children }: { children: React.ReactNode }) { if (reconciled.has(server.id)) continue reconciled.add(server.id) void reconcileRemoteSessionStatuses(server.id) + void reconcilePendingActionsForServer(server.id) } - }, [reconcileRemoteSessionStatuses]) + }, [reconcileRemoteSessionStatuses, reconcilePendingActionsForServer]) useEffect(() => { const enabledIds = new Set(enabledRemoteServers.map((server) => server.id)) @@ -1058,7 +1132,7 @@ export function EventProvider({ children }: { children: React.ReactNode }) { streamConnectedRef.current = true reconciledRemoteServerIdsRef.current.clear() initialFetchDoneRef.current = false - void reconcilePendingActionsForDirectories(collectTrackedDirectories()) + void reconcilePendingActionsForServer(LOCAL_OPENCODE_SERVER_ID) fetchInitialSessionStatuses() reconcileRemoteServersOnce(enabledRemoteServersRef.current) void syncRunningShells(collectTrackedDirectories()) @@ -1079,7 +1153,7 @@ export function EventProvider({ children }: { children: React.ReactNode }) { const directories = subscribedDirectories.filter( (key) => parseServerDirectoryKey(key).serverId === serverId, ) - void reconcilePendingActionsForDirectories(directories) + void reconcilePendingActionsForServer(serverId, subscribedDirectories) void reconcileRemoteSessionStatuses(serverId) void syncRunningShells(directories) invalidateSessionListCachesDebounced(queryClient, undefined, serverId) @@ -1090,7 +1164,7 @@ export function EventProvider({ children }: { children: React.ReactNode }) { return } const directories = [...new Set([...collectTrackedDirectories(), ...subscribedDirectories])] - void reconcilePendingActionsForDirectories(directories) + void reconcilePendingActionsForServer(LOCAL_OPENCODE_SERVER_ID, subscribedDirectories) void fetchInitialSessionStatuses() void syncRunningShells(directories) invalidateChildSessionCaches(queryClient) @@ -1120,7 +1194,7 @@ export function EventProvider({ children }: { children: React.ReactNode }) { subscription.dispose() subscriptionRef.current = null } - }, [addPermission, removePermission, addForm, removeForm, forgetPendingActionsForSession, rememberSessionDirectory, fetchInitialPendingData, fetchInitialSessionStatuses, startStatusPoll, stopStatusPoll, queryClient, handleHealthChange, reconcilePendingActionsForDirectories, collectTrackedDirectories, syncRunningShells, resolveSessionAncestry, reconcileRemoteSessionStatuses, reconcileRemoteServersOnce, invalidateRemoteStatusSyncs]) + }, [addPermission, removePermission, addForm, removeForm, forgetPendingActionsForSession, rememberSessionDirectory, fetchInitialSessionStatuses, startStatusPoll, stopStatusPoll, queryClient, handleHealthChange, reconcilePendingActionsForDirectories, reconcilePendingActionsForServer, collectTrackedDirectories, syncRunningShells, resolveSessionAncestry, reconcileRemoteSessionStatuses, reconcileRemoteServersOnce, invalidateRemoteStatusSyncs]) useEffect(() => { reposRef.current = repos From 539f10f9a3bbfcab1b65a8c10e21233de3ac7191 Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Tue, 20 Oct 2026 18:24:03 -0400 Subject: [PATCH 3/3] fix(events): ignore stale pending-action results for disabled servers and gone sessions --- frontend/src/contexts/EventContext.tsx | 33 +++++++++++++++++++------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/frontend/src/contexts/EventContext.tsx b/frontend/src/contexts/EventContext.tsx index 023db05ac..a96aee7bb 100644 --- a/frontend/src/contexts/EventContext.tsx +++ b/frontend/src/contexts/EventContext.tsx @@ -373,6 +373,7 @@ export function EventProvider({ children }: { children: React.ReactNode }) { const subscriptionRef = useRef | null>(null) const reposRef = useRef(null) const ancestryInFlightRef = useRef>(new Set()) + const forgottenSessionIDsRef = useRef(new Set()) const { data: repos } = useQuery({ queryKey: ['repos'], @@ -386,6 +387,15 @@ export function EventProvider({ children }: { children: React.ReactNode }) { useEffect(() => { enabledRemoteServersRef.current = enabledRemoteServers }, [enabledRemoteServers]) + const serversLoadedRef = useRef(serversLoaded) + useEffect(() => { + serversLoadedRef.current = serversLoaded + }, [serversLoaded]) + const isRemoteServerDisabled = useCallback((serverId: string): boolean => { + return serversLoadedRef.current + && !isLocalOpenCodeServer(serverId) + && !enabledRemoteServersRef.current.some((server) => server.id === serverId) + }, []) const remoteProjectQueries = useQueries({ queries: enabledRemoteServers.map((server) => ({ queryKey: serverScopedKey(['opencode', 'projects'], server.id), @@ -539,6 +549,7 @@ export function EventProvider({ children }: { children: React.ReactNode }) { }, []) const forgetPendingActionsForSession = useCallback((sessionID: string) => { + forgottenSessionIDsRef.current.add(sessionID) setPermissionsBySession((prev) => withoutSessionKey(prev, sessionID)) setFormsBySession((prev) => withoutSessionKey(prev, sessionID)) }, []) @@ -548,7 +559,11 @@ export function EventProvider({ children }: { children: React.ReactNode }) { permissions: PermissionRequest[], serverId: string, ) => { - permissions.forEach(permission => { + if (isRemoteServerDisabled(serverId)) return + const keptPermissions = permissions.filter( + (permission) => !forgottenSessionIDsRef.current.has(permission.sessionID), + ) + keptPermissions.forEach(permission => { rememberSessionDirectory(permission.sessionID, directory, serverId) }) @@ -557,18 +572,20 @@ export function EventProvider({ children }: { children: React.ReactNode }) { prev, buildServerDirectoryKey(serverId, directory), serverId, - permissions.map(permission => ({ ...permission, serverId })), + keptPermissions.map(permission => ({ ...permission, serverId })), useSessionStatus.getState().directories, ), ) - }, [rememberSessionDirectory]) + }, [rememberSessionDirectory, isRemoteServerDisabled]) const reconcileFormsForDirectory = useCallback(( directory: string, forms: FormInfo[], serverId: string, ) => { - forms.forEach(form => { + if (isRemoteServerDisabled(serverId)) return + const keptForms = forms.filter((form) => !forgottenSessionIDsRef.current.has(form.sessionID)) + keptForms.forEach(form => { rememberSessionDirectory(form.sessionID, directory, serverId) }) @@ -577,11 +594,11 @@ export function EventProvider({ children }: { children: React.ReactNode }) { prev, buildServerDirectoryKey(serverId, directory), serverId, - forms.map(form => ({ ...form, serverId })), + keptForms.map(form => ({ ...form, serverId })), useSessionStatus.getState().directories, ), ) - }, [rememberSessionDirectory]) + }, [rememberSessionDirectory, isRemoteServerDisabled]) useEffect(() => { const permissionCount = allPermissions.length @@ -741,7 +758,7 @@ export function EventProvider({ children }: { children: React.ReactNode }) { directoryKeys.add(buildServerDirectoryKey(serverId, session.location.directory)) } catch (error) { if (error instanceof FetchError && error.statusCode === 404) { - prunePendingActions((item) => item.sessionID === sessionID && item.serverId === serverId) + forgetPendingActionsForSession(sessionID) } } })) @@ -752,7 +769,7 @@ export function EventProvider({ children }: { children: React.ReactNode }) { collectTrackedDirectories, findSessionDirectoryKey, rememberSessionDirectory, - prunePendingActions, + forgetPendingActionsForSession, reconcilePendingActionsForDirectories, ])