Skip to content

Commit a0e150e

Browse files
committed
fix: only disable parallel tool calls when injectable tools are present
Signed-off-by: Danny Kopping <danny@coder.com>
1 parent 250e790 commit a0e150e

10 files changed

Lines changed: 138 additions & 4 deletions

File tree

intercept/chatcompletions/base.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,10 @@ func (i *interceptionBase) writeUpstreamError(w http.ResponseWriter, oaiErr *err
171171
}
172172
}
173173

174+
func (i *interceptionBase) HasInjectableTools() bool {
175+
return i.mcpProxy != nil && len(i.mcpProxy.ListTools()) > 0
176+
}
177+
174178
func sumUsage(ref, in openai.CompletionUsage) openai.CompletionUsage {
175179
return openai.CompletionUsage{
176180
CompletionTokens: ref.CompletionTokens + in.CompletionTokens,

intercept/chatcompletions/streaming.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func (i *StreamingInterception) ProcessRequest(w http.ResponseWriter, r *http.Re
9999

100100
// TODO: implement parallel tool calls.
101101
// TODO: don't send if not supported by model (i.e. o4-mini).
102-
if len(i.req.Tools) > 0 { // If no tools are specified but this setting is set, it'll cause a 400 Bad Request.
102+
if i.HasInjectableTools() && len(i.req.Tools) > 0 { // If no tools are specified but this setting is set, it'll cause a 400 Bad Request.
103103
i.req.ParallelToolCalls = openai.Bool(false)
104104
}
105105

intercept/interceptor.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,7 @@ type Interceptor interface {
3434
// by the model, so any single tool call ID is sufficient to identify the
3535
// parent interception.
3636
CorrelatingToolCallID() *string
37+
// HasInjectableTools returns true if an [mcp.ServerProxier] has been provided
38+
// and contains tools which must be injected into requests.
39+
HasInjectableTools() bool
3740
}

intercept/messages/base.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,10 @@ func (i *interceptionBase) writeUpstreamError(w http.ResponseWriter, antErr *Err
315315
}
316316
}
317317

318+
func (i *interceptionBase) HasInjectableTools() bool {
319+
return i.mcpProxy != nil && len(i.mcpProxy.ListTools()) > 0
320+
}
321+
318322
// accumulateUsage accumulates usage statistics from source into dest.
319323
// It handles both [anthropic.Usage] and [anthropic.MessageDeltaUsage] types through [any].
320324
// The function uses reflection to handle the differences between the types:

intercept/responses/base.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,10 @@ func (i *responsesInterceptionBase) recordTokenUsage(ctx context.Context, respon
326326
}
327327
}
328328

329+
func (i *responsesInterceptionBase) HasInjectableTools() bool {
330+
return i.mcpProxy != nil && len(i.mcpProxy.ListTools()) > 0
331+
}
332+
329333
// responseCopier helper struct to send original response to the client
330334
type responseCopier struct {
331335
buff deltaBuffer

intercept/responses/blocking.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ func (i *BlockingResponsesInterceptor) ProcessRequest(w http.ResponseWriter, r *
5959
}
6060

6161
i.injectTools()
62-
i.disableParallelToolCalls()
6362

6463
var (
6564
response *responses.Response

intercept/responses/injected_tools.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ func (i *responsesInterceptionBase) injectTools() {
2525
return
2626
}
2727

28+
// If there are injectable tools, disable parallel tool calls.
29+
i.disableParallelToolCalls()
30+
2831
// Inject tools.
2932
for _, tool := range tools {
3033
var params map[string]any

intercept/responses/streaming.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ func (i *StreamingResponsesInterceptor) ProcessRequest(w http.ResponseWriter, r
7070
}
7171

7272
i.injectTools()
73-
i.disableParallelToolCalls()
7473

7574
events := eventstream.NewEventStream(ctx, i.logger.Named("sse-sender"), nil)
7675
go events.Start(w, r)

internal/integrationtest/bridge_test.go

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1308,6 +1308,93 @@ func TestAnthropicToolChoiceParallelDisabled(t *testing.T) {
13081308
}
13091309
}
13101310

1311+
// TestChatCompletionsParallelToolCallsDisabled verifies that parallel_tool_calls
1312+
// is set to false only when injectable MCP tools are present and the request
1313+
// includes tools.
1314+
//
1315+
// NOTE: the blocking chat completions interceptor does not yet disable
1316+
// parallel_tool_calls (unlike the streaming path). The blocking sub-tests
1317+
// document this gap. See intercept/chatcompletions/blocking.go.
1318+
func TestChatCompletionsParallelToolCallsDisabled(t *testing.T) {
1319+
t.Parallel()
1320+
1321+
cases := []struct {
1322+
name string
1323+
streaming bool
1324+
withInjectedTools bool
1325+
expectParallelToolCalls bool
1326+
}{
1327+
// Streaming with injected tools: parallel_tool_calls should be forced false.
1328+
{
1329+
name: "streaming/with_injected_tools",
1330+
streaming: true,
1331+
withInjectedTools: true,
1332+
expectParallelToolCalls: false,
1333+
},
1334+
// Streaming without injected tools: parallel_tool_calls preserved.
1335+
{
1336+
name: "streaming/no_injected_tools",
1337+
streaming: true,
1338+
withInjectedTools: false,
1339+
expectParallelToolCalls: true,
1340+
},
1341+
// Blocking: parallel_tool_calls is NOT yet disabled even with
1342+
// injected tools. These tests document the current behavior gap
1343+
// vs the streaming path.
1344+
{
1345+
name: "blocking/with_injected_tools",
1346+
streaming: false,
1347+
withInjectedTools: true,
1348+
expectParallelToolCalls: true, // TODO: should be false
1349+
},
1350+
{
1351+
name: "blocking/no_injected_tools",
1352+
streaming: false,
1353+
withInjectedTools: false,
1354+
expectParallelToolCalls: true,
1355+
},
1356+
}
1357+
1358+
for _, tc := range cases {
1359+
t.Run(tc.name, func(t *testing.T) {
1360+
t.Parallel()
1361+
1362+
ctx, cancel := context.WithTimeout(t.Context(), time.Second*30)
1363+
t.Cleanup(cancel)
1364+
1365+
fix := fixtures.Parse(t, fixtures.OaiChatSingleBuiltinTool)
1366+
upstream := newMockUpstream(t, ctx, newFixtureResponse(fix))
1367+
1368+
var opts []bridgeOption
1369+
if tc.withInjectedTools {
1370+
opts = append(opts, withMCP(setupMCPForTest(t, defaultTracer)))
1371+
}
1372+
bridgeServer := newBridgeTestServer(t, ctx, upstream.URL, opts...)
1373+
1374+
reqBody := fix.Request()
1375+
var err error
1376+
reqBody, err = sjson.SetBytes(reqBody, "stream", tc.streaming)
1377+
require.NoError(t, err)
1378+
reqBody, err = sjson.SetBytes(reqBody, "parallel_tool_calls", true)
1379+
require.NoError(t, err)
1380+
1381+
resp := bridgeServer.makeRequest(t, http.MethodPost, pathOpenAIChatCompletions, reqBody)
1382+
_, err = io.ReadAll(resp.Body)
1383+
require.NoError(t, err)
1384+
1385+
received := upstream.receivedRequests()
1386+
require.Len(t, received, 1)
1387+
1388+
var upstreamReq map[string]any
1389+
require.NoError(t, json.Unmarshal(received[0].Body, &upstreamReq))
1390+
1391+
ptc, ok := upstreamReq["parallel_tool_calls"].(bool)
1392+
require.True(t, ok, "parallel_tool_calls should be present in upstream request")
1393+
assert.Equal(t, tc.expectParallelToolCalls, ptc)
1394+
})
1395+
}
1396+
}
1397+
13111398
func TestThinkingAdaptiveIsPreserved(t *testing.T) {
13121399
t.Parallel()
13131400

internal/integrationtest/responses_test.go

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -442,58 +442,83 @@ func TestResponsesParallelToolsOverwritten(t *testing.T) {
442442
name string
443443
request string
444444
streaming bool
445+
injectMCP bool
445446
expectParallelToolCalls bool
446447
expectParallelToolCallsValue bool
447448
}{
448449
{
449450
name: "blocking_with_tools",
450451
request: `{"input": "hello", "model": "gpt-4o-mini", "stream": false, "parallel_tool_calls": true, "tools": [{"type": "function", "name": "test", "parameters": {}}]}`,
451452
streaming: false,
453+
injectMCP: true,
452454
expectParallelToolCalls: true,
453455
expectParallelToolCallsValue: false,
454456
},
455457
{
456458
name: "streaming_with_tools",
457459
request: `{"input": "hello", "model": "gpt-4o-mini", "stream": true, "parallel_tool_calls": true, "tools": [{"type": "function", "name": "test", "parameters": {}}]}`,
458460
streaming: true,
461+
injectMCP: true,
459462
expectParallelToolCalls: true,
460463
expectParallelToolCallsValue: false,
461464
},
462465
{
463466
name: "blocking_with_tools_no_parallel_param",
464467
request: `{"input": "hello", "model": "gpt-4o-mini", "stream": false, "tools": [{"type": "function", "name": "test", "parameters": {}}]}`,
465468
streaming: false,
469+
injectMCP: true,
466470
expectParallelToolCalls: true,
467471
expectParallelToolCallsValue: false,
468472
},
469473
{
470474
name: "streaming_with_tools_no_parallel_param",
471475
request: `{"input": "hello", "model": "gpt-4o-mini", "stream": true, "tools": [{"type": "function", "name": "test", "parameters": {}}]}`,
472476
streaming: true,
477+
injectMCP: true,
473478
expectParallelToolCalls: true,
474479
expectParallelToolCallsValue: false,
475480
},
476481
{
477482
name: "blocking_without_tools",
478483
request: `{"input": "hello", "model": "gpt-4o-mini", "stream": false}`,
479484
streaming: false,
485+
injectMCP: true,
480486
},
481487
{
482488
name: "streaming_without_tools",
483489
request: `{"input": "hello", "model": "gpt-4o-mini", "stream": true}`,
484490
streaming: true,
491+
injectMCP: true,
485492
},
486493
{
487494
name: "blocking_without_tools_parallel_true",
488495
request: `{"input": "hello", "model": "gpt-4o-mini", "stream": false, "parallel_tool_calls": true}`,
489496
streaming: false,
497+
injectMCP: true,
490498
expectParallelToolCalls: true,
491499
expectParallelToolCallsValue: true,
492500
},
493501
{
494502
name: "streaming_without_tools_parallel_true",
495503
request: `{"input": "hello", "model": "gpt-4o-mini", "stream": true, "parallel_tool_calls": true}`,
496504
streaming: true,
505+
injectMCP: true,
506+
expectParallelToolCalls: true,
507+
expectParallelToolCallsValue: true,
508+
},
509+
{
510+
name: "blocking_without_injectable_tools_true",
511+
request: `{"input": "hello", "model": "gpt-4o-mini", "stream": false, "parallel_tool_calls": true}`,
512+
streaming: false,
513+
injectMCP: false,
514+
expectParallelToolCalls: true,
515+
expectParallelToolCallsValue: true,
516+
},
517+
{
518+
name: "streaming_without_injectable_tools_true",
519+
request: `{"input": "hello", "model": "gpt-4o-mini", "stream": true, "parallel_tool_calls": true}`,
520+
streaming: true,
521+
injectMCP: false,
497522
expectParallelToolCalls: true,
498523
expectParallelToolCallsValue: true,
499524
},
@@ -511,7 +536,13 @@ func TestResponsesParallelToolsOverwritten(t *testing.T) {
511536
fix = fixtures.OaiResponsesStreamingSimple
512537
}
513538
upstream := newMockUpstream(t, ctx, newFixtureResponse(fixtures.Parse(t, fix)))
514-
bridgeServer := newBridgeTestServer(t, ctx, upstream.URL)
539+
540+
var opts []bridgeOption
541+
if tc.injectMCP {
542+
mockMCP := setupMCPForTest(t, defaultTracer)
543+
opts = append(opts, withMCP(mockMCP))
544+
}
545+
bridgeServer := newBridgeTestServer(t, ctx, upstream.URL, opts...)
515546

516547
resp := bridgeServer.makeRequest(t, http.MethodPost, pathOpenAIResponses, []byte(tc.request))
517548
_, err := io.ReadAll(resp.Body)

0 commit comments

Comments
 (0)