Keep a refused provider save on screen, and its discovered prices

src/modules/agent-network/AIProviderModal.tsx

@@ -28,6 +28,7 @@ import {
   ExternalLinkIcon,
   KeyRound,
   ListIcon,
+  Loader2,
   MinusCircleIcon,
   PlusCircle,
   PlusIcon,
@@ -212,6 +213,22 @@ export default function AIProviderModal({
   );
   const [apiKey, setApiKey] = useState(isEdit ? MASKED_API_KEY : "");
   const [bootstrapCluster, setBootstrapCluster] = useState<string>("");
+  // 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
+  >();
+  // 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;
   const [models, setModels] = useState<EditableModel[]>(() =>
     (provider?.models ?? []).map(withModelKey),
   );
@@ -372,6 +389,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 +451,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<string>();
-    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<string>();
+    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 +503,8 @@ export default function AIProviderModal({
           identityHeaderGroups: identityHeaderGroups.trim(),
         }
       : {};
-    if (isEdit && provider) {
-      await updateProvider(provider.id, {
+    if (targetProvider) {
+      const saved = await updateProvider(targetProvider.id, {
         providerId,
         name,
         upstreamUrl,
@@ -492,6 +516,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 +534,7 @@ export default function AIProviderModal({
       );
       if (!bootstrapped) return;
     }
-    await addProvider({
+    const created = await addProvider({
       providerId,
       name,
       upstreamUrl,
@@ -517,6 +546,7 @@ export default function AIProviderModal({
       models: submittedModels,
       enabled: true,
     });
+    if (!created) return;
     handleClose();
   };
 
@@ -613,10 +643,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(() => {
@@ -630,18 +659,81 @@ 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<AIProvider | undefined> => {
+    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;
+    }
+
+    setSavingBeforeDiscovery(true);
+    let saved: AIProvider | undefined;
+    try {
+      saved = await persistForDiscovery();
+    } finally {
+      setSavingBeforeDiscovery(false);
+    }
+    if (!saved) return;
+    await discovered.discover({
+      catalog_provider_id: providerId,
+      provider_id: saved.id,
+    });
   };
 
+  // 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
@@ -1419,11 +1511,17 @@ export default function AIProviderModal({
                 <Button
                   variant={"secondary"}
                   size={"xs"}
-                  disabled={discovered.isLoading || !canDiscoverModels}
+                  disabled={discoveryInFlight || !canDiscoverModels}
                   onClick={loadModelsFromProvider}
                 >
-                  <RefreshCwIcon size={13} />
-                  {discovered.isLoading
+                  {discoveryInFlight ? (
+                    <Loader2 size={13} className={"animate-spin"} />
+                  ) : (
+                    <RefreshCwIcon size={13} />
+                  )}
+                  {savingBeforeDiscovery
+                    ? "Saving provider…"
+                    : discovered.isLoading
                     ? "Loading models…"
                     : "Load models from provider"}
                 </Button>
@@ -1432,6 +1530,12 @@ export default function AIProviderModal({
                     Enter the endpoint URL and API key first.
                   </HelpText>
                 )}
+                {canDiscoverModels && !useSavedCredential && !discoveryInFlight && (
+                  <HelpText className={"!mb-0"}>
+                    This saves the provider first, so the endpoint and key are
+                    checked before the vendor is asked.
+                  </HelpText>
+                )}
                 {discovered.notSupported && (
                   <HelpText className={"!mb-0"}>
                     This provider has no model listing endpoint — the catalog
@@ -1447,7 +1551,7 @@ export default function AIProviderModal({
                 )}
               </div>
 
-              {!discovered.isLoading &&
+              {!discoveryInFlight &&
                 !discovered.error &&
                 discovered.models.length > 0 && (
                   <HelpText className={"!mb-0"}>
@@ -1581,7 +1685,7 @@ export default function AIProviderModal({
                   <Button
                     variant={"primary"}
                     onClick={handleSubmit}
-                    disabled={!canContinueFromProvider}
+                    disabled={!canContinueFromProvider || discoveryInFlight}
                   >
                     {isEdit ? (
                       "Save Changes"
@@ -1603,7 +1707,7 @@ export default function AIProviderModal({
                 <Button
                   variant={"primary"}
                   onClick={handleSubmit}
-                  disabled={!canContinueFromProvider}
+                  disabled={!canContinueFromProvider || discoveryInFlight}
                 >
                   {isEdit ? (
                     "Save Changes"