Skip to content

Commit fe0dfd1

Browse files
committed
fix(webview): isolate parallel mode and provider profile writes
Port vps2 F3 (mode/profile writes) from upstream 978, hunk-by-hunk against the F1c..CS residual: - ClineProvider: add repointPersistedViewStates() to re-point currentApiConfigName across per-view entries when a profile is renamed or replaced, and prune orphaned entries; validate handleModeSwitch slugs against the custom-modes manager and no-op with a log on unknown modes; drop the as-any cast in delegateParentAndOpenChild. - Task: route mode switches through provider.handleModeSwitch(task) and keep the submitted message on failure instead of setMode(). - SwitchModeTool and specs: durable per-view mode writes. - webviewMessageHandler: no change vs base - the kimi-code OAuth hunk in the residual is CS-only divergence (not-ported register item 1), not part of #978/#979. - webviewMessageHandler.spec: ported only #979's 4 mock fields + defaultModeSlug import; the stack-side legacy-repair test, Key-aware getValue mock and em-dash comment are retained (register item 4). #979's mode-routing WMH.spec describe ("routes mode messages through handleModeSwitch instead of writing ContextProxy directly") exists in neither CS nor the stack and is ported by no unit (open question, logged). - Tests: H3/H4 durable handleModeSwitch writes in ClineProvider.spec.ts; profile-mutation, profile-activation and handleModeSwitch-integration describes (incl. A4 non-focused-target regression and new mutation-killing tests) in ClineProvider.parallelMode.spec.ts; sticky-mode and webviewMessageHandler spec updates; retain the setViewStateId __proto__ guard + spec test - shipped F1a hardening; the residual's guard removal is lineage divergence, not F3 content. - eslint-suppressions.json: no-explicit-any counts decrease for core/webview/ClineProvider.ts (12 -> 11) and core/webview/__tests__/ClineProvider.sticky-mode.spec.ts (36 -> 33). Upstream: #978 (vps2 F3) - issue #978; content ported hunk-by-hunk from the F1c..CS residual, cross-checked against upstream PR
1 parent 530d4f6 commit fe0dfd1

10 files changed

Lines changed: 970 additions & 65 deletions

‎src/core/task/Task.ts‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1790,8 +1790,26 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
17901790

