🛡️ feat: Lock YAML-Defined MCP Servers In Admin Panel

src/components/configuration/ConfigPage.tsx

@@ -35,6 +35,7 @@ import { StickyActionBar } from '@/components/shared';
 import { ConfigTabContent } from './ConfigTabContent';
 import { ImportYamlDialog } from './ImportYamlDialog';
 import { ContentToolbar } from './ContentToolbar';
+import { validateMcpCrossField } from './sections/McpServersRenderer';
 import { mergeIndexedArrayEdits } from './utils';
 import { SystemCapabilities } from '@/constants';
 import { ConfigTabBar } from './ConfigTabBar';
@@ -130,6 +131,15 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi
     return new Set(Object.keys(flattenObject(dbOverrides)));
   }, [dbOverrides]);
 
+  const baseRecordKeys = useMemo(() => {
+    const result: Record<string, Set<string>> = {};
+    const yamlMcpKeys = baseConfigData?.yamlMcpKeys;
+    if (yamlMcpKeys && Array.isArray(yamlMcpKeys)) {
+      result.mcpServers = new Set(yamlMcpKeys);
+    }
+    return result;
+  }, [baseConfigData]);
+
   const hasUnmappedSections = useMemo(
     () =>
       schemaTree.some(
@@ -344,41 +354,88 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi
     return scopeResolvedValues ?? {};
   }, [isEditingScope, flatBaseline, scopeResolvedValues]);
 
