From de02eeb8569a00dbbafdbe0ba34f6a068b287019 Mon Sep 17 00:00:00 2001
From: Wenchao An <ansusu1025@gmail.com>
Date: Thu, 17 Sep 2026 08:31:16 +0800
Subject: [PATCH] fix(frontend): keep MCP configuration dialog within viewport
 (#5492)

* fix(frontend): keep MCP configuration dialog within viewport

* fix(frontend): keep MCP dialog actions outside fallback scroll area
---
 .../capabilities/mcp-plugin-manager.tsx       |  41 +++--
 frontend/tests/e2e/mcp-settings.spec.ts       | 163 ++++++++++++++++++
 2 files changed, 186 insertions(+), 18 deletions(-)

diff --git a/frontend/src/components/workspace/capabilities/mcp-plugin-manager.tsx b/frontend/src/components/workspace/capabilities/mcp-plugin-manager.tsx
index 6321f12bc..c6035fbde 100644
--- a/frontend/src/components/workspace/capabilities/mcp-plugin-manager.tsx
+++ b/frontend/src/components/workspace/capabilities/mcp-plugin-manager.tsx
@@ -287,14 +287,16 @@ function MCPServerList({
         open={editor !== null}
         onOpenChange={(open) => !open && !isWriting && closeEditor()}
       >
-        <DialogContent>
-          <DialogHeader>
+        <DialogContent className="flex max-h-[calc(100dvh-2rem)] flex-col overflow-hidden sm:max-w-2xl">
+          <DialogHeader className="shrink-0 pr-6 break-words">
             <DialogTitle>
               {editor?.mode === "edit"
                 ? t.settings.tools.editServer
                 : t.settings.tools.addServer}
             </DialogTitle>
-            <DialogDescription>
+          </DialogHeader>
+          <div className="-m-1 flex min-h-0 flex-col gap-4 overflow-y-auto p-1">
+            <DialogDescription className="shrink-0 text-center break-words sm:text-left">
               {editor?.mode === "edit"
                 ? t.settings.tools.editServerDescription.replace(
                     "{name}",
@@ -302,21 +304,24 @@ function MCPServerList({
                   )
                 : t.settings.tools.addServerDescription}
             </DialogDescription>
-          </DialogHeader>
-          <Textarea
-            className="min-h-52 font-mono text-xs"
-            aria-label={t.settings.tools.serverDefinitionLabel}
-            spellCheck={false}
-            value={definition}
-            placeholder={t.settings.tools.addServerPlaceholder}
-            onChange={(event) => setDefinition(event.target.value)}
-          />
-          {definitionError && (
-            <div className="text-destructive text-sm" role="alert">
-              {definitionError}
-            </div>
-          )}
-          <DialogFooter>
+            <Textarea
+              className="field-sizing-fixed h-96 min-h-24 resize-none overflow-auto font-mono text-xs"
+              aria-label={t.settings.tools.serverDefinitionLabel}
+              spellCheck={false}
+              value={definition}
+              placeholder={t.settings.tools.addServerPlaceholder}
+              onChange={(event) => setDefinition(event.target.value)}
+            />
+            {definitionError && (
+              <div
+                className="text-destructive shrink-0 text-sm break-words"
+                role="alert"
+              >
+                {definitionError}
+              </div>
+            )}
+          </div>
+          <DialogFooter className="shrink-0">
             <Button
               variant="outline"
               disabled={isWriting}
diff --git a/frontend/tests/e2e/mcp-settings.spec.ts b/frontend/tests/e2e/mcp-settings.spec.ts
index 7c4f266cd..1a620c64d 100644
--- a/frontend/tests/e2e/mcp-settings.spec.ts
+++ b/frontend/tests/e2e/mcp-settings.spec.ts
@@ -2,6 +2,169 @@ import { expect, test } from "@playwright/test";
 
 import { mockLangGraphAPI } from "./utils/mock-api";
 
+for (const { viewport, name } of [
+  { viewport: { width: 500, height: 280 }, name: "github" },
+  {
+    viewport: { width: 844, height: 390 },
+    name: `github-${"long-server-name-".repeat(30)}`,
+  },
+]) {
+  test(`MCP dialog actions remain reachable during fallback scrolling at ${viewport.width}x${viewport.height}`, async ({
+    page,
+  }, testInfo) => {
+    await page.setViewportSize(viewport);
+    mockLangGraphAPI(page);
+    const server = { enabled: false, command: "npx", args: ["example"] };
+    await page.route("**/api/mcp/config", (route) =>
+      route.fulfill({ json: { mcp_servers: { [name]: server } } }),
+    );
+    await page.goto("/workspace/capabilities");
+
+    for (const mode of ["edit", "add"] as const) {
+      await page
+        .getByRole("button", {
+          name: mode === "edit" ? `Edit ${name}` : "Add MCP plugin",
+          exact: true,
+        })
+        .click();
+      const dialog = page.getByRole("dialog");
+      const textbox = dialog.getByRole("textbox");
+      if (mode === "add") {
+        await textbox.fill(JSON.stringify({ mcpServers: { [name]: server } }));
+        // The duplicate-name validation message also contains user text.
+        await dialog.getByRole("button", { name: "Save", exact: true }).click();
+        await expect(dialog.getByRole("alert")).toBeVisible();
+      }
+
+      // Find the actual overflowing ancestor, so this also exercises the
+      // pre-fix dialog-level fallback rather than assuming a wrapper exists.
+      expect(
+        await textbox.evaluate((element) => {
+          for (
+            let parent = element.parentElement;
+            parent;
+            parent = parent.parentElement
+          ) {
+            if (
+              /auto|scroll/.test(getComputedStyle(parent).overflowY) &&
+              parent.scrollHeight > parent.clientHeight
+            ) {
+              parent.scrollTop = parent.scrollHeight;
+              return parent.scrollTop;
+            }
+          }
+          return 0;
+        }),
+      ).toBeGreaterThan(0);
+
+      const expectActionsVisible = async () => {
+        for (const element of [
+          dialog,
+          dialog.getByRole("heading"),
+          dialog.getByRole("button", { name: "Close", exact: true }),
+          dialog.getByRole("button", { name: "Save", exact: true }),
+          dialog.getByRole("button", { name: "Cancel", exact: true }),
+        ]) {
+          await expect(element).toBeInViewport({ ratio: 1 });
+        }
+      };
+      await expectActionsVisible();
+      await textbox.fill("{invalid");
+      await dialog.getByRole("button", { name: "Save", exact: true }).click();
+      await dialog.getByRole("alert").scrollIntoViewIfNeeded();
+      await expectActionsVisible();
+      await page.screenshot({
+        path: testInfo.outputPath(`${mode}-fallback.png`),
+      });
+      await dialog
+        .getByRole("button", {
+          name: mode === "edit" ? "Close" : "Cancel",
+          exact: true,
+        })
+        .click();
+      await expect(dialog).toBeHidden();
+    }
+  });
+}
+
+for (const viewport of [
+  { width: 1280, height: 720 },
+  { width: 390, height: 667 },
+  { width: 844, height: 390 },
+]) {
+  test(`long MCP definitions stay usable at ${viewport.width}x${viewport.height}`, async ({
+    page,
+  }, testInfo) => {
+    const errors: string[] = [];
+    page.on("pageerror", (error) => errors.push(error.message));
+    await page.setViewportSize(viewport);
+    mockLangGraphAPI(page);
+    const server = {
+      enabled: false,
+      description: "GitHub MCP server for repository operations",
+      type: "stdio",
+      command: "npx",
+      args: ["-y", "@modelcontextprotocol/server-github"],
+      env: Object.fromEntries(
+        Array.from({ length: 60 }, (_, index) => [`TEST_${index}`, "example"]),
+      ),
+    };
+    await page.route("**/api/mcp/config", (route) =>
+      route.fulfill({ json: { mcp_servers: { github: server } } }),
+    );
+    await page.goto("/workspace/capabilities");
+
+    for (const mode of ["edit", "add"] as const) {
+      await page
+        .getByRole("button", {
+          name: mode === "edit" ? "Edit github" : "Add MCP plugin",
+          exact: true,
+        })
+        .click();
+      const dialog = page.getByRole("dialog");
+      const textbox = dialog.getByRole("textbox");
+      if (mode === "add") {
+        await textbox.fill(
+          JSON.stringify({ mcpServers: { github: server } }, null, 2),
+        );
+      }
+      const expectWithinViewport = async () => {
+        for (const element of [
+          dialog,
+          dialog.getByRole("heading"),
+          textbox,
+          dialog.getByRole("button", { name: "Cancel", exact: true }),
+          dialog.getByRole("button", { name: "Save", exact: true }),
+          dialog.getByRole("button", { name: "Close", exact: true }),
+        ]) {
+          await expect(element).toBeInViewport({ ratio: 1 });
+        }
+      };
+      await expectWithinViewport();
+      expect(
+        await textbox.evaluate(
+          (element) => element.scrollHeight > element.clientHeight,
+        ),
+      ).toBe(true);
+      await textbox.hover();
+      await page.mouse.wheel(0, 10000);
+      await expect
+        .poll(() => textbox.evaluate((element) => element.scrollTop))
+        .toBeGreaterThan(0);
+      await expectWithinViewport();
+      await page.screenshot({ path: testInfo.outputPath(`${mode}.png`) });
+
+      await textbox.fill("{invalid");
+      await dialog.getByRole("button", { name: "Save", exact: true }).click();
+      await expect(dialog.getByRole("alert")).toBeVisible();
+      await expectWithinViewport();
+      await dialog.getByRole("button", { name: "Cancel", exact: true }).click();
+      await expect(dialog).toBeHidden();
+    }
+    expect(errors).toEqual([]);
+  });
+}
+
 test.describe("MCP server settings", () => {
   test("edits one server without dropping advanced fields or siblings", async ({
     page,