From 177dbe486dde0263f3322f750daabe730ccd4f8e Mon Sep 17 00:00:00 2001 From: bashrusakh Date: Wed, 19 Aug 2026 06:08:07 +1100 Subject: [PATCH] fix(sync): clear dismissed questions deterministically after reject/reply --- CHANGELOG.md | 1 + packages/ui/src/sync/session-actions.test.ts | 93 ++++++++++++++++++++ packages/ui/src/sync/session-actions.ts | 17 ++++ 3 files changed, 111 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 65659752..74c774a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/packages/ui/src/sync/session-actions.test.ts b/packages/ui/src/sync/session-actions.test.ts index 4870e39f..48cacb3a 100644 --- a/packages/ui/src/sync/session-actions.test.ts +++ b/packages/ui/src/sync/session-actions.test.ts @@ -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) => { diff --git a/packages/ui/src/sync/session-actions.ts b/packages/ui/src/sync/session-actions.ts index 806cc913..1d9f306e 100644 --- a/packages/ui/src/sync/session-actions.ts +++ b/packages/ui/src/sync/session-actions.ts @@ -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 { if (!sessionId) return false