Skip to content

refactor(phase-1): abstract protocol version negotiation - #93

Merged
zoedsoupe merged 2 commits into
mainfrom
refactor/phase-1-protocol-version-layer
Feb 28, 2026
Merged

zoedsoupe merged 2 commits into
mainfrom
refactor/phase-1-protocol-version-layer

Conversation

@zoedsoupe

@zoedsoupe zoedsoupe commented Feb 28, 2026 •

Copy link
Copy Markdown
Owner

Problem

The codebase has version-specific protocol logic scattered across multiple modules (Anubis.Protocol,
Anubis.MCP.Message, Anubis.Server.Base, Anubis.Server). Adding support for a new MCP spec version means touching many
places, and the server macro module had incorrect hardcoded version strings (2024-05-11, 2024-10-07) that don't exist
in the supported versions list. This is Phase 1 of the architecture refactor plan addressing issues hermes#179 and
hermes#141.

Solution

  • Created Anubis.Protocol.Behaviour defining callbacks each version module must implement (version, features, schemas,
    methods)
  • Created per-version modules (V2024_11_05, V2025_03_26, V2025_06_18) under lib/anubis/protocol/ encoding
    version-specific schemas, features, and method lists
  • Created Anubis.Protocol.Registry as the central dispatch point mapping version strings to modules, with negotiation
    support
  • Refactored Anubis.Protocol to delegate to the Registry while preserving its entire public API (zero breaking
    changes)
  • Updated Server.Base to use Registry.negotiate/2 instead of a private negotiation function
  • Fixed Anubis.Server macro to derive @protocol_versions from the Registry instead of a hardcoded (incorrect) list
  • Stored the negotiated protocol module in Session, Client.State, and Frame.private for downstream use in later phases
  • Added 64 new tests covering the Registry, version modules, behaviour compliance, feature inheritance, and backward
    compatibility

Rationale

Version modules inherit from their predecessor (V2025_03_26 delegates to V2024_11_05 for unchanged schemas) to avoid
duplication while making differences explicit. The Registry is a compile-time map (not a GenServer) since version data
is static. Anubis.Protocol's public API was preserved via defdelegate and thin wrappers so this is a purely internal
refactor — no downstream code changes required. Storing the resolved protocol module in session/connection state
enables later phases to dispatch version-specific logic without re-resolving.

Summary by CodeRabbit

Release Notes

  • New Features
    • Added multi-version MCP protocol support with dynamic version negotiation and fallback handling.
    • Introduced version-aware feature discovery to identify capabilities across supported protocol versions.
    • Enabled automatic protocol module resolution during client initialization.

@coderabbitai

coderabbitai Bot commented Feb 28, 2026 •

Copy link
Copy Markdown

Warning

Rate limit exceeded

@zoedsoupe has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 22 minutes and 49 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between d9b6ff1 and 1a8f477.

📒 Files selected for processing (1)
  • lib/anubis/server/session.ex

Walkthrough

This pull request introduces a protocol registry pattern to centralize MCP protocol version management. A new Registry module serves as the single source of truth for supported versions (2024-11-05, 2025-03-26, 2025-06-18), replacing hard-coded constants scattered throughout Anubis.Protocol. A Behaviour module standardizes how each version module implements required callbacks. Anubis.Protocol delegates to Registry for version lookups, feature checks, and negotiation. The protocol module is threaded through Client.State initialization and Server session/frame setup to enable downstream access to version-specific logic. Server.Base uses Registry.negotiate to determine both version and implementation module during protocol handshake.

Sequence Diagram

