Skip to content

Remove customizable separators; improve resource separator #526

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
May 21, 2025

Conversation

jlowin
Copy link
Owner

@jlowin jlowin commented May 21, 2025

This is technically a breaking change because it removes user-customizable separators when mounting servers under a prefix. However, these customizable prefixes caused a problem for resources in particular because the resource prefix became part of the URI protocol, which has even more restrictions than other parts of the URI (like the path). For example, if you mounted a server with the prefix "sub_server" it would fail because protocols can't have underscores.

This PR modifies resource prefixes to become part of the path not the protocol. In other words, protocol://path/to/resource becomes protocol://prefix/path/to/resource in the new setup, which is far more natural. On the plus side this improves our support for arbitrary prefix mounts.

As an extension, customziable separators are no longer supported and will give a deprecation warning.

@Copilot Copilot AI review requested due to automatic review settings May 21, 2025 00:33
@github-actions github-actions bot added documentation Improvements or additions to documentation tests labels May 21, 2025
@jlowin jlowin added enhancement New feature or request breaking change and removed documentation Improvements or additions to documentation tests labels May 21, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes support for custom separators when mounting or importing servers and updates resource prefixes to be part of the path rather than the protocol.

  • Moves resource prefix handling into helper functions (add_resource_prefix, remove_resource_prefix, has_resource_prefix)
  • Deprecates *_separator parameters with warnings and removes validation via Pydantic
  • Updates tests, docs, and mounting/import logic to use the new protocol://prefix/path format

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/server/test_server.py Adds parametrized tests for new resource prefix helpers
tests/server/test_openapi.py Updates expected prefixed URIs in OpenAPI tests
tests/server/test_mount.py Removes custom separator usage and updates assertions
tests/server/test_import_server.py Updates import tests to new prefix format
tests/deprecated/test_mount_separators.py Adds deprecation warning tests for separator args
tests/deprecated/test_deprecated.py Mirrors deprecation tests for mount/import
src/fastmcp/server/server.py Implements add/remove/has_resource_prefix, deprecates separators, updates mount/import logic
docs/servers/composition.mdx Updates documentation to reflect new prefix format and removes custom separator section
Comments suppressed due to low confidence (1)

src/fastmcp/server/server.py:1315

  • [nitpick] The variable name match shadows the built-in re.match function's return type. Consider renaming it to m or match_obj for clarity and to avoid confusion.
match = re.match(r"^([^:]+://)(.*?)$", uri)

@github-actions github-actions bot added documentation Improvements or additions to documentation tests labels May 21, 2025
@jlowin jlowin merged commit e0e9511 into main May 21, 2025
5 checks passed
@jlowin jlowin deleted the remove-separators branch May 21, 2025 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change documentation Improvements or additions to documentation enhancement New feature or request tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant