Remove Server#notify_progress broadcast API
## Motivation and Context `Server#notify_progress` broadcasts progress notifications to all connected clients. Progress notifications are tied to a specific request's `progressToken` and have no meaningful broadcast use case. Neither the Python SDK nor the TypeScript SDK provides a server-level progress broadcast capability. The MCP specification requires that progress notifications only reference tokens provided in an active request: > Progress notifications MUST only reference tokens that: > - Were provided in an active request > - Are associated with an in-progress operation Ref: https://modelcontextprotocol.io/specification/2025-11-25/basic/utilities/progress Broadcasting progress to all clients does not comply with this requirement, since clients that did not originate the request have no matching `progressToken`. The introduction of `ServerSession` for per-connection state (#275) made this removal possible: `Progress` and `ServerContext` now use `notification_target:` (a `ServerSession`) to route notifications to the originating session only. ### Changes - Removed `Server#notify_progress` entirely. The method is no longer needed as `notification_target` in `call_tool_with_args` now passes `session` directly (nil when no session is available, in which case `Progress#report` and `ServerContext#notify_log_message` are no-ops). - Removed `ServerContext#notify_progress` as well. Progress notifications should only be sent via `server_context.report_progress`, which enforces the correct `progressToken` from the originating request. - Added nil guards to `Progress#report` and `ServerContext#notify_log_message` for when no session is available. - Rewrote progress tests to use `ServerSession#handle` instead of `Server#handle`, reflecting that progress notifications are always session-scoped. - Removed 4 tests that called `server.notify_progress` directly as a public broadcast API. - Removed `notify_progress` from the README.md notification methods list. - Removed the "Server-Side: Direct `notify_progress` Usage" section from the README.md. Progress notifications should be sent via `server_context.report_progress` inside tool handlers, which automatically scopes them to the originating client session. ## Breaking Changes Progress notifications are scoped to a specific request via `progressToken` per the MCP specification, so the broadcast behavior of `Server#notify_progress` was a spec violation. This is treated as a bug fix and is made without a deprecation period. This aligns the Ruby SDK with the Python and TypeScript SDKs, neither of which provides a server-level progress broadcast API. `Server#notify_progress` and `ServerContext#notify_progress` are no longer available. Users should use `server_context.report_progress` inside tool handlers instead, which provides session-scoped delivery with the correct `progressToken`. This feature was only recently introduced in mcp 0.9.0: https://github.com/modelcontextprotocol/ruby-sdk/releases/tag/v0.9.0 So an early release should help limit its impact on adoption.
This commit is contained in:
Koichi ITO
parent
8969edf4b9
commit
b89f6e315d
3 changed files with 39 additions and 109 deletions
|
|
@ -27,17 +27,25 @@ module MCP
|
|||
@server = Server.new(name: "test_server")
|
||||
@transport = MockTransport.new(@server)
|
||||
@server.transport = @transport
|
||||
@session = ServerSession.new(server: @server, transport: @transport)
|
||||
end
|
||||
|
||||
test "#report is a no-op when progress_token is nil" do
|
||||
progress = Progress.new(notification_target: @server, progress_token: nil)
|
||||
progress = Progress.new(notification_target: @session, progress_token: nil)
|
||||
progress.report(50, total: 100, message: "halfway")
|
||||
|
||||
assert_equal 0, @transport.notifications.size
|
||||
end
|
||||
|
||||
test "#report is a no-op when notification_target is nil" do
|
||||
progress = Progress.new(notification_target: nil, progress_token: "token-1")
|
||||
progress.report(50, total: 100, message: "halfway")
|
||||
|
||||
assert_equal 0, @transport.notifications.size
|
||||
end
|
||||
|
||||
test "#report sends notification when progress_token is present" do
|
||||
progress = Progress.new(notification_target: @server, progress_token: "token-1")
|
||||
progress = Progress.new(notification_target: @session, progress_token: "token-1")
|
||||
progress.report(50, total: 100, message: "halfway")
|
||||
|
||||
assert_equal 1, @transport.notifications.size
|
||||
|
|
@ -50,7 +58,7 @@ module MCP
|
|||
end
|
||||
|
||||
test "#report omits total and message when not provided" do
|
||||
progress = Progress.new(notification_target: @server, progress_token: "token-1")
|
||||
progress = Progress.new(notification_target: @session, progress_token: "token-1")
|
||||
progress.report(50)
|
||||
|
||||
assert_equal 1, @transport.notifications.size
|
||||
|
|
|
|||
|
|
@ -61,6 +61,13 @@ module MCP
|
|||
server_context.report_progress(50, total: 100)
|
||||
end
|
||||
|
||||
test "ServerContext#notify_log_message is a no-op when notification_target is nil" do
|
||||
progress = Progress.new(notification_target: nil, progress_token: nil)
|
||||
server_context = ServerContext.new(nil, progress: progress, notification_target: nil)
|
||||
|
||||
assert_nothing_raised { server_context.notify_log_message(data: "test", level: "info") }
|
||||
end
|
||||
|
||||
# Tool without server_context parameter
|
||||
class SimpleToolWithoutContext < Tool
|
||||
tool_name "simple_without_context"
|
||||
|
|
|
|||
|
|
@ -90,59 +90,7 @@ module MCP
|
|||
|
||||
@mock_transport = MockTransport.new(@server)
|
||||
@server.transport = @mock_transport
|
||||
end
|
||||
|
||||
test "#notify_progress sends correct notification through transport with all params" do
|
||||
@server.notify_progress(progress_token: "token-1", progress: 50, total: 100, message: "halfway")
|
||||
|
||||
assert_equal 1, @mock_transport.notifications.size
|
||||
notification = @mock_transport.notifications.first
|
||||
assert_equal Methods::NOTIFICATIONS_PROGRESS, notification[:method]
|
||||
assert_equal "token-1", notification[:params]["progressToken"]
|
||||
assert_equal 50, notification[:params]["progress"]
|
||||
assert_equal 100, notification[:params]["total"]
|
||||
assert_equal "halfway", notification[:params]["message"]
|
||||
end
|
||||
|
||||
test "#notify_progress omits total and message when nil" do
|
||||
@server.notify_progress(progress_token: "token-1", progress: 50)
|
||||
|
||||
assert_equal 1, @mock_transport.notifications.size
|
||||
notification = @mock_transport.notifications.first
|
||||
assert_equal Methods::NOTIFICATIONS_PROGRESS, notification[:method]
|
||||
assert_equal "token-1", notification[:params]["progressToken"]
|
||||
assert_equal 50, notification[:params]["progress"]
|
||||
refute notification[:params].key?("total")
|
||||
refute notification[:params].key?("message")
|
||||
end
|
||||
|
||||
test "#notify_progress does nothing without transport" do
|
||||
server_without_transport = Server.new(name: "test_server")
|
||||
|
||||
assert_nothing_raised do
|
||||
server_without_transport.notify_progress(progress_token: "token-1", progress: 50)
|
||||
end
|
||||
end
|
||||
|
||||
test "#notify_progress handles transport errors gracefully" do
|
||||
error_transport = Class.new(MockTransport) do
|
||||
def send_notification(method, params = nil)
|
||||
raise StandardError, "Transport error"
|
||||
end
|
||||
end.new(@server)
|
||||
|
||||
@server.transport = error_transport
|
||||
|
||||
@server.configuration.exception_reporter.expects(:call).once.with do |exception, context|
|
||||
assert_kind_of StandardError, exception
|
||||
assert_equal "Transport error", exception.message
|
||||
assert_equal({ notification: "progress" }, context)
|
||||
true
|
||||
end
|
||||
|
||||
assert_nothing_raised do
|
||||
@server.notify_progress(progress_token: "token-1", progress: 50)
|
||||
end
|
||||
@session = ServerSession.new(server: @server, transport: @mock_transport)
|
||||
end
|
||||
|
||||
test "tool with progress parameter receives Progress instance and sends notifications via _meta.progressToken" do
|
||||
|
|
@ -157,7 +105,7 @@ module MCP
|
|||
},
|
||||
}
|
||||
|
||||
response = @server.handle(request)
|
||||
response = @session.handle(request)
|
||||
|
||||
assert response[:result]
|
||||
assert_equal "ToolWithProgress: Hello", response[:result][:content][0][:text]
|
||||
|
|
@ -190,7 +138,7 @@ module MCP
|
|||
},
|
||||
}
|
||||
|
||||
response = @server.handle(request)
|
||||
response = @session.handle(request)
|
||||
|
||||
assert response[:result]
|
||||
assert_equal "SimpleToolWithoutProgress: Hello", response[:result][:content][0][:text]
|
||||
|
|
@ -215,6 +163,7 @@ module MCP
|
|||
server_context: { user: "test" },
|
||||
)
|
||||
server.transport = @mock_transport
|
||||
session = ServerSession.new(server: server, transport: @mock_transport)
|
||||
|
||||
request = {
|
||||
jsonrpc: "2.0",
|
||||
|
|
@ -226,7 +175,7 @@ module MCP
|
|||
},
|
||||
}
|
||||
|
||||
server.handle(request)
|
||||
session.handle(request)
|
||||
|
||||
assert_instance_of ServerContext, received_context
|
||||
assert_nothing_raised { received_context.report_progress(50) }
|
||||
|
|
@ -245,7 +194,7 @@ module MCP
|
|||
},
|
||||
}
|
||||
|
||||
response = @server.handle(request)
|
||||
response = @session.handle(request)
|
||||
|
||||
assert response[:result]
|
||||
assert_equal "ToolWithContextAndProgress: Hello context=test_user", response[:result][:content][0][:text]
|
||||
|
|
@ -269,7 +218,7 @@ module MCP
|
|||
},
|
||||
}
|
||||
|
||||
response = @server.handle(request)
|
||||
response = @session.handle(request)
|
||||
|
||||
assert response[:result]
|
||||
assert_equal "ToolWithKwargs: progress=present", response[:result][:content][0][:text]
|
||||
|
|
@ -290,6 +239,8 @@ module MCP
|
|||
Tool::Response.new([{ type: "text", text: "block_tool done" }])
|
||||
end
|
||||
|
||||
session = ServerSession.new(server: server, transport: @mock_transport)
|
||||
|
||||
request = {
|
||||
jsonrpc: "2.0",
|
||||
id: 1,
|
||||
|
|
@ -301,7 +252,7 @@ module MCP
|
|||
},
|
||||
}
|
||||
|
||||
response = server.handle(request)
|
||||
response = session.handle(request)
|
||||
|
||||
assert response[:result]
|
||||
assert_equal "block_tool done", response[:result][:content][0][:text]
|
||||
|
|
@ -325,6 +276,8 @@ module MCP
|
|||
Tool::Response.new([{ type: "text", text: "done" }])
|
||||
end
|
||||
|
||||
session = ServerSession.new(server: server, transport: @mock_transport)
|
||||
|
||||
request = {
|
||||
jsonrpc: "2.0",
|
||||
id: 1,
|
||||
|
|
@ -336,7 +289,7 @@ module MCP
|
|||
},
|
||||
}
|
||||
|
||||
server.handle(request)
|
||||
session.handle(request)
|
||||
|
||||
assert_equal 5, @mock_transport.notifications.size
|
||||
@mock_transport.notifications.each_with_index do |n, i|
|
||||
|
|
@ -358,7 +311,7 @@ module MCP
|
|||
}
|
||||
|
||||
# Should not raise and should return nil (notification, no id).
|
||||
result = @server.handle(request)
|
||||
result = @session.handle(request)
|
||||
assert_nil result
|
||||
end
|
||||
end
|
||||
|
|
|
|||
Loading…
Reference in a new issue