From a01a2d91461a57809e944de7758477b92617ab01 Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Thu, 30 Jul 2026 20:55:13 +0000 Subject: [PATCH] Preserve executor paths in read command actions (#36223) ## Why Read command actions were omitted when a selected environment used a path convention foreign to the app-server host. Clients need these actions to refer to files in the executor's filesystem, not paths interpreted on the host. ## What changed - Resolve read paths against the executor's `PathUri` and serialize them using the executor's native POSIX, Windows drive, or UNC convention. - Accept same-drive Windows-relative paths while continuing to reject paths that require another drive's executor-local current directory. - Document the path convention used by `commandExecution` notifications. ## Testing Cover native and cross-platform path serialization, Windows drive-relative joins, and selected-environment `item/started` and `item/completed` notifications. GitOrigin-RevId: 3c5a9bb1caafd35e04a0a0cceab91fee590372b9 --- .../analytics/src/analytics_client_tests.rs | 2 +- ...CommandExecutionRequestApprovalParams.json | 6 +- .../schema/json/ServerNotification.json | 2 +- .../schema/json/ServerRequest.json | 2 +- .../codex_app_server_protocol.schemas.json | 2 +- .../codex_app_server_protocol.v2.schemas.json | 2 +- .../json/v2/ItemCompletedNotification.json | 2 +- .../json/v2/ItemStartedNotification.json | 2 +- .../schema/json/v2/ReviewStartResponse.json | 2 +- .../schema/json/v2/ThreadForkResponse.json | 2 +- .../schema/json/v2/ThreadListResponse.json | 2 +- .../json/v2/ThreadMetadataUpdateResponse.json | 2 +- .../schema/json/v2/ThreadReadResponse.json | 2 +- .../schema/json/v2/ThreadResumeResponse.json | 2 +- .../json/v2/ThreadRollbackResponse.json | 2 +- .../schema/json/v2/ThreadStartResponse.json | 2 +- .../json/v2/ThreadStartedNotification.json | 2 +- .../json/v2/ThreadUnarchiveResponse.json | 2 +- .../json/v2/TurnCompletedNotification.json | 2 +- .../schema/json/v2/TurnStartResponse.json | 2 +- .../json/v2/TurnStartedNotification.json | 2 +- .../schema/typescript/v2/CommandAction.ts | 4 +- .../src/protocol/item_builders.rs | 44 ++++--- .../src/protocol/item_builders_tests.rs | 94 ++++++++++----- .../src/protocol/v2/item.rs | 6 +- codex-rs/app-server/README.md | 1 + .../tests/suite/v2/selected_environment.rs | 109 ++++++++++++++++++ codex-rs/utils/path-uri/src/lib.rs | 30 +++-- codex-rs/utils/path-uri/src/tests.rs | 17 ++- 29 files changed, 259 insertions(+), 92 deletions(-) diff --git a/codex-rs/analytics/src/analytics_client_tests.rs b/codex-rs/analytics/src/analytics_client_tests.rs index 5314d1c9ef..e7e8c7cdf4 100644 --- a/codex-rs/analytics/src/analytics_client_tests.rs +++ b/codex-rs/analytics/src/analytics_client_tests.rs @@ -2449,7 +2449,7 @@ async fn item_lifecycle_notifications_publish_command_execution_event() { CommandAction::Read { command: "cat README.md".to_string(), name: "README.md".to_string(), - path: test_path_buf("/tmp/README.md").abs(), + path: test_path_buf("/tmp/README.md").abs().into(), }, CommandAction::ListFiles { command: "ls".to_string(), diff --git a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json index da480a296e..fd87c4fb32 100644 --- a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json @@ -1,10 +1,6 @@ { "$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": { "entries": { @@ -95,7 +91,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 0d96710c28..4e1e7e86b1 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -798,7 +798,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/ServerRequest.json b/codex-rs/app-server-protocol/schema/json/ServerRequest.json index 971dde2498..3397e2178f 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ServerRequest.json @@ -164,7 +164,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ 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 10aea90a00..ea8503f064 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 @@ -7649,7 +7649,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/v2/AbsolutePathBuf" + "$ref": "#/definitions/v2/LegacyAppPathString" }, "type": { "enum": [ 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 fb3811047e..f076266a40 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 @@ -3827,7 +3827,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ItemCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/ItemCompletedNotification.json index b80ab71aae..b919faceeb 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ItemCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ItemCompletedNotification.json @@ -82,7 +82,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ItemStartedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/ItemStartedNotification.json index 70f46ed222..4cb0c6f977 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ItemStartedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ItemStartedNotification.json @@ -82,7 +82,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ReviewStartResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ReviewStartResponse.json index 0169150eee..fb2a4df87a 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ReviewStartResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ReviewStartResponse.json @@ -220,7 +220,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json index beb11fc042..15463263a8 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json @@ -301,7 +301,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadListResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadListResponse.json index 020258690f..dbf07dbb3d 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadListResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadListResponse.json @@ -223,7 +223,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadMetadataUpdateResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadMetadataUpdateResponse.json index d759eab1ce..ecf78e336d 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadMetadataUpdateResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadMetadataUpdateResponse.json @@ -223,7 +223,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadReadResponse.json index 17ebd74b14..4a417d5181 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadReadResponse.json @@ -223,7 +223,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json index 16bcc27f91..e188cccd91 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json @@ -301,7 +301,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadRollbackResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadRollbackResponse.json index 56ca9d6091..0a528fea69 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadRollbackResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadRollbackResponse.json @@ -223,7 +223,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json index a8f9f373d7..c7558cdc5e 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json @@ -301,7 +301,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartedNotification.json index bea9304afc..6c208bd937 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartedNotification.json @@ -223,7 +223,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadUnarchiveResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadUnarchiveResponse.json index b693425bdb..ade6fa80b7 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadUnarchiveResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadUnarchiveResponse.json @@ -223,7 +223,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/TurnCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/TurnCompletedNotification.json index 5c08f256c7..f870af56c0 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/TurnCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/TurnCompletedNotification.json @@ -220,7 +220,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/TurnStartResponse.json b/codex-rs/app-server-protocol/schema/json/v2/TurnStartResponse.json index 6ec43412ef..717375a6ac 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/TurnStartResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/TurnStartResponse.json @@ -220,7 +220,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/TurnStartedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/TurnStartedNotification.json index 18f79d4406..daa5b22462 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/TurnStartedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/TurnStartedNotification.json @@ -220,7 +220,7 @@ "type": "string" }, "path": { - "$ref": "#/definitions/AbsolutePathBuf" + "$ref": "#/definitions/LegacyAppPathString" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/CommandAction.ts b/codex-rs/app-server-protocol/schema/typescript/v2/CommandAction.ts index a17fb06a0c..91c92c9cbf 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/CommandAction.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CommandAction.ts @@ -1,6 +1,6 @@ // 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 { LegacyAppPathString } from "../LegacyAppPathString"; -export type CommandAction = { "type": "read", command: string, name: string, path: AbsolutePathBuf, } | { "type": "listFiles", command: string, path: string | null, } | { "type": "search", command: string, query: string | null, path: string | null, } | { "type": "unknown", command: string, }; +export type CommandAction = { "type": "read", command: string, name: string, path: LegacyAppPathString, } | { "type": "listFiles", command: string, path: string | null, } | { "type": "search", command: string, query: string | null, path: string | null, } | { "type": "unknown", command: string, }; diff --git a/codex-rs/app-server-protocol/src/protocol/item_builders.rs b/codex-rs/app-server-protocol/src/protocol/item_builders.rs index 5b5b9d2cfa..0887cb89f7 100644 --- a/codex-rs/app-server-protocol/src/protocol/item_builders.rs +++ b/codex-rs/app-server-protocol/src/protocol/item_builders.rs @@ -37,7 +37,6 @@ use codex_protocol::review_format::REVIEW_FALLBACK_MESSAGE; use codex_protocol::review_format::render_review_output_text; use codex_shell_command::parse_command::parse_command; use codex_shell_command::parse_command::shlex_join; -use codex_utils_path_uri::PathConvention; use codex_utils_path_uri::PathUri; use std::collections::HashMap; use std::path::PathBuf; @@ -122,33 +121,32 @@ pub(crate) fn command_actions_for_path_uri( parsed_cmd: &[ParsedCommand], cwd: &PathUri, ) -> Vec { - // TODO(anp): Carry PathUri into CommandAction so foreign Read actions retain resolved paths. - // Until then, omit those actions rather than project a foreign cwd onto the host. - let native_cwd = if cwd.infer_path_convention() == Some(PathConvention::native()) { - cwd.to_abs_path().ok() - } else { - None - }; - parsed_cmd .iter() .cloned() .filter_map(|parsed| match parsed { - ParsedCommand::Read { cmd, name, path } => match native_cwd.as_ref() { - Some(native_cwd) => Some(CommandAction::Read { - command: cmd, - name, - path: native_cwd.join(path), - }), - None => { - warn!( - command = cmd, - %cwd, - "omitting read command action whose path cannot be resolved against a foreign cwd" - ); - None + ParsedCommand::Read { cmd, name, path } => { + // Resolve against the executor's URI, not the app-server's filesystem. POSIX + // non-UTF-8 paths are percent-encoded, not opaque. Expanding `~` or resolving a + // genuinely opaque cwd would require executor-native state unavailable here. + match cwd.join(path.to_string_lossy().as_ref()) { + Ok(path) => Some(CommandAction::Read { + command: cmd, + name, + path: path.into(), + }), + Err(error) => { + warn!( + command = cmd, + %cwd, + file_path = %path.display(), + %error, + "omitting read action: invalid file path or cwd" + ); + None + } } - }, + } ParsedCommand::ListFiles { cmd, path } => { Some(CommandAction::ListFiles { command: cmd, path }) } diff --git a/codex-rs/app-server-protocol/src/protocol/item_builders_tests.rs b/codex-rs/app-server-protocol/src/protocol/item_builders_tests.rs index b892fcf505..d87427a78f 100644 --- a/codex-rs/app-server-protocol/src/protocol/item_builders_tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/item_builders_tests.rs @@ -1,41 +1,73 @@ use super::*; use pretty_assertions::assert_eq; +use serde_json::json; #[test] -fn foreign_read_is_omitted_without_dropping_other_command_actions() { - #[cfg(windows)] - let cwd = PathUri::parse("file:///usr/local/src").expect("valid foreign POSIX cwd"); - #[cfg(not(windows))] - let cwd = PathUri::parse("file:///C:/src").expect("valid foreign Windows cwd"); - let parsed_cmd = vec![ - ParsedCommand::Read { - cmd: "cat file.txt".to_string(), - name: "file.txt".to_string(), - path: PathBuf::from("file.txt"), - }, - ParsedCommand::ListFiles { - cmd: "ls".to_string(), - path: Some("subdir".to_string()), - }, - ParsedCommand::Search { - cmd: "rg needle".to_string(), - query: Some("needle".to_string()), - path: Some("src".to_string()), - }, - ]; - - assert_eq!( - command_actions_for_path_uri(&parsed_cmd, &cwd), - vec![ - CommandAction::ListFiles { - command: "ls".to_string(), +fn read_command_actions_preserve_native_and_foreign_paths() { + for (cwd_uri, relative_path, expected_path) in [ + ( + "file:///home/alice/repo", + "src/main.rs", + "/home/alice/repo/src/main.rs", + ), + ( + "file:///C:/Users/Alice%20Smith/repo", + r"src\main.rs", + r"C:\Users\Alice Smith\repo\src\main.rs", + ), + ( + "file:///C:/Users/Alice%20Smith/repo", + r"C:src\main.rs", + r"C:\Users\Alice Smith\repo\src\main.rs", + ), + ( + "file://server/share/repo", + r"src\main.rs", + r"\\server\share\repo\src\main.rs", + ), + ] { + let cwd = PathUri::parse(cwd_uri).expect("valid cross-platform cwd"); + let command = format!("cat {relative_path}"); + let parsed_cmd = vec![ + ParsedCommand::Read { + cmd: command.clone(), + name: "main.rs".to_string(), + path: PathBuf::from(relative_path), + }, + ParsedCommand::ListFiles { + cmd: "ls".to_string(), path: Some("subdir".to_string()), }, - CommandAction::Search { - command: "rg needle".to_string(), + ParsedCommand::Search { + cmd: "rg needle".to_string(), query: Some("needle".to_string()), path: Some("src".to_string()), }, - ] - ); + ]; + + assert_eq!( + serde_json::to_value(command_actions_for_path_uri(&parsed_cmd, &cwd)) + .expect("command actions should serialize"), + json!([ + { + "type": "read", + "command": command, + "name": "main.rs", + "path": expected_path, + }, + { + "type": "listFiles", + "command": "ls", + "path": "subdir", + }, + { + "type": "search", + "command": "rg needle", + "query": "needle", + "path": "src", + }, + ]), + "resolving command actions against {cwd_uri}", + ); + } } 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 1099f710ae..c25209c21b 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/item.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/item.rs @@ -122,7 +122,7 @@ pub enum CommandAction { Read { command: String, name: String, - path: AbsolutePathBuf, + path: LegacyAppPathString, }, ListFiles { command: String, @@ -186,7 +186,7 @@ impl CommandAction { } => CoreParsedCommand::Read { cmd, name, - path: path.into_path_buf(), + path: PathBuf::from(path.into_string()), }, CommandAction::ListFiles { command: cmd, path } => { CoreParsedCommand::ListFiles { cmd, path } @@ -205,7 +205,7 @@ impl CommandAction { CoreParsedCommand::Read { cmd, name, path } => CommandAction::Read { command: cmd, name, - path: cwd.join(path), + path: cwd.join(path).into(), }, CoreParsedCommand::ListFiles { cmd, path } => { CommandAction::ListFiles { command: cmd, path } diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index cf6d6fdf13..0980343753 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1502,6 +1502,7 @@ The app-server streams JSON-RPC notifications while a turn is running. Each turn - `plan` — `{id, text}` emitted for plan-mode turns; plan text can stream via `item/plan/delta` (experimental). - `reasoning` — `{id, summary, content}` where `summary` holds streamed reasoning summaries (applicable for most OpenAI models) and `content` holds raw reasoning blocks (applicable for e.g. open source models). - `commandExecution` — `{id, pluginId?, scriptPath?, command, cwd, status, commandActions, aggregatedOutput?, exitCode?, durationMs?}` for sandboxed commands; `pluginId` is present only for commands attributed to a trusted first-party plugin, newly attributed items also include `scriptPath` as a safe `/`-separated path relative to the trusted plugin root, older history may omit `scriptPath`, and `status` is `inProgress`, `completed`, `failed`, or `declined`. + `cwd` and read `commandActions[].path` use the executor's native path convention, even when the app-server runs on a different operating system. For example, an app-server running on Linux can return `C:\repo\src\main.rs` for a Windows executor; clients must not interpret that path as local to the app-server. - `fileChange` — `{id, changes, status}` describing proposed edits; `changes` list `{path, kind, diff}` and `status` is `inProgress`, `completed`, `failed`, or `declined`. - `mcpToolCall` — `{id, server, tool, status, arguments, appContext, mcpAppResourceUri?, pluginId, readOnlyHint, result?, error?}` describing MCP calls; `appContext` is `{connectorId, linkId, resourceUri, appName, actionName}` for calls through a trusted MCP app, where `connectorId` identifies the connector that owns the tool, `linkId` identifies the app link, `resourceUri` points to the widget template, `appName` is the connector's display name, and `actionName` is the stable connector `Action.name`. `readOnlyHint` is `true` for read-only tools, `false` for write-capable tools, and `null` when the annotation is unavailable, including older rollout entries. The hint describes tool capability, not whether an invocation succeeded or performed a write; use `status`, `result`, and `error` to determine the execution outcome. `appName` and `actionName` may be null for older rollout entries. The top-level `mcpAppResourceUri` is deprecated and temporarily duplicated for client migration. `tool` identifies the raw MCP tool. `status` is `inProgress`, `completed`, or `failed`. - `collabToolCall` — `{id, tool, status, senderThreadId, receiverThreadId?, newThreadId?, prompt?, agentStatus?}` describing collab tool calls (`spawn_agent`, `send_input`, `resume_agent`, `wait`, `close_agent`); `status` is `inProgress`, `completed`, or `failed`. diff --git a/codex-rs/app-server/tests/suite/v2/selected_environment.rs b/codex-rs/app-server/tests/suite/v2/selected_environment.rs index f7fb2c82eb..cf653f25ad 100644 --- a/codex-rs/app-server/tests/suite/v2/selected_environment.rs +++ b/codex-rs/app-server/tests/suite/v2/selected_environment.rs @@ -6,12 +6,18 @@ use anyhow::Result; use app_test_support::MockResponsesConfig; use app_test_support::PathBufExt; use app_test_support::TestAppServer; +use app_test_support::create_final_assistant_message_sse_response; +use app_test_support::create_mock_responses_server_sequence; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::UserInput as V2UserInput; +use codex_features::Feature; +use codex_shell_command::shell_detect::ShellType; +use codex_shell_command::shell_detect::detect_shell_type; use core_test_support::responses; use pretty_assertions::assert_eq; +use serde_json::json; use tempfile::TempDir; use tokio::time::timeout; @@ -206,3 +212,106 @@ async fn turn_model_context_uses_selected_environment() -> Result<()> { ); Ok(()) } + +#[tokio::test] +async fn command_execution_notifications_preserve_selected_environment_paths() -> Result<()> { + let command_arguments = serde_json::to_string(&json!({ + "cmd": "cat main.rs", + "yield_time_ms": 10_000, + }))?; + let server = create_mock_responses_server_sequence(vec![ + responses::sse(vec![ + responses::ev_response_created("resp-1"), + responses::ev_function_call( + "selected-environment-read", + "exec_command", + &command_arguments, + ), + responses::ev_completed("resp-1"), + ]), + create_final_assistant_message_sse_response("done")?, + ]) + .await; + let codex_home = TempDir::new()?; + MockResponsesConfig::new(&server.uri()) + .with_root_config("compact_prompt = \"compact\"\nmodel_auto_compact_token_limit = 100000") + .with_provider_config("supports_websockets = false") + .with_sandbox_mode("danger-full-access") + .enable_feature(Feature::UnifiedExec) + .write(codex_home.path())?; + let mut app_server = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .build_initialized() + .await?; + + let (expected_path, shell) = { + let environment = app_server.auto_env()?; + let path = environment.selection().cwd.join("main.rs")?; + environment + .environment() + .get_filesystem() + .write_file(&path, b"fn main() {}\n".to_vec(), /*sandbox*/ None) + .await?; + ( + path.inferred_native_path_string(), + environment.environment().info().await?.shell, + ) + }; + let thread = app_server + .start_thread(ThreadStartParams::default()) + .await? + .thread; + timeout( + DEFAULT_READ_TIMEOUT, + app_server.start_turn_and_wait_for_completion(text_turn_params(thread.id, "read main.rs")), + ) + .await??; + + let expected_actions = match shell.name.as_str() { + // Windows shell scripts are not yet parsed into file-read command actions. + "powershell" => { + let command = if detect_shell_type(&shell.path) == Some(ShellType::PowerShell) { + "cat main.rs".to_string() + } else { + shlex::try_join([shell.path.as_str(), "-Command", "cat main.rs"])? + }; + json!([{ + "type": "unknown", + "command": command, + }]) + } + "cmd" => { + let command = shlex::try_join([shell.path.as_str(), "/c", "cat main.rs"])?; + json!([{ + "type": "unknown", + "command": command, + }]) + } + _ => json!([{ + "type": "read", + "command": "cat main.rs", + "name": "main.rs", + "path": expected_path, + }]), + }; + + for method in ["item/started", "item/completed"] { + let notification = timeout( + DEFAULT_READ_TIMEOUT, + app_server.read_stream_until_matching_notification(method, |notification| { + notification.method == method + && notification + .params + .as_ref() + .is_some_and(|params| params["item"]["id"] == "selected-environment-read") + }), + ) + .await??; + let params = notification + .params + .context("command execution notification should include params")?; + assert_eq!(params["item"]["commandActions"], expected_actions); + } + + Ok(()) +} diff --git a/codex-rs/utils/path-uri/src/lib.rs b/codex-rs/utils/path-uri/src/lib.rs index 5e372cc2d1..e4bb02ce33 100644 --- a/codex-rs/utils/path-uri/src/lib.rs +++ b/codex-rs/utils/path-uri/src/lib.rs @@ -325,14 +325,17 @@ impl PathUri { /// Path text is interpreted using the POSIX or Windows convention inferred /// from the base URI. An absolute path replaces the base URI's path, while a /// relative path is appended lexically. Windows root-relative paths retain - /// the base drive or UNC share, while drive-relative paths are rejected. + /// the base drive or UNC share. Same-drive relative paths are appended to + /// the base, while other-drive relative paths are rejected because their + /// current directory belongs to the executor. /// Empty and `.` segments are ignored, while `..` removes one segment /// without escaping the POSIX root, Windows drive, or UNC share. Literal /// `%`, `?`, and `#` characters are percent-encoded as filename text. Paths /// containing a null character are rejected because they cannot be safely /// converted to native paths. /// Opaque fallback URIs created by [`Self::from_abs_path`] reject non-empty - /// joins. + /// joins. Home-directory expansion also requires executor-native context + /// and is intentionally not performed. pub fn join(&self, path: &str) -> Result { if path.contains('\0') { return Err(PathUriParseError::InvalidFileUriPath { @@ -353,13 +356,26 @@ impl PathUri { return Ok(absolute); } let path_bytes = path.as_bytes(); - if convention == PathConvention::Windows + let path = if convention == PathConvention::Windows && matches!(path_bytes, [drive, b':', ..] if drive.is_ascii_alphabetic()) { - return Err(PathUriParseError::InvalidFileUriPath { - path: path.to_string(), - }); - } + let same_drive = self + .0 + .path_segments() + .and_then(|mut segments| segments.find(|segment| !segment.is_empty())) + .is_some_and(|segment| { + matches!(segment.as_bytes(), [drive, b':'] if drive.eq_ignore_ascii_case(&path_bytes[0])) + }); + if !same_drive { + return Err(PathUriParseError::InvalidFileUriPath { + path: path.to_string(), + }); + } + &path[2..] + } else { + path + }; + let path_bytes = path.as_bytes(); if decode_bad_path_uri(&self.0).is_some() { return Err(PathUriParseError::InvalidFileUriPath { path: self.to_string(), diff --git a/codex-rs/utils/path-uri/src/tests.rs b/codex-rs/utils/path-uri/src/tests.rs index 333bd5dc09..941f434bc2 100644 --- a/codex-rs/utils/path-uri/src/tests.rs +++ b/codex-rs/utils/path-uri/src/tests.rs @@ -915,7 +915,22 @@ fn join_windows_root_relative_path_preserves_drive_or_share() { } #[test] -fn join_rejects_windows_drive_relative_path() { +fn join_resolves_windows_same_drive_relative_path() { + for (base, path, expected) in [ + ("file:///C:/base", r"C:tmp", "file:///C:/base/tmp"), + ("file:///C:/base", r"c:tmp", "file:///C:/base/tmp"), + ("file:///C:/base/dir", r"C:..\tmp", "file:///C:/base/tmp"), + ("file:///C:/base", "C:", "file:///C:/base"), + ] { + let base = PathUri::parse(base).expect("valid base URI"); + let expected = PathUri::parse(expected).expect("valid expected URI"); + + assert_eq!(base.join(path), Ok(expected), "joining {path}"); + } +} + +#[test] +fn join_rejects_windows_other_drive_relative_path() { let base = PathUri::parse("file:///C:/base").expect("valid base URI"); assert_eq!(