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..98bbe193b2 --- /dev/null +++ b/apps/vscode-e2e/src/suite/resume-eviction-race.test.ts @@ -0,0 +1,97 @@ +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.ok( + afterEviction.task?.includes("RESUME_EVICTION_RACE_SMOKE"), + `Title must not be clobbered by eviction mid-resume.\n` + + ` Expected to contain: "RESUME_EVICTION_RACE_SMOKE"\n` + + ` Got: "${afterEviction.task}"`, + ) + }) +}) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index f55078b6ff..31047c4515 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. try { - // 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 + } 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.resume-eviction-race.spec.ts b/src/core/task/__tests__/Task.resume-eviction-race.spec.ts new file mode 100644 index 0000000000..fba2a84d9c --- /dev/null +++ b/src/core/task/__tests__/Task.resume-eviction-race.spec.ts @@ -0,0 +1,207 @@ +// 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 } +} + +function makeMockProvider(updateTaskHistory: ReturnType): ClineProvider { + return { + log: vi.fn(), + taskHistoryStore: { get: () => undefined }, + 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" } }, + }, + } as unknown as ClineProvider +} + +// ─── 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) + + const updateTaskHistory = vi.fn().mockResolvedValue([]) + const mockProvider = makeMockProvider(updateTaskHistory) + + const task = new Task({ + provider: mockProvider, + 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. No "no_messages" write must reach + // the history store. + expect(updateTaskHistory).not.toHaveBeenCalledWith( + expect.objectContaining({ task: expect.stringContaining("no_messages") }), + ) + + // 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 + }) +})