Preserve provider_config_id until bound-delete query is replaced

This update path always writes provider_config_id as NULL, but provider deletion still soft-deletes bound models via SoftDeleteBoundChatModelConfigsByProviderConfigID (which filters on chat_model_configs.provider_config_id). After any model edit, that row no longer matches the delete query, so deleting a provider leaves previously bound models undeleted and drops their attachment rows via FK cascade; chats then reference a model that now fails runtime resolution (no provider configs attached) instead of preserving historical binding.

Useful? React with 👍 / 👎.

Seed migration attachments from legacy bound config first

The backfill inserts attachments by joining every enabled provider in the same family, but it never reads the existing chat_model_configs.provider_config_id binding. On upgrade, models that were explicitly bound to a specific provider config can be reordered to a different first attachment (or attached to multiple configs they were never bound to), which changes runtime provider selection as soon as resolveLanguageModelForConfig starts using attachment priority.

Useful? React with 👍 / 👎.

Clear fallback API key when attached config has no key

This helper only overwrites keys when dbAPIKey is non-empty, so attachment resolution can inherit a family-level fallback key from another config in baseKeys. In resolveLanguageModelForConfig, that inherited key makes resolvedKeys.APIKey(...) non-empty, so an attachment with no own key is treated as usable instead of being skipped, breaking per-config fallback ordering and key isolation for same-provider attachments.

Useful? React with 👍 / 👎.

P0 JOIN queries bypass dbcrypt decryption, sending encrypted ciphertext as LLM API keys in enterprise deployments. (Knuckle P0)

GetModelProviderConfigs and GetModelProviderConfigsByModelIDs SELECT cp.api_key via a JOIN with chat_providers. The enterprise/dbcrypt layer intercepts queries that return database.ChatProvider and decrypts APIKey. These new queries return custom row types with no dbcrypt wrapper.

Verified: enterprise/dbcrypt/dbcrypt.go has wrappers for GetChatProviderByID, GetEnabledChatProviders, InsertChatProvider, UpdateChatProvider, but none for GetModelProviderConfigs or GetModelProviderConfigsByModelIDs. The tests pass because dbtestutil.NewDB does not wrap with dbcrypt.

Two consequences: (1) resolveLanguageModelForConfig sends ciphertext as bearer tokens, every LLM call returns 401, chat is dead for enterprise+dbcrypt. (2) The ciphertext overwrites correctly resolved user keys in baseKeys.

Fix: either add dbcrypt wrappers for both queries (requires selecting cp.api_key_key_id in the JOIN), or stop selecting cp.api_key in the join query and load the decrypted key via the existing GetChatProviderByID when the runtime needs it.

🤖

P1 Backfill ignores existing provider_config_id bindings, expanding all models to every enabled family provider. (Mafuuu P2, Knuckle P1, Knov P0)

The backfill joins chat_model_configs cmc JOIN chat_providers cp ON cp.provider = cmc.provider AND cp.enabled = TRUE. This matches ALL enabled providers in the family, regardless of whether cmc.provider_config_id pointed to one specific provider.

Scenario: two OpenAI configs (A created Jan 1, B created Feb 1). Model was bound to B. Backfill creates A at priority 0, B at priority 1. Runtime picks A. The model silently switches providers, potentially using a different API key and base URL.

Also, models bound to disabled providers get zero attachments (the JOIN filters cp.enabled = TRUE), making them permanently unusable. The old code fell back to family defaults.

Fix: distinguish bound from unbound models in the backfill. Bound models should get a single priority-0 attachment for their explicit binding. Unbound models get all enabled family providers.

🤖

P1 Provider credential policy flags (CentralApiKeyEnabled, AllowUserApiKey, AllowCentralApiKeyFallback) are ignored at runtime. (Mafuuu P1, Meruem P1, Hisoka P2, Kite P2, Knov P2, Knuckle P2)

Six reviewers independently flagged this. ProviderKeysFromConfig(baseKeys, attachment.Provider, attachment.ProviderApiKey, attachment.ProviderBaseUrl) unconditionally overlays the DB-stored API key. The join query does not even SELECT central_api_key_enabled, allow_user_api_key, or allow_central_api_key_fallback.

The deleted resolveProviderAPIKeysForModel routed every provider through ResolveUserProviderKeys, which evaluated all three flags. The deleted TestResolveProviderAPIKeysForModel (436 lines) covered these scenarios. No replacement tests exist.

Concrete failures:

• CentralApiKeyEnabled=false with a non-empty DB key: the key is used anyway.
• AllowUserApiKey=true with a user key: the DB key overwrites the user key in baseKeys.
• AllowUserApiKey=false: user keys leak through baseKeys from family-level resolution.

Fix: add the three policy columns to GetModelProviderConfigs and evaluate them per-attachment, or call ResolveUserProviderKeys per attachment.

🤖

P2 Provider family change without provider_config_ids silently creates cross-family orphan attachments. (Mafuuu P2, Meruem P1, Kite P2, Kurapika P2, Knov P1)

