diff --git a/config/test.exs b/config/test.exs index 45cd3bd..09a2408 100644 --- a/config/test.exs +++ b/config/test.exs @@ -49,3 +49,8 @@ config :mcp_registry, :discovery, config :mcp_registry, :official_registry, page_delay_ms: 0, req_options: [plug: {Req.Test, McpRegistry.OfficialRegistry}, retry_delay: 0, max_retries: 2] + +# Off under test: the cache table is global while async tests each hold their +# own database sandbox, so one test can be served a value computed from rows +# another test can see and it cannot. See McpRegistry.Cache. +config :mcp_registry, :cache_enabled, false diff --git a/lib/mcp_registry/cache.ex b/lib/mcp_registry/cache.ex index 9f16e87..fd8c0b1 100644 --- a/lib/mcp_registry/cache.ex +++ b/lib/mcp_registry/cache.ex @@ -15,6 +15,14 @@ defmodule McpRegistry.Cache do Reads go straight to ETS with no GenServer in the path, so a hot page does not serialise behind a single process. The GenServer exists only to own the table so it survives the caller. + + ## Off under test + + The table is global while `async: true` tests each hold their own database + sandbox, so a value one test computed can be handed to another that cannot + see the rows behind it. That is a flaky failure with no bearing on the code + under test, and caching a count nobody is measuring buys nothing, so + `config/test.exs` turns it off. """ use GenServer @@ -34,6 +42,14 @@ defmodule McpRegistry.Cache do must never be the reason something fails. """ def fetch(key, ttl \\ nil, fun) when is_function(fun, 0) do + if Application.get_env(:mcp_registry, :cache_enabled, true) do + cached(key, ttl, fun) + else + fun.() + end + end + + defp cached(key, ttl, fun) do ttl = ttl || day() now = System.monotonic_time(:millisecond) diff --git a/lib/mcp_registry/registry/capability.ex b/lib/mcp_registry/registry/capability.ex index b47e3e7..e571b4c 100644 --- a/lib/mcp_registry/registry/capability.ex +++ b/lib/mcp_registry/registry/capability.ex @@ -33,9 +33,17 @@ defmodule McpRegistry.Registry.Capability do slug and cost length. Other schemes stay: `file`, `ui` and `s3` distinguish resources that would otherwise read alike. - Two names can in principle collide — `review_diff` and `review-diff`, or - the same URL under both http and https. `find/2` then answers with the - first, which is a worse page than it could be but never a wrong one. + A name with no Latin letters or digits at all — 192 of the catalogue's + 21,837 prompts and resources are written in scripts that have none — would + otherwise reduce to nothing, and every such name on a server would collide + on the same empty slug with only the first reachable. Those fall back to a + short hash of the name, which is an unlovely URL but a working one, and the + page still shows the name as its publisher wrote it. + + Two names that both survive can still in principle collide — `review_diff` + and `review-diff`, or the same URL under http and https. `find/2` then + answers with the first, which is a worse page than it could be but never a + wrong one. """ def slug(name) when is_binary(name) do name @@ -45,11 +53,21 @@ defmodule McpRegistry.Registry.Capability do |> String.trim("-") |> String.slice(0, 120) |> case do - "" -> "item" + "" -> "item-" <> hash(name) slug -> slug end end + # Stable across runs and processes: `:erlang.phash2/1` is specified to be + # portable, unlike `:erlang.phash/2`, so a URL minted today still resolves + # tomorrow. Base 36 keeps it short. + defp hash(name) do + name + |> :erlang.phash2() + |> Integer.to_string(36) + |> String.downcase() + end + @doc """ Finds the entry whose slug matches, or `nil`. diff --git a/test/mcp_registry_web/live/capabilities_test.exs b/test/mcp_registry_web/live/capabilities_test.exs index 4f56f1f..6706049 100644 --- a/test/mcp_registry_web/live/capabilities_test.exs +++ b/test/mcp_registry_web/live/capabilities_test.exs @@ -31,6 +31,18 @@ defmodule McpRegistryWeb.ServerLive.CapabilitiesTest do assert Capability.slug("ui://widget/x") == "ui-widget-x" end + test "a name with no latin characters still gets its own URL" do + # 192 of the catalogue's items are written in scripts with no latin + # letters at all. Reduced to nothing they would share one slug, and + # only the first on each server would have been reachable. + a = Capability.slug("文档") + b = Capability.slug("指南") + + refute a == b + refute a == "item" + assert Capability.find(["文档", "指南"], b) == "指南" + end + test "find matches on the slug, so a rewritten name is still reachable" do items = ["file:///alerts.json", "review_diff"]