Merge pull request #469 from jstar0/fix/preserve-explicit-tool-content
Preserve explicit tool response content
This commit is contained in:
commit
565f4de781
4 changed files with 109 additions and 20 deletions
|
|
@ -745,11 +745,15 @@ module MCP
|
||||||
|
|
||||||
progress_token = request.dig(:_meta, :progressToken)
|
progress_token = request.dig(:_meta, :progressToken)
|
||||||
|
|
||||||
result = call_tool_with_args(
|
response = call_tool_with_args(
|
||||||
tool, arguments, server_context_with_meta(request), progress_token: progress_token, session: session, related_request_id: related_request_id, cancellation: cancellation
|
tool, arguments, server_context_with_meta(request), progress_token: progress_token, session: session, related_request_id: related_request_id, cancellation: cancellation
|
||||||
)
|
)
|
||||||
|
result = response.to_h
|
||||||
validate_tool_call_result!(tool, result)
|
validate_tool_call_result!(tool, result)
|
||||||
serialize_structured_content_fallback(result)
|
serialize_structured_content_fallback(
|
||||||
|
result,
|
||||||
|
content_provided: response.respond_to?(:content_provided?) && response.content_provided?,
|
||||||
|
)
|
||||||
rescue RequestHandlerError, CancelledError
|
rescue RequestHandlerError, CancelledError
|
||||||
# CancelledError is intentionally not wrapped so `handle_request` can turn it into
|
# CancelledError is intentionally not wrapped so `handle_request` can turn it into
|
||||||
# `JsonRpcHandler::NO_RESPONSE` per the MCP cancellation spec.
|
# `JsonRpcHandler::NO_RESPONSE` per the MCP cancellation spec.
|
||||||
|
|
@ -986,14 +990,18 @@ module MCP
|
||||||
|
|
||||||
# Per SEP-2106, `structuredContent` may be any JSON value, not only an object.
|
# Per SEP-2106, `structuredContent` may be any JSON value, not only an object.
|
||||||
# Clients on older protocol versions may only read `content`,
|
# Clients on older protocol versions may only read `content`,
|
||||||
# so when a tool returns non-object structured content without providing
|
# so when a tool returns non-object structured content without explicitly
|
||||||
# any content blocks, mirror the value into `content` as serialized JSON text.
|
# providing `content`, mirror the value into serialized JSON text.
|
||||||
def serialize_structured_content_fallback(result)
|
def serialize_structured_content_fallback(result, content_provided: false)
|
||||||
structured = result[:structuredContent]
|
structured = result[:structuredContent]
|
||||||
return result if structured.nil? || structured.is_a?(Hash)
|
return result if structured.nil? || structured.is_a?(Hash)
|
||||||
|
return result if content_provided
|
||||||
return result unless result[:content].nil? || result[:content].empty?
|
return result unless result[:content].nil? || result[:content].empty?
|
||||||
|
|
||||||
result.merge(content: [{ type: "text", text: JSON.generate(structured) }])
|
serialized = JSON.generate(structured)
|
||||||
|
return result if JSON.parse(serialized).is_a?(Hash)
|
||||||
|
|
||||||
|
result.merge(content: [{ type: "text", text: serialized }])
|
||||||
end
|
end
|
||||||
|
|
||||||
# Whether a tool/prompt handler opts in to receiving an `MCP::ServerContext`.
|
# Whether a tool/prompt handler opts in to receiving an `MCP::ServerContext`.
|
||||||
|
|
@ -1026,9 +1034,9 @@ module MCP
|
||||||
related_request_id: related_request_id,
|
related_request_id: related_request_id,
|
||||||
cancellation: cancellation,
|
cancellation: cancellation,
|
||||||
)
|
)
|
||||||
tool.call(**args, server_context: server_context).to_h
|
tool.call(**args, server_context: server_context)
|
||||||
else
|
else
|
||||||
tool.call(**args).to_h
|
tool.call(**args)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -7,18 +7,24 @@ module MCP
|
||||||
|
|
||||||
attr_reader :content, :structured_content, :meta
|
attr_reader :content, :structured_content, :meta
|
||||||
|
|
||||||
def initialize(content = nil, deprecated_error = NOT_GIVEN, error: false, structured_content: nil, meta: nil)
|
def initialize(content = NOT_GIVEN, deprecated_error = NOT_GIVEN, error: false, structured_content: nil, meta: nil)
|
||||||
if deprecated_error != NOT_GIVEN
|
if deprecated_error != NOT_GIVEN
|
||||||
warn("Passing `error` with the 2nd argument of `Response.new` is deprecated. Use keyword argument like `Response.new(content, error: error)` instead.", uplevel: 1)
|
warn("Passing `error` with the 2nd argument of `Response.new` is deprecated. Use keyword argument like `Response.new(content, error: error)` instead.", uplevel: 1)
|
||||||
error = deprecated_error
|
error = deprecated_error
|
||||||
end
|
end
|
||||||
|
|
||||||
@content = content || []
|
content_given = !content.equal?(NOT_GIVEN)
|
||||||
|
@content_provided = content_given && !content.nil?
|
||||||
|
@content = content_given ? (content || []) : []
|
||||||
@error = error
|
@error = error
|
||||||
@structured_content = structured_content
|
@structured_content = structured_content
|
||||||
@meta = meta
|
@meta = meta
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def content_provided?
|
||||||
|
@content_provided
|
||||||
|
end
|
||||||
|
|
||||||
def error?
|
def error?
|
||||||
!!@error
|
!!@error
|
||||||
end
|
end
|
||||||
|
|
|
||||||
|
|
@ -3156,6 +3156,16 @@ module MCP
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
class JsonObjectValue
|
||||||
|
def initialize(value)
|
||||||
|
@value = value
|
||||||
|
end
|
||||||
|
|
||||||
|
def to_json(*args)
|
||||||
|
@value.to_json(*args)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
test "server_context_with_meta uses accessor method, not ivar directly" do
|
test "server_context_with_meta uses accessor method, not ivar directly" do
|
||||||
subclass = Class.new(Server) do
|
subclass = Class.new(Server) do
|
||||||
def server_context
|
def server_context
|
||||||
|
|
@ -3247,23 +3257,28 @@ module MCP
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
test "#handle tools/call mirrors non-object structuredContent into serialized text content" do
|
test "#handle tools/call mirrors non-object structuredContent when content is omitted or nil" do
|
||||||
# Per SEP-2106, `structuredContent` may be any JSON value. Older clients may only read `content`,
|
# Per SEP-2106, `structuredContent` may be any JSON value. Older clients may only read `content`,
|
||||||
# so the server adds a serialized fallback when the tool provided no content blocks.
|
# so the server adds a serialized fallback when the tool provided no content blocks.
|
||||||
server = Server.new(name: "structured_test", tools: [])
|
server = Server.new(name: "structured_test", tools: [])
|
||||||
server.define_tool(name: "array_tool") do
|
server.define_tool(name: "nil_content_tool") do
|
||||||
Tool::Response.new(nil, structured_content: [1, 2])
|
Tool::Response.new(nil, structured_content: [1, 2])
|
||||||
end
|
end
|
||||||
|
server.define_tool(name: "omitted_content_tool") do
|
||||||
|
Tool::Response.new(structured_content: [1, 2])
|
||||||
|
end
|
||||||
|
|
||||||
response = server.handle({
|
["nil_content_tool", "omitted_content_tool"].each_with_index do |tool_name, index|
|
||||||
jsonrpc: "2.0",
|
response = server.handle({
|
||||||
method: "tools/call",
|
jsonrpc: "2.0",
|
||||||
id: 1,
|
method: "tools/call",
|
||||||
params: { name: "array_tool", arguments: {} },
|
id: index + 1,
|
||||||
})
|
params: { name: tool_name, arguments: {} },
|
||||||
|
})
|
||||||
|
|
||||||
assert_equal [1, 2], response.dig(:result, :structuredContent)
|
assert_equal [1, 2], response.dig(:result, :structuredContent)
|
||||||
assert_equal [{ type: "text", text: "[1,2]" }], response.dig(:result, :content)
|
assert_equal [{ type: "text", text: "[1,2]" }], response.dig(:result, :content)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
test "#handle tools/call does not overwrite explicit content when structuredContent is non-object" do
|
test "#handle tools/call does not overwrite explicit content when structuredContent is non-object" do
|
||||||
|
|
@ -3299,6 +3314,60 @@ module MCP
|
||||||
assert_empty response.dig(:result, :content)
|
assert_empty response.dig(:result, :content)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "#handle tools/call recognizes object-like structuredContent by its JSON shape" do
|
||||||
|
structured_content = JsonObjectValue.new(answer: 42)
|
||||||
|
server = Server.new(name: "structured_test", tools: [])
|
||||||
|
server.define_tool(name: "object_tool") do
|
||||||
|
Tool::Response.new(nil, structured_content: structured_content)
|
||||||
|
end
|
||||||
|
|
||||||
|
response = server.handle({
|
||||||
|
jsonrpc: "2.0",
|
||||||
|
method: "tools/call",
|
||||||
|
id: 1,
|
||||||
|
params: { name: "object_tool", arguments: {} },
|
||||||
|
})
|
||||||
|
|
||||||
|
assert_equal structured_content, response.dig(:result, :structuredContent)
|
||||||
|
assert_empty response.dig(:result, :content)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "#handle tools/call preserves explicit empty content for non-object structuredContent" do
|
||||||
|
server = Server.new(name: "structured_test", tools: [])
|
||||||
|
server.define_tool(name: "array_tool") do
|
||||||
|
Tool::Response.new([], structured_content: [1, 2])
|
||||||
|
end
|
||||||
|
|
||||||
|
response = server.handle({
|
||||||
|
jsonrpc: "2.0",
|
||||||
|
method: "tools/call",
|
||||||
|
id: 1,
|
||||||
|
params: { name: "array_tool", arguments: {} },
|
||||||
|
})
|
||||||
|
|
||||||
|
assert_equal [1, 2], response.dig(:result, :structuredContent)
|
||||||
|
assert_empty response.dig(:result, :content)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "#handle tools/call preserves explicit empty content for object-like structuredContent" do
|
||||||
|
structured_content = JsonObjectValue.new(answer: 42)
|
||||||
|
|
||||||
|
server = Server.new(name: "structured_test", tools: [])
|
||||||
|
server.define_tool(name: "object_tool") do
|
||||||
|
Tool::Response.new([], structured_content: structured_content)
|
||||||
|
end
|
||||||
|
|
||||||
|
response = server.handle({
|
||||||
|
jsonrpc: "2.0",
|
||||||
|
method: "tools/call",
|
||||||
|
id: 1,
|
||||||
|
params: { name: "object_tool", arguments: {} },
|
||||||
|
})
|
||||||
|
|
||||||
|
assert_equal structured_content, response.dig(:result, :structuredContent)
|
||||||
|
assert_empty response.dig(:result, :content)
|
||||||
|
end
|
||||||
|
|
||||||
test "list results carry ttlMs and cacheScope when ttl_ms is configured" do
|
test "list results carry ttlMs and cacheScope when ttl_ms is configured" do
|
||||||
# SEP-2549 cache hints. The cacheScope default is "public", matching
|
# SEP-2549 cache hints. The cacheScope default is "public", matching
|
||||||
# the spec default and the Python SDK.
|
# the spec default and the Python SDK.
|
||||||
|
|
|
||||||
|
|
@ -16,6 +16,12 @@ module MCP
|
||||||
refute response.error?
|
refute response.error?
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "#content_provided? distinguishes empty content from omitted or nil content" do
|
||||||
|
assert Response.new([]).content_provided?
|
||||||
|
refute Response.new.content_provided?
|
||||||
|
refute Response.new(nil).content_provided?
|
||||||
|
end
|
||||||
|
|
||||||
test "#initialize with content and error set to true" do
|
test "#initialize with content and error set to true" do
|
||||||
content = [{
|
content = [{
|
||||||
type: "text",
|
type: "text",
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue