fix(settings): stabilize MCP permission layout
This commit is contained in:
ptma
GitHub
parent
727290fa32
commit
5fcebb5c08
|
|
@ -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<HTMLElement>('[role="radiogroup"]') : null;
|
||||
group?.querySelector<HTMLElement>(`[data-mcp-execution-mode="${nextMode}"]`)?.focus();
|
||||
onMcpExecutionModeChange(nextMode);
|
||||
}
|
||||
|
||||
function onMcpAllowedConnectionIdsChange(allowedConnectionIds: string[] | null) {
|
||||
void saveMcpPolicy({ allowedConnectionIds });
|
||||
}
|
||||
|
|
@ -4888,42 +4908,54 @@ onUnmounted(cleanupPreviewEditor);
|
|||
<Label id="mcp-execution-mode-label">{{ t("settings.mcpExecutionMode") }}</Label>
|
||||
<p class="text-xs text-muted-foreground">{{ t("settings.mcpExecutionModeDescription") }}</p>
|
||||
</div>
|
||||
<fieldset :disabled="mcpPolicyControlsDisabled" :aria-busy="mcpPolicyLoading" aria-labelledby="mcp-execution-mode-label">
|
||||
<legend class="sr-only">{{ t("settings.mcpExecutionMode") }}</legend>
|
||||
<div class="grid grid-cols-1 rounded-md bg-muted p-1 sm:grid-cols-3">
|
||||
<label
|
||||
:class="[
|
||||
'flex min-h-10 items-center justify-center rounded px-3 py-2 text-center text-sm font-medium transition-colors has-[:focus-visible]:ring-[3px] has-[:focus-visible]:ring-ring/50',
|
||||
mcpExecutionMode === 'read_only' ? 'bg-background text-foreground shadow-sm dark:bg-input/30' : 'text-muted-foreground',
|
||||
mcpPolicyControlsDisabled ? 'cursor-not-allowed opacity-50' : 'cursor-pointer hover:text-foreground',
|
||||
]"
|
||||
>
|
||||
<input class="sr-only" type="radio" name="mcp-execution-mode" value="read_only" :checked="mcpExecutionMode === 'read_only'" @click="onMcpExecutionModeChange($event, 'read_only')" />
|
||||
<span>{{ t("settings.mcpExecutionModeReadOnly") }}</span>
|
||||
</label>
|
||||
<label
|
||||
:class="[
|
||||
'flex min-h-10 items-center justify-center gap-1.5 rounded px-3 py-2 text-center text-sm font-medium transition-colors has-[:focus-visible]:ring-[3px] has-[:focus-visible]:ring-ring/50',
|
||||
mcpExecutionMode === 'safe_write' ? 'bg-background text-foreground shadow-sm dark:bg-input/30' : 'text-muted-foreground',
|
||||
mcpPolicyControlsDisabled ? 'cursor-not-allowed opacity-50' : 'cursor-pointer hover:text-foreground',
|
||||
]"
|
||||
>
|
||||
<input class="sr-only" type="radio" name="mcp-execution-mode" value="safe_write" :checked="mcpExecutionMode === 'safe_write'" @click="onMcpExecutionModeChange($event, 'safe_write')" />
|
||||
<span>{{ t("settings.mcpExecutionModeSafeWrite") }}</span>
|
||||
<span class="text-[10px] font-normal text-green-600 dark:text-green-400">{{ t("settings.mcpExecutionModeRecommended") }}</span>
|
||||
</label>
|
||||
<label
|
||||
:class="[
|
||||
'flex min-h-10 items-center justify-center rounded px-3 py-2 text-center text-sm font-medium transition-colors has-[:focus-visible]:ring-[3px] has-[:focus-visible]:ring-ring/50',
|
||||
mcpExecutionMode === 'high_risk_write' ? 'bg-background text-foreground shadow-sm dark:bg-input/30' : 'text-muted-foreground',
|
||||
mcpPolicyControlsDisabled ? 'cursor-not-allowed opacity-50' : 'cursor-pointer hover:text-foreground',
|
||||
]"
|
||||
>
|
||||
<input class="sr-only" type="radio" name="mcp-execution-mode" value="high_risk_write" :checked="mcpExecutionMode === 'high_risk_write'" @click="onMcpExecutionModeChange($event, 'high_risk_write')" />
|
||||
<span>{{ t("settings.mcpExecutionModeHighRiskWrite") }}</span>
|
||||
</label>
|
||||
</div>
|
||||
</fieldset>
|
||||
<div class="grid grid-cols-1 p-1 sm:grid-cols-3 gap-2.5" role="radiogroup" aria-labelledby="mcp-execution-mode-label">
|
||||
<Button
|
||||
:disabled="mcpPolicyControlsDisabled"
|
||||
type="button"
|
||||
role="radio"
|
||||
data-mcp-execution-mode="read_only"
|
||||
:aria-checked="mcpExecutionMode === 'read_only'"
|
||||
:tabindex="mcpExecutionMode === 'read_only' ? 0 : -1"
|
||||
variant="outline"
|
||||
class="settings-choice-card h-auto justify-center border p-3"
|
||||
:class="mcpExecutionMode === 'read_only' ? 'settings-choice-card--selected border-blue-300 ring-2 ring-blue-300/50' : ''"
|
||||
@click="onMcpExecutionModeChange('read_only')"
|
||||
@keydown="onMcpExecutionModeKeydown($event, 'read_only')"
|
||||
>
|
||||
<span>{{ t("settings.mcpExecutionModeReadOnly") }}</span>
|
||||
</Button>
|
||||
<Button
|
||||
:disabled="mcpPolicyControlsDisabled"
|
||||
type="button"
|
||||
role="radio"
|
||||
data-mcp-execution-mode="safe_write"
|
||||
:aria-checked="mcpExecutionMode === 'safe_write'"
|
||||
:tabindex="mcpExecutionMode === 'safe_write' ? 0 : -1"
|
||||
variant="outline"
|
||||
class="settings-choice-card h-auto justify-center border p-3"
|
||||
:class="mcpExecutionMode === 'safe_write' ? 'settings-choice-card--selected border-blue-300 ring-2 ring-blue-300/50' : ''"
|
||||
@click="onMcpExecutionModeChange('safe_write')"
|
||||
@keydown="onMcpExecutionModeKeydown($event, 'safe_write')"
|
||||
>
|
||||
<span>{{ t("settings.mcpExecutionModeSafeWrite") }}</span>
|
||||
<span class="text-[10px] font-normal text-green-600 dark:text-green-400">{{ t("settings.mcpExecutionModeRecommended") }}</span>
|
||||
</Button>
|
||||
<Button
|
||||
:disabled="mcpPolicyControlsDisabled"
|
||||
type="button"
|
||||
role="radio"
|
||||
data-mcp-execution-mode="high_risk_write"
|
||||
:aria-checked="mcpExecutionMode === 'high_risk_write'"
|
||||
:tabindex="mcpExecutionMode === 'high_risk_write' ? 0 : -1"
|
||||
variant="outline"
|
||||
class="settings-choice-card h-auto justify-center border p-3"
|
||||
:class="mcpExecutionMode === 'high_risk_write' ? 'settings-choice-card--selected border-blue-300 ring-2 ring-blue-300/50' : ''"
|
||||
@click="onMcpExecutionModeChange('high_risk_write')"
|
||||
@keydown="onMcpExecutionModeKeydown($event, 'high_risk_write')"
|
||||
>
|
||||
<span>{{ t("settings.mcpExecutionModeHighRiskWrite") }}</span>
|
||||
</Button>
|
||||
</div>
|
||||
<!-- Keep every translation in one grid cell so mode changes cannot reflow the capability matrix. -->
|
||||
<div data-mcp-execution-mode-description class="grid text-xs">
|
||||
<p class="col-start-1 row-start-1 text-muted-foreground" :class="mcpExecutionMode === 'read_only' ? 'visible' : 'invisible'">
|
||||
|
|
|
|||
|
|
@ -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<HTMLElement>('[role="radiogroup"]') : null;
|
||||
group?.querySelector<HTMLElement>(`[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], () => {
|
|||
<div class="flex flex-wrap items-start justify-between gap-3">
|
||||
<div class="min-w-0 space-y-1">
|
||||
<div class="flex flex-wrap items-center gap-2">
|
||||
<div class="text-sm font-medium">{{ t("settings.mcpScopeConnection") }}</div>
|
||||
<div :id="`${pickerId}-mode-label`" class="text-sm font-medium">{{ t("settings.mcpScopeConnection") }}</div>
|
||||
<Badge variant="outline" class="rounded-md font-normal">{{ allowedSummary }}</Badge>
|
||||
</div>
|
||||
<p class="text-xs text-muted-foreground">{{ t("settings.mcpScopeConnectionDescription") }}</p>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<fieldset :disabled="disabled" class="grid grid-cols-2 rounded-md bg-muted p-1" :aria-label="t('settings.mcpScopeMode')">
|
||||
<label
|
||||
:class="[
|
||||
'flex min-h-11 cursor-pointer flex-col items-center justify-center rounded px-2 py-1.5 text-center transition-colors has-[:focus-visible]:ring-[3px] has-[:focus-visible]:ring-ring/50',
|
||||
scopeMode === 'all' ? 'bg-background text-foreground shadow-sm dark:bg-input/30' : 'text-muted-foreground hover:text-foreground',
|
||||
disabled ? 'cursor-not-allowed opacity-50' : '',
|
||||
]"
|
||||
<div class="grid grid-cols-1 p-1 sm:grid-cols-2 gap-2.5" role="radiogroup" :aria-labelledby="`${pickerId}-mode-label`">
|
||||
<Button
|
||||
:disabled="disabled"
|
||||
type="button"
|
||||
role="radio"
|
||||
data-scope-mode="all"
|
||||
:aria-checked="scopeMode === 'all'"
|
||||
:tabindex="scopeMode === 'all' ? 0 : -1"
|
||||
variant="outline"
|
||||
class="settings-choice-card h-auto justify-center border p-3"
|
||||
:class="[scopeMode === 'all' ? 'settings-choice-card--selected border-blue-300 ring-2 ring-blue-300/50' : '', disabled ? 'cursor-not-allowed opacity-50' : '']"
|
||||
@click="setScopeMode('all')"
|
||||
@keydown="onScopeModeKeydown($event, 'all')"
|
||||
>
|
||||
<input class="sr-only" type="radio" name="mcp-scope-mode" value="all" :checked="scopeMode === 'all'" @change="setScopeMode('all')" />
|
||||
<span class="text-sm font-medium">{{ t("settings.mcpScopeModeAll") }}</span>
|
||||
<span class="mcp-scope-mode-description text-[11px] leading-tight text-muted-foreground">{{ t("settings.mcpScopeModeAllDescription") }}</span>
|
||||
</label>
|
||||
<label
|
||||
:class="[
|
||||
'flex min-h-11 cursor-pointer flex-col items-center justify-center rounded px-2 py-1.5 text-center transition-colors has-[:focus-visible]:ring-[3px] has-[:focus-visible]:ring-ring/50',
|
||||
scopeMode === 'selected' ? 'bg-background text-foreground shadow-sm dark:bg-input/30' : 'text-muted-foreground hover:text-foreground',
|
||||
disabled ? 'cursor-not-allowed opacity-50' : '',
|
||||
]"
|
||||
<div class="w-full min-w-0 text-center">
|
||||
<div class="text-sm font-medium">{{ t("settings.mcpScopeModeAll") }}</div>
|
||||
<div class="text-xs text-muted-foreground truncate">{{ t("settings.mcpScopeModeAllDescription") }}</div>
|
||||
</div>
|
||||
</Button>
|
||||
<Button
|
||||
:disabled="disabled"
|
||||
type="button"
|
||||
role="radio"
|
||||
data-scope-mode="selected"
|
||||
:aria-checked="scopeMode === 'selected'"
|
||||
:tabindex="scopeMode === 'selected' ? 0 : -1"
|
||||
variant="outline"
|
||||
class="settings-choice-card h-auto justify-center border p-3"
|
||||
:class="[scopeMode === 'selected' ? 'settings-choice-card--selected border-blue-300 ring-2 ring-blue-300/50' : '', disabled ? 'cursor-not-allowed opacity-50' : '']"
|
||||
@click="setScopeMode('selected')"
|
||||
@keydown="onScopeModeKeydown($event, 'selected')"
|
||||
>
|
||||
<input class="sr-only" type="radio" name="mcp-scope-mode" value="selected" :checked="scopeMode === 'selected'" @change="setScopeMode('selected')" />
|
||||
<span class="text-sm font-medium">{{ t("settings.mcpScopeModeSelected") }}</span>
|
||||
<span class="mcp-scope-mode-description text-[11px] leading-tight text-muted-foreground">{{ t("settings.mcpScopeModeSelectedDescription") }}</span>
|
||||
</label>
|
||||
</fieldset>
|
||||
<div class="w-full min-w-0 text-center">
|
||||
<div class="text-sm font-medium">{{ t("settings.mcpScopeModeSelected") }}</div>
|
||||
<div class="text-xs text-muted-foreground truncate">{{ t("settings.mcpScopeModeSelectedDescription") }}</div>
|
||||
</div>
|
||||
</Button>
|
||||
</div>
|
||||
|
||||
<div class="relative">
|
||||
<Search class="pointer-events-none absolute left-2.5 top-1/2 h-4 w-4 -translate-y-1/2 text-muted-foreground" />
|
||||
|
|
|
|||
|
|
@ -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")],
|
||||
|
|
|
|||
|
|
@ -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('<fieldset :disabled="mcpPolicyControlsDisabled"');
|
||||
|
||||
const loadingStart = settingsDialogSource.indexOf("mcpPolicyLoading.value = true;");
|
||||
const policyLoad = settingsDialogSource.indexOf("await settingsStore.initMcpGlobalPolicy(true);");
|
||||
|
|
@ -103,6 +102,13 @@ describe("MCP policy settings state", () => {
|
|||
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", () => {
|
||||
|
|
|
|||
Loading…
Reference in New Issue