From 0b619afc87ae51df852e61ab9f10c3403d3288fc Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Tue, 24 Mar 2026 15:42:45 -0700 Subject: [PATCH 1/8] Drop sandbox_permissions from sandbox exec requests (#15665) ## Summary - drop `sandbox_permissions` from the sandboxing `ExecOptions` and `ExecRequest` adapter types - remove the now-unused plumbing from shell, unified exec, JS REPL, and apply-patch runtime call sites - default reconstructed `ExecParams` to `SandboxPermissions::UseDefault` where the lower-level API still requires the field ## Testing - `just fmt` - `just argument-comment-lint` - `cargo test -p codex-core` (still running locally; first failures observed in `suite::cli_stream::responses_mode_stream_cli`, `suite::cli_stream::responses_mode_stream_cli_supports_openai_base_url_config_override`, and `suite::cli_stream::responses_mode_stream_cli_supports_openai_base_url_env_fallback`) --- codex-rs/app-server/src/command_exec.rs | 4 ---- codex-rs/core/src/exec.rs | 12 ++++-------- codex-rs/core/src/sandboxing/mod.rs | 8 -------- codex-rs/core/src/tasks/user_shell.rs | 3 --- .../core/src/tools/handlers/apply_patch.rs | 3 --- codex-rs/core/src/tools/js_repl/mod.rs | 3 --- .../core/src/tools/runtimes/apply_patch.rs | 4 ---- .../src/tools/runtimes/apply_patch_tests.rs | 1 - codex-rs/core/src/tools/runtimes/shell.rs | 2 -- .../tools/runtimes/shell/unix_escalation.rs | 19 ------------------- .../runtimes/shell/unix_escalation_tests.rs | 6 ------ .../core/src/tools/runtimes/unified_exec.rs | 4 ---- 12 files changed, 4 insertions(+), 65 deletions(-) diff --git a/codex-rs/app-server/src/command_exec.rs b/codex-rs/app-server/src/command_exec.rs index 5bb9a2719c..8f92cd0ba2 100644 --- a/codex-rs/app-server/src/command_exec.rs +++ b/codex-rs/app-server/src/command_exec.rs @@ -744,11 +744,9 @@ mod tests { sandbox: SandboxType::WindowsRestrictedToken, windows_sandbox_level: WindowsSandboxLevel::Disabled, windows_sandbox_private_desktop: false, - sandbox_permissions: codex_core::sandboxing::SandboxPermissions::UseDefault, sandbox_policy: sandbox_policy.clone(), file_system_sandbox_policy: FileSystemSandboxPolicy::from(&sandbox_policy), network_sandbox_policy: NetworkSandboxPolicy::from(&sandbox_policy), - justification: None, arg0: None, } } @@ -858,11 +856,9 @@ mod tests { sandbox: SandboxType::None, windows_sandbox_level: WindowsSandboxLevel::Disabled, windows_sandbox_private_desktop: false, - sandbox_permissions: codex_core::sandboxing::SandboxPermissions::UseDefault, sandbox_policy: sandbox_policy.clone(), file_system_sandbox_policy: FileSystemSandboxPolicy::from(&sandbox_policy), network_sandbox_policy: NetworkSandboxPolicy::from(&sandbox_policy), - justification: None, arg0: None, }, started_network_proxy: None, diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 7f4664858a..0229e1dd57 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -241,10 +241,10 @@ pub fn build_exec_request( expiration, capture_policy, network, - sandbox_permissions, + sandbox_permissions: _, windows_sandbox_level, windows_sandbox_private_desktop, - justification, + justification: _, arg0: _, } = params; if let Some(network) = network.as_ref() { @@ -268,8 +268,6 @@ pub fn build_exec_request( let options = ExecOptions { expiration, capture_policy, - sandbox_permissions, - justification, }; let exec_req = manager .transform(SandboxTransformRequest { @@ -309,11 +307,9 @@ pub(crate) async fn execute_exec_request( sandbox, windows_sandbox_level, windows_sandbox_private_desktop, - sandbox_permissions, sandbox_policy: _sandbox_policy_from_env, file_system_sandbox_policy, network_sandbox_policy, - justification, arg0, } = exec_request; let _ = _sandbox_policy_from_env; @@ -325,10 +321,10 @@ pub(crate) async fn execute_exec_request( capture_policy, env, network: network.clone(), - sandbox_permissions, + sandbox_permissions: SandboxPermissions::UseDefault, windows_sandbox_level, windows_sandbox_private_desktop, - justification, + justification: None, arg0, }; diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 2b2f3c2364..81f2a0af2c 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -30,8 +30,6 @@ use std::path::PathBuf; pub(crate) struct ExecOptions { pub(crate) expiration: ExecExpiration, pub(crate) capture_policy: ExecCapturePolicy, - pub(crate) sandbox_permissions: SandboxPermissions, - pub(crate) justification: Option, } #[derive(Debug)] @@ -45,11 +43,9 @@ pub struct ExecRequest { pub sandbox: SandboxType, pub windows_sandbox_level: WindowsSandboxLevel, pub windows_sandbox_private_desktop: bool, - pub sandbox_permissions: SandboxPermissions, pub sandbox_policy: SandboxPolicy, pub file_system_sandbox_policy: FileSystemSandboxPolicy, pub network_sandbox_policy: NetworkSandboxPolicy, - pub justification: Option, pub arg0: Option, } @@ -74,8 +70,6 @@ impl ExecRequest { let ExecOptions { expiration, capture_policy, - sandbox_permissions, - justification, } = options; if !network_sandbox_policy.is_enabled() { env.insert( @@ -97,11 +91,9 @@ impl ExecRequest { sandbox, windows_sandbox_level, windows_sandbox_private_desktop, - sandbox_permissions, sandbox_policy, file_system_sandbox_policy, network_sandbox_policy, - justification, arg0, } } diff --git a/codex-rs/core/src/tasks/user_shell.rs b/codex-rs/core/src/tasks/user_shell.rs index 33a7c682b4..682902f2d6 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -25,7 +25,6 @@ use crate::protocol::ExecCommandStatus; use crate::protocol::SandboxPolicy; use crate::protocol::TurnStartedEvent; use crate::sandboxing::ExecRequest; -use crate::sandboxing::SandboxPermissions; use crate::state::TaskKind; use crate::tools::format_exec_output_str; use crate::tools::runtimes::maybe_wrap_shell_lc_with_snapshot; @@ -173,11 +172,9 @@ pub(crate) async fn execute_user_shell_command( .config .permissions .windows_sandbox_private_desktop, - sandbox_permissions: SandboxPermissions::UseDefault, sandbox_policy: sandbox_policy.clone(), file_system_sandbox_policy: FileSystemSandboxPolicy::from(&sandbox_policy), network_sandbox_policy: NetworkSandboxPolicy::from(&sandbox_policy), - justification: None, arg0: None, }; diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index bbe3572c26..62ebde48b8 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -200,8 +200,6 @@ impl ToolHandler for ApplyPatchHandler { file_paths, changes, exec_approval_requirement: apply.exec_approval_requirement, - sandbox_permissions: effective_additional_permissions - .sandbox_permissions, additional_permissions: effective_additional_permissions .additional_permissions, permissions_preapproved: effective_additional_permissions @@ -305,7 +303,6 @@ pub(crate) async fn intercept_apply_patch( file_paths: approval_keys, changes, exec_approval_requirement: apply.exec_approval_requirement, - sandbox_permissions: effective_additional_permissions.sandbox_permissions, additional_permissions: effective_additional_permissions .additional_permissions, permissions_preapproved: effective_additional_permissions diff --git a/codex-rs/core/src/tools/js_repl/mod.rs b/codex-rs/core/src/tools/js_repl/mod.rs index 73c0b34ead..27d8cb9e65 100644 --- a/codex-rs/core/src/tools/js_repl/mod.rs +++ b/codex-rs/core/src/tools/js_repl/mod.rs @@ -40,7 +40,6 @@ use crate::exec_env::create_env; use crate::function_tool::FunctionCallError; use crate::original_image_detail::normalize_output_image_detail; use crate::sandboxing::ExecOptions; -use crate::sandboxing::SandboxPermissions; use crate::tools::ToolRouter; use crate::tools::context::SharedTurnDiffTracker; use crate::truncate::TruncationPolicy; @@ -1058,8 +1057,6 @@ impl JsReplManager { let options = ExecOptions { expiration: ExecExpiration::DefaultTimeout, capture_policy: ExecCapturePolicy::ShellTool, - sandbox_permissions: SandboxPermissions::UseDefault, - justification: None, }; let exec_env = sandbox .transform(SandboxTransformRequest { diff --git a/codex-rs/core/src/tools/runtimes/apply_patch.rs b/codex-rs/core/src/tools/runtimes/apply_patch.rs index d1a6efe001..7f5686e47d 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch.rs @@ -10,7 +10,6 @@ use crate::guardian::GuardianApprovalRequest; use crate::guardian::review_approval_request; use crate::guardian::routes_approval_to_guardian; use crate::sandboxing::ExecOptions; -use crate::sandboxing::SandboxPermissions; use crate::sandboxing::execute_env; use crate::tools::sandboxing::Approvable; use crate::tools::sandboxing::ApprovalCtx; @@ -40,7 +39,6 @@ pub struct ApplyPatchRequest { pub file_paths: Vec, pub changes: std::collections::HashMap, pub exec_approval_requirement: ExecApprovalRequirement, - pub sandbox_permissions: SandboxPermissions, pub additional_permissions: Option, pub permissions_preapproved: bool, pub timeout_ms: Option, @@ -206,8 +204,6 @@ impl ToolRuntime for ApplyPatchRuntime { let options = ExecOptions { expiration: req.timeout_ms.into(), capture_policy: ExecCapturePolicy::ShellTool, - sandbox_permissions: req.sandbox_permissions, - justification: None, }; let env = attempt .env_for(command, options, /*network*/ None) diff --git a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs index d2812b5ecf..79e35a9c2d 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs @@ -48,7 +48,6 @@ fn guardian_review_request_includes_patch_context() { reason: None, proposed_execpolicy_amendment: None, }, - sandbox_permissions: SandboxPermissions::UseDefault, additional_permissions: None, permissions_preapproved: false, timeout_ms: None, diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 6582557f85..ecb5822124 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -254,8 +254,6 @@ impl ToolRuntime for ShellRuntime { let options = ExecOptions { expiration: req.timeout_ms.into(), capture_policy: ExecCapturePolicy::ShellTool, - sandbox_permissions: req.sandbox_permissions, - justification: req.justification.clone(), }; let env = attempt .env_for(command, options, req.network.as_ref()) diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 0131616c34..8c4f24ed05 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -120,8 +120,6 @@ pub(super) async fn try_run_zsh_fork( let options = ExecOptions { expiration: req.timeout_ms.into(), capture_policy: ExecCapturePolicy::ShellTool, - sandbox_permissions: req.sandbox_permissions, - justification: req.justification.clone(), }; let sandbox_exec_request = attempt .env_for(command, options, req.network.as_ref()) @@ -136,11 +134,9 @@ pub(super) async fn try_run_zsh_fork( sandbox, windows_sandbox_level, windows_sandbox_private_desktop: _windows_sandbox_private_desktop, - sandbox_permissions, sandbox_policy, file_system_sandbox_policy, network_sandbox_policy, - justification, arg0, } = sandbox_exec_request; let ParsedShellCommand { script, login, .. } = extract_shell_script(&command)?; @@ -161,8 +157,6 @@ pub(super) async fn try_run_zsh_fork( env: sandbox_env, network: sandbox_network, windows_sandbox_level, - sandbox_permissions, - justification, arg0, sandbox_policy_cwd: ctx.turn.cwd.clone(), macos_seatbelt_profile_extensions: ctx @@ -267,8 +261,6 @@ pub(crate) async fn prepare_unified_exec_zsh_fork( env: exec_request.env.clone(), network: exec_request.network.clone(), windows_sandbox_level: exec_request.windows_sandbox_level, - sandbox_permissions: exec_request.sandbox_permissions, - justification: exec_request.justification.clone(), arg0: exec_request.arg0.clone(), sandbox_policy_cwd: ctx.turn.cwd.clone(), macos_seatbelt_profile_extensions: ctx @@ -864,8 +856,6 @@ struct CoreShellCommandExecutor { env: HashMap, network: Option, windows_sandbox_level: WindowsSandboxLevel, - sandbox_permissions: SandboxPermissions, - justification: Option, arg0: Option, sandbox_policy_cwd: PathBuf, #[cfg_attr(not(target_os = "macos"), allow(dead_code))] @@ -916,11 +906,9 @@ impl ShellCommandExecutor for CoreShellCommandExecutor { sandbox: self.sandbox, windows_sandbox_level: self.windows_sandbox_level, windows_sandbox_private_desktop: false, - sandbox_permissions: self.sandbox_permissions, sandbox_policy: self.sandbox_policy.clone(), file_system_sandbox_policy: self.file_system_sandbox_policy.clone(), network_sandbox_policy: self.network_sandbox_policy, - justification: self.justification.clone(), arg0: self.arg0.clone(), }, /*stdout_stream*/ None, @@ -1044,11 +1032,6 @@ impl CoreShellCommandExecutor { self.windows_sandbox_level, self.network.is_some(), ); - let sandbox_permissions = if additional_permissions.is_some() { - SandboxPermissions::WithAdditionalPermissions - } else { - SandboxPermissions::UseDefault - }; let command = SandboxCommand { program: program.clone(), args: args.to_vec(), @@ -1059,8 +1042,6 @@ impl CoreShellCommandExecutor { let options = ExecOptions { expiration: ExecExpiration::DefaultTimeout, capture_policy: ExecCapturePolicy::ShellTool, - sandbox_permissions, - justification: self.justification.clone(), }; let exec_request = sandbox_manager.transform(SandboxTransformRequest { command, diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index 6373697c3b..25cbb71c02 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -660,8 +660,6 @@ async fn prepare_escalated_exec_turn_default_preserves_macos_seatbelt_extensions file_system_sandbox_policy: read_only_file_system_sandbox_policy(), network_sandbox_policy: NetworkSandboxPolicy::Restricted, windows_sandbox_level: WindowsSandboxLevel::Disabled, - sandbox_permissions: SandboxPermissions::UseDefault, - justification: None, arg0: None, sandbox_policy_cwd: cwd.to_path_buf(), macos_seatbelt_profile_extensions: Some(MacOsSeatbeltProfileExtensions { @@ -712,8 +710,6 @@ async fn prepare_escalated_exec_permissions_preserve_macos_seatbelt_extensions() file_system_sandbox_policy: unrestricted_file_system_sandbox_policy(), network_sandbox_policy: NetworkSandboxPolicy::Enabled, windows_sandbox_level: WindowsSandboxLevel::Disabled, - sandbox_permissions: SandboxPermissions::UseDefault, - justification: None, arg0: None, sandbox_policy_cwd: cwd.to_path_buf(), macos_seatbelt_profile_extensions: None, @@ -787,8 +783,6 @@ async fn prepare_escalated_exec_permission_profile_unions_turn_and_requested_mac file_system_sandbox_policy: read_only_file_system_sandbox_policy(), network_sandbox_policy: NetworkSandboxPolicy::from(&sandbox_policy), windows_sandbox_level: WindowsSandboxLevel::Disabled, - sandbox_permissions: SandboxPermissions::UseDefault, - justification: None, arg0: None, sandbox_policy_cwd: cwd.to_path_buf(), macos_seatbelt_profile_extensions: Some(MacOsSeatbeltProfileExtensions { diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index f59d38556b..3f8ab5f6ce 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -218,8 +218,6 @@ impl<'a> ToolRuntime for UnifiedExecRunt let options = ExecOptions { expiration: ExecExpiration::DefaultTimeout, capture_policy: ExecCapturePolicy::ShellTool, - sandbox_permissions: req.sandbox_permissions, - justification: req.justification.clone(), }; let exec_env = attempt .env_for(command, options, req.network.as_ref()) @@ -265,8 +263,6 @@ impl<'a> ToolRuntime for UnifiedExecRunt let options = ExecOptions { expiration: ExecExpiration::DefaultTimeout, capture_policy: ExecCapturePolicy::ShellTool, - sandbox_permissions: req.sandbox_permissions, - justification: req.justification.clone(), }; let exec_env = attempt .env_for(command, options, req.network.as_ref()) From 062fa7a2bb96b7b6f5fd0bc1edb7c48d75024114 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Tue, 24 Mar 2026 15:45:40 -0700 Subject: [PATCH 2/8] Move string truncation helpers into codex-utils-string (#15572) - move the shared byte-based middle truncation logic from `core` into `codex-utils-string` - keep token-specific truncation in `codex-core` so rollout can reuse the shared helper in the next stacked PR --------- Co-authored-by: Codex --- codex-rs/Cargo.lock | 11 + codex-rs/Cargo.toml | 2 + codex-rs/core/Cargo.toml | 1 + codex-rs/core/src/codex.rs | 4 +- .../src/codex/rollout_reconstruction_tests.rs | 16 +- codex-rs/core/src/codex_tests.rs | 2 +- codex-rs/core/src/compact.rs | 6 +- codex-rs/core/src/context_manager/history.rs | 12 +- .../core/src/context_manager/history_tests.rs | 6 +- codex-rs/core/src/error.rs | 4 +- codex-rs/core/src/guardian/prompt.rs | 6 +- codex-rs/core/src/lib.rs | 1 - codex-rs/core/src/memories/prompts.rs | 4 +- .../core/src/models_manager/model_info.rs | 2 +- codex-rs/core/src/realtime_context.rs | 4 +- codex-rs/core/src/rollout/list.rs | 2 +- codex-rs/core/src/rollout/recorder.rs | 8 +- codex-rs/core/src/rollout/recorder_tests.rs | 2 +- codex-rs/core/src/rollout/tests.rs | 2 +- codex-rs/core/src/state/session.rs | 2 +- codex-rs/core/src/tools/code_mode/mod.rs | 6 +- codex-rs/core/src/tools/context.rs | 4 +- codex-rs/core/src/tools/js_repl/mod.rs | 4 +- codex-rs/core/src/tools/mod.rs | 6 +- codex-rs/core/src/truncate.rs | 363 ------------------ codex-rs/core/src/unified_exec/process.rs | 4 +- .../core/src/unified_exec/process_manager.rs | 2 +- codex-rs/protocol/Cargo.toml | 1 + codex-rs/protocol/src/protocol.rs | 46 +++ codex-rs/utils/output-truncation/BUILD.bazel | 6 + codex-rs/utils/output-truncation/Cargo.toml | 15 + codex-rs/utils/output-truncation/src/lib.rs | 142 +++++++ .../output-truncation}/src/truncate_tests.rs | 106 ++--- codex-rs/utils/string/src/lib.rs | 9 + codex-rs/utils/string/src/truncate.rs | 156 ++++++++ codex-rs/utils/string/src/truncate/tests.rs | 71 ++++ 36 files changed, 551 insertions(+), 487 deletions(-) delete mode 100644 codex-rs/core/src/truncate.rs create mode 100644 codex-rs/utils/output-truncation/BUILD.bazel create mode 100644 codex-rs/utils/output-truncation/Cargo.toml create mode 100644 codex-rs/utils/output-truncation/src/lib.rs rename codex-rs/{core => utils/output-truncation}/src/truncate_tests.rs (77%) create mode 100644 codex-rs/utils/string/src/truncate.rs create mode 100644 codex-rs/utils/string/src/truncate/tests.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 935fc413a5..697b8f5d32 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1902,6 +1902,7 @@ dependencies = [ "codex-utils-cargo-bin", "codex-utils-home-dir", "codex-utils-image", + "codex-utils-output-truncation", "codex-utils-pty", "codex-utils-readiness", "codex-utils-stream-parser", @@ -2399,6 +2400,7 @@ dependencies = [ "codex-git-utils", "codex-utils-absolute-path", "codex-utils-image", + "codex-utils-string", "icu_decimal", "icu_locale_core", "icu_provider", @@ -2891,6 +2893,15 @@ dependencies = [ "codex-ollama", ] +[[package]] +name = "codex-utils-output-truncation" +version = "0.0.0" +dependencies = [ + "codex-protocol", + "codex-utils-string", + "pretty_assertions", +] + [[package]] name = "codex-utils-pty" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index e071ec807e..ebd88c945f 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -65,6 +65,7 @@ members = [ "utils/sleep-inhibitor", "utils/approval-presets", "utils/oss", + "utils/output-truncation", "utils/fuzzy-match", "utils/stream-parser", "codex-client", @@ -154,6 +155,7 @@ codex-utils-home-dir = { path = "utils/home-dir" } codex-utils-image = { path = "utils/image" } codex-utils-json-to-toml = { path = "utils/json-to-toml" } codex-utils-oss = { path = "utils/oss" } +codex-utils-output-truncation = { path = "utils/output-truncation" } codex-utils-pty = { path = "utils/pty" } codex-utils-readiness = { path = "utils/readiness" } codex-utils-rustls-provider = { path = "utils/rustls-provider" } diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 82ea260055..51dd305b35 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -55,6 +55,7 @@ codex-utils-absolute-path = { workspace = true } codex-utils-cache = { workspace = true } codex-utils-image = { workspace = true } codex-utils-home-dir = { workspace = true } +codex-utils-output-truncation = { workspace = true } codex-utils-pty = { workspace = true } codex-utils-readiness = { workspace = true } codex-secrets = { workspace = true } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index b299bec369..2f8c2877f3 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -46,7 +46,6 @@ use crate::stream_events_utils::handle_output_item_done; use crate::stream_events_utils::last_assistant_message_from_item; use crate::stream_events_utils::raw_assistant_output_text_from_item; use crate::stream_events_utils::record_completed_response_item; -use crate::truncate::TruncationPolicy; use crate::turn_metadata::TurnMetadataState; use crate::util::error_or_panic; use async_channel::Receiver; @@ -117,6 +116,7 @@ use codex_protocol::request_user_input::RequestUserInputResponse; use codex_rmcp_client::ElicitationResponse; use codex_rmcp_client::OAuthCredentialsStoreMode; use codex_terminal_detection::user_agent; +use codex_utils_output_truncation::TruncationPolicy; use codex_utils_stream_parser::AssistantTextChunk; use codex_utils_stream_parser::AssistantTextStreamParser; use codex_utils_stream_parser::ProposedPlanSegment; @@ -1007,7 +1007,7 @@ impl TurnContext { user_instructions: self.user_instructions.clone(), developer_instructions: self.developer_instructions.clone(), final_output_json_schema: self.final_output_json_schema.clone(), - truncation_policy: Some(self.truncation_policy.into()), + truncation_policy: Some(self.truncation_policy), } } diff --git a/codex-rs/core/src/codex/rollout_reconstruction_tests.rs b/codex-rs/core/src/codex/rollout_reconstruction_tests.rs index 906c72c92a..1ed668d56f 100644 --- a/codex-rs/core/src/codex/rollout_reconstruction_tests.rs +++ b/codex-rs/core/src/codex/rollout_reconstruction_tests.rs @@ -77,7 +77,7 @@ async fn record_initial_history_resumed_bare_turn_context_does_not_hydrate_previ user_instructions: None, developer_instructions: None, final_output_json_schema: None, - truncation_policy: Some(turn_context.truncation_policy.into()), + truncation_policy: Some(turn_context.truncation_policy), }; let rollout_items = vec![RolloutItem::TurnContext(previous_context_item)]; @@ -116,7 +116,7 @@ async fn record_initial_history_resumed_hydrates_previous_turn_settings_from_lif user_instructions: None, developer_instructions: None, final_output_json_schema: None, - truncation_policy: Some(turn_context.truncation_policy.into()), + truncation_policy: Some(turn_context.truncation_policy), }; let turn_id = previous_context_item .turn_id @@ -866,7 +866,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis user_instructions: None, developer_instructions: None, final_output_json_schema: None, - truncation_policy: Some(turn_context.truncation_policy.into()), + truncation_policy: Some(turn_context.truncation_policy), }; let previous_turn_id = previous_context_item .turn_id @@ -938,7 +938,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis user_instructions: None, developer_instructions: None, final_output_json_schema: None, - truncation_policy: Some(turn_context.truncation_policy.into()), + truncation_policy: Some(turn_context.truncation_policy), })) .expect("serialize expected reference context item") ); @@ -967,7 +967,7 @@ async fn record_initial_history_resumed_aborted_turn_without_id_clears_active_tu user_instructions: None, developer_instructions: None, final_output_json_schema: None, - truncation_policy: Some(turn_context.truncation_policy.into()), + truncation_policy: Some(turn_context.truncation_policy), }; let previous_turn_id = previous_context_item .turn_id @@ -1073,7 +1073,7 @@ async fn record_initial_history_resumed_unmatched_abort_preserves_active_turn_fo user_instructions: None, developer_instructions: None, final_output_json_schema: None, - truncation_policy: Some(turn_context.truncation_policy.into()), + truncation_policy: Some(turn_context.truncation_policy), }; let rollout_items = vec![ @@ -1175,7 +1175,7 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_compaction_clea user_instructions: None, developer_instructions: None, final_output_json_schema: None, - truncation_policy: Some(turn_context.truncation_policy.into()), + truncation_policy: Some(turn_context.truncation_policy), }; let previous_turn_id = previous_context_item .turn_id @@ -1319,7 +1319,7 @@ async fn record_initial_history_resumed_replaced_incomplete_compacted_turn_clear user_instructions: None, developer_instructions: None, final_output_json_schema: None, - truncation_policy: Some(turn_context.truncation_policy.into()), + truncation_policy: Some(turn_context.truncation_policy), }; let previous_turn_id = previous_context_item .turn_id diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index 8846f0a643..0ba0a1beb0 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -1287,7 +1287,7 @@ async fn record_initial_history_forked_hydrates_previous_turn_settings() { user_instructions: None, developer_instructions: None, final_output_json_schema: None, - truncation_policy: Some(turn_context.truncation_policy.into()), + truncation_policy: Some(turn_context.truncation_policy), }; let turn_id = previous_context_item .turn_id diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index 7686c5b65c..82fd5c15e3 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -15,9 +15,6 @@ use crate::protocol::CompactedItem; use crate::protocol::EventMsg; use crate::protocol::TurnStartedEvent; use crate::protocol::WarningEvent; -use crate::truncate::TruncationPolicy; -use crate::truncate::approx_token_count; -use crate::truncate::truncate_text; use crate::util::backoff; use codex_protocol::items::ContextCompactionItem; use codex_protocol::items::TurnItem; @@ -25,6 +22,9 @@ use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; use codex_protocol::user_input::UserInput; +use codex_utils_output_truncation::TruncationPolicy; +use codex_utils_output_truncation::approx_token_count; +use codex_utils_output_truncation::truncate_text; use futures::prelude::*; use tracing::error; diff --git a/codex-rs/core/src/context_manager/history.rs b/codex-rs/core/src/context_manager/history.rs index b518d8ce4f..ec2df30d3b 100644 --- a/codex-rs/core/src/context_manager/history.rs +++ b/codex-rs/core/src/context_manager/history.rs @@ -3,12 +3,6 @@ use crate::context_manager::normalize; use crate::event_mapping::has_non_contextual_dev_message_content; use crate::event_mapping::is_contextual_dev_message_content; use crate::event_mapping::is_contextual_user_message_content; -use crate::truncate::TruncationPolicy; -use crate::truncate::approx_bytes_for_tokens; -use crate::truncate::approx_token_count; -use crate::truncate::approx_tokens_from_byte_count_i64; -use crate::truncate::truncate_function_output_items_with_policy; -use crate::truncate::truncate_text; use base64::Engine; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; use codex_protocol::models::BaseInstructions; @@ -25,6 +19,12 @@ use codex_protocol::protocol::TokenUsageInfo; use codex_protocol::protocol::TurnContextItem; use codex_utils_cache::BlockingLruCache; use codex_utils_cache::sha1_digest; +use codex_utils_output_truncation::TruncationPolicy; +use codex_utils_output_truncation::approx_bytes_for_tokens; +use codex_utils_output_truncation::approx_token_count; +use codex_utils_output_truncation::approx_tokens_from_byte_count_i64; +use codex_utils_output_truncation::truncate_function_output_items_with_policy; +use codex_utils_output_truncation::truncate_text; use std::num::NonZeroUsize; use std::ops::Deref; use std::sync::LazyLock; diff --git a/codex-rs/core/src/context_manager/history_tests.rs b/codex-rs/core/src/context_manager/history_tests.rs index 0c92edcfb6..3c508e05ff 100644 --- a/codex-rs/core/src/context_manager/history_tests.rs +++ b/codex-rs/core/src/context_manager/history_tests.rs @@ -1,6 +1,4 @@ use super::*; -use crate::truncate; -use crate::truncate::TruncationPolicy; use base64::Engine; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; use codex_git_utils::GhostCommit; @@ -23,6 +21,8 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::InterAgentCommunication; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::TurnContextItem; +use codex_utils_output_truncation::TruncationPolicy; +use codex_utils_output_truncation::truncate_text; use image::ImageBuffer; use image::ImageFormat; use image::Rgba; @@ -179,7 +179,7 @@ fn reasoning_with_encrypted_content(len: usize) -> ResponseItem { } fn truncate_exec_output(content: &str) -> String { - truncate::truncate_text(content, TruncationPolicy::Tokens(EXEC_FORMAT_MAX_TOKENS)) + truncate_text(content, TruncationPolicy::Tokens(EXEC_FORMAT_MAX_TOKENS)) } fn approx_token_count_for_text(text: &str) -> i64 { diff --git a/codex-rs/core/src/error.rs b/codex-rs/core/src/error.rs index b84180d160..b0c55a55c6 100644 --- a/codex-rs/core/src/error.rs +++ b/codex-rs/core/src/error.rs @@ -2,8 +2,6 @@ use crate::exec::ExecToolCallOutput; use crate::network_policy_decision::NetworkPolicyDecisionPayload; use crate::token_data::KnownPlan; use crate::token_data::PlanType; -use crate::truncate::TruncationPolicy; -use crate::truncate::truncate_text; use chrono::DateTime; use chrono::Datelike; use chrono::Local; @@ -15,6 +13,8 @@ use codex_protocol::ThreadId; use codex_protocol::protocol::CodexErrorInfo; use codex_protocol::protocol::ErrorEvent; use codex_protocol::protocol::RateLimitSnapshot; +use codex_utils_output_truncation::TruncationPolicy; +use codex_utils_output_truncation::truncate_text; use reqwest::StatusCode; use serde_json; use std::io; diff --git a/codex-rs/core/src/guardian/prompt.rs b/codex-rs/core/src/guardian/prompt.rs index be02916487..5741a65fc0 100644 --- a/codex-rs/core/src/guardian/prompt.rs +++ b/codex-rs/core/src/guardian/prompt.rs @@ -7,9 +7,9 @@ use serde_json::Value; use crate::codex::Session; use crate::compact::content_items_to_text; use crate::event_mapping::is_contextual_user_message_content; -use crate::truncate::approx_bytes_for_tokens; -use crate::truncate::approx_token_count; -use crate::truncate::approx_tokens_from_byte_count; +use codex_utils_output_truncation::approx_bytes_for_tokens; +use codex_utils_output_truncation::approx_token_count; +use codex_utils_output_truncation::approx_tokens_from_byte_count; use super::GUARDIAN_MAX_MESSAGE_ENTRY_TOKENS; use super::GUARDIAN_MAX_MESSAGE_TRANSCRIPT_TOKENS; diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index decebc84e0..d4a8ae93aa 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -78,7 +78,6 @@ mod stream_events_utils; pub mod test_support; mod text_encoding; pub use codex_login::token_data; -mod truncate; mod unified_exec; pub mod windows_sandbox; pub use client::X_RESPONSESAPI_INCLUDE_TIMING_METRICS_HEADER; diff --git a/codex-rs/core/src/memories/prompts.rs b/codex-rs/core/src/memories/prompts.rs index 1659e1c1c7..851179426f 100644 --- a/codex-rs/core/src/memories/prompts.rs +++ b/codex-rs/core/src/memories/prompts.rs @@ -1,13 +1,13 @@ use crate::memories::memory_root; use crate::memories::phase_one; use crate::memories::storage::rollout_summary_file_stem_from_parts; -use crate::truncate::TruncationPolicy; -use crate::truncate::truncate_text; use askama::Template; use codex_protocol::openai_models::ModelInfo; use codex_state::Phase2InputSelection; use codex_state::Stage1Output; use codex_state::Stage1OutputRef; +use codex_utils_output_truncation::TruncationPolicy; +use codex_utils_output_truncation::truncate_text; use std::path::Path; use tokio::fs; use tracing::warn; diff --git a/codex-rs/core/src/models_manager/model_info.rs b/codex-rs/core/src/models_manager/model_info.rs index 7d3c6e9d10..055a6459d4 100644 --- a/codex-rs/core/src/models_manager/model_info.rs +++ b/codex-rs/core/src/models_manager/model_info.rs @@ -10,8 +10,8 @@ use codex_protocol::openai_models::WebSearchToolType; use codex_protocol::openai_models::default_input_modalities; use crate::config::Config; -use crate::truncate::approx_bytes_for_tokens; use codex_features::Feature; +use codex_utils_output_truncation::approx_bytes_for_tokens; use tracing::warn; pub const BASE_INSTRUCTIONS: &str = include_str!("../../prompt.md"); diff --git a/codex-rs/core/src/realtime_context.rs b/codex-rs/core/src/realtime_context.rs index bcb24c5673..1b845db1e9 100644 --- a/codex-rs/core/src/realtime_context.rs +++ b/codex-rs/core/src/realtime_context.rs @@ -1,13 +1,13 @@ use crate::codex::Session; use crate::compact::content_items_to_text; use crate::event_mapping::is_contextual_user_message_content; -use crate::truncate::TruncationPolicy; -use crate::truncate::truncate_text; use chrono::Utc; use codex_git_utils::resolve_root_git_project_for_trust; use codex_protocol::models::ResponseItem; use codex_state::SortKey; use codex_state::ThreadMetadata; +use codex_utils_output_truncation::TruncationPolicy; +use codex_utils_output_truncation::truncate_text; use dirs::home_dir; use std::cmp::Reverse; use std::collections::HashMap; diff --git a/codex-rs/core/src/rollout/list.rs b/codex-rs/core/src/rollout/list.rs index bf32e23c8d..6ae5908d8a 100644 --- a/codex-rs/core/src/rollout/list.rs +++ b/codex-rs/core/src/rollout/list.rs @@ -1,7 +1,7 @@ use async_trait::async_trait; use std::cmp::Reverse; use std::ffi::OsStr; -use std::io::{self}; +use std::io; use std::num::NonZero; use std::ops::ControlFlow; use std::path::Path; diff --git a/codex-rs/core/src/rollout/recorder.rs b/codex-rs/core/src/rollout/recorder.rs index 25e7ab2034..30b0734df1 100644 --- a/codex-rs/core/src/rollout/recorder.rs +++ b/codex-rs/core/src/rollout/recorder.rs @@ -1,7 +1,7 @@ //! Persist Codex session rollouts (.jsonl) so sessions can be replayed or inspected later. +use std::fs; use std::fs::File; -use std::fs::{self}; use std::io::Error as IoError; use std::path::Path; use std::path::PathBuf; @@ -17,8 +17,8 @@ use time::format_description::FormatItem; use time::format_description::well_known::Rfc3339; use time::macros::format_description; use tokio::io::AsyncWriteExt; +use tokio::sync::mpsc; use tokio::sync::mpsc::Sender; -use tokio::sync::mpsc::{self}; use tokio::sync::oneshot; use tracing::info; use tracing::trace; @@ -44,8 +44,6 @@ use crate::default_client::originator; use crate::path_utils; use crate::state_db; use crate::state_db::StateDbHandle; -use crate::truncate::TruncationPolicy; -use crate::truncate::truncate_text; use codex_git_utils::collect_git_info; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::GitInfo as ProtocolGitInfo; @@ -58,6 +56,8 @@ use codex_protocol::protocol::SessionMetaLine; use codex_protocol::protocol::SessionSource; use codex_state::StateRuntime; use codex_state::ThreadMetadataBuilder; +use codex_utils_output_truncation::TruncationPolicy; +use codex_utils_output_truncation::truncate_text; /// Records all [`ResponseItem`]s for a session and flushes them to disk after /// every update. diff --git a/codex-rs/core/src/rollout/recorder_tests.rs b/codex-rs/core/src/rollout/recorder_tests.rs index 8ca7b58a6b..9443c877b4 100644 --- a/codex-rs/core/src/rollout/recorder_tests.rs +++ b/codex-rs/core/src/rollout/recorder_tests.rs @@ -10,8 +10,8 @@ use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::TurnContextItem; use codex_protocol::protocol::UserMessageEvent; use pretty_assertions::assert_eq; +use std::fs; use std::fs::File; -use std::fs::{self}; use std::io::Write; use std::path::Path; use std::path::PathBuf; diff --git a/codex-rs/core/src/rollout/tests.rs b/codex-rs/core/src/rollout/tests.rs index 44e536e50e..17c1792d32 100644 --- a/codex-rs/core/src/rollout/tests.rs +++ b/codex-rs/core/src/rollout/tests.rs @@ -1,9 +1,9 @@ #![allow(clippy::unwrap_used, clippy::expect_used)] use std::ffi::OsStr; +use std::fs; use std::fs::File; use std::fs::FileTimes; -use std::fs::{self}; use std::io::Write; use std::path::Path; diff --git a/codex-rs/core/src/state/session.rs b/codex-rs/core/src/state/session.rs index 81978160a6..1a14236163 100644 --- a/codex-rs/core/src/state/session.rs +++ b/codex-rs/core/src/state/session.rs @@ -13,8 +13,8 @@ use crate::protocol::RateLimitSnapshot; use crate::protocol::TokenUsage; use crate::protocol::TokenUsageInfo; use crate::session_startup_prewarm::SessionStartupPrewarmHandle; -use crate::truncate::TruncationPolicy; use codex_protocol::protocol::TurnContextItem; +use codex_utils_output_truncation::TruncationPolicy; /// Persistent, session-scoped state previously stored directly on `Session`. pub(crate) struct SessionState { diff --git a/codex-rs/core/src/tools/code_mode/mod.rs b/codex-rs/core/src/tools/code_mode/mod.rs index a4838d2463..a1f21a6227 100644 --- a/codex-rs/core/src/tools/code_mode/mod.rs +++ b/codex-rs/core/src/tools/code_mode/mod.rs @@ -27,11 +27,11 @@ use crate::tools::parallel::ToolCallRuntime; use crate::tools::router::ToolCall; use crate::tools::router::ToolCallSource; use crate::tools::router::ToolRouterParams; -use crate::truncate::TruncationPolicy; -use crate::truncate::formatted_truncate_text_content_items_with_policy; -use crate::truncate::truncate_function_output_items_with_policy; use crate::unified_exec::resolve_max_tokens; use codex_features::Feature; +use codex_utils_output_truncation::TruncationPolicy; +use codex_utils_output_truncation::formatted_truncate_text_content_items_with_policy; +use codex_utils_output_truncation::truncate_function_output_items_with_policy; pub(crate) use execute_handler::CodeModeExecuteHandler; use response_adapter::into_function_call_output_content_items; diff --git a/codex-rs/core/src/tools/context.rs b/codex-rs/core/src/tools/context.rs index 74efb0989b..12db451027 100644 --- a/codex-rs/core/src/tools/context.rs +++ b/codex-rs/core/src/tools/context.rs @@ -4,8 +4,6 @@ use crate::codex::TurnContext; use crate::tools::TELEMETRY_PREVIEW_MAX_BYTES; use crate::tools::TELEMETRY_PREVIEW_MAX_LINES; use crate::tools::TELEMETRY_PREVIEW_TRUNCATION_NOTICE; -use crate::truncate::TruncationPolicy; -use crate::truncate::formatted_truncate_text; use crate::turn_diff_tracker::TurnDiffTracker; use crate::unified_exec::resolve_max_tokens; use codex_protocol::mcp::CallToolResult; @@ -16,6 +14,8 @@ use codex_protocol::models::ResponseInputItem; use codex_protocol::models::SearchToolCallParams; use codex_protocol::models::ShellToolCallParams; use codex_protocol::models::function_call_output_content_items_to_text; +use codex_utils_output_truncation::TruncationPolicy; +use codex_utils_output_truncation::formatted_truncate_text; use codex_utils_string::take_bytes_at_char_boundary; use serde::Serialize; use serde_json::Value as JsonValue; diff --git a/codex-rs/core/src/tools/js_repl/mod.rs b/codex-rs/core/src/tools/js_repl/mod.rs index 27d8cb9e65..0a29fb1200 100644 --- a/codex-rs/core/src/tools/js_repl/mod.rs +++ b/codex-rs/core/src/tools/js_repl/mod.rs @@ -42,12 +42,12 @@ use crate::original_image_detail::normalize_output_image_detail; use crate::sandboxing::ExecOptions; use crate::tools::ToolRouter; use crate::tools::context::SharedTurnDiffTracker; -use crate::truncate::TruncationPolicy; -use crate::truncate::truncate_text; use codex_sandboxing::SandboxCommand; use codex_sandboxing::SandboxManager; use codex_sandboxing::SandboxTransformRequest; use codex_sandboxing::SandboxablePreference; +use codex_utils_output_truncation::TruncationPolicy; +use codex_utils_output_truncation::truncate_text; pub(crate) const JS_REPL_PRAGMA_PREFIX: &str = "// codex-js-repl:"; const KERNEL_SOURCE: &str = include_str!("kernel.js"); diff --git a/codex-rs/core/src/tools/mod.rs b/codex-rs/core/src/tools/mod.rs index 4e495190ec..87a01c8e3b 100644 --- a/codex-rs/core/src/tools/mod.rs +++ b/codex-rs/core/src/tools/mod.rs @@ -15,9 +15,9 @@ pub mod sandboxing; pub mod spec; use crate::exec::ExecToolCallOutput; -use crate::truncate::TruncationPolicy; -use crate::truncate::formatted_truncate_text; -use crate::truncate::truncate_text; +use codex_utils_output_truncation::TruncationPolicy; +use codex_utils_output_truncation::formatted_truncate_text; +use codex_utils_output_truncation::truncate_text; pub use router::ToolRouter; use serde::Serialize; diff --git a/codex-rs/core/src/truncate.rs b/codex-rs/core/src/truncate.rs deleted file mode 100644 index 707fbe22ef..0000000000 --- a/codex-rs/core/src/truncate.rs +++ /dev/null @@ -1,363 +0,0 @@ -//! Utilities for truncating large chunks of output while preserving a prefix -//! and suffix on UTF-8 boundaries, and helpers for line/token‑based truncation -//! used across the core crate. - -use codex_protocol::models::FunctionCallOutputContentItem; -use codex_protocol::openai_models::TruncationMode; -use codex_protocol::openai_models::TruncationPolicyConfig; -use codex_protocol::protocol::TruncationPolicy as ProtocolTruncationPolicy; - -const APPROX_BYTES_PER_TOKEN: usize = 4; - -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub enum TruncationPolicy { - Bytes(usize), - Tokens(usize), -} - -impl From for ProtocolTruncationPolicy { - fn from(value: TruncationPolicy) -> Self { - match value { - TruncationPolicy::Bytes(bytes) => Self::Bytes(bytes), - TruncationPolicy::Tokens(tokens) => Self::Tokens(tokens), - } - } -} - -impl From for TruncationPolicy { - fn from(config: TruncationPolicyConfig) -> Self { - match config.mode { - TruncationMode::Bytes => Self::Bytes(config.limit as usize), - TruncationMode::Tokens => Self::Tokens(config.limit as usize), - } - } -} - -impl TruncationPolicy { - /// Returns a token budget derived from this policy. - /// - /// - For `Tokens`, this is the explicit token limit. - /// - For `Bytes`, this is an approximate token budget using the global - /// bytes-per-token heuristic. - pub fn token_budget(&self) -> usize { - match self { - TruncationPolicy::Bytes(bytes) => { - usize::try_from(approx_tokens_from_byte_count(*bytes)).unwrap_or(usize::MAX) - } - TruncationPolicy::Tokens(tokens) => *tokens, - } - } - - /// Returns a byte budget derived from this policy. - /// - /// - For `Bytes`, this is the explicit byte limit. - /// - For `Tokens`, this is an approximate byte budget using the global - /// bytes-per-token heuristic. - pub fn byte_budget(&self) -> usize { - match self { - TruncationPolicy::Bytes(bytes) => *bytes, - TruncationPolicy::Tokens(tokens) => approx_bytes_for_tokens(*tokens), - } - } -} - -impl std::ops::Mul for TruncationPolicy { - type Output = Self; - - fn mul(self, multiplier: f64) -> Self::Output { - match self { - TruncationPolicy::Bytes(bytes) => { - TruncationPolicy::Bytes((bytes as f64 * multiplier).ceil() as usize) - } - TruncationPolicy::Tokens(tokens) => { - TruncationPolicy::Tokens((tokens as f64 * multiplier).ceil() as usize) - } - } - } -} - -pub(crate) fn formatted_truncate_text(content: &str, policy: TruncationPolicy) -> String { - if content.len() <= policy.byte_budget() { - return content.to_string(); - } - let total_lines = content.lines().count(); - let result = truncate_text(content, policy); - format!("Total output lines: {total_lines}\n\n{result}") -} - -pub(crate) fn truncate_text(content: &str, policy: TruncationPolicy) -> String { - match policy { - TruncationPolicy::Bytes(_) => truncate_with_byte_estimate(content, policy), - TruncationPolicy::Tokens(_) => { - let (truncated, _) = truncate_with_token_budget(content, policy); - truncated - } - } -} - -pub(crate) fn formatted_truncate_text_content_items_with_policy( - items: &[FunctionCallOutputContentItem], - policy: TruncationPolicy, -) -> (Vec, Option) { - let text_segments = items - .iter() - .filter_map(|item| match item { - FunctionCallOutputContentItem::InputText { text } => Some(text.as_str()), - FunctionCallOutputContentItem::InputImage { .. } => None, - }) - .collect::>(); - - if text_segments.is_empty() { - return (items.to_vec(), None); - } - - let mut combined = String::new(); - for text in &text_segments { - if !combined.is_empty() { - combined.push('\n'); - } - combined.push_str(text); - } - - if combined.len() <= policy.byte_budget() { - return (items.to_vec(), None); - } - - let mut out = vec![FunctionCallOutputContentItem::InputText { - text: formatted_truncate_text(&combined, policy), - }]; - out.extend(items.iter().filter_map(|item| match item { - FunctionCallOutputContentItem::InputImage { image_url, detail } => { - Some(FunctionCallOutputContentItem::InputImage { - image_url: image_url.clone(), - detail: *detail, - }) - } - FunctionCallOutputContentItem::InputText { .. } => None, - })); - - (out, Some(approx_token_count(&combined))) -} - -/// Globally truncate function output items to fit within the given -/// truncation policy's budget, preserving as many text/image items as -/// possible and appending a summary for any omitted text items. -pub(crate) fn truncate_function_output_items_with_policy( - items: &[FunctionCallOutputContentItem], - policy: TruncationPolicy, -) -> Vec { - let mut out: Vec = Vec::with_capacity(items.len()); - let mut remaining_budget = match policy { - TruncationPolicy::Bytes(_) => policy.byte_budget(), - TruncationPolicy::Tokens(_) => policy.token_budget(), - }; - let mut omitted_text_items = 0usize; - - for it in items { - match it { - FunctionCallOutputContentItem::InputText { text } => { - if remaining_budget == 0 { - omitted_text_items += 1; - continue; - } - - let cost = match policy { - TruncationPolicy::Bytes(_) => text.len(), - TruncationPolicy::Tokens(_) => approx_token_count(text), - }; - - if cost <= remaining_budget { - out.push(FunctionCallOutputContentItem::InputText { text: text.clone() }); - remaining_budget = remaining_budget.saturating_sub(cost); - } else { - let snippet_policy = match policy { - TruncationPolicy::Bytes(_) => TruncationPolicy::Bytes(remaining_budget), - TruncationPolicy::Tokens(_) => TruncationPolicy::Tokens(remaining_budget), - }; - let snippet = truncate_text(text, snippet_policy); - if snippet.is_empty() { - omitted_text_items += 1; - } else { - out.push(FunctionCallOutputContentItem::InputText { text: snippet }); - } - remaining_budget = 0; - } - } - FunctionCallOutputContentItem::InputImage { image_url, detail } => { - out.push(FunctionCallOutputContentItem::InputImage { - image_url: image_url.clone(), - detail: *detail, - }); - } - } - } - - if omitted_text_items > 0 { - out.push(FunctionCallOutputContentItem::InputText { - text: format!("[omitted {omitted_text_items} text items ...]"), - }); - } - - out -} - -/// Truncate the middle of a UTF-8 string to at most `max_tokens` tokens, -/// preserving the beginning and the end. Returns the possibly truncated string -/// and `Some(original_token_count)` if truncation occurred; otherwise returns -/// the original string and `None`. -fn truncate_with_token_budget(s: &str, policy: TruncationPolicy) -> (String, Option) { - if s.is_empty() { - return (String::new(), None); - } - let max_tokens = policy.token_budget(); - - let byte_len = s.len(); - if max_tokens > 0 && byte_len <= approx_bytes_for_tokens(max_tokens) { - return (s.to_string(), None); - } - - let truncated = truncate_with_byte_estimate(s, policy); - let approx_total_usize = approx_token_count(s); - let approx_total = u64::try_from(approx_total_usize).unwrap_or(u64::MAX); - if truncated == s { - (truncated, None) - } else { - (truncated, Some(approx_total)) - } -} - -/// Truncate a string using a byte budget derived from the token budget, without -/// performing any real tokenization. This keeps the logic purely byte-based and -/// uses a bytes placeholder in the truncated output. -fn truncate_with_byte_estimate(s: &str, policy: TruncationPolicy) -> String { - if s.is_empty() { - return String::new(); - } - - let total_chars = s.chars().count(); - let max_bytes = policy.byte_budget(); - - if max_bytes == 0 { - // No budget to show content; just report that everything was truncated. - let marker = format_truncation_marker( - policy, - removed_units_for_source(policy, s.len(), total_chars), - ); - return marker; - } - - if s.len() <= max_bytes { - return s.to_string(); - } - - let total_bytes = s.len(); - - let (left_budget, right_budget) = split_budget(max_bytes); - - let (removed_chars, left, right) = split_string(s, left_budget, right_budget); - - let marker = format_truncation_marker( - policy, - removed_units_for_source(policy, total_bytes.saturating_sub(max_bytes), removed_chars), - ); - - assemble_truncated_output(left, right, &marker) -} - -fn split_string(s: &str, beginning_bytes: usize, end_bytes: usize) -> (usize, &str, &str) { - if s.is_empty() { - return (0, "", ""); - } - - let len = s.len(); - let tail_start_target = len.saturating_sub(end_bytes); - let mut prefix_end = 0usize; - let mut suffix_start = len; - let mut removed_chars = 0usize; - let mut suffix_started = false; - - for (idx, ch) in s.char_indices() { - let char_end = idx + ch.len_utf8(); - if char_end <= beginning_bytes { - prefix_end = char_end; - continue; - } - - if idx >= tail_start_target { - if !suffix_started { - suffix_start = idx; - suffix_started = true; - } - continue; - } - - removed_chars = removed_chars.saturating_add(1); - } - - if suffix_start < prefix_end { - suffix_start = prefix_end; - } - - let before = &s[..prefix_end]; - let after = &s[suffix_start..]; - - (removed_chars, before, after) -} - -fn format_truncation_marker(policy: TruncationPolicy, removed_count: u64) -> String { - match policy { - TruncationPolicy::Tokens(_) => format!("…{removed_count} tokens truncated…"), - TruncationPolicy::Bytes(_) => format!("…{removed_count} chars truncated…"), - } -} - -fn split_budget(budget: usize) -> (usize, usize) { - let left = budget / 2; - (left, budget - left) -} - -fn removed_units_for_source( - policy: TruncationPolicy, - removed_bytes: usize, - removed_chars: usize, -) -> u64 { - match policy { - TruncationPolicy::Tokens(_) => approx_tokens_from_byte_count(removed_bytes), - TruncationPolicy::Bytes(_) => u64::try_from(removed_chars).unwrap_or(u64::MAX), - } -} - -fn assemble_truncated_output(prefix: &str, suffix: &str, marker: &str) -> String { - let mut out = String::with_capacity(prefix.len() + marker.len() + suffix.len() + 1); - out.push_str(prefix); - out.push_str(marker); - out.push_str(suffix); - out -} - -pub(crate) fn approx_token_count(text: &str) -> usize { - let len = text.len(); - len.saturating_add(APPROX_BYTES_PER_TOKEN.saturating_sub(1)) / APPROX_BYTES_PER_TOKEN -} - -pub(crate) fn approx_bytes_for_tokens(tokens: usize) -> usize { - tokens.saturating_mul(APPROX_BYTES_PER_TOKEN) -} - -pub(crate) fn approx_tokens_from_byte_count(bytes: usize) -> u64 { - let bytes_u64 = bytes as u64; - bytes_u64.saturating_add((APPROX_BYTES_PER_TOKEN as u64).saturating_sub(1)) - / (APPROX_BYTES_PER_TOKEN as u64) -} - -pub(crate) fn approx_tokens_from_byte_count_i64(bytes: i64) -> i64 { - if bytes <= 0 { - return 0; - } - let bytes = usize::try_from(bytes).unwrap_or(usize::MAX); - i64::try_from(approx_tokens_from_byte_count(bytes)).unwrap_or(i64::MAX) -} - -#[cfg(test)] -#[path = "truncate_tests.rs"] -mod tests; diff --git a/codex-rs/core/src/unified_exec/process.rs b/codex-rs/core/src/unified_exec/process.rs index 58faed27de..68cf18bce1 100644 --- a/codex-rs/core/src/unified_exec/process.rs +++ b/codex-rs/core/src/unified_exec/process.rs @@ -15,9 +15,9 @@ use tokio_util::sync::CancellationToken; use crate::exec::ExecToolCallOutput; use crate::exec::StreamOutput; use crate::exec::is_likely_sandbox_denied; -use crate::truncate::TruncationPolicy; -use crate::truncate::formatted_truncate_text; use codex_sandboxing::SandboxType; +use codex_utils_output_truncation::TruncationPolicy; +use codex_utils_output_truncation::formatted_truncate_text; use codex_utils_pty::ExecCommandSession; use codex_utils_pty::SpawnedPty; diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 52d668c000..a760249d61 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -27,7 +27,6 @@ use crate::tools::orchestrator::ToolOrchestrator; use crate::tools::runtimes::unified_exec::UnifiedExecRequest as UnifiedExecToolRequest; use crate::tools::runtimes::unified_exec::UnifiedExecRuntime; use crate::tools::sandboxing::ToolCtx; -use crate::truncate::approx_token_count; use crate::unified_exec::ExecCommandRequest; use crate::unified_exec::MAX_UNIFIED_EXEC_PROCESSES; use crate::unified_exec::MAX_YIELD_TIME_MS; @@ -50,6 +49,7 @@ use crate::unified_exec::process::OutputBuffer; use crate::unified_exec::process::OutputHandles; use crate::unified_exec::process::SpawnLifecycleHandle; use crate::unified_exec::process::UnifiedExecProcess; +use codex_utils_output_truncation::approx_token_count; const UNIFIED_EXEC_ENV: [(&str, &str); 10] = [ ("NO_COLOR", "1"), diff --git a/codex-rs/protocol/Cargo.toml b/codex-rs/protocol/Cargo.toml index eaaf791bb8..ebfdaddabb 100644 --- a/codex-rs/protocol/Cargo.toml +++ b/codex-rs/protocol/Cargo.toml @@ -16,6 +16,7 @@ codex-execpolicy = { workspace = true } codex-git-utils = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-image = { workspace = true } +codex-utils-string = { workspace = true } icu_decimal = { workspace = true } icu_locale_core = { workspace = true } icu_provider = { workspace = true, features = ["sync"] } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index c40514e3db..6856f19417 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -7,6 +7,7 @@ use std::collections::HashMap; use std::collections::HashSet; use std::ffi::OsStr; use std::fmt; +use std::ops::Mul; use std::path::Path; use std::path::PathBuf; use std::str::FromStr; @@ -2629,6 +2630,51 @@ pub enum TruncationPolicy { Tokens(usize), } +impl From for TruncationPolicy { + fn from(config: crate::openai_models::TruncationPolicyConfig) -> Self { + match config.mode { + crate::openai_models::TruncationMode::Bytes => Self::Bytes(config.limit as usize), + crate::openai_models::TruncationMode::Tokens => Self::Tokens(config.limit as usize), + } + } +} + +impl TruncationPolicy { + pub fn token_budget(&self) -> usize { + match self { + TruncationPolicy::Bytes(bytes) => { + usize::try_from(codex_utils_string::approx_tokens_from_byte_count(*bytes)) + .unwrap_or(usize::MAX) + } + TruncationPolicy::Tokens(tokens) => *tokens, + } + } + + pub fn byte_budget(&self) -> usize { + match self { + TruncationPolicy::Bytes(bytes) => *bytes, + TruncationPolicy::Tokens(tokens) => { + codex_utils_string::approx_bytes_for_tokens(*tokens) + } + } + } +} + +impl Mul for TruncationPolicy { + type Output = Self; + + fn mul(self, multiplier: f64) -> Self::Output { + match self { + TruncationPolicy::Bytes(bytes) => { + TruncationPolicy::Bytes((bytes as f64 * multiplier).ceil() as usize) + } + TruncationPolicy::Tokens(tokens) => { + TruncationPolicy::Tokens((tokens as f64 * multiplier).ceil() as usize) + } + } + } +} + #[derive(Serialize, Deserialize, Clone, JsonSchema)] pub struct RolloutLine { pub timestamp: String, diff --git a/codex-rs/utils/output-truncation/BUILD.bazel b/codex-rs/utils/output-truncation/BUILD.bazel new file mode 100644 index 0000000000..eefd22ddb2 --- /dev/null +++ b/codex-rs/utils/output-truncation/BUILD.bazel @@ -0,0 +1,6 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "output-truncation", + crate_name = "codex_utils_output_truncation", +) diff --git a/codex-rs/utils/output-truncation/Cargo.toml b/codex-rs/utils/output-truncation/Cargo.toml new file mode 100644 index 0000000000..7ad0ccfd46 --- /dev/null +++ b/codex-rs/utils/output-truncation/Cargo.toml @@ -0,0 +1,15 @@ +[package] +edition.workspace = true +license.workspace = true +name = "codex-utils-output-truncation" +version.workspace = true + +[lints] +workspace = true + +[dependencies] +codex-protocol = { workspace = true } +codex-utils-string = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } diff --git a/codex-rs/utils/output-truncation/src/lib.rs b/codex-rs/utils/output-truncation/src/lib.rs new file mode 100644 index 0000000000..24b1630da1 --- /dev/null +++ b/codex-rs/utils/output-truncation/src/lib.rs @@ -0,0 +1,142 @@ +//! Helpers for truncating tool and exec output using [`TruncationPolicy`](codex_protocol::protocol::TruncationPolicy). + +use codex_protocol::models::FunctionCallOutputContentItem; +pub use codex_utils_string::approx_bytes_for_tokens; +pub use codex_utils_string::approx_token_count; +pub use codex_utils_string::approx_tokens_from_byte_count; +use codex_utils_string::truncate_middle_chars; +use codex_utils_string::truncate_middle_with_token_budget; + +pub use codex_protocol::protocol::TruncationPolicy; + +pub fn formatted_truncate_text(content: &str, policy: TruncationPolicy) -> String { + if content.len() <= policy.byte_budget() { + return content.to_string(); + } + + let total_lines = content.lines().count(); + let result = truncate_text(content, policy); + format!("Total output lines: {total_lines}\n\n{result}") +} + +pub fn truncate_text(content: &str, policy: TruncationPolicy) -> String { + match policy { + TruncationPolicy::Bytes(bytes) => truncate_middle_chars(content, bytes), + TruncationPolicy::Tokens(tokens) => truncate_middle_with_token_budget(content, tokens).0, + } +} + +pub fn formatted_truncate_text_content_items_with_policy( + items: &[FunctionCallOutputContentItem], + policy: TruncationPolicy, +) -> (Vec, Option) { + let text_segments = items + .iter() + .filter_map(|item| match item { + FunctionCallOutputContentItem::InputText { text } => Some(text.as_str()), + FunctionCallOutputContentItem::InputImage { .. } => None, + }) + .collect::>(); + + if text_segments.is_empty() { + return (items.to_vec(), None); + } + + let mut combined = String::new(); + for text in &text_segments { + if !combined.is_empty() { + combined.push('\n'); + } + combined.push_str(text); + } + + if combined.len() <= policy.byte_budget() { + return (items.to_vec(), None); + } + + let mut out = vec![FunctionCallOutputContentItem::InputText { + text: formatted_truncate_text(&combined, policy), + }]; + out.extend(items.iter().filter_map(|item| match item { + FunctionCallOutputContentItem::InputImage { image_url, detail } => { + Some(FunctionCallOutputContentItem::InputImage { + image_url: image_url.clone(), + detail: *detail, + }) + } + FunctionCallOutputContentItem::InputText { .. } => None, + })); + + (out, Some(approx_token_count(&combined))) +} + +pub fn truncate_function_output_items_with_policy( + items: &[FunctionCallOutputContentItem], + policy: TruncationPolicy, +) -> Vec { + let mut out: Vec = Vec::with_capacity(items.len()); + let mut remaining_budget = match policy { + TruncationPolicy::Bytes(_) => policy.byte_budget(), + TruncationPolicy::Tokens(_) => policy.token_budget(), + }; + let mut omitted_text_items = 0usize; + + for item in items { + match item { + FunctionCallOutputContentItem::InputText { text } => { + if remaining_budget == 0 { + omitted_text_items += 1; + continue; + } + + let cost = match policy { + TruncationPolicy::Bytes(_) => text.len(), + TruncationPolicy::Tokens(_) => approx_token_count(text), + }; + + if cost <= remaining_budget { + out.push(FunctionCallOutputContentItem::InputText { text: text.clone() }); + remaining_budget = remaining_budget.saturating_sub(cost); + } else { + let snippet_policy = match policy { + TruncationPolicy::Bytes(_) => TruncationPolicy::Bytes(remaining_budget), + TruncationPolicy::Tokens(_) => TruncationPolicy::Tokens(remaining_budget), + }; + let snippet = truncate_text(text, snippet_policy); + if snippet.is_empty() { + omitted_text_items += 1; + } else { + out.push(FunctionCallOutputContentItem::InputText { text: snippet }); + } + remaining_budget = 0; + } + } + FunctionCallOutputContentItem::InputImage { image_url, detail } => { + out.push(FunctionCallOutputContentItem::InputImage { + image_url: image_url.clone(), + detail: *detail, + }); + } + } + } + + if omitted_text_items > 0 { + out.push(FunctionCallOutputContentItem::InputText { + text: format!("[omitted {omitted_text_items} text items ...]"), + }); + } + + out +} + +pub fn approx_tokens_from_byte_count_i64(bytes: i64) -> i64 { + if bytes <= 0 { + return 0; + } + + let bytes = usize::try_from(bytes).unwrap_or(usize::MAX); + i64::try_from(approx_tokens_from_byte_count(bytes)).unwrap_or(i64::MAX) +} + +#[cfg(test)] +mod truncate_tests; diff --git a/codex-rs/core/src/truncate_tests.rs b/codex-rs/utils/output-truncation/src/truncate_tests.rs similarity index 77% rename from codex-rs/core/src/truncate_tests.rs rename to codex-rs/utils/output-truncation/src/truncate_tests.rs index 5a61a9a26d..f159a6b62f 100644 --- a/codex-rs/core/src/truncate_tests.rs +++ b/codex-rs/utils/output-truncation/src/truncate_tests.rs @@ -1,49 +1,13 @@ -use super::TruncationPolicy; -use super::approx_token_count; -use super::formatted_truncate_text; -use super::formatted_truncate_text_content_items_with_policy; -use super::split_string; -use super::truncate_function_output_items_with_policy; -use super::truncate_text; -use super::truncate_with_token_budget; +use crate::TruncationPolicy; +use crate::approx_token_count; +use crate::approx_tokens_from_byte_count_i64; +use crate::formatted_truncate_text; +use crate::formatted_truncate_text_content_items_with_policy; +use crate::truncate_function_output_items_with_policy; +use crate::truncate_text; use codex_protocol::models::FunctionCallOutputContentItem; use pretty_assertions::assert_eq; -#[test] -fn split_string_works() { - assert_eq!(split_string("hello world", 5, 5), (1, "hello", "world")); - assert_eq!(split_string("abc", 0, 0), (3, "", "")); -} - -#[test] -fn split_string_handles_empty_string() { - assert_eq!(split_string("", 4, 4), (0, "", "")); -} - -#[test] -fn split_string_only_keeps_prefix_when_tail_budget_is_zero() { - assert_eq!(split_string("abcdef", 3, 0), (3, "abc", "")); -} - -#[test] -fn split_string_only_keeps_suffix_when_prefix_budget_is_zero() { - assert_eq!(split_string("abcdef", 0, 3), (3, "", "def")); -} - -#[test] -fn split_string_handles_overlapping_budgets_without_removal() { - assert_eq!(split_string("abcdef", 4, 4), (0, "abcd", "ef")); -} - -#[test] -fn split_string_respects_utf8_boundaries() { - assert_eq!(split_string("😀abc😀", 5, 5), (1, "😀a", "c😀")); - - assert_eq!(split_string("😀😀😀😀😀", 1, 1), (5, "", "")); - assert_eq!(split_string("😀😀😀😀😀", 7, 7), (3, "😀", "😀")); - assert_eq!(split_string("😀😀😀😀😀", 8, 8), (1, "😀😀", "😀😀")); -} - #[test] fn truncate_bytes_less_than_placeholder_returns_placeholder() { let content = "example output"; @@ -126,31 +90,6 @@ fn truncate_tokens_reports_original_line_count_when_truncated() { ); } -#[test] -fn truncate_with_token_budget_returns_original_when_under_limit() { - let s = "short output"; - let limit = 100; - let (out, original) = truncate_with_token_budget(s, TruncationPolicy::Tokens(limit)); - assert_eq!(out, s); - assert_eq!(original, None); -} - -#[test] -fn truncate_with_token_budget_reports_truncation_at_zero_limit() { - let s = "abcdef"; - let (out, original) = truncate_with_token_budget(s, TruncationPolicy::Tokens(0)); - assert_eq!(out, "…2 tokens truncated…"); - assert_eq!(original, Some(2)); -} - -#[test] -fn truncate_middle_tokens_handles_utf8_content() { - let s = "😀😀😀😀😀😀😀😀😀😀\nsecond line with text\n"; - let (out, tokens) = truncate_with_token_budget(s, TruncationPolicy::Tokens(8)); - assert_eq!(out, "😀😀😀😀…8 tokens truncated… line with text\n"); - assert_eq!(tokens, Some(16)); -} - #[test] fn truncate_middle_bytes_handles_utf8_content() { let s = "😀😀😀😀😀😀😀😀😀😀\nsecond line with text\n"; @@ -185,7 +124,6 @@ fn truncates_across_multiple_under_limit_texts_and_reports_omitted() { let output = truncate_function_output_items_with_policy(&items, TruncationPolicy::Tokens(limit)); - // Expect: t1 (full), t2 (full), image, t3 (truncated), summary mentioning 2 omitted. assert_eq!(output.len(), 5); let first_text = match &output[0] { @@ -245,6 +183,29 @@ fn formatted_truncate_text_content_items_with_policy_returns_original_under_limi assert_eq!(original_token_count, None); } +#[test] +fn formatted_truncate_text_content_items_with_policy_preserves_empty_leading_text_behavior() { + let items = vec![ + FunctionCallOutputContentItem::InputText { + text: String::new(), + }, + FunctionCallOutputContentItem::InputText { + text: "abc".to_string(), + }, + ]; + + let (output, original_token_count) = + formatted_truncate_text_content_items_with_policy(&items, TruncationPolicy::Bytes(0)); + + assert_eq!( + output, + vec![FunctionCallOutputContentItem::InputText { + text: "Total output lines: 1\n\n…3 chars truncated…".to_string(), + }] + ); + assert_eq!(original_token_count, Some(1)); +} + #[test] fn formatted_truncate_text_content_items_with_policy_merges_text_and_appends_images() { let items = vec![ @@ -311,3 +272,10 @@ fn formatted_truncate_text_content_items_with_policy_merges_all_text_for_token_b ); assert_eq!(original_token_count, Some(5)); } + +#[test] +fn byte_count_conversion_clamps_non_positive_values() { + assert_eq!(approx_tokens_from_byte_count_i64(/*bytes*/ -1), 0); + assert_eq!(approx_tokens_from_byte_count_i64(/*bytes*/ 0), 0); + assert_eq!(approx_tokens_from_byte_count_i64(/*bytes*/ 5), 2); +} diff --git a/codex-rs/utils/string/src/lib.rs b/codex-rs/utils/string/src/lib.rs index 0cb218f349..a667a517b2 100644 --- a/codex-rs/utils/string/src/lib.rs +++ b/codex-rs/utils/string/src/lib.rs @@ -1,3 +1,11 @@ +mod truncate; + +pub use truncate::approx_bytes_for_tokens; +pub use truncate::approx_token_count; +pub use truncate::approx_tokens_from_byte_count; +pub use truncate::truncate_middle_chars; +pub use truncate::truncate_middle_with_token_budget; + // Truncate a &str to a byte budget at a char boundary (prefix) #[inline] pub fn take_bytes_at_char_boundary(s: &str, maxb: usize) -> &str { @@ -112,6 +120,7 @@ fn parse_markdown_hash_location_point(point: &str) -> Option<(&str, Option<&str> } #[cfg(test)] +#[allow(warnings, clippy::all)] mod tests { use super::find_uuids; use super::normalize_markdown_hash_location_suffix; diff --git a/codex-rs/utils/string/src/truncate.rs b/codex-rs/utils/string/src/truncate.rs new file mode 100644 index 0000000000..fa5b871774 --- /dev/null +++ b/codex-rs/utils/string/src/truncate.rs @@ -0,0 +1,156 @@ +//! Utilities for truncating large chunks of output while preserving a prefix +//! and suffix on UTF-8 boundaries. + +const APPROX_BYTES_PER_TOKEN: usize = 4; + +/// Truncate a string to `max_bytes` using a character-count marker. +pub fn truncate_middle_chars(s: &str, max_bytes: usize) -> String { + truncate_with_byte_estimate(s, max_bytes, /*use_tokens*/ false) +} + +/// Truncate the middle of a UTF-8 string to at most `max_tokens` approximate +/// tokens, preserving the beginning and the end. Returns the possibly +/// truncated string and `Some(original_token_count)` if truncation occurred; +/// otherwise returns the original string and `None`. +pub fn truncate_middle_with_token_budget(s: &str, max_tokens: usize) -> (String, Option) { + if s.is_empty() { + return (String::new(), None); + } + + if max_tokens > 0 && s.len() <= approx_bytes_for_tokens(max_tokens) { + return (s.to_string(), None); + } + + let truncated = truncate_with_byte_estimate( + s, + approx_bytes_for_tokens(max_tokens), + /*use_tokens*/ true, + ); + let total_tokens = u64::try_from(approx_token_count(s)).unwrap_or(u64::MAX); + + if truncated == s { + (truncated, None) + } else { + (truncated, Some(total_tokens)) + } +} + +fn truncate_with_byte_estimate(s: &str, max_bytes: usize, use_tokens: bool) -> String { + if s.is_empty() { + return String::new(); + } + + let total_chars = s.chars().count(); + + if max_bytes == 0 { + return format_truncation_marker( + use_tokens, + removed_units(use_tokens, s.len(), total_chars), + ); + } + + if s.len() <= max_bytes { + return s.to_string(); + } + + let total_bytes = s.len(); + let (left_budget, right_budget) = split_budget(max_bytes); + let (removed_chars, left, right) = split_string(s, left_budget, right_budget); + let marker = format_truncation_marker( + use_tokens, + removed_units( + use_tokens, + total_bytes.saturating_sub(max_bytes), + removed_chars, + ), + ); + + assemble_truncated_output(left, right, &marker) +} + +pub fn approx_token_count(text: &str) -> usize { + let len = text.len(); + len.saturating_add(APPROX_BYTES_PER_TOKEN.saturating_sub(1)) / APPROX_BYTES_PER_TOKEN +} + +pub fn approx_bytes_for_tokens(tokens: usize) -> usize { + tokens.saturating_mul(APPROX_BYTES_PER_TOKEN) +} + +pub fn approx_tokens_from_byte_count(bytes: usize) -> u64 { + let bytes_u64 = bytes as u64; + bytes_u64.saturating_add((APPROX_BYTES_PER_TOKEN as u64).saturating_sub(1)) + / (APPROX_BYTES_PER_TOKEN as u64) +} + +fn split_string(s: &str, beginning_bytes: usize, end_bytes: usize) -> (usize, &str, &str) { + if s.is_empty() { + return (0, "", ""); + } + + let len = s.len(); + let tail_start_target = len.saturating_sub(end_bytes); + let mut prefix_end = 0usize; + let mut suffix_start = len; + let mut removed_chars = 0usize; + let mut suffix_started = false; + + for (idx, ch) in s.char_indices() { + let char_end = idx + ch.len_utf8(); + if char_end <= beginning_bytes { + prefix_end = char_end; + continue; + } + + if idx >= tail_start_target { + if !suffix_started { + suffix_start = idx; + suffix_started = true; + } + continue; + } + + removed_chars = removed_chars.saturating_add(1); + } + + if suffix_start < prefix_end { + suffix_start = prefix_end; + } + + let before = &s[..prefix_end]; + let after = &s[suffix_start..]; + + (removed_chars, before, after) +} + +fn split_budget(budget: usize) -> (usize, usize) { + let left = budget / 2; + (left, budget - left) +} + +fn format_truncation_marker(use_tokens: bool, removed_count: u64) -> String { + if use_tokens { + format!("…{removed_count} tokens truncated…") + } else { + format!("…{removed_count} chars truncated…") + } +} + +fn removed_units(use_tokens: bool, removed_bytes: usize, removed_chars: usize) -> u64 { + if use_tokens { + approx_tokens_from_byte_count(removed_bytes) + } else { + u64::try_from(removed_chars).unwrap_or(u64::MAX) + } +} + +fn assemble_truncated_output(prefix: &str, suffix: &str, marker: &str) -> String { + let mut out = String::with_capacity(prefix.len() + marker.len() + suffix.len() + 1); + out.push_str(prefix); + out.push_str(marker); + out.push_str(suffix); + out +} + +#[cfg(test)] +mod tests; diff --git a/codex-rs/utils/string/src/truncate/tests.rs b/codex-rs/utils/string/src/truncate/tests.rs new file mode 100644 index 0000000000..5d517e86d4 --- /dev/null +++ b/codex-rs/utils/string/src/truncate/tests.rs @@ -0,0 +1,71 @@ +use super::split_string; +use super::truncate_middle_chars; +use super::truncate_middle_with_token_budget; +use pretty_assertions::assert_eq; + +#[test] +fn split_string_works() { + assert_eq!(split_string("hello world", 5, 5), (1, "hello", "world")); + assert_eq!(split_string("abc", 0, 0), (3, "", "")); +} + +#[test] +fn split_string_handles_empty_string() { + assert_eq!(split_string("", 4, 4), (0, "", "")); +} + +#[test] +fn split_string_only_keeps_prefix_when_tail_budget_is_zero() { + assert_eq!(split_string("abcdef", 3, 0), (3, "abc", "")); +} + +#[test] +fn split_string_only_keeps_suffix_when_prefix_budget_is_zero() { + assert_eq!(split_string("abcdef", 0, 3), (3, "", "def")); +} + +#[test] +fn split_string_handles_overlapping_budgets_without_removal() { + assert_eq!(split_string("abcdef", 4, 4), (0, "abcd", "ef")); +} + +#[test] +fn split_string_respects_utf8_boundaries() { + assert_eq!(split_string("😀abc😀", 5, 5), (1, "😀a", "c😀")); + + assert_eq!(split_string("😀😀😀😀😀", 1, 1), (5, "", "")); + assert_eq!(split_string("😀😀😀😀😀", 7, 7), (3, "😀", "😀")); + assert_eq!(split_string("😀😀😀😀😀", 8, 8), (1, "😀😀", "😀😀")); +} + +#[test] +fn truncate_with_token_budget_returns_original_when_under_limit() { + let s = "short output"; + let limit = 100; + let (out, original) = truncate_middle_with_token_budget(s, limit); + assert_eq!(out, s); + assert_eq!(original, None); +} + +#[test] +fn truncate_with_token_budget_reports_truncation_at_zero_limit() { + let s = "abcdef"; + let (out, original) = truncate_middle_with_token_budget(s, 0); + assert_eq!(out, "…2 tokens truncated…"); + assert_eq!(original, Some(2)); +} + +#[test] +fn truncate_middle_tokens_handles_utf8_content() { + let s = "😀😀😀😀😀😀😀😀😀😀\nsecond line with text\n"; + let (out, tokens) = truncate_middle_with_token_budget(s, 8); + assert_eq!(out, "😀😀😀😀…8 tokens truncated… line with text\n"); + assert_eq!(tokens, Some(16)); +} + +#[test] +fn truncate_middle_bytes_handles_utf8_content() { + let s = "😀😀😀😀😀😀😀😀😀😀\nsecond line with text\n"; + let out = truncate_middle_chars(s, 20); + assert_eq!(out, "😀😀…21 chars truncated…with text\n"); +} From 301b17c2a198c8c1ea9438dbf97949e226052f6c Mon Sep 17 00:00:00 2001 From: Ruslan Nigmatullin Date: Tue, 24 Mar 2026 15:52:13 -0700 Subject: [PATCH 3/8] app-server: add filesystem watch support (#14533) ### Summary Add the v2 app-server filesystem watch RPCs and notifications, wire them through the message processor, and implement connection-scoped watches with notify-backed change delivery. This also updates the schema fixtures, app-server documentation, and the v2 integration coverage for watch and unwatch behavior. This allows clients to efficiently watch for filesystem updates, e.g. to react on branch changes. ### Testing - exercise watch lifecycles for directory changes, atomic file replacement, missing-file targets, and unwatch cleanup --- .../schema/json/ClientRequest.json | 78 ++++ .../schema/json/ServerNotification.json | 45 +++ .../codex_app_server_protocol.schemas.json | 155 +++++++ .../codex_app_server_protocol.v2.schemas.json | 155 +++++++ .../schema/json/v2/FsChangedNotification.json | 29 ++ .../schema/json/v2/FsUnwatchParams.json | 15 + .../schema/json/v2/FsUnwatchResponse.json | 6 + .../schema/json/v2/FsWatchParams.json | 25 ++ .../schema/json/v2/FsWatchResponse.json | 30 ++ .../schema/typescript/ClientRequest.ts | 4 +- .../schema/typescript/ServerNotification.ts | 3 +- .../typescript/v2/FsChangedNotification.ts | 17 + .../schema/typescript/v2/FsUnwatchParams.ts | 12 + .../schema/typescript/v2/FsUnwatchResponse.ts | 8 + .../schema/typescript/v2/FsWatchParams.ts | 13 + .../schema/typescript/v2/FsWatchResponse.ts | 17 + .../schema/typescript/v2/index.ts | 5 + .../src/protocol/common.rs | 30 ++ .../app-server-protocol/src/protocol/v2.rs | 73 ++++ codex-rs/app-server/README.md | 25 ++ .../app-server/src/codex_message_processor.rs | 4 +- codex-rs/app-server/src/fs_api.rs | 4 +- codex-rs/app-server/src/fs_watch.rs | 379 ++++++++++++++++++ codex-rs/app-server/src/lib.rs | 1 + codex-rs/app-server/src/message_processor.rs | 53 +++ .../app-server/tests/common/mcp_process.rs | 15 + codex-rs/app-server/tests/suite/v2/fs.rs | 198 +++++++++ .../src/app/app_server_adapter.rs | 1 + codex-rs/tui_app_server/src/chatwidget.rs | 1 + 29 files changed, 1396 insertions(+), 5 deletions(-) create mode 100644 codex-rs/app-server-protocol/schema/json/v2/FsChangedNotification.json create mode 100644 codex-rs/app-server-protocol/schema/json/v2/FsUnwatchParams.json create mode 100644 codex-rs/app-server-protocol/schema/json/v2/FsUnwatchResponse.json create mode 100644 codex-rs/app-server-protocol/schema/json/v2/FsWatchParams.json create mode 100644 codex-rs/app-server-protocol/schema/json/v2/FsWatchResponse.json create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/FsChangedNotification.ts create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/FsUnwatchParams.ts create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/FsUnwatchResponse.ts create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/FsWatchParams.ts create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/FsWatchResponse.ts create mode 100644 codex-rs/app-server/src/fs_watch.rs diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index 903b26b80b..e245c17954 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -781,6 +781,36 @@ ], "type": "object" }, + "FsUnwatchParams": { + "description": "Stop filesystem watch notifications for a prior `fs/watch`.", + "properties": { + "watchId": { + "description": "Watch identifier returned by `fs/watch`.", + "type": "string" + } + }, + "required": [ + "watchId" + ], + "type": "object" + }, + "FsWatchParams": { + "description": "Start filesystem watch notifications for an absolute path.", + "properties": { + "path": { + "allOf": [ + { + "$ref": "#/definitions/AbsolutePathBuf" + } + ], + "description": "Absolute file or directory path to watch." + } + }, + "required": [ + "path" + ], + "type": "object" + }, "FsWriteFileParams": { "description": "Write a file on the host filesystem.", "properties": { @@ -4000,6 +4030,54 @@ "title": "Fs/copyRequest", "type": "object" }, + { + "properties": { + "id": { + "$ref": "#/definitions/RequestId" + }, + "method": { + "enum": [ + "fs/watch" + ], + "title": "Fs/watchRequestMethod", + "type": "string" + }, + "params": { + "$ref": "#/definitions/FsWatchParams" + } + }, + "required": [ + "id", + "method", + "params" + ], + "title": "Fs/watchRequest", + "type": "object" + }, + { + "properties": { + "id": { + "$ref": "#/definitions/RequestId" + }, + "method": { + "enum": [ + "fs/unwatch" + ], + "title": "Fs/unwatchRequestMethod", + "type": "string" + }, + "params": { + "$ref": "#/definitions/FsUnwatchParams" + } + }, + "required": [ + "id", + "method", + "params" + ], + "title": "Fs/unwatchRequest", + "type": "object" + }, { "properties": { "id": { diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index b576581cf9..37f7ff8c5f 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -1,6 +1,10 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { + "AbsolutePathBuf": { + "description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.", + "type": "string" + }, "AccountLoginCompletedNotification": { "properties": { "error": { @@ -998,6 +1002,27 @@ ], "type": "object" }, + "FsChangedNotification": { + "description": "Filesystem watch notification emitted for `fs/watch` subscribers.", + "properties": { + "changedPaths": { + "description": "File or directory paths associated with this event.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "watchId": { + "description": "Watch identifier returned by `fs/watch`.", + "type": "string" + } + }, + "required": [ + "changedPaths", + "watchId" + ], + "type": "object" + }, "FuzzyFileSearchMatchType": { "enum": [ "file", @@ -4394,6 +4419,26 @@ "title": "App/list/updatedNotification", "type": "object" }, + { + "properties": { + "method": { + "enum": [ + "fs/changed" + ], + "title": "Fs/changedNotificationMethod", + "type": "string" + }, + "params": { + "$ref": "#/definitions/FsChangedNotification" + } + }, + "required": [ + "method", + "params" + ], + "title": "Fs/changedNotification", + "type": "object" + }, { "properties": { "method": { 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 104a727f3c..91f32bede9 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 @@ -883,6 +883,54 @@ "title": "Fs/copyRequest", "type": "object" }, + { + "properties": { + "id": { + "$ref": "#/definitions/v2/RequestId" + }, + "method": { + "enum": [ + "fs/watch" + ], + "title": "Fs/watchRequestMethod", + "type": "string" + }, + "params": { + "$ref": "#/definitions/v2/FsWatchParams" + } + }, + "required": [ + "id", + "method", + "params" + ], + "title": "Fs/watchRequest", + "type": "object" + }, + { + "properties": { + "id": { + "$ref": "#/definitions/v2/RequestId" + }, + "method": { + "enum": [ + "fs/unwatch" + ], + "title": "Fs/unwatchRequestMethod", + "type": "string" + }, + "params": { + "$ref": "#/definitions/v2/FsUnwatchParams" + } + }, + "required": [ + "id", + "method", + "params" + ], + "title": "Fs/unwatchRequest", + "type": "object" + }, { "properties": { "id": { @@ -4053,6 +4101,26 @@ "title": "App/list/updatedNotification", "type": "object" }, + { + "properties": { + "method": { + "enum": [ + "fs/changed" + ], + "title": "Fs/changedNotificationMethod", + "type": "string" + }, + "params": { + "$ref": "#/definitions/v2/FsChangedNotification" + } + }, + "required": [ + "method", + "params" + ], + "title": "Fs/changedNotification", + "type": "object" + }, { "properties": { "method": { @@ -7444,6 +7512,29 @@ ], "type": "string" }, + "FsChangedNotification": { + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Filesystem watch notification emitted for `fs/watch` subscribers.", + "properties": { + "changedPaths": { + "description": "File or directory paths associated with this event.", + "items": { + "$ref": "#/definitions/v2/AbsolutePathBuf" + }, + "type": "array" + }, + "watchId": { + "description": "Watch identifier returned by `fs/watch`.", + "type": "string" + } + }, + "required": [ + "changedPaths", + "watchId" + ], + "title": "FsChangedNotification", + "type": "object" + }, "FsCopyParams": { "$schema": "http://json-schema.org/draft-07/schema#", "description": "Copy a file or directory tree on the host filesystem.", @@ -7698,6 +7789,70 @@ "title": "FsRemoveResponse", "type": "object" }, + "FsUnwatchParams": { + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Stop filesystem watch notifications for a prior `fs/watch`.", + "properties": { + "watchId": { + "description": "Watch identifier returned by `fs/watch`.", + "type": "string" + } + }, + "required": [ + "watchId" + ], + "title": "FsUnwatchParams", + "type": "object" + }, + "FsUnwatchResponse": { + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Successful response for `fs/unwatch`.", + "title": "FsUnwatchResponse", + "type": "object" + }, + "FsWatchParams": { + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Start filesystem watch notifications for an absolute path.", + "properties": { + "path": { + "allOf": [ + { + "$ref": "#/definitions/v2/AbsolutePathBuf" + } + ], + "description": "Absolute file or directory path to watch." + } + }, + "required": [ + "path" + ], + "title": "FsWatchParams", + "type": "object" + }, + "FsWatchResponse": { + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Created watch handle returned by `fs/watch`.", + "properties": { + "path": { + "allOf": [ + { + "$ref": "#/definitions/v2/AbsolutePathBuf" + } + ], + "description": "Canonicalized path associated with the watch." + }, + "watchId": { + "description": "Connection-scoped watch identifier used for `fs/unwatch` and `fs/changed`.", + "type": "string" + } + }, + "required": [ + "path", + "watchId" + ], + "title": "FsWatchResponse", + "type": "object" + }, "FsWriteFileParams": { "$schema": "http://json-schema.org/draft-07/schema#", "description": "Write a file on the host filesystem.", 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 8f4f962167..30bb2efb22 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 @@ -1417,6 +1417,54 @@ "title": "Fs/copyRequest", "type": "object" }, + { + "properties": { + "id": { + "$ref": "#/definitions/RequestId" + }, + "method": { + "enum": [ + "fs/watch" + ], + "title": "Fs/watchRequestMethod", + "type": "string" + }, + "params": { + "$ref": "#/definitions/FsWatchParams" + } + }, + "required": [ + "id", + "method", + "params" + ], + "title": "Fs/watchRequest", + "type": "object" + }, + { + "properties": { + "id": { + "$ref": "#/definitions/RequestId" + }, + "method": { + "enum": [ + "fs/unwatch" + ], + "title": "Fs/unwatchRequestMethod", + "type": "string" + }, + "params": { + "$ref": "#/definitions/FsUnwatchParams" + } + }, + "required": [ + "id", + "method", + "params" + ], + "title": "Fs/unwatchRequest", + "type": "object" + }, { "properties": { "id": { @@ -4037,6 +4085,29 @@ ], "type": "string" }, + "FsChangedNotification": { + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Filesystem watch notification emitted for `fs/watch` subscribers.", + "properties": { + "changedPaths": { + "description": "File or directory paths associated with this event.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "watchId": { + "description": "Watch identifier returned by `fs/watch`.", + "type": "string" + } + }, + "required": [ + "changedPaths", + "watchId" + ], + "title": "FsChangedNotification", + "type": "object" + }, "FsCopyParams": { "$schema": "http://json-schema.org/draft-07/schema#", "description": "Copy a file or directory tree on the host filesystem.", @@ -4291,6 +4362,70 @@ "title": "FsRemoveResponse", "type": "object" }, + "FsUnwatchParams": { + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Stop filesystem watch notifications for a prior `fs/watch`.", + "properties": { + "watchId": { + "description": "Watch identifier returned by `fs/watch`.", + "type": "string" + } + }, + "required": [ + "watchId" + ], + "title": "FsUnwatchParams", + "type": "object" + }, + "FsUnwatchResponse": { + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Successful response for `fs/unwatch`.", + "title": "FsUnwatchResponse", + "type": "object" + }, + "FsWatchParams": { + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Start filesystem watch notifications for an absolute path.", + "properties": { + "path": { + "allOf": [ + { + "$ref": "#/definitions/AbsolutePathBuf" + } + ], + "description": "Absolute file or directory path to watch." + } + }, + "required": [ + "path" + ], + "title": "FsWatchParams", + "type": "object" + }, + "FsWatchResponse": { + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Created watch handle returned by `fs/watch`.", + "properties": { + "path": { + "allOf": [ + { + "$ref": "#/definitions/AbsolutePathBuf" + } + ], + "description": "Canonicalized path associated with the watch." + }, + "watchId": { + "description": "Connection-scoped watch identifier used for `fs/unwatch` and `fs/changed`.", + "type": "string" + } + }, + "required": [ + "path", + "watchId" + ], + "title": "FsWatchResponse", + "type": "object" + }, "FsWriteFileParams": { "$schema": "http://json-schema.org/draft-07/schema#", "description": "Write a file on the host filesystem.", @@ -8540,6 +8675,26 @@ "title": "App/list/updatedNotification", "type": "object" }, + { + "properties": { + "method": { + "enum": [ + "fs/changed" + ], + "title": "Fs/changedNotificationMethod", + "type": "string" + }, + "params": { + "$ref": "#/definitions/FsChangedNotification" + } + }, + "required": [ + "method", + "params" + ], + "title": "Fs/changedNotification", + "type": "object" + }, { "properties": { "method": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/FsChangedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/FsChangedNotification.json new file mode 100644 index 0000000000..ab26588ecb --- /dev/null +++ b/codex-rs/app-server-protocol/schema/json/v2/FsChangedNotification.json @@ -0,0 +1,29 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "definitions": { + "AbsolutePathBuf": { + "description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.", + "type": "string" + } + }, + "description": "Filesystem watch notification emitted for `fs/watch` subscribers.", + "properties": { + "changedPaths": { + "description": "File or directory paths associated with this event.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "watchId": { + "description": "Watch identifier returned by `fs/watch`.", + "type": "string" + } + }, + "required": [ + "changedPaths", + "watchId" + ], + "title": "FsChangedNotification", + "type": "object" +} \ No newline at end of file diff --git a/codex-rs/app-server-protocol/schema/json/v2/FsUnwatchParams.json b/codex-rs/app-server-protocol/schema/json/v2/FsUnwatchParams.json new file mode 100644 index 0000000000..4b988d97aa --- /dev/null +++ b/codex-rs/app-server-protocol/schema/json/v2/FsUnwatchParams.json @@ -0,0 +1,15 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Stop filesystem watch notifications for a prior `fs/watch`.", + "properties": { + "watchId": { + "description": "Watch identifier returned by `fs/watch`.", + "type": "string" + } + }, + "required": [ + "watchId" + ], + "title": "FsUnwatchParams", + "type": "object" +} \ No newline at end of file diff --git a/codex-rs/app-server-protocol/schema/json/v2/FsUnwatchResponse.json b/codex-rs/app-server-protocol/schema/json/v2/FsUnwatchResponse.json new file mode 100644 index 0000000000..daa80ad656 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/json/v2/FsUnwatchResponse.json @@ -0,0 +1,6 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Successful response for `fs/unwatch`.", + "title": "FsUnwatchResponse", + "type": "object" +} \ No newline at end of file diff --git a/codex-rs/app-server-protocol/schema/json/v2/FsWatchParams.json b/codex-rs/app-server-protocol/schema/json/v2/FsWatchParams.json new file mode 100644 index 0000000000..cf80c7b101 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/json/v2/FsWatchParams.json @@ -0,0 +1,25 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "definitions": { + "AbsolutePathBuf": { + "description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.", + "type": "string" + } + }, + "description": "Start filesystem watch notifications for an absolute path.", + "properties": { + "path": { + "allOf": [ + { + "$ref": "#/definitions/AbsolutePathBuf" + } + ], + "description": "Absolute file or directory path to watch." + } + }, + "required": [ + "path" + ], + "title": "FsWatchParams", + "type": "object" +} \ No newline at end of file diff --git a/codex-rs/app-server-protocol/schema/json/v2/FsWatchResponse.json b/codex-rs/app-server-protocol/schema/json/v2/FsWatchResponse.json new file mode 100644 index 0000000000..b516636a09 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/json/v2/FsWatchResponse.json @@ -0,0 +1,30 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "definitions": { + "AbsolutePathBuf": { + "description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.", + "type": "string" + } + }, + "description": "Created watch handle returned by `fs/watch`.", + "properties": { + "path": { + "allOf": [ + { + "$ref": "#/definitions/AbsolutePathBuf" + } + ], + "description": "Canonicalized path associated with the watch." + }, + "watchId": { + "description": "Connection-scoped watch identifier used for `fs/unwatch` and `fs/changed`.", + "type": "string" + } + }, + "required": [ + "path", + "watchId" + ], + "title": "FsWatchResponse", + "type": "object" +} \ No newline at end of file diff --git a/codex-rs/app-server-protocol/schema/typescript/ClientRequest.ts b/codex-rs/app-server-protocol/schema/typescript/ClientRequest.ts index 5e03a26ca2..3c77652cec 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ClientRequest.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ClientRequest.ts @@ -26,6 +26,8 @@ import type { FsGetMetadataParams } from "./v2/FsGetMetadataParams"; import type { FsReadDirectoryParams } from "./v2/FsReadDirectoryParams"; import type { FsReadFileParams } from "./v2/FsReadFileParams"; import type { FsRemoveParams } from "./v2/FsRemoveParams"; +import type { FsUnwatchParams } from "./v2/FsUnwatchParams"; +import type { FsWatchParams } from "./v2/FsWatchParams"; import type { FsWriteFileParams } from "./v2/FsWriteFileParams"; import type { GetAccountParams } from "./v2/GetAccountParams"; import type { ListMcpServerStatusParams } from "./v2/ListMcpServerStatusParams"; @@ -61,4 +63,4 @@ import type { WindowsSandboxSetupStartParams } from "./v2/WindowsSandboxSetupSta /** * Request from the client to the server. */ -export type ClientRequest ={ "method": "initialize", id: RequestId, params: InitializeParams, } | { "method": "thread/start", id: RequestId, params: ThreadStartParams, } | { "method": "thread/resume", id: RequestId, params: ThreadResumeParams, } | { "method": "thread/fork", id: RequestId, params: ThreadForkParams, } | { "method": "thread/archive", id: RequestId, params: ThreadArchiveParams, } | { "method": "thread/unsubscribe", id: RequestId, params: ThreadUnsubscribeParams, } | { "method": "thread/name/set", id: RequestId, params: ThreadSetNameParams, } | { "method": "thread/metadata/update", id: RequestId, params: ThreadMetadataUpdateParams, } | { "method": "thread/unarchive", id: RequestId, params: ThreadUnarchiveParams, } | { "method": "thread/compact/start", id: RequestId, params: ThreadCompactStartParams, } | { "method": "thread/shellCommand", id: RequestId, params: ThreadShellCommandParams, } | { "method": "thread/rollback", id: RequestId, params: ThreadRollbackParams, } | { "method": "thread/list", id: RequestId, params: ThreadListParams, } | { "method": "thread/loaded/list", id: RequestId, params: ThreadLoadedListParams, } | { "method": "thread/read", id: RequestId, params: ThreadReadParams, } | { "method": "skills/list", id: RequestId, params: SkillsListParams, } | { "method": "plugin/list", id: RequestId, params: PluginListParams, } | { "method": "plugin/read", id: RequestId, params: PluginReadParams, } | { "method": "app/list", id: RequestId, params: AppsListParams, } | { "method": "fs/readFile", id: RequestId, params: FsReadFileParams, } | { "method": "fs/writeFile", id: RequestId, params: FsWriteFileParams, } | { "method": "fs/createDirectory", id: RequestId, params: FsCreateDirectoryParams, } | { "method": "fs/getMetadata", id: RequestId, params: FsGetMetadataParams, } | { "method": "fs/readDirectory", id: RequestId, params: FsReadDirectoryParams, } | { "method": "fs/remove", id: RequestId, params: FsRemoveParams, } | { "method": "fs/copy", id: RequestId, params: FsCopyParams, } | { "method": "skills/config/write", id: RequestId, params: SkillsConfigWriteParams, } | { "method": "plugin/install", id: RequestId, params: PluginInstallParams, } | { "method": "plugin/uninstall", id: RequestId, params: PluginUninstallParams, } | { "method": "turn/start", id: RequestId, params: TurnStartParams, } | { "method": "turn/steer", id: RequestId, params: TurnSteerParams, } | { "method": "turn/interrupt", id: RequestId, params: TurnInterruptParams, } | { "method": "review/start", id: RequestId, params: ReviewStartParams, } | { "method": "model/list", id: RequestId, params: ModelListParams, } | { "method": "experimentalFeature/list", id: RequestId, params: ExperimentalFeatureListParams, } | { "method": "mcpServer/oauth/login", id: RequestId, params: McpServerOauthLoginParams, } | { "method": "config/mcpServer/reload", id: RequestId, params: undefined, } | { "method": "mcpServerStatus/list", id: RequestId, params: ListMcpServerStatusParams, } | { "method": "windowsSandbox/setupStart", id: RequestId, params: WindowsSandboxSetupStartParams, } | { "method": "account/login/start", id: RequestId, params: LoginAccountParams, } | { "method": "account/login/cancel", id: RequestId, params: CancelLoginAccountParams, } | { "method": "account/logout", id: RequestId, params: undefined, } | { "method": "account/rateLimits/read", id: RequestId, params: undefined, } | { "method": "feedback/upload", id: RequestId, params: FeedbackUploadParams, } | { "method": "command/exec", id: RequestId, params: CommandExecParams, } | { "method": "command/exec/write", id: RequestId, params: CommandExecWriteParams, } | { "method": "command/exec/terminate", id: RequestId, params: CommandExecTerminateParams, } | { "method": "command/exec/resize", id: RequestId, params: CommandExecResizeParams, } | { "method": "config/read", id: RequestId, params: ConfigReadParams, } | { "method": "externalAgentConfig/detect", id: RequestId, params: ExternalAgentConfigDetectParams, } | { "method": "externalAgentConfig/import", id: RequestId, params: ExternalAgentConfigImportParams, } | { "method": "config/value/write", id: RequestId, params: ConfigValueWriteParams, } | { "method": "config/batchWrite", id: RequestId, params: ConfigBatchWriteParams, } | { "method": "configRequirements/read", id: RequestId, params: undefined, } | { "method": "account/read", id: RequestId, params: GetAccountParams, } | { "method": "getConversationSummary", id: RequestId, params: GetConversationSummaryParams, } | { "method": "gitDiffToRemote", id: RequestId, params: GitDiffToRemoteParams, } | { "method": "getAuthStatus", id: RequestId, params: GetAuthStatusParams, } | { "method": "fuzzyFileSearch", id: RequestId, params: FuzzyFileSearchParams, }; +export type ClientRequest ={ "method": "initialize", id: RequestId, params: InitializeParams, } | { "method": "thread/start", id: RequestId, params: ThreadStartParams, } | { "method": "thread/resume", id: RequestId, params: ThreadResumeParams, } | { "method": "thread/fork", id: RequestId, params: ThreadForkParams, } | { "method": "thread/archive", id: RequestId, params: ThreadArchiveParams, } | { "method": "thread/unsubscribe", id: RequestId, params: ThreadUnsubscribeParams, } | { "method": "thread/name/set", id: RequestId, params: ThreadSetNameParams, } | { "method": "thread/metadata/update", id: RequestId, params: ThreadMetadataUpdateParams, } | { "method": "thread/unarchive", id: RequestId, params: ThreadUnarchiveParams, } | { "method": "thread/compact/start", id: RequestId, params: ThreadCompactStartParams, } | { "method": "thread/shellCommand", id: RequestId, params: ThreadShellCommandParams, } | { "method": "thread/rollback", id: RequestId, params: ThreadRollbackParams, } | { "method": "thread/list", id: RequestId, params: ThreadListParams, } | { "method": "thread/loaded/list", id: RequestId, params: ThreadLoadedListParams, } | { "method": "thread/read", id: RequestId, params: ThreadReadParams, } | { "method": "skills/list", id: RequestId, params: SkillsListParams, } | { "method": "plugin/list", id: RequestId, params: PluginListParams, } | { "method": "plugin/read", id: RequestId, params: PluginReadParams, } | { "method": "app/list", id: RequestId, params: AppsListParams, } | { "method": "fs/readFile", id: RequestId, params: FsReadFileParams, } | { "method": "fs/writeFile", id: RequestId, params: FsWriteFileParams, } | { "method": "fs/createDirectory", id: RequestId, params: FsCreateDirectoryParams, } | { "method": "fs/getMetadata", id: RequestId, params: FsGetMetadataParams, } | { "method": "fs/readDirectory", id: RequestId, params: FsReadDirectoryParams, } | { "method": "fs/remove", id: RequestId, params: FsRemoveParams, } | { "method": "fs/copy", id: RequestId, params: FsCopyParams, } | { "method": "fs/watch", id: RequestId, params: FsWatchParams, } | { "method": "fs/unwatch", id: RequestId, params: FsUnwatchParams, } | { "method": "skills/config/write", id: RequestId, params: SkillsConfigWriteParams, } | { "method": "plugin/install", id: RequestId, params: PluginInstallParams, } | { "method": "plugin/uninstall", id: RequestId, params: PluginUninstallParams, } | { "method": "turn/start", id: RequestId, params: TurnStartParams, } | { "method": "turn/steer", id: RequestId, params: TurnSteerParams, } | { "method": "turn/interrupt", id: RequestId, params: TurnInterruptParams, } | { "method": "review/start", id: RequestId, params: ReviewStartParams, } | { "method": "model/list", id: RequestId, params: ModelListParams, } | { "method": "experimentalFeature/list", id: RequestId, params: ExperimentalFeatureListParams, } | { "method": "mcpServer/oauth/login", id: RequestId, params: McpServerOauthLoginParams, } | { "method": "config/mcpServer/reload", id: RequestId, params: undefined, } | { "method": "mcpServerStatus/list", id: RequestId, params: ListMcpServerStatusParams, } | { "method": "windowsSandbox/setupStart", id: RequestId, params: WindowsSandboxSetupStartParams, } | { "method": "account/login/start", id: RequestId, params: LoginAccountParams, } | { "method": "account/login/cancel", id: RequestId, params: CancelLoginAccountParams, } | { "method": "account/logout", id: RequestId, params: undefined, } | { "method": "account/rateLimits/read", id: RequestId, params: undefined, } | { "method": "feedback/upload", id: RequestId, params: FeedbackUploadParams, } | { "method": "command/exec", id: RequestId, params: CommandExecParams, } | { "method": "command/exec/write", id: RequestId, params: CommandExecWriteParams, } | { "method": "command/exec/terminate", id: RequestId, params: CommandExecTerminateParams, } | { "method": "command/exec/resize", id: RequestId, params: CommandExecResizeParams, } | { "method": "config/read", id: RequestId, params: ConfigReadParams, } | { "method": "externalAgentConfig/detect", id: RequestId, params: ExternalAgentConfigDetectParams, } | { "method": "externalAgentConfig/import", id: RequestId, params: ExternalAgentConfigImportParams, } | { "method": "config/value/write", id: RequestId, params: ConfigValueWriteParams, } | { "method": "config/batchWrite", id: RequestId, params: ConfigBatchWriteParams, } | { "method": "configRequirements/read", id: RequestId, params: undefined, } | { "method": "account/read", id: RequestId, params: GetAccountParams, } | { "method": "getConversationSummary", id: RequestId, params: GetConversationSummaryParams, } | { "method": "gitDiffToRemote", id: RequestId, params: GitDiffToRemoteParams, } | { "method": "getAuthStatus", id: RequestId, params: GetAuthStatusParams, } | { "method": "fuzzyFileSearch", id: RequestId, params: FuzzyFileSearchParams, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/ServerNotification.ts b/codex-rs/app-server-protocol/schema/typescript/ServerNotification.ts index d9e2df7797..85ebe847f4 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ServerNotification.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ServerNotification.ts @@ -15,6 +15,7 @@ import type { ContextCompactedNotification } from "./v2/ContextCompactedNotifica import type { DeprecationNoticeNotification } from "./v2/DeprecationNoticeNotification"; import type { ErrorNotification } from "./v2/ErrorNotification"; import type { FileChangeOutputDeltaNotification } from "./v2/FileChangeOutputDeltaNotification"; +import type { FsChangedNotification } from "./v2/FsChangedNotification"; import type { HookCompletedNotification } from "./v2/HookCompletedNotification"; import type { HookStartedNotification } from "./v2/HookStartedNotification"; import type { ItemCompletedNotification } from "./v2/ItemCompletedNotification"; @@ -56,4 +57,4 @@ import type { WindowsWorldWritableWarningNotification } from "./v2/WindowsWorldW /** * Notification sent from the server to the client. */ -export type ServerNotification = { "method": "error", "params": ErrorNotification } | { "method": "thread/started", "params": ThreadStartedNotification } | { "method": "thread/status/changed", "params": ThreadStatusChangedNotification } | { "method": "thread/archived", "params": ThreadArchivedNotification } | { "method": "thread/unarchived", "params": ThreadUnarchivedNotification } | { "method": "thread/closed", "params": ThreadClosedNotification } | { "method": "skills/changed", "params": SkillsChangedNotification } | { "method": "thread/name/updated", "params": ThreadNameUpdatedNotification } | { "method": "thread/tokenUsage/updated", "params": ThreadTokenUsageUpdatedNotification } | { "method": "turn/started", "params": TurnStartedNotification } | { "method": "hook/started", "params": HookStartedNotification } | { "method": "turn/completed", "params": TurnCompletedNotification } | { "method": "hook/completed", "params": HookCompletedNotification } | { "method": "turn/diff/updated", "params": TurnDiffUpdatedNotification } | { "method": "turn/plan/updated", "params": TurnPlanUpdatedNotification } | { "method": "item/started", "params": ItemStartedNotification } | { "method": "item/autoApprovalReview/started", "params": ItemGuardianApprovalReviewStartedNotification } | { "method": "item/autoApprovalReview/completed", "params": ItemGuardianApprovalReviewCompletedNotification } | { "method": "item/completed", "params": ItemCompletedNotification } | { "method": "rawResponseItem/completed", "params": RawResponseItemCompletedNotification } | { "method": "item/agentMessage/delta", "params": AgentMessageDeltaNotification } | { "method": "item/plan/delta", "params": PlanDeltaNotification } | { "method": "command/exec/outputDelta", "params": CommandExecOutputDeltaNotification } | { "method": "item/commandExecution/outputDelta", "params": CommandExecutionOutputDeltaNotification } | { "method": "item/commandExecution/terminalInteraction", "params": TerminalInteractionNotification } | { "method": "item/fileChange/outputDelta", "params": FileChangeOutputDeltaNotification } | { "method": "serverRequest/resolved", "params": ServerRequestResolvedNotification } | { "method": "item/mcpToolCall/progress", "params": McpToolCallProgressNotification } | { "method": "mcpServer/oauthLogin/completed", "params": McpServerOauthLoginCompletedNotification } | { "method": "mcpServer/startupStatus/updated", "params": McpServerStatusUpdatedNotification } | { "method": "account/updated", "params": AccountUpdatedNotification } | { "method": "account/rateLimits/updated", "params": AccountRateLimitsUpdatedNotification } | { "method": "app/list/updated", "params": AppListUpdatedNotification } | { "method": "item/reasoning/summaryTextDelta", "params": ReasoningSummaryTextDeltaNotification } | { "method": "item/reasoning/summaryPartAdded", "params": ReasoningSummaryPartAddedNotification } | { "method": "item/reasoning/textDelta", "params": ReasoningTextDeltaNotification } | { "method": "thread/compacted", "params": ContextCompactedNotification } | { "method": "model/rerouted", "params": ModelReroutedNotification } | { "method": "deprecationNotice", "params": DeprecationNoticeNotification } | { "method": "configWarning", "params": ConfigWarningNotification } | { "method": "fuzzyFileSearch/sessionUpdated", "params": FuzzyFileSearchSessionUpdatedNotification } | { "method": "fuzzyFileSearch/sessionCompleted", "params": FuzzyFileSearchSessionCompletedNotification } | { "method": "thread/realtime/started", "params": ThreadRealtimeStartedNotification } | { "method": "thread/realtime/itemAdded", "params": ThreadRealtimeItemAddedNotification } | { "method": "thread/realtime/transcriptUpdated", "params": ThreadRealtimeTranscriptUpdatedNotification } | { "method": "thread/realtime/outputAudio/delta", "params": ThreadRealtimeOutputAudioDeltaNotification } | { "method": "thread/realtime/error", "params": ThreadRealtimeErrorNotification } | { "method": "thread/realtime/closed", "params": ThreadRealtimeClosedNotification } | { "method": "windows/worldWritableWarning", "params": WindowsWorldWritableWarningNotification } | { "method": "windowsSandbox/setupCompleted", "params": WindowsSandboxSetupCompletedNotification } | { "method": "account/login/completed", "params": AccountLoginCompletedNotification }; +export type ServerNotification = { "method": "error", "params": ErrorNotification } | { "method": "thread/started", "params": ThreadStartedNotification } | { "method": "thread/status/changed", "params": ThreadStatusChangedNotification } | { "method": "thread/archived", "params": ThreadArchivedNotification } | { "method": "thread/unarchived", "params": ThreadUnarchivedNotification } | { "method": "thread/closed", "params": ThreadClosedNotification } | { "method": "skills/changed", "params": SkillsChangedNotification } | { "method": "thread/name/updated", "params": ThreadNameUpdatedNotification } | { "method": "thread/tokenUsage/updated", "params": ThreadTokenUsageUpdatedNotification } | { "method": "turn/started", "params": TurnStartedNotification } | { "method": "hook/started", "params": HookStartedNotification } | { "method": "turn/completed", "params": TurnCompletedNotification } | { "method": "hook/completed", "params": HookCompletedNotification } | { "method": "turn/diff/updated", "params": TurnDiffUpdatedNotification } | { "method": "turn/plan/updated", "params": TurnPlanUpdatedNotification } | { "method": "item/started", "params": ItemStartedNotification } | { "method": "item/autoApprovalReview/started", "params": ItemGuardianApprovalReviewStartedNotification } | { "method": "item/autoApprovalReview/completed", "params": ItemGuardianApprovalReviewCompletedNotification } | { "method": "item/completed", "params": ItemCompletedNotification } | { "method": "rawResponseItem/completed", "params": RawResponseItemCompletedNotification } | { "method": "item/agentMessage/delta", "params": AgentMessageDeltaNotification } | { "method": "item/plan/delta", "params": PlanDeltaNotification } | { "method": "command/exec/outputDelta", "params": CommandExecOutputDeltaNotification } | { "method": "item/commandExecution/outputDelta", "params": CommandExecutionOutputDeltaNotification } | { "method": "item/commandExecution/terminalInteraction", "params": TerminalInteractionNotification } | { "method": "item/fileChange/outputDelta", "params": FileChangeOutputDeltaNotification } | { "method": "serverRequest/resolved", "params": ServerRequestResolvedNotification } | { "method": "item/mcpToolCall/progress", "params": McpToolCallProgressNotification } | { "method": "mcpServer/oauthLogin/completed", "params": McpServerOauthLoginCompletedNotification } | { "method": "mcpServer/startupStatus/updated", "params": McpServerStatusUpdatedNotification } | { "method": "account/updated", "params": AccountUpdatedNotification } | { "method": "account/rateLimits/updated", "params": AccountRateLimitsUpdatedNotification } | { "method": "app/list/updated", "params": AppListUpdatedNotification } | { "method": "fs/changed", "params": FsChangedNotification } | { "method": "item/reasoning/summaryTextDelta", "params": ReasoningSummaryTextDeltaNotification } | { "method": "item/reasoning/summaryPartAdded", "params": ReasoningSummaryPartAddedNotification } | { "method": "item/reasoning/textDelta", "params": ReasoningTextDeltaNotification } | { "method": "thread/compacted", "params": ContextCompactedNotification } | { "method": "model/rerouted", "params": ModelReroutedNotification } | { "method": "deprecationNotice", "params": DeprecationNoticeNotification } | { "method": "configWarning", "params": ConfigWarningNotification } | { "method": "fuzzyFileSearch/sessionUpdated", "params": FuzzyFileSearchSessionUpdatedNotification } | { "method": "fuzzyFileSearch/sessionCompleted", "params": FuzzyFileSearchSessionCompletedNotification } | { "method": "thread/realtime/started", "params": ThreadRealtimeStartedNotification } | { "method": "thread/realtime/itemAdded", "params": ThreadRealtimeItemAddedNotification } | { "method": "thread/realtime/transcriptUpdated", "params": ThreadRealtimeTranscriptUpdatedNotification } | { "method": "thread/realtime/outputAudio/delta", "params": ThreadRealtimeOutputAudioDeltaNotification } | { "method": "thread/realtime/error", "params": ThreadRealtimeErrorNotification } | { "method": "thread/realtime/closed", "params": ThreadRealtimeClosedNotification } | { "method": "windows/worldWritableWarning", "params": WindowsWorldWritableWarningNotification } | { "method": "windowsSandbox/setupCompleted", "params": WindowsSandboxSetupCompletedNotification } | { "method": "account/login/completed", "params": AccountLoginCompletedNotification }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/FsChangedNotification.ts b/codex-rs/app-server-protocol/schema/typescript/v2/FsChangedNotification.ts new file mode 100644 index 0000000000..2e9bd0d6ec --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/FsChangedNotification.ts @@ -0,0 +1,17 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { AbsolutePathBuf } from "../AbsolutePathBuf"; + +/** + * Filesystem watch notification emitted for `fs/watch` subscribers. + */ +export type FsChangedNotification = { +/** + * Watch identifier returned by `fs/watch`. + */ +watchId: string, +/** + * File or directory paths associated with this event. + */ +changedPaths: Array, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/FsUnwatchParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/FsUnwatchParams.ts new file mode 100644 index 0000000000..b21befdb5c --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/FsUnwatchParams.ts @@ -0,0 +1,12 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +/** + * Stop filesystem watch notifications for a prior `fs/watch`. + */ +export type FsUnwatchParams = { +/** + * Watch identifier returned by `fs/watch`. + */ +watchId: string, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/FsUnwatchResponse.ts b/codex-rs/app-server-protocol/schema/typescript/v2/FsUnwatchResponse.ts new file mode 100644 index 0000000000..02507d2c00 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/FsUnwatchResponse.ts @@ -0,0 +1,8 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +/** + * Successful response for `fs/unwatch`. + */ +export type FsUnwatchResponse = Record; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/FsWatchParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/FsWatchParams.ts new file mode 100644 index 0000000000..d6d956b288 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/FsWatchParams.ts @@ -0,0 +1,13 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { AbsolutePathBuf } from "../AbsolutePathBuf"; + +/** + * Start filesystem watch notifications for an absolute path. + */ +export type FsWatchParams = { +/** + * Absolute file or directory path to watch. + */ +path: AbsolutePathBuf, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/FsWatchResponse.ts b/codex-rs/app-server-protocol/schema/typescript/v2/FsWatchResponse.ts new file mode 100644 index 0000000000..1927272821 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/FsWatchResponse.ts @@ -0,0 +1,17 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { AbsolutePathBuf } from "../AbsolutePathBuf"; + +/** + * Created watch handle returned by `fs/watch`. + */ +export type FsWatchResponse = { +/** + * Connection-scoped watch identifier used for `fs/unwatch` and `fs/changed`. + */ +watchId: string, +/** + * Canonicalized path associated with the watch. + */ +path: AbsolutePathBuf, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts index 1178f7c2ee..2ee447bba2 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -97,6 +97,7 @@ export type { FileChangeOutputDeltaNotification } from "./FileChangeOutputDeltaN export type { FileChangeRequestApprovalParams } from "./FileChangeRequestApprovalParams"; export type { FileChangeRequestApprovalResponse } from "./FileChangeRequestApprovalResponse"; export type { FileUpdateChange } from "./FileUpdateChange"; +export type { FsChangedNotification } from "./FsChangedNotification"; export type { FsCopyParams } from "./FsCopyParams"; export type { FsCopyResponse } from "./FsCopyResponse"; export type { FsCreateDirectoryParams } from "./FsCreateDirectoryParams"; @@ -110,6 +111,10 @@ export type { FsReadFileParams } from "./FsReadFileParams"; export type { FsReadFileResponse } from "./FsReadFileResponse"; export type { FsRemoveParams } from "./FsRemoveParams"; export type { FsRemoveResponse } from "./FsRemoveResponse"; +export type { FsUnwatchParams } from "./FsUnwatchParams"; +export type { FsUnwatchResponse } from "./FsUnwatchResponse"; +export type { FsWatchParams } from "./FsWatchParams"; +export type { FsWatchResponse } from "./FsWatchResponse"; export type { FsWriteFileParams } from "./FsWriteFileParams"; export type { FsWriteFileResponse } from "./FsWriteFileResponse"; export type { GetAccountParams } from "./GetAccountParams"; diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index d4a8c70dd5..80cb24f0ff 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -326,6 +326,14 @@ client_request_definitions! { params: v2::FsCopyParams, response: v2::FsCopyResponse, }, + FsWatch => "fs/watch" { + params: v2::FsWatchParams, + response: v2::FsWatchResponse, + }, + FsUnwatch => "fs/unwatch" { + params: v2::FsUnwatchParams, + response: v2::FsUnwatchResponse, + }, SkillsConfigWrite => "skills/config/write" { params: v2::SkillsConfigWriteParams, response: v2::SkillsConfigWriteResponse, @@ -899,6 +907,7 @@ server_notification_definitions! { AccountUpdated => "account/updated" (v2::AccountUpdatedNotification), AccountRateLimitsUpdated => "account/rateLimits/updated" (v2::AccountRateLimitsUpdatedNotification), AppListUpdated => "app/list/updated" (v2::AppListUpdatedNotification), + FsChanged => "fs/changed" (v2::FsChangedNotification), ReasoningSummaryTextDelta => "item/reasoning/summaryTextDelta" (v2::ReasoningSummaryTextDeltaNotification), ReasoningSummaryPartAdded => "item/reasoning/summaryPartAdded" (v2::ReasoningSummaryPartAddedNotification), ReasoningTextDelta => "item/reasoning/textDelta" (v2::ReasoningTextDeltaNotification), @@ -1478,6 +1487,27 @@ mod tests { Ok(()) } + #[test] + fn serialize_fs_watch() -> Result<()> { + let request = ClientRequest::FsWatch { + request_id: RequestId::Integer(10), + params: v2::FsWatchParams { + path: absolute_path("tmp/repo/.git"), + }, + }; + assert_eq!( + json!({ + "method": "fs/watch", + "id": 10, + "params": { + "path": absolute_path_string("tmp/repo/.git") + } + }), + serde_json::to_value(&request)?, + ); + Ok(()) + } + #[test] fn serialize_list_experimental_features() -> Result<()> { let request = ClientRequest::ExperimentalFeatureList { diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 09647e05cc..fab3fe3190 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -2301,6 +2301,52 @@ pub struct FsCopyParams { #[ts(export_to = "v2/")] pub struct FsCopyResponse {} +/// Start filesystem watch notifications for an absolute path. +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct FsWatchParams { + /// Absolute file or directory path to watch. + pub path: AbsolutePathBuf, +} + +/// Created watch handle returned by `fs/watch`. +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct FsWatchResponse { + /// Connection-scoped watch identifier used for `fs/unwatch` and `fs/changed`. + pub watch_id: String, + /// Canonicalized path associated with the watch. + pub path: AbsolutePathBuf, +} + +/// Stop filesystem watch notifications for a prior `fs/watch`. +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct FsUnwatchParams { + /// Watch identifier returned by `fs/watch`. + pub watch_id: String, +} + +/// Successful response for `fs/unwatch`. +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct FsUnwatchResponse {} + +/// Filesystem watch notification emitted for `fs/watch` subscribers. +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct FsChangedNotification { + /// Watch identifier returned by `fs/watch`. + pub watch_id: String, + /// File or directory paths associated with this event. + pub changed_paths: Vec, +} + /// PTY size in character cells for `command/exec` PTY sessions. #[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] @@ -6497,6 +6543,33 @@ mod tests { assert_eq!(decoded, response); } + #[test] + fn fs_changed_notification_round_trips() { + let notification = FsChangedNotification { + watch_id: "0195ec6b-1d6f-7c2e-8c7a-56f2c4a8b9d1".to_string(), + changed_paths: vec![ + absolute_path("tmp/repo/.git/HEAD"), + absolute_path("tmp/repo/.git/FETCH_HEAD"), + ], + }; + + let value = serde_json::to_value(¬ification).expect("serialize fs/changed notification"); + assert_eq!( + value, + json!({ + "watchId": "0195ec6b-1d6f-7c2e-8c7a-56f2c4a8b9d1", + "changedPaths": [ + absolute_path_string("tmp/repo/.git/HEAD"), + absolute_path_string("tmp/repo/.git/FETCH_HEAD"), + ], + }) + ); + + let decoded = serde_json::from_value::(value) + .expect("deserialize fs/changed notification"); + assert_eq!(decoded, notification); + } + #[test] fn command_exec_params_default_optional_streaming_flags() { let params = serde_json::from_value::(json!({ diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index c238cc73e3..76312e81ea 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -159,6 +159,9 @@ Example with notification opt-out: - `fs/readDirectory` — list direct child entries for an absolute directory path; each entry contains `fileName`, `isDirectory`, and `isFile`, and `fileName` is just the child name, not a path. - `fs/remove` — remove an absolute file or directory tree; `recursive` and `force` default to `true`. - `fs/copy` — copy between absolute paths; directory copies require `recursive: true`. +- `fs/watch` — subscribe this connection to filesystem change notifications for an absolute file or directory path; returns a `watchId` and canonicalized `path`. +- `fs/unwatch` — stop sending notifications for a prior `fs/watch`; returns `{}`. +- `fs/changed` — notification emitted when watched paths change, including the `watchId` and `changedPaths`. - `model/list` — list available models (set `includeHidden: true` to include entries with `hidden: true`), with reasoning effort options, optional legacy `upgrade` model ids, optional `upgradeInfo` metadata (`model`, `upgradeCopy`, `modelLink`, `migrationMarkdown`), and optional `availabilityNux` metadata. - `experimentalFeature/list` — list feature flags with stage metadata (`beta`, `underDevelopment`, `stable`, etc.), enabled/default-enabled state, and cursor pagination. For non-beta flags, `displayName`/`description`/`announcement` are `null`. - `collaborationMode/list` — list available collaboration mode presets (experimental, no pagination). This response omits built-in developer instructions; clients should either pass `settings.developer_instructions: null` when setting a mode to use Codex's built-in instructions, or provide their own instructions explicitly. @@ -795,6 +798,28 @@ All filesystem paths in this section must be absolute. - `fs/readFile` always returns base64 bytes via `dataBase64`, and `fs/writeFile` always expects base64 bytes in `dataBase64`. - `fs/copy` handles both file copies and directory-tree copies; it requires `recursive: true` when `sourcePath` is a directory. Recursive copies traverse regular files, directories, and symlinks; other entry types are skipped. +### Example: Filesystem watch + +`fs/watch` accepts absolute file or directory paths. Watching a file emits `fs/changed` for that file path, including updates delivered via replace or rename operations. + +```json +{ "method": "fs/watch", "id": 44, "params": { + "path": "/Users/me/project/.git/HEAD" +} } +{ "id": 44, "result": { + "watchId": "0195ec6b-1d6f-7c2e-8c7a-56f2c4a8b9d1", + "path": "/Users/me/project/.git/HEAD" +} } +{ "method": "fs/changed", "params": { + "watchId": "0195ec6b-1d6f-7c2e-8c7a-56f2c4a8b9d1", + "changedPaths": ["/Users/me/project/.git/HEAD"] +} } +{ "method": "fs/unwatch", "id": 45, "params": { + "watchId": "0195ec6b-1d6f-7c2e-8c7a-56f2c4a8b9d1" +} } +{ "id": 45, "result": {} } +``` + ## Events Event notifications are the server-initiated event stream for thread lifecycles, turn lifecycles, and the items within them. After you start or resume a thread, keep reading stdout for `thread/started`, `thread/archived`, `thread/unarchived`, `thread/closed`, `turn/*`, and `item/*` notifications. diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 32eb28c12b..1d58e7317d 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -893,7 +893,9 @@ impl CodexMessageProcessor { | ClientRequest::FsGetMetadata { .. } | ClientRequest::FsReadDirectory { .. } | ClientRequest::FsRemove { .. } - | ClientRequest::FsCopy { .. } => { + | ClientRequest::FsCopy { .. } + | ClientRequest::FsWatch { .. } + | ClientRequest::FsUnwatch { .. } => { warn!("Filesystem request reached CodexMessageProcessor unexpectedly"); } ClientRequest::ConfigRequirementsRead { .. } => { diff --git a/codex-rs/app-server/src/fs_api.rs b/codex-rs/app-server/src/fs_api.rs index 1f8a32362f..57b355f818 100644 --- a/codex-rs/app-server/src/fs_api.rs +++ b/codex-rs/app-server/src/fs_api.rs @@ -159,7 +159,7 @@ impl FsApi { } } -fn invalid_request(message: impl Into) -> JSONRPCErrorError { +pub(crate) fn invalid_request(message: impl Into) -> JSONRPCErrorError { JSONRPCErrorError { code: INVALID_REQUEST_ERROR_CODE, message: message.into(), @@ -167,7 +167,7 @@ fn invalid_request(message: impl Into) -> JSONRPCErrorError { } } -fn map_fs_error(err: io::Error) -> JSONRPCErrorError { +pub(crate) fn map_fs_error(err: io::Error) -> JSONRPCErrorError { if err.kind() == io::ErrorKind::InvalidInput { invalid_request(err.to_string()) } else { diff --git a/codex-rs/app-server/src/fs_watch.rs b/codex-rs/app-server/src/fs_watch.rs new file mode 100644 index 0000000000..309bee4a64 --- /dev/null +++ b/codex-rs/app-server/src/fs_watch.rs @@ -0,0 +1,379 @@ +use crate::outgoing_message::ConnectionId; +use crate::outgoing_message::OutgoingMessageSender; +use codex_app_server_protocol::FsChangedNotification; +use codex_app_server_protocol::FsUnwatchParams; +use codex_app_server_protocol::FsUnwatchResponse; +use codex_app_server_protocol::FsWatchParams; +use codex_app_server_protocol::FsWatchResponse; +use codex_app_server_protocol::JSONRPCErrorError; +use codex_app_server_protocol::ServerNotification; +use codex_core::file_watcher::FileWatcher; +use codex_core::file_watcher::FileWatcherEvent; +use codex_core::file_watcher::FileWatcherSubscriber; +use codex_core::file_watcher::Receiver; +use codex_core::file_watcher::WatchPath; +use codex_core::file_watcher::WatchRegistration; +use codex_utils_absolute_path::AbsolutePathBuf; +use std::collections::HashMap; +use std::collections::HashSet; +use std::hash::Hash; +use std::path::PathBuf; +use std::sync::Arc; +use std::time::Duration; +use tokio::sync::Mutex as AsyncMutex; +#[cfg(test)] +use tokio::sync::mpsc; +use tokio::sync::oneshot; +use tokio::time::Instant; +use tracing::warn; +use uuid::Uuid; + +const FS_CHANGED_NOTIFICATION_DEBOUNCE: Duration = Duration::from_millis(200); + +struct DebouncedReceiver { + rx: Receiver, + interval: Duration, + changed_paths: HashSet, + next_allowance: Option, +} + +impl DebouncedReceiver { + fn new(rx: Receiver, interval: Duration) -> Self { + Self { + rx, + interval, + changed_paths: HashSet::new(), + next_allowance: None, + } + } + + async fn recv(&mut self) -> Option { + while self.changed_paths.is_empty() { + self.changed_paths.extend(self.rx.recv().await?.paths); + } + let next_allowance = *self + .next_allowance + .get_or_insert_with(|| Instant::now() + self.interval); + + loop { + tokio::select! { + event = self.rx.recv() => self.changed_paths.extend(event?.paths), + _ = tokio::time::sleep_until(next_allowance) => break, + } + } + + Some(FileWatcherEvent { + paths: self.changed_paths.drain().collect(), + }) + } +} + +#[derive(Clone)] +pub(crate) struct FsWatchManager { + outgoing: Arc, + file_watcher: Arc, + state: Arc>, +} + +#[derive(Default)] +struct FsWatchState { + entries: HashMap, +} + +struct WatchEntry { + terminate_tx: oneshot::Sender>, + _subscriber: FileWatcherSubscriber, + _registration: WatchRegistration, +} + +#[derive(Clone, Debug, Eq, Hash, PartialEq)] +struct WatchKey { + connection_id: ConnectionId, + watch_id: String, +} + +impl FsWatchManager { + pub(crate) fn new(outgoing: Arc) -> Self { + let file_watcher = match FileWatcher::new() { + Ok(file_watcher) => Arc::new(file_watcher), + Err(err) => { + warn!("filesystem watch manager falling back to noop core watcher: {err}"); + Arc::new(FileWatcher::noop()) + } + }; + Self::new_with_file_watcher(outgoing, file_watcher) + } + + fn new_with_file_watcher( + outgoing: Arc, + file_watcher: Arc, + ) -> Self { + Self { + outgoing, + file_watcher, + state: Arc::new(AsyncMutex::new(FsWatchState::default())), + } + } + + pub(crate) async fn watch( + &self, + connection_id: ConnectionId, + params: FsWatchParams, + ) -> Result { + let watch_id = Uuid::now_v7().to_string(); + let outgoing = self.outgoing.clone(); + let (subscriber, rx) = self.file_watcher.add_subscriber(); + let watch_root = params.path.to_path_buf().clone(); + let registration = subscriber.register_paths(vec![WatchPath { + path: params.path.to_path_buf(), + recursive: false, + }]); + let (terminate_tx, terminate_rx) = oneshot::channel(); + + self.state.lock().await.entries.insert( + WatchKey { + connection_id, + watch_id: watch_id.clone(), + }, + WatchEntry { + terminate_tx, + _subscriber: subscriber, + _registration: registration, + }, + ); + + let task_watch_id = watch_id.clone(); + tokio::spawn(async move { + let mut rx = DebouncedReceiver::new(rx, FS_CHANGED_NOTIFICATION_DEBOUNCE); + tokio::pin!(terminate_rx); + loop { + let event = tokio::select! { + biased; + _ = &mut terminate_rx => break, + event = rx.recv() => match event { + Some(event) => event, + None => break, + }, + }; + let mut changed_paths = event + .paths + .into_iter() + .filter_map(|path| { + match AbsolutePathBuf::resolve_path_against_base(&path, &watch_root) { + Ok(path) => Some(path), + Err(err) => { + warn!( + "failed to normalize watch event path ({}) for {}: {err}", + path.display(), + watch_root.display() + ); + None + } + } + }) + .collect::>(); + changed_paths.sort_by(|left, right| left.as_path().cmp(right.as_path())); + if !changed_paths.is_empty() { + outgoing + .send_server_notification_to_connection_and_wait( + connection_id, + ServerNotification::FsChanged(FsChangedNotification { + watch_id: task_watch_id.clone(), + changed_paths, + }), + ) + .await; + } + } + }); + + Ok(FsWatchResponse { + watch_id, + path: params.path, + }) + } + + pub(crate) async fn unwatch( + &self, + connection_id: ConnectionId, + params: FsUnwatchParams, + ) -> Result { + let watch_key = WatchKey { + connection_id, + watch_id: params.watch_id, + }; + let entry = self.state.lock().await.entries.remove(&watch_key); + if let Some(entry) = entry { + // Wait for the oneshot to be destroyed by the task to ensure that no notifications + // are send after the unwatch response. + let (done_tx, done_rx) = oneshot::channel(); + let _ = entry.terminate_tx.send(done_tx); + let _ = done_rx.await; + } + Ok(FsUnwatchResponse {}) + } + + pub(crate) async fn connection_closed(&self, connection_id: ConnectionId) { + let mut state = self.state.lock().await; + state + .entries + .extract_if(|key, _| key.connection_id == connection_id) + .count(); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use codex_utils_absolute_path::AbsolutePathBuf; + use pretty_assertions::assert_eq; + use tempfile::TempDir; + use uuid::Version; + + fn absolute_path(path: PathBuf) -> AbsolutePathBuf { + assert!( + path.is_absolute(), + "path must be absolute: {}", + path.display() + ); + AbsolutePathBuf::try_from(path).expect("path should be absolute") + } + + fn manager_with_noop_watcher() -> FsWatchManager { + const OUTGOING_BUFFER: usize = 1; + let (tx, _rx) = mpsc::channel(OUTGOING_BUFFER); + FsWatchManager::new_with_file_watcher( + Arc::new(OutgoingMessageSender::new(tx)), + Arc::new(FileWatcher::noop()), + ) + } + + #[tokio::test] + async fn watch_returns_a_v7_id_and_tracks_the_owner_scoped_entry() { + let temp_dir = TempDir::new().expect("temp dir"); + let head_path = temp_dir.path().join("HEAD"); + std::fs::write(&head_path, "ref: refs/heads/main\n").expect("write HEAD"); + + let manager = manager_with_noop_watcher(); + let path = absolute_path(head_path); + let response = manager + .watch(ConnectionId(1), FsWatchParams { path: path.clone() }) + .await + .expect("watch should succeed"); + + assert_eq!(response.path, path); + let watch_id = Uuid::parse_str(&response.watch_id).expect("watch id should be a UUID"); + assert_eq!(watch_id.get_version(), Some(Version::SortRand)); + + let state = manager.state.lock().await; + assert_eq!( + state.entries.keys().cloned().collect::>(), + HashSet::from([WatchKey { + connection_id: ConnectionId(1), + watch_id: response.watch_id, + }]) + ); + } + + #[tokio::test] + async fn unwatch_is_scoped_to_the_connection_that_created_the_watch() { + let temp_dir = TempDir::new().expect("temp dir"); + let head_path = temp_dir.path().join("HEAD"); + std::fs::write(&head_path, "ref: refs/heads/main\n").expect("write HEAD"); + + let manager = manager_with_noop_watcher(); + let response = manager + .watch( + ConnectionId(1), + FsWatchParams { + path: absolute_path(head_path), + }, + ) + .await + .expect("watch should succeed"); + let watch_key = WatchKey { + connection_id: ConnectionId(1), + watch_id: response.watch_id.clone(), + }; + + manager + .unwatch( + ConnectionId(2), + FsUnwatchParams { + watch_id: response.watch_id.clone(), + }, + ) + .await + .expect("foreign unwatch should be a no-op"); + assert!(manager.state.lock().await.entries.contains_key(&watch_key)); + + manager + .unwatch( + ConnectionId(1), + FsUnwatchParams { + watch_id: response.watch_id, + }, + ) + .await + .expect("owner unwatch should succeed"); + assert!(!manager.state.lock().await.entries.contains_key(&watch_key)); + } + + #[tokio::test] + async fn connection_closed_removes_only_that_connections_watches() { + let temp_dir = TempDir::new().expect("temp dir"); + let head_path = temp_dir.path().join("HEAD"); + let fetch_head_path = temp_dir.path().join("FETCH_HEAD"); + let packed_refs_path = temp_dir.path().join("packed-refs"); + std::fs::write(&head_path, "ref: refs/heads/main\n").expect("write HEAD"); + std::fs::write(&fetch_head_path, "old-fetch\n").expect("write FETCH_HEAD"); + std::fs::write(&packed_refs_path, "refs\n").expect("write packed-refs"); + + let manager = manager_with_noop_watcher(); + let response_1 = manager + .watch( + ConnectionId(1), + FsWatchParams { + path: absolute_path(head_path), + }, + ) + .await + .expect("first watch should succeed"); + let response_2 = manager + .watch( + ConnectionId(1), + FsWatchParams { + path: absolute_path(fetch_head_path), + }, + ) + .await + .expect("second watch should succeed"); + let response_3 = manager + .watch( + ConnectionId(2), + FsWatchParams { + path: absolute_path(packed_refs_path), + }, + ) + .await + .expect("third watch should succeed"); + + manager.connection_closed(ConnectionId(1)).await; + + assert_eq!( + manager + .state + .lock() + .await + .entries + .keys() + .cloned() + .collect::>(), + HashSet::from([WatchKey { + connection_id: ConnectionId(2), + watch_id: response_3.watch_id, + }]) + ); + assert_ne!(response_1.watch_id, response_2.watch_id); + } +} diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 63f72d5224..dc2512aaec 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -67,6 +67,7 @@ mod error_code; mod external_agent_config_api; mod filters; mod fs_api; +mod fs_watch; mod fuzzy_file_search; pub mod in_process; mod message_processor; diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 96fe437c05..ad7f5ca626 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -11,6 +11,7 @@ use crate::config_api::ConfigApi; use crate::error_code::INVALID_REQUEST_ERROR_CODE; use crate::external_agent_config_api::ExternalAgentConfigApi; use crate::fs_api::FsApi; +use crate::fs_watch::FsWatchManager; use crate::outgoing_message::ConnectionId; use crate::outgoing_message::ConnectionRequestId; use crate::outgoing_message::OutgoingMessageSender; @@ -36,6 +37,8 @@ use codex_app_server_protocol::FsGetMetadataParams; use codex_app_server_protocol::FsReadDirectoryParams; use codex_app_server_protocol::FsReadFileParams; use codex_app_server_protocol::FsRemoveParams; +use codex_app_server_protocol::FsUnwatchParams; +use codex_app_server_protocol::FsWatchParams; use codex_app_server_protocol::FsWriteFileParams; use codex_app_server_protocol::InitializeResponse; use codex_app_server_protocol::JSONRPCError; @@ -152,6 +155,7 @@ pub(crate) struct MessageProcessor { external_agent_config_api: ExternalAgentConfigApi, fs_api: FsApi, auth_manager: Arc, + fs_watch_manager: FsWatchManager, config: Arc, config_warnings: Arc>, } @@ -248,6 +252,7 @@ impl MessageProcessor { ); let external_agent_config_api = ExternalAgentConfigApi::new(config.codex_home.clone()); let fs_api = FsApi::default(); + let fs_watch_manager = FsWatchManager::new(outgoing.clone()); Self { outgoing, @@ -256,6 +261,7 @@ impl MessageProcessor { external_agent_config_api, fs_api, auth_manager, + fs_watch_manager, config, config_warnings: Arc::new(config_warnings), } @@ -467,6 +473,7 @@ impl MessageProcessor { pub(crate) async fn connection_closed(&mut self, connection_id: ConnectionId) { self.outgoing.connection_closed(connection_id).await; + self.fs_watch_manager.connection_closed(connection_id).await; self.codex_message_processor .connection_closed(connection_id) .await; @@ -759,6 +766,28 @@ impl MessageProcessor { ) .await; } + ClientRequest::FsWatch { request_id, params } => { + self.handle_fs_watch( + ConnectionRequestId { + connection_id, + request_id, + }, + connection_id, + params, + ) + .await; + } + ClientRequest::FsUnwatch { request_id, params } => { + self.handle_fs_unwatch( + ConnectionRequestId { + connection_id, + request_id, + }, + connection_id, + params, + ) + .await; + } other => { // Box the delegated future so this wrapper's async state machine does not // inline the full `CodexMessageProcessor::process_request` future, which @@ -910,6 +939,30 @@ impl MessageProcessor { Err(error) => self.outgoing.send_error(request_id, error).await, } } + + async fn handle_fs_watch( + &self, + request_id: ConnectionRequestId, + connection_id: ConnectionId, + params: FsWatchParams, + ) { + match self.fs_watch_manager.watch(connection_id, params).await { + Ok(response) => self.outgoing.send_response(request_id, response).await, + Err(error) => self.outgoing.send_error(request_id, error).await, + } + } + + async fn handle_fs_unwatch( + &self, + request_id: ConnectionRequestId, + connection_id: ConnectionId, + params: FsUnwatchParams, + ) { + match self.fs_watch_manager.unwatch(connection_id, params).await { + Ok(response) => self.outgoing.send_response(request_id, response).await, + Err(error) => self.outgoing.send_error(request_id, error).await, + } + } } #[cfg(test)] diff --git a/codex-rs/app-server/tests/common/mcp_process.rs b/codex-rs/app-server/tests/common/mcp_process.rs index 1a132ccee1..067e06ec43 100644 --- a/codex-rs/app-server/tests/common/mcp_process.rs +++ b/codex-rs/app-server/tests/common/mcp_process.rs @@ -31,6 +31,8 @@ use codex_app_server_protocol::FsGetMetadataParams; use codex_app_server_protocol::FsReadDirectoryParams; use codex_app_server_protocol::FsReadFileParams; use codex_app_server_protocol::FsRemoveParams; +use codex_app_server_protocol::FsUnwatchParams; +use codex_app_server_protocol::FsWatchParams; use codex_app_server_protocol::FsWriteFileParams; use codex_app_server_protocol::GetAccountParams; use codex_app_server_protocol::GetAuthStatusParams; @@ -790,6 +792,19 @@ impl McpProcess { self.send_request("fs/copy", params).await } + pub async fn send_fs_watch_request(&mut self, params: FsWatchParams) -> anyhow::Result { + let params = Some(serde_json::to_value(params)?); + self.send_request("fs/watch", params).await + } + + pub async fn send_fs_unwatch_request( + &mut self, + params: FsUnwatchParams, + ) -> anyhow::Result { + let params = Some(serde_json::to_value(params)?); + self.send_request("fs/unwatch", params).await + } + /// Send an `account/logout` JSON-RPC request. pub async fn send_logout_account_request(&mut self) -> anyhow::Result { self.send_request("account/logout", /*params*/ None).await diff --git a/codex-rs/app-server/tests/suite/v2/fs.rs b/codex-rs/app-server/tests/suite/v2/fs.rs index bc8ae20ec1..e113904809 100644 --- a/codex-rs/app-server/tests/suite/v2/fs.rs +++ b/codex-rs/app-server/tests/suite/v2/fs.rs @@ -4,11 +4,15 @@ use app_test_support::McpProcess; use app_test_support::to_response; use base64::Engine; use base64::engine::general_purpose::STANDARD; +use codex_app_server_protocol::FsChangedNotification; use codex_app_server_protocol::FsCopyParams; use codex_app_server_protocol::FsGetMetadataResponse; use codex_app_server_protocol::FsReadDirectoryEntry; use codex_app_server_protocol::FsReadFileResponse; +use codex_app_server_protocol::FsUnwatchParams; +use codex_app_server_protocol::FsWatchResponse; use codex_app_server_protocol::FsWriteFileParams; +use codex_app_server_protocol::JSONRPCNotification; use codex_app_server_protocol::RequestId; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; @@ -17,6 +21,8 @@ use std::path::PathBuf; use tempfile::TempDir; use tokio::time::Duration; use tokio::time::timeout; +use uuid::Uuid; +use uuid::Version; #[cfg(unix)] use std::os::unix::fs::symlink; @@ -611,3 +617,195 @@ async fn fs_copy_rejects_standalone_fifo_source() -> Result<()> { Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn fs_watch_directory_reports_changed_child_paths_and_unwatch_stops_notifications() +-> Result<()> { + let codex_home = TempDir::new()?; + let git_dir = codex_home.path().join("repo").join(".git"); + let fetch_head = git_dir.join("FETCH_HEAD"); + std::fs::create_dir_all(&git_dir)?; + std::fs::write(&fetch_head, "old\n")?; + + let mut mcp = initialized_mcp(&codex_home).await?; + let watch_request_id = mcp + .send_fs_watch_request(codex_app_server_protocol::FsWatchParams { + path: absolute_path(git_dir.clone()), + }) + .await?; + let watch_response: FsWatchResponse = to_response( + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(watch_request_id)), + ) + .await??, + )?; + assert_eq!(watch_response.path, absolute_path(git_dir.clone())); + let watch_id = Uuid::parse_str(&watch_response.watch_id)?; + assert_eq!(watch_id.get_version(), Some(Version::SortRand)); + + std::fs::write(&fetch_head, "updated\n")?; + + // Kernel file watching is not reliable in every sandboxed test environment. + // Keep validating notification shape when the backend does emit, but do not + // fail the whole suite if no OS event arrives. + if let Some(changed) = maybe_fs_changed_notification(&mut mcp).await? { + assert_eq!(changed.watch_id, watch_response.watch_id.clone()); + assert_eq!( + changed.changed_paths, + vec![absolute_path(fetch_head.clone())] + ); + } + while timeout( + Duration::from_millis(200), + mcp.read_stream_until_notification_message("fs/changed"), + ) + .await + .is_ok() + {} + + let unwatch_request_id = mcp + .send_fs_unwatch_request(FsUnwatchParams { + watch_id: watch_response.watch_id, + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(unwatch_request_id)), + ) + .await??; + + std::fs::write(git_dir.join("packed-refs"), "refs\n")?; + let maybe_notification = timeout( + Duration::from_millis(1500), + mcp.read_stream_until_notification_message("fs/changed"), + ) + .await; + assert!( + maybe_notification.is_err(), + "fs/unwatch should stop future change notifications" + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn fs_watch_file_reports_atomic_replace_events() -> Result<()> { + let codex_home = TempDir::new()?; + let git_dir = codex_home.path().join("repo").join(".git"); + let head_path = git_dir.join("HEAD"); + std::fs::create_dir_all(&git_dir)?; + std::fs::write(&head_path, "ref: refs/heads/main\n")?; + + let mut mcp = initialized_mcp(&codex_home).await?; + let watch_request_id = mcp + .send_fs_watch_request(codex_app_server_protocol::FsWatchParams { + path: absolute_path(head_path.clone()), + }) + .await?; + let watch_response: FsWatchResponse = to_response( + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(watch_request_id)), + ) + .await??, + )?; + assert_eq!(watch_response.path, absolute_path(head_path.clone())); + + replace_file_atomically(&head_path, "ref: refs/heads/feature\n")?; + + if let Some(changed) = maybe_fs_changed_notification(&mut mcp).await? { + assert_eq!( + changed, + FsChangedNotification { + watch_id: watch_response.watch_id, + changed_paths: vec![absolute_path(head_path.clone())], + } + ); + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn fs_watch_allows_missing_file_targets() -> Result<()> { + let codex_home = TempDir::new()?; + let git_dir = codex_home.path().join("repo").join(".git"); + let fetch_head = git_dir.join("FETCH_HEAD"); + std::fs::create_dir_all(&git_dir)?; + + let mut mcp = initialized_mcp(&codex_home).await?; + let watch_request_id = mcp + .send_fs_watch_request(codex_app_server_protocol::FsWatchParams { + path: absolute_path(fetch_head.clone()), + }) + .await?; + let watch_response: FsWatchResponse = to_response( + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(watch_request_id)), + ) + .await??, + )?; + assert_eq!(watch_response.path, absolute_path(fetch_head.clone())); + + replace_file_atomically(&fetch_head, "origin/main\n")?; + + if let Some(changed) = maybe_fs_changed_notification(&mut mcp).await? { + assert_eq!( + changed, + FsChangedNotification { + watch_id: watch_response.watch_id, + changed_paths: vec![absolute_path(fetch_head.clone())], + } + ); + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn fs_watch_rejects_relative_paths() -> Result<()> { + let codex_home = TempDir::new()?; + let mut mcp = initialized_mcp(&codex_home).await?; + + let watch_id = mcp + .send_raw_request("fs/watch", Some(json!({ "path": "relative-path" }))) + .await?; + expect_error_message( + &mut mcp, + watch_id, + "Invalid request: AbsolutePathBuf deserialized without a base path", + ) + .await?; + + Ok(()) +} + +fn fs_changed_notification(notification: JSONRPCNotification) -> Result { + let params = notification + .params + .context("fs/changed notification should include params")?; + Ok(serde_json::from_value::(params)?) +} + +async fn maybe_fs_changed_notification( + mcp: &mut McpProcess, +) -> Result> { + match timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("fs/changed"), + ) + .await + { + Ok(notification) => Ok(Some(fs_changed_notification(notification?)?)), + Err(_) => Ok(None), + } +} + +fn replace_file_atomically(path: &PathBuf, contents: &str) -> Result<()> { + let temp_path = path.with_extension("lock"); + std::fs::write(&temp_path, contents)?; + std::fs::rename(temp_path, path)?; + Ok(()) +} diff --git a/codex-rs/tui_app_server/src/app/app_server_adapter.rs b/codex-rs/tui_app_server/src/app/app_server_adapter.rs index 20f13589c9..c358b9cef1 100644 --- a/codex-rs/tui_app_server/src/app/app_server_adapter.rs +++ b/codex-rs/tui_app_server/src/app/app_server_adapter.rs @@ -388,6 +388,7 @@ fn server_notification_thread_target( | ServerNotification::FuzzyFileSearchSessionUpdated(_) | ServerNotification::FuzzyFileSearchSessionCompleted(_) | ServerNotification::CommandExecOutputDelta(_) + | ServerNotification::FsChanged(_) | ServerNotification::WindowsWorldWritableWarning(_) | ServerNotification::WindowsSandboxSetupCompleted(_) | ServerNotification::AccountLoginCompleted(_) => None, diff --git a/codex-rs/tui_app_server/src/chatwidget.rs b/codex-rs/tui_app_server/src/chatwidget.rs index b444dc13bc..df73ef20a3 100644 --- a/codex-rs/tui_app_server/src/chatwidget.rs +++ b/codex-rs/tui_app_server/src/chatwidget.rs @@ -6169,6 +6169,7 @@ impl ChatWidget { | ServerNotification::McpServerStatusUpdated(_) | ServerNotification::McpServerOauthLoginCompleted(_) | ServerNotification::AppListUpdated(_) + | ServerNotification::FsChanged(_) | ServerNotification::ContextCompacted(_) | ServerNotification::FuzzyFileSearchSessionUpdated(_) | ServerNotification::FuzzyFileSearchSessionCompleted(_) From 6323f0104d17d211029faab149231ba787f7da37 Mon Sep 17 00:00:00 2001 From: canvrno-oai Date: Tue, 24 Mar 2026 16:03:40 -0700 Subject: [PATCH 4/8] Use delayed shimmer for plugin loading headers in tui and tui_app_server (#15674) - Add a small delayed loading header for plugin list/detail loading messages in the TUI. Keep existing text for the first 1s, then show shimmer on the loading line. - Apply the same behavior in both tui and tui_app_server. https://github.com/user-attachments/assets/71dd35e4-7e3b-4e7b-867a-3c13dc395d3a --- codex-rs/tui/src/chatwidget/plugins.rs | 98 ++++++++++++++++--- ...t__tests__plugins_popup_loading_state.snap | 2 +- .../tui_app_server/src/chatwidget/plugins.rs | 98 ++++++++++++++++--- ...t__tests__plugins_popup_loading_state.snap | 2 +- 4 files changed, 168 insertions(+), 32 deletions(-) diff --git a/codex-rs/tui/src/chatwidget/plugins.rs b/codex-rs/tui/src/chatwidget/plugins.rs index 079f70e226..e44081f761 100644 --- a/codex-rs/tui/src/chatwidget/plugins.rs +++ b/codex-rs/tui/src/chatwidget/plugins.rs @@ -1,4 +1,6 @@ use std::path::PathBuf; +use std::time::Duration; +use std::time::Instant; use super::ChatWidget; use crate::app_event::AppEvent; @@ -7,6 +9,9 @@ use crate::bottom_pane::SelectionItem; use crate::bottom_pane::SelectionViewParams; use crate::history_cell; use crate::render::renderable::ColumnRenderable; +use crate::render::renderable::Renderable; +use crate::shimmer::shimmer_spans; +use crate::tui::FrameRequester; use codex_app_server_protocol::PluginDetail; use codex_app_server_protocol::PluginInstallPolicy; use codex_app_server_protocol::PluginInstallResponse; @@ -17,10 +22,76 @@ use codex_app_server_protocol::PluginSummary; use codex_app_server_protocol::PluginUninstallResponse; use codex_features::Feature; use codex_utils_absolute_path::AbsolutePathBuf; +use ratatui::buffer::Buffer; +use ratatui::layout::Rect; use ratatui::style::Stylize; use ratatui::text::Line; +use ratatui::widgets::Paragraph; +use ratatui::widgets::WidgetRef; const PLUGINS_SELECTION_VIEW_ID: &str = "plugins-selection"; +const LOADING_ANIMATION_DELAY: Duration = Duration::from_secs(1); +const LOADING_ANIMATION_INTERVAL: Duration = Duration::from_millis(100); + +struct DelayedLoadingHeader { + started_at: Instant, + frame_requester: FrameRequester, + animations_enabled: bool, + loading_text: String, + note: Option, +} + +impl DelayedLoadingHeader { + fn new( + frame_requester: FrameRequester, + animations_enabled: bool, + loading_text: String, + note: Option, + ) -> Self { + Self { + started_at: Instant::now(), + frame_requester, + animations_enabled, + loading_text, + note, + } + } +} + +impl Renderable for DelayedLoadingHeader { + fn render(&self, area: Rect, buf: &mut Buffer) { + if area.is_empty() { + return; + } + + let mut lines = Vec::with_capacity(3); + lines.push(Line::from("Plugins".bold())); + + let now = Instant::now(); + let elapsed = now.saturating_duration_since(self.started_at); + if elapsed < LOADING_ANIMATION_DELAY { + self.frame_requester + .schedule_frame_in(LOADING_ANIMATION_DELAY - elapsed); + lines.push(Line::from(self.loading_text.as_str().dim())); + } else if self.animations_enabled { + self.frame_requester + .schedule_frame_in(LOADING_ANIMATION_INTERVAL); + lines.push(Line::from(shimmer_spans(self.loading_text.as_str()))); + } else { + lines.push(Line::from(self.loading_text.as_str().dim())); + } + + if let Some(note) = &self.note { + lines.push(Line::from(note.as_str().dim())); + } + + Paragraph::new(lines).render_ref(area, buf); + } + + fn desired_height(&self, _width: u16) -> u16 { + 2 + u16::from(self.note.is_some()) + } +} #[derive(Debug, Clone, Default)] pub(super) enum PluginsCacheState { @@ -474,16 +545,14 @@ impl ChatWidget { } fn plugins_loading_popup_params(&self) -> SelectionViewParams { - let mut header = ColumnRenderable::new(); - header.push(Line::from("Plugins".bold())); - header.push(Line::from("Loading available plugins...".dim())); - header.push(Line::from( - "Available marketplaces will appear here when ready.".dim(), - )); - SelectionViewParams { view_id: Some(PLUGINS_SELECTION_VIEW_ID), - header: Box::new(header), + header: Box::new(DelayedLoadingHeader::new( + self.frame_requester.clone(), + self.config.animations, + "Loading available plugins...".to_string(), + Some("This first pass shows the ChatGPT marketplace only.".to_string()), + )), items: vec![SelectionItem { name: "Loading plugins...".to_string(), description: Some("This updates when the marketplace list is ready.".to_string()), @@ -495,15 +564,14 @@ impl ChatWidget { } fn plugin_detail_loading_popup_params(&self, plugin_display_name: &str) -> SelectionViewParams { - let mut header = ColumnRenderable::new(); - header.push(Line::from("Plugins".bold())); - header.push(Line::from( - format!("Loading details for {plugin_display_name}...").dim(), - )); - SelectionViewParams { view_id: Some(PLUGINS_SELECTION_VIEW_ID), - header: Box::new(header), + header: Box::new(DelayedLoadingHeader::new( + self.frame_requester.clone(), + self.config.animations, + format!("Loading details for {plugin_display_name}..."), + /*note*/ None, + )), items: vec![SelectionItem { name: "Loading plugin details...".to_string(), description: Some("This updates when plugin details load.".to_string()), diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugins_popup_loading_state.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugins_popup_loading_state.snap index 42eff7f81f..2b0f6837eb 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugins_popup_loading_state.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugins_popup_loading_state.snap @@ -4,6 +4,6 @@ expression: popup --- Plugins Loading available plugins... - Available marketplaces will appear here when ready. + This first pass shows the ChatGPT marketplace only. › Loading plugins... This updates when the marketplace list is ready. diff --git a/codex-rs/tui_app_server/src/chatwidget/plugins.rs b/codex-rs/tui_app_server/src/chatwidget/plugins.rs index 079f70e226..e44081f761 100644 --- a/codex-rs/tui_app_server/src/chatwidget/plugins.rs +++ b/codex-rs/tui_app_server/src/chatwidget/plugins.rs @@ -1,4 +1,6 @@ use std::path::PathBuf; +use std::time::Duration; +use std::time::Instant; use super::ChatWidget; use crate::app_event::AppEvent; @@ -7,6 +9,9 @@ use crate::bottom_pane::SelectionItem; use crate::bottom_pane::SelectionViewParams; use crate::history_cell; use crate::render::renderable::ColumnRenderable; +use crate::render::renderable::Renderable; +use crate::shimmer::shimmer_spans; +use crate::tui::FrameRequester; use codex_app_server_protocol::PluginDetail; use codex_app_server_protocol::PluginInstallPolicy; use codex_app_server_protocol::PluginInstallResponse; @@ -17,10 +22,76 @@ use codex_app_server_protocol::PluginSummary; use codex_app_server_protocol::PluginUninstallResponse; use codex_features::Feature; use codex_utils_absolute_path::AbsolutePathBuf; +use ratatui::buffer::Buffer; +use ratatui::layout::Rect; use ratatui::style::Stylize; use ratatui::text::Line; +use ratatui::widgets::Paragraph; +use ratatui::widgets::WidgetRef; const PLUGINS_SELECTION_VIEW_ID: &str = "plugins-selection"; +const LOADING_ANIMATION_DELAY: Duration = Duration::from_secs(1); +const LOADING_ANIMATION_INTERVAL: Duration = Duration::from_millis(100); + +struct DelayedLoadingHeader { + started_at: Instant, + frame_requester: FrameRequester, + animations_enabled: bool, + loading_text: String, + note: Option, +} + +impl DelayedLoadingHeader { + fn new( + frame_requester: FrameRequester, + animations_enabled: bool, + loading_text: String, + note: Option, + ) -> Self { + Self { + started_at: Instant::now(), + frame_requester, + animations_enabled, + loading_text, + note, + } + } +} + +impl Renderable for DelayedLoadingHeader { + fn render(&self, area: Rect, buf: &mut Buffer) { + if area.is_empty() { + return; + } + + let mut lines = Vec::with_capacity(3); + lines.push(Line::from("Plugins".bold())); + + let now = Instant::now(); + let elapsed = now.saturating_duration_since(self.started_at); + if elapsed < LOADING_ANIMATION_DELAY { + self.frame_requester + .schedule_frame_in(LOADING_ANIMATION_DELAY - elapsed); + lines.push(Line::from(self.loading_text.as_str().dim())); + } else if self.animations_enabled { + self.frame_requester + .schedule_frame_in(LOADING_ANIMATION_INTERVAL); + lines.push(Line::from(shimmer_spans(self.loading_text.as_str()))); + } else { + lines.push(Line::from(self.loading_text.as_str().dim())); + } + + if let Some(note) = &self.note { + lines.push(Line::from(note.as_str().dim())); + } + + Paragraph::new(lines).render_ref(area, buf); + } + + fn desired_height(&self, _width: u16) -> u16 { + 2 + u16::from(self.note.is_some()) + } +} #[derive(Debug, Clone, Default)] pub(super) enum PluginsCacheState { @@ -474,16 +545,14 @@ impl ChatWidget { } fn plugins_loading_popup_params(&self) -> SelectionViewParams { - let mut header = ColumnRenderable::new(); - header.push(Line::from("Plugins".bold())); - header.push(Line::from("Loading available plugins...".dim())); - header.push(Line::from( - "Available marketplaces will appear here when ready.".dim(), - )); - SelectionViewParams { view_id: Some(PLUGINS_SELECTION_VIEW_ID), - header: Box::new(header), + header: Box::new(DelayedLoadingHeader::new( + self.frame_requester.clone(), + self.config.animations, + "Loading available plugins...".to_string(), + Some("This first pass shows the ChatGPT marketplace only.".to_string()), + )), items: vec![SelectionItem { name: "Loading plugins...".to_string(), description: Some("This updates when the marketplace list is ready.".to_string()), @@ -495,15 +564,14 @@ impl ChatWidget { } fn plugin_detail_loading_popup_params(&self, plugin_display_name: &str) -> SelectionViewParams { - let mut header = ColumnRenderable::new(); - header.push(Line::from("Plugins".bold())); - header.push(Line::from( - format!("Loading details for {plugin_display_name}...").dim(), - )); - SelectionViewParams { view_id: Some(PLUGINS_SELECTION_VIEW_ID), - header: Box::new(header), + header: Box::new(DelayedLoadingHeader::new( + self.frame_requester.clone(), + self.config.animations, + format!("Loading details for {plugin_display_name}..."), + /*note*/ None, + )), items: vec![SelectionItem { name: "Loading plugin details...".to_string(), description: Some("This updates when plugin details load.".to_string()), diff --git a/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__plugins_popup_loading_state.snap b/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__plugins_popup_loading_state.snap index 846c73fe47..f1f18c1eaf 100644 --- a/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__plugins_popup_loading_state.snap +++ b/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__plugins_popup_loading_state.snap @@ -4,6 +4,6 @@ expression: popup --- Plugins Loading available plugins... - Available marketplaces will appear here when ready. + This first pass shows the ChatGPT marketplace only. › Loading plugins... This updates when the marketplace list is ready. From 24c4ecaaac7ba9205e9c48376de7450a087f1576 Mon Sep 17 00:00:00 2001 From: Ruslan Nigmatullin Date: Tue, 24 Mar 2026 16:13:34 -0700 Subject: [PATCH 5/8] app-server: Return codex home in initialize response (#15689) This allows clients to get enough information to interact with the codex skills/configuration/etc. --- .../json/codex_app_server_protocol.schemas.json | 13 +++++++++++++ .../schema/json/v1/InitializeResponse.json | 15 +++++++++++++++ .../schema/typescript/InitializeResponse.ts | 5 +++++ codex-rs/app-server-protocol/src/protocol/v1.rs | 2 ++ codex-rs/app-server/README.md | 2 +- codex-rs/app-server/src/message_processor.rs | 14 ++++++++++++++ codex-rs/app-server/tests/suite/v2/initialize.rs | 7 +++++++ 7 files changed, 57 insertions(+), 1 deletion(-) 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 91f32bede9..ad79d0987e 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 @@ -1,6 +1,10 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { + "AbsolutePathBuf": { + "description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.", + "type": "string" + }, "AdditionalFileSystemPermissions": { "properties": { "read": { @@ -2305,6 +2309,14 @@ "InitializeResponse": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { + "codexHome": { + "allOf": [ + { + "$ref": "#/definitions/v2/AbsolutePathBuf" + } + ], + "description": "Absolute path to the server's $CODEX_HOME directory." + }, "platformFamily": { "description": "Platform family for the running app-server target, for example `\"unix\"` or `\"windows\"`.", "type": "string" @@ -2318,6 +2330,7 @@ } }, "required": [ + "codexHome", "platformFamily", "platformOs", "userAgent" diff --git a/codex-rs/app-server-protocol/schema/json/v1/InitializeResponse.json b/codex-rs/app-server-protocol/schema/json/v1/InitializeResponse.json index ada38a6582..1de65f82f4 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/InitializeResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v1/InitializeResponse.json @@ -1,6 +1,20 @@ { "$schema": "http://json-schema.org/draft-07/schema#", + "definitions": { + "AbsolutePathBuf": { + "description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.", + "type": "string" + } + }, "properties": { + "codexHome": { + "allOf": [ + { + "$ref": "#/definitions/AbsolutePathBuf" + } + ], + "description": "Absolute path to the server's $CODEX_HOME directory." + }, "platformFamily": { "description": "Platform family for the running app-server target, for example `\"unix\"` or `\"windows\"`.", "type": "string" @@ -14,6 +28,7 @@ } }, "required": [ + "codexHome", "platformFamily", "platformOs", "userAgent" diff --git a/codex-rs/app-server-protocol/schema/typescript/InitializeResponse.ts b/codex-rs/app-server-protocol/schema/typescript/InitializeResponse.ts index 47978fc8d1..c1d0a822f2 100644 --- a/codex-rs/app-server-protocol/schema/typescript/InitializeResponse.ts +++ b/codex-rs/app-server-protocol/schema/typescript/InitializeResponse.ts @@ -1,8 +1,13 @@ // GENERATED CODE! DO NOT MODIFY BY HAND! // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { AbsolutePathBuf } from "./AbsolutePathBuf"; export type InitializeResponse = { userAgent: string, +/** + * Absolute path to the server's $CODEX_HOME directory. + */ +codexHome: AbsolutePathBuf, /** * Platform family for the running app-server target, for example * `"unix"` or `"windows"`. diff --git a/codex-rs/app-server-protocol/src/protocol/v1.rs b/codex-rs/app-server-protocol/src/protocol/v1.rs index 67e552dea3..6aa2e9fa30 100644 --- a/codex-rs/app-server-protocol/src/protocol/v1.rs +++ b/codex-rs/app-server-protocol/src/protocol/v1.rs @@ -56,6 +56,8 @@ pub struct InitializeCapabilities { #[serde(rename_all = "camelCase")] pub struct InitializeResponse { pub user_agent: String, + /// Absolute path to the server's $CODEX_HOME directory. + pub codex_home: AbsolutePathBuf, /// Platform family for the running app-server target, for example /// `"unix"` or `"windows"`. pub platform_family: String, diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 76312e81ea..b877c4a035 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -75,7 +75,7 @@ Use the thread APIs to create, list, or archive conversations. Drive a conversat ## Initialization -Clients must send a single `initialize` request per transport connection before invoking any other method on that connection, then acknowledge with an `initialized` notification. The server returns the user agent string it will present to upstream services plus `platformFamily` and `platformOs` strings describing the app-server runtime target; subsequent requests issued before initialization receive a `"Not initialized"` error, and repeated `initialize` calls on the same connection receive an `"Already initialized"` error. +Clients must send a single `initialize` request per transport connection before invoking any other method on that connection, then acknowledge with an `initialized` notification. The server returns the user agent string it will present to upstream services, `codexHome` for the server's Codex home directory, and `platformFamily` and `platformOs` strings describing the app-server runtime target; subsequent requests issued before initialization receive a `"Not initialized"` error, and repeated `initialize` calls on the same connection receive an `"Already initialized"` error. `initialize.params.capabilities` also supports per-connection notification opt-out via `optOutNotificationMethods`, which is a list of exact method names to suppress for that connection. Matching is exact (no wildcards/prefixes). Unknown method names are accepted and ignored. diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index ad7f5ca626..a67c810bd0 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -8,6 +8,7 @@ use std::sync::atomic::Ordering; use crate::codex_message_processor::CodexMessageProcessor; use crate::codex_message_processor::CodexMessageProcessorArgs; use crate::config_api::ConfigApi; +use crate::error_code::INTERNAL_ERROR_CODE; use crate::error_code::INVALID_REQUEST_ERROR_CODE; use crate::external_agent_config_api::ExternalAgentConfigApi; use crate::fs_api::FsApi; @@ -590,8 +591,21 @@ impl MessageProcessor { } let user_agent = get_codex_user_agent(); + let codex_home = match self.config.codex_home.clone().try_into() { + Ok(codex_home) => codex_home, + Err(err) => { + let error = JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message: format!("Invalid CODEX_HOME: {err}"), + data: None, + }; + self.outgoing.send_error(connection_request_id, error).await; + return; + } + }; let response = InitializeResponse { user_agent, + codex_home, platform_family: std::env::consts::FAMILY.to_string(), platform_os: std::env::consts::OS.to_string(), }; diff --git a/codex-rs/app-server/tests/suite/v2/initialize.rs b/codex-rs/app-server/tests/suite/v2/initialize.rs index 9b5f0cacc8..165160468f 100644 --- a/codex-rs/app-server/tests/suite/v2/initialize.rs +++ b/codex-rs/app-server/tests/suite/v2/initialize.rs @@ -14,6 +14,7 @@ use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::UserInput as V2UserInput; +use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_cargo_bin::cargo_bin; use core_test_support::fs_wait; use pretty_assertions::assert_eq; @@ -30,6 +31,7 @@ async fn initialize_uses_client_info_name_as_originator() -> Result<()> { let responses = Vec::new(); let server = create_mock_responses_server_sequence_unchecked(responses).await; let codex_home = TempDir::new()?; + let expected_codex_home = AbsolutePathBuf::try_from(codex_home.path().canonicalize()?)?; create_config_toml(codex_home.path(), &server.uri(), "never")?; let mut mcp = McpProcess::new(codex_home.path()).await?; @@ -48,11 +50,13 @@ async fn initialize_uses_client_info_name_as_originator() -> Result<()> { }; let InitializeResponse { user_agent, + codex_home: response_codex_home, platform_family, platform_os, } = to_response::(response)?; assert!(user_agent.starts_with("codex_vscode/")); + assert_eq!(response_codex_home, expected_codex_home); assert_eq!(platform_family, std::env::consts::FAMILY); assert_eq!(platform_os, std::env::consts::OS); Ok(()) @@ -63,6 +67,7 @@ async fn initialize_respects_originator_override_env_var() -> Result<()> { let responses = Vec::new(); let server = create_mock_responses_server_sequence_unchecked(responses).await; let codex_home = TempDir::new()?; + let expected_codex_home = AbsolutePathBuf::try_from(codex_home.path().canonicalize()?)?; create_config_toml(codex_home.path(), &server.uri(), "never")?; let mut mcp = McpProcess::new_with_env( codex_home.path(), @@ -88,11 +93,13 @@ async fn initialize_respects_originator_override_env_var() -> Result<()> { }; let InitializeResponse { user_agent, + codex_home: response_codex_home, platform_family, platform_os, } = to_response::(response)?; assert!(user_agent.starts_with("codex_originator_via_env_var/")); + assert_eq!(response_codex_home, expected_codex_home); assert_eq!(platform_family, std::env::consts::FAMILY); assert_eq!(platform_os, std::env::consts::OS); Ok(()) From 4a210faf338c9a63c27c4216c1588097472fa2b3 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 24 Mar 2026 16:32:31 -0700 Subject: [PATCH 6/8] fix: keep rmcp-client env vars as OsString (#15363) ## Why This is a follow-up to #15360. That change fixed the `arg0` helper setup, but `rmcp-client` still coerced stdio transport environment values into UTF-8 `String`s before program resolution and process spawn. If `PATH` or another inherited environment value contains non-UTF-8 bytes, that loses fidelity before it reaches `which` and `Command`. ## What changed - change `create_env_for_mcp_server()` to return `HashMap` and read inherited values with `std::env::var_os()` - change `TransportRecipe::Stdio.env`, `RmcpClient::new_stdio_client()`, and `program_resolver::resolve()` to keep stdio transport env values in `OsString` form within `rmcp-client` - keep the `codex-core` config boundary stringly, but convert configured stdio env values to `OsString` once when constructing the transport - update the rmcp-client stdio test fixtures and callers to use `OsString` env maps - add a Unix regression test that verifies `create_env_for_mcp_server()` preserves a non-UTF-8 `PATH` ## How to verify - `cargo test -p codex-rmcp-client` - `cargo test -p codex-core mcp_connection_manager` - `just argument-comment-lint` Targeted coverage in this change includes `utils::tests::create_env_preserves_path_when_it_is_not_utf8`, while the updated stdio transport path is exercised by the existing rmcp-client tests that construct `RmcpClient::new_stdio_client()`. --- codex-rs/core/src/mcp_connection_manager.rs | 7 ++- codex-rs/rmcp-client/src/program_resolver.rs | 52 ++++++++++--------- codex-rs/rmcp-client/src/rmcp_client.rs | 4 +- codex-rs/rmcp-client/src/utils.rs | 40 ++++++++++---- .../tests/process_group_cleanup.rs | 4 +- 5 files changed, 67 insertions(+), 40 deletions(-) diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 938d6d0b2b..eac8485048 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -1440,7 +1440,12 @@ async fn make_rmcp_client( } => { let command_os: OsString = command.into(); let args_os: Vec = args.into_iter().map(Into::into).collect(); - RmcpClient::new_stdio_client(command_os, args_os, env, &env_vars, cwd) + let env_os = env.map(|env| { + env.into_iter() + .map(|(key, value)| (key.into(), value.into())) + .collect::>() + }); + RmcpClient::new_stdio_client(command_os, args_os, env_os, &env_vars, cwd) .await .map_err(|err| StartupOutcomeError::from(anyhow!(err))) } diff --git a/codex-rs/rmcp-client/src/program_resolver.rs b/codex-rs/rmcp-client/src/program_resolver.rs index e62336e263..d9be2741c7 100644 --- a/codex-rs/rmcp-client/src/program_resolver.rs +++ b/codex-rs/rmcp-client/src/program_resolver.rs @@ -13,18 +13,13 @@ use std::collections::HashMap; use std::ffi::OsString; -#[cfg(windows)] -use std::env; -#[cfg(windows)] -use tracing::debug; - /// Resolves a program to its executable path on Unix systems. /// /// Unix systems handle PATH resolution and script execution natively through /// the kernel's shebang (`#!`) mechanism, so this function simply returns /// the program name unchanged. #[cfg(unix)] -pub fn resolve(program: OsString, _env: &HashMap) -> std::io::Result { +pub fn resolve(program: OsString, _env: &HashMap) -> std::io::Result { Ok(program) } @@ -38,25 +33,22 @@ pub fn resolve(program: OsString, _env: &HashMap) -> std::io::Re /// This enables tools like `npx`, `pnpm`, and `yarn` to work correctly on Windows /// without requiring users to specify full paths or extensions in their configuration. #[cfg(windows)] -pub fn resolve(program: OsString, env: &HashMap) -> std::io::Result { +pub fn resolve(program: OsString, env: &HashMap) -> std::io::Result { // Get current directory for relative path resolution - let cwd = env::current_dir() + let cwd = std::env::current_dir() .map_err(|e| std::io::Error::other(format!("Failed to get current directory: {e}")))?; // Extract PATH from environment for search locations - let search_path = env.get("PATH"); + let search_path = env.get(std::ffi::OsStr::new("PATH")); // Attempt resolution via which crate match which::which_in(&program, search_path, &cwd) { Ok(resolved) => { - debug!("Resolved {:?} to {:?}", program, resolved); + tracing::debug!("Resolved {program:?} to {resolved:?}"); Ok(resolved.into_os_string()) } Err(e) => { - debug!( - "Failed to resolve {:?}: {}. Using original path", - program, e - ); + tracing::debug!("Failed to resolve {program:?}: {e}. Using original path"); // Fallback to original program - let Command::new() handle the error Ok(program) } @@ -146,7 +138,7 @@ mod tests { // Held to prevent the temporary directory from being deleted. _temp_dir: TempDir, program_name: String, - mcp_env: HashMap, + mcp_env: HashMap, } impl TestExecutableEnv { @@ -160,10 +152,10 @@ mod tests { // Build a clean environment with the temp dir in the PATH. let mut extra_env = HashMap::new(); - extra_env.insert("PATH".to_string(), Self::build_path(dir_path)); + extra_env.insert(OsString::from("PATH"), Self::build_path_env_var(dir_path)); #[cfg(windows)] - extra_env.insert("PATHEXT".to_string(), Self::ensure_cmd_extension()); + extra_env.insert(OsString::from("PATHEXT"), Self::ensure_cmd_extension()); let mcp_env = create_env_for_mcp_server(Some(extra_env), &[]); @@ -202,20 +194,30 @@ mod tests { } /// Prepends the given directory to the system's PATH variable. - fn build_path(dir: &Path) -> String { - let current = std::env::var("PATH").unwrap_or_default(); - let sep = if cfg!(windows) { ";" } else { ":" }; - format!("{}{sep}{current}", dir.to_string_lossy()) + fn build_path_env_var(dir: &Path) -> OsString { + let mut path = OsString::from(dir.as_os_str()); + if let Some(current) = std::env::var_os("PATH") { + let sep = if cfg!(windows) { ";" } else { ":" }; + path.push(sep); + path.push(current); + } + path } /// Ensures `.CMD` is in the `PATHEXT` variable on Windows for script discovery. #[cfg(windows)] - fn ensure_cmd_extension() -> String { - let current = std::env::var("PATHEXT").unwrap_or_default(); - if current.to_uppercase().contains(".CMD") { + fn ensure_cmd_extension() -> OsString { + let current = std::env::var_os("PATHEXT").unwrap_or_default(); + if current + .to_string_lossy() + .to_ascii_uppercase() + .contains(".CMD") + { current } else { - format!(".CMD;{current}") + let mut path_ext = OsString::from(".CMD;"); + path_ext.push(current); + path_ext } } } diff --git a/codex-rs/rmcp-client/src/rmcp_client.rs b/codex-rs/rmcp-client/src/rmcp_client.rs index 55a3603ed7..aa460c21b9 100644 --- a/codex-rs/rmcp-client/src/rmcp_client.rs +++ b/codex-rs/rmcp-client/src/rmcp_client.rs @@ -390,7 +390,7 @@ enum TransportRecipe { Stdio { program: OsString, args: Vec, - env: Option>, + env: Option>, env_vars: Vec, cwd: Option, }, @@ -478,7 +478,7 @@ impl RmcpClient { pub async fn new_stdio_client( program: OsString, args: Vec, - env: Option>, + env: Option>, env_vars: &[String], cwd: Option, ) -> io::Result { diff --git a/codex-rs/rmcp-client/src/utils.rs b/codex-rs/rmcp-client/src/utils.rs index df09b0f8be..cb906aaaa9 100644 --- a/codex-rs/rmcp-client/src/utils.rs +++ b/codex-rs/rmcp-client/src/utils.rs @@ -5,16 +5,17 @@ use reqwest::header::HeaderName; use reqwest::header::HeaderValue; use std::collections::HashMap; use std::env; +use std::ffi::OsString; pub(crate) fn create_env_for_mcp_server( - extra_env: Option>, + extra_env: Option>, env_vars: &[String], -) -> HashMap { +) -> HashMap { DEFAULT_ENV_VARS .iter() .copied() .chain(env_vars.iter().map(String::as_str)) - .filter_map(|var| env::var(var).ok().map(|value| (var.to_string(), value))) + .filter_map(|var| env::var_os(var).map(|value| (OsString::from(var), value))) .chain(extra_env.unwrap_or_default()) .collect() } @@ -140,7 +141,7 @@ mod tests { use pretty_assertions::assert_eq; use serial_test::serial; - use std::ffi::OsString; + use std::ffi::OsStr; struct EnvVarGuard { key: String, @@ -148,10 +149,10 @@ mod tests { } impl EnvVarGuard { - fn set(key: &str, value: &str) -> Self { + fn set(key: &str, value: impl AsRef) -> Self { let original = std::env::var_os(key); unsafe { - std::env::set_var(key, value); + std::env::set_var(key, value.as_ref()); } Self { key: key.to_string(), @@ -177,9 +178,12 @@ mod tests { #[tokio::test] async fn create_env_honors_overrides() { let value = "custom".to_string(); - let env = - create_env_for_mcp_server(Some(HashMap::from([("TZ".into(), value.clone())])), &[]); - assert_eq!(env.get("TZ"), Some(&value)); + let expected = OsString::from(&value); + let env = create_env_for_mcp_server( + Some(HashMap::from([(OsString::from("TZ"), expected.clone())])), + &[], + ); + assert_eq!(env.get(OsStr::new("TZ")), Some(&expected)); } #[test] @@ -187,8 +191,24 @@ mod tests { fn create_env_includes_additional_whitelisted_variables() { let custom_var = "EXTRA_RMCP_ENV"; let value = "from-env"; + let expected = OsString::from(value); let _guard = EnvVarGuard::set(custom_var, value); let env = create_env_for_mcp_server(None, &[custom_var.to_string()]); - assert_eq!(env.get(custom_var), Some(&value.to_string())); + assert_eq!(env.get(OsStr::new(custom_var)), Some(&expected)); + } + + #[cfg(unix)] + #[test] + #[serial(extra_rmcp_env)] + fn create_env_preserves_path_when_it_is_not_utf8() { + use std::os::unix::ffi::OsStrExt; + + let raw_path = std::ffi::OsStr::from_bytes(b"/tmp/codex-\xFF/bin"); + let expected = raw_path.to_os_string(); + let _guard = EnvVarGuard::set("PATH", raw_path); + + let env = create_env_for_mcp_server(None, &[]); + + assert_eq!(env.get(OsStr::new("PATH")), Some(&expected)); } } diff --git a/codex-rs/rmcp-client/tests/process_group_cleanup.rs b/codex-rs/rmcp-client/tests/process_group_cleanup.rs index bb033af875..2d6c220ce8 100644 --- a/codex-rs/rmcp-client/tests/process_group_cleanup.rs +++ b/codex-rs/rmcp-client/tests/process_group_cleanup.rs @@ -73,8 +73,8 @@ async fn drop_kills_wrapper_process_group() -> Result<()> { ), ], Some(HashMap::from([( - "CHILD_PID_FILE".to_string(), - child_pid_file_str, + OsString::from("CHILD_PID_FILE"), + OsString::from(child_pid_file_str), )])), &[], None, From 05b967c79a72e409cd9d8d6536ca29201e4873fb Mon Sep 17 00:00:00 2001 From: canvrno-oai Date: Tue, 24 Mar 2026 17:40:14 -0700 Subject: [PATCH 7/8] Remove provenance filtering in $mentions for apps and skills from plugins (#15700) - Removes provenance filtering in the mentions feature for apps and skills that were installed as part of a plugin. - All skills and apps for a plugin are mentionable with this change. --- codex-rs/tui/src/bottom_pane/chat_composer.rs | 112 ++---------------- .../src/bottom_pane/chat_composer.rs | 62 ++-------- 2 files changed, 24 insertions(+), 150 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 7e5831ab5b..6d5c92d490 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -3584,41 +3584,8 @@ impl ChatComposer { fn mention_items(&self) -> Vec { let mut mentions = Vec::new(); - let plugin_namespaces: HashSet = - self.plugins.as_ref().map_or_else(HashSet::new, |plugins| { - plugins - .iter() - .filter_map(|plugin| { - let (plugin_name, _) = plugin - .config_name - .split_once('@') - .unwrap_or((plugin.config_name.as_str(), "")); - let plugin_name = plugin_name.trim(); - if plugin_name.is_empty() { - None - } else { - Some(plugin_name.to_ascii_lowercase()) - } - }) - .collect() - }); - let plugin_display_names: HashSet = - self.plugins.as_ref().map_or_else(HashSet::new, |plugins| { - plugins - .iter() - .map(|plugin| plugin.display_name.to_ascii_lowercase()) - .collect() - }); - if let Some(skills) = self.skills.as_ref() { for skill in skills { - let is_plugin_namespaced_skill = - skill.name.split_once(':').is_some_and(|(namespace, _)| { - plugin_namespaces.contains(&namespace.to_ascii_lowercase()) - }); - if is_plugin_namespaced_skill { - continue; - } let display_name = skill_display_name(skill).to_string(); let description = skill_description(skill); let skill_name = skill.name.clone(); @@ -3698,30 +3665,18 @@ impl ChatComposer { if !connector.is_accessible || !connector.is_enabled { continue; } - let plugin_backed_connector = connector - .plugin_display_names - .iter() - .any(|name| plugin_display_names.contains(&name.to_ascii_lowercase())); - if plugin_backed_connector { - continue; - } let display_name = connectors::connector_display_label(connector); let description = Some(Self::connector_brief_description(connector)); let slug = codex_core::connectors::connector_mention_slug(connector); let search_terms = vec![display_name.clone(), connector.id.clone(), slug.clone()]; let connector_id = connector.id.as_str(); - let category_tag = if connector.plugin_display_names.is_empty() { - "[App]".to_string() - } else { - "[Plugin]".to_string() - }; mentions.push(MentionItem { display_name: display_name.clone(), description, insert_text: format!("${slug}"), search_terms, path: Some(format!("app://{connector_id}")), - category_tag: Some(category_tag), + category_tag: Some("[App]".to_string()), sort_rank: 1, }); } @@ -5428,7 +5383,7 @@ mod tests { } #[test] - fn mention_items_hide_plugin_owned_skill_and_app_duplicates() { + fn mention_items_show_plugin_owned_skill_and_app_duplicates() { let (tx, _rx) = unbounded_channel::(); let sender = AppEventSender::new(tx); let mut composer = ChatComposer::new( @@ -5491,63 +5446,20 @@ mod tests { })); let mentions = composer.mention_items(); - assert_eq!(mentions.len(), 1); + assert_eq!(mentions.len(), 3); + assert_eq!(mentions[0].category_tag, Some("[Skill]".to_string())); + assert_eq!( + mentions[0].path, + Some("/tmp/repo/google-calendar/SKILL.md".to_string()) + ); assert_eq!(mentions[0].display_name, "Google Calendar".to_string()); - assert_eq!(mentions[0].category_tag, Some("[Plugin]".to_string())); + assert_eq!(mentions[1].category_tag, Some("[Plugin]".to_string())); assert_eq!( - mentions[0].path, - Some("plugin://google-calendar@debug".to_string()) - ); - } - - #[test] - fn mention_items_hide_plugin_owned_app_duplicates_when_provenance_matches() { - let (tx, _rx) = unbounded_channel::(); - let sender = AppEventSender::new(tx); - let mut composer = ChatComposer::new( - true, - sender, - false, - "Ask Codex to do anything".to_string(), - false, - ); - composer.set_connectors_enabled(true); - composer.set_text_content("$goog".to_string(), Vec::new(), Vec::new()); - composer.set_plugin_mentions(Some(vec![PluginCapabilitySummary { - config_name: "google-calendar@debug".to_string(), - display_name: "Google Calendar".to_string(), - description: None, - has_skills: false, - mcp_server_names: vec!["google-calendar".to_string()], - app_connector_ids: vec![codex_core::plugins::AppConnectorId( - "google_calendar".to_string(), - )], - }])); - composer.set_connector_mentions(Some(ConnectorsSnapshot { - connectors: vec![AppInfo { - id: "google_calendar".to_string(), - name: "Google Calendar".to_string(), - description: Some("Look up events and availability".to_string()), - logo_url: None, - logo_url_dark: None, - distribution_channel: None, - branding: None, - app_metadata: None, - labels: None, - install_url: Some("https://example.test/google-calendar".to_string()), - is_accessible: true, - is_enabled: true, - plugin_display_names: vec!["Google Calendar".to_string()], - }], - })); - - let mentions = composer.mention_items(); - assert_eq!(mentions.len(), 1); - assert_eq!(mentions[0].category_tag, Some("[Plugin]".to_string())); - assert_eq!( - mentions[0].path, + mentions[1].path, Some("plugin://google-calendar@debug".to_string()) ); + assert_eq!(mentions[2].category_tag, Some("[App]".to_string())); + assert_eq!(mentions[2].path, Some("app://google_calendar".to_string())); } #[test] diff --git a/codex-rs/tui_app_server/src/bottom_pane/chat_composer.rs b/codex-rs/tui_app_server/src/bottom_pane/chat_composer.rs index d7dc0ae096..fea3b8c2ad 100644 --- a/codex-rs/tui_app_server/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui_app_server/src/bottom_pane/chat_composer.rs @@ -3599,41 +3599,8 @@ impl ChatComposer { fn mention_items(&self) -> Vec { let mut mentions = Vec::new(); - let plugin_namespaces: HashSet = - self.plugins.as_ref().map_or_else(HashSet::new, |plugins| { - plugins - .iter() - .filter_map(|plugin| { - let (plugin_name, _) = plugin - .config_name - .split_once('@') - .unwrap_or((plugin.config_name.as_str(), "")); - let plugin_name = plugin_name.trim(); - if plugin_name.is_empty() { - None - } else { - Some(plugin_name.to_ascii_lowercase()) - } - }) - .collect() - }); - let plugin_display_names: HashSet = - self.plugins.as_ref().map_or_else(HashSet::new, |plugins| { - plugins - .iter() - .map(|plugin| plugin.display_name.to_ascii_lowercase()) - .collect() - }); - if let Some(skills) = self.skills.as_ref() { for skill in skills { - let is_plugin_namespaced_skill = - skill.name.split_once(':').is_some_and(|(namespace, _)| { - plugin_namespaces.contains(&namespace.to_ascii_lowercase()) - }); - if is_plugin_namespaced_skill { - continue; - } let display_name = skill_display_name(skill).to_string(); let description = skill_description(skill); let skill_name = skill.name.clone(); @@ -3713,30 +3680,18 @@ impl ChatComposer { if !connector.is_accessible || !connector.is_enabled { continue; } - let plugin_backed_connector = connector - .plugin_display_names - .iter() - .any(|name| plugin_display_names.contains(&name.to_ascii_lowercase())); - if plugin_backed_connector { - continue; - } let display_name = connectors::connector_display_label(connector); let description = Some(Self::connector_brief_description(connector)); let slug = codex_core::connectors::connector_mention_slug(connector); let search_terms = vec![display_name.clone(), connector.id.clone(), slug.clone()]; let connector_id = connector.id.as_str(); - let category_tag = if connector.plugin_display_names.is_empty() { - "[App]".to_string() - } else { - "[Plugin]".to_string() - }; mentions.push(MentionItem { display_name: display_name.clone(), description, insert_text: format!("${slug}"), search_terms, path: Some(format!("app://{connector_id}")), - category_tag: Some(category_tag), + category_tag: Some("[App]".to_string()), sort_rank: 1, }); } @@ -5443,7 +5398,7 @@ mod tests { } #[test] - fn mention_items_hide_plugin_owned_skill_and_app_duplicates() { + fn mention_items_show_plugin_owned_skill_and_app_duplicates() { let (tx, _rx) = unbounded_channel::(); let sender = AppEventSender::new(tx); let mut composer = ChatComposer::new( @@ -5506,13 +5461,20 @@ mod tests { })); let mentions = composer.mention_items(); - assert_eq!(mentions.len(), 1); - assert_eq!(mentions[0].display_name, "Google Calendar".to_string()); - assert_eq!(mentions[0].category_tag, Some("[Plugin]".to_string())); + assert_eq!(mentions.len(), 3); + assert_eq!(mentions[0].category_tag, Some("[Skill]".to_string())); assert_eq!( mentions[0].path, + Some("/tmp/repo/google-calendar/SKILL.md".to_string()) + ); + assert_eq!(mentions[0].display_name, "Google Calendar".to_string()); + assert_eq!(mentions[1].category_tag, Some("[Plugin]".to_string())); + assert_eq!( + mentions[1].path, Some("plugin://google-calendar@debug".to_string()) ); + assert_eq!(mentions[2].category_tag, Some("[App]".to_string())); + assert_eq!(mentions[2].path, Some("app://google_calendar".to_string())); } #[test] From 38b638d89d89bdc2a6bc43d0bc2d084254df29b9 Mon Sep 17 00:00:00 2001 From: canvrno-oai Date: Tue, 24 Mar 2026 17:40:26 -0700 Subject: [PATCH 8/8] Add legal link to TUI /plugin details (#15692) - Adds language and "[learn more](https://help.openai.com/en/articles/11487775-apps-in-chatgpt)" link to plugin details pages. - Message is hidden when plugin is installed image --- codex-rs/tui/src/chatwidget/plugins.rs | 56 ++++++++++++- ...ests__plugin_detail_popup_installable.snap | 5 +- ..._tests__plugin_detail_popup_installed.snap | 15 ++++ codex-rs/tui/src/chatwidget/tests.rs | 83 ++++++++++++++++++- codex-rs/tui/src/onboarding/mod.rs | 1 + .../tui_app_server/src/chatwidget/plugins.rs | 56 ++++++++++++- ...ests__plugin_detail_popup_installable.snap | 5 +- ..._tests__plugin_detail_popup_installed.snap | 15 ++++ .../tui_app_server/src/chatwidget/tests.rs | 83 ++++++++++++++++++- codex-rs/tui_app_server/src/onboarding/mod.rs | 1 + 10 files changed, 308 insertions(+), 12 deletions(-) create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugin_detail_popup_installed.snap create mode 100644 codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__plugin_detail_popup_installed.snap diff --git a/codex-rs/tui/src/chatwidget/plugins.rs b/codex-rs/tui/src/chatwidget/plugins.rs index e44081f761..ce08cc8791 100644 --- a/codex-rs/tui/src/chatwidget/plugins.rs +++ b/codex-rs/tui/src/chatwidget/plugins.rs @@ -8,6 +8,7 @@ use crate::bottom_pane::ColumnWidthMode; use crate::bottom_pane::SelectionItem; use crate::bottom_pane::SelectionViewParams; use crate::history_cell; +use crate::onboarding::mark_url_hyperlink; use crate::render::renderable::ColumnRenderable; use crate::render::renderable::Renderable; use crate::shimmer::shimmer_spans; @@ -24,10 +25,12 @@ use codex_features::Feature; use codex_utils_absolute_path::AbsolutePathBuf; use ratatui::buffer::Buffer; use ratatui::layout::Rect; +use ratatui::prelude::Widget; use ratatui::style::Stylize; use ratatui::text::Line; use ratatui::widgets::Paragraph; use ratatui::widgets::WidgetRef; +use ratatui::widgets::Wrap; const PLUGINS_SELECTION_VIEW_ID: &str = "plugins-selection"; const LOADING_ANIMATION_DELAY: Duration = Duration::from_secs(1); @@ -93,6 +96,29 @@ impl Renderable for DelayedLoadingHeader { } } +const APPS_HELP_ARTICLE_URL: &str = "https://help.openai.com/en/articles/11487775-apps-in-chatgpt"; + +struct PluginDisclosureLine { + line: Line<'static>, +} + +impl Renderable for PluginDisclosureLine { + fn render(&self, area: Rect, buf: &mut Buffer) { + Paragraph::new(self.line.clone()) + .wrap(Wrap { trim: false }) + .render(area, buf); + mark_url_hyperlink(buf, area, APPS_HELP_ARTICLE_URL); + } + + fn desired_height(&self, width: u16) -> u16 { + Paragraph::new(self.line.clone()) + .wrap(Wrap { trim: false }) + .line_count(width) + .try_into() + .unwrap_or(u16::MAX) + } +} + #[derive(Debug, Clone, Default)] pub(super) enum PluginsCacheState { #[default] @@ -812,13 +838,37 @@ impl ChatWidget { ) -> SelectionViewParams { let marketplace_label = plugin.marketplace_name.clone(); let display_name = plugin_display_name(&plugin.summary); - let status_label = plugin_status_label(&plugin.summary); + let detail_status_label = if plugin.summary.installed { + if plugin.summary.enabled { + "Installed" + } else { + "Installed · Disabled" + } + } else { + match plugin.summary.install_policy { + PluginInstallPolicy::NotAvailable => "Not installable", + PluginInstallPolicy::Available => "Can be installed", + PluginInstallPolicy::InstalledByDefault => "Available by default", + } + }; let mut header = ColumnRenderable::new(); header.push(Line::from("Plugins".bold())); header.push(Line::from( - format!("{display_name} · {marketplace_label}").bold(), + format!("{display_name} · {detail_status_label} · {marketplace_label}").bold(), )); - header.push(Line::from(status_label.dim())); + if !plugin.summary.installed { + header.push(PluginDisclosureLine { + line: Line::from(vec![ + "Data shared with this app is subject to the app's ".into(), + "terms of service".bold(), + " and ".into(), + "privacy policy".bold(), + ". ".into(), + "Learn more".cyan().underlined(), + ".".into(), + ]), + }); + } if let Some(description) = plugin_detail_description(plugin) { header.push(Line::from(description.dim())); } diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugin_detail_popup_installable.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugin_detail_popup_installable.snap index 0388e3d161..b9e5683c46 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugin_detail_popup_installable.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugin_detail_popup_installable.snap @@ -3,8 +3,9 @@ source: tui/src/chatwidget/tests.rs expression: popup --- Plugins - Figma · ChatGPT Marketplace - Available + Figma · Can be installed · ChatGPT Marketplace + Data shared with this app is subject to the app's terms of service and privacy policy. Learn + more. Turn Figma files into implementation context. › 1. Back to plugins Return to the plugin list. diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugin_detail_popup_installed.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugin_detail_popup_installed.snap new file mode 100644 index 0000000000..71ae46d78d --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugin_detail_popup_installed.snap @@ -0,0 +1,15 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: popup +--- + Plugins + Figma · Installed · ChatGPT Marketplace + Turn Figma files into implementation context. + +› 1. Back to plugins Return to the plugin list. + 2. Uninstall plugin Remove this plugin now. + Skills design-review, extract-copy + Apps Figma, Slack + MCP Servers figma-mcp, docs-mcp + + Press esc to close. diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index fad59d5cbc..e1314e27f0 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -7083,6 +7083,40 @@ fn render_bottom_popup(chat: &ChatWidget, width: u16) -> String { lines.join("\n") } +fn strip_osc8_for_snapshot(text: &str) -> String { + // Snapshots should assert the visible popup text, not terminal hyperlink escapes. + let bytes = text.as_bytes(); + let mut stripped = String::with_capacity(text.len()); + let mut i = 0; + + while i < bytes.len() { + if bytes[i..].starts_with(b"\x1B]8;;") { + i += 5; + while i < bytes.len() { + if bytes[i] == b'\x07' { + i += 1; + break; + } + if i + 1 < bytes.len() && bytes[i] == b'\x1B' && bytes[i + 1] == b'\\' { + i += 2; + break; + } + i += 1; + } + continue; + } + + let ch = text[i..] + .chars() + .next() + .expect("slice should always contain a char"); + stripped.push(ch); + i += ch.len_utf8(); + } + + stripped +} + fn plugins_test_absolute_path(path: &str) -> AbsolutePathBuf { AbsolutePathBuf::try_from( std::env::temp_dir() @@ -7338,7 +7372,54 @@ async fn plugin_detail_popup_snapshot_shows_install_actions_and_capability_summa ); let popup = render_bottom_popup(&chat, 100); - assert_snapshot!("plugin_detail_popup_installable", popup); + assert_snapshot!( + "plugin_detail_popup_installable", + strip_osc8_for_snapshot(&popup) + ); +} + +#[tokio::test] +async fn plugin_detail_popup_hides_disclosure_for_installed_plugins() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + chat.set_feature_enabled(Feature::Plugins, true); + + let summary = plugins_test_summary( + "plugin-figma", + "figma", + Some("Figma"), + Some("Design handoff."), + true, + true, + PluginInstallPolicy::Available, + ); + let response = plugins_test_response(vec![plugins_test_curated_marketplace(vec![ + summary.clone(), + ])]); + let cwd = chat.config.cwd.clone(); + chat.on_plugins_loaded(cwd.clone(), Ok(response)); + chat.add_plugins_output(); + chat.on_plugin_detail_loaded( + cwd, + Ok(PluginReadResponse { + plugin: plugins_test_detail( + summary, + Some("Turn Figma files into implementation context."), + &["design-review", "extract-copy"], + &[("Figma", true), ("Slack", false)], + &["figma-mcp", "docs-mcp"], + ), + }), + ); + + let popup = render_bottom_popup(&chat, 100); + assert!( + !popup.contains("Data shared with this app is subject to the app's"), + "expected installed plugin details to hide the disclosure line, got:\n{popup}" + ); + assert_snapshot!( + "plugin_detail_popup_installed", + strip_osc8_for_snapshot(&popup) + ); } #[tokio::test] diff --git a/codex-rs/tui/src/onboarding/mod.rs b/codex-rs/tui/src/onboarding/mod.rs index d4cfd6d1f4..8a4dc760fa 100644 --- a/codex-rs/tui/src/onboarding/mod.rs +++ b/codex-rs/tui/src/onboarding/mod.rs @@ -1,5 +1,6 @@ mod auth; pub mod onboarding_screen; mod trust_directory; +pub(crate) use auth::mark_url_hyperlink; pub use trust_directory::TrustDirectorySelection; mod welcome; diff --git a/codex-rs/tui_app_server/src/chatwidget/plugins.rs b/codex-rs/tui_app_server/src/chatwidget/plugins.rs index e44081f761..ce08cc8791 100644 --- a/codex-rs/tui_app_server/src/chatwidget/plugins.rs +++ b/codex-rs/tui_app_server/src/chatwidget/plugins.rs @@ -8,6 +8,7 @@ use crate::bottom_pane::ColumnWidthMode; use crate::bottom_pane::SelectionItem; use crate::bottom_pane::SelectionViewParams; use crate::history_cell; +use crate::onboarding::mark_url_hyperlink; use crate::render::renderable::ColumnRenderable; use crate::render::renderable::Renderable; use crate::shimmer::shimmer_spans; @@ -24,10 +25,12 @@ use codex_features::Feature; use codex_utils_absolute_path::AbsolutePathBuf; use ratatui::buffer::Buffer; use ratatui::layout::Rect; +use ratatui::prelude::Widget; use ratatui::style::Stylize; use ratatui::text::Line; use ratatui::widgets::Paragraph; use ratatui::widgets::WidgetRef; +use ratatui::widgets::Wrap; const PLUGINS_SELECTION_VIEW_ID: &str = "plugins-selection"; const LOADING_ANIMATION_DELAY: Duration = Duration::from_secs(1); @@ -93,6 +96,29 @@ impl Renderable for DelayedLoadingHeader { } } +const APPS_HELP_ARTICLE_URL: &str = "https://help.openai.com/en/articles/11487775-apps-in-chatgpt"; + +struct PluginDisclosureLine { + line: Line<'static>, +} + +impl Renderable for PluginDisclosureLine { + fn render(&self, area: Rect, buf: &mut Buffer) { + Paragraph::new(self.line.clone()) + .wrap(Wrap { trim: false }) + .render(area, buf); + mark_url_hyperlink(buf, area, APPS_HELP_ARTICLE_URL); + } + + fn desired_height(&self, width: u16) -> u16 { + Paragraph::new(self.line.clone()) + .wrap(Wrap { trim: false }) + .line_count(width) + .try_into() + .unwrap_or(u16::MAX) + } +} + #[derive(Debug, Clone, Default)] pub(super) enum PluginsCacheState { #[default] @@ -812,13 +838,37 @@ impl ChatWidget { ) -> SelectionViewParams { let marketplace_label = plugin.marketplace_name.clone(); let display_name = plugin_display_name(&plugin.summary); - let status_label = plugin_status_label(&plugin.summary); + let detail_status_label = if plugin.summary.installed { + if plugin.summary.enabled { + "Installed" + } else { + "Installed · Disabled" + } + } else { + match plugin.summary.install_policy { + PluginInstallPolicy::NotAvailable => "Not installable", + PluginInstallPolicy::Available => "Can be installed", + PluginInstallPolicy::InstalledByDefault => "Available by default", + } + }; let mut header = ColumnRenderable::new(); header.push(Line::from("Plugins".bold())); header.push(Line::from( - format!("{display_name} · {marketplace_label}").bold(), + format!("{display_name} · {detail_status_label} · {marketplace_label}").bold(), )); - header.push(Line::from(status_label.dim())); + if !plugin.summary.installed { + header.push(PluginDisclosureLine { + line: Line::from(vec![ + "Data shared with this app is subject to the app's ".into(), + "terms of service".bold(), + " and ".into(), + "privacy policy".bold(), + ". ".into(), + "Learn more".cyan().underlined(), + ".".into(), + ]), + }); + } if let Some(description) = plugin_detail_description(plugin) { header.push(Line::from(description.dim())); } diff --git a/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__plugin_detail_popup_installable.snap b/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__plugin_detail_popup_installable.snap index 399f160799..487141a2ae 100644 --- a/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__plugin_detail_popup_installable.snap +++ b/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__plugin_detail_popup_installable.snap @@ -3,8 +3,9 @@ source: tui_app_server/src/chatwidget/tests.rs expression: popup --- Plugins - Figma · ChatGPT Marketplace - Available + Figma · Can be installed · ChatGPT Marketplace + Data shared with this app is subject to the app's terms of service and privacy policy. Learn + more. Turn Figma files into implementation context. › 1. Back to plugins Return to the plugin list. diff --git a/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__plugin_detail_popup_installed.snap b/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__plugin_detail_popup_installed.snap new file mode 100644 index 0000000000..651a323aa2 --- /dev/null +++ b/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__plugin_detail_popup_installed.snap @@ -0,0 +1,15 @@ +--- +source: tui_app_server/src/chatwidget/tests.rs +expression: popup +--- + Plugins + Figma · Installed · ChatGPT Marketplace + Turn Figma files into implementation context. + +› 1. Back to plugins Return to the plugin list. + 2. Uninstall plugin Remove this plugin now. + Skills design-review, extract-copy + Apps Figma, Slack + MCP Servers figma-mcp, docs-mcp + + Press esc to close. diff --git a/codex-rs/tui_app_server/src/chatwidget/tests.rs b/codex-rs/tui_app_server/src/chatwidget/tests.rs index a684806128..c3c2ef7692 100644 --- a/codex-rs/tui_app_server/src/chatwidget/tests.rs +++ b/codex-rs/tui_app_server/src/chatwidget/tests.rs @@ -7680,6 +7680,40 @@ fn render_bottom_popup(chat: &ChatWidget, width: u16) -> String { lines.join("\n") } +fn strip_osc8_for_snapshot(text: &str) -> String { + // Snapshots should assert the visible popup text, not terminal hyperlink escapes. + let bytes = text.as_bytes(); + let mut stripped = String::with_capacity(text.len()); + let mut i = 0; + + while i < bytes.len() { + if bytes[i..].starts_with(b"\x1B]8;;") { + i += 5; + while i < bytes.len() { + if bytes[i] == b'\x07' { + i += 1; + break; + } + if i + 1 < bytes.len() && bytes[i] == b'\x1B' && bytes[i + 1] == b'\\' { + i += 2; + break; + } + i += 1; + } + continue; + } + + let ch = text[i..] + .chars() + .next() + .expect("slice should always contain a char"); + stripped.push(ch); + i += ch.len_utf8(); + } + + stripped +} + fn plugins_test_absolute_path(path: &str) -> AbsolutePathBuf { AbsolutePathBuf::try_from( std::env::temp_dir() @@ -7935,7 +7969,54 @@ async fn plugin_detail_popup_snapshot_shows_install_actions_and_capability_summa ); let popup = render_bottom_popup(&chat, 100); - assert_snapshot!("plugin_detail_popup_installable", popup); + assert_snapshot!( + "plugin_detail_popup_installable", + strip_osc8_for_snapshot(&popup) + ); +} + +#[tokio::test] +async fn plugin_detail_popup_hides_disclosure_for_installed_plugins() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + chat.set_feature_enabled(Feature::Plugins, true); + + let summary = plugins_test_summary( + "plugin-figma", + "figma", + Some("Figma"), + Some("Design handoff."), + true, + true, + PluginInstallPolicy::Available, + ); + let response = plugins_test_response(vec![plugins_test_curated_marketplace(vec![ + summary.clone(), + ])]); + let cwd = chat.config.cwd.clone(); + chat.on_plugins_loaded(cwd.clone(), Ok(response)); + chat.add_plugins_output(); + chat.on_plugin_detail_loaded( + cwd, + Ok(PluginReadResponse { + plugin: plugins_test_detail( + summary, + Some("Turn Figma files into implementation context."), + &["design-review", "extract-copy"], + &[("Figma", true), ("Slack", false)], + &["figma-mcp", "docs-mcp"], + ), + }), + ); + + let popup = render_bottom_popup(&chat, 100); + assert!( + !popup.contains("Data shared with this app is subject to the app's"), + "expected installed plugin details to hide the disclosure line, got:\n{popup}" + ); + assert_snapshot!( + "plugin_detail_popup_installed", + strip_osc8_for_snapshot(&popup) + ); } #[tokio::test] diff --git a/codex-rs/tui_app_server/src/onboarding/mod.rs b/codex-rs/tui_app_server/src/onboarding/mod.rs index d4cfd6d1f4..8a4dc760fa 100644 --- a/codex-rs/tui_app_server/src/onboarding/mod.rs +++ b/codex-rs/tui_app_server/src/onboarding/mod.rs @@ -1,5 +1,6 @@ mod auth; pub mod onboarding_screen; mod trust_directory; +pub(crate) use auth::mark_url_hyperlink; pub use trust_directory::TrustDirectorySelection; mod welcome;