fix(core): clarify request permissions results

Return model-visible status and guidance from `request_permissions` so
follow-up turns know the UI decision has already resolved. This keeps
approved permissions framed as active instead of pending approval.

Update prompt and tool descriptions with the same contract, and cover the
granted and denied output shapes plus the follow-up request payload.
This commit is contained in:
Felipe Coury
2026-04-12 15:32:11 -03:00
parent afefd3b1b1
commit 755c7ff1ee
5 changed files with 222 additions and 3 deletions

View File

@@ -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.",
})
);
}
}

View File

@@ -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<Value> {
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<Value> {
let args = json!({
"command": command,
@@ -311,6 +326,90 @@ fn normalized_directory_write_permissions(path: &Path) -> Result<RequestPermissi
})
}
#[tokio::test(flavor = "current_thread")]
async fn request_permissions_tool_output_tells_model_decision_is_resolved() -> 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(())
}

View File

@@ -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]

View File

@@ -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()
}

View File

@@ -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 {