+  /** Container paths inferred from leaf baselines, used to tell apart subtree-deletes from no-op writes. */
+  const baselineIntermediates = useMemo(() => {
+    const set = new Set<string>();
+    for (const leaf of Object.keys(scopeBaseline)) {
+      const parts = leaf.split('.');
+      for (let i = 1; i < parts.length; i++) {
+        set.add(parts.slice(0, i).join('.'));
+      }
+    }
+    return set;
+  }, [scopeBaseline]);
+
+  /** Container paths walked directly off the structured config, so an orphaned `{}` entry whose flatten dropped (or never produced) any leaf is still recognized as a real subtree-delete target. */
+  const baselineContainerPaths = useMemo(() => {
+    const set = new Set<string>();
+    const walk = (obj: unknown, prefix: string): void => {
+      if (obj == null || typeof obj !== 'object' || Array.isArray(obj)) return;
+      for (const k of Object.keys(obj as Record<string, unknown>)) {
+        const path = prefix ? `${prefix}.${k}` : k;
+        const v = (obj as Record<string, unknown>)[k];
+        if (v !== null && typeof v === 'object' && !Array.isArray(v)) {
+          set.add(path);
+          walk(v, path);
+        }
+      }
+    };
+    walk(baseActiveConfigValues, '');
+    return set;
+  }, [baseActiveConfigValues]);
+
   const handleFieldChange = useCallback(
     (path: string, value: t.ConfigValue) => {
-      startTransition(() => {
-        setTouchedPaths((prev) => {
-          if (prev.has(path)) return prev;
-          const next = new Set(prev);
-          next.add(path);
+      setTouchedPaths((prev) => {
+        if (prev.has(path)) return prev;
+        const next = new Set(prev);
+        next.add(path);
+        return next;
+      });
+      setEditedValues((prev) => {
+        const baseline = scopeBaseline[path];
+        const match =
+          value === baseline ||
+          (typeof value === 'object' &&
+            typeof baseline === 'object' &&
+            JSON.stringify(value) === JSON.stringify(baseline));
+        /** A container-path undefined write must survive; baseline only stores leaves, so it would otherwise match `undefined === undefined` and get pruned. baselineContainerPaths catches the orphaned empty-object case where leaf-derived intermediates miss the entry. */
+        const isContainerDelete =
+          value === undefined &&
+          (baselineIntermediates.has(path) || baselineContainerPaths.has(path));
+        /** When the user deleted a container entry and is now writing descendants under it (delete-then-recreate of an MCP server), the new leaf must persist even if it matches baseline so the post-DELETE recreate is not missing required fields, and the ancestor-undefined must outlive the descendant write so handleConfirmSave can DELETE the entry before PATCHing the new leaves. */
+        const hasPendingAncestorDelete = Object.keys(prev).some(
+          (existing) =>
+            existing !== path && path.startsWith(`${existing}.`) && prev[existing] === undefined,
+        );
+        if (match && !isContainerDelete && !hasPendingAncestorDelete) {
+          const next = { ...prev };
+          delete next[path];
           return next;
-        });
-        setEditedValues((prev) => {
-          const baseline = scopeBaseline[path];
-          const match =
-            value === baseline ||
-            (typeof value === 'object' &&
-              typeof baseline === 'object' &&
-              JSON.stringify(value) === JSON.stringify(baseline));
-          if (match) {
-            const next = { ...prev };
-            delete next[path];
-            return next;
+        }
+        const next = { ...prev, [path]: value };
+        if (Array.isArray(value)) {
+          const prefix = `${path}.`;
+          for (const k of Object.keys(next)) {
+            if (k.startsWith(prefix) && /\.\d+$/.test(k)) delete next[k];
           }
-          const next = { ...prev, [path]: value };
-          if (Array.isArray(value)) {
-            const prefix = `${path}.`;
-            for (const k of Object.keys(next)) {
-              if (k.startsWith(prefix) && /\.\d+$/.test(k)) delete next[k];
-            }
+        }
+        const indexMatch = /^(.+)\.\d+$/.exec(path);
+        if (indexMatch) delete next[indexMatch[1]];
+        /** Two-way dedup: drop ancestors that the new leaf supersedes AND descendants that the new parent supersedes. An ancestor whose value is `undefined` expresses "delete this whole subtree" and must outlive subsequent descendant writes so DELETE-then-PATCH ordering at save time can fully replace the entry instead of leaking stale fields. */
+        for (const existing of Object.keys(next)) {
+          if (existing === path) continue;
+          const newIsDescendant = path.startsWith(`${existing}.`);
+          const newIsAncestor = existing.startsWith(`${path}.`);
+          if (newIsDescendant && next[existing] === undefined) continue;
+          if (newIsDescendant || newIsAncestor) {
+            delete next[existing];
           }
-          const indexMatch = /^(.+)\.\d+$/.exec(path);
-          if (indexMatch) delete next[indexMatch[1]];
-          return next;
-        });
+        }
+        return next;
       });
     },
-    [scopeBaseline],
+    [scopeBaseline, baselineIntermediates, baselineContainerPaths],
   );
 
   const isDirty = Object.keys(editedValues).length > 0;
@@ -453,11 +510,47 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi
     const touched = [...touchedPaths].filter((p) => p in editedValues);
     if (touched.length === 0) return;
 
+    /** Per-leaf saves can land an MCP entry in a transport state whose required siblings are missing (e.g. type=stdio with no command/args). Server-side per-field validation only sees one path at a time, so do the cross-field check here against the merged effective entry before any PATCH fires. Use baseActiveConfigValues so scope-mode edits validate against the scope-resolved baseline (where prior scope overrides supply some required fields) instead of the base config alone. */
+    const mcpBaseline = (() => {
+      const v = baseActiveConfigValues?.mcpServers;
+      if (v && typeof v === 'object' && !Array.isArray(v)) {
+        return v as Record<string, t.ConfigValue>;
+      }
+      return {};
+    })();
+    const mcpEdits: Array<[string, t.ConfigValue]> = touched
+      .filter((p) => p.startsWith('mcpServers.'))
+      .map((p) => [p, editedValues[p]] as [string, t.ConfigValue]);
+    /** In scope mode, a leaf reset (undefined write) only removes the scope override and reveals the inherited base value, so feed configValues as the resetFallback. In base mode there is nothing to inherit from. */
+    const mcpResetFallback = (() => {
+      if (!isEditingScope) return undefined;
+      const v = configValues?.mcpServers;
+      if (v && typeof v === 'object' && !Array.isArray(v)) {
+        return v as Record<string, t.ConfigValue>;
+      }
+      return undefined;
+    })();
+    if (mcpEdits.length > 0) {
+      const mcpErrors = validateMcpCrossField(mcpBaseline, mcpEdits, mcpResetFallback);
+      if (mcpErrors.length > 0) {
+        const { entryKey, missingField } = mcpErrors[0];
+        const message = localize('com_config_mcp_invalid_after_edit', {
+          entry: entryKey,
+          field: missingField,
+        });
+        setSaveError(message);
+        showToast({ type: 'error', message }, 5000);
+        return;
+      }
+    }
+
     const saves = touched
       .filter((p) => editedValues[p] !== undefined)
       .map((p) => ({
         fieldPath: p,
-        value: /\.\d+$/.test(p) ? deepSerializeKVPairs(editedValues[p]) : serializeKVPairs(editedValues[p]),
+        value: /\.\d+$/.test(p)
+          ? deepSerializeKVPairs(editedValues[p])
+          : serializeKVPairs(editedValues[p]),
       }));
     const resets = touched.filter((p) => editedValues[p] === undefined);
 
@@ -466,42 +559,37 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi
     showToast({ type: 'saving' });
 
     try {
-      const promises: Promise<unknown>[] = [];
-
-      if (saves.length > 0) {
-        if (isEditingScope) {
-          promises.push(
-            bulkSaveProfileValuesFn({
+      /** Resets must land before saves so a delete-then-recreate at the same path (e.g. MCP entry replaced with different fields) wipes stale fields first and the new leaf PATCHes don't race against the DELETE. */
+      if (resets.length > 0) {
+        const resetPromises = resets.map((fieldPath) => {
+          if (isEditingScope) {
+            return removeFieldProfileValueFn({
               data: {
+                fieldPath,
                 principalType: editingScope!.principalType,
                 principalId: editingScope!.principalId,
-                entries: saves,
               },
-            }),
-          );
-        } else {
-          promises.push(saveBaseConfigFn({ data: { entries: saves } }));
-        }
+            });
+          }
+          return resetBaseConfigFieldFn({ data: { fieldPath } });
+        });
+        await Promise.all(resetPromises);
       }
 
-      for (const fieldPath of resets) {
+      if (saves.length > 0) {
         if (isEditingScope) {
-          promises.push(
-            removeFieldProfileValueFn({
-              data: {
-                fieldPath,
-                principalType: editingScope!.principalType,
-                principalId: editingScope!.principalId,
-              },
-            }),
-          );
+          await bulkSaveProfileValuesFn({
+            data: {
+              principalType: editingScope!.principalType,
+              principalId: editingScope!.principalId,
+              entries: saves,
+            },
+          });
         } else {
-          promises.push(resetBaseConfigFieldFn({ data: { fieldPath } }));
+          await saveBaseConfigFn({ data: { entries: saves } });
         }
       }
 
-      await Promise.all(promises);
-
       if (isEditingScope) {
         invalidateAndResetScope();
       } else {
@@ -522,6 +610,9 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi
     invalidateAndResetBase,
     invalidateAndResetScope,
     saving,
+    baseActiveConfigValues,
+    configValues,
+    localize,
   ]);
 
   const serializedEditedValues = useMemo(() => {
@@ -540,7 +631,10 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi
       const segments = path.split('.');
       let current: t.ConfigValue = configValues;
       for (const seg of segments) {
-        if (current == null || typeof current !== 'object') { current = undefined; break; }
+        if (current == null || typeof current !== 'object') {
+          current = undefined;
+          break;
+        }
         current = Array.isArray(current)
           ? (current as t.ConfigValue[])[Number(seg)]
           : (current as Record<string, t.ConfigValue>)[seg];
@@ -827,6 +921,9 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi
               sectionPermissions={sectionPermissions}
               schemaDefaults={schemaDefaults}
               showConfiguredOnly={showConfiguredOnly}
+              baseRecordKeys={baseRecordKeys}
+              onValidationError={(message) => showToast({ type: 'error', message }, 5000)}
+              scopeMode={isEditingScope}
             />
           </div>
         </div>

src/components/configuration/ConfigTabContent.tsx

@@ -54,6 +54,9 @@ export function ConfigTabContent({
   sectionPermissions,
   schemaDefaults,
   showConfiguredOnly,
+  baseRecordKeys,
+  onValidationError,
+  scopeMode,
 }: t.ConfigTabContentProps) {
   const localize = useLocalize();
   const fieldsDisabled = readOnly;
@@ -164,6 +167,7 @@ export function ConfigTabContent({
       getValue,
       onChange: onFieldChange,
       onResetField,
+      editedValues,
       disabled: sectionDisabled,
       profileMap,
       previewMode,
@@ -179,6 +183,9 @@ export function ConfigTabContent({
       pendingResets,
       schemaDefaults,
       showConfiguredOnly,
+      yamlBaseKeys: baseRecordKeys?.[dataKey],
+      onValidationError,
+      scopeMode,
     };
     return (
       <>
@@ -232,7 +239,7 @@ export function ConfigTabContent({
   const isInlineSection = (section: t.ConfigSectionConfig): boolean =>
     Boolean(
       (section.sectionField && isSimpleScalar(section.sectionField)) ||
-        (section.fields.length === 1 && isSimpleScalar(section.fields[0])),
+      (section.fields.length === 1 && isSimpleScalar(section.fields[0])),
     );
 
   type SectionGroup =