From db2aa57d73b265acc4f14b67c2cad0ab620dd49e Mon Sep 17 00:00:00 2001 From: Owen Lin Date: Thu, 13 Nov 2025 16:25:17 -0800 Subject: [PATCH] [app-server] small fixes for JSON schema export and one-of types (#6614) A partner is consuming our generated JSON schema bundle for app-server and identified a few issues: - not all polymorphic / one-of types have a type descriminator - `"$ref": "#/definitions/v2/SandboxPolicy"` is missing - "Option<>" is an invalid schema name, and also unnecessary This PR: - adds the type descriminator to the various types that are missing it except for `SessionSource` and `SubAgentSource` because they are serialized to disk (adding this would break backwards compat for resume), and they should not be necessary to consume for an integration with app-server. - removes the special handling in `export.rs` of various types like SandboxPolicy, which turned out to be unnecessary and incorrect - filters out `Option<>` which was auto-generated for request params that don't need a body For context, we currently pull in wayyy more types than we need through the `EventMsg` god object which we are **not** planning to expose in API v2 (this is how I suspect `SessionSource` and `SubAgentSource` are being pulled in). But until we have all the necessary v2 notifications in place that will allow us to remove `EventMsg`, we will keep exporting it for now. --- codex-rs/app-server-protocol/src/export.rs | 27 +++------- .../app-server-protocol/src/protocol/v2.rs | 4 +- .../app-server/tests/suite/v2/thread_list.rs | 49 +++---------------- codex-rs/protocol/src/items.rs | 4 ++ codex-rs/protocol/src/protocol.rs | 6 ++- codex-rs/tui/src/chatwidget/tests.rs | 31 ++++++++++++ 6 files changed, 56 insertions(+), 65 deletions(-) diff --git a/codex-rs/app-server-protocol/src/export.rs b/codex-rs/app-server-protocol/src/export.rs index a1408d61d..11296e8e5 100644 --- a/codex-rs/app-server-protocol/src/export.rs +++ b/codex-rs/app-server-protocol/src/export.rs @@ -13,10 +13,7 @@ use crate::export_server_responses; use anyhow::Context; use anyhow::Result; use anyhow::anyhow; -use codex_protocol::parse_command::ParsedCommand; use codex_protocol::protocol::EventMsg; -use codex_protocol::protocol::FileChange; -use codex_protocol::protocol::SandboxPolicy; use schemars::JsonSchema; use schemars::schema_for; use serde::Serialize; @@ -120,10 +117,6 @@ pub fn generate_json(out_dir: &Path) -> Result<()> { |d| write_json_schema_with_return::(d, "ClientNotification"), |d| write_json_schema_with_return::(d, "ServerNotification"), |d| write_json_schema_with_return::(d, "EventMsg"), - |d| write_json_schema_with_return::(d, "FileChange"), - |d| write_json_schema_with_return::(d, "InputItem"), - |d| write_json_schema_with_return::(d, "ParsedCommand"), - |d| write_json_schema_with_return::(d, "SandboxPolicy"), ]; let mut schemas: Vec = Vec::new(); @@ -152,13 +145,10 @@ fn build_schema_bundle(schemas: Vec) -> Result { "ClientNotification", "ClientRequest", "EventMsg", - "FileChange", - "InputItem", - "ParsedCommand", - "SandboxPolicy", "ServerNotification", "ServerRequest", ]; + const IGNORED_DEFINITIONS: &[&str] = &["Option<()>"]; let namespaced_types = collect_namespaced_types(&schemas); let mut definitions = Map::new(); @@ -171,6 +161,10 @@ fn build_schema_bundle(schemas: Vec) -> Result { in_v1_dir, } = schema; + if IGNORED_DEFINITIONS.contains(&logical_name.as_str()) { + continue; + } + if let Some(ref ns) = namespace { rewrite_refs_to_namespace(&mut value, ns); } @@ -181,6 +175,9 @@ fn build_schema_bundle(schemas: Vec) -> Result { && let Value::Object(defs_obj) = defs { for (def_name, mut def_schema) in defs_obj { + if IGNORED_DEFINITIONS.contains(&def_name.as_str()) { + continue; + } if SPECIAL_DEFINITIONS.contains(&def_name.as_str()) { continue; } @@ -386,14 +383,6 @@ fn variant_definition_name(base: &str, variant: &Value) -> Option { }); } - if let Some(mode_literal) = literal_from_property(props, "mode") { - let pascal = to_pascal_case(mode_literal); - return Some(match base { - "SandboxPolicy" => format!("{pascal}SandboxPolicy"), - _ => format!("{pascal}{base}"), - }); - } - if props.len() == 1 && let Some(key) = props.keys().next() { diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 4642798c6..cc74a8de7 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -57,8 +57,8 @@ v2_enum_from_core!( ); #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] -#[serde(tag = "mode", rename_all = "camelCase")] -#[ts(tag = "mode")] +#[serde(tag = "type", rename_all = "camelCase")] +#[ts(tag = "type")] #[ts(export_to = "v2/")] pub enum SandboxPolicy { DangerFullAccess, diff --git a/codex-rs/app-server/tests/suite/v2/thread_list.rs b/codex-rs/app-server/tests/suite/v2/thread_list.rs index 09ef0ebfd..464fb4eee 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_list.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_list.rs @@ -6,10 +6,8 @@ use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::ThreadListParams; use codex_app_server_protocol::ThreadListResponse; -use serde_json::json; use tempfile::TempDir; use tokio::time::timeout; -use uuid::Uuid; const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); @@ -150,46 +148,13 @@ async fn thread_list_respects_provider_filter() -> Result<()> { "X", Some("mock_provider"), )?; // mock_provider - // one with a different provider - let uuid = Uuid::new_v4(); - let dir = codex_home - .path() - .join("sessions") - .join("2025") - .join("01") - .join("02"); - std::fs::create_dir_all(&dir)?; - let file_path = dir.join(format!("rollout-2025-01-02T11-00-00-{uuid}.jsonl")); - let lines = [ - json!({ - "timestamp": "2025-01-02T11:00:00Z", - "type": "session_meta", - "payload": { - "id": uuid, - "timestamp": "2025-01-02T11:00:00Z", - "cwd": "/", - "originator": "codex", - "cli_version": "0.0.0", - "instructions": null, - "source": "vscode", - "model_provider": "other_provider" - } - }) - .to_string(), - json!({ - "timestamp": "2025-01-02T11:00:00Z", - "type":"response_item", - "payload": {"type":"message","role":"user","content":[{"type":"input_text","text":"X"}]} - }) - .to_string(), - json!({ - "timestamp": "2025-01-02T11:00:00Z", - "type":"event_msg", - "payload": {"type":"user_message","message":"X","kind":"plain"} - }) - .to_string(), - ]; - std::fs::write(file_path, lines.join("\n") + "\n")?; + let _b = create_fake_rollout( + codex_home.path(), + "2025-01-02T11-00-00", + "2025-01-02T11:00:00Z", + "X", + Some("other_provider"), + )?; let mut mcp = McpProcess::new(codex_home.path()).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; diff --git a/codex-rs/protocol/src/items.rs b/codex-rs/protocol/src/items.rs index 27a9b9474..36ee0be07 100644 --- a/codex-rs/protocol/src/items.rs +++ b/codex-rs/protocol/src/items.rs @@ -11,6 +11,8 @@ use serde::Serialize; use ts_rs::TS; #[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema)] +#[serde(tag = "type")] +#[ts(tag = "type")] pub enum TurnItem { UserMessage(UserMessageItem), AgentMessage(AgentMessageItem), @@ -25,6 +27,8 @@ pub struct UserMessageItem { } #[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema)] +#[serde(tag = "type")] +#[ts(tag = "type")] pub enum AgentMessageContent { Text { text: String }, } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 2b9b43d79..95e64089c 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -241,7 +241,7 @@ pub enum AskForApproval { /// Determines execution restrictions for model shell commands. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Display, JsonSchema, TS)] #[strum(serialize_all = "kebab-case")] -#[serde(tag = "mode", rename_all = "kebab-case")] +#[serde(tag = "type", rename_all = "kebab-case")] pub enum SandboxPolicy { /// No restrictions whatsoever. Use with caution. #[serde(rename = "danger-full-access")] @@ -432,6 +432,7 @@ pub struct Event { /// NOTE: Make sure none of these values have optional types, as it will mess up the extension code-gen. #[derive(Debug, Clone, Deserialize, Serialize, Display, JsonSchema, TS)] #[serde(tag = "type", rename_all = "snake_case")] +#[ts(tag = "type")] #[strum(serialize_all = "snake_case")] pub enum EventMsg { /// Error while executing a submission @@ -1443,7 +1444,8 @@ pub enum ReviewDecision { } #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, JsonSchema, TS)] -#[serde(rename_all = "snake_case")] +#[serde(tag = "type", rename_all = "snake_case")] +#[ts(tag = "type")] pub enum FileChange { Add { content: String, diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 217e0fb30..f50ba9e9a 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -100,6 +100,37 @@ fn upgrade_event_payload_for_tests(mut payload: serde_json::Value) -> serde_json serde_json::Value::String(aggregated), ); } + } else if ty == "patch_apply_begin" + && let Some(changes) = m.get_mut("changes").and_then(|v| v.as_object_mut()) + { + for change in changes.values_mut() { + if change.get("type").is_some() { + continue; + } + if let Some(change_obj) = change.as_object_mut() + && change_obj.len() == 1 + && let Some((legacy_kind, legacy_value)) = change_obj + .iter() + .next() + .map(|(k, v)| (k.clone(), v.clone())) + { + change_obj.clear(); + change_obj.insert( + "type".to_string(), + serde_json::Value::String(legacy_kind.clone()), + ); + match legacy_value { + serde_json::Value::Object(payload) => { + for (k, v) in payload { + change_obj.insert(k, v); + } + } + other => { + change_obj.insert("content".to_string(), other); + } + } + } + } } } payload