Skip to content

fix(provider): isolate profile mutations from focused tasks #1087

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
navedmerchant merged 7 commits into from
Aug 6, 2026
Merged

fix(provider): isolate profile mutations from focused tasks #1087

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f1e74ec
fix(provider): isolate profile mutations from focused tasks
edelauna Aug 1, 2026
fd33c9c
test(e2e): restore provider profile after cross-profile subtasks
edelauna Aug 1, 2026
1f3dfa9
fix(provider): recover timed-out profile mutations
edelauna Aug 1, 2026
b8646bb
test(provider): target queued mode switches explicitly
edelauna Aug 1, 2026
07fd132
fix(provider): preserve queued profile mutation ordering
edelauna Aug 1, 2026
18df117
fix(provider): abort timed-out profile mutations before state writes
edelauna Aug 5, 2026
0c1a2db
Merge branch 'main' into agent/provider-profile-isolation
navedmerchant Aug 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions apps/vscode-e2e/src/suite/subtasks.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,8 @@ suite("Roo Code Subtasks", function () {
...parentProfile,
openRouterModelId: "openai/gpt-4.1-mini",
}
const priorModeApiConfigs = api.getConfiguration().modeApiConfigs ?? {}
const priorConfiguration = api.getConfiguration()
const priorActiveProfile = api.getActiveProfile()
const parentProfileId = await api.upsertProfile("subtask-parent-profile", parentProfile, true)
const childProfileId = await api.upsertProfile("subtask-child-profile", childProfile, false)
await api.setConfiguration({
Expand Down Expand Up @@ -735,7 +736,10 @@ suite("Roo Code Subtasks", function () {
)
} finally {
api.off(RooCodeEventName.Message, messageHandler)
await api.setConfiguration({ modeApiConfigs: priorModeApiConfigs })
await api.setConfiguration(priorConfiguration)
if (priorActiveProfile) {
await api.setActiveProfile(priorActiveProfile)
}
await api.deleteProfile("subtask-child-profile").catch(() => {})
await api.deleteProfile("subtask-parent-profile").catch(() => {})
while (api.getCurrentTaskStack().length > 0) {
Expand Down
230 changes: 172 additions & 58 deletions src/core/webview/ClineProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,68 @@ export class ClineProvider
private taskHistoryStoreInitialized = false
private globalStateWriteThroughTimer: ReturnType<typeof setTimeout> | null = null
private static readonly GLOBAL_STATE_WRITE_THROUGH_DEBOUNCE_MS = 5000 // 5 seconds
private static readonly PENDING_OPERATION_TIMEOUT_MS = 30000 // 30 seconds
public static readonly PENDING_OPERATION_TIMEOUT_MS = 30000 // 30 seconds
private providerProfileMutationQueue = Promise.resolve()

private runDelegationTransition<T>(parentTaskId: string, fn: () => Promise<T>): Promise<T> {
this.delegationTransitionLocks ??= new Map()
return runDelegationTransition(this.delegationTransitionLocks, parentTaskId, fn)
}

private enqueueProviderProfileMutation<T>(fn: (signal: AbortSignal) => Promise<T>): Promise<T> {
const controller = new AbortController()
// Run fn after either outcome so a rejected mutation never poisons the queue.
const run = this.providerProfileMutationQueue.then(
() => fn(controller.signal),
() => fn(controller.signal),
)
const callerResult = this.withProviderProfileMutationTimeout(run, () => {
controller.abort()
this.log("Provider profile mutation timed out; aborting in-flight mutation")
})

void run.then(
() => {
if (controller.signal.aborted) {
this.log("Provider profile mutation completed after cancellation")
}
},
(error) => {
if (controller.signal.aborted) {
this.log(
`Provider profile mutation errored after cancellation: ${
error instanceof Error ? error.message : String(error)
}`,
)
}
},
)

// Advance from the timeout-bounded result. Each fn checks its AbortSignal before
// writing state, so advancing the queue on timeout cannot produce stale overwrites.
this.providerProfileMutationQueue = callerResult.then(
() => undefined,
() => undefined,
)
return callerResult
}
Comment on lines +236 to +243

@coderabbitai coderabbitai Bot Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

The comment claims a stronger guarantee than the code provides.

The queue advances from callerResult, so after a 30-second timeout a new mutation starts while the abandoned mutation may still run. The comment states that each fn checks its AbortSignal before writing state. The current implementations do not meet that condition:

  • handleModeSwitchUnlocked writes task history, _taskMode, mode, and emits ModeChanged (lines 1588-1612) before any abort check.
  • handleModeSwitchUnlocked writes listApiConfigMeta (line 1632) and calls setModeConfig (line 1668) after the last abort check, with several awaits in between.
  • activateProviderProfileUnlocked performs all context and profile writes after its single check at line 1869.
  • upsertProviderProfile performs the whole activation block (lines 1747-1777) after its single check at line 1745.

An abandoned mutation can therefore overwrite state written by the mutation that followed it in the queue. The caller also receives a rejection while the write can still land later, so the reported outcome and the persisted state can disagree.

Add an abort check immediately before each state write, or route writes through a helper that returns early when the signal is aborted. Alternatively, weaken the comment to describe the actual guarantee.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/webview/ClineProvider.ts` around lines 236 - 243, Ensure the queue
comment matches the actual mutation behavior: either add abort-signal checks
immediately before every state write in handleModeSwitchUnlocked,
activateProviderProfileUnlocked, and upsertProviderProfile (including writes
after awaits), or weaken the comment to state only the guarantee currently
provided by advancing from callerResult. Preserve the existing timeout and queue
semantics.


private withProviderProfileMutationTimeout<T>(operation: Promise<T>, onTimeout: () => void): Promise<T> {
let timeoutId: ReturnType<typeof setTimeout> | undefined
const timeout = new Promise<never>((_, reject) => {
timeoutId = setTimeout(() => {
onTimeout()
reject(new Error("Provider profile mutation timed out"))
}, ClineProvider.PENDING_OPERATION_TIMEOUT_MS)
})

return Promise.race([operation, timeout]).finally(() => {
if (timeoutId) {
clearTimeout(timeoutId)
}
})
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

private readonly pendingEditOperations: PendingEditOperationStore

private cloudOrganizationsCache: CloudOrganizationMembership[] | null = null
Expand Down Expand Up @@ -1507,9 +1563,21 @@ export class ClineProvider
/**
* Handle switching to a new mode, including updating the associated API configuration
* @param newMode The mode to switch to
* @param targetTask The task whose in-memory mode should be updated. Defaults to the
* current task. Pass null to apply only global mode/profile effects for a pending child.
*/
public async handleModeSwitch(newMode: Mode) {
const task = this.getCurrentTask()
public async handleModeSwitch(newMode: Mode, targetTask: Task | null | undefined = this.getCurrentTask()) {
return this.enqueueProviderProfileMutation((signal) =>
this.handleModeSwitchUnlocked(newMode, targetTask, signal),
)
}

private async handleModeSwitchUnlocked(
newMode: Mode,
targetTask: Task | null | undefined,
signal?: AbortSignal,
): Promise<void> {
const task = targetTask
Comment thread
coderabbitai[bot] marked this conversation as resolved.

if (task) {
TelemetryService.instance.captureModeSwitch(task.taskId, newMode)
Expand Down Expand Up @@ -1546,14 +1614,20 @@ export class ClineProvider
// If workspace lock is on, keep the current API config — don't load mode-specific config
const lockApiConfigAcrossModes = this.context.workspaceState.get("lockApiConfigAcrossModes", false)
if (lockApiConfigAcrossModes) {
await this.postStateToWebview()
if (targetTask !== null) {
await this.postStateToWebview()
}
return
}

if (signal?.aborted) return

// Load the saved API config for the new mode if it exists.
const savedConfigId = await this.providerSettingsManager.getModeConfigId(newMode)
const listApiConfig = await this.providerSettingsManager.listConfig()

if (signal?.aborted) return

// Update listApiConfigMeta first to ensure UI has latest data.
await this.updateGlobalState("listApiConfigMeta", listApiConfig)

Expand All @@ -1572,7 +1646,11 @@ export class ClineProvider
const hasActualSettings = !!fullProfile.apiProvider

if (hasActualSettings) {
await this.activateProviderProfile({ name: profile.name })
await this.activateProviderProfileUnlocked(
{ name: profile.name },
targetTask === null ? { skipCurrentTaskRebuild: true } : undefined,
signal,
)
} else {
// The task will continue with the current/default configuration.
}
Expand All @@ -1592,7 +1670,9 @@ export class ClineProvider
}
}

await this.postStateToWebview()
if (targetTask !== null) {
await this.postStateToWebview()
}
}

// Provider Profile Management
Expand All @@ -1608,8 +1688,9 @@ export class ClineProvider
*/
private updateTaskApiHandlerIfNeeded(
providerSettings: ProviderSettings,
options: { forceRebuild?: boolean } = {},
options: { forceRebuild?: boolean; skipCurrentTaskRebuild?: boolean } = {},
): void {
if (options.skipCurrentTaskRebuild) return
const task = this.getCurrentTask()
if (!task) return

Expand Down Expand Up @@ -1653,45 +1734,49 @@ export class ClineProvider
activate: boolean = true,
): Promise<string | undefined> {
try {
// TODO: Do we need to be calling `activateProfile`? It's not
// clear to me what the source of truth should be; in some cases
// we rely on the `ContextProxy`'s data store and in other cases
// we rely on the `ProviderSettingsManager`'s data store. It might
// be simpler to unify these two.
const id = await this.providerSettingsManager.saveConfig(name, providerSettings)

if (activate) {
const { mode } = await this.getState()

// These promises do the following:
// 1. Adds or updates the list of provider profiles.
// 2. Sets the current provider profile.
// 3. Sets the current mode's provider profile.
// 4. Copies the provider settings to the context.
//
// Note: 1, 2, and 4 can be done in one `ContextProxy` call:
// this.contextProxy.setValues({ ...providerSettings, listApiConfigMeta: ..., currentApiConfigName: ... })
// We should probably switch to that and verify that it works.
// I left the original implementation in just to be safe.
await Promise.all([
this.updateGlobalState("listApiConfigMeta", await this.providerSettingsManager.listConfig()),
this.updateGlobalState("currentApiConfigName", name),
this.providerSettingsManager.setModeConfig(mode, id),
this.contextProxy.setProviderSettings(providerSettings),
])

// Change the provider for the current task.
// TODO: We should rename `buildApiHandler` for clarity (e.g. `getProviderClient`).
this.updateTaskApiHandlerIfNeeded(providerSettings, { forceRebuild: true })

// Keep the current task's sticky provider profile in sync with the newly-activated profile.
await this.persistStickyProviderProfileToCurrentTask(name)
} else {
await this.updateGlobalState("listApiConfigMeta", await this.providerSettingsManager.listConfig())
}
return await this.enqueueProviderProfileMutation(async (signal) => {
// TODO: Do we need to be calling `activateProfile`? It's not
// clear to me what the source of truth should be; in some cases
// we rely on the `ContextProxy`'s data store and in other cases
// we rely on the `ProviderSettingsManager`'s data store. It might
// be simpler to unify these two.
const id = await this.providerSettingsManager.saveConfig(name, providerSettings)

if (signal.aborted) return id

if (activate) {
const { mode } = await this.getState()

// These promises do the following:
// 1. Adds or updates the list of provider profiles.
// 2. Sets the current provider profile.
// 3. Sets the current mode's provider profile.
// 4. Copies the provider settings to the context.
//
// Note: 1, 2, and 4 can be done in one `ContextProxy` call:
// this.contextProxy.setValues({ ...providerSettings, listApiConfigMeta: ..., currentApiConfigName: ... })
// We should probably switch to that and verify that it works.
// I left the original implementation in just to be safe.
await Promise.all([
this.updateGlobalState("listApiConfigMeta", await this.providerSettingsManager.listConfig()),
this.updateGlobalState("currentApiConfigName", name),
this.providerSettingsManager.setModeConfig(mode, id),
this.contextProxy.setProviderSettings(providerSettings),
])

// Change the provider for the current task.
// TODO: We should rename `buildApiHandler` for clarity (e.g. `getProviderClient`).
this.updateTaskApiHandlerIfNeeded(providerSettings, { forceRebuild: true })

// Keep the current task's sticky provider profile in sync with the newly-activated profile.
await this.persistStickyProviderProfileToCurrentTask(name)
} else {
await this.updateGlobalState("listApiConfigMeta", await this.providerSettingsManager.listConfig())
}

await this.postStateToWebview()
return id
await this.postStateToWebview()
return id
})
} catch (error) {
this.log(
`Error create new api configuration: ${JSON.stringify(error, Object.getOwnPropertyNames(error), 2)}`,
Expand Down Expand Up @@ -1725,7 +1810,11 @@ export class ClineProvider
await this.postStateToWebview()
}

private async persistStickyProviderProfileToCurrentTask(apiConfigName: string): Promise<void> {
private async persistStickyProviderProfileToCurrentTask(
apiConfigName: string,
options: { skipCurrentTaskRebuild?: boolean } = {},
): Promise<void> {
if (options.skipCurrentTaskRebuild) return
const task = this.getCurrentTask()
if (!task) {
return
Expand Down Expand Up @@ -1755,19 +1844,42 @@ export class ClineProvider

async activateProviderProfile(
args: { name: string } | { id: string },
options?: { persistModeConfig?: boolean; persistTaskHistory?: boolean },
options?: {
persistModeConfig?: boolean
persistTaskHistory?: boolean
skipCurrentTaskRebuild?: boolean
},
) {
return this.enqueueProviderProfileMutation((signal) =>
this.activateProviderProfileUnlocked(args, options, signal),
)
}

private async activateProviderProfileUnlocked(
args: { name: string } | { id: string },
options?: {
persistModeConfig?: boolean
persistTaskHistory?: boolean
skipCurrentTaskRebuild?: boolean
},
signal?: AbortSignal,
): Promise<void> {
const { name, id, ...providerSettings } = await this.providerSettingsManager.activateProfile(args)

if (signal?.aborted) return

const persistModeConfig = options?.persistModeConfig ?? true
const persistTaskHistory = options?.persistTaskHistory ?? true

// See `upsertProviderProfile` for a description of what this is doing.
await Promise.all([
this.contextProxy.setValue("listApiConfigMeta", await this.providerSettingsManager.listConfig()),
this.contextProxy.setValue("currentApiConfigName", name),
this.contextProxy.setProviderSettings(providerSettings),
])
const skipCurrentTaskRebuild = options?.skipCurrentTaskRebuild ?? false

if (!skipCurrentTaskRebuild) {
// See `upsertProviderProfile` for a description of what this is doing.
await Promise.all([
this.contextProxy.setValue("listApiConfigMeta", await this.providerSettingsManager.listConfig()),
this.contextProxy.setValue("currentApiConfigName", name),
this.contextProxy.setProviderSettings(providerSettings),
])
}
Comment on lines 1867 to +1882

@coderabbitai coderabbitai Bot Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

fd -t f 'ProviderSettingsManager.ts' --exec ast-grep outline {} --items all

fd -t f 'ProviderSettingsManager.ts' --exec rg -n -C 25 'activateProfile' {}

Repository: Zoo-Code-Org/Zoo-Code

Length of output: 3025

Avoid synchronizing the store when skipCurrentTaskRebuild is true.

activateProfile writes currentApiConfigName to providerProfiles; this path then leaves ContextProxy.currentApiConfigName unchanged. Call providerSettingsManager.activateProfile(args) only in the non-skipped branch, or do not persist the active profile in activateProfile for this preparation-only path.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/webview/ClineProvider.ts` around lines 1867 - 1882, Update the
profile activation flow around activateProfile so skipCurrentTaskRebuild also
prevents synchronizing the active profile into ContextProxy. Either call
providerSettingsManager.activateProfile(args) only when rebuilding, or provide
an equivalent preparation-only path that avoids persisting currentApiConfigName;
preserve the existing synchronization behavior when skipCurrentTaskRebuild is
false.


const { mode } = await this.getState()

Expand All @@ -1776,17 +1888,19 @@ export class ClineProvider
}

// Change the provider for the current task.
this.updateTaskApiHandlerIfNeeded(providerSettings, { forceRebuild: true })
this.updateTaskApiHandlerIfNeeded(providerSettings, { forceRebuild: true, skipCurrentTaskRebuild })

// Update the current task's sticky provider profile, unless this activation is
// being used purely as a non-persisting restoration (e.g., reopening a task from history).
if (persistTaskHistory) {
await this.persistStickyProviderProfileToCurrentTask(name)
await this.persistStickyProviderProfileToCurrentTask(name, { skipCurrentTaskRebuild })
}

await this.postStateToWebview()
if (!skipCurrentTaskRebuild) {
await this.postStateToWebview()
}

if (providerSettings.apiProvider) {
if (providerSettings.apiProvider && !skipCurrentTaskRebuild) {
this.emit(RooCodeEventName.ProviderProfileChanged, { name, provider: providerSettings.apiProvider })
}
}
Expand Down
Loading
Loading