Skip to content

Commit 4dccf7d

Browse files
committed
fix(responses): pass through client-side tool_calls instead of dispatching them
The /v1/responses warm path always engages multi-step orchestration and dispatches every tool_call returned by the model through HttpToolExecutor. When a client supplies their own function tools in the request body (client-side tool calling per the OpenAI Responses spec), the executor looks them up in the server-side tool_sources registry, doesn't find them, and fails the step with `Tool not found`. Fix: thread the set of registered tool names from `resolve_tools_for_request` through PendingResponseInput into the transition function. When a model_call returns tool_calls and any name is missing from that set, complete the response with the model's payload — assembly already emits `function_call` output items in OpenAI Responses shape, so the client receives the calls and can execute them locally. The whole tool_call batch passes through (not just the unregistered ones) because partial dispatch would leave the upstream conversation expecting tool results for calls the loop never ran.
1 parent 18e0c8e commit 4dccf7d

5 files changed

Lines changed: 143 additions & 18 deletions

File tree

dwctl/src/responses/middleware.rs

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -602,13 +602,6 @@ async fn warm_path_setup<P: PoolProvider + Clone + Send + Sync + 'static>(
602602
model: &str,
603603
) -> Option<(uuid::Uuid, Arc<crate::tool_executor::ResolvedToolSet>, onwards::UpstreamTarget)> {
604604
let created_by = response_store::lookup_created_by(&state.dwctl_pool, Some(api_key)).await;
605-
let pending = response_store::PendingResponseInput {
606-
body: request_value.to_string(),
607-
api_key: Some(api_key.to_string()),
608-
created_by,
609-
base_url: state.loopback_base_url.clone(),
610-
};
611-
let head_step_uuid = state.response_store.register_pending(pending);
612605

613606
let resolved = match crate::tool_injection::resolve_tools_for_request(&state.dwctl_pool, api_key, Some(model)).await {
614607
Ok(Some(set)) => Arc::new(set),
@@ -625,6 +618,25 @@ async fn warm_path_setup<P: PoolProvider + Clone + Send + Sync + 'static>(
625618
}
626619
};
627620

621+
// The transition function uses these names to decide which
622+
// tool_calls returned by the model can be auto-dispatched and
623+
// which must be passed through to the client as `function_call`
624+
// output items. Any tool the user supplies in their request body
625+
// that isn't registered in `tool_sources` ends up outside this set
626+
// and gets the client-side passthrough treatment — without this,
627+
// HttpToolExecutor would try to dispatch the unknown name and the
628+
// step would fail with `Tool not found`.
629+
let resolved_tool_names = resolved.tools.keys().cloned().collect();
630+
631+
let pending = response_store::PendingResponseInput {
632+
body: request_value.to_string(),
633+
api_key: Some(api_key.to_string()),
634+
created_by,
635+
base_url: state.loopback_base_url.clone(),
636+
resolved_tool_names,
637+
};
638+
let head_step_uuid = state.response_store.register_pending(pending);
639+
628640
let upstream = onwards::UpstreamTarget {
629641
url: format!("{}/v1/chat/completions", state.loopback_base_url),
630642
api_key: Some(api_key.to_string()),

dwctl/src/responses/store.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
//! All fusillade operations go through the `Storage` trait via `request_manager`.
55
//! The only raw SQL is the `api_keys` lookup which queries a dwctl-owned table.
66
7-
use std::collections::HashMap;
7+
use std::collections::{HashMap, HashSet};
88
use std::sync::{Arc, RwLock};
99

1010
use async_trait::async_trait;
@@ -45,6 +45,18 @@ pub struct PendingResponseInput {
4545
/// `base_url` column points at the dwctl loopback so onwards can
4646
/// pick a target / honor strict mode at fire time.
4747
pub base_url: String,
48+
/// Names of server-side tools registered for this request (resolved
49+
/// from `tool_sources` joined with the user's groups + the deployment).
50+
/// The transition function uses this to decide which tool_calls
51+
/// returned by the model can be auto-dispatched server-side and which
52+
/// must be passed through to the client as `function_call` output items.
53+
///
54+
/// When a tool_call's name is missing from this set, it's treated as a
55+
/// client-side tool: the loop completes with the model's response, and
56+
/// `assemble_response` surfaces the call as a `function_call` item per
57+
/// the OpenAI Responses contract — the client is expected to execute
58+
/// it and submit the result via a follow-up request.
59+
pub resolved_tool_names: HashSet<String>,
4860
}
4961

5062
/// Header set by the responses middleware so the outlet handler knows which
@@ -741,7 +753,7 @@ impl<P: PoolProvider + Clone + Send + Sync + 'static> MultiStepStore for Fusilla
741753
let pending = self.pending_input(request_id)?;
742754
let parsed = super::transition::parse_parent_request(&pending.body).map_err(StoreError::StorageError)?;
743755
let chain = <Self as MultiStepStore>::list_chain(self, request_id, scope_parent).await?;
744-
Ok(super::transition::decide_next_action(&parsed, &chain))
756+
Ok(super::transition::decide_next_action(&parsed, &chain, &pending.resolved_tool_names))
745757
}
746758

