mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-10-07 15:28:34 +08:00
App 2052/optionally sync mcp config with secrets (#1001)
* add toggles to form fields * 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 <openhands@all-hands.dev> * refactor: update the code based on feedback --------- Co-authored-by: openhands <openhands@all-hands.dev> Co-authored-by: hieptl <hieptl.developer@gmail.com>
This commit is contained in:
committed by
GitHub
co-authored by
openhands
hieptl
parent
936b950dc0
commit
8b3ebbedba
@@ -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(<InstallServerModal entry={STDIO_ENTRY} onClose={vi.fn()} />);
|
||||
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(<InstallServerModal entry={STDIO_ENTRY} onClose={vi.fn()} />);
|
||||
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(<InstallServerModal entry={STDIO_ENTRY} onClose={vi.fn()} />);
|
||||
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(<InstallServerModal entry={STDIO_ENTRY} onClose={vi.fn()} />);
|
||||
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(<InstallServerModal entry={STDIO_ENTRY} onClose={vi.fn()} />);
|
||||
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(<InstallServerModal entry={STDIO_ENTRY} onClose={onClose} />);
|
||||
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(<InstallServerModal entry={STDIO_ENTRY} onClose={onClose} />);
|
||||
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(<InstallServerModal entry={STDIO_ENTRY} onClose={onClose} />);
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
@@ -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}
|
||||
<span data-testid="styled-tooltip-content">{content}</span>
|
||||
</>
|
||||
),
|
||||
}));
|
||||
|
||||
describe("SaveAsSecretToggle", () => {
|
||||
// ── rendering ──────────────────────────────────────────────────────────────
|
||||
|
||||
it("attaches data-testid to the label using fieldKey", () => {
|
||||
render(
|
||||
<SaveAsSecretToggle fieldKey="MY_KEY" checked={false} onToggle={vi.fn()} />,
|
||||
);
|
||||
expect(
|
||||
screen.getByTestId("mcp-install-save-secret-MY_KEY"),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders the field key inside a <code> element", () => {
|
||||
render(
|
||||
<SaveAsSecretToggle fieldKey="MY_KEY" checked={false} onToggle={vi.fn()} />,
|
||||
);
|
||||
const codeEl = screen.getByText("MY_KEY");
|
||||
expect(codeEl.tagName.toLowerCase()).toBe("code");
|
||||
});
|
||||
|
||||
it("renders the checkbox unchecked when checked=false", () => {
|
||||
render(
|
||||
<SaveAsSecretToggle fieldKey="KEY" checked={false} onToggle={vi.fn()} />,
|
||||
);
|
||||
expect(screen.getByRole("checkbox")).not.toBeChecked();
|
||||
});
|
||||
|
||||
it("renders the checkbox checked when checked=true", () => {
|
||||
render(
|
||||
<SaveAsSecretToggle fieldKey="KEY" checked={true} onToggle={vi.fn()} />,
|
||||
);
|
||||
expect(screen.getByRole("checkbox")).toBeChecked();
|
||||
});
|
||||
|
||||
// ── accessibility ──────────────────────────────────────────────────────────
|
||||
|
||||
it("the info button has an aria-label describing its purpose", () => {
|
||||
render(
|
||||
<SaveAsSecretToggle fieldKey="KEY" checked={false} onToggle={vi.fn()} />,
|
||||
);
|
||||
// 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(
|
||||
<SaveAsSecretToggle fieldKey="KEY" checked={false} onToggle={vi.fn()} />,
|
||||
);
|
||||
// The decorative <span> 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(
|
||||
<SaveAsSecretToggle fieldKey="KEY" checked={false} onToggle={onToggle} />,
|
||||
);
|
||||
fireEvent.click(screen.getByRole("checkbox"));
|
||||
expect(onToggle).toHaveBeenCalledWith(true);
|
||||
});
|
||||
|
||||
it("calls onToggle(false) when the checked checkbox is clicked", () => {
|
||||
const onToggle = vi.fn();
|
||||
render(
|
||||
<SaveAsSecretToggle fieldKey="KEY" checked={true} onToggle={onToggle} />,
|
||||
);
|
||||
fireEvent.click(screen.getByRole("checkbox"));
|
||||
expect(onToggle).toHaveBeenCalledWith(false);
|
||||
});
|
||||
|
||||
// ── tooltip ────────────────────────────────────────────────────────────────
|
||||
|
||||
it("passes tooltip text to StyledTooltip as its content prop", () => {
|
||||
render(
|
||||
<SaveAsSecretToggle fieldKey="KEY" checked={false} onToggle={vi.fn()} />,
|
||||
);
|
||||
// The mock renders StyledTooltip's content prop into a <span>.
|
||||
// t(key) => key, so the rendered text is the raw i18n key.
|
||||
expect(screen.getByTestId("styled-tooltip-content")).toHaveTextContent(
|
||||
"MCP$SAVE_AS_SECRET_TOOLTIP",
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,178 @@
|
||||
import React from "react";
|
||||
import { renderHook, waitFor } from "@testing-library/react";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
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;
|
||||
|
||||
// The hook reads a QueryClient via useQueryClient(), so renderHook needs a
|
||||
// provider. We hand back the client too so tests can spy on invalidateQueries.
|
||||
let queryClient: QueryClient;
|
||||
const createWrapper = () => {
|
||||
queryClient = new QueryClient({
|
||||
defaultOptions: { queries: { retry: false } },
|
||||
});
|
||||
return ({ children }: { children: React.ReactNode }) =>
|
||||
React.createElement(QueryClientProvider, { client: queryClient }, children);
|
||||
};
|
||||
|
||||
const renderSaveHook = () =>
|
||||
renderHook(() => useSaveFieldsAsSecrets(), { wrapper: createWrapper() });
|
||||
|
||||
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 } = renderSaveHook();
|
||||
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 } = renderSaveHook();
|
||||
result.current([f("KEY")], { KEY: "" }, { KEY: true });
|
||||
expect(SecretsService.createSecret).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("skips a checked field whose value is whitespace-only", () => {
|
||||
const { result } = renderSaveHook();
|
||||
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 } = renderSaveHook();
|
||||
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 } = renderSaveHook();
|
||||
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 } = renderSaveHook();
|
||||
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 } = renderSaveHook();
|
||||
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 } = renderSaveHook();
|
||||
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 } = renderSaveHook();
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
||||
// ── cache invalidation ──────────────────────────────────────────────────────
|
||||
|
||||
it("invalidates the secrets query caches after a successful save", async () => {
|
||||
const { result } = renderSaveHook();
|
||||
const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries");
|
||||
result.current([f("KEY")], { KEY: "val" }, { KEY: true });
|
||||
await waitFor(() => {
|
||||
expect(invalidateSpy).toHaveBeenCalledWith({
|
||||
queryKey: ["secrets-search"],
|
||||
});
|
||||
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ["secrets"] });
|
||||
});
|
||||
});
|
||||
|
||||
it("does not invalidate the secrets query caches when every save fails", async () => {
|
||||
vi.spyOn(SecretsService, "createSecret").mockRejectedValue(
|
||||
new Error("fail"),
|
||||
);
|
||||
const { result } = renderSaveHook();
|
||||
const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries");
|
||||
result.current([f("KEY")], { KEY: "val" }, { KEY: true });
|
||||
await waitFor(() =>
|
||||
expect(vi.mocked(displayErrorToast)).toHaveBeenCalledTimes(1),
|
||||
);
|
||||
expect(invalidateSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -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";
|
||||
|
||||
@@ -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";
|
||||
import { modalTitleLgClassName } from "#/utils/modal-classes";
|
||||
|
||||
interface InstallServerModalProps {
|
||||
@@ -30,6 +32,7 @@ interface InstallServerModalProps {
|
||||
interface FieldState {
|
||||
values: Record<string, string>;
|
||||
errors: Record<string, string | null>;
|
||||
savedAsSecret: Record<string, boolean>;
|
||||
}
|
||||
|
||||
function optionNeedsCredentialField(
|
||||
@@ -50,11 +53,14 @@ function isCredentialOptional(option: McpMarketplaceConnectionOption): boolean {
|
||||
|
||||
function makeInitialState(entry: MarketplaceEntry): FieldState {
|
||||
const values: Record<string, string> = {};
|
||||
const savedAsSecret: Record<string, boolean> = {};
|
||||
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] = "";
|
||||
@@ -62,7 +68,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
|
||||
@@ -79,10 +85,16 @@ 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<FieldState>(() =>
|
||||
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<string | null>(null);
|
||||
const option = getInstallableMcpConnectionOption(entry);
|
||||
const template = option?.transport;
|
||||
@@ -91,12 +103,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":
|
||||
@@ -121,6 +141,18 @@ 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.
|
||||
// Uses stateRef.current to avoid reading a stale closure snapshot.
|
||||
if (template?.kind === "stdio") {
|
||||
saveFieldsAsSecrets(
|
||||
template.envFields ?? [],
|
||||
stateRef.current.values,
|
||||
stateRef.current.savedAsSecret,
|
||||
);
|
||||
}
|
||||
},
|
||||
onError: (err: unknown) => {
|
||||
const message = retrieveAxiosErrorMessage(err as AxiosError);
|
||||
@@ -294,8 +326,17 @@ export function InstallServerModal({
|
||||
{state.errors[field.key] && (
|
||||
<p className="text-xs text-red-500">{state.errors[field.key]}</p>
|
||||
)}
|
||||
{field.key in state.savedAsSecret && (
|
||||
<SaveAsSecretToggle
|
||||
fieldKey={field.key}
|
||||
checked={state.savedAsSecret[field.key]}
|
||||
onToggle={(v) => toggleSecret(field.key, v)}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
))}
|
||||
{/* argFields are CLI arguments, not credentials — they don't need
|
||||
a "save as secret" toggle and are excluded from savedAsSecret. */}
|
||||
{(stdio.argFields ?? []).map((field) => (
|
||||
<div key={field.key} className="flex flex-col gap-1">
|
||||
<SettingsInput
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
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 (
|
||||
<label
|
||||
data-testid={`mcp-install-save-secret-${fieldKey}`}
|
||||
className={cn(
|
||||
"flex items-center gap-2 px-3 py-2 mt-0.5 rounded-lg border cursor-pointer transition-colors",
|
||||
checked
|
||||
? "border-green-500/35 bg-green-500/10"
|
||||
: "border-[var(--oh-border)] bg-transparent hover:bg-white/[0.03]",
|
||||
)}
|
||||
>
|
||||
{/* sr-only keeps the real checkbox in the accessibility tree so AT
|
||||
users can toggle it without seeing the custom visual track. */}
|
||||
<input
|
||||
className="sr-only"
|
||||
id={`mcp-save-secret-checkbox-${fieldKey}`}
|
||||
type="checkbox"
|
||||
checked={checked}
|
||||
onChange={(e) => onToggle(e.target.checked)}
|
||||
/>
|
||||
{/* aria-hidden: purely decorative — the checkbox above is the semantic control. */}
|
||||
<span
|
||||
aria-hidden="true"
|
||||
className={cn(
|
||||
"relative inline-flex h-[22px] w-[40px] shrink-0 items-center rounded-full border transition-colors duration-200",
|
||||
checked
|
||||
? "border-green-500 bg-green-500"
|
||||
: "border-[var(--oh-border)] bg-surface-raised",
|
||||
)}
|
||||
>
|
||||
<span
|
||||
className={cn(
|
||||
"inline-block size-4 rounded-full transition-transform duration-200",
|
||||
checked
|
||||
? "translate-x-[21px] bg-white"
|
||||
: "translate-x-[2px] bg-[var(--oh-muted)]",
|
||||
)}
|
||||
/>
|
||||
</span>
|
||||
<span className="text-sm">{t(I18nKey.MCP$ALSO_SAVE_AS_SECRET)}</span>
|
||||
<code
|
||||
className={cn(
|
||||
"ml-auto text-[11px] font-mono tracking-tight border rounded px-1.5 py-0.5",
|
||||
checked
|
||||
? "text-green-500 border-green-500/35 bg-white/[0.04]"
|
||||
: "text-tertiary-alt border-[var(--oh-border)]",
|
||||
)}
|
||||
>
|
||||
{fieldKey}
|
||||
</code>
|
||||
<StyledTooltip
|
||||
content={t(I18nKey.MCP$SAVE_AS_SECRET_TOOLTIP)}
|
||||
placement="top"
|
||||
>
|
||||
{/* button so the tooltip is keyboard-reachable; type=button prevents
|
||||
accidental form submission when the user presses Enter. */}
|
||||
<button
|
||||
type="button"
|
||||
aria-label={t(I18nKey.MCP$SAVE_AS_SECRET_TOOLTIP)}
|
||||
className="flex items-center justify-center size-[15px] shrink-0 rounded-full border border-[var(--oh-muted)] text-tertiary-alt text-[9px] font-bold cursor-help"
|
||||
onClick={(e) => e.preventDefault()}
|
||||
>
|
||||
?
|
||||
</button>
|
||||
</StyledTooltip>
|
||||
</label>
|
||||
);
|
||||
}
|
||||
@@ -0,0 +1,79 @@
|
||||
import { useCallback } from "react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { useQueryClient } from "@tanstack/react-query";
|
||||
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";
|
||||
|
||||
/** Truncates a list of key names for display in toasts. */
|
||||
function formatKeyList(keys: string[]): string {
|
||||
const MAX = 3;
|
||||
if (keys.length <= MAX) return keys.join(", ");
|
||||
return `${keys.slice(0, MAX).join(", ")} … (+${keys.length - MAX})`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns a stable, fire-and-forget function that upserts checked envFields
|
||||
* into the Secrets store. MCP server config and the Secrets store are
|
||||
* separate — this bridges the gap so Automation Server can access credentials
|
||||
* without a separate manual step. Internally, `SecretsService.createSecret`
|
||||
* is an upsert, so existing secrets with the same name are overwritten safely.
|
||||
*/
|
||||
export function useSaveFieldsAsSecrets() {
|
||||
const { t } = useTranslation("openhands");
|
||||
const queryClient = useQueryClient();
|
||||
|
||||
return useCallback(
|
||||
(
|
||||
envFields: MarketplaceField[],
|
||||
values: Record<string, string>,
|
||||
savedAsSecret: Record<string, boolean>,
|
||||
): 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(),
|
||||
field.label,
|
||||
),
|
||||
),
|
||||
).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) {
|
||||
// Refresh any cached secrets lists so a newly-saved secret shows up
|
||||
// in Settings → Secrets immediately. Without this, the 5-minute
|
||||
// staleTime on the secrets query (use-get-secrets.ts) can keep a
|
||||
// previously-loaded list stale and hide the new secret. Mirrors the
|
||||
// invalidation done by secret-form.tsx and secrets-settings.tsx.
|
||||
queryClient.invalidateQueries({ queryKey: ["secrets-search"] });
|
||||
queryClient.invalidateQueries({ queryKey: ["secrets"] });
|
||||
|
||||
displaySuccessToast(
|
||||
t(I18nKey.MCP$SECRETS_SAVED, { keys: formatKeyList(saved) }),
|
||||
);
|
||||
}
|
||||
if (failed.length > 0) {
|
||||
displayErrorToast(
|
||||
t(I18nKey.MCP$SECRETS_SAVE_FAILED, { keys: formatKeyList(failed) }),
|
||||
);
|
||||
}
|
||||
});
|
||||
},
|
||||
[t, queryClient],
|
||||
);
|
||||
}
|
||||
@@ -13837,6 +13837,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": "Saved to secrets: {{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 サーバーを削除しました。",
|
||||
|
||||
Reference in New Issue
Block a user