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/task/Task.ts b/src/core/task/Task.ts index aa6026e56b..d964fb19b6 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 { 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" @@ -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,106 @@ 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. + * 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) { + return + } + + const provider = this.providerRef.deref() + if (!provider || provider.context?.extensionMode === EXTENSION_MODE_TEST) { + return + } + + const controller = new AbortController() + 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, + taskId: this.taskId, + }) + + if (controller.signal.aborted || this.abort || this.abandoned) { + return + } + + // 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 + } + + try { + await provider.setGeneratedTaskTitle(this.taskId, desiredTitle) + return + } catch (error) { + console.debug( + `[Task#generateTitleInBackground] Title write attempt ${attempt} failed for ${this.taskId}:`, + error, + ) + + 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) + } + } + } + } 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 +3648,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 +3713,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 }) } @@ -5684,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 new file mode 100644 index 0000000000..87420ddb17 --- /dev/null +++ b/src/core/task/__tests__/Task.titleGeneration.spec.ts @@ -0,0 +1,382 @@ +import { describe, it, expect, vi, beforeEach } from "vitest" +import type { ProviderSettings } from "@roo-code/types" +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 +// so `generateTitleInBackground` exercises the real fallback/retry decisions. +vi.mock("../taskTitleGenerator", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + generateTaskTitle: vi.fn(), + } +}) + +// 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 + 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[] } + initiateTaskLoop: ReturnType +} + +const getTaskTitleAccess = (task: Task) => task as unknown as TaskTitleAccess + +describe("Task title generation", () => { + const mockApiConfig: ProviderSettings = { + apiProvider: providerIdentifiers.anthropic, + apiKey: "test-key", + } + + let mockSetGeneratedTaskTitle: ReturnType + let mockProvider: { + setGeneratedTaskTitle: ReturnType + postStateToWebviewWithoutTaskHistory: ReturnType + } + + beforeEach(() => { + vi.clearAllMocks() + mockSetGeneratedTaskTitle = vi.fn().mockResolvedValue(true) + mockProvider = { + setGeneratedTaskTitle: mockSetGeneratedTaskTitle, + postStateToWebviewWithoutTaskHistory: vi.fn().mockResolvedValue(undefined), + } + }) + + 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" + + Object.assign(task, { + taskId, + instanceId: "inst-1", + 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 }), + markInitialized: vi.fn(), + initiateTaskLoop: vi.fn().mockResolvedValue(undefined), + driveRunEvent: vi.fn(), + }) + + return task + } + + describe("generateTitleInBackground", () => { + it("applies the generated title through the atomic conditional write", 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: taskTitleGen.DEFAULT_TITLE_TIMEOUT_MS, + }), + ) + // 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("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(mockSetGeneratedTaskTitle).toHaveBeenCalledTimes(2) + expect(mockSetGeneratedTaskTitle).toHaveBeenLastCalledWith("task-title-1", "Summarized Title") + }) + + it("applies the truncated-prompt fallback title after the write retries are exhausted (issue #2333)", async () => { + vi.mocked(taskTitleGen.generateTaskTitle).mockResolvedValueOnce("Summarized 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") + + // 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 setGeneratedTaskTitle 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(mockSetGeneratedTaskTitle).not.toHaveBeenCalled() + }) + + it("does not call setGeneratedTaskTitle 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(mockSetGeneratedTaskTitle).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("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() + 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() + }) + + 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 new file mode 100644 index 0000000000..23187a1c51 --- /dev/null +++ b/src/core/task/__tests__/taskTitleGenerator.spec.ts @@ -0,0 +1,185 @@ +import { describe, it, expect, vi, beforeEach } from "vitest" +import type { ProviderSettings } from "@roo-code/types" +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" + +vi.mock("../../../utils/single-completion-handler", () => ({ + singleCompletionHandler: vi.fn(), +})) + +describe("taskTitleGenerator", () => { + const mockApiConfig: ProviderSettings = { + apiProvider: providerIdentifiers.anthropic, + apiKey: "test-key", + } + + beforeEach(() => { + 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 " + 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: 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 () => { + 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..e11737f0a7 --- /dev/null +++ b/src/core/task/taskTitleGenerator.ts @@ -0,0 +1,114 @@ +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 +// 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 + 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 +} + +/** + * 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. + * 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/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 6cec3bdbe7..21ba6b1c4c 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -2831,6 +2831,37 @@ export class ClineProvider } } + /* + * Applies an automatically generated title for a history item (issue #2333). + * 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() + if (!trimmed) { + return false + } + + const { applied } = await this.taskHistoryStore.setGeneratedTitleIfUnset(id, 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 6fe48fa364..3f54777c6b 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, 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}`, + }, 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")