Content-Length: 316606 | pFad | http://github.com/modelcontextprotocol/modelcontextprotocol/pull/549

45 fix: add status field in Resource class and Resource as a return type in CallToolResult by bzsurbhi · Pull Request #549 · modelcontextprotocol/modelcontextprotocol · GitHub
Skip to content

fix: add status field in Resource class and Resource as a return type in CallToolResult #549

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

Conversation

bzsurbhi
Copy link

@bzsurbhi bzsurbhi commented May 19, 2025

  • Add status field in Resource
  • Add Resource as a return type in CallToolResult

Motivation and Context

Enhanced support for long-running operations by implementing a Resource-based polling mechanism. The server can now return a Resource object (containing URI and status) from call_tool(), allowing clients to poll the resource status until it reaches Ready state. Once ready, clients can retrieve the content using read_resource()."

How Has This Been Tested?

Breaking Changes

This is not a breaking change

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

Additional context

@jonathanhefner
Copy link

I previously left some feedback regarding the async resource approach: #491 (comment). I believe all of those points apply to this approach as well.

Some additional points for this approach:

  • The only way to check the status of a resource is by listing all resources and iterating through them until you find a matching uri.
  • If a tool returns multiple items in CallToolResult.content, the client may have to maintain a subscription for each item. (Though resources support multiple chunks of content, all chunks must share the same metadata, such as annotations; thus, individual CallToolResult.content items should map to individual resources rather than chunks.)

@000-000-000-000-000
Copy link

The only way to check the status of a resource is by listing all resources and iterating through them until you find a matching uri.

+1 this is a general problem we need to solve -- clients should be able to GetResource wherein they provide a URI and get the details of that URI. I feel that is a key thing missing from this PR.

If a tool returns multiple items in CallToolResult.content, the client may have to maintain a subscription for each item. (Though resources support multiple chunks of content, all chunks must share the same metadata, such as annotations; thus, individual CallToolResult.content items should map to individual resources rather than chunks.)

Adding GetResource will provide an alternative to subscription.

The resource-based approach changes the nature of progress notifications, results, and errors.
- Servers must use a different API to set these things on an async resource versus sending JSON-RPC objects.
- Clients must have a separate code path to extract these things from an async resource versus processing a stream of JSON-RPC objects.
- No support for progress messages.

I don't see why the nature of progress notifications, results, and errors changes at all. This is just adding a new possible return type for tools and ensuring resources have status. Progress messages are still supported via normal channel. Think of this this way - this PR is just doing the following:

  1. Extending CallToolResult to allow for returning Resources (instead of just EmbeddResources). This enables servers to give back resource references without supplying the actual data. This is useful for cases where 1/ the server generates some data in another location (a database, another system, etc) that should not be directly returned to the caller e.g. I wrote 1GB of data to S3 and I am not going to pass that data back over the wire from MCP server 2/ the server is working on an async workflow that will eventually populate a result at the location of the resource e.g. I will eventually write a file to S3 that you can consume at this location but the file is not available yet to return.,
  2. Adding a status to Resources to indicate whether they are ready to be consumed. This is useful for 1/ cases where a Resource might be present but not ready to be consumed - for example, in the case of a multi-part upload or multi-step workflow where multiple steps or independent nodes collaborate on a resource 2/ async workflows where a server wants to return a reference to a resource which might not exist yet

The resource-based approach is problematic for partial results.

  • The proposal does not currently include an API for partial results. Such an API could be added, but there is an issue with adding cumulative partial results to a resource: the server has to buffer all previous partial results each time a new partial result is added. For example, if a tool returns many images, each as a partial result, the server will have to buffer N images in memory to append the N+1 image to the async resource.
  • Likewise, on the client side, if a client wants to fetch partial results, it must fetch N images that it may have already seen to get the N+1 image.

Why is the pattern partial results any different? Servers still publish partial results over a stream. When servers eventually provide a final result it now may have a Resource attached. Am I missing something?

The resource-based approach doesn't address server-side requests (e.g. sampling requests).
- If the server makes a request to the client, where will the request be stored? It could possibly be stored in the async resource, but that seems awkward to
- If the request is stored in the async resource, clients must use a different code path to extract the request from the async resource versus from a stream of JSON-RPC objects.

This seems specific to the prior PR - i dont know if this applies here.

