Skip to content
Merged
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
18 changes: 18 additions & 0 deletions apps/vscode-e2e/fixtures/resume-eviction-race.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"fixtures": [
{
"match": {
"userMessage": "RESUME_EVICTION_RACE_SMOKE"
},
"response": {
"toolCalls": [
{
"name": "attempt_completion",
"arguments": "{\"result\":\"Resume eviction smoke completed.\"}",
"id": "call_resume_eviction_001"
}
]
}
}
]
}
96 changes: 96 additions & 0 deletions apps/vscode-e2e/src/suite/resume-eviction-race.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import * as assert from "assert"

import { setDefaultSuiteTimeout } from "./test-utils"
import { waitUntilCompleted, waitFor } from "./utils"

// Regression test for the "Work #1 (no message)" title-clobber bug reported
// against Zoo Code v3.76.0 (Discord, 2026-08-06).
//
// Root cause: Task#resumeTaskFromHistory() is started fire-and-forget by
// scheduleTask() after createTaskWithHistoryItem() adds the task to the
// registry, so `clineMessages` is [] until the first disk read resolves.
// ClineProvider#evictCurrentTask() (called by clearCurrentTask / the
// Back-to-parent / Go-to-subtask buttons) calls abortTask(), which calls
// saveClineMessages() → taskMetadata() while the array is still empty.
// taskMetadata() then persists the "no_messages" placeholder title,
// permanently clobbering the real title in the history store.
//
// The test exercises the race by:
// 1. Running a task to completion so a real title is persisted.
// 2. Starting resumeTask() (same path as showTaskWithId) without awaiting it.
// 3. Polling until the task appears on the stack, then immediately evicting —
// the task is on the stack but its message load is still in flight.
// 4. Asserting the stored title still matches the original.
//
// NOTE: Because the extension host reads task messages from disk in the same
// process as this test, the I/O window is very tight (< 1ms on local disk).
// The race is not reliably triggerable from the e2e layer; the canonical
// regression anchor is the unit test in
// src/core/task/__tests__/Task.resume-eviction-race.spec.ts, which controls
// the timing via a deferred promise. This e2e test serves as a smoke test that
// the resume-then-evict flow does not blow up and that the stored title is
// correct after a round-trip.
suite("Resume eviction race (title clobber regression)", function () {
setDefaultSuiteTimeout(this)

test("evicting a mid-resume task does not overwrite its stored title", async () => {
const api = globalThis.api

const ORIGINAL_TITLE =
"RESUME_EVICTION_RACE_SMOKE: complete immediately with 'Resume eviction smoke completed.'"

// Step 1 — run a task to completion so a real title is persisted.
const taskId = await waitUntilCompleted({
api,
start: () =>
api.startNewTask({
configuration: {
mode: "ask",
autoApprovalEnabled: true,
enableCheckpoints: false,
},
text: ORIGINAL_TITLE,
}),
})

const beforeResume = await api.getTaskHistoryItem(taskId)
assert.ok(beforeResume, "Task should be in history after completion")
assert.ok(
beforeResume.task?.includes("RESUME_EVICTION_RACE_SMOKE"),
`Persisted title before resume should contain the prompt marker (got "${beforeResume.task}")`,
)

// Drain the stack so we start clean.
while (api.getCurrentTaskStack().length > 0) {
await api.clearCurrentTask()
}

// Step 2 — fire resumeTask() without awaiting it. resumeTask() calls
// createTaskWithHistoryItem() which adds the task to the registry and
// calls scheduleTask() (fire-and-forget). The task's run() and
// resumeTaskFromHistory() start in the background.
const resumePromise = api.resumeTask(taskId)

// Step 3 — wait only until the task appears on the stack (i.e.
// createTaskWithHistoryItem has returned and addClineToStack has run),
// then immediately evict. This minimises the gap between the eviction
// and the in-flight message load, giving the best chance of hitting the
// race window before readTaskMessages() resolves.
await waitFor(() => api.getCurrentTaskStack().includes(taskId))
await api.clearCurrentTask()

// Let the resume settle.
await resumePromise.catch(() => {})

// Step 4 — the stored title must still be the real one.
const afterEviction = await api.getTaskHistoryItem(taskId)
assert.ok(afterEviction, "Task should still be in history after eviction")

// Before the fix this would be "Task #N (No messages)" / "工作 #N (無訊息)".
assert.strictEqual(
afterEviction.task,
beforeResume.task,
`Title must not change during resume eviction. Got: "${afterEviction.task}"`,
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
})
})
11 changes: 9 additions & 2 deletions src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2258,9 +2258,16 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
console.error(`Error during task ${this.taskId}.${this.instanceId} disposal:`, error)
// Don't rethrow - we want abort to always succeed
}
// Save the countdown message in the automatic retry or other content.
// Guard: a history task whose message load has not finished yet has
// clineMessages = []. Saving now would call taskMetadata() with an
// empty array, which writes the "no messages" placeholder as the
// title and permanently clobbers the real title in the history store
// (the "Work #1 (no message)" / "工作 #1 (無訊息)" bug, v3.76.0).
// The on-disk data is still correct at this point, so skip the save.
if (this._isHistoryTask && this.clineMessages.length === 0) {
return
}
try {
// Save the countdown message in the automatic retry or other content.
await this.saveClineMessages()
} catch (error) {
console.error(`Error saving messages during abort for task ${this.taskId}.${this.instanceId}:`, error)
Expand Down
115 changes: 114 additions & 1 deletion src/core/task/__tests__/Task.persistence.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,30 @@ import * as os from "os"
import * as path from "path"
import * as vscode from "vscode"

import type { GlobalState, ProviderSettings } from "@roo-code/types"
import type { ClineMessage, GlobalState, ProviderSettings } from "@roo-code/types"
import { TelemetryService } from "@roo-code/telemetry"

import { Task } from "../Task"
import { ClineProvider } from "../../webview/ClineProvider"
import { ContextProxy } from "../../config/ContextProxy"

type TaskPersistenceAccess = {
resumeTaskFromHistory: () => Promise<void>
saveClineMessages: () => Promise<boolean>
}

function getTaskPersistenceAccess(task: Task): TaskPersistenceAccess {
return task as unknown as TaskPersistenceAccess
Comment thread
edelauna marked this conversation as resolved.
}

function createDeferred<T>() {
let resolve!: (value: T) => void
const promise = new Promise<T>((resolvePromise) => {
resolve = resolvePromise
})
return { promise, resolve }
}

// ─── Hoisted mocks ───────────────────────────────────────────────────────────

const {
Expand Down Expand Up @@ -470,6 +487,102 @@ describe("Task persistence", () => {
})
})

