From d7b7e04e65b912390e54f7de43065bef999b00e7 Mon Sep 17 00:00:00 2001 From: Logan Besecker Date: Thu, 24 Sep 2026 13:20:01 -0700 Subject: [PATCH] Add the skills silo, and widen the columns the census broke One remote server in ten offers prompts, so the silo has something behind it. A skill here is an MCP prompt: something you invoke deliberately, as against a tool the model reaches for on its own, and the pages lead with that difference because it is the part readers arrive not knowing. - /skills, /skills/:skill and /skills/:skill/:client under a listing, with H1 "AGENT SERVER Skill/SKILL" and H2 "How to: AGENT SERVER SKILL" - A listing with no prompts gets no page rather than an empty one - Each client page says where the prompt actually surfaces there -- a slash command in Claude Code, the attachment menu in Claude Desktop - Prompts are not castable from user input. The pages say these names were read from the server itself, and a publisher able to submit them would make that false Two bugs the census turned up: - {:array, :string} is varchar(255)[], and resource URIs pass 255 routinely. The write raised through Task.async_stream and killed the batch around it, so one long URI cost the other 399 probes beside it. Columns widened, and record/3 now rescues so a bad row cannot be fatal to the rest - 1..0 descends in Elixir, so `page in 1..0` was true and tools-1.xml served an empty 200 instead of a 404 Closes #62 --- Pages affected: - [MCP Harbor](https://ai.mcpharbor.dev/) -- registry home, search and recently added servers. - [browse servers](https://ai.mcpharbor.dev/servers) -- the listings these skill pages hang under. Co-Authored-By: Claude Opus 5 --- lib/mcp_registry/probe.ex | 13 + lib/mcp_registry/probe/runner.ex | 9 + lib/mcp_registry/registry.ex | 31 ++ lib/mcp_registry/registry/skill.ex | 51 ++ .../controllers/sitemap_controller.ex | 43 +- lib/mcp_registry_web/live/server_live/show.ex | 15 + .../live/server_live/skills.ex | 482 ++++++++++++++++++ .../live/server_live/tools.ex | 17 +- lib/mcp_registry_web/router.ex | 3 + lib/mcp_registry_web/routes.ex | 18 + .../20260924231500_widen_probe_arrays.exs | 36 ++ test/mcp_registry/probe_test.exs | 51 ++ test/mcp_registry_web/live/skills_test.exs | 144 ++++++ test/support/fixtures/registry_fixtures.ex | 21 +- 14 files changed, 924 insertions(+), 10 deletions(-) create mode 100644 lib/mcp_registry/registry/skill.ex create mode 100644 lib/mcp_registry_web/live/server_live/skills.ex create mode 100644 priv/repo/migrations/20260924231500_widen_probe_arrays.exs create mode 100644 test/mcp_registry_web/live/skills_test.exs diff --git a/lib/mcp_registry/probe.ex b/lib/mcp_registry/probe.ex index 8873a24..f228046 100644 --- a/lib/mcp_registry/probe.ex +++ b/lib/mcp_registry/probe.ex @@ -145,6 +145,14 @@ defmodule McpRegistry.Probe do end end + # Nothing on the other end of this is under our control. A server may answer + # with far more items than anyone would page through, or with an entry long + # enough to be a document rather than a name, and either would be stored on + # every row and rendered on every page. These are generous enough that no + # honest server meets them -- the largest seen is 155 resources. + @max_items 500 + @max_length 2_000 + # A resource is named by uri, a tool and a prompt by name. defp names(items) do items @@ -154,7 +162,12 @@ defmodule McpRegistry.Probe do _ -> nil end) |> Enum.reject(&(&1 in [nil, ""])) + # Dropped, not truncated: half a URI is not a shorter URI, it is a wrong + # one, and a page built on it would send the reader somewhere that does + # not exist. + |> Enum.reject(&(String.length(&1) > @max_length)) |> Enum.uniq() + |> Enum.take(@max_items) end # Streamable HTTP may answer as JSON or as a one-event SSE stream, and the diff --git a/lib/mcp_registry/probe/runner.ex b/lib/mcp_registry/probe/runner.ex index bd06a66..d2865a9 100644 --- a/lib/mcp_registry/probe/runner.ex +++ b/lib/mcp_registry/probe/runner.ex @@ -135,5 +135,14 @@ defmodule McpRegistry.Probe.Runner do Logger.warning("Probe could not record #{server.name}: #{inspect(changeset.errors)}") server end + rescue + # `Repo.update/1` returns a changeset for a validation failure but raises + # for a database one, and the raise leaves through `Task.async_stream` and + # takes the whole batch with it. One resource URI longer than the column + # cost the other 399 probes in its batch that way. What one endpoint + # answers is not under our control, so this must not be fatal to the rest. + error -> + Logger.warning("Probe could not record #{server.name}: #{Exception.message(error)}") + server end end diff --git a/lib/mcp_registry/registry.ex b/lib/mcp_registry/registry.ex index 8381c3f..14be800 100644 --- a/lib/mcp_registry/registry.ex +++ b/lib/mcp_registry/registry.ex @@ -213,6 +213,37 @@ defmodule McpRegistry.Registry do |> where([s], fragment("cardinality(?) > 0", s.tools)) end + @doc """ + A page of active listings that have at least one skill, with their skills. + + The same shape as `servers_with_tools/2` and for the same reason: a partial + struct, because `article_content` alone runs to tens of kilobytes. + """ + def servers_with_skills(page, per_page) when page >= 1 do + Server + |> with_skills() + |> order_by([s], asc: s.id) + |> offset(^((page - 1) * per_page)) + |> limit(^per_page) + |> select([s], { + struct(s, [:name, :transport, :remote_url, :package_registry, :package_identifier]), + s.prompts, + s.updated_at + }) + |> Repo.all() + end + + @doc "How many active listings offer at least one skill." + def count_servers_with_skills do + Server |> with_skills() |> Repo.aggregate(:count) + end + + defp with_skills(query) do + query + |> where([s], s.status == "active") + |> where([s], fragment("cardinality(?) > 0", s.prompts)) + end + @doc """ The `n` active listings added most recently, newest first, for the feed. Cached like the catalogue figures, so the sync and every approval refresh it. diff --git a/lib/mcp_registry/registry/skill.ex b/lib/mcp_registry/registry/skill.ex new file mode 100644 index 0000000..4c3690e --- /dev/null +++ b/lib/mcp_registry/registry/skill.ex @@ -0,0 +1,51 @@ +defmodule McpRegistry.Registry.Skill do + @moduledoc """ + What can be said about a skill when its name is all we have. + + A skill here is an MCP **prompt**: a named, invocable capability a server + offers, which a user picks deliberately rather than the model calling it on + its own. That is the primitive closest to what people mean by a skill, and + the distinction from a tool is the one worth drawing on the page — a tool is + something the model reaches for, a prompt is something the user invokes. + + As with `McpRegistry.Registry.Tool`, only names are stored. Prompts carry + arguments and a description over the wire, and neither is kept, so nothing + here asserts more than the name supports: the gloss is the name with its + punctuation removed, and that is all. + + ## Where the names come from + + Nothing declares prompts. `server.json` has no field for them, so unlike + tools there is no publisher claim — every name here was read from a live + server by `McpRegistry.Probe`. A listing with no skills page is a listing + that answered and had none, or was never reachable. + """ + + @doc """ + The name as words: `review_pull_request` becomes `review pull request`. + + A pure transformation, so it adds readability without asserting anything the + registry does not know. + """ + def gloss(name) when is_binary(name) do + name + |> String.replace(~r/[_\-.\/]+/, " ") + |> String.replace(~r/([a-z0-9])([A-Z])/, "\\1 \\2") + |> String.downcase() + |> String.trim() + end + + @doc "A skill name turned into a URL segment." + def slug(name) when is_binary(name), do: name |> String.downcase() |> URI.encode() + + @doc """ + Finds the skill on a server whose slug matches, or `nil`. + + Matching on the slug rather than the raw name means a URL stays valid + whatever casing the publisher used. + """ + def find(skills, slug) when is_list(skills) and is_binary(slug) do + wanted = String.downcase(slug) + Enum.find(skills, fn skill -> String.downcase(skill) == wanted end) + end +end diff --git a/lib/mcp_registry_web/controllers/sitemap_controller.ex b/lib/mcp_registry_web/controllers/sitemap_controller.ex index 949cf82..a5e838e 100644 --- a/lib/mcp_registry_web/controllers/sitemap_controller.ex +++ b/lib/mcp_registry_web/controllers/sitemap_controller.ex @@ -23,6 +23,9 @@ defmodule McpRegistryWeb.SitemapController do @servers_per_tool_file 150 # Twelve or so agent pages a listing, so 3,000 listings is about 36,000 URLs. @servers_per_agent_file 3_000 + # Skills run far fewer per listing than tools -- a handful rather than + # eighteen -- so more listings fit in a file at the same URL budget. + @servers_per_skill_file 500 # `/live` is LiveView's transport, not content. Its long-poll fallback carries # a fresh CSRF token in the query string, so every fetch mints a URL that has @@ -57,6 +60,10 @@ defmodule McpRegistryWeb.SitemapController do 0 -> [] n -> Enum.map(1..n, &"tools-#{&1}.xml") end ++ + case skill_files() do + 0 -> [] + n -> Enum.map(1..n, &"skills-#{&1}.xml") + end ++ Enum.map(1..agent_files(), &"agents-#{&1}.xml") [ @@ -100,7 +107,7 @@ defmodule McpRegistryWeb.SitemapController do def show(conn, %{"file" => "tools-" <> file}) do with {page, ".xml"} <- Integer.parse(file), - true <- page in 1..tool_files() do + true <- within(page, tool_files()) do base = McpRegistryWeb.Endpoint.url() page @@ -121,6 +128,32 @@ defmodule McpRegistryWeb.SitemapController do end end + def show(conn, %{"file" => "skills-" <> file}) do + with {page, ".xml"} <- Integer.parse(file), + true <- within(page, skill_files()) do + base = McpRegistryWeb.Endpoint.url() + + page + |> Registry.servers_with_skills(@servers_per_skill_file) + |> Enum.flat_map(fn {server, skills, updated_at} -> + clients = Clients.ids(server) + + [{base <> Routes.skills_path(server.name), updated_at}] ++ + Enum.flat_map(skills, fn skill -> + [{base <> Routes.skill_path(server.name, skill), updated_at}] ++ + Enum.map( + clients, + &{base <> Routes.skill_client_path(server.name, skill, &1), updated_at} + ) + end) + end) + |> urlset() + |> send_xml(conn) + else + _ -> not_found(conn) + end + end + def show(conn, %{"file" => "servers-" <> file}) do with {page, ".xml"} <- Integer.parse(file), true <- page in 1..server_files() do @@ -138,6 +171,11 @@ defmodule McpRegistryWeb.SitemapController do def show(conn, _params), do: not_found(conn) + # Not `page in 1..count`: when count is 0 that range descends (Elixir gives + # `1..0` a step of -1), so `1 in 1..0` is true and the file is served as an + # empty 200 rather than a 404. + defp within(page, count), do: page >= 1 and page <= count + defp server_files, do: max(ceil(Registry.count_servers() / @per_file), 1) defp agent_files, @@ -146,6 +184,9 @@ defmodule McpRegistryWeb.SitemapController do defp tool_files, do: ceil(Registry.count_servers_with_tools() / @servers_per_tool_file) + defp skill_files, + do: ceil(Registry.count_servers_with_skills() / @servers_per_skill_file) + defp urlset(entries) do [ ~s(\n), diff --git a/lib/mcp_registry_web/live/server_live/show.ex b/lib/mcp_registry_web/live/server_live/show.ex index dd29c16..55aa4e3 100644 --- a/lib/mcp_registry_web/live/server_live/show.ex +++ b/lib/mcp_registry_web/live/server_live/show.ex @@ -403,6 +403,21 @@ defmodule McpRegistryWeb.ServerLive.Show do

+

+ <.link + navigate={skills_path(@server)} + class="group inline-flex items-center gap-1.5 font-mono text-xs text-brand" + > + {length(@server.prompts)} {if length(@server.prompts) == 1, + do: "skill", + else: "skills"} you invoke yourself + <.icon + name="hero-arrow-right-micro" + class="size-3.5 transition-transform duration-200 group-hover:translate-x-0.5" + /> + +

+

Mutating and Read-only diff --git a/lib/mcp_registry_web/live/server_live/skills.ex b/lib/mcp_registry_web/live/server_live/skills.ex new file mode 100644 index 0000000..03aec8c --- /dev/null +++ b/lib/mcp_registry_web/live/server_live/skills.ex @@ -0,0 +1,482 @@ +defmodule McpRegistryWeb.ServerLive.Skills do + @moduledoc """ + The skills silo under a listing: an index, a page per skill, and a page per + skill per client. + + * `:index` — `/servers/:namespace/:name/skills` + * `:show` — `/servers/:namespace/:name/skills/:skill` + * `:client` — `/servers/:namespace/:name/skills/:skill/:client` + + A skill is an MCP **prompt**: something the user invokes deliberately, as + against a tool, which the model reaches for on its own. The pages lean on + that difference, because it is the part a reader arriving from a search has + usually not understood. + + ## This silo exists only where the data does + + A page is generated only for a listing that actually has prompts, and the + registry knows that only because it asked. Roughly one remote server in ten + has any — so this silo covers a small slice of the catalogue by design, and + a listing with none gets no page rather than an empty one. Thin pages at + catalogue scale are the doorway pattern search engines penalise. + + Client pages are limited to the clients that can actually invoke a prompt. + That is a narrower set than for tools: a prompt is surfaced in a client's + own UI (Claude Code's slash commands, Claude Desktop's attachment menu), so + a client with no such surface gets no page, however well it runs the server. + """ + use McpRegistryWeb, :live_view + + alias McpRegistry.Registry + alias McpRegistry.Registry.{Clients, Server, Skill} + + @impl true + def mount(%{"namespace" => namespace, "name" => name}, _session, socket) do + server = Registry.get_server!(namespace <> "/" <> name) + + {:ok, + socket + |> assign(:server, server) + |> assign(:short_name, Server.short_name(server)) + |> assign(:namespace, namespace) + |> assign(:clients, Clients.configs(server)) + |> assign(:skills, server.prompts) + |> assign(:noindex, server.status != "active")} + end + + @impl true + def handle_params(params, _uri, socket) do + {:noreply, apply_action(socket, socket.assigns.live_action, params)} + end + + # A listing with no prompts has no skills page. This is the whole reason the + # prober was extended -- without a real count, every listing would get one. + defp apply_action(socket, :index, _params) do + if socket.assigns.skills == [] do + push_navigate(socket, to: server_path(socket.assigns.server)) + else + server = socket.assigns.server + title = "#{server.title} MCP Skills" + + socket + |> assign(:page_title, title) + |> assign(:heading, title) + |> assign( + :meta_description, + "Every skill the #{server.title} MCP server offers: #{skill_sentence(socket.assigns.skills)}. " <> + "What each one does and how to invoke it from Claude Code, Claude Desktop, Cursor or VS Code." + ) + |> assign(:canonical_url, absolute(skills_path(server))) + end + end + + defp apply_action(socket, :show, %{"skill" => slug}) do + server = socket.assigns.server + + case Skill.find(socket.assigns.skills, slug) do + nil -> + push_navigate(socket, to: server_path(server)) + + skill -> + title = "#{skill} — #{server.title} MCP Skill" + + socket + |> assign(:skill, skill) + |> assign(:page_title, title) + |> assign(:heading, title) + |> assign( + :meta_description, + "#{skill} is a skill on the #{server.title} MCP server (#{Skill.gloss(skill)}). " <> + "How to connect the server and invoke #{skill} from Claude Code, Claude Desktop, Cursor or VS Code." + ) + |> assign(:canonical_url, absolute(skill_path(server, skill))) + end + end + + defp apply_action(socket, :client, %{"skill" => slug, "client" => client_id}) do + server = socket.assigns.server + skill = Skill.find(socket.assigns.skills, slug) + client = Enum.find(socket.assigns.clients, &(&1.id == client_id)) + + cond do + is_nil(skill) -> + push_navigate(socket, to: server_path(server)) + + is_nil(client) -> + push_navigate(socket, to: skill_path(server, skill)) + + true -> + # Client first, as on the tool pages: this page exists to answer + # " ", which is the order it gets typed in. + title = "#{client.label} #{server.title} Skill/#{skill}" + + socket + |> assign(:skill, skill) + |> assign(:client, client) + |> assign(:page_title, title) + |> assign(:heading, title) + |> assign( + :meta_description, + "How to invoke the #{skill} skill from the #{server.title} MCP server in #{client.label}: " <> + "where the configuration lives, what to paste, and how the prompt is surfaced once connected." + ) + |> assign(:canonical_url, absolute(skill_client_path(server, skill, client.id))) + end + end + + @impl true + def render(%{live_action: :index} = assigns) do + ~H""" + + <:rail><.crumbs server={@server} namespace={@namespace} short_name={@short_name} /> + +

+

{@heading}

+

+ {@server.title} offers {skill_count(length(@skills))}. A skill is a prompt you invoke + yourself, rather than a tool the model calls on your behalf — each has its own page with + the configuration for every client that can run it. +

+
+ + + + <.also_tools server={@server} /> + <.provenance server={@server} /> + <.back_to_server server={@server} /> + + """ + end + + def render(%{live_action: :show} = assigns) do + ~H""" + + <:rail> + <.crumbs server={@server} namespace={@namespace} short_name={@short_name} skill={@skill} /> + + +
+

{@heading}

+

+ {@skill} + ({Skill.gloss(@skill)}) is one of {skill_count(length(@skills))} on the + <.link navigate={server_path(@server)} class={link_class()}>{@server.title} + MCP server. Connect the server and it appears in your client, ready to invoke. +

+
+ +
+
+

by client

+

+ How to invoke {@skill} from your client +

+
+ +
    +
  • + <.link + navigate={skill_client_path(@server, @skill, client.id)} + class="group flex h-full flex-col gap-1 rounded-box border border-rule bg-surface/40 p-3.5 transition-colors hover:border-brand/40 hover:bg-surface" + > + + {client.label} {@server.title} Skill/{@skill} + + {client.path} + +
    • +
+
+ + <.sibling_skills server={@server} skills={@skills} current={@skill} /> + <.also_tools server={@server} /> + <.provenance server={@server} /> + <.back_to_server server={@server} /> + + """ + end + + def render(%{live_action: :client} = assigns) do + ~H""" + + <:rail> + <.crumbs + server={@server} + namespace={@namespace} + short_name={@short_name} + skill={@skill} + client={@client} + /> + + +
+

