Skip to content

Commit 403bf78

Browse files
PierrunoYTclaude
PierrunoYT
and
claude
committed
fix(webview): clear a new mode binding and rebuild the task from its own config on rollback
Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
1 parent 22b95b2 commit 403bf78

5 files changed

Lines changed: 135 additions & 3 deletions

File tree

‎src/core/config/ProviderSettingsManager.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -560,6 +560,23 @@ export class ProviderSettingsManager {
560560
}
561561
}
562562

563+
/**
564+
* Remove the API config binding of a specific mode, e.g. to undo a binding created by a failed upsert.
565+
*/
566+
public async clearModeConfig(mode: Mode) {
567+
try {
568+
return await this.lock(async () => {
569+
const providerProfiles = await this.load()
570+
if (providerProfiles.modeApiConfigs && mode in providerProfiles.modeApiConfigs) {
571+
delete providerProfiles.modeApiConfigs[mode]
572+
await this.store(providerProfiles)
573+
}
574+
})
575+
} catch (error) {
576+
throw new Error(`Failed to clear mode config: ${error}`)
577+
}
578+
}
579+
563580
/**
564581
* Get the API config ID for a specific mode.
565582
*/

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1352,6 +1352,47 @@ describe("ProviderSettingsManager", () => {
13521352
})
13531353
})
13541354

1355+
describe("ClearModeConfig", () => {
1356+
const profilesWithModes = (modeApiConfigs?: Record<string, string>) =>
1357+
JSON.stringify({
1358+
currentApiConfigName: "default",
1359+
apiConfigs: { default: { id: "default" } },
1360+
modeApiConfigs,
1361+
migrations: { rateLimitSecondsMigrated: true },
1362+
})
1363+
1364+
const lastStored = () => {
1365+
const calls = mockSecrets.store.mock.calls
1366+
return JSON.parse(calls[calls.length - 1][1])
1367+
}
1368+
1369+
it("removes only the binding of the given mode", async () => {
1370+
mockSecrets.get.mockResolvedValue(profilesWithModes({ code: "code-id", architect: "architect-id" }))
1371+
1372+
await providerSettingsManager.clearModeConfig("code")
1373+
1374+
expect(lastStored().modeApiConfigs).toEqual({ architect: "architect-id" })
1375+
})
1376+
1377+
it.each([
1378+
{ name: "the mode has no binding", modeApiConfigs: { architect: "architect-id" } },
1379+
{ name: "no mode map is stored", modeApiConfigs: undefined },
1380+
])("does not write when $name", async ({ modeApiConfigs }) => {
1381+
mockSecrets.get.mockResolvedValue(profilesWithModes(modeApiConfigs))
1382+
mockSecrets.store.mockClear()
1383+
1384+
await providerSettingsManager.clearModeConfig("code")
1385+
1386+
expect(mockSecrets.store).not.toHaveBeenCalled()
1387+
})
1388+
1389+
it("should throw error if secrets storage fails", async () => {
1390+
mockSecrets.get.mockRejectedValue(new Error("Storage failed"))
1391+
1392+
await expect(providerSettingsManager.clearModeConfig("code")).rejects.toThrow("Failed to clear mode config")
1393+
})
1394+
})
1395+
13551396
describe("HasConfig", () => {
13561397
it("should return true for existing config", async () => {
13571398
const existingConfig: ProviderProfiles = {

‎src/core/webview/ClineProvider.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1945,12 +1945,18 @@ export class ClineProvider
19451945
previousProfile = stored
19461946
}
19471947

1948+
// A delegated child can run with a handoff configuration that differs from the global
1949+
// settings, so rollback rebuilds this task from its own captured configuration.
1950+
const task = this.getCurrentTask()
1951+
19481952
return {
19491953
previousProfile,
19501954
listApiConfigMeta: this.getGlobalState("listApiConfigMeta"),
19511955
currentApiConfigName: this.getGlobalState("currentApiConfigName"),
19521956
modeConfigId: await this.providerSettingsManager.getModeConfigId(mode),
19531957
providerSettings: this.contextProxy.getProviderSettings(),
1958+
task,
1959+
taskApiConfiguration: task?.apiConfiguration ? structuredClone(task.apiConfiguration) : undefined,
19541960
}
19551961
}
19561962

@@ -1969,14 +1975,15 @@ export class ClineProvider
19691975
}),
19701976
this.updateGlobalState("listApiConfigMeta", snapshot.listApiConfigMeta),
19711977
this.updateGlobalState("currentApiConfigName", snapshot.currentApiConfigName),
1978+
// A mode with no previous binding must not stay bound to a profile that is being removed.
19721979
snapshot.modeConfigId
19731980
? this.providerSettingsManager.setModeConfig(mode, snapshot.modeConfigId)
1974-
: Promise.resolve(),
1981+
: this.providerSettingsManager.clearModeConfig(mode),
19751982
this.contextProxy.setProviderSettings(snapshot.providerSettings),
19761983
])
19771984

1978-
if (activate) {
1979-
this.updateTaskApiHandlerIfNeeded(snapshot.providerSettings, { forceRebuild: true })
1985+
if (activate && snapshot.task && snapshot.taskApiConfiguration && this.getCurrentTask() === snapshot.task) {
1986+
this.updateTaskApiHandlerIfNeeded(snapshot.taskApiConfiguration, { forceRebuild: true })
19801987
}
19811988

19821989
const failed = results.find((result): result is PromiseRejectedResult => result.status === "rejected")

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

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,7 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
255255
},
256256
]),
257257
setModeConfig: vi.fn(),
258+
clearModeConfig: vi.fn().mockResolvedValue(undefined),
258259
getModeConfigId: vi.fn().mockResolvedValue(undefined),
259260
activateProfile: vi.fn().mockResolvedValue({
260261
name: "test-config",
@@ -422,6 +423,69 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
422423
expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("errors.create_api_config")
423424
})
424425

426+
test("removes a mode binding that did not exist before when activation fails", async () => {
427+
const manager = provider["providerSettingsManager"]
428+
const { mode } = await provider.getState()
429+
vi.mocked(manager.getModeConfigId).mockResolvedValue(undefined)
430+
vi.spyOn(provider.contextProxy, "setProviderSettings").mockRejectedValueOnce(new Error("write failed"))
431+
432+
await provider.upsertProviderProfile("test-config", {
433+
apiProvider: providerIdentifiers.openrouter,
434+
openRouterModelId: "anthropic/claude",
435+
})
436+
437+
expect(manager.clearModeConfig).toHaveBeenCalledWith(mode)
438+
expect(manager.setModeConfig).not.toHaveBeenLastCalledWith(mode, undefined)
439+
})
440+
441+
test("rebuilds a delegated task from its own configuration, not the global settings, when activation fails", async () => {
442+
const manager = provider["providerSettingsManager"]
443+
const childConfiguration = {
444+
apiProvider: providerIdentifiers.anthropic,
445+
apiModelId: "claude-handoff-model",
446+
}
447+
expect(provider.contextProxy.getProviderSettings()).not.toMatchObject(childConfiguration)
448+
const childTask = new Task({ ...defaultTaskOptions, apiConfiguration: childConfiguration })
449+
await provider.addClineToStack(childTask)
450+
vi.mocked(manager.hasConfig).mockResolvedValue(true)
451+
vi.mocked(manager.getProfile).mockResolvedValue({ name: "test-config", id: "test-id" })
452+
vi.spyOn(provider.contextProxy, "setProviderSettings").mockRejectedValueOnce(new Error("write failed"))
453+
454+
await provider.upsertProviderProfile("test-config", {
455+
apiProvider: providerIdentifiers.openrouter,
456+
openRouterModelId: "anthropic/claude",
457+
})
458+
459+
expect(childTask.updateApiConfiguration).toHaveBeenLastCalledWith(childConfiguration)
460+
})
461+
462+
test("reports a partial state, after running every rollback step, when the rollback itself fails", async () => {
463+
const manager = provider["providerSettingsManager"]
464+
vi.mocked(manager.hasConfig).mockResolvedValue(true)
465+
vi.mocked(manager.getProfile).mockResolvedValue({ name: "test-config", id: "test-id" })
466+
vi.mocked(manager.getModeConfigId).mockResolvedValue("previous-mode-id")
467+
const previousSettings = provider.contextProxy.getProviderSettings()
468+
const setProviderSettings = vi
469+
.spyOn(provider.contextProxy, "setProviderSettings")
470+
.mockRejectedValueOnce(new Error("activation write failed"))
471+
// Activation writes the mode binding first, then the rollback fails to restore it.
472+
vi.mocked(manager.setModeConfig)
473+
.mockResolvedValueOnce(undefined)
474+
.mockRejectedValue(new Error("restore failed"))
475+
476+
const result = await provider.upsertProviderProfile("test-config", {
477+
apiProvider: providerIdentifiers.openrouter,
478+
openRouterModelId: "anthropic/claude",
479+
})
480+
481+
expect(result).toBeUndefined()
482+
// The failed mode restore did not stop the other steps.
483+
expect(manager.saveConfig).toHaveBeenLastCalledWith("test-config", { id: "test-id" })
484+
expect(setProviderSettings).toHaveBeenLastCalledWith(previousSettings)
485+
expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("errors.update_profile_model_partial")
486+
expect(vscode.window.showErrorMessage).not.toHaveBeenCalledWith("errors.create_api_config")
487+
})
488+
425489
test("deletes a profile that did not exist before when activation fails", async () => {
426490
const manager = provider["providerSettingsManager"]
427491
vi.mocked(manager.hasConfig).mockResolvedValueOnce(false).mockResolvedValue(true)

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3463,6 +3463,7 @@ describe("ClineProvider", () => {
34633463
hasConfig: vi.fn().mockResolvedValue(false),
34643464
getModeConfigId: vi.fn().mockResolvedValue(undefined),
34653465
setModeConfig: vi.fn(),
3466+
clearModeConfig: vi.fn().mockResolvedValue(undefined),
34663467
saveConfig: vi.fn().mockResolvedValue(undefined),
34673468
listConfig: vi
34683469
.fn()
@@ -3513,6 +3514,7 @@ describe("ClineProvider", () => {
35133514
hasConfig: vi.fn().mockResolvedValue(false),
35143515
getModeConfigId: vi.fn().mockResolvedValue(undefined),
35153516
setModeConfig: vi.fn(),
3517+
clearModeConfig: vi.fn().mockResolvedValue(undefined),
35163518
saveConfig: vi.fn().mockResolvedValue(undefined),
35173519
listConfig: vi
35183520
.fn()
@@ -3568,6 +3570,7 @@ describe("ClineProvider", () => {
35683570
hasConfig: vi.fn().mockResolvedValue(false),
35693571
getModeConfigId: vi.fn().mockResolvedValue(undefined),
35703572
setModeConfig: vi.fn(),
3573+
clearModeConfig: vi.fn().mockResolvedValue(undefined),
35713574
saveConfig: vi.fn().mockResolvedValue(undefined),
35723575
listConfig: vi
35733576
.fn()

0 commit comments

Comments
 (0)