feat(desktop): rework MCP editor dialog and inspector UX

apps/desktop/renderer-architecture.json

@@ -94,7 +94,6 @@
     "src/renderer/mcp-brand-marks.tsx",
     "src/renderer/mcp-catalog.ts",
     "src/renderer/mcp-command-line.ts",
-    "src/renderer/mcp-editor-validation.ts",
     "src/renderer/mcp-page-model.ts",
     "src/renderer/mcp-page.tsx",
     "src/renderer/model-catalog-choices.ts",
@@ -2125,17 +2124,6 @@
         "actionFactories": [],
         "dependencyPaths": {}
       },
-      "src/renderer/mcp-editor-validation.ts": {
-        "bridgePaths": {},
-        "environmentCapabilities": {},
-        "hookCalls": {},
-        "lifecycleMethods": {},
-        "unresolvedDependencies": 0,
-        "actionFactories": [],
-        "dependencyPaths": {
-          "./mcp-command-line.js": 1
-        }
-      },
       "src/renderer/mcp-page-model.ts": {
         "bridgePaths": {},
         "environmentCapabilities": {},
@@ -2181,11 +2169,10 @@
         "actionFactories": [],
         "dependencyPaths": {
           "./default-runtime-host-operation.js": 1,
+          "./features/module-hub/index.js": 1,
           "./locales/mcp-copy": 1,
           "./mcp-brand-marks": 1,
           "./mcp-catalog": 1,
-          "./mcp-command-line": 1,
-          "./mcp-editor-validation": 1,
           "./mcp-page-model": 1,
           "./settings/settings-error-copy": 1,
           "@astryxdesign/core": 1,

apps/desktop/src/main/__tests__/mcp-editor-validation.test.ts

@@ -19,16 +19,49 @@
 
 import assert from 'node:assert/strict';
 import { describe, it } from 'node:test';
-import { validateMcpEditorDraft } from '../../renderer/mcp-editor-validation.js';
+import { liveEditorErrors, validateMcpEditorDraft } from '../../renderer/mcp-page-model.js';
 
 describe('MCP editor validation', () => {
+  it('reports substantive URL and command errors for live first-edit display', () => {
+    // The page shows every non-presence error on the FIRST edit; these are
+    // the codes that must therefore exist immediately, not only on save.
+    assert.deepEqual(
+      validateMcpEditorDraft({ id: 'a', kind: 'remote', commandLine: '', url: 'http://lan.example/mcp', headers: '' }),
+      { url: 'insecure-url' },
+    );
+    assert.deepEqual(
+      validateMcpEditorDraft({ id: 'a', kind: 'remote', commandLine: '', url: 'not a url', headers: '' }),
+      { url: 'invalid-url' },
+    );
+    assert.deepEqual(
+      validateMcpEditorDraft({ id: 'a', kind: 'stdio', commandLine: 'npx "unterminated', url: '', headers: '' }),
+      { commandLine: 'unbalanced-quote' },
+    );
+  });
+
+
+  it('rejects a remote URL with embedded credentials, mirroring the store', () => {
+    assert.deepEqual(
+      validateMcpEditorDraft({
+        id: 'api',
+        kind: 'remote',
+        commandLine: '',
+        url: 'https://user:pass@example.com/mcp',
+        headers: '',
+      }),
+      { url: 'url-credentials' },
+    );
+  });
+
+
   it('requires a server id and the selected transport endpoint', () => {
     assert.deepEqual(
       validateMcpEditorDraft({
         id: ' ',
         kind: 'stdio',
         commandLine: '',
         url: '',
+        headers: '',
       }),
       { id: 'required', commandLine: 'required' },
     );
@@ -38,6 +71,7 @@ describe('MCP editor validation', () => {
         kind: 'remote',
         commandLine: '',
         url: ' ',
+        headers: '',
       }),
       { id: 'required', url: 'required' },
     );
@@ -50,6 +84,7 @@ describe('MCP editor validation', () => {
         kind: 'stdio',
         commandLine: 'npx -y @modelcontextprotocol/server-filesystem "/my folder"',
         url: '',
+        headers: '',
       }),
       {},
     );
@@ -59,6 +94,7 @@ describe('MCP editor validation', () => {
         kind: 'stdio',
         commandLine: 'npx "unterminated',
         url: '',
+        headers: '',
       }),
       { commandLine: 'unbalanced-quote' },
     );
@@ -70,18 +106,41 @@ describe('MCP editor validation', () => {
         kind: 'stdio',
         commandLine: '""',
         url: '',
+        headers: '',
       }),
       { commandLine: 'required' },
     );
   });
 
