From d5973566854965df7b1a7e83cbed019aba53fdc4 Mon Sep 17 00:00:00 2001 From: daewoongoh Date: Fri, 6 Nov 2026 22:04:18 +0900 Subject: [PATCH] feat(task): auto-generate session title from the first user message - Start background title generation for root tasks without blocking task loop - Support positive rateLimitSeconds settings with model title generation - Enforce 15,000 ms timeout boundary with independent test coverage - Connect title generation to task and provider disposal to prevent orphaned requests - Propagate abort signal and timeout across completion providers - Dispose stalled single-completion handlers immediately upon abortion --- src/api/providers/__tests__/openai.spec.ts | 15 + .../providers/__tests__/openrouter.spec.ts | 23 + .../base-openai-compatible-provider.ts | 8 +- src/api/providers/openai-native.ts | 8 +- src/api/providers/openai.ts | 12 +- src/api/providers/openrouter.ts | 18 +- src/core/task-persistence/TaskHistoryStore.ts | 68 +++ .../TaskHistoryStore.crossInstance.spec.ts | 98 +++ src/core/task/Task.ts | 34 +- .../task/__tests__/Task.session-title.spec.ts | 325 ++++++++++ src/core/task/__tests__/session-title.spec.ts | 569 ++++++++++++++++++ src/core/task/session-title.ts | 225 +++++++ src/core/webview/ClineProvider.ts | 46 +- .../ClineProvider.taskHistory.spec.ts | 128 ++++ src/utils/single-completion-handler.ts | 13 + 15 files changed, 1572 insertions(+), 18 deletions(-) create mode 100644 src/core/task/__tests__/Task.session-title.spec.ts create mode 100644 src/core/task/__tests__/session-title.spec.ts create mode 100644 src/core/task/session-title.ts diff --git a/src/api/providers/__tests__/openai.spec.ts b/src/api/providers/__tests__/openai.spec.ts index 7c45f22180..924b6854e0 100644 --- a/src/api/providers/__tests__/openai.spec.ts +++ b/src/api/providers/__tests__/openai.spec.ts @@ -986,6 +986,21 @@ describe("OpenAiHandler", () => { const result = await handler.completePrompt("Test prompt") expect(result).toBe("") }) + + it("forwards abort signal and timeout to the SDK request", async () => { + const controller = new AbortController() + await handler.completePrompt("Test prompt", { + abortSignal: controller.signal, + timeoutMs: 15_000, + }) + expect(mockCreate).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + signal: controller.signal, + timeout: 15_000, + }), + ) + }) }) describe.each([ diff --git a/src/api/providers/__tests__/openrouter.spec.ts b/src/api/providers/__tests__/openrouter.spec.ts index fec3e4a2e2..0c7f9860e8 100644 --- a/src/api/providers/__tests__/openrouter.spec.ts +++ b/src/api/providers/__tests__/openrouter.spec.ts @@ -999,6 +999,29 @@ describe("OpenRouterHandler", () => { ) }) + it("forwards abort signal and timeout to the SDK request", async () => { + const handler = new OpenRouterHandler(mockOptions) + const mockResponse = { choices: [{ message: { content: "test completion" } }] } + // AGENTS.md: access private members with bracket notation without as any + const createSpy = vi + .spyOn(handler["client"].chat.completions, "create") + .mockResolvedValue(mockResponse as unknown as OpenAI.Chat.Completions.ChatCompletion) + + const controller = new AbortController() + await handler.completePrompt("test prompt", { + abortSignal: controller.signal, + timeoutMs: 15_000, + }) + + expect(createSpy).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + signal: controller.signal, + timeout: 15_000, + }), + ) + }) + it("handles API errors and captures telemetry", async () => { const handler = new OpenRouterHandler(mockOptions) const mockError = { diff --git a/src/api/providers/base-openai-compatible-provider.ts b/src/api/providers/base-openai-compatible-provider.ts index b177726f85..84bc897c10 100644 --- a/src/api/providers/base-openai-compatible-provider.ts +++ b/src/api/providers/base-openai-compatible-provider.ts @@ -14,6 +14,7 @@ import { BaseProvider } from "./base-provider" import { handleOpenAIError } from "./utils/error-handler" import { calculateApiCostOpenAI } from "../../shared/cost" import { extractReasoningFromDelta } from "./utils/extract-reasoning" +import { hasUsableTimeout, toSdkTimeoutMs } from "./utils/abort-signal" type BaseOpenAiCompatibleProviderOptions = ApiHandlerOptions & { providerName: string @@ -212,7 +213,7 @@ export abstract class BaseOpenAiCompatibleProvider } } - async completePrompt(prompt: string, _options?: CompletePromptOptions): Promise { + async completePrompt(prompt: string, options?: CompletePromptOptions): Promise { const { id: modelId, info: modelInfo } = this.getModel() const params: OpenAI.Chat.Completions.ChatCompletionCreateParams = { @@ -226,7 +227,10 @@ export abstract class BaseOpenAiCompatibleProvider } try { - const response = await this.client.chat.completions.create(params) + const response = await this.client.chat.completions.create(params, { + signal: options?.abortSignal, + ...(hasUsableTimeout(options?.timeoutMs) ? { timeout: toSdkTimeoutMs(options.timeoutMs) } : {}), + }) // Check for provider-specific error responses (e.g., MiniMax base_resp) const responseAny = response as any diff --git a/src/api/providers/openai-native.ts b/src/api/providers/openai-native.ts index d6ab76a0f8..47b81f4ce2 100644 --- a/src/api/providers/openai-native.ts +++ b/src/api/providers/openai-native.ts @@ -34,6 +34,7 @@ import { NOT_PROVIDED } from "./constants" import type { SingleCompletionHandler, ApiHandlerCreateMessageMetadata, CompletePromptOptions } from "../index" import { isMcpTool } from "../../utils/mcp-name" import { sanitizeOpenAiCallId } from "../../utils/tool-id" +import { mergeAbortSignalAndTimeout } from "./utils/abort-signal" export type OpenAiNativeModel = ReturnType @@ -1643,8 +1644,13 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio } // Make the non-streaming request + const requestSignal = mergeAbortSignalAndTimeout(options?.abortSignal, options?.timeoutMs) + const combinedSignal = requestSignal + ? AbortSignal.any([this.abortController.signal, requestSignal]) + : this.abortController.signal + const response = await (this.client as any).responses.create(requestBody, { - signal: this.abortController.signal, + signal: combinedSignal, }) // Extract text from the response diff --git a/src/api/providers/openai.ts b/src/api/providers/openai.ts index 98ea18df8a..fe5bc88cc8 100644 --- a/src/api/providers/openai.ts +++ b/src/api/providers/openai.ts @@ -29,6 +29,7 @@ import { BaseProvider } from "./base-provider" import type { SingleCompletionHandler, ApiHandlerCreateMessageMetadata, CompletePromptOptions } from "../index" import { handleOpenAIError } from "./utils/error-handler" import { extractReasoningFromDelta } from "./utils/extract-reasoning" +import { hasUsableTimeout, toSdkTimeoutMs } from "./utils/abort-signal" type OpenAiUsage = | { @@ -354,7 +355,7 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl } } - async completePrompt(prompt: string, _options?: CompletePromptOptions): Promise { + async completePrompt(prompt: string, options?: CompletePromptOptions): Promise { try { const isAzureAiInference = this._isAzureAiInference(this.options.openAiBaseUrl) const model = this.getModel() @@ -372,10 +373,11 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl let response try { - response = await this.client.chat.completions.create( - requestOptions, - isAzureAiInference ? { path: OPENAI_AZURE_AI_INFERENCE_PATH } : {}, - ) + response = await this.client.chat.completions.create(requestOptions, { + signal: options?.abortSignal, + ...(hasUsableTimeout(options?.timeoutMs) ? { timeout: toSdkTimeoutMs(options.timeoutMs) } : {}), + ...(isAzureAiInference ? { path: OPENAI_AZURE_AI_INFERENCE_PATH } : {}), + }) } catch (error) { throw handleOpenAIError(error, this.providerName) } diff --git a/src/api/providers/openrouter.ts b/src/api/providers/openrouter.ts index 6d2057a7a3..da524acd67 100644 --- a/src/api/providers/openrouter.ts +++ b/src/api/providers/openrouter.ts @@ -45,6 +45,7 @@ import type { import { handleOpenAIError } from "./utils/error-handler" import { generateImageWithProvider, ImageGenerationResult } from "./utils/image-generation" import { applyRouterToolPreferences } from "./utils/router-tool-preferences" +import { hasUsableTimeout, toSdkTimeoutMs } from "./utils/abort-signal" // Add custom interface for OpenRouter params. type OpenRouterChatCompletionParams = OpenAI.Chat.ChatCompletionCreateParams & { @@ -600,7 +601,7 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH return { id, info, topP: isDeepSeekR1 ? 0.95 : undefined, ...params } } - async completePrompt(prompt: string, _options?: CompletePromptOptions) { + async completePrompt(prompt: string, options?: CompletePromptOptions) { const { id: modelId, maxTokens, temperature, reasoning } = await this.fetchModel() const completionParams: OpenRouterChatCompletionParams = { @@ -622,14 +623,21 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH } // Add Anthropic beta header for fine-grained tool streaming when using Anthropic models - const requestOptions = modelId.startsWith("anthropic/") - ? { headers: { "x-anthropic-beta": "fine-grained-tool-streaming-2025-05-14" } } - : undefined + const requestOptions: OpenAI.RequestOptions = { + ...(modelId.startsWith("anthropic/") + ? { headers: { "x-anthropic-beta": "fine-grained-tool-streaming-2025-05-14" } } + : {}), + ...(options?.abortSignal ? { signal: options.abortSignal } : {}), + ...(hasUsableTimeout(options?.timeoutMs) ? { timeout: toSdkTimeoutMs(options.timeoutMs) } : {}), + } let response try { - response = await this.client.chat.completions.create(completionParams, requestOptions) + response = await this.client.chat.completions.create( + completionParams, + Object.keys(requestOptions).length > 0 ? requestOptions : undefined, + ) } catch (error) { // Try to parse as OpenRouter error structure using Zod const parseResult = OpenRouterErrorResponseSchema.safeParse(error) diff --git a/src/core/task-persistence/TaskHistoryStore.ts b/src/core/task-persistence/TaskHistoryStore.ts index c1c50cca1e..12f772a6ee 100644 --- a/src/core/task-persistence/TaskHistoryStore.ts +++ b/src/core/task-persistence/TaskHistoryStore.ts @@ -26,6 +26,14 @@ function mergeWithDisk(delta: Partial): (existing: unknown, incomin return (existing, incoming) => mergeHistoryDelta(existing, incoming as HistoryItem, delta) } +/** Thrown inside the `setTitleIfUnset` merge callback to abort the write without touching the file. */ +// Stryker disable next-line StringLiteral: diagnostic error message only, caught internally +class TitleWriteSkipped extends Error { + constructor(readonly reason: "missing" | "titled") { + super(`title write skipped: ${reason}`) + } +} + /** * Durable intent for the one repair that spans an active delegated child and * its parent. Task files remain authoritative; this file only records the @@ -996,6 +1004,66 @@ export class TaskHistoryStore { // ────────────────────────────── Atomic read-modify-write ────────────────────────────── + /** + * Set `title` only while the item has none, checking the title on disk inside the + * per-file lock. The cache can be stale when another store (for example a second + * provider on the same storage path) saved a manual title, and a delta merge would + * otherwise overwrite it. + * + * @returns "conflict" when a title already exists or the record was deleted on disk (the cache is refreshed). + * @throws If the task ID is not present in the cache. + */ + public setTitleIfUnset(taskId: string, title: string): Promise<"applied" | "conflict"> { + return this.withLock(async () => { + const current = this.cache.get(taskId) + if (!current) { + throw new Error(`[TaskHistoryStore] setTitleIfUnset: task ${taskId} not found in cache`) + } + if (current.title) { + return "conflict" + } + + const filePath = await this.getTaskFilePath(taskId) + let written: HistoryItem = { ...current, title } + try { + await safeWriteJson(filePath, written, { + merge: (existing, incoming) => { + // Throwing aborts the write, so a deleted record is never recreated from the cache + // and an existing title is never rewritten. + // Stryker disable next-line ConditionalExpression: defense-in-depth, corrupted disk content cannot be merged + if (!existing || typeof existing !== "object" || !("id" in existing)) { + throw new TitleWriteSkipped("missing") + } + if ((existing as HistoryItem).title) { + throw new TitleWriteSkipped("titled") + } + written = mergeHistoryDelta(existing, incoming as HistoryItem, { title }) + return written + }, + }) + } catch (error) { + if (!(error instanceof TitleWriteSkipped)) { + throw error + } + if (error.reason === "missing") { + this.cache.delete(taskId) + } else { + const diskItem = await this.readTaskFile(taskId) + if (diskItem) { + this.cache.set(taskId, diskItem) + } + } + return "conflict" + } + + this.cache.set(taskId, written) + if (this.onWrite) { + await this.onWrite(this.getAll()) + } + return "applied" + }) + } + /** * Read a HistoryItem from the in-memory cache and write back an updated version, * all within a single lock acquisition so no concurrent writer can interleave diff --git a/src/core/task-persistence/__tests__/TaskHistoryStore.crossInstance.spec.ts b/src/core/task-persistence/__tests__/TaskHistoryStore.crossInstance.spec.ts index 5b85c9b4e2..98efed9a61 100644 --- a/src/core/task-persistence/__tests__/TaskHistoryStore.crossInstance.spec.ts +++ b/src/core/task-persistence/__tests__/TaskHistoryStore.crossInstance.spec.ts @@ -73,6 +73,104 @@ describe("TaskHistoryStore cross-instance safety", () => { await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => {}) }) + describe("setTitleIfUnset", () => { + it("keeps a manual title that another instance wrote after this cache was loaded", async () => { + await storeA.initialize() + await storeB.initialize() + await storeA.upsert(makeHistoryItem({ id: "title-task", task: "Original prompt" })) + await storeB.reconcile() + expect(storeB.get("title-task")?.title).toBeUndefined() + + // Instance A still caches no title when B renames the task on disk. + await storeB.atomicReadAndUpdate("title-task", (item) => ({ ...item, title: "Manual title" })) + expect(storeA.get("title-task")?.title).toBeUndefined() + + await expect(storeA.setTitleIfUnset("title-task", "Auto title")).resolves.toBe("conflict") + + expect(storeA.get("title-task")?.title).toBe("Manual title") + await storeB.reconcile() + expect(storeB.get("title-task")?.title).toBe("Manual title") + }) + + it("does not recreate a task that another instance deleted", async () => { + await storeA.initialize() + await storeB.initialize() + await storeA.upsert(makeHistoryItem({ id: "title-deleted", task: "Original prompt" })) + await storeB.reconcile() + + await storeB.delete("title-deleted") + expect(storeA.get("title-deleted")).toBeDefined() + + await expect(storeA.setTitleIfUnset("title-deleted", "Auto title")).resolves.toBe("conflict") + + expect(storeA.get("title-deleted")).toBeUndefined() + await storeB.reconcile() + expect(storeB.get("title-deleted")).toBeUndefined() + }) + + it("applies the title when no instance has set one", async () => { + await storeA.initialize() + await storeA.upsert(makeHistoryItem({ id: "title-free", task: "Original prompt" })) + + await expect(storeA.setTitleIfUnset("title-free", "Auto title")).resolves.toBe("applied") + + expect(storeA.get("title-free")).toMatchObject({ title: "Auto title", task: "Original prompt" }) + }) + + it("persists the title to disk so reloading from storage preserves it", async () => { + await storeA.initialize() + await storeA.upsert(makeHistoryItem({ id: "persist-check", task: "Original task prompt" })) + + await expect(storeA.setTitleIfUnset("persist-check", "Persisted title")).resolves.toBe("applied") + + // Reload a fresh store instance pointing to the same disk directory + const storeReloaded = new TaskHistoryStore(tmpDir) + await storeReloaded.initialize() + + const diskItem = storeReloaded.get("persist-check") + expect(diskItem).toBeDefined() + expect(diskItem?.title).toBe("Persisted title") + expect(diskItem?.task).toBe("Original task prompt") + storeReloaded.dispose() + }) + + it("throws when the task is not in the cache", async () => { + await storeA.initialize() + await expect(storeA.setTitleIfUnset("missing-id", "Auto title")).rejects.toThrow("not found in cache") + }) + + it("returns conflict immediately when cached item already has a title", async () => { + await storeA.initialize() + await storeA.upsert(makeHistoryItem({ id: "already-titled", title: "Existing", task: "Prompt" })) + await expect(storeA.setTitleIfUnset("already-titled", "New title")).resolves.toBe("conflict") + }) + + it("notifies onWrite callback when title is successfully applied", async () => { + const onWriteSpy = vi.fn().mockResolvedValue(undefined) + const storeWithOnWrite = new TaskHistoryStore(tmpDir, { onWrite: onWriteSpy }) + await storeWithOnWrite.initialize() + await storeWithOnWrite.upsert(makeHistoryItem({ id: "title-onwrite", task: "Prompt" })) + onWriteSpy.mockClear() + + await expect(storeWithOnWrite.setTitleIfUnset("title-onwrite", "Notified title")).resolves.toBe("applied") + expect(onWriteSpy).toHaveBeenCalledWith( + expect.arrayContaining([expect.objectContaining({ id: "title-onwrite", title: "Notified title" })]), + ) + storeWithOnWrite.dispose() + }) + + it("re-throws unexpected errors from safeWriteJson", async () => { + await storeA.initialize() + await storeA.upsert(makeHistoryItem({ id: "title-io-err", task: "Prompt" })) + + const { safeWriteJson } = await import("../../../utils/safeWriteJson") + const mockedSafeWrite = vi.mocked(safeWriteJson) + mockedSafeWrite.mockRejectedValueOnce(new Error("disk IO error")) + + await expect(storeA.setTitleIfUnset("title-io-err", "Auto title")).rejects.toThrow("disk IO error") + }) + }) + it("two instances can write different tasks without conflict", async () => { await storeA.initialize() await storeB.initialize() diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index b911479bc6..e3e7cd0d05 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -145,6 +145,7 @@ import { prepareApiConversationMessage } from "./apiConversationHistory" import { shouldAddUserMessageToHistory } from "./messageCounting" import { type TaskExecutionContext } from "./providerHandoff" import { initialRunState, type Ask, type RunEvent, type RunState } from "./run-state/runState" +import { generateSessionTitle, cancelSessionTitle } from "./session-title" import { applyRunEvent, type RunRejection } from "./run-state/runStateShadow" import { divergenceKey, @@ -286,6 +287,9 @@ export class PendingActionSettlementError extends Error { } } +// Literal, not the vscode enum: specs that mock "vscode" without ExtensionMode would throw on the read. +const EXTENSION_MODE_TEST = 3 + export class Task extends EventEmitter implements TaskLike { readonly taskId: string readonly rootTaskId?: string @@ -3110,6 +3114,31 @@ export class Task extends EventEmitter implements TaskLike { await this.say("text", task, images) + // Fire-and-forget session title generation: the run must never wait + // on it and no title failure may surface in the chat. Only a root + // task's first message triggers this — resumes go through + // resumeTaskFromHistory, subtask continuations re-enter the task + // loop directly, and both must keep their existing names. `say` + // above persisted the history item, so the conditional write will + // find it. + // The extra provider request would break E2E and visual tests that assert + // exact request sequences and rendered task names, so Test mode skips it. + if (!this.disposalPromise && !this.parentTask && task) { + const provider = this.providerRef.deref() + if (provider && !provider.disposed && provider.context.extensionMode !== EXTENSION_MODE_TEST) { + void generateSessionTitle({ + provider, + taskId: this.taskId, + userMessage: task, + apiConfiguration: this.apiConfiguration, + }).catch((error) => { + // generateSessionTitle never rejects by contract; this + // arm is defense-in-depth for future regressions. + console.error(`[Task#startTask] session title generation failed: ${error}`) + }) + } + } + // Check for too many MCP tools and warn the user const { enabledToolCount, enabledServerCount } = await this.getEnabledMcpToolsCount() if (enabledToolCount > MAX_MCP_TOOLS_THRESHOLD) { @@ -3695,6 +3724,7 @@ export class Task extends EventEmitter implements TaskLike { console.log(`[Task#dispose] disposing task ${this.taskId}.${this.instanceId}`) this.cancelAssistantMessagePersistence() this.cancelPendingPartialMessageUpdate() + cancelSessionTitle(this.taskId) // Release handler-owned host resources (e.g. VS Code configuration listeners). // This is the fallback for leases that never settle; later lease releases @@ -4419,7 +4449,7 @@ export class Task extends EventEmitter implements TaskLike { nativeToolCallParserScope, ) - // Stryker disable next-line ConditionalExpression, EqualityOperator: equivalent mutants, streamingKey is always defined here, so both conditions reduce to the partialToolUse check + // Stryker disable next-line ConditionalExpression,EqualityOperator: equivalent mutants, streamingKey is always defined here, so both conditions reduce to the partialToolUse check if (partialToolUse && streamingKey !== undefined) { // Reuse the compound key built above: it is exactly the key the start event // registered under, so the delta lookup needs no second encoding. @@ -4855,7 +4885,7 @@ export class Task extends EventEmitter implements TaskLike { // name (provider quirk), the compound-key lookup above misses and both the // parser entry and the dedup tracking would stay stale. Resolve the tracked // entry by id and target the real compound key for finalization and cleanup. - // Stryker disable next-line ConditionalExpression, LogicalOperator, EqualityOperator: equivalent mutants, the both-miss case is unreachable via the raw-chunk pipeline + // Stryker disable next-line ConditionalExpression,LogicalOperator,EqualityOperator: equivalent mutants, the both-miss case is unreachable via the raw-chunk pipeline if (finalToolUse === null && toolUseIndex === undefined) { const resolved = NativeToolCallParser.getStreamingToolCallById( event.id, diff --git a/src/core/task/__tests__/Task.session-title.spec.ts b/src/core/task/__tests__/Task.session-title.spec.ts new file mode 100644 index 0000000000..1d59d0fae6 --- /dev/null +++ b/src/core/task/__tests__/Task.session-title.spec.ts @@ -0,0 +1,325 @@ +// npx vitest run core/task/__tests__/Task.session-title.spec.ts + +import type { HistoryItem, ProviderSettings } from "@roo-code/types" +import { providerIdentifiers } from "@roo-code/types/provider-identifiers" + +import { Task } from "../Task" +import { ClineProvider } from "../../webview/ClineProvider" +import type { GenerateSessionTitleOptions } from "../session-title" + +vi.mock("../session-title", () => ({ + generateSessionTitle: vi.fn().mockResolvedValue(undefined), + cancelSessionTitle: vi.fn(), +})) + +// Import the mocked module for spy access. +import { generateSessionTitle, cancelSessionTitle } from "../session-title" + +vi.mock("@roo-code/telemetry", () => ({ + TelemetryService: { + hasInstance: vi.fn().mockReturnValue(true), + createInstance: vi.fn(), + get instance() { + return { + captureTaskCreated: vi.fn(), + captureTaskRestarted: vi.fn(), + captureModeSwitch: vi.fn(), + captureConversationMessage: vi.fn(), + captureLlmCompletion: vi.fn(), + captureConsecutiveMistakeError: vi.fn(), + captureCodeActionUsed: vi.fn(), + setProvider: vi.fn(), + } + }, + }, +})) + +vi.mock("vscode", () => { + const mockDisposable = { dispose: vi.fn() } + + return { + TabInputTextDiff: vi.fn(), + CodeActionKind: { + QuickFix: { value: "quickfix" }, + RefactorRewrite: { value: "refactor.rewrite" }, + }, + window: { + createTextEditorDecorationType: vi.fn().mockReturnValue({ dispose: vi.fn() }), + showErrorMessage: vi.fn(), + }, + workspace: { + getConfiguration: vi.fn(() => ({ get: (_k: string, d: unknown) => d })), + workspaceFolders: [{ uri: { fsPath: "/mock/workspace/path" }, name: "mock-workspace", index: 0 }], + createFileSystemWatcher: vi.fn(() => ({ + onDidCreate: vi.fn(() => mockDisposable), + onDidDelete: vi.fn(() => mockDisposable), + onDidChange: vi.fn(() => mockDisposable), + dispose: vi.fn(), + })), + onDidSaveTextDocument: vi.fn(() => mockDisposable), + }, + env: { uriScheme: "vscode", language: "en" }, + EventEmitter: vi.fn(), + Disposable: { from: vi.fn() }, + TabInputText: vi.fn(), + version: "1.85.0", + } +}) + +vi.mock("../../environment/getEnvironmentDetails", () => ({ + getEnvironmentDetails: vi.fn().mockResolvedValue(""), +})) + +vi.mock("../../ignore/RooIgnoreController") + +vi.mock("p-wait-for", () => ({ + default: vi.fn().mockImplementation(async () => Promise.resolve()), +})) + +vi.mock("delay", () => ({ + __esModule: true, + default: vi.fn().mockResolvedValue(undefined), +})) + +type StartTaskAccess = { + startTask: (task?: string, images?: string[]) => Promise + resumeTaskFromHistory: () => Promise + getEnabledMcpToolsCount: () => Promise<{ enabledToolCount: number; enabledServerCount: number }> + initiateTaskLoop: (userContent: unknown[]) => Promise +} + +// The private-member double assertion is the established Task-spec pattern +// (see Task.spec.ts getTaskTestAccess): the spies below must reach private +// startup collaborators without widening their production visibility. +function getStartTaskAccess(task: Task): StartTaskAccess { + return task as unknown as StartTaskAccess +} + +type ProviderStateShaped = Partial>> + +/** + * The trigger path only reads provider startup collaborators; the double is + * cast once here (AGENTS.md) naming the members tests rely on: postStateToWebviewWithoutTaskHistory, + * getState, updateTaskHistory, log, on, off. + */ +function createMockProvider(disposed = false): ClineProvider { + return { + disposed, + context: { globalStorageUri: { fsPath: "/test/storage" } }, + getState: vi.fn().mockResolvedValue({} as ProviderStateShaped), + log: vi.fn(), + on: vi.fn(), + off: vi.fn(), + postStateToWebview: vi.fn().mockResolvedValue(undefined), + postStateToWebviewWithoutTaskHistory: vi.fn().mockResolvedValue(undefined), + updateTaskHistory: vi.fn().mockResolvedValue(undefined), + } as unknown as ClineProvider +} + +function createTestTask(provider: ClineProvider, parentTask?: Task, historyItem?: HistoryItem): Task { + const apiConfiguration: ProviderSettings = { + apiProvider: providerIdentifiers.anthropic, + apiModelId: "claude-3-5-sonnet-20241022", + apiKey: "test-api-key", + } + + return new Task({ + provider, + apiConfiguration, + // A history item must be the only task source or it is not a history + // task; omit the task text for that case. + task: historyItem ? undefined : "example root task", + parentTask, + historyItem, + startTask: false, + }) +} + +function stubStartup(task: Task): void { + vi.spyOn(task, "say").mockResolvedValue(undefined) + const taskAccess = getStartTaskAccess(task) + vi.spyOn(taskAccess, "getEnabledMcpToolsCount").mockResolvedValue({ enabledToolCount: 0, enabledServerCount: 0 }) + vi.spyOn(taskAccess, "initiateTaskLoop").mockResolvedValue(undefined) +} + +function lastTitleCall(): GenerateSessionTitleOptions | undefined { + const calls = vi.mocked(generateSessionTitle).mock.calls + return calls.length > 0 ? calls[calls.length - 1][0] : undefined +} + +describe("Task#startTask session title trigger", () => { + beforeEach(() => { + vi.mocked(generateSessionTitle).mockClear() + vi.mocked(cancelSessionTitle).mockClear() + }) + + it("fires title generation for a root task without awaiting it", async () => { + const provider = createMockProvider() + const task = createTestTask(provider) + stubStartup(task) + + let resolved = false + vi.mocked(generateSessionTitle).mockImplementation(async () => { + resolved = true + }) + + await getStartTaskAccess(task).startTask("title trigger message") + + const call = lastTitleCall() + expect(call).toBeDefined() + expect(call?.taskId).toBe(task.taskId) + expect(call?.userMessage).toBe("title trigger message") + expect(call?.provider).toBe(provider) + // The trigger is fire-and-forget: startTask resolved and the generation + // ran to completion without startTask ever awaiting it beyond its own + // synchronous promise chain. + expect(resolved).toBe(true) + }) + + it("starts title generation only after the first message is stored", async () => { + const provider = createMockProvider() + const task = createTestTask(provider) + stubStartup(task) + + let releaseSay: () => void = () => {} + vi.spyOn(task, "say").mockImplementation( + () => + new Promise((resolve) => { + releaseSay = () => resolve(undefined) + }), + ) + + const started = getStartTaskAccess(task).startTask("stored first") + await Promise.resolve() + expect(vi.mocked(generateSessionTitle)).not.toHaveBeenCalled() + + releaseSay() + await started + + expect(lastTitleCall()?.userMessage).toBe("stored first") + }) + + it("does not wait for a pending title before starting the task loop", async () => { + const provider = createMockProvider() + const task = createTestTask(provider) + stubStartup(task) + + let releaseTitle: () => void = () => {} + vi.mocked(generateSessionTitle).mockImplementation( + () => + new Promise((resolve) => { + releaseTitle = resolve + }), + ) + + // startTask resolving while the title promise is still pending proves it is not awaited. + await getStartTaskAccess(task).startTask("slow title message") + + expect(getStartTaskAccess(task).initiateTaskLoop).toHaveBeenCalled() + expect(lastTitleCall()?.userMessage).toBe("slow title message") + releaseTitle() + }) + + it.each([ + ["Production", 1, true], + ["Development", 2, true], + ["Test", 3, false], + ])("in %s extension mode fires title generation: %s", async (_name, extensionMode, shouldFire) => { + const provider = createMockProvider() + // The double only carries the context members the trigger reads. + Object.assign(provider.context, { extensionMode }) + const task = createTestTask(provider) + stubStartup(task) + + await getStartTaskAccess(task).startTask("mode gated message") + + expect(vi.mocked(generateSessionTitle).mock.calls.length > 0).toBe(shouldFire) + }) + + it("does not fire for a subtask", async () => { + const provider = createMockProvider() + const parentTask = createTestTask(provider) + const childTask = createTestTask(provider, parentTask) + stubStartup(childTask) + + await getStartTaskAccess(childTask).startTask("child task message") + + expect(vi.mocked(generateSessionTitle)).not.toHaveBeenCalled() + }) + + it("does not fire when the root message is empty", async () => { + const provider = createMockProvider() + const task = createTestTask(provider) + stubStartup(task) + + await getStartTaskAccess(task).startTask(undefined) + + expect(vi.mocked(generateSessionTitle)).not.toHaveBeenCalled() + }) + + it("routes a history task away from startTask, so resumes never trigger", async () => { + const provider = createMockProvider() + const historyItem: HistoryItem = { + id: "resume-title-task", + number: 3, + task: "historical task", + ts: Date.now() - 60_000, + tokensIn: 10, + tokensOut: 5, + totalCost: 0.01, + } + // A history item with no task text marks the task as a history task, so + // run() routes through resumeTaskFromHistory and never reaches the + // startTask trigger (Task.ts `_isHistoryTask` routing). + const task = createTestTask(provider, undefined, historyItem) + const taskAccess = getStartTaskAccess(task) + const startTaskSpy = vi.spyOn(taskAccess, "startTask") + vi.spyOn(taskAccess, "resumeTaskFromHistory").mockResolvedValue(undefined) + + await task.run() + + expect(startTaskSpy).not.toHaveBeenCalled() + expect(vi.mocked(generateSessionTitle)).not.toHaveBeenCalled() + }) + + it("keeps the run alive when generation rejects", async () => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + const provider = createMockProvider() + const task = createTestTask(provider) + stubStartup(task) + vi.mocked(generateSessionTitle).mockRejectedValue(new Error("boom")) + + await expect(getStartTaskAccess(task).startTask("survives rejection")).resolves.toBeUndefined() + expect(consoleErrorSpy).toHaveBeenCalledWith( + expect.stringContaining("[Task#startTask] session title generation failed: Error: boom"), + ) + consoleErrorSpy.mockRestore() + }) + + it("cancels in-flight session title generation when task is disposed", async () => { + const provider = createMockProvider() + const task = createTestTask(provider) + await task.dispose() + expect(vi.mocked(cancelSessionTitle)).toHaveBeenCalledWith(task.taskId) + }) + + it("does not fire session title generation if task is already disposed", async () => { + const provider = createMockProvider() + const task = createTestTask(provider) + stubStartup(task) + await task.dispose() + vi.mocked(generateSessionTitle).mockClear() + + await getStartTaskAccess(task).startTask("message after disposal") + expect(vi.mocked(generateSessionTitle)).not.toHaveBeenCalled() + }) + + it("does not fire session title generation if provider is disposed", async () => { + const provider = createMockProvider(true) + const task = createTestTask(provider) + stubStartup(task) + + await getStartTaskAccess(task).startTask("message with disposed provider") + expect(vi.mocked(generateSessionTitle)).not.toHaveBeenCalled() + }) +}) diff --git a/src/core/task/__tests__/session-title.spec.ts b/src/core/task/__tests__/session-title.spec.ts new file mode 100644 index 0000000000..847edeee21 --- /dev/null +++ b/src/core/task/__tests__/session-title.spec.ts @@ -0,0 +1,569 @@ +// npx vitest run core/task/__tests__/session-title.spec.ts + +import { providerIdentifiers, type ProviderSettings } from "@roo-code/types" + +import type { CompletePromptOptions } from "../../../api" +import type { ClineProvider } from "../../webview/ClineProvider" +import { + FALLBACK_TITLE_MAX_CHARS, + SESSION_TITLE_TIMEOUT_MS, + fallbackTitle, + generateSessionTitle, + cancelSessionTitle, + sanitizeUserMessageForTitle, + extractLlmTitle, + type SessionTitleApplyResult, +} from "../session-title" + +type CompletePromptCall = { + prompt: string + apiConfiguration: ProviderSettings + options: CompletePromptOptions | undefined +} + +type ApplyTitleCall = { taskId: string; title: string } + +/** + * The orchestration only needs the one provider method it calls. The double is + * typed structurally and cast once at the construction site (AGENTS.md): the + * test never reads other provider members. + */ +function createTitleProvider( + overrides: { + applySessionTitle?: (taskId: string, title: string) => Promise + disposed?: boolean + } = {}, +): ClineProvider { + const provider = { + disposed: overrides.disposed ?? false, + applySessionTitle: overrides.applySessionTitle ?? (async () => "applied" as const), + } + return provider as unknown as ClineProvider +} + +function createCompletePromptRecorder(impl: (call: CompletePromptCall) => Promise): { + fn: (config: ProviderSettings, prompt: string, options?: CompletePromptOptions) => Promise + calls: CompletePromptCall[] +} { + const calls: CompletePromptCall[] = [] + const fn = (apiConfiguration: ProviderSettings, prompt: string, options: CompletePromptCall["options"]) => { + const call = { prompt, apiConfiguration, options } + calls.push(call) + return impl(call) + } + return { fn, calls } +} + +const testApiConfiguration: ProviderSettings = { + apiProvider: providerIdentifiers.anthropic, + apiModelId: "claude-3-5-sonnet-20241022", + apiKey: "test-api-key", +} + +describe("sanitizeUserMessageForTitle", () => { + it.each([ + ["plain message", "plain message"], + [" padded ", "padded"], + ["before hidden after", "before after"], + ["a\nbreal", "real"], + ["start unterminated", "start"], + ["only system", ""], + ["axbyc", "abc"], + ["keep stray close", "keep stray close"], + ])("sanitizes %j", (input, expected) => { + expect(sanitizeUserMessageForTitle(input)).toBe(expected) + }) + + it("handles many unterminated openings in linear time", () => { + const input = "".repeat(50_000) + const startedAt = performance.now() + + expect(sanitizeUserMessageForTitle(input)).toBe("") + expect(performance.now() - startedAt).toBeLessThan(500) + }) +}) + +describe("extractLlmTitle", () => { + it.each([ + ["Fix login bug in auth flow", "Fix login bug in auth flow"], + ["Fix Title: parsing in task history", "Fix Title: parsing in task history"], + ['Fix "quoted" search terms', 'Fix "quoted" search terms'], + ['"Leading quote only', "Leading quote only"], + ['Trailing quote only"', "Trailing quote only"], + ['"""Multiple quoted"""', "Multiple quoted"], + ["' Spaced quotes '", "Spaced quotes"], + ["Title: Fix login bug", "Fix login bug"], + ["title: lowercase prefix", "lowercase prefix"], + ['"Quoted title"', "Quoted title"], + ["'Single quoted'", "Single quoted"], + ["“Curly quoted”", "Curly quoted"], + ["First line\nSecond line", "First line"], + ['Title: "Nested cleanup"', "Nested cleanup"], + ])("extracts %j", (input, expected) => { + expect(extractLlmTitle(input)).toBe(expected) + }) + + it.each([[""], [" "], ["\n\n"], ['""'], ["Title:"]])("returns undefined for %j", (input) => { + expect(extractLlmTitle(input)).toBeUndefined() + }) +}) + +describe("fallbackTitle", () => { + it.each([ + ["\n\nsecond\nthird", "second third"], + ["Fix\nlogin bug", "Fix login bug"], + [" leading space ", "leading space"], + ])("collapses whitespace in %j", (input, expected) => { + expect(fallbackTitle(input)).toBe(expected) + }) + + it("cuts at the character limit before collapsing whitespace", () => { + const input = "a".repeat(FALLBACK_TITLE_MAX_CHARS - 2) + " ".repeat(5) + "bcd" + expect(fallbackTitle(input)).toBe("a".repeat(FALLBACK_TITLE_MAX_CHARS - 2)) + expect(fallbackTitle("ab" + " ".repeat(60) + "cd")).toBe("ab") + }) + + it("slices the first 42 characters before trimming or collapsing leading whitespace", () => { + const input = " ".repeat(40) + "ABCDEF" + expect(fallbackTitle(input)).toBe("AB") + }) + + it("caps the fallback at the character limit", () => { + const long = "a".repeat(FALLBACK_TITLE_MAX_CHARS + 10) + expect(fallbackTitle(long)).toBe("a".repeat(FALLBACK_TITLE_MAX_CHARS)) + expect(fallbackTitle(long)).toHaveLength(FALLBACK_TITLE_MAX_CHARS) + }) + + it.each([[""], [" \n \t"]])("returns undefined for %j", (input) => { + expect(fallbackTitle(input)).toBeUndefined() + }) +}) + +describe("generateSessionTitle", () => { + it("applies the cleaned LLM title", async () => { + const recorder = createCompletePromptRecorder(async () => 'Title: "Fix Login Bug"\nextra') + const applied: ApplyTitleCall[] = [] + const provider = createTitleProvider({ + applySessionTitle: async (taskId, title) => { + applied.push({ taskId, title }) + return "applied" + }, + }) + + await generateSessionTitle({ + provider, + taskId: "task-1", + userMessage: "please fix the login bug", + apiConfiguration: testApiConfiguration, + completePrompt: recorder.fn, + }) + + expect(recorder.calls).toHaveLength(1) + expect(recorder.calls[0].prompt).toContain("please fix the login bug") + expect(recorder.calls[0].prompt).toContain("Create a descriptive 5-10 word title") + expect(recorder.calls[0].options?.taskId).toBe("task-1") + expect(recorder.calls[0].options?.timeoutMs).toBe(15_000) + expect(recorder.calls[0].apiConfiguration).toBe(testApiConfiguration) + expect(applied).toEqual([{ taskId: "task-1", title: "Fix Login Bug" }]) + }) + + it("falls back to the start of the message when the LLM fails", async () => { + const recorder = createCompletePromptRecorder(async () => { + throw new Error("api down") + }) + const applied: ApplyTitleCall[] = [] + const provider = createTitleProvider({ + applySessionTitle: async (taskId, title) => { + applied.push({ taskId, title }) + return "applied" + }, + }) + + await generateSessionTitle({ + provider, + taskId: "task-2", + userMessage: "first line of the request\nsecond line", + apiConfiguration: testApiConfiguration, + completePrompt: recorder.fn, + }) + + expect(applied).toEqual([{ taskId: "task-2", title: "first line of the request second line" }]) + }) + + it.each([[""], [" "], ['""'], ["Title:"]])( + "falls back to the message start when the LLM returns the unusable response %j", + async (response) => { + const recorder = createCompletePromptRecorder(async () => response) + const applied: ApplyTitleCall[] = [] + const provider = createTitleProvider({ + applySessionTitle: async (taskId, title) => { + applied.push({ taskId, title }) + return "applied" + }, + }) + + await generateSessionTitle({ + provider, + taskId: "task-unusable", + userMessage: "Short first line\nwith more text after it that goes on and on", + apiConfiguration: testApiConfiguration, + completePrompt: recorder.fn, + }) + + expect(applied).toEqual([{ taskId: "task-unusable", title: "Short first line with more text after it t" }]) + }, + ) + + it("falls back when the LLM times out", async () => { + vi.useFakeTimers() + try { + const recorder = createCompletePromptRecorder(() => new Promise(() => {})) + const applied: ApplyTitleCall[] = [] + const provider = createTitleProvider({ + applySessionTitle: async (taskId, title) => { + applied.push({ taskId, title }) + return "applied" + }, + }) + + const generation = generateSessionTitle({ + provider, + taskId: "task-timeout", + userMessage: "timeout message body", + apiConfiguration: testApiConfiguration, + completePrompt: recorder.fn, + }) + // Drain microtasks so the raced completion settles first. + await vi.advanceTimersByTimeAsync(SESSION_TITLE_TIMEOUT_MS + 1) + await generation + + expect(recorder.calls[0].options?.abortSignal?.aborted).toBe(true) + expect(applied).toEqual([{ taskId: "task-timeout", title: "timeout message body" }]) + } finally { + vi.useRealTimers() + } + }) + + it("applies the fallback title precisely at the 15,000 ms timeout boundary without exceeding it", async () => { + vi.useFakeTimers() + try { + const recorder = createCompletePromptRecorder(() => new Promise(() => {})) + const applied: ApplyTitleCall[] = [] + const provider = createTitleProvider({ + applySessionTitle: async (taskId, title) => { + applied.push({ taskId, title }) + return "applied" + }, + }) + + const generation = generateSessionTitle({ + provider, + taskId: "task-timeout-boundary", + userMessage: "boundary timeout user message", + apiConfiguration: testApiConfiguration, + completePrompt: recorder.fn, + }) + + // 14,999 ms: timeout has not triggered yet + await vi.advanceTimersByTimeAsync(14_999) + expect(recorder.calls[0].options?.abortSignal?.aborted).toBe(false) + expect(applied).toHaveLength(0) + + // 15,000 ms: exact independent timeout boundary triggers fallback + await vi.advanceTimersByTimeAsync(1) + await generation + + expect(recorder.calls[0].options?.abortSignal?.aborted).toBe(true) + expect(applied).toEqual([{ taskId: "task-timeout-boundary", title: "boundary timeout user message" }]) + } finally { + vi.useRealTimers() + } + }) + + it.each([[""], [" "], ["only system content"]])( + "skips everything for sanitized-empty message %j", + async (userMessage) => { + const recorder = createCompletePromptRecorder(async () => "should not be called") + const provider = createTitleProvider() + + await generateSessionTitle({ + provider, + taskId: "task-skip", + userMessage, + apiConfiguration: testApiConfiguration, + completePrompt: recorder.fn, + }) + + expect(recorder.calls).toHaveLength(0) + }, + ) + + it("stops immediately on conflict so a manual title always wins", async () => { + const recorder = createCompletePromptRecorder(async () => "Generated Title") + let applyCalls = 0 + const provider = createTitleProvider({ + applySessionTitle: async () => { + applyCalls++ + return "conflict" + }, + }) + + await generateSessionTitle({ + provider, + taskId: "task-conflict", + userMessage: "some root message", + apiConfiguration: testApiConfiguration, + completePrompt: recorder.fn, + }) + + expect(applyCalls).toBe(1) + }) + + it("retries a failing write up to the retry cap, then gives up", async () => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + const recorder = createCompletePromptRecorder(async () => "Generated Title") + let applyCalls = 0 + const provider = createTitleProvider({ + applySessionTitle: async () => { + applyCalls++ + throw new Error("disk on fire") + }, + }) + + await generateSessionTitle({ + provider, + taskId: "task-retry", + userMessage: "some root message", + apiConfiguration: testApiConfiguration, + completePrompt: recorder.fn, + }) + + // 4 attempts for LLM title + 4 attempts for fallback title = 8 total attempts. + expect(applyCalls).toBe(8) + expect(consoleErrorSpy).toHaveBeenCalledWith( + expect.stringContaining("[session-title] giving up on task task-retry after 4 write attempts:"), + expect.any(Error), + ) + consoleErrorSpy.mockRestore() + }) + + it("attempts the fallback title when LLM title write retries fail", async () => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + const recorder = createCompletePromptRecorder(async () => "A".repeat(500)) + const applied: ApplyTitleCall[] = [] + const provider = createTitleProvider({ + applySessionTitle: async (taskId, title) => { + if (title.length > 50) { + throw new Error("payload too large") + } + applied.push({ taskId, title }) + return "applied" + }, + }) + + await generateSessionTitle({ + provider, + taskId: "task-llm-write-fail", + userMessage: "Fix login issue with auth flow", + apiConfiguration: testApiConfiguration, + completePrompt: recorder.fn, + }) + + expect(applied).toEqual([{ taskId: "task-llm-write-fail", title: "Fix login issue with auth flow" }]) + consoleErrorSpy.mockRestore() + }) + + it("recovers when a retried write succeeds", async () => { + const recorder = createCompletePromptRecorder(async () => "Generated Title") + let applyCalls = 0 + const provider = createTitleProvider({ + applySessionTitle: async () => { + applyCalls++ + if (applyCalls < 3) { + throw new Error("transient") + } + return "applied" + }, + }) + + await generateSessionTitle({ + provider, + taskId: "task-retry-recover", + userMessage: "some root message", + apiConfiguration: testApiConfiguration, + completePrompt: recorder.fn, + }) + + expect(applyCalls).toBe(3) + }) + + it("runs only one generation per task at a time", async () => { + let release: (() => void) | undefined + const gate = new Promise((resolve) => { + release = resolve + }) + const recorder = createCompletePromptRecorder(async () => { + await gate + return "First Title" + }) + const applied: ApplyTitleCall[] = [] + const provider = createTitleProvider({ + applySessionTitle: async (taskId, title) => { + applied.push({ taskId, title }) + return "applied" + }, + }) + + const first = generateSessionTitle({ + provider, + taskId: "task-dedup", + userMessage: "root message", + apiConfiguration: testApiConfiguration, + completePrompt: recorder.fn, + }) + const second = generateSessionTitle({ + provider, + taskId: "task-dedup", + userMessage: "root message", + apiConfiguration: testApiConfiguration, + completePrompt: recorder.fn, + }) + release!() + await Promise.all([first, second]) + + expect(applied).toHaveLength(1) + expect(applied[0].title).toBe("First Title") + }) + + it("aborts in-flight generation and does not apply title when cancelled", async () => { + let abortSignal: AbortSignal | undefined + let resolveCompletion!: (val: string) => void + const completionPromise = new Promise((resolve) => { + resolveCompletion = resolve + }) + const recorder = createCompletePromptRecorder(async ({ options }) => { + abortSignal = options?.abortSignal + return completionPromise + }) + const applied: ApplyTitleCall[] = [] + const provider = createTitleProvider({ + applySessionTitle: async (taskId, title) => { + applied.push({ taskId, title }) + return "applied" + }, + }) + + const generation = generateSessionTitle({ + provider, + taskId: "task-cancel", + userMessage: "User prompt to name", + apiConfiguration: testApiConfiguration, + completePrompt: recorder.fn, + }) + + expect(recorder.calls).toHaveLength(1) + expect(abortSignal?.aborted).toBe(false) + + cancelSessionTitle("task-cancel") + + expect(abortSignal?.aborted).toBe(true) + resolveCompletion("Generated title") + await generation + + expect(applied).toHaveLength(0) + }) + + it("does not start generation or apply title when provider is disposed", async () => { + const recorder = createCompletePromptRecorder(async () => "Generated Title") + const applied: ApplyTitleCall[] = [] + const provider = createTitleProvider({ + disposed: true, + applySessionTitle: async (taskId, title) => { + applied.push({ taskId, title }) + return "applied" + }, + }) + + await generateSessionTitle({ + provider, + taskId: "task-disposed", + userMessage: "some message", + apiConfiguration: testApiConfiguration, + completePrompt: recorder.fn, + }) + + expect(recorder.calls).toHaveLength(0) + expect(applied).toHaveLength(0) + }) + + it("generates a short model title for new root tasks with a positive rateLimitSeconds setting", async () => { + const recorder = createCompletePromptRecorder(async () => "Generated Title Under Rate Limit") + const applied: ApplyTitleCall[] = [] + const provider = createTitleProvider({ + applySessionTitle: async (taskId, title) => { + applied.push({ taskId, title }) + return "applied" + }, + }) + + await generateSessionTitle({ + provider, + taskId: "task-ratelimit-active", + userMessage: "Fix urgent production issue in login flow", + apiConfiguration: { ...testApiConfiguration, rateLimitSeconds: 5 }, + completePrompt: recorder.fn, + }) + + expect(recorder.calls).toHaveLength(1) + expect(applied).toEqual([{ taskId: "task-ratelimit-active", title: "Generated Title Under Rate Limit" }]) + }) + + it("stops silently when cancelled during write retries", async () => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + const recorder = createCompletePromptRecorder(async () => "Generated Title") + let applyCalls = 0 + const provider = createTitleProvider({ + applySessionTitle: async () => { + applyCalls++ + cancelSessionTitle("task-cancel-retry") + throw new Error("transient failure") + }, + }) + + await generateSessionTitle({ + provider, + taskId: "task-cancel-retry", + userMessage: "some root message", + apiConfiguration: testApiConfiguration, + completePrompt: recorder.fn, + }) + + expect(applyCalls).toBe(1) + expect(consoleErrorSpy).not.toHaveBeenCalled() + consoleErrorSpy.mockRestore() + }) + + it("safely ignores cancellation for an untracked task", () => { + expect(() => cancelSessionTitle("non-existent-task")).not.toThrow() + }) + + it("never rejects even when everything fails", async () => { + const provider = createTitleProvider({ + applySessionTitle: async () => { + throw new Error("write failed") + }, + }) + + await expect( + generateSessionTitle({ + provider, + taskId: "task-quiet", + userMessage: "root message", + apiConfiguration: testApiConfiguration, + completePrompt: async () => { + throw new Error("llm failed") + }, + }), + ).resolves.toBeUndefined() + }) +}) diff --git a/src/core/task/session-title.ts b/src/core/task/session-title.ts new file mode 100644 index 0000000000..674c0737a5 --- /dev/null +++ b/src/core/task/session-title.ts @@ -0,0 +1,225 @@ +import type { ProviderSettings } from "@roo-code/types" + +import type { ClineProvider } from "../webview/ClineProvider" +import { singleCompletionHandler } from "../../utils/single-completion-handler" + +/** Hard bound on the single-shot title request (15 seconds). */ +export const SESSION_TITLE_TIMEOUT_MS = 15_000 +/** Character cap for the fallback title taken from the user message (42 chars). */ +export const FALLBACK_TITLE_MAX_CHARS = 42 +/** Write attempts given up on after this many retries (3 retries). */ +const TITLE_WRITE_MAX_RETRIES = 3 + +// Stryker disable next-line StringLiteral: diagnostic label, does not alter control flow +const TITLE_PROMPT_PREFIX = `Create a descriptive 5-10 word title for the user message below. Use the user's language. +Output only the title. + +` + +/** Outcome of `ClineProvider.applySessionTitle`. */ +export type SessionTitleApplyResult = "applied" | "conflict" + +/** Removes system-injected `` blocks and trims. */ +export function sanitizeUserMessageForTitle(raw: string): string { + const open = "" + const close = "" + let result = "" + let cursor = 0 + // Stryker disable next-line EqualityOperator: cursor reaches raw.length on trailing text; <= is equivalent as indexOf returns -1 at length + while (cursor < raw.length) { + const start = raw.indexOf(open, cursor) + if (start === -1) { + result += raw.slice(cursor) + break + } + result += raw.slice(cursor, start) + // Stryker disable next-line ArithmeticOperator: equivalent mutant, (18 chars) cannot start in the [start-17, start+17] window around (17 chars) + const end = raw.indexOf(close, start + open.length) + // An unterminated block would otherwise leak the rest of the message as user text; drop it. + if (end === -1) { + break + } + cursor = end + close.length + } + return result.trim() +} + +/** First line, `Title:` prefix removed, surrounding quotes removed. */ +export function extractLlmTitle(raw: string): string | undefined { + const firstLine = raw.split("\n")[0].trim() + const withoutPrefix = firstLine.replace(/^title:\s*/i, "") + const withoutQuotes = withoutPrefix.replace(/^["'“”‘’]+|["'“”‘’]+$/g, "") + const title = withoutQuotes.trim() + return title.length > 0 ? title : undefined +} + +/** First 42 characters of the message, then whitespace collapsed. */ +export function fallbackTitle(rawMessage: string): string | undefined { + const head = rawMessage.slice(0, FALLBACK_TITLE_MAX_CHARS) + return head.replace(/\s+/g, " ").trim() || undefined +} + +type CompletePromptFn = typeof singleCompletionHandler + +export interface GenerateSessionTitleOptions { + provider: ClineProvider + taskId: string + /** The raw root user message that started the run. */ + userMessage: string + /** The API configuration the session itself uses (same model reuse). */ + apiConfiguration: ProviderSettings + /** Injectable for tests; defaults to the shared single-shot completion helper. */ + completePrompt?: CompletePromptFn +} + +interface InFlightTitle { + controller: AbortController + userCancelled: boolean +} + +/** In-flight title generation abort controllers by task ID. */ +const inFlightTitles = new Map() + +/** + * Cancels any pending or in-flight title generation for a task. + * Called when a task is manually renamed, cleared, or deleted so an auto-generated title + * never overwrites the user's manual intent. + */ +export function cancelSessionTitle(taskId: string): void { + const entry = inFlightTitles.get(taskId) + if (entry) { + entry.userCancelled = true + entry.controller.abort() + } +} + +/** + * Generates and applies a session title from the root user message. + * + * Fire-and-forget by contract: never rejects, never blocks a run, and every + * failure degrades quietly (fallback title, or no write at all). A manual + * rename always wins over the generated title. + */ +export async function generateSessionTitle(options: GenerateSessionTitleOptions): Promise { + const { provider, taskId, userMessage, apiConfiguration, completePrompt = singleCompletionHandler } = options + + if (provider.disposed) { + return + } + + const sanitized = sanitizeUserMessageForTitle(userMessage) + if (!sanitized) { + return + } + + if (inFlightTitles.has(taskId)) { + return + } + const inFlight: InFlightTitle = { + controller: new AbortController(), + userCancelled: false, + } + inFlightTitles.set(taskId, inFlight) + + try { + let title: string | undefined + if (!inFlight.userCancelled) { + try { + title = await completeTitleViaLlm( + completePrompt, + apiConfiguration, + taskId, + sanitized, + inFlight.controller, + ) + } catch (error) { + // LLM failure, cancellation, or timeout falls back to the message's first line. + if (!inFlight.userCancelled) { + console.error(`[session-title] LLM call failed for task ${taskId}:`, error) + } + } + } + let applied = false + if (title) { + applied = await applyTitleWithRetry(provider, taskId, title, () => inFlight.userCancelled) + } + if (!applied && !inFlight.userCancelled) { + const fallback = fallbackTitle(userMessage) + if (fallback && fallback !== title) { + await applyTitleWithRetry(provider, taskId, fallback, () => inFlight.userCancelled) + } + } + } catch (error) { + // Best-effort by design: a failed title must never surface in the chat. + // Stryker disable next-line BlockStatement,ConditionalExpression,StringLiteral: defense-in-depth, applyTitleWithRetry handles all expected errors internally + if (!inFlight.userCancelled) { + console.error(`[session-title] generation failed for task ${taskId}:`, error) + } + } finally { + inFlightTitles.delete(taskId) + } +} + +async function completeTitleViaLlm( + completePrompt: CompletePromptFn, + apiConfiguration: ProviderSettings, + taskId: string, + sanitized: string, + controller: AbortController, +): Promise { + let timeoutId: ReturnType | undefined + try { + const completion = completePrompt(apiConfiguration, TITLE_PROMPT_PREFIX + sanitized, { + taskId, + timeoutMs: SESSION_TITLE_TIMEOUT_MS, + abortSignal: controller.signal, + }) + const timeout = new Promise((_, reject) => { + timeoutId = setTimeout(() => { + controller.abort() + reject(new Error(`[session-title] timed out after ${SESSION_TITLE_TIMEOUT_MS}ms`)) + }, SESSION_TITLE_TIMEOUT_MS) + }) + // Stryker disable next-line BlockStatement,ConditionalExpression,StringLiteral: equivalent mutant, abort handler covers both immediate and event-driven rejections + const abortPromise = new Promise((_, reject) => { + if (controller.signal.aborted) { + reject(new Error("[session-title] aborted")) + } else { + controller.signal.addEventListener("abort", () => reject(new Error("[session-title] aborted")), { + once: true, + }) + } + }) + const raw = await Promise.race([completion, timeout, abortPromise]) + return extractLlmTitle(raw) + } finally { + clearTimeout(timeoutId) + } +} + +async function applyTitleWithRetry( + provider: ClineProvider, + taskId: string, + title: string, + isCancelled: () => boolean, +): Promise { + for (let attempt = 0; ; attempt++) { + if (isCancelled() || provider.disposed) { + return false + } + try { + await provider.applySessionTitle(taskId, title) + return true + } catch (error) { + if (isCancelled() || provider.disposed || attempt >= TITLE_WRITE_MAX_RETRIES) { + if (!isCancelled() && !provider.disposed) { + console.error( + `[session-title] giving up on task ${taskId} after ${attempt + 1} write attempts:`, + error, + ) + } + return false + } + } + } +} diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 8fff156425..1941157f2d 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -76,6 +76,7 @@ import { providerIdentifiers, } from "@roo-code/types" import { RateLimitClock, createRateLimitClock } from "../task/RateLimitClock" +import { cancelSessionTitle, type SessionTitleApplyResult } from "../task/session-title" import { TaskRegistry } from "../task/TaskRegistry" import { TaskScheduler } from "../task/TaskScheduler" import { @@ -272,6 +273,9 @@ export class ClineProvider private taskEventListeners: WeakMap void>> = new WeakMap() private currentWorkspacePath: string | undefined private _disposed = false + public get disposed(): boolean { + return this._disposed + } private lastWebviewHeartbeatAt = 0 // Bumped on every heartbeat or grace reset, including events in the same // millisecond; pending recovery must yield to either sign of renewed grace. @@ -2809,6 +2813,7 @@ export class ClineProvider * clears it so the history falls back to the original prompt. */ async renameTaskWithId(id: string, title: string | undefined) { + cancelSessionTitle(id) if (!this.taskHistoryStore.get(id)) { this.log(`[renameTaskWithId] task ${id} not found in history`) return @@ -2825,10 +2830,43 @@ export class ClineProvider if (this.isViewLaunched) { const updatedItem = this.taskHistoryStore.get(id) + // Stryker disable next-line ConditionalExpression: equivalent mutant, setTitleIfUnset guarantees the item is in the cache + if (updatedItem) { + await this.postMessageToWebview({ type: "taskHistoryItemUpdated", taskHistoryItem: updatedItem }) + } + } + } + + /* + * Applies an auto-generated session title, but only while the item still + * carries its default name. The title check runs inside the store's + * lock and re-reads the title on disk, so a manual rename that lands while a title is + * generating always wins and is never overwritten. Returns "conflict" + * when nothing was applied (an existing title won, or the title was + * blank); throws on transient write failures so the caller can retry. + */ + async applySessionTitle(id: string, title: string): Promise { + const trimmed = title.trim() + if (!trimmed) { + return "conflict" + } + + const result = await this.taskHistoryStore.setTitleIfUnset(id, trimmed) + if (result === "conflict") { + return "conflict" + } + + this.recentTasksCache = undefined + + if (this.isViewLaunched) { + const updatedItem = this.taskHistoryStore.get(id) + // Stryker disable next-line ConditionalExpression: equivalent mutant, setTitleIfUnset guarantees the item is in the cache if (updatedItem) { await this.postMessageToWebview({ type: "taskHistoryItemUpdated", taskHistoryItem: updatedItem }) } } + + return "applied" } /* Condenses a task's message history to use fewer tokens. */ @@ -2844,6 +2882,7 @@ export class ClineProvider // this function deletes a task from task history, and deletes its checkpoints and delete the task folder // If the task has subtasks (childIds), they will also be deleted recursively async deleteTaskWithId(id: string, cascadeSubtasks: boolean = true) { + cancelSessionTitle(id) try { await this.getTaskWithId(id) @@ -3637,9 +3676,10 @@ export class ClineProvider * Updates a task in the task history and optionally broadcasts the updated history to the webview. * Now delegates to TaskHistoryStore for per-task file persistence. * - * This writer never owns the `title` field: `title` is written only by the rename path - * (`renameTaskWithId` via `taskHistoryStore.atomicReadAndUpdate`). Callers here build items - * from snapshots read before taking the store lock, so any title they carry can be stale. + * This writer never owns the `title` field: `title` is written only by the title-owning paths + * (`renameTaskWithId` via `atomicReadAndUpdate` and `applySessionTitle` via `setTitleIfUnset`). + * Callers here build items from snapshots read before taking the store lock, so any title they + * carry can be stale. * * @param item The history item to update or add * @param options.broadcast Whether to broadcast the updated history to the webview (default: true) diff --git a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts index 7b6223e2e2..a0910800e0 100644 --- a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts @@ -11,6 +11,8 @@ import { ContextProxy } from "../../config/ContextProxy" import { Task } from "../../task/Task" import { ProfileValidator } from "../../../shared/ProfileValidator" import { ClineProvider } from "../ClineProvider" +import { generateSessionTitle } from "../../task/session-title" +import * as sessionTitleModule from "../../task/session-title" // Mock setup vi.mock("p-wait-for", () => ({ @@ -653,6 +655,35 @@ describe("ClineProvider Task History Synchronization", () => { expect(getStored("task-race")).toMatchObject({ title: "Renamed", tokensIn: 500 }) }) + it("cancels in-flight session title generation and preserves a manual title clear", async () => { + await provider.updateTaskHistory(createHistoryItem({ id: "task-manual-clear", task: "Original prompt" }), { + broadcast: false, + }) + let resolveCompletion!: (title: string) => void + const completionPromise = new Promise((resolve) => { + resolveCompletion = resolve + }) + const generation = generateSessionTitle({ + provider, + taskId: "task-manual-clear", + userMessage: "Original prompt", + apiConfiguration: { + apiProvider: providerIdentifiers.anthropic, + apiModelId: "claude-3-5-sonnet-20241022", + apiKey: "test-api-key", + }, + completePrompt: async () => completionPromise, + }) + + await provider.renameTaskWithId("task-manual-clear", "") + expect(getStored("task-manual-clear")?.title).toBeUndefined() + + resolveCompletion("Generated Title") + await generation + + expect(getStored("task-manual-clear")?.title).toBeUndefined() + }) + it("does not write a title through a full-item update", async () => { await provider.updateTaskHistory( createHistoryItem({ id: "task-title-owner", task: "Original prompt", title: "Stale title" }), @@ -690,6 +721,103 @@ describe("ClineProvider Task History Synchronization", () => { }) }) + describe("applySessionTitle", () => { + const getStored = (id: string) => provider.taskHistoryStore.get(id) + + it("stores a trimmed title and broadcasts the updated item", async () => { + await provider.resolveWebviewView(mockWebviewView) + provider.isViewLaunched = true + await provider.updateTaskHistory(createHistoryItem({ id: "task-auto-title", task: "Original prompt" })) + mockPostMessage.mockClear() + + const result = await provider.applySessionTitle("task-auto-title", " Auto Title ") + + expect(result).toBe("applied") + expect(getStored("task-auto-title")).toMatchObject({ title: "Auto Title", task: "Original prompt" }) + const updates = findCallsByType(mockPostMessage.mock.calls, "taskHistoryItemUpdated") + expect(updates.at(-1)?.[0].taskHistoryItem).toMatchObject({ id: "task-auto-title", title: "Auto Title" }) + }) + + it("returns conflict and keeps the existing title when one is already set", async () => { + await provider.updateTaskHistory(createHistoryItem({ id: "task-auto-conflict", task: "Original prompt" }), { + broadcast: false, + }) + await provider.renameTaskWithId("task-auto-conflict", "Manual title") + + const result = await provider.applySessionTitle("task-auto-conflict", "Auto Title") + + expect(result).toBe("conflict") + expect(getStored("task-auto-conflict")).toMatchObject({ title: "Manual title" }) + }) + + it.each([ + ["an empty string", ""], + ["whitespace", " "], + ])("returns conflict without writing for %s", async (_label, title) => { + await provider.updateTaskHistory(createHistoryItem({ id: "task-auto-blank", task: "Original prompt" }), { + broadcast: false, + }) + + const result = await provider.applySessionTitle("task-auto-blank", title) + + expect(result).toBe("conflict") + expect(getStored("task-auto-blank")?.title).toBeUndefined() + }) + + it("throws for an unknown task so the caller can give up", async () => { + await expect(provider.applySessionTitle("missing-task", "Auto Title")).rejects.toThrow("not found in cache") + }) + + it("keeps the auto title when the task later saves metadata without one", async () => { + const item = createHistoryItem({ id: "task-auto-keep", task: "Original prompt", status: "active" }) + await provider.updateTaskHistory(item, { broadcast: false }) + await provider.applySessionTitle("task-auto-keep", "Auto Title") + + // Mirrors Task.saveClineMessages(), whose taskMetadata() result never carries a title. + await provider.updateTaskHistory({ ...item, tokensIn: 500 }, { broadcast: false }) + + expect(getStored("task-auto-keep")).toMatchObject({ title: "Auto Title", tokensIn: 500 }) + }) + + it("keeps a concurrent task save that is queued before the auto title", async () => { + const item = createHistoryItem({ id: "task-auto-race", task: "Original prompt", status: "active" }) + await provider.updateTaskHistory(item, { broadcast: false }) + + // Start the save without awaiting it, so the auto title runs while the save is queued. + const save = provider.updateTaskHistory({ ...item, tokensIn: 500 }, { broadcast: false }) + const autoTitle = provider.applySessionTitle("task-auto-race", "Auto Title") + await Promise.all([save, autoTitle]) + + expect(getStored("task-auto-race")).toMatchObject({ title: "Auto Title", tokensIn: 500 }) + }) + + it("does not broadcast when the view is not launched", async () => { + await provider.resolveWebviewView(mockWebviewView) + provider.isViewLaunched = false + await provider.updateTaskHistory(createHistoryItem({ id: "task-auto-silent", task: "Original prompt" }), { + broadcast: false, + }) + mockPostMessage.mockClear() + + const result = await provider.applySessionTitle("task-auto-silent", "Auto Title") + + expect(result).toBe("applied") + expect(getStored("task-auto-silent")).toMatchObject({ title: "Auto Title" }) + expect(findCallsByType(mockPostMessage.mock.calls, "taskHistoryItemUpdated")).toHaveLength(0) + }) + + it("cancels pending session title generation when deleting a task", async () => { + const cancelSpy = vi.spyOn(sessionTitleModule, "cancelSessionTitle") + try { + await provider.deleteTaskWithId("task-cancel-on-delete") + } catch { + // Task doesn't exist on disk, delete will catch/proceed + } + expect(cancelSpy).toHaveBeenCalledWith("task-cancel-on-delete") + cancelSpy.mockRestore() + }) + }) + describe("broadcastTaskHistoryUpdate", () => { it("sends taskHistoryUpdated message with sorted history", async () => { await provider.resolveWebviewView(mockWebviewView) diff --git a/src/utils/single-completion-handler.ts b/src/utils/single-completion-handler.ts index 148f440461..45e9cff9c3 100644 --- a/src/utils/single-completion-handler.ts +++ b/src/utils/single-completion-handler.ts @@ -20,6 +20,16 @@ export async function singleCompletionHandler( const handler = buildApiHandler(apiConfiguration) + let onAbort: (() => void) | undefined + if (options?.abortSignal) { + onAbort = () => disposeApiHandler(handler) + if (options.abortSignal.aborted) { + disposeApiHandler(handler) + } else { + options.abortSignal.addEventListener("abort", onAbort, { once: true }) + } + } + try { // Check if handler supports single completions if (!("completePrompt" in handler)) { @@ -28,6 +38,9 @@ export async function singleCompletionHandler( return await (handler as SingleCompletionHandler).completePrompt(promptText, options) } finally { + if (onAbort && options?.abortSignal) { + options.abortSignal.removeEventListener("abort", onAbort) + } // Handlers such as VS Code LM / GitHub Copilot hold listeners that must be released. disposeApiHandler(handler) }