From 732f3d23e40f4b5a02acaf018077d6d005bfb30b Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 10 Mar 2026 15:36:35 +0200 Subject: [PATCH 1/6] fix: only disable parallel tool calls when injectable tools are present Signed-off-by: Danny Kopping --- .../chatcompletions/single_builtin_tool.txtar | 1 + intercept/chatcompletions/base.go | 10 +++ intercept/chatcompletions/streaming.go | 6 -- intercept/interceptor.go | 3 + intercept/messages/base.go | 4 + intercept/responses/base.go | 4 + intercept/responses/blocking.go | 1 - intercept/responses/injected_tools.go | 3 + intercept/responses/streaming.go | 1 - internal/integrationtest/bridge_test.go | 77 +++++++++++++++++++ internal/integrationtest/responses_test.go | 33 +++++++- 11 files changed, 134 insertions(+), 9 deletions(-) diff --git a/fixtures/openai/chatcompletions/single_builtin_tool.txtar b/fixtures/openai/chatcompletions/single_builtin_tool.txtar index 0eae8212..55e53f96 100644 --- a/fixtures/openai/chatcompletions/single_builtin_tool.txtar +++ b/fixtures/openai/chatcompletions/single_builtin_tool.txtar @@ -9,6 +9,7 @@ LLM (https://llm.datasette.io/) configured with a simple "read_file" tool. } ], "model": "gpt-4.1", + "parallel_tool_calls": true, "tools": [ { "type": "function", diff --git a/intercept/chatcompletions/base.go b/intercept/chatcompletions/base.go index 7a755e06..ad1712a7 100644 --- a/intercept/chatcompletions/base.go +++ b/intercept/chatcompletions/base.go @@ -107,6 +107,12 @@ func (i *interceptionBase) injectTools() { return } + // Disable parallel tool calls when injectable tools are present to simplify the inner agentic loop. + // Only set when there are tools in the request, otherwise it causes a 400 Bad Request. + if i.HasInjectableTools() && len(i.req.Tools) > 0 { + i.req.ParallelToolCalls = openai.Bool(false) + } + // Inject tools. for _, tool := range i.mcpProxy.ListTools() { fn := openai.ChatCompletionToolUnionParam{ @@ -171,6 +177,10 @@ func (i *interceptionBase) writeUpstreamError(w http.ResponseWriter, oaiErr *err } } +func (i *interceptionBase) HasInjectableTools() bool { + return i.mcpProxy != nil && len(i.mcpProxy.ListTools()) > 0 +} + func sumUsage(ref, in openai.CompletionUsage) openai.CompletionUsage { return openai.CompletionUsage{ CompletionTokens: ref.CompletionTokens + in.CompletionTokens, diff --git a/intercept/chatcompletions/streaming.go b/intercept/chatcompletions/streaming.go index ff3b78c6..550bb448 100644 --- a/intercept/chatcompletions/streaming.go +++ b/intercept/chatcompletions/streaming.go @@ -97,12 +97,6 @@ func (i *StreamingInterception) ProcessRequest(w http.ResponseWriter, r *http.Re _ = events.Shutdown(streamCtx) // Catch-all in case it doesn't get shutdown after stream completes. }() - // TODO: implement parallel tool calls. - // TODO: don't send if not supported by model (i.e. o4-mini). - if len(i.req.Tools) > 0 { // If no tools are specified but this setting is set, it'll cause a 400 Bad Request. - i.req.ParallelToolCalls = openai.Bool(false) - } - // Force responses to only have one choice. // It's unnecessary to generate multiple responses, and would complicate our stream processing logic if // multiple choices were returned. diff --git a/intercept/interceptor.go b/intercept/interceptor.go index cbd29d62..fee427cf 100644 --- a/intercept/interceptor.go +++ b/intercept/interceptor.go @@ -34,4 +34,7 @@ type Interceptor interface { // by the model, so any single tool call ID is sufficient to identify the // parent interception. CorrelatingToolCallID() *string + // HasInjectableTools returns true if an [mcp.ServerProxier] has been provided + // and contains tools which must be injected into requests. + HasInjectableTools() bool } diff --git a/intercept/messages/base.go b/intercept/messages/base.go index c61a3a7c..547f7ae8 100644 --- a/intercept/messages/base.go +++ b/intercept/messages/base.go @@ -315,6 +315,10 @@ func (i *interceptionBase) writeUpstreamError(w http.ResponseWriter, antErr *Err } } +func (i *interceptionBase) HasInjectableTools() bool { + return i.mcpProxy != nil && len(i.mcpProxy.ListTools()) > 0 +} + // accumulateUsage accumulates usage statistics from source into dest. // It handles both [anthropic.Usage] and [anthropic.MessageDeltaUsage] types through [any]. // The function uses reflection to handle the differences between the types: diff --git a/intercept/responses/base.go b/intercept/responses/base.go index 8b7c3ded..9aafbd0e 100644 --- a/intercept/responses/base.go +++ b/intercept/responses/base.go @@ -326,6 +326,10 @@ func (i *responsesInterceptionBase) recordTokenUsage(ctx context.Context, respon } } +func (i *responsesInterceptionBase) HasInjectableTools() bool { + return i.mcpProxy != nil && len(i.mcpProxy.ListTools()) > 0 +} + // responseCopier helper struct to send original response to the client type responseCopier struct { buff deltaBuffer diff --git a/intercept/responses/blocking.go b/intercept/responses/blocking.go index 3e94a6cc..0c11a541 100644 --- a/intercept/responses/blocking.go +++ b/intercept/responses/blocking.go @@ -59,7 +59,6 @@ func (i *BlockingResponsesInterceptor) ProcessRequest(w http.ResponseWriter, r * } i.injectTools() - i.disableParallelToolCalls() var ( response *responses.Response diff --git a/intercept/responses/injected_tools.go b/intercept/responses/injected_tools.go index 8a478012..885c7e1b 100644 --- a/intercept/responses/injected_tools.go +++ b/intercept/responses/injected_tools.go @@ -25,6 +25,9 @@ func (i *responsesInterceptionBase) injectTools() { return } + // If there are injectable tools, disable parallel tool calls. + i.disableParallelToolCalls() + // Inject tools. for _, tool := range tools { var params map[string]any diff --git a/intercept/responses/streaming.go b/intercept/responses/streaming.go index 6925d86f..38f5771b 100644 --- a/intercept/responses/streaming.go +++ b/intercept/responses/streaming.go @@ -70,7 +70,6 @@ func (i *StreamingResponsesInterceptor) ProcessRequest(w http.ResponseWriter, r } i.injectTools() - i.disableParallelToolCalls() events := eventstream.NewEventStream(ctx, i.logger.Named("sse-sender"), nil) go events.Start(w, r) diff --git a/internal/integrationtest/bridge_test.go b/internal/integrationtest/bridge_test.go index 04f5ad14..12e0bf19 100644 --- a/internal/integrationtest/bridge_test.go +++ b/internal/integrationtest/bridge_test.go @@ -1308,6 +1308,83 @@ func TestAnthropicToolChoiceParallelDisabled(t *testing.T) { } } +// TestChatCompletionsParallelToolCallsDisabled verifies that parallel_tool_calls +// is set to false only when injectable MCP tools are present and the request +// includes tools. +func TestChatCompletionsParallelToolCallsDisabled(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + streaming bool + withInjectedTools bool + expectParallelToolCalls bool + }{ + // Streaming with injected tools: parallel_tool_calls should be forced false. + { + name: "streaming/with_injected_tools", + streaming: true, + withInjectedTools: true, + expectParallelToolCalls: false, + }, + // Streaming without injected tools: parallel_tool_calls preserved. + { + name: "streaming/no_injected_tools", + streaming: true, + withInjectedTools: false, + expectParallelToolCalls: true, + }, + // Blocking with injected tools: parallel_tool_calls should be forced false. + { + name: "blocking/with_injected_tools", + streaming: false, + withInjectedTools: true, + expectParallelToolCalls: false, + }, + { + name: "blocking/no_injected_tools", + streaming: false, + withInjectedTools: false, + expectParallelToolCalls: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(t.Context(), time.Second*30) + t.Cleanup(cancel) + + fix := fixtures.Parse(t, fixtures.OaiChatSingleBuiltinTool) + upstream := newMockUpstream(t, ctx, newFixtureResponse(fix)) + + var opts []bridgeOption + if tc.withInjectedTools { + opts = append(opts, withMCP(setupMCPForTest(t, defaultTracer))) + } + bridgeServer := newBridgeTestServer(t, ctx, upstream.URL, opts...) + + reqBody, err := sjson.SetBytes(fix.Request(), "stream", tc.streaming) + require.NoError(t, err) + + resp := bridgeServer.makeRequest(t, http.MethodPost, pathOpenAIChatCompletions, reqBody) + _, err = io.ReadAll(resp.Body) + require.NoError(t, err) + + received := upstream.receivedRequests() + require.Len(t, received, 1) + + var upstreamReq map[string]any + require.NoError(t, json.Unmarshal(received[0].Body, &upstreamReq)) + + ptc, ok := upstreamReq["parallel_tool_calls"].(bool) + require.True(t, ok, "parallel_tool_calls should be present in upstream request") + assert.Equal(t, tc.expectParallelToolCalls, ptc) + }) + } +} + func TestThinkingAdaptiveIsPreserved(t *testing.T) { t.Parallel() diff --git a/internal/integrationtest/responses_test.go b/internal/integrationtest/responses_test.go index 483ce903..ab51d39f 100644 --- a/internal/integrationtest/responses_test.go +++ b/internal/integrationtest/responses_test.go @@ -442,6 +442,7 @@ func TestResponsesParallelToolsOverwritten(t *testing.T) { name string request string streaming bool + injectMCP bool expectParallelToolCalls bool expectParallelToolCallsValue bool }{ @@ -449,6 +450,7 @@ func TestResponsesParallelToolsOverwritten(t *testing.T) { name: "blocking_with_tools", request: `{"input": "hello", "model": "gpt-4o-mini", "stream": false, "parallel_tool_calls": true, "tools": [{"type": "function", "name": "test", "parameters": {}}]}`, streaming: false, + injectMCP: true, expectParallelToolCalls: true, expectParallelToolCallsValue: false, }, @@ -456,6 +458,7 @@ func TestResponsesParallelToolsOverwritten(t *testing.T) { name: "streaming_with_tools", request: `{"input": "hello", "model": "gpt-4o-mini", "stream": true, "parallel_tool_calls": true, "tools": [{"type": "function", "name": "test", "parameters": {}}]}`, streaming: true, + injectMCP: true, expectParallelToolCalls: true, expectParallelToolCallsValue: false, }, @@ -463,6 +466,7 @@ func TestResponsesParallelToolsOverwritten(t *testing.T) { name: "blocking_with_tools_no_parallel_param", request: `{"input": "hello", "model": "gpt-4o-mini", "stream": false, "tools": [{"type": "function", "name": "test", "parameters": {}}]}`, streaming: false, + injectMCP: true, expectParallelToolCalls: true, expectParallelToolCallsValue: false, }, @@ -470,6 +474,7 @@ func TestResponsesParallelToolsOverwritten(t *testing.T) { name: "streaming_with_tools_no_parallel_param", request: `{"input": "hello", "model": "gpt-4o-mini", "stream": true, "tools": [{"type": "function", "name": "test", "parameters": {}}]}`, streaming: true, + injectMCP: true, expectParallelToolCalls: true, expectParallelToolCallsValue: false, }, @@ -477,16 +482,19 @@ func TestResponsesParallelToolsOverwritten(t *testing.T) { name: "blocking_without_tools", request: `{"input": "hello", "model": "gpt-4o-mini", "stream": false}`, streaming: false, + injectMCP: true, }, { name: "streaming_without_tools", request: `{"input": "hello", "model": "gpt-4o-mini", "stream": true}`, streaming: true, + injectMCP: true, }, { name: "blocking_without_tools_parallel_true", request: `{"input": "hello", "model": "gpt-4o-mini", "stream": false, "parallel_tool_calls": true}`, streaming: false, + injectMCP: true, expectParallelToolCalls: true, expectParallelToolCallsValue: true, }, @@ -494,6 +502,23 @@ func TestResponsesParallelToolsOverwritten(t *testing.T) { name: "streaming_without_tools_parallel_true", request: `{"input": "hello", "model": "gpt-4o-mini", "stream": true, "parallel_tool_calls": true}`, streaming: true, + injectMCP: true, + expectParallelToolCalls: true, + expectParallelToolCallsValue: true, + }, + { + name: "blocking_without_injectable_tools_true", + request: `{"input": "hello", "model": "gpt-4o-mini", "stream": false, "parallel_tool_calls": true}`, + streaming: false, + injectMCP: false, + expectParallelToolCalls: true, + expectParallelToolCallsValue: true, + }, + { + name: "streaming_without_injectable_tools_true", + request: `{"input": "hello", "model": "gpt-4o-mini", "stream": true, "parallel_tool_calls": true}`, + streaming: true, + injectMCP: false, expectParallelToolCalls: true, expectParallelToolCallsValue: true, }, @@ -511,7 +536,13 @@ func TestResponsesParallelToolsOverwritten(t *testing.T) { fix = fixtures.OaiResponsesStreamingSimple } upstream := newMockUpstream(t, ctx, newFixtureResponse(fixtures.Parse(t, fix))) - bridgeServer := newBridgeTestServer(t, ctx, upstream.URL) + + var opts []bridgeOption + if tc.injectMCP { + mockMCP := setupMCPForTest(t, defaultTracer) + opts = append(opts, withMCP(mockMCP)) + } + bridgeServer := newBridgeTestServer(t, ctx, upstream.URL, opts...) resp := bridgeServer.makeRequest(t, http.MethodPost, pathOpenAIResponses, []byte(tc.request)) _, err := io.ReadAll(resp.Body) From 930f09aab369a34fdb4e93023b43cc1262e447d3 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 11 Mar 2026 17:16:39 +0200 Subject: [PATCH 2/6] fix(chatcompletions): only modify parallel_tool_calls if set, and only if injected tools provided Signed-off-by: Danny Kopping --- .../chatcompletions/single_builtin_tool.txtar | 1 - intercept/chatcompletions/base.go | 6 +- internal/integrationtest/bridge_test.go | 134 ++++++++++++------ 3 files changed, 91 insertions(+), 50 deletions(-) diff --git a/fixtures/openai/chatcompletions/single_builtin_tool.txtar b/fixtures/openai/chatcompletions/single_builtin_tool.txtar index 55e53f96..0eae8212 100644 --- a/fixtures/openai/chatcompletions/single_builtin_tool.txtar +++ b/fixtures/openai/chatcompletions/single_builtin_tool.txtar @@ -9,7 +9,6 @@ LLM (https://llm.datasette.io/) configured with a simple "read_file" tool. } ], "model": "gpt-4.1", - "parallel_tool_calls": true, "tools": [ { "type": "function", diff --git a/intercept/chatcompletions/base.go b/intercept/chatcompletions/base.go index ad1712a7..27b58f4f 100644 --- a/intercept/chatcompletions/base.go +++ b/intercept/chatcompletions/base.go @@ -17,6 +17,7 @@ import ( "github.com/google/uuid" "github.com/openai/openai-go/v3" "github.com/openai/openai-go/v3/option" + "github.com/openai/openai-go/v3/packages/param" "github.com/openai/openai-go/v3/shared" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -108,8 +109,9 @@ func (i *interceptionBase) injectTools() { } // Disable parallel tool calls when injectable tools are present to simplify the inner agentic loop. - // Only set when there are tools in the request, otherwise it causes a 400 Bad Request. - if i.HasInjectableTools() && len(i.req.Tools) > 0 { + // Only set when tools are defined in the request, otherwise it causes a 400 Bad Request. + // The value is only changed if it is present. + if i.HasInjectableTools() && !param.IsOmitted(i.req.ParallelToolCalls) { i.req.ParallelToolCalls = openai.Bool(false) } diff --git a/internal/integrationtest/bridge_test.go b/internal/integrationtest/bridge_test.go index 12e0bf19..7088af13 100644 --- a/internal/integrationtest/bridge_test.go +++ b/internal/integrationtest/bridge_test.go @@ -21,6 +21,7 @@ import ( "github.com/coder/aibridge/mcp" "github.com/coder/aibridge/provider" "github.com/coder/aibridge/recorder" + "github.com/coder/aibridge/utils" "github.com/google/uuid" "github.com/openai/openai-go/v3" oaissestream "github.com/openai/openai-go/v3/packages/ssestream" @@ -1315,73 +1316,112 @@ func TestChatCompletionsParallelToolCallsDisabled(t *testing.T) { t.Parallel() cases := []struct { - name string - streaming bool - withInjectedTools bool - expectParallelToolCalls bool + name string + fixture []byte + withInjectedTools bool + initialSetting *bool + expectedSetting *bool }{ - // Streaming with injected tools: parallel_tool_calls should be forced false. + // With injected tools and builtin tools: parallel_tool_calls should be forced false. + { + name: "with_injected_tools", + fixture: fixtures.OaiChatSingleBuiltinTool, + withInjectedTools: true, + initialSetting: utils.PtrTo(true), + expectedSetting: utils.PtrTo(false), + }, + // With builtin tools and without injected tools: parallel_tool_calls should be preserved. { - name: "streaming/with_injected_tools", - streaming: true, - withInjectedTools: true, - expectParallelToolCalls: false, + name: "no_injected_tools", + fixture: fixtures.OaiChatSingleBuiltinTool, + withInjectedTools: false, + initialSetting: utils.PtrTo(true), + expectedSetting: utils.PtrTo(true), }, - // Streaming without injected tools: parallel_tool_calls preserved. + // With injected tools and without builtin tools: parallel_tool_calls should be forced false. { - name: "streaming/no_injected_tools", - streaming: true, - withInjectedTools: false, - expectParallelToolCalls: true, + name: "injected_tools_and_no_builtin_tools", + fixture: fixtures.OaiChatSimple, + withInjectedTools: true, + initialSetting: utils.PtrTo(true), + expectedSetting: utils.PtrTo(false), + }, + // Request without parallel_tool_calls set should not have it modified, regardless of injected/builtin tools. + { + name: "parallel_tool_calls_unset_with_no_tools", + fixture: fixtures.OaiChatSimple, + withInjectedTools: false, + initialSetting: nil, + expectedSetting: nil, + }, + { + name: "parallel_tool_calls_unset_with_injected_tools", + fixture: fixtures.OaiChatSimple, + withInjectedTools: true, + initialSetting: nil, + expectedSetting: nil, }, - // Blocking with injected tools: parallel_tool_calls should be forced false. { - name: "blocking/with_injected_tools", - streaming: false, - withInjectedTools: true, - expectParallelToolCalls: false, + name: "parallel_tool_calls_unset_with_builtin_tools", + fixture: fixtures.OaiChatSingleBuiltinTool, + withInjectedTools: false, + initialSetting: nil, + expectedSetting: nil, }, { - name: "blocking/no_injected_tools", - streaming: false, - withInjectedTools: false, - expectParallelToolCalls: true, + name: "parallel_tool_calls_unset_with_builtin_and_injected_tools", + fixture: fixtures.OaiChatSingleBuiltinTool, + withInjectedTools: true, + initialSetting: nil, + expectedSetting: nil, }, } for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() + for _, streaming := range []bool{true, false} { + t.Run(fmt.Sprintf("%s/streaming=%v", tc.name, streaming), func(t *testing.T) { + t.Parallel() - ctx, cancel := context.WithTimeout(t.Context(), time.Second*30) - t.Cleanup(cancel) + ctx, cancel := context.WithTimeout(t.Context(), time.Second*30) + t.Cleanup(cancel) - fix := fixtures.Parse(t, fixtures.OaiChatSingleBuiltinTool) - upstream := newMockUpstream(t, ctx, newFixtureResponse(fix)) + fix := fixtures.Parse(t, tc.fixture) + upstream := newMockUpstream(t, ctx, newFixtureResponse(fix)) - var opts []bridgeOption - if tc.withInjectedTools { - opts = append(opts, withMCP(setupMCPForTest(t, defaultTracer))) - } - bridgeServer := newBridgeTestServer(t, ctx, upstream.URL, opts...) + var opts []bridgeOption + if tc.withInjectedTools { + opts = append(opts, withMCP(setupMCPForTest(t, defaultTracer))) + } + bridgeServer := newBridgeTestServer(t, ctx, upstream.URL, opts...) - reqBody, err := sjson.SetBytes(fix.Request(), "stream", tc.streaming) - require.NoError(t, err) + var ( + reqBody = fix.Request() + err error + ) + if tc.initialSetting != nil { + reqBody, err = sjson.SetBytes(reqBody, "parallel_tool_calls", *tc.initialSetting) + require.NoError(t, err) + } + reqBody, err = sjson.SetBytes(reqBody, "stream", streaming) + require.NoError(t, err) - resp := bridgeServer.makeRequest(t, http.MethodPost, pathOpenAIChatCompletions, reqBody) - _, err = io.ReadAll(resp.Body) - require.NoError(t, err) + resp := bridgeServer.makeRequest(t, http.MethodPost, pathOpenAIChatCompletions, reqBody) + _, err = io.ReadAll(resp.Body) + require.NoError(t, err) - received := upstream.receivedRequests() - require.Len(t, received, 1) + received := upstream.receivedRequests() + require.Len(t, received, 1) - var upstreamReq map[string]any - require.NoError(t, json.Unmarshal(received[0].Body, &upstreamReq)) + var upstreamReq map[string]any + require.NoError(t, json.Unmarshal(received[0].Body, &upstreamReq)) - ptc, ok := upstreamReq["parallel_tool_calls"].(bool) - require.True(t, ok, "parallel_tool_calls should be present in upstream request") - assert.Equal(t, tc.expectParallelToolCalls, ptc) - }) + ptc, ok := upstreamReq["parallel_tool_calls"].(bool) + require.Equal(t, tc.initialSetting != nil, ok) + if tc.expectedSetting != nil { + assert.Equal(t, *tc.expectedSetting, ptc) + } + }) + } } } From 90f506768a79e9f334357c1a12e7821135259081 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 11 Mar 2026 17:48:28 +0200 Subject: [PATCH 3/6] fix(messages): only modify parallel_tool_calls if set, and only if injected tools provided Signed-off-by: Danny Kopping --- fixtures/anthropic/single_builtin_tool.txtar | 16 +++ intercept/messages/base.go | 13 +- internal/integrationtest/bridge_test.go | 135 ++++++++++++++++--- 3 files changed, 139 insertions(+), 25 deletions(-) diff --git a/fixtures/anthropic/single_builtin_tool.txtar b/fixtures/anthropic/single_builtin_tool.txtar index 5df793b1..50ca93f1 100644 --- a/fixtures/anthropic/single_builtin_tool.txtar +++ b/fixtures/anthropic/single_builtin_tool.txtar @@ -4,6 +4,22 @@ Claude Code has builtin tools to (e.g.) explore the filesystem. { "model": "claude-sonnet-4-20250514", "max_tokens": 1024, + "tools": [ + { + "name": "Read", + "description": "Read the contents of a file at the given path.", + "input_schema": { + "type": "object", + "properties": { + "file_path": { + "type": "string", + "description": "The absolute path to the file to read" + } + }, + "required": ["file_path"] + } + } + ], "messages": [ { "role": "user", diff --git a/intercept/messages/base.go b/intercept/messages/base.go index 547f7ae8..31d10dd2 100644 --- a/intercept/messages/base.go +++ b/intercept/messages/base.go @@ -101,19 +101,15 @@ func (s *interceptionBase) baseTraceAttributes(r *http.Request, streaming bool) } func (i *interceptionBase) injectTools() { - if i.req == nil || i.mcpProxy == nil { + if i.req == nil || i.mcpProxy == nil || !i.HasInjectableTools() { return } - tools := i.mcpProxy.ListTools() - if len(tools) == 0 { - // No injected tools: no need to influence parallel tool calling. - return - } + i.disableParallelToolCalls() // Inject tools. var injectedTools []anthropic.ToolUnionParam - for _, tool := range tools { + for _, tool := range i.mcpProxy.ListTools() { injectedTools = append(injectedTools, anthropic.ToolUnionParam{ OfTool: &anthropic.ToolParam{ InputSchema: anthropic.ToolInputSchemaParam{ @@ -137,7 +133,9 @@ func (i *interceptionBase) injectTools() { if err != nil { i.logger.Warn(context.Background(), "failed to set inject tools in request payload", slog.Error(err)) } +} +func (i *interceptionBase) disableParallelToolCalls() { // Note: Parallel tool calls are disabled to avoid tool_use/tool_result block mismatches. // https://github.com/coder/aibridge/issues/2 toolChoiceType := i.req.ToolChoice.GetType() @@ -163,6 +161,7 @@ func (i *interceptionBase) injectTools() { case string(constant.ValueOf[constant.None]()): // No-op; if tool_choice=none then tools are not used at all. } + var err error i.payload, err = sjson.SetBytes(i.payload, "tool_choice", i.req.ToolChoice) if err != nil { i.logger.Warn(context.Background(), "failed to set tool_choice in request payload", slog.Error(err)) diff --git a/internal/integrationtest/bridge_test.go b/internal/integrationtest/bridge_test.go index 7088af13..8afc77ef 100644 --- a/internal/integrationtest/bridge_test.go +++ b/internal/integrationtest/bridge_test.go @@ -1192,62 +1192,162 @@ func TestAnthropicToolChoiceParallelDisabled(t *testing.T) { cases := []struct { name string + fixture []byte toolChoice any // nil, or map with "type" key. withInjectedTools bool - expectDisableParallel bool + expectDisableParallel *bool // nil = field should not be present, non-nil = expected value. expectToolChoiceTypeInRequest string }{ - // With injected tools - disable_parallel_tool_use should be set. + // With injected tools - disable_parallel_tool_use should be set to true. { name: "with injected tools: no tool_choice defined defaults to auto", + fixture: fixtures.AntSimple, toolChoice: nil, withInjectedTools: true, - expectDisableParallel: true, + expectDisableParallel: utils.PtrTo(true), expectToolChoiceTypeInRequest: toolChoiceAuto, }, { name: "with injected tools: tool_choice auto", + fixture: fixtures.AntSimple, toolChoice: map[string]any{"type": toolChoiceAuto}, withInjectedTools: true, - expectDisableParallel: true, + expectDisableParallel: utils.PtrTo(true), expectToolChoiceTypeInRequest: toolChoiceAuto, }, { name: "with injected tools: tool_choice any", + fixture: fixtures.AntSimple, toolChoice: map[string]any{"type": toolChoiceAny}, withInjectedTools: true, - expectDisableParallel: true, + expectDisableParallel: utils.PtrTo(true), expectToolChoiceTypeInRequest: toolChoiceAny, }, { name: "with injected tools: tool_choice tool", + fixture: fixtures.AntSimple, toolChoice: map[string]any{"type": toolChoiceTool, "name": "some_tool"}, withInjectedTools: true, - expectDisableParallel: true, + expectDisableParallel: utils.PtrTo(true), expectToolChoiceTypeInRequest: toolChoiceTool, }, { name: "with injected tools: tool_choice none", + fixture: fixtures.AntSimple, toolChoice: map[string]any{"type": toolChoiceNone}, withInjectedTools: true, - expectDisableParallel: false, + expectDisableParallel: nil, expectToolChoiceTypeInRequest: toolChoiceNone, }, - // Without injected tools - disable_parallel_tool_use should NOT be set. + // With injected tools and builtin tools - disable_parallel_tool_use should be set to true. { - name: "without injected tools: tool_choice auto", + name: "with injected and builtin tools: no tool_choice defined defaults to auto", + fixture: fixtures.AntSingleBuiltinTool, + toolChoice: nil, + withInjectedTools: true, + expectDisableParallel: utils.PtrTo(true), + expectToolChoiceTypeInRequest: toolChoiceAuto, + }, + { + name: "with injected and builtin tools: tool_choice auto", + fixture: fixtures.AntSingleBuiltinTool, + toolChoice: map[string]any{"type": toolChoiceAuto}, + withInjectedTools: true, + expectDisableParallel: utils.PtrTo(true), + expectToolChoiceTypeInRequest: toolChoiceAuto, + }, + { + name: "with injected and builtin tools: tool_choice any", + fixture: fixtures.AntSingleBuiltinTool, + toolChoice: map[string]any{"type": toolChoiceAny}, + withInjectedTools: true, + expectDisableParallel: utils.PtrTo(true), + expectToolChoiceTypeInRequest: toolChoiceAny, + }, + { + name: "with injected and builtin tools: tool_choice tool", + fixture: fixtures.AntSingleBuiltinTool, + toolChoice: map[string]any{"type": toolChoiceTool, "name": "some_tool"}, + withInjectedTools: true, + expectDisableParallel: utils.PtrTo(true), + expectToolChoiceTypeInRequest: toolChoiceTool, + }, + { + name: "with injected and builtin tools: tool_choice none", + fixture: fixtures.AntSingleBuiltinTool, + toolChoice: map[string]any{"type": toolChoiceNone}, + withInjectedTools: true, + expectDisableParallel: nil, + expectToolChoiceTypeInRequest: toolChoiceNone, + }, + // Without injected or builtin tools - disable_parallel_tool_use should NOT be set. + { + name: "without injected tools or builtin tools: tool_choice auto", + fixture: fixtures.AntSimple, toolChoice: map[string]any{"type": toolChoiceAuto}, withInjectedTools: false, - expectDisableParallel: false, + expectDisableParallel: nil, expectToolChoiceTypeInRequest: toolChoiceAuto, }, { - name: "without injected tools: tool_choice any", + name: "without injected tools or builtin tools: tool_choice any", + fixture: fixtures.AntSimple, toolChoice: map[string]any{"type": toolChoiceAny}, withInjectedTools: false, - expectDisableParallel: false, + expectDisableParallel: nil, expectToolChoiceTypeInRequest: toolChoiceAny, }, + // With builtin tools but without injected tools - disable_parallel_tool_use should NOT be set. + { + name: "with builtin tools only: tool_choice auto", + fixture: fixtures.AntSingleBuiltinTool, + toolChoice: map[string]any{"type": toolChoiceAuto}, + withInjectedTools: false, + expectDisableParallel: nil, + expectToolChoiceTypeInRequest: toolChoiceAuto, + }, + { + name: "with builtin tools only: tool_choice any", + fixture: fixtures.AntSingleBuiltinTool, + toolChoice: map[string]any{"type": toolChoiceAny}, + withInjectedTools: false, + expectDisableParallel: nil, + expectToolChoiceTypeInRequest: toolChoiceAny, + }, + // Request already has disable_parallel_tool_use set - with injected tools it should be set to true. + { + name: "with injected tools: request already disables parallel", + fixture: fixtures.AntSimple, + toolChoice: map[string]any{"type": toolChoiceAuto, "disable_parallel_tool_use": true}, + withInjectedTools: true, + expectDisableParallel: utils.PtrTo(true), + expectToolChoiceTypeInRequest: toolChoiceAuto, + }, + { + name: "with injected tools: request explicitly enables parallel", + fixture: fixtures.AntSimple, + toolChoice: map[string]any{"type": toolChoiceAuto, "disable_parallel_tool_use": false}, + withInjectedTools: true, + expectDisableParallel: utils.PtrTo(true), + expectToolChoiceTypeInRequest: toolChoiceAuto, + }, + // Request already has disable_parallel_tool_use set - without injected tools it should be preserved. + { + name: "without injected tools: request already disables parallel", + fixture: fixtures.AntSimple, + toolChoice: map[string]any{"type": toolChoiceAuto, "disable_parallel_tool_use": true}, + withInjectedTools: false, + expectDisableParallel: utils.PtrTo(true), + expectToolChoiceTypeInRequest: toolChoiceAuto, + }, + { + name: "without injected tools: request explicitly enables parallel", + fixture: fixtures.AntSimple, + toolChoice: map[string]any{"type": toolChoiceAuto, "disable_parallel_tool_use": false}, + withInjectedTools: false, + expectDisableParallel: utils.PtrTo(false), + expectToolChoiceTypeInRequest: toolChoiceAuto, + }, } for _, tc := range cases { @@ -1265,7 +1365,7 @@ func TestAnthropicToolChoiceParallelDisabled(t *testing.T) { mockMCP = newNoopMCPManager() } - fix := fixtures.Parse(t, fixtures.AntSimple) + fix := fixtures.Parse(t, tc.fixture) upstream := newMockUpstream(t, ctx, newFixtureResponse(fix)) bridgeServer := newBridgeTestServer(t, ctx, upstream.URL, @@ -1299,11 +1399,10 @@ func TestAnthropicToolChoiceParallelDisabled(t *testing.T) { // See https://platform.claude.com/docs/en/agents-and-tools/tool-use/implement-tool-use#parallel-tool-use disableParallel, hasDisableParallel := toolChoice["disable_parallel_tool_use"].(bool) - if tc.expectDisableParallel { - require.True(t, hasDisableParallel, "expected disable_parallel_tool_use in tool_choice") - assert.True(t, disableParallel, "expected disable_parallel_tool_use to be true") - } else { - assert.False(t, hasDisableParallel, "expected disable_parallel_tool_use to not be set") + require.Equal(t, tc.expectDisableParallel != nil, hasDisableParallel, + "disable_parallel_tool_use presence mismatch") + if tc.expectDisableParallel != nil { + assert.Equal(t, *tc.expectDisableParallel, disableParallel) } }) } From 116f02f584a2424c570d92df10a0155a46c99f7e Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Thu, 12 Mar 2026 11:35:30 +0200 Subject: [PATCH 4/6] chore: unify logic & tests Signed-off-by: Danny Kopping --- intercept/chatcompletions/base.go | 5 +- intercept/responses/injected_tools.go | 20 +- internal/integrationtest/bridge_test.go | 109 +++++++++-- internal/integrationtest/responses_test.go | 217 +++++++++++---------- 4 files changed, 215 insertions(+), 136 deletions(-) diff --git a/intercept/chatcompletions/base.go b/intercept/chatcompletions/base.go index 27b58f4f..7ebf8ff2 100644 --- a/intercept/chatcompletions/base.go +++ b/intercept/chatcompletions/base.go @@ -17,7 +17,6 @@ import ( "github.com/google/uuid" "github.com/openai/openai-go/v3" "github.com/openai/openai-go/v3/option" - "github.com/openai/openai-go/v3/packages/param" "github.com/openai/openai-go/v3/shared" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -109,9 +108,7 @@ func (i *interceptionBase) injectTools() { } // Disable parallel tool calls when injectable tools are present to simplify the inner agentic loop. - // Only set when tools are defined in the request, otherwise it causes a 400 Bad Request. - // The value is only changed if it is present. - if i.HasInjectableTools() && !param.IsOmitted(i.req.ParallelToolCalls) { + if i.HasInjectableTools() { i.req.ParallelToolCalls = openai.Bool(false) } diff --git a/intercept/responses/injected_tools.go b/intercept/responses/injected_tools.go index 885c7e1b..b6abc293 100644 --- a/intercept/responses/injected_tools.go +++ b/intercept/responses/injected_tools.go @@ -16,20 +16,14 @@ import ( ) func (i *responsesInterceptionBase) injectTools() { - if i.req == nil || i.mcpProxy == nil { + if i.req == nil || i.mcpProxy == nil || !i.HasInjectableTools() { return } - tools := i.mcpProxy.ListTools() - if len(tools) == 0 { - return - } - - // If there are injectable tools, disable parallel tool calls. i.disableParallelToolCalls() // Inject tools. - for _, tool := range tools { + for _, tool := range i.mcpProxy.ListTools() { var params map[string]any if tool.Params != nil { @@ -70,12 +64,10 @@ func (i *responsesInterceptionBase) injectTools() { // TODO: implement parallel tool calls. func (i *responsesInterceptionBase) disableParallelToolCalls() { // Disable parallel tool calls to simplify inner agentic loop; best-effort. - if len(i.req.Tools) > 0 { - var err error - i.reqPayload, err = sjson.SetBytes(i.reqPayload, "parallel_tool_calls", false) - if err != nil { - i.logger.Warn(context.Background(), "failed to disable parallel_tool_calls", slog.Error(err)) - } + var err error + i.reqPayload, err = sjson.SetBytes(i.reqPayload, "parallel_tool_calls", false) + if err != nil { + i.logger.Warn(context.Background(), "failed to disable parallel_tool_calls", slog.Error(err)) } } diff --git a/internal/integrationtest/bridge_test.go b/internal/integrationtest/bridge_test.go index 8afc77ef..b7cb7d9e 100644 --- a/internal/integrationtest/bridge_test.go +++ b/internal/integrationtest/bridge_test.go @@ -1314,6 +1314,39 @@ func TestAnthropicToolChoiceParallelDisabled(t *testing.T) { expectDisableParallel: nil, expectToolChoiceTypeInRequest: toolChoiceAny, }, + { + name: "with builtin tools only: request explicitly disables parallel", + fixture: fixtures.AntSingleBuiltinTool, + toolChoice: map[string]any{"type": toolChoiceAuto, "disable_parallel_tool_use": true}, + withInjectedTools: false, + expectDisableParallel: utils.PtrTo(true), + expectToolChoiceTypeInRequest: toolChoiceAuto, + }, + { + name: "with builtin tools only: request explicitly enables parallel", + fixture: fixtures.AntSingleBuiltinTool, + toolChoice: map[string]any{"type": toolChoiceAuto, "disable_parallel_tool_use": false}, + withInjectedTools: false, + expectDisableParallel: utils.PtrTo(false), + expectToolChoiceTypeInRequest: toolChoiceAuto, + }, + // Without injected or builtin tools - disable_parallel_tool_use should be preserved if set. + { + name: "no tools: request explicitly disables parallel", + fixture: fixtures.AntSimple, + toolChoice: map[string]any{"type": toolChoiceAuto, "disable_parallel_tool_use": true}, + withInjectedTools: false, + expectDisableParallel: utils.PtrTo(true), + expectToolChoiceTypeInRequest: toolChoiceAuto, + }, + { + name: "no tools: request explicitly enables parallel", + fixture: fixtures.AntSimple, + toolChoice: map[string]any{"type": toolChoiceAuto, "disable_parallel_tool_use": false}, + withInjectedTools: false, + expectDisableParallel: utils.PtrTo(false), + expectToolChoiceTypeInRequest: toolChoiceAuto, + }, // Request already has disable_parallel_tool_use set - with injected tools it should be set to true. { name: "with injected tools: request already disables parallel", @@ -1423,54 +1456,89 @@ func TestChatCompletionsParallelToolCallsDisabled(t *testing.T) { }{ // With injected tools and builtin tools: parallel_tool_calls should be forced false. { - name: "with_injected_tools", + name: "with injected and builtin tools: parallel_tool_calls true", fixture: fixtures.OaiChatSingleBuiltinTool, withInjectedTools: true, initialSetting: utils.PtrTo(true), expectedSetting: utils.PtrTo(false), }, - // With builtin tools and without injected tools: parallel_tool_calls should be preserved. { - name: "no_injected_tools", + name: "with injected and builtin tools: parallel_tool_calls false", fixture: fixtures.OaiChatSingleBuiltinTool, - withInjectedTools: false, - initialSetting: utils.PtrTo(true), - expectedSetting: utils.PtrTo(true), + withInjectedTools: true, + initialSetting: utils.PtrTo(false), + expectedSetting: utils.PtrTo(false), + }, + { + name: "with injected and builtin tools: parallel_tool_calls unset", + fixture: fixtures.OaiChatSingleBuiltinTool, + withInjectedTools: true, + initialSetting: nil, + expectedSetting: utils.PtrTo(false), }, - // With injected tools and without builtin tools: parallel_tool_calls should be forced false. + // With injected tools but without builtin tools: parallel_tool_calls should be forced false. { - name: "injected_tools_and_no_builtin_tools", + name: "with injected tools only: parallel_tool_calls true", fixture: fixtures.OaiChatSimple, withInjectedTools: true, initialSetting: utils.PtrTo(true), expectedSetting: utils.PtrTo(false), }, - // Request without parallel_tool_calls set should not have it modified, regardless of injected/builtin tools. { - name: "parallel_tool_calls_unset_with_no_tools", + name: "with injected tools only: parallel_tool_calls false", fixture: fixtures.OaiChatSimple, - withInjectedTools: false, - initialSetting: nil, - expectedSetting: nil, + withInjectedTools: true, + initialSetting: utils.PtrTo(false), + expectedSetting: utils.PtrTo(false), }, { - name: "parallel_tool_calls_unset_with_injected_tools", + name: "with injected tools only: parallel_tool_calls unset", fixture: fixtures.OaiChatSimple, withInjectedTools: true, initialSetting: nil, - expectedSetting: nil, + expectedSetting: utils.PtrTo(false), + }, + // With builtin tools but without injected tools: parallel_tool_calls should be preserved. + { + name: "with builtin tools only: parallel_tool_calls true", + fixture: fixtures.OaiChatSingleBuiltinTool, + withInjectedTools: false, + initialSetting: utils.PtrTo(true), + expectedSetting: utils.PtrTo(true), + }, + { + name: "with builtin tools only: parallel_tool_calls false", + fixture: fixtures.OaiChatSingleBuiltinTool, + withInjectedTools: false, + initialSetting: utils.PtrTo(false), + expectedSetting: utils.PtrTo(false), }, { - name: "parallel_tool_calls_unset_with_builtin_tools", + name: "with builtin tools only: parallel_tool_calls unset", fixture: fixtures.OaiChatSingleBuiltinTool, withInjectedTools: false, initialSetting: nil, expectedSetting: nil, }, + // Without any tools: nothing is modified. { - name: "parallel_tool_calls_unset_with_builtin_and_injected_tools", - fixture: fixtures.OaiChatSingleBuiltinTool, - withInjectedTools: true, + name: "no tools: parallel_tool_calls true", + fixture: fixtures.OaiChatSimple, + withInjectedTools: false, + initialSetting: utils.PtrTo(true), + expectedSetting: utils.PtrTo(true), + }, + { + name: "no tools: parallel_tool_calls false", + fixture: fixtures.OaiChatSimple, + withInjectedTools: false, + initialSetting: utils.PtrTo(false), + expectedSetting: utils.PtrTo(false), + }, + { + name: "no tools: parallel_tool_calls unset", + fixture: fixtures.OaiChatSimple, + withInjectedTools: false, initialSetting: nil, expectedSetting: nil, }, @@ -1515,7 +1583,8 @@ func TestChatCompletionsParallelToolCallsDisabled(t *testing.T) { require.NoError(t, json.Unmarshal(received[0].Body, &upstreamReq)) ptc, ok := upstreamReq["parallel_tool_calls"].(bool) - require.Equal(t, tc.initialSetting != nil, ok) + require.Equal(t, tc.expectedSetting != nil, ok, + "parallel_tool_calls presence mismatch") if tc.expectedSetting != nil { assert.Equal(t, *tc.expectedSetting, ptc) } diff --git a/internal/integrationtest/responses_test.go b/internal/integrationtest/responses_test.go index ab51d39f..eee1235f 100644 --- a/internal/integrationtest/responses_test.go +++ b/internal/integrationtest/responses_test.go @@ -3,6 +3,7 @@ package integrationtest import ( "context" "encoding/json" + "fmt" "io" "net" "net/http" @@ -18,8 +19,11 @@ import ( "github.com/coder/aibridge/fixtures" "github.com/coder/aibridge/provider" "github.com/coder/aibridge/recorder" + "github.com/coder/aibridge/utils" "github.com/openai/openai-go/v3/responses" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/tidwall/sjson" ) type keyVal struct { @@ -438,130 +442,147 @@ func TestResponsesBackgroundModeForbidden(t *testing.T) { func TestResponsesParallelToolsOverwritten(t *testing.T) { t.Parallel() - tests := []struct { - name string - request string - streaming bool - injectMCP bool - expectParallelToolCalls bool - expectParallelToolCallsValue bool + cases := []struct { + name string + fixture [2][]byte // [blocking, streaming] fixture pair. + withInjectedTools bool + initialSetting *bool + expectedSetting *bool // nil = field should not be present, non-nil = expected value. }{ + // With injected tools and builtin tools: parallel_tool_calls should be forced false. { - name: "blocking_with_tools", - request: `{"input": "hello", "model": "gpt-4o-mini", "stream": false, "parallel_tool_calls": true, "tools": [{"type": "function", "name": "test", "parameters": {}}]}`, - streaming: false, - injectMCP: true, - expectParallelToolCalls: true, - expectParallelToolCallsValue: false, + name: "with injected and builtin tools: parallel_tool_calls true", + fixture: [2][]byte{fixtures.OaiResponsesBlockingSingleBuiltinTool, fixtures.OaiResponsesStreamingBuiltinTool}, + withInjectedTools: true, + initialSetting: utils.PtrTo(true), + expectedSetting: utils.PtrTo(false), }, { - name: "streaming_with_tools", - request: `{"input": "hello", "model": "gpt-4o-mini", "stream": true, "parallel_tool_calls": true, "tools": [{"type": "function", "name": "test", "parameters": {}}]}`, - streaming: true, - injectMCP: true, - expectParallelToolCalls: true, - expectParallelToolCallsValue: false, + name: "with injected and builtin tools: parallel_tool_calls false", + fixture: [2][]byte{fixtures.OaiResponsesBlockingSingleBuiltinTool, fixtures.OaiResponsesStreamingBuiltinTool}, + withInjectedTools: true, + initialSetting: utils.PtrTo(false), + expectedSetting: utils.PtrTo(false), }, { - name: "blocking_with_tools_no_parallel_param", - request: `{"input": "hello", "model": "gpt-4o-mini", "stream": false, "tools": [{"type": "function", "name": "test", "parameters": {}}]}`, - streaming: false, - injectMCP: true, - expectParallelToolCalls: true, - expectParallelToolCallsValue: false, + name: "with injected and builtin tools: parallel_tool_calls unset", + fixture: [2][]byte{fixtures.OaiResponsesBlockingSingleBuiltinTool, fixtures.OaiResponsesStreamingBuiltinTool}, + withInjectedTools: true, + initialSetting: nil, + expectedSetting: utils.PtrTo(false), }, + // With injected tools but without builtin tools: parallel_tool_calls should be forced false. { - name: "streaming_with_tools_no_parallel_param", - request: `{"input": "hello", "model": "gpt-4o-mini", "stream": true, "tools": [{"type": "function", "name": "test", "parameters": {}}]}`, - streaming: true, - injectMCP: true, - expectParallelToolCalls: true, - expectParallelToolCallsValue: false, + name: "with injected tools only: parallel_tool_calls true", + fixture: [2][]byte{fixtures.OaiResponsesBlockingSimple, fixtures.OaiResponsesStreamingSimple}, + withInjectedTools: true, + initialSetting: utils.PtrTo(true), + expectedSetting: utils.PtrTo(false), }, { - name: "blocking_without_tools", - request: `{"input": "hello", "model": "gpt-4o-mini", "stream": false}`, - streaming: false, - injectMCP: true, + name: "with injected tools only: parallel_tool_calls false", + fixture: [2][]byte{fixtures.OaiResponsesBlockingSimple, fixtures.OaiResponsesStreamingSimple}, + withInjectedTools: true, + initialSetting: utils.PtrTo(false), + expectedSetting: utils.PtrTo(false), }, { - name: "streaming_without_tools", - request: `{"input": "hello", "model": "gpt-4o-mini", "stream": true}`, - streaming: true, - injectMCP: true, + name: "with injected tools only: parallel_tool_calls unset", + fixture: [2][]byte{fixtures.OaiResponsesBlockingSimple, fixtures.OaiResponsesStreamingSimple}, + withInjectedTools: true, + initialSetting: nil, + expectedSetting: utils.PtrTo(false), }, + // With builtin tools but without injected tools: parallel_tool_calls should be preserved. { - name: "blocking_without_tools_parallel_true", - request: `{"input": "hello", "model": "gpt-4o-mini", "stream": false, "parallel_tool_calls": true}`, - streaming: false, - injectMCP: true, - expectParallelToolCalls: true, - expectParallelToolCallsValue: true, + name: "with builtin tools only: parallel_tool_calls true", + fixture: [2][]byte{fixtures.OaiResponsesBlockingSingleBuiltinTool, fixtures.OaiResponsesStreamingBuiltinTool}, + withInjectedTools: false, + initialSetting: utils.PtrTo(true), + expectedSetting: utils.PtrTo(true), }, { - name: "streaming_without_tools_parallel_true", - request: `{"input": "hello", "model": "gpt-4o-mini", "stream": true, "parallel_tool_calls": true}`, - streaming: true, - injectMCP: true, - expectParallelToolCalls: true, - expectParallelToolCallsValue: true, + name: "with builtin tools only: parallel_tool_calls false", + fixture: [2][]byte{fixtures.OaiResponsesBlockingSingleBuiltinTool, fixtures.OaiResponsesStreamingBuiltinTool}, + withInjectedTools: false, + initialSetting: utils.PtrTo(false), + expectedSetting: utils.PtrTo(false), }, { - name: "blocking_without_injectable_tools_true", - request: `{"input": "hello", "model": "gpt-4o-mini", "stream": false, "parallel_tool_calls": true}`, - streaming: false, - injectMCP: false, - expectParallelToolCalls: true, - expectParallelToolCallsValue: true, + name: "with builtin tools only: parallel_tool_calls unset", + fixture: [2][]byte{fixtures.OaiResponsesBlockingSingleBuiltinTool, fixtures.OaiResponsesStreamingBuiltinTool}, + withInjectedTools: false, + initialSetting: nil, + expectedSetting: nil, }, + // Without any tools: nothing is modified. { - name: "streaming_without_injectable_tools_true", - request: `{"input": "hello", "model": "gpt-4o-mini", "stream": true, "parallel_tool_calls": true}`, - streaming: true, - injectMCP: false, - expectParallelToolCalls: true, - expectParallelToolCallsValue: true, + name: "no tools: parallel_tool_calls true", + fixture: [2][]byte{fixtures.OaiResponsesBlockingSimple, fixtures.OaiResponsesStreamingSimple}, + withInjectedTools: false, + initialSetting: utils.PtrTo(true), + expectedSetting: utils.PtrTo(true), + }, + { + name: "no tools: parallel_tool_calls false", + fixture: [2][]byte{fixtures.OaiResponsesBlockingSimple, fixtures.OaiResponsesStreamingSimple}, + withInjectedTools: false, + initialSetting: utils.PtrTo(false), + expectedSetting: utils.PtrTo(false), + }, + { + name: "no tools: parallel_tool_calls unset", + fixture: [2][]byte{fixtures.OaiResponsesBlockingSimple, fixtures.OaiResponsesStreamingSimple}, + withInjectedTools: false, + initialSetting: nil, + expectedSetting: nil, }, } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(t.Context(), time.Second*30) - t.Cleanup(cancel) - - fix := fixtures.OaiResponsesBlockingSimple - if tc.streaming { - fix = fixtures.OaiResponsesStreamingSimple - } - upstream := newMockUpstream(t, ctx, newFixtureResponse(fixtures.Parse(t, fix))) - - var opts []bridgeOption - if tc.injectMCP { - mockMCP := setupMCPForTest(t, defaultTracer) - opts = append(opts, withMCP(mockMCP)) - } - bridgeServer := newBridgeTestServer(t, ctx, upstream.URL, opts...) + for _, tc := range cases { + for i, streaming := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/streaming=%v", tc.name, streaming), func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(t.Context(), time.Second*30) + t.Cleanup(cancel) + + fix := fixtures.Parse(t, tc.fixture[i]) + upstream := newMockUpstream(t, ctx, newFixtureResponse(fix)) + + var opts []bridgeOption + if tc.withInjectedTools { + opts = append(opts, withMCP(setupMCPForTest(t, defaultTracer))) + } + bridgeServer := newBridgeTestServer(t, ctx, upstream.URL, opts...) + + var ( + reqBody = fix.Request() + err error + ) + if tc.initialSetting != nil { + reqBody, err = sjson.SetBytes(reqBody, "parallel_tool_calls", *tc.initialSetting) + require.NoError(t, err) + } + + resp := bridgeServer.makeRequest(t, http.MethodPost, pathOpenAIResponses, reqBody) + _, err = io.ReadAll(resp.Body) + require.NoError(t, err) - resp := bridgeServer.makeRequest(t, http.MethodPost, pathOpenAIResponses, []byte(tc.request)) - _, err := io.ReadAll(resp.Body) - require.NoError(t, err) + received := upstream.receivedRequests() + require.Len(t, received, 1) - received := upstream.receivedRequests() - require.Len(t, received, 1) + var upstreamReq map[string]any + require.NoError(t, json.Unmarshal(received[0].Body, &upstreamReq)) - var receivedRequest map[string]any - require.NoError(t, json.Unmarshal(received[0].Body, &receivedRequest)) - if tc.expectParallelToolCalls { - parallelToolCalls, ok := receivedRequest["parallel_tool_calls"].(bool) - require.True(t, ok, "parallel_tool_calls should be present in upstream request") - require.Equal(t, tc.expectParallelToolCallsValue, parallelToolCalls) - } else { - _, ok := receivedRequest["parallel_tool_calls"] - require.False(t, ok, "parallel_tool_calls should not be present when not set") - } - }) + ptc, ok := upstreamReq["parallel_tool_calls"].(bool) + require.Equal(t, tc.expectedSetting != nil, ok, + "parallel_tool_calls presence mismatch") + if tc.expectedSetting != nil { + assert.Equal(t, *tc.expectedSetting, ptc) + } + }) + } } } From d3f67bafe20dd2ed472cb710341a29782b7a3ccb Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Thu, 12 Mar 2026 11:42:05 +0200 Subject: [PATCH 5/6] chore: unifying logic, removing HasInjectableTools from interface Signed-off-by: Danny Kopping --- intercept/chatcompletions/base.go | 8 +++----- intercept/interceptor.go | 3 --- intercept/messages/base.go | 4 ++-- intercept/responses/base.go | 2 +- intercept/responses/injected_tools.go | 2 +- 5 files changed, 7 insertions(+), 12 deletions(-) diff --git a/intercept/chatcompletions/base.go b/intercept/chatcompletions/base.go index 7ebf8ff2..b85bc7b4 100644 --- a/intercept/chatcompletions/base.go +++ b/intercept/chatcompletions/base.go @@ -103,14 +103,12 @@ func (i *interceptionBase) newErrorResponse(err error) map[string]any { } func (i *interceptionBase) injectTools() { - if i.req == nil || i.mcpProxy == nil { + if i.req == nil || i.mcpProxy == nil || !i.hasInjectableTools() { return } // Disable parallel tool calls when injectable tools are present to simplify the inner agentic loop. - if i.HasInjectableTools() { - i.req.ParallelToolCalls = openai.Bool(false) - } + i.req.ParallelToolCalls = openai.Bool(false) // Inject tools. for _, tool := range i.mcpProxy.ListTools() { @@ -176,7 +174,7 @@ func (i *interceptionBase) writeUpstreamError(w http.ResponseWriter, oaiErr *err } } -func (i *interceptionBase) HasInjectableTools() bool { +func (i *interceptionBase) hasInjectableTools() bool { return i.mcpProxy != nil && len(i.mcpProxy.ListTools()) > 0 } diff --git a/intercept/interceptor.go b/intercept/interceptor.go index fee427cf..cbd29d62 100644 --- a/intercept/interceptor.go +++ b/intercept/interceptor.go @@ -34,7 +34,4 @@ type Interceptor interface { // by the model, so any single tool call ID is sufficient to identify the // parent interception. CorrelatingToolCallID() *string - // HasInjectableTools returns true if an [mcp.ServerProxier] has been provided - // and contains tools which must be injected into requests. - HasInjectableTools() bool } diff --git a/intercept/messages/base.go b/intercept/messages/base.go index 31d10dd2..37522380 100644 --- a/intercept/messages/base.go +++ b/intercept/messages/base.go @@ -101,7 +101,7 @@ func (s *interceptionBase) baseTraceAttributes(r *http.Request, streaming bool) } func (i *interceptionBase) injectTools() { - if i.req == nil || i.mcpProxy == nil || !i.HasInjectableTools() { + if i.req == nil || i.mcpProxy == nil || !i.hasInjectableTools() { return } @@ -314,7 +314,7 @@ func (i *interceptionBase) writeUpstreamError(w http.ResponseWriter, antErr *Err } } -func (i *interceptionBase) HasInjectableTools() bool { +func (i *interceptionBase) hasInjectableTools() bool { return i.mcpProxy != nil && len(i.mcpProxy.ListTools()) > 0 } diff --git a/intercept/responses/base.go b/intercept/responses/base.go index 9aafbd0e..69db3878 100644 --- a/intercept/responses/base.go +++ b/intercept/responses/base.go @@ -326,7 +326,7 @@ func (i *responsesInterceptionBase) recordTokenUsage(ctx context.Context, respon } } -func (i *responsesInterceptionBase) HasInjectableTools() bool { +func (i *responsesInterceptionBase) hasInjectableTools() bool { return i.mcpProxy != nil && len(i.mcpProxy.ListTools()) > 0 } diff --git a/intercept/responses/injected_tools.go b/intercept/responses/injected_tools.go index b6abc293..c3934fa3 100644 --- a/intercept/responses/injected_tools.go +++ b/intercept/responses/injected_tools.go @@ -16,7 +16,7 @@ import ( ) func (i *responsesInterceptionBase) injectTools() { - if i.req == nil || i.mcpProxy == nil || !i.HasInjectableTools() { + if i.req == nil || i.mcpProxy == nil || !i.hasInjectableTools() { return } From 3466ab67c57fa64a0d048c51c40b90391e2120b9 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Thu, 12 Mar 2026 15:43:22 +0200 Subject: [PATCH 6/6] chore: add missing test cases Signed-off-by: Danny Kopping --- internal/integrationtest/bridge_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/integrationtest/bridge_test.go b/internal/integrationtest/bridge_test.go index b7cb7d9e..01eb5815 100644 --- a/internal/integrationtest/bridge_test.go +++ b/internal/integrationtest/bridge_test.go @@ -1280,6 +1280,22 @@ func TestAnthropicToolChoiceParallelDisabled(t *testing.T) { expectDisableParallel: nil, expectToolChoiceTypeInRequest: toolChoiceNone, }, + { + name: "with injected and builtin tools: request already disables parallel", + fixture: fixtures.AntSingleBuiltinTool, + toolChoice: map[string]any{"type": toolChoiceAuto, "disable_parallel_tool_use": true}, + withInjectedTools: true, + expectDisableParallel: utils.PtrTo(true), + expectToolChoiceTypeInRequest: toolChoiceAuto, + }, + { + name: "with injected and builtin tools: request explicitly enables parallel", + fixture: fixtures.AntSingleBuiltinTool, + toolChoice: map[string]any{"type": toolChoiceAuto, "disable_parallel_tool_use": false}, + withInjectedTools: true, + expectDisableParallel: utils.PtrTo(true), + expectToolChoiceTypeInRequest: toolChoiceAuto, + }, // Without injected or builtin tools - disable_parallel_tool_use should NOT be set. { name: "without injected tools or builtin tools: tool_choice auto",