mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Make MultiAgentV2 wait minimum configurable (#20052)
## Why MultiAgentV2 `wait_agent` currently clamps short waits to a fixed 10 second minimum. That default is still useful for preventing tight polling loops, but it is too rigid for environments that need faster mailbox wake-up checks or a larger minimum to discourage frequent polling. This PR makes the minimum wait timeout configurable from the existing MultiAgentV2 feature config section, so operators can tune the behavior without changing the legacy multi-agent tool surface. ## What Changed - Added `features.multi_agent_v2.min_wait_timeout_ms`. - Defaulted the new setting to the existing 10 second floor. - Validated the configured value as `1..=3600000`, matching the existing one hour maximum wait bound. - Applied the configured minimum to MultiAgentV2 `wait_agent` runtime clamping. - Plumbed the configured minimum into the `wait_agent` tool schema, including the effective default when the minimum is above the normal 30 second default. - Regenerated `core/config.schema.json`. ## Verification - `cargo test -p codex-features` - `cargo test -p codex-tools` - `cargo test -p codex-core --lib multi_agent_v2` - `just fix -p codex-core`
This commit is contained in:
committed by
GitHub
Unverified
parent
1de7a9bf69
commit
34d71d43eb
@@ -1,5 +1,7 @@
|
||||
use crate::agent::AgentStatus;
|
||||
use crate::config::Config;
|
||||
use crate::config::DEFAULT_MULTI_AGENT_V2_MIN_WAIT_TIMEOUT_MS;
|
||||
use crate::config::MAX_MULTI_AGENT_V2_WAIT_TIMEOUT_MS;
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
@@ -26,9 +28,9 @@ use serde_json::Value as JsonValue;
|
||||
use std::collections::HashMap;
|
||||
|
||||
/// Minimum wait timeout to prevent tight polling loops from burning CPU.
|
||||
pub(crate) const MIN_WAIT_TIMEOUT_MS: i64 = 10_000;
|
||||
pub(crate) const MIN_WAIT_TIMEOUT_MS: i64 = DEFAULT_MULTI_AGENT_V2_MIN_WAIT_TIMEOUT_MS;
|
||||
pub(crate) const DEFAULT_WAIT_TIMEOUT_MS: i64 = 30_000;
|
||||
pub(crate) const MAX_WAIT_TIMEOUT_MS: i64 = 3600 * 1000;
|
||||
pub(crate) const MAX_WAIT_TIMEOUT_MS: i64 = MAX_MULTI_AGENT_V2_WAIT_TIMEOUT_MS;
|
||||
|
||||
pub(crate) fn function_arguments(payload: ToolPayload) -> Result<String, FunctionCallError> {
|
||||
match payload {
|
||||
|
||||
@@ -2742,6 +2742,59 @@ async fn multi_agent_v2_wait_agent_accepts_timeout_only_argument() {
|
||||
assert_eq!(success, None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn multi_agent_v2_wait_agent_uses_configured_min_timeout() {
|
||||
let (session, mut turn) = make_session_and_context().await;
|
||||
let mut config = (*turn.config).clone();
|
||||
config
|
||||
.features
|
||||
.enable(Feature::MultiAgentV2)
|
||||
.expect("test config should allow feature update");
|
||||
config.multi_agent_v2.min_wait_timeout_ms = 50;
|
||||
turn.config = Arc::new(config);
|
||||
let session = Arc::new(session);
|
||||
let turn = Arc::new(turn);
|
||||
|
||||
let early = timeout(
|
||||
Duration::from_millis(/*millis*/ 20),
|
||||
WaitAgentHandlerV2.handle(invocation(
|
||||
session.clone(),
|
||||
turn.clone(),
|
||||
"wait_agent",
|
||||
function_payload(json!({"timeout_ms": 1})),
|
||||
)),
|
||||
)
|
||||
.await;
|
||||
assert!(
|
||||
early.is_err(),
|
||||
"wait_agent should not return before the configured minimum timeout"
|
||||
);
|
||||
|
||||
let output = timeout(
|
||||
Duration::from_secs(/*secs*/ 1),
|
||||
WaitAgentHandlerV2.handle(invocation(
|
||||
session,
|
||||
turn,
|
||||
"wait_agent",
|
||||
function_payload(json!({"timeout_ms": 1})),
|
||||
)),
|
||||
)
|
||||
.await
|
||||
.expect("configured minimum should be shorter than the test timeout")
|
||||
.expect("wait_agent should succeed");
|
||||
let (content, success) = expect_text_output(output);
|
||||
let result: crate::tools::handlers::multi_agents_v2::wait::WaitAgentResult =
|
||||
serde_json::from_str(&content).expect("wait_agent result should be json");
|
||||
assert_eq!(
|
||||
result,
|
||||
crate::tools::handlers::multi_agents_v2::wait::WaitAgentResult {
|
||||
message: "Wait timed out.".to_string(),
|
||||
timed_out: true,
|
||||
}
|
||||
);
|
||||
assert_eq!(success, None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn wait_agent_returns_not_found_for_missing_agents() {
|
||||
let (mut session, turn) = make_session_and_context().await;
|
||||
|
||||
@@ -28,13 +28,18 @@ impl ToolHandler for Handler {
|
||||
let arguments = function_arguments(payload)?;
|
||||
let args: WaitArgs = parse_arguments(&arguments)?;
|
||||
let timeout_ms = args.timeout_ms.unwrap_or(DEFAULT_WAIT_TIMEOUT_MS);
|
||||
let min_timeout_ms = turn
|
||||
.config
|
||||
.multi_agent_v2
|
||||
.min_wait_timeout_ms
|
||||
.clamp(1, MAX_WAIT_TIMEOUT_MS);
|
||||
let timeout_ms = match timeout_ms {
|
||||
ms if ms <= 0 => {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"timeout_ms must be greater than zero".to_owned(),
|
||||
));
|
||||
}
|
||||
ms => ms.clamp(MIN_WAIT_TIMEOUT_MS, MAX_WAIT_TIMEOUT_MS),
|
||||
ms => ms.clamp(min_timeout_ms, MAX_WAIT_TIMEOUT_MS),
|
||||
};
|
||||
|
||||
let mut mailbox_seq_rx = session.subscribe_mailbox_seq();
|
||||
|
||||
@@ -124,6 +124,16 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
});
|
||||
let default_agent_type_description =
|
||||
crate::agent::role::spawn_tool_spec::build(&std::collections::BTreeMap::new());
|
||||
let min_wait_timeout_ms = if config.multi_agent_v2 {
|
||||
config
|
||||
.wait_agent_min_timeout_ms
|
||||
.unwrap_or(MIN_WAIT_TIMEOUT_MS)
|
||||
.clamp(1, MAX_WAIT_TIMEOUT_MS)
|
||||
} else {
|
||||
MIN_WAIT_TIMEOUT_MS
|
||||
};
|
||||
let default_wait_timeout_ms =
|
||||
DEFAULT_WAIT_TIMEOUT_MS.clamp(min_wait_timeout_ms, MAX_WAIT_TIMEOUT_MS);
|
||||
let plan = build_tool_registry_plan(
|
||||
config,
|
||||
ToolRegistryPlanParams {
|
||||
@@ -138,8 +148,8 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
dynamic_tools,
|
||||
default_agent_type_description: &default_agent_type_description,
|
||||
wait_agent_timeouts: WaitAgentTimeoutOptions {
|
||||
default_timeout_ms: DEFAULT_WAIT_TIMEOUT_MS,
|
||||
min_timeout_ms: MIN_WAIT_TIMEOUT_MS,
|
||||
default_timeout_ms: default_wait_timeout_ms,
|
||||
min_timeout_ms: min_wait_timeout_ms,
|
||||
max_timeout_ms: MAX_WAIT_TIMEOUT_MS,
|
||||
},
|
||||
},
|
||||
|
||||
@@ -784,6 +784,35 @@ async fn spawn_agent_description_uses_configured_usage_hint_text() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn multi_agent_v2_wait_agent_schema_uses_configured_min_timeout() {
|
||||
let wait_agent_min_timeout_ms = Some(60_000);
|
||||
let tools_config = multi_agent_v2_tools_config()
|
||||
.await
|
||||
.with_wait_agent_min_timeout_ms(wait_agent_min_timeout_ms);
|
||||
let (tools, _) = build_specs(
|
||||
&tools_config,
|
||||
/*mcp_tools*/ None,
|
||||
/*deferred_mcp_tools*/ None,
|
||||
&[],
|
||||
)
|
||||
.build();
|
||||
let wait_agent = find_tool(&tools, "wait_agent");
|
||||
let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = &wait_agent.spec else {
|
||||
panic!("wait_agent should be a function tool");
|
||||
};
|
||||
let timeout_description = parameters
|
||||
.properties
|
||||
.as_ref()
|
||||
.and_then(|properties| properties.get("timeout_ms"))
|
||||
.and_then(|schema| schema.description.as_deref());
|
||||
|
||||
assert_eq!(
|
||||
timeout_description,
|
||||
Some("Optional timeout in milliseconds. Defaults to 60000, min 60000, max 3600000.")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn tool_suggest_requires_apps_and_plugins_features() {
|
||||
let model_info = search_capable_model_info().await;
|
||||
|
||||
Reference in New Issue
Block a user