Show and consume rate limit reset credits#144
Open
johnknott wants to merge 1 commit into
Open
Conversation
Contributor
Greptile SummaryThis PR adds a
Confidence Score: 4/5Safe to merge; the reset command is gated behind --yes, only operates on ChatGPT accounts, and the new RESETS column is additive with proper memory management throughout. The core reset flow, HTTP POST helper, UUID generation, and column rendering are all correct. Two minor issues: error.InvalidResponse from the JSON parser is missing from isHandledCliError (unlike the other new reset errors), and the removal of the plan/last initial-width caps in boundWidths silently changes narrow-terminal column layout beyond what the new column requires. src/cli/table_layout.zig (boundWidths cap changes) and src/workflows/preflight.zig (missing error.InvalidResponse in isHandledCliError) Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant CLI as codex-auth reset
participant Registry
participant AuthFile as auth.json
participant API as ChatGPT API
User->>CLI: codex-auth reset work --yes
CLI->>Registry: loadRegistry + syncActiveAccount
CLI->>Registry: resolveSwitchQueryLocally(selector)
Registry-->>CLI: account_key
CLI->>AuthFile: parseAuthInfo(auth_path)
AuthFile-->>CLI: access_token, chatgpt_account_id
CLI->>API: POST /wham/rate-limit-reset-credits/consume
API-->>CLI: code, windows_reset
CLI->>Registry: decrement reset_credits by 1
CLI->>Registry: saveRegistry
CLI->>User: Consumed reset credit for work
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant CLI as codex-auth reset
participant Registry
participant AuthFile as auth.json
participant API as ChatGPT API
User->>CLI: codex-auth reset work --yes
CLI->>Registry: loadRegistry + syncActiveAccount
CLI->>Registry: resolveSwitchQueryLocally(selector)
Registry-->>CLI: account_key
CLI->>AuthFile: parseAuthInfo(auth_path)
AuthFile-->>CLI: access_token, chatgpt_account_id
CLI->>API: POST /wham/rate-limit-reset-credits/consume
API-->>CLI: code, windows_reset
CLI->>Registry: decrement reset_credits by 1
CLI->>Registry: saveRegistry
CLI->>User: Consumed reset credit for work
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
src/workflows/preflight.zig:27-31
`error.InvalidResponse` missing from handled errors
`parseResetConsumeResponse` can return `error.InvalidResponse` when the API response body isn't a JSON object. This error propagates through `handleReset` (which does print a user-facing message via `printResetConsumeFailedError`) but isn't listed in `isHandledCliError`, unlike `error.RequestFailed`. Depending on how the root handles unrecognised errors, the user could see an internal "BUG: unhandled error" trace after the already-printed friendly message. Adding `error.InvalidResponse` here would keep the treatment consistent with the rest of the new reset errors.
### Issue 2 of 2
src/cli/table_layout.zig:129-134
Silent behavior change to `plan` and `last` initial-allocation caps
The previous code capped `plan` at `"PLAN".len` (4) and `last` at 3 chars during the first `growBoundedWidth` pass; both caps have been removed and replaced with the full column widths. On narrow terminals where space is limited, this allows longer plan names (e.g., "Business") and longer last-activity strings to consume space in the first pass, leaving less room for the email column — confirmed by the test expectation changing from `"very-l.mple.com"` to `"very-l.com"`. The change seems intentional (the test was updated), but it is an observable live-TUI layout change beyond just adding the new RESETS column, and it isn't mentioned in the PR description.
Reviews (1): Last reviewed commit: "feat: show and consume reset credits" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+27
to
+31
| err == error.ResetRequiresConfirmation or | ||
| err == error.MultipleAccountsMatched or | ||
| err == error.UnsupportedAuthMode or | ||
| err == error.MissingAuth or | ||
| err == error.RequestFailed or |
Contributor
There was a problem hiding this comment.
error.InvalidResponse missing from handled errors
parseResetConsumeResponse can return error.InvalidResponse when the API response body isn't a JSON object. This error propagates through handleReset (which does print a user-facing message via printResetConsumeFailedError) but isn't listed in isHandledCliError, unlike error.RequestFailed. Depending on how the root handles unrecognised errors, the user could see an internal "BUG: unhandled error" trace after the already-printed friendly message. Adding error.InvalidResponse here would keep the treatment consistent with the rest of the new reset errors.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/workflows/preflight.zig
Line: 27-31
Comment:
`error.InvalidResponse` missing from handled errors
`parseResetConsumeResponse` can return `error.InvalidResponse` when the API response body isn't a JSON object. This error propagates through `handleReset` (which does print a user-facing message via `printResetConsumeFailedError`) but isn't listed in `isHandledCliError`, unlike `error.RequestFailed`. Depending on how the root handles unrecognised errors, the user could see an internal "BUG: unhandled error" trace after the already-printed friendly message. Adding `error.InvalidResponse` here would keep the treatment consistent with the rest of the new reset errors.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
129
to
+134
| growBoundedWidth(&remaining, &bounded.email, @min(widths.email, live_account_ident_width)); | ||
| growBoundedWidth(&remaining, &bounded.rate_5h, @min(widths.rate_5h, @max(@as(usize, 4), "5H".len))); | ||
| growBoundedWidth(&remaining, &bounded.rate_week, @min(widths.rate_week, "WEEKLY".len)); | ||
| growBoundedWidth(&remaining, &bounded.plan, @min(widths.plan, "PLAN".len)); | ||
| growBoundedWidth(&remaining, &bounded.last, @min(widths.last, @as(usize, 3))); | ||
| growBoundedWidth(&remaining, &bounded.plan, widths.plan); | ||
| growBoundedWidth(&remaining, &bounded.last, widths.last); | ||
| growBoundedWidth(&remaining, &bounded.resets, @min(widths.resets, "RESETS".len)); |
Contributor
There was a problem hiding this comment.
plan and last initial-allocation caps
The previous code capped plan at "PLAN".len (4) and last at 3 chars during the first growBoundedWidth pass; both caps have been removed and replaced with the full column widths. On narrow terminals where space is limited, this allows longer plan names (e.g., "Business") and longer last-activity strings to consume space in the first pass, leaving less room for the email column — confirmed by the test expectation changing from "very-l.mple.com" to "very-l.com". The change seems intentional (the test was updated), but it is an observable live-TUI layout change beyond just adding the new RESETS column, and it isn't mentioned in the PR description.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/cli/table_layout.zig
Line: 129-134
Comment:
Silent behavior change to `plan` and `last` initial-allocation caps
The previous code capped `plan` at `"PLAN".len` (4) and `last` at 3 chars during the first `growBoundedWidth` pass; both caps have been removed and replaced with the full column widths. On narrow terminals where space is limited, this allows longer plan names (e.g., "Business") and longer last-activity strings to consume space in the first pass, leaving less room for the email column — confirmed by the test expectation changing from `"very-l.mple.com"` to `"very-l.com"`. The change seems intentional (the test was updated), but it is an observable live-TUI layout change beyond just adding the new RESETS column, and it isn't mentioned in the PR description.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Owner
|
Thanks, this is useful. I think we should narrow the scope of this PR. For now, I think we only need to surface the available count in I would prefer not to add the |
Author
|
Cool! Fair enough!
…On Thu, 18 Jun 2026 at 21:55, Loongphy ***@***.***> wrote:
*Loongphy* left a comment (Loongphy/codex-auth#144)
<#144 (comment)>
Thanks, this is useful. I think we should narrow the scope of this PR.
For now, I think we only need to surface the available count in list. I
would not add it to other account-selection surfaces in this PR, and the
column label should be RESET CREDITS.
I would prefer not to add the reset command to codex-auth. The official
CLI/TUI can own the reset-consumption flow later, so I do not think we need
to implement it here as well. There is already follow-up work on the Codex
side: openai/codex#27915 (comment)
<openai/codex#27915 (comment)>
—
Reply to this email directly, view it on GitHub
<#144?email_source=notifications&email_token=AABREN33N6KZHJ65EPRQTTT5ARJM7A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINZUGYYDKOJRGE2KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4746059114>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABREN52HYG2USF2FACMWM35ARJM7AVCNFSNUABGKJSXA33TNF2G64TZHMYTCNJUGI2DAOJUGA5US43TOVSTWNBWHEYTKNRRGI4DDILWAI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
--
Cheers, John
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds support for ChatGPT rate limit reset credits in codex-auth.
The
listcommand now shows aRESETScolumn with the number of banked reset credits returned by the usage API. This is especially useful on Linux, where there is currently no easy way to view or spend these banked rate limit resets because they are not shown in codex-cli.This also adds:
codex-auth reset <query> --yesto consume one reset credit for a stored ChatGPT accountrate_limit_reset_credits.available_countThe consume command requires
--yesso reset credits are not spent accidentally.