Skip to content

Conversation

@owenlin0
Copy link
Contributor

@owenlin0 owenlin0 commented Oct 27, 2025

Makes sense to move this struct to app-server-protocol/ since we want to serialize as camelCase, but we don't for structs defined in protocol/

It was:

export type Account = { "type": "ApiKey", api_key: string, } | { "type": "chatgpt", email: string | null, plan_type: PlanType, };

But we want:

export type Account = { "type": "apiKey", apiKey: string, } | { "type": "chatgpt", email: string | null, planType: PlanType, };
@owenlin0 owenlin0 changed the title fix: move account struct to app-server-protocol and fix apiKey name Oct 27, 2025
@owenlin0 owenlin0 marked this pull request as ready for review October 27, 2025 19:47
@owenlin0 owenlin0 requested a review from bolinfest October 27, 2025 19:47
Copy link
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 239 to 250
#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema, TS)]
#[serde(tag = "type", rename_all = "camelCase")]
#[ts(tag = "type")]
pub enum Account {
#[serde(rename = "apiKey")]
#[ts(rename = "apiKey")]
ApiKey { api_key: String },
#[serde(rename = "chatgpt")]
#[ts(rename = "chatgpt")]
ChatGpt {
email: Option<String>,
plan_type: PlanType,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Use camelCase for Account payload fields

The new Account enum is intended to serialize as camelCase, but the fields api_key and plan_type remain in snake_case. #[serde(rename_all = "camelCase")] on the enum only affects variant names, so JSON and ts-rs output will still be { "type": "apiKey", "api_key": … } and { "type": "chatgpt", "plan_type": … }, which contradicts the stated goal of producing apiKey/planType. Any client updated to expect camelCase properties will fail to deserialize these messages. Rename the fields or add per-field/variant serde and ts renames to produce camelCase.

Useful? React with 👍 / 👎.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@owenlin0 can you verify this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

codex is right, updated and added a quick test to verify

@owenlin0 owenlin0 merged commit 67a219f into main Oct 27, 2025
20 checks passed
@owenlin0 owenlin0 deleted the owen/move_account_struct branch October 27, 2025 21:06
@github-actions github-actions bot locked and limited conversation to collaborators Oct 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

3 participants