Merge pull request #2663 from makeittech/fix/ope-236-question-tool-stuck

fix(sync): route question/permission replies by the request's own session directory
This commit is contained in:
Serhii Dziupin
2026-08-06 11:39:53 +03:00
committed by GitHub
4 changed files with 256 additions and 4 deletions
+4
View File
@@ -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`
@@ -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,
+87 -4
View File
@@ -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<Record<string, unknown>>): 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<Array<{ id: string }> | undefined>) {
if (requests?.some((request) => request.id === requestId)) {
return directory
}
for (const requests of Object.values(requestMap) as Array<Array<{ id: string; sessionID?: string }> | 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
}
+6
View File
@@ -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) {