diff --git a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json index 3968da2a39ff..78e75d7c460c 100644 --- a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json @@ -25,6 +25,7 @@ ] }, "read": { + "description": "This will be removed in favor of `entries`.", "items": { "$ref": "#/definitions/AbsolutePathBuf" }, @@ -34,6 +35,7 @@ ] }, "write": { + "description": "This will be removed in favor of `entries`.", "items": { "$ref": "#/definitions/AbsolutePathBuf" }, diff --git a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json index c2e26c5532dc..ef268908f946 100644 --- a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json @@ -25,6 +25,7 @@ ] }, "read": { + "description": "This will be removed in favor of `entries`.", "items": { "$ref": "#/definitions/AbsolutePathBuf" }, @@ -34,6 +35,7 @@ ] }, "write": { + "description": "This will be removed in favor of `entries`.", "items": { "$ref": "#/definitions/AbsolutePathBuf" }, diff --git a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json index e066108e6fef..f49165296a3f 100644 --- a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json +++ b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalResponse.json @@ -25,6 +25,7 @@ ] }, "read": { + "description": "This will be removed in favor of `entries`.", "items": { "$ref": "#/definitions/AbsolutePathBuf" }, @@ -34,6 +35,7 @@ ] }, "write": { + "description": "This will be removed in favor of `entries`.", "items": { "$ref": "#/definitions/AbsolutePathBuf" }, diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 38acf1c90291..59b3f5b45a03 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -84,6 +84,7 @@ ] }, "read": { + "description": "This will be removed in favor of `entries`.", "items": { "$ref": "#/definitions/AbsolutePathBuf" }, @@ -93,6 +94,7 @@ ] }, "write": { + "description": "This will be removed in favor of `entries`.", "items": { "$ref": "#/definitions/AbsolutePathBuf" }, diff --git a/codex-rs/app-server-protocol/schema/json/ServerRequest.json b/codex-rs/app-server-protocol/schema/json/ServerRequest.json index 016dbe3415a8..84bf5247392b 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ServerRequest.json @@ -25,6 +25,7 @@ ] }, "read": { + "description": "This will be removed in favor of `entries`.", "items": { "$ref": "#/definitions/AbsolutePathBuf" }, @@ -34,6 +35,7 @@ ] }, "write": { + "description": "This will be removed in favor of `entries`.", "items": { "$ref": "#/definitions/AbsolutePathBuf" }, 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 516e33aa7ac1..3eaf25b9b2d1 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 @@ -5298,6 +5298,7 @@ ] }, "read": { + "description": "This will be removed in favor of `entries`.", "items": { "$ref": "#/definitions/v2/AbsolutePathBuf" }, @@ -5307,6 +5308,7 @@ ] }, "write": { + "description": "This will be removed in favor of `entries`.", "items": { "$ref": "#/definitions/v2/AbsolutePathBuf" }, 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 67a4cb186fd6..b1f9935150ca 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 @@ -148,6 +148,7 @@ ] }, "read": { + "description": "This will be removed in favor of `entries`.", "items": { "$ref": "#/definitions/AbsolutePathBuf" }, @@ -157,6 +158,7 @@ ] }, "write": { + "description": "This will be removed in favor of `entries`.", "items": { "$ref": "#/definitions/AbsolutePathBuf" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewCompletedNotification.json index 27716e59b079..e4a278330c71 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewCompletedNotification.json @@ -25,6 +25,7 @@ ] }, "read": { + "description": "This will be removed in favor of `entries`.", "items": { "$ref": "#/definitions/AbsolutePathBuf" }, @@ -34,6 +35,7 @@ ] }, "write": { + "description": "This will be removed in favor of `entries`.", "items": { "$ref": "#/definitions/AbsolutePathBuf" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewStartedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewStartedNotification.json index c039f2191af6..b4ad6af44b74 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewStartedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewStartedNotification.json @@ -25,6 +25,7 @@ ] }, "read": { + "description": "This will be removed in favor of `entries`.", "items": { "$ref": "#/definitions/AbsolutePathBuf" }, @@ -34,6 +35,7 @@ ] }, "write": { + "description": "This will be removed in favor of `entries`.", "items": { "$ref": "#/definitions/AbsolutePathBuf" }, diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts index 2be8f7f0e7ca..e29263b95fac 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts @@ -4,4 +4,12 @@ import type { AbsolutePathBuf } from "../AbsolutePathBuf"; import type { FileSystemSandboxEntry } from "./FileSystemSandboxEntry"; -export type AdditionalFileSystemPermissions = { read: Array | null, write: Array | null, globScanMaxDepth?: number, entries?: Array, }; +export type AdditionalFileSystemPermissions = { +/** + * This will be removed in favor of `entries`. + */ +read: Array | null, +/** + * This will be removed in favor of `entries`. + */ +write: Array | null, globScanMaxDepth?: number, entries?: Array, }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 655ad2d0adcd..937678793a21 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -1273,7 +1273,9 @@ impl From for NetworkApprovalContext { #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] pub struct AdditionalFileSystemPermissions { + /// This will be removed in favor of `entries`. pub read: Option>, + /// This will be removed in favor of `entries`. pub write: Option>, #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] @@ -1286,11 +1288,26 @@ pub struct AdditionalFileSystemPermissions { impl From for AdditionalFileSystemPermissions { fn from(value: CoreFileSystemPermissions) -> Self { if let Some((read, write)) = value.legacy_read_write_roots() { + let mut entries = Vec::with_capacity( + read.as_ref().map_or(0, Vec::len) + write.as_ref().map_or(0, Vec::len), + ); + if let Some(paths) = read.as_ref() { + entries.extend(paths.iter().map(|path| FileSystemSandboxEntry { + path: FileSystemPath::Path { path: path.clone() }, + access: FileSystemAccessMode::Read, + })); + } + if let Some(paths) = write.as_ref() { + entries.extend(paths.iter().map(|path| FileSystemSandboxEntry { + path: FileSystemPath::Path { path: path.clone() }, + access: FileSystemAccessMode::Write, + })); + } Self { read, write, glob_scan_max_depth: None, - entries: None, + entries: Some(entries), } } else { Self { @@ -7764,6 +7781,45 @@ mod tests { ); } + #[test] + fn additional_file_system_permissions_populates_entries_for_legacy_roots() { + let read_only_path = absolute_path("read-only"); + let read_write_path = absolute_path("read-write"); + let core_permissions = CoreFileSystemPermissions::from_read_write_roots( + Some(vec![read_only_path.clone()]), + Some(vec![read_write_path.clone()]), + ); + + let permissions = AdditionalFileSystemPermissions::from(core_permissions.clone()); + + assert_eq!( + permissions, + AdditionalFileSystemPermissions { + read: Some(vec![read_only_path.clone()]), + write: Some(vec![read_write_path.clone()]), + glob_scan_max_depth: None, + entries: Some(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: read_only_path, + }, + access: FileSystemAccessMode::Read, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: read_write_path, + }, + access: FileSystemAccessMode::Write, + }, + ]), + } + ); + assert_eq!( + CoreFileSystemPermissions::from(permissions), + core_permissions + ); + } + #[test] fn additional_file_system_permissions_rejects_zero_glob_scan_depth() { serde_json::from_value::(json!({ diff --git a/codex-rs/app-server/tests/suite/v2/request_permissions.rs b/codex-rs/app-server/tests/suite/v2/request_permissions.rs index 4a4eb9340ef5..7acdc1b12f23 100644 --- a/codex-rs/app-server/tests/suite/v2/request_permissions.rs +++ b/codex-rs/app-server/tests/suite/v2/request_permissions.rs @@ -78,12 +78,32 @@ async fn request_permissions_round_trip() -> Result<()> { assert_eq!(params.item_id, "call1"); assert!(params.cwd.as_path().is_absolute()); assert_eq!(params.reason, Some("Select a workspace root".to_string())); - let requested_writes = params + let requested_file_system = params .permissions .file_system - .and_then(|file_system| file_system.write) + .expect("request should include file system permissions"); + let requested_writes = requested_file_system + .write + .clone() .expect("request should include write permissions"); assert_eq!(requested_writes.len(), 2); + assert_eq!( + requested_file_system.entries, + Some(vec![ + codex_app_server_protocol::FileSystemSandboxEntry { + path: codex_app_server_protocol::FileSystemPath::Path { + path: requested_writes[0].clone(), + }, + access: codex_app_server_protocol::FileSystemAccessMode::Write, + }, + codex_app_server_protocol::FileSystemSandboxEntry { + path: codex_app_server_protocol::FileSystemPath::Path { + path: requested_writes[1].clone(), + }, + access: codex_app_server_protocol::FileSystemAccessMode::Write, + }, + ]) + ); let resolved_request_id = request_id.clone(); mcp.send_response( diff --git a/codex-rs/tui/src/app/app_server_requests.rs b/codex-rs/tui/src/app/app_server_requests.rs index 3149dd9f5975..5c2c69385517 100644 --- a/codex-rs/tui/src/app/app_server_requests.rs +++ b/codex-rs/tui/src/app/app_server_requests.rs @@ -562,7 +562,20 @@ mod tests { read: Some(vec![absolute_path(read_path)]), write: Some(vec![absolute_path(write_path)]), glob_scan_max_depth: None, - entries: None, + entries: Some(vec![ + codex_app_server_protocol::FileSystemSandboxEntry { + path: codex_app_server_protocol::FileSystemPath::Path { + path: absolute_path(read_path), + }, + access: codex_app_server_protocol::FileSystemAccessMode::Read, + }, + codex_app_server_protocol::FileSystemSandboxEntry { + path: codex_app_server_protocol::FileSystemPath::Path { + path: absolute_path(write_path), + }, + access: codex_app_server_protocol::FileSystemAccessMode::Write, + }, + ]), }), }, scope: PermissionGrantScope::Session, diff --git a/codex-rs/tui/src/app/thread_session_state.rs b/codex-rs/tui/src/app/thread_session_state.rs index cbf40d0d7541..d33df0f681f1 100644 --- a/codex-rs/tui/src/app/thread_session_state.rs +++ b/codex-rs/tui/src/app/thread_session_state.rs @@ -108,6 +108,7 @@ mod tests { approval_policy: AskForApproval::Never, approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), + permission_profile: None, cwd: cwd.abs(), instruction_source_paths: Vec::new(), reasoning_effort: None, @@ -155,7 +156,7 @@ mod tests { .insert(side_thread_id, SideThreadState::new(main_thread_id)); app.config.permissions.approval_policy = codex_config::Constrained::allow_any(AskForApproval::OnRequest); - app.config.approvals_reviewer = ApprovalsReviewer::GuardianSubagent; + app.config.approvals_reviewer = ApprovalsReviewer::AutoReview; app.config.permissions.sandbox_policy = codex_config::Constrained::allow_any(SandboxPolicy::new_workspace_write_policy()); @@ -164,7 +165,7 @@ mod tests { let expected_main_session = ThreadSessionState { approval_policy: AskForApproval::OnRequest, - approvals_reviewer: ApprovalsReviewer::GuardianSubagent, + approvals_reviewer: ApprovalsReviewer::AutoReview, sandbox_policy: SandboxPolicy::new_workspace_write_policy(), ..main_session }; diff --git a/codex-rs/tui/src/app_server_approval_conversions.rs b/codex-rs/tui/src/app_server_approval_conversions.rs index a26f9b2a902f..a0d86db7d61c 100644 --- a/codex-rs/tui/src/app_server_approval_conversions.rs +++ b/codex-rs/tui/src/app_server_approval_conversions.rs @@ -93,7 +93,20 @@ mod tests { read: Some(vec![absolute_path("/tmp/read-only")]), write: Some(vec![absolute_path("/tmp/write")]), glob_scan_max_depth: None, - entries: None, + entries: Some(vec![ + codex_app_server_protocol::FileSystemSandboxEntry { + path: codex_app_server_protocol::FileSystemPath::Path { + path: absolute_path("/tmp/read-only"), + }, + access: codex_app_server_protocol::FileSystemAccessMode::Read, + }, + codex_app_server_protocol::FileSystemSandboxEntry { + path: codex_app_server_protocol::FileSystemPath::Path { + path: absolute_path("/tmp/write"), + }, + access: codex_app_server_protocol::FileSystemAccessMode::Write, + }, + ]), }), } );