[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.
This commit is contained in:
Owen Lin
GitHub
parent
b8ec97c0ef
commit
db2aa57d73
6 changed files with 56 additions and 65 deletions
|
|
@ -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::<crate::ClientNotification>(d, "ClientNotification"),
|
||||
|d| write_json_schema_with_return::<crate::ServerNotification>(d, "ServerNotification"),
|
||||
|d| write_json_schema_with_return::<EventMsg>(d, "EventMsg"),
|
||||
|d| write_json_schema_with_return::<FileChange>(d, "FileChange"),
|
||||
|d| write_json_schema_with_return::<crate::protocol::v1::InputItem>(d, "InputItem"),
|
||||
|d| write_json_schema_with_return::<ParsedCommand>(d, "ParsedCommand"),
|
||||
|d| write_json_schema_with_return::<SandboxPolicy>(d, "SandboxPolicy"),
|
||||
];
|
||||
|
||||
let mut schemas: Vec<GeneratedSchema> = Vec::new();
|
||||
|
|
@ -152,13 +145,10 @@ fn build_schema_bundle(schemas: Vec<GeneratedSchema>) -> Result<Value> {
|
|||
"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<GeneratedSchema>) -> Result<Value> {
|
|||
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<GeneratedSchema>) -> Result<Value> {
|
|||
&& 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<String> {
|
|||
});
|
||||
}
|
||||
|
||||
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()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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??;
|
||||
|
|
|
|||
|
|
@ -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 },
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue