Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -551,12 +551,9 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
this.emit(RooCodeEventName.QueuedMessagesUpdated, this.taskId, this.messageQueueService.messages)
void this.providerRef
.deref()
?.postStateToWebviewWithoutTaskHistory()
?.postStateToWebviewThrottled()
.catch((error) => {
console.error(
"[Task#messageQueueStateChangedHandler] postStateToWebviewWithoutTaskHistory failed:",
error,
)
console.error("[Task#messageQueueStateChangedHandler] postStateToWebviewThrottled failed:", error)
})
}

Expand Down Expand Up @@ -1047,9 +1044,13 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
private async addToClineMessages(message: ClineMessage) {
this.clineMessages.push(message)
const provider = this.providerRef.deref()
// Avoid resending large, mostly-static fields (notably taskHistory) on every chat message update.
// taskHistory is maintained in-memory in the webview and updated via taskHistoryItemUpdated.
await provider?.postStateToWebviewWithoutTaskHistory()
// Unanswered asks must reach the webview before Message listeners can respond against its state.
const requiresImmediateState =
message.partial === true || (message.type === "ask" && message.isAnswered !== true)
await provider?.postStateToWebviewThrottled()
if (requiresImmediateState) {
await provider?.flushPostStateToWebviewThrottled()
}
this.emit(RooCodeEventName.Message, { action: "created", message })
await this.saveClineMessages()

Expand Down Expand Up @@ -2250,6 +2251,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
// Force final token usage update before abort event
this.emitFinalTokenUsageUpdate()

await this.providerRef.deref()?.flushPostStateToWebviewThrottled()

this.emit(RooCodeEventName.TaskAborted)

try {
Expand Down
225 changes: 220 additions & 5 deletions src/core/task/__tests__/Task.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { Anthropic } from "@anthropic-ai/sdk"

import {
providerIdentifiers,
RooCodeEventName,
type GlobalState,
type ProviderSettings,
type ModelInfo,
Expand All @@ -27,9 +28,12 @@ import type { ApiMessage } from "../../task-persistence"

type TaskTestAccess = {
getSystemPrompt: () => Promise<string>
getEnabledMcpToolsCount: () => Promise<{ enabledToolCount: number; enabledServerCount: number }>
initiateTaskLoop: (userContent: Anthropic.Messages.ContentBlockParam[]) => Promise<void>
startTask: (task?: string, images?: string[]) => Promise<void>
resumeTaskFromHistory: () => Promise<void>
presentAssistantMessageSafe: () => void
addToClineMessages: (message: import("@roo-code/types").ClineMessage) => Promise<void>
updateClineMessage: (message: import("@roo-code/types").ClineMessage) => Promise<void>
saveClineMessages: () => Promise<boolean>
safeEnsureModelFetched: () => Promise<void>
Expand Down Expand Up @@ -338,6 +342,8 @@ describe("Cline", () => {
mockProvider.postMessageToWebview = vi.fn().mockResolvedValue(undefined)
mockProvider.postStateToWebview = vi.fn().mockResolvedValue(undefined)
mockProvider.postStateToWebviewWithoutTaskHistory = vi.fn().mockResolvedValue(undefined)
mockProvider.postStateToWebviewThrottled = vi.fn().mockResolvedValue(undefined)
mockProvider.flushPostStateToWebviewThrottled = vi.fn().mockResolvedValue(undefined)
mockProvider.getTaskWithId = vi.fn().mockImplementation(async (id) => ({
historyItem: {
id,
Expand Down Expand Up @@ -1029,6 +1035,8 @@ describe("Cline", () => {
say: vi.fn(),
postStateToWebview: vi.fn().mockResolvedValue(undefined),
postStateToWebviewWithoutTaskHistory: vi.fn().mockResolvedValue(undefined),
postStateToWebviewThrottled: vi.fn().mockResolvedValue(undefined),
flushPostStateToWebviewThrottled: vi.fn().mockResolvedValue(undefined),
postMessageToWebview: vi.fn().mockResolvedValue(undefined),
updateTaskHistory: vi.fn().mockResolvedValue(undefined),
}
Expand Down Expand Up @@ -1663,6 +1671,137 @@ describe("Cline", () => {
})
})

describe("webview state throttling", () => {
it("schedules a complete new message without forcing an immediate state push", async () => {
const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
task: "test task",
startTask: false,
})
vi.spyOn(getTaskTestAccess(task), "saveClineMessages").mockResolvedValue(true)
const message = {
ts: Date.now(),
type: "say" as const,
say: "text" as const,
text: "message",
}

await getTaskTestAccess(task).addToClineMessages(message)

expect(mockProvider.postStateToWebviewThrottled).toHaveBeenCalledOnce()
expect(mockProvider.postStateToWebviewThrottled).toHaveBeenCalledWith()
expect(mockProvider.flushPostStateToWebviewThrottled).not.toHaveBeenCalled()
expect(mockProvider.postStateToWebviewWithoutTaskHistory).not.toHaveBeenCalled()
})

it("waits for an unanswered ask flush before emitting the message", async () => {
const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
task: "test task",
startTask: false,
})
const taskAccess = getTaskTestAccess(task)
vi.spyOn(taskAccess, "saveClineMessages").mockResolvedValue(true)
let releaseFlush!: () => void
const pendingFlush = new Promise<void>((resolve) => {
releaseFlush = resolve
})
const flushSpy = vi.mocked(mockProvider.flushPostStateToWebviewThrottled).mockReturnValueOnce(pendingFlush)
const messageListener = vi.fn()
task.on(RooCodeEventName.Message, messageListener)
const message = {
ts: 1,
type: "ask" as const,
ask: "resume_task" as const,
}

const addPromise = taskAccess.addToClineMessages(message)

await Promise.resolve()
expect(mockProvider.postStateToWebviewThrottled).toHaveBeenCalledOnce()
expect(mockProvider.postStateToWebviewThrottled).toHaveBeenCalledWith()
expect(flushSpy).toHaveBeenCalledOnce()
expect(flushSpy).toHaveBeenCalledWith()
expect(messageListener).not.toHaveBeenCalled()

releaseFlush()
await addPromise

expect(flushSpy.mock.invocationCallOrder[0]).toBeLessThan(messageListener.mock.invocationCallOrder[0])
expect(messageListener).toHaveBeenCalledWith({ action: "created", message })
})
Comment on lines +1698 to +1734

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate files =="
git ls-files | rg '(^|/)Task\.ts$|(^|/)Task\.spec\.ts$' || true

echo "== Task.spec relevant ranges =="
for f in $(git ls-files | rg 'Task\.spec\.ts$'); do
  echo "--- $f lines 1650-1900"
  sed -n '1650,1900p' "$f" | nl -ba -v1650
done

echo "== Task.ts outline/search around candidate symbols =="
for f in $(git ls-files | rg '^(src/core/task)/Task\.ts$|Task\.ts$'); do
  echo "--- $f"
  wc -l "$f"
  ast-grep outline "$f" --match addToClineMessages --view expanded || true
  ast-grep outline "$f" --match abortTask --view expanded || true
  ast-grep outline "$f" --match messageQueueStateChangedHandler --view expanded || true
  echo "--- postState/flush occurrences"
  rg -n "postStateToWebviewThrottled|flushPostStateToWebviewThrottled|messageQueueStateChangedHandler|TaskAborted|dispose\(" "$f" -C 4
done

Repository: Zoo-Code-Org/Zoo-Code

Length of output: 367


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Task.spec relevant ranges =="
for f in $(git ls-files | rg 'Task\.spec\.ts$'); do
  echo "--- $f lines 1650-1900"
  sed -n '1650,1900p' "$f" | cat -n
done

echo "== Task.ts candidate occurrences =="
f="src/core/task/Task.ts"
echo "--- $f line count"
wc -l "$f"
echo "--- postState/flush/messageQueue occurrences"
grep -n "postStateToWebviewThrottled\|flushPostStateToWebviewThrottled\|messageQueueStateChangedHandler\|TaskAborted\|dispose(" "$f" -C 8 || true

echo "== addTask/messageQueue abort-like handlers in Task.ts =="
# Use Python to print concise named function blocks if possible without heavy runtime.
python3 - <<'PY'
from pathlib import Path
p=Path('src/core/task/Task.ts')
text=p.read_text()
for needle in ['addToClineMessages', 'messageQueueStateChangedHandler', 'abortTask']:
    print(f'---- {needle} ----')
    i=text.find(needle)
    while i!=-1:
        start=max(0,i-800)
        end=min(len(text),i+2200)
        print(text[start:end])
        print('----')
        i=text.find(needle,end)
PY

Repository: Zoo-Code-Org/Zoo-Code

Length of output: 46233


Add rejection-path coverage for the throttled provider calls.

addToClineMessages awaits postStateToWebviewThrottled() before emitting RooCodeEventName.Message. If flushPostStateToWebviewThrottled() rejects, execution stops before this.emit(RooCodeEventName.Message, ...) and saveClineMessages(). abortTask awaits flushPostStateToWebviewThrottled() before emitting RooCodeEventName.TaskAborted, and disposal only wraps dispose()/saveClineMessages(), so a rejection can prevent post-state flush completion and expose an unwrapped rejection before abort completion.

Add tests in this block, and a rejection-path test near the abort tests, that assert the logged or observable behavior and do not let these rejections propagate.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/task/__tests__/Task.spec.ts` around lines 1698 - 1734, Add
rejection-path tests for addToClineMessages and abortTask, using rejected
throttled provider calls to verify RooCodeEventName.Message and
RooCodeEventName.TaskAborted behavior remains observable as appropriate. Cover
disposal’s flush path as needed, asserting errors are logged or otherwise
handled and that the rejections do not escape or prevent completion of the
surrounding operation.


it("keeps an already answered ask on the throttled path", async () => {
const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
task: "test task",
startTask: false,
})
vi.spyOn(getTaskTestAccess(task), "saveClineMessages").mockResolvedValue(true)

await getTaskTestAccess(task).addToClineMessages({
ts: 1,
type: "ask",
ask: "tool",
isAnswered: true,
})

expect(mockProvider.postStateToWebviewThrottled).toHaveBeenCalledOnce()
expect(mockProvider.postStateToWebviewThrottled).toHaveBeenCalledWith()
expect(mockProvider.flushPostStateToWebviewThrottled).not.toHaveBeenCalled()
})

it("waits for a new partial message flush before a following message update", async () => {
const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
task: "test task",
startTask: false,
})
const taskAccess = getTaskTestAccess(task)
vi.spyOn(taskAccess, "saveClineMessages").mockResolvedValue(true)
let releaseFlush!: () => void
const pendingFlush = new Promise<void>((resolve) => {
releaseFlush = resolve
})
const flushSpy = vi.mocked(mockProvider.flushPostStateToWebviewThrottled).mockReturnValueOnce(pendingFlush)
const updatePostSpy = vi.mocked(mockProvider.postMessageToWebview)
const partialMessage = {
ts: 1,
type: "say" as const,
say: "text" as const,
text: "partial message",
partial: true,
}
let partialAddSettled = false
const addThenUpdate = taskAccess.addToClineMessages(partialMessage).then(async () => {
partialAddSettled = true
await taskAccess.updateClineMessage({ ...partialMessage, text: "updated partial" })
})

await Promise.resolve()
expect(mockProvider.postStateToWebviewThrottled).toHaveBeenCalledWith()
expect(flushSpy).toHaveBeenCalledWith()
expect(partialAddSettled).toBe(false)
expect(updatePostSpy).not.toHaveBeenCalled()

releaseFlush()
await addThenUpdate

expect(flushSpy.mock.invocationCallOrder[0]).toBeLessThan(updatePostSpy.mock.invocationCallOrder[0])
expect(updatePostSpy).toHaveBeenCalledWith({
type: "messageUpdated",
clineMessage: {
...partialMessage,
text: "updated partial",
},
})
})
})

describe("abortTask", () => {
it("should set abort flag and emit TaskAborted event", async () => {
const task = new Task({
Expand Down Expand Up @@ -1707,6 +1846,37 @@ describe("Cline", () => {
expect(disposeSpy).toHaveBeenCalled()
})

it("flushes pending state before TaskAborted and disposal while queue state is intact", async () => {
const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
task: "test task",
startTask: false,
})
let queuedMessagesAtFlush = -1
const flushSpy = vi.mocked(mockProvider.flushPostStateToWebviewThrottled).mockImplementation(async () => {
queuedMessagesAtFlush = task.messageQueueService.messages.length
})
const emitSpy = vi.spyOn(task, "emit")
const disposeSpy = vi.spyOn(task, "dispose").mockImplementation(() => {})

task.messageQueueService.addMessage("queued text")
await task.abortTask()

const taskAbortedCallIndex = (emitSpy.mock.calls as unknown[][]).findIndex(
([event]) => event === "taskAborted",
)
expect(taskAbortedCallIndex).toBeGreaterThanOrEqual(0)
expect(queuedMessagesAtFlush).toBe(1)
expect(flushSpy).toHaveBeenCalledWith()
expect(flushSpy.mock.invocationCallOrder[0]).toBeLessThan(
emitSpy.mock.invocationCallOrder[taskAbortedCallIndex],
)
expect(emitSpy.mock.invocationCallOrder[taskAbortedCallIndex]).toBeLessThan(
disposeSpy.mock.invocationCallOrder[0],
)
})

it("should work with TaskLike interface", async () => {
const task = new Task({
provider: mockProvider,
Expand Down Expand Up @@ -2741,6 +2911,50 @@ describe("Cline", () => {
})
})

describe("startTask", () => {
it("posts a clean state immediately before adding the first task message", async () => {
const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
task: "new task",
startTask: false,
})
const taskAccess = getTaskTestAccess(task)

task.clineMessages = [{ ts: 1, type: "say", say: "text", text: "stale message" }]

let resolvePostState: (() => void) | undefined
const pendingPostState = new Promise<void>((resolve) => {
resolvePostState = resolve
})
const postStateSpy = vi
.mocked(mockProvider.postStateToWebviewWithoutTaskHistory)
.mockImplementationOnce(async () => {
expect(task.clineMessages).toEqual([])
await pendingPostState
})
const saySpy = vi.spyOn(task, "say").mockResolvedValue(undefined)
vi.spyOn(taskAccess, "getEnabledMcpToolsCount").mockResolvedValue({
enabledToolCount: 0,
enabledServerCount: 0,
})
const initiateTaskLoopSpy = vi.spyOn(taskAccess, "initiateTaskLoop").mockResolvedValue(undefined)

const startPromise = taskAccess.startTask("new task")

expect(postStateSpy).toHaveBeenCalledTimes(1)
expect(mockProvider.postStateToWebviewThrottled).not.toHaveBeenCalled()
expect(saySpy).not.toHaveBeenCalled()

resolvePostState?.()
await startPromise

expect(saySpy).toHaveBeenCalledOnce()
expect(saySpy).toHaveBeenCalledWith("text", "new task", undefined)
expect(initiateTaskLoopSpy).toHaveBeenCalledOnce()
})
})

describe("start()", () => {
it("should be a no-op if the task was already started in the constructor", () => {
const task = new Task({
Expand Down Expand Up @@ -2859,9 +3073,9 @@ describe("Cline", () => {
resumeSpy.mockRestore()
})

it("logs (instead of crashing) when postStateToWebviewWithoutTaskHistory rejects from the queue handler", async () => {
it("logs (instead of crashing) when postStateToWebviewThrottled rejects from the queue handler", async () => {
const boom = new Error("postState boom")
mockProvider.postStateToWebviewWithoutTaskHistory = vi.fn().mockRejectedValue(boom)
mockProvider.postStateToWebviewThrottled = vi.fn().mockRejectedValue(boom)

const task = new Task({
provider: mockProvider,
Expand All @@ -2870,13 +3084,14 @@ describe("Cline", () => {
startTask: false,
})

// Triggers messageQueueStateChangedHandler -> void postStateToWebviewWithoutTaskHistory()
// Triggers messageQueueStateChangedHandler -> void postStateToWebviewThrottled()
task.messageQueueService.addMessage("queued text")
await flushMicrotasks()

expect(mockProvider.postStateToWebviewWithoutTaskHistory).toHaveBeenCalled()
expect(mockProvider.postStateToWebviewThrottled).toHaveBeenCalledWith()
expect(mockProvider.postStateToWebviewWithoutTaskHistory).not.toHaveBeenCalled()
expect(consoleErrorSpy).toHaveBeenCalledWith(
"[Task#messageQueueStateChangedHandler] postStateToWebviewWithoutTaskHistory failed:",
"[Task#messageQueueStateChangedHandler] postStateToWebviewThrottled failed:",
boom,
)
})
Expand Down
2 changes: 2 additions & 0 deletions src/core/task/__tests__/Task.throttle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ describe("Task token usage throttling", () => {
log: vi.fn(),
postStateToWebview: vi.fn().mockResolvedValue(undefined),
postStateToWebviewWithoutTaskHistory: vi.fn().mockResolvedValue(undefined),
postStateToWebviewThrottled: vi.fn().mockResolvedValue(undefined),
flushPostStateToWebviewThrottled: vi.fn().mockResolvedValue(undefined),
updateTaskHistory: vi.fn().mockResolvedValue(undefined),
}

Expand Down
Loading
Loading