17911791
if (provider) {
17921792
if (mode) {
1793-
await provider.setMode(mode)
1794-
this._taskMode = mode
1793+
// Route through the shared mode-switch handler so the switch is
1794+
// validated and recorded like any other mode change (task history,
1795+
// TaskModeSwitched, and — when this is the focused task — the view's
1796+
// durable mode pin + ModeChanged broadcast). The handler writes this
1797+
// task's mode only after validation and persistence, so an unknown
1798+
// slug leaves the task mode untouched instead of recording a bad one.
1799+
// A mode-switch failure (e.g. a task-history write failure) must not
1800+
// swallow the submitted message: log it locally and continue delivery.
1801+
// Let the constructor-started mode initialization settle first: its
1802+
// deferred provider-state read would otherwise resolve after the switch
1803+
// and clobber the explicitly selected mode with the pre-switch value.
1804+
try {
1805+
await this.waitForModeInitialization()
1806+
await provider.handleModeSwitch(mode, this)
1807+
} catch (error) {
1808+
console.error(
1809+
`[Task#submitUserMessage] Mode switch to ${mode} failed (taskId=${this.taskId}):`,
1810+
error,
1811+
)
1812+
}
17951813
}
17961814

17971815
if (providerProfile) {

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

Lines changed: 80 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ type TaskTestAccess = {
4242
safeEnsureModelFetched: () => Promise<void>
4343
addToApiConversationHistory: (message: unknown, reasoning?: string) => Promise<void>
4444
resetAssistantMessagePersistence: () => void
45+
// Private on Task; the provider-owned mode write (ClineProvider.handleModeSwitch)
46+
// sets it, and tests mirror that write through this helper.
47+
_taskMode: string | undefined
4548
}
4649

4750
type TaskAskResult = Awaited<ReturnType<Task["ask"]>>
@@ -1412,6 +1415,7 @@ describe("Cline", () => {
14121415
getMcpHub: vi.fn().mockReturnValue(undefined),
14131416
getSkillsManager: vi.fn().mockReturnValue(undefined),
14141417
say: vi.fn(),
1418+
handleModeSwitch: vi.fn().mockResolvedValue(undefined),
14151419
postStateToWebview: vi.fn().mockResolvedValue(undefined),
14161420
postStateToWebviewWithoutTaskHistory: vi.fn().mockResolvedValue(undefined),
14171421
postStateToWebviewThrottled: vi.fn().mockResolvedValue(undefined),
@@ -1967,7 +1971,13 @@ describe("Cline", () => {
19671971
mode: "ask",
19681972
mcpEnabled: false,
19691973
} as unknown as ProviderState)
1970-
vi.spyOn(mockProvider, "setMode").mockResolvedValue(undefined)
1974+
vi.spyOn(mockProvider, "handleModeSwitch").mockImplementation(async (mode, targetTask) => {
1975+
// Mirror ClineProvider.handleModeSwitch: after validation and persistence
1976+
// the provider owns the task's mode write.
1977+
if (targetTask) {
1978+
getTaskTestAccess(targetTask)._taskMode = mode
1979+
}
1980+
})
19711981
const task = new Task({
19721982
provider: mockProvider,
19731983
apiConfiguration: mockApiConfig,
@@ -1976,6 +1986,10 @@ describe("Cline", () => {
19761986
})
19771987
vi.spyOn(task, "handleWebviewAskResponse").mockImplementation(() => {})
19781988

1989+
// Let the task's initial mode ("ask", from provider state) settle first, so the
1990+
// mode selected with the user message is the task's final mode write.
1991+
await task.getTaskMode()
1992+
19791993
await task.submitUserMessage("switch modes", undefined, "code")
19801994
vi.spyOn(getTaskTestAccess(task), "getSystemPrompt").mockResolvedValue("mock system prompt")
19811995
const stream = (async function* () {
@@ -1988,10 +2002,74 @@ describe("Cline", () => {
19882002

19892003
await task.attemptApiRequest().next()
19902004

1991-
expect(mockProvider.setMode).toHaveBeenCalledWith("code")
2005+
expect(mockProvider.handleModeSwitch).toHaveBeenCalledWith("code", task)
19922006
expect(requireDefined(createMessage.mock.calls[0])[2]?.mode).toBe("code")
19932007
})
19942008

2009+
it("still delivers the user message when the mode switch fails", async () => {
2010+
const task = new Task({
2011+
provider: mockProvider,
2012+
apiConfiguration: mockApiConfig,
2013+
task: "initial task",
2014+
startTask: false,
2015+
})
2016+
const switchError = new Error("task history write failed")
2017+
vi.spyOn(mockProvider, "handleModeSwitch").mockRejectedValue(switchError)
2018+
const handleResponseSpy = vi.spyOn(task, "handleWebviewAskResponse").mockImplementation(() => {})
2019+
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {})
2020+
2021+
await task.submitUserMessage("still delivered", undefined, "code")
2022+
2023+
// The mode-switch rejection is caught and logged locally...
2024+
expect(consoleErrorSpy).toHaveBeenCalledWith(
2025+
`[Task#submitUserMessage] Mode switch to code failed (taskId=${task.taskId}):`,
2026+
switchError,
2027+
)
2028+
// ...and the pending ask is still answered, so the submitted text is not lost.
2029+
expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "still delivered", [])
2030+
consoleErrorSpy.mockRestore()
2031+
})
2032+
2033+
it("keeps the selected mode when the deferred mode initialization settles after the submission", async () => {
2034+
// The constructor-started initializeTaskMode() is still awaiting the
2035+
// provider state. submitUserMessage must let that initialization settle
2036+
// before switching: without the await, the deferred read resolves after
2037+
// the switch and clobbers the selected mode with the pre-switch value.
2038+
// One shared pending promise: both the mode and api-config initializers
2039+
// await provider.getState(), and a per-call promise would leave the mode
2040+
// initializer's own read pending forever.
2041+
let releaseState: (state: ProviderState) => void = () => {}
2042+
const deferredState = new Promise<ProviderState>((resolve) => {
2043+
releaseState = resolve
2044+
})
2045+
vi.spyOn(mockProvider, "getState").mockImplementation(() => deferredState)
2046+
vi.spyOn(mockProvider, "handleModeSwitch").mockImplementation(async (mode, targetTask) => {
2047+
if (targetTask) {
2048+
getTaskTestAccess(targetTask)._taskMode = mode
2049+
}
2050+
})
2051+
const task = new Task({
2052+
provider: mockProvider,
2053+
apiConfiguration: mockApiConfig,
2054+
task: "initial task",
2055+
startTask: false,
2056+
})
2057+
vi.spyOn(task, "handleWebviewAskResponse").mockImplementation(() => {})
2058+
2059+
const submitted = task.submitUserMessage("select while initializing", undefined, "architect")
2060+
2061+
// Resolve the pending provider state only after the submission has started,
2062+
// carrying the provider's pre-switch mode.
2063+
releaseState({ mode: "ask" } as unknown as ProviderState)
2064+
2065+
await submitted
2066+
2067+
expect(mockProvider.handleModeSwitch).toHaveBeenCalledWith("architect", task)
2068+
// The explicit selection survives: the deferred initialization ran before,
2069+
// not after, the mode write.
2070+
expect(await task.getTaskMode()).toBe("architect")
2071+
})
2072+
19952073
it("stores a provider profile selected through submitUserMessage", async () => {
19962074
const selectedConfiguration: ProviderSettings = {
19972075
...mockApiConfig,

‎src/core/tools/SwitchModeTool.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import delay from "delay"
22

33
import { Task } from "../task/Task"
44
import { formatResponse } from "../prompts/responses"
5-
import { defaultModeSlug, getModeBySlug } from "../../shared/modes"
5+
import { getModeBySlug } from "../../shared/modes"
66
import { BaseTool, ToolCallbacks } from "./BaseTool"
77
import type { ToolUse } from "../../shared/tools"
88

@@ -39,7 +39,9 @@ export class SwitchModeTool extends BaseTool<"switch_mode"> {
3939
}
4040

4141
// Check if already in requested mode
42-
const currentMode = (await task.providerRef.deref()?.getState())?.mode ?? defaultModeSlug
42+
// the task's own mode (awaits taskModeReady and applies the default slug) instead of the provider
43+
// state, which may be stale or focused on another task.
44+
const currentMode = await task.getTaskMode()
4345

4446
if (currentMode === mode_slug) {
4547
task.recordToolError("switch_mode")
@@ -55,8 +57,9 @@ export class SwitchModeTool extends BaseTool<"switch_mode"> {
5557
return
5658
}
5759

58-
// Switch the mode using shared handler
59-
await task.providerRef.deref()?.handleModeSwitch(mode_slug)
60+
// Switch the mode using shared handler. Pass this task explicitly so the
61+
// switch is scoped to it rather than the provider's currently focused task.
62+
await task.providerRef.deref()?.handleModeSwitch(mode_slug, task)
6063

6164
pushToolResult(
6265
`Successfully switched from ${getModeBySlug(currentMode)?.name ?? currentMode} mode to ${

‎src/core/tools/__tests__/switchModeTool.spec.ts‎

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,22 @@ describe("SwitchModeTool", () => {
3333
let mockCallbacks: ToolCallbacks
3434
let mockHandleModeSwitch: ReturnType<typeof vi.fn>
3535
let mockGetState: ReturnType<typeof vi.fn>
36+
let mockGetTaskMode: ReturnType<typeof vi.fn>
3637

3738
beforeEach(() => {
3839
vi.clearAllMocks()
3940

4041
mockHandleModeSwitch = vi.fn().mockResolvedValue(undefined)
4142
mockGetState = vi.fn().mockResolvedValue({ mode: "code", customModes: [] })
43+
mockGetTaskMode = vi.fn().mockResolvedValue("code")
4244

4345
mockTask = {
4446
consecutiveMistakeCount: 0,
4547
recordToolError: vi.fn(),
4648
didToolFailInCurrentTurn: false,
4749
sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing parameter error"),
4850
ask: vi.fn().mockResolvedValue({}),
51+
getTaskMode: mockGetTaskMode,
4952
providerRef: {
5053
deref: vi.fn().mockReturnValue({
5154
getState: mockGetState,
@@ -165,8 +168,8 @@ describe("SwitchModeTool", () => {
165168
}),
166169
)
167170

168-
// Should have called handleModeSwitch with the target slug
169-
expect(mockHandleModeSwitch).toHaveBeenCalledWith("architect")
171+
// Should have called handleModeSwitch with the target slug and the task
172+
expect(mockHandleModeSwitch).toHaveBeenCalledWith("architect", mockTask)
170173

171174
// Should have pushed success result
172175
expect(mockCallbacks.pushToolResult).toHaveBeenCalledWith(
@@ -184,7 +187,7 @@ describe("SwitchModeTool", () => {
184187
JSON.stringify({ tool: "switchMode", mode: "ask", reason: "" }),
185188
)
186189

187-
expect(mockHandleModeSwitch).toHaveBeenCalledWith("ask")
190+
expect(mockHandleModeSwitch).toHaveBeenCalledWith("ask", mockTask)
188191

189192
expect(mockCallbacks.pushToolResult).toHaveBeenCalledWith("Successfully switched from Code mode to Ask mode.")
190193
})
@@ -244,7 +247,7 @@ describe("SwitchModeTool", () => {
244247
// Should have asked for approval first
245248
expect(mockCallbacks.askApproval).toHaveBeenCalled()
246249
// Should have called handleModeSwitch (which throws)
247-
expect(mockHandleModeSwitch).toHaveBeenCalledWith("architect")
250+
expect(mockHandleModeSwitch).toHaveBeenCalledWith("architect", mockTask)
248251
// Error should be caught and reported
249252
expect(mockCallbacks.handleError).toHaveBeenCalledWith("switching mode", switchError)
250253
})
@@ -303,7 +306,7 @@ describe("SwitchModeTool", () => {
303306
await switchModeTool.handle(mockTask, block, mockCallbacks)
304307

305308
expect(mockCallbacks.askApproval).toHaveBeenCalled()
306-
expect(mockHandleModeSwitch).toHaveBeenCalledWith("custom-mode")
309+
expect(mockHandleModeSwitch).toHaveBeenCalledWith("custom-mode", mockTask)
307310
expect(mockCallbacks.pushToolResult).toHaveBeenCalledWith(
308311
"Successfully switched from Code mode to Custom Mode mode because: testing custom modes.",
309312
)
@@ -325,31 +328,36 @@ describe("SwitchModeTool", () => {
325328
expect(mockCallbacks.askApproval).toHaveBeenCalledWith("tool", expectedMessage)
326329
})
327330

328-
// ===== getState with custom modes =====
331+
// ===== current mode source =====
329332

330-
it("should read current mode from providerRef state", async () => {
331-
// Set current mode to "architect"
332-
mockGetState.mockResolvedValue({ mode: "architect", customModes: [] })
333+
it("should read the current mode from task.getTaskMode, not provider state", async () => {
334+
// The provider state still reports the stale "code" mode while the task is actually in
335+
// "architect". The switch report must use the task's own mode.
336+
mockGetState.mockResolvedValue({ mode: "code", customModes: [] })
337+
mockGetTaskMode.mockResolvedValue("architect")
333338

334339
const block = createBlock({ mode_slug: "code", reason: "switching back" })
335340

336341
await switchModeTool.handle(mockTask, block, mockCallbacks)
337342

338-
expect(mockHandleModeSwitch).toHaveBeenCalledWith("code")
343+
expect(mockGetTaskMode).toHaveBeenCalledTimes(1)
344+
expect(mockHandleModeSwitch).toHaveBeenCalledWith("code", mockTask)
339345
expect(mockCallbacks.pushToolResult).toHaveBeenCalledWith(
340346
"Successfully switched from Architect mode to Code mode because: switching back.",
341347
)
342348
})
343349

344-
it("should use defaultModeSlug when getState returns no mode", async () => {
345-
mockGetState.mockResolvedValue({})
350+
it("should use the default slug reported by task.getTaskMode when no mode is active", async () => {
351+
// task.getTaskMode applies the default slug when the task has no explicit mode, so the
352+
// tool reports switching from the default (Code) mode.
353+
mockGetState.mockResolvedValue({ customModes: [] })
354+
mockGetTaskMode.mockResolvedValue("code")
346355

347356
const block = createBlock({ mode_slug: "ask", reason: "test" })
348357

349358
await switchModeTool.handle(mockTask, block, mockCallbacks)
350359

351-
// defaultModeSlug is "code" (from mock)
352-
// Should report switching from Code mode
360+
expect(mockGetTaskMode).toHaveBeenCalledTimes(1)
353361
expect(mockCallbacks.pushToolResult).toHaveBeenCalledWith(
354362
"Successfully switched from Code mode to Ask mode because: test.",
355363
)

0 commit comments

Comments
 (0)