Skip to content

Commit af2e19b

Browse files
joshspicerCopilot
andcommitted
Avoid expanding Rust permission result API
Preserve approve-all fail-closed behavior by logging and returning the existing user-not-available decision instead of adding a public enum variant. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 363ad33 commit af2e19b

3 files changed

Lines changed: 17 additions & 15 deletions

File tree

rust/src/handler.rs

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,6 @@ pub enum PermissionResult {
4545
/// Decline to respond to this request, allowing another connected
4646
/// client to answer instead. The SDK suppresses the response.
4747
NoResult,
48-
/// The handler could not safely decide the request.
49-
Error(String),
5048
}
5149

5250
impl PermissionResult {
@@ -85,6 +83,11 @@ impl From<PermissionDecision> for PermissionResult {
8583
}
8684
}
8785

86+
pub(crate) fn permission_handler_failure(message: &str) -> PermissionResult {
87+
tracing::error!(error = message, "permission handler failed");
88+
PermissionResult::user_not_available()
89+
}
90+
8891
/// Response to a user input request.
8992
#[derive(Debug, Clone)]
9093
pub struct UserInputResponse {
@@ -288,8 +291,8 @@ impl PermissionHandler for ApproveAllHandler {
288291
data: PermissionRequestData,
289292
) -> PermissionResult {
290293
if data.managed_settings_enabled {
291-
PermissionResult::Error(
292-
"ApproveAllHandler cannot be used when managed settings are enabled".into(),
294+
permission_handler_failure(
295+
"ApproveAllHandler cannot be used when managed settings are enabled",
293296
)
294297
} else {
295298
PermissionResult::approve_once()
@@ -333,7 +336,7 @@ mod tests {
333336
}
334337

335338
#[tokio::test]
336-
async fn approve_all_handler_errors_when_managed_settings_enabled() {
339+
async fn approve_all_handler_fails_when_managed_settings_enabled() {
337340
let result = ApproveAllHandler
338341
.handle(
339342
SessionId::from("s1"),
@@ -344,7 +347,10 @@ mod tests {
344347
},
345348
)
346349
.await;
347-
assert!(matches!(result, PermissionResult::Error(_)));
350+
assert!(matches!(
351+
result,
352+
PermissionResult::Decision(PermissionDecision::UserNotAvailable(_))
353+
));
348354
}
349355

350356
#[tokio::test]

rust/src/permission.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use std::sync::Arc;
1616

1717
use async_trait::async_trait;
1818

19-
use crate::handler::{PermissionHandler, PermissionResult};
19+
use crate::handler::{PermissionHandler, PermissionResult, permission_handler_failure};
2020
use crate::types::{PermissionRequestData, RequestId, SessionId};
2121

2222
/// Return a [`PermissionHandler`] that approves ordinary requests.
@@ -120,8 +120,8 @@ impl PermissionHandler for PolicyHandler {
120120
};
121121
if approved {
122122
if matches!(self.policy, Policy::ApproveAll) && data.managed_settings_enabled {
123-
PermissionResult::Error(
124-
"approve-all policy cannot be used when managed settings are enabled".into(),
123+
permission_handler_failure(
124+
"approve-all policy cannot be used when managed settings are enabled",
125125
)
126126
} else {
127127
PermissionResult::approve_once()
@@ -154,14 +154,14 @@ mod tests {
154154
}
155155

156156
#[tokio::test]
157-
async fn approve_all_errors_when_managed_settings_enabled() {
157+
async fn approve_all_fails_when_managed_settings_enabled() {
158158
let h = approve_all();
159159
let mut request = data();
160160
request.managed_settings_enabled = true;
161161
assert!(matches!(
162162
h.handle(SessionId::from("s"), RequestId::new("1"), request)
163163
.await,
164-
PermissionResult::Error(_)
164+
PermissionResult::Decision(crate::types::PermissionDecision::UserNotAvailable(_))
165165
));
166166
}
167167

rust/src/session.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1557,10 +1557,6 @@ fn permission_request_data(
15571557
fn notification_permission_payload(result: &PermissionResult) -> Option<Value> {
15581558
match result {
15591559
PermissionResult::NoResult => None,
1560-
PermissionResult::Error(message) => {
1561-
tracing::error!(error = %message, "permission handler failed");
1562-
Some(serde_json::json!({ "kind": "user-not-available" }))
1563-
}
15641560
PermissionResult::Decision(decision) => Some(
15651561
serde_json::to_value(decision).expect("serializing permission decision should succeed"),
15661562
),

0 commit comments

Comments
 (0)