scriptoriumadmin/Roo-Code
1
0
Fork 0
mirror of https://github.com/RooVetGit/Roo-Code.git synced 2026-08-28 05:27:24 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: update API handler when toolProtocol changes (#9599)

Browse source
This commit is contained in:
Matt Rubens 2025-11-25 23:54:12 -05:00 • committed by GitHub
parent 099ea6dc6a
commit 0fe55b9403
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 64 additions and 55 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
src/core/task/Task.ts
View file

@ -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(
@ -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)
@ -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)
}
}

24
src/core/webview/ClineProvider.ts
View file

@ -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)
const newToolProtocol = providerSettings.toolProtocol
if (forceRebuild || prevProvider !== newProvider || prevModelId !== newModelId) {
task.api = buildApiHandler(providerSettings)
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[] {

78
src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts
View file

@ -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", {
@ -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,
@ -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",
@ -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: {
@ -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",
@ -329,8 +326,8 @@ 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",
@ -338,7 +335,7 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
)
})
test("rebuilds API handler when model changes", async () => {
test("calls updateApiConfiguration when model changes", async () => {
const mockTask = new Task({
...defaultTaskOptions,
apiConfiguration: {
@ -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",
@ -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",
@ -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: {
@ -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",
@ -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: {
@ -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",
@ -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",
@ -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: {
@ -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",
@ -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",
@ -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",
@ -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",
@ -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")
})

11
src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts
View file

@ -73,6 +73,7 @@ vi.mock("../../task/Task", () => ({
setRootTask: vi.fn(),
emit: vi.fn(),
parentTask: options.parentTask,
updateApiConfiguration: vi.fn(),
})),
}))
@ -334,6 +335,7 @@ describe("ClineProvider - Sticky Mode", () => {
saveClineMessages: vi.fn(),
clineMessages: [],
apiConversationHistory: [],
updateApiConfiguration: vi.fn(),
}
// Add task to provider stack
@ -781,6 +783,7 @@ describe("ClineProvider - Sticky Mode", () => {
saveClineMessages: vi.fn(),
clineMessages: [],
apiConversationHistory: [],
updateApiConfiguration: vi.fn(),
}
// Add task to provider stack
@ -847,6 +850,7 @@ describe("ClineProvider - Sticky Mode", () => {
}),
clineMessages: [],
apiConversationHistory: [],
updateApiConfiguration: vi.fn(),
}
// Add task to provider stack
@ -898,6 +902,7 @@ describe("ClineProvider - Sticky Mode", () => {
saveClineMessages: vi.fn(),
clineMessages: [],
apiConversationHistory: [],
updateApiConfiguration: vi.fn(),
}
// Add task to provider stack
@ -932,6 +937,7 @@ describe("ClineProvider - Sticky Mode", () => {
saveClineMessages: vi.fn(),
clineMessages: [],
apiConversationHistory: [],
updateApiConfiguration: vi.fn(),
}
// Add task to provider stack
@ -987,6 +993,7 @@ describe("ClineProvider - Sticky Mode", () => {
saveClineMessages: vi.fn(),
clineMessages: [],
apiConversationHistory: [],
updateApiConfiguration: vi.fn(),
}
// Add task to provider stack
@ -1033,6 +1040,7 @@ describe("ClineProvider - Sticky Mode", () => {
saveClineMessages: vi.fn(),
clineMessages: [],
apiConversationHistory: [],
updateApiConfiguration: vi.fn(),
}
const task2 = {
@ -1042,6 +1050,7 @@ describe("ClineProvider - Sticky Mode", () => {
saveClineMessages: vi.fn(),
clineMessages: [],
apiConversationHistory: [],
updateApiConfiguration: vi.fn(),
}
const task3 = {
@ -1051,6 +1060,7 @@ describe("ClineProvider - Sticky Mode", () => {
saveClineMessages: vi.fn(),
clineMessages: [],
apiConversationHistory: [],
updateApiConfiguration: vi.fn(),
}
// Add tasks to provider stack
@ -1192,6 +1202,7 @@ describe("ClineProvider - Sticky Mode", () => {
saveClineMessages: vi.fn(),
clineMessages: [],
apiConversationHistory: [],
updateApiConfiguration: vi.fn(),
}))
// Add all tasks to provider