Skip to content

Commit 510689f

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 dbd7ec2 commit 510689f

10 files changed

Lines changed: 915 additions & 65 deletions

‎src/core/task/Task.ts‎

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

16771677
if (provider) {
16781678
if (mode) {
1679-
await provider.setMode(mode)
1680-
this._taskMode = mode
1679+
// Route through the shared mode-switch handler so the switch is
1680+
// validated and recorded like any other mode change (task history,
1681+
// TaskModeSwitched, and — when this is the focused task — the view's
1682+
// durable mode pin + ModeChanged broadcast). The handler writes this
1683+
// task's mode only after validation and persistence, so an unknown
1684+
// slug leaves the task mode untouched instead of recording a bad one.
1685+
// A mode-switch failure (e.g. a task-history write failure) must not
1686+
// swallow the submitted message: log it locally and continue delivery.
1687+
try {
1688+
await provider.handleModeSwitch(mode, this)
1689+
} catch (error) {
1690+
console.error(
1691+
`[Task#submitUserMessage] Mode switch to ${mode} failed (taskId=${this.taskId}):`,
1692+
error,
1693+
)
1694+
}
16811695
}
16821696

16831697
if (providerProfile) {

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

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

4649
type TaskAskResult = Awaited<ReturnType<Task["ask"]>>
@@ -1411,6 +1414,7 @@ describe("Cline", () => {
14111414
getMcpHub: vi.fn().mockReturnValue(undefined),
14121415
getSkillsManager: vi.fn().mockReturnValue(undefined),
14131416
say: vi.fn(),
1417+
handleModeSwitch: vi.fn().mockResolvedValue(undefined),
14141418
postStateToWebview: vi.fn().mockResolvedValue(undefined),
14151419
postStateToWebviewWithoutTaskHistory: vi.fn().mockResolvedValue(undefined),
14161420
postStateToWebviewThrottled: vi.fn().mockResolvedValue(undefined),
@@ -1966,7 +1970,13 @@ describe("Cline", () => {
19661970
mode: "ask",
19671971
mcpEnabled: false,
19681972
} as unknown as ProviderState)
1969-
vi.spyOn(mockProvider, "setMode").mockResolvedValue(undefined)
1973+
vi.spyOn(mockProvider, "handleModeSwitch").mockImplementation(async (mode, targetTask) => {
1974+
// Mirror ClineProvider.handleModeSwitch: after validation and persistence
1975+
// the provider owns the task's mode write.
1976+
if (targetTask) {
1977+
getTaskTestAccess(targetTask)._taskMode = mode
1978+
}
1979+
})
19701980
const task = new Task({
19711981
provider: mockProvider,
19721982
apiConfiguration: mockApiConfig,
@@ -1975,6 +1985,10 @@ describe("Cline", () => {
19751985
})
19761986
vi.spyOn(task, "handleWebviewAskResponse").mockImplementation(() => {})
19771987

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

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

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

2008+
it("still delivers the user message when the mode switch fails", async () => {
2009+
const task = new Task({
2010+
provider: mockProvider,
2011+
apiConfiguration: mockApiConfig,
2012+
task: "initial task",
2013+
startTask: false,
2014+
})
2015+
const switchError = new Error("task history write failed")
2016+
vi.spyOn(mockProvider, "handleModeSwitch").mockRejectedValue(switchError)
2017+
const handleResponseSpy = vi.spyOn(task, "handleWebviewAskResponse").mockImplementation(() => {})
2018+
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {})
2019+
2020+
await task.submitUserMessage("still delivered", undefined, "code")
2021+
2022+
// The mode-switch rejection is caught and logged locally...
2023+
expect(consoleErrorSpy).toHaveBeenCalledWith(
2024+
`[Task#submitUserMessage] Mode switch to code failed (taskId=${task.taskId}):`,
2025+
switchError,
2026+
)
2027+
// ...and the pending ask is still answered, so the submitted text is not lost.
2028+
expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "still delivered", [])
2029+
consoleErrorSpy.mockRestore()
2030+
})
2031+
19942032
it("stores a provider profile selected through submitUserMessage", async () => {
19952033
const selectedConfiguration: ProviderSettings = {
19962034
...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)