From 9de568372da86141ced85e4153494b0b159f2171 Mon Sep 17 00:00:00 2001 From: jif Date: Tue, 2 Jun 2026 21:09:34 +0200 Subject: [PATCH] Propagate permission approval environment id (#25862) ## Stack 1. #25850 - Key request-permission grants by environment: stores and applies sticky permission grants per environment id. 2. #25858 - Add `environmentId` to `request_permissions`: lets the model target a selected environment and resolves relative permission paths against it. 3. This PR (#25862) - Propagate permission approval environment id: carries the selected environment id through approval events, app-server requests, TUI prompts, and delegate forwarding. 4. #25867 - Add remote request permissions integration coverage: verifies the selected remote environment across request, approval, grant reuse, and exec. This PR is stacked on #25858, and #25867 is stacked on this PR. ## Why PR2 lets the model bind a `request_permissions` call to a selected environment, but the approval event and client-facing request still needed to carry that binding. For CCA, the user-facing prompt and delegated approval path should know which environment the grant applies to instead of relying on cwd alone. ## What Changed - Added optional `environmentId` to `RequestPermissionsEvent`. - Emit the selected environment id from core permission approval events. - Preserve the environment id through delegate forwarding, including cwd-based delegated requests. - Added `environmentId` to app-server permission approval params, generated schema/TypeScript artifacts, and README examples. - Preserve and display the environment id in TUI permission approval prompts. - Updated focused core, app-server protocol, and TUI conversion coverage. ## Testing Not run locally per instruction. Performed read-only `git diff --check`. --- codex-rs/analytics/src/analytics_client_tests.rs | 1 + .../json/PermissionsRequestApprovalParams.json | 9 ++++++++- .../schema/json/ServerRequest.json | 9 ++++++++- .../json/codex_app_server_protocol.schemas.json | 9 ++++++++- .../v2/PermissionsRequestApprovalParams.ts | 2 +- .../src/protocol/v2/permissions.rs | 2 ++ .../app-server-protocol/src/protocol/v2/tests.rs | 2 ++ codex-rs/app-server/README.md | 3 ++- codex-rs/app-server/src/bespoke_event_handling.rs | 1 + codex-rs/core/src/codex_delegate.rs | 2 +- codex-rs/core/src/codex_delegate_tests.rs | 6 +++++- codex-rs/core/src/session/mod.rs | 13 +++++++++++-- codex-rs/core/src/session/tests.rs | 9 +++++++++ codex-rs/protocol/src/request_permissions.rs | 9 +++++++++ codex-rs/tui/src/app/app_server_requests.rs | 1 + codex-rs/tui/src/app/event_dispatch.rs | 8 ++++++++ codex-rs/tui/src/app/tests.rs | 6 +++++- codex-rs/tui/src/app/thread_routing.rs | 1 + codex-rs/tui/src/bottom_pane/approval_overlay.rs | 10 ++++++++++ codex-rs/tui/src/chatwidget.rs | 1 + .../tui/src/chatwidget/tests/approval_requests.rs | 2 ++ codex-rs/tui/src/chatwidget/tool_requests.rs | 1 + 22 files changed, 97 insertions(+), 10 deletions(-) diff --git a/codex-rs/analytics/src/analytics_client_tests.rs b/codex-rs/analytics/src/analytics_client_tests.rs index 61ab5f6b2..0a11412ef 100644 --- a/codex-rs/analytics/src/analytics_client_tests.rs +++ b/codex-rs/analytics/src/analytics_client_tests.rs @@ -835,6 +835,7 @@ fn sample_permissions_approval_request(request_id: i64) -> ServerRequest { thread_id: "thread-1".to_string(), turn_id: "turn-1".to_string(), item_id: "permissions-1".to_string(), + environment_id: None, started_at_ms: 1_000, cwd: test_path_buf("/tmp").abs(), reason: Some("need network".to_string()), diff --git a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json index 337a46989..961e24c2c 100644 --- a/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/PermissionsRequestApprovalParams.json @@ -285,6 +285,13 @@ "cwd": { "$ref": "#/definitions/AbsolutePathBuf" }, + "environmentId": { + "default": null, + "type": [ + "null", + "string" + ] + }, "itemId": { "type": "string" }, @@ -319,4 +326,4 @@ ], "title": "PermissionsRequestApprovalParams", "type": "object" -} \ No newline at end of file +} diff --git a/codex-rs/app-server-protocol/schema/json/ServerRequest.json b/codex-rs/app-server-protocol/schema/json/ServerRequest.json index 411095f2c..13bac2e11 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ServerRequest.json @@ -1590,6 +1590,13 @@ "cwd": { "$ref": "#/definitions/AbsolutePathBuf" }, + "environmentId": { + "default": null, + "type": [ + "null", + "string" + ] + }, "itemId": { "type": "string" }, @@ -1998,4 +2005,4 @@ } ], "title": "ServerRequest" -} \ No newline at end of file +} 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 cdb8f9553..6aa4c5f35 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 @@ -3783,6 +3783,13 @@ "cwd": { "$ref": "#/definitions/v2/AbsolutePathBuf" }, + "environmentId": { + "default": null, + "type": [ + "null", + "string" + ] + }, "itemId": { "type": "string" }, @@ -19232,4 +19239,4 @@ }, "title": "CodexAppServerProtocol", "type": "object" -} \ No newline at end of file +} diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/PermissionsRequestApprovalParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/PermissionsRequestApprovalParams.ts index 509f60923..d0677f5f0 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/PermissionsRequestApprovalParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/PermissionsRequestApprovalParams.ts @@ -4,7 +4,7 @@ import type { AbsolutePathBuf } from "../AbsolutePathBuf"; import type { RequestPermissionProfile } from "./RequestPermissionProfile"; -export type PermissionsRequestApprovalParams = { threadId: string, turnId: string, itemId: string, +export type PermissionsRequestApprovalParams = { threadId: string, turnId: string, itemId: string, environmentId: string | null, /** * Unix timestamp (in milliseconds) when this approval request started. */ diff --git a/codex-rs/app-server-protocol/src/protocol/v2/permissions.rs b/codex-rs/app-server-protocol/src/protocol/v2/permissions.rs index dfc194227..45fc5c7e1 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/permissions.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/permissions.rs @@ -663,6 +663,8 @@ pub struct PermissionsRequestApprovalParams { pub thread_id: String, pub turn_id: String, pub item_id: String, + #[serde(default)] + pub environment_id: Option, /// Unix timestamp (in milliseconds) when this approval request started. #[ts(type = "number")] pub started_at_ms: i64, diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index 1e7558363..1c1e92b36 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -400,6 +400,7 @@ fn permissions_request_approval_uses_request_permission_profile() { "threadId": "thr_123", "turnId": "turn_123", "itemId": "call_123", + "environmentId": "remote", "startedAtMs": 1, "cwd": absolute_path_string("repo"), "reason": "Select a workspace root", @@ -416,6 +417,7 @@ fn permissions_request_approval_uses_request_permission_profile() { .expect("permissions request should deserialize"); assert_eq!(params.cwd, absolute_path("repo")); + assert_eq!(params.environment_id.as_deref(), Some("remote")); assert_eq!( params.permissions, RequestPermissionProfile { diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index f3b55337c..fdc34e524 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1379,7 +1379,7 @@ the client can offer session-scoped and/or persistent approval choices. ### Permission requests -The built-in `request_permissions` tool sends an `item/permissions/requestApproval` JSON-RPC request to the client with the requested permission profile. This v2 payload mirrors the command-execution `additionalPermissions` shape: it can request network access and additional filesystem access. The `cwd` field identifies the directory used to resolve project-root permissions and relative deny globs. +The built-in `request_permissions` tool sends an `item/permissions/requestApproval` JSON-RPC request to the client with the requested permission profile. This v2 payload mirrors the command-execution `additionalPermissions` shape: it can request network access and additional filesystem access. The `environmentId` and `cwd` fields identify the environment and directory used to resolve project-root permissions and relative deny globs. ```json { @@ -1389,6 +1389,7 @@ The built-in `request_permissions` tool sends an `item/permissions/requestApprov "threadId": "thr_123", "turnId": "turn_123", "itemId": "call_123", + "environmentId": "local", "cwd": "/Users/me/project", "reason": "Select a workspace root", "permissions": { diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 43e3d305b..f4a5f3b45 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -761,6 +761,7 @@ pub(crate) async fn apply_bespoke_event_handling( thread_id: conversation_id.to_string(), turn_id: request.turn_id.clone(), item_id: request.call_id.clone(), + environment_id: request.environment_id.clone(), started_at_ms: request.started_at_ms, cwd: request_cwd.clone(), reason: request.reason, diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index f5f04d2db..678a6e5d1 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -755,7 +755,7 @@ async fn handle_request_permissions( ) { let call_id = event.call_id; let args = RequestPermissionsArgs { - environment_id: None, + environment_id: event.environment_id, reason: event.reason, permissions: event.permissions, }; diff --git a/codex-rs/core/src/codex_delegate_tests.rs b/codex-rs/core/src/codex_delegate_tests.rs index 64e0c1641..7dabb09d3 100644 --- a/codex-rs/core/src/codex_delegate_tests.rs +++ b/codex-rs/core/src/codex_delegate_tests.rs @@ -183,9 +183,11 @@ async fn run_codex_thread_interactive_respects_pre_cancelled_spawn() { #[tokio::test] async fn handle_request_permissions_uses_tool_call_id_for_round_trip() { - let (parent_session, parent_ctx, rx_events) = + let (parent_session, mut parent_ctx, rx_events) = crate::session::tests::make_session_and_context_with_rx().await; *parent_session.active_turn.lock().await = Some(crate::state::ActiveTurn::default()); + let parent_ctx_mut = Arc::get_mut(&mut parent_ctx).expect("single turn context ref"); + parent_ctx_mut.environments.turn_environments[0].environment_id = "remote".to_string(); let (tx_sub, rx_sub) = bounded(SUBMISSION_CHANNEL_CAPACITY); let (_tx_events, rx_events_child) = bounded(SUBMISSION_CHANNEL_CAPACITY); @@ -228,6 +230,7 @@ async fn handle_request_permissions_uses_tool_call_id_for_round_trip() { RequestPermissionsEvent { call_id: request_call_id, turn_id: "child-turn-1".to_string(), + environment_id: Some("remote".to_string()), started_at_ms: 0, reason: Some("need access".to_string()), permissions: RequestPermissionProfile { @@ -252,6 +255,7 @@ async fn handle_request_permissions_uses_tool_call_id_for_round_trip() { panic!("expected RequestPermissions event"); }; assert_eq!(request.call_id, call_id.clone()); + assert_eq!(request.environment_id.as_deref(), Some("remote")); assert_eq!(request.cwd, Some(delegated_cwd)); parent_session diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 0abd3c6ec..5a92c9c82 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -2251,6 +2251,7 @@ impl Session { let event = EventMsg::RequestPermissions(RequestPermissionsEvent { call_id: call_id.clone(), turn_id: turn_context.sub_id.clone(), + environment_id: Some(environment.environment_id.clone()), started_at_ms: now_unix_timestamp_ms(), reason: args.reason, permissions: requested_permissions, @@ -2279,14 +2280,22 @@ impl Session { cwd: AbsolutePathBuf, cancellation_token: CancellationToken, ) -> Option { - let Some(primary_environment) = turn_context.environments.primary() else { + let turn_environment = match args.environment_id.as_deref() { + Some(environment_id) => turn_context + .environments + .turn_environments + .iter() + .find(|environment| environment.environment_id == environment_id), + None => turn_context.environments.primary(), + }; + let Some(turn_environment) = turn_environment else { return Some(RequestPermissionsResponse { permissions: RequestPermissionProfile::default(), scope: PermissionGrantScope::Turn, strict_auto_review: false, }); }; - let mut environment = primary_environment.selection(); + let mut environment = turn_environment.selection(); environment.cwd = cwd; self.request_permissions_for_environment( turn_context, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 6f54e54b1..30043937c 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -5399,6 +5399,10 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests() panic!("expected request_permissions event"); }; assert_eq!(request.call_id, call_id); + assert_eq!( + request.environment_id.as_deref(), + Some(codex_exec_server::LOCAL_ENVIRONMENT_ID) + ); #[allow(deprecated)] let turn_cwd = turn_context.cwd.clone(); assert_eq!(request.cwd, Some(turn_cwd)); @@ -5499,6 +5503,7 @@ async fn request_permissions_tool_resolves_relative_paths_against_selected_envir }), ..Default::default() }; + assert_eq!(request.environment_id.as_deref(), Some("remote")); assert_eq!(request.permissions, expected_permissions); session @@ -5616,6 +5621,10 @@ async fn request_permissions_response_materializes_session_cwd_grants_before_rec let EventMsg::RequestPermissions(request) = request_event.msg else { panic!("expected request_permissions event"); }; + assert_eq!( + request.environment_id.as_deref(), + Some(codex_exec_server::LOCAL_ENVIRONMENT_ID) + ); let request_cwd = request.cwd.clone().expect("request cwd"); session diff --git a/codex-rs/protocol/src/request_permissions.rs b/codex-rs/protocol/src/request_permissions.rs index 43e8b8ee9..1752363ec 100644 --- a/codex-rs/protocol/src/request_permissions.rs +++ b/codex-rs/protocol/src/request_permissions.rs @@ -79,6 +79,15 @@ pub struct RequestPermissionsEvent { /// Uses `#[serde(default)]` for backwards compatibility. #[serde(default)] pub turn_id: String, + #[serde( + default, + rename = "environmentId", + alias = "environment_id", + skip_serializing_if = "Option::is_none" + )] + #[ts(optional)] + #[ts(rename = "environmentId")] + pub environment_id: Option, #[ts(type = "number")] pub started_at_ms: i64, #[serde(skip_serializing_if = "Option::is_none")] diff --git a/codex-rs/tui/src/app/app_server_requests.rs b/codex-rs/tui/src/app/app_server_requests.rs index 184333302..c2580bd07 100644 --- a/codex-rs/tui/src/app/app_server_requests.rs +++ b/codex-rs/tui/src/app/app_server_requests.rs @@ -489,6 +489,7 @@ mod tests { thread_id: "thread-1".to_string(), turn_id: "turn-1".to_string(), item_id: "perm-1".to_string(), + environment_id: None, started_at_ms: 0, cwd: absolute_path(if cfg!(windows) { r"C:\tmp" } else { "/tmp" }), reason: None, diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 43e117984..3570491d1 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1856,12 +1856,20 @@ impl App { )); } ApprovalRequest::Permissions { + environment_id, permissions, reason, .. } => { let _ = tui.enter_alt_screen(); let mut lines = Vec::new(); + if let Some(environment_id) = environment_id { + lines.push(Line::from(vec![ + "Environment: ".into(), + environment_id.bold(), + ])); + lines.push(Line::from("")); + } if let Some(reason) = reason { lines.push(Line::from(vec!["Reason: ".into(), reason.italic()])); lines.push(Line::from("")); diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index f33f689ac..eb5e10c50 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -2539,6 +2539,7 @@ async fn inactive_thread_permissions_approval_preserves_file_system_permissions( thread_id: thread_id.to_string(), turn_id: "turn-approval".to_string(), item_id: "call-approval".to_string(), + environment_id: Some("remote".to_string()), started_at_ms: 0, cwd: test_absolute_path("/tmp"), reason: Some("Need access to .git".to_string()), @@ -2557,7 +2558,9 @@ async fn inactive_thread_permissions_approval_preserves_file_system_permissions( }; let Some(ThreadInteractiveRequest::Approval(ApprovalRequest::Permissions { - permissions, .. + environment_id, + permissions, + .. })) = app .interactive_request_for_thread_request(thread_id, &request) .await @@ -2565,6 +2568,7 @@ async fn inactive_thread_permissions_approval_preserves_file_system_permissions( panic!("expected permissions approval request"); }; + assert_eq!(environment_id.as_deref(), Some("remote")); assert_eq!( permissions, RequestPermissionProfile { diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index f2c5969f2..1bd28d5f6 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -310,6 +310,7 @@ impl App { thread_id, thread_label, call_id: params.item_id.clone(), + environment_id: params.environment_id.clone(), reason: params.reason.clone(), permissions: params.permissions.clone().into(), }), diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 17b17f359..f318dd210 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -84,6 +84,7 @@ pub(crate) enum ApprovalRequest { thread_id: ThreadId, thread_label: Option, call_id: String, + environment_id: Option, reason: Option, permissions: RequestPermissionProfile, }, @@ -713,6 +714,7 @@ fn build_header(request: &ApprovalRequest) -> Box { } ApprovalRequest::Permissions { thread_label, + environment_id, reason, permissions, .. @@ -725,6 +727,13 @@ fn build_header(request: &ApprovalRequest) -> Box { ])); header.push(Line::from("")); } + if let Some(environment_id) = environment_id { + header.push(Line::from(vec![ + "Environment: ".into(), + environment_id.clone().bold(), + ])); + header.push(Line::from("")); + } if let Some(reason) = reason { header.push(Line::from(vec!["Reason: ".into(), reason.clone().italic()])); header.push(Line::from("")); @@ -1227,6 +1236,7 @@ mod tests { thread_id: ThreadId::new(), thread_label: None, call_id: "test".to_string(), + environment_id: None, reason: Some("need workspace access".to_string()), permissions: RequestPermissionProfile { network: Some(NetworkPermissions { diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 246e16d69..c9827a4e7 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -864,6 +864,7 @@ fn request_permissions_from_params( RequestPermissionsEvent { turn_id: params.turn_id, call_id: params.item_id, + environment_id: params.environment_id, started_at_ms: params.started_at_ms, reason: params.reason, permissions: params.permissions.into(), diff --git a/codex-rs/tui/src/chatwidget/tests/approval_requests.rs b/codex-rs/tui/src/chatwidget/tests/approval_requests.rs index d732177ce..18fc15564 100644 --- a/codex-rs/tui/src/chatwidget/tests/approval_requests.rs +++ b/codex-rs/tui/src/chatwidget/tests/approval_requests.rs @@ -281,6 +281,7 @@ fn app_server_request_permissions_preserves_file_system_permissions() { thread_id: "thread-1".to_string(), turn_id: "turn-1".to_string(), item_id: "item-1".to_string(), + environment_id: Some("remote".to_string()), started_at_ms: 0, cwd: cwd.clone(), reason: Some("Select a workspace root".to_string()), @@ -310,6 +311,7 @@ fn app_server_request_permissions_preserves_file_system_permissions() { } ); assert_eq!(request.cwd, Some(cwd)); + assert_eq!(request.environment_id.as_deref(), Some("remote")); } #[tokio::test] diff --git a/codex-rs/tui/src/chatwidget/tool_requests.rs b/codex-rs/tui/src/chatwidget/tool_requests.rs index 58bbd7cef..300156345 100644 --- a/codex-rs/tui/src/chatwidget/tool_requests.rs +++ b/codex-rs/tui/src/chatwidget/tool_requests.rs @@ -441,6 +441,7 @@ impl ChatWidget { thread_id: self.thread_id.unwrap_or_default(), thread_label: None, call_id: ev.call_id, + environment_id: ev.environment_id, reason: ev.reason, permissions: ev.permissions, };