From 4dc05d3773ea6e82137836637d00b9b89859471e Mon Sep 17 00:00:00 2001 From: JD Date: Mon, 18 May 2026 12:48:39 +0000 Subject: [PATCH] fix(ui): preserve question queue order when upserting duplicate requests Fixes #448 - Questions can deadlock with "Waiting for earlier responses" when a question arrives first as a global entry and later resolves to a tool part with a newer enqueuedAt timestamp. Root cause: - upsertQuestion replaced the entire queue entry on duplicate request id, losing the original enqueuedAt value. - If question-1 arrived at t=1000 (global), then question-2 at t=1500, and question-1 resolved to a tool part at t=2000, the newer enqueuedAt (2000) would place question-1 behind question-2 in the sorted queue. - This broke the active interruption ordering, leaving both questions showing "Waiting for earlier responses." Fix: - Add mergeQuestionEntry mirroring mergePermissionEntry behavior. - Preserve original enqueuedAt using Math.min(existing, new). - Keep newer messageId/partId when resolving from global to tool part. - Clean up stale byMessage entries for the same request id. - Sort queue by enqueuedAt and recalculate active after each upsert. Validation: - npm run typecheck --workspace @codenomad/ui - node --import tsx --test packages/ui/src/stores/message-v2/instance-store.test.ts - Added 3 regression tests covering duplicate resolution and ordering. --- .../stores/message-v2/instance-store.test.ts | 57 +++++++++++++++++++ .../src/stores/message-v2/instance-store.ts | 30 ++++++++-- 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/packages/ui/src/stores/message-v2/instance-store.test.ts b/packages/ui/src/stores/message-v2/instance-store.test.ts index ee9f36e0..c4cee055 100644 --- a/packages/ui/src/stores/message-v2/instance-store.test.ts +++ b/packages/ui/src/stores/message-v2/instance-store.test.ts @@ -35,3 +35,60 @@ describe("message-v2 permission state", () => { assert.equal(store.getPermissionState(undefined, "permission-2")?.active, true) }) }) + +describe("message-v2 question state", () => { + it("keeps one question attachment when a duplicate moves from global to a tool part", () => { + const store = createInstanceMessageStore("instance-1") + + store.upsertQuestion({ + request: { id: "question-1", sessionID: "session-1", questions: [] }, + enqueuedAt: 1_000, + }) + store.upsertQuestion({ + request: { id: "question-1", sessionID: "session-1", questions: [] }, + messageId: "message-1", + partId: "part-1", + enqueuedAt: 2_000, + }) + + assert.equal(store.state.questions.queue.length, 1) + assert.equal(store.getQuestionState(undefined, "question-1"), null) + assert.equal(store.getQuestionState("message-1", "part-1")?.entry.request.id, "question-1") + assert.equal(store.getQuestionState("message-1", "part-1")?.active, true) + }) + + it("recalculates the active question after removing the first queue entry", () => { + const store = createInstanceMessageStore("instance-1") + + store.upsertQuestion({ request: { id: "question-1", sessionID: "session-1", questions: [] }, enqueuedAt: 1_000 }) + store.upsertQuestion({ request: { id: "question-2", sessionID: "session-1", questions: [] }, enqueuedAt: 2_000 }) + store.removeQuestion("question-1") + + assert.equal(store.state.questions.active?.request.id, "question-2") + assert.equal(store.getQuestionState(undefined, "question-2")?.active, true) + }) + + it("preserves original enqueuedAt when a question is upserted with a newer timestamp", () => { + const store = createInstanceMessageStore("instance-1") + + store.upsertQuestion({ request: { id: "question-1", sessionID: "session-1", questions: [] }, enqueuedAt: 1_000 }) + store.upsertQuestion({ request: { id: "question-2", sessionID: "session-1", questions: [] }, enqueuedAt: 1_500 }) + store.upsertQuestion({ + request: { id: "question-1", sessionID: "session-1", questions: [] }, + messageId: "message-1", + partId: "part-1", + enqueuedAt: 2_000, + }) + + // Queue stays ordered by original enqueue time, not the newer upsert time + assert.equal(store.state.questions.queue.length, 2) + assert.equal(store.state.questions.queue[0].request.id, "question-1") + assert.equal(store.state.questions.queue[0].enqueuedAt, 1_000) + assert.equal(store.state.questions.queue[1].request.id, "question-2") + assert.equal(store.state.questions.queue[1].enqueuedAt, 1_500) + assert.equal(store.state.questions.active?.request.id, "question-1") + + // Resolved question-1 is reachable at its tool part location + assert.equal(store.getQuestionState("message-1", "part-1")?.active, true) + }) +}) diff --git a/packages/ui/src/stores/message-v2/instance-store.ts b/packages/ui/src/stores/message-v2/instance-store.ts index 3373a4cd..58a0fb2b 100644 --- a/packages/ui/src/stores/message-v2/instance-store.ts +++ b/packages/ui/src/stores/message-v2/instance-store.ts @@ -983,13 +983,36 @@ export function createInstanceMessageStore(instanceId: string, hooks?: MessageSt return { entry, active } } - function upsertQuestion(entry: QuestionEntry) { + function mergeQuestionEntry(entry: QuestionEntry): QuestionEntry { + const existing = state.questions.queue.find((item) => item.request.id === entry.request.id) + if (!existing) return entry + return { + ...entry, + messageId: entry.messageId ?? existing.messageId, + partId: entry.partId ?? existing.partId, + enqueuedAt: Math.min(existing.enqueuedAt, entry.enqueuedAt), + } + } + + function upsertQuestion(input: QuestionEntry) { + const entry = mergeQuestionEntry(input) const messageKey = entry.messageId ?? "__global__" const partKey = entry.partId ?? entry.request?.id ?? "__global__" setState( "questions", produce((draft) => { + Object.keys(draft.byMessage).forEach((existingMessageKey) => { + const partEntries = draft.byMessage[existingMessageKey] + Object.keys(partEntries).forEach((existingPartKey) => { + if (partEntries[existingPartKey].request.id === entry.request.id) { + delete partEntries[existingPartKey] + } + }) + if (Object.keys(partEntries).length === 0) { + delete draft.byMessage[existingMessageKey] + } + }) draft.byMessage[messageKey] = draft.byMessage[messageKey] ?? {} draft.byMessage[messageKey][partKey] = entry const existingIndex = draft.queue.findIndex((item) => item.request.id === entry.request.id) @@ -998,9 +1021,8 @@ export function createInstanceMessageStore(instanceId: string, hooks?: MessageSt } else { draft.queue[existingIndex] = entry } - if (!draft.active || draft.active.request.id === entry.request.id) { - draft.active = entry - } + draft.queue.sort((left, right) => left.enqueuedAt - right.enqueuedAt) + draft.active = draft.queue[0] ?? null }), ) }