fix(sync): clear dismissed questions deterministically after reject/reply
This commit is contained in:
@@ -32,6 +32,7 @@ All notable changes to this project will be documented in this file.
|
||||
- Desktop: browser pages served from a self-signed loopback HTTPS address now load instead of being blocked by the certificate warning.
|
||||
- Browser: typing a comment on a page no longer triggers app shortcuts.
|
||||
- Skills Catalog: the source is now named ClawHub instead of "ClawdHub" (thanks to @makeittech).
|
||||
- Chat: dismissing an agent's clarifying questions no longer leaves the session stuck on the question screen — the next task shows its thinking and final response again.
|
||||
|
||||
## [1.18.4] - 2026-08-14
|
||||
|
||||
|
||||
@@ -1461,6 +1461,99 @@ describe("rejectQuestion passes directory", () => {
|
||||
})
|
||||
})
|
||||
|
||||
function sessionFixture(id: string): Session {
|
||||
// SAFETY: the question flow only reads session id/time; the fixture is
|
||||
// intentionally minimal and matches the existing fixtures in this file.
|
||||
return { id, time: { created: 1 } } as Session
|
||||
}
|
||||
|
||||
function actionsSdk(): OpencodeClient {
|
||||
// SAFETY: mockSdk implements the question/permission/session surface that
|
||||
// session-actions uses; this cast is the established pattern in this file.
|
||||
return mockSdk as never
|
||||
}
|
||||
|
||||
describe("question dismissal clears pending state without the SSE echo (issues #2911, #2448)", () => {
|
||||
beforeEach(() => {
|
||||
replyCalls.length = 0
|
||||
scopedClientDirectories.length = 0
|
||||
questionReplyError = null
|
||||
questionRejectError = null
|
||||
})
|
||||
|
||||
test("rejectQuestion clears the question from the child store on success", async () => {
|
||||
const question = buildQuestion("q-1", "session-a")
|
||||
const store = createStore({}, {
|
||||
session: [sessionFixture("session-a")],
|
||||
question: { "session-a": [question] },
|
||||
})
|
||||
const childStores = createChildStores([["/test/project", store]])
|
||||
|
||||
const { setActionRefs, rejectQuestion } = await import("./session-actions")
|
||||
setActionRefs(actionsSdk(), childStores, () => "/test/project")
|
||||
|
||||
await rejectQuestion("session-a", "q-1")
|
||||
|
||||
// The backend confirmed the rejection. The local pending state must be gone
|
||||
// even if the SSE `question.rejected` event is lost (SSE gap), otherwise the
|
||||
// session stays in "waiting for answer" and the next task never renders
|
||||
// thinking/final response (issues #2911, #2448).
|
||||
expect(store.getState().question["session-a"]).toBe(undefined)
|
||||
})
|
||||
|
||||
test("respondToQuestion clears the question from the child store on success", async () => {
|
||||
const question = buildQuestion("q-1", "session-a")
|
||||
const store = createStore({}, {
|
||||
session: [sessionFixture("session-a")],
|
||||
question: { "session-a": [question] },
|
||||
})
|
||||
const childStores = createChildStores([["/test/project", store]])
|
||||
|
||||
const { setActionRefs, respondToQuestion } = await import("./session-actions")
|
||||
setActionRefs(actionsSdk(), childStores, () => "/test/project")
|
||||
|
||||
await respondToQuestion("session-a", "q-1", [["Yes"]])
|
||||
|
||||
expect(store.getState().question["session-a"]).toBe(undefined)
|
||||
})
|
||||
|
||||
test("dismissOpenQuestionsForSession leaves the store cleared when the reject succeeds", async () => {
|
||||
const question = buildQuestion("q-root", "session-a")
|
||||
const store = createStore({}, {
|
||||
session: [sessionFixture("session-a")],
|
||||
question: { "session-a": [question] },
|
||||
})
|
||||
const childStores = createChildStores([["/test/project", store]])
|
||||
|
||||
const { setActionRefs, dismissOpenQuestionsForSession } = await import("./session-actions")
|
||||
setActionRefs(actionsSdk(), childStores, () => "/test/project")
|
||||
|
||||
const dismissed = await dismissOpenQuestionsForSession("session-a")
|
||||
|
||||
expect(dismissed).toBe(true)
|
||||
// The optimistic clear already removed it before the round-trip; the
|
||||
// successful reject must not resurrect it.
|
||||
expect(store.getState().question["session-a"]).toBe(undefined)
|
||||
})
|
||||
|
||||
test("reply/reject actions on an already-cleared store stay no-ops (SSE echo equivalent)", async () => {
|
||||
// A later (or duplicated) SSE echo for an already-cleared request must not
|
||||
// error or resurrect state — the reducer only removes when present.
|
||||
const store = createStore({}, {
|
||||
session: [sessionFixture("session-a")],
|
||||
question: {},
|
||||
})
|
||||
|
||||
const { setActionRefs, rejectQuestion, respondToQuestion } = await import("./session-actions")
|
||||
setActionRefs(actionsSdk(), createChildStores([["/test/project", store]]), () => "/test/project")
|
||||
|
||||
await respondToQuestion("session-a", "q-gone", [["Yes"]])
|
||||
await rejectQuestion("session-a", "q-gone")
|
||||
|
||||
expect(store.getState().question["session-a"]).toBe(undefined)
|
||||
})
|
||||
})
|
||||
|
||||
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) => {
|
||||
|
||||
@@ -1687,6 +1687,14 @@ export async function respondToQuestion(
|
||||
if (assertSdkData(result, "question.reply") !== true) {
|
||||
throw new Error("Question reply failed")
|
||||
}
|
||||
// A successful reply is authoritative: the backend resolved the question,
|
||||
// so clear it from the local store deterministically instead of waiting
|
||||
// for the SSE `question.replied` event. A lost event (SSE gap) would leave
|
||||
// the question pending forever, which keeps the session in "waiting for
|
||||
// answer" — the next task's thinking and final response never render
|
||||
// (issues #2911, #2448). The later SSE event is a no-op (the reducer only
|
||||
// removes when present).
|
||||
removeQuestionRequestFromChildStores(sessionId, requestId)
|
||||
} catch (error) {
|
||||
if (isQuestionRequestNotFoundError(error)) {
|
||||
removeQuestionRequestFromChildStores(sessionId, requestId)
|
||||
@@ -1712,6 +1720,11 @@ export async function rejectQuestion(
|
||||
if (assertSdkData(result, "question.reject") !== true) {
|
||||
throw new Error("Question rejection failed")
|
||||
}
|
||||
// A successful rejection is authoritative: the backend resolved the
|
||||
// question, so clear it from the local store deterministically (see
|
||||
// respondToQuestion for the lost-SSE-event rationale — issues #2911,
|
||||
// #2448). The later SSE `question.rejected` event is a no-op.
|
||||
removeQuestionRequestFromChildStores(sessionId, requestId)
|
||||
} catch (error) {
|
||||
if (isQuestionRequestNotFoundError(error)) {
|
||||
removeQuestionRequestFromChildStores(sessionId, requestId)
|
||||
@@ -1743,6 +1756,10 @@ export async function rejectQuestion(
|
||||
* abort the session so the OpenCode runner reaches `idle` — otherwise the new
|
||||
* prompt arrives while the run is still active and is discarded by the runner's
|
||||
* `ensureRunning`.
|
||||
*
|
||||
* A successful reject clears the local store deterministically (see
|
||||
* {@link rejectQuestion}) so a lost `question.rejected` SSE event cannot leave
|
||||
* the session in the pending "waiting for answer" state (issues #2911, #2448).
|
||||
*/
|
||||
export async function dismissOpenQuestionsForSession(sessionId: string): Promise<boolean> {
|
||||
if (!sessionId) return false
|
||||
|
||||
Reference in New Issue
Block a user