fix: use per-provider env key for custom providers in Models dialog#737
fix: use per-provider env key for custom providers in Models dialog#737365diascollaboration-prog wants to merge 3 commits into
Conversation
Custom providers with unrecognised base URLs all resolved to the shared CUSTOM_API_KEY env var via expectedEnvKeyForUrl(). Adding a second custom provider with a different URL would overwrite the first provider's key, making it impossible to run two custom providers simultaneously. Introduce customProviderEnvKey() that falls back to CUSTOM_PROVIDER_<NAME>_KEY (matching the naming already used by seedDefaults() in models.ts) when the URL is not in URL_KEY_MAP. Apply the same key derivation in openEditModal() so the field reads back the correct value, with a fallback to CUSTOM_API_KEY for keys written by older versions. Fixes fathah#681
|
Thanks for the fix. The direction makes sense, but I think this currently only fixes the Models dialog storage/display side, not the full runtime path. For known custom-compatible hosts, this should still work because we already resolve URL-specific env vars like The issue is unknown custom base URLs. This PR saves the key as:
But the active chat/gateway path does not consistently read that key. In the normal active config path, unknown custom URLs still rely on Can we either:
Without one of those, this may regress unknown custom providers even though the dialog looks correct. |
…E>_KEY config-health was not aware of per-provider env keys written by the Models dialog and seedDefaults(), so it could report MODEL_KEY_MISSING even when the gateway runtime would resolve the key correctly. Mirror the same baseUrl->name lookup that hermes.ts already uses so the health check and the runtime stay in sync.
|
Thanks for the detailed review, fathah. You're right that the original PR only covered the dialog side. I've pushed a second commit that extends customEndpointKeyResolvable() in config.ts to also resolve CUSTOM_PROVIDER__KEY:
This way the config-health path and the runtime gateway path stay in sync — no false MODEL_KEY_MISSING warning for a provider whose key was saved under CUSTOM_PROVIDER__KEY. The gateway runtime at hermes.ts:startGateway already had the CUSTOM_PROVIDER__KEY fallback (lines 2370–2383), so chat itself should resolve the key correctly once it's written. The gap was only in the health-check side, which this commit closes. |
Greptile SummaryThis PR fixes a key-collision bug where every custom provider with an unrecognised base URL shared the single
Confidence Score: 4/5The core fix is sound and correctly addresses the key-collision problem for new providers; existing providers get a safe read-back fallback. The fix correctly isolates per-provider env keys and aligns the GUI dialog with the existing seedDefaults() naming convention. Three minor concerns were found: customProviderEnvKey is inserted between two import blocks rather than before them; renaming a provider during edit leaves the old CUSTOM_PROVIDER__KEY orphaned in the env file with no cleanup; and the health-check find() in config.ts only picks up the first model matching a given base URL, so two differently-named providers sharing the same unknown URL would still trigger a false health warning for the second one. None of these affect the primary scenario described in the PR. Both changed files warrant a second look: Models.tsx for the import ordering and the missing old-key cleanup on rename, and config.ts for the single-match limitation in the health-check lookup. Important Files Changed
|
| } | ||
|
|
||
| if (formApiKey.trim() && formProvider === "custom") { | ||
| const envKey = expectedEnvKeyForUrl(formBaseUrl.trim()); | ||
| const envKey = customProviderEnvKey(formName.trim(), formBaseUrl.trim()); | ||
| await window.hermesAPI.setEnv(envKey, formApiKey.trim()); | ||
| } |
There was a problem hiding this comment.
Orphaned env key when provider name is changed during edit
When a user edits an existing custom provider and renames it (e.g. "Foo" → "Bar"), handleSave writes the API key to CUSTOM_PROVIDER_BAR_KEY while CUSTOM_PROVIDER_FOO_KEY is left behind with a stale value. The old key is never deleted. Since the save path has access to both editingModel?.name (old name) and formName (new name), it could issue a setEnv(oldKey, "") call when the name changes during an edit.
| try { | ||
| // eslint-disable-next-line @typescript-eslint/no-require-imports -- call-time require; models.ts has no dep on config.ts so no cycle. | ||
| const modelsMod = require("./models") as typeof import("./models"); | ||
| const matching = modelsMod.readModels().find((m) => m.baseUrl === baseUrl); | ||
| if (matching) { | ||
| candidates.add( | ||
| "CUSTOM_PROVIDER_" + | ||
| matching.name.replace(/[^A-Za-z0-9]/g, "_").toUpperCase() + | ||
| "_KEY", | ||
| ); | ||
| } | ||
| } catch { | ||
| /* ignore */ |
There was a problem hiding this comment.
readModels().find() returns only the first match when two providers share a base URL
customEndpointKeyResolvable resolves the candidate key by matching the baseUrl argument against the model list. If two custom providers share the same base URL (both unrecognised by URL_KEY_MAP), find returns only the first and only that provider's CUSTOM_PROVIDER_<NAME>_KEY is added to candidates. The health check would falsely report the second provider's key as missing even though the key exists under its own name.
Problem
When adding multiple custom providers with unrecognised base URLs, they all resolve to the shared
CUSTOM_API_KEYenv var viaexpectedEnvKeyForUrl(). The second provider's key silently overwrites the first, making it impossible to run two custom providers simultaneously. Closes #681.Root cause
expectedEnvKeyForUrl()returnsCUSTOM_API_KEYas a fallback for any URL not inURL_KEY_MAP. The save path inhandleSave()and the read-back path inopenEditModal()both use this function, so all unknown-URL providers share one bucket.Note:
seedDefaults()inmodels.tsalready writesCUSTOM_PROVIDER_<NAME>_KEYcorrectly for providers seeded fromconfig.yaml. This fix brings the GUI dialog into alignment with that existing behavior.Fix
Introduce
customProviderEnvKey(name, baseUrl)— a thin wrapper overexpectedEnvKeyForUrlthat falls back toCUSTOM_PROVIDER_<NAME>_KEY(same naming asseedDefaults) instead of the sharedCUSTOM_API_KEYwhen the URL is unknown.Three call-site changes, all in
Models.tsx:handleSave()— writes the per-provider keyopenEditModal()— reads the per-provider key, falls back toCUSTOM_API_KEYfor keys written by older versionsTesting
.env(CUSTOM_PROVIDER_<NAME>_KEY)