The old code tracked providerChanged and auto-resolved a new provider binding. The new code removed providerChanged tracking entirely.

PATCH {"provider": "anthropic"} on an openai model without provider_config_ids: existing openai attachments remain. At runtime, resolveLanguageModelForConfig uses attachment.Provider ("openai") to build the SDK client with modelConfig.Model (possibly an anthropic model name). The model is silently broken.

Tests ProviderChangePreservesExistingAttachments and ProviderChangePreservesAttachmentsEvenWithNoMatchingProvider assert this as intentional. But the runtime consequence is not tested.

Fix: either require provider_config_ids when provider changes (reject with 400), or auto-delete stale attachments and auto-attach the new family's provider.

🤖

P2 ModelFromConfig error halts the fallback loop instead of continuing to the next attachment. (Netero P2, Chopper P2, Bisky P2)

The loop returns immediately when chatprovider.ModelFromConfig fails for the first enabled+keyed attachment. A misconfiguration in the highest-priority attachment stops the entire fallback chain.

The error message carries no identification of which provider config was attempted. An operator receiving "create model: invalid base URL" cannot tell which attachment out of three has the problem.

No test exercises this path. FallbackToSecondProvider tests disabled and keyless skipping but not SDK-construction failures.

Fix: log the per-attachment error at debug level with the provider config ID, continue to the next attachment. Return hard only after all attachments are exhausted.

🤖

P2 Frontend silently swallows update when user removes all provider config attachments. (Chopper P2)

if (existingOrderedIds.length > 0 && currentIds.length === 0) {
    return;
}

The user can remove all attachments via the X buttons. When they click Save, this guard silently returns without sending a request, showing an error, or providing visual feedback. The backend would reject with 400 ("Provider config IDs cannot be empty"), but the request is never sent.

Fix: either disable the Save button when zero attachments remain, show a validation error, or let the request through so the backend's error message reaches the user.

🤖

P2 Down migration is lossy: rolling back leaves all models permanently unbound. (Meruem P2, Kite P3)

The up migration backfills the join table from existing data. The runtime code sets provider_config_id = uuid.NullUUID{} on every insert/update. The down migration only drops the join table. After rollback, every model has provider_config_id = NULL. The three queries that depend on this column (GetEnabledChatModelConfigs, SoftDeleteBound..., SoftDeleteUnbound...) will treat all models as unbound.

Fix: restore provider_config_id from the priority-0 row in chat_model_provider_configs before dropping the table.

🤖

P2 HasAPIKey checks only the DB-stored key, diverging from the provider-level effectiveChatProviderConfigHasAPIKey logic. (Chopper P2)

convertProviderConfigAttachments sets HasAPIKey: row.ProviderApiKey != "". The provider listing endpoint uses effectiveChatProviderConfigHasAPIKey, which merges deployment-level keys (e.g., CODER_AI_OPENAI_API_KEY env var). A provider with no DB key but an env var shows HasAPIKey: false in the attachment, misleading the admin.

Additionally, ProviderKeysFromConfig uses strings.TrimSpace(dbAPIKey), so a whitespace-only DB key shows HasAPIKey: true here but is treated as empty at runtime. (Knov Note)

Fix: use the same effectiveChatProviderConfigHasAPIKey computation, or add a TrimSpace before the empty check.

🤖

P3 resolveLanguageModelForConfig queries the DB per chat message, bypassing the config cache. (Netero P3, Meruem P3, Kite Note)

The old code used p.configCache.EnabledProviders and p.configCache.EnabledProviderByID. The new code calls p.db.GetModelProviderConfigs(ctx, modelConfig.ID) directly. This adds a DB round-trip per message. The attachment data is stable between config changes and would benefit from the existing configCache pattern.

This also creates a split: cached for user keys (via resolveUserProviderAPIKeys), uncached for provider attachments. Stale cache entries and fresh DB reads can produce contradictory state within a single resolution.

🤖

Keep provider_config_id populated until delete cleanup is migrated

Writing ProviderConfigID as uuid.NullUUID{} here breaks provider-deletion cleanup, because deleteChatProvider in the same file still soft-deletes bound models through SoftDeleteBoundChatModelConfigsByProviderConfigID (legacy chat_model_configs.provider_config_id). In a multi-config family, deleting one provider skips unbound cleanup, cascades away attachment rows, and leaves the model enabled but unattached; runtime resolution then fails with no provider configs attached. Keep the legacy column in sync (or migrate delete cleanup to the attachment table) before clearing it on create/update.

Useful? React with 👍 / 👎.

Allow env-key fallback when a model has no provider attachments

This hard failure regresses env-only setups: the commit permits models with zero attachments (e.g., createChatModelConfig leaves resolvedProviderConfigIDs empty when no enabled DB provider exists, and migration 000466 backfills only from chat_providers rows), but resolveLanguageModelForConfig now immediately errors instead of using existing deployment/user key fallback. That makes previously valid env-backed models unavailable with no provider configs attached.

Useful? React with 👍 / 👎.