Skip to content

fix: update API handler when toolProtocol changes #9599

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
mrubens merged 1 commit into from
Nov 26, 2025
Merged

fix: update API handler when toolProtocol changes #9599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions src/core/task/Task.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
try {
const newState = await provider.getState()
if (newState?.apiConfiguration) {
await this.updateApiConfiguration(newState.apiConfiguration)
this.updateApiConfiguration(newState.apiConfiguration)
}
} catch (error) {
console.error(
Expand Down Expand Up @@ -1167,7 +1167,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
*
* @param newApiConfiguration - The new API configuration to use
*/
public async updateApiConfiguration(newApiConfiguration: ProviderSettings): Promise<void> {
public updateApiConfiguration(newApiConfiguration: ProviderSettings): void {
// Update the configuration and rebuild the API handler
this.apiConfiguration = newApiConfiguration
this.api = buildApiHandler(newApiConfiguration)
Expand Down Expand Up @@ -1222,7 +1222,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
// This ensures the parser state is synchronized with the selected model
const newState = await provider.getState()
if (newState?.apiConfiguration) {
await this.updateApiConfiguration(newState.apiConfiguration)
this.updateApiConfiguration(newState.apiConfiguration)
}
}

Expand Down
26 changes: 19 additions & 7 deletions src/core/webview/ClineProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1322,16 +1322,28 @@ export class ClineProvider
const prevConfig = task.apiConfiguration
const prevProvider = prevConfig?.apiProvider
const prevModelId = prevConfig ? getModelId(prevConfig) : undefined
const prevToolProtocol = prevConfig?.toolProtocol
const newProvider = providerSettings.apiProvider
const newModelId = getModelId(providerSettings)

if (forceRebuild || prevProvider !== newProvider || prevModelId !== newModelId) {
task.api = buildApiHandler(providerSettings)
const newToolProtocol = providerSettings.toolProtocol

const needsRebuild =
forceRebuild ||
prevProvider !== newProvider ||
prevModelId !== newModelId ||
prevToolProtocol !== newToolProtocol

if (needsRebuild) {
// Use updateApiConfiguration which handles both API handler rebuild and parser sync.
// This is important when toolProtocol changes - the assistantMessageParser needs to be
// created/destroyed to match the new protocol (XML vs native).
// Note: updateApiConfiguration is declared async but has no actual async operations,
// so we can safely call it without awaiting.
task.updateApiConfiguration(providerSettings)
} else {
// No rebuild needed, just sync apiConfiguration
;(task as any).apiConfiguration = providerSettings
}

// Always sync the task's apiConfiguration with the latest provider settings.
// Note: Task.apiConfiguration is declared readonly in types, so we cast to any for runtime update.
;(task as any).apiConfiguration = providerSettings
}

getProviderProfileEntries(): ProviderSettingsEntry[] {
Expand Down
78 changes: 32 additions & 46 deletions src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ vi.mock("../../task/Task", () => ({
overwriteApiConversationHistory: vi.fn(),
taskId: options?.historyItem?.id || "test-task-id",
emit: vi.fn(),
updateApiConfiguration: vi.fn().mockImplementation(function (this: any, newConfig: any) {
this.apiConfiguration = newConfig
}),
}
// Define apiConfiguration as a property so tests can read it
Object.defineProperty(mockTask, "apiConfiguration", {
Expand Down Expand Up @@ -250,7 +253,7 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
})

describe("upsertProviderProfile", () => {
test("rebuilds API handler when provider/model unchanged but profile settings changed (explicit save)", async () => {
test("calls updateApiConfiguration when provider/model unchanged but profile settings changed (explicit save)", async () => {
// Create a task with the current config
const mockTask = new Task({
...defaultTaskOptions,
Expand All @@ -259,19 +262,15 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
openRouterModelId: "openai/gpt-4",
},
})
const originalApi = {
mockTask.api = {
getModel: vi.fn().mockReturnValue({
id: "openai/gpt-4",
info: { contextWindow: 128000 },
}),
}
mockTask.api = originalApi as any
} as any

await provider.addClineToStack(mockTask)

// Clear the mock to track new calls
buildApiHandlerMock.mockClear()

// Save settings with SAME provider and model (simulating Save button click)
await provider.upsertProviderProfile(
"test-config",
Expand All @@ -285,22 +284,22 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
true,
)

// Verify buildApiHandler WAS called because we force rebuild on explicit save/switch
expect(buildApiHandlerMock).toHaveBeenCalledWith(
// Verify updateApiConfiguration was called because we force rebuild on explicit save/switch
expect(mockTask.updateApiConfiguration).toHaveBeenCalledWith(
expect.objectContaining({
apiProvider: "openrouter",
openRouterModelId: "openai/gpt-4",
rateLimitSeconds: 5,
modelTemperature: 0.7,
}),
)
// Verify the task's api property was reassigned (new client)
expect(mockTask.api).not.toBe(originalApi)
// Verify task.apiConfiguration was synchronized with non-model fields
// Verify task.apiConfiguration was synchronized
expect((mockTask as any).apiConfiguration.openRouterModelId).toBe("openai/gpt-4")
expect((mockTask as any).apiConfiguration.rateLimitSeconds).toBe(5)
expect((mockTask as any).apiConfiguration.modelTemperature).toBe(0.7)
})

test("rebuilds API handler when provider changes", async () => {
test("calls updateApiConfiguration when provider changes", async () => {
const mockTask = new Task({
...defaultTaskOptions,
apiConfiguration: {
Expand All @@ -317,8 +316,6 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {

await provider.addClineToStack(mockTask)

buildApiHandlerMock.mockClear()

// Change provider to anthropic
await provider.upsertProviderProfile(
"test-config",
Expand All @@ -329,16 +326,16 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
true,
)

// Verify buildApiHandler WAS called since provider changed
expect(buildApiHandlerMock).toHaveBeenCalledWith(
// Verify updateApiConfiguration was called since provider changed
expect(mockTask.updateApiConfiguration).toHaveBeenCalledWith(
expect.objectContaining({
apiProvider: "anthropic",
apiModelId: "claude-3-5-sonnet-20241022",
}),
)
})

test("rebuilds API handler when model changes", async () => {
test("calls updateApiConfiguration when model changes", async () => {
const mockTask = new Task({
...defaultTaskOptions,
apiConfiguration: {
Expand All @@ -355,8 +352,6 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {

await provider.addClineToStack(mockTask)

buildApiHandlerMock.mockClear()

// Change model to different model
await provider.upsertProviderProfile(
"test-config",
Expand All @@ -367,8 +362,8 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
true,
)

// Verify buildApiHandler WAS called since model changed
expect(buildApiHandlerMock).toHaveBeenCalledWith(
// Verify updateApiConfiguration was called since model changed
expect(mockTask.updateApiConfiguration).toHaveBeenCalledWith(
expect.objectContaining({
apiProvider: "openrouter",
openRouterModelId: "anthropic/claude-3-5-sonnet-20241022",
Expand All @@ -395,7 +390,7 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
})

describe("activateProviderProfile", () => {
test("rebuilds API handler when provider/model unchanged but settings differ (explicit profile switch)", async () => {
test("calls updateApiConfiguration when provider/model unchanged but settings differ (explicit profile switch)", async () => {
const mockTask = new Task({
...defaultTaskOptions,
apiConfiguration: {
Expand All @@ -404,18 +399,15 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
modelTemperature: 0.3,
},
})
const originalApi = {
mockTask.api = {
getModel: vi.fn().mockReturnValue({
id: "openai/gpt-4",
info: { contextWindow: 128000 },
}),
}
mockTask.api = originalApi as any
} as any

await provider.addClineToStack(mockTask)

buildApiHandlerMock.mockClear()

// Mock activateProfile to return same provider/model but different non-model setting
;(provider as any).providerSettingsManager.activateProfile = vi.fn().mockResolvedValue({
name: "test-config",
Expand All @@ -428,22 +420,20 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {

await provider.activateProviderProfile({ name: "test-config" })

// Verify buildApiHandler WAS called due to forced rebuild on explicit switch
expect(buildApiHandlerMock).toHaveBeenCalledWith(
// Verify updateApiConfiguration was called due to forced rebuild on explicit switch
expect(mockTask.updateApiConfiguration).toHaveBeenCalledWith(
expect.objectContaining({
apiProvider: "openrouter",
openRouterModelId: "openai/gpt-4",
}),
)
// Verify the API reference changed
expect(mockTask.api).not.toBe(originalApi)
// Verify task.apiConfiguration was synchronized
expect((mockTask as any).apiConfiguration.openRouterModelId).toBe("openai/gpt-4")
expect((mockTask as any).apiConfiguration.modelTemperature).toBe(0.9)
expect((mockTask as any).apiConfiguration.rateLimitSeconds).toBe(7)
})

test("rebuilds API handler when provider changes and syncs task.apiConfiguration", async () => {
test("calls updateApiConfiguration when provider changes and syncs task.apiConfiguration", async () => {
const mockTask = new Task({
...defaultTaskOptions,
apiConfiguration: {
Expand All @@ -460,8 +450,6 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {

await provider.addClineToStack(mockTask)

buildApiHandlerMock.mockClear()

// Mock activateProfile to return different provider
;(provider as any).providerSettingsManager.activateProfile = vi.fn().mockResolvedValue({
name: "anthropic-config",
Expand All @@ -472,8 +460,8 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {

await provider.activateProviderProfile({ name: "anthropic-config" })

// Verify buildApiHandler WAS called
expect(buildApiHandlerMock).toHaveBeenCalledWith(
// Verify updateApiConfiguration was called
expect(mockTask.updateApiConfiguration).toHaveBeenCalledWith(
expect.objectContaining({
apiProvider: "anthropic",
apiModelId: "claude-3-5-sonnet-20241022",
Expand All @@ -484,7 +472,7 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
expect((mockTask as any).apiConfiguration.apiModelId).toBe("claude-3-5-sonnet-20241022")
})

test("rebuilds API handler when model changes and syncs task.apiConfiguration", async () => {
test("calls updateApiConfiguration when model changes and syncs task.apiConfiguration", async () => {
const mockTask = new Task({
...defaultTaskOptions,
apiConfiguration: {
Expand All @@ -501,8 +489,6 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {

await provider.addClineToStack(mockTask)

buildApiHandlerMock.mockClear()

// Mock activateProfile to return different model
;(provider as any).providerSettingsManager.activateProfile = vi.fn().mockResolvedValue({
name: "test-config",
Expand All @@ -513,8 +499,8 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {

await provider.activateProviderProfile({ name: "test-config" })

// Verify buildApiHandler WAS called
expect(buildApiHandlerMock).toHaveBeenCalledWith(
// Verify updateApiConfiguration was called
expect(mockTask.updateApiConfiguration).toHaveBeenCalledWith(
expect.objectContaining({
apiProvider: "openrouter",
openRouterModelId: "anthropic/claude-3-5-sonnet-20241022",
Expand Down Expand Up @@ -545,7 +531,6 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
await provider.addClineToStack(mockTask)

// First switch: A -> B (openrouter -> anthropic)
buildApiHandlerMock.mockClear()
;(provider as any).providerSettingsManager.activateProfile = vi.fn().mockResolvedValue({
name: "anthropic-config",
id: "anthropic-id",
Expand All @@ -554,12 +539,12 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
})
await provider.activateProviderProfile({ name: "anthropic-config" })

expect(buildApiHandlerMock).toHaveBeenCalled()
expect(mockTask.updateApiConfiguration).toHaveBeenCalled()
expect((mockTask as any).apiConfiguration.apiProvider).toBe("anthropic")
expect((mockTask as any).apiConfiguration.apiModelId).toBe("claude-3-5-sonnet-20241022")

// Second switch: B -> A (anthropic -> openrouter gpt-4)
buildApiHandlerMock.mockClear()
;(mockTask.updateApiConfiguration as any).mockClear()
;(provider as any).providerSettingsManager.activateProfile = vi.fn().mockResolvedValue({
name: "test-config",
id: "test-id",
Expand All @@ -568,7 +553,8 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
})
await provider.activateProviderProfile({ name: "test-config" })

// API handler may or may not rebuild depending on mock model id, but apiConfiguration must be updated
// updateApiConfiguration called again, and apiConfiguration must be updated
expect(mockTask.updateApiConfiguration).toHaveBeenCalled()
expect((mockTask as any).apiConfiguration.apiProvider).toBe("openrouter")
expect((mockTask as any).apiConfiguration.openRouterModelId).toBe("openai/gpt-4")
})
Expand Down
Loading
Loading