feat(providers): generalize accounts UI and per-provider fallback - #36
Conversation
|
✅ Target branch corrected This pull request now targets The |
github-actions
Bot
changed the title
📝 WalkthroughWalkthroughThe PR removes Codex Auth from the GUI and redirects legacy hashes to Providers. It adds ordered provider/model fallback configuration, validation, management API support, retry routing, logging, localization, and end-to-end coverage. ChangesProvider fallback and navigation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ProvidersPage
participant ProviderSettings
participant ProviderRoutes
participant ResponsesCore
participant UpstreamProvider
Client->>ProvidersPage: open provider settings
ProvidersPage->>ProviderSettings: provide peer provider metadata
ProviderSettings->>ProviderRoutes: PATCH fallback targets
ProviderRoutes-->>ProviderSettings: normalized provider configuration
Client->>ResponsesCore: send plain model request
ResponsesCore->>UpstreamProvider: attempt configured route
UpstreamProvider-->>ResponsesCore: retryable failure
ResponsesCore->>UpstreamProvider: attempt fallback route
UpstreamProvider-->>ResponsesCore: fallback response
ResponsesCore-->>Client: response with preserved model identity
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // below, which the combo path deliberately skips. | ||
| if (!options.comboAttempt && !isThreadSpawnRequest(req.headers)) { | ||
| const plan = providerFallbackPlan(config, { provider: route.providerName, modelId: route.modelId }); | ||
| if (plan) { |
There was a problem hiding this comment.
A request to an Anthropic provider with a usable fallback is delegated to handleComboResponses, which invokes the successful primary hop with comboAttempt: true. The Anthropic terminal guard requires !options.comboAttempt, so the primary response cannot make its normal bounded continuation even when no fallback is needed. This produces a completed but incomplete response when the first response announces a plan and needs the continuation to emit its tool call.
Context Used: Focking gretig zijn en niet stoppen tot perfectie.... (source)
Artifacts
Focused reproduction source for direct and fallback Anthropic primary requests
- This executable Bun script sends matched successful Anthropic primary requests with and without a usable fallback and checks whether the terminal continuation forwards the tool call, demonstrating the fallback-specific bypass.
Observed fallback reproduction output
- This captured command output shows the direct case made two upstream calls and forwarded the tool call, while the usable-fallback case made one call and omitted it, proving the incomplete successful response.
Existing direct Anthropic terminal guard test output
- This captured test output shows the existing direct Anthropic terminal-guard integration test passed, establishing the control behavior that the fallback path loses.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/server/responses/core.ts
Line: 955
Comment:
**Fallback disables Anthropic terminal continuation**
A request to an Anthropic provider with a usable fallback is delegated to `handleComboResponses`, which invokes the successful primary hop with `comboAttempt: true`. The Anthropic terminal guard requires `!options.comboAttempt`, so the primary response cannot make its normal bounded continuation even when no fallback is needed. This produces a completed but incomplete response when the first response announces a plan and needs the continuation to emit its tool call.
**Context Used:** Focking gretig zijn en niet stoppen tot perfectie.... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/management/provider-routes.ts (1)
373-395: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd a dependent-fallback check to DELETE /api/providers.
This handler already blocks deleting a provider referenced by a combo target (
dependentCombos, lines 377-386), but it does not check whether any OTHER provider's newfallbackarray (introduced by this PR) references the provider being deleted.Failure mode: with provider
aconfigured asfallback: [{ provider: "b", model: "m2" }],DELETE /api/providers?name=bsucceeds today with no 409.providerFallbackPlan'susable()check in src/providers/fallback.ts (lines 145-148) will silently skip the now-missing target at runtime, so a live request does not crash — but providera's config now carries a permanently dangling reference thatproviderFallbackIssueswould reject asfallback[0].provider "b" is not configuredon the next full-schema validation pass (src/config.ts lines 564-570), for example on process restart vialoadConfig(). That is exactly the kind of validation failure thestreamModecomment in src/config.ts warns can trip a "backup-and-defaults repair path" that wipes providers/pool accounts.Mirror the existing
dependentCombospattern: scanconfig.providersfor any OTHER provider whosefallbackarray referencesname, and return 409 with the offending provider names if any are found.🛠️ Proposed fix: block deletion when another provider's fallback references this one
const dependentCombos = Object.entries(config.combos ?? {}) .filter(([, combo]) => combo.targets.some(target => target.provider === name)) .map(([id]) => id) .sort((a, b) => a.localeCompare(b)); if (dependentCombos.length > 0) { return jsonResponse({ error: `cannot delete provider "${name}" while combos depend on it`, combos: dependentCombos, }, 409); } + const dependentFallbackProviders = Object.entries(config.providers) + .filter(([providerName, p]) => providerName !== name + && Array.isArray(p.fallback) + && p.fallback.some(target => target?.provider === name)) + .map(([providerName]) => providerName) + .sort((a, b) => a.localeCompare(b)); + if (dependentFallbackProviders.length > 0) { + return jsonResponse({ + error: `cannot delete provider "${name}" while other providers list it as a fallback target`, + providers: dependentFallbackProviders, + }, 409); + } const { saveConfigPreservingClaudeCode: save } = await import("../../config"); delete config.providers[name];Run this to confirm whether
providerManagementConfigError(used by the POST handler) already validates a new provider'sfallbackfield against currently configured providers, in case POST has the same class of gap:#!/bin/bash #!/bin/bash ast-grep run --pattern 'function providerManagementConfigError($$$) { $$$ }' --lang typescript src rg -n -A 20 'function providerManagementConfigError' src🤖 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/management/provider-routes.ts` around lines 373 - 395, Update the DELETE /api/providers handler around dependentCombos to also scan other providers’ fallback arrays for references to the provider being deleted, excluding the provider itself. Collect and deterministically sort the offending provider names, and return a 409 response identifying them before mutating config; preserve the existing combo dependency and deletion behavior.
🤖 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 `@gui/src/app-routing.ts`:
- Around line 84-89: Update the app route initialization and hash-change
handling in use-app-route-state.ts to pass hashes through resolveAppHashChange,
ensuring legacy codex-auth and codex-auth/... hashes resolve to the providers
page instead of readPageFromHash’s dashboard fallback. Preserve replacement of
the legacy hash with the providers hash for both initial loads and subsequent
hash changes.
In `@gui/tests/provider-settings-fallback.test.tsx`:
- Around line 34-52: Add an interaction-focused test alongside “ProviderSettings
renders configured fallback targets” that edits a fallback provider or model,
submits the form, and asserts the emitted onUpdateProvider patch contains the
exact ordered fallback array. Add a second assertion covering an incomplete
fallback row and verify submission does not emit an update.
In `@src/providers/fallback.ts`:
- Around line 33-47: Bound retained state for synthetic fallback combo IDs
created by syntheticComboId and providerFallbackPlan, grouping entries by
physical provider rather than model ID. Add eviction or a per-provider cap for
these IDs and apply the same policy to both targetCooldowns used by
coolComboTarget/isComboTargetInCooldown and selectionState used by
pickComboTarget. Preserve existing behavior for explicit user combo IDs and
ensure evicted synthetic IDs are removed from both state maps.
In `@src/server/responses/core.ts`:
- Around line 950-959: Clear previous_response_id after
expandPreviousResponseInput completes, or immediately before
concreteComboRequestBody creates each fallback child body, so recursive
handleResponses calls cannot expand the same history twice. Preserve normal
previous-response behavior for the initial request, and add a regression case in
tests/server-combo-failover-e2e.test.ts covering a chained plain provider
fallback and asserting the child request contains no duplicated restored input.
---
Outside diff comments:
In `@src/server/management/provider-routes.ts`:
- Around line 373-395: Update the DELETE /api/providers handler around
dependentCombos to also scan other providers’ fallback arrays for references to
the provider being deleted, excluding the provider itself. Collect and
deterministically sort the offending provider names, and return a 409 response
identifying them before mutating config; preserve the existing combo dependency
and deletion 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: ceb34580-7da8-4905-8f50-ed00f903983a
📒 Files selected for processing (27)
gui/src/App.tsxgui/src/app-routing.tsgui/src/components/provider-workspace/ProviderDetails.tsxgui/src/components/provider-workspace/ProviderSettings.tsxgui/src/components/provider-workspace/types.tsgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/nl.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/src/pages/Providers.tsxgui/src/pages/providers-shared.tsgui/src/provider-workspace/catalog.tsgui/src/styles/provider-workspace-settings.cssgui/tests/dashboard-tabs.test.tsgui/tests/provider-settings-fallback.test.tsxgui/tests/sidebar-codex-auth.test.tssrc/config.tssrc/providers/fallback.tssrc/server/auth-cors.tssrc/server/management/provider-routes.tssrc/server/responses/core.tssrc/types.tstests/provider-fallback.test.tstests/server-combo-failover-e2e.test.ts
| // Account management used to be a Codex-only destination. Providers now owns | ||
| // OAuth accounts, API-key pools and the OpenAI/Codex pool in one place. | ||
| if (rawHash === "codex-auth" || rawHash.startsWith("codex-auth/")) { | ||
| return { page: "providers", replaceTo: "providers" }; | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 \
'readPageFromHash|resolveAppHashChange|hashchange|location\.hash' \
gui/src --glob '*.ts' --glob '*.tsx'Repository: OnlineChefGroep/opencodex
Length of output: 16661
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '--- gui/src/app-routing.ts ---\n'
cat -n gui/src/app-routing.ts
printf '\n--- gui/src/use-app-route-state.ts ---\n'
cat -n gui/src/use-app-route-state.ts
printf '\n--- gui/src/App.tsx relevant ---\n'
sed -n '1,170p' gui/src/App.tsx | cat -n
printf '\n--- deterministic legacy hash behavior probe ---\n'
python3 - <<'PY'
from pathlib import Path
src = Path('gui/src/app-routing.ts').read_text()
checks = {
'VALID_PAGES_contains_codex_auth': '"codex-auth"' in split_line(src, 'VALID_PAGES'),
'resolveAppHashChange_reads_page_first': 'const nextPage = readPageFromHash(rawHash);' in src and 'if (pageId === ("debug" as Page)) return "logs";' in extract_function(src, 'resolveAppHashChange'),
'resolveAppHashChange_redirects_codex_auth': 'rawHash === "codex-auth" || rawHash.startsWith("codex-auth/")' in extract_function(src, 'resolveAppHashChange'),
'useAppRouteState_initializes_from_readPageFromHash': 'useState<Page>(readPageFromHash)' in Path('gui/src/use-app-route-state.ts').read_text(),
'useAppRouteState_hashchange_uses_applyHashAction': 'applyHashAction(normalizeHashPath(window.location.hash));' in Path('gui/src/use-app-route-state.ts').read_text(),
}
for k, v in checks.items():
print(f'{k}={v}')
def split_line(text, start):
for line in text.splitlines():
if start in line:
return line
return ''
def extract_function(text, name):
i=text.find(f'export function {name}')
j=text.find('\n}\n', i)
if j == -1:
j=text.find('};', i)
return text[i:j+2] if i!=-1 else ''
PYRepository: OnlineChefGroep/opencodex
Length of output: 254
Route codex-auth legacy hashes through resolveAppHashChange.
#codex-auth/accounts initializes or updates to the fallback page "dashboard" because readPageFromHash returns "dashboard" when "codex-auth" is not in VALID_PAGES. Only resolveAppHashChange maps those legacy hashes to "providers". In gui/src/use-app-route-state.ts, the initial useState<Page>(readPageFromHash) can pick up that legacy hash before hashchange; route the initial hash and hashchange updates through resolveAppHashChange so bookmarked codex-auth links open the Providers page without the old hash.
🤖 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 `@gui/src/app-routing.ts` around lines 84 - 89, Update the app route
initialization and hash-change handling in use-app-route-state.ts to pass hashes
through resolveAppHashChange, ensuring legacy codex-auth and codex-auth/...
hashes resolve to the providers page instead of readPageFromHash’s dashboard
fallback. Preserve replacement of the legacy hash with the providers hash for
both initial loads and subsequent hash changes.
| test("ProviderSettings renders configured fallback targets", () => { | ||
| const html = renderToStaticMarkup( | ||
| <LanguageProvider> | ||
| <ProviderSettings | ||
| item={item} | ||
| peerProviders={[ | ||
| { name: "google-antigravity", models: ["gemini-3.6-flash"] }, | ||
| { name: "deepseek", models: ["deepseek-v4-flash"], defaultModel: "deepseek-v4-flash" }, | ||
| { name: "cursor", models: ["claude-sonnet-5"] }, | ||
| ]} | ||
| /> | ||
| </LanguageProvider>, | ||
| ); | ||
|
|
||
| expect(html).toContain("Fallback providers"); | ||
| expect(html).toContain("deepseek"); | ||
| expect(html).toContain("deepseek-v4-flash"); | ||
| expect(html).toContain("Add fallback"); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift
Add an interaction test for fallback persistence.
This test only verifies static markup. It does not invoke onUpdateProvider.
Add a focused UI test that changes a fallback provider or model, saves the form, and asserts the exact ordered fallback array in the emitted patch. Also assert that an incomplete row blocks the update.
🤖 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 `@gui/tests/provider-settings-fallback.test.tsx` around lines 34 - 52, Add an
interaction-focused test alongside “ProviderSettings renders configured fallback
targets” that edits a fallback provider or model, submits the form, and asserts
the emitted onUpdateProvider patch contains the exact ordered fallback array.
Add a second assertion covering an incomplete fallback row and verify submission
does not emit an update.
| function syntheticComboId(provider: string, model: string): string { | ||
| return `provider-fallback\u0000${provider}\u0000${model}`; | ||
| } | ||
|
|
||
| /** True when `id` came from `syntheticComboId` rather than the user's combos map. */ | ||
| export function isProviderFallbackComboId(id: string): boolean { | ||
| return id.startsWith("provider-fallback\u0000"); | ||
| } | ||
|
|
||
| /** Human-readable form of a combo id for logs and error messages (NUL is not printable). */ | ||
| export function comboIdLabel(id: string): string { | ||
| if (!isProviderFallbackComboId(id)) return id; | ||
| const [, provider, model] = id.split("\u0000"); | ||
| return `fallback:${provider}/${model}`; | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Locate the combo cooldown/target-state store to check for eviction/TTL bounding.
set -euo pipefail
fd -e ts . src/combos --exec rg -n -C5 'isComboTargetInCooldown|advanceComboAfterFailure|noteComboSuccess|pickComboTarget' {} \;
rg -n -C5 'new Map|WeakMap|setInterval|evict|ttl' src/combosRepository: OnlineChefGroep/opencodex
Length of output: 3255
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching failover/resolve/types/fallback:"
fd -e ts '^(failover|resolve|types|fallback)\.ts$' src | sort
echo
echo "Relevant source snippets:"
for f in src/combos/failover.ts src/combos/resolve.ts src/combos/types.ts src/providers/fallback.ts; do
echo "--- $f ---"
if [ -f "$f" ]; then
wc -l "$f"
sed -n '1,220p' "$f" | nl -ba
fi
done
echo
echo "Cross-repo usages of comboId creation/path:"
rg -n -C3 "syntheticComboId|providerFallbackPlan|pickComboTarget|tryPickComboModel|comboIdLabel|clearComboSelectionState" src tests || trueRepository: OnlineChefGroep/opencodex
Length of output: 489
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
ls -l src/combos/*.ts src/providers/fallback.ts
echo
echo "src/combos/failover.ts:"
cat -n src/combos/failover.ts | sed -n '1,180p'
echo
echo "src/combos/resolve.ts:"
cat -n src/combos/resolve.ts | sed -n '1,230p'
echo
echo "src/combos/types.ts:"
cat -n src/combos/types.ts | sed -n '1,220p'
echo
echo "src/providers/fallback.ts:"
cat -n src/providers/fallback.ts | sed -n '1,220p'
echo
echo "Code references:"
rg -n -C3 "syntheticComboId|providerFallbackPlan|pickComboTarget|tryPickComboModel|comboIdLabel|clearComboSelectionState|combineProviderFallback|fallback" src tests || trueRepository: OnlineChefGroep/opencodex
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Combos failover:"
sed -n '1,160p' src/combos/failover.ts
echo
echo "Combos resolve:"
sed -n '1,220p' src/combos/resolve.ts
echo
echo "Combos types:"
sed -n '1,160p' src/combos/types.ts
echo
echo "Provider fallback:"
sed -n '1,210p' src/providers/fallback.ts
echo
echo "Focused search for synthetic combo id callers and cooldown helpers:"
rg -n -C4 "syntheticComboId|providerFallbackPlan|providerFallbackTargets|clearComboTargetCooldowns|coolComboTarget|isComboTargetInCooldown|advanceComboAfterFailure|noteComboSuccess|pickComboTarget" src -g '*.ts'
echo
echo "Programmatic static lookup for cooldownMapKey definitions/usages and interval/eviction references in src/combos/src/providers:"
rg -n "cooldownMapKey\\(|cooldownMap|new Map|WeakMap|setInterval|evict|ttl|clearComboTargetCooldowns" src/combos src/providers src/types.tsRepository: OnlineChefGroep/opencodex
Length of output: 46645
Bound cooldown/growth state for synthetic fallback combo ids.
providerFallbackPlan at src/providers/fallback.ts:161 creates a distinct syntheticComboId per (provider, modelId) pair. coolComboTarget() and isComboTargetInCooldown() use src/combos/failover.ts:10 targetCooldowns, and pickComboTarget() uses src/combos/resolve.ts:29 selectionState, both plain process maps. These entries are only removed when the key expires (MAX_COOLDOWN_MS for cooldowns) or when an explicit combo/management path deletes a combo id; plain model fallback ids have no eviction or per-provider cap. If callers request many distinct model ids against a provider with fallback configured, memory grows unboundedly. Add an eviction policy or cap the number of retained synthetic combo ids per physical provider, and apply the same bound to selectionState.
🤖 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/providers/fallback.ts` around lines 33 - 47, Bound retained state for
synthetic fallback combo IDs created by syntheticComboId and
providerFallbackPlan, grouping entries by physical provider rather than model
ID. Add eviction or a per-provider cap for these IDs and apply the same policy
to both targetCooldowns used by coolComboTarget/isComboTargetInCooldown and
selectionState used by pickComboTarget. Preserve existing behavior for explicit
user combo IDs and ensure evicted synthetic IDs are removed from both state
maps.
| // Per-provider fallback replays the request across the provider's configured targets using the | ||
| // combo hop loop. Thread spawns are excluded: they carry their own Codex account/model fallback | ||
| // below, which the combo path deliberately skips. | ||
| if (!options.comboAttempt && !isThreadSpawnRequest(req.headers)) { | ||
| const plan = providerFallbackPlan(config, { provider: route.providerName, modelId: route.modelId }); | ||
| if (plan) { | ||
| return handleComboResponses(req, body, plan.comboId, plan.config, logCtx, options); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect expandPreviousResponseInput and concreteComboRequestBody for idempotency
# on an already-expanded body / whether previous_response_id survives into the combo child body.
set -euo pipefail
rg -n -B2 -A 25 'function expandPreviousResponseInput' src
rg -n -B2 -A 30 'function concreteComboRequestBody' src
rg -n 'previous_response_id' src/server/responses src/combosRepository: OnlineChefGroep/opencodex
Length of output: 4778
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- locate core combo helpers and request module ---\n'
rg -n 'handleComboResponses|expandPreviousResponseInput|sanitizeEncryptedContentInPlace|previous_response_id|concreteComboRequestBody' src/server/responses/core.ts src/combos/request.ts src/responses/state.ts
printf '\n--- core.ts relevant sections ---\n'
sed -n '840,975p' src/server/responses/core.ts
sed -n '1130,1205p' src/server/responses/core.ts
sed -n '2035,2070p' src/server/responses/core.ts
printf '\n--- combos request.ts ---\n'
cat -n src/combos/request.ts
printf '\n--- state.ts relevant section ---\n'
sed -n '300,340p' src/responses/state.tsRepository: OnlineChefGroep/opencodex
Length of output: 254
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- locate core combo helpers and request module ---'
rg -n 'handleComboResponses|expandPreviousResponseInput|sanitizeEncryptedContentInPlace|previous_response_id|concreteComboRequestBody' src/server/responses/core.ts src/combos/request.ts src/responses/state.ts || true
echo
echo '--- core.ts relevant sections ---'
sed -n '840,975p' src/server/responses/core.ts
sed -n '1130,1205p' src/server/responses/core.ts
sed -n '2035,2070p' src/server/responses/core.ts || true
echo
echo '--- combos request.ts ---'
cat -n src/combos/request.ts
echo
echo '--- state.ts relevant section ---'
sed -n '300,340p' src/responses/state.tsRepository: OnlineChefGroep/opencodex
Length of output: 19024
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- core.ts handleComboResponses section ---'
sed -n '620,730p' src/server/responses/core.ts
echo
echo '--- tests file presence ---'
git ls-files | rg 'server-combo-failover-e2e\.test\.ts$' || true
echo
echo '--- deterministic state shape probe from source ---'
python3 - <<'PY'
import re
from pathlib import Path
src = Path('src/responses/state.ts').read_text()
m = re.search(r'export function expandPreviousResponseInput\(body: unknown\): unknown \{(?P<body>.*?)\n\}', src, re.S)
print(m.group(0) if m else 'expandPreviousResponseInput not found')
print('reassigns input:', 'input: [...previous.items, ...inputItems(request.input)]' in src)
print('clears previous_response_id:', 'delete request.previous_response_id' in src or 'delete \\.previous_response_id' in src)
PYRepository: OnlineChefGroep/opencodex
Length of output: 5441
Clear previous_response_id before replaying fallback combo child requests.
expandPreviousResponseInput expands input but leaves previous_response_id on the request; it also reuses the cached expanded request shape via replayedInputPrefixLengths. Plain requests that fall back through providerFallbackPlan call handleComboResponses with that expanded body, then concreteComboRequestBody clones it as childBody; the recursive handleResponses call can expand the same previous_response_id again and duplicate the restored items in the child request sent to the fallback target. This is less likely to persist through an already-expanded body, but plain provider fallback now enters this route for the first time, so clear the field after the first expansion or before concreteComboRequestBody builds the child request, and add a regression test in tests/server-combo-failover-e2e.test.ts that sends a previous_response_id-chained plain request against a fallback provider and asserts duplicated input is not sent to the child.
🤖 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/responses/core.ts` around lines 950 - 959, Clear
previous_response_id after expandPreviousResponseInput completes, or immediately
before concreteComboRequestBody creates each fallback child body, so recursive
handleResponses calls cannot expand the same history twice. Preserve normal
previous-response behavior for the initial request, and add a regression case in
tests/server-combo-failover-e2e.test.ts covering a chained plain provider
fallback and asserting the child request contains no duplicated restored input.
Collapse Codex-only auth nav into Providers, add configurable per-provider fallback hops, and keep runtime failover on the existing combo engine so sofie patches stay reproducible. Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
a20daf9 to
c23de7a
Compare
github-actions
Bot
changed the title
… fallback hops - keep the Anthropic terminal-continuation guard active on the primary hop of a per-provider fallback chain (synthetic combos are not user combos) - hand the pre-expansion body to the fallback combo so previous_response_id is expanded exactly once - sweep expired combo-target cooldowns so synthetic combo ids cannot accumulate state forever - resolve the initial GUI route from the hash through resolveAppHashChange so a bookmarked #codex-auth lands on Providers - cover the fallback editor interaction (ordered patch, incomplete row blocks save) Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
React Doctor's no-array-index-as-key fired on the fallback editor: rows are user-reorderable and removable, so the index is not a stable identity. Carry a per-row id in form state and drive updates/removal off it. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
1 participant
Summary
Retargeted from
maintodevper the branch policy, and the single feature commit wasreplayed onto
devso this PR carries only the feature diff instead of draggingmain'slineage into the integration branch. Cross-lineage resolutions: the provider list response keeps
dev'sapiKeyTransport/ client-hide fields plus the newfallbackfield, the per-providerfallback hop in
src/server/responses/core.tsnow sits next todev's subagent quota priming(the two branches are mutually exclusive, thread spawns skip the fallback path), and the
gui/src/i18n/nl.tsedit was dropped becausedevhas no Dutch locale.Test plan
bun run typecheck,bun run test(only pre-existing Docker-sandbox service/CLI failures),bun run privacy:scancd gui && bun test tests && bun run lint && bun run lint:i18n && bun run buildMade with Cursor
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is enabled.Summary by CodeRabbit
Greptile Summary
This change adds provider-level fallback configuration and management controls, routing eligible requests through the existing failover engine. Two reproduced failures remain: Anthropic requests with a configured fallback can return incomplete tool-driven responses even when the primary succeeds, and deleting a referenced provider can leave the saved configuration invalid so the application falls back to defaults on reload.
Confidence Score: 4/5
What T-Rex did
Comments Outside Diff (3)
src/server/management/provider-routes.ts, line 387 (link)Deleting a provider checks dependencies from combos but not from other providers'
fallbacklists. Deleting B after configuring A to fall back to B returns success while retaining A's target for B. On the next configuration load, strict fallback validation rejects the persisted file and the application replaces the active configuration with defaults.Context Used: Focking gretig zijn en niet stoppen tot perfectie.... (source)
Artifacts
Focused API reproduction source after PR #36
Endpoint behavior before PR #36
Endpoint behavior after PR #36 with stale fallback
Prompt To Fix With AI
General comment
src/server/responses/core.ts:955, an Anthropic route with a fallback entershandleComboResponses. That function invokes the primary child withcomboAttempt: trueat line 742. The terminal guard requires!options.comboAttemptat line 1911, so a successful Anthropic primary cannot perform its normal bounded continuation. The reproduced response completed but omitted the requested tool call after the model only announced its plan.comboAttemptrepresents both actual fallback-loop execution and a condition that disables the Anthropic terminal guard; the successful first hop retains that flag even though no fallback was needed.General comment
DELETE /api/providers?name=breturns HTTP 200 OK and persists the removal even when provider A containsfallback: [{ provider: "b", model: "fallback-model" }]. The fallback remains in the file. On a later configuration reload, strict fallback validation rejects the file,readConfigDiagnostics()reportssource=fallback, andloadConfig()backs up the invalid file and replaces the active configuration with defaults.src/server/management/provider-routes.tschecks combo dependencies and removes provider context-cap state, but neither checks provider fallback dependencies nor removes fallback targets pointing at the deleted provider. In contrast,src/config.tsvalidates every fallback target against the current provider map.readConfigDiagnostics().source === "file"after reload.Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat(providers): generalize account UI a..." | Re-trigger Greptile
Context used: