Skip to content

test: add regression test for issue #2001 (progress related_request_id)#2747

Open
kaushik701 wants to merge 4 commits into
modelcontextprotocol:mainfrom
kaushik701:test/fix-2001-progress-related-request-id
Open

test: add regression test for issue #2001 (progress related_request_id)#2747
kaushik701 wants to merge 4 commits into
modelcontextprotocol:mainfrom
kaushik701:test/fix-2001-progress-related-request-id

Conversation

@kaushik701
Copy link
Copy Markdown

Adds a dedicated regression test file for issue #2001.

Context.report_progress() was silently dropping progress notifications in stateless HTTP / SSE transports because send_progress_notification() was called without related_request_id. The fix (already in main via mcpserver/context.py) passes related_request_id=self.request_id, consistent with send_log_message().

This test file covers:

  • The primary regression: related_request_id is forwarded on every call
  • Edge case: no progress_token -> notification is skipped (no-op)
  • Edge case: integer progress_token (e.g. 0) works correctly

Closes #2001

Motivation and Context

How Has This Been Tested?

Breaking Changes

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

…ss related_request_id)

Adds a dedicated regression test file for issue modelcontextprotocol#2001.

Context.report_progress() was silently dropping progress notifications
in stateless HTTP / SSE transports because send_progress_notification()
was called without related_request_id. The fix (already in main via
mcpserver/context.py) passes related_request_id=self.request_id,
consistent with send_log_message().

This test file covers:
- The primary regression: related_request_id is forwarded on every call
- Edge case: no progress_token -> notification is skipped (no-op)
- Edge case: integer progress_token (e.g. 0) works correctly

Closes modelcontextprotocol#2001
Copy link
Copy Markdown

@StantonMatt StantonMatt left a comment

Choose a reason for hiding this comment

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

I checked this against the current test tree and I think this PR mostly duplicates coverage that is already on main.

The main regression for #2001 is already covered by tests/server/mcpserver/test_server.py::test_report_progress_passes_related_request_id, including the related_request_id assertion. The integer progress_token=0 case is already covered by tests/issues/test_176_progress_token.py::test_progress_token_zero_first_call, also with related_request_id. The no-token no-op behavior is covered at the interaction level by tests/interaction/mcpserver/test_context.py::test_report_progress_without_a_progress_token_sends_nothing.

I verified this branch and the overlapping tests locally on 0d3e9ef:

  • uv run --frozen pytest tests/issues/test_2001_progress_related_request_id.py tests/server/mcpserver/test_server.py::test_report_progress_passes_related_request_id tests/issues/test_176_progress_token.py -q -> 5 passed
  • uv run --frozen ruff check tests/issues/test_2001_progress_related_request_id.py tests/server/mcpserver/test_server.py tests/issues/test_176_progress_token.py
  • uv run --frozen ruff format --check tests/issues/test_2001_progress_related_request_id.py tests/server/mcpserver/test_server.py tests/issues/test_176_progress_token.py
  • uv run --frozen pyright tests/issues/test_2001_progress_related_request_id.py

So the tests pass, but I do not see much new coverage from adding this extra issue file as-is. If maintainers want an issue-specific file for #2001, it may be worth trimming it to only a missing case, otherwise this can probably be closed in favor of the existing regression and interaction tests.

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.

Progress notifications not delivered via SSE in stateless HTTP mode

2 participants