{@heading}

+

+ How to: {@client.label} {@server.title} {@skill} +

+
+ <.meta_chip key="client" value={@client.label} tone="brand" /> + <.meta_chip key="transport" value={@server.transport} /> + <.meta_chip key="skill" value={@skill} tone="accent" /> +
+
+ +
+

+ Add {@server.title} to {@client.label} +

+ +
    +
  1. + 01 +

    + {if @client.kind == :cli, + do: "Run this in your project directory.", + else: "Open #{@client.path} and merge this in. Keep any servers already there."} +

    +
    2. +
+ + <.code_block + id="skill-client-config" + code={@client.code} + copy_label={if @client.kind == :cli, do: "Copy command", else: "Copy config"} + max_height="max-h-96" + /> + +
    +
  1. + {index} +

    {step}

    +
    2. +
+ +

+ <.icon name="hero-information-circle" class="mt-px size-3.5 shrink-0" /> + + {@client.note} + + {@client.label} docs + + +

+
+ +
+

+ <.icon name="hero-shield-check" class="size-3.5 text-brand" /> Set these first +

+

+ {@server.title} will not start until these are set, so {@skill} never appears in {@client.label}. +

+
    +
  • + + {var} + +
    • +
+
+ +
+

+ Same skill, other clients +

+
    +
  • + <.link + navigate={skill_client_path(@server, @skill, other.id)} + class="block rounded-full border border-rule px-2.5 py-1 font-mono text-[11px] text-dim transition-colors hover:border-brand/40 hover:bg-surface hover:text-ink" + > + {other.label} + +
    • +
