diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 4ba2996c91..87d2ed0bec 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -410,7 +410,6 @@ export class Task extends EventEmitter implements TaskLike { private readonly _isHistoryTask: boolean // No streaming parser is required. assistantMessageParser?: undefined - private providerProfileChangeListener?: (config: { name: string; provider?: string }) => void // Native tool call streaming state (track which index each tool is at) private streamingToolCallIndices: Map = new Map() @@ -562,9 +561,6 @@ export class Task extends EventEmitter implements TaskLike { this.messageQueueService.on("stateChanged", this.messageQueueStateChangedHandler) - // Listen for provider profile changes to update parser state - this.setupProviderProfileChangeListener(provider) - // Set up diff strategy this.diffStrategy = new MultiSearchReplaceDiffStrategy(diffFuzzyThreshold) @@ -690,35 +686,6 @@ export class Task extends EventEmitter implements TaskLike { } } - /** - * Sets up a listener for provider profile changes. - * - * @private - * @param provider - The ClineProvider instance to listen to - */ - private setupProviderProfileChangeListener(provider: ClineProvider): void { - // Only set up listener if provider has the on method (may not exist in test mocks) - if (typeof provider.on !== "function") { - return - } - - this.providerProfileChangeListener = async () => { - try { - const newState = await provider.getState() - if (newState?.apiConfiguration) { - this.updateApiConfiguration(newState.apiConfiguration) - } - } catch (error) { - console.error( - `[Task#${this.taskId}.${this.instanceId}] Failed to update API configuration on profile change:`, - error, - ) - } - } - - provider.on(RooCodeEventName.ProviderProfileChanged, this.providerProfileChangeListener) - } - /** * Wait for the task mode to be initialized before proceeding. * This method ensures that any operations depending on the task mode @@ -1537,6 +1504,7 @@ export class Task extends EventEmitter implements TaskLike { if (provider) { if (mode) { await provider.setMode(mode) + this._taskMode = mode } if (providerProfile) { @@ -1545,6 +1513,7 @@ export class Task extends EventEmitter implements TaskLike { // Update this task's API configuration to match the new profile // This ensures the parser state is synchronized with the selected model const newState = await provider.getState() + this.setTaskApiConfigName(newState?.currentApiConfigName ?? providerProfile) if (newState?.apiConfiguration) { this.updateApiConfiguration(newState.apiConfiguration) } @@ -1591,7 +1560,9 @@ export class Task extends EventEmitter implements TaskLike { // Get condensing configuration const state = await this.providerRef.deref()?.getState() const customCondensingPrompt = state?.customSupportPrompts?.CONDENSE - const { mode, apiConfiguration } = state ?? {} + // Use task-local values, not provider state, to prevent cross-task configuration leaks. + const mode = await this.getTaskMode() + const apiConfiguration = this.apiConfiguration const { contextTokens: prevContextTokens } = this.getTokenUsage() @@ -2277,19 +2248,6 @@ export class Task extends EventEmitter implements TaskLike { console.error("Error cancelling current request:", error) } - // Remove provider profile change listener - try { - if (this.providerProfileChangeListener) { - const provider = this.providerRef.deref() - if (provider) { - provider.off(RooCodeEventName.ProviderProfileChanged, this.providerProfileChangeListener) - } - this.providerProfileChangeListener = undefined - } - } catch (error) { - console.error("Error removing provider profile change listener:", error) - } - // Dispose message queue and remove event listeners. try { if (this.messageQueueStateChangedHandler) { @@ -2581,7 +2539,7 @@ export class Task extends EventEmitter implements TaskLike { const showRooIgnoredFiles = state?.showRooIgnoredFiles ?? false const includeDiagnosticMessages = state?.includeDiagnosticMessages ?? true const maxDiagnosticMessages = state?.maxDiagnosticMessages ?? 50 - const currentMode = state?.mode ?? defaultModeSlug + const currentMode = await this.getTaskMode() const { content: parsedUserContent, mode: slashCommandMode } = await processUserContentMentions({ userContent: currentUserContent, @@ -3782,16 +3740,11 @@ export class Task extends EventEmitter implements TaskLike { const state = await this.providerRef.deref()?.getState() - const { - mode, - customModes, - customModePrompts, - customInstructions, - experiments, - language, - apiConfiguration, - enableSubfolderRules, - } = state ?? {} + const { customModes, customModePrompts, customInstructions, experiments, language, enableSubfolderRules } = + state ?? {} + // Use task-local values, not provider state, to prevent cross-task configuration leaks. + const mode = await this.getTaskMode() + const apiConfiguration = this.apiConfiguration return await (async () => { const provider = this.providerRef.deref() @@ -3857,7 +3810,10 @@ export class Task extends EventEmitter implements TaskLike { private async handleContextWindowExceededError(): Promise { const state = await this.providerRef.deref()?.getState() - const { profileThresholds = {}, mode, apiConfiguration } = state ?? {} + const { profileThresholds = {} } = state ?? {} + // Use task-local values, not provider state, to prevent cross-task configuration leaks. + const mode = await this.getTaskMode() + const apiConfiguration = this.apiConfiguration const { contextTokens } = this.getTokenUsage() await this.safeEnsureModelFetched() @@ -3997,9 +3953,7 @@ export class Task extends EventEmitter implements TaskLike { * the `api_req_rate_limit_wait` say type (not an error). */ private async maybeWaitForProviderRateLimit(retryAttempt: number): Promise { - const state = await this.providerRef.deref()?.getState() - const rateLimitSeconds = - state?.apiConfiguration?.rateLimitSeconds ?? this.apiConfiguration?.rateLimitSeconds ?? 0 + const rateLimitSeconds = this.apiConfiguration?.rateLimitSeconds ?? 0 const lastRequestTime = this.rateLimitClock.getLastRequestTime() if (rateLimitSeconds <= 0 || !lastRequestTime) { @@ -4032,14 +3986,15 @@ export class Task extends EventEmitter implements TaskLike { const state = await this.providerRef.deref()?.getState() const { - apiConfiguration, autoApprovalEnabled, requestDelaySeconds, - mode, autoCondenseContext = true, autoCondenseContextPercent = 100, profileThresholds = {}, } = state ?? {} + // Use task-local values, not provider state, to prevent cross-task configuration leaks. + const mode = await this.getTaskMode() + const apiConfiguration = this.apiConfiguration // Get condensing configuration for automatic triggers. const customCondensingPrompt = state?.customSupportPrompts?.CONDENSE @@ -4452,7 +4407,7 @@ export class Task extends EventEmitter implements TaskLike { // Respect provider rate limit window let rateLimitDelay = 0 - const rateLimit = (state?.apiConfiguration ?? this.apiConfiguration)?.rateLimitSeconds || 0 + const rateLimit = this.apiConfiguration?.rateLimitSeconds ?? 0 const lastRequestTime = this.rateLimitClock.getLastRequestTime() if (lastRequestTime && rateLimit > 0) { const elapsed = performance.now() - lastRequestTime diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 330241e221..21038887d6 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -16,6 +16,7 @@ import { import { TelemetryService } from "@roo-code/telemetry" import { Task } from "../Task" +import { SYSTEM_PROMPT } from "../../prompts/system" import { createRateLimitClock } from "../RateLimitClock" import { summarizeConversation } from "../../condense" import { ClineProvider } from "../../webview/ClineProvider" @@ -223,6 +224,15 @@ vi.mock("../../condense", async (importOriginal) => { }), } }) + +vi.mock("../../prompts/system", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + SYSTEM_PROMPT: vi.fn(actual.SYSTEM_PROMPT), + } +}) + // Mock storagePathManager to prevent dynamic import issues. vi.mock("../../../utils/storage", () => ({ getTaskDirectoryPath: vi @@ -451,6 +461,92 @@ describe("Cline", () => { }) }) + describe("task-local configuration isolation", () => { + it("uses the task mode and API configuration when focused provider state differs", async () => { + const taskApiConfiguration: ProviderSettings = { + ...mockApiConfig, + todoListEnabled: true, + } + vi.spyOn(mockProvider, "getState").mockResolvedValue({ mode: "architect", mcpEnabled: false }) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: taskApiConfiguration, + task: "test task", + startTask: false, + }) + await task.getTaskMode() + + vi.spyOn(mockProvider, "getState").mockResolvedValue({ + mode: "code", + mcpEnabled: false, + apiConfiguration: { ...mockApiConfig, todoListEnabled: false }, + }) + vi.mocked(SYSTEM_PROMPT).mockResolvedValueOnce("mock system prompt") + + await getTaskTestAccess(task).getSystemPrompt() + + const systemPromptCall = requireDefined(vi.mocked(SYSTEM_PROMPT).mock.calls.at(-1)) + const [, , , , , mode, , , , , , , settings] = systemPromptCall + expect(mode).toBe("architect") + expect(settings).toMatchObject({ todoListEnabled: true }) + }) + + it("uses the task mode when manually condensing after focused state changes", async () => { + vi.spyOn(mockProvider, "getState").mockResolvedValue({ mode: "architect", mcpEnabled: false }) + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + await task.getTaskMode() + vi.spyOn(mockProvider, "getState").mockResolvedValue({ mode: "code", mcpEnabled: false }) + vi.spyOn(getTaskTestAccess(task), "getSystemPrompt").mockResolvedValue("mock system prompt") + + await task.condenseContext() + + const [options] = requireDefined(vi.mocked(summarizeConversation).mock.calls.at(-1)) + expect(options.metadata?.mode).toBe("architect") + }) + + it("uses the task mode in request metadata when focused provider state differs", async () => { + vi.spyOn(mockProvider, "getState").mockResolvedValue({ + mode: "ask", + mcpEnabled: false, + autoApprovalEnabled: true, + requestDelaySeconds: 0, + }) + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + await task.getTaskMode() + vi.spyOn(getTaskTestAccess(task), "getSystemPrompt").mockResolvedValue("mock system prompt") + + vi.spyOn(mockProvider, "getState").mockResolvedValue({ + mode: "code", + mcpEnabled: false, + autoApprovalEnabled: true, + requestDelaySeconds: 0, + }) + const stream = (async function* () { + yield { type: "text", text: "response" } as ApiStreamChunk + })() + const createMessage = vi.spyOn(task.api, "createMessage").mockReturnValue(stream) + task.apiConversationHistory = [ + { role: "user", content: [{ type: "text", text: "test message" }], ts: Date.now() }, + ] + + await task.attemptApiRequest().next() + + const metadata = requireDefined(createMessage.mock.calls[0])[2] + expect(metadata?.mode).toBe("ask") + }) + }) + describe("sayAndCreateMissingParamError", () => { it("surfaces a localized error notice and returns the missing-parameter tool error for both relPath branches", async () => { const cline = new Task({ @@ -750,7 +846,7 @@ describe("Cline", () => { expect(mockDelay).toHaveBeenCalledWith(1000) }) - it("should respect rate limit window in retry backoff", async () => { + it("uses the task rate limit in retry backoff when focused provider state differs", async () => { const clock = createRateLimitClock() const rateLimitConfig = { ...mockApiConfig, @@ -815,7 +911,10 @@ describe("Cline", () => { const providerState = await mockProvider.getState() vi.spyOn(mockProvider, "getState").mockResolvedValue({ ...providerState, - apiConfiguration: rateLimitConfig, + apiConfiguration: { + ...mockApiConfig, + rateLimitSeconds: 1, + }, autoApprovalEnabled: true, requestDelaySeconds: 3, }) @@ -823,10 +922,18 @@ describe("Cline", () => { const iterator = cline.attemptApiRequest(0) await iterator.next() - // rateLimitSeconds=10 > exponentialDelay=ceil(3*2^0)=3, so + // The task rateLimitSeconds=10 (rather than the focused provider's 1) + // exceeds exponentialDelay=ceil(3*2^0)=3, so // finalDelay=10 and the countdown loop fires delay(1000) ten times. expect(mockDelay).toHaveBeenCalledWith(1000) expect(mockDelay).toHaveBeenCalledTimes(10) + const countdownMessages = saySpy.mock.calls.filter( + ([type, text, , partial]) => + type === "api_req_retry_delayed" && partial && typeof text === "string", + ) + expect(countdownMessages.map(([, text]) => text)).toEqual( + Array.from({ length: 10 }, (_, index) => `API Error\n${10 - index}`), + ) expect(clock.getLastRequestTime()).toBeDefined() }) @@ -1574,6 +1681,57 @@ describe("Cline", () => { expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() }) + it("uses a mode selected through submitUserMessage in the next API request", async () => { + vi.spyOn(mockProvider, "getState").mockResolvedValue({ mode: "ask", mcpEnabled: false }) + vi.spyOn(mockProvider, "setMode").mockResolvedValue(undefined) + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "initial task", + startTask: false, + }) + vi.spyOn(task, "handleWebviewAskResponse").mockImplementation(() => {}) + + await task.submitUserMessage("switch modes", undefined, "code") + vi.spyOn(getTaskTestAccess(task), "getSystemPrompt").mockResolvedValue("mock system prompt") + const stream = (async function* () { + yield { type: "text", text: "response" } as ApiStreamChunk + })() + const createMessage = vi.spyOn(task.api, "createMessage").mockReturnValue(stream) + task.apiConversationHistory = [ + { role: "user", content: [{ type: "text", text: "test message" }], ts: Date.now() }, + ] + + await task.attemptApiRequest().next() + + expect(mockProvider.setMode).toHaveBeenCalledWith("code") + expect(requireDefined(createMessage.mock.calls[0])[2]?.mode).toBe("code") + }) + + it("stores a provider profile selected through submitUserMessage", async () => { + const selectedConfiguration: ProviderSettings = { + ...mockApiConfig, + apiModelId: "selected-model", + } + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "initial task", + startTask: false, + }) + task.setTaskApiConfigName("previous-profile") + vi.spyOn(mockProvider, "setProviderProfile").mockResolvedValue(undefined) + vi.spyOn(mockProvider, "getState").mockResolvedValue({ + currentApiConfigName: "selected-profile", + apiConfiguration: selectedConfiguration, + }) + vi.spyOn(task, "handleWebviewAskResponse").mockImplementation(() => {}) + + await task.submitUserMessage("switch profiles", undefined, undefined, "selected-profile") + + expect(task.taskApiConfigName).toBe("selected-profile") + }) + it("should handle empty messages gracefully", async () => { const task = new Task({ provider: mockProvider, @@ -2411,12 +2569,14 @@ describe("Cline", () => { }) it("should propagate AbortController signal through attemptApiRequest context-window retry path", async () => { + vi.spyOn(mockProvider, "getState").mockResolvedValue({ mode: "architect", mcpEnabled: false }) const task = new Task({ provider: mockProvider, apiConfiguration: mockApiConfig, task: "test task", startTask: false, }) + await task.getTaskMode() vi.spyOn(getTaskTestAccess(task), "getSystemPrompt").mockResolvedValue("mock system prompt") vi.spyOn(task, "getTokenUsage").mockReturnValue({ @@ -2511,6 +2671,7 @@ describe("Cline", () => { expect(summarizeConversation).toHaveBeenCalled() const [options] = vi.mocked(summarizeConversation).mock.calls.at(-1)! expect(options.metadata?.taskId).toBe(task.taskId) + expect(options.metadata?.mode).toBe("architect") expect(options.metadata?.abortSignal).toBeInstanceOf(AbortSignal) expect(options.metadata?.abortSignal?.aborted).toBe(false) })