From 0587e8be535b4c1af508e11b023d2a5d82d28883 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Fri, 30 Oct 2026 17:17:49 +0900 Subject: [PATCH 1/7] fix(task): strip trailing partial text when resuming an interrupted task What: resumeTaskFromHistory() now removes a trailing partial `text` bubble alongside the partial `reasoning` bubble it already removed, keeping every other partial say type (e.g. command output). Why: the webview derives its streaming lock from the message list alone. A trailing message with partial === true, or a last api_req_started whose text JSON has no cost, both mean "request still in flight": ChatRow renders "API Request..." and ChatTextArea swaps Send for Stop, so every message the user types is queued instead of sent. An abandoned task switch skips abortStream() (the only finalizer that un-partials the trailing message and writes cost/cancelReason), so a task abandoned mid-stream persists that tail. The existing resume self-heal removed partial reasoning but not partial text, so switching back into such a task stayed stuck on every future resume until the webview was reloaded. Fixes #257. Impact: a resumed task no longer ends on a streaming artifact, so the chat re-enables its input. Partial command output and finalized messages are preserved. The transient switch-window lock and the deeper fix at abandonment time remain out of scope. --- src/core/task/Task.ts | 19 ++- .../task/__tests__/Task.persistence.spec.ts | 108 ++++++++++++++++++ 2 files changed, 122 insertions(+), 5 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 3202358c09..45fcc92dae 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -3145,15 +3145,24 @@ export class Task extends EventEmitter implements TaskLike { } } - // Incomplete reasoning has no matching API-history entry and would become - // an orphaned bubble when the resumed request starts fresh reasoning. + // Partial reasoning and text have no API-history entry, since the assistant + // message is only written once the stream completes, so an interrupted stream + // leaves them orphaned. A trailing partial bubble also keeps the webview in its + // streaming state, which leaves the resumed chat stuck on "API Request..." with + // an input that only queues (see #257). Other partial say types (e.g. command + // output) record work that already ran, so they are kept. while (modifiedClineMessages.length > 0) { const lastMessage = modifiedClineMessages[modifiedClineMessages.length - 1] - if (lastMessage.type === "say" && lastMessage.say === "reasoning" && lastMessage.partial === true) { - modifiedClineMessages.pop() - } else { + const isPartialStreamBubble = + lastMessage.type === "say" && + (lastMessage.say === "reasoning" || lastMessage.say === "text") && + lastMessage.partial === true + + if (!isPartialStreamBubble) { break } + + modifiedClineMessages.pop() } // Since we don't use `api_req_finished` anymore, we need to check if the diff --git a/src/core/task/__tests__/Task.persistence.spec.ts b/src/core/task/__tests__/Task.persistence.spec.ts index 109eee8dc0..7b023facfe 100644 --- a/src/core/task/__tests__/Task.persistence.spec.ts +++ b/src/core/task/__tests__/Task.persistence.spec.ts @@ -2332,6 +2332,114 @@ describe("Task persistence", () => { expect(mockSaveTaskMessages).not.toHaveBeenCalled() }) + // #257: the webview derives `isStreaming` from the message list, so a trailing + // `partial` bubble or a cost-less `api_req_started` keeps the resumed chat on + // "API Request..." with the send button replaced by Stop. Resume must trim it. + it.each([ + { + name: "partial text", + tail: [{ ts: 2, type: "say" as const, say: "text" as const, text: "Half a sentence", partial: true }], + }, + { + name: "partial text behind a dangling api_req_started", + tail: [ + { + ts: 2, + type: "say" as const, + say: "api_req_started" as const, + text: '{"apiProtocol":"anthropic"}', + }, + { ts: 3, type: "say" as const, say: "text" as const, text: "Half a sentence", partial: true }, + ], + }, + { + name: "a dangling api_req_started alone", + tail: [ + { + ts: 2, + type: "say" as const, + say: "api_req_started" as const, + text: '{"apiProtocol":"anthropic"}', + }, + ], + }, + ])("strips a $name streaming tail before prompting to resume", async ({ tail }) => { + mockReadTaskMessages.mockResolvedValue([ + { ts: 1, type: "say", say: "text", text: "Original task" }, + ...tail, + ]) + mockReadApiMessages.mockResolvedValue([ + { role: "user", content: [{ type: "text", text: "Original task" }] }, + ]) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + historyItem: { + id: "issue-257-streaming-tail", + number: 1, + ts: 1, + task: "Original task", + tokensIn: 10, + tokensOut: 5, + totalCost: 0.001, + }, + startTask: false, + }) + vi.spyOn(task, "ask").mockImplementation(async () => { + // The resumed chat must not end on anything the webview reads as streaming. + expect(task.clineMessages.at(-1)).toEqual( + expect.objectContaining({ say: "text", text: "Original task" }), + ) + expect(task.clineMessages).not.toContainEqual(expect.objectContaining({ partial: true })) + expect(task.clineMessages).not.toContainEqual(expect.objectContaining({ say: "api_req_started" })) + throw new Error("stop after hydration") + }) + + await expect(getTaskPersistenceAccess(task).resumeTaskFromHistory()).rejects.toThrow("stop after hydration") + expect(mockSaveTaskMessages).not.toHaveBeenCalled() + }) + + it("keeps partial command output and finalized text when resuming", async () => { + mockReadTaskMessages.mockResolvedValue([ + { ts: 1, type: "say", say: "text", text: "Original task" }, + { ts: 2, type: "say", say: "text", text: "Finalized answer", partial: false }, + { ts: 3, type: "say", say: "command_output", text: "npm test\n", partial: true }, + ]) + mockReadApiMessages.mockResolvedValue([ + { role: "user", content: [{ type: "text", text: "Original task" }] }, + ]) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + historyItem: { + id: "issue-257-preserve-non-stream-tail", + number: 1, + ts: 1, + task: "Original task", + tokensIn: 10, + tokensOut: 5, + totalCost: 0.001, + }, + startTask: false, + }) + vi.spyOn(task, "ask").mockImplementation(async () => { + // Partial command output records work that already ran; dropping it would + // erase evidence of the interrupted command from the transcript. + expect(task.clineMessages).toContainEqual( + expect.objectContaining({ say: "command_output", text: "npm test\n", partial: true }), + ) + expect(task.clineMessages).toContainEqual( + expect.objectContaining({ say: "text", text: "Finalized answer", partial: false }), + ) + throw new Error("stop after hydration") + }) + + await expect(getTaskPersistenceAccess(task).resumeTaskFromHistory()).rejects.toThrow("stop after hydration") + expect(mockSaveTaskMessages).not.toHaveBeenCalled() + }) + it("stops before hydrating either history when the task is evicted during the API read", async () => { const apiMessagesDeferred = createDeferred }>>() From 7f665f1bdd16f1be2c24b9161e0230597bb36f78 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Fri, 30 Oct 2026 18:31:50 +0900 Subject: [PATCH 2/7] test(webview): pin the resume strip against the webview streaming heuristic What: add a webview-ui spec that renders ChatView with the message shapes Task.resumeTaskFromHistory() produces and asserts the `isStreaming` prop it passes to ChatTextArea. The host strip list is read out of Task.ts, and the streaming partial say types are declared explicitly so a narrowed host list fails the spec. Why: Task.persistence.spec.ts:2335-2441 pins the host-side strip list {reasoning, text}, and ChatView.tsx:566-608 derives the streaming lock from the message list alone. The coupling between the two lived only in a comment, so dropping a say type from the host strip list, or changing the memo, would silently re-lock a resumed chat with no failing test. Impact: test-only, zero production lines. Narrowing the host strip list to `reasoning` fails 2 of the 8 cases; the committed src blob is unchanged (Task.ts 14/5, Task.persistence.spec.ts 108/0). --- ...atView.resume-strip-not-streaming.spec.tsx | 278 ++++++++++++++++++ 1 file changed, 278 insertions(+) create mode 100644 webview-ui/src/components/chat/__tests__/ChatView.resume-strip-not-streaming.spec.tsx diff --git a/webview-ui/src/components/chat/__tests__/ChatView.resume-strip-not-streaming.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.resume-strip-not-streaming.spec.tsx new file mode 100644 index 0000000000..b922d365b1 --- /dev/null +++ b/webview-ui/src/components/chat/__tests__/ChatView.resume-strip-not-streaming.spec.tsx @@ -0,0 +1,278 @@ +// pnpm --filter @roo-code/vscode-webview test src/components/chat/__tests__/ChatView.resume-strip-not-streaming.spec.tsx + +import fs from "fs" +import path from "path" +import { fileURLToPath } from "url" + +import React from "react" +import { renderWithExtensionState, makeExtensionState, waitFor } from "@/utils/test-utils" + +import type { ClineMessage } from "@roo-code/types" + +import ChatView, { ChatViewProps } from "../ChatView" + +// #257: `Task.resumeTaskFromHistory()` trims a streaming tail before hydrating the +// resumed chat (src/core/task/Task.ts:3154-3184), and the webview derives its +// streaming lock from the message list alone (ChatView.tsx:566-608). The host spec +// (Task.persistence.spec.ts:2335-2441) pins the strip; this spec pins the other half +// of the contract: the shape the strip guarantees must not read as streaming, and +// the shape it removes must. The strip list is read from the host source instead of +// being copied here, so widening or narrowing it on the host side fails this spec. + +// __tests__ -> chat -> components -> src -> webview-ui -> repo root +const HOST_TASK_SOURCE_PATH = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "../../../../../src/core/task/Task.ts", +) + +/** + * Extracts the `say` types the host resume cleanup pops from the source expression + * `(lastMessage.say === "reasoning" || lastMessage.say === "text")`. Reading the + * source keeps this spec coupled to the host strip list; a hand-copied list would + * let the two sides drift silently, which is the coupling this spec exists to catch. + */ +const readHostStrippedPartialSayTypes = (): string[] => { + const source = fs.readFileSync(HOST_TASK_SOURCE_PATH, "utf8") + const start = source.indexOf("const isPartialStreamBubble =") + + if (start === -1) { + throw new Error(`Could not find the resume strip expression in ${HOST_TASK_SOURCE_PATH}`) + } + + const end = source.indexOf("lastMessage.partial === true", start) + + if (end === -1) { + throw new Error(`Could not find the end of the resume strip expression in ${HOST_TASK_SOURCE_PATH}`) + } + + return [...source.slice(start, end).matchAll(/lastMessage\.say === "([^"]+)"/g)].map((match) => match[1]!) +} + +const HOST_STRIPPED_PARTIAL_SAY_TYPES = readHostStrippedPartialSayTypes() + +/** + * `say` types the streaming loop can emit with `partial: true` and a text body. + * Producers: `presentAssistantMessage.ts:443` (text) and `Task.ts:4215` (reasoning). + * Every one of them must be in the host strip list, otherwise an interrupted stream + * leaves the bubble behind and the resumed chat re-locks on `partial === true`. + * + * This list is the contract the host strip is checked against, so it is written + * explicitly here instead of being derived from the host source. Deriving both sides + * from the same expression would let a narrowed host list pass unnoticed. + */ +const STREAMING_PARTIAL_SAY_TYPES = ["reasoning", "text"] as const + +/** + * Applies the resume-time cleanup from `Task.resumeTaskFromHistory()` so the + * post-strip assertions are derived from the pre-strip history the host reads off + * disk instead of being hand-written twice. + */ +const applyHostResumeStrip = (messages: ClineMessage[]): ClineMessage[] => { + const stripped = [...messages] + + while (stripped.length > 0) { + const lastMessage = stripped[stripped.length - 1] + + if ( + lastMessage?.type !== "say" || + !HOST_STRIPPED_PARTIAL_SAY_TYPES.includes(lastMessage.say ?? "") || + lastMessage.partial !== true + ) { + break + } + + stripped.pop() + } + + const lastApiReqStartedIndex = stripped.findLastIndex( + (message) => message.type === "say" && message.say === "api_req_started", + ) + + if (lastApiReqStartedIndex !== -1) { + const lastApiReqStarted = stripped[lastApiReqStartedIndex] + const { cost, cancelReason } = JSON.parse(lastApiReqStarted?.text || "{}") + + if (cost === undefined && cancelReason === undefined) { + stripped.splice(lastApiReqStartedIndex, 1) + } + } + + return stripped +} + +vi.mock("@src/utils/vscode", () => ({ + vscode: { + postMessage: vi.fn(), + }, +})) + +vi.mock("use-sound", () => ({ + default: vi.fn().mockImplementation(() => [vi.fn()]), +})) + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ + t: (key: string) => key, + }), + initReactI18next: { + type: "3rdParty", + init: () => {}, + }, + Trans: ({ i18nKey, children }: { i18nKey: string; children?: React.ReactNode }) => <>{children || i18nKey}, +})) + +vi.mock("../ChatRow", () => ({ + default: function MockChatRow({ message }: { message: ClineMessage }) { + return
{JSON.stringify(message)}
+ }, +})) + +// The send button is the user-visible half of the lock: `isStreaming` swaps Send +// for Stop and routes input into the queue instead of sending. +vi.mock("../ChatTextArea", () => ({ + ChatTextArea: React.forwardRef(function MockChatTextArea( + { isStreaming }: { isStreaming?: boolean }, + _ref: React.ForwardedRef, + ) { + return
+ }), +})) + +vi.mock("../AutoApproveMenu", () => ({ + default: () => null, +})) + +vi.mock("react-virtuoso", () => ({ + Virtuoso: function MockVirtuoso({ + data, + itemContent, + }: { + data: ClineMessage[] + itemContent: (index: number, item: ClineMessage) => React.ReactNode + }) { + return ( +
+ {data.map((item, index) => ( +
{itemContent(index, item)}
+ ))} +
+ ) + }, +})) + +vi.mock("../../common/VersionIndicator", () => ({ + default: () => null, +})) + +vi.mock("@src/components/welcome/RooTips", () => ({ + default: () => null, +})) + +vi.mock("@src/components/welcome/RooHero", () => ({ + default: () => null, +})) + +vi.mock("../common/TelemetryBanner", () => ({ + default: () => null, +})) + +const defaultProps: ChatViewProps = { + isHidden: false, + showAnnouncement: false, + hideAnnouncement: () => {}, +} + +const renderWithMessages = (clineMessages: ClineMessage[]) => + renderWithExtensionState(, { + state: makeExtensionState({ clineMessages }), + }) + +const expectStreaming = async (expected: boolean) => { + await waitFor(() => { + expect(document.querySelector('[data-testid="chat-textarea"]')).toHaveAttribute( + "data-is-streaming", + String(expected), + ) + }) +} + +const taskMessage: ClineMessage = { type: "say", say: "text", ts: 1, text: "Original task" } +const settledApiReqStarted: ClineMessage = { + type: "say", + say: "api_req_started", + ts: 3, + text: JSON.stringify({ apiProtocol: "anthropic", cost: 0.002 }), +} +const danglingApiReqStarted: ClineMessage = { + type: "say", + say: "api_req_started", + ts: 3, + text: JSON.stringify({ apiProtocol: "anthropic" }), +} + +describe("ChatView - resume strip leaves a non-streaming chat", () => { + beforeEach(() => vi.clearAllMocks()) + + it("keeps every streaming partial say type in the host strip list", () => { + // A silent change to the parse target would otherwise leave the assertions + // below running against an empty list and passing for the wrong reason. + expect(HOST_STRIPPED_PARTIAL_SAY_TYPES.length).toBeGreaterThan(0) + // Dropping a say type from the host strip list is exactly the decoupling this + // spec exists to catch: the bubble survives resume and the chat re-locks. + expect(HOST_STRIPPED_PARTIAL_SAY_TYPES).toEqual(expect.arrayContaining([...STREAMING_PARTIAL_SAY_TYPES])) + }) + + it.each(STREAMING_PARTIAL_SAY_TYPES)( + "reads a trailing partial %s bubble as streaming before the host strips it", + async (say) => { + // This is the shape that reaches the webview when the host strip list loses a + // say type: the resumed chat renders "API Request..." with a Stop button. + renderWithMessages([taskMessage, { type: "say", say, ts: 2, text: "Half a sentence", partial: true }]) + + await expectStreaming(true) + }, + ) + + it("reads a trailing dangling api_req_started as streaming before the host strips it", async () => { + renderWithMessages([taskMessage, danglingApiReqStarted]) + + await expectStreaming(true) + }) + + it.each(STREAMING_PARTIAL_SAY_TYPES)( + "reads the post-strip shape left by stripping a partial %s tail as not streaming", + async (say) => { + const partialTail: ClineMessage = { type: "say", say, ts: 2, text: "Half a sentence", partial: true } + const postStrip = applyHostResumeStrip([taskMessage, partialTail]) + + // If the host strip list loses this say type, the partial tail survives and the + // resumed chat stays locked, so this assertion is what catches the drift. + expect(postStrip).toEqual([taskMessage]) + renderWithMessages(postStrip) + + await expectStreaming(false) + }, + ) + + it("reads the post-strip shape left by stripping a dangling api_req_started as not streaming", async () => { + const postStrip = applyHostResumeStrip([taskMessage, settledApiReqStarted, danglingApiReqStarted]) + + expect(postStrip).toEqual([taskMessage, settledApiReqStarted]) + renderWithMessages(postStrip) + + await expectStreaming(false) + }) + + it("does not stream on a kept partial command_output tail", async () => { + // The host deliberately keeps partial command output (it records work that + // already ran). `combineCommandSequences` folds it into the command ask, so it + // never becomes the last message the streaming check looks at. + renderWithMessages([ + taskMessage, + { type: "ask", ask: "command", ts: 2, text: "npm test", partial: false }, + { type: "say", say: "command_output", ts: 3, text: "npm test\n", partial: true }, + ]) + + await expectStreaming(false) + }) +}) From 423d83b6aa688dd9373e4239e1ad02d0de1576c4 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Fri, 30 Oct 2026 18:52:37 +0900 Subject: [PATCH 3/7] fix(task): restore interrupted text bubbles as finalized rows on resume What: resumeTaskFromHistory() now collects the trailing partial `reasoning` and `text` bubbles instead of dropping them outright, and after reading API history re-adds each partial `text` bubble whose complete text already appears in the last assistant turn, with `partial` cleared. Partial `reasoning` and unmatched `text` bubbles are still discarded. Why: CodeRabbit flagged that the strip discards text the model still retains. The claim is reachable: recursivelyMakeRooRequests appends the assistant turn to API history before it releases the streaming presenter that clears the bubble's `partial` flag, so an abort landing between those two steps leaves a partial bubble and a complete turn coexisting. Dropping the row erased text that is still in the model's context. The recommendation as written would have reintroduced #257: ChatView's streaming memo returns true for any trailing `partial === true` row, so retaining the row unchanged re-locks the resumed chat on "API Request..." with a queuing input. Clearing `partial` keeps the text visible as finalized content and leaves the input enabled, which is strictly better than dropping the row. The comparison re-applies the streaming-time `` strip, because presentAssistantMessage strips that markup from the bubble but the API turn is built from the raw accumulator. Impact: an interrupted answer that already reached API history survives resume as finalized text instead of vanishing. Orphaned partial text, partial reasoning, and the #257 lock behavior are unchanged. The adjacent comment now states that partial text can already have an API-history entry. --- src/core/task/Task.ts | 56 ++++- .../task/__tests__/Task.persistence.spec.ts | 221 ++++++++++++++++++ ...atView.resume-strip-not-streaming.spec.tsx | 52 ++++- 3 files changed, 322 insertions(+), 7 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 45fcc92dae..b3eaa26de0 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -236,6 +236,35 @@ function queuedResponseForAsk(type: ClineAsk, text?: string): QueuedAskResolutio return { response: "messageResponse", requiresDurableAck: type === "completion_result" } } +/** + * Whether an assistant API turn already carries the complete text of a streamed + * `say("text")` bubble that was interrupted before its UI finalize. + * + * `recursivelyMakeRooRequests` appends the assistant turn to API history before it + * releases the streaming presenter that clears the bubble's `partial` flag, so an + * abort landing between those two steps leaves a partial bubble and a complete + * turn coexisting. The bubble's text is then not orphaned and must not be + * discarded. + * + * The comparison re-applies the streaming-time `` strip that + * `presentAssistantMessage` applies to the bubble but not to the raw accumulator + * the API turn is built from. + */ +function hasAssistantTextTurnFor(apiMessages: ApiMessage[], partialText: string): boolean { + const expected = partialText.replace(/\s?/g, "").replace(/\s?<\/thinking>/g, "") + const lastAssistantTurn = findLast(apiMessages, (message) => message.role === "assistant") + + if (!lastAssistantTurn || !Array.isArray(lastAssistantTurn.content)) { + return false + } + + return lastAssistantTurn.content.some( + (block) => + block.type === "text" && + block.text.replace(/\s?/g, "").replace(/\s?<\/thinking>/g, "") === expected, + ) +} + const FORCED_CONTEXT_REDUCTION_PERCENT = 75 // Keep 75% of context (remove 25%) on context window errors const MAX_CONTEXT_WINDOW_RETRIES = 3 // Maximum retries for context window errors @@ -3145,12 +3174,20 @@ export class Task extends EventEmitter implements TaskLike { } } - // Partial reasoning and text have no API-history entry, since the assistant - // message is only written once the stream completes, so an interrupted stream - // leaves them orphaned. A trailing partial bubble also keeps the webview in its + // A trailing partial `reasoning` or `text` bubble keeps the webview in its // streaming state, which leaves the resumed chat stuck on "API Request..." with // an input that only queues (see #257). Other partial say types (e.g. command // output) record work that already ran, so they are kept. + // + // The two types are not equally orphaned, though. `recursivelyMakeRooRequests` + // appends the assistant turn to API history before it releases the streaming + // presenter that clears the bubble's `partial` flag, so an abort landing between + // those two steps leaves a partial text bubble whose complete text is already in + // API history. Those are restored below as finalized rows instead of being + // discarded; reasoning keeps the plain drop because it is never re-rendered + // from API history. + const trailingPartialStreamBubbles: ClineMessage[] = [] + while (modifiedClineMessages.length > 0) { const lastMessage = modifiedClineMessages[modifiedClineMessages.length - 1] const isPartialStreamBubble = @@ -3162,7 +3199,7 @@ export class Task extends EventEmitter implements TaskLike { break } - modifiedClineMessages.pop() + trailingPartialStreamBubbles.unshift(modifiedClineMessages.pop()!) } // Since we don't use `api_req_finished` anymore, we need to check if the @@ -3191,6 +3228,17 @@ export class Task extends EventEmitter implements TaskLike { return } + // Restore the interrupted text bubbles whose complete text already reached API + // history, as finalized rows. Clearing `partial` is what keeps the retention from + // re-locking the chat: ChatView's streaming memo returns true for any trailing + // `partial === true` row (ChatView.tsx:583-587), so a retained partial would + // reintroduce exactly the #257 lock this cleanup exists to remove. + for (const bubble of trailingPartialStreamBubbles) { + if (bubble.say === "text" && hasAssistantTextTurnFor(savedApiConversationHistory, bubble.text ?? "")) { + modifiedClineMessages.push({ ...bubble, partial: false }) + } + } + // Avoid a standalone write during hydration. The resume ask will persist only // after all history reads succeed and the task is still active. this.hydrateClineMessages(modifiedClineMessages) diff --git a/src/core/task/__tests__/Task.persistence.spec.ts b/src/core/task/__tests__/Task.persistence.spec.ts index 7b023facfe..b7336291f3 100644 --- a/src/core/task/__tests__/Task.persistence.spec.ts +++ b/src/core/task/__tests__/Task.persistence.spec.ts @@ -2440,6 +2440,227 @@ describe("Task persistence", () => { expect(mockSaveTaskMessages).not.toHaveBeenCalled() }) + // The assistant turn is appended to API history before the presenter that + // clears `partial` runs, so an abort between them leaves a partial text bubble + // whose complete text is already persisted. Dropping it would erase text the + // model can still see; keeping it partial would re-lock the resumed chat on + // ChatView's `partial === true` heuristic. Restore it as a finalized row. + it("restores a partial text bubble as finalized when the complete turn is in API history", async () => { + mockReadTaskMessages.mockResolvedValue([ + { ts: 1, type: "say", say: "text", text: "Original task" }, + { ts: 2, type: "say", say: "text", text: "Half a sentence", partial: true }, + ]) + mockReadApiMessages.mockResolvedValue([ + { role: "user", content: [{ type: "text", text: "Original task" }] }, + { role: "assistant", content: [{ type: "text", text: "Half a sentence" }] }, + ]) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + historyItem: { + id: "issue-257-restore-partial-text", + number: 1, + ts: 1, + task: "Original task", + tokensIn: 10, + tokensOut: 5, + totalCost: 0.001, + }, + startTask: false, + }) + vi.spyOn(task, "ask").mockImplementation(async () => { + // Retained, but finalized: the text survives and the row no longer reads as + // streaming, so the resumed chat keeps its input enabled. + expect(task.clineMessages).toContainEqual( + expect.objectContaining({ say: "text", text: "Half a sentence", partial: false }), + ) + expect(task.clineMessages).not.toContainEqual(expect.objectContaining({ partial: true })) + throw new Error("stop after hydration") + }) + + await expect(getTaskPersistenceAccess(task).resumeTaskFromHistory()).rejects.toThrow("stop after hydration") + expect(mockSaveTaskMessages).not.toHaveBeenCalled() + }) + + it("compares the partial text against API history after the streaming thinking-tag strip", async () => { + // presentAssistantMessage strips `` from the bubble but the API turn is + // built from the raw accumulator, so the two texts differ by exactly that markup. + mockReadTaskMessages.mockResolvedValue([ + { ts: 1, type: "say", say: "text", text: "Original task" }, + { ts: 2, type: "say", say: "text", text: "Answer in progress", partial: true }, + ]) + mockReadApiMessages.mockResolvedValue([ + { role: "user", content: [{ type: "text", text: "Original task" }] }, + { role: "assistant", content: [{ type: "text", text: "\nAnswer in progress" }] }, + ]) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + historyItem: { + id: "issue-257-restore-thinking-strip", + number: 1, + ts: 1, + task: "Original task", + tokensIn: 10, + tokensOut: 5, + totalCost: 0.001, + }, + startTask: false, + }) + vi.spyOn(task, "ask").mockImplementation(async () => { + expect(task.clineMessages).toContainEqual( + expect.objectContaining({ say: "text", text: "Answer in progress", partial: false }), + ) + throw new Error("stop after hydration") + }) + + await expect(getTaskPersistenceAccess(task).resumeTaskFromHistory()).rejects.toThrow("stop after hydration") + expect(mockSaveTaskMessages).not.toHaveBeenCalled() + }) + + it.each([ + { + name: "the last assistant turn holds different text", + apiHistory: [ + { role: "assistant" as const, content: [{ type: "text" as const, text: "An older answer" }] }, + ], + }, + { + name: "the last assistant turn has no text block", + apiHistory: [ + { + role: "assistant" as const, + content: [{ type: "tool_use" as const, id: "toolu_1", name: "read_file", input: {} }], + }, + ], + }, + { + name: "API history holds no assistant turn at all", + apiHistory: [] as Array<{ role: "assistant"; content: Array<{ type: "text"; text: string }> }>, + }, + { + // Pins the comparison to the LAST assistant turn: an older turn that happens + // to match must not resurrect a bubble the newest turn does not account for. + name: "only an older assistant turn holds the text", + apiHistory: [ + { role: "assistant" as const, content: [{ type: "text" as const, text: "Half a sentence" }] }, + { role: "user" as const, content: [{ type: "text" as const, text: "Follow up" }] }, + { role: "assistant" as const, content: [{ type: "text" as const, text: "A newer answer" }] }, + ], + }, + ])("strips the partial text bubble when $name", async ({ apiHistory }) => { + // No matching turn means the text exists nowhere else, so the orphaned bubble is + // dropped exactly as before this change. + mockReadTaskMessages.mockResolvedValue([ + { ts: 1, type: "say", say: "text", text: "Original task" }, + { ts: 2, type: "say", say: "text", text: "Half a sentence", partial: true }, + ]) + mockReadApiMessages.mockResolvedValue([ + { role: "user", content: [{ type: "text", text: "Original task" }] }, + ...apiHistory, + ]) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + historyItem: { + id: "issue-257-orphaned-partial-text", + number: 1, + ts: 1, + task: "Original task", + tokensIn: 10, + tokensOut: 5, + totalCost: 0.001, + }, + startTask: false, + }) + vi.spyOn(task, "ask").mockImplementation(async () => { + expect(task.clineMessages).toContainEqual( + expect.objectContaining({ say: "text", text: "Original task" }), + ) + expect(task.clineMessages).not.toContainEqual(expect.objectContaining({ text: "Half a sentence" })) + throw new Error("stop after hydration") + }) + + await expect(getTaskPersistenceAccess(task).resumeTaskFromHistory()).rejects.toThrow("stop after hydration") + expect(mockSaveTaskMessages).not.toHaveBeenCalled() + }) + + it("keeps dropping a partial reasoning bubble even when an assistant turn exists", async () => { + // Reasoning is never re-rendered from API history, so a matching assistant turn + // must not resurrect the bubble. This pins the CR-requested split. + mockReadTaskMessages.mockResolvedValue([ + { ts: 1, type: "say", say: "text", text: "Original task" }, + { ts: 2, type: "say", say: "reasoning", text: "Thinking about it", partial: true }, + ]) + mockReadApiMessages.mockResolvedValue([ + { role: "user", content: [{ type: "text", text: "Original task" }] }, + { role: "assistant", content: [{ type: "text", text: "Thinking about it" }] }, + ]) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + historyItem: { + id: "issue-257-reasoning-still-dropped", + number: 1, + ts: 1, + task: "Original task", + tokensIn: 10, + tokensOut: 5, + totalCost: 0.001, + }, + startTask: false, + }) + vi.spyOn(task, "ask").mockImplementation(async () => { + expect(task.clineMessages).not.toContainEqual(expect.objectContaining({ say: "reasoning" })) + throw new Error("stop after hydration") + }) + + await expect(getTaskPersistenceAccess(task).resumeTaskFromHistory()).rejects.toThrow("stop after hydration") + expect(mockSaveTaskMessages).not.toHaveBeenCalled() + }) + + it("does not restore a partial text bubble when the task is evicted during the API read", async () => { + // The restore sits after the abort guard, so an evicted task must not resurrect + // the row into a history it is about to discard. + const apiMessagesDeferred = + createDeferred }>>() + mockReadTaskMessages.mockResolvedValue([ + { ts: 1, type: "say", say: "text", text: "Original task" }, + { ts: 2, type: "say", say: "text", text: "Half a sentence", partial: true }, + ]) + mockReadApiMessages.mockReturnValue(apiMessagesDeferred.promise) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + historyItem: { + id: "issue-257-restore-evicted", + number: 1, + ts: 1, + task: "Original task", + tokensIn: 10, + tokensOut: 5, + totalCost: 0.001, + }, + startTask: false, + }) + const resumePromise = getTaskPersistenceAccess(task).resumeTaskFromHistory() + await vi.waitFor(() => expect(mockReadApiMessages).toHaveBeenCalled()) + + await task.abortTask(true) + mockSaveTaskMessages.mockClear() + vi.mocked(mockProvider.updateTaskHistory).mockClear() + apiMessagesDeferred.resolve([{ role: "assistant", content: [{ type: "text", text: "Half a sentence" }] }]) + await resumePromise + + expect(task.clineMessages).toHaveLength(0) + expect(mockSaveTaskMessages).not.toHaveBeenCalled() + }) + it("stops before hydrating either history when the task is evicted during the API read", async () => { const apiMessagesDeferred = createDeferred }>>() diff --git a/webview-ui/src/components/chat/__tests__/ChatView.resume-strip-not-streaming.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.resume-strip-not-streaming.spec.tsx index b922d365b1..554fd669d7 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.resume-strip-not-streaming.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.resume-strip-not-streaming.spec.tsx @@ -12,7 +12,7 @@ import type { ClineMessage } from "@roo-code/types" import ChatView, { ChatViewProps } from "../ChatView" // #257: `Task.resumeTaskFromHistory()` trims a streaming tail before hydrating the -// resumed chat (src/core/task/Task.ts:3154-3184), and the webview derives its +// resumed chat (src/core/task/Task.ts:3176-3240), and the webview derives its // streaming lock from the message list alone (ChatView.tsx:566-608). The host spec // (Task.persistence.spec.ts:2335-2441) pins the strip; this spec pins the other half // of the contract: the shape the strip guarantees must not read as streaming, and @@ -66,9 +66,15 @@ const STREAMING_PARTIAL_SAY_TYPES = ["reasoning", "text"] as const * Applies the resume-time cleanup from `Task.resumeTaskFromHistory()` so the * post-strip assertions are derived from the pre-strip history the host reads off * disk instead of being hand-written twice. + * + * `assistantTexts` mirrors the API-history turns the host reads before deciding + * whether an interrupted text bubble can be restored: a partial `text` bubble whose + * complete text is already in API history comes back as a finalized row, everything + * else in the streaming tail is dropped. */ -const applyHostResumeStrip = (messages: ClineMessage[]): ClineMessage[] => { +const applyHostResumeStrip = (messages: ClineMessage[], assistantTexts: string[] = []): ClineMessage[] => { const stripped = [...messages] + const trailingPartialStreamBubbles: ClineMessage[] = [] while (stripped.length > 0) { const lastMessage = stripped[stripped.length - 1] @@ -81,7 +87,7 @@ const applyHostResumeStrip = (messages: ClineMessage[]): ClineMessage[] => { break } - stripped.pop() + trailingPartialStreamBubbles.unshift(stripped.pop()!) } const lastApiReqStartedIndex = stripped.findLastIndex( @@ -97,6 +103,12 @@ const applyHostResumeStrip = (messages: ClineMessage[]): ClineMessage[] => { } } + for (const bubble of trailingPartialStreamBubbles) { + if (bubble.say === "text" && assistantTexts.includes(bubble.text ?? "")) { + stripped.push({ ...bubble, partial: false }) + } + } + return stripped } @@ -275,4 +287,38 @@ describe("ChatView - resume strip leaves a non-streaming chat", () => { await expectStreaming(false) }) + + it("reads the restored finalized text shape as not streaming", async () => { + // The assistant turn is appended to API history before the presenter that + // clears `partial` runs, so an abort between them leaves a partial text bubble + // whose complete text is already persisted. The host restores it with + // `partial: false`; retaining the text must not reinstate the streaming lock. + const partialTail: ClineMessage = { type: "say", say: "text", ts: 2, text: "Half a sentence", partial: true } + const postStrip = applyHostResumeStrip([taskMessage, partialTail], ["Half a sentence"]) + + expect(postStrip).toEqual([taskMessage, { ...partialTail, partial: false }]) + renderWithMessages(postStrip) + + await expectStreaming(false) + }) + + it("still streams if a restored text row is left partial", async () => { + // Guards the `partial: false` half of the restore: keeping the row without + // clearing the flag would reintroduce exactly the #257 lock the strip removes. + renderWithMessages([taskMessage, { type: "say", say: "text", ts: 2, text: "Half a sentence", partial: true }]) + + await expectStreaming(true) + }) + + it("drops a partial text tail whose text is absent from API history", async () => { + // No matching turn means the text exists nowhere else, so the bubble stays + // orphaned and is removed rather than restored. + const partialTail: ClineMessage = { type: "say", say: "text", ts: 2, text: "Half a sentence", partial: true } + const postStrip = applyHostResumeStrip([taskMessage, partialTail], ["An older answer"]) + + expect(postStrip).toEqual([taskMessage]) + renderWithMessages(postStrip) + + await expectStreaming(false) + }) }) From 38d242e2e0e19b9b60e093a6786833d9db318f3e Mon Sep 17 00:00:00 2001 From: myk1yt Date: Fri, 30 Oct 2026 21:18:39 +0900 Subject: [PATCH 4/7] fix(task): require the last API turn to be the assistant turn for the resume restore What: hasAssistantTextTurnFor() now reads the final API message and returns false unless its role is assistant, instead of scanning back to the last assistant turn with findLast. A new Task.persistence case covers a trailing user turn. Why: the previous check reached backwards past a newer user turn. That shape is reachable: each iteration appends the request's user turn first, streams, then throws at the post-stream abort guard before appending the assistant turn. An abort at that guard therefore leaves API history ending on the user turn while a say("text") bubble is still partial. findLast would then match an older assistant turn and finalize a bubble whose text is absent from the newest turn the model sees, laundering unsettled text into the transcript as if it were settled. The stricter check cannot lose the legitimate restore case: there is no API mutation between the assistant append and the presenter release, reasoning is embedded inside the assistant message rather than appended as its own entry, and flushPendingToolResultsToHistory returns early once abort is set, so the assistant append really is the last entry whenever the restore should fire. Impact: a partial bubble is restored only when the assistant turn that carries its text is the last API message. Orphaned text, partial reasoning, and the #257 lock behavior are unchanged, and the restore still always writes partial: false. --- src/core/task/Task.ts | 12 +++++++++--- src/core/task/__tests__/Task.persistence.spec.ts | 11 +++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index b3eaa26de0..51c3b8e712 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -246,19 +246,25 @@ function queuedResponseForAsk(type: ClineAsk, text?: string): QueuedAskResolutio * turn coexisting. The bubble's text is then not orphaned and must not be * discarded. * + * The turn must be the LAST API message, not merely the last assistant one. An + * abort before the append throws at the post-stream guard, so API history keeps + * ending on that request's user turn while the bubble stays partial; matching an + * older assistant turn there would finalize text the model does not actually + * have, purely because the model happened to repeat itself. + * * The comparison re-applies the streaming-time `` strip that * `presentAssistantMessage` applies to the bubble but not to the raw accumulator * the API turn is built from. */ function hasAssistantTextTurnFor(apiMessages: ApiMessage[], partialText: string): boolean { const expected = partialText.replace(/\s?/g, "").replace(/\s?<\/thinking>/g, "") - const lastAssistantTurn = findLast(apiMessages, (message) => message.role === "assistant") + const lastTurn = apiMessages.at(-1) - if (!lastAssistantTurn || !Array.isArray(lastAssistantTurn.content)) { + if (lastTurn?.role !== "assistant" || !Array.isArray(lastTurn.content)) { return false } - return lastAssistantTurn.content.some( + return lastTurn.content.some( (block) => block.type === "text" && block.text.replace(/\s?/g, "").replace(/\s?<\/thinking>/g, "") === expected, diff --git a/src/core/task/__tests__/Task.persistence.spec.ts b/src/core/task/__tests__/Task.persistence.spec.ts index b7336291f3..de2730b456 100644 --- a/src/core/task/__tests__/Task.persistence.spec.ts +++ b/src/core/task/__tests__/Task.persistence.spec.ts @@ -2550,6 +2550,17 @@ describe("Task persistence", () => { { role: "assistant" as const, content: [{ type: "text" as const, text: "A newer answer" }] }, ], }, + { + // The reachable shape when the abort lands before the assistant append: the + // post-stream abort guard throws first, so API history still ends on this + // request's user turn while the bubble stays partial. A matching older + // assistant turn (the model repeating itself) must not finalize it. + name: "a user turn follows the matching assistant turn", + apiHistory: [ + { role: "assistant" as const, content: [{ type: "text" as const, text: "Half a sentence" }] }, + { role: "user" as const, content: [{ type: "text" as const, text: "Original task" }] }, + ], + }, ])("strips the partial text bubble when $name", async ({ apiHistory }) => { // No matching turn means the text exists nowhere else, so the orphaned bubble is // dropped exactly as before this change. From 4bef5f8a68014bbcab2f58a2ec44d86edfbaa4c4 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Sat, 31 Oct 2026 08:34:03 +0900 Subject: [PATCH 5/7] test(webview): pin the restore's partial-flag half from its own output What: the "still streams if a restored text row is left partial" case now builds its input by running the restore and putting `partial` back, instead of hand-writing the same message the `it.each` arm already covers. Why: the hand-written input was byte-identical to the `text` arm of the streaming `it.each`, so the case asserted a duplicate of a shape already pinned and never exercised the restore path it is named for. Deriving the input from the restore output keeps the `partial: false` half pinned to the code that clears it. Impact: test-only. The case still fails when the restore leaves `partial` set, and now also fails when the restore drops the row instead of re-adding it. --- .../ChatView.resume-strip-not-streaming.spec.tsx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/webview-ui/src/components/chat/__tests__/ChatView.resume-strip-not-streaming.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.resume-strip-not-streaming.spec.tsx index 554fd669d7..fb5c67d643 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.resume-strip-not-streaming.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.resume-strip-not-streaming.spec.tsx @@ -305,7 +305,15 @@ describe("ChatView - resume strip leaves a non-streaming chat", () => { it("still streams if a restored text row is left partial", async () => { // Guards the `partial: false` half of the restore: keeping the row without // clearing the flag would reintroduce exactly the #257 lock the strip removes. - renderWithMessages([taskMessage, { type: "say", say: "text", ts: 2, text: "Half a sentence", partial: true }]) + // The input is the restore's own output with the flag put back, so this case + // stays pinned to the restore path instead of repeating the `text` arm above. + const partialTail: ClineMessage = { type: "say", say: "text", ts: 2, text: "Half a sentence", partial: true } + const restored = applyHostResumeStrip([taskMessage, partialTail], ["Half a sentence"]) + const leftPartial = restored.map((message) => + message.say === "text" ? { ...message, partial: true } : message, + ) + + renderWithMessages(leftPartial) await expectStreaming(true) }) From fd440d2d5ef2c278f04fccb36cbb1224ecb94735 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Sat, 31 Oct 2026 11:03:37 +0900 Subject: [PATCH 6/7] test(task): pin the restore against mixed turns and the closing thinking tag What: add two resumeTaskFromHistory cases for the partial-text restore, one where the matching assistant turn also carries a tool_use block and one where the matching text carries a closing tag with its whitespace. Why: the existing cases exercised only a text-only turn and only the opening tag, so the block-type filter and the closing-tag half of the strip were unpinned against regression. Impact: no production change; coverage only. Both cases were mutation-proven against the code they cover. --- .../task/__tests__/Task.persistence.spec.ts | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/src/core/task/__tests__/Task.persistence.spec.ts b/src/core/task/__tests__/Task.persistence.spec.ts index de2730b456..8d20115376 100644 --- a/src/core/task/__tests__/Task.persistence.spec.ts +++ b/src/core/task/__tests__/Task.persistence.spec.ts @@ -2520,6 +2520,60 @@ describe("Task persistence", () => { expect(mockSaveTaskMessages).not.toHaveBeenCalled() }) + it.each([ + { + // The turn that reaches API history right before the presenter release can + // also carry tool_use blocks; only the text block has to match. This shape + // additionally re-enters the synthetic tool_result path below, so the restore + // must survive it rather than depend on the turn being text-only. + name: "the matching assistant turn also carries a tool_use block", + assistantContent: [ + { type: "text" as const, text: "Half a sentence" }, + { type: "tool_use" as const, id: "toolu_1", name: "read_file", input: {} }, + ], + }, + { + // Pins the closing-tag half of the strip, which the `` case above + // leaves unexercised: the bubble drops the markup, the accumulator keeps it. + name: "the matching text carries a closing thinking tag and its whitespace", + assistantContent: [{ type: "text" as const, text: "Half a sentence\n" }], + }, + ])("restores the partial text bubble when $name", async ({ assistantContent }) => { + mockReadTaskMessages.mockResolvedValue([ + { ts: 1, type: "say", say: "text", text: "Original task" }, + { ts: 2, type: "say", say: "text", text: "Half a sentence", partial: true }, + ]) + mockReadApiMessages.mockResolvedValue([ + { role: "user", content: [{ type: "text", text: "Original task" }] }, + { role: "assistant", content: assistantContent }, + ]) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + historyItem: { + id: "issue-257-restore-shape", + number: 1, + ts: 1, + task: "Original task", + tokensIn: 10, + tokensOut: 5, + totalCost: 0.001, + }, + startTask: false, + }) + vi.spyOn(task, "ask").mockImplementation(async () => { + expect(task.clineMessages).toContainEqual( + expect.objectContaining({ say: "text", text: "Half a sentence", partial: false }), + ) + expect(task.clineMessages).not.toContainEqual(expect.objectContaining({ partial: true })) + throw new Error("stop after hydration") + }) + + await expect(getTaskPersistenceAccess(task).resumeTaskFromHistory()).rejects.toThrow("stop after hydration") + expect(mockSaveTaskMessages).not.toHaveBeenCalled() + }) + it.each([ { name: "the last assistant turn holds different text", From a30ee29ffd65d2a735415e95884fcb36a0646351 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Sat, 31 Oct 2026 12:04:43 +0900 Subject: [PATCH 7/7] refactor(task): share the thinking-tag strip between streaming and resume What: extract `stripThinkingTags` into `src/utils/text-normalization.ts` and call it from both `presentAssistantMessage` (streaming bubble write) and `hasAssistantTextTurnFor` (resume-time partial-bubble matching). Why: the two-regex `` strip was copy-pasted in three places. Resume matching compares the stripped bubble text against the raw API accumulator, so if either copy changed alone the two texts would stop comparing equal and restored bubbles would silently stop being restored. Sharing one helper makes that divergence a compile-time impossibility instead of a latent bug. Impact: no behavior change; the helper is byte-identical to both former copies. `text-normalization.spec.ts` pins each half of the strip separately, so a single-regex change fails rather than passing silently. --- .../presentAssistantMessage.ts | 6 +++--- src/core/task/Task.ts | 12 +++++------ .../__tests__/text-normalization.spec.ts | 20 ++++++++++++++++++- src/utils/text-normalization.ts | 16 +++++++++++++++ 4 files changed, 43 insertions(+), 11 deletions(-) diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 73d48b2e7c..ab365e9d00 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -45,6 +45,7 @@ import { codebaseSearchTool } from "../tools/CodebaseSearchTool" import { formatResponse } from "../prompts/responses" import { sanitizeToolUseId } from "../../utils/tool-id" +import { stripThinkingTags } from "../../utils/text-normalization" /** * Maps a raw, potentially model-controlled tool name to a safe analytics key. @@ -435,9 +436,8 @@ async function presentAssistantMessageBlock(cline: Task, policySnapshot: Request // Have to do this for partial and complete since sending // content in thinking tags to markdown renderer will // automatically be removed. - // Strip any streamed tags from text output. - content = content.replace(/\s?/g, "") - content = content.replace(/\s?<\/thinking>/g, "") + // Shared with resume-time matching in Task so the two cannot diverge. + content = stripThinkingTags(content) } await cline.say("text", content, undefined, block.partial) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 51c3b8e712..99c546520b 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -95,6 +95,7 @@ import { calculateApiCostAnthropic, calculateApiCostOpenAI } from "../../shared/ import { getWorkspacePath } from "../../utils/path" import { sanitizeToolUseId } from "../../utils/tool-id" import { getTaskDirectoryPath } from "../../utils/storage" +import { stripThinkingTags } from "../../utils/text-normalization" // prompts import { formatResponse } from "../prompts/responses" @@ -254,21 +255,18 @@ function queuedResponseForAsk(type: ClineAsk, text?: string): QueuedAskResolutio * * The comparison re-applies the streaming-time `` strip that * `presentAssistantMessage` applies to the bubble but not to the raw accumulator - * the API turn is built from. + * the API turn is built from. Both sites call the same `stripThinkingTags`, so the + * two cannot drift apart. */ function hasAssistantTextTurnFor(apiMessages: ApiMessage[], partialText: string): boolean { - const expected = partialText.replace(/\s?/g, "").replace(/\s?<\/thinking>/g, "") + const expected = stripThinkingTags(partialText) const lastTurn = apiMessages.at(-1) if (lastTurn?.role !== "assistant" || !Array.isArray(lastTurn.content)) { return false } - return lastTurn.content.some( - (block) => - block.type === "text" && - block.text.replace(/\s?/g, "").replace(/\s?<\/thinking>/g, "") === expected, - ) + return lastTurn.content.some((block) => block.type === "text" && stripThinkingTags(block.text) === expected) } const FORCED_CONTEXT_REDUCTION_PERCENT = 75 // Keep 75% of context (remove 25%) on context window errors diff --git a/src/utils/__tests__/text-normalization.spec.ts b/src/utils/__tests__/text-normalization.spec.ts index e672617d18..848bb0c600 100644 --- a/src/utils/__tests__/text-normalization.spec.ts +++ b/src/utils/__tests__/text-normalization.spec.ts @@ -1,4 +1,4 @@ -import { normalizeString, unescapeHtmlEntities } from "../text-normalization" +import { normalizeString, stripThinkingTags, unescapeHtmlEntities } from "../text-normalization" describe("Text normalization utilities", () => { describe("normalizeString", () => { @@ -101,4 +101,22 @@ describe("Text normalization utilities", () => { expect(unescapeHtmlEntities(input)).toBe(expected) }) }) + + describe("stripThinkingTags", () => { + // Streaming writes the bubble through this helper and resume matching compares + // through it, so each half of the strip is pinned separately: a change to one + // regex alone must fail here rather than silently stop restoring bubbles. + it.each([ + { name: "an opening tag", input: "\nHalf a sentence", expected: "Half a sentence" }, + { name: "a closing tag", input: "Half a sentence\n", expected: "Half a sentence" }, + { name: "both tags", input: "\nHalf a sentence\n", expected: "Half a sentence" }, + { name: "repeated tags", input: "\ntext", expected: "text" }, + ])("strips $name", ({ input, expected }) => { + expect(stripThinkingTags(input)).toBe(expected) + }) + + it("leaves text without tags untouched", () => { + expect(stripThinkingTags("Half a sentence")).toBe("Half a sentence") + }) + }) }) diff --git a/src/utils/text-normalization.ts b/src/utils/text-normalization.ts index 9e25d140c4..ec5cffa745 100644 --- a/src/utils/text-normalization.ts +++ b/src/utils/text-normalization.ts @@ -76,6 +76,22 @@ export function normalizeString(str: string, options: NormalizeOptions = DEFAULT return normalized } +/** + * Strips streamed `` tags from model text. + * + * The streaming presenter strips these before writing a chat bubble, but the API + * turn is built from the raw accumulator, so the two texts differ by exactly this + * markup. Resume-time matching has to re-apply the same strip to compare them, and + * it must stay byte-identical to the streaming one; a divergence would silently + * stop matching bubbles from being restored. + * + * @param text The text to strip + * @returns The text with `` and `` tags and their adjacent whitespace removed + */ +export function stripThinkingTags(text: string): string { + return text.replace(/\s?/g, "").replace(/\s?<\/thinking>/g, "") +} + /** * Unescapes common HTML entities in a string *