From a554c2096eaecacf55e8f6dc9e7df3a5c4020e9b Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 09:17:57 +0900 Subject: [PATCH 01/37] fix(dispatcher): make logging/setLevel a library builtin Advertising the logging capability (use Urchin.Server, logging: true) now makes logging/setLevel succeed and apply the level to the session even when the server does not export set_log_level/2; an exported set_log_level/2 is still called as a hook. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 7 ++++ lib/urchin/dispatcher.ex | 23 +++++++++-- test/urchin/dispatcher_test.exs | 67 ++++++++++++++++++++++++++++++++- 3 files changed, 92 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6547917..b14ae5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `application/json`. - `SECURITY.md` with a threat model, deployment checklist and vulnerability reporting. +### Changed + +- `logging/setLevel` is now a library builtin: advertising the `logging` capability (via + `use Urchin.Server, logging: true`) makes `logging/setLevel` succeed and apply the level to + the session even when the server does not export `set_log_level/2`. An exported + `set_log_level/2` is still invoked as a hook. + ## [0.2.0] - 2026-06-05 ### Added diff --git a/lib/urchin/dispatcher.ex b/lib/urchin/dispatcher.ex index 338c90a..6e76431 100644 --- a/lib/urchin/dispatcher.ex +++ b/lib/urchin/dispatcher.ex @@ -10,7 +10,7 @@ defmodule Urchin.Dispatcher do require Logger - alias Urchin.{Context, Error, Protocol, Result} + alias Urchin.{Context, Error, Protocol, Result, Session} @doc """ Handles an `initialize` request. @@ -169,10 +169,17 @@ defmodule Urchin.Dispatcher do end defp do_handle(server, "logging/setLevel", params, ctx) do - with_callback(server, :set_log_level, 2, fn -> - level = require_string(params, "level") + level = require_string(params, "level") + + # logging/setLevel is a library builtin: apply the level to the session first, then call + # the server's set_log_level/2 as an optional hook when it is defined. + set_session_log_level(ctx, level) + + if exported?(server, :set_log_level, 2) do empty_result(server.set_log_level(level, ctx), ctx) - end) + else + {:ok, %{}} + end end defp do_handle(_server, method, _params, _ctx) do @@ -264,6 +271,14 @@ defmodule Urchin.Dispatcher do end end + # Apply the client-requested log level to the session when one exists; a nil session + # (e.g. a handler invoked in a unit test) is a no-op. + defp set_session_log_level(%Context{session: session}, level) when is_pid(session) do + Session.set_log_level(session, level) + end + + defp set_session_log_level(_ctx, _level), do: :ok + defp capabilities(server) do if exported?(server, :capabilities, 0), do: server.capabilities(), else: %{} end diff --git a/test/urchin/dispatcher_test.exs b/test/urchin/dispatcher_test.exs index dceb407..57eb49e 100644 --- a/test/urchin/dispatcher_test.exs +++ b/test/urchin/dispatcher_test.exs @@ -1,8 +1,25 @@ +defmodule Urchin.DispatcherTest.LoggingServer do + @moduledoc false + # Exports set_log_level/2 so the builtin-plus-hook path can be exercised. + use Urchin.Server, name: "logging", version: "1.0.0", logging: true + + @impl true + def set_log_level(level, ctx) do + case ctx.assigns do + %{test_pid: pid} -> send(pid, {:set_log_level_called, level}) + _ -> :ok + end + + :ok + end +end + defmodule Urchin.DispatcherTest do use ExUnit.Case, async: true - alias Urchin.{Context, Dispatcher} + alias Urchin.{Context, Dispatcher, Session} alias Urchin.Test.EchoServer + alias Urchin.DispatcherTest.LoggingServer defp ctx, do: %Context{} @@ -263,4 +280,52 @@ defmodule Urchin.DispatcherTest do assert error.code == -32_602 end end + + describe "logging/setLevel" do + test "succeeds as a builtin without a server callback" do + assert {:ok, %{}} = + Dispatcher.handle_request( + EchoServer, + "logging/setLevel", + %{"level" => "warning"}, + ctx() + ) + end + + test "rejects a missing level param" do + assert {:error, error} = + Dispatcher.handle_request(EchoServer, "logging/setLevel", %{}, ctx()) + + assert error.code == -32_602 + end + + test "reflects the requested level on the session" do + {:ok, _id, pid} = Session.start(server: EchoServer, protocol_version: "2025-11-25") + on_exit(fn -> Session.terminate(pid) end) + + assert {:ok, %{}} = + Dispatcher.handle_request( + EchoServer, + "logging/setLevel", + %{"level" => "error"}, + %Context{session: pid} + ) + + assert Session.snapshot(pid).min_log_level == "error" + end + + test "invokes an exported set_log_level/2 hook" do + ctx = %Context{assigns: %{test_pid: self()}} + + assert {:ok, %{}} = + Dispatcher.handle_request( + LoggingServer, + "logging/setLevel", + %{"level" => "info"}, + ctx + ) + + assert_received {:set_log_level_called, "info"} + end + end end From d5e8ec6db9bdfb7195a4ba40aa7f950902342b61 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 09:20:50 +0900 Subject: [PATCH 02/37] feat(lifecycle): add enforce_initialized gating option Surface the session's initialized flag and add an opt-in :enforce_initialized transport option (default false) that rejects operation requests received before notifications/initialized with invalid_request; ping and logging/setLevel are always allowed. Default preserves current behavior. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 4 ++ lib/urchin/context.ex | 4 ++ lib/urchin/dispatcher.ex | 28 ++++++++++++- lib/urchin/session.ex | 3 +- lib/urchin/transport/streamable_http.ex | 9 ++++- test/urchin/dispatcher_test.exs | 35 ++++++++++++++++ .../urchin/transport/streamable_http_test.exs | 40 +++++++++++++++++++ 7 files changed, 120 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b14ae5a..fe71c25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `415 Unsupported Media Type` for POST requests whose `Content-Type` is not `application/json`. - `SECURITY.md` with a threat model, deployment checklist and vulnerability reporting. +- `:enforce_initialized` transport option (default `false`) rejecting operation requests + received before the client sends `notifications/initialized` with `invalid_request`; + `ping` and `logging/setLevel` are always allowed. The default may be flipped to `true` in + a future minor release. ### Changed diff --git a/lib/urchin/context.ex b/lib/urchin/context.ex index a6cb0db..1ba42fc 100644 --- a/lib/urchin/context.ex +++ b/lib/urchin/context.ex @@ -35,6 +35,8 @@ defmodule Urchin.Context do min_log_level: "debug", expose_internal_errors: false, validate_arguments: false, + initialized: false, + enforce_initialized: false, cancelled_ref: nil ] @@ -54,6 +56,8 @@ defmodule Urchin.Context do min_log_level: String.t(), expose_internal_errors: boolean(), validate_arguments: boolean(), + initialized: boolean(), + enforce_initialized: boolean(), cancelled_ref: reference() | nil } diff --git a/lib/urchin/dispatcher.ex b/lib/urchin/dispatcher.ex index 6e76431..1e9bb1b 100644 --- a/lib/urchin/dispatcher.ex +++ b/lib/urchin/dispatcher.ex @@ -64,7 +64,11 @@ defmodule Urchin.Dispatcher do end def handle_request(server, method, params, ctx) do - do_handle(server, method, params, ctx) + # Lifecycle gate: when enforce_initialized is on and notifications/initialized has not been + # received, reject operation requests other than ping and logging with invalid_request. + with :ok <- check_initialized(method, ctx) do + do_handle(server, method, params, ctx) + end rescue error in Urchin.Error -> {:error, error} @@ -77,6 +81,28 @@ defmodule Urchin.Dispatcher do {:error, Error.internal_error(generic_or(ctx, "Handler threw: " <> inspect(value)))} end + # The gate is a no-op unless :enforce_initialized is set and the session is not yet + # initialized. `notifications/initialized` is a notification routed straight into the + # session, so it never reaches this request-only path. + defp check_initialized(_method, %Context{enforce_initialized: false}), do: :ok + defp check_initialized(_method, %Context{initialized: true}), do: :ok + + defp check_initialized(method, %Context{}) do + if pre_init_allowed?(method) do + :ok + else + {:error, + Error.invalid_request( + "Server not initialized: send notifications/initialized before #{method}" + )} + end + end + + # ping and logging are allowed before initialization completes (per the MCP lifecycle). + defp pre_init_allowed?("ping"), do: true + defp pre_init_allowed?("logging/setLevel"), do: true + defp pre_init_allowed?(_method), do: false + # ping is always available regardless of declared capabilities. defp do_handle(_server, "ping", _params, _ctx), do: {:ok, %{}} diff --git a/lib/urchin/session.ex b/lib/urchin/session.ex index 973fd80..cf53cf7 100644 --- a/lib/urchin/session.ex +++ b/lib/urchin/session.ex @@ -194,7 +194,8 @@ defmodule Urchin.Session do protocol_version: state.protocol_version, client_info: state.client_info, client_capabilities: state.client_capabilities, - min_log_level: state.min_log_level + min_log_level: state.min_log_level, + initialized: state.initialized } {:reply, snapshot, touch(state)} diff --git a/lib/urchin/transport/streamable_http.ex b/lib/urchin/transport/streamable_http.ex index e7ed5f1..f60cb05 100644 --- a/lib/urchin/transport/streamable_http.ex +++ b/lib/urchin/transport/streamable_http.ex @@ -33,6 +33,10 @@ defmodule Urchin.Transport.StreamableHTTP do * `:validate_arguments` - validate `tools/call` arguments against each tool's `input_schema` (DSL tools) before the handler runs, rejecting a mismatch with `invalid_params` (default `false`). See `Urchin.Schema` for the supported subset. + * `:enforce_initialized` - reject operation requests received before the client has sent + `notifications/initialized` with `invalid_request`; `ping` and `logging/setLevel` are + always allowed (default `false`). The default may be flipped to `true` in a future + minor release. * `:auth` - an `Urchin.Auth` (or keyword options) to require OAuth 2.1 bearer tokens on every request; `nil` (default) serves MCP unauthenticated. The metadata discovery endpoint is served by `Urchin.Endpoint`/`Urchin.Auth.Metadata`, not this plug. @@ -69,6 +73,7 @@ defmodule Urchin.Transport.StreamableHTTP do validate_protocol_version: Keyword.get(opts, :validate_protocol_version, true), expose_internal_errors: Keyword.get(opts, :expose_internal_errors, false), validate_arguments: Keyword.get(opts, :validate_arguments, false), + enforce_initialized: Keyword.get(opts, :enforce_initialized, false), max_sessions: positive_integer_opt!(opts, :max_sessions), session_idle_timeout: positive_integer_opt!(opts, :session_idle_timeout), session_max_lifetime: positive_integer_opt!(opts, :session_max_lifetime), @@ -244,7 +249,9 @@ defmodule Urchin.Transport.StreamableHTTP do auth: conn_auth(conn), min_log_level: snapshot.min_log_level, expose_internal_errors: config.expose_internal_errors, - validate_arguments: config.validate_arguments + validate_arguments: config.validate_arguments, + initialized: snapshot.initialized, + enforce_initialized: config.enforce_initialized, } {task_pid, task_ref} = diff --git a/test/urchin/dispatcher_test.exs b/test/urchin/dispatcher_test.exs index 57eb49e..f28da4d 100644 --- a/test/urchin/dispatcher_test.exs +++ b/test/urchin/dispatcher_test.exs @@ -328,4 +328,39 @@ defmodule Urchin.DispatcherTest do assert_received {:set_log_level_called, "info"} end end + + describe "initialized gating" do + test "does not gate by default even when not initialized" do + assert {:ok, %{tools: _}} = + Dispatcher.handle_request(EchoServer, "tools/list", %{}, %Context{}) + end + + test "rejects operation requests before initialized when enforced" do + ctx = %Context{enforce_initialized: true, initialized: false} + assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/list", %{}, ctx) + assert error.code == -32_600 + end + + test "allows ping before initialized when enforced" do + ctx = %Context{enforce_initialized: true, initialized: false} + assert {:ok, %{}} = Dispatcher.handle_request(EchoServer, "ping", %{}, ctx) + end + + test "allows logging/setLevel before initialized when enforced" do + ctx = %Context{enforce_initialized: true, initialized: false} + + assert {:ok, %{}} = + Dispatcher.handle_request( + EchoServer, + "logging/setLevel", + %{"level" => "info"}, + ctx + ) + end + + test "allows operation requests once initialized" do + ctx = %Context{enforce_initialized: true, initialized: true} + assert {:ok, %{tools: _}} = Dispatcher.handle_request(EchoServer, "tools/list", %{}, ctx) + end + end end diff --git a/test/urchin/transport/streamable_http_test.exs b/test/urchin/transport/streamable_http_test.exs index 66749b7..8a782fb 100644 --- a/test/urchin/transport/streamable_http_test.exs +++ b/test/urchin/transport/streamable_http_test.exs @@ -323,4 +323,44 @@ defmodule Urchin.Transport.StreamableHTTPTest do assert status.("x-application/json") == 415 end end + + describe "enforce_initialized" do + test "gates operation requests until notifications/initialized" do + opts = StreamableHTTP.init(server: EchoServer, enforce_initialized: true) + + init_conn = + post( + %{ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: %{ + "protocolVersion" => "2025-11-25", + "capabilities" => %{}, + "clientInfo" => %{"name" => "c", "version" => "1"} + } + }, + [], + opts + ) + + assert init_conn.status == 200 + [session_id] = get_resp_header(init_conn, "mcp-session-id") + headers = [{"mcp-session-id", session_id}, {"mcp-protocol-version", "2025-11-25"}] + + # Before notifications/initialized: rejected as invalid_request. + before = post(%{jsonrpc: "2.0", id: 2, method: "tools/list"}, headers, opts) + assert before.status == 200 + assert Jason.decode!(before.resp_body)["error"]["code"] == -32_600 + + # Acknowledge initialization. + ack = post(%{jsonrpc: "2.0", method: "notifications/initialized"}, headers, opts) + assert ack.status == 202 + + # After: allowed. + after_conn = post(%{jsonrpc: "2.0", id: 3, method: "tools/list"}, headers, opts) + assert after_conn.status == 200 + assert is_list(Jason.decode!(after_conn.resp_body)["result"]["tools"]) + end + end end From a30f115334a3e86e12a986f36a0e90dbb314d648 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 09:22:19 +0900 Subject: [PATCH 03/37] feat(dispatcher): add tool_errors option for tool-result errors Add a :tool_errors transport option (:json_rpc default | :result). With :result, a tools/call handler's {:error, binary} is returned as an isError CallToolResult so the model can self-correct, instead of a JSON-RPC internal error. Protocol errors via {:error, %Urchin.Error{}} always remain JSON-RPC errors. Default preserves current behavior. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 4 ++++ lib/urchin/context.ex | 2 ++ lib/urchin/dispatcher.ex | 12 +++++++++++- lib/urchin/transport/streamable_http.ex | 20 ++++++++++++++++++++ test/support/echo_server.ex | 10 ++++++++++ test/urchin/dispatcher_test.exs | 23 +++++++++++++++++++++++ 6 files changed, 70 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe71c25..a2077fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 received before the client sends `notifications/initialized` with `invalid_request`; `ping` and `logging/setLevel` are always allowed. The default may be flipped to `true` in a future minor release. +- `:tool_errors` transport option (`:json_rpc` default | `:result`). With `:result`, a + `tools/call` handler's `{:error, message}` (string) is returned as a `CallToolResult` with + `isError: true` so the model can self-correct, instead of a JSON-RPC internal error. A + protocol error returned as `{:error, %Urchin.Error{}}` is always a JSON-RPC error. ### Changed diff --git a/lib/urchin/context.ex b/lib/urchin/context.ex index 1ba42fc..79571c4 100644 --- a/lib/urchin/context.ex +++ b/lib/urchin/context.ex @@ -37,6 +37,7 @@ defmodule Urchin.Context do validate_arguments: false, initialized: false, enforce_initialized: false, + tool_errors: :json_rpc, cancelled_ref: nil ] @@ -58,6 +59,7 @@ defmodule Urchin.Context do validate_arguments: boolean(), initialized: boolean(), enforce_initialized: boolean(), + tool_errors: :json_rpc | :result, cancelled_ref: reference() | nil } diff --git a/lib/urchin/dispatcher.ex b/lib/urchin/dispatcher.ex index 1e9bb1b..3ab4d8e 100644 --- a/lib/urchin/dispatcher.ex +++ b/lib/urchin/dispatcher.ex @@ -220,7 +220,7 @@ defmodule Urchin.Dispatcher do %Result.CallTool{} = result -> {:ok, Result.CallTool.to_map(result)} {:ok, content} when is_list(content) -> {:ok, %{content: content, isError: false}} {:ok, content, opts} when is_list(content) -> {:ok, call_tool_map(content, opts)} - other -> normalize_error(other, ctx) + other -> tool_error_result(other, ctx) end rescue exception -> @@ -234,6 +234,16 @@ defmodule Urchin.Dispatcher do {:ok, %{content: [Urchin.Content.text(text)], isError: true}} end + # With tool_errors: :result, a handler's {:error, binary} becomes an isError tool result + # (so the model can self-correct) instead of a JSON-RPC error. A protocol-level + # {:error, %Error{}} and every other shape still go through normalize_error. + defp tool_error_result({:error, message}, %Context{tool_errors: :result}) + when is_binary(message) do + {:ok, %{content: [Urchin.Content.text(message)], isError: true}} + end + + defp tool_error_result(other, ctx), do: normalize_error(other, ctx) + defp call_tool_map(content, opts) do %{content: content, isError: opts[:is_error] || false} |> maybe_put(:structuredContent, opts[:structured_content]) diff --git a/lib/urchin/transport/streamable_http.ex b/lib/urchin/transport/streamable_http.ex index f60cb05..6488bf5 100644 --- a/lib/urchin/transport/streamable_http.ex +++ b/lib/urchin/transport/streamable_http.ex @@ -37,6 +37,11 @@ defmodule Urchin.Transport.StreamableHTTP do `notifications/initialized` with `invalid_request`; `ping` and `logging/setLevel` are always allowed (default `false`). The default may be flipped to `true` in a future minor release. + * `:tool_errors` - how a `tools/call` handler's `{:error, binary}` is surfaced: + `:json_rpc` (default) returns it as a JSON-RPC internal error; `:result` returns it as a + `CallToolResult` with `isError: true` so the model can self-correct. A protocol error + returned as `{:error, %Urchin.Error{}}` is always a JSON-RPC error. Other methods are + unaffected. * `:auth` - an `Urchin.Auth` (or keyword options) to require OAuth 2.1 bearer tokens on every request; `nil` (default) serves MCP unauthenticated. The metadata discovery endpoint is served by `Urchin.Endpoint`/`Urchin.Auth.Metadata`, not this plug. @@ -74,6 +79,7 @@ defmodule Urchin.Transport.StreamableHTTP do expose_internal_errors: Keyword.get(opts, :expose_internal_errors, false), validate_arguments: Keyword.get(opts, :validate_arguments, false), enforce_initialized: Keyword.get(opts, :enforce_initialized, false), + tool_errors: tool_errors_opt!(opts), max_sessions: positive_integer_opt!(opts, :max_sessions), session_idle_timeout: positive_integer_opt!(opts, :session_idle_timeout), session_max_lifetime: positive_integer_opt!(opts, :session_max_lifetime), @@ -252,6 +258,7 @@ defmodule Urchin.Transport.StreamableHTTP do validate_arguments: config.validate_arguments, initialized: snapshot.initialized, enforce_initialized: config.enforce_initialized, + tool_errors: config.tool_errors } {task_pid, task_ref} = @@ -525,6 +532,19 @@ defmodule Urchin.Transport.StreamableHTTP do end end + # :tool_errors selects how a tool handler's {:error, binary} surfaces. Default :json_rpc + # keeps the current behavior; :result turns it into an isError tool result. Fail fast on a + # bad value at startup, matching how the session-limit options are validated. + defp tool_errors_opt!(opts) do + case Keyword.get(opts, :tool_errors, :json_rpc) do + value when value in [:json_rpc, :result] -> + value + + other -> + raise ArgumentError, ":tool_errors must be :json_rpc or :result, got: #{inspect(other)}" + end + end + ## Origin / Accept defp validate_origin(conn, config) do diff --git a/test/support/echo_server.ex b/test/support/echo_server.ex index 596b6d2..432d0e8 100644 --- a/test/support/echo_server.ex +++ b/test/support/echo_server.ex @@ -47,6 +47,16 @@ defmodule Urchin.Test.EchoServer do {:error, {:db, "postgres://secret@host"}} end + tool "failing", description: "Returns a string error reason" do + _ = {args, ctx} + {:error, "tool said no"} + end + + tool "protocol_error", description: "Returns an Urchin.Error protocol error" do + _ = {args, ctx} + {:error, Urchin.Error.invalid_params("bad")} + end + tool "secret", description: "Requires the secret:read scope", scopes: ["secret:read"] do diff --git a/test/urchin/dispatcher_test.exs b/test/urchin/dispatcher_test.exs index f28da4d..c391c7b 100644 --- a/test/urchin/dispatcher_test.exs +++ b/test/urchin/dispatcher_test.exs @@ -363,4 +363,27 @@ defmodule Urchin.DispatcherTest do assert {:ok, %{tools: _}} = Dispatcher.handle_request(EchoServer, "tools/list", %{}, ctx) end end + + describe "tool_errors option" do + test "default :json_rpc keeps a binary tool error as a JSON-RPC error" do + params = %{"name" => "failing", "arguments" => %{}} + assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx()) + assert error.code == -32_603 + assert error.message == "tool said no" + end + + test ":result turns a binary tool error into an isError result" do + ctx = %Context{tool_errors: :result} + params = %{"name" => "failing", "arguments" => %{}} + assert {:ok, result} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) + assert result == %{content: [%{type: "text", text: "tool said no"}], isError: true} + end + + test ":result still passes a protocol error through as a JSON-RPC error" do + ctx = %Context{tool_errors: :result} + params = %{"name" => "protocol_error", "arguments" => %{}} + assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) + assert error.code == -32_602 + end + end end From 8499d5e82d3d24f70c54032377aed6919cfacdf3 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 09:22:48 +0900 Subject: [PATCH 04/37] feat(server): validate tool names and reject duplicates at compile time Duplicate tool names within a server are now always rejected at compile time (a silently shadowed duplicate was previously accepted). Add an opt-in validate_tool_names: true option to additionally enforce that every literal tool name matches ~r/^[a-zA-Z0-9_.-]{1,128}$/ (default false). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 7 ++++ lib/urchin/server.ex | 57 +++++++++++++++++++++++++++++- test/urchin/server_test.exs | 70 +++++++++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2077fa..8daaf5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `tools/call` handler's `{:error, message}` (string) is returned as a `CallToolResult` with `isError: true` so the model can self-correct, instead of a JSON-RPC internal error. A protocol error returned as `{:error, %Urchin.Error{}}` is always a JSON-RPC error. +- `validate_tool_names: true` option for `use Urchin.Server` enforcing, at compile time, that + every literal tool name matches `~r/^[a-zA-Z0-9_.-]{1,128}$/` (default `false`). ### Changed @@ -48,6 +50,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 the session even when the server does not export `set_log_level/2`. An exported `set_log_level/2` is still invoked as a hook. +### Fixed + +- Duplicate tool names within a server are now rejected at compile time (a silently shadowed + duplicate was previously accepted, with the last declaration winning). + ## [0.2.0] - 2026-06-05 ### Added diff --git a/lib/urchin/server.ex b/lib/urchin/server.ex index 9184374..e993aa5 100644 --- a/lib/urchin/server.ex +++ b/lib/urchin/server.ex @@ -34,6 +34,11 @@ defmodule Urchin.Server do Capabilities are derived automatically from the declared features. + Duplicate tool names within a server are rejected at compile time. Pass + `validate_tool_names: true` to `use Urchin.Server` to additionally enforce that every literal + tool name matches `~r/^[a-zA-Z0-9_.-]{1,128}$/` (default `false`); a non-matching name raises + `ArgumentError`. + ## Behaviour Implement the callbacks directly for full control or stateful servers. All @@ -54,6 +59,11 @@ defmodule Urchin.Server do alias Urchin.{Context, Error} + # Constrained tool-name charset. The MCP schema imposes no pattern, but this is the + # de-facto convention shared by common tool-calling SDKs; dots and dashes are permitted + # for namespacing. Enforced only when the server opts in via :validate_tool_names. + @tool_name_pattern ~r/^[a-zA-Z0-9_.-]{1,128}$/ + @type cursor :: String.t() | nil @type list_result(item) :: {:ok, [item]} | {:ok, [item], cursor()} | {:error, Error.t() | String.t()} @@ -247,6 +257,48 @@ defmodule Urchin.Server do raise ArgumentError, "tool :scopes must be a list of strings, got: #{inspect(other)}" end + # Duplicate tool names within a server are always rejected at compile time (a silently + # shadowed duplicate is a bug). The name-pattern check is opt-in via :validate_tool_names. + # Non-literal names (a variable or call) cannot be compared statically and are skipped, + # mirroring handler_name/2. + defp validate_tool_names!(tool_dispatch, opts) do + names = + tool_dispatch + |> Enum.map(fn {name, _fname, _scopes} -> name end) + |> Enum.filter(&is_binary/1) + + validate_unique_tool_names!(names) + + if Keyword.get(opts, :validate_tool_names, false) do + Enum.each(names, &validate_tool_name_pattern!/1) + end + + :ok + end + + defp validate_tool_name_pattern!(name) do + if not Regex.match?(@tool_name_pattern, name) do + raise ArgumentError, + "tool name #{inspect(name)} is invalid; must match #{inspect(@tool_name_pattern.source)}" + end + end + + defp validate_unique_tool_names!(names) do + duplicates = + names + |> Enum.frequencies() + |> Enum.filter(fn {_name, count} -> count > 1 end) + |> Enum.map(fn {name, _count} -> name end) + + case duplicates do + [] -> + :ok + + dups -> + raise ArgumentError, "duplicate tool name(s): #{Enum.map_join(dups, ", ", &inspect/1)}" + end + end + # Generates a deterministic private handler name. Determinism matters because Mix's # incremental compiler assumes stable output for unchanged sources. defp handler_name(prefix, name) when is_binary(name) do @@ -263,7 +315,10 @@ defmodule Urchin.Server do mod = env.module opts = Module.get_attribute(mod, :mcp_opts) || [] - has_tools? = Module.get_attribute(mod, :mcp_tool_dispatch) != [] + tool_dispatch = Module.get_attribute(mod, :mcp_tool_dispatch) || [] + validate_tool_names!(tool_dispatch, opts) + + has_tools? = tool_dispatch != [] has_resources? = Module.get_attribute(mod, :mcp_resources) != [] has_templates? = Module.get_attribute(mod, :mcp_resource_templates) != [] has_prompts? = Module.get_attribute(mod, :mcp_prompt_dispatch) != [] diff --git a/test/urchin/server_test.exs b/test/urchin/server_test.exs index 290fcab..2158061 100644 --- a/test/urchin/server_test.exs +++ b/test/urchin/server_test.exs @@ -32,4 +32,74 @@ defmodule Urchin.ServerTest do assert [{Urchin.ServerTest.GoodScopes, _}] = Code.compile_string(source) end end + + describe "tool name validation" do + test "rejects an invalid tool name at compile time when opted in" do + source = """ + defmodule Urchin.ServerTest.BadName do + use Urchin.Server, name: "bad", version: "1.0.0", validate_tool_names: true + + tool "bad name!" do + {:ok, [Urchin.Content.text("ok")]} + end + end + """ + + assert_raise ArgumentError, ~r/tool name .* is invalid/, fn -> + Code.compile_string(source) + end + end + + test "rejects duplicate tool names at compile time (always on)" do + source = """ + defmodule Urchin.ServerTest.DupNames do + use Urchin.Server, name: "dup", version: "1.0.0" + + tool "dup" do + {:ok, [Urchin.Content.text("a")]} + end + + tool "dup" do + {:ok, [Urchin.Content.text("b")]} + end + end + """ + + assert_raise ArgumentError, ~r/duplicate tool name/, fn -> + Code.compile_string(source) + end + end + + test "accepts valid, unique names when opted in" do + source = """ + defmodule Urchin.ServerTest.GoodNames do + use Urchin.Server, name: "good-names", version: "1.0.0", validate_tool_names: true + + tool "echo" do + {:ok, [Urchin.Content.text("a")]} + end + + tool "my.tool-1" do + {:ok, [Urchin.Content.text("b")]} + end + end + """ + + assert [{Urchin.ServerTest.GoodNames, _} | _] = Code.compile_string(source) + end + + test "does not enforce the name pattern by default" do + source = """ + defmodule Urchin.ServerTest.UncheckedName do + use Urchin.Server, name: "unchecked", version: "1.0.0" + + tool "bad name!" do + {:ok, [Urchin.Content.text("a")]} + end + end + """ + + assert [{Urchin.ServerTest.UncheckedName, _} | _] = Code.compile_string(source) + end + end end From 0cb8e27d47e24a6ceed639172ebadffa2ddfc14c Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 09:23:10 +0900 Subject: [PATCH 05/37] feat(transport): add sse_buffer_limit option Expose the per-session GET-stream replay buffer size as a :sse_buffer_limit transport option, forwarded to Session.start only when set (default nil keeps the Session default of 100). Validated as a positive integer at startup. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 3 ++ lib/urchin/transport/streamable_http.ex | 32 +++++++++++++------ test/urchin/session_lifecycle_test.exs | 11 +++++++ .../urchin/transport/streamable_http_test.exs | 15 +++++++++ 4 files changed, 51 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8daaf5b..7b5cfe0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 protocol error returned as `{:error, %Urchin.Error{}}` is always a JSON-RPC error. - `validate_tool_names: true` option for `use Urchin.Server` enforcing, at compile time, that every literal tool name matches `~r/^[a-zA-Z0-9_.-]{1,128}$/` (default `false`). +- `:sse_buffer_limit` transport option (default `100`) forwarding the per-session GET-stream + replay buffer size to the session; previously only configurable on `Urchin.Session` + directly. ### Changed diff --git a/lib/urchin/transport/streamable_http.ex b/lib/urchin/transport/streamable_http.ex index 6488bf5..2c0049f 100644 --- a/lib/urchin/transport/streamable_http.ex +++ b/lib/urchin/transport/streamable_http.ex @@ -42,6 +42,8 @@ defmodule Urchin.Transport.StreamableHTTP do `CallToolResult` with `isError: true` so the model can self-correct. A protocol error returned as `{:error, %Urchin.Error{}}` is always a JSON-RPC error. Other methods are unaffected. + * `:sse_buffer_limit` - the maximum number of recent general-stream (GET SSE) events each + session keeps for resumption replay (default `100`). A positive integer or `nil`. * `:auth` - an `Urchin.Auth` (or keyword options) to require OAuth 2.1 bearer tokens on every request; `nil` (default) serves MCP unauthenticated. The metadata discovery endpoint is served by `Urchin.Endpoint`/`Urchin.Auth.Metadata`, not this plug. @@ -83,6 +85,7 @@ defmodule Urchin.Transport.StreamableHTTP do max_sessions: positive_integer_opt!(opts, :max_sessions), session_idle_timeout: positive_integer_opt!(opts, :session_idle_timeout), session_max_lifetime: positive_integer_opt!(opts, :session_max_lifetime), + sse_buffer_limit: positive_integer_opt!(opts, :sse_buffer_limit), auth: Auth.coerce!(Keyword.get(opts, :auth)) } end @@ -174,16 +177,20 @@ defmodule Urchin.Transport.StreamableHTTP do end defp start_session(conn, config, id, result, meta, server_state, reservation) do - case Session.start( - server: config.server, - server_state: server_state, - protocol_version: meta.protocol_version, - client_info: meta.client_info, - client_capabilities: meta.client_capabilities, - min_log_level: config.min_log_level, - idle_timeout: config.session_idle_timeout, - max_lifetime: config.session_max_lifetime - ) do + session_opts = + [ + server: config.server, + server_state: server_state, + protocol_version: meta.protocol_version, + client_info: meta.client_info, + client_capabilities: meta.client_capabilities, + min_log_level: config.min_log_level, + idle_timeout: config.session_idle_timeout, + max_lifetime: config.session_max_lifetime + ] + |> maybe_put_buffer_limit(config.sse_buffer_limit) + + case Session.start(session_opts) do {:ok, session_id, pid} -> # Hand the reserved slot to the session. If the limiter no longer knows the # reservation (e.g. it restarted since reserve/1), the session would be uncounted, @@ -545,6 +552,11 @@ defmodule Urchin.Transport.StreamableHTTP do end end + # When nil, omit the key so the Session keeps its own default (@default_buffer_limit). + # Passing buffer_limit: nil would defeat Keyword.get's default and crash push_general. + defp maybe_put_buffer_limit(opts, nil), do: opts + defp maybe_put_buffer_limit(opts, limit), do: Keyword.put(opts, :buffer_limit, limit) + ## Origin / Accept defp validate_origin(conn, config) do diff --git a/test/urchin/session_lifecycle_test.exs b/test/urchin/session_lifecycle_test.exs index 481d525..873f2e9 100644 --- a/test/urchin/session_lifecycle_test.exs +++ b/test/urchin/session_lifecycle_test.exs @@ -66,4 +66,15 @@ defmodule Urchin.SessionLifecycleTest do Session.terminate(pid) assert_receive :mcp_close, 1_000 end + + test "buffer_limit caps the general-stream replay buffer" do + {:ok, _id, pid} = start(buffer_limit: 1) + Session.notify(pid, "notifications/message", %{"n" => 1}) + Session.notify(pid, "notifications/message", %{"n" => 2}) + + {:ok, "g0", replay} = Session.register_general_stream(pid, self(), {"g0", 0}) + assert length(replay) == 1 + + Session.terminate(pid) + end end diff --git a/test/urchin/transport/streamable_http_test.exs b/test/urchin/transport/streamable_http_test.exs index 8a782fb..b10186f 100644 --- a/test/urchin/transport/streamable_http_test.exs +++ b/test/urchin/transport/streamable_http_test.exs @@ -363,4 +363,19 @@ defmodule Urchin.Transport.StreamableHTTPTest do assert is_list(Jason.decode!(after_conn.resp_body)["result"]["tools"]) end end + + describe "sse_buffer_limit option" do + test "validates and defaults" do + assert %{sse_buffer_limit: nil} = StreamableHTTP.init(server: EchoServer) + assert %{sse_buffer_limit: 5} = StreamableHTTP.init(server: EchoServer, sse_buffer_limit: 5) + + assert_raise ArgumentError, fn -> + StreamableHTTP.init(server: EchoServer, sse_buffer_limit: 0) + end + + assert_raise ArgumentError, fn -> + StreamableHTTP.init(server: EchoServer, sse_buffer_limit: -1) + end + end + end end From cd939511813c5c62274eedc4bd2b870e5df451f3 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 09:23:23 +0900 Subject: [PATCH 06/37] docs: scope the resumable SSE claim to the GET stream The README bullet implied every SSE stream is resumable; only the GET (general) stream supports Last-Event-ID replay. POST request streams are not replayable. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 2 ++ README.md | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b5cfe0..577d24a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Duplicate tool names within a server are now rejected at compile time (a silently shadowed duplicate was previously accepted, with the last declaration winning). +- README no longer claims unqualified "resumable SSE streams"; resumption is scoped to the + GET stream, matching the implementation. ## [0.2.0] - 2026-06-05 diff --git a/README.md b/README.md index 8812934..6784bf5 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ specification over the **Streamable HTTP** transport. - Mount as a `Plug` into Phoenix/Plug pipelines, or run standalone with Bandit. - Tools, resources, resource templates, prompts, completion and logging. - Server-initiated requests over SSE: sampling, elicitation and roots. -- Progress notifications, cancellation, pagination and resumable SSE streams. +- Progress notifications, cancellation, pagination and a resumable GET SSE stream. - Optional OAuth 2.1 authorization: RFC 9728 discovery and pluggable token validation. > This library implements the server side only. The stdio transport is intentionally From 326b5c2013a233ee5a7e0fe7a753fb3245adc687 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 09:44:18 +0900 Subject: [PATCH 07/37] docs: clarify :sse_buffer_limit default is nil (CodeRabbit) The transport option defaults to nil; an unset value preserves Urchin.Session's internal default buffer of 100. The previous wording (default 100) conflated the transport option default with the effective session fallback. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 6 +++--- lib/urchin/transport/streamable_http.ex | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 577d24a..6fefe6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,9 +42,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 protocol error returned as `{:error, %Urchin.Error{}}` is always a JSON-RPC error. - `validate_tool_names: true` option for `use Urchin.Server` enforcing, at compile time, that every literal tool name matches `~r/^[a-zA-Z0-9_.-]{1,128}$/` (default `false`). -- `:sse_buffer_limit` transport option (default `100`) forwarding the per-session GET-stream - replay buffer size to the session; previously only configurable on `Urchin.Session` - directly. +- `:sse_buffer_limit` transport option (default `nil`, preserving the session's internal + default of `100`) forwarding the per-session GET-stream replay buffer size to the session; + previously only configurable on `Urchin.Session` directly. ### Changed diff --git a/lib/urchin/transport/streamable_http.ex b/lib/urchin/transport/streamable_http.ex index 2c0049f..1a95d32 100644 --- a/lib/urchin/transport/streamable_http.ex +++ b/lib/urchin/transport/streamable_http.ex @@ -43,7 +43,8 @@ defmodule Urchin.Transport.StreamableHTTP do returned as `{:error, %Urchin.Error{}}` is always a JSON-RPC error. Other methods are unaffected. * `:sse_buffer_limit` - the maximum number of recent general-stream (GET SSE) events each - session keeps for resumption replay (default `100`). A positive integer or `nil`. + session keeps for resumption replay. Defaults to `nil`, which preserves the session's + internal default of `100`. A positive integer or `nil`. * `:auth` - an `Urchin.Auth` (or keyword options) to require OAuth 2.1 bearer tokens on every request; `nil` (default) serves MCP unauthenticated. The metadata discovery endpoint is served by `Urchin.Endpoint`/`Urchin.Auth.Metadata`, not this plug. From 3c7a282eb8119b525cb9344982a4435c1c8de183 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 09:57:52 +0900 Subject: [PATCH 08/37] fix(dispatcher): harden logging/setLevel and restrict pre-init to ping Addresses review of the P0 PR: - Validate the requested log level against the MCP levels; reject unknown values with invalid_params (-32602) instead of storing them on the session. - Offer logging/setLevel only when the server advertises the logging capability; otherwise return method_not_found (-32601). - Update the session level only after the optional set_log_level/2 hook succeeds, so a failed hook leaves no state change. - Allow only ping before notifications/initialized (client-side logging/setLevel is no longer pre-init exempt); fix the related comment and docs. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 13 ++--- lib/urchin/context.ex | 4 ++ lib/urchin/dispatcher.ex | 51 +++++++++++++----- lib/urchin/transport/streamable_http.ex | 5 +- test/urchin/dispatcher_test.exs | 69 +++++++++++++++++++++++-- 5 files changed, 118 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fefe6c..8cc7a61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,8 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `SECURITY.md` with a threat model, deployment checklist and vulnerability reporting. - `:enforce_initialized` transport option (default `false`) rejecting operation requests received before the client sends `notifications/initialized` with `invalid_request`; - `ping` and `logging/setLevel` are always allowed. The default may be flipped to `true` in - a future minor release. + only `ping` is allowed. The default may be flipped to `true` in a future minor release. - `:tool_errors` transport option (`:json_rpc` default | `:result`). With `:result`, a `tools/call` handler's `{:error, message}` (string) is returned as a `CallToolResult` with `isError: true` so the model can self-correct, instead of a JSON-RPC internal error. A @@ -48,10 +47,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- `logging/setLevel` is now a library builtin: advertising the `logging` capability (via - `use Urchin.Server, logging: true`) makes `logging/setLevel` succeed and apply the level to - the session even when the server does not export `set_log_level/2`. An exported - `set_log_level/2` is still invoked as a hook. +- `logging/setLevel` is now a library builtin: when the server advertises the `logging` + capability (via `use Urchin.Server, logging: true`) it succeeds and applies the level to the + session even without a `set_log_level/2` callback. The level is validated against the MCP log + levels (`invalid_params` otherwise), an exported `set_log_level/2` still runs as a hook, and + the session level is updated only after the hook succeeds. Servers that do not advertise + `logging` return `method_not_found`. ### Fixed diff --git a/lib/urchin/context.ex b/lib/urchin/context.ex index 79571c4..5bc4cee 100644 --- a/lib/urchin/context.ex +++ b/lib/urchin/context.ex @@ -65,6 +65,10 @@ defmodule Urchin.Context do @default_request_timeout 30_000 + @doc "Returns the valid MCP log levels, in increasing severity order." + @spec log_levels() :: [String.t()] + def log_levels, do: @log_levels + @doc "Returns the user state established by `c:Urchin.Server.init/1`." @spec state(t()) :: term() def state(%__MODULE__{state: state}), do: state diff --git a/lib/urchin/dispatcher.ex b/lib/urchin/dispatcher.ex index 3ab4d8e..051faca 100644 --- a/lib/urchin/dispatcher.ex +++ b/lib/urchin/dispatcher.ex @@ -65,7 +65,7 @@ defmodule Urchin.Dispatcher do def handle_request(server, method, params, ctx) do # Lifecycle gate: when enforce_initialized is on and notifications/initialized has not been - # received, reject operation requests other than ping and logging with invalid_request. + # received, reject operation requests other than ping with invalid_request. with :ok <- check_initialized(method, ctx) do do_handle(server, method, params, ctx) end @@ -98,9 +98,9 @@ defmodule Urchin.Dispatcher do end end - # ping and logging are allowed before initialization completes (per the MCP lifecycle). + # Only ping is allowed before the client sends notifications/initialized; per the MCP + # lifecycle a client should not send other requests until initialization completes. defp pre_init_allowed?("ping"), do: true - defp pre_init_allowed?("logging/setLevel"), do: true defp pre_init_allowed?(_method), do: false # ping is always available regardless of declared capabilities. @@ -195,16 +195,19 @@ defmodule Urchin.Dispatcher do end defp do_handle(server, "logging/setLevel", params, ctx) do - level = require_string(params, "level") - - # logging/setLevel is a library builtin: apply the level to the session first, then call - # the server's set_log_level/2 as an optional hook when it is defined. - set_session_log_level(ctx, level) - - if exported?(server, :set_log_level, 2) do - empty_result(server.set_log_level(level, ctx), ctx) + # logging/setLevel is a library builtin, available only when the server advertises the + # logging capability. The level is validated, the optional set_log_level/2 hook runs, and + # the session level is updated only after both succeed, so a failed call leaves no change. + if logging_advertised?(server) do + level = require_string(params, "level") + + with :ok <- validate_log_level(level), + :ok <- run_log_level_hook(server, level, ctx) do + set_session_log_level(ctx, level) + {:ok, %{}} + end else - {:ok, %{}} + {:error, Error.method_not_found("Server does not support logging/setLevel")} end end @@ -315,6 +318,30 @@ defmodule Urchin.Dispatcher do defp set_session_log_level(_ctx, _level), do: :ok + # logging/setLevel is offered only when the server advertises the logging capability. + defp logging_advertised?(server), do: Map.has_key?(capabilities(server), :logging) + + defp validate_log_level(level) do + if level in Context.log_levels() do + :ok + else + {:error, Error.invalid_params("Invalid log level: " <> level)} + end + end + + # Runs the optional set_log_level/2 hook; a missing hook is a no-op success. + defp run_log_level_hook(server, level, ctx) do + if exported?(server, :set_log_level, 2) do + case server.set_log_level(level, ctx) do + :ok -> :ok + {:ok, _} -> :ok + other -> normalize_error(other, ctx) + end + else + :ok + end + end + defp capabilities(server) do if exported?(server, :capabilities, 0), do: server.capabilities(), else: %{} end diff --git a/lib/urchin/transport/streamable_http.ex b/lib/urchin/transport/streamable_http.ex index 1a95d32..828d15f 100644 --- a/lib/urchin/transport/streamable_http.ex +++ b/lib/urchin/transport/streamable_http.ex @@ -34,9 +34,8 @@ defmodule Urchin.Transport.StreamableHTTP do `input_schema` (DSL tools) before the handler runs, rejecting a mismatch with `invalid_params` (default `false`). See `Urchin.Schema` for the supported subset. * `:enforce_initialized` - reject operation requests received before the client has sent - `notifications/initialized` with `invalid_request`; `ping` and `logging/setLevel` are - always allowed (default `false`). The default may be flipped to `true` in a future - minor release. + `notifications/initialized` with `invalid_request`; only `ping` is allowed (default + `false`). The default may be flipped to `true` in a future minor release. * `:tool_errors` - how a `tools/call` handler's `{:error, binary}` is surfaced: `:json_rpc` (default) returns it as a JSON-RPC internal error; `:result` returns it as a `CallToolResult` with `isError: true` so the model can self-correct. A protocol error diff --git a/test/urchin/dispatcher_test.exs b/test/urchin/dispatcher_test.exs index c391c7b..a26bd72 100644 --- a/test/urchin/dispatcher_test.exs +++ b/test/urchin/dispatcher_test.exs @@ -14,12 +14,27 @@ defmodule Urchin.DispatcherTest.LoggingServer do end end +defmodule Urchin.DispatcherTest.NoLoggingServer do + @moduledoc false + # Advertises no logging capability. + use Urchin.Server, name: "no-logging", version: "1.0.0" +end + +defmodule Urchin.DispatcherTest.FailingLoggingServer do + @moduledoc false + # Advertises logging (via the callback) but the set_log_level/2 hook always fails. + use Urchin.Server, name: "failing-logging", version: "1.0.0" + + @impl true + def set_log_level(_level, _ctx), do: {:error, "nope"} +end + defmodule Urchin.DispatcherTest do use ExUnit.Case, async: true alias Urchin.{Context, Dispatcher, Session} alias Urchin.Test.EchoServer - alias Urchin.DispatcherTest.LoggingServer + alias Urchin.DispatcherTest.{LoggingServer, NoLoggingServer, FailingLoggingServer} defp ctx, do: %Context{} @@ -327,6 +342,52 @@ defmodule Urchin.DispatcherTest do assert_received {:set_log_level_called, "info"} end + + test "rejects an invalid level with -32602 and leaves the session unchanged" do + {:ok, _id, pid} = Session.start(server: EchoServer, protocol_version: "2025-11-25") + on_exit(fn -> Session.terminate(pid) end) + + assert {:error, error} = + Dispatcher.handle_request( + EchoServer, + "logging/setLevel", + %{"level" => "verbose"}, + %Context{session: pid} + ) + + assert error.code == -32_602 + assert Session.snapshot(pid).min_log_level != "verbose" + end + + test "is not available unless the server advertises the logging capability" do + assert {:error, error} = + Dispatcher.handle_request( + NoLoggingServer, + "logging/setLevel", + %{"level" => "info"}, + ctx() + ) + + assert error.code == -32_601 + end + + test "leaves the session unchanged when the set_log_level/2 hook fails" do + {:ok, _id, pid} = + Session.start(server: FailingLoggingServer, protocol_version: "2025-11-25") + + on_exit(fn -> Session.terminate(pid) end) + before = Session.snapshot(pid).min_log_level + + assert {:error, _error} = + Dispatcher.handle_request( + FailingLoggingServer, + "logging/setLevel", + %{"level" => "warning"}, + %Context{session: pid} + ) + + assert Session.snapshot(pid).min_log_level == before + end end describe "initialized gating" do @@ -346,16 +407,18 @@ defmodule Urchin.DispatcherTest do assert {:ok, %{}} = Dispatcher.handle_request(EchoServer, "ping", %{}, ctx) end - test "allows logging/setLevel before initialized when enforced" do + test "rejects logging/setLevel before initialized when enforced (only ping is allowed)" do ctx = %Context{enforce_initialized: true, initialized: false} - assert {:ok, %{}} = + assert {:error, error} = Dispatcher.handle_request( EchoServer, "logging/setLevel", %{"level" => "info"}, ctx ) + + assert error.code == -32_600 end test "allows operation requests once initialized" do From 0d308928c8ec540ca2f41c6b751bf72ec10707cc Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 09:57:52 +0900 Subject: [PATCH 09/37] test(transport): verify sse_buffer_limit reaches the session Add an integration test that initializes a session through the transport with sse_buffer_limit: 1 and confirms the replay buffer is capped, proving the option is forwarded from init/1 to Session.start. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../urchin/transport/streamable_http_test.exs | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/test/urchin/transport/streamable_http_test.exs b/test/urchin/transport/streamable_http_test.exs index b10186f..b6f13d4 100644 --- a/test/urchin/transport/streamable_http_test.exs +++ b/test/urchin/transport/streamable_http_test.exs @@ -377,5 +377,38 @@ defmodule Urchin.Transport.StreamableHTTPTest do StreamableHTTP.init(server: EchoServer, sse_buffer_limit: -1) end end + + test "is forwarded to the session created by the transport" do + opts = StreamableHTTP.init(server: EchoServer, sse_buffer_limit: 1) + + conn = + post( + %{ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: %{ + "protocolVersion" => "2025-11-25", + "capabilities" => %{}, + "clientInfo" => %{"name" => "c", "version" => "1"} + } + }, + [], + opts + ) + + assert conn.status == 200 + [session_id] = get_resp_header(conn, "mcp-session-id") + pid = Urchin.Session.whereis(session_id) + + Urchin.Session.notify(pid, "notifications/message", %{"n" => 1}) + Urchin.Session.notify(pid, "notifications/message", %{"n" => 2}) + + # With the buffer capped at 1, only the most recent event is available to replay. + {:ok, "g0", replay} = Urchin.Session.register_general_stream(pid, self(), {"g0", 0}) + assert length(replay) == 1 + + Urchin.Session.terminate(pid) + end end end From 2cd9a5b7074c3eab14dd70d791b2483aeb16ccd6 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 10:56:28 +0900 Subject: [PATCH 10/37] fix(lifecycle): commit notifications/initialized and log level synchronously Make notifications/initialized a synchronous Session.mark_initialized/1 call and turn Session.set_log_level/2 into a call, so once the client has the 202 ack the next request always observes the committed state in the snapshot. Removes any reliance on mailbox-ordering reasoning for the enforce_initialized gate. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/urchin/session.ex | 18 +++++++++++++----- lib/urchin/transport/streamable_http.ex | 12 ++++++++++++ test/urchin/session_lifecycle_test.exs | 8 ++++++++ 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/lib/urchin/session.ex b/lib/urchin/session.ex index cf53cf7..946a87c 100644 --- a/lib/urchin/session.ex +++ b/lib/urchin/session.ex @@ -116,7 +116,11 @@ defmodule Urchin.Session do @doc "Sets the minimum log level the client wishes to receive." @spec set_log_level(pid(), String.t()) :: :ok - def set_log_level(pid, level), do: GenServer.cast(pid, {:set_log_level, level}) + def set_log_level(pid, level), do: GenServer.call(pid, {:set_log_level, level}) + + @doc "Marks the session initialized synchronously (after notifications/initialized)." + @spec mark_initialized(pid()) :: :ok + def mark_initialized(pid), do: GenServer.call(pid, :mark_initialized) @doc "Registers (or replaces) the GET general stream, returning events to replay." @spec register_general_stream(pid(), pid(), {String.t(), non_neg_integer()} | nil) :: @@ -201,6 +205,14 @@ defmodule Urchin.Session do {:reply, snapshot, touch(state)} end + def handle_call(:mark_initialized, _from, state) do + {:reply, :ok, touch(%{state | initialized: true})} + end + + def handle_call({:set_log_level, level}, _from, state) do + {:reply, :ok, %{state | min_log_level: level}} + end + def handle_call({:start_request, request_id, task_pid, owner_pid}, _from, state) do seq = state.post_seq + 1 stream_id = "p" <> Integer.to_string(seq) @@ -269,10 +281,6 @@ defmodule Urchin.Session do {:noreply, %{state | outbound: drop_outbound(state.outbound, id)}} end - def handle_cast({:set_log_level, level}, state) do - {:noreply, %{state | min_log_level: level}} - end - def handle_cast({:subscribe, uri}, state) do {:noreply, %{state | subscriptions: MapSet.put(state.subscriptions, uri)}} end diff --git a/lib/urchin/transport/streamable_http.ex b/lib/urchin/transport/streamable_http.ex index 828d15f..b561431 100644 --- a/lib/urchin/transport/streamable_http.ex +++ b/lib/urchin/transport/streamable_http.ex @@ -225,6 +225,18 @@ defmodule Urchin.Transport.StreamableHTTP do end # Notifications and responses are acknowledged with 202 and routed into the session. + # notifications/initialized is committed synchronously so that, once the client has the + # 202, a subsequent request always observes initialized: true in the session snapshot. + defp route_session_message( + conn, + _config, + session_pid, + {:notification, "notifications/initialized", _params} + ) do + :ok = Session.mark_initialized(session_pid) + send_resp(conn, 202, "") + end + defp route_session_message(conn, _config, session_pid, {:notification, _m, _p} = msg) do Session.handle_client_message(session_pid, msg) send_resp(conn, 202, "") diff --git a/test/urchin/session_lifecycle_test.exs b/test/urchin/session_lifecycle_test.exs index 873f2e9..56be190 100644 --- a/test/urchin/session_lifecycle_test.exs +++ b/test/urchin/session_lifecycle_test.exs @@ -77,4 +77,12 @@ defmodule Urchin.SessionLifecycleTest do Session.terminate(pid) end + + test "mark_initialized/1 synchronously sets initialized" do + {:ok, _id, pid} = start([]) + refute Session.snapshot(pid).initialized + assert :ok = Session.mark_initialized(pid) + assert Session.snapshot(pid).initialized + Session.terminate(pid) + end end From 93eaa957cdb9e999f1d89a12308ab76cc3a9626d Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 10:56:29 +0900 Subject: [PATCH 11/37] fix(dispatcher): accept string-keyed logging capability A hand-written capabilities/0 may return JSON-shaped string keys; logging_advertised? now checks both :logging and "logging". Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/urchin/dispatcher.ex | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/urchin/dispatcher.ex b/lib/urchin/dispatcher.ex index 051faca..08df4f8 100644 --- a/lib/urchin/dispatcher.ex +++ b/lib/urchin/dispatcher.ex @@ -319,7 +319,11 @@ defmodule Urchin.Dispatcher do defp set_session_log_level(_ctx, _level), do: :ok # logging/setLevel is offered only when the server advertises the logging capability. - defp logging_advertised?(server), do: Map.has_key?(capabilities(server), :logging) + # Accept both atom (DSL-derived) and string (hand-written, JSON-shaped) capability keys. + defp logging_advertised?(server) do + caps = capabilities(server) + Map.has_key?(caps, :logging) or Map.has_key?(caps, "logging") + end defp validate_log_level(level) do if level in Context.log_levels() do From 453320ade6cbc9195ddb189cfb3632611787ccd6 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 10:56:29 +0900 Subject: [PATCH 12/37] docs: scope duplicate-tool-name rejection to DSL-declared names Compile-time duplicate detection only inspects DSL declarations; reword the moduledoc and CHANGELOG so it does not imply hand-written list_tools/2 is covered. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 4 ++-- lib/urchin/server.ex | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cc7a61..b37f0ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,8 +56,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Duplicate tool names within a server are now rejected at compile time (a silently shadowed - duplicate was previously accepted, with the last declaration winning). +- Duplicate tool names declared via the DSL are now rejected at compile time (a silently + shadowed duplicate was previously accepted, with the last declaration winning). - README no longer claims unqualified "resumable SSE streams"; resumption is scoped to the GET stream, matching the implementation. diff --git a/lib/urchin/server.ex b/lib/urchin/server.ex index e993aa5..7ea1470 100644 --- a/lib/urchin/server.ex +++ b/lib/urchin/server.ex @@ -34,7 +34,7 @@ defmodule Urchin.Server do Capabilities are derived automatically from the declared features. - Duplicate tool names within a server are rejected at compile time. Pass + Duplicate tool names declared via the DSL are rejected at compile time. Pass `validate_tool_names: true` to `use Urchin.Server` to additionally enforce that every literal tool name matches `~r/^[a-zA-Z0-9_.-]{1,128}$/` (default `false`); a non-matching name raises `ArgumentError`. From a0c393dac867cc6bbba002891f09051abd68a1fd Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 13:07:55 +0900 Subject: [PATCH 13/37] fix(session): fall back to default replay buffer when buffer_limit is nil Session.start(buffer_limit: nil) previously stored nil, crashing push_general via Enum.take(_, nil). The transport already guarded against this; this hardens the public Session API itself. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/urchin/session.ex | 2 +- test/urchin/session_lifecycle_test.exs | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/urchin/session.ex b/lib/urchin/session.ex index 946a87c..157cd86 100644 --- a/lib/urchin/session.ex +++ b/lib/urchin/session.ex @@ -166,7 +166,7 @@ defmodule Urchin.Session do general_stream_id: "g0", general_seq: 0, general_buffer: [], - buffer_limit: Keyword.get(opts, :buffer_limit, @default_buffer_limit), + buffer_limit: Keyword.get(opts, :buffer_limit) || @default_buffer_limit, inflight: %{}, cancelled: MapSet.new(), outbound: %{}, diff --git a/test/urchin/session_lifecycle_test.exs b/test/urchin/session_lifecycle_test.exs index 56be190..450646d 100644 --- a/test/urchin/session_lifecycle_test.exs +++ b/test/urchin/session_lifecycle_test.exs @@ -85,4 +85,14 @@ defmodule Urchin.SessionLifecycleTest do assert Session.snapshot(pid).initialized Session.terminate(pid) end + + test "buffer_limit: nil falls back to the default replay buffer" do + {:ok, _id, pid} = start(buffer_limit: nil) + Session.notify(pid, "notifications/message", %{"n" => 1}) + + {:ok, "g0", replay} = Session.register_general_stream(pid, self(), {"g0", 0}) + assert length(replay) == 1 + + Session.terminate(pid) + end end From 83600e532c5e2372d1c64f236e4871a89bf46591 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 13:46:02 +0900 Subject: [PATCH 14/37] feat(dispatcher): default tool_errors to :result for spec compliance Per review: a tools/call handler's {:error, binary} is now returned as an isError CallToolResult by default (the MCP-spec behavior) so models can self-correct. Set tool_errors: :json_rpc for the legacy JSON-RPC-error behavior. Protocol errors via {:error, %Urchin.Error{}} still surface as JSON-RPC errors. This changes the prior default where such a handler error became a JSON-RPC error. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 10 ++++++---- lib/urchin/context.ex | 2 +- lib/urchin/transport/streamable_http.ex | 10 +++++----- test/urchin/dispatcher_test.exs | 16 ++++++++-------- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b37f0ff..13a3b7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,10 +35,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `:enforce_initialized` transport option (default `false`) rejecting operation requests received before the client sends `notifications/initialized` with `invalid_request`; only `ping` is allowed. The default may be flipped to `true` in a future minor release. -- `:tool_errors` transport option (`:json_rpc` default | `:result`). With `:result`, a - `tools/call` handler's `{:error, message}` (string) is returned as a `CallToolResult` with - `isError: true` so the model can self-correct, instead of a JSON-RPC internal error. A - protocol error returned as `{:error, %Urchin.Error{}}` is always a JSON-RPC error. +- `:tool_errors` transport option (`:result` default | `:json_rpc`). By default a `tools/call` + handler's `{:error, message}` (string) is now returned as a `CallToolResult` with + `isError: true` so the model can self-correct (the spec-compliant behavior); set `:json_rpc` + for the legacy behavior of returning a JSON-RPC internal error. A protocol error returned as + `{:error, %Urchin.Error{}}` is always a JSON-RPC error. Note: this changes the prior behavior + where such a handler error became a JSON-RPC error. - `validate_tool_names: true` option for `use Urchin.Server` enforcing, at compile time, that every literal tool name matches `~r/^[a-zA-Z0-9_.-]{1,128}$/` (default `false`). - `:sse_buffer_limit` transport option (default `nil`, preserving the session's internal diff --git a/lib/urchin/context.ex b/lib/urchin/context.ex index 5bc4cee..6c50fe6 100644 --- a/lib/urchin/context.ex +++ b/lib/urchin/context.ex @@ -37,7 +37,7 @@ defmodule Urchin.Context do validate_arguments: false, initialized: false, enforce_initialized: false, - tool_errors: :json_rpc, + tool_errors: :result, cancelled_ref: nil ] diff --git a/lib/urchin/transport/streamable_http.ex b/lib/urchin/transport/streamable_http.ex index b561431..fce2b60 100644 --- a/lib/urchin/transport/streamable_http.ex +++ b/lib/urchin/transport/streamable_http.ex @@ -37,10 +37,10 @@ defmodule Urchin.Transport.StreamableHTTP do `notifications/initialized` with `invalid_request`; only `ping` is allowed (default `false`). The default may be flipped to `true` in a future minor release. * `:tool_errors` - how a `tools/call` handler's `{:error, binary}` is surfaced: - `:json_rpc` (default) returns it as a JSON-RPC internal error; `:result` returns it as a - `CallToolResult` with `isError: true` so the model can self-correct. A protocol error - returned as `{:error, %Urchin.Error{}}` is always a JSON-RPC error. Other methods are - unaffected. + `:result` (default) returns it as a `CallToolResult` with `isError: true` so the model can + self-correct (the spec-compliant behavior); `:json_rpc` returns it as a JSON-RPC internal + error. A protocol error returned as `{:error, %Urchin.Error{}}` is always a JSON-RPC error. + Other methods are unaffected. * `:sse_buffer_limit` - the maximum number of recent general-stream (GET SSE) events each session keeps for resumption replay. Defaults to `nil`, which preserves the session's internal default of `100`. A positive integer or `nil`. @@ -555,7 +555,7 @@ defmodule Urchin.Transport.StreamableHTTP do # keeps the current behavior; :result turns it into an isError tool result. Fail fast on a # bad value at startup, matching how the session-limit options are validated. defp tool_errors_opt!(opts) do - case Keyword.get(opts, :tool_errors, :json_rpc) do + case Keyword.get(opts, :tool_errors, :result) do value when value in [:json_rpc, :result] -> value diff --git a/test/urchin/dispatcher_test.exs b/test/urchin/dispatcher_test.exs index a26bd72..9bb043f 100644 --- a/test/urchin/dispatcher_test.exs +++ b/test/urchin/dispatcher_test.exs @@ -428,18 +428,18 @@ defmodule Urchin.DispatcherTest do end describe "tool_errors option" do - test "default :json_rpc keeps a binary tool error as a JSON-RPC error" do + test "the default (:result) returns a binary tool error as an isError result" do params = %{"name" => "failing", "arguments" => %{}} - assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx()) - assert error.code == -32_603 - assert error.message == "tool said no" + assert {:ok, result} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx()) + assert result == %{content: [%{type: "text", text: "tool said no"}], isError: true} end - test ":result turns a binary tool error into an isError result" do - ctx = %Context{tool_errors: :result} + test ":json_rpc keeps a binary tool error as a JSON-RPC error" do + ctx = %Context{tool_errors: :json_rpc} params = %{"name" => "failing", "arguments" => %{}} - assert {:ok, result} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) - assert result == %{content: [%{type: "text", text: "tool said no"}], isError: true} + assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) + assert error.code == -32_603 + assert error.message == "tool said no" end test ":result still passes a protocol error through as a JSON-RPC error" do From 389ee33af451f0433ec59fc8829ea6280fdffe1c Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 14:17:14 +0900 Subject: [PATCH 15/37] fix(server): anchor tool-name validation with \A...\z ^...$ matches before a trailing newline, so a tool name like "abc\n" passed validation; \A...\z requires a full-string match. Adds a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/urchin/server.ex | 4 ++-- test/urchin/server_test.exs | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/urchin/server.ex b/lib/urchin/server.ex index 7ea1470..8dc150b 100644 --- a/lib/urchin/server.ex +++ b/lib/urchin/server.ex @@ -36,7 +36,7 @@ defmodule Urchin.Server do Duplicate tool names declared via the DSL are rejected at compile time. Pass `validate_tool_names: true` to `use Urchin.Server` to additionally enforce that every literal - tool name matches `~r/^[a-zA-Z0-9_.-]{1,128}$/` (default `false`); a non-matching name raises + tool name matches `~r/\A[a-zA-Z0-9_.-]{1,128}\z/` (default `false`); a non-matching name raises `ArgumentError`. ## Behaviour @@ -62,7 +62,7 @@ defmodule Urchin.Server do # Constrained tool-name charset. The MCP schema imposes no pattern, but this is the # de-facto convention shared by common tool-calling SDKs; dots and dashes are permitted # for namespacing. Enforced only when the server opts in via :validate_tool_names. - @tool_name_pattern ~r/^[a-zA-Z0-9_.-]{1,128}$/ + @tool_name_pattern ~r/\A[a-zA-Z0-9_.-]{1,128}\z/ @type cursor :: String.t() | nil @type list_result(item) :: diff --git a/test/urchin/server_test.exs b/test/urchin/server_test.exs index 2158061..9041203 100644 --- a/test/urchin/server_test.exs +++ b/test/urchin/server_test.exs @@ -50,6 +50,22 @@ defmodule Urchin.ServerTest do end end + test "rejects a tool name with a trailing newline at compile time" do + source = """ + defmodule Urchin.ServerTest.NewlineName do + use Urchin.Server, name: "nl", version: "1.0.0", validate_tool_names: true + + tool "abc\\n" do + {:ok, [Urchin.Content.text("ok")]} + end + end + """ + + assert_raise ArgumentError, ~r/tool name .* is invalid/, fn -> + Code.compile_string(source) + end + end + test "rejects duplicate tool names at compile time (always on)" do source = """ defmodule Urchin.ServerTest.DupNames do From dabe2f9e8a27078c0ba54d6cdb1fdea98f9f7881 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 14:17:14 +0900 Subject: [PATCH 16/37] fix(dispatcher): clean error if the session dies during logging/setLevel set_session_log_level now catches the GenServer.call exit from a dead session and returns invalid_request instead of crashing the handler with a generic error. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/urchin/dispatcher.ex | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/urchin/dispatcher.ex b/lib/urchin/dispatcher.ex index 08df4f8..14b76d2 100644 --- a/lib/urchin/dispatcher.ex +++ b/lib/urchin/dispatcher.ex @@ -202,8 +202,8 @@ defmodule Urchin.Dispatcher do level = require_string(params, "level") with :ok <- validate_log_level(level), - :ok <- run_log_level_hook(server, level, ctx) do - set_session_log_level(ctx, level) + :ok <- run_log_level_hook(server, level, ctx), + :ok <- set_session_log_level(ctx, level) do {:ok, %{}} end else @@ -311,9 +311,13 @@ defmodule Urchin.Dispatcher do end # Apply the client-requested log level to the session when one exists; a nil session - # (e.g. a handler invoked in a unit test) is a no-op. + # (e.g. a handler invoked in a unit test) is a no-op. If the session died mid-request, + # surface a clean error rather than a generic crash from the GenServer.call exit. defp set_session_log_level(%Context{session: session}, level) when is_pid(session) do Session.set_log_level(session, level) + :ok + catch + :exit, _ -> {:error, Error.invalid_request("Session not found")} end defp set_session_log_level(_ctx, _level), do: :ok From dddae9bace14576cc8f8e811734cba6b2bd789d5 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 14:17:14 +0900 Subject: [PATCH 17/37] test(transport): cover tool_errors, logging/setLevel and notification wiring Integration tests through StreamableHTTP.init/1 -> Context -> Dispatcher: default tool_errors :result returns an isError result, logging/setLevel updates the session min_log_level, and client notifications are accepted (202) before initialized. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../urchin/transport/streamable_http_test.exs | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/test/urchin/transport/streamable_http_test.exs b/test/urchin/transport/streamable_http_test.exs index b6f13d4..8ac62b1 100644 --- a/test/urchin/transport/streamable_http_test.exs +++ b/test/urchin/transport/streamable_http_test.exs @@ -411,4 +411,77 @@ defmodule Urchin.Transport.StreamableHTTPTest do Urchin.Session.terminate(pid) end end + + describe "tool_errors over the transport" do + test "a handler {:error, message} becomes an isError result by default" do + {session_id, _} = init_session() + + conn = + call_with_session( + %{ + jsonrpc: "2.0", + id: 20, + method: "tools/call", + params: %{name: "failing", arguments: %{}} + }, + session_id + ) + + assert conn.status == 200 + body = Jason.decode!(conn.resp_body) + assert body["result"]["isError"] == true + assert body["result"]["content"] == [%{"type" => "text", "text" => "tool said no"}] + end + end + + describe "logging/setLevel over the transport" do + test "updates the session min_log_level" do + {session_id, _} = init_session() + pid = Urchin.Session.whereis(session_id) + + conn = + call_with_session( + %{jsonrpc: "2.0", id: 21, method: "logging/setLevel", params: %{level: "error"}}, + session_id + ) + + assert conn.status == 200 + assert Jason.decode!(conn.resp_body)["result"] == %{} + assert Urchin.Session.snapshot(pid).min_log_level == "error" + end + end + + describe "enforce_initialized notifications" do + test "client notifications are accepted with 202 before initialized" do + opts = StreamableHTTP.init(server: EchoServer, enforce_initialized: true) + + init = + post( + %{ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: %{ + "protocolVersion" => "2025-11-25", + "capabilities" => %{}, + "clientInfo" => %{"name" => "c", "version" => "1"} + } + }, + [], + opts + ) + + [session_id] = get_resp_header(init, "mcp-session-id") + headers = [{"mcp-session-id", session_id}, {"mcp-protocol-version", "2025-11-25"}] + + cancelled = + post( + %{jsonrpc: "2.0", method: "notifications/cancelled", params: %{requestId: "x"}}, + headers, + opts + ) + + assert cancelled.status == 202 + end + end end From 3116120d8c0dbc7024ef97064e2f7dcc22be89b4 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 14:17:14 +0900 Subject: [PATCH 18/37] docs: tool-name regex, enforce_initialized strict mode, GET-only SSE replay Sync the tool-name pattern wording to \A...\z, note that enforce_initialized: true is required for strict MCP lifecycle compliance, and clarify that only the GET general stream is replayed. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 5 +++-- README.md | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13a3b7f..3929019 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,7 +34,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `SECURITY.md` with a threat model, deployment checklist and vulnerability reporting. - `:enforce_initialized` transport option (default `false`) rejecting operation requests received before the client sends `notifications/initialized` with `invalid_request`; - only `ping` is allowed. The default may be flipped to `true` in a future minor release. + only `ping` is allowed. The default may be flipped to `true` in a future minor release; set + `true` for strict MCP lifecycle compliance. - `:tool_errors` transport option (`:result` default | `:json_rpc`). By default a `tools/call` handler's `{:error, message}` (string) is now returned as a `CallToolResult` with `isError: true` so the model can self-correct (the spec-compliant behavior); set `:json_rpc` @@ -42,7 +43,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `{:error, %Urchin.Error{}}` is always a JSON-RPC error. Note: this changes the prior behavior where such a handler error became a JSON-RPC error. - `validate_tool_names: true` option for `use Urchin.Server` enforcing, at compile time, that - every literal tool name matches `~r/^[a-zA-Z0-9_.-]{1,128}$/` (default `false`). + every literal tool name matches `~r/\A[a-zA-Z0-9_.-]{1,128}\z/` (default `false`). - `:sse_buffer_limit` transport option (default `nil`, preserving the session's internal default of `100`) forwarding the per-session GET-stream replay buffer size to the session; previously only configurable on `Urchin.Session` directly. diff --git a/README.md b/README.md index 6784bf5..e5cfb84 100644 --- a/README.md +++ b/README.md @@ -370,7 +370,8 @@ The transport implements: a single endpoint serving POST/GET/DELETE, the JSON-vs-SSE response decision, `202 Accepted` for notifications and responses, `Origin` validation, `MCP-Session-Id` management, the `MCP-Protocol-Version` header, SSE priming events, per-stream event ids, and `Last-Event-ID` resumption of the GET -stream. +stream. Urchin currently replays the GET general stream only; POST request streams are +not replayed (the spec permits, but does not require, replaying either). ### Not included From ec45e88c3f6d5c2f5ee1b38c1d5335c4bf83c788 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 16:05:22 +0900 Subject: [PATCH 19/37] docs: align transport docs with the tool_errors :result default The :tool_errors default is :result, but several docs still described the legacy :json_rpc behavior or omitted the new transport options: - server.ex moduledoc: for tools/call an {:error, binary} (and a raised exception) is an isError CallToolResult by default, not a JSON-RPC error; only {:error, %Urchin.Error{}} and the other callbacks stay JSON-RPC errors. - streamable_http.ex: fix the tool_errors_opt!/1 comment that still claimed the default is :json_rpc. - README: add the :enforce_initialized, :tool_errors and :sse_buffer_limit rows to the options table, and make the manual scope-check example return an Urchin.Error so it stays a JSON-RPC error like the declarative scopes path. - CHANGELOG / SECURITY: separate error redaction from the tools/call response envelope, which is governed by :tool_errors. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 4 +++- README.md | 8 +++++++- SECURITY.md | 6 ++++-- lib/urchin/server.ex | 8 ++++++-- lib/urchin/transport/streamable_http.ex | 7 ++++--- 5 files changed, 24 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3929019..e9829f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `:expose_internal_errors` transport option (default `false`). Unexpected exceptions and malformed handler returns are now logged in full but return a generic message to the client; enable the option to surface the detail in development. Deliberate `Urchin.Error` - values and `{:error, message}` returns still pass through unchanged. + values and `{:error, message}` returns are never redacted — their `message`/`data` reach the + client unchanged. Whether a `tools/call` string error is delivered as a JSON-RPC error or an + `isError` result is governed separately by `:tool_errors` (see below). - Capability guards: `Urchin.Context.create_message/3`, `elicit/3` and `list_roots/2` return an error without contacting the client when it did not advertise the matching `sampling`/`elicitation`/`roots` capability. diff --git a/README.md b/README.md index e5cfb84..db9d6ff 100644 --- a/README.md +++ b/README.md @@ -287,7 +287,10 @@ tool "delete", description: "Delete a file" do if Urchin.Auth.Claims.has_scope?(Urchin.Context.auth(ctx), "files:write") do {:ok, [Urchin.Content.text("deleted")]} else - {:error, "files:write scope required"} + # Return an Urchin.Error so the denial is a JSON-RPC `invalid_request`, matching the + # declarative `scopes:` path. A bare string `{:error, "..."}` would instead surface as a + # `CallToolResult` with `isError: true` under the default `tool_errors: :result`. + {:error, Urchin.Error.invalid_request("files:write scope required")} end end ``` @@ -345,6 +348,9 @@ Passed to `Urchin.Transport.StreamableHTTP`, `Urchin.Endpoint` or `Urchin.start_ | `:validate_protocol_version` | `true` | validate the `MCP-Protocol-Version` header | | `:expose_internal_errors` | `false` | return raised-exception messages to the client (dev only); exceptions are always logged | | `:validate_arguments` | `false` | validate `tools/call` arguments against each tool's `input_schema` (see `Urchin.Schema`) | +| `:enforce_initialized` | `false` | reject operation requests received before `notifications/initialized` with `invalid_request`; only `ping` is allowed | +| `:tool_errors` | `:result` | how a `tools/call` handler's `{:error, binary}` is surfaced: `:result` returns a `CallToolResult` with `isError: true` so the model can self-correct; `:json_rpc` returns a JSON-RPC internal error. `{:error, %Urchin.Error{}}` is always a JSON-RPC error; other methods are unaffected | +| `:sse_buffer_limit` | `nil` | max recent GET-stream (general SSE) events kept per session for resumption replay (`nil` keeps the session default of `100`) | | `:max_sessions` | `nil` | reject new sessions with `503` past this many, atomically and before the server's `init/1` runs (`nil` = unlimited) | | `:session_idle_timeout` | `nil` | terminate a session after this many ms without client activity; a session serving a request is not reaped (`nil` = never) | | `:session_max_lifetime` | `nil` | terminate a session this many ms after creation regardless of activity; set above your longest tool run (`nil` = never) | diff --git a/SECURITY.md b/SECURITY.md index 853b7df..c5c3047 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -17,8 +17,10 @@ and does not protect against, and what you must add before exposing a server pub - **Error redaction.** Unexpected exceptions and malformed handler returns are logged in full and replaced with a generic message before reaching clients (`:expose_internal_errors`, default `false`, opts into the detail for development). Deliberate errors — `Urchin.Error` - values and `{:error, message}` returns — pass through unchanged, so keep secrets and - internals out of their `message`/`data`. + values and `{:error, message}` returns — are not redacted; their `message`/`data` reach the + client unchanged, so keep secrets and internals out of them. For `tools/call`, a string + `{:error, message}` is by default surfaced as a `CallToolResult` with `isError: true` (see + `:tool_errors`) rather than a JSON-RPC error. - **Capability-gated server-initiated requests.** `sampling/createMessage`, `elicitation/create` and `roots/list` are only sent when the client advertised the capability. diff --git a/lib/urchin/server.ex b/lib/urchin/server.ex index 8dc150b..daedbd6 100644 --- a/lib/urchin/server.ex +++ b/lib/urchin/server.ex @@ -53,8 +53,12 @@ defmodule Urchin.Server do * `read_resource/2`: `{:ok, contents}` or `{:error, reason}` * `get_prompt/3`: `{:ok, messages}` or `{:ok, messages, description}` - Any `{:error, reason}` where `reason` is a string or `Urchin.Error` becomes a JSON-RPC - error; raised exceptions become internal errors. + For every callback, an `{:error, %Urchin.Error{}}` becomes that JSON-RPC error. For + `call_tool/3`, an `{:error, binary}` (and a raised exception) is by default surfaced as a + `CallToolResult` with `isError: true` so the model can self-correct; set the transport's + `:tool_errors` to `:json_rpc` for the legacy behavior of returning a JSON-RPC internal error. + For the other callbacks an `{:error, binary}` becomes a JSON-RPC internal error, and raised + exceptions become internal errors. """ alias Urchin.{Context, Error} diff --git a/lib/urchin/transport/streamable_http.ex b/lib/urchin/transport/streamable_http.ex index fce2b60..5a4dc07 100644 --- a/lib/urchin/transport/streamable_http.ex +++ b/lib/urchin/transport/streamable_http.ex @@ -551,9 +551,10 @@ defmodule Urchin.Transport.StreamableHTTP do end end - # :tool_errors selects how a tool handler's {:error, binary} surfaces. Default :json_rpc - # keeps the current behavior; :result turns it into an isError tool result. Fail fast on a - # bad value at startup, matching how the session-limit options are validated. + # :tool_errors selects how a tool handler's {:error, binary} surfaces. Default :result returns + # it as an isError CallToolResult (spec-compliant, lets the model self-correct); :json_rpc is the + # opt-in legacy mode that returns a JSON-RPC internal error instead. Fail fast on a bad value at + # startup, matching how the session-limit options are validated. defp tool_errors_opt!(opts) do case Keyword.get(opts, :tool_errors, :result) do value when value in [:json_rpc, :result] -> From 3e6bfac37c223822a53494c89bcd4d77e229b115 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 16:05:22 +0900 Subject: [PATCH 20/37] test(transport): cover tool_errors :json_rpc path and init validation Add transport-level coverage for tool_errors: a :json_rpc handler error surfaces as a JSON-RPC error end-to-end, and StreamableHTTP.init validates the option (:result default, :json_rpc accepted, any other value raises). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../urchin/transport/streamable_http_test.exs | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/test/urchin/transport/streamable_http_test.exs b/test/urchin/transport/streamable_http_test.exs index 8ac62b1..82fdb6f 100644 --- a/test/urchin/transport/streamable_http_test.exs +++ b/test/urchin/transport/streamable_http_test.exs @@ -432,6 +432,44 @@ defmodule Urchin.Transport.StreamableHTTPTest do assert body["result"]["isError"] == true assert body["result"]["content"] == [%{"type" => "text", "text" => "tool said no"}] end + + test "with tool_errors: :json_rpc a handler {:error, message} becomes a JSON-RPC error" do + json_rpc_opts = StreamableHTTP.init(server: EchoServer, tool_errors: :json_rpc) + {session_id, _} = init_session() + + conn = + post( + %{ + jsonrpc: "2.0", + id: 21, + method: "tools/call", + params: %{name: "failing", arguments: %{}} + }, + [{"mcp-session-id", session_id}, {"mcp-protocol-version", "2025-11-25"}], + json_rpc_opts + ) + + assert conn.status == 200 + body = Jason.decode!(conn.resp_body) + assert body["error"]["code"] == -32_603 + assert body["error"]["message"] == "tool said no" + refute Map.has_key?(body, "result") + end + end + + describe "tool_errors option validation" do + test "defaults to :result and accepts :json_rpc" do + assert %{tool_errors: :result} = StreamableHTTP.init(server: EchoServer) + + assert %{tool_errors: :json_rpc} = + StreamableHTTP.init(server: EchoServer, tool_errors: :json_rpc) + end + + test "rejects an unknown value" do + assert_raise ArgumentError, fn -> + StreamableHTTP.init(server: EchoServer, tool_errors: :bad) + end + end end describe "logging/setLevel over the transport" do From 17961a748a0803a5471dec289ece10ad2fdce708 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 22:54:31 +0900 Subject: [PATCH 21/37] fix(transport): handle a session that vanishes during notifications/initialized This PR made notifications/initialized commit synchronously via a GenServer.call so the next request observes initialized: true. lookup_session/2 only proves the session was alive a moment earlier, so a session that terminates in between would make the call exit and crash the Plug process. Catch the exit and return a clean 404 "Session not found", mirroring lookup_session/2 and the existing logging/setLevel guard. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/urchin/transport/streamable_http.ex | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/lib/urchin/transport/streamable_http.ex b/lib/urchin/transport/streamable_http.ex index 5a4dc07..1d431a3 100644 --- a/lib/urchin/transport/streamable_http.ex +++ b/lib/urchin/transport/streamable_http.ex @@ -233,8 +233,10 @@ defmodule Urchin.Transport.StreamableHTTP do session_pid, {:notification, "notifications/initialized", _params} ) do - :ok = Session.mark_initialized(session_pid) - send_resp(conn, 202, "") + case mark_initialized_safe(session_pid) do + :ok -> send_resp(conn, 202, "") + {:error, status, error} -> send_error(conn, status, nil, error) + end end defp route_session_message(conn, _config, session_pid, {:notification, _m, _p} = msg) do @@ -486,6 +488,17 @@ defmodule Urchin.Transport.StreamableHTTP do end end + # notifications/initialized commits synchronously via a GenServer.call so the next request + # observes initialized: true. lookup_session/2 only proves the session was alive a moment ago, so + # a session that terminates in between would make the call exit and crash the Plug process; catch + # that and report a clean "Session not found", mirroring lookup_session/2 (and set_log_level). + defp mark_initialized_safe(session_pid) do + Session.mark_initialized(session_pid) + :ok + catch + :exit, _ -> {:error, 404, Error.invalid_request("Session not found")} + end + defp check_protocol_version(_conn, %{validate_protocol_version: false}), do: :ok defp check_protocol_version(conn, _config) do From e427c8c4d7476d281ba0cea198ec165e9b5d961c Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 22:55:09 +0900 Subject: [PATCH 22/37] feat(dispatcher): surface input-schema validation failures as tool errors Per the MCP tool error semantics, input validation is a tool-execution error, not a protocol error. With tool_errors: :result (the default) a tools/call input_schema mismatch is now an isError CallToolResult so the model can self-correct; under :json_rpc it stays a JSON-RPC invalid_params error (the legacy behavior). Malformed requests (missing/invalid name, non-object params), unknown tools, and scope denials remain JSON-RPC errors. Also fix the Urchin.Server moduledoc, which wrongly implied :tool_errors governs raised tool exceptions: a raising tool is always reported as an isError CallToolResult, regardless of :tool_errors. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 6 ++++-- README.md | 2 +- lib/urchin/dispatcher.ex | 10 ++++++++++ lib/urchin/server.ex | 19 +++++++++++-------- lib/urchin/transport/streamable_http.ex | 5 +++-- test/urchin/dispatcher_test.exs | 21 +++++++++++++++++++-- 6 files changed, 48 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9829f0..d25b7de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,8 +20,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 against `ctx.auth` before the handler runs, failing closed when the request carries no authorization. - `:validate_arguments` transport option (default `false`) validates `tools/call` arguments - against each DSL tool's `input_schema` and rejects a mismatch with `invalid_params` before - the handler runs. `Urchin.Schema` implements the supported (minimal) JSON Schema subset. + against each DSL tool's `input_schema` before the handler runs. A mismatch is treated as a + tool-input error: surfaced as a `CallToolResult` with `isError: true` by default (per + `:tool_errors`), or a JSON-RPC `invalid_params` error under `tool_errors: :json_rpc`. + `Urchin.Schema` implements the supported (minimal) JSON Schema subset. - `:expose_internal_errors` transport option (default `false`). Unexpected exceptions and malformed handler returns are now logged in full but return a generic message to the client; enable the option to surface the detail in development. Deliberate `Urchin.Error` diff --git a/README.md b/README.md index db9d6ff..d74a1ab 100644 --- a/README.md +++ b/README.md @@ -347,7 +347,7 @@ Passed to `Urchin.Transport.StreamableHTTP`, `Urchin.Endpoint` or `Urchin.start_ | `:request_timeout` | `60_000` | per-request handler timeout (ms) | | `:validate_protocol_version` | `true` | validate the `MCP-Protocol-Version` header | | `:expose_internal_errors` | `false` | return raised-exception messages to the client (dev only); exceptions are always logged | -| `:validate_arguments` | `false` | validate `tools/call` arguments against each tool's `input_schema` (see `Urchin.Schema`) | +| `:validate_arguments` | `false` | validate `tools/call` arguments against each tool's `input_schema` before the handler runs; a mismatch is surfaced per `:tool_errors` (an `isError` result by default). See `Urchin.Schema` | | `:enforce_initialized` | `false` | reject operation requests received before `notifications/initialized` with `invalid_request`; only `ping` is allowed | | `:tool_errors` | `:result` | how a `tools/call` handler's `{:error, binary}` is surfaced: `:result` returns a `CallToolResult` with `isError: true` so the model can self-correct; `:json_rpc` returns a JSON-RPC internal error. `{:error, %Urchin.Error{}}` is always a JSON-RPC error; other methods are unaffected | | `:sse_buffer_limit` | `nil` | max recent GET-stream (general SSE) events kept per session for resumption replay (`nil` keeps the session default of `100`) | diff --git a/lib/urchin/dispatcher.ex b/lib/urchin/dispatcher.ex index 14b76d2..a7186d4 100644 --- a/lib/urchin/dispatcher.ex +++ b/lib/urchin/dispatcher.ex @@ -223,6 +223,7 @@ defmodule Urchin.Dispatcher do %Result.CallTool{} = result -> {:ok, Result.CallTool.to_map(result)} {:ok, content} when is_list(content) -> {:ok, %{content: content, isError: false}} {:ok, content, opts} when is_list(content) -> {:ok, call_tool_map(content, opts)} + {:error, {:invalid_tool_input, reason}} -> invalid_tool_input(reason, ctx) other -> tool_error_result(other, ctx) end rescue @@ -247,6 +248,15 @@ defmodule Urchin.Dispatcher do defp tool_error_result(other, ctx), do: normalize_error(other, ctx) + # An input-schema validation failure (:validate_arguments) is a tool-execution error per the MCP + # tool error semantics: with tool_errors: :result it is an isError CallToolResult so the model can + # self-correct; with :json_rpc it stays a JSON-RPC invalid_params error (the legacy behavior). + defp invalid_tool_input(reason, %Context{tool_errors: :result}) do + {:ok, %{content: [Urchin.Content.text(reason)], isError: true}} + end + + defp invalid_tool_input(reason, _ctx), do: {:error, Error.invalid_params(reason)} + defp call_tool_map(content, opts) do %{content: content, isError: opts[:is_error] || false} |> maybe_put(:structuredContent, opts[:structured_content]) diff --git a/lib/urchin/server.ex b/lib/urchin/server.ex index daedbd6..8f7d862 100644 --- a/lib/urchin/server.ex +++ b/lib/urchin/server.ex @@ -53,12 +53,12 @@ defmodule Urchin.Server do * `read_resource/2`: `{:ok, contents}` or `{:error, reason}` * `get_prompt/3`: `{:ok, messages}` or `{:ok, messages, description}` - For every callback, an `{:error, %Urchin.Error{}}` becomes that JSON-RPC error. For - `call_tool/3`, an `{:error, binary}` (and a raised exception) is by default surfaced as a - `CallToolResult` with `isError: true` so the model can self-correct; set the transport's - `:tool_errors` to `:json_rpc` for the legacy behavior of returning a JSON-RPC internal error. - For the other callbacks an `{:error, binary}` becomes a JSON-RPC internal error, and raised - exceptions become internal errors. + For every callback, an `{:error, %Urchin.Error{}}` becomes that JSON-RPC error. A `call_tool/3` + handler's `{:error, binary}` is by default surfaced as a `CallToolResult` with `isError: true` + so the model can self-correct; set the transport's `:tool_errors` to `:json_rpc` to return a + JSON-RPC internal error instead. A tool that raises is always reported as an `isError` + `CallToolResult`, regardless of `:tool_errors`. For the other callbacks an `{:error, binary}` + becomes a JSON-RPC internal error and a raised exception becomes an internal error. """ alias Urchin.{Context, Error} @@ -489,7 +489,7 @@ defmodule Urchin.Server do @doc false @spec __validate_tool_args__(String.t(), map(), Context.t(), [Urchin.Tool.t()]) :: - :ok | {:error, Urchin.Error.t()} + :ok | {:error, {:invalid_tool_input, String.t()}} def __validate_tool_args__(_name, _args, %Context{validate_arguments: false}, _tools), do: :ok def __validate_tool_args__(name, args, _ctx, tools) do @@ -500,9 +500,12 @@ defmodule Urchin.Server do if tool.name == name, do: tool.input_schema || %{"type" => "object"} end) + # An input-schema violation is a tool-input error, not a protocol error; the dispatcher shapes + # it per :tool_errors (an isError result by default). Unknown-tool and malformed-request errors + # stay protocol-level JSON-RPC errors. case Urchin.Schema.validate(schema, args) do :ok -> :ok - {:error, reason} -> {:error, Urchin.Error.invalid_params(reason)} + {:error, reason} -> {:error, {:invalid_tool_input, reason}} end end diff --git a/lib/urchin/transport/streamable_http.ex b/lib/urchin/transport/streamable_http.ex index 1d431a3..4439556 100644 --- a/lib/urchin/transport/streamable_http.ex +++ b/lib/urchin/transport/streamable_http.ex @@ -31,8 +31,9 @@ defmodule Urchin.Transport.StreamableHTTP do * `:expose_internal_errors` - return raised-exception messages to the client instead of a generic error (default `false`). Exceptions are always logged; enable only in development. * `:validate_arguments` - validate `tools/call` arguments against each tool's - `input_schema` (DSL tools) before the handler runs, rejecting a mismatch with - `invalid_params` (default `false`). See `Urchin.Schema` for the supported subset. + `input_schema` (DSL tools) before the handler runs (default `false`). A mismatch is a + tool-input error surfaced per `:tool_errors`: an `isError` `CallToolResult` by default, or a + JSON-RPC `invalid_params` error under `:json_rpc`. See `Urchin.Schema` for the subset. * `:enforce_initialized` - reject operation requests received before the client has sent `notifications/initialized` with `invalid_request`; only `ping` is allowed (default `false`). The default may be flipped to `true` in a future minor release. diff --git a/test/urchin/dispatcher_test.exs b/test/urchin/dispatcher_test.exs index 9bb043f..ddaea48 100644 --- a/test/urchin/dispatcher_test.exs +++ b/test/urchin/dispatcher_test.exs @@ -174,9 +174,18 @@ defmodule Urchin.DispatcherTest do end describe "argument validation" do - test "rejects arguments that violate the input schema when enabled" do + test "an input-schema violation is an isError tool result by default (tool_errors: :result)" do ctx = %Context{validate_arguments: true} params = %{"name" => "add", "arguments" => %{"a" => 1}} + assert {:ok, result} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) + assert result.isError == true + assert [%{type: "text", text: text}] = result.content + assert text =~ "b" + end + + test "an input-schema violation is a JSON-RPC invalid_params error under :json_rpc" do + ctx = %Context{validate_arguments: true, tool_errors: :json_rpc} + params = %{"name" => "add", "arguments" => %{"a" => 1}} assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) assert error.code == -32_602 assert error.message =~ "b" @@ -198,10 +207,18 @@ defmodule Urchin.DispatcherTest do Dispatcher.handle_request(EchoServer, "tools/call", params, ctx()) end - test "validates an omitted input_schema as an object" do + test "validates an omitted input_schema as an object (isError result by default)" do # A tool without an input_schema still must receive an object, not a bare value. ctx = %Context{validate_arguments: true} params = %{"name" => "no_schema", "arguments" => "not-an-object"} + + assert {:ok, %{isError: true}} = + Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) + end + + test "an omitted-input_schema violation is invalid_params under :json_rpc" do + ctx = %Context{validate_arguments: true, tool_errors: :json_rpc} + params = %{"name" => "no_schema", "arguments" => "not-an-object"} assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) assert error.code == -32_602 end From 19ff1cc8adcf042169126ff88f886bab9f6a3bf0 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sat, 6 Jun 2026 23:30:51 +0900 Subject: [PATCH 23/37] fix(dispatcher): treat non-object tools/call arguments as a protocol error A non-object CallToolRequestParams.arguments violates the request shape, so it must be a JSON-RPC invalid_params error regardless of :tool_errors, not an isError tool result. Input-schema validation now only runs on object arguments. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 2 +- lib/urchin/dispatcher.ex | 13 ++++++++++++- lib/urchin/server.ex | 7 ++++--- test/urchin/dispatcher_test.exs | 19 +++++++++++++------ 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index d74a1ab..3eece2a 100644 --- a/README.md +++ b/README.md @@ -347,7 +347,7 @@ Passed to `Urchin.Transport.StreamableHTTP`, `Urchin.Endpoint` or `Urchin.start_ | `:request_timeout` | `60_000` | per-request handler timeout (ms) | | `:validate_protocol_version` | `true` | validate the `MCP-Protocol-Version` header | | `:expose_internal_errors` | `false` | return raised-exception messages to the client (dev only); exceptions are always logged | -| `:validate_arguments` | `false` | validate `tools/call` arguments against each tool's `input_schema` before the handler runs; a mismatch is surfaced per `:tool_errors` (an `isError` result by default). See `Urchin.Schema` | +| `:validate_arguments` | `false` | validate `tools/call` arguments against each tool's `input_schema` before the handler runs; a mismatch is surfaced per `:tool_errors` (an `isError` result by default). A non-object `arguments` is a malformed request and is always a JSON-RPC `invalid_params` error, regardless of this option or `:tool_errors`. See `Urchin.Schema` | | `:enforce_initialized` | `false` | reject operation requests received before `notifications/initialized` with `invalid_request`; only `ping` is allowed | | `:tool_errors` | `:result` | how a `tools/call` handler's `{:error, binary}` is surfaced: `:result` returns a `CallToolResult` with `isError: true` so the model can self-correct; `:json_rpc` returns a JSON-RPC internal error. `{:error, %Urchin.Error{}}` is always a JSON-RPC error; other methods are unaffected | | `:sse_buffer_limit` | `nil` | max recent GET-stream (general SSE) events kept per session for resumption replay (`nil` keeps the session default of `100`) | diff --git a/lib/urchin/dispatcher.ex b/lib/urchin/dispatcher.ex index a7186d4..0aca989 100644 --- a/lib/urchin/dispatcher.ex +++ b/lib/urchin/dispatcher.ex @@ -116,7 +116,10 @@ defmodule Urchin.Dispatcher do with_callback(server, :call_tool, 3, fn -> name = require_string(params, "name") args = Map.get(params, "arguments", %{}) - call_tool_result(server, name, args, %{ctx | progress_token: progress_token(params)}) + + with :ok <- require_arguments_object(args) do + call_tool_result(server, name, args, %{ctx | progress_token: progress_token(params)}) + end end) end @@ -390,6 +393,14 @@ defmodule Urchin.Dispatcher do end end + # CallToolRequestParams.arguments is, when present, an object. A non-object value is a malformed + # request (CallToolRequest shape violation), not a tool input-schema error, so it stays a + # protocol-level JSON-RPC error regardless of :tool_errors. + defp require_arguments_object(args) when is_map(args), do: :ok + + defp require_arguments_object(_args), + do: {:error, Error.invalid_params("tools/call arguments must be an object")} + defp require_map(params, key) do case Map.get(params, key) do value when is_map(value) -> value diff --git a/lib/urchin/server.ex b/lib/urchin/server.ex index 8f7d862..8de03af 100644 --- a/lib/urchin/server.ex +++ b/lib/urchin/server.ex @@ -500,9 +500,10 @@ defmodule Urchin.Server do if tool.name == name, do: tool.input_schema || %{"type" => "object"} end) - # An input-schema violation is a tool-input error, not a protocol error; the dispatcher shapes - # it per :tool_errors (an isError result by default). Unknown-tool and malformed-request errors - # stay protocol-level JSON-RPC errors. + # By the time args reaches here it is already an object (the dispatcher rejects a non-object + # CallToolRequestParams.arguments as a protocol error). What remains is input-schema validation + # (missing required field, wrong property type, ...), which is a tool-input error: the dispatcher + # shapes it per :tool_errors (an isError result by default), not a JSON-RPC error. case Urchin.Schema.validate(schema, args) do :ok -> :ok {:error, reason} -> {:error, {:invalid_tool_input, reason}} diff --git a/test/urchin/dispatcher_test.exs b/test/urchin/dispatcher_test.exs index ddaea48..65ca81c 100644 --- a/test/urchin/dispatcher_test.exs +++ b/test/urchin/dispatcher_test.exs @@ -207,21 +207,28 @@ defmodule Urchin.DispatcherTest do Dispatcher.handle_request(EchoServer, "tools/call", params, ctx()) end - test "validates an omitted input_schema as an object (isError result by default)" do - # A tool without an input_schema still must receive an object, not a bare value. + test "non-object arguments are a protocol error, not a tool input error (tool_errors: :result)" do + # CallToolRequestParams.arguments is, when present, an object. A non-object value violates the + # request shape, so it stays a JSON-RPC error even under the default :result mode. ctx = %Context{validate_arguments: true} params = %{"name" => "no_schema", "arguments" => "not-an-object"} - - assert {:ok, %{isError: true}} = - Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) + assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) + assert error.code == -32_602 end - test "an omitted-input_schema violation is invalid_params under :json_rpc" do + test "non-object arguments are invalid_params under :json_rpc" do ctx = %Context{validate_arguments: true, tool_errors: :json_rpc} params = %{"name" => "no_schema", "arguments" => "not-an-object"} assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) assert error.code == -32_602 end + + test "non-object arguments are rejected even without argument validation" do + # The request-shape check is independent of :validate_arguments. + params = %{"name" => "no_schema", "arguments" => ["not", "an", "object"]} + assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx()) + assert error.code == -32_602 + end end describe "resources" do From c112e251187f929736479f34cecb0f455cc07242 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sun, 7 Jun 2026 01:09:17 +0900 Subject: [PATCH 24/37] feat!: enforce MCP spec compliance by default Make the previously opt-in spec-compliance behaviors the default, with no opt-out, and validate the requests the spec requires: - Validate tools/call arguments against each tool's input schema; a mismatch is an isError CallToolResult. Parameterless tools default to a closed object schema (additionalProperties: false). - Reject operation requests received before notifications/initialized; only ping and logging/setLevel are allowed before initialization. - Surface a tools/call handler's {:error, binary} as an isError CallToolResult; protocol errors stay JSON-RPC errors. - Validate literal tool names against the spec pattern at compile time. - Require protocolVersion, capabilities and clientInfo on initialize, and a serverInfo carrying a string name and version. - Validate the MCP-Protocol-Version header on DELETE, matching POST and GET. - Cap completion/complete results at 100 values, setting hasMore when truncated. Removes the :tool_errors, :validate_arguments and :enforce_initialized transport options and the validate_tool_names server option; the behaviors they gated are now always on. These options were never part of a release. BREAKING CHANGE: spec-compliance behaviors are always on and several previously lenient defaults now reject non-conforming input. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/urchin/context.ex | 6 - lib/urchin/dispatcher.ex | 144 +++++++++---- lib/urchin/server.ex | 42 ++-- lib/urchin/tool.ex | 12 +- lib/urchin/transport/streamable_http.ex | 54 ++--- test/urchin/core_test.exs | 2 +- test/urchin/dispatcher_test.exs | 190 +++++++++++------- test/urchin/integration_test.exs | 7 + test/urchin/server_test.exs | 24 +-- .../transport/streamable_http_auth_test.exs | 16 +- .../urchin/transport/streamable_http_test.exs | 118 ++++------- 11 files changed, 326 insertions(+), 289 deletions(-) diff --git a/lib/urchin/context.ex b/lib/urchin/context.ex index 6c50fe6..d10c051 100644 --- a/lib/urchin/context.ex +++ b/lib/urchin/context.ex @@ -34,10 +34,7 @@ defmodule Urchin.Context do assigns: %{}, min_log_level: "debug", expose_internal_errors: false, - validate_arguments: false, initialized: false, - enforce_initialized: false, - tool_errors: :result, cancelled_ref: nil ] @@ -56,10 +53,7 @@ defmodule Urchin.Context do assigns: map(), min_log_level: String.t(), expose_internal_errors: boolean(), - validate_arguments: boolean(), initialized: boolean(), - enforce_initialized: boolean(), - tool_errors: :json_rpc | :result, cancelled_ref: reference() | nil } diff --git a/lib/urchin/dispatcher.ex b/lib/urchin/dispatcher.ex index 0aca989..01cc328 100644 --- a/lib/urchin/dispatcher.ex +++ b/lib/urchin/dispatcher.ex @@ -22,24 +22,25 @@ defmodule Urchin.Dispatcher do @spec initialize(module(), map(), Context.t()) :: {:ok, map(), map()} | {:error, Error.t()} def initialize(server, params, ctx) when is_map(params) do - requested = Map.get(params, "protocolVersion", Protocol.latest_version()) - negotiated = Protocol.negotiate(requested) - - result = - %{ - protocolVersion: negotiated, - capabilities: capabilities(server), - serverInfo: server.server_info() + with :ok <- validate_initialize_params(params) do + negotiated = Protocol.negotiate(Map.fetch!(params, "protocolVersion")) + + result = + %{ + protocolVersion: negotiated, + capabilities: capabilities(server), + serverInfo: validated_server_info(server) + } + |> maybe_put(:instructions, instructions(server)) + + meta = %{ + protocol_version: negotiated, + client_info: Map.fetch!(params, "clientInfo"), + client_capabilities: Map.fetch!(params, "capabilities") } - |> maybe_put(:instructions, instructions(server)) - meta = %{ - protocol_version: negotiated, - client_info: Map.get(params, "clientInfo"), - client_capabilities: Map.get(params, "capabilities", %{}) - } - - {:ok, result, meta} + {:ok, result, meta} + end rescue error in Urchin.Error -> {:error, error} @@ -52,6 +53,53 @@ defmodule Urchin.Dispatcher do {:error, Error.invalid_params("initialize params must be an object")} end + # The client MUST send protocolVersion (string), capabilities (object) and clientInfo (with a + # string name and version) in the initialize request; a missing or mistyped field is a protocol + # error rather than a silently-defaulted value. + defp validate_initialize_params(params) do + with :ok <- ensure_string(params, "protocolVersion"), + :ok <- ensure_object(params, "capabilities"), + :ok <- ensure_object(params, "clientInfo") do + validate_client_info(Map.fetch!(params, "clientInfo")) + end + end + + defp validate_client_info(info) do + if is_binary(info[:name] || info["name"]) and is_binary(info[:version] || info["version"]) do + :ok + else + {:error, Error.invalid_params("initialize: clientInfo requires a string name and version")} + end + end + + defp ensure_string(params, key) do + case Map.get(params, key) do + value when is_binary(value) -> :ok + _ -> {:error, Error.invalid_params(~s(initialize: missing or invalid string "#{key}"))} + end + end + + defp ensure_object(params, key) do + case Map.get(params, key) do + value when is_map(value) -> :ok + _ -> {:error, Error.invalid_params(~s(initialize: missing or invalid object "#{key}"))} + end + end + + # serverInfo MUST carry a string name and version. The DSL enforces this at the + # __server_info__ boundary, but a hand-written server_info/0 could omit them and produce a + # malformed InitializeResult; surface that as an internal error instead of shipping it. + defp validated_server_info(server) do + info = server.server_info() + + if is_map(info) and is_binary(info[:name] || info["name"]) and + is_binary(info[:version] || info["version"]) do + info + else + raise Error.internal_error("serverInfo must include a string name and version") + end + end + @doc """ Handles an operational (post-initialization) request, returning `{:ok, result_map}` or `{:error, Urchin.Error.t()}`. @@ -64,8 +112,8 @@ defmodule Urchin.Dispatcher do end def handle_request(server, method, params, ctx) do - # Lifecycle gate: when enforce_initialized is on and notifications/initialized has not been - # received, reject operation requests other than ping with invalid_request. + # Lifecycle gate: until notifications/initialized has been received, reject operation requests + # other than ping and logging/setLevel with invalid_request. with :ok <- check_initialized(method, ctx) do do_handle(server, method, params, ctx) end @@ -81,10 +129,9 @@ defmodule Urchin.Dispatcher do {:error, Error.internal_error(generic_or(ctx, "Handler threw: " <> inspect(value)))} end - # The gate is a no-op unless :enforce_initialized is set and the session is not yet - # initialized. `notifications/initialized` is a notification routed straight into the - # session, so it never reaches this request-only path. - defp check_initialized(_method, %Context{enforce_initialized: false}), do: :ok + # Until the client sends notifications/initialized the session is not initialized; only the + # pre-init methods are honoured. `notifications/initialized` is a notification routed straight + # into the session, so it never reaches this request-only path. defp check_initialized(_method, %Context{initialized: true}), do: :ok defp check_initialized(method, %Context{}) do @@ -98,9 +145,11 @@ defmodule Urchin.Dispatcher do end end - # Only ping is allowed before the client sends notifications/initialized; per the MCP - # lifecycle a client should not send other requests until initialization completes. + # Only ping and logging/setLevel are allowed before the client sends + # notifications/initialized; per the MCP lifecycle a client should not send other requests + # until initialization completes. defp pre_init_allowed?("ping"), do: true + defp pre_init_allowed?("logging/setLevel"), do: true defp pre_init_allowed?(_method), do: false # ping is always available regardless of declared capabilities. @@ -241,25 +290,21 @@ defmodule Urchin.Dispatcher do {:ok, %{content: [Urchin.Content.text(text)], isError: true}} end - # With tool_errors: :result, a handler's {:error, binary} becomes an isError tool result - # (so the model can self-correct) instead of a JSON-RPC error. A protocol-level - # {:error, %Error{}} and every other shape still go through normalize_error. - defp tool_error_result({:error, message}, %Context{tool_errors: :result}) - when is_binary(message) do + # A handler's {:error, binary} is a tool-execution error: it becomes an isError CallToolResult + # (so the model can self-correct) per the MCP tool error semantics, not a JSON-RPC error. A + # protocol-level {:error, %Error{}} and every other shape still go through normalize_error. + defp tool_error_result({:error, message}, _ctx) when is_binary(message) do {:ok, %{content: [Urchin.Content.text(message)], isError: true}} end defp tool_error_result(other, ctx), do: normalize_error(other, ctx) - # An input-schema validation failure (:validate_arguments) is a tool-execution error per the MCP - # tool error semantics: with tool_errors: :result it is an isError CallToolResult so the model can - # self-correct; with :json_rpc it stays a JSON-RPC invalid_params error (the legacy behavior). - defp invalid_tool_input(reason, %Context{tool_errors: :result}) do + # An input-schema validation failure is likewise a tool-execution error: an isError + # CallToolResult so the model can self-correct, not a JSON-RPC protocol error. + defp invalid_tool_input(reason, _ctx) do {:ok, %{content: [Urchin.Content.text(reason)], isError: true}} end - defp invalid_tool_input(reason, _ctx), do: {:error, Error.invalid_params(reason)} - defp call_tool_map(content, opts) do %{content: content, isError: opts[:is_error] || false} |> maybe_put(:structuredContent, opts[:structured_content]) @@ -277,12 +322,31 @@ defmodule Urchin.Dispatcher do defp empty_result({:ok, _}, _ctx), do: {:ok, %{}} defp empty_result(other, ctx), do: normalize_error(other, ctx) - defp completion(values) when is_list(values), do: %{values: values} + @max_completion_values 100 + + defp completion(values) when is_list(values), do: build_completion(values, nil, nil) defp completion(%{} = completion) do - %{values: get_either(completion, :values, "values", [])} - |> maybe_put(:total, get_either(completion, :total, "total", nil)) - |> maybe_put(:hasMore, get_either(completion, :has_more, "hasMore", nil)) + build_completion( + get_either(completion, :values, "values", []), + get_either(completion, :total, "total", nil), + get_either(completion, :has_more, "hasMore", nil) + ) + end + + # The completion spec caps values at 100 per response; when a handler returns more, the list is + # truncated to the top 100 (already ranked by relevance) and hasMore is necessarily true. + defp build_completion(values, total, has_more) when is_list(values) do + {capped, has_more} = + if length(values) > @max_completion_values do + {Enum.take(values, @max_completion_values), true} + else + {values, has_more} + end + + %{values: capped} + |> maybe_put(:total, total) + |> maybe_put(:hasMore, has_more) end # Reads a value that may be keyed by atom or string, preserving false/0 values. @@ -395,7 +459,7 @@ defmodule Urchin.Dispatcher do # CallToolRequestParams.arguments is, when present, an object. A non-object value is a malformed # request (CallToolRequest shape violation), not a tool input-schema error, so it stays a - # protocol-level JSON-RPC error regardless of :tool_errors. + # protocol-level JSON-RPC error rather than an isError tool result. defp require_arguments_object(args) when is_map(args), do: :ok defp require_arguments_object(_args), diff --git a/lib/urchin/server.ex b/lib/urchin/server.ex index 8de03af..4d3a58f 100644 --- a/lib/urchin/server.ex +++ b/lib/urchin/server.ex @@ -34,10 +34,8 @@ defmodule Urchin.Server do Capabilities are derived automatically from the declared features. - Duplicate tool names declared via the DSL are rejected at compile time. Pass - `validate_tool_names: true` to `use Urchin.Server` to additionally enforce that every literal - tool name matches `~r/\A[a-zA-Z0-9_.-]{1,128}\z/` (default `false`); a non-matching name raises - `ArgumentError`. + Duplicate tool names declared via the DSL are rejected at compile time, and every literal tool + name must match `~r/\A[a-zA-Z0-9_.-]{1,128}\z/`; a non-matching name raises `ArgumentError`. ## Behaviour @@ -54,18 +52,16 @@ defmodule Urchin.Server do * `get_prompt/3`: `{:ok, messages}` or `{:ok, messages, description}` For every callback, an `{:error, %Urchin.Error{}}` becomes that JSON-RPC error. A `call_tool/3` - handler's `{:error, binary}` is by default surfaced as a `CallToolResult` with `isError: true` - so the model can self-correct; set the transport's `:tool_errors` to `:json_rpc` to return a - JSON-RPC internal error instead. A tool that raises is always reported as an `isError` - `CallToolResult`, regardless of `:tool_errors`. For the other callbacks an `{:error, binary}` - becomes a JSON-RPC internal error and a raised exception becomes an internal error. + handler's `{:error, binary}` is surfaced as a `CallToolResult` with `isError: true` so the model + can self-correct, as is a tool that raises. For the other callbacks an `{:error, binary}` becomes + a JSON-RPC internal error and a raised exception becomes an internal error. """ alias Urchin.{Context, Error} # Constrained tool-name charset. The MCP schema imposes no pattern, but this is the # de-facto convention shared by common tool-calling SDKs; dots and dashes are permitted - # for namespacing. Enforced only when the server opts in via :validate_tool_names. + # for namespacing. Enforced for every literal tool name at compile time. @tool_name_pattern ~r/\A[a-zA-Z0-9_.-]{1,128}\z/ @type cursor :: String.t() | nil @@ -261,21 +257,17 @@ defmodule Urchin.Server do raise ArgumentError, "tool :scopes must be a list of strings, got: #{inspect(other)}" end - # Duplicate tool names within a server are always rejected at compile time (a silently - # shadowed duplicate is a bug). The name-pattern check is opt-in via :validate_tool_names. - # Non-literal names (a variable or call) cannot be compared statically and are skipped, - # mirroring handler_name/2. - defp validate_tool_names!(tool_dispatch, opts) do + # Duplicate tool names within a server are rejected at compile time (a silently shadowed + # duplicate is a bug), and every literal name must match @tool_name_pattern. Non-literal names + # (a variable or call) cannot be compared statically and are skipped, mirroring handler_name/2. + defp validate_tool_names!(tool_dispatch) do names = tool_dispatch |> Enum.map(fn {name, _fname, _scopes} -> name end) |> Enum.filter(&is_binary/1) validate_unique_tool_names!(names) - - if Keyword.get(opts, :validate_tool_names, false) do - Enum.each(names, &validate_tool_name_pattern!/1) - end + Enum.each(names, &validate_tool_name_pattern!/1) :ok end @@ -320,7 +312,7 @@ defmodule Urchin.Server do opts = Module.get_attribute(mod, :mcp_opts) || [] tool_dispatch = Module.get_attribute(mod, :mcp_tool_dispatch) || [] - validate_tool_names!(tool_dispatch, opts) + validate_tool_names!(tool_dispatch) has_tools? = tool_dispatch != [] has_resources? = Module.get_attribute(mod, :mcp_resources) != [] @@ -490,20 +482,18 @@ defmodule Urchin.Server do @doc false @spec __validate_tool_args__(String.t(), map(), Context.t(), [Urchin.Tool.t()]) :: :ok | {:error, {:invalid_tool_input, String.t()}} - def __validate_tool_args__(_name, _args, %Context{validate_arguments: false}, _tools), do: :ok - def __validate_tool_args__(name, args, _ctx, tools) do - # Use the same effective schema the wire advertises: an omitted input_schema means an - # object, so validation is not silently skipped for a tool that declared no schema. + # Use the same effective schema the wire advertises (Tool.default_input_schema/0 when the tool + # declared none), so validation is not silently skipped for a tool that declared no schema. schema = Enum.find_value(tools, fn tool -> - if tool.name == name, do: tool.input_schema || %{"type" => "object"} + if tool.name == name, do: tool.input_schema || Urchin.Tool.default_input_schema() end) # By the time args reaches here it is already an object (the dispatcher rejects a non-object # CallToolRequestParams.arguments as a protocol error). What remains is input-schema validation # (missing required field, wrong property type, ...), which is a tool-input error: the dispatcher - # shapes it per :tool_errors (an isError result by default), not a JSON-RPC error. + # shapes it as an isError CallToolResult, not a JSON-RPC error. case Urchin.Schema.validate(schema, args) do :ok -> :ok {:error, reason} -> {:error, {:invalid_tool_input, reason}} diff --git a/lib/urchin/tool.ex b/lib/urchin/tool.ex index eb332ee..430868a 100644 --- a/lib/urchin/tool.ex +++ b/lib/urchin/tool.ex @@ -3,7 +3,8 @@ defmodule Urchin.Tool do A tool definition advertised via `tools/list`. Mirrors the `Tool` type from the MCP schema. `input_schema` is a JSON Schema object - describing the tool arguments; when omitted it defaults to an empty object schema. + describing the tool arguments; when omitted it defaults to `default_input_schema/0`, an + object that accepts no properties. """ alias Urchin.WireFormat @@ -54,10 +55,17 @@ defmodule Urchin.Tool do defp fetch_name!(%{name: name}) when is_binary(name), do: name defp fetch_name!(_), do: raise(ArgumentError, "tool requires a string :name") + @doc """ + The input schema advertised for a tool that declares none: an object accepting no + properties, per the MCP recommendation for parameterless tools. + """ + @spec default_input_schema() :: map() + def default_input_schema, do: %{"type" => "object", "additionalProperties" => false} + @doc "Serializes the tool to its JSON-RPC wire shape." @spec to_map(t()) :: map() def to_map(%__MODULE__{} = tool) do - %{name: tool.name, inputSchema: tool.input_schema || %{"type" => "object"}} + %{name: tool.name, inputSchema: tool.input_schema || default_input_schema()} |> WireFormat.maybe_put(:title, tool.title) |> WireFormat.maybe_put(:description, tool.description) |> WireFormat.maybe_put(:outputSchema, tool.output_schema) diff --git a/lib/urchin/transport/streamable_http.ex b/lib/urchin/transport/streamable_http.ex index 4439556..5a07b69 100644 --- a/lib/urchin/transport/streamable_http.ex +++ b/lib/urchin/transport/streamable_http.ex @@ -30,18 +30,6 @@ defmodule Urchin.Transport.StreamableHTTP do tool run, since it can expire a session mid-request. * `:expose_internal_errors` - return raised-exception messages to the client instead of a generic error (default `false`). Exceptions are always logged; enable only in development. - * `:validate_arguments` - validate `tools/call` arguments against each tool's - `input_schema` (DSL tools) before the handler runs (default `false`). A mismatch is a - tool-input error surfaced per `:tool_errors`: an `isError` `CallToolResult` by default, or a - JSON-RPC `invalid_params` error under `:json_rpc`. See `Urchin.Schema` for the subset. - * `:enforce_initialized` - reject operation requests received before the client has sent - `notifications/initialized` with `invalid_request`; only `ping` is allowed (default - `false`). The default may be flipped to `true` in a future minor release. - * `:tool_errors` - how a `tools/call` handler's `{:error, binary}` is surfaced: - `:result` (default) returns it as a `CallToolResult` with `isError: true` so the model can - self-correct (the spec-compliant behavior); `:json_rpc` returns it as a JSON-RPC internal - error. A protocol error returned as `{:error, %Urchin.Error{}}` is always a JSON-RPC error. - Other methods are unaffected. * `:sse_buffer_limit` - the maximum number of recent general-stream (GET SSE) events each session keeps for resumption replay. Defaults to `nil`, which preserves the session's internal default of `100`. A positive integer or `nil`. @@ -49,6 +37,11 @@ defmodule Urchin.Transport.StreamableHTTP do every request; `nil` (default) serves MCP unauthenticated. The metadata discovery endpoint is served by `Urchin.Endpoint`/`Urchin.Auth.Metadata`, not this plug. + The transport enforces the spec by default and these behaviors are not configurable: it + validates `tools/call` arguments against each tool's input schema, rejects operation requests + received before `notifications/initialized` (only `ping` and `logging/setLevel` are allowed + pre-init), and surfaces a tool handler's `{:error, binary}` as an `isError` `CallToolResult`. + The plug reads the raw request body itself, so mount it before any JSON body parser. """ @@ -80,9 +73,6 @@ defmodule Urchin.Transport.StreamableHTTP do request_timeout: Keyword.get(opts, :request_timeout, 60_000), validate_protocol_version: Keyword.get(opts, :validate_protocol_version, true), expose_internal_errors: Keyword.get(opts, :expose_internal_errors, false), - validate_arguments: Keyword.get(opts, :validate_arguments, false), - enforce_initialized: Keyword.get(opts, :enforce_initialized, false), - tool_errors: tool_errors_opt!(opts), max_sessions: positive_integer_opt!(opts, :max_sessions), session_idle_timeout: positive_integer_opt!(opts, :session_idle_timeout), session_max_lifetime: positive_integer_opt!(opts, :session_max_lifetime), @@ -277,10 +267,7 @@ defmodule Urchin.Transport.StreamableHTTP do auth: conn_auth(conn), min_log_level: snapshot.min_log_level, expose_internal_errors: config.expose_internal_errors, - validate_arguments: config.validate_arguments, - initialized: snapshot.initialized, - enforce_initialized: config.enforce_initialized, - tool_errors: config.tool_errors + initialized: snapshot.initialized } {task_pid, task_ref} = @@ -455,13 +442,14 @@ defmodule Urchin.Transport.StreamableHTTP do end defp handle_delete(conn, config) do - case lookup_session(conn, config) do - {:ok, session_pid} -> - Session.terminate(session_pid) - send_resp(conn, 204, "") - - {:error, status, error} -> - send_error(conn, status, nil, error) + # DELETE is a post-initialize request, so it carries the MCP-Protocol-Version header like POST + # and GET; validate it before terminating the session. + with {:ok, session_pid} <- lookup_session(conn, config), + :ok <- check_protocol_version(conn, config) do + Session.terminate(session_pid) + send_resp(conn, 204, "") + else + {:error, status, error} -> send_error(conn, status, nil, error) end end @@ -565,20 +553,6 @@ defmodule Urchin.Transport.StreamableHTTP do end end - # :tool_errors selects how a tool handler's {:error, binary} surfaces. Default :result returns - # it as an isError CallToolResult (spec-compliant, lets the model self-correct); :json_rpc is the - # opt-in legacy mode that returns a JSON-RPC internal error instead. Fail fast on a bad value at - # startup, matching how the session-limit options are validated. - defp tool_errors_opt!(opts) do - case Keyword.get(opts, :tool_errors, :result) do - value when value in [:json_rpc, :result] -> - value - - other -> - raise ArgumentError, ":tool_errors must be :json_rpc or :result, got: #{inspect(other)}" - end - end - # When nil, omit the key so the Session keeps its own default (@default_buffer_limit). # Passing buffer_limit: nil would defeat Keyword.get's default and crash push_general. defp maybe_put_buffer_limit(opts, nil), do: opts diff --git a/test/urchin/core_test.exs b/test/urchin/core_test.exs index 99140db..8e3d69a 100644 --- a/test/urchin/core_test.exs +++ b/test/urchin/core_test.exs @@ -119,7 +119,7 @@ defmodule Urchin.CoreTest do assert decoded == %{ "name" => "t", "description" => "d", - "inputSchema" => %{"type" => "object"} + "inputSchema" => %{"type" => "object", "additionalProperties" => false} } end end diff --git a/test/urchin/dispatcher_test.exs b/test/urchin/dispatcher_test.exs index 65ca81c..505cef3 100644 --- a/test/urchin/dispatcher_test.exs +++ b/test/urchin/dispatcher_test.exs @@ -29,14 +29,40 @@ defmodule Urchin.DispatcherTest.FailingLoggingServer do def set_log_level(_level, _ctx), do: {:error, "nope"} end +defmodule Urchin.DispatcherTest.BadInfoServer do + @moduledoc false + # A hand-written server whose server_info/0 omits the required version field. + @behaviour Urchin.Server + + @impl true + def server_info, do: %{name: "bad"} + + @impl true + def capabilities, do: %{} +end + +defmodule Urchin.DispatcherTest.BigCompletionServer do + @moduledoc false + # Returns more than the 100-value completion cap so truncation can be exercised. + use Urchin.Server, name: "big-completion", version: "1.0.0", completions: true + + @impl true + def complete(_ref, _argument, _context, _ctx) do + {:ok, %{values: Enum.map(1..150, &"v#{&1}")}} + end +end + defmodule Urchin.DispatcherTest do use ExUnit.Case, async: true alias Urchin.{Context, Dispatcher, Session} alias Urchin.Test.EchoServer alias Urchin.DispatcherTest.{LoggingServer, NoLoggingServer, FailingLoggingServer} + alias Urchin.DispatcherTest.{BadInfoServer, BigCompletionServer} - defp ctx, do: %Context{} + # The default context represents an initialized session; the lifecycle gate is exercised + # explicitly in the "initialized gating" describe with initialized: false. + defp ctx, do: %Context{initialized: true} describe "initialize/3" do test "negotiates a supported version and reports capabilities" do @@ -58,10 +84,48 @@ defmodule Urchin.DispatcherTest do end test "falls back to latest for an unsupported version" do - params = %{"protocolVersion" => "1999-01-01", "capabilities" => %{}} + params = %{ + "protocolVersion" => "1999-01-01", + "capabilities" => %{}, + "clientInfo" => %{"name" => "c", "version" => "1"} + } + assert {:ok, result, _meta} = Dispatcher.initialize(EchoServer, params, ctx()) assert result.protocolVersion == Urchin.protocol_version() end + + test "rejects a missing protocolVersion" do + params = %{"capabilities" => %{}, "clientInfo" => %{"name" => "c", "version" => "1"}} + assert {:error, error} = Dispatcher.initialize(EchoServer, params, ctx()) + assert error.code == -32_602 + end + + test "rejects a missing capabilities object" do + params = %{ + "protocolVersion" => "2025-11-25", + "clientInfo" => %{"name" => "c", "version" => "1"} + } + + assert {:error, error} = Dispatcher.initialize(EchoServer, params, ctx()) + assert error.code == -32_602 + end + + test "rejects clientInfo without a string name and version" do + params = %{"protocolVersion" => "2025-11-25", "capabilities" => %{}, "clientInfo" => %{}} + assert {:error, error} = Dispatcher.initialize(EchoServer, params, ctx()) + assert error.code == -32_602 + end + + test "rejects a serverInfo missing name or version" do + params = %{ + "protocolVersion" => "2025-11-25", + "capabilities" => %{}, + "clientInfo" => %{"name" => "c", "version" => "1"} + } + + assert {:error, error} = Dispatcher.initialize(BadInfoServer, params, ctx()) + assert error.code == -32_603 + end end describe "tools" do @@ -103,7 +167,7 @@ defmodule Urchin.DispatcherTest do test "a raising tool exposes its message when expose_internal_errors is set" do params = %{"name" => "boom", "arguments" => %{}} - ctx = %Context{expose_internal_errors: true} + ctx = %Context{expose_internal_errors: true, initialized: true} assert {:ok, result} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) assert result.content == [%{type: "text", text: "kaboom"}] end @@ -123,7 +187,7 @@ defmodule Urchin.DispatcherTest do test "a non-binary handler error reason is exposed when configured" do params = %{"name" => "leaky", "arguments" => %{}} - ctx = %Context{expose_internal_errors: true} + ctx = %Context{expose_internal_errors: true, initialized: true} assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) assert error.message =~ "postgres" end @@ -142,7 +206,7 @@ defmodule Urchin.DispatcherTest do end test "runs the handler when the required scope is granted" do - ctx = %Context{auth: %Claims{scopes: ["secret:read"]}} + ctx = %Context{auth: %Claims{scopes: ["secret:read"]}, initialized: true} assert {:ok, %{content: [%{type: "text", text: "classified"}], isError: false}} = call_secret(ctx) @@ -152,79 +216,63 @@ defmodule Urchin.DispatcherTest do end test "denies when the granted scopes are insufficient" do - assert {:error, error} = call_secret(%Context{auth: %Claims{scopes: ["other"]}}) + assert {:error, error} = + call_secret(%Context{auth: %Claims{scopes: ["other"]}, initialized: true}) + assert error.message =~ "scope" end test "denies (fail closed) when the request carries no authorization" do - assert {:error, error} = call_secret(%Context{auth: nil}) + assert {:error, error} = call_secret(%Context{auth: nil, initialized: true}) assert error.message =~ "scope" end test "a denied call never executes the handler" do - assert {:error, _} = call_secret(%Context{auth: %Claims{scopes: ["other"]}}) + assert {:error, _} = + call_secret(%Context{auth: %Claims{scopes: ["other"]}, initialized: true}) + refute_received :secret_executed end test "scope denial has a stable error code and the required scopes in data" do - assert {:error, error} = call_secret(%Context{auth: %Claims{scopes: []}}) + assert {:error, error} = call_secret(%Context{auth: %Claims{scopes: []}, initialized: true}) assert error.code == -32_600 assert error.data == %{required_scopes: ["secret:read"]} end end describe "argument validation" do - test "an input-schema violation is an isError tool result by default (tool_errors: :result)" do - ctx = %Context{validate_arguments: true} + test "an input-schema violation is an isError tool result" do params = %{"name" => "add", "arguments" => %{"a" => 1}} - assert {:ok, result} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) + assert {:ok, result} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx()) assert result.isError == true assert [%{type: "text", text: text}] = result.content assert text =~ "b" end - test "an input-schema violation is a JSON-RPC invalid_params error under :json_rpc" do - ctx = %Context{validate_arguments: true, tool_errors: :json_rpc} - params = %{"name" => "add", "arguments" => %{"a" => 1}} - assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) - assert error.code == -32_602 - assert error.message =~ "b" - end - - test "accepts valid arguments when enabled" do - ctx = %Context{validate_arguments: true} + test "accepts valid arguments" do params = %{"name" => "add", "arguments" => %{"a" => 1, "b" => 2}} assert {:ok, %{structuredContent: %{"sum" => 3}}} = - Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) - end - - test "does not validate when disabled (the default)" do - # Without validation the bad arguments reach the handler, which fails at runtime. - params = %{"name" => "add", "arguments" => %{"a" => 1}} - - assert {:ok, %{isError: true}} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx()) end - test "non-object arguments are a protocol error, not a tool input error (tool_errors: :result)" do - # CallToolRequestParams.arguments is, when present, an object. A non-object value violates the - # request shape, so it stays a JSON-RPC error even under the default :result mode. - ctx = %Context{validate_arguments: true} - params = %{"name" => "no_schema", "arguments" => "not-an-object"} - assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) - assert error.code == -32_602 + test "a tool that declares no schema rejects unexpected properties" do + # An omitted input_schema defaults to an object accepting no properties. + params = %{"name" => "no_schema", "arguments" => %{"extra" => 1}} + assert {:ok, result} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx()) + assert result.isError == true end - test "non-object arguments are invalid_params under :json_rpc" do - ctx = %Context{validate_arguments: true, tool_errors: :json_rpc} + test "non-object arguments are a protocol error, not a tool input error" do + # CallToolRequestParams.arguments is, when present, an object. A non-object value violates the + # request shape, so it stays a JSON-RPC error rather than an isError tool result. params = %{"name" => "no_schema", "arguments" => "not-an-object"} - assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) + assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx()) assert error.code == -32_602 end - test "non-object arguments are rejected even without argument validation" do - # The request-shape check is independent of :validate_arguments. + test "array arguments are rejected as a protocol error" do params = %{"name" => "no_schema", "arguments" => ["not", "an", "object"]} assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx()) assert error.code == -32_602 @@ -305,6 +353,24 @@ defmodule Urchin.DispatcherTest do assert completion.hasMore == false end + test "caps completion values at 100 and forces hasMore when truncated" do + params = %{ + "ref" => %{"type" => "ref/prompt", "name" => "x"}, + "argument" => %{"name" => "n", "value" => "v"} + } + + assert {:ok, %{completion: completion}} = + Dispatcher.handle_request( + BigCompletionServer, + "completion/complete", + params, + ctx() + ) + + assert length(completion.values) == 100 + assert completion.hasMore == true + end + test "ping" do assert {:ok, %{}} = Dispatcher.handle_request(EchoServer, "ping", %{}, ctx()) end @@ -415,61 +481,45 @@ defmodule Urchin.DispatcherTest do end describe "initialized gating" do - test "does not gate by default even when not initialized" do - assert {:ok, %{tools: _}} = - Dispatcher.handle_request(EchoServer, "tools/list", %{}, %Context{}) - end - - test "rejects operation requests before initialized when enforced" do - ctx = %Context{enforce_initialized: true, initialized: false} + test "rejects operation requests before initialized" do + ctx = %Context{initialized: false} assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/list", %{}, ctx) assert error.code == -32_600 end - test "allows ping before initialized when enforced" do - ctx = %Context{enforce_initialized: true, initialized: false} + test "allows ping before initialized" do + ctx = %Context{initialized: false} assert {:ok, %{}} = Dispatcher.handle_request(EchoServer, "ping", %{}, ctx) end - test "rejects logging/setLevel before initialized when enforced (only ping is allowed)" do - ctx = %Context{enforce_initialized: true, initialized: false} + test "allows logging/setLevel before initialized" do + ctx = %Context{initialized: false} - assert {:error, error} = + assert {:ok, %{}} = Dispatcher.handle_request( EchoServer, "logging/setLevel", %{"level" => "info"}, ctx ) - - assert error.code == -32_600 end test "allows operation requests once initialized" do - ctx = %Context{enforce_initialized: true, initialized: true} + ctx = %Context{initialized: true} assert {:ok, %{tools: _}} = Dispatcher.handle_request(EchoServer, "tools/list", %{}, ctx) end end - describe "tool_errors option" do - test "the default (:result) returns a binary tool error as an isError result" do + describe "tool errors" do + test "a binary tool error becomes an isError result" do params = %{"name" => "failing", "arguments" => %{}} assert {:ok, result} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx()) assert result == %{content: [%{type: "text", text: "tool said no"}], isError: true} end - test ":json_rpc keeps a binary tool error as a JSON-RPC error" do - ctx = %Context{tool_errors: :json_rpc} - params = %{"name" => "failing", "arguments" => %{}} - assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) - assert error.code == -32_603 - assert error.message == "tool said no" - end - - test ":result still passes a protocol error through as a JSON-RPC error" do - ctx = %Context{tool_errors: :result} + test "a protocol error is still surfaced as a JSON-RPC error" do params = %{"name" => "protocol_error", "arguments" => %{}} - assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx) + assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx()) assert error.code == -32_602 end end diff --git a/test/urchin/integration_test.exs b/test/urchin/integration_test.exs index 108e390..7aa2e2e 100644 --- a/test/urchin/integration_test.exs +++ b/test/urchin/integration_test.exs @@ -45,6 +45,13 @@ defmodule Urchin.IntegrationTest do {200, headers, resp} = SSEClient.request(port, "POST", "/mcp", json_headers(), body) session_id = Enum.find_value(headers, fn {k, v} -> if k == "mcp-session-id", do: v end) + + # Complete the handshake so subsequent operation requests pass the lifecycle gate. + initialized = Jason.encode!(%{jsonrpc: "2.0", method: "notifications/initialized"}) + + {202, _h, _b} = + SSEClient.request(port, "POST", "/mcp", session_headers(session_id), initialized) + {session_id, Jason.decode!(resp)} end diff --git a/test/urchin/server_test.exs b/test/urchin/server_test.exs index 9041203..0796027 100644 --- a/test/urchin/server_test.exs +++ b/test/urchin/server_test.exs @@ -34,10 +34,10 @@ defmodule Urchin.ServerTest do end describe "tool name validation" do - test "rejects an invalid tool name at compile time when opted in" do + test "rejects an invalid tool name at compile time" do source = """ defmodule Urchin.ServerTest.BadName do - use Urchin.Server, name: "bad", version: "1.0.0", validate_tool_names: true + use Urchin.Server, name: "bad", version: "1.0.0" tool "bad name!" do {:ok, [Urchin.Content.text("ok")]} @@ -53,7 +53,7 @@ defmodule Urchin.ServerTest do test "rejects a tool name with a trailing newline at compile time" do source = """ defmodule Urchin.ServerTest.NewlineName do - use Urchin.Server, name: "nl", version: "1.0.0", validate_tool_names: true + use Urchin.Server, name: "nl", version: "1.0.0" tool "abc\\n" do {:ok, [Urchin.Content.text("ok")]} @@ -86,10 +86,10 @@ defmodule Urchin.ServerTest do end end - test "accepts valid, unique names when opted in" do + test "accepts valid, unique names" do source = """ defmodule Urchin.ServerTest.GoodNames do - use Urchin.Server, name: "good-names", version: "1.0.0", validate_tool_names: true + use Urchin.Server, name: "good-names", version: "1.0.0" tool "echo" do {:ok, [Urchin.Content.text("a")]} @@ -103,19 +103,5 @@ defmodule Urchin.ServerTest do assert [{Urchin.ServerTest.GoodNames, _} | _] = Code.compile_string(source) end - - test "does not enforce the name pattern by default" do - source = """ - defmodule Urchin.ServerTest.UncheckedName do - use Urchin.Server, name: "unchecked", version: "1.0.0" - - tool "bad name!" do - {:ok, [Urchin.Content.text("a")]} - end - end - """ - - assert [{Urchin.ServerTest.UncheckedName, _} | _] = Code.compile_string(source) - end end end diff --git a/test/urchin/transport/streamable_http_auth_test.exs b/test/urchin/transport/streamable_http_auth_test.exs index 8ddcd1e..4ab4591 100644 --- a/test/urchin/transport/streamable_http_auth_test.exs +++ b/test/urchin/transport/streamable_http_auth_test.exs @@ -69,14 +69,20 @@ defmodule Urchin.Transport.StreamableHTTPAuthTest do conn = initialize([{"authorization", "Bearer alice-token"}]) [session_id] = get_resp_header(conn, "mcp-session-id") + authed = [ + {"authorization", "Bearer alice-token"}, + {"mcp-session-id", session_id}, + {"mcp-protocol-version", "2025-11-25"} + ] + + # Complete the handshake so the tool call passes the lifecycle gate. + ack = post(%{jsonrpc: "2.0", method: "notifications/initialized"}, authed) + assert ack.status == 202 + conn = post( %{jsonrpc: "2.0", id: 2, method: "tools/call", params: %{name: "whoami", arguments: %{}}}, - [ - {"authorization", "Bearer alice-token"}, - {"mcp-session-id", session_id}, - {"mcp-protocol-version", "2025-11-25"} - ] + authed ) assert conn.status == 200 diff --git a/test/urchin/transport/streamable_http_test.exs b/test/urchin/transport/streamable_http_test.exs index 82fdb6f..a3e4eb5 100644 --- a/test/urchin/transport/streamable_http_test.exs +++ b/test/urchin/transport/streamable_http_test.exs @@ -52,6 +52,11 @@ defmodule Urchin.Transport.StreamableHTTPTest do assert conn.status == 200 [session_id] = get_resp_header(conn, "mcp-session-id") + + # Complete the handshake so subsequent operation requests pass the lifecycle gate. + ack = call_with_session(%{jsonrpc: "2.0", method: "notifications/initialized"}, session_id) + assert ack.status == 202 + {session_id, Jason.decode!(conn.resp_body)} end @@ -236,7 +241,11 @@ defmodule Urchin.Transport.StreamableHTTPTest do jsonrpc: "2.0", id: 1, method: "initialize", - params: %{"protocolVersion" => "2025-11-25", "capabilities" => %{}} + params: %{ + "protocolVersion" => "2025-11-25", + "capabilities" => %{}, + "clientInfo" => %{"name" => "c", "version" => "1"} + } }, [{"origin", "http://localhost:3000"}] ) @@ -324,41 +333,35 @@ defmodule Urchin.Transport.StreamableHTTPTest do end end - describe "enforce_initialized" do + describe "initialized lifecycle gate" do test "gates operation requests until notifications/initialized" do - opts = StreamableHTTP.init(server: EchoServer, enforce_initialized: true) - init_conn = - post( - %{ - jsonrpc: "2.0", - id: 1, - method: "initialize", - params: %{ - "protocolVersion" => "2025-11-25", - "capabilities" => %{}, - "clientInfo" => %{"name" => "c", "version" => "1"} - } - }, - [], - opts - ) + post(%{ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: %{ + "protocolVersion" => "2025-11-25", + "capabilities" => %{}, + "clientInfo" => %{"name" => "c", "version" => "1"} + } + }) assert init_conn.status == 200 [session_id] = get_resp_header(init_conn, "mcp-session-id") headers = [{"mcp-session-id", session_id}, {"mcp-protocol-version", "2025-11-25"}] # Before notifications/initialized: rejected as invalid_request. - before = post(%{jsonrpc: "2.0", id: 2, method: "tools/list"}, headers, opts) + before = post(%{jsonrpc: "2.0", id: 2, method: "tools/list"}, headers) assert before.status == 200 assert Jason.decode!(before.resp_body)["error"]["code"] == -32_600 # Acknowledge initialization. - ack = post(%{jsonrpc: "2.0", method: "notifications/initialized"}, headers, opts) + ack = post(%{jsonrpc: "2.0", method: "notifications/initialized"}, headers) assert ack.status == 202 # After: allowed. - after_conn = post(%{jsonrpc: "2.0", id: 3, method: "tools/list"}, headers, opts) + after_conn = post(%{jsonrpc: "2.0", id: 3, method: "tools/list"}, headers) assert after_conn.status == 200 assert is_list(Jason.decode!(after_conn.resp_body)["result"]["tools"]) end @@ -412,8 +415,8 @@ defmodule Urchin.Transport.StreamableHTTPTest do end end - describe "tool_errors over the transport" do - test "a handler {:error, message} becomes an isError result by default" do + describe "tool errors over the transport" do + test "a handler {:error, message} becomes an isError result" do {session_id, _} = init_session() conn = @@ -432,44 +435,6 @@ defmodule Urchin.Transport.StreamableHTTPTest do assert body["result"]["isError"] == true assert body["result"]["content"] == [%{"type" => "text", "text" => "tool said no"}] end - - test "with tool_errors: :json_rpc a handler {:error, message} becomes a JSON-RPC error" do - json_rpc_opts = StreamableHTTP.init(server: EchoServer, tool_errors: :json_rpc) - {session_id, _} = init_session() - - conn = - post( - %{ - jsonrpc: "2.0", - id: 21, - method: "tools/call", - params: %{name: "failing", arguments: %{}} - }, - [{"mcp-session-id", session_id}, {"mcp-protocol-version", "2025-11-25"}], - json_rpc_opts - ) - - assert conn.status == 200 - body = Jason.decode!(conn.resp_body) - assert body["error"]["code"] == -32_603 - assert body["error"]["message"] == "tool said no" - refute Map.has_key?(body, "result") - end - end - - describe "tool_errors option validation" do - test "defaults to :result and accepts :json_rpc" do - assert %{tool_errors: :result} = StreamableHTTP.init(server: EchoServer) - - assert %{tool_errors: :json_rpc} = - StreamableHTTP.init(server: EchoServer, tool_errors: :json_rpc) - end - - test "rejects an unknown value" do - assert_raise ArgumentError, fn -> - StreamableHTTP.init(server: EchoServer, tool_errors: :bad) - end - end end describe "logging/setLevel over the transport" do @@ -489,25 +454,19 @@ defmodule Urchin.Transport.StreamableHTTPTest do end end - describe "enforce_initialized notifications" do + describe "client notifications before initialized" do test "client notifications are accepted with 202 before initialized" do - opts = StreamableHTTP.init(server: EchoServer, enforce_initialized: true) - init = - post( - %{ - jsonrpc: "2.0", - id: 1, - method: "initialize", - params: %{ - "protocolVersion" => "2025-11-25", - "capabilities" => %{}, - "clientInfo" => %{"name" => "c", "version" => "1"} - } - }, - [], - opts - ) + post(%{ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: %{ + "protocolVersion" => "2025-11-25", + "capabilities" => %{}, + "clientInfo" => %{"name" => "c", "version" => "1"} + } + }) [session_id] = get_resp_header(init, "mcp-session-id") headers = [{"mcp-session-id", session_id}, {"mcp-protocol-version", "2025-11-25"}] @@ -515,8 +474,7 @@ defmodule Urchin.Transport.StreamableHTTPTest do cancelled = post( %{jsonrpc: "2.0", method: "notifications/cancelled", params: %{requestId: "x"}}, - headers, - opts + headers ) assert cancelled.status == 202 From b1662768c532b0b64b122c7c370a1ae4b69e5570 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sun, 7 Jun 2026 01:09:31 +0900 Subject: [PATCH 25/37] docs: document spec-compliance-by-default and bump to 0.3.0 Rewrite the unreleased CHANGELOG entries to describe the now always-on behaviors, drop the removed transport/server options from the README options table and SECURITY notes, and bump the version to 0.3.0 (a breaking change under pre-1.0 semantic versioning). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 53 +++++++++++++++++++++++++++++++--------------------- README.md | 12 ++++++++---- SECURITY.md | 10 +++++----- mix.exs | 2 +- 4 files changed, 46 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d25b7de..8070015 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +This release makes the server enforce the MCP specification by default. Several behaviors +that were previously absent or lenient are now always on; see Changed for the breaking +details and how to adapt. + ### Added - Session lifecycle limits: `:max_sessions` (reject new sessions with `503` past a cap — @@ -19,41 +23,50 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Declarative tool scopes: `tool "name", scopes: ["files:write"], ...` enforces the scopes against `ctx.auth` before the handler runs, failing closed when the request carries no authorization. -- `:validate_arguments` transport option (default `false`) validates `tools/call` arguments - against each DSL tool's `input_schema` before the handler runs. A mismatch is treated as a - tool-input error: surfaced as a `CallToolResult` with `isError: true` by default (per - `:tool_errors`), or a JSON-RPC `invalid_params` error under `tool_errors: :json_rpc`. - `Urchin.Schema` implements the supported (minimal) JSON Schema subset. - `:expose_internal_errors` transport option (default `false`). Unexpected exceptions and malformed handler returns are now logged in full but return a generic message to the client; enable the option to surface the detail in development. Deliberate `Urchin.Error` values and `{:error, message}` returns are never redacted — their `message`/`data` reach the - client unchanged. Whether a `tools/call` string error is delivered as a JSON-RPC error or an - `isError` result is governed separately by `:tool_errors` (see below). + client unchanged. - Capability guards: `Urchin.Context.create_message/3`, `elicit/3` and `list_roots/2` return an error without contacting the client when it did not advertise the matching `sampling`/`elicitation`/`roots` capability. - `415 Unsupported Media Type` for POST requests whose `Content-Type` is not `application/json`. - `SECURITY.md` with a threat model, deployment checklist and vulnerability reporting. -- `:enforce_initialized` transport option (default `false`) rejecting operation requests - received before the client sends `notifications/initialized` with `invalid_request`; - only `ping` is allowed. The default may be flipped to `true` in a future minor release; set - `true` for strict MCP lifecycle compliance. -- `:tool_errors` transport option (`:result` default | `:json_rpc`). By default a `tools/call` - handler's `{:error, message}` (string) is now returned as a `CallToolResult` with - `isError: true` so the model can self-correct (the spec-compliant behavior); set `:json_rpc` - for the legacy behavior of returning a JSON-RPC internal error. A protocol error returned as - `{:error, %Urchin.Error{}}` is always a JSON-RPC error. Note: this changes the prior behavior - where such a handler error became a JSON-RPC error. -- `validate_tool_names: true` option for `use Urchin.Server` enforcing, at compile time, that - every literal tool name matches `~r/\A[a-zA-Z0-9_.-]{1,128}\z/` (default `false`). - `:sse_buffer_limit` transport option (default `nil`, preserving the session's internal default of `100`) forwarding the per-session GET-stream replay buffer size to the session; previously only configurable on `Urchin.Session` directly. ### Changed +The following are now enforced by default, with no opt-out, for MCP spec compliance. They are +breaking relative to `0.2.0`. + +- `tools/call` arguments are validated against each tool's `input_schema` before the handler + runs; a mismatch is returned as a `CallToolResult` with `isError: true` so the model can + self-correct. A tool that declares no `input_schema` now defaults to an object that accepts + no properties (`additionalProperties: false`), so unexpected arguments are rejected — declare + an explicit `input_schema` to accept arbitrary fields. A non-object `arguments` value is a + malformed request and remains a JSON-RPC `invalid_params` error. `Urchin.Schema` implements + the supported (minimal) JSON Schema subset. +- Operation requests received before the client sends `notifications/initialized` are rejected + with `invalid_request`; only `ping` and `logging/setLevel` are allowed before initialization. + Clients must complete the lifecycle handshake before issuing other requests. +- A `tools/call` handler's `{:error, message}` (string) is returned as a `CallToolResult` with + `isError: true` so the model can self-correct. A protocol error returned as + `{:error, %Urchin.Error{}}` is always a JSON-RPC error. (Previously a string handler error + became a JSON-RPC internal error.) +- Tool names are validated at compile time: every literal name must match + `~r/\A[a-zA-Z0-9_.-]{1,128}\z/`, and duplicate names within a server are rejected (a silently + shadowed duplicate was previously accepted, with the last declaration winning). +- `initialize` requires `protocolVersion` (string), `capabilities` (object) and `clientInfo` + (with a string `name` and `version`); a missing or mistyped field is an `invalid_params` + error rather than a silently-defaulted value. The server's `serverInfo` must likewise carry a + string `name` and `version`. +- The `MCP-Protocol-Version` header is validated on `DELETE`, matching `POST` and `GET`. +- `completion/complete` results are capped at 100 values; a handler returning more is truncated + to the top 100 (already ranked by relevance) with `hasMore` set. - `logging/setLevel` is now a library builtin: when the server advertises the `logging` capability (via `use Urchin.Server, logging: true`) it succeeds and applies the level to the session even without a `set_log_level/2` callback. The level is validated against the MCP log @@ -63,8 +76,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Duplicate tool names declared via the DSL are now rejected at compile time (a silently - shadowed duplicate was previously accepted, with the last declaration winning). - README no longer claims unqualified "resumable SSE streams"; resumption is scoped to the GET stream, matching the implementation. diff --git a/README.md b/README.md index 3eece2a..87246ff 100644 --- a/README.md +++ b/README.md @@ -289,7 +289,7 @@ tool "delete", description: "Delete a file" do else # Return an Urchin.Error so the denial is a JSON-RPC `invalid_request`, matching the # declarative `scopes:` path. A bare string `{:error, "..."}` would instead surface as a - # `CallToolResult` with `isError: true` under the default `tool_errors: :result`. + # `CallToolResult` with `isError: true`. {:error, Urchin.Error.invalid_request("files:write scope required")} end end @@ -347,9 +347,6 @@ Passed to `Urchin.Transport.StreamableHTTP`, `Urchin.Endpoint` or `Urchin.start_ | `:request_timeout` | `60_000` | per-request handler timeout (ms) | | `:validate_protocol_version` | `true` | validate the `MCP-Protocol-Version` header | | `:expose_internal_errors` | `false` | return raised-exception messages to the client (dev only); exceptions are always logged | -| `:validate_arguments` | `false` | validate `tools/call` arguments against each tool's `input_schema` before the handler runs; a mismatch is surfaced per `:tool_errors` (an `isError` result by default). A non-object `arguments` is a malformed request and is always a JSON-RPC `invalid_params` error, regardless of this option or `:tool_errors`. See `Urchin.Schema` | -| `:enforce_initialized` | `false` | reject operation requests received before `notifications/initialized` with `invalid_request`; only `ping` is allowed | -| `:tool_errors` | `:result` | how a `tools/call` handler's `{:error, binary}` is surfaced: `:result` returns a `CallToolResult` with `isError: true` so the model can self-correct; `:json_rpc` returns a JSON-RPC internal error. `{:error, %Urchin.Error{}}` is always a JSON-RPC error; other methods are unaffected | | `:sse_buffer_limit` | `nil` | max recent GET-stream (general SSE) events kept per session for resumption replay (`nil` keeps the session default of `100`) | | `:max_sessions` | `nil` | reject new sessions with `503` past this many, atomically and before the server's `init/1` runs (`nil` = unlimited) | | `:session_idle_timeout` | `nil` | terminate a session after this many ms without client activity; a session serving a request is not reaped (`nil` = never) | @@ -358,6 +355,13 @@ Passed to `Urchin.Transport.StreamableHTTP`, `Urchin.Endpoint` or `Urchin.start_ `Urchin.Endpoint`/`Urchin.start_link/2` additionally accept `:port`, `:ip`, `:scheme` and `:path`. +Some MCP behaviors are enforced unconditionally and have no option: `tools/call` arguments are +validated against each tool's `input_schema` (a mismatch is an `isError` `CallToolResult`; a tool +with no schema accepts no properties); operation requests before `notifications/initialized` are +rejected (`ping` and `logging/setLevel` excepted); a `tools/call` handler's `{:error, binary}` is +returned as an `isError` `CallToolResult`; tool names are validated at compile time; and +`completion/complete` results are capped at 100 values. + ## Specification coverage | Area | Methods | diff --git a/SECURITY.md b/SECURITY.md index c5c3047..ec04d98 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -19,8 +19,8 @@ and does not protect against, and what you must add before exposing a server pub default `false`, opts into the detail for development). Deliberate errors — `Urchin.Error` values and `{:error, message}` returns — are not redacted; their `message`/`data` reach the client unchanged, so keep secrets and internals out of them. For `tools/call`, a string - `{:error, message}` is by default surfaced as a `CallToolResult` with `isError: true` (see - `:tool_errors`) rather than a JSON-RPC error. + `{:error, message}` is surfaced as a `CallToolResult` with `isError: true` rather than a + JSON-RPC error. - **Capability-gated server-initiated requests.** `sampling/createMessage`, `elicitation/create` and `roots/list` are only sent when the client advertised the capability. @@ -28,8 +28,8 @@ and does not protect against, and what you must add before exposing a server pub `ctx.auth` before the handler runs, failing closed when the request carries no authorization (only meaningful when `ctx.auth` is populated, typically by `:auth` or an upstream `Urchin.Auth.Plug`). -- **Opt-in argument validation.** `:validate_arguments` checks `tools/call` arguments - against each tool's `input_schema`. It is a minimal subset of JSON Schema (see +- **Argument validation.** `tools/call` arguments are validated against each tool's + `input_schema` before the handler runs. It is a minimal subset of JSON Schema (see `Urchin.Schema`), so unsupported keywords and `output_schema` are still your handler's responsibility. - **Bounded request bodies** (`@max_body`, ~8 MB) and a per-request handler timeout. @@ -51,7 +51,7 @@ Urchin does **not** yet provide these; supply them in your deployment: 4. **Per-tool authorization beyond scopes.** Declarative `scopes:` covers scope checks; add app-specific authorization (ownership, tenancy, row-level access) in handlers via `ctx.auth`. -5. **Full input validation.** Enable `:validate_arguments` for structural checks, but +5. **Full input validation.** Structural checks against `input_schema` run automatically, but validate unsupported JSON Schema keywords, business rules and `output_schema` in your handler — `Urchin.Schema` is a minimal subset. diff --git a/mix.exs b/mix.exs index d2c93e4..7c4df7b 100644 --- a/mix.exs +++ b/mix.exs @@ -1,7 +1,7 @@ defmodule Urchin.MixProject do use Mix.Project - @version "0.2.0" + @version "0.3.0" @source_url "https://github.com/urth-inc/urchin" def project do From cb7fa3238fdf2ab5cb7e71e3a455d6ae82e8527c Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sun, 7 Jun 2026 01:33:36 +0900 Subject: [PATCH 26/37] fix(dispatcher): keep a raised Urchin.Error a JSON-RPC error in tools/call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit call_tool_result/4 rescued every exception into an isError CallToolResult, so a handler raising Urchin.Error became a tool result while a returned {:error, %Urchin.Error{}} became a JSON-RPC error — an asymmetry that also contradicted the Urchin.Error docs. Rescue Urchin.Error first and surface it as a JSON-RPC error; any other exception still becomes an isError result. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/urchin/dispatcher.ex | 8 +++++++- test/support/echo_server.ex | 5 +++++ test/urchin/dispatcher_test.exs | 7 +++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/urchin/dispatcher.ex b/lib/urchin/dispatcher.ex index 01cc328..b191f62 100644 --- a/lib/urchin/dispatcher.ex +++ b/lib/urchin/dispatcher.ex @@ -279,8 +279,14 @@ defmodule Urchin.Dispatcher do other -> tool_error_result(other, ctx) end rescue + error in Urchin.Error -> + # A deliberately raised Urchin.Error is a protocol-level error, matching a returned + # {:error, %Urchin.Error{}}: it stays a JSON-RPC error rather than an isError result. + {:error, error} + exception -> - # A tool that raises reports a tool-execution error so the model can self-correct. + # A tool that raises any other exception reports a tool-execution error so the model + # can self-correct. Logger.error( "Urchin tool #{name} crashed: " <> Exception.format(:error, exception, __STACKTRACE__) diff --git a/test/support/echo_server.ex b/test/support/echo_server.ex index 432d0e8..7c868d5 100644 --- a/test/support/echo_server.ex +++ b/test/support/echo_server.ex @@ -57,6 +57,11 @@ defmodule Urchin.Test.EchoServer do {:error, Urchin.Error.invalid_params("bad")} end + tool "raise_protocol", description: "Raises an Urchin.Error protocol error" do + _ = {args, ctx} + raise Urchin.Error.invalid_params("raised bad") + end + tool "secret", description: "Requires the secret:read scope", scopes: ["secret:read"] do diff --git a/test/urchin/dispatcher_test.exs b/test/urchin/dispatcher_test.exs index 505cef3..5011cb3 100644 --- a/test/urchin/dispatcher_test.exs +++ b/test/urchin/dispatcher_test.exs @@ -522,5 +522,12 @@ defmodule Urchin.DispatcherTest do assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx()) assert error.code == -32_602 end + + test "a raised Urchin.Error is a JSON-RPC error, not an isError result" do + params = %{"name" => "raise_protocol", "arguments" => %{}} + assert {:error, error} = Dispatcher.handle_request(EchoServer, "tools/call", params, ctx()) + assert error.code == -32_602 + assert error.message == "raised bad" + end end end From 0d8d0dc5d6666508f590e9cb6d62cefabae7c1d5 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Sun, 7 Jun 2026 01:33:43 +0900 Subject: [PATCH 27/37] docs: scope argument validation to DSL tools and clarify the tool-name convention Automatic input-schema validation lives in the DSL-generated call_tool/3, so a hand-written Urchin.Server validates its own arguments; say so in the README, SECURITY notes, transport moduledoc and CHANGELOG. Also note that the tool-name pattern is a tool-calling-SDK convention Urchin enforces, not an MCP-mandated pattern. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 22 ++++++++++++---------- README.md | 11 ++++++----- SECURITY.md | 8 ++++---- lib/urchin/transport/streamable_http.ex | 7 ++++--- 4 files changed, 26 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8070015..8975944 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,13 +43,14 @@ details and how to adapt. The following are now enforced by default, with no opt-out, for MCP spec compliance. They are breaking relative to `0.2.0`. -- `tools/call` arguments are validated against each tool's `input_schema` before the handler - runs; a mismatch is returned as a `CallToolResult` with `isError: true` so the model can - self-correct. A tool that declares no `input_schema` now defaults to an object that accepts - no properties (`additionalProperties: false`), so unexpected arguments are rejected — declare - an explicit `input_schema` to accept arbitrary fields. A non-object `arguments` value is a - malformed request and remains a JSON-RPC `invalid_params` error. `Urchin.Schema` implements - the supported (minimal) JSON Schema subset. +- A DSL tool's `tools/call` arguments are validated against its `input_schema` before the + handler runs; a mismatch is returned as a `CallToolResult` with `isError: true` so the model + can self-correct. A tool that declares no `input_schema` now defaults to an object that + accepts no properties (`additionalProperties: false`), so unexpected arguments are rejected — + declare an explicit `input_schema` to accept arbitrary fields. A non-object `arguments` value + is a malformed request and remains a JSON-RPC `invalid_params` error. Servers that implement + `call_tool/3` by hand validate their own arguments. `Urchin.Schema` implements the supported + (minimal) JSON Schema subset. - Operation requests received before the client sends `notifications/initialized` are rejected with `invalid_request`; only `ping` and `logging/setLevel` are allowed before initialization. Clients must complete the lifecycle handshake before issuing other requests. @@ -57,9 +58,10 @@ breaking relative to `0.2.0`. `isError: true` so the model can self-correct. A protocol error returned as `{:error, %Urchin.Error{}}` is always a JSON-RPC error. (Previously a string handler error became a JSON-RPC internal error.) -- Tool names are validated at compile time: every literal name must match - `~r/\A[a-zA-Z0-9_.-]{1,128}\z/`, and duplicate names within a server are rejected (a silently - shadowed duplicate was previously accepted, with the last declaration winning). +- Literal tool names are validated at compile time against `~r/\A[a-zA-Z0-9_.-]{1,128}\z/` — a + de-facto convention shared by common tool-calling SDKs that Urchin now enforces, though the + MCP schema itself imposes no pattern — and duplicate names within a server are rejected (a + silently shadowed duplicate was previously accepted, with the last declaration winning). - `initialize` requires `protocolVersion` (string), `capabilities` (object) and `clientInfo` (with a string `name` and `version`); a missing or mistyped field is an `invalid_params` error rather than a silently-defaulted value. The server's `serverInfo` must likewise carry a diff --git a/README.md b/README.md index 87246ff..fd5167d 100644 --- a/README.md +++ b/README.md @@ -355,11 +355,12 @@ Passed to `Urchin.Transport.StreamableHTTP`, `Urchin.Endpoint` or `Urchin.start_ `Urchin.Endpoint`/`Urchin.start_link/2` additionally accept `:port`, `:ip`, `:scheme` and `:path`. -Some MCP behaviors are enforced unconditionally and have no option: `tools/call` arguments are -validated against each tool's `input_schema` (a mismatch is an `isError` `CallToolResult`; a tool -with no schema accepts no properties); operation requests before `notifications/initialized` are -rejected (`ping` and `logging/setLevel` excepted); a `tools/call` handler's `{:error, binary}` is -returned as an `isError` `CallToolResult`; tool names are validated at compile time; and +Some MCP behaviors are enforced unconditionally and have no option: a DSL tool's `tools/call` +arguments are validated against its `input_schema` (a mismatch is an `isError` `CallToolResult`; a +tool with no schema accepts no properties — servers that implement `call_tool/3` by hand validate +their own arguments); operation requests before `notifications/initialized` are rejected (`ping` +and `logging/setLevel` excepted); a `tools/call` handler's `{:error, binary}` is returned as an +`isError` `CallToolResult`; literal tool names are validated at compile time; and `completion/complete` results are capped at 100 values. ## Specification coverage diff --git a/SECURITY.md b/SECURITY.md index ec04d98..a5af851 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -28,10 +28,10 @@ and does not protect against, and what you must add before exposing a server pub `ctx.auth` before the handler runs, failing closed when the request carries no authorization (only meaningful when `ctx.auth` is populated, typically by `:auth` or an upstream `Urchin.Auth.Plug`). -- **Argument validation.** `tools/call` arguments are validated against each tool's - `input_schema` before the handler runs. It is a minimal subset of JSON Schema (see - `Urchin.Schema`), so unsupported keywords and `output_schema` are still your handler's - responsibility. +- **Argument validation.** A DSL tool's `tools/call` arguments are validated against its + `input_schema` before the handler runs (a hand-written `call_tool/3` validates its own + arguments). It is a minimal subset of JSON Schema (see `Urchin.Schema`), so unsupported + keywords and `output_schema` are still your handler's responsibility. - **Bounded request bodies** (`@max_body`, ~8 MB) and a per-request handler timeout. - **Session lifecycle limits** (opt-in): `:max_sessions`, `:session_idle_timeout` and `:session_max_lifetime`. Without them a session persists until the client sends `DELETE`, diff --git a/lib/urchin/transport/streamable_http.ex b/lib/urchin/transport/streamable_http.ex index 5a07b69..ba5a47b 100644 --- a/lib/urchin/transport/streamable_http.ex +++ b/lib/urchin/transport/streamable_http.ex @@ -38,9 +38,10 @@ defmodule Urchin.Transport.StreamableHTTP do endpoint is served by `Urchin.Endpoint`/`Urchin.Auth.Metadata`, not this plug. The transport enforces the spec by default and these behaviors are not configurable: it - validates `tools/call` arguments against each tool's input schema, rejects operation requests - received before `notifications/initialized` (only `ping` and `logging/setLevel` are allowed - pre-init), and surfaces a tool handler's `{:error, binary}` as an `isError` `CallToolResult`. + validates a DSL tool's `tools/call` arguments against its input schema (a hand-written + `call_tool/3` validates its own arguments), rejects operation requests received before + `notifications/initialized` (only `ping` and `logging/setLevel` are allowed pre-init), and + surfaces a tool handler's `{:error, binary}` as an `isError` `CallToolResult`. The plug reads the raw request body itself, so mount it before any JSON body parser. """ From 4b8b6072f748c3ecd27b6a0387ee7eca07f675fa Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Mon, 8 Jun 2026 10:56:18 +0900 Subject: [PATCH 28/37] fix(server): stop enforcing a tool-name pattern (match the spec) The MCP schema imposes no pattern on tool names, so rejecting non-conforming literal names at compile time was stricter than the spec required. Drop the pattern check entirely; only duplicate names within a server are still rejected, which prevents an unambiguous bug rather than enforcing a style. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 7 +++---- README.md | 2 +- lib/urchin/server.ex | 34 +++++++++------------------------- test/urchin/server_test.exs | 26 ++++---------------------- 4 files changed, 17 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8975944..5fc2f3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,10 +58,9 @@ breaking relative to `0.2.0`. `isError: true` so the model can self-correct. A protocol error returned as `{:error, %Urchin.Error{}}` is always a JSON-RPC error. (Previously a string handler error became a JSON-RPC internal error.) -- Literal tool names are validated at compile time against `~r/\A[a-zA-Z0-9_.-]{1,128}\z/` — a - de-facto convention shared by common tool-calling SDKs that Urchin now enforces, though the - MCP schema itself imposes no pattern — and duplicate names within a server are rejected (a - silently shadowed duplicate was previously accepted, with the last declaration winning). +- Duplicate tool names within a server are rejected at compile time (a silently shadowed + duplicate was previously accepted, with the last declaration winning). No tool-name pattern is + enforced, matching the MCP schema, which imposes none. - `initialize` requires `protocolVersion` (string), `capabilities` (object) and `clientInfo` (with a string `name` and `version`); a missing or mistyped field is an `invalid_params` error rather than a silently-defaulted value. The server's `serverInfo` must likewise carry a diff --git a/README.md b/README.md index fd5167d..2f0e715 100644 --- a/README.md +++ b/README.md @@ -360,7 +360,7 @@ arguments are validated against its `input_schema` (a mismatch is an `isError` ` tool with no schema accepts no properties — servers that implement `call_tool/3` by hand validate their own arguments); operation requests before `notifications/initialized` are rejected (`ping` and `logging/setLevel` excepted); a `tools/call` handler's `{:error, binary}` is returned as an -`isError` `CallToolResult`; literal tool names are validated at compile time; and +`isError` `CallToolResult`; duplicate tool names are rejected at compile time; and `completion/complete` results are capped at 100 values. ## Specification coverage diff --git a/lib/urchin/server.ex b/lib/urchin/server.ex index 4d3a58f..8186bbf 100644 --- a/lib/urchin/server.ex +++ b/lib/urchin/server.ex @@ -34,8 +34,8 @@ defmodule Urchin.Server do Capabilities are derived automatically from the declared features. - Duplicate tool names declared via the DSL are rejected at compile time, and every literal tool - name must match `~r/\A[a-zA-Z0-9_.-]{1,128}\z/`; a non-matching name raises `ArgumentError`. + Duplicate tool names declared via the DSL are rejected at compile time. No tool-name pattern is + enforced, matching the MCP schema, which imposes none. ## Behaviour @@ -59,11 +59,6 @@ defmodule Urchin.Server do alias Urchin.{Context, Error} - # Constrained tool-name charset. The MCP schema imposes no pattern, but this is the - # de-facto convention shared by common tool-calling SDKs; dots and dashes are permitted - # for namespacing. Enforced for every literal tool name at compile time. - @tool_name_pattern ~r/\A[a-zA-Z0-9_.-]{1,128}\z/ - @type cursor :: String.t() | nil @type list_result(item) :: {:ok, [item]} | {:ok, [item], cursor()} | {:error, Error.t() | String.t()} @@ -258,25 +253,14 @@ defmodule Urchin.Server do end # Duplicate tool names within a server are rejected at compile time (a silently shadowed - # duplicate is a bug), and every literal name must match @tool_name_pattern. Non-literal names - # (a variable or call) cannot be compared statically and are skipped, mirroring handler_name/2. + # duplicate is a bug). The MCP schema imposes no tool-name pattern, so none is enforced. + # Non-literal names (a variable or call) cannot be compared statically and are skipped, + # mirroring handler_name/2. defp validate_tool_names!(tool_dispatch) do - names = - tool_dispatch - |> Enum.map(fn {name, _fname, _scopes} -> name end) - |> Enum.filter(&is_binary/1) - - validate_unique_tool_names!(names) - Enum.each(names, &validate_tool_name_pattern!/1) - - :ok - end - - defp validate_tool_name_pattern!(name) do - if not Regex.match?(@tool_name_pattern, name) do - raise ArgumentError, - "tool name #{inspect(name)} is invalid; must match #{inspect(@tool_name_pattern.source)}" - end + tool_dispatch + |> Enum.map(fn {name, _fname, _scopes} -> name end) + |> Enum.filter(&is_binary/1) + |> validate_unique_tool_names!() end defp validate_unique_tool_names!(names) do diff --git a/test/urchin/server_test.exs b/test/urchin/server_test.exs index 0796027..734c4b7 100644 --- a/test/urchin/server_test.exs +++ b/test/urchin/server_test.exs @@ -34,10 +34,10 @@ defmodule Urchin.ServerTest do end describe "tool name validation" do - test "rejects an invalid tool name at compile time" do + test "does not enforce a tool-name pattern (the MCP schema imposes none)" do source = """ - defmodule Urchin.ServerTest.BadName do - use Urchin.Server, name: "bad", version: "1.0.0" + defmodule Urchin.ServerTest.UnusualName do + use Urchin.Server, name: "unusual", version: "1.0.0" tool "bad name!" do {:ok, [Urchin.Content.text("ok")]} @@ -45,25 +45,7 @@ defmodule Urchin.ServerTest do end """ - assert_raise ArgumentError, ~r/tool name .* is invalid/, fn -> - Code.compile_string(source) - end - end - - test "rejects a tool name with a trailing newline at compile time" do - source = """ - defmodule Urchin.ServerTest.NewlineName do - use Urchin.Server, name: "nl", version: "1.0.0" - - tool "abc\\n" do - {:ok, [Urchin.Content.text("ok")]} - end - end - """ - - assert_raise ArgumentError, ~r/tool name .* is invalid/, fn -> - Code.compile_string(source) - end + assert [{Urchin.ServerTest.UnusualName, _} | _] = Code.compile_string(source) end test "rejects duplicate tool names at compile time (always on)" do From df0482a108893eac463a66b20949deb532d2c819 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Mon, 8 Jun 2026 14:29:02 +0900 Subject: [PATCH 29/37] refactor(session): set the initialized flag only via mark_initialized/1 notifications/initialized was committed synchronously by the transport through mark_initialized/1 and also set initialized: true in the async handle_client notification path. The transport never routes the notification to the async path, so that clause was dead and risked drifting from the synchronous one. Drop it and let mark_initialized/1 be the single source. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/urchin/session.ex | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/urchin/session.ex b/lib/urchin/session.ex index 157cd86..5cdfe74 100644 --- a/lib/urchin/session.ex +++ b/lib/urchin/session.ex @@ -108,8 +108,9 @@ defmodule Urchin.Session do @doc """ Handles a client-originated notification or response delivered over POST. - Notifications (`notifications/cancelled`, `notifications/initialized`, ...) update - session state; responses are correlated to a pending outbound request. + Notifications (`notifications/cancelled`, ...) update session state; responses are correlated + to a pending outbound request. `notifications/initialized` is committed synchronously via + `mark_initialized/1`, not through this path. """ @spec handle_client_message(pid(), Urchin.JSONRPC.decoded()) :: :ok def handle_client_message(pid, message), do: GenServer.cast(pid, {:client_message, message}) @@ -343,9 +344,8 @@ defmodule Urchin.Session do ## Internal: client message handling - defp handle_client({:notification, "notifications/initialized", _params}, state) do - %{state | initialized: true} - end + # notifications/initialized is committed synchronously via mark_initialized/1 (the transport + # routes it there so the next request observes initialized: true), so it is not handled here. defp handle_client({:notification, "notifications/cancelled", params}, state) do request_id = Map.get(params, "requestId") From b640c1c44e9290edc17665727e5be6c09195f915 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Mon, 8 Jun 2026 14:29:13 +0900 Subject: [PATCH 30/37] docs: note that MCP tool-name recommendations still apply Urchin enforces no tool-name pattern, but the MCP spec still recommends a conservative charset, a length bound and no whitespace. Say servers should follow those recommendations rather than implying names are unconstrained. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 5 +++-- lib/urchin/server.ex | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fc2f3a..3a1231d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,8 +59,9 @@ breaking relative to `0.2.0`. `{:error, %Urchin.Error{}}` is always a JSON-RPC error. (Previously a string handler error became a JSON-RPC internal error.) - Duplicate tool names within a server are rejected at compile time (a silently shadowed - duplicate was previously accepted, with the last declaration winning). No tool-name pattern is - enforced, matching the MCP schema, which imposes none. + duplicate was previously accepted, with the last declaration winning). Urchin enforces no + tool-name pattern (the MCP schema imposes none); servers should still follow the MCP naming + recommendations. - `initialize` requires `protocolVersion` (string), `capabilities` (object) and `clientInfo` (with a string `name` and `version`); a missing or mistyped field is an `invalid_params` error rather than a silently-defaulted value. The server's `serverInfo` must likewise carry a diff --git a/lib/urchin/server.ex b/lib/urchin/server.ex index 8186bbf..c4ae601 100644 --- a/lib/urchin/server.ex +++ b/lib/urchin/server.ex @@ -34,8 +34,9 @@ defmodule Urchin.Server do Capabilities are derived automatically from the declared features. - Duplicate tool names declared via the DSL are rejected at compile time. No tool-name pattern is - enforced, matching the MCP schema, which imposes none. + Duplicate tool names declared via the DSL are rejected at compile time. Urchin enforces no + tool-name pattern (the MCP schema imposes none); servers should still follow the MCP naming + recommendations (a conservative charset, a length bound, no whitespace). ## Behaviour From ac3bae9adb71116458f5301cd80831a9bc02155d Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Mon, 8 Jun 2026 14:29:13 +0900 Subject: [PATCH 31/37] test(dispatcher): mark logging/setLevel tests as the initialized path logging/setLevel is allowed before initialization, so a direct test with initialized: false read ambiguously. Set initialized: true on the normal-path logging tests; the pre-init case is covered explicitly in the gating describe. Co-Authored-By: Claude Opus 4.8 (1M context) --- test/urchin/dispatcher_test.exs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/urchin/dispatcher_test.exs b/test/urchin/dispatcher_test.exs index 5011cb3..c2ed3ce 100644 --- a/test/urchin/dispatcher_test.exs +++ b/test/urchin/dispatcher_test.exs @@ -413,14 +413,14 @@ defmodule Urchin.DispatcherTest do EchoServer, "logging/setLevel", %{"level" => "error"}, - %Context{session: pid} + %Context{session: pid, initialized: true} ) assert Session.snapshot(pid).min_log_level == "error" end test "invokes an exported set_log_level/2 hook" do - ctx = %Context{assigns: %{test_pid: self()}} + ctx = %Context{assigns: %{test_pid: self()}, initialized: true} assert {:ok, %{}} = Dispatcher.handle_request( @@ -442,7 +442,7 @@ defmodule Urchin.DispatcherTest do EchoServer, "logging/setLevel", %{"level" => "verbose"}, - %Context{session: pid} + %Context{session: pid, initialized: true} ) assert error.code == -32_602 @@ -473,7 +473,7 @@ defmodule Urchin.DispatcherTest do FailingLoggingServer, "logging/setLevel", %{"level" => "warning"}, - %Context{session: pid} + %Context{session: pid, initialized: true} ) assert Session.snapshot(pid).min_log_level == before From 82e7768193198d317a80d1b29c4438854e347210 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Mon, 8 Jun 2026 15:05:18 +0900 Subject: [PATCH 32/37] docs: clarify that only non-Urchin.Error raises become isError results A tool that raises Urchin.Error is surfaced as a JSON-RPC error, like a returned {:error, %Urchin.Error{}}; only other exceptions become an isError result. The moduledoc read as if any raise became isError. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/urchin/server.ex | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/urchin/server.ex b/lib/urchin/server.ex index c4ae601..efffdfb 100644 --- a/lib/urchin/server.ex +++ b/lib/urchin/server.ex @@ -52,10 +52,11 @@ defmodule Urchin.Server do * `read_resource/2`: `{:ok, contents}` or `{:error, reason}` * `get_prompt/3`: `{:ok, messages}` or `{:ok, messages, description}` - For every callback, an `{:error, %Urchin.Error{}}` becomes that JSON-RPC error. A `call_tool/3` - handler's `{:error, binary}` is surfaced as a `CallToolResult` with `isError: true` so the model - can self-correct, as is a tool that raises. For the other callbacks an `{:error, binary}` becomes - a JSON-RPC internal error and a raised exception becomes an internal error. + For every callback, a returned or raised `Urchin.Error` becomes that JSON-RPC error. A + `call_tool/3` handler's `{:error, binary}` is surfaced as a `CallToolResult` with `isError: true` + so the model can self-correct, as is a tool that raises any other exception. For the other + callbacks an `{:error, binary}` becomes a JSON-RPC internal error and any other raised exception + becomes an internal error. """ alias Urchin.{Context, Error} From 4a887c5d9bcd28b40a50670325cff1e74f7d7030 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Mon, 8 Jun 2026 15:22:40 +0900 Subject: [PATCH 33/37] fix(dispatcher): gate logging/setLevel before initialization Only ping is exempt from the lifecycle gate. The MCP lifecycle's pings-and-logging exception covers the server's own requests/notifications, not the client's logging/setLevel, so a pre-init logging/setLevel is now rejected with invalid_request like any other operation request. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/urchin/dispatcher.ex | 10 +++++----- test/urchin/dispatcher_test.exs | 8 ++++++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/urchin/dispatcher.ex b/lib/urchin/dispatcher.ex index b191f62..7ae4830 100644 --- a/lib/urchin/dispatcher.ex +++ b/lib/urchin/dispatcher.ex @@ -113,7 +113,7 @@ defmodule Urchin.Dispatcher do def handle_request(server, method, params, ctx) do # Lifecycle gate: until notifications/initialized has been received, reject operation requests - # other than ping and logging/setLevel with invalid_request. + # other than ping with invalid_request. with :ok <- check_initialized(method, ctx) do do_handle(server, method, params, ctx) end @@ -145,11 +145,11 @@ defmodule Urchin.Dispatcher do end end - # Only ping and logging/setLevel are allowed before the client sends - # notifications/initialized; per the MCP lifecycle a client should not send other requests - # until initialization completes. + # Only ping is allowed before the client sends notifications/initialized; per the MCP lifecycle + # the client should not send requests other than pings until initialization completes. The + # pings-and-logging exception in the spec is for the server's own requests/notifications, not + # the client's logging/setLevel. defp pre_init_allowed?("ping"), do: true - defp pre_init_allowed?("logging/setLevel"), do: true defp pre_init_allowed?(_method), do: false # ping is always available regardless of declared capabilities. diff --git a/test/urchin/dispatcher_test.exs b/test/urchin/dispatcher_test.exs index c2ed3ce..95ddaf3 100644 --- a/test/urchin/dispatcher_test.exs +++ b/test/urchin/dispatcher_test.exs @@ -492,16 +492,20 @@ defmodule Urchin.DispatcherTest do assert {:ok, %{}} = Dispatcher.handle_request(EchoServer, "ping", %{}, ctx) end - test "allows logging/setLevel before initialized" do + test "rejects logging/setLevel before initialized (only ping is exempt)" do + # The lifecycle's pre-init exception for logging is the server's own requests, not the + # client's logging/setLevel, so it is gated like any other operation request. ctx = %Context{initialized: false} - assert {:ok, %{}} = + assert {:error, error} = Dispatcher.handle_request( EchoServer, "logging/setLevel", %{"level" => "info"}, ctx ) + + assert error.code == -32_600 end test "allows operation requests once initialized" do From ff4e7b18a5a1d2d5f68ef7e57a6afaa05807e02e Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Mon, 8 Jun 2026 15:22:41 +0900 Subject: [PATCH 34/37] docs: scope duplicate detection to literal names and fix the pre-init wording Duplicate-name rejection only compares literal names (non-literal names cannot be compared at compile time), so say "duplicate literal tool names". Also correct the lifecycle wording so only ping is described as exempt before initialization. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 11 +++++++---- README.md | 4 ++-- lib/urchin/server.ex | 7 ++++--- lib/urchin/transport/streamable_http.ex | 4 ++-- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a1231d..6de2994 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,14 +52,17 @@ breaking relative to `0.2.0`. `call_tool/3` by hand validate their own arguments. `Urchin.Schema` implements the supported (minimal) JSON Schema subset. - Operation requests received before the client sends `notifications/initialized` are rejected - with `invalid_request`; only `ping` and `logging/setLevel` are allowed before initialization. - Clients must complete the lifecycle handshake before issuing other requests. + with `invalid_request`; only `ping` is allowed before initialization (the lifecycle's + pings-and-logging exception is for the server's own requests, not the client's + `logging/setLevel`). Clients must complete the lifecycle handshake before issuing other + requests. - A `tools/call` handler's `{:error, message}` (string) is returned as a `CallToolResult` with `isError: true` so the model can self-correct. A protocol error returned as `{:error, %Urchin.Error{}}` is always a JSON-RPC error. (Previously a string handler error became a JSON-RPC internal error.) -- Duplicate tool names within a server are rejected at compile time (a silently shadowed - duplicate was previously accepted, with the last declaration winning). Urchin enforces no +- Duplicate literal tool names within a server are rejected at compile time (a silently shadowed + duplicate was previously accepted, with the last declaration winning); non-literal names (a + variable or expression) cannot be compared statically and are not checked. Urchin enforces no tool-name pattern (the MCP schema imposes none); servers should still follow the MCP naming recommendations. - `initialize` requires `protocolVersion` (string), `capabilities` (object) and `clientInfo` diff --git a/README.md b/README.md index 2f0e715..a7297d7 100644 --- a/README.md +++ b/README.md @@ -359,8 +359,8 @@ Some MCP behaviors are enforced unconditionally and have no option: a DSL tool's arguments are validated against its `input_schema` (a mismatch is an `isError` `CallToolResult`; a tool with no schema accepts no properties — servers that implement `call_tool/3` by hand validate their own arguments); operation requests before `notifications/initialized` are rejected (`ping` -and `logging/setLevel` excepted); a `tools/call` handler's `{:error, binary}` is returned as an -`isError` `CallToolResult`; duplicate tool names are rejected at compile time; and +excepted); a `tools/call` handler's `{:error, binary}` is returned as an `isError` +`CallToolResult`; duplicate literal tool names are rejected at compile time; and `completion/complete` results are capped at 100 values. ## Specification coverage diff --git a/lib/urchin/server.ex b/lib/urchin/server.ex index efffdfb..9edc968 100644 --- a/lib/urchin/server.ex +++ b/lib/urchin/server.ex @@ -34,9 +34,10 @@ defmodule Urchin.Server do Capabilities are derived automatically from the declared features. - Duplicate tool names declared via the DSL are rejected at compile time. Urchin enforces no - tool-name pattern (the MCP schema imposes none); servers should still follow the MCP naming - recommendations (a conservative charset, a length bound, no whitespace). + Duplicate literal tool names declared via the DSL are rejected at compile time (non-literal + names cannot be compared statically and are not checked). Urchin enforces no tool-name pattern + (the MCP schema imposes none); servers should still follow the MCP naming recommendations (a + conservative charset, a length bound, no whitespace). ## Behaviour diff --git a/lib/urchin/transport/streamable_http.ex b/lib/urchin/transport/streamable_http.ex index ba5a47b..4d7b4eb 100644 --- a/lib/urchin/transport/streamable_http.ex +++ b/lib/urchin/transport/streamable_http.ex @@ -40,8 +40,8 @@ defmodule Urchin.Transport.StreamableHTTP do The transport enforces the spec by default and these behaviors are not configurable: it validates a DSL tool's `tools/call` arguments against its input schema (a hand-written `call_tool/3` validates its own arguments), rejects operation requests received before - `notifications/initialized` (only `ping` and `logging/setLevel` are allowed pre-init), and - surfaces a tool handler's `{:error, binary}` as an `isError` `CallToolResult`. + `notifications/initialized` (only `ping` is allowed pre-init), and surfaces a tool handler's + `{:error, binary}` as an `isError` `CallToolResult`. The plug reads the raw request body itself, so mount it before any JSON body parser. """ From 615e7367cb7b96b08c90df937e4fe86cfc934ecd Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Mon, 8 Jun 2026 15:22:41 +0900 Subject: [PATCH 35/37] test(transport): cover DELETE with an unsupported protocol version Co-Authored-By: Claude Opus 4.8 (1M context) --- test/urchin/transport/streamable_http_test.exs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/urchin/transport/streamable_http_test.exs b/test/urchin/transport/streamable_http_test.exs index a3e4eb5..f50b1b8 100644 --- a/test/urchin/transport/streamable_http_test.exs +++ b/test/urchin/transport/streamable_http_test.exs @@ -278,6 +278,19 @@ defmodule Urchin.Transport.StreamableHTTPTest do assert wait_for_termination(session_id) == :ok end + test "DELETE rejects an unsupported MCP-Protocol-Version" do + {session_id, _} = init_session() + + conn = + conn(:delete, "/") + |> put_req_header("mcp-session-id", session_id) + |> put_req_header("mcp-protocol-version", "1999-01-01") + |> StreamableHTTP.call(@opts) + + assert conn.status == 400 + assert Jason.decode!(conn.resp_body)["error"]["code"] == -32_600 + end + test "DELETE is 405 when disabled" do opts = StreamableHTTP.init(server: EchoServer, allow_delete: false) {session_id, _} = init_session() From 4440526a89c3a56c400d9c1df43ec59a5500b176 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Mon, 8 Jun 2026 16:08:56 +0900 Subject: [PATCH 36/37] feat(dispatcher): validate completion/complete params and result shape Previously only ref and argument were checked for being maps. Validate the CompleteRequestParams shape per spec: ref as a ref/prompt (name) or ref/resource (uri) union, argument.name/value as required strings, and context.arguments values as strings, returning invalid_params when malformed. The result is also checked (values as strings, total a number, hasMore a boolean); a non-conforming result is an internal error rather than a shipped, non-spec response. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/urchin/dispatcher.ex | 88 ++++++++++++++++++++++++++++++--- test/urchin/dispatcher_test.exs | 67 ++++++++++++++++++++++++- 2 files changed, 148 insertions(+), 7 deletions(-) diff --git a/lib/urchin/dispatcher.ex b/lib/urchin/dispatcher.ex index 7ae4830..65cb8d2 100644 --- a/lib/urchin/dispatcher.ex +++ b/lib/urchin/dispatcher.ex @@ -235,9 +235,9 @@ defmodule Urchin.Dispatcher do defp do_handle(server, "completion/complete", params, ctx) do with_callback(server, :complete, 4, fn -> - ref = require_map(params, "ref") - argument = require_map(params, "argument") - completion_context = Map.get(params, "context", %{}) + ref = require_completion_ref(params) + argument = require_completion_argument(params) + completion_context = require_completion_context(params) case server.complete(ref, argument, completion_context, ctx) do {:ok, completion} -> {:ok, %{completion: completion(completion)}} @@ -340,9 +340,17 @@ defmodule Urchin.Dispatcher do ) end - # The completion spec caps values at 100 per response; when a handler returns more, the list is - # truncated to the top 100 (already ranked by relevance) and hasMore is necessarily true. - defp build_completion(values, total, has_more) when is_list(values) do + defp completion(_other) do + raise Error.internal_error("completion result must be a map or a list of values") + end + + # CompleteResult.completion is `{ values: string[], total?: number, hasMore?: boolean }`. A + # malformed result is a server bug, so it surfaces as an internal error rather than shipping a + # non-conforming response. The spec also caps values at 100 per response; when a handler returns + # more, the list is truncated to the top 100 (already ranked) and hasMore is necessarily true. + defp build_completion(values, total, has_more) do + validate_completion_result!(values, total, has_more) + {capped, has_more} = if length(values) > @max_completion_values do {Enum.take(values, @max_completion_values), true} @@ -355,6 +363,22 @@ defmodule Urchin.Dispatcher do |> maybe_put(:hasMore, has_more) end + defp validate_completion_result!(values, total, has_more) do + cond do + not (is_list(values) and Enum.all?(values, &is_binary/1)) -> + raise Error.internal_error("completion values must be a list of strings") + + not (is_nil(total) or is_number(total)) -> + raise Error.internal_error("completion total must be a number") + + not (is_nil(has_more) or is_boolean(has_more)) -> + raise Error.internal_error("completion hasMore must be a boolean") + + true -> + :ok + end + end + # Reads a value that may be keyed by atom or string, preserving false/0 values. defp get_either(map, atom_key, string_key, default) do case Map.fetch(map, atom_key) do @@ -478,6 +502,58 @@ defmodule Urchin.Dispatcher do end end + # CompleteRequestParams.ref is a discriminated union: ref/prompt carries a string name, + # ref/resource carries a string uri. Anything else is a malformed request. + defp require_completion_ref(params) do + ref = require_map(params, "ref") + + case Map.get(ref, "type") do + "ref/prompt" -> _ = require_string(ref, "name") + "ref/resource" -> _ = require_string(ref, "uri") + other -> raise Error.invalid_params("completion ref.type is invalid: #{inspect(other)}") + end + + ref + end + + # CompleteRequestParams.argument is `{ name: string, value: string }`, both required. + defp require_completion_argument(params) do + argument = require_map(params, "argument") + _ = require_string(argument, "name") + _ = require_string(argument, "value") + argument + end + + # CompleteRequestParams.context is optional; its `arguments` map, when present, maps argument + # names to string values. + defp require_completion_context(params) do + case Map.get(params, "context") do + nil -> + %{} + + %{} = context -> + validate_context_arguments!(Map.get(context, "arguments")) + context + + _ -> + raise Error.invalid_params("completion context must be an object") + end + end + + defp validate_context_arguments!(nil), do: :ok + + defp validate_context_arguments!(arguments) when is_map(arguments) do + if Enum.all?(arguments, fn {_k, v} -> is_binary(v) end) do + :ok + else + raise Error.invalid_params("completion context.arguments values must be strings") + end + end + + defp validate_context_arguments!(_other) do + raise Error.invalid_params("completion context.arguments must be an object") + end + # Rescued exceptions are always logged in full but, by default, are not surfaced to the # client. Set the transport's :expose_internal_errors to return the message instead. defp handler_error(exception, stacktrace, ctx, label) do diff --git a/test/urchin/dispatcher_test.exs b/test/urchin/dispatcher_test.exs index 95ddaf3..132eb03 100644 --- a/test/urchin/dispatcher_test.exs +++ b/test/urchin/dispatcher_test.exs @@ -52,13 +52,24 @@ defmodule Urchin.DispatcherTest.BigCompletionServer do end end +defmodule Urchin.DispatcherTest.BadCompletionServer do + @moduledoc false + # Returns a non-conforming completion result (values are not strings). + use Urchin.Server, name: "bad-completion", version: "1.0.0", completions: true + + @impl true + def complete(_ref, _argument, _context, _ctx) do + {:ok, %{values: [1, 2, 3]}} + end +end + defmodule Urchin.DispatcherTest do use ExUnit.Case, async: true alias Urchin.{Context, Dispatcher, Session} alias Urchin.Test.EchoServer alias Urchin.DispatcherTest.{LoggingServer, NoLoggingServer, FailingLoggingServer} - alias Urchin.DispatcherTest.{BadInfoServer, BigCompletionServer} + alias Urchin.DispatcherTest.{BadInfoServer, BigCompletionServer, BadCompletionServer} # The default context represents an initialized session; the lifecycle gate is exercised # explicitly in the "initialized gating" describe with initialized: false. @@ -371,6 +382,60 @@ defmodule Urchin.DispatcherTest do assert completion.hasMore == true end + test "rejects an argument without a string name and value" do + params = %{ + "ref" => %{"type" => "ref/prompt", "name" => "greet"}, + "argument" => %{"name" => "name"} + } + + assert {:error, error} = + Dispatcher.handle_request(EchoServer, "completion/complete", params, ctx()) + + assert error.code == -32_602 + end + + test "rejects an unknown ref type" do + params = %{ + "ref" => %{"type" => "ref/bogus"}, + "argument" => %{"name" => "name", "value" => "Sa"} + } + + assert {:error, error} = + Dispatcher.handle_request(EchoServer, "completion/complete", params, ctx()) + + assert error.code == -32_602 + end + + test "rejects non-string context.arguments values" do + params = %{ + "ref" => %{"type" => "ref/prompt", "name" => "greet"}, + "argument" => %{"name" => "name", "value" => "Sa"}, + "context" => %{"arguments" => %{"prior" => 1}} + } + + assert {:error, error} = + Dispatcher.handle_request(EchoServer, "completion/complete", params, ctx()) + + assert error.code == -32_602 + end + + test "a non-conforming completion result is an internal error" do + params = %{ + "ref" => %{"type" => "ref/prompt", "name" => "x"}, + "argument" => %{"name" => "n", "value" => "v"} + } + + assert {:error, error} = + Dispatcher.handle_request( + BadCompletionServer, + "completion/complete", + params, + ctx() + ) + + assert error.code == -32_603 + end + test "ping" do assert {:ok, %{}} = Dispatcher.handle_request(EchoServer, "ping", %{}, ctx()) end From a21eb3f26708c1d683f62aaa9b216380421d9e25 Mon Sep 17 00:00:00 2001 From: Akira Ueno Date: Mon, 8 Jun 2026 16:08:56 +0900 Subject: [PATCH 37/37] feat(tool): require object input and output schemas Tool.new/1 kept input_schema/output_schema as given, so a non-object schema (e.g. type: array, or a non-map) could be advertised in tools/list and skew the now-always-on argument validation. Validate that each, when present, is a JSON Schema object with root type "object", per the MCP tools spec; the DSL rejects a non-conforming schema at compile time. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 9 +++++++-- lib/urchin/tool.ex | 25 +++++++++++++++++++++++-- test/urchin/core_test.exs | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6de2994..bc9652b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,8 +70,13 @@ breaking relative to `0.2.0`. error rather than a silently-defaulted value. The server's `serverInfo` must likewise carry a string `name` and `version`. - The `MCP-Protocol-Version` header is validated on `DELETE`, matching `POST` and `GET`. -- `completion/complete` results are capped at 100 values; a handler returning more is truncated - to the top 100 (already ranked by relevance) with `hasMore` set. +- `completion/complete` request params are validated (`ref` as a `ref/prompt`/`ref/resource` + union, `argument.name`/`value` as strings, `context.arguments` values as strings) and return + `invalid_params` when malformed. Results are capped at 100 values — a handler returning more is + truncated to the top 100 (already ranked by relevance) with `hasMore` set — and a + non-conforming result shape (non-string `values`, etc.) is an internal error. +- A tool's `input_schema` and `output_schema` must be JSON Schema objects whose root `type` is + `"object"` (per the MCP tools spec); the DSL rejects a non-conforming schema at compile time. - `logging/setLevel` is now a library builtin: when the server advertises the `logging` capability (via `use Urchin.Server, logging: true`) it succeeds and applies the level to the session even without a `set_log_level/2` callback. The level is validated against the MCP log diff --git a/lib/urchin/tool.ex b/lib/urchin/tool.ex index 430868a..ea47149 100644 --- a/lib/urchin/tool.ex +++ b/lib/urchin/tool.ex @@ -43,8 +43,8 @@ defmodule Urchin.Tool do name: fetch_name!(attrs), title: attrs[:title], description: attrs[:description], - input_schema: attrs[:input_schema], - output_schema: attrs[:output_schema], + input_schema: validate_object_schema!(attrs[:input_schema], :input_schema), + output_schema: validate_object_schema!(attrs[:output_schema], :output_schema), annotations: attrs[:annotations], execution: attrs[:execution], icons: attrs[:icons], @@ -55,6 +55,27 @@ defmodule Urchin.Tool do defp fetch_name!(%{name: name}) when is_binary(name), do: name defp fetch_name!(_), do: raise(ArgumentError, "tool requires a string :name") + # inputSchema and outputSchema are JSON Schema objects whose root `type` is "object" (MCP tools + # spec). nil is allowed: input_schema falls back to default_input_schema/0 and output_schema is + # optional. A non-object schema would advertise a non-conforming tools/list entry. + defp validate_object_schema!(nil, _field), do: nil + + defp validate_object_schema!(schema, field) when is_map(schema) do + case schema["type"] || schema[:type] do + "object" -> + schema + + other -> + raise ArgumentError, + ~s(tool #{field} must be a JSON Schema object with "type": "object", got type: #{inspect(other)}) + end + end + + defp validate_object_schema!(other, field) do + raise ArgumentError, + "tool #{field} must be a map (a JSON Schema object), got: #{inspect(other)}" + end + @doc """ The input schema advertised for a tool that declares none: an object accepting no properties, per the MCP recommendation for parameterless tools. diff --git a/test/urchin/core_test.exs b/test/urchin/core_test.exs index 8e3d69a..54fd540 100644 --- a/test/urchin/core_test.exs +++ b/test/urchin/core_test.exs @@ -124,6 +124,38 @@ defmodule Urchin.CoreTest do end end + describe "Urchin.Tool schema validation" do + test "accepts an object input and output schema" do + tool = + Tool.new( + name: "t", + input_schema: %{"type" => "object", "properties" => %{}}, + output_schema: %{"type" => "object"} + ) + + assert tool.input_schema["type"] == "object" + assert tool.output_schema["type"] == "object" + end + + test "rejects an input schema whose root type is not object" do + assert_raise ArgumentError, ~r/input_schema.*type.*object/, fn -> + Tool.new(name: "t", input_schema: %{"type" => "array"}) + end + end + + test "rejects a non-map input schema" do + assert_raise ArgumentError, ~r/input_schema must be a map/, fn -> + Tool.new(name: "t", input_schema: "nope") + end + end + + test "rejects an output schema whose root type is not object" do + assert_raise ArgumentError, ~r/output_schema.*type.*object/, fn -> + Tool.new(name: "t", output_schema: %{"type" => "string"}) + end + end + end + describe "Urchin.URITemplate" do test "matches a single-segment variable" do assert {:ok, %{"name" => "world"}} =