From efd96c46c71ae660042032f42b3750e46a7f5308 Mon Sep 17 00:00:00 2001 From: Owen Lin Date: Tue, 3 Feb 2026 11:51:37 -0800 Subject: [PATCH] fix(app-server): fix TS annotations for optional fields on requests (#10412) This updates our generated TypeScript types to be more correct with how the server actually behaves, **specifically for JSON-RPC requests**. Before this PR, we'd generate `field: T | null`. After this PR, we will have `field?: T | null`. The latter matches how the server actually works, in that if an optional field is omitted, the server will treat it as null. This also makes it less annoying in theory for clients to upgrade to newer versions of Codex, since adding a new optional field to a JSON-RPC request should not require a client change. NOTE: This only applies to JSON-RPC requests. All other payloads (i.e. responses, notifications) will return `field: T | null` as usual. --- AGENTS.md | 40 +++++++++ .../schema/typescript/v2/AppsListParams.ts | 4 +- .../v2/ChatgptAuthTokensRefreshParams.ts | 2 +- .../schema/typescript/v2/CommandExecParams.ts | 2 +- .../CommandExecutionRequestApprovalParams.ts | 10 +-- .../typescript/v2/ConfigBatchWriteParams.ts | 2 +- .../schema/typescript/v2/ConfigReadParams.ts | 2 +- .../typescript/v2/ConfigValueWriteParams.ts | 2 +- .../typescript/v2/FeedbackUploadParams.ts | 2 +- .../v2/FileChangeRequestApprovalParams.ts | 4 +- .../v2/ListMcpServerStatusParams.ts | 4 +- .../v2/McpServerOauthLoginParams.ts | 2 +- .../schema/typescript/v2/ModelListParams.ts | 4 +- .../schema/typescript/v2/ReviewStartParams.ts | 2 +- .../schema/typescript/v2/ThreadForkParams.ts | 4 +- .../schema/typescript/v2/ThreadListParams.ts | 12 +-- .../typescript/v2/ThreadLoadedListParams.ts | 4 +- .../typescript/v2/ThreadResumeParams.ts | 6 +- .../schema/typescript/v2/ThreadStartParams.ts | 2 +- .../schema/typescript/v2/TurnStartParams.ts | 18 ++-- codex-rs/app-server-protocol/src/export.rs | 27 ++++-- .../app-server-protocol/src/protocol/v2.rs | 82 +++++++++++++++++-- 22 files changed, 179 insertions(+), 58 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 4b6f8992e..77c48ddba 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -112,3 +112,43 @@ If you don’t have the tool: let request = mock.single_request(); // assert using request.function_call_output(call_id) or request.json_body() or other helpers. ``` + +## App-server API Development Best Practices + +These guidelines apply to app-server protocol work in `codex-rs`, especially: + +- `app-server-protocol/src/protocol/common.rs` +- `app-server-protocol/src/protocol/v2.rs` +- `app-server/README.md` + +### Core Rules + +- All active API development should happen in app-server v2. Do not add new API surface area to v1. +- Follow payload naming consistently: + `*Params` for request payloads, `*Response` for responses, and `*Notification` for notifications. +- Expose RPC methods as `/` and keep `` singular (for example, `thread/read`, `app/list`). +- Always expose fields as camelCase on the wire with `#[serde(rename_all = "camelCase")]` unless a tagged union or explicit compatibility requirement needs a targeted rename. +- Always set `#[ts(export_to = "v2/")]` on v2 request/response/notification types so generated TypeScript lands in the correct namespace. +- Never use `#[serde(skip_serializing_if = "Option::is_none")]` for v2 API payload fields. + Exception: client->server requests that intentionally have no params may use: + `params: #[ts(type = "undefined")] #[serde(skip_serializing_if = "Option::is_none")] Option<()>`. +- For client->server JSON-RPC request payloads (`*Params`) only, every optional field must be annotated with `#[ts(optional = nullable)]`. Do not use `#[ts(optional = nullable)]` outside client->server request payloads (`*Params`). +- For client->server JSON-RPC request payloads only, and you want to express a boolean field where omission means `false`, use `#[serde(default, skip_serializing_if = "std::ops::Not::not")] pub field: bool` over `Option`. +- For new list methods, implement cursor pagination by default: + request fields `pub cursor: Option` and `pub limit: Option`, + response fields `pub data: Vec<...>` and `pub next_cursor: Option`. +- Keep Rust and TS wire renames aligned. If a field or variant uses `#[serde(rename = "...")]`, add matching `#[ts(rename = "...")]`. +- For discriminated unions, use explicit tagging in both serializers: + `#[serde(tag = "type", ...)]` and `#[ts(tag = "type", ...)]`. +- Prefer plain `String` IDs at the API boundary (do UUID parsing/conversion internally if needed). +- Timestamps should be integer Unix seconds (`i64`) and named `*_at` (for example, `created_at`, `updated_at`, `resets_at`). +- For experimental API surface area: + use `#[experimental("method/or/field")]`, derive `ExperimentalApi` when field-level gating is needed, and use `inspect_params: true` in `common.rs` when only some fields of a method are experimental. + +### Development Workflow + +- Update docs/examples when API behavior changes (at minimum `app-server/README.md`). +- Regenerate schema fixtures when API shapes change: + `just write-app-server-schema` + (and `just write-app-server-schema --experimental` when experimental API fixtures are affected). +- Validate with `cargo test -p codex-app-server-protocol`. diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AppsListParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AppsListParams.ts index fe925998b..a3e6fbf62 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/AppsListParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AppsListParams.ts @@ -6,8 +6,8 @@ export type AppsListParams = { /** * Opaque pagination cursor returned by a previous call. */ -cursor: string | null, +cursor?: string | null, /** * Optional page size; defaults to a reasonable server-side value. */ -limit: number | null, }; +limit?: number | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ChatgptAuthTokensRefreshParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ChatgptAuthTokensRefreshParams.ts index 75e677f5c..4393c7f7a 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ChatgptAuthTokensRefreshParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ChatgptAuthTokensRefreshParams.ts @@ -13,4 +13,4 @@ export type ChatgptAuthTokensRefreshParams = { reason: ChatgptAuthTokensRefreshR * This may be `null` when the prior ID token did not include a workspace * identifier (`chatgpt_account_id`) or when the token could not be parsed. */ -previousAccountId: string | null, }; +previousAccountId?: string | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecParams.ts index fc9c6a39c..847e19d69 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecParams.ts @@ -3,4 +3,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { SandboxPolicy } from "./SandboxPolicy"; -export type CommandExecParams = { command: Array, timeoutMs: number | null, cwd: string | null, sandboxPolicy: SandboxPolicy | null, }; +export type CommandExecParams = { command: Array, timeoutMs?: number | null, cwd?: string | null, sandboxPolicy?: SandboxPolicy | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts index 3bad1b467..12b252143 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts @@ -8,20 +8,20 @@ export type CommandExecutionRequestApprovalParams = { threadId: string, turnId: /** * Optional explanatory reason (e.g. request for network access). */ -reason: string | null, +reason?: string | null, /** * The command to be executed. */ -command?: string, +command?: string | null, /** * The command's working directory. */ -cwd?: string, +cwd?: string | null, /** * Best-effort parsed command actions for friendly display. */ -commandActions?: Array, +commandActions?: Array | null, /** * Optional proposed execpolicy amendment to allow similar commands without prompting. */ -proposedExecpolicyAmendment: ExecPolicyAmendment | null, }; +proposedExecpolicyAmendment?: ExecPolicyAmendment | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ConfigBatchWriteParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ConfigBatchWriteParams.ts index c4012fc33..77df84e3b 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ConfigBatchWriteParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ConfigBatchWriteParams.ts @@ -7,4 +7,4 @@ export type ConfigBatchWriteParams = { edits: Array, /** * Path to the config file to write; defaults to the user's `config.toml` when omitted. */ -filePath: string | null, expectedVersion: string | null, }; +filePath?: string | null, expectedVersion?: string | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ConfigReadParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ConfigReadParams.ts index 3cb2394bc..c5d5bc874 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ConfigReadParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ConfigReadParams.ts @@ -8,4 +8,4 @@ export type ConfigReadParams = { includeLayers: boolean, * return the effective config as seen from that directory (i.e., including any * project layers between `cwd` and the project/repo root). */ -cwd: string | null, }; +cwd?: string | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ConfigValueWriteParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ConfigValueWriteParams.ts index 2dedfe8d0..9204760f8 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ConfigValueWriteParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ConfigValueWriteParams.ts @@ -8,4 +8,4 @@ export type ConfigValueWriteParams = { keyPath: string, value: JsonValue, mergeS /** * Path to the config file to write; defaults to the user's `config.toml` when omitted. */ -filePath: string | null, expectedVersion: string | null, }; +filePath?: string | null, expectedVersion?: string | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/FeedbackUploadParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/FeedbackUploadParams.ts index aeccc4577..3066e6540 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/FeedbackUploadParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/FeedbackUploadParams.ts @@ -2,4 +2,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -export type FeedbackUploadParams = { classification: string, reason: string | null, threadId: string | null, includeLogs: boolean, }; +export type FeedbackUploadParams = { classification: string, reason?: string | null, threadId?: string | null, includeLogs: boolean, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/FileChangeRequestApprovalParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/FileChangeRequestApprovalParams.ts index a7dbfbb7c..a7951b685 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/FileChangeRequestApprovalParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/FileChangeRequestApprovalParams.ts @@ -6,9 +6,9 @@ export type FileChangeRequestApprovalParams = { threadId: string, turnId: string /** * Optional explanatory reason (e.g. request for extra write access). */ -reason: string | null, +reason?: string | null, /** * [UNSTABLE] 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). */ -grantRoot: string | null, }; +grantRoot?: string | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ListMcpServerStatusParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ListMcpServerStatusParams.ts index 8027087fd..05c02c19f 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ListMcpServerStatusParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ListMcpServerStatusParams.ts @@ -6,8 +6,8 @@ export type ListMcpServerStatusParams = { /** * Opaque pagination cursor returned by a previous call. */ -cursor: string | null, +cursor?: string | null, /** * Optional page size; defaults to a server-defined value. */ -limit: number | null, }; +limit?: number | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/McpServerOauthLoginParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/McpServerOauthLoginParams.ts index 0ad8f3948..a61c30460 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/McpServerOauthLoginParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/McpServerOauthLoginParams.ts @@ -2,4 +2,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -export type McpServerOauthLoginParams = { name: string, scopes?: Array, timeoutSecs?: bigint, }; +export type McpServerOauthLoginParams = { name: string, scopes?: Array | null, timeoutSecs?: bigint | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ModelListParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ModelListParams.ts index a2b7214b7..b0bc5326c 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ModelListParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ModelListParams.ts @@ -6,8 +6,8 @@ export type ModelListParams = { /** * Opaque pagination cursor returned by a previous call. */ -cursor: string | null, +cursor?: string | null, /** * Optional page size; defaults to a reasonable server-side value. */ -limit: number | null, }; +limit?: number | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ReviewStartParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ReviewStartParams.ts index c36533f2e..363e6dda3 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ReviewStartParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ReviewStartParams.ts @@ -9,4 +9,4 @@ export type ReviewStartParams = { threadId: string, target: ReviewTarget, * Where to run the review: inline (default) on the current thread or * detached on a new thread (returned in `reviewThreadId`). */ -delivery: ReviewDelivery | null, }; +delivery?: ReviewDelivery | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ThreadForkParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ThreadForkParams.ts index 4ecb1b39d..eae506c2b 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ThreadForkParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ThreadForkParams.ts @@ -19,8 +19,8 @@ export type ThreadForkParams = { threadId: string, * [UNSTABLE] Specify the rollout path to fork from. * If specified, the thread_id param will be ignored. */ -path: string | null, +path?: string | null, /** * Configuration overrides for the forked thread, if any. */ -model: string | null, modelProvider: string | null, cwd: string | null, approvalPolicy: AskForApproval | null, sandbox: SandboxMode | null, config: { [key in string]?: JsonValue } | null, baseInstructions: string | null, developerInstructions: string | null, }; +model?: string | null, modelProvider?: string | null, cwd?: string | null, approvalPolicy?: AskForApproval | null, sandbox?: SandboxMode | null, config?: { [key in string]?: JsonValue } | null, baseInstructions?: string | null, developerInstructions?: string | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ThreadListParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ThreadListParams.ts index 357fdd21c..c54f323f5 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ThreadListParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ThreadListParams.ts @@ -8,27 +8,27 @@ export type ThreadListParams = { /** * Opaque pagination cursor returned by a previous call. */ -cursor: string | null, +cursor?: string | null, /** * Optional page size; defaults to a reasonable server-side value. */ -limit: number | null, +limit?: number | null, /** * Optional sort key; defaults to created_at. */ -sortKey: ThreadSortKey | null, +sortKey?: ThreadSortKey | null, /** * Optional provider filter; when set, only sessions recorded under these * providers are returned. When present but empty, includes all providers. */ -modelProviders: Array | null, +modelProviders?: Array | null, /** * Optional source filter; when set, only sessions from these source kinds * are returned. When omitted or empty, defaults to interactive sources. */ -sourceKinds: Array | null, +sourceKinds?: Array | null, /** * Optional archived filter; when set to true, only archived threads are returned. * If false or null, only non-archived threads are returned. */ -archived: boolean | null, }; +archived?: boolean | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ThreadLoadedListParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ThreadLoadedListParams.ts index 5419b00bd..ef1e0ac08 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ThreadLoadedListParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ThreadLoadedListParams.ts @@ -6,8 +6,8 @@ export type ThreadLoadedListParams = { /** * Opaque pagination cursor returned by a previous call. */ -cursor: string | null, +cursor?: string | null, /** * Optional page size; defaults to no limit. */ -limit: number | null, }; +limit?: number | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ThreadResumeParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ThreadResumeParams.ts index dca685aa4..64d1f77d8 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ThreadResumeParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ThreadResumeParams.ts @@ -24,13 +24,13 @@ export type ThreadResumeParams = { threadId: string, * If specified, the thread will be resumed with the provided history * instead of loaded from disk. */ -history: Array | null, +history?: Array | null, /** * [UNSTABLE] Specify the rollout path to resume from. * If specified, the thread_id param will be ignored. */ -path: string | null, +path?: string | null, /** * Configuration overrides for the resumed thread, if any. */ -model: string | null, modelProvider: string | null, cwd: string | null, approvalPolicy: AskForApproval | null, sandbox: SandboxMode | null, config: { [key in string]?: JsonValue } | null, baseInstructions: string | null, developerInstructions: string | null, personality: Personality | null, }; +model?: string | null, modelProvider?: string | null, cwd?: string | null, approvalPolicy?: AskForApproval | null, sandbox?: SandboxMode | null, config?: { [key in string]?: JsonValue } | null, baseInstructions?: string | null, developerInstructions?: string | null, personality?: Personality | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ThreadStartParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ThreadStartParams.ts index 3b4c20590..b0f1d1e2e 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ThreadStartParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ThreadStartParams.ts @@ -6,7 +6,7 @@ import type { JsonValue } from "../serde_json/JsonValue"; import type { AskForApproval } from "./AskForApproval"; import type { SandboxMode } from "./SandboxMode"; -export type ThreadStartParams = {model: string | null, modelProvider: string | null, cwd: string | null, approvalPolicy: AskForApproval | null, sandbox: SandboxMode | null, config: { [key in string]?: JsonValue } | null, baseInstructions: string | null, developerInstructions: string | null, personality: Personality | null, ephemeral: boolean | null, /** +export type ThreadStartParams = {model?: string | null, modelProvider?: string | null, cwd?: string | null, approvalPolicy?: AskForApproval | null, sandbox?: SandboxMode | null, config?: { [key in string]?: JsonValue } | null, baseInstructions?: string | null, developerInstructions?: string | null, personality?: Personality | null, ephemeral?: boolean | null, /** * If true, opt into emitting raw response items on the event stream. * * This is for internal use only (e.g. Codex Cloud). diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/TurnStartParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/TurnStartParams.ts index 4987c9e5c..8cf1aaf20 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/TurnStartParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/TurnStartParams.ts @@ -14,37 +14,37 @@ export type TurnStartParams = { threadId: string, input: Array, /** * Override the working directory for this turn and subsequent turns. */ -cwd: string | null, +cwd?: string | null, /** * Override the approval policy for this turn and subsequent turns. */ -approvalPolicy: AskForApproval | null, +approvalPolicy?: AskForApproval | null, /** * Override the sandbox policy for this turn and subsequent turns. */ -sandboxPolicy: SandboxPolicy | null, +sandboxPolicy?: SandboxPolicy | null, /** * Override the model for this turn and subsequent turns. */ -model: string | null, +model?: string | null, /** * Override the reasoning effort for this turn and subsequent turns. */ -effort: ReasoningEffort | null, +effort?: ReasoningEffort | null, /** * Override the reasoning summary for this turn and subsequent turns. */ -summary: ReasoningSummary | null, +summary?: ReasoningSummary | null, /** * Override the personality for this turn and subsequent turns. */ -personality: Personality | null, +personality?: Personality | null, /** * Optional JSON Schema used to constrain the final assistant message for this turn. */ -outputSchema: JsonValue | null, +outputSchema?: JsonValue | null, /** * EXPERIMENTAL - set a pre-set collaboration mode. * Takes precedence over model, reasoning_effort, and developer instructions if set. */ -collaborationMode: CollaborationMode | null, }; +collaborationMode?: CollaborationMode | null, }; diff --git a/codex-rs/app-server-protocol/src/export.rs b/codex-rs/app-server-protocol/src/export.rs index 7878f72c0..6de7c114c 100644 --- a/codex-rs/app-server-protocol/src/export.rs +++ b/codex-rs/app-server-protocol/src/export.rs @@ -1402,8 +1402,8 @@ mod tests { use uuid::Uuid; #[test] - fn generated_ts_has_no_optional_nullable_fields() -> Result<()> { - // Assert that there are no types of the form "?: T | null" in the generated TS files. + fn generated_ts_optional_nullable_fields_only_in_params() -> Result<()> { + // Assert that "?: T | null" only appears in generated *Params types. let output_dir = std::env::temp_dir().join(format!("codex_ts_types_{}", Uuid::now_v7())); fs::create_dir(&output_dir)?; @@ -1463,6 +1463,13 @@ mod tests { } if matches!(path.extension().and_then(|ext| ext.to_str()), Some("ts")) { + // Only allow "?: T | null" in objects representing JSON-RPC requests, + // which we assume are called "*Params". + let allow_optional_nullable = path + .file_stem() + .and_then(|stem| stem.to_str()) + .is_some_and(|stem| stem.ends_with("Params")); + let contents = fs::read_to_string(&path)?; if contents.contains("| undefined") { undefined_offenders.push(path.clone()); @@ -1583,9 +1590,11 @@ mod tests { } // If the last non-whitespace before ':' is '?', then this is an - // optional field with a nullable type (i.e., "?: T | null"), - // which we explicitly disallow. - if field_prefix.chars().rev().find(|c| !c.is_whitespace()) == Some('?') { + // optional field with a nullable type (i.e., "?: T | null"). + // These are only allowed in *Params types. + if field_prefix.chars().rev().find(|c| !c.is_whitespace()) == Some('?') + && !allow_optional_nullable + { let line_number = contents[..abs_idx].chars().filter(|c| *c == '\n').count() + 1; let offending_line_end = contents[line_start_idx..] @@ -1613,12 +1622,12 @@ mod tests { "Generated TypeScript still includes unions with `undefined` in {undefined_offenders:?}" ); - // If this assertion fails, it means a field was generated as - // "?: T | null" — i.e., both optional (undefined) and nullable (null). - // We only want either "?: T" or ": T | null". + // If this assertion fails, it means a field was generated as "?: T | null", + // which is both optional (undefined) and nullable (null), for a type not ending + // in "Params" (which represent JSON-RPC requests). assert!( optional_nullable_offenders.is_empty(), - "Generated TypeScript has optional fields with nullable types (disallowed '?: T | null'), add #[ts(optional)] to fix:\n{optional_nullable_offenders:?}" + "Generated TypeScript has optional nullable fields outside *Params types (disallowed '?: T | null'):\n{optional_nullable_offenders:?}" ); Ok(()) diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 431be8219..7ce8cc2ed 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -480,6 +480,7 @@ pub struct ConfigReadParams { /// Optional working directory to resolve project config layers. If specified, /// return the effective config as seen from that directory (i.e., including any /// project layers between `cwd` and the project/repo root). + #[ts(optional = nullable)] pub cwd: Option, } @@ -525,7 +526,9 @@ pub struct ConfigValueWriteParams { pub value: JsonValue, pub merge_strategy: MergeStrategy, /// Path to the config file to write; defaults to the user's `config.toml` when omitted. + #[ts(optional = nullable)] pub file_path: Option, + #[ts(optional = nullable)] pub expected_version: Option, } @@ -535,7 +538,9 @@ pub struct ConfigValueWriteParams { pub struct ConfigBatchWriteParams { pub edits: Vec, /// Path to the config file to write; defaults to the user's `config.toml` when omitted. + #[ts(optional = nullable)] pub file_path: Option, + #[ts(optional = nullable)] pub expected_version: Option, } @@ -935,6 +940,7 @@ pub struct ChatgptAuthTokensRefreshParams { /// /// This may be `null` when the prior ID token did not include a workspace /// identifier (`chatgpt_account_id`) or when the token could not be parsed. + #[ts(optional = nullable)] pub previous_account_id: Option, } @@ -979,8 +985,10 @@ pub struct GetAccountResponse { #[ts(export_to = "v2/")] pub struct ModelListParams { /// Opaque pagination cursor returned by a previous call. + #[ts(optional = nullable)] pub cursor: Option, /// Optional page size; defaults to a reasonable server-side value. + #[ts(optional = nullable)] pub limit: Option, } @@ -1039,8 +1047,10 @@ pub struct CollaborationModeListResponse { #[ts(export_to = "v2/")] pub struct ListMcpServerStatusParams { /// Opaque pagination cursor returned by a previous call. + #[ts(optional = nullable)] pub cursor: Option, /// Optional page size; defaults to a server-defined value. + #[ts(optional = nullable)] pub limit: Option, } @@ -1070,8 +1080,10 @@ pub struct ListMcpServerStatusResponse { #[ts(export_to = "v2/")] pub struct AppsListParams { /// Opaque pagination cursor returned by a previous call. + #[ts(optional = nullable)] pub cursor: Option, /// Optional page size; defaults to a reasonable server-side value. + #[ts(optional = nullable)] pub limit: Option, } @@ -1116,10 +1128,10 @@ pub struct McpServerRefreshResponse {} pub struct McpServerOauthLoginParams { pub name: String, #[serde(default, skip_serializing_if = "Option::is_none")] - #[ts(optional)] + #[ts(optional = nullable)] pub scopes: Option>, #[serde(default, skip_serializing_if = "Option::is_none")] - #[ts(optional)] + #[ts(optional = nullable)] pub timeout_secs: Option, } @@ -1135,7 +1147,9 @@ pub struct McpServerOauthLoginResponse { #[ts(export_to = "v2/")] pub struct FeedbackUploadParams { pub classification: String, + #[ts(optional = nullable)] pub reason: Option, + #[ts(optional = nullable)] pub thread_id: Option, pub include_logs: bool, } @@ -1153,8 +1167,11 @@ pub struct FeedbackUploadResponse { pub struct CommandExecParams { pub command: Vec, #[ts(type = "number | null")] + #[ts(optional = nullable)] pub timeout_ms: Option, + #[ts(optional = nullable)] pub cwd: Option, + #[ts(optional = nullable)] pub sandbox_policy: Option, } @@ -1175,21 +1192,33 @@ pub struct CommandExecResponse { #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] pub struct ThreadStartParams { + #[ts(optional = nullable)] pub model: Option, + #[ts(optional = nullable)] pub model_provider: Option, + #[ts(optional = nullable)] pub cwd: Option, + #[ts(optional = nullable)] pub approval_policy: Option, + #[ts(optional = nullable)] pub sandbox: Option, + #[ts(optional = nullable)] pub config: Option>, + #[ts(optional = nullable)] pub base_instructions: Option, + #[ts(optional = nullable)] pub developer_instructions: Option, + #[ts(optional = nullable)] pub personality: Option, + #[ts(optional = nullable)] pub ephemeral: Option, #[experimental("thread/start.dynamicTools")] + #[ts(optional = nullable)] pub dynamic_tools: Option>, /// Test-only experimental field used to validate experimental gating and /// schema filtering behavior in a stable way. #[experimental("thread/start.mockExperimentalField")] + #[ts(optional = nullable)] pub mock_experimental_field: Option, /// If true, opt into emitting raw response items on the event stream. /// @@ -1204,6 +1233,7 @@ pub struct ThreadStartParams { #[ts(export_to = "v2/")] pub struct MockExperimentalMethodParams { /// Test-only payload field. + #[ts(optional = nullable)] pub value: Option, } @@ -1246,21 +1276,32 @@ pub struct ThreadResumeParams { /// [UNSTABLE] FOR CODEX CLOUD - DO NOT USE. /// If specified, the thread will be resumed with the provided history /// instead of loaded from disk. + #[ts(optional = nullable)] pub history: Option>, /// [UNSTABLE] Specify the rollout path to resume from. /// If specified, the thread_id param will be ignored. + #[ts(optional = nullable)] pub path: Option, /// Configuration overrides for the resumed thread, if any. + #[ts(optional = nullable)] pub model: Option, + #[ts(optional = nullable)] pub model_provider: Option, + #[ts(optional = nullable)] pub cwd: Option, + #[ts(optional = nullable)] pub approval_policy: Option, + #[ts(optional = nullable)] pub sandbox: Option, + #[ts(optional = nullable)] pub config: Option>, + #[ts(optional = nullable)] pub base_instructions: Option, + #[ts(optional = nullable)] pub developer_instructions: Option, + #[ts(optional = nullable)] pub personality: Option, } @@ -1292,16 +1333,25 @@ pub struct ThreadForkParams { /// [UNSTABLE] Specify the rollout path to fork from. /// If specified, the thread_id param will be ignored. + #[ts(optional = nullable)] pub path: Option, /// Configuration overrides for the forked thread, if any. + #[ts(optional = nullable)] pub model: Option, + #[ts(optional = nullable)] pub model_provider: Option, + #[ts(optional = nullable)] pub cwd: Option, + #[ts(optional = nullable)] pub approval_policy: Option, + #[ts(optional = nullable)] pub sandbox: Option, + #[ts(optional = nullable)] pub config: Option>, + #[ts(optional = nullable)] pub base_instructions: Option, + #[ts(optional = nullable)] pub developer_instructions: Option, } @@ -1386,19 +1436,25 @@ pub struct ThreadRollbackResponse { #[ts(export_to = "v2/")] pub struct ThreadListParams { /// Opaque pagination cursor returned by a previous call. + #[ts(optional = nullable)] pub cursor: Option, /// Optional page size; defaults to a reasonable server-side value. + #[ts(optional = nullable)] pub limit: Option, /// Optional sort key; defaults to created_at. + #[ts(optional = nullable)] pub sort_key: Option, /// Optional provider filter; when set, only sessions recorded under these /// providers are returned. When present but empty, includes all providers. + #[ts(optional = nullable)] pub model_providers: Option>, /// Optional source filter; when set, only sessions from these source kinds /// are returned. When omitted or empty, defaults to interactive sources. + #[ts(optional = nullable)] pub source_kinds: Option>, /// Optional archived filter; when set to true, only archived threads are returned. /// If false or null, only non-archived threads are returned. + #[ts(optional = nullable)] pub archived: Option, } @@ -1443,8 +1499,10 @@ pub struct ThreadListResponse { #[ts(export_to = "v2/")] pub struct ThreadLoadedListParams { /// Opaque pagination cursor returned by a previous call. + #[ts(optional = nullable)] pub cursor: Option, /// Optional page size; defaults to no limit. + #[ts(optional = nullable)] pub limit: Option, } @@ -1832,24 +1890,33 @@ pub struct TurnStartParams { pub thread_id: String, pub input: Vec, /// Override the working directory for this turn and subsequent turns. + #[ts(optional = nullable)] pub cwd: Option, /// Override the approval policy for this turn and subsequent turns. + #[ts(optional = nullable)] pub approval_policy: Option, /// Override the sandbox policy for this turn and subsequent turns. + #[ts(optional = nullable)] pub sandbox_policy: Option, /// Override the model for this turn and subsequent turns. + #[ts(optional = nullable)] pub model: Option, /// Override the reasoning effort for this turn and subsequent turns. + #[ts(optional = nullable)] pub effort: Option, /// Override the reasoning summary for this turn and subsequent turns. + #[ts(optional = nullable)] pub summary: Option, /// Override the personality for this turn and subsequent turns. + #[ts(optional = nullable)] pub personality: Option, /// Optional JSON Schema used to constrain the final assistant message for this turn. + #[ts(optional = nullable)] pub output_schema: Option, /// EXPERIMENTAL - set a pre-set collaboration mode. /// Takes precedence over model, reasoning_effort, and developer instructions if set. + #[ts(optional = nullable)] pub collaboration_mode: Option, } @@ -1863,6 +1930,7 @@ pub struct ReviewStartParams { /// Where to run the review: inline (default) on the current thread or /// detached on a new thread (returned in `reviewThreadId`). #[serde(default)] + #[ts(optional = nullable)] pub delivery: Option, } @@ -2644,20 +2712,22 @@ pub struct CommandExecutionRequestApprovalParams { pub turn_id: String, pub item_id: String, /// Optional explanatory reason (e.g. request for network access). + #[ts(optional = nullable)] pub reason: Option, /// The command to be executed. #[serde(default, skip_serializing_if = "Option::is_none")] - #[ts(optional)] + #[ts(optional = nullable)] pub command: Option, /// The command's working directory. #[serde(default, skip_serializing_if = "Option::is_none")] - #[ts(optional)] + #[ts(optional = nullable)] pub cwd: Option, /// Best-effort parsed command actions for friendly display. #[serde(default, skip_serializing_if = "Option::is_none")] - #[ts(optional)] + #[ts(optional = nullable)] pub command_actions: Option>, /// Optional proposed execpolicy amendment to allow similar commands without prompting. + #[ts(optional = nullable)] pub proposed_execpolicy_amendment: Option, } @@ -2676,9 +2746,11 @@ pub struct FileChangeRequestApprovalParams { pub turn_id: String, pub item_id: String, /// Optional explanatory reason (e.g. request for extra write access). + #[ts(optional = nullable)] pub reason: Option, /// [UNSTABLE] 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). + #[ts(optional = nullable)] pub grant_root: Option, }