feat(app-server): expose rate-limit reset credits#28143
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f8ed5c895
ℹ️ 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
owenlin0
left a comment
There was a problem hiding this comment.
just some API design nits
| #[serde(rename_all = "camelCase")] | ||
| #[ts(export_to = "v2/")] | ||
| pub struct ConsumeAccountRateLimitResetCreditParams { | ||
| pub redeem_request_id: String, |
There was a problem hiding this comment.
should we just call this idempotency_key? IMO it's a well-known term and conveys the expectation that the client should be using this to represent the same logical "reset".
also, worth documenting that we recommend a UUID for this field?
| Reset, | ||
| NothingToReset, | ||
| NoCredit, | ||
| AlreadyRedeemed, |
There was a problem hiding this comment.
this is for the idempotent case?
There was a problem hiding this comment.
Yup this is returned when the same id is retried after redemption is completed.
| #[serde(rename_all = "camelCase")] | ||
| #[ts(export_to = "v2/")] | ||
| pub struct ConsumeAccountRateLimitResetCreditResponse { | ||
| pub code: ConsumeAccountRateLimitResetCreditCode, |
There was a problem hiding this comment.
nit: can we call this result or outcome? and rename the struct accordingly? code kind of feels like it should be an int (like JSON-RPC or HTTP status code or something)
| #[ts(export_to = "v2/")] | ||
| pub struct ConsumeAccountRateLimitResetCreditResponse { | ||
| pub code: ConsumeAccountRateLimitResetCreditCode, | ||
| pub windows_reset: i64, |
There was a problem hiding this comment.
hmm, do we need to return the number of rate limit windows reset? seems like a weird contract. IMO let's keep it simple and only return result / outcome? either way, the client still has to refetch the latest rate limits via account/rateLimits/read
| #[ts(export_to = "v2/", rename_all = "snake_case")] | ||
| pub enum ConsumeAccountRateLimitResetCreditCode { | ||
| Reset, | ||
| NothingToReset, |
There was a problem hiding this comment.
worth documenting when this would be returned? it's not obvious to me
|
|
||
| #[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)] | ||
| #[serde(rename_all = "snake_case")] | ||
| #[ts(export_to = "v2/", rename_all = "snake_case")] |
There was a problem hiding this comment.
use camelCase for consistency with the rest of the app-server API (I thought our agents.md mentions this, but maybe not? 🤔 )
owenlin0
left a comment
There was a problem hiding this comment.
let's make the camelCase update but pre-approving
Why
Codex users can earn personal rate-limit reset credits, but app-server clients do not currently have an API for reading or redeeming them. This adds the backend and protocol foundation used by the
/usageTUI flow in #28154.What changed
account/rateLimits/readwith a nullablerateLimitResetCreditssummary sourced from the existing usage response.outcome; clients refetchaccount/rateLimits/readfor updated window state.AGENTS.mdthat new app-server string enum values use camelCase on the wire.Validation
just test -p codex-app-server-protocol— 231 passed.just test -p codex-backend-client— 14 passed.codex-app-serverreset-credit tests — 5 passed.codex-tuiprotocol response fixture test — passed.just fix -p codex-backend-client -p codex-app-server-protocol -p codex-app-server— passed.just fmt— passed.