sequenceDiagram
    participant Client as Client
    participant Server as Server.Base
    participant Registry as Protocol.Registry
    participant VersionMod as Version Module<br/>(V2024_11_05, etc.)
    participant Session as Server.Session
    
    Client->>Server: negotiate protocol version
    Server->>Registry: negotiate(client_version, server_versions)
    Registry->>Registry: check supported?(version)
    Registry-->>Server: {:ok, version, module}
    Server->>Session: update_from_initialization(..., protocol_module: module)
    Session->>Session: store protocol_module in state
    Server->>VersionMod: call version-specific behavior
    VersionMod-->>Server: supported_features, request_params_schema, etc.
    Server-->>Client: negotiation complete + capabilities
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Rationale: High heterogeneity across 11+ files with interdependencies (Registry → Version modules → Protocol delegations → Client/Server state threading). New architectural pattern requires verifying Registry correctness, Behaviour compliance across three version modules with inheritance chains (2025-03-26 extends 2024-11-05; 2025-06-18 extends 2025-03-26), schema delegation accuracy, and proper protocol_module propagation through initialization paths. Logic density is moderate but spread across multiple integration points. Comprehensive test coverage mitigates some review burden.

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 62.30% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed Title accurately summarizes the main refactoring effort: abstracting protocol version negotiation into a centralized Registry and Behaviour pattern.
Description check ✅ Passed Description comprehensively covers Problem, Solution, and Rationale sections with specific details about what was refactored, why, and the architectural trade-offs made.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/phase-1-protocol-version-layer

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/anubis/mcp/message.ex`:
- Around line 642-647: The doctest examples for
Message.progress_params_schema_for show raw maps but the function actually
delegates to Registry.progress_params_schema/1 and returns {:ok, map()} |
:error; update the examples in the moduledoc for progress_params_schema_for to
show the actual tuple return shapes (e.g., {:ok, %{"progressToken" => ...}} or
:error) so the examples match the function's real return value and behavior.

In `@lib/anubis/protocol/registry.ex`:
- Around line 20-28: The module currently hardcodes `@latest_version` and
`@fallback_version` separate from `@versions` causing drift; instead compute them at
compile time from `@versions` so they stay in sync: derive a sorted list from
Map.keys(`@versions`) (e.g. Enum.sort/1 then reverse), assign `@supported_versions`
from that list and set `@latest_version` to the first element and
`@fallback_version` to the second (or a sensible default if missing), replacing
the manual `@latest_version` and `@fallback_version` entries; reference the existing
`@versions`, `@supported_versions`, `@latest_version` and `@fallback_version` attributes
when making the change.
- Around line 115-128: negotiate/2 currently crashes on an empty server_versions
list and only uses the head as a fallback; change negotiate to first handle the
empty list (return :error) and otherwise compute the fallback as the
maximum/latest value from server_versions (e.g. Enum.max(server_versions))
instead of using the head, then keep the selection logic (use client_version if
present in server_versions, otherwise use that computed latest) and call
get(version) as before to return {:ok, version, mod} or :error; reference
negotiate/2, client_version, server_versions, latest and get/1 when making the
change.

In `@lib/anubis/server/base.ex`:
- Around line 412-422: Replace the unsafe pattern match on
Anubis.Protocol.Registry.negotiate/2 with explicit handling for both {:ok,
protocol_version, protocol_module} and :error (or {:error, reason}) so the
server doesn't crash during handshake; call Session.update_from_initialization
only on the {:ok, ...} branch and on :error log the failure (including reason),
perform any handshake-failure response/cleanup and return an error tuple instead
of letting it raise. Reference: Anubis.Protocol.Registry.negotiate/2 and
Session.update_from_initialization.

In `@lib/anubis/server/frame.ex`:
- Around line 372-375: The `@type` private_t is missing the newly used
:protocol_module field referenced by get_protocol_module/1; update the private_t
type definition to include protocol_module :: module() | nil (or the exact
intended type) so the type contract stays in sync with the
Map.get(frame.private, :protocol_module) usage in get_protocol_module/1.

In `@test/anubis/protocol_test.exs`:
- Line 2: Replace the test case module's base so it uses the MCP test harness:
change the top line that currently says "use ExUnit.Case, async: true" to use
"Anubis.MCP.Case" (preserving any supported options like async if applicable) so
the protocol tests leverage the Anubis.MCP.Case builders, setup helpers, and
domain assertions instead of plain ExUnit.Case.

In `@test/anubis/protocol/registry_test.exs`:
- Line 2: Replace the top-level test harness from ExUnit.Case to the project
standard Anubis.MCP.Case: change the `use ExUnit.Case, async: true` invocation
to `use Anubis.MCP.Case` (retain `async: true` only if Anubis.MCP.Case supports
it), and run the suite to ensure any MCP-specific setup/hooks or helpers used by
`Anubis.MCP.Case` are available for the registry tests; update any imports or
setup blocks in this file that assumed ExUnit-only helpers to the MCP
equivalents if tests fail.

In `@test/anubis/protocol/version_modules_test.exs`:
- Line 2: Replace the ExUnit test base with the project's MCP test base: change
the test module's "use ExUnit.Case, async: true" to "use Anubis.MCP.Case, async:
true" so the suite uses MCP helpers and domain assertions; ensure any
ExUnit-specific imports or setup are compatible with Anubis.MCP.Case and remove
or adapt conflicting mocks/helpers if present.

ℹ️ Review info

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5a61c65 and d9b6ff1.

📒 Files selected for processing (15)
  • lib/anubis/client/state.ex
  • lib/anubis/mcp/message.ex
  • lib/anubis/protocol.ex
  • lib/anubis/protocol/behaviour.ex
  • lib/anubis/protocol/registry.ex
  • lib/anubis/protocol/v2024_11_05.ex
  • lib/anubis/protocol/v2025_03_26.ex
  • lib/anubis/protocol/v2025_06_18.ex
  • lib/anubis/server.ex
  • lib/anubis/server/base.ex
  • lib/anubis/server/frame.ex
  • lib/anubis/server/session.ex
  • test/anubis/protocol/registry_test.exs
  • test/anubis/protocol/version_modules_test.exs
  • test/anubis/protocol_test.exs

Comment thread lib/anubis/mcp/message.ex
Comment on lines +642 to +647
iex> Message.progress_params_schema_for("2024-11-05")
%{"progressToken" => {:required, {:either, {:string, :integer}}}, ...}

iex> Message.progress_params_schema_for("2025-03-26")
%{"progressToken" => ..., "message" => :string}
"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

P3 — Doctest examples don’t match actual return shape

Line 648 declares {:ok, map()} | :error, and Line 650 delegates to Registry.progress_params_schema/1 (which returns tuples), but Line 642 and Line 645 examples show raw maps.

📝 Suggested doc fix
-      iex> Message.progress_params_schema_for("2024-11-05")
-      %{"progressToken" => {:required, {:either, {:string, :integer}}}, ...}
+      iex> Message.progress_params_schema_for("2024-11-05")
+      {:ok, %{"progressToken" => {:required, {:either, {:string, :integer}}}, ...}}
@@
-      iex> Message.progress_params_schema_for("2025-03-26")
-      %{"progressToken" => ..., "message" => :string}
+      iex> Message.progress_params_schema_for("2025-03-26")
+      {:ok, %{"progressToken" => ..., "message" => :string}}

Also applies to: 648-651

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/anubis/mcp/message.ex` around lines 642 - 647, The doctest examples for
Message.progress_params_schema_for show raw maps but the function actually
delegates to Registry.progress_params_schema/1 and returns {:ok, map()} |
:error; update the examples in the moduledoc for progress_params_schema_for to
show the actual tuple return shapes (e.g., {:ok, %{"progressToken" => ...}} or
:error) so the examples match the function's real return value and behavior.

