Skip to content

Commit db2aa57

Browse files
authored
[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.
1 parent b8ec97c commit db2aa57

File tree

6 files changed

+56
-65
lines changed

6 files changed

+56
-65
lines changed

codex-rs/app-server-protocol/src/export.rs

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,7 @@ use crate::export_server_responses;
1313
use anyhow::Context;
1414
use anyhow::Result;
1515
use anyhow::anyhow;
16-
use codex_protocol::parse_command::ParsedCommand;
1716
use codex_protocol::protocol::EventMsg;
18-
use codex_protocol::protocol::FileChange;
19-
use codex_protocol::protocol::SandboxPolicy;
2017
use schemars::JsonSchema;
2118
use schemars::schema_for;
2219
use serde::Serialize;
@@ -120,10 +117,6 @@ pub fn generate_json(out_dir: &Path) -> Result<()> {
120117
|d| write_json_schema_with_return::<crate::ClientNotification>(d, "ClientNotification"),
121118
|d| write_json_schema_with_return::<crate::ServerNotification>(d, "ServerNotification"),
122119
|d| write_json_schema_with_return::<EventMsg>(d, "EventMsg"),
123-
|d| write_json_schema_with_return::<FileChange>(d, "FileChange"),
124-
|d| write_json_schema_with_return::<crate::protocol::v1::InputItem>(d, "InputItem"),
125-
|d| write_json_schema_with_return::<ParsedCommand>(d, "ParsedCommand"),
126-
|d| write_json_schema_with_return::<SandboxPolicy>(d, "SandboxPolicy"),
127120
];
128121

129122
let mut schemas: Vec<GeneratedSchema> = Vec::new();
@@ -152,13 +145,10 @@ fn build_schema_bundle(schemas: Vec<GeneratedSchema>) -> Result<Value> {
152145
"ClientNotification",
153146
"ClientRequest",
154147
"EventMsg",
155-
"FileChange",
156-
"InputItem",
157-
"ParsedCommand",
158-
"SandboxPolicy",
159148
"ServerNotification",
160149
"ServerRequest",
161150
];
151+
const IGNORED_DEFINITIONS: &[&str] = &["Option<()>"];
162152

163153
let namespaced_types = collect_namespaced_types(&schemas);
164154
let mut definitions = Map::new();
@@ -171,6 +161,10 @@ fn build_schema_bundle(schemas: Vec<GeneratedSchema>) -> Result<Value> {
171161
in_v1_dir,
172162
} = schema;
173163

164+
if IGNORED_DEFINITIONS.contains(&logical_name.as_str()) {
165+
continue;
166+
}
167+
174168
if let Some(ref ns) = namespace {
175169
rewrite_refs_to_namespace(&mut value, ns);
176170
}
@@ -181,6 +175,9 @@ fn build_schema_bundle(schemas: Vec<GeneratedSchema>) -> Result<Value> {
181175
&& let Value::Object(defs_obj) = defs
182176
{
183177
for (def_name, mut def_schema) in defs_obj {
178+
if IGNORED_DEFINITIONS.contains(&def_name.as_str()) {
179+
continue;
180+
}
184181
if SPECIAL_DEFINITIONS.contains(&def_name.as_str()) {
185182
continue;
186183
}
@@ -386,14 +383,6 @@ fn variant_definition_name(base: &str, variant: &Value) -> Option<String> {
386383
});
387384
}
388385

389-
if let Some(mode_literal) = literal_from_property(props, "mode") {
390-
let pascal = to_pascal_case(mode_literal);
391-
return Some(match base {
392-
"SandboxPolicy" => format!("{pascal}SandboxPolicy"),
393-
_ => format!("{pascal}{base}"),
394-
});
395-
}
396-
397386
if props.len() == 1
398387
&& let Some(key) = props.keys().next()
399388
{

codex-rs/app-server-protocol/src/protocol/v2.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ v2_enum_from_core!(
5757
);
5858

5959
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)]
60-
#[serde(tag = "mode", rename_all = "camelCase")]
61-
#[ts(tag = "mode")]
60+
#[serde(tag = "type", rename_all = "camelCase")]
61+
#[ts(tag = "type")]
6262
#[ts(export_to = "v2/")]
6363
pub enum SandboxPolicy {
6464
DangerFullAccess,

codex-rs/app-server/tests/suite/v2/thread_list.rs

Lines changed: 7 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,8 @@ use codex_app_server_protocol::JSONRPCResponse;
66
use codex_app_server_protocol::RequestId;
77
use codex_app_server_protocol::ThreadListParams;
88
use codex_app_server_protocol::ThreadListResponse;
9-
use serde_json::json;
109
use tempfile::TempDir;
1110
use tokio::time::timeout;
12-
use uuid::Uuid;
1311

1412
const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
1513

@@ -150,46 +148,13 @@ async fn thread_list_respects_provider_filter() -> Result<()> {
150148
"X",
151149
Some("mock_provider"),
152150
)?; // mock_provider
153-
// one with a different provider
154-
let uuid = Uuid::new_v4();
155-
let dir = codex_home
156-
.path()
157-
.join("sessions")
158-
.join("2025")
159-
.join("01")
160-
.join("02");
161-
std::fs::create_dir_all(&dir)?;
162-
let file_path = dir.join(format!("rollout-2025-01-02T11-00-00-{uuid}.jsonl"));
163-
let lines = [
164-
json!({
165-
"timestamp": "2025-01-02T11:00:00Z",
166-
"type": "session_meta",
167-
"payload": {
168-
"id": uuid,
169-
"timestamp": "2025-01-02T11:00:00Z",
170-
"cwd": "/",
171-
"originator": "codex",
172-
"cli_version": "0.0.0",
173-
"instructions": null,
174-
"source": "vscode",
175-
"model_provider": "other_provider"
176-
}
177-
})
178-
.to_string(),
179-
json!({
180-
"timestamp": "2025-01-02T11:00:00Z",
181-
"type":"response_item",
182-
"payload": {"type":"message","role":"user","content":[{"type":"input_text","text":"X"}]}
183-
})
184-
.to_string(),
185-
json!({
186-
"timestamp": "2025-01-02T11:00:00Z",
187-
"type":"event_msg",
188-
"payload": {"type":"user_message","message":"X","kind":"plain"}
189-
})
190-
.to_string(),
191-
];
192-
std::fs::write(file_path, lines.join("\n") + "\n")?;
151+
let _b = create_fake_rollout(
152+
codex_home.path(),
153+
"2025-01-02T11-00-00",
154+
"2025-01-02T11:00:00Z",
155+
"X",
156+
Some("other_provider"),
157+
)?;
193158

194159
let mut mcp = McpProcess::new(codex_home.path()).await?;
195160
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;

codex-rs/protocol/src/items.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ use serde::Serialize;
1111
use ts_rs::TS;
1212

1313
#[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema)]
14+
#[serde(tag = "type")]
15+
#[ts(tag = "type")]
1416
pub enum TurnItem {
1517
UserMessage(UserMessageItem),
1618
AgentMessage(AgentMessageItem),
@@ -25,6 +27,8 @@ pub struct UserMessageItem {
2527
}
2628

2729
#[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema)]
30+
#[serde(tag = "type")]
31+
#[ts(tag = "type")]
2832
pub enum AgentMessageContent {
2933
Text { text: String },
3034
}

codex-rs/protocol/src/protocol.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ pub enum AskForApproval {
241241
/// Determines execution restrictions for model shell commands.
242242
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Display, JsonSchema, TS)]
243243
#[strum(serialize_all = "kebab-case")]
244-
#[serde(tag = "mode", rename_all = "kebab-case")]
244+
#[serde(tag = "type", rename_all = "kebab-case")]
245245
pub enum SandboxPolicy {
246246
/// No restrictions whatsoever. Use with caution.
247247
#[serde(rename = "danger-full-access")]
@@ -432,6 +432,7 @@ pub struct Event {
432432
/// NOTE: Make sure none of these values have optional types, as it will mess up the extension code-gen.
433433
#[derive(Debug, Clone, Deserialize, Serialize, Display, JsonSchema, TS)]
434434
#[serde(tag = "type", rename_all = "snake_case")]
435+
#[ts(tag = "type")]
435436
#[strum(serialize_all = "snake_case")]
436437
pub enum EventMsg {
437438
/// Error while executing a submission
@@ -1443,7 +1444,8 @@ pub enum ReviewDecision {
14431444
}
14441445

14451446
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, JsonSchema, TS)]
1446-
#[serde(rename_all = "snake_case")]
1447+
#[serde(tag = "type", rename_all = "snake_case")]
1448+
#[ts(tag = "type")]
14471449
pub enum FileChange {
14481450
Add {
14491451
content: String,

codex-rs/tui/src/chatwidget/tests.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,37 @@ fn upgrade_event_payload_for_tests(mut payload: serde_json::Value) -> serde_json
100100
serde_json::Value::String(aggregated),
101101
);
102102
}
103+
} else if ty == "patch_apply_begin"
104+
&& let Some(changes) = m.get_mut("changes").and_then(|v| v.as_object_mut())
105+
{
106+
for change in changes.values_mut() {
107+
if change.get("type").is_some() {
108+
continue;
109+
}
110+
if let Some(change_obj) = change.as_object_mut()
111+
&& change_obj.len() == 1
112+
&& let Some((legacy_kind, legacy_value)) = change_obj
113+
.iter()
114+
.next()
115+
.map(|(k, v)| (k.clone(), v.clone()))
116+
{
117+
change_obj.clear();
118+
change_obj.insert(
119+
"type".to_string(),
120+
serde_json::Value::String(legacy_kind.clone()),
121+
);
122+
match legacy_value {
123+
serde_json::Value::Object(payload) => {
124+
for (k, v) in payload {
125+
change_obj.insert(k, v);
126+
}
127+
}
128+
other => {
129+
change_obj.insert("content".to_string(), other);
130+
}
131+
}
132+
}
133+
}
103134
}
104135
}
105136
payload

0 commit comments

Comments
 (0)