fix(opencode-go): surface clear error on subscription quota exhaustion - #1749
Conversation
📝 WalkthroughWalkthroughAdds quota_exhausted handling for OpenAI-compatible failures, OpenCode Go quota messaging, retry gating updates, and a JSON fallback path for streamed responses. Expands tests for quota handling, retry behavior, and stream conversion. ChangesOpenCode Go Quota Exhaustion Handling
Estimated code review effort: 4 (Complex) | ~55 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/api/errors.opencodeGo.test.ts (1)
1-159:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd focused regression coverage for the new terminal retry branch.
This suite validates message/classification paths, but it does not cover the
shouldRetrychange insrc/services/api/withRetry.ts(Line 815-Line 830). Please add targeted tests for:
- OpenCode-Go
FreeUsageLimitError→ non-retryable- OpenCode-Go
GoUsageLimitError→ non-retryable- Non-OpenCode 429 with similar body markers → existing retry policy preserved
As per coding guidelines, "Add or update tests when the change affects behavior" and "Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/api/errors.opencodeGo.test.ts` around lines 1 - 159, The current test suite validates error message generation and classification paths but lacks focused regression tests for the retry behavior logic in the shouldRetry function from withRetry.ts. Add three new targeted test cases to this file: first, test that OpenCode-Go FreeUsageLimitError errors should not be retried (shouldRetry returns false), second, test that OpenCode-Go GoUsageLimitError errors should not be retried, and third, test that non-OpenCode 429 errors with similar error body structures should follow the existing retry policy (not be treated as non-retryable). Each test should create an appropriate error using makeGoError or APIError.generate and pass it to the shouldRetry function to verify the expected retry behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/api/errors.ts`:
- Around line 453-458: The OpenCode-Go detection logic currently checks both the
request URL header and the environment variable fallback independently with OR
semantics, causing misclassification when the header contains a non-OpenCode URL
but the environment variable contains an OpenCode URL. Fix this by establishing
proper precedence: only use envBaseUrl as a fallback when the url from the
x-opencode-request-url header is empty, not when it's present but doesn't match
OpenCode. Change the isOpencodeGo assignment to check url first, and only fall
back to checking envBaseUrl if url is empty, so that the actual request URL
takes precedence over the environment configuration.
In `@src/services/api/withRetry.ts`:
- Around line 823-827: The condition checking the OpenCode-Go gateway at lines
823-827 has a precedence issue where the OPENAI_BASE_URL environment variable
check can incorrectly override the header-based check. To fix this, restructure
the condition to prioritize the header check first by using `&&` logic to ensure
that if a header-based check determines it is not OpenCode-Go, the environment
variable check should not override that decision. The header from
error.headers?.get?.('x-opencode-request-url') should be checked as the primary
condition, and only if the header doesn't explicitly indicate opencode.ai/zen/go
should the fallback to
process.env.OPENAI_BASE_URL?.includes('opencode.ai/zen/go') be evaluated.
---
Outside diff comments:
In `@src/services/api/errors.opencodeGo.test.ts`:
- Around line 1-159: The current test suite validates error message generation
and classification paths but lacks focused regression tests for the retry
behavior logic in the shouldRetry function from withRetry.ts. Add three new
targeted test cases to this file: first, test that OpenCode-Go
FreeUsageLimitError errors should not be retried (shouldRetry returns false),
second, test that OpenCode-Go GoUsageLimitError errors should not be retried,
and third, test that non-OpenCode 429 errors with similar error body structures
should follow the existing retry policy (not be treated as non-retryable). Each
test should create an appropriate error using makeGoError or APIError.generate
and pass it to the shouldRetry function to verify the expected retry behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 39f3d9f6-eb68-4162-8016-9e577f15c002
📒 Files selected for processing (3)
src/services/api/errors.opencodeGo.test.tssrc/services/api/errors.tssrc/services/api/withRetry.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (13)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/errors.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/errors.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/errors.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/errors.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/errors.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/errors.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/errors.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/errors.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/errors.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/errors.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/services/api/errors.opencodeGo.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/services/api/errors.opencodeGo.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/errors.opencodeGo.test.ts
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found an issue that needs to be addressed before this is ready.
Findings
- [P2] Complete CodeRabbit's request to make the request URL authoritative
src/services/api/errors.ts:453
src/services/api/withRetry.ts:823
CodeRabbit's two header-precedence comments are still valid. Both the message parser and retry gate currently ORx-opencode-request-urlwithOPENAI_BASE_URL, so if a real response says it came from a non-OpenCode URL while stale env/profile state still hasOPENAI_BASE_URL=https://opencode.ai/zen/go/v1, the PR misclassifies the unrelated 429 as OpenCode Go quota exhaustion. I reproduced that shape withx-opencode-request-url: https://api.openai.com/v1/messages:getAssistantMessageFromErroremits the OpenCode Go subscription message, andwithRetrystops after one attempt instead of preserving the existing retry policy. Please complete CodeRabbit's request by making the explicit request URL authoritative, only falling back toOPENAI_BASE_URLwhen that header is absent, and add regression coverage for the retry path as well as the message path.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/integrations/runtimeMetadata.test.ts`:
- Around line 242-257: The test cases for resolveOpenAIShimRuntimeContext are
incomplete and only assert preserveReasoningContent and thinkingRequestFormat,
but do not validate the other newly inferred shim configuration fields. Add
expect assertions for the maxTokensField, removeBodyFields,
requireReasoningContentOnAssistantMessages, and reasoningContentFallback
properties in both the GLM paths test case (testing openrouter/zhipu/glm-5.2
model) and the direct glm model names test case (testing glm-5.2 model). These
assertions should validate that result.openaiShimConfig contains the correct
values for each of these fields to prevent silent regressions in GLM request
shaping behavior.
In `@src/services/api/openaiShim.ts`:
- Around line 1580-1633: The non-streaming JSON handler in the
openaiStreamToAnthropic function is missing logic to emit tool_use content
blocks from the choice.message.tool_calls array. After handling
reasoning_content and content blocks but before yielding message_delta, add code
to iterate through any tool_calls present in choice.message.tool_calls and yield
corresponding content_block_start, content_block_delta, and content_block_stop
events with type 'tool_use' for each tool call. Reference the
_convertNonStreamingResponse function to see the correct pattern for extracting
and formatting tool_calls, then apply that same logic here to ensure tool calls
are not dropped when a provider returns a complete JSON response instead of a
stream.
In `@src/services/api/withRetry.test.ts`:
- Around line 214-244: The test modifies the global
`process.env.OPENCLAUDE_RETRY_DELAY_MS` variable without restoring it afterward,
which can cause state leakage to subsequent tests. Save the original value of
`process.env.OPENCLAUDE_RETRY_DELAY_MS` before setting it to '1', then restore
it after the test completes using either a try/finally block or by storing and
resetting it at the end of the test function. This ensures the global
environment state is properly isolated and does not affect other tests.
In `@src/tools/FileReadTool/FileReadTool.ts`:
- Line 818: In the FileReadTool.ts file, locate the system reminder message
around line 818 that discusses reading files and malware analysis. This message
currently lacks an explicit refusal clause that prevents improving or augmenting
malware. Add back the explicit refusal-to-improve-malware sentence that
strengthens this security control, or if this change was intentional, coordinate
with maintainers for explicit approval of the policy change since this is a
security-sensitive path.
In `@tests/sdk/query-lifecycle.test.ts`:
- Around line 223-238: The test for interrupt() with reason "interrupt"
currently aborts the AbortController immediately before the query begins
iteration, which bypasses the stopHooks branch that checks signal.reason. Modify
the test to start consuming the query first (by iterating through at least one
message from the query) before calling ac.abort('interrupt'), then continue
draining the remaining messages and verify that the user cancellation message is
still suppressed in this in-flight interruption scenario. This ensures the test
actually exercises the signal.reason check within the stopHooks path rather than
just testing early abort behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 62f14ac3-b45f-4ff7-8503-0e99e5d38e5d
📒 Files selected for processing (14)
src/cli/print.tssrc/constants/prompts.tssrc/hooks/useReplBridge.tsxsrc/integrations/runtimeMetadata.test.tssrc/integrations/runtimeMetadata.tssrc/query/stopHooks.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiShim.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/tools/FileReadTool/FileReadTool.tstests/sdk/query-lifecycle.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (15)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/hooks/useReplBridge.tsxsrc/integrations/runtimeMetadata.tssrc/cli/print.tssrc/services/api/openaiErrorClassification.test.tssrc/integrations/runtimeMetadata.test.tssrc/tools/FileReadTool/FileReadTool.tssrc/query/stopHooks.tssrc/services/api/withRetry.test.tssrc/services/api/openaiShim.tssrc/constants/prompts.tssrc/services/api/withRetry.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/hooks/useReplBridge.tsxsrc/integrations/runtimeMetadata.tssrc/cli/print.tssrc/services/api/openaiErrorClassification.test.tssrc/integrations/runtimeMetadata.test.tssrc/tools/FileReadTool/FileReadTool.tssrc/query/stopHooks.tssrc/services/api/withRetry.test.tssrc/services/api/openaiShim.tstests/sdk/query-lifecycle.test.tssrc/constants/prompts.tssrc/services/api/withRetry.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/hooks/useReplBridge.tsxsrc/integrations/runtimeMetadata.tssrc/cli/print.tssrc/services/api/openaiErrorClassification.test.tssrc/integrations/runtimeMetadata.test.tssrc/tools/FileReadTool/FileReadTool.tssrc/query/stopHooks.tssrc/services/api/withRetry.test.tssrc/services/api/openaiShim.tstests/sdk/query-lifecycle.test.tssrc/constants/prompts.tssrc/services/api/withRetry.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/hooks/useReplBridge.tsxsrc/integrations/runtimeMetadata.tssrc/cli/print.tssrc/services/api/openaiErrorClassification.test.tssrc/integrations/runtimeMetadata.test.tssrc/tools/FileReadTool/FileReadTool.tssrc/query/stopHooks.tssrc/services/api/withRetry.test.tssrc/services/api/openaiShim.tstests/sdk/query-lifecycle.test.tssrc/constants/prompts.tssrc/services/api/withRetry.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/hooks/useReplBridge.tsxsrc/integrations/runtimeMetadata.tssrc/cli/print.tssrc/services/api/openaiErrorClassification.test.tssrc/integrations/runtimeMetadata.test.tssrc/tools/FileReadTool/FileReadTool.tssrc/query/stopHooks.tssrc/services/api/withRetry.test.tssrc/services/api/openaiShim.tstests/sdk/query-lifecycle.test.tssrc/constants/prompts.tssrc/services/api/withRetry.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/hooks/useReplBridge.tsxsrc/integrations/runtimeMetadata.tssrc/cli/print.tssrc/services/api/openaiErrorClassification.test.tssrc/integrations/runtimeMetadata.test.tssrc/tools/FileReadTool/FileReadTool.tssrc/query/stopHooks.tssrc/services/api/withRetry.test.tssrc/services/api/openaiShim.tstests/sdk/query-lifecycle.test.tssrc/constants/prompts.tssrc/services/api/withRetry.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/integrations/runtimeMetadata.tssrc/services/api/openaiErrorClassification.test.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/withRetry.test.tssrc/services/api/openaiShim.tssrc/services/api/withRetry.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.ts
src/integrations/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Check existing provider implementations before adding a new pattern
Files:
src/integrations/runtimeMetadata.tssrc/integrations/runtimeMetadata.test.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/integrations/runtimeMetadata.tssrc/services/api/openaiErrorClassification.test.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/withRetry.test.tssrc/services/api/openaiShim.tssrc/services/api/withRetry.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/withRetry.test.tssrc/services/api/openaiShim.tssrc/services/api/withRetry.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/withRetry.test.tssrc/services/api/openaiShim.tssrc/services/api/withRetry.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/services/api/openaiErrorClassification.test.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/withRetry.test.tstests/sdk/query-lifecycle.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/services/api/openaiErrorClassification.test.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/withRetry.test.tstests/sdk/query-lifecycle.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/openaiErrorClassification.test.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/withRetry.test.tstests/sdk/query-lifecycle.test.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/FileReadTool/FileReadTool.ts
🔇 Additional comments (10)
src/constants/prompts.ts (1)
22-24: LGTM!Also applies to: 289-292, 303-307, 363-367, 736-740
src/integrations/runtimeMetadata.ts (1)
201-211: LGTM!src/cli/print.ts (1)
2852-2852: LGTM!src/hooks/useReplBridge.tsx (1)
396-396: LGTM!src/query/stopHooks.ts (1)
325-329: LGTM!tests/sdk/query-lifecycle.test.ts (1)
240-268: LGTM!src/services/api/openaiErrorClassification.ts (1)
8-8: LGTM!Also applies to: 41-41, 204-220, 338-352, 470-495
src/services/api/errors.ts (1)
125-130: LGTM!src/services/api/withRetry.ts (1)
123-140: LGTM!src/services/api/openaiErrorClassification.test.ts (1)
225-260: LGTM!Also applies to: 270-300
98c6161 to
349eff6
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/services/api/withRetry.test.ts (1)
213-244:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore
OPENCLAUDE_RETRY_DELAY_MSafter this test to prevent env leakage.This test mutates global env state and leaves it set, which can affect subsequent retry tests.
As per path instructions, "Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state...".
Suggested fix
test('does not retry quota/allotment exhaustion failures', async () => { - process.env.OPENCLAUDE_RETRY_DELAY_MS = '1' - const { CannotRetryError, withRetry } = - await importFreshWithRetryModule('openai') - const error = APIError.generate( - 402, - undefined, - 'OpenAI API error 402: Payment Required [openai_category=quota_exhausted,host=opencode.ai] Hint: Provider quota or usage allotment has run out.', - new Headers(), - ) - let attempts = 0 - - await expect( - drainAsyncGenerator( - withRetry( - async () => ({} as Anthropic), - async () => { - attempts++ - throw error - }, - { - maxRetries: 2, - model: 'glm-5.1', - thinkingConfig: { type: 'disabled' }, - }, - ), - ), - ).rejects.toBeInstanceOf(CannotRetryError) - - expect(attempts).toBe(1) + const prevRetryDelay = process.env.OPENCLAUDE_RETRY_DELAY_MS + process.env.OPENCLAUDE_RETRY_DELAY_MS = '1' + try { + const { CannotRetryError, withRetry } = + await importFreshWithRetryModule('openai') + const error = APIError.generate( + 402, + undefined, + 'OpenAI API error 402: Payment Required [openai_category=quota_exhausted,host=opencode.ai] Hint: Provider quota or usage allotment has run out.', + new Headers(), + ) + let attempts = 0 + + await expect( + drainAsyncGenerator( + withRetry( + async () => ({} as Anthropic), + async () => { + attempts++ + throw error + }, + { + maxRetries: 2, + model: 'glm-5.1', + thinkingConfig: { type: 'disabled' }, + }, + ), + ), + ).rejects.toBeInstanceOf(CannotRetryError) + + expect(attempts).toBe(1) + } finally { + if (prevRetryDelay === undefined) { + delete process.env.OPENCLAUDE_RETRY_DELAY_MS + } else { + process.env.OPENCLAUDE_RETRY_DELAY_MS = prevRetryDelay + } + } })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/api/withRetry.test.ts` around lines 213 - 244, The test function "does not retry quota/allotment exhaustion failures" sets process.env.OPENCLAUDE_RETRY_DELAY_MS but does not restore it after execution, causing environment pollution for subsequent tests. After the final expect assertion, add cleanup code to delete or restore the original value of process.env.OPENCLAUDE_RETRY_DELAY_MS so that the global environment state is not modified for other tests.Source: Path instructions
src/services/api/openaiShim.ts (1)
1580-1633:⚠️ Potential issue | 🟠 Major | ⚡ Quick winJSON fast-path still drops
tool_calls, causing missed tool execution.The non-streaming
application/jsonbranch emits thinking/text then ends the message, but never convertschoice.message.tool_callsintotool_usecontent blocks. Tool-calling responses from providers that ignore streaming are silently degraded.Suggested fix
if (content) { yield { type: 'content_block_start', index: contentBlockIndex, content_block: { type: 'text', text: '' }, } yield { type: 'content_block_delta', index: contentBlockIndex, delta: { type: 'text_delta', text: content }, } yield { type: 'content_block_stop', index: contentBlockIndex, } contentBlockIndex++ } + // Preserve non-streaming tool calls in JSON fallback responses. + const toolCalls = Array.isArray(choice?.message?.tool_calls) + ? choice.message.tool_calls + : [] + for (const tc of toolCalls) { + const fnName = tc?.function?.name + const argsRaw = tc?.function?.arguments + if (!fnName) continue + let input: unknown = {} + if (typeof argsRaw === 'string' && argsRaw.length > 0) { + try { + input = JSON.parse(argsRaw) + } catch { + input = { arguments: argsRaw } + } + } + yield { + type: 'content_block_start', + index: contentBlockIndex, + content_block: { + type: 'tool_use', + id: tc?.id ?? `tool_${contentBlockIndex}`, + name: fnName, + input, + }, + } + yield { type: 'content_block_stop', index: contentBlockIndex } + contentBlockIndex++ + } + yield { type: 'message_delta',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/api/openaiShim.ts` around lines 1580 - 1633, The non-streaming response conversion in the openaiShim.ts function currently handles reasoning and content blocks but completely ignores tool_calls from the parsed response. After the content block handling (the if (content) block), add a new section that checks for choice.message.tool_calls and converts each tool call into a tool_use content block. For each tool call in the array, emit content_block_start with type 'tool_use', content_block_delta with the tool use details, and content_block_stop events, incrementing contentBlockIndex after each tool call, following the same pattern used for reasoning and content blocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/services/api/openaiShim.ts`:
- Around line 1580-1633: The non-streaming response conversion in the
openaiShim.ts function currently handles reasoning and content blocks but
completely ignores tool_calls from the parsed response. After the content block
handling (the if (content) block), add a new section that checks for
choice.message.tool_calls and converts each tool call into a tool_use content
block. For each tool call in the array, emit content_block_start with type
'tool_use', content_block_delta with the tool use details, and
content_block_stop events, incrementing contentBlockIndex after each tool call,
following the same pattern used for reasoning and content blocks.
In `@src/services/api/withRetry.test.ts`:
- Around line 213-244: The test function "does not retry quota/allotment
exhaustion failures" sets process.env.OPENCLAUDE_RETRY_DELAY_MS but does not
restore it after execution, causing environment pollution for subsequent tests.
After the final expect assertion, add cleanup code to delete or restore the
original value of process.env.OPENCLAUDE_RETRY_DELAY_MS so that the global
environment state is not modified for other tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e1878e52-fa2a-4c7f-b68c-12de8161602f
📒 Files selected for processing (7)
src/services/api/errors.opencodeGo.test.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiShim.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (13)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/errors.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/errors.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/errors.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/errors.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/errors.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/withRetry.test.tssrc/services/api/errors.opencodeGo.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/withRetry.test.tssrc/services/api/errors.opencodeGo.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/errors.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/errors.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/errors.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/errors.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/errors.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/withRetry.test.tssrc/services/api/errors.opencodeGo.test.ts
🔇 Additional comments (9)
src/services/api/withRetry.ts (1)
123-140: LGTM!Also applies to: 830-846, 1068-1069
src/services/api/errors.opencodeGo.test.ts (1)
10-10: LGTM!Also applies to: 161-210
src/services/api/openaiErrorClassification.test.ts (1)
234-263: LGTM!src/services/api/openaiErrorClassification.ts (4)
1-16: LGTM!
41-41: LGTM!
204-220: LGTM!
338-352: LGTM!src/services/api/errors.ts (2)
125-130: LGTM!
449-485: LGTM!
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
-
[P2] Preserve tool calls in the streaming JSON fallback
src/services/api/openaiShim.ts:1580
CodeRabbit's tool-call comment is still valid. The newapplication/jsonfallback insideopenaiStreamToAnthropicconverts onlyreasoning_contentandmessage.content, then returns before the normal stream parser or_convertNonStreamingResponsecan handlechoice.message.tool_calls. When an OpenAI-compatible provider ignoresstream: trueand returns a full JSON response containingtool_calls, the shim now emits notool_useblock and ends the message instead, so the requested tool is never executed. Please complete that review request by reusing the existing non-streaming conversion behavior or adding equivalenttool_callshandling in this fallback path. -
[P2] Normalize JSON fallback stop reasons before emitting stream events
src/services/api/openaiShim.ts:1621
The same JSON fallback passeschoice.finish_reasondirectly into the Anthropic-shapedmessage_delta. That leaks OpenAI values such asstop,length, andtool_callsinstead of the rest of this shim'send_turn,max_tokens, andtool_usevalues. I reproduced the fallback withstream: trueJSON responses and sawstop_reason: "length"andstop_reason: "tool_calls"emitted verbatim. Downstream code expects the Anthropic stop-reason vocabulary, so max-token truncation and tool-use turns from providers that return JSON on a streaming request are misclassified. Please map these finish reasons the same way the streaming parser and_convertNonStreamingResponsealready do. -
[P2] Convert array content before emitting text deltas
src/services/api/openaiShim.ts:1582
The JSON fallback also assignschoice.message.contentdirectly tocontentand then emits it asdelta: { type: 'text_delta', text: content }. OpenAI-compatible JSON responses can use array content parts, and_convertNonStreamingResponsealready joins text parts before returning Anthropic content. In the new fallback, the same response shape produces a stream event whosetext_delta.textis an array of content objects rather than a string, which is not a valid Anthropic text delta and can break consumers that concatenate or render streamed text. Please normalize array content to text blocks before emitting the delta, or route the fallback through the existing non-streaming converter. -
[P2] Keep think-tag filtering on JSON fallback text
src/services/api/openaiShim.ts:1582
Both the regular streaming parser and_convertNonStreamingResponsestrip<think>...</think>blocks before surfacing assistant text, and this file has regression tests for that behavior. The new JSON fallback bypasses both paths and emitschoice.message.contentverbatim; with a JSON response containing<think>private plan</think>visible answer, the streamed text delta includes the entire<think>block. Providers that ignore streaming but return JSON can therefore expose hidden reasoning that the shim normally filters. Please run JSON fallback text through the same think-tag filter/stripper before emitting it. -
[P2] Preserve raw text tool-call fallback behavior for JSON responses
src/services/api/openaiShim.ts:1582
The established streaming and non-streaming converters also detect raw tool-call JSON embedded in assistant text, such as{"name":"Bash","arguments":{"command":"pwd"}}, and convert it to atool_useblock so local/Ollama-style providers can still execute tools. The new JSON fallback emits that raw JSON as ordinary text and ends the turn, so providers that return a non-SSE JSON response on a streamed request lose the tool call even when the existing converter logic would have recovered it. Please reuse the existing raw text tool-call parsing or delegate this fallback to the non-streaming converter.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found an issue that needs to be addressed before this is ready.
Findings
-
[P2] Preserve the new OpenCode Go guidance through the retry wrapper
src/services/api/withRetry.ts:299
The earlyisQuotaExhausted(error)block runs before the PR's OpenCode Go-specific retry/message handling can surface the new guidance. I reproduced an OpenCode GoFreeUsageLimitErrorwithx-opencode-request-url: https://opencode.ai/zen/go/v1/messagesthroughwithRetry; instead of preservingOpenCode Go free usage exhausted · Subscribe at https://opencode.ai/go, the thrownCannotRetryErrorwraps the old generic text:API quota exhausted or not enabled. Fix: Enable billing for your provider.... That means at least the free-tier exhaustion path can still show the unclear message this PR is meant to replace. Please let the OpenCode Go quota APIError survive as the original error, or otherwise preserve the specific OpenCode Go assistant message before the generic quota guard converts it. -
[P2] Complete CodeRabbit's request to preserve non-stream JSON conversion semantics
src/services/api/openaiShim.ts:1580
CodeRabbit's current JSON-fallback review item is still valid. The newapplication/jsonbranch insideopenaiStreamToAnthropichand-rolls a much smaller converter than the existing non-streaming path: it only emitsreasoning_contentandmessage.content, then forwardschoice.finish_reasondirectly. That dropschoice.message.tool_calls, turnsfinish_reason: "tool_calls"or"stop"into invalid Anthropic stop reasons instead oftool_use/end_turn, skips array-content normalization, bypasses<think>...</think>stripping, and loses the raw text tool-call recovery that_convertNonStreamingResponsealready applies. A provider that ignoresstream: trueand returns a normal JSON chat completion can therefore appear successful while silently ending a tool turn as plain text or exposing hidden reasoning. Please complete that review request by reusing the established non-stream converter for this fallback, or by mirroring its tool-call, stop-reason, text-normalization, think-filtering, and raw-tool-call behavior before yielding stream events.
…verter in JSON fallback Addresses the two remaining P2 review items on Twigpine#1749. withRetry: the early isQuotaExhausted guard wrapped OpenCode Go FreeUsageLimitError/GoUsageLimitError 429s in the generic "API quota exhausted or not enabled" message, clobbering the actionable subscribe/ reset guidance. Skip the generic guard for OpenCode Go quota errors so they fall through to the standard shouldRetry=false terminal path, which rethrows the original APIError and lets getAssistantMessageFromError surface the specific message. Consolidate detection in a shared isOpenCodeGoQuotaError predicate (errors.ts) and drop the duplicated inline header check in shouldRetry. openaiShim: the application/json fallback in openaiStreamToAnthropic hand-rolled a thin converter that dropped tool_calls, forwarded raw OpenAI finish_reason values as Anthropic stop reasons, skipped array-content normalization, bypassed <think> stripping, and lost raw text tool-call recovery. Extract the established non-streaming conversion into a shared convertNonStreamingResponseToAnthropicMessage and route the fallback through it, re-emitting the result as stream events. _convertNonStreamingResponse now delegates to the same function. Adds regression coverage: a withRetry test proving the OpenCode Go message survives the retry loop, and JSON-fallback tests for tool_calls, stop-reason mapping, <think> stripping, array content, and raw text tool-call recovery.
|
Addressed both remaining P2 items in d502556. 1. Preserve the OpenCode Go guidance through the retry wrapper ( 2. Reuse the established non-stream converter in the JSON fallback ( Verification (mutation-proofed): reverting both production changes while keeping the new tests makes them fail — the withRetry regression (asserts the original |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/api/openaiShim.test.ts`:
- Around line 7335-7353: The collectFallbackEvents helper leaves a global fetch
stub installed, which can leak state across tests. Update collectFallbackEvents
in openaiShim.test.ts to save the existing globalThis.fetch before overriding
it, then restore it in a finally block after the stream is consumed. Keep the
fix localized to the helper and ensure createOpenAIShimClient and
makeJsonChatCompletion still work with the temporary stub.
In `@src/services/api/openaiShim.ts`:
- Around line 1544-1547: Treat empty tool call arrays as missing in the raw
recovery paths. In openaiShim.ts, update the logic around
parseRawToolCallsRequestedText so that the check uses the actual length of
choice.message.tool_calls instead of relying on truthiness, and apply the same
hasStructuredToolCalls-style condition in both the strippedContent branch and
the array-content branch. This ensures raw “Tool calls requested” recovery only
skips when structured tool calls are truly present.
In `@src/services/api/withRetry.ts`:
- Around line 299-304: OpenCode Go quota errors are currently reaching the
fast-mode 429 fallback in withRetry before the terminal quota path, which can
trigger an unnecessary retry or cooldown instead of surfacing the quota
guidance. Update the retry handling in src/services/api/withRetry.ts so that
isOpenCodeGoQuotaError(error) is intercepted before the fast-mode branch and
immediately returned as a CannotRetryError(error, retryContext). Keep the
existing quota-exhausted flow for non-OpenCode-Go errors so shouldRetry() and
getAssistantMessageFromError still behave as intended.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 56dcc5af-b046-4a41-94ac-bd7fcc48b44d
📒 Files selected for processing (5)
src/services/api/errors.tssrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (13)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/services/api/openaiShim.test.tssrc/services/api/errors.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/api/openaiShim.test.tssrc/services/api/errors.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/services/api/openaiShim.test.tssrc/services/api/errors.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/services/api/openaiShim.test.tssrc/services/api/errors.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/services/api/openaiShim.test.tssrc/services/api/errors.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/services/api/openaiShim.test.tssrc/services/api/withRetry.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/services/api/openaiShim.test.tssrc/services/api/withRetry.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/services/api/openaiShim.test.tssrc/services/api/errors.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/services/api/openaiShim.test.tssrc/services/api/errors.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/services/api/openaiShim.test.tssrc/services/api/errors.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/openaiShim.test.tssrc/services/api/errors.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/openaiShim.test.tssrc/services/api/errors.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/openaiShim.test.tssrc/services/api/withRetry.test.ts
🔇 Additional comments (5)
src/services/api/withRetry.test.ts (1)
245-301: Same env-leak feedback applies here.This test sets
process.env.OPENCLAUDE_RETRY_DELAY_MSwithout restoring it, so later tests can inherit the forced retry delay. Wrap the mutation intry/finallyor restore it in local cleanup.src/services/api/errors.ts (1)
489-495: LGTM!src/services/api/openaiShim.ts (1)
1741-1798: LGTM!Also applies to: 3635-3638
src/services/api/openaiShim.test.ts (1)
7355-7504: LGTM!src/services/api/withRetry.ts (1)
829-829: 🎯 Functional CorrectnessNo change needed for
shouldRetry
The test intentionally narrowsshouldRetryto a one-argument type, so the direct call does not create a strict typecheck issue.> Likely an incorrect or invalid review comment.
…calls as absent Addresses CodeRabbit's review on Twigpine#1749. - [Major] withRetry.ts: throw CannotRetryError for isOpenCodeGoQuotaError BEFORE the fast-mode 429 fallback. Previously the guard only *skipped* the generic quota throw, so an OpenCode Go 429 while fast mode was active hit the fast-mode retry/cooldown path instead of surfacing the quota message immediately. Wrapping the original APIError still preserves the OpenCode Go assistant message via getAssistantMessageFromError. Adds a fast-mode regression test (mutation-checked) and a forceFastMode option on the test helper. - [Minor] openaiShim.ts: an empty tool_calls array is truthy, which skipped the raw "Tool calls requested" recovery in convertNonStreamingResponseToAnthropicMessage. Gate on a single hasStructuredToolCalls (length > 0) check across both the string- and array-content raw-recovery paths and the structured loop. Adds a JSON-fallback regression test for tool_calls: [] (mutation-checked). - [Minor] openaiShim.test.ts: collectFallbackEvents now saves and restores globalThis.fetch in a finally block so the stub can't leak past the helper.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/services/api/withRetry.test.ts (1)
311-355: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRestore
OPENCLAUDE_RETRY_DELAY_MSafter this test.Line 315 mutates global env state and never restores it, so later retry tests can inherit the shortened delay and become order-dependent.
Suggested fix
test('terminates OpenCode Go quota 429 immediately in fast mode (no fast-mode retry/cooldown)', async () => { + const previousRetryDelay = process.env.OPENCLAUDE_RETRY_DELAY_MS // Regression for `#1749` (CodeRabbit): the OpenCode Go terminal throw must run // BEFORE the fast-mode 429 fallback, otherwise fast mode retries/cooldowns a // quota-exhausted subscription instead of surfacing the quota message. process.env.OPENCLAUDE_RETRY_DELAY_MS = '1' - const { CannotRetryError, withRetry } = - await importFreshWithRetryModule('openai', { forceFastMode: true }) - const error = APIError.generate( - 429, - undefined, - JSON.stringify({ - error: { type: 'GoUsageLimitError', message: 'subscription limit reached' }, - }), - new Headers({ - 'x-opencode-request-url': 'https://opencode.ai/zen/go/v1/messages', - }), - ) - let attempts = 0 - - let caught: unknown try { - await drainAsyncGenerator( - withRetry( - async () => ({} as Anthropic), - async () => { - attempts++ - throw error - }, - { - maxRetries: 2, - model: 'glm-5.1', - thinkingConfig: { type: 'disabled' }, - fastMode: true, - }, - ), - ) - } catch (e) { - caught = e + const { CannotRetryError, withRetry } = + await importFreshWithRetryModule('openai', { forceFastMode: true }) + const error = APIError.generate( + 429, + undefined, + JSON.stringify({ + error: { type: 'GoUsageLimitError', message: 'subscription limit reached' }, + }), + new Headers({ + 'x-opencode-request-url': 'https://opencode.ai/zen/go/v1/messages', + }), + ) + let attempts = 0 + + let caught: unknown + try { + await drainAsyncGenerator( + withRetry( + async () => ({} as Anthropic), + async () => { + attempts++ + throw error + }, + { + maxRetries: 2, + model: 'glm-5.1', + thinkingConfig: { type: 'disabled' }, + fastMode: true, + }, + ), + ) + } catch (e) { + caught = e + } + + expect(caught).toBeInstanceOf(CannotRetryError) + expect(attempts).toBe(1) + expect((caught as { originalError?: unknown }).originalError).toBe(error) + } finally { + if (previousRetryDelay === undefined) { + delete process.env.OPENCLAUDE_RETRY_DELAY_MS + } else { + process.env.OPENCLAUDE_RETRY_DELAY_MS = previousRetryDelay + } } - - expect(caught).toBeInstanceOf(CannotRetryError) - // Fired exactly once — fast mode did not retry or enter cooldown. - expect(attempts).toBe(1) - expect((caught as { originalError?: unknown }).originalError).toBe(error) })As per path instructions, “Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state...”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/api/withRetry.test.ts` around lines 311 - 355, This test mutates the global OPENCLAUDE_RETRY_DELAY_MS environment variable and leaves it changed, which can leak into later retry tests. Update withRetry.test’s fast-mode quota test to save the previous env value before setting it, then restore it in a cleanup path after the test (for example via a try/finally or test hook) so the suite stays isolated. Use the existing withRetry and importFreshWithRetryModule test setup as the place to apply and undo the env change.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/api/openaiShim.test.ts`:
- Around line 7512-7542: The empty tool_calls regression is only covered for
string-based assistant content, so the array-content recovery path in the
fallback logic is still untested. Add a companion test alongside
collectFallbackEvents that uses an assistant message with content as an array
and tool_calls: [] , then assert the same tool_use recovery via the existing
content_block_start lookup so the array branch in openaiShim cannot regress.
Keep the new case aligned with the current JSON fallback test naming and reuse
the same assertions around the recovered Bash tool call.
---
Duplicate comments:
In `@src/services/api/withRetry.test.ts`:
- Around line 311-355: This test mutates the global OPENCLAUDE_RETRY_DELAY_MS
environment variable and leaves it changed, which can leak into later retry
tests. Update withRetry.test’s fast-mode quota test to save the previous env
value before setting it, then restore it in a cleanup path after the test (for
example via a try/finally or test hook) so the suite stays isolated. Use the
existing withRetry and importFreshWithRetryModule test setup as the place to
apply and undo the env change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 10d64b6c-4d41-4bf5-acde-bfb5bc30c187
📒 Files selected for processing (4)
src/services/api/openaiShim.test.tssrc/services/api/openaiShim.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (13)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/services/api/openaiShim.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/api/openaiShim.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/services/api/openaiShim.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/services/api/openaiShim.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/services/api/openaiShim.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/services/api/openaiShim.test.tssrc/services/api/withRetry.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/services/api/openaiShim.test.tssrc/services/api/withRetry.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/services/api/openaiShim.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/services/api/openaiShim.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/services/api/openaiShim.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/openaiShim.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/openaiShim.test.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.tssrc/services/api/openaiShim.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/openaiShim.test.tssrc/services/api/withRetry.test.ts
🔇 Additional comments (4)
src/services/api/withRetry.ts (1)
299-307: LGTM!src/services/api/withRetry.test.ts (1)
76-97: LGTM!src/services/api/openaiShim.ts (1)
1524-1654: LGTM!src/services/api/openaiShim.test.ts (1)
7338-7357: LGTM!
|
Addressed all three CodeRabbit findings in d5a2198.
Validation: |
…nt too Addresses CodeRabbit's follow-up on Twigpine#1749: the empty-tool_calls regression only exercised the string-content path, but the hasStructuredToolCalls fix also gates the array-content branch. Add a companion JSON-fallback test with array-form message.content and tool_calls: [] so the array branch can't regress silently. Mutation-checked: reverting hasStructuredToolCalls to a truthiness check fails it.
|
Addressed the follow-up in the latest commit — added a companion |
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
No findings. The current code implements the stated goal and the prior valid-reviewer requests. Validation is clean.
@kevincodex1 LGTM
…verter in JSON fallback Addresses the two remaining P2 review items on Twigpine#1749. withRetry: the early isQuotaExhausted guard wrapped OpenCode Go FreeUsageLimitError/GoUsageLimitError 429s in the generic "API quota exhausted or not enabled" message, clobbering the actionable subscribe/ reset guidance. Skip the generic guard for OpenCode Go quota errors so they fall through to the standard shouldRetry=false terminal path, which rethrows the original APIError and lets getAssistantMessageFromError surface the specific message. Consolidate detection in a shared isOpenCodeGoQuotaError predicate (errors.ts) and drop the duplicated inline header check in shouldRetry. openaiShim: the application/json fallback in openaiStreamToAnthropic hand-rolled a thin converter that dropped tool_calls, forwarded raw OpenAI finish_reason values as Anthropic stop reasons, skipped array-content normalization, bypassed <think> stripping, and lost raw text tool-call recovery. Extract the established non-streaming conversion into a shared convertNonStreamingResponseToAnthropicMessage and route the fallback through it, re-emitting the result as stream events. _convertNonStreamingResponse now delegates to the same function. Adds regression coverage: a withRetry test proving the OpenCode Go message survives the retry loop, and JSON-fallback tests for tool_calls, stop-reason mapping, <think> stripping, array content, and raw text tool-call recovery.
…calls as absent Addresses CodeRabbit's review on Twigpine#1749. - [Major] withRetry.ts: throw CannotRetryError for isOpenCodeGoQuotaError BEFORE the fast-mode 429 fallback. Previously the guard only *skipped* the generic quota throw, so an OpenCode Go 429 while fast mode was active hit the fast-mode retry/cooldown path instead of surfacing the quota message immediately. Wrapping the original APIError still preserves the OpenCode Go assistant message via getAssistantMessageFromError. Adds a fast-mode regression test (mutation-checked) and a forceFastMode option on the test helper. - [Minor] openaiShim.ts: an empty tool_calls array is truthy, which skipped the raw "Tool calls requested" recovery in convertNonStreamingResponseToAnthropicMessage. Gate on a single hasStructuredToolCalls (length > 0) check across both the string- and array-content raw-recovery paths and the structured loop. Adds a JSON-fallback regression test for tool_calls: [] (mutation-checked). - [Minor] openaiShim.test.ts: collectFallbackEvents now saves and restores globalThis.fetch in a finally block so the stub can't leak past the helper.
…nt too Addresses CodeRabbit's follow-up on Twigpine#1749: the empty-tool_calls regression only exercised the string-content path, but the hasStructuredToolCalls fix also gates the array-content branch. Add a companion JSON-fallback test with array-form message.content and tool_calls: [] so the array branch can't regress silently. Mutation-checked: reverting hasStructuredToolCalls to a truthiness check fails it.
6fc4c8f to
360037c
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/services/api/withRetry.test.ts (1)
221-355: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRestore
OPENCLAUDE_RETRY_DELAY_MSin these tests.Lines 222, 257, and 315 mutate global env state without cleanup, so retry timing can leak into later tests. Wrap each test in
try/finallyor use a small restore helper. This was already flagged on the earlier quota test and now applies to the added OpenCode Go cases too.Proposed helper pattern
+async function withRetryDelayEnv<T>(value: string, fn: () => Promise<T>): Promise<T> { + const previous = process.env.OPENCLAUDE_RETRY_DELAY_MS + process.env.OPENCLAUDE_RETRY_DELAY_MS = value + try { + return await fn() + } finally { + if (previous === undefined) { + delete process.env.OPENCLAUDE_RETRY_DELAY_MS + } else { + process.env.OPENCLAUDE_RETRY_DELAY_MS = previous + } + } +}As per path instructions, “Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state...”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/api/withRetry.test.ts` around lines 221 - 355, The three retry tests mutate OPENCLAUDE_RETRY_DELAY_MS but never restore it, which can leak retry timing into later tests. Update each affected test in withRetry.test.ts to save the prior env value before setting it and restore it afterward with try/finally or a shared cleanup helper. Use the existing test blocks around withRetry, CannotRetryError, and drainAsyncGenerator to keep the fix local and consistent.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/api/errors.ts`:
- Around line 449-464: The OpenCode Go detection in parseOpenCodeGoLimitError is
reading a header that the openaiShim error path does not populate, so the
classification can miss provider/profile requests to opencode.ai/zen/go. Fix
this by sourcing the request URL from the existing compatibility metadata
already available on the APIError path, or update openaiShim to attach the
x-opencode-request-url header when constructing errors, and make sure
parseOpenCodeGoLimitError uses that same source consistently.
- Around line 788-814: The OpenCode Go quota handling in
getAssistantMessageFromError is still bypassed on the shim path because
classifyOpenAIHttpFailure() marks these 429s as quota_exhausted before the new
parseOpenCodeGoLimitError branch runs. Move the OpenCode Go detection ahead of
the generic OpenAI-compatibility marker handling, or add a special-case inside
that marker block so FreeUsageLimitError and GoUsageLimitError are preserved.
Keep the existing createAssistantAPIErrorMessage flow and reuse
parseOpenCodeGoLimitError, APIError, and the quota_exhausted handling in
src/services/api/errors.ts.
In `@src/services/api/openaiErrorClassification.ts`:
- Around line 204-218: The quota classification in isQuotaExhaustedMessage is
too broad because it matches the standalone token billing, causing unrelated
400/403/429 responses to be misclassified as quota_exhausted. Tighten the checks
in this helper and the related early branch in openaiErrorClassification so they
only match quota-specific phrases like credit, quota, payment required, or usage
limit, and avoid treating generic billing-related bad requests as exhausted
quota.
In `@src/services/api/withRetry.ts`:
- Around line 305-307: The quota-handling logic in withRetry is still wrapping
OpenAI-compatible quota APIErrors in a generic Error before they reach
getAssistantMessageFromError, so preserve them just like the existing
isOpenCodeGoQuotaError branch. Update the isQuotaExhausted guard to detect the
OpenAI-compatible quota category and throw a CannotRetryError with the original
APIError instead of wrapping it generically. Keep the handling aligned with the
existing retryContext flow so both quota-preservation branches behave
consistently.
---
Duplicate comments:
In `@src/services/api/withRetry.test.ts`:
- Around line 221-355: The three retry tests mutate OPENCLAUDE_RETRY_DELAY_MS
but never restore it, which can leak retry timing into later tests. Update each
affected test in withRetry.test.ts to save the prior env value before setting it
and restore it afterward with try/finally or a shared cleanup helper. Use the
existing test blocks around withRetry, CannotRetryError, and drainAsyncGenerator
to keep the fix local and consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d08ed620-1a46-45ee-935d-dbfc217603dd
📒 Files selected for processing (8)
src/services/api/errors.opencodeGo.test.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.tssrc/services/api/withRetry.test.tssrc/services/api/withRetry.ts
📜 Review details
⚠️ CI failures not shown inline (4)
GitHub Actions: PR Checks / typecheck: fix(opencode-go): surface clear error on subscription quota exhaustion
Conclusion: failure
##[group]Run bun run typecheck
�[36;1mbun run typecheck�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
$ tsc --noEmit
src/services/api/openaiShim.test.ts(8599,1): error TS1005: '}' expected.
##[error]Process completed with exit code 2.
GitHub Actions: PR Checks / smoke-and-tests: fix(opencode-go): surface clear error on subscription quota exhaustion
Conclusion: failure
##[group]src/components/agents/new-agent-creation/wizard-steps/wizardSteps.test.tsx:
(pass) ColorStep does not create a final agent when required wizard data is missing [35.00ms]
(pass) MemoryStep falls back to an empty system prompt when wizard data is incomplete [30.00ms]
(pass) ConfirmStep renders a fallback instead of a blank screen for incomplete wizard data [6.00ms]
##[endgroup]
1 tests failed:
5096 pass
1 fail
1 error
13123 expect() calls
Ran 5097 tests across 488 files. [41.52s]
error: script "test:full" exited with code 1
error: script "check" exited with code 1
�[?1006l�[?1003l�[?1002l�[?1000l�[>4m�[<u�[?1004l�[?2004l�[?25h�]9;4;0;��[?1006l�[?1003l�[?1002l�[?1000l�[>4m�[<u�[?1004l�[?2004l�[?25h�]9;4;0;��[?1006l�[?1003l�[?1002l�[?1000l�[>4m�[<u�[?1004l�[?2004l�[?25h�]9;4;0;��[?1006l�[?1003l�[?1002l�[?1000l�[>4m�[<u�[?1004l�[?2004l�[?25h�]9;4;0;�
##[error]Process completed with exit code 1.
GitHub Actions: PR Checks / 0_typecheck.txt: fix(opencode-go): surface clear error on subscription quota exhaustion
Conclusion: failure
##[group]Run bun run typecheck
�[36;1mbun run typecheck�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
$ tsc --noEmit
src/services/api/openaiShim.test.ts(8599,1): error TS1005: '}' expected.
##[error]Process completed with exit code 2.
GitHub Actions: PR Checks / 1_smoke-and-tests.txt: fix(opencode-go): surface clear error on subscription quota exhaustion
Conclusion: failure
##[group]src/components/agents/new-agent-creation/wizard-steps/wizardSteps.test.tsx:
(pass) ColorStep does not create a final agent when required wizard data is missing [35.00ms]
(pass) MemoryStep falls back to an empty system prompt when wizard data is incomplete [30.00ms]
(pass) ConfirmStep renders a fallback instead of a blank screen for incomplete wizard data [6.00ms]
##[endgroup]
1 tests failed:
5096 pass
1 fail
1 error
13123 expect() calls
Ran 5097 tests across 488 files. [41.52s]
error: script "test:full" exited with code 1
error: script "check" exited with code 1
�[?1006l�[?1003l�[?1002l�[?1000l�[>4m�[<u�[?1004l�[?2004l�[?25h�]9;4;0;��[?1006l�[?1003l�[?1002l�[?1000l�[>4m�[<u�[?1004l�[?2004l�[?25h�]9;4;0;��[?1006l�[?1003l�[?1002l�[?1000l�[>4m�[<u�[?1004l�[?2004l�[?25h�]9;4;0;��[?1006l�[?1003l�[?1002l�[?1000l�[>4m�[<u�[?1004l�[?2004l�[?25h�]9;4;0;�
##[error]Process completed with exit code 1.
🧰 Additional context used
📓 Path-based instructions (13)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.tssrc/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/withRetry.test.tssrc/services/api/errors.tssrc/services/api/openaiShim.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.tssrc/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/withRetry.test.tssrc/services/api/errors.tssrc/services/api/openaiShim.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.tssrc/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/withRetry.test.tssrc/services/api/errors.tssrc/services/api/openaiShim.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.tssrc/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/withRetry.test.tssrc/services/api/errors.tssrc/services/api/openaiShim.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.tssrc/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/withRetry.test.tssrc/services/api/errors.tssrc/services/api/openaiShim.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/withRetry.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/withRetry.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.tssrc/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/withRetry.test.tssrc/services/api/errors.tssrc/services/api/openaiShim.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.tssrc/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/withRetry.test.tssrc/services/api/errors.tssrc/services/api/openaiShim.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.tssrc/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/withRetry.test.tssrc/services/api/errors.tssrc/services/api/openaiShim.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.tssrc/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/withRetry.test.tssrc/services/api/errors.tssrc/services/api/openaiShim.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.tssrc/services/api/withRetry.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/withRetry.test.tssrc/services/api/errors.tssrc/services/api/openaiShim.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.tssrc/services/api/errors.opencodeGo.test.tssrc/services/api/withRetry.test.ts
🪛 GitHub Actions: PR Checks / 0_typecheck.txt
src/services/api/openaiShim.test.ts
[error] 8599-8599: TypeScript compilation error TS1005: '}' expected.
🪛 GitHub Actions: PR Checks / typecheck
src/services/api/openaiShim.test.ts
[error] 8599-8599: TypeScript compilation error: TS1005: '}' expected.
🔇 Additional comments (7)
src/services/api/openaiShim.ts (3)
1636-1766: LGTM!
4040-4043: LGTM!
1870-1923: 🩺 Stability & AvailabilityNo issue: JSON fallback still emits
message_startfirst. The fallback path starts aftermessage_start, so re-emittingcontent_block_*events here preserves the stream shape.> Likely an incorrect or invalid review comment.src/services/api/openaiShim.test.ts (2)
8356-8380: LGTM!
8382-8598: LGTM!src/services/api/withRetry.ts (1)
48-48: LGTM!Also applies to: 123-140, 839-845, 1067-1068
src/services/api/withRetry.test.ts (1)
76-97: LGTM!
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Findings
-
[P1] Fix the failing typecheck and smoke checks before this is ready
src/services/api/openaiShim.test.ts:8599
The current head does not parse:openaiShim.test.tsends after the final JSON fallback test without closing the surrounding block, so both GitHub'stypecheckcheck and a localbun run typecheckfail withTS1005: '}' expected. The focused test run also stops on the same unexpected EOF after the other changed API tests pass. Please fix the syntax error and rerun the affected checks before addressing the behavioral review items below. -
[P2] Complete CodeRabbit's request to preserve the OpenCode Go message on shim errors
src/services/api/errors.ts:776
The OpenCode Go-specific branch is still unreachable for shim-generated quota errors that carry the new OpenAI compatibility marker:getAssistantMessageFromError()maps any[openai_category=quota_exhausted,...]message through the generic quota handler before it ever callsparseOpenCodeGoLimitError(). The shim error path builds exactly that marker from the request URL, but passes through provider response headers rather than the syntheticx-opencode-request-urlheader that the parser expects, so OpenCode Go users can still get the generic "Provider quota or usage allotment has run out" guidance instead of the PR'sOpenCode Go free usage exhausted/ subscription-limit message. Please complete CodeRabbit's still-valid request by making the shim path and assistant-message ordering use one consistent source for the OpenCode Go request URL and by testing that a real shim-shapedFreeUsageLimitError/GoUsageLimitErrorpreserves the OpenCode Go message.
|
Fixed P1: added missing to close the GitHub Copilot test before the JSON fallback regression block. and both pass (176 tests). P2 (OpenCode Go message preservation in shim errors) still needs behavioral work — flagged for author. |
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Findings
[P2] Complete CodeRabbit's request to preserve the OpenCode Go message on shim errors
src/services/api/errors.ts:776
The OpenCode Go-specific branch is still bypassed for shim-generated quota errors. openaiStreamToAnthropic() throws an APIError with the [openai_category=quota_exhausted,...] marker, and getAssistantMessageFromError() maps that generic marker before it calls parseOpenCodeGoLimitError(). The thrown shim error also uses the provider response headers rather than a synthetic x-opencode-request-url header, and the message uses parsed.error.message when present, so the OpenCode Go parser may not see either the request URL or the original FreeUsageLimitError / GoUsageLimitError type. OpenCode Go users can still receive the generic provider-quota guidance instead of the PR's actionable OpenCode Go free/subscription message. Please complete CodeRabbit's still-valid request by wiring the shim request URL and original error type through one consistent source, moving the OpenCode Go detection ahead of the generic marker mapping, and adding a shim-shaped regression test.
[P2] Preserve OpenAI-compatible quota APIErrors through the retry wrapper
src/services/api/withRetry.ts:308
CodeRabbit's quota-preservation request is still valid for non-OpenCode OpenAI-compatible quota errors. isQuotaExhausted() matches [openai_category=quota_exhausted], but the retry loop then wraps that APIError in a new generic Error, so the later getAssistantMessageFromError() path cannot use the new quota_exhausted category mapping added in this PR. That means the new provider-specific compatibility guidance can be replaced by the older generic API quota exhausted or not enabled message. Please preserve the original APIError for OpenAI-compatible quota_exhausted markers the same way the OpenCode Go branch preserves its original error.
[P2] Tighten quota classification so generic billing 4xx responses are not terminal
src/services/api/openaiErrorClassification.ts:204
The new isQuotaExhaustedMessage() treats the standalone word billing as quota exhaustion, and classifyOpenAIHttpFailure() applies that branch to 400/403/429 responses before the existing invalid-request/auth handling. A provider response such as a malformed billing header/field error can now become non-retryable quota_exhausted guidance even though the user needs to fix the request/provider setup, not enable billing. Please narrow this to quota-specific phrases such as billing limit, quota, credits, payment required, or usage limit, and add a negative test for a non-quota billing-related 4xx body.
[P3] Clean up diff-check failures in the changed tests
src/services/api/errors.opencodeGo.test.ts:234
git diff --check origin/main...HEAD reports trailing whitespace in errors.opencodeGo.test.ts and an extra blank line at EOF in openaiErrorClassification.test.ts. Please remove those whitespace issues so the patch stays clean and does not trip diff hygiene checks.
…nt too Addresses CodeRabbit's follow-up on Twigpine#1749: the empty-tool_calls regression only exercised the string-content path, but the hasStructuredToolCalls fix also gates the array-content branch. Add a companion JSON-fallback test with array-form message.content and tool_calls: [] so the array branch can't regress silently. Mutation-checked: reverting hasStructuredToolCalls to a truthiness check fails it.
2bfe3ef to
d4e75e2
Compare
The opencode.ai/zen/go gateway returns 429 with FreeUsageLimitError or GoUsageLimitError in the body when a user's Go subscription quota runs out. Previously these fell through to the generic "Request rejected (429)" path, causing a mysterious stop with no actionable hint. Detect the opencode-go-specific error markers, surface a clear message with the upgrade URL (free tier) or reset duration + workspace + limit name (paid tier), and skip retry — the quota is terminal until reset. Mirrors the canonical implementation in anomalyco/opencode packages/opencode/src/session/retry.ts.
…verter in JSON fallback Addresses the two remaining P2 review items on Twigpine#1749. withRetry: the early isQuotaExhausted guard wrapped OpenCode Go FreeUsageLimitError/GoUsageLimitError 429s in the generic "API quota exhausted or not enabled" message, clobbering the actionable subscribe/ reset guidance. Skip the generic guard for OpenCode Go quota errors so they fall through to the standard shouldRetry=false terminal path, which rethrows the original APIError and lets getAssistantMessageFromError surface the specific message. Consolidate detection in a shared isOpenCodeGoQuotaError predicate (errors.ts) and drop the duplicated inline header check in shouldRetry. openaiShim: the application/json fallback in openaiStreamToAnthropic hand-rolled a thin converter that dropped tool_calls, forwarded raw OpenAI finish_reason values as Anthropic stop reasons, skipped array-content normalization, bypassed <think> stripping, and lost raw text tool-call recovery. Extract the established non-streaming conversion into a shared convertNonStreamingResponseToAnthropicMessage and route the fallback through it, re-emitting the result as stream events. _convertNonStreamingResponse now delegates to the same function. Adds regression coverage: a withRetry test proving the OpenCode Go message survives the retry loop, and JSON-fallback tests for tool_calls, stop-reason mapping, <think> stripping, array content, and raw text tool-call recovery.
…calls as absent Addresses CodeRabbit's review on Twigpine#1749. - [Major] withRetry.ts: throw CannotRetryError for isOpenCodeGoQuotaError BEFORE the fast-mode 429 fallback. Previously the guard only *skipped* the generic quota throw, so an OpenCode Go 429 while fast mode was active hit the fast-mode retry/cooldown path instead of surfacing the quota message immediately. Wrapping the original APIError still preserves the OpenCode Go assistant message via getAssistantMessageFromError. Adds a fast-mode regression test (mutation-checked) and a forceFastMode option on the test helper. - [Minor] openaiShim.ts: an empty tool_calls array is truthy, which skipped the raw "Tool calls requested" recovery in convertNonStreamingResponseToAnthropicMessage. Gate on a single hasStructuredToolCalls (length > 0) check across both the string- and array-content raw-recovery paths and the structured loop. Adds a JSON-fallback regression test for tool_calls: [] (mutation-checked). - [Minor] openaiShim.test.ts: collectFallbackEvents now saves and restores globalThis.fetch in a finally block so the stub can't leak past the helper.
…nt too Addresses CodeRabbit's follow-up on Twigpine#1749: the empty-tool_calls regression only exercised the string-content path, but the hasStructuredToolCalls fix also gates the array-content branch. Add a companion JSON-fallback test with array-form message.content and tool_calls: [] so the array branch can't regress silently. Mutation-checked: reverting hasStructuredToolCalls to a truthiness check fails it.
d4e75e2 to
369af89
Compare
Stale after follow-up commits: current head resolves the requested changes, review threads are resolved, and current checks are passing.
3 participants
Summary
The opencode.ai/zen/go gateway returns
429withGoUsageLimitErrorin the response body when a user's Go subscription quota runs out. Previously these fell through to the genericRequest rejected (429)path, causing the CLI to "mysteriously stop working" with no actionable hint for the user.This PR adds detection for the opencode-go-specific error markers and surfaces a clear, actionable message. It also fixes an endless "thinking..." hang by intercepting JSON error payloads returned on streaming paths and throwing them cleanly instead of silently ending the stream.
What an opencode-go user now sees
OpenCode Go subscription limit hit:
Changes
src/services/api/openaiShim.tsapplication/jsonresponses at the beginning ofopenaiStreamToAnthropicstreaming reader loop. If it's a JSON error, it parses it and throws it cleanly, preventing endless thinking spinner hangs when the stream ends empty.src/services/api/errors.tsparseOpenCodeGoLimitError()— detectsGoUsageLimitErrorin the error body, scoped to the opencode-go gateway via thex-opencode-request-urlheader (withOPENAI_BASE_URLenv fallback for direct-mode usage). ExtractslimitName,workspace, andretry-afterseconds.formatResetDuration()— formats seconds as2d,2d 3h,2h 6m, etc.OPENCODE_GO_USAGE_LIMIT_ERROR_MESSAGEexported constant.getAssistantMessageFromError()— new branch that callsparseOpenCodeGoLimitError()before the generic 429 handler, returns the clear message with reset time, workspace, and limit name.classifyAPIError()— newopencode_go_quota_exhaustedanalytics bucket (distinct from genericrate_limit).src/services/api/openaiErrorClassification.tsquota_exhaustedcategory mapping for402status codes and client errors with billing/credit/quota keywords.src/services/api/withRetry.tsshouldRetry()— treats opencode-go quota errors as terminal. Retrying burns the same 429 and hides the actionable message behind repeated retries.isQuotaExhausted()— updated to abort retries for402and quota-related errors.src/services/api/errors.opencodeGo.test.ts(new)src/services/api/openaiErrorClassification.test.tsandsrc/services/api/withRetry.test.tsquota_exhaustedclassification and retry loop termination.Why this approach
anomalyco/opencodepackages/opencode/src/session/retry.ts.opencode.ai/zen/goin the request URL orOPENAI_BASE_URL, so it cannot misfire on other OpenAI-compatible providers.Test plan
bun test src/services/api/errors.opencodeGo.test.ts— passbun test src/services/api/openaiErrorClassification.test.ts src/services/api/withRetry.test.ts— passbun run typecheck— cleanbun run smoke(will run in CI)bun run check(will run in CI)Summary by CodeRabbit
Release Notes
New Features
quota_exhaustedclassification for OpenAI-compatible HTTP errors (402/403/429) as non-retryable guidance.application/jsonhandling by converting it into consistent Anthropic-style streaming events.Bug Fixes
Tests
<think>stripping).