+  it('rejects an id that would silently overwrite an existing server', () => {
+    const draft = {
+      id: ' notion ',
+      kind: 'stdio',
+      commandLine: 'npx server',
+      url: '',
+      headers: '',
+    } as const;
+    assert.deepEqual(
+      validateMcpEditorDraft(draft, { existingIds: ['notion', 'filesystem'] }),
+      { id: 'duplicate-id' },
+    );
+    // Edit mode passes no existingIds — writing over your own id is the
+    // point of editing.
+    assert.deepEqual(validateMcpEditorDraft(draft), {});
+    assert.deepEqual(
+      validateMcpEditorDraft(draft, { existingIds: ['filesystem'] }),
+      {},
+    );
+  });
+
   it('accepts only HTTP(S) URLs for remote servers', () => {
     assert.deepEqual(
       validateMcpEditorDraft({
         id: 'remote',
         kind: 'remote',
         commandLine: '',
         url: 'not a url',
+        headers: '',
       }),
       { url: 'invalid-url' },
     );
@@ -91,6 +150,7 @@ describe('MCP editor validation', () => {
         kind: 'remote',
         commandLine: '',
         url: 'file:///tmp/server',
+        headers: '',
       }),
       { url: 'invalid-url' },
     );
@@ -100,8 +160,88 @@ describe('MCP editor validation', () => {
         kind: 'remote',
         commandLine: '',
         url: 'https://example.com/mcp',
+        headers: '',
       }),
       {},
     );
   });
+
+  it('mirrors the store rule: no Authorization header on an OAuth server', () => {
+    // The dialog has no OAuth field — the block rides the draft opaquely —
+    // so without this mirror the placeholder invites exactly the header the
+    // store rejects, and the save bounces as a raw untranslated toast.
+    const base = {
+      id: 'notion',
+      kind: 'remote' as const,
+      commandLine: '',
+      url: 'https://mcp.notion.com/mcp',
+    };
+    assert.deepEqual(
+      validateMcpEditorDraft(
+        { ...base, headers: 'Authorization=Bearer t\nX-Workspace=w1' },
+        { hasOAuth: true },
+      ),
+      { headers: 'oauth-authorization-conflict' },
+    );
+    // Case-insensitive, like the store's check.
+    assert.deepEqual(
+      validateMcpEditorDraft({ ...base, headers: 'authorization=Bearer t' }, { hasOAuth: true }),
+      { headers: 'oauth-authorization-conflict' },
+    );
+    // No oauth block → the header is the user's to configure.
+    assert.deepEqual(
+      validateMcpEditorDraft({ ...base, headers: 'Authorization=Bearer t' }, {}),
+      {},
+    );
+    // OAuth with other headers is fine.
+    assert.deepEqual(
+      validateMcpEditorDraft({ ...base, headers: 'X-Workspace=w1' }, { hasOAuth: true }),
+      {},
+    );
+  });
+
+  it('mirrors the store rule: cleartext http only for loopback hosts', () => {
+    const draft = (url: string) =>
+      validateMcpEditorDraft({ id: 'remote', kind: 'remote', commandLine: '', url, headers: '' });
+    assert.deepEqual(draft('http://192.168.1.50:8080/mcp'), { url: 'insecure-url' });
+    assert.deepEqual(draft('http://example.com/mcp'), { url: 'insecure-url' });
+    // `*.localhost` is no longer a loopback trust root: Node resolves it
+    // through the system resolver, so its loopback-ness is not guaranteed.
+    assert.deepEqual(draft('http://dev.localhost/mcp'), { url: 'insecure-url' });
+    for (const url of [
+      'http://127.0.0.1:8080/mcp',
+      'http://localhost:3000/mcp',
+      'http://[::1]:3000/mcp',
+    ]) {
+      assert.deepEqual(draft(url), {}, url);
+    }
+  });
+
+  it('gates live errors: required shows only where a save already flagged it', () => {
+    // A sibling's visible error must not smuggle a fresh 必填 onto a field
+    // the user just cleared but has not "left" via a save attempt.
+    assert.deepEqual(
+      liveEditorErrors({ id: 'duplicate-id', url: 'required' }, { id: 'duplicate-id' }),
+      { id: 'duplicate-id' },
+    );
+    // After a save attempt flagged the field, editing keeps the verdict
+    // current — including the required state itself.
+    assert.deepEqual(
+      liveEditorErrors({ url: 'required' }, { url: 'required' }),
+      { url: 'required' },
+    );
+    assert.deepEqual(liveEditorErrors({}, { url: 'required' }), {});
+    // Substantive errors are always live, even on a clean slate.
+    assert.deepEqual(
+      liveEditorErrors({ url: 'insecure-url' }, {}),
+      { url: 'insecure-url' },
+    );
+    // A transport-kind switch revalidates every field through the same
+    // gate: the other kind's stale errors drop, and the new kind's empty
+    // fields stay quiet until save.
+    assert.deepEqual(
+      liveEditorErrors({ url: 'required' }, { commandLine: 'unbalanced-quote' }),
+      {},
+    );
+  });
 });

apps/desktop/src/main/__tests__/mcp-ipc-main.test.ts

@@ -818,3 +818,72 @@ test('MCP config commit is not rolled back by a capability publication failure',
     'Host disconnected',
   ]);
 });
+
+test('import merges against the store state at commit time, not the snapshot a renderer loaded', async () => {
+  const handlers = new Map<string, (...args: any[]) => Promise<any>>();
+  let config: McpConfigFile = {
+    version: MCP_CONFIG_VERSION,
+    mcpServers: { alpha: { command: 'node' } },
+  };
+  registerMcpIpcMain({
+    ipcMain: { handle(channel, handler) { handlers.set(channel, handler as (...args: any[]) => Promise<any>); } },
+    store: {
+      get: async () => config,
+      transform: async (apply) => { config = await apply(config); return config; },
+      upsert: async (_serverId, _server) => config,
+      remove: async () => config,
+    },
+    manager: {
+      cancelConnect: () => false,
+      forgetServerCredentials: async () => {},
+      sync: async () => {},
+      statuses: () => [],
+      test: async () => { throw new Error('not used'); },
+    },
+    oauth: {
+      isActive: () => false,
+      cancelLogin: () => false,
+      login: async () => { throw new Error('not used'); },
+      logout: async () => { throw new Error('not used'); },
+      resumeLogin: async () => undefined,
+    },
+    ensureReady: async () => {},
+    publishCapabilities: async () => {},
+    onPublicationError: () => {},
+    emitChanged: () => {},
+  });
+
+  const getConfig = handlers.get('mcp:getConfig');
+  const importConfig = handlers.get('mcp:importConfig');
+  assert.ok(getConfig && importConfig);
+
+  // A renderer loads {alpha} — the snapshot an import dialog would sit on.
+  const rendererSnapshot = await getConfig({});
+  assert.deepEqual(Object.keys(rendererSnapshot.mcpServers), ['alpha']);
+
+  // While the dialog is open, a concurrent writer (marketplace install,
+  // another window, another Host client) commits `beta` with a credential.
+  config = {
+    version: MCP_CONFIG_VERSION,
+    mcpServers: {
+      ...config.mcpServers,
+      beta: {
+        url: 'https://mcp.beta.example/mcp',
+        oauth: { clientId: 'beta-client', clientSecret: 'beta-secret' },
+      },
+    },
+  };
+
+  // The import must merge against the CURRENT store state inside the lane —
+  // a renderer-side merge of the stale snapshot would erase beta entirely.
+  const next = await importConfig({}, '{"gamma":{"command":"npx","args":["gamma"]}}');
+  assert.equal(next.status, 'imported');
+  assert.deepEqual(Object.keys(config.mcpServers).sort(), ['alpha', 'beta', 'gamma']);
+  const storedBeta = config.mcpServers.beta;
+  assert.ok(storedBeta && 'url' in storedBeta);
+  assert.equal(storedBeta.oauth?.clientSecret, 'beta-secret');
+  // The response the renderer adopts also carries beta — masked, never raw.
+  const returnedBeta = next.config.mcpServers.beta;
+  assert.ok(returnedBeta && 'url' in returnedBeta);
+  assert.notEqual(returnedBeta.oauth?.clientSecret, 'beta-secret');
+});

