Skip to content

Commit 8da5c6e

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 #979 (stale head e21ff41)
1 parent aa0f3b1 commit 8da5c6e

10 files changed

Lines changed: 919 additions & 65 deletions

‎src/core/task/Task.ts‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1656,8 +1656,22 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
16561656

16571657
if (provider) {
16581658
if (mode) {
1659-
await provider.setMode(mode)
1660-
this._taskMode = mode
1659+
// Route through the shared mode-switch handler so the switch is
1660+
// validated and recorded like any other mode change (task history,
1661+
// TaskModeSwitched, and — when this is the focused task — the view's
1662+
// durable mode pin + ModeChanged broadcast). The handler writes this
1663+
// task's mode only after validation and persistence, so an unknown
1664+
// slug leaves the task mode untouched instead of recording a bad one.
1665+
// A mode-switch failure (e.g. a task-history write failure) must not
1666+
// swallow the submitted message: log it locally and continue delivery.
1667+
try {
1668+
await provider.handleModeSwitch(mode, this)
1669+
} catch (error) {
1670+
console.error(
1671+
`[Task#submitUserMessage] Mode switch to ${mode} failed (taskId=${this.taskId}):`,
1672+
error,
1673+
)
1674+
}
16611675
}
16621676

16631677
if (providerProfile) {

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

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ type TaskTestAccess = {
4040
saveClineMessages: () => Promise<boolean>
4141
safeEnsureModelFetched: () => Promise<void>
4242
addToApiConversationHistory: (message: unknown, reasoning?: string) => Promise<void>
43+
// Private on Task; the provider-owned mode write (ClineProvider.handleModeSwitch)
44+
// sets it, and tests mirror that write through this helper.
45+
_taskMode: string | undefined
4346
}
4447

4548
type TaskAskResult = Awaited<ReturnType<Task["ask"]>>
@@ -1231,6 +1234,7 @@ describe("Cline", () => {
12311234
getMcpHub: vi.fn().mockReturnValue(undefined),
12321235
getSkillsManager: vi.fn().mockReturnValue(undefined),
12331236
say: vi.fn(),
1237+
handleModeSwitch: vi.fn().mockResolvedValue(undefined),
12341238
postStateToWebview: vi.fn().mockResolvedValue(undefined),
12351239
postStateToWebviewWithoutTaskHistory: vi.fn().mockResolvedValue(undefined),
12361240
postStateToWebviewThrottled: vi.fn().mockResolvedValue(undefined),
@@ -1786,7 +1790,13 @@ describe("Cline", () => {
17861790
mode: "ask",
17871791
mcpEnabled: false,
17881792
} as unknown as ProviderState)
1789-
vi.spyOn(mockProvider, "setMode").mockResolvedValue(undefined)
1793+
vi.spyOn(mockProvider, "handleModeSwitch").mockImplementation(async (mode, targetTask) => {
1794+
// Mirror ClineProvider.handleModeSwitch: after validation and persistence
1795+
// the provider owns the task's mode write.
1796+
if (targetTask) {
1797+
getTaskTestAccess(targetTask)._taskMode = mode
1798+
}
1799+
})
17901800
const task = new Task({
17911801
provider: mockProvider,
17921802
apiConfiguration: mockApiConfig,
@@ -1795,6 +1805,10 @@ describe("Cline", () => {
17951805
})
17961806
vi.spyOn(task, "handleWebviewAskResponse").mockImplementation(() => {})
17971807

1808+
// Let the task's initial mode ("ask", from provider state) settle first, so the
1809+
// mode selected with the user message is the task's final mode write.
1810+
await task.getTaskMode()
1811+
17981812
await task.submitUserMessage("switch modes", undefined, "code")
17991813
vi.spyOn(getTaskTestAccess(task), "getSystemPrompt").mockResolvedValue("mock system prompt")
18001814
const stream = (async function* () {
@@ -1807,10 +1821,34 @@ describe("Cline", () => {
18071821

18081822
await task.attemptApiRequest().next()
18091823

1810-
expect(mockProvider.setMode).toHaveBeenCalledWith("code")
1824+
expect(mockProvider.handleModeSwitch).toHaveBeenCalledWith("code", task)
18111825
expect(requireDefined(createMessage.mock.calls[0])[2]?.mode).toBe("code")
18121826
})
18131827

1828+
it("still delivers the user message when the mode switch fails", async () => {
1829+
const task = new Task({
1830+
provider: mockProvider,
1831+
apiConfiguration: mockApiConfig,
1832+
task: "initial task",
1833+
startTask: false,
1834+
})
1835+
const switchError = new Error("task history write failed")
1836+
vi.spyOn(mockProvider, "handleModeSwitch").mockRejectedValue(switchError)
1837+
const handleResponseSpy = vi.spyOn(task, "handleWebviewAskResponse").mockImplementation(() => {})
1838+
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {})
1839+
1840+
await task.submitUserMessage("still delivered", undefined, "code")
1841+
1842+
// The mode-switch rejection is caught and logged locally...
1843+
expect(consoleErrorSpy).toHaveBeenCalledWith(
1844+
`[Task#submitUserMessage] Mode switch to code failed (taskId=${task.taskId}):`,
1845+
switchError,
1846+
)
1847+
// ...and the pending ask is still answered, so the submitted text is not lost.
1848+
expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "still delivered", [])
1849+
consoleErrorSpy.mockRestore()
1850+
})
1851+
18141852
it("stores a provider profile selected through submitUserMessage", async () => {
18151853
const selectedConfiguration: ProviderSettings = {
18161854
...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)