Skip to content

Commit 8e5b23f

Browse files
antfubotclaude
antfubot
and
claude
committed
refactor(mcp): trim resource template compile and tests
- Compile a template with one split instead of a matchAll loop and two brace checks - Drop the stop > end check: the last value's stop <= position check covers it - Reuse McpResourceTemplateEntry for resources/list and inline the templates/list handler - Fold the percent-decode and no-match tests into one match() test - Keep the hub test inside the existing describe instead of moving callMcp Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent 5c9c804 commit 8e5b23f

3 files changed

Lines changed: 83 additions & 130 deletions

File tree

‎packages/agentic/src/mcp/__tests__/mcp-server.test.ts‎

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -641,8 +641,6 @@ describe('mcp adapter (in-memory)', () => {
641641
try {
642642
const a = await client.readResource({ uri: 'test://file/a.ts' })
643643
expect(a.contents).toEqual([{ uri: 'test://file/a.ts', mimeType: 'text/plain', text: 'a.ts from test://file/a.ts' }])
644-
const b = await client.readResource({ uri: 'test://file/b.json' })
645-
expect(b.contents).toEqual([{ uri: 'test://file/b.json', mimeType: 'text/plain', text: 'b.json from test://file/b.json' }])
646644

647645
const read = await client.readResource({ uri: 'test://repo/devframes/devframe/issue/42' })
648646
const c = read.contents[0] as { text: string, mimeType?: string }
@@ -654,26 +652,11 @@ describe('mcp adapter (in-memory)', () => {
654652
}
655653
})
656654

657-
it('percent-decodes the values', async () => {
658-
const { client, cleanup } = await bootPair(true, { resourceTemplates: [file] })
659-
try {
660-
const read = await client.readResource({ uri: 'test://file/src%2Fmy%20file.ts' })
661-
expect(read.contents[0]).toMatchObject({ text: 'src/my file.ts from test://file/src%2Fmy%20file.ts' })
662-
}
663-
finally {
664-
await cleanup()
665-
}
666-
})
667-
668-
it('matches one segment per value, so an extra `/` matches no template', async () => {
669-
const { client, cleanup } = await bootPair(true, { resourceTemplates: [file] })
670-
try {
671-
for (const uri of ['test://file/src/a.ts', 'test://file/', 'test://file/%E0%A4%A'])
672-
await expect(client.readResource({ uri })).rejects.toThrow(`unknown resource URI "${uri}"`)
673-
}
674-
finally {
675-
await cleanup()
676-
}
655+
it('percent-decodes one non-empty segment per value', () => {
656+
const [compiled] = compileResourceTemplates([file])
657+
expect(compiled!.match('test://file/src%2Fmy%20file.ts')).toEqual({ path: 'src/my file.ts' })
658+
for (const uri of ['test://file/src/a.ts', 'test://file/', 'test://file/%E0%A4%A'])
659+
expect(compiled!.match(uri)).toBeUndefined()
677660
})
678661

679662
it('reads with the first template that matches', async () => {

‎packages/agentic/src/mcp/build-server.ts‎

Lines changed: 27 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { Tool } from '@modelcontextprotocol/server'
22
import type { StandardSchemaV1 } from '@standard-schema/spec'
33
import type { RpcFunctionDefinitionAnyWithContext } from 'devframe/rpc'
4-
import type { AgentTool, CreateMcpServerOptions, DevframeDefinition, DevframeHost, DevframeNodeContext, McpContentBlock, McpResourcesChangedHook, McpResourceTemplate, McpServerHandle } from 'devframe/types'
4+
import type { AgentTool, CreateMcpServerOptions, DevframeDefinition, DevframeHost, DevframeNodeContext, McpContentBlock, McpResourcesChangedHook, McpResourceTemplate, McpResourceTemplateEntry, McpServerHandle } from 'devframe/types'
55
import { homedir } from 'node:os'
66
import process from 'node:process'
77
import { Server } from '@modelcontextprotocol/server'
@@ -68,8 +68,11 @@ export function buildMcpServerFromContext(
6868
registerToolHandlers(server, ctx, options.exposeSharedState)
6969
const templates = options.resourceTemplates ?? []
7070
registerResourceHandlers(server, ctx, options.exposeSharedState, templates)
71-
if (templates.length)
72-
registerResourceTemplateHandlers(server, templates)
71+
if (templates.length) {
72+
server.setRequestHandler('resources/templates/list', async () => ({
73+
resourceTemplates: templates.map(({ template: { uriTemplate, name, title, description, mimeType } }) => ({ uriTemplate, name, title, description, mimeType })),
74+
}))
75+
}
7376

7477
return server
7578
}
@@ -341,7 +344,7 @@ function registerResourceHandlers(
341344
const stateFilter = sharedStateFilter(exposeSharedState)
342345

343346
server.setRequestHandler('resources/list', async () => {
344-
const resources: ResourceListing[] = ctx.agent.list().resources.map(resource => ({
347+
const resources: McpResourceTemplateEntry[] = ctx.agent.list().resources.map(resource => ({
345348
uri: resource.uri,
346349
name: resource.name,
347350
description: resource.description,
@@ -429,24 +432,8 @@ function registerResourceHandlers(
429432
})
430433
}
431434

432-
/** One entry of the `resources/list` result. */
433-
interface ResourceListing {
434-
uri: string
435-
name: string
436-
title?: string
437-
description?: string
438-
mimeType?: string
439-
}
440-
441-
/** Serve `resources/templates/list`: one entry per template, in the given order. */
442-
function registerResourceTemplateHandlers(server: Server, templates: CompiledResourceTemplate[]): void {
443-
server.setRequestHandler('resources/templates/list', async () => ({
444-
resourceTemplates: templates.map(({ template: { uriTemplate, name, title, description, mimeType } }) => ({ uriTemplate, name, title, description, mimeType })),
445-
}))
446-
}
447-
448435
/** One `{name}` expression of a URI template. */
449-
const TEMPLATE_EXPRESSION_RE = /\{([^{}]*)\}/g
436+
const TEMPLATE_EXPRESSION_RE = /\{([^{}]*)\}/
450437
/** RFC 6570 `varname`: letters, digits and `_`, with single dots between them. */
451438
const TEMPLATE_VARNAME_RE = /^\w+(?:\.\w+)*$/
452439

@@ -461,40 +448,25 @@ const TEMPLATE_VARNAME_RE = /^\w+(?:\.\w+)*$/
461448
* @internal
462449
*/
463450
export function compileResourceTemplates(templates: McpResourceTemplate[] = []): CompiledResourceTemplate[] {
464-
return templates.map(compileResourceTemplate)
465-
}
466-
467-
function compileResourceTemplate(template: McpResourceTemplate): CompiledResourceTemplate {
468-
const { name, uriTemplate } = template
469-
const invalid = (reason: string) => diagnostics.DF0084({ name, uriTemplate, reason })
470-
const names: string[] = []
471-
// The text around the expressions: one more than `names`.
472-
const literals: string[] = []
473-
let last = 0
474-
for (const expression of uriTemplate.matchAll(TEMPLATE_EXPRESSION_RE)) {
475-
const literal = uriTemplate.slice(last, expression.index)
476-
if (/[{}]/.test(literal))
451+
return templates.map((template) => {
452+
const { name, uriTemplate } = template
453+
const invalid = (reason: string) => diagnostics.DF0084({ name, uriTemplate, reason })
454+
// The capture group keeps each name: literals at even indexes, names at odd ones.
455+
const parts = uriTemplate.split(TEMPLATE_EXPRESSION_RE)
456+
const literals = parts.filter((_, index) => index % 2 === 0)
457+
const names = parts.filter((_, index) => index % 2 === 1)
458+
if (literals.some(literal => /[{}]/.test(literal)))
477459
throw invalid('a `{` or `}` is outside an expression.')
478-
const varname = expression[1]!
479-
if (!TEMPLATE_VARNAME_RE.test(varname))
480-
throw invalid(`the expression "{${varname}}" is not a plain \`{name}\`.`)
481-
if (names.includes(varname))
482-
throw invalid(`the name "${varname}" is used more than once.`)
483-
if (names.length > 0 && literal === '')
484-
throw invalid(`"{${names.at(-1)}}" and "{${varname}}" have nothing between them, so a URI cannot show where one ends.`)
485-
literals.push(literal)
486-
names.push(varname)
487-
last = expression.index + expression[0].length
488-
}
489-
const tail = uriTemplate.slice(last)
490-
if (/[{}]/.test(tail))
491-
throw invalid('a `{` or `}` is outside an expression.')
492-
literals.push(tail)
493-
494-
return {
495-
template,
496-
match: uri => matchTemplate(uri, literals, names),
497-
}
460+
names.forEach((varname, index) => {
461+
if (!TEMPLATE_VARNAME_RE.test(varname))
462+
throw invalid(`the expression "{${varname}}" is not a plain \`{name}\`.`)
463+
if (names.indexOf(varname) !== index)
464+
throw invalid(`the name "${varname}" is used more than once.`)
465+
if (index > 0 && literals[index] === '')
466+
throw invalid(`"{${names[index - 1]}}" and "{${varname}}" have nothing between them, so a URI cannot show where one ends.`)
467+
})
468+
return { template, match: uri => matchTemplate(uri, literals, names) }
469+
})
498470
}
499471

500472
/**
@@ -520,7 +492,7 @@ function matchTemplate(uri: string, literals: string[], names: string[]): Record
520492
const isLast = index === names.length - 1
521493
const next = literals[index + 1]!
522494
const stop = isLast ? end : findLiteral(uri, next, position)
523-
if (stop <= position || stop > end)
495+
if (stop <= position)
524496
return undefined
525497
const raw = uri.slice(position, stop)
526498
if (raw.includes('/'))

‎packages/hub/src/node/__tests__/initiate.test.ts‎

Lines changed: 51 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -44,22 +44,6 @@ function connectWsClient(url: string) {
4444
)
4545
}
4646

47-
/** POST one JSON-RPC request to the hub's aggregate MCP route and return the raw body. */
48-
async function callMcp(hub: ReturnType<typeof initHub>, method: string, params?: Record<string, unknown>): Promise<string> {
49-
const origin = 'http://localhost:3000'
50-
const res = await hub.handler(new Request(`${origin}/__devframes/__mcp`, {
51-
method: 'POST',
52-
headers: {
53-
'content-type': 'application/json',
54-
'accept': 'application/json, text/event-stream',
55-
origin,
56-
},
57-
body: JSON.stringify({ jsonrpc: '2.0', id: 1, method, params }),
58-
}))
59-
expect(res.status).toBe(200)
60-
return await res.text()
61-
}
62-
6347
describe('initHub', () => {
6448
it('publishes an empty renderer manifest when no renderers are registered', async () => {
6549
expect.assertions(2)
@@ -340,7 +324,9 @@ describe('initHub', () => {
340324
}
341325
})
342326

343-
describe('aggregate MCP shared-state exposure', () => {
327+
describe('aggregate MCP resources', () => {
328+
const origin = 'http://localhost:3000'
329+
344330
function makeStateFrame(): DevframeDefinition {
345331
const frame = makeFrame('state')
346332
return {
@@ -353,6 +339,20 @@ describe('initHub', () => {
353339
}
354340
}
355341

342+
async function callMcp(hub: ReturnType<typeof initHub>, method: string, params?: Record<string, unknown>): Promise<string> {
343+
const res = await hub.handler(new Request(`${origin}/__devframes/__mcp`, {
344+
method: 'POST',
345+
headers: {
346+
'content-type': 'application/json',
347+
'accept': 'application/json, text/event-stream',
348+
origin,
349+
},
350+
body: JSON.stringify({ jsonrpc: '2.0', id: 1, method, params }),
351+
}))
352+
expect(res.status).toBe(200)
353+
return await res.text()
354+
}
355+
356356
it('exposes every shared state by default', async () => {
357357
const wsPort = await getPort({ port: 18235, host: '127.0.0.1' })
358358
const hub = initHub({ base: DEVFRAMES_HUB_BASE, auth: false, host: '127.0.0.1', ws: { port: wsPort }, mcp: true, devframes: [makeStateFrame()] })
@@ -421,6 +421,40 @@ describe('initHub', () => {
421421
await hub.close()
422422
}
423423
})
424+
425+
it('serves mcp.resourceTemplates and wires mcp.onResourcesChanged to the hub context', async () => {
426+
const wsPort = await getPort({ port: 18242, host: '127.0.0.1' })
427+
const unsubscribe = vi.fn()
428+
const onResourcesChanged = vi.fn(() => unsubscribe)
429+
const hub = initHub({
430+
base: DEVFRAMES_HUB_BASE,
431+
auth: false,
432+
host: '127.0.0.1',
433+
ws: { port: wsPort },
434+
mcp: {
435+
resourceTemplates: [{
436+
uriTemplate: 'hub://frame/{id}',
437+
name: 'frame',
438+
list: () => [{ uri: 'hub://frame/alpha', name: 'alpha' }],
439+
read: params => ({ json: { id: params.id } }),
440+
}],
441+
onResourcesChanged,
442+
},
443+
devframes: [makeFrame('alpha')],
444+
})
445+
446+
try {
447+
await hub.ready
448+
expect(await callMcp(hub, 'resources/templates/list')).toContain('"uriTemplate":"hub://frame/{id}"')
449+
expect(onResourcesChanged).toHaveBeenCalledWith(expect.any(Function), await hub.context)
450+
expect(await callMcp(hub, 'resources/list')).toContain('"uri":"hub://frame/alpha"')
451+
expect(await callMcp(hub, 'resources/read', { uri: 'hub://frame/beta' })).toContain('\\"id\\": \\"beta\\"')
452+
}
453+
finally {
454+
await hub.close()
455+
}
456+
expect(unsubscribe).toHaveBeenCalledTimes(1)
457+
})
424458
})
425459

426460
describe('aggregate MCP instructions', () => {
@@ -481,42 +515,6 @@ describe('initHub', () => {
481515
})
482516
})
483517

484-
describe('aggregate MCP resource templates', () => {
485-
it('serves mcp.resourceTemplates and wires mcp.onResourcesChanged to the hub context', async () => {
486-
const wsPort = await getPort({ port: 18242, host: '127.0.0.1' })
487-
const unsubscribe = vi.fn()
488-
const onResourcesChanged = vi.fn(() => unsubscribe)
489-
const hub = initHub({
490-
base: DEVFRAMES_HUB_BASE,
491-
auth: false,
492-
host: '127.0.0.1',
493-
ws: { port: wsPort },
494-
mcp: {
495-
resourceTemplates: [{
496-
uriTemplate: 'hub://frame/{id}',
497-
name: 'frame',
498-
list: () => [{ uri: 'hub://frame/alpha', name: 'alpha' }],
499-
read: params => ({ json: { id: params.id } }),
500-
}],
501-
onResourcesChanged,
502-
},
503-
devframes: [makeFrame('alpha')],
504-
})
505-
506-
try {
507-
await hub.ready
508-
expect(await callMcp(hub, 'resources/templates/list')).toContain('"uriTemplate":"hub://frame/{id}"')
509-
expect(onResourcesChanged).toHaveBeenCalledWith(expect.any(Function), await hub.context)
510-
expect(await callMcp(hub, 'resources/list')).toContain('"uri":"hub://frame/alpha"')
511-
expect(await callMcp(hub, 'resources/read', { uri: 'hub://frame/beta' })).toContain('\\"id\\": \\"beta\\"')
512-
}
513-
finally {
514-
await hub.close()
515-
}
516-
expect(unsubscribe).toHaveBeenCalledTimes(1)
517-
})
518-
})
519-
520518
it('aggregate MCP omitted: mounts once a mounted frame exposes agent tools', async () => {
521519
const wsPort = await getPort({ port: 18233, host: '127.0.0.1' })
522520
const hub = initHub({ base: DEVFRAMES_HUB_BASE, auth: false, host: '127.0.0.1', ws: { port: wsPort }, devframes: [makeFrame('alpha')] })

0 commit comments

Comments
 (0)