Skip to content

feat: add remove_prompt() and remove_resource() for parity with remove_tool()#2333

Open
goingforstudying-ctrl wants to merge 4 commits intomodelcontextprotocol:mainfrom
goingforstudying-ctrl:fix-issue-2331-remove-prompt-resource
Open

feat: add remove_prompt() and remove_resource() for parity with remove_tool()#2333
goingforstudying-ctrl wants to merge 4 commits intomodelcontextprotocol:mainfrom
goingforstudying-ctrl:fix-issue-2331-remove-prompt-resource

Conversation

@goingforstudying-ctrl
Copy link

Summary

MCPServer exposes remove_tool(name) but has no equivalent for prompts or resources.
This was part of the original ask in #711, which was closed when remove_tool()
landed — but the resource and prompt sides were never addressed.

Changes

Primitive add_* remove_*
Tool ✅ (existing)
Prompt (new)
Resource (new)
Resource Template (new)

API Additions

  • PromptManager.remove_prompt(name) — raises PromptError if not found
  • ResourceManager.remove_resource(uri) — raises ResourceError if not found
  • ResourceManager.remove_template(uri_template) — raises ResourceError if not found
  • MCPServer.remove_prompt(name) — thin wrapper delegating to the manager
  • MCPServer.remove_resource(uri) — thin wrapper delegating to the manager
  • MCPServer.remove_resource_template(uri_template) — thin wrapper
  • PromptError exception class in exceptions.py

Purely additive, no breaking changes.

Use Case

Multi-tenant / multi-instance deployments where the same server image serves
different clients. Users can now filter all primitives per-instance without
reaching into private internals.

Fixes #2331

…e_tool()

Add methods to dynamically remove prompts and resources, matching the
existing remove_tool() functionality:

- Add PromptError exception class for prompt operation errors
- Add PromptManager.remove_prompt(name) - raises PromptError if not found
- Add ResourceManager.remove_resource(uri) - raises ResourceError if not found
- Add ResourceManager.remove_template(uri_template) - raises ResourceError
- Add MCPServer.remove_prompt(name) - thin wrapper to prompt manager
- Add MCPServer.remove_resource(uri) - thin wrapper to resource manager
- Add MCPServer.remove_resource_template(uri_template) - thin wrapper

This enables multi-tenant/multi-instance deployments where servers need
to dynamically manage their registered primitives.

Fixes modelcontextprotocol#2331
Copy link

@rgoldstein1989 rgoldstein1989 left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up! A couple of things I noticed:

Missing tests

The project requires 100% code coverage (fail_under = 100 in CI), and CONTRIBUTING.md states "Add tests for new functionality." This PR will fail CI without tests. For reference, the existing remove_tool() has tests in both test_tool_manager.py and test_server.py covering:

  • Removing an existing item
  • Removing a non-existent item (error path)
  • Removing one item from multiple and verifying list_* results
  • Calling/reading a removed item through the client (verifying the protocol-level error)

The same coverage would be needed for remove_prompt, remove_resource, and remove_resource_template.

remove_resource uri type

The uri parameter on ResourceManager.remove_resource() is typed as str, but resources are stored via str(resource.uri) where resource.uri can be AnyUrl. If a caller passes an AnyUrl object, the lookup will fail silently (no match in the dict) and raise ResourceError even though the resource exists.

The fix is to accept AnyUrl | str and normalize before lookup, matching how the rest of the manager handles URIs:

def remove_resource(self, uri: AnyUrl | str) -> None:
    uri_str = str(uri)
    if uri_str not in self._resources:
        raise ResourceError(f"Unknown resource: {uri}")
    del self._resources[uri_str]

get_resource() already does this same str(uri) normalization at the top of the method.

- Fix remove_resource() to accept AnyUrl | str and normalize with str()
- Add comprehensive tests for remove_resource() and remove_template()
- Add tests for remove_prompt()
- All tests cover: removing existing, removing non-existent (error path),
  removing from multiple, and verifying protocol-level errors
@goingforstudying-ctrl
Copy link
Author

@rgoldstein1989 Thanks for the detailed review! I've addressed both issues:

  1. Added comprehensive tests for all new functionality:

    • : existing, non-existent, multiple prompts, protocol-level error
    • : existing, non-existent, AnyUrl support, multiple resources, protocol-level error
    • : existing, non-existent, multiple templates
  2. Fixed URI type: Now accepts and normalizes with before lookup, matching behavior.

All tests should now pass. Please take another look when you have time!

PR-Contributor added 2 commits March 23, 2026 03:39
Add comprehensive tests in test_server.py following the same pattern
as remove_tool tests:

- test_remove_prompt: basic removal from server
- test_remove_nonexistent_prompt: error path
- test_remove_prompt_and_list: verify client list_prompts reflects removal

- test_remove_resource: basic removal from server
- test_remove_nonexistent_resource: error path
- test_remove_resource_and_list: verify client list_resources reflects removal
- test_remove_resource_template: template removal
- test_remove_nonexistent_template: template error path

These tests verify the protocol-level behavior when resources/prompts
are removed, ensuring clients see the changes correctly.
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.

Add remove_prompt() and remove_resource() for parity with remove_tool()

2 participants