Skip to content

Commit 6120ad9

Browse files
allquixoticclaude
andcommitted
test(concurrency): lock in per-conversation mode/API-config isolation
Add focused regression tests proving that concurrent conversations keep independent mode and provider-profile state, so a background (non-visible) running task can never pick up the foreground conversation's mode/config. Task.task-scoped-state-isolation.spec.ts: - getTaskScopedState() (the read used by the request/tool path) returns the task's own mode/apiConfiguration/currentApiConfigName, not the window-global ContextProxy values. - Two concurrent tasks each report their own scoped state from one provider. - A background task's scoped state is unaffected when the global (visible) state changes. - The provider-profile-change listener is isTaskVisible-gated: it applies for the visible task and is a no-op for background tasks. ClineProvider.concurrent-isolation.spec.ts: - handleModeSwitch on the visible task leaves a background task's _taskMode and API handler untouched. - activateProviderProfile on the visible task does not rebuild a background task's API handler or change its sticky profile name. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 348433f commit 6120ad9

2 files changed

Lines changed: 620 additions & 0 deletions

File tree

Lines changed: 296 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,296 @@
1+
// npx vitest run core/task/__tests__/Task.task-scoped-state-isolation.spec.ts
2+
//
3+
// Regression guard for per-conversation mode / API-config isolation.
4+
//
5+
// CRC runs multiple conversations concurrently. The window-global ContextProxy
6+
// state (`mode`, `currentApiConfigName`, provider settings) always reflects the
7+
// *visible* conversation. The danger is that a background (non-visible) running
8+
// task could read that global state during its request/tool loop and silently
9+
// pick up another conversation's mode or provider profile.
10+
//
11+
// These tests pin down the two invariants that prevent that leak:
12+
// 1. `getTaskScopedState()` (the state read used by the request path —
13+
// attemptApiRequest, tool gating, backoff) returns the task's OWN
14+
// snapshot (mode, apiConfiguration, currentApiConfigName) even when the
15+
// provider's global getState() reports different values.
16+
// 2. The provider-profile-change listener installed by each Task is a no-op
17+
// unless that task is the visible one, so activating a profile for the
18+
// foreground conversation does not rewrite a background task's handler.
19+
20+
import * as vscode from "vscode"
21+
22+
import type { ProviderSettings, HistoryItem } from "@roo-code/types"
23+
import { RooCodeEventName } from "@roo-code/types"
24+
25+
import { Task } from "../Task"
26+
import { ClineProvider } from "../../webview/ClineProvider"
27+
28+
vi.mock("vscode", () => {
29+
const mockDisposable = { dispose: vi.fn() }
30+
const mockEventEmitter = { event: vi.fn(), fire: vi.fn() }
31+
const mockTextDocument = { uri: { fsPath: "/mock/workspace/path/file.ts" } }
32+
const mockTextEditor = { document: mockTextDocument }
33+
const mockTab = { input: { uri: { fsPath: "/mock/workspace/path/file.ts" } } }
34+
const mockTabGroup = { tabs: [mockTab] }
35+
36+
return {
37+
TabInputTextDiff: vi.fn(),
38+
CodeActionKind: {
39+
QuickFix: { value: "quickfix" },
40+
RefactorRewrite: { value: "refactor.rewrite" },
41+
},
42+
window: {
43+
createTextEditorDecorationType: vi.fn().mockReturnValue({ dispose: vi.fn() }),
44+
visibleTextEditors: [mockTextEditor],
45+
tabGroups: {
46+
all: [mockTabGroup],
47+
close: vi.fn(),
48+
onDidChangeTabs: vi.fn(() => ({ dispose: vi.fn() })),
49+
},
50+
showErrorMessage: vi.fn(),
51+
},
52+
workspace: {
53+
getConfiguration: vi.fn(() => ({ get: (_k: string, d: any) => d })),
54+
workspaceFolders: [{ uri: { fsPath: "/mock/workspace/path" }, name: "mock-workspace", index: 0 }],
55+
createFileSystemWatcher: vi.fn(() => ({
56+
onDidCreate: vi.fn(() => mockDisposable),
57+
onDidDelete: vi.fn(() => mockDisposable),
58+
onDidChange: vi.fn(() => mockDisposable),
59+
dispose: vi.fn(),
60+
})),
61+
fs: { stat: vi.fn().mockResolvedValue({ type: 1 }) },
62+
onDidSaveTextDocument: vi.fn(() => mockDisposable),
63+
},
64+
env: { uriScheme: "vscode", language: "en" },
65+
EventEmitter: vi.fn().mockImplementation(function () {
66+
return mockEventEmitter
67+
}),
68+
Disposable: { from: vi.fn() },
69+
TabInputText: vi.fn(),
70+
version: "1.85.0",
71+
}
72+
})
73+
74+
vi.mock("../../environment/getEnvironmentDetails", () => ({
75+
getEnvironmentDetails: vi.fn().mockResolvedValue(""),
76+
}))
77+
78+
vi.mock("../../ignore/RooIgnoreController")
79+
80+
vi.mock("p-wait-for", () => ({
81+
default: vi.fn().mockImplementation(async () => Promise.resolve()),
82+
}))
83+
84+
vi.mock("delay", () => ({
85+
__esModule: true,
86+
default: vi.fn().mockResolvedValue(undefined),
87+
}))
88+
89+
const CONFIG_A: ProviderSettings = {
90+
apiProvider: "anthropic",
91+
apiModelId: "claude-3-5-sonnet-20241022",
92+
apiKey: "key-a",
93+
} as any
94+
95+
const CONFIG_B: ProviderSettings = {
96+
apiProvider: "openai",
97+
openAiApiKey: "key-b",
98+
openAiModelId: "gpt-4o",
99+
} as any
100+
101+
// The provider's window-global state. Deliberately different from BOTH tasks'
102+
// snapshots so any leak (a task reading this instead of its own snapshot) is
103+
// immediately visible in the assertions.
104+
const GLOBAL_STATE = {
105+
mode: "debug",
106+
currentApiConfigName: "global-profile",
107+
apiConfiguration: { apiProvider: "openrouter", openRouterApiKey: "global-key" } as ProviderSettings,
108+
}
109+
110+
type Handlers = Record<string, ((...args: any[]) => void) | undefined>
111+
112+
function makeProvider(handlers: Handlers, opts?: { isTaskVisible?: () => boolean; getState?: () => any }) {
113+
return {
114+
context: { globalStorageUri: { fsPath: "/test/storage" } },
115+
getState: vi.fn().mockImplementation(() => Promise.resolve(opts?.getState?.() ?? { ...GLOBAL_STATE })),
116+
log: vi.fn(),
117+
on: vi.fn().mockImplementation((event: string, cb: (...args: any[]) => void) => {
118+
handlers[event] = cb
119+
}),
120+
off: vi.fn(),
121+
isTaskVisible: vi.fn().mockImplementation(() => opts?.isTaskVisible?.() ?? false),
122+
postStateToWebview: vi.fn().mockResolvedValue(undefined),
123+
postStateToWebviewWithoutTaskHistory: vi.fn().mockResolvedValue(undefined),
124+
updateTaskHistory: vi.fn().mockResolvedValue(undefined),
125+
postTaskStateToWebview: vi.fn().mockResolvedValue(undefined),
126+
} as unknown as ClineProvider
127+
}
128+
129+
function historyItem(id: string, mode: string, apiConfigName: string): HistoryItem {
130+
return {
131+
id,
132+
number: 1,
133+
ts: Date.now(),
134+
task: `task ${id}`,
135+
tokensIn: 0,
136+
tokensOut: 0,
137+
cacheWrites: 0,
138+
cacheReads: 0,
139+
totalCost: 0,
140+
mode,
141+
apiConfigName,
142+
} as HistoryItem
143+
}
144+
145+
describe("Task - per-conversation scoped-state isolation", () => {
146+
afterEach(() => {
147+
vi.clearAllMocks()
148+
})
149+
150+
it("getTaskScopedState returns the task's own mode/apiConfiguration/apiConfigName, not the global state", async () => {
151+
const handlers: Handlers = {}
152+
const provider = makeProvider(handlers)
153+
154+
const task = new Task({
155+
provider,
156+
apiConfiguration: CONFIG_A,
157+
historyItem: historyItem("task-a", "code", "profile-a"),
158+
startTask: false,
159+
})
160+
161+
const scoped = await (task as any).getTaskScopedState()
162+
163+
// Task-scoped — must win over GLOBAL_STATE.
164+
expect(scoped.mode).toBe("code")
165+
expect(scoped.currentApiConfigName).toBe("profile-a")
166+
expect(scoped.apiConfiguration).toBe(CONFIG_A)
167+
168+
// Sanity: the global state really is different, so the assertions above
169+
// can only pass if the request path is reading task-scoped values.
170+
expect(GLOBAL_STATE.mode).not.toBe("code")
171+
expect(GLOBAL_STATE.currentApiConfigName).not.toBe("profile-a")
172+
})
173+
174+
it("two concurrent tasks each report their own scoped state from one shared provider", async () => {
175+
const handlers: Handlers = {}
176+
const provider = makeProvider(handlers)
177+
178+
const taskA = new Task({
179+
provider,
180+
apiConfiguration: CONFIG_A,
181+
historyItem: historyItem("task-a", "code", "profile-a"),
182+
startTask: false,
183+
})
184+
const taskB = new Task({
185+
provider,
186+
apiConfiguration: CONFIG_B,
187+
historyItem: historyItem("task-b", "architect", "profile-b"),
188+
startTask: false,
189+
})
190+
191+
const [scopedA, scopedB] = await Promise.all([
192+
(taskA as any).getTaskScopedState(),
193+
(taskB as any).getTaskScopedState(),
194+
])
195+
196+
expect(scopedA.mode).toBe("code")
197+
expect(scopedA.currentApiConfigName).toBe("profile-a")
198+
expect(scopedA.apiConfiguration).toBe(CONFIG_A)
199+
200+
expect(scopedB.mode).toBe("architect")
201+
expect(scopedB.currentApiConfigName).toBe("profile-b")
202+
expect(scopedB.apiConfiguration).toBe(CONFIG_B)
203+
})
204+
205+
it("a background task's scoped state is unaffected when the global (visible) state changes", async () => {
206+
const handlers: Handlers = {}
207+
// Mutable global so we can simulate the user switching mode/profile on the
208+
// foreground conversation while this task runs in the background.
209+
const mutableGlobal = { ...GLOBAL_STATE }
210+
const provider = makeProvider(handlers, { getState: () => ({ ...mutableGlobal }) })
211+
212+
const background = new Task({
213+
provider,
214+
apiConfiguration: CONFIG_A,
215+
historyItem: historyItem("bg", "code", "profile-a"),
216+
startTask: false,
217+
})
218+
219+
const before = await (background as any).getTaskScopedState()
220+
expect(before.mode).toBe("code")
221+
expect(before.currentApiConfigName).toBe("profile-a")
222+
expect(before.apiConfiguration).toBe(CONFIG_A)
223+
224+
// Foreground conversation switches mode + profile -> global state moves.
225+
mutableGlobal.mode = "ask"
226+
mutableGlobal.currentApiConfigName = "some-other-profile"
227+
mutableGlobal.apiConfiguration = { apiProvider: "bedrock" } as ProviderSettings
228+
229+
const after = await (background as any).getTaskScopedState()
230+
expect(after.mode).toBe("code")
231+
expect(after.currentApiConfigName).toBe("profile-a")
232+
expect(after.apiConfiguration).toBe(CONFIG_A)
233+
})
234+
235+
describe("provider-profile-change listener gating", () => {
236+
it("ignores ProviderProfileChanged events when the task is NOT visible", async () => {
237+
const handlers: Handlers = {}
238+
const provider = makeProvider(handlers, {
239+
isTaskVisible: () => false,
240+
getState: () => ({ ...GLOBAL_STATE }),
241+
})
242+
243+
const task = new Task({
244+
provider,
245+
apiConfiguration: CONFIG_A,
246+
historyItem: historyItem("bg", "code", "profile-a"),
247+
startTask: false,
248+
})
249+
250+
const updateSpy = vi.spyOn(task, "updateApiConfiguration")
251+
const setNameSpy = vi.spyOn(task, "setTaskApiConfigName")
252+
253+
const listener = handlers[RooCodeEventName.ProviderProfileChanged]
254+
expect(listener).toBeTypeOf("function")
255+
256+
await listener!({ name: "global-profile", provider: "openrouter" })
257+
258+
expect(updateSpy).not.toHaveBeenCalled()
259+
expect(setNameSpy).not.toHaveBeenCalled()
260+
// Background task keeps its own handler/config.
261+
expect(task.apiConfiguration).toBe(CONFIG_A)
262+
})
263+
264+
it("applies ProviderProfileChanged events when the task IS visible", async () => {
265+
const handlers: Handlers = {}
266+
const newConfig = { apiProvider: "openai", openAiApiKey: "switched", openAiModelId: "gpt-4o" }
267+
const provider = makeProvider(handlers, {
268+
isTaskVisible: () => true,
269+
getState: () => ({
270+
mode: "code",
271+
currentApiConfigName: "switched-profile",
272+
apiConfiguration: newConfig,
273+
}),
274+
})
275+
276+
const task = new Task({
277+
provider,
278+
apiConfiguration: CONFIG_A,
279+
historyItem: historyItem("fg", "code", "profile-a"),
280+
startTask: false,
281+
})
282+
283+
const updateSpy = vi.spyOn(task, "updateApiConfiguration")
284+
const setNameSpy = vi.spyOn(task, "setTaskApiConfigName")
285+
286+
const listener = handlers[RooCodeEventName.ProviderProfileChanged]
287+
expect(listener).toBeTypeOf("function")
288+
289+
await listener!({ name: "switched-profile", provider: "openai" })
290+
291+
expect(updateSpy).toHaveBeenCalledWith(newConfig)
292+
expect(setNameSpy).toHaveBeenCalledWith("switched-profile")
293+
expect(task.apiConfiguration).toBe(newConfig)
294+
})
295+
})
296+
})

0 commit comments

Comments
 (0)