Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<resource>/<method>` and keep `<resource>` 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<bool>`.
- For new list methods, implement cursor pagination by default:
request fields `pub cursor: Option<String>` and `pub limit: Option<u32>`,
response fields `pub data: Vec<...>` and `pub next_cursor: Option<String>`.
- 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`.
Original file line number Diff line number Diff line change
Expand Up @@ -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, };
Original file line number Diff line number Diff line change
Expand Up @@ -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, };
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>, timeoutMs: number | null, cwd: string | null, sandboxPolicy: SandboxPolicy | null, };
export type CommandExecParams = { command: Array<string>, timeoutMs?: number | null, cwd?: string | null, sandboxPolicy?: SandboxPolicy | null, };
Original file line number Diff line number Diff line change
Expand Up @@ -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<CommandAction>,
commandActions?: Array<CommandAction> | null,
/**
* Optional proposed execpolicy amendment to allow similar commands without prompting.
*/
proposedExecpolicyAmendment: ExecPolicyAmendment | null, };
proposedExecpolicyAmendment?: ExecPolicyAmendment | null, };
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ export type ConfigBatchWriteParams = { edits: Array<ConfigEdit>,
/**
* 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, };
Original file line number Diff line number Diff line change
Expand Up @@ -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, };
Original file line number Diff line number Diff line change
Expand Up @@ -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, };
Original file line number Diff line number Diff line change
Expand Up @@ -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, };
Original file line number Diff line number Diff line change
Expand Up @@ -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, };
Original file line number Diff line number Diff line change
Expand Up @@ -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, };
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>, timeoutSecs?: bigint, };
export type McpServerOauthLoginParams = { name: string, scopes?: Array<string> | null, timeoutSecs?: bigint | null, };
Original file line number Diff line number Diff line change
Expand Up @@ -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, };
Original file line number Diff line number Diff line change
Expand Up @@ -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, };
Original file line number Diff line number Diff line change
Expand Up @@ -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, };
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> | null,
modelProviders?: Array<string> | null,
/**
* Optional source filter; when set, only sessions from these source kinds
* are returned. When omitted or empty, defaults to interactive sources.
*/
sourceKinds: Array<ThreadSourceKind> | null,
sourceKinds?: Array<ThreadSourceKind> | 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, };
Original file line number Diff line number Diff line change
Expand Up @@ -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, };
Original file line number Diff line number Diff line change
Expand Up @@ -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<ResponseItem> | null,
history?: Array<ResponseItem> | 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, };
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,37 +14,37 @@ export type TurnStartParams = { threadId: string, input: Array<UserInput>,
/**
* 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, };
27 changes: 18 additions & 9 deletions codex-rs/app-server-protocol/src/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;

Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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..]
Expand Down Expand Up @@ -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(())
Expand Down
Loading
Loading