diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e9cee483..7416b255 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -194,7 +194,7 @@ jobs: mock_pid=$! trap 'kill "$mock_pid" 2>/dev/null || true' EXIT printf '%s\n' \ - '{"model": "deepseek/deepseek-v4-flash", "provider": {"deepseek": {"credential": "deepseek", "enabled": true, "base_url": "http://127.0.0.1:8801"}}}' \ + '{"model": "deepseek/deepseek-v4-flash", "provider": {"deepseek": {"credential": "deepseek", "enabled": true, "base_url": "http://127.0.0.1:8801", "wire_api": "openai_chat_completions"}}}' \ > "$home/providers.json" printf '%s\n' '{"deepseek": {"type": "api_key", "key": "ci-mock-key"}}' > "$home/auth.json" chmod 600 "$home/auth.json" diff --git a/apps/desktop/packages/devo-ai-sdk/src/v2/client.ts b/apps/desktop/packages/devo-ai-sdk/src/v2/client.ts index bbf9d81b..a64b77d7 100644 --- a/apps/desktop/packages/devo-ai-sdk/src/v2/client.ts +++ b/apps/desktop/packages/devo-ai-sdk/src/v2/client.ts @@ -1486,9 +1486,7 @@ class NativeClient { }, discover: async (params: ProviderDiscoverParams): Promise => { const result = (await this.requestCanonical("provider/discover", params)) as ProviderDiscoverResult - // Discover mutates the connection model directory; composer selectors - // read model/preferences which must not keep a pre-discover snapshot. - this.invalidateConfigOptionCaches() + // Discovery is read-only. Callers may explicitly save selected results. return result }, auth: async () => ({ data: [] }), diff --git a/apps/desktop/src/main/ipc-handlers-oauth.test.ts b/apps/desktop/src/main/ipc-handlers-oauth.test.ts index 77b6a286..7f564691 100644 --- a/apps/desktop/src/main/ipc-handlers-oauth.test.ts +++ b/apps/desktop/src/main/ipc-handlers-oauth.test.ts @@ -25,6 +25,7 @@ const logins: Array> = [] const writes: Array<{ provider: string access: string + configureConnection?: boolean gate: Deferred }> = [] let stored: Record = {} @@ -90,12 +91,12 @@ mock.module("./provider-oauth", () => ({ mock.module("./devo-manager", () => ({ requestNative: ( method: string, - params: { provider: string; access: string }, + params: { provider: string; access: string; configureConnection?: boolean }, ) => { if (method !== "credential/set") throw new Error(`Unexpected RPC: ${method}`) const gate = deferred() - writes.push({ provider: params.provider, access: params.access, gate }) + writes.push({ provider: params.provider, access: params.access, configureConnection: params.configureConnection, gate }) return gate.promise.then(() => { stored[params.provider] = params.access }) @@ -220,6 +221,17 @@ describe("Desktop window recreation", () => { }) describe("Desktop OAuth credential cancellation", () => { + test("explicit OAuth login configures a connection only after credentials arrive", async () => { + const pending = login(1, "openai-codex") + expect(writes).toHaveLength(0) + logins[0].resolve({ access: "new" }) + await flush() + expect(writes.map(({ provider, configureConnection }) => ({ provider, configureConnection }))) + .toEqual([{ provider: "openai-codex", configureConnection: true }]) + writes[0].gate.resolve() + await pending + }) + test("a late OAuth response after cancel cannot start credential/set", async () => { const pending = login(1) expect(cancel(1)).toBe(true) diff --git a/apps/desktop/src/main/ipc-handlers.ts b/apps/desktop/src/main/ipc-handlers.ts index c3db41ce..93bea37f 100644 --- a/apps/desktop/src/main/ipc-handlers.ts +++ b/apps/desktop/src/main/ipc-handlers.ts @@ -122,7 +122,7 @@ async function persistDesktopOAuthCredential( onSaving() await requestNative( "credential/set", - credentialSetParamsFromDesktopOAuth(providerId, credential), + { ...credentialSetParamsFromDesktopOAuth(providerId, credential), configureConnection: true }, ) } finally { release() diff --git a/apps/desktop/src/renderer/components/onboarding/steps/provider-setup-step.test.tsx b/apps/desktop/src/renderer/components/onboarding/steps/provider-setup-step.test.tsx index ad009874..58560dbc 100644 --- a/apps/desktop/src/renderer/components/onboarding/steps/provider-setup-step.test.tsx +++ b/apps/desktop/src/renderer/components/onboarding/steps/provider-setup-step.test.tsx @@ -139,8 +139,8 @@ async function render() { ), ) } -async function click(text: string) { - const button = [...mount.querySelectorAll("button")].find((button) => +async function click(text: string, selector = "button") { + const button = [...mount.querySelectorAll(selector)].find((button) => button.textContent?.includes(text), ) if (!button) throw new Error(`Missing button ${text}`) @@ -217,7 +217,7 @@ test("onboarding reuses API and OAuth dialogs and catalog refetches preserve OAu await render() expect(oauthProps.provider).toBe(original) await act(async () => oauthProps.onClose()) - await click("ChatGPT") + await click("ChatGPT", '[aria-label="Provider catalog"] button') expect(mount.textContent).toContain("Manage ChatGPT") }) diff --git a/apps/desktop/src/renderer/components/onboarding/steps/provider-setup-step.tsx b/apps/desktop/src/renderer/components/onboarding/steps/provider-setup-step.tsx index 874aa77d..2db00d17 100644 --- a/apps/desktop/src/renderer/components/onboarding/steps/provider-setup-step.tsx +++ b/apps/desktop/src/renderer/components/onboarding/steps/provider-setup-step.tsx @@ -14,6 +14,7 @@ import { invalidateProviderDependentQueries } from "../../../lib/invalidate-prov import { compareConnectedFirst } from "../../../lib/providers" import { ConnectProviderDialog } from "../../settings/connect-provider-dialog" import { ConnectionDetailDialog } from "../../settings/connection-detail-dialog" +import { DesktopOAuthActions } from "../../settings/desktop-oauth-actions" import { isDesktopOAuthProvider } from "../../settings/desktop-oauth-providers" import { ProviderIcon } from "../../settings/provider-icon" import { TemplateConnectDialog } from "../../settings/template-connect-dialog" @@ -101,6 +102,7 @@ export function ProviderSetupStep({ Choose from the live catalog or search for a model you want to use.

+
({ })) mock.module("./provider-icon", () => ({ ProviderIcon: () => null })) mock.module("./model-edit-dialog", () => ({ ModelEditDialog: () => null })) -const discover = mock(async (_params: unknown) => {}) +const discoveredModels = { fresh: { name: "Fresh model", wireApi: "openai_responses" as const, baseUrl: "https://gateway.example/v1", requestModel: "upstream", cost: { input: 2, output: 3 }, capabilities: { vision: true }, contextWindow: 100000 } } +const discover = mock(async (_params: unknown) => ({ providerId: "openai-codex", models: discoveredModels })) +const upsert = mock(async (_params: unknown) => ({})) const modelRemove = mock(async (_params: unknown) => {}) const refreshCatalog = mock(async (_params: unknown) => ({ data: { status: "updated" }, })) mock.module("../../services/connection-manager", () => ({ getBaseClient: () => ({ - provider: { discover, modelRemove }, + provider: { discover, modelRemove, upsert }, model: { refreshCatalog }, }), })) @@ -79,6 +81,8 @@ afterEach(async () => { await act(async () => root?.unmount()) mount?.remove() discover.mockReset() + discover.mockImplementation(async () => ({ providerId: "openai-codex", models: discoveredModels })) + upsert.mockReset() modelRemove.mockReset() refreshCatalog.mockReset() changed.mockReset() @@ -115,12 +119,12 @@ describe("connected provider details", () => { changed: 1, }) }) - test("Codex refreshes its authenticated provider catalog", async () => { + test("Codex discovery displays temporary models without changing configuration", async () => { refreshCatalog.mockImplementation(async () => ({ data: { status: "updated" }, })) await renderDialog() - await click("Refresh catalog") + await click("Discover") expect({ refresh: refreshCatalog.mock.calls, discover: discover.mock.calls, @@ -128,14 +132,16 @@ describe("connected provider details", () => { }).toEqual({ refresh: [], discover: [[{ providerId: "openai-codex", forceRefresh: true }]], - changed: 1, + changed: 0, }) + expect(mount.textContent).toContain("Fresh model") + expect(upsert.mock.calls).toEqual([]) }) test("failed discovery is visible and can be retried", async () => { discover.mockImplementationOnce(async () => { throw new Error("Provider unavailable") }) - discover.mockImplementation(async () => {}) + discover.mockImplementation(async () => ({ providerId: "settings-qa", models: discoveredModels })) await renderDialog("settings-qa") await click("Discover") expect(mount.querySelector('[role="alert"]')?.textContent).toBe( @@ -146,6 +152,17 @@ describe("connected provider details", () => { alert: mount.querySelector('[role="alert"]'), attempts: discover.mock.calls.length, changed: changed.mock.calls.length, - }).toEqual({ alert: null, attempts: 2, changed: 1 }) + }).toEqual({ alert: null, attempts: 2, changed: 0 }) + }) + test("saving a discovered model persists only the explicit selection and preserves routing", async () => { + await renderDialog() + await click("Discover") + await click("Save") + expect(upsert.mock.calls).toEqual([[{ provider: { + ...provider, + models: { fresh: { origin: "user", baseUrl: "https://gateway.example/v1", wireApi: "openai_responses", requestModel: "upstream" } }, + } }]]) + expect(changed.mock.calls.length).toBe(1) }) + }) diff --git a/apps/desktop/src/renderer/components/settings/connection-detail-dialog.tsx b/apps/desktop/src/renderer/components/settings/connection-detail-dialog.tsx index 7618b802..f5f68a35 100644 --- a/apps/desktop/src/renderer/components/settings/connection-detail-dialog.tsx +++ b/apps/desktop/src/renderer/components/settings/connection-detail-dialog.tsx @@ -22,7 +22,7 @@ import { import { Input } from "@devo/ui/components/input" import { Spinner } from "@devo/ui/components/spinner" import { PlusIcon, RefreshCwIcon, SearchIcon, Trash2Icon } from "lucide-react" -import { useCallback, useMemo, useState } from "react" +import { useCallback, useEffect, useMemo, useRef, useState } from "react" import { getBaseClient } from "../../services/connection-manager" import { effectiveContextWindowTokens, @@ -51,10 +51,20 @@ export function ConnectionDetailDialog({ const [removingModel, setRemovingModel] = useState(null) const [addingModel, setAddingModel] = useState(false) const [error, setError] = useState(null) - const catalogRefresh = provider.id === "openai-codex" + const [discovered, setDiscovered] = useState>({}) + const [savingModel, setSavingModel] = useState(null) + const discoveryVersion = useRef(0) + useEffect(() => { + discoveryVersion.current++ + setDiscovering(false) + setDiscovered({}) + setSearch("") + setError(null) + return () => { discoveryVersion.current++ } + }, [provider.id, open]) const models = useMemo(() => { - const entries = Object.entries({ ...provider.models, ...connectionModels }) + const entries = Object.entries({ ...provider.models, ...discovered, ...connectionModels }) if (!search.trim()) return entries const lower = search.toLowerCase() return entries.filter( @@ -62,27 +72,51 @@ export function ConnectionDetailDialog({ id.toLowerCase().includes(lower) || (m.name ?? "").toLowerCase().includes(lower), ) - }, [provider.models, connectionModels, search]) + }, [provider.models, discovered, connectionModels, search]) const handleDiscover = useCallback(async () => { + const version = ++discoveryVersion.current setDiscovering(true) setError(null) try { const client = getBaseClient() if (!client) throw new Error("Not connected to server") - await client.provider.discover({ + const result = await client.provider.discover({ providerId: provider.id, forceRefresh: true, }) - onChanged() + if (version === discoveryVersion.current) setDiscovered(result.models) } catch (error) { + if (version !== discoveryVersion.current) return setError( error instanceof Error ? error.message : "Failed to discover models", ) } finally { - setDiscovering(false) + if (version === discoveryVersion.current) setDiscovering(false) } - }, [provider.id, onChanged]) + }, [provider.id]) + + const handleSaveModel = useCallback(async (modelId: string, model: CatalogModelInfo) => { + setSavingModel(modelId) + setError(null) + try { + const client = getBaseClient() + if (!client) throw new Error("Not connected to server") + await client.provider.upsert({ + provider: { ...provider, models: { [modelId]: { + origin: "user", + baseUrl: model.baseUrl, + wireApi: model.wireApi, + requestModel: model.requestModel, + } } }, + }) + onChanged() + } catch (error) { + setError(error instanceof Error ? error.message : "Failed to save model") + } finally { + setSavingModel(null) + } + }, [provider, onChanged]) const handleRemoveModel = useCallback( async (modelId: string) => { @@ -118,15 +152,15 @@ export function ConnectionDetailDialog({ Connected
- {Object.keys({ ...provider.models, ...connectionModels }).length}{" "} + {Object.keys({ ...provider.models, ...discovered, ...connectionModels }).length}{" "} model - {Object.keys({ ...provider.models, ...connectionModels }) + {Object.keys({ ...provider.models, ...discovered, ...connectionModels }) .length !== 1 ? "s" : ""}{" "} available. {provider.baseUrl ? ` Endpoint: ${provider.baseUrl}` : ""} - {" Catalog models cannot be removed."} + {" Discovery is temporary. Save a model to keep it in your configuration."} @@ -152,7 +186,7 @@ export function ConnectionDetailDialog({ ) : ( )} - {catalogRefresh ? "Refresh catalog" : "Discover"} + Discover + )} {connectionModels[modelId]?.origin === "user" && provider.models?.[modelId]?.origin !== "remote" && ( })) +mock.module("@devo/ui/components/input", () => ({ Input: (props: any) => })) +mock.module("@devo/ui/components/spinner", () => ({ Spinner: () => null })) +mock.module("@devo/ui/components/skeleton", () => ({ Skeleton: () => null })) +mock.module("@devo/ui/components/dialog", () => ({ Dialog: () => null, DialogContent: wrap, DialogDescription: wrap, DialogFooter: wrap, DialogHeader: wrap, DialogTitle: wrap })) +let catalog: any = { providers: [], connectedIds: new Set(), templateIds: new Set(), connectionModels: {} } +let error: string | null = null +let loading = false +mock.module("../../hooks/use-devo-data", () => ({ useProviderCatalog: () => ({ data: catalog, loading, error, reload: () => {} }) })) +mock.module("../../hooks/use-model-directory", () => ({ useModelDirectory: () => ({ refreshCatalog: () => {}, refreshing: false }) })) +mock.module("../../hooks/use-server", () => ({ useServerConnection: () => ({ connected: true }) })) +mock.module("../../lib/invalidate-provider-queries", () => ({ invalidateProviderDependentQueries: writes })) +mock.module("../../services/connection-manager", () => ({ getBaseClient: () => ({ provider: { disconnect: writes } }) })) +mock.module("./provider-icon", () => ({ ProviderIcon: () => null })) +mock.module("./connect-provider-dialog", () => ({ ConnectProviderDialog: ({ provider }: any) =>
OAuth login
})) +mock.module("./connection-detail-dialog", () => ({ ConnectionDetailDialog: () =>
Discover and save
})) +mock.module("./template-connect-dialog", () => ({ TemplateConnectDialog: () => null })) +mock.module("./custom-provider-dialog", () => ({ CustomProviderDialog: () => null })) +const { ProviderSettings } = await import("./provider-settings") +const { ProviderSetupStep } = await import("../onboarding/steps/provider-setup-step") +const { createRoot } = await import("react-dom/client") +let root: Root | undefined +let mount: HTMLElement +async function render(node: ReactNode) { + mount = document.createElement("div"); document.body.append(mount); root = createRoot(mount) + await act(async () => root?.render(node)) +} +afterEach(async () => { + await act(async () => root?.unmount()); mount?.remove() + catalog = { providers: [], connectedIds: new Set(), templateIds: new Set(), connectionModels: {} } + error = null; loading = false; writes.mockClear(); network.mockClear() +}) +for (const surface of ["settings", "onboarding"] as const) { + test(`${surface}: empty directory retains all login actions without render side effects`, async () => { + await render(surface === "settings" ? : {}} onSkip={() => {}} />) + const section = mount.querySelector('[aria-label="Account sign-in"]')! + expect([...section.querySelectorAll("button")].map(button => button.textContent)).toEqual([ + "Sign in with ChatGPT", "Sign in with Claude Pro/Max", "Sign in with GitHub Copilot", "Sign in with xAI", + ]) + expect({ writes: writes.mock.calls, network: network.mock.calls, providers: catalog.providers }).toEqual({ writes: [], network: [], providers: [] }) + expect(mount.querySelector('[role="dialog"]')).toBeNull() + await act(async () => (section.querySelector("button") as HTMLElement).click()) + const dialog = mount.querySelector('[role="dialog"]')! + expect({ id: dialog.getAttribute("data-provider"), models: dialog.getAttribute("data-models"), providers: catalog.providers }).toEqual({ id: "openai-codex", models: "{}", providers: [] }) + }) +} +test("Settings retains login when the directory fails", async () => { + error = "Offline" + await render() + await act(async () => (mount.querySelector('[aria-label="Account sign-in"] button') as HTMLElement).click()) + expect(mount.querySelector('[role="dialog"]')?.getAttribute("data-provider")).toBe("openai-codex") +}) +test("a saved OAuth-only connection absent from the remote directory can open discovery", async () => { + catalog = { providers: [{ id: "openai-codex", name: "My Codex", models: {}, enabled: true, wireApis: [] }], + connectedIds: new Set(["openai-codex"]), templateIds: new Set(), connectionModels: {} } + await render() + const button = [...mount.querySelectorAll("button")].find(button => button.textContent?.includes("My Codex"))! + await act(async () => button.click()) + expect(mount.querySelector('[role="dialog"]')?.textContent).toBe("Discover and save") + expect(writes.mock.calls).toEqual([]) +}) + +for (const outcome of ["loaded", "error"] as const) { + test(`Settings keeps an active login through loading → ${outcome}`, async () => { + loading = true + await render() + await act(async () => (mount.querySelector('[aria-label="Account sign-in"] button') as HTMLElement).click()) + const dialog = mount.querySelector('[role="dialog"]')! + expect(dialog.getAttribute("data-provider")).toBe("openai-codex") + loading = false + error = outcome === "error" ? "Offline" : null + await act(async () => root?.render()) + expect(mount.querySelector('[role="dialog"]')).toBe(dialog) + expect(dialog.getAttribute("data-models")).toBe("{}") + }) +} diff --git a/apps/desktop/src/renderer/components/settings/desktop-oauth-actions.tsx b/apps/desktop/src/renderer/components/settings/desktop-oauth-actions.tsx new file mode 100644 index 00000000..55d16b23 --- /dev/null +++ b/apps/desktop/src/renderer/components/settings/desktop-oauth-actions.tsx @@ -0,0 +1,38 @@ +import { Button } from "@devo/ui/components/button" +import { useState } from "react" +import type { CatalogProvider } from "../../hooks/use-devo-data" +import { ConnectProviderDialog } from "./connect-provider-dialog" +import { desktopOAuthActions } from "./desktop-oauth-providers" + +/** Account login adapters stay available independently of directory availability. */ +export function DesktopOAuthActions({ onConnected }: { onConnected: () => void }) { + const [provider, setProvider] = useState(null) + return ( +
+
+

Account sign-in

+

+ Connect an existing subscription. Available models come from the live directory or your configuration. +

+
+
+ {desktopOAuthActions.map(({ providerId, label }) => ( + + ))} +
+ {provider && setProvider(null)} + onConnected={() => { setProvider(null); onConnected() }} />} +
+ ) +} diff --git a/apps/desktop/src/renderer/components/settings/desktop-oauth-providers.ts b/apps/desktop/src/renderer/components/settings/desktop-oauth-providers.ts index c819a10a..476264e8 100644 --- a/apps/desktop/src/renderer/components/settings/desktop-oauth-providers.ts +++ b/apps/desktop/src/renderer/components/settings/desktop-oauth-providers.ts @@ -33,3 +33,10 @@ export function resolveProviderAuthMethods( ): ProviderAuthMethod[] { return DESKTOP_OAUTH_METHODS[providerId] ?? pluginMethods ?? [{ type: "api", label: "API Key" }] } + +/** Retained adapter actions, not entries in the live model directory. */ +export const desktopOAuthActions = Object.entries(DESKTOP_OAUTH_METHODS).flatMap( + ([providerId, methods]) => methods + .filter((method) => method.type === "oauth") + .map((method) => ({ providerId, label: method.label })), +) diff --git a/apps/desktop/src/renderer/components/settings/model-edit-dialog.test.tsx b/apps/desktop/src/renderer/components/settings/model-edit-dialog.test.tsx new file mode 100644 index 00000000..b5b164c1 --- /dev/null +++ b/apps/desktop/src/renderer/components/settings/model-edit-dialog.test.tsx @@ -0,0 +1,108 @@ +import { afterEach, expect, mock, test } from "bun:test" +import type { CatalogProviderInfo } from "@devo-ai/sdk/v2/client" +import { Window } from "happy-dom" +import { act, type ReactNode } from "react" +import type { Root } from "react-dom/client" + +const dom = new Window({ url: "http://localhost" }) +Object.assign(globalThis, { + window: dom, document: dom.document, navigator: dom.navigator, + Node: dom.Node, Element: dom.Element, HTMLElement: dom.HTMLElement, + IS_REACT_ACT_ENVIRONMENT: true, +}) +const wrap = ({ children }: { children?: ReactNode }) =>
{children}
+mock.module("@devo/ui/components/dialog", () => ({ + Dialog: wrap, DialogContent: wrap, DialogDescription: wrap, + DialogHeader: wrap, DialogTitle: wrap, +})) +mock.module("@devo/ui/components/select", () => ({ + Select: wrap, SelectContent: wrap, SelectGroup: wrap, + SelectItem: wrap, SelectTrigger: wrap, SelectValue: () => null, +})) +mock.module("@devo/ui/components/switch", () => ({ + Switch: ({ checked, onCheckedChange, disabled, "aria-label": label }: { + checked: boolean; onCheckedChange: (value: boolean) => void + disabled?: boolean; "aria-label"?: string + }) => , +})) +const upsert = mock(async (_params: unknown) => ({})) +const remove = mock(async (_params: unknown) => ({})) +mock.module("../../services/connection-manager", () => ({ + getBaseClient: () => ({ provider: { upsert, modelRemove: remove } }), +})) +const { createRoot } = await import("react-dom/client") +const { ModelEditDialog } = await import("./model-edit-dialog") +const provider: CatalogProviderInfo = { + id: "gateway", name: "Gateway", enabled: true, + baseUrl: "https://gateway.example/v1", wireApis: ["openai_chat_completions"], + modelOverrides: { untouched: { name: "Existing user title" } }, + models: { + remote: { name: "Remote model", origin: "remote", enabled: true, + baseUrl: "https://model.example/v1", wireApi: "openai_responses", + requestModel: "upstream-id", contextWindow: 100000 }, + untouched: { name: "Other catalog model", origin: "remote" }, + }, +} +let root: Root | undefined +let mount: HTMLElement +const saved = mock(() => {}) +const close = mock((_open: boolean) => {}) +afterEach(async () => { + await act(async () => root?.unmount()) + mount?.remove() + upsert.mockClear(); remove.mockClear(); saved.mockClear(); close.mockClear() +}) +async function renderEditor() { + mount = document.createElement("div") + document.body.append(mount) + root = createRoot(mount) + await act(async () => root?.render()) +} +async function click(text: string) { + const button = [...mount.querySelectorAll("button")].find(button => button.textContent === text) + if (!button) throw new Error(`Missing button: ${text}`) + await act(async () => button.click()) +} +test("saving a remote model sends only changed metadata and pins its route", async () => { + const original = structuredClone(provider) + await renderEditor() + await click("Enabled") + const name = mount.querySelector("#model-name")! + await act(async () => { + Object.getOwnPropertyDescriptor(window.HTMLInputElement.prototype, "value")!.set!.call(name, "My remote") + name.dispatchEvent(new window.Event("input", { bubbles: true })) + }) + await click("Save") + expect(upsert.mock.calls).toEqual([[{ provider: expect.objectContaining({ + id: "gateway", wireApis: ["openai_chat_completions", "openai_responses"], + modelOverrides: { remote: { name: "My remote", enabled: false } }, + models: { remote: { + origin: "remote", baseUrl: "https://model.example/v1", + wireApi: "openai_responses", requestModel: "upstream-id", + } }, + }) }]]) + expect({ removed: remove.mock.calls, saved: saved.mock.calls, closed: close.mock.calls }) + .toEqual({ removed: [], saved: [[]], closed: [[false]] }) + expect(provider).toEqual(original) +}) +test("opening and cancelling a catalog model does not persist any catalog entries", async () => { + await renderEditor() + await click("Cancel") + expect({ upserts: upsert.mock.calls, removed: remove.mock.calls, saved: saved.mock.calls }) + .toEqual({ upserts: [], removed: [], saved: [] }) +}) + +test("saving unchanged remote controls does not freeze inherited metadata", async () => { + await renderEditor() + await click("Save") + expect(upsert.mock.calls).toEqual([[{ provider: expect.objectContaining({ + modelOverrides: {}, + models: { remote: { + origin: "remote", baseUrl: "https://model.example/v1", + wireApi: "openai_responses", requestModel: "upstream-id", + } }, + }) }]]) +}) diff --git a/apps/desktop/src/renderer/components/settings/model-edit-dialog.tsx b/apps/desktop/src/renderer/components/settings/model-edit-dialog.tsx index 47c410b3..b5fc4add 100644 --- a/apps/desktop/src/renderer/components/settings/model-edit-dialog.tsx +++ b/apps/desktop/src/renderer/components/settings/model-edit-dialog.tsx @@ -39,7 +39,7 @@ import { Switch } from "@devo/ui/components/switch" import { Textarea } from "@devo/ui/components/textarea" import { cn } from "@devo/ui/lib/utils" import { ChevronDownIcon, PlusIcon, Trash2Icon } from "lucide-react" -import { useCallback, useId, useMemo, useState } from "react" +import { useCallback, useId, useMemo, useRef, useState } from "react" import { getBaseClient } from "../../services/connection-manager" import { effectiveContextWindowTokens, contextWindowPercentFromAbsolute } from "../../lib/providers" @@ -230,7 +230,7 @@ export function ModelEditDialog({ const [modelId, setModelId] = useState(originalModelId) const [displayName, setDisplayName] = useState(model.name ?? "") const [wireApi, setWireApi] = useState( - isCreate ? providerDefaultWire : effectiveWireApi(model, provider), + effectiveWireApi(model, provider), ) const [enabled, setEnabled] = useState(model.enabled !== false) const [advancedOpen, setAdvancedOpen] = useState(false) @@ -272,6 +272,19 @@ export function ModelEditDialog({ }, ) + // Compare editor controls, not normalized output, so untouched defaults do + // not become user overrides merely because the dialog was opened. + const metadataDraft = { + name: displayName, enabled, contextWindow, + effectiveContextWindowPercent: contextWindow, + maxTokens, temperature, topP, topK, inputModalities: modalities, + headers, request: requestBody, options: optionsBody, + reasoningCapability: [reasoningMode, reasoningLevels], + defaultReasoningSelection, defaultReasoningEffort: defaultReasoningSelection, + variants: effortEncodings, + } + const initialMetadataDraft = useRef(metadataDraft) + const [saving, setSaving] = useState(false) const [error, setError] = useState(null) @@ -445,8 +458,24 @@ export function ModelEditDialog({ ? provider.wireApis : [...provider.wireApis, wireApi] + const editingRemote = !isCreate && model.origin === "remote" + const editedMetadata: CatalogModelInfo = Object.fromEntries( + Object.entries(metadataDraft) + .filter(([key, value]) => + JSON.stringify(value) !== JSON.stringify( + initialMetadataDraft.current[key as keyof typeof metadataDraft], + ) && JSON.stringify(nextModel[key as keyof CatalogModelInfo]) !== + JSON.stringify(model[key as keyof CatalogModelInfo]), + ) + .map(([key]) => [key, nextModel[key as keyof CatalogModelInfo]]), + ) const updatedModels: Record = { - [nextId]: nextModel, + [nextId]: editingRemote ? { + origin: "remote", + baseUrl: nextModel.baseUrl, + wireApi: nextModel.wireApi, + requestModel: nextModel.requestModel, + } : nextModel, } await client.provider.upsert({ @@ -456,6 +485,9 @@ export function ModelEditDialog({ // Server merges models by insert; send only this model so we // do not copy the full template+connection catalog into the overlay. models: updatedModels, + modelOverrides: editingRemote && Object.keys(editedMetadata).length > 0 + ? { [nextId]: editedMetadata } + : {}, }, }) @@ -475,6 +507,7 @@ export function ModelEditDialog({ setSaving(false) } }, [ + metadataDraft, modelId, originalModelId, existingModels, diff --git a/apps/desktop/src/renderer/components/settings/provider-settings.tsx b/apps/desktop/src/renderer/components/settings/provider-settings.tsx index 0b4c41f4..fb966d58 100644 --- a/apps/desktop/src/renderer/components/settings/provider-settings.tsx +++ b/apps/desktop/src/renderer/components/settings/provider-settings.tsx @@ -32,6 +32,7 @@ import { SettingsSection } from "./settings-section" import { settingsBannerClass, settingsPageClass } from "./settings-surface" import { TemplateConnectDialog } from "./template-connect-dialog" import { ConnectProviderDialog } from "./connect-provider-dialog" +import { DesktopOAuthActions } from "./desktop-oauth-actions" import { isDesktopOAuthProvider } from "./desktop-oauth-providers" import { CustomProviderDialog } from "./custom-provider-dialog" import { ConnectionDetailDialog } from "./connection-detail-dialog" @@ -106,13 +107,20 @@ export function ProviderSettings() { }, [handleSaved]) if (loading) { - return + return ( +
+ setCustomDialogOpen(true)} /> + + +
+ ) } if (error) { return (
setCustomDialogOpen(true)} /> +