diff --git a/apps/vscode/webview-ui/src/components/settings/common/ApiKeyField.tsx b/apps/vscode/webview-ui/src/components/settings/common/ApiKeyField.tsx index 597efaae2b..ad9a3d603d 100644 --- a/apps/vscode/webview-ui/src/components/settings/common/ApiKeyField.tsx +++ b/apps/vscode/webview-ui/src/components/settings/common/ApiKeyField.tsx @@ -29,6 +29,7 @@ export const ApiKeyField = ({ }: ApiKeyFieldProps) => { const [localValue, setLocalValue] = useState(initialValue) const isFocusedRef = useRef(false) + const hasPendingUserEditRef = useRef(false) const prevInitialValueRef = useRef(initialValue) useEffect(() => { @@ -42,12 +43,18 @@ export const ApiKeyField = ({ // Do not replace their in-progress input with the new mask, or subsequent saves only // persist the suffix typed after that rerender. if (!isFocusedRef.current) { + hasPendingUserEditRef.current = false setLocalValue(initialValue) } }, [initialValue]) useDebounceEffect( () => { + if (!hasPendingUserEditRef.current) { + return + } + + hasPendingUserEditRef.current = false onChange(localValue) }, 100, @@ -63,7 +70,10 @@ export const ApiKeyField = ({ onFocus={() => { isFocusedRef.current = true }} - onInput={(e: any) => setLocalValue(e.target.value)} + onInput={(e) => { + hasPendingUserEditRef.current = true + setLocalValue((e.target as HTMLInputElement | null)?.value ?? "") + }} placeholder={placeholder} required={true} style={{ width: "100%" }} diff --git a/apps/vscode/webview-ui/src/components/settings/providers/GenericProviderSettings.test.tsx b/apps/vscode/webview-ui/src/components/settings/providers/GenericProviderSettings.test.tsx index 97d899e699..e808b2147a 100644 --- a/apps/vscode/webview-ui/src/components/settings/providers/GenericProviderSettings.test.tsx +++ b/apps/vscode/webview-ui/src/components/settings/providers/GenericProviderSettings.test.tsx @@ -1,3 +1,4 @@ +import type { ProviderConfigResponse } from "@shared/proto/cline/models" import { ApiFormat } from "@shared/proto/cline/models" import { fireEvent, render, screen, waitFor } from "@testing-library/react" import type { ChangeEventHandler, ReactNode } from "react" @@ -6,6 +7,8 @@ import { useProviderConfig } from "@/hooks/useProviderConfig" import { useProviderModels } from "@/hooks/useProviderModels" import { GenericProviderSettings } from "./GenericProviderSettings" +const providerConfig = (config: Partial): ProviderConfigResponse => config as ProviderConfigResponse + vi.mock("@/hooks/useProviderModels", () => ({ useProviderModels: vi.fn(), })) @@ -147,7 +150,11 @@ describe("GenericProviderSettings", () => { refresh: vi.fn(), fingerprint: "fingerprint", }) - vi.mocked(useProviderConfig).mockReturnValue({ config: { apiKeyLength: 12 } as any, write, commitSelection: vi.fn() }) + vi.mocked(useProviderConfig).mockReturnValue({ + config: providerConfig({ apiKeyLength: 12 }), + write, + commitSelection: vi.fn(), + }) render( { await waitFor(() => expect(write).toHaveBeenCalledWith({ apiKey: "new-secret" })) }) + it("does not clear API keys while provider config is loading", async () => { + const write = vi.fn(async () => undefined) + vi.mocked(useProviderModels).mockReturnValue({ + models: {}, + defaultModelId: "", + isLoading: false, + isStale: false, + error: undefined, + refresh: vi.fn(), + fingerprint: "fingerprint", + }) + vi.mocked(useProviderConfig).mockReturnValue({ config: undefined, write, commitSelection: vi.fn() }) + + render( + , + ) + + await new Promise((resolve) => setTimeout(resolve, 150)) + expect(write).not.toHaveBeenCalled() + }) + + it("does not save when provider config hydrates a saved API key", async () => { + const write = vi.fn(async () => undefined) + vi.mocked(useProviderModels).mockReturnValue({ + models: {}, + defaultModelId: "", + isLoading: false, + isStale: false, + error: undefined, + refresh: vi.fn(), + fingerprint: "fingerprint", + }) + vi.mocked(useProviderConfig).mockReturnValue({ config: undefined, write, commitSelection: vi.fn() }) + + const { rerender } = render( + , + ) + + await new Promise((resolve) => setTimeout(resolve, 150)) + expect(write).not.toHaveBeenCalled() + + vi.mocked(useProviderConfig).mockReturnValue({ + config: providerConfig({ apiKeyLength: 8 }), + write, + commitSelection: vi.fn(), + }) + rerender( + , + ) + + expect(screen.getByDisplayValue("••••••••")).toBeInTheDocument() + await new Promise((resolve) => setTimeout(resolve, 150)) + expect(write).not.toHaveBeenCalled() + }) + + it("does not write a mask if config hydrates after a blurred edit", async () => { + const write = vi.fn(async () => undefined) + vi.mocked(useProviderModels).mockReturnValue({ + models: {}, + defaultModelId: "", + isLoading: false, + isStale: false, + error: undefined, + refresh: vi.fn(), + fingerprint: "fingerprint", + }) + vi.mocked(useProviderConfig).mockReturnValue({ config: undefined, write, commitSelection: vi.fn() }) + + const { rerender } = render( + , + ) + + const apiKeyInput = screen.getByPlaceholderText("Enter API Key...") + fireEvent.input(apiKeyInput, { target: { value: "partial-key" } }) + fireEvent.blur(apiKeyInput) + + vi.mocked(useProviderConfig).mockReturnValue({ + config: providerConfig({ apiKeyLength: 8 }), + write, + commitSelection: vi.fn(), + }) + rerender( + , + ) + + await new Promise((resolve) => setTimeout(resolve, 150)) + expect(write).not.toHaveBeenCalledWith({ apiKey: "••••••••" }) + expect(write).not.toHaveBeenCalled() + }) + + it("still allows users to clear a saved API key", async () => { + const write = vi.fn(async () => undefined) + vi.mocked(useProviderModels).mockReturnValue({ + models: {}, + defaultModelId: "", + isLoading: false, + isStale: false, + error: undefined, + refresh: vi.fn(), + fingerprint: "fingerprint", + }) + vi.mocked(useProviderConfig).mockReturnValue({ + config: providerConfig({ apiKeyLength: 8 }), + write, + commitSelection: vi.fn(), + }) + + render( + , + ) + + fireEvent.input(screen.getByDisplayValue("••••••••"), { target: { value: "" } }) + + await waitFor(() => expect(write).toHaveBeenCalledWith({ apiKey: "" })) + }) + it("does not persist mask characters when editing a saved API key", async () => { const write = vi.fn(async () => undefined) vi.mocked(useProviderModels).mockReturnValue({ @@ -179,7 +338,11 @@ describe("GenericProviderSettings", () => { refresh: vi.fn(), fingerprint: "fingerprint", }) - vi.mocked(useProviderConfig).mockReturnValue({ config: { apiKeyLength: 7 } as any, write, commitSelection: vi.fn() }) + vi.mocked(useProviderConfig).mockReturnValue({ + config: providerConfig({ apiKeyLength: 7 }), + write, + commitSelection: vi.fn(), + }) render( { refresh: vi.fn(), fingerprint: "fingerprint", }) - vi.mocked(useProviderConfig).mockReturnValue({ config: { apiKeyLength: 0 } as any, write, commitSelection: vi.fn() }) + vi.mocked(useProviderConfig).mockReturnValue({ + config: providerConfig({ apiKeyLength: 0 }), + write, + commitSelection: vi.fn(), + }) const { rerender } = render( { await waitFor(() => expect(write).toHaveBeenCalledWith({ apiKey: "max" })) - vi.mocked(useProviderConfig).mockReturnValue({ config: { apiKeyLength: 3 } as any, write, commitSelection: vi.fn() }) + vi.mocked(useProviderConfig).mockReturnValue({ + config: providerConfig({ apiKeyLength: 3 }), + write, + commitSelection: vi.fn(), + }) rerender( { fingerprint: "fingerprint", }) vi.mocked(useProviderConfig).mockReturnValue({ - config: { baseUrl: "https://custom.example", apiKeyLength: 0 } as any, + config: providerConfig({ baseUrl: "https://custom.example", apiKeyLength: 0 }), write, commitSelection: vi.fn(), })