fix(acp): surface actionable quota errors - #47805
Conversation
ACP cannot show retry actions while waiting hours for quota reset. End those retries so the pending prompt returns its provider error. Keep transient retries and default CLI action handling. Refs louiselm-ygtu Refs codex/01a07c27-5e8e-7e11-b285-8f49bcdbcf3f
|
The following comment was made by an LLM, it may be inaccurate: Based on my search, I found related PRs addressing quota retry handling: Related PRs:
These are not duplicates of PR #47805, but they are closely related PRs addressing similar quota retry handling. PR #47805 appears to be a continuation or refinement of the quota error handling work, specifically surfacing errors in the ACP client context. |
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped to ACP-only behavior, is consistent with existing retry/action semantics, and is backed by targeted subprocess tests covering both the regression and non-regression cases.
Pull request overview
This PR fixes ACP (OPENCODE_CLIENT=acp) sessions getting stuck “pending” for hours on actionable quota limits by stopping the interactive quota-retry path in the retry policy and letting the original provider error surface back through ACP’s existing error mapping.
Changes:
- Stop retries for retryable errors that include an interactive
actionwhen running under ACP, so the provider error is returned instead of waiting for long quota resets. - Extend the local HTTP test LLM server to emit response headers (e.g.,
retry-after) so retry behavior can be exercised realistically. - Add ACP subprocess coverage for quota-limit errors after a completed tool call, plus coverage that transient 429/503 still retry and default CLI quota retries remain interactive.
File summaries
| File | Description |
|---|---|
| packages/opencode/src/session/retry.ts | Ends actionable quota retries for ACP by short-circuiting when retry.action is present. |
| packages/opencode/test/lib/llm-server.ts | Allows HTTP error test responses to include headers like retry-after. |
| packages/opencode/test/cli/acp/prompt-error.test.ts | Adds regression and behavior tests validating ACP error surfacing and preserving existing retry behavior elsewhere. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Issue for this PR
Closes #47804
Related: #33055.
Type of change
What does this PR do?
Stop actionable quota retries when
OPENCODE_CLIENT=acp. ACP currently discards retry status, so an hours-long quota reset leavessession/promptsilently pending. Ending that retry lets the existing ACP error mapping return the original provider message.Ordinary transient retries and default CLI quota actions remain unchanged. No protocol additions, dependencies, or client-side watchdog changes.
How did you verify your code works?
Five subprocess cases use an isolated local HTTP provider: Go/free quota rejection after a completed tool, recovery in the same session, transient 429/503 retries, and default CLI quota retries. No credentials or real provider requests.
From
packages/opencode:bun test --timeout 60000 --only-failures test/cli/acp test/acp test/session/retry.test.ts test/session/processor-effect.test.ts bun typecheckRebased onto
devat57ef382: 225 tests pass; package type-checking and Prettier pass. The pre-push workspace type-check also passes all 30 tasks. Restoring upstream's unchanged retry code makes the Go-quota regression time out; restoring the fix makes it pass. Targeted lint reports zero errors and seven warnings on unchanged upstream lines.Earlier full-package run on the 1.18.29-based patch: 3575 passed, one unrelated
test/tool/write.test.tsmode assertion failed under umask 002 and passed under 022. That is not a full-suite result for the rebased branch.Screenshots / recordings
Not applicable: ACP error propagation, covered by subprocess tests.
Checklist