diff --git a/packages/ui/src/sync/DOCUMENTATION.md b/packages/ui/src/sync/DOCUMENTATION.md index 420004a2..349e10d7 100644 --- a/packages/ui/src/sync/DOCUMENTATION.md +++ b/packages/ui/src/sync/DOCUMENTATION.md @@ -257,6 +257,10 @@ Examples of global-store updates performed in `session-actions.ts`: - `deleteSession()` / `deleteSessions()` -> wait for server confirmation or `404`, then remove the session and its persisted state - `moveSessionToDirectory()` -> move the session between directory stores and update the global directory index +### Blocking-request (question/permission) reply routing + +`respondToQuestion`, `rejectQuestion`, `respondToPermission`, and `dismissPermission` route the reply through `resolveDirectoryForBlockingRequest`. The directory chosen decides which OpenCode instance resolves the pending request, so it must be the **session record's own server-confirmed directory** (ownership), never the containing child-store key (containment): a project store legitimately holds its worktree sessions, and a reply addressed to the parent instance makes the server answer `QuestionNotFoundError` while the question stays pending in the worktree instance — the session is then stuck on the running question tool with no recovery. When a reply/reject comes back not-found, the stale request is removed locally and a `settled-running-tool` tail materialization is enqueued so the trailing tool part converges to the server's actual state instead of leaving the UI on "asking question" forever. + ### Restore (unarchive) contract The OpenCode server cannot clear `time.archived` over HTTP: `session.update` diff --git a/packages/ui/src/sync/session-actions.test.ts b/packages/ui/src/sync/session-actions.test.ts index 84c276f8..d815b6dd 100644 --- a/packages/ui/src/sync/session-actions.test.ts +++ b/packages/ui/src/sync/session-actions.test.ts @@ -1461,6 +1461,165 @@ describe("rejectQuestion passes directory", () => { }) }) +describe("blocking request reply routing and stale recovery (issue OPE-236)", () => { + const materializationCalls: Array<{ directory: string; sessionID: string; messageID: string }> = [] + const enqueueMaterialization = (directory: string, sessionID: string, messageID: string) => { + materializationCalls.push({ directory, sessionID, messageID }) + } + + beforeEach(() => { + replyCalls.length = 0 + scopedClientDirectories.length = 0 + questionReplyError = null + questionRejectError = null + materializationCalls.length = 0 + }) + + test("routes the question reply by the request's own session directory, not the containing store key", async () => { + // The question was asked by a worktree session whose record lives in the + // parent store (containment). The reply must be addressed to the session's + // own server-confirmed directory — otherwise the server resolves the + // parent instance, does not find the pending question, and answers + // QuestionNotFoundError, leaving the session stuck on "asking question". + const question = buildQuestion("q-wt", "session-wt") + const store = createStore({}, { + session: [{ id: "session-wt", directory: "/test/project/wt" } as Session], + question: { "session-wt": [question] }, + }) + const childStores = createChildStores([["/test/project", store]]) + + const { setActionRefs, respondToQuestion } = await import("./session-actions") + setActionRefs(mockSdk as unknown as OpencodeClient, childStores, () => "/test/project", enqueueMaterialization) + + await respondToQuestion("session-wt", "q-wt", [["Yes"]]) + + expect(scopedClientDirectories).toEqual(["/test/project/wt"]) + expect(replyCalls[0]?.params.directory).toBe("/test/project/wt") + expect(replyCalls[0]?.params.requestID).toBe("q-wt") + }) + + test("routes permission replies by the request's own session directory", async () => { + const permission = buildPermission("perm-wt", "session-wt") + const store = createStore( + { "session-wt": [permission] }, + { + session: [{ id: "session-wt", directory: "/test/project/wt" } as Session], + }, + ) + const childStores = createChildStores([["/test/project", store]]) + + const { setActionRefs, respondToPermission } = await import("./session-actions") + setActionRefs(mockSdk as unknown as OpencodeClient, childStores, () => "/test/project", enqueueMaterialization) + + await respondToPermission("session-wt", "perm-wt", "once") + + expect(scopedClientDirectories).toEqual(["/test/project/wt"]) + expect(replyCalls[0]?.params.directory).toBe("/test/project/wt") + expect(replyCalls[0]?.params.requestID).toBe("perm-wt") + }) + + test("falls back to the containing store key when the session record carries no directory", async () => { + const question = buildQuestion("q-1", "session-a") + const store = createStore({}, { + session: [{ id: "session-a" } as Session], + question: { "session-a": [question] }, + }) + const childStores = createChildStores([["/test/project", store]]) + + const { setActionRefs, respondToQuestion } = await import("./session-actions") + setActionRefs(mockSdk as unknown as OpencodeClient, childStores, () => "/test/project", enqueueMaterialization) + + await respondToQuestion("session-a", "q-1", [["Yes"]]) + + expect(scopedClientDirectories).toEqual(["/test/project"]) + expect(replyCalls[0]?.params.directory).toBe("/test/project") + }) + + test("enqueues settled-running-tool tail recovery when the question reply is not found", async () => { + const question = buildQuestion("q-stale", "session-a") + const store = createStore({}, { + session: [{ id: "session-a" } as Session], + question: { "session-a": [question] }, + message: { + "session-a": [{ id: "msg-1", sessionID: "session-a", role: "assistant", time: { created: 1 } } as Message], + }, + part: { + "msg-1": [{ + id: "prt-1", + messageID: "msg-1", + sessionID: "session-a", + type: "tool", + tool: "question", + state: { status: "running" }, + } as Part], + }, + }) + const childStores = createChildStores([["/test/project", store]]) + questionReplyError = Object.assign(new Error("question.reply failed (404): QuestionNotFoundError"), { status: 404 }) + + const { setActionRefs, respondToQuestion } = await import("./session-actions") + setActionRefs(mockSdk as unknown as OpencodeClient, childStores, () => "/test/project", enqueueMaterialization) + + let thrown: unknown + try { + await respondToQuestion("session-a", "q-stale", [["Yes"]]) + } catch (error) { + thrown = error + } + + expect(thrown).toBeInstanceOf(Error) + // The stale request is gone from the store and the trailing running tool + // part is reconciled instead of leaving the UI stuck on "asking question". + expect(store.getState().question["session-a"]).toBe(undefined) + expect(materializationCalls).toEqual([{ directory: "/test/project", sessionID: "session-a", messageID: "msg-1" }]) + }) + + test("enqueues tail recovery on reject not-found but not on success", async () => { + const question = buildQuestion("q-1", "session-a") + const store = createStore({}, { + session: [{ id: "session-a" } as Session], + question: { "session-a": [question] }, + message: { + "session-a": [{ id: "msg-1", sessionID: "session-a", role: "assistant", time: { created: 1 } } as Message], + }, + part: { + "msg-1": [{ + id: "prt-1", + messageID: "msg-1", + sessionID: "session-a", + type: "tool", + tool: "question", + state: { status: "running" }, + } as Part], + }, + }) + const childStores = createChildStores([["/test/project", store]]) + + const { setActionRefs, rejectQuestion } = await import("./session-actions") + setActionRefs(mockSdk as unknown as OpencodeClient, childStores, () => "/test/project", enqueueMaterialization) + + // Success: no recovery enqueued — the normal question.rejected event flow clears state. + await rejectQuestion("session-a", "q-1") + expect(materializationCalls).toEqual([]) + + // Not-found: the request is stale server-side; the tail must be reconciled. + questionRejectError = Object.assign(new Error("question.reject failed (404): QuestionNotFoundError"), { status: 404 }) + const stale = buildQuestion("q-stale", "session-a") + store.setState({ question: { "session-a": [stale] } }) + + let thrown: unknown + try { + await rejectQuestion("session-a", "q-stale") + } catch (error) { + thrown = error + } + + expect(thrown).toBeInstanceOf(Error) + expect(store.getState().question["session-a"]).toBe(undefined) + expect(materializationCalls).toEqual([{ directory: "/test/project", sessionID: "session-a", messageID: "msg-1" }]) + }) +}) + function buildQuestion(id: string, sessionId: string): QuestionRequest { return { id, diff --git a/packages/ui/src/sync/session-actions.ts b/packages/ui/src/sync/session-actions.ts index 1b695891..aa8cdb10 100644 --- a/packages/ui/src/sync/session-actions.ts +++ b/packages/ui/src/sync/session-actions.ts @@ -30,6 +30,8 @@ import { getImperativeSessionMessageLoader } from "./session-message-loader" import { cleanupPersistedSessionState } from "./session-deletion-cleanup" import { getRuntimeKey } from "@/lib/runtime-switch" import { isAmbiguousTransportFailure } from "@/lib/relay/transport-error" +import { getStaleRunningToolMessageID } from "./materialization" +import { normalizePath } from "@/lib/pathNormalization" const MESSAGE_REFETCH_LIMIT = 100 const SEND_CONFIRMATION_REFETCH_LIMIT = 30 @@ -52,6 +54,10 @@ const UNREVERT_REFETCH_RETRY_MS = 150 let _sdk: OpencodeClient | null = null let _childStores: ChildStoreManager | null = null let _getDirectory: () => string = () => "" +// Optional ref into the sync layer's session-tail materialization queue. Used +// to reconcile a trailing running tool part after a blocking request is +// confirmed stale server-side (see recoverStaleBlockingRequest). +let _enqueueSessionMaterialization: ((directory: string, sessionID: string, messageID: string) => void) | null = null type OptimisticAddInput = { sessionID: string; directory?: string | null; message: Message; parts: Part[] } type OptimisticRemoveInput = { sessionID: string; directory?: string | null; messageID: string } type OptimisticConfirmInput = OptimisticRemoveInput @@ -139,10 +145,12 @@ export function setActionRefs( sdk: OpencodeClient, childStores: ChildStoreManager, getDirectory: () => string, + enqueueSessionMaterialization?: (directory: string, sessionID: string, messageID: string) => void, ) { _sdk = sdk _childStores = childStores _getDirectory = getDirectory + _enqueueSessionMaterialization = enqueueSessionMaterialization ?? null } export function setOptimisticRefs( @@ -480,6 +488,29 @@ function restoreFilePartsToInput(fileParts: Array>): voi } } +/** + * Server-confirmed directory that owns a session, from the session record + * (`directory`, then `project.worktree`). Mirrors the authoritative source in + * session-directory-resolution: holding a session in a child store proves + * containment, not ownership — a project's session list legitimately includes + * the sessions of its worktrees so the sidebar can group them — so reading + * ownership from the containing store reports the parent for a session that + * lives in a worktree, and every fetch is then addressed to a directory that + * does not own it. + */ +function resolveSessionOwnedDirectory(session: Session): string | null { + const record = session as Session & { + directory?: string | null + project?: { worktree?: string | null } | null + } + const raw = typeof record.directory === "string" && record.directory.trim().length > 0 + ? record.directory + : typeof record.project?.worktree === "string" && record.project.worktree.trim().length > 0 + ? record.project.worktree + : null + return raw ? normalizePath(raw) : null +} + function resolveDirectoryForBlockingRequest( type: "permission" | "question", sessionId: string, @@ -493,10 +524,28 @@ function resolveDirectoryForBlockingRequest( for (const [directory, store] of stores.children) { const state = store.getState() const requestMap = type === "permission" ? state.permission : state.question - for (const requests of Object.values(requestMap) as Array | undefined>) { - if (requests?.some((request) => request.id === requestId)) { - return directory - } + for (const requests of Object.values(requestMap) as Array | undefined>) { + const request = requests?.find((candidate) => candidate.id === requestId) + if (!request) continue + + // Ownership beats containment. The request belongs to one specific + // session, and the reply must reach the instance that actually tracks + // it — the directory the session record's server-confirmed `directory` + // names. The containing store's key only proves containment: a project + // store holds its worktree sessions too, and a reply addressed to the + // parent instance makes the server answer QuestionNotFoundError while + // the question stays pending in the worktree instance, leaving the + // session stuck on the running question tool. Fall back to the store + // key only when the session record carries no directory. + const requestSessionID = typeof request.sessionID === "string" && request.sessionID.length > 0 + ? request.sessionID + : sessionId + const sessionRecord = requestSessionID + ? state.session.find((s) => s.id === requestSessionID) + : undefined + const ownedDirectory = sessionRecord ? resolveSessionOwnedDirectory(sessionRecord) : null + if (ownedDirectory) return ownedDirectory + return directory } } @@ -537,6 +586,38 @@ export function isQuestionRequestNotFoundError(error: unknown): boolean { return /Question(?:\.)?NotFoundError|Question request not found/i.test(message) } +/** + * Reconcile the trailing assistant tool part after a blocking request turned + * out to be stale server-side (reply/reject answered with not-found). The + * local request is removed (the server no longer tracks it), but the + * question/permission tool part can remain `running` with the session busy — + * the UI would stay on "asking question" with no recovery until the user + * stops the run. Enqueue the sync layer's settled-running-tool tail + * materialization so the part converges to the server's actual state. + */ +function recoverStaleBlockingRequest(sessionId: string): void { + const stores = _childStores + const enqueue = _enqueueSessionMaterialization + if (!stores || !enqueue || !sessionId) return + + for (const [directory, store] of stores.children) { + const state = store.getState() + if ( + !state.session.some((session) => session.id === sessionId) + && !Object.prototype.hasOwnProperty.call(state.message, sessionId) + && !Object.prototype.hasOwnProperty.call(state.session_status ?? {}, sessionId) + && !Object.prototype.hasOwnProperty.call(state.question ?? {}, sessionId) + ) { + continue + } + const messageID = getStaleRunningToolMessageID(state, sessionId) + if (messageID) { + enqueue(directory, sessionId, messageID) + } + return + } +} + function removeQuestionRequestFromChildStores(sessionId: string, requestId: string): boolean { const stores = _childStores if (!stores || !requestId) return false @@ -1581,6 +1662,7 @@ export async function respondToQuestion( } catch (error) { if (isQuestionRequestNotFoundError(error)) { removeQuestionRequestFromChildStores(sessionId, requestId) + recoverStaleBlockingRequest(sessionId) } throw error } @@ -1605,6 +1687,7 @@ export async function rejectQuestion( } catch (error) { if (isQuestionRequestNotFoundError(error)) { removeQuestionRequestFromChildStores(sessionId, requestId) + recoverStaleBlockingRequest(sessionId) } throw error } diff --git a/packages/ui/src/sync/sync-context.tsx b/packages/ui/src/sync/sync-context.tsx index 846e6fcc..91f027a1 100644 --- a/packages/ui/src/sync/sync-context.tsx +++ b/packages/ui/src/sync/sync-context.tsx @@ -2360,6 +2360,12 @@ export function SyncProvider(props: { props.sdk, childStores, () => opencodeClient.getDirectory() || props.directory, + (directory, sessionID, messageID) => { + enqueueSessionMaterialization(directory, sessionID, childStores, { + reason: "settled-running-tool", + messageID, + }) + }, ) return () => { if (getImperativeSessionMessageLoader() === messageLoader) {