agent/codex
1
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

Add environmentId to request_permissions (#25858)

Browse Source 
## Stack

1. #25850 - Key request-permission grants by environment: stores and
applies sticky permission grants per environment id.
2. This PR (#25858) - Add `environmentId` to `request_permissions`: lets
the model target a selected environment and resolves relative permission
paths against it.
3. #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 #25850; #25862 and #25867 are stacked on this PR.

## Why

PR1 made request-permission grants internally environment-keyed, but the
model-facing `request_permissions` tool could still only target the
primary environment. For CCA and multi-environment turns, the tool needs
an explicit way to bind a permission request to a selected attached
environment before resolving relative paths.

## What Changed

- Added optional `environmentId` to `RequestPermissionsArgs`, with
`environment_id` accepted as an alias.
- Exposed `environmentId` in the `request_permissions` tool schema and
description.
- Resolve the selected environment before parsing filesystem permission
paths, so relative paths bind to the selected environment cwd.
- Route validated tool calls through
`request_permissions_for_environment` directly instead of duplicating
environment lookup in `Session::request_permissions`.
- Reject unknown environment ids with a model-facing error.
- Updated focused request-permissions and Guardian call sites for the
new optional field.

## Testing

Not run locally per instruction.
This commit is contained in:
jif
2026-06-02 20:51:25 +02:00
committed by GitHub
Unverified
parent 8e4b92d294
commit e29071e4c9
8 changed files with 119 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/codex_delegate.rs
+1
View File
@@ -755,6 +755,7 @@ async fn handle_request_permissions(
) {
let call_id = event.call_id;
let args = RequestPermissionsArgs {
environment_id: None,
reason: event.reason,
permissions: event.permissions,
};
codex-rs/core/src/session/mod.rs
-24
View File
@@ -2112,30 +2112,6 @@ impl Session {
rx_approve
}
pub async fn request_permissions(
self: &Arc<Self>,
turn_context: &Arc<TurnContext>,
call_id: String,
args: RequestPermissionsArgs,
cancellation_token: CancellationToken,
) -> Option<RequestPermissionsResponse> {
let Some(turn_environment) = turn_context.environments.primary() else {
return Some(RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
});
};
self.request_permissions_for_environment(
turn_context,
call_id,
args,
turn_environment.selection(),
cancellation_token,
)
.await
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
codex-rs/core/src/session/tests.rs
+59 -4
View File
@@ -5365,11 +5365,17 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests()
let turn_context = Arc::clone(&turn_context);
let call_id = call_id.clone();
async move {
let environment = turn_context
.environments
.primary()
.expect("primary environment")
.selection();
session
.request_permissions(
.request_permissions_for_environment(
&turn_context,
call_id,
codex_protocol::request_permissions::RequestPermissionsArgs {
environment_id: None,
reason: Some("need network".to_string()),
permissions: RequestPermissionProfile {
network: Some(codex_protocol::models::NetworkPermissions {
@@ -5378,6 +5384,7 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests()
..RequestPermissionProfile::default()
},
},
environment,
CancellationToken::new(),
)
.await
@@ -5409,7 +5416,7 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests()
}
#[tokio::test]
async fn request_permissions_tool_resolves_relative_paths_against_primary_environment() {
async fn request_permissions_tool_resolves_relative_paths_against_selected_environment() {
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
*session.active_turn.lock().await = Some(ActiveTurn::default());
let environment_cwd = {
@@ -5429,6 +5436,7 @@ async fn request_permissions_tool_resolves_relative_paths_against_primary_enviro
mcp_elicitations: true,
}))
.expect("test setup should allow updating approval policy");
turn_context_mut.environments.turn_environments[0].environment_id = "remote".to_string();
turn_context_mut.environments.turn_environments[0].cwd = environment_cwd.clone();
let call_id = "call-1".to_string();
@@ -5451,6 +5459,7 @@ async fn request_permissions_tool_resolves_relative_paths_against_primary_enviro
source: ToolCallSource::Direct,
payload: ToolPayload::Function {
arguments: json!({
"environment_id": "remote",
"reason": "need write",
"permissions": {
"file_system": {
@@ -5509,6 +5518,38 @@ async fn request_permissions_tool_resolves_relative_paths_against_primary_enviro
.expect("request_permissions handler should succeed");
}
#[tokio::test]
async fn request_permissions_tool_rejects_unknown_environment_id() {
let (session, turn_context) = make_session_and_context().await;
let result = RequestPermissionsHandler
.handle(ToolInvocation {
session: Arc::new(session),
turn: Arc::new(turn_context),
cancellation_token: CancellationToken::new(),
tracker: Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())),
call_id: "call-1".to_string(),
tool_name: codex_tools::ToolName::plain("request_permissions"),
source: ToolCallSource::Direct,
payload: ToolPayload::Function {
arguments: json!({
"environment_id": "missing",
"permissions": {
"network": {
"enabled": true,
},
},
})
.to_string(),
},
})
.await;
let Err(FunctionCallError::RespondToModel(output)) = result else {
panic!("expected unknown environment id to be rejected");
};
assert_eq!(output, "unknown turn environment id `missing`");
}
#[tokio::test]
async fn request_permissions_response_materializes_session_cwd_grants_before_recording() {
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
@@ -5547,14 +5588,21 @@ async fn request_permissions_response_materializes_session_cwd_grants_before_rec
let call_id = call_id.clone();
let requested_permissions = requested_permissions.clone();
async move {
let environment = turn_context
.environments
.primary()
.expect("primary environment")
.selection();
session
.request_permissions(
.request_permissions_for_environment(
&turn_context,
call_id,
codex_protocol::request_permissions::RequestPermissionsArgs {
environment_id: None,
reason: Some("need cwd write".to_string()),
permissions: requested_permissions,
},
environment,
CancellationToken::new(),
)
.await
@@ -5627,11 +5675,17 @@ async fn request_permissions_is_auto_denied_when_granular_policy_blocks_tool_req
let session = Arc::new(session);
let turn_context = Arc::new(turn_context);
let call_id = "call-1".to_string();
let environment = turn_context
.environments
.primary()
.expect("primary environment")
.selection();
let response = session
.request_permissions(
.request_permissions_for_environment(
&turn_context,
call_id,
codex_protocol::request_permissions::RequestPermissionsArgs {
environment_id: None,
reason: Some("need network".to_string()),
permissions: RequestPermissionProfile {
network: Some(codex_protocol::models::NetworkPermissions {
@@ -5640,6 +5694,7 @@ async fn request_permissions_is_auto_denied_when_granular_policy_blocks_tool_req
..RequestPermissionProfile::default()
},
},
environment,
CancellationToken::new(),
)
.await;
codex-rs/core/src/session/tests/guardian_tests.rs
+16 -2
View File
@@ -124,15 +124,22 @@ async fn request_permissions_routes_to_guardian_when_reviewer_is_enabled() {
}),
..RequestPermissionProfile::default()
};
let environment = turn_context
.environments
.primary()
.expect("primary environment")
.selection();
let response = tokio::time::timeout(
Duration::from_secs(45),
session.request_permissions(
session.request_permissions_for_environment(
&turn_context,
"perm-call-1".to_string(),
RequestPermissionsArgs {
environment_id: None,
reason: Some("need network".to_string()),
permissions: requested_permissions.clone(),
},
environment,
CancellationToken::new(),
),
)
@@ -213,14 +220,21 @@ async fn request_permissions_guardian_review_stops_when_cancelled() {
let requested_permissions = requested_permissions.clone();
let cancellation_token = cancellation_token.clone();
async move {
let environment = turn_context
.environments
.primary()
.expect("primary environment")
.selection();
session
.request_permissions(
.request_permissions_for_environment(
&turn_context,
"perm-call-cancelled".to_string(),
RequestPermissionsArgs {
environment_id: None,
reason: Some("need network".to_string()),
permissions: requested_permissions,
},
environment,
cancellation_token,
)
.await
codex-rs/core/src/tools/handlers/request_permissions.rs
+20 -2
View File
@@ -6,16 +6,25 @@ use crate::tools::context::FunctionToolOutput;
use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolPayload;
use crate::tools::context::boxed_tool_output;
use crate::tools::handlers::parse_arguments;
use crate::tools::handlers::parse_arguments_with_base_path;
use crate::tools::handlers::resolve_tool_environment;
use crate::tools::handlers::shell_spec::create_request_permissions_tool;
use crate::tools::handlers::shell_spec::request_permissions_tool_description;
use crate::tools::registry::CoreToolRuntime;
use crate::tools::registry::ToolExecutor;
use codex_tools::ToolName;
use codex_tools::ToolSpec;
use serde::Deserialize;
pub struct RequestPermissionsHandler;
#[derive(Deserialize)]
struct RequestPermissionsEnvironmentArgs {
#[serde(default, rename = "environment_id", alias = "environmentId")]
environment_id: Option<String>,
}
#[async_trait::async_trait]
impl ToolExecutor<ToolInvocation> for RequestPermissionsHandler {
fn tool_name(&self) -> ToolName {
@@ -48,7 +57,10 @@ impl ToolExecutor<ToolInvocation> for RequestPermissionsHandler {
}
};
let Some(turn_environment) = turn.environments.primary() else {
let environment_args: RequestPermissionsEnvironmentArgs = parse_arguments(&arguments)?;
let Some(turn_environment) =
resolve_tool_environment(turn.as_ref(), environment_args.environment_id.as_deref())?
else {
return Err(FunctionCallError::RespondToModel(
"request_permissions requires a primary environment".to_string(),
));
@@ -65,7 +77,13 @@ impl ToolExecutor<ToolInvocation> for RequestPermissionsHandler {
}
let response = session
.request_permissions(&turn, call_id, args, cancellation_token)
.request_permissions_for_environment(
&turn,
call_id,
args,
turn_environment.selection(),
cancellation_token,
)
.await
.ok_or_else(|| {
FunctionCallError::RespondToModel(
codex-rs/core/src/tools/handlers/shell_spec.rs
+8 -1
View File
@@ -229,6 +229,13 @@ pub fn create_request_permissions_tool(description: String) -> ToolSpec {
"Optional short explanation for why additional permissions are needed.".to_string(),
)),
),
(
"environment_id".to_string(),
JsonSchema::string(Some(
"Environment id from <environment_context>. Omit to use the primary environment."
.to_string(),
)),
),
("permissions".to_string(), permission_profile_schema()),
]);
@@ -247,7 +254,7 @@ pub fn create_request_permissions_tool(description: String) -> ToolSpec {
}
pub fn request_permissions_tool_description() -> String {
"Request additional filesystem or network permissions from the user and wait for the client to grant a subset of the requested permission profile. Granted permissions apply automatically to later shell-like commands in the current turn, or for the rest of the session if the client approves them at session scope."
"Request additional filesystem or network permissions from the user and wait for the client to grant a subset of the requested permission profile. Use environment_id to target a specific attached environment; omit it to use the primary environment. Relative filesystem paths resolve against the selected environment cwd. Granted permissions apply automatically to later shell-like commands in the current turn, or for the rest of the session if the client approves them at session scope."
.to_string()
}
codex-rs/core/src/tools/handlers/shell_spec_tests.rs
+7
View File
@@ -172,6 +172,13 @@ fn request_permissions_tool_includes_full_permission_schema() {
"Optional short explanation for why additional permissions are needed.".to_string(),
)),
),
(
"environment_id".to_string(),
JsonSchema::string(Some(
"Environment id from <environment_context>. Omit to use the primary environment."
.to_string(),
)),
),
("permissions".to_string(), permission_profile_schema()),
]);
codex-rs/protocol/src/request_permissions.rs
+8
View File
@@ -48,6 +48,14 @@ impl From<AdditionalPermissionProfile> for RequestPermissionProfile {
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)]
pub struct RequestPermissionsArgs {
#[serde(
default,
rename = "environment_id",
alias = "environmentId",
skip_serializing_if = "Option::is_none"
)]
#[ts(optional)]
pub environment_id: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub reason: Option<String>,
pub permissions: RequestPermissionProfile,