feat: warn on implicit stdio initialization
This commit is contained in:
parent
dd7dacae47
commit
b6462feef0
2 changed files with 116 additions and 19 deletions
|
|
@ -134,7 +134,10 @@ module MCP
|
||||||
|
|
||||||
def send_request(request:)
|
def send_request(request:)
|
||||||
start unless @started
|
start unless @started
|
||||||
connect unless @initialized
|
unless @initialized
|
||||||
|
warn("Calling `MCP::Client::Stdio#send_request` without calling `MCP::Client#connect` is deprecated. Use `MCP::Client#connect` before sending requests instead.", uplevel: 1)
|
||||||
|
connect
|
||||||
|
end
|
||||||
|
|
||||||
write_message(request)
|
write_message(request)
|
||||||
read_response(request)
|
read_response(request)
|
||||||
|
|
|
||||||
|
|
@ -9,6 +9,9 @@ require "mcp/client/tool"
|
||||||
module MCP
|
module MCP
|
||||||
class Client
|
class Client
|
||||||
class StdioTest < Minitest::Test
|
class StdioTest < Minitest::Test
|
||||||
|
IMPLICIT_CONNECT_DEPRECATION_WARNING =
|
||||||
|
/Calling `MCP::Client::Stdio#send_request` without calling `MCP::Client#connect` is deprecated\. Use `MCP::Client#connect` before sending requests instead\./.freeze
|
||||||
|
|
||||||
def test_send_request_starts_process_and_returns_response
|
def test_send_request_starts_process_and_returns_response
|
||||||
stdin_read, stdin_write = IO.pipe
|
stdin_read, stdin_write = IO.pipe
|
||||||
stdout_read, stdout_write = IO.pipe
|
stdout_read, stdout_write = IO.pipe
|
||||||
|
|
@ -56,7 +59,10 @@ module MCP
|
||||||
stdout_write.flush
|
stdout_write.flush
|
||||||
end
|
end
|
||||||
|
|
||||||
|
response = nil
|
||||||
|
assert_implicit_connect_deprecation_warning do
|
||||||
response = transport.send_request(request: request)
|
response = transport.send_request(request: request)
|
||||||
|
end
|
||||||
|
|
||||||
assert_equal("test-id", response["id"])
|
assert_equal("test-id", response["id"])
|
||||||
assert_equal(1, response.dig("result", "tools").size)
|
assert_equal(1, response.dig("result", "tools").size)
|
||||||
|
|
@ -123,7 +129,9 @@ module MCP
|
||||||
stdout_write.flush
|
stdout_write.flush
|
||||||
end
|
end
|
||||||
|
|
||||||
|
assert_implicit_connect_deprecation_warning do
|
||||||
transport.send_request(request: request)
|
transport.send_request(request: request)
|
||||||
|
end
|
||||||
|
|
||||||
assert_equal(["initialize", "notifications/initialized", "tools/list"], received_methods)
|
assert_equal(["initialize", "notifications/initialized", "tools/list"], received_methods)
|
||||||
ensure
|
ensure
|
||||||
|
|
@ -184,10 +192,13 @@ module MCP
|
||||||
stdout_write.flush
|
stdout_write.flush
|
||||||
end
|
end
|
||||||
|
|
||||||
|
response = nil
|
||||||
|
assert_implicit_connect_deprecation_warning do
|
||||||
response = transport.send_request(request: request)
|
response = transport.send_request(request: request)
|
||||||
|
end
|
||||||
|
|
||||||
assert_equal("test-id", response["id"])
|
assert_equal("test-id", response["id"])
|
||||||
assert_equal([], response.dig("result", "tools"))
|
assert_empty(response.dig("result", "tools"))
|
||||||
ensure
|
ensure
|
||||||
server_thread.join
|
server_thread.join
|
||||||
stdin_read.close
|
stdin_read.close
|
||||||
|
|
@ -217,9 +228,12 @@ module MCP
|
||||||
method: "tools/list",
|
method: "tools/list",
|
||||||
}
|
}
|
||||||
|
|
||||||
|
error = nil
|
||||||
|
assert_implicit_connect_deprecation_warning do
|
||||||
error = assert_raises(RequestHandlerError) do
|
error = assert_raises(RequestHandlerError) do
|
||||||
transport.send_request(request: request)
|
transport.send_request(request: request)
|
||||||
end
|
end
|
||||||
|
end
|
||||||
|
|
||||||
assert_equal("Server process has exited", error.message)
|
assert_equal("Server process has exited", error.message)
|
||||||
assert_equal(:internal_error, error.error_type)
|
assert_equal(:internal_error, error.error_type)
|
||||||
|
|
@ -268,9 +282,12 @@ module MCP
|
||||||
stdout_write.close
|
stdout_write.close
|
||||||
end
|
end
|
||||||
|
|
||||||
|
error = nil
|
||||||
|
assert_implicit_connect_deprecation_warning do
|
||||||
error = assert_raises(RequestHandlerError) do
|
error = assert_raises(RequestHandlerError) do
|
||||||
transport.send_request(request: request)
|
transport.send_request(request: request)
|
||||||
end
|
end
|
||||||
|
end
|
||||||
|
|
||||||
assert_equal("Server process closed stdout unexpectedly", error.message)
|
assert_equal("Server process closed stdout unexpectedly", error.message)
|
||||||
assert_equal(:internal_error, error.error_type)
|
assert_equal(:internal_error, error.error_type)
|
||||||
|
|
@ -380,7 +397,9 @@ module MCP
|
||||||
stdout_write.flush
|
stdout_write.flush
|
||||||
end
|
end
|
||||||
|
|
||||||
|
assert_implicit_connect_deprecation_warning do
|
||||||
transport.send_request(request: { jsonrpc: "2.0", id: "first", method: "tools/list" })
|
transport.send_request(request: { jsonrpc: "2.0", id: "first", method: "tools/list" })
|
||||||
|
end
|
||||||
transport.send_request(request: { jsonrpc: "2.0", id: "second", method: "tools/list" })
|
transport.send_request(request: { jsonrpc: "2.0", id: "second", method: "tools/list" })
|
||||||
|
|
||||||
assert_equal(
|
assert_equal(
|
||||||
|
|
@ -444,9 +463,12 @@ module MCP
|
||||||
stdout_write.flush
|
stdout_write.flush
|
||||||
end
|
end
|
||||||
|
|
||||||
|
error = nil
|
||||||
|
assert_implicit_connect_deprecation_warning do
|
||||||
error = assert_raises(RequestHandlerError) do
|
error = assert_raises(RequestHandlerError) do
|
||||||
transport.send_request(request: request)
|
transport.send_request(request: request)
|
||||||
end
|
end
|
||||||
|
end
|
||||||
|
|
||||||
assert_equal("Failed to parse server response", error.message)
|
assert_equal("Failed to parse server response", error.message)
|
||||||
assert_equal(:internal_error, error.error_type)
|
assert_equal(:internal_error, error.error_type)
|
||||||
|
|
@ -486,9 +508,12 @@ module MCP
|
||||||
stdout_write.flush
|
stdout_write.flush
|
||||||
end
|
end
|
||||||
|
|
||||||
|
error = nil
|
||||||
|
assert_implicit_connect_deprecation_warning do
|
||||||
error = assert_raises(RequestHandlerError) do
|
error = assert_raises(RequestHandlerError) do
|
||||||
transport.send_request(request: request)
|
transport.send_request(request: request)
|
||||||
end
|
end
|
||||||
|
end
|
||||||
|
|
||||||
assert_equal("Server initialization failed: Invalid Request", error.message)
|
assert_equal("Server initialization failed: Invalid Request", error.message)
|
||||||
assert_equal(:internal_error, error.error_type)
|
assert_equal(:internal_error, error.error_type)
|
||||||
|
|
@ -563,9 +588,12 @@ module MCP
|
||||||
stdin_read.gets
|
stdin_read.gets
|
||||||
end
|
end
|
||||||
|
|
||||||
|
error = nil
|
||||||
|
assert_implicit_connect_deprecation_warning do
|
||||||
error = assert_raises(RequestHandlerError) do
|
error = assert_raises(RequestHandlerError) do
|
||||||
transport.send_request(request: request)
|
transport.send_request(request: request)
|
||||||
end
|
end
|
||||||
|
end
|
||||||
|
|
||||||
assert_equal("Timed out waiting for server response", error.message)
|
assert_equal("Timed out waiting for server response", error.message)
|
||||||
assert_equal(:internal_error, error.error_type)
|
assert_equal(:internal_error, error.error_type)
|
||||||
|
|
@ -616,7 +644,9 @@ module MCP
|
||||||
end
|
end
|
||||||
|
|
||||||
# Complete handshake with a successful request
|
# Complete handshake with a successful request
|
||||||
|
assert_implicit_connect_deprecation_warning do
|
||||||
transport.send_request(request: { jsonrpc: "2.0", id: "setup", method: "ping" })
|
transport.send_request(request: { jsonrpc: "2.0", id: "setup", method: "ping" })
|
||||||
|
end
|
||||||
server_thread.join
|
server_thread.join
|
||||||
|
|
||||||
# Now close stdin to simulate broken pipe
|
# Now close stdin to simulate broken pipe
|
||||||
|
|
@ -708,9 +738,12 @@ module MCP
|
||||||
stdout_write.flush
|
stdout_write.flush
|
||||||
end
|
end
|
||||||
|
|
||||||
|
error = nil
|
||||||
|
assert_implicit_connect_deprecation_warning do
|
||||||
error = assert_raises(RequestHandlerError) do
|
error = assert_raises(RequestHandlerError) do
|
||||||
transport.send_request(request: request)
|
transport.send_request(request: request)
|
||||||
end
|
end
|
||||||
|
end
|
||||||
|
|
||||||
assert_equal("Server initialization failed: missing result in response", error.message)
|
assert_equal("Server initialization failed: missing result in response", error.message)
|
||||||
assert_equal(:internal_error, error.error_type)
|
assert_equal(:internal_error, error.error_type)
|
||||||
|
|
@ -768,6 +801,59 @@ module MCP
|
||||||
stdout_write.close
|
stdout_write.close
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def test_send_request_does_not_warn_after_explicit_connect
|
||||||
|
stdin_read, stdin_write = IO.pipe
|
||||||
|
stdout_read, stdout_write = IO.pipe
|
||||||
|
stderr_read, _ = IO.pipe
|
||||||
|
|
||||||
|
Open3.stubs(:popen3).returns([stdin_write, stdout_read, stderr_read, mock_wait_thread])
|
||||||
|
|
||||||
|
transport = Stdio.new(command: "ruby", args: ["server.rb"])
|
||||||
|
|
||||||
|
server_thread = Thread.new do
|
||||||
|
init_line = stdin_read.gets
|
||||||
|
init_request = JSON.parse(init_line)
|
||||||
|
stdout_write.puts(JSON.generate(
|
||||||
|
jsonrpc: "2.0",
|
||||||
|
id: init_request["id"],
|
||||||
|
result: {
|
||||||
|
protocolVersion: "2025-11-25",
|
||||||
|
capabilities: {},
|
||||||
|
serverInfo: { name: "test-server", version: "1.0.0" },
|
||||||
|
},
|
||||||
|
))
|
||||||
|
stdout_write.flush
|
||||||
|
|
||||||
|
stdin_read.gets
|
||||||
|
|
||||||
|
ping_line = stdin_read.gets
|
||||||
|
ping_request = JSON.parse(ping_line)
|
||||||
|
stdout_write.puts(JSON.generate(
|
||||||
|
jsonrpc: "2.0",
|
||||||
|
id: ping_request["id"],
|
||||||
|
result: {},
|
||||||
|
))
|
||||||
|
stdout_write.flush
|
||||||
|
end
|
||||||
|
|
||||||
|
assert_silent do
|
||||||
|
transport.connect
|
||||||
|
end
|
||||||
|
|
||||||
|
response = nil
|
||||||
|
assert_silent do
|
||||||
|
response = transport.send_request(request: { jsonrpc: "2.0", id: "ping-id", method: "ping" })
|
||||||
|
end
|
||||||
|
|
||||||
|
assert_equal("ping-id", response["id"])
|
||||||
|
ensure
|
||||||
|
server_thread.join
|
||||||
|
stdin_read.close
|
||||||
|
stdin_write.close
|
||||||
|
stdout_read.close
|
||||||
|
stdout_write.close
|
||||||
|
end
|
||||||
|
|
||||||
def test_connect_caches_server_info
|
def test_connect_caches_server_info
|
||||||
transport, server_thread, pipes = stub_successful_connect
|
transport, server_thread, pipes = stub_successful_connect
|
||||||
|
|
||||||
|
|
@ -1141,6 +1227,14 @@ module MCP
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
|
def assert_implicit_connect_deprecation_warning(&block)
|
||||||
|
original_verbose = $VERBOSE
|
||||||
|
$VERBOSE = false
|
||||||
|
assert_output(nil, IMPLICIT_CONNECT_DEPRECATION_WARNING, &block)
|
||||||
|
ensure
|
||||||
|
$VERBOSE = original_verbose
|
||||||
|
end
|
||||||
|
|
||||||
def stub_successful_connect
|
def stub_successful_connect
|
||||||
stdin_read, stdin_write = IO.pipe
|
stdin_read, stdin_write = IO.pipe
|
||||||
stdout_read, stdout_write = IO.pipe
|
stdout_read, stdout_write = IO.pipe
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue