Skip to content

Fix DRF Integration Body Processing and API Failure Issues #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

zacharypodbela
Copy link
Contributor

@zacharypodbela zacharypodbela commented Aug 8, 2025

Summary

This PR fixes critical bugs in the Django REST Framework integration and adopts a cleaner architectural approach using composition over method patching.

Bugs Fixed

1. Body Parameter Mapping Issue

  • Github Issue Link: Closes When using DRF decorators, tool schema provided by MCP Server does not match what DRF views can actually process #39
  • Problem: MCP tool calls made with data nested inside a body property (e.g., {"body": {"title": "...", "content": "..."}}) were not being properly extracted for DRF serializers
  • Symptom: DRF validation errors showing required fields as missing, even though they were present in the MCP call
  • Root Cause: The request initialization flow was returning the original MCP JSON-RPC request instead of the fabricated request containing the extracted body data
  • Fix: Proper data flow from MCP body parameters to DRF serializer data

2. Global ViewSet Patching Breaking Normal HTTP Requests

  • Github Issue Link: Closes drf_publish_list_mcp_tool() breaks GET requests to DRF ListAPIView #32
  • Problem: The initialize_request method was being globally patched on ViewSet classes, affecting ALL requests to those ViewSets, including normal API requests
  • Symptom: Normal HTTP API requests (e.g., curl localhost:8000/posts/) failed with AttributeError: 'WSGIRequest' object has no attribute 'original_request'
  • Root Cause: Method patching modified the class permanently, expecting MCP-specific request attributes on normal Django requests
  • Fix: MCP requests conform to the same interface expectations as normal API requests and patching has been removed all together

Technical Strategy

Instead of trying to bypass or hack DRF's request creation, we pre-process the MCP request into exactly what DRF expects (a simple Django HttpRequest) and then let DRF run its full lifecycle using its intended mechanisms.

The benefits of this approach are:

  1. Let DRF's unmodified initialize_request do its job naturally
  2. DRF creates a proper rest_framework.request.Request with all the bells and whistles
  3. Full DRF lifecycle runs: parsers, authentication, permissions, etc.

The potential drawbacks are:

  1. We are using a Factory from DRF's testing library, which is probably not intended to be used for runtime purposes. In future version of DRF this may break (as the factory is likely to be extended to suit new testing needs).

Demo / Testing

Here are screenshots of my really simple Blog Post app (see code here) working with Claude Desktop:
LIST
CREATE
UPDATE
DESTROY
LIST_FINAL

Here's my really simple Blog Post app normal API calls working:
Standard_API_Call

All tests in the bird_counter example app pass: Screenshot 2025-08-07 at 11 04 03 PM

@omarbenhamid
Copy link
Owner

@Artui this PR proposes a clean way as it seems to manage the request, but removes part of the logic you coded. Can you please confirm that it does not break your use case ?

@omarbenhamid omarbenhamid self-assigned this Aug 9, 2025
@Artui
Copy link
Contributor

Artui commented Aug 9, 2025

@Artui this PR proposes a clean way as it seems to manage the request, but removes part of the logic you coded. Can you please confirm that it does not break your use case ?

Yeah it will

@Artui
Copy link
Contributor

Artui commented Aug 9, 2025

@Artui this PR proposes a clean way as it seems to manage the request, but removes part of the logic you coded. Can you please confirm that it does not break your use case ?

But I can keep using the older version for now

@zacharypodbela
Copy link
Contributor Author

@Artui what use case would this break? If you share more info I can try and figure out a solution that continues to support it, while fixing the 2 issues.

@omarbenhamid
Copy link
Owner

Well to give you an opinion on the topic :

  • There two requests that live in parallel : the Django Request made by the MCP Client to the MCP Server, and the "Fake" DRF request which is transmitted with the view. The two have to be isolated and that's the reason why the DRF Request Wrapper exists: to fake a request to DRF without giving access to the MCP Request from client to server.

I don't have enough expertise on DRF internal to judge but I noticed that many issues are solved by giving somehow access to the original request, so maybe we should go this way. I appreciate about this MR that it uses APIRequestFactory which is test class, and that's an issue, but it is maintained by DRF team as DRF evolves and then keep request preparation right so I would tend to merge it.

@Artui

But I can keep using the older version for now
We should try not to accept regression, so if you have time to give it a try and provide some stacktrace or more insight to make this more robust, that would be perfect.

@Artui
Copy link
Contributor

Artui commented Aug 10, 2025

@Artui what use case would this break? If you share more info I can try and figure out a solution that continues to support it, while fixing the 2 issues.

I was attempting to fix the issue of having some arbitrary metadata fields on a django request object passed into MCP tools. Our views depend on some request.<field_name> properties being there, but because of how the tool wrapper is built - this call here actually will "reset" the WSGI request that is passed into MCP view
image

Here's a simplistic diagram that explains the issue
image

@zacharypodbela
Copy link
Contributor Author

Ah that makes sense.

In your diagram you said "without data from middleware" -- Can you share more details on what data the middleware adds when an API call comes in that is missing when the Request is passed in via the Tool flow?

@ubongpr7
Copy link

Please i will like to ask, i get an error saying that viewset does not have attribute 'action' when i publishish and try the viewsets endpoint with my agent, again what is the fix for the missing have not been able to see that here. Is it part of a new release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants