[app-server] remove serde(skip_serializing_if = "Option::is_none") annotations (#5939)

We had this annotation everywhere in app-server APIs which made it so
that fields get serialized as `field?: T`, meaning if the field as
`None` we would omit the field in the payload. Removing this annotation
changes it so that we return `field: T | null` instead, which makes
codex app-server's API more aligned with the convention of public OpenAI
APIs like Responses.

Separately, remove the `#[ts(optional_fields = nullable)]` annotations
that were recently added which made all the TS types become `field?: T |
null` which is not great since clients need to handle undefined and
null.

I think generally it'll be best to have optional types be either:
- `field: T | null` (preferred, aligned with public OpenAI APIs)
- `field?: T` where we have to, such as types generated from the MCP
schema:
https://github.com/modelcontextprotocol/modelcontextprotocol/blob/main/schema/2025-06-18/schema.ts
(see changes to `mcp-types/`)

I updated @etraut-openai's unit test to check that all generated TS
types are one or the other, not both (so will error if we have a type
that has `field?: T | null`). I don't think there's currently a good use
case for that - but we can always revisit.
This commit is contained in:
Owen Lin
2025-10-30 11:18:53 -07:00
committed by GitHub
parent 9572cfc782
commit 89c00611c2
13 changed files with 265 additions and 193 deletions

View File

@@ -248,7 +248,6 @@ pub enum Account {
#[serde(rename = "chatgpt", rename_all = "camelCase")]
#[ts(rename = "chatgpt", rename_all = "camelCase")]
ChatGpt {
#[ts(optional = nullable)]
email: Option<String>,
plan_type: PlanType,
},
@@ -267,11 +266,9 @@ pub struct InitializeParams {
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct ClientInfo {
pub name: String,
#[serde(skip_serializing_if = "Option::is_none")]
pub title: Option<String>,
pub version: String,
}
@@ -283,42 +280,33 @@ pub struct InitializeResponse {
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct NewConversationParams {
/// Optional override for the model name (e.g. "o3", "o4-mini").
#[serde(skip_serializing_if = "Option::is_none")]
pub model: Option<String>,
/// Override the model provider to use for this session.
#[serde(skip_serializing_if = "Option::is_none")]
pub model_provider: Option<String>,
/// Configuration profile from config.toml to specify default options.
#[serde(skip_serializing_if = "Option::is_none")]
pub profile: Option<String>,
/// Working directory for the session. If relative, it is resolved against
/// the server process's current working directory.
#[serde(skip_serializing_if = "Option::is_none")]
pub cwd: Option<String>,
/// Approval policy for shell commands generated by the model:
/// `untrusted`, `on-failure`, `on-request`, `never`.
#[serde(skip_serializing_if = "Option::is_none")]
pub approval_policy: Option<AskForApproval>,
/// Sandbox mode: `read-only`, `workspace-write`, or `danger-full-access`.
#[serde(skip_serializing_if = "Option::is_none")]
pub sandbox: Option<SandboxMode>,
/// Individual config settings that will override what is in
/// CODEX_HOME/config.toml.
#[serde(skip_serializing_if = "Option::is_none")]
pub config: Option<HashMap<String, serde_json::Value>>,
/// The set of instructions to use instead of the default ones.
#[serde(skip_serializing_if = "Option::is_none")]
pub base_instructions: Option<String>,
/// Developer instructions that will be sent as a `developer` role message.
@@ -330,29 +318,24 @@ pub struct NewConversationParams {
pub compact_prompt: Option<String>,
/// Whether to include the apply patch tool in the conversation.
#[serde(skip_serializing_if = "Option::is_none")]
pub include_apply_patch_tool: Option<bool>,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct NewConversationResponse {
pub conversation_id: ConversationId,
pub model: String,
/// Note this could be ignored by the model.
#[serde(skip_serializing_if = "Option::is_none")]
pub reasoning_effort: Option<ReasoningEffort>,
pub rollout_path: PathBuf,
}
#[derive(Serialize, Deserialize, Debug, Clone, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct ResumeConversationResponse {
pub conversation_id: ConversationId,
pub model: String,
#[serde(skip_serializing_if = "Option::is_none")]
pub initial_messages: Option<Vec<EventMsg>>,
pub rollout_path: PathBuf,
}
@@ -381,57 +364,46 @@ pub struct GetConversationSummaryResponse {
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct ListConversationsParams {
/// Optional page size; defaults to a reasonable server-side value.
#[serde(skip_serializing_if = "Option::is_none")]
pub page_size: Option<usize>,
/// Opaque pagination cursor returned by a previous call.
#[serde(skip_serializing_if = "Option::is_none")]
pub cursor: Option<String>,
/// Optional model provider filter (matches against session metadata).
/// - None => filter by the server's default model provider
/// - Some([]) => no filtering, include all providers
/// - Some([...]) => only include sessions with one of the specified providers
#[serde(skip_serializing_if = "Option::is_none")]
pub model_providers: Option<Vec<String>>,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct ConversationSummary {
pub conversation_id: ConversationId,
pub path: PathBuf,
pub preview: String,
/// RFC3339 timestamp string for the session start, if available.
#[serde(skip_serializing_if = "Option::is_none")]
pub timestamp: Option<String>,
/// Model provider recorded for the session (resolved when absent in metadata).
pub model_provider: String,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct ListConversationsResponse {
pub items: Vec<ConversationSummary>,
/// Opaque cursor to pass to the next call to continue after the last item.
/// if None, there are no more items to return.
#[serde(skip_serializing_if = "Option::is_none")]
pub next_cursor: Option<String>,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct ListModelsParams {
/// Optional page size; defaults to a reasonable server-side value.
#[serde(skip_serializing_if = "Option::is_none")]
pub page_size: Option<usize>,
/// Opaque pagination cursor returned by a previous call.
#[serde(skip_serializing_if = "Option::is_none")]
pub cursor: Option<String>,
}
@@ -456,24 +428,19 @@ pub struct ReasoningEffortOption {
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct ListModelsResponse {
pub items: Vec<Model>,
/// Opaque cursor to pass to the next call to continue after the last item.
/// if None, there are no more items to return.
#[serde(skip_serializing_if = "Option::is_none")]
pub next_cursor: Option<String>,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct UploadFeedbackParams {
pub classification: String,
#[serde(skip_serializing_if = "Option::is_none")]
pub reason: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub conversation_id: Option<ConversationId>,
pub include_logs: bool,
}
@@ -501,7 +468,6 @@ pub enum LoginAccountParams {
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct LoginAccountResponse {
/// Only set if the login method is ChatGPT.
@@ -518,20 +484,15 @@ pub struct LoginAccountResponse {
pub struct LogoutAccountResponse {}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct ResumeConversationParams {
/// Absolute path to the rollout JSONL file, when explicitly resuming a known rollout.
#[serde(skip_serializing_if = "Option::is_none")]
pub path: Option<PathBuf>,
/// If the rollout path is not known, it can be discovered via the conversation id at the cost of extra latency.
#[serde(skip_serializing_if = "Option::is_none")]
pub conversation_id: Option<ConversationId>,
/// if the rollout path or conversation id is not known, it can be resumed from given history
#[serde(skip_serializing_if = "Option::is_none")]
pub history: Option<Vec<ResponseItem>>,
/// Optional overrides to apply when spawning the resumed session.
#[serde(skip_serializing_if = "Option::is_none")]
pub overrides: Option<NewConversationParams>,
}
@@ -610,19 +571,15 @@ pub struct LogoutChatGptParams {}
pub struct LogoutChatGptResponse {}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct GetAuthStatusParams {
/// If true, include the current auth token (if available) in the response.
#[serde(skip_serializing_if = "Option::is_none")]
pub include_token: Option<bool>,
/// If true, attempt to refresh the token before returning status.
#[serde(skip_serializing_if = "Option::is_none")]
pub refresh_token: Option<bool>,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct ExecOneOffCommandParams {
/// Command argv to execute.
@@ -631,10 +588,8 @@ pub struct ExecOneOffCommandParams {
/// If not specified, a sensible default is used server-side.
pub timeout_ms: Option<u64>,
/// Optional working directory for the process. Defaults to server config cwd.
#[serde(skip_serializing_if = "Option::is_none")]
pub cwd: Option<PathBuf>,
/// Optional explicit sandbox policy overriding the server default.
#[serde(skip_serializing_if = "Option::is_none")]
pub sandbox_policy: Option<SandboxPolicy>,
}
@@ -654,17 +609,13 @@ pub struct GetAccountRateLimitsResponse {
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
#[ts(optional_fields = nullable)]
pub struct GetAuthStatusResponse {
#[serde(skip_serializing_if = "Option::is_none")]
pub auth_method: Option<AuthMode>,
#[serde(skip_serializing_if = "Option::is_none")]
pub auth_token: Option<String>,
// Indicates that auth method must be valid to use the server.
// This can be false if using a custom provider that is configured
// with requires_openai_auth == false.
#[serde(skip_serializing_if = "Option::is_none")]
pub requires_openai_auth: Option<bool>,
}
@@ -675,7 +626,6 @@ pub struct GetUserAgentResponse {
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct UserInfoResponse {
/// Note: `alleged_user_email` is not currently verified. We read it from
@@ -692,15 +642,12 @@ pub struct GetUserSavedConfigResponse {
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct SetDefaultModelParams {
/// If set to None, this means `model` should be cleared in config.toml.
#[serde(skip_serializing_if = "Option::is_none")]
pub model: Option<String>,
/// If set to None, this means `model_reasoning_effort` should be cleared
/// in config.toml.
#[serde(skip_serializing_if = "Option::is_none")]
pub reasoning_effort: Option<ReasoningEffort>,
}
@@ -712,46 +659,32 @@ pub struct SetDefaultModelResponse {}
/// client-configurable settings that can be specified in the NewConversation
/// and SendUserTurn requests.
#[derive(Deserialize, Debug, Clone, PartialEq, Serialize, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct UserSavedConfig {
/// Approvals
#[serde(skip_serializing_if = "Option::is_none")]
pub approval_policy: Option<AskForApproval>,
#[serde(skip_serializing_if = "Option::is_none")]
pub sandbox_mode: Option<SandboxMode>,
#[serde(skip_serializing_if = "Option::is_none")]
pub sandbox_settings: Option<SandboxSettings>,
#[serde(skip_serializing_if = "Option::is_none")]
pub forced_chatgpt_workspace_id: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub forced_login_method: Option<ForcedLoginMethod>,
/// Model-specific configuration
#[serde(skip_serializing_if = "Option::is_none")]
pub model: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub model_reasoning_effort: Option<ReasoningEffort>,
#[serde(skip_serializing_if = "Option::is_none")]
pub model_reasoning_summary: Option<ReasoningSummary>,
#[serde(skip_serializing_if = "Option::is_none")]
pub model_verbosity: Option<Verbosity>,
/// Tools
#[serde(skip_serializing_if = "Option::is_none")]
pub tools: Option<Tools>,
/// Profiles
#[serde(skip_serializing_if = "Option::is_none")]
pub profile: Option<String>,
#[serde(default)]
pub profiles: HashMap<String, Profile>,
}
/// MCP representation of a [`codex_core::config_profile::ConfigProfile`].
#[derive(Deserialize, Debug, Clone, PartialEq, Serialize, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct Profile {
pub model: Option<String>,
@@ -764,29 +697,23 @@ pub struct Profile {
pub model_verbosity: Option<Verbosity>,
pub chatgpt_base_url: Option<String>,
}
/// MCP representation of a [`codex_core::config::ToolsToml`].
#[derive(Deserialize, Debug, Clone, PartialEq, Serialize, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct Tools {
#[serde(skip_serializing_if = "Option::is_none")]
pub web_search: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
pub view_image: Option<bool>,
}
/// MCP representation of a [`codex_core::config::types::SandboxWorkspaceWrite`].
#[derive(Deserialize, Debug, Clone, PartialEq, Serialize, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct SandboxSettings {
#[serde(default)]
pub writable_roots: Vec<PathBuf>,
#[serde(skip_serializing_if = "Option::is_none")]
pub network_access: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
pub exclude_tmpdir_env_var: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
pub exclude_slash_tmp: Option<bool>,
}
@@ -798,7 +725,6 @@ pub struct SendUserMessageParams {
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct SendUserTurnParams {
pub conversation_id: ConversationId,
@@ -807,7 +733,6 @@ pub struct SendUserTurnParams {
pub approval_policy: AskForApproval,
pub sandbox_policy: SandboxPolicy,
pub model: String,
#[serde(skip_serializing_if = "Option::is_none")]
pub effort: Option<ReasoningEffort>,
pub summary: ReasoningSummary,
}
@@ -942,7 +867,6 @@ server_request_definitions! {
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct ApplyPatchApprovalParams {
pub conversation_id: ConversationId,
@@ -951,16 +875,13 @@ pub struct ApplyPatchApprovalParams {
pub call_id: String,
pub file_changes: HashMap<PathBuf, FileChange>,
/// Optional explanatory reason (e.g. request for extra write access).
#[serde(skip_serializing_if = "Option::is_none")]
pub reason: Option<String>,
/// When set, the agent is asking the user to allow writes under this root
/// for the remainder of the session (unclear if this is honored today).
#[serde(skip_serializing_if = "Option::is_none")]
pub grant_root: Option<PathBuf>,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct ExecCommandApprovalParams {
pub conversation_id: ConversationId,
@@ -969,9 +890,7 @@ pub struct ExecCommandApprovalParams {
pub call_id: String,
pub command: Vec<String>,
pub cwd: PathBuf,
#[serde(skip_serializing_if = "Option::is_none")]
pub reason: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub risk: Option<SandboxCommandAssessment>,
pub parsed_cmd: Vec<ParsedCommand>,
}
@@ -987,26 +906,22 @@ pub struct ApplyPatchApprovalResponse {
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
#[ts(rename_all = "camelCase")]
pub struct FuzzyFileSearchParams {
pub query: String,
pub roots: Vec<String>,
// if provided, will cancel any previous request that used the same value
#[serde(skip_serializing_if = "Option::is_none")]
pub cancellation_token: Option<String>,
}
/// Superset of [`codex_file_search::FileMatch`]
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
pub struct FuzzyFileSearchResult {
pub root: String,
pub path: String,
pub file_name: String,
pub score: u32,
#[serde(skip_serializing_if = "Option::is_none")]
pub indices: Option<Vec<u32>>,
}
@@ -1016,18 +931,15 @@ pub struct FuzzyFileSearchResponse {
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct LoginChatGptCompleteNotification {
#[schemars(with = "String")]
pub login_id: Uuid,
pub success: bool,
#[serde(skip_serializing_if = "Option::is_none")]
pub error: Option<String>,
}
#[derive(Serialize, Deserialize, Debug, Clone, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct SessionConfiguredNotification {
/// Name left as session_id instead of conversation_id for backwards compatibility.
@@ -1037,7 +949,6 @@ pub struct SessionConfiguredNotification {
pub model: String,
/// The effort the model is putting into reasoning about the user's request.
#[serde(skip_serializing_if = "Option::is_none")]
pub reasoning_effort: Option<ReasoningEffort>,
/// Identifier of the history log file (inode on Unix, 0 otherwise).
@@ -1049,18 +960,15 @@ pub struct SessionConfiguredNotification {
/// Optional initial messages (as events) for resumed sessions.
/// When present, UIs can use these to seed the history.
#[serde(skip_serializing_if = "Option::is_none")]
pub initial_messages: Option<Vec<EventMsg>>,
pub rollout_path: PathBuf,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[ts(optional_fields = nullable)]
#[serde(rename_all = "camelCase")]
pub struct AuthStatusChangeNotification {
/// Current authentication method; omitted if signed out.
#[serde(skip_serializing_if = "Option::is_none")]
pub auth_method: Option<AuthMode>,
}
@@ -1144,7 +1052,14 @@ mod tests {
"id": 42,
"params": {
"model": "gpt-5-codex",
"approvalPolicy": "on-request"
"modelProvider": null,
"profile": null,
"cwd": null,
"approvalPolicy": "on-request",
"sandbox": null,
"config": null,
"baseInstructions": null,
"includeApplyPatchTool": null
}
}),
serde_json::to_value(&request)?,
@@ -1217,6 +1132,7 @@ mod tests {
"command": ["echo", "hello"],
"cwd": "/tmp",
"reason": "because tests",
"risk": null,
"parsedCmd": [
{
"type": "unknown",
@@ -1361,7 +1277,10 @@ mod tests {
json!({
"method": "model/list",
"id": 6,
"params": {}
"params": {
"pageSize": null,
"cursor": null
}
}),
serde_json::to_value(&request)?,
);