From 6689db2ca09b8d20cd1eaaf1e8f640ccfb6cbc7d Mon Sep 17 00:00:00 2001
From: Piotr Usewicz <piotr@layer22.com>
Date: Mon, 11 Aug 2025 16:15:18 +0200
Subject: [PATCH] Nicer error API

Both the combination of positional argument and naming does not feel
like Ruby.

This change makes `is_error` argument a keyword argument called `error`.

Thanks to this, the usage is more self-explanatory:

```ruby
response = Response.new([], error: true)
```
---
 lib/mcp/tool/response.rb       | 19 +++++++--
 test/mcp/tool/response_test.rb | 77 ++++++++++++++++++++++++++++++++++
 test/mcp/tool_test.rb          |  4 +-
 3 files changed, 94 insertions(+), 6 deletions(-)
 create mode 100644 test/mcp/tool/response_test.rb

diff --git a/lib/mcp/tool/response.rb b/lib/mcp/tool/response.rb
index abf6ff48..1fd3bfb4 100644
--- a/lib/mcp/tool/response.rb
+++ b/lib/mcp/tool/response.rb
@@ -3,15 +3,26 @@
 module MCP
   class Tool
     class Response
-      attr_reader :content, :is_error
+      NOT_GIVEN = Object.new.freeze
+
+      attr_reader :content
+
+      def initialize(content, deprecated_error = NOT_GIVEN, error: false)
+        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)
+          error = deprecated_error
+        end
 
-      def initialize(content, is_error = false)
         @content = content
-        @is_error = is_error
+        @error = error
+      end
+
+      def error?
+        !!@error
       end
 
       def to_h
-        { content:, isError: is_error }.compact
+        { content:, isError: error? }.compact
       end
     end
   end
diff --git a/test/mcp/tool/response_test.rb b/test/mcp/tool/response_test.rb
new file mode 100644
index 00000000..8c2978e5
--- /dev/null
+++ b/test/mcp/tool/response_test.rb
@@ -0,0 +1,77 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+module MCP
+  class Tool
+    class ResponseTest < ActiveSupport::TestCase
+      test "#initialize with content" do
+        content = [{
+          type: "text",
+          text: "Unauthorized",
+        }]
+        response = Response.new(content)
+
+        assert_equal content, response.content
+        refute response.error?
+      end
+
+      test "#initialize with content and error set to true" do
+        content = [{
+          type: "text",
+          text: "Unauthorized",
+        }]
+        response = Response.new(content, error: true)
+
+        assert_equal content, response.content
+        assert response.error?
+      end
+
+      test "#initialize with content and error explicitly set to false" do
+        content = [{
+          type: "text",
+          text: "Unauthorized",
+        }]
+        response = Response.new(content, error: false)
+
+        assert_equal content, response.content
+        refute response.error?
+      end
+
+      test "#error? for a standard response" do
+        response = Response.new(nil, error: false)
+        refute response.error?
+      end
+
+      test "#error? for an error response" do
+        response = Response.new(nil, error: true)
+        assert response.error?
+      end
+
+      test "#to_h for a standard response" do
+        content = [{
+          type: "text",
+          text: "Unauthorized",
+        }]
+        response = Response.new(content)
+        actual = response.to_h
+
+        assert_equal [:content, :isError].sort, actual.keys.sort
+        assert_equal content, actual[:content]
+        refute actual[:isError]
+      end
+
+      test "#to_h for an error response" do
+        content = [{
+          type: "text",
+          text: "Unauthorized",
+        }]
+        response = Response.new(content, error: true)
+        actual = response.to_h
+        assert_equal [:content, :isError].sort, actual.keys.sort
+        assert_equal content, actual[:content]
+        assert actual[:isError]
+      end
+    end
+  end
+end
diff --git a/test/mcp/tool_test.rb b/test/mcp/tool_test.rb
index abd3fcb3..76c95444 100644
--- a/test/mcp/tool_test.rb
+++ b/test/mcp/tool_test.rb
@@ -45,7 +45,7 @@ def call(message:, server_context: nil)
       tool = TestTool
       response = tool.call(message: "test")
       assert_equal response.content, [{ type: "text", content: "OK" }]
-      assert_equal response.is_error, false
+      refute response.error?
     end
 
     test "allows declarative definition of tools as classes" do
@@ -250,7 +250,7 @@ def call(message:, server_context: nil)
       tool = TypedTestTool
       response = tool.call(message: "test")
       assert_equal response.content, [{ type: "text", content: "OK" }]
-      assert_equal response.is_error, false
+      refute response.error?
     end
 
     class TestToolWithoutServerContext < Tool