feat(mcp-ts): added mvp v2 prompts, resources methods with paginationToken - #4296
poshinchen wants to merge 2 commits into
Conversation
|
Assessment: Comment Clean, well-documented MVP for MCP v2 prompts/resources. The migration to Review Categories
Nice work on the thorough JSDoc and the lazy-connect |
1a581cd to
855cc69
Compare
|
Updated assessment: Approve (non-blocking nits) Re-reviewed after the description was filled in and after cross-checking the Python SDK. This is a faithful parity port of
Remaining items are all non-blocking: (1) an optional one-line Nice, tight PR with strong test coverage and clear parity rationale. |
|
@strandly-the-agent review the changes and parity between python: #3984 |
strandly-the-agent
left a comment
There was a problem hiding this comment.
TL;DR — parity with Python holds on names, params and wire shape; one deliberate divergence worth a decision. No blockers. (Reviewed only the PR-authored commit 855cc69; the rest of the file list is #4093, which this is stacked on.)
Parity vs #3984 / mcp_client.py
- ✅
paginationToken↔pagination_token; wire is{ cursor }on both sides (paginationParams()↔_pagination_params()→PaginatedRequestParams(cursor=…)). Nonext_cursorcompat shim needed in TS — single vendor line, camelCase. - ✅ Method names ↔
*_sync;promptId↔prompt_id;readResource(string | URL)↔read_resource_sync(AnyUrl | str); raw vendor result types returned on both sides. ⚠️ No-token semantics diverge. Pythonlist_prompts_sync()returns page 1 withnext_cursor(caller walks). TSlistPrompts()hands the vendor clientundefined, which aggregates all pages, deletesnextCursor, and throwsListPaginationExceededatlistMaxPages(default 64) — a cap Python's own loop doesn't have. And since page 1 is never returned raw, a TS caller has no way to start the per-page walk the vendor suggests as the fallback. Inline thread with a suggestion.- ⚪
args?optional vs Python required — Python's docstring already says "Optional arguments", so TS optional is the better spelling; fine as-is.
5 new public methods + an exported type and no api/needs-review label (#4093 has one) — flagging for the API owner.
✅ Verified (head 855cc69, on top of #4093 fd2e762)
npm run build✅ ·tsc --noEmitsrc + test/integ ✅ ·eslint✅ ·prettier --check✅- Unit
vitest --project unit-node src/mcp: 164/164 ✅ - Integ
test/integ/mcp/mcp.test.node.ts -t "prompts and resources": 1/1 ✅ against the real stdio fixture server (prompt render, static resource, templated resource viaURL). - Vendor behaviour read from
@modelcontextprotocol/client@2.0.0dist/index.mjs:listPrompts(~L3558) →params?.cursor !== undefined⇒ single raw page; else_serveFromCachethen_listAllPages(~L3657), which throwsSdkError(ListPaginationExceeded)whenpages >= listMaxPagesanddelete acc.nextCursoron success. new Client(...)atclient.ts:240-262passes nolistMaxPages, so the vendor default 64 applies and isn't user-configurable.new URL('file:///a b.txt').toString()→file:///a%20b.txt; string input is passed through untouched, so onlyURLcallers get normalised — same posture as Python'sAnyUrl.
Questions (non-blocking)
- Is aggregate-by-default the intended TS contract (matches
listTools()), accepting the divergence from Python's page-1 default? If yes, the JSDoc caveat plus alistMaxPagespassthrough onMcpClientOptionsis the minimum; if no, returning the raw first page on no-token restores parity exactly. - Should the vendor result types (
ListPromptsResult,GetPromptResult, …) be re-exported fromstrands-ts/src/mcp/index.tsso callers can annotate without importing@modelcontextprotocol/clientdirectly? Python users get them frommcp.types, so there's an argument either way.
Appendix — non-blocking (3)
- ⚪ Server without
prompts/resourcescapability: vendor returns{ prompts: [] }with aconsole.debug, where Python surfaces the server's method-not-found error. Vendor behaviour, just a known asymmetry. - ⚪
McpListOptionsvs the existingMcpListToolsOptions(which has nopaginationToken): slightly asymmetric naming, butlistToolsreturnsMcpTool[]rather than a paginated result, so a shared type wouldn't fit. - ⚪ Stacked on #4093 — GitHub's file list shows both; collapses to 6 files once #4093 merges.
855cc69 to
94e6055
Compare
|
@poshinchen Did the parity pass against Signature parity matrix
Deliberate divergences — all fine:
One thing worth a quick confirm (non-blocking): Also confirmed the wire mapping is correct: |
Description
The TypeScript
McpClientexposes tools and nothing else. Python already has prompt and resource methods on main (list_prompts_sync,get_prompt_sync,list_resources_sync,read_resource_sync,list_resource_templates_sync), and the only TypeScript route to the same data is the rawclientgetter.Therefore, this PR adds the five missing methods.
The methods return the MCP library's result objects unchanged, as Python does. Every list method takes a
paginationTokenparameter, the TypeScript spelling of Python'spagination_token(PR). With no token the client fetches and aggregates every page, the same waylistToolsalready drains pages. With a token it fetches one page, and the result'snextCursorfeeds the next call.NOTE: The aggregation is a deliberate divergence from Python, whose list methods return page one plus a token. The MCP client caps the aggregate walk at 64 pages and throws
ListPaginationExceededpast that, soMcpClientOptionsgains alistMaxPagespassthrough as the escape hatch for large catalogs (0 disables the cap). The cap is stated into the docstrings.Public API Changes
Added:
listPrompts,getPrompt,listResources,readResource, andlistResourceTemplatesonMcpClient, theMcpListOptionsinterface carryingpaginationToken, and alistMaxPagesoption onMcpClientOptionscapping the no-token aggregation:The methods connect lazily. When an earlier connect attempt failed with
continueOnErrorset, they throw the same errorcallTooldoes.Related Issues
Closes #3273. Part of #1659.
Unblocks #3277 (completions).
Documentation PR
Site docs for prompts and resources follow with the MCP docs refresh noted in #4093.
Type of Change
New feature
Testing
hatch run prepareChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.