mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Remove the redundant tool dispatch wrapper (#33866)
## What changed - Remove `ToolRegistry::dispatch_any`, which always forwarded a missing terminal outcome. - Update registry and dispatch trace tests to call `dispatch_any_with_terminal_outcome` with `None` explicitly. GitOrigin-RevId: aa3acb2ed480fd452bc25301a351f46e759bbfab
This commit is contained in:
@@ -392,15 +392,6 @@ impl ToolRegistry {
|
||||
Some(tool.waits_for_runtime_cancellation())
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
pub(crate) async fn dispatch_any(
|
||||
&self,
|
||||
invocation: ToolInvocation,
|
||||
) -> Result<AnyToolResult, FunctionCallError> {
|
||||
self.dispatch_any_with_terminal_outcome(invocation, /*terminal_outcome_reached*/ None)
|
||||
.await
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::await_holding_invalid_type,
|
||||
reason = "tool dispatch must keep active-turn accounting atomic"
|
||||
|
||||
@@ -399,20 +399,26 @@ async fn dispatch_notifies_tool_lifecycle_contributors() -> anyhow::Result<()> {
|
||||
let turn = Arc::new(turn);
|
||||
|
||||
registry
|
||||
.dispatch_any(test_invocation(
|
||||
Arc::clone(&session),
|
||||
Arc::clone(&turn),
|
||||
"ok-call",
|
||||
ok_tool.clone(),
|
||||
))
|
||||
.dispatch_any_with_terminal_outcome(
|
||||
test_invocation(
|
||||
Arc::clone(&session),
|
||||
Arc::clone(&turn),
|
||||
"ok-call",
|
||||
ok_tool.clone(),
|
||||
),
|
||||
/*terminal_outcome_reached*/ None,
|
||||
)
|
||||
.await?;
|
||||
let err = match registry
|
||||
.dispatch_any(test_invocation(
|
||||
Arc::clone(&session),
|
||||
Arc::clone(&turn),
|
||||
"failing-call",
|
||||
failing_tool.clone(),
|
||||
))
|
||||
.dispatch_any_with_terminal_outcome(
|
||||
test_invocation(
|
||||
Arc::clone(&session),
|
||||
Arc::clone(&turn),
|
||||
"failing-call",
|
||||
failing_tool.clone(),
|
||||
),
|
||||
/*terminal_outcome_reached*/ None,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(_) => panic!("failing handler should return an error"),
|
||||
|
||||
@@ -78,27 +78,33 @@ async fn dispatch_lifecycle_trace_records_direct_and_code_mode_requesters() -> a
|
||||
let turn = Arc::new(turn);
|
||||
|
||||
registry
|
||||
.dispatch_any(test_invocation(
|
||||
Arc::clone(&session),
|
||||
Arc::clone(&turn),
|
||||
"direct-call",
|
||||
"test_tool",
|
||||
ToolCallSource::Direct,
|
||||
"{}",
|
||||
))
|
||||
.dispatch_any_with_terminal_outcome(
|
||||
test_invocation(
|
||||
Arc::clone(&session),
|
||||
Arc::clone(&turn),
|
||||
"direct-call",
|
||||
"test_tool",
|
||||
ToolCallSource::Direct,
|
||||
"{}",
|
||||
),
|
||||
/*terminal_outcome_reached*/ None,
|
||||
)
|
||||
.await?;
|
||||
registry
|
||||
.dispatch_any(test_invocation(
|
||||
session,
|
||||
turn,
|
||||
"code-mode-call",
|
||||
"test_tool",
|
||||
ToolCallSource::CodeMode {
|
||||
cell_id: "cell-1".to_string(),
|
||||
runtime_tool_call_id: "tool-1".to_string(),
|
||||
},
|
||||
"{}",
|
||||
))
|
||||
.dispatch_any_with_terminal_outcome(
|
||||
test_invocation(
|
||||
session,
|
||||
turn,
|
||||
"code-mode-call",
|
||||
"test_tool",
|
||||
ToolCallSource::CodeMode {
|
||||
cell_id: "cell-1".to_string(),
|
||||
runtime_tool_call_id: "tool-1".to_string(),
|
||||
},
|
||||
"{}",
|
||||
),
|
||||
/*terminal_outcome_reached*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let replayed = codex_rollout_trace::replay_bundle(single_bundle_dir(temp.path())?)?;
|
||||
@@ -157,14 +163,17 @@ async fn dispatch_lifecycle_trace_records_unsupported_tool_failures() -> anyhow:
|
||||
let turn = Arc::new(turn);
|
||||
|
||||
let result = registry
|
||||
.dispatch_any(test_invocation(
|
||||
session,
|
||||
turn,
|
||||
"unsupported-call",
|
||||
"missing_tool",
|
||||
ToolCallSource::Direct,
|
||||
"{}",
|
||||
))
|
||||
.dispatch_any_with_terminal_outcome(
|
||||
test_invocation(
|
||||
session,
|
||||
turn,
|
||||
"unsupported-call",
|
||||
"missing_tool",
|
||||
ToolCallSource::Direct,
|
||||
"{}",
|
||||
),
|
||||
/*terminal_outcome_reached*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(matches!(result, Err(FunctionCallError::RespondToModel(_))));
|
||||
@@ -189,16 +198,19 @@ async fn dispatch_lifecycle_trace_records_incompatible_payload_failures() -> any
|
||||
let turn = Arc::new(turn);
|
||||
|
||||
let result = registry
|
||||
.dispatch_any(test_invocation_with_payload(
|
||||
session,
|
||||
turn,
|
||||
"incompatible-call",
|
||||
codex_tools::ToolName::plain("test_tool"),
|
||||
ToolCallSource::Direct,
|
||||
ToolPayload::Custom {
|
||||
input: "{}".to_string(),
|
||||
},
|
||||
))
|
||||
.dispatch_any_with_terminal_outcome(
|
||||
test_invocation_with_payload(
|
||||
session,
|
||||
turn,
|
||||
"incompatible-call",
|
||||
codex_tools::ToolName::plain("test_tool"),
|
||||
ToolCallSource::Direct,
|
||||
ToolPayload::Custom {
|
||||
input: "{}".to_string(),
|
||||
},
|
||||
),
|
||||
/*terminal_outcome_reached*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(matches!(result, Err(FunctionCallError::Fatal(_))));
|
||||
@@ -221,14 +233,17 @@ async fn missing_code_mode_wait_traces_only_the_wait_tool_call() -> anyhow::Resu
|
||||
let turn = Arc::new(turn);
|
||||
|
||||
registry
|
||||
.dispatch_any(test_invocation(
|
||||
session,
|
||||
turn,
|
||||
"wait-call",
|
||||
WAIT_TOOL_NAME,
|
||||
ToolCallSource::Direct,
|
||||
r#"{"cell_id":"noop","terminate":true}"#,
|
||||
))
|
||||
.dispatch_any_with_terminal_outcome(
|
||||
test_invocation(
|
||||
session,
|
||||
turn,
|
||||
"wait-call",
|
||||
WAIT_TOOL_NAME,
|
||||
ToolCallSource::Direct,
|
||||
r#"{"cell_id":"noop","terminate":true}"#,
|
||||
),
|
||||
/*terminal_outcome_reached*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let replayed = codex_rollout_trace::replay_bundle(single_bundle_dir(temp.path())?)?;
|
||||
|
||||
Reference in New Issue
Block a user