From 8259f8d35a582ec57f8494219baf7259fb55a0ac Mon Sep 17 00:00:00 2001 From: neubig Date: Mon, 27 Jul 2026 13:05:13 +0000 Subject: [PATCH 1/4] feat: add canonical MCP settings operations Co-authored-by: openhands --- README.md | 21 ++ .../agent-server-contract-types.test.ts | 71 +++++++ src/__tests__/api-clients.test.ts | 2 +- src/__tests__/mcp-settings-client.test.ts | 191 ++++++++++++++++++ src/client/mcp-client.ts | 74 +++++-- src/client/settings-client.ts | 33 +++ src/clients.ts | 19 ++ src/index.ts | 24 ++- src/models/agent-server-api.ts | 2 + src/models/api.ts | 7 + src/models/mcp-settings.ts | 58 ++++++ 11 files changed, 482 insertions(+), 20 deletions(-) create mode 100644 src/__tests__/mcp-settings-client.test.ts create mode 100644 src/models/mcp-settings.ts diff --git a/README.md b/README.md index 177da1c..518a9d8 100644 --- a/README.md +++ b/README.md @@ -189,6 +189,27 @@ const tools = await manager.tools.listTools(); const acpCount = await manager.acp.countConversations(); ``` +### Updating MCP settings + +Persisted MCP servers use their settings-map key as stable identity. Add or +update one server with a sparse patch instead of fetching and resending the +whole catalog: + +```typescript +const settings = manager.settings; + +await settings.patchMcpServer('github', { + url: 'https://example.test/mcp', +}); + +await settings.deleteMcpServer('old-server'); +``` + +An omitted patch field preserves its stored value, including credentials. +Supplying a value replaces it, and `null` explicitly clears a supported +optional field or deletes a map entry. Each helper sends one +`PATCH /api/settings` request. + If you need the lower-level endpoint-specific clients directly, import them from the secondary entrypoint: ```typescript diff --git a/src/__tests__/agent-server-contract-types.test.ts b/src/__tests__/agent-server-contract-types.test.ts index 3996dce..3f49e6f 100644 --- a/src/__tests__/agent-server-contract-types.test.ts +++ b/src/__tests__/agent-server-contract-types.test.ts @@ -1,16 +1,41 @@ +import type { MCPClient } from '../client/mcp-client'; import type { SettingsClient } from '../client/settings-client'; import type { + AgentServerMCPOAuthCallbackRequest, + AgentServerMCPOAuthCallbackResponse, + AgentServerMCPOAuthStatusResponse, + AgentServerMCPStartOAuthRequest, + AgentServerMCPStartOAuthResponse, + AgentServerMCPTestRequest, + AgentServerMCPTestResponse, AgentServerSettingsPatchRequest, AgentServerSettingsPatchResponse, AgentServerSettingsResponse, AgentServerSettingsSchema, } from '../models/agent-server-api'; +import type { + MCPAuthCredential, + MCPConfig, + MCPConfigPatch, + MCPServer, + MCPServerPatch, +} from '../models/mcp-settings'; type Equal = (() => Value extends Left ? 1 : 2) extends () => Value extends Right ? 1 : 2 ? true : false; +type IsAny = 0 extends 1 & Value ? true : false; +type IsBareUnknown = + IsAny extends true + ? false + : unknown extends Value + ? [keyof Value] extends [never] + ? true + : false + : false; + function assertType(_value: Type): void { // Compiling this call is the assertion. } @@ -30,4 +55,50 @@ describe('Agent Server generated contract aliases', () => { Equal>, AgentServerSettingsPatchResponse> >(true); }); + + it('keeps important MCP contract exports stronger than any or bare unknown', () => { + assertType, false>>(true); + assertType, false>>(true); + assertType, false>>(true); + assertType, false>>(true); + assertType, false>>(true); + assertType, false>>(true); + assertType, false>>(true); + assertType, false>>(true); + assertType, false>>(true); + assertType, false>>(true); + }); + + it('keeps MCP client methods checked against generated operations', () => { + assertType[0], AgentServerMCPTestRequest>>(true); + assertType>, AgentServerMCPTestResponse>>( + true + ); + assertType[0], AgentServerMCPStartOAuthRequest>>( + true + ); + assertType< + Equal>, AgentServerMCPStartOAuthResponse> + >(true); + assertType< + Equal>, AgentServerMCPOAuthStatusResponse> + >(true); + assertType< + Equal[1], AgentServerMCPOAuthCallbackRequest> + >(true); + assertType< + Equal< + Awaited>, + AgentServerMCPOAuthCallbackResponse + > + >(true); + }); + + it('distinguishes omitted patch fields from explicit clearing', () => { + const omitted = {} satisfies MCPServerPatch; + const cleared = { auth: null } satisfies MCPServerPatch; + + expect('auth' in omitted).toBe(false); + expect(cleared.auth).toBeNull(); + }); }); diff --git a/src/__tests__/api-clients.test.ts b/src/__tests__/api-clients.test.ts index d3435dd..c7332c3 100644 --- a/src/__tests__/api-clients.test.ts +++ b/src/__tests__/api-clients.test.ts @@ -2484,7 +2484,7 @@ describe('Auxiliary API clients', () => { const request = { name: 'superhuman-mail', server: { - transport: 'http' as const, + type: 'http' as const, url: 'https://mcp.mail.superhuman.com/mcp', auth: { strategy: 'oauth2' as const, diff --git a/src/__tests__/mcp-settings-client.test.ts b/src/__tests__/mcp-settings-client.test.ts new file mode 100644 index 0000000..e45d822 --- /dev/null +++ b/src/__tests__/mcp-settings-client.test.ts @@ -0,0 +1,191 @@ +import { SettingsClient } from '../client/settings-client'; +import type { MCPConfig, MCPServerPatch } from '../models/mcp-settings'; + +type JsonObject = Record; + +function clone(value: Value): Value { + return structuredClone(value); +} + +function mergePatch(target: JsonObject, patch: JsonObject): JsonObject { + const result = clone(target); + for (const [key, value] of Object.entries(patch)) { + if (value === null) { + delete result[key]; + } else if ( + typeof value === 'object' && + !Array.isArray(value) && + typeof result[key] === 'object' && + result[key] !== null && + !Array.isArray(result[key]) + ) { + result[key] = mergePatch(result[key] as JsonObject, value as JsonObject); + } else { + result[key] = clone(value); + } + } + return result; +} + +function installStatefulSettingsServer(initial: MCPConfig) { + const catalog = clone(initial) as unknown as JsonObject; + const fetchMock = jest.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { + const request = JSON.parse(String(init?.body)) as { + agent_settings_diff: { mcp_config: Record }; + }; + for (const [settingsKey, patch] of Object.entries(request.agent_settings_diff.mcp_config)) { + if (patch === null) { + delete catalog[settingsKey]; + } else { + catalog[settingsKey] = mergePatch( + (catalog[settingsKey] as JsonObject | undefined) ?? {}, + patch as JsonObject + ); + } + } + return new Response( + JSON.stringify({ + agent_settings: { mcp_config: catalog }, + conversation_settings: {}, + llm_api_key_is_set: false, + }), + { status: 200, headers: { 'content-type': 'application/json' } } + ); + }); + global.fetch = fetchMock as typeof fetch; + return { + fetchMock, + getCatalog: () => clone(catalog), + }; +} + +const initialCatalog = (): MCPConfig => ({ + github: { + transport: 'http', + url: 'https://api.githubcopilot.com/mcp/', + auth: { strategy: 'bearer', value: 'github-secret' }, + headers: { 'X-Existing': 'keep-me', 'X-Remove': 'old' }, + }, + filesystem: { + transport: 'stdio', + command: 'npx', + args: ['-y', '@modelcontextprotocol/server-filesystem', '/workspace'], + env: { FILESYSTEM_TOKEN: 'filesystem-secret' }, + }, +}); + +describe('SettingsClient canonical MCP mutations', () => { + it('adds one named server while preserving siblings and credentials', async () => { + const server = installStatefulSettingsServer(initialCatalog()); + const client = new SettingsClient({ host: 'http://example.com' }); + + await client.patchMcpServer('docs', { + transport: 'http', + url: 'https://example.test/mcp', + auth: { strategy: 'bearer', value: 'docs-secret' }, + }); + + expect(server.fetchMock).toHaveBeenCalledTimes(1); + expect(server.getCatalog()).toMatchObject({ + github: { auth: { strategy: 'bearer', value: 'github-secret' } }, + filesystem: { env: { FILESYSTEM_TOKEN: 'filesystem-secret' } }, + docs: { + transport: 'http', + url: 'https://example.test/mcp', + auth: { strategy: 'bearer', value: 'docs-secret' }, + }, + }); + expect(server.fetchMock).toHaveBeenCalledWith( + 'http://example.com/api/settings', + expect.objectContaining({ + method: 'PATCH', + body: JSON.stringify({ + agent_settings_diff: { + mcp_config: { + docs: { + transport: 'http', + url: 'https://example.test/mcp', + auth: { strategy: 'bearer', value: 'docs-secret' }, + }, + }, + }, + }), + }) + ); + }); + + it('preserves stored auth when a sparse update omits it', async () => { + const server = installStatefulSettingsServer(initialCatalog()); + const client = new SettingsClient({ host: 'http://example.com' }); + + await client.patchMcpServer('github', { + url: 'https://example.test/github-mcp', + }); + + expect(server.fetchMock).toHaveBeenCalledTimes(1); + expect(server.getCatalog().github).toMatchObject({ + url: 'https://example.test/github-mcp', + auth: { strategy: 'bearer', value: 'github-secret' }, + }); + }); + + it('replaces and explicitly clears auth', async () => { + const server = installStatefulSettingsServer(initialCatalog()); + const client = new SettingsClient({ host: 'http://example.com' }); + + await client.patchMcpServer('github', { + auth: { strategy: 'api_key', header_name: 'X-API-Key', value: 'replacement' }, + }); + expect(server.getCatalog().github).toMatchObject({ + auth: { strategy: 'api_key', header_name: 'X-API-Key', value: 'replacement' }, + }); + + await client.patchMcpServer('github', { auth: null }); + expect(server.fetchMock).toHaveBeenCalledTimes(2); + expect(server.getCatalog().github).not.toHaveProperty('auth'); + }); + + it('deletes a map entry without altering sibling credentials', async () => { + const server = installStatefulSettingsServer(initialCatalog()); + const client = new SettingsClient({ host: 'http://example.com' }); + + await client.deleteMcpServer('filesystem'); + + expect(server.fetchMock).toHaveBeenCalledTimes(1); + expect(server.getCatalog()).toEqual({ + github: { + transport: 'http', + url: 'https://api.githubcopilot.com/mcp/', + auth: { strategy: 'bearer', value: 'github-secret' }, + headers: { 'X-Existing': 'keep-me', 'X-Remove': 'old' }, + }, + }); + expect(server.fetchMock).toHaveBeenCalledWith( + 'http://example.com/api/settings', + expect.objectContaining({ + method: 'PATCH', + body: JSON.stringify({ + agent_settings_diff: { mcp_config: { filesystem: null } }, + }), + }) + ); + }); + + it('uses null to delete one nested map value while preserving its siblings', async () => { + const server = installStatefulSettingsServer(initialCatalog()); + const client = new SettingsClient({ host: 'http://example.com' }); + + await client.patchMcpServer('github', { + headers: { 'X-Remove': null }, + }); + + expect(server.fetchMock).toHaveBeenCalledTimes(1); + expect(server.getCatalog().github).toMatchObject({ + headers: { 'X-Existing': 'keep-me' }, + auth: { strategy: 'bearer', value: 'github-secret' }, + }); + expect( + (server.getCatalog().github as JsonObject).headers as Record + ).not.toHaveProperty('X-Remove'); + }); +}); diff --git a/src/client/mcp-client.ts b/src/client/mcp-client.ts index c7edd6e..673edfb 100644 --- a/src/client/mcp-client.ts +++ b/src/client/mcp-client.ts @@ -4,11 +4,20 @@ import { } from './agent-server-compatibility'; import { HttpClient } from './http-client'; import type { - MCPOAuthCallbackRequest, - MCPOAuthStartResponse, - MCPOAuthStatusResponse, - MCPTestRequest, - MCPTestResponse, + AgentServerMCPOAuthCallbackRequest, + AgentServerMCPOAuthCallbackResponse, + AgentServerMCPOAuthStatusResponse, + AgentServerMCPStartOAuthRequest, + AgentServerMCPStartOAuthResponse, + AgentServerMCPTestRequest, + AgentServerMCPTestResponse, +} from '../models/agent-server-api'; +import type { + MCPOAuthCallbackRequest as LegacyMCPOAuthCallbackRequest, + MCPOAuthStartResponse as LegacyMCPOAuthStartResponse, + MCPOAuthStatusResponse as LegacyMCPOAuthStatusResponse, + MCPTestRequest as LegacyMCPTestRequest, + MCPTestResponse as LegacyMCPTestResponse, } from '../models/api'; export interface MCPClientOptions { @@ -32,17 +41,37 @@ export class MCPClient { }); } - async testServer(request: MCPTestRequest): Promise { + /** + * @deprecated Pass `AgentServerMCPTestRequest` for generated contract + * checking. This overload keeps existing callers source compatible while + * they migrate. + */ + async testServer(request: LegacyMCPTestRequest): Promise; + async testServer(request: AgentServerMCPTestRequest): Promise; + async testServer( + request: LegacyMCPTestRequest | AgentServerMCPTestRequest + ): Promise { await assertAgentServerSupports(this.client, AgentServerFeatureRequirements.mcpTest); - const response = await this.client.post('/api/mcp/test', request, { + const response = await this.client.post('/api/mcp/test', request, { timeout: (request.timeout ?? 15) * 1000 + 5000, }); return response.data; } - async startOAuth(request: MCPTestRequest): Promise { + /** + * @deprecated Pass `AgentServerMCPStartOAuthRequest` for generated contract + * checking. This overload keeps existing callers source compatible while + * they migrate. + */ + async startOAuth(request: LegacyMCPTestRequest): Promise; + async startOAuth( + request: AgentServerMCPStartOAuthRequest + ): Promise; + async startOAuth( + request: LegacyMCPTestRequest | AgentServerMCPStartOAuthRequest + ): Promise { await assertAgentServerSupports(this.client, AgentServerFeatureRequirements.mcpOAuth); - const response = await this.client.post( + const response = await this.client.post( '/api/mcp/oauth/start', request, { @@ -52,20 +81,37 @@ export class MCPClient { return response.data; } - async getOAuthStatus(jobId: string): Promise { + /** @deprecated Use the generated `AgentServerMCPOAuthStatusResponse`. */ + async getOAuthStatus(jobId: string): Promise; + async getOAuthStatus(jobId: string): Promise; + async getOAuthStatus( + jobId: string + ): Promise { await assertAgentServerSupports(this.client, AgentServerFeatureRequirements.mcpOAuth); - const response = await this.client.get( + const response = await this.client.get( `/api/mcp/oauth/status/${encodeURIComponent(jobId)}` ); return response.data; } + /** + * @deprecated Pass `AgentServerMCPOAuthCallbackRequest` and consume + * `AgentServerMCPOAuthCallbackResponse` for generated contract checking. + */ + async submitOAuthCallback( + jobId: string, + request: LegacyMCPOAuthCallbackRequest + ): Promise; + async submitOAuthCallback( + jobId: string, + request: AgentServerMCPOAuthCallbackRequest + ): Promise; async submitOAuthCallback( jobId: string, - request: MCPOAuthCallbackRequest - ): Promise { + request: LegacyMCPOAuthCallbackRequest | AgentServerMCPOAuthCallbackRequest + ): Promise { await assertAgentServerSupports(this.client, AgentServerFeatureRequirements.mcpOAuth); - const response = await this.client.post( + const response = await this.client.post( `/api/mcp/oauth/callback/${encodeURIComponent(jobId)}`, request ); diff --git a/src/client/settings-client.ts b/src/client/settings-client.ts index 1374ee5..54a24d7 100644 --- a/src/client/settings-client.ts +++ b/src/client/settings-client.ts @@ -20,6 +20,7 @@ import type { AgentServerSettingsResponse, AgentServerSettingsSchema, } from '../models/agent-server-api'; +import type { MCPConfigPatch, MCPServerPatch } from '../models/mcp-settings'; export interface SettingsClientOptions { host: string; @@ -84,6 +85,38 @@ export class SettingsClient { return response.data; } + /** + * Add or sparsely update one named MCP server without reading or resending + * the rest of the stored catalog. + */ + async patchMcpServer( + settingsKey: string, + patch: MCPServerPatch + ): Promise { + const mcpConfigPatch = { + [settingsKey]: patch, + } satisfies MCPConfigPatch; + return this.updateSettings({ + agent_settings_diff: { + mcp_config: mcpConfigPatch, + }, + }); + } + + /** + * Delete one named MCP server without altering sibling entries. + */ + async deleteMcpServer(settingsKey: string): Promise { + const mcpConfigPatch = { + [settingsKey]: null, + } satisfies MCPConfigPatch; + return this.updateSettings({ + agent_settings_diff: { + mcp_config: mcpConfigPatch, + }, + }); + } + async listSecrets(): Promise { const response = await this.client.get('/api/settings/secrets'); return response.data; diff --git a/src/clients.ts b/src/clients.ts index 1d568d8..258c918 100644 --- a/src/clients.ts +++ b/src/clients.ts @@ -90,12 +90,31 @@ export type { AgentServerMCPStartOAuthResponse, AgentServerMCPTestRequest, AgentServerMCPTestResponse, + AgentServerMCPToolCall, AgentServerMCPToolCallResult, AgentServerSettingsPatchRequest, AgentServerSettingsPatchResponse, AgentServerSettingsResponse, AgentServerSettingsSchema, } from './models/agent-server-api'; +export type { + MCPAuthCredential, + MCPAuthCredential as CanonicalMCPAuthCredential, + MCPConfig, + MCPConfigPatch, + MCPOAuthAuthentication, + MCPOAuthAuthentication as CanonicalMCPOAuthAuthentication, + MCPOAuthState, + MCPOAuthState as CanonicalMCPOAuthState, + MCPServer, + MCPServer as CanonicalMCPServer, + MCPServerPatch, + MCPTransport, + MCPTransport as CanonicalMCPTransport, + RemoteMCPServer, + RemoteMCPTransport, + StdioMCPServer, +} from './models/mcp-settings'; export type { OpenHandsClientKind, OpenHandsClientOptions, diff --git a/src/index.ts b/src/index.ts index c459a51..06d19e4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -299,12 +299,31 @@ export type { AgentServerMCPStartOAuthResponse, AgentServerMCPTestRequest, AgentServerMCPTestResponse, + AgentServerMCPToolCall, AgentServerMCPToolCallResult, AgentServerSettingsPatchRequest, AgentServerSettingsPatchResponse, AgentServerSettingsResponse, AgentServerSettingsSchema, } from './models/agent-server-api'; +export type { + MCPAuthCredential, + MCPAuthCredential as CanonicalMCPAuthCredential, + MCPConfig, + MCPConfigPatch, + MCPOAuthAuthentication, + MCPOAuthAuthentication as CanonicalMCPOAuthAuthentication, + MCPOAuthState, + MCPOAuthState as CanonicalMCPOAuthState, + MCPServer, + MCPServer as CanonicalMCPServer, + MCPServerPatch, + MCPTransport, + MCPTransport as CanonicalMCPTransport, + RemoteMCPServer, + RemoteMCPTransport, + StdioMCPServer, +} from './models/mcp-settings'; export type { AliveStatus, @@ -381,13 +400,8 @@ export type { StdioMCPServerSpec, RemoteMCPServerType, RemoteMCPServerSpec, - MCPTransport, MCPOAuthClientAuthMethod, MCPJsonValue, - MCPOAuthAuthentication, - MCPOAuthState, - MCPAuthCredential, - MCPServer, MCPServerSpec, MCPToolCallSpec, MCPTestRequest, diff --git a/src/models/agent-server-api.ts b/src/models/agent-server-api.ts index 5ad635a..549e83d 100644 --- a/src/models/agent-server-api.ts +++ b/src/models/agent-server-api.ts @@ -9,6 +9,7 @@ import type { GetConversationSettingsSchemaApiSettingsConversationSchemaGetResponse, GetMcpOauthStatusApiMcpOauthStatusJobIdGetResponse, GetSettingsApiSettingsGetResponse, + McpToolCallSpec, McpToolCallResult, StartMcpOauthApiMcpOauthStartPostData, StartMcpOauthApiMcpOauthStartPostResponse, @@ -29,6 +30,7 @@ export type AgentServerSettingsPatchResponse = UpdateSettingsApiSettingsPatchRes export type AgentServerMCPTestRequest = TestMcpServerApiMcpTestPostData['body']; export type AgentServerMCPTestResponse = TestMcpServerApiMcpTestPostResponse; +export type AgentServerMCPToolCall = McpToolCallSpec; export type AgentServerMCPStartOAuthRequest = StartMcpOauthApiMcpOauthStartPostData['body']; export type AgentServerMCPStartOAuthResponse = StartMcpOauthApiMcpOauthStartPostResponse; export type AgentServerMCPOAuthStatusResponse = GetMcpOauthStatusApiMcpOauthStatusJobIdGetResponse; diff --git a/src/models/api.ts b/src/models/api.ts index 519839e..78d8182 100644 --- a/src/models/api.ts +++ b/src/models/api.ts @@ -464,6 +464,7 @@ export interface HooksResponse { hook_config?: HookConfig | null; } +/** @deprecated Use `AgentServerMCPTestRequest["server"]` for MCP test payloads. */ export interface StdioMCPServerSpec { type: 'stdio'; command: string; @@ -474,6 +475,7 @@ export interface StdioMCPServerSpec { export type RemoteMCPServerType = 'http' | 'shttp' | 'streamable-http' | 'sse'; +/** @deprecated Use `AgentServerMCPTestRequest["server"]` for MCP test payloads. */ export interface RemoteMCPServerSpec { type: RemoteMCPServerType; url: string; @@ -538,13 +540,16 @@ export interface MCPServer { auth?: MCPAuthCredential | null; } +/** @deprecated Use `AgentServerMCPTestRequest["server"]` for MCP test payloads. */ export type MCPServerSpec = StdioMCPServerSpec | RemoteMCPServerSpec | MCPServer; +/** @deprecated Use `AgentServerMCPToolCall`. */ export interface MCPToolCallSpec { name: string; arguments?: Record; } +/** @deprecated Use `AgentServerMCPTestRequest`. */ export interface MCPTestRequest { server: MCPServerSpec; name?: string; @@ -552,6 +557,7 @@ export interface MCPTestRequest { tool_call?: MCPToolCallSpec | null; } +/** @deprecated Use `AgentServerMCPToolCallResult`. */ export interface MCPToolCallResult { is_error: boolean; text: string; @@ -573,6 +579,7 @@ export interface MCPTestFailure { error_kind: MCPTestFailureKind; } +/** @deprecated Use `AgentServerMCPTestResponse`. */ export type MCPTestResponse = MCPTestSuccess | MCPTestFailure; export interface MCPOAuthStartResponse { diff --git a/src/models/mcp-settings.ts b/src/models/mcp-settings.ts new file mode 100644 index 0000000..c389fe4 --- /dev/null +++ b/src/models/mcp-settings.ts @@ -0,0 +1,58 @@ +/** + * Canonical persisted MCP settings types. + * + * The generated Agent Server component remains the field-level source of + * truth. These aliases add the stable transport discrimination and sparse + * RFC 7386 mutation semantics expected by client callers. + */ +import type { + McpServerInputWritable, + McpoAuthAuthenticationInputWritable, + McpoAuthStateInputWritable, +} from '../generated/agent-server-schema'; + +type GeneratedMCPServer = McpServerInputWritable; +type GeneratedMCPTransport = NonNullable; +type GeneratedMCPAuthCredential = NonNullable; +type GeneratedMCPEnv = NonNullable; +type GeneratedMCPHeaders = NonNullable; + +type MCPDisplayFields = Pick; +type MCPStdioFields = Pick; +type MCPRemoteFields = Pick< + GeneratedMCPServer, + 'headers' | 'auth' | 'sse_read_timeout' | 'keep_alive' +>; + +export type MCPTransport = GeneratedMCPTransport; +export type RemoteMCPTransport = Exclude; +export type MCPAuthCredential = GeneratedMCPAuthCredential; +export type MCPOAuthAuthentication = McpoAuthAuthenticationInputWritable; +export type MCPOAuthState = McpoAuthStateInputWritable; + +export type StdioMCPServer = MCPDisplayFields & + MCPStdioFields & { + transport: 'stdio'; + command: string; + url?: never; + }; + +export type RemoteMCPServer = MCPDisplayFields & + MCPRemoteFields & { + transport: RemoteMCPTransport; + url: string; + command?: never; + }; + +export type MCPServer = StdioMCPServer | RemoteMCPServer; +export type MCPConfig = Record; + +type MCPStringMapPatch = Record | null; + +export type MCPServerPatch = Omit, 'env' | 'headers' | 'auth'> & { + env?: MCPStringMapPatch; + headers?: MCPStringMapPatch; + auth?: MCPAuthCredential | null; +}; + +export type MCPConfigPatch = Record; From 191070f75af0bd1700ba601fc119e2d5304f1dfb Mon Sep 17 00:00:00 2001 From: neubig Date: Mon, 27 Jul 2026 17:47:03 +0000 Subject: [PATCH 2/4] refactor: derive MCP patch fields from generated contract --- src/__tests__/agent-server-contract-types.test.ts | 12 ++++++++++++ src/models/mcp-settings.ts | 14 +++++++------- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/__tests__/agent-server-contract-types.test.ts b/src/__tests__/agent-server-contract-types.test.ts index 3f49e6f..aafbb1b 100644 --- a/src/__tests__/agent-server-contract-types.test.ts +++ b/src/__tests__/agent-server-contract-types.test.ts @@ -1,5 +1,6 @@ import type { MCPClient } from '../client/mcp-client'; import type { SettingsClient } from '../client/settings-client'; +import type { McpServerInputWritable } from '../generated/agent-server-schema'; import type { AgentServerMCPOAuthCallbackRequest, AgentServerMCPOAuthCallbackResponse, @@ -101,4 +102,15 @@ describe('Agent Server generated contract aliases', () => { expect('auth' in omitted).toBe(false); expect(cleared.auth).toBeNull(); }); + + it('derives every MCP patch field from the generated server contract', () => { + assertType>(true); + assertType | null | undefined>>( + true + ); + assertType | null | undefined>>( + true + ); + assertType>(true); + }); }); diff --git a/src/models/mcp-settings.ts b/src/models/mcp-settings.ts index c389fe4..9c4422a 100644 --- a/src/models/mcp-settings.ts +++ b/src/models/mcp-settings.ts @@ -14,8 +14,6 @@ import type { type GeneratedMCPServer = McpServerInputWritable; type GeneratedMCPTransport = NonNullable; type GeneratedMCPAuthCredential = NonNullable; -type GeneratedMCPEnv = NonNullable; -type GeneratedMCPHeaders = NonNullable; type MCPDisplayFields = Pick; type MCPStdioFields = Pick; @@ -47,12 +45,14 @@ export type RemoteMCPServer = MCPDisplayFields & export type MCPServer = StdioMCPServer | RemoteMCPServer; export type MCPConfig = Record; -type MCPStringMapPatch = Record | null; +type MergePatchField = string extends keyof NonNullable + ? NonNullable extends Record + ? Record | null + : Field + : Field; -export type MCPServerPatch = Omit, 'env' | 'headers' | 'auth'> & { - env?: MCPStringMapPatch; - headers?: MCPStringMapPatch; - auth?: MCPAuthCredential | null; +export type MCPServerPatch = { + [Field in keyof GeneratedMCPServer]?: MergePatchField; }; export type MCPConfigPatch = Record; From dfe83a59349d607219138592bdb8476315339ab1 Mon Sep 17 00:00:00 2001 From: neubig Date: Tue, 28 Jul 2026 18:20:17 +0000 Subject: [PATCH 3/4] refactor: use released MCP patch contract --- .../agent-server-contract-types.test.ts | 25 ++++++----- src/client/mcp-client.ts | 41 +++++-------------- src/clients.ts | 5 --- src/index.ts | 5 --- src/models/api.ts | 4 ++ src/models/mcp-settings.ts | 16 ++------ 6 files changed, 30 insertions(+), 66 deletions(-) diff --git a/src/__tests__/agent-server-contract-types.test.ts b/src/__tests__/agent-server-contract-types.test.ts index aafbb1b..75bbd75 100644 --- a/src/__tests__/agent-server-contract-types.test.ts +++ b/src/__tests__/agent-server-contract-types.test.ts @@ -1,6 +1,9 @@ import type { MCPClient } from '../client/mcp-client'; import type { SettingsClient } from '../client/settings-client'; -import type { McpServerInputWritable } from '../generated/agent-server-schema'; +import type { + McpConfigPatchWritable, + McpServerPatchWritable, +} from '../generated/agent-server-schema'; import type { AgentServerMCPOAuthCallbackRequest, AgentServerMCPOAuthCallbackResponse, @@ -37,6 +40,11 @@ type IsBareUnknown = : false : false; +type GeneratedMCPClientContract = { + testServer(request: AgentServerMCPTestRequest): Promise; + startOAuth(request: AgentServerMCPStartOAuthRequest): Promise; +}; + function assertType(_value: Type): void { // Compiling this call is the assertion. } @@ -71,16 +79,7 @@ describe('Agent Server generated contract aliases', () => { }); it('keeps MCP client methods checked against generated operations', () => { - assertType[0], AgentServerMCPTestRequest>>(true); - assertType>, AgentServerMCPTestResponse>>( - true - ); - assertType[0], AgentServerMCPStartOAuthRequest>>( - true - ); - assertType< - Equal>, AgentServerMCPStartOAuthResponse> - >(true); + assertType(true); assertType< Equal>, AgentServerMCPOAuthStatusResponse> >(true); @@ -104,13 +103,13 @@ describe('Agent Server generated contract aliases', () => { }); it('derives every MCP patch field from the generated server contract', () => { - assertType>(true); + assertType>(true); + assertType>(true); assertType | null | undefined>>( true ); assertType | null | undefined>>( true ); - assertType>(true); }); }); diff --git a/src/client/mcp-client.ts b/src/client/mcp-client.ts index 673edfb..20cb781 100644 --- a/src/client/mcp-client.ts +++ b/src/client/mcp-client.ts @@ -13,9 +13,7 @@ import type { AgentServerMCPTestResponse, } from '../models/agent-server-api'; import type { - MCPOAuthCallbackRequest as LegacyMCPOAuthCallbackRequest, MCPOAuthStartResponse as LegacyMCPOAuthStartResponse, - MCPOAuthStatusResponse as LegacyMCPOAuthStatusResponse, MCPTestRequest as LegacyMCPTestRequest, MCPTestResponse as LegacyMCPTestResponse, } from '../models/api'; @@ -41,13 +39,12 @@ export class MCPClient { }); } + async testServer(request: AgentServerMCPTestRequest): Promise; /** - * @deprecated Pass `AgentServerMCPTestRequest` for generated contract - * checking. This overload keeps existing callers source compatible while - * they migrate. + * @deprecated Pass `AgentServerMCPTestRequest`. This overload only keeps + * older request payloads source compatible while callers migrate. */ async testServer(request: LegacyMCPTestRequest): Promise; - async testServer(request: AgentServerMCPTestRequest): Promise; async testServer( request: LegacyMCPTestRequest | AgentServerMCPTestRequest ): Promise { @@ -58,15 +55,14 @@ export class MCPClient { return response.data; } - /** - * @deprecated Pass `AgentServerMCPStartOAuthRequest` for generated contract - * checking. This overload keeps existing callers source compatible while - * they migrate. - */ - async startOAuth(request: LegacyMCPTestRequest): Promise; async startOAuth( request: AgentServerMCPStartOAuthRequest ): Promise; + /** + * @deprecated Pass `AgentServerMCPStartOAuthRequest`. This overload only + * keeps older request payloads source compatible while callers migrate. + */ + async startOAuth(request: LegacyMCPTestRequest): Promise; async startOAuth( request: LegacyMCPTestRequest | AgentServerMCPStartOAuthRequest ): Promise { @@ -81,12 +77,7 @@ export class MCPClient { return response.data; } - /** @deprecated Use the generated `AgentServerMCPOAuthStatusResponse`. */ - async getOAuthStatus(jobId: string): Promise; - async getOAuthStatus(jobId: string): Promise; - async getOAuthStatus( - jobId: string - ): Promise { + async getOAuthStatus(jobId: string): Promise { await assertAgentServerSupports(this.client, AgentServerFeatureRequirements.mcpOAuth); const response = await this.client.get( `/api/mcp/oauth/status/${encodeURIComponent(jobId)}` @@ -94,22 +85,10 @@ export class MCPClient { return response.data; } - /** - * @deprecated Pass `AgentServerMCPOAuthCallbackRequest` and consume - * `AgentServerMCPOAuthCallbackResponse` for generated contract checking. - */ - async submitOAuthCallback( - jobId: string, - request: LegacyMCPOAuthCallbackRequest - ): Promise; async submitOAuthCallback( jobId: string, request: AgentServerMCPOAuthCallbackRequest - ): Promise; - async submitOAuthCallback( - jobId: string, - request: LegacyMCPOAuthCallbackRequest | AgentServerMCPOAuthCallbackRequest - ): Promise { + ): Promise { await assertAgentServerSupports(this.client, AgentServerFeatureRequirements.mcpOAuth); const response = await this.client.post( `/api/mcp/oauth/callback/${encodeURIComponent(jobId)}`, diff --git a/src/clients.ts b/src/clients.ts index 258c918..50cc04a 100644 --- a/src/clients.ts +++ b/src/clients.ts @@ -99,18 +99,13 @@ export type { } from './models/agent-server-api'; export type { MCPAuthCredential, - MCPAuthCredential as CanonicalMCPAuthCredential, MCPConfig, MCPConfigPatch, MCPOAuthAuthentication, - MCPOAuthAuthentication as CanonicalMCPOAuthAuthentication, MCPOAuthState, - MCPOAuthState as CanonicalMCPOAuthState, MCPServer, - MCPServer as CanonicalMCPServer, MCPServerPatch, MCPTransport, - MCPTransport as CanonicalMCPTransport, RemoteMCPServer, RemoteMCPTransport, StdioMCPServer, diff --git a/src/index.ts b/src/index.ts index 06d19e4..85b524d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -308,18 +308,13 @@ export type { } from './models/agent-server-api'; export type { MCPAuthCredential, - MCPAuthCredential as CanonicalMCPAuthCredential, MCPConfig, MCPConfigPatch, MCPOAuthAuthentication, - MCPOAuthAuthentication as CanonicalMCPOAuthAuthentication, MCPOAuthState, - MCPOAuthState as CanonicalMCPOAuthState, MCPServer, - MCPServer as CanonicalMCPServer, MCPServerPatch, MCPTransport, - MCPTransport as CanonicalMCPTransport, RemoteMCPServer, RemoteMCPTransport, StdioMCPServer, diff --git a/src/models/api.ts b/src/models/api.ts index 78d8182..cf91d9c 100644 --- a/src/models/api.ts +++ b/src/models/api.ts @@ -480,6 +480,7 @@ export interface RemoteMCPServerSpec { type: RemoteMCPServerType; url: string; headers?: Record; + /** @deprecated Use the tagged `auth` credential instead. */ api_key?: string | null; auth?: MCPAuthCredential | null; timeout?: number | null; @@ -582,6 +583,7 @@ export interface MCPTestFailure { /** @deprecated Use `AgentServerMCPTestResponse`. */ export type MCPTestResponse = MCPTestSuccess | MCPTestFailure; +/** @deprecated Use `AgentServerMCPStartOAuthResponse`. */ export interface MCPOAuthStartResponse { ok: boolean; job_id?: string | null; @@ -592,6 +594,7 @@ export interface MCPOAuthStartResponse { export type MCPOAuthProbeStatus = 'pending' | 'authorizing' | 'succeeded' | 'failed'; +/** @deprecated Use `AgentServerMCPOAuthStatusResponse`. */ export interface MCPOAuthStatusResponse { ok: boolean; status: MCPOAuthProbeStatus; @@ -605,6 +608,7 @@ export interface MCPOAuthStatusResponse { error_kind?: MCPTestFailureKind | null; } +/** @deprecated Use `AgentServerMCPOAuthCallbackRequest`. */ export interface MCPOAuthCallbackRequest { callback_url: string; } diff --git a/src/models/mcp-settings.ts b/src/models/mcp-settings.ts index 9c4422a..9d83478 100644 --- a/src/models/mcp-settings.ts +++ b/src/models/mcp-settings.ts @@ -6,7 +6,9 @@ * RFC 7386 mutation semantics expected by client callers. */ import type { + McpConfigPatchWritable, McpServerInputWritable, + McpServerPatchWritable, McpoAuthAuthenticationInputWritable, McpoAuthStateInputWritable, } from '../generated/agent-server-schema'; @@ -44,15 +46,5 @@ export type RemoteMCPServer = MCPDisplayFields & export type MCPServer = StdioMCPServer | RemoteMCPServer; export type MCPConfig = Record; - -type MergePatchField = string extends keyof NonNullable - ? NonNullable extends Record - ? Record | null - : Field - : Field; - -export type MCPServerPatch = { - [Field in keyof GeneratedMCPServer]?: MergePatchField; -}; - -export type MCPConfigPatch = Record; +export type MCPServerPatch = McpServerPatchWritable; +export type MCPConfigPatch = McpConfigPatchWritable; From 127e0e7844a23dc985008da664f9c580d6adadc9 Mon Sep 17 00:00:00 2001 From: neubig Date: Tue, 28 Jul 2026 21:03:27 +0000 Subject: [PATCH 4/4] feat: use dedicated MCP settings endpoints --- README.md | 14 ++- .../agent-server-contract-types.test.ts | 17 ++++ src/__tests__/mcp-settings-client.test.ts | 91 ++++++++++++------- src/client/settings-client.ts | 48 +++++----- 4 files changed, 110 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index 518a9d8..ab8623e 100644 --- a/README.md +++ b/README.md @@ -191,24 +191,28 @@ const acpCount = await manager.acp.countConversations(); ### Updating MCP settings -Persisted MCP servers use their settings-map key as stable identity. Add or -update one server with a sparse patch instead of fetching and resending the -whole catalog: +Persisted MCP servers use their settings-map key as stable identity. Create, +update, or delete one server without fetching and resending the whole catalog: ```typescript const settings = manager.settings; -await settings.patchMcpServer('github', { +await settings.createMcpServer('github', { + transport: 'http', url: 'https://example.test/mcp', }); +await settings.patchMcpServer('github', { + url: 'https://example.test/mcp/v2', +}); + await settings.deleteMcpServer('old-server'); ``` An omitted patch field preserves its stored value, including credentials. Supplying a value replaces it, and `null` explicitly clears a supported optional field or deletes a map entry. Each helper sends one -`PATCH /api/settings` request. +request to the corresponding Agent Server MCP settings operation. If you need the lower-level endpoint-specific clients directly, import them from the secondary entrypoint: diff --git a/src/__tests__/agent-server-contract-types.test.ts b/src/__tests__/agent-server-contract-types.test.ts index 75bbd75..bf3e54b 100644 --- a/src/__tests__/agent-server-contract-types.test.ts +++ b/src/__tests__/agent-server-contract-types.test.ts @@ -63,6 +63,23 @@ describe('Agent Server generated contract aliases', () => { assertType< Equal>, AgentServerSettingsPatchResponse> >(true); + assertType[1], MCPServer>>(true); + assertType[1], MCPServerPatch>>(true); + assertType< + Equal< + Awaited>, + AgentServerSettingsPatchResponse + > + >(true); + assertType< + Equal>, AgentServerSettingsPatchResponse> + >(true); + assertType< + Equal< + Awaited>, + AgentServerSettingsPatchResponse + > + >(true); }); it('keeps important MCP contract exports stronger than any or bare unknown', () => { diff --git a/src/__tests__/mcp-settings-client.test.ts b/src/__tests__/mcp-settings-client.test.ts index e45d822..5e66fcc 100644 --- a/src/__tests__/mcp-settings-client.test.ts +++ b/src/__tests__/mcp-settings-client.test.ts @@ -1,5 +1,5 @@ import { SettingsClient } from '../client/settings-client'; -import type { MCPConfig, MCPServerPatch } from '../models/mcp-settings'; +import type { MCPConfig, MCPServer } from '../models/mcp-settings'; type JsonObject = Record; @@ -29,27 +29,39 @@ function mergePatch(target: JsonObject, patch: JsonObject): JsonObject { function installStatefulSettingsServer(initial: MCPConfig) { const catalog = clone(initial) as unknown as JsonObject; - const fetchMock = jest.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { - const request = JSON.parse(String(init?.body)) as { - agent_settings_diff: { mcp_config: Record }; - }; - for (const [settingsKey, patch] of Object.entries(request.agent_settings_diff.mcp_config)) { - if (patch === null) { - delete catalog[settingsKey]; - } else { - catalog[settingsKey] = mergePatch( - (catalog[settingsKey] as JsonObject | undefined) ?? {}, - patch as JsonObject - ); + const fetchMock = jest.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = new URL(String(input)); + const settingsKey = decodeURIComponent(url.pathname.split('/').at(-1) ?? ''); + const method = init?.method; + const body = init?.body ? (JSON.parse(String(init.body)) as JsonObject) : undefined; + + if (method === 'POST') { + if (settingsKey in catalog) { + return new Response(null, { status: 409, statusText: 'Conflict' }); } + catalog[settingsKey] = clone(body as unknown as MCPServer) as unknown as JsonObject; + } else if (method === 'PATCH') { + if (!(settingsKey in catalog)) { + return new Response(null, { status: 404, statusText: 'Not Found' }); + } + catalog[settingsKey] = mergePatch(catalog[settingsKey] as JsonObject, body ?? {}); + } else if (method === 'DELETE') { + if (!(settingsKey in catalog)) { + return new Response(null, { status: 404, statusText: 'Not Found' }); + } + delete catalog[settingsKey]; } + return new Response( JSON.stringify({ agent_settings: { mcp_config: catalog }, conversation_settings: {}, llm_api_key_is_set: false, }), - { status: 200, headers: { 'content-type': 'application/json' } } + { + status: method === 'POST' ? 201 : 200, + headers: { 'content-type': 'application/json' }, + } ); }); global.fetch = fetchMock as typeof fetch; @@ -79,7 +91,7 @@ describe('SettingsClient canonical MCP mutations', () => { const server = installStatefulSettingsServer(initialCatalog()); const client = new SettingsClient({ host: 'http://example.com' }); - await client.patchMcpServer('docs', { + await client.createMcpServer('docs', { transport: 'http', url: 'https://example.test/mcp', auth: { strategy: 'bearer', value: 'docs-secret' }, @@ -96,19 +108,13 @@ describe('SettingsClient canonical MCP mutations', () => { }, }); expect(server.fetchMock).toHaveBeenCalledWith( - 'http://example.com/api/settings', + 'http://example.com/api/settings/mcp/docs', expect.objectContaining({ - method: 'PATCH', + method: 'POST', body: JSON.stringify({ - agent_settings_diff: { - mcp_config: { - docs: { - transport: 'http', - url: 'https://example.test/mcp', - auth: { strategy: 'bearer', value: 'docs-secret' }, - }, - }, - }, + transport: 'http', + url: 'https://example.test/mcp', + auth: { strategy: 'bearer', value: 'docs-secret' }, }), }) ); @@ -127,6 +133,13 @@ describe('SettingsClient canonical MCP mutations', () => { url: 'https://example.test/github-mcp', auth: { strategy: 'bearer', value: 'github-secret' }, }); + expect(server.fetchMock).toHaveBeenCalledWith( + 'http://example.com/api/settings/mcp/github', + expect.objectContaining({ + method: 'PATCH', + body: JSON.stringify({ url: 'https://example.test/github-mcp' }), + }) + ); }); it('replaces and explicitly clears auth', async () => { @@ -161,12 +174,9 @@ describe('SettingsClient canonical MCP mutations', () => { }, }); expect(server.fetchMock).toHaveBeenCalledWith( - 'http://example.com/api/settings', + 'http://example.com/api/settings/mcp/filesystem', expect.objectContaining({ - method: 'PATCH', - body: JSON.stringify({ - agent_settings_diff: { mcp_config: { filesystem: null } }, - }), + method: 'DELETE', }) ); }); @@ -188,4 +198,23 @@ describe('SettingsClient canonical MCP mutations', () => { (server.getCatalog().github as JsonObject).headers as Record ).not.toHaveProperty('X-Remove'); }); + + it('enforces create and existing-server preconditions', async () => { + const server = installStatefulSettingsServer(initialCatalog()); + const client = new SettingsClient({ host: 'http://example.com' }); + + await expect( + client.createMcpServer('github', { + transport: 'http', + url: 'https://replacement.example/mcp', + }) + ).rejects.toMatchObject({ status: 409 }); + await expect( + client.patchMcpServer('missing', { url: 'https://example.test/mcp' }) + ).rejects.toMatchObject({ status: 404 }); + await expect(client.deleteMcpServer('missing')).rejects.toMatchObject({ status: 404 }); + + expect(server.fetchMock).toHaveBeenCalledTimes(3); + expect(server.getCatalog()).toEqual(initialCatalog()); + }); }); diff --git a/src/client/settings-client.ts b/src/client/settings-client.ts index 54a24d7..868edd3 100644 --- a/src/client/settings-client.ts +++ b/src/client/settings-client.ts @@ -20,7 +20,7 @@ import type { AgentServerSettingsResponse, AgentServerSettingsSchema, } from '../models/agent-server-api'; -import type { MCPConfigPatch, MCPServerPatch } from '../models/mcp-settings'; +import type { MCPServer, MCPServerPatch } from '../models/mcp-settings'; export interface SettingsClientOptions { host: string; @@ -85,36 +85,36 @@ export class SettingsClient { return response.data; } - /** - * Add or sparsely update one named MCP server without reading or resending - * the rest of the stored catalog. - */ + /** Create one named MCP server without reading or resending the catalog. */ + async createMcpServer( + settingsKey: string, + server: MCPServer + ): Promise { + const response = await this.client.post( + `/api/settings/mcp/${encodeURIComponent(settingsKey)}`, + server + ); + return response.data; + } + + /** Sparsely update one existing named MCP server. */ async patchMcpServer( settingsKey: string, patch: MCPServerPatch ): Promise { - const mcpConfigPatch = { - [settingsKey]: patch, - } satisfies MCPConfigPatch; - return this.updateSettings({ - agent_settings_diff: { - mcp_config: mcpConfigPatch, - }, - }); + const response = await this.client.patch( + `/api/settings/mcp/${encodeURIComponent(settingsKey)}`, + patch + ); + return response.data; } - /** - * Delete one named MCP server without altering sibling entries. - */ + /** Delete one existing named MCP server. */ async deleteMcpServer(settingsKey: string): Promise { - const mcpConfigPatch = { - [settingsKey]: null, - } satisfies MCPConfigPatch; - return this.updateSettings({ - agent_settings_diff: { - mcp_config: mcpConfigPatch, - }, - }); + const response = await this.client.delete( + `/api/settings/mcp/${encodeURIComponent(settingsKey)}` + ); + return response.data; } async listSecrets(): Promise {