Skip to content

fix(server): pass typed list request messages to list middleware hooks - #5732

Merged
zzstoatzz merged 2 commits into
PrefectHQ:mainfrom
BlueX888:fix/prep-middleware-list-message-shape-drift
Nov 2, 2026
Merged

fix(server): pass typed list request messages to list middleware hooks#5732
zzstoatzz merged 2 commits into
PrefectHQ:mainfrom
BlueX888:fix/prep-middleware-list-message-shape-drift

Conversation

@BlueX888

@BlueX888 BlueX888 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Description

Closes #5710

Summary

The four typed list hooks on Middleware declare context.message as a request model: on_list_tools / on_list_resources / on_list_resource_templates / on_list_prompts are typed MiddlewareContext[mt.ListToolsRequest] / [mt.ListResourcesRequest] / [mt.ListResourceTemplatesRequest] / [mt.ListPromptsRequest] (fastmcp_slim/fastmcp/server/middleware/middleware.py:262-285). At runtime list_tools receives ListToolsRequest(method="tools/list"), but the other three list methods dispatched middleware with message={}, so a middleware written against the declared types can read context.message.method on tools/list yet hits AttributeError on resources/list, resources/templates/list, and prompts/list.

Why

list_tools builds the typed model at fastmcp_slim/fastmcp/server/server.py:835, but list_resources, list_resource_templates, and list_prompts passed a bare {} as the middleware message at server.py:976, :1111, and :1245.

Changes

  • fastmcp_slim/fastmcp/server/server.py: construct mcp_types.ListResourcesRequest(method="resources/list"), mcp_types.ListResourceTemplatesRequest(method="resources/templates/list"), and mcp_types.ListPromptsRequest(method="prompts/list") for the three middleware contexts, mirroring the tools path.
  • tests/server/middleware/test_middleware.py: add test_list_hooks_receive_typed_request_messages, asserting each of the four list hooks receives its declared request model with the corresponding method.

Testing

  • New test on the unpatched tree: AssertionError: on_list_resources received dict, expected ListResourcesRequest (1 failed).
  • New test with the fix: uv run pytest "tests/server/middleware/test_middleware.py::TestMiddlewareHooks::test_list_hooks_receive_typed_request_messages" → 1 passed.
  • uv run pytest tests/server/middleware/ → 325 passed.
  • uv run pytest tests/server/ → 9 failed, 4308 passed, 2 skipped; the 9 failures are pre-existing proxy/SSE/mount connection tests that fail identically on main without this change.
  • uv run prek run --all-files → all checks pass (ruff check, ruff format, ty, prettier, gitleaks, codespell).

Contribution type

  • Bug fix (simple, well-scoped fix for a clearly broken behavior)
  • Documentation improvement
  • Enhancement (maintainers typically implement enhancements — see CONTRIBUTING.md)

Checklist

  • This PR addresses an existing issue (or fixes a self-evident bug)
  • I have read CONTRIBUTING.md
  • I have added tests that cover my changes
  • I have run uv run prek run --all-files and all checks pass
  • I have self-reviewed my changes
  • If I used an LLM, it followed the repo's contributing conventions (not generic output)

@github-actions

This comment has been minimized.

Sign in to view
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-13T17:50:50.939951Z 7385941 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@marvin-context-protocol marvin-context-protocol Bot added bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. server Related to FastMCP server implementation or server-side functionality. labels Oct 13, 2026
BrickerP
BrickerP approved these changes Oct 14, 2026
View reviewed changes

@BrickerP BrickerP left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Approve. This is a clean alignment of the three list middleware paths with the typed contract the hooks already declare.

What was wrong

on_list_tools / on_list_resources / on_list_resource_templates / on_list_prompts are typed as MiddlewareContext[List*Request], and list_tools already builds mcp_types.ListToolsRequest(method="tools/list"). The other three list methods passed message={}, so middleware that reads context.message.method (or anything else on the request model) works for tools/list and raises AttributeError on resources/list, resources/templates/list, and prompts/list. Same drift shows up in StructuredLoggingMiddleware(include_payloads=True) as empty {} payloads with payload_type dict.

Fix

The three sites in server.py now construct the matching mcp_types.ListResourcesRequest / ListResourceTemplatesRequest / ListPromptsRequest with the same method strings already used on MiddlewareContext.method. That mirrors the tools path at ~L835 and matches the declared hook types — no new abstraction, no behavior change beyond the message shape middleware sees.

Tests

test_list_hooks_receive_typed_request_messages exercises all four list client calls and asserts both isinstance(..., List*Request) and message.method for each hook. That would have failed on main with the dict payload and covers the regression cleanly.

