core/core-agent-ide
8
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: remove serde(flatten) annotation for TurnError (#7499)

Browse source 
The problem with using `serde(flatten)` on Turn status is that it
conditionally serializes the `error` field, which is not the pattern we
want in API v2 where all fields on an object should always be returned.

```
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct Turn {
    pub id: String,
    /// Only populated on a `thread/resume` response.
    /// For all other responses and notifications returning a Turn,
    /// the items field will be an empty list.
    pub items: Vec<ThreadItem>,
    #[serde(flatten)]
    pub status: TurnStatus,
}

#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[serde(tag = "status", rename_all = "camelCase")]
#[ts(tag = "status", export_to = "v2/")]
pub enum TurnStatus {
    Completed,
    Interrupted,
    Failed { error: TurnError },
    InProgress,
}
```

serializes to:
```
{
  "id": "turn-123",
  "items": [],
  "status": "completed"
}

{
  "id": "turn-123",
  "items": [],
  "status": "failed",
  "error": {
    "message": "Tool timeout",
    "codexErrorInfo": null
  }
}
```

Instead we want:
```
{
  "id": "turn-123",
  "items": [],
  "status": "completed",
  "error": null
}

{
  "id": "turn-123",
  "items": [],
  "status": "failed",
  "error": {
    "message": "Tool timeout",
    "codexErrorInfo": null
  }
}
```
This commit is contained in:
Owen Lin 2025-12-02 13:39:10 -08:00 • committed by GitHub
parent 5ebdc9af1b
commit 77c457121e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 42 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
codex-rs/app-server-protocol/src/protocol/thread_history.rs
View file

@ -1,5 +1,6 @@
use crate::protocol::v2::ThreadItem;
use crate::protocol::v2::Turn;
use crate::protocol::v2::TurnError;
use crate::protocol::v2::TurnStatus;
use crate::protocol::v2::UserInput;
use codex_protocol::protocol::AgentReasoningEvent;
@ -142,6 +143,7 @@ impl ThreadHistoryBuilder {
PendingTurn {
id: self.next_turn_id(),
items: Vec::new(),
error: None,
status: TurnStatus::Completed,
}
}
@ -190,6 +192,7 @@ impl ThreadHistoryBuilder {
struct PendingTurn {
id: String,
items: Vec<ThreadItem>,
error: Option<TurnError>,
status: TurnStatus,
}
@ -198,6 +201,7 @@ impl From<PendingTurn> for Turn {
Self {
id: value.id,
items: value.items,
error: value.error,
status: value.status,
}
}

9
codex-rs/app-server-protocol/src/protocol/v2.rs
View file

@ -877,8 +877,9 @@ pub struct Turn {
/// For all other responses and notifications returning a Turn,
/// the items field will be an empty list.
pub items: Vec<ThreadItem>,
#[serde(flatten)]
pub status: TurnStatus,
/// Only populated when the Turn's status is failed.
pub error: Option<TurnError>,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, Error)]
@ -900,12 +901,12 @@ pub struct ErrorNotification {
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[serde(tag = "status", rename_all = "camelCase")]
#[ts(tag = "status", export_to = "v2/")]
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub enum TurnStatus {
Completed,
Interrupted,
Failed { error: TurnError },
Failed,
InProgress,
}

4
codex-rs/app-server-test-client/src/main.rs
View file

@ -563,7 +563,9 @@ impl CodexClient {
ServerNotification::TurnCompleted(payload) => {
if payload.turn.id == turn_id {
println!("\n< turn/completed notification: {:?}", payload.turn.status);
if let TurnStatus::Failed { error } = &payload.turn.status {
if payload.turn.status == TurnStatus::Failed
&& let Some(error) = payload.turn.error
{
println!("[turn error] {}", error.message);
}
break;

54
codex-rs/app-server/src/bespoke_event_handling.rs
View file

@ -718,6 +718,7 @@ async fn emit_turn_completed_with_status(
conversation_id: ConversationId,
event_turn_id: String,
status: TurnStatus,
error: Option<TurnError>,
outgoing: &OutgoingMessageSender,
) {
let notification = TurnCompletedNotification {
@ -725,6 +726,7 @@ async fn emit_turn_completed_with_status(
turn: Turn {
id: event_turn_id,
items: vec![],
error,
status,
},
};
@ -813,13 +815,12 @@ async fn handle_turn_complete(
) {
let turn_summary = find_and_remove_turn_summary(conversation_id, turn_summary_store).await;
let status = if let Some(error) = turn_summary.last_error {
TurnStatus::Failed { error }
} else {
TurnStatus::Completed
let (status, error) = match turn_summary.last_error {
Some(error) => (TurnStatus::Failed, Some(error)),
None => (TurnStatus::Completed, None),
};
emit_turn_completed_with_status(conversation_id, event_turn_id, status, outgoing).await;
emit_turn_completed_with_status(conversation_id, event_turn_id, status, error, outgoing).await;
}
async fn handle_turn_interrupted(
@ -834,6 +835,7 @@ async fn handle_turn_interrupted(
conversation_id,
event_turn_id,
TurnStatus::Interrupted,
None,
outgoing,
)
.await;
@ -1306,6 +1308,7 @@ mod tests {
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
assert_eq!(n.turn.id, event_turn_id);
assert_eq!(n.turn.status, TurnStatus::Completed);
assert_eq!(n.turn.error, None);
}
other => bail!("unexpected message: {other:?}"),
}
@ -1346,6 +1349,7 @@ mod tests {
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
assert_eq!(n.turn.id, event_turn_id);
assert_eq!(n.turn.status, TurnStatus::Interrupted);
assert_eq!(n.turn.error, None);
}
other => bail!("unexpected message: {other:?}"),
}
@ -1385,14 +1389,13 @@ mod tests {
match msg {
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
assert_eq!(n.turn.id, event_turn_id);
assert_eq!(n.turn.status, TurnStatus::Failed);
assert_eq!(
n.turn.status,
TurnStatus::Failed {
error: TurnError {
message: "bad".to_string(),
codex_error_info: Some(V2CodexErrorInfo::Other),
}
}
n.turn.error,
Some(TurnError {
message: "bad".to_string(),
codex_error_info: Some(V2CodexErrorInfo::Other),
})
);
}
other => bail!("unexpected message: {other:?}"),
@ -1653,14 +1656,13 @@ mod tests {
match msg {
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
assert_eq!(n.turn.id, a_turn1);
assert_eq!(n.turn.status, TurnStatus::Failed);
assert_eq!(
n.turn.status,
TurnStatus::Failed {
error: TurnError {
message: "a1".to_string(),
codex_error_info: Some(V2CodexErrorInfo::BadRequest),
}
}
n.turn.error,
Some(TurnError {
message: "a1".to_string(),
codex_error_info: Some(V2CodexErrorInfo::BadRequest),
})
);
}
other => bail!("unexpected message: {other:?}"),
@ -1674,14 +1676,13 @@ mod tests {
match msg {
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
assert_eq!(n.turn.id, b_turn1);
assert_eq!(n.turn.status, TurnStatus::Failed);
assert_eq!(
n.turn.status,
TurnStatus::Failed {
error: TurnError {
message: "b1".to_string(),
codex_error_info: None,
}
}
n.turn.error,
Some(TurnError {
message: "b1".to_string(),
codex_error_info: None,
})
);
}
other => bail!("unexpected message: {other:?}"),
@ -1696,6 +1697,7 @@ mod tests {
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
assert_eq!(n.turn.id, a_turn2);
assert_eq!(n.turn.status, TurnStatus::Completed);
assert_eq!(n.turn.error, None);
}
other => bail!("unexpected message: {other:?}"),
}

2
codex-rs/app-server/src/codex_message_processor.rs
View file

@ -2455,6 +2455,7 @@ impl CodexMessageProcessor {
let turn = Turn {
id: turn_id.clone(),
items: vec![],
error: None,
status: TurnStatus::InProgress,
};
@ -2496,6 +2497,7 @@ impl CodexMessageProcessor {
Turn {
id: turn_id,
items,
error: None,
status: TurnStatus::InProgress,
}
}