fix(providers): correct provider settings behavior

[Mingholy]

Summary

• Fix provider settings correctness issues across provider/config/model selection flows.
• Tighten provider behavior so settings and model selection stay consistent.
• Cover the corrected provider behavior with focused server tests.

Test plan

• cd web && bunx tsc -b
• bun run build
• git diff --check
• Focused provider correctness tests were run during implementation.
• Lint not run: root, web, and server package manifests do not define a lint script.

Screenshots

1. Missing-key hint in the model selector

The newly surfaced "missing key" hint appears when a personal provider references a ${VAR} that is not resolvable from the vault env. Reproduced by configuring a provider (OpenRouter -> ${OPENROUTER_API_KEY}) without defining that variable, then opening the model selector.

Before (main)	After (this PR)
Keyless personal provider is silently dropped from /api/providers -- the model selector gives no indication that a provider is unusable or why.	The provider is surfaced as disabled with missingVar metadata, and the model selector shows an amber hint: Missing key: OpenRouter needs OPENROUTER_API_KEY in Settings / AI Providers.

2. Renaming a workspace provider warns that the stored key cannot be carried over

Renaming a workspace provider that has a stored key (Settings -> Workspace AI Providers). Because the key reference is derived from the provider name, the old reference cannot survive the rename -- the new code clears hasKey and warns the user to re-enter the key.

Before (main)	After (this PR)
After rename the API key still shows stored and the provider stays enabled -- misleading, since the stored key reference no longer resolves.	hasKey is cleared (key field empties, provider disabled) and a warning appears: Renamed "idealab" to "idealab-renamed". Re-enter and test the workspace API key before saving; the stored key reference cannot be preserved through this rename.

[Mingholy]

Code Review: PR #58

Summary

This PR fixes several interrelated bugs in the provider settings flow across server and web:

1. saveWorkspaceConfig provider set replacement -- Providers deleted in the admin UI were not actually removed from the config because the old code merged into the existing map instead of replacing it.
2. getProvidersResponse extraction -- Inline code in the /api/providers handler was extracted to a reusable function in config.ts, with new behavior: personal providers whose ${VAR} references don't resolve are now surfaced as disabled entries with missingVar metadata instead of being silently dropped.
3. Settings page apiKeyRef preservation -- Saving personal provider settings (e.g. changing models or baseUrl) no longer silently overwrites a custom ${CUSTOM_KEY} reference with a generated ${PROVIDERNAME_API_KEY}.
4. Admin dialog rename warning -- Renaming a workspace provider now warns the user that the API key cannot survive the rename and must be re-entered.
5. Model selector missing-key hint -- Amber banner in the model dropdown shows which personal providers have unresolved env vars.

Bug Verification

Bug 1 (provider deletion): Confirmed real. In main, saveWorkspaceConfig does merged.providers[name] = ... inside a loop over cfg.providers, but never removes entries absent from cfg.providers. When the admin UI sends the desired provider set, stale providers survive.

Bug 2 (silent provider drop): Confirmed real. The old /api/providers handler only overlaid personal providers where hasKey was true. A user who configures a personal provider with ${MY_KEY} but hasn't yet created the vault env file would see the provider vanish from the UI with no explanation.

Bug 3 (apiKey ref overwrite): Confirmed real. The old SettingsPage.tsx save logic unconditionally wrote apiKey: \${${providerEnvVarName(name)}}`` regardless of whether the user touched the key. If the provider was originally configured with a non-standard var name, saving any other field (models, baseUrl, enabled) would silently break the key reference.

