From 5fcebb5c0845d8449a0fbbb84d9c74c6e922ffe1 Mon Sep 17 00:00:00 2001 From: ptma Date: Tue, 21 Jul 2026 16:59:02 +0800 Subject: [PATCH] fix(settings): stabilize MCP permission layout --- .../editor/EditorSettingsDialog.vue | 108 ++++++++++++------ .../settings/McpConnectionScopePicker.vue | 73 ++++++++---- .../McpConnectionScopePicker.spec.ts | 28 ++++- .../__tests__/mcp/mcpPolicySelection.spec.ts | 8 +- 4 files changed, 151 insertions(+), 66 deletions(-) diff --git a/apps/desktop/src/components/editor/EditorSettingsDialog.vue b/apps/desktop/src/components/editor/EditorSettingsDialog.vue index b93f33730..a012b5fd1 100644 --- a/apps/desktop/src/components/editor/EditorSettingsDialog.vue +++ b/apps/desktop/src/components/editor/EditorSettingsDialog.vue @@ -1430,6 +1430,7 @@ const mcpInstalling = ref(false); const mcpInstallMessage = ref(""); const mcpInstallError = ref(false); const mcpExecutionMode = computed(() => mcpExecutionModeFromPolicy(settingsStore.mcpGlobalPolicy)); +const mcpExecutionModeOptions: McpExecutionMode[] = ["read_only", "safe_write", "high_risk_write"]; const mcpAllowedConnectionIds = computed(() => settingsStore.mcpGlobalPolicy.allowedConnectionIds); const mcpSelectableConnections = computed(() => connectionStore.connections); const mcpPolicyControlsDisabled = computed(() => @@ -1452,15 +1453,34 @@ async function saveMcpPolicy(partial: { readOnly?: boolean; allowDangerousSql?: } } -function onMcpExecutionModeChange(event: Event, mode: McpExecutionMode) { +function onMcpExecutionModeChange(mode: McpExecutionMode) { if (mode === mcpExecutionMode.value) return; if (mode === "high_risk_write" && !window.confirm(t("settings.mcpExecutionModeHighRiskConfirm"))) { - event.preventDefault(); return; } void saveMcpPolicy(mcpPolicyFieldsForExecutionMode(mode)); } +function onMcpExecutionModeKeydown(event: KeyboardEvent, mode: McpExecutionMode) { + if (mcpPolicyControlsDisabled.value) return; + const currentIndex = mcpExecutionModeOptions.indexOf(mode); + let nextIndex: number | undefined; + if (event.key === "Home") nextIndex = 0; + else if (event.key === "End") nextIndex = mcpExecutionModeOptions.length - 1; + else if (event.key === "ArrowRight" || event.key === "ArrowDown") nextIndex = (currentIndex + 1) % mcpExecutionModeOptions.length; + else if (event.key === "ArrowLeft" || event.key === "ArrowUp") nextIndex = (currentIndex - 1 + mcpExecutionModeOptions.length) % mcpExecutionModeOptions.length; + if (nextIndex === undefined || nextIndex === currentIndex) return; + + event.preventDefault(); + event.stopPropagation(); + const nextMode = mcpExecutionModeOptions[nextIndex]; + // These cards visually replace native radios, so preserve the radio-group keyboard contract. + const currentTarget = event.currentTarget; + const group = currentTarget instanceof HTMLElement ? currentTarget.closest('[role="radiogroup"]') : null; + group?.querySelector(`[data-mcp-execution-mode="${nextMode}"]`)?.focus(); + onMcpExecutionModeChange(nextMode); +} + function onMcpAllowedConnectionIdsChange(allowedConnectionIds: string[] | null) { void saveMcpPolicy({ allowedConnectionIds }); } @@ -4888,42 +4908,54 @@ onUnmounted(cleanupPreviewEditor);

{{ t("settings.mcpExecutionModeDescription") }}

-
- {{ t("settings.mcpExecutionMode") }} -
- - - -
-
+
+ + + +

diff --git a/apps/desktop/src/components/settings/McpConnectionScopePicker.vue b/apps/desktop/src/components/settings/McpConnectionScopePicker.vue index b6b2a255f..c158fd702 100644 --- a/apps/desktop/src/components/settings/McpConnectionScopePicker.vue +++ b/apps/desktop/src/components/settings/McpConnectionScopePicker.vue @@ -75,6 +75,19 @@ function setScopeMode(mode: ScopeMode) { emitAllowedConnectionIds(mode === "all" ? null : [...connectionIds.value]); } +function onScopeModeKeydown(event: KeyboardEvent, mode: ScopeMode) { + if (props.disabled || !["ArrowLeft", "ArrowRight", "ArrowUp", "ArrowDown", "Home", "End"].includes(event.key)) return; + event.preventDefault(); + event.stopPropagation(); + const nextMode: ScopeMode = event.key === "Home" || event.key === "ArrowLeft" || event.key === "ArrowUp" ? "all" : "selected"; + if (nextMode === mode) return; + // These cards visually replace native radios, so preserve the radio-group keyboard contract. + const currentTarget = event.currentTarget; + const group = currentTarget instanceof HTMLElement ? currentTarget.closest('[role="radiogroup"]') : null; + group?.querySelector(`[data-scope-mode="${nextMode}"]`)?.focus(); + setScopeMode(nextMode); +} + function restorePendingFocus() { const pending = pendingFocus.value; if (!pending || props.busy) return; @@ -129,37 +142,51 @@ watch([policyKey, () => groups.value.allowed.length], () => {

-
{{ t("settings.mcpScopeConnection") }}
+
{{ t("settings.mcpScopeConnection") }}
{{ allowedSummary }}

{{ t("settings.mcpScopeConnectionDescription") }}

-
-
+ + +
diff --git a/apps/desktop/src/components/settings/__tests__/McpConnectionScopePicker.spec.ts b/apps/desktop/src/components/settings/__tests__/McpConnectionScopePicker.spec.ts index e1d5e341a..798c5c05b 100644 --- a/apps/desktop/src/components/settings/__tests__/McpConnectionScopePicker.spec.ts +++ b/apps/desktop/src/components/settings/__tests__/McpConnectionScopePicker.spec.ts @@ -66,16 +66,36 @@ describe("McpConnectionScopePicker", () => { allowedConnectionIds: null, "onUpdate:allowedConnectionIds": update, }); - const modeInputs = findAll(mounted.root, (node) => node.type === "input" && node.props.name === "mcp-scope-mode"); + const modeButtons = findAll(mounted.root, (node) => { + return node.type === "button" && node.props["class"]?.includes("settings-choice-card"); + }); - expect(modeInputs.find((input) => input.props.value === "all")?.props.checked).toBe(true); + expect(modeButtons.find((node) => node.props["data-scope-mode"] === "all")?.props["class"]?.includes("settings-choice-card--selected")).toBe(true); + expect(modeButtons.find((node) => node.props["data-scope-mode"] === "all")?.props.role).toBe("radio"); + expect(modeButtons.find((node) => node.props["data-scope-mode"] === "all")?.props["aria-checked"]).toBe(true); + expect(modeButtons.find((node) => node.props["data-scope-mode"] === "selected")?.props.tabindex).toBe(-1); dispatch( - findOne(mounted.root, (node) => node.type === "input" && node.props.value === "selected"), - "change", + findOne(mounted.root, (node) => node.type === "button" && node.props["data-scope-mode"] === "selected"), + "click", ); expect(update).toHaveBeenCalledWith(["one", "two"]); }); + it("supports radio-group arrow-key selection", () => { + const update = vi.fn(); + const mounted = mountComponent(McpConnectionScopePicker, { + connections: [connection("one"), connection("two")], + allowedConnectionIds: null, + "onUpdate:allowedConnectionIds": update, + }); + const allMode = findOne(mounted.root, (node) => node.type === "button" && node.props["data-scope-mode"] === "all"); + + const event = dispatch(allMode, "keydown", { key: "ArrowRight" }); + + expect(event.defaultPrevented).toBe(true); + expect(update).toHaveBeenCalledWith(["one", "two"]); + }); + it("shows unavailable allowlist entries only in the allowed pane", () => { const mounted = mountComponent(McpConnectionScopePicker, { connections: [connection("one")], diff --git a/apps/desktop/src/lib/__tests__/mcp/mcpPolicySelection.spec.ts b/apps/desktop/src/lib/__tests__/mcp/mcpPolicySelection.spec.ts index a72b0249d..cac05360f 100644 --- a/apps/desktop/src/lib/__tests__/mcp/mcpPolicySelection.spec.ts +++ b/apps/desktop/src/lib/__tests__/mcp/mcpPolicySelection.spec.ts @@ -82,7 +82,6 @@ describe("MCP policy settings state", () => { expect(settingsDialogSource).toContain("if (mcpPolicyControlsDisabled.value) return;"); expect(settingsDialogSource).toContain(':disabled="mcpPolicyControlsDisabled"'); expect(settingsDialogSource).toContain('@update:allowed-connection-ids="onMcpAllowedConnectionIdsChange"'); - expect(settingsDialogSource).toContain('
{ expect(descriptionSource.match(/col-start-1 row-start-1/g)).toHaveLength(3); expect(descriptionSource.match(/\? 'visible' : 'invisible'/g)).toHaveLength(3); }); + + it("keeps execution mode cards accessible as a keyboard radio group", () => { + expect(settingsDialogSource).toContain('role="radiogroup" aria-labelledby="mcp-execution-mode-label"'); + expect(settingsDialogSource.match(/role="radio"/g)).toHaveLength(3); + expect(settingsDialogSource).toContain(":aria-checked=\"mcpExecutionMode === 'safe_write'\""); + expect(settingsDialogSource).toContain("onMcpExecutionModeKeydown($event, 'safe_write')"); + }); }); describe("MCP connection search", () => {