From 063de40deb41d17de7ac679e92ea976a614a6530 Mon Sep 17 00:00:00 2001 From: HeyItsChloe Date: Mon, 1 Jun 2026 13:19:51 -0700 Subject: [PATCH 1/3] add toggles to form fields --- src/components/features/mcp-page/index.ts | 1 + .../mcp-page/install-server-modal.tsx | 35 ++++++++- .../mcp-page/save-as-secret-toggle.tsx | 73 +++++++++++++++++++ .../mutation/use-save-fields-as-secrets.ts | 52 +++++++++++++ src/i18n/translation.json | 68 +++++++++++++++++ 5 files changed, 228 insertions(+), 1 deletion(-) create mode 100644 src/components/features/mcp-page/save-as-secret-toggle.tsx create mode 100644 src/hooks/mutation/use-save-fields-as-secrets.ts diff --git a/src/components/features/mcp-page/index.ts b/src/components/features/mcp-page/index.ts index c64f0a148..d696d7657 100644 --- a/src/components/features/mcp-page/index.ts +++ b/src/components/features/mcp-page/index.ts @@ -3,6 +3,7 @@ export { InstalledServerCard } from "./installed-server-card"; export { MarketplaceSection } from "./marketplace-section"; export { MarketplaceCard } from "./marketplace-card"; export { InstallServerModal } from "./install-server-modal"; +export { SaveAsSecretToggle } from "./save-as-secret-toggle"; export { CustomServerEditor } from "./custom-server-editor"; export { McpToolbar } from "./mcp-toolbar"; export type { McpSectionFilter } from "./mcp-section-filter"; diff --git a/src/components/features/mcp-page/install-server-modal.tsx b/src/components/features/mcp-page/install-server-modal.tsx index 62047fca1..1284d536c 100644 --- a/src/components/features/mcp-page/install-server-modal.tsx +++ b/src/components/features/mcp-page/install-server-modal.tsx @@ -7,6 +7,7 @@ import { ModalBackdrop } from "#/components/shared/modals/modal-backdrop"; import { ModalCloseButton } from "#/components/shared/modals/modal-close-button"; import { BrandButton } from "#/components/features/settings/brand-button"; import { SettingsInput } from "#/components/features/settings/settings-input"; +import { SaveAsSecretToggle } from "#/components/features/mcp-page/save-as-secret-toggle"; import { I18nKey } from "#/i18n/declaration"; import type { IntegrationCatalogEntry as MarketplaceEntry } from "@openhands/extensions/integrations"; import { McpLogoBadge } from "#/components/features/mcp-logo-badge"; @@ -19,6 +20,7 @@ import { type McpMarketplaceConnectionOption, } from "#/utils/mcp-marketplace-utils"; import { retrieveAxiosErrorMessage } from "#/utils/retrieve-axios-error-message"; +import { useSaveFieldsAsSecrets } from "#/hooks/mutation/use-save-fields-as-secrets"; interface InstallServerModalProps { entry: MarketplaceEntry; @@ -29,6 +31,7 @@ interface InstallServerModalProps { interface FieldState { values: Record; errors: Record; + savedAsSecret: Record; } function optionNeedsCredentialField( @@ -49,11 +52,14 @@ function isCredentialOptional(option: McpMarketplaceConnectionOption): boolean { function makeInitialState(entry: MarketplaceEntry): FieldState { const values: Record = {}; + const savedAsSecret: Record = {}; const option = getInstallableMcpConnectionOption(entry); const template = option?.transport; if (template?.kind === "stdio") { for (const field of template.envFields ?? []) { values[field.key] = ""; + // Pre-check password fields; non-password fields default to off. + savedAsSecret[field.key] = field.type === "password"; } for (const field of template.argFields ?? []) { values[field.key] = ""; @@ -61,7 +67,7 @@ function makeInitialState(entry: MarketplaceEntry): FieldState { } else if (optionNeedsCredentialField(option)) { values.api_key = ""; } - return { values, errors: {} }; + return { values, errors: {}, savedAsSecret }; } // The marketplace install modal is intentionally add-only: clicking @@ -78,6 +84,7 @@ export function InstallServerModal({ const { t } = useTranslation("openhands"); const { mutate: addMcpServer, isPending: isAdding } = useAddMcpServer(); const { mutate: testMcpServer, isPending: isTesting } = useTestMcpServer(); + const saveFieldsAsSecrets = useSaveFieldsAsSecrets(); const [state, setState] = React.useState(() => makeInitialState(entry), @@ -90,12 +97,20 @@ export function InstallServerModal({ const setValue = (key: string, value: string) => { setState((prev) => ({ + ...prev, values: { ...prev.values, [key]: value }, errors: { ...prev.errors, [key]: null }, })); setGlobalError(null); }; + const toggleSecret = (key: string, value: boolean) => { + setState((prev) => ({ + ...prev, + savedAsSecret: { ...prev.savedAsSecret, [key]: value }, + })); + }; + const makeTestErrorMessage = (failure: MCPTestFailure): string => { switch (failure.error_kind) { case "timeout": @@ -120,6 +135,17 @@ export function InstallServerModal({ displaySuccessToast(t(I18nKey.MCP$INSTALL_SUCCESS)); onSuccess?.(entry); onClose(); + + // Save checked envFields as secrets in the background so the + // Automation Server can access them without a separate manual step. + // Runs after onClose so failures don't block the modal from closing. + if (template?.kind === "stdio") { + saveFieldsAsSecrets( + template.envFields ?? [], + state.values, + state.savedAsSecret, + ); + } }, onError: (err: unknown) => { const message = retrieveAxiosErrorMessage(err as AxiosError); @@ -293,6 +319,13 @@ export function InstallServerModal({ {state.errors[field.key] && (

{state.errors[field.key]}

)} + {field.key in state.savedAsSecret && ( + toggleSecret(field.key, v)} + /> + )} ))} {(stdio.argFields ?? []).map((field) => ( diff --git a/src/components/features/mcp-page/save-as-secret-toggle.tsx b/src/components/features/mcp-page/save-as-secret-toggle.tsx new file mode 100644 index 000000000..044e54ad5 --- /dev/null +++ b/src/components/features/mcp-page/save-as-secret-toggle.tsx @@ -0,0 +1,73 @@ +import { useTranslation } from "react-i18next"; +import { cn } from "#/utils/utils"; +import { StyledTooltip } from "#/components/shared/buttons/styled-tooltip"; +import { I18nKey } from "#/i18n/declaration"; + +interface SaveAsSecretToggleProps { + fieldKey: string; + checked: boolean; + onToggle: (value: boolean) => void; +} + +export function SaveAsSecretToggle({ + fieldKey, + checked, + onToggle, +}: SaveAsSecretToggleProps) { + const { t } = useTranslation("openhands"); + + return ( + + ); +} diff --git a/src/hooks/mutation/use-save-fields-as-secrets.ts b/src/hooks/mutation/use-save-fields-as-secrets.ts new file mode 100644 index 000000000..0f470c57a --- /dev/null +++ b/src/hooks/mutation/use-save-fields-as-secrets.ts @@ -0,0 +1,52 @@ +import { useTranslation } from "react-i18next"; +import type { MarketplaceField } from "@openhands/extensions/integrations"; +import { SecretsService } from "#/api/secrets-service"; +import { I18nKey } from "#/i18n/declaration"; +import { + displayErrorToast, + displaySuccessToast, +} from "#/utils/custom-toast-handlers"; + +/** + * Returns a fire-and-forget function that saves checked envFields as secrets. + * MCP server config and the Secrets store are separate — this bridges the gap + * so Automation Server can access credentials when running automations. + */ +export function useSaveFieldsAsSecrets() { + const { t } = useTranslation("openhands"); + + return ( + envFields: MarketplaceField[], + values: Record, + savedAsSecret: Record, + ): void => { + const fieldsToSave = envFields.filter( + (field) => savedAsSecret[field.key] && (values[field.key] ?? "").trim(), + ); + if (fieldsToSave.length === 0) return; + + Promise.allSettled( + fieldsToSave.map((field) => + SecretsService.createSecret(field.key, values[field.key].trim()), + ), + ).then((results) => { + const saved = fieldsToSave + .filter((_, i) => results[i].status === "fulfilled") + .map((f) => f.key); + const failed = fieldsToSave + .filter((_, i) => results[i].status === "rejected") + .map((f) => f.key); + + if (saved.length > 0) { + displaySuccessToast( + t(I18nKey.MCP$SECRETS_SAVED, { keys: saved.join(", ") }), + ); + } + if (failed.length > 0) { + displayErrorToast( + t(I18nKey.MCP$SECRETS_SAVE_FAILED, { keys: failed.join(", ") }), + ); + } + }); + }; +} diff --git a/src/i18n/translation.json b/src/i18n/translation.json index 9d8de6c0e..49efab4c4 100644 --- a/src/i18n/translation.json +++ b/src/i18n/translation.json @@ -13752,6 +13752,74 @@ "uk": "Сервер MCP збережено.", "ca": "Servidor MCP desat." }, + "MCP$SAVE_AS_SECRET_TOOLTIP": { + "en": "MCP credentials aren't shared with automations. Save as a secret to make this value available to automations.", + "ja": "MCPの認証情報はオートメーションと共有されません。この値をオートメーションで使用できるようにするには、シークレットとして保存してください。", + "zh-CN": "MCP 凭据不与自动化共享。将其保存为密钥以使该值对自动化可用。", + "zh-TW": "MCP 憑證不與自動化共享。將其儲存為密鑰以使該值對自動化可用。", + "ko-KR": "MCP 자격 증명은 자동화와 공유되지 않습니다. 이 값을 자동화에서 사용할 수 있도록 시크릿으로 저장하세요.", + "no": "MCP-legitimasjon deles ikke med automatiseringer. Lagre som en hemmelighet for å gjøre denne verdien tilgjengelig for automatiseringer.", + "it": "Le credenziali MCP non sono condivise con le automazioni. Salva come segreto per rendere questo valore disponibile alle automazioni.", + "pt": "As credenciais MCP não são compartilhadas com automações. Salve como segredo para disponibilizar este valor às automações.", + "es": "Las credenciales MCP no se comparten con las automatizaciones. Guarda como secreto para que este valor esté disponible para las automatizaciones.", + "ar": "بيانات اعتماد MCP لا تُشارك مع الأتمتة. احفظ كسر لإتاحة هذه القيمة للأتمتة.", + "fr": "Les identifiants MCP ne sont pas partagés avec les automatisations. Enregistrez comme secret pour rendre cette valeur disponible aux automatisations.", + "tr": "MCP kimlik bilgileri otomasyonlarla paylaşılmaz. Bu değeri otomasyonlar için kullanılabilir hale getirmek üzere gizli olarak kaydedin.", + "de": "MCP-Anmeldedaten werden nicht mit Automatisierungen geteilt. Als Secret speichern, um diesen Wert für Automatisierungen verfügbar zu machen.", + "uk": "Облікові дані MCP не передаються автоматизаціям. Збережіть як секрет, щоб зробити це значення доступним для автоматизацій.", + "ca": "Les credencials MCP no es comparteixen amb les automatitzacions. Desa com a secret per fer aquest valor disponible per a les automatitzacions." + }, + "MCP$ALSO_SAVE_AS_SECRET": { + "en": "Also save as secret", + "ja": "シークレットとしても保存", + "zh-CN": "同时保存为密钥", + "zh-TW": "同時儲存為密鑰", + "ko-KR": "시크릿으로도 저장", + "no": "Lagre også som hemmelighet", + "it": "Salva anche come segreto", + "pt": "Salvar também como segredo", + "es": "Guardar también como secreto", + "ar": "حفظ أيضاً كسر", + "fr": "Enregistrer aussi comme secret", + "tr": "Gizli olarak da kaydet", + "de": "Auch als Secret speichern", + "uk": "Також зберегти як секрет", + "ca": "Desar també com a secret" + }, + "MCP$SECRETS_SAVED": { + "en": "Secret saved: {{keys}}", + "ja": "シークレットを保存しました: {{keys}}", + "zh-CN": "密钥已保存：{{keys}}", + "zh-TW": "密鑰已儲存：{{keys}}", + "ko-KR": "시크릿이 저장되었습니다: {{keys}}", + "no": "Hemmelighet lagret: {{keys}}", + "it": "Segreto salvato: {{keys}}", + "pt": "Segredo salvo: {{keys}}", + "es": "Secreto guardado: {{keys}}", + "ar": "تم حفظ السر: {{keys}}", + "fr": "Secret enregistré : {{keys}}", + "tr": "Gizli kaydedildi: {{keys}}", + "de": "Secret gespeichert: {{keys}}", + "uk": "Секрет збережено: {{keys}}", + "ca": "Secret desat: {{keys}}" + }, + "MCP$SECRETS_SAVE_FAILED": { + "en": "Failed to save secret: {{keys}}", + "ja": "シークレットの保存に失敗しました: {{keys}}", + "zh-CN": "密钥保存失败：{{keys}}", + "zh-TW": "密鑰儲存失敗：{{keys}}", + "ko-KR": "시크릿 저장에 실패했습니다: {{keys}}", + "no": "Kunne ikke lagre hemmelighet: {{keys}}", + "it": "Impossibile salvare il segreto: {{keys}}", + "pt": "Falha ao salvar segredo: {{keys}}", + "es": "Error al guardar el secreto: {{keys}}", + "ar": "فشل حفظ السر: {{keys}}", + "fr": "Échec de l'enregistrement du secret : {{keys}}", + "tr": "Gizli kaydedilemedi: {{keys}}", + "de": "Fehler beim Speichern des Secrets: {{keys}}", + "uk": "Не вдалося зберегти секрет: {{keys}}", + "ca": "Error en desar el secret: {{keys}}" + }, "MCP$REMOVE_SUCCESS": { "en": "MCP server removed.", "ja": "MCP サーバーを削除しました。", From b3d2178ae855ed215505d84b35a7edd24ea227b8 Mon Sep 17 00:00:00 2001 From: openhands Date: Mon, 1 Jun 2026 21:00:41 +0000 Subject: [PATCH 2/3] refactor(mcp-secrets): improve save-as-secret hook, toggle a11y, and add tests - useSaveFieldsAsSecrets: wrap returned fn in useCallback for stable identity, pass field.label as secret description, truncate key lists >3 in toasts, update JSDoc to document upsert behaviour - SaveAsSecretToggle: replace hidden input with sr-only for AT accessibility, move data-testid to label, add aria-hidden to decorative track, replace div with button for keyboard-reachable info trigger with aria-label - InstallServerModal: add stateRef to avoid stale closure in onSuccess callback, add comment explaining why argFields have no secret toggle - translation.json: plural-safe English wording for MCP$SECRETS_SAVED - tests: add use-save-fields-as-secrets.test.ts (9 tests), add save-as-secret-toggle.test.tsx (9 tests), extend install-server-modal with save-as-secret describe block (8 tests); 35/35 pass Co-authored-by: openhands --- .../mcp-page/install-server-modal.test.tsx | 194 ++++++++++++++++++ .../mcp-page/save-as-secret-toggle.test.tsx | 113 ++++++++++ .../use-save-fields-as-secrets.test.ts | 130 ++++++++++++ .../mcp-page/install-server-modal.tsx | 12 +- .../mcp-page/save-as-secret-toggle.tsx | 20 +- .../mutation/use-save-fields-as-secrets.ts | 87 ++++---- src/i18n/translation.json | 2 +- 7 files changed, 516 insertions(+), 42 deletions(-) create mode 100644 __tests__/components/features/mcp-page/save-as-secret-toggle.test.tsx create mode 100644 __tests__/hooks/mutation/use-save-fields-as-secrets.test.ts diff --git a/__tests__/components/features/mcp-page/install-server-modal.test.tsx b/__tests__/components/features/mcp-page/install-server-modal.test.tsx index a5d29f604..21f7730f4 100644 --- a/__tests__/components/features/mcp-page/install-server-modal.test.tsx +++ b/__tests__/components/features/mcp-page/install-server-modal.test.tsx @@ -3,6 +3,7 @@ import { render, screen, fireEvent, waitFor } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import SettingsService from "#/api/settings-service/settings-service.api"; import McpService from "#/api/mcp-service/mcp-service.api"; +import { SecretsService } from "#/api/secrets-service"; import { MOCK_DEFAULT_USER_SETTINGS } from "#/mocks/handlers"; import { ActiveBackendProvider } from "#/contexts/active-backend-context"; import { InstallServerModal } from "#/components/features/mcp-page/install-server-modal"; @@ -378,4 +379,197 @@ describe("InstallServerModal", () => { ), ); }); + + // --------------------------------------------------------------------------- + // Save-as-secret toggle behaviour + // --------------------------------------------------------------------------- + + // Synthetic stdio entry with one password-type envField, one text-type + // envField, and one argField. This gives us complete control over field + // types without depending on the live integration catalog. + const STDIO_ENTRY = { + id: "synthetic-stdio", + kind: "mcp", + name: "Synthetic Stdio Server", + description: "Stdio server used to test the save-as-secret feature.", + iconBg: "#000000", + defaultConnectionOptionId: "stdio", + connectionOptions: [ + { + id: "stdio", + provider: "mcp", + transport: { + kind: "stdio", + serverName: "test-server", + command: "npx", + args: ["-y", "test-mcp"], + envFields: [ + { + key: "API_KEY", + label: "API Key", + type: "password", + required: true, + placeholder: "Enter API key", + }, + { + key: "USERNAME", + label: "Username", + type: "text", + required: false, + placeholder: "Enter username", + }, + ], + argFields: [ + { + key: "EXTRA_ARG", + label: "Extra Arg", + type: "text", + required: false, + placeholder: "optional", + }, + ], + }, + auth: { strategy: "api_key", apiKeyOptional: true }, + }, + ], + } as unknown as MarketplaceEntry; + + describe("InstallServerModal — save as secret", () => { + beforeEach(() => { + vi.spyOn(SecretsService, "createSecret").mockResolvedValue(); + }); + + it("pre-checks the toggle for password-type envFields", async () => { + renderWith(); + await screen.findByTestId("mcp-install-modal"); + + const toggle = screen.getByTestId("mcp-install-save-secret-API_KEY"); + expect(toggle.querySelector("input[type='checkbox']")).toBeChecked(); + }); + + it("leaves non-password envFields unchecked by default", async () => { + renderWith(); + await screen.findByTestId("mcp-install-modal"); + + const toggle = screen.getByTestId("mcp-install-save-secret-USERNAME"); + expect(toggle.querySelector("input[type='checkbox']")).not.toBeChecked(); + }); + + it("does not render a toggle for argFields", async () => { + renderWith(); + await screen.findByTestId("mcp-install-modal"); + + expect( + screen.queryByTestId("mcp-install-save-secret-EXTRA_ARG"), + ).not.toBeInTheDocument(); + }); + + it("toggling the checkbox updates its checked state", async () => { + renderWith(); + await screen.findByTestId("mcp-install-modal"); + + // USERNAME starts unchecked; clicking it should flip to checked. + const toggle = screen.getByTestId("mcp-install-save-secret-USERNAME"); + const checkbox = toggle.querySelector( + "input[type='checkbox']", + ) as HTMLInputElement; + expect(checkbox).not.toBeChecked(); + + fireEvent.click(checkbox); + + expect(checkbox).toBeChecked(); + }); + + it("setValue preserves savedAsSecret state when a field value changes", async () => { + // Before the ...prev bug-fix in setValue, calling onChange on any field + // would reset savedAsSecret to {}, unchecking all toggles silently. + renderWith(); + await screen.findByTestId("mcp-install-modal"); + + // API_KEY starts pre-checked. Typing a new value should leave it checked. + fireEvent.change(screen.getByTestId("mcp-install-field-API_KEY"), { + target: { value: "new-value" }, + }); + + const toggle = screen.getByTestId("mcp-install-save-secret-API_KEY"); + expect(toggle.querySelector("input[type='checkbox']")).toBeChecked(); + }); + + it("calls createSecret for checked envFields after a successful install", async () => { + vi.spyOn(SettingsService, "saveSettings").mockResolvedValue(true); + const onClose = vi.fn(); + renderWith(); + await screen.findByTestId("mcp-install-modal"); + await waitFor(() => expect(SettingsService.getSettings).toHaveBeenCalled()); + + // Fill in the required password field (API_KEY is pre-checked as secret). + fireEvent.change(screen.getByTestId("mcp-install-field-API_KEY"), { + target: { value: "my-api-key" }, + }); + fireEvent.click(screen.getByTestId("mcp-install-submit")); + + await waitFor(() => expect(onClose).toHaveBeenCalledTimes(1)); + await waitFor(() => + expect(SecretsService.createSecret).toHaveBeenCalledWith( + "API_KEY", + "my-api-key", + "API Key", + ), + ); + // USERNAME was unchecked, so no secret call for it. + expect(SecretsService.createSecret).not.toHaveBeenCalledWith( + "USERNAME", + expect.anything(), + expect.anything(), + ); + }); + + it("does not call createSecret when all toggles are unchecked before install", async () => { + vi.spyOn(SettingsService, "saveSettings").mockResolvedValue(true); + const onClose = vi.fn(); + renderWith(); + await screen.findByTestId("mcp-install-modal"); + await waitFor(() => expect(SettingsService.getSettings).toHaveBeenCalled()); + + fireEvent.change(screen.getByTestId("mcp-install-field-API_KEY"), { + target: { value: "my-api-key" }, + }); + + // Uncheck the pre-checked API_KEY toggle before submitting. + const toggle = screen.getByTestId("mcp-install-save-secret-API_KEY"); + fireEvent.click(toggle.querySelector("input[type='checkbox']")!); + + fireEvent.click(screen.getByTestId("mcp-install-submit")); + await waitFor(() => expect(onClose).toHaveBeenCalledTimes(1)); + + // Flush the fire-and-forget microtask chain. + await Promise.resolve(); + await Promise.resolve(); + expect(SecretsService.createSecret).not.toHaveBeenCalled(); + }); + + it("closes the modal even when the background secret save fails", async () => { + vi.spyOn(SettingsService, "saveSettings").mockResolvedValue(true); + vi.spyOn(SecretsService, "createSecret").mockRejectedValue( + new Error("forbidden"), + ); + const onClose = vi.fn(); + renderWith(); + await screen.findByTestId("mcp-install-modal"); + await waitFor(() => expect(SettingsService.getSettings).toHaveBeenCalled()); + + fireEvent.change(screen.getByTestId("mcp-install-field-API_KEY"), { + target: { value: "my-api-key" }, + }); + fireEvent.click(screen.getByTestId("mcp-install-submit")); + + // The modal must close regardless of the secret-save outcome. + await waitFor(() => expect(onClose).toHaveBeenCalledTimes(1)); + // Secret save errors use toasts, not the modal inline error element. + expect( + screen.queryByTestId("mcp-install-modal-error"), + ).not.toBeInTheDocument(); + }); + }); + }); diff --git a/__tests__/components/features/mcp-page/save-as-secret-toggle.test.tsx b/__tests__/components/features/mcp-page/save-as-secret-toggle.test.tsx new file mode 100644 index 000000000..4ff4578c8 --- /dev/null +++ b/__tests__/components/features/mcp-page/save-as-secret-toggle.test.tsx @@ -0,0 +1,113 @@ +import type { ReactNode } from "react"; +import { render, screen, fireEvent } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; +import { SaveAsSecretToggle } from "#/components/features/mcp-page/save-as-secret-toggle"; + +// HeroUI's Tooltip (used inside StyledTooltip) only mounts its content on a +// real hover event, which jsdom doesn't fire. Stub it so the content renders +// eagerly and is queryable in tests. +vi.mock("#/components/shared/buttons/styled-tooltip", () => ({ + StyledTooltip: ({ + content, + children, + }: { + content: ReactNode; + children: ReactNode; + }) => ( + <> + {children} + {content} + + ), +})); + +describe("SaveAsSecretToggle", () => { + // ── rendering ────────────────────────────────────────────────────────────── + + it("attaches data-testid to the label using fieldKey", () => { + render( + , + ); + expect( + screen.getByTestId("mcp-install-save-secret-MY_KEY"), + ).toBeInTheDocument(); + }); + + it("renders the field key inside a element", () => { + render( + , + ); + const codeEl = screen.getByText("MY_KEY"); + expect(codeEl.tagName.toLowerCase()).toBe("code"); + }); + + it("renders the checkbox unchecked when checked=false", () => { + render( + , + ); + expect(screen.getByRole("checkbox")).not.toBeChecked(); + }); + + it("renders the checkbox checked when checked=true", () => { + render( + , + ); + expect(screen.getByRole("checkbox")).toBeChecked(); + }); + + // ── accessibility ────────────────────────────────────────────────────────── + + it("the info button has an aria-label describing its purpose", () => { + render( + , + ); + // t(key) => key in tests, so aria-label equals the raw i18n key. + const infoBtn = screen.getByRole("button"); + expect(infoBtn).toHaveAttribute( + "aria-label", + "MCP$SAVE_AS_SECRET_TOOLTIP", + ); + }); + + it("the visual track is hidden from the accessibility tree (aria-hidden)", () => { + const { container } = render( + , + ); + // The decorative that forms the visual slider track. + const track = container.querySelector("span[aria-hidden='true']"); + expect(track).toBeInTheDocument(); + }); + + // ── interaction ──────────────────────────────────────────────────────────── + + it("calls onToggle(true) when the unchecked checkbox is clicked", () => { + const onToggle = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByRole("checkbox")); + expect(onToggle).toHaveBeenCalledWith(true); + }); + + it("calls onToggle(false) when the checked checkbox is clicked", () => { + const onToggle = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByRole("checkbox")); + expect(onToggle).toHaveBeenCalledWith(false); + }); + + // ── tooltip ──────────────────────────────────────────────────────────────── + + it("passes tooltip text to StyledTooltip as its content prop", () => { + render( + , + ); + // The mock renders StyledTooltip's content prop into a . + // t(key) => key, so the rendered text is the raw i18n key. + expect(screen.getByTestId("styled-tooltip-content")).toHaveTextContent( + "MCP$SAVE_AS_SECRET_TOOLTIP", + ); + }); +}); diff --git a/__tests__/hooks/mutation/use-save-fields-as-secrets.test.ts b/__tests__/hooks/mutation/use-save-fields-as-secrets.test.ts new file mode 100644 index 000000000..d2f1f3236 --- /dev/null +++ b/__tests__/hooks/mutation/use-save-fields-as-secrets.test.ts @@ -0,0 +1,130 @@ +import { renderHook, waitFor } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { SecretsService } from "#/api/secrets-service"; +import { useSaveFieldsAsSecrets } from "#/hooks/mutation/use-save-fields-as-secrets"; +import type { MarketplaceField } from "@openhands/extensions/integrations"; +import { displayErrorToast, displaySuccessToast } from "#/utils/custom-toast-handlers"; + +// Replace both toast functions with vi.fn() instances so we can assert on them. +vi.mock("#/utils/custom-toast-handlers", () => ({ + displaySuccessToast: vi.fn(), + displayErrorToast: vi.fn(), +})); + +// Minimal field factory — the hook only needs `key` and `label`. +const f = (key: string, label = key): MarketplaceField => + ({ key, label }) as unknown as MarketplaceField; + +describe("useSaveFieldsAsSecrets", () => { + beforeEach(() => { + vi.spyOn(SecretsService, "createSecret").mockResolvedValue(); + vi.mocked(displaySuccessToast).mockClear(); + vi.mocked(displayErrorToast).mockClear(); + }); + + afterEach(() => vi.clearAllMocks()); + + // ── early-return guard ────────────────────────────────────────────────────── + + it("does nothing when no fields are checked", () => { + const { result } = renderHook(() => useSaveFieldsAsSecrets()); + result.current([f("KEY")], { KEY: "val" }, { KEY: false }); + expect(SecretsService.createSecret).not.toHaveBeenCalled(); + }); + + it("skips a checked field whose value is an empty string", () => { + const { result } = renderHook(() => useSaveFieldsAsSecrets()); + result.current([f("KEY")], { KEY: "" }, { KEY: true }); + expect(SecretsService.createSecret).not.toHaveBeenCalled(); + }); + + it("skips a checked field whose value is whitespace-only", () => { + const { result } = renderHook(() => useSaveFieldsAsSecrets()); + result.current([f("KEY")], { KEY: " " }, { KEY: true }); + expect(SecretsService.createSecret).not.toHaveBeenCalled(); + }); + + // ── createSecret call arguments ──────────────────────────────────────────── + + it("calls createSecret with the trimmed value and field label as description", () => { + const { result } = renderHook(() => useSaveFieldsAsSecrets()); + result.current( + [f("API_KEY", "API Key")], + { API_KEY: " sk-secret " }, + { API_KEY: true }, + ); + expect(SecretsService.createSecret).toHaveBeenCalledWith( + "API_KEY", + "sk-secret", + "API Key", + ); + }); + + it("only calls createSecret for checked fields, skipping unchecked ones", () => { + const { result } = renderHook(() => useSaveFieldsAsSecrets()); + result.current( + [f("CHECKED"), f("SKIPPED")], + { CHECKED: "val", SKIPPED: "val" }, + { CHECKED: true, SKIPPED: false }, + ); + expect(SecretsService.createSecret).toHaveBeenCalledTimes(1); + expect(SecretsService.createSecret).toHaveBeenCalledWith( + "CHECKED", + "val", + "CHECKED", + ); + }); + + // ── Promise.allSettled behaviour ──────────────────────────────────────────── + + it("calls createSecret for every checked field even if one of them rejects", () => { + vi.spyOn(SecretsService, "createSecret") + .mockRejectedValueOnce(new Error("conflict")) + .mockResolvedValue(); + const { result } = renderHook(() => useSaveFieldsAsSecrets()); + result.current( + [f("A"), f("B")], + { A: "val-a", B: "val-b" }, + { A: true, B: true }, + ); + // Both are called synchronously inside the allSettled map. + expect(SecretsService.createSecret).toHaveBeenCalledTimes(2); + }); + + // ── toast behaviour ───────────────────────────────────────────────────────── + + it("shows only a success toast when all fields save successfully", async () => { + const { result } = renderHook(() => useSaveFieldsAsSecrets()); + result.current([f("KEY")], { KEY: "val" }, { KEY: true }); + await waitFor(() => + expect(vi.mocked(displaySuccessToast)).toHaveBeenCalledTimes(1), + ); + expect(vi.mocked(displayErrorToast)).not.toHaveBeenCalled(); + }); + + it("shows only an error toast when all fields fail to save", async () => { + vi.spyOn(SecretsService, "createSecret").mockRejectedValue(new Error("fail")); + const { result } = renderHook(() => useSaveFieldsAsSecrets()); + result.current([f("KEY")], { KEY: "val" }, { KEY: true }); + await waitFor(() => + expect(vi.mocked(displayErrorToast)).toHaveBeenCalledTimes(1), + ); + expect(vi.mocked(displaySuccessToast)).not.toHaveBeenCalled(); + }); + + it("shows both success and error toasts on partial failure", async () => { + vi.spyOn(SecretsService, "createSecret") + .mockResolvedValueOnce() + .mockRejectedValueOnce(new Error("fail")); + const { result } = renderHook(() => useSaveFieldsAsSecrets()); + result.current( + [f("OK_KEY"), f("FAIL_KEY")], + { OK_KEY: "val", FAIL_KEY: "val" }, + { OK_KEY: true, FAIL_KEY: true }, + ); + await waitFor(() => { + expect(vi.mocked(displaySuccessToast)).toHaveBeenCalledTimes(1); + expect(vi.mocked(displayErrorToast)).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/src/components/features/mcp-page/install-server-modal.tsx b/src/components/features/mcp-page/install-server-modal.tsx index 1284d536c..f1aa3d8d0 100644 --- a/src/components/features/mcp-page/install-server-modal.tsx +++ b/src/components/features/mcp-page/install-server-modal.tsx @@ -89,6 +89,11 @@ export function InstallServerModal({ const [state, setState] = React.useState(() => makeInitialState(entry), ); + // Always holds the latest state so async callbacks (onSuccess) never read + // stale closure values, even under React concurrent-mode scheduling. + const stateRef = React.useRef(state); + stateRef.current = state; + const [globalError, setGlobalError] = React.useState(null); const option = getInstallableMcpConnectionOption(entry); const template = option?.transport; @@ -139,11 +144,12 @@ export function InstallServerModal({ // Save checked envFields as secrets in the background so the // Automation Server can access them without a separate manual step. // Runs after onClose so failures don't block the modal from closing. + // Uses stateRef.current to avoid reading a stale closure snapshot. if (template?.kind === "stdio") { saveFieldsAsSecrets( template.envFields ?? [], - state.values, - state.savedAsSecret, + stateRef.current.values, + stateRef.current.savedAsSecret, ); } }, @@ -328,6 +334,8 @@ export function InstallServerModal({ )} ))} + {/* argFields are CLI arguments, not credentials — they don't need + a "save as secret" toggle and are excluded from savedAsSecret. */} {(stdio.argFields ?? []).map((field) => (
+ {/* sr-only keeps the real checkbox in the accessibility tree so AT + users can toggle it without seeing the custom visual track. */} onToggle(e.target.checked)} /> + {/* aria-hidden: purely decorative — the checkbox above is the semantic control. */}