diff --git a/.github/dotslash-unsigned-config.json b/.github/dotslash-unsigned-config.json deleted file mode 100644 index 65c44d5e8d..0000000000 --- a/.github/dotslash-unsigned-config.json +++ /dev/null @@ -1,124 +0,0 @@ -{ - "outputs": { - "codex-unsigned": { - "platforms": { - "macos-aarch64": { - "regex": "^codex-aarch64-apple-darwin-unsigned\\.zst$", - "path": "codex" - }, - "macos-x86_64": { - "regex": "^codex-x86_64-apple-darwin-unsigned\\.zst$", - "path": "codex" - }, - "linux-x86_64": { - "regex": "^codex-x86_64-unknown-linux-musl-bundle\\.tar\\.zst$", - "path": "codex" - }, - "linux-aarch64": { - "regex": "^codex-aarch64-unknown-linux-musl-bundle\\.tar\\.zst$", - "path": "codex" - }, - "windows-x86_64": { - "regex": "^codex-x86_64-pc-windows-msvc\\.exe\\.zst$", - "path": "codex.exe" - }, - "windows-aarch64": { - "regex": "^codex-aarch64-pc-windows-msvc\\.exe\\.zst$", - "path": "codex.exe" - } - } - }, - "codex-app-server-unsigned": { - "platforms": { - "macos-aarch64": { - "regex": "^codex-app-server-aarch64-apple-darwin-unsigned\\.zst$", - "path": "codex-app-server" - }, - "macos-x86_64": { - "regex": "^codex-app-server-x86_64-apple-darwin-unsigned\\.zst$", - "path": "codex-app-server" - }, - "linux-x86_64": { - "regex": "^codex-app-server-x86_64-unknown-linux-musl\\.zst$", - "path": "codex-app-server" - }, - "linux-aarch64": { - "regex": "^codex-app-server-aarch64-unknown-linux-musl\\.zst$", - "path": "codex-app-server" - }, - "windows-x86_64": { - "regex": "^codex-app-server-x86_64-pc-windows-msvc\\.exe\\.zst$", - "path": "codex-app-server.exe" - }, - "windows-aarch64": { - "regex": "^codex-app-server-aarch64-pc-windows-msvc\\.exe\\.zst$", - "path": "codex-app-server.exe" - } - } - }, - "codex-responses-api-proxy-unsigned": { - "platforms": { - "macos-aarch64": { - "regex": "^codex-responses-api-proxy-aarch64-apple-darwin-unsigned\\.zst$", - "path": "codex-responses-api-proxy" - }, - "macos-x86_64": { - "regex": "^codex-responses-api-proxy-x86_64-apple-darwin-unsigned\\.zst$", - "path": "codex-responses-api-proxy" - }, - "linux-x86_64": { - "regex": "^codex-responses-api-proxy-x86_64-unknown-linux-musl\\.zst$", - "path": "codex-responses-api-proxy" - }, - "linux-aarch64": { - "regex": "^codex-responses-api-proxy-aarch64-unknown-linux-musl\\.zst$", - "path": "codex-responses-api-proxy" - }, - "windows-x86_64": { - "regex": "^codex-responses-api-proxy-x86_64-pc-windows-msvc\\.exe\\.zst$", - "path": "codex-responses-api-proxy.exe" - }, - "windows-aarch64": { - "regex": "^codex-responses-api-proxy-aarch64-pc-windows-msvc\\.exe\\.zst$", - "path": "codex-responses-api-proxy.exe" - } - } - }, - "bwrap": { - "platforms": { - "linux-x86_64": { - "regex": "^bwrap-x86_64-unknown-linux-musl\\.zst$", - "path": "bwrap" - }, - "linux-aarch64": { - "regex": "^bwrap-aarch64-unknown-linux-musl\\.zst$", - "path": "bwrap" - } - } - }, - "codex-command-runner": { - "platforms": { - "windows-x86_64": { - "regex": "^codex-command-runner-x86_64-pc-windows-msvc\\.exe\\.zst$", - "path": "codex-command-runner.exe" - }, - "windows-aarch64": { - "regex": "^codex-command-runner-aarch64-pc-windows-msvc\\.exe\\.zst$", - "path": "codex-command-runner.exe" - } - } - }, - "codex-windows-sandbox-setup": { - "platforms": { - "windows-x86_64": { - "regex": "^codex-windows-sandbox-setup-x86_64-pc-windows-msvc\\.exe\\.zst$", - "path": "codex-windows-sandbox-setup.exe" - }, - "windows-aarch64": { - "regex": "^codex-windows-sandbox-setup-aarch64-pc-windows-msvc\\.exe\\.zst$", - "path": "codex-windows-sandbox-setup.exe" - } - } - } - } -} diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index 2dc92cf684..0e13aa366c 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -1223,14 +1223,6 @@ jobs: tag: ${{ github.ref_name }} config: .github/dotslash-config.json - - if: ${{ env.SIGN_MACOS == 'false' }} - uses: facebook/dotslash-publish-release@9c9ec027515c34db9282a09a25a9cab5880b2c52 # v2 - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - with: - tag: ${{ github.ref_name }} - config: .github/dotslash-unsigned-config.json - - if: ${{ env.SIGN_MACOS == 'true' }} uses: facebook/dotslash-publish-release@9c9ec027515c34db9282a09a25a9cab5880b2c52 # v2 env: @@ -1265,7 +1257,15 @@ jobs: # npm docs: https://docs.npmjs.com/trusted-publishers publish-npm: # Publish to npm for stable releases and alpha pre-releases with numeric suffixes. - if: ${{ needs.release.outputs.should_publish_npm == 'true' }} + # promote_signed intentionally skips build jobs that are ancestors of release; + # include the !cancelled() status function so Actions does not apply its implicit + # success() check to the whole dependency chain before evaluating release outputs. + if: >- + ${{ + !cancelled() && + needs.release.result == 'success' && + needs.release.outputs.should_publish_npm == 'true' + }} name: publish-npm needs: release runs-on: ubuntu-latest @@ -1423,7 +1423,12 @@ jobs: # need release follow-up, but should not invalidate the Rust release itself. publish-python-runtime: # Publish to PyPI for stable releases and alpha pre-releases with numeric suffixes. - if: ${{ needs.release.outputs.should_publish_python_runtime == 'true' }} + if: >- + ${{ + !cancelled() && + needs.release.result == 'success' && + needs.release.outputs.should_publish_python_runtime == 'true' + }} name: publish-python-runtime needs: release runs-on: ubuntu-latest @@ -1464,7 +1469,13 @@ jobs: needs: release # Only publish stable/mainline releases to WinGet; pre-releases include a # '-' in the semver string (e.g., 1.2.3-alpha.1). - if: ${{ needs.release.outputs.sign_macos == 'true' && !contains(needs.release.outputs.version, '-') }} + if: >- + ${{ + !cancelled() && + needs.release.result == 'success' && + needs.release.outputs.sign_macos == 'true' && + !contains(needs.release.outputs.version, '-') + }} # This job only invokes a GitHub Action to open/update the winget-pkgs PR; # it does not execute Windows-only tooling, so Linux is sufficient. runs-on: ubuntu-latest @@ -1484,7 +1495,12 @@ jobs: update-branch: name: Update latest-alpha-cli branch - if: ${{ needs.release.outputs.sign_macos == 'true' }} + if: >- + ${{ + !cancelled() && + needs.release.result == 'success' && + needs.release.outputs.sign_macos == 'true' + }} permissions: contents: write needs: release diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 50de8dc946..eb2439401b 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1907,6 +1907,7 @@ dependencies = [ "codex-hooks", "codex-login", "codex-mcp", + "codex-memories-extension", "codex-memories-write", "codex-model-provider", "codex-model-provider-info", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 14b1264a81..7207fd3aba 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -470,7 +470,6 @@ unwrap_used = "deny" [workspace.metadata.cargo-shear] ignored = [ "codex-agent-graph-store", - "codex-memories-extension", "icu_provider", "openssl-sys", "codex-v8-poc", diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index f40d16feb6..fffc610128 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -2729,6 +2729,22 @@ "title": "CompactionResponseItem", "type": "object" }, + { + "properties": { + "type": { + "enum": [ + "compaction_trigger" + ], + "title": "CompactionTriggerResponseItemType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "CompactionTriggerResponseItem", + "type": "object" + }, { "properties": { "encrypted_content": { diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 12cb9d83b8..935ec98e88 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -14065,6 +14065,22 @@ "title": "CompactionResponseItem", "type": "object" }, + { + "properties": { + "type": { + "enum": [ + "compaction_trigger" + ], + "title": "CompactionTriggerResponseItemType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "CompactionTriggerResponseItem", + "type": "object" + }, { "properties": { "encrypted_content": { diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 4a199057d8..ccf68b28d8 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -10614,6 +10614,22 @@ "title": "CompactionResponseItem", "type": "object" }, + { + "properties": { + "type": { + "enum": [ + "compaction_trigger" + ], + "title": "CompactionTriggerResponseItemType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "CompactionTriggerResponseItem", + "type": "object" + }, { "properties": { "encrypted_content": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json index 6973d15baa..bd88872fb6 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json @@ -732,6 +732,22 @@ "title": "CompactionResponseItem", "type": "object" }, + { + "properties": { + "type": { + "enum": [ + "compaction_trigger" + ], + "title": "CompactionTriggerResponseItemType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "CompactionTriggerResponseItem", + "type": "object" + }, { "properties": { "encrypted_content": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json index 27674afc7b..e8a437f003 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json @@ -799,6 +799,22 @@ "title": "CompactionResponseItem", "type": "object" }, + { + "properties": { + "type": { + "enum": [ + "compaction_trigger" + ], + "title": "CompactionTriggerResponseItemType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "CompactionTriggerResponseItem", + "type": "object" + }, { "properties": { "encrypted_content": { diff --git a/codex-rs/app-server-protocol/schema/typescript/ResponseItem.ts b/codex-rs/app-server-protocol/schema/typescript/ResponseItem.ts index 6fa9beee25..e5e960ff81 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ResponseItem.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ResponseItem.ts @@ -14,4 +14,4 @@ export type ResponseItem = { "type": "message", role: string, content: Array::new(); codex_guardian::install(&mut builder, guardian_agent_spawner); + codex_memories_extension::install(&mut builder); Arc::new(builder.build()) } diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index 37cb41b3e1..89a35afebe 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -1,6 +1,8 @@ mod layer_io; #[cfg(target_os = "macos")] mod macos; +#[cfg(test)] +mod tests; use self::layer_io::LoadedConfigLayers; use crate::CONFIG_TOML_FILE; @@ -211,19 +213,35 @@ pub async fn load_config_layers_state( // Add the base user config layer. When profile-v2 is selected, add the // profile config as a second user layer on top so the profile only needs to // contain overrides. - let base_user_file = AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, codex_home); - layers.push( - load_user_config_layer( - fs, - &base_user_file, - /*profile*/ None, - ignore_user_config, - strict_config, - ) - .await?, - ); - let active_user_file = overrides.user_config_path(codex_home)?; + let base_user_file = AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, codex_home); + let base_user_layer = load_user_config_layer( + fs, + &base_user_file, + /*profile*/ None, + ignore_user_config, + strict_config, + ) + .await?; + if let Some(active_user_profile) = active_user_profile.as_ref() + && base_user_layer.config.as_table().is_some_and(|config| { + config + .get("profiles") + .and_then(TomlValue::as_table) + .is_some_and(|profiles| profiles.contains_key(active_user_profile.as_str())) + }) + { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!( + "--profile-v2 `{active_user_profile}` cannot be used while {} contains legacy `[profiles.{active_user_profile}]` config; move those settings into {} or remove `[profiles.{active_user_profile}]`", + base_user_file.as_path().display(), + active_user_file.as_path().display() + ), + )); + } + layers.push(base_user_layer); + if active_user_file != base_user_file { layers.push( load_user_config_layer( diff --git a/codex-rs/config/src/loader/tests.rs b/codex-rs/config/src/loader/tests.rs new file mode 100644 index 0000000000..2d9462bfa7 --- /dev/null +++ b/codex-rs/config/src/loader/tests.rs @@ -0,0 +1,172 @@ +use super::*; +use async_trait::async_trait; +use codex_file_system::CopyOptions; +use codex_file_system::CreateDirectoryOptions; +use codex_file_system::FileMetadata; +use codex_file_system::FileSystemResult; +use codex_file_system::FileSystemSandboxContext; +use codex_file_system::ReadDirectoryEntry; +use codex_file_system::RemoveOptions; +use pretty_assertions::assert_eq; +use tempfile::tempdir; + +struct TestFileSystem; + +#[async_trait] +impl ExecutorFileSystem for TestFileSystem { + async fn read_file( + &self, + path: &AbsolutePathBuf, + _sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult> { + tokio::fs::read(path.as_path()).await + } + + async fn write_file( + &self, + _path: &AbsolutePathBuf, + _contents: Vec, + _sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + unimplemented!("test filesystem only supports reads") + } + + async fn create_directory( + &self, + _path: &AbsolutePathBuf, + _create_directory_options: CreateDirectoryOptions, + _sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + unimplemented!("test filesystem only supports reads") + } + + async fn get_metadata( + &self, + _path: &AbsolutePathBuf, + _sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult { + unimplemented!("test filesystem only supports reads") + } + + async fn read_directory( + &self, + _path: &AbsolutePathBuf, + _sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult> { + unimplemented!("test filesystem only supports reads") + } + + async fn remove( + &self, + _path: &AbsolutePathBuf, + _remove_options: RemoveOptions, + _sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + unimplemented!("test filesystem only supports reads") + } + + async fn copy( + &self, + _source_path: &AbsolutePathBuf, + _destination_path: &AbsolutePathBuf, + _copy_options: CopyOptions, + _sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + unimplemented!("test filesystem only supports reads") + } +} + +#[tokio::test] +async fn profile_v2_rejects_matching_legacy_profile_in_base_user_config() { + let tmp = tempdir().expect("tempdir"); + let selected_config = tmp.path().join("work.config.toml"); + + std::fs::write( + tmp.path().join(CONFIG_TOML_FILE), + r#" +model = "gpt-main" + +[profiles.work] +model = "gpt-work" +"#, + ) + .expect("write default user config"); + std::fs::write(&selected_config, r#"model = "gpt-work-v2""#) + .expect("write selected user config"); + + let mut overrides = LoaderOverrides::without_managed_config_for_tests(); + overrides.user_config_path = Some(AbsolutePathBuf::resolve_path_against_base( + "work.config.toml", + tmp.path(), + )); + overrides.user_config_profile = Some("work".parse().expect("profile-v2 name")); + + let err = load_config_layers_state( + &TestFileSystem, + tmp.path(), + /*cwd*/ None, + &[], + overrides, + CloudRequirementsLoader::default(), + &crate::NoopThreadConfigLoader, + ) + .await + .expect_err("profile-v2 should reject a matching legacy profile in base user config"); + + assert_eq!( + err.kind(), + io::ErrorKind::InvalidData, + "a matching legacy profile should be a hard config error" + ); + let message = err.to_string(); + assert!( + message.contains("--profile-v2 `work` cannot be used"), + "unexpected error message: {message}" + ); + assert!( + message.contains("config.toml"), + "unexpected error message: {message}" + ); + assert!( + message.contains("[profiles.work]"), + "unexpected error message: {message}" + ); +} + +#[tokio::test] +async fn profile_v2_allows_unrelated_legacy_profiles_in_base_user_config() { + let tmp = tempdir().expect("tempdir"); + let selected_config = tmp.path().join("work.config.toml"); + + std::fs::write( + tmp.path().join(CONFIG_TOML_FILE), + r#" +model = "gpt-main" + +[profiles.dev] +model = "gpt-dev" +"#, + ) + .expect("write default user config"); + std::fs::write(&selected_config, r#"model = "gpt-work-v2""#) + .expect("write selected user config"); + + let mut overrides = LoaderOverrides::without_managed_config_for_tests(); + overrides.user_config_path = Some(AbsolutePathBuf::resolve_path_against_base( + "work.config.toml", + tmp.path(), + )); + overrides.user_config_profile = Some("work".parse().expect("profile-v2 name")); + + load_config_layers_state( + &TestFileSystem, + tmp.path(), + /*cwd*/ None, + &[], + overrides, + CloudRequirementsLoader::default(), + &crate::NoopThreadConfigLoader, + ) + .await + .expect("profile-v2 should allow unrelated legacy profiles in base user config"); +} diff --git a/codex-rs/core/src/agent/control.rs b/codex-rs/core/src/agent/control.rs index d973f40e45..329c6048b0 100644 --- a/codex-rs/core/src/agent/control.rs +++ b/codex-rs/core/src/agent/control.rs @@ -116,6 +116,7 @@ fn keep_forked_rollout_item(item: &RolloutItem) -> bool { | ResponseItem::WebSearchCall { .. } | ResponseItem::ImageGenerationCall { .. } | ResponseItem::Compaction { .. } + | ResponseItem::CompactionTrigger | ResponseItem::ContextCompaction { .. } | ResponseItem::Other, ) => false, diff --git a/codex-rs/core/src/arc_monitor.rs b/codex-rs/core/src/arc_monitor.rs index d1e679a631..a4f7e038e0 100644 --- a/codex-rs/core/src/arc_monitor.rs +++ b/codex-rs/core/src/arc_monitor.rs @@ -384,6 +384,7 @@ fn build_arc_monitor_message_item( | ResponseItem::ToolSearchOutput { .. } | ResponseItem::ImageGenerationCall { .. } | ResponseItem::Compaction { .. } + | ResponseItem::CompactionTrigger | ResponseItem::ContextCompaction { .. } | ResponseItem::Other => None, } diff --git a/codex-rs/core/src/compact_remote.rs b/codex-rs/core/src/compact_remote.rs index cc31d50b13..bc684647eb 100644 --- a/codex-rs/core/src/compact_remote.rs +++ b/codex-rs/core/src/compact_remote.rs @@ -302,6 +302,7 @@ fn should_keep_compacted_history_item(item: &ResponseItem) -> bool { ResponseItem::Message { role, .. } if role == "assistant" => true, ResponseItem::Message { .. } => false, ResponseItem::Compaction { .. } | ResponseItem::ContextCompaction { .. } => true, + ResponseItem::CompactionTrigger => false, ResponseItem::Reasoning { .. } | ResponseItem::LocalShellCall { .. } | ResponseItem::FunctionCall { .. } diff --git a/codex-rs/core/src/compact_remote_v2.rs b/codex-rs/core/src/compact_remote_v2.rs index 4dafb95f8e..3f5368a960 100644 --- a/codex-rs/core/src/compact_remote_v2.rs +++ b/codex-rs/core/src/compact_remote_v2.rs @@ -12,6 +12,10 @@ use crate::compact_remote::build_compact_request_log_data; use crate::compact_remote::log_remote_compact_failure; use crate::compact_remote::process_compacted_history; use crate::compact_remote::trim_function_call_history_to_fit_context_window; +use crate::hook_runtime::PostCompactHookOutcome; +use crate::hook_runtime::PreCompactHookOutcome; +use crate::hook_runtime::run_post_compact_hooks; +use crate::hook_runtime::run_pre_compact_hooks; use crate::session::session::Session; use crate::session::turn::built_tools; use crate::session::turn_context::TurnContext; @@ -97,6 +101,21 @@ async fn run_remote_compact_task_inner( phase, ) .await; + let pre_compact_outcome = run_pre_compact_hooks(sess, turn_context, trigger).await; + match pre_compact_outcome { + PreCompactHookOutcome::Continue => {} + PreCompactHookOutcome::Stopped { reason } => { + let error = reason.unwrap_or_else(|| "PreCompact hook stopped execution".to_string()); + attempt + .track( + sess.as_ref(), + codex_analytics::CompactionStatus::Interrupted, + Some(error), + ) + .await; + return Err(CodexErr::TurnAborted); + } + } let result = run_remote_compact_task_inner_impl( sess, turn_context, @@ -104,13 +123,16 @@ async fn run_remote_compact_task_inner( initial_context_injection, ) .await; - attempt - .track( - sess.as_ref(), - compaction_status_from_result(&result), - result.as_ref().err().map(ToString::to_string), - ) - .await; + let status = compaction_status_from_result(&result); + let error = result.as_ref().err().map(ToString::to_string); + if result.is_ok() { + let post_compact_outcome = run_post_compact_hooks(sess, turn_context, trigger).await; + if let PostCompactHookOutcome::Stopped = post_compact_outcome { + attempt.track(sess.as_ref(), status, error).await; + return Err(CodexErr::TurnAborted); + } + } + attempt.track(sess.as_ref(), status, error.clone()).await; if let Err(err) = result { let event = EventMsg::Error( err.to_error_event(Some("Error running remote compact task".to_string())), @@ -165,9 +187,7 @@ async fn run_remote_compact_task_inner_impl( ) .await?; let mut input = prompt_input.clone(); - input.push(ResponseItem::ContextCompaction { - encrypted_content: None, - }); + input.push(ResponseItem::CompactionTrigger); let prompt = Prompt { input, tools: tool_router.model_visible_specs(), @@ -276,38 +296,25 @@ async fn run_remote_compaction_request_v2( Err(err) }) .await?; - collect_context_compaction_output(stream).await + collect_compaction_output(stream).await } -async fn collect_context_compaction_output( +async fn collect_compaction_output( mut stream: ResponseStream, ) -> CodexResult<(ResponseItem, String)> { let mut output_item_count = 0usize; - let mut context_compaction_count = 0usize; - let mut context_compaction_output = None; + let mut compaction_count = 0usize; + let mut compaction_output = None; let mut completed_response_id = None; while let Some(event) = stream.next().await { match event? { ResponseEvent::OutputItemDone(item) => { output_item_count += 1; - match item { - ResponseItem::ContextCompaction { - encrypted_content: Some(_), - } => { - context_compaction_count += 1; - if context_compaction_output.is_none() { - context_compaction_output = Some(item); - } + if let ResponseItem::Compaction { .. } = item { + compaction_count += 1; + if compaction_output.is_none() { + compaction_output = Some(item); } - ResponseItem::ContextCompaction { - encrypted_content: None, - } => { - return Err(CodexErr::Fatal( - "remote compaction v2 returned context_compaction without encrypted_content" - .to_string(), - )); - } - _ => {} } } ResponseEvent::Completed { response_id, .. } => { @@ -324,16 +331,16 @@ async fn collect_context_compaction_output( )); }; - if context_compaction_count != 1 { + if compaction_count != 1 { return Err(CodexErr::Fatal(format!( - "remote compaction v2 expected exactly one context_compaction output item, got {context_compaction_count} from {output_item_count} output items" + "remote compaction v2 expected exactly one compaction output item, got {compaction_count} from {output_item_count} output items" ))); } - let Some(context_compaction_output) = context_compaction_output else { - unreachable!("context compaction output must exist when count is exactly one"); + let Some(compaction_output) = compaction_output else { + unreachable!("compaction output must exist when count is exactly one"); }; - Ok((context_compaction_output, response_id)) + Ok((compaction_output, response_id)) } fn build_v2_compacted_history( @@ -410,8 +417,8 @@ mod tests { encrypted_content: "old".to_string(), }, ]; - let output = ResponseItem::ContextCompaction { - encrypted_content: Some("new".to_string()), + let output = ResponseItem::Compaction { + encrypted_content: "new".to_string(), }; let history = build_v2_compacted_history(&input, output.clone()); @@ -428,9 +435,9 @@ mod tests { } #[tokio::test] - async fn collect_context_compaction_output_accepts_additional_output_items() { - let context_compaction = ResponseItem::ContextCompaction { - encrypted_content: Some("encrypted".to_string()), + async fn collect_compaction_output_accepts_additional_output_items() { + let compaction = ResponseItem::Compaction { + encrypted_content: "encrypted".to_string(), }; let stream = response_stream(vec![ Ok(ResponseEvent::OutputItemDone(message( @@ -438,7 +445,7 @@ mod tests { "IGNORED_COMPACT_REPLY", Some(MessagePhase::FinalAnswer), ))), - Ok(ResponseEvent::OutputItemDone(context_compaction.clone())), + Ok(ResponseEvent::OutputItemDone(compaction.clone())), Ok(ResponseEvent::Completed { response_id: "resp-compact".to_string(), token_usage: None, @@ -446,11 +453,11 @@ mod tests { }), ]); - let (output, response_id) = collect_context_compaction_output(stream) + let (output, response_id) = collect_compaction_output(stream) .await - .expect("context compaction should be collected"); + .expect("compaction should be collected"); - assert_eq!(output, context_compaction); + assert_eq!(output, compaction); assert_eq!(response_id, "resp-compact"); } } diff --git a/codex-rs/core/src/context_manager/history.rs b/codex-rs/core/src/context_manager/history.rs index 80c057e0eb..cfd9297382 100644 --- a/codex-rs/core/src/context_manager/history.rs +++ b/codex-rs/core/src/context_manager/history.rs @@ -400,6 +400,7 @@ impl ContextManager { | ResponseItem::ImageGenerationCall { .. } | ResponseItem::CustomToolCall { .. } | ResponseItem::Compaction { .. } + | ResponseItem::CompactionTrigger | ResponseItem::ContextCompaction { .. } | ResponseItem::Other => item.clone(), } @@ -490,6 +491,7 @@ fn is_api_message(message: &ResponseItem) -> bool { | ResponseItem::ImageGenerationCall { .. } | ResponseItem::Compaction { .. } | ResponseItem::ContextCompaction { .. } => true, + ResponseItem::CompactionTrigger => false, ResponseItem::Other => false, } } @@ -688,6 +690,7 @@ fn is_model_generated_item(item: &ResponseItem) -> bool { | ResponseItem::LocalShellCall { .. } | ResponseItem::Compaction { .. } | ResponseItem::ContextCompaction { .. } => true, + ResponseItem::CompactionTrigger => false, ResponseItem::FunctionCallOutput { .. } | ResponseItem::ToolSearchOutput { .. } | ResponseItem::CustomToolCallOutput { .. } diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 142e39ea97..2dd97710e8 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -2690,14 +2690,6 @@ impl Session { { developer_sections.push(developer_instructions.to_string()); } - // Add developer instructions for memories. - if turn_context.features.enabled(Feature::MemoryTool) - && turn_context.config.memories.use_memories - && let Some(memory_prompt) = - build_memory_tool_developer_instructions(&turn_context.config.codex_home).await - { - developer_sections.push(memory_prompt); - } // Add developer instructions from collaboration_mode if they exist and are non-empty if turn_context.config.include_collaboration_mode_instructions && let Some(collab_instructions) = @@ -3438,8 +3430,6 @@ pub(crate) fn emit_subagent_session_started( }); } -use codex_memories_read::build_memory_tool_developer_instructions; - /// Builds the hook engine for one config snapshot, including any enabled plugin hooks. async fn build_hooks_for_config( config: &Config, diff --git a/codex-rs/core/src/session/review.rs b/codex-rs/core/src/session/review.rs index acb4adb945..497cddd912 100644 --- a/codex-rs/core/src/session/review.rs +++ b/codex-rs/core/src/session/review.rs @@ -46,7 +46,7 @@ pub(super) async fn spawn_review_thread( .with_image_generation_capability(provider_capabilities.image_generation) .with_web_search_capability(provider_capabilities.web_search) .with_unified_exec_shell_mode_for_session( - crate::tools::spec::tool_user_shell_type(sess.services.user_shell.as_ref()), + crate::tools::tool_user_shell_type(sess.services.user_shell.as_ref()), sess.services.shell_zsh_path.as_ref(), sess.services.main_execve_wrapper_exe.as_ref(), ) diff --git a/codex-rs/core/src/session/tests/guardian_tests.rs b/codex-rs/core/src/session/tests/guardian_tests.rs index 718b9bf05f..5db4aab985 100644 --- a/codex-rs/core/src/session/tests/guardian_tests.rs +++ b/codex-rs/core/src/session/tests/guardian_tests.rs @@ -5,8 +5,9 @@ use crate::exec_policy::ExecPolicyManager; use crate::guardian::GUARDIAN_REVIEWER_NAME; use crate::sandboxing::SandboxPermissions; use crate::test_support::models_manager_with_provider; -use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolCallSource; +use crate::tools::context::ToolOutput; +use crate::tools::context::ToolPayload; use crate::turn_diff_tracker::TurnDiffTracker; use codex_app_server_protocol::ConfigLayerSource; use codex_config::ConfigLayerEntry; @@ -22,8 +23,8 @@ use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::models::AdditionalPermissionProfile as PermissionProfile; use codex_protocol::models::ContentItem; use codex_protocol::models::NetworkPermissions; +use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; -use codex_protocol::models::function_call_output_content_items_to_text; use codex_protocol::protocol::AskForApproval; use codex_protocol::request_permissions::PermissionGrantScope; use codex_protocol::request_permissions::RequestPermissionProfile; @@ -48,8 +49,23 @@ use tempfile::tempdir; use tokio::time::timeout; use tokio_util::sync::CancellationToken; -fn expect_text_output(output: &FunctionToolOutput) -> String { - function_call_output_content_items_to_text(&output.body).unwrap_or_default() +fn expect_text_output(output: &T) -> String +where + T: ToolOutput + ?Sized, +{ + let response = output.to_response_item( + "call-guardian", + &ToolPayload::Function { + arguments: "{}".to_string(), + }, + ); + match response { + ResponseInputItem::FunctionCallOutput { output, .. } + | ResponseInputItem::CustomToolCallOutput { output, .. } => { + output.body.to_text().unwrap_or_default() + } + other => panic!("expected function output, got {other:?}"), + } } #[tokio::test] diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 86710e4871..aee4bd360f 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -2002,6 +2002,7 @@ async fn try_run_sampling_request( | ResponseItem::WebSearchCall { .. } | ResponseItem::ImageGenerationCall { .. } | ResponseItem::Compaction { .. } + | ResponseItem::CompactionTrigger | ResponseItem::ContextCompaction { .. } | ResponseItem::Other => false, }; diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index cab18b8561..92c7893d5e 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -527,7 +527,7 @@ impl Session { .with_image_generation_capability(provider_capabilities.image_generation) .with_web_search_capability(provider_capabilities.web_search) .with_unified_exec_shell_mode_for_session( - crate::tools::spec::tool_user_shell_type(user_shell), + crate::tools::tool_user_shell_type(user_shell), shell_zsh_path, main_execve_wrapper_exe, ) diff --git a/codex-rs/core/src/tools/code_mode/execute_handler.rs b/codex-rs/core/src/tools/code_mode/execute_handler.rs index 26ef021ad2..e101e1a800 100644 --- a/codex-rs/core/src/tools/code_mode/execute_handler.rs +++ b/codex-rs/core/src/tools/code_mode/execute_handler.rs @@ -2,8 +2,9 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -89,8 +90,6 @@ impl CodeModeExecuteHandler { #[async_trait::async_trait] impl ToolExecutor for CodeModeExecuteHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain(PUBLIC_TOOL_NAME) } @@ -99,7 +98,10 @@ impl ToolExecutor for CodeModeExecuteHandler { Some(self.spec.clone()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -110,9 +112,10 @@ impl ToolExecutor for CodeModeExecuteHandler { } = invocation; match payload { - ToolPayload::Custom { input } if is_exec_tool_name(&tool_name) => { - self.execute(session, turn, call_id, input).await - } + ToolPayload::Custom { input } if is_exec_tool_name(&tool_name) => self + .execute(session, turn, call_id, input) + .await + .map(boxed_tool_output), _ => Err(FunctionCallError::RespondToModel(format!( "{PUBLIC_TOOL_NAME} expects raw JavaScript source text" ))), @@ -120,7 +123,7 @@ impl ToolExecutor for CodeModeExecuteHandler { } } -impl ToolHandler for CodeModeExecuteHandler { +impl CoreToolRuntime for CodeModeExecuteHandler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Custom { .. }) } diff --git a/codex-rs/core/src/tools/code_mode/wait_handler.rs b/codex-rs/core/src/tools/code_mode/wait_handler.rs index 4f828c8910..725535339f 100644 --- a/codex-rs/core/src/tools/code_mode/wait_handler.rs +++ b/codex-rs/core/src/tools/code_mode/wait_handler.rs @@ -1,11 +1,11 @@ use serde::Deserialize; use crate::function_tool::FunctionCallError; -use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -43,8 +43,6 @@ where #[async_trait::async_trait] impl ToolExecutor for CodeModeWaitHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain(WAIT_TOOL_NAME) } @@ -53,7 +51,10 @@ impl ToolExecutor for CodeModeWaitHandler { Some(create_wait_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -99,6 +100,7 @@ impl ToolExecutor for CodeModeWaitHandler { } handle_runtime_response(&exec, wait_response.into(), args.max_tokens, started_at) .await + .map(boxed_tool_output) .map_err(FunctionCallError::RespondToModel) } _ => Err(FunctionCallError::RespondToModel(format!( @@ -108,4 +110,4 @@ impl ToolExecutor for CodeModeWaitHandler { } } -impl ToolHandler for CodeModeWaitHandler {} +impl CoreToolRuntime for CodeModeWaitHandler {} diff --git a/codex-rs/core/src/tools/context.rs b/codex-rs/core/src/tools/context.rs index 0f3ae631a0..5fc19a2a2e 100644 --- a/codex-rs/core/src/tools/context.rs +++ b/codex-rs/core/src/tools/context.rs @@ -28,6 +28,13 @@ use tokio_util::sync::CancellationToken; pub use codex_tools::ToolOutput; pub use codex_tools::ToolPayload; +pub(crate) fn boxed_tool_output(output: T) -> Box +where + T: ToolOutput + 'static, +{ + Box::new(output) +} + pub type SharedTurnDiffTracker = Arc>; #[derive(Clone, Debug, Eq, PartialEq)] @@ -91,6 +98,10 @@ impl ToolOutput for McpToolOutput { }) } + fn post_tool_use_input(&self, _payload: &ToolPayload) -> Option { + Some(self.tool_input.clone()) + } + fn post_tool_use_response(&self, _call_id: &str, _payload: &ToolPayload) -> Option { serde_json::to_value(&self.result).ok() } @@ -327,6 +338,20 @@ impl ToolOutput for ExecCommandToolOutput { ) } + fn post_tool_use_id(&self, call_id: &str) -> String { + if self.event_call_id.is_empty() { + call_id.to_string() + } else { + self.event_call_id.clone() + } + } + + fn post_tool_use_input(&self, _payload: &ToolPayload) -> Option { + self.hook_command + .as_ref() + .map(|command| serde_json::json!({ "command": command })) + } + fn post_tool_use_response(&self, _call_id: &str, _payload: &ToolPayload) -> Option { if self.process_id.is_some() || self.hook_command.is_none() { return None; diff --git a/codex-rs/core/src/tools/handlers/agent_jobs/report_agent_job_result.rs b/codex-rs/core/src/tools/handlers/agent_jobs/report_agent_job_result.rs index e08eb4b5fb..4641e793c8 100644 --- a/codex-rs/core/src/tools/handlers/agent_jobs/report_agent_job_result.rs +++ b/codex-rs/core/src/tools/handlers/agent_jobs/report_agent_job_result.rs @@ -2,9 +2,10 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::agent_jobs_spec::create_report_agent_job_result_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -14,8 +15,6 @@ pub struct ReportAgentJobResultHandler; #[async_trait::async_trait] impl ToolExecutor for ReportAgentJobResultHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("report_agent_job_result") } @@ -24,7 +23,10 @@ impl ToolExecutor for ReportAgentJobResultHandler { Some(create_report_agent_job_result_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, payload, .. } = invocation; @@ -38,11 +40,11 @@ impl ToolExecutor for ReportAgentJobResultHandler { } }; - handle(session, arguments).await + handle(session, arguments).await.map(boxed_tool_output) } } -impl ToolHandler for ReportAgentJobResultHandler { +impl CoreToolRuntime for ReportAgentJobResultHandler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/agent_jobs/spawn_agents_on_csv.rs b/codex-rs/core/src/tools/handlers/agent_jobs/spawn_agents_on_csv.rs index a384f2d8cc..0e51390e79 100644 --- a/codex-rs/core/src/tools/handlers/agent_jobs/spawn_agents_on_csv.rs +++ b/codex-rs/core/src/tools/handlers/agent_jobs/spawn_agents_on_csv.rs @@ -2,9 +2,10 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::agent_jobs_spec::create_spawn_agents_on_csv_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_tools::ToolName; use codex_tools::ToolSpec; use codex_utils_absolute_path::AbsolutePathBuf; @@ -15,8 +16,6 @@ pub struct SpawnAgentsOnCsvHandler; #[async_trait::async_trait] impl ToolExecutor for SpawnAgentsOnCsvHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("spawn_agents_on_csv") } @@ -25,7 +24,10 @@ impl ToolExecutor for SpawnAgentsOnCsvHandler { Some(create_spawn_agents_on_csv_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -42,11 +44,13 @@ impl ToolExecutor for SpawnAgentsOnCsvHandler { } }; - handle(session, turn, arguments).await + handle(session, turn, arguments) + .await + .map(boxed_tool_output) } } -impl ToolHandler for SpawnAgentsOnCsvHandler { +impl CoreToolRuntime for SpawnAgentsOnCsvHandler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index c8e2461e03..500751137b 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -17,8 +17,8 @@ use crate::tools::context::ApplyPatchToolOutput; use crate::tools::context::FunctionToolOutput; use crate::tools::context::SharedTurnDiffTracker; use crate::tools::context::ToolInvocation; -use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::events::ToolEmitter; use crate::tools::events::ToolEventCtx; use crate::tools::handlers::apply_granted_turn_permissions; @@ -27,11 +27,11 @@ use crate::tools::handlers::resolve_tool_environment; use crate::tools::handlers::updated_hook_command; use crate::tools::hook_names::HookToolName; use crate::tools::orchestrator::ToolOrchestrator; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolArgumentDiffConsumer; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use crate::tools::runtimes::apply_patch::ApplyPatchRequest; use crate::tools::runtimes::apply_patch::ApplyPatchRuntime; use crate::tools::sandboxing::ToolCtx; @@ -299,8 +299,6 @@ async fn effective_patch_permissions( #[async_trait::async_trait] impl ToolExecutor for ApplyPatchHandler { - type Output = ApplyPatchToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("apply_patch") } @@ -309,7 +307,10 @@ impl ToolExecutor for ApplyPatchHandler { Some(create_apply_patch_freeform_tool(self.multi_environment)) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -359,7 +360,7 @@ impl ToolExecutor for ApplyPatchHandler { { InternalApplyPatchInvocation::Output(item) => { let content = item?; - Ok(ApplyPatchToolOutput::from_text(content)) + Ok(boxed_tool_output(ApplyPatchToolOutput::from_text(content))) } InternalApplyPatchInvocation::DelegateToRuntime(apply) => { let changes = convert_apply_patch_to_protocol(&apply.action); @@ -414,7 +415,7 @@ impl ToolExecutor for ApplyPatchHandler { Some(&tracker), ); let content = emitter.finish(event_ctx, out, delta.as_ref()).await?; - Ok(ApplyPatchToolOutput::from_text(content)) + Ok(boxed_tool_output(ApplyPatchToolOutput::from_text(content))) } } } @@ -438,7 +439,7 @@ impl ToolExecutor for ApplyPatchHandler { } } -impl ToolHandler for ApplyPatchHandler { +impl CoreToolRuntime for ApplyPatchHandler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Custom { .. }) } @@ -472,7 +473,7 @@ impl ToolHandler for ApplyPatchHandler { fn post_tool_use_payload( &self, invocation: &ToolInvocation, - result: &Self::Output, + result: &dyn crate::tools::context::ToolOutput, ) -> Option { let tool_response = result.post_tool_use_response(&invocation.call_id, &invocation.payload)?; diff --git a/codex-rs/core/src/tools/handlers/dynamic.rs b/codex-rs/core/src/tools/handlers/dynamic.rs index 29dc5199e3..5d20128ff3 100644 --- a/codex-rs/core/src/tools/handlers/dynamic.rs +++ b/codex-rs/core/src/tools/handlers/dynamic.rs @@ -4,10 +4,11 @@ use crate::session::turn_context::TurnContext; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; use crate::tools::registry::ToolExposure; -use crate::tools::registry::ToolHandler; use crate::tools::tool_search_entry::ToolSearchInfo; use crate::turn_timing::now_unix_timestamp_ms; use codex_protocol::dynamic_tools::DynamicToolCallRequest; @@ -62,8 +63,6 @@ impl DynamicToolHandler { #[async_trait::async_trait] impl ToolExecutor for DynamicToolHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { self.tool_name.clone() } @@ -76,7 +75,10 @@ impl ToolExecutor for DynamicToolHandler { self.exposure } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -117,11 +119,14 @@ impl ToolExecutor for DynamicToolHandler { .into_iter() .map(FunctionCallOutputContentItem::from) .collect::>(); - Ok(FunctionToolOutput::from_content(body, Some(success))) + Ok(boxed_tool_output(FunctionToolOutput::from_content( + body, + Some(success), + ))) } } -impl ToolHandler for DynamicToolHandler { +impl CoreToolRuntime for DynamicToolHandler { fn search_info(&self) -> Option { ToolSearchInfo::from_spec( self.search_text.clone(), diff --git a/codex-rs/core/src/tools/handlers/extension_tools.rs b/codex-rs/core/src/tools/handlers/extension_tools.rs index c4018fe936..37ad2d65fd 100644 --- a/codex-rs/core/src/tools/handlers/extension_tools.rs +++ b/codex-rs/core/src/tools/handlers/extension_tools.rs @@ -1,7 +1,5 @@ use std::sync::Arc; -use codex_extension_api::ExtensionToolExecutor; -use codex_extension_api::ExtensionToolOutput; use codex_tools::ToolCall as ExtensionToolCall; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -13,18 +11,16 @@ use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; use crate::tools::flat_tool_name; use crate::tools::hook_names::HookToolName; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; -pub(crate) struct ExtensionToolHandler { - executor: Arc, -} +pub(crate) struct ExtensionToolAdapter(Arc>); -impl ExtensionToolHandler { - pub(crate) fn new(executor: Arc) -> Self { - Self { executor } +impl ExtensionToolAdapter { + pub(crate) fn new(executor: Arc>) -> Self { + Self(executor) } fn arguments_from_payload<'a>(&self, payload: &'a ToolPayload) -> Option<&'a str> { @@ -36,23 +32,32 @@ impl ExtensionToolHandler { } #[async_trait::async_trait] -impl ToolExecutor for ExtensionToolHandler { - type Output = ExtensionToolOutput; - +impl ToolExecutor for ExtensionToolAdapter { fn tool_name(&self) -> ToolName { - self.executor.tool_name() + self.0.tool_name() } fn spec(&self) -> Option { - self.executor.spec() + self.0.spec() } - async fn handle(&self, invocation: ToolInvocation) -> Result { - self.executor.handle(to_extension_call(&invocation)).await + fn exposure(&self) -> crate::tools::registry::ToolExposure { + self.0.exposure() + } + + fn supports_parallel_tool_calls(&self) -> bool { + self.0.supports_parallel_tool_calls() + } + + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { + self.0.handle(to_extension_call(&invocation)).await } } -impl ToolHandler for ExtensionToolHandler { +impl CoreToolRuntime for ExtensionToolAdapter { fn matches_kind(&self, payload: &ToolPayload) -> bool { self.arguments_from_payload(payload).is_some() } @@ -68,7 +73,7 @@ impl ToolHandler for ExtensionToolHandler { fn post_tool_use_payload( &self, invocation: &ToolInvocation, - result: &Self::Output, + result: &dyn ToolOutput, ) -> Option { let arguments = self.arguments_from_payload(&invocation.payload)?; Some(PostToolUsePayload { @@ -104,22 +109,20 @@ mod tests { use pretty_assertions::assert_eq; use serde_json::json; - use super::ExtensionToolHandler; + use super::ExtensionToolAdapter; use crate::tools::context::ToolCallSource; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::hook_names::HookToolName; + use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; - use crate::tools::registry::ToolHandler; use crate::turn_diff_tracker::TurnDiffTracker; struct StubExtensionExecutor; #[async_trait::async_trait] impl codex_extension_api::ToolExecutor for StubExtensionExecutor { - type Output = codex_tools::JsonToolOutput; - fn tool_name(&self) -> codex_tools::ToolName { codex_tools::ToolName::plain("extension_echo") } @@ -148,14 +151,16 @@ mod tests { async fn handle( &self, _call: codex_tools::ToolCall, - ) -> Result { - Ok(codex_tools::JsonToolOutput::new(json!({ "ok": true }))) + ) -> Result, codex_tools::FunctionCallError> { + Ok(Box::new(codex_tools::JsonToolOutput::new( + json!({ "ok": true }), + ))) } } #[tokio::test] async fn exposes_generic_hook_payloads() { - let handler = ExtensionToolHandler::new(Arc::new(StubExtensionExecutor)); + let handler = ExtensionToolAdapter::new(Arc::new(StubExtensionExecutor)); let (session, turn) = crate::session::tests::make_session_and_context().await; let invocation = ToolInvocation { session: session.into(), @@ -172,14 +177,14 @@ mod tests { let output = codex_tools::JsonToolOutput::new(json!({ "ok": true })); assert_eq!( - ToolHandler::pre_tool_use_payload(&handler, &invocation), + CoreToolRuntime::pre_tool_use_payload(&handler, &invocation), Some(PreToolUsePayload { tool_name: HookToolName::new("extension_echo"), tool_input: json!({ "message": "hello" }), }) ); assert_eq!( - ToolHandler::post_tool_use_payload(&handler, &invocation, &output), + CoreToolRuntime::post_tool_use_payload(&handler, &invocation, &output), Some(PostToolUsePayload { tool_name: HookToolName::new("extension_echo"), tool_use_id: "call-extension".to_string(), diff --git a/codex-rs/core/src/tools/handlers/goal/create_goal.rs b/codex-rs/core/src/tools/handlers/goal/create_goal.rs index f7575e1e52..9ae0107f34 100644 --- a/codex-rs/core/src/tools/handlers/goal/create_goal.rs +++ b/codex-rs/core/src/tools/handlers/goal/create_goal.rs @@ -1,13 +1,13 @@ use crate::function_tool::FunctionCallError; use crate::goals::CreateGoalRequest; -use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::goal_spec::CREATE_GOAL_TOOL_NAME; use crate::tools::handlers::goal_spec::create_create_goal_tool; use crate::tools::handlers::parse_arguments; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -20,8 +20,6 @@ pub struct CreateGoalHandler; #[async_trait::async_trait] impl ToolExecutor for CreateGoalHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain(CREATE_GOAL_TOOL_NAME) } @@ -30,7 +28,10 @@ impl ToolExecutor for CreateGoalHandler { Some(create_create_goal_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -70,8 +71,8 @@ impl ToolExecutor for CreateGoalHandler { FunctionCallError::RespondToModel(format_goal_error(err)) } })?; - goal_response(Some(goal), CompletionBudgetReport::Omit) + goal_response(Some(goal), CompletionBudgetReport::Omit).map(boxed_tool_output) } } -impl ToolHandler for CreateGoalHandler {} +impl CoreToolRuntime for CreateGoalHandler {} diff --git a/codex-rs/core/src/tools/handlers/goal/get_goal.rs b/codex-rs/core/src/tools/handlers/goal/get_goal.rs index 20691b8b5b..ff40227fab 100644 --- a/codex-rs/core/src/tools/handlers/goal/get_goal.rs +++ b/codex-rs/core/src/tools/handlers/goal/get_goal.rs @@ -1,11 +1,11 @@ use crate::function_tool::FunctionCallError; -use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::goal_spec::GET_GOAL_TOOL_NAME; use crate::tools::handlers::goal_spec::create_get_goal_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -17,8 +17,6 @@ pub struct GetGoalHandler; #[async_trait::async_trait] impl ToolExecutor for GetGoalHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain(GET_GOAL_TOOL_NAME) } @@ -27,7 +25,10 @@ impl ToolExecutor for GetGoalHandler { Some(create_get_goal_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, payload, .. } = invocation; @@ -38,7 +39,7 @@ impl ToolExecutor for GetGoalHandler { .get_thread_goal() .await .map_err(|err| FunctionCallError::RespondToModel(format_goal_error(err)))?; - goal_response(goal, CompletionBudgetReport::Omit) + goal_response(goal, CompletionBudgetReport::Omit).map(boxed_tool_output) } _ => Err(FunctionCallError::RespondToModel( "get_goal handler received unsupported payload".to_string(), @@ -47,4 +48,4 @@ impl ToolExecutor for GetGoalHandler { } } -impl ToolHandler for GetGoalHandler {} +impl CoreToolRuntime for GetGoalHandler {} diff --git a/codex-rs/core/src/tools/handlers/goal/update_goal.rs b/codex-rs/core/src/tools/handlers/goal/update_goal.rs index bb8ed10efd..8cf8ce546a 100644 --- a/codex-rs/core/src/tools/handlers/goal/update_goal.rs +++ b/codex-rs/core/src/tools/handlers/goal/update_goal.rs @@ -1,14 +1,14 @@ use crate::function_tool::FunctionCallError; use crate::goals::GoalRuntimeEvent; use crate::goals::SetGoalRequest; -use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::goal_spec::UPDATE_GOAL_TOOL_NAME; use crate::tools::handlers::goal_spec::create_update_goal_tool; use crate::tools::handlers::parse_arguments; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_protocol::protocol::ThreadGoalStatus; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -22,8 +22,6 @@ pub struct UpdateGoalHandler; #[async_trait::async_trait] impl ToolExecutor for UpdateGoalHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain(UPDATE_GOAL_TOOL_NAME) } @@ -32,7 +30,10 @@ impl ToolExecutor for UpdateGoalHandler { Some(create_update_goal_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -73,8 +74,8 @@ impl ToolExecutor for UpdateGoalHandler { ) .await .map_err(|err| FunctionCallError::RespondToModel(format_goal_error(err)))?; - goal_response(Some(goal), CompletionBudgetReport::Include) + goal_response(Some(goal), CompletionBudgetReport::Include).map(boxed_tool_output) } } -impl ToolHandler for UpdateGoalHandler {} +impl CoreToolRuntime for UpdateGoalHandler {} diff --git a/codex-rs/core/src/tools/handlers/mcp.rs b/codex-rs/core/src/tools/handlers/mcp.rs index f82f741346..6f4f79505b 100644 --- a/codex-rs/core/src/tools/handlers/mcp.rs +++ b/codex-rs/core/src/tools/handlers/mcp.rs @@ -6,15 +6,15 @@ use crate::mcp_tool_call::handle_mcp_tool_call; use crate::original_image_detail::can_request_original_image_detail; use crate::tools::context::McpToolOutput; use crate::tools::context::ToolInvocation; -use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::flat_tool_name; use crate::tools::hook_names::HookToolName; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolExecutor; use crate::tools::registry::ToolExposure; -use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolTelemetryTags; use crate::tools::tool_search_entry::ToolSearchInfo; use codex_mcp::ToolInfo; @@ -47,8 +47,6 @@ impl McpHandler { #[async_trait::async_trait] impl ToolExecutor for McpHandler { - type Output = McpToolOutput; - fn tool_name(&self) -> ToolName { self.tool_info.canonical_tool_name() } @@ -89,7 +87,10 @@ impl ToolExecutor for McpHandler { self.tool_info.supports_parallel_tool_calls } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -119,17 +120,17 @@ impl ToolExecutor for McpHandler { ) .await; - Ok(McpToolOutput { + Ok(boxed_tool_output(McpToolOutput { result: result.result, tool_input: result.tool_input, wall_time: started.elapsed(), original_image_detail_supported: can_request_original_image_detail(&turn.model_info), truncation_policy: turn.truncation_policy, - }) + })) } } -impl ToolHandler for McpHandler { +impl CoreToolRuntime for McpHandler { fn search_info(&self) -> Option { let source_name = self .tool_info @@ -156,12 +157,17 @@ impl ToolHandler for McpHandler { ) } - async fn telemetry_tags(&self, _invocation: &ToolInvocation) -> ToolTelemetryTags { - let mut tags = vec![("mcp_server", self.tool_info.server_name.clone())]; - if let Some(origin) = &self.tool_info.server_origin { - tags.push(("mcp_server_origin", origin.clone())); - } - tags + fn telemetry_tags<'a>( + &'a self, + _invocation: &'a ToolInvocation, + ) -> futures::future::BoxFuture<'a, ToolTelemetryTags> { + Box::pin(async { + let mut tags = vec![("mcp_server", self.tool_info.server_name.clone())]; + if let Some(origin) = &self.tool_info.server_origin { + tags.push(("mcp_server_origin", origin.clone())); + } + tags + }) } fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { @@ -200,7 +206,7 @@ impl ToolHandler for McpHandler { fn post_tool_use_payload( &self, invocation: &ToolInvocation, - result: &Self::Output, + result: &dyn crate::tools::context::ToolOutput, ) -> Option { let ToolPayload::Function { .. } = &invocation.payload else { return None; @@ -211,7 +217,7 @@ impl ToolHandler for McpHandler { Some(PostToolUsePayload { tool_name: HookToolName::new(self.tool_name().to_string()), tool_use_id: invocation.call_id.clone(), - tool_input: result.tool_input.clone(), + tool_input: result.post_tool_use_input(&invocation.payload)?, tool_response, }) } diff --git a/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resource_templates.rs b/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resource_templates.rs index 71749e9cd1..866c7a9e12 100644 --- a/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resource_templates.rs +++ b/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resource_templates.rs @@ -1,12 +1,12 @@ use std::time::Instant; use crate::function_tool::FunctionCallError; -use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::mcp_resource_spec::create_list_mcp_resource_templates_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_protocol::models::function_call_output_content_items_to_text; use codex_protocol::protocol::McpInvocation; use codex_tools::ToolName; @@ -28,8 +28,6 @@ pub struct ListMcpResourceTemplatesHandler; #[async_trait::async_trait] impl ToolExecutor for ListMcpResourceTemplatesHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("list_mcp_resource_templates") } @@ -46,7 +44,10 @@ impl ToolExecutor for ListMcpResourceTemplatesHandler { clippy::await_holding_invalid_type, reason = "MCP resource template listing reads through the session-owned manager guard" )] - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -131,7 +132,7 @@ impl ToolExecutor for ListMcpResourceTemplatesHandler { Ok(call_tool_result_from_content(&content, output.success)), ) .await; - Ok(output) + Ok(boxed_tool_output(output)) } Err(err) => { let duration = start.elapsed(); @@ -166,4 +167,4 @@ impl ToolExecutor for ListMcpResourceTemplatesHandler { } } -impl ToolHandler for ListMcpResourceTemplatesHandler {} +impl CoreToolRuntime for ListMcpResourceTemplatesHandler {} diff --git a/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resources.rs b/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resources.rs index 08b387376b..c87747eea4 100644 --- a/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resources.rs +++ b/codex-rs/core/src/tools/handlers/mcp_resource/list_mcp_resources.rs @@ -1,12 +1,12 @@ use std::time::Instant; use crate::function_tool::FunctionCallError; -use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::mcp_resource_spec::create_list_mcp_resources_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_protocol::models::function_call_output_content_items_to_text; use codex_protocol::protocol::McpInvocation; use codex_tools::ToolName; @@ -28,8 +28,6 @@ pub struct ListMcpResourcesHandler; #[async_trait::async_trait] impl ToolExecutor for ListMcpResourcesHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("list_mcp_resources") } @@ -46,7 +44,10 @@ impl ToolExecutor for ListMcpResourcesHandler { clippy::await_holding_invalid_type, reason = "MCP resource listing reads through the session-owned manager guard" )] - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -129,7 +130,7 @@ impl ToolExecutor for ListMcpResourcesHandler { Ok(call_tool_result_from_content(&content, output.success)), ) .await; - Ok(output) + Ok(boxed_tool_output(output)) } Err(err) => { let duration = start.elapsed(); @@ -164,4 +165,4 @@ impl ToolExecutor for ListMcpResourcesHandler { } } -impl ToolHandler for ListMcpResourcesHandler {} +impl CoreToolRuntime for ListMcpResourcesHandler {} diff --git a/codex-rs/core/src/tools/handlers/mcp_resource/read_mcp_resource.rs b/codex-rs/core/src/tools/handlers/mcp_resource/read_mcp_resource.rs index bd8172ac74..3bb4d7f0ec 100644 --- a/codex-rs/core/src/tools/handlers/mcp_resource/read_mcp_resource.rs +++ b/codex-rs/core/src/tools/handlers/mcp_resource/read_mcp_resource.rs @@ -1,12 +1,12 @@ use std::time::Instant; use crate::function_tool::FunctionCallError; -use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::mcp_resource_spec::create_read_mcp_resource_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_protocol::models::function_call_output_content_items_to_text; use codex_protocol::protocol::McpInvocation; use codex_tools::ToolName; @@ -28,8 +28,6 @@ pub struct ReadMcpResourceHandler; #[async_trait::async_trait] impl ToolExecutor for ReadMcpResourceHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("read_mcp_resource") } @@ -42,7 +40,10 @@ impl ToolExecutor for ReadMcpResourceHandler { true } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -112,7 +113,7 @@ impl ToolExecutor for ReadMcpResourceHandler { Ok(call_tool_result_from_content(&content, output.success)), ) .await; - Ok(output) + Ok(boxed_tool_output(output)) } Err(err) => { let duration = start.elapsed(); @@ -147,4 +148,4 @@ impl ToolExecutor for ReadMcpResourceHandler { } } -impl ToolHandler for ReadMcpResourceHandler {} +impl CoreToolRuntime for ReadMcpResourceHandler {} diff --git a/codex-rs/core/src/tools/handlers/multi_agents.rs b/codex-rs/core/src/tools/handlers/multi_agents.rs index d0c5edfb77..1587c4e840 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents.rs @@ -13,10 +13,11 @@ use crate::session::turn_context::TurnContext; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; pub(crate) use crate::tools::handlers::multi_agents_common::*; use crate::tools::handlers::parse_arguments; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_protocol::ThreadId; use codex_protocol::models::ResponseInputItem; use codex_protocol::openai_models::ReasoningEffort; diff --git a/codex-rs/core/src/tools/handlers/multi_agents/close_agent.rs b/codex-rs/core/src/tools/handlers/multi_agents/close_agent.rs index 6459a899fe..805f569b8e 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/close_agent.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/close_agent.rs @@ -7,8 +7,6 @@ pub(crate) struct Handler; #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = CloseAgentResult; - fn tool_name(&self) -> ToolName { ToolName::plain("close_agent") } @@ -17,8 +15,11 @@ impl ToolExecutor for Handler { Some(create_close_agent_tool_v1()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { - handle_close_agent(invocation).await + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { + handle_close_agent(invocation).await.map(boxed_tool_output) } } @@ -105,7 +106,7 @@ async fn handle_close_agent( }) } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents/resume_agent.rs b/codex-rs/core/src/tools/handlers/multi_agents/resume_agent.rs index 9dab2d999d..71b29fb423 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/resume_agent.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/resume_agent.rs @@ -9,8 +9,6 @@ pub(crate) struct Handler; #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = ResumeAgentResult; - fn tool_name(&self) -> ToolName { ToolName::plain("resume_agent") } @@ -19,8 +17,11 @@ impl ToolExecutor for Handler { Some(create_resume_agent_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { - handle_resume_agent(invocation).await + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { + handle_resume_agent(invocation).await.map(boxed_tool_output) } } @@ -133,7 +134,7 @@ async fn handle_resume_agent( Ok(ResumeAgentResult { status }) } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents/send_input.rs b/codex-rs/core/src/tools/handlers/multi_agents/send_input.rs index a6067e5b12..74f7eb05b4 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/send_input.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/send_input.rs @@ -8,8 +8,6 @@ pub(crate) struct Handler; #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = SendInputResult; - fn tool_name(&self) -> ToolName { ToolName::plain("send_input") } @@ -18,7 +16,10 @@ impl ToolExecutor for Handler { Some(create_send_input_tool_v1()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -85,11 +86,11 @@ impl ToolExecutor for Handler { .await; let submission_id = result?; - Ok(SendInputResult { submission_id }) + Ok(boxed_tool_output(SendInputResult { submission_id })) } } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs index 91f5990210..d19849d443 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs @@ -24,8 +24,6 @@ impl Handler { #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = SpawnAgentResult; - fn tool_name(&self) -> ToolName { ToolName::plain("spawn_agent") } @@ -34,8 +32,11 @@ impl ToolExecutor for Handler { Some(create_spawn_agent_tool_v1(self.options.clone())) } - async fn handle(&self, invocation: ToolInvocation) -> Result { - handle_spawn_agent(invocation).await + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { + handle_spawn_agent(invocation).await.map(boxed_tool_output) } } @@ -198,7 +199,7 @@ async fn handle_spawn_agent( }) } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents/wait.rs b/codex-rs/core/src/tools/handlers/multi_agents/wait.rs index d325edeeb0..a69784b901 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/wait.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/wait.rs @@ -29,8 +29,6 @@ impl Handler { #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = WaitAgentResult; - fn tool_name(&self) -> ToolName { ToolName::plain("wait_agent") } @@ -39,7 +37,10 @@ impl ToolExecutor for Handler { Some(create_wait_agent_tool_v1(self.options)) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -197,11 +198,11 @@ impl ToolExecutor for Handler { ) .await; - Ok(result) + Ok(boxed_tool_output(result)) } } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index c6a0007f13..0044bf8522 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -317,7 +317,8 @@ async fn spawn_agent_fork_context_rejects_agent_type_override() { })), )) .await - .expect_err("fork_context should reject agent_type overrides"); + .err() + .expect("fork_context should reject agent_type overrides"); assert_eq!( err, @@ -351,7 +352,8 @@ async fn spawn_agent_fork_context_rejects_child_model_overrides() { })), )) .await - .expect_err("forked spawn should reject child model overrides"); + .err() + .expect("forked spawn should reject child model overrides"); assert_eq!( err, @@ -395,7 +397,8 @@ async fn multi_agent_v2_spawn_fork_turns_all_rejects_agent_type_override() { })), )) .await - .expect_err("fork_turns=all should reject agent_type overrides"); + .err() + .expect("fork_turns=all should reject agent_type overrides"); assert_eq!( err, @@ -435,7 +438,8 @@ async fn multi_agent_v2_spawn_defaults_to_full_fork_and_rejects_child_model_over })), )) .await - .expect_err("default full fork should reject child model overrides"); + .err() + .expect("default full fork should reject child model overrides"); assert_eq!( err, @@ -505,7 +509,8 @@ async fn spawn_agent_service_tier_override_validates_the_effective_child_model() })), )) .await - .expect_err("unknown service tier should be rejected"); + .err() + .expect("unknown service tier should be rejected"); assert_eq!( err, @@ -530,7 +535,8 @@ async fn spawn_agent_service_tier_override_validates_the_effective_child_model() })), )) .await - .expect_err("tier unsupported by the final child model should be rejected"); + .err() + .expect("tier unsupported by the final child model should be rejected"); assert_eq!( err, @@ -1116,7 +1122,8 @@ async fn multi_agent_v2_spawn_rejects_legacy_fork_context() { })), )) .await - .expect_err("legacy fork_context should be rejected"); + .err() + .expect("legacy fork_context should be rejected"); assert_eq!( err, @@ -1155,7 +1162,8 @@ async fn multi_agent_v2_spawn_rejects_invalid_fork_turns_string() { })), )) .await - .expect_err("invalid fork_turns should be rejected"); + .err() + .expect("invalid fork_turns should be rejected"); assert_eq!( err, @@ -1194,7 +1202,8 @@ async fn multi_agent_v2_spawn_rejects_zero_fork_turns() { })), )) .await - .expect_err("zero turn count should be rejected"); + .err() + .expect("zero turn count should be rejected"); assert_eq!( err, @@ -3630,7 +3639,8 @@ async fn multi_agent_v2_close_agent_rejects_root_target_and_id() { function_payload(json!({"target": "/root"})), )) .await - .expect_err("close_agent should reject the root path"); + .err() + .expect("close_agent should reject the root path"); assert_eq!( root_path_error, FunctionCallError::RespondToModel("root is not a spawned agent".to_string()) @@ -3644,7 +3654,8 @@ async fn multi_agent_v2_close_agent_rejects_root_target_and_id() { function_payload(json!({"target": root.thread_id.to_string()})), )) .await - .expect_err("close_agent should reject the root thread id"); + .err() + .expect("close_agent should reject the root thread id"); assert_eq!( root_id_error, FunctionCallError::RespondToModel("root is not a spawned agent".to_string()) diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2.rs index 0190fe04ac..068f393379 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2.rs @@ -6,10 +6,11 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::multi_agents_common::*; use crate::tools::handlers::parse_arguments; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_protocol::AgentPath; use codex_protocol::models::ResponseInputItem; use codex_protocol::openai_models::ReasoningEffort; diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/close_agent.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/close_agent.rs index 7b575f8258..180cf1a9d8 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/close_agent.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/close_agent.rs @@ -7,8 +7,6 @@ pub(crate) struct Handler; #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = CloseAgentResult; - fn tool_name(&self) -> ToolName { ToolName::plain("close_agent") } @@ -17,8 +15,11 @@ impl ToolExecutor for Handler { Some(create_close_agent_tool_v2()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { - handle_close_agent(invocation).await + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { + handle_close_agent(invocation).await.map(boxed_tool_output) } } @@ -117,7 +118,7 @@ async fn handle_close_agent( }) } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/followup_task.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/followup_task.rs index 7d111f0418..4b3edcd487 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/followup_task.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/followup_task.rs @@ -2,7 +2,6 @@ use super::message_tool::FollowupTaskArgs; use super::message_tool::MessageDeliveryMode; use super::message_tool::handle_message_string_tool; use super::*; -use crate::tools::context::FunctionToolOutput; use crate::tools::handlers::multi_agents_spec::create_followup_task_tool; use codex_tools::ToolSpec; @@ -10,8 +9,6 @@ pub(crate) struct Handler; #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("followup_task") } @@ -20,7 +17,10 @@ impl ToolExecutor for Handler { Some(create_followup_task_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let arguments = function_arguments(invocation.payload.clone())?; let args: FollowupTaskArgs = parse_arguments(&arguments)?; handle_message_string_tool( @@ -30,10 +30,11 @@ impl ToolExecutor for Handler { args.message, ) .await + .map(boxed_tool_output) } } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/list_agents.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/list_agents.rs index 8b0ee551e3..b8b9072da9 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/list_agents.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/list_agents.rs @@ -7,8 +7,6 @@ pub(crate) struct Handler; #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = ListAgentsResult; - fn tool_name(&self) -> ToolName { ToolName::plain("list_agents") } @@ -17,7 +15,10 @@ impl ToolExecutor for Handler { Some(create_list_agents_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -37,11 +38,11 @@ impl ToolExecutor for Handler { .await .map_err(collab_spawn_error)?; - Ok(ListAgentsResult { agents }) + Ok(boxed_tool_output(ListAgentsResult { agents })) } } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/send_message.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/send_message.rs index 584feec61f..202e28e0c9 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/send_message.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/send_message.rs @@ -2,7 +2,6 @@ use super::message_tool::MessageDeliveryMode; use super::message_tool::SendMessageArgs; use super::message_tool::handle_message_string_tool; use super::*; -use crate::tools::context::FunctionToolOutput; use crate::tools::handlers::multi_agents_spec::create_send_message_tool; use codex_tools::ToolSpec; @@ -10,8 +9,6 @@ pub(crate) struct Handler; #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("send_message") } @@ -20,7 +17,10 @@ impl ToolExecutor for Handler { Some(create_send_message_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let arguments = function_arguments(invocation.payload.clone())?; let args: SendMessageArgs = parse_arguments(&arguments)?; handle_message_string_tool( @@ -30,10 +30,11 @@ impl ToolExecutor for Handler { args.message, ) .await + .map(boxed_tool_output) } } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs index 3ad7b87145..2daa1f07db 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs @@ -26,8 +26,6 @@ impl Handler { #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = SpawnAgentResult; - fn tool_name(&self) -> ToolName { ToolName::plain("spawn_agent") } @@ -36,8 +34,11 @@ impl ToolExecutor for Handler { Some(create_spawn_agent_tool_v2(self.options.clone())) } - async fn handle(&self, invocation: ToolInvocation) -> Result { - handle_spawn_agent(invocation).await + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { + handle_spawn_agent(invocation).await.map(boxed_tool_output) } } @@ -229,7 +230,7 @@ async fn handle_spawn_agent( } } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/wait.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/wait.rs index 53c976c9d4..8913411b53 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/wait.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/wait.rs @@ -21,8 +21,6 @@ impl Handler { #[async_trait::async_trait] impl ToolExecutor for Handler { - type Output = WaitAgentResult; - fn tool_name(&self) -> ToolName { ToolName::plain("wait_agent") } @@ -31,7 +29,10 @@ impl ToolExecutor for Handler { Some(create_wait_agent_tool_v2(self.options)) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -97,11 +98,11 @@ impl ToolExecutor for Handler { ) .await; - Ok(result) + Ok(boxed_tool_output(result)) } } -impl ToolHandler for Handler { +impl CoreToolRuntime for Handler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } diff --git a/codex-rs/core/src/tools/handlers/plan.rs b/codex-rs/core/src/tools/handlers/plan.rs index 9868c77ca7..f5c94826f7 100644 --- a/codex-rs/core/src/tools/handlers/plan.rs +++ b/codex-rs/core/src/tools/handlers/plan.rs @@ -2,9 +2,10 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::plan_spec::create_update_plan_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_protocol::config_types::ModeKind; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseInputItem; @@ -46,8 +47,6 @@ impl ToolOutput for PlanToolOutput { #[async_trait::async_trait] impl ToolExecutor for PlanHandler { - type Output = PlanToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("update_plan") } @@ -56,7 +55,10 @@ impl ToolExecutor for PlanHandler { Some(create_update_plan_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -85,11 +87,11 @@ impl ToolExecutor for PlanHandler { .send_event(turn.as_ref(), EventMsg::PlanUpdate(args)) .await; - Ok(PlanToolOutput) + Ok(boxed_tool_output(PlanToolOutput)) } } -impl ToolHandler for PlanHandler {} +impl CoreToolRuntime for PlanHandler {} fn parse_update_plan_arguments(arguments: &str) -> Result { serde_json::from_str::(arguments).map_err(|e| { diff --git a/codex-rs/core/src/tools/handlers/request_permissions.rs b/codex-rs/core/src/tools/handlers/request_permissions.rs index 2f2e71677e..007c8d2208 100644 --- a/codex-rs/core/src/tools/handlers/request_permissions.rs +++ b/codex-rs/core/src/tools/handlers/request_permissions.rs @@ -5,11 +5,12 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments_with_base_path; use crate::tools::handlers::shell_spec::create_request_permissions_tool; use crate::tools::handlers::shell_spec::request_permissions_tool_description; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -17,8 +18,6 @@ pub struct RequestPermissionsHandler; #[async_trait::async_trait] impl ToolExecutor for RequestPermissionsHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("request_permissions") } @@ -29,7 +28,10 @@ impl ToolExecutor for RequestPermissionsHandler { )) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -75,8 +77,11 @@ impl ToolExecutor for RequestPermissionsHandler { )) })?; - Ok(FunctionToolOutput::from_text(content, Some(true))) + Ok(boxed_tool_output(FunctionToolOutput::from_text( + content, + Some(true), + ))) } } -impl ToolHandler for RequestPermissionsHandler {} +impl CoreToolRuntime for RequestPermissionsHandler {} diff --git a/codex-rs/core/src/tools/handlers/request_plugin_install.rs b/codex-rs/core/src/tools/handlers/request_plugin_install.rs index 6073831e02..3b53fd007a 100644 --- a/codex-rs/core/src/tools/handlers/request_plugin_install.rs +++ b/codex-rs/core/src/tools/handlers/request_plugin_install.rs @@ -32,10 +32,11 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments; use crate::tools::handlers::request_plugin_install_spec::create_request_plugin_install_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; #[derive(Default)] pub struct RequestPluginInstallHandler { @@ -52,8 +53,6 @@ impl RequestPluginInstallHandler { #[async_trait::async_trait] impl ToolExecutor for RequestPluginInstallHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain(REQUEST_PLUGIN_INSTALL_TOOL_NAME) } @@ -70,7 +69,10 @@ impl ToolExecutor for RequestPluginInstallHandler { clippy::await_holding_invalid_type, reason = "plugin install discovery reads through the session-owned manager guard" )] - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { payload, session, @@ -190,11 +192,14 @@ impl ToolExecutor for RequestPluginInstallHandler { )) })?; - Ok(FunctionToolOutput::from_text(content, Some(true))) + Ok(boxed_tool_output(FunctionToolOutput::from_text( + content, + Some(true), + ))) } } -impl ToolHandler for RequestPluginInstallHandler {} +impl CoreToolRuntime for RequestPluginInstallHandler {} async fn maybe_persist_disabled_install_request( session: &crate::session::session::Session, diff --git a/codex-rs/core/src/tools/handlers/request_user_input.rs b/codex-rs/core/src/tools/handlers/request_user_input.rs index 7597cb6004..8384e02637 100644 --- a/codex-rs/core/src/tools/handlers/request_user_input.rs +++ b/codex-rs/core/src/tools/handlers/request_user_input.rs @@ -2,14 +2,15 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments; use crate::tools::handlers::request_user_input_spec::REQUEST_USER_INPUT_TOOL_NAME; use crate::tools::handlers::request_user_input_spec::create_request_user_input_tool; use crate::tools::handlers::request_user_input_spec::normalize_request_user_input_args; use crate::tools::handlers::request_user_input_spec::request_user_input_tool_description; use crate::tools::handlers::request_user_input_spec::request_user_input_unavailable_message; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_protocol::config_types::ModeKind; use codex_protocol::request_user_input::RequestUserInputArgs; use codex_tools::ToolName; @@ -21,8 +22,6 @@ pub struct RequestUserInputHandler { #[async_trait::async_trait] impl ToolExecutor for RequestUserInputHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain(REQUEST_USER_INPUT_TOOL_NAME) } @@ -33,7 +32,10 @@ impl ToolExecutor for RequestUserInputHandler { )) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -80,11 +82,14 @@ impl ToolExecutor for RequestUserInputHandler { )) })?; - Ok(FunctionToolOutput::from_text(content, Some(true))) + Ok(boxed_tool_output(FunctionToolOutput::from_text( + content, + Some(true), + ))) } } -impl ToolHandler for RequestUserInputHandler {} +impl CoreToolRuntime for RequestUserInputHandler {} #[cfg(test)] #[path = "request_user_input_tests.rs"] diff --git a/codex-rs/core/src/tools/handlers/shell/shell_command.rs b/codex-rs/core/src/tools/handlers/shell/shell_command.rs index 54d74f13b7..53156c5cfa 100644 --- a/codex-rs/core/src/tools/handlers/shell/shell_command.rs +++ b/codex-rs/core/src/tools/handlers/shell/shell_command.rs @@ -10,19 +10,18 @@ use crate::function_tool::FunctionCallError; use crate::maybe_emit_implicit_skill_invocation; use crate::session::turn_context::TurnContext; use crate::shell::Shell; -use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; -use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments_with_base_path; use crate::tools::handlers::resolve_workdir_base_path; use crate::tools::handlers::rewrite_function_string_argument; use crate::tools::handlers::updated_hook_command; use crate::tools::hook_names::HookToolName; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use crate::tools::runtimes::shell::ShellRuntimeBackend; use codex_tools::ToolSpec; @@ -129,8 +128,6 @@ impl From for ShellCommandHandler { #[async_trait::async_trait] impl ToolExecutor for ShellCommandHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("shell_command") } @@ -148,7 +145,10 @@ impl ToolExecutor for ShellCommandHandler { self.options.is_some() } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -201,10 +201,11 @@ impl ToolExecutor for ShellCommandHandler { shell_runtime_backend: self.shell_runtime_backend(), }) .await + .map(boxed_tool_output) } } -impl ToolHandler for ShellCommandHandler { +impl CoreToolRuntime for ShellCommandHandler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } @@ -240,7 +241,7 @@ impl ToolHandler for ShellCommandHandler { fn post_tool_use_payload( &self, invocation: &ToolInvocation, - result: &Self::Output, + result: &dyn crate::tools::context::ToolOutput, ) -> Option { let tool_response = result.post_tool_use_response(&invocation.call_id, &invocation.payload)?; diff --git a/codex-rs/core/src/tools/handlers/shell_tests.rs b/codex-rs/core/src/tools/handlers/shell_tests.rs index 660bac6789..eda23533a3 100644 --- a/codex-rs/core/src/tools/handlers/shell_tests.rs +++ b/codex-rs/core/src/tools/handlers/shell_tests.rs @@ -18,7 +18,7 @@ use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::handlers::ShellCommandHandler; use crate::tools::hook_names::HookToolName; -use crate::tools::registry::ToolHandler; +use crate::tools::registry::CoreToolRuntime; use crate::turn_diff_tracker::TurnDiffTracker; use codex_shell_command::is_safe_command::is_known_safe_command; use codex_shell_command::powershell::try_find_powershell_executable_blocking; diff --git a/codex-rs/core/src/tools/handlers/test_sync.rs b/codex-rs/core/src/tools/handlers/test_sync.rs index feee6470a3..d3c95dea7e 100644 --- a/codex-rs/core/src/tools/handlers/test_sync.rs +++ b/codex-rs/core/src/tools/handlers/test_sync.rs @@ -12,10 +12,11 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments; use crate::tools::handlers::test_sync_spec::create_test_sync_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -58,8 +59,6 @@ fn barrier_map() -> &'static tokio::sync::Mutex> { #[async_trait::async_trait] impl ToolExecutor for TestSyncHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("test_sync_tool") } @@ -72,7 +71,10 @@ impl ToolExecutor for TestSyncHandler { true } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { payload, .. } = invocation; let arguments = match payload { @@ -102,11 +104,14 @@ impl ToolExecutor for TestSyncHandler { sleep(Duration::from_millis(delay)).await; } - Ok(FunctionToolOutput::from_text("ok".to_string(), Some(true))) + Ok(boxed_tool_output(FunctionToolOutput::from_text( + "ok".to_string(), + Some(true), + ))) } } -impl ToolHandler for TestSyncHandler {} +impl CoreToolRuntime for TestSyncHandler {} async fn wait_on_barrier(args: BarrierArgs) -> Result<(), FunctionCallError> { if args.participants == 0 { diff --git a/codex-rs/core/src/tools/handlers/tool_search.rs b/codex-rs/core/src/tools/handlers/tool_search.rs index 579826ac04..ffb25cb880 100644 --- a/codex-rs/core/src/tools/handlers/tool_search.rs +++ b/codex-rs/core/src/tools/handlers/tool_search.rs @@ -2,9 +2,10 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::context::ToolSearchOutput; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::tool_search_spec::create_tool_search_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use crate::tools::tool_search_entry::ToolSearchEntry; use crate::tools::tool_search_entry::ToolSearchInfo; use bm25::Document; @@ -54,8 +55,6 @@ impl ToolSearchHandler { #[async_trait::async_trait] impl ToolExecutor for ToolSearchHandler { - type Output = ToolSearchOutput; - fn tool_name(&self) -> ToolName { ToolName::plain(TOOL_SEARCH_TOOL_NAME) } @@ -74,7 +73,7 @@ impl ToolExecutor for ToolSearchHandler { async fn handle( &self, invocation: ToolInvocation, - ) -> Result { + ) -> Result, FunctionCallError> { let ToolInvocation { payload, .. } = invocation; let args = match payload { @@ -101,16 +100,16 @@ impl ToolExecutor for ToolSearchHandler { } if self.entries.is_empty() { - return Ok(ToolSearchOutput { tools: Vec::new() }); + return Ok(boxed_tool_output(ToolSearchOutput { tools: Vec::new() })); } let tools = self.search(query, limit)?; - Ok(ToolSearchOutput { tools }) + Ok(boxed_tool_output(ToolSearchOutput { tools })) } } -impl ToolHandler for ToolSearchHandler {} +impl CoreToolRuntime for ToolSearchHandler {} impl ToolSearchHandler { fn search( diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 22e0a16dd8..b4e7bd6acf 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -2,7 +2,6 @@ use crate::sandboxing::SandboxPermissions; use crate::shell::Shell; use crate::shell::ShellType; use crate::shell::get_shell_by_model_provided_path; -use crate::tools::context::ExecCommandToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; @@ -88,23 +87,19 @@ pub(crate) struct ResolvedCommand { fn post_unified_exec_tool_use_payload( invocation: &ToolInvocation, - result: &ExecCommandToolOutput, + result: &dyn ToolOutput, ) -> Option { let ToolPayload::Function { .. } = &invocation.payload else { return None; }; - let command = result.hook_command.clone()?; - let tool_use_id = if result.event_call_id.is_empty() { - invocation.call_id.clone() - } else { - result.event_call_id.clone() - }; + let tool_input = result.post_tool_use_input(&invocation.payload)?; + let tool_use_id = result.post_tool_use_id(&invocation.call_id); let tool_response = result.post_tool_use_response(&tool_use_id, &invocation.payload)?; Some(PostToolUsePayload { tool_name: HookToolName::bash(), tool_use_id, - tool_input: serde_json::json!({ "command": command }), + tool_input, tool_response, }) } diff --git a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs index 2df924b5f4..b048227cf1 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs @@ -5,6 +5,7 @@ use crate::maybe_emit_implicit_skill_invocation; use crate::tools::context::ExecCommandToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::apply_granted_turn_permissions; use crate::tools::handlers::apply_patch::intercept_apply_patch; use crate::tools::handlers::implicit_granted_permissions; @@ -15,10 +16,10 @@ use crate::tools::handlers::resolve_tool_environment; use crate::tools::handlers::rewrite_function_string_argument; use crate::tools::handlers::updated_hook_command; use crate::tools::hook_names::HookToolName; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use crate::unified_exec::ExecCommandRequest; use crate::unified_exec::UnifiedExecContext; use crate::unified_exec::UnifiedExecError; @@ -70,8 +71,6 @@ impl ExecCommandHandler { #[async_trait::async_trait] impl ToolExecutor for ExecCommandHandler { - type Output = ExecCommandToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("exec_command") } @@ -90,7 +89,10 @@ impl ToolExecutor for ExecCommandHandler { true } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -234,7 +236,7 @@ impl ToolExecutor for ExecCommandHandler { .await? { manager.release_process_id(process_id).await; - return Ok(ExecCommandToolOutput { + return Ok(boxed_tool_output(ExecCommandToolOutput { event_call_id: String::new(), chunk_id: String::new(), wall_time: std::time::Duration::ZERO, @@ -244,7 +246,7 @@ impl ToolExecutor for ExecCommandHandler { exit_code: None, original_token_count: None, hook_command: None, - }); + })); } emit_unified_exec_tty_metric(&turn.session_telemetry, tty); @@ -273,11 +275,11 @@ impl ToolExecutor for ExecCommandHandler { ) .await { - Ok(response) => Ok(response), + Ok(response) => Ok(boxed_tool_output(response)), Err(UnifiedExecError::SandboxDenied { output, .. }) => { let output_text = output.aggregated_output.text; let original_token_count = approx_token_count(&output_text); - Ok(ExecCommandToolOutput { + Ok(boxed_tool_output(ExecCommandToolOutput { event_call_id: context.call_id.clone(), chunk_id: generate_chunk_id(), wall_time: output.duration, @@ -289,7 +291,7 @@ impl ToolExecutor for ExecCommandHandler { exit_code: Some(output.exit_code), original_token_count: Some(original_token_count), hook_command: Some(hook_command), - }) + })) } Err(err) => Err(FunctionCallError::RespondToModel(format!( "exec_command failed for `{command_for_display}`: {err:?}" @@ -298,7 +300,7 @@ impl ToolExecutor for ExecCommandHandler { } } -impl ToolHandler for ExecCommandHandler { +impl CoreToolRuntime for ExecCommandHandler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } @@ -340,7 +342,7 @@ impl ToolHandler for ExecCommandHandler { fn post_tool_use_payload( &self, invocation: &ToolInvocation, - result: &Self::Output, + result: &dyn crate::tools::context::ToolOutput, ) -> Option { post_unified_exec_tool_use_payload(invocation, result) } diff --git a/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs b/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs index 29ee4b4ea3..3dea7092f5 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs @@ -1,11 +1,11 @@ use crate::function_tool::FunctionCallError; -use crate::tools::context::ExecCommandToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use crate::unified_exec::WriteStdinRequest; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::TerminalInteractionEvent; @@ -33,8 +33,6 @@ pub struct WriteStdinHandler; #[async_trait::async_trait] impl ToolExecutor for WriteStdinHandler { - type Output = ExecCommandToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("write_stdin") } @@ -43,7 +41,10 @@ impl ToolExecutor for WriteStdinHandler { Some(create_write_stdin_tool()) } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { let ToolInvocation { session, turn, @@ -86,11 +87,11 @@ impl ToolExecutor for WriteStdinHandler { .send_event(turn.as_ref(), EventMsg::TerminalInteraction(interaction)) .await; - Ok(response) + Ok(boxed_tool_output(response)) } } -impl ToolHandler for WriteStdinHandler { +impl CoreToolRuntime for WriteStdinHandler { fn matches_kind(&self, payload: &ToolPayload) -> bool { matches!(payload, ToolPayload::Function { .. }) } @@ -98,7 +99,7 @@ impl ToolHandler for WriteStdinHandler { fn post_tool_use_payload( &self, invocation: &ToolInvocation, - result: &Self::Output, + result: &dyn crate::tools::context::ToolOutput, ) -> Option { post_unified_exec_tool_use_payload(invocation, result) } diff --git a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs index 6ab2752b94..50a8e44214 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs @@ -13,7 +13,7 @@ use crate::tools::context::ToolCallSource; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::hook_names::HookToolName; -use crate::tools::registry::ToolHandler; +use crate::tools::registry::CoreToolRuntime; use crate::turn_diff_tracker::TurnDiffTracker; use tokio::sync::Mutex; diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index 7727fb4370..7ad6a79829 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -16,12 +16,13 @@ use crate::original_image_detail::can_request_original_image_detail; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments; use crate::tools::handlers::resolve_tool_environment; use crate::tools::handlers::view_image_spec::ViewImageToolOptions; use crate::tools::handlers::view_image_spec::create_view_image_tool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -64,8 +65,6 @@ enum ViewImageDetail { #[async_trait::async_trait] impl ToolExecutor for ViewImageHandler { - type Output = ViewImageOutput; - fn tool_name(&self) -> ToolName { ToolName::plain("view_image") } @@ -78,7 +77,10 @@ impl ToolExecutor for ViewImageHandler { true } - async fn handle(&self, invocation: ToolInvocation) -> Result { + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { if !invocation .turn .model_info @@ -195,14 +197,14 @@ impl ToolExecutor for ViewImageHandler { session.emit_turn_item_started(turn.as_ref(), &item).await; session.emit_turn_item_completed(turn.as_ref(), item).await; - Ok(ViewImageOutput { + Ok(boxed_tool_output(ViewImageOutput { image_url, image_detail, - }) + })) } } -impl ToolHandler for ViewImageHandler {} +impl CoreToolRuntime for ViewImageHandler {} pub struct ViewImageOutput { image_url: String, diff --git a/codex-rs/core/src/tools/mod.rs b/codex-rs/core/src/tools/mod.rs index 3073d9f9da..5cd943e199 100644 --- a/codex-rs/core/src/tools/mod.rs +++ b/codex-rs/core/src/tools/mod.rs @@ -11,9 +11,7 @@ pub(crate) mod registry; pub(crate) mod router; pub(crate) mod runtimes; pub(crate) mod sandboxing; -pub(crate) mod spec; pub(crate) mod spec_plan; -pub(crate) mod spec_plan_types; pub(crate) mod tool_dispatch_trace; pub(crate) mod tool_search_entry; @@ -48,6 +46,18 @@ pub(crate) fn flat_tool_name(tool_name: &ToolName) -> Cow<'_, str> { } } +pub(crate) fn tool_user_shell_type( + user_shell: &crate::shell::Shell, +) -> codex_tools::ToolUserShellType { + match user_shell.shell_type { + crate::shell::ShellType::Zsh => codex_tools::ToolUserShellType::Zsh, + crate::shell::ShellType::Bash => codex_tools::ToolUserShellType::Bash, + crate::shell::ShellType::PowerShell => codex_tools::ToolUserShellType::PowerShell, + crate::shell::ShellType::Sh => codex_tools::ToolUserShellType::Sh, + crate::shell::ShellType::Cmd => codex_tools::ToolUserShellType::Cmd, + } +} + /// Format the combined exec output for sending back to the model. /// Includes exit code and duration metadata; truncates large bodies safely. pub fn format_exec_output_for_model_structured( diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 730c3b7ef9..1cf0e00623 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -1,5 +1,4 @@ use std::collections::HashMap; -use std::future::Future; use std::sync::Arc; use std::time::Duration; @@ -35,7 +34,11 @@ pub(crate) type ToolTelemetryTags = Vec<(&'static str, String)>; pub use codex_tools::ToolExecutor; pub use codex_tools::ToolExposure; -pub trait ToolHandler: ToolExecutor { +/// Typed runtime contract for locally executed tools. +/// +/// Implementers provide the shared `ToolExecutor` behavior plus optional +/// core-owned metadata for hooks, telemetry, tool search, and argument diffs. +pub(crate) trait CoreToolRuntime: ToolExecutor { fn search_info(&self) -> Option { None } @@ -47,17 +50,17 @@ pub trait ToolHandler: ToolExecutor { ) } - fn telemetry_tags( - &self, - _invocation: &ToolInvocation, - ) -> impl Future + Send { - async { Vec::new() } + fn telemetry_tags<'a>( + &'a self, + _invocation: &'a ToolInvocation, + ) -> BoxFuture<'a, ToolTelemetryTags> { + Box::pin(async { Vec::new() }) } fn post_tool_use_payload( &self, _invocation: &ToolInvocation, - _result: &Self::Output, + _result: &dyn ToolOutput, ) -> Option { None } @@ -154,117 +157,10 @@ pub(crate) struct PostToolUsePayload { pub(crate) tool_response: Value, } -/// Object-safe registry entry for heterogeneous tool handlers. -/// -/// Concrete handlers keep their typed `ToolExecutor::Output`; the registry -/// boxes that output only after typed hooks have run. -pub(crate) trait RegisteredTool: Send + Sync { - fn tool_name(&self) -> ToolName; - - fn spec(&self) -> Option; - - fn exposure(&self) -> ToolExposure; - - fn search_info(&self) -> Option; - - fn supports_parallel_tool_calls(&self) -> bool; - - fn matches_kind(&self, payload: &ToolPayload) -> bool; - - fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option; - - fn with_updated_hook_input( - &self, - invocation: ToolInvocation, - updated_input: Value, - ) -> Result; - - fn telemetry_tags<'a>( - &'a self, - invocation: &'a ToolInvocation, - ) -> BoxFuture<'a, ToolTelemetryTags>; - - fn create_diff_consumer(&self) -> Option>; - fn handle_any<'a>( - &'a self, - invocation: ToolInvocation, - ) -> BoxFuture<'a, Result>; -} - -impl RegisteredTool for T -where - T: ToolHandler, -{ - fn tool_name(&self) -> ToolName { - ToolExecutor::tool_name(self) - } - - fn spec(&self) -> Option { - ToolExecutor::spec(self) - } - - fn exposure(&self) -> ToolExposure { - ToolExecutor::exposure(self) - } - - fn search_info(&self) -> Option { - ToolHandler::search_info(self) - } - - fn supports_parallel_tool_calls(&self) -> bool { - ToolExecutor::supports_parallel_tool_calls(self) - } - - fn matches_kind(&self, payload: &ToolPayload) -> bool { - ToolHandler::matches_kind(self, payload) - } - - fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { - ToolHandler::pre_tool_use_payload(self, invocation) - } - - fn with_updated_hook_input( - &self, - invocation: ToolInvocation, - updated_input: Value, - ) -> Result { - ToolHandler::with_updated_hook_input(self, invocation, updated_input) - } - - fn telemetry_tags<'a>( - &'a self, - invocation: &'a ToolInvocation, - ) -> BoxFuture<'a, ToolTelemetryTags> { - Box::pin(ToolHandler::telemetry_tags(self, invocation)) - } - - fn create_diff_consumer(&self) -> Option> { - ToolHandler::create_diff_consumer(self) - } - fn handle_any<'a>( - &'a self, - invocation: ToolInvocation, - ) -> BoxFuture<'a, Result> { - Box::pin(async move { - let call_id = invocation.call_id.clone(); - let payload = invocation.payload.clone(); - let output = ToolExecutor::handle(self, invocation.clone()).await?; - let post_tool_use_payload = - ToolHandler::post_tool_use_payload(self, &invocation, &output); - Ok(AnyToolResult { - call_id, - payload, - result: Box::new(output), - post_tool_use_payload, - }) - }) - } -} - pub(crate) fn override_tool_exposure( - handler: Arc, + handler: Arc, exposure: ToolExposure, -) -> Arc { +) -> Arc { if handler.exposure() == exposure { return handler; } @@ -273,11 +169,12 @@ pub(crate) fn override_tool_exposure( } struct ExposureOverride { - handler: Arc, + handler: Arc, exposure: ToolExposure, } -impl RegisteredTool for ExposureOverride { +#[async_trait::async_trait] +impl ToolExecutor for ExposureOverride { fn tool_name(&self) -> ToolName { self.handler.tool_name() } @@ -290,14 +187,23 @@ impl RegisteredTool for ExposureOverride { self.exposure } - fn search_info(&self) -> Option { - self.handler.search_info() - } - fn supports_parallel_tool_calls(&self) -> bool { self.handler.supports_parallel_tool_calls() } + async fn handle( + &self, + invocation: ToolInvocation, + ) -> Result, FunctionCallError> { + self.handler.handle(invocation).await + } +} + +impl CoreToolRuntime for ExposureOverride { + fn search_info(&self) -> Option { + self.handler.search_info() + } + fn matches_kind(&self, payload: &ToolPayload) -> bool { self.handler.matches_kind(payload) } @@ -306,6 +212,14 @@ impl RegisteredTool for ExposureOverride { self.handler.pre_tool_use_payload(invocation) } + fn post_tool_use_payload( + &self, + invocation: &ToolInvocation, + result: &dyn ToolOutput, + ) -> Option { + self.handler.post_tool_use_payload(invocation, result) + } + fn with_updated_hook_input( &self, invocation: ToolInvocation, @@ -325,22 +239,28 @@ impl RegisteredTool for ExposureOverride { fn create_diff_consumer(&self) -> Option> { self.handler.create_diff_consumer() } - - fn handle_any<'a>( - &'a self, - invocation: ToolInvocation, - ) -> BoxFuture<'a, Result> { - self.handler.handle_any(invocation) - } } pub struct ToolRegistry { - handlers: HashMap>, + tools: HashMap>, } impl ToolRegistry { - fn new(handlers: HashMap>) -> Self { - Self { handlers } + fn new(tools: HashMap>) -> Self { + Self { tools } + } + + pub(crate) fn from_tools(tools: impl IntoIterator>) -> Self { + let mut tools_by_name = HashMap::new(); + for tool in tools { + let name = tool.tool_name(); + if tools_by_name.contains_key(&name) { + error_or_panic(format!("tool {name} already registered")); + continue; + } + tools_by_name.insert(name, tool); + } + Self::new(tools_by_name) } #[cfg(test)] @@ -351,35 +271,35 @@ impl ToolRegistry { #[cfg(test)] pub(crate) fn with_handler_for_test(handler: Arc) -> Self where - T: ToolHandler + 'static, + T: CoreToolRuntime + 'static, { let name = handler.tool_name(); - Self::new(HashMap::from([(name, handler as Arc)])) + Self::new(HashMap::from([(name, handler as Arc)])) } - fn handler(&self, name: &ToolName) -> Option> { - self.handlers.get(name).map(Arc::clone) + fn tool(&self, name: &ToolName) -> Option> { + self.tools.get(name).map(Arc::clone) } pub(crate) fn tool_exposure(&self, name: &ToolName) -> Option { - self.handlers.get(name).map(|handler| handler.exposure()) + self.tools.get(name).map(|tool| tool.exposure()) } #[cfg(test)] - pub(crate) fn has_handler(&self, name: &ToolName) -> bool { - self.handler(name).is_some() + pub(crate) fn has_tool(&self, name: &ToolName) -> bool { + self.tool(name).is_some() } pub(crate) fn create_diff_consumer( &self, name: &ToolName, ) -> Option> { - self.handler(name)?.create_diff_consumer() + self.tool(name)?.create_diff_consumer() } pub(crate) fn supports_parallel_tool_calls(&self, name: &ToolName) -> Option { - let handler = self.handler(name)?; - Some(handler.supports_parallel_tool_calls()) + let tool = self.tool(name)?; + Some(tool.supports_parallel_tool_calls()) } #[expect( @@ -422,8 +342,8 @@ impl ToolRegistry { } let dispatch_trace = ToolDispatchTrace::start(&invocation); - let handler = match self.handler(&tool_name) { - Some(handler) => handler, + let tool = match self.tool(&tool_name) { + Some(tool) => tool, None => { let message = unsupported_tool_call_message(&invocation.payload, &tool_name); let log_payload = invocation.payload.log_payload(); @@ -443,7 +363,7 @@ impl ToolRegistry { } }; - let telemetry_tags = handler.telemetry_tags(&invocation).await; + let telemetry_tags = tool.telemetry_tags(&invocation).await; let mut tool_result_tags = Vec::with_capacity(base_tool_result_tags.len() + telemetry_tags.len()); let mut extra_trace_fields = Vec::new(); @@ -455,7 +375,7 @@ impl ToolRegistry { tool_result_tags.push((*key, value.as_str())); } } - if !handler.matches_kind(&invocation.payload) { + if !tool.matches_kind(&invocation.payload) { let message = format!("tool {tool_name} invoked with incompatible payload"); let log_payload = invocation.payload.log_payload(); otel.tool_result_with_tags( @@ -473,7 +393,7 @@ impl ToolRegistry { return Err(err); } - if let Some(pre_tool_use_payload) = handler.pre_tool_use_payload(&invocation) { + if let Some(pre_tool_use_payload) = tool.pre_tool_use_payload(&invocation) { match run_pre_tool_use_hooks( &invocation.session, &invocation.turn, @@ -491,7 +411,7 @@ impl ToolRegistry { PreToolUseHookResult::Continue { updated_input: Some(updated_input), } => { - invocation = handler.with_updated_hook_input(invocation, updated_input)?; + invocation = tool.with_updated_hook_input(invocation, updated_input)?; } PreToolUseHookResult::Continue { updated_input: None, @@ -511,10 +431,10 @@ impl ToolRegistry { &tool_result_tags, &extra_trace_fields, || { - let handler = handler.clone(); + let tool = tool.clone(); let response_cell = &response_cell; async move { - match handler.handle_any(invocation_for_tool).await { + match handle_any_tool(tool.as_ref(), invocation_for_tool).await { Ok(result) => { let preview = result.result.log_preview(); let success = result.result.success_for_logging(); @@ -620,52 +540,21 @@ impl ToolRegistry { } } -pub struct ToolRegistryBuilder { - handlers: HashMap>, - specs: Vec, -} - -impl ToolRegistryBuilder { - pub fn new() -> Self { - Self { - handlers: HashMap::new(), - specs: Vec::new(), - } - } - - pub(crate) fn push_spec(&mut self, spec: ToolSpec) { - self.specs.push(spec); - } - - pub(crate) fn register_tool(&mut self, handler: Arc) { - self.register_tool_internal(handler, /*include_spec*/ true); - } - - pub(crate) fn register_tool_without_spec(&mut self, handler: Arc) { - self.register_tool_internal(handler, /*include_spec*/ false); - } - - fn register_tool_internal(&mut self, handler: Arc, include_spec: bool) { - let name = handler.tool_name(); - if self.handlers.contains_key(&name) { - error_or_panic(format!("handler for tool {name} already registered")); - return; - } - - if include_spec - && handler.exposure().is_direct() - && let Some(spec) = handler.spec() - { - self.push_spec(spec); - } - - self.handlers.insert(name, handler); - } - - pub fn build(self) -> (Vec, ToolRegistry) { - let registry = ToolRegistry::new(self.handlers); - (self.specs, registry) - } +async fn handle_any_tool( + tool: &dyn CoreToolRuntime, + invocation: ToolInvocation, +) -> Result { + let call_id = invocation.call_id.clone(); + let payload = invocation.payload.clone(); + let output = tool.handle(invocation.clone()).await?; + let post_tool_use_payload = + CoreToolRuntime::post_tool_use_payload(tool, &invocation, output.as_ref()); + Ok(AnyToolResult { + call_id, + payload, + result: output, + post_tool_use_payload, + }) } fn unsupported_tool_call_message(payload: &ToolPayload, tool_name: &ToolName) -> String { diff --git a/codex-rs/core/src/tools/registry_tests.rs b/codex-rs/core/src/tools/registry_tests.rs index b6c8eadadf..defacf33c0 100644 --- a/codex-rs/core/src/tools/registry_tests.rs +++ b/codex-rs/core/src/tools/registry_tests.rs @@ -1,7 +1,4 @@ use super::*; -use crate::tools::handlers::GetGoalHandler; -use crate::tools::handlers::goal_spec::GET_GOAL_TOOL_NAME; -use crate::tools::handlers::goal_spec::create_get_goal_tool; use pretty_assertions::assert_eq; struct TestHandler { @@ -10,21 +7,21 @@ struct TestHandler { #[async_trait::async_trait] impl ToolExecutor for TestHandler { - type Output = crate::tools::context::FunctionToolOutput; - fn tool_name(&self) -> codex_tools::ToolName { self.tool_name.clone() } - async fn handle(&self, _invocation: ToolInvocation) -> Result { - Ok(crate::tools::context::FunctionToolOutput::from_text( - "ok".to_string(), - Some(true), + async fn handle( + &self, + _invocation: ToolInvocation, + ) -> Result, FunctionCallError> { + Ok(Box::new( + crate::tools::context::FunctionToolOutput::from_text("ok".to_string(), Some(true)), )) } } -impl ToolHandler for TestHandler {} +impl CoreToolRuntime for TestHandler {} #[test] fn handler_looks_up_namespaced_aliases_explicitly() { @@ -34,18 +31,18 @@ fn handler_looks_up_namespaced_aliases_explicitly() { let namespaced_name = codex_tools::ToolName::namespaced(namespace, tool_name); let plain_handler = Arc::new(TestHandler { tool_name: plain_name.clone(), - }) as Arc; + }) as Arc; let namespaced_handler = Arc::new(TestHandler { tool_name: namespaced_name.clone(), - }) as Arc; + }) as Arc; let registry = ToolRegistry::new(HashMap::from([ (plain_name.clone(), Arc::clone(&plain_handler)), (namespaced_name.clone(), Arc::clone(&namespaced_handler)), ])); - let plain = registry.handler(&plain_name); - let namespaced = registry.handler(&namespaced_name); - let missing_namespaced = registry.handler(&codex_tools::ToolName::namespaced( + let plain = registry.tool(&plain_name); + let namespaced = registry.tool(&namespaced_name); + let missing_namespaced = registry.tool(&codex_tools::ToolName::namespaced( "mcp__codex_apps__calendar", tool_name, )); @@ -64,15 +61,3 @@ fn handler_looks_up_namespaced_aliases_explicitly() { .is_some_and(|handler| Arc::ptr_eq(handler, &namespaced_handler)) ); } - -#[test] -fn register_tool_adds_executor_and_spec() { - let mut builder = ToolRegistryBuilder::new(); - builder.register_tool(Arc::new(GetGoalHandler)); - - let (specs, registry) = builder.build(); - - assert_eq!(specs.len(), 1); - assert_eq!(specs[0], create_get_goal_tool()); - assert!(registry.has_handler(&codex_tools::ToolName::plain(GET_GOAL_TOOL_NAME))); -} diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index 0f111922c6..2477ba347c 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -5,18 +5,16 @@ use crate::tools::context::SharedTurnDiffTracker; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::registry::AnyToolResult; -use crate::tools::registry::RegisteredTool; use crate::tools::registry::ToolArgumentDiffConsumer; -use crate::tools::registry::ToolExposure; use crate::tools::registry::ToolRegistry; -use crate::tools::spec::collect_tool_router_parts; -use crate::tools::spec_plan::build_tool_registry_builder_from_executors; -use codex_extension_api::ExtensionToolExecutor; +use crate::tools::spec_plan::build_tool_router; use codex_mcp::ToolInfo; use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_protocol::models::ResponseItem; use codex_protocol::models::SearchToolCallParams; use codex_tools::DiscoverableTool; +use codex_tools::ToolCall as ExtensionToolCall; +use codex_tools::ToolExecutor; use codex_tools::ToolName; use codex_tools::ToolSpec; use codex_tools::ToolsConfig; @@ -42,42 +40,16 @@ pub(crate) struct ToolRouterParams<'a> { pub(crate) mcp_tools: Option>, pub(crate) deferred_mcp_tools: Option>, pub(crate) discoverable_tools: Option>, - pub(crate) extension_tool_executors: Vec>, + pub(crate) extension_tool_executors: Vec>>, pub(crate) dynamic_tools: &'a [DynamicToolSpec], } impl ToolRouter { pub fn from_config(config: &ToolsConfig, params: ToolRouterParams<'_>) -> Self { - let ToolRouterParams { - mcp_tools, - deferred_mcp_tools, - discoverable_tools, - extension_tool_executors, - dynamic_tools, - } = params; - let parts = collect_tool_router_parts( - config, - mcp_tools, - deferred_mcp_tools, - discoverable_tools, - &extension_tool_executors, - dynamic_tools, - ); - Self::from_executors(config, parts.executors, parts.hosted_specs) + build_tool_router(config, params) } - pub(crate) fn from_executors( - config: &ToolsConfig, - executors: Vec>, - hosted_specs: Vec, - ) -> Self { - let builder = build_tool_registry_builder_from_executors(config, executors, hosted_specs); - let (specs, registry) = builder.build(); - let model_visible_specs = specs - .into_iter() - .filter(|spec| !is_hidden_by_code_mode_only(config, ®istry, spec)) - .collect(); - + pub(crate) fn from_parts(registry: ToolRegistry, model_visible_specs: Vec) -> Self { Self { registry, model_visible_specs, @@ -182,22 +154,9 @@ impl ToolRouter { } } -fn is_hidden_by_code_mode_only( - config: &ToolsConfig, - registry: &ToolRegistry, - spec: &ToolSpec, -) -> bool { - if !config.code_mode_only_enabled || !codex_code_mode::is_code_mode_nested_tool(spec.name()) { - return false; - } - - let exposure = registry - .tool_exposure(&ToolName::plain(spec.name())) - .unwrap_or(ToolExposure::Direct); - exposure != ToolExposure::DirectModelOnly -} - -pub(crate) fn extension_tool_executors(session: &Session) -> Vec> { +pub(crate) fn extension_tool_executors( + session: &Session, +) -> Vec>> { session .services .extensions diff --git a/codex-rs/core/src/tools/router_tests.rs b/codex-rs/core/src/tools/router_tests.rs index dd791d9135..c9bb4bcafc 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -7,7 +7,6 @@ use crate::turn_diff_tracker::TurnDiffTracker; use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionRegistry; use codex_extension_api::ExtensionRegistryBuilder; -use codex_extension_api::ExtensionToolExecutor; use codex_extension_api::ResponsesApiTool; use codex_extension_api::ToolCall as ExtensionToolCall; use codex_extension_api::ToolExecutor; @@ -37,7 +36,7 @@ impl codex_extension_api::ToolContributor for ExtensionEchoContributor { &self, _session_store: &ExtensionData, _thread_store: &ExtensionData, - ) -> Vec> { + ) -> Vec>> { vec![Arc::new(ExtensionEchoExecutor)] } } @@ -46,8 +45,6 @@ struct ExtensionEchoExecutor; #[async_trait::async_trait] impl ToolExecutor for ExtensionEchoExecutor { - type Output = codex_tools::JsonToolOutput; - fn tool_name(&self) -> ToolName { ToolName::namespaced("extension/", "echo") } @@ -78,14 +75,14 @@ impl ToolExecutor for ExtensionEchoExecutor { async fn handle( &self, call: ExtensionToolCall, - ) -> Result { + ) -> Result, codex_tools::FunctionCallError> { let arguments: serde_json::Value = serde_json::from_str(call.function_arguments()?).expect("test arguments should parse"); - Ok(codex_tools::JsonToolOutput::new(json!({ + Ok(Box::new(codex_tools::JsonToolOutput::new(json!({ "arguments": arguments, "callId": call.call_id, "ok": true, - }))) + })))) } } diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs deleted file mode 100644 index 64dfdb8316..0000000000 --- a/codex-rs/core/src/tools/spec.rs +++ /dev/null @@ -1,94 +0,0 @@ -use crate::config::DEFAULT_MULTI_AGENT_V2_DEFAULT_WAIT_TIMEOUT_MS; -use crate::config::DEFAULT_MULTI_AGENT_V2_MAX_WAIT_TIMEOUT_MS; -use crate::config::DEFAULT_MULTI_AGENT_V2_MIN_WAIT_TIMEOUT_MS; -use crate::shell::Shell; -use crate::shell::ShellType; -use crate::tools::handlers::multi_agents_common::DEFAULT_WAIT_TIMEOUT_MS; -use crate::tools::handlers::multi_agents_common::MAX_WAIT_TIMEOUT_MS; -use crate::tools::handlers::multi_agents_common::MIN_WAIT_TIMEOUT_MS; -use crate::tools::handlers::multi_agents_spec::WaitAgentTimeoutOptions; -use crate::tools::registry::RegisteredTool; -use crate::tools::spec_plan::collect_tool_executors; -use crate::tools::spec_plan::hosted_model_tool_specs; -use crate::tools::spec_plan_types::ToolRegistryBuildParams; -use codex_extension_api::ExtensionToolExecutor; -use codex_mcp::ToolInfo; -use codex_protocol::dynamic_tools::DynamicToolSpec; -use codex_tools::DiscoverableTool; -use codex_tools::ToolUserShellType; -use codex_tools::ToolsConfig; -use std::sync::Arc; - -pub(crate) fn tool_user_shell_type(user_shell: &Shell) -> ToolUserShellType { - match user_shell.shell_type { - ShellType::Zsh => ToolUserShellType::Zsh, - ShellType::Bash => ToolUserShellType::Bash, - ShellType::PowerShell => ToolUserShellType::PowerShell, - ShellType::Sh => ToolUserShellType::Sh, - ShellType::Cmd => ToolUserShellType::Cmd, - } -} - -pub(crate) struct ToolRouterParts { - pub(crate) executors: Vec>, - pub(crate) hosted_specs: Vec, -} - -pub(crate) fn collect_tool_router_parts( - config: &ToolsConfig, - mcp_tools: Option>, - deferred_mcp_tools: Option>, - discoverable_tools: Option>, - extension_tool_executors: &[Arc], - dynamic_tools: &[DynamicToolSpec], -) -> ToolRouterParts { - let default_agent_type_description = - crate::agent::role::spawn_tool_spec::build(&std::collections::BTreeMap::new()); - let (min_wait_timeout_ms, max_wait_timeout_ms, default_wait_timeout_ms) = - if config.multi_agent_v2 { - let min_wait_timeout_ms = config - .wait_agent_min_timeout_ms - .unwrap_or(DEFAULT_MULTI_AGENT_V2_MIN_WAIT_TIMEOUT_MS); - let max_wait_timeout_ms = config - .wait_agent_max_timeout_ms - .unwrap_or(DEFAULT_MULTI_AGENT_V2_MAX_WAIT_TIMEOUT_MS); - let default_wait_timeout_ms = config - .wait_agent_default_timeout_ms - .unwrap_or(DEFAULT_MULTI_AGENT_V2_DEFAULT_WAIT_TIMEOUT_MS); - ( - min_wait_timeout_ms, - max_wait_timeout_ms, - default_wait_timeout_ms, - ) - } else { - ( - MIN_WAIT_TIMEOUT_MS, - MAX_WAIT_TIMEOUT_MS, - DEFAULT_WAIT_TIMEOUT_MS, - ) - }; - let executors = collect_tool_executors( - config, - ToolRegistryBuildParams { - mcp_tools: mcp_tools.as_deref(), - deferred_mcp_tools: deferred_mcp_tools.as_deref(), - discoverable_tools: discoverable_tools.as_deref(), - extension_tool_executors, - 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, - max_timeout_ms: max_wait_timeout_ms, - }, - }, - ); - ToolRouterParts { - executors, - hosted_specs: hosted_model_tool_specs(config), - } -} - -#[cfg(test)] -#[path = "spec_tests.rs"] -mod tests; diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index 24f59a0f04..be5e6741d4 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -1,3 +1,6 @@ +use crate::config::DEFAULT_MULTI_AGENT_V2_DEFAULT_WAIT_TIMEOUT_MS; +use crate::config::DEFAULT_MULTI_AGENT_V2_MAX_WAIT_TIMEOUT_MS; +use crate::config::DEFAULT_MULTI_AGENT_V2_MIN_WAIT_TIMEOUT_MS; use crate::tools::code_mode::execute_spec::create_code_mode_tool; use crate::tools::handlers::ApplyPatchHandler; use crate::tools::handlers::CodeModeExecuteHandler; @@ -24,13 +27,17 @@ use crate::tools::handlers::ViewImageHandler; use crate::tools::handlers::WriteStdinHandler; use crate::tools::handlers::agent_jobs::ReportAgentJobResultHandler; use crate::tools::handlers::agent_jobs::SpawnAgentsOnCsvHandler; -use crate::tools::handlers::extension_tools::ExtensionToolHandler; +use crate::tools::handlers::extension_tools::ExtensionToolAdapter; use crate::tools::handlers::multi_agents::CloseAgentHandler; use crate::tools::handlers::multi_agents::ResumeAgentHandler; use crate::tools::handlers::multi_agents::SendInputHandler; use crate::tools::handlers::multi_agents::SpawnAgentHandler; use crate::tools::handlers::multi_agents::WaitAgentHandler; +use crate::tools::handlers::multi_agents_common::DEFAULT_WAIT_TIMEOUT_MS; +use crate::tools::handlers::multi_agents_common::MAX_WAIT_TIMEOUT_MS; +use crate::tools::handlers::multi_agents_common::MIN_WAIT_TIMEOUT_MS; use crate::tools::handlers::multi_agents_spec::SpawnAgentToolOptions; +use crate::tools::handlers::multi_agents_spec::WaitAgentTimeoutOptions; use crate::tools::handlers::multi_agents_v2::CloseAgentHandler as CloseAgentHandlerV2; use crate::tools::handlers::multi_agents_v2::FollowupTaskHandler as FollowupTaskHandlerV2; use crate::tools::handlers::multi_agents_v2::ListAgentsHandler as ListAgentsHandlerV2; @@ -41,17 +48,21 @@ use crate::tools::handlers::view_image_spec::ViewImageToolOptions; use crate::tools::hosted_spec::WebSearchToolOptions; use crate::tools::hosted_spec::create_image_generation_tool; use crate::tools::hosted_spec::create_web_search_tool; -use crate::tools::registry::RegisteredTool; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExposure; -use crate::tools::registry::ToolRegistryBuilder; +use crate::tools::registry::ToolRegistry; use crate::tools::registry::override_tool_exposure; -use crate::tools::spec_plan_types::ToolRegistryBuildParams; -use crate::tools::spec_plan_types::agent_type_description; -use codex_extension_api::ExtensionToolExecutor; +use crate::tools::router::ToolRouter; +use crate::tools::router::ToolRouterParams; +use codex_mcp::ToolInfo; +use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_protocol::openai_models::ConfigShellToolType; +use codex_tools::DiscoverableTool; use codex_tools::ResponsesApiNamespaceTool; use codex_tools::TOOL_SEARCH_TOOL_NAME; +use codex_tools::ToolCall as ExtensionToolCall; use codex_tools::ToolEnvironmentMode; +use codex_tools::ToolExecutor; use codex_tools::ToolName; use codex_tools::ToolSpec; use codex_tools::ToolsConfig; @@ -62,74 +73,94 @@ use std::collections::HashSet; use std::sync::Arc; use tracing::warn; -pub(crate) fn build_tool_registry_builder_from_executors( +#[derive(Clone, Copy)] +struct ToolRegistryBuildParams<'a> { + mcp_tools: Option<&'a [ToolInfo]>, + deferred_mcp_tools: Option<&'a [ToolInfo]>, + discoverable_tools: Option<&'a [DiscoverableTool]>, + extension_tool_executors: &'a [Arc>], + dynamic_tools: &'a [DynamicToolSpec], + default_agent_type_description: &'a str, + wait_agent_timeouts: WaitAgentTimeoutOptions, +} + +pub(crate) fn build_tool_router(config: &ToolsConfig, params: ToolRouterParams<'_>) -> ToolRouter { + let (model_visible_specs, registry) = build_tool_specs_and_registry(config, params); + ToolRouter::from_parts(registry, model_visible_specs) +} + +fn build_tool_specs_and_registry( config: &ToolsConfig, - executors: Vec>, - hosted_specs: Vec, -) -> ToolRegistryBuilder { - let mut builder = ToolRegistryBuilder::new(); - let deferred_tools_available = executors - .iter() - .any(|executor| executor.exposure() == ToolExposure::Deferred); - - for executor in build_code_mode_executors( + params: ToolRouterParams<'_>, +) -> (Vec, ToolRegistry) { + let ToolRouterParams { + mcp_tools, + deferred_mcp_tools, + discoverable_tools, + extension_tool_executors, + dynamic_tools, + } = params; + let default_agent_type_description = + crate::agent::role::spawn_tool_spec::build(&std::collections::BTreeMap::new()); + let mut executors = collect_tool_executors( config, - &executors, - config.search_tool && deferred_tools_available, - ) { - builder.register_tool(executor); - } - - let mut non_deferred_specs = Vec::new(); - let mut deferred_search_infos = Vec::new(); - for executor in &executors { - match executor.exposure() { - ToolExposure::Direct | ToolExposure::DirectModelOnly => { - if let Some(spec) = executor.spec() { - non_deferred_specs.push((spec, executor.exposure())); - } - } - ToolExposure::Deferred => { - if let Some(search_info) = executor.search_info() { - deferred_search_infos.push(search_info); - } - } - } - } - - non_deferred_specs.extend( - hosted_specs - .into_iter() - .map(|spec| (spec, ToolExposure::Direct)), + ToolRegistryBuildParams { + mcp_tools: mcp_tools.as_deref(), + deferred_mcp_tools: deferred_mcp_tools.as_deref(), + discoverable_tools: discoverable_tools.as_deref(), + extension_tool_executors: &extension_tool_executors, + dynamic_tools, + default_agent_type_description: &default_agent_type_description, + wait_agent_timeouts: wait_agent_timeout_options(config), + }, ); + append_tool_search_executor(config, &mut executors); + prepend_code_mode_executors(config, &mut executors); + build_model_visible_specs_and_registry(config, executors, hosted_model_tool_specs(config)) +} - let non_deferred_specs = non_deferred_specs - .into_iter() - .map(|(spec, exposure)| { - if config.code_mode_enabled && exposure != ToolExposure::DirectModelOnly { - codex_tools::augment_tool_spec_for_code_mode(spec) - } else { - spec - } - }) - .collect(); - - for spec in merge_into_namespaces(non_deferred_specs) { - if !config.namespace_tools && matches!(spec, ToolSpec::Namespace(_)) { +fn build_model_visible_specs_and_registry( + config: &ToolsConfig, + executors: Vec>, + hosted_specs: Vec, +) -> (Vec, ToolRegistry) { + let mut specs = Vec::new(); + let mut seen_tool_names = HashSet::new(); + for executor in &executors { + if !seen_tool_names.insert(executor.tool_name()) { continue; } - builder.push_spec(spec); + if executor.exposure().is_direct() + && let Some(spec) = executor.spec() + { + specs.push(spec_for_model_request(config, executor.exposure(), spec)); + } } + specs.extend(hosted_specs); - for executor in executors { - builder.register_tool_without_spec(executor); + let registry = ToolRegistry::from_tools(executors); + let model_visible_specs = merge_into_namespaces(specs) + .into_iter() + .filter(|spec| config.namespace_tools || !matches!(spec, ToolSpec::Namespace(_))) + .filter(|spec| !is_hidden_by_code_mode_only(config, ®istry, spec)) + .collect(); + + (model_visible_specs, registry) +} + +fn spec_for_model_request( + config: &ToolsConfig, + exposure: ToolExposure, + spec: ToolSpec, +) -> ToolSpec { + if config.code_mode_enabled + && exposure != ToolExposure::DirectModelOnly + && codex_code_mode::is_code_mode_nested_tool(spec.name()) + { + codex_tools::augment_tool_spec_for_code_mode(spec) + } else { + spec } - - if config.search_tool && config.namespace_tools && !deferred_search_infos.is_empty() { - builder.register_tool(Arc::new(ToolSearchHandler::new(deferred_search_infos))); - } - - builder } pub(crate) fn hosted_model_tool_specs(config: &ToolsConfig) -> Vec { @@ -147,11 +178,56 @@ pub(crate) fn hosted_model_tool_specs(config: &ToolsConfig) -> Vec { specs } +fn wait_agent_timeout_options(config: &ToolsConfig) -> WaitAgentTimeoutOptions { + if config.multi_agent_v2 { + return WaitAgentTimeoutOptions { + default_timeout_ms: config + .wait_agent_default_timeout_ms + .unwrap_or(DEFAULT_MULTI_AGENT_V2_DEFAULT_WAIT_TIMEOUT_MS), + min_timeout_ms: config + .wait_agent_min_timeout_ms + .unwrap_or(DEFAULT_MULTI_AGENT_V2_MIN_WAIT_TIMEOUT_MS), + max_timeout_ms: config + .wait_agent_max_timeout_ms + .unwrap_or(DEFAULT_MULTI_AGENT_V2_MAX_WAIT_TIMEOUT_MS), + }; + } + + WaitAgentTimeoutOptions { + default_timeout_ms: DEFAULT_WAIT_TIMEOUT_MS, + min_timeout_ms: MIN_WAIT_TIMEOUT_MS, + max_timeout_ms: MAX_WAIT_TIMEOUT_MS, + } +} + +fn agent_type_description(config: &ToolsConfig, default_agent_type_description: &str) -> String { + if config.agent_type_description.is_empty() { + default_agent_type_description.to_string() + } else { + config.agent_type_description.clone() + } +} + +fn is_hidden_by_code_mode_only( + config: &ToolsConfig, + registry: &ToolRegistry, + spec: &ToolSpec, +) -> bool { + if !config.code_mode_only_enabled || !codex_code_mode::is_code_mode_nested_tool(spec.name()) { + return false; + } + + let exposure = registry + .tool_exposure(&ToolName::plain(spec.name())) + .unwrap_or(ToolExposure::Direct); + exposure != ToolExposure::DirectModelOnly +} + fn build_code_mode_executors( config: &ToolsConfig, - executors: &[Arc], + executors: &[Arc], deferred_tools_available: bool, -) -> Vec> { +) -> Vec> { if !config.code_mode_enabled { return vec![]; } @@ -254,12 +330,12 @@ fn code_mode_namespace_descriptions( namespace_descriptions } -pub(crate) fn collect_tool_executors( +fn collect_tool_executors( config: &ToolsConfig, params: ToolRegistryBuildParams<'_>, -) -> Vec> { +) -> Vec> { let exec_permission_approvals_enabled = config.exec_permission_approvals_enabled; - let mut executors = Vec::>::new(); + let mut executors = Vec::>::new(); if config.environment_mode.has_environment() { let include_environment_id = @@ -444,10 +520,43 @@ pub(crate) fn collect_tool_executors( executors } +fn append_tool_search_executor( + config: &ToolsConfig, + executors: &mut Vec>, +) { + if !(config.search_tool && config.namespace_tools) { + return; + } + + let search_infos = executors + .iter() + .filter(|executor| executor.exposure() == ToolExposure::Deferred) + .filter_map(|executor| executor.search_info()) + .collect::>(); + if search_infos.is_empty() { + return; + } + + executors.push(Arc::new(ToolSearchHandler::new(search_infos))); +} + +fn prepend_code_mode_executors( + config: &ToolsConfig, + executors: &mut Vec>, +) { + let deferred_tools_available = config.search_tool + && executors + .iter() + .any(|executor| executor.exposure() == ToolExposure::Deferred); + let code_mode_executors = + build_code_mode_executors(config, executors, deferred_tools_available); + executors.splice(0..0, code_mode_executors); +} + fn append_extension_tool_executors( config: &ToolsConfig, - executors: &[Arc], - registered_executors: &mut Vec>, + executors: &[Arc>], + registered_executors: &mut Vec>, ) { if executors.is_empty() { return; @@ -473,17 +582,17 @@ fn append_extension_tool_executors( for executor in executors.iter().cloned() { let tool_name = executor.tool_name(); if !reserved_tool_names.insert(tool_name.clone()) { - warn!("Skipping extension tool `{tool_name}`: handler already registered"); + warn!("Skipping extension tool `{tool_name}`: tool already registered"); continue; } - registered_executors.push(Arc::new(ExtensionToolHandler::new(executor))); + registered_executors.push(Arc::new(ExtensionToolAdapter::new(executor))); } } fn multi_agent_v2_handler( - handler: impl RegisteredTool + 'static, + handler: impl CoreToolRuntime + 'static, exposure: ToolExposure, -) -> Arc { +) -> Arc { override_tool_exposure(Arc::new(handler), exposure) } @@ -512,6 +621,9 @@ fn code_mode_namespace_name<'a>( .map(|namespace_description| namespace_description.name.as_str()) } +#[cfg(test)] +#[path = "spec_plan_model_tests.rs"] +mod model_tests; #[cfg(test)] #[path = "spec_plan_tests.rs"] mod tests; diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_plan_model_tests.rs similarity index 87% rename from codex-rs/core/src/tools/spec_tests.rs rename to codex-rs/core/src/tools/spec_plan_model_tests.rs index 8653810ba4..dcc1be30ed 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_model_tests.rs @@ -3,17 +3,18 @@ use crate::shell::Shell; use crate::shell::ShellType; use crate::test_support::construct_model_info_offline; use crate::tools::ToolRouter; -use crate::tools::registry::ToolRegistryBuilder; use crate::tools::router::ToolRouterParams; -use crate::tools::spec_plan::build_tool_registry_builder_from_executors; +use crate::tools::tool_user_shell_type; use codex_app_server_protocol::AppInfo; use codex_features::Feature; use codex_features::Features; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; +use codex_mcp::ToolInfo; use codex_models_manager::bundled_models_response; use codex_models_manager::model_info::with_config_overrides; use codex_protocol::config_types::WebSearchMode; use codex_protocol::config_types::WindowsSandboxLevel; +use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_protocol::models::PermissionProfile; use codex_protocol::openai_models::ConfigShellToolType; use codex_protocol::openai_models::ModelInfo; @@ -40,8 +41,6 @@ use std::collections::BTreeMap; use std::path::PathBuf; use std::sync::Arc; -use super::*; - fn mcp_tool(name: &str, description: &str, input_schema: serde_json::Value) -> rmcp::model::Tool { rmcp::model::Tool { name: name.to_string().into(), @@ -56,21 +55,6 @@ fn mcp_tool(name: &str, description: &str, input_schema: serde_json::Value) -> r } } -fn mcp_tool_info(tool: rmcp::model::Tool) -> ToolInfo { - ToolInfo { - server_name: "test_server".to_string(), - supports_parallel_tool_calls: false, - server_origin: None, - callable_name: tool.name.to_string(), - callable_namespace: "mcp__test_server__".to_string(), - namespace_description: None, - tool, - connector_id: None, - connector_name: None, - plugin_display_names: Vec::new(), - } -} - fn mcp_tool_info_with_display_name(display_name: &str, tool: rmcp::model::Tool) -> ToolInfo { let (callable_namespace, callable_name) = display_name .rsplit_once('/') @@ -240,13 +224,12 @@ async fn multi_agent_v2_tools_config() -> ToolsConfig { } fn multi_agent_v2_spawn_agent_description(tools_config: &ToolsConfig) -> String { - let (tools, _) = build_specs( + let tools = build_specs( tools_config, /*mcp_tools*/ None, /*deferred_mcp_tools*/ None, &[], - ) - .build(); + ); let spawn_agent = find_tool(&tools, "spawn_agent"); let ToolSpec::Function(ResponsesApiTool { description, .. }) = spawn_agent else { panic!("spawn_agent should be a function tool"); @@ -266,13 +249,13 @@ async fn model_info_from_models_json(slug: &str) -> ModelInfo { with_config_overrides(model, &config.to_models_manager_config()) } -/// Builds the tool registry builder while collecting tool specs for later serialization. +/// Builds tool specs and the matching registry from the same executor list. fn build_specs( config: &ToolsConfig, mcp_tools: Option>, deferred_mcp_tools: Option>, dynamic_tools: &[DynamicToolSpec], -) -> ToolRegistryBuilder { +) -> Vec { build_specs_with_inputs_for_test( config, mcp_tools, @@ -288,18 +271,22 @@ fn build_specs_with_inputs_for_test( mcp_tools: Option>, deferred_mcp_tools: Option>, discoverable_tools: Option>, - extension_tool_executors: &[Arc], + extension_tool_executors: &[Arc< + dyn codex_extension_api::ToolExecutor, + >], dynamic_tools: &[DynamicToolSpec], -) -> ToolRegistryBuilder { - let parts = collect_tool_router_parts( +) -> Vec { + ToolRouter::from_config( config, - mcp_tools, - deferred_mcp_tools, - discoverable_tools, - extension_tool_executors, - dynamic_tools, - ); - build_tool_registry_builder_from_executors(config, parts.executors, parts.hosted_specs) + ToolRouterParams { + mcp_tools, + deferred_mcp_tools, + discoverable_tools, + extension_tool_executors: extension_tool_executors.to_vec(), + dynamic_tools, + }, + ) + .model_visible_specs() } #[tokio::test] @@ -319,13 +306,12 @@ async fn get_memory_requires_feature_flag() { permission_profile: &PermissionProfile::Disabled, windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, /*mcp_tools*/ None, /*deferred_mcp_tools*/ None, &[], - ) - .build(); + ); assert!( !tools.iter().any(|t| t.name() == "get_memory"), "get_memory should be disabled when memory_tool feature is off" @@ -633,13 +619,12 @@ async fn test_build_specs_default_shell_present() { permission_profile: &PermissionProfile::Disabled, windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, Some(Vec::new()), /*deferred_mcp_tools*/ None, &[], - ) - .build(); + ); // Only check the shell variant and a couple of core tools. let mut subset = vec!["exec_command", "write_stdin", "update_plan"]; @@ -774,13 +759,12 @@ async fn multi_agent_v2_wait_agent_schema_uses_configured_timeouts() { .with_wait_agent_min_timeout_ms(wait_agent_min_timeout_ms) .with_wait_agent_max_timeout_ms(wait_agent_max_timeout_ms) .with_wait_agent_default_timeout_ms(wait_agent_default_timeout_ms); - let (tools, _) = build_specs( + 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 else { panic!("wait_agent should be a function tool"); @@ -825,15 +809,14 @@ async fn request_plugin_install_requires_apps_and_plugins_features() { permission_profile: &PermissionProfile::Disabled, windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs_with_inputs_for_test( + let tools = build_specs_with_inputs_for_test( &tools_config, /*mcp_tools*/ None, /*deferred_mcp_tools*/ None, discoverable_tools.clone(), /*extension_tool_executors*/ &[], &[], - ) - .build(); + ); assert!( !tools @@ -862,13 +845,12 @@ async fn search_tool_is_hidden_without_deferred_tools() { windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, /*mcp_tools*/ None, Some(Vec::new()), &[], - ) - .build(); + ); assert!( !tools .iter() @@ -894,7 +876,7 @@ async fn search_tool_description_falls_back_to_connector_name_without_descriptio windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, /*mcp_tools*/ None, Some(vec![ToolInfo { @@ -914,8 +896,7 @@ async fn search_tool_description_falls_back_to_connector_name_without_descriptio plugin_display_names: Vec::new(), }]), &[], - ) - .build(); + ); let search_tool = find_tool(&tools, TOOL_SEARCH_TOOL_NAME); let ToolSpec::ToolSearch { description, .. } = search_tool else { panic!("expected tool_search tool"); @@ -925,119 +906,6 @@ async fn search_tool_description_falls_back_to_connector_name_without_descriptio assert!(!description.contains("- Calendar:")); } -#[tokio::test] -async fn search_tool_registers_namespaced_mcp_tool_aliases() { - let model_info = search_capable_model_info().await; - let mut features = Features::with_defaults(); - features.enable(Feature::Apps); - features.enable(Feature::ToolSearch); - let available_models = Vec::new(); - let tools_config = ToolsConfig::new(&ToolsConfigParams { - model_info: &model_info, - available_models: &available_models, - features: &features, - image_generation_tool_auth_allowed: true, - web_search_mode: Some(WebSearchMode::Cached), - session_source: SessionSource::Cli, - permission_profile: &PermissionProfile::Disabled, - windows_sandbox_level: WindowsSandboxLevel::Disabled, - }); - - let (_, registry) = build_specs( - &tools_config, - /*mcp_tools*/ None, - Some(vec![ - ToolInfo { - server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), - supports_parallel_tool_calls: false, - server_origin: None, - callable_name: "_create_event".to_string(), - callable_namespace: "mcp__codex_apps__calendar".to_string(), - namespace_description: None, - tool: mcp_tool( - "calendar-create-event", - "Create calendar event", - serde_json::json!({"type": "object"}), - ), - connector_id: Some("calendar".to_string()), - connector_name: Some("Calendar".to_string()), - plugin_display_names: Vec::new(), - }, - ToolInfo { - server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(), - supports_parallel_tool_calls: false, - server_origin: None, - callable_name: "_list_events".to_string(), - callable_namespace: "mcp__codex_apps__calendar".to_string(), - namespace_description: None, - tool: mcp_tool( - "calendar-list-events", - "List calendar events", - serde_json::json!({"type": "object"}), - ), - connector_id: Some("calendar".to_string()), - connector_name: Some("Calendar".to_string()), - plugin_display_names: Vec::new(), - }, - ToolInfo { - server_name: "rmcp".to_string(), - supports_parallel_tool_calls: false, - server_origin: None, - callable_name: "echo".to_string(), - callable_namespace: "mcp__rmcp__".to_string(), - namespace_description: None, - tool: mcp_tool("echo", "Echo", serde_json::json!({"type": "object"})), - connector_id: None, - connector_name: None, - plugin_display_names: Vec::new(), - }, - ]), - &[], - ) - .build(); - - let app_alias = ToolName::namespaced("mcp__codex_apps__calendar", "_create_event"); - let mcp_alias = ToolName::namespaced("mcp__rmcp__", "echo"); - - assert!(registry.has_handler(&ToolName::plain(TOOL_SEARCH_TOOL_NAME))); - assert!(registry.has_handler(&app_alias)); - assert!(registry.has_handler(&mcp_alias)); -} - -#[tokio::test] -async fn direct_mcp_tools_register_namespaced_handlers() { - let config = test_config().await; - let model_info = construct_model_info_offline("gpt-5.4", &config); - let mut features = Features::with_defaults(); - features.enable(Feature::UnifiedExec); - let available_models = Vec::new(); - let tools_config = ToolsConfig::new(&ToolsConfigParams { - model_info: &model_info, - available_models: &available_models, - features: &features, - image_generation_tool_auth_allowed: true, - web_search_mode: Some(WebSearchMode::Cached), - session_source: SessionSource::Cli, - permission_profile: &PermissionProfile::Disabled, - windows_sandbox_level: WindowsSandboxLevel::Disabled, - }); - - let (_, registry) = build_specs( - &tools_config, - Some(vec![mcp_tool_info(mcp_tool( - "echo", - "Echo", - serde_json::json!({"type": "object"}), - ))]), - /*deferred_mcp_tools*/ None, - &[], - ) - .build(); - - assert!(registry.has_handler(&ToolName::namespaced("mcp__test_server__", "echo"))); - assert!(!registry.has_handler(&ToolName::plain("mcp__test_server__echo"))); -} - #[tokio::test] async fn test_mcp_tool_property_missing_type_defaults_to_string() { let config = test_config().await; @@ -1056,7 +924,7 @@ async fn test_mcp_tool_property_missing_type_defaults_to_string() { windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, Some(vec![mcp_tool_info_with_display_name( "dash/search", @@ -1073,8 +941,7 @@ async fn test_mcp_tool_property_missing_type_defaults_to_string() { )]), /*deferred_mcp_tools*/ None, &[], - ) - .build(); + ); let tool = find_namespace_function_tool(&tools, "dash/", "search"); assert_eq!( @@ -1116,7 +983,7 @@ async fn test_mcp_tool_preserves_integer_schema() { windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, Some(vec![mcp_tool_info_with_display_name( "dash/paginate", @@ -1131,8 +998,7 @@ async fn test_mcp_tool_preserves_integer_schema() { )]), /*deferred_mcp_tools*/ None, &[], - ) - .build(); + ); let tool = find_namespace_function_tool(&tools, "dash/", "paginate"); assert_eq!( @@ -1174,7 +1040,7 @@ async fn test_mcp_tool_array_without_items_gets_default_string_items() { windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, Some(vec![mcp_tool_info_with_display_name( "dash/tags", @@ -1189,8 +1055,7 @@ async fn test_mcp_tool_array_without_items_gets_default_string_items() { )]), /*deferred_mcp_tools*/ None, &[], - ) - .build(); + ); let tool = find_namespace_function_tool(&tools, "dash/", "tags"); assert_eq!( @@ -1235,7 +1100,7 @@ async fn test_mcp_tool_anyof_defaults_to_string() { windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, Some(vec![mcp_tool_info_with_display_name( "dash/value", @@ -1252,8 +1117,7 @@ async fn test_mcp_tool_anyof_defaults_to_string() { )]), /*deferred_mcp_tools*/ None, &[], - ) - .build(); + ); let tool = find_namespace_function_tool(&tools, "dash/", "value"); assert_eq!( @@ -1300,7 +1164,7 @@ async fn test_get_openai_tools_mcp_tools_with_additional_properties_schema() { permission_profile: &PermissionProfile::Disabled, windows_sandbox_level: WindowsSandboxLevel::Disabled, }); - let (tools, _) = build_specs( + let tools = build_specs( &tools_config, Some(vec![mcp_tool_info_with_display_name( "test_server/do_something_cool", @@ -1334,8 +1198,7 @@ async fn test_get_openai_tools_mcp_tools_with_additional_properties_schema() { )]), /*deferred_mcp_tools*/ None, &[], - ) - .build(); + ); let tool = find_namespace_function_tool(&tools, "test_server/", "do_something_cool"); assert_eq!( diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index dd690b5631..063d75e6e8 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -26,7 +26,6 @@ use crate::tools::handlers::view_image_spec::ViewImageToolOptions; use crate::tools::handlers::view_image_spec::create_view_image_tool; use crate::tools::registry::ToolRegistry; use codex_app_server_protocol::AppInfo; -use codex_extension_api::ExtensionToolExecutor; use codex_extension_api::ToolCall as ExtensionToolCall; use codex_extension_api::ToolExecutor; use codex_features::Feature; @@ -74,7 +73,10 @@ const DEFAULT_WAIT_TIMEOUT_MS: i64 = 30_000; const MIN_WAIT_TIMEOUT_MS: i64 = 10_000; const MAX_WAIT_TIMEOUT_MS: i64 = 3_600_000; -fn extension_tool_executor(name: &str, description: &str) -> Arc { +fn extension_tool_executor( + name: &str, + description: &str, +) -> Arc> { struct SpecOnlyExtensionExecutor { name: String, description: String, @@ -82,8 +84,6 @@ fn extension_tool_executor(name: &str, description: &str) -> Arc for SpecOnlyExtensionExecutor { - type Output = codex_tools::JsonToolOutput; - fn tool_name(&self) -> ToolName { ToolName::plain(self.name.as_str()) } @@ -109,7 +109,7 @@ fn extension_tool_executor(name: &str, description: &str) -> Arc Result { + ) -> Result, codex_tools::FunctionCallError> { panic!("spec planning should not execute extension tools") } } @@ -1440,7 +1440,7 @@ fn namespace_specs_are_hidden_when_namespace_tools_are_disabled() { ); assert_lacks_tool_name(&tools, "mcp__sample__"); - assert!(registry.has_handler(&ToolName::namespaced("mcp__sample__", "echo"))); + assert!(registry.has_tool(&ToolName::namespaced("mcp__sample__", "echo"))); } #[test] @@ -1646,11 +1646,11 @@ fn search_tool_description_lists_each_mcp_source_once() { assert!(description.contains("- rmcp: Remote memory tools.")); assert!(!description.contains("mcp__rmcp__echo")); - assert!(registry.has_handler(&ToolName::namespaced( + assert!(registry.has_tool(&ToolName::namespaced( "mcp__codex_apps__calendar", "_create_event", ))); - assert!(registry.has_handler(&ToolName::namespaced("mcp__rmcp__", "echo"))); + assert!(registry.has_tool(&ToolName::namespaced("mcp__rmcp__", "echo"))); } #[test] @@ -1758,7 +1758,7 @@ fn no_search_tool_when_namespaces_disabled() { ); assert_lacks_tool_name(&tools, TOOL_SEARCH_TOOL_NAME); - assert!(!registry.has_handler(&ToolName::plain(TOOL_SEARCH_TOOL_NAME))); + assert!(!registry.has_tool(&ToolName::plain(TOOL_SEARCH_TOOL_NAME))); } #[test] @@ -1816,9 +1816,9 @@ fn search_tool_registers_for_deferred_dynamic_tools() { assert!(description.contains("- Dynamic tools: Tools provided by the current Codex thread.")); assert_contains_tool_names(&tools, &[TOOL_SEARCH_TOOL_NAME]); assert_lacks_tool_name(&tools, "codex_app"); - assert!(registry.has_handler(&ToolName::plain(TOOL_SEARCH_TOOL_NAME))); - assert!(registry.has_handler(&ToolName::namespaced("codex_app", "automation_update"))); - assert!(registry.has_handler(&ToolName::namespaced("codex_app", "automation_list"))); + assert!(registry.has_tool(&ToolName::plain(TOOL_SEARCH_TOOL_NAME))); + assert!(registry.has_tool(&ToolName::namespaced("codex_app", "automation_update"))); + assert!(registry.has_tool(&ToolName::namespaced("codex_app", "automation_list"))); } #[test] @@ -1865,9 +1865,9 @@ fn search_tool_is_hidden_for_deferred_dynamic_tools_when_namespace_tools_are_dis assert_lacks_tool_name(&tools, TOOL_SEARCH_TOOL_NAME); assert_lacks_tool_name(&tools, "codex_app"); assert_lacks_tool_name(&tools, "plain_dynamic"); - assert!(!registry.has_handler(&ToolName::plain(TOOL_SEARCH_TOOL_NAME))); - assert!(registry.has_handler(&ToolName::namespaced("codex_app", "automation_update"))); - assert!(registry.has_handler(&ToolName::plain("plain_dynamic"))); + assert!(!registry.has_tool(&ToolName::plain(TOOL_SEARCH_TOOL_NAME))); + assert!(registry.has_tool(&ToolName::namespaced("codex_app", "automation_update"))); + assert!(registry.has_tool(&ToolName::plain("plain_dynamic"))); } #[test] @@ -2007,7 +2007,7 @@ fn request_plugin_install_description_lists_discoverable_tools() { /*extension_tool_executors*/ &[], &[], ); - assert!(registry.has_handler(&ToolName::plain(REQUEST_PLUGIN_INSTALL_TOOL_NAME))); + assert!(registry.has_tool(&ToolName::plain(REQUEST_PLUGIN_INSTALL_TOOL_NAME))); let request_plugin_install = find_tool(&tools, REQUEST_PLUGIN_INSTALL_TOOL_NAME); let ToolSpec::Function(ResponsesApiTool { @@ -2109,18 +2109,19 @@ fn code_mode_augments_mcp_tool_descriptions_with_namespaced_sample() { &[], ); - let ResponsesApiTool { description, .. } = - find_namespace_function_tool(&tools, "mcp__sample__", "echo"); + let ToolSpec::Freeform(FreeformTool { description, .. }) = find_tool(&tools, "exec") else { + panic!("expected freeform tool"); + }; - assert_eq!( - description, - r#"Echo text + assert!(description.contains( + r#"### `mcp__sample__echo` +Echo text exec tool declaration: ```ts declare const tools: { mcp__sample__echo(args: { message: string; }): Promise; }; ```"# - ); + )); } #[test] @@ -2413,7 +2414,7 @@ fn build_specs_with_inputs_for_test( mcp_tools: Option>, deferred_mcp_tools: Option>, discoverable_tools: Option>, - extension_tool_executors: &[Arc], + extension_tool_executors: &[Arc>], dynamic_tools: &[DynamicToolSpec], ) -> (Vec, ToolRegistry) { let mcp_tool_inputs = mcp_tools.as_ref().map(|mcp_tools| { @@ -2431,13 +2432,10 @@ fn build_specs_with_inputs_for_test( default_agent_type_description: DEFAULT_AGENT_TYPE_DESCRIPTION, wait_agent_timeouts: wait_agent_timeout_options(), }; - let executors = collect_tool_executors(config, params); - let builder = build_tool_registry_builder_from_executors( - config, - executors, - hosted_model_tool_specs(config), - ); - builder.build() + let mut executors = collect_tool_executors(config, params); + append_tool_search_executor(config, &mut executors); + prepend_code_mode_executors(config, &mut executors); + build_model_visible_specs_and_registry(config, executors, hosted_model_tool_specs(config)) } fn mcp_tool(name: &str, description: &str, input_schema: serde_json::Value) -> rmcp::model::Tool { @@ -2539,18 +2537,19 @@ fn code_mode_augments_mcp_tool_descriptions_with_structured_output_sample() { &[], ); - let ResponsesApiTool { description, .. } = - find_namespace_function_tool(&tools, "mcp__sample__", "echo"); + let ToolSpec::Freeform(FreeformTool { description, .. }) = find_tool(&tools, "exec") else { + panic!("expected freeform tool"); + }; - assert_eq!( - description, - r#"Echo text + assert!(description.contains( + r#"### `mcp__sample__echo` +Echo text exec tool declaration: ```ts declare const tools: { mcp__sample__echo(args: { message: string; }): Promise>; }; ```"# - ); + )); } fn discoverable_connector(id: &str, name: &str, description: &str) -> DiscoverableTool { diff --git a/codex-rs/core/src/tools/spec_plan_types.rs b/codex-rs/core/src/tools/spec_plan_types.rs deleted file mode 100644 index 247fa05715..0000000000 --- a/codex-rs/core/src/tools/spec_plan_types.rs +++ /dev/null @@ -1,29 +0,0 @@ -use crate::tools::handlers::multi_agents_spec::WaitAgentTimeoutOptions; -use codex_extension_api::ExtensionToolExecutor; -use codex_mcp::ToolInfo; -use codex_protocol::dynamic_tools::DynamicToolSpec; -use codex_tools::DiscoverableTool; -use codex_tools::ToolsConfig; -use std::sync::Arc; - -#[derive(Clone, Copy)] -pub struct ToolRegistryBuildParams<'a> { - pub mcp_tools: Option<&'a [ToolInfo]>, - pub deferred_mcp_tools: Option<&'a [ToolInfo]>, - pub discoverable_tools: Option<&'a [DiscoverableTool]>, - pub extension_tool_executors: &'a [Arc], - pub dynamic_tools: &'a [DynamicToolSpec], - pub default_agent_type_description: &'a str, - pub wait_agent_timeouts: WaitAgentTimeoutOptions, -} - -pub(crate) fn agent_type_description( - config: &ToolsConfig, - default_agent_type_description: &str, -) -> String { - if config.agent_type_description.is_empty() { - default_agent_type_description.to_string() - } else { - config.agent_type_description.clone() - } -} diff --git a/codex-rs/core/src/tools/tool_dispatch_trace_tests.rs b/codex-rs/core/src/tools/tool_dispatch_trace_tests.rs index cb0eff9ca9..bccaf60c82 100644 --- a/codex-rs/core/src/tools/tool_dispatch_trace_tests.rs +++ b/codex-rs/core/src/tools/tool_dispatch_trace_tests.rs @@ -21,8 +21,8 @@ use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolCallSource; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; +use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolRegistry; use crate::turn_diff_tracker::TurnDiffTracker; @@ -32,18 +32,22 @@ struct TestHandler { #[async_trait::async_trait] impl ToolExecutor for TestHandler { - type Output = FunctionToolOutput; - fn tool_name(&self) -> codex_tools::ToolName { self.tool_name.clone() } - async fn handle(&self, _invocation: ToolInvocation) -> Result { - Ok(FunctionToolOutput::from_text("ok".to_string(), Some(true))) + async fn handle( + &self, + _invocation: ToolInvocation, + ) -> Result, FunctionCallError> { + Ok(Box::new(FunctionToolOutput::from_text( + "ok".to_string(), + Some(true), + ))) } } -impl ToolHandler for TestHandler {} +impl CoreToolRuntime for TestHandler {} #[tokio::test] async fn dispatch_lifecycle_trace_records_direct_and_code_mode_requesters() -> anyhow::Result<()> { diff --git a/codex-rs/core/src/turn_timing.rs b/codex-rs/core/src/turn_timing.rs index 3d35afae95..570e4a6e32 100644 --- a/codex-rs/core/src/turn_timing.rs +++ b/codex-rs/core/src/turn_timing.rs @@ -185,6 +185,7 @@ fn response_item_records_turn_ttft(item: &ResponseItem) -> bool { | ResponseItem::ImageGenerationCall { .. } | ResponseItem::Compaction { .. } | ResponseItem::ContextCompaction { .. } => true, + ResponseItem::CompactionTrigger => false, ResponseItem::FunctionCallOutput { .. } | ResponseItem::CustomToolCallOutput { .. } | ResponseItem::ToolSearchOutput { .. } diff --git a/codex-rs/core/tests/suite/client_websockets.rs b/codex-rs/core/tests/suite/client_websockets.rs index 030aecd29e..a14ee6518f 100755 --- a/codex-rs/core/tests/suite/client_websockets.rs +++ b/codex-rs/core/tests/suite/client_websockets.rs @@ -253,7 +253,7 @@ async fn responses_websocket_sends_response_processed_after_remote_compaction_v2 json!({ "type": "response.output_item.done", "item": { - "type": "context_compaction", + "type": "compaction", "encrypted_content": "ENCRYPTED_CONTEXT_COMPACTION_SUMMARY", } }), diff --git a/codex-rs/core/tests/suite/compact_remote.rs b/codex-rs/core/tests/suite/compact_remote.rs index 8849d80773..720ae10599 100644 --- a/codex-rs/core/tests/suite/compact_remote.rs +++ b/codex-rs/core/tests/suite/compact_remote.rs @@ -697,7 +697,7 @@ async fn remote_manual_compact_chatgpt_auth_reuses_service_tier_and_prompt_cache } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn remote_compact_v2_reuses_context_compaction_for_followups() -> Result<()> { +async fn remote_compact_v2_reuses_compaction_trigger_for_followups() -> Result<()> { skip_if_no_network!(Ok(())); let harness = TestCodexHarness::with_builder( @@ -721,7 +721,7 @@ async fn remote_compact_v2_reuses_context_compaction_for_followups() -> Result<( serde_json::json!({ "type": "response.output_item.done", "item": { - "type": "context_compaction", + "type": "compaction", "encrypted_content": "ENCRYPTED_CONTEXT_COMPACTION_SUMMARY", } }), @@ -778,8 +778,8 @@ async fn remote_compact_v2_reuses_context_compaction_for_followups() -> Result<( assert_eq!(compact_request.path(), "/v1/responses"); let compact_body = compact_request.body_json().to_string(); assert!( - compact_body.contains("\"type\":\"context_compaction\""), - "expected v2 compaction request to include the context_compaction trigger item" + compact_body.contains("\"type\":\"compaction_trigger\""), + "expected v2 compaction request to include the compaction_trigger item" ); assert!( !compact_body.contains("ENCRYPTED_CONTEXT_COMPACTION_SUMMARY"), @@ -789,12 +789,12 @@ async fn remote_compact_v2_reuses_context_compaction_for_followups() -> Result<( let follow_up_request = response_requests.last().expect("follow-up request missing"); let follow_up_body = follow_up_request.body_json().to_string(); assert!( - follow_up_body.contains("\"type\":\"context_compaction\""), - "expected follow-up request to preserve the v2 context_compaction item" + follow_up_body.contains("\"type\":\"compaction\""), + "expected follow-up request to preserve the compaction item" ); assert!( follow_up_body.contains("ENCRYPTED_CONTEXT_COMPACTION_SUMMARY"), - "expected follow-up request to include the context compaction payload" + "expected follow-up request to include the compaction payload" ); assert!( follow_up_body.contains("hello remote compact"), @@ -805,8 +805,7 @@ async fn remote_compact_v2_reuses_context_compaction_for_followups() -> Result<( } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn remote_compact_v2_accepts_additional_output_items_before_context_compaction() -> Result<()> -{ +async fn remote_compact_v2_accepts_additional_output_items_before_compaction() -> Result<()> { skip_if_no_network!(Ok(())); let harness = TestCodexHarness::with_builder( @@ -831,7 +830,7 @@ async fn remote_compact_v2_accepts_additional_output_items_before_context_compac serde_json::json!({ "type": "response.output_item.done", "item": { - "type": "context_compaction", + "type": "compaction", "encrypted_content": "ENCRYPTED_CONTEXT_COMPACTION_SUMMARY", } }), @@ -878,12 +877,12 @@ async fn remote_compact_v2_accepts_additional_output_items_before_context_compac let follow_up_request = response_requests.last().expect("follow-up request missing"); let follow_up_body = follow_up_request.body_json().to_string(); assert!( - follow_up_body.contains("\"type\":\"context_compaction\""), - "expected follow-up request to preserve the v2 context_compaction item" + follow_up_body.contains("\"type\":\"compaction\""), + "expected follow-up request to preserve the compaction item" ); assert!( follow_up_body.contains("ENCRYPTED_CONTEXT_COMPACTION_SUMMARY"), - "expected follow-up request to include the context compaction payload" + "expected follow-up request to include the compaction payload" ); assert!( !follow_up_body.contains("IGNORED_COMPACT_REPLY"), diff --git a/codex-rs/core/tests/suite/compact_remote_parity.rs b/codex-rs/core/tests/suite/compact_remote_parity.rs new file mode 100644 index 0000000000..96616fa9cb --- /dev/null +++ b/codex-rs/core/tests/suite/compact_remote_parity.rs @@ -0,0 +1,1160 @@ +#![allow(clippy::expect_used)] + +use std::fs; +use std::path::Path; +use std::path::PathBuf; + +use anyhow::Result; +use codex_features::Feature; +use codex_login::CodexAuth; +use codex_protocol::config_types::ServiceTier; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::Op; +use codex_protocol::protocol::RolloutItem; +use codex_protocol::protocol::RolloutLine; +use codex_protocol::user_input::UserInput; +use core_test_support::hooks::trust_discovered_hooks; +use core_test_support::responses; +use core_test_support::responses::ResponseMock; +use core_test_support::skip_if_no_network; +use core_test_support::test_codex::TestCodexHarness; +use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event; +use pretty_assertions::assert_eq; +use serde_json::Value; +use serde_json::json; + +const FIXED_CWD: &str = "/tmp/codex_remote_compaction_parity_workspace"; +const IMAGE_URL: &str = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+/p9sAAAAASUVORK5CYII="; +const SUMMARY: &str = "REMOTE_COMPACTION_PARITY_ENCRYPTED_SUMMARY"; +const DUMMY_FUNCTION_NAME: &str = "test_tool"; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum Mode { + Legacy, + V2, +} + +#[derive(Clone, Copy, Debug)] +enum AuthCase { + ChatGpt, + ApiKey, +} + +impl AuthCase { + fn build(self) -> CodexAuth { + match self { + AuthCase::ChatGpt => CodexAuth::create_dummy_chatgpt_auth_for_testing(), + AuthCase::ApiKey => CodexAuth::from_api_key("dummy"), + } + } +} + +#[derive(Clone, Copy, Debug)] +struct RunSettings { + auth: AuthCase, + service_tier_fast: bool, +} + +impl Default for RunSettings { + fn default() -> Self { + Self { + auth: AuthCase::ChatGpt, + service_tier_fast: false, + } + } +} + +#[derive(Clone, Copy, Debug)] +enum Step { + Assistant, + ReasoningAssistant, + FunctionTool, + ShellTool, + ImageAssistant, + WebSearchAssistant, +} + +impl Step { + fn label(self) -> &'static str { + match self { + Step::Assistant => "assistant", + Step::ReasoningAssistant => "reasoning_assistant", + Step::FunctionTool => "function_tool", + Step::ShellTool => "shell_tool", + Step::ImageAssistant => "image_assistant", + Step::WebSearchAssistant => "web_search_assistant", + } + } +} + +#[derive(Debug)] +struct Scenario { + name: &'static str, + steps: &'static [Step], +} + +#[derive(Debug)] +struct Capture { + compact_body: Value, + follow_up_body: Value, + replacement_history: Value, + normal_response_requests: usize, + compact_requests: usize, +} + +const ASSISTANT_ONLY: &[Step] = &[Step::Assistant]; +const REASONING_IMAGE: &[Step] = &[Step::ReasoningAssistant, Step::ImageAssistant]; +const TOOL_MIX: &[Step] = &[Step::Assistant, Step::FunctionTool, Step::ShellTool]; +const FULL_MIX: &[Step] = &[ + Step::ReasoningAssistant, + Step::FunctionTool, + Step::ImageAssistant, + Step::ShellTool, + Step::WebSearchAssistant, + Step::Assistant, +]; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn remote_compaction_parity_manual_transcripts() -> Result<()> { + skip_if_no_network!(Ok(())); + + let scenarios = [ + Scenario { + name: "assistant_only", + steps: ASSISTANT_ONLY, + }, + Scenario { + name: "reasoning_image", + steps: REASONING_IMAGE, + }, + Scenario { + name: "tool_mix", + steps: TOOL_MIX, + }, + Scenario { + name: "full_mix", + steps: FULL_MIX, + }, + ]; + + for scenario in scenarios { + compare_manual_scenario(&scenario, RunSettings::default()).await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn remote_compaction_parity_v2_api_key_sends_service_tier_upgrade() -> Result<()> { + skip_if_no_network!(Ok(())); + + let scenario = Scenario { + name: "api_key_service_tier", + steps: TOOL_MIX, + }; + let settings = RunSettings { + auth: AuthCase::ApiKey, + service_tier_fast: true, + }; + let legacy = run_manual_session(&scenario, Mode::Legacy, settings).await?; + let v2 = run_manual_session(&scenario, Mode::V2, settings).await?; + + assert_eq!( + legacy.compact_body.get("service_tier"), + None, + "legacy /responses/compact should continue omitting service_tier for API-key auth" + ); + assert_eq!( + v2.compact_body.get("service_tier").and_then(Value::as_str), + Some(ServiceTier::Fast.request_value()), + "v2 compaction should send service_tier through /responses for API-key auth" + ); + + assert_compact_requests_eq_except_v2_service_tier("api-key service tier", &legacy, &v2); + assert_follow_up_and_history_eq("api-key service tier", &legacy, &v2); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn remote_compaction_parity_manual_hooks() -> Result<()> { + skip_if_no_network!(Ok(())); + + let legacy = run_manual_hook_session(Mode::Legacy).await?; + let v2 = run_manual_hook_session(Mode::V2).await?; + assert_json_eq("manual compact hook payload parity mismatch", &legacy, &v2); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn remote_compaction_parity_pre_turn_auto() -> Result<()> { + skip_if_no_network!(Ok(())); + + let legacy = run_pre_turn_auto_session(Mode::Legacy).await?; + let v2 = run_pre_turn_auto_session(Mode::V2).await?; + assert_capture_eq("pre-turn auto", &legacy, &v2); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn remote_compaction_parity_mid_turn_auto() -> Result<()> { + skip_if_no_network!(Ok(())); + + let legacy = run_mid_turn_auto_session(Mode::Legacy).await?; + let v2 = run_mid_turn_auto_session(Mode::V2).await?; + assert_capture_eq("mid-turn auto", &legacy, &v2); + Ok(()) +} + +async fn compare_manual_scenario(scenario: &Scenario, settings: RunSettings) -> Result<()> { + let legacy = run_manual_session(scenario, Mode::Legacy, settings).await?; + let v2 = run_manual_session(scenario, Mode::V2, settings).await?; + assert_capture_eq(scenario.name, &legacy, &v2); + Ok(()) +} + +fn assert_capture_eq(label: &str, legacy: &Capture, v2: &Capture) { + assert_eq!( + legacy.compact_requests, 1, + "legacy compact endpoint should be called exactly once for {label}", + ); + assert_eq!( + v2.compact_requests, 0, + "v2 should not call /responses/compact for {label}", + ); + + let legacy_compact = compact_request_view(&legacy.compact_body, Mode::Legacy); + let v2_compact = compact_request_view(&v2.compact_body, Mode::V2); + assert_json_eq( + &format!("compact request parity mismatch for {label}"), + &legacy_compact, + &v2_compact, + ); + + let legacy_follow_up = follow_up_request_view(&legacy.follow_up_body); + let v2_follow_up = follow_up_request_view(&v2.follow_up_body); + assert_json_eq( + &format!("post-compact follow-up request parity mismatch for {label}"), + &legacy_follow_up, + &v2_follow_up, + ); + + assert_json_eq( + &format!("replacement history parity mismatch for {label}"), + &legacy.replacement_history, + &v2.replacement_history, + ); + + println!( + "PARITY_OK scenario={} normal_response_requests={} compact_input_items={} replacement_history_items={} follow_up_input_items={}", + label, + legacy.normal_response_requests, + compact_input_len(&legacy.compact_body, Mode::Legacy), + replacement_history_len(&legacy.replacement_history), + follow_up_input_len(&legacy.follow_up_body) + ); +} + +fn assert_compact_requests_eq_except_v2_service_tier(label: &str, legacy: &Capture, v2: &Capture) { + assert_eq!( + legacy.compact_requests, 1, + "legacy compact endpoint should be called exactly once for {label}", + ); + assert_eq!( + v2.compact_requests, 0, + "v2 should not call /responses/compact for {label}", + ); + + let legacy_compact = compact_request_view(&legacy.compact_body, Mode::Legacy); + let mut v2_compact = compact_request_view(&v2.compact_body, Mode::V2); + remove_object_field(&mut v2_compact, "service_tier"); + assert_json_eq( + &format!("compact request parity mismatch for {label} after service_tier upgrade"), + &legacy_compact, + &v2_compact, + ); +} + +fn assert_follow_up_and_history_eq(label: &str, legacy: &Capture, v2: &Capture) { + let legacy_follow_up = follow_up_request_view(&legacy.follow_up_body); + let v2_follow_up = follow_up_request_view(&v2.follow_up_body); + assert_json_eq( + &format!("post-compact follow-up request parity mismatch for {label}"), + &legacy_follow_up, + &v2_follow_up, + ); + + assert_json_eq( + &format!("replacement history parity mismatch for {label}"), + &legacy.replacement_history, + &v2.replacement_history, + ); +} + +async fn run_manual_session( + scenario: &Scenario, + mode: Mode, + settings: RunSettings, +) -> Result { + let mut response_bodies = response_bodies_for_scenario(scenario); + if mode == Mode::V2 { + response_bodies.push(compaction_v2_response_body()); + } + response_bodies.push(after_compact_response_body(scenario.name)); + + let harness = build_harness(mode, settings, /*hooks*/ false).await?; + let rollout_path = rollout_path(&harness); + let codex = harness.test().codex.clone(); + + let responses_mock = responses::mount_sse_sequence(harness.server(), response_bodies).await; + let compact_mock = mount_legacy_compact_if_needed(&harness, mode).await; + + for (idx, step) in scenario.steps.iter().enumerate() { + submit_user_input(&codex, user_input_for_step(scenario.name, idx, *step)).await?; + } + + codex.submit(Op::Compact).await?; + wait_for_turn_complete(&codex).await; + + submit_user_input( + &codex, + vec![UserInput::Text { + text: format!("{}_AFTER_COMPACT_USER", scenario.name), + text_elements: Vec::new(), + }], + ) + .await?; + + capture_from_requests( + mode, + &codex, + &rollout_path, + &responses_mock, + compact_mock.as_ref(), + follow_up_index(responses_mock.requests().len()), + ) + .await +} + +async fn run_pre_turn_auto_session(mode: Mode) -> Result { + let response_bodies = match mode { + Mode::Legacy => vec![ + responses::sse(vec![ + responses::ev_assistant_message("pre-turn-first-message", "PRE_TURN_FIRST_REPLY"), + responses::ev_completed_with_tokens( + "pre-turn-first-response", + /*total_tokens*/ 500, + ), + ]), + after_compact_response_body("pre_turn_auto"), + ], + Mode::V2 => vec![ + responses::sse(vec![ + responses::ev_assistant_message("pre-turn-first-message", "PRE_TURN_FIRST_REPLY"), + responses::ev_completed_with_tokens( + "pre-turn-first-response", + /*total_tokens*/ 500, + ), + ]), + compaction_v2_response_body(), + after_compact_response_body("pre_turn_auto"), + ], + }; + let harness = build_auto_harness(mode).await?; + let rollout_path = rollout_path(&harness); + let codex = harness.test().codex.clone(); + let responses_mock = responses::mount_sse_sequence(harness.server(), response_bodies).await; + let compact_mock = mount_legacy_compact_if_needed(&harness, mode).await; + + submit_user_input( + &codex, + vec![UserInput::Text { + text: "pre_turn_auto_before".to_string(), + text_elements: Vec::new(), + }], + ) + .await?; + submit_user_input( + &codex, + vec![UserInput::Text { + text: "pre_turn_auto_after".to_string(), + text_elements: Vec::new(), + }], + ) + .await?; + + capture_from_requests( + mode, + &codex, + &rollout_path, + &responses_mock, + compact_mock.as_ref(), + follow_up_index(responses_mock.requests().len()), + ) + .await +} + +async fn run_mid_turn_auto_session(mode: Mode) -> Result { + let response_bodies = match mode { + Mode::Legacy => vec![ + responses::sse(vec![ + responses::ev_function_call("mid-turn-call", DUMMY_FUNCTION_NAME, "{}"), + responses::ev_completed_with_tokens( + "mid-turn-call-response", + /*total_tokens*/ 500, + ), + ]), + after_compact_response_body("mid_turn_auto"), + ], + Mode::V2 => vec![ + responses::sse(vec![ + responses::ev_function_call("mid-turn-call", DUMMY_FUNCTION_NAME, "{}"), + responses::ev_completed_with_tokens( + "mid-turn-call-response", + /*total_tokens*/ 500, + ), + ]), + compaction_v2_response_body(), + after_compact_response_body("mid_turn_auto"), + ], + }; + let harness = build_auto_harness(mode).await?; + let rollout_path = rollout_path(&harness); + let codex = harness.test().codex.clone(); + let responses_mock = responses::mount_sse_sequence(harness.server(), response_bodies).await; + let compact_mock = mount_legacy_compact_if_needed(&harness, mode).await; + + submit_user_input( + &codex, + vec![UserInput::Text { + text: "mid_turn_auto_user".to_string(), + text_elements: Vec::new(), + }], + ) + .await?; + + capture_from_requests( + mode, + &codex, + &rollout_path, + &responses_mock, + compact_mock.as_ref(), + follow_up_index(responses_mock.requests().len()), + ) + .await +} + +async fn run_manual_hook_session(mode: Mode) -> Result { + let response_bodies = match mode { + Mode::Legacy => vec![responses::sse(vec![ + responses::ev_assistant_message("hook-first-message", "HOOK_FIRST_REPLY"), + responses::ev_completed("hook-first-response"), + ])], + Mode::V2 => vec![ + responses::sse(vec![ + responses::ev_assistant_message("hook-first-message", "HOOK_FIRST_REPLY"), + responses::ev_completed("hook-first-response"), + ]), + compaction_v2_response_body(), + ], + }; + let harness = build_harness(mode, RunSettings::default(), /*hooks*/ true).await?; + let codex = harness.test().codex.clone(); + responses::mount_sse_sequence(harness.server(), response_bodies).await; + let compact_mock = mount_legacy_compact_if_needed(&harness, mode).await; + + submit_user_input( + &codex, + vec![UserInput::Text { + text: "manual_hooks_before".to_string(), + text_elements: Vec::new(), + }], + ) + .await?; + codex.submit(Op::Compact).await?; + wait_for_turn_complete(&codex).await; + + if let Some(compact_mock) = compact_mock { + assert_eq!(compact_mock.requests().len(), 1); + } + + let home = harness.test().codex_home_path(); + let pre = hook_log_view(&home.join("pre_compact_manual_log.jsonl"))?; + let post = hook_log_view(&home.join("post_compact_manual_log.jsonl"))?; + Ok(json!({ + "pre": pre, + "post": post, + })) +} + +async fn build_auto_harness(mode: Mode) -> Result { + build_harness_inner( + mode, + RunSettings::default(), + /*hooks*/ false, + Some(200), + ) + .await +} + +async fn build_harness(mode: Mode, settings: RunSettings, hooks: bool) -> Result { + build_harness_inner(mode, settings, hooks, /*auto_compact_limit*/ None).await +} + +async fn build_harness_inner( + mode: Mode, + settings: RunSettings, + hooks: bool, + auto_compact_limit: Option, +) -> Result { + fs::create_dir_all(FIXED_CWD)?; + let mut builder = test_codex().with_auth(settings.auth.build()); + if hooks { + builder = builder.with_pre_build_hook(write_manual_compact_hooks); + } + TestCodexHarness::with_builder(builder.with_config(move |config| { + config.cwd = codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(PathBuf::from( + FIXED_CWD, + )) + .expect("fixed cwd should be absolute"); + config.user_instructions = Some("PARITY_USER_INSTRUCTIONS".to_string()); + config.developer_instructions = Some("PARITY_DEVELOPER_INSTRUCTIONS".to_string()); + if settings.service_tier_fast { + config.service_tier = Some(ServiceTier::Fast.request_value().to_string()); + } + config.model_auto_compact_token_limit = auto_compact_limit; + if hooks { + trust_discovered_hooks(config); + } + if mode == Mode::V2 { + let _ = config.features.enable(Feature::RemoteCompactionV2); + } + })) + .await +} + +fn rollout_path(harness: &TestCodexHarness) -> PathBuf { + harness + .test() + .session_configured + .rollout_path + .clone() + .expect("rollout path") +} + +async fn mount_legacy_compact_if_needed( + harness: &TestCodexHarness, + mode: Mode, +) -> Option { + match mode { + Mode::Legacy => Some( + responses::mount_compact_user_history_with_summary_once(harness.server(), SUMMARY) + .await, + ), + Mode::V2 => None, + } +} + +fn follow_up_index(request_count: usize) -> usize { + request_count.checked_sub(1).expect("follow-up request") +} + +async fn capture_from_requests( + mode: Mode, + codex: &codex_core::CodexThread, + rollout_path: &Path, + responses_mock: &ResponseMock, + compact_mock: Option<&ResponseMock>, + follow_up_index: usize, +) -> Result { + let response_requests = responses_mock.requests(); + let follow_up_body = response_requests + .get(follow_up_index) + .expect("follow-up request should be present") + .body_json(); + + let (compact_body, compact_requests) = match (mode, compact_mock) { + (Mode::Legacy, Some(compact_mock)) => { + let compact_requests = compact_mock.requests().len(); + (compact_mock.single_request().body_json(), compact_requests) + } + (Mode::V2, None) => { + let compact_index = follow_up_index + .checked_sub(1) + .expect("v2 compact request should precede follow-up"); + (response_requests[compact_index].body_json(), 0) + } + (Mode::Legacy, None) | (Mode::V2, Some(_)) => panic!("unexpected compact mock state"), + }; + + codex.submit(Op::Shutdown).await?; + wait_for_event(codex, |ev| matches!(ev, EventMsg::ShutdownComplete)).await; + + Ok(Capture { + compact_body, + follow_up_body, + replacement_history: replacement_history_from_rollout(rollout_path)?, + normal_response_requests: response_requests.len(), + compact_requests, + }) +} + +async fn submit_user_input(codex: &codex_core::CodexThread, items: Vec) -> Result<()> { + codex + .submit(Op::UserInput { + environments: None, + items, + final_output_json_schema: None, + responsesapi_client_metadata: None, + }) + .await?; + wait_for_turn_complete(codex).await; + Ok(()) +} + +async fn wait_for_turn_complete(codex: &codex_core::CodexThread) { + wait_for_event(codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; +} + +fn user_input_for_step(scenario_name: &str, idx: usize, step: Step) -> Vec { + let mut items = Vec::new(); + if matches!(step, Step::ImageAssistant) { + items.push(UserInput::Image { + image_url: IMAGE_URL.to_string(), + }); + } + items.push(UserInput::Text { + text: format!("{}_USER_TURN_{}_{}", scenario_name, idx, step.label()), + text_elements: Vec::new(), + }); + items +} + +fn response_bodies_for_scenario(scenario: &Scenario) -> Vec { + scenario + .steps + .iter() + .enumerate() + .flat_map(|(idx, step)| response_bodies_for_step(scenario.name, idx, *step)) + .collect() +} + +fn response_bodies_for_step(scenario_name: &str, idx: usize, step: Step) -> Vec { + let response_id = format!("{scenario_name}-{idx}-{}", step.label()); + match step { + Step::Assistant => vec![responses::sse(vec![ + responses::ev_assistant_message( + &format!("{response_id}-message"), + &format!("{scenario_name} assistant reply {idx}"), + ), + responses::ev_completed(&format!("{response_id}-response")), + ])], + Step::ReasoningAssistant => vec![responses::sse(vec![ + responses::ev_reasoning_item( + &format!("{response_id}-reasoning"), + &["PARITY_REASONING_SUMMARY"], + &["parity raw reasoning content"], + ), + responses::ev_assistant_message( + &format!("{response_id}-message"), + &format!("{scenario_name} reasoning reply {idx}"), + ), + responses::ev_completed(&format!("{response_id}-response")), + ])], + Step::FunctionTool => vec![ + responses::sse(vec![ + responses::ev_function_call( + &format!("{response_id}-call"), + DUMMY_FUNCTION_NAME, + r#"{"case":"parity"}"#, + ), + responses::ev_completed(&format!("{response_id}-tool-response")), + ]), + responses::sse(vec![ + responses::ev_assistant_message( + &format!("{response_id}-final-message"), + &format!("{scenario_name} function follow-up {idx}"), + ), + responses::ev_completed(&format!("{response_id}-final-response")), + ]), + ], + Step::ShellTool => vec![ + responses::sse(vec![ + responses::ev_shell_command_call( + &format!("{response_id}-shell-call"), + &format!("echo {scenario_name}_{idx}_SHELL_TOOL"), + ), + responses::ev_completed(&format!("{response_id}-shell-response")), + ]), + responses::sse(vec![ + responses::ev_assistant_message( + &format!("{response_id}-final-message"), + &format!("{scenario_name} shell follow-up {idx}"), + ), + responses::ev_completed(&format!("{response_id}-final-response")), + ]), + ], + Step::ImageAssistant => vec![responses::sse(vec![ + responses::ev_assistant_message( + &format!("{response_id}-message"), + &format!("{scenario_name} image reply {idx}"), + ), + responses::ev_completed(&format!("{response_id}-response")), + ])], + Step::WebSearchAssistant => vec![responses::sse(vec![ + responses::ev_web_search_call_done( + &format!("{response_id}-web-search"), + "completed", + &format!("{scenario_name} parity query"), + ), + responses::ev_assistant_message( + &format!("{response_id}-message"), + &format!("{scenario_name} web search reply {idx}"), + ), + responses::ev_completed(&format!("{response_id}-response")), + ])], + } +} + +fn compaction_v2_response_body() -> String { + responses::sse(vec![ + json!({ + "type": "response.output_item.done", + "item": { + "type": "compaction", + "encrypted_content": SUMMARY, + } + }), + responses::ev_completed("remote-compaction-v2-response"), + ]) +} + +fn after_compact_response_body(scenario_name: &str) -> String { + responses::sse(vec![ + responses::ev_assistant_message( + &format!("{scenario_name}-after-compact-message"), + &format!("{scenario_name} after compact reply"), + ), + responses::ev_completed(&format!("{scenario_name}-after-compact-response")), + ]) +} + +fn compact_request_view(body: &Value, mode: Mode) -> Value { + let mut input = body + .get("input") + .and_then(Value::as_array) + .cloned() + .expect("compact request should include input"); + if mode == Mode::V2 { + let trigger = input + .pop() + .expect("v2 compact input should end with trigger"); + assert_eq!( + trigger, + json!({"type": "compaction_trigger"}), + "v2 compact input should append exactly one compaction_trigger" + ); + } + + let mut selected = selected_request_fields(body, SelectedFieldsMode::Compact); + selected["input"] = normalize_value(Value::Array(input)); + canonical_json(&normalize_value(selected)) +} + +fn follow_up_request_view(body: &Value) -> Value { + let mut selected = selected_request_fields(body, SelectedFieldsMode::FollowUp); + selected["input"] = normalize_value( + body.get("input") + .cloned() + .expect("follow-up request should include input"), + ); + canonical_json(&normalize_value(selected)) +} + +fn replacement_history_from_rollout(path: &Path) -> Result { + let rollout_text = fs::read_to_string(path)?; + let mut replacement_history = None; + for line in rollout_text + .lines() + .map(str::trim) + .filter(|line| !line.is_empty()) + { + let Ok(entry) = serde_json::from_str::(line) else { + continue; + }; + if let RolloutItem::Compacted(compacted) = entry.item + && compacted.message.is_empty() + && let Some(items) = compacted.replacement_history + { + let values = items + .into_iter() + .map(|item| serde_json::to_value(item).expect("serialize replacement item")) + .collect::>(); + replacement_history = Some(Value::Array(values)); + } + } + let replacement_history = + replacement_history.expect("expected compacted rollout replacement history"); + Ok(canonical_json(&normalize_value(replacement_history))) +} + +fn write_manual_compact_hooks(home: &Path) { + write_hook_script( + &home.join("pre_compact_manual.py"), + &home.join("pre_compact_manual_log.jsonl"), + ); + write_hook_script( + &home.join("post_compact_manual.py"), + &home.join("post_compact_manual_log.jsonl"), + ); + let hooks = json!({ + "hooks": { + "PreCompact": [{ + "matcher": "manual", + "hooks": [{ + "type": "command", + "command": python_hook_command(&home.join("pre_compact_manual.py")), + }] + }], + "PostCompact": [{ + "matcher": "manual", + "hooks": [{ + "type": "command", + "command": python_hook_command(&home.join("post_compact_manual.py")), + }] + }] + } + }); + fs::write(home.join("hooks.json"), hooks.to_string()).expect("write hooks.json"); +} + +fn write_hook_script(script_path: &Path, log_path: &Path) { + let script = format!( + r#"import json +from pathlib import Path +import sys + +payload = json.load(sys.stdin) +with Path(r"{log_path}").open("a", encoding="utf-8") as handle: + handle.write(json.dumps(payload, sort_keys=True) + "\n") +"#, + log_path = log_path.display(), + ); + fs::write(script_path, script).expect("write compact hook script"); +} + +fn python_hook_command(script_path: &Path) -> String { + format!("python3 \"{}\"", script_path.display()) +} + +fn hook_log_view(path: &Path) -> Result { + let text = fs::read_to_string(path)?; + let values = text + .lines() + .filter(|line| !line.trim().is_empty()) + .map(|line| { + let payload: Value = serde_json::from_str(line).expect("parse compact hook payload"); + json!({ + "hook_event_name": payload["hook_event_name"].clone(), + "trigger": payload["trigger"].clone(), + "model": payload["model"].clone(), + "has_reason": payload.get("reason").is_some(), + "has_phase": payload.get("phase").is_some(), + "has_implementation": payload.get("implementation").is_some(), + "has_status": payload.get("status").is_some(), + "has_error": payload.get("error").is_some(), + }) + }) + .collect::>(); + Ok(Value::Array(values)) +} + +#[derive(Clone, Copy)] +enum SelectedFieldsMode { + Compact, + FollowUp, +} + +fn selected_request_fields(body: &Value, mode: SelectedFieldsMode) -> Value { + let mut selected = serde_json::Map::new(); + let fields: &[&str] = match mode { + SelectedFieldsMode::Compact => &[ + "model", + "instructions", + "parallel_tool_calls", + "reasoning", + "service_tier", + "prompt_cache_key", + "text", + "tools", + "previous_response_id", + ], + SelectedFieldsMode::FollowUp => &[ + "model", + "instructions", + "parallel_tool_calls", + "reasoning", + "service_tier", + "prompt_cache_key", + "text", + "tools", + "tool_choice", + "previous_response_id", + "store", + "stream", + "include", + ], + }; + for field in fields { + if let Some(value) = body.get(field) { + selected.insert((*field).to_string(), normalize_value(value.clone())); + } + } + Value::Object(selected) +} + +fn normalize_value(value: Value) -> Value { + match value { + Value::String(text) => Value::String(normalize_string(&text)), + Value::Array(values) => Value::Array(values.into_iter().map(normalize_value).collect()), + Value::Object(map) => Value::Object( + map.into_iter() + .map(|(key, value)| (key, normalize_value(value))) + .collect(), + ), + Value::Null | Value::Bool(_) | Value::Number(_) => value, + } +} + +fn normalize_string(value: &str) -> String { + if is_uuid_like(value) { + return "".to_string(); + } + + let mut text = value.to_string(); + normalize_tmp_prefix_before_marker(&mut text, "/skills/"); + normalize_tmp_prefix_before_marker(&mut text, "\\skills\\"); + + let mut search_start = 0; + let wall_time_prefix = "Wall time: "; + let wall_time_suffix = " seconds"; + while let Some(relative_start) = text[search_start..].find(wall_time_prefix) { + let value_start = search_start + relative_start + wall_time_prefix.len(); + let Some(relative_end) = text[value_start..].find(wall_time_suffix) else { + break; + }; + let value_end = value_start + relative_end; + let value = &text[value_start..value_end]; + if !value.is_empty() + && value.chars().any(|ch| ch.is_ascii_digit()) + && value.chars().all(|ch| ch.is_ascii_digit() || ch == '.') + { + text.replace_range(value_start..value_end, ""); + search_start = value_start + "".len() + wall_time_suffix.len(); + } else { + search_start = value_end + wall_time_suffix.len(); + } + } + text +} + +fn is_uuid_like(value: &str) -> bool { + let bytes = value.as_bytes(); + bytes.len() == 36 + && [8, 13, 18, 23].iter().all(|idx| bytes[*idx] == b'-') + && bytes + .iter() + .enumerate() + .all(|(idx, byte)| [8, 13, 18, 23].contains(&idx) || byte.is_ascii_hexdigit()) +} + +fn normalize_tmp_prefix_before_marker(text: &mut String, marker: &str) { + let mut search_start = 0; + while let Some(relative_marker_index) = text[search_start..].find(marker) { + let marker_index = search_start + relative_marker_index; + let prefix = &text[..marker_index]; + let windows_appdata_temp_start = prefix + .rfind("/AppData/Local/Temp/.tmp") + .and_then(|temp_index| prefix[..temp_index].rfind(":/Users/")) + .and_then(|colon_index| colon_index.checked_sub(1)) + .or_else(|| { + prefix + .rfind("\\AppData\\Local\\Temp\\.tmp") + .and_then(|temp_index| prefix[..temp_index].rfind(":\\Users\\")) + .and_then(|colon_index| colon_index.checked_sub(1)) + }); + let start = prefix + .rfind("/private/var/folders/") + .or_else(|| prefix.rfind("/var/folders/")) + .or_else(|| prefix.rfind("/private/tmp/.tmp")) + .or_else(|| prefix.rfind("/tmp/.tmp")) + .or(windows_appdata_temp_start); + if let Some(start_index) = start { + text.replace_range(start_index..marker_index, ""); + search_start = start_index + "".len() + marker.len(); + } else { + search_start = marker_index + marker.len(); + } + } +} + +#[test] +fn normalize_string_rewrites_linux_temp_skill_paths() { + let text = normalize_string( + "file: /tmp/.tmp5YYdK3/skills/.system/imagegen/SKILL.md and \ + /private/tmp/.tmpw3wqF9/skills/custom/SKILL.md", + ); + + assert_eq!( + text, + "file: /skills/.system/imagegen/SKILL.md and \ + /skills/custom/SKILL.md" + ); +} + +#[test] +fn normalize_string_rewrites_windows_temp_skill_paths() { + let text = normalize_string( + "file: C:/Users/runneradmin/AppData/Local/Temp/.tmpDuYxa3/skills/.system/imagegen/SKILL.md and \ + C:\\Users\\runneradmin\\AppData\\Local\\Temp\\.tmpiP36Yr\\skills\\custom\\SKILL.md", + ); + + assert_eq!( + text, + "file: /skills/.system/imagegen/SKILL.md and \ + \\skills\\custom\\SKILL.md" + ); +} + +#[test] +fn normalize_string_rewrites_shell_wall_times() { + let text = normalize_string( + "Exit code: 0\nWall time: 0 seconds\nOutput:\nok\n\ + Exit code: 0\nWall time: 0.1 seconds\nOutput:\nok", + ); + + assert_eq!( + text, + "Exit code: 0\nWall time: seconds\nOutput:\nok\n\ + Exit code: 0\nWall time: seconds\nOutput:\nok" + ); +} + +fn canonical_json(value: &Value) -> Value { + match value { + Value::Object(map) => { + let mut entries = map.iter().collect::>(); + entries.sort_by(|(left_key, _), (right_key, _)| left_key.cmp(right_key)); + Value::Object( + entries + .into_iter() + .map(|(key, value)| (key.clone(), canonical_json(value))) + .collect(), + ) + } + Value::Array(values) => Value::Array(values.iter().map(canonical_json).collect()), + Value::Null | Value::Bool(_) | Value::Number(_) | Value::String(_) => value.clone(), + } +} + +fn remove_object_field(value: &mut Value, field: &str) { + if let Value::Object(map) = value { + map.remove(field); + } +} + +fn assert_json_eq(label: &str, left: &Value, right: &Value) { + if left != right { + panic!("{}\n{}", label, first_json_diff(left, right, "$")); + } +} + +fn first_json_diff(left: &Value, right: &Value, path: &str) -> String { + match (left, right) { + (Value::Object(left_map), Value::Object(right_map)) => { + let mut keys = left_map.keys().chain(right_map.keys()).collect::>(); + keys.sort(); + keys.dedup(); + for key in keys { + match (left_map.get(key), right_map.get(key)) { + (Some(left_value), Some(right_value)) if left_value != right_value => { + return first_json_diff(left_value, right_value, &format!("{path}.{key}")); + } + (None, Some(right_value)) => { + return format!( + "{path}.{key}: missing on left, right={}", + short_json(right_value) + ); + } + (Some(left_value), None) => { + return format!( + "{path}.{key}: left={}, missing on right", + short_json(left_value) + ); + } + (Some(_), Some(_)) | (None, None) => {} + } + } + format!("{path}: object mismatch") + } + (Value::Array(left_values), Value::Array(right_values)) => { + let len = left_values.len().min(right_values.len()); + for idx in 0..len { + if left_values[idx] != right_values[idx] { + return first_json_diff( + &left_values[idx], + &right_values[idx], + &format!("{path}[{idx}]"), + ); + } + } + if left_values.len() != right_values.len() { + return format!( + "{path}: array len left={} right={}", + left_values.len(), + right_values.len() + ); + } + format!("{path}: array mismatch") + } + _ => format!( + "{path}: left={} right={}", + short_json(left), + short_json(right) + ), + } +} + +fn short_json(value: &Value) -> String { + let text = serde_json::to_string(value).expect("serialize short json value"); + const MAX_LEN: usize = 1000; + if text.len() <= MAX_LEN { + text + } else { + let prefix = text.chars().take(MAX_LEN).collect::(); + format!("{prefix}...<{} chars>", text.len()) + } +} + +fn compact_input_len(body: &Value, mode: Mode) -> usize { + let len = body + .get("input") + .and_then(Value::as_array) + .map(Vec::len) + .unwrap_or_default(); + match mode { + Mode::Legacy => len, + Mode::V2 => len.saturating_sub(1), + } +} + +fn follow_up_input_len(body: &Value) -> usize { + body.get("input") + .and_then(Value::as_array) + .map(Vec::len) + .unwrap_or_default() +} + +fn replacement_history_len(body: &Value) -> usize { + body.as_array().map(Vec::len).unwrap_or_default() +} diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 0d89516d3d..87772aa279 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -43,6 +43,7 @@ mod codex_delegate; mod collaboration_instructions; mod compact; mod compact_remote; +mod compact_remote_parity; mod compact_resume_fork; mod deprecation_notice; mod exec; diff --git a/codex-rs/ext/extension-api/src/contributors.rs b/codex-rs/ext/extension-api/src/contributors.rs index ed9665f228..a8c2001200 100644 --- a/codex-rs/ext/extension-api/src/contributors.rs +++ b/codex-rs/ext/extension-api/src/contributors.rs @@ -4,12 +4,13 @@ use std::sync::Arc; use codex_protocol::items::TurnItem; use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::TokenUsageInfo; +use codex_tools::ToolCall; +use codex_tools::ToolExecutor; use crate::ExtensionData; mod prompt; mod thread_lifecycle; -mod tools; mod turn_lifecycle; pub use prompt::PromptFragment; @@ -17,8 +18,6 @@ pub use prompt::PromptSlot; pub use thread_lifecycle::ThreadResumeInput; pub use thread_lifecycle::ThreadStartInput; pub use thread_lifecycle::ThreadStopInput; -pub use tools::ExtensionToolExecutor; -pub use tools::ExtensionToolOutput; pub use turn_lifecycle::TurnAbortInput; pub use turn_lifecycle::TurnStartInput; pub use turn_lifecycle::TurnStopInput; @@ -106,7 +105,7 @@ pub trait ToolContributor: Send + Sync { &self, session_store: &ExtensionData, thread_store: &ExtensionData, - ) -> Vec>; + ) -> Vec>>; } /// Future returned by one claimed approval-review contribution. diff --git a/codex-rs/ext/extension-api/src/contributors/tools.rs b/codex-rs/ext/extension-api/src/contributors/tools.rs deleted file mode 100644 index a9f0f6dc32..0000000000 --- a/codex-rs/ext/extension-api/src/contributors/tools.rs +++ /dev/null @@ -1,15 +0,0 @@ -use codex_tools::JsonToolOutput; -use codex_tools::ToolCall; -use codex_tools::ToolExecutor; - -/// Model-facing output returned by extension-owned tools. -pub type ExtensionToolOutput = JsonToolOutput; - -/// Thin alias for extension-owned executable tools. -/// -/// Extensions implement the shared `ToolExecutor` contract directly; -/// the marker keeps contributor signatures readable while preserving one -/// executable-tool abstraction across host and extension tools. -pub trait ExtensionToolExecutor: ToolExecutor {} - -impl ExtensionToolExecutor for T where T: ToolExecutor {} diff --git a/codex-rs/ext/extension-api/src/lib.rs b/codex-rs/ext/extension-api/src/lib.rs index a6d850468b..e8be7e0309 100644 --- a/codex-rs/ext/extension-api/src/lib.rs +++ b/codex-rs/ext/extension-api/src/lib.rs @@ -11,6 +11,7 @@ pub use codex_tools::ResponsesApiTool; pub use codex_tools::ToolCall; pub use codex_tools::ToolExecutor; pub use codex_tools::ToolName; +pub use codex_tools::ToolOutput; pub use codex_tools::ToolPayload; pub use codex_tools::ToolSpec; pub use codex_tools::parse_tool_input_schema; @@ -18,8 +19,6 @@ pub use contributors::ApprovalReviewContributor; pub use contributors::ApprovalReviewFuture; pub use contributors::ConfigContributor; pub use contributors::ContextContributor; -pub use contributors::ExtensionToolExecutor; -pub use contributors::ExtensionToolOutput; pub use contributors::PromptFragment; pub use contributors::PromptSlot; pub use contributors::ThreadLifecycleContributor; diff --git a/codex-rs/ext/memories/src/extension.rs b/codex-rs/ext/memories/src/extension.rs index 2b49559183..1a52eb2cb9 100644 --- a/codex-rs/ext/memories/src/extension.rs +++ b/codex-rs/ext/memories/src/extension.rs @@ -1,6 +1,7 @@ use std::sync::Arc; use codex_core::config::Config; +use codex_extension_api::ConfigContributor; use codex_extension_api::ContextContributor; use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionRegistryBuilder; @@ -25,6 +26,15 @@ pub(crate) struct MemoriesExtensionConfig { pub(crate) codex_home: AbsolutePathBuf, } +impl MemoriesExtensionConfig { + fn from_config(config: &Config) -> Self { + Self { + enabled: config.features.enabled(Feature::MemoryTool) && config.memories.use_memories, + codex_home: config.codex_home.clone(), + } + } +} + impl ContextContributor for MemoriesExtension { fn contribute<'a>( &'a self, @@ -50,11 +60,21 @@ impl ContextContributor for MemoriesExtension { impl ThreadLifecycleContributor for MemoriesExtension { fn on_thread_start(&self, input: ThreadStartInput<'_, Config>) { - input.thread_store.insert(MemoriesExtensionConfig { - enabled: input.config.features.enabled(Feature::MemoryTool) - && input.config.memories.use_memories, - codex_home: input.config.codex_home.clone(), - }); + input + .thread_store + .insert(MemoriesExtensionConfig::from_config(input.config)); + } +} + +impl ConfigContributor for MemoriesExtension { + fn on_config_changed( + &self, + _session_store: &ExtensionData, + thread_store: &ExtensionData, + _previous_config: &Config, + new_config: &Config, + ) { + thread_store.insert(MemoriesExtensionConfig::from_config(new_config)); } } @@ -63,7 +83,7 @@ impl ToolContributor for MemoriesExtension { &self, _session_store: &ExtensionData, thread_store: &ExtensionData, - ) -> Vec> { + ) -> Vec>> { let Some(config) = thread_store.get::() else { return Vec::new(); }; @@ -79,6 +99,8 @@ impl ToolContributor for MemoriesExtension { pub fn install(registry: &mut ExtensionRegistryBuilder) { let extension = Arc::new(MemoriesExtension); registry.thread_lifecycle_contributor(extension.clone()); - registry.prompt_contributor(extension.clone()); - registry.tool_contributor(extension); + registry.config_contributor(extension.clone()); + registry.prompt_contributor(extension); + // Keep the read/retrieval tools out of app-server until that rollout is intentional. + // registry.tool_contributor(extension); } diff --git a/codex-rs/ext/memories/src/tests.rs b/codex-rs/ext/memories/src/tests.rs index 47c58f4686..b732ca22c2 100644 --- a/codex-rs/ext/memories/src/tests.rs +++ b/codex-rs/ext/memories/src/tests.rs @@ -3,10 +3,10 @@ use std::sync::Arc; use codex_extension_api::ContextContributor; use codex_extension_api::ExtensionData; -use codex_extension_api::ExtensionToolExecutor; use codex_extension_api::PromptSlot; use codex_extension_api::ToolCall; use codex_extension_api::ToolContributor; +use codex_extension_api::ToolExecutor; use codex_extension_api::ToolName; use codex_extension_api::ToolPayload; use codex_tools::ToolOutput; @@ -290,20 +290,23 @@ async fn search_tool_rejects_legacy_single_query() { .to_string(), }; - let err = tool + let result = tool .handle(ToolCall { call_id: "call-1".to_string(), tool_name: memory_tool_name(crate::SEARCH_TOOL_NAME), payload, }) - .await - .expect_err("legacy query field should be rejected"); + .await; + let err = match result { + Ok(_) => panic!("legacy query field should be rejected"), + Err(err) => err, + }; assert!(err.to_string().contains("unknown field")); assert!(err.to_string().contains("query")); } -fn memory_tool(memory_root: &Path, tool_name: &str) -> Arc { +fn memory_tool(memory_root: &Path, tool_name: &str) -> Arc> { let expected_tool_name = memory_tool_name(tool_name); crate::tools::memory_tools(LocalMemoriesBackend::from_memory_root(memory_root)) .into_iter() diff --git a/codex-rs/ext/memories/src/tools/list.rs b/codex-rs/ext/memories/src/tools/list.rs index 1509c2344c..baed3c30a2 100644 --- a/codex-rs/ext/memories/src/tools/list.rs +++ b/codex-rs/ext/memories/src/tools/list.rs @@ -39,8 +39,6 @@ impl ToolExecutor for ListTool where B: MemoriesBackend, { - type Output = JsonToolOutput; - fn tool_name(&self) -> ToolName { memory_tool_name(LIST_TOOL_NAME) } @@ -55,7 +53,8 @@ where async fn handle( &self, call: ToolCall, - ) -> Result { + ) -> Result, codex_extension_api::FunctionCallError> + { let backend = self.backend.clone(); let args: ListArgs = parse_args(&call)?; let response = backend @@ -70,6 +69,6 @@ where }) .await .map_err(backend_error_to_function_call)?; - Ok(JsonToolOutput::new(json!(response))) + Ok(Box::new(JsonToolOutput::new(json!(response)))) } } diff --git a/codex-rs/ext/memories/src/tools/mod.rs b/codex-rs/ext/memories/src/tools/mod.rs index f95922b28f..b78a125652 100644 --- a/codex-rs/ext/memories/src/tools/mod.rs +++ b/codex-rs/ext/memories/src/tools/mod.rs @@ -1,9 +1,9 @@ use std::sync::Arc; -use codex_extension_api::ExtensionToolExecutor; use codex_extension_api::FunctionCallError; use codex_extension_api::ResponsesApiTool; use codex_extension_api::ToolCall; +use codex_extension_api::ToolExecutor; use codex_extension_api::ToolName; use codex_extension_api::ToolSpec; use codex_extension_api::parse_tool_input_schema; @@ -23,7 +23,7 @@ mod list; mod read; mod search; -pub(crate) fn memory_tools(backend: B) -> Vec> +pub(crate) fn memory_tools(backend: B) -> Vec>> where B: MemoriesBackend, { diff --git a/codex-rs/ext/memories/src/tools/read.rs b/codex-rs/ext/memories/src/tools/read.rs index 06d8a1752c..6e7f771d75 100644 --- a/codex-rs/ext/memories/src/tools/read.rs +++ b/codex-rs/ext/memories/src/tools/read.rs @@ -38,8 +38,6 @@ impl ToolExecutor for ReadTool where B: MemoriesBackend, { - type Output = JsonToolOutput; - fn tool_name(&self) -> ToolName { memory_tool_name(READ_TOOL_NAME) } @@ -54,7 +52,8 @@ where async fn handle( &self, call: ToolCall, - ) -> Result { + ) -> Result, codex_extension_api::FunctionCallError> + { let backend = self.backend.clone(); let args: ReadArgs = parse_args(&call)?; let response = backend @@ -66,6 +65,6 @@ where }) .await .map_err(backend_error_to_function_call)?; - Ok(JsonToolOutput::new(json!(response))) + Ok(Box::new(JsonToolOutput::new(json!(response)))) } } diff --git a/codex-rs/ext/memories/src/tools/search.rs b/codex-rs/ext/memories/src/tools/search.rs index f7cab7de6c..422ce06b7e 100644 --- a/codex-rs/ext/memories/src/tools/search.rs +++ b/codex-rs/ext/memories/src/tools/search.rs @@ -47,8 +47,6 @@ impl ToolExecutor for SearchTool where B: MemoriesBackend, { - type Output = JsonToolOutput; - fn tool_name(&self) -> ToolName { memory_tool_name(SEARCH_TOOL_NAME) } @@ -63,14 +61,15 @@ where async fn handle( &self, call: ToolCall, - ) -> Result { + ) -> Result, codex_extension_api::FunctionCallError> + { let backend = self.backend.clone(); let args: SearchArgs = parse_args(&call)?; let response = backend .search(args.into_request()) .await .map_err(backend_error_to_function_call)?; - Ok(JsonToolOutput::new(json!(response))) + Ok(Box::new(JsonToolOutput::new(json!(response)))) } } diff --git a/codex-rs/otel/src/events/session_telemetry.rs b/codex-rs/otel/src/events/session_telemetry.rs index 49d8b8eb7e..6394c73ea2 100644 --- a/codex-rs/otel/src/events/session_telemetry.rs +++ b/codex-rs/otel/src/events/session_telemetry.rs @@ -1141,6 +1141,7 @@ impl SessionTelemetry { ResponseItem::WebSearchCall { .. } => "web_search_call".into(), ResponseItem::ImageGenerationCall { .. } => "image_generation_call".into(), ResponseItem::Compaction { .. } => "compaction".into(), + ResponseItem::CompactionTrigger => "compaction_trigger".into(), ResponseItem::ContextCompaction { .. } => "context_compaction".into(), ResponseItem::Other => "other".into(), } diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 512dbd5444..2706a47c82 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -887,7 +887,10 @@ pub enum ResponseItem { result: String, }, #[serde(alias = "compaction_summary")] - Compaction { encrypted_content: String }, + Compaction { + encrypted_content: String, + }, + CompactionTrigger, ContextCompaction { #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] @@ -2407,20 +2410,28 @@ mod tests { } #[test] - fn serializes_context_compaction_trigger_without_payload() -> Result<()> { - let item = ResponseItem::ContextCompaction { - encrypted_content: None, - }; + fn serializes_compaction_trigger_without_payload() -> Result<()> { + let item = ResponseItem::CompactionTrigger; assert_eq!( serde_json::to_value(item)?, serde_json::json!({ - "type": "context_compaction", + "type": "compaction_trigger", }) ); Ok(()) } + #[test] + fn deserializes_compaction_trigger_without_payload() -> Result<()> { + let json = r#"{"type":"compaction_trigger"}"#; + + let item: ResponseItem = serde_json::from_str(json)?; + + assert_eq!(item, ResponseItem::CompactionTrigger); + Ok(()) + } + #[test] fn deserializes_legacy_ghost_snapshot_as_other() -> Result<()> { let json = r#"{ diff --git a/codex-rs/rollout/src/policy.rs b/codex-rs/rollout/src/policy.rs index 9c42f8db61..ceb617763f 100644 --- a/codex-rs/rollout/src/policy.rs +++ b/codex-rs/rollout/src/policy.rs @@ -79,6 +79,7 @@ pub fn should_persist_response_item(item: &ResponseItem) -> bool { | ResponseItem::ImageGenerationCall { .. } | ResponseItem::Compaction { .. } | ResponseItem::ContextCompaction { .. } => true, + ResponseItem::CompactionTrigger => false, ResponseItem::Other => false, } } @@ -99,6 +100,7 @@ pub fn should_persist_response_item_for_memories(item: &ResponseItem) -> bool { ResponseItem::Reasoning { .. } | ResponseItem::ImageGenerationCall { .. } | ResponseItem::Compaction { .. } + | ResponseItem::CompactionTrigger | ResponseItem::ContextCompaction { .. } | ResponseItem::Other => false, } diff --git a/codex-rs/tools/src/tool_executor.rs b/codex-rs/tools/src/tool_executor.rs index 62237740ba..2557dc3490 100644 --- a/codex-rs/tools/src/tool_executor.rs +++ b/codex-rs/tools/src/tool_executor.rs @@ -36,8 +36,6 @@ impl ToolExposure { /// top without reopening the spec/runtime split. #[async_trait::async_trait] pub trait ToolExecutor: Send + Sync { - type Output: ToolOutput + 'static; - /// The concrete tool name handled by this runtime instance. fn tool_name(&self) -> ToolName; @@ -53,5 +51,8 @@ pub trait ToolExecutor: Send + Sync { false } - async fn handle(&self, invocation: Invocation) -> Result; + async fn handle( + &self, + invocation: Invocation, + ) -> Result, FunctionCallError>; } diff --git a/codex-rs/tools/src/tool_output.rs b/codex-rs/tools/src/tool_output.rs index 2044295174..64a8164da3 100644 --- a/codex-rs/tools/src/tool_output.rs +++ b/codex-rs/tools/src/tool_output.rs @@ -20,6 +20,16 @@ pub trait ToolOutput: Send { fn to_response_item(&self, call_id: &str, payload: &ToolPayload) -> ResponseInputItem; + /// Returns the tool call id exposed to `PostToolUse` hooks for this output. + fn post_tool_use_id(&self, call_id: &str) -> String { + call_id.to_string() + } + + /// Returns the tool input exposed to `PostToolUse` hooks for this output. + fn post_tool_use_input(&self, _payload: &ToolPayload) -> Option { + None + } + /// Returns the stable value exposed to `PostToolUse` hooks for this tool output. /// /// Tool handlers decide whether a tool participates in `PostToolUse`, but @@ -52,6 +62,14 @@ where (**self).to_response_item(call_id, payload) } + fn post_tool_use_id(&self, call_id: &str) -> String { + (**self).post_tool_use_id(call_id) + } + + fn post_tool_use_input(&self, payload: &ToolPayload) -> Option { + (**self).post_tool_use_input(payload) + } + fn post_tool_use_response(&self, call_id: &str, payload: &ToolPayload) -> Option { (**self).post_tool_use_response(call_id, payload) }