Skip to content

Feature/resource progress #800

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

davemssavage
Copy link

Experimenting with the idea of passing back resources with progress notification

Motivation and Context

Tinkering to see if this idea works modelcontextprotocol/modelcontextprotocol#549 (comment)

How Has This Been Tested?

Unit testing only so far

Breaking Changes

None expected

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

Quick patch to test idea will need tests to prove it works

@hesreallyhim
Copy link

Looks nice, it sounds from your language like maybe this is a draft?

@davemssavage davemssavage marked this pull request as draft May 24, 2025 17:39
@davemssavage
Copy link
Author

Yep, couple of tweeks to make, forgot to tag it as draft

@@ -173,6 +179,7 @@ async def send_progress_notification(
progress: float,
total: float | None = None,
message: str | None = None,
# TODO decide whether clients can send resource progress too?

Choose a reason for hiding this comment

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

hey i'm working on resources stuff right now so i hope you don't mind if I'm commenting as you work, what is the notion of a client resource in general? i thought they were owned by the server

Choose a reason for hiding this comment

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

the protocol indicates resources are offered by servers to clients

Choose a reason for hiding this comment

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

to me a client progress notification would not be about a resource, but some other process.

Copy link
Author

@davemssavage davemssavage May 24, 2025

Choose a reason for hiding this comment

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

Yep I think that makes, I think that's what I guessed too but hadn't got around to validating my hunch, I might tidy this TODO up if there really is no use case for a client resource, it was more a reminder to think about it before finishing off the patch

Choose a reason for hiding this comment

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

well client progress notifications are within the spec, it's just they're not about resources, they're about long-running process:

The Model Context Protocol (MCP) supports optional progress tracking for long-running operations through notification messages. Either side can send progress notifications to provide updates about operation status.

Not sure what the use case is or if it's relevant to your goal, but yeah.

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.

2 participants