Notes

  • Issue #5710 is unassigned; this PR is the only open fix referencing it.
  • CI looks green across the Python matrix / integration / MCP conformance / static analysis (issue-link check skipped as expected for this labeling).
  • No concerns about constructor kwargs: these request models are used elsewhere the same way, and only method is required for the middleware surface here.

LGTM.

@asasemahmed

asasemahmed commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Checked this against main (e2fcc41) and the fix looks right. The new test fails on main (the hook receives a dict) and passes here, tests/server/middleware is green, and the typed models now also reach on_message and on_request for these three methods, not just the typed list hooks. That is a nice side effect, since those hooks are annotated with mt.Request. One visible change worth knowing: with StructuredLoggingMiddleware(include_payloads=True) the list payload goes from {} / payload_type dict to {"method":"resources/list","params":null} / ListResourcesRequest. That is the intended outcome, just a change in log output.

One limitation I think is worth a note, though not a blocker for this PR: the message is built with only method, so params is always None. On a paginated client call (I used a small page size and passed the returned nextCursor to a second tools/list), the second request reaches on_list_tools as {'method': 'tools/list'}, with no params.cursor. This is true on main for tools and now for the other three as well. So even though the hooks are typed as ListXRequest, a middleware that reads context.message.params.cursor cannot tell page 1 from page 2. The cursor is only available in _on_list_* in mcp_operations.py, and self.list_tools() is called before pagination is applied, so threading it through would be a separate, larger change. Perhaps a short comment near the three construction sites, or a follow-up issue, so the typed signature does not imply the cursor is available.

@BlueX888
BlueX888 force-pushed the fix/prep-middleware-list-message-shape-drift branch from 7385941 to 4ec278f Compare October 17, 2026 22:04
@BlueX888

BlueX888 commented Oct 17, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough check — the limitation you flagged is real, and I took the first option: each of the three construction sites now carries a short comment (new commit 4ec278f, branch also rebased onto the current main) noting that the message is synthesized with only method, so params stays None and pagination state (e.g. the client's nextCursor) is not visible to the list middleware hooks.

Your pagination repro matches the code path: self.list_*() runs before pagination is applied in _on_list_*, and the message is built with only method — so a middleware reading context.message.params.cursor cannot tell page 1 from page 2. As you noted, threading the cursor through would be a separate, larger change (it would have to move list construction after pagination or plumb the raw params in), so it is left out of this PR to keep the change minimal.

Validation on the new head: tests/server/middleware is green (286 passed, 39 skipped), ruff check/format and ty pass, and the full GitHub test matrix (Python 3.10–3.15, integration and smoke suites) is green. The one red check, static_analysis, is a pre-existing codespell complaint against docs/changelog.mdx ("unparseable", from the v4.1.1 changelog entry on current main) — not touched by this branch.

@BlueX888
fix(server): pass typed list request messages to list middleware hooks
6e90791 
on_list_resources, on_list_resource_templates, and on_list_prompts
received a bare empty dict as context.message, unlike on_list_tools
which receives mt.ListToolsRequest. Pass the declared request models
so middleware written against the typed hook signatures can read
message.method on every list method.
@BlueX888
docs(server): note list middleware request messages carry no params
0b6fc72 
The synthesized ListXRequest messages are built with only `method`, so
`params` stays None and pagination state (e.g. the client's nextCursor)
is not visible to list middleware hooks. Add a short comment at each
construction site so the typed hook signature does not imply the
cursor is available. Docs-only; no behavior change.
@BlueX888
BlueX888 force-pushed the fix/prep-middleware-list-message-shape-drift branch from 4ec278f to 0b6fc72 Compare October 26, 2026 01:18
zzstoatzz added a commit that referenced this pull request Nov 2, 2026
@zzstoatzz
verify #5732
599a24b
@zzstoatzz
zzstoatzz merged commit a87b0e4 into PrefectHQ:main Nov 2, 2026
20 checks passed
zzstoatzz pushed a commit that referenced this pull request Nov 9, 2026
@BlueX888 @zzstoatzz
fix(server): pass typed list request messages to list middleware hooks (
94cec24 
#5732)

(cherry picked from commit a87b0e4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@BrickerP BrickerP BrickerP approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. server Related to FastMCP server implementation or server-side functionality.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

list_resources/list_resource_templates/list_prompts dispatch middleware with message={} while list_tools sends a typed request model

4 participants

@BlueX888 @asasemahmed @BrickerP @zzstoatzz