Comment on lines +20 to +28
@versions %{
"2024-11-05" => Anubis.Protocol.V2024_11_05,
"2025-03-26" => Anubis.Protocol.V2025_03_26,
"2025-06-18" => Anubis.Protocol.V2025_06_18
}

@latest_version "2025-06-18"
@fallback_version "2025-03-26"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

P2 — Avoid metadata drift between @versions and “latest/fallback” constants

Line 26 and Line 27 are manually maintained while Line 50 computes supported versions dynamically. One missed update later and negotiation metadata silently lies (future-you won’t enjoy that debugging session).

🔧 Suggested compile-time sync
   `@versions` %{
     "2024-11-05" => Anubis.Protocol.V2024_11_05,
     "2025-03-26" => Anubis.Protocol.V2025_03_26,
     "2025-06-18" => Anubis.Protocol.V2025_06_18
   }
 
-  `@latest_version` "2025-06-18"
+  `@supported_versions` `@versions` |> Map.keys() |> Enum.sort(:desc)
+  `@latest_version` hd(`@supported_versions`)
   `@fallback_version` "2025-03-26"
+
+  unless Map.has_key?(`@versions`, `@fallback_version`) do
+    raise ArgumentError, "@fallback_version must exist in `@versions`"
+  end
@@
-  def supported_versions do
-    `@versions` |> Map.keys() |> Enum.sort(:desc)
-  end
+  def supported_versions, do: `@supported_versions`

