Skip to content

Show OpenCode provider quota failures as usage limits - #31937

Merged
brennanb2025 merged 5 commits into
mainfrom
brennanb2025/sta-11590-opencode-quota
Oct 23, 2026
Merged

brennanb2025 merged 5 commits into
mainfrom
brennanb2025/sta-11590-opencode-quota

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​125 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​125
Prod 4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​76 $\color{#cf222e}{\Huge{\mathbf{−}}}$​12 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​64

Problem

An OpenCode chat that reached its provider usage limit showed a generic “Internal error” instead of the existing usage-limit row and recovery actions. OpenCode returned a structured provider.quota error from its Agent Client Protocol (ACP) prompt request, but Orca treated that category as generic. That category also includes payment and expired-plan failures, so classifying every quota error as a pause would hide corrective instructions on the phone. Tracked in STA-11590.

User-facing change

OpenCode quota and rate-limit failures now use the existing warning usage-limit row, with the existing desktop Continue action when eligible. The phone shows the host's usage-limit sentence and any known reset time; this PR adds no phone Continue control. A reset time appears only when valid structured metadata supplies one; current OpenCode ACP errors supply none.

HTTP 402 and documented payment, empty-balance, overdue-invoice, expired GLM Coding Plan, missing model access and expired enterprise-package failures keep the existing generic error row and full corrective instruction. The separate provider.free-limit category also stays generic, retaining its connect/subscribe/switch-model instruction. No new error wording or controls are introduced.

Implementation and review path

The common pattern interprets a provider's failed reply at a provider-specific normalization boundary before shared failure handling. An optional ACP dialect hook feeds the existing usage-limit failure fact and existing turn/journal handling; other dialects retain their fallback. No saved-field definition, wire operation, transport or launch path changes.

Full review path. The change is small (88 changed production lines across four production files, plus 125 added test lines), but it affects accepted-turn endings, journaled failure facts and existing recovery actions, so the light-path risky-area and local-effect criteria do not hold. History includes the related shared usage-limit work in #30396 and error-detail preservation in #26666. A full plan and independent architecture/correctness review were completed before this review fix. Two review rounds and a fresh final-head comparison/readiness/performance review found no remaining source-proven findings; QA remains incomplete as listed below.

Intended classification differences and transport limits

  • Documented non-reset entitlement failures stay generic, including quota 429s. HTTP status alone cannot distinguish an expired plan or missing model entitlement from a resetting usage window. OpenCode documents expired GLM Coding Plan, bracket-wrapped expiry, missing glm-5 access and expired enterprise packages in provider-error.test.ts, and payment-method correction in cohere.test.ts. Preserving these instructions is intentional even when a native quota/status classifier would classify the 429 as usage. Resetting 1308/1310 examples remain on the usage path.
  • ACP loses status and timing. OpenCode's ACP translation and error serialization preserve category and message but discard native HTTP status and retry timing. The classifier excludes the documented corrective phrases that survive, rather than replacing the supported ACP transport. Optional structured reset coverage is a synthetic contract, not a captured current OpenCode timing payload.
  • Unknown wording remains lossy by design. A quota-category failure with undocumented billing/entitlement wording and no typed status remains a usage-limit row under the agreed category default. Desktop retains the provider detail; phone uses the existing host sentence. No reset timestamp is invented. This limitation is intentional and is separate from the documented non-reset exclusions above.

Validation

  • 30 host regression and contract tests passed across the two named ACP files after the main merge; the named mobile message file passed all 43 tests. Named tests ran through tc-gate. The mobile addition independently passed focused review.
  • Host tables cover exact upstream payment, expired-plan, wrapped-plan, missing-access and enterprise messages; resetting 1308/1310 messages; generic free-limit; typed 402 and existing billing cases. The original quota regression failed before the classifier was added. Phone contract cases exercise journal-item projection and the notice renderer: warning sentence, absent reset, reader-local supplied reset, and visible payment/entitlement instructions.
  • Current head is d4796e09d9e0e60497b857626227893f20936ddd. Full PR CI and native package/persistence checks passed on this exact head.
  • Real M4Air QA ran on 5fc2afa75f35196b1ec7c3ab423ed36c179271bf, compared with main 4dd97f02d5e240ab5d756017a946f8a9617678c6, using installed OpenCode 2.0.21. A baseline-created chat survived the actual restart/upgrade and returned two real replies. OpenCode, registered OpenCode2 and OMP controls returned real replies. OpenCode2 used a QA-home alias to the installed binary and an offered model through existing sign-in; installer detection was not tested.
  • Later commit 8d4eaf03a9cbfaf4580fe5f89a96f7e709864176 added only phone tests. The current head merges main 89e93cfa387188ac63b0ec711da4bf02873225bf because GitHub reported a conflict. Its sole conflict was in the host test; both main’s shell-result table and the PR’s quota/billing tables were retained. The four production files changed by this PR are unchanged across both commits, preserving the earlier real QA evidence under the test-only and non-overlapping-merge rules.
  • Controlled replay of the historical real-shaped quota payload changed main’s generic error into the existing warning and Continue row. A separate synthetic reset rendered a reader-local time. Billing/expired-plan/model-access/402/free-limit fixtures stayed generic. Continue after replay in an established real conversation produced a genuine reply. These are labelled replay/fixture checks: bounded ordinary provider requests succeeded, so no naturally occurring provider quota was observed.
  • The unchanged phone consumer is covered by contract tests under the current QA rules. Earlier browser setup failures remain historical non-passes; the queued iOS ticket was released before admission. No live pairing, transport or simulator pass is claimed. Grok and Antigravity are unchanged dialects; the real OMP control covered their shared default path.
  • QA evidence accepted. Brennan accepted this labelled replay of the reporter’s real-shaped quota response plus the genuine Continue reply as trigger evidence on 2026-10-23: "Yeah sure 31937 yes" (relayed by the lane owner). This acceptance does not assert that a naturally occurring provider quota was observed. The single review status comment retains the screenshots, actual QA head, subsequent commits and evidence limits.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 22, 2026 09:38
@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 48a85753-ed33-4e2c-98af-fac53858d979
📥 Commits

Reviewing files that changed from the base of the PR and between 8d4eaf0 and d4796e0.

📒 Files selected for processing: 1
  • src/main/acp/acp-timeline-opencode-dialect.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📝 Walkthrough

The ACP dialect contract now supports prompt-failure metadata and usage limits on turn endings. The OpenCode dialect classifies selected quota and rate-limit errors and derives reset times when possible. The translator passes this metadata to turn failure handling. Failure rows store usage limits and include them in usage-limit facts. Tests cover quota and rate-limit errors, billing-related errors, and mobile status-row rendering.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to d4796

The documented account-budget failures remain generic errors. No actionable merge-blocking defect is established; live quota and phone QA remain incomplete.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 6 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check Warning The description clearly explains the problem, user-facing behavior, implementation, classification rules, limitations, and validation results. However, it does not follow the required template in seve… Add the required template sections. Link the tracked issue with Fixes #<issue-number> or an equivalent actual issue reference. Add before/after visual evidence, or write N/A with a reason if there is no visual or interaction change. Com…
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary change: treating OpenCode provider quota failures as usage limits.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description clearly explains the problem, user-facing behavior, implementation, classification rules, limitations, and validation results. However, it does not follow the required template in several required areas: it has no actual linked issue under Fixes #, no Visual Proof section or N/A explanation, no AI Disclosure section, no Agent skill upstream boundary declaration, and no completed checklist.

Resolution

Add the required template sections. Link the tracked issue with Fixes #&lt;issue-number&gt; or an equivalent actual issue reference. Add before/after visual evidence, or write N/A with a reason if there is no visual or interaction change. Complete the AI Disclosure, Agent skill upstream boundary, Notes, and Checklist sections. Keep the existing detailed testing and QA limitations, including the statement that QA is incomplete and the PR is not ready to merge.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a08006e7-7a9d-4ee4-ba14-a92743683bb0
📥 Commits

Reviewing files that changed from the base of the PR and between 770e41f and 9df9f16.

📒 Files selected for processing: 2
  • src/main/acp/acp-dialects/opencode-dialect.ts
  • src/main/acp/acp-timeline-opencode-dialect.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

Comment thread src/main/acp/acp-dialects/opencode-dialect.ts
@brennanb2025

brennanb2025 commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor Author

OpenCode provider quota errors previously appeared as generic “Internal error” rows because their structured category was lost before shared failure handling. This PR routes those failures to the existing usage warning and eligible desktop Continue action, preserving documented payment and expired-plan instructions as generic errors.

Review status: complete; quota-replay evidence accepted. Current head is d4796e09d9e0e60497b857626227893f20936ddd. The production review took two rounds, with no remaining source-proven findings or deferred source fixes. The documented payment-method, expired-plan, model-access and enterprise-expiry exclusions were fixed and independently re-reviewed. Fresh final comparison with the common pattern and source readiness/performance review are complete. The later phone-test addition received an independent focused review with no changes needed.

Verified:

  • 30 named host regression/contract tests passed after the conflict-required main merge. The named mobile file passed all 43 tests. Current-head CI: Full PR CI and native package/persistence checks passed on this exact head.
  • Real M4Air QA ran on 5fc2afa75f35196b1ec7c3ab423ed36c179271bf. A chat created by main survived an actual restart/upgrade and returned two real replies. OpenCode, registered OpenCode2 and OMP controls returned real replies. OpenCode2 used a QA-home alias to the installed binary and an offered model through existing sign-in; installer detection was not tested.
  • Controlled replay, using the historical real-shaped quota payload, changed main’s generic error into the warning/Continue row. A separate synthetic reset case rendered a local reset time. Billing, expired-plan, model-access, HTTP 402 and free-limit fixtures stayed generic. Continue after a quota fixture in an established real conversation produced a genuine reply. These checks do not demonstrate a naturally occurring provider quota. Brennan accepted this labelled replay of the reporter’s real-shaped quota response plus the genuine Continue reply as trigger evidence on 2026-10-23: "Yeah sure 31937 yes" (relayed by the lane owner). This acceptance does not assert that a naturally occurring provider quota was observed.
  • The unchanged phone consumer is covered by tests under the current QA scope: actual journal-item projection and phone notice rendering preserve the host warning, omit an absent reset, format a supplied reset in the reader’s local clock, and display generic payment/entitlement instructions. The tests use the existing native-view harness; they do not claim live pairing, transport or device proof.

Evidence retained across later commits: 8d4eaf03a9cbfaf4580fe5f89a96f7e709864176 adds only phone contract tests. Merge d4796e09d9e0e60497b857626227893f20936ddd incorporates main 89e93cfa387188ac63b0ec711da4bf02873225bf; its only conflict was in the host test, resolved by preserving both main’s shell-result table and the PR’s quota/billing tables. None of the four production files changed by this PR changed across either commit. Earlier real QA remains applicable under the test-only and non-overlapping-main-merge rules; CI covers the merged head.

The earlier live phone browser setup failed twice before session-route proof. Those attempts remain historical non-passes. Since this PR changes no phone consumer code, the new unchanged-consumer rule replaces the additional live-phone requirement with contract coverage. The queued iOS ticket was released before admission; no simulator pass is claimed.

Grok and Antigravity are unchanged dialects; their shared default path is covered by the real OMP control. No additional live controls or waivers are claimed. The automated account-budget suggestion was resolved against the settled category default: that message supplies neither a corrective payment/renewal instruction nor a documented non-reset condition, so it remains usage-category output without an invented reset time. This does not assert that the budget actually resets. Current ACP status/timing loss and the documented unknown-wording limitation remain intentional, as described in the PR body.

All test copies and QA tickets are stopped/released. Reports, screenshots, raw captures and cleanup evidence are retained with their original archive hashes. No new phone or provider-trigger evidence is implied by the test-only follow-up.

For Brennan's review

The judgment points are the documented corrective-message exclusions and dialect classification before shared failure completion.

Same quota-payload replay, main versus the QA head 5fc2afa (production files unchanged on the current head):

Main: generic error (fixture) Final: usage warning/Continue (fixture)
Main quota replay Final quota replay
Synthetic reset comparison, billing exclusion, real continuation and upgrade replies
Main: synthetic reset payload Final: synthetic reset payload
Main synthetic reset Final synthetic reset

Billing fixture retains its corrective instruction:

Final billing fixture

Quota fixture in an established conversation, followed by a genuine Continue reply:

Fixture then real Continue reply

Baseline-created chat after upgrade, with two genuine replies on the QA head:

Real upgrade replies

@brennanb2025
brennanb2025 merged commit cd1f190 into main Oct 23, 2026
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant