Fix cross-provider key name collision in fallback lookup

[SahilChoudhary22]

Summary

Fixes the startup sync (UpdateProvidersConfig) to scope key name lookups by provider_id. Previously, when the sync encountered a key whose UUID didn't match any existing row, it fell back to a global name lookup (Where("name = ?")) that could match a key belonging to a different provider, silently overwriting it.

The fix adds AND provider_id = ? to the name fallback query, ensuring it only matches keys within the same provider. This aligns with the new composite unique index (provider_id, name) introduced in the sibling PR, where key names are unique per-provider rather than globally.

Changes

• rdb.go: Changed the name fallback lookup in UpdateProvidersConfig from Where("name = ?") (global) to Where("name = ? AND provider_id = ?") (scoped to the current provider)
• rdb_test.go: Added TestUpdateProvidersConfig_CrossProviderSameKeyName regression test that creates two providers each with a key named "default" and verifies both keys are preserved after the sync

Type of change

• Bug fix
• Feature
• Refactor
• Documentation
• Chore/CI

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations
• Plugins
• UI (React)
• Docs

How to test

The regression test directly validates the fix:

cd framework/configstore && go test -run TestUpdateProvidersConfig_CrossProviderSameKeyName -v

On the old (buggy) code, this test fails with "[] should have 1 item(s), but has 0" for the first provider. On the fixed code, it passes.

All existing tests still pass:

cd framework/configstore && go test ./...

Screenshots/Recordings

N/A — No UI changes.

Breaking changes

• Yes
• No

The fix only changes the startup sync behavior from "wrong" (global lookup, data-corrupting) to "right" (scoped lookup). No API or schema changes.

Related issues

Fixes the root cause of the HTTP 500 error on provider update: the startup sync was corrupting key ownership between providers, and the subsequent single-provider update couldn't create a new key because the name was already taken globally.

Security considerations

None.

Checklist

• I read docs/contributing/README.md and followed the guidelines
• I added/updated tests where appropriate
• I updated documentation where needed
• I verified builds succeed (Go and UI)
• I verified the CI pipeline passes locally if applicable

Why this matters

Before this change, API key names had to be unique across all providers. If you had two providers - say OpenAI and Anthropic - they could not both have an API key named "default". The second one would collide with the first and fail with an error.

After this change, key names only need to be unique per provider. Each provider can independently have a "default" key (or any other name) without conflict.

[coderabbitai]

📝 Walkthrough

Walkthrough

Scopes the fallback lookup in UpdateProvidersConfig to include provider_id when resolving keys by name, and adds a test ensuring providers with identical key names keep distinct key rows and values.

Changes

Configuration key lookup fix

Layer / File(s)	Summary
Provider-scoped key lookup framework/configstore/rdb.go	UpdateProvidersConfig fallback path queries keys by both name and provider_id instead of name alone, preventing collisions when multiple providers share a key name.
Cross-provider collision test framework/configstore/rdb_test.go	New test TestUpdateProvidersConfig_CrossProviderSameKeyName verifies that two providers with identically-named keys ("default") retain separate, unchanged rows and values after UpdateProvidersConfig executes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 I hopped through code both near and far,
Two keys named "default" beneath the star,
Scoped by provider, they now stay apart,
Each burrow keeps its own secret heart. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Description check	✅ Passed	The pull request description is comprehensive and follows the template structure with all required sections properly filled out.
Title check	✅ Passed	The title 'Fix cross-provider key name collision in fallback lookup' accurately describes the main change: scoping the name fallback lookup in UpdateProvidersConfig by provider_id to prevent cross-provider key collisions.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 05-18-fix_configstore_scope_updateprovidersconfig_name_fallback_by_provider_id

---

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 and usage tips.}

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[SahilChoudhary22]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• Fix cross-provider key name collision in fallback lookup #3570 👈 (View in Graphite)
• fix(configstore): change config_keys.name unique index to composite (provider_id, name) #3569
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

[greptile-apps]

Confidence Score: 5/5

Safe to merge — the change is a targeted one-liner that corrects a clearly wrong query predicate, and the new regression test directly validates the fixed behavior.

The fix is minimal and precisely scoped: one additional AND provider_id = ? bind parameter in a single GORM query. The surrounding logic is unchanged, the new parameter (dbProvider.ID) is already in scope and correctly reflects the current provider being iterated, and the regression test confirms both providers keep their own keys after the sync.

No files require special attention.

Important Files Changed

Filename	Overview
framework/configstore/rdb.go	Single-line fix scoping the name-fallback query in UpdateProvidersConfig to the current provider_id, preventing cross-provider key corruption
framework/configstore/rdb_test.go	Adds a regression test covering the cross-provider same-name collision that the fix addresses; test correctly exercises the fallback code path within a single UpdateProvidersConfig call

_{Reviews (2): Last reviewed commit: "fix(configstore): scope UpdateProvidersC..." | Re-trigger Greptile}

[SahilChoudhary22]

As discussed in Slack, this change has been marked as good to have and moved to the backlog for now.

While scoping API key name lookups by provider_id resolves the immediate cross-provider collision issue, it would also require follow-up changes to logging, since logs currently only capture the API key name and not the associated provider. To avoid introducing ambiguity there, we’re deferring the broader fix for now.

In the meantime, this is being addressed via #3574, which improves the user-facing error message to clearly communicate that API key names must be unique across providers.