From bacf9c9ad02e8b3493c9c3bbd75be42f18fd0086 Mon Sep 17 00:00:00 2001 From: derhornspieler <15236687+derhornspieler@users.noreply.github.com> Date: Sun, 23 Aug 2026 20:08:06 -0400 Subject: [PATCH] fix(ui): harden the Add Provider wizard against three failure modes A council reviewed the four findings the review bots raised on the wizard and ruled these three worth fixing before human review, since all of them are in code this PR introduces. Creation no longer treats a failed deployment lookup as an empty list. That lookup is what makes a retry skip rows already created, so swallowing its error turned a partial-failure retry into duplicate deployments. It now stops and says why, with the rows still on screen. Whether the wizard creates or updates a credential is derived from the name it actually saved rather than a flag that was set once and never cleared. Renaming after a save used to take the update path against a name the server had never seen, which dead-ended the flow with no way forward but a reload. Create is disabled while any model name is blank, instead of submitting an empty model_name and producing a deployment that cannot be addressed. Each fix carries a regression test that fails when the fix is reverted. --- .../AddProviderPanel.integration.test.tsx | 76 +++++++++++++++++++ .../panels/add-provider/AddProviderPanel.tsx | 21 +++-- .../panels/add-provider/ReviewModelsStep.tsx | 9 ++- 3 files changed, 99 insertions(+), 7 deletions(-) diff --git a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.integration.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.integration.test.tsx index c7493b3870a..04452271e65 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.integration.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.integration.test.tsx @@ -333,4 +333,80 @@ describe("AddProviderPanel", () => { ); expect(await screen.findByText("claude-3-opus")).toBeInTheDocument(); }); + it("refuses to create when the deployment lookup fails, rather than duplicating saved rows", async () => { + discoverProviderModelsCall.mockResolvedValue({ models: ["claude-3-opus"] }); + listAllModelsCall.mockRejectedValue(new Error("proxy unreachable")); + const { user } = await setup(); + + await chooseProvider(user, "Anthropic"); + await user.type(screen.getByLabelText("Credential name"), "anthropic-prod"); + await user.click(screen.getByRole("button", { name: /Next/ })); + await user.type(await screen.findByLabelText("API Key"), "sk-ant-test"); + await user.click(screen.getByRole("button", { name: "Save credential" })); + + await screen.findByText("claude-3-opus"); + await user.click(screen.getByRole("button", { name: "Create 1 model" })); + + expect(await screen.findByText(/could duplicate ones already saved/i)).toBeInTheDocument(); + expect(createProviderModelCall).not.toHaveBeenCalled(); + }); + + it("creates rather than PATCHes after the credential name changes following a save", async () => { + discoverProviderModelsCall.mockResolvedValue({ models: ["claude-3-opus"] }); + const { user } = await setup(); + + await chooseProvider(user, "Anthropic"); + await user.type(screen.getByLabelText("Credential name"), "anthropic-first"); + await user.click(screen.getByRole("button", { name: /Next/ })); + + await chooseSelectOption( + user, + await screen.findByRole("combobox", { name: "Authentication method" }), + "Workload Identity Federation (LiteLLM-signed)", + ); + fireEvent.change(await screen.findByLabelText("Organization ID"), { target: { value: "org-1" } }); + fireEvent.change(screen.getByLabelText("Issuer URL"), { target: { value: "https://proxy.example.com" } }); + fireEvent.change(screen.getByLabelText("Issuer Subject"), { target: { value: "litellm-proxy" } }); + fireEvent.change(screen.getByLabelText("Signing Key Reference"), { target: { value: "os.environ/SIGNING_KEY" } }); + await user.click(screen.getByRole("button", { name: "Save credential" })); + + // The JWKS step is the one place the wizard pauses after a save, so it is the route back to + // the name field. Renaming there must create the new credential, never PATCH the old name. + await screen.findByText("Register this JWKS with Anthropic"); + await user.click(screen.getByRole("button", { name: /Back/ })); + await user.click(await screen.findByRole("button", { name: /Back/ })); + + const nameInput = await screen.findByLabelText("Credential name"); + await user.clear(nameInput); + await user.type(nameInput, "anthropic-second"); + await user.click(screen.getByRole("button", { name: /Next/ })); + + credentialCreateCall.mockClear(); + credentialUpdateCall.mockClear(); + await user.click(await screen.findByRole("button", { name: /Save credential/ })); + + await waitFor(() => + expect(credentialCreateCall).toHaveBeenCalledWith( + "test-access-token", + expect.objectContaining({ credential_name: "anthropic-second" }), + ), + ); + expect(credentialUpdateCall).not.toHaveBeenCalled(); + }); + + it("blocks creation while any model name is blank", async () => { + discoverProviderModelsCall.mockResolvedValue({ models: ["claude-3-opus"] }); + const { user } = await setup(); + + await chooseProvider(user, "Anthropic"); + await user.type(screen.getByLabelText("Credential name"), "anthropic-prod"); + await user.click(screen.getByRole("button", { name: /Next/ })); + await user.type(await screen.findByLabelText("API Key"), "sk-ant-test"); + await user.click(screen.getByRole("button", { name: "Save credential" })); + + const nameCell = await screen.findByDisplayValue("claude-3-opus"); + await user.clear(nameCell); + + expect(screen.getByRole("button", { name: /Create 1 model/ })).toBeDisabled(); + }); }); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.tsx b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.tsx index b46105bc84d..ccf5f65f4ec 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/AddProviderPanel.tsx @@ -90,12 +90,13 @@ export default function AddProviderPanel() { const [step, setStep] = React.useState("provider"); const [selectedProvider, setSelectedProvider] = React.useState(null); const [credentialName, setCredentialName] = React.useState(""); - const [credentialSaved, setCredentialSaved] = React.useState(false); + const [savedCredentialName, setSavedCredentialName] = React.useState(null); const [savedValues, setSavedValues] = React.useState>({}); const [federationRuleId, setFederationRuleId] = React.useState(""); const [jwks, setJwks] = React.useState(null); const [jwksError, setJwksError] = React.useState(null); const [discoveryError, setDiscoveryError] = React.useState(null); + const [createError, setCreateError] = React.useState(null); const [isDiscovering, setIsDiscovering] = React.useState(false); const [rows, setRows] = React.useState([]); const [isCreating, setIsCreating] = React.useState(false); @@ -124,8 +125,11 @@ export default function AddProviderPanel() { ); const litellmProvider = selectedProviderInfo?.litellm_provider ?? ""; + const credentialSaved = savedCredentialName !== null && savedCredentialName === credentialName; + const nameCollision = credentialName.length > 0 && + credentialName !== savedCredentialName && (credentialsResponse?.credentials ?? []).some((c) => c.credential_name === credentialName); const goTo = (next: WizardStep) => setStep(next); @@ -160,7 +164,7 @@ export default function AddProviderPanel() { await credentialUpdateCall(accessToken, credentialName, updatePayload); } setSavedValues(values); - setCredentialSaved(true); + setSavedCredentialName(credentialName); setFederationRuleId( typeof values.anthropic_federation_rule_id === "string" ? values.anthropic_federation_rule_id : "", ); @@ -231,13 +235,19 @@ export default function AddProviderPanel() { setIsCreating(true); setCreationResults([]); setAliasCollisions([]); + setCreateError(null); goTo("creating"); - let existing: DeploymentInfoRow[] = []; + let existing: DeploymentInfoRow[]; try { existing = (await listAllModelsCall(accessToken)).data; - } catch { - existing = []; + } catch (error) { + setIsCreating(false); + setCreateError( + `Could not read the existing deployments, so creating now could duplicate ones already saved. ${extractProxyErrorMessage(error)}`, + ); + goTo("review"); + return; } const pending = rowsPendingCreation(rows, litellmProvider, credentialName, existing); const pendingIds = new Set(pending.map((r) => r.id)); @@ -346,6 +356,7 @@ export default function AddProviderPanel() { { goTo("discover"); void runDiscovery(); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/ReviewModelsStep.tsx b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/ReviewModelsStep.tsx index a7a9b8525d4..20326a48736 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/ReviewModelsStep.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/panels/add-provider/ReviewModelsStep.tsx @@ -13,13 +13,16 @@ import { buildManualRow, type DiscoveredModelRow } from "./wizardLogic"; interface ReviewModelsStepProps { rows: DiscoveredModelRow[]; setRows: React.Dispatch>; + createError: string | null; onBack: () => void; onCreateModels: () => void; } -const ReviewModelsStep: React.FC = ({ rows, setRows, onBack, onCreateModels }) => { +const ReviewModelsStep: React.FC = ({ rows, setRows, createError, onBack, onCreateModels }) => { const [manualId, setManualId] = React.useState(""); + const hasBlankName = rows.some((row) => row.modelName.trim() === ""); + const updateRow = (id: string, patch: Partial) => setRows((current) => current.map((row) => (row.id === id ? { ...row, ...patch } : row))); @@ -113,11 +116,13 @@ const ReviewModelsStep: React.FC = ({ rows, setRows, onBa + {createError &&

{createError}

} +
-