Also applies to: 49-51

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/anubis/protocol/registry.ex` around lines 20 - 28, The module currently
hardcodes `@latest_version` and `@fallback_version` separate from `@versions` causing
drift; instead compute them at compile time from `@versions` so they stay in sync:
derive a sorted list from Map.keys(`@versions`) (e.g. Enum.sort/1 then reverse),
assign `@supported_versions` from that list and set `@latest_version` to the first
element and `@fallback_version` to the second (or a sensible default if missing),
replacing the manual `@latest_version` and `@fallback_version` entries; reference
the existing `@versions`, `@supported_versions`, `@latest_version` and
`@fallback_version` attributes when making the change.

Comment on lines +115 to +128
@spec negotiate(version(), [version()]) :: {:ok, version(), module()} | :error
def negotiate(client_version, [latest | _] = server_versions) do
version =
if client_version in server_versions do
client_version
else
latest
end

case get(version) do
{:ok, mod} -> {:ok, version, mod}
:error -> :error
end
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

P1 — negotiate/2 has a trapdoor: empty list crash + fragile fallback selection

Line 116 crashes on [] (FunctionClauseError), and Line 121/Line 124 can return :error even when server_versions contains a later supported version (because only the head is used as fallback). This makes negotiation brittle under imperfect input.

💡 Suggested hardening patch
+  def negotiate(_client_version, []), do: :error
+
-  def negotiate(client_version, [latest | _] = server_versions) do
-    version =
-      if client_version in server_versions do
-        client_version
-      else
-        latest
-      end
+  def negotiate(client_version, server_versions) when is_list(server_versions) do
+    version =
+      cond do
+        client_version in server_versions and supported?(client_version) ->
+          client_version
+
+        true ->
+          Enum.find(server_versions, &supported?/1)
+      end
 
-    case get(version) do
+    case version && get(version) do
       {:ok, mod} -> {:ok, version, mod}
       :error -> :error
+      _ -> :error
     end
   end
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/anubis/protocol/registry.ex` around lines 115 - 128, negotiate/2
currently crashes on an empty server_versions list and only uses the head as a
fallback; change negotiate to first handle the empty list (return :error) and
otherwise compute the fallback as the maximum/latest value from server_versions
(e.g. Enum.max(server_versions)) instead of using the head, then keep the
selection logic (use client_version if present in server_versions, otherwise use
that computed latest) and call get(version) as before to return {:ok, version,
mod} or :error; reference negotiate/2, client_version, server_versions, latest
and get/1 when making the change.

Comment thread lib/anubis/server/base.ex
Comment on lines +412 to 422
{:ok, protocol_version, protocol_module} =
Anubis.Protocol.Registry.negotiate(requested_version, state.supported_versions)

