diff --git a/README.md b/README.md index 9e8a096..d63b22f 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,6 @@ The server provides the following notification methods: - `notify_tools_list_changed` - Send a notification when the tools list changes - `notify_prompts_list_changed` - Send a notification when the prompts list changes - `notify_resources_list_changed` - Send a notification when the resources list changes -- `notify_progress` - Send a progress notification for long-running operations - `notify_log_message` - Send a structured logging notification message #### Session Scoping @@ -178,24 +177,10 @@ The `server_context.report_progress` method accepts: - `total:` (optional) — total expected value, so clients can display a percentage - `message:` (optional) — human-readable status message -#### Server-Side: Direct `notify_progress` Usage - -You can also call `notify_progress` directly on the server instance: - -```ruby -server.notify_progress( - progress_token: "token-123", - progress: 50, - total: 100, # optional - message: "halfway" # optional -) -``` - **Key Features:** - Tools report progress via `server_context.report_progress` - `report_progress` is a no-op when no `progressToken` was provided by the client -- `notify_progress` is a no-op when no transport is configured - Supports both numeric and string progress tokens ### Logging diff --git a/lib/mcp/progress.rb b/lib/mcp/progress.rb index 7c36a94..8843a0d 100644 --- a/lib/mcp/progress.rb +++ b/lib/mcp/progress.rb @@ -9,6 +9,7 @@ def initialize(notification_target:, progress_token:) def report(progress, total: nil, message: nil) return unless @progress_token + return unless @notification_target @notification_target.notify_progress( progress_token: @progress_token, diff --git a/lib/mcp/server.rb b/lib/mcp/server.rb index ffcce58..0f7fc6f 100644 --- a/lib/mcp/server.rb +++ b/lib/mcp/server.rb @@ -116,7 +116,7 @@ def initialize( # @param request [Hash] A parsed JSON-RPC request. # @param session [ServerSession, nil] Per-connection session. Passed by # `ServerSession#handle` for session-scoped notification delivery. - # When `nil`, notifications broadcast to all sessions. + # When `nil`, progress and logging notifications from tool handlers are silently skipped. # @return [Hash, nil] The JSON-RPC response, or `nil` for notifications. def handle(request, session: nil) JsonRpcHandler.handle(request) do |method| @@ -129,7 +129,7 @@ def handle(request, session: nil) # @param request [String] A JSON-RPC request as a JSON string. # @param session [ServerSession, nil] Per-connection session. Passed by # `ServerSession#handle_json` for session-scoped notification delivery. - # When `nil`, notifications broadcast to all sessions. + # When `nil`, progress and logging notifications from tool handlers are silently skipped. # @return [String, nil] The JSON-RPC response as JSON, or `nil` for notifications. def handle_json(request, session: nil) JsonRpcHandler.handle_json(request) do |method| @@ -186,21 +186,6 @@ def notify_resources_list_changed report_exception(e, { notification: "resources_list_changed" }) end - def notify_progress(progress_token:, progress:, total: nil, message: nil) - return unless @transport - - params = { - "progressToken" => progress_token, - "progress" => progress, - "total" => total, - "message" => message, - }.compact - - @transport.send_notification(Methods::NOTIFICATIONS_PROGRESS, params) - rescue => e - report_exception(e, notification: "progress") - end - def notify_log_message(data:, level:, logger: nil) return unless @transport return unless logging_message_notification&.should_notify?(level) @@ -524,9 +509,8 @@ def call_tool_with_args(tool, arguments, context, progress_token: nil, session: args = arguments&.transform_keys(&:to_sym) || {} if accepts_server_context?(tool.method(:call)) - notification_target = session || self - progress = Progress.new(notification_target: notification_target, progress_token: progress_token) - server_context = ServerContext.new(context, progress: progress, notification_target: notification_target) + progress = Progress.new(notification_target: session, progress_token: progress_token) + server_context = ServerContext.new(context, progress: progress, notification_target: session) tool.call(**args, server_context: server_context).to_h else tool.call(**args).to_h diff --git a/lib/mcp/server_context.rb b/lib/mcp/server_context.rb index 458d2cd..7d1f2d9 100644 --- a/lib/mcp/server_context.rb +++ b/lib/mcp/server_context.rb @@ -18,22 +18,14 @@ def report_progress(progress, total: nil, message: nil) @progress.report(progress, total: total, message: message) end - # Sends a progress notification scoped to the originating session. - # - # @param progress_token [String, Integer] The token identifying the operation. - # @param progress [Numeric] Current progress value. - # @param total [Numeric, nil] Total expected value. - # @param message [String, nil] Human-readable status message. - def notify_progress(progress_token:, progress:, total: nil, message: nil) - @notification_target.notify_progress(progress_token: progress_token, progress: progress, total: total, message: message) - end - # Sends a log message notification scoped to the originating session. # # @param data [Object] The log data to send. # @param level [String] Log level (e.g., `"debug"`, `"info"`, `"error"`). # @param logger [String, nil] Logger name. def notify_log_message(data:, level:, logger: nil) + return unless @notification_target + @notification_target.notify_log_message(data: data, level: level, logger: logger) end diff --git a/test/mcp/progress_test.rb b/test/mcp/progress_test.rb index a0349b5..491245b 100644 --- a/test/mcp/progress_test.rb +++ b/test/mcp/progress_test.rb @@ -27,17 +27,25 @@ def handle_request(request); end @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 @@ def handle_request(request); end 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 diff --git a/test/mcp/server_context_test.rb b/test/mcp/server_context_test.rb index 6fe7ea6..159dae8 100644 --- a/test/mcp/server_context_test.rb +++ b/test/mcp/server_context_test.rb @@ -61,6 +61,13 @@ def context.custom_method 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" diff --git a/test/mcp/server_progress_test.rb b/test/mcp/server_progress_test.rb index cf066fa..f31dfaa 100644 --- a/test/mcp/server_progress_test.rb +++ b/test/mcp/server_progress_test.rb @@ -90,59 +90,7 @@ def call(**kwargs) @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 @@ def send_notification(method, params = nil) }, } - response = @server.handle(request) + response = @session.handle(request) assert response[:result] assert_equal "ToolWithProgress: Hello", response[:result][:content][0][:text] @@ -190,7 +138,7 @@ def send_notification(method, params = nil) }, } - response = @server.handle(request) + response = @session.handle(request) assert response[:result] assert_equal "SimpleToolWithoutProgress: Hello", response[:result][:content][0][:text] @@ -215,6 +163,7 @@ def send_notification(method, params = nil) server_context: { user: "test" }, ) server.transport = @mock_transport + session = ServerSession.new(server: server, transport: @mock_transport) request = { jsonrpc: "2.0", @@ -226,7 +175,7 @@ def send_notification(method, params = nil) }, } - server.handle(request) + session.handle(request) assert_instance_of ServerContext, received_context assert_nothing_raised { received_context.report_progress(50) } @@ -245,7 +194,7 @@ def send_notification(method, params = nil) }, } - 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 @@ def send_notification(method, params = nil) }, } - 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 @@ def send_notification(method, params = nil) 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 @@ def send_notification(method, params = nil) }, } - 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 @@ def send_notification(method, params = nil) 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 @@ def send_notification(method, params = nil) }, } - 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 @@ def send_notification(method, params = nil) } # Should not raise and should return nil (notification, no id). - result = @server.handle(request) + result = @session.handle(request) assert_nil result end end