mirror of
https://github.com/openai/codex.git
synced 2026-10-29 19:08:43 +00:00
app-server: preserve command approval cwd
This commit is contained in:
+1
-1
@@ -547,7 +547,7 @@
|
||||
"cwd": {
|
||||
"anyOf": [
|
||||
{
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/ApiPathString"
|
||||
},
|
||||
{
|
||||
"type": "null"
|
||||
|
||||
+1
-1
@@ -374,7 +374,7 @@
|
||||
"cwd": {
|
||||
"anyOf": [
|
||||
{
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/ApiPathString"
|
||||
},
|
||||
{
|
||||
"type": "null"
|
||||
|
||||
+1
-1
@@ -2378,7 +2378,7 @@
|
||||
"cwd": {
|
||||
"anyOf": [
|
||||
{
|
||||
"$ref": "#/definitions/v2/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/v2/ApiPathString"
|
||||
},
|
||||
{
|
||||
"type": "null"
|
||||
|
||||
Generated
+2
-2
@@ -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<CommandAction> | null, /**
|
||||
|
||||
@@ -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<AbsolutePathBuf>,
|
||||
pub cwd: Option<ApiPathString>,
|
||||
/// Best-effort parsed command actions for friendly display.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional = nullable)]
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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),
|
||||
),
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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>() -> T
|
||||
where
|
||||
T: DeserializeOwned,
|
||||
|
||||
Reference in New Issue
Block a user