747759
async fn record_step(

dwctl/src/responses/transition.rs

Lines changed: 108 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@
5656
//! tests) stay decoupled from the strict-mode adapter — wiring those in
5757
//! is a future cleanup once the multi-step path is live.
5858
59+
use std::collections::HashSet;
60+
5961
use onwards::{ChainStep, NextAction, StepDescriptor, StepKind, StepState};
6062
use serde_json::{Value, json};
6163

@@ -239,10 +241,16 @@ pub(crate) fn prepare_followup_model_call(parsed: &ParsedRequest, chain: &[Chain
239241
/// Decide the next action given:
240242
/// - `parsed`: the parent fusillade request body
241243
/// - `chain`: completed/failed steps in the current scope, in sequence order
244+
/// - `resolved_tool_names`: names of server-side tools registered for this
245+
/// request. Tool_calls whose name is in this set get auto-dispatched as
246+
/// server-side `ToolCall` steps; tool_calls outside the set are
247+
/// passed through to the client as `function_call` output items by
248+
/// completing the response with the model's payload (the assembly step
249+
/// surfaces them per the OpenAI Responses contract).
242250
///
243251
/// Returns the action the loop should take. Pure function over its inputs;
244252
/// no I/O.
245-
pub(crate) fn decide_next_action(parsed: &ParsedRequest, chain: &[ChainStep]) -> NextAction {
253+
pub(crate) fn decide_next_action(parsed: &ParsedRequest, chain: &[ChainStep], resolved_tool_names: &HashSet<String>) -> NextAction {
246254
if chain.is_empty() {
247255
return NextAction::AppendSteps(vec![prepare_initial_model_call(parsed)]);
248256
}
@@ -275,9 +283,31 @@ pub(crate) fn decide_next_action(parsed: &ParsedRequest, chain: &[ChainStep]) ->
275283
let tool_calls = extract_tool_calls(&response);
276284
if tool_calls.is_empty() {
277285
// No tool calls — the model returned final output.
278-
NextAction::Complete(response)
279-
} else {
286+
return NextAction::Complete(response);
287+
}
288+
289+
// Server-side dispatch is only safe when every tool_call
290+
// names a server-registered tool (i.e., one with a row in
291+
// `tool_sources` for this request's user/deployment). If
292+
// any name is unregistered, it's a client-side function
293+
// tool — the model must have seen it because the user put
294+
// it in the request body — and we cannot dispatch it. The
295+
// OpenAI Responses contract for that case is to surface
296+
// every tool_call as a `function_call` output item and let
297+
// the client run them and submit results in a follow-up.
298+
//
299+
// We bail out for the *whole* fan-out (rather than partial
300+
// dispatch) because the model expects results for every
301+
// call it emitted before producing its next message; a
302+
// mixed dispatch would leave the conversation in a state
303+
// the upstream model can't reason about.
304+
let all_registered = tool_calls
305+
.iter()
306+
.all(|step| tool_call_name(&step.request_payload).is_some_and(|name| resolved_tool_names.contains(name)));
307+
if all_registered {
280308
NextAction::AppendSteps(tool_calls)
309+
} else {
310+
NextAction::Complete(response)
281311
}
282312
}
283313
StepKind::ToolCall => {
@@ -290,6 +320,10 @@ pub(crate) fn decide_next_action(parsed: &ParsedRequest, chain: &[ChainStep]) ->
290320
}
291321
}
292322

323+
fn tool_call_name(payload: &Value) -> Option<&str> {
324+
payload.get("name").and_then(|n| n.as_str())
325+
}
326+
293327
#[cfg(test)]
294328
mod tests {
295329
use super::*;
@@ -322,6 +356,10 @@ mod tests {
322356
assert_eq!(p.initial_messages.len(), 1);
323357
}
324358

359+
fn names(items: &[&str]) -> HashSet<String> {
360+
items.iter().map(|s| s.to_string()).collect()
361+
}
362+
325363
#[test]
326364
fn empty_chain_emits_initial_model_call() {
327365
let parsed = ParsedRequest {
@@ -330,7 +368,7 @@ mod tests {
330368
tools: None,
331369
stream: false,
332370
};
333-
match decide_next_action(&parsed, &[]) {
371+
match decide_next_action(&parsed, &[], &HashSet::new()) {
334372
NextAction::AppendSteps(steps) => {
335373
assert_eq!(steps.len(), 1);
336374
assert!(matches!(steps[0].kind, StepKind::ModelCall));
@@ -341,7 +379,7 @@ mod tests {
341379
}
342380

343381
#[test]
344-
fn model_call_with_tool_calls_emits_fan_out() {
382+
fn model_call_with_registered_tool_calls_emits_fan_out() {
345383
let parsed = ParsedRequest {
346384
model: "m".into(),
347385
initial_messages: vec![],
@@ -360,7 +398,7 @@ mod tests {
360398
}]
361399
});
362400
let chain = vec![step("s1", 1, StepKind::ModelCall, StepState::Completed, Some(response))];
363-
match decide_next_action(&parsed, &chain) {
401+
match decide_next_action(&parsed, &chain, &names(&["a", "b"])) {
364402
NextAction::AppendSteps(steps) => {
365403
assert_eq!(steps.len(), 2);
366404
assert_eq!(steps[0].request_payload["name"], "a");
@@ -372,6 +410,67 @@ mod tests {
372410
}
373411
}
374412

413+
#[test]
414+
fn model_call_with_unregistered_tool_completes_for_client_dispatch() {
415+
// The user supplied a client-side function tool in the request
416+
// body; the model emits a tool_call for it. With no row in
417+
// `tool_sources` for this name, the loop must NOT try to
418+
// dispatch it (HttpToolExecutor would fail with NotFound) —
419+
// instead it completes with the model's response so assembly
420+
// can surface a `function_call` output item to the client.
421+
let parsed = ParsedRequest {
422+
model: "m".into(),
423+
initial_messages: vec![],
424+
tools: None,
425+
stream: false,
426+
};
427+
let response = json!({
428+
"choices": [{
429+
"message": {
430+
"role": "assistant",
431+
"tool_calls": [
432+
{"id": "call_1", "type": "function", "function": {"name": "read_pages", "arguments": "{\"id\":1}"}},
433+
]
434+
}
435+
}]
436+
});
437+
let chain = vec![step("s1", 1, StepKind::ModelCall, StepState::Completed, Some(response.clone()))];
438+
match decide_next_action(&parsed, &chain, &HashSet::new()) {
439+
NextAction::Complete(v) => assert_eq!(v, response),
440+
other => panic!("expected Complete for unregistered tool, got {other:?}"),
441+
}
442+
}
443+
444+
#[test]
445+
fn model_call_with_mixed_registered_and_unregistered_completes() {
446+
// If even one tool_call in a fan-out is unregistered, the whole
447+
// batch passes through to the client. Partial dispatch would
448+
// leave the model expecting results for tool_calls the loop
449+
// never ran.
450+
let parsed = ParsedRequest {
451+
model: "m".into(),
452+
initial_messages: vec![],
453+
tools: None,
454+
stream: false,
455+
};
456+
let response = json!({
457+
"choices": [{
458+
"message": {
459+
"role": "assistant",
460+
"tool_calls": [
461+
{"id": "call_1", "type": "function", "function": {"name": "weather", "arguments": "{}"}},
462+
{"id": "call_2", "type": "function", "function": {"name": "client_only", "arguments": "{}"}},
463+
]
464+
}
465+
}]
466+
});
467+
let chain = vec![step("s1", 1, StepKind::ModelCall, StepState::Completed, Some(response.clone()))];
468+
match decide_next_action(&parsed, &chain, &names(&["weather"])) {
469+
NextAction::Complete(v) => assert_eq!(v, response),
470+
other => panic!("expected Complete for mixed tool_calls, got {other:?}"),
471+
}
472+
}
473+
375474
#[test]
376475
fn model_call_without_tool_calls_completes() {
377476
let parsed = ParsedRequest {
@@ -386,7 +485,7 @@ mod tests {
386485
}]
387486
});
388487
let chain = vec![step("s1", 1, StepKind::ModelCall, StepState::Completed, Some(response.clone()))];
389-
match decide_next_action(&parsed, &chain) {
488+
match decide_next_action(&parsed, &chain, &HashSet::new()) {
390489
NextAction::Complete(v) => assert_eq!(v, response),
391490
_ => panic!("expected Complete"),
392491
}
@@ -412,7 +511,7 @@ mod tests {
412511
step("s1", 1, StepKind::ModelCall, StepState::Completed, Some(model_response)),
413512
step("s2", 2, StepKind::ToolCall, StepState::Completed, Some(json!({"result": 1}))),
414513
];
415-
match decide_next_action(&parsed, &chain) {
514+
match decide_next_action(&parsed, &chain, &names(&["a"])) {
416515
NextAction::AppendSteps(steps) => {
417516
assert_eq!(steps.len(), 1);
418517
assert!(matches!(steps[0].kind, StepKind::ModelCall));
@@ -437,7 +536,7 @@ mod tests {
437536
};
438537
let mut s = step("s1", 1, StepKind::ModelCall, StepState::Failed, None);
439538
s.error = Some(json!({"type": "upstream_500"}));
440-
match decide_next_action(&parsed, &[s]) {
539+
match decide_next_action(&parsed, &[s], &HashSet::new()) {
441540
NextAction::Fail(v) => assert_eq!(v, json!({"type": "upstream_500"})),
442541
_ => panic!("expected Fail"),
443542
}

dwctl/src/test/multi_step_executor.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ where
6262
api_key: None,
6363
created_by: None,
6464
base_url: base_url.to_string(),
65+
resolved_tool_names: std::collections::HashSet::new(),
6566
})
6667
.to_string()
6768
}

dwctl/src/test/responses.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,7 @@ async fn test_multi_step_chain_assembles_and_is_retrievable_via_get(pool: PgPool
315315
api_key: None,
316316
created_by: Some("test-user".to_string()),
317317
base_url: "http://upstream-mock".to_string(),
318+
resolved_tool_names: std::collections::HashSet::new(),
318319
});
319320
let request_id = head_uuid.to_string();
320321

0 commit comments

Comments
 (0)