From dfa7b45dd0baaae6c14eeae9e2f4c13de064b505 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Mon, 10 Aug 2026 00:38:31 +0300 Subject: [PATCH] fix(mobile): recover pending questions after cold start --- .../ui/src/components/chat/ChatContainer.tsx | 21 ++++++ packages/ui/src/sync/DOCUMENTATION.md | 2 + .../__tests__/session-switch-resync.test.ts | 59 +++++++++++++++++ .../ui/src/sync/question-recovery.test.ts | 66 +++++++++++++++++++ packages/ui/src/sync/question-recovery.ts | 47 +++++++++++++ packages/ui/src/sync/sync-context.tsx | 53 +++++++++++---- packages/ui/src/sync/use-sync.ts | 21 +++++- 7 files changed, 257 insertions(+), 12 deletions(-) create mode 100644 packages/ui/src/sync/question-recovery.test.ts create mode 100644 packages/ui/src/sync/question-recovery.ts diff --git a/packages/ui/src/components/chat/ChatContainer.tsx b/packages/ui/src/components/chat/ChatContainer.tsx index 6799093a..476b2f14 100644 --- a/packages/ui/src/components/chat/ChatContainer.tsx +++ b/packages/ui/src/components/chat/ChatContainer.tsx @@ -13,6 +13,7 @@ import { useGlobalSyncStore } from '@/sync/global-sync-store'; import MessageList, { type MessageListHandle } from './MessageList'; import { PermissionCard } from './PermissionCard'; import { QuestionCard } from './QuestionCard'; +import { hasActiveQuestionToolInCurrentTurn, recoverPendingQuestionWithRetry } from '@/sync/question-recovery'; import { StatusRowContainer } from './StatusRowContainer'; import { SessionRecapNote } from '@/components/chat/SessionRecapSpacer'; import ScrollToBottomButton from './components/ScrollToBottomButton'; @@ -618,6 +619,26 @@ export const ChatContainer: React.FC = ({ active = true, aut const sessionPermissions = useScopedBlockingPermissions(currentSessionId, effectiveSessionDirectory); const sessionQuestions = useScopedBlockingQuestions(currentSessionId, effectiveSessionDirectory); + const hasUnreconciledQuestionTool = React.useMemo( + () => !sessionQuestions.some((question) => question.sessionID === currentSessionId) + && hasActiveQuestionToolInCurrentTurn(sessionMessages), + [currentSessionId, sessionMessages, sessionQuestions], + ); + + React.useEffect(() => { + if (!active || !currentSessionId || !effectiveSessionDirectory || !hasUnreconciledQuestionTool) return; + let cancelled = false; + + void recoverPendingQuestionWithRetry( + () => sync.recoverPendingQuestions(currentSessionId, effectiveSessionDirectory), + { isCancelled: () => cancelled }, + ); + + return () => { + cancelled = true; + }; + }, [active, currentSessionId, effectiveSessionDirectory, hasUnreconciledQuestionTool, sync]); + const sessionIsWorking = React.useMemo(() => { if (!currentSessionId || sessionPermissions.length > 0 || sessionQuestions.length > 0) { return false; diff --git a/packages/ui/src/sync/DOCUMENTATION.md b/packages/ui/src/sync/DOCUMENTATION.md index c4486cb5..d61b8833 100644 --- a/packages/ui/src/sync/DOCUMENTATION.md +++ b/packages/ui/src/sync/DOCUMENTATION.md @@ -72,6 +72,8 @@ The composer compares normalized attachment MIME types with the selected model's - Demand is deduplicated by normalized directory and can be promoted while queued. - The complete known project/worktree set is always published. Collapsed and off-screen directories remain background demand, so they refresh eventually rather than waiting for expansion. - A bootstrap holds its scheduler slot through critical state and the authoritative directory session-list fetch. Deferrable command/MCP/LSP/VCS/question/permission enrichment starts afterward without extending slot ownership or competing with the initial session-list request. +- A system-resume signal, including Capacitor foreground resume, refreshes pending questions and permissions only for the active materialized directory. The refresh is deduplicated while in flight, preserves existing state on fetch failure, and leaves unopened directories untouched; normal stream reconnect recovery remains the broader catch-up path. +- When a materialized current turn contains a pending/running question tool but that session's pending question record is missing, the mounted chat performs a question-only recovery scoped to that session. It tries at most three times with delays of 0, 500, and 1,500 ms, stops when the chat unmounts or changes sessions, and guards every attempt against runtime changes. This closes cold-start races without adding requests to ordinary session opens or scanning unrelated sessions and directories. - A mounted directory-store consumer pins that store for its lifetime. Eviction may dispose only unmounted directories, so optimistic actions and realtime events cannot move to a replacement store while visible React consumers remain subscribed to an older identity. - Reconfiguration and runtime switching invalidate stale generations. A stale completion must not publish state into the new runtime. - Failure is recorded as `failed`; it is not converted into a successful empty snapshot. Forced demand can retry failed or completed work. diff --git a/packages/ui/src/sync/__tests__/session-switch-resync.test.ts b/packages/ui/src/sync/__tests__/session-switch-resync.test.ts index 5c860956..b3874a0d 100644 --- a/packages/ui/src/sync/__tests__/session-switch-resync.test.ts +++ b/packages/ui/src/sync/__tests__/session-switch-resync.test.ts @@ -70,6 +70,7 @@ import { getRuntimeKey } from "@/lib/runtime-switch" const { createEventRoutingIndex, handleEvent, + resyncBlockingRequestsForActiveDirectory, resyncBlockingRequestsForDirectory, setActiveSession, } = await import("../sync-context") @@ -130,6 +131,34 @@ describe("resyncBlockingRequestsForDirectory", () => { expect(listPendingPermissionsCalls[0]).toEqual({ directories: ["/repo"] }) }) + test("resume recovery refreshes blocking requests only for the active materialized directory", async () => { + const childStores = new ChildStoreManager() + childStores.ensureChild("/resume-active", { bootstrap: false }).setState({ + session: [{ id: "ses_a", title: "ses_a", time: { created: 1, updated: 1 }, version: "1" } as State["session"][number]], + }) + childStores.ensureChild("/resume-inactive", { bootstrap: false }).setState({ + session: [{ id: "ses_b", title: "ses_b", time: { created: 1, updated: 1 }, version: "1" } as State["session"][number]], + }) + pendingQuestionsResponse = [buildQuestion()] + + await resyncBlockingRequestsForActiveDirectory("/resume-active", childStores) + + expect(listPendingQuestionsCalls).toEqual([{ directories: ["/resume-active"] }]) + expect(listPendingPermissionsCalls).toEqual([{ directories: ["/resume-active"] }]) + expect(childStores.getChild("/resume-active")?.getState().question.ses_a?.[0]?.id).toBe("que_1") + expect(childStores.getChild("/resume-inactive")?.getState().question.ses_b).toBe(undefined) + }) + + test("resume recovery does not materialize or fetch an unopened directory", async () => { + const childStores = new ChildStoreManager() + + await resyncBlockingRequestsForActiveDirectory("/unopened", childStores) + + expect(childStores.getChild("/unopened")).toBe(undefined) + expect(listPendingQuestionsCalls).toHaveLength(0) + expect(listPendingPermissionsCalls).toHaveLength(0) + }) + test("merges newly fetched questions/permissions into the directory store", async () => { const store = createDirectoryStore({}) pendingQuestionsResponse = [buildQuestion()] @@ -187,6 +216,36 @@ describe("resyncBlockingRequestsForDirectory", () => { expect(listPendingPermissionsCalls).toHaveLength(0) }) + test("recovers an explicit session candidate before directory bootstrap materializes it", async () => { + const store = createDirectoryStore({ session: [] }) + pendingQuestionsResponse = [buildQuestion()] + + await resyncBlockingRequestsForDirectory("/repo", store, ["ses_a"], { includePermissions: false }) + + expect(listPendingQuestionsCalls).toEqual([{ directories: ["/repo"] }]) + expect(listPendingPermissionsCalls).toHaveLength(0) + expect(store.getState().question.ses_a?.[0]?.id).toBe("que_1") + }) + + test("limits explicit question-only recovery to the requested session", async () => { + const store = createDirectoryStore({ + session: [ + { id: "ses_a", title: "ses_a", time: { created: 1, updated: 1 }, version: "1" }, + { id: "ses_b", title: "ses_b", time: { created: 1, updated: 1 }, version: "1" }, + ] as State["session"], + }) + pendingQuestionsResponse = [ + buildQuestion(), + buildQuestion({ id: "que_b", sessionID: "ses_b" }), + ] + + await resyncBlockingRequestsForDirectory("/repo", store, ["ses_a"], { includePermissions: false }) + + expect(store.getState().question.ses_a?.[0]?.id).toBe("que_1") + expect(store.getState().question.ses_b).toBe(undefined) + expect(listPendingPermissionsCalls).toHaveLength(0) + }) + // Regression: prior to the fix, listPendingQuestions silently returned [] on // fetch failure, indistinguishable from a successful empty server response. // The resync then walked the candidate set and deleted any question that diff --git a/packages/ui/src/sync/question-recovery.test.ts b/packages/ui/src/sync/question-recovery.test.ts new file mode 100644 index 00000000..6cb95cf9 --- /dev/null +++ b/packages/ui/src/sync/question-recovery.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, test } from "bun:test" +import type { Message, Part } from "@opencode-ai/sdk/v2/client" +import { hasActiveQuestionToolInCurrentTurn, recoverPendingQuestionWithRetry } from "./question-recovery" + +const message = (role: "user" | "assistant", parts: Part[] = []) => ({ + info: { id: `${role}-${parts.length}`, sessionID: "ses_1", role } as Message, + parts, +}) + +const questionTool = (status: "pending" | "running" | "completed"): Part => ({ + id: `tool-${status}`, + sessionID: "ses_1", + messageID: "assistant-1", + type: "tool", + tool: "question", + state: { status, input: {}, output: "", title: "", metadata: {}, time: { start: 1, end: status === "completed" ? 2 : undefined } }, +} as Part) + +describe("hasActiveQuestionToolInCurrentTurn", () => { + test("detects a pending or running question in the current turn", () => { + expect(hasActiveQuestionToolInCurrentTurn([message("user"), message("assistant", [questionTool("pending")])])).toBe(true) + expect(hasActiveQuestionToolInCurrentTurn([message("user"), message("assistant", [questionTool("running")])])).toBe(true) + }) + + test("ignores completed questions and active questions from an older turn", () => { + expect(hasActiveQuestionToolInCurrentTurn([message("assistant", [questionTool("completed")])])).toBe(false) + expect(hasActiveQuestionToolInCurrentTurn([ + message("assistant", [questionTool("running")]), + message("user"), + message("assistant"), + ])).toBe(false) + }) +}) + +describe("recoverPendingQuestionWithRetry", () => { + test("retries the cold-start inconsistency with bounded delays and stops on recovery", async () => { + const delays: number[] = [] + let attempts = 0 + + const recovered = await recoverPendingQuestionWithRetry( + async () => { + attempts += 1 + return attempts === 3 + }, + { sleep: async (delayMs) => { delays.push(delayMs) } }, + ) + + expect(recovered).toBe(true) + expect(attempts).toBe(3) + expect(delays).toEqual([500, 1500]) + }) + + test("does no more work after cancellation", async () => { + let attempts = 0 + const recovered = await recoverPendingQuestionWithRetry( + async () => { + attempts += 1 + return false + }, + { isCancelled: () => true, sleep: async () => undefined }, + ) + + expect(recovered).toBe(false) + expect(attempts).toBe(0) + }) +}) diff --git a/packages/ui/src/sync/question-recovery.ts b/packages/ui/src/sync/question-recovery.ts new file mode 100644 index 00000000..144921db --- /dev/null +++ b/packages/ui/src/sync/question-recovery.ts @@ -0,0 +1,47 @@ +import type { Message, Part, ToolPart } from "@opencode-ai/sdk/v2/client" + +type MessageRecord = { + info: Message + parts: Part[] +} + +const RECOVERY_DELAYS_MS = [0, 500, 1500] as const + +const isActiveQuestionTool = (part: Part): boolean => { + if (part.type !== "tool" || part.tool !== "question") return false + const status = (part as ToolPart).state.status + return status === "pending" || status === "running" +} + +/** + * A persisted running question tool without a matching pending-request record + * is the cold-start recovery signal. Only inspect the current turn so an old, + * stale tool cannot trigger network work after the user has continued chatting. + */ +export function hasActiveQuestionToolInCurrentTurn(messages: readonly MessageRecord[]): boolean { + for (let index = messages.length - 1; index >= 0; index -= 1) { + const message = messages[index] + if (!message) continue + if (message.info.role === "user") return false + if (message.parts.some(isActiveQuestionTool)) return true + } + return false +} + +export async function recoverPendingQuestionWithRetry( + recover: () => Promise, + options?: { + isCancelled?: () => boolean + sleep?: (delayMs: number) => Promise + }, +): Promise { + const isCancelled = options?.isCancelled ?? (() => false) + const sleep = options?.sleep ?? ((delayMs) => new Promise((resolve) => setTimeout(resolve, delayMs))) + + for (const delayMs of RECOVERY_DELAYS_MS) { + if (delayMs > 0) await sleep(delayMs) + if (isCancelled()) return false + if (await recover()) return true + } + return false +} diff --git a/packages/ui/src/sync/sync-context.tsx b/packages/ui/src/sync/sync-context.tsx index 08911863..288796d5 100644 --- a/packages/ui/src/sync/sync-context.tsx +++ b/packages/ui/src/sync/sync-context.tsx @@ -1178,23 +1178,24 @@ const updateRoutingIndexFromEvent = ( * recovery paths only; normal session switches rely on primary SSE reducer * state for `question.asked` / `permission.asked` events. When * `candidateSessionIds` is omitted, every session known to the directory store - * is treated as a candidate. + * is treated as a candidate; when provided, recovery is limited to those IDs. */ export async function resyncBlockingRequestsForDirectory( directory: string, store: StoreApi, candidateSessionIds?: string[], + options?: { includePermissions?: boolean }, ) { const before = store.getState() - const knownSessionIds = new Set([ + const candidateIds = new Set(candidateSessionIds ?? [ ...before.session.map((session) => session.id), ...Object.keys(before.message ?? {}), ...Object.keys(before.session_status ?? {}), ...Object.keys(before.question ?? {}), ...Object.keys(before.permission ?? {}), ]) - const candidates = candidateSessionIds ?? Array.from(knownSessionIds) - if (candidates.length === 0) return + if (candidateIds.size === 0) return + const candidates = Array.from(candidateIds) // Re-fetch pending questions that may have been asked during an SSE gap, // reconnect window, or directory materialization gap. @@ -1204,12 +1205,12 @@ export async function resyncBlockingRequestsForDirectory( ) const pendingQuestions = await opencodeClient.listPendingQuestions({ directories: [directory] }) const grouped: Record = {} - for (const q of pendingQuestions) { - if (!q?.id || !q.sessionID) continue - if (!knownSessionIds.has(q.sessionID)) continue - const list = grouped[q.sessionID] - if (list) list.push(q) - else grouped[q.sessionID] = [q] + for (const question of pendingQuestions) { + if (!question?.id || !question.sessionID) continue + if (!candidateIds.has(question.sessionID)) continue + const list = grouped[question.sessionID] + if (list) list.push(question) + else grouped[question.sessionID] = [question] } for (const sessionId of Object.keys(grouped)) { grouped[sessionId].sort((a, b) => (a.id < b.id ? -1 : a.id > b.id ? 1 : 0)) @@ -1256,6 +1257,8 @@ export async function resyncBlockingRequestsForDirectory( // Non-fatal: question resync best-effort } + if (options?.includePermissions === false) return + // Re-fetch pending permissions — same rationale as questions. try { const beforeSignatures = new Map( @@ -1265,7 +1268,7 @@ export async function resyncBlockingRequestsForDirectory( const grouped: Record = {} for (const permission of pendingPermissions) { if (!permission?.id || !permission.sessionID) continue - if (!knownSessionIds.has(permission.sessionID)) continue + if (!candidateIds.has(permission.sessionID)) continue const list = grouped[permission.sessionID] if (list) list.push(permission) else grouped[permission.sessionID] = [permission] @@ -1330,6 +1333,15 @@ export async function resyncBlockingRequestsForDirectory( } } +export async function resyncBlockingRequestsForActiveDirectory( + directory: string, + childStores: ChildStoreManager, +) { + const store = childStores.getChild(directory) + if (!store) return + await resyncBlockingRequestsForDirectory(directory, store) +} + async function resyncDirectoryAfterReconnect( directory: string, store: StoreApi, @@ -1928,6 +1940,7 @@ export function SyncProvider(props: { const lastFullResyncAtByDirectoryRef = useRef(new Map()) const lastChildDiscoveryAtByDirectoryRef = useRef(new Map()) const resyncingDirectoriesRef = useRef(new Set()) + const blockingRequestResyncingDirectoriesRef = useRef(new Set()) const statusPollingDirectoriesRef = useRef(new Set()) const pipelineReconnectRef = useRef<((reason?: string) => void) | null>(null) const pipelineHasConnectedRef = useRef(false) @@ -1961,6 +1974,24 @@ export function SyncProvider(props: { }) }, [childStores, routingIndex]) + useEffect(() => { + if (typeof window === "undefined") return + + const onSystemResume = () => { + const directory = currentDirectoryRef.current + if (!directory || !childStores.getChild(directory)) return + + const resyncing = blockingRequestResyncingDirectoriesRef.current + if (resyncing.has(directory)) return + resyncing.add(directory) + void resyncBlockingRequestsForActiveDirectory(directory, childStores) + .finally(() => resyncing.delete(directory)) + } + + window.addEventListener("openchamber:system-resume", onSystemResume) + return () => window.removeEventListener("openchamber:system-resume", onSystemResume) + }, [childStores]) + // Configure child store manager useEffect(() => { void usePermissionStore.getState().hydrate().catch(() => undefined) diff --git a/packages/ui/src/sync/use-sync.ts b/packages/ui/src/sync/use-sync.ts index 83e179e2..629821f7 100644 --- a/packages/ui/src/sync/use-sync.ts +++ b/packages/ui/src/sync/use-sync.ts @@ -11,6 +11,7 @@ import { useSessionMessageLoader, useSyncDirectory, useSyncSDK, + resyncBlockingRequestsForDirectory, } from "./sync-context" import { dropSessionCaches, getProtectedSessionCacheIds } from "./session-cache" import { stripSessionDiffSnapshots } from "./sanitize" @@ -123,6 +124,23 @@ export function useSync() { const messageLoader = useSessionMessageLoader() const runtimeKey = getRuntimeKey() + const recoverPendingQuestions = useCallback( + async (sessionID: string, directoryOverride?: string): Promise => { + const targetDirectory = directoryOverride || directory + if (!sessionID || !targetDirectory || getRuntimeKey() !== runtimeKey) return false + const targetStore = childStores.ensureChild(targetDirectory, { + priority: "selected", + reason: "selected-session", + }) + await resyncBlockingRequestsForDirectory(targetDirectory, targetStore, [sessionID], { + includePermissions: false, + }) + if (getRuntimeKey() !== runtimeKey) return false + return (targetStore.getState().question[sessionID]?.length ?? 0) > 0 + }, + [childStores, directory, runtimeKey], + ) + const keyFor = useCallback( (sessionID: string, directoryOverride = directory) => `${runtimeKey}\n${directoryOverride}\n${sessionID}`, [directory, runtimeKey], @@ -408,12 +426,13 @@ export function useSync() { hasMore, isLoading, isComplete, + recoverPendingQuestions, optimistic: { add: optimisticAdd, remove: optimisticRemove, confirm: optimisticConfirm, }, }), - [syncSession, prefetchSession, loadMore, loadCompleteHistory, hasMore, isLoading, isComplete, optimisticAdd, optimisticRemove, optimisticConfirm], + [syncSession, prefetchSession, loadMore, loadCompleteHistory, hasMore, isLoading, isComplete, recoverPendingQuestions, optimisticAdd, optimisticRemove, optimisticConfirm], ) }