From b185c69333f7001e5d35b9bd0cc7595fe353ccc5 Mon Sep 17 00:00:00 2001 From: me2seeks Date: Fri, 28 Aug 2026 10:07:15 +0800 Subject: [PATCH 1/6] refactor(mcp): share credential retirement classification Generated-by: Codex --- apps/desktop/src/main/mcp-ipc-main.ts | 9 +--- .../mcp-credential-retirement.test.ts | 51 +++++++++++++++++++ packages/core/src/mcp.ts | 14 +++++ packages/mcp/src/index.ts | 13 +---- 4 files changed, 69 insertions(+), 18 deletions(-) create mode 100644 packages/core/src/__tests__/mcp-credential-retirement.test.ts diff --git a/apps/desktop/src/main/mcp-ipc-main.ts b/apps/desktop/src/main/mcp-ipc-main.ts index 7b0a1dbed4..38c1081aab 100644 --- a/apps/desktop/src/main/mcp-ipc-main.ts +++ b/apps/desktop/src/main/mcp-ipc-main.ts @@ -20,7 +20,7 @@ import type { IpcMain } from 'electron'; import { MCP_CONFIG_VERSION, - isMcpStdioConfig, + mcpConfigChangeRetiresCredentials, type McpConfigAddResult, type McpConfigFile, type McpConfigImportResult, @@ -369,12 +369,7 @@ function credentialRetirements(current: McpConfigFile, next: McpConfigFile): str const incoming = Object.hasOwn(next.mcpServers, serverId) ? next.mcpServers[serverId] : undefined; - if (!incoming) { - retired.push(serverId); - continue; - } - if (isMcpStdioConfig(server)) continue; - if (isMcpStdioConfig(incoming) || incoming.url !== server.url) retired.push(serverId); + if (mcpConfigChangeRetiresCredentials(server, incoming)) retired.push(serverId); } return retired; } diff --git a/packages/core/src/__tests__/mcp-credential-retirement.test.ts b/packages/core/src/__tests__/mcp-credential-retirement.test.ts new file mode 100644 index 0000000000..3d3afb29b0 --- /dev/null +++ b/packages/core/src/__tests__/mcp-credential-retirement.test.ts @@ -0,0 +1,51 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import { mcpConfigChangeRetiresCredentials, type McpServerConfig } from '../mcp.js'; + +describe('MCP credential retirement', () => { + const stdio: McpServerConfig = { command: 'server' }; + const remote: McpServerConfig = { url: 'https://one.example/mcp' }; + + it('retires credentials when an id is removed, repointed, or changes transport', () => { + assert.equal(mcpConfigChangeRetiresCredentials(remote, undefined), true); + assert.equal( + mcpConfigChangeRetiresCredentials(remote, { url: 'https://two.example/mcp' }), + true, + ); + assert.equal(mcpConfigChangeRetiresCredentials(remote, stdio), true); + // A stale credential record can exist under a currently-stdio id after + // an offline edit. Do not let it become the new remote endpoint's token. + assert.equal(mcpConfigChangeRetiresCredentials(stdio, remote), true); + }); + + it('keeps credentials when endpoint ownership does not change', () => { + assert.equal( + mcpConfigChangeRetiresCredentials(remote, { + ...remote, + headers: { Authorization: 'Bearer replacement' }, + enabled: false, + }), + false, + ); + assert.equal(mcpConfigChangeRetiresCredentials(stdio, { command: 'different-server' }), false); + }); +}); diff --git a/packages/core/src/mcp.ts b/packages/core/src/mcp.ts index f057f5c153..2cc941ed73 100644 --- a/packages/core/src/mcp.ts +++ b/packages/core/src/mcp.ts @@ -343,6 +343,20 @@ export function isMcpStdioConfig(config: McpServerConfig): config is McpStdioSer return 'command' in config; } +/** True when a configuration change can leave credentials bound to an + * endpoint the server id no longer names. Removals retire regardless of + * transport so a stale record cannot survive the id being freed for reuse. */ +export function mcpConfigChangeRetiresCredentials( + previous: McpServerConfig, + next: McpServerConfig | undefined, +): boolean { + if (!next) return true; + const previousStdio = isMcpStdioConfig(previous); + const nextStdio = isMcpStdioConfig(next); + if (previousStdio || nextStdio) return previousStdio !== nextStdio; + return previous.url !== next.url; +} + export function resolveMcpProtocolPreference(config: McpServerConfig): McpProtocolPreference { return config.protocol ?? 'legacy'; } diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index a7010d4692..f2cea4064d 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -46,6 +46,7 @@ import { import { isMcpStdioConfig, isNonLoopbackCleartextHttp, + mcpConfigChangeRetiresCredentials, resolveMcpProtocolPreference, type McpBoundTool, type McpCallResult, @@ -444,7 +445,7 @@ export class McpClientManager { // retries the erase against the still-owed old config. const owed = current.credentialCleanupOwed ? current.credentialCleanupOwed - : remoteUrlChanged(current.config, serverConfig) + : mcpConfigChangeRetiresCredentials(current.config, serverConfig) ? current.config : undefined; if (owed) { @@ -2571,16 +2572,6 @@ async function connectCandidate( } } -/** True when a reconfigured server no longer talks to the endpoint its - * stored credentials were issued for: the URL changed, or the entry - * switched between stdio and remote. A headers-only edit returns false. */ -function remoteUrlChanged(previous: McpServerConfig, next: McpServerConfig): boolean { - const previousStdio = isMcpStdioConfig(previous); - const nextStdio = isMcpStdioConfig(next); - if (previousStdio || nextStdio) return previousStdio !== nextStdio; - return previous.url !== next.url; -} - function stableConfigFingerprint(config: McpServerConfig): string { return JSON.stringify(sortValue(config)); } From f25aee59d6bc291aba5d77781a5efc1266d5e1ef Mon Sep 17 00:00:00 2001 From: me2seeks Date: Fri, 28 Aug 2026 10:21:36 +0800 Subject: [PATCH 2/6] feat(cli): serialize TUI MCP management actions Generated-by: Codex --- .../src/__tests__/pi-tui-mcp-status.test.ts | 14 +- .../cli/src/__tests__/pi-tui-runner.test.ts | 3 + .../cli/src/__tests__/tui-mcp-control.test.ts | 439 +++++++++++++++++- packages/cli/src/tui-mcp-control.ts | 410 +++++++++++++++- 4 files changed, 835 insertions(+), 31 deletions(-) diff --git a/packages/cli/src/__tests__/pi-tui-mcp-status.test.ts b/packages/cli/src/__tests__/pi-tui-mcp-status.test.ts index e7f43d7c19..f510e02339 100644 --- a/packages/cli/src/__tests__/pi-tui-mcp-status.test.ts +++ b/packages/cli/src/__tests__/pi-tui-mcp-status.test.ts @@ -29,11 +29,14 @@ describe('MCP status overlay', () => { locale: 'en', surface: surface({ initialization: 'ready', + configuration: 'ready', publication: 'published', toolCount: 2, servers: [ { serverId: 'filesystem', + configured: true, + synchronized: true, state: 'connected', transport: 'stdio', negotiatedProtocol: { era: 'modern', revision: '2026-07-28' }, @@ -70,10 +73,18 @@ describe('MCP status overlay', () => { locale: 'zh', surface: surface({ initialization: 'ready', + configuration: 'ready', publication: 'not_published', toolCount: 0, servers: [ - { serverId: 'oauth', state: 'needs-auth', transport: 'streamable-http', toolCount: 0 }, + { + serverId: 'oauth', + configured: true, + synchronized: true, + state: 'needs-auth', + transport: 'streamable-http', + toolCount: 0, + }, ], }), viewportRows: () => 6, @@ -92,6 +103,7 @@ describe('MCP status overlay', () => { let closed = 0; const mcp = surface({ initialization: 'ready', + configuration: 'ready', publication: 'not_published', toolCount: 0, servers: [], diff --git a/packages/cli/src/__tests__/pi-tui-runner.test.ts b/packages/cli/src/__tests__/pi-tui-runner.test.ts index f91bfe575d..b37151fc64 100644 --- a/packages/cli/src/__tests__/pi-tui-runner.test.ts +++ b/packages/cli/src/__tests__/pi-tui-runner.test.ts @@ -2219,11 +2219,14 @@ describe('Maka Pi TUI runner', () => { mcp: { snapshot: () => ({ initialization: 'ready', + configuration: 'ready', publication: 'published', toolCount: 1, servers: [ { serverId: 'filesystem', + configured: true, + synchronized: true, state: 'connected', transport: 'stdio', negotiatedProtocol: { era: 'modern', revision: '2026-07-28' }, diff --git a/packages/cli/src/__tests__/tui-mcp-control.test.ts b/packages/cli/src/__tests__/tui-mcp-control.test.ts index ed7b04f55f..1091c11d00 100644 --- a/packages/cli/src/__tests__/tui-mcp-control.test.ts +++ b/packages/cli/src/__tests__/tui-mcp-control.test.ts @@ -34,7 +34,7 @@ test('TUI MCP startup stays backgrounded and publishes the discovered snapshot', const controller = createTuiMcpController( { workspaceRoot: '/unused', connection: connection.connection }, { - configStore: { get: () => config.promise }, + configStore: configStoreHarness(() => config.promise), manager: manager.manager, createProvider: () => provider('provider-1'), }, @@ -49,6 +49,8 @@ test('TUI MCP startup stays backgrounded and publishes the discovered snapshot', assert.deepEqual(controller.snapshot().servers, [ { serverId: 'local', + configured: false, + synchronized: false, state: 'connected', transport: 'stdio', negotiatedProtocol: { era: 'legacy', revision: '2024-11-05' }, @@ -73,7 +75,7 @@ test('TUI MCP publication coalesces a discovery change behind the in-flight revi const controller = createTuiMcpController( { workspaceRoot: '/unused', connection: connection.connection }, { - configStore: { get: async () => emptyConfig() }, + configStore: configStoreHarness(async () => emptyConfig()), manager: manager.manager, createProvider: () => { const id = `provider-${providers.length + 1}`; @@ -98,7 +100,7 @@ test('TUI MCP invalidates a lost generation and republishes on its replacement', const controller = createTuiMcpController( { workspaceRoot: '/unused', connection: connection.connection }, { - configStore: { get: async () => emptyConfig() }, + configStore: configStoreHarness(async () => emptyConfig()), manager: manager.manager, createProvider: () => provider(`provider-${connection.replacements.length + 1}`), }, @@ -119,7 +121,7 @@ test('TUI MCP unregisters the current generation when discovery removes every to const controller = createTuiMcpController( { workspaceRoot: '/unused', connection: connection.connection }, { - configStore: { get: async () => emptyConfig() }, + configStore: configStoreHarness(async () => emptyConfig()), manager: manager.manager, createProvider: (current) => current.toolSnapshot().tools.length === 0 ? undefined : provider('provider'), @@ -140,11 +142,9 @@ test('TUI MCP fails closed when its saved config cannot be read', async () => { const controller = createTuiMcpController( { workspaceRoot: '/unused', connection: connection.connection }, { - configStore: { - get: async () => { - throw new Error('config contains secret-value'); - }, - }, + configStore: configStoreHarness(async () => { + throw new Error('config contains secret-value'); + }), manager: manager.manager, createProvider: () => provider('must-not-publish'), }, @@ -168,7 +168,7 @@ test('TUI MCP unregisters a publication that settles while close is waiting', as const controller = createTuiMcpController( { workspaceRoot: '/unused', connection: connection.connection }, { - configStore: { get: async () => emptyConfig() }, + configStore: configStoreHarness(async () => emptyConfig()), manager: manager.manager, createProvider: () => provider('provider'), }, @@ -182,6 +182,405 @@ test('TUI MCP unregisters a publication that settles while close is waiting', as assert.equal(manager.closed, 1); }); +test('TUI MCP add commits before manager synchronization and reports Host convergence', async () => { + const order: string[] = []; + const store = mutableConfigStore(emptyConfig(), order); + const manager = managementManager(order); + const connection = connectionHarness(); + const controller = createTuiMcpController( + { workspaceRoot: '/unused', connection: connection.connection }, + { + configStore: store.store, + manager: manager.manager, + createProvider: () => undefined, + }, + ); + await waitFor(() => controller.snapshot().initialization === 'ready'); + order.length = 0; + + const result = await controller.execute({ + kind: 'add', + serverId: 'docs', + config: { url: 'https://docs.example/mcp', protocol: 'auto' }, + }); + + assert.deepEqual(result, { status: 'applied', effect: 'published' }); + assert.deepEqual(order, ['get', 'transform', 'sync']); + assert.deepEqual((await store.store.get()).mcpServers.docs, { + enabled: true, + url: 'https://docs.example/mcp', + transport: 'auto', + protocol: 'auto', + }); + assert.equal(controller.snapshot().configuration, 'ready'); + await controller.close(); +}); + +test('TUI MCP retires endpoint credentials before persistence and aborts on cleanup failure', async () => { + const order: string[] = []; + const store = mutableConfigStore( + { + version: 3, + mcpServers: { docs: { url: 'https://old.example/mcp' } }, + }, + order, + ); + const manager = managementManager(order, { credentialFailure: true }); + const connection = connectionHarness(); + const controller = createTuiMcpController( + { workspaceRoot: '/unused', connection: connection.connection }, + { configStore: store.store, manager: manager.manager, createProvider: () => undefined }, + ); + await waitFor(() => controller.snapshot().initialization === 'ready'); + order.length = 0; + const edit = controller.configForEdit('docs'); + assert.ok(edit); + + const result = await controller.execute({ + kind: 'edit', + serverId: 'docs', + expectedRevision: edit.revision, + config: { url: 'https://new.example/mcp' }, + }); + + assert.deepEqual(result, { status: 'failed', reason: 'credential-cleanup-failed' }); + assert.deepEqual(order, ['get', 'forget:docs']); + const stored = (await store.store.get()).mcpServers.docs; + assert.ok(stored && 'url' in stored); + assert.equal(stored.url, 'https://old.example/mcp'); + await controller.close(); +}); + +test('TUI MCP edit rejects a stale revision without touching credentials or disk', async () => { + const order: string[] = []; + const store = mutableConfigStore( + { version: 3, mcpServers: { docs: { url: 'https://one.example/mcp' } } }, + order, + ); + const manager = managementManager(order); + const connection = connectionHarness(); + const controller = createTuiMcpController( + { workspaceRoot: '/unused', connection: connection.connection }, + { configStore: store.store, manager: manager.manager, createProvider: () => undefined }, + ); + await waitFor(() => controller.snapshot().initialization === 'ready'); + const edit = controller.configForEdit('docs'); + assert.ok(edit); + store.replace({ version: 3, mcpServers: { docs: { url: 'https://other.example/mcp' } } }); + order.length = 0; + + const result = await controller.execute({ + kind: 'edit', + serverId: 'docs', + expectedRevision: edit.revision, + config: { url: 'https://new.example/mcp' }, + }); + + assert.deepEqual(result, { status: 'conflict', reason: 'stale_edit' }); + assert.deepEqual(order, ['get']); + await controller.close(); +}); + +test('TUI MCP import preserves unrelated external edits and rejects changed preview entries', async () => { + const order: string[] = []; + const store = mutableConfigStore( + { version: 3, mcpServers: { existing: { command: 'one' } } }, + order, + ); + const manager = managementManager(order); + const connection = connectionHarness(); + const controller = createTuiMcpController( + { workspaceRoot: '/unused', connection: connection.connection }, + { configStore: store.store, manager: manager.manager, createProvider: () => undefined }, + ); + await waitFor(() => controller.snapshot().initialization === 'ready'); + const preview = controller.previewImport('{"docs":{"url":"https://docs.example/mcp"}}'); + assert.equal(preview.status, 'ready'); + if (preview.status !== 'ready') throw new Error('preview did not prepare'); + store.replace({ + version: 3, + mcpServers: { existing: { command: 'externally-edited' } }, + }); + + assert.deepEqual( + await controller.execute({ kind: 'commit_import', previewId: preview.preview.previewId }), + { + status: 'applied', + effect: 'published', + }, + ); + const existing = (await store.store.get()).mcpServers.existing; + assert.ok(existing && 'command' in existing); + assert.equal(existing.command, 'externally-edited'); + + const stale = controller.previewImport('{"docs":{"url":"https://replacement.example/mcp"}}'); + assert.equal(stale.status, 'ready'); + if (stale.status !== 'ready') throw new Error('preview did not prepare'); + store.replace({ + version: 3, + mcpServers: { + ...(await store.store.get()).mcpServers, + docs: { url: 'https://concurrent.example/mcp' }, + }, + }); + assert.deepEqual( + await controller.execute({ kind: 'commit_import', previewId: stale.preview.previewId }), + { + status: 'conflict', + reason: 'stale_import', + }, + ); + await controller.close(); +}); + +test('TUI MCP keeps a durable mutation visible when manager synchronization fails', async () => { + const order: string[] = []; + const store = mutableConfigStore(emptyConfig(), order); + const manager = managementManager(order); + const connection = connectionHarness(); + const controller = createTuiMcpController( + { workspaceRoot: '/unused', connection: connection.connection }, + { configStore: store.store, manager: manager.manager, createProvider: () => undefined }, + ); + await waitFor(() => controller.snapshot().initialization === 'ready'); + manager.failNextSync(); + + const result = await controller.execute({ + kind: 'add', + serverId: 'local', + config: { command: 'server' }, + }); + + assert.deepEqual(result, { status: 'applied', effect: 'sync_failed' }); + assert.equal(controller.snapshot().configuration, 'out_of_sync'); + assert.deepEqual(controller.snapshot().servers[0], { + serverId: 'local', + configured: true, + synchronized: false, + state: 'disconnected', + toolCount: 0, + }); + assert.ok((await store.store.get()).mcpServers.local); + await controller.close(); +}); + +test('TUI MCP reports a committed action as pending while the Host is unavailable', async () => { + const order: string[] = []; + const store = mutableConfigStore(emptyConfig(), order); + const manager = managementManager(order); + const connection = connectionHarness(); + connection.emit({ kind: 'unavailable' }); + const controller = createTuiMcpController( + { workspaceRoot: '/unused', connection: connection.connection }, + { configStore: store.store, manager: manager.manager, createProvider: () => undefined }, + ); + await waitFor(() => controller.snapshot().initialization === 'ready'); + + assert.deepEqual( + await controller.execute({ kind: 'add', serverId: 'local', config: { command: 'server' } }), + { status: 'applied', effect: 'pending_host' }, + ); + await controller.close(); +}); + +test('TUI MCP close fences an admitted mutation before persistence', async () => { + const actionRead = deferredValue(); + let reads = 0; + let transforms = 0; + const store = { + get: async () => { + reads += 1; + return reads === 1 ? emptyConfig() : actionRead.promise; + }, + transform: async (apply: (current: McpConfigFile) => McpConfigFile) => { + transforms += 1; + return apply(emptyConfig()); + }, + }; + const manager = managementManager([]); + const connection = connectionHarness(); + const controller = createTuiMcpController( + { workspaceRoot: '/unused', connection: connection.connection }, + { configStore: store, manager: manager.manager, createProvider: () => undefined }, + ); + await waitFor(() => controller.snapshot().initialization === 'ready'); + const executing = controller.execute({ + kind: 'add', + serverId: 'late', + config: { command: 'server' }, + }); + await waitFor(() => reads === 2); + const closing = controller.close(); + actionRead.resolve(emptyConfig()); + + assert.deepEqual(await executing, { status: 'failed', reason: 'closed' }); + await closing; + assert.equal(transforms, 0); +}); + +test('TUI MCP waits for manager synchronization before publishing an action snapshot', async () => { + const store = mutableConfigStore(emptyConfig(), []); + const actionSync = deferred(); + let syncCount = 0; + let listener: (() => void) | undefined; + let revision = 0; + const manager = { + sync: async () => { + syncCount += 1; + revision += 1; + listener?.(); + if (syncCount === 2) await actionSync.promise; + }, + statuses: () => [], + toolSnapshot: () => ({ revision, tools: [{}] }) as unknown as McpToolSnapshot, + callTool: async () => ({ content: [] }), + test: async () => ({ ok: true, status: connectedStatus('local', 1), latencyMs: 1 }), + reconnect: async () => connectedStatus('local', 1), + forgetServerCredentials: async () => undefined, + onChange: (next: () => void) => { + listener = next; + return () => { + listener = undefined; + }; + }, + close: async () => undefined, + } as unknown as ReturnType['manager']; + const connection = connectionHarness(); + const controller = createTuiMcpController( + { workspaceRoot: '/unused', connection: connection.connection }, + { configStore: store.store, manager, createProvider: () => provider('provider') }, + ); + await waitFor(() => controller.snapshot().publication === 'published'); + connection.replacements.length = 0; + + const executing = controller.execute({ + kind: 'add', + serverId: 'local', + config: { command: 'server' }, + }); + await waitFor(() => syncCount === 2); + assert.equal(connection.replacements.length, 0); + assert.equal(controller.snapshot().configuration, 'synchronizing'); + actionSync.resolve(); + + assert.deepEqual(await executing, { status: 'applied', effect: 'published' }); + assert.equal(connection.replacements.length, 1); + await controller.close(); +}); + +test('TUI MCP rebases an action over an unrelated concurrent config edit', async () => { + let config: McpConfigFile = { + version: 3, + mcpServers: { existing: { command: 'before' } }, + }; + const store = { + get: async () => structuredClone(config), + transform: async (apply: (current: McpConfigFile) => McpConfigFile) => { + config = apply({ + version: 3, + mcpServers: { existing: { command: 'concurrent' } }, + }); + return structuredClone(config); + }, + }; + const manager = managementManager([]); + const connection = connectionHarness(); + const controller = createTuiMcpController( + { workspaceRoot: '/unused', connection: connection.connection }, + { configStore: store, manager: manager.manager, createProvider: () => undefined }, + ); + await waitFor(() => controller.snapshot().initialization === 'ready'); + + assert.deepEqual( + await controller.execute({ kind: 'add', serverId: 'local', config: { command: 'server' } }), + { status: 'applied', effect: 'published' }, + ); + const existing = config.mcpServers.existing; + assert.ok(existing && 'command' in existing); + assert.equal(existing.command, 'concurrent'); + assert.ok(config.mcpServers.local); + await controller.close(); +}); + +function mutableConfigStore(initial: McpConfigFile, order: string[]) { + let config = structuredClone(initial); + const store = { + get: async () => { + order.push('get'); + return structuredClone(config); + }, + transform: async (apply: (current: McpConfigFile) => McpConfigFile) => { + order.push('transform'); + config = structuredClone(apply(structuredClone(config))); + return structuredClone(config); + }, + }; + return { + store, + replace(next: McpConfigFile) { + config = structuredClone(next); + }, + }; +} + +function managementManager( + order: string[], + options: { readonly credentialFailure?: boolean } = {}, +) { + let listener: (() => void) | undefined; + let syncFailure = false; + let revision = 0; + const statuses: McpServerStatus[] = []; + const manager = { + sync: async () => { + order.push('sync'); + if (syncFailure) { + syncFailure = false; + throw new Error('sync failed'); + } + revision += 1; + listener?.(); + }, + statuses: () => statuses, + toolSnapshot: () => ({ revision, tools: [] }) as McpToolSnapshot, + callTool: async () => ({ content: [] }), + test: async (serverId: string) => ({ + ok: true, + status: connectedStatus(serverId, 0), + latencyMs: 1, + }), + reconnect: async (serverId: string) => connectedStatus(serverId, 0), + forgetServerCredentials: async (serverId: string) => { + order.push(`forget:${serverId}`); + if (options.credentialFailure) throw new Error('credential cleanup failed'); + }, + onChange: (next: () => void) => { + listener = next; + return () => { + if (listener === next) listener = undefined; + }; + }, + close: async () => undefined, + } as unknown as Pick< + McpClientManager, + | 'sync' + | 'statuses' + | 'toolSnapshot' + | 'callTool' + | 'test' + | 'reconnect' + | 'forgetServerCredentials' + | 'onChange' + | 'close' + >; + return { + manager, + failNextSync() { + syncFailure = true; + }, + }; +} + function managerHarness(revision: number, statuses: McpServerStatus[]) { let currentRevision = revision; let toolCount = revision === 0 ? 0 : 1; @@ -196,6 +595,9 @@ function managerHarness(revision: number, statuses: McpServerStatus[]) { tools: new Array(toolCount).fill({}), }) as McpToolSnapshot, callTool: async () => ({ content: [] }), + test: async () => ({ ok: true, status: connectedStatus('local', 1), latencyMs: 1 }), + reconnect: async () => connectedStatus('local', 1), + forgetServerCredentials: async () => undefined, onChange: (next: () => void) => { listener = next; return () => { @@ -207,7 +609,15 @@ function managerHarness(revision: number, statuses: McpServerStatus[]) { }, } as unknown as Pick< McpClientManager, - 'sync' | 'statuses' | 'toolSnapshot' | 'callTool' | 'onChange' | 'close' + | 'sync' + | 'statuses' + | 'toolSnapshot' + | 'callTool' + | 'onChange' + | 'test' + | 'reconnect' + | 'forgetServerCredentials' + | 'close' >; return { manager, @@ -290,6 +700,13 @@ function emptyConfig(): McpConfigFile { return { version: 3, mcpServers: {} }; } +function configStoreHarness(get: () => Promise) { + return { + get, + transform: async (apply: (current: McpConfigFile) => McpConfigFile) => apply(await get()), + }; +} + async function waitFor(condition: () => boolean): Promise { for (let attempt = 0; attempt < 1_000 && !condition(); attempt += 1) { await new Promise((resolve) => setImmediate(resolve)); diff --git a/packages/cli/src/tui-mcp-control.ts b/packages/cli/src/tui-mcp-control.ts index ff91bd6c6c..04506d92c9 100644 --- a/packages/cli/src/tui-mcp-control.ts +++ b/packages/cli/src/tui-mcp-control.ts @@ -17,10 +17,27 @@ * under the License. */ -import type { McpServerStatus } from '@maka/core/mcp'; +import { createHash, randomUUID } from 'node:crypto'; +import { + MCP_CONFIG_VERSION, + mcpConfigChangeRetiresCredentials, + resolveMcpProtocolPreference, + type McpConfigFile, + type McpConfigSourceFailureReason, + type McpProtocolPreference, + type McpServerConfig, + type McpServerStatus, + type McpTestResult, +} from '@maka/core/mcp'; import { createCredentialMcpOAuthStorage, McpClientManager } from '@maka/mcp'; import { createFileCredentialStore } from '@maka/storage/credential-store'; -import { createMcpConfigStore, type McpConfigStore } from '@maka/storage/mcp-config-store'; +import { + createMcpConfigStore, + McpConfigSourceError, + normalizeMcpConfig, + normalizeMcpImport, + type McpConfigStore, +} from '@maka/storage/mcp-config-store'; import type { ClientCapabilityProvider, RuntimeHostConnectionAvailability, @@ -40,6 +57,8 @@ export type TuiMcpPublicationState = export interface TuiMcpServerSnapshot { readonly serverId: string; + readonly configured: boolean; + readonly synchronized: boolean; readonly state: McpServerStatus['state']; readonly transport?: McpServerStatus['transport']; readonly negotiatedProtocol?: McpServerStatus['negotiatedProtocol']; @@ -49,6 +68,7 @@ export interface TuiMcpServerSnapshot { export interface TuiMcpSnapshot { readonly initialization: 'loading' | 'ready' | 'error'; + readonly configuration: 'ready' | 'synchronizing' | 'out_of_sync'; readonly publication: TuiMcpPublicationState; readonly toolCount: number; readonly servers: readonly TuiMcpServerSnapshot[]; @@ -59,13 +79,88 @@ export interface TuiMcpSurface { subscribe(listener: () => void): () => void; } -export interface TuiMcpController extends TuiMcpSurface { +export interface TuiMcpEditConfig { + readonly config: McpServerConfig; + readonly revision: string; +} + +export interface TuiMcpImportEntry { + readonly serverId: string; + readonly change: 'add' | 'replace'; + readonly transport: 'stdio' | 'remote'; + readonly protocol: McpProtocolPreference; +} + +export interface TuiMcpImportPreview { + readonly previewId: string; + readonly entries: readonly TuiMcpImportEntry[]; +} + +export type TuiMcpImportPreviewResult = + | { readonly status: 'ready'; readonly preview: TuiMcpImportPreview } + | { + readonly status: 'invalid'; + readonly reason: McpConfigSourceFailureReason | 'invalid-config' | 'not-ready'; + }; + +export type TuiMcpAction = + | { readonly kind: 'add'; readonly serverId: string; readonly config: McpServerConfig } + | { + readonly kind: 'edit'; + readonly serverId: string; + readonly config: McpServerConfig; + readonly expectedRevision: string; + } + | { readonly kind: 'commit_import'; readonly previewId: string } + | { readonly kind: 'set_enabled'; readonly serverId: string; readonly enabled: boolean } + | { readonly kind: 'remove'; readonly serverId: string } + | { readonly kind: 'test'; readonly serverId: string } + | { readonly kind: 'reconnect'; readonly serverId: string }; + +export type TuiMcpActionEffect = + | 'published' + | 'pending_host' + | 'sync_failed' + | 'publication_failed'; + +export type TuiMcpActionResult = + | { readonly status: 'applied'; readonly effect: TuiMcpActionEffect } + | { readonly status: 'tested'; readonly test: McpTestResult; readonly effect: TuiMcpActionEffect } + | { + readonly status: 'conflict'; + readonly reason: 'exists' | 'stale_config' | 'stale_edit' | 'stale_import' | 'missing'; + } + | { + readonly status: 'failed'; + readonly reason: + | 'closed' + | 'invalid-config' + | 'credential-cleanup-failed' + | 'persist-failed' + | 'manager-failed'; + }; + +export interface TuiMcpManagement extends TuiMcpSurface { + configForEdit(serverId: string): TuiMcpEditConfig | undefined; + previewImport(source: string): TuiMcpImportPreviewResult; + execute(action: TuiMcpAction): Promise; +} + +export interface TuiMcpController extends TuiMcpManagement { close(): Promise; } type TuiMcpManager = Pick< McpClientManager, - 'sync' | 'statuses' | 'toolSnapshot' | 'callTool' | 'onChange' | 'close' + | 'sync' + | 'statuses' + | 'toolSnapshot' + | 'callTool' + | 'onChange' + | 'test' + | 'reconnect' + | 'forgetServerCredentials' + | 'close' >; type TuiMcpConnection = Pick< @@ -74,7 +169,7 @@ type TuiMcpConnection = Pick< >; interface TuiMcpControllerDeps { - readonly configStore: Pick; + readonly configStore: Pick; readonly manager: TuiMcpManager; readonly createProvider: (manager: TuiMcpManager) => ClientCapabilityProvider | undefined; } @@ -109,6 +204,16 @@ class TuiMcpControllerImpl implements TuiMcpController { readonly #initialization: Promise; #availability: RuntimeHostConnectionAvailability = { kind: 'unavailable' }; #closed = false; + #config: McpConfigFile | undefined; + #preparedImport: + | { + readonly previewId: string; + readonly imported: McpConfigFile; + readonly basis: ReadonlyMap; + } + | undefined; + #actionLane: Promise = Promise.resolve(); + #publicationSuppressed = false; #publicationRequested = false; #publicationTask: Promise | undefined; #published: @@ -120,6 +225,7 @@ class TuiMcpControllerImpl implements TuiMcpController { | undefined; #snapshot: TuiMcpSnapshot = freezeSnapshot({ initialization: 'loading', + configuration: 'synchronizing', publication: 'waiting', toolCount: 0, servers: [], @@ -131,7 +237,9 @@ class TuiMcpControllerImpl implements TuiMcpController { this.#disposeManagerChange = deps.manager.onChange(() => { try { this.#refreshManagerSnapshot(); - if (this.#snapshot.initialization === 'ready') this.#requestPublication(); + if (this.#snapshot.initialization === 'ready' && !this.#publicationSuppressed) { + this.#requestPublication(); + } } catch { // An observation must never break the MCP manager's state transition. } @@ -160,18 +268,68 @@ class TuiMcpControllerImpl implements TuiMcpController { return () => this.#listeners.delete(listener); } + configForEdit(serverId: string): TuiMcpEditConfig | undefined { + const config = this.#config?.mcpServers[serverId]; + if (!config) return undefined; + return { config: structuredClone(config), revision: configRevision(config) }; + } + + previewImport(source: string): TuiMcpImportPreviewResult { + const current = this.#config; + if (this.#closed || !current || this.#snapshot.initialization !== 'ready') { + return { status: 'invalid', reason: 'not-ready' }; + } + let imported: McpConfigFile; + try { + imported = normalizeMcpImport(source); + } catch (error) { + this.#preparedImport = undefined; + return { + status: 'invalid', + reason: error instanceof McpConfigSourceError ? error.reason : 'invalid-config', + }; + } + const previewId = randomUUID(); + const basis = new Map(); + const entries = Object.entries(imported.mcpServers).map(([serverId, config]) => { + const previous = current.mcpServers[serverId]; + basis.set(serverId, configRevision(previous)); + return Object.freeze({ + serverId, + change: previous ? ('replace' as const) : ('add' as const), + transport: 'command' in config ? ('stdio' as const) : ('remote' as const), + protocol: resolveMcpProtocolPreference(config), + }); + }); + this.#preparedImport = { previewId, imported, basis }; + return { + status: 'ready', + preview: Object.freeze({ previewId, entries: Object.freeze(entries) }), + }; + } + + execute(action: TuiMcpAction): Promise { + if (this.#closed) return Promise.resolve({ status: 'failed', reason: 'closed' }); + return this.#serializeAction(() => this.#executeAction(action)); + } + async close(): Promise { if (this.#closed) return; this.#closed = true; this.#disposeManagerChange(); this.#disposeConnectionAvailability(); this.#listeners.clear(); + this.#preparedImport = undefined; + this.#publicationRequested = false; + const managerClosing = this.#deps.manager.close(); + await this.#actionLane.catch(() => undefined); + this.#config = undefined; await this.#publicationTask?.catch(() => undefined); if (this.#availability.kind === 'connected') { await this.#connection.unregisterClientCapabilities().catch(() => undefined); } this.#published = undefined; - await this.#deps.manager.close(); + await managerClosing; await this.#initialization.catch(() => undefined); } @@ -181,7 +339,8 @@ class TuiMcpControllerImpl implements TuiMcpController { if (this.#closed) return; await this.#deps.manager.sync(config); if (this.#closed) return; - this.#refreshManagerSnapshot('ready'); + this.#config = cloneConfig(config); + this.#refreshManagerSnapshot('ready', 'ready'); this.#requestPublication(); } catch { if (this.#closed) return; @@ -189,19 +348,204 @@ class TuiMcpControllerImpl implements TuiMcpController { } } - #refreshManagerSnapshot(initialization = this.#snapshot.initialization): void { + #serializeAction(work: () => Promise): Promise { + const run = this.#actionLane.then(work, work); + this.#actionLane = run.then( + () => undefined, + () => undefined, + ); + return run; + } + + async #executeAction(action: TuiMcpAction): Promise { + if (this.#closed) return { status: 'failed', reason: 'closed' }; + if (action.kind === 'test') { + try { + const test = await this.#deps.manager.test(action.serverId); + return { status: 'tested', test, effect: await this.#settlePublication() }; + } catch { + return { status: 'failed', reason: 'manager-failed' }; + } + } + if (action.kind === 'reconnect') { + try { + await this.#deps.manager.reconnect(action.serverId); + return { status: 'applied', effect: await this.#settlePublication() }; + } catch { + this.#refreshManagerSnapshot(); + return { status: 'failed', reason: 'manager-failed' }; + } + } + return this.#commitMutation(action); + } + + async #commitMutation( + action: Exclude, + ): Promise { + let current: McpConfigFile; + try { + current = await this.#deps.configStore.get(); + } catch { + return { status: 'failed', reason: 'persist-failed' }; + } + if (this.#closed) return { status: 'failed', reason: 'closed' }; + const prepared = this.#prepareMutation(current, action); + if ('status' in prepared) return prepared; + const { next } = prepared; + const retirements = Object.entries(current.mcpServers) + .filter(([serverId, previous]) => + mcpConfigChangeRetiresCredentials(previous, next.mcpServers[serverId]), + ) + .map(([serverId]) => serverId); + try { + for (const serverId of retirements) { + await this.#deps.manager.forgetServerCredentials(serverId); + if (this.#closed) return { status: 'failed', reason: 'closed' }; + } + } catch { + return { status: 'failed', reason: 'credential-cleanup-failed' }; + } + const affectedServerIds = this.#affectedServerIds(action); + const basis = new Map( + affectedServerIds.map((serverId) => [serverId, configRevision(current.mcpServers[serverId])]), + ); + let committed: McpConfigFile; + try { + committed = await this.#deps.configStore.transform((actual) => { + for (const [serverId, revision] of basis) { + if (configRevision(actual.mcpServers[serverId]) !== revision) { + throw new TuiMcpConfigDriftError(); + } + } + const rebased = this.#prepareMutation(actual, action); + if ('status' in rebased) throw new TuiMcpConfigDriftError(); + return rebased.next; + }); + } catch (error) { + if (error instanceof TuiMcpConfigDriftError) { + return { status: 'conflict', reason: mutationConflictReason(action) }; + } + return { status: 'failed', reason: 'persist-failed' }; + } + if (this.#closed) return { status: 'failed', reason: 'closed' }; + this.#preparedImport = undefined; + this.#config = cloneConfig(committed); + this.#updateSnapshot({ configuration: 'synchronizing' }); + this.#refreshManagerSnapshot(); + this.#publicationSuppressed = true; + try { + await this.#deps.manager.sync(committed); + } catch { + this.#publicationSuppressed = false; + this.#updateSnapshot({ configuration: 'out_of_sync' }); + this.#refreshManagerSnapshot(); + await this.#settlePublication(); + return { status: 'applied', effect: 'sync_failed' }; + } + this.#publicationSuppressed = false; + if (this.#closed) return { status: 'failed', reason: 'closed' }; + this.#updateSnapshot({ configuration: 'ready' }); + this.#refreshManagerSnapshot(); + return { status: 'applied', effect: await this.#settlePublication() }; + } + + #affectedServerIds(action: Exclude): string[] { + if (action.kind !== 'commit_import') return [action.serverId]; + return [...(this.#preparedImport?.basis.keys() ?? [])]; + } + + #prepareMutation( + current: McpConfigFile, + action: Exclude, + ): + | { readonly next: McpConfigFile } + | Extract { + const servers = { ...current.mcpServers }; + if (action.kind === 'add') { + if (Object.hasOwn(servers, action.serverId)) return { status: 'conflict', reason: 'exists' }; + servers[action.serverId] = action.config; + } else if (action.kind === 'edit') { + const previous = servers[action.serverId]; + if (!previous) return { status: 'conflict', reason: 'missing' }; + if (configRevision(previous) !== action.expectedRevision) { + return { status: 'conflict', reason: 'stale_edit' }; + } + servers[action.serverId] = action.config; + } else if (action.kind === 'set_enabled') { + const previous = servers[action.serverId]; + if (!previous) return { status: 'conflict', reason: 'missing' }; + servers[action.serverId] = { ...previous, enabled: action.enabled }; + } else if (action.kind === 'remove') { + if (!Object.hasOwn(servers, action.serverId)) { + return { status: 'conflict', reason: 'missing' }; + } + delete servers[action.serverId]; + } else { + const prepared = this.#preparedImport; + if (!prepared || prepared.previewId !== action.previewId) { + return { status: 'conflict', reason: 'stale_import' }; + } + for (const [serverId, revision] of prepared.basis) { + if (configRevision(servers[serverId]) !== revision) { + return { status: 'conflict', reason: 'stale_import' }; + } + } + Object.assign(servers, prepared.imported.mcpServers); + } + try { + return { + next: normalizeMcpConfig({ version: MCP_CONFIG_VERSION, mcpServers: servers }), + }; + } catch { + return { status: 'failed', reason: 'invalid-config' }; + } + } + + async #settlePublication(): Promise { + this.#requestPublication(); + while (!this.#closed && (this.#publicationTask || this.#publicationRequested)) { + await this.#publicationTask?.catch(() => undefined); + } + if (this.#snapshot.publication === 'error') return 'publication_failed'; + if (this.#snapshot.publication === 'host_unavailable') return 'pending_host'; + return 'published'; + } + + #refreshManagerSnapshot( + initialization = this.#snapshot.initialization, + configuration = this.#snapshot.configuration, + ): void { const statuses = this.#deps.manager.statuses(); + const statusById = new Map(statuses.map((status) => [status.serverId, status])); + const serverIds = new Set([ + ...Object.keys(this.#config?.mcpServers ?? {}), + ...statuses.map((status) => status.serverId), + ]); this.#snapshot = freezeSnapshot({ initialization, + configuration, publication: this.#snapshot.publication, toolCount: this.#deps.manager.toolSnapshot().tools.length, - servers: statuses.map(projectServerStatus), + servers: [...serverIds] + .sort((left, right) => left.localeCompare(right)) + .map((serverId) => + projectServerStatus( + serverId, + this.#config?.mcpServers[serverId] !== undefined, + statusById.get(serverId), + configuration === 'ready', + ), + ), }); this.#notify(); } #requestPublication(): void { - if (this.#closed || this.#snapshot.initialization !== 'ready') return; + if (this.#closed) { + this.#publicationRequested = false; + return; + } + if (this.#snapshot.initialization !== 'ready' || this.#publicationSuppressed) return; this.#publicationRequested = true; if (this.#publicationTask) return; this.#publicationTask = this.#runPublicationQueue().finally(() => { @@ -266,7 +610,9 @@ class TuiMcpControllerImpl implements TuiMcpController { ); } - #updateSnapshot(update: Partial>): void { + #updateSnapshot( + update: Partial>, + ): void { this.#snapshot = freezeSnapshot({ ...this.#snapshot, ...update }); this.#notify(); } @@ -282,14 +628,21 @@ class TuiMcpControllerImpl implements TuiMcpController { } } -function projectServerStatus(status: McpServerStatus): TuiMcpServerSnapshot { +function projectServerStatus( + serverId: string, + configured: boolean, + status: McpServerStatus | undefined, + configurationSynchronized: boolean, +): TuiMcpServerSnapshot { return { - serverId: status.serverId, - state: status.state, - ...(status.transport ? { transport: status.transport } : {}), - ...(status.negotiatedProtocol ? { negotiatedProtocol: status.negotiatedProtocol } : {}), - toolCount: status.toolCount, - ...(status.error ? { error: status.error } : {}), + serverId, + configured, + synchronized: configurationSynchronized && configured && status !== undefined, + state: status?.state ?? 'disconnected', + ...(status?.transport ? { transport: status.transport } : {}), + ...(status?.negotiatedProtocol ? { negotiatedProtocol: status.negotiatedProtocol } : {}), + toolCount: status?.toolCount ?? 0, + ...(status?.error ? { error: status.error } : {}), }; } @@ -313,3 +666,22 @@ async function closeProvider(provider: ClientCapabilityProvider | undefined): Pr // A rejected provider never crossed into Host ownership. } } + +function cloneConfig(config: McpConfigFile): McpConfigFile { + return structuredClone(config); +} + +function configRevision(config: McpConfigFile | McpServerConfig | undefined): string { + if (!config) return 'missing'; + return createHash('sha256').update(JSON.stringify(config)).digest('hex'); +} + +function mutationConflictReason( + action: Exclude, +): Extract['reason'] { + if (action.kind === 'commit_import') return 'stale_import'; + if (action.kind === 'edit') return 'stale_edit'; + return 'stale_config'; +} + +class TuiMcpConfigDriftError extends Error {} From 44e156c618a925c81a0d316b1069ceffc5dbc603 Mon Sep 17 00:00:00 2001 From: me2seeks Date: Fri, 28 Aug 2026 10:53:33 +0800 Subject: [PATCH 3/6] feat(tui): manage MCP servers from /mcp Generated-by: Codex --- .../src/main/__tests__/mcp-ipc-main.test.ts | 9 + apps/desktop/src/main/mcp-ipc-main.ts | 2 + .../src/__tests__/pi-tui-mcp-status.test.ts | 20 +- .../cli/src/__tests__/pi-tui-runner.test.ts | 184 +++++ .../cli/src/__tests__/tui-mcp-control.test.ts | 89 ++- packages/cli/src/pi-tui-mcp-status.ts | 693 ++++++++++++++++-- packages/cli/src/pi-tui-runner.ts | 12 +- packages/cli/src/runtime-host-tui-context.ts | 4 +- packages/cli/src/tui-mcp-control.ts | 39 +- packages/cli/src/tui-primary-guidance.ts | 4 +- packages/mcp/src/index.ts | 7 +- packages/storage/src/mcp-config-store.ts | 6 +- 12 files changed, 976 insertions(+), 93 deletions(-) diff --git a/apps/desktop/src/main/__tests__/mcp-ipc-main.test.ts b/apps/desktop/src/main/__tests__/mcp-ipc-main.test.ts index 107c66856b..f199aacf13 100644 --- a/apps/desktop/src/main/__tests__/mcp-ipc-main.test.ts +++ b/apps/desktop/src/main/__tests__/mcp-ipc-main.test.ts @@ -492,6 +492,15 @@ test('a URL change retires the old endpoint credentials before the write, and an assert.ok(kept && 'url' in kept); assert.equal(kept.url, 'https://old.example.com/mcp'); + // Policy failures are known before the credential-first transaction: an + // invalid replacement must not log the user out when it cannot be saved. + calls.length = 0; + await assert.rejects( + upsert({}, 'remote', { url: 'http://public.example.com/mcp' }), + /must use https/u, + ); + assert.deepEqual(calls, []); + // Same repoint with a healthy credential store: erase strictly precedes // the write. An unchanged-URL upsert afterwards does not erase at all. eraseFails = false; diff --git a/apps/desktop/src/main/mcp-ipc-main.ts b/apps/desktop/src/main/mcp-ipc-main.ts index 38c1081aab..2035f1c871 100644 --- a/apps/desktop/src/main/mcp-ipc-main.ts +++ b/apps/desktop/src/main/mcp-ipc-main.ts @@ -29,6 +29,7 @@ import { } from '@maka/core/mcp'; import type { McpClientManager } from '@maka/mcp'; import { + assertMcpEndpointPolicyOnChanges, McpServerExistsError, McpConfigSourceError, normalizeMcpConfig, @@ -110,6 +111,7 @@ export function registerMcpIpcMain(deps: McpIpcMainDeps): void { ): Promise => { const current = await deps.store.get(); const next = mutate(current); + assertMcpEndpointPolicyOnChanges(current, next); // The authoritative gate: every server this commit semantically touches // is re-checked INSIDE the lane. The handler-entry checks are advisory // fast-fails; this one cannot race a login claim, because claims travel diff --git a/packages/cli/src/__tests__/pi-tui-mcp-status.test.ts b/packages/cli/src/__tests__/pi-tui-mcp-status.test.ts index f510e02339..c8717d0f77 100644 --- a/packages/cli/src/__tests__/pi-tui-mcp-status.test.ts +++ b/packages/cli/src/__tests__/pi-tui-mcp-status.test.ts @@ -19,13 +19,13 @@ import assert from 'node:assert/strict'; import { describe, test } from 'node:test'; -import { McpStatusOverlay } from '../pi-tui-mcp-status.js'; -import type { TuiMcpSnapshot, TuiMcpSurface } from '../tui-mcp-control.js'; +import { McpManagementOverlay } from '../pi-tui-mcp-status.js'; +import type { TuiMcpManagement, TuiMcpSnapshot } from '../tui-mcp-control.js'; import { stripAnsi } from '../tui-ansi.js'; -describe('MCP status overlay', () => { +describe('MCP management overlay', () => { test('renders the local publication and negotiated server status', () => { - const overlay = new McpStatusOverlay({ + const overlay = new McpManagementOverlay({ locale: 'en', surface: surface({ initialization: 'ready', @@ -55,7 +55,7 @@ describe('MCP status overlay', () => { }); test('states the remote limitation instead of implying an empty local config', () => { - const overlay = new McpStatusOverlay({ + const overlay = new McpManagementOverlay({ locale: 'zh', viewportRows: () => 6, onClose: () => undefined, @@ -69,7 +69,7 @@ describe('MCP status overlay', () => { }); test('localizes manager states without changing their source values', () => { - const overlay = new McpStatusOverlay({ + const overlay = new McpManagementOverlay({ locale: 'zh', surface: surface({ initialization: 'ready', @@ -114,7 +114,7 @@ describe('MCP status overlay', () => { disposed += 1; }; }; - const overlay = new McpStatusOverlay({ + const overlay = new McpManagementOverlay({ locale: 'en', surface: mcp, viewportRows: () => 6, @@ -133,9 +133,13 @@ describe('MCP status overlay', () => { function surface( snapshot: TuiMcpSnapshot, -): TuiMcpSurface & { subscribe(listener: () => void): () => void } { +): TuiMcpManagement & { subscribe(listener: () => void): () => void } { return { snapshot: () => snapshot, subscribe: () => () => undefined, + configForEdit: () => undefined, + previewImport: () => ({ status: 'invalid', reason: 'invalid-config' }), + discardImportPreview: () => undefined, + execute: async () => ({ status: 'failed', reason: 'manager-failed' }), }; } diff --git a/packages/cli/src/__tests__/pi-tui-runner.test.ts b/packages/cli/src/__tests__/pi-tui-runner.test.ts index b37151fc64..1c0b3eb283 100644 --- a/packages/cli/src/__tests__/pi-tui-runner.test.ts +++ b/packages/cli/src/__tests__/pi-tui-runner.test.ts @@ -77,6 +77,7 @@ import { import { AUTO_RECAP_IDLE_MS } from '../session-recap.js'; import { BUSY_SPINNER_FRAMES } from '../tui-attention.js'; import { EXPANSION_COLLAPSE_CONFIRM_WINDOW_MS } from '../pi-transcript.js'; +import type { TuiMcpAction, TuiMcpManagement } from '../tui-mcp-control.js'; import { autocompleteSuggestionLines, assertBottomPickerPlacement, @@ -2235,6 +2236,10 @@ describe('Maka Pi TUI runner', () => { ], }), subscribe: () => () => undefined, + configForEdit: () => undefined, + previewImport: () => ({ status: 'invalid', reason: 'invalid-config' }), + discardImportPreview: () => undefined, + execute: async () => ({ status: 'failed', reason: 'manager-failed' }), }, }); @@ -2248,6 +2253,185 @@ describe('Maka Pi TUI runner', () => { await run; }); + test('/mcp previews pasted JSON and completes guided setup in-frame', async () => { + const terminal = new FakeTerminal(); + const driver = new SlashCommandDriver(); + const actions: TuiMcpAction[] = []; + const discardedPreviews: string[] = []; + let previewSource = ''; + const mcp: TuiMcpManagement = { + snapshot: () => ({ + initialization: 'ready', + configuration: 'ready', + publication: 'not_published', + toolCount: 0, + servers: [], + }), + subscribe: () => () => undefined, + configForEdit: () => undefined, + previewImport: (source) => { + previewSource = source; + return { + status: 'ready', + preview: { + previewId: 'preview-1', + entries: [ + { + serverId: 'docs', + change: 'add', + transport: 'remote', + protocol: 'auto', + }, + ], + }, + }; + }, + discardImportPreview: (previewId) => discardedPreviews.push(previewId), + execute: async (action) => { + actions.push(action); + return { status: 'applied', effect: 'published' }; + }, + }; + const run = runMakaPiTui({ + title: 'Maka', + driver, + cwd: '/repo', + model: 'deepseek-v4-flash', + connectionSlug: 'deepseek', + permissionMode: 'ask', + terminal, + mcp, + }); + + terminal.input('/mcp'); + terminal.input('\r'); + await waitFor(() => plainTerminalOutput(terminal.screenOutput()).includes('MCP SERVERS')); + terminal.input('a'); + terminal.input('j'); + terminal.input('secret-draft'); + await waitFor(() => plainTerminalOutput(terminal.screenOutput()).includes('secret-draft')); + terminal.input('\x1b'); + await waitFor(() => !plainTerminalOutput(terminal.screenOutput()).includes('secret-draft')); + terminal.input('a'); + terminal.input('j'); + terminal.input('\x1f'); + await delay(0); + assert.doesNotMatch(plainTerminalOutput(terminal.screenOutput()), /secret-draft/u); + terminal.input('{"docs":{"url":"https://docs.example/mcp","protocol":"auto"}}'); + terminal.input('\r'); + await waitFor(() => + plainTerminalOutput(terminal.screenOutput()).includes('Import MCP servers?'), + ); + assert.equal(actions.length, 0); + assert.equal(previewSource.includes('docs.example'), true); + terminal.input('\x1b'); + assert.deepEqual(discardedPreviews, ['preview-1']); + terminal.input('a'); + terminal.input('j'); + terminal.input('{"docs":{"url":"https://docs.example/mcp","protocol":"auto"}}'); + terminal.input('\r'); + await waitFor(() => + plainTerminalOutput(terminal.screenOutput()).includes('Import MCP servers?'), + ); + terminal.input('y'); + await waitFor(() => actions.length === 1); + assert.deepEqual(actions[0], { kind: 'commit_import', previewId: 'preview-1' }); + await waitFor(() => plainTerminalOutput(terminal.screenOutput()).includes('tools refreshed')); + + terminal.input('a'); + terminal.input('g'); + terminal.input('local'); + terminal.input('\r'); + terminal.input('1'); + terminal.input('mcp-server'); + terminal.input('\r'); + terminal.input('[]'); + terminal.input('\r'); + terminal.input('3'); + terminal.input('\r'); + terminal.input('{}'); + terminal.input('\r'); + await waitFor(() => + plainTerminalOutput(terminal.screenOutput()).includes('Add this MCP server?'), + ); + terminal.input('y'); + await waitFor(() => actions.length === 2); + assert.deepEqual(actions[1], { + kind: 'add', + serverId: 'local', + config: { + enabled: true, + command: 'mcp-server', + args: [], + env: {}, + protocol: '2026-07-28', + }, + }); + terminal.input('q'); + exitMaka(terminal); + await run; + }); + + test('/mcp ignores a late action result after the user cancels its busy view', async () => { + const terminal = new FakeTerminal(); + const driver = new SlashCommandDriver(); + const result = deferred>>(); + const actions: TuiMcpAction[] = []; + const mcp: TuiMcpManagement = { + snapshot: () => ({ + initialization: 'ready', + configuration: 'ready', + publication: 'not_published', + toolCount: 0, + servers: [ + { + serverId: 'docs', + configured: true, + synchronized: true, + enabled: false, + configuredTransport: 'remote', + configuredProtocol: 'auto', + state: 'disabled', + toolCount: 0, + }, + ], + }), + subscribe: () => () => undefined, + configForEdit: () => undefined, + previewImport: () => ({ status: 'invalid', reason: 'invalid-config' }), + discardImportPreview: () => undefined, + execute: async (action) => { + actions.push(action); + return result.promise; + }, + }; + const run = runMakaPiTui({ + title: 'Maka', + driver, + cwd: '/repo', + model: 'deepseek-v4-flash', + connectionSlug: 'deepseek', + permissionMode: 'ask', + terminal, + mcp, + }); + + terminal.input('/mcp'); + terminal.input('\r'); + await waitFor(() => plainTerminalOutput(terminal.screenOutput()).includes('docs')); + terminal.input(' '); + await waitFor(() => actions.length === 1); + assert.deepEqual(actions[0], { kind: 'set_enabled', serverId: 'docs', enabled: true }); + terminal.input('\x1b'); + await waitFor(() => plainTerminalOutput(terminal.screenOutput()).includes('docs')); + result.resolve({ status: 'failed', reason: 'manager-failed' }); + await delay(0); + assert.doesNotMatch(plainTerminalOutput(terminal.screenOutput()), /connection action failed/u); + terminal.input('q'); + exitMaka(terminal); + await run; + }); + test('clears an unsent draft on Ctrl-C without closing Maka', async () => { const terminal = new FakeTerminal(); const driver = new SlashCommandDriver(); diff --git a/packages/cli/src/__tests__/tui-mcp-control.test.ts b/packages/cli/src/__tests__/tui-mcp-control.test.ts index 1091c11d00..9a3db41412 100644 --- a/packages/cli/src/__tests__/tui-mcp-control.test.ts +++ b/packages/cli/src/__tests__/tui-mcp-control.test.ts @@ -251,6 +251,36 @@ test('TUI MCP retires endpoint credentials before persistence and aborts on clea await controller.close(); }); +test('TUI MCP rejects an invalid endpoint before retiring the previous credentials', async () => { + const order: string[] = []; + const store = mutableConfigStore( + { version: 3, mcpServers: { docs: { url: 'https://old.example/mcp' } } }, + order, + ); + const manager = managementManager(order); + const connection = connectionHarness(); + const controller = createTuiMcpController( + { workspaceRoot: '/unused', connection: connection.connection }, + { configStore: store.store, manager: manager.manager, createProvider: () => undefined }, + ); + await waitFor(() => controller.snapshot().initialization === 'ready'); + const edit = controller.configForEdit('docs'); + assert.ok(edit); + order.length = 0; + + assert.deepEqual( + await controller.execute({ + kind: 'edit', + serverId: 'docs', + expectedRevision: edit.revision, + config: { url: 'http://public.example/mcp' }, + }), + { status: 'failed', reason: 'invalid-config' }, + ); + assert.deepEqual(order, ['get']); + await controller.close(); +}); + test('TUI MCP edit rejects a stale revision without touching credentials or disk', async () => { const order: string[] = []; const store = mutableConfigStore( @@ -357,7 +387,9 @@ test('TUI MCP keeps a durable mutation visible when manager synchronization fail serverId: 'local', configured: true, synchronized: false, - state: 'disconnected', + enabled: true, + configuredTransport: 'stdio', + configuredProtocol: 'legacy', toolCount: 0, }); assert.ok((await store.store.get()).mcpServers.local); @@ -502,6 +534,47 @@ test('TUI MCP rebases an action over an unrelated concurrent config edit', async await controller.close(); }); +test('TUI MCP manages enabled state, tests, reconnects, and removes through one lane', async () => { + const order: string[] = []; + const store = mutableConfigStore( + { + version: 3, + mcpServers: { docs: { enabled: false, url: 'https://docs.example/mcp' } }, + }, + order, + ); + const manager = managementManager(order); + const connection = connectionHarness(); + const controller = createTuiMcpController( + { workspaceRoot: '/unused', connection: connection.connection }, + { configStore: store.store, manager: manager.manager, createProvider: () => undefined }, + ); + await waitFor(() => controller.snapshot().initialization === 'ready'); + order.length = 0; + + assert.deepEqual( + await controller.execute({ kind: 'set_enabled', serverId: 'docs', enabled: true }), + { status: 'applied', effect: 'published' }, + ); + assert.equal((await store.store.get()).mcpServers.docs?.enabled, true); + const tested = await controller.execute({ kind: 'test', serverId: 'docs' }); + assert.equal(tested.status, 'tested'); + assert.deepEqual(await controller.execute({ kind: 'reconnect', serverId: 'docs' }), { + status: 'applied', + effect: 'published', + }); + assert.deepEqual(await controller.execute({ kind: 'remove', serverId: 'docs' }), { + status: 'applied', + effect: 'published', + }); + assert.deepEqual( + order.filter((entry) => entry.startsWith('test') || entry.startsWith('reconnect')), + ['test:docs', 'reconnect:docs'], + ); + assert.equal((await store.store.get()).mcpServers.docs, undefined); + await controller.close(); +}); + function mutableConfigStore(initial: McpConfigFile, order: string[]) { let config = structuredClone(initial); const store = { @@ -544,12 +617,14 @@ function managementManager( statuses: () => statuses, toolSnapshot: () => ({ revision, tools: [] }) as McpToolSnapshot, callTool: async () => ({ content: [] }), - test: async (serverId: string) => ({ - ok: true, - status: connectedStatus(serverId, 0), - latencyMs: 1, - }), - reconnect: async (serverId: string) => connectedStatus(serverId, 0), + test: async (serverId: string) => { + order.push(`test:${serverId}`); + return { ok: true, status: connectedStatus(serverId, 0), latencyMs: 1 }; + }, + reconnect: async (serverId: string) => { + order.push(`reconnect:${serverId}`); + return connectedStatus(serverId, 0); + }, forgetServerCredentials: async (serverId: string) => { order.push(`forget:${serverId}`); if (options.credentialFailure) throw new Error('credential cleanup failed'); diff --git a/packages/cli/src/pi-tui-mcp-status.ts b/packages/cli/src/pi-tui-mcp-status.ts index 105a8b0a89..f10807a555 100644 --- a/packages/cli/src/pi-tui-mcp-status.ts +++ b/packages/cli/src/pi-tui-mcp-status.ts @@ -18,28 +18,89 @@ */ import { + Editor, Key, matchesKey, truncateToWidth, visibleWidth, type Component, + type TUI, } from '@earendil-works/pi-tui'; +import type { McpProtocolPreference, McpServerConfig } from '@maka/core/mcp'; import type { UiLocale } from '@maka/core/ui-locale'; -import type { TuiMcpServerSnapshot, TuiMcpSurface } from './tui-mcp-control.js'; -import { ansi } from './tui-ansi.js'; +import { normalizeMcpConfig } from '@maka/storage/mcp-config-store'; +import type { + TuiMcpAction, + TuiMcpActionResult, + TuiMcpImportPreview, + TuiMcpManagement, + TuiMcpServerSnapshot, +} from './tui-mcp-control.js'; +import { ansi, editorTheme } from './tui-ansi.js'; const CHROME_ROWS = 2; -export class McpStatusOverlay implements Component { +type GuidedDraft = { + serverId: string; + transport?: 'stdio' | 'remote'; + command?: string; + args?: string[]; + url?: string; + cwd?: string; + env?: Record; + headers?: Record; + protocol?: McpProtocolPreference; +}; + +type InputKind = + | 'server_id' + | 'command' + | 'args' + | 'url' + | 'cwd' + | 'env' + | 'headers' + | 'edit' + | 'import'; + +type McpOverlayPhase = + | { kind: 'list' } + | { kind: 'add_choice' } + | { kind: 'transport'; draft: GuidedDraft } + | { kind: 'protocol'; draft: GuidedDraft } + | { + kind: 'input'; + input: InputKind; + draft?: GuidedDraft; + serverId?: string; + revision?: string; + } + | { kind: 'confirm_add'; draft: GuidedDraft } + | { kind: 'confirm_import'; preview: TuiMcpImportPreview } + | { kind: 'confirm_remove'; serverId: string } + | { kind: 'busy'; label: string }; + +/** One in-frame state machine for status, editing, confirmation, and errors. + * Raw config values live only in the editor and are cleared on every exit; + * no management result is written into the conversation transcript. */ +export class McpManagementOverlay implements Component { private top = 0; private documentRows = 0; private bodyRows = 0; + private selected = 0; + private phase: McpOverlayPhase = { kind: 'list' }; + private notice: { level: 'info' | 'error'; text: string } | undefined; private readonly dispose: () => void; + private editor: Editor | undefined; + private closed = false; + private actionAttempt = 0; constructor( private readonly input: { readonly locale: UiLocale; - readonly surface?: TuiMcpSurface; + readonly tui?: TUI; + readonly surface?: TuiMcpManagement; + canManage?(): boolean; viewportRows(): number; onClose(): void; onChange(): void; @@ -48,20 +109,53 @@ export class McpStatusOverlay implements Component { this.dispose = input.surface?.subscribe(input.onChange) ?? (() => undefined); } - invalidate(): void {} + invalidate(): void { + this.editor?.invalidate(); + } handleInput(data: string): void { + if (this.phase.kind === 'input') { + if (matchesKey(data, Key.escape) || matchesKey(data, Key.ctrl('c'))) { + this.backToList(); + } else { + this.editor?.handleInput(data); + } + return; + } + if (this.phase.kind === 'busy') { + if (matchesKey(data, Key.escape)) { + this.actionAttempt += 1; + this.backToList(); + } else if (matchesKey(data, 'q')) { + this.actionAttempt += 1; + this.close(); + } + return; + } if (matchesKey(data, Key.escape) || matchesKey(data, 'q')) { - this.dispose(); - this.input.onClose(); + if (this.phase.kind === 'list') this.close(); + else this.backToList(); return; } - if (matchesKey(data, Key.up)) this.scrollBy(-1); - else if (matchesKey(data, Key.down)) this.scrollBy(1); - else if (matchesKey(data, Key.pageUp)) this.scrollBy(-Math.max(1, this.bodyRows)); - else if (matchesKey(data, Key.pageDown)) this.scrollBy(Math.max(1, this.bodyRows)); - else if (matchesKey(data, Key.home)) this.scrollTo(0); - else if (matchesKey(data, Key.end)) this.scrollTo(this.maxTop()); + if (this.phase.kind === 'list') this.handleListInput(data); + else if (this.phase.kind === 'add_choice') this.handleAddChoice(data); + else if (this.phase.kind === 'transport') this.handleTransport(data); + else if (this.phase.kind === 'protocol') this.handleProtocol(data); + else if (this.phase.kind === 'confirm_add' && matchesKey(data, 'y')) { + try { + const config = guidedConfig(this.phase.draft); + if (config) { + void this.runAction({ kind: 'add', serverId: this.phase.draft.serverId, config }); + } + } catch { + this.notice = { level: 'error', text: copy(this.input.locale, 'invalid') }; + this.backToList(false); + } + } else if (this.phase.kind === 'confirm_import' && matchesKey(data, 'y')) { + void this.runAction({ kind: 'commit_import', previewId: this.phase.preview.previewId }); + } else if (this.phase.kind === 'confirm_remove' && matchesKey(data, 'y')) { + void this.runAction({ kind: 'remove', serverId: this.phase.serverId }); + } } render(width: number): string[] { @@ -69,7 +163,7 @@ export class McpStatusOverlay implements Component { const viewportRows = Math.max(1, Math.floor(this.input.viewportRows())); const showFooter = viewportRows > 2; this.bodyRows = Math.max(0, viewportRows - (showFooter ? CHROME_ROWS : 1)); - const document = this.document(); + const document = this.document(safeWidth); this.documentRows = document.length; this.top = clamp(this.top, 0, this.maxTop()); const visible = document.slice(this.top, this.top + this.bodyRows); @@ -87,56 +181,318 @@ export class McpStatusOverlay implements Component { ), ]; if (!showFooter) return [header, ...body]; - const footer = - this.input.locale === 'zh' - ? '↑/↓ 滚动 · PgUp/PgDn 翻页 · Home/End 跳转 · q/Esc 关闭' - : '↑/↓ scroll · PgUp/PgDn page · Home/End jump · q/Esc close'; - return [header, ...body, padLine(ansi.dim(footer), safeWidth)]; + return [header, ...body, padLine(ansi.dim(this.footer()), safeWidth)]; + } + + private management(): TuiMcpManagement | undefined { + return this.input.surface; + } + + private handleListInput(data: string): void { + const servers = this.input.surface?.snapshot().servers ?? []; + if (matchesKey(data, Key.up)) { + this.selected = clamp(this.selected - 1, 0, servers.length - 1); + } else if (matchesKey(data, Key.down)) { + this.selected = clamp(this.selected + 1, 0, servers.length - 1); + } else if (matchesKey(data, Key.pageUp)) this.scrollBy(-Math.max(1, this.bodyRows)); + else if (matchesKey(data, Key.pageDown)) this.scrollBy(Math.max(1, this.bodyRows)); + else if (matchesKey(data, Key.home)) this.scrollTo(0); + else if (matchesKey(data, Key.end)) this.scrollTo(this.maxTop()); + else if (matchesKey(data, 'a') && this.management()) this.phase = { kind: 'add_choice' }; + else { + const server = servers[this.selected]; + if (!server || !this.management()) return; + if (matchesKey(data, Key.enter)) this.startEdit(server.serverId); + else if (matchesKey(data, Key.space)) { + void this.runAction({ + kind: 'set_enabled', + serverId: server.serverId, + enabled: !server.enabled, + }); + } else if (matchesKey(data, 't')) { + void this.runAction({ kind: 'test', serverId: server.serverId }); + } else if (matchesKey(data, 'r')) { + void this.runAction({ kind: 'reconnect', serverId: server.serverId }); + } else if (matchesKey(data, 'd') && server.configured) { + this.phase = { kind: 'confirm_remove', serverId: server.serverId }; + } + } + this.input.onChange(); + } + + private handleAddChoice(data: string): void { + if (matchesKey(data, 'g')) this.startInput('server_id', { serverId: '' }); + else if (matchesKey(data, 'j')) this.startInput('import'); + } + + private handleTransport(data: string): void { + if (this.phase.kind !== 'transport') return; + if (matchesKey(data, '1')) { + this.phase.draft.transport = 'stdio'; + this.startInput('command', this.phase.draft); + } else if (matchesKey(data, '2')) { + this.phase.draft.transport = 'remote'; + this.startInput('url', this.phase.draft); + } + } + + private handleProtocol(data: string): void { + if (this.phase.kind !== 'protocol') return; + const protocol = matchesKey(data, '1') + ? 'legacy' + : matchesKey(data, '2') + ? 'auto' + : matchesKey(data, '3') + ? '2026-07-28' + : undefined; + if (!protocol) return; + this.phase.draft.protocol = protocol; + this.startInput(this.phase.draft.transport === 'stdio' ? 'cwd' : 'headers', this.phase.draft); + } + + private startEdit(serverId: string): void { + const edit = this.management()?.configForEdit(serverId); + if (!edit) { + this.notice = { level: 'error', text: copy(this.input.locale, 'missing') }; + return; + } + this.startInput( + 'edit', + undefined, + serverId, + edit.revision, + JSON.stringify(edit.config, null, 2), + ); + } + + private startInput( + input: InputKind, + draft?: GuidedDraft, + serverId?: string, + revision?: string, + value = '', + ): void { + if (!this.input.tui) return; + this.clearEditor(); + this.notice = undefined; + this.phase = { kind: 'input', input, draft, serverId, revision }; + this.editor = new Editor(this.input.tui, editorTheme(), { paddingX: 0 }); + this.editor.onSubmit = (submitted) => this.submitInput(submitted); + this.editor.setText(value); + this.editor.focused = true; + this.input.onChange(); + } + + private submitInput(value: string): void { + if (this.phase.kind !== 'input') return; + const phase = this.phase; + const trimmed = value.trim(); + try { + if (phase.input === 'server_id') { + if (!trimmed) throw new Error(); + const draft = phase.draft ?? { serverId: '' }; + draft.serverId = trimmed; + this.clearEditor(); + this.phase = { kind: 'transport', draft }; + } else if (phase.input === 'command') { + if (!trimmed || !phase.draft) throw new Error(); + phase.draft.command = trimmed; + this.startInput('args', phase.draft); + } else if (phase.input === 'args') { + if (!phase.draft) throw new Error(); + phase.draft.args = trimmed ? stringArray(trimmed) : undefined; + this.clearEditor(); + this.phase = { kind: 'protocol', draft: phase.draft }; + } else if (phase.input === 'url') { + if (!trimmed || !phase.draft) throw new Error(); + phase.draft.url = trimmed; + this.clearEditor(); + this.phase = { kind: 'protocol', draft: phase.draft }; + } else if (phase.input === 'cwd') { + if (!phase.draft) throw new Error(); + phase.draft.cwd = trimmed || undefined; + this.startInput('env', phase.draft); + } else if (phase.input === 'env') { + if (!phase.draft) throw new Error(); + phase.draft.env = trimmed ? stringMap(trimmed) : undefined; + this.clearEditor(); + this.phase = { kind: 'confirm_add', draft: phase.draft }; + } else if (phase.input === 'headers') { + if (!phase.draft) throw new Error(); + phase.draft.headers = trimmed ? stringMap(trimmed) : undefined; + this.clearEditor(); + this.phase = { kind: 'confirm_add', draft: phase.draft }; + } else if (phase.input === 'edit') { + if (!phase.serverId || !phase.revision) throw new Error(); + const config = normalizeOneServer(phase.serverId, trimmed); + this.clearEditor(); + void this.runAction({ + kind: 'edit', + serverId: phase.serverId, + expectedRevision: phase.revision, + config, + }); + } else { + const preview = this.management()?.previewImport(value); + if (!preview || preview.status !== 'ready') throw new Error(); + this.clearEditor(); + this.phase = { kind: 'confirm_import', preview: preview.preview }; + } + this.notice = undefined; + } catch { + this.notice = { level: 'error', text: copy(this.input.locale, 'invalid') }; + } + this.input.onChange(); } - private document(): string[] { + private async runAction(action: TuiMcpAction): Promise { + const management = this.management(); + if (!management || this.phase.kind === 'busy') return; + if (this.input.canManage && !this.input.canManage()) { + this.backToList(false); + this.notice = { level: 'error', text: copy(this.input.locale, 'turn_active') }; + this.input.onChange(); + return; + } + this.clearEditor(); + const attempt = ++this.actionAttempt; + this.phase = { kind: 'busy', label: actionLabel(action, this.input.locale) }; + this.input.onChange(); + let result: TuiMcpActionResult; + try { + result = await management.execute(action); + } catch { + result = { status: 'failed', reason: 'manager-failed' }; + } + if (this.closed || attempt !== this.actionAttempt) return; + this.phase = { kind: 'list' }; + this.notice = actionNotice(result, this.input.locale); + this.input.onChange(); + } + + private document(width: number): string[] { const snapshot = this.input.surface?.snapshot(); - if (!snapshot) { - return this.input.locale === 'zh' - ? [ - ansi.yellow('当前 TUI 未连接本地 MCP 控制面。'), - '远程 Runtime Host 的客户端 MCP 工具关联将在后续版本提供。', - ] - : [ - ansi.yellow('This TUI is not connected to a local MCP control plane.'), - 'Client MCP tool association for remote Runtime Hosts is planned for a later release.', - ]; + if (!snapshot) return unavailableDocument(this.input.locale); + if (this.phase.kind === 'input') return this.inputDocument(width); + if (this.phase.kind === 'add_choice') { + return [ + heading(this.input.locale, 'Add MCP server', '添加 MCP 服务器'), + '', + 'g Guided setup', + 'j Paste JSON', + ]; + } + if (this.phase.kind === 'transport') { + return [ + heading(this.input.locale, 'Transport', '传输方式'), + '', + '1 stdio', + '2 Streamable HTTP', + ]; + } + if (this.phase.kind === 'protocol') { + return [ + heading(this.input.locale, 'Protocol preference', '协议偏好'), + '', + '1 legacy', + '2 auto', + '3 2026-07-28', + ]; + } + if (this.phase.kind === 'confirm_add') { + return confirmAddDocument(this.phase.draft, this.input.locale); } + if (this.phase.kind === 'confirm_import') { + return confirmImportDocument(this.phase.preview, this.input.locale); + } + if (this.phase.kind === 'confirm_remove') { + return [ + ansi.red( + heading( + this.input.locale, + `Remove ${this.phase.serverId}?`, + `删除 ${this.phase.serverId}？`, + ), + ), + '', + confirmCopy(this.input.locale), + ]; + } + if (this.phase.kind === 'busy') return [ansi.yellow(this.phase.label)]; const lines = [publicationLine(snapshot, this.input.locale)]; - if (snapshot.initialization === 'loading') { - lines.push( - this.input.locale === 'zh' - ? '正在读取 mcp.json 并发现工具…' - : 'Loading mcp.json and discovering tools…', - ); - return lines; + if (snapshot.configuration !== 'ready') { + lines.push(configurationLine(snapshot.configuration, this.input.locale)); } - if (snapshot.initialization === 'error') { + if (this.notice) { lines.push( - this.input.locale === 'zh' - ? ansi.red('无法读取或应用 MCP 配置；没有向 Runtime Host 发布工具。') - : ansi.red( - 'MCP configuration could not be loaded; no tools were published to the Runtime Host.', - ), + this.notice.level === 'error' ? ansi.red(this.notice.text) : ansi.green(this.notice.text), ); - return lines; } - if (snapshot.servers.length === 0) { - lines.push( - this.input.locale === 'zh' ? '尚未配置 MCP 服务器。' : 'No MCP servers are configured.', - ); - return lines; + if (snapshot.initialization === 'loading') return [...lines, loadingCopy(this.input.locale)]; + if (snapshot.initialization === 'error') { + return [...lines, ansi.red(loadErrorCopy(this.input.locale))]; } + if (snapshot.servers.length === 0) return [...lines, '', emptyCopy(this.input.locale)]; lines.push(''); - for (const server of snapshot.servers) lines.push(...serverLines(server, this.input.locale)); + this.selected = clamp(this.selected, 0, snapshot.servers.length - 1); + snapshot.servers.forEach((server, index) => { + lines.push(...serverLines(server, this.input.locale, index === this.selected)); + }); return lines; } + private inputDocument(width: number): string[] { + if (this.phase.kind !== 'input' || !this.editor) return []; + const label = inputLabel(this.phase.input, this.input.locale); + const hint = inputHint(this.phase.input, this.input.locale); + return [ + ansi.bold(label), + ...(hint ? [ansi.dim(hint)] : []), + '', + ...this.editor.render(Math.max(1, width - 2)), + ...(this.notice ? [ansi.red(this.notice.text)] : []), + ]; + } + + private footer(): string { + if (this.phase.kind !== 'list') return this.input.locale === 'zh' ? 'Esc 返回' : 'Esc back'; + if (!this.management()) { + return this.input.locale === 'zh' ? '↑/↓ 滚动 · q/Esc 关闭' : '↑/↓ scroll · q/Esc close'; + } + return this.input.locale === 'zh' + ? 'a 添加 · Enter 编辑 · Space 启用/停用 · t 测试 · r 重连 · d 删除 · Esc 关闭' + : 'a Add · Enter Edit · Space Enable/disable · t Test · r Reconnect · d Remove · Esc Close'; + } + + private backToList(clearNotice = true): void { + if (this.phase.kind === 'confirm_import') { + this.management()?.discardImportPreview(this.phase.preview.previewId); + } + this.clearEditor(); + this.phase = { kind: 'list' }; + if (clearNotice) this.notice = undefined; + this.input.onChange(); + } + + private clearEditor(): void { + if (!this.editor) return; + this.editor.focused = false; + this.editor.onSubmit = undefined; + this.editor.onChange = undefined; + this.editor = undefined; + } + + private close(): void { + if (this.closed) return; + this.closed = true; + if (this.phase.kind === 'confirm_import') { + this.management()?.discardImportPreview(this.phase.preview.previewId); + } + this.clearEditor(); + this.dispose(); + this.input.onClose(); + } + private scrollBy(delta: number): void { this.scrollTo(this.top + delta); } @@ -151,8 +507,48 @@ export class McpStatusOverlay implements Component { } } +function normalizeOneServer(serverId: string, source: string): McpServerConfig { + const value: unknown = JSON.parse(source); + return normalizeMcpConfig({ version: 3, mcpServers: { [serverId]: value } }).mcpServers[serverId]; +} + +function guidedConfig(draft: GuidedDraft): McpServerConfig | undefined { + if (!draft.transport || !draft.protocol) return undefined; + const raw = + draft.transport === 'stdio' + ? { + command: draft.command, + args: draft.args, + cwd: draft.cwd, + env: draft.env, + protocol: draft.protocol, + } + : { + url: draft.url, + transport: 'auto', + headers: draft.headers, + protocol: draft.protocol, + }; + return normalizeMcpConfig({ version: 3, mcpServers: { [draft.serverId]: raw } }).mcpServers[ + draft.serverId + ]; +} + +function stringArray(source: string): string[] { + const value: unknown = JSON.parse(source); + if (!Array.isArray(value) || value.some((entry) => typeof entry !== 'string')) throw new Error(); + return value; +} + +function stringMap(source: string): Record { + const value: unknown = JSON.parse(source); + if (!value || typeof value !== 'object' || Array.isArray(value)) throw new Error(); + if (Object.values(value).some((entry) => typeof entry !== 'string')) throw new Error(); + return value as Record; +} + function publicationLine( - snapshot: ReturnType, + snapshot: ReturnType, locale: UiLocale, ): string { const publication = { @@ -167,20 +563,204 @@ function publicationLine( return `${ansi.bold(publication)} · ${tools}`; } -function serverLines(server: TuiMcpServerSnapshot, locale: UiLocale): string[] { +function serverLines(server: TuiMcpServerSnapshot, locale: UiLocale, selected: boolean): string[] { const protocol = server.negotiatedProtocol ? `${server.negotiatedProtocol.era} ${server.negotiatedProtocol.revision}` - : undefined; + : server.configuredProtocol; + const transport = server.transport ?? server.configuredTransport; const tools = locale === 'zh' ? `${server.toolCount} 个工具` : `${server.toolCount} tools`; - const details = [stateLabel(server.state, locale), server.transport, protocol, tools] + const sync = !server.synchronized + ? locale === 'zh' + ? '配置待同步' + : 'config pending' + : undefined; + const details = [stateLabel(server.state, locale), transport, protocol, tools, sync] .filter(Boolean) .join(' · '); + const cursor = selected ? ansi.accent('›') : ' '; return [ - `${statusMarker(server.state)} ${ansi.bold(server.serverId)} ${details}`, - ...(server.error ? [` ${ansi.red(server.error)}`] : []), + `${cursor} ${statusMarker(server.state)} ${ansi.bold(server.serverId)} ${details}`, + ...(server.error ? [` ${ansi.red(server.error)}`] : []), ]; } +function actionNotice( + result: TuiMcpActionResult, + locale: UiLocale, +): { level: 'info' | 'error'; text: string } { + if (result.status === 'conflict') return { level: 'error', text: copy(locale, result.reason) }; + if (result.status === 'failed') return { level: 'error', text: copy(locale, result.reason) }; + if (result.status === 'tested') { + if (result.test.ok && result.effect === 'publication_failed') { + return { level: 'error', text: copy(locale, 'test_publication_failed') }; + } + if (result.test.ok && result.effect === 'pending_host') { + return { level: 'info', text: copy(locale, 'test_pending_host') }; + } + return { + level: result.test.ok ? 'info' : 'error', + text: result.test.ok ? copy(locale, 'test_ok') : copy(locale, 'test_failed'), + }; + } + return { + level: + result.effect === 'sync_failed' || result.effect === 'publication_failed' ? 'error' : 'info', + text: copy(locale, result.effect), + }; +} + +function copy(locale: UiLocale, key: string): string { + const en: Record = { + exists: 'That server ID already exists.', + stale_config: 'MCP configuration changed; retry the action.', + stale_edit: 'This server changed; reopen it before editing.', + stale_import: 'An imported entry changed; preview the import again.', + missing: 'That server no longer exists.', + closed: 'The MCP controller is closed.', + 'invalid-config': 'The server configuration is invalid.', + 'credential-cleanup-failed': + 'Stored credentials could not be removed; the configuration was not changed.', + 'persist-failed': 'The configuration could not be saved.', + 'manager-failed': 'The MCP connection action failed.', + turn_active: 'MCP cannot be changed while a turn or another control action is running.', + invalid: 'Check the value and try again.', + published: 'Configuration saved and tools refreshed.', + pending_host: 'Configuration saved; publication is waiting for Runtime Host.', + sync_failed: 'Configuration saved, but the MCP manager is out of sync.', + publication_failed: 'Configuration saved, but capability publication failed.', + test_ok: 'Connection test passed.', + test_failed: 'Connection test failed.', + test_publication_failed: 'Connection test passed, but capability publication failed.', + test_pending_host: 'Connection test passed; publication is waiting for Runtime Host.', + }; + const zh: Record = { + exists: '该服务器 ID 已存在。', + stale_config: 'MCP 配置已变化，请重试。', + stale_edit: '该服务器已变化，请重新打开后编辑。', + stale_import: '导入项已变化，请重新预览。', + missing: '该服务器已不存在。', + closed: 'MCP 控制器已关闭。', + 'invalid-config': '服务器配置无效。', + 'credential-cleanup-failed': '无法删除旧凭据，配置未修改。', + 'persist-failed': '无法保存配置。', + 'manager-failed': 'MCP 连接操作失败。', + turn_active: 'Turn 或其他控制操作运行期间不能修改 MCP。', + invalid: '请检查输入后重试。', + published: '配置已保存，工具已刷新。', + pending_host: '配置已保存，正等待 Runtime Host 发布。', + sync_failed: '配置已保存，但 MCP Manager 尚未同步。', + publication_failed: '配置已保存，但 capability 发布失败。', + test_ok: '连接测试通过。', + test_failed: '连接测试失败。', + test_publication_failed: '连接测试通过，但 capability 发布失败。', + test_pending_host: '连接测试通过，正等待 Runtime Host 发布。', + }; + return (locale === 'zh' ? zh : en)[key] ?? key; +} + +function confirmAddDocument(draft: GuidedDraft, locale: UiLocale): string[] { + return [ + heading(locale, 'Add this MCP server?', '添加该 MCP 服务器？'), + '', + `${draft.serverId} · ${draft.transport} · ${draft.protocol}`, + draft.transport === 'stdio' ? (draft.command ?? '') : (draft.url ?? ''), + '', + confirmCopy(locale), + ]; +} + +function confirmImportDocument(preview: TuiMcpImportPreview, locale: UiLocale): string[] { + return [ + heading(locale, 'Import MCP servers?', '导入 MCP 服务器？'), + '', + ...preview.entries.map( + (entry) => + `${entry.change === 'add' ? '+' : '~'} ${entry.serverId} · ${entry.transport} · ${entry.protocol}`, + ), + '', + confirmCopy(locale), + ]; +} + +function actionLabel(action: TuiMcpAction, locale: UiLocale): string { + if (locale === 'zh') return '正在应用 MCP 更改…'; + if (action.kind === 'test') return 'Testing MCP server…'; + if (action.kind === 'reconnect') return 'Reconnecting MCP server…'; + return 'Applying MCP configuration…'; +} + +function inputLabel(kind: InputKind, locale: UiLocale): string { + const labels: Record = { + server_id: ['Server ID', '服务器 ID'], + command: ['Command', '命令'], + args: ['Arguments', '参数'], + url: ['Streamable HTTP URL', 'Streamable HTTP URL'], + cwd: ['Working directory', '工作目录'], + env: ['Environment', '环境变量'], + headers: ['Request headers', '请求头'], + edit: ['Edit server JSON', '编辑服务器 JSON'], + import: ['Paste MCP JSON', '粘贴 MCP JSON'], + }; + return labels[kind][locale === 'zh' ? 1 : 0]; +} + +function inputHint(kind: InputKind, locale: UiLocale): string { + const optional = locale === 'zh' ? '可留空' : 'optional'; + if (kind === 'args') return `JSON string array, ${optional}`; + if (kind === 'env' || kind === 'headers') return `JSON string map, ${optional}`; + if (kind === 'cwd') return optional; + return locale === 'zh' ? 'Enter 提交 · Esc 返回' : 'Enter submit · Esc back'; +} + +function configurationLine(state: 'synchronizing' | 'out_of_sync', locale: UiLocale): string { + if (state === 'synchronizing') { + return locale === 'zh' ? '配置同步中…' : 'Configuration is synchronizing…'; + } + return ansi.red( + locale === 'zh' + ? '持久化配置与 MCP Manager 尚未同步。' + : 'Durable configuration and MCP Manager are out of sync.', + ); +} + +function unavailableDocument(locale: UiLocale): string[] { + return locale === 'zh' + ? [ + ansi.yellow('当前 TUI 未连接本地 MCP 控制面。'), + '远程 Runtime Host 的客户端 MCP 工具关联将在后续版本提供。', + ] + : [ + ansi.yellow('This TUI is not connected to a local MCP control plane.'), + 'Client MCP tool association for remote Runtime Hosts is planned for a later release.', + ]; +} + +function heading(locale: UiLocale, en: string, zh: string): string { + return ansi.bold(locale === 'zh' ? zh : en); +} + +function confirmCopy(locale: UiLocale): string { + return locale === 'zh' ? 'y 确认 · Esc 取消' : 'y Confirm · Esc cancel'; +} + +function loadingCopy(locale: UiLocale): string { + return locale === 'zh' + ? '正在读取 mcp.json 并发现工具…' + : 'Loading mcp.json and discovering tools…'; +} + +function loadErrorCopy(locale: UiLocale): string { + return locale === 'zh' + ? '无法读取或应用 MCP 配置；没有向 Runtime Host 发布工具。' + : 'MCP configuration could not be loaded; no tools were published to the Runtime Host.'; +} + +function emptyCopy(locale: UiLocale): string { + return locale === 'zh' + ? '尚未配置 MCP 服务器。按 a 添加。' + : 'No MCP servers are configured. Press a to add one.'; +} + function statusMarker(state: TuiMcpServerSnapshot['state']): string { if (state === 'connected') return ansi.green('●'); if (state === 'connecting') return ansi.yellow('●'); @@ -189,6 +769,7 @@ function statusMarker(state: TuiMcpServerSnapshot['state']): string { } function stateLabel(state: TuiMcpServerSnapshot['state'], locale: UiLocale): string { + if (!state) return locale === 'zh' ? '仅已配置' : 'configured only'; if (locale === 'en') return state; return { disabled: '已停用', diff --git a/packages/cli/src/pi-tui-runner.ts b/packages/cli/src/pi-tui-runner.ts index fa2cb30812..ff9974a792 100644 --- a/packages/cli/src/pi-tui-runner.ts +++ b/packages/cli/src/pi-tui-runner.ts @@ -122,8 +122,8 @@ import { runMakaPiTuiTurn, type MakaPiTuiTurnRequest } from './pi-tui-turn.js'; import { editorTheme, selectListTheme } from './tui-ansi.js'; import { MakaAutocompleteAboveEditorComponent } from './tui-autocomplete-layout.js'; import { TranscriptViewerOverlay } from './pi-tui-transcript-viewer.js'; -import { McpStatusOverlay } from './pi-tui-mcp-status.js'; -import type { TuiMcpSurface } from './tui-mcp-control.js'; +import { McpManagementOverlay } from './pi-tui-mcp-status.js'; +import type { TuiMcpManagement } from './tui-mcp-control.js'; import { createShellRunElapsedTicker } from './shell-run-elapsed-ticker.js'; import { createShellRunHydrationController } from './shell-run-hydration.js'; import { sessionStatusBadge } from './tui-session-status.js'; @@ -230,8 +230,8 @@ export interface MakaPiTuiInput { * whose listProviders/verify/save calls persist the connection + curated models * via the host-owned stores. */ onboarding?: MakaOnboardingSurface; - /** Client-owned MCP status and publication projection. Local TUI only in PR1. */ - mcp?: TuiMcpSurface; + /** Client-owned MCP management and publication surface. Local TUI only. */ + mcp?: TuiMcpManagement; /** First-run mode: auto-open the onboarding wizard on launch instead of * waiting for /setup (used when the CLI starts with no configured connection). */ firstRun?: boolean; @@ -2652,9 +2652,11 @@ export async function runMakaPiTui(input: MakaPiTuiInput): Promise { const showMcpStatus = (): void => { let overlay: OverlayHandle | undefined; - const viewer = new McpStatusOverlay({ + const viewer = new McpManagementOverlay({ locale, + tui, ...(input.mcp ? { surface: input.mcp } : {}), + canManage: () => !busy, viewportRows: () => terminal.rows, onClose: () => overlay?.hide(), onChange: () => tui.requestRender(), diff --git a/packages/cli/src/runtime-host-tui-context.ts b/packages/cli/src/runtime-host-tui-context.ts index b2e22909e2..51f4021eee 100644 --- a/packages/cli/src/runtime-host-tui-context.ts +++ b/packages/cli/src/runtime-host-tui-context.ts @@ -63,7 +63,7 @@ import { import { createTuiMcpController, type TuiMcpController, - type TuiMcpSurface, + type TuiMcpManagement, } from './tui-mcp-control.js'; export interface RuntimeHostTuiContext { @@ -91,7 +91,7 @@ export interface RuntimeHostTuiContext { }; readonly recap: SessionRecapGenerator; readonly onboarding: ReturnType; - readonly mcp?: TuiMcpSurface; + readonly mcp?: TuiMcpManagement; readonly profile: RuntimeHostProfile; close(): Promise; } diff --git a/packages/cli/src/tui-mcp-control.ts b/packages/cli/src/tui-mcp-control.ts index 04506d92c9..33e7192055 100644 --- a/packages/cli/src/tui-mcp-control.ts +++ b/packages/cli/src/tui-mcp-control.ts @@ -33,6 +33,7 @@ import { createCredentialMcpOAuthStorage, McpClientManager } from '@maka/mcp'; import { createFileCredentialStore } from '@maka/storage/credential-store'; import { createMcpConfigStore, + assertMcpEndpointPolicyOnChanges, McpConfigSourceError, normalizeMcpConfig, normalizeMcpImport, @@ -59,7 +60,10 @@ export interface TuiMcpServerSnapshot { readonly serverId: string; readonly configured: boolean; readonly synchronized: boolean; - readonly state: McpServerStatus['state']; + readonly enabled?: boolean; + readonly configuredTransport?: 'stdio' | 'remote'; + readonly configuredProtocol?: McpProtocolPreference; + readonly state?: McpServerStatus['state']; readonly transport?: McpServerStatus['transport']; readonly negotiatedProtocol?: McpServerStatus['negotiatedProtocol']; readonly toolCount: number; @@ -143,6 +147,7 @@ export type TuiMcpActionResult = export interface TuiMcpManagement extends TuiMcpSurface { configForEdit(serverId: string): TuiMcpEditConfig | undefined; previewImport(source: string): TuiMcpImportPreviewResult; + discardImportPreview(previewId: string): void; execute(action: TuiMcpAction): Promise; } @@ -308,6 +313,10 @@ class TuiMcpControllerImpl implements TuiMcpController { }; } + discardImportPreview(previewId: string): void { + if (this.#preparedImport?.previewId === previewId) this.#preparedImport = undefined; + } + execute(action: TuiMcpAction): Promise { if (this.#closed) return Promise.resolve({ status: 'failed', reason: 'closed' }); return this.#serializeAction(() => this.#executeAction(action)); @@ -376,7 +385,9 @@ class TuiMcpControllerImpl implements TuiMcpController { return { status: 'failed', reason: 'manager-failed' }; } } - return this.#commitMutation(action); + const result = await this.#commitMutation(action); + if (action.kind === 'commit_import') this.discardImportPreview(action.previewId); + return result; } async #commitMutation( @@ -392,6 +403,11 @@ class TuiMcpControllerImpl implements TuiMcpController { const prepared = this.#prepareMutation(current, action); if ('status' in prepared) return prepared; const { next } = prepared; + try { + assertMcpEndpointPolicyOnChanges(current, next); + } catch { + return { status: 'failed', reason: 'invalid-config' }; + } const retirements = Object.entries(current.mcpServers) .filter(([serverId, previous]) => mcpConfigChangeRetiresCredentials(previous, next.mcpServers[serverId]), @@ -399,7 +415,7 @@ class TuiMcpControllerImpl implements TuiMcpController { .map(([serverId]) => serverId); try { for (const serverId of retirements) { - await this.#deps.manager.forgetServerCredentials(serverId); + await this.#deps.manager.forgetServerCredentials(serverId, current.mcpServers[serverId]); if (this.#closed) return { status: 'failed', reason: 'closed' }; } } catch { @@ -531,7 +547,7 @@ class TuiMcpControllerImpl implements TuiMcpController { .map((serverId) => projectServerStatus( serverId, - this.#config?.mcpServers[serverId] !== undefined, + this.#config?.mcpServers[serverId], statusById.get(serverId), configuration === 'ready', ), @@ -630,15 +646,22 @@ class TuiMcpControllerImpl implements TuiMcpController { function projectServerStatus( serverId: string, - configured: boolean, + config: McpServerConfig | undefined, status: McpServerStatus | undefined, configurationSynchronized: boolean, ): TuiMcpServerSnapshot { return { serverId, - configured, - synchronized: configurationSynchronized && configured && status !== undefined, - state: status?.state ?? 'disconnected', + configured: config !== undefined, + synchronized: configurationSynchronized && config !== undefined && status !== undefined, + ...(config + ? { + enabled: config.enabled !== false, + configuredTransport: 'command' in config ? ('stdio' as const) : ('remote' as const), + configuredProtocol: resolveMcpProtocolPreference(config), + } + : {}), + ...(status ? { state: status.state } : {}), ...(status?.transport ? { transport: status.transport } : {}), ...(status?.negotiatedProtocol ? { negotiatedProtocol: status.negotiatedProtocol } : {}), toolCount: status?.toolCount ?? 0, diff --git a/packages/cli/src/tui-primary-guidance.ts b/packages/cli/src/tui-primary-guidance.ts index 6c0ddd634c..552c41df0f 100644 --- a/packages/cli/src/tui-primary-guidance.ts +++ b/packages/cli/src/tui-primary-guidance.ts @@ -56,7 +56,7 @@ const TUI_PRIMARY_GUIDANCE = { goal: '查看自主目标状态', graph: '查看、启用、停用 Graph 模式，或执行一次 Graph 任务', help: '查看命令和快捷键', - mcp: '查看客户端 MCP 服务器和工具发布状态', + mcp: '管理客户端 MCP 服务器和工具发布状态', model: '选择模型', move: '将当前会话移到其他目录', new: '新建会话', @@ -106,7 +106,7 @@ const TUI_PRIMARY_GUIDANCE = { goal: 'Show autonomous goal status', graph: 'Show, enable, disable, or run one Graph turn', help: 'Show commands and keybindings', - mcp: 'Show client-owned MCP servers and tool publication status', + mcp: 'Manage client-owned MCP servers and tool publication status', model: 'Select model', move: 'Move current session to another directory', new: 'Start a new session', diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index f2cea4064d..38d809ff18 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -1597,8 +1597,11 @@ export class McpClientManager { * its config is removed, so a delete failure aborts the removal while * everything is still recoverable — instead of leaving an orphaned token * a same-id re-add would inherit. */ - async forgetServerCredentials(serverId: string): Promise { - await this.forgetAuthorization(serverId, this.connections.get(serverId)?.config); + async forgetServerCredentials( + serverId: string, + previousConfig = this.connections.get(serverId)?.config, + ): Promise { + await this.forgetAuthorization(serverId, previousConfig); } /** Drops any stored OAuth record for a server that is being removed or diff --git a/packages/storage/src/mcp-config-store.ts b/packages/storage/src/mcp-config-store.ts index 4003c8c4b4..5204988ff8 100644 --- a/packages/storage/src/mcp-config-store.ts +++ b/packages/storage/src/mcp-config-store.ts @@ -136,7 +136,7 @@ class FileMcpConfigStore implements McpConfigStore { return this.serial(async () => { const current = await this.readOrCreate(); const next = normalizeMcpConfig(apply(current)); - enforceEndpointPolicyOnChanges(current, next); + assertMcpEndpointPolicyOnChanges(current, next); await this.write(next); return next; }); @@ -150,7 +150,7 @@ class FileMcpConfigStore implements McpConfigStore { version: MCP_CONFIG_VERSION, mcpServers: { ...current.mcpServers, [serverId]: config }, }); - enforceEndpointPolicyOnChanges(current, next); + assertMcpEndpointPolicyOnChanges(current, next); await this.write(next); return next; }); @@ -234,7 +234,7 @@ export function assertMcpEndpointPolicy(server: McpServerConfig, serverId: strin } } -function enforceEndpointPolicyOnChanges( +export function assertMcpEndpointPolicyOnChanges( previous: McpConfigFile | undefined, next: McpConfigFile, ): void { From 613a40f8d02334216d3677f8307573c436949c4d Mon Sep 17 00:00:00 2001 From: me2seeks Date: Fri, 28 Aug 2026 15:23:35 +0800 Subject: [PATCH 4/6] fix(mcp): serialize config mutations across processes Generated-by: Codex --- .../src/main/__tests__/mcp-ipc-main.test.ts | 66 ++++---- apps/desktop/src/main/mcp-ipc-main.ts | 69 ++++---- .../cli/src/__tests__/tui-mcp-control.test.ts | 147 ++++++++++++++++-- packages/cli/src/tui-mcp-control.ts | 86 ++++------ .../src/__tests__/mcp-config-store.test.ts | 29 +++- packages/storage/src/mcp-config-store.ts | 83 +++++----- 6 files changed, 298 insertions(+), 182 deletions(-) diff --git a/apps/desktop/src/main/__tests__/mcp-ipc-main.test.ts b/apps/desktop/src/main/__tests__/mcp-ipc-main.test.ts index f199aacf13..996cf0614a 100644 --- a/apps/desktop/src/main/__tests__/mcp-ipc-main.test.ts +++ b/apps/desktop/src/main/__tests__/mcp-ipc-main.test.ts @@ -40,7 +40,7 @@ test('MCP IPC commits config before publishing capabilities and emitting status' get: async () => config, transform: async (apply) => { calls.push('store'); - config = apply(config); + config = await apply(config); return config; }, upsert: async (serverId, server) => { @@ -111,12 +111,12 @@ test('MCP IPC commits config before publishing capabilities and emitting status' assert.equal(added.status, 'added'); assert.deepEqual(added.config.mcpServers.brave, { command: 'npx' }); assert.deepEqual(calls, ['store', 'sync', 'emit', 'publish']); - // A taken id comes back as data, not an IPC error, and commits nothing — - // the existence check now fails before the transaction ever reaches the - // credential-erase or write steps. + // A taken id comes back as data, not an IPC error. The check runs against + // the locked transaction snapshot, but reaches neither credential cleanup + // nor the file replacement. calls.length = 0; assert.deepEqual(await add({}, 'brave', { command: 'other' }), { status: 'exists' }); - assert.deepEqual(calls, []); + assert.deepEqual(calls, ['store']); calls.length = 0; const testHandler = handlers.get('mcp:test'); @@ -130,7 +130,7 @@ test('MCP IPC commits config before publishing capabilities and emitting status' assert.ok(cancelInstall); const cancelled = await cancelInstall({}, 'fixture'); assert.equal(cancelled.mcpServers.fixture, undefined); - assert.deepEqual(calls, ['cancel', 'forget', 'store', 'sync', 'emit', 'publish']); + assert.deepEqual(calls, ['cancel', 'store', 'forget', 'sync', 'emit', 'publish']); }); test('MCP remove aborts before touching the config when credential deletion fails', async () => { @@ -142,7 +142,7 @@ test('MCP remove aborts before touching the config when credential deletion fail store: { get: async () => config, transform: async (apply) => { - config = apply(config); + config = await apply(config); return config; }, upsert: async (serverId, server) => { @@ -209,7 +209,7 @@ test('MCP IPC redacts clientSecret toward the renderer and restores the sentinel store: { get: async () => config, transform: async (apply) => { - config = apply(config); + config = await apply(config); return config; }, upsert: async (serverId, server) => { @@ -315,11 +315,13 @@ test('MCP market cancellation waits for an in-flight config write before rolling store: { get: async () => config, transform: async (apply) => { - calls.push('write:start'); + calls.push('transaction:start'); markWriteStarted(); await writeGate; - config = apply(config); - calls.push('write:end'); + const next = await apply(config); + calls.push('write'); + config = next; + calls.push('transaction:end'); return config; }, upsert: async (serverId, server) => { @@ -372,8 +374,8 @@ test('MCP market cancellation waits for an in-flight config write before rolling // The cancellation's own removal is a full transaction on the same lane: // credentials retire first, then the conditional write. assert.deepEqual(calls, [ - 'write:start', 'cancel', 'write:end', - 'forget', 'write:start', 'write:end', + 'transaction:start', 'cancel', 'write', 'transaction:end', + 'transaction:start', 'forget', 'write', 'transaction:end', 'sync', 'emit', 'publish', ]); }); @@ -394,7 +396,7 @@ test('an active login on a secret-bearing server does not veto edits to another ipcMain: { handle(channel, handler) { handlers.set(channel, handler as (...args: any[]) => Promise); } }, store: { get: async () => config, - transform: async (apply) => { config = apply(config); return config; }, + transform: async (apply) => { config = await apply(config); return config; }, upsert: async (_serverId, _server) => config, remove: async () => config, }, @@ -452,7 +454,13 @@ test('a URL change retires the old endpoint credentials before the write, and an ipcMain: { handle(channel, handler) { handlers.set(channel, handler as (...args: any[]) => Promise); } }, store: { get: async () => config, - transform: async (apply) => { calls.push('write'); config = apply(config); return config; }, + transform: async (apply) => { + calls.push('transaction:start'); + const next = await apply(config); + calls.push('write'); + config = next; + return config; + }, upsert: async (_serverId, _server) => config, remove: async () => config, }, @@ -487,7 +495,7 @@ test('a URL change retires the old endpoint credentials before the write, and an upsert({}, 'remote', { url: 'https://new.example.com/mcp' }), /credential store unavailable/u, ); - assert.deepEqual(calls, ['forget']); + assert.deepEqual(calls, ['transaction:start', 'forget']); const kept = config.mcpServers.remote; assert.ok(kept && 'url' in kept); assert.equal(kept.url, 'https://old.example.com/mcp'); @@ -499,17 +507,17 @@ test('a URL change retires the old endpoint credentials before the write, and an upsert({}, 'remote', { url: 'http://public.example.com/mcp' }), /must use https/u, ); - assert.deepEqual(calls, []); + assert.deepEqual(calls, ['transaction:start']); // Same repoint with a healthy credential store: erase strictly precedes // the write. An unchanged-URL upsert afterwards does not erase at all. eraseFails = false; calls.length = 0; await upsert({}, 'remote', { url: 'https://new.example.com/mcp' }); - assert.deepEqual(calls, ['forget', 'write', 'sync']); + assert.deepEqual(calls, ['transaction:start', 'forget', 'write', 'sync']); calls.length = 0; await upsert({}, 'remote', { url: 'https://new.example.com/mcp', enabled: false }); - assert.deepEqual(calls, ['write', 'sync']); + assert.deepEqual(calls, ['transaction:start', 'write', 'sync']); }); test('cancelling an install rolls back only its own write, never a newer same-id config', async () => { @@ -522,7 +530,7 @@ test('cancelling an install rolls back only its own write, never a newer same-id ipcMain: { handle(channel, handler) { handlers.set(channel, handler as (...args: any[]) => Promise); } }, store: { get: async () => config, - transform: async (apply) => { config = apply(config); return config; }, + transform: async (apply) => { config = await apply(config); return config; }, upsert: async (_serverId, _server) => config, remove: async () => config, }, @@ -595,7 +603,7 @@ test('a login claim travels the shared lane and cannot land inside an open trans transform: async (apply) => { markWriteStarted(); await writeGate; - config = apply(config); + config = await apply(config); return config; }, upsert: async (_serverId, _server) => config, @@ -707,7 +715,7 @@ test('cancelling an install through the REAL store rolls the entry back despite } }); -test('the config write fails closed when the snapshot drifts under the transaction', async () => { +test('the config commit applies its mutation to the transaction snapshot', async () => { const handlers = new Map Promise>(); const config: McpConfigFile = { version: MCP_CONFIG_VERSION, mcpServers: {} }; const drifted: McpConfigFile = { @@ -719,10 +727,10 @@ test('the config write fails closed when the snapshot drifts under the transacti ipcMain: { handle(channel, handler) { handlers.set(channel, handler as (...args: any[]) => Promise); } }, store: { get: async () => config, - // Simulates an out-of-band writer landing between the snapshot read - // and the serialized write: apply() observes a different config. + // The store supplies the current snapshot after acquiring its shared + // lock. The mutation must preserve an unrelated edit already in it. transform: async (apply) => { - const next = apply(drifted); + const next = await apply(drifted); wrote = true; return next; }, @@ -751,8 +759,10 @@ test('the config write fails closed when the snapshot drifts under the transacti const upsert = handlers.get('mcp:upsert'); assert.ok(upsert); - await assert.rejects(upsert({}, 'fixture', { command: 'node' }), /changed while/u); - assert.equal(wrote, false); + const next = await upsert({}, 'fixture', { command: 'node' }); + assert.equal(wrote, true); + assert.ok(next.mcpServers.intruder); + assert.ok(next.mcpServers.fixture); }); test('MCP config commit is not rolled back by a capability publication failure', async () => { @@ -768,7 +778,7 @@ test('MCP config commit is not rolled back by a capability publication failure', store: { get: async () => config, transform: async (apply) => { - config = apply(config); + config = await apply(config); return config; }, upsert: async (serverId, server) => { diff --git a/apps/desktop/src/main/mcp-ipc-main.ts b/apps/desktop/src/main/mcp-ipc-main.ts index 2035f1c871..0cb59d44fe 100644 --- a/apps/desktop/src/main/mcp-ipc-main.ts +++ b/apps/desktop/src/main/mcp-ipc-main.ts @@ -94,54 +94,45 @@ export function registerMcpIpcMain(deps: McpIpcMainDeps): void { ); } }; - // Every config mutation is one transaction on one lane: + // Every config mutation is one transaction on one lane and one shared file + // lock: // read the authoritative snapshot → restore sentinels and apply the // active-login gate against it → erase the credentials this commit // orphans (removed servers, repointed endpoints) → persist. - // The credential erasure is asynchronous, so it cannot live inside the - // store's synchronous transform; the lane serializes the whole sequence - // instead, and the final transform still fails closed if the snapshot - // drifted under an out-of-band writer. Credentials go first so a failed - // erase aborts the commit while everything is still configured and - // retryable — never a persisted removal whose token a same-id re-add - // could inherit after a restart. + // The lane also excludes Desktop OAuth claims; the store transaction makes + // the same sequence linearizable against TUI and other process writers. + // Credentials go first so a failed erase aborts the commit while everything + // is still configured and retryable — never a persisted removal whose token + // a same-id re-add could inherit after a restart. const inMutationLane = deps.exclusiveLane ?? createMcpExclusiveLane(); const commitConfig = async ( mutate: (current: McpConfigFile) => McpConfigFile, - ): Promise => { - const current = await deps.store.get(); - const next = mutate(current); - assertMcpEndpointPolicyOnChanges(current, next); - // The authoritative gate: every server this commit semantically touches - // is re-checked INSIDE the lane. The handler-entry checks are advisory - // fast-fails; this one cannot race a login claim, because claims travel - // the same lane. - for (const serverId of new Set([ - ...Object.keys(current.mcpServers), - ...Object.keys(next.mcpServers), - ])) { - const before = current.mcpServers[serverId]; - const after = next.mcpServers[serverId]; - if (JSON.stringify(before) !== JSON.stringify(after)) assertNoActiveLogin(serverId); - } - // Erases are per-server and not transactional as a set: if one fails - // partway, the commit aborts with the EARLIER servers already logged - // out. That partial effect is deliberately in the fail-closed direction - // — a re-login is recoverable, a credential outliving its removed or - // repointed config is not. - for (const serverId of credentialRetirements(current, next)) { - await deps.manager.forgetServerCredentials(serverId); - } - const snapshot = JSON.stringify(current); - return deps.store.transform((actual) => { - if (JSON.stringify(actual) !== snapshot) { - throw new Error( - 'MCP configuration changed while this update was being prepared — retry the operation', - ); + ): Promise => + deps.store.transform(async (current) => { + const next = mutate(current); + assertMcpEndpointPolicyOnChanges(current, next); + // The authoritative gate: every server this commit semantically touches + // is re-checked INSIDE the lane. The handler-entry checks are advisory + // fast-fails; this one cannot race a login claim, because claims travel + // the same lane. + for (const serverId of new Set([ + ...Object.keys(current.mcpServers), + ...Object.keys(next.mcpServers), + ])) { + const before = current.mcpServers[serverId]; + const after = next.mcpServers[serverId]; + if (JSON.stringify(before) !== JSON.stringify(after)) assertNoActiveLogin(serverId); + } + // Erases are per-server and not transactional as a set: if one fails + // partway, the commit aborts with the EARLIER servers already logged + // out. That partial effect is deliberately in the fail-closed direction + // — a re-login is recoverable, a credential outliving its removed or + // repointed config is not. + for (const serverId of credentialRetirements(current, next)) { + await deps.manager.forgetServerCredentials(serverId); } return next; }); - }; // The renderer is semi-trusted (SECURITY.md §3): every config that crosses // toward it leaves with clientSecret replaced by the sentinel, and every // config it sends back has sentinels restored from disk before the store diff --git a/packages/cli/src/__tests__/tui-mcp-control.test.ts b/packages/cli/src/__tests__/tui-mcp-control.test.ts index 9a3db41412..d8836dcf90 100644 --- a/packages/cli/src/__tests__/tui-mcp-control.test.ts +++ b/packages/cli/src/__tests__/tui-mcp-control.test.ts @@ -18,6 +18,9 @@ */ import assert from 'node:assert/strict'; +import { mkdtemp, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; import test from 'node:test'; import type { McpConfigFile, McpServerStatus, McpToolSnapshot } from '@maka/core/mcp'; import type { McpClientManager } from '@maka/mcp'; @@ -25,6 +28,7 @@ import type { ClientCapabilityProvider, RuntimeHostConnectionAvailability, } from '@maka/runtime-host/client'; +import { createMcpConfigStore } from '@maka/storage/mcp-config-store'; import { createTuiMcpController } from '../tui-mcp-control.js'; test('TUI MCP startup stays backgrounded and publishes the discovered snapshot', async () => { @@ -205,7 +209,7 @@ test('TUI MCP add commits before manager synchronization and reports Host conver }); assert.deepEqual(result, { status: 'applied', effect: 'published' }); - assert.deepEqual(order, ['get', 'transform', 'sync']); + assert.deepEqual(order, ['transform', 'sync']); assert.deepEqual((await store.store.get()).mcpServers.docs, { enabled: true, url: 'https://docs.example/mcp', @@ -244,7 +248,7 @@ test('TUI MCP retires endpoint credentials before persistence and aborts on clea }); assert.deepEqual(result, { status: 'failed', reason: 'credential-cleanup-failed' }); - assert.deepEqual(order, ['get', 'forget:docs']); + assert.deepEqual(order, ['transform', 'forget:docs']); const stored = (await store.store.get()).mcpServers.docs; assert.ok(stored && 'url' in stored); assert.equal(stored.url, 'https://old.example/mcp'); @@ -277,7 +281,7 @@ test('TUI MCP rejects an invalid endpoint before retiring the previous credentia }), { status: 'failed', reason: 'invalid-config' }, ); - assert.deepEqual(order, ['get']); + assert.deepEqual(order, ['transform']); await controller.close(); }); @@ -307,7 +311,7 @@ test('TUI MCP edit rejects a stale revision without touching credentials or disk }); assert.deepEqual(result, { status: 'conflict', reason: 'stale_edit' }); - assert.deepEqual(order, ['get']); + assert.deepEqual(order, ['transform']); await controller.close(); }); @@ -416,17 +420,23 @@ test('TUI MCP reports a committed action as pending while the Host is unavailabl }); test('TUI MCP close fences an admitted mutation before persistence', async () => { - const actionRead = deferredValue(); + const transactionAdmission = deferred(); let reads = 0; let transforms = 0; + let writes = 0; const store = { get: async () => { reads += 1; - return reads === 1 ? emptyConfig() : actionRead.promise; + return emptyConfig(); }, - transform: async (apply: (current: McpConfigFile) => McpConfigFile) => { + transform: async ( + apply: (current: McpConfigFile) => McpConfigFile | Promise, + ) => { transforms += 1; - return apply(emptyConfig()); + await transactionAdmission.promise; + const next = await apply(emptyConfig()); + writes += 1; + return next; }, }; const manager = managementManager([]); @@ -441,13 +451,14 @@ test('TUI MCP close fences an admitted mutation before persistence', async () => serverId: 'late', config: { command: 'server' }, }); - await waitFor(() => reads === 2); + await waitFor(() => transforms === 1); const closing = controller.close(); - actionRead.resolve(emptyConfig()); + transactionAdmission.resolve(); assert.deepEqual(await executing, { status: 'failed', reason: 'closed' }); await closing; - assert.equal(transforms, 0); + assert.equal(reads, 1); + assert.equal(writes, 0); }); test('TUI MCP waits for manager synchronization before publishing an action snapshot', async () => { @@ -507,8 +518,10 @@ test('TUI MCP rebases an action over an unrelated concurrent config edit', async }; const store = { get: async () => structuredClone(config), - transform: async (apply: (current: McpConfigFile) => McpConfigFile) => { - config = apply({ + transform: async ( + apply: (current: McpConfigFile) => McpConfigFile | Promise, + ) => { + config = await apply({ version: 3, mcpServers: { existing: { command: 'concurrent' } }, }); @@ -534,6 +547,102 @@ test('TUI MCP rebases an action over an unrelated concurrent config edit', async await controller.close(); }); +test('independent TUI controllers preserve concurrent additions in one workspace', async (t) => { + const root = await mkdtemp(join(tmpdir(), 'maka-tui-mcp-concurrent-')); + t.after(() => rm(root, { recursive: true, force: true })); + await createMcpConfigStore(root).transform((current) => current); + const left = createTuiMcpController( + { workspaceRoot: root, connection: connectionHarness().connection }, + { + configStore: createMcpConfigStore(root), + manager: managementManager([]).manager, + createProvider: () => undefined, + }, + ); + const right = createTuiMcpController( + { workspaceRoot: root, connection: connectionHarness().connection }, + { + configStore: createMcpConfigStore(root), + manager: managementManager([]).manager, + createProvider: () => undefined, + }, + ); + await waitFor( + () => left.snapshot().initialization === 'ready' && right.snapshot().initialization === 'ready', + ); + + const [leftResult, rightResult] = await Promise.all([ + left.execute({ kind: 'add', serverId: 'left', config: { command: 'left-server' } }), + right.execute({ kind: 'add', serverId: 'right', config: { command: 'right-server' } }), + ]); + + assert.equal(leftResult.status, 'applied'); + assert.equal(rightResult.status, 'applied'); + const saved = await createMcpConfigStore(root).get(); + assert.ok(saved.mcpServers.left); + assert.ok(saved.mcpServers.right); + await Promise.all([left.close(), right.close()]); +}); + +test('same-server credential retirement stays inside the shared config transaction', async (t) => { + const root = await mkdtemp(join(tmpdir(), 'maka-tui-mcp-retirement-')); + t.after(() => rm(root, { recursive: true, force: true })); + await createMcpConfigStore(root).upsert('docs', { url: 'https://old.example/mcp' }); + const retirement = deferred(); + const leftOrder: string[] = []; + const rightOrder: string[] = []; + const left = createTuiMcpController( + { workspaceRoot: root, connection: connectionHarness().connection }, + { + configStore: createMcpConfigStore(root), + manager: managementManager(leftOrder, { credentialWait: retirement.promise }).manager, + createProvider: () => undefined, + }, + ); + const right = createTuiMcpController( + { workspaceRoot: root, connection: connectionHarness().connection }, + { + configStore: createMcpConfigStore(root), + manager: managementManager(rightOrder).manager, + createProvider: () => undefined, + }, + ); + await waitFor( + () => left.snapshot().initialization === 'ready' && right.snapshot().initialization === 'ready', + ); + const leftEdit = left.configForEdit('docs'); + const rightEdit = right.configForEdit('docs'); + assert.ok(leftEdit); + assert.ok(rightEdit); + + const first = left.execute({ + kind: 'edit', + serverId: 'docs', + expectedRevision: leftEdit.revision, + config: { url: 'https://left.example/mcp' }, + }); + await waitFor(() => leftOrder.includes('forget:docs')); + const second = right.execute({ + kind: 'edit', + serverId: 'docs', + expectedRevision: rightEdit.revision, + config: { url: 'https://right.example/mcp' }, + }); + for (let attempt = 0; attempt < 10; attempt += 1) { + await new Promise((resolve) => setImmediate(resolve)); + } + assert.equal(rightOrder.includes('forget:docs'), false); + + retirement.resolve(); + assert.deepEqual(await first, { status: 'applied', effect: 'published' }); + assert.deepEqual(await second, { status: 'conflict', reason: 'stale_edit' }); + assert.equal(rightOrder.includes('forget:docs'), false); + const saved = (await createMcpConfigStore(root).get()).mcpServers.docs; + assert.ok(saved && 'url' in saved); + assert.equal(saved.url, 'https://left.example/mcp'); + await Promise.all([left.close(), right.close()]); +}); + test('TUI MCP manages enabled state, tests, reconnects, and removes through one lane', async () => { const order: string[] = []; const store = mutableConfigStore( @@ -582,9 +691,11 @@ function mutableConfigStore(initial: McpConfigFile, order: string[]) { order.push('get'); return structuredClone(config); }, - transform: async (apply: (current: McpConfigFile) => McpConfigFile) => { + transform: async ( + apply: (current: McpConfigFile) => McpConfigFile | Promise, + ) => { order.push('transform'); - config = structuredClone(apply(structuredClone(config))); + config = structuredClone(await apply(structuredClone(config))); return structuredClone(config); }, }; @@ -598,7 +709,7 @@ function mutableConfigStore(initial: McpConfigFile, order: string[]) { function managementManager( order: string[], - options: { readonly credentialFailure?: boolean } = {}, + options: { readonly credentialFailure?: boolean; readonly credentialWait?: Promise } = {}, ) { let listener: (() => void) | undefined; let syncFailure = false; @@ -628,6 +739,7 @@ function managementManager( forgetServerCredentials: async (serverId: string) => { order.push(`forget:${serverId}`); if (options.credentialFailure) throw new Error('credential cleanup failed'); + await options.credentialWait; }, onChange: (next: () => void) => { listener = next; @@ -778,7 +890,8 @@ function emptyConfig(): McpConfigFile { function configStoreHarness(get: () => Promise) { return { get, - transform: async (apply: (current: McpConfigFile) => McpConfigFile) => apply(await get()), + transform: async (apply: (current: McpConfigFile) => McpConfigFile | Promise) => + apply(await get()), }; } diff --git a/packages/cli/src/tui-mcp-control.ts b/packages/cli/src/tui-mcp-control.ts index 33e7192055..dfaab64064 100644 --- a/packages/cli/src/tui-mcp-control.ts +++ b/packages/cli/src/tui-mcp-control.ts @@ -393,54 +393,39 @@ class TuiMcpControllerImpl implements TuiMcpController { async #commitMutation( action: Exclude, ): Promise { - let current: McpConfigFile; - try { - current = await this.#deps.configStore.get(); - } catch { - return { status: 'failed', reason: 'persist-failed' }; - } - if (this.#closed) return { status: 'failed', reason: 'closed' }; - const prepared = this.#prepareMutation(current, action); - if ('status' in prepared) return prepared; - const { next } = prepared; - try { - assertMcpEndpointPolicyOnChanges(current, next); - } catch { - return { status: 'failed', reason: 'invalid-config' }; - } - const retirements = Object.entries(current.mcpServers) - .filter(([serverId, previous]) => - mcpConfigChangeRetiresCredentials(previous, next.mcpServers[serverId]), - ) - .map(([serverId]) => serverId); - try { - for (const serverId of retirements) { - await this.#deps.manager.forgetServerCredentials(serverId, current.mcpServers[serverId]); - if (this.#closed) return { status: 'failed', reason: 'closed' }; - } - } catch { - return { status: 'failed', reason: 'credential-cleanup-failed' }; - } - const affectedServerIds = this.#affectedServerIds(action); - const basis = new Map( - affectedServerIds.map((serverId) => [serverId, configRevision(current.mcpServers[serverId])]), - ); let committed: McpConfigFile; try { - committed = await this.#deps.configStore.transform((actual) => { - for (const [serverId, revision] of basis) { - if (configRevision(actual.mcpServers[serverId]) !== revision) { - throw new TuiMcpConfigDriftError(); + committed = await this.#deps.configStore.transform(async (current) => { + if (this.#closed) { + throw new TuiMcpMutationError({ status: 'failed', reason: 'closed' }); + } + const prepared = this.#prepareMutation(current, action); + if ('status' in prepared) throw new TuiMcpMutationError(prepared); + const { next } = prepared; + try { + assertMcpEndpointPolicyOnChanges(current, next); + } catch { + throw new TuiMcpMutationError({ status: 'failed', reason: 'invalid-config' }); + } + try { + for (const [serverId, previous] of Object.entries(current.mcpServers)) { + if (!mcpConfigChangeRetiresCredentials(previous, next.mcpServers[serverId])) continue; + await this.#deps.manager.forgetServerCredentials(serverId, previous); + if (this.#closed) { + throw new TuiMcpMutationError({ status: 'failed', reason: 'closed' }); + } } + } catch (error) { + if (error instanceof TuiMcpMutationError) throw error; + throw new TuiMcpMutationError({ + status: 'failed', + reason: 'credential-cleanup-failed', + }); } - const rebased = this.#prepareMutation(actual, action); - if ('status' in rebased) throw new TuiMcpConfigDriftError(); - return rebased.next; + return next; }); } catch (error) { - if (error instanceof TuiMcpConfigDriftError) { - return { status: 'conflict', reason: mutationConflictReason(action) }; - } + if (error instanceof TuiMcpMutationError) return error.result; return { status: 'failed', reason: 'persist-failed' }; } if (this.#closed) return { status: 'failed', reason: 'closed' }; @@ -465,11 +450,6 @@ class TuiMcpControllerImpl implements TuiMcpController { return { status: 'applied', effect: await this.#settlePublication() }; } - #affectedServerIds(action: Exclude): string[] { - if (action.kind !== 'commit_import') return [action.serverId]; - return [...(this.#preparedImport?.basis.keys() ?? [])]; - } - #prepareMutation( current: McpConfigFile, action: Exclude, @@ -699,12 +679,8 @@ function configRevision(config: McpConfigFile | McpServerConfig | undefined): st return createHash('sha256').update(JSON.stringify(config)).digest('hex'); } -function mutationConflictReason( - action: Exclude, -): Extract['reason'] { - if (action.kind === 'commit_import') return 'stale_import'; - if (action.kind === 'edit') return 'stale_edit'; - return 'stale_config'; +class TuiMcpMutationError extends Error { + constructor(readonly result: Extract) { + super(result.reason); + } } - -class TuiMcpConfigDriftError extends Error {} diff --git a/packages/storage/src/__tests__/mcp-config-store.test.ts b/packages/storage/src/__tests__/mcp-config-store.test.ts index 853cf634fe..7bb12413ee 100644 --- a/packages/storage/src/__tests__/mcp-config-store.test.ts +++ b/packages/storage/src/__tests__/mcp-config-store.test.ts @@ -218,7 +218,8 @@ test('allows SSE only with an omitted or explicit legacy protocol', () => { test('transform sees the latest committed config, not a caller snapshot', async () => { // The restore-plus-mutation seam: a marker-bearing write that derived its // restores from a stale snapshot could roll a rotated secret back. Inside - // transform, apply() must observe the concurrent writer's commit. + // transform, apply() must observe the concurrent writer's commit under the + // shared file transaction. const root = await tempRoot(); const store = createMcpConfigStore(root); await store.upsert('local', { command: 'npx', env: { TOKEN: 'v1' } }); @@ -236,6 +237,32 @@ test('transform sees the latest committed config, not a caller snapshot', async assert.equal(final.env?.TOKEN, 'v2-rotated'); }); +test('two independent stores preserve concurrent additions to one workspace', async () => { + const root = await tempRoot(); + await createMcpConfigStore(root).get(); + const desktop = createMcpConfigStore(root); + const tui = createMcpConfigStore(root); + + await Promise.all([ + desktop.upsert('desktop', { command: 'desktop-server' }), + tui.upsert('tui', { command: 'tui-server' }), + ]); + + const saved = await createMcpConfigStore(root).get(); + assert.equal( + saved.mcpServers.desktop && 'command' in saved.mcpServers.desktop + ? saved.mcpServers.desktop.command + : undefined, + 'desktop-server', + ); + assert.equal( + saved.mcpServers.tui && 'command' in saved.mcpServers.tui + ? saved.mcpServers.tui.command + : undefined, + 'tui-server', + ); +}); + test('serializes concurrent updates without corrupting the file', async () => { const root = await tempRoot(); const store = createMcpConfigStore(root); diff --git a/packages/storage/src/mcp-config-store.ts b/packages/storage/src/mcp-config-store.ts index 5204988ff8..3bdcf1cd1b 100644 --- a/packages/storage/src/mcp-config-store.ts +++ b/packages/storage/src/mcp-config-store.ts @@ -32,6 +32,7 @@ import { type McpServerConfig, type McpStdioServerConfig, } from '@maka/core/mcp'; +import { withFileUpdateLock } from './file-update-lock.js'; const MAX_SERVERS = 100; const MAX_ID_LENGTH = 128; @@ -41,11 +42,13 @@ const FORBIDDEN_KEYS = new Set(['__proto__', 'prototype', 'constructor']); export interface McpConfigStore { get(): Promise; - /** One serialized read-transform-write. `apply` sees the CURRENT on-disk - * config and returns the next one, inside the store's write queue — the - * seam for restore-plus-mutation flows whose separate get()-then-write - * would race a concurrent writer and roll a rotated secret back. */ - transform(apply: (current: McpConfigFile) => McpConfigFile): Promise; + /** One cross-process read-transform-write transaction. `apply` sees the + * current on-disk config and may finish asynchronous effects that must + * precede the commit, such as retiring credentials. The shared file lock + * remains held until the replacement document is durable. */ + transform( + apply: (current: McpConfigFile) => McpConfigFile | Promise, + ): Promise; upsert(serverId: string, config: McpServerConfig): Promise; remove(serverId: string): Promise; } @@ -124,18 +127,23 @@ export function normalizeMcpImport(source: string): McpConfigFile { } class FileMcpConfigStore implements McpConfigStore { - private queue: Promise = Promise.resolve(); - constructor(private readonly path: string) {} async get(): Promise { - return this.serial(async () => this.readOrCreate()); + try { + return await this.read(); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + return this.withUpdateLock(() => this.readOrCreate()); + } } - async transform(apply: (current: McpConfigFile) => McpConfigFile): Promise { - return this.serial(async () => { + async transform( + apply: (current: McpConfigFile) => McpConfigFile | Promise, + ): Promise { + return this.withUpdateLock(async () => { const current = await this.readOrCreate(); - const next = normalizeMcpConfig(apply(current)); + const next = normalizeMcpConfig(await apply(current)); assertMcpEndpointPolicyOnChanges(current, next); await this.write(next); return next; @@ -144,35 +152,33 @@ class FileMcpConfigStore implements McpConfigStore { async upsert(serverId: string, config: McpServerConfig): Promise { assertSafeKey(serverId, 'server id'); - return this.serial(async () => { - const current = await this.readOrCreate(); - const next = normalizeMcpConfig({ + return this.transform((current) => + normalizeMcpConfig({ version: MCP_CONFIG_VERSION, mcpServers: { ...current.mcpServers, [serverId]: config }, - }); - assertMcpEndpointPolicyOnChanges(current, next); - await this.write(next); - return next; - }); + }), + ); } async remove(serverId: string): Promise { assertSafeKey(serverId, 'server id'); - return this.serial(async () => { - const current = await this.readOrCreate(); + return this.transform((current) => { const { [serverId]: _removed, ...mcpServers } = current.mcpServers; - const next: McpConfigFile = { version: MCP_CONFIG_VERSION, mcpServers }; - await this.write(next); - return next; + return { version: MCP_CONFIG_VERSION, mcpServers }; }); } + private async read(): Promise { + const text = await readFile(this.path, 'utf8'); + if (Buffer.byteLength(text, 'utf8') > MAX_CONFIG_BYTES) { + throw new Error('MCP config exceeds 1 MiB'); + } + return normalizeMcpConfig(JSON.parse(text)); + } + private async readOrCreate(): Promise { try { - const text = await readFile(this.path, 'utf8'); - if (Buffer.byteLength(text, 'utf8') > MAX_CONFIG_BYTES) - throw new Error('MCP config exceeds 1 MiB'); - return normalizeMcpConfig(JSON.parse(text)); + return await this.read(); } catch (error) { if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; const empty = createDefaultMcpConfig(); @@ -181,6 +187,13 @@ class FileMcpConfigStore implements McpConfigStore { } } + private async withUpdateLock(operation: () => Promise): Promise { + const dir = dirname(this.path); + await mkdir(dir, { recursive: true, mode: 0o700 }); + if (process.platform !== 'win32') await chmod(dir, 0o700); + return withFileUpdateLock(this.path, operation); + } + private async write(config: McpConfigFile): Promise { const dir = dirname(this.path); await mkdir(dir, { recursive: true, mode: 0o700 }); @@ -199,20 +212,6 @@ class FileMcpConfigStore implements McpConfigStore { await rm(tempPath, { force: true }).catch(() => {}); } } - - private async serial(operation: () => Promise): Promise { - const previous = this.queue; - let release!: () => void; - this.queue = new Promise((resolve) => { - release = resolve; - }); - await previous; - try { - return await operation(); - } finally { - release(); - } - } } /** Endpoint security policy, enforced at the WRITE boundary for new or From 1f9b2a12cbfa63ba1d4f7eb49acadb2f3cfd687f Mon Sep 17 00:00:00 2001 From: me2seeks Date: Fri, 28 Aug 2026 15:23:35 +0800 Subject: [PATCH 5/6] fix(tui): keep MCP selection visible Generated-by: Codex --- .../src/__tests__/pi-tui-mcp-status.test.ts | 56 ++++++++++++++++++- packages/cli/src/pi-tui-mcp-status.ts | 50 +++++++++++++---- 2 files changed, 95 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/__tests__/pi-tui-mcp-status.test.ts b/packages/cli/src/__tests__/pi-tui-mcp-status.test.ts index c8717d0f77..81770f71bb 100644 --- a/packages/cli/src/__tests__/pi-tui-mcp-status.test.ts +++ b/packages/cli/src/__tests__/pi-tui-mcp-status.test.ts @@ -20,7 +20,7 @@ import assert from 'node:assert/strict'; import { describe, test } from 'node:test'; import { McpManagementOverlay } from '../pi-tui-mcp-status.js'; -import type { TuiMcpManagement, TuiMcpSnapshot } from '../tui-mcp-control.js'; +import type { TuiMcpAction, TuiMcpManagement, TuiMcpSnapshot } from '../tui-mcp-control.js'; import { stripAnsi } from '../tui-ansi.js'; describe('MCP management overlay', () => { @@ -129,6 +129,60 @@ describe('MCP management overlay', () => { assert.equal(disposed, 1); assert.equal(closed, 1); }); + + test('keeps long-list selection visible and applies actions to the visible server', async () => { + const actions: TuiMcpAction[] = []; + const mcp = surface({ + initialization: 'ready', + configuration: 'ready', + publication: 'not_published', + toolCount: 0, + servers: Array.from({ length: 8 }, (_, index) => ({ + serverId: `s${index}`, + configured: true, + synchronized: true, + enabled: true, + configuredTransport: 'stdio' as const, + configuredProtocol: 'legacy' as const, + ...(index === 5 ? { state: 'error' as const, error: 'visible diagnostic' } : {}), + toolCount: 0, + })), + }); + mcp.execute = async (action) => { + actions.push(action); + return { status: 'applied', effect: 'published' }; + }; + const overlay = new McpManagementOverlay({ + locale: 'en', + surface: mcp, + viewportRows: () => 6, + onClose: () => undefined, + onChange: () => undefined, + }); + overlay.render(100); + + for (let index = 0; index < 5; index += 1) overlay.handleInput('\u001b[B'); + let text = overlay.render(100).map(stripAnsi).join('\n'); + assert.match(text, /› ● s5/u); + assert.match(text, /visible diagnostic/u); + + overlay.handleInput(' '); + await new Promise((resolve) => setImmediate(resolve)); + assert.deepEqual(actions, [{ kind: 'set_enabled', serverId: 's5', enabled: false }]); + + overlay.handleInput('\u001b[H'); + text = overlay.render(100).map(stripAnsi).join('\n'); + assert.match(text, /› ○ s0/u); + overlay.handleInput('\u001b[6~'); + text = overlay.render(100).map(stripAnsi).join('\n'); + assert.match(text, /› ○ s4/u); + overlay.handleInput('\u001b[F'); + text = overlay.render(100).map(stripAnsi).join('\n'); + assert.match(text, /› ○ s7/u); + overlay.handleInput('\u001b[5~'); + text = overlay.render(100).map(stripAnsi).join('\n'); + assert.match(text, /› ○ s4/u); + }); }); function surface( diff --git a/packages/cli/src/pi-tui-mcp-status.ts b/packages/cli/src/pi-tui-mcp-status.ts index f10807a555..a4f967eb7a 100644 --- a/packages/cli/src/pi-tui-mcp-status.ts +++ b/packages/cli/src/pi-tui-mcp-status.ts @@ -88,6 +88,7 @@ export class McpManagementOverlay implements Component { private documentRows = 0; private bodyRows = 0; private selected = 0; + private serverRows: { start: number; end: number }[] = []; private phase: McpOverlayPhase = { kind: 'list' }; private notice: { level: 'info' | 'error'; text: string } | undefined; private readonly dispose: () => void; @@ -165,6 +166,7 @@ export class McpManagementOverlay implements Component { this.bodyRows = Math.max(0, viewportRows - (showFooter ? CHROME_ROWS : 1)); const document = this.document(safeWidth); this.documentRows = document.length; + this.keepSelectionVisible(); this.top = clamp(this.top, 0, this.maxTop()); const visible = document.slice(this.top, this.top + this.bodyRows); const start = visible.length === 0 ? 0 : this.top + 1; @@ -194,10 +196,12 @@ export class McpManagementOverlay implements Component { this.selected = clamp(this.selected - 1, 0, servers.length - 1); } else if (matchesKey(data, Key.down)) { this.selected = clamp(this.selected + 1, 0, servers.length - 1); - } else if (matchesKey(data, Key.pageUp)) this.scrollBy(-Math.max(1, this.bodyRows)); - else if (matchesKey(data, Key.pageDown)) this.scrollBy(Math.max(1, this.bodyRows)); - else if (matchesKey(data, Key.home)) this.scrollTo(0); - else if (matchesKey(data, Key.end)) this.scrollTo(this.maxTop()); + } else if (matchesKey(data, Key.pageUp)) { + this.moveSelectionByPage(-1, servers.length); + } else if (matchesKey(data, Key.pageDown)) { + this.moveSelectionByPage(1, servers.length); + } else if (matchesKey(data, Key.home)) this.selected = 0; + else if (matchesKey(data, Key.end)) this.selected = Math.max(0, servers.length - 1); else if (matchesKey(data, 'a') && this.management()) this.phase = { kind: 'add_choice' }; else { const server = servers[this.selected]; @@ -371,6 +375,7 @@ export class McpManagementOverlay implements Component { } private document(width: number): string[] { + this.serverRows = []; const snapshot = this.input.surface?.snapshot(); if (!snapshot) return unavailableDocument(this.input.locale); if (this.phase.kind === 'input') return this.inputDocument(width); @@ -436,7 +441,10 @@ export class McpManagementOverlay implements Component { lines.push(''); this.selected = clamp(this.selected, 0, snapshot.servers.length - 1); snapshot.servers.forEach((server, index) => { - lines.push(...serverLines(server, this.input.locale, index === this.selected)); + const rows = serverLines(server, this.input.locale, index === this.selected); + const start = lines.length; + lines.push(...rows); + this.serverRows.push({ start, end: lines.length - 1 }); }); return lines; } @@ -493,13 +501,35 @@ export class McpManagementOverlay implements Component { this.input.onClose(); } - private scrollBy(delta: number): void { - this.scrollTo(this.top + delta); + private keepSelectionVisible(): void { + const rows = this.serverRows[this.selected]; + if (!rows || this.bodyRows <= 0) return; + if (rows.end - rows.start + 1 > this.bodyRows || rows.start < this.top) { + this.top = rows.start; + } else if (rows.end >= this.top + this.bodyRows) { + this.top = rows.end - this.bodyRows + 1; + } } - private scrollTo(next: number): void { - this.top = clamp(next, 0, this.maxTop()); - this.input.onChange(); + private moveSelectionByPage(direction: -1 | 1, serverCount: number): void { + if (serverCount === 0) { + this.selected = 0; + return; + } + const current = this.serverRows[this.selected]; + if (!current || this.serverRows.length !== serverCount) { + this.selected = clamp( + this.selected + direction * Math.max(1, this.bodyRows), + 0, + serverCount - 1, + ); + return; + } + const targetRow = current.start + direction * Math.max(1, this.bodyRows); + const target = this.serverRows.findIndex( + (rows) => targetRow >= rows.start && targetRow <= rows.end, + ); + this.selected = target < 0 ? (direction < 0 ? 0 : Math.max(0, serverCount - 1)) : target; } private maxTop(): number { From b7ed6fda1994644d0e0791e570a0c15e29f9297b Mon Sep 17 00:00:00 2001 From: me2seeks Date: Fri, 28 Aug 2026 16:00:57 +0800 Subject: [PATCH 6/6] fix(mcp): recover config lock after process exit Generated-by: Codex --- .../fixtures/mcp-config-lock-holder.ts | 29 ++++++++++++++++ .../src/__tests__/mcp-config-store.test.ts | 34 +++++++++++++++++++ packages/storage/src/mcp-config-store.ts | 27 +++++++++++---- 3 files changed, 83 insertions(+), 7 deletions(-) create mode 100644 packages/storage/src/__tests__/fixtures/mcp-config-lock-holder.ts diff --git a/packages/storage/src/__tests__/fixtures/mcp-config-lock-holder.ts b/packages/storage/src/__tests__/fixtures/mcp-config-lock-holder.ts new file mode 100644 index 0000000000..3c34f50bd6 --- /dev/null +++ b/packages/storage/src/__tests__/fixtures/mcp-config-lock-holder.ts @@ -0,0 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { createMcpConfigStore } from '../../mcp-config-store.js'; + +const root = process.argv[2]; +if (!root) throw new Error('Missing MCP config workspace root'); + +await createMcpConfigStore(root).transform(async (current) => { + process.send?.('locked'); + await new Promise(() => setInterval(() => undefined, 1_000)); + return current; +}); diff --git a/packages/storage/src/__tests__/mcp-config-store.test.ts b/packages/storage/src/__tests__/mcp-config-store.test.ts index 7bb12413ee..faff8d4dab 100644 --- a/packages/storage/src/__tests__/mcp-config-store.test.ts +++ b/packages/storage/src/__tests__/mcp-config-store.test.ts @@ -18,6 +18,7 @@ */ import assert from 'node:assert/strict'; +import { fork } from 'node:child_process'; import { mkdtemp, readFile, rm, stat, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -263,6 +264,39 @@ test('two independent stores preserve concurrent additions to one workspace', as ); }); +test('a new store commits after a killed MCP config writer releases its native lock', async (t) => { + const root = await tempRoot(); + const holder = fork(new URL('./fixtures/mcp-config-lock-holder.js', import.meta.url), [root], { + stdio: ['ignore', 'ignore', 'inherit', 'ipc'], + }); + t.after(() => { + if (holder.exitCode === null && holder.signalCode === null) holder.kill('SIGKILL'); + }); + await new Promise((resolve, reject) => { + holder.once('message', (message) => { + if (message === 'locked') resolve(); + else reject(new Error(`Unexpected child message: ${String(message)}`)); + }); + holder.once('error', reject); + holder.once('exit', (code, signal) => { + reject(new Error(`MCP config lock holder exited early (${String(code)}, ${signal})`)); + }); + }); + + holder.kill('SIGKILL'); + await new Promise((resolve) => holder.once('exit', () => resolve())); + + const saved = await createMcpConfigStore(root).upsert('recovered', { + command: 'recovered-server', + }); + const recovered = saved.mcpServers.recovered; + assert.ok(recovered && 'command' in recovered); + assert.equal(recovered.command, 'recovered-server'); + const reopened = (await createMcpConfigStore(root).get()).mcpServers.recovered; + assert.ok(reopened && 'command' in reopened); + assert.equal(reopened.command, 'recovered-server'); +}); + test('serializes concurrent updates without corrupting the file', async () => { const root = await tempRoot(); const store = createMcpConfigStore(root); diff --git a/packages/storage/src/mcp-config-store.ts b/packages/storage/src/mcp-config-store.ts index 3bdcf1cd1b..c362d86eda 100644 --- a/packages/storage/src/mcp-config-store.ts +++ b/packages/storage/src/mcp-config-store.ts @@ -32,7 +32,7 @@ import { type McpServerConfig, type McpStdioServerConfig, } from '@maka/core/mcp'; -import { withFileUpdateLock } from './file-update-lock.js'; +import { withProcessLifetimeFileUpdateLock } from './process-lifetime-file-update-lock.js'; const MAX_SERVERS = 100; const MAX_ID_LENGTH = 128; @@ -127,6 +127,8 @@ export function normalizeMcpImport(source: string): McpConfigFile { } class FileMcpConfigStore implements McpConfigStore { + private directoryReady: Promise | undefined; + constructor(private readonly path: string) {} async get(): Promise { @@ -188,16 +190,13 @@ class FileMcpConfigStore implements McpConfigStore { } private async withUpdateLock(operation: () => Promise): Promise { - const dir = dirname(this.path); - await mkdir(dir, { recursive: true, mode: 0o700 }); - if (process.platform !== 'win32') await chmod(dir, 0o700); - return withFileUpdateLock(this.path, operation); + await this.ensureDirectory(); + return withProcessLifetimeFileUpdateLock(this.path, operation); } private async write(config: McpConfigFile): Promise { const dir = dirname(this.path); - await mkdir(dir, { recursive: true, mode: 0o700 }); - if (process.platform !== 'win32') await chmod(dir, 0o700); + await this.ensureDirectory(); const tempPath = join(dir, `.mcp-${randomUUID()}.tmp`); try { await writeFile(tempPath, `${JSON.stringify(config, null, 2)}\n`, { @@ -212,6 +211,20 @@ class FileMcpConfigStore implements McpConfigStore { await rm(tempPath, { force: true }).catch(() => {}); } } + + private ensureDirectory(): Promise { + if (this.directoryReady) return this.directoryReady; + const dir = dirname(this.path); + const ready = (async () => { + await mkdir(dir, { recursive: true, mode: 0o700 }); + if (process.platform !== 'win32') await chmod(dir, 0o700); + })(); + this.directoryReady = ready; + void ready.catch(() => { + if (this.directoryReady === ready) this.directoryReady = undefined; + }); + return ready; + } } /** Endpoint security policy, enforced at the WRITE boundary for new or