Skip to content

Commit 279e9da

Browse files
easonliang28easonliang28
committed
fix(webview): scope profile-save task rebuilds to the task's own profile
Port of the sibling-unit fix: a non-activating profile save force-rebuilt the API handler of every view pinned to the saved profile, including views whose active task follows a different sticky profile. Both rebuild sites (the acting view's pinned branch and the pinned-sibling refresh) are now guarded by a taskApiConfigName match; the pinned view's overlay still refreshes. Regression tests cover both sites. Test hygiene: prune assertions compare full kept entries; the two re-key storage tests seed viewStates through the durable write path so their assertions can fail; webview state-payload test renamed.
1 parent 8f6749a commit 279e9da

4 files changed

Lines changed: 121 additions & 15 deletions

File tree

‎src/core/webview/ClineProvider.ts‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2418,7 +2418,13 @@ export class ClineProvider
24182418
// serving the settings captured before the save under the same name.
24192419
if (this.pinnedProfileName === name) {
24202420
await this._saveViewLocalStateFromMutation({ apiConfiguration: providerSettings })
2421-
this.updateTaskApiHandlerIfNeeded(providerSettings, { forceRebuild: true })
2421+
// Only rebuild when the task actually runs under the saved profile: a
2422+
// view pinned to this profile can hold a task that follows a different
2423+
// sticky profile, and the forced rebuild would silently switch that
2424+
// conversation to the saved profile's settings.
2425+
if (this.getCurrentTask()?.taskApiConfigName === name) {
2426+
this.updateTaskApiHandlerIfNeeded(providerSettings, { forceRebuild: true })
2427+
}
24222428
}
24232429
await this.refreshViewLocalStateForUpdatedProfile(name, providerSettings)
24242430
}
@@ -2668,9 +2674,13 @@ export class ClineProvider
26682674
// The active task of a pinned view holds its own API handler built from
26692675
// the profile's settings: rebuild it with the saved settings or the
26702676
// running conversation keeps sending the old token even though the
2671-
// view's buffer has caught up. Direct private access across sibling
2672-
// instances of the same class is compile-time safe.
2673-
instance.updateTaskApiHandlerIfNeeded(providerSettings, { forceRebuild: true })
2677+
// view's buffer has caught up. Only rebuild when the task actually runs
2678+
// under the saved profile: a task that follows a different sticky
2679+
// profile must keep its own settings. Direct private access across
2680+
// sibling instances of the same class is compile-time safe.
2681+
if (instance.getCurrentTask()?.taskApiConfigName === name) {
2682+
instance.updateTaskApiHandlerIfNeeded(providerSettings, { forceRebuild: true })
2683+
}
26742684
await instance.postStateToWebview()
26752685
}),
26762686
)

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