:ok =
Session.update_from_initialization(
session.name,
protocol_version,
client_info,
client_capabilities
client_capabilities,
protocol_module: protocol_module
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

P1 – Handle negotiation failure explicitly (avoid initialize-time faceplant)

Line 412 assumes Registry.negotiate/2 always returns {:ok, ...}. It can return :error, which would raise and crash the server during handshake.

🛠️ Suggested resilient flow
-    {:ok, protocol_version, protocol_module} =
-      Anubis.Protocol.Registry.negotiate(requested_version, state.supported_versions)
-
-    :ok =
-      Session.update_from_initialization(
-        session.name,
-        protocol_version,
-        client_info,
-        client_capabilities,
-        protocol_module: protocol_module
-      )
-
-    result = %{
-      "protocolVersion" => protocol_version,
-      "serverInfo" => state.server_info,
-      "capabilities" => state.capabilities
-    }
-
-    Logging.server_event("initializing", %{
-      client_info: params["clientInfo"],
-      client_capabilities: params["capabilities"],
-      protocol_version: protocol_version
-    })
-
-    Telemetry.execute(
-      Telemetry.event_server_response(),
-      %{system_time: System.system_time()},
-      %{method: "initialize", status: :success}
-    )
-
-    {:reply, {:ok, Message.build_response(result, request["id"])}, state}
+    case Anubis.Protocol.Registry.negotiate(requested_version, state.supported_versions) do
+      {:ok, protocol_version, protocol_module} ->
+        :ok =
+          Session.update_from_initialization(
+            session.name,
+            protocol_version,
+            client_info,
+            client_capabilities,
+            protocol_module: protocol_module
+          )
+
+        result = %{
+          "protocolVersion" => protocol_version,
+          "serverInfo" => state.server_info,
+          "capabilities" => state.capabilities
+        }
+
+        Logging.server_event("initializing", %{
+          client_info: params["clientInfo"],
+          client_capabilities: params["capabilities"],
+          protocol_version: protocol_version
+        })
+
+        Telemetry.execute(
+          Telemetry.event_server_response(),
+          %{system_time: System.system_time()},
+          %{method: "initialize", status: :success}
+        )
+
+        {:reply, {:ok, Message.build_response(result, request["id"])}, state}
+
+      :error ->
+        error =
+          Error.protocol(:invalid_params, %{
+            protocolVersion: requested_version,
+            supported: state.supported_versions
+          })
+
+        {:reply, {:ok, Error.build_json_rpc(error, request["id"])}, state}
+    end

As per coding guidelines "Pattern match with {:ok, _} and {:error, reason} for error handling".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/anubis/server/base.ex` around lines 412 - 422, Replace the unsafe pattern
match on Anubis.Protocol.Registry.negotiate/2 with explicit handling for both
{:ok, protocol_version, protocol_module} and :error (or {:error, reason}) so the
server doesn't crash during handshake; call Session.update_from_initialization
only on the {:ok, ...} branch and on :error log the failure (including reason),
perform any handshake-failure response/cleanup and return an error tuple instead
of letting it raise. Reference: Anubis.Protocol.Registry.negotiate/2 and
Session.update_from_initialization.

Comment on lines +372 to +375
@spec get_protocol_module(t) :: module() | nil
def get_protocol_module(%__MODULE__{} = frame) do
Map.get(frame.private, :protocol_module)
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

P3 – Keep private_t in sync with the new :protocol_module field

Line 374 reads :protocol_module, but @type private_t doesn’t declare it. Tiny drift now, bigger confusion later.

🧩 Suggested contract update
 `@type` private_t :: %{
           optional(:session_id) => String.t(),
           optional(:client_info) => map(),
           optional(:client_capabilities) => map(),
           optional(:protocol_version) => String.t(),
+          optional(:protocol_module) => module(),
           optional(:server_module) => module(),
           optional(:server_registry) => module(),
           optional(:pagination_limit) => non_neg_integer(),
           optional(:__mcp_components__) => list(server_component_t)
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@spec get_protocol_module(t) :: module() | nil
def get_protocol_module(%__MODULE__{} = frame) do
Map.get(frame.private, :protocol_module)
end
`@type` private_t :: %{
optional(:session_id) => String.t(),
optional(:client_info) => map(),
optional(:client_capabilities) => map(),
optional(:protocol_version) => String.t(),
optional(:protocol_module) => module(),
optional(:server_module) => module(),
optional(:server_registry) => module(),
optional(:pagination_limit) => non_neg_integer(),
optional(:__mcp_components__) => list(server_component_t)
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/anubis/server/frame.ex` around lines 372 - 375, The `@type` private_t is
missing the newly used :protocol_module field referenced by
get_protocol_module/1; update the private_t type definition to include
protocol_module :: module() | nil (or the exact intended type) so the type
contract stays in sync with the Map.get(frame.private, :protocol_module) usage
in get_protocol_module/1.

@@ -0,0 +1,73 @@
defmodule Anubis.ProtocolTest do
use ExUnit.Case, async: true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

P2 – Apply the MCP test harness here too

Line 2 should use Anubis.MCP.Case rather than ExUnit.Case to keep protocol tests aligned with repo test conventions.

🔧 Minimal patch
-  use ExUnit.Case, async: true
+  use Anubis.MCP.Case, async: true

As per coding guidelines "Use Anubis.MCP.Case for comprehensive MCP protocol testing with builders, setup functions, helpers, and domain-specific assertions instead of mocking libraries".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
use ExUnit.Case, async: true
use Anubis.MCP.Case, async: true
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/anubis/protocol_test.exs` at line 2, Replace the test case module's base
so it uses the MCP test harness: change the top line that currently says "use
ExUnit.Case, async: true" to use "Anubis.MCP.Case" (preserving any supported
options like async if applicable) so the protocol tests leverage the
Anubis.MCP.Case builders, setup helpers, and domain assertions instead of plain
ExUnit.Case.

@@ -0,0 +1,149 @@
defmodule Anubis.Protocol.RegistryTest do
use ExUnit.Case, async: true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

P2 – Standardize this suite on Anubis.MCP.Case

Line 2 currently uses ExUnit.Case; for MCP protocol/registry tests, the project standard is Anubis.MCP.Case.

🔧 Minimal patch
-  use ExUnit.Case, async: true
+  use Anubis.MCP.Case, async: true

As per coding guidelines "Use Anubis.MCP.Case for comprehensive MCP protocol testing with builders, setup functions, helpers, and domain-specific assertions instead of mocking libraries".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
use ExUnit.Case, async: true
use Anubis.MCP.Case, async: true
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/anubis/protocol/registry_test.exs` at line 2, Replace the top-level test
harness from ExUnit.Case to the project standard Anubis.MCP.Case: change the
`use ExUnit.Case, async: true` invocation to `use Anubis.MCP.Case` (retain
`async: true` only if Anubis.MCP.Case supports it), and run the suite to ensure
any MCP-specific setup/hooks or helpers used by `Anubis.MCP.Case` are available
for the registry tests; update any imports or setup blocks in this file that
assumed ExUnit-only helpers to the MCP equivalents if tests fail.

@@ -0,0 +1,202 @@
defmodule Anubis.Protocol.VersionModulesTest do
use ExUnit.Case, async: true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

P2 – Use Anubis.MCP.Case for protocol test suites

Line 2 uses ExUnit.Case; these MCP protocol tests should use the project’s MCP test base for consistency across helpers/assertions.

🔧 Minimal patch
-  use ExUnit.Case, async: true
+  use Anubis.MCP.Case, async: true

As per coding guidelines "Use Anubis.MCP.Case for comprehensive MCP protocol testing with builders, setup functions, helpers, and domain-specific assertions instead of mocking libraries".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
use ExUnit.Case, async: true
use Anubis.MCP.Case, async: true
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/anubis/protocol/version_modules_test.exs` at line 2, Replace the ExUnit
test base with the project's MCP test base: change the test module's "use
ExUnit.Case, async: true" to "use Anubis.MCP.Case, async: true" so the suite
uses MCP helpers and domain assertions; ensure any ExUnit-specific imports or
setup are compatible with Anubis.MCP.Case and remove or adapt conflicting
mocks/helpers if present.

@zoedsoupe

Copy link
Copy Markdown
Owner Author

i think those are acceptable for now

@zoedsoupe
zoedsoupe merged commit 05a2362 into main Feb 28, 2026
10 checks passed
@zoedsoupe
zoedsoupe deleted the refactor/phase-1-protocol-version-layer branch February 28, 2026 19:06
@zoedsoupe zoedsoupe mentioned this pull request Feb 28, 2026
zoedsoupe added a commit that referenced this pull request Feb 28, 2026
🚀 Want to release this?
---


##
[0.17.1](v0.17.0...v0.17.1)
(2026-02-28)


### Bug Fixes

* Check Process.alive? before sending to SSE handler
([#82](#82))
([e1dc705](e1dc705))


### Code Refactoring

* **phase-1:** abstract protocol version negotiation
([#93](#93))
([05a2362](05a2362))
* **phase-2:** transport layer as functions, backward compatible
([#95](#95))
([105d6a9](105d6a9))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
zoedsoupe added a commit that referenced this pull request Jul 16, 2026
## Problem

The codebase has version-specific protocol logic scattered across
multiple modules (Anubis.Protocol,
Anubis.MCP.Message, Anubis.Server.Base, Anubis.Server). Adding support
for a new MCP spec version means touching many
places, and the server macro module had incorrect hardcoded version
strings (2024-05-11, 2024-10-07) that don't exist
in the supported versions list. This is Phase 1 of the architecture
refactor plan addressing issues hermes#179 and
hermes#141.

## Solution

- Created Anubis.Protocol.Behaviour defining callbacks each version
module must implement (version, features, schemas,
 methods)
- Created per-version modules (V2024_11_05, V2025_03_26, V2025_06_18)
under lib/anubis/protocol/ encoding
version-specific schemas, features, and method lists
- Created Anubis.Protocol.Registry as the central dispatch point mapping
version strings to modules, with negotiation
support
- Refactored Anubis.Protocol to delegate to the Registry while
preserving its entire public API (zero breaking
changes)
- Updated Server.Base to use Registry.negotiate/2 instead of a private
negotiation function
- Fixed Anubis.Server macro to derive @protocol_versions from the
Registry instead of a hardcoded (incorrect) list
- Stored the negotiated protocol module in Session, Client.State, and
Frame.private for downstream use in later phases
- Added 64 new tests covering the Registry, version modules, behaviour
compliance, feature inheritance, and backward
compatibility

## Rationale

Version modules inherit from their predecessor (V2025_03_26 delegates to
V2024_11_05 for unchanged schemas) to avoid
duplication while making differences explicit. The Registry is a
compile-time map (not a GenServer) since version data
is static. Anubis.Protocol's public API was preserved via defdelegate
and thin wrappers so this is a purely internal
refactor — no downstream code changes required. Storing the resolved
protocol module in session/connection state
enables later phases to dispatch version-specific logic without
re-resolving.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

## Release Notes

* **New Features**
* Added multi-version MCP protocol support with dynamic version
negotiation and fallback handling.
* Introduced version-aware feature discovery to identify capabilities
across supported protocol versions.
* Enabled automatic protocol module resolution during client
initialization.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
zoedsoupe added a commit that referenced this pull request Jul 16, 2026
🚀 Want to release this?
---


##
[0.17.1](v0.17.0...v0.17.1)
(2026-02-28)


### Bug Fixes

* Check Process.alive? before sending to SSE handler
([#82](#82))
([6887fe4](6887fe4))


### Code Refactoring

* **phase-1:** abstract protocol version negotiation
([#93](#93))
([f76ee3d](f76ee3d))
* **phase-2:** transport layer as functions, backward compatible
([#95](#95))
([7c45279](7c45279))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
This was referenced Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant