Skip to content

Conversation

@owenlin0
Copy link
Collaborator

@owenlin0 owenlin0 commented Feb 2, 2026

This updates our generated TypeScript types to be more correct with how the server actually behaves, specifically for JSON-RPC requests.

Before this PR, we'd generate field: T | null. After this PR, we will have field?: T | null. The latter matches how the server actually works, in that if an optional field is omitted, the server will treat it as null. This also makes it less annoying in theory for clients to upgrade to newer versions of Codex, since adding a new optional field to a JSON-RPC request should not require a client change.

NOTE: This only applies to JSON-RPC requests. All other payloads (i.e. responses, notifications) will return field: T | null as usual.

@owenlin0 owenlin0 force-pushed the owen/fix_ts_request_annotations branch from 34e3480 to 1acf0e3 Compare February 2, 2026 21:55
@owenlin0 owenlin0 marked this pull request as ready for review February 2, 2026 23:35
@owenlin0 owenlin0 requested review from gpeal and shijie-oai February 3, 2026 16:41
@owenlin0 owenlin0 force-pushed the owen/fix_ts_request_annotations branch from 1acf0e3 to 8182773 Compare February 3, 2026 16:45
@owenlin0 owenlin0 requested a review from bolinfest February 3, 2026 16:52
Copy link
Collaborator

@gpeal gpeal left a comment

Choose a reason for hiding this comment

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

Does this change ALL nullable fields to be nullable or optional or only actually optional ones?

I can't find the PR but I'm almost positive that, last year, used to do this and explicitly moved away from it because of the ambiguitiy between null and undefined in many cases. Let's make sure we square this with that decision before moving forward

Maybe it was this one?

@bolinfest @etraut-openai do you remember when/where we did that?

@owenlin0
Copy link
Collaborator Author

owenlin0 commented Feb 3, 2026

@gpeal Yes, I was the one who landed a change to explicitly move away from field?: T | null everywhere (this was all structs, whether they were sent by the client or server). This PR: #5939

However there is a distinction between structs sent by the client vs. structs sent by the server, and this only reintroduces field?: T | null for payloads sent by client->server.

@owenlin0 owenlin0 force-pushed the owen/fix_ts_request_annotations branch from 7b28dbe to 22f3837 Compare February 3, 2026 19:19
Copy link
Collaborator

@bolinfest bolinfest left a comment

Choose a reason for hiding this comment

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

Thanks for the thorough AGENTS.md update!

@owenlin0 owenlin0 merged commit efd96c4 into main Feb 3, 2026
55 of 59 checks passed
@owenlin0 owenlin0 deleted the owen/fix_ts_request_annotations branch February 3, 2026 19:51
@github-actions github-actions bot locked and limited conversation to collaborators Feb 3, 2026
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.

4 participants