Commit Graph

5 Commits

  • Add missing "nullable" macro to protocol structs that contain optional fields (#5901)
    This PR addresses a current hole in the TypeScript code generation for
    the API server protocol. Fields that are marked as "Optional<>" in the
    Rust code are serialized such that the value is omitted when it is
    deserialized — appearing as `undefined`, but the TS type indicates
    (incorrectly) that it is always defined but possibly `null`. This can
    lead to subtle errors that the TypeScript compiler doesn't catch. The
    fix is to include the `#[ts(optional_fields = nullable)]` macro for all
    protocol structs that contain one or more `Optional<>` fields.
    
    This PR also includes a new test that validates that all TS protocol
    code containing "| null" in its type is marked optional ("?") to catch
    cases where `#[ts(optional_fields = nullable)]` is omitted.
  • [app-server] Annotate more exported types with a title (#5879)
    Follow-up to https://github.com/openai/codex/pull/5063
    
    Refined the app-server export pipeline so JSON Schema variants and
    discriminator fields are annotated with descriptive, stable titles
    before writing the bundle. This eliminates anonymous enum names in the
    generated Pydantic models (goodbye Type7) while keeping downstream
    tooling simple. Added shared helpers to derive titles and literals, and
    reused them across the traversal logic for clarity. Running just fix -p
    codex-app-server-protocol, just fmt, and cargo test -p
    codex-app-server-protocol validates the change.
  • fix: revert "[app-server] fix account/read response annotation (#5642)" (#5796)
    Revert #5642 because this generates:
    
    ```
    // GENERATED CODE! DO NOT MODIFY BY HAND!
    
    // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
    
    export type GetAccountResponse = Account | null;
    ```
    
    But `Account` is unknown.
    
    The unique use of `#[ts(export)]` on `GetAccountResponse` is also
    suspicious as are the changes to
    `codex-rs/app-server-protocol/src/export.rs` since the existing system
    has worked fine for quite some time.
    
    Though a pure backout of #5642 puts things in a state where, as the PR
    noted, the following does not work:
    
    ```
    cargo run -p codex-app-server-protocol --bin export -- --out DIR
    ```
    
    So in addition to the backout, this PR adds:
    
    ```rust
    #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
    #[serde(rename_all = "camelCase")]
    pub struct GetAccountResponse {
        pub account: Account,
    }
    ```
    
    and changes `GetAccount.response` as follows:
    
    ```diff
    -        response: Option<Account>,
    +        response: GetAccountResponse,
    ```
    
    making it consistent with other types.
    
    With this change, I verified that both of the following work:
    
    ```
    just codex generate-ts --out /tmp/somewhere
    cargo run -p codex-app-server-protocol --bin export -- --out /tmp/somewhere-else
    ```
    
    The generated TypeScript is as follows:
    
    ```typescript
    // GetAccountResponse.ts
    import type { Account } from "./Account";
    
    export type GetAccountResponse = { account: Account, };
    ```
    
    and
    
    ```typescript
    // Account.ts
    import type { PlanType } from "./PlanType";
    
    export type Account = { "type": "ApiKey", api_key: string, } | { "type": "chatgpt", email: string | null, plan_type: PlanType, };
    ```
    
    Though while the inconsistency between `"type": "ApiKey"` and `"type":
    "chatgpt"` is quite concerning, I'm not sure if that format is ever
    written to disk in any case, but @owenlin0, I would recommend looking
    into that.
    
    Also, it appears that the types in `codex-rs/protocol/src/account.rs`
    are used exclusively by the `app-server-protocol` crate, so perhaps they
    should just be moved there?
  • [app-server] fix account/read response annotation (#5642)
    The API schema export is currently broken:
    ```
    > cargo run -p codex-app-server-protocol --bin export -- --out DIR
    Error: this type cannot be exported
    ```
    
    This PR fixes the error message so we get more info:
    ```
    > cargo run -p codex-app-server-protocol --bin export -- --out DIR
    Error: failed to export client responses: dependency core::option::Option<codex_protocol::account::Account> cannot be exported
    ```
    
    And fixes the root cause which is the `account/read` response.
  • Generate JSON schema for app-server protocol (#5063)
    Add annotations and an export script that let us generate app-server
    protocol types as typescript and JSONSchema.
    
    The script itself is a bit hacky because we need to manually label some
    of the types. Unfortunately it seems that enum variants don't get good
    names by default and end up with something like `EventMsg1`,
    `EventMsg2`, etc. I'm not an expert in this by any means, but since this
    is only run manually and we already need to enumerate the types required
    to describe the protocol, it didn't seem that much worse. An ideal
    solution here would be to have some kind of root that we could generate
    schemas for in one go, but I'm not sure if that's compatible with how we
    generate the protocol today.