diff --git a/server/src/config.ts b/server/src/config.ts index 78f04a40..6f1359b8 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -108,6 +108,20 @@ export type ProviderConfig = { enabled: boolean } +export type ProviderInfo = { + models: ModelEntry[] + baseUrl: string + source: "personal" | "workspace" + enabled: boolean + hasKey: boolean + missingVar?: string +} + +export type ProvidersResponse = { + providers: Record + default: string +} + export type RemoteSpec = { /** clone URL; empty string or omitted = local-only, don't clone */ git?: string @@ -645,6 +659,60 @@ export function describeApiKeyRef( return { kind: "mixed", exists: false } } +export async function getProvidersResponse(user: string): Promise { + const wCfg = await loadConfig() + const providers: Record = {} + if (wCfg.providers) { + for (const [name, p] of Object.entries(wCfg.providers)) { + const hasKey = typeof p.apiKey === "string" && p.apiKey.length > 0 + providers[name] = { + models: p.models, + baseUrl: p.baseUrl, + source: "workspace", + enabled: hasKey ? p.enabled : false, + hasKey, + } + } + } + + let active = wCfg.default ?? "" + try { + const pCfg = await loadPersonalConfig(user) + const rawDisk = await readPersonalDiskRaw(user).catch(() => null) + for (const [name, p] of Object.entries(pCfg.providers)) { + const hasKey = typeof p.apiKey === "string" && p.apiKey.length > 0 + if (hasKey) { + providers[name] = { + models: p.models, + baseUrl: p.baseUrl, + source: "personal", + enabled: p.enabled !== false, + hasKey, + } + } else if (!providers[name]) { + const rawApiKey = (rawDisk?.providers as any)?.[name]?.apiKey + const ref = describeApiKeyRef(rawApiKey, user) + providers[name] = { + models: p.models, + baseUrl: p.baseUrl, + source: "personal", + enabled: false, + hasKey: false, + ...(ref.kind === "var" && ref.varName && !ref.exists ? { missingVar: ref.varName } : {}), + } + } + } + active = pCfg.default || + Object.keys(pCfg.providers).find(n => { + const p = pCfg.providers[n] + return typeof p.apiKey === "string" && p.apiKey.length > 0 + }) || + active + } catch {} + + return { providers, default: active } +} + /** * Apply a structural patch to personal//.loopat/config.json. Accepts * partial fields from `PersonalConfigDisk`; only fields present on the @@ -844,20 +912,22 @@ export async function saveWorkspaceConfig(cfg: Partial): Promis const existing = await loadConfig() const merged: WorkspaceConfig = { ...existing } if (cfg.providers !== undefined) { - merged.providers = merged.providers ?? {} + const existingProviders = existing.providers ?? {} + const nextProviders: Record = {} for (const [name, p] of Object.entries(cfg.providers)) { - const existingProv = merged.providers[name] + const existingProv = existingProviders[name] const incoming = p as any const models: ModelEntry[] = incoming.models?.length > 0 ? incoming.models.map((m: any) => normalizeModelEntry(m as ModelEntryDisk)) : existingProv?.models ?? [] - merged.providers[name] = { + nextProviders[name] = { models, baseUrl: incoming.baseUrl ?? existingProv?.baseUrl ?? "", apiKey: incoming.apiKey || existingProv?.apiKey || "", enabled: incoming.enabled !== undefined ? incoming.enabled : (existingProv?.enabled ?? true), } as any } + merged.providers = nextProviders } if (cfg.default !== undefined) merged.default = cfg.default if (cfg.knowledge !== undefined) merged.knowledge = cfg.knowledge diff --git a/server/src/index.ts b/server/src/index.ts index 4ed51088..b052af13 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -56,7 +56,7 @@ import { personalReposDir, loopsDir, } from "./paths" -import { loadConfig, loadPersonalConfig, savePersonalConfig, saveWorkspaceConfig, getActiveProvider, readPersonalDiskRaw, savePersonalDisk, describeApiKeyRef, writeVaultEnv, deleteVaultEnv, loadA2AConfig, saveA2AConfig, type ProviderConfig, type ModelEntry } from "./config" +import { loadConfig, loadPersonalConfig, savePersonalConfig, saveWorkspaceConfig, getActiveProvider, readPersonalDiskRaw, savePersonalDisk, describeApiKeyRef, writeVaultEnv, deleteVaultEnv, getProvidersResponse, loadA2AConfig, saveA2AConfig, type ProviderConfig, type ModelEntry } from "./config" import { queryUserTokenUsage, queryWorkspaceTokenUsage, queryDailyTokenUsage, queryLoopTokenUsage } from "./usage" import { createApiToken, listApiTokens, revokeApiToken } from "./api-tokens" import { listBoards, createBoard, renameBoard, listKanbanColumns, addCard, toggleCard, deleteCard, moveCard, updateCardMeta, updateCardBlock, reorderCards, createColumn, deleteColumn, readKanbanConfig, saveColumnOrder, setColumnColor, renameColumn, assignDriverForCard, createLoopFromCard, linkLoopToCard, kanbanUserCtx } from "./kanban" @@ -264,31 +264,11 @@ app.get("/api/serve/check-port", requireAuth, async (c) => { // ── providers (auth required) ── // Merges personal + workspace configs. Personal providers take precedence // (they carry per-user apiKeys via secrets/). Source field indicates origin. +// Active default mirrors runtime resolution: a configured personal provider can +// become active even when workspace has a default. app.get("/api/providers", requireAuth, async (c) => { - const wCfg = await loadConfig() - const providers: Record = {} - if (wCfg.providers) { - for (const [name, p] of Object.entries(wCfg.providers)) { - const hasKey = typeof p.apiKey === "string" && p.apiKey.length > 0 - providers[name] = { models: p.models, baseUrl: p.baseUrl, source: "workspace", enabled: hasKey ? p.enabled : false, hasKey } - } - } - // Overlay personal providers (they take precedence) - let active = wCfg.default ?? "" const userId = c.get("userId") as string - try { - const pCfg = await loadPersonalConfig(userId) - for (const [name, p] of Object.entries(pCfg.providers)) { - const hasKey = typeof p.apiKey === "string" && p.apiKey.length > 0 - // Only overlay if the user actually configured this provider (has a key). - // Template/preset providers without a key should not shadow workspace config. - if (hasKey) { - providers[name] = { models: p.models, baseUrl: p.baseUrl, source: "personal", enabled: p.enabled !== false, hasKey } - } - } - active = pCfg.default || active - } catch {} - return c.json({ providers, default: active }) + return c.json(await getProvidersResponse(userId)) }) // Test a provider + model connection by making a minimal Messages API call. diff --git a/server/test/provider-settings-fixes.test.ts b/server/test/provider-settings-fixes.test.ts new file mode 100644 index 00000000..e2d4c320 --- /dev/null +++ b/server/test/provider-settings-fixes.test.ts @@ -0,0 +1,157 @@ +/** + * Regression coverage for provider settings correctness fixes. + */ +import { test, expect, describe, afterAll, beforeEach } from "bun:test" +import { mkdir, rm, writeFile } from "node:fs/promises" +import { join } from "node:path" + +const ownsHome = process.env.LOOPAT_HOME === undefined +if (ownsHome) process.env.LOOPAT_HOME = `/tmp/loopat-provider-settings-${process.pid}` + +const HOME = process.env.LOOPAT_HOME! +await mkdir(HOME, { recursive: true }) +if (ownsHome) { + await rm(HOME, { recursive: true, force: true }) + await mkdir(HOME, { recursive: true }) + await writeFile(join(HOME, "config.json"), JSON.stringify({ + knowledge: { git: "" }, + notes: { git: "" }, + repos: [], + providers: {}, + })) +} + +const { + loadConfig, + saveWorkspaceConfig, + getProvidersResponse, + clearPersonalCache, +} = await import("../src/config") +const { + personalLoopatDir, + personalLoopatConfigPath, + personalVaultEnvsDir, + personalVaultEnvPath, +} = await import("../src/paths") + +const USER = "providerfix" + +async function writePersonalProviders(providers: Record): Promise { + await mkdir(personalLoopatDir(USER), { recursive: true }) + await mkdir(personalVaultEnvsDir(USER, "default"), { recursive: true }) + await writeFile(personalLoopatConfigPath(USER), JSON.stringify({ providers }, null, 2) + "\n") + clearPersonalCache(USER) +} + +beforeEach(async () => { + await rm(personalLoopatDir(USER), { recursive: true, force: true }) + clearPersonalCache(USER) +}) + +afterAll(async () => { + if (ownsHome) await rm(HOME, { recursive: true, force: true }) +}) + +describe("saveWorkspaceConfig provider replacement", () => { + test("replaces submitted provider map while preserving unrelated config", async () => { + await saveWorkspaceConfig({ + knowledge: { git: "git@example.com:team/knowledge.git" }, + serveDomain: "serve.example.com", + providers: { + keep: { + baseUrl: "https://keep.example", + apiKey: "sk-keep", + models: [{ id: "keep-model" }], + enabled: true, + }, + remove: { + baseUrl: "https://remove.example", + apiKey: "sk-remove", + models: [{ id: "remove-model" }], + enabled: true, + }, + }, + default: "keep", + }) + + await saveWorkspaceConfig({ + providers: { + keep: { + baseUrl: "https://keep-new.example", + models: [{ id: "keep-model" }], + enabled: false, + }, + }, + }) + + const cfg = await loadConfig() + expect(Object.keys(cfg.providers ?? {})).toEqual(["keep"]) + expect(cfg.providers?.keep.apiKey).toBe("sk-keep") + expect(cfg.providers?.keep.baseUrl).toBe("https://keep-new.example") + expect(cfg.providers?.keep.enabled).toBe(false) + expect(cfg.knowledge?.git).toBe("git@example.com:team/knowledge.git") + expect(cfg.serveDomain).toBe("serve.example.com") + }) +}) + +describe("provider response availability", () => { + test("includes unresolved personal ${VAR} providers as disabled with missing var metadata", async () => { + await saveWorkspaceConfig({ + knowledge: { git: "" }, + providers: {}, + default: "", + }) + await writePersonalProviders({ + default: "ghost", + ghost: { + baseUrl: "https://ghost.example", + apiKey: "${GHOST_API_KEY}", + models: [{ id: "ghost-model" }], + }, + }) + + const j = await getProvidersResponse(USER) + + expect(j.providers.ghost).toMatchObject({ + baseUrl: "https://ghost.example", + source: "personal", + enabled: false, + hasKey: false, + missingVar: "GHOST_API_KEY", + }) + expect(j.providers.ghost.models).toEqual([{ id: "ghost-model" }]) + }) + + test("defaults to keyed personal provider when personal has no explicit default", async () => { + await saveWorkspaceConfig({ + knowledge: { git: "" }, + providers: { + workspace: { + baseUrl: "https://workspace.example", + apiKey: "sk-workspace", + models: [{ id: "workspace-model" }], + enabled: true, + }, + }, + default: "workspace", + }) + await writePersonalProviders({ + personal: { + baseUrl: "https://personal.example", + apiKey: "${PERSONAL_API_KEY}", + models: [{ id: "personal-model" }], + }, + }) + await writeFile(personalVaultEnvPath(USER, "default", "PERSONAL_API_KEY"), "sk-personal\n") + clearPersonalCache(USER) + + const j = await getProvidersResponse(USER) + + expect(j.default).toBe("personal") + expect(j.providers.personal).toMatchObject({ + source: "personal", + enabled: true, + hasKey: true, + }) + }) +}) diff --git a/web/src/api.ts b/web/src/api.ts index e006387e..e9c6f562 100644 --- a/web/src/api.ts +++ b/web/src/api.ts @@ -1085,7 +1085,7 @@ export async function listTopics(): Promise { } export type ModelEntry = { id: string; maxContextTokens?: number } -export type ProviderInfo = { model?: string; models: ModelEntry[]; baseUrl: string; source: "personal" | "workspace"; enabled: boolean; hasKey: boolean } +export type ProviderInfo = { model?: string; models: ModelEntry[]; baseUrl: string; source: "personal" | "workspace"; enabled: boolean; hasKey: boolean; missingVar?: string } export type ProvidersResponse = { providers: Record; default: string } export async function getProviders(): Promise { const r = await apiFetch("/api/providers") diff --git a/web/src/components/chat/ModelSelector.tsx b/web/src/components/chat/ModelSelector.tsx index cb8e8fe7..9bb2cfd1 100644 --- a/web/src/components/chat/ModelSelector.tsx +++ b/web/src/components/chat/ModelSelector.tsx @@ -80,6 +80,11 @@ export default function ModelSelector() { return map; }, [flatModels]); + const missingKeyProviders = useMemo(() => { + if (!providers) return []; + return Object.entries(providers.providers).filter(([, info]) => !info.hasKey && !!info.missingVar); + }, [providers]); + const isClaudeModel = (model: string) => model.toLowerCase().startsWith("claude"); const onPick = useCallback(async (item: FlatModel) => { setOpen(false); @@ -226,6 +231,17 @@ export default function ModelSelector() { )} + {missingKeyProviders.length > 0 && ( +
+ {missingKeyProviders.map(([name, info]) => ( +
+ Missing key: {name} needs{" "} + {info.missingVar} in Settings / AI Providers. +
+ ))} +
+ )} + {/* Footer with shortcuts */}
diff --git a/web/src/components/dialog/AdminDialog.tsx b/web/src/components/dialog/AdminDialog.tsx index 4c1c6ddb..98a22cff 100644 --- a/web/src/components/dialog/AdminDialog.tsx +++ b/web/src/components/dialog/AdminDialog.tsx @@ -372,6 +372,7 @@ export function WorkspacePanel() { const renameProvider = (oldName: string) => { const newName = provRenameValue.trim() if (!newName || newName === oldName || newName === "default") { setEditingProvName(null); return } + const warnReEnterKey = !!(draft?.providers[oldName]?.hasKey && !draft.providers[oldName].keyDirty && !draft.providers[newName]) setDraft((d) => { if (!d || !d.providers[oldName]) return d if (d.providers[newName]) return d @@ -382,8 +383,11 @@ export function WorkspacePanel() { } else if (d.default.startsWith(`${oldName}/`)) { newDefault = newName + d.default.slice(oldName.length) } - return { ...d, default: newDefault, providers: { ...rest, [newName]: prov } } + return { ...d, default: newDefault, providers: { ...rest, [newName]: { ...prov, hasKey: false } } } }) + if (warnReEnterKey) { + setErr(`Renamed "${oldName}" to "${newName}". Re-enter and test the workspace API key before saving; the stored key reference cannot be preserved through this rename.`) + } setEditingProvName(null) } diff --git a/web/src/pages/SettingsPage.tsx b/web/src/pages/SettingsPage.tsx index b1c9c94e..f885c2fd 100644 --- a/web/src/pages/SettingsPage.tsx +++ b/web/src/pages/SettingsPage.tsx @@ -354,6 +354,7 @@ type ProvidersDraft = { enabled: boolean apiKeyNewValue: string apiKeyStored: boolean + apiKeyRef?: string }> } @@ -400,6 +401,7 @@ function ProvidersSection({ disk, refExists, onChanged, disabled }: { enabled: p.enabled !== false, apiKeyNewValue: "", apiKeyStored: !!refInfo?.exists, + apiKeyRef: p.apiKey, } } } @@ -539,7 +541,9 @@ function ProvidersSection({ disk, refExists, onChanged, disabled }: { })) providersOut[name] = { baseUrl: p.baseUrl, - apiKey: `\${${providerEnvVarName(name)}}`, + apiKey: p.apiKeyNewValue.trim() + ? `\${${providerEnvVarName(name)}}` + : (p.apiKeyRef ?? `\${${providerEnvVarName(name)}}`), ...(models.length > 0 ? { models } : {}), ...(p.enabled ? {} : { enabled: false }), } @@ -1083,4 +1087,3 @@ function ApiTokensSection() {
) } -