From 1e4e3c20f0aa6cb6e3aab2252bb45052ee5b30ca Mon Sep 17 00:00:00 2001 From: daewoongoh Date: Sun, 8 Nov 2026 21:57:26 +0900 Subject: [PATCH 1/8] feat: summarize initial prompt to generate task title in background --- src/core/task/Task.ts | 82 +++++++ .../__tests__/Task.titleGeneration.spec.ts | 218 ++++++++++++++++++ .../task/__tests__/taskTitleGenerator.spec.ts | 158 +++++++++++++ src/core/task/taskTitleGenerator.ts | 100 ++++++++ src/shared/support-prompt.ts | 6 + .../src/context/ExtensionStateContext.tsx | 4 +- .../__tests__/ExtensionStateContext.spec.tsx | 29 +++ 7 files changed, 596 insertions(+), 1 deletion(-) create mode 100644 src/core/task/__tests__/Task.titleGeneration.spec.ts create mode 100644 src/core/task/__tests__/taskTitleGenerator.spec.ts create mode 100644 src/core/task/taskTitleGenerator.ts diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index aa6026e56b..f3f4ac6d06 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -143,6 +143,7 @@ import { validateAndFixToolResultIds } from "./validateToolResultIds" import { mergeConsecutiveApiMessages } from "./mergeConsecutiveApiMessages" import { prepareApiConversationMessage } from "./apiConversationHistory" import { shouldAddUserMessageToHistory } from "./messageCounting" +import { generateTaskTitle } from "./taskTitleGenerator" import { type TaskExecutionContext } from "./providerHandoff" import { abort, abortReason, initialRunState, type Ask, type RunEvent, type RunState } from "./run-state/runState" import { applyRunEvent, type RunRejection } from "./run-state/runStateShadow" @@ -242,6 +243,9 @@ function queuedResponseForAsk(type: ClineAsk, text?: string): QueuedAskResolutio 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 +// Literal, not the vscode enum: specs that mock "vscode" without ExtensionMode would throw on the read. +const EXTENSION_MODE_TEST = 3 + export interface ToolExecutionContext { toolCallId: string didReject: boolean @@ -393,6 +397,11 @@ export class Task extends EventEmitter implements TaskLike { providerRef: WeakRef private readonly globalStoragePath: string currentRequestAbortController?: AbortController + /** + * Controller for background title summarization of the initial user prompt. + * Aborted when the task is cancelled, disposed, or explicitly aborted. + */ + titleAbortController?: AbortController /** * Controller for the waiter on an in-flight `ensureModelFetched()` call (see * safeEnsureModelFetched). Aborting it detaches this task from the provider-side @@ -3108,6 +3117,11 @@ export class Task extends EventEmitter implements TaskLike { await this.say("text", task, images) + // Generate a concise task title in the background for new top-level tasks. + if (task && !this.parentTask) { + void this.generateTitleInBackground(task) + } + // Check for too many MCP tools and warn the user const { enabledToolCount, enabledServerCount } = await this.getEnabledMcpToolsCount() if (enabledToolCount > MAX_MCP_TOOLS_THRESHOLD) { @@ -3181,6 +3195,66 @@ export class Task extends EventEmitter implements TaskLike { } } + /** + * Generates a concise title for the task in the background using the initial prompt. + * If the user cancels the task, or if generation fails/times out, the title is left + * unchanged so the original prompt remains the task title. + */ + private async generateTitleInBackground(taskText: string): Promise { + if (this.abort || this.abandoned) { + return + } + + const provider = this.providerRef.deref() + if (!provider || provider.context?.extensionMode === EXTENSION_MODE_TEST) { + return + } + + const controller = new AbortController() + this.titleAbortController = controller + + try { + const title = await generateTaskTitle(taskText, this.apiConfiguration, { + abortSignal: controller.signal, + timeoutMs: 10000, + taskId: this.taskId, + }) + + if (controller.signal.aborted || this.abort || this.abandoned || !title) { + return + } + + // In tests or fast completion paths, wait briefly until the task is recorded in the store + for (let i = 0; i < 20; i++) { + if (controller.signal.aborted || this.abort || this.abandoned) { + return + } + if (provider.taskHistoryStore.get(this.taskId)) { + break + } + await delay(50) + } + + // If the task already has a custom title set (e.g. user manually renamed it), do not overwrite + const existing = provider.taskHistoryStore.get(this.taskId) + if (existing?.title) { + return + } + + if (controller.signal.aborted || this.abort || this.abandoned) { + return + } + + await provider.renameTaskWithId(this.taskId, title) + } catch (error) { + console.error(`[Task#generateTitleInBackground] Failed for ${this.taskId}:`, error) + } finally { + if (this.titleAbortController === controller) { + this.titleAbortController = undefined + } + } + } + private async resumeTaskFromHistory() { try { const modifiedClineMessages = [...(await this.getSavedClineMessages())] @@ -3534,6 +3608,10 @@ export class Task extends EventEmitter implements TaskLike { this.metadataFetchAbortController.abort() this.metadataFetchAbortController = undefined } + if (this.titleAbortController) { + this.titleAbortController.abort() + this.titleAbortController = undefined + } } /** @@ -3595,6 +3673,10 @@ export class Task extends EventEmitter implements TaskLike { if (isAbandoned) { this.abandoned = true } + if (this.titleAbortController) { + this.titleAbortController.abort() + this.titleAbortController = undefined + } this.driveRunEvent({ tag: "abortRequested", abandoned: isAbandoned }) } diff --git a/src/core/task/__tests__/Task.titleGeneration.spec.ts b/src/core/task/__tests__/Task.titleGeneration.spec.ts new file mode 100644 index 0000000000..438116cf32 --- /dev/null +++ b/src/core/task/__tests__/Task.titleGeneration.spec.ts @@ -0,0 +1,218 @@ +import { describe, it, expect, vi, beforeEach } from "vitest" +import type { ProviderSettings, HistoryItem } from "@roo-code/types" +import { providerIdentifiers } from "@roo-code/types" + +import { Task } from "../Task" +import * as taskTitleGen from "../taskTitleGenerator" +import { setAbortLatch } from "../../../test-utils/run-state" + +vi.mock("../taskTitleGenerator", () => ({ + generateTaskTitle: vi.fn(), +})) + +type TaskTitleAccess = { + generateTitleInBackground(taskText: string): Promise + startTask(task?: string, images?: string[]): Promise + cancelCurrentRequest(): void + requestAbort(isAbandoned: boolean): void + titleAbortController?: AbortController + abort: boolean + abandoned: boolean + taskId: string + instanceId: string + apiConfiguration: ProviderSettings + parentTask?: Task + metadata: { task?: string; images?: string[] } +} + +const getTaskTitleAccess = (task: Task) => task as unknown as TaskTitleAccess + +describe("Task title generation", () => { + const mockApiConfig: ProviderSettings = { + apiProvider: providerIdentifiers.anthropic, + apiKey: "test-key", + } + + let mockHistoryStore: Map + let mockRenameTaskWithId: ReturnType + let mockProvider: { + renameTaskWithId: ReturnType + taskHistoryStore: { + get: (id: string) => HistoryItem | undefined + } + postStateToWebviewWithoutTaskHistory: ReturnType + } + + beforeEach(() => { + vi.clearAllMocks() + mockHistoryStore = new Map() + mockRenameTaskWithId = vi.fn().mockResolvedValue(undefined) + mockProvider = { + renameTaskWithId: mockRenameTaskWithId, + taskHistoryStore: { + get: (id: string) => mockHistoryStore.get(id), + }, + postStateToWebviewWithoutTaskHistory: vi.fn().mockResolvedValue(undefined), + } + }) + + function createTestTask(options: { parentTask?: Task; initialTitle?: string } = {}) { + const task = Object.create(Task.prototype) as Task + setAbortLatch(task, false) + const taskId = "task-title-1" + mockHistoryStore.set(taskId, { + id: taskId, + ts: Date.now(), + task: "Initial prompt message", + title: options.initialTitle, + } as HistoryItem) + + Object.assign(task, { + taskId, + instanceId: "inst-1", + apiConfiguration: mockApiConfig, + providerRef: { deref: () => mockProvider }, + abandoned: false, + parentTask: options.parentTask, + metadata: { task: "Initial prompt message" }, + say: vi.fn().mockResolvedValue(undefined), + getEnabledMcpToolsCount: vi.fn().mockResolvedValue({ enabledToolCount: 0, enabledServerCount: 0 }), + markInitialized: vi.fn(), + initiateTaskLoop: vi.fn().mockResolvedValue(undefined), + driveRunEvent: vi.fn(), + }) + + return task + } + + describe("generateTitleInBackground", () => { + it("calls generateTaskTitle and updates task title via renameTaskWithId", async () => { + vi.mocked(taskTitleGen.generateTaskTitle).mockResolvedValueOnce("Summarized Title") + const task = createTestTask() + + await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") + + expect(taskTitleGen.generateTaskTitle).toHaveBeenCalledWith( + "Initial prompt message", + mockApiConfig, + expect.objectContaining({ + taskId: "task-title-1", + timeoutMs: 10000, + }), + ) + expect(mockRenameTaskWithId).toHaveBeenCalledWith("task-title-1", "Summarized Title") + }) + + it("does not call renameTaskWithId when generateTaskTitle returns undefined", async () => { + vi.mocked(taskTitleGen.generateTaskTitle).mockResolvedValueOnce(undefined) + const task = createTestTask() + + await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") + + expect(mockRenameTaskWithId).not.toHaveBeenCalled() + }) + + it("does not overwrite if the task already has a custom title set", async () => { + vi.mocked(taskTitleGen.generateTaskTitle).mockResolvedValueOnce("Summarized Title") + const task = createTestTask({ initialTitle: "User Custom Title" }) + + await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") + + expect(mockRenameTaskWithId).not.toHaveBeenCalled() + }) + + it("does not call renameTaskWithId if task is aborted during generation", async () => { + const task = createTestTask() + vi.mocked(taskTitleGen.generateTaskTitle).mockImplementationOnce(async () => { + setAbortLatch(task, true) + return "Summarized Title" + }) + + await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") + + expect(mockRenameTaskWithId).not.toHaveBeenCalled() + }) + + it("does not call renameTaskWithId if task is abandoned during generation", async () => { + const task = createTestTask() + vi.mocked(taskTitleGen.generateTaskTitle).mockImplementationOnce(async () => { + task.abandoned = true + return "Summarized Title" + }) + + await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") + + expect(mockRenameTaskWithId).not.toHaveBeenCalled() + }) + + it("cleans up titleAbortController when finished", async () => { + vi.mocked(taskTitleGen.generateTaskTitle).mockResolvedValueOnce("Summarized Title") + const task = createTestTask() + + await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") + + expect(getTaskTitleAccess(task).titleAbortController).toBeUndefined() + }) + }) + + describe("cancellation and abort handling", () => { + it("aborts titleAbortController in cancelCurrentRequest", () => { + const task = createTestTask() + const access = getTaskTitleAccess(task) + const controller = new AbortController() + const abortSpy = vi.spyOn(controller, "abort") + access.titleAbortController = controller + + access.cancelCurrentRequest() + + expect(abortSpy).toHaveBeenCalled() + expect(access.titleAbortController).toBeUndefined() + }) + + it("aborts titleAbortController in requestAbort", () => { + const task = createTestTask() + const access = getTaskTitleAccess(task) + const controller = new AbortController() + const abortSpy = vi.spyOn(controller, "abort") + access.titleAbortController = controller + + access.requestAbort(false) + + expect(abortSpy).toHaveBeenCalled() + expect(access.titleAbortController).toBeUndefined() + }) + }) + + describe("startTask background trigger", () => { + it("starts background title generation for top-level task with prompt", async () => { + const task = createTestTask() + const access = getTaskTitleAccess(task) + const titleBgSpy = vi.spyOn(access, "generateTitleInBackground").mockResolvedValue(undefined) + + await access.startTask("Initial prompt message") + + expect(titleBgSpy).toHaveBeenCalledWith("Initial prompt message") + }) + + it("does not start title generation for child/subtasks", async () => { + const parent = createTestTask() + const child = createTestTask({ parentTask: parent }) + const access = getTaskTitleAccess(child) + const titleBgSpy = vi.spyOn(access, "generateTitleInBackground").mockResolvedValue(undefined) + + await access.startTask("Child subtask prompt") + + expect(titleBgSpy).not.toHaveBeenCalled() + }) + + it("does not start title generation when prompt is omitted", async () => { + const task = createTestTask() + const access = getTaskTitleAccess(task) + const titleBgSpy = vi.spyOn(access, "generateTitleInBackground").mockResolvedValue(undefined) + + await access.startTask(undefined) + + expect(titleBgSpy).not.toHaveBeenCalled() + }) + }) +}) diff --git a/src/core/task/__tests__/taskTitleGenerator.spec.ts b/src/core/task/__tests__/taskTitleGenerator.spec.ts new file mode 100644 index 0000000000..dfec61c849 --- /dev/null +++ b/src/core/task/__tests__/taskTitleGenerator.spec.ts @@ -0,0 +1,158 @@ +import { describe, it, expect, vi, beforeEach } from "vitest" +import type { ProviderSettings } from "@roo-code/types" +import { providerIdentifiers } from "@roo-code/types" + +import { + generateTaskTitle, + cleanGeneratedTitle, + MAX_TITLE_LENGTH, + MAX_INPUT_PROMPT_LENGTH, +} from "../taskTitleGenerator" +import { singleCompletionHandler } from "../../../utils/single-completion-handler" + +vi.mock("../../../utils/single-completion-handler", () => ({ + singleCompletionHandler: vi.fn(), +})) + +describe("taskTitleGenerator", () => { + const mockApiConfig: ProviderSettings = { + apiProvider: providerIdentifiers.anthropic, + apiKey: "test-key", + } + + beforeEach(() => { + vi.clearAllMocks() + }) + + describe("cleanGeneratedTitle", () => { + it("normalizes whitespace and linebreaks", () => { + const input = " Fix the\n\nlogin issue " + expect(cleanGeneratedTitle(input)).toBe("Fix the login issue") + }) + + it("removes enclosing double quotes", () => { + expect(cleanGeneratedTitle('"Fix login page"')).toBe("Fix login page") + }) + + it("removes enclosing single quotes and backticks", () => { + expect(cleanGeneratedTitle("'Fix login page'")).toBe("Fix login page") + expect(cleanGeneratedTitle("`Fix login page`")).toBe("Fix login page") + }) + + it("removes trailing punctuation (periods, exclamation marks)", () => { + expect(cleanGeneratedTitle("Fix login page.")).toBe("Fix login page") + expect(cleanGeneratedTitle("Fix login page!!!")).toBe("Fix login page") + expect(cleanGeneratedTitle("Fix login page!?")).toBe("Fix login page") + }) + + it("truncates titles exceeding MAX_TITLE_LENGTH", () => { + const longTitle = "A".repeat(MAX_TITLE_LENGTH + 20) + const cleaned = cleanGeneratedTitle(longTitle) + expect(cleaned.length).toBe(MAX_TITLE_LENGTH) + expect(cleaned).toBe("A".repeat(MAX_TITLE_LENGTH)) + }) + }) + + describe("generateTaskTitle", () => { + it("returns undefined when prompt is empty or only whitespace", async () => { + expect(await generateTaskTitle("", mockApiConfig)).toBeUndefined() + expect(await generateTaskTitle(" \n\t ", mockApiConfig)).toBeUndefined() + expect(singleCompletionHandler).not.toHaveBeenCalled() + }) + + it("returns undefined when abortSignal is already aborted", async () => { + const controller = new AbortController() + controller.abort() + + const result = await generateTaskTitle("Do something", mockApiConfig, { + abortSignal: controller.signal, + }) + + expect(result).toBeUndefined() + expect(singleCompletionHandler).not.toHaveBeenCalled() + }) + + it("returns undefined when apiConfiguration is invalid or missing apiProvider", async () => { + const invalidConfig = {} as unknown as ProviderSettings + expect(await generateTaskTitle("Do something", invalidConfig)).toBeUndefined() + expect(singleCompletionHandler).not.toHaveBeenCalled() + }) + + it("generates and cleans title on successful completion", async () => { + vi.mocked(singleCompletionHandler).mockResolvedValueOnce(' "Fix Login Bug." ') + + const result = await generateTaskTitle("Please fix the login page button bug", mockApiConfig, { + taskId: "task-123", + }) + + expect(result).toBe("Fix Login Bug") + expect(singleCompletionHandler).toHaveBeenCalledWith( + mockApiConfig, + expect.stringContaining("Please fix the login page button bug"), + expect.objectContaining({ + taskId: "task-123", + timeoutMs: 10000, + }), + ) + }) + + it("truncates prompt exceeding MAX_INPUT_PROMPT_LENGTH before sending", async () => { + const longPrompt = "x".repeat(MAX_INPUT_PROMPT_LENGTH + 500) + vi.mocked(singleCompletionHandler).mockResolvedValueOnce("Summary Title") + + await generateTaskTitle(longPrompt, mockApiConfig) + + const calledPrompt = vi.mocked(singleCompletionHandler).mock.calls[0]?.[1] as string + expect(calledPrompt).toContain("x".repeat(MAX_INPUT_PROMPT_LENGTH)) + expect(calledPrompt).not.toContain("x".repeat(MAX_INPUT_PROMPT_LENGTH + 1)) + }) + + it("returns undefined gracefully when singleCompletionHandler throws", async () => { + vi.mocked(singleCompletionHandler).mockRejectedValueOnce(new Error("API rate limited")) + + const result = await generateTaskTitle("Do something", mockApiConfig) + expect(result).toBeUndefined() + }) + + it("aborts when parent signal triggers during completion", async () => { + const controller = new AbortController() + vi.mocked(singleCompletionHandler).mockImplementationOnce(async (_config, _prompt, opts) => { + controller.abort() + if (opts?.abortSignal?.aborted) { + throw new Error("Aborted") + } + return "Completed Title" + }) + + const result = await generateTaskTitle("Do something", mockApiConfig, { + abortSignal: controller.signal, + }) + + expect(result).toBeUndefined() + }) + + it("aborts when timeout is reached", async () => { + vi.mocked(singleCompletionHandler).mockImplementationOnce( + (_config, _prompt, opts) => + new Promise((resolve, reject) => { + opts?.abortSignal?.addEventListener("abort", () => { + reject(new Error("Timeout aborted")) + }) + }), + ) + + const result = await generateTaskTitle("Do something", mockApiConfig, { + timeoutMs: 50, + }) + + expect(result).toBeUndefined() + }) + + it("returns undefined when cleaned title is empty", async () => { + vi.mocked(singleCompletionHandler).mockResolvedValueOnce(' "" ') + + const result = await generateTaskTitle("Do something", mockApiConfig) + expect(result).toBeUndefined() + }) + }) +}) diff --git a/src/core/task/taskTitleGenerator.ts b/src/core/task/taskTitleGenerator.ts new file mode 100644 index 0000000000..edb22c8429 --- /dev/null +++ b/src/core/task/taskTitleGenerator.ts @@ -0,0 +1,100 @@ +import type { ProviderSettings } from "@roo-code/types" + +import { supportPrompt } from "../../shared/support-prompt" +import { singleCompletionHandler } from "../../utils/single-completion-handler" + +export const MAX_INPUT_PROMPT_LENGTH = 1000 +export const MAX_TITLE_LENGTH = 50 +export const DEFAULT_TITLE_TIMEOUT_MS = 10000 + +export interface GenerateTaskTitleOptions { + abortSignal?: AbortSignal + timeoutMs?: number + taskId?: string +} + +/** + * Normalizes raw LLM output into a single-line, clean title. + * Strips surrounding quotes, trailing punctuation, collapses whitespace, + * and limits the length to MAX_TITLE_LENGTH characters. + */ +export function cleanGeneratedTitle(rawTitle: string): string { + let cleaned = rawTitle.replace(/\s+/g, " ").trim() + // Strip opening/closing quotes if present + cleaned = cleaned.replace(/^["'`]|["'`]$/g, "").trim() + // Strip trailing periods or exclamation marks + cleaned = cleaned.replace(/[.!?]+$/, "").trim() + + if (cleaned.length > MAX_TITLE_LENGTH) { + cleaned = cleaned.slice(0, MAX_TITLE_LENGTH).trim() + } + + return cleaned +} + +/** + * Generates a concise summary title for a task from its initial prompt. + * Runs with a strict timeout and length limit to prevent hanging. + * Returns undefined on error, timeout, or cancellation so callers safely fall back. + */ +export async function generateTaskTitle( + prompt: string, + apiConfiguration: ProviderSettings, + options?: GenerateTaskTitleOptions, +): Promise { + const trimmedPrompt = prompt?.trim() + if (!trimmedPrompt) { + return undefined + } + + if (options?.abortSignal?.aborted) { + return undefined + } + + if (!apiConfiguration || !apiConfiguration.apiProvider) { + return undefined + } + + const timeoutMs = options?.timeoutMs ?? DEFAULT_TITLE_TIMEOUT_MS + const controller = new AbortController() + const parentSignal = options?.abortSignal + + const onParentAbort = () => { + controller.abort() + } + + if (parentSignal) { + parentSignal.addEventListener("abort", onParentAbort, { once: true }) + } + + const timeoutId = setTimeout(() => { + controller.abort() + }, timeoutMs) + + try { + const promptSnippet = trimmedPrompt.slice(0, MAX_INPUT_PROMPT_LENGTH) + const titlePrompt = supportPrompt.create("TASK_TITLE", { userInput: promptSnippet }) + + const rawTitle = await singleCompletionHandler(apiConfiguration, titlePrompt, { + abortSignal: controller.signal, + timeoutMs, + taskId: options?.taskId, + }) + + if (controller.signal.aborted || parentSignal?.aborted) { + return undefined + } + + const cleaned = cleanGeneratedTitle(rawTitle) + return cleaned.length > 0 ? cleaned : undefined + } catch (error) { + // All errors (timeout, cancellation, API errors) fall back gracefully + console.debug("[taskTitleGenerator] Title generation failed or cancelled:", error) + return undefined + } finally { + clearTimeout(timeoutId) + if (parentSignal) { + parentSignal.removeEventListener("abort", onParentAbort) + } + } +} diff --git a/src/shared/support-prompt.ts b/src/shared/support-prompt.ts index 6fe48fa364..1022272332 100644 --- a/src/shared/support-prompt.ts +++ b/src/shared/support-prompt.ts @@ -50,8 +50,14 @@ type SupportPromptType = | "TERMINAL_FIX" | "TERMINAL_EXPLAIN" | "NEW_TASK" + | "TASK_TITLE" const supportPromptConfigs: Record = { + TASK_TITLE: { + template: `Generate a concise title summarizing the following user prompt. The title should be in the same language as the prompt, 3 to 8 words long (maximum 50 characters). Reply with ONLY the title itself, with no quotation marks, no ending punctuation, and no explanations: + +\${userInput}`, + }, ENHANCE: { template: `Generate an enhanced version of this prompt (reply with only the enhanced prompt - no conversation, explanations, lead-in, bullet points, placeholders, or surrounding quotes): diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index 770f051dc5..2c32e8e6ad 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/webview-ui/src/context/ExtensionStateContext.tsx @@ -614,7 +614,9 @@ export const ExtensionStateContextProvider: React.FC<{ ...prevState, taskHistory: nextHistory, currentTaskItem: - prevState.currentTaskItem?.id === item.id ? item : prevState.currentTaskItem, + prevState.currentTaskItem?.id === item.id || prevState.currentTaskId === item.id + ? item + : prevState.currentTaskItem, } }) break diff --git a/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx b/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx index 424255245a..474f8d0522 100644 --- a/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx +++ b/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx @@ -174,6 +174,35 @@ describe("ExtensionStateContext", () => { expect(JSON.parse(screen.getByTestId("current-task-item").textContent!)).toEqual(updated) }) + it("updates currentTaskItem on taskHistoryItemUpdated when currentTaskId matches even if currentTaskItem was unset", () => { + const item = { + id: "task-live", + number: 1, + ts: 100, + task: "Live prompt", + title: "Summarized Title", + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + } + render( + + + , + ) + + act(() => { + window.dispatchEvent( + new MessageEvent("message", { + data: { type: "taskHistoryItemUpdated", taskHistoryItem: item }, + }), + ) + }) + + expect(JSON.parse(screen.getByTestId("current-task-item").textContent!)).toEqual(item) + }) + it("does not restore a key dropped by a snapshot message", () => { const payload: ClineMessage = { ts: 1, type: "say", say: "text", text: "seed", partial: false } const internSpy = vi.spyOn(StringCache.prototype, "intern") From b84b4f656c3e914164fc89e274a18098d39ad1cd Mon Sep 17 00:00:00 2001 From: daewoongoh Date: Mon, 9 Nov 2026 10:51:19 +0900 Subject: [PATCH 2/8] feat: refine background task title generation Update title generation in Task and ClineProvider, adjust the title summary prompt, and extend tests for the generator and task history. --- src/core/task/Task.ts | 70 +++++++--- .../__tests__/Task.titleGeneration.spec.ts | 130 +++++++++++++----- .../task/__tests__/taskTitleGenerator.spec.ts | 29 +++- src/core/task/taskTitleGenerator.ts | 16 ++- src/core/webview/ClineProvider.ts | 39 ++++++ .../ClineProvider.taskHistory.spec.ts | 67 +++++++++ src/shared/support-prompt.ts | 2 +- 7 files changed, 293 insertions(+), 60 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index f3f4ac6d06..5f5aa0befd 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -143,7 +143,7 @@ import { validateAndFixToolResultIds } from "./validateToolResultIds" import { mergeConsecutiveApiMessages } from "./mergeConsecutiveApiMessages" import { prepareApiConversationMessage } from "./apiConversationHistory" import { shouldAddUserMessageToHistory } from "./messageCounting" -import { generateTaskTitle } from "./taskTitleGenerator" +import { DEFAULT_TITLE_TIMEOUT_MS, fallbackTaskTitle, generateTaskTitle } from "./taskTitleGenerator" import { type TaskExecutionContext } from "./providerHandoff" import { abort, abortReason, initialRunState, type Ask, type RunEvent, type RunState } from "./run-state/runState" import { applyRunEvent, type RunRejection } from "./run-state/runStateShadow" @@ -3195,10 +3195,19 @@ export class Task extends EventEmitter implements TaskLike { } } + // Issue #2333: bounded attempts for the atomic generated-title write. The + // task may not be recorded in the history store yet when the title is + // ready; each attempt retries the atomic check-and-write under the lock. + private static readonly TITLE_WRITE_ATTEMPTS = 3 + private static readonly TITLE_WRITE_RETRY_DELAY_MS = 50 + /** * Generates a concise title for the task in the background using the initial prompt. - * If the user cancels the task, or if generation fails/times out, the title is left - * unchanged so the original prompt remains the task title. + * When generation fails, times out, or returns no title, the title falls back to + * the first 42 characters of the initial prompt (issue #2333). If the user cancels + * the task, no title is written. A manual rename always wins: the existing-title + * check and the write happen inside one atomic store update, so a rename queued + * ahead of this write is never overwritten. */ private async generateTitleInBackground(taskText: string): Promise { if (this.abort || this.abandoned) { @@ -3214,38 +3223,55 @@ export class Task extends EventEmitter implements TaskLike { this.titleAbortController = controller try { - const title = await generateTaskTitle(taskText, this.apiConfiguration, { + const generated = await generateTaskTitle(taskText, this.apiConfiguration, { abortSignal: controller.signal, - timeoutMs: 10000, + timeoutMs: DEFAULT_TITLE_TIMEOUT_MS, taskId: this.taskId, }) - if (controller.signal.aborted || this.abort || this.abandoned || !title) { + if (controller.signal.aborted || this.abort || this.abandoned) { return } - // In tests or fast completion paths, wait briefly until the task is recorded in the store - for (let i = 0; i < 20; i++) { + // Issue #2333 fallback: when generation fails, times out, or returns + // no usable title, use the first 42 characters of the initial prompt. + const fallback = fallbackTaskTitle(taskText) + const desiredTitle = generated ?? fallback + + // The atomic write only applies while the item has no title. A + // failure (e.g. the task not yet recorded in the store) is retried a + // bounded number of times before the truncated-prompt fallback title + // gets a final attempt. + for (let attempt = 1; attempt <= Task.TITLE_WRITE_ATTEMPTS; attempt++) { if (controller.signal.aborted || this.abort || this.abandoned) { return } - if (provider.taskHistoryStore.get(this.taskId)) { - break - } - await delay(50) - } - // If the task already has a custom title set (e.g. user manually renamed it), do not overwrite - const existing = provider.taskHistoryStore.get(this.taskId) - if (existing?.title) { - return - } + try { + await provider.setGeneratedTaskTitle(this.taskId, desiredTitle) + return + } catch (error) { + console.debug( + `[Task#generateTitleInBackground] Title write attempt ${attempt} failed for ${this.taskId}:`, + error, + ) - if (controller.signal.aborted || this.abort || this.abandoned) { - return + if (attempt === Task.TITLE_WRITE_ATTEMPTS) { + // Retries exhausted: final attempt with the truncated-prompt + // fallback title (issue #2333). + try { + await provider.setGeneratedTaskTitle(this.taskId, fallback) + } catch (fallbackError) { + console.error( + `[Task#generateTitleInBackground] Fallback title failed for ${this.taskId}:`, + fallbackError, + ) + } + } else { + await delay(Task.TITLE_WRITE_RETRY_DELAY_MS) + } + } } - - await provider.renameTaskWithId(this.taskId, title) } catch (error) { console.error(`[Task#generateTitleInBackground] Failed for ${this.taskId}:`, error) } finally { diff --git a/src/core/task/__tests__/Task.titleGeneration.spec.ts b/src/core/task/__tests__/Task.titleGeneration.spec.ts index 438116cf32..0b1c4321e5 100644 --- a/src/core/task/__tests__/Task.titleGeneration.spec.ts +++ b/src/core/task/__tests__/Task.titleGeneration.spec.ts @@ -1,14 +1,20 @@ import { describe, it, expect, vi, beforeEach } from "vitest" -import type { ProviderSettings, HistoryItem } from "@roo-code/types" +import type { ProviderSettings } from "@roo-code/types" import { providerIdentifiers } from "@roo-code/types" import { Task } from "../Task" import * as taskTitleGen from "../taskTitleGenerator" import { setAbortLatch } from "../../../test-utils/run-state" -vi.mock("../taskTitleGenerator", () => ({ - generateTaskTitle: vi.fn(), -})) +// Keep the real constants and the fallback helper; only the LLM call is mocked +// so `generateTitleInBackground` exercises the real fallback/retry decisions. +vi.mock("../taskTitleGenerator", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + generateTaskTitle: vi.fn(), + } +}) type TaskTitleAccess = { generateTitleInBackground(taskText: string): Promise @@ -23,6 +29,7 @@ type TaskTitleAccess = { apiConfiguration: ProviderSettings parentTask?: Task metadata: { task?: string; images?: string[] } + initiateTaskLoop: ReturnType } const getTaskTitleAccess = (task: Task) => task as unknown as TaskTitleAccess @@ -33,39 +40,25 @@ describe("Task title generation", () => { apiKey: "test-key", } - let mockHistoryStore: Map - let mockRenameTaskWithId: ReturnType + let mockSetGeneratedTaskTitle: ReturnType let mockProvider: { - renameTaskWithId: ReturnType - taskHistoryStore: { - get: (id: string) => HistoryItem | undefined - } + setGeneratedTaskTitle: ReturnType postStateToWebviewWithoutTaskHistory: ReturnType } beforeEach(() => { vi.clearAllMocks() - mockHistoryStore = new Map() - mockRenameTaskWithId = vi.fn().mockResolvedValue(undefined) + mockSetGeneratedTaskTitle = vi.fn().mockResolvedValue(true) mockProvider = { - renameTaskWithId: mockRenameTaskWithId, - taskHistoryStore: { - get: (id: string) => mockHistoryStore.get(id), - }, + setGeneratedTaskTitle: mockSetGeneratedTaskTitle, postStateToWebviewWithoutTaskHistory: vi.fn().mockResolvedValue(undefined), } }) - function createTestTask(options: { parentTask?: Task; initialTitle?: string } = {}) { + function createTestTask(options: { parentTask?: Task } = {}) { const task = Object.create(Task.prototype) as Task setAbortLatch(task, false) const taskId = "task-title-1" - mockHistoryStore.set(taskId, { - id: taskId, - ts: Date.now(), - task: "Initial prompt message", - title: options.initialTitle, - } as HistoryItem) Object.assign(task, { taskId, @@ -86,7 +79,7 @@ describe("Task title generation", () => { } describe("generateTitleInBackground", () => { - it("calls generateTaskTitle and updates task title via renameTaskWithId", async () => { + it("applies the generated title through the atomic conditional write", async () => { vi.mocked(taskTitleGen.generateTaskTitle).mockResolvedValueOnce("Summarized Title") const task = createTestTask() @@ -97,31 +90,74 @@ describe("Task title generation", () => { mockApiConfig, expect.objectContaining({ taskId: "task-title-1", - timeoutMs: 10000, + timeoutMs: taskTitleGen.DEFAULT_TITLE_TIMEOUT_MS, }), ) - expect(mockRenameTaskWithId).toHaveBeenCalledWith("task-title-1", "Summarized Title") + // Issue #2333: the model request is limited to 15 seconds. + expect(taskTitleGen.DEFAULT_TITLE_TIMEOUT_MS).toBe(15000) + expect(mockSetGeneratedTaskTitle).toHaveBeenCalledTimes(1) + expect(mockSetGeneratedTaskTitle).toHaveBeenCalledWith("task-title-1", "Summarized Title") }) - it("does not call renameTaskWithId when generateTaskTitle returns undefined", async () => { + it("falls back to the first 42 characters of the prompt when generation fails (issue #2333)", async () => { vi.mocked(taskTitleGen.generateTaskTitle).mockResolvedValueOnce(undefined) + const longPrompt = "Please fix the login page button alignment issue on mobile devices" + const task = createTestTask() + + await getTaskTitleAccess(task).generateTitleInBackground(longPrompt) + + const expectedFallback = taskTitleGen.fallbackTaskTitle(longPrompt) + expect(expectedFallback).toHaveLength(42) + expect(mockSetGeneratedTaskTitle).toHaveBeenCalledTimes(1) + expect(mockSetGeneratedTaskTitle).toHaveBeenCalledWith("task-title-1", expectedFallback) + }) + + it("keeps an existing title instead of overwriting it (manual rename wins)", async () => { + vi.mocked(taskTitleGen.generateTaskTitle).mockResolvedValueOnce("Summarized Title") + mockSetGeneratedTaskTitle.mockResolvedValueOnce(false) + const task = createTestTask() + + await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") + + // `false` means the item already had a title: no retry, no overwrite. + expect(mockSetGeneratedTaskTitle).toHaveBeenCalledTimes(1) + }) + + it("retries the atomic write when the task is not yet recorded in the store", async () => { + vi.mocked(taskTitleGen.generateTaskTitle).mockResolvedValueOnce("Summarized Title") + mockSetGeneratedTaskTitle + .mockRejectedValueOnce(new Error("task not found in cache")) + .mockResolvedValueOnce(true) const task = createTestTask() await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") - expect(mockRenameTaskWithId).not.toHaveBeenCalled() + expect(mockSetGeneratedTaskTitle).toHaveBeenCalledTimes(2) + expect(mockSetGeneratedTaskTitle).toHaveBeenLastCalledWith("task-title-1", "Summarized Title") }) - it("does not overwrite if the task already has a custom title set", async () => { + it("applies the truncated-prompt fallback title after the write retries are exhausted (issue #2333)", async () => { vi.mocked(taskTitleGen.generateTaskTitle).mockResolvedValueOnce("Summarized Title") - const task = createTestTask({ initialTitle: "User Custom Title" }) + mockSetGeneratedTaskTitle + .mockRejectedValueOnce(new Error("task not found in cache")) + .mockRejectedValueOnce(new Error("task not found in cache")) + .mockRejectedValueOnce(new Error("task not found in cache")) + .mockResolvedValueOnce(true) + const task = createTestTask() await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") - expect(mockRenameTaskWithId).not.toHaveBeenCalled() + // Three attempts with the generated title, then one with the fallback. + expect(mockSetGeneratedTaskTitle).toHaveBeenCalledTimes(4) + expect(mockSetGeneratedTaskTitle).toHaveBeenNthCalledWith(1, "task-title-1", "Summarized Title") + expect(mockSetGeneratedTaskTitle).toHaveBeenNthCalledWith(3, "task-title-1", "Summarized Title") + expect(mockSetGeneratedTaskTitle).toHaveBeenLastCalledWith( + "task-title-1", + taskTitleGen.fallbackTaskTitle("Initial prompt message"), + ) }) - it("does not call renameTaskWithId if task is aborted during generation", async () => { + it("does not call setGeneratedTaskTitle if task is aborted during generation", async () => { const task = createTestTask() vi.mocked(taskTitleGen.generateTaskTitle).mockImplementationOnce(async () => { setAbortLatch(task, true) @@ -130,10 +166,10 @@ describe("Task title generation", () => { await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") - expect(mockRenameTaskWithId).not.toHaveBeenCalled() + expect(mockSetGeneratedTaskTitle).not.toHaveBeenCalled() }) - it("does not call renameTaskWithId if task is abandoned during generation", async () => { + it("does not call setGeneratedTaskTitle if task is abandoned during generation", async () => { const task = createTestTask() vi.mocked(taskTitleGen.generateTaskTitle).mockImplementationOnce(async () => { task.abandoned = true @@ -142,7 +178,7 @@ describe("Task title generation", () => { await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") - expect(mockRenameTaskWithId).not.toHaveBeenCalled() + expect(mockSetGeneratedTaskTitle).not.toHaveBeenCalled() }) it("cleans up titleAbortController when finished", async () => { @@ -214,5 +250,29 @@ describe("Task title generation", () => { expect(titleBgSpy).not.toHaveBeenCalled() }) + + it("starts the task loop without waiting for title generation to settle", async () => { + // Regression evidence: the background generation must never block the + // task loop. Keep generation pending and verify the loop already ran. + let resolveGeneration!: (title: string | undefined) => void + vi.mocked(taskTitleGen.generateTaskTitle).mockImplementationOnce( + () => + new Promise((resolve) => { + resolveGeneration = resolve + }), + ) + const task = createTestTask() + const access = getTaskTitleAccess(task) + + const started = access.startTask("Initial prompt message") + + // Generation is still pending here; the task loop must have started. + await vi.waitFor(() => expect(access.initiateTaskLoop).toHaveBeenCalled()) + + resolveGeneration("Summarized Title") + await started + await vi.waitFor(() => expect(mockSetGeneratedTaskTitle).toHaveBeenCalled()) + expect(mockSetGeneratedTaskTitle).toHaveBeenCalledWith("task-title-1", "Summarized Title") + }) }) }) diff --git a/src/core/task/__tests__/taskTitleGenerator.spec.ts b/src/core/task/__tests__/taskTitleGenerator.spec.ts index dfec61c849..23187a1c51 100644 --- a/src/core/task/__tests__/taskTitleGenerator.spec.ts +++ b/src/core/task/__tests__/taskTitleGenerator.spec.ts @@ -5,8 +5,11 @@ import { providerIdentifiers } from "@roo-code/types" import { generateTaskTitle, cleanGeneratedTitle, + fallbackTaskTitle, MAX_TITLE_LENGTH, MAX_INPUT_PROMPT_LENGTH, + FALLBACK_TITLE_MAX_LENGTH, + DEFAULT_TITLE_TIMEOUT_MS, } from "../taskTitleGenerator" import { singleCompletionHandler } from "../../../utils/single-completion-handler" @@ -24,6 +27,27 @@ describe("taskTitleGenerator", () => { vi.clearAllMocks() }) + describe("fallbackTaskTitle", () => { + it("returns the first 42 characters of the prompt (issue #2333)", () => { + expect(fallbackTaskTitle("Fix the login page button alignment issue on mobile devices")).toBe( + "Fix the login page button alignment issue", + ) + }) + + it("truncates prompts longer than FALLBACK_TITLE_MAX_LENGTH", () => { + const longPrompt = "x".repeat(FALLBACK_TITLE_MAX_LENGTH + 10) + expect(fallbackTaskTitle(longPrompt)).toBe("x".repeat(FALLBACK_TITLE_MAX_LENGTH)) + }) + + it("normalizes whitespace before truncating", () => { + expect(fallbackTaskTitle(" Fix the\n\nlogin issue ")).toBe("Fix the login issue") + }) + + it("keeps shorter prompts unchanged", () => { + expect(fallbackTaskTitle("Short prompt")).toBe("Short prompt") + }) + }) + describe("cleanGeneratedTitle", () => { it("normalizes whitespace and linebreaks", () => { const input = " Fix the\n\nlogin issue " @@ -91,9 +115,12 @@ describe("taskTitleGenerator", () => { expect.stringContaining("Please fix the login page button bug"), expect.objectContaining({ taskId: "task-123", - timeoutMs: 10000, + timeoutMs: DEFAULT_TITLE_TIMEOUT_MS, }), ) + + // Issue #2333: the model request for the title is limited to 15 seconds. + expect(DEFAULT_TITLE_TIMEOUT_MS).toBe(15000) }) it("truncates prompt exceeding MAX_INPUT_PROMPT_LENGTH before sending", async () => { diff --git a/src/core/task/taskTitleGenerator.ts b/src/core/task/taskTitleGenerator.ts index edb22c8429..e11737f0a7 100644 --- a/src/core/task/taskTitleGenerator.ts +++ b/src/core/task/taskTitleGenerator.ts @@ -5,7 +5,11 @@ import { singleCompletionHandler } from "../../utils/single-completion-handler" export const MAX_INPUT_PROMPT_LENGTH = 1000 export const MAX_TITLE_LENGTH = 50 -export const DEFAULT_TITLE_TIMEOUT_MS = 10000 +// Issue #2333: the model request for the title must be limited to 15 seconds. +export const DEFAULT_TITLE_TIMEOUT_MS = 15000 +// Issue #2333: when generation fails or times out, the title falls back to the +// first 42 characters of the initial prompt. +export const FALLBACK_TITLE_MAX_LENGTH = 42 export interface GenerateTaskTitleOptions { abortSignal?: AbortSignal @@ -32,6 +36,16 @@ export function cleanGeneratedTitle(rawTitle: string): string { return cleaned } +/** + * Issue #2333 fallback: the first FALLBACK_TITLE_MAX_LENGTH characters of the + * initial prompt, used when title generation fails, times out, or returns no + * usable title. + */ +export function fallbackTaskTitle(prompt: string): string { + const normalized = (prompt ?? "").replace(/\s+/g, " ").trim() + return normalized.slice(0, FALLBACK_TITLE_MAX_LENGTH).trim() +} + /** * Generates a concise summary title for a task from its initial prompt. * Runs with a strict timeout and length limit to prevent hanging. diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 6cec3bdbe7..df86ac20a1 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -2831,6 +2831,45 @@ export class ClineProvider } } + /* + * Applies an automatically generated title for a history item (issue #2333). + * The existing-title check and the write happen inside one atomic store + * update, so a manual rename queued ahead of this call is never overwritten + * by the generated title. Returns true when the title was applied, false + * when the item already had a title. Throws when the task is not in the + * store yet so the caller can retry. + */ + async setGeneratedTaskTitle(id: string, title: string): Promise { + const trimmed = title.trim() + if (!trimmed) { + return false + } + + let applied = false + await this.taskHistoryStore.atomicReadAndUpdate(id, (historyItem) => { + // Preserve any title set before this write: a manual rename, or an + // earlier generated title, always wins over a later generated one. + if (historyItem.title) { + return historyItem + } + applied = true + return { ...historyItem, title: trimmed } + }) + + if (applied) { + this.recentTasksCache = undefined + + if (this.isViewLaunched) { + const updatedItem = this.taskHistoryStore.get(id) + if (updatedItem) { + await this.postMessageToWebview({ type: "taskHistoryItemUpdated", taskHistoryItem: updatedItem }) + } + } + } + + return applied + } + /* Condenses a task's message history to use fewer tokens. */ async condenseTaskContext(taskId: string) { const task = this.taskRegistry.getById(taskId) diff --git a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts index 7b6223e2e2..5a74e04cc3 100644 --- a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts @@ -690,6 +690,73 @@ describe("ClineProvider Task History Synchronization", () => { }) }) + describe("setGeneratedTaskTitle", () => { + const getStored = (id: string) => provider.taskHistoryStore.get(id) + + it("applies the generated title when the task has none and broadcasts the update", async () => { + await provider.resolveWebviewView(mockWebviewView) + provider.isViewLaunched = true + await provider.updateTaskHistory(createHistoryItem({ id: "task-gen", task: "Original prompt" }), { + broadcast: false, + }) + mockPostMessage.mockClear() + + await expect(provider.setGeneratedTaskTitle("task-gen", "Generated Title")).resolves.toBe(true) + + expect(getStored("task-gen")).toMatchObject({ title: "Generated Title", task: "Original prompt" }) + const updates = findCallsByType(mockPostMessage.mock.calls, "taskHistoryItemUpdated") + expect(updates.at(-1)?.[0].taskHistoryItem).toMatchObject({ id: "task-gen", title: "Generated Title" }) + }) + + it("does not overwrite a manual rename queued before the generated-title write", async () => { + const item = createHistoryItem({ id: "task-gen-race", task: "Original prompt", status: "active" }) + await provider.updateTaskHistory(item, { broadcast: false }) + + // The manual rename queues on the store lock first; the generated-title + // write runs behind it and must see the manual title inside the same + // atomic update instead of overwriting it. + const manual = provider.renameTaskWithId("task-gen-race", "Manual Title") + const generated = provider.setGeneratedTaskTitle("task-gen-race", "Generated Title") + const [generatedApplied] = await Promise.all([generated, manual]) + + expect(generatedApplied).toBe(false) + expect(getStored("task-gen-race")?.title).toBe("Manual Title") + }) + + it("lets a manual rename win when it is queued after the generated title", async () => { + await provider.updateTaskHistory(createHistoryItem({ id: "task-gen-after", task: "Original prompt" }), { + broadcast: false, + }) + + const generated = provider.setGeneratedTaskTitle("task-gen-after", "Generated Title") + const manual = provider.renameTaskWithId("task-gen-after", "Manual Title") + await Promise.all([generated, manual]) + + // Last writer wins: the user's explicit rename is the final title. + expect(getStored("task-gen-after")?.title).toBe("Manual Title") + }) + + it("keeps an existing title and reports that nothing was applied", async () => { + await provider.updateTaskHistory(createHistoryItem({ id: "task-gen-existing", task: "Original prompt" }), { + broadcast: false, + }) + await provider.renameTaskWithId("task-gen-existing", "Manual Title") + mockPostMessage.mockClear() + + await expect(provider.setGeneratedTaskTitle("task-gen-existing", "Generated Title")).resolves.toBe(false) + + expect(getStored("task-gen-existing")?.title).toBe("Manual Title") + const updates = findCallsByType(mockPostMessage.mock.calls, "taskHistoryItemUpdated") + expect(updates).toHaveLength(0) + }) + + it("throws when the task is not yet recorded so the caller can retry", async () => { + await expect(provider.setGeneratedTaskTitle("missing-task", "Generated Title")).rejects.toThrow( + "missing-task", + ) + }) + }) + describe("broadcastTaskHistoryUpdate", () => { it("sends taskHistoryUpdated message with sorted history", async () => { await provider.resolveWebviewView(mockWebviewView) diff --git a/src/shared/support-prompt.ts b/src/shared/support-prompt.ts index 1022272332..3f54777c6b 100644 --- a/src/shared/support-prompt.ts +++ b/src/shared/support-prompt.ts @@ -54,7 +54,7 @@ type SupportPromptType = const supportPromptConfigs: Record = { TASK_TITLE: { - template: `Generate a concise title summarizing the following user prompt. The title should be in the same language as the prompt, 3 to 8 words long (maximum 50 characters). Reply with ONLY the title itself, with no quotation marks, no ending punctuation, and no explanations: + template: `Generate a concise title summarizing the following user prompt. The title should be in the same language as the prompt, 5 to 10 words long (maximum 50 characters). Reply with ONLY the title itself, with no quotation marks, no ending punctuation, and no explanations: \${userInput}`, }, From d798435b6ff076f1f5a7564a5464ba4a05af948f Mon Sep 17 00:00:00 2001 From: daewoongoh Date: Mon, 9 Nov 2026 11:03:26 +0900 Subject: [PATCH 3/8] feat: coordinate title generation with provider rate limit Make the background title request wait out the shared provider rate-limit window and record on the shared clock, without emitting task-loop UX. --- src/core/task/Task.ts | 41 +++++++ .../__tests__/Task.titleGeneration.spec.ts | 108 +++++++++++++++++- 2 files changed, 147 insertions(+), 2 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 5f5aa0befd..d964fb19b6 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -3223,6 +3223,20 @@ export class Task extends EventEmitter implements TaskLike { this.titleAbortController = controller try { + // The title request participates in the same provider rate-limit + // coordination as task requests: it waits out the configured window + // (silently — the countdown say UX belongs to the task loop) and + // records on the shared clock so subsequent task/subtask requests + // honour it. `startTask` schedules this method with `void`, so the + // agent never waits on the title result. + await this.waitForProviderRateLimitWindow(controller.signal) + + if (controller.signal.aborted || this.abort || this.abandoned) { + return + } + + this.rateLimitClock.recordRequest() + const generated = await generateTaskTitle(taskText, this.apiConfiguration, { abortSignal: controller.signal, timeoutMs: DEFAULT_TITLE_TIMEOUT_MS, @@ -5792,6 +5806,33 @@ export class Task extends EventEmitter implements TaskLike { } } + /** + * Silent companion to `maybeWaitForProviderRateLimit` for background provider + * requests (e.g. title generation): waits out the remaining provider + * rate-limit window without emitting the task-loop countdown UX, since a + * second concurrent partial `api_req_rate_limit_wait` say would interleave + * with the task's own countdown. Resolves early (not rejects) when the given + * signal aborts; callers must still re-check abort state after the wait and + * call `rateLimitClock.recordRequest()` right before issuing the request, + * mirroring the task loop. + */ + private async waitForProviderRateLimitWindow(signal: AbortSignal): Promise { + const rateLimitSeconds = this.apiConfiguration?.rateLimitSeconds ?? 0 + const lastRequestTime = this.rateLimitClock.getLastRequestTime() + if (rateLimitSeconds <= 0 || !lastRequestTime) { + return + } + + const remainingMs = rateLimitSeconds * 1000 - (performance.now() - lastRequestTime) + if (remainingMs <= 0) { + return + } + + await delay(remainingMs, { signal }).catch(() => { + // Aborted mid-wait: cancellation, not a failure. + }) + } + public async *attemptApiRequest( retryAttempt: number = 0, options: { skipProviderRateLimit?: boolean; requestModelInfo?: ModelInfo } = {}, diff --git a/src/core/task/__tests__/Task.titleGeneration.spec.ts b/src/core/task/__tests__/Task.titleGeneration.spec.ts index 0b1c4321e5..87420ddb17 100644 --- a/src/core/task/__tests__/Task.titleGeneration.spec.ts +++ b/src/core/task/__tests__/Task.titleGeneration.spec.ts @@ -4,6 +4,7 @@ import { providerIdentifiers } from "@roo-code/types" import { Task } from "../Task" import * as taskTitleGen from "../taskTitleGenerator" +import { createRateLimitClock, type RateLimitClock } from "../RateLimitClock" import { setAbortLatch } from "../../../test-utils/run-state" // Keep the real constants and the fallback helper; only the LLM call is mocked @@ -16,6 +17,15 @@ vi.mock("../taskTitleGenerator", async (importOriginal) => { } }) +// Mock delay so the title request's rate-limit wait can be observed without +// real timers (same pattern as the rate-limit tests in Task.spec). +vi.mock("delay", () => ({ + __esModule: true, + default: vi.fn().mockResolvedValue(undefined), +})) + +import delay from "delay" + type TaskTitleAccess = { generateTitleInBackground(taskText: string): Promise startTask(task?: string, images?: string[]): Promise @@ -55,7 +65,13 @@ describe("Task title generation", () => { } }) - function createTestTask(options: { parentTask?: Task } = {}) { + function createTestTask( + options: { + parentTask?: Task + rateLimitClock?: RateLimitClock + apiConfiguration?: ProviderSettings + } = {}, + ) { const task = Object.create(Task.prototype) as Task setAbortLatch(task, false) const taskId = "task-title-1" @@ -63,10 +79,11 @@ describe("Task title generation", () => { Object.assign(task, { taskId, instanceId: "inst-1", - apiConfiguration: mockApiConfig, + apiConfiguration: options.apiConfiguration ?? mockApiConfig, providerRef: { deref: () => mockProvider }, abandoned: false, parentTask: options.parentTask, + rateLimitClock: options.rateLimitClock ?? createRateLimitClock(), metadata: { task: "Initial prompt message" }, say: vi.fn().mockResolvedValue(undefined), getEnabledMcpToolsCount: vi.fn().mockResolvedValue({ enabledToolCount: 0, enabledServerCount: 0 }), @@ -191,6 +208,93 @@ describe("Task title generation", () => { }) }) + describe("generateTitleInBackground rate-limit coordination", () => { + it("waits out the remaining provider rate-limit window before the title request", async () => { + const clock = createRateLimitClock() + const task = createTestTask({ + rateLimitClock: clock, + apiConfiguration: { ...mockApiConfig, rateLimitSeconds: 10 }, + }) + vi.mocked(taskTitleGen.generateTaskTitle).mockResolvedValueOnce("Summarized Title") + const mockDelay = vi.mocked(delay) + // Record the wait options while the controller still exists; it is + // cleaned up in the method's `finally` block. + let observedSignal: unknown + mockDelay.mockImplementationOnce((_ms: number, opts?: { signal?: AbortSignal }) => { + observedSignal = opts?.signal + return Promise.resolve(undefined) + }) + + // A prior request (e.g. the previous task's) leaves a timestamp on the + // shared clock, so the title request must not bypass the window. + clock.recordRequest() + + await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") + + expect(mockDelay).toHaveBeenCalledTimes(1) + const waitedMs = mockDelay.mock.calls[0]?.[0] + expect(waitedMs).toBeGreaterThan(0) + expect(waitedMs).toBeLessThanOrEqual(10_000) + // The wait is abort-aware so cancellation can interrupt it. + expect(observedSignal).toBeInstanceOf(AbortSignal) + // The wait happens before the provider request is issued. + expect(mockDelay.mock.invocationCallOrder[0]).toBeLessThan( + vi.mocked(taskTitleGen.generateTaskTitle).mock.invocationCallOrder[0], + ) + }) + + it("records the title request on the shared rate-limit clock", async () => { + const clock = createRateLimitClock() + const task = createTestTask({ rateLimitClock: clock }) + vi.mocked(taskTitleGen.generateTaskTitle).mockResolvedValueOnce("Summarized Title") + + expect(clock.getLastRequestTime()).toBeUndefined() + + await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") + + // Subsequent task/subtask requests see this timestamp and honour it. + expect(clock.getLastRequestTime()).toBeDefined() + }) + + it("does not wait when no rate limit is configured or the clock is empty", async () => { + const task = createTestTask({ apiConfiguration: { ...mockApiConfig, rateLimitSeconds: 10 } }) + vi.mocked(taskTitleGen.generateTaskTitle).mockResolvedValueOnce("Summarized Title") + const mockDelay = vi.mocked(delay) + + // rateLimitSeconds is set, but the shared clock has no recorded + // request yet — nothing to wait for. + await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") + + expect(mockDelay).not.toHaveBeenCalled() + expect(taskTitleGen.generateTaskTitle).toHaveBeenCalledTimes(1) + }) + + it("skips the title request when the task is aborted during the rate-limit wait", async () => { + const clock = createRateLimitClock() + const task = createTestTask({ + rateLimitClock: clock, + apiConfiguration: { ...mockApiConfig, rateLimitSeconds: 10 }, + }) + const access = getTaskTitleAccess(task) + clock.recordRequest() + const timestampBeforeWait = clock.getLastRequestTime() + // Simulate the wait being interrupted by cancellation: the delay + // promise rejects like `delay` does on an aborted signal. + vi.mocked(delay).mockImplementationOnce(() => { + access.titleAbortController?.abort() + return Promise.reject(new Error("The operation was aborted")) + }) + + await access.generateTitleInBackground("Initial prompt message") + + // No provider request, no phantom clock timestamp that would delay + // the next request. + expect(taskTitleGen.generateTaskTitle).not.toHaveBeenCalled() + expect(clock.getLastRequestTime()).toBe(timestampBeforeWait) + expect(mockSetGeneratedTaskTitle).not.toHaveBeenCalled() + }) + }) + describe("cancellation and abort handling", () => { it("aborts titleAbortController in cancelCurrentRequest", () => { const task = createTestTask() From 592f4b2ca55b95fc3324e7b0b2903ef7d4fa2175 Mon Sep 17 00:00:00 2001 From: daewoongoh Date: Mon, 9 Nov 2026 13:12:03 +0900 Subject: [PATCH 4/8] fix: preserve manual title rename over generated title across hosts Decide the generated-title write from the persisted record inside the per-file lock instead of the store cache, so a rename committed by another provider's store is never overwritten. --- src/core/task-persistence/TaskHistoryStore.ts | 72 +++++++++++++++++++ .../TaskHistoryStore.realConcurrency.spec.ts | 61 ++++++++++++++++ src/core/webview/ClineProvider.ts | 22 ++---- 3 files changed, 140 insertions(+), 15 deletions(-) diff --git a/src/core/task-persistence/TaskHistoryStore.ts b/src/core/task-persistence/TaskHistoryStore.ts index c1c50cca1e..3059f6fc01 100644 --- a/src/core/task-persistence/TaskHistoryStore.ts +++ b/src/core/task-persistence/TaskHistoryStore.ts @@ -1186,6 +1186,78 @@ export class TaskHistoryStore { }) } + /** + * Disk-authoritative conditional write for a generated task title. + * + * The decision reads the persisted record inside the per-file advisory + * lock's merge callback, so a manual rename committed by another store + * (another extension host) after this store's cache was last refreshed is + * preserved instead of overwritten: every provider owns a separate + * TaskHistoryStore with a separate in-process lock and cache, so a + * cache-only existing-title check is not sufficient. An existing disk + * title — a manual rename or an earlier generated title — always wins, + * and the record's other disk fields are preserved when the generated + * title is applied. + * + * @returns The authoritative disk record and whether the generated title + * was applied. + * @throws If the task ID is not present in the cache, so the caller can retry. + */ + public async setGeneratedTitleIfUnset( + taskId: string, + title: string, + ): Promise<{ item: HistoryItem; applied: boolean }> { + return this.withLock(async () => { + const cached = this.cache.get(taskId) + if (!cached) { + throw new Error(`[TaskHistoryStore] setGeneratedTitleIfUnset: task ${taskId} not found in cache`) + } + const trimmed = title.trim() + if (!trimmed) { + return { item: cached, applied: false } + } + + const filePath = await this.getTaskFilePath(taskId) + const incoming: HistoryItem = { ...cached, title: trimmed } + // Seed the decision from the cache, then let the locked disk record + // override it inside the merge callback. When the per-file lock + // cannot hand back the disk record (a safeWriteJson double that + // never invokes `merge`), the cache decision stands. + let applied = !cached.title + let authoritative: HistoryItem = cached.title ? cached : incoming + await safeWriteJson(filePath, incoming, { + merge: (existing) => { + if (existing && typeof existing === "object" && "id" in existing) { + const disk = existing as HistoryItem + if (disk.title) { + // A manual rename or an earlier generated title + // from any host is already persisted; keep it. + applied = false + authoritative = disk + return disk + } + // Apply only the title onto the locked disk record so + // fields written by another host are preserved. + applied = true + authoritative = { ...disk, title: trimmed } + return authoritative + } + // No readable disk record; fall back to writing the + // cached item with the title applied, matching the + // delta-write fallback for a missing file. + applied = true + authoritative = incoming + return incoming + }, + }) + this.cache.set(taskId, authoritative) + if (this.onWrite) { + await this.onWrite(this.getAll()) + } + return { item: authoritative, applied } + }) + } + // ────────────────────────────── Private: Write lock ────────────────────────────── /** diff --git a/src/core/task-persistence/__tests__/TaskHistoryStore.realConcurrency.spec.ts b/src/core/task-persistence/__tests__/TaskHistoryStore.realConcurrency.spec.ts index e3e4445f35..1e2960da9a 100644 --- a/src/core/task-persistence/__tests__/TaskHistoryStore.realConcurrency.spec.ts +++ b/src/core/task-persistence/__tests__/TaskHistoryStore.realConcurrency.spec.ts @@ -421,4 +421,65 @@ describe("TaskHistoryStore real cross-host locking", () => { await fs.rm(storagePath, { recursive: true, force: true }) } }) + + it("preserves a manual rename committed by another store over a stale-cache generated title", async () => { + const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "task-history-generated-title-")) + const storeA = new TaskHistoryStore(storagePath) + const storeB = new TaskHistoryStore(storagePath) + const filePath = path.join(storagePath, "tasks", "shared-task", "history_item.json") + + try { + await storeA.initialize() + await storeA.upsert(item("shared-task")) + // storeB hydrates its cache before the rename, so its cached copy + // never sees the manual title. + await storeB.initialize() + + // The manual rename commits to disk through store A before store + // B attempts the generated-title write. + await storeA.atomicReadAndUpdate("shared-task", (current) => ({ ...current, title: "Manual Title" })) + + const result = await storeB.setGeneratedTitleIfUnset("shared-task", "Generated Title") + + expect(result.applied).toBe(false) + expect(result.item.title).toBe("Manual Title") + // The stale cache is refreshed with the authoritative disk record. + expect(storeB.get("shared-task")?.title).toBe("Manual Title") + expect(JSON.parse(await fs.readFile(filePath, "utf8"))).toMatchObject({ title: "Manual Title" }) + } finally { + storeA.dispose() + storeB.dispose() + await fs.rm(storagePath, { recursive: true, force: true }) + } + }) + + it("applies the generated title onto the locked disk record when no disk title exists", async () => { + const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "task-history-generated-title-apply-")) + const storeA = new TaskHistoryStore(storagePath) + const storeB = new TaskHistoryStore(storagePath) + const filePath = path.join(storagePath, "tasks", "shared-task", "history_item.json") + + try { + await storeA.initialize() + await storeA.upsert(item("shared-task")) + await storeB.initialize() + + // Another host persists an unrelated field; storeB's cache stays + // stale, so the apply must read the locked disk record. + await storeA.atomicReadAndUpdate("shared-task", (current) => ({ ...current, totalCost: 42 })) + + const result = await storeB.setGeneratedTitleIfUnset("shared-task", "Generated Title") + + expect(result.applied).toBe(true) + expect(result.item).toMatchObject({ title: "Generated Title", totalCost: 42 }) + expect(JSON.parse(await fs.readFile(filePath, "utf8"))).toMatchObject({ + title: "Generated Title", + totalCost: 42, + }) + } finally { + storeA.dispose() + storeB.dispose() + await fs.rm(storagePath, { recursive: true, force: true }) + } + }) }) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index df86ac20a1..21ba6b1c4c 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -2833,11 +2833,12 @@ export class ClineProvider /* * Applies an automatically generated title for a history item (issue #2333). - * The existing-title check and the write happen inside one atomic store - * update, so a manual rename queued ahead of this call is never overwritten - * by the generated title. Returns true when the title was applied, false - * when the item already had a title. Throws when the task is not in the - * store yet so the caller can retry. + * The existing-title check reads the persisted record inside the same + * per-file lock as the write, so a manual rename committed by another + * provider's store after this store's cache was last refreshed is never + * overwritten by the generated title. Returns true when the title was + * applied, false when the item already had a title. Throws when the task + * is not in the store yet so the caller can retry. */ async setGeneratedTaskTitle(id: string, title: string): Promise { const trimmed = title.trim() @@ -2845,16 +2846,7 @@ export class ClineProvider return false } - let applied = false - await this.taskHistoryStore.atomicReadAndUpdate(id, (historyItem) => { - // Preserve any title set before this write: a manual rename, or an - // earlier generated title, always wins over a later generated one. - if (historyItem.title) { - return historyItem - } - applied = true - return { ...historyItem, title: trimmed } - }) + const { applied } = await this.taskHistoryStore.setGeneratedTitleIfUnset(id, trimmed) if (applied) { this.recentTasksCache = undefined From 83936a54d51d9e71ea8b5348170bae492d1c2df0 Mon Sep 17 00:00:00 2001 From: daewoongoh Date: Mon, 9 Nov 2026 14:25:34 +0900 Subject: [PATCH 5/8] fix: address review feedback on background task title generation - Fail the generated-title write closed when the locked disk record is missing, malformed, or belongs to another task, and drop the stale cache entry instead of recreating a task deleted by another host - Propagate cancellation into the queued store write, skip the fallback write after cancellation, and cancel pending generation on rename - Settle title generation on timeout or parent cancellation even when the provider ignores the abort signal - Use the saved TASK_TITLE support prompt and add its translations - Publish the authoritative history item when the generated title is not applied - Re-check the shared rate-limit clock after each title wait so the title request stays spaced from a task request dispatched meanwhile --- src/core/task-persistence/TaskHistoryStore.ts | 55 ++++-- .../TaskHistoryStore.realConcurrency.spec.ts | 82 ++++++++- src/core/task/Task.ts | 63 +++++-- .../__tests__/Task.titleGeneration.spec.ts | 174 +++++++++++++++++- .../task/__tests__/taskTitleGenerator.spec.ts | 44 ++++- src/core/task/taskTitleGenerator.ts | 27 ++- src/core/webview/ClineProvider.ts | 25 ++- .../ClineProvider.taskHistory.spec.ts | 58 +++++- webview-ui/src/i18n/locales/ca/prompts.json | 4 + webview-ui/src/i18n/locales/de/prompts.json | 4 + webview-ui/src/i18n/locales/en/prompts.json | 4 + webview-ui/src/i18n/locales/es/prompts.json | 4 + webview-ui/src/i18n/locales/fr/prompts.json | 4 + webview-ui/src/i18n/locales/hi/prompts.json | 4 + webview-ui/src/i18n/locales/id/prompts.json | 4 + webview-ui/src/i18n/locales/it/prompts.json | 4 + webview-ui/src/i18n/locales/ja/prompts.json | 4 + webview-ui/src/i18n/locales/ko/prompts.json | 4 + webview-ui/src/i18n/locales/nl/prompts.json | 4 + webview-ui/src/i18n/locales/pl/prompts.json | 4 + .../src/i18n/locales/pt-BR/prompts.json | 4 + webview-ui/src/i18n/locales/ru/prompts.json | 4 + webview-ui/src/i18n/locales/tr/prompts.json | 4 + webview-ui/src/i18n/locales/vi/prompts.json | 4 + .../src/i18n/locales/zh-CN/prompts.json | 4 + .../src/i18n/locales/zh-TW/prompts.json | 4 + 26 files changed, 540 insertions(+), 60 deletions(-) diff --git a/src/core/task-persistence/TaskHistoryStore.ts b/src/core/task-persistence/TaskHistoryStore.ts index 3059f6fc01..f399d0cd7e 100644 --- a/src/core/task-persistence/TaskHistoryStore.ts +++ b/src/core/task-persistence/TaskHistoryStore.ts @@ -1199,21 +1199,28 @@ export class TaskHistoryStore { * and the record's other disk fields are preserved when the generated * title is applied. * - * @returns The authoritative disk record and whether the generated title - * was applied. + * Deletion by another host is authoritative: when no persisted record + * exists, or the persisted record is malformed or carries a different task + * ID, nothing is written, the stale cache entry is dropped, and the result + * has no item. A signal aborted while the call waits on the store lock + * skips the write, so a cancelled task's title is never persisted. + * + * @returns The authoritative record (absent when the write was rejected) + * and whether the generated title was applied. * @throws If the task ID is not present in the cache, so the caller can retry. */ public async setGeneratedTitleIfUnset( taskId: string, title: string, - ): Promise<{ item: HistoryItem; applied: boolean }> { + options: { signal?: AbortSignal } = {}, + ): Promise<{ item: HistoryItem | undefined; applied: boolean }> { return this.withLock(async () => { const cached = this.cache.get(taskId) if (!cached) { throw new Error(`[TaskHistoryStore] setGeneratedTitleIfUnset: task ${taskId} not found in cache`) } const trimmed = title.trim() - if (!trimmed) { + if (!trimmed || options.signal?.aborted) { return { item: cached, applied: false } } @@ -1225,9 +1232,21 @@ export class TaskHistoryStore { // never invokes `merge`), the cache decision stands. let applied = !cached.title let authoritative: HistoryItem = cached.title ? cached : incoming - await safeWriteJson(filePath, incoming, { - merge: (existing) => { - if (existing && typeof existing === "object" && "id" in existing) { + let rejectedDiskRecord = false + try { + await safeWriteJson(filePath, incoming, { + merge: (existing) => { + // Writing the cached item back would recreate a task another + // host deleted or overwrite a record that is not this task's, + // so a missing, malformed, or mismatched record fails closed. + // A throwing merge writes nothing. + const parsed = historyItemSchema.safeParse(existing) + if (!parsed.success || parsed.data.id !== taskId) { + rejectedDiskRecord = true + throw new Error( + `[TaskHistoryStore] setGeneratedTitleIfUnset: task ${taskId} has no valid disk record`, + ) + } const disk = existing as HistoryItem if (disk.title) { // A manual rename or an earlier generated title @@ -1241,15 +1260,19 @@ export class TaskHistoryStore { applied = true authoritative = { ...disk, title: trimmed } return authoritative - } - // No readable disk record; fall back to writing the - // cached item with the title applied, matching the - // delta-write fallback for a missing file. - applied = true - authoritative = incoming - return incoming - }, - }) + }, + }) + } catch (error) { + if (!rejectedDiskRecord) { + throw error + } + this.cache.delete(taskId) + this.taskFileMtimes.delete(taskId) + if (this.onWrite) { + await this.onWrite(this.getAll()) + } + return { item: undefined, applied: false } + } this.cache.set(taskId, authoritative) if (this.onWrite) { await this.onWrite(this.getAll()) diff --git a/src/core/task-persistence/__tests__/TaskHistoryStore.realConcurrency.spec.ts b/src/core/task-persistence/__tests__/TaskHistoryStore.realConcurrency.spec.ts index 1e2960da9a..d2c8227b73 100644 --- a/src/core/task-persistence/__tests__/TaskHistoryStore.realConcurrency.spec.ts +++ b/src/core/task-persistence/__tests__/TaskHistoryStore.realConcurrency.spec.ts @@ -442,7 +442,7 @@ describe("TaskHistoryStore real cross-host locking", () => { const result = await storeB.setGeneratedTitleIfUnset("shared-task", "Generated Title") expect(result.applied).toBe(false) - expect(result.item.title).toBe("Manual Title") + expect(result.item?.title).toBe("Manual Title") // The stale cache is refreshed with the authoritative disk record. expect(storeB.get("shared-task")?.title).toBe("Manual Title") expect(JSON.parse(await fs.readFile(filePath, "utf8"))).toMatchObject({ title: "Manual Title" }) @@ -482,4 +482,84 @@ describe("TaskHistoryStore real cross-host locking", () => { await fs.rm(storagePath, { recursive: true, force: true }) } }) + + describe("setGeneratedTitleIfUnset title input and cancellation", () => { + async function withSingleStore( + run: (store: TaskHistoryStore, filePath: string, onWrite: ReturnType) => Promise, + ) { + const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "task-history-generated-title-input-")) + const onWrite = vi.fn(async () => {}) + const store = new TaskHistoryStore(storagePath, { onWrite }) + const filePath = path.join(storagePath, "tasks", "solo-task", "history_item.json") + try { + await store.initialize() + await store.upsert(item("solo-task")) + onWrite.mockClear() + await run(store, filePath, onWrite) + } finally { + store.dispose() + await fs.rm(storagePath, { recursive: true, force: true }) + } + } + + it("persists a padded title trimmed", async () => { + await withSingleStore(async (store, filePath) => { + const result = await store.setGeneratedTitleIfUnset("solo-task", " Padded Title ") + + expect(result).toMatchObject({ applied: true, item: { title: "Padded Title" } }) + expect(JSON.parse(await fs.readFile(filePath, "utf8"))).toMatchObject({ title: "Padded Title" }) + }) + }) + + it.each([ + ["a blank title", " ", undefined], + ["an aborted signal", "Generated Title", AbortSignal.abort()], + ])("does not write or notify for %s", async (_label, title, signal) => { + await withSingleStore(async (store, filePath, onWrite) => { + const before = await fs.readFile(filePath, "utf8") + + const result = await store.setGeneratedTitleIfUnset("solo-task", title, { signal }) + + expect(result.applied).toBe(false) + expect(await fs.readFile(filePath, "utf8")).toBe(before) + expect(onWrite).not.toHaveBeenCalled() + expect(store.get("solo-task")?.title).toBeUndefined() + }) + }) + }) + + describe("setGeneratedTitleIfUnset with a rejected disk record", () => { + it.each([ + ["deleted by another host", async (filePath: string) => fs.rm(filePath)], + ["unreadable", async (filePath: string) => fs.writeFile(filePath, "{not json")], + [ + "persisted under a different task ID", + async (filePath: string) => fs.writeFile(filePath, JSON.stringify(item("other-task"))), + ], + ])("fails closed without writing when the disk record is %s", async (_label, mutateDisk) => { + const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "task-history-generated-title-reject-")) + const onWrite = vi.fn(async () => {}) + const store = new TaskHistoryStore(storagePath, { onWrite }) + const filePath = path.join(storagePath, "tasks", "shared-task", "history_item.json") + + try { + await store.initialize() + await store.upsert(item("shared-task")) + // Another host changes the disk record after this store cached it. + await mutateDisk(filePath) + const before = await fs.readFile(filePath, "utf8").catch(() => undefined) + onWrite.mockClear() + + const result = await store.setGeneratedTitleIfUnset("shared-task", "Generated Title") + + expect(result).toEqual({ item: undefined, applied: false }) + expect(await fs.readFile(filePath, "utf8").catch(() => undefined)).toBe(before) + expect(store.get("shared-task")).toBeUndefined() + expect(onWrite).toHaveBeenCalledTimes(1) + } finally { + store.dispose() + await fs.rm(storagePath, { recursive: true, force: true }) + } + }) + }) }) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index d964fb19b6..dc497b1a57 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -3237,10 +3237,15 @@ export class Task extends EventEmitter implements TaskLike { this.rateLimitClock.recordRequest() + // A saved TASK_TITLE support prompt replaces the built-in template; + // an unreadable state falls back to the built-in template. + const state = await provider.getState().catch(() => undefined) + const generated = await generateTaskTitle(taskText, this.apiConfiguration, { abortSignal: controller.signal, timeoutMs: DEFAULT_TITLE_TIMEOUT_MS, taskId: this.taskId, + customSupportPrompts: state?.customSupportPrompts, }) if (controller.signal.aborted || this.abort || this.abandoned) { @@ -3262,7 +3267,7 @@ export class Task extends EventEmitter implements TaskLike { } try { - await provider.setGeneratedTaskTitle(this.taskId, desiredTitle) + await provider.setGeneratedTaskTitle(this.taskId, desiredTitle, controller.signal) return } catch (error) { console.debug( @@ -3271,10 +3276,15 @@ export class Task extends EventEmitter implements TaskLike { ) if (attempt === Task.TITLE_WRITE_ATTEMPTS) { + // The task may have been cancelled while the last write + // attempt was pending; a cancelled task gets no title. + if (controller.signal.aborted || this.abort || this.abandoned) { + return + } // Retries exhausted: final attempt with the truncated-prompt // fallback title (issue #2333). try { - await provider.setGeneratedTaskTitle(this.taskId, fallback) + await provider.setGeneratedTaskTitle(this.taskId, fallback, controller.signal) } catch (fallbackError) { console.error( `[Task#generateTitleInBackground] Fallback title failed for ${this.taskId}:`, @@ -3632,6 +3642,17 @@ export class Task extends EventEmitter implements TaskLike { ]) } + /** + * Stops background title generation so a pending generated or fallback + * title is never written, e.g. after the user renames or clears the title. + */ + public cancelTitleGeneration(): void { + if (this.titleAbortController) { + this.titleAbortController.abort() + this.titleAbortController = undefined + } + } + /** * Cancels the current HTTP request if one is in progress. * This immediately aborts the underlying stream rather than waiting for the next chunk. @@ -3648,10 +3669,7 @@ export class Task extends EventEmitter implements TaskLike { this.metadataFetchAbortController.abort() this.metadataFetchAbortController = undefined } - if (this.titleAbortController) { - this.titleAbortController.abort() - this.titleAbortController = undefined - } + this.cancelTitleGeneration() } /** @@ -3713,10 +3731,7 @@ export class Task extends EventEmitter implements TaskLike { if (isAbandoned) { this.abandoned = true } - if (this.titleAbortController) { - this.titleAbortController.abort() - this.titleAbortController = undefined - } + this.cancelTitleGeneration() this.driveRunEvent({ tag: "abortRequested", abandoned: isAbandoned }) } @@ -5815,22 +5830,32 @@ export class Task extends EventEmitter implements TaskLike { * signal aborts; callers must still re-check abort state after the wait and * call `rateLimitClock.recordRequest()` right before issuing the request, * mirroring the task loop. + * + * A task request can dispatch on the shared clock while this one waits, so + * the latest timestamp is re-read after every wait: the request is spaced + * from the most recent dispatch, not from the timestamp it first read. */ private async waitForProviderRateLimitWindow(signal: AbortSignal): Promise { const rateLimitSeconds = this.apiConfiguration?.rateLimitSeconds ?? 0 - const lastRequestTime = this.rateLimitClock.getLastRequestTime() - if (rateLimitSeconds <= 0 || !lastRequestTime) { + if (rateLimitSeconds <= 0) { return } - const remainingMs = rateLimitSeconds * 1000 - (performance.now() - lastRequestTime) - if (remainingMs <= 0) { - return - } + while (!signal.aborted) { + const lastRequestTime = this.rateLimitClock.getLastRequestTime() + if (!lastRequestTime) { + return + } - await delay(remainingMs, { signal }).catch(() => { - // Aborted mid-wait: cancellation, not a failure. - }) + const remainingMs = rateLimitSeconds * 1000 - (performance.now() - lastRequestTime) + if (remainingMs <= 0) { + return + } + + await delay(remainingMs, { signal }).catch(() => { + // Aborted mid-wait: cancellation, not a failure. + }) + } } public async *attemptApiRequest( diff --git a/src/core/task/__tests__/Task.titleGeneration.spec.ts b/src/core/task/__tests__/Task.titleGeneration.spec.ts index 87420ddb17..5c07a4cbee 100644 --- a/src/core/task/__tests__/Task.titleGeneration.spec.ts +++ b/src/core/task/__tests__/Task.titleGeneration.spec.ts @@ -1,6 +1,7 @@ -import { describe, it, expect, vi, beforeEach } from "vitest" +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" import type { ProviderSettings } from "@roo-code/types" import { providerIdentifiers } from "@roo-code/types" +import * as vscode from "vscode" import { Task } from "../Task" import * as taskTitleGen from "../taskTitleGenerator" @@ -31,6 +32,7 @@ type TaskTitleAccess = { startTask(task?: string, images?: string[]): Promise cancelCurrentRequest(): void requestAbort(isAbandoned: boolean): void + cancelTitleGeneration(): void titleAbortController?: AbortController abort: boolean abandoned: boolean @@ -54,6 +56,8 @@ describe("Task title generation", () => { let mockProvider: { setGeneratedTaskTitle: ReturnType postStateToWebviewWithoutTaskHistory: ReturnType + getState: ReturnType + context: { extensionMode: vscode.ExtensionMode } } beforeEach(() => { @@ -62,6 +66,8 @@ describe("Task title generation", () => { mockProvider = { setGeneratedTaskTitle: mockSetGeneratedTaskTitle, postStateToWebviewWithoutTaskHistory: vi.fn().mockResolvedValue(undefined), + getState: vi.fn().mockResolvedValue({}), + context: { extensionMode: vscode.ExtensionMode.Development }, } }) @@ -113,7 +119,11 @@ describe("Task title generation", () => { // Issue #2333: the model request is limited to 15 seconds. expect(taskTitleGen.DEFAULT_TITLE_TIMEOUT_MS).toBe(15000) expect(mockSetGeneratedTaskTitle).toHaveBeenCalledTimes(1) - expect(mockSetGeneratedTaskTitle).toHaveBeenCalledWith("task-title-1", "Summarized Title") + expect(mockSetGeneratedTaskTitle).toHaveBeenCalledWith( + "task-title-1", + "Summarized Title", + expect.any(AbortSignal), + ) }) it("falls back to the first 42 characters of the prompt when generation fails (issue #2333)", async () => { @@ -126,7 +136,11 @@ describe("Task title generation", () => { const expectedFallback = taskTitleGen.fallbackTaskTitle(longPrompt) expect(expectedFallback).toHaveLength(42) expect(mockSetGeneratedTaskTitle).toHaveBeenCalledTimes(1) - expect(mockSetGeneratedTaskTitle).toHaveBeenCalledWith("task-title-1", expectedFallback) + expect(mockSetGeneratedTaskTitle).toHaveBeenCalledWith( + "task-title-1", + expectedFallback, + expect.any(AbortSignal), + ) }) it("keeps an existing title instead of overwriting it (manual rename wins)", async () => { @@ -150,7 +164,11 @@ describe("Task title generation", () => { await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") expect(mockSetGeneratedTaskTitle).toHaveBeenCalledTimes(2) - expect(mockSetGeneratedTaskTitle).toHaveBeenLastCalledWith("task-title-1", "Summarized Title") + expect(mockSetGeneratedTaskTitle).toHaveBeenLastCalledWith( + "task-title-1", + "Summarized Title", + expect.any(AbortSignal), + ) }) it("applies the truncated-prompt fallback title after the write retries are exhausted (issue #2333)", async () => { @@ -166,11 +184,22 @@ describe("Task title generation", () => { // Three attempts with the generated title, then one with the fallback. expect(mockSetGeneratedTaskTitle).toHaveBeenCalledTimes(4) - expect(mockSetGeneratedTaskTitle).toHaveBeenNthCalledWith(1, "task-title-1", "Summarized Title") - expect(mockSetGeneratedTaskTitle).toHaveBeenNthCalledWith(3, "task-title-1", "Summarized Title") + expect(mockSetGeneratedTaskTitle).toHaveBeenNthCalledWith( + 1, + "task-title-1", + "Summarized Title", + expect.any(AbortSignal), + ) + expect(mockSetGeneratedTaskTitle).toHaveBeenNthCalledWith( + 3, + "task-title-1", + "Summarized Title", + expect.any(AbortSignal), + ) expect(mockSetGeneratedTaskTitle).toHaveBeenLastCalledWith( "task-title-1", taskTitleGen.fallbackTaskTitle("Initial prompt message"), + expect.any(AbortSignal), ) }) @@ -198,6 +227,80 @@ describe("Task title generation", () => { expect(mockSetGeneratedTaskTitle).not.toHaveBeenCalled() }) + it("skips title generation and the title write in ExtensionMode.Test", async () => { + mockProvider.context.extensionMode = vscode.ExtensionMode.Test + const task = createTestTask() + + await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") + + expect(taskTitleGen.generateTaskTitle).not.toHaveBeenCalled() + expect(mockSetGeneratedTaskTitle).not.toHaveBeenCalled() + }) + + it.each([ + ["a saved TASK_TITLE prompt", { customSupportPrompts: { TASK_TITLE: "Custom ${userInput}" } }], + ["an unreadable state", undefined], + ])("passes the custom support prompts from %s", async (_label, state) => { + if (state) { + mockProvider.getState.mockResolvedValueOnce(state) + } else { + mockProvider.getState.mockRejectedValueOnce(new Error("state unavailable")) + } + vi.mocked(taskTitleGen.generateTaskTitle).mockResolvedValueOnce("Summarized Title") + const task = createTestTask() + + await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") + + expect(taskTitleGen.generateTaskTitle).toHaveBeenCalledWith( + "Initial prompt message", + mockApiConfig, + expect.objectContaining({ customSupportPrompts: state?.customSupportPrompts }), + ) + expect(mockSetGeneratedTaskTitle).toHaveBeenCalledTimes(1) + }) + + it("does not write the fallback title when cancelled during the third failed write", async () => { + vi.mocked(taskTitleGen.generateTaskTitle).mockResolvedValueOnce("Summarized Title") + const task = createTestTask() + const access = getTaskTitleAccess(task) + mockSetGeneratedTaskTitle + .mockRejectedValueOnce(new Error("task not found in cache")) + .mockRejectedValueOnce(new Error("task not found in cache")) + .mockImplementationOnce(async () => { + access.cancelTitleGeneration() + throw new Error("task not found in cache") + }) + + await access.generateTitleInBackground("Initial prompt message") + + // Three attempts with the generated title and no fallback write. + expect(mockSetGeneratedTaskTitle).toHaveBeenCalledTimes(3) + expect(mockSetGeneratedTaskTitle).not.toHaveBeenCalledWith( + "task-title-1", + taskTitleGen.fallbackTaskTitle("Initial prompt message"), + expect.anything(), + ) + }) + + it("passes a signal to the title write that cancellation aborts", async () => { + vi.mocked(taskTitleGen.generateTaskTitle).mockResolvedValueOnce("Summarized Title") + const task = createTestTask() + const access = getTaskTitleAccess(task) + let writeSignal: AbortSignal | undefined + mockSetGeneratedTaskTitle.mockImplementationOnce( + async (_id: string, _title: string, signal: AbortSignal) => { + writeSignal = signal + access.cancelTitleGeneration() + return false + }, + ) + + await access.generateTitleInBackground("Initial prompt message") + + // The store re-checks this signal once the queued write takes the lock. + expect(writeSignal?.aborted).toBe(true) + }) + it("cleans up titleAbortController when finished", async () => { vi.mocked(taskTitleGen.generateTaskTitle).mockResolvedValueOnce("Summarized Title") const task = createTestTask() @@ -209,6 +312,24 @@ describe("Task title generation", () => { }) describe("generateTitleInBackground rate-limit coordination", () => { + // The rate-limit wait re-reads the shared clock after each wait, so the + // mocked delay advances a controlled `performance.now()` like a real wait. + let now: number + + beforeEach(() => { + now = 1_000_000 + vi.spyOn(performance, "now").mockImplementation(() => now) + vi.mocked(delay).mockImplementation((ms: number) => { + now += ms + return Promise.resolve(undefined) + }) + }) + + afterEach(() => { + vi.mocked(performance.now).mockRestore() + vi.mocked(delay).mockReset().mockResolvedValue(undefined) + }) + it("waits out the remaining provider rate-limit window before the title request", async () => { const clock = createRateLimitClock() const task = createTestTask({ @@ -220,8 +341,9 @@ describe("Task title generation", () => { // Record the wait options while the controller still exists; it is // cleaned up in the method's `finally` block. let observedSignal: unknown - mockDelay.mockImplementationOnce((_ms: number, opts?: { signal?: AbortSignal }) => { + mockDelay.mockImplementationOnce((ms: number, opts?: { signal?: AbortSignal }) => { observedSignal = opts?.signal + now += ms return Promise.resolve(undefined) }) @@ -293,6 +415,38 @@ describe("Task title generation", () => { expect(clock.getLastRequestTime()).toBe(timestampBeforeWait) expect(mockSetGeneratedTaskTitle).not.toHaveBeenCalled() }) + + it("re-waits when a task request dispatches during the title request's wait", async () => { + const clock = createRateLimitClock() + const task = createTestTask({ + rateLimitClock: clock, + apiConfiguration: { ...mockApiConfig, rateLimitSeconds: 10 }, + }) + let taskDispatchTime: number | undefined + let titleDispatchTime: number | undefined + vi.mocked(taskTitleGen.generateTaskTitle).mockImplementationOnce(async () => { + titleDispatchTime = now + return "Summarized Title" + }) + // When the title's wait ends, the task loop's wait for the same + // window has also ended and it dispatches first on the shared clock. + vi.mocked(delay).mockImplementationOnce((ms: number) => { + now += ms + clock.recordRequest() + taskDispatchTime = now + return Promise.resolve(undefined) + }) + clock.recordRequest() + + await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") + + // The title request is spaced a full window from the task dispatch + // instead of proceeding from the stale timestamp it first read. + expect(delay).toHaveBeenCalledTimes(2) + expect(titleDispatchTime).toBeDefined() + expect(taskDispatchTime).toBeDefined() + expect((titleDispatchTime ?? 0) - (taskDispatchTime ?? 0)).toBeGreaterThanOrEqual(10_000) + }) }) describe("cancellation and abort handling", () => { @@ -376,7 +530,11 @@ describe("Task title generation", () => { resolveGeneration("Summarized Title") await started await vi.waitFor(() => expect(mockSetGeneratedTaskTitle).toHaveBeenCalled()) - expect(mockSetGeneratedTaskTitle).toHaveBeenCalledWith("task-title-1", "Summarized Title") + expect(mockSetGeneratedTaskTitle).toHaveBeenCalledWith( + "task-title-1", + "Summarized Title", + expect.any(AbortSignal), + ) }) }) }) diff --git a/src/core/task/__tests__/taskTitleGenerator.spec.ts b/src/core/task/__tests__/taskTitleGenerator.spec.ts index 23187a1c51..d41f80df98 100644 --- a/src/core/task/__tests__/taskTitleGenerator.spec.ts +++ b/src/core/task/__tests__/taskTitleGenerator.spec.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from "vitest" +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" import type { ProviderSettings } from "@roo-code/types" import { providerIdentifiers } from "@roo-code/types" @@ -175,6 +175,48 @@ describe("taskTitleGenerator", () => { expect(result).toBeUndefined() }) + describe("with a completion that ignores the abort signal", () => { + beforeEach(() => { + vi.useFakeTimers() + vi.mocked(singleCompletionHandler).mockImplementationOnce(() => new Promise(() => {})) + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it("settles with undefined once the timeout elapses", async () => { + const result = generateTaskTitle("Do something", mockApiConfig, { timeoutMs: 1_000 }) + + await vi.advanceTimersByTimeAsync(1_000) + + await expect(result).resolves.toBeUndefined() + }) + + it("settles with undefined when the parent signal aborts", async () => { + const controller = new AbortController() + const result = generateTaskTitle("Do something", mockApiConfig, { + abortSignal: controller.signal, + timeoutMs: 60_000, + }) + + await vi.advanceTimersByTimeAsync(0) + controller.abort() + + await expect(result).resolves.toBeUndefined() + }) + }) + + it("uses a customized TASK_TITLE support prompt when one is saved", async () => { + vi.mocked(singleCompletionHandler).mockResolvedValueOnce("Custom Title") + + await generateTaskTitle("Do something", mockApiConfig, { + customSupportPrompts: { TASK_TITLE: "My template: ${userInput}" }, + }) + + expect(vi.mocked(singleCompletionHandler).mock.calls[0]?.[1]).toBe("My template: Do something") + }) + it("returns undefined when cleaned title is empty", async () => { vi.mocked(singleCompletionHandler).mockResolvedValueOnce(' "" ') diff --git a/src/core/task/taskTitleGenerator.ts b/src/core/task/taskTitleGenerator.ts index e11737f0a7..84bde2d70a 100644 --- a/src/core/task/taskTitleGenerator.ts +++ b/src/core/task/taskTitleGenerator.ts @@ -1,6 +1,6 @@ import type { ProviderSettings } from "@roo-code/types" -import { supportPrompt } from "../../shared/support-prompt" +import { supportPrompt, type CustomSupportPrompts } from "../../shared/support-prompt" import { singleCompletionHandler } from "../../utils/single-completion-handler" export const MAX_INPUT_PROMPT_LENGTH = 1000 @@ -15,6 +15,7 @@ export interface GenerateTaskTitleOptions { abortSignal?: AbortSignal timeoutMs?: number taskId?: string + customSupportPrompts?: CustomSupportPrompts } /** @@ -85,15 +86,32 @@ export async function generateTaskTitle( controller.abort() }, timeoutMs) + // Timeout and parent cancellation settle the title request on their own + // instead of relying on the provider to honour the abort signal, so a + // provider that ignores it cannot hold the title flow open. + let onAbort: (() => void) | undefined + const aborted = new Promise((_resolve, reject) => { + onAbort = () => reject(new Error("Title generation timed out or was cancelled")) + controller.signal.addEventListener("abort", onAbort, { once: true }) + }) + try { const promptSnippet = trimmedPrompt.slice(0, MAX_INPUT_PROMPT_LENGTH) - const titlePrompt = supportPrompt.create("TASK_TITLE", { userInput: promptSnippet }) + const titlePrompt = supportPrompt.create( + "TASK_TITLE", + { userInput: promptSnippet }, + options?.customSupportPrompts, + ) - const rawTitle = await singleCompletionHandler(apiConfiguration, titlePrompt, { + const completion = singleCompletionHandler(apiConfiguration, titlePrompt, { abortSignal: controller.signal, timeoutMs, taskId: options?.taskId, }) + // Whichever side loses the race settles with nobody awaiting it. + completion.catch(() => {}) + aborted.catch(() => {}) + const rawTitle = await Promise.race([completion, aborted]) if (controller.signal.aborted || parentSignal?.aborted) { return undefined @@ -107,6 +125,9 @@ export async function generateTaskTitle( return undefined } finally { clearTimeout(timeoutId) + if (onAbort) { + controller.signal.removeEventListener("abort", onAbort) + } if (parentSignal) { parentSignal.removeEventListener("abort", onParentAbort) } diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 21ba6b1c4c..a54c480312 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -2814,6 +2814,11 @@ export class ClineProvider return } + // The user's choice — including clearing the title — supersedes a + // generated title still pending for a running task. Cancel it before + // queuing the rename so a queued generated-title write sees the abort. + this.taskRegistry.getById(id)?.cancelTitleGeneration() + // Read and write under the store lock so a queued task save or delegation // transition cannot be overwritten by a stale snapshot of the item. const trimmed = title?.trim() @@ -2836,26 +2841,26 @@ export class ClineProvider * The existing-title check reads the persisted record inside the same * per-file lock as the write, so a manual rename committed by another * provider's store after this store's cache was last refreshed is never - * overwritten by the generated title. Returns true when the title was - * applied, false when the item already had a title. Throws when the task - * is not in the store yet so the caller can retry. + * overwritten by the generated title. The store's authoritative item is + * published even when the generated title is not applied, so a title + * preserved from another host reaches the local header and task list. A + * signal aborted while the write is queued skips it. Returns true when the + * title was applied, false otherwise. Throws when the task is not in the + * store yet so the caller can retry. */ - async setGeneratedTaskTitle(id: string, title: string): Promise { + async setGeneratedTaskTitle(id: string, title: string, signal?: AbortSignal): Promise { const trimmed = title.trim() if (!trimmed) { return false } - const { applied } = await this.taskHistoryStore.setGeneratedTitleIfUnset(id, trimmed) + const { item, applied } = await this.taskHistoryStore.setGeneratedTitleIfUnset(id, trimmed, { signal }) - if (applied) { + if (item && item.title) { this.recentTasksCache = undefined if (this.isViewLaunched) { - const updatedItem = this.taskHistoryStore.get(id) - if (updatedItem) { - await this.postMessageToWebview({ type: "taskHistoryItemUpdated", taskHistoryItem: updatedItem }) - } + await this.postMessageToWebview({ type: "taskHistoryItemUpdated", taskHistoryItem: item }) } } diff --git a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts index 5a74e04cc3..6ed3c3376b 100644 --- a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts @@ -736,7 +736,9 @@ describe("ClineProvider Task History Synchronization", () => { expect(getStored("task-gen-after")?.title).toBe("Manual Title") }) - it("keeps an existing title and reports that nothing was applied", async () => { + it("keeps an existing title and publishes the authoritative item", async () => { + await provider.resolveWebviewView(mockWebviewView) + provider.isViewLaunched = true await provider.updateTaskHistory(createHistoryItem({ id: "task-gen-existing", task: "Original prompt" }), { broadcast: false, }) @@ -747,7 +749,59 @@ describe("ClineProvider Task History Synchronization", () => { expect(getStored("task-gen-existing")?.title).toBe("Manual Title") const updates = findCallsByType(mockPostMessage.mock.calls, "taskHistoryItemUpdated") - expect(updates).toHaveLength(0) + expect(updates.at(-1)?.[0].taskHistoryItem).toMatchObject({ + id: "task-gen-existing", + title: "Manual Title", + }) + }) + + it("publishes a title preserved from another host when the generated title is not applied", async () => { + await provider.resolveWebviewView(mockWebviewView) + provider.isViewLaunched = true + const item = createHistoryItem({ id: "task-gen-remote", task: "Original prompt" }) + await provider.updateTaskHistory(item, { broadcast: false }) + // The store read a rename committed on disk by another host. + vi.spyOn(provider.taskHistoryStore, "setGeneratedTitleIfUnset").mockResolvedValueOnce({ + item: { ...item, title: "Remote Title" }, + applied: false, + }) + mockPostMessage.mockClear() + + await expect(provider.setGeneratedTaskTitle("task-gen-remote", "Generated Title")).resolves.toBe(false) + + const updates = findCallsByType(mockPostMessage.mock.calls, "taskHistoryItemUpdated") + expect(updates.at(-1)?.[0].taskHistoryItem).toMatchObject({ id: "task-gen-remote", title: "Remote Title" }) + }) + + it.each([ + ["a new title", "Manual Title"], + ["a cleared title", ""], + ])("cancels pending title generation of the running task on %s", async (_label, title) => { + await provider.updateTaskHistory(createHistoryItem({ id: "task-gen-clear", task: "Original prompt" }), { + broadcast: false, + }) + // Task is module-mocked in this suite, so the double supplies the method. + const cancelTitleGeneration = vi.fn() + const runningTask = Object.assign(Object.create(Task.prototype) as Task, { cancelTitleGeneration }) + vi.spyOn(provider["taskRegistry"], "getById").mockImplementation((id: string) => + id === "task-gen-clear" ? runningTask : undefined, + ) + + await provider.renameTaskWithId("task-gen-clear", title) + + expect(cancelTitleGeneration).toHaveBeenCalledTimes(1) + }) + + it("skips the write when the signal is aborted before the queued write runs", async () => { + await provider.updateTaskHistory(createHistoryItem({ id: "task-gen-abort", task: "Original prompt" }), { + broadcast: false, + }) + + await expect( + provider.setGeneratedTaskTitle("task-gen-abort", "Generated Title", AbortSignal.abort()), + ).resolves.toBe(false) + + expect(getStored("task-gen-abort")?.title).toBeUndefined() }) it("throws when the task is not yet recorded so the caller can retry", async () => { diff --git a/webview-ui/src/i18n/locales/ca/prompts.json b/webview-ui/src/i18n/locales/ca/prompts.json index 7e1db1e819..5dc180a27c 100644 --- a/webview-ui/src/i18n/locales/ca/prompts.json +++ b/webview-ui/src/i18n/locales/ca/prompts.json @@ -129,6 +129,10 @@ "NEW_TASK": { "label": "Iniciar nova tasca", "description": "Inicieu una nova tasca amb l'entrada proporcionada. Disponible a la paleta de comandes." + }, + "TASK_TITLE": { + "label": "Títol de la tasca", + "description": "Genera un títol breu per a una tasca nova a partir de la seva sol·licitud inicial. S'executa en segon pla quan comença una tasca." } } }, diff --git a/webview-ui/src/i18n/locales/de/prompts.json b/webview-ui/src/i18n/locales/de/prompts.json index 5d376bedc8..c72618ef0f 100644 --- a/webview-ui/src/i18n/locales/de/prompts.json +++ b/webview-ui/src/i18n/locales/de/prompts.json @@ -129,6 +129,10 @@ "NEW_TASK": { "label": "Neue Aufgabe starten", "description": "Starte eine neue Aufgabe mit deiner Eingabe. Verfügbar in der Befehlspalette." + }, + "TASK_TITLE": { + "label": "Aufgabentitel", + "description": "Erzeugt aus der ersten Eingabe einen kurzen Titel für eine neue Aufgabe. Läuft im Hintergrund, wenn eine Aufgabe startet." } } }, diff --git a/webview-ui/src/i18n/locales/en/prompts.json b/webview-ui/src/i18n/locales/en/prompts.json index 31d3ae82b8..0570948a51 100644 --- a/webview-ui/src/i18n/locales/en/prompts.json +++ b/webview-ui/src/i18n/locales/en/prompts.json @@ -128,6 +128,10 @@ "NEW_TASK": { "label": "Start New Task", "description": "Start a new task with user input. Available in the Command Palette." + }, + "TASK_TITLE": { + "label": "Task Title", + "description": "Generate a short title for a new task from its initial prompt. Runs in the background when a task starts." } } }, diff --git a/webview-ui/src/i18n/locales/es/prompts.json b/webview-ui/src/i18n/locales/es/prompts.json index a11975e0ab..e570902fbd 100644 --- a/webview-ui/src/i18n/locales/es/prompts.json +++ b/webview-ui/src/i18n/locales/es/prompts.json @@ -129,6 +129,10 @@ "NEW_TASK": { "label": "Iniciar nueva tarea", "description": "Inicia una nueva tarea con entrada del usuario. Disponible en la Paleta de comandos." + }, + "TASK_TITLE": { + "label": "Título de la tarea", + "description": "Genera un título breve para una tarea nueva a partir de su solicitud inicial. Se ejecuta en segundo plano al iniciar una tarea." } } }, diff --git a/webview-ui/src/i18n/locales/fr/prompts.json b/webview-ui/src/i18n/locales/fr/prompts.json index c1e613710c..dd77c0fa08 100644 --- a/webview-ui/src/i18n/locales/fr/prompts.json +++ b/webview-ui/src/i18n/locales/fr/prompts.json @@ -129,6 +129,10 @@ "NEW_TASK": { "label": "Démarrer une nouvelle tâche", "description": "Démarre une nouvelle tâche avec ton entrée. Disponible dans la palette de commandes." + }, + "TASK_TITLE": { + "label": "Titre de la tâche", + "description": "Génère un titre court pour une nouvelle tâche à partir de sa demande initiale. S'exécute en arrière-plan au démarrage d'une tâche." } } }, diff --git a/webview-ui/src/i18n/locales/hi/prompts.json b/webview-ui/src/i18n/locales/hi/prompts.json index a610828619..3aa94922b7 100644 --- a/webview-ui/src/i18n/locales/hi/prompts.json +++ b/webview-ui/src/i18n/locales/hi/prompts.json @@ -129,6 +129,10 @@ "NEW_TASK": { "label": "नया कार्य शुरू करें", "description": "इनपुट के साथ नया कार्य शुरू करें। कमांड पैलेट में उपलब्ध है।" + }, + "TASK_TITLE": { + "label": "कार्य शीर्षक", + "description": "नए कार्य के शुरुआती प्रॉम्प्ट से एक छोटा शीर्षक बनाता है। कार्य शुरू होने पर बैकग्राउंड में चलता है।" } } }, diff --git a/webview-ui/src/i18n/locales/id/prompts.json b/webview-ui/src/i18n/locales/id/prompts.json index a5bb8d8405..10a98073af 100644 --- a/webview-ui/src/i18n/locales/id/prompts.json +++ b/webview-ui/src/i18n/locales/id/prompts.json @@ -129,6 +129,10 @@ "NEW_TASK": { "label": "Mulai Tugas Baru", "description": "Mulai tugas baru dengan input pengguna. Tersedia di Command Palette." + }, + "TASK_TITLE": { + "label": "Judul Tugas", + "description": "Membuat judul singkat untuk tugas baru dari prompt awalnya. Berjalan di latar belakang saat tugas dimulai." } } }, diff --git a/webview-ui/src/i18n/locales/it/prompts.json b/webview-ui/src/i18n/locales/it/prompts.json index a7e7816b80..46356cceac 100644 --- a/webview-ui/src/i18n/locales/it/prompts.json +++ b/webview-ui/src/i18n/locales/it/prompts.json @@ -129,6 +129,10 @@ "NEW_TASK": { "label": "Avvia nuova attività", "description": "Avvia una nuova attività con il tuo input. Disponibile nella palette dei comandi." + }, + "TASK_TITLE": { + "label": "Titolo dell'attività", + "description": "Genera un titolo breve per una nuova attività a partire dal prompt iniziale. Viene eseguito in background all'avvio di un'attività." } } }, diff --git a/webview-ui/src/i18n/locales/ja/prompts.json b/webview-ui/src/i18n/locales/ja/prompts.json index 2ac7743c7e..90cb8d7372 100644 --- a/webview-ui/src/i18n/locales/ja/prompts.json +++ b/webview-ui/src/i18n/locales/ja/prompts.json @@ -129,6 +129,10 @@ "NEW_TASK": { "label": "新しいタスクを開始", "description": "入力内容で新しいタスクを開始できます。コマンドパレットから利用できます。" + }, + "TASK_TITLE": { + "label": "タスクタイトル", + "description": "新しいタスクの最初のプロンプトから短いタイトルを生成します。タスク開始時にバックグラウンドで実行されます。" } } }, diff --git a/webview-ui/src/i18n/locales/ko/prompts.json b/webview-ui/src/i18n/locales/ko/prompts.json index bf626fe170..d4596649e5 100644 --- a/webview-ui/src/i18n/locales/ko/prompts.json +++ b/webview-ui/src/i18n/locales/ko/prompts.json @@ -129,6 +129,10 @@ "NEW_TASK": { "label": "새 작업 시작", "description": "입력한 내용으로 새 작업을 시작할 수 있습니다. 명령 팔레트에서 이용 가능합니다." + }, + "TASK_TITLE": { + "label": "작업 제목", + "description": "새 작업의 첫 프롬프트로 짧은 제목을 생성합니다. 작업이 시작되면 백그라운드에서 실행됩니다." } } }, diff --git a/webview-ui/src/i18n/locales/nl/prompts.json b/webview-ui/src/i18n/locales/nl/prompts.json index bd316a9ded..44a1a4bd31 100644 --- a/webview-ui/src/i18n/locales/nl/prompts.json +++ b/webview-ui/src/i18n/locales/nl/prompts.json @@ -129,6 +129,10 @@ "NEW_TASK": { "label": "Nieuwe taak starten", "description": "Start een nieuwe taak met gebruikersinvoer. Beschikbaar via de Command Palette." + }, + "TASK_TITLE": { + "label": "Taaktitel", + "description": "Genereert een korte titel voor een nieuwe taak op basis van de eerste prompt. Wordt op de achtergrond uitgevoerd wanneer een taak start." } } }, diff --git a/webview-ui/src/i18n/locales/pl/prompts.json b/webview-ui/src/i18n/locales/pl/prompts.json index a1b665d435..76fd08a9f7 100644 --- a/webview-ui/src/i18n/locales/pl/prompts.json +++ b/webview-ui/src/i18n/locales/pl/prompts.json @@ -129,6 +129,10 @@ "NEW_TASK": { "label": "Rozpocznij nowe zadanie", "description": "Rozpocznij nowe zadanie z wprowadzonymi danymi. Dostępne w palecie poleceń." + }, + "TASK_TITLE": { + "label": "Tytuł zadania", + "description": "Generuje krótki tytuł nowego zadania na podstawie jego początkowego promptu. Działa w tle po rozpoczęciu zadania." } } }, diff --git a/webview-ui/src/i18n/locales/pt-BR/prompts.json b/webview-ui/src/i18n/locales/pt-BR/prompts.json index 1d61323ca7..7723a82d5b 100644 --- a/webview-ui/src/i18n/locales/pt-BR/prompts.json +++ b/webview-ui/src/i18n/locales/pt-BR/prompts.json @@ -129,6 +129,10 @@ "NEW_TASK": { "label": "Iniciar Nova Tarefa", "description": "Inicie uma nova tarefa com a entrada fornecida. Disponível na paleta de comandos." + }, + "TASK_TITLE": { + "label": "Título da tarefa", + "description": "Gera um título curto para uma nova tarefa a partir do prompt inicial. É executado em segundo plano quando uma tarefa começa." } } }, diff --git a/webview-ui/src/i18n/locales/ru/prompts.json b/webview-ui/src/i18n/locales/ru/prompts.json index ba2cc0593e..5309d4777d 100644 --- a/webview-ui/src/i18n/locales/ru/prompts.json +++ b/webview-ui/src/i18n/locales/ru/prompts.json @@ -129,6 +129,10 @@ "NEW_TASK": { "label": "Начать новую задачу", "description": "Начать новую задачу с пользовательским вводом. Доступно в палитре команд." + }, + "TASK_TITLE": { + "label": "Название задачи", + "description": "Создаёт короткое название новой задачи по её первому запросу. Выполняется в фоне при запуске задачи." } } }, diff --git a/webview-ui/src/i18n/locales/tr/prompts.json b/webview-ui/src/i18n/locales/tr/prompts.json index 77a4d3434d..5cde4a4fc7 100644 --- a/webview-ui/src/i18n/locales/tr/prompts.json +++ b/webview-ui/src/i18n/locales/tr/prompts.json @@ -129,6 +129,10 @@ "NEW_TASK": { "label": "Yeni Görev Başlat", "description": "Girdiyle yeni bir görev başlat. Komut paletinde kullanılabilir." + }, + "TASK_TITLE": { + "label": "Görev Başlığı", + "description": "Yeni bir görev için ilk isteminden kısa bir başlık oluşturur. Görev başladığında arka planda çalışır." } } }, diff --git a/webview-ui/src/i18n/locales/vi/prompts.json b/webview-ui/src/i18n/locales/vi/prompts.json index 6ddbfb169d..807311195d 100644 --- a/webview-ui/src/i18n/locales/vi/prompts.json +++ b/webview-ui/src/i18n/locales/vi/prompts.json @@ -129,6 +129,10 @@ "NEW_TASK": { "label": "Bắt đầu tác vụ mới", "description": "Bắt đầu tác vụ mới với nội dung đã nhập. Có sẵn trong bảng lệnh." + }, + "TASK_TITLE": { + "label": "Tiêu đề tác vụ", + "description": "Tạo tiêu đề ngắn cho tác vụ mới từ lời nhắc ban đầu. Chạy ở chế độ nền khi tác vụ bắt đầu." } } }, diff --git a/webview-ui/src/i18n/locales/zh-CN/prompts.json b/webview-ui/src/i18n/locales/zh-CN/prompts.json index 14aa3eb95c..aea22aaa3f 100644 --- a/webview-ui/src/i18n/locales/zh-CN/prompts.json +++ b/webview-ui/src/i18n/locales/zh-CN/prompts.json @@ -129,6 +129,10 @@ "NEW_TASK": { "label": "新任务", "description": "控制开始新任务时的用户提示词。可在首页对话框中使用。" + }, + "TASK_TITLE": { + "label": "任务标题", + "description": "根据新任务的初始提示生成简短标题。任务开始时在后台运行。" } } }, diff --git a/webview-ui/src/i18n/locales/zh-TW/prompts.json b/webview-ui/src/i18n/locales/zh-TW/prompts.json index f83207bd45..362fec9fdf 100644 --- a/webview-ui/src/i18n/locales/zh-TW/prompts.json +++ b/webview-ui/src/i18n/locales/zh-TW/prompts.json @@ -128,6 +128,10 @@ "NEW_TASK": { "label": "開始新工作", "description": "根據使用者輸入開始一個新的任務。可在命令選擇區使用。" + }, + "TASK_TITLE": { + "label": "任務標題", + "description": "根據新任務的初始提示產生簡短標題。任務開始時在背景執行。" } } }, From 2c5c2c08818c785757cab009a7d0ce0c2e9444b8 Mon Sep 17 00:00:00 2001 From: daewoongoh Date: Mon, 9 Nov 2026 15:02:27 +0900 Subject: [PATCH 6/8] fix: address second-round CodeRabbit review feedback on task title generation - Store: re-check abort signal inside per-file lock merge callback to prevent writes after cancellation. Added tests for cancellation during lock wait and storage failure propagation. - Rate-limit: agent countdown re-reads shared clock after waiting, handling case where title request dispatched mid-countdown. Both wait paths exercise in test. - Provider state read: wrap in Promise.race with abort signal for abort-aware reads before rate-limit wait, ensuring clock record precedes request dispatch. - Provider write: handle evicted items (item === undefined) by broadcasting taskHistoryUpdate to refresh task list from store. - Tests: added title-first ordering, agent countdown re-wait, stalled state read cancellation, queued-write cancellation with lock hold, and eviction broadcast. Deferred to follow-up issue: - Propagating abort signal through all provider implementations (11 providers ignore abortSignal, including Poe and Kimi Code). - Cross-host title clear persistence (requires HistoryItem schema change). --- src/core/task-persistence/TaskHistoryStore.ts | 15 ++++- .../TaskHistoryStore.realConcurrency.spec.ts | 63 +++++++++++++++++++ src/core/task/Task.ts | 50 +++++++++++---- .../__tests__/Task.titleGeneration.spec.ts | 62 ++++++++++++++++++ src/core/webview/ClineProvider.ts | 10 ++- .../ClineProvider.taskHistory.spec.ts | 38 +++++++++-- 6 files changed, 217 insertions(+), 21 deletions(-) diff --git a/src/core/task-persistence/TaskHistoryStore.ts b/src/core/task-persistence/TaskHistoryStore.ts index f399d0cd7e..cf06c1186f 100644 --- a/src/core/task-persistence/TaskHistoryStore.ts +++ b/src/core/task-persistence/TaskHistoryStore.ts @@ -1202,8 +1202,9 @@ export class TaskHistoryStore { * Deletion by another host is authoritative: when no persisted record * exists, or the persisted record is malformed or carries a different task * ID, nothing is written, the stale cache entry is dropped, and the result - * has no item. A signal aborted while the call waits on the store lock - * skips the write, so a cancelled task's title is never persisted. + * has no item. A signal aborted while the call waits on the store lock or + * the per-file lock skips the write, so a cancelled task's title is never + * persisted. * * @returns The authoritative record (absent when the write was rejected) * and whether the generated title was applied. @@ -1233,9 +1234,16 @@ export class TaskHistoryStore { let applied = !cached.title let authoritative: HistoryItem = cached.title ? cached : incoming let rejectedDiskRecord = false + let cancelled = false try { await safeWriteJson(filePath, incoming, { merge: (existing) => { + // Cancellation can land while waiting on the per-file lock; + // a throwing merge writes nothing. + if (options.signal?.aborted) { + cancelled = true + throw new Error(`[TaskHistoryStore] setGeneratedTitleIfUnset: task ${taskId} cancelled`) + } // Writing the cached item back would recreate a task another // host deleted or overwrite a record that is not this task's, // so a missing, malformed, or mismatched record fails closed. @@ -1263,6 +1271,9 @@ export class TaskHistoryStore { }, }) } catch (error) { + if (cancelled) { + return { item: cached, applied: false } + } if (!rejectedDiskRecord) { throw error } diff --git a/src/core/task-persistence/__tests__/TaskHistoryStore.realConcurrency.spec.ts b/src/core/task-persistence/__tests__/TaskHistoryStore.realConcurrency.spec.ts index d2c8227b73..8a584c9f2e 100644 --- a/src/core/task-persistence/__tests__/TaskHistoryStore.realConcurrency.spec.ts +++ b/src/core/task-persistence/__tests__/TaskHistoryStore.realConcurrency.spec.ts @@ -526,6 +526,69 @@ describe("TaskHistoryStore real cross-host locking", () => { expect(store.get("solo-task")?.title).toBeUndefined() }) }) + + it("skips the write when cancelled while waiting on the per-file lock", async () => { + const actualFs = await vi.importActual("fs/promises") + await withSingleStore(async (store, filePath, onWrite) => { + const before = await fs.readFile(filePath, "utf8") + const controller = new AbortController() + let signalReadStarted!: () => void + const readStarted = new Promise((resolve) => { + signalReadStarted = resolve + }) + let releaseRead!: () => void + const readCanContinue = new Promise((resolve) => { + releaseRead = resolve + }) + // Pause inside the per-file lock's disk read, before the merge. + vi.mocked(fs.readFile).mockImplementation(async (...args: Parameters) => { + if (args[0] === filePath) { + signalReadStarted() + await readCanContinue + } + return actualFs.readFile(...args) + }) + + try { + const write = store.setGeneratedTitleIfUnset("solo-task", "Generated Title", { + signal: controller.signal, + }) + await readStarted + controller.abort() + releaseRead() + + await expect(write).resolves.toMatchObject({ applied: false }) + } finally { + vi.mocked(fs.readFile).mockImplementation(actualFs.readFile) + } + expect(await fs.readFile(filePath, "utf8")).toBe(before) + expect(onWrite).not.toHaveBeenCalled() + expect(store.get("solo-task")?.title).toBeUndefined() + }) + }) + + it("propagates an ordinary storage failure and keeps the cached task", async () => { + const actualFs = await vi.importActual("fs/promises") + await withSingleStore(async (store, filePath, onWrite) => { + const storageError = Object.assign(new Error("permission denied"), { code: "EACCES" }) + vi.mocked(fs.readFile).mockImplementation(async (...args: Parameters) => { + if (args[0] === filePath) { + throw storageError + } + return actualFs.readFile(...args) + }) + + try { + await expect(store.setGeneratedTitleIfUnset("solo-task", "Generated Title")).rejects.toBe( + storageError, + ) + } finally { + vi.mocked(fs.readFile).mockImplementation(actualFs.readFile) + } + expect(store.get("solo-task")).toMatchObject({ id: "solo-task" }) + expect(onWrite).not.toHaveBeenCalled() + }) + }) }) describe("setGeneratedTitleIfUnset with a rejected disk record", () => { diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index dc497b1a57..4ea2a1ed40 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -3229,6 +3229,18 @@ export class Task extends EventEmitter implements TaskLike { // records on the shared clock so subsequent task/subtask requests // honour it. `startTask` schedules this method with `void`, so the // agent never waits on the title result. + // + // A saved TASK_TITLE support prompt replaces the built-in template. The + // state is read before the rate-limit wait so nothing awaits between + // recording on the shared clock and dispatching; an unreadable state + // falls back to the built-in template, and cancellation ends the read. + const state = await Promise.race([ + provider.getState().catch(() => undefined), + new Promise((resolve) => { + controller.signal.addEventListener("abort", () => resolve(undefined), { once: true }) + }), + ]) + await this.waitForProviderRateLimitWindow(controller.signal) if (controller.signal.aborted || this.abort || this.abandoned) { @@ -3237,10 +3249,6 @@ export class Task extends EventEmitter implements TaskLike { this.rateLimitClock.recordRequest() - // A saved TASK_TITLE support prompt replaces the built-in template; - // an unreadable state falls back to the built-in template. - const state = await provider.getState().catch(() => undefined) - const generated = await generateTaskTitle(taskText, this.apiConfiguration, { abortSignal: controller.signal, timeoutMs: DEFAULT_TITLE_TIMEOUT_MS, @@ -5797,25 +5805,41 @@ export class Task extends EventEmitter implements TaskLike { private async maybeWaitForProviderRateLimit(retryAttempt: number): Promise { const rateLimitSeconds = this.apiConfiguration?.rateLimitSeconds ?? 0 - const lastRequestTime = this.rateLimitClock.getLastRequestTime() - if (rateLimitSeconds <= 0 || !lastRequestTime) { + let lastRequestTime = this.rateLimitClock.getLastRequestTime() + // Only show the countdown UX on the first attempt. Retry flows have their own delay messaging. + if (rateLimitSeconds <= 0 || !lastRequestTime || retryAttempt !== 0) { return } - const now = performance.now() - const timeSinceLastRequest = now - lastRequestTime - const rateLimitDelay = Math.ceil( - Math.min(rateLimitSeconds, Math.max(0, rateLimitSeconds * 1000 - timeSinceLastRequest) / 1000), - ) + let waited = false + while (lastRequestTime) { + const timeSinceLastRequest = performance.now() - lastRequestTime + const rateLimitDelay = Math.ceil( + Math.min(rateLimitSeconds, Math.max(0, rateLimitSeconds * 1000 - timeSinceLastRequest) / 1000), + ) + if (rateLimitDelay <= 0) { + break + } - // Only show the countdown UX on the first attempt. Retry flows have their own delay messaging. - if (rateLimitDelay > 0 && retryAttempt === 0) { + waited = true for (let i = rateLimitDelay; i > 0; i--) { // Send structured JSON data for i18n-safe transport const delayMessage = JSON.stringify({ seconds: i }) await this.say("api_req_rate_limit_wait", delayMessage, undefined, true) await delay(1000) } + + // A background title request can dispatch on the shared clock during + // the countdown; re-wait from that dispatch instead of proceeding + // from the stale deadline. + const latestRequestTime = this.rateLimitClock.getLastRequestTime() + if (latestRequestTime === lastRequestTime) { + break + } + lastRequestTime = latestRequestTime + } + + if (waited) { // Finalize the partial message so the UI doesn't keep rendering an in-progress spinner. await this.say("api_req_rate_limit_wait", undefined, undefined, false) } diff --git a/src/core/task/__tests__/Task.titleGeneration.spec.ts b/src/core/task/__tests__/Task.titleGeneration.spec.ts index 5c07a4cbee..b0bc833275 100644 --- a/src/core/task/__tests__/Task.titleGeneration.spec.ts +++ b/src/core/task/__tests__/Task.titleGeneration.spec.ts @@ -33,6 +33,7 @@ type TaskTitleAccess = { cancelCurrentRequest(): void requestAbort(isAbandoned: boolean): void cancelTitleGeneration(): void + maybeWaitForProviderRateLimit(retryAttempt: number): Promise titleAbortController?: AbortController abort: boolean abandoned: boolean @@ -227,6 +228,20 @@ describe("Task title generation", () => { expect(mockSetGeneratedTaskTitle).not.toHaveBeenCalled() }) + it("stops without a title request when cancelled during a stalled state read", async () => { + mockProvider.getState.mockReturnValueOnce(new Promise(() => {})) + const task = createTestTask() + const access = getTaskTitleAccess(task) + + const generation = access.generateTitleInBackground("Initial prompt message") + await vi.waitFor(() => expect(mockProvider.getState).toHaveBeenCalled()) + access.cancelTitleGeneration() + await generation + + expect(taskTitleGen.generateTaskTitle).not.toHaveBeenCalled() + expect(mockSetGeneratedTaskTitle).not.toHaveBeenCalled() + }) + it("skips title generation and the title write in ExtensionMode.Test", async () => { mockProvider.context.extensionMode = vscode.ExtensionMode.Test const task = createTestTask() @@ -447,6 +462,53 @@ describe("Task title generation", () => { expect(taskDispatchTime).toBeDefined() expect((titleDispatchTime ?? 0) - (taskDispatchTime ?? 0)).toBeGreaterThanOrEqual(10_000) }) + + it("re-waits the agent countdown when a title request dispatches during it", async () => { + const clock = createRateLimitClock() + const task = createTestTask({ + rateLimitClock: clock, + apiConfiguration: { ...mockApiConfig, rateLimitSeconds: 10 }, + }) + let titleDispatchTime: number | undefined + let countdownTicks = 0 + // Each countdown tick advances one second; midway through the agent's + // countdown, the title request's wait ends and it dispatches first. + vi.mocked(delay).mockImplementation((ms: number) => { + now += ms + countdownTicks++ + if (countdownTicks === 5) { + clock.recordRequest() + titleDispatchTime = now + } + return Promise.resolve(undefined) + }) + clock.recordRequest() + + await getTaskTitleAccess(task).maybeWaitForProviderRateLimit(0) + const agentDispatchTime = now + + // The agent is spaced a full window from the title dispatch instead + // of proceeding from the deadline it computed before the countdown. + expect(titleDispatchTime).toBeDefined() + expect(agentDispatchTime - (titleDispatchTime ?? 0)).toBeGreaterThanOrEqual(10_000) + }) + + it("reads the support prompts before the rate-limit wait", async () => { + const clock = createRateLimitClock() + const task = createTestTask({ + rateLimitClock: clock, + apiConfiguration: { ...mockApiConfig, rateLimitSeconds: 10 }, + }) + vi.mocked(taskTitleGen.generateTaskTitle).mockResolvedValueOnce("Summarized Title") + clock.recordRequest() + + await getTaskTitleAccess(task).generateTitleInBackground("Initial prompt message") + + // Nothing awaits between recording on the shared clock and dispatching. + expect(mockProvider.getState.mock.invocationCallOrder[0]).toBeLessThan( + vi.mocked(delay).mock.invocationCallOrder[0], + ) + }) }) describe("cancellation and abort handling", () => { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index a54c480312..720ea4c4ce 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -2843,7 +2843,8 @@ export class ClineProvider * provider's store after this store's cache was last refreshed is never * overwritten by the generated title. The store's authoritative item is * published even when the generated title is not applied, so a title - * preserved from another host reaches the local header and task list. A + * preserved from another host reaches the local header and task list; a + * task the store dropped as deleted on disk refreshes the task list. A * signal aborted while the write is queued skips it. Returns true when the * title was applied, false otherwise. Throws when the task is not in the * store yet so the caller can retry. @@ -2856,7 +2857,12 @@ export class ClineProvider const { item, applied } = await this.taskHistoryStore.setGeneratedTitleIfUnset(id, trimmed, { signal }) - if (item && item.title) { + if (!item) { + // The store dropped a task deleted or invalidated on disk by another + // host; refresh the task list so it no longer shows the stale entry. + this.recentTasksCache = undefined + await this.broadcastTaskHistoryUpdate() + } else if (item.title) { this.recentTasksCache = undefined if (this.isViewLaunched) { diff --git a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts index 6ed3c3376b..56b45d4c8b 100644 --- a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts @@ -792,18 +792,48 @@ describe("ClineProvider Task History Synchronization", () => { expect(cancelTitleGeneration).toHaveBeenCalledTimes(1) }) - it("skips the write when the signal is aborted before the queued write runs", async () => { + it("skips a queued write when the task is cancelled before the write runs", async () => { await provider.updateTaskHistory(createHistoryItem({ id: "task-gen-abort", task: "Original prompt" }), { broadcast: false, }) + // Hold the store lock so the generated-title write queues behind it. + let releaseLock: (() => void) | undefined + const held = provider.taskHistoryStore["withLock"]( + () => + new Promise((resolve) => { + releaseLock = resolve + }), + ) + await vi.waitFor(() => expect(releaseLock).toBeDefined()) + const controller = new AbortController() - await expect( - provider.setGeneratedTaskTitle("task-gen-abort", "Generated Title", AbortSignal.abort()), - ).resolves.toBe(false) + const generated = provider.setGeneratedTaskTitle("task-gen-abort", "Generated Title", controller.signal) + controller.abort() + releaseLock?.() + await held + await expect(generated).resolves.toBe(false) expect(getStored("task-gen-abort")?.title).toBeUndefined() }) + it("refreshes the task list when the store drops a task deleted by another host", async () => { + await provider.resolveWebviewView(mockWebviewView) + provider.isViewLaunched = true + await provider.updateTaskHistory(createHistoryItem({ id: "task-gen-deleted", task: "Original prompt" }), { + broadcast: false, + }) + vi.spyOn(provider.taskHistoryStore, "setGeneratedTitleIfUnset").mockResolvedValueOnce({ + item: undefined, + applied: false, + }) + mockPostMessage.mockClear() + + await expect(provider.setGeneratedTaskTitle("task-gen-deleted", "Generated Title")).resolves.toBe(false) + + expect(findCallsByType(mockPostMessage.mock.calls, "taskHistoryUpdated")).toHaveLength(1) + expect(findCallsByType(mockPostMessage.mock.calls, "taskHistoryItemUpdated")).toHaveLength(0) + }) + it("throws when the task is not yet recorded so the caller can retry", async () => { await expect(provider.setGeneratedTaskTitle("missing-task", "Generated Title")).rejects.toThrow( "missing-task", From 76c4c4799a8c3350a74b756d21655ec815452d7e Mon Sep 17 00:00:00 2001 From: daewoongoh Date: Mon, 9 Nov 2026 15:58:35 +0900 Subject: [PATCH 7/8] fix: prevent concurrent title and agent requests from observing the same rate-limit window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reserve the rate-limit slot synchronously when the window is found open, eliminating any await between the availability check and clock recording. This prevents a race where both title and agent requests could observe an open window across separate await points. Renamed functions: - maybeWaitForProviderRateLimit → reserveProviderRateLimitSlot (now records inside) - waitForProviderRateLimitWindow → reserveBackgroundRateLimitSlot (returns true/false) Added 6 concurrent-request tests that verify both dispatch orders (title-first, agent-first) across three window states (empty, expired, pending), confirming each request reserves a separate slot a full window apart. --- src/core/task/Task.ts | 67 +++++++++---------- .../__tests__/Task.titleGeneration.spec.ts | 62 ++++++++++++++++- 2 files changed, 91 insertions(+), 38 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 4ea2a1ed40..565c245cec 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -3241,14 +3241,10 @@ export class Task extends EventEmitter implements TaskLike { }), ]) - await this.waitForProviderRateLimitWindow(controller.signal) - - if (controller.signal.aborted || this.abort || this.abandoned) { + if (!(await this.reserveBackgroundRateLimitSlot(controller.signal))) { return } - this.rateLimitClock.recordRequest() - const generated = await generateTaskTitle(taskText, this.apiConfiguration, { abortSignal: controller.signal, timeoutMs: DEFAULT_TITLE_TIMEOUT_MS, @@ -4149,8 +4145,7 @@ export class Task extends EventEmitter implements TaskLike { // we build environment details (which can take time). This ensures // subsequent requests (including subtasks) still honour the // provider rate-limit window. - await this.maybeWaitForProviderRateLimit(currentItem.retryAttempt ?? 0) - this.rateLimitClock.recordRequest() + await this.reserveProviderRateLimitSlot(currentItem.retryAttempt ?? 0) await this.say( "api_req_started", @@ -5797,17 +5792,23 @@ export class Task extends EventEmitter implements TaskLike { } /** - * Enforce the user-configured provider rate limit. + * Enforce the user-configured provider rate limit and reserve the request + * slot on the shared clock. + * + * The slot is recorded synchronously at the point the window is found open, + * so a concurrent background request on the same clock cannot also observe + * the open window across an `await` before either one records. * * NOTE: This is intentionally treated as expected behavior and is surfaced via * the `api_req_rate_limit_wait` say type (not an error). */ - private async maybeWaitForProviderRateLimit(retryAttempt: number): Promise { + private async reserveProviderRateLimitSlot(retryAttempt: number): Promise { const rateLimitSeconds = this.apiConfiguration?.rateLimitSeconds ?? 0 let lastRequestTime = this.rateLimitClock.getLastRequestTime() // Only show the countdown UX on the first attempt. Retry flows have their own delay messaging. if (rateLimitSeconds <= 0 || !lastRequestTime || retryAttempt !== 0) { + this.rateLimitClock.recordRequest() return } @@ -5839,6 +5840,8 @@ export class Task extends EventEmitter implements TaskLike { lastRequestTime = latestRequestTime } + this.rateLimitClock.recordRequest() + if (waited) { // Finalize the partial message so the UI doesn't keep rendering an in-progress spinner. await this.say("api_req_rate_limit_wait", undefined, undefined, false) @@ -5846,40 +5849,34 @@ export class Task extends EventEmitter implements TaskLike { } /** - * Silent companion to `maybeWaitForProviderRateLimit` for background provider + * Silent companion to `reserveProviderRateLimitSlot` for background provider * requests (e.g. title generation): waits out the remaining provider * rate-limit window without emitting the task-loop countdown UX, since a * second concurrent partial `api_req_rate_limit_wait` say would interleave - * with the task's own countdown. Resolves early (not rejects) when the given - * signal aborts; callers must still re-check abort state after the wait and - * call `rateLimitClock.recordRequest()` right before issuing the request, - * mirroring the task loop. + * with the task's own countdown. * - * A task request can dispatch on the shared clock while this one waits, so - * the latest timestamp is re-read after every wait: the request is spaced - * from the most recent dispatch, not from the timestamp it first read. + * The latest timestamp is re-read after every wait so the request is spaced + * from the most recent dispatch. Returns `true` after recording the slot + * synchronously with the open-window check; returns `false` without + * recording when the signal aborts or the task is aborted or abandoned. */ - private async waitForProviderRateLimitWindow(signal: AbortSignal): Promise { + private async reserveBackgroundRateLimitSlot(signal: AbortSignal): Promise { const rateLimitSeconds = this.apiConfiguration?.rateLimitSeconds ?? 0 - if (rateLimitSeconds <= 0) { - return - } - while (!signal.aborted) { + while (!signal.aborted && !this.abort && !this.abandoned) { const lastRequestTime = this.rateLimitClock.getLastRequestTime() - if (!lastRequestTime) { - return - } - - const remainingMs = rateLimitSeconds * 1000 - (performance.now() - lastRequestTime) + const remainingMs = lastRequestTime ? rateLimitSeconds * 1000 - (performance.now() - lastRequestTime) : 0 if (remainingMs <= 0) { - return + this.rateLimitClock.recordRequest() + return true } await delay(remainingMs, { signal }).catch(() => { // Aborted mid-wait: cancellation, not a failure. }) } + + return false } public async *attemptApiRequest( @@ -5916,18 +5913,18 @@ export class Task extends EventEmitter implements TaskLike { // Get condensing configuration for automatic triggers. const customCondensingPrompt = state?.customSupportPrompts?.CONDENSE - if (!options.skipProviderRateLimit) { - await this.maybeWaitForProviderRateLimit(retryAttempt) - } - // Update last request time right before making the request so that subsequent // requests — even from new subtasks — will honour the provider's rate-limit. // - // NOTE: When recursivelyMakeClineRequests handles rate limiting, it sets the - // timestamp earlier to include the environment details build. We still set it + // NOTE: When recursivelyMakeClineRequests handles rate limiting, it reserves the + // slot earlier to include the environment details build. We still record it // here for direct callers (tests) and for the case where we didn't rate-limit // in the caller. - this.rateLimitClock.recordRequest() + if (options.skipProviderRateLimit) { + this.rateLimitClock.recordRequest() + } else { + await this.reserveProviderRateLimitSlot(retryAttempt) + } // Thread the request state snapshot into prompt generation so the prompt // and the runtime tools (built below from the same `state`) stay aligned diff --git a/src/core/task/__tests__/Task.titleGeneration.spec.ts b/src/core/task/__tests__/Task.titleGeneration.spec.ts index b0bc833275..a58142a889 100644 --- a/src/core/task/__tests__/Task.titleGeneration.spec.ts +++ b/src/core/task/__tests__/Task.titleGeneration.spec.ts @@ -33,7 +33,8 @@ type TaskTitleAccess = { cancelCurrentRequest(): void requestAbort(isAbandoned: boolean): void cancelTitleGeneration(): void - maybeWaitForProviderRateLimit(retryAttempt: number): Promise + reserveProviderRateLimitSlot(retryAttempt: number): Promise + reserveBackgroundRateLimitSlot(signal: AbortSignal): Promise titleAbortController?: AbortController abort: boolean abandoned: boolean @@ -484,8 +485,8 @@ describe("Task title generation", () => { }) clock.recordRequest() - await getTaskTitleAccess(task).maybeWaitForProviderRateLimit(0) - const agentDispatchTime = now + await getTaskTitleAccess(task).reserveProviderRateLimitSlot(0) + const agentDispatchTime = clock.getLastRequestTime() ?? 0 // The agent is spaced a full window from the title dispatch instead // of proceeding from the deadline it computed before the countdown. @@ -493,6 +494,61 @@ describe("Task title generation", () => { expect(agentDispatchTime - (titleDispatchTime ?? 0)).toBeGreaterThanOrEqual(10_000) }) + it.each([ + { window: "an empty clock", first: "title" }, + { window: "an empty clock", first: "agent" }, + { window: "an expired window", first: "title" }, + { window: "an expired window", first: "agent" }, + { window: "a pending window", first: "title" }, + { window: "a pending window", first: "agent" }, + ])( + "spaces concurrent title and agent reservations with $window when the $first starts first", + async ({ window, first }) => { + const clock = createRateLimitClock() + const task = createTestTask({ + rateLimitClock: clock, + apiConfiguration: { ...mockApiConfig, rateLimitSeconds: 10 }, + }) + const access = getTaskTitleAccess(task) + if (window !== "an empty clock") { + clock.recordRequest() + } + if (window === "an expired window") { + now += 20_000 + } + const recordedAt: number[] = [] + const record = clock.recordRequest.bind(clock) + vi.spyOn(clock, "recordRequest").mockImplementation(() => { + recordedAt.push(now) + record() + }) + const startTitle = async () => { + expect(await access.reserveBackgroundRateLimitSlot(new AbortController().signal)).toBe(true) + } + const startAgent = () => access.reserveProviderRateLimitSlot(0) + const [startFirst, startSecond] = + first === "title" ? [startTitle, startAgent] : [startAgent, startTitle] + // Start the second request on the next microtask after the first one + // reads the clock: any await between that read and its reservation + // would let both requests observe the same open window. + let second: Promise | undefined + const read = clock.getLastRequestTime.bind(clock) + vi.spyOn(clock, "getLastRequestTime").mockImplementation(() => { + if (!second) { + second = Promise.resolve().then(startSecond) + } + return read() + }) + + await startFirst() + await second + + // Each request reserves its own slot, a full window after the other. + expect(recordedAt).toHaveLength(2) + expect(Math.abs((recordedAt[1] ?? 0) - (recordedAt[0] ?? 0))).toBeGreaterThanOrEqual(10_000) + }, + ) + it("reads the support prompts before the rate-limit wait", async () => { const clock = createRateLimitClock() const task = createTestTask({ From e6fb7e48f6caf6dc161c6c279b465c16185eb57c Mon Sep 17 00:00:00 2001 From: daewoongoh Date: Mon, 9 Nov 2026 16:34:13 +0900 Subject: [PATCH 8/8] fix: revalidate rate-limit before agent dispatch after preparation and during retries When agent preparation takes longer than the configured rate-limit window or when title generation dispatches during retry backoff, the agent can dispatch too soon without waiting. Add revalidation logic that defers dispatch when another request has recorded on the shared clock since the agent's initial reservation. Track this task's own rate-limit reservations in ownRateLimitReservation. Before dispatch (both in the rate-limited path and skipProviderRateLimit path), call reserveAgentDispatchSlot() which silently waits out the window from any other request that recorded meanwhile, then records synchronously. Added 4 concurrent-dispatch tests: - Title while agent prepares longer than window - No wait when only agent's own reservation exists - Retry during rate-limit wait (rate-limited path) - Retry during rate-limit wait (skipProviderRateLimit path) These tests detect when the fix is reverted, failing the dispatch-timing assertions for all scenarios except when only the agent's reservation exists. --- src/core/task/Task.ts | 46 +++++++++-- src/core/task/__tests__/Task.spec.ts | 110 ++++++++++++++++++++++++++- 2 files changed, 149 insertions(+), 7 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 565c245cec..c178b5ef75 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -437,6 +437,8 @@ export class Task extends EventEmitter implements TaskLike { /** Handlers replaced during in-flight work; disposed when that work settles, or with the task. */ private retiredApis = new Set() private rateLimitClock: RateLimitClock + /** Shared-clock timestamp this task's agent loop last recorded; background requests never set it. */ + private ownRateLimitReservation?: number private autoApprovalHandler: AutoApprovalHandler toolRepetitionDetector: ToolRepetitionDetector @@ -5808,7 +5810,7 @@ export class Task extends EventEmitter implements TaskLike { let lastRequestTime = this.rateLimitClock.getLastRequestTime() // Only show the countdown UX on the first attempt. Retry flows have their own delay messaging. if (rateLimitSeconds <= 0 || !lastRequestTime || retryAttempt !== 0) { - this.rateLimitClock.recordRequest() + await this.reserveAgentDispatchSlot() return } @@ -5840,7 +5842,7 @@ export class Task extends EventEmitter implements TaskLike { lastRequestTime = latestRequestTime } - this.rateLimitClock.recordRequest() + this.recordAgentRateLimitReservation() if (waited) { // Finalize the partial message so the UI doesn't keep rendering an in-progress spinner. @@ -5848,6 +5850,38 @@ export class Task extends EventEmitter implements TaskLike { } } + private recordAgentRateLimitReservation(): void { + this.rateLimitClock.recordRequest() + this.ownRateLimitReservation = this.rateLimitClock.getLastRequestTime() + } + + /** + * Reserve the agent's dispatch slot without the countdown UX. When the + * latest timestamp on the shared clock is this task's own earlier + * reservation (e.g. one taken before building environment details), the + * slot is recorded immediately. When another request — such as background + * title generation, during preparation or retry backoff — recorded since, + * the window is silently waited out from that dispatch first. + */ + private async reserveAgentDispatchSlot(): Promise { + const rateLimitSeconds = this.apiConfiguration?.rateLimitSeconds ?? 0 + + while (!this.abort && !this.abandoned) { + const lastRequestTime = this.rateLimitClock.getLastRequestTime() + const remainingMs = + lastRequestTime && lastRequestTime !== this.ownRateLimitReservation + ? rateLimitSeconds * 1000 - (performance.now() - lastRequestTime) + : 0 + if (remainingMs <= 0) { + break + } + + await delay(remainingMs) + } + + this.recordAgentRateLimitReservation() + } + /** * Silent companion to `reserveProviderRateLimitSlot` for background provider * requests (e.g. title generation): waits out the remaining provider @@ -5917,11 +5951,11 @@ export class Task extends EventEmitter implements TaskLike { // requests — even from new subtasks — will honour the provider's rate-limit. // // NOTE: When recursivelyMakeClineRequests handles rate limiting, it reserves the - // slot earlier to include the environment details build. We still record it - // here for direct callers (tests) and for the case where we didn't rate-limit - // in the caller. + // slot earlier to include the environment details build. Re-reserving here + // waits out any request (e.g. title generation) that dispatched on the shared + // clock since then, including during retry backoff. if (options.skipProviderRateLimit) { - this.rateLimitClock.recordRequest() + await this.reserveAgentDispatchSlot() } else { await this.reserveProviderRateLimitSlot(retryAttempt) } diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index c56d5291f6..1967287da9 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -25,7 +25,7 @@ import { TelemetryService } from "@roo-code/telemetry" import { MODEL_FETCH_TIMEOUT_MS, Task } from "../Task" import { SYSTEM_PROMPT } from "../../prompts/system" -import { createRateLimitClock } from "../RateLimitClock" +import { createRateLimitClock, type RateLimitClock } from "../RateLimitClock" import { summarizeConversation } from "../../condense" import { getEnvironmentDetails } from "../../environment/getEnvironmentDetails" import { ClineProvider } from "../../webview/ClineProvider" @@ -56,6 +56,7 @@ type TaskTestAccess = { saveClineMessages: () => Promise safeEnsureModelFetched: () => Promise backoffAndAnnounce: (retryAttempt: number, error: unknown) => Promise + reserveProviderRateLimitSlot: (retryAttempt: number) => Promise requestPolicySnapshot?: RequestPolicySnapshot getFilesReadByRooSafely: (context: string) => Promise addToApiConversationHistory: (message: unknown, reasoning?: string) => Promise @@ -4024,6 +4025,113 @@ describe("Cline", () => { expect(lastTime).toBeDefined() expect(lastTime).toBeGreaterThan(0) }) + + describe("dispatch revalidation against other requests on the shared clock", () => { + // `delay` advances a controlled `performance.now()` like a real wait, + // so dispatch times can be compared against the 5s window. + let now: number + + beforeEach(() => { + now = 1_000_000 + vi.spyOn(performance, "now").mockImplementation(() => now) + mockDelay.mockImplementation((ms: number) => { + now += ms + return Promise.resolve(undefined) + }) + }) + + afterEach(() => { + vi.mocked(performance.now).mockRestore() + mockDelay.mockReset().mockResolvedValue(undefined) + }) + + function createDispatchTask(clock: RateLimitClock) { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + rateLimitClock: clock, + }) + vi.spyOn(getTaskTestAccess(task), "getSystemPrompt").mockResolvedValue("mock system prompt") + vi.spyOn(task, "say").mockResolvedValue(undefined) + const dispatchedAt: number[] = [] + const createMessage = vi.spyOn(task.api, "createMessage") + createMessage.mockImplementation(() => { + dispatchedAt.push(now) + return asyncStreamFrom([{ type: "text", text: "response" }]) + }) + return { task, dispatchedAt, createMessage } + } + + it("waits out a request that dispatched while the agent prepared longer than the window", async () => { + const clock = createRateLimitClock() + const { task, dispatchedAt } = createDispatchTask(clock) + + // The agent reserves before building environment details... + await getTaskTestAccess(task).reserveProviderRateLimitSlot(0) + // ...which takes longer than the window, so a title request dispatches meanwhile. + now += 7_000 + clock.recordRequest() + const otherDispatchAt = now + + await task.attemptApiRequest(0, { skipProviderRateLimit: true }).next() + + expect(dispatchedAt).toHaveLength(1) + expect((dispatchedAt[0] ?? 0) - otherDispatchAt).toBeGreaterThanOrEqual(5_000) + }) + + it("dispatches without waiting when only its own earlier reservation is on the clock", async () => { + const clock = createRateLimitClock() + const { task, dispatchedAt } = createDispatchTask(clock) + + await getTaskTestAccess(task).reserveProviderRateLimitSlot(0) + const reservedAt = now + now += 1_000 + + await task.attemptApiRequest(0, { skipProviderRateLimit: true }).next() + + expect(mockDelay).not.toHaveBeenCalled() + expect(dispatchedAt).toEqual([reservedAt + 1_000]) + }) + + it.each([ + { path: "the rate-limited path", options: {} }, + { path: "the inherited skipProviderRateLimit path", options: { skipProviderRateLimit: true } }, + ])("waits out a request that dispatched during retry backoff on $path", async ({ options }) => { + mockProvider.getState.mockResolvedValue({ + apiConfiguration: mockApiConfig, + mcpEnabled: false, + autoApprovalEnabled: true, + requestDelaySeconds: 1, + }) + const clock = createRateLimitClock() + const { task, dispatchedAt, createMessage } = createDispatchTask(clock) + createMessage.mockImplementationOnce(() => { + dispatchedAt.push(now) + return (async function* (): AsyncGenerator { + yield* [] + throw Object.assign(new Error("server error"), { status: 500 }) + })() + }) + // A title request dispatches on the first backoff tick. + let otherDispatchAt: number | undefined + mockDelay.mockImplementation((ms: number) => { + now += ms + if (otherDispatchAt === undefined) { + clock.recordRequest() + otherDispatchAt = now + } + return Promise.resolve(undefined) + }) + + await task.attemptApiRequest(0, options).next() + + expect(dispatchedAt).toHaveLength(2) + expect(otherDispatchAt).toBeDefined() + expect((dispatchedAt[1] ?? 0) - (otherDispatchAt ?? 0)).toBeGreaterThanOrEqual(5_000) + }) + }) }) describe("Dynamic Strategy Selection", () => {