fix(sync): route question/permission replies by the request's own session directory
Answering a question tool (or a permission prompt) could leave the session permanently stuck on "asking question": resolveDirectoryForBlockingRequest returned the containing child-store key, which only proves containment. For a worktree session (or any session whose record is grouped under a parent project store), the reply was addressed to the parent directory's OpenCode instance, where the pending request does not exist - the server answered QuestionNotFoundError, the local request was removed, and the trailing question-tool part stayed running with no recovery until Stop. Resolve the directory from the request's own session record (server- confirmed ownership: session.directory, then project.worktree) before falling back to the containing store key. When a reply/reject comes back not-found, also enqueue the settled-running-tool tail materialization so the tool part converges to the server's actual state instead of leaving the UI stuck. Refs OPE-236
This commit is contained in:
@@ -255,6 +255,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
|
- `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
|
- `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
|
### Restore (unarchive) contract
|
||||||
|
|
||||||
The OpenCode server cannot clear `time.archived` over HTTP: `session.update`
|
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 {
|
function buildQuestion(id: string, sessionId: string): QuestionRequest {
|
||||||
return {
|
return {
|
||||||
id,
|
id,
|
||||||
|
|||||||
@@ -30,6 +30,8 @@ import { getImperativeSessionMessageLoader } from "./session-message-loader"
|
|||||||
import { cleanupPersistedSessionState } from "./session-deletion-cleanup"
|
import { cleanupPersistedSessionState } from "./session-deletion-cleanup"
|
||||||
import { getRuntimeKey } from "@/lib/runtime-switch"
|
import { getRuntimeKey } from "@/lib/runtime-switch"
|
||||||
import { isAmbiguousTransportFailure } from "@/lib/relay/transport-error"
|
import { isAmbiguousTransportFailure } from "@/lib/relay/transport-error"
|
||||||
|
import { getStaleRunningToolMessageID } from "./materialization"
|
||||||
|
import { normalizePath } from "@/lib/pathNormalization"
|
||||||
|
|
||||||
const MESSAGE_REFETCH_LIMIT = 100
|
const MESSAGE_REFETCH_LIMIT = 100
|
||||||
const SEND_CONFIRMATION_REFETCH_LIMIT = 30
|
const SEND_CONFIRMATION_REFETCH_LIMIT = 30
|
||||||
@@ -52,6 +54,10 @@ const UNREVERT_REFETCH_RETRY_MS = 150
|
|||||||
let _sdk: OpencodeClient | null = null
|
let _sdk: OpencodeClient | null = null
|
||||||
let _childStores: ChildStoreManager | null = null
|
let _childStores: ChildStoreManager | null = null
|
||||||
let _getDirectory: () => string = () => ""
|
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 OptimisticAddInput = { sessionID: string; directory?: string | null; message: Message; parts: Part[] }
|
||||||
type OptimisticRemoveInput = { sessionID: string; directory?: string | null; messageID: string }
|
type OptimisticRemoveInput = { sessionID: string; directory?: string | null; messageID: string }
|
||||||
type OptimisticConfirmInput = OptimisticRemoveInput
|
type OptimisticConfirmInput = OptimisticRemoveInput
|
||||||
@@ -139,10 +145,12 @@ export function setActionRefs(
|
|||||||
sdk: OpencodeClient,
|
sdk: OpencodeClient,
|
||||||
childStores: ChildStoreManager,
|
childStores: ChildStoreManager,
|
||||||
getDirectory: () => string,
|
getDirectory: () => string,
|
||||||
|
enqueueSessionMaterialization?: (directory: string, sessionID: string, messageID: string) => void,
|
||||||
) {
|
) {
|
||||||
_sdk = sdk
|
_sdk = sdk
|
||||||
_childStores = childStores
|
_childStores = childStores
|
||||||
_getDirectory = getDirectory
|
_getDirectory = getDirectory
|
||||||
|
_enqueueSessionMaterialization = enqueueSessionMaterialization ?? null
|
||||||
}
|
}
|
||||||
|
|
||||||
export function setOptimisticRefs(
|
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(
|
function resolveDirectoryForBlockingRequest(
|
||||||
type: "permission" | "question",
|
type: "permission" | "question",
|
||||||
sessionId: string,
|
sessionId: string,
|
||||||
@@ -493,10 +524,28 @@ function resolveDirectoryForBlockingRequest(
|
|||||||
for (const [directory, store] of stores.children) {
|
for (const [directory, store] of stores.children) {
|
||||||
const state = store.getState()
|
const state = store.getState()
|
||||||
const requestMap = type === "permission" ? state.permission : state.question
|
const requestMap = type === "permission" ? state.permission : state.question
|
||||||
for (const requests of Object.values(requestMap) as Array<Array<{ id: string }> | undefined>) {
|
for (const requests of Object.values(requestMap) as Array<Array<{ id: string; sessionID?: string }> | undefined>) {
|
||||||
if (requests?.some((request) => request.id === requestId)) {
|
const request = requests?.find((candidate) => candidate.id === requestId)
|
||||||
return directory
|
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)
|
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 {
|
function removeQuestionRequestFromChildStores(sessionId: string, requestId: string): boolean {
|
||||||
const stores = _childStores
|
const stores = _childStores
|
||||||
if (!stores || !requestId) return false
|
if (!stores || !requestId) return false
|
||||||
@@ -1581,6 +1662,7 @@ export async function respondToQuestion(
|
|||||||
} catch (error) {
|
} catch (error) {
|
||||||
if (isQuestionRequestNotFoundError(error)) {
|
if (isQuestionRequestNotFoundError(error)) {
|
||||||
removeQuestionRequestFromChildStores(sessionId, requestId)
|
removeQuestionRequestFromChildStores(sessionId, requestId)
|
||||||
|
recoverStaleBlockingRequest(sessionId)
|
||||||
}
|
}
|
||||||
throw error
|
throw error
|
||||||
}
|
}
|
||||||
@@ -1605,6 +1687,7 @@ export async function rejectQuestion(
|
|||||||
} catch (error) {
|
} catch (error) {
|
||||||
if (isQuestionRequestNotFoundError(error)) {
|
if (isQuestionRequestNotFoundError(error)) {
|
||||||
removeQuestionRequestFromChildStores(sessionId, requestId)
|
removeQuestionRequestFromChildStores(sessionId, requestId)
|
||||||
|
recoverStaleBlockingRequest(sessionId)
|
||||||
}
|
}
|
||||||
throw error
|
throw error
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2260,6 +2260,12 @@ export function SyncProvider(props: {
|
|||||||
props.sdk,
|
props.sdk,
|
||||||
childStores,
|
childStores,
|
||||||
() => opencodeClient.getDirectory() || props.directory,
|
() => opencodeClient.getDirectory() || props.directory,
|
||||||
|
(directory, sessionID, messageID) => {
|
||||||
|
enqueueSessionMaterialization(directory, sessionID, childStores, {
|
||||||
|
reason: "settled-running-tool",
|
||||||
|
messageID,
|
||||||
|
})
|
||||||
|
},
|
||||||
)
|
)
|
||||||
return () => {
|
return () => {
|
||||||
if (getImperativeSessionMessageLoader() === messageLoader) {
|
if (getImperativeSessionMessageLoader() === messageLoader) {
|
||||||
|
|||||||
Reference in New Issue
Block a user