From 169376e70e730add7ade2031ca251e817c4757e6 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Wed, 20 May 2026 12:02:32 -0600 Subject: [PATCH 1/4] [MCP] foundation: component scaffold + config schema + boot gating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lands the empty MCP component, the mcp: config block, and a config-gated registration hook in the operations server. Until #614 ships the Streamable HTTP transport, flipping mcp.operations.enabled: true serves a 503 with `{error:"mcp_not_implemented", profile:"operations"}` from the configured mountPath — enough to prove the gate end-to-end. The application-profile call site is intentionally deferred to #614, where the transport will register itself via server.http(...). The shared registerMcpProfile() supports both profiles already (verified by tests). Defaults: mcp.{operations,application}.enabled: false. Existing deployments see no change on upgrade. Closes #613 Tracking: HarperFast/harper#465 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 --- components/mcp/index.ts | 72 +++++++++++++++ server/operationsServer.ts | 16 ++++ static/defaultConfig.yaml | 34 +++++++ unitTests/components/mcp/index.test.js | 96 ++++++++++++++++++++ unitTests/validation/configValidator.test.js | 71 +++++++++++++++ utility/hdbTerms.ts | 21 +++++ validation/configValidator.ts | 30 ++++++ 7 files changed, 340 insertions(+) create mode 100644 components/mcp/index.ts create mode 100644 unitTests/components/mcp/index.test.js diff --git a/components/mcp/index.ts b/components/mcp/index.ts new file mode 100644 index 000000000..7c9ba547f --- /dev/null +++ b/components/mcp/index.ts @@ -0,0 +1,72 @@ +/** + * Native MCP (Model Context Protocol) server component for Harper. + * + * Foundation PR (#613): exports a config-gated registration hook used by the + * operations and HTTP host servers. The hook installs a placeholder route + * that returns HTTP 503 with body `{ error: 'mcp_not_implemented', profile }` + * until the Streamable HTTP transport lands in #614. Tracking: #465. + */ +import harperLogger from '../../utility/logging/harper_logger.ts'; + +export type McpProfile = 'operations' | 'application'; + +interface McpProfileConfig { + enabled?: boolean; + mountPath?: string; +} + +interface FullConfig { + mcp?: { + operations?: McpProfileConfig; + application?: McpProfileConfig; + }; +} + +interface FastifyLike { + post: (path: string, ...rest: unknown[]) => unknown; +} + +export interface RegisterMcpProfileArgs { + profile: McpProfile; + host: FastifyLike; + config: FullConfig; +} + +const DEFAULT_MOUNT_PATH = '/mcp'; + +/** + * Register the MCP profile on its host server when enabled in config. + * + * The stub responder is intentionally minimal — sub-issue #614 replaces it + * with the real Streamable HTTP transport without changing this gate. + */ +export function registerMcpProfile({ profile, host, config }: RegisterMcpProfileArgs): void { + const profileConfig = config?.mcp?.[profile]; + if (!profileConfig?.enabled) { + harperLogger.trace(`MCP ${profile} profile disabled, skipping registration`); + return; + } + + const mountPath = profileConfig.mountPath ?? DEFAULT_MOUNT_PATH; + host.post(mountPath, createStubHandler(profile)); + harperLogger.info(`MCP ${profile} profile registered at ${mountPath}`); +} + +/** + * Builds the placeholder 503 handler. Returned function is Fastify-compatible: + * `(request, reply)` where `reply` exposes `code()`, `header()`, and `send()`. + */ +export function createStubHandler(profile: McpProfile) { + return async function mcpStubHandler(_request: unknown, reply: McpReply): Promise { + reply.code(503); + reply.header('Retry-After', '0'); + reply.header('Content-Type', 'application/json'); + reply.send({ error: 'mcp_not_implemented', profile }); + }; +} + +interface McpReply { + code: (status: number) => McpReply; + header: (name: string, value: string) => McpReply; + send: (body: unknown) => McpReply; +} diff --git a/server/operationsServer.ts b/server/operationsServer.ts index 92e58f5e8..b6f847e72 100644 --- a/server/operationsServer.ts +++ b/server/operationsServer.ts @@ -25,6 +25,7 @@ import { } from './serverHelpers/serverHandlers.js'; import { registerBunFastifyInstance } from './http.ts'; import { registerContentHandlers } from './serverHelpers/contentTypes.ts'; +import { registerMcpProfile } from '../components/mcp/index.ts'; import type { OperationFunctionName } from './serverHelpers/serverUtilities.ts'; type ParsedSqlObject = any; import { generateJsonApi } from '../resources/openApi.ts'; @@ -181,6 +182,21 @@ function buildServer(isHttps: boolean, resources: Resources): FastifyInstance { }); registerContentHandlers(app); + if (env.get(terms.CONFIG_PARAMS.MCP_OPERATIONS_ENABLED)) { + registerMcpProfile({ + profile: 'operations', + host: app, + config: { + mcp: { + operations: { + enabled: true, + mountPath: env.get(terms.CONFIG_PARAMS.MCP_OPERATIONS_MOUNTPATH) ?? '/mcp', + }, + }, + }, + }); + } + // Add a simple health check app.get('/health', () => 'Harper is running.'); diff --git a/static/defaultConfig.yaml b/static/defaultConfig.yaml index 926d32320..ce47d230e 100644 --- a/static/defaultConfig.yaml +++ b/static/defaultConfig.yaml @@ -77,3 +77,37 @@ tls: privateKey: null node: hostname: null +mcp: + operations: + enabled: false + mountPath: /mcp + allow: + - describe_* + - list_* + - search_* + - get_* + - system_information + - read_log + - read_audit_log + deny: [] + maxTools: 200 + rateLimit: + perToolPerSecond: 10 + perToolBurst: 20 + sessionConcurrency: 25 + sessionPerSecond: 100 + application: + enabled: false + mountPath: /mcp + allow: [] + deny: [] + maxTools: 500 + searchMaxResults: 100 + rateLimit: + perToolPerSecond: 25 + perToolBurst: 50 + sessionConcurrency: 50 + sessionPerSecond: 200 + session: + idleTimeoutSeconds: 1800 + allowClientDelete: true diff --git a/unitTests/components/mcp/index.test.js b/unitTests/components/mcp/index.test.js new file mode 100644 index 000000000..d10d86c69 --- /dev/null +++ b/unitTests/components/mcp/index.test.js @@ -0,0 +1,96 @@ +const assert = require('node:assert/strict'); +const { registerMcpProfile, createStubHandler } = require('#src/components/mcp/index'); + +function makeFakeFastify() { + const calls = []; + return { + calls, + post(path, handler) { + calls.push({ path, handler }); + }, + }; +} + +function makeFakeReply() { + const reply = { + statusCode: undefined, + headers: {}, + body: undefined, + code(status) { + this.statusCode = status; + return this; + }, + header(name, value) { + this.headers[name] = value; + return this; + }, + send(payload) { + this.body = payload; + return this; + }, + }; + return reply; +} + +describe('components/mcp/index', () => { + describe('registerMcpProfile', () => { + it('does nothing when the profile is disabled', () => { + const host = makeFakeFastify(); + registerMcpProfile({ + profile: 'operations', + host, + config: { mcp: { operations: { enabled: false } } }, + }); + assert.equal(host.calls.length, 0); + }); + + it('does nothing when the mcp config block is absent', () => { + const host = makeFakeFastify(); + registerMcpProfile({ profile: 'operations', host, config: {} }); + assert.equal(host.calls.length, 0); + }); + + it('registers POST /mcp when operations profile is enabled with defaults', () => { + const host = makeFakeFastify(); + registerMcpProfile({ + profile: 'operations', + host, + config: { mcp: { operations: { enabled: true } } }, + }); + assert.equal(host.calls.length, 1); + assert.equal(host.calls[0].path, '/mcp'); + assert.equal(typeof host.calls[0].handler, 'function'); + }); + + it('honors a custom mountPath', () => { + const host = makeFakeFastify(); + registerMcpProfile({ + profile: 'application', + host, + config: { mcp: { application: { enabled: true, mountPath: '/agent' } } }, + }); + assert.equal(host.calls.length, 1); + assert.equal(host.calls[0].path, '/agent'); + }); + }); + + describe('stub handler', () => { + it('returns 503 with mcp_not_implemented body for the operations profile', async () => { + const handler = createStubHandler('operations'); + const reply = makeFakeReply(); + await handler({}, reply); + assert.equal(reply.statusCode, 503); + assert.equal(reply.headers['Retry-After'], '0'); + assert.equal(reply.headers['Content-Type'], 'application/json'); + assert.deepEqual(reply.body, { error: 'mcp_not_implemented', profile: 'operations' }); + }); + + it('returns 503 with mcp_not_implemented body for the application profile', async () => { + const handler = createStubHandler('application'); + const reply = makeFakeReply(); + await handler({}, reply); + assert.equal(reply.statusCode, 503); + assert.deepEqual(reply.body, { error: 'mcp_not_implemented', profile: 'application' }); + }); + }); +}); diff --git a/unitTests/validation/configValidator.test.js b/unitTests/validation/configValidator.test.js index ead5294c9..6b9452685 100644 --- a/unitTests/validation/configValidator.test.js +++ b/unitTests/validation/configValidator.test.js @@ -385,4 +385,75 @@ describe('Test configValidator module', () => { "Invalid logging.rotation.interval value. Value should be a number followed by unit e.g. '10D'" ); }); + + describe('mcp config', () => { + it('validates clean when mcp block is absent', () => { + const result = configValidator(testUtils.deepClone(FAKE_CONFIG), true); + expect(result.error).to.be.undefined; + }); + + it('validates clean and applies defaults when only mcp.operations.enabled is given', () => { + const config = testUtils.deepClone(FAKE_CONFIG); + config.mcp = { operations: { enabled: false } }; + const result = configValidator(config, true); + expect(result.error).to.be.undefined; + expect(result.value.mcp.operations.enabled).to.equal(false); + expect(result.value.mcp.operations.mountPath).to.equal('/mcp'); + }); + + it('validates clean when the full mcp block from defaults is supplied', () => { + const config = testUtils.deepClone(FAKE_CONFIG); + config.mcp = { + operations: { + enabled: false, + mountPath: '/mcp', + allow: ['describe_*', 'list_*'], + deny: [], + maxTools: 200, + rateLimit: { + perToolPerSecond: 10, + perToolBurst: 20, + sessionConcurrency: 25, + sessionPerSecond: 100, + }, + }, + application: { + enabled: false, + mountPath: '/mcp', + allow: [], + deny: [], + maxTools: 500, + searchMaxResults: 100, + rateLimit: { + perToolPerSecond: 25, + perToolBurst: 50, + sessionConcurrency: 50, + sessionPerSecond: 200, + }, + }, + session: { + idleTimeoutSeconds: 1800, + allowClientDelete: true, + }, + }; + const result = configValidator(config, true); + expect(result.error).to.be.undefined; + }); + + it('rejects mcp.operations.enabled with a non-boolean', () => { + const config = testUtils.deepClone(FAKE_CONFIG); + config.mcp = { operations: { enabled: 'yes' } }; + const result = configValidator(config, true); + expect(result.error).to.not.be.undefined; + expect(result.error.message).to.include("'mcp.operations.enabled' must be a boolean"); + }); + + it('rejects mcp.operations.mountPath with a non-string', () => { + const config = testUtils.deepClone(FAKE_CONFIG); + config.mcp = { operations: { mountPath: 42 } }; + const result = configValidator(config, true); + expect(result.error).to.not.be.undefined; + expect(result.error.message).to.include("'mcp.operations.mountPath' must be a string"); + }); + }); }); diff --git a/utility/hdbTerms.ts b/utility/hdbTerms.ts index 0c65031b0..a3355b71b 100644 --- a/utility/hdbTerms.ts +++ b/utility/hdbTerms.ts @@ -523,6 +523,27 @@ export const CONFIG_PARAMS = { OPERATIONSAPI_NETWORK_TIMEOUT: 'operationsApi_network_timeout', OPERATIONSAPI_SYSINFO_NETWORK: 'operationsApi_sysInfo_network', OPERATIONSAPI_SYSINFO_DISK: 'operationsApi_sysInfo_disk', + MCP_OPERATIONS_ENABLED: 'mcp_operations_enabled', + MCP_OPERATIONS_MOUNTPATH: 'mcp_operations_mountPath', + MCP_OPERATIONS_ALLOW: 'mcp_operations_allow', + MCP_OPERATIONS_DENY: 'mcp_operations_deny', + MCP_OPERATIONS_MAXTOOLS: 'mcp_operations_maxTools', + MCP_OPERATIONS_RATELIMIT_PERTOOLPERSECOND: 'mcp_operations_rateLimit_perToolPerSecond', + MCP_OPERATIONS_RATELIMIT_PERTOOLBURST: 'mcp_operations_rateLimit_perToolBurst', + MCP_OPERATIONS_RATELIMIT_SESSIONCONCURRENCY: 'mcp_operations_rateLimit_sessionConcurrency', + MCP_OPERATIONS_RATELIMIT_SESSIONPERSECOND: 'mcp_operations_rateLimit_sessionPerSecond', + MCP_APPLICATION_ENABLED: 'mcp_application_enabled', + MCP_APPLICATION_MOUNTPATH: 'mcp_application_mountPath', + MCP_APPLICATION_ALLOW: 'mcp_application_allow', + MCP_APPLICATION_DENY: 'mcp_application_deny', + MCP_APPLICATION_MAXTOOLS: 'mcp_application_maxTools', + MCP_APPLICATION_SEARCHMAXRESULTS: 'mcp_application_searchMaxResults', + MCP_APPLICATION_RATELIMIT_PERTOOLPERSECOND: 'mcp_application_rateLimit_perToolPerSecond', + MCP_APPLICATION_RATELIMIT_PERTOOLBURST: 'mcp_application_rateLimit_perToolBurst', + MCP_APPLICATION_RATELIMIT_SESSIONCONCURRENCY: 'mcp_application_rateLimit_sessionConcurrency', + MCP_APPLICATION_RATELIMIT_SESSIONPERSECOND: 'mcp_application_rateLimit_sessionPerSecond', + MCP_SESSION_IDLETIMEOUTSECONDS: 'mcp_session_idleTimeoutSeconds', + MCP_SESSION_ALLOWCLIENTDELETE: 'mcp_session_allowClientDelete', REPLICATION: 'replication', REPLICATION_HOSTNAME: 'replication_hostname', REPLICATION_URL: 'replication_url', diff --git a/validation/configValidator.ts b/validation/configValidator.ts index 73bcab9aa..ec9214018 100644 --- a/validation/configValidator.ts +++ b/validation/configValidator.ts @@ -66,6 +66,35 @@ export function configValidator(configJson, skipFsValidation = false) { privateKey: pemFileConstraints, }); + // MCP — sub-issue #613 lands the config surface ahead of the transport (#614). + // Both profiles default to enabled:false so existing deployments are unchanged on upgrade. + const mcpRateLimitSchema = Joi.object({ + perToolPerSecond: number.min(0).optional(), + perToolBurst: number.min(0).optional(), + sessionConcurrency: number.min(0).optional(), + sessionPerSecond: number.min(0).optional(), + }); + const mcpOperationsSchema = Joi.object({ + enabled: boolean.optional().default(false), + mountPath: string.optional().default('/mcp'), + allow: array.items(string).optional(), + deny: array.items(string).optional(), + maxTools: number.min(1).optional(), + rateLimit: mcpRateLimitSchema.optional(), + }); + const mcpApplicationSchema = mcpOperationsSchema.keys({ + searchMaxResults: number.min(1).optional(), + }); + const mcpSessionSchema = Joi.object({ + idleTimeoutSeconds: number.min(1).optional(), + allowClientDelete: boolean.optional(), + }); + const mcpSchema = Joi.object({ + operations: mcpOperationsSchema.optional(), + application: mcpApplicationSchema.optional(), + session: mcpSessionSchema.optional(), + }); + const configSchema = Joi.object({ authentication: Joi.alternatives( Joi.object({ @@ -195,6 +224,7 @@ export function configValidator(configJson, skipFsValidation = false) { maxFreeSpaceToLoad: number.optional(), maxFreeSpaceToRetain: number.optional(), }).required(), + mcp: mcpSchema.optional(), ignoreScripts: boolean.optional(), tls: Joi.alternatives([Joi.array().items(tlsConstraints), tlsConstraints]), }); From da9b7d87aa7b974a5a0f1dc1636cbe12ce1f12d5 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Wed, 20 May 2026 12:50:18 -0600 Subject: [PATCH 2/4] fix(mcp): coerce env-sourced enable flag + wire authHandler on stub MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gemini cross-model review surfaced two issues: 1. **Truthiness regression.** `env.get(MCP_OPERATIONS_ENABLED)` can return the literal string `'false'` when sourced from an environment variable, which is truthy in JS — `mcp.operations.enabled: false` would still register the route. Switched to the in-file pattern used for `studioOn` (commonUtils.isEmpty + toString().toLowerCase() === 'true'). 2. **Auth missing on stub route.** The /mcp stub had no preValidation while the rest of the operations port uses authHandler. Pre-staging the design's auth-first stance from #465: registerMcpProfile now accepts optional `routeOptions` (forwarded as Fastify's 3-arg post), and the operations call site passes `{preValidation:[authHandler]}`. Also removed the redundant `?? '/mcp'` fallback at the call site — registerMcpProfile already defaults the mount path. New tests cover routeOptions forwarding and the strict-boolean contract. Refs #613. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 --- components/mcp/index.ts | 11 ++++++-- server/operationsServer.ts | 6 +++-- unitTests/components/mcp/index.test.js | 35 ++++++++++++++++++++++++-- 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/components/mcp/index.ts b/components/mcp/index.ts index 7c9ba547f..66f719859 100644 --- a/components/mcp/index.ts +++ b/components/mcp/index.ts @@ -30,6 +30,8 @@ export interface RegisterMcpProfileArgs { profile: McpProfile; host: FastifyLike; config: FullConfig; + /** Route-level options forwarded to the host's `post(path, options, handler)` 3-arg form (e.g., Fastify `preValidation`). */ + routeOptions?: Record; } const DEFAULT_MOUNT_PATH = '/mcp'; @@ -40,7 +42,7 @@ const DEFAULT_MOUNT_PATH = '/mcp'; * The stub responder is intentionally minimal — sub-issue #614 replaces it * with the real Streamable HTTP transport without changing this gate. */ -export function registerMcpProfile({ profile, host, config }: RegisterMcpProfileArgs): void { +export function registerMcpProfile({ profile, host, config, routeOptions }: RegisterMcpProfileArgs): void { const profileConfig = config?.mcp?.[profile]; if (!profileConfig?.enabled) { harperLogger.trace(`MCP ${profile} profile disabled, skipping registration`); @@ -48,7 +50,12 @@ export function registerMcpProfile({ profile, host, config }: RegisterMcpProfile } const mountPath = profileConfig.mountPath ?? DEFAULT_MOUNT_PATH; - host.post(mountPath, createStubHandler(profile)); + const handler = createStubHandler(profile); + if (routeOptions) { + host.post(mountPath, routeOptions, handler); + } else { + host.post(mountPath, handler); + } harperLogger.info(`MCP ${profile} profile registered at ${mountPath}`); } diff --git a/server/operationsServer.ts b/server/operationsServer.ts index b6f847e72..695ddca93 100644 --- a/server/operationsServer.ts +++ b/server/operationsServer.ts @@ -182,7 +182,8 @@ function buildServer(isHttps: boolean, resources: Resources): FastifyInstance { }); registerContentHandlers(app); - if (env.get(terms.CONFIG_PARAMS.MCP_OPERATIONS_ENABLED)) { + const mcpOperationsEnabled = env.get(terms.CONFIG_PARAMS.MCP_OPERATIONS_ENABLED); + if (!commonUtils.isEmpty(mcpOperationsEnabled) && mcpOperationsEnabled.toString().toLowerCase() === 'true') { registerMcpProfile({ profile: 'operations', host: app, @@ -190,10 +191,11 @@ function buildServer(isHttps: boolean, resources: Resources): FastifyInstance { mcp: { operations: { enabled: true, - mountPath: env.get(terms.CONFIG_PARAMS.MCP_OPERATIONS_MOUNTPATH) ?? '/mcp', + mountPath: env.get(terms.CONFIG_PARAMS.MCP_OPERATIONS_MOUNTPATH), }, }, }, + routeOptions: { preValidation: [authHandler] }, }); } diff --git a/unitTests/components/mcp/index.test.js b/unitTests/components/mcp/index.test.js index d10d86c69..c16827bc1 100644 --- a/unitTests/components/mcp/index.test.js +++ b/unitTests/components/mcp/index.test.js @@ -5,8 +5,12 @@ function makeFakeFastify() { const calls = []; return { calls, - post(path, handler) { - calls.push({ path, handler }); + post(path, optsOrHandler, maybeHandler) { + if (typeof optsOrHandler === 'function') { + calls.push({ path, options: undefined, handler: optsOrHandler }); + } else { + calls.push({ path, options: optsOrHandler, handler: maybeHandler }); + } }, }; } @@ -72,6 +76,33 @@ describe('components/mcp/index', () => { assert.equal(host.calls.length, 1); assert.equal(host.calls[0].path, '/agent'); }); + + it('forwards routeOptions to the host as the second argument', () => { + const host = makeFakeFastify(); + const sentinel = { preValidation: ['fake-auth-handler'] }; + registerMcpProfile({ + profile: 'operations', + host, + config: { mcp: { operations: { enabled: true } } }, + routeOptions: sentinel, + }); + assert.equal(host.calls.length, 1); + assert.deepEqual(host.calls[0].options, sentinel); + assert.equal(typeof host.calls[0].handler, 'function'); + }); + + it('treats only strict-boolean enabled:true as enabled (no string truthiness)', () => { + // Regression: env-sourced configs can deliver the literal string 'false', + // which is truthy in JS. The caller is responsible for coercing — this test + // just pins the component contract that `enabled` is read as-is. + const host = makeFakeFastify(); + registerMcpProfile({ + profile: 'operations', + host, + config: { mcp: { operations: { enabled: 0 } } }, + }); + assert.equal(host.calls.length, 0); + }); }); describe('stub handler', () => { From 54139bbef61e435a741979e24ced3729464eb5af Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Wed, 20 May 2026 13:12:18 -0600 Subject: [PATCH 3/4] refactor(mcp): presence-based enablement, drop `enabled` field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Harper convention (matches `replication`): a profile is enabled iff its config sub-block is present, not via an `enabled: true/false` flag. - Drop `mcp..enabled` from the Joi schema. - Drop the `mcp:` block from static/defaultConfig.yaml entirely — opt-in. - Drop `MCP_OPERATIONS_ENABLED` / `MCP_APPLICATION_ENABLED` config keys. - Component: skip registration when `config.mcp.` is absent. - Operations call site: gate on presence of `MCP_OPERATIONS_MOUNTPATH` (Joi always defaults `mountPath` to `/mcp` when the block is set). Backward compatibility: existing deployments have no `mcp:` block, so this preserves the "MCP off by default" behavior — same outcome as the prior `enabled: false` defaults, fewer keys to maintain. Refs #613. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 --- components/mcp/index.ts | 15 ++++---- server/operationsServer.ts | 16 ++++----- static/defaultConfig.yaml | 34 ------------------ unitTests/components/mcp/index.test.js | 37 +++++++------------- unitTests/validation/configValidator.test.js | 24 ++++++------- utility/hdbTerms.ts | 2 -- validation/configValidator.ts | 4 +-- 7 files changed, 39 insertions(+), 93 deletions(-) diff --git a/components/mcp/index.ts b/components/mcp/index.ts index 66f719859..08503d7ee 100644 --- a/components/mcp/index.ts +++ b/components/mcp/index.ts @@ -1,17 +1,18 @@ /** * Native MCP (Model Context Protocol) server component for Harper. * - * Foundation PR (#613): exports a config-gated registration hook used by the - * operations and HTTP host servers. The hook installs a placeholder route - * that returns HTTP 503 with body `{ error: 'mcp_not_implemented', profile }` - * until the Streamable HTTP transport lands in #614. Tracking: #465. + * Foundation PR (#613): exports a presence-gated registration hook used by + * the operations and HTTP host servers. The hook installs a placeholder + * route that returns HTTP 503 with body `{ error: 'mcp_not_implemented', + * profile }` until the Streamable HTTP transport lands in #614. A profile + * is enabled when its sub-block exists in config (matches Harper's + * `replication` convention — no explicit `enabled` flag). Tracking: #465. */ import harperLogger from '../../utility/logging/harper_logger.ts'; export type McpProfile = 'operations' | 'application'; interface McpProfileConfig { - enabled?: boolean; mountPath?: string; } @@ -44,8 +45,8 @@ const DEFAULT_MOUNT_PATH = '/mcp'; */ export function registerMcpProfile({ profile, host, config, routeOptions }: RegisterMcpProfileArgs): void { const profileConfig = config?.mcp?.[profile]; - if (!profileConfig?.enabled) { - harperLogger.trace(`MCP ${profile} profile disabled, skipping registration`); + if (!profileConfig) { + harperLogger.trace(`MCP ${profile} profile not configured, skipping registration`); return; } diff --git a/server/operationsServer.ts b/server/operationsServer.ts index 695ddca93..6f03b7c99 100644 --- a/server/operationsServer.ts +++ b/server/operationsServer.ts @@ -182,19 +182,15 @@ function buildServer(isHttps: boolean, resources: Resources): FastifyInstance { }); registerContentHandlers(app); - const mcpOperationsEnabled = env.get(terms.CONFIG_PARAMS.MCP_OPERATIONS_ENABLED); - if (!commonUtils.isEmpty(mcpOperationsEnabled) && mcpOperationsEnabled.toString().toLowerCase() === 'true') { + const mcpOperationsMountPath = env.get(terms.CONFIG_PARAMS.MCP_OPERATIONS_MOUNTPATH); + if (!commonUtils.isEmpty(mcpOperationsMountPath)) { + // Presence of mountPath (always defaulted by Joi when the mcp.operations + // block is configured) gates the registration — Harper's `replication`- + // style convention rather than an explicit `enabled` field. registerMcpProfile({ profile: 'operations', host: app, - config: { - mcp: { - operations: { - enabled: true, - mountPath: env.get(terms.CONFIG_PARAMS.MCP_OPERATIONS_MOUNTPATH), - }, - }, - }, + config: { mcp: { operations: { mountPath: mcpOperationsMountPath } } }, routeOptions: { preValidation: [authHandler] }, }); } diff --git a/static/defaultConfig.yaml b/static/defaultConfig.yaml index ce47d230e..926d32320 100644 --- a/static/defaultConfig.yaml +++ b/static/defaultConfig.yaml @@ -77,37 +77,3 @@ tls: privateKey: null node: hostname: null -mcp: - operations: - enabled: false - mountPath: /mcp - allow: - - describe_* - - list_* - - search_* - - get_* - - system_information - - read_log - - read_audit_log - deny: [] - maxTools: 200 - rateLimit: - perToolPerSecond: 10 - perToolBurst: 20 - sessionConcurrency: 25 - sessionPerSecond: 100 - application: - enabled: false - mountPath: /mcp - allow: [] - deny: [] - maxTools: 500 - searchMaxResults: 100 - rateLimit: - perToolPerSecond: 25 - perToolBurst: 50 - sessionConcurrency: 50 - sessionPerSecond: 200 - session: - idleTimeoutSeconds: 1800 - allowClientDelete: true diff --git a/unitTests/components/mcp/index.test.js b/unitTests/components/mcp/index.test.js index c16827bc1..c244c0656 100644 --- a/unitTests/components/mcp/index.test.js +++ b/unitTests/components/mcp/index.test.js @@ -38,28 +38,28 @@ function makeFakeReply() { describe('components/mcp/index', () => { describe('registerMcpProfile', () => { - it('does nothing when the profile is disabled', () => { + it('does nothing when the mcp config block is absent', () => { const host = makeFakeFastify(); - registerMcpProfile({ - profile: 'operations', - host, - config: { mcp: { operations: { enabled: false } } }, - }); + registerMcpProfile({ profile: 'operations', host, config: {} }); assert.equal(host.calls.length, 0); }); - it('does nothing when the mcp config block is absent', () => { + it('does nothing when the profile sub-block is absent under mcp', () => { const host = makeFakeFastify(); - registerMcpProfile({ profile: 'operations', host, config: {} }); + registerMcpProfile({ + profile: 'operations', + host, + config: { mcp: { application: { mountPath: '/x' } } }, + }); assert.equal(host.calls.length, 0); }); - it('registers POST /mcp when operations profile is enabled with defaults', () => { + it('registers POST /mcp when the operations profile block is present', () => { const host = makeFakeFastify(); registerMcpProfile({ profile: 'operations', host, - config: { mcp: { operations: { enabled: true } } }, + config: { mcp: { operations: {} } }, }); assert.equal(host.calls.length, 1); assert.equal(host.calls[0].path, '/mcp'); @@ -71,7 +71,7 @@ describe('components/mcp/index', () => { registerMcpProfile({ profile: 'application', host, - config: { mcp: { application: { enabled: true, mountPath: '/agent' } } }, + config: { mcp: { application: { mountPath: '/agent' } } }, }); assert.equal(host.calls.length, 1); assert.equal(host.calls[0].path, '/agent'); @@ -83,26 +83,13 @@ describe('components/mcp/index', () => { registerMcpProfile({ profile: 'operations', host, - config: { mcp: { operations: { enabled: true } } }, + config: { mcp: { operations: {} } }, routeOptions: sentinel, }); assert.equal(host.calls.length, 1); assert.deepEqual(host.calls[0].options, sentinel); assert.equal(typeof host.calls[0].handler, 'function'); }); - - it('treats only strict-boolean enabled:true as enabled (no string truthiness)', () => { - // Regression: env-sourced configs can deliver the literal string 'false', - // which is truthy in JS. The caller is responsible for coercing — this test - // just pins the component contract that `enabled` is read as-is. - const host = makeFakeFastify(); - registerMcpProfile({ - profile: 'operations', - host, - config: { mcp: { operations: { enabled: 0 } } }, - }); - assert.equal(host.calls.length, 0); - }); }); describe('stub handler', () => { diff --git a/unitTests/validation/configValidator.test.js b/unitTests/validation/configValidator.test.js index 6b9452685..92b35d9b8 100644 --- a/unitTests/validation/configValidator.test.js +++ b/unitTests/validation/configValidator.test.js @@ -387,25 +387,24 @@ describe('Test configValidator module', () => { }); describe('mcp config', () => { - it('validates clean when mcp block is absent', () => { + it('validates clean when the mcp block is absent (profile off)', () => { const result = configValidator(testUtils.deepClone(FAKE_CONFIG), true); expect(result.error).to.be.undefined; + expect(result.value.mcp).to.be.undefined; }); - it('validates clean and applies defaults when only mcp.operations.enabled is given', () => { + it('applies the default mountPath when mcp.operations is present but empty', () => { const config = testUtils.deepClone(FAKE_CONFIG); - config.mcp = { operations: { enabled: false } }; + config.mcp = { operations: {} }; const result = configValidator(config, true); expect(result.error).to.be.undefined; - expect(result.value.mcp.operations.enabled).to.equal(false); expect(result.value.mcp.operations.mountPath).to.equal('/mcp'); }); - it('validates clean when the full mcp block from defaults is supplied', () => { + it('validates clean when both profile blocks are supplied with full keys', () => { const config = testUtils.deepClone(FAKE_CONFIG); config.mcp = { operations: { - enabled: false, mountPath: '/mcp', allow: ['describe_*', 'list_*'], deny: [], @@ -418,7 +417,6 @@ describe('Test configValidator module', () => { }, }, application: { - enabled: false, mountPath: '/mcp', allow: [], deny: [], @@ -440,20 +438,20 @@ describe('Test configValidator module', () => { expect(result.error).to.be.undefined; }); - it('rejects mcp.operations.enabled with a non-boolean', () => { + it('rejects mcp.operations.mountPath with a non-string', () => { const config = testUtils.deepClone(FAKE_CONFIG); - config.mcp = { operations: { enabled: 'yes' } }; + config.mcp = { operations: { mountPath: 42 } }; const result = configValidator(config, true); expect(result.error).to.not.be.undefined; - expect(result.error.message).to.include("'mcp.operations.enabled' must be a boolean"); + expect(result.error.message).to.include("'mcp.operations.mountPath' must be a string"); }); - it('rejects mcp.operations.mountPath with a non-string', () => { + it('rejects mcp.operations.maxTools below 1', () => { const config = testUtils.deepClone(FAKE_CONFIG); - config.mcp = { operations: { mountPath: 42 } }; + config.mcp = { operations: { maxTools: 0 } }; const result = configValidator(config, true); expect(result.error).to.not.be.undefined; - expect(result.error.message).to.include("'mcp.operations.mountPath' must be a string"); + expect(result.error.message).to.include("'mcp.operations.maxTools' must be greater than or equal to 1"); }); }); }); diff --git a/utility/hdbTerms.ts b/utility/hdbTerms.ts index a3355b71b..d33992e43 100644 --- a/utility/hdbTerms.ts +++ b/utility/hdbTerms.ts @@ -523,7 +523,6 @@ export const CONFIG_PARAMS = { OPERATIONSAPI_NETWORK_TIMEOUT: 'operationsApi_network_timeout', OPERATIONSAPI_SYSINFO_NETWORK: 'operationsApi_sysInfo_network', OPERATIONSAPI_SYSINFO_DISK: 'operationsApi_sysInfo_disk', - MCP_OPERATIONS_ENABLED: 'mcp_operations_enabled', MCP_OPERATIONS_MOUNTPATH: 'mcp_operations_mountPath', MCP_OPERATIONS_ALLOW: 'mcp_operations_allow', MCP_OPERATIONS_DENY: 'mcp_operations_deny', @@ -532,7 +531,6 @@ export const CONFIG_PARAMS = { MCP_OPERATIONS_RATELIMIT_PERTOOLBURST: 'mcp_operations_rateLimit_perToolBurst', MCP_OPERATIONS_RATELIMIT_SESSIONCONCURRENCY: 'mcp_operations_rateLimit_sessionConcurrency', MCP_OPERATIONS_RATELIMIT_SESSIONPERSECOND: 'mcp_operations_rateLimit_sessionPerSecond', - MCP_APPLICATION_ENABLED: 'mcp_application_enabled', MCP_APPLICATION_MOUNTPATH: 'mcp_application_mountPath', MCP_APPLICATION_ALLOW: 'mcp_application_allow', MCP_APPLICATION_DENY: 'mcp_application_deny', diff --git a/validation/configValidator.ts b/validation/configValidator.ts index ec9214018..027675ded 100644 --- a/validation/configValidator.ts +++ b/validation/configValidator.ts @@ -67,7 +67,8 @@ export function configValidator(configJson, skipFsValidation = false) { }); // MCP — sub-issue #613 lands the config surface ahead of the transport (#614). - // Both profiles default to enabled:false so existing deployments are unchanged on upgrade. + // Presence-based enablement: a profile is on iff its sub-block exists in + // config (same convention as `replication`). No `enabled` field. const mcpRateLimitSchema = Joi.object({ perToolPerSecond: number.min(0).optional(), perToolBurst: number.min(0).optional(), @@ -75,7 +76,6 @@ export function configValidator(configJson, skipFsValidation = false) { sessionPerSecond: number.min(0).optional(), }); const mcpOperationsSchema = Joi.object({ - enabled: boolean.optional().default(false), mountPath: string.optional().default('/mcp'), allow: array.items(string).optional(), deny: array.items(string).optional(), From d5c8ea32362d17b24c6d2c671666fe8e93e9f9f1 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Wed, 20 May 2026 13:43:49 -0600 Subject: [PATCH 4/4] fix(mcp): gate on nested-config block presence via getConfigObj() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit against Harper's config patterns surfaced a real bug: only six hardcoded Joi defaults are re-applied to configDoc in `config/configUtils.js:483-490`, so the Joi `.default('/mcp')` on `mcp.operations.mountPath` never reaches `env.get(...)`. The previous gate `env.get(MCP_OPERATIONS_MOUNTPATH)` would silently return undefined when a user wrote `mcp: { operations: {} }`, leaving MCP off despite configuration. Switched the call site to: - Import `getConfigObj` from `config/configUtils.js` (matches the existing pattern in `bin/run.ts:355`). - Gate on block presence: `if (fullConfig.mcp?.operations)`. - Pass the full merged config object through to `registerMcpProfile` instead of synthesizing a fake `{mcp:{...}}` from individual env keys. Drops the boolean-string-coercion shim and the misleading "mountPath as presence sentinel" comment — the component already falls back to `DEFAULT_MOUNT_PATH = '/mcp'` when mountPath is absent on the nested config. Refs #613. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 --- server/operationsServer.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/server/operationsServer.ts b/server/operationsServer.ts index 6f03b7c99..dee6e530a 100644 --- a/server/operationsServer.ts +++ b/server/operationsServer.ts @@ -25,6 +25,7 @@ import { } from './serverHelpers/serverHandlers.js'; import { registerBunFastifyInstance } from './http.ts'; import { registerContentHandlers } from './serverHelpers/contentTypes.ts'; +import { getConfigObj } from '../config/configUtils.js'; import { registerMcpProfile } from '../components/mcp/index.ts'; import type { OperationFunctionName } from './serverHelpers/serverUtilities.ts'; type ParsedSqlObject = any; @@ -182,15 +183,17 @@ function buildServer(isHttps: boolean, resources: Resources): FastifyInstance { }); registerContentHandlers(app); - const mcpOperationsMountPath = env.get(terms.CONFIG_PARAMS.MCP_OPERATIONS_MOUNTPATH); - if (!commonUtils.isEmpty(mcpOperationsMountPath)) { - // Presence of mountPath (always defaulted by Joi when the mcp.operations - // block is configured) gates the registration — Harper's `replication`- - // style convention rather than an explicit `enabled` field. + // Presence-based enablement (matches Harper's `replication` convention): + // register iff `mcp.operations` is present in the merged config. The + // nested config tree from `getConfigObj()` is used directly here because + // Joi defaults under `mcp` are not propagated to env.get's flat map — + // only six hardcoded defaults are re-applied in configUtils.validateConfig. + const fullConfig = getConfigObj() ?? {}; + if (fullConfig.mcp?.operations) { registerMcpProfile({ profile: 'operations', host: app, - config: { mcp: { operations: { mountPath: mcpOperationsMountPath } } }, + config: fullConfig, routeOptions: { preValidation: [authHandler] }, }); }