Skip to content

Conversation

@MuhammadHassaanArain
Copy link
Contributor

Summary

Adds MCP Resources support (list, read, subscribe) to complete protocol implementation.

Changes

  • list_resources() - discover available resources
  • read_resource(uri) - read resource content
  • subscribe_resource(uri) - stream resource updates

Resources enable agents to access files, databases, and data streams per the MCP spec.

Details

  • Non-breaking change
  • Follows existing SDK patterns
  • Proper error handling and async/await
  • Delegates to ClientSession

Happy to add tests or docs if needed!

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 79 to +83
[tool.uv.workspace]
members = ["agents"]
members = [
"agents",
"checking",
]

Choose a reason for hiding this comment

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

P1 Badge Remove nonexistent workspace member checking

Adding "checking" to tool.uv.workspace.members introduces a build-time failure because the repository contains no checking/ package. Running uv or make commands now causes uv to error with “No such workspace member ‘checking’”, preventing dependency installs or packaging. This looks unrelated to the MCP resources feature and should be reverted or the directory added.

Useful? React with 👍 / 👎.

Comment on lines 78 to 93
"""Invoke a tool on the server."""
pass

@abc.abstractmethod
async def list_resources(self)-> ListResourcesResult:
"""List resources available on the server."""
pass
@abc.abstractmethod
async def read_resource(self, uri:str)-> ReadResourceResult:
"""Read the content of a resource by URI."""
pass
@abc.abstractmethod
async def subscribe_resource(self, uri:str, **kwargs) -> AsyncIterator[SessionMessage]:
"""Subscribe to resource updates."""
pass

Choose a reason for hiding this comment

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

P1 Badge New abstract resource methods break existing MCPServer subclasses

Introducing list_resources, read_resource, and subscribe_resource as abstract methods on MCPServer means every existing subclass must implement them. Classes like tests/mcp/test_prompt_server.FakeMCPPromptServer (and any user-defined servers) still lack these methods, so instantiating them now raises TypeError: Can't instantiate abstract class … and the current test suite will fail. Consider providing default implementations or updating all subclasses.

Useful? React with 👍 / 👎.

@seratch seratch marked this pull request as draft November 10, 2025 03:52
@seratch
Copy link
Member

seratch commented Nov 10, 2025

Thanks for sending this patch. However, we don't plan to add these methods for now because the core part of this SDK does not use MCP resources at all.

@seratch seratch closed this Nov 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants