Fix Docker provider fallback and auth error handling#168
Open
friuns2 wants to merge 16 commits into
Open
Conversation
Owner
Author
|
/review |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements OpenCode Zen runtime fallback for unauthenticated Docker, prioritizes /codex-api/provider-models over Codex model/list with fallback rules, maps first-turn thread materialization errors to in-progress empty turns, surfaces per-turn stream failures into thread turns and UI with a "Send feedback" action, renames free-mode persistence to webui-custom-providers.json, and updates docs/tests. ChangesOpenCode Zen Docker Auth, Provider Models, and Turn Error Handling
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code Review by Qodo
1.
|
Review Summary by Qodo
WalkthroughsDescription• Handle Docker first-turn live-state pending materialization gracefully • Load provider models before Codex model list for accurate selectors • Render failed turn errors as persisted chat messages with feedback • Avoid persisting unauthenticated OpenCode Zen fallback configuration Diagramflowchart LR
A["Docker startup"] --> B["Check auth state"]
B --> C["No auth"]
B --> D["Auth exists"]
C --> E["Runtime Zen fallback<br/>no persist"]
D --> F["Use Codex provider"]
E --> G["Load provider models first"]
F --> G
G --> H["Model selector ready"]
I["First turn starts"] --> J["Live-state poll"]
J --> K["Materialization pending?"]
K -->|Yes| L["Return empty state<br/>no error"]
K -->|No| M["Return turn data"]
N["Turn fails"] --> O["Merge stream errors<br/>into thread result"]
O --> P["Render error in chat<br/>with feedback button"]
File Changes1. src/api/codexGateway.ts
|
Code Review by QodoNew Review StartedThis review has been superseded by a new analysis |
Owner
Author
|
/review |
Code Review by Qodo
1. Overlay hides new errors
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/composables/useDesktopState.ts`:
- Around line 1524-1526: The current hasPersistedTurnError check hides live
overlay errors if any persisted 'turnError' exists in the thread; change it to
only suppress when a persisted turnError matches the current live error. Update
the logic that computes hasPersistedTurnError (which uses
persistedMessagesByThreadId and is used when computing errorText with
turnErrorByThreadId, isInProgress, and threadId) so the .some(...) predicate
also compares the persisted message content to
turnErrorByThreadId.value[threadId]?.message (or the appropriate identifier on
the current turn error) before treating it as suppression.
In `@src/server/codexAppServerBridge.ts`:
- Around line 3352-3358: The routes that currently read the FREE_MODE_STATE_FILE
directly should use the same resolver so runtime and HTTP proxy paths see the
same fallback: replace direct calls to readFreeModeStateSync inside the handlers
for the /codex-api/zen-proxy/v1/responses and
/codex-api/custom-proxy/v1/responses endpoints with
ensureDefaultFreeModeStateForMissingAuthSync(statePath) (or call that helper
from a shared resolver function) and handle a null return consistently; ensure
the handlers accept the in-memory fallback returned by
ensureDefaultFreeModeStateForMissingAuthSync (or short-circuit if null) so the
Docker Zen fallback behaves identically to the app-server runtime path.
In `@tests.md`:
- Line 3376: The sentence in tests.md that currently reads "No config files
outside the project are modified (state stored in
`~/.codex/webui-custom-providers.json`)" is self-contradictory; update it to
clearly state the exception for user-level state by rewording to something like:
"Project config files are not modified; only user-level state is written to
`~/.codex/webui-custom-providers.json`." Locate the sentence mentioning
`~/.codex/webui-custom-providers.json` and replace it with this unambiguous
phrasing so test expectations no longer conflict.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 48dd27b2-953c-460f-90b3-c52b826e9521
📒 Files selected for processing (18)
llm-wiki/raw/fixes/opencode-zen-docker-auth-provider-models.mdllm-wiki/wiki/concepts/opencode-zen-big-pickle.mdllm-wiki/wiki/entities/codex-web-local.mdllm-wiki/wiki/index.mdllm-wiki/wiki/log.mdllm-wiki/wiki/overview.mdsrc/api/codexGateway.test.tssrc/api/codexGateway.tssrc/api/normalizers/v2.test.tssrc/api/normalizers/v2.tssrc/components/content/ThreadConversation.vuesrc/composables/useDesktopState.tssrc/server/codexAppServerBridge.archive.test.tssrc/server/codexAppServerBridge.tssrc/server/freeMode.test.tssrc/server/freeMode.tssrc/style.csstests.md
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Owner
Author
|
/review |
|
Persistent review updated to latest commit 8d63a4d |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/server/codexAppServerBridge.ts (1)
3352-3358:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRoute handlers still bypass the new in-memory fallback.
ensureDefaultFreeModeStateForMissingAuthSync()now returns the Zen fallback without writingFREE_MODE_STATE_FILE, but/codex-api/zen-proxy/v1/responsesand/codex-api/custom-proxy/v1/responsesstill read that file directly. On a fresh no-auth startup, the app-server and/codex-api/provider-modelswill see Zen fallback while those proxy routes still see “no state”, so the Docker fallback path can break as soon as the request goes through the proxy.Suggested follow-up
- const state = JSON.parse(readFileSync(statePath, 'utf8')) as FreeModeState - bearerToken = state.apiKey ?? '' - wireApi = state.wireApi === 'responses' ? 'responses' : 'chat' + const state = ensureDefaultFreeModeStateForMissingAuthSync(statePath) + bearerToken = state?.apiKey ?? '' + wireApi = state?.wireApi === 'responses' ? 'responses' : 'chat'Apply the same resolver in the custom-proxy handler as well.
🤖 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/server/codexAppServerBridge.ts` around lines 3352 - 3358, Route handlers for /codex-api/zen-proxy/v1/responses and /codex-api/custom-proxy/v1/responses still call readFreeModeStateSync directly and thus bypass the in-memory fallback provided by ensureDefaultFreeModeStateForMissingAuthSync; update those handlers to call ensureDefaultFreeModeStateForMissingAuthSync(statePath) (or reuse its resolved value) instead of readFreeModeStateSync so they receive the Zen fallback when auth is missing, and mirror the same change in the custom-proxy handler so both proxy routes behave consistently with createDefaultOpenCodeZenFreeModeState().
🤖 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/server/codexAppServerBridge.ts`:
- Around line 3352-3358: Route handlers for /codex-api/zen-proxy/v1/responses
and /codex-api/custom-proxy/v1/responses still call readFreeModeStateSync
directly and thus bypass the in-memory fallback provided by
ensureDefaultFreeModeStateForMissingAuthSync; update those handlers to call
ensureDefaultFreeModeStateForMissingAuthSync(statePath) (or reuse its resolved
value) instead of readFreeModeStateSync so they receive the Zen fallback when
auth is missing, and mirror the same change in the custom-proxy handler so both
proxy routes behave consistently with createDefaultOpenCodeZenFreeModeState().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d0a34474-ae20-424c-9dec-b0755d13c392
📒 Files selected for processing (5)
src/composables/useDesktopState.test.tssrc/composables/useDesktopState.tssrc/server/codexAppServerBridge.tstests.mdwhatToTest.md
✅ Files skipped from review due to trivial changes (2)
- whatToTest.md
- tests.md
Comment on lines
+3
to
+8
| ## [2026-05-13] ingest | OpenCode Zen Docker auth and provider models | ||
| - Added source: `raw/fixes/opencode-zen-docker-auth-provider-models.md`. | ||
| - Updated wiki page: `concepts/opencode-zen-big-pickle.md`. | ||
| - Documents: no-auth Zen runtime fallback, auth-mounted Docker switching back to Codex defaults, first-turn materialization as a transient live-state condition, provider-model-first loading, and the build-time Docker install workaround for runtime `pnpm dlx` OOM risk. | ||
| - Updated `overview.md`, `entities/codex-web-local.md`, and `index.md`. | ||
|
|
There was a problem hiding this comment.
1. Wiki log entry not appended 📘 Rule violation ◔ Observability
The new ingest entry in llm-wiki/wiki/log.md was inserted at the top of the file instead of being appended, breaking the append-only audit trail requirement. This can undermine traceability expectations for wiki ingest operations.
Agent Prompt
## Issue description
`llm-wiki/wiki/log.md` must be append-only, but the new ingest entry was prepended near the top of the file.
## Issue Context
Compliance requires that ingest operations append exactly one new entry to the end of `llm-wiki/wiki/log.md` without rewriting existing history.
## Fix Focus Areas
- llm-wiki/wiki/log.md[3-8]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/server/freeMode.test.ts (1)
37-55: ⚡ Quick winAdd coverage for Zen/user-key and auth-disabled guard branches.
Current assertions only validate
openrouter+customKeycases. Please also cover theLine 212behavior (Zen with non-empty API key should not suppress) and the!hasUsableCodexAuth/!enabledguards so future edits don’t regress fallback selection.Suggested test additions
it('suppresses community fallback providers when Codex auth appears', () => { @@ expect(shouldSuppressCommunityFreeModeForCodexAuth({ enabled: true, apiKey: 'user-key', model: FREE_MODE_DEFAULT_MODEL, customKey: true, provider: 'openrouter', wireApi: 'responses', }, true)).toBe(false) + + expect(shouldSuppressCommunityFreeModeForCodexAuth({ + enabled: true, + apiKey: 'zen-user-key', + model: OPENCODE_ZEN_DEFAULT_MODEL, + customKey: false, + provider: 'opencode-zen', + wireApi: 'responses', + }, true)).toBe(false) + + expect(shouldSuppressCommunityFreeModeForCodexAuth({ + enabled: false, + apiKey: null, + model: OPENCODE_ZEN_DEFAULT_MODEL, + provider: 'opencode-zen', + wireApi: 'responses', + }, true)).toBe(false) + + expect(shouldSuppressCommunityFreeModeForCodexAuth({ + enabled: true, + apiKey: 'community-key', + model: FREE_MODE_DEFAULT_MODEL, + customKey: false, + provider: 'openrouter', + wireApi: 'responses', + }, false)).toBe(false) })🤖 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/server/freeMode.test.ts` around lines 37 - 55, Add tests to cover the missing branches in shouldSuppressCommunityFreeModeForCodexAuth: assert that when provider is 'zen' with a non-empty apiKey (e.g., apiKey: 'user-key', customKey: true/false as appropriate) the function returns false (Line 212 behavior), add a case where hasUsableCodexAuth is false to ensure it returns false, and add a case where enabled is false to ensure the guard prevents suppression; use the same FREE_MODE_DEFAULT_MODEL in each test to mirror existing cases and reference the function name shouldSuppressCommunityFreeModeForCodexAuth and constant FREE_MODE_DEFAULT_MODEL to locate where to add these assertions.llm-wiki/wiki/log.md (1)
4-13: ⚡ Quick winUse relative markdown links for newly added log references.
These new entries add file references as inline code paths, but they should be markdown links for in-wiki navigation consistency.
Proposed patch
- - Added source: `raw/fixes/copied-auth-provider-promotion.md`. - - Updated wiki page: `concepts/opencode-zen-big-pickle.md`. + - Added source: [raw/fixes/copied-auth-provider-promotion.md](../raw/fixes/copied-auth-provider-promotion.md). + - Updated wiki page: [concepts/opencode-zen-big-pickle.md](./concepts/opencode-zen-big-pickle.md). - Documents: suppressing community fallback provider state after valid Codex auth appears, preserving user-configured providers, importing copied auth into Accounts, provider-scoped Codex model persistence, stale feedback-row cleanup, and packaged Docker validation. - - Updated `index.md`. + - Updated [index.md](./index.md). - - Added source: `raw/fixes/opencode-zen-docker-auth-provider-models.md`. - - Updated wiki page: `concepts/opencode-zen-big-pickle.md`. + - Added source: [raw/fixes/opencode-zen-docker-auth-provider-models.md](../raw/fixes/opencode-zen-docker-auth-provider-models.md). + - Updated wiki page: [concepts/opencode-zen-big-pickle.md](./concepts/opencode-zen-big-pickle.md). - Documents: no-auth Zen runtime fallback, auth-mounted Docker switching back to Codex defaults, first-turn materialization as a transient live-state condition, provider-model-first loading, and the build-time Docker install workaround for runtime `pnpm dlx` OOM risk. - - Updated `overview.md`, `entities/codex-web-local.md`, and `index.md`. + - Updated [overview.md](./overview.md), [entities/codex-web-local.md](./entities/codex-web-local.md), and [index.md](./index.md).As per coding guidelines, "
llm-wiki/wiki/**/*.md: Add wiki links using relative markdown links inllm-wiki/wiki/pages".🤖 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 `@llm-wiki/wiki/log.md` around lines 4 - 13, Replace the inline code-style file references in llm-wiki/wiki/log.md with relative markdown links so they navigate within the wiki; for each occurrence of raw/fixes/copied-auth-provider-promotion.md, concepts/opencode-zen-big-pickle.md, raw/fixes/opencode-zen-docker-auth-provider-models.md, overview.md, entities/codex-web-local.md, and index.md convert the backticked path into a relative link like [filename](./relative/path) pointing into the llm-wiki/wiki/ tree, preserving the displayed text but using Markdown link syntax for consistent in-wiki navigation.
🤖 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/App.vue`:
- Around line 4042-4058: The import path using externalAuthImportAttempted +
refreshAccountsFromAuth can mutate accounts.value without guaranteeing a
refreshAll run after the import completes or before a provider-change-triggered
refresh starts; modify the logic around
externalAuthImportAttempted/refreshAccountsFromAuth/loadAccountsState so you
await the import/fallback (use the Promise from
refreshAccountsFromAuth().catch(...) or loadAccountsState when fallback runs),
detect whether the auth/accounts actually changed (compare previous accounts or
set a flag), and then call refreshAll(...) once after both the awaited
import/fallback and any provider selection change have settled (use
selectedProvider/previousProvider to detect provider change) to ensure
quota/models are refreshed only after account import finishes and to avoid
racing refreshes.
---
Nitpick comments:
In `@llm-wiki/wiki/log.md`:
- Around line 4-13: Replace the inline code-style file references in
llm-wiki/wiki/log.md with relative markdown links so they navigate within the
wiki; for each occurrence of raw/fixes/copied-auth-provider-promotion.md,
concepts/opencode-zen-big-pickle.md,
raw/fixes/opencode-zen-docker-auth-provider-models.md, overview.md,
entities/codex-web-local.md, and index.md convert the backticked path into a
relative link like [filename](./relative/path) pointing into the llm-wiki/wiki/
tree, preserving the displayed text but using Markdown link syntax for
consistent in-wiki navigation.
In `@src/server/freeMode.test.ts`:
- Around line 37-55: Add tests to cover the missing branches in
shouldSuppressCommunityFreeModeForCodexAuth: assert that when provider is 'zen'
with a non-empty apiKey (e.g., apiKey: 'user-key', customKey: true/false as
appropriate) the function returns false (Line 212 behavior), add a case where
hasUsableCodexAuth is false to ensure it returns false, and add a case where
enabled is false to ensure the guard prevents suppression; use the same
FREE_MODE_DEFAULT_MODEL in each test to mirror existing cases and reference the
function name shouldSuppressCommunityFreeModeForCodexAuth and constant
FREE_MODE_DEFAULT_MODEL to locate where to add these assertions.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e59d000b-dea1-48ea-8290-652994064484
📒 Files selected for processing (13)
llm-wiki/raw/fixes/copied-auth-provider-promotion.mdllm-wiki/wiki/concepts/opencode-zen-big-pickle.mdllm-wiki/wiki/index.mdllm-wiki/wiki/log.mdsrc/App.vuesrc/api/codexGateway.tssrc/composables/useDesktopState.test.tssrc/composables/useDesktopState.tssrc/server/codexAppServerBridge.archive.test.tssrc/server/codexAppServerBridge.tssrc/server/freeMode.test.tssrc/server/freeMode.tstests.md
🚧 Files skipped from review as they are similar to previous changes (4)
- src/composables/useDesktopState.test.ts
- tests.md
- src/server/codexAppServerBridge.archive.test.ts
- src/server/codexAppServerBridge.ts
Owner
Author
|
/review |
Code Review by QodoNew Review StartedThis review has been superseded by a new analysis |
Owner
Author
|
/review |
|
Persistent review updated to latest commit d939a17 |
Owner
Author
|
/review |
|
Persistent review updated to latest commit b51f44e |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
Screenshots
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests