mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Preserve diagnostic reasons for MCP user verification failures (#51808)
## Why User-verification cancellations previously discarded failure reasons, leaving MCP servers unable to distinguish user cancellation from unavailable approval, interruption, or invalid proof. ## What changed - Carry a bounded `openai/userVerificationReason` value in response `_meta` for verification failures, declines, and cancellations across the TUI, app server, and MCP client. - Preserve recognized reasons and classify invalid responses, missing or invalid proofs, response errors, closed response channels, interruptions, and unavailable approval. - Discard unknown reasons and unrelated metadata. Keep proof material out of decline and cancellation responses, and keep display messages and raw error diagnostics out of response metadata. ## Testing Add and update tests for typed failure propagation, metadata filtering, invalid proofs, response-channel failures, interrupted requests, and unavailable approval, including TUI RPC and native MCP response coverage. GitOrigin-RevId: 6b7f855ce8eb13e759b5b86d0dc2b1afdfcff7b3
This commit is contained in:
@@ -3,18 +3,34 @@
|
||||
use codex_app_server_protocol::McpServerElicitationAction;
|
||||
use codex_app_server_protocol::McpServerElicitationRequestResponse;
|
||||
use codex_app_server_protocol::UserVerificationProof;
|
||||
use codex_rmcp_client::UserVerificationReason;
|
||||
use serde::Deserialize;
|
||||
use tokio::sync::oneshot;
|
||||
|
||||
use crate::outgoing_message::ClientRequestResult;
|
||||
use crate::server_request_error::is_turn_transition_server_request_error;
|
||||
|
||||
pub(crate) fn from_client_result(
|
||||
result: Result<ClientRequestResult, oneshot::error::RecvError>,
|
||||
) -> McpServerElicitationRequestResponse {
|
||||
let response = result.ok().and_then(Result::ok).and_then(|value| {
|
||||
serde_json::from_value::<McpServerElicitationRequestResponse>(value).ok()
|
||||
});
|
||||
let mut reason = UserVerificationReason::InvalidResponse;
|
||||
let response = match result {
|
||||
Ok(Ok(value)) => serde_json::from_value::<McpServerElicitationRequestResponse>(value).ok(),
|
||||
Ok(Err(error)) => {
|
||||
reason = if is_turn_transition_server_request_error(&error) {
|
||||
UserVerificationReason::Interrupted
|
||||
} else {
|
||||
UserVerificationReason::ResponseError
|
||||
};
|
||||
None
|
||||
}
|
||||
Err(_) => {
|
||||
reason = UserVerificationReason::ResponseChannelClosed;
|
||||
None
|
||||
}
|
||||
};
|
||||
if let Some(mut response) = response {
|
||||
let client_reason = UserVerificationReason::from_meta(response.meta.as_ref());
|
||||
response.meta = None;
|
||||
match response.action {
|
||||
McpServerElicitationAction::Accept => {
|
||||
@@ -26,9 +42,11 @@ pub(crate) fn from_client_result(
|
||||
// The MCP boundary validates the encoded proof and size limits.
|
||||
return response;
|
||||
}
|
||||
reason = UserVerificationReason::InvalidProof;
|
||||
}
|
||||
McpServerElicitationAction::Decline | McpServerElicitationAction::Cancel => {
|
||||
response.content = None;
|
||||
response.meta = client_reason.map(UserVerificationReason::into_meta);
|
||||
return response;
|
||||
}
|
||||
}
|
||||
@@ -36,7 +54,7 @@ pub(crate) fn from_client_result(
|
||||
McpServerElicitationRequestResponse {
|
||||
action: McpServerElicitationAction::Cancel,
|
||||
content: None,
|
||||
meta: None,
|
||||
meta: Some(reason.into_meta()),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -19,18 +19,82 @@ fn accepted_device_proof_uses_existing_content_and_discards_metadata() {
|
||||
|
||||
#[test]
|
||||
fn invalid_response_or_accept_without_proof_cancels() {
|
||||
for response in [
|
||||
json!({"invalid": "response"}),
|
||||
json!({"action": "accept", "content": null, "_meta": null}),
|
||||
json!({"action": "accept", "content": {}, "_meta": null}),
|
||||
for (response, reason) in [
|
||||
(json!({"invalid": "response"}), "invalidResponse"),
|
||||
(
|
||||
json!({"action": "accept", "content": null, "_meta": null}),
|
||||
"invalidProof",
|
||||
),
|
||||
(
|
||||
json!({"action": "accept", "content": {}, "_meta": null}),
|
||||
"invalidProof",
|
||||
),
|
||||
] {
|
||||
assert_eq!(
|
||||
from_client_result(Ok(Ok(response))),
|
||||
McpServerElicitationRequestResponse {
|
||||
action: McpServerElicitationAction::Cancel,
|
||||
content: None,
|
||||
meta: None,
|
||||
meta: Some(json!({"openai/userVerificationReason": reason})),
|
||||
}
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn terminal_response_keeps_only_a_known_reason() {
|
||||
for (input, expected) in [
|
||||
(
|
||||
json!({"openai/userVerificationReason": "timeout", "secret": "discard"}),
|
||||
Some(json!({"openai/userVerificationReason": "timeout"})),
|
||||
),
|
||||
(
|
||||
json!({"openai/userVerificationReason": "arbitrary secret"}),
|
||||
None,
|
||||
),
|
||||
(json!(null), None),
|
||||
] {
|
||||
assert_eq!(
|
||||
from_client_result(Ok(Ok(
|
||||
json!({"action": "cancel", "content": {"signature": "discard"}, "_meta": input})
|
||||
))),
|
||||
McpServerElicitationRequestResponse {
|
||||
action: McpServerElicitationAction::Cancel,
|
||||
content: None,
|
||||
meta: expected
|
||||
}
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn runtime_failures_keep_their_reason_without_error_text() {
|
||||
for (data, reason) in [
|
||||
(Some(json!({"reason": "turnTransition"})), "interrupted"),
|
||||
(None, "responseError"),
|
||||
] {
|
||||
let response = from_client_result(Ok(Err(codex_app_server_protocol::JSONRPCErrorError {
|
||||
code: -1,
|
||||
message: "private error details".into(),
|
||||
data,
|
||||
})));
|
||||
assert_eq!(
|
||||
response,
|
||||
McpServerElicitationRequestResponse {
|
||||
action: McpServerElicitationAction::Cancel,
|
||||
content: None,
|
||||
meta: Some(json!({"openai/userVerificationReason": reason}))
|
||||
}
|
||||
);
|
||||
}
|
||||
let (sender, receiver) = oneshot::channel();
|
||||
drop(sender);
|
||||
assert_eq!(
|
||||
from_client_result(receiver.await),
|
||||
McpServerElicitationRequestResponse {
|
||||
action: McpServerElicitationAction::Cancel,
|
||||
content: None,
|
||||
meta: Some(json!({"openai/userVerificationReason": "responseChannelClosed"}))
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
@@ -460,7 +460,10 @@ impl ElicitationRequestManager {
|
||||
return Ok(ElicitationResponse {
|
||||
action: ElicitationAction::Cancel,
|
||||
content: None,
|
||||
meta: None,
|
||||
meta: Some(
|
||||
codex_rmcp_client::UserVerificationReason::ApprovalUnavailable
|
||||
.into_meta(),
|
||||
),
|
||||
});
|
||||
}
|
||||
return user_verification_elicitation::route(
|
||||
|
||||
@@ -982,7 +982,7 @@ async fn user_verification_cancels_when_no_app_can_receive_the_request() {
|
||||
ElicitationResponse {
|
||||
action: ElicitationAction::Cancel,
|
||||
content: None,
|
||||
meta: None
|
||||
meta: Some(json!({"openai/userVerificationReason": "approvalUnavailable"}))
|
||||
},
|
||||
);
|
||||
}
|
||||
@@ -1016,7 +1016,7 @@ async fn user_verification_cancels_for_an_event_receiver_without_host_activation
|
||||
ElicitationResponse {
|
||||
action: ElicitationAction::Cancel,
|
||||
content: None,
|
||||
meta: None,
|
||||
meta: Some(json!({"openai/userVerificationReason": "approvalUnavailable"})),
|
||||
},
|
||||
);
|
||||
assert!(events.try_recv().is_err());
|
||||
@@ -1099,7 +1099,7 @@ async fn user_verification_rejects_attached_servers_even_if_they_use_the_plugin_
|
||||
ElicitationResponse {
|
||||
action: ElicitationAction::Cancel,
|
||||
content: None,
|
||||
meta: None
|
||||
meta: Some(json!({"openai/userVerificationReason": "approvalUnavailable"}))
|
||||
},
|
||||
);
|
||||
assert!(events.try_recv().is_err());
|
||||
|
||||
@@ -27,7 +27,7 @@ pub(super) async fn route(
|
||||
return Ok(ElicitationResponse {
|
||||
action: ElicitationAction::Cancel,
|
||||
content: None,
|
||||
meta: None,
|
||||
meta: Some(codex_rmcp_client::UserVerificationReason::ApprovalUnavailable.into_meta()),
|
||||
});
|
||||
};
|
||||
router
|
||||
|
||||
@@ -151,7 +151,14 @@ impl ElicitationClientService {
|
||||
return Ok(ElicitationResponse {
|
||||
action: ElicitationAction::Cancel,
|
||||
content: None,
|
||||
meta: None,
|
||||
meta: user_verification.then(|| {
|
||||
if cancellations.saturated {
|
||||
crate::UserVerificationReason::ApprovalUnavailable
|
||||
} else {
|
||||
crate::UserVerificationReason::Interrupted
|
||||
}
|
||||
.into_meta()
|
||||
}),
|
||||
});
|
||||
}
|
||||
cancellations.pending.insert(id.clone(), cancel_tx);
|
||||
@@ -167,14 +174,14 @@ impl ElicitationClientService {
|
||||
return Ok(ElicitationResponse {
|
||||
action: ElicitationAction::Cancel,
|
||||
content: None,
|
||||
meta: None,
|
||||
meta: user_verification.then(|| crate::UserVerificationReason::Interrupted.into_meta()),
|
||||
});
|
||||
}
|
||||
_ = cancel_rx => {
|
||||
return Ok(ElicitationResponse {
|
||||
action: ElicitationAction::Cancel,
|
||||
content: None,
|
||||
meta: None,
|
||||
meta: user_verification.then(|| crate::UserVerificationReason::Interrupted.into_meta()),
|
||||
});
|
||||
}
|
||||
response = (self.send_elicitation)(id, request) => response,
|
||||
|
||||
@@ -101,3 +101,4 @@ pub use startup_error::is_authentication_required_error;
|
||||
pub use stdio_server_launcher::ExecutorStdioServerLauncher;
|
||||
pub use stdio_server_launcher::LocalStdioServerLauncher;
|
||||
pub use stdio_server_launcher::StdioServerLauncher;
|
||||
pub use user_verification::UserVerificationReason;
|
||||
|
||||
@@ -5,12 +5,45 @@ use base64::engine::general_purpose::URL_SAFE_NO_PAD;
|
||||
use rmcp::model::CustomRequest;
|
||||
use rmcp::model::ElicitationAction;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
|
||||
use crate::rmcp_client::Elicitation;
|
||||
use crate::rmcp_client::ElicitationResponse;
|
||||
|
||||
pub(crate) const MODE: &str = "openai/userVerification";
|
||||
|
||||
/// Bounded diagnostic reason only; never a proof or an authorization decision.
|
||||
#[derive(Clone, Copy, Deserialize, Serialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub enum UserVerificationReason {
|
||||
UserCancelled,
|
||||
UserDeclined,
|
||||
CredentialMissing,
|
||||
BiometricsUnavailable,
|
||||
ProviderUnavailable,
|
||||
AuthenticationFailed,
|
||||
Timeout,
|
||||
ProviderError,
|
||||
ServiceError,
|
||||
InvalidParams,
|
||||
InvalidProof,
|
||||
InvalidResponse,
|
||||
ResponseError,
|
||||
ResponseChannelClosed,
|
||||
Interrupted,
|
||||
ApprovalUnavailable,
|
||||
}
|
||||
|
||||
impl UserVerificationReason {
|
||||
pub fn from_meta(meta: Option<&serde_json::Value>) -> Option<Self> {
|
||||
Self::deserialize(meta?.get("openai/userVerificationReason")?).ok()
|
||||
}
|
||||
|
||||
pub fn into_meta(self) -> serde_json::Value {
|
||||
serde_json::json!({"openai/userVerificationReason": self})
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
#[serde(rename_all = "camelCase", deny_unknown_fields)]
|
||||
struct RequestParams {
|
||||
@@ -53,6 +86,7 @@ pub(crate) fn parse_request(request: CustomRequest) -> Result<Elicitation, rmcp:
|
||||
|
||||
/// Accept only a bounded proof, and never return proof material for cancellation or rejection.
|
||||
pub(crate) fn validate_response(mut response: ElicitationResponse) -> ElicitationResponse {
|
||||
let reason = UserVerificationReason::from_meta(response.meta.as_ref());
|
||||
response.meta = None;
|
||||
match response.action {
|
||||
ElicitationAction::Accept => {
|
||||
@@ -69,9 +103,15 @@ pub(crate) fn validate_response(mut response: ElicitationResponse) -> Elicitatio
|
||||
}
|
||||
tracing::warn!("user-verification acceptance omitted a valid proof; cancelling");
|
||||
response.action = ElicitationAction::Cancel;
|
||||
response.meta = Some(UserVerificationReason::InvalidProof.into_meta());
|
||||
}
|
||||
ElicitationAction::Decline | ElicitationAction::Cancel => {
|
||||
response.meta = reason.map(UserVerificationReason::into_meta);
|
||||
}
|
||||
_ => {
|
||||
response.action = ElicitationAction::Cancel;
|
||||
response.meta = Some(UserVerificationReason::InvalidResponse.into_meta());
|
||||
}
|
||||
ElicitationAction::Decline | ElicitationAction::Cancel => {}
|
||||
_ => response.action = ElicitationAction::Cancel,
|
||||
}
|
||||
response.content = None;
|
||||
response
|
||||
|
||||
@@ -369,7 +369,10 @@ async fn cancelling_one_verification_leaves_the_mcp_connection_and_other_request
|
||||
anyhow::bail!("expected a cancellation response");
|
||||
};
|
||||
assert_eq!(cancel.id, first_id);
|
||||
assert_eq!(serde_json::to_value(cancel.result)?["action"], "cancel");
|
||||
assert_eq!(
|
||||
serde_json::to_value(cancel.result)?,
|
||||
json!({"action": "cancel", "_meta": {"openai/userVerificationReason": "interrupted"}})
|
||||
);
|
||||
|
||||
for request_id in [first_id, RequestId::Number(999)] {
|
||||
server
|
||||
|
||||
@@ -110,7 +110,9 @@ async fn user_verification_remembers_cancellation_before_request_handler_runs()
|
||||
assert_eq!(
|
||||
serde_json::to_value(response)?,
|
||||
json!({
|
||||
"jsonrpc": "2.0", "id": 1, "result": {"action": "cancel"}
|
||||
"jsonrpc": "2.0", "id": 1, "result": {
|
||||
"action": "cancel", "_meta": {"openai/userVerificationReason": "interrupted"}
|
||||
}
|
||||
})
|
||||
);
|
||||
assert!(
|
||||
@@ -159,7 +161,12 @@ async fn user_verification_early_cancellation_storage_fails_closed_at_capacity()
|
||||
})))),
|
||||
RequestContext::new(RequestId::Number(5000), client.peer().clone()),
|
||||
).await?;
|
||||
assert_eq!(serde_json::to_value(response)?, json!({"action": "cancel"}));
|
||||
assert_eq!(
|
||||
serde_json::to_value(response)?,
|
||||
json!({
|
||||
"action": "cancel", "_meta": {"openai/userVerificationReason": "approvalUnavailable"}
|
||||
})
|
||||
);
|
||||
// Ordinary requests still work after verification fails closed.
|
||||
server
|
||||
.send(serde_json::from_value::<ServerJsonRpcMessage>(json!({
|
||||
|
||||
@@ -131,12 +131,12 @@ fn user_verification_cancels_acceptance_without_a_valid_bounded_proof() {
|
||||
validate_response(ElicitationResponse {
|
||||
action: ElicitationAction::Accept,
|
||||
content,
|
||||
meta: None,
|
||||
meta: Some(json!({"openai/userVerificationReason": "userCancelled"})),
|
||||
}),
|
||||
ElicitationResponse {
|
||||
action: ElicitationAction::Cancel,
|
||||
content: None,
|
||||
meta: None,
|
||||
meta: Some(json!({"openai/userVerificationReason": "invalidProof"})),
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -149,12 +149,14 @@ fn user_verification_decline_and_cancel_discard_proof_material() {
|
||||
validate_response(ElicitationResponse {
|
||||
action: action.clone(),
|
||||
content: Some(json!({"credentialId": "AQID", "signature": "BAUG"})),
|
||||
meta: Some(json!({"secret": "discarded"})),
|
||||
meta: Some(
|
||||
json!({"secret": "discarded", "openai/userVerificationReason": "timeout"})
|
||||
),
|
||||
}),
|
||||
ElicitationResponse {
|
||||
action,
|
||||
content: None,
|
||||
meta: None
|
||||
meta: Some(json!({"openai/userVerificationReason": "timeout"}))
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
@@ -116,7 +116,10 @@ async fn native_mrtr_returns_validated_proof_or_cancellation_in_content() -> any
|
||||
(
|
||||
ElicitationAction::Accept,
|
||||
json!({"signature": "invalid"}),
|
||||
json!({"action": "cancel"}),
|
||||
json!({
|
||||
"action": "cancel",
|
||||
"_meta": {"openai/userVerificationReason": "invalidProof"},
|
||||
}),
|
||||
),
|
||||
] {
|
||||
let server = MockServer::start().await;
|
||||
|
||||
@@ -220,25 +220,39 @@ impl PendingAppServerRequests {
|
||||
request_id,
|
||||
response,
|
||||
} => {
|
||||
let (decision, content) = match response {
|
||||
let (decision, content, meta) = match response {
|
||||
crate::app_command::UserVerificationResponse::Accept { proof } => (
|
||||
codex_app_server_protocol::McpServerElicitationAction::Accept,
|
||||
Some(
|
||||
serde_json::to_value(proof)
|
||||
.map_err(|_| "Invalid verification proof".to_string())?,
|
||||
),
|
||||
None,
|
||||
),
|
||||
crate::app_command::UserVerificationResponse::Cancel => (
|
||||
codex_app_server_protocol::McpServerElicitationAction::Cancel,
|
||||
None,
|
||||
Some(serde_json::json!({"openai/userVerificationReason": "userCancelled"})),
|
||||
),
|
||||
crate::app_command::UserVerificationResponse::Failed { error } => {
|
||||
// Serialize only the closed protocol reason, never the display message.
|
||||
let error = serde_json::to_value(error)
|
||||
.map_err(|_| "Invalid verification error".to_string())?;
|
||||
(
|
||||
codex_app_server_protocol::McpServerElicitationAction::Cancel,
|
||||
None,
|
||||
Some(
|
||||
serde_json::json!({"openai/userVerificationReason": error["reason"]}),
|
||||
),
|
||||
)
|
||||
}
|
||||
};
|
||||
AppCommand::ResolveElicitation {
|
||||
server_name,
|
||||
request_id,
|
||||
decision,
|
||||
content,
|
||||
meta: None,
|
||||
meta,
|
||||
}
|
||||
}
|
||||
op => op,
|
||||
|
||||
@@ -14,11 +14,14 @@ use uuid::Uuid;
|
||||
|
||||
use super::App;
|
||||
use super::app_server_requests::ResolvedAppServerRequest;
|
||||
use super::user_verification_errors::verification_error_message;
|
||||
use super::user_verification_errors::verification_failure;
|
||||
use crate::app_command::AppCommand;
|
||||
use crate::app_command::UserVerificationFailure;
|
||||
use crate::app_command::UserVerificationResponse;
|
||||
use crate::app_event::AppEvent;
|
||||
use crate::app_server_session::AppServerSession;
|
||||
use codex_app_server_protocol::UserVerificationErrorDetails;
|
||||
use codex_app_server_protocol::UserVerificationUnavailableReason;
|
||||
|
||||
impl App {
|
||||
pub(super) async fn start_user_verification(
|
||||
@@ -47,7 +50,11 @@ impl App {
|
||||
thread_id,
|
||||
server_name,
|
||||
request_id,
|
||||
UserVerificationResponse::Cancel,
|
||||
UserVerificationResponse::Failed {
|
||||
error: UserVerificationErrorDetails::Unavailable {
|
||||
reason: UserVerificationUnavailableReason::ProviderUnavailable,
|
||||
},
|
||||
},
|
||||
);
|
||||
return Ok(());
|
||||
}
|
||||
@@ -74,7 +81,7 @@ impl App {
|
||||
result = verification => result,
|
||||
}
|
||||
.map(|response| response.proof)
|
||||
.map_err(|error| verification_error_message(&error).to_string());
|
||||
.map_err(|error| verification_failure(&error));
|
||||
app_event_tx.send(AppEvent::UserVerificationFinished {
|
||||
thread_id,
|
||||
server_name,
|
||||
@@ -94,7 +101,7 @@ impl App {
|
||||
server_name: String,
|
||||
request_id: RequestId,
|
||||
attempt_id: Uuid,
|
||||
result: Result<UserVerificationProof, String>,
|
||||
result: Result<UserVerificationProof, UserVerificationFailure>,
|
||||
) -> color_eyre::Result<()> {
|
||||
if self.abandoned_side_threads.contains(&thread_id)
|
||||
|| !self
|
||||
@@ -106,18 +113,20 @@ impl App {
|
||||
}
|
||||
let response = match result {
|
||||
Ok(proof) => UserVerificationResponse::Accept { proof },
|
||||
Err(message) => {
|
||||
Err(failure) => {
|
||||
self.enqueue_thread_notification(
|
||||
thread_id,
|
||||
ServerNotification::Warning(WarningNotification {
|
||||
thread_id: Some(thread_id.to_string()),
|
||||
message,
|
||||
message: failure.message.to_string(),
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
// Another trusted device may be supported later. For now every incomplete
|
||||
// verification cancels the original elicitation without a fallback provider.
|
||||
UserVerificationResponse::Cancel
|
||||
UserVerificationResponse::Failed {
|
||||
error: failure.details,
|
||||
}
|
||||
}
|
||||
};
|
||||
self.chat_widget
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
//! Safe user-facing messages for typed local verification failures.
|
||||
//! Keeps safe display messages and bounded categories for local verification failures.
|
||||
|
||||
use codex_app_server_client::TypedRequestError;
|
||||
use codex_app_server_protocol::UserVerificationCancellationReason;
|
||||
@@ -7,15 +7,22 @@ use codex_app_server_protocol::UserVerificationFailureReason;
|
||||
use codex_app_server_protocol::UserVerificationInvalidRequestReason;
|
||||
use codex_app_server_protocol::UserVerificationUnavailableReason;
|
||||
|
||||
pub(super) fn verification_error_message(error: &TypedRequestError) -> &'static str {
|
||||
use crate::app_command::UserVerificationFailure;
|
||||
|
||||
pub(super) fn verification_failure(error: &TypedRequestError) -> UserVerificationFailure {
|
||||
let TypedRequestError::Server { source, .. } = error else {
|
||||
return "Could not complete user verification with the local Codex binary.";
|
||||
return UserVerificationFailure {
|
||||
message: "Could not complete user verification with the local Codex binary.",
|
||||
details: UserVerificationErrorDetails::Failed {
|
||||
reason: UserVerificationFailureReason::ServiceError,
|
||||
},
|
||||
};
|
||||
};
|
||||
let details = source
|
||||
.data
|
||||
.clone()
|
||||
.and_then(|data| serde_json::from_value::<UserVerificationErrorDetails>(data).ok());
|
||||
match details {
|
||||
let message = match details {
|
||||
Some(UserVerificationErrorDetails::InvalidRequest {
|
||||
reason: UserVerificationInvalidRequestReason::InvalidParams,
|
||||
}) => "The local Codex binary could not verify this request.",
|
||||
@@ -47,6 +54,12 @@ pub(super) fn verification_error_message(error: &TypedRequestError) -> &'static
|
||||
reason: UserVerificationFailureReason::ServiceError,
|
||||
})
|
||||
| None => "The local Codex binary could not complete user verification.",
|
||||
};
|
||||
UserVerificationFailure {
|
||||
message,
|
||||
details: details.unwrap_or(UserVerificationErrorDetails::Failed {
|
||||
reason: UserVerificationFailureReason::ServiceError,
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -13,7 +13,7 @@ fn user_verification_errors_use_typed_reasons_without_raw_provider_diagnostics()
|
||||
},
|
||||
};
|
||||
assert_eq!(
|
||||
verification_error_message(&error),
|
||||
verification_failure(&error).message,
|
||||
"No user-verification credential is available in the local Codex binary."
|
||||
);
|
||||
let malformed = TypedRequestError::Server {
|
||||
@@ -27,7 +27,7 @@ fn user_verification_errors_use_typed_reasons_without_raw_provider_diagnostics()
|
||||
},
|
||||
};
|
||||
assert_eq!(
|
||||
verification_error_message(&malformed),
|
||||
verification_failure(&malformed).message,
|
||||
"The local Codex binary could not complete user verification."
|
||||
);
|
||||
}
|
||||
@@ -52,6 +52,6 @@ fn unavailable_reasons_use_device_and_session_neutral_copy() {
|
||||
data: Some(serde_json::json!({ "type": "unavailable", "reason": reason })),
|
||||
},
|
||||
};
|
||||
assert_eq!(verification_error_message(&error), expected);
|
||||
assert_eq!(verification_failure(&error).message, expected);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -62,7 +62,7 @@ fn user_verification_cancellation_invalidates_the_inflight_result() {
|
||||
resolution,
|
||||
Some(AppServerRequestResolution {
|
||||
request_id: request.id().clone(),
|
||||
result: serde_json::json!({ "action": "cancel", "content": null, "_meta": null }),
|
||||
result: serde_json::json!({ "action": "cancel", "content": null, "_meta": {"openai/userVerificationReason": "userCancelled"} }),
|
||||
})
|
||||
);
|
||||
assert!(
|
||||
|
||||
@@ -29,6 +29,11 @@ fn verification_request() -> ServerRequest {
|
||||
enum RpcScenario {
|
||||
Success,
|
||||
MethodNotFound,
|
||||
VerificationError {
|
||||
error_type: &'static str,
|
||||
reason: &'static str,
|
||||
expected_reason: &'static str,
|
||||
},
|
||||
CancelWhilePending,
|
||||
RemoteWorkspace,
|
||||
}
|
||||
@@ -56,6 +61,26 @@ async fn user_verification_remote_workspace_dismisses_the_waiting_prompt() -> co
|
||||
run_verification_rpc_scenario(RpcScenario::RemoteWorkspace).await
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn user_verification_controller_preserves_typed_error_reasons_in_the_rpc_response()
|
||||
-> color_eyre::Result<()> {
|
||||
for (error_type, reason, expected_reason) in [
|
||||
("invalidRequest", "invalidParams", "invalidParams"),
|
||||
("unavailable", "credentialMissing", "credentialMissing"),
|
||||
("cancelled", "userCancelled", "userCancelled"),
|
||||
("failed", "authenticationFailed", "authenticationFailed"),
|
||||
("unavailable", "private native failure", "serviceError"),
|
||||
] {
|
||||
run_verification_rpc_scenario(RpcScenario::VerificationError {
|
||||
error_type,
|
||||
reason,
|
||||
expected_reason,
|
||||
})
|
||||
.await?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn run_verification_rpc_scenario(scenario: RpcScenario) -> color_eyre::Result<()> {
|
||||
use codex_app_server_protocol::JSONRPCMessage;
|
||||
use tokio_tungstenite::tungstenite::Message;
|
||||
@@ -106,6 +131,13 @@ async fn run_verification_rpc_scenario(scenario: RpcScenario) -> color_eyre::Res
|
||||
}
|
||||
if scenario == RpcScenario::MethodNotFound {
|
||||
serde_json::json!({ "id": request.id, "error": { "code": -32601, "message": "unknown method: private server diagnostic" } })
|
||||
} else if let RpcScenario::VerificationError {
|
||||
error_type,
|
||||
reason,
|
||||
..
|
||||
} = scenario
|
||||
{
|
||||
serde_json::json!({ "id": request.id, "error": { "code": -32603, "message": "private native provider diagnostic", "data": { "type": error_type, "reason": reason } } })
|
||||
} else {
|
||||
serde_json::json!({ "id": request.id, "result": { "proof": proof } })
|
||||
}
|
||||
@@ -128,10 +160,22 @@ async fn run_verification_rpc_scenario(scenario: RpcScenario) -> color_eyre::Res
|
||||
);
|
||||
response_received = true;
|
||||
assert_eq!(response.id, RequestId::Integer(7));
|
||||
let expected = if scenario == RpcScenario::Success {
|
||||
serde_json::json!({ "action": "accept", "content": proof, "_meta": null })
|
||||
} else {
|
||||
serde_json::json!({ "action": "cancel", "content": null, "_meta": null })
|
||||
let expected = match scenario {
|
||||
RpcScenario::Success => {
|
||||
serde_json::json!({ "action": "accept", "content": proof, "_meta": null })
|
||||
}
|
||||
_ => {
|
||||
let reason = match scenario {
|
||||
RpcScenario::MethodNotFound => "serviceError",
|
||||
RpcScenario::CancelWhilePending => "userCancelled",
|
||||
RpcScenario::RemoteWorkspace => "providerUnavailable",
|
||||
RpcScenario::VerificationError {
|
||||
expected_reason, ..
|
||||
} => expected_reason,
|
||||
RpcScenario::Success => unreachable!(),
|
||||
};
|
||||
serde_json::json!({ "action": "cancel", "content": null, "_meta": {"openai/userVerificationReason": reason} })
|
||||
}
|
||||
};
|
||||
assert_eq!(response.result, expected);
|
||||
if let Some(id) = pending_verification_id.take() {
|
||||
@@ -217,7 +261,10 @@ async fn run_verification_rpc_scenario(scenario: RpcScenario) -> color_eyre::Res
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
if matches!(scenario, RpcScenario::Success | RpcScenario::MethodNotFound) {
|
||||
if matches!(
|
||||
scenario,
|
||||
RpcScenario::Success | RpcScenario::MethodNotFound | RpcScenario::VerificationError { .. }
|
||||
) {
|
||||
let completion =
|
||||
tokio::time::timeout(std::time::Duration::from_secs(/*secs*/ 5), event_rx.recv())
|
||||
.await?
|
||||
|
||||
@@ -9,6 +9,7 @@ use codex_app_server_protocol::RequestId as AppServerRequestId;
|
||||
use codex_app_server_protocol::ReviewTarget;
|
||||
use codex_app_server_protocol::ToolRequestUserInputResponse;
|
||||
use codex_app_server_protocol::UserInput;
|
||||
use codex_app_server_protocol::UserVerificationErrorDetails;
|
||||
use codex_app_server_protocol::UserVerificationProof;
|
||||
use codex_config::types::ApprovalsReviewer;
|
||||
use codex_protocol::ThreadId;
|
||||
@@ -196,6 +197,16 @@ pub(crate) enum UserVerificationResponse {
|
||||
proof: UserVerificationProof,
|
||||
},
|
||||
Cancel,
|
||||
Failed {
|
||||
error: UserVerificationErrorDetails,
|
||||
},
|
||||
}
|
||||
|
||||
/// Carries only a safe display message and the closed native error category.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub(crate) struct UserVerificationFailure {
|
||||
pub(crate) message: &'static str,
|
||||
pub(crate) details: UserVerificationErrorDetails,
|
||||
}
|
||||
|
||||
impl AppCommand {
|
||||
|
||||
@@ -440,7 +440,10 @@ pub(crate) enum AppEvent {
|
||||
server_name: String,
|
||||
request_id: AppServerRequestId,
|
||||
attempt_id: Uuid,
|
||||
result: Result<codex_app_server_protocol::UserVerificationProof, String>,
|
||||
result: Result<
|
||||
codex_app_server_protocol::UserVerificationProof,
|
||||
crate::app_command::UserVerificationFailure,
|
||||
>,
|
||||
},
|
||||
|
||||
/// Interrupt, fork, and retry a safety-buffered turn with the server-selected model.
|
||||
|
||||
Reference in New Issue
Block a user