Skip to content

fix(platform): surface actionable error when whole fallback chain is unconfigured (#1455)#1940

Open
larryro wants to merge 1 commit into
mainfrom
tale/xs7ef8bt6yrnehcv26kn657spd8962rd
Open

fix(platform): surface actionable error when whole fallback chain is unconfigured (#1455)#1940
larryro wants to merge 1 commit into
mainfrom
tale/xs7ef8bt6yrnehcv26kn657spd8962rd

Conversation

@larryro

@larryro larryro commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes #1455.

When the agent fallback model chain (e.g. gemini → gemini-flash → mistral-large → … → qwen-next) is exhausted because every model fails to resolve for a configuration reason — most commonly no API key is configured on any provider — the loop previously threw the last entry's per-model MissingApiKeyError. The user saw a confusing message implicating only the tail model (qwen-next) after "multiple retries", with no hint that the real problem is that no provider is configured.

This matches the issue's expected behaviour: "the system should stop early with a meaningful configuration error."

What changed

  • providers/errors.ts
    • Added buildChainExhaustionError(...): when no model ever reached a live provider call (attemptedCount === 0) and every failure was a config/resolution error, it collapses the chain into a single actionable NoProviderAvailableError (which classifyChatErrorCode maps to missing_api_key → the localized "Open Settings → AI providers" hint). An already-terminal NoProviderAvailableError is passed through untouched; a chain where a real attempt happened still surfaces the genuine runtime error.
    • Moved the shared FRIENDLY_NO_PROVIDER copy here so the provider loader and the fallback chain share one source of truth (no string drift).
  • providers/file_actions.ts — imports FRIENDLY_NO_PROVIDER instead of redefining it.
  • lib/agent_chat/internal_actions.ts — the fallback loop now records each resolution (config) failure and routes the chain-exhausted throw through buildChainExhaustionError.

No new user-visible strings (the friendly copy already existed and is rendered via the existing missing_api_key chat-error code), so no i18n/docs changes are required.

Tests

  • Added unit coverage in providers/errors.test.ts for buildChainExhaustionError (all-unconfigured collapse, runtime-error passthrough, terminal passthrough, fallbacks). errors.test.ts: 51 passed.
  • oxlint --type-aware clean on all changed files.

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@larryro, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 2 minutes and 8 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6b59542f-501d-4428-ae47-d4b9bce55876

📥 Commits

Reviewing files that changed from the base of the PR and between b418075 and b7389b6.

📒 Files selected for processing (4)
  • services/platform/convex/lib/agent_chat/internal_actions.ts
  • services/platform/convex/providers/errors.test.ts
  • services/platform/convex/providers/errors.ts
  • services/platform/convex/providers/file_actions.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch tale/xs7ef8bt6yrnehcv26kn657spd8962rd

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.


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

❤️ Share

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

@larryro

larryro commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator Author

Desk Review — #1455 fallback chain unconfigured-error fix

Verdict: READY TO MERGE.

CI

All required checks green (gh pr checks): Type check, Lint, Format, Knip, Unit, Build (all services), 16× Playwright platform shards, Smoke test, CodeRabbit, Opengrep — all pass. No failed or pending checks; only the expected fork-PR / container variants are skipping.

Tests (run locally on this branch)

  • bunx vitest --run --project server convex/providers/errors.test.ts51 passed
  • bunx vitest --run --project server convex/providers convex/lib/agent_chat lib/agent_response lib/shared82 files, 1302 passed

What the change does

When the whole fallback chain is exhausted with no model ever reaching a live provider call (attemptedCount === 0 — every entry failed during in-process resolution because no usable API key is configured), buildChainExhaustionError collapses the chain into a single actionable NoProviderAvailableError('missing_api_key') carrying the FRIENDLY_NO_PROVIDER copy, instead of surfacing the tail model's confusing per-model message. The tail-entry resolution failure now breaks into this unified handler rather than throwing directly.

Verified correct

  • Trigger path is real: MissingApiKeyError is thrown during resolution (resolveModelDataInlineresolveModelApiKey), so attemptedCount stays 0 for an all-unconfigured chain and the collapse fires. Confirmed by reading resolve_model_node.ts / file_actions.ts.
  • No false-positive collapse: attemptedCount (init 0 @ line 347) increments only inside the try after a successful resolution and before the provider call (line 621). Any chain with a configured model that reached a provider returns the genuine runtime error unchanged (attemptedCount > 0 gate).
  • Error-code routing: the FRIENDLY_NO_PROVIDER string matches classifyChatErrorCode's /no api key is configured for this organization/i, mapping to missing_api_key and the actionable UI hint. Notably also improves the single-unconfigured-model case (previously fell through to generic).
  • No cleanup regression: the break lands directly on the buildChainExhaustionError throw with no intervening code; the outer catch (stream error, status clear, failed-message save, NonRetryableError) is error-source-agnostic.
  • NoProviderAvailableError passthrough preserved; dead-scope/runtime-error paths still surface the true cause.
  • No secret leakage: detail list contains model ids and the secretsEnv variable name only — never key values.
  • Elegance: FRIENDLY_NO_PROVIDER now has exactly one definition (moved to errors.ts, re-imported in file_actions.ts); no duplication/dead code; Knip green.

Non-blocking notes (optional)

  1. The loop→buildChainExhaustionError integration (the tail-resolution break) has no end-to-end test; the helper itself is fully unit-covered. An integration test would harden against future loop refactors.
  2. errors.test.ts asserts only the first/last model appear in details; adding mistral-large would fully match the "every failed model is preserved" comment.

These do not affect merge-readiness.

READY TO MERGE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Fallback model chain fails across all providers – no successful response even after multiple retries

1 participant