Lines changed: 98 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1865,8 +1865,8 @@ describe("ClineProvider", () => {
18651865
const pruned = provider["prunePersistedViewStates"](states)
18661866

18671867
expect(Object.keys(pruned)).toHaveLength(50)
1868-
expect(pruned["view-54"]).toBeDefined()
1869-
expect(pruned["view-5"]).toBeDefined()
1868+
expect(pruned["view-54"]).toEqual({ mode: "mode-54", updatedAt: 54 })
1869+
expect(pruned["view-5"]).toEqual({ mode: "mode-5", updatedAt: 5 })
18701870
expect(pruned["view-4"]).toBeUndefined()
18711871

18721872
await provider.dispose()
@@ -8307,6 +8307,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => {
83078307
// minimal double without the full provider/services graph.
83088308
const siblingTask = {
83098309
apiConfiguration: { apiProvider: providerIdentifiers.anthropic, apiKey: "old-key" },
8310+
taskApiConfigName: "saved-profile",
83108311
updateApiConfiguration: vi.fn(),
83118312
}
83128313
vi.spyOn(sibling, "getCurrentTask").mockReturnValue(
@@ -8351,6 +8352,101 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => {
83518352
}
83528353
})
83538354

8355+
it("does not rebuild a pinned sibling's task that runs under another profile", async () => {
8356+
// Defensive: the doubles in this suite can leave console writes pending
8357+
// at worker teardown.
8358+
vi.spyOn(console, "log").mockImplementation(() => {})
8359+
vi.spyOn(console, "warn").mockImplementation(() => {})
8360+
vi.spyOn(console, "error").mockImplementation(() => {})
8361+
const sibling = new ClineProvider(
8362+
mockContext,
8363+
mockOutputChannel,
8364+
"editor",
8365+
new ContextProxy(mockContext),
8366+
new WebviewFocusTracker(),
8367+
)
8368+
try {
8369+
// The sibling pins the saved profile, but its active task follows a
8370+
// different sticky profile: the save must refresh the view overlay
8371+
// without switching that conversation to the saved profile's settings.
8372+
sibling["viewLocalState"].currentApiConfigName = "saved-profile"
8373+
sibling["viewLocalState"].apiConfiguration = {
8374+
apiProvider: providerIdentifiers.anthropic,
8375+
apiKey: "old-key",
8376+
}
8377+
vi.spyOn(sibling, "postStateToWebview").mockResolvedValue(undefined)
8378+
8379+
const siblingTask = {
8380+
apiConfiguration: { apiProvider: providerIdentifiers.anthropic, apiKey: "other-key" },
8381+
taskApiConfigName: "other-profile",
8382+
updateApiConfiguration: vi.fn(),
8383+
}
8384+
vi.spyOn(sibling, "getCurrentTask").mockReturnValue(
8385+
siblingTask as unknown as ReturnType<typeof sibling.getCurrentTask>,
8386+
)
8387+
vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined)
8388+
// @ts-ignore - Replace providerSettingsManager with a test double.
8389+
provider.providerSettingsManager = {
8390+
saveConfig: vi.fn().mockResolvedValue("saved-id"),
8391+
listConfig: vi.fn().mockResolvedValue([]),
8392+
setModeConfig: vi.fn(),
8393+
}
8394+
8395+
await provider.upsertProviderProfile(
8396+
"saved-profile",
8397+
{ apiProvider: providerIdentifiers.anthropic, apiKey: "new-key" },
8398+
false,
8399+
)
8400+
8401+
// The pinned view's overlay still catches up with the saved settings...
8402+
expect(sibling["viewLocalState"].apiConfiguration).toEqual({
8403+
apiProvider: providerIdentifiers.anthropic,
8404+
apiKey: "new-key",
8405+
})
8406+
// ...but the task runs under another profile: its handler must not be
8407+
// switched to the saved profile's settings.
8408+
expect(siblingTask.updateApiConfiguration).not.toHaveBeenCalled()
8409+
// Let queued view-local persistence settle while the console spies are
8410+
// still installed, so teardown has no pending writes.
8411+
await new Promise((resolve) => setTimeout(resolve, 50))
8412+
} finally {
8413+
await sibling.dispose()
8414+
}
8415+
})
8416+
8417+
it("does not rebuild the acting view's task when it runs under another profile", async () => {
8418+
// The acting view pins the saved profile while its own task follows a
8419+
// different sticky profile: the non-activating save refreshes the view
8420+
// overlay but must leave the task's handler alone.
8421+
const task = {
8422+
apiConfiguration: { apiProvider: providerIdentifiers.anthropic, apiKey: "other-key" },
8423+
taskApiConfigName: "other-profile",
8424+
updateApiConfiguration: vi.fn(),
8425+
}
8426+
vi.spyOn(provider, "getCurrentTask").mockReturnValue(
8427+
task as unknown as ReturnType<typeof provider.getCurrentTask>,
8428+
)
8429+
vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined)
8430+
provider["viewLocalState"].currentApiConfigName = "saved-profile"
8431+
// @ts-ignore - Replace providerSettingsManager with a test double.
8432+
provider.providerSettingsManager = {
8433+
saveConfig: vi.fn().mockResolvedValue("saved-id"),
8434+
listConfig: vi.fn().mockResolvedValue([]),
8435+
setModeConfig: vi.fn(),
8436+
}
8437+
8438+
await provider.upsertProviderProfile(
8439+
"saved-profile",
8440+
{ apiProvider: providerIdentifiers.anthropic, apiKey: "new-key" },
8441+
false,
8442+
)
8443+
8444+
expect(provider["viewLocalState"].apiConfiguration).toEqual({
8445+
apiProvider: providerIdentifiers.anthropic,
8446+
apiKey: "new-key",
8447+
})
8448+
expect(task.updateApiConfiguration).not.toHaveBeenCalled()
8449+
})
83548450
it("does not leak shared provider keys into a pinned view's api configuration", async () => {
83558451
vi.spyOn(console, "log").mockImplementation(() => {})
83568452
vi.spyOn(console, "warn").mockImplementation(() => {})

‎src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1579,10 +1579,10 @@ describe("ClineProvider - Sticky Mode", () => {
15791579
it("does not resolve a view id that names an Object.prototype key through the chain", async () => {
15801580
await provider.resolveWebviewView(mockWebviewView)
15811581

1582-
mockContext.globalState.get = vi.fn().mockImplementation((key: string) => {
1583-
if (key === "viewStates") return { "stable-test-view": { mode: "architect" } }
1584-
return undefined
1585-
})
1582+
// Seed through the durable write path so later writes land in the same
1583+
// backing map the assertions read: a frozen get() literal would make every
1584+
// post-write assertion read the seed no matter what the provider wrote.
1585+
await provider.contextProxy.setValue("viewStates", { "stable-test-view": { mode: "architect" } })
15861586

15871587
const states = provider["getPersistedViewStates"]({ fresh: true })
15881588
expect(Object.getPrototypeOf(states)).toBeNull()
@@ -1673,10 +1673,10 @@ describe("ClineProvider - Sticky Mode", () => {
16731673
// Temporary ids restart from 0 on every host start, so the same name can
16741674
// belong to a different session. An entry this instance never wrote must not
16751675
// be adopted as this view's state.
1676-
mockContext.globalState.get = vi.fn().mockImplementation((key: string) => {
1677-
if (key === "viewStates") return { [provider.viewId]: { mode: "architect" } }
1678-
return undefined
1679-
})
1676+
// Seed through the durable write path so the assertions below read the
1677+
// same backing map a re-key adoption would write to: a frozen get()
1678+
// literal made the negative assertion unable to fail.
1679+
await provider.contextProxy.setValue("viewStates", { [provider.viewId]: { mode: "architect" } })
16801680

16811681
await provider["setViewStateId"]("stable-test-view")
16821682

‎webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ describe("ExtensionStateContext", () => {
168168
}
169169
})
170170

171-
it("reseeds view-local mode and API profile from a new state payload after local edits", () => {
171+
it("applies mode and profile from a later state payload after local edits", () => {
172172
render(
173173
<ExtensionStateContextProvider>
174174
<ViewLocalStateTestComponent />

0 commit comments

Comments
 (0)