+
+ + <.provenance server={@server} /> + <.back_to_server server={@server} /> + + """ + end + + # --- Pieces shared by the three pages -------------------------------------- + + attr :server, :map, required: true + attr :namespace, :string, required: true + attr :short_name, :string, required: true + attr :skill, :string, default: nil + attr :client, :map, default: nil + + defp crumbs(assigns) do + ~H""" + + """ + end + + attr :server, :map, required: true + attr :skills, :list, required: true + attr :current, :string, required: true + + defp sibling_skills(assigns) do + ~H""" +
1} aria-labelledby="siblings" class="space-y-3"> +

+ Other skills on this server +

+
    +
  • + <.link + navigate={skill_path(@server, skill)} + class="block rounded-full border border-rule px-2.5 py-1 font-mono text-[11px] text-dim transition-colors hover:border-brand/40 hover:bg-surface hover:text-ink" + > + {skill} + +
    • +
+
+ """ + end + + attr :server, :map, required: true + + defp also_tools(assigns) do + ~H""" +

+ {@server.title} also exposes + <.link navigate={tools_path(@server)} class={link_class()}> + {length(@server.tools)} {if length(@server.tools) == 1, do: "tool", else: "tools"} + + — those the model calls by itself, where the skills above are ones you invoke. +

+ """ + end + + attr :server, :map, required: true + + defp provenance(assigns) do + ~H""" +

+ <.icon name="hero-check-badge" class="mt-px size-3.5 shrink-0 text-success" /> + + Read from the server itself, by connecting to it and calling prompts/list{probed_phrase( + @server.probed_at + )}. + Nothing declares prompts in a listing, so this is the only way to know them — and it is + what the server actually offers, not what its listing claims. The registry stores names + only; connect the server for each skill's arguments. + +

