Skip to content

fix(providers): map gateway model ids before cloud fallback - #130

Merged
adriannoes merged 4 commits into
developmentfrom
fix/providers-model-mapping
Oct 17, 2026
Merged

adriannoes merged 4 commits into
developmentfrom
fix/providers-model-mapping

Conversation

@adriannoes

Copy link
Copy Markdown
Collaborator

Summary

Consolidates the last cursor high-severity providers fix (originally cursor/high-severity-issues-d140, PR #118) as an operator-authored commit.

  • fix(providers): map gateway model ids before cloud fallback — config.example.toml sets agent.model to a vendor-prefixed id (anthropic/claude-...). Both cloud backends forwarded the raw string: Anthropic rejects the vendor prefix (HTTP 4xx) and OpenAI returns 400 when it receives a Claude id after an Anthropic 5xx, both stopping the fallback chain before local. Model ids are now stripped per backend before the request.

Rebased on top of #124 (merged): the anthropic test file was fused manually so the tool-result grouping regressions and the model-prefix test both run.

Testing

  • make test_anthropic, make test_openai, make test_provider — passed
  • CI=true make test — all 46 test binaries passed
  • make static — passed

Supersedes #118.

config.example.toml sets agent.model to anthropic/claude-sonnet-4-20250514.
Both cloud backends forwarded that string. Anthropic rejects the vendor
prefix with HTTP 4xx, which stops the fallback chain before local.
OpenAI receives the same Claude id after an Anthropic 5xx and returns
HTTP 400, which also stops the chain.

Strip a matching vendor prefix and skip a foreign cloud id with a
non-HTTP error so the router can continue.

Supersedes #118.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Repo context. ShellClaw is a C99 agent for Jetson/Pi with a provider router (fallback_chain: anthropic, openai, local). The router stops on HTTP 4xx and continues on anything else (provider_error_allows_fallback_retry). I could not link or run the tests here (no libcurl dev headers in the review VM), so this review is from reading the diff and the router/config code.

Summary. The bug is real: config.example.toml ships anthropic/claude-..., Anthropic rejects the prefix with a 4xx, and the chain stops before local. The fix is small and correctly built on the existing contract (an error string without HTTP lets the router move on). The resolver is clear, bounds-safe, and has a good table test. I have no merge blockers. The main risks are the heuristic's behavior for custom OpenAI endpoints and the missing router-level test for the claim in the PR title.

Must Fix. None.

Should Fix

  1. src/providers/provider_common.c:111 Custom OpenAI-compatible endpoints regress. [providers.openai] endpoint (or ENV_OPENAI_ENDPOINT) can point at OpenRouter/LiteLLM, where anthropic/claude-... and openai/gpt-4o are valid ids. After this change the first is skipped and the second is stripped to gpt-4o, which OpenRouter does not accept. config.example.toml even has a [providers.openrouter] block. Either only apply mapping when the endpoint is the default api.openai.com URL, or document in the changelog/config comment that vendor prefixes are no longer forwarded to a custom endpoint.
  2. src/providers/provider_common.c:121 Unknown vendor prefixes still pass through unchanged (google/gemini, meta-llama/x, ollama/qwen). Sent to Anthropic, that is still a 4xx that stops the chain, which is the same bug class this PR fixes. Anthropic ids never contain /, so for the anthropic backend any non-matching prefix could be skipped. Also "anthropic/" or "/claude" take the bare path (line 119) and are sent verbatim, which gives a 4xx.
  3. Missing router-level regression test. The PR claims the chain now runs through to local, but nothing in tests/test_router.c exercises anthropic skipped -> openai skipped -> next backend. The unit tests only check the helper and each backend in isolation. Per AGENTS.md (bug fix needs a failing regression test first), a router test with the example model would lock the actual behavior.
  4. tests/test_anthropic.c has no test that Anthropic skips a foreign id (openai/gpt-4o, gpt-4o) with an error that has no HTTP . Only the OpenAI skip path is covered end to end.
  5. src/providers/anthropic.c:427 The SHELLCLAW_TEST hook is inlined into build_and_send, which is now roughly 70 lines (the repo rule is 5 to 40). Consider extracting the model resolution and error formatting into a helper (the same snprintf block now exists in both anthropic.c and openai.c), and keep the test hook out of the request path.
  6. src/providers/provider_common.c:85 The bare-id heuristics are prefix based (o1, o3, o4, gpt-, claude) and case sensitive, so GPT-4o or Claude-... bypass the skip. Matching is also silent: when an id is skipped, nothing logs why beyond the last-error snapshot. A one-time stderr line would help operators.

Nice to Have

  • tests/test_anthropic.c:493 anthropic_test_set_skip_http(0) and the env restore sit after the ASSERTs, so a failing assert leaves skip mode on and the key set for later tests. Reset in a common cleanup path.
  • config.example.toml:5 The comment explains behavior but not that other vendors' ids are skipped on purpose. A one-line example for local-only users would help. The adjacent # api_base = ... comment does not match the real key (endpoint), which is existing debt but easy to fix here.
  • provider_model_id_for_backend returns a pointer into the config-owned string. That is fine today because both callers copy it before the call returns, but say so in the Doxygen.

Positive Highlights

  • Uses the router's existing error classification instead of adding a new signal, and the skip message deliberately avoids HTTP . test_prefixed_claude_model_skips_without_http asserts exactly that.
  • test_model_id_for_backend is a dense table covering prefix strip, pointer identity, NULL/empty, and bare ids. The helper is bounds-safe (vendor_matches_backend only reads backend[vendor_len] after an equal prefix).
  • CHANGELOG entry is present and explains the user-visible effect.
Open in Web View Automation 

Sent by Cursor Automation: Code review

# Conflicts:
#	CHANGELOG.md
@adriannoes
adriannoes merged commit 7df572d into development Oct 17, 2026
1 check passed
@adriannoes
adriannoes deleted the fix/providers-model-mapping branch October 17, 2026 15:51
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.

1 participant