fix(dashboard): isolate MCP settings scopes (#2116)
## Summary - bind Global and Project MCP editors to their raw values from `/api/settings/scopes` - keep edits isolated in the owning scope instead of mutating the merged project-effective form - persist changed MCP scopes independently of which Settings section is visible when Save is pressed - add unit and SettingsModal regressions for opposite scope values and edit → navigate → save ## Root cause `SettingsModal` passed the merged project-effective `form.mcpServers` to both MCP sections. A project override could therefore appear as the Global MCP value. Saving the apparent global change could then be dropped as a no-op when compared with the actual global-scoped value. The first fix still tied save routing to the active section. The follow-up carries both raw scoped MCP values through `splitSettingsSave`, where changed-only comparisons persist each owning scope even after navigation. ## Testing - `pnpm exec vitest run --project dashboard-app app/__tests__/settings-save-split.test.ts` (32 passed on PR branch) - `pnpm exec vitest run --project dashboard-app-quality-settings app/components/__tests__/SettingsModal.general.test.tsx` (84 passed) - `pnpm run typecheck` - `pnpm exec eslint app/components/SettingsModal.tsx app/components/settings/save-split.ts app/__tests__/settings-save-split.test.ts app/components/__tests__/SettingsModal.general.test.tsx` - `pnpm run build` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed MCP settings so global and project configurations remain correctly separated. * Prevented inherited global settings from appearing as project overrides. * Preserved MCP edits when navigating between Settings sections. * Ensured unchanged settings are not unnecessarily saved. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Phil Larson
7
.changeset/mcp-settings-scope-state.md
Normal file
7
.changeset/mcp-settings-scope-state.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Keep Global and Project MCP settings bound to their own scopes in the Settings UI.
|
||||
category: fix
|
||||
dev: SettingsModal now reads and edits MCP server configuration from the raw scoped settings response rather than the merged project-effective form, and save splitting persists changed MCP scopes independently of the currently visible section. This prevents project MCP overrides from appearing as global values, making global saves no-op, or losing edits after navigation.
|
||||
@@ -15,7 +15,7 @@
|
||||
*/
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { isGlobalSettingsKey, isProjectSettingsKey } from "@fusion/core";
|
||||
import { splitSettingsSave, MODEL_LANE_KEYS } from "../components/settings/save-split";
|
||||
import { resolveScopedMcpSettings, splitSettingsSave, MODEL_LANE_KEYS } from "../components/settings/save-split";
|
||||
|
||||
// Sanity-anchor the scope of the concrete keys this test relies on, so the
|
||||
// assertions below remain meaningful if core's catalog ever shifts.
|
||||
@@ -47,6 +47,36 @@ describe("scope anchors", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveScopedMcpSettings", () => {
|
||||
const globalServer = { name: "global-docs", transport: "stdio", command: "global-mcp" } as const;
|
||||
const projectServer = { name: "deepwiki", transport: "stdio", command: "project-mcp" } as const;
|
||||
const scopedSettings = {
|
||||
global: { mcpServers: { enabled: false, servers: [globalServer] } },
|
||||
project: { mcpServers: { enabled: true, servers: [projectServer] } },
|
||||
} as never;
|
||||
|
||||
it("shows raw global MCP state instead of the merged project-effective state", () => {
|
||||
expect(resolveScopedMcpSettings("global", scopedSettings)).toEqual({
|
||||
enabled: false,
|
||||
servers: [globalServer],
|
||||
});
|
||||
});
|
||||
|
||||
it("shows raw project MCP state without duplicating inherited global servers", () => {
|
||||
expect(resolveScopedMcpSettings("project", scopedSettings)).toEqual({
|
||||
enabled: true,
|
||||
servers: [projectServer],
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves an absent project override so global MCP settings remain inherited", () => {
|
||||
expect(resolveScopedMcpSettings("project", {
|
||||
global: { mcpServers: { enabled: true, servers: [globalServer] } },
|
||||
project: {},
|
||||
} as never)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("splitSettingsSave", () => {
|
||||
it("routes one global + one project edit into the right patches", () => {
|
||||
const initialValues = { language: "en", maxConcurrent: 2 } as never;
|
||||
@@ -393,6 +423,68 @@ describe("splitSettingsSave", () => {
|
||||
expect(projectResult.projectPatch).toEqual({ mcpServers: projectMcp });
|
||||
});
|
||||
|
||||
it("persists changed MCP scopes after navigating away from the MCP sections", () => {
|
||||
const initialGlobalMcp = { enabled: false, servers: [] } as const;
|
||||
const initialProjectMcp = { enabled: true, servers: [{ name: "deepwiki", transport: "stdio", command: "docs" }] } as const;
|
||||
const nextGlobalMcp = { enabled: true, servers: [{ name: "global-docs", transport: "stdio", command: "docs" }] } as const;
|
||||
const nextProjectMcp = { enabled: false, servers: [{ name: "deepwiki", transport: "stdio", command: "docs" }] } as const;
|
||||
|
||||
const { globalPatch, projectPatch } = splitSettingsSave({
|
||||
payload: { mcpServers: initialProjectMcp, language: "en" },
|
||||
initialValues: { language: "en", mcpServers: initialProjectMcp } as never,
|
||||
initialScopedValues: {
|
||||
global: { mcpServers: initialGlobalMcp },
|
||||
project: { mcpServers: initialProjectMcp },
|
||||
} as never,
|
||||
activeSection: "global-general",
|
||||
scopedMcpValues: {
|
||||
global: nextGlobalMcp,
|
||||
project: nextProjectMcp,
|
||||
},
|
||||
});
|
||||
|
||||
expect(globalPatch).toEqual({ mcpServers: nextGlobalMcp });
|
||||
expect(projectPatch).toEqual({ mcpServers: nextProjectMcp });
|
||||
});
|
||||
|
||||
it("does not materialize inherited global MCP settings as a project override on a no-op save", () => {
|
||||
const globalMcp = { enabled: true, servers: [{ name: "global-docs", transport: "stdio", command: "docs" }] } as const;
|
||||
const { globalPatch, projectPatch } = splitSettingsSave({
|
||||
payload: { language: "en" },
|
||||
initialValues: { language: "en", mcpServers: globalMcp } as never,
|
||||
initialScopedValues: {
|
||||
global: { mcpServers: globalMcp },
|
||||
project: {},
|
||||
} as never,
|
||||
activeSection: "general",
|
||||
scopedMcpValues: {
|
||||
global: globalMcp,
|
||||
project: undefined,
|
||||
},
|
||||
});
|
||||
|
||||
expect(globalPatch).toEqual({});
|
||||
expect(projectPatch).toEqual({});
|
||||
});
|
||||
|
||||
it("persists scoped MCP edits when the initial scoped snapshot is unavailable", () => {
|
||||
const globalMcp = { enabled: true, servers: [{ name: "global-docs", transport: "stdio", command: "docs" }] } as const;
|
||||
const projectMcp = { enabled: true, servers: [{ name: "project-docs", transport: "stdio", command: "project-docs" }] } as const;
|
||||
const { globalPatch, projectPatch } = splitSettingsSave({
|
||||
payload: {},
|
||||
initialValues: null,
|
||||
initialScopedValues: null,
|
||||
activeSection: "general",
|
||||
scopedMcpValues: {
|
||||
global: globalMcp,
|
||||
project: projectMcp,
|
||||
},
|
||||
});
|
||||
|
||||
expect(globalPatch).toEqual({ mcpServers: globalMcp });
|
||||
expect(projectPatch).toEqual({ mcpServers: projectMcp });
|
||||
});
|
||||
|
||||
it("maps flattened remote access fields to the canonical global remoteAccess patch", () => {
|
||||
const { globalPatch, projectPatch } = splitSettingsSave({
|
||||
payload: {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useState, useEffect, useCallback, useMemo, useRef, type CSSProperties, type KeyboardEvent as ReactKeyboardEvent, type MouseEvent, type PointerEvent as ReactPointerEvent } from "react";
|
||||
import { useState, useEffect, useCallback, useMemo, useRef, type CSSProperties, type Dispatch, type KeyboardEvent as ReactKeyboardEvent, type MouseEvent, type PointerEvent as ReactPointerEvent, type SetStateAction } from "react";
|
||||
import { Globe, Folder, RefreshCw, Star, HelpCircle, Settings as SettingsIcon, Search, X as SearchToggleCloseIcon } from "lucide-react";
|
||||
import {
|
||||
getErrorMessage,
|
||||
@@ -10,7 +10,7 @@ import type { Settings, GlobalSettings, ThemeMode, ColorTheme, ModelPreset } fro
|
||||
import { DEFAULT_GLOBAL_SETTINGS } from "@fusion/core";
|
||||
import { fetchSettings, fetchSettingsByScope, updateSettings, updateGlobalSettings, fetchAuthStatus, loginProvider, logoutProvider, cancelProviderLogin, saveApiKey, clearApiKey, fetchModels, testNotification, fetchBackups, createBackup, exportSettings, importSettings, fetchMemoryFile, fetchMemoryFiles, saveMemoryFile, compactMemory, fetchGlobalConcurrency, updateGlobalConcurrency, installQmd, testMemoryRetrieval, triggerMemoryDreams, fetchGitRemotes, fetchGitRemotesDetailed, fetchGitBranches, fetchProjects, fetchDashboardHealth, checkForUpdates, installUpdate, fetchRemoteSettings, fetchRemoteStatus, installCloudflared, fetchRemoteQr, fetchRemoteUrl, submitProviderManualCode } from "../api";
|
||||
import type { AuthProvider, ManualOAuthCodeInfo, ModelInfo, BackupListResponse, SettingsExportData, MemoryFileInfo, MemoryRetrievalTestResult, GitRemote, GitRemoteDetailed, ProjectInfo, RemoteStatus, UpdateCheckResponse, UpdateInstallResponse, OAuthDeviceCodeInfo } from "../api";
|
||||
import { splitSettingsSave } from "./settings/save-split";
|
||||
import { resolveScopedMcpSettings, splitSettingsSave, type McpSettingsScope } from "./settings/save-split";
|
||||
import {
|
||||
ALL_PROJECT_RESET_KEYS,
|
||||
getResetIneligibleReason,
|
||||
@@ -1045,6 +1045,30 @@ export function SettingsModal({
|
||||
const [globalGitlabSettings, setGlobalGitlabSettings] = useState<GlobalGitlabSettings | null>(null);
|
||||
// Track initial scoped values for null-as-delete semantics on project overrides
|
||||
const [initialScopedValues, setInitialScopedValues] = useState<{ global: GlobalSettings; project: Partial<Settings> } | null>(null);
|
||||
const mcpFormForScope = useCallback((scope: McpSettingsScope): Settings => ({
|
||||
...form,
|
||||
mcpServers: resolveScopedMcpSettings(scope, scopedSettings),
|
||||
}), [form, scopedSettings]);
|
||||
const setMcpFormForScope = useCallback((scope: McpSettingsScope): Dispatch<SetStateAction<Settings>> => (update) => {
|
||||
setScopedSettings((current) => {
|
||||
if (!current) return current;
|
||||
const currentForm = {
|
||||
...form,
|
||||
mcpServers: resolveScopedMcpSettings(scope, current),
|
||||
};
|
||||
const nextForm = typeof update === "function" ? update(currentForm) : update;
|
||||
if (scope === "global") {
|
||||
return {
|
||||
...current,
|
||||
global: { ...current.global, mcpServers: nextForm.mcpServers },
|
||||
};
|
||||
}
|
||||
return {
|
||||
...current,
|
||||
project: { ...current.project, mcpServers: nextForm.mcpServers },
|
||||
};
|
||||
});
|
||||
}, [form]);
|
||||
// Find the first non-group-header section for visibility fallback handling
|
||||
const firstNonHeaderSection = SETTINGS_SECTIONS.find((s) => !s.isGroupHeader);
|
||||
const [activeSection, setActiveSection] = useState<SectionId>(() => {
|
||||
@@ -3109,6 +3133,10 @@ export function SettingsModal({
|
||||
initialValues,
|
||||
initialScopedValues,
|
||||
activeSection,
|
||||
scopedMcpValues: scopedSettings ? {
|
||||
global: resolveScopedMcpSettings("global", scopedSettings),
|
||||
project: resolveScopedMcpSettings("project", scopedSettings),
|
||||
} : undefined,
|
||||
});
|
||||
|
||||
// Save both scopes in parallel if they have changes.
|
||||
@@ -3133,7 +3161,7 @@ export function SettingsModal({
|
||||
} finally {
|
||||
setIsSaving(false);
|
||||
}
|
||||
}, [form, globalGitlabSettings, globalMaxConcurrent, prefixError, presetDraft, initialValues, initialScopedValues, onClose, addToast, projectId, activeSection, isSaving, t]);
|
||||
}, [form, globalGitlabSettings, globalMaxConcurrent, prefixError, presetDraft, initialValues, initialScopedValues, scopedSettings, onClose, addToast, projectId, activeSection, isSaving, t]);
|
||||
|
||||
/*
|
||||
FNXC:SettingsReset 2026-07-04-00:25:
|
||||
@@ -3505,8 +3533,8 @@ export function SettingsModal({
|
||||
return (
|
||||
<GlobalMcpSection
|
||||
scopeBanner={renderScopeBanner()}
|
||||
form={form}
|
||||
setForm={setForm}
|
||||
form={mcpFormForScope("global")}
|
||||
setForm={setMcpFormForScope("global")}
|
||||
projectId={projectId}
|
||||
addToast={addToast}
|
||||
/>
|
||||
@@ -3515,8 +3543,8 @@ export function SettingsModal({
|
||||
return (
|
||||
<ProjectMcpSection
|
||||
scopeBanner={renderScopeBanner()}
|
||||
form={form}
|
||||
setForm={setForm}
|
||||
form={mcpFormForScope("project")}
|
||||
setForm={setMcpFormForScope("project")}
|
||||
globalSettings={scopedSettings?.global ?? null}
|
||||
projectId={projectId}
|
||||
addToast={addToast}
|
||||
|
||||
@@ -198,6 +198,74 @@ vi.mock("../FileBrowser", () => ({
|
||||
describe("SettingsModal", () => {
|
||||
installSettingsModalEnv();
|
||||
|
||||
const deepwikiServer = {
|
||||
name: "deepwiki",
|
||||
transport: "stdio" as const,
|
||||
command: "npx",
|
||||
args: ["-y", "mcp-remote", "https://mcp.deepwiki.com/sse"],
|
||||
};
|
||||
|
||||
it("binds Global MCP controls to raw global settings instead of the merged project value", async () => {
|
||||
mockFetchSettings.mockResolvedValue({
|
||||
...defaultSettings,
|
||||
mcpServers: { enabled: true, servers: [deepwikiServer] },
|
||||
});
|
||||
mockFetchSettingsByScope.mockResolvedValue({
|
||||
global: { ...defaultSettings, mcpServers: { enabled: false, servers: [] } },
|
||||
project: { mcpServers: { enabled: true, servers: [deepwikiServer] } },
|
||||
});
|
||||
|
||||
renderModal({ initialSection: "global-mcp", projectId: "proj-1" });
|
||||
await waitForSettingsModalReady();
|
||||
|
||||
expect(screen.getByRole("checkbox", { name: /Enable MCP servers for this scope/i })).not.toBeChecked();
|
||||
expect(screen.queryByTestId("mcp-server-row-deepwiki")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("binds Project MCP controls to raw project settings", async () => {
|
||||
mockFetchSettings.mockResolvedValue({
|
||||
...defaultSettings,
|
||||
mcpServers: { enabled: true, servers: [deepwikiServer] },
|
||||
});
|
||||
mockFetchSettingsByScope.mockResolvedValue({
|
||||
global: { ...defaultSettings, mcpServers: { enabled: false, servers: [] } },
|
||||
project: { mcpServers: { enabled: true, servers: [deepwikiServer] } },
|
||||
});
|
||||
|
||||
renderModal({ initialSection: "mcp", projectId: "proj-1" });
|
||||
await waitForSettingsModalReady();
|
||||
|
||||
expect(screen.getByRole("checkbox", { name: /Enable MCP servers for this scope/i })).toBeChecked();
|
||||
expect(await screen.findByTestId("mcp-server-row-deepwiki")).toHaveTextContent("project local");
|
||||
});
|
||||
|
||||
it("persists a scoped MCP edit after navigating to another section before saving", async () => {
|
||||
mockFetchSettings.mockResolvedValue({
|
||||
...defaultSettings,
|
||||
mcpServers: { enabled: true, servers: [deepwikiServer] },
|
||||
});
|
||||
mockFetchSettingsByScope.mockResolvedValue({
|
||||
global: { ...defaultSettings, mcpServers: { enabled: false, servers: [] } },
|
||||
project: { mcpServers: { enabled: true, servers: [deepwikiServer] } },
|
||||
});
|
||||
|
||||
renderModal({ initialSection: "mcp", projectId: "proj-1" });
|
||||
await waitForSettingsModalReady();
|
||||
|
||||
await settingsModalUser.click(screen.getByRole("checkbox", { name: /Enable MCP servers for this scope/i }));
|
||||
await settingsModalUser.click(screen.getByRole("button", { name: /^General$/ }));
|
||||
await settingsModalUser.click(screen.getByRole("button", { name: "Save" }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockUpdateSettings).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
mcpServers: { enabled: false, servers: [deepwikiServer] },
|
||||
}),
|
||||
"proj-1",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it("applies keyboard CSS variables when mobile keyboard is open", async () => {
|
||||
mockUseMobileKeyboard.mockReturnValue({
|
||||
keyboardOpen: true,
|
||||
|
||||
@@ -26,7 +26,7 @@
|
||||
* the actual `updateGlobalSettings`/`updateSettings` writes.
|
||||
*/
|
||||
import { isGlobalSettingsKey, isProjectSettingsKey } from "@fusion/core";
|
||||
import type { GlobalSettings, Settings } from "@fusion/core";
|
||||
import type { GlobalSettings, McpServersSettings, Settings } from "@fusion/core";
|
||||
|
||||
/**
|
||||
* Project-scoped model-override keys whose overrides track inheritance
|
||||
@@ -225,8 +225,10 @@ export interface SaveSplitInput {
|
||||
initialValues: Settings | null;
|
||||
/** Initial scoped values, used to detect changed/cleared project overrides. */
|
||||
initialScopedValues: { global: GlobalSettings; project: Partial<Settings> } | null;
|
||||
/** The active section id; gates where `githubTrackingDefaultRepo` is written. */
|
||||
/** The active section id; gates where section-owned values are written. */
|
||||
activeSection: string;
|
||||
/** Current raw MCP values for both scopes, preserved even after section navigation. */
|
||||
scopedMcpValues?: { global: McpServersSettings | undefined; project: McpServersSettings | undefined };
|
||||
}
|
||||
|
||||
export interface SaveSplitResult {
|
||||
@@ -234,6 +236,24 @@ export interface SaveSplitResult {
|
||||
projectPatch: Partial<Settings>;
|
||||
}
|
||||
|
||||
export type McpSettingsScope = "global" | "project";
|
||||
export type ScopedSettingsValues = { global: GlobalSettings; project: Partial<Settings> };
|
||||
|
||||
/**
|
||||
* Return the raw MCP value owned by one settings scope.
|
||||
*
|
||||
* FNXC:McpSettingsScopes 2026-07-14-21:59:
|
||||
* SettingsModal's general form is project-effective, so MCP editing and saving must use the raw values returned by `/api/settings/scopes`. Preserve `undefined` for an absent project override: normalizing it to `{ enabled: false, servers: [] }` would replace global inheritance with an explicit disabled project setting on a no-op save.
|
||||
*/
|
||||
export function resolveScopedMcpSettings(
|
||||
scope: McpSettingsScope,
|
||||
scopedSettings: ScopedSettingsValues | null,
|
||||
): McpServersSettings | undefined {
|
||||
return scope === "global"
|
||||
? scopedSettings?.global.mcpServers
|
||||
: scopedSettings?.project.mcpServers;
|
||||
}
|
||||
|
||||
function hasOwn(obj: object | null | undefined, key: string): boolean {
|
||||
return !!obj && Object.prototype.hasOwnProperty.call(obj, key);
|
||||
}
|
||||
@@ -356,6 +376,7 @@ export function splitSettingsSave({
|
||||
initialValues,
|
||||
initialScopedValues,
|
||||
activeSection,
|
||||
scopedMcpValues,
|
||||
}: SaveSplitInput): SaveSplitResult {
|
||||
const globalPatch: Partial<GlobalSettings> = {};
|
||||
|
||||
@@ -377,6 +398,9 @@ export function splitSettingsSave({
|
||||
if ((key === "gitlabEnabled" || key === "gitlabInstanceUrl" || key === "gitlabApiBaseUrl" || key === "gitlabAuthToken" || key === "gitlabAuthTokenType") && activeSection !== "global-general") {
|
||||
continue;
|
||||
}
|
||||
if (key === "mcpServers" && scopedMcpValues) {
|
||||
continue;
|
||||
}
|
||||
if (key === "mcpServers" && activeSection !== "global-mcp") {
|
||||
continue;
|
||||
}
|
||||
@@ -437,6 +461,7 @@ export function splitSettingsSave({
|
||||
if (key === "customProviders") continue; // persisted via dedicated routes, not save-split (see global branch above)
|
||||
if (key === "githubTrackingDefaultRepo" && activeSection === "global-general") continue;
|
||||
if ((key === "gitlabEnabled" || key === "gitlabInstanceUrl" || key === "gitlabApiBaseUrl" || key === "gitlabAuthToken" || key === "gitlabAuthTokenType") && activeSection === "global-general") continue;
|
||||
if (key === "mcpServers" && scopedMcpValues) continue;
|
||||
if (key === "mcpServers" && activeSection === "global-mcp") continue;
|
||||
if (!isProjectSettingsKey(key)) continue;
|
||||
|
||||
@@ -466,5 +491,21 @@ export function splitSettingsSave({
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:McpSettingsScopes 2026-07-14-22:10:
|
||||
Saving must not discard scoped MCP edits merely because the initial scoped snapshot is unavailable. Compare against an undefined baseline in that case so the current raw scoped values are still persisted.
|
||||
*/
|
||||
if (scopedMcpValues) {
|
||||
const initialGlobalMcp = resolveScopedMcpSettings("global", initialScopedValues);
|
||||
if (!settingsValueEquals(scopedMcpValues.global, initialGlobalMcp)) {
|
||||
(globalPatch as Record<string, unknown>).mcpServers = scopedMcpValues.global ?? null;
|
||||
}
|
||||
|
||||
const initialProjectMcp = resolveScopedMcpSettings("project", initialScopedValues);
|
||||
if (!settingsValueEquals(scopedMcpValues.project, initialProjectMcp)) {
|
||||
(projectPatch as Record<string, unknown>).mcpServers = scopedMcpValues.project ?? null;
|
||||
}
|
||||
}
|
||||
|
||||
return { globalPatch, projectPatch };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user