From a55b0c4bcc42089d7b53a1aebd5b31709c536c55 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sun, 26 Oct 2025 18:57:42 -0700 Subject: [PATCH] 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, + 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? --- codex-rs/app-server-protocol/src/export.rs | 30 ++++---------------- codex-rs/app-server-protocol/src/protocol.rs | 12 ++++---- 2 files changed, 12 insertions(+), 30 deletions(-) diff --git a/codex-rs/app-server-protocol/src/export.rs b/codex-rs/app-server-protocol/src/export.rs index 486abf8e..21ba3fac 100644 --- a/codex-rs/app-server-protocol/src/export.rs +++ b/codex-rs/app-server-protocol/src/export.rs @@ -23,7 +23,6 @@ use std::io::Write; use std::path::Path; use std::path::PathBuf; use std::process::Command; -use ts_rs::ExportError; use ts_rs::TS; const HEADER: &str = "// GENERATED CODE! DO NOT MODIFY BY HAND!\n\n"; @@ -105,19 +104,6 @@ macro_rules! for_each_schema_type { }; } -fn export_ts_with_context(label: &str, export: F) -> Result<()> -where - F: FnOnce() -> std::result::Result<(), ExportError>, -{ - match export() { - Ok(()) => Ok(()), - Err(ExportError::CannotBeExported(ty)) => Err(anyhow!( - "failed to export {label}: dependency {ty} cannot be exported" - )), - Err(err) => Err(err.into()), - } -} - pub fn generate_types(out_dir: &Path, prettier: Option<&Path>) -> Result<()> { generate_ts(out_dir, prettier)?; generate_json(out_dir)?; @@ -127,17 +113,13 @@ pub fn generate_types(out_dir: &Path, prettier: Option<&Path>) -> Result<()> { pub fn generate_ts(out_dir: &Path, prettier: Option<&Path>) -> Result<()> { ensure_dir(out_dir)?; - export_ts_with_context("ClientRequest", || ClientRequest::export_all_to(out_dir))?; - export_ts_with_context("client responses", || export_client_responses(out_dir))?; - export_ts_with_context("ClientNotification", || { - ClientNotification::export_all_to(out_dir) - })?; + ClientRequest::export_all_to(out_dir)?; + export_client_responses(out_dir)?; + ClientNotification::export_all_to(out_dir)?; - export_ts_with_context("ServerRequest", || ServerRequest::export_all_to(out_dir))?; - export_ts_with_context("server responses", || export_server_responses(out_dir))?; - export_ts_with_context("ServerNotification", || { - ServerNotification::export_all_to(out_dir) - })?; + ServerRequest::export_all_to(out_dir)?; + export_server_responses(out_dir)?; + ServerNotification::export_all_to(out_dir)?; generate_index_ts(out_dir)?; diff --git a/codex-rs/app-server-protocol/src/protocol.rs b/codex-rs/app-server-protocol/src/protocol.rs index 71a4d77f..ab7341a2 100644 --- a/codex-rs/app-server-protocol/src/protocol.rs +++ b/codex-rs/app-server-protocol/src/protocol.rs @@ -225,6 +225,12 @@ client_request_definitions! { }, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +pub struct GetAccountResponse { + pub account: Account, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema, TS)] #[serde(rename_all = "camelCase")] pub struct InitializeParams { @@ -535,12 +541,6 @@ pub struct GetAccountRateLimitsResponse { pub rate_limits: RateLimitSnapshot, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] -#[serde(transparent)] -#[ts(export)] -#[ts(type = "Account | null")] -pub struct GetAccountResponse(#[ts(type = "Account | null")] pub Option); - #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] pub struct GetAuthStatusResponse {