From c1390e21a5e5764475405c03e3ce57c2512248c2 Mon Sep 17 00:00:00 2001 From: mlsmaycon Date: Mon, 24 Aug 2026 06:47:21 +0000 Subject: [PATCH 01/13] Keep a refused provider save on screen, and its discovered prices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things the provider modal got wrong once the backend started checking a provider's url and credential before storing them. A refused save closed the modal anyway. handleSubmit called handleClose unconditionally, so a rejection threw away the key the operator had just typed — and the API never returns a key, so there was nothing to type over on the way back in. Both paths now stop before closing, which needed updateProvider to report whether it succeeded rather than returning void. Discovered models arrived priced at zero. The merge hardcoded 0/0 for every model the catalog did not already carry by exact id, on the reasoning that the discovery response carried no prices — which stopped being true when the endpoint began returning the same rates the proxy bills with. Bedrock felt all of it: its listing returns geography-prefixed ids that never match a catalog entry by string, so an account's entire model list registered at zero while the API was reporting a rate for each one. Exact-id matching stays. Collapsing a geography-prefixed id onto its catalog entry would hand back the bare form, and only the prefixed one is invocable at AWS. --- src/modules/agent-network/AIProviderModal.tsx | 26 ++++++++++++++----- .../agent-network/AIProvidersProvider.tsx | 9 +++++-- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/modules/agent-network/AIProviderModal.tsx b/src/modules/agent-network/AIProviderModal.tsx index 75f0600ab..7a3d83191 100644 --- a/src/modules/agent-network/AIProviderModal.tsx +++ b/src/modules/agent-network/AIProviderModal.tsx @@ -480,7 +480,7 @@ export default function AIProviderModal({ } : {}; if (isEdit && provider) { - await updateProvider(provider.id, { + const saved = await updateProvider(provider.id, { providerId, name, upstreamUrl, @@ -492,6 +492,11 @@ export default function AIProviderModal({ // Only forward the API key when the user actually rotated it ...(apiKey && apiKey.trim() !== MASKED_API_KEY ? { apiKey } : {}), }); + // The url and credential are checked against the vendor before the + // change is stored, so a save can be refused for a reason the operator + // has to fix here. Closing would throw away the key they just typed — + // and it never comes back from the API to be typed over again. + if (!saved) return; handleClose(); return; } @@ -505,7 +510,7 @@ export default function AIProviderModal({ ); if (!bootstrapped) return; } - await addProvider({ + const created = await addProvider({ providerId, name, upstreamUrl, @@ -517,6 +522,7 @@ export default function AIProviderModal({ models: submittedModels, enabled: true, }); + if (!created) return; handleClose(); }; @@ -567,8 +573,8 @@ export default function AIProviderModal({ // catalog does not already carry. Both the per-row picker and "Add More" // read this one list, so merging here is all the wiring either needs. // - // A catalog entry wins on collision — it carries prices, and the discovery - // response deliberately carries none. + // A catalog entry wins on collision. Both sides price from the same table, + // so the rates agree; the catalog's label is the curated one. const catalogModelOptions = useMemo(() => { const base = catalog?.models ?? []; if (discovered.models.length === 0) return base; @@ -579,8 +585,16 @@ export default function AIProviderModal({ .map((m) => ({ id: m.id, label: m.label || m.id, - input_per_1k: 0, - output_per_1k: 0, + // The rates the response carries, not zeros. A Bedrock listing + // returns geography-prefixed ids that never match a catalog entry by + // string, so every one of them arrives through this branch — zeroing + // here priced a whole provider's models at nothing while the API was + // reporting what each of them costs. + input_per_1k: m.input_per_1k, + output_per_1k: m.output_per_1k, + cached_input_per_1k: m.cached_input_per_1k, + cache_read_per_1k: m.cache_read_per_1k, + cache_creation_per_1k: m.cache_creation_per_1k, pricing_known: m.pricing_known, })); return [...base, ...extra]; diff --git a/src/modules/agent-network/AIProvidersProvider.tsx b/src/modules/agent-network/AIProvidersProvider.tsx index 68fdd14f2..033b9b6fe 100644 --- a/src/modules/agent-network/AIProvidersProvider.tsx +++ b/src/modules/agent-network/AIProvidersProvider.tsx @@ -495,7 +495,10 @@ type AIProvidersContextValue = { closeWizard: () => void; isWizardOpen: boolean; addProvider: (input: ProviderConnectInput) => Promise; - updateProvider: (id: string, updates: ProviderUpdateInput) => Promise; + // Resolves false when the save was refused — the backend checks a provider's + // url and credential before storing them, so a rejected edit must leave the + // form open with what the operator typed still in it. + updateProvider: (id: string, updates: ProviderUpdateInput) => Promise; toggleProvider: (id: string) => Promise; deleteProvider: (id: string) => Promise; addPolicy: ( @@ -687,7 +690,7 @@ export default function AIProvidersProvider({ children }: Readonly) { const updateProvider = useCallback( async (id: string, updates: ProviderUpdateInput) => { const existing = (apiProviders ?? []).find((p) => p.id === id); - if (!existing) return; + if (!existing) return false; const merged: APIProviderRequest = { provider_id: updates.providerId ?? existing.provider_id, name: updates.name ?? existing.name, @@ -719,11 +722,13 @@ export default function AIProvidersProvider({ children }: Readonly) { title: "Provider updated", description: "Settings saved.", }); + return true; } catch (err) { notify({ title: "Failed to update provider", description: err instanceof Error ? err.message : String(err), }); + return false; } }, [apiProviders, providersApi, mutate], From 718b81ca8dd4dc5a607d79ab0e23cb975a43b1f4 Mon Sep 17 00:00:00 2001 From: mlsmaycon Date: Mon, 24 Aug 2026 09:42:27 +0000 Subject: [PATCH 02/13] Save the provider before asking the vendor for its models MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Loading models sent the typed API key to the discovery endpoint while the provider itself was still unsaved. Every way that can go wrong — a key the vendor refuses, an endpoint that does not answer — surfaced against a record that did not exist, so there was nothing for the operator to correct except the fields in front of them, and no saved state to try again from. The provider is now written first, which is where the upstream and the credential are checked, so a bad pair fails on the save with the reason attached. Discovery then asks by record id and the key stays server-side. The modal tracks the record it created so the Save that follows updates it rather than creating a second one, and a reopen clears it — carrying it over would send the next session's edits to the previous session's provider. The consequence worth naming: pressing the button on a new provider creates one, so cancelling afterwards leaves it behind. That is the trade the check asks for, and the button now says it saves. --- src/modules/agent-network/AIProviderModal.tsx | 133 ++++++++++++++---- 1 file changed, 105 insertions(+), 28 deletions(-) diff --git a/src/modules/agent-network/AIProviderModal.tsx b/src/modules/agent-network/AIProviderModal.tsx index 7a2859f00..de7654d4f 100644 --- a/src/modules/agent-network/AIProviderModal.tsx +++ b/src/modules/agent-network/AIProviderModal.tsx @@ -212,6 +212,17 @@ export default function AIProviderModal({ ); const [apiKey, setApiKey] = useState(isEdit ? MASKED_API_KEY : ""); const [bootstrapCluster, setBootstrapCluster] = useState(""); + // Loading models saves the provider first, so a modal opened on "Connect" + // can be addressing a stored record by the time the operator presses Save. + // Everything that needs the record on the server reads targetProvider; + // isEdit keeps meaning "opened on an existing provider", which is what the + // titles and the masked-key state are about. + const [createdProvider, setCreatedProvider] = useState< + AIProvider | undefined + >(); + // The record on the server this modal is working against: the one it was + // opened on, or the one it created in order to load models. + const targetProvider = provider ?? createdProvider; const [models, setModels] = useState(() => (provider?.models ?? []).map(withModelKey), ); @@ -372,6 +383,10 @@ export default function AIProviderModal({ const reset = () => { setTab("provider"); + // A record created to load models belongs to the session that created it. + // Carrying it into the next one would send that session's edits to the + // wrong provider. + setCreatedProvider(undefined); if (isEdit && provider) { setProviderId(provider.providerId); setName(provider.name); @@ -430,23 +445,26 @@ export default function AIProviderModal({ return out; }, [catalog?.extra_headers, extraValues]); - const handleSubmit = async () => { - if (!catalog) return; - // Drop rows the operator never filled in (an added-but-empty custom - // row, or the empty fallback row when the catalog is exhausted) — - // the API rejects models without an id, which would fail the whole - // save over a leftover blank line. Duplicate ids are collapsed to the - // first row too: the catalog dropdown can't offer an id twice, but two - // custom rows can be typed with the same id, and shipping both would - // send an ambiguous price for the model. - const seenModelIds = new Set(); - const submittedModels = models + // Drop rows the operator never filled in (an added-but-empty custom row, or + // the empty fallback row when the catalog is exhausted) — the API rejects + // models without an id, which would fail the whole save over a leftover + // blank line. Duplicate ids are collapsed to the first row too: the catalog + // dropdown can't offer an id twice, but two custom rows can be typed with + // the same id, and shipping both would send an ambiguous price. + const submittableModels = () => { + const seen = new Set(); + return models .map((m) => ({ ...m, id: m.id.trim() })) .filter((m) => { - if (m.id === "" || seenModelIds.has(m.id)) return false; - seenModelIds.add(m.id); + if (m.id === "" || seen.has(m.id)) return false; + seen.add(m.id); return true; }); + }; + + const handleSubmit = async () => { + if (!catalog) return; + const submittedModels = submittableModels(); // Saving an unpriced model is silent and irreversible in effect: every // request against it records $0, and the usage that was already spent @@ -479,8 +497,8 @@ export default function AIProviderModal({ identityHeaderGroups: identityHeaderGroups.trim(), } : {}; - if (isEdit && provider) { - const saved = await updateProvider(provider.id, { + if (targetProvider) { + const saved = await updateProvider(targetProvider.id, { providerId, name, upstreamUrl, @@ -619,10 +637,9 @@ export default function AIProviderModal({ // when the URL differs; canDiscoverModels will block discovery until the // operator provides one. const useSavedCredential = - isEdit && - !!provider?.id && - providerId === provider.providerId && - upstreamUrl === provider.upstreamUrl && + !!targetProvider?.id && + providerId === targetProvider.providerId && + upstreamUrl === targetProvider.upstreamUrl && apiKey.trim() === MASKED_API_KEY; const canDiscoverModels = useMemo(() => { @@ -636,16 +653,70 @@ export default function AIProviderModal({ ); }, [useSavedCredential, upstreamUrl, apiKey]); + // persistForDiscovery stores what is on screen so the listing can be asked + // for by record id. The save is where the upstream and the credential are + // checked against the vendor, so a wrong key fails against the form's own + // fields with the reason attached — rather than the listing failing later + // with the key held only in the browser. + // + // Returns the stored record, or undefined when the save was refused; the + // helpers have already told the operator why. + const persistForDiscovery = async (): Promise => { + const identityOverrides = customizableIdentity + ? { + identityHeaderUserId: identityHeaderUserId.trim(), + identityHeaderGroups: identityHeaderGroups.trim(), + } + : {}; + const common = { + providerId, + name, + upstreamUrl, + extraValues: sanitizedExtraValues, + ...identityOverrides, + skipTlsVerification: isCustomKind ? skipTlsVerification : false, + metadataDisabled, + models: submittableModels(), + }; + + if (targetProvider) { + const saved = await updateProvider(targetProvider.id, { + ...common, + ...(apiKey && apiKey.trim() !== MASKED_API_KEY ? { apiKey } : {}), + }); + return saved ? targetProvider : undefined; + } + + // A provider cannot exist before the account has an endpoint. + if (!settingsBootstrapped) { + const bootstrapped = await bootstrapAgentNetworkSettings( + bootstrapCluster.trim(), + ); + if (!bootstrapped) return undefined; + } + + const created = await addProvider({ ...common, apiKey, enabled: true }); + if (created) setCreatedProvider(created); + return created; + }; + const loadModelsFromProvider = async () => { - const found = await discovered.discover( - useSavedCredential && provider?.id - ? { catalog_provider_id: providerId, provider_id: provider.id } - : { - catalog_provider_id: providerId, - upstream_url: upstreamUrl.trim(), - api_key: apiKey.trim(), - }, - ); + // The form still describes the stored record, so its credential is the one + // to test and there is nothing to write first. + if (useSavedCredential && targetProvider?.id) { + await discovered.discover({ + catalog_provider_id: providerId, + provider_id: targetProvider.id, + }); + return; + } + + const saved = await persistForDiscovery(); + if (!saved) return; + await discovered.discover({ + catalog_provider_id: providerId, + provider_id: saved.id, + }); }; // A discovery result describes one provider, endpoint and credential. Once @@ -1438,6 +1509,12 @@ export default function AIProviderModal({ Enter the endpoint URL and API key first. )} + {canDiscoverModels && !useSavedCredential && ( + + This saves the provider first, so the endpoint and key are + checked before the vendor is asked. + + )} {discovered.notSupported && ( This provider has no model listing endpoint — the catalog From 120e2367cb3de9e89005d6b6df10ae8a8d9ab940 Mon Sep 17 00:00:00 2001 From: mlsmaycon Date: Mon, 24 Aug 2026 15:59:51 +0000 Subject: [PATCH 03/13] Keep a refused provider save on the form, and make it look refused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three things a save that can now be refused exposed. The page came down under the modal. providersApi used the default error handler, which sends anything in 401..500 to the global error boundary — so a 422 naming the field to correct tore down the form holding it. The operator saw a toast and lost the key they had typed. It now handles its own errors, which is what the settings bootstrap already does beside it and for the same reason. Every failure toast was green with a check mark. notify() only turns red through its promise path, which none of these use, so fifteen failures in this file announced themselves as successes. They go through one helper now. Loading models had no feedback while it saved. That save is where the vendor is called, so it is the slow part — a timeout sat there with an idle-looking button, and pressing it again is the obvious response. The button now spins, says which phase it is in, and is disabled along with Save until both finish. --- src/modules/agent-network/AIProviderModal.tsx | 37 ++++++++++--- .../agent-network/AIProvidersProvider.tsx | 53 +++++++++++++------ 2 files changed, 66 insertions(+), 24 deletions(-) diff --git a/src/modules/agent-network/AIProviderModal.tsx b/src/modules/agent-network/AIProviderModal.tsx index de7654d4f..453660a30 100644 --- a/src/modules/agent-network/AIProviderModal.tsx +++ b/src/modules/agent-network/AIProviderModal.tsx @@ -28,6 +28,7 @@ import { ExternalLinkIcon, KeyRound, ListIcon, + Loader2, MinusCircleIcon, PlusCircle, PlusIcon, @@ -220,6 +221,11 @@ export default function AIProviderModal({ const [createdProvider, setCreatedProvider] = useState< AIProvider | undefined >(); + // Loading models saves before it asks, and that save waits on the vendor. + // Without its own flag the button would sit idle-looking through the slowest + // part of the operation — a timeout can take seconds with nothing on screen, + // and the obvious response to that is to press it again. + const [savingBeforeDiscovery, setSavingBeforeDiscovery] = useState(false); // The record on the server this modal is working against: the one it was // opened on, or the one it created in order to load models. const targetProvider = provider ?? createdProvider; @@ -711,7 +717,13 @@ export default function AIProviderModal({ return; } - const saved = await persistForDiscovery(); + setSavingBeforeDiscovery(true); + let saved: AIProvider | undefined; + try { + saved = await persistForDiscovery(); + } finally { + setSavingBeforeDiscovery(false); + } if (!saved) return; await discovered.discover({ catalog_provider_id: providerId, @@ -719,6 +731,9 @@ export default function AIProviderModal({ }); }; + // One flag for the button: the two phases are one action to the operator. + const discoveryInFlight = savingBeforeDiscovery || discovered.isLoading; + // A discovery result describes one provider, endpoint and credential. Once // any of those changes on screen, the previous answer is about a // configuration that is no longer being edited, so it is dropped rather than @@ -1496,11 +1511,17 @@ export default function AIProviderModal({ @@ -1509,7 +1530,7 @@ export default function AIProviderModal({ Enter the endpoint URL and API key first. )} - {canDiscoverModels && !useSavedCredential && ( + {canDiscoverModels && !useSavedCredential && !discoveryInFlight && ( This saves the provider first, so the endpoint and key are checked before the vendor is asked. @@ -1530,7 +1551,7 @@ export default function AIProviderModal({ )} - {!discovered.isLoading && + {!discoveryInFlight && !discovered.error && discovered.models.length > 0 && ( @@ -1664,7 +1685,7 @@ export default function AIProviderModal({ @@ -1530,12 +1447,6 @@ export default function AIProviderModal({ Enter the endpoint URL and API key first. )} - {canDiscoverModels && !useSavedCredential && !discoveryInFlight && ( - - This saves the provider first, so the endpoint and key are - checked before the vendor is asked. - - )} {discovered.notSupported && ( This provider has no model listing endpoint — the catalog From b61513dfee02b20aeb82886ec938f353fbfc79c0 Mon Sep 17 00:00:00 2001 From: mlsmaycon Date: Wed, 26 Aug 2026 06:37:56 +0000 Subject: [PATCH 09/13] Show the provider save is in flight while the vendor is checked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Saving a provider now waits on a vendor round trip, and an upstream that never answers holds it until the request times out. The footer button did not move for any of that, so the only reading available was that the click had not registered — and a second one starts a second create. The button takes the same treatment the model listing already has: a spinner, a label saying what it is waiting on, and no clicks until the answer arrives. --- ...gent-network-provider-save-refused.spec.ts | 29 ++++- src/modules/agent-network/AIProviderModal.tsx | 119 ++++++++++-------- 2 files changed, 91 insertions(+), 57 deletions(-) diff --git a/e2e/tests/agent-network-provider-save-refused.spec.ts b/e2e/tests/agent-network-provider-save-refused.spec.ts index 64d058f30..77863c186 100644 --- a/e2e/tests/agent-network-provider-save-refused.spec.ts +++ b/e2e/tests/agent-network-provider-save-refused.spec.ts @@ -3,9 +3,13 @@ * * Saving a provider checks its upstream URL and credential against the vendor * before storing anything, so a 422 naming the field to correct is an ordinary - * outcome of the form rather than a server fault. Three things have to hold, + * outcome of the form rather than a server fault. Four things have to hold, * and each one has been wrong at some point: * + * - while the check runs the submit says so and refuses a second click. The + * vendor call is the slow part and an unreachable upstream drags it out to + * a timeout, which looked exactly like a button that had not registered + * the first click. * - exactly ONE toast, the shared "Request failed with status code N", which * quotes the API's own sentence. A second toast from the save path said the * same thing under a vaguer title. @@ -60,6 +64,11 @@ async function newAgentNetworkPage(browser: Browser): Promise<{ return { page, close: () => context.close() }; } +// How long the mocked create takes to answer. The real check is a vendor round +// trip, so a save is never instant; holding the response gives the in-flight +// assertions a window to run in without racing the toast. +const REFUSAL_DELAY_MS = 1500; + // refuseProviderCreate answers the create with the 422 the credential check // produces. Only POST is intercepted: the page still lists providers, and the // settings bootstrap that may precede the create is left alone. @@ -67,6 +76,7 @@ async function refuseProviderCreate(page: Page) { await page.route(PROVIDERS_ENDPOINT, async (route) => { if (route.request().method() !== "POST") return route.continue(); const origin = route.request().headers()["origin"] || "*"; + await new Promise((resolve) => setTimeout(resolve, REFUSAL_DELAY_MS)); await route.fulfill({ status: 422, headers: { @@ -100,14 +110,21 @@ test.describe await page.getByPlaceholder("sk-...").first().fill("sk-e2e-refused-key"); // The submit lives on the Models tab — the Provider tab's primary button - // only advances to it. + // only advances to it. Matched on both spellings of its label so the + // same locator follows it into the in-flight state. await page.getByRole("tab", { name: "Models" }).click({ force: true }); const submit = page - .getByRole("button", { name: /Connect Provider/ }) + .getByRole("button", { name: /Connect(ing)? [Pp]rovider/ }) .last(); await expect(submit).toBeEnabled(); await submit.click({ force: true }); + // ---- the wait is visible while it lasts ---- + // A second create is not idempotent, so the button has to say it is + // working and stop taking clicks rather than sit there looking untouched. + await expect(submit).toContainText("Connecting provider"); + await expect(submit).toBeDisabled(); + // ---- the toast says what the API said ---- const title = page.getByTestId(TITLE_TESTID).first(); await expect(title).toContainText("Request failed with status code 422"); @@ -133,8 +150,12 @@ test.describe // ---- the form is still there, still holding what was typed ---- // The submit only exists while the modal is open, so its presence is the - // check that nothing closed underneath the toast. + // check that nothing closed underneath the toast. It also has to come + // back out of the in-flight state, or the retry the toast asks for is + // impossible. await expect(submit).toBeVisible(); + await expect(submit).toBeEnabled(); + await expect(submit).toContainText("Connect Provider"); await page.getByRole("tab", { name: "Provider" }).click({ force: true }); await expect( page.locator(`input[value="${providerName}"]`), diff --git a/src/modules/agent-network/AIProviderModal.tsx b/src/modules/agent-network/AIProviderModal.tsx index cfeffdd26..25f10e1ac 100644 --- a/src/modules/agent-network/AIProviderModal.tsx +++ b/src/modules/agent-network/AIProviderModal.tsx @@ -204,6 +204,10 @@ export default function AIProviderModal({ const settingsBootstrapped = !!settings; const [tab, setTab] = useState("provider"); + // A save reaches the vendor to check the url and credential before storing + // anything, so it holds for as long as that round trip takes — up to a + // timeout. Nothing else on the footer moves while it does. + const [saveInFlight, setSaveInFlight] = useState(false); const [providerId, setProviderId] = useState( provider?.providerId ?? "openai_api", ); @@ -480,51 +484,56 @@ export default function AIProviderModal({ identityHeaderGroups: identityHeaderGroups.trim(), } : {}; - if (isEdit && provider) { - const saved = await updateProvider(provider.id, { + setSaveInFlight(true); + try { + if (isEdit && provider) { + const saved = await updateProvider(provider.id, { + providerId, + name, + upstreamUrl, + models: submittedModels, + extraValues: sanitizedExtraValues, + ...identityOverrides, + skipTlsVerification: isCustomKind ? skipTlsVerification : false, + metadataDisabled, + // Only forward the API key when the user actually rotated it + ...(apiKey && apiKey.trim() !== MASKED_API_KEY ? { apiKey } : {}), + }); + // The url and credential are checked against the vendor before the + // change is stored, so a save can be refused for a reason the operator + // has to fix here. Closing would throw away the key they just typed — + // and it never comes back from the API to be typed over again. + if (!saved) return; + handleClose(); + return; + } + // First create: bootstrap the account's endpoint before the provider + // exists, as an explicit settings POST. A failure keeps the wizard open + // (the provider isn't created either) so the operator sees the error and + // can retry — this used to be a silent backend side effect. + if (!settingsBootstrapped) { + const bootstrapped = await bootstrapAgentNetworkSettings( + bootstrapCluster.trim(), + ); + if (!bootstrapped) return; + } + const created = await addProvider({ providerId, name, upstreamUrl, - models: submittedModels, + apiKey, extraValues: sanitizedExtraValues, ...identityOverrides, skipTlsVerification: isCustomKind ? skipTlsVerification : false, metadataDisabled, - // Only forward the API key when the user actually rotated it - ...(apiKey && apiKey.trim() !== MASKED_API_KEY ? { apiKey } : {}), + models: submittedModels, + enabled: true, }); - // The url and credential are checked against the vendor before the - // change is stored, so a save can be refused for a reason the operator - // has to fix here. Closing would throw away the key they just typed — - // and it never comes back from the API to be typed over again. - if (!saved) return; + if (!created) return; handleClose(); - return; - } - // First create: bootstrap the account's endpoint before the provider - // exists, as an explicit settings POST. A failure keeps the wizard open - // (the provider isn't created either) so the operator sees the error and - // can retry — this used to be a silent backend side effect. - if (!settingsBootstrapped) { - const bootstrapped = await bootstrapAgentNetworkSettings( - bootstrapCluster.trim(), - ); - if (!bootstrapped) return; + } finally { + setSaveInFlight(false); } - const created = await addProvider({ - providerId, - name, - upstreamUrl, - apiKey, - extraValues: sanitizedExtraValues, - ...identityOverrides, - skipTlsVerification: isCustomKind ? skipTlsVerification : false, - metadataDisabled, - models: submittedModels, - enabled: true, - }); - if (!created) return; - handleClose(); }; // providerOptions are sorted into three groups, first-party AI Providers @@ -653,6 +662,24 @@ export default function AIProviderModal({ // and the result line, so one definition keeps them from drifting apart. const discoveryInFlight = discovered.isLoading; + // The footer renders the same submit twice — once on the models tab and once + // on the mappings tab — so its state is defined once here. + const submitDisabled = + !canContinueFromProvider || discoveryInFlight || saveInFlight; + const submitLabel = saveInFlight ? ( + <> + + {isEdit ? "Saving changes…" : "Connecting provider…"} + + ) : isEdit ? ( + "Save Changes" + ) : ( + <> + + Connect Provider + + ); + // A discovery result describes one provider, endpoint and credential. Once // any of those changes on screen, the previous answer is about a // configuration that is no longer being edited, so it is dropped rather than @@ -1596,16 +1623,9 @@ export default function AIProviderModal({ )} @@ -1618,16 +1638,9 @@ export default function AIProviderModal({ )} From a44cb84e0d2197c5f0f2aee74a9da3d8351026bd Mon Sep 17 00:00:00 2001 From: mlsmaycon Date: Wed, 26 Aug 2026 07:05:51 +0000 Subject: [PATCH 10/13] Load models against a retyped URL without asking for the key back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changing an existing provider's endpoint disabled Load models until an API key was typed over the mask — a key the API never returns, so the only way to list models for the new URL was to invent a reason to rotate the credential. The request now carries the record id and the URL on the form together, which the API reads as the stored credential against the typed endpoint. Switching the vendor dropdown still requires a fresh key: there the stored one belongs to a different vendor. --- src/modules/agent-network/AIProviderModal.tsx | 33 ++++++++++--------- .../agent-network/useDiscoveredModels.ts | 2 ++ 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/modules/agent-network/AIProviderModal.tsx b/src/modules/agent-network/AIProviderModal.tsx index 25f10e1ac..5aedc2902 100644 --- a/src/modules/agent-network/AIProviderModal.tsx +++ b/src/modules/agent-network/AIProviderModal.tsx @@ -616,27 +616,24 @@ export default function AIProviderModal({ // provider has to supply the key the operator is typing. // // That reuse is only right while the form still describes the record the - // credential belongs to. The API resolves a provider_id request entirely - // from the stored row — vendor, upstream and key — so switching the vendor - // dropdown and then asking by record id answers with the OLD vendor's models - // and offers them for the new one. A replacement key typed over the mask is - // the same mistake in the other direction: the operator wants that key - // tested, not the one already saved. + // credential belongs to. The API takes the vendor from the stored row, so + // switching the vendor dropdown and then asking by record id answers with + // the OLD vendor's models and offers them for the new one. A replacement key + // typed over the mask is the same mistake in the other direction: the + // operator wants that key tested, not the one already saved. // - // Changing the upstream URL invalidates the saved path: discovery sends - // provider_id and the API resolves the URL from the stored row, so a changed - // URL would silently test the old endpoint. Require a freshly entered key - // when the URL differs; canDiscoverModels will block discovery until the - // operator provides one. + // A retyped URL is neither. It is sent with the record id and overrides the + // stored upstream, so the endpoint on the form is the one listed against — + // asking for the key back would be asking for something the API never + // returned. const useSavedCredential = isEdit && !!provider?.id && providerId === provider.providerId && - upstreamUrl === provider.upstreamUrl && apiKey.trim() === MASKED_API_KEY; const canDiscoverModels = useMemo(() => { - if (useSavedCredential) return true; + if (useSavedCredential) return upstreamUrl.trim() !== ""; return ( upstreamUrl.trim() !== "" && apiKey.trim() !== "" && @@ -649,7 +646,11 @@ export default function AIProviderModal({ const loadModelsFromProvider = async () => { await discovered.discover( useSavedCredential && provider?.id - ? { catalog_provider_id: providerId, provider_id: provider.id } + ? { + catalog_provider_id: providerId, + provider_id: provider.id, + upstream_url: upstreamUrl.trim(), + } : { catalog_provider_id: providerId, upstream_url: upstreamUrl.trim(), @@ -1471,7 +1472,9 @@ export default function AIProviderModal({ {!canDiscoverModels && ( - Enter the endpoint URL and API key first. + {useSavedCredential + ? "Enter the endpoint URL first." + : "Enter the endpoint URL and API key first."} )} {discovered.notSupported && ( diff --git a/src/modules/agent-network/useDiscoveredModels.ts b/src/modules/agent-network/useDiscoveredModels.ts index f2abe34de..d58d8bef7 100644 --- a/src/modules/agent-network/useDiscoveredModels.ts +++ b/src/modules/agent-network/useDiscoveredModels.ts @@ -27,6 +27,8 @@ type DiscoveryResponse = { models: DiscoveredModel[] }; type DiscoveryRequest = { catalog_provider_id: string; + // Sent alongside provider_id, this overrides the record's stored upstream, + // which is how a retyped URL is listed against before it is saved. upstream_url?: string; // Exactly one of these. api_key is for a provider being typed in and not yet // saved; provider_id reuses a saved record's stored credential, which is how From f0c1810fc3d6df4f5d8f2bebf502a2119bc041e6 Mon Sep 17 00:00:00 2001 From: mlsmaycon Date: Wed, 26 Aug 2026 14:25:50 +0000 Subject: [PATCH 11/13] Do not report a stale provider list as a failed save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three things a review found in the write paths. A revalidation that failed after a successful create or update was caught alongside the write itself, so the modal stayed open on a form whose next submit would create a second provider. The mutate now runs after the write is known to have succeeded, and its own failure leaves a stale list rather than a reported failure. An update against a provider missing from the cached list returned false without saying anything, which read as a save that did nothing. It now says the provider is gone and to reload. The refused-save spec navigates through navigateTo, which dismisses the setup modal and clears the scroll lock it leaves behind — the Escape press it had was covering for the first of those and not the second. --- ...gent-network-provider-save-refused.spec.ts | 8 +-- .../agent-network/AIProvidersProvider.tsx | 49 +++++++++++++------ 2 files changed, 39 insertions(+), 18 deletions(-) diff --git a/e2e/tests/agent-network-provider-save-refused.spec.ts b/e2e/tests/agent-network-provider-save-refused.spec.ts index 77863c186..c7622920c 100644 --- a/e2e/tests/agent-network-provider-save-refused.spec.ts +++ b/e2e/tests/agent-network-provider-save-refused.spec.ts @@ -23,7 +23,7 @@ * dashboard's handling of the response, not the vendor call that produces it. */ import { type Browser, expect, type Page, test } from "@playwright/test"; -import { loginToApp } from "../helpers/auth"; +import { loginToApp, navigateTo } from "../helpers/auth"; import { generateRandomName } from "../helpers/utils"; const AGENT_NETWORK_CONFIG_KEY = "netbird-test-agent-network"; @@ -97,8 +97,10 @@ test.describe try { await refuseProviderCreate(page); - await page.goto("/agent-network/providers"); - await page.keyboard.press("Escape"); + // navigateTo rather than goto: it dismisses the setup modal that greets a + // fresh account and clears the scroll lock it leaves behind, either of + // which swallows the clicks below. + await navigateTo(page, "/agent-network/providers"); await page .getByRole("button", { name: "Connect Provider" }) diff --git a/src/modules/agent-network/AIProvidersProvider.tsx b/src/modules/agent-network/AIProvidersProvider.tsx index 2b93fae49..ce0cdb492 100644 --- a/src/modules/agent-network/AIProvidersProvider.tsx +++ b/src/modules/agent-network/AIProvidersProvider.tsx @@ -511,7 +511,10 @@ type AIProvidersContextValue = { // Resolves false when the save was refused — the backend checks a provider's // url and credential before storing them, so a rejected edit must leave the // form open with what the operator typed still in it. - updateProvider: (id: string, updates: ProviderUpdateInput) => Promise; + updateProvider: ( + id: string, + updates: ProviderUpdateInput, + ) => Promise; toggleProvider: (id: string) => Promise; deleteProvider: (id: string) => Promise; addPolicy: ( @@ -686,19 +689,24 @@ export default function AIProvidersProvider({ children }: Readonly) { const addProvider = useCallback( async (input: ProviderConnectInput) => { + let created: APIProvider; try { - const created = await providersApi.post(toCreateRequest(input)); - await mutate(); - notify({ - title: "AI provider connected", - description: `${created.name} is now available on your agent network endpoint.`, - }); - return fromAPI(created); + created = await providersApi.post(toCreateRequest(input)); } catch { // Reported already by the shared request-failed toast. Returning // undefined is what keeps the modal open on the fields to correct. return undefined; } + // Outside the catch: the provider exists from here on, and a failed + // revalidation is a stale list rather than a failed create. Reporting it + // as one would hold the modal open on a form whose next submit creates a + // second provider. + await mutate().catch(() => undefined); + notify({ + title: "AI provider connected", + description: `${created.name} is now available on your agent network endpoint.`, + }); + return fromAPI(created); }, [providersApi, mutate], ); @@ -706,7 +714,16 @@ export default function AIProvidersProvider({ children }: Readonly) { const updateProvider = useCallback( async (id: string, updates: ProviderUpdateInput) => { const existing = (apiProviders ?? []).find((p) => p.id === id); - if (!existing) return false; + if (!existing) { + // The update merges onto the record as this browser last saw it, so a + // provider deleted elsewhere leaves nothing to merge onto. Silence + // here read as a save that did nothing. + notifyFailure({ + title: "Provider not updated", + description: "This provider is no longer available. Reload the page.", + }); + return false; + } const merged: APIProviderRequest = { provider_id: updates.providerId ?? existing.provider_id, name: updates.name ?? existing.name, @@ -733,16 +750,18 @@ export default function AIProvidersProvider({ children }: Readonly) { }; try { await providersApi.put(merged, `/${id}`); - await mutate(); - notify({ - title: "Provider updated", - description: "Settings saved.", - }); - return true; } catch { // Reported already by the shared request-failed toast. return false; } + // See addProvider: a failed revalidation is a stale list, not a failed + // write, and must not send the operator back to resubmit one. + await mutate().catch(() => undefined); + notify({ + title: "Provider updated", + description: "Settings saved.", + }); + return true; }, [apiProviders, providersApi, mutate], ); From 5669f0c59a9aa5c8478aaf5e773660b0054af50b Mon Sep 17 00:00:00 2001 From: mlsmaycon Date: Wed, 26 Aug 2026 15:33:05 +0000 Subject: [PATCH 12/13] Select the refused-save spec by test id, and wait for the refusal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spec picked its controls out by role, placeholder and input value, and read the toast's failure styling off a tailwind class. The repo's e2e guide asks for data-testid and for adding them to components where they are missing, so the three controls it drives now carry one, and the notification tile names the state its colour encodes. It also waits for the create response and asserts the 422 rather than inferring it from the toast. The in-flight assertions still run before that await — the mock holds the response open for exactly that window. The dedicated browser context stays, and the header now says why: the Agent Network menu is gated behind a localStorage override that has to be set before the first navigation, and a 422 route left on the worker-scoped shared page would follow every later test. --- ...gent-network-provider-save-refused.spec.ts | 51 ++++++++++++------- src/components/Notification.tsx | 16 ++++++ src/modules/agent-network/AIProviderModal.tsx | 4 ++ 3 files changed, 54 insertions(+), 17 deletions(-) diff --git a/e2e/tests/agent-network-provider-save-refused.spec.ts b/e2e/tests/agent-network-provider-save-refused.spec.ts index c7622920c..22cb5e08c 100644 --- a/e2e/tests/agent-network-provider-save-refused.spec.ts +++ b/e2e/tests/agent-network-provider-save-refused.spec.ts @@ -21,6 +21,13 @@ * The refusal is mocked rather than provoked: the backend check ships with a * management build these tests do not pin, and what is under test here is the * dashboard's handling of the response, not the vendor call that produces it. + * + * Like the other agent-network specs, this one builds its own context rather + * than taking the shared dashboardAsOwner fixture: the Agent Network menu is + * deployment-gated behind a localStorage override that has to be in place + * before the first navigation, which addInitScript on an own context is the + * way to do. The route interception below is a second reason — the shared page + * is worker-scoped, and a 422 left on it would follow every later test. */ import { type Browser, expect, type Page, test } from "@playwright/test"; import { loginToApp, navigateTo } from "../helpers/auth"; @@ -108,25 +115,37 @@ test.describe .click({ force: true }); const providerName = generateRandomName(PROVIDER_PREFIX); - await page.locator('input[value="OpenAI API"]').fill(providerName); - await page.getByPlaceholder("sk-...").first().fill("sk-e2e-refused-key"); + await page + .getByTestId("agent-network-provider-name-input") + .fill(providerName); + await page + .getByTestId("agent-network-provider-key-input") + .fill("sk-e2e-refused-key"); // The submit lives on the Models tab — the Provider tab's primary button - // only advances to it. Matched on both spellings of its label so the - // same locator follows it into the in-flight state. + // only advances to it. await page.getByRole("tab", { name: "Models" }).click({ force: true }); - const submit = page - .getByRole("button", { name: /Connect(ing)? [Pp]rovider/ }) - .last(); + const submit = page.getByTestId("agent-network-provider-submit"); await expect(submit).toBeEnabled(); + + const refusal = page.waitForResponse( + (resp) => + PROVIDERS_ENDPOINT.test(resp.url()) && + resp.request().method() === "POST", + { timeout: 30_000 }, + ); await submit.click({ force: true }); // ---- the wait is visible while it lasts ---- // A second create is not idempotent, so the button has to say it is // working and stop taking clicks rather than sit there looking untouched. + // Asserted before the response is awaited: the mock holds it open for + // exactly this window. await expect(submit).toContainText("Connecting provider"); await expect(submit).toBeDisabled(); + expect((await refusal).status()).toBe(422); + // ---- the toast says what the API said ---- const title = page.getByTestId(TITLE_TESTID).first(); await expect(title).toContainText("Request failed with status code 422"); @@ -143,12 +162,10 @@ test.describe // ---- styled as a failure, not a success ---- // notify() paints the icon tile green with a check unless the caller // says otherwise, which is how a refusal once looked like a success. - await expect( - page.locator("[data-toast-notification] .bg-red-500").first(), - ).toBeVisible(); - await expect( - page.locator("[data-toast-notification] .bg-green-500"), - ).toHaveCount(0); + await expect(page.getByTestId("notification-icon")).toHaveAttribute( + "data-variant", + "error", + ); // ---- the form is still there, still holding what was typed ---- // The submit only exists while the modal is open, so its presence is the @@ -160,13 +177,13 @@ test.describe await expect(submit).toContainText("Connect Provider"); await page.getByRole("tab", { name: "Provider" }).click({ force: true }); await expect( - page.locator(`input[value="${providerName}"]`), - ).toBeVisible(); + page.getByTestId("agent-network-provider-name-input"), + ).toHaveValue(providerName); // The key matters most: the API never returns one, so a form that lost // it leaves the operator with nothing to correct. await expect( - page.locator('input[value="sk-e2e-refused-key"]'), - ).toBeVisible(); + page.getByTestId("agent-network-provider-key-input"), + ).toHaveValue("sk-e2e-refused-key"); } finally { await close(); } diff --git a/src/components/Notification.tsx b/src/components/Notification.tsx index a309e99d9..b36541c77 100644 --- a/src/components/Notification.tsx +++ b/src/components/Notification.tsx @@ -180,6 +180,22 @@ export default function Notification({ >
setName(e.target.value)} placeholder={"e.g. OpenAI"} @@ -1625,6 +1627,7 @@ export default function AIProviderModal({ ) : (