Skip to content

fix: GET request /mcp #1

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

Merged
merged 3 commits into from
Aug 13, 2025

Conversation

trueberryless
Copy link
Contributor

@trueberryless trueberryless commented Aug 11, 2025

People (myself included) expect to see something when calling https://mcp.docs.astro.build/mcp in their browser (which is a GET request) because they initially do not understand that a MCP server works with POST requests.

I therefore added a redirect rule which redirects all HTTP methods to the Astro docs with status 303 ("See other"). But because Netlify edge functions take precedence over redirects, the MCP Server with HTTP method POST still works unaffected 🙌

Copy link

netlify bot commented Aug 11, 2025

‼️ Deploy request for docs-mcp rejected.

Name Link
🔨 Latest commit 2965e33

@trueberryless trueberryless marked this pull request as draft August 11, 2025 15:57
@trueberryless trueberryless marked this pull request as ready for review August 11, 2025 15:59
@trueberryless trueberryless marked this pull request as draft August 11, 2025 16:13
@trueberryless trueberryless marked this pull request as ready for review August 11, 2025 16:14
@trueberryless trueberryless marked this pull request as draft August 11, 2025 16:22
path: "/mcp", // Intercept POST /mcp
method: "POST",
},
];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Netlify unfortunately does not support an array for the config. To allow the static index file to be served AND redirect GET requests to /mcp to / we would need to do some cheeky hacks:

if (method === "GET" && pathname === "/") {
  return fetch(new URL("/", req.url)); // Fetch the static asset from Netlify's CDN
}

I'm not sure if this is performant or not, maybe there is a better solution 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Much cleaner solution found 2e08d68

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can just return nothing and it will respond from the origin

@trueberryless trueberryless force-pushed the trueberryless-patch-1 branch from 8329f2a to 2e08d68 Compare August 11, 2025 16:36
@trueberryless trueberryless marked this pull request as ready for review August 11, 2025 16:41
@trueberryless
Copy link
Contributor Author

trueberryless commented Aug 11, 2025

Choosing 303 as redirect status code because it indicates "see other" (when GET request happens), instead of "temp redirect" as 302 does: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status/303

Copy link
Collaborator

@ascorbic ascorbic left a comment

Choose a reason for hiding this comment

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

I'm not able to check right now but I don't think this will work. The redirect will apply to POST requests too

ascorbic

This comment was marked as duplicate.

@ascorbic
Copy link
Collaborator

ascorbic commented Aug 12, 2025

Someone with access to the Netlify org should be able to approve the deploy requests here https://app.netlify.com/projects/docs-mcp/deploys
That way you can test the deploy preview

@trueberryless
Copy link
Contributor Author

I'm not able to check right now but I don't think this will work. The redirect will apply to POST requests too

This is also one of my concerns, in the Netlify docs they do not state anything about HTTP methods when using redirect, MDN just states

The method to retrieve the redirected resource is always GET.

Which I didn't quite understood right, however Wikipedia makes it quite clear then:

The response to the request can be found under another URI using the GET method. When received in response to a POST (or PUT/DELETE), the client should presume that the server has received the data and should issue a new GET request to the given URI.

I wanted to avoid having a mcp.html with this content:

<!doctype html><meta http-equiv="refresh" content="0;url=https://docs.astro.build/en/guides/build-with-ai/">

as this would mean Netlify has to send HTML to the client instead of redirecting directly on server-side, but maybe that's the only solution here... 🤔

@trueberryless
Copy link
Contributor Author

However, I would really like to test it out in the preview, as the Netlify docs say:

Note that for each request, Netlify processes edge functions before redirects.

https://docs.netlify.com/manage/routing/redirects/overview/#rule-processing-order
https://docs.netlify.com/build/edge-functions/declarations/#declaration-processing-order

Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Deploy preview: https://deploy-preview-1--docs-mcp.netlify.app/

The redirect appears to be working, and POST-ing does too?

~ ❱ curl -IL https://deploy-preview-1--docs-mcp.netlify.app/mcp
HTTP/2 303
content-type: text/html; charset=UTF-8

~ ❱ curl -IL -X 'POST' https://deploy-preview-1--docs-mcp.netlify.app/mcp
HTTP/2 500 
content-type: application/json

Although I’m assuming a 500 error for my empty POST is expected. Someone with an AI editor would need to test to be sure it’s working correctly.

Maybe Netlify’s static redirects like this only handle GET requests? Not sure.

@trueberryless
Copy link
Contributor Author

trueberryless commented Aug 12, 2025

Thank you very much for triggering the deployment preview! 🙌

Maybe Netlify’s static redirects like this only handle GET requests? Not sure.

No, that was my mistake, they handle all methods, but edge functions take precedence over redirects

It's mentioned after the code block here: https://docs.netlify.com/manage/routing/redirects/overview/#rule-processing-order

And after the ordered list here: https://docs.netlify.com/build/edge-functions/declarations/#declaration-processing-order

Co-authored-by: Adam Matthiesen <[email protected]>
Co-authored-by: Chris Swithinbank <[email protected]>
@trueberryless trueberryless force-pushed the trueberryless-patch-1 branch from 69367cc to 37531e0 Compare August 13, 2025 11:13
Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Confirmed the preview deploy is still working by adding it as an MCP server in VS Code. Chat sessions were able to successfully query the server and receive docs content.

Thanks @trueberryless 🙌

@ematipico ematipico merged commit 3846aa6 into withastro:main Aug 13, 2025
4 checks passed
@trueberryless trueberryless deleted the trueberryless-patch-1 branch August 14, 2025 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants