From f7493d41cb21a14d67c8e337a177912feeb4f782 Mon Sep 17 00:00:00 2001 From: Simon Klee Date: Mon, 18 May 2026 10:08:45 +0200 Subject: [PATCH] tui: dismiss question when tool part ends Questions remained visible after the underlying tool call completed or errored because nothing removed them from the store. Clear each question once its associated tool part reaches a terminal state, and fix recovery to preserve correct blocker priority across all active questions. --- .../src/cli/cmd/run/stream.transport.ts | 9 ++-- .../opencode/src/cli/cmd/tui/context/sync.tsx | 45 +++++++++++++------ .../test/cli/run/stream.transport.test.ts | 12 ++--- packages/opencode/test/cli/tui/sync.test.ts | 39 ++++++++++++++++ 4 files changed, 83 insertions(+), 22 deletions(-) create mode 100644 packages/opencode/test/cli/tui/sync.test.ts diff --git a/packages/opencode/src/cli/cmd/run/stream.transport.ts b/packages/opencode/src/cli/cmd/run/stream.transport.ts index f1f64d7c4..a9aa8ca39 100644 --- a/packages/opencode/src/cli/cmd/run/stream.transport.ts +++ b/packages/opencode/src/cli/cmd/run/stream.transport.ts @@ -530,16 +530,19 @@ function createLayer(input: StreamInput) { const matching = questions.filter(matches) if (matching.length > 0) { - state.data.questions = state.data.questions.filter(matches) + const active = new Set(questions.map((request) => request.id)) + state.data.questions = state.data.questions.filter((request) => active.has(request.id)) bootstrapSessionData({ data: state.data, messages: [], permissions: [], - questions: matching, + questions, }) - for (const request of matching) { + for (const request of questions) { seedBlocker(request.id) } + const priority = Math.min(0, ...state.blockers.values()) - 1 + for (const request of matching) state.blockers.set(request.id, priority) input.trace?.write("question.recover", { sessionID: input.sessionID, requests: matching.map((request) => request.id), diff --git a/packages/opencode/src/cli/cmd/tui/context/sync.tsx b/packages/opencode/src/cli/cmd/tui/context/sync.tsx index 9f8a384f7..fc0103946 100644 --- a/packages/opencode/src/cli/cmd/tui/context/sync.tsx +++ b/packages/opencode/src/cli/cmd/tui/context/sync.tsx @@ -34,6 +34,14 @@ import path from "path" import { useKV } from "./kv" import { aggregateFailures } from "./aggregate-failures" +export function questionToolRequestIndex(requests: readonly QuestionRequest[] | undefined, part: Part) { + if (part.type !== "tool") return -1 + if (part.state.status !== "completed" && part.state.status !== "error") return -1 + return requests?.findIndex( + (request) => request.tool?.messageID === part.messageID && request.tool?.callID === part.callID, + ) ?? -1 +} + export const { use: useSync, provider: SyncProvider } = createSimpleContext({ name: "Sync", init: () => { @@ -304,23 +312,32 @@ export const { use: useSync, provider: SyncProvider } = createSimpleContext({ break } case "message.part.updated": { - const parts = store.part[event.properties.part.messageID] + const part = event.properties.part + const parts = store.part[part.messageID] if (!parts) { - setStore("part", event.properties.part.messageID, [event.properties.part]) - break + setStore("part", part.messageID, [part]) } - const result = Binary.search(parts, event.properties.part.id, (p) => p.id) - if (result.found) { - setStore("part", event.properties.part.messageID, result.index, reconcile(event.properties.part)) - break + if (parts) { + const result = Binary.search(parts, part.id, (p) => p.id) + if (result.found) setStore("part", part.messageID, result.index, reconcile(part)) + if (!result.found) + setStore( + "part", + part.messageID, + produce((draft) => { + draft.splice(result.index, 0, part) + }), + ) } - setStore( - "part", - event.properties.part.messageID, - produce((draft) => { - draft.splice(result.index, 0, event.properties.part) - }), - ) + const index = questionToolRequestIndex(store.question[part.sessionID], part) + if (index !== -1) + setStore( + "question", + part.sessionID, + produce((draft) => { + draft.splice(index, 1) + }), + ) break } diff --git a/packages/opencode/test/cli/run/stream.transport.test.ts b/packages/opencode/test/cli/run/stream.transport.test.ts index 54eaee38f..18f3ab5a5 100644 --- a/packages/opencode/test/cli/run/stream.transport.test.ts +++ b/packages/opencode/test/cli/run/stream.transport.test.ts @@ -835,7 +835,7 @@ describe("run stream transport", () => { callID: "call-question-1", }, } - const stale = { + const other = { ...request, id: "question-old", tool: { messageID: "msg-old", callID: "call-question-old" }, @@ -845,7 +845,7 @@ describe("run stream transport", () => { stream: src.stream, questions: async () => { questionCalls += 1 - return ok(questionCalls === 1 ? [stale] : [stale, request]) + return ok(questionCalls === 1 ? [other] : [other, request]) }, promptAsync: async () => { queueMicrotask(() => { @@ -930,11 +930,13 @@ describe("run stream transport", () => { expect( await waitFor(() => { const item = ui.events.findLast((event) => event.type === "stream.view") - return item?.type === "stream.view" && item.view.type === "prompt" ? item : undefined + return item?.type === "stream.view" && item.view.type === "question" && item.view.request.id === other.id + ? item.view + : undefined }), ).toEqual({ - type: "stream.view", - view: { type: "prompt" }, + type: "question", + request: other, }) ctrl.abort() diff --git a/packages/opencode/test/cli/tui/sync.test.ts b/packages/opencode/test/cli/tui/sync.test.ts new file mode 100644 index 000000000..06ad3efc2 --- /dev/null +++ b/packages/opencode/test/cli/tui/sync.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, test } from "bun:test" +import type { QuestionRequest, ToolPart } from "@opencode-ai/sdk/v2" +import { questionToolRequestIndex } from "@/cli/cmd/tui/context/sync" + +const request = { + id: "question-new", + sessionID: "session-1", + questions: [], + tool: { messageID: "msg-new", callID: "call-new" }, +} satisfies QuestionRequest + +function part(status: "running" | "completed" | "error", tool = "question", callID = "call-new"): ToolPart { + return { + id: "part-new", + sessionID: "session-1", + messageID: "msg-new", + type: "tool", + callID, + tool, + state: + status === "running" + ? { status, input: {}, time: { start: 1 } } + : status === "completed" + ? { status, input: {}, output: "", title: "question", metadata: {}, time: { start: 1, end: 2 } } + : { status, input: {}, error: "Tool execution aborted", time: { start: 1, end: 2 } }, + } +} + +describe("tui sync", () => { + test("matches terminal tool-owned question requests", () => { + const stale = { ...request, id: "question-old", tool: { messageID: "msg-old", callID: "call-old" } } + + expect(questionToolRequestIndex([stale, request], part("error"))).toBe(1) + expect(questionToolRequestIndex([stale, request], part("completed"))).toBe(1) + expect(questionToolRequestIndex([stale, request], part("completed", "plan_exit"))).toBe(1) + expect(questionToolRequestIndex([stale, request], part("running"))).toBe(-1) + expect(questionToolRequestIndex([stale, request], part("error", "bash", "call-other"))).toBe(-1) + }) +})