apps/desktop/src/renderer/features/module-hub/index.ts

@@ -21,6 +21,7 @@ export { useModuleHubController } from './controller/use-module-hub-controller.j
 export { ModuleHubServicesProvider } from './services-context.js';
 export type {
   ModuleHubClipboardService,
+  ModuleHubMcpEditorService,
   ModuleHubServices,
 } from './ports.js';
 export { ModuleHubHost } from './ui/module-hub-host.js';

apps/desktop/src/renderer/features/module-hub/ports.ts

@@ -23,6 +23,11 @@ import type {
   DailyReviewRange,
   DailyReviewSummary,
 } from '@maka/core/daily-review';
+import type {
+  McpConfigAddResult,
+  McpServerConfig,
+  McpServerStatus,
+} from '@maka/core/mcp';
 import type { Result } from '@maka/core/result';
 import type {
   CreateScheduledTaskInput,
@@ -235,6 +240,22 @@ export interface ModuleHubClipboardService {
   writeText(text: string): Promise<void>;
 }
 
+/** The MCP editor/inspector mutations the hub's MCP page performs beyond
+ * the page's base bridge surface: creating a server, and the OAuth login
+ * lifecycle. A port rather than direct bridge access — only the platform
+ * zone may touch the global bridge, and the page receives this service
+ * through the feature seam. */
+export interface ModuleHubMcpEditorService {
+  add(
+    serverId: string,
+    config: McpServerConfig,
+    host: ModuleHubRuntimeHostRef,
+  ): Promise<McpConfigAddResult>;
+  login(serverId: string, host: ModuleHubRuntimeHostRef): Promise<McpServerStatus>;
+  logout(serverId: string, host: ModuleHubRuntimeHostRef): Promise<McpServerStatus>;
+  cancelLogin(serverId: string, host: ModuleHubRuntimeHostRef): Promise<boolean>;
+}
+
 /** Environment capabilities owned by the Module Hub feature slice. */
 export interface ModuleHubServices {
   runtimeHosts: ModuleHubRuntimeHostsService;
@@ -243,4 +264,5 @@ export interface ModuleHubServices {
   clientSettings: ModuleHubClientSettingsService;
   dailyReview: ModuleHubDailyReviewService;
   clipboard: ModuleHubClipboardService;
+  mcpEditor: ModuleHubMcpEditorService;
 }

apps/desktop/src/renderer/features/module-hub/testing.ts

@@ -170,6 +170,12 @@ export function createFakeModuleHubServices(
     clipboard: {
       writeText: async () => undefined,
     },
+    mcpEditor: {
+      add: async () => notConfigured('mcpEditor.add'),
+      login: async () => notConfigured('mcpEditor.login'),
+      logout: async () => notConfigured('mcpEditor.logout'),
+      cancelLogin: async () => false,
+    },
     ...overrides,
   };
 }