From bd47c95c4c0f908c5c2594a20d74091ac3b8f40d Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sun, 14 Jun 2026 06:34:35 +0000 Subject: [PATCH] app-server: preserve command approval cwd --- .../CommandExecutionRequestApprovalParams.json | 2 +- .../schema/json/ServerRequest.json | 2 +- .../codex_app_server_protocol.schemas.json | 2 +- .../CommandExecutionRequestApprovalParams.ts | 4 ++-- .../src/protocol/v2/item.rs | 3 ++- codex-rs/app-server-test-client/src/lib.rs | 2 +- .../app-server/src/bespoke_event_handling.rs | 18 ++++++++++++------ codex-rs/app-server/src/transport_tests.rs | 10 ++++++++-- .../tui/src/app/pending_interactive_replay.rs | 5 ++--- codex-rs/tui/src/app/tests.rs | 3 ++- codex-rs/tui/src/app/thread_events.rs | 3 ++- .../tui/src/app_server_approval_conversions.rs | 16 ++++++++++++++++ codex-rs/tui/src/chatwidget.rs | 4 +++- codex-rs/tui/src/chatwidget/tests.rs | 1 + .../src/chatwidget/tests/approval_requests.rs | 4 ++-- codex-rs/tui/src/chatwidget/tests/exec_flow.rs | 2 +- codex-rs/tui/src/test_support.rs | 7 +++++++ 17 files changed, 64 insertions(+), 24 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json index c59141103c..80380f858f 100644 --- a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json @@ -547,7 +547,7 @@ "cwd": { "anyOf": [ { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/ApiPathString" }, { "type": "null" diff --git a/codex-rs/app-server-protocol/schema/json/ServerRequest.json b/codex-rs/app-server-protocol/schema/json/ServerRequest.json index 34ccaa47e9..76368529fe 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ServerRequest.json @@ -374,7 +374,7 @@ "cwd": { "anyOf": [ { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/ApiPathString" }, { "type": "null" 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 719abb3ce3..3739e0cb93 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 @@ -2378,7 +2378,7 @@ "cwd": { "anyOf": [ { - "$ref": "#/definitions/v2/AbsolutePathBuf" + "$ref": "#/definitions/v2/ApiPathString" }, { "type": "null" diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts index 0e9100836a..8d0c329eb1 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts @@ -1,7 +1,7 @@ // 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"; +import type { ApiPathString } from "../ApiPathString"; import type { CommandAction } from "./CommandAction"; import type { ExecPolicyAmendment } from "./ExecPolicyAmendment"; import type { NetworkApprovalContext } from "./NetworkApprovalContext"; @@ -31,7 +31,7 @@ networkApprovalContext?: NetworkApprovalContext | null, /** command?: string | null, /** * The command's working directory. */ -cwd?: AbsolutePathBuf | null, /** +cwd?: ApiPathString | null, /** * Best-effort parsed command actions for friendly display. */ commandActions?: Array | null, /** diff --git a/codex-rs/app-server-protocol/src/protocol/v2/item.rs b/codex-rs/app-server-protocol/src/protocol/v2/item.rs index 502eb62e10..721900c133 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/item.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/item.rs @@ -31,6 +31,7 @@ use codex_protocol::protocol::PatchApplyStatus as CorePatchApplyStatus; use codex_protocol::protocol::ReviewDecision as CoreReviewDecision; use codex_protocol::protocol::SubAgentActivityKind as CoreSubAgentActivityKind; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::ApiPathString; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; @@ -1336,7 +1337,7 @@ pub struct CommandExecutionRequestApprovalParams { /// The command's working directory. #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional = nullable)] - pub cwd: Option, + pub cwd: Option, /// Best-effort parsed command actions for friendly display. #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional = nullable)] diff --git a/codex-rs/app-server-test-client/src/lib.rs b/codex-rs/app-server-test-client/src/lib.rs index ebe49758d3..20f64fda3e 100644 --- a/codex-rs/app-server-test-client/src/lib.rs +++ b/codex-rs/app-server-test-client/src/lib.rs @@ -1987,7 +1987,7 @@ impl CodexClient { println!("< command: {command}"); } if let Some(cwd) = cwd.as_ref() { - println!("< cwd: {}", cwd.display()); + println!("< cwd: {cwd}"); } if let Some(command_actions) = command_actions.as_ref() && !command_actions.is_empty() diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index b4eb2b10c3..4fa8dd9762 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -565,13 +565,19 @@ pub(crate) async fn apply_bespoke_event_handling( } = ev; #[expect( clippy::expect_used, - reason = "legacy command approval payloads still require a host-native cwd" + reason = "command approval paths are absolute and have an inferred convention" )] - let cwd = { - let cwd = ApiPathString::from_path_uri(&cwd, PathConvention::native()) + let (api_cwd, cwd) = { + let convention = cwd + .infer_path_convention() + .expect("command cwd should have a native path convention"); + let api_cwd = ApiPathString::from_path_uri(&cwd, convention) + .expect("command cwd should render using its native path convention"); + let host_cwd = ApiPathString::from_path_uri(&cwd, PathConvention::native()) .expect("command cwd should render using the app-server host path convention"); - AbsolutePathBuf::try_from(cwd.into_string()) - .expect("projected command cwd should be absolute on the app-server host") + let host_cwd = AbsolutePathBuf::try_from(host_cwd.into_string()) + .expect("projected command cwd should be absolute on the app-server host"); + (api_cwd, host_cwd) }; let command_actions = parsed_cmd .iter() @@ -599,7 +605,7 @@ pub(crate) async fn apply_bespoke_event_handling( CommandExecutionApprovalPresentation::Command(completion_item) => ( None, Some(completion_item.command.clone()), - Some(completion_item.cwd.clone()), + Some(api_cwd), Some(completion_item.command_actions.clone()), Some(completion_item), ), diff --git a/codex-rs/app-server/src/transport_tests.rs b/codex-rs/app-server/src/transport_tests.rs index 4b9b387aaa..a4922b24b3 100644 --- a/codex-rs/app-server/src/transport_tests.rs +++ b/codex-rs/app-server/src/transport_tests.rs @@ -5,6 +5,8 @@ use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ThreadRealtimeStartedNotification; use codex_protocol::protocol::RealtimeConversationVersion; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::ApiPathString; +use codex_utils_path_uri::PathConvention; use pretty_assertions::assert_eq; use serde_json::json; use tokio::time::Duration; @@ -14,6 +16,10 @@ fn absolute_path(path: &str) -> AbsolutePathBuf { AbsolutePathBuf::from_absolute_path(path).expect("absolute path") } +fn api_path(path: &str) -> ApiPathString { + ApiPathString::from_abs_path(&absolute_path(path), PathConvention::native()).expect("API path") +} + fn thread_realtime_started_notification() -> ServerNotification { ServerNotification::ThreadRealtimeStarted(ThreadRealtimeStartedNotification { thread_id: "thread-1".to_string(), @@ -253,7 +259,7 @@ async fn command_execution_request_approval_strips_additional_permissions_withou reason: Some("Need extra read access".to_string()), network_approval_context: None, command: Some("cat file".to_string()), - cwd: Some(absolute_path("/tmp")), + cwd: Some(api_path("/tmp")), command_actions: None, additional_permissions: Some( codex_app_server_protocol::AdditionalPermissionProfile { @@ -318,7 +324,7 @@ async fn command_execution_request_approval_keeps_additional_permissions_with_ca reason: Some("Need extra read access".to_string()), network_approval_context: None, command: Some("cat file".to_string()), - cwd: Some(absolute_path("/tmp")), + cwd: Some(api_path("/tmp")), command_actions: None, additional_permissions: Some( codex_app_server_protocol::AdditionalPermissionProfile { diff --git a/codex-rs/tui/src/app/pending_interactive_replay.rs b/codex-rs/tui/src/app/pending_interactive_replay.rs index 4f90dbe9cf..5fbb9326ad 100644 --- a/codex-rs/tui/src/app/pending_interactive_replay.rs +++ b/codex-rs/tui/src/app/pending_interactive_replay.rs @@ -565,6 +565,7 @@ mod tests { use super::super::ThreadBufferedEvent; use super::super::ThreadEventStore; use crate::app_command::AppCommand as Op; + use crate::test_support::test_api_path; use codex_app_server_protocol::CommandExecutionApprovalDecision; use codex_app_server_protocol::CommandExecutionRequestApprovalParams; use codex_app_server_protocol::FileChangeRequestApprovalParams; @@ -583,8 +584,6 @@ mod tests { use codex_app_server_protocol::Turn; use codex_app_server_protocol::TurnCompletedNotification; use codex_app_server_protocol::TurnStatus; - use codex_utils_absolute_path::test_support::PathBufExt; - use codex_utils_absolute_path::test_support::test_path_buf; use pretty_assertions::assert_eq; use std::collections::BTreeMap; use std::collections::HashMap; @@ -618,7 +617,7 @@ mod tests { reason: None, network_approval_context: None, command: Some("echo hi".to_string()), - cwd: Some(test_path_buf("/tmp").abs()), + cwd: Some(test_api_path("/tmp")), command_actions: None, additional_permissions: None, proposed_execpolicy_amendment: None, diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index c3c83e6d42..0373a1a013 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -25,6 +25,7 @@ use crate::history_cell::UserHistoryCell; use crate::history_cell::new_session_info; use crate::multi_agents::AgentPickerThreadEntry; use crate::multi_agents::SubAgentActivityDisplay; +use crate::test_support::test_api_path; use assert_matches::assert_matches; use crate::app_command::AppCommand as Op; @@ -4693,7 +4694,7 @@ fn exec_approval_request( reason: Some("needs approval".to_string()), network_approval_context: None, command: Some("echo hello".to_string()), - cwd: Some(test_path_buf("/tmp/project").abs()), + cwd: Some(test_api_path("/tmp/project")), command_actions: None, additional_permissions: None, proposed_execpolicy_amendment: None, diff --git a/codex-rs/tui/src/app/thread_events.rs b/codex-rs/tui/src/app/thread_events.rs index 48dc1d0848..44cf29c6ae 100644 --- a/codex-rs/tui/src/app/thread_events.rs +++ b/codex-rs/tui/src/app/thread_events.rs @@ -335,6 +335,7 @@ impl ThreadEventChannel { mod tests { use super::*; use crate::test_support::PathBufExt; + use crate::test_support::test_api_path; use crate::test_support::test_path_buf; use codex_app_server_protocol::AskForApproval; use codex_app_server_protocol::CommandExecutionRequestApprovalParams; @@ -491,7 +492,7 @@ mod tests { reason: Some("needs approval".to_string()), network_approval_context: None, command: Some("echo hello".to_string()), - cwd: Some(test_path_buf("/tmp/project").abs()), + cwd: Some(test_api_path("/tmp/project")), command_actions: None, additional_permissions: None, proposed_execpolicy_amendment: None, diff --git a/codex-rs/tui/src/app_server_approval_conversions.rs b/codex-rs/tui/src/app_server_approval_conversions.rs index 2208bcabbf..15c8060e30 100644 --- a/codex-rs/tui/src/app_server_approval_conversions.rs +++ b/codex-rs/tui/src/app_server_approval_conversions.rs @@ -11,6 +11,9 @@ use codex_app_server_protocol::FileUpdateChange; use codex_app_server_protocol::GrantedPermissionProfile; use codex_app_server_protocol::PatchChangeKind; use codex_protocol::request_permissions::RequestPermissionProfile as CoreRequestPermissionProfile; +use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::ApiPathString; +use codex_utils_path_uri::PathConvention; use std::collections::HashMap; use std::path::PathBuf; @@ -49,6 +52,19 @@ pub(crate) fn file_update_changes_to_display( .collect() } +pub(crate) fn command_approval_cwd_to_host( + cwd: Option<&ApiPathString>, + fallback_cwd: &AbsolutePathBuf, +) -> AbsolutePathBuf { + cwd.and_then(|cwd| { + let convention = cwd.infer_absolute_path_convention()?; + let cwd = cwd.to_path_uri(convention).ok()?; + let cwd = ApiPathString::from_path_uri(&cwd, PathConvention::native()).ok()?; + AbsolutePathBuf::try_from(cwd.into_string()).ok() + }) + .unwrap_or_else(|| fallback_cwd.clone()) +} + #[cfg(test)] mod tests { use super::file_update_changes_to_display; diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 80be785553..611ccdaf55 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -44,6 +44,7 @@ use std::time::Instant; use crate::app::app_server_requests::ResolvedAppServerRequest; use crate::app_command::AppCommand; use crate::app_event::HistoryLookupResponse; +use crate::app_server_approval_conversions::command_approval_cwd_to_host; use crate::app_server_approval_conversions::file_update_changes_to_display; use crate::approval_events::ApplyPatchApprovalRequestEvent; use crate::approval_events::ExecApprovalRequestEvent; @@ -832,6 +833,7 @@ fn exec_approval_request_from_params( params: CommandExecutionRequestApprovalParams, fallback_cwd: &AbsolutePathBuf, ) -> ExecApprovalRequestEvent { + let cwd = command_approval_cwd_to_host(params.cwd.as_ref(), fallback_cwd); ExecApprovalRequestEvent { call_id: params.item_id, command: params @@ -839,7 +841,7 @@ fn exec_approval_request_from_params( .as_deref() .map(split_command_string) .unwrap_or_default(), - cwd: params.cwd.unwrap_or_else(|| fallback_cwd.clone()), + cwd, reason: params.reason, network_approval_context: params.network_approval_context, additional_permissions: params.additional_permissions, diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index a34d393842..4bd16a1625 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -21,6 +21,7 @@ pub(super) use crate::legacy_core::config::ConfigBuilder; pub(super) use crate::model_catalog::ModelCatalog; pub(super) use crate::test_backend::VT100Backend; pub(super) use crate::test_support::PathBufExt; +pub(super) use crate::test_support::test_api_path; pub(super) use crate::test_support::test_path_buf; pub(super) use crate::test_support::test_path_display; pub(super) use crate::token_usage::TokenUsage; diff --git a/codex-rs/tui/src/chatwidget/tests/approval_requests.rs b/codex-rs/tui/src/chatwidget/tests/approval_requests.rs index 69d75ddc67..91c66b2626 100644 --- a/codex-rs/tui/src/chatwidget/tests/approval_requests.rs +++ b/codex-rs/tui/src/chatwidget/tests/approval_requests.rs @@ -63,7 +63,7 @@ fn app_server_exec_approval_request_splits_shell_wrapped_command() { shlex::try_join(["/bin/zsh", "-lc", script]) .expect("round-trippable shell wrapper"), ), - cwd: Some(test_path_buf("/tmp").abs()), + cwd: Some(test_api_path("/tmp")), command_actions: None, additional_permissions: None, proposed_execpolicy_amendment: None, @@ -104,7 +104,7 @@ fn app_server_exec_approval_request_preserves_permissions_context() { protocol: codex_app_server_protocol::NetworkApprovalProtocol::Socks5Tcp, }), command: Some("ls".to_string()), - cwd: Some(test_path_buf("/tmp").abs()), + cwd: Some(test_api_path("/tmp")), command_actions: None, additional_permissions: Some(AppServerAdditionalPermissionProfile { network: Some(AppServerAdditionalNetworkPermissions { diff --git a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs index ca1c9bc681..8233f6dac4 100644 --- a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs +++ b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs @@ -62,7 +62,7 @@ fn app_server_exec_approval_request_splits_shell_wrapped_command() { shlex::try_join(["/bin/zsh", "-lc", script]) .expect("round-trippable shell wrapper"), ), - cwd: Some(test_path_buf("/tmp").abs()), + cwd: Some(test_api_path("/tmp")), command_actions: None, additional_permissions: None, proposed_execpolicy_amendment: None, diff --git a/codex-rs/tui/src/test_support.rs b/codex-rs/tui/src/test_support.rs index 44e9899379..e82a78ee2e 100644 --- a/codex-rs/tui/src/test_support.rs +++ b/codex-rs/tui/src/test_support.rs @@ -6,6 +6,8 @@ use codex_models_manager::bundled_models_response; use codex_protocol::openai_models::ModelPreset; pub(crate) use codex_utils_absolute_path::test_support::PathBufExt; pub(crate) use codex_utils_absolute_path::test_support::test_path_buf; +use codex_utils_path_uri::ApiPathString; +use codex_utils_path_uri::PathConvention; use serde::Serialize; use serde::de::DeserializeOwned; @@ -22,6 +24,11 @@ pub(crate) fn test_path_display(path: &str) -> String { test_path_buf(path).display().to_string() } +pub(crate) fn test_api_path(path: &str) -> ApiPathString { + ApiPathString::from_abs_path(&test_path_buf(path).abs(), PathConvention::native()) + .expect("API path") +} + pub(crate) fn session_source_cli() -> T where T: DeserializeOwned,