diff --git a/apps/vscode-e2e/fixtures/resume-eviction-race.json b/apps/vscode-e2e/fixtures/resume-eviction-race.json new file mode 100644 index 0000000000..18851a44c9 --- /dev/null +++ b/apps/vscode-e2e/fixtures/resume-eviction-race.json @@ -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" + } + ] + } + } + ] +} diff --git a/apps/vscode-e2e/src/suite/resume-eviction-race.test.ts b/apps/vscode-e2e/src/suite/resume-eviction-race.test.ts new file mode 100644 index 0000000000..53f33cb4e5 --- /dev/null +++ b/apps/vscode-e2e/src/suite/resume-eviction-race.test.ts @@ -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}"`, + ) + }) +}) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index f55078b6ff..b728e43b9a 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -2258,9 +2258,16 @@ export class Task extends EventEmitter 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) diff --git a/src/core/task/__tests__/Task.persistence.spec.ts b/src/core/task/__tests__/Task.persistence.spec.ts index 1761db5bc3..60510a71d1 100644 --- a/src/core/task/__tests__/Task.persistence.spec.ts +++ b/src/core/task/__tests__/Task.persistence.spec.ts @@ -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 + saveClineMessages: () => Promise +} + +function getTaskPersistenceAccess(task: Task): TaskPersistenceAccess { + return task as unknown as TaskPersistenceAccess +} + +function createDeferred() { + let resolve!: (value: T) => void + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise + }) + return { promise, resolve } +} + // ─── Hoisted mocks ─────────────────────────────────────────────────────────── const { @@ -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() + 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() + 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", () => { diff --git a/src/core/task/__tests__/Task.resume-eviction-race.spec.ts b/src/core/task/__tests__/Task.resume-eviction-race.spec.ts new file mode 100644 index 0000000000..31e532e0e9 --- /dev/null +++ b/src/core/task/__tests__/Task.resume-eviction-race.spec.ts @@ -0,0 +1,225 @@ +// cd src && npx vitest run core/task/__tests__/Task.resume-eviction-race.spec.ts +// +// Regression anchor for the "Work #1 (no message)" title-clobber bug +// (Zoo Code v3.76.0, Discord report 2026-08-06). +// +// Root cause: resumeTaskFromHistory() starts with an async disk read. Until +// that read resolves, clineMessages is []. evictCurrentTask() calls +// abortTask(), which called saveClineMessages() -> taskMetadata(). With an +// empty array, taskMetadata() writes the "no_messages" placeholder as the +// title, permanently clobbering the real one in the history store. +// +// Fix: abortTask() skips saveClineMessages() for history tasks whose message +// load has not completed. The on-disk data is already correct at that point. +import * as os from "os" +import * as path from "path" + +import type { ClineMessage, GlobalState, HistoryItem, ProviderSettings } from "@roo-code/types" +import { TelemetryService } from "@roo-code/telemetry" + +import { Task } from "../Task" +import { ClineProvider } from "../../webview/ClineProvider" + +// ─── Hoisted mocks ─────────────────────────────────────────────────────────── + +const { mockSaveApiMessages, mockSaveTaskMessages, mockReadApiMessages, mockReadTaskMessages, mockPWaitFor } = + vi.hoisted(() => ({ + mockSaveApiMessages: vi.fn().mockResolvedValue(undefined), + mockSaveTaskMessages: vi.fn().mockResolvedValue(undefined), + mockReadApiMessages: vi.fn().mockResolvedValue([]), + // Controlled per-test via a deferred promise so we can hold the "disk + // read" open while a rival navigation aborts the still-loading task. + mockReadTaskMessages: vi.fn<() => Promise>(), + mockPWaitFor: vi.fn().mockResolvedValue(undefined), + })) + +// ─── Module mocks ──────────────────────────────────────────────────────────── +// vscode and fs/promises are globally aliased in vitest.config — no inline +// mock needed. + +vi.mock("delay", () => ({ __esModule: true, default: vi.fn().mockResolvedValue(undefined) })) +vi.mock("execa", () => ({ execa: vi.fn() })) +vi.mock("p-wait-for", () => ({ default: mockPWaitFor })) + +// taskMetadata is NOT mocked — the real implementation is under test. +vi.mock("../../task-persistence", async (importOriginal) => { + const mod = await importOriginal() + return { + ...mod, + saveApiMessages: mockSaveApiMessages, + saveTaskMessages: mockSaveTaskMessages, + readApiMessages: mockReadApiMessages, + readTaskMessages: mockReadTaskMessages, + TaskHistoryStore: vi.fn().mockImplementation(function () { + return { + initialize: vi.fn().mockResolvedValue(undefined), + dispose: vi.fn(), + get: vi.fn(), + getAll: vi.fn().mockReturnValue([]), + upsert: vi.fn().mockResolvedValue([]), + delete: vi.fn().mockResolvedValue(undefined), + deleteMany: vi.fn().mockResolvedValue(undefined), + reconcile: vi.fn().mockResolvedValue(undefined), + initialized: Promise.resolve(), + } + }), + } +}) + +vi.mock("../../mentions", () => ({ + parseMentions: vi + .fn() + .mockImplementation((text) => + Promise.resolve({ text: `processed: ${text}`, mode: undefined, contentBlocks: [] }), + ), + openMention: vi.fn(), + getLatestTerminalOutput: vi.fn(), +})) +vi.mock("../../../integrations/misc/extract-text", () => ({ + extractTextFromFile: vi.fn().mockResolvedValue("Mock file content"), +})) +vi.mock("../../environment/getEnvironmentDetails", () => ({ + getEnvironmentDetails: vi.fn().mockResolvedValue(""), +})) +vi.mock("../../ignore/RooIgnoreController") +vi.mock("../../../utils/storage", () => ({ + getTaskDirectoryPath: vi + .fn() + .mockImplementation((globalStoragePath, taskId) => Promise.resolve(`${globalStoragePath}/tasks/${taskId}`)), + getSettingsDirectoryPath: vi + .fn() + .mockImplementation((globalStoragePath) => Promise.resolve(`${globalStoragePath}/settings`)), +})) +vi.mock("../../../utils/fs", () => ({ fileExistsAtPath: vi.fn().mockReturnValue(false) })) + +// ─── Helpers ───────────────────────────────────────────────────────────────── + +function createDeferred() { + let resolve!: (value: T) => void + const promise = new Promise((res) => { + resolve = res + }) + return { promise, resolve } +} + +/** + * Minimal slice of ClineProvider that Task reads during construction and abort. + * All types are derived from ClineProvider so TypeScript validates property + * names and signatures without requiring the full class to be satisfied. + */ +type MockProvider = Pick & { + taskHistoryStore: Pick + context: { + globalStorageUri: Pick + globalState: Pick + workspaceState: Pick + secrets: Pick + extensionUri: Pick + extension: Pick + } +} + +function makeMockProvider(updateTaskHistory: ReturnType): MockProvider { + return { + log: vi.fn(), + taskHistoryStore: { get: () => undefined }, + // vi.fn() is not directly assignable to the typed method signature. + updateTaskHistory: updateTaskHistory as unknown as ClineProvider["updateTaskHistory"], + context: { + globalStorageUri: { fsPath: path.join(os.tmpdir(), "test-storage") }, + globalState: { + get: vi.fn().mockImplementation((_key: keyof GlobalState) => undefined), + update: vi.fn().mockResolvedValue(undefined), + keys: vi.fn().mockReturnValue([]), + }, + workspaceState: { + get: vi.fn().mockImplementation(() => undefined), + update: vi.fn().mockResolvedValue(undefined), + keys: vi.fn().mockReturnValue([]), + }, + secrets: { + get: vi.fn().mockResolvedValue(undefined), + store: vi.fn().mockResolvedValue(undefined), + delete: vi.fn().mockResolvedValue(undefined), + }, + extensionUri: { fsPath: "/mock/extension/path" }, + extension: { packageJSON: { version: "1.0.0" } }, + }, + } +} + +// ─── Tests ─────────────────────────────────────────────────────────────────── + +describe("Task resume/eviction race (Work #1 (no message) regression)", () => { + let mockApiConfig: ProviderSettings + + beforeEach(() => { + vi.clearAllMocks() + + if (!TelemetryService.hasInstance()) { + TelemetryService.createInstance([]) + } + + mockApiConfig = { + apiProvider: "anthropic", + apiModelId: "claude-3-5-sonnet-20241022", + apiKey: "test-api-key", + } + }) + + it("does not clobber the real task title when evicted mid-resume", async () => { + const REAL_TITLE = "Write a short paragraph about the benefits of regular code reviews" + + const historyItem: HistoryItem = { + id: "parent-task-1", + number: 1, + task: REAL_TITLE, + ts: Date.now() - 60_000, + tokensIn: 500, + tokensOut: 300, + totalCost: 0.01, + workspace: path.join(os.tmpdir(), "mock-workspace"), + } + + // Hold the disk read open so the task is aborted while clineMessages is + // still empty — the same window a user hits by navigating away quickly. + const readDeferred = createDeferred() + mockReadTaskMessages + .mockReturnValueOnce(readDeferred.promise) // first read: held open to simulate the race window + .mockResolvedValue([]) // second read (resumeTaskFromHistory:2023): post-abort, safe fallback + + const updateTaskHistory = vi.fn().mockResolvedValue([]) + const mockProvider = makeMockProvider(updateTaskHistory) + + const task = new Task({ + provider: mockProvider as unknown as ClineProvider, + apiConfiguration: mockApiConfig, + historyItem, + taskNumber: historyItem.number, + startTask: false, + }) + + // Fire task.run() without awaiting — mirrors the fire-and-forget pattern + // in ClineProvider#createTaskWithHistoryItem. For history tasks, run() + // calls resumeTaskFromHistory(), which starts with an async disk read. + const runPromise = task.run().catch(() => { + // After abort, downstream steps (e.g. ask()) throw — expected. + }) + + // Abort while the disk read is still in flight, as evictCurrentTask() + // does when the user navigates away before messages load. + await task.abortTask(true) + + // The fix: saveClineMessages() must not be called for a history task + // with clineMessages still empty. Verify the call was skipped entirely, + // not just that the specific "no_messages" key was not written. + expect(updateTaskHistory).not.toHaveBeenCalled() + + // Let the read resolve so the promise does not leak into the next test. + readDeferred.resolve([ + { ts: historyItem.ts, type: "say", say: "text", text: REAL_TITLE }, + { ts: historyItem.ts + 1, type: "say", say: "completion_result", text: "Done." }, + ]) + await runPromise + }) +})