Skip to content

Conversation

bolinfest
Copy link
Collaborator

@bolinfest bolinfest commented Sep 30, 2025

Manually curating protocol-ts/src/lib.rs was error-prone, as expected. I finally asked Codex to write some Rust macros so we can ensure that:

  • For every variant of ClientRequest and ServerRequest, there is an associated params and response type.
  • All response types are included automatically in the output of codex generate-ts.

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

Your team has set up Codex to 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 👍.

Comment on lines 674 to 676
paste! {
$(pub const [<$variant:snake:upper _METHOD>]: &str = stringify!([<$variant:camel>] );)*
}
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 Generate server method strings in PascalCase

The new server_request_definitions! macro derives the method constants with stringify!([<$variant:camel>]), which produces values like "ApplyPatchApproval". These strings are used in OutgoingMessageSender::send_request to populate the JSON‑RPC method field, but the ServerRequest enum is declared with #[serde(rename_all = "camelCase")], meaning clients expect method names such as "applyPatchApproval". Sending PascalCase names will cause clients and generated TS bindings to fail to match the request variants, so approval prompts will no longer be recognized.

Useful? React with 👍 / 👎.

@bolinfest bolinfest force-pushed the pr4529 branch 2 times, most recently from 33aa436 to ebe0b01 Compare September 30, 2025 23:11
@bolinfest bolinfest merged commit 32853ec into main Oct 1, 2025
40 checks passed
@bolinfest bolinfest deleted the pr4529 branch October 1, 2025 01:06
@github-actions github-actions bot locked and limited conversation to collaborators Oct 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants