Skip to content

Commit 065116c

Browse files
committed
fix(webview): sync the view-local buffer through wrapped apiConfiguration mutations
- E1: move the provider-release switch test into switchModeTool.spec where the switchModeTool layer is the narrowest proof - E2: assert the state postMessage shape in the mode-lock switch test - E3: prune invalid persisted view-state entries before applying the cap - wrap the upsert and activate buffer-refresh calls so the view-local apiConfiguration buffer tracks the activated profile again; flat provider-settings keys no longer merge into the buffer
1 parent 0df7399 commit 065116c

3 files changed

Lines changed: 40 additions & 40 deletions

File tree

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,4 +362,38 @@ describe("SwitchModeTool", () => {
362362
"Successfully switched from Code mode to Ask mode because: test.",
363363
)
364364
})
365+
366+
// SwitchModeTool routes the switch through task.providerRef.deref()?.handleModeSwitch:
367+
// when the provider was already disposed the deref is undefined, so the optional chain
368+
// must swallow the call and the tool still reports success instead of erroring out.
369+
it("should report a successful switch when the provider reference is already released", async () => {
370+
const toolTask = {
371+
consecutiveMistakeCount: 0,
372+
recordToolError: vi.fn(),
373+
didToolFailInCurrentTurn: false,
374+
sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing parameter error"),
375+
ask: vi.fn().mockResolvedValue({}),
376+
getTaskMode: vi.fn().mockResolvedValue("code"),
377+
providerRef: {
378+
deref: vi.fn().mockReturnValue(undefined),
379+
},
380+
} as unknown as Task // structural double: the tool only reads the fields above
381+
const callbacks: ToolCallbacks = {
382+
askApproval: vi.fn().mockResolvedValue(true),
383+
handleError: vi.fn(),
384+
pushToolResult: vi.fn(),
385+
}
386+
const block = {
387+
type: "tool_use" as const,
388+
name: "switch_mode" as const,
389+
params: { mode_slug: "architect", reason: "test" },
390+
partial: false,
391+
nativeArgs: { mode_slug: "architect", reason: "test" },
392+
} as unknown as ToolUse<"switch_mode"> // mirrors the createBlock helper above
393+
394+
await switchModeTool.handle(toolTask, block, callbacks)
395+
396+
expect(callbacks.handleError).not.toHaveBeenCalled()
397+
expect(callbacks.pushToolResult).toHaveBeenCalledWith(expect.stringContaining("Successfully switched"))
398+
})
365399
})

‎src/core/webview/ClineProvider.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -669,6 +669,7 @@ export class ClineProvider
669669
private prunePersistedViewStates(states: Record<string, PersistedViewState>): Record<string, PersistedViewState> {
670670
return Object.fromEntries(
671671
Object.entries(states)
672+
.filter(([, entry]) => !!entry && typeof entry === "object")
672673
.sort(([, a], [, b]) => (b.updatedAt ?? 0) - (a.updatedAt ?? 0))
673674
.slice(0, ClineProvider.MAX_PERSISTED_VIEW_STATES),
674675
)
@@ -2346,7 +2347,7 @@ export class ClineProvider
23462347
// view-local mutation path: also refresh this view's buffer so a stale
23472348
// loaded apiConfiguration cannot keep shadowing the new settings in
23482349
// getState().
2349-
this._saveViewLocalStateFromMutation(providerSettings),
2350+
this._saveViewLocalStateFromMutation({ apiConfiguration: providerSettings }),
23502351
])
23512352

23522353
// Other live views may have buffered this profile's settings earlier;
@@ -2553,7 +2554,7 @@ export class ClineProvider
25532554
// view-local mutation path: also refresh this view's buffer so a stale
25542555
// loaded apiConfiguration cannot keep shadowing the new settings in
25552556
// getState().
2556-
this._saveViewLocalStateFromMutation(providerSettings),
2557+
this._saveViewLocalStateFromMutation({ apiConfiguration: providerSettings }),
25572558
])
25582559

25592560
// Other live views may have buffered this profile's settings earlier;

‎src/core/webview/__tests__/ClineProvider.parallelMode.spec.ts‎

Lines changed: 3 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,9 @@ import {
1515
import { defaultModeSlug } from "../../../shared/modes"
1616
import { ContextProxy } from "../../config/ContextProxy"
1717
import { ClineProvider } from "../ClineProvider"
18-
import { switchModeTool } from "../../tools/SwitchModeTool"
1918
import { TelemetryService } from "@roo-code/telemetry"
2019

2120
import type { Task } from "../../task/Task"
22-
import type { ToolCallbacks } from "../../tools/BaseTool"
23-
import type { ToolUse } from "../../../shared/tools"
2421

2522
// Mock p-wait-for
2623
vi.mock("p-wait-for", () => ({
@@ -1111,7 +1108,9 @@ describe("ClineProvider - Parallel Mode Support", () => {
11111108
await provider.handleModeSwitch("architect")
11121109

11131110
expect(getModeConfigIdSpy).not.toHaveBeenCalled()
1114-
expect(postMessage).toHaveBeenCalled()
1111+
expect(postMessage).toHaveBeenCalledWith(
1112+
expect.objectContaining({ type: "state", state: expect.objectContaining({ mode: "architect" }) }),
1113+
)
11151114

11161115
await provider.dispose()
11171116
})
@@ -1225,40 +1224,6 @@ describe("ClineProvider - Parallel Mode Support", () => {
12251224
await provider.dispose()
12261225
})
12271226

1228-
// SwitchModeTool routes the switch through task.providerRef.deref()?.handleModeSwitch:
1229-
// when the provider was already disposed the deref is undefined, so the optional chain
1230-
// must swallow the call and the tool still reports success instead of erroring out.
1231-
it("should report a successful switch when the provider reference is already released", async () => {
1232-
const toolTask = {
1233-
consecutiveMistakeCount: 0,
1234-
recordToolError: vi.fn(),
1235-
didToolFailInCurrentTurn: false,
1236-
sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing parameter error"),
1237-
ask: vi.fn().mockResolvedValue({}),
1238-
getTaskMode: vi.fn().mockResolvedValue("code"),
1239-
providerRef: {
1240-
deref: vi.fn().mockReturnValue(undefined),
1241-
},
1242-
} as unknown as Task // structural double: the tool only reads the fields above
1243-
const callbacks: ToolCallbacks = {
1244-
askApproval: vi.fn().mockResolvedValue(true),
1245-
handleError: vi.fn(),
1246-
pushToolResult: vi.fn(),
1247-
}
1248-
const block = {
1249-
type: "tool_use" as const,
1250-
name: "switch_mode" as const,
1251-
params: { mode_slug: "architect", reason: "test" },
1252-
partial: false,
1253-
nativeArgs: { mode_slug: "architect", reason: "test" },
1254-
} as unknown as ToolUse<"switch_mode"> // mirrors createBlock in switchModeTool.spec.ts
1255-
1256-
await switchModeTool.handle(toolTask, block, callbacks)
1257-
1258-
expect(callbacks.handleError).not.toHaveBeenCalled()
1259-
expect(callbacks.pushToolResult).toHaveBeenCalledWith(expect.stringContaining("Successfully switched"))
1260-
})
1261-
12621227
// K1: a no-task switch captures its target before any task is focused; a task gains
12631228
// focus before the queued mutation runs. The view-level durable write and the
12641229
// ModeChanged broadcast belong to the view and must still happen.

0 commit comments

Comments
 (0)