docs: generate the MCP tool reference from the server's own tools/list - #193
Merged
Conversation
The MCP reference was hand-written, and it was the last page describing a surface a reader acts on with nothing keeping it level with the code. It named four input names. The server exposes eleven. Absent entirely were `trace_id` — the input that makes `trace_detail` fetch a specific trace rather than a guess — `search` and `severity` on `search_logs`, and every input of all four dashboard tools, including which are required. The page also stated that `service_performance` and `trace_detail` "additionally accept `service`", which reads as a closed list and is wrong: `search_logs` accepts it too. `internal/mcp/describe.go` follows internal/api/describe.go. It does not read the registration calls: it starts a real server over an in-memory transport and issues `tools/list`, the same request a connecting agent makes, and publishes the answer. A generator that parsed registerTools would describe the shape of the code; this asks the server the question its clients ask, so a tool renamed, re-annotated, or given a new input cannot be documented as it used to be. Mutation semantics come from the server's annotations rather than from the tool's name. Absent hints default to true per the MCP spec, because reading a missing destructiveHint as false would publish a mutating tool as safe — and "this tool does not modify anything" is a claim someone connects an agent on. The server is built with a non-nil dashboard service deliberately: a nil one registers no dashboard tools at all, so the page would have quietly described five tools instead of nine, two of which mutate. A test pins that. site.yml listed only internal/config as a generator input while claiming to enumerate them. It now lists all of them — internal/alert, internal/api, internal/agent, internal/mcp and cmd/fanout had accumulated behind it. Verified locally: the drift guard fires (a changed tool description fails `--check` naming the page, and passes again on restore); the summary table's anchors resolve in the built HTML; the three internal links exist; llms.txt indexes the page with its summary and both read_when lines; `-race` and `-count=3` are clean. Closes #181.
Review findings on the generated MCP reference.
The page stated "Every tool is closed-world: it reads or writes what this
instance holds and reaches nothing else" as flat prose, while ToolDoc.OpenWorld
was computed and never read. A tool registered without an openWorldHint defaults
to open-world per the spec, and would have been republished under that blanket
promise. `--check` could not catch it: a regenerated page carrying the same
false claim is self-consistent. The renderer now refuses.
Tool titles and descriptions were interpolated into MDX with no escaping. A
description containing a pipe would have split its table row into phantom
columns — wrong output rather than broken output, so nothing downstream would
have noticed; check-tables.mjs asserts that tables are wrapped, not that their
rows have the right shape. One containing `<` or `{` would have failed the site
build instead. Both are escaped now, along with newlines, which end a row.
The escaping is a no-op on every current description, which is why it needed
adversarial input to verify rather than a passing build.
Also: an empty Title was accepted where an empty Description was refused, and
the renderer emits the title as `**%s** — description`, so it would have
published a heading beginning with stray emphasis. The scope test filtered by
tool name and asserted nothing when the filter matched nothing, so renaming
either tool would have switched it off silently; it also compared inputs as a
subset, so a field added to QueryInput would have passed while the page's claim
that the two share one scope went stale. And the ServerSession was leaked on the
client.Connect error path — the same leak the deferred close exists to prevent,
on the path that returns before reaching it. The handshake is now bounded, so a
transport that never completes fails the build rather than hanging it.
Each guard verified by breaking it: dropping OpenWorldHint, blanking a Title,
and putting `p50 | p95 | p99 over a <window> with {braces}` in a description —
which now renders as one cell of literal text and builds.
Second review round. Three guards; the generated page is byte-identical. `schemaType` returned `any` for a property whose schema names no type, while `toolInputs`' own comment two functions above says it "refuses anything it does not recognise rather than treating an unreadable schema as 'no inputs'". The comment described what was meant, not what the code did. The SDK emits `$ref`, `anyOf` or a bare `enum` for shapes it cannot reduce to one type — making an input a pointer or an interface is enough — and `any` publishes as a deliberate "accepts anything" rather than "not determined". It is an error now, naming the keys the schema does carry so the message points at the cause. The summary table links each row to the tool's own section, whose heading is the tool name. That holds only while the name is already what a slugger would produce from it: a name with a capital or a dot slugs to something else and every row link on the page goes dead. Nothing downstream catches a dead in-page anchor — `--check` compares the generator's output to itself, check-tables.mjs checks wrappers, and the build succeeds — so the assumption is now checked where it is made. site.yml's check step still described the reference as generated from internal/config, internal/alert and internal/api, contradicting the trigger paths eight lines above that now correctly name internal/agent, internal/mcp and cmd/fanout too. It points at that list rather than restating it, so the next source to be added cannot leave it stale again. Verified by breaking each: a tool renamed to `Observability.Overview` is refused by name, and schemaType has tests for `$ref`, `anyOf`, a bare enum, `["null"]` and an empty type, plus that it still renders unions with null dropped.
vishr
added a commit
that referenced
this pull request
Aug 24, 2026
#193) * docs: generate the MCP tool reference from the server's own tools/list The MCP reference was hand-written, and it was the last page describing a surface a reader acts on with nothing keeping it level with the code. It named four input names. The server exposes eleven. Absent entirely were `trace_id` — the input that makes `trace_detail` fetch a specific trace rather than a guess — `search` and `severity` on `search_logs`, and every input of all four dashboard tools, including which are required. The page also stated that `service_performance` and `trace_detail` "additionally accept `service`", which reads as a closed list and is wrong: `search_logs` accepts it too. `internal/mcp/describe.go` follows internal/api/describe.go. It does not read the registration calls: it starts a real server over an in-memory transport and issues `tools/list`, the same request a connecting agent makes, and publishes the answer. A generator that parsed registerTools would describe the shape of the code; this asks the server the question its clients ask, so a tool renamed, re-annotated, or given a new input cannot be documented as it used to be. Mutation semantics come from the server's annotations rather than from the tool's name. Absent hints default to true per the MCP spec, because reading a missing destructiveHint as false would publish a mutating tool as safe — and "this tool does not modify anything" is a claim someone connects an agent on. The server is built with a non-nil dashboard service deliberately: a nil one registers no dashboard tools at all, so the page would have quietly described five tools instead of nine, two of which mutate. A test pins that. site.yml listed only internal/config as a generator input while claiming to enumerate them. It now lists all of them — internal/alert, internal/api, internal/agent, internal/mcp and cmd/fanout had accumulated behind it. Verified locally: the drift guard fires (a changed tool description fails `--check` naming the page, and passes again on restore); the summary table's anchors resolve in the built HTML; the three internal links exist; llms.txt indexes the page with its summary and both read_when lines; `-race` and `-count=3` are clean. Closes #181. * docs: check the MCP page's claims instead of asserting them Review findings on the generated MCP reference. The page stated "Every tool is closed-world: it reads or writes what this instance holds and reaches nothing else" as flat prose, while ToolDoc.OpenWorld was computed and never read. A tool registered without an openWorldHint defaults to open-world per the spec, and would have been republished under that blanket promise. `--check` could not catch it: a regenerated page carrying the same false claim is self-consistent. The renderer now refuses. Tool titles and descriptions were interpolated into MDX with no escaping. A description containing a pipe would have split its table row into phantom columns — wrong output rather than broken output, so nothing downstream would have noticed; check-tables.mjs asserts that tables are wrapped, not that their rows have the right shape. One containing `<` or `{` would have failed the site build instead. Both are escaped now, along with newlines, which end a row. The escaping is a no-op on every current description, which is why it needed adversarial input to verify rather than a passing build. Also: an empty Title was accepted where an empty Description was refused, and the renderer emits the title as `**%s** — description`, so it would have published a heading beginning with stray emphasis. The scope test filtered by tool name and asserted nothing when the filter matched nothing, so renaming either tool would have switched it off silently; it also compared inputs as a subset, so a field added to QueryInput would have passed while the page's claim that the two share one scope went stale. And the ServerSession was leaked on the client.Connect error path — the same leak the deferred close exists to prevent, on the path that returns before reaching it. The handshake is now bounded, so a transport that never completes fails the build rather than hanging it. Each guard verified by breaking it: dropping OpenWorldHint, blanking a Title, and putting `p50 | p95 | p99 over a <window> with {braces}` in a description — which now renders as one cell of literal text and builds. * docs: fail on a schema or tool name the MCP page cannot render Second review round. Three guards; the generated page is byte-identical. `schemaType` returned `any` for a property whose schema names no type, while `toolInputs`' own comment two functions above says it "refuses anything it does not recognise rather than treating an unreadable schema as 'no inputs'". The comment described what was meant, not what the code did. The SDK emits `$ref`, `anyOf` or a bare `enum` for shapes it cannot reduce to one type — making an input a pointer or an interface is enough — and `any` publishes as a deliberate "accepts anything" rather than "not determined". It is an error now, naming the keys the schema does carry so the message points at the cause. The summary table links each row to the tool's own section, whose heading is the tool name. That holds only while the name is already what a slugger would produce from it: a name with a capital or a dot slugs to something else and every row link on the page goes dead. Nothing downstream catches a dead in-page anchor — `--check` compares the generator's output to itself, check-tables.mjs checks wrappers, and the build succeeds — so the assumption is now checked where it is made. site.yml's check step still described the reference as generated from internal/config, internal/alert and internal/api, contradicting the trigger paths eight lines above that now correctly name internal/agent, internal/mcp and cmd/fanout too. It points at that list rather than restating it, so the next source to be added cannot leave it stale again. Verified by breaking each: a tool renamed to `Observability.Overview` is refused by name, and schemaType has tests for `$ref`, `anyOf`, a bare enum, `["null"]` and an empty type, plus that it still renders unions with null dropped.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The MCP reference was the last page describing a surface a reader acts on with
nothing keeping it level with the code. It named four input names. The server
exposes eleven.
What the hand-written page was missing
window,namespace,service,limittrace_id,search,severity,id,name,state,descriptionSeven inputs were absent entirely, including:
trace_id— the input that makestrace_detailfetch a specific tracerather than let the server pick a recent one. Its absence meant the page
documented only half of what that tool does.
searchandseverityonsearch_logs— the two filters that make logtriage useful.
dashboard_getneeds anid; the page never said so.The page also stated that
service_performanceandtrace_detail"additionally accept
service". That reads as a closed list, and it is wrong —search_logsaccepts it too.Approach
internal/mcp/describe.gofollows the patterninternal/api/describe.goestablished. It does not read the registration calls. It starts a real
server over an in-memory transport and issues
tools/list— the same request aconnecting agent makes — and publishes the answer.
A generator that parsed
registerToolswould be describing the shape of thecode. This asks the server the question its clients ask, so a tool renamed,
re-annotated, or given a new input cannot be documented as it used to be.
Two decisions worth calling out:
Absent hints default to
trueper the MCP spec, because reading a missingdestructiveHintasfalsewould publish a mutating tool as safe. "This tooldoes not modify anything" is a claim someone connects an agent on.
one registers no dashboard tools at all, so the page would have quietly
described five tools instead of nine — two of which mutate. A test pins that.
Nested object properties are not flattened. A JSON Schema is the precise
artefact and a table is not the place to restate one, so a nested input is
published with its own type and description.
Also here
site.ymllisted onlyinternal/configas a generator input while its commentclaimed to enumerate them. It had fallen three behind — the generator also reads
internal/alert,internal/api,internal/agent,internal/mcpandcmd/fanout. Not load-bearing, since a regenerated page changessite/**anyway, but a list that claims to be complete should be.
Verification
--checkfails namingmcp-tools.mdx; restored → passesDescribeToolsagainstListToolsover a live sessiondashboard_updateis destructive and not read-onlyid="dashboard_create"etc. resolve in built HTML, so the summary table links workroles,http-routes,connect-over-mcpall exist indistllms.txtread_whenlines-race,-count=3just checkCloses #181.