Skip to content

Commit bf2cea1

Browse files
committed
test(task): strengthen task configuration isolation coverage
1 parent 59414bd commit bf2cea1

2 files changed

Lines changed: 37 additions & 98 deletions

File tree

src/core/task/Task.ts

Lines changed: 6 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
410410
private readonly _isHistoryTask: boolean
411411
// No streaming parser is required.
412412
assistantMessageParser?: undefined
413-
private providerProfileChangeListener?: (config: { name: string; provider?: string }) => void
414413

415414
// Native tool call streaming state (track which index each tool is at)
416415
private streamingToolCallIndices: Map<string, number> = new Map()
@@ -562,9 +561,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
562561

563562
this.messageQueueService.on("stateChanged", this.messageQueueStateChangedHandler)
564563

565-
// Listen for provider profile changes to update parser state
566-
this.setupProviderProfileChangeListener(provider)
567-
568564
// Set up diff strategy
569565
this.diffStrategy = new MultiSearchReplaceDiffStrategy(diffFuzzyThreshold)
570566

@@ -690,39 +686,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
690686
}
691687
}
692688

693-
/**
694-
* Sets up a listener for provider profile changes.
695-
*
696-
* @private
697-
* @param provider - The ClineProvider instance to listen to
698-
*/
699-
private setupProviderProfileChangeListener(provider: ClineProvider): void {
700-
// Only set up listener if provider has the on method (may not exist in test mocks)
701-
if (typeof provider.on !== "function") {
702-
return
703-
}
704-
705-
this.providerProfileChangeListener = async () => {
706-
if (provider.getCurrentTask()?.taskId !== this.taskId) {
707-
return
708-
}
709-
710-
try {
711-
const newState = await provider.getState()
712-
if (newState?.apiConfiguration) {
713-
this.updateApiConfiguration(newState.apiConfiguration)
714-
}
715-
} catch (error) {
716-
console.error(
717-
`[Task#${this.taskId}.${this.instanceId}] Failed to update API configuration on profile change:`,
718-
error,
719-
)
720-
}
721-
}
722-
723-
provider.on(RooCodeEventName.ProviderProfileChanged, this.providerProfileChangeListener)
724-
}
725-
726689
/**
727690
* Wait for the task mode to be initialized before proceeding.
728691
* This method ensures that any operations depending on the task mode
@@ -1541,7 +1504,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
15411504
if (provider) {
15421505
if (mode) {
15431506
await provider.setMode(mode)
1544-
await this.waitForModeInitialization()
15451507
this._taskMode = mode
15461508
}
15471509

@@ -1597,6 +1559,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
15971559
// Get condensing configuration
15981560
const state = await this.providerRef.deref()?.getState()
15991561
const customCondensingPrompt = state?.customSupportPrompts?.CONDENSE
1562+
// Use task-local values, not provider state, to prevent cross-task configuration leaks.
16001563
const mode = await this.getTaskMode()
16011564
const apiConfiguration = this.apiConfiguration
16021565

@@ -2284,19 +2247,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
22842247
console.error("Error cancelling current request:", error)
22852248
}
22862249

2287-
// Remove provider profile change listener
2288-
try {
2289-
if (this.providerProfileChangeListener) {
2290-
const provider = this.providerRef.deref()
2291-
if (provider) {
2292-
provider.off(RooCodeEventName.ProviderProfileChanged, this.providerProfileChangeListener)
2293-
}
2294-
this.providerProfileChangeListener = undefined
2295-
}
2296-
} catch (error) {
2297-
console.error("Error removing provider profile change listener:", error)
2298-
}
2299-
23002250
// Dispose message queue and remove event listeners.
23012251
try {
23022252
if (this.messageQueueStateChangedHandler) {
@@ -2588,7 +2538,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
25882538
const showRooIgnoredFiles = state?.showRooIgnoredFiles ?? false
25892539
const includeDiagnosticMessages = state?.includeDiagnosticMessages ?? true
25902540
const maxDiagnosticMessages = state?.maxDiagnosticMessages ?? 50
2591-
const currentMode = state?.mode ?? defaultModeSlug
2541+
const currentMode = await this.getTaskMode()
25922542

25932543
const { content: parsedUserContent, mode: slashCommandMode } = await processUserContentMentions({
25942544
userContent: currentUserContent,
@@ -3791,6 +3741,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
37913741

37923742
const { customModes, customModePrompts, customInstructions, experiments, language, enableSubfolderRules } =
37933743
state ?? {}
3744+
// Use task-local values, not provider state, to prevent cross-task configuration leaks.
37943745
const mode = await this.getTaskMode()
37953746
const apiConfiguration = this.apiConfiguration
37963747

@@ -3859,6 +3810,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
38593810
private async handleContextWindowExceededError(): Promise<void> {
38603811
const state = await this.providerRef.deref()?.getState()
38613812
const { profileThresholds = {} } = state ?? {}
3813+
// Use task-local values, not provider state, to prevent cross-task configuration leaks.
38623814
const mode = await this.getTaskMode()
38633815
const apiConfiguration = this.apiConfiguration
38643816

@@ -4039,6 +3991,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
40393991
autoCondenseContextPercent = 100,
40403992
profileThresholds = {},
40413993
} = state ?? {}
3994+
// Use task-local values, not provider state, to prevent cross-task configuration leaks.
40423995
const mode = await this.getTaskMode()
40433996
const apiConfiguration = this.apiConfiguration
40443997

@@ -4453,7 +4406,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
44534406

44544407
// Respect provider rate limit window
44554408
let rateLimitDelay = 0
4456-
const rateLimit = this.apiConfiguration?.rateLimitSeconds || 0
4409+
const rateLimit = this.apiConfiguration?.rateLimitSeconds ?? 0
44574410
const lastRequestTime = this.rateLimitClock.getLastRequestTime()
44584411
if (lastRequestTime && rateLimit > 0) {
44594412
const elapsed = performance.now() - lastRequestTime

src/core/task/__tests__/Task.spec.ts

Lines changed: 31 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { Anthropic } from "@anthropic-ai/sdk"
88

99
import {
1010
providerIdentifiers,
11-
RooCodeEventName,
1211
type GlobalState,
1312
type ProviderSettings,
1413
type ModelInfo,
@@ -488,8 +487,27 @@ describe("Cline", () => {
488487
await getTaskTestAccess(task).getSystemPrompt()
489488

490489
const systemPromptCall = requireDefined(vi.mocked(SYSTEM_PROMPT).mock.calls.at(-1))
491-
expect(systemPromptCall[5]).toBe("architect")
492-
expect(systemPromptCall[12]).toMatchObject({ todoListEnabled: true })
490+
const [, , , , , mode, , , , , , , settings] = systemPromptCall
491+
expect(mode).toBe("architect")
492+
expect(settings).toMatchObject({ todoListEnabled: true })
493+
})
494+
495+
it("uses the task mode when manually condensing after focused state changes", async () => {
496+
vi.spyOn(mockProvider, "getState").mockResolvedValue({ mode: "architect", mcpEnabled: false })
497+
const task = new Task({
498+
provider: mockProvider,
499+
apiConfiguration: mockApiConfig,
500+
task: "test task",
501+
startTask: false,
502+
})
503+
await task.getTaskMode()
504+
vi.spyOn(mockProvider, "getState").mockResolvedValue({ mode: "code", mcpEnabled: false })
505+
vi.spyOn(getTaskTestAccess(task), "getSystemPrompt").mockResolvedValue("mock system prompt")
506+
507+
await task.condenseContext()
508+
509+
const [options] = requireDefined(vi.mocked(summarizeConversation).mock.calls.at(-1))
510+
expect(options.metadata?.mode).toBe("architect")
493511
})
494512

495513
it("uses the task mode in request metadata when focused provider state differs", async () => {
@@ -527,48 +545,6 @@ describe("Cline", () => {
527545
const metadata = requireDefined(createMessage.mock.calls[0])[2]
528546
expect(metadata?.mode).toBe("ask")
529547
})
530-
531-
it("only applies profile changes to the focused task", async () => {
532-
const parentConfiguration: ProviderSettings = {
533-
...mockApiConfig,
534-
apiModelId: "parent-model",
535-
rateLimitSeconds: 4,
536-
}
537-
const childConfiguration: ProviderSettings = {
538-
...mockApiConfig,
539-
apiModelId: "child-model",
540-
rateLimitSeconds: 8,
541-
}
542-
const activeConfiguration: ProviderSettings = {
543-
...mockApiConfig,
544-
apiModelId: "active-model",
545-
rateLimitSeconds: 12,
546-
}
547-
const parent = new Task({
548-
provider: mockProvider,
549-
apiConfiguration: parentConfiguration,
550-
taskId: "parent-task",
551-
task: "parent task",
552-
startTask: false,
553-
})
554-
const child = new Task({
555-
provider: mockProvider,
556-
apiConfiguration: childConfiguration,
557-
taskId: "child-task",
558-
task: "child task",
559-
startTask: false,
560-
})
561-
vi.spyOn(mockProvider, "getCurrentTask").mockReturnValue(child)
562-
vi.spyOn(mockProvider, "getState").mockResolvedValue({ apiConfiguration: activeConfiguration })
563-
564-
mockProvider.emit(RooCodeEventName.ProviderProfileChanged, {
565-
name: "active-profile",
566-
provider: activeConfiguration.apiProvider,
567-
})
568-
569-
await vi.waitFor(() => expect(child.apiConfiguration).toEqual(activeConfiguration))
570-
expect(parent.apiConfiguration).toEqual(parentConfiguration)
571-
})
572548
})
573549

574550
describe("sayAndCreateMissingParamError", () => {
@@ -951,6 +927,13 @@ describe("Cline", () => {
951927
// finalDelay=10 and the countdown loop fires delay(1000) ten times.
952928
expect(mockDelay).toHaveBeenCalledWith(1000)
953929
expect(mockDelay).toHaveBeenCalledTimes(10)
930+
const countdownMessages = saySpy.mock.calls.filter(
931+
([type, text, , partial]) =>
932+
type === "api_req_retry_delayed" && partial && typeof text === "string",
933+
)
934+
expect(countdownMessages.map(([, text]) => text)).toEqual(
935+
Array.from({ length: 10 }, (_, index) => `API Error\n<retry_timer>${10 - index}</retry_timer>`),
936+
)
954937
expect(clock.getLastRequestTime()).toBeDefined()
955938
})
956939

@@ -2562,12 +2545,14 @@ describe("Cline", () => {
25622545
})
25632546

25642547
it("should propagate AbortController signal through attemptApiRequest context-window retry path", async () => {
2548+
vi.spyOn(mockProvider, "getState").mockResolvedValue({ mode: "architect", mcpEnabled: false })
25652549
const task = new Task({
25662550
provider: mockProvider,
25672551
apiConfiguration: mockApiConfig,
25682552
task: "test task",
25692553
startTask: false,
25702554
})
2555+
await task.getTaskMode()
25712556

25722557
vi.spyOn(getTaskTestAccess(task), "getSystemPrompt").mockResolvedValue("mock system prompt")
25732558
vi.spyOn(task, "getTokenUsage").mockReturnValue({
@@ -2662,6 +2647,7 @@ describe("Cline", () => {
26622647
expect(summarizeConversation).toHaveBeenCalled()
26632648
const [options] = vi.mocked(summarizeConversation).mock.calls.at(-1)!
26642649
expect(options.metadata?.taskId).toBe(task.taskId)
2650+
expect(options.metadata?.mode).toBe("architect")
26652651
expect(options.metadata?.abortSignal).toBeInstanceOf(AbortSignal)
26662652
expect(options.metadata?.abortSignal?.aborted).toBe(false)
26672653
})

0 commit comments

Comments
 (0)