Repository navigation
fix: type the Go and ChatGPT usage limits - #57897
kitlangton wants to merge 2 commits into
Conversation
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
The change looks sound. Core now gives the Console Go-limit 429 and ChatGPT's subscription_sharing_usage_limit_exceeded their own error types, and Desktop and the TUI match on the type instead of a copied message. The removed session.status account_rate_limit listener had nothing in v2 feeding it, so removing it loses nothing.
I couldn't verify it end to end: the simulated model can only stream, so it can't return a real 429 with a GoUsageLimitError body. The new core, session-ui and TUI tests pass, and typecheck is clean in core, app, tui and session-ui. The Desktop dialog unit test fails the same way on the base (a solid-js server-side environment error), so that isn't caused by this change. Could you add before/after screenshots of the TUI notice and the Desktop "Go limit reached" dialog?
Public API: the error.type on session.step.failed events (and stored assistant errors) changes in two cases:
- Console
GoUsageLimitError:provider.quota→provider.go-limit, and its message is now core's own text - ChatGPT
subscription_sharing_usage_limit_exceeded: whatever type the failure got before (e.g.provider.invalid-request/provider.unknown) →provider.chatgpt-limit
The schema's type is a free string, so nothing breaks structurally. Clients or plugins that matched provider.quota for Go limits, or the old type for ChatGPT limits, would need to match the new values.
No blocking concerns.
Related: #56798
44ad706 to
9228371
Compare
a8938aa to
511c02b
Compare
kitlangton
changed the title
There was a problem hiding this comment.
The new commits look sound. Desktop no longer opens a Go dialog: it gives the prompt back and leaves a muted timeline line. The ChatGPT dialog still matches on provider.chatgpt-limit, and the core and TUI changes are the same as in the last review. None of my earlier threads were open.
I checked the core, session-ui and TUI tests (all pass) and type-checked app, core, tui and session-ui with tsc -b (clean). The packages' typecheck script now calls bun check, which the installed Bun doesn't have. I still couldn't reproduce a real Go-limit 429 end to end, because the simulated model can't return one.
Public API: no change since the last review. On session.step.failed, the Go Console limit moves from provider.quota to provider.go-limit and the ChatGPT limit becomes provider.chatgpt-limit. Consumers that match the old types need updating.
Not blocking: nothing uses the go_upsell_account_rate_limit_* saved-state keys or the dialog.usageExceeded.accountRateLimit.* strings any more. They can be removed here or with #58008.
511c02b to
f2c7793
Compare
There was a problem hiding this comment.
Still sound after the rebase. My earlier note is addressed: the retired go_upsell_account_rate_limit_* saved-state keys and the dialog.usageExceeded.accountRateLimit.* strings are gone, and the dialog test checks that the old keys are dropped. In the TUI, a Go limit now uses the shared usage-limit notice, with the reset time taken from retryAfterMs. It still gives the prompt back and skips both the upsell and the toast.
Checked with the core session-error, session-ui rows-current, TUI notifications/app-lifecycle and usage-limits tests (all pass) and tsc -b in app, core, tui, session-ui and ui (clean). I still couldn't reproduce a real Go-limit 429 end to end, because the simulated model can't return one.
Public API: unchanged since my last review. On session.step.failed, the Go Console limit moves from provider.quota to provider.go-limit and the ChatGPT limit becomes provider.chatgpt-limit. Consumers that match the old types need updating.
No new concerns.
There was a problem hiding this comment.
The new commit only removes the Record<string, string> annotation from the locale dict exports. That is a type-only change: no translation text changes, and every locale is now declared the same way as the ones that already lacked it. tsc -b is clean in ui, app and session-ui. None of my earlier threads were open.
Public API: unchanged since my last review. On session.step.failed, the Go Console limit moves from provider.quota to provider.go-limit and the ChatGPT limit becomes provider.chatgpt-limit. Consumers that match the old types need updating.
No new concerns.
What it does. Builds on #58694 and #58731. In v2, a Go subscriber who hit their Go limit got a red error and lost their prompt, and the ChatGPT usage-limit dialog never opened. Core now types both limits, so they get the same treatment as Adam's free and budget limits: the TUI shows its usage-limit notice and gives the prompt back, and Desktop opens the ChatGPT dialog again. The Desktop Go tray is separate (#58008).
How it works.
GoUsageLimitError429 becomesprovider.go-limit(keepingretryAfterMs), andsubscription_sharing_usage_limit_exceededbecomesprovider.chatgpt-limit.useLimitNoticehandlesprovider.go-limitthrough the existingUsageLimitNotice, with the reset time fromretryAfterMs. The prompt is restored like the free limit, the Go upsell and failure toast are skipped.session.statusaccount_rate_limitdialog is removed, along with itsaccountRateLimitandgo_upsell_account_rate_limit_*strings.Risk. Changes the public
session.step.failederror type: the Go Console limit moves fromprovider.quotatoprovider.go-limit, and the ChatGPT limit becomesprovider.chatgpt-limit. Plugins matchingprovider.quotafor these need updating. Until #58008 lands, Desktop users who hit a Go limit see only the muted timeline line.Proof. No media: the visible changes are a muted line and the existing notice.
session-error: 13 pass (real Console body withretry-after).rows-current: 52 pass. Appusage-exceeded-dialogs: 2 pass.e2e/regression/composer.spec.ts(free limit, Go limit, ChatGPT dialog): 3 pass.Added by the opencode bot (AI-generated).
Related: #56798
Public API: session.step.failed error.type: Go Console limit provider.quota → provider.go-limit; ChatGPT usage limit → provider.chatgpt-limit (structurally compatible; consumers matching the old types need updating)