From c51df2b79098383b6285c84721e3c76329b91318 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Thu, 2 Jul 2026 21:05:35 -0600 Subject: [PATCH 01/12] Fix loadedFromSource being unobservable on string-id gets on caching tables (#1575) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Mirror loadedFromSource onto the request context so it is observable with plain-id gets (#1571) The cache-disposition flag was only set on the RequestTarget of the get; with a plain string id the static dispatch mints an internal target the caller never sees, so Context.loadedFromSource (declared but never assigned) always read undefined. Now every site that sets the flag on the target mirrors it onto the context via a shared helper. Co-Authored-By: Claude Fable 5 * Set cache disposition on cache hits in the loadAsInstance=false get path and ensureLoaded; document exact stale semantics Review findings (Gemini): the loadAsInstance=false get path and ensureLoaded() set the flag on source fetch but never wrote false on cache hits, leaving a stale true on shared contexts; and with allowStaleWhileRevalidate the flag reads false (the cache-hit overwrite wins over the synchronous true at fetch start), while staleIfError fallbacks read true — jsdoc now states both. Co-Authored-By: Claude Fable 5 * Guard primitive targets in setLoadedFromSource Instance-API gets on loadAsInstance=false tables can pass a primitive id as the target; assigning a property on a primitive throws in strict mode (Codex review). Context mirroring still applies. Co-Authored-By: Claude Fable 5 * refactor(resources): remove dead wasLoadedFromSource() stub (#1576) wasLoadedFromSource() was declared on ResourceInterface and documented as reporting cache disposition, but its only implementation was the base-class stub that always returned undefined — no subclass ever overrode it, and get() returns a plain RecordObject with no instance methods anyway. We've standardized on context.loadedFromSource / target.loadedFromSource (this PR) as the supported way to observe cache disposition, so remove the misleading interface declaration and stub. Non-breaking: the method never returned a meaningful value. The only in-repo callers (OCSP/CRL cert verification) invoke it via `(x as any).?.()` on a plain record, so they were already reading undefined and are unaffected. Docs: HarperFast/documentation#563. Closes #1576. Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Kyle Bernhardy Co-authored-by: Claude Fable 5 Co-authored-by: Kris Zyp --- resources/RequestTarget.ts | 1 + resources/Resource.ts | 4 --- resources/ResourceInterface.ts | 6 ++++- resources/Table.ts | 24 ++++++++++++----- unitTests/resources/caching.test.js | 42 ++++++++++++++++++++++++++++- 5 files changed, 65 insertions(+), 12 deletions(-) diff --git a/resources/RequestTarget.ts b/resources/RequestTarget.ts index 7aac447653..21e68e802d 100644 --- a/resources/RequestTarget.ts +++ b/resources/RequestTarget.ts @@ -56,6 +56,7 @@ export class RequestTarget extends URLSearchParams { declare previousResidency?: string[]; // Action tracking + /** Cache disposition of a get on a caching table; also mirrored onto the request Context. */ declare loadedFromSource?: boolean; declare createdNewId?: string; diff --git a/resources/Resource.ts b/resources/Resource.ts index f7dd908547..4c87569b7c 100644 --- a/resources/Resource.ts +++ b/resources/Resource.ts @@ -68,10 +68,6 @@ export class Resource implements ResourceInterface< return true; // Subclasses should override if needed } - wasLoadedFromSource(): boolean | void { - // Subclasses should override if needed - } - addTo(_property: keyof Record, _value: Record[keyof Record]): void { throw new Error('Not implemented'); } diff --git a/resources/ResourceInterface.ts b/resources/ResourceInterface.ts index 0d776746bf..1c3abdd76e 100644 --- a/resources/ResourceInterface.ts +++ b/resources/ResourceInterface.ts @@ -49,7 +49,6 @@ export interface ResourceInterface subscribe?(request: SubscriptionRequest): AsyncIterable | Promise>; doesExist(): boolean; - wasLoadedFromSource(): boolean | void; getCurrentUser(): User | undefined; } @@ -95,6 +94,11 @@ export interface Context { sourceApply?: boolean; originatingOperation?: OperationFunctionName; previousResidency?: string[]; + /** Cache disposition of the most recent get on a caching table in this context: true if the get + * fetched from source — including when a source error fell back to a stale record (staleIfError); + * false if served from cache — including stale-while-revalidate responses (the source fetch + * continues in the background) and waits on another request's in-flight source fetch. + * Subsequent gets in the same context overwrite it. */ loadedFromSource?: boolean; nodeName?: string; resourceCache?: Map; diff --git a/resources/Table.ts b/resources/Table.ts index d49083cfd1..5ef1c3a5d4 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -727,7 +727,7 @@ export function makeTable(options) { // return 504 (rather than 404) if there is no content and the cache-control header // dictates not to go to source if (!this.doesExist()) throw new ServerError('Entry is not cached', 504); - if (hasSourceGet && target) target.loadedFromSource = false; // mark it as cached + if (hasSourceGet) setLoadedFromSource(target, request, false); // mark it as cached } else if (resourceOptions?.ensureLoaded) { const loadingFromSource = ensureLoadedFromSource( (this.constructor as any).source, @@ -743,7 +743,7 @@ export function makeTable(options) { TableResource._updateResource(this, entry); return this; }); - } else if (hasSourceGet) target.loadedFromSource = false; // mark it as cached + } else if (hasSourceGet) setLoadedFromSource(target, request, false); // mark it as cached } return this; } @@ -776,7 +776,7 @@ export function makeTable(options) { this.#record = entry.value; this.#version = entry.version; }); - } + } else if (hasSourceGet) setLoadedFromSource(undefined, this.getContext(), false); // mark it as cached } // #section: lifecycle-admin static getNewId(): any { @@ -1224,6 +1224,7 @@ export function makeTable(options) { // return 504 (rather than 404) if there is no content and the cache-control header // dictates not to go to source if (!entry?.value) throw new ServerError('Entry is not cached', 504); + if (hasSourceGet) setLoadedFromSource(target, context, false); // mark it as cached } else if (ensureLoaded) { const loadingFromSource = ensureLoadedFromSource( constructor.source, @@ -1236,7 +1237,7 @@ export function makeTable(options) { if (loadingFromSource) { txn?.disregardReadTxn(); // this could take some time, so don't keep the transaction open if possible return loadingFromSource.then((entry) => entry?.value); - } + } else if (hasSourceGet) setLoadedFromSource(target, context, false); // mark it as cached } return entry?.value; }); @@ -4554,6 +4555,17 @@ export function makeTable(options) { } } + function setLoadedFromSource( + target: RequestTarget | undefined, + context: Context | undefined, + loadedFromSource: boolean + ) { + // mirror the flag onto the context: callers that pass a plain id get an internal + // RequestTarget they never see, so the context is their only way to observe cache disposition (#1571) + // target may be a primitive id on instance-API calls, which can't hold the flag + if (target && typeof target === 'object') target.loadedFromSource = loadedFromSource; + if (context) context.loadedFromSource = loadedFromSource; + } function ensureLoadedFromSource(source: typeof TableResource, id, entry, context, resource?, target?) { if (context?.onlyIfCached) { if (!entry?.value) throw new ServerError('Entry is not cached', 504); @@ -4786,7 +4798,7 @@ export function makeTable(options) { whenResolved(getFromSource(source, id, primaryStore.getEntry(id), context, target)); else { // served from cache after waiting for another request to resolve - if (target) target.loadedFromSource = false; + setLoadedFromSource(target, context, false); whenResolved(entry); } }; @@ -4801,7 +4813,7 @@ export function makeTable(options) { }); } // lock acquired — this request will actually load from source - if (target) target.loadedFromSource = true; + setLoadedFromSource(target, context, true); const existingRecord = existingEntry?.value; // it is important to remember that this is _NOT_ part of the current transaction; nothing is changing diff --git a/unitTests/resources/caching.test.js b/unitTests/resources/caching.test.js index 146d9f1f06..1cbaf9cc9f 100644 --- a/unitTests/resources/caching.test.js +++ b/unitTests/resources/caching.test.js @@ -133,6 +133,44 @@ describe('Caching', () => { assert.equal(target23.loadedFromSource, true); }); + it('loadedFromSource is observable on the context with a plain id', async function () { + // with a plain id, the static get dispatch mints an internal RequestTarget the caller + // never sees, so the flag must be mirrored onto the context (#1571) + CachingTable.setTTLExpiration(30); + await CachingTable.invalidate(31); + let context = {}; + let result = await CachingTable.get(31, context); + assert.equal(result.id, 31); + assert.equal(context.loadedFromSource, true); + context = {}; + result = await CachingTable.get(31, context); + assert.equal(result.id, 31); + assert.equal(context.loadedFromSource, false); + context = { onlyIfCached: true }; + result = await CachingTable.get(31, context); + assert.equal(result.id, 31); + assert.equal(context.loadedFromSource, false); + }); + + it('loadedFromSource is observable on the context with loadAsInstance = false', async function () { + const previousLoadAsInstance = CachingTable.loadAsInstance; + try { + CachingTable.loadAsInstance = false; + CachingTable.setTTLExpiration(30); + await CachingTable.invalidate(32); + let context = {}; + let result = await CachingTable.get(32, context); + assert.equal(result.id, 32); + assert.equal(context.loadedFromSource, true); + context = {}; + result = await CachingTable.get(32, context); + assert.equal(result.id, 32); + assert.equal(context.loadedFromSource, false); + } finally { + CachingTable.loadAsInstance = previousLoadAsInstance; + } + }); + it('Cache stampede is handled', async function () { try { CachingTable.setTTLExpiration(0.01); @@ -228,8 +266,10 @@ describe('Caching', () => { events = []; await new Promise((resolve) => setTimeout(resolve, 10)); // should be stale but not evicted - let result = await CachingTableStaleWhileRevalidate.get(23); + const swrContext = {}; + let result = await CachingTableStaleWhileRevalidate.get(23, swrContext); assert(result); // should exist in database even though it is stale + assert.equal(swrContext.loadedFromSource, false); // stale value served from cache while revalidating assert.equal(sourceRequests, 1); // the source request should be started assert.equal(sourceResponses, 0); // the source request should not be completed yet // the source request should be completed From 0eb2d0b95490570c78e5c2ded83bf55541dbddd9 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Mon, 6 Jul 2026 08:17:55 -0600 Subject: [PATCH 02/12] Add MCP custom content resources via static mcpResources (#1609) Component authors can now expose arbitrary text/blob content under author-chosen URIs on the application profile, parallel to mcpTools (#622) and mcpPrompts: entries declare a fixed uri or an RFC-6570-style uriTemplate ({name} one segment, {+name} cross-segment), served by an instance method dispatched on the LIVE registry class, with optional per-parameter completion values and resources list_changed support. Exported-Resource descriptors now list under harper+rest:// (the spec reserves https:// for web-fetchable resources); legacy http(s):// URIs still read and subscribe. Reserved schemes are rejected in mcpResources declarations so authors cannot shadow the built-in surfaces. Also fixes a latent registration gap: component entry loading is asynchronous past the boot awaits, so tableless components' custom mcpTools/mcpPrompts/mcpResources could register after the initial walk with no schema event to refresh it. Resources gains a monotonic registrationVersion and the MCP transport rebuilds lazily per request when the registry moved. Co-Authored-By: Claude Fable 5 --- components/mcp/customResourceRegistry.ts | 213 ++++++++++++++++++ components/mcp/listChanged.ts | 26 ++- components/mcp/resources.ts | 148 ++++++++++-- components/mcp/tools/application.ts | 174 +++++++++++++- components/mcp/transport.ts | 11 + .../fixtures/mcp-custom-resources/config.yaml | 6 + .../mcp-custom-resources/resources.js | 56 +++++ integrationTests/mcp/custom-resources.test.ts | 111 +++++++++ resources/Resources.ts | 11 + .../mcp/customResourceRegistry.test.js | 132 +++++++++++ unitTests/components/mcp/resources.test.js | 208 +++++++++++++++-- .../components/mcp/tools/application.test.js | 143 ++++++++++++ 12 files changed, 1196 insertions(+), 43 deletions(-) create mode 100644 components/mcp/customResourceRegistry.ts create mode 100644 integrationTests/fixtures/mcp-custom-resources/config.yaml create mode 100644 integrationTests/fixtures/mcp-custom-resources/resources.js create mode 100644 integrationTests/mcp/custom-resources.test.ts create mode 100644 unitTests/components/mcp/customResourceRegistry.test.js diff --git a/components/mcp/customResourceRegistry.ts b/components/mcp/customResourceRegistry.ts new file mode 100644 index 0000000000..4e8b9003d3 --- /dev/null +++ b/components/mcp/customResourceRegistry.ts @@ -0,0 +1,213 @@ +/** + * MCP custom-resource registry — component-author content resources (#1609). + * Mirrors `toolRegistry.ts` / `promptRegistry.ts`. + * + * The discovered resource surface (`resources.ts`) exposes Resource *descriptors* + * and `harper://` metadata; it has no way to serve author-defined content (a docs + * page, a rendered report) under an author-chosen URI. Authors publish such + * content via `static mcpResources` on a Resource (see + * `registerCustomMcpResources` in `tools/application.ts`): each entry declares a + * fixed `uri` or an RFC-6570-style `uriTemplate` (`{name}` matches one path + * segment, `{+name}` matches across segments) and a `read` that returns `text` + * or `blob` content per MCP §server/resources (rev 2025-06-18). + * + * Like custom tools (#622), RBAC is delegated to the Resource: entries are + * listed to every authenticated user on the profile, and the author's method + * enforces any access control it needs at read time. + */ +import type { McpProfile } from './transport.ts'; +import type { AuthedUser } from './toolRegistry.ts'; + +/** Context passed to a custom resource `read` (subset of a tool call's context). */ +export interface ResourceReadContext { + user: AuthedUser; + profile: McpProfile; + sessionId?: string; +} + +/** What an author `read` may return; a bare string means text content. */ +export interface CustomResourceContent { + text?: string; + /** Base64-encoded binary content. */ + blob?: string; + mimeType?: string; +} +export type CustomResourceReadResult = string | CustomResourceContent | object; + +export interface CustomResourceDef { + /** Fixed URI — exactly one of `uri` / `uriTemplate` is set. */ + uri?: string; + /** URI template with `{name}` / `{+name}` placeholders. */ + uriTemplate?: string; + name: string; + title?: string; + description?: string; + mimeType?: string; + profile: McpProfile; + /** Author-declared completion candidates per template parameter (#1349 §3.2). */ + completions?: Readonly>>; + read: ( + params: Record, + context: ResourceReadContext + ) => CustomResourceReadResult | Promise; +} + +interface CompiledDef { + def: CustomResourceDef; + /** Present for template entries only. */ + regex?: RegExp; + paramNames?: string[]; +} + +const registry = new Map(); + +/** + * Compile a URI template into a matcher. `{name}` matches a single path + * segment (`[^/]+`); `{+name}` matches across segments (`.+`), mirroring + * RFC 6570 level-2 reserved expansion, which is how MCP clients construct + * URIs from templates. Throws on malformed templates so registration can + * warn-and-skip the entry. + */ +export function compileUriTemplate(template: string): { regex: RegExp; paramNames: string[] } { + const paramNames: string[] = []; + let pattern = ''; + let index = 0; + while (index < template.length) { + const open = template.indexOf('{', index); + if (open === -1) { + pattern += escapeRegex(template.slice(index)); + break; + } + pattern += escapeRegex(template.slice(index, open)); + const close = template.indexOf('}', open); + if (close === -1) throw new Error(`unterminated '{' in uriTemplate: ${template}`); + let name = template.slice(open + 1, close); + const reserved = name.startsWith('+'); + if (reserved) name = name.slice(1); + if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(name)) { + throw new Error(`invalid template parameter '{${template.slice(open + 1, close)}}' in uriTemplate: ${template}`); + } + paramNames.push(name); + pattern += reserved ? '(.+)' : '([^/]+)'; + index = close + 1; + } + if (paramNames.length === 0) throw new Error(`uriTemplate has no parameters (use \`uri\` instead): ${template}`); + return { regex: new RegExp(`^${pattern}$`), paramNames }; +} + +function escapeRegex(literal: string): string { + return literal.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +/** Register a custom resource. Template entries are compiled here; a bad template throws. */ +export function addCustomResource(def: CustomResourceDef): void { + const compiled: CompiledDef = { def }; + if (def.uriTemplate) { + const { regex, paramNames } = compileUriTemplate(def.uriTemplate); + compiled.regex = regex; + compiled.paramNames = paramNames; + } + let list = registry.get(def.profile); + if (!list) { + list = []; + registry.set(def.profile, list); + } + list.push(compiled); +} + +export function clearProfileCustomResources(profile: McpProfile): void { + registry.delete(profile); +} + +/** Snapshot for restore-on-failure during re-registration (see registerApplicationTools). */ +export function snapshotProfileCustomResources(profile: McpProfile): CustomResourceDef[] { + return (registry.get(profile) ?? []).map((c) => c.def); +} + +/** Fixed-URI entries, shaped for `resources/list`. */ +export function listCustomResources( + profile: McpProfile +): Array<{ uri: string; name: string; title?: string; description?: string; mimeType?: string }> { + const out: Array<{ uri: string; name: string; title?: string; description?: string; mimeType?: string }> = []; + for (const { def } of registry.get(profile) ?? []) { + if (!def.uri) continue; + out.push({ + uri: def.uri, + name: def.name, + ...(def.title ? { title: def.title } : {}), + ...(def.description ? { description: def.description } : {}), + ...(def.mimeType ? { mimeType: def.mimeType } : {}), + }); + } + return out; +} + +/** Template entries, shaped for `resources/templates/list`. */ +export function listCustomResourceTemplates( + profile: McpProfile +): Array<{ uriTemplate: string; name: string; title?: string; description?: string; mimeType?: string }> { + const out: Array<{ uriTemplate: string; name: string; title?: string; description?: string; mimeType?: string }> = []; + for (const { def } of registry.get(profile) ?? []) { + if (!def.uriTemplate) continue; + out.push({ + uriTemplate: def.uriTemplate, + name: def.name, + ...(def.title ? { title: def.title } : {}), + ...(def.description ? { description: def.description } : {}), + ...(def.mimeType ? { mimeType: def.mimeType } : {}), + }); + } + return out; +} + +/** + * Match a `resources/read` URI against the profile's custom entries: fixed URIs + * first (exact string match), then templates in registration order. Registered + * custom URIs take precedence over the discovered surface, so this runs before + * the `harper://` / app-resource dispatch in `readResource`. + */ +export function matchCustomResource( + profile: McpProfile, + uri: string +): { def: CustomResourceDef; params: Record } | undefined { + const list = registry.get(profile); + if (!list) return undefined; + for (const { def } of list) { + if (def.uri === uri) return { def, params: {} }; + } + for (const { def, regex, paramNames } of list) { + if (!regex || !paramNames) continue; + const match = regex.exec(uri); + if (!match) continue; + const params: Record = {}; + for (let i = 0; i < paramNames.length; i++) { + params[paramNames[i]] = decodeURIComponentSafe(match[i + 1]); + } + return { def, params }; + } + return undefined; +} + +/** Author-declared completion candidates for a template parameter, if any. */ +export function customResourceCompletionValues( + profile: McpProfile, + uriTemplate: string, + argName: string +): string[] | undefined { + for (const { def } of registry.get(profile) ?? []) { + if (def.uriTemplate !== uriTemplate) continue; + const values = def.completions?.[argName]; + return values ? [...values] : undefined; + } + return undefined; +} + +// Clients may or may not percent-encode template expansions; a stray '%' must +// not turn a read into a URIError. +function decodeURIComponentSafe(value: string): string { + try { + return decodeURIComponent(value); + } catch { + return value; + } +} diff --git a/components/mcp/listChanged.ts b/components/mcp/listChanged.ts index f5c2f10ed2..5625f81596 100644 --- a/components/mcp/listChanged.ts +++ b/components/mcp/listChanged.ts @@ -15,7 +15,7 @@ * nothing. */ import harperLogger from '../../utility/logging/harper_logger.ts'; -import { listResources } from './resources.ts'; +import { listResources, listResourceTemplates } from './resources.ts'; import { type RegisteredSession, forEachSessionByProfile, @@ -106,7 +106,16 @@ function toolsListNames(profile: McpProfile, session: RegisteredSession): Array< function resourcesListUris(profile: McpProfile, session: RegisteredSession): Array<{ uri: string }> { const result = listResources({ user: session.user, profile, limit: MAX_RESOURCES_PAGE }); - return result.resources.map((r) => ({ uri: r.uri })); + const uris = result.resources.map((r) => ({ uri: r.uri })); + // Templates are part of the advertised resource surface too: a rebuild can + // add/remove a custom mcpResources uriTemplate while the fixed-URI set stays + // identical (#1609). Fold them into the diffed snapshot (prefixed so a + // template can't collide with a fixed URI of the same spelling). + const templates = listResourceTemplates(profile, undefined, MAX_RESOURCES_PAGE); + for (const t of templates.resourceTemplates) { + uris.push({ uri: `template:${t.uriTemplate}` }); + } + return uris; } function sameSet( @@ -155,6 +164,19 @@ function maybeNotifyResourcesChanged(record: RegisteredSession): void { } } +/** + * Re-diff every session's visible resource list on a profile and push + * `notifications/resources/list_changed` to the sessions whose list actually + * changed. Called by the application registration after a rebuild so custom + * `mcpResources` additions/removals propagate (#1609); the per-session diff in + * `maybeNotifyResourcesChanged` keeps no-op rebuilds silent. + */ +export function notifyResourcesListChanged(profile: McpProfile): void { + for (const record of snapshotSessions(profile)) { + maybeNotifyResourcesChanged(record); + } +} + /** * Push `notifications/prompts/list_changed` to every session on a profile. * Prompts carry no per-user RBAC (they're generic templates, §3.5), so unlike diff --git a/components/mcp/resources.ts b/components/mcp/resources.ts index 53b6acbafd..80a4bf1729 100644 --- a/components/mcp/resources.ts +++ b/components/mcp/resources.ts @@ -2,21 +2,24 @@ * MCP resources capability — implements `resources/list`, `resources/read`, * and `resources/templates/list` per MCP §server/resources (rev 2025-06-18). * - * Two URI schemes: - * - `https://[:]/` for app-exported Resources. The same - * URL the REST API uses. Resolved **in-process** via - * `Resources.getMatch(path)` — never makes an outbound HTTP request. + * Three URI surfaces: + * - `harper+rest://[:]/` for app-exported Resource + * descriptors (#1609 — the spec reserves https:// for web-fetchable + * resources). Resolved **in-process** via `Resources.getMatch(path)`; + * legacy `http(s)://` URIs from older listings still read/subscribe. * - `harper://...` for synthetic / metadata resources that don't have a * real HTTP endpoint: * harper://about — server version, profile, capabilities * harper://schema/{database}/{table} — Table.attributes (RBAC-filtered at read time) * harper://openapi — OpenAPI 3.0.3 document * harper://operations — ops-profile only; canonical ops list + * - author-chosen custom schemes (e.g. `docs:///...`) for component-declared + * content resources (`static mcpResources`, #1609) — registered in + * customResourceRegistry.ts and matched before the discovered surfaces. * - * Unlike the tool registry, resources aren't *registered* — they're - * *discovered* at request time. Apps register their `Resource` classes - * through Harper's normal flow; this module enumerates the global - * `resources` registry and adds the synthetic URIs that v1 exposes. + * The descriptor/metadata surfaces aren't *registered* — they're *discovered* + * at request time from the global `resources` registry; custom content + * resources are registered by the application-profile walk. * * Security model: list time only checks REST-method presence on the * Resource class. Resource access is determined programmatically (per- @@ -34,6 +37,12 @@ import { CONFIG_PARAMS, OPERATIONS_ENUM } from '../../utility/hdbTerms.ts'; import harperLogger from '../../utility/logging/harper_logger.ts'; import { SERVER_CAPABILITIES, SERVER_INFO, SUPPORTED_PROTOCOL_VERSIONS } from './lifecycle.ts'; import { encodeCursor } from './pagination.ts'; +import { + customResourceCompletionValues, + listCustomResources, + listCustomResourceTemplates, + matchCustomResource, +} from './customResourceRegistry.ts'; import type { McpProfile } from './transport.ts'; // Harper's resource graph (Resources, generateJsonApi, Server) initializes @@ -110,6 +119,13 @@ export interface ResourceTemplate { const HARPER_SCHEME = 'harper:'; const HTTPS_SCHEME = 'https:'; const HTTP_SCHEME = 'http:'; +/** + * Scheme for exported-Resource descriptor URIs. The MCP spec reserves + * `https://` for resources a client can fetch directly from the web; these + * descriptors resolve in-process (RBAC-gated), so they get a custom scheme + * (#1609). `http(s)://` URIs from older listings still read (back-compat). + */ +const HARPER_REST_SCHEME = 'harper+rest:'; const DEFAULT_LIMIT = 200; @@ -219,19 +235,20 @@ export function listResourceTemplates( description: 'Attribute definitions for a Harper table, RBAC-filtered by attribute_permissions', mimeType: 'application/json', }); - const serverHttpURL = guessAppHttpUrlPrefix(); - if (serverHttpURL) { + const appUriPrefix = appResourceUriPrefix(); + if (appUriPrefix) { all.push({ - uriTemplate: `${serverHttpURL}/{resourcePath}`, + uriTemplate: `${appUriPrefix}/{resourcePath}`, name: 'Application resource', - description: - 'A Resource exported on the HTTP port. The URI is the canonical REST URL; resolution is in-process.', + description: 'A Resource exported on the HTTP port, resolved in-process (not fetchable from the web directly).', mimeType: 'application/json', }); // One concrete template per parameterised route, with `{param}` placeholders for its `:param`/`*wildcard` // segments — more discoverable than the generic `{resourcePath}` catch-all above. - for (const template of enumerateParamRouteTemplates(serverHttpURL)) all.push(template); + for (const template of enumerateParamRouteTemplates(appUriPrefix)) all.push(template); } + // Author-registered custom resource templates (#1609). + for (const template of listCustomResourceTemplates('application')) all.push(template); } const start = offset ?? 0; const max = limit && limit > 0 ? limit : DEFAULT_LIMIT; @@ -260,6 +277,8 @@ export interface CompleteResourceArgs { user: AuthedUser; /** Caller's profile — resource templates exist only on `application`. */ profile: McpProfile; + /** The `ref/resource` URI (template) being completed against, when the client sent one. */ + refUri?: string; } /** @@ -277,6 +296,14 @@ export function completeResourceArgument(args: CompleteResourceArgs): Completion const { argument, context, user, profile } = args; if (profile !== 'application') return capCompletion([]); const partial = (argument.value ?? '').toLowerCase(); + // Custom mcpResources templates complete from author-declared values (#1609); + // the ref URI selects which template's declaration applies. + if (args.refUri) { + const values = customResourceCompletionValues(profile, args.refUri, argument.name); + if (values) { + return capCompletion(values.filter((v) => v.toLowerCase().startsWith(partial)).sort()); + } + } let candidates: string[] = []; if (argument.name === 'database') { const dbs = new Set(); @@ -355,7 +382,9 @@ export async function subscribeToResource( } catch { return null; } - if (parsed.protocol !== HTTPS_SCHEME && parsed.protocol !== HTTP_SCHEME) return null; + if (parsed.protocol !== HARPER_REST_SCHEME && parsed.protocol !== HTTPS_SCHEME && parsed.protocol !== HTTP_SCHEME) { + return null; + } const path = parsed.pathname.replace(/^\/+/, ''); let stream: ResourceChangeStream | null; @@ -480,6 +509,14 @@ export interface ReadResourceFail { export async function readResource(args: ReadResourceArgs): Promise { const { uri, user, profile } = args; + + // Author-registered custom resources match first — a registered URI (fixed or + // template) always wins over the discovered surface (#1609). + const custom = matchCustomResource(profile, uri); + if (custom) { + return readCustomResource(uri, custom, user, profile); + } + let parsed: URL; try { parsed = new URL(uri); @@ -490,15 +527,72 @@ export async function readResource(args: ReadResourceArgs): Promise>, + user: AuthedUser, + profile: McpProfile +): Promise { + const { def, params } = custom; + try { + const result = await def.read(params, { user, profile }); + if (typeof result === 'string') { + return { ok: true, contents: [{ uri, mimeType: def.mimeType ?? 'text/plain', text: result }] }; + } + if (result && typeof result === 'object') { + const content = result as { text?: unknown; blob?: unknown; mimeType?: unknown }; + if (typeof content.text === 'string') { + return { + ok: true, + contents: [ + { + uri, + mimeType: typeof content.mimeType === 'string' ? content.mimeType : (def.mimeType ?? 'text/plain'), + text: content.text, + }, + ], + }; + } + if (typeof content.blob === 'string') { + return { + ok: true, + contents: [ + { + uri, + mimeType: + typeof content.mimeType === 'string' ? content.mimeType : (def.mimeType ?? 'application/octet-stream'), + blob: content.blob, + }, + ], + }; + } + return jsonContent(uri, result); + } + return { ok: false, reason: `custom resource '${def.name}' returned no content for: ${uri}` }; + } catch (err) { + // Author read errors surface as read failures (JSON-RPC error at the + // transport), with the raw error only in the server log — same + // sanitization posture as custom tools. + harperLogger.error(`MCP custom resource '${def.name}' read failed for ${uri}:`, err); + return { ok: false, reason: `custom resource '${def.name}' failed to read: ${uri}` }; + } +} + // ─── Enumeration ─────────────────────────────────────────────────────── function enumerate(profile: McpProfile): ResourceDescriptor[] { @@ -541,10 +635,14 @@ function enumerate(profile: McpProfile): ResourceDescriptor[] { }); } - // https://... — one per exported Resource that has class-level REST methods. + // harper+rest://... — one per exported Resource that has class-level REST methods. // Per-record access is decided by each Resource's `allow{Read,...}` predicate // at read time, so the list filter only checks method presence. for (const entry of enumerateAppHttpResources()) out.push(entry); + + // Author-registered custom content resources (#1609) — fixed URIs only; + // templates are listed by `resources/templates/list`. + for (const entry of listCustomResources('application')) out.push(entry); } out.sort((a, b) => (a.uri < b.uri ? -1 : a.uri > b.uri ? 1 : 0)); @@ -576,7 +674,7 @@ function enumerateTableBackedResources(): Array<{ db: string; table: string; des } function enumerateAppHttpResources(): ResourceDescriptor[] { - const prefix = guessAppHttpUrlPrefix(); + const prefix = appResourceUriPrefix(); if (!prefix) return []; const out: ResourceDescriptor[] = []; for (const [path, entry] of getResources()) { @@ -816,6 +914,18 @@ function filterAttributesByPermissions(attributes: any[], attributePermissions: return attributes.filter((a) => !denied.has(a?.name)); } +/** + * URI prefix for exported-Resource descriptors: the app HTTP host/port under + * the `harper+rest:` scheme (#1609). Derived from the HTTP prefix so the + * authority still identifies the instance, but the scheme signals "resolve + * via MCP, not the web" per spec guidance. + */ +function appResourceUriPrefix(): string | undefined { + const httpPrefix = guessAppHttpUrlPrefix(); + if (!httpPrefix) return undefined; + return httpPrefix.replace(/^https?:/, HARPER_REST_SCHEME); +} + function guessAppHttpUrlPrefix(): string | undefined { // Best-effort URL prefix construction. Hostname comes from the server // module post-boot; the port comes from config. In tests where neither diff --git a/components/mcp/tools/application.ts b/components/mcp/tools/application.ts index 03e19f17ce..f16d2a60fb 100644 --- a/components/mcp/tools/application.ts +++ b/components/mcp/tools/application.ts @@ -39,7 +39,15 @@ import { type PromptDef, type PromptGetResult, } from '../promptRegistry.ts'; -import { notifyPromptsListChanged } from '../listChanged.ts'; +import { + addCustomResource, + clearProfileCustomResources, + snapshotProfileCustomResources, + type CustomResourceDef, + type CustomResourceReadResult, + type ResourceReadContext, +} from '../customResourceRegistry.ts'; +import { notifyPromptsListChanged, notifyResourcesListChanged } from '../listChanged.ts'; import { decodeCursor, encodeCursor } from '../pagination.ts'; import { type AttributePermissionEntry, @@ -124,6 +132,28 @@ interface ResourceClassLike { }>; render: (args: Record) => PromptGetResult | Promise; }>; + /** + * Component-author opt-in: publish custom content resources (#1609). Each + * entry exposes a fixed `uri` or an RFC-6570-style `uriTemplate` (`{name}` + * matches one path segment, `{+name}` matches across segments — custom + * schemes like `docs:///{+path}` are fine per the MCP spec). `method` names + * an instance method invoked as `(params, context)` on the LIVE registry + * class at read time (same dispatch as `mcpTools`); it returns a string + * (text content), `{ text }`, `{ blob, mimeType }` (base64 binary), or any + * other object (serialized as JSON). RBAC is delegated to the Resource. + * `completions` optionally declares candidate values per template parameter + * for `completion/complete`. + */ + mcpResources?: ReadonlyArray<{ + uri?: string; + uriTemplate?: string; + name: string; + title?: string; + description?: string; + mimeType?: string; + method: string; + completions?: Readonly>>; + }>; } interface ToolAnnotationsLike { @@ -826,6 +856,110 @@ function registerCustomMcpPrompts(ResourceClass: ResourceClassLike, path: string return count; } +// Warn-once dedup for mcpResources entries missing a description (same telemetry +// pattern as custom tools). Keyed by `${path}:${name}`; test seam resets it. +const _warnedResourceMissingDesc = new Set(); + +export function _resetCustomResourceWarningsForTest(): void { + _warnedResourceMissingDesc.clear(); +} + +/** + * #1609 — Component-author opt-in. Walk `ResourceClass.mcpResources` and + * register each entry as a custom content resource served by `resources/read`. + * Reads dispatch to the named instance method on the LIVE registry class + * (see `liveResource` — same reasoning as custom tools: the exported + * `resources.js` subclass carries the author's access control). Invalid + * entries are skipped with a warn so one bad entry can't take down the + * profile rebuild. + */ +function registerCustomMcpResources(ResourceClass: ResourceClassLike, path: string): number { + const resources = ResourceClass.mcpResources; + if (!Array.isArray(resources) || resources.length === 0) return 0; + let count = 0; + for (const def of resources) { + const hasUri = typeof def?.uri === 'string' && def.uri.length > 0; + const hasTemplate = typeof def?.uriTemplate === 'string' && def.uriTemplate.length > 0; + if (!def?.name || !def?.method || hasUri === hasTemplate) { + harperLogger.warn( + `MCP application profile: skipping invalid mcpResources entry on '${path}' (needs name + method + exactly one of uri/uriTemplate): ${JSON.stringify(def)}` + ); + continue; + } + // Reserved schemes belong to the discovered surface — a custom entry under + // harper:// (or the descriptor/web schemes) would shadow built-ins like + // harper://schema/... for every client of this instance. + const declaredUri = (hasUri ? def.uri : def.uriTemplate) as string; + if (/^(harper|harper\+rest|https?):/i.test(declaredUri)) { + harperLogger.warn( + `MCP application profile: skipping mcpResource '${def.name}' on '${path}': uri scheme is reserved (harper:, harper+rest:, http:, https:); use a custom scheme like docs:///...` + ); + continue; + } + const methodName = def.method; + if (typeof (ResourceClass.prototype as Record)?.[methodName] !== 'function') { + harperLogger.warn( + `MCP application profile: '${path}' declares mcpResource '${def.name}' for method '${methodName}', but no such instance method exists on the prototype` + ); + continue; + } + const dedupKey = `${path}:${def.name}`; + if (!def.description && !_warnedResourceMissingDesc.has(dedupKey)) { + _warnedResourceMissingDesc.add(dedupKey); + harperLogger.warn( + `MCP application: Resource '${path}' exposes mcpResource '${def.name}' without a description. ` + + `Clients surface it to models/users by description; add { description: '...' } to the mcpResources entry.` + ); + } + const registryDef: CustomResourceDef = { + ...(hasUri ? { uri: def.uri } : {}), + ...(hasTemplate ? { uriTemplate: def.uriTemplate } : {}), + name: def.name, + ...(def.title ? { title: def.title } : {}), + ...(def.description ? { description: def.description } : {}), + ...(def.mimeType ? { mimeType: def.mimeType } : {}), + ...(def.completions ? { completions: def.completions } : {}), + profile: 'application', + read: makeCustomResourceReader(path, ResourceClass, methodName), + }; + try { + addCustomResource(registryDef); + count++; + } catch (err) { + // compileUriTemplate rejects malformed templates + harperLogger.warn( + `MCP application profile: skipping mcpResource '${def.name}' on '${path}': ${(err as Error).message}` + ); + } + } + return count; +} + +/** + * Build the read dispatcher for a custom resource. Mirrors + * `makeCustomMethodHandler`: resolve the live class, construct an instance + * with the caller's context, invoke `(params, context)`. Unlike tools, errors + * are NOT wrapped here — `resources/read` surfaces failures as JSON-RPC + * errors, which `readResource` handles. + */ +function makeCustomResourceReader(path: string, capturedClass: ResourceClassLike, methodName: string) { + return async function ( + params: Record, + context: ResourceReadContext + ): Promise { + const ResourceClass = liveResource(path, capturedClass); + const Ctor = ResourceClass as unknown as new (id: unknown, ctx: unknown) => Record; + const instance = new Ctor(undefined, buildContext(context.user)); + const method = instance[methodName] as + | ((p: Record, ctx: ResourceReadContext) => CustomResourceReadResult) + | undefined; + if (typeof method !== 'function') { + throw new Error(`method '${methodName}' is not a function on the constructed Resource`); + } + return await method.call(instance, params ?? {}, context); + }; +} + function makeCustomMethodHandler(toolName: string, path: string, capturedClass: ResourceClassLike, methodName: string) { return async function (args: unknown, context: ToolCallContext): Promise { try { @@ -871,6 +1005,30 @@ function makeCustomMethodHandler(toolName: string, path: string, capturedClass: // Gates `refreshApplicationTools` so schema-change rebuilds only happen when the // application profile is actually enabled. let applicationToolsRegistered = false; +// Resources.registrationVersion at the last walk. Component entry loading is +// asynchronous past the boot awaits (chokidar's initial scan completes after +// loadComponentDirectories resolves), so tableless components with custom +// mcpTools/mcpPrompts/mcpResources can register AFTER the initial walk without +// any schema-change event to trigger a refresh. The transport compares this on +// each request and rebuilds lazily when the registry moved (#1609). +let lastWalkedRegistrationVersion = -1; + +/** + * Lazy freshness gate, called per MCP request on the application profile: if + * the Resources registry changed since the last walk, rebuild. An integer + * compare in the common case. + */ +export function ensureApplicationToolsFresh(): void { + if (!applicationToolsRegistered) return; + const resources = loadResources(); + if (!resources) return; + // Coerce a missing version to 0 on BOTH sides (here and in the walk snapshot): + // an undefined comparison would fail every request and turn the lazy check + // into a synchronous rebuild per call. + const currentVersion = (resources as { registrationVersion?: number }).registrationVersion ?? 0; + if (currentVersion === lastWalkedRegistrationVersion) return; + registerApplicationTools(); +} /** * Rebuild the application tool registry from the CURRENT schema graph. Tools are @@ -896,6 +1054,9 @@ export function registerApplicationTools(): void { return; } applicationToolsRegistered = true; + // Capture BEFORE walking: a registration landing mid-walk bumps the version + // past this snapshot, so the next request's freshness check re-walks. + lastWalkedRegistrationVersion = (resources as { registrationVersion?: number }).registrationVersion ?? 0; // Atomic idempotent rebuild: drop any application tools from a prior pass so a // removed/renamed table doesn't leave a stale tool behind. Snapshot the prior // set first so a throw mid-loop (e.g. a malformed custom tool on a @table) @@ -903,19 +1064,23 @@ export function registerApplicationTools(): void { // change. Registration is synchronous, so no reader observes the gap. const previousTools = snapshotProfileTools('application'); const previousPrompts = snapshotProfilePrompts('application'); + const previousCustomResources = snapshotProfileCustomResources('application'); const previousPromptNames = previousPrompts .map((p) => p.name) .sort() .join(''); clearProfileTools('application'); clearProfilePrompts('application'); + clearProfileCustomResources('application'); try { buildApplicationTools(resources); } catch (err) { clearProfileTools('application'); clearProfilePrompts('application'); + clearProfileCustomResources('application'); for (const def of previousTools) addTool(def); for (const def of previousPrompts) addPrompt(def); + for (const def of previousCustomResources) addCustomResource(def); throw err; } // Tell connected sessions if the prompt set actually changed (added/removed), @@ -927,6 +1092,9 @@ export function registerApplicationTools(): void { if (currentPromptNames !== previousPromptNames) { notifyPromptsListChanged('application'); } + // Custom resources feed resources/list; the notifier diffs each session's + // visible URI set itself, so no-op rebuilds don't spam (#1609). + notifyResourcesListChanged('application'); } function buildApplicationTools(resources: ResourcesRegistry): void { @@ -947,7 +1115,8 @@ function buildApplicationTools(resources: ResourcesRegistry): void { const hasVerbs = verbs.get || verbs.search || verbs.create || verbs.updatePut || verbs.updatePatch || verbs.delete; const hasCustomTools = Array.isArray(ResourceClass?.mcpTools) && ResourceClass.mcpTools.length > 0; const hasCustomPrompts = Array.isArray(ResourceClass?.mcpPrompts) && ResourceClass.mcpPrompts.length > 0; - if (!hasVerbs && !hasCustomTools && !hasCustomPrompts) continue; + const hasCustomResources = Array.isArray(ResourceClass?.mcpResources) && ResourceClass.mcpResources.length > 0; + if (!hasVerbs && !hasCustomTools && !hasCustomPrompts && !hasCustomResources) continue; const databaseName = ResourceClass?.databaseName; const tableName = ResourceClass?.tableName; const suffix = uniqueSuffix(path, databaseName, claimedSuffixes); @@ -966,6 +1135,7 @@ function buildApplicationTools(resources: ResourcesRegistry): void { } toolsRegistered += registerCustomMcpTools(ResourceClass, path); registerCustomMcpPrompts(ResourceClass, path); + registerCustomMcpResources(ResourceClass, path); } harperLogger.info( `MCP application profile: considered ${resourcesConsidered} resource(s), registered ${toolsRegistered} tool(s)` diff --git a/components/mcp/transport.ts b/components/mcp/transport.ts index 9f62e2ff25..efde0b8115 100644 --- a/components/mcp/transport.ts +++ b/components/mcp/transport.ts @@ -38,6 +38,7 @@ import { seedSessionSnapshot } from './listChanged.ts'; import { tryAdmit } from './rateLimit.ts'; import { deleteSession, loadSession, saveSession, touchSession, type McpSessionRecord } from './session.ts'; import { listResources, listResourceTemplates, readResource, completeResourceArgument } from './resources.ts'; +import { ensureApplicationToolsFresh } from './tools/application.ts'; import { getPrompt, listPrompts, completePromptArgument } from './promptRegistry.ts'; import { addResourceSubscription, @@ -115,6 +116,13 @@ export async function handleMcpRequest(request: NormRequest): Promise `- docs:///${p}`) + .join('\n'), + mimeType: 'text/markdown', + }; + } + + async readPage(params) { + const body = PAGES[params.path]; + if (!body) throw new Error(`no such page: ${params.path}`); + return { text: body, mimeType: 'text/markdown' }; + } + + async readLogo() { + // 1x1 transparent PNG + return { + blob: 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg==', + mimeType: 'image/png', + }; + } +} + +DocsPages.mcpResources = [ + { + uri: 'docs:///index', + name: 'docs index', + description: 'List of all documentation pages', + mimeType: 'text/markdown', + method: 'readIndex', + }, + { + uri: 'docs:///logo', + name: 'docs logo', + description: 'Site logo (binary content)', + method: 'readLogo', + }, + { + uriTemplate: 'docs:///{+path}', + name: 'docs page', + description: 'A documentation page by path', + mimeType: 'text/markdown', + method: 'readPage', + completions: { path: ['guides/install.md', 'guides/deploy.md', 'reference/config.md'] }, + }, +]; diff --git a/integrationTests/mcp/custom-resources.test.ts b/integrationTests/mcp/custom-resources.test.ts new file mode 100644 index 0000000000..b9af407c56 --- /dev/null +++ b/integrationTests/mcp/custom-resources.test.ts @@ -0,0 +1,111 @@ +/** + * #1609 — MCP application-profile custom content resources. The fixture + * component publishes a docs-style surface via `static mcpResources`: a fixed + * text index, a fixed binary logo, and a `docs:///{+path}` template with + * author-declared completions. Also asserts the exported-Resource descriptor + * scheme change: descriptors list under `harper+rest://` and legacy `https://` + * URIs still read. + * + * Reproduction: + * npm run test:integration -- "integrationTests/mcp/custom-resources.test.ts" + */ +import { suite, test, before, after } from 'node:test'; +import { deepStrictEqual, ok, strictEqual } from 'node:assert/strict'; +import { resolve } from 'node:path'; +import { setupHarperWithFixture, teardownHarper, type ContextWithHarper } from '@harperfast/integration-testing'; + +const FIXTURE_PATH = resolve(import.meta.dirname, '../fixtures/mcp-custom-resources'); + +function basicAuth(username: string, password: string): string { + return `Basic ${Buffer.from(`${username}:${password}`).toString('base64')}`; +} + +suite('MCP custom content resources (#1609)', (ctx: ContextWithHarper) => { + let auth: string; + let sessionId: string | undefined; + let rpcId = 0; + + async function rpc(method: string, params?: unknown): Promise { + const res = await fetch(new URL('/mcp', ctx.harper.httpURL), { + method: 'POST', + headers: { + 'content-type': 'application/json', + 'accept': 'application/json, text/event-stream', + 'authorization': auth, + ...(sessionId ? { 'mcp-session-id': sessionId, 'mcp-protocol-version': '2025-06-18' } : {}), + }, + body: JSON.stringify({ jsonrpc: '2.0', id: ++rpcId, method, params: params ?? {} }), + }); + sessionId = res.headers.get('mcp-session-id') ?? sessionId; + const text = await res.text(); + strictEqual(res.status, 200, `${method} should 200: ${text}`); + return JSON.parse(text); + } + + before(async () => { + await setupHarperWithFixture(ctx, FIXTURE_PATH, { + config: { mcp: { application: { mountPath: '/mcp' } } }, + }); + auth = basicAuth(ctx.harper.admin.username, ctx.harper.admin.password); + await rpc('initialize', { + protocolVersion: '2025-06-18', + capabilities: {}, + clientInfo: { name: 'custom-resources-it', version: '0' }, + }); + }); + + after(async () => { + await teardownHarper(ctx); + }); + + test('resources/list includes the fixed custom URIs and harper+rest:// descriptors only', async () => { + const { result } = await rpc('resources/list', {}); + const uris: string[] = result.resources.map((r: any) => r.uri); + ok(uris.includes('docs:///index'), `docs:///index listed: ${JSON.stringify(uris)}`); + ok(uris.includes('docs:///logo'), 'docs:///logo listed'); + ok( + uris.every((u) => !u.startsWith('https://') && !u.startsWith('http://')), + `no web-scheme descriptors remain: ${JSON.stringify(uris.filter((u) => u.startsWith('http')))}` + ); + }); + + test('resources/templates/list includes the custom template', async () => { + const { result } = await rpc('resources/templates/list', {}); + const templates: string[] = result.resourceTemplates.map((t: any) => t.uriTemplate); + ok(templates.includes('docs:///{+path}'), `template listed: ${JSON.stringify(templates)}`); + }); + + test('resources/read returns text content for the fixed index', async () => { + const { result } = await rpc('resources/read', { uri: 'docs:///index' }); + strictEqual(result.contents[0].uri, 'docs:///index'); + strictEqual(result.contents[0].mimeType, 'text/markdown'); + ok(result.contents[0].text.includes('docs:///guides/install.md')); + }); + + test('resources/read resolves a template URI across path segments', async () => { + const { result } = await rpc('resources/read', { uri: 'docs:///guides/install.md' }); + strictEqual(result.contents[0].mimeType, 'text/markdown'); + ok(result.contents[0].text.startsWith('# Install')); + }); + + test('resources/read returns blob content for binary resources', async () => { + const { result } = await rpc('resources/read', { uri: 'docs:///logo' }); + strictEqual(result.contents[0].mimeType, 'image/png'); + ok(typeof result.contents[0].blob === 'string' && result.contents[0].blob.length > 0); + strictEqual(result.contents[0].text, undefined); + }); + + test('an author read error surfaces as a JSON-RPC error, not a 500', async () => { + const body = await rpc('resources/read', { uri: 'docs:///no/such/page.md' }); + ok(body.error, 'JSON-RPC error returned'); + ok(!JSON.stringify(body.error).includes('no such page'), 'raw author error text does not leak'); + }); + + test('completion/complete serves author-declared template values', async () => { + const { result } = await rpc('completion/complete', { + ref: { type: 'ref/resource', uri: 'docs:///{+path}' }, + argument: { name: 'path', value: 'guides/' }, + }); + deepStrictEqual(result.completion.values.sort(), ['guides/deploy.md', 'guides/install.md']); + }); +}); diff --git a/resources/Resources.ts b/resources/Resources.ts index 0231f0a947..55fb786052 100644 --- a/resources/Resources.ts +++ b/resources/Resources.ts @@ -110,6 +110,15 @@ export class Resources extends Map { allTypes: Map = new Map(); + /** + * Monotonic registration version, bumped on every set/delete. Consumers that + * derive surfaces from a registry walk (the MCP tool/prompt/resource + * registries) compare this to the version they last walked and rebuild + * lazily — component entry loading is asynchronous past the boot awaits, so + * there is no reliable "all resources registered" moment to hook (#1609). + */ + registrationVersion = 0; + /** * Parameterised routes (paths containing `:param` or `*wildcard` segments). These are kept out of the base Map so * the exact/prefix matching fast path is untouched; they are only consulted by {@link getMatch} when no static @@ -120,6 +129,7 @@ export class Resources extends Map { // @ts-expect-error override with different signature set(path: string, resource: any, exportTypes?: { [key: string]: boolean }, force?: boolean): void { if (!resource) throw new Error('Must provide a resource'); + this.registrationVersion++; if (path.startsWith('/')) path = path.replace(/^\/+/, ''); const entry = { Resource: resource, @@ -164,6 +174,7 @@ export class Resources extends Map { // it in sync — otherwise a removed/cleared route would keep matching against an unloaded Resource class. delete(path: string): boolean { if (path.startsWith('/')) path = path.replace(/^\/+/, ''); + this.registrationVersion++; const mapDeleted = super.delete(path); const pattern = path.endsWith('/') ? path.replace(/\/+$/, '') : path; // patterns are stored trailing-slash-free const before = this.paramRoutes.length; diff --git a/unitTests/components/mcp/customResourceRegistry.test.js b/unitTests/components/mcp/customResourceRegistry.test.js new file mode 100644 index 0000000000..ed658d9bbe --- /dev/null +++ b/unitTests/components/mcp/customResourceRegistry.test.js @@ -0,0 +1,132 @@ +const assert = require('node:assert/strict'); +const { + addCustomResource, + clearProfileCustomResources, + snapshotProfileCustomResources, + listCustomResources, + listCustomResourceTemplates, + matchCustomResource, + customResourceCompletionValues, + compileUriTemplate, +} = require('#src/components/mcp/customResourceRegistry'); + +const read = async () => 'content'; + +describe('mcp/customResourceRegistry (#1609)', () => { + afterEach(() => { + clearProfileCustomResources('application'); + }); + + describe('compileUriTemplate', () => { + it('matches {name} within a single segment only', () => { + const { regex, paramNames } = compileUriTemplate('docs:///{section}/index'); + assert.deepEqual(paramNames, ['section']); + assert.ok(regex.test('docs:///guides/index')); + assert.ok(!regex.test('docs:///guides/nested/index')); + }); + + it('matches {+name} across segments (reserved expansion)', () => { + const { regex, paramNames } = compileUriTemplate('docs:///{+path}'); + assert.deepEqual(paramNames, ['path']); + assert.ok(regex.test('docs:///getting-started/install.md')); + }); + + it('escapes regex metacharacters in literal parts', () => { + const { regex } = compileUriTemplate('notes+v2://a.b/{id}'); + assert.ok(regex.test('notes+v2://a.b/42')); + assert.ok(!regex.test('notesXv2://aXb/42')); + }); + + it('throws on unterminated braces, invalid names, and parameterless templates', () => { + assert.throws(() => compileUriTemplate('docs:///{path')); + assert.throws(() => compileUriTemplate('docs:///{bad-name}')); + assert.throws(() => compileUriTemplate('docs:///static')); + }); + }); + + describe('matchCustomResource', () => { + it('fixed URIs match exactly and win over templates', () => { + addCustomResource({ uriTemplate: 'docs:///{+path}', name: 'page', profile: 'application', read }); + addCustomResource({ uri: 'docs:///index', name: 'index', profile: 'application', read }); + const match = matchCustomResource('application', 'docs:///index'); + assert.equal(match.def.name, 'index'); + assert.deepEqual(match.params, {}); + }); + + it('template match extracts and decodes params', () => { + addCustomResource({ uriTemplate: 'docs:///{+path}', name: 'page', profile: 'application', read }); + const match = matchCustomResource('application', 'docs:///guides/getting%20started.md'); + assert.equal(match.def.name, 'page'); + assert.deepEqual(match.params, { path: 'guides/getting started.md' }); + }); + + it('a stray percent in the URI does not throw (URIError safety)', () => { + addCustomResource({ uriTemplate: 'docs:///{+path}', name: 'page', profile: 'application', read }); + const match = matchCustomResource('application', 'docs:///100%valid'); + assert.deepEqual(match.params, { path: '100%valid' }); + }); + + it('returns undefined for unknown URIs and other profiles', () => { + addCustomResource({ uri: 'docs:///index', name: 'index', profile: 'application', read }); + assert.equal(matchCustomResource('application', 'docs:///missing'), undefined); + assert.equal(matchCustomResource('operations', 'docs:///index'), undefined); + }); + }); + + describe('listing', () => { + it('separates fixed URIs (resources/list) from templates (templates/list)', () => { + addCustomResource({ + uri: 'docs:///index', + name: 'index', + title: 'Docs index', + description: 'All pages', + mimeType: 'text/markdown', + profile: 'application', + read, + }); + addCustomResource({ uriTemplate: 'docs:///{+path}', name: 'page', profile: 'application', read }); + const fixed = listCustomResources('application'); + assert.deepEqual(fixed, [ + { + uri: 'docs:///index', + name: 'index', + title: 'Docs index', + description: 'All pages', + mimeType: 'text/markdown', + }, + ]); + const templates = listCustomResourceTemplates('application'); + assert.equal(templates.length, 1); + assert.equal(templates[0].uriTemplate, 'docs:///{+path}'); + }); + }); + + describe('completions', () => { + it('returns author-declared values for a template param, undefined otherwise', () => { + addCustomResource({ + uriTemplate: 'docs:///{section}/{page}', + name: 'page', + profile: 'application', + completions: { section: ['guides', 'reference'] }, + read, + }); + assert.deepEqual(customResourceCompletionValues('application', 'docs:///{section}/{page}', 'section'), [ + 'guides', + 'reference', + ]); + assert.equal(customResourceCompletionValues('application', 'docs:///{section}/{page}', 'page'), undefined); + assert.equal(customResourceCompletionValues('application', 'docs:///{other}', 'section'), undefined); + }); + }); + + describe('snapshot / clear (rebuild support)', () => { + it('snapshot returns defs that re-register cleanly after a clear', () => { + addCustomResource({ uri: 'docs:///index', name: 'index', profile: 'application', read }); + const snapshot = snapshotProfileCustomResources('application'); + clearProfileCustomResources('application'); + assert.equal(matchCustomResource('application', 'docs:///index'), undefined); + for (const def of snapshot) addCustomResource(def); + assert.ok(matchCustomResource('application', 'docs:///index')); + }); + }); +}); diff --git a/unitTests/components/mcp/resources.test.js b/unitTests/components/mcp/resources.test.js index fcbacedea7..3f18f2b139 100644 --- a/unitTests/components/mcp/resources.test.js +++ b/unitTests/components/mcp/resources.test.js @@ -309,8 +309,8 @@ describe('mcp/resources', () => { _setResourcesForTest(map); const uris = listResourceTemplates('application').resourceTemplates.map((t) => t.uriTemplate); - assert.ok(uris.includes('https://app.test:9926/widget/{id}/action/{action}')); - assert.ok(uris.includes('https://app.test:9926/files/{rest}')); + assert.ok(uris.includes('harper+rest://app.test:9926/widget/{id}/action/{action}')); + assert.ok(uris.includes('harper+rest://app.test:9926/files/{rest}')); }); it('honors exportTypes.mcp === false, @hidden, and verb presence', () => { @@ -329,7 +329,7 @@ describe('mcp/resources', () => { _setResourcesForTest(map); const uris = listResourceTemplates('application').resourceTemplates.map((t) => t.uriTemplate); - assert.ok(uris.includes('https://app.test:9926/ok/{id}')); + assert.ok(uris.includes('harper+rest://app.test:9926/ok/{id}')); assert.ok(!uris.some((u) => u.includes('/mcpoff/'))); assert.ok(!uris.some((u) => u.includes('/hidden/'))); assert.ok(!uris.some((u) => u.includes('/noverbs/'))); @@ -516,7 +516,7 @@ describe('mcp/resources', () => { }); }); - describe('listResources — https:// app Resources (verb-presence gating)', () => { + describe('listResources — harper+rest:// app Resources (verb-presence gating)', () => { beforeEach(() => { // Override the URL prefix so enumerateAppHttpResources actually // emits entries (otherwise it returns [] and the verb filter is @@ -542,22 +542,22 @@ describe('mcp/resources', () => { it('includes Resources whose prototype defines REST verbs', () => { const result = listResources({ user: SUPER, profile: 'application' }); - const httpUris = result.resources.filter((r) => r.uri.startsWith('https://')).map((r) => r.uri); - assert.ok(httpUris.includes('https://app.test:9926/HasVerbs')); + const httpUris = result.resources.filter((r) => r.uri.startsWith('harper+rest://')).map((r) => r.uri); + assert.ok(httpUris.includes('harper+rest://app.test:9926/HasVerbs')); }); it('excludes Resources with no REST verbs on the prototype', () => { const result = listResources({ user: SUPER, profile: 'application' }); - const httpUris = result.resources.filter((r) => r.uri.startsWith('https://')).map((r) => r.uri); - assert.ok(!httpUris.includes('https://app.test:9926/NoVerbs')); + const httpUris = result.resources.filter((r) => r.uri.startsWith('harper+rest://')).map((r) => r.uri); + assert.ok(!httpUris.includes('harper+rest://app.test:9926/NoVerbs')); }); it('lists the same https:// surface for any caller (no list-time RBAC)', () => { const sup = listResources({ user: SUPER, profile: 'application' }).resources.filter((r) => - r.uri.startsWith('https://') + r.uri.startsWith('harper+rest://') ); const nob = listResources({ user: NOBODY, profile: 'application' }).resources.filter((r) => - r.uri.startsWith('https://') + r.uri.startsWith('harper+rest://') ); assert.deepEqual( sup.map((r) => r.uri), @@ -574,7 +574,7 @@ describe('mcp/resources', () => { _setHttpUrlPrefixForTest(undefined); }); - it('skips Resources with exportTypes.mcp === false from both harper:// schema and https:// enumeration', () => { + it('skips Resources with exportTypes.mcp === false from both harper:// schema and harper+rest:// enumeration', () => { const Public = makeTableResource({ databaseName: 'data', tableName: 'public' }); const Hidden = makeTableResource({ databaseName: 'data', tableName: 'hidden' }); const map = new Map([ @@ -592,9 +592,9 @@ describe('mcp/resources', () => { }; _setResourcesForTest(map); const result = listResources({ user: SUPER, profile: 'application' }); - const httpUris = result.resources.filter((r) => r.uri.startsWith('https://')).map((r) => r.uri); + const httpUris = result.resources.filter((r) => r.uri.startsWith('harper+rest://')).map((r) => r.uri); const schemaUris = result.resources.filter((r) => r.uri.startsWith('harper://schema/')).map((r) => r.uri); - assert.ok(httpUris.includes('https://app.test:9926/Public')); + assert.ok(httpUris.includes('harper+rest://app.test:9926/Public')); assert.ok(!httpUris.some((u) => u.endsWith('/Hidden'))); assert.ok(schemaUris.includes('harper://schema/data/public')); assert.ok(!schemaUris.includes('harper://schema/data/hidden')); @@ -616,9 +616,9 @@ describe('mcp/resources', () => { }; _setResourcesForTest(map); const result = listResources({ user: SUPER, profile: 'application' }); - const httpUris = result.resources.filter((r) => r.uri.startsWith('https://')).map((r) => r.uri); + const httpUris = result.resources.filter((r) => r.uri.startsWith('harper+rest://')).map((r) => r.uri); const schemaUris = result.resources.filter((r) => r.uri.startsWith('harper://schema/')).map((r) => r.uri); - assert.ok(httpUris.includes('https://app.test:9926/NoHttp')); + assert.ok(httpUris.includes('harper+rest://app.test:9926/NoHttp')); assert.ok(schemaUris.includes('harper://schema/data/nohttp')); }); @@ -638,7 +638,7 @@ describe('mcp/resources', () => { }; _setResourcesForTest(map); const res = await readResource({ - uri: 'https://app.test:9926/Hidden', + uri: 'harper+rest://app.test:9926/Hidden', user: SUPER, profile: 'application', }); @@ -668,13 +668,13 @@ describe('mcp/resources', () => { }); describe('enumerate — description prefix + @hidden suppression', () => { - it('prepends ResourceClass.description to https://* resource entries', () => { + it('prepends ResourceClass.description to harper+rest://* resource entries', () => { _setHttpUrlPrefixForTest('https://localhost'); const Product = makeTableResource({ databaseName: 'data', tableName: 'product' }); Product.description = 'Product catalog — what shows up in the storefront listing.'; _setResourcesForTest(makeFakeResources([['Product', Product]])); const { resources } = listResources({ user: SUPER, profile: 'application' }); - const product = resources.find((r) => r.uri === 'https://localhost/Product'); + const product = resources.find((r) => r.uri === 'harper+rest://localhost/Product'); assert.ok(product, 'Product https resource present'); assert.match(product.description, /Product catalog/, 'prefixed with class description'); assert.match(product.description, /Application resource at \/Product/, 'still has the default suffix'); @@ -698,7 +698,7 @@ describe('mcp/resources', () => { _setResourcesForTest(makeFakeResources([['HiddenThing', HiddenThing]])); const { resources } = listResources({ user: SUPER, profile: 'application' }); const uris = resources.map((r) => r.uri); - assert.ok(!uris.includes('https://localhost/HiddenThing'), 'https entry suppressed'); + assert.ok(!uris.includes('harper+rest://localhost/HiddenThing'), 'https entry suppressed'); assert.ok(!uris.includes('harper://schema/data/hidden_thing'), 'schema entry suppressed'); }); @@ -707,7 +707,7 @@ describe('mcp/resources', () => { const Plain = makeTableResource({ databaseName: 'data', tableName: 'plain' }); _setResourcesForTest(makeFakeResources([['Plain', Plain]])); const { resources } = listResources({ user: SUPER, profile: 'application' }); - const plain = resources.find((r) => r.uri === 'https://localhost/Plain'); + const plain = resources.find((r) => r.uri === 'harper+rest://localhost/Plain'); assert.ok(plain); assert.match(plain.description, /^Application resource at \/Plain/, 'no prefix when no class description'); }); @@ -733,3 +733,171 @@ describe('mcp/resources', () => { }); }); }); + +// --------------------------------------------------------------------------- +// Custom mcpResources (#1609) + harper+rest:// descriptor scheme +// --------------------------------------------------------------------------- + +const { addCustomResource, clearProfileCustomResources } = require('#src/components/mcp/customResourceRegistry'); + +describe('custom content resources (#1609)', () => { + afterEach(() => { + clearProfileCustomResources('application'); + _setHttpUrlPrefixForTest(undefined); + _setResourcesForTest(undefined); + }); + + it('fixed custom URIs appear in resources/list', () => { + addCustomResource({ + uri: 'docs:///index', + name: 'docs index', + description: 'All documentation pages', + mimeType: 'text/markdown', + profile: 'application', + read: async () => 'x', + }); + _setResourcesForTest(makeFakeResources([])); + const { resources } = listResources({ user: SUPER, profile: 'application' }); + const entry = resources.find((r) => r.uri === 'docs:///index'); + assert.ok(entry, 'custom fixed URI listed'); + assert.equal(entry.mimeType, 'text/markdown'); + }); + + it('custom templates appear in resources/templates/list', () => { + addCustomResource({ + uriTemplate: 'docs:///{+path}', + name: 'docs page', + profile: 'application', + read: async () => 'x', + }); + _setResourcesForTest(makeFakeResources([])); + const { resourceTemplates } = listResourceTemplates('application'); + assert.ok(resourceTemplates.some((t) => t.uriTemplate === 'docs:///{+path}')); + }); + + it('readResource dispatches a template read with extracted params; string result is text', async () => { + let seenParams, seenContext; + addCustomResource({ + uriTemplate: 'docs:///{+path}', + name: 'docs page', + mimeType: 'text/markdown', + profile: 'application', + read: async (params, context) => { + seenParams = params; + seenContext = context; + return `# Page ${params.path}`; + }, + }); + const res = await readResource({ uri: 'docs:///guides/install.md', user: SUPER, profile: 'application' }); + assert.equal(res.ok, true); + assert.deepEqual(seenParams, { path: 'guides/install.md' }); + assert.equal(seenContext.profile, 'application'); + assert.equal(seenContext.user, SUPER); + assert.equal(res.contents[0].uri, 'docs:///guides/install.md'); + assert.equal(res.contents[0].mimeType, 'text/markdown'); + assert.equal(res.contents[0].text, '# Page guides/install.md'); + }); + + it('blob results pass through with their mimeType', async () => { + addCustomResource({ + uri: 'docs:///logo', + name: 'logo', + profile: 'application', + read: async () => ({ blob: 'aGVsbG8=', mimeType: 'image/png' }), + }); + const res = await readResource({ uri: 'docs:///logo', user: SUPER, profile: 'application' }); + assert.equal(res.ok, true); + assert.equal(res.contents[0].blob, 'aGVsbG8='); + assert.equal(res.contents[0].mimeType, 'image/png'); + assert.equal(res.contents[0].text, undefined); + }); + + it('plain-object results serialize as JSON', async () => { + addCustomResource({ + uri: 'docs:///toc', + name: 'toc', + profile: 'application', + read: async () => ({ pages: ['a', 'b'] }), + }); + const res = await readResource({ uri: 'docs:///toc', user: SUPER, profile: 'application' }); + assert.equal(res.ok, true); + assert.equal(res.contents[0].mimeType, 'application/json'); + assert.deepEqual(JSON.parse(res.contents[0].text), { pages: ['a', 'b'] }); + }); + + it('author read errors surface as a sanitized failure (raw error stays in the log)', async () => { + addCustomResource({ + uri: 'docs:///boom', + name: 'boom', + profile: 'application', + read: async () => { + throw new Error('https://internal.svc key=sk-abc123 exploded'); + }, + }); + const res = await readResource({ uri: 'docs:///boom', user: SUPER, profile: 'application' }); + assert.equal(res.ok, false); + assert.ok(!/sk-abc123/.test(res.reason), 'raw error must not leak'); + assert.match(res.reason, /failed to read/); + }); + + it('completion uses author-declared values selected by refUri', () => { + addCustomResource({ + uriTemplate: 'docs:///{section}/{page}', + name: 'page', + profile: 'application', + completions: { section: ['guides', 'reference', 'release-notes'] }, + read: async () => 'x', + }); + const result = completeResourceArgument({ + argument: { name: 'section', value: 're' }, + user: SUPER, + profile: 'application', + refUri: 'docs:///{section}/{page}', + }); + assert.deepEqual(result.values, ['reference', 'release-notes']); + }); +}); + +describe('harper+rest:// descriptor scheme (#1609)', () => { + afterEach(() => { + _setHttpUrlPrefixForTest(undefined); + _setResourcesForTest(undefined); + }); + + it('exported-Resource descriptors list under harper+rest://', () => { + _setHttpUrlPrefixForTest('https://app.test:9926'); + _setResourcesForTest( + makeFakeResources([['Product', makeTableResource({ databaseName: 'data', tableName: 'product' })]]) + ); + const { resources } = listResources({ user: SUPER, profile: 'application' }); + assert.ok(resources.some((r) => r.uri === 'harper+rest://app.test:9926/Product')); + assert.ok(!resources.some((r) => r.uri.startsWith('https://')), 'no https:// descriptors remain'); + }); + + it('templates list under harper+rest://', () => { + _setHttpUrlPrefixForTest('https://app.test:9926'); + _setResourcesForTest(makeFakeResources([])); + const { resourceTemplates } = listResourceTemplates('application'); + assert.ok(resourceTemplates.some((t) => t.uriTemplate === 'harper+rest://app.test:9926/{resourcePath}')); + }); + + it('reads resolve under both the new scheme and legacy http(s) URIs', async () => { + _setHttpUrlPrefixForTest('https://app.test:9926'); + _setResourcesForTest( + makeFakeResources([['Product', makeTableResource({ databaseName: 'data', tableName: 'product' })]]) + ); + const viaNew = await readResource({ + uri: 'harper+rest://app.test:9926/Product', + user: SUPER, + profile: 'application', + }); + assert.equal(viaNew.ok, true); + assert.equal(JSON.parse(viaNew.contents[0].text).table, 'product'); + const viaLegacy = await readResource({ + uri: 'harper+rest://app.test:9926/Product', + user: SUPER, + profile: 'application', + }); + assert.equal(viaLegacy.ok, true, 'http(s) URIs from older listings must keep reading'); + }); +}); diff --git a/unitTests/components/mcp/tools/application.test.js b/unitTests/components/mcp/tools/application.test.js index da51f12994..7cbdb08bbf 100644 --- a/unitTests/components/mcp/tools/application.test.js +++ b/unitTests/components/mcp/tools/application.test.js @@ -942,3 +942,146 @@ describe('mcp/tools/application — handler dispatch', () => { assert.match(payload.message, /access denied/); }); }); + +// --------------------------------------------------------------------------- +// Custom mcpResources opt-in (#1609) +// --------------------------------------------------------------------------- + +const { + matchCustomResource, + listCustomResources: listCustomResourceDefs, + listCustomResourceTemplates: listCustomResourceTemplateDefs, + clearProfileCustomResources, +} = require('#src/components/mcp/customResourceRegistry'); +const { readResource: readResourceForCustom } = require('#src/components/mcp/resources'); + +describe('mcp/tools/application — custom mcpResources opt-in (#1609)', () => { + beforeEach(() => { + _resetRegistryForTest(); + _setRequestTargetForTest(FakeRequestTarget); + clearProfileCustomResources('application'); + }); + afterEach(() => { + _resetRegistryForTest(); + _setResourcesForTest(undefined); + _setRequestTargetForTest(undefined); + _resetApplicationToolsRegisteredForTest(); + clearProfileCustomResources('application'); + }); + + it('registers fixed and template entries from a static mcpResources declaration', () => { + class Docs { + async readPage() { + return 'x'; + } + } + Docs.mcpResources = [ + { + uri: 'docs:///index', + name: 'docs index', + description: 'All pages', + mimeType: 'text/markdown', + method: 'readPage', + }, + { + uriTemplate: 'docs:///{+path}', + name: 'docs page', + description: 'One page', + mimeType: 'text/markdown', + method: 'readPage', + completions: { path: ['guides/install.md'] }, + }, + ]; + _setResourcesForTest(makeRegistry([['Docs', { Resource: Docs }]])); + registerApplicationTools(); + assert.equal(listCustomResourceDefs('application').length, 1); + assert.equal(listCustomResourceTemplateDefs('application').length, 1); + assert.ok(matchCustomResource('application', 'docs:///index')); + assert.ok(matchCustomResource('application', 'docs:///a/b/c.md')); + }); + + it('read dispatches to the named instance method with template params and read context', async () => { + let captured; + class Docs { + async readPage(params, context) { + captured = { params, profile: context.profile }; + return { text: `page:${params.path}`, mimeType: 'text/markdown' }; + } + } + Docs.mcpResources = [{ uriTemplate: 'docs:///{+path}', name: 'docs page', description: 'd', method: 'readPage' }]; + _setResourcesForTest(makeRegistry([['Docs', { Resource: Docs }]])); + registerApplicationTools(); + const res = await readResourceForCustom({ + uri: 'docs:///guides/install.md', + user: SUPER, + profile: 'application', + }); + assert.equal(res.ok, true); + assert.deepEqual(captured, { params: { path: 'guides/install.md' }, profile: 'application' }); + assert.equal(res.contents[0].text, 'page:guides/install.md'); + assert.equal(res.contents[0].mimeType, 'text/markdown'); + }); + + it('dispatches on the LIVE registry class so a later-registered subclass wins', async () => { + class Base { + async readPage() { + return 'base'; + } + } + Base.mcpResources = [{ uri: 'docs:///index', name: 'docs index', description: 'd', method: 'readPage' }]; + const registry = makeRegistry([['Docs', { Resource: Base }]]); + _setResourcesForTest(registry); + registerApplicationTools(); + class Sub extends Base { + async readPage() { + return 'sub'; + } + } + // component reload swaps the registry entry in place — reads must see Sub + registry.get('Docs').Resource = Sub; + const res = await readResourceForCustom({ uri: 'docs:///index', user: SUPER, profile: 'application' }); + assert.equal(res.contents[0].text, 'sub'); + }); + + it('skips invalid entries: missing method, both/neither of uri+uriTemplate, malformed template', () => { + class Bad { + async ok() { + return 'x'; + } + } + Bad.mcpResources = [ + { uri: 'a:///1', name: 'no-method' }, + { uri: 'a:///2', uriTemplate: 'a:///{x}', name: 'both', method: 'ok' }, + { name: 'neither', method: 'ok' }, + { uriTemplate: 'a:///{bad', name: 'malformed', method: 'ok' }, + { uri: 'a:///5', name: 'missing-fn', method: 'doesNotExist' }, + { uri: 'harper://schema/data/shadow', name: 'reserved-harper', description: 'd', method: 'ok' }, + { uriTemplate: 'https://example.com/{x}', name: 'reserved-web', description: 'd', method: 'ok' }, + { uri: 'a:///good', name: 'good', description: 'd', method: 'ok' }, + ]; + _setResourcesForTest(makeRegistry([['Bad', { Resource: Bad }]])); + registerApplicationTools(); + const fixed = listCustomResourceDefs('application'); + assert.deepEqual( + fixed.map((r) => r.uri), + ['a:///good'] + ); + assert.equal(listCustomResourceTemplateDefs('application').length, 0); + }); + + it('rebuild clears stale custom resources (removed class leaves no entry behind)', () => { + class Docs { + async readPage() { + return 'x'; + } + } + Docs.mcpResources = [{ uri: 'docs:///index', name: 'docs index', description: 'd', method: 'readPage' }]; + _setResourcesForTest(makeRegistry([['Docs', { Resource: Docs }]])); + registerApplicationTools(); + assert.ok(matchCustomResource('application', 'docs:///index')); + _setResourcesForTest(makeRegistry([])); + _resetApplicationToolsRegisteredForTest(); + registerApplicationTools(); + assert.equal(matchCustomResource('application', 'docs:///index'), undefined); + }); +}); From 3af972ed27cc4f19607a8b31e58e32048ec6a8b3 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Mon, 6 Jul 2026 08:28:41 -0600 Subject: [PATCH 03/12] Notify tools/list_changed after lazy rebuilds; require a literal custom scheme Codex final review: the lazy per-request rebuild could change the tool set without emitting notifications/tools/list_changed (only prompts and resources were notified), and the reserved-scheme guard could be bypassed with a parameterized scheme position like {scheme}://{+path}. The notifier per-session diffs, so no-op rebuilds stay silent; schemes must now be literal and non-reserved. Co-Authored-By: Claude Fable 5 --- components/mcp/listChanged.ts | 15 +++++++++++++++ components/mcp/tools/application.ts | 18 ++++++++++++------ .../components/mcp/tools/application.test.js | 2 ++ 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/components/mcp/listChanged.ts b/components/mcp/listChanged.ts index 5625f81596..2b713c8b5a 100644 --- a/components/mcp/listChanged.ts +++ b/components/mcp/listChanged.ts @@ -177,6 +177,21 @@ export function notifyResourcesListChanged(profile: McpProfile): void { } } +/** + * Re-diff every session's visible tool list on a profile and push + * `notifications/tools/list_changed` to the sessions whose list actually + * changed. The schema-change handler already does this, but the lazy + * per-request rebuild (`ensureApplicationToolsFresh`, #1609) can add/remove + * custom `mcpTools` outside any schema event — without this, a session that + * initialized before a tableless component registered keeps a stale tool + * list until it happens to re-poll `tools/list`. + */ +export function notifyToolsListChanged(profile: McpProfile): void { + for (const record of snapshotSessions(profile)) { + maybeNotifyToolsChanged(record); + } +} + /** * Push `notifications/prompts/list_changed` to every session on a profile. * Prompts carry no per-user RBAC (they're generic templates, §3.5), so unlike diff --git a/components/mcp/tools/application.ts b/components/mcp/tools/application.ts index f16d2a60fb..d7756af0ea 100644 --- a/components/mcp/tools/application.ts +++ b/components/mcp/tools/application.ts @@ -47,7 +47,7 @@ import { type CustomResourceReadResult, type ResourceReadContext, } from '../customResourceRegistry.ts'; -import { notifyPromptsListChanged, notifyResourcesListChanged } from '../listChanged.ts'; +import { notifyPromptsListChanged, notifyResourcesListChanged, notifyToolsListChanged } from '../listChanged.ts'; import { decodeCursor, encodeCursor } from '../pagination.ts'; import { type AttributePermissionEntry, @@ -886,13 +886,15 @@ function registerCustomMcpResources(ResourceClass: ResourceClassLike, path: stri ); continue; } - // Reserved schemes belong to the discovered surface — a custom entry under - // harper:// (or the descriptor/web schemes) would shadow built-ins like - // harper://schema/... for every client of this instance. + // Custom entries match BEFORE the discovered surfaces, so the scheme must be + // a LITERAL, non-reserved custom scheme: a reserved scheme (or a template + // whose scheme position contains a parameter, e.g. `{scheme}://...`) could + // shadow built-ins like harper://schema/... for every client of this instance. const declaredUri = (hasUri ? def.uri : def.uriTemplate) as string; - if (/^(harper|harper\+rest|https?):/i.test(declaredUri)) { + const schemeMatch = /^([A-Za-z][A-Za-z0-9+.-]*):/.exec(declaredUri); + if (!schemeMatch || /^(harper|harper\+rest|https?)$/i.test(schemeMatch[1])) { harperLogger.warn( - `MCP application profile: skipping mcpResource '${def.name}' on '${path}': uri scheme is reserved (harper:, harper+rest:, http:, https:); use a custom scheme like docs:///...` + `MCP application profile: skipping mcpResource '${def.name}' on '${path}': the uri must start with a literal custom scheme (harper:, harper+rest:, http:, https: are reserved); use e.g. docs:///...` ); continue; } @@ -1095,6 +1097,10 @@ export function registerApplicationTools(): void { // Custom resources feed resources/list; the notifier diffs each session's // visible URI set itself, so no-op rebuilds don't spam (#1609). notifyResourcesListChanged('application'); + // The lazy per-request rebuild (ensureApplicationToolsFresh) can change the + // tool set outside any schema event; the notifier per-session diffs, so this + // is silent when nothing changed. + notifyToolsListChanged('application'); } function buildApplicationTools(resources: ResourcesRegistry): void { diff --git a/unitTests/components/mcp/tools/application.test.js b/unitTests/components/mcp/tools/application.test.js index 7cbdb08bbf..e30af10cf4 100644 --- a/unitTests/components/mcp/tools/application.test.js +++ b/unitTests/components/mcp/tools/application.test.js @@ -1057,6 +1057,8 @@ describe('mcp/tools/application — custom mcpResources opt-in (#1609)', () => { { uri: 'a:///5', name: 'missing-fn', method: 'doesNotExist' }, { uri: 'harper://schema/data/shadow', name: 'reserved-harper', description: 'd', method: 'ok' }, { uriTemplate: 'https://example.com/{x}', name: 'reserved-web', description: 'd', method: 'ok' }, + { uriTemplate: '{scheme}://{+path}', name: 'param-scheme', description: 'd', method: 'ok' }, + { uriTemplate: 'har{rest}://{+path}', name: 'partial-scheme', description: 'd', method: 'ok' }, { uri: 'a:///good', name: 'good', description: 'd', method: 'ok' }, ]; _setResourcesForTest(makeRegistry([['Bad', { Resource: Bad }]])); From 2c37e9e170f566b1bc551e9a091d6e938fb276f8 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Mon, 6 Jul 2026 08:49:46 -0600 Subject: [PATCH 04/12] Reject duplicate template params; fix back-compat test to actually use a legacy URI Gemini bot review: docs:///{name}/{name} silently overwrote the first captured value; and the scheme back-compat test requested harper+rest for both cases (an overzealous scheme-migration sed), so the legacy http(s) path was untested. Co-Authored-By: Claude Fable 5 --- components/mcp/customResourceRegistry.ts | 4 ++++ unitTests/components/mcp/customResourceRegistry.test.js | 3 ++- unitTests/components/mcp/resources.test.js | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/components/mcp/customResourceRegistry.ts b/components/mcp/customResourceRegistry.ts index 4e8b9003d3..92b09fa8e9 100644 --- a/components/mcp/customResourceRegistry.ts +++ b/components/mcp/customResourceRegistry.ts @@ -87,6 +87,10 @@ export function compileUriTemplate(template: string): { regex: RegExp; paramName if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(name)) { throw new Error(`invalid template parameter '{${template.slice(open + 1, close)}}' in uriTemplate: ${template}`); } + if (paramNames.includes(name)) { + // a repeated name would silently overwrite the earlier captured value + throw new Error(`duplicate template parameter '{${name}}' in uriTemplate: ${template}`); + } paramNames.push(name); pattern += reserved ? '(.+)' : '([^/]+)'; index = close + 1; diff --git a/unitTests/components/mcp/customResourceRegistry.test.js b/unitTests/components/mcp/customResourceRegistry.test.js index ed658d9bbe..86c73d0e0c 100644 --- a/unitTests/components/mcp/customResourceRegistry.test.js +++ b/unitTests/components/mcp/customResourceRegistry.test.js @@ -37,10 +37,11 @@ describe('mcp/customResourceRegistry (#1609)', () => { assert.ok(!regex.test('notesXv2://aXb/42')); }); - it('throws on unterminated braces, invalid names, and parameterless templates', () => { + it('throws on unterminated braces, invalid names, parameterless templates, and duplicate params', () => { assert.throws(() => compileUriTemplate('docs:///{path')); assert.throws(() => compileUriTemplate('docs:///{bad-name}')); assert.throws(() => compileUriTemplate('docs:///static')); + assert.throws(() => compileUriTemplate('docs:///{name}/{name}'), /duplicate template parameter/); }); }); diff --git a/unitTests/components/mcp/resources.test.js b/unitTests/components/mcp/resources.test.js index 3f18f2b139..2d431e20cf 100644 --- a/unitTests/components/mcp/resources.test.js +++ b/unitTests/components/mcp/resources.test.js @@ -894,7 +894,7 @@ describe('harper+rest:// descriptor scheme (#1609)', () => { assert.equal(viaNew.ok, true); assert.equal(JSON.parse(viaNew.contents[0].text).table, 'product'); const viaLegacy = await readResource({ - uri: 'harper+rest://app.test:9926/Product', + uri: 'https://app.test:9926/Product', user: SUPER, profile: 'application', }); From 98a3cb7959228c4bfae506bab6b43b7d91b1c5b3 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Mon, 6 Jul 2026 08:56:58 -0600 Subject: [PATCH 05/12] Fix descriptor authority when the port config uses host:port bind form guessAppHttpUrlPrefix appended the raw HTTP_PORT/HTTP_SECUREPORT config value as if it were a bare port, producing authorities like harper+rest://127.0.0.9:127.0.0.9:9926 when the port was configured as a bind address (host:port). Extract just the port. Pre-existing (legacy https:// descriptors were equally malformed); surfaced by runtime verification of #1609 and rolled into this PR at the user's request. Co-Authored-By: Claude Fable 5 --- components/mcp/resources.ts | 17 +++++++++++++++-- unitTests/components/mcp/resources.test.js | 12 ++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/components/mcp/resources.ts b/components/mcp/resources.ts index 80a4bf1729..ba4c9b64e1 100644 --- a/components/mcp/resources.ts +++ b/components/mcp/resources.ts @@ -950,13 +950,26 @@ function guessAppHttpUrlPrefix(): string | undefined { // Standard deployment: prefer the HTTPS port. Fall back to the plain // HTTP port for dev setups that don't configure TLS. - const securePort = env.get(CONFIG_PARAMS.HTTP_SECUREPORT); + const securePort = normalizePortForUrl(env.get(CONFIG_PARAMS.HTTP_SECUREPORT)); if (securePort) return `https://${hostname}:${securePort}`; - const httpPort = env.get(CONFIG_PARAMS.HTTP_PORT); + const httpPort = normalizePortForUrl(env.get(CONFIG_PARAMS.HTTP_PORT)); if (httpPort) return `http://${hostname}:${httpPort}`; return undefined; } +/** + * Port config accepts a bare port or a `host:port` bind-address form + * (e.g. `--HTTP_PORT=127.0.0.9:9926`); descriptor URLs need only the port — + * appending the raw value produced authorities like `host:host:port` (#1609). + * Exported for unit tests. + */ +export function normalizePortForUrl(value: unknown): string | undefined { + if (value == null || value === '') return undefined; + const str = String(value); + const colon = str.lastIndexOf(':'); + return colon === -1 ? str : str.slice(colon + 1); +} + function jsonContent(uri: string, body: unknown): ReadResourceOk { return { ok: true, diff --git a/unitTests/components/mcp/resources.test.js b/unitTests/components/mcp/resources.test.js index 2d431e20cf..a703c00ccc 100644 --- a/unitTests/components/mcp/resources.test.js +++ b/unitTests/components/mcp/resources.test.js @@ -901,3 +901,15 @@ describe('harper+rest:// descriptor scheme (#1609)', () => { assert.equal(viaLegacy.ok, true, 'http(s) URIs from older listings must keep reading'); }); }); + +describe('normalizePortForUrl (#1609 descriptor authority)', () => { + const { normalizePortForUrl } = require('#src/components/mcp/resources'); + it('passes bare ports through and extracts the port from host:port bind forms', () => { + assert.equal(normalizePortForUrl(9926), '9926'); + assert.equal(normalizePortForUrl('9926'), '9926'); + assert.equal(normalizePortForUrl('127.0.0.9:9926'), '9926'); + assert.equal(normalizePortForUrl('[::1]:9926'), '9926'); + assert.equal(normalizePortForUrl(undefined), undefined); + assert.equal(normalizePortForUrl(''), undefined); + }); +}); From 698993424437f1ad90dd7c37340420d02e4ebac2 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Mon, 6 Jul 2026 09:17:22 -0600 Subject: [PATCH 06/12] Use plain node:assert in the new test file per AGENTS.md house style Co-Authored-By: Claude Fable 5 --- unitTests/components/mcp/customResourceRegistry.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unitTests/components/mcp/customResourceRegistry.test.js b/unitTests/components/mcp/customResourceRegistry.test.js index 86c73d0e0c..e2306673cf 100644 --- a/unitTests/components/mcp/customResourceRegistry.test.js +++ b/unitTests/components/mcp/customResourceRegistry.test.js @@ -1,4 +1,4 @@ -const assert = require('node:assert/strict'); +const assert = require('node:assert'); const { addCustomResource, clearProfileCustomResources, From 0a9083e2bd988f31cf827a9887969500f3433aec Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Mon, 6 Jul 2026 09:38:53 -0600 Subject: [PATCH 07/12] Pin the anonymous public-docs path: no-auth list/read integration case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Custom resources impose no auth of their own (listing is per-profile, reads delegate to the author's method — parity with mcpTools' visibleTo: () => true). The new integration case runs a full no-Authorization-header session end-to-end; the registry doc no longer implies a login gate. Co-Authored-By: Claude Fable 5 --- components/mcp/customResourceRegistry.ts | 6 ++- integrationTests/mcp/custom-resources.test.ts | 38 +++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/components/mcp/customResourceRegistry.ts b/components/mcp/customResourceRegistry.ts index 92b09fa8e9..5ac1ae9fae 100644 --- a/components/mcp/customResourceRegistry.ts +++ b/components/mcp/customResourceRegistry.ts @@ -12,8 +12,10 @@ * or `blob` content per MCP §server/resources (rev 2025-06-18). * * Like custom tools (#622), RBAC is delegated to the Resource: entries are - * listed to every authenticated user on the profile, and the author's method - * enforces any access control it needs at read time. + * listed to every session on the profile — including anonymous/unauthenticated + * sessions where the deployment allows them (the public-docs case #1609 is + * built around) — and the author's method enforces any access control it + * needs at read time. */ import type { McpProfile } from './transport.ts'; import type { AuthedUser } from './toolRegistry.ts'; diff --git a/integrationTests/mcp/custom-resources.test.ts b/integrationTests/mcp/custom-resources.test.ts index b9af407c56..dfbae3a994 100644 --- a/integrationTests/mcp/custom-resources.test.ts +++ b/integrationTests/mcp/custom-resources.test.ts @@ -108,4 +108,42 @@ suite('MCP custom content resources (#1609)', (ctx: ContextWithHarper) => { }); deepStrictEqual(result.completion.values.sort(), ['guides/deploy.md', 'guides/install.md']); }); + + test('custom resources list and read without an Authorization header (#1609 public-docs case)', async () => { + // The custom-resource layer imposes no auth of its own: entries list per + // profile (no per-user filter) and reads delegate access control to the + // author's method — parity with mcpTools' visibleTo: () => true. This + // session sends NO Authorization header end-to-end (the instance's + // authorizeLocal maps the local connection to a user, as a public-docs + // deployment's anonymous mapping would). + let anonSession: string | undefined; + let anonId = 100; + async function anonRpc(method: string, params?: unknown): Promise { + const res = await fetch(new URL('/mcp', ctx.harper.httpURL), { + method: 'POST', + headers: { + 'content-type': 'application/json', + 'accept': 'application/json, text/event-stream', + ...(anonSession ? { 'mcp-session-id': anonSession, 'mcp-protocol-version': '2025-06-18' } : {}), + }, + body: JSON.stringify({ jsonrpc: '2.0', id: ++anonId, method, params: params ?? {} }), + }); + anonSession = res.headers.get('mcp-session-id') ?? anonSession; + const text = await res.text(); + strictEqual(res.status, 200, `${method} without auth should 200: ${text}`); + return JSON.parse(text); + } + await anonRpc('initialize', { + protocolVersion: '2025-06-18', + capabilities: {}, + clientInfo: { name: 'anon-docs-client', version: '0' }, + }); + const list = await anonRpc('resources/list', {}); + ok( + list.result.resources.some((r: any) => r.uri === 'docs:///index'), + 'custom resource listed without auth' + ); + const read = await anonRpc('resources/read', { uri: 'docs:///guides/install.md' }); + ok(read.result.contents[0].text.startsWith('# Install'), 'custom template read served without auth'); + }); }); From 7747af314e02fb2c3b2900014751e077e82a51b6 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Mon, 6 Jul 2026 19:17:37 -0600 Subject: [PATCH 08/12] Add MCP per-client rate limiting and a durable operator quota hook (#1610) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Session-scoped buckets are trivially cycled by an anonymous client (initialize, call, drop session, repeat) and reset on restart — no cost control for public unauthenticated tools. Two additions, both opt-in: - Per-client-identity token buckets (rateLimit.ts): keyed on client identity (socket IP, or the first value of a config-named trusted header for proxied deployments), surviving session cycling. Config perClientPerSecond/perClientBurst per profile, default off; burst floors at one whole token so fractional sustained rates still admit; per-profile config now cached with a 10s TTL to keep the per-call identity resolution off the env-lookup path; startup warning when an identity header is configured (spoofing trap). - Durable quota hook (quota.ts): mcp..quota.resource names an exported Resource whose static method (default allowMcpCall) is called per admitted tools/call with {identity, tool, user, profile, sessionId}; deny with {allowed:false, message?, retryAfterSeconds?} surfacing as kind:'quota_exceeded'. Fail-closed on misconfiguration or hook errors (#1422 precedent); dispatches on the live registry class; runs after the in-memory admit so throttled clients cannot spam a table-backed hook. Stacked on feat/mcp-custom-resources-1609 for the lazy registry freshness (#1613) that makes custom-tool fixtures deterministic. Co-Authored-By: Claude Fable 5 --- components/mcp/adapters/fastify.ts | 3 + components/mcp/adapters/harperHttp.ts | 3 + components/mcp/audit.ts | 2 +- components/mcp/quota.ts | 136 ++++++++++++++ components/mcp/rateLimit.ts | 166 ++++++++++++++++-- components/mcp/transport.ts | 57 +++++- .../fixtures/mcp-quota/config.yaml | 10 ++ .../fixtures/mcp-quota/resources.js | 31 ++++ .../fixtures/mcp-quota/schema.graphql | 5 + integrationTests/mcp/quota.test.ts | 147 ++++++++++++++++ unitTests/components/mcp/quota.test.js | 119 +++++++++++++ unitTests/components/mcp/rateLimit.test.js | 105 ++++++++++- utility/hdbTerms.ts | 10 ++ 13 files changed, 774 insertions(+), 20 deletions(-) create mode 100644 components/mcp/quota.ts create mode 100644 integrationTests/fixtures/mcp-quota/config.yaml create mode 100644 integrationTests/fixtures/mcp-quota/resources.js create mode 100644 integrationTests/fixtures/mcp-quota/schema.graphql create mode 100644 integrationTests/mcp/quota.test.ts create mode 100644 unitTests/components/mcp/quota.test.js diff --git a/components/mcp/adapters/fastify.ts b/components/mcp/adapters/fastify.ts index c2cb400657..33dc06efb0 100644 --- a/components/mcp/adapters/fastify.ts +++ b/components/mcp/adapters/fastify.ts @@ -21,6 +21,8 @@ interface FastifyLikeRequest { method: string; headers: Record; body: unknown; + /** Client socket IP (Fastify's request.ip). */ + ip?: string; /** * `authAndEnsureUserOnRequest` sets the full user (incl. role + permission * tree) on `req.hdb_user`. Used for session binding (`username`) and @@ -53,6 +55,7 @@ export function createFastifyHandler(profile: McpProfile) { user: request.hdb_user?.username ?? '', userObject: (request.hdb_user ?? undefined) as NormRequest['userObject'], profile, + clientIp: request.ip, }; const res = await handleMcpRequest(norm); diff --git a/components/mcp/adapters/harperHttp.ts b/components/mcp/adapters/harperHttp.ts index 9f82f11977..2e19a692db 100644 --- a/components/mcp/adapters/harperHttp.ts +++ b/components/mcp/adapters/harperHttp.ts @@ -38,6 +38,8 @@ interface HarperHttpRequest { */ user?: { username?: string; role?: unknown }; isWebSocket?: boolean; + /** Client socket IP (Harper Request getter; present on Node and Bun). */ + ip?: string; } interface HarperHttpResponse { @@ -61,6 +63,7 @@ export function createHarperHttpHandler(profile: McpProfile) { user: request.user?.username ?? '', userObject: request.user as NormRequest['userObject'], profile, + clientIp: request.ip, }; const res = await handleMcpRequest(norm); diff --git a/components/mcp/audit.ts b/components/mcp/audit.ts index 4e133d859b..314d5657a9 100644 --- a/components/mcp/audit.ts +++ b/components/mcp/audit.ts @@ -19,7 +19,7 @@ export interface AuditEntry { tool: string; user: string; args: object; - status: 'ok' | 'isError' | 'rate_limited' | 'protocol_error'; + status: 'ok' | 'isError' | 'rate_limited' | 'quota_exceeded' | 'protocol_error'; durationMs: number; errorMessage?: string; } diff --git a/components/mcp/quota.ts b/components/mcp/quota.ts new file mode 100644 index 0000000000..f1e9093c5d --- /dev/null +++ b/components/mcp/quota.ts @@ -0,0 +1,136 @@ +/** + * Durable, operator-pluggable quota hook for MCP `tools/call` (#1610). + * + * The in-memory buckets in `rateLimit.ts` bound instantaneous rates but are + * per-worker and reset on restart — insufficient as a COST control for a + * public unauthenticated tool (an LLM-backed `answer`, say). This hook lets + * the operator implement a durable policy (e.g. a persisted per-IP daily + * counter table) behind config: + * + * mcp: + * application: + * quota: + * resource: McpQuota # exported Resource path + * method: allowMcpCall # static method on it (this is the default) + * + * Before each admitted tools/call, Harper calls + * `QuotaClass.allowMcpCall({ identity, tool, user, profile, sessionId })`. + * Return `true` (or any truthy non-object) to allow; return + * `{ allowed: false, message?, retryAfterSeconds? }` to deny — the denial + * surfaces to the client as `isError` with `kind: 'quota_exceeded'`. + * Counting is the hook's business: increment on check, or on success via + * your own bookkeeping — Harper calls once per attempted tool call. + * + * FAIL-CLOSED: a hook that throws (or a configured resource/method that + * doesn't resolve) DENIES the call. Cost protection that silently disables + * itself on a bug is worse than a hard failure (#1422 set this precedent + * for allow* hooks). The raw error goes to the server log only. + * + * Dispatch uses the LIVE registry class, same as custom tools — an exported + * subclass replacing the entry on reload wins. + */ +import * as env from '../../utility/environment/environmentManager.ts'; +import { CONFIG_PARAMS } from '../../utility/hdbTerms.ts'; +import harperLogger from '../../utility/logging/harper_logger.ts'; +import type { McpProfile } from './transport.ts'; +import type { AuthedUser } from './toolRegistry.ts'; + +export interface QuotaCheckInfo { + /** Client identity from `resolveClientIdentity` (socket IP or trusted-header value); may be undefined. */ + identity?: string; + tool: string; + user: AuthedUser; + profile: McpProfile; + sessionId: string; +} + +export interface QuotaDenial { + allowed: false; + /** Shown to the client verbatim — author-controlled, keep it safe. */ + message?: string; + retryAfterSeconds?: number; +} + +export type QuotaDecision = { allowed: true } | QuotaDenial; + +const CONFIG_KEYS: Record = { + operations: { + resource: CONFIG_PARAMS.MCP_OPERATIONS_QUOTA_RESOURCE, + method: CONFIG_PARAMS.MCP_OPERATIONS_QUOTA_METHOD, + }, + application: { + resource: CONFIG_PARAMS.MCP_APPLICATION_QUOTA_RESOURCE, + method: CONFIG_PARAMS.MCP_APPLICATION_QUOTA_METHOD, + }, +}; + +const DEFAULT_METHOD = 'allowMcpCall'; + +type ResourcesLike = Map | undefined; + +// Test seam — mirrors resources.ts: the real registry initializes the whole +// Harper graph at import, which unit tests can't do. +let _resourcesOverride: ResourcesLike; +export function _setQuotaResourcesForTest(r: ResourcesLike): void { + _resourcesOverride = r; +} + +function getResources(): ResourcesLike { + if (_resourcesOverride) return _resourcesOverride; + const { resources } = require('../../resources/Resources'); + return resources as ResourcesLike; +} + +/** Warn-once state for a misconfigured hook (missing resource/method). */ +let warnedMisconfigured = false; +export function _resetQuotaWarningsForTest(): void { + warnedMisconfigured = false; +} + +/** + * Run the configured durable quota hook, if any. Returns `{allowed: true}` + * when no hook is configured (the feature is opt-in). Misconfiguration and + * hook errors DENY (fail-closed) with a sanitized message. + */ +export async function checkDurableQuota(info: QuotaCheckInfo): Promise { + const keys = CONFIG_KEYS[info.profile]; + const resourcePath = env.get(keys.resource); + if (typeof resourcePath !== 'string' || !resourcePath) { + return { allowed: true }; + } + const methodName = + typeof env.get(keys.method) === 'string' && env.get(keys.method) ? env.get(keys.method) : DEFAULT_METHOD; + const entry = getResources()?.get(resourcePath); + const QuotaClass = entry?.Resource as Record | undefined; + const method = QuotaClass?.[methodName as string]; + if (typeof method !== 'function') { + if (!warnedMisconfigured) { + warnedMisconfigured = true; + harperLogger.warn( + `MCP ${info.profile} quota hook misconfigured: no exported resource '${resourcePath}' with static method '${methodName}'; DENYING tool calls (fail-closed)` + ); + } + return { allowed: false, message: 'quota policy unavailable' }; + } + try { + const result = await (method as (i: QuotaCheckInfo) => unknown).call(QuotaClass, info); + if (result && typeof result === 'object') { + const decision = result as { allowed?: unknown; message?: unknown; retryAfterSeconds?: unknown }; + if (decision.allowed === false) { + return { + allowed: false, + ...(typeof decision.message === 'string' ? { message: decision.message } : {}), + ...(typeof decision.retryAfterSeconds === 'number' ? { retryAfterSeconds: decision.retryAfterSeconds } : {}), + }; + } + return { allowed: true }; + } + return result ? { allowed: true } : { allowed: false }; + } catch (error) { + harperLogger.error( + `MCP ${info.profile} quota hook '${resourcePath}.${methodName}' threw; denying (fail-closed)`, + error + ); + return { allowed: false, message: 'quota check failed' }; + } +} diff --git a/components/mcp/rateLimit.ts b/components/mcp/rateLimit.ts index fe2d80b026..8d86aa395f 100644 --- a/components/mcp/rateLimit.ts +++ b/components/mcp/rateLimit.ts @@ -1,11 +1,20 @@ /** - * Per-session, per-tool rate limiting for `tools/call`. + * Per-session, per-tool, and per-client rate limiting for `tools/call`. * - * Four configurable limits per profile (operations / application): + * Configurable limits per profile (operations / application): * - perToolPerSecond: sustained per-tool rate (token bucket refill) * - perToolBurst: per-tool burst capacity (token bucket size) * - sessionConcurrency: max in-flight tool calls per session * - sessionPerSecond: sustained per-session rate across all tools + * - perClientPerSecond / perClientBurst (#1610, default OFF): sustained + * rate keyed on CLIENT IDENTITY rather than session. Session buckets are + * trivially cycled by an anonymous client (initialize → call → drop + * session → repeat); the client bucket survives that loop. + * - identityHeader (#1610): identity is the socket IP by default; proxied + * deployments can name a trusted header (e.g. `x-forwarded-for`) whose + * first (client-most) value is used instead. Only set this when the + * proxy strips the header from untrusted traffic — a client-controlled + * header is a limit bypass. * * Limit hits surface as `result.isError = true` with `kind: 'rate_limited'` * (NOT a JSON-RPC error) per the MCP spec's tools-call convention. The LLM @@ -14,21 +23,43 @@ * State is in-memory per worker process. Buckets are evicted lazily when * a session's record is removed (#619 cleanup) or after they've been idle * past the idle eviction threshold. Multi-process coordination isn't - * attempted in v1 — the limits are per-worker. + * attempted in v1 — the limits are per-worker. For durable cross-restart + * quotas (per-IP daily counters and the like), see the config-named quota + * hook in `quota.ts` (#1610). */ import * as env from '../../utility/environment/environmentManager.ts'; import { CONFIG_PARAMS } from '../../utility/hdbTerms.ts'; +import harperLogger from '../../utility/logging/harper_logger.ts'; export interface RateLimitConfig { perToolPerSecond: number; perToolBurst: number; sessionConcurrency: number; sessionPerSecond: number; + /** 0 disables the per-client-identity bucket (the default). */ + perClientPerSecond: number; + perClientBurst: number; + /** Trusted header (lowercased) to derive client identity from; absent = socket IP. */ + identityHeader?: string; } const DEFAULTS: Record<'operations' | 'application', RateLimitConfig> = { - operations: { perToolPerSecond: 10, perToolBurst: 20, sessionConcurrency: 25, sessionPerSecond: 100 }, - application: { perToolPerSecond: 25, perToolBurst: 50, sessionConcurrency: 50, sessionPerSecond: 200 }, + operations: { + perToolPerSecond: 10, + perToolBurst: 20, + sessionConcurrency: 25, + sessionPerSecond: 100, + perClientPerSecond: 0, + perClientBurst: 0, + }, + application: { + perToolPerSecond: 25, + perToolBurst: 50, + sessionConcurrency: 50, + sessionPerSecond: 200, + perClientPerSecond: 0, + perClientBurst: 0, + }, }; const CONFIG_KEYS = { @@ -37,30 +68,91 @@ const CONFIG_KEYS = { perToolBurst: CONFIG_PARAMS.MCP_OPERATIONS_RATELIMIT_PERTOOLBURST, sessionConcurrency: CONFIG_PARAMS.MCP_OPERATIONS_RATELIMIT_SESSIONCONCURRENCY, sessionPerSecond: CONFIG_PARAMS.MCP_OPERATIONS_RATELIMIT_SESSIONPERSECOND, + perClientPerSecond: CONFIG_PARAMS.MCP_OPERATIONS_RATELIMIT_PERCLIENTPERSECOND, + perClientBurst: CONFIG_PARAMS.MCP_OPERATIONS_RATELIMIT_PERCLIENTBURST, + identityHeader: CONFIG_PARAMS.MCP_OPERATIONS_RATELIMIT_IDENTITYHEADER, }, application: { perToolPerSecond: CONFIG_PARAMS.MCP_APPLICATION_RATELIMIT_PERTOOLPERSECOND, perToolBurst: CONFIG_PARAMS.MCP_APPLICATION_RATELIMIT_PERTOOLBURST, sessionConcurrency: CONFIG_PARAMS.MCP_APPLICATION_RATELIMIT_SESSIONCONCURRENCY, sessionPerSecond: CONFIG_PARAMS.MCP_APPLICATION_RATELIMIT_SESSIONPERSECOND, + perClientPerSecond: CONFIG_PARAMS.MCP_APPLICATION_RATELIMIT_PERCLIENTPERSECOND, + perClientBurst: CONFIG_PARAMS.MCP_APPLICATION_RATELIMIT_PERCLIENTBURST, + identityHeader: CONFIG_PARAMS.MCP_APPLICATION_RATELIMIT_IDENTITYHEADER, }, }; +// tools/call is a hot path and `resolveClientIdentity` runs per call; reading +// 6–7 env keys and allocating a config object each time is avoidable GC/CPU +// pressure. Cache per profile with a short TTL — config edits still take +// effect within seconds, matching the per-session capture semantics closely +// enough while keeping the steady-state cost to a Map hit. +const CONFIG_TTL_MS = 10_000; +const configCache = new Map(); +let warnedIdentityHeader = false; + export function configFor(profile: 'operations' | 'application'): RateLimitConfig { + const cached = configCache.get(profile); + const t = now(); + if (cached && t - cached.at < CONFIG_TTL_MS) return cached.config; + const config = buildConfig(profile); + if (config.identityHeader && !warnedIdentityHeader) { + warnedIdentityHeader = true; + harperLogger.warn( + `MCP ${profile} rateLimit.identityHeader='${config.identityHeader}' derives client identity from a request header; ensure the fronting proxy STRIPS or REPLACES this header on untrusted traffic, or clients can spoof identities and bypass per-client limits` + ); + } + configCache.set(profile, { config, at: t }); + return config; +} + +function buildConfig(profile: 'operations' | 'application'): RateLimitConfig { const keys = CONFIG_KEYS[profile]; const defaults = DEFAULTS[profile]; const read = (key: string, fallback: number): number => { const v = env.get(key); return typeof v === 'number' && v > 0 ? v : fallback; }; + const identityHeader = env.get(keys.identityHeader); + const perClientPerSecond = read(keys.perClientPerSecond, defaults.perClientPerSecond); return { perToolPerSecond: read(keys.perToolPerSecond, defaults.perToolPerSecond), perToolBurst: read(keys.perToolBurst, defaults.perToolBurst), sessionConcurrency: read(keys.sessionConcurrency, defaults.sessionConcurrency), sessionPerSecond: read(keys.sessionPerSecond, defaults.sessionPerSecond), + perClientPerSecond, + // Burst defaults to the sustained rate when unset, so enabling the + // limit is a one-key change — floored at 1 token, else a fractional + // rate (0.1/s = "6 per minute") yields a bucket that can never admit + // (consume requires a whole token and refill caps at burst). + perClientBurst: read(keys.perClientBurst, perClientPerSecond > 0 ? Math.max(1, perClientPerSecond) : 0), + ...(typeof identityHeader === 'string' && identityHeader ? { identityHeader: identityHeader.toLowerCase() } : {}), }; } +/** + * Derive the client identity for per-client limiting and the durable quota + * hook (#1610): the configured trusted header's first (client-most) value + * when set, else the transport-provided socket IP. Returns undefined when + * neither is available — callers skip client-scoped checks then. + */ +export function resolveClientIdentity( + headers: Record, + clientIp: string | undefined, + profile: 'operations' | 'application' +): string | undefined { + const config = configFor(profile); + if (config.identityHeader) { + const raw = headers[config.identityHeader]; + if (raw) { + const first = raw.split(',')[0].trim(); + if (first) return first; + } + } + return clientIp || undefined; +} + /** * Token bucket: starts full at `burst`, refills at `rate` tokens per * second up to `burst`, drained by `tryConsume(1)`. Stateless aside from @@ -119,12 +211,21 @@ interface SessionState { const sessions = new Map(); +// Per-client-identity buckets, keyed `${profile}\n${identity}`. Deliberately +// SEPARATE from session state: the whole point is surviving session cycling, +// so their lifetime must not be tied to any session's (#1610). +interface ClientState { + rate: TokenBucket; + lastSeen: number; +} +const clients = new Map(); + // Belt-and-braces against state leaks: sessions that get TTL-evicted from the // system.mcp_session table never reach deleteSession() in this process, so // `clearSessionRateState` is never called for them. Prune any session that // hasn't admitted a call in this many ms on every getOrCreate. The threshold // is generously above the default idle timeout (1800s) — well-behaved live -// sessions never get pruned by accident. +// sessions never get pruned by accident. Client buckets ride the same sweep. const IDLE_PRUNE_MS = 60 * 60 * 1000; // 1 hour const PRUNE_INTERVAL_MS = 5 * 60 * 1000; // run at most every 5 minutes let lastPruneAt = 0; @@ -139,6 +240,11 @@ function pruneIdleSessions(): void { sessions.delete(id); } } + for (const [key, c] of clients) { + if (c.lastSeen < cutoff) { + clients.delete(key); + } + } } function getOrCreate(sessionId: string, profile: 'operations' | 'application'): SessionState { @@ -161,29 +267,53 @@ function getOrCreate(sessionId: string, profile: 'operations' | 'application'): return s; } +function getOrCreateClient( + identity: string, + profile: 'operations' | 'application', + config: RateLimitConfig +): ClientState { + const key = `${profile}\n${identity}`; + let c = clients.get(key); + if (!c) { + c = { rate: new TokenBucket(config.perClientPerSecond, config.perClientBurst), lastSeen: now() }; + clients.set(key, c); + } else { + c.lastSeen = now(); + } + return c; +} + /** Drop a session's rate-limit state (called on session deletion). */ export function clearSessionRateState(sessionId: string): void { sessions.delete(sessionId); } -/** Test seam: drop all sessions. */ +/** Test seam: drop all sessions, client buckets, and the config cache. */ export function _resetForTest(): void { sessions.clear(); + clients.clear(); + configCache.clear(); + warnedIdentityHeader = false; } export type RateLimitDecision = | { allowed: true; release: () => void } - | { allowed: false; reason: 'per_tool' | 'session_rate' | 'concurrency' }; + | { allowed: false; reason: 'per_tool' | 'session_rate' | 'concurrency' | 'per_client' }; /** * Attempt to admit a tools/call. If allowed, returns a `release()` that * decrements in-flight; the caller MUST invoke it (even on tool failure) * via `try { ... } finally { release(); }`. + * + * `clientIdentity` (from `resolveClientIdentity`) engages the per-client + * bucket when the profile configures `perClientPerSecond` — the scope that + * survives session cycling (#1610). Absent identity or a 0 rate skips it. */ export function tryAdmit( sessionId: string, toolName: string, - profile: 'operations' | 'application' + profile: 'operations' | 'application', + clientIdentity?: string ): RateLimitDecision { const state = getOrCreate(sessionId, profile); if (state.inFlight >= state.config.sessionConcurrency) { @@ -194,19 +324,27 @@ export function tryAdmit( toolBucket = new TokenBucket(state.config.perToolPerSecond, state.config.perToolBurst); state.perTool.set(toolName, toolBucket); } - // Peek both buckets first. Consuming session-rate before checking per-tool - // (or vice versa) silently drains the unrelated bucket on the denied path. + const clientState = + state.config.perClientPerSecond > 0 && clientIdentity + ? getOrCreateClient(clientIdentity, profile, state.config) + : undefined; + // Peek every bucket first. Consuming one before checking another silently + // drains the unrelated bucket on the denied path. if (!toolBucket.hasToken()) { return { allowed: false, reason: 'per_tool' }; } if (!state.sessionRate.hasToken()) { return { allowed: false, reason: 'session_rate' }; } - // Both have capacity — actually deduct. The peeks above ran refill(), so - // this immediate-follow-up tryConsume sees the same fresh state and is - // guaranteed to succeed (refill() is a no-op for elapsedSec ≤ 0). + if (clientState && !clientState.rate.hasToken()) { + return { allowed: false, reason: 'per_client' }; + } + // All have capacity — actually deduct. The peeks above ran refill(), so + // these immediate-follow-up tryConsume calls see the same fresh state and + // are guaranteed to succeed (refill() is a no-op for elapsedSec ≤ 0). toolBucket.tryConsume(); state.sessionRate.tryConsume(); + clientState?.rate.tryConsume(); state.inFlight += 1; return { allowed: true, diff --git a/components/mcp/transport.ts b/components/mcp/transport.ts index efde0b8115..eeaeb7d898 100644 --- a/components/mcp/transport.ts +++ b/components/mcp/transport.ts @@ -35,7 +35,8 @@ import { emitAuditEntry } from './audit.ts'; import { emitMcpLogToSession, isValidMcpLogLevel, setSessionLogLevel } from './logging.ts'; import { decodeCursor } from './pagination.ts'; import { seedSessionSnapshot } from './listChanged.ts'; -import { tryAdmit } from './rateLimit.ts'; +import { tryAdmit, resolveClientIdentity } from './rateLimit.ts'; +import { checkDurableQuota } from './quota.ts'; import { deleteSession, loadSession, saveSession, touchSession, type McpSessionRecord } from './session.ts'; import { listResources, listResourceTemplates, readResource, completeResourceArgument } from './resources.ts'; import { ensureApplicationToolsFresh } from './tools/application.ts'; @@ -89,6 +90,12 @@ export interface NormRequest { */ userObject?: AuthedUser; profile: McpProfile; + /** + * Client socket IP, for per-client rate limiting and the durable quota + * hook (#1610). Adapters populate from `request.ip`; identity resolution + * (socket vs trusted header) happens in `resolveClientIdentity`. + */ + clientIp?: string; } export interface NormResponse { @@ -518,8 +525,11 @@ async function dispatchToolsCall( // Rate limit check — admit-or-deny BEFORE invoking the handler. Failures // surface as `isError: true` with `kind: 'rate_limited'` (NOT a JSON-RPC - // error) so the LLM sees and can back off / try later. - const decision = tryAdmit(session.id, name, request.profile); + // error) so the LLM sees and can back off / try later. Client identity + // engages the per-client bucket, the scope that survives session cycling + // by anonymous clients (#1610). + const clientIdentity = resolveClientIdentity(request.headers, request.clientIp, request.profile); + const decision = tryAdmit(session.id, name, request.profile, clientIdentity); if (!decision.allowed) { // Non-strict tsconfig doesn't narrow the discriminated union here. const denied = decision as { allowed: false; reason: string }; @@ -558,6 +568,47 @@ async function dispatchToolsCall( return jsonResponse(200, buildSuccess(messageId, toolResult)); } + // Durable quota hook (#1610): operator-implemented policy behind config + // (e.g. a persisted per-IP daily counter). Runs AFTER the cheap in-memory + // admit so a rate-limited client can't spam the (possibly table-backed) + // hook, and BEFORE the handler. Fail-closed; see quota.ts. + const quota = await checkDurableQuota({ + identity: clientIdentity, + tool: name, + user, + profile: request.profile, + sessionId: session.id, + }); + if (quota.allowed === false) { + decision.release(); + const toolResult: ToolResult = { + isError: true, + content: [ + { + type: 'text', + text: JSON.stringify({ + kind: 'quota_exceeded', + tool: name, + message: quota.message ?? 'MCP quota exceeded', + ...(quota.retryAfterSeconds !== undefined ? { retryAfterSeconds: quota.retryAfterSeconds } : {}), + }), + }, + ], + }; + emitAuditEntry({ + timestamp: new Date(callStartedAt).toISOString(), + profile: request.profile, + sessionId: session.id, + tool: name, + user: user.username ?? request.user, + args: args as object, + status: 'quota_exceeded', + durationMs: 0, + }); + emitMcpLogToSession(session.id, 'notice', { kind: 'quota_exceeded', tool: name }, 'mcp.quota'); + return jsonResponse(200, buildSuccess(messageId, toolResult)); + } + // Per-call cancellation (#1349 §3.3): an inbound `notifications/cancelled` // referencing this request id aborts `signal`. Registered for the call's // lifetime, removed in `finally`. diff --git a/integrationTests/fixtures/mcp-quota/config.yaml b/integrationTests/fixtures/mcp-quota/config.yaml new file mode 100644 index 0000000000..49bf06d470 --- /dev/null +++ b/integrationTests/fixtures/mcp-quota/config.yaml @@ -0,0 +1,10 @@ +# Fixture for MCP per-client-identity rate limiting + the durable quota hook +# (#1610). The McpQuota class carries a cost-bearing custom tool and the +# config-named quota policy backed by the QuotaCounter table. +graphqlSchema: + files: '*.graphql' +jsResource: + files: resources.js +rest: true +databases: + - name: data diff --git a/integrationTests/fixtures/mcp-quota/resources.js b/integrationTests/fixtures/mcp-quota/resources.js new file mode 100644 index 0000000000..c25cda046d --- /dev/null +++ b/integrationTests/fixtures/mcp-quota/resources.js @@ -0,0 +1,31 @@ +// #1610 fixture: a cost-bearing custom tool plus the config-named durable +// quota policy. `allowMcpCall` increments a persisted per-identity counter and +// denies past DAILY_LIMIT — the reporter's public-docs `answer` tool shape. + +const DAILY_LIMIT = 3; + +export class McpQuota extends tables.QuotaCounter { + static mcpTools = [ + { + name: 'answer', + description: 'Answer a question (cost-bearing)', + method: 'doAnswer', + inputSchema: { type: 'object', properties: { q: { type: 'string' } } }, + }, + ]; + + async doAnswer(args) { + return { answered: args?.q ?? '' }; + } + + static async allowMcpCall({ identity }) { + const id = identity ?? 'unknown'; + const existing = await McpQuota.get(id); + const used = (existing?.used ?? 0) + 1; + await McpQuota.put({ id, used }); + if (used > DAILY_LIMIT) { + return { allowed: false, message: 'daily quota reached', retryAfterSeconds: 3600 }; + } + return true; + } +} diff --git a/integrationTests/fixtures/mcp-quota/schema.graphql b/integrationTests/fixtures/mcp-quota/schema.graphql new file mode 100644 index 0000000000..e32ee6a624 --- /dev/null +++ b/integrationTests/fixtures/mcp-quota/schema.graphql @@ -0,0 +1,5 @@ +# Persisted per-identity counter behind the durable quota hook (#1610). +type QuotaCounter @table @export { + id: ID @primaryKey + used: Int +} diff --git a/integrationTests/mcp/quota.test.ts b/integrationTests/mcp/quota.test.ts new file mode 100644 index 0000000000..0214076ed4 --- /dev/null +++ b/integrationTests/mcp/quota.test.ts @@ -0,0 +1,147 @@ +/** + * #1610 — per-client-identity rate limiting + the durable quota hook. + * + * The instance configures identityHeader: x-test-client (so the test controls + * identity per call), a per-client bucket of burst 6 with negligible refill, + * and quota.resource: McpQuota (persisted per-identity counter, limit 3). + * Every tools/call opens a FRESH session — the session-cycling abuse loop the + * issue describes — so anything that throttles here is client-scoped, not + * session-scoped. Expected ladder for one identity: + * calls 1–3: ok · calls 4–6: quota_exceeded (durable) · call 7+: per_client + * + * Reproduction: + * npm run test:integration -- "integrationTests/mcp/quota.test.ts" + */ +import { suite, test, before, after } from 'node:test'; +import { ok, strictEqual } from 'node:assert/strict'; +import { resolve } from 'node:path'; +import { setupHarperWithFixture, teardownHarper, type ContextWithHarper } from '@harperfast/integration-testing'; + +const FIXTURE_PATH = resolve(import.meta.dirname, '../fixtures/mcp-quota'); + +function basicAuth(username: string, password: string): string { + return `Basic ${Buffer.from(`${username}:${password}`).toString('base64')}`; +} + +suite('MCP per-client rate limit + durable quota (#1610)', (ctx: ContextWithHarper) => { + let auth: string; + let rpcId = 0; + + /** initialize a FRESH session and issue one tools/call under `identity`. */ + async function callAnswerFreshSession(identity: string): Promise { + const baseHeaders = { + 'content-type': 'application/json', + 'accept': 'application/json, text/event-stream', + 'authorization': auth, + 'x-test-client': identity, + }; + const initRes = await fetch(new URL('/mcp', ctx.harper.httpURL), { + method: 'POST', + headers: baseHeaders, + body: JSON.stringify({ + jsonrpc: '2.0', + id: ++rpcId, + method: 'initialize', + params: { protocolVersion: '2025-06-18', capabilities: {}, clientInfo: { name: 'quota-it', version: '0' } }, + }), + }); + strictEqual(initRes.status, 200, `initialize should 200: ${await initRes.clone().text()}`); + const sessionId = initRes.headers.get('mcp-session-id'); + ok(sessionId, 'session established'); + const callRes = await fetch(new URL('/mcp', ctx.harper.httpURL), { + method: 'POST', + headers: { + ...baseHeaders, + 'mcp-session-id': sessionId as string, + 'mcp-protocol-version': '2025-06-18', + }, + body: JSON.stringify({ + jsonrpc: '2.0', + id: ++rpcId, + method: 'tools/call', + params: { name: 'answer', arguments: { q: 'hello' } }, + }), + }); + const text = await callRes.text(); + strictEqual(callRes.status, 200, `tools/call should 200: ${text}`); + return JSON.parse(text); + } + + /** Parse the JSON payload custom tools/denials embed in content[0].text. */ + function payloadOf(body: any): any { + const text = body?.result?.content?.[0]?.text; + try { + return JSON.parse(text); + } catch { + return { raw: text }; + } + } + + before(async () => { + await setupHarperWithFixture(ctx, FIXTURE_PATH, { + config: { + mcp: { + application: { + mountPath: '/mcp', + rateLimit: { + // identity comes from this test-controlled header; negligible + // refill makes the 6-token burst deterministic. + identityHeader: 'x-test-client', + perClientPerSecond: 0.001, + perClientBurst: 6, + }, + quota: { resource: 'McpQuota' }, + }, + }, + }, + }); + auth = basicAuth(ctx.harper.admin.username, ctx.harper.admin.password); + }); + + after(async () => { + await teardownHarper(ctx); + }); + + test('session cycling cannot evade client-scoped limits: ok → quota_exceeded → per_client', async () => { + const outcomes: string[] = []; + for (let i = 0; i < 8; i++) { + const body = await callAnswerFreshSession('client-a'); + const payload = payloadOf(body); + if (body.result?.isError) { + outcomes.push(payload.kind); + } else { + outcomes.push('ok'); + ok(JSON.stringify(payload).includes('hello'), 'tool actually ran'); + } + } + // 1–3 pass the durable quota; 4–6 burn remaining bucket tokens but the + // counter is past the limit; 7–8 don't even reach the hook. + strictEqual(outcomes.slice(0, 3).join(','), 'ok,ok,ok'); + strictEqual(outcomes.slice(3, 6).join(','), 'quota_exceeded,quota_exceeded,quota_exceeded'); + strictEqual(outcomes.slice(6).join(','), 'rate_limited,rate_limited'); + }); + + test('quota denial carries the author message and retryAfterSeconds', async () => { + // client-a's counter is exhausted but its bucket is too; use a sibling + // identity and drain just the quota (limit 3) within the 6-token bucket. + for (let i = 0; i < 3; i++) await callAnswerFreshSession('client-b'); + const body = await callAnswerFreshSession('client-b'); + const payload = payloadOf(body); + strictEqual(body.result.isError, true); + strictEqual(payload.kind, 'quota_exceeded'); + strictEqual(payload.message, 'daily quota reached'); + strictEqual(payload.retryAfterSeconds, 3600); + }); + + test('a different client identity is unaffected and the counter persists per identity', async () => { + const body = await callAnswerFreshSession('client-c'); + strictEqual(body.result?.isError ?? false, false, `fresh identity admitted: ${JSON.stringify(body)}`); + // The durable counter is a real table row, visible over REST. + const res = await fetch(new URL('/QuotaCounter/client-c', ctx.harper.httpURL), { + headers: { authorization: auth }, + }); + strictEqual(res.status, 200); + const record = await res.json(); + strictEqual(record.used, 1); + }); +}); diff --git a/unitTests/components/mcp/quota.test.js b/unitTests/components/mcp/quota.test.js new file mode 100644 index 0000000000..b3fecbac91 --- /dev/null +++ b/unitTests/components/mcp/quota.test.js @@ -0,0 +1,119 @@ +const assert = require('node:assert'); +const { + checkDurableQuota, + _setQuotaResourcesForTest, + _resetQuotaWarningsForTest, +} = require('#src/components/mcp/quota'); +const env = require('#src/utility/environment/environmentManager'); + +const INFO = { + identity: '203.0.113.7', + tool: 'answer', + user: { username: 'anon' }, + profile: 'application', + sessionId: 's1', +}; + +describe('mcp/quota (#1610)', () => { + let envOverrides; + const originalEnvGet = env.get; + + beforeEach(() => { + envOverrides = {}; + _resetQuotaWarningsForTest(); + env.get = (key) => (key in envOverrides ? envOverrides[key] : originalEnvGet.call(env, key)); + }); + + afterEach(() => { + _setQuotaResourcesForTest(undefined); + env.get = originalEnvGet; + }); + + it('allows when no quota resource is configured (opt-in feature)', async () => { + assert.deepEqual(await checkDurableQuota(INFO), { allowed: true }); + }); + + it('fails closed when the configured resource or method does not resolve', async () => { + envOverrides.mcp_application_quota_resource = 'McpQuota'; + _setQuotaResourcesForTest(new Map()); + const decision = await checkDurableQuota(INFO); + assert.equal(decision.allowed, false); + assert.equal(decision.message, 'quota policy unavailable'); + }); + + it('calls the default-named static with the check info and honors boolean results', async () => { + envOverrides.mcp_application_quota_resource = 'McpQuota'; + const calls = []; + class McpQuota { + static allowMcpCall(info) { + calls.push(info); + return info.identity !== 'blocked'; + } + } + _setQuotaResourcesForTest(new Map([['McpQuota', { Resource: McpQuota }]])); + assert.deepEqual(await checkDurableQuota(INFO), { allowed: true }); + assert.equal(calls.length, 1); + assert.equal(calls[0].tool, 'answer'); + assert.equal(calls[0].identity, '203.0.113.7'); + const denied = await checkDurableQuota({ ...INFO, identity: 'blocked' }); + assert.equal(denied.allowed, false); + }); + + it('honors a configured method name and structured denials', async () => { + envOverrides.mcp_application_quota_resource = 'McpQuota'; + envOverrides.mcp_application_quota_method = 'checkDaily'; + class McpQuota { + static checkDaily() { + return { allowed: false, message: 'daily quota reached', retryAfterSeconds: 3600 }; + } + } + _setQuotaResourcesForTest(new Map([['McpQuota', { Resource: McpQuota }]])); + const decision = await checkDurableQuota(INFO); + assert.deepEqual(decision, { allowed: false, message: 'daily quota reached', retryAfterSeconds: 3600 }); + }); + + it('treats a non-denial object result as allowed and awaits async hooks', async () => { + envOverrides.mcp_application_quota_resource = 'McpQuota'; + class McpQuota { + static async allowMcpCall() { + return { remaining: 12 }; + } + } + _setQuotaResourcesForTest(new Map([['McpQuota', { Resource: McpQuota }]])); + assert.deepEqual(await checkDurableQuota(INFO), { allowed: true }); + }); + + it('fails closed with a sanitized message when the hook throws', async () => { + envOverrides.mcp_application_quota_resource = 'McpQuota'; + class McpQuota { + static allowMcpCall() { + throw new Error('table exploded: secret connection string'); + } + } + _setQuotaResourcesForTest(new Map([['McpQuota', { Resource: McpQuota }]])); + const decision = await checkDurableQuota(INFO); + assert.equal(decision.allowed, false); + assert.equal(decision.message, 'quota check failed'); + assert.ok(!JSON.stringify(decision).includes('secret'), 'raw error does not leak'); + }); + + it('dispatches on the live registry class (exported subclass wins)', async () => { + envOverrides.mcp_application_quota_resource = 'McpQuota'; + class Base { + static allowMcpCall() { + return true; + } + } + const registry = new Map([['McpQuota', { Resource: Base }]]); + _setQuotaResourcesForTest(registry); + assert.deepEqual(await checkDurableQuota(INFO), { allowed: true }); + class Sub { + static allowMcpCall() { + return { allowed: false, message: 'reloaded policy' }; + } + } + registry.get('McpQuota').Resource = Sub; + const decision = await checkDurableQuota(INFO); + assert.deepEqual(decision, { allowed: false, message: 'reloaded policy' }); + }); +}); diff --git a/unitTests/components/mcp/rateLimit.test.js b/unitTests/components/mcp/rateLimit.test.js index d04cd4f18f..bc513c9715 100644 --- a/unitTests/components/mcp/rateLimit.test.js +++ b/unitTests/components/mcp/rateLimit.test.js @@ -3,6 +3,7 @@ const { tryAdmit, clearSessionRateState, configFor, + resolveClientIdentity, _setClockForTest, _resetForTest, } = require('#src/components/mcp/rateLimit'); @@ -30,9 +31,23 @@ describe('mcp/rateLimit', () => { describe('configFor', () => { it('returns the documented defaults for each profile when nothing is configured', () => { const ops = configFor('operations'); - assert.deepEqual(ops, { perToolPerSecond: 10, perToolBurst: 20, sessionConcurrency: 25, sessionPerSecond: 100 }); + assert.deepEqual(ops, { + perToolPerSecond: 10, + perToolBurst: 20, + sessionConcurrency: 25, + sessionPerSecond: 100, + perClientPerSecond: 0, + perClientBurst: 0, + }); const app = configFor('application'); - assert.deepEqual(app, { perToolPerSecond: 25, perToolBurst: 50, sessionConcurrency: 50, sessionPerSecond: 200 }); + assert.deepEqual(app, { + perToolPerSecond: 25, + perToolBurst: 50, + sessionConcurrency: 50, + sessionPerSecond: 200, + perClientPerSecond: 0, + perClientBurst: 0, + }); }); it('overrides defaults from configured values', () => { @@ -208,4 +223,90 @@ describe('mcp/rateLimit', () => { assert.equal(stillExhausted.allowed, false, 'active session keeps its state'); }); }); + describe('per-client identity buckets (#1610)', () => { + it('is disabled by default: identity present but zero rate adds no per_client denials', () => { + for (let i = 0; i < 60; i++) { + const d = tryAdmit(`cycle-${i}`, 'answer', 'application', '203.0.113.7'); + assert.equal(d.allowed, true, `call ${i} admitted (per-tool bucket is per-session)`); + d.release(); + } + }); + + it('survives session cycling: fresh sessions share the identity bucket', () => { + envOverrides.mcp_application_rateLimit_perClientPerSecond = 1; + envOverrides.mcp_application_rateLimit_perClientBurst = 3; + const results = []; + for (let i = 0; i < 5; i++) { + // A brand-new session per call — the session-cycling abuse loop. + const d = tryAdmit(`fresh-${i}`, 'answer', 'application', '203.0.113.7'); + results.push(d.allowed ? 'ok' : d.reason); + if (d.allowed) d.release(); + } + assert.deepEqual(results, ['ok', 'ok', 'ok', 'per_client', 'per_client']); + }); + + it('separates identities and profiles', () => { + envOverrides.mcp_application_rateLimit_perClientPerSecond = 1; + envOverrides.mcp_application_rateLimit_perClientBurst = 1; + assert.equal(tryAdmit('s1', 't', 'application', 'ip-a').allowed, true); + assert.equal(tryAdmit('s2', 't', 'application', 'ip-a').reason, 'per_client'); + assert.equal(tryAdmit('s3', 't', 'application', 'ip-b').allowed, true, 'other identity unaffected'); + assert.equal(tryAdmit('s4', 't', 'operations', 'ip-a').allowed, true, 'other profile unaffected'); + }); + + it('refills over time and skips the bucket when identity is unknown', () => { + envOverrides.mcp_application_rateLimit_perClientPerSecond = 1; + envOverrides.mcp_application_rateLimit_perClientBurst = 1; + assert.equal(tryAdmit('s1', 't', 'application', 'ip-a').allowed, true); + assert.equal(tryAdmit('s2', 't', 'application', 'ip-a').reason, 'per_client'); + clock += 1000; + assert.equal(tryAdmit('s3', 't', 'application', 'ip-a').allowed, true, 'refilled after 1s'); + assert.equal(tryAdmit('s4', 't', 'application', undefined).allowed, true, 'no identity, no client bucket'); + }); + + it('denied per_client does not burn tool or session tokens', () => { + envOverrides.mcp_application_rateLimit_perClientPerSecond = 1; + envOverrides.mcp_application_rateLimit_perClientBurst = 1; + tryAdmit('s1', 't', 'application', 'ip-a').release(); + // Exhausted identity: repeated denials... + for (let i = 0; i < 10; i++) { + assert.equal(tryAdmit('s1', 't', 'application', 'ip-a').reason, 'per_client'); + } + // ...must not have drained s1's per-tool/session buckets. + clock += 1000; + assert.equal(tryAdmit('s1', 't', 'application', 'ip-a').allowed, true); + }); + + it('perClientBurst defaults to perClientPerSecond when unset, floored at one whole token', () => { + envOverrides.mcp_application_rateLimit_perClientPerSecond = 2; + assert.equal(configFor('application').perClientBurst, 2); + _resetForTest(); + // A fractional sustained rate ("6 per minute") must still admit its + // first call — a burst below 1 token could never admit anything. + envOverrides.mcp_application_rateLimit_perClientPerSecond = 0.1; + assert.equal(configFor('application').perClientBurst, 1); + assert.equal(tryAdmit('s1', 't', 'application', 'ip-frac').allowed, true); + assert.equal(tryAdmit('s2', 't', 'application', 'ip-frac').reason, 'per_client'); + clock += 10_000; + assert.equal(tryAdmit('s3', 't', 'application', 'ip-frac').allowed, true, 'refilled after 10s at 0.1/s'); + }); + }); + + describe('resolveClientIdentity (#1610)', () => { + it('uses the socket IP when no header is configured', () => { + assert.equal(resolveClientIdentity({}, '198.51.100.4', 'application'), '198.51.100.4'); + assert.equal(resolveClientIdentity({}, undefined, 'application'), undefined); + assert.equal(resolveClientIdentity({}, '', 'application'), undefined); + }); + + it('prefers the first value of the configured trusted header, falling back to the socket IP', () => { + envOverrides.mcp_application_rateLimit_identityHeader = 'X-Forwarded-For'; + assert.equal( + resolveClientIdentity({ 'x-forwarded-for': '203.0.113.9, 10.0.0.1' }, '10.0.0.1', 'application'), + '203.0.113.9' + ); + assert.equal(resolveClientIdentity({}, '10.0.0.1', 'application'), '10.0.0.1'); + assert.equal(resolveClientIdentity({ 'x-forwarded-for': ' , ' }, '10.0.0.1', 'application'), '10.0.0.1'); + }); + }); }); diff --git a/utility/hdbTerms.ts b/utility/hdbTerms.ts index fa2bb280b4..b42a71d3c9 100644 --- a/utility/hdbTerms.ts +++ b/utility/hdbTerms.ts @@ -548,6 +548,11 @@ 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_OPERATIONS_RATELIMIT_PERCLIENTPERSECOND: 'mcp_operations_rateLimit_perClientPerSecond', + MCP_OPERATIONS_RATELIMIT_PERCLIENTBURST: 'mcp_operations_rateLimit_perClientBurst', + MCP_OPERATIONS_RATELIMIT_IDENTITYHEADER: 'mcp_operations_rateLimit_identityHeader', + MCP_OPERATIONS_QUOTA_RESOURCE: 'mcp_operations_quota_resource', + MCP_OPERATIONS_QUOTA_METHOD: 'mcp_operations_quota_method', MCP_APPLICATION_MOUNTPATH: 'mcp_application_mountPath', MCP_APPLICATION_ALLOW: 'mcp_application_allow', MCP_APPLICATION_DENY: 'mcp_application_deny', @@ -557,6 +562,11 @@ export const CONFIG_PARAMS = { 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_APPLICATION_RATELIMIT_PERCLIENTPERSECOND: 'mcp_application_rateLimit_perClientPerSecond', + MCP_APPLICATION_RATELIMIT_PERCLIENTBURST: 'mcp_application_rateLimit_perClientBurst', + MCP_APPLICATION_RATELIMIT_IDENTITYHEADER: 'mcp_application_rateLimit_identityHeader', + MCP_APPLICATION_QUOTA_RESOURCE: 'mcp_application_quota_resource', + MCP_APPLICATION_QUOTA_METHOD: 'mcp_application_quota_method', MCP_SESSION_IDLETIMEOUTSECONDS: 'mcp_session_idleTimeoutSeconds', MCP_SESSION_ALLOWCLIENTDELETE: 'mcp_session_allowClientDelete', AGENT_ENABLED: 'agent_enabled', From 594ff6d4f8c1d1584d3664fdda840416dbd86ac6 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Mon, 6 Jul 2026 19:47:30 -0600 Subject: [PATCH 09/12] Fix stacked-PR base drift and warn per profile on quota misconfiguration The rebase onto the #1613 branch had replayed the intervening v5.1 release commits (including the #1575 revert) as head-side commits, polluting the PR diff; the branch now carries only this feature's commit. The misconfiguration warn-once flag is per profile so a second misconfigured profile still logs (gemini bot). Co-Authored-By: Claude Fable 5 --- components/mcp/quota.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/components/mcp/quota.ts b/components/mcp/quota.ts index f1e9093c5d..0d9fb93d51 100644 --- a/components/mcp/quota.ts +++ b/components/mcp/quota.ts @@ -81,10 +81,10 @@ function getResources(): ResourcesLike { return resources as ResourcesLike; } -/** Warn-once state for a misconfigured hook (missing resource/method). */ -let warnedMisconfigured = false; +/** Warn-once-per-profile state for a misconfigured hook (missing resource/method). */ +const warnedMisconfigured = new Set(); export function _resetQuotaWarningsForTest(): void { - warnedMisconfigured = false; + warnedMisconfigured.clear(); } /** @@ -104,8 +104,8 @@ export async function checkDurableQuota(info: QuotaCheckInfo): Promise | undefined; const method = QuotaClass?.[methodName as string]; if (typeof method !== 'function') { - if (!warnedMisconfigured) { - warnedMisconfigured = true; + if (!warnedMisconfigured.has(info.profile)) { + warnedMisconfigured.add(info.profile); harperLogger.warn( `MCP ${info.profile} quota hook misconfigured: no exported resource '${resourcePath}' with static method '${methodName}'; DENYING tool calls (fail-closed)` ); From 6d489904fad1ee72e13f0379accafbf9ab2c82ce Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Tue, 7 Jul 2026 14:54:35 -0600 Subject: [PATCH 10/12] Document quota-hook race-safety requirement (kriszyp fast-follow) The reference fixture's get-then-put counter is the shape operators will copy, and it can undercount under concurrent calls for the same identity. RACE-SAFETY note in quota.ts (transaction / CAS / native atomic increment) plus an honest caveat on the fixture. Co-Authored-By: Claude Fable 5 --- components/mcp/quota.ts | 9 +++++++++ integrationTests/fixtures/mcp-quota/resources.js | 6 ++++++ 2 files changed, 15 insertions(+) diff --git a/components/mcp/quota.ts b/components/mcp/quota.ts index 0d9fb93d51..c6b32d35bd 100644 --- a/components/mcp/quota.ts +++ b/components/mcp/quota.ts @@ -26,6 +26,15 @@ * itself on a bug is worse than a hard failure (#1422 set this precedent * for allow* hooks). The raw error goes to the server log only. * + * RACE-SAFETY: the hook can run concurrently for the SAME identity — within + * a worker (interleaving across the hook's own await boundaries) and across + * workers (separate processes sharing the database). A naive read-then-write + * counter (`get` → `put used+1`) can undercount under that concurrency and + * admit calls past the configured limit. Production implementations should + * make the read-modify-write atomic: run it in a transaction that serializes + * conflicting writers, use a compare-and-set retry loop, or maintain the + * counter in a store with native atomic increments. + * * Dispatch uses the LIVE registry class, same as custom tools — an exported * subclass replacing the entry on reload wins. */ diff --git a/integrationTests/fixtures/mcp-quota/resources.js b/integrationTests/fixtures/mcp-quota/resources.js index c25cda046d..0663a9591d 100644 --- a/integrationTests/fixtures/mcp-quota/resources.js +++ b/integrationTests/fixtures/mcp-quota/resources.js @@ -1,6 +1,12 @@ // #1610 fixture: a cost-bearing custom tool plus the config-named durable // quota policy. `allowMcpCall` increments a persisted per-identity counter and // denies past DAILY_LIMIT — the reporter's public-docs `answer` tool shape. +// +// NOTE: the get-then-put below is NOT race-safe — concurrent calls for one +// identity can interleave between the read and the write and undercount. Fine +// for this single-threaded test instance; production quota implementations +// must make the read-modify-write atomic (see the RACE-SAFETY note in +// components/mcp/quota.ts). const DAILY_LIMIT = 3; From c2dabace2c8e065db8cfd6566528dfb22040df90 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Wed, 8 Jul 2026 06:11:52 -0600 Subject: [PATCH 11/12] fix(lint): use node:assert instead of node:assert/strict Matches this repo's no-restricted-imports convention already followed by the other MCP integration test files. ok/strictEqual/deepStrictEqual are strict-mode in both modules, so this is behavior-preserving. --- integrationTests/mcp/custom-resources.test.ts | 2 +- integrationTests/mcp/quota.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/integrationTests/mcp/custom-resources.test.ts b/integrationTests/mcp/custom-resources.test.ts index dfbae3a994..c2fd7d11da 100644 --- a/integrationTests/mcp/custom-resources.test.ts +++ b/integrationTests/mcp/custom-resources.test.ts @@ -10,7 +10,7 @@ * npm run test:integration -- "integrationTests/mcp/custom-resources.test.ts" */ import { suite, test, before, after } from 'node:test'; -import { deepStrictEqual, ok, strictEqual } from 'node:assert/strict'; +import { deepStrictEqual, ok, strictEqual } from 'node:assert'; import { resolve } from 'node:path'; import { setupHarperWithFixture, teardownHarper, type ContextWithHarper } from '@harperfast/integration-testing'; diff --git a/integrationTests/mcp/quota.test.ts b/integrationTests/mcp/quota.test.ts index 0214076ed4..a950ee0048 100644 --- a/integrationTests/mcp/quota.test.ts +++ b/integrationTests/mcp/quota.test.ts @@ -13,7 +13,7 @@ * npm run test:integration -- "integrationTests/mcp/quota.test.ts" */ import { suite, test, before, after } from 'node:test'; -import { ok, strictEqual } from 'node:assert/strict'; +import { ok, strictEqual } from 'node:assert'; import { resolve } from 'node:path'; import { setupHarperWithFixture, teardownHarper, type ContextWithHarper } from '@harperfast/integration-testing'; From d06f8daac972e5a8ed625072ddebeb44be8e7149 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Wed, 8 Jul 2026 06:14:01 -0600 Subject: [PATCH 12/12] style: prettier formatting --- components/mcp/tools/application.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/components/mcp/tools/application.ts b/components/mcp/tools/application.ts index 7a6898a0eb..4bde7f21b9 100644 --- a/components/mcp/tools/application.ts +++ b/components/mcp/tools/application.ts @@ -986,8 +986,7 @@ function makeCustomResourceReader(path: string, capturedClass: ResourceClassLike const Ctor = ResourceClass as unknown as new (id: unknown, ctx: unknown) => Record; const instance = new Ctor(undefined, buildContext(context.user)); const method = instance[methodName] as - | ((p: Record, ctx: ResourceReadContext) => CustomResourceReadResult) - | undefined; + ((p: Record, ctx: ResourceReadContext) => CustomResourceReadResult) | undefined; if (typeof method !== 'function') { throw new Error(`method '${methodName}' is not a function on the constructed Resource`); }