Skip to content

Commit da97f98

Browse files
committed
fix(webview): invalidate live sibling view state on reset and settings import
- ClineProvider: new broadcastResetToAllInstances() clears each live instance's view-local cache and issues the single global contextProxy setValue("viewStates", undefined) write (single write-queue clear; no secrets involved, no prune-cap regression). - resetState: awaits broadcastResetToAllInstances() before the final postStateToWebview so parallel tabs do not keep stale durable/in-memory per-view state. - importExport: ImportWithProviderOptions.provider gains optional broadcastResetToAllInstances?(); importSettingsWithFeedback calls it in a guarded try/catch (log-only) after a successful import, so a failing broadcast never fails the import. - importExport spec: 3 new tests (broadcast called when available / skipped when missing / import result preserved when broadcast throws, console.warn asserted; the skip test also asserts the broadcast-failure warn is NOT reached). Provider identifiers use providerIdentifiers.* per the zoo/no-raw-provider-identifiers rule (lint-required adaptation from #981's raw-string casts; no semantic change). - parallelMode spec: appends the CS source-of-record describes (multi-instance isolation, _clearViewLocalState) — 5 new tests. - ClineProvider spec: forward fix of the F3 resetState sentinel (F4's global viewStates clear removes the key; the F3-era toEqual({}) expectation is replaced by toBeUndefined()) plus a new cross-instance resetState test pinning the multi-instance broadcast path (sibling view-local cache cleared; sibling and caller each post state exactly once). - webviewMessageHandler.ts was NOT edited: the importSettings case already passes the full ClineProvider, which structurally satisfies the extended provider type and reaches the real broadcast method — #981's structural wrapper hunk is redundant in this stack. Upstream: #980 / PR #981 (vps2 F4)
1 parent 7d91ff4 commit da97f98

5 files changed

Lines changed: 342 additions & 1 deletion

File tree

‎src/core/config/__tests__/importExport.spec.ts‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -956,6 +956,164 @@ describe("importExport", () => {
956956
expect(mockProvider.settingsImportedAt).toBeUndefined()
957957
})
958958

959+
it("should call broadcastResetToAllInstances after successful import when available", async () => {
960+
const filePath = "/mock/path/settings.json"
961+
const mockFileContent = JSON.stringify({
962+
providerProfiles: {
963+
currentApiConfigName: "valid-profile",
964+
apiConfigs: {
965+
"valid-profile": {
966+
apiProvider: providerIdentifiers.openai,
967+
apiKey: "test-key",
968+
id: "valid-id",
969+
},
970+
},
971+
},
972+
globalSettings: { mode: "code" },
973+
})
974+
975+
;(fs.readFile as Mock).mockResolvedValue(mockFileContent)
976+
;(fs.access as Mock).mockResolvedValue(undefined)
977+
mockProviderSettingsManager.export.mockResolvedValue({
978+
currentApiConfigName: "default",
979+
apiConfigs: { default: { apiProvider: providerIdentifiers.anthropic, id: "default-id" } },
980+
})
981+
mockProviderSettingsManager.listConfig.mockResolvedValue([
982+
{ name: "valid-profile", id: "valid-id", apiProvider: providerIdentifiers.openai },
983+
])
984+
985+
const mockProvider = {
986+
settingsImportedAt: 0,
987+
postStateToWebview: vi.fn().mockResolvedValue(undefined),
988+
broadcastResetToAllInstances: vi.fn().mockResolvedValue(undefined),
989+
}
990+
991+
await importSettingsWithFeedback(
992+
{
993+
providerSettingsManager: mockProviderSettingsManager,
994+
contextProxy: mockContextProxy,
995+
customModesManager: mockCustomModesManager,
996+
provider: mockProvider,
997+
},
998+
filePath,
999+
)
1000+
1001+
expect(mockProvider.postStateToWebview).toHaveBeenCalledTimes(1)
1002+
expect(mockProvider.broadcastResetToAllInstances).toHaveBeenCalledTimes(1)
1003+
expect(vscode.window.showInformationMessage).toHaveBeenCalledWith(
1004+
expect.stringContaining("settings_imported"),
1005+
)
1006+
})
1007+
1008+
it("should skip broadcastResetToAllInstances when callback is missing", async () => {
1009+
const filePath = "/mock/path/settings.json"
1010+
const mockFileContent = JSON.stringify({
1011+
providerProfiles: {
1012+
currentApiConfigName: "valid-profile",
1013+
apiConfigs: {
1014+
"valid-profile": {
1015+
apiProvider: providerIdentifiers.openai,
1016+
apiKey: "test-key",
1017+
id: "valid-id",
1018+
},
1019+
},
1020+
},
1021+
globalSettings: { mode: "code" },
1022+
})
1023+
1024+
;(fs.readFile as Mock).mockResolvedValue(mockFileContent)
1025+
;(fs.access as Mock).mockResolvedValue(undefined)
1026+
mockProviderSettingsManager.export.mockResolvedValue({
1027+
currentApiConfigName: "default",
1028+
apiConfigs: { default: { apiProvider: providerIdentifiers.anthropic, id: "default-id" } },
1029+
})
1030+
mockProviderSettingsManager.listConfig.mockResolvedValue([
1031+
{ name: "valid-profile", id: "valid-id", apiProvider: providerIdentifiers.openai },
1032+
])
1033+
1034+
const mockProvider = {
1035+
settingsImportedAt: 0,
1036+
postStateToWebview: vi.fn().mockResolvedValue(undefined),
1037+
}
1038+
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {})
1039+
1040+
await importSettingsWithFeedback(
1041+
{
1042+
providerSettingsManager: mockProviderSettingsManager,
1043+
contextProxy: mockContextProxy,
1044+
customModesManager: mockCustomModesManager,
1045+
provider: mockProvider,
1046+
},
1047+
filePath,
1048+
)
1049+
1050+
expect(mockProvider.postStateToWebview).toHaveBeenCalledTimes(1)
1051+
expect(vscode.window.showInformationMessage).toHaveBeenCalledWith(
1052+
expect.stringContaining("settings_imported"),
1053+
)
1054+
// A missing callback must not reach the broadcast guard's failure path.
1055+
expect(consoleWarnSpy).not.toHaveBeenCalledWith(
1056+
expect.stringContaining("Failed to broadcast reset after settings import"),
1057+
)
1058+
consoleWarnSpy.mockRestore()
1059+
})
1060+
1061+
it("should keep successful import result when broadcastResetToAllInstances throws", async () => {
1062+
const filePath = "/mock/path/settings.json"
1063+
const mockFileContent = JSON.stringify({
1064+
providerProfiles: {
1065+
currentApiConfigName: "valid-profile",
1066+
apiConfigs: {
1067+
"valid-profile": {
1068+
apiProvider: providerIdentifiers.openai,
1069+
apiKey: "test-key",
1070+
id: "valid-id",
1071+
},
1072+
},
1073+
},
1074+
globalSettings: { mode: "code" },
1075+
})
1076+
1077+
;(fs.readFile as Mock).mockResolvedValue(mockFileContent)
1078+
;(fs.access as Mock).mockResolvedValue(undefined)
1079+
mockProviderSettingsManager.export.mockResolvedValue({
1080+
currentApiConfigName: "default",
1081+
apiConfigs: { default: { apiProvider: providerIdentifiers.anthropic, id: "default-id" } },
1082+
})
1083+
mockProviderSettingsManager.listConfig.mockResolvedValue([
1084+
{ name: "valid-profile", id: "valid-id", apiProvider: providerIdentifiers.openai },
1085+
])
1086+
1087+
const broadcastError = new Error("broadcast failed")
1088+
const mockProvider = {
1089+
settingsImportedAt: 0,
1090+
postStateToWebview: vi.fn().mockResolvedValue(undefined),
1091+
broadcastResetToAllInstances: vi.fn().mockRejectedValue(broadcastError),
1092+
}
1093+
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {})
1094+
1095+
await importSettingsWithFeedback(
1096+
{
1097+
providerSettingsManager: mockProviderSettingsManager,
1098+
contextProxy: mockContextProxy,
1099+
customModesManager: mockCustomModesManager,
1100+
provider: mockProvider,
1101+
},
1102+
filePath,
1103+
)
1104+
1105+
expect(mockProvider.postStateToWebview).toHaveBeenCalledTimes(1)
1106+
expect(mockProvider.broadcastResetToAllInstances).toHaveBeenCalledTimes(1)
1107+
expect(consoleWarnSpy).toHaveBeenCalledWith(
1108+
expect.stringContaining("Failed to broadcast reset after settings import"),
1109+
)
1110+
expect(vscode.window.showInformationMessage).toHaveBeenCalledWith(
1111+
expect.stringContaining("settings_imported"),
1112+
)
1113+
1114+
consoleWarnSpy.mockRestore()
1115+
})
1116+
9591117
it("should handle multiple profiles with mixed valid and invalid providers", async () => {
9601118
;(vscode.window.showOpenDialog as Mock).mockResolvedValue([{ fsPath: "/mock/path/settings.json" }])
9611119

‎src/core/config/importExport.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ type ImportWithProviderOptions = ImportOptions & {
3636
provider: {
3737
settingsImportedAt?: number
3838
postStateToWebview: () => Promise<void>
39+
broadcastResetToAllInstances?(): Promise<void>
3940
}
4041
}
4142

@@ -385,6 +386,18 @@ export const importSettingsWithFeedback = async (
385386
if (result.success) {
386387
provider.settingsImportedAt = Date.now()
387388
await provider.postStateToWebview()
389+
390+
// Broadcast invalidation to all other live ClineProvider instances so parallel
391+
// tabs don't keep stale view-local state after a settings import.
392+
try {
393+
if (provider.broadcastResetToAllInstances) {
394+
await provider.broadcastResetToAllInstances()
395+
}
396+
} catch (error) {
397+
// Log but do not fail the import if broadcast fails — the import itself succeeded.
398+
console.warn(`Failed to broadcast reset after settings import: ${error}`)
399+
}
400+
388401
provider.settingsImportedAt = undefined
389402
const warnings = "warnings" in result ? result.warnings : undefined
390403

‎src/core/webview/ClineProvider.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3693,6 +3693,23 @@ export class ClineProvider
36933693
this.viewLocalState = {}
36943694
}
36953695

3696+
/**
3697+
* Broadcast a reset/import invalidation to all live ClineProvider instances, clearing
3698+
* both in-memory view-local caches and durable per-view selections so stale view state
3699+
* cannot mask imported/reset shared state after reload.
3700+
*/
3701+
async broadcastResetToAllInstances(): Promise<void> {
3702+
const allInstances = ClineProvider.getAllInstances()
3703+
for (const instance of allInstances) {
3704+
instance._clearViewLocalState()
3705+
await instance.contextProxy.setValue("viewStates", undefined)
3706+
3707+
if (instance !== this) {
3708+
await instance.postStateToWebview()
3709+
}
3710+
}
3711+
}
3712+
36963713
// dev
36973714

36983715
async resetState() {
@@ -3730,6 +3747,10 @@ export class ClineProvider
37303747
await this.providerSettingsManager.resetAllConfigs()
37313748
await this.customModesManager.resetCustomModes()
37323749
await this.removeClineFromStack()
3750+
3751+
// Clear durable and in-memory per-view state across live instances so parallel tabs don't keep stale state.
3752+
await this.broadcastResetToAllInstances()
3753+
37333754
await this.postStateToWebview()
37343755
await this.postMessageToWebview({ type: "action", action: "chatButtonClicked" })
37353756
}

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

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1353,4 +1353,120 @@ describe("ClineProvider - Parallel Mode Support", () => {
13531353
await provider.dispose()
13541354
})
13551355
})
1356+
1357+
describe("multi-instance isolation", () => {
1358+
it("should maintain independent state across three instances", async () => {
1359+
const provider1 = new ClineProvider(
1360+
mockContext,
1361+
mockOutputChannel,
1362+
"sidebar",
1363+
new ContextProxy(mockContext),
1364+
)
1365+
const provider2 = new ClineProvider(mockContext, mockOutputChannel, "editor", new ContextProxy(mockContext))
1366+
const provider3 = new ClineProvider(mockContext, mockOutputChannel, "editor", new ContextProxy(mockContext))
1367+
1368+
await provider1.saveViewState("mode", "code")
1369+
await provider1.saveViewState("currentApiConfigName", "profile-1")
1370+
await provider2.saveViewState("mode", "architect")
1371+
await provider2.saveViewState("currentApiConfigName", "profile-2")
1372+
await provider3.saveViewState("mode", "debugger")
1373+
await provider3.saveViewState("currentApiConfigName", "profile-3")
1374+
1375+
const state1 = await provider1.getState()
1376+
const state2 = await provider2.getState()
1377+
const state3 = await provider3.getState()
1378+
1379+
expect(state1.mode).toBe("code")
1380+
expect(state1.currentApiConfigName).toBe("profile-1")
1381+
expect(state2.mode).toBe("architect")
1382+
expect(state2.currentApiConfigName).toBe("profile-2")
1383+
expect(state3.mode).toBe("debugger")
1384+
expect(state3.currentApiConfigName).toBe("profile-3")
1385+
1386+
await provider1.dispose()
1387+
await provider2.dispose()
1388+
await provider3.dispose()
1389+
})
1390+
1391+
it("should handle mode switch in one instance without affecting others", async () => {
1392+
const postMessage1 = vi.fn()
1393+
const postMessage2 = vi.fn()
1394+
const provider1 = new ClineProvider(
1395+
mockContext,
1396+
mockOutputChannel,
1397+
"sidebar",
1398+
new ContextProxy(mockContext),
1399+
)
1400+
const provider2 = new ClineProvider(mockContext, mockOutputChannel, "editor", new ContextProxy(mockContext))
1401+
1402+
await provider1.resolveWebviewView(createMockWebviewView(postMessage1))
1403+
await provider2.resolveWebviewView(createMockWebviewView(postMessage2))
1404+
await provider1.saveViewState("mode", "code")
1405+
await provider2.saveViewState("mode", "debugger")
1406+
1407+
await provider1.handleModeSwitch("architect")
1408+
1409+
const state1 = await provider1.getState()
1410+
const state2 = await provider2.getState()
1411+
1412+
expect(state1.mode).toBe("architect")
1413+
expect(state2.mode).toBe("debugger")
1414+
expect(provider2["viewLocalState"].mode).toBe("debugger")
1415+
1416+
await provider1.dispose()
1417+
await provider2.dispose()
1418+
})
1419+
})
1420+
1421+
describe("_clearViewLocalState", () => {
1422+
it("should clear all view-local state values", async () => {
1423+
const provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext))
1424+
1425+
await provider.saveViewState("mode", "architect")
1426+
await provider.saveViewState("currentApiConfigName", "my-profile")
1427+
await provider.saveViewState("apiConfiguration", { apiProvider: providerIdentifiers.openrouter })
1428+
1429+
expect(provider["viewLocalState"].mode).toBe("architect")
1430+
expect(provider["viewLocalState"].currentApiConfigName).toBe("my-profile")
1431+
expect(provider["viewLocalState"].apiConfiguration).toEqual({
1432+
apiProvider: providerIdentifiers.openrouter,
1433+
})
1434+
1435+
// Call _clearViewLocalState
1436+
provider["_clearViewLocalState"]()
1437+
1438+
// All values should be cleared
1439+
expect(provider["viewLocalState"]).toEqual({})
1440+
1441+
await provider.dispose()
1442+
})
1443+
1444+
it("should cause getState to fall back to contextProxy values after clear", async () => {
1445+
const provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext))
1446+
1447+
await provider.saveViewState("mode", "architect")
1448+
1449+
let state = await provider.getState()
1450+
expect(state.mode).toBe("architect")
1451+
1452+
// Clear viewLocalState
1453+
provider["_clearViewLocalState"]()
1454+
1455+
// getState should now fall back to contextProxy (global) state
1456+
state = await provider.getState()
1457+
expect(state.mode).toBe("code") // Default from mock context
1458+
1459+
await provider.dispose()
1460+
})
1461+
1462+
it("should be safe to call on empty viewLocalState", async () => {
1463+
const provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext))
1464+
1465+
// Should not throw even if viewLocalState is already empty
1466+
expect(provider["_clearViewLocalState"]()).toBeUndefined()
1467+
expect(provider["viewLocalState"]).toEqual({})
1468+
1469+
await provider.dispose()
1470+
})
1471+
})
13561472
})

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

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1542,9 +1542,42 @@ describe("ClineProvider", () => {
15421542
await provider.saveViewState("mode", "architect")
15431543
await provider.resetState()
15441544
expect(provider["viewLocalState"]).toEqual({})
1545-
expect(mockContext.globalState.get("viewStates")).toEqual({})
1545+
// F4 cross-instance broadcast clears the entire durable viewStates map, so the
1546+
// key is removed (undefined) rather than left as an empty object.
1547+
expect(mockContext.globalState.get("viewStates")).toBeUndefined()
15461548
await provider.dispose()
15471549
})
1550+
1551+
it("should clear a sibling's view-local state and post sibling state during a cross-instance resetState", async () => {
1552+
const provider1 = new ClineProvider(
1553+
mockContext,
1554+
mockOutputChannel,
1555+
"sidebar",
1556+
new ContextProxy(mockContext),
1557+
)
1558+
const provider2 = new ClineProvider(mockContext, mockOutputChannel, "editor", new ContextProxy(mockContext))
1559+
const post1 = vi.spyOn(provider1, "postStateToWebview").mockResolvedValue(undefined)
1560+
const post2 = vi.spyOn(provider2, "postStateToWebview").mockResolvedValue(undefined)
1561+
// @ts-ignore - Replace customModesManager with a test double (the real reset writes to disk).
1562+
provider1.customModesManager = { resetCustomModes: vi.fn().mockResolvedValue(undefined), dispose: vi.fn() }
1563+
// The modal answer is a string label; the last-typed vscode overload expects a MessageItem.
1564+
vi.mocked(vscode.window.showInformationMessage).mockResolvedValue(
1565+
t("common:answers.yes") as unknown as vscode.MessageItem,
1566+
)
1567+
1568+
await provider2.saveViewState("mode", "architect")
1569+
await provider1.resetState()
1570+
1571+
// The sibling's in-memory view-local cache must be cleared by the broadcast.
1572+
expect(provider2["viewLocalState"]).toEqual({})
1573+
// The sibling receives exactly one posted state (from the broadcast); the caller
1574+
// receives exactly one (its final reset post), never a double post from the broadcast.
1575+
expect(post2).toHaveBeenCalledTimes(1)
1576+
expect(post1).toHaveBeenCalledTimes(1)
1577+
1578+
await provider1.dispose()
1579+
await provider2.dispose()
1580+
})
15481581
})
15491582

15501583
describe("local state isolation", () => {

0 commit comments

Comments
 (0)