chore: improve serialization of ServerNotification (#3193)

This PR introduces introduces a new
`OutgoingMessage::AppServerNotification` variant that is designed to
wrap a `ServerNotification`, which makes the serialization more
straightforward compared to
`OutgoingMessage::Notification(OutgoingNotification)`. We still use the
latter for serializing an `Event` as a `JSONRPCMessage::Notification`,
but I will try to get away from that in the near future.

With this change, now the generated TypeScript type for
`ServerNotification` is:

```typescript
export type ServerNotification =
  | { "method": "authStatusChange", "params": AuthStatusChangeNotification }
  | { "method": "loginChatGptComplete", "params": LoginChatGptCompleteNotification };
```

whereas before it was:

```typescript
export type ServerNotification =
  | { type: "auth_status_change"; data: AuthStatusChangeNotification }
  | { type: "login_chat_gpt_complete"; data: LoginChatGptCompleteNotification };
```

Once the `Event`s are migrated to the `ServerNotification` enum in Rust,
it should be considerably easier to work with notifications on the
TypeScript side, as it will be possible to `switch (message.method)` and
check for exhaustiveness.

Though we will probably need to introduce:

```typescript
export type ServerMessage = ServerRequest | ServerNotification;
```

and then we still need to group all of the `ServerResponse` types
together, as well.

codex-rs/mcp-server/src/codex_message_processor.rs

@@ -815,9 +815,9 @@ impl CodexMessageProcessor {
                         };
 
                         // For now, we send a notification for every event,
-                        // JSON-serializing the `Event` as-is, but we will move
-                        // to creating a special enum for notifications with a
-                        // stable wire format.
+                        // JSON-serializing the `Event` as-is, but these should
+                        // be migrated to be variants of `ServerNotification`
+                        // instead.
                         let method = format!("codex/event/{}", event.msg);
                         let mut params = match serde_json::to_value(event.clone()) {
                             Ok(serde_json::Value::Object(map)) => map,

codex-rs/mcp-server/src/outgoing_message.rs

@@ -97,6 +97,9 @@ impl OutgoingMessageSender {
         }
     }
 
+    /// This is used with the MCP server, but not the more general JSON-RPC app
+    /// server. Prefer [`OutgoingMessageSender::send_server_notification`] where
+    /// possible.
     pub(crate) async fn send_event_as_notification(
         &self,
         event: &Event,
@@ -123,14 +126,9 @@ impl OutgoingMessageSender {
     }
 
     pub(crate) async fn send_server_notification(&self, notification: ServerNotification) {
-        let method = format!("codex/event/{notification}");
-        let params = match serde_json::to_value(&notification) {
-            Ok(serde_json::Value::Object(mut map)) => map.remove("data"),
-            _ => None,
-        };
-        let outgoing_message =
-            OutgoingMessage::Notification(OutgoingNotification { method, params });
-        let _ = self.sender.send(outgoing_message);
+        let _ = self
+            .sender
+            .send(OutgoingMessage::AppServerNotification(notification));
     }
 
     pub(crate) async fn send_notification(&self, notification: OutgoingNotification) {
@@ -148,6 +146,9 @@ impl OutgoingMessageSender {
 pub(crate) enum OutgoingMessage {
     Request(OutgoingRequest),
     Notification(OutgoingNotification),
+    /// AppServerNotification is specific to the case where this is run as an
+    /// "app server" as opposed to an MCP server.
+    AppServerNotification(ServerNotification),
     Response(OutgoingResponse),
     Error(OutgoingError),
 }
@@ -171,6 +172,21 @@ impl From<OutgoingMessage> for JSONRPCMessage {
                     params,
                 })
             }
+            AppServerNotification(notification) => {
+                let method = notification.to_string();
+                let params = match notification.to_params() {
+                    Ok(params) => Some(params),
+                    Err(err) => {
+                        warn!("failed to serialize notification params: {err}");
+                        None
+                    }
+                };
+                JSONRPCMessage::Notification(JSONRPCNotification {
+                    jsonrpc: JSONRPC_VERSION.into(),
+                    method,
+                    params,
+                })
+            }
             Response(OutgoingResponse { id, result }) => {
                 JSONRPCMessage::Response(JSONRPCResponse {
                     jsonrpc: JSONRPC_VERSION.into(),
@@ -242,6 +258,7 @@ pub(crate) struct OutgoingError {
 mod tests {
     use codex_core::protocol::EventMsg;
     use codex_core::protocol::SessionConfiguredEvent;
+    use codex_protocol::mcp_protocol::LoginChatGptCompleteNotification;
     use pretty_assertions::assert_eq;
     use serde_json::json;
     use uuid::Uuid;
@@ -324,4 +341,29 @@ mod tests {
         });
         assert_eq!(params.unwrap(), expected_params);
     }
+
+    #[test]
+    fn verify_server_notification_serialization() {
+        let notification =
+            ServerNotification::LoginChatGptComplete(LoginChatGptCompleteNotification {
+                login_id: Uuid::nil(),
+                success: true,
+                error: None,
+            });
+
+        let jsonrpc_notification: JSONRPCMessage =
+            OutgoingMessage::AppServerNotification(notification).into();
+        assert_eq!(
+            JSONRPCMessage::Notification(JSONRPCNotification {
+                jsonrpc: "2.0".into(),
+                method: "loginChatGptComplete".into(),
+                params: Some(json!({
+                    "loginId": Uuid::nil(),
+                    "success": true,
+                })),
+            }),
+            jsonrpc_notification,
+            "ensure the strum macros serialize the method field correctly"
+        );
+    }
 }

codex-rs/protocol/src/mcp_protocol.rs

@@ -551,8 +551,8 @@ pub struct AuthStatusChangeNotification {
 }
 
 #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, TS, Display)]
-#[serde(tag = "type", content = "data", rename_all = "snake_case")]
-#[strum(serialize_all = "snake_case")]
+#[serde(tag = "method", content = "params", rename_all = "camelCase")]
+#[strum(serialize_all = "camelCase")]
 pub enum ServerNotification {
     /// Authentication status changed
     AuthStatusChange(AuthStatusChangeNotification),
@@ -561,6 +561,15 @@ pub enum ServerNotification {
     LoginChatGptComplete(LoginChatGptCompleteNotification),
 }
 
+impl ServerNotification {
+    pub fn to_params(self) -> Result<serde_json::Value, serde_json::Error> {
+        match self {
+            ServerNotification::AuthStatusChange(params) => serde_json::to_value(params),
+            ServerNotification::LoginChatGptComplete(params) => serde_json::to_value(params),
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;