From 0eb2d0b95490570c78e5c2ded83bf55541dbddd9 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Mon, 6 Jul 2026 08:17:55 -0600 Subject: [PATCH 1/7] 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 2/7] 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 3/7] 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 4/7] 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 5/7] 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 6/7] 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 a49b8b1c8784bb0f1e318c36b237978de57f3baf Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Tue, 7 Jul 2026 15:45:58 -0600 Subject: [PATCH 7/7] Reject encoded separators in {name} template segments (traversal guard) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit {name} captures ran against the still-encoded URI, so %2F/%5C slipped through the [^/]+ segment class and decoded to real separators after the boundary check — defeating the one-segment contract authors rely on for path construction (kriszyp review). Such URIs now simply fail to match; {+name} reserved expansion still decodes them by design. Co-Authored-By: Claude Fable 5 --- components/mcp/customResourceRegistry.ts | 55 +++++++++++++------ .../mcp/customResourceRegistry.test.js | 26 +++++++-- 2 files changed, 60 insertions(+), 21 deletions(-) diff --git a/components/mcp/customResourceRegistry.ts b/components/mcp/customResourceRegistry.ts index 5ac1ae9fae..3e7259799b 100644 --- a/components/mcp/customResourceRegistry.ts +++ b/components/mcp/customResourceRegistry.ts @@ -58,20 +58,27 @@ interface CompiledDef { def: CustomResourceDef; /** Present for template entries only. */ regex?: RegExp; - paramNames?: string[]; + params?: TemplateParam[]; +} + +interface TemplateParam { + name: string; + /** `{+name}` — reserved expansion, may span segments. */ + reserved: boolean; } 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. + * segment (`[^/]+`), and an encoded separator — `%2F`/`%5C` — in the capture + * rejects the match, so the one-segment contract survives percent-decoding); + * `{+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[] = []; +export function compileUriTemplate(template: string): { regex: RegExp; params: TemplateParam[] } { + const params: TemplateParam[] = []; let pattern = ''; let index = 0; while (index < template.length) { @@ -89,18 +96,25 @@ 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)) { + if (params.some((p) => p.name === name)) { // a repeated name would silently overwrite the earlier captured value throw new Error(`duplicate template parameter '{${name}}' in uriTemplate: ${template}`); } - paramNames.push(name); + params.push({ name, reserved }); 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 }; + if (params.length === 0) throw new Error(`uriTemplate has no parameters (use \`uri\` instead): ${template}`); + return { regex: new RegExp(`^${pattern}$`), params }; } +// `{name}` captures run against the still-encoded URI, so a client can smuggle +// a separator through the `[^/]+` class as %2F (or %5C) and have it decode to +// a real slash/backslash AFTER the segment boundary was checked — defeating +// the one-segment contract authors rely on for path construction. Reject the +// match instead (the URI simply doesn't fit a single-segment slot). +const ENCODED_SEPARATOR = /%2f|%5c/i; + function escapeRegex(literal: string): string { return literal.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); } @@ -109,9 +123,9 @@ function escapeRegex(literal: string): string { export function addCustomResource(def: CustomResourceDef): void { const compiled: CompiledDef = { def }; if (def.uriTemplate) { - const { regex, paramNames } = compileUriTemplate(def.uriTemplate); + const { regex, params } = compileUriTemplate(def.uriTemplate); compiled.regex = regex; - compiled.paramNames = paramNames; + compiled.params = params; } let list = registry.get(def.profile); if (!list) { @@ -181,14 +195,21 @@ export function matchCustomResource( for (const { def } of list) { if (def.uri === uri) return { def, params: {} }; } - for (const { def, regex, paramNames } of list) { - if (!regex || !paramNames) continue; + for (const { def, regex, params: templateParams } of list) { + if (!regex || !templateParams) 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]); + let separatorSmuggled = false; + for (let i = 0; i < templateParams.length; i++) { + const raw = match[i + 1]; + if (!templateParams[i].reserved && ENCODED_SEPARATOR.test(raw)) { + separatorSmuggled = true; + break; + } + params[templateParams[i].name] = decodeURIComponentSafe(raw); } + if (separatorSmuggled) continue; return { def, params }; } return undefined; diff --git a/unitTests/components/mcp/customResourceRegistry.test.js b/unitTests/components/mcp/customResourceRegistry.test.js index e2306673cf..7b3166b8ca 100644 --- a/unitTests/components/mcp/customResourceRegistry.test.js +++ b/unitTests/components/mcp/customResourceRegistry.test.js @@ -19,15 +19,15 @@ describe('mcp/customResourceRegistry (#1609)', () => { describe('compileUriTemplate', () => { it('matches {name} within a single segment only', () => { - const { regex, paramNames } = compileUriTemplate('docs:///{section}/index'); - assert.deepEqual(paramNames, ['section']); + const { regex, params } = compileUriTemplate('docs:///{section}/index'); + assert.deepEqual(params, [{ name: 'section', reserved: false }]); 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']); + const { regex, params } = compileUriTemplate('docs:///{+path}'); + assert.deepEqual(params, [{ name: 'path', reserved: true }]); assert.ok(regex.test('docs:///getting-started/install.md')); }); @@ -61,6 +61,24 @@ describe('mcp/customResourceRegistry (#1609)', () => { assert.deepEqual(match.params, { path: 'guides/getting started.md' }); }); + it('rejects encoded separators smuggled into a {name} segment (traversal guard)', () => { + addCustomResource({ uriTemplate: 'docs:///{page}', name: 'one-seg', profile: 'application', read }); + // %2F / %5C pass the [^/]+ class raw, then would decode to real + // separators — the one-segment contract must reject the match. + assert.equal(matchCustomResource('application', 'docs:///..%2F..%2Fetc%2Fpasswd'), undefined); + assert.equal(matchCustomResource('application', 'docs:///..%2f..%2fsecrets'), undefined); + assert.equal(matchCustomResource('application', 'docs:///a%5Cb'), undefined); + // Ordinary encoded characters still decode fine in a {name} slot. + const match = matchCustomResource('application', 'docs:///getting%20started'); + assert.deepEqual(match.params, { page: 'getting started' }); + }); + + it('still decodes %2F inside {+name} (reserved expansion spans segments by design)', () => { + addCustomResource({ uriTemplate: 'files:///{+path}', name: 'multi', profile: 'application', read }); + const match = matchCustomResource('application', 'files:///a%2Fb/c'); + assert.deepEqual(match.params, { path: 'a/b/c' }); + }); + 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');