mirror of
https://github.com/openai/codex.git
synced 2026-10-29 19:08:43 +00:00
app-server: preserve foreign command action paths
This commit is contained in:
@@ -2142,7 +2142,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(),
|
||||
|
||||
+1
-5
@@ -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": [
|
||||
|
||||
@@ -779,7 +779,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
+1
-1
@@ -164,7 +164,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
+1
-1
@@ -7321,7 +7321,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/v2/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/v2/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
+1
-1
@@ -3561,7 +3561,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
+1
-1
@@ -82,7 +82,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
@@ -82,7 +82,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
@@ -220,7 +220,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
@@ -301,7 +301,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
@@ -223,7 +223,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
+1
-1
@@ -223,7 +223,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
@@ -223,7 +223,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
@@ -301,7 +301,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
@@ -223,7 +223,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
@@ -301,7 +301,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
+1
-1
@@ -223,7 +223,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
@@ -223,7 +223,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
+1
-1
@@ -220,7 +220,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
@@ -220,7 +220,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
@@ -220,7 +220,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"type": {
|
||||
"enum": [
|
||||
|
||||
@@ -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, };
|
||||
|
||||
@@ -35,7 +35,6 @@ use codex_protocol::protocol::PatchApplyBeginEvent;
|
||||
use codex_protocol::protocol::PatchApplyEndEvent;
|
||||
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;
|
||||
@@ -129,31 +128,28 @@ pub fn build_command_execution_end_item(payload: &ExecCommandEndEvent) -> Thread
|
||||
}
|
||||
|
||||
fn command_actions_for_path_uri(parsed_cmd: &[ParsedCommand], cwd: &PathUri) -> Vec<CommandAction> {
|
||||
// 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 } => {
|
||||
let path = path.to_string_lossy();
|
||||
match cwd.join(&path) {
|
||||
Ok(path) => Some(CommandAction::Read {
|
||||
command: cmd,
|
||||
name,
|
||||
path: path.into(),
|
||||
}),
|
||||
Err(error) => {
|
||||
warn!(
|
||||
command = cmd,
|
||||
path = %path,
|
||||
%cwd,
|
||||
%error,
|
||||
"omitting read command action whose path cannot be resolved against its cwd"
|
||||
);
|
||||
None
|
||||
}
|
||||
}
|
||||
},
|
||||
ParsedCommand::ListFiles { cmd, path } => {
|
||||
|
||||
@@ -2,7 +2,7 @@ use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn foreign_read_is_omitted_without_dropping_other_command_actions() {
|
||||
fn foreign_read_retains_target_native_path_without_dropping_other_command_actions() {
|
||||
#[cfg(windows)]
|
||||
let cwd = PathUri::parse("file:///usr/local/src").expect("valid foreign POSIX cwd");
|
||||
#[cfg(not(windows))]
|
||||
@@ -23,10 +23,26 @@ fn foreign_read_is_omitted_without_dropping_other_command_actions() {
|
||||
path: Some("src".to_string()),
|
||||
},
|
||||
];
|
||||
let read_path = cwd
|
||||
.join("file.txt")
|
||||
.expect("relative read path should resolve against foreign cwd");
|
||||
#[cfg(windows)]
|
||||
let expected_native_path = "/usr/local/src/file.txt";
|
||||
#[cfg(not(windows))]
|
||||
let expected_native_path = r"C:\src\file.txt";
|
||||
assert_eq!(
|
||||
read_path.inferred_native_path_string(),
|
||||
expected_native_path
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
command_actions_for_path_uri(&parsed_cmd, &cwd),
|
||||
vec![
|
||||
CommandAction::Read {
|
||||
command: "cat file.txt".to_string(),
|
||||
name: "file.txt".to_string(),
|
||||
path: read_path.into(),
|
||||
},
|
||||
CommandAction::ListFiles {
|
||||
command: "ls".to_string(),
|
||||
path: Some("subdir".to_string()),
|
||||
|
||||
@@ -110,7 +110,7 @@ pub enum CommandAction {
|
||||
Read {
|
||||
command: String,
|
||||
name: String,
|
||||
path: AbsolutePathBuf,
|
||||
path: LegacyAppPathString,
|
||||
},
|
||||
ListFiles {
|
||||
command: String,
|
||||
@@ -174,7 +174,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 }
|
||||
@@ -193,7 +193,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 }
|
||||
|
||||
@@ -1410,6 +1410,7 @@ There are additional item-specific events:
|
||||
|
||||
- `item/commandExecution/outputDelta` — streams stdout/stderr for the command; append deltas in order to render live output alongside `aggregatedOutput` in the final item.
|
||||
Final `commandExecution` items include parsed `commandActions`, `status`, `exitCode`, and `durationMs` so the UI can summarize what ran and whether it succeeded.
|
||||
- Resolved read paths in `commandActions` use the selected execution environment's native path syntax.
|
||||
|
||||
#### fileChange
|
||||
|
||||
|
||||
Reference in New Issue
Block a user