Skip to content

fix: Resolve URIs #262

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 4 commits into
base: main
Choose a base branch
from

Conversation

denniskawurek
Copy link
Contributor

Motivation and Context

Currently, the URL resolving is broken under different circumstances when adding a base URL to the path.

There are quite a few issues regarding this in this repo, but also in the Spring AI repos. Example: #261

Basically, the main issue is that the resolveUri method works for the message endpoint, but not for the SSE endpoint. So an URL resolver for the SSE endpoint is added.

How Has This Been Tested?

See WebMvcSseCustomPathIntegrationTests

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Open questions

This PR is currently WIP, because for now it is a little bit unclear for how to handle the context path.
I guess this is not the same as the baseUrl, but the main path of the servlet to reach? As an example, for SSE we would get:

http://localhost + PORT + CONTEXT_PATH + BASE_URL + SSE

If this is correct, then some additional changes need to be made to support this.

@denniskawurek
Copy link
Contributor Author

denniskawurek commented May 26, 2025

I found some time to dig deeper into it and pushed changes which fix the behaviour for the Webflux and WebMvc transport providers.

One of the main issues is that the sse connection returns the message endpoint to the client. This endpoints needs to be the full path with the context under which the server is running.
The downside is that another parameter is needed in the constructors to take this endpoint.

Unfortunately this can't be part of the baseUrl, because it is not really part of the URL and will break the mapping.

However, I tested running a server with Spring-AI with configurations like:

spring.ai.mcp.server:
  type: ASYNC
  base-url: /mcp
  sse-endpoint: /connect

spring:
  webflux:
    base-path: /v1/api

and instead of spring.webflux for webmvc:

servlet:
 context-path: /v1/api

With this solution I can connect succesfully to such routes after applying small changes in the Spring - AI code.

A big clean up of the code is still needed, though.

Edit: Cleanup done and PR marked as ready for review.

@denniskawurek denniskawurek marked this pull request as ready for review June 1, 2025 18:08
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.

1 participant