fix(sdk): rank standard provider keys above custom model lists (#5117)

[Souptik96]

Summary

Fixes bug 1 of #5117: on a slug-less default connection, a custom provider whose models list happens to include a model the standard key also serves resolves instead of that standard key.

The root cause is narrower than "model matches are returned first." A provider_key candidate carries no model list at all — _provider_key_candidate populates neither model_slugs nor model_keys — so matches_model() is structurally always False for a standard key. The moment any custom provider advertises the requested id, _candidate_pool's model_matches list is non-empty and the if model.provider: provider-family branch is never reached. The standard key is not out-ranked; it is excluded from consideration. (On current main this is connections.py:417-431; the issue cites ~364-378, which has since drifted.)

The fix adds _default_candidate_pool, used only by _choose_default, which ranks provider-family matches on standard keys ahead of a custom provider's model-name match. Only the ranking changes — the resulting pool still passes through the existing single-candidate, default-slug and ambiguity rules untouched.

_choose_named deliberately keeps _candidate_pool as-is: when a user names a connection by slug, narrowing that slug's records by model id is the correct behaviour, and there is a test pinning that it still does.

Two judgement calls I'd like a maintainer's read on

1. A behaviour change beyond the strict letter of the issue. When a user has two standard keys for the same provider family and a custom provider advertising the requested model, the ranked pool is now genuinely ambiguous, so resolution raises AmbiguousConnectionError where today it silently resolves to the custom provider. I think failing loud is correct there and consistent with how _choose_default already treats competing defaults — and it is arguably the same class of complaint as bug 2 — but it is a change the issue does not ask for, so it is pinned by its own test (test_default_two_standard_keys_with_a_custom_model_match_is_ambiguous) rather than left implicit. Happy to gate it differently if you'd rather.

2. Deliberate custom-provider routing. A user who wants their custom gateway for a model their standard key also serves now gets the standard key on a slug-less default connection. The remedy is to name the connection by slug, which is unchanged. This follows the issue's own stated preference ("the standard OpenAI key would have worked and is what the user most likely intended"), but it is a real trade-off worth naming.

Scope: bug 1 only

Bug 2 is not addressed here. services/oss/src/agent/app.py on current main is 168 lines and contains no _resolve_session_connection — that logic has moved since the issue was filed, so the path in the description is stale and the two fixes no longer live near each other. It needs re-locating before it can be fixed properly, and doing that blind in this PR would be guesswork.

On process: @sumit1kr posted a correct analysis of both bugs on 7 July and asked to be assigned. That went unanswered for 24 days and the issue was never assigned, so per CONTRIBUTING ("An issue may only be assigned to one person for up to one week... If the issue remains unsolved after a week, it will be unassigned and made available to others") I've picked up bug 1 and left bug 2 to them. I credited their analysis on the issue before opening this. If they are still on it, close this and I'll step aside.

Testing

Verified locally

Environment: Windows, Python 3.13.4, uv 0.11.14 (the pinned UV_VERSION), ruff 0.15.12 (the pinned CI version).

pytest oss/tests/pytest/unit/agents/platform/test_connections_http.py

	Result
pristine 53aaf6f5a	27 passed
this branch	32 passed (+5 new)
negative control — revert only connections.py, keep the new tests	3 failed / 29 passed

The negative control fails on exactly the three tests that assert the new ranking, while both guard tests (custom provider still wins with no matching standard key; the named path still narrows by model id) pass unpatched — which is the point, since they assert preserved behaviour.

Full unit layer, the same set CI runs (pytest oss/tests/pytest/unit, -n auto via pytest.ini):

	Result
pristine 53aaf6f5a	1 failed, 1838 passed, 1 skipped, 10 xfailed
this branch	1 failed, 1843 passed, 1 skipped, 10 xfailed

+5 passed is exactly the tests added, and the one failure is identical on both sides: agents/test_streaming.py::test_cli_stream_terminal_only_on_empty_request, which fails on pristine main too with FileNotFoundError: [WinError 2] from subprocess — a Windows-only environment failure unrelated to this change.

One flake, disclosed: the first branch run also failed agents/test_runner_transport_timeout.py::test_subprocess_stream_survives_a_run_longer_than_the_idle_window. It passes 4/4 in isolation and did not recur on a second full run (which completed in 36s vs 73s, so the first run was under heavier machine load). It is an idle-window timing test running under -n auto, so I've treated it as load-sensitive rather than caused by this change — the change touches only candidate ranking and cannot affect subprocess streaming.

Linting, at CI's pinned ruff==0.15.12: ruff format --check → 465 files already formatted; ruff check → All checks passed!

Added or updated tests

Five tests in oss/tests/pytest/unit/agents/platform/test_connections_http.py:

• test_default_standard_key_outranks_custom_provider_model_list — the issue's exact scenario, bare gpt-4o-mini with both connections present.
• test_default_standard_key_outranks_custom_provider_with_explicit_prefix — same with the provider stated rather than inferred from the catalog.
• test_default_custom_provider_still_wins_without_a_matching_standard_key — guards against over-narrowing: an unrelated family's key must not suppress the custom provider that genuinely serves the model.
• test_named_connection_still_narrows_by_model_id — guards that _choose_named is unaffected.
• test_default_two_standard_keys_with_a_custom_model_match_is_ambiguous — pins judgement call 1 above.

QA follow-up

• Not reproduced against a live stack. The issue was found by probing custom-provider connections end to end against the dev stack; I verified at the resolver boundary via the existing fake_http unit seam, not against a real vault, so a quick end-to-end confirmation with two real overlapping connections is worth doing.
• One deviation from uv.lock, disclosed. uv sync --locked cannot complete on this machine: litellm==1.92.0 resolves to an sdist that builds a pyo3/maturin Rust extension, and this box has neither Rust nor MSVC build tools. I synced every other locked dependency and installed litellm==1.94.1 (wheel-only) so the import chain in agenta/sdk/utils/assets.py resolves. This does not affect what is under test: supported_llm_models is agenta's own hard-coded dict in assets.py, and litellm is used there only for cost_calculator.cost_per_token. The repo already tolerates litellm build drift — test_supported_llm_models.py xfails 10 OpenRouter ids with "not yet in this litellm build's model_cost". Worth a confirming run on CI's exact lock.
• The web/API/services CI jobs also trigger on sdks/python/** paths; this change is Python-only and touches no TypeScript, so I have not run them locally.

Demo

No UI in this change, so this is a capture of the actual resolver behaviour before and after. The script builds the two vault records the issue describes — a slug-less standard OpenAI provider_key and a slug-less custom_provider whose models list also advertises gpt-4o-mini — and prints which credential actually resolves. Run against pristine main 53aaf6f5a, then against this branch:

Verbatim output, pristine main:

vault contains:
  1. provider_key    openai            -> sk-STANDARD-key
  2. custom_provider my-gw  (openai)   -> sk-CUSTOM-gateway,  models=['gpt-4o-mini']

running model: gpt-4o-mini   (slug-less default connection)

  resolved provider    : openai
  resolved credential  : sk-CUSTOM-gateway

  => the custom gateway shadowed the standard key.  BUG (#5117 bug 1)

and on this branch, same script, same input:

  resolved provider    : openai
  resolved credential  : sk-STANDARD-key

  => the standard OpenAI key resolved.  CORRECT

The repro script is not included in the diff (it is scratch, not a test — the equivalent assertions live in test_connections_http.py); the image is hosted on a branch of my fork so it stays out of this PR's two files. Happy to attach it as a proper test fixture or a scripts/ helper if you'd find it useful.

Checklist

• I have included a video or screen recording for UI changes, or marked Demo as N/A
• Relevant tests pass locally
• Relevant linting and formatting pass locally
• I have signed the CLA, or I will sign it when the bot prompts me

_{Note on the auto-close: the PR template says the demo is "required for UI changes... if this is not a UI change, write N/A", so I first wrote N/A. 13-check-pr-contribution.yml is stricter than the template — it requires media for any non-exempt file, and sdks/python/agenta/** is not exempt. Worth reconciling the template wording with the workflow, since a Python-only SDK fix reads as exempt from the template alone.}

[vercel]

@Souptik96 is attempting to deploy a commit to the agenta projects Team on Vercel.

A member of the Team first needs to authorize it.

[CLAassistant]

All committers have signed the CLA.

[github-actions]

✅ Thanks @Souptik96! This PR now meets the contribution requirements and has been reopened. A maintainer will review it soon.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7ddebcbe-2b45-43e3-b609-de5bf7b735ae

📥 Commits

Reviewing files that changed from the base of the PR and between 53aaf6f and 3d35f66.

📒 Files selected for processing (2)

• sdks/python/agenta/sdk/agents/platform/connections.py
• sdks/python/oss/tests/pytest/unit/agents/platform/test_connections_http.py

---

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes

  • Improved default connection selection when standard provider keys and custom-provider model matches are both available.
  • Preserved custom-provider selection when no matching standard key exists.
  • Maintained expected behavior for named connections and ambiguous matches.

• Tests

  • Added coverage for provider precedence, explicit provider prefixes, named connections, and ambiguity handling.

Walkthrough

Slug-less default connection resolution now prioritizes matching standard provider keys over custom-provider model matches. Named connections and ambiguity handling remain unchanged. Regression tests cover the updated resolution cases.

Changes

Default connection resolution

Layer / File(s)	Summary
Default candidate pool ranking sdks/python/agenta/sdk/agents/platform/connections.py	_default_candidate_pool prioritizes matching provider_key candidates. _choose_default uses this pool for slug-less resolution.
Resolution regression coverage sdks/python/oss/tests/pytest/unit/agents/platform/test_connections_http.py	Tests cover standard-key precedence, custom-provider fallback, named custom connections, and ambiguity handling.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

• Agenta-AI/agenta 5117 — Addresses standard provider-key precedence over custom-provider model matches.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the fix: standard provider keys now rank above custom model lists during SDK connection resolution.
Description check	✅ Passed	The description directly explains the resolver bug, implementation, preserved behavior, tests, and scope of the changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Fix failing CI checks

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[Souptik96]

@coderabbitai review

Context for the skip: this PR was auto-closed by 13-check-pr-contribution.yml about 20 seconds after it opened (I had marked Demo as N/A, following the template's "required for UI changes" wording — the workflow is stricter and wants media for any non-exempt file, which includes sdks/python/agenta/**). The review was skipped during that window and the PR has since been updated with a before/after capture and automatically reopened, so there is now a diff to look at.

Two notes for whoever reviews:

• The red Vercel check is Authorization required to deploy, which is the fork-authorization gate rather than anything in this change. I believe this is the same limitation @mmabrouk described on fix(sdk): make LocalSandboxNotAllowedError.type reflect the refused provider #5581 — fork PRs can't reach the build secrets.
• CLA is not signed yet. That is on me and I'll get it signed; flagging it so it isn't mistaken for an unanswered request.

[coderabbitai]

@Souptik96 I will review the current diff for #5601.

I will treat the Vercel Authorization required to deploy status as a fork authorization limitation. I will not treat it as evidence of a defect in this change.

The Comment image supports the stated regression case and the focused test results. The pending CLA remains an external merge requirement.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.