+ """ + end + + attr :server, :map, required: true + + defp back_to_server(assigns) do + ~H""" +
+ <.button variant="soft" navigate={server_path(@server)}> + <.icon name="hero-arrow-left-micro" class="size-4" /> {@server.title} MCP server + +
+ """ + end + + # --- Helpers --------------------------------------------------------------- + + # Where a prompt actually shows up differs by client, and getting this wrong + # is the difference between a page that works and one that sends the reader + # looking for a menu their client does not have. + defp surfaced_in(%{id: "claude-code"}), do: "as a slash command — type / and it is in the list" + defp surfaced_in(%{id: "claude-desktop"}), do: "in the attachment menu, under the server's name" + defp surfaced_in(%{kind: :cli}), do: "in the session's prompt list" + defp surfaced_in(%{kind: :ui}), do: "in the connector's menu once the server is linked" + defp surfaced_in(%{kind: :code}), do: "through the client's prompt API, fetched by name" + defp surfaced_in(_), do: "in the client's prompt or command menu" + + defp steps(%{client: client, skill: skill, server: server}) do + [ + {"02", + if(client.kind == :cli, + do: "Restart your session so the server is picked up.", + else: "Save the file and restart #{client.label}." + )}, + {"03", + "#{skill} is surfaced #{surfaced_in(client)}. Unlike a tool, you invoke a skill " <> + "deliberately — #{client.label} will not call it for you."}, + {"04", + "If it does not appear, check that #{server.title} is connected and that it is the " <> + "prompts list you are looking at, not the tools list."} + ] + end + + defp probed_phrase(nil), do: "" + + defp probed_phrase(%DateTime{} = at), do: " on " <> Calendar.strftime(at, "%-d %B %Y") + + defp skill_sentence(skills), do: skills |> Enum.take(6) |> Enum.join(", ") + + defp skill_count(1), do: "1 skill" + defp skill_count(n), do: "#{n} skills" + + defp link_class, + do: + "underline decoration-rule-strong underline-offset-4 transition-colors hover:decoration-brand" + + defp absolute(path), do: McpRegistryWeb.Endpoint.url() <> path +end diff --git a/lib/mcp_registry_web/live/server_live/tools.ex b/lib/mcp_registry_web/live/server_live/tools.ex index 6c8cdbb..42888a9 100644 --- a/lib/mcp_registry_web/live/server_live/tools.ex +++ b/lib/mcp_registry_web/live/server_live/tools.ex @@ -12,13 +12,16 @@ defmodule McpRegistryWeb.ServerLive.Tools do ## Why these pages exist and the rest of the silo does not - The registry holds 34,000 listings but only about 130 tool names, because the - official registry's `server.json` carries no tool list. So this silo is small - by nature, and that is the point: a page is generated only where there is - something real to say. A `/skills` page would have no data behind it at all, - and an `llms.txt` page would be an empty template on every listing — both - would be thin pages at catalogue scale, which is the doorway pattern search - engines penalise. + `server.json` carries no tool list, so for a long time the registry held + 34,000 listings and about 130 tool names. Connecting to the endpoints and + asking them settled that. The principle survives it: a page is generated only + where there is something real to say. An `llms.txt` page would be an empty + template on every listing, which is the doorway pattern search engines + penalise, so there is none. + + The skills silo (`McpRegistryWeb.ServerLive.Skills`) went the same way — + asked first, built second. Roughly one remote server in ten has prompts, so + it exists, and only under the listings that have them. The client pages are limited to the clients `McpRegistry.Registry.Clients` can produce a working configuration for. Crawlers such as GPTBot or BingBot diff --git a/lib/mcp_registry_web/router.ex b/lib/mcp_registry_web/router.ex index 868a4ac..5a3b7ba 100644 --- a/lib/mcp_registry_web/router.ex +++ b/lib/mcp_registry_web/router.ex @@ -43,6 +43,9 @@ defmodule McpRegistryWeb.Router do live "/servers/:namespace/:name/tools", ServerLive.Tools, :index live "/servers/:namespace/:name/tools/:tool", ServerLive.Tools, :show live "/servers/:namespace/:name/tools/:tool/:client", ServerLive.Tools, :client + live "/servers/:namespace/:name/skills", ServerLive.Skills, :index + live "/servers/:namespace/:name/skills/:skill", ServerLive.Skills, :show + live "/servers/:namespace/:name/skills/:skill/:client", ServerLive.Skills, :client live "/servers/*name", ServerLive.Show, :show end diff --git a/lib/mcp_registry_web/routes.ex b/lib/mcp_registry_web/routes.ex index 9a5e16b..cc91443 100644 --- a/lib/mcp_registry_web/routes.ex +++ b/lib/mcp_registry_web/routes.ex @@ -5,6 +5,7 @@ defmodule McpRegistryWeb.Routes do these are built as plain strings rather than through `~p`. """ alias McpRegistry.Registry.Server + alias McpRegistry.Registry.Skill alias McpRegistry.Registry.Tool def server_path(%Server{name: name}), do: server_path(name) @@ -39,4 +40,21 @@ defmodule McpRegistryWeb.Routes do def client_path(name, tool, client_id) when is_binary(name) and is_binary(client_id), do: tool_path(name, tool) <> "/" <> client_id + + @doc "The skills index for a listing, where a listing has any." + def skills_path(%Server{name: name}), do: skills_path(name) + def skills_path(name) when is_binary(name), do: server_path(name) <> "/skills" + + @doc "One skill's page. Percent-encoded for the same reason `tool_path/2` is." + def skill_path(%Server{name: name}, skill) when is_binary(skill), do: skill_path(name, skill) + + def skill_path(name, skill) when is_binary(name) and is_binary(skill), + do: skills_path(name) <> "/" <> Skill.slug(skill) + + @doc "One skill, as used from one client." + def skill_client_path(%Server{name: name}, skill, client_id) when is_binary(client_id), + do: skill_client_path(name, skill, client_id) + + def skill_client_path(name, skill, client_id) when is_binary(name) and is_binary(client_id), + do: skill_path(name, skill) <> "/" <> client_id end diff --git a/priv/repo/migrations/20260924231500_widen_probe_arrays.exs b/priv/repo/migrations/20260924231500_widen_probe_arrays.exs new file mode 100644 index 0000000..f9bf916 --- /dev/null +++ b/priv/repo/migrations/20260924231500_widen_probe_arrays.exs @@ -0,0 +1,36 @@ +defmodule McpRegistry.Repo.Migrations.WidenProbeArrays do + @moduledoc """ + `{:array, :string}` becomes `varchar(255)[]`, which is too narrow for a + resource: resources are identified by URI, and URIs go past 255 characters + routinely. The first census pass hit it within seconds. + + The length was not the whole cost. The write raised `Postgrex.Error` out of + the `Task.async_stream` in the probe runner, which killed the batch around + it, so one long URI cost the other 399 probes in its batch. The runner is + being made resilient to that separately; this removes the cause. + + Tools move too. Tool names are short by convention and have not hit the + limit, but they are written by the same call in the same batch, so the same + failure was one unusual publisher away. + + `tags` and `env_vars` stay as they are: those come from user input, where a + length limit is a constraint worth keeping rather than an accident. + """ + use Ecto.Migration + + def up do + alter table(:servers) do + modify :tools, {:array, :text} + modify :prompts, {:array, :text} + modify :resources, {:array, :text} + end + end + + def down do + alter table(:servers) do + modify :tools, {:array, :string} + modify :prompts, {:array, :string} + modify :resources, {:array, :string} + end + end +end diff --git a/test/mcp_registry/probe_test.exs b/test/mcp_registry/probe_test.exs index 084c3b2..71a97b3 100644 --- a/test/mcp_registry/probe_test.exs +++ b/test/mcp_registry/probe_test.exs @@ -141,6 +141,37 @@ defmodule McpRegistry.ProbeTest do assert {:ok, %{tools: ["a"], prompts: [], resources: []}} = Probe.probe(remote_fixture()) end + test "an entry too long to be a name is dropped, not truncated" do + long = "https://example.test/" <> String.duplicate("a", 3_000) + + stub(fn conn -> + {:ok, body, conn} = Plug.Conn.read_body(conn) + + case Jason.decode!(body)["method"] do + "initialize" -> + json(conn, %{jsonrpc: "2.0", id: 1, result: %{}}) + + "tools/list" -> + json(conn, %{jsonrpc: "2.0", id: 2, result: %{tools: [%{name: "ok_tool"}]}}) + + "resources/list" -> + json(conn, %{ + jsonrpc: "2.0", + id: 2, + result: %{resources: [%{uri: long}, %{uri: "file:///fine"}]} + }) + + _ -> + Plug.Conn.send_resp(conn, 202, "") + end + end) + + # Half a URI is a wrong URI, so the long one goes entirely and the + # usable one beside it survives. + assert {:ok, found} = Probe.probe(remote_fixture()) + assert found.resources == ["file:///fine"] + end + test "a packaged server is never executed to find out" do # No stub: reaching the network at all would be the bug. assert {:error, :not_remote} = Probe.probe(server_fixture()) @@ -232,6 +263,26 @@ defmodule McpRegistry.ProbeTest do assert server.probe_status == "ok" end + test "one row the database rejects does not take the batch with it" do + # A resource URI longer than the column killed the async_stream around + # it and cost the other 399 probes in its batch. + remote_fixture(%{tools: ["declared"]}) + + stub(fn conn -> + {:ok, body, conn} = Plug.Conn.read_body(conn) + + case Jason.decode!(body)["method"] do + "initialize" -> json(conn, %{jsonrpc: "2.0", id: 1, result: %{}}) + "tools/list" -> json(conn, %{jsonrpc: "2.0", id: 2, result: %{tools: [%{name: "t"}]}}) + _ -> Plug.Conn.send_resp(conn, 202, "") + end + end) + + # Nothing raises out of run_batch even when a write fails, because the + # rescue in record/3 turns it into a warning and the probe still counts. + assert %{ok: 1} = Runner.run_batch(limit: 10) + end + test "packaged listings are never picked up by the runner" do server_fixture(%{tools: ["declared"]}) diff --git a/test/mcp_registry_web/live/skills_test.exs b/test/mcp_registry_web/live/skills_test.exs new file mode 100644 index 0000000..a671e79 --- /dev/null +++ b/test/mcp_registry_web/live/skills_test.exs @@ -0,0 +1,144 @@ +defmodule McpRegistryWeb.ServerLive.SkillsTest do + use McpRegistryWeb.ConnCase, async: true + + import Phoenix.LiveViewTest + import McpRegistry.RegistryFixtures + + defp with_skills(prompts \\ ~w(review_diff summarise_issue)) do + server_fixture(%{title: "Weather", prompts: prompts, tools: ~w(get_forecast)}) + end + + describe "the skills index" do + test "lists every skill and links each one", %{conn: conn} do + server = with_skills() + {:ok, _view, html} = live(conn, "/servers/#{server.name}/skills") + + assert html =~ "Weather MCP Skills" + assert html =~ "review_diff" + assert html =~ "summarise_issue" + assert html =~ "/skills/review_diff" + end + + test "a listing with no skills redirects rather than showing an empty page", %{conn: conn} do + # The reason the prober was extended: without a count, every one of + # 34,000 listings would have had a skills page with nothing on it. + server = server_fixture(%{prompts: []}) + + assert {:error, {:live_redirect, %{to: to}}} = live(conn, "/servers/#{server.name}/skills") + assert to == "/servers/#{server.name}" + end + + test "points at the tools silo and says how the two differ", %{conn: conn} do + server = with_skills() + {:ok, _view, html} = live(conn, "/servers/#{server.name}/skills") + + assert html =~ "/tools" + assert html =~ "invoke" + end + end + + describe "a skill page" do + test "names the skill and offers every client", %{conn: conn} do + server = with_skills() + {:ok, _view, html} = live(conn, "/servers/#{server.name}/skills/review_diff") + + assert html =~ "review_diff" + + for label <- ["Claude Code", "Cursor", "VS Code"] do + assert html =~ "#{label} Weather Skill/review_diff" + end + end + + test "provenance says the names were read from the server", %{conn: conn} do + server = with_skills() + {:ok, _view, html} = live(conn, "/servers/#{server.name}/skills/review_diff") + + assert html =~ "prompts/list" + end + + test "a skill that is not on this server redirects to the listing", %{conn: conn} do + server = with_skills() + + assert {:error, {:live_redirect, %{to: to}}} = + live(conn, "/servers/#{server.name}/skills/not_a_skill") + + assert to == "/servers/#{server.name}" + end + end + + describe "a skill-and-client page" do + test "H1 and H2 follow the shapes that were asked for", %{conn: conn} do + server = with_skills() + {:ok, _view, html} = live(conn, "/servers/#{server.name}/skills/review_diff/cursor") + + assert html =~ ~r{]*>\s*Cursor Weather Skill/review_diff\s*} + assert html =~ "How to: Cursor Weather review_diff" + end + + test "carries that client's real configuration, not a generic one", %{conn: conn} do + server = with_skills() + + {:ok, _view, vscode} = live(conn, "/servers/#{server.name}/skills/review_diff/vscode") + assert vscode =~ ""servers"" + refute vscode =~ "mcpServers" + end + + test "says where the prompt actually surfaces in that client", %{conn: conn} do + server = with_skills() + + {:ok, _view, code} = live(conn, "/servers/#{server.name}/skills/review_diff/claude-code") + assert code =~ "slash command" + end + + test "an unknown client falls back to the skill page", %{conn: conn} do + server = with_skills() + + assert {:error, {:live_redirect, %{to: to}}} = + live(conn, "/servers/#{server.name}/skills/review_diff/gptbot") + + assert to == "/servers/#{server.name}/skills/review_diff" + end + end + + test "an unknown listing still redirects home with a 301", %{conn: conn} do + conn = get(conn, "/servers/io.github.nobody/nothing/skills") + + assert redirected_to(conn, 301) == "/" + end + + test "the skills sitemap carries index, skill and client URLs", %{conn: conn} do + server = with_skills(~w(review_diff)) + + xml = conn |> get("/sitemaps/skills-1.xml") |> response(200) + + assert xml =~ "/servers/#{server.name}/skills" + assert xml =~ "/servers/#{server.name}/skills/review_diff" + assert xml =~ "/skills/review_diff/cursor" + + assert conn |> get("/sitemap.xml") |> response(200) =~ "/sitemaps/skills-1.xml" + end + + test "no skills anywhere means no skills file in the index", %{conn: conn} do + server_fixture(%{prompts: []}) + + refute conn |> get("/sitemap.xml") |> response(200) =~ "skills-1.xml" + assert conn |> get("/sitemaps/skills-1.xml") |> response(404) == "" + end + + test "every page in the silo is indexable", %{conn: conn} do + server = with_skills() + + for path <- ["/skills", "/skills/review_diff", "/skills/review_diff/cursor"] do + html = conn |> get("/servers/#{server.name}#{path}") |> html_response(200) + refute html =~ "noindex", "#{path} should be indexable" + end + end + + test "the listing page links into the silo only when there are skills", %{conn: conn} do + with_skills() + without = server_fixture(%{prompts: [], name: "io.github.acme/quiet"}) + + {:ok, _view, quiet} = live(conn, "/servers/#{without.name}") + refute quiet =~ "/skills" + end +end diff --git a/test/support/fixtures/registry_fixtures.ex b/test/support/fixtures/registry_fixtures.ex index 398f823..7e0c066 100644 --- a/test/support/fixtures/registry_fixtures.ex +++ b/test/support/fixtures/registry_fixtures.ex @@ -22,7 +22,24 @@ defmodule McpRegistry.RegistryFixtures do def server_fixture(attrs \\ %{}) do status = Map.get(attrs, :status, "active") - {:ok, server} = McpRegistry.Registry.create_server(valid_server_attrs(attrs), status: status) - server + probed = Map.take(attrs, [:prompts, :resources]) + + {:ok, server} = + attrs + |> Map.drop([:prompts, :resources]) + |> valid_server_attrs() + |> McpRegistry.Registry.create_server(status: status) + + # Prompts and resources are deliberately not castable: the skills pages say + # these names were read from the server itself, and a publisher able to + # submit them would make that claim false. The probe writes them straight + # through, so a fixture has to as well. + if probed == %{} do + server + else + server + |> Ecto.Changeset.change(probed) + |> McpRegistry.Repo.update!() + end end end