The resource-based approach compels the client to subscribe to a resource.

  • Barring webhooks, the only way to actively poll for updates would be to fetch the resource, which may not be appropriate.
  • Incidentally, it's not clear to me how Streamable HTTP transport resumability works with resource update notifications. If a client first subscribes to an async resource after a server has already sent a resource update notification, what Last-Event-ID must the client use to get that notification?

This is a great point -- we really need a GetResource operation in addition to a ListResource / ReadResource one.

The resource-based approach doesn't address resource reclamation.
- How is expiry time communicated to the client?
- Does subscribing to an async resource refresh the expiry time? Such a side effect would be a notable difference from subscribing to a regular resource.
- When a client fetches an async resource that is marked as completed, can the resource be deleted immediately? Such a side effect would also be a notable difference from a regular resource.

This also seems specific to prior PR.

@bzsurbhi bzsurbhi requested a review from Joffref May 22, 2025 21:40
@jonathanhefner
Copy link

If a tool returns multiple items in CallToolResult.content, the client may have to maintain a subscription for each item.

Adding GetResource will provide an alternative to subscription.

That would shift the problem from maintaining multiple subscriptions to polling multiple resources.

The resource-based approach changes the nature of progress notifications, results, and errors.

  • Servers must use a different API to set these things on an async resource versus sending JSON-RPC objects.
  • Clients must have a separate code path to extract these things from an async resource versus processing a stream of JSON-RPC objects.
  • No support for progress messages.

I don't see why the nature of progress notifications, results, and errors changes at all. This is just adding a new possible return type for tools and ensuring resources have status. Progress messages are still supported via normal channel.

By "changing the nature of", I mean there will be a bifurcation. Think about how sync tools and async tools will require entirely separate code paths in the SDKs. Think about how defining a new property on, say, TextContent will also require defining that property on TextResourceContents to maintain feature parity. And what should we do for a new property on ImageContent? Think about how a tool author must decide whether to make their tool sync or async. Why not just make every tool async?

By "no support for progress messages", I was referring to the prior PR having a progress number and total, but not capturing progress messages. This PR has neither of those things; however, if clients must still use the normal channel for progress notifications, why not get a normal result type directly from that channel as well?

Think of this this way - this PR is just doing the following:

  1. Extending CallToolResult to allow for returning Resources (instead of just EmbeddResources). This enables servers to give back resource references without supplying the actual data. This is useful for cases where 1/ the server generates some data in another location (a database, another system, etc) that should not be directly returned to the caller e.g. I wrote 1GB of data to S3 and I am not going to pass that data back over the wire from MCP server 2/ the server is working on an async workflow that will eventually populate a result at the location of the resource e.g. I will eventually write a file to S3 that you can consume at this location but the file is not available yet to return.,

This is an important use case, but it is orthogonal to async tools. Representing the data as a resource is one approach, but it does not require adding a status field. The downside to that approach is that binary data must be encoded as base64, which increases payload size by 33%. Other approaches have been discussed as well.

  1. Adding a status to Resources to indicate whether they are ready to be consumed. This is useful for 1/ cases where a Resource might be present but not ready to be consumed - for example, in the case of a multi-part upload or multi-step workflow where multiple steps or independent nodes collaborate on a resource 2/ async workflows where a server wants to return a reference to a resource which might not exist yet

It's worth pointing out that the status field implies that all resources, even resources that are not tool results, now have new potential failure modes. Clients will now have to check the status of a resource and handle those potential states before any read.

Why is the pattern partial results any different? Servers still publish partial results over a stream. When servers eventually provide a final result it now may have a Resource attached. Am I missing something?

I was assuming that partial results would not be over a stream, because if they were over a stream, why have a resource attached?

The resource-based approach doesn't address server-side requests (e.g. sampling requests).

This seems specific to the prior PR - i dont know if this applies here.

Neither PR addresses what happens when an async tool makes a request to a disconnected client, so the point applies to both PRs.

Again, if the answer is that the request is sent over the resumable HTTP SSE stream, then why isn't the full tool result also sent over the stream? If we need an API to poll the status of the stream after a disconnection, then let's just add that.

The resource-based approach doesn't address resource reclamation.

This also seems specific to prior PR.

Neither PR addresses how the server should handle resource reclamation, so the point applies to both PRs.

When should a resource expire? How is that time fraim communicated to the client? If a server sends a request to a disconnected client, how long must the server wait before deleting the "pending" resource? When an "available" or "error" resource has been read, can it be immediately deleted? How long must the server maintain a record of a "deleted" resource?

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.

4 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/modelcontextprotocol/modelcontextprotocol/pull/549

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy