diff --git a/codex-rs/core/src/tools/handlers/request_permissions.rs b/codex-rs/core/src/tools/handlers/request_permissions.rs index 440bb18ca1..28fc7e0f40 100644 --- a/codex-rs/core/src/tools/handlers/request_permissions.rs +++ b/codex-rs/core/src/tools/handlers/request_permissions.rs @@ -1,5 +1,9 @@ +use codex_protocol::request_permissions::PermissionGrantScope; +use codex_protocol::request_permissions::RequestPermissionProfile; use codex_protocol::request_permissions::RequestPermissionsArgs; +use codex_protocol::request_permissions::RequestPermissionsResponse; use codex_sandboxing::policy_transforms::normalize_additional_permissions; +use serde::Serialize; use crate::function_tool::FunctionCallError; use crate::tools::context::FunctionToolOutput; @@ -11,6 +15,39 @@ use crate::tools::registry::ToolKind; pub struct RequestPermissionsHandler; +#[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +enum RequestPermissionsToolStatus { + Granted, + Denied, +} + +#[derive(Debug, Serialize, PartialEq, Eq)] +struct RequestPermissionsToolOutput { + status: RequestPermissionsToolStatus, + scope: PermissionGrantScope, + permissions: RequestPermissionProfile, + message: &'static str, +} + +fn output_for_response(response: RequestPermissionsResponse) -> RequestPermissionsToolOutput { + if response.permissions.is_empty() { + RequestPermissionsToolOutput { + status: RequestPermissionsToolStatus::Denied, + scope: response.scope, + permissions: response.permissions, + message: "The user has already denied or declined this permission request. Do not say that approval is still pending.", + } + } else { + RequestPermissionsToolOutput { + status: RequestPermissionsToolStatus::Granted, + scope: response.scope, + permissions: response.permissions, + message: "The user has already approved this permission request. These permissions are active now; do not ask the user to approve them again.", + } + } +} + impl ToolHandler for RequestPermissionsHandler { type Output = FunctionToolOutput; @@ -56,7 +93,8 @@ impl ToolHandler for RequestPermissionsHandler { ) })?; - let content = serde_json::to_string(&response).map_err(|err| { + let tool_output = output_for_response(response); + let content = serde_json::to_string(&tool_output).map_err(|err| { FunctionCallError::Fatal(format!( "failed to serialize request_permissions response: {err}" )) @@ -65,3 +103,66 @@ impl ToolHandler for RequestPermissionsHandler { Ok(FunctionToolOutput::from_text(content, Some(true))) } } + +#[cfg(test)] +mod tests { + use codex_protocol::models::NetworkPermissions; + use codex_protocol::request_permissions::PermissionGrantScope; + use codex_protocol::request_permissions::RequestPermissionProfile; + use codex_protocol::request_permissions::RequestPermissionsResponse; + use pretty_assertions::assert_eq; + use serde_json::json; + + use super::output_for_response; + + #[test] + fn request_permissions_tool_output_marks_granted_permissions_as_active() { + let permissions = RequestPermissionProfile { + network: Some(NetworkPermissions { + enabled: Some(true), + }), + file_system: None, + }; + + let output = output_for_response(RequestPermissionsResponse { + permissions, + scope: PermissionGrantScope::Session, + }); + + assert_eq!( + serde_json::to_value(output).expect("serialize tool output"), + json!({ + "status": "granted", + "scope": "session", + "permissions": { + "network": { + "enabled": true, + }, + "file_system": null, + }, + "message": "The user has already approved this permission request. These permissions are active now; do not ask the user to approve them again.", + }) + ); + } + + #[test] + fn request_permissions_tool_output_marks_empty_permissions_as_denied() { + let output = output_for_response(RequestPermissionsResponse { + permissions: RequestPermissionProfile::default(), + scope: PermissionGrantScope::Turn, + }); + + assert_eq!( + serde_json::to_value(output).expect("serialize tool output"), + json!({ + "status": "denied", + "scope": "turn", + "permissions": { + "network": null, + "file_system": null, + }, + "message": "The user has already denied or declined this permission request. Do not say that approval is still pending.", + }) + ); + } +} diff --git a/codex-rs/core/tests/suite/request_permissions.rs b/codex-rs/core/tests/suite/request_permissions.rs index 2cfd1cf6f7..32de8e56ed 100644 --- a/codex-rs/core/tests/suite/request_permissions.rs +++ b/codex-rs/core/tests/suite/request_permissions.rs @@ -115,6 +115,21 @@ fn request_permissions_tool_event( Ok(ev_function_call(call_id, "request_permissions", &args_str)) } +fn request_permissions_tool_event_with_scope( + call_id: &str, + reason: &str, + permissions: &RequestPermissionProfile, + scope: PermissionGrantScope, +) -> Result { + let args = json!({ + "reason": reason, + "permissions": permissions, + "scope": scope, + }); + let args_str = serde_json::to_string(&args)?; + Ok(ev_function_call(call_id, "request_permissions", &args_str)) +} + fn shell_command_event(call_id: &str, command: &str) -> Result { let args = json!({ "command": command, @@ -311,6 +326,90 @@ fn normalized_directory_write_permissions(path: &Path) -> Result Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let approval_policy = AskForApproval::OnRequest; + let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let sandbox_policy_for_config = sandbox_policy.clone(); + + let mut builder = test_codex().with_config(move |config| { + config.permissions.approval_policy = Constrained::allow_any(approval_policy); + config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config); + config + .features + .enable(Feature::RequestPermissionsTool) + .expect("test config should allow feature update"); + }); + let test = builder.build(&server).await?; + + let requested_dir = test.workspace_path("resolved-permissions-output"); + fs::create_dir_all(&requested_dir)?; + let requested_permissions = requested_directory_write_permissions(&requested_dir); + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-resolved-permissions-1"), + request_permissions_tool_event_with_scope( + "permissions-call", + "Allow writing outside the workspace", + &requested_permissions, + PermissionGrantScope::Session, + )?, + ev_completed("resp-resolved-permissions-1"), + ]), + sse(vec![ + ev_response_created("resp-resolved-permissions-2"), + ev_assistant_message("msg-resolved-permissions-1", "done"), + ev_completed("resp-resolved-permissions-2"), + ]), + ], + ) + .await; + + submit_turn( + &test, + "allow writing outside the workspace in this session", + approval_policy, + sandbox_policy, + ) + .await?; + + let granted_permissions = expect_request_permissions_event(&test, "permissions-call").await; + test.codex + .submit(Op::RequestPermissionsResponse { + id: "permissions-call".to_string(), + response: RequestPermissionsResponse { + permissions: granted_permissions.clone(), + scope: PermissionGrantScope::Session, + }, + }) + .await?; + wait_for_completion(&test).await; + + let output = responses + .function_call_output_text("permissions-call") + .expect("request_permissions tool output"); + let output_json: Value = serde_json::from_str(&output)?; + let message = output_json["message"] + .as_str() + .expect("model-facing resolution message"); + assert_eq!(output_json["status"], json!("granted")); + assert_eq!(output_json["scope"], json!("session")); + assert_eq!( + output_json["permissions"], + serde_json::to_value(&granted_permissions)? + ); + assert!(message.contains("already approved")); + assert!(message.contains("active now")); + assert!(message.contains("do not ask")); + + Ok(()) +} + #[tokio::test(flavor = "current_thread")] async fn with_additional_permissions_requires_approval_under_on_request() -> Result<()> { skip_if_no_network!(Ok(())); @@ -475,6 +574,8 @@ async fn request_permissions_tool_is_auto_denied_when_granular_request_permissio ); let call_output = results.single_request().function_call_output(call_id); + let output_json: Value = + serde_json::from_str(call_output["output"].as_str().unwrap_or_default())?; let result: RequestPermissionsResponse = serde_json::from_str(call_output["output"].as_str().unwrap_or_default())?; assert_eq!( @@ -484,6 +585,13 @@ async fn request_permissions_tool_is_auto_denied_when_granular_request_permissio scope: PermissionGrantScope::Turn, } ); + assert_eq!(output_json["status"], json!("denied")); + assert!( + output_json["message"] + .as_str() + .unwrap_or_default() + .contains("already denied") + ); Ok(()) } diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 49bd6a998b..27be020b4d 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -636,7 +636,7 @@ fn granular_prompt_intro_text() -> &'static str { } fn request_permissions_tool_prompt_section() -> &'static str { - "# request_permissions Tool\n\nThe built-in `request_permissions` tool is available in this session. Invoke it immediately when the user asks conversationally to allow specific filesystem or network access, for example \"allow writing to ~/Downloads in this session\" or \"grant access to /tmp/output\". Request only the specific permissions required. Set `scope` to `session` when the user explicitly asks for session-long access; otherwise use `turn`." + "# request_permissions Tool\n\nThe built-in `request_permissions` tool is available in this session. Invoke it immediately when the user asks conversationally to allow specific filesystem or network access, for example \"allow writing to ~/Downloads in this session\" or \"grant access to /tmp/output\". Request only the specific permissions required. Set `scope` to `session` when the user explicitly asks for session-long access; otherwise use `turn`. After this tool returns, the user has already approved or denied the request in the UI. If permissions were granted, treat them as active now; do not say \"once you approve\" or ask the user to approve the same request again. If no permissions were granted, say the request was not approved." } fn request_permission_preset_tool_prompt_section() -> &'static str { @@ -1811,6 +1811,8 @@ mod tests { let text = instructions.into_text(); assert!(text.contains("`approval_policy` is `unless-trusted`")); assert!(text.contains("# request_permissions Tool")); + assert!(text.contains("After this tool returns, the user has already approved or denied")); + assert!(text.contains("do not say \"once you approve\"")); } #[test] diff --git a/codex-rs/tools/src/local_tool.rs b/codex-rs/tools/src/local_tool.rs index 274cd5d399..a49ce62d4d 100644 --- a/codex-rs/tools/src/local_tool.rs +++ b/codex-rs/tools/src/local_tool.rs @@ -302,7 +302,7 @@ pub fn create_request_permissions_tool(description: String) -> ToolSpec { } pub fn request_permissions_tool_description() -> String { - "Open a permissions request for specific filesystem or network access, such as writing to a named path like ~/Downloads. Use this immediately when the user conversationally asks to allow access to a specific path or network permission. Granted permissions apply automatically to later shell-like commands in the current turn, or for the rest of the session when scope is \"session\" and the client approves them at session scope." + "Open a permissions request for specific filesystem or network access, such as writing to a named path like ~/Downloads. Use this immediately when the user conversationally asks to allow access to a specific path or network permission. The returned result means the user has already approved or denied the request in the UI. Granted permissions apply automatically to later shell-like commands in the current turn, or for the rest of the session when scope is \"session\" and the client approves them at session scope. After the tool returns, do not ask the user to approve the same request again." .to_string() } diff --git a/codex-rs/tools/src/local_tool_tests.rs b/codex-rs/tools/src/local_tool_tests.rs index c71578a6c6..8911253416 100644 --- a/codex-rs/tools/src/local_tool_tests.rs +++ b/codex-rs/tools/src/local_tool_tests.rs @@ -337,6 +337,14 @@ fn request_permissions_tool_includes_full_permission_schema() { ); } +#[test] +fn request_permissions_tool_description_marks_returned_decision_as_resolved() { + let description = request_permissions_tool_description(); + + assert!(description.contains("the user has already approved or denied the request")); + assert!(description.contains("do not ask the user to approve the same request again")); +} + #[test] fn shell_command_tool_matches_expected_spec() { let tool = create_shell_command_tool(CommandToolOptions {