Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { UploadProps } from "antd/es/upload";
import React, { useState } from "react";
import ProviderSpecificFields from "../add_model/provider_specific_fields";
import { Providers, providerLogoMap } from "../provider_info_helpers";
import { resetCredentialFormOnProviderChange } from "./credential_form_helpers";
const { Link } = Typography;

interface AddCredentialsModalProps {
Expand Down Expand Up @@ -59,8 +60,7 @@ const AddCredentialsModal: React.FC<AddCredentialsModalProps> = ({ open, onCance
<AntdSelect
showSearch
onChange={(value) => {
setSelectedProvider(value as Providers);
form.setFieldValue("custom_llm_provider", value);
resetCredentialFormOnProviderChange(form, value as Providers, setSelectedProvider);
}}
>
{Object.entries(Providers).map(([providerEnum, providerDisplayName]) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { useEffect, useState } from "react";
import ProviderSpecificFields from "../add_model/provider_specific_fields";
import { CredentialItem } from "../networking";
import { Providers, providerLogoMap } from "../provider_info_helpers";
import { resetCredentialFormOnProviderChange } from "./credential_form_helpers";
const { Link } = Typography;

interface EditCredentialsModalProps {
Expand Down Expand Up @@ -92,8 +93,7 @@ export default function EditCredentialsModal({
<AntdSelect
showSearch
onChange={(value) => {
setSelectedProvider(value as Providers);
form.setFieldValue("custom_llm_provider", value);
resetCredentialFormOnProviderChange(form, value as Providers, setSelectedProvider);
}}
>
{Object.entries(Providers).map(([providerEnum, providerDisplayName]) => (
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import type { FormInstance } from "antd";
import { describe, expect, it, vi } from "vitest";
import { Providers } from "../provider_info_helpers";
import { resetCredentialFormOnProviderChange } from "./credential_form_helpers";

/**
* Build a minimal FormInstance stub that records calls. We don't depend
* on the full Antd API surface — only the three methods the helper uses.
*/
function makeFormStub(initialFields: Record<string, unknown> = {}) {
const fields: Record<string, unknown> = { ...initialFields };
const stub = {
getFieldValue: vi.fn((key: string) => fields[key]),
setFieldValue: vi.fn((key: string, value: unknown) => {
fields[key] = value;
}),
resetFields: vi.fn(() => {
Object.keys(fields).forEach((k) => delete fields[k]);
}),
};
return { stub: stub as unknown as FormInstance, fields, calls: stub };
}

describe("resetCredentialFormOnProviderChange", () => {
it("clears all fields when switching providers", () => {
// Simulate the OpenAI->Google AI Studio leak: api_base picked up
// OpenAI's default value and the user typed a custom URL.
const { stub, fields, calls } = makeFormStub({
credential_name: "my-prod-key",
custom_llm_provider: "OpenAI",
api_base: "https://api.openai.com/v1",
api_key: "sk-stale-openai-key",
organization: "org-leak",
});
const setSelectedProvider = vi.fn();

resetCredentialFormOnProviderChange(stub, Providers.Google_AI_Studio, setSelectedProvider);

expect(calls.resetFields).toHaveBeenCalledTimes(1);
// Provider-specific fields must be gone so the next render starts
// from the new provider's default_value, not OpenAI's leftover.
expect(fields.api_base).toBeUndefined();
expect(fields.api_key).toBeUndefined();
expect(fields.organization).toBeUndefined();
});

it("preserves credential_name across the switch", () => {
// credential_name is user-supplied metadata, not provider-specific.
// The admin shouldn't have to retype it just because they re-picked
// the provider.
const { stub, fields } = makeFormStub({
credential_name: "my-prod-key",
custom_llm_provider: "OpenAI",
api_base: "https://api.openai.com/v1",
});

resetCredentialFormOnProviderChange(stub, Providers.Google_AI_Studio, vi.fn());

expect(fields.credential_name).toBe("my-prod-key");
});

it("updates custom_llm_provider and selectedProvider state to the new value", () => {
const { stub, fields } = makeFormStub({ credential_name: "x" });
const setSelectedProvider = vi.fn();

resetCredentialFormOnProviderChange(stub, Providers.Google_AI_Studio, setSelectedProvider);

expect(fields.custom_llm_provider).toBe(Providers.Google_AI_Studio);
expect(setSelectedProvider).toHaveBeenCalledExactlyOnceWith(Providers.Google_AI_Studio);
});

it("does not call setFieldValue('credential_name', undefined) when the name was unset", () => {
// Edge case: brand-new modal with no name typed yet. We shouldn't
// explicitly write `undefined` back into the form (Antd treats that
// as a touched empty field, triggering the "required" validation
// prematurely).
const { stub, calls } = makeFormStub({});

resetCredentialFormOnProviderChange(stub, Providers.Anthropic, vi.fn());

const credentialNameCalls = calls.setFieldValue.mock.calls.filter(
([key]) => key === "credential_name",
);
expect(credentialNameCalls).toHaveLength(0);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import type { FormInstance } from "antd";
import { Providers } from "../provider_info_helpers";

/**
* Reset the credential form when the user switches providers.
*
* Why: provider-specific fields (api_base, api_key, organization, ...)
* share a single Antd Form state across providers. Without this reset,
* the previous provider's values stick around — most visibly, OpenAI's
* default `api_base` (https://api.openai.com/v1) carries over when the
* user switches to Google AI Studio, overriding that provider's own
* default_value.
*
* Strategy: blow away the whole form, then restore the provider-agnostic
* fields (credential name + the new provider id) so the newly rendered
* `ProviderSpecificFields` can apply its own defaults from a clean slate.
*
* The credential name is preserved because it's a user-supplied label
* that shouldn't reset just because the admin re-selected a provider.
*/
export function resetCredentialFormOnProviderChange(
form: FormInstance,
newProvider: Providers,
setSelectedProvider: (p: Providers) => void,
): void {
const preservedName = form.getFieldValue("credential_name");
form.resetFields();
if (preservedName !== undefined) {
form.setFieldValue("credential_name", preservedName);
}
setSelectedProvider(newProvider);
form.setFieldValue("custom_llm_provider", newProvider);
}
Loading