mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Centralize JSON-RPC response serialization through payloads (#47089)
## What changed Serialize `ClientResponse` JSON-RPC results through the untagged `ClientResponsePayload` enum, and serialize payloads directly instead of matching on each variant. Preserve existing result shapes and request IDs. ## Testing Add regression tests covering v1 and v2 payloads with integer and string request IDs, legacy interrupt responses, and serialization errors from non-UTF-8 paths on Unix across typed, borrowed, and owned response conversions. GitOrigin-RevId: d46403b02f910a0d3e41a48a5bc6faa2581873e1
This commit is contained in:
@@ -319,13 +319,14 @@ macro_rules! client_request_definitions {
|
||||
pub fn into_jsonrpc_parts(
|
||||
self,
|
||||
) -> std::result::Result<(RequestId, crate::Result), serde_json::Error> {
|
||||
match self {
|
||||
let (request_id, response) = match self {
|
||||
$(
|
||||
Self::$variant { request_id, response } => {
|
||||
serde_json::to_value(response).map(|result| (request_id, result))
|
||||
(request_id, ClientResponsePayload::$variant(response))
|
||||
}
|
||||
)*
|
||||
}
|
||||
};
|
||||
serde_json::to_value(response).map(|result| (request_id, result))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -363,16 +364,7 @@ macro_rules! client_request_definitions {
|
||||
&self,
|
||||
request_id: RequestId,
|
||||
) -> std::result::Result<(RequestId, crate::Result), serde_json::Error> {
|
||||
match self {
|
||||
$(
|
||||
Self::$variant(response) => {
|
||||
serde_json::to_value(response).map(|result| (request_id, result))
|
||||
}
|
||||
)*
|
||||
Self::InterruptConversation(response) => {
|
||||
serde_json::to_value(response).map(|result| (request_id, result))
|
||||
}
|
||||
}
|
||||
serde_json::to_value(self).map(|result| (request_id, result))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -37,3 +37,107 @@ fn interrupt_conversation_payload_stays_jsonrpc_only() -> Result<()> {
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn client_response_jsonrpc_parts_preserve_payloads_and_request_ids() -> Result<()> {
|
||||
let payloads = [
|
||||
(
|
||||
ClientResponsePayload::GetAuthStatus(v1::GetAuthStatusResponse {
|
||||
auth_method: Some(AuthMode::Chatgpt),
|
||||
auth_token: None,
|
||||
requires_openai_auth: Some(true),
|
||||
}),
|
||||
json!({
|
||||
"authMethod": "chatgpt",
|
||||
"authToken": null,
|
||||
"requiresOpenaiAuth": true,
|
||||
}),
|
||||
),
|
||||
(
|
||||
ClientResponsePayload::GetAccount(v2::GetAccountResponse {
|
||||
account: Some(v2::Account::ApiKey {}),
|
||||
requires_openai_auth: false,
|
||||
workspace_routing: None,
|
||||
}),
|
||||
json!({
|
||||
"account": { "type": "apiKey" },
|
||||
"requiresOpenaiAuth": false,
|
||||
"workspaceRouting": null,
|
||||
}),
|
||||
),
|
||||
];
|
||||
|
||||
for (payload, expected_result) in payloads {
|
||||
for request_id in [RequestId::Integer(7), RequestId::String("request-7".into())] {
|
||||
let expected = (request_id.clone(), expected_result.clone());
|
||||
let response = payload
|
||||
.clone()
|
||||
.into_client_response(request_id.clone())
|
||||
.expect("request-backed payload has a typed response");
|
||||
|
||||
assert_eq!(response.into_jsonrpc_parts()?, expected);
|
||||
assert_eq!(payload.to_jsonrpc_parts(request_id.clone())?, expected);
|
||||
assert_eq!(payload.clone().into_jsonrpc_parts(request_id)?, expected);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn client_response_jsonrpc_parts_preserve_legacy_interrupt() -> Result<()> {
|
||||
let request_id = RequestId::String("interrupt-7".into());
|
||||
let payload = ClientResponsePayload::InterruptConversation(v1::InterruptConversationResponse {
|
||||
abort_reason: TurnAbortReason::Interrupted,
|
||||
});
|
||||
let expected = (request_id.clone(), json!({ "abortReason": "interrupted" }));
|
||||
|
||||
assert!(
|
||||
payload
|
||||
.clone()
|
||||
.into_client_response(request_id.clone())
|
||||
.is_none()
|
||||
);
|
||||
assert_eq!(payload.to_jsonrpc_parts(request_id.clone())?, expected);
|
||||
assert_eq!(payload.into_jsonrpc_parts(request_id)?, expected);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn client_response_jsonrpc_parts_preserve_serialization_errors() {
|
||||
use std::ffi::OsString;
|
||||
use std::os::unix::ffi::OsStringExt;
|
||||
|
||||
let request_id = RequestId::Integer(7);
|
||||
let response = v1::GetConversationSummaryResponse {
|
||||
summary: v1::ConversationSummary {
|
||||
conversation_id: codex_protocol::ThreadId::from_u128(/*value*/ 7),
|
||||
path: PathBuf::from(OsString::from_vec(vec![0xff])),
|
||||
preview: String::new(),
|
||||
timestamp: None,
|
||||
updated_at: None,
|
||||
model_provider: String::new(),
|
||||
cwd: PathBuf::new(),
|
||||
cli_version: String::new(),
|
||||
source: codex_protocol::protocol::SessionSource::Exec,
|
||||
git_info: None,
|
||||
},
|
||||
};
|
||||
let describe = |error: serde_json::Error| (error.classify(), error.to_string());
|
||||
let expected = describe(serde_json::to_value(&response).unwrap_err());
|
||||
let payload = ClientResponsePayload::GetConversationSummary(response.clone());
|
||||
let typed = ClientResponse::GetConversationSummary {
|
||||
request_id: request_id.clone(),
|
||||
response,
|
||||
};
|
||||
|
||||
assert_eq!(describe(typed.into_jsonrpc_parts().unwrap_err()), expected);
|
||||
assert_eq!(
|
||||
describe(payload.to_jsonrpc_parts(request_id.clone()).unwrap_err()),
|
||||
expected
|
||||
);
|
||||
assert_eq!(
|
||||
describe(payload.into_jsonrpc_parts(request_id).unwrap_err()),
|
||||
expected
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user