// ── abortTask history hydration guard ─────────────────────────────────

describe("abortTask", () => {
it("skips persistence when a history task aborts before messages load", async () => {
const messagesDeferred = createDeferred<ClineMessage[]>()
mockReadTaskMessages.mockReturnValueOnce(messagesDeferred.promise)

const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
historyItem: {
id: "history-task",
number: 1,
ts: Date.now(),
task: "Original task title",
tokensIn: 10,
tokensOut: 5,
totalCost: 0.001,
},
startTask: false,
})

const resumePromise = task.run().catch(() => {})

await task.abortTask()

expect(mockSaveTaskMessages).not.toHaveBeenCalled()
expect(mockProvider.updateTaskHistory).not.toHaveBeenCalled()

messagesDeferred.resolve([])
await resumePromise
})

it("persists a history task when messages load before abort", async () => {
const messages = [
{
ts: Date.now(),
type: "say" as const,
say: "text" as const,
text: "Loaded task message",
},
] satisfies ClineMessage[]
const messagesDeferred = createDeferred<typeof messages>()
mockReadTaskMessages.mockReturnValueOnce(messagesDeferred.promise).mockResolvedValue(messages)

const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
historyItem: {
id: "history-task",
number: 1,
ts: Date.now(),
task: "Original task title",
tokensIn: 10,
tokensOut: 5,
totalCost: 0.001,
},
startTask: false,
})
vi.spyOn(task, "ask").mockResolvedValue({ response: "noButtonClicked" })

mockReadApiMessages.mockResolvedValue([
{
role: "user",
content: [{ type: "text", text: "Original task" }],
},
])

const resumePromise = getTaskPersistenceAccess(task).resumeTaskFromHistory()
messagesDeferred.resolve(messages)
await resumePromise

const saveCallsBeforeAbort = mockSaveTaskMessages.mock.calls.length
expect(saveCallsBeforeAbort).toBeGreaterThan(0)
expect(mockProvider.updateTaskHistory).toHaveBeenCalled()

await task.abortTask()
expect(mockSaveTaskMessages.mock.calls.length).toBeGreaterThan(saveCallsBeforeAbort)
})

it("persists an empty non-history task when aborted", async () => {
const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
task: "New task",
startTask: false,
})
const saveClineMessagesSpy = vi.spyOn(getTaskPersistenceAccess(task), "saveClineMessages")

await task.abortTask()

expect(saveClineMessagesSpy).toHaveBeenCalledTimes(1)
expect(mockSaveTaskMessages).toHaveBeenCalledTimes(1)
})
})

// ── flushPendingToolResultsToHistory — save failure/success ───────────

describe("flushPendingToolResultsToHistory persistence", () => {
Expand Down
Loading
Loading