Bug 4 (rename key loss): Partially a bug, partially UX improvement. After rename, the server-side saveWorkspaceConfig looks up existingProv by the new name (which doesn't exist), so apiKey falls to "". The warning is the correct behavior given this server-side constraint.

Fix Analysis

All four fixes correctly address their respective bugs. A few observations:

Default resolution change (getProvidersResponse):

active = pCfg.default || Object.keys(pCfg.providers)[0] || active

The old code was pCfg.default || active. The new fallback to Object.keys(pCfg.providers)[0] can set the default to a provider without a key -- e.g., a user adds a personal provider template but hasn't set the vault env yet. The UI would show this keyless provider as the "active" default, but pickProvider at runtime would skip it (it has requireKey logic). This creates a subtle mismatch between the UI indicator and actual runtime behavior. Not a blocker -- pickProvider is the safety net -- but worth being aware of. If you want strict correctness, filter to keyed providers before falling back:

active = pCfg.default || Object.keys(pCfg.providers).find(n => {
  const p = pCfg.providers[n]
  return typeof p.apiKey === "string" && p.apiKey.length > 0
}) || active

readPersonalDiskRaw extra read: The new getProvidersResponse calls readPersonalDiskRaw to extract the raw ${VAR} reference for missingVar. This is an additional uncached disk read on every GET /api/providers. loadPersonalConfig is already called (and cached); readPersonalDiskRaw bypasses that cache. For a settings endpoint the cost is negligible, but if this endpoint is polled frequently it could add up. Consider caching or combining with the existing read.

AdminDialog warnReEnterKey: The check !draft.providers[newName] is redundant -- the if (d.providers[newName]) return d guard inside setDraft already prevents the rename when the target name exists. Harmless, but unnecessary.

Code Quality

• Extraction to getProvidersResponse is a solid improvement -- it centralizes provider response assembly, makes it testable, and exposes proper ProviderInfo / ProvidersResponse types.
• Test coverage is well-targeted: the new test file covers the specific regression scenarios (provider deletion, missing var metadata, default fallback). Setup and teardown are clean.
• apiKeyRef tracking adds one field to the draft type and uses it precisely where needed. Minimal surface area.
• Minor: trailing newline removal at end of SettingsPage.tsx (line 1089) is fine but unrelated.

Security

No security concerns. The missingVar field only exposes the env var name (e.g., GHOST_API_KEY), never the value. The readPersonalDiskRaw call is scoped to the authenticated user's personal dir.

Verdict

LGTM with one minor suggestion. The core fixes are correct, well-scoped, and properly tested. The only substantive nit is the default-resolution fallback to Object.keys(pCfg.providers)[0] which can pick a keyless provider as "active" in the API response -- consider filtering to keyed providers in that fallback. Otherwise, good to merge.

[Mingholy]

已修复 default provider 回退逻辑（getProvidersResponse，server/src/config.ts 第 705 行）。

改动说明：

原来的 fallback：
```ts
active = pCfg.default || Object.keys(pCfg.providers)[0] || active
```
当 pCfg.default 为空时会直接取 providers 列表第一个，可能选到没有 API key 的 provider（如 ${MY_KEY} 引用但 vault env 文件不存在），导致 UI 显示一个 keyless provider 为 active default，与 runtime pickProvider 的跳过逻辑不一致。

修复后只回退到有真实 apiKey 的 provider：
```ts
active = pCfg.default ||
Object.keys(pCfg.providers).find(n => {
const p = pCfg.providers[n]
return typeof p.apiKey === "string" && p.apiKey.length > 0
}) ||
active
```

commit: 08b8646

[Mingholy]

最终 Review 结论 · ✅ 可合并

意图与达成度：四个关联 bug 在最新代码（08b8646）中均被正确、最小化修复：

1. 删除的 provider 未真正移除 —— config.ts:914-930 改为构建全新 nextProviders 整体替换，并回退保留未提交字段。
2. 未解析的个人 provider 被静默丢弃 —— getProvidersResponse（config.ts:692-703）以 enabled:false/hasKey:false/missingVar 暴露。
3. 保存覆盖自定义 ${CUSTOM_KEY} 引用 —— SettingsPage.tsx:544-546 仅在用户输入新 key 时生成标准 env 名，否则保留 apiKeyRef。
4. workspace provider 重命名后 key 丢失 —— AdminDialog.tsx:375-390 增加重命名警告并置 hasKey=false（UX 缓解）。

回归测试 provider-settings-fixes.test.ts 3 pass。

历史 review comment：default 回退可能选中无 key 的 provider —— 已在 08b8646 改为 find() 仅回退到有真实 apiKey 的 provider（config.ts:705-710），并加回归测试。✅ 两个 nit（额外磁盘读 / 冗余判断）原 review 判定 harmless，不阻塞。

安全（密钥泄漏 / SSRF）：clean。

• 响应 ProviderInfo 不含 apiKey，仅 hasKey:boolean + missingVar（env 变量名非值）；describeApiKeyRef 从不返回值；DOM 无密钥。本 PR 未暴露此前被掩码的任何密钥，write-only 语义保持。
• baseUrl 校验/网络调用路径未改动，未引入新 SSRF 面。
• /api/providers 仍 requireAuth，按当前 userId 限定个人目录读取，无越权。

遗留 nit（非阻塞）：SettingsPage.tsx:484-499 个人 provider 重命名后携旧 apiKeyRef，相比旧行为是改善（非回归），可选地对齐 AdminDialog 的重命名提示。

无 Critical/High/Medium 问题。

_{🤖 Generated with Claude Code}