From 537f71f91b1ee96f4119a7e79143774943ceec2d Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Mon, 24 Aug 2026 07:43:54 -0700 Subject: [PATCH 1/3] docs: generate the MCP tool reference from the server's own tools/list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .github/workflows/site.yml | 16 +- cmd/fanout-docgen/main.go | 12 +- cmd/fanout-docgen/mcptools.go | 156 +++++++++++ internal/mcp/describe.go | 244 ++++++++++++++++++ internal/mcp/describe_test.go | 125 +++++++++ site/src/content/docs/reference/mcp-tools.mdx | 181 +++++++++---- 6 files changed, 683 insertions(+), 51 deletions(-) create mode 100644 cmd/fanout-docgen/mcptools.go create mode 100644 internal/mcp/describe.go create mode 100644 internal/mcp/describe_test.go diff --git a/.github/workflows/site.yml b/.github/workflows/site.yml index b094a6ca..435e07fa 100644 --- a/.github/workflows/site.yml +++ b/.github/workflows/site.yml @@ -16,13 +16,21 @@ name: Site on: push: branches: [main] - # The whole site is one build, but only these inputs can change it. The - # configuration type is in the list because the settings pages are generated - # from it: a change there with a regenerated page is a documentation change - # whether or not anything under site/ was touched by hand. + # The whole site is one build, but only these inputs can change it. Every + # source the generator reads is listed, because a change there with a + # regenerated page is a documentation change whether or not anything under + # site/ was touched by hand: settings come from internal/config, alert + # variables from internal/alert, the roles matrix and route table from + # internal/api plus internal/agent and cmd/fanout, and the MCP tools from + # internal/mcp. paths: - "site/**" - "internal/config/**" + - "internal/alert/**" + - "internal/api/**" + - "internal/agent/**" + - "internal/mcp/**" + - "cmd/fanout/**" - "cmd/fanout-docgen/**" - ".github/workflows/site.yml" # Publishing the current main on demand, without an empty commit. diff --git a/cmd/fanout-docgen/main.go b/cmd/fanout-docgen/main.go index 58370025..72f68c55 100644 --- a/cmd/fanout-docgen/main.go +++ b/cmd/fanout-docgen/main.go @@ -109,6 +109,14 @@ func run(source, alertSource, routeDirs, outDir string, check bool) error { } pages["roles.mdx"] = rolesPage + // The MCP tool surface, from the server's own tools/list answer rather than + // from its registration calls — the same question a connecting agent asks. + toolsPage, err := renderMCPTools() + if err != nil { + return err + } + pages["mcp-tools.mdx"] = toolsPage + var stale []string for name, body := range pages { path := filepath.Join(outDir, name) @@ -157,8 +165,8 @@ func run(source, alertSource, routeDirs, outDir string, check bool) error { } } fmt.Printf( - "fanout-docgen: wrote %d page(s) covering %d setting(s), %d alert variable(s) and %d route(s)\n", - len(pages), len(fields), len(alertEnvCount), len(routeCount), + "fanout-docgen: wrote %d page(s) covering %d setting(s), %d alert variable(s), %d route(s) and %d MCP tool(s)\n", + len(pages), len(fields), len(alertEnvCount), len(routeCount), len(toolCount), ) return nil } diff --git a/cmd/fanout-docgen/mcptools.go b/cmd/fanout-docgen/mcptools.go new file mode 100644 index 00000000..dffe9ae8 --- /dev/null +++ b/cmd/fanout-docgen/mcptools.go @@ -0,0 +1,156 @@ +package main + +import ( + "context" + "fmt" + "strings" + + "github.com/labstack/fanout/internal/mcp" +) + +// toolCount is set by renderMCPTools so the summary line can report it. +var toolCount []mcp.ToolDoc + +// renderMCPTools writes the MCP reference from the server's own `tools/list` +// answer. +// +// The page it replaces was hand-written, and its risk was the same one that had +// already bitten the roles page: it described a tool surface a reader would act +// on, with nothing keeping it level with the server. A tool renamed, or given an +// input, or re-annotated from additive to destructive, would have left the page +// confidently wrong — and "this tool does not modify anything" is a claim +// someone connects an agent on. +func renderMCPTools() ([]byte, error) { + tools, err := mcp.DescribeTools(context.Background()) + if err != nil { + return nil, err + } + + var b strings.Builder + + b.WriteString("---\n") + b.WriteString("title: \"MCP tools\"\n") + b.WriteString("description: \"Every tool a Fanout instance exposes over MCP, with its inputs and whether it changes anything.\"\n") + fmt.Fprintf(&b, + "summary: \"The %d tools served at /mcp, each with its inputs and mutation semantics, taken from the server's own tools/list answer.\"\n", + len(tools), + ) + b.WriteString("read_when:\n") + b.WriteString(" - \"You are connecting an agent and want to know what it can call.\"\n") + b.WriteString(" - \"You need to know which tools mutate state and which cannot.\"\n") + b.WriteString("status: preview\n") + b.WriteString("generated: true\n") + b.WriteString("---\n\n") + + b.WriteString("{/* Generated by cmd/fanout-docgen, which asks a running MCP server for its\n") + b.WriteString(" tool list rather than reading the registrations. Edit the generator, not\n") + b.WriteString(" this page. */}\n\n") + + fmt.Fprintf(&b, + "A Fanout instance serves MCP at `/mcp`. %s, listed below exactly as the\nserver reports them to a connecting client.\n\n", + count(len(tools), "tool"), + ) + + b.WriteString("Every tool is **closed-world**: it reads or writes what this instance holds\n") + b.WriteString("and reaches nothing else.\n\n") + + b.WriteString("| Tool | Changes state |\n") + b.WriteString("|---|---|\n") + for _, tool := range tools { + fmt.Fprintf(&b, "| [`%s`](#%s) | %s |\n", tool.Name, tool.Name, effect(tool)) + } + b.WriteString("\n") + + for _, tool := range tools { + fmt.Fprintf(&b, "## %s\n\n", tool.Name) + fmt.Fprintf(&b, "**%s** — %s\n\n", tool.Title, tool.Description) + fmt.Fprintf(&b, "%s\n\n", effectProse(tool)) + + if len(tool.Inputs) == 0 { + b.WriteString("Takes no arguments.\n\n") + continue + } + + b.WriteString("| Input | Type | Required | Means |\n") + b.WriteString("|---|---|---|---|\n") + for _, input := range tool.Inputs { + required := "no" + if input.Required { + required = "**yes**" + } + description := input.Description + if description == "" { + description = "—" + } + fmt.Fprintf(&b, "| `%s` | `%s` | %s | %s |\n", + input.Name, input.Type, required, description) + } + b.WriteString("\n") + } + + b.WriteString("## Authentication\n\n") + b.WriteString("MCP uses OAuth, not a static key, and **not the ingest token** — an agent\n") + b.WriteString("presenting the ingest token is rejected. A client discovers the endpoints,\n") + b.WriteString("registers itself, and is issued a token bound to this instance's resource\n") + b.WriteString("URI. [Connect an agent](/guides/connect-over-mcp) covers the flow.\n\n") + + b.WriteString("The dashboard tools act on the authenticated user's **own** dashboards. The\n") + b.WriteString("capability boundary is `dashboards:manage-own`: an agent acts as the user\n") + b.WriteString("whose credential it holds and cannot reach anyone else's. A tool description\n") + b.WriteString("that tells a model when to call it is guidance to the model, not a\n") + b.WriteString("permission check — see [roles](/reference/roles).\n\n") + + b.WriteString("## What is not here\n\n") + b.WriteString("No tool writes telemetry, changes configuration, manages users, or edits\n") + b.WriteString("alert rules. Ingest is OTLP only and the rest is HTTP surface — see\n") + b.WriteString("[HTTP routes](/reference/http-routes). An agent connected over MCP can read\n") + b.WriteString("everything the instance knows and manage its own dashboards, and that is the\n") + b.WriteString("whole envelope.\n") + + toolCount = tools + return []byte(b.String()), nil +} + +// effect renders a tool's mutation semantics for the summary table, from the +// annotations the server sends rather than from the tool's name. +func effect(tool mcp.ToolDoc) string { + if tool.ReadOnly { + return "No — read-only" + } + if tool.Destructive { + return "**Yes — replaces existing state**" + } + return "**Yes — additive**" +} + +func effectProse(tool mcp.ToolDoc) string { + switch { + case tool.ReadOnly: + return "Read-only. Calling it changes nothing." + case tool.Destructive && tool.Idempotent: + return "**Replaces existing state.** Calling it twice with the same arguments " + + "leaves the same result as calling it once, but the first call has already " + + "overwritten what was there." + case tool.Destructive: + return "**Replaces existing state.**" + default: + return "**Additive.** It creates something new and alters nothing that exists." + } +} + +// count renders "One tool" / "Nine tools" so the opening line reads as prose +// rather than as a field. +func count(n int, noun string) string { + words := []string{ + "No", "One", "Two", "Three", "Four", "Five", + "Six", "Seven", "Eight", "Nine", "Ten", "Eleven", "Twelve", + } + plural := noun + "s" + if n == 1 { + plural = noun + } + if n < len(words) { + return words[n] + " " + plural + } + return fmt.Sprintf("%d %s", n, plural) +} diff --git a/internal/mcp/describe.go b/internal/mcp/describe.go new file mode 100644 index 00000000..f01111ab --- /dev/null +++ b/internal/mcp/describe.go @@ -0,0 +1,244 @@ +package mcp + +import ( + "context" + "fmt" + "sort" + "strings" + + "github.com/modelcontextprotocol/go-sdk/mcp" + + "github.com/labstack/fanout/internal/dashboard" +) + +// This file exists so the MCP reference can be generated from the server itself +// rather than from a description of it. +// +// 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 — then publishes the answer. A generator that parsed +// registerTools would be describing 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. +// +// Nothing here decides anything. If one of these disagrees with the server, the +// accessor is wrong, because the server is what agents talk to. + +// ToolDoc is what a reader needs before pointing an agent at a tool: what it is +// called, what it does, whether it changes anything, and what it accepts. +type ToolDoc struct { + Name string + Title string + Description string + + // ReadOnly is the server's own readOnlyHint. + ReadOnly bool + // Destructive is meaningful only when ReadOnly is false. The MCP spec + // defaults it to true when the server sends no hint, which is the safe + // reading and the one published. + Destructive bool + // Idempotent is meaningful only when ReadOnly is false. + Idempotent bool + // OpenWorld reports whether the tool may reach beyond this instance. The + // spec defaults it to true when absent. + OpenWorld bool + + Inputs []ToolInput +} + +// ToolInput is one top-level parameter. 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 and a client reads the schema for the rest. +type ToolInput struct { + Name string + Type string + Description string + Required bool +} + +// DescribeTools reports every tool this server exposes, as the server itself +// reports them over MCP. +// +// The server is constructed with no query backend and an empty dashboard +// service because registration touches neither: the tool set is fixed at +// construction and no handler runs here. A nil dashboard service would register +// no dashboard tools at all, so one is supplied — otherwise this would quietly +// document a smaller surface than an instance serves. +func DescribeTools(ctx context.Context) ([]ToolDoc, error) { + server := New(nil, dashboard.New(nil), "docgen") + + serverTransport, clientTransport := mcp.NewInMemoryTransports() + + type connection struct { + session *mcp.ServerSession + err error + } + // Buffered, so the goroutine cannot block if this returns before reading it. + connected := make(chan connection, 1) + go func() { + session, err := server.MCP().Connect(ctx, serverTransport, nil) + connected <- connection{session: session, err: err} + }() + + client := mcp.NewClient(&mcp.Implementation{Name: "fanout-docgen", Version: "docgen"}, nil) + session, err := client.Connect(ctx, clientTransport, nil) + if err != nil { + return nil, fmt.Errorf("connecting to the MCP server: %w", err) + } + defer func() { _ = session.Close() }() + + served := <-connected + if served.err != nil { + return nil, fmt.Errorf("serving the in-memory MCP transport: %w", served.err) + } + // Closed explicitly rather than left to the client's close: this is called + // repeatedly by tests, and a server session per call would accumulate. + defer func() { _ = served.session.Close() }() + + listed, err := session.ListTools(ctx, nil) + if err != nil { + return nil, fmt.Errorf("listing tools: %w", err) + } + if len(listed.Tools) == 0 { + // An empty reference reads as "this instance exposes no tools", which is + // a stronger and wronger claim than a missing page. + return nil, fmt.Errorf("the MCP server reported no tools; has registration changed?") + } + + docs := make([]ToolDoc, 0, len(listed.Tools)) + for _, tool := range listed.Tools { + doc := ToolDoc{ + Name: tool.Name, + Title: tool.Title, + Description: tool.Description, + // Absent hints default to true per the MCP spec. Reading them as + // false would publish a mutating tool as safe. + Destructive: true, + OpenWorld: true, + } + if a := tool.Annotations; a != nil { + doc.ReadOnly = a.ReadOnlyHint + doc.Idempotent = a.IdempotentHint + if a.DestructiveHint != nil { + doc.Destructive = *a.DestructiveHint + } + if a.OpenWorldHint != nil { + doc.OpenWorld = *a.OpenWorldHint + } + } + if doc.Description == "" { + return nil, fmt.Errorf( + "tool %s has no description; a calling model chooses tools by description, "+ + "so an empty one is a bug rather than a blank cell", + tool.Name, + ) + } + + inputs, err := toolInputs(tool.Name, tool.InputSchema) + if err != nil { + return nil, err + } + doc.Inputs = inputs + + docs = append(docs, doc) + } + + sort.Slice(docs, func(i, j int) bool { return docs[i].Name < docs[j].Name }) + return docs, nil +} + +// toolInputs reads a tool's top-level input properties out of its generated +// JSON Schema. +// +// The schema arrives as a map rather than a typed value: Tool.InputSchema is +// `any`, and a client receives it having been through JSON. So this reads it as +// what it is, and refuses anything it does not recognise rather than treating an +// unreadable schema as "no inputs" — a tool published as taking nothing when it +// requires an argument is worse than no page. +func toolInputs(toolName string, raw any) ([]ToolInput, error) { + if raw == nil { + return nil, nil + } + schema, ok := raw.(map[string]any) + if !ok { + return nil, fmt.Errorf( + "tool %s: input schema is %T, not a JSON object; the reference cannot describe its parameters", + toolName, raw, + ) + } + + properties, ok := schema["properties"] + if !ok { + // A tool that genuinely takes nothing, e.g. dashboard_list. + return nil, nil + } + fields, ok := properties.(map[string]any) + if !ok { + return nil, fmt.Errorf("tool %s: schema properties are %T, not a JSON object", toolName, properties) + } + + required := map[string]bool{} + if list, ok := schema["required"].([]any); ok { + for _, name := range list { + if s, ok := name.(string); ok { + required[s] = true + } + } + } + + inputs := make([]ToolInput, 0, len(fields)) + for name, raw := range fields { + property, ok := raw.(map[string]any) + if !ok { + return nil, fmt.Errorf( + "tool %s: input %q has schema %T, not a JSON object, so it would publish as a bare name", + toolName, name, raw, + ) + } + description, _ := property["description"].(string) + inputs = append(inputs, ToolInput{ + Name: name, + Type: schemaType(property), + Description: description, + Required: required[name], + }) + } + + // Required first, then alphabetical: someone wiring up a call needs the + // mandatory inputs before the optional ones. + sort.Slice(inputs, func(i, j int) bool { + if inputs[i].Required != inputs[j].Required { + return inputs[i].Required + } + return inputs[i].Name < inputs[j].Name + }) + return inputs, nil +} + +// schemaType renders a property's type for a table cell. +// +// JSON Schema allows a type to be a string or a list, and the SDK emits a list +// for a nullable field — a nullable array arrives as ["null","array"]. The null +// is dropped because every optional input is nullable and saying so in every row +// carries no information; what remains is rendered as a union rather than +// letting the first member silently win. +func schemaType(property map[string]any) string { + switch t := property["type"].(type) { + case string: + return t + case []any: + out := make([]string, 0, len(t)) + for _, entry := range t { + name, ok := entry.(string) + if !ok || name == "null" { + continue + } + out = append(out, name) + } + if len(out) > 0 { + return strings.Join(out, " or ") + } + } + return "any" +} diff --git a/internal/mcp/describe_test.go b/internal/mcp/describe_test.go new file mode 100644 index 00000000..3ebaa9b6 --- /dev/null +++ b/internal/mcp/describe_test.go @@ -0,0 +1,125 @@ +package mcp + +import ( + "context" + "testing" + + "github.com/labstack/fanout/internal/dashboard" +) + +// DescribeTools must agree with what a real client sees, because that agreement +// is the only reason to generate the page from it rather than write it. +func TestDescribeToolsMatchesWhatAClientIsServed(t *testing.T) { + docs, err := DescribeTools(context.Background()) + if err != nil { + t.Fatalf("DescribeTools: %v", err) + } + + server := New(nil, dashboard.New(nil), "test") + session := connectTestClient(t, server, nil) + listed, err := session.ListTools(context.Background(), nil) + if err != nil { + t.Fatalf("ListTools: %v", err) + } + + if len(docs) != len(listed.Tools) { + t.Fatalf("DescribeTools reported %d tools, a client is served %d", len(docs), len(listed.Tools)) + } + + served := map[string]string{} + for _, tool := range listed.Tools { + served[tool.Name] = tool.Description + } + for _, doc := range docs { + description, ok := served[doc.Name] + if !ok { + t.Errorf("DescribeTools reports %q, which no client is served", doc.Name) + continue + } + if doc.Description != description { + t.Errorf("%s description differs from the one served", doc.Name) + } + } +} + +// The dashboard tools register only when a dashboard service is present. If +// DescribeTools ever passed nil, the reference would silently lose four tools — +// two of which mutate — while still reading as complete. +func TestDescribeToolsIncludesTheDashboardTools(t *testing.T) { + docs, err := DescribeTools(context.Background()) + if err != nil { + t.Fatalf("DescribeTools: %v", err) + } + + found := map[string]ToolDoc{} + for _, doc := range docs { + found[doc.Name] = doc + } + + for _, name := range []string{"dashboard_list", "dashboard_get", "dashboard_create", "dashboard_update"} { + if _, ok := found[name]; !ok { + t.Errorf("%s is registered but absent from DescribeTools", name) + } + } + + // dashboard_update replaces rather than merges, and that is the single most + // important fact on the page. + update, ok := found["dashboard_update"] + if !ok { + t.Fatal("dashboard_update missing") + } + if update.ReadOnly { + t.Error("dashboard_update reported read-only") + } + if !update.Destructive { + t.Error("dashboard_update reported non-destructive; it replaces a dashboard's design") + } +} + +// Every tool must arrive with the facts the page is built from. A blank cell +// reads as "nothing to say" rather than "not generated". +func TestDescribeToolsReportsCompleteMetadata(t *testing.T) { + docs, err := DescribeTools(context.Background()) + if err != nil { + t.Fatalf("DescribeTools: %v", err) + } + + for _, doc := range docs { + if doc.Name == "" || doc.Title == "" || doc.Description == "" { + t.Errorf("%+v: incomplete metadata", doc) + } + if doc.OpenWorld { + t.Errorf("%s reports open-world; every Fanout tool is closed-world", doc.Name) + } + for _, input := range doc.Inputs { + if input.Type == "" { + t.Errorf("%s input %q has no type", doc.Name, input.Name) + } + } + } +} + +// The observability tools share one scope input, and the page says so. If the +// shared struct gained or lost a field, the claim would go stale silently. +func TestDescribeToolsReportsTheSharedObservabilityScope(t *testing.T) { + docs, err := DescribeTools(context.Background()) + if err != nil { + t.Fatalf("DescribeTools: %v", err) + } + + want := map[string]string{"window": "string", "namespace": "string", "limit": "integer"} + for _, doc := range docs { + if doc.Name != "observability_overview" && doc.Name != "service_topology" { + continue + } + got := map[string]string{} + for _, input := range doc.Inputs { + got[input.Name] = input.Type + } + for name, typ := range want { + if got[name] != typ { + t.Errorf("%s input %q is %q, want %q", doc.Name, name, got[name], typ) + } + } + } +} diff --git a/site/src/content/docs/reference/mcp-tools.mdx b/site/src/content/docs/reference/mcp-tools.mdx index bd9cc36a..d06627c1 100644 --- a/site/src/content/docs/reference/mcp-tools.mdx +++ b/site/src/content/docs/reference/mcp-tools.mdx @@ -1,71 +1,162 @@ --- -title: MCP tools -description: Every tool a Fanout instance exposes over MCP, with its inputs and whether it mutates. -summary: Five read-only observability tools and four dashboard tools, with the shared window/namespace/limit inputs. +title: "MCP tools" +description: "Every tool a Fanout instance exposes over MCP, with its inputs and whether it changes anything." +summary: "The 9 tools served at /mcp, each with its inputs and mutation semantics, taken from the server's own tools/list answer." read_when: - - You are connecting an agent and want to know what it can call. - - You need to know which tools mutate state and which cannot. + - "You are connecting an agent and want to know what it can call." + - "You need to know which tools mutate state and which cannot." status: preview +generated: true --- -A Fanout instance serves MCP at `/mcp`. Nine tools, in two groups. +{/* Generated by cmd/fanout-docgen, which asks a running MCP server for its + tool list rather than reading the registrations. Edit the generator, not + this page. */} -## Observability +A Fanout instance serves MCP at `/mcp`. Nine tools, listed below exactly as the +server reports them to a connecting client. -All five are read-only and closed-world — they read the telemetry this instance -holds and reach nothing else. +Every tool is **closed-world**: it reads or writes what this instance holds +and reaches nothing else. -| Tool | Returns | +| Tool | Changes state | |---|---| -| `observability_overview` | Service health for a bounded window. The triage starting point. | -| `service_topology` | Services and observed dependency edges, with health, traffic, latency and errors. | -| `service_performance` | Activity, errors, latency, endpoints, cross-signal correlation and change over time, for one service or all. | -| `trace_detail` | One exact trace, or the most relevant recent error or slow trace — spans, waterfall, flame graph and correlated logs. | -| `search_logs` | Filtered logs with a severity timeline and links back to correlated traces. | +| [`dashboard_create`](#dashboard_create) | **Yes — additive** | +| [`dashboard_get`](#dashboard_get) | No — read-only | +| [`dashboard_list`](#dashboard_list) | No — read-only | +| [`dashboard_update`](#dashboard_update) | **Yes — replaces existing state** | +| [`observability_overview`](#observability_overview) | No — read-only | +| [`search_logs`](#search_logs) | No — read-only | +| [`service_performance`](#service_performance) | No — read-only | +| [`service_topology`](#service_topology) | No — read-only | +| [`trace_detail`](#trace_detail) | No — read-only | -### Shared inputs +## dashboard_create -Every observability tool takes the same scope, and every field is optional: +**Create dashboard** — Create a complete named dashboard for the authenticated user. This is additive and does not alter existing dashboards. -| Input | Means | -|---|---| -| `window` | Time window such as `15m`, `1h`, `24h`. Defaults to `1h`. | -| `namespace` | OpenTelemetry service namespace. Omit it to span every namespace. | -| `limit` | Maximum services, edges or endpoints to return, 1–500. | +**Additive.** It creates something new and alters nothing that exists. -`service_performance` and `trace_detail` additionally accept `service`, an exact -OpenTelemetry service name — omit it for the whole system. +| Input | Type | Required | Means | +|---|---|---|---| +| `name` | `string` | **yes** | Short, unique dashboard name | +| `state` | `object` | **yes** | Complete widget registry, 12-column layout, and shared filters | +| `description` | `string` | no | Concise purpose of this dashboard | -## Dashboards +## dashboard_get -These act on the **authenticated user's own** dashboards, and two of them -mutate: +**Get dashboard** — Read one named dashboard, including its widgets, filters, and 12-column layout. -| Tool | Effect | -|---|---| -| `dashboard_list` | Read — named dashboards and widget counts | -| `dashboard_get` | Read — one dashboard, with widgets, filters and 12-column layout | -| `dashboard_create` | **Additive** — creates a new dashboard, alters no existing one | -| `dashboard_update` | **Replacing** — overwrites a dashboard's name, widgets, filters and layout | - -`dashboard_update` replaces rather than merges. Its own description tells a -calling agent to use it only after the user has explicitly asked to change that -dashboard, which is the behaviour to expect from a well-behaved client — but -it is guidance to the model, not a permission check. The capability boundary is -`dashboards:manage-own`: an agent acts as the user whose credential it holds and -cannot reach anyone else's dashboards. +Read-only. Calling it changes nothing. + +| Input | Type | Required | Means | +|---|---|---|---| +| `id` | `string` | **yes** | Dashboard ID returned by dashboard_list or dashboard_create | + +## dashboard_list + +**List dashboards** — List the authenticated user's named dashboards and widget counts before creating or changing one. + +Read-only. Calling it changes nothing. + +Takes no arguments. + +## dashboard_update + +**Replace dashboard design** — Replace an existing dashboard's name, widgets, shared filters, and layout. Only call after the user explicitly asks to change that dashboard. + +**Replaces existing state.** Calling it twice with the same arguments leaves the same result as calling it once, but the first call has already overwritten what was there. + +| Input | Type | Required | Means | +|---|---|---|---| +| `id` | `string` | **yes** | Dashboard ID to update | +| `name` | `string` | **yes** | Short, unique dashboard name | +| `state` | `object` | **yes** | Complete replacement widget registry, 12-column layout, and shared filters | +| `description` | `string` | no | Concise purpose of this dashboard | + +## observability_overview + +**System health overview** — Summarize service health for a bounded telemetry window. Start here for incident triage. + +Read-only. Calling it changes nothing. + +| Input | Type | Required | Means | +|---|---|---|---| +| `limit` | `integer` | no | Maximum services or edges to return, from 1 to 500 | +| `namespace` | `string` | no | OpenTelemetry service namespace; empty queries all namespaces | +| `window` | `string` | no | Time window such as 15m, 1h, or 24h; defaults to 1h | + +## search_logs + +**Log explorer** — Search and filter logs with a severity timeline and links back to correlated traces. + +Read-only. Calling it changes nothing. + +| Input | Type | Required | Means | +|---|---|---|---| +| `limit` | `integer` | no | Maximum log entries to return, from 1 to 500 | +| `namespace` | `string` | no | OpenTelemetry service namespace; empty queries all namespaces | +| `search` | `string` | no | Optional case-insensitive text contained in the log body | +| `service` | `string` | no | Optional exact OpenTelemetry service name | +| `severity` | `string` | no | Optional exact severity such as ERROR, WARN, or INFO | +| `window` | `string` | no | Time window such as 15m, 1h, or 24h; defaults to 1h | + +## service_performance + +**Service performance explorer** — Inspect activity, errors, latency, endpoints, cross-signal correlation, and change over time for one service or the system. + +Read-only. Calling it changes nothing. + +| Input | Type | Required | Means | +|---|---|---|---| +| `limit` | `integer` | no | Maximum endpoints to return, from 1 to 500 | +| `namespace` | `string` | no | OpenTelemetry service namespace; empty queries all namespaces | +| `service` | `string` | no | Optional exact OpenTelemetry service name; omit for the whole system | +| `window` | `string` | no | Time window such as 15m, 1h, or 24h; defaults to 1h | + +## service_topology + +**Service dependency topology** — Return services and observed dependency edges with health, traffic, latency, and error data. + +Read-only. Calling it changes nothing. + +| Input | Type | Required | Means | +|---|---|---|---| +| `limit` | `integer` | no | Maximum services or edges to return, from 1 to 500 | +| `namespace` | `string` | no | OpenTelemetry service namespace; empty queries all namespaces | +| `window` | `string` | no | Time window such as 15m, 1h, or 24h; defaults to 1h | + +## trace_detail + +**Trace detail** — Inspect an exact trace, or select the most relevant recent error or slow trace, with spans, waterfall, flame graph, and correlated logs. + +Read-only. Calling it changes nothing. + +| Input | Type | Required | Means | +|---|---|---|---| +| `limit` | `integer` | no | Maximum spans and correlated logs to return, from 1 to 500 | +| `namespace` | `string` | no | OpenTelemetry service namespace; empty queries all namespaces | +| `service` | `string` | no | Optional service filter when choosing a recent trace | +| `trace_id` | `string` | no | Exact trace ID; omit to inspect the most relevant recent error or slow trace | +| `window` | `string` | no | Trace lookup window such as 1h or 24h; defaults to 1h | ## Authentication MCP uses OAuth, not a static key, and **not the ingest token** — an agent presenting the ingest token is rejected. A client discovers the endpoints, -registers itself, and is issued a token bound to this instance's resource URI. -[Connect an agent](/guides/connect-over-mcp) covers the flow and the two scopes. +registers itself, and is issued a token bound to this instance's resource +URI. [Connect an agent](/guides/connect-over-mcp) covers the flow. + +The dashboard tools act on the authenticated user's **own** dashboards. The +capability boundary is `dashboards:manage-own`: an agent acts as the user +whose credential it holds and cannot reach anyone else's. A tool description +that tells a model when to call it is guidance to the model, not a +permission check — see [roles](/reference/roles). ## What is not here -There is no tool that writes telemetry, changes configuration, manages users, or -edits alert rules. Ingest is OTLP only, and the rest is HTTP API surface — see -[endpoints](/reference/endpoints). An agent connected over MCP can read +No tool writes telemetry, changes configuration, manages users, or edits +alert rules. Ingest is OTLP only and the rest is HTTP surface — see +[HTTP routes](/reference/http-routes). An agent connected over MCP can read everything the instance knows and manage its own dashboards, and that is the whole envelope. From 7a0385fbbb79bfdbcced7594c5920afbc39b7a2d Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Mon, 24 Aug 2026 07:57:02 -0700 Subject: [PATCH 2/3] docs: check the MCP page's claims instead of asserting them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 with {braces}` in a description — which now renders as one cell of literal text and builds. --- cmd/fanout-docgen/mcptools.go | 55 +++++++++++++++++-- internal/mcp/describe.go | 30 +++++++++- internal/mcp/describe_test.go | 29 ++++++++-- site/src/content/docs/reference/mcp-tools.mdx | 2 +- 4 files changed, 103 insertions(+), 13 deletions(-) diff --git a/cmd/fanout-docgen/mcptools.go b/cmd/fanout-docgen/mcptools.go index dffe9ae8..b840162d 100644 --- a/cmd/fanout-docgen/mcptools.go +++ b/cmd/fanout-docgen/mcptools.go @@ -32,8 +32,8 @@ func renderMCPTools() ([]byte, error) { b.WriteString("title: \"MCP tools\"\n") b.WriteString("description: \"Every tool a Fanout instance exposes over MCP, with its inputs and whether it changes anything.\"\n") fmt.Fprintf(&b, - "summary: \"The %d tools served at /mcp, each with its inputs and mutation semantics, taken from the server's own tools/list answer.\"\n", - len(tools), + "summary: \"%s served at /mcp, each with its inputs and mutation semantics, taken from the server's own tools/list answer.\"\n", + count(len(tools), "tool"), ) b.WriteString("read_when:\n") b.WriteString(" - \"You are connecting an agent and want to know what it can call.\"\n") @@ -51,6 +51,24 @@ func renderMCPTools() ([]byte, error) { count(len(tools), "tool"), ) + // The closed-world sentence below is a safety claim a reader acts on. It is + // asserted in prose, so it has to be checked against the annotations rather + // than trusted: a tool registered without an openWorldHint defaults to + // open-world per the spec, and would otherwise be republished under a + // blanket promise that it reaches nothing else. --check cannot catch this, + // because a regenerated page carrying the same false claim is + // self-consistent. + for _, tool := range tools { + if tool.OpenWorld { + return nil, fmt.Errorf( + "tool %s is open-world (it may reach beyond this instance), but the page "+ + "states that every tool is closed-world; either annotate it "+ + "OpenWorldHint:false in internal/mcp or stop making the claim for all tools", + tool.Name, + ) + } + } + b.WriteString("Every tool is **closed-world**: it reads or writes what this instance holds\n") b.WriteString("and reaches nothing else.\n\n") @@ -63,7 +81,7 @@ func renderMCPTools() ([]byte, error) { for _, tool := range tools { fmt.Fprintf(&b, "## %s\n\n", tool.Name) - fmt.Fprintf(&b, "**%s** — %s\n\n", tool.Title, tool.Description) + fmt.Fprintf(&b, "**%s** — %s\n\n", mdx(tool.Title), mdx(tool.Description)) fmt.Fprintf(&b, "%s\n\n", effectProse(tool)) if len(tool.Inputs) == 0 { @@ -83,7 +101,7 @@ func renderMCPTools() ([]byte, error) { description = "—" } fmt.Fprintf(&b, "| `%s` | `%s` | %s | %s |\n", - input.Name, input.Type, required, description) + input.Name, input.Type, required, cell(description)) } b.WriteString("\n") } @@ -154,3 +172,32 @@ func count(n int, noun string) string { } return fmt.Sprintf("%d %s", n, plural) } + +// mdx escapes a value interpolated into MDX prose. +// +// Tool titles and descriptions are authored in Go string literals and reach this +// page verbatim. A description containing `<` or `{` would break the MDX build, +// and a backslash would be eaten — so the characters MDX treats as syntax are +// escaped rather than left to chance. Newlines are collapsed because a hard +// break inside a table cell ends the row. +func mdx(value string) string { + replacer := strings.NewReplacer( + "\\", "\\\\", + "<", "\\<", + "{", "\\{", + "\r\n", " ", + "\n", " ", + "\r", " ", + ) + return strings.TrimSpace(replacer.Replace(value)) +} + +// cell escapes a value for a Markdown table cell. +// +// On top of mdx's escaping, a literal pipe has to be escaped or it splits the +// row into phantom columns — output that is wrong rather than broken, so nothing +// downstream would catch it: check-tables.mjs asserts that tables are wrapped, +// not that their rows have the right shape. +func cell(value string) string { + return strings.ReplaceAll(mdx(value), "|", "\\|") +} diff --git a/internal/mcp/describe.go b/internal/mcp/describe.go index f01111ab..c1bfd20d 100644 --- a/internal/mcp/describe.go +++ b/internal/mcp/describe.go @@ -5,6 +5,7 @@ import ( "fmt" "sort" "strings" + "time" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -66,6 +67,12 @@ type ToolInput struct { // no dashboard tools at all, so one is supplied — otherwise this would quietly // document a smaller surface than an instance serves. func DescribeTools(ctx context.Context) ([]ToolDoc, error) { + // Bounded so a handshake that never completes fails the build instead of + // hanging it. The transport is in-memory and this should take microseconds; + // the timeout is a backstop, not a budget. + ctx, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() + server := New(nil, dashboard.New(nil), "docgen") serverTransport, clientTransport := mcp.NewInMemoryTransports() @@ -84,17 +91,25 @@ func DescribeTools(ctx context.Context) ([]ToolDoc, error) { client := mcp.NewClient(&mcp.Implementation{Name: "fanout-docgen", Version: "docgen"}, nil) session, err := client.Connect(ctx, clientTransport, nil) if err != nil { + // The goroutine may still have produced a session. Closing it here is the + // same leak the deferred close below exists to prevent — this path is just + // the one that returns before reaching it. + if served := <-connected; served.session != nil { + _ = served.session.Close() + } return nil, fmt.Errorf("connecting to the MCP server: %w", err) } defer func() { _ = session.Close() }() served := <-connected + if served.session != nil { + // Closed explicitly rather than left to the client's close: this is called + // repeatedly by tests, and a server session per call would accumulate. + defer func() { _ = served.session.Close() }() + } if served.err != nil { return nil, fmt.Errorf("serving the in-memory MCP transport: %w", served.err) } - // Closed explicitly rather than left to the client's close: this is called - // repeatedly by tests, and a server session per call would accumulate. - defer func() { _ = served.session.Close() }() listed, err := session.ListTools(ctx, nil) if err != nil { @@ -134,6 +149,15 @@ func DescribeTools(ctx context.Context) ([]ToolDoc, error) { tool.Name, ) } + if doc.Title == "" { + // The reference renders the title as `**%s** — description`, so an + // empty one publishes a heading line starting with a bare `****`. + return nil, fmt.Errorf( + "tool %s has no title; the reference renders one for every tool and an "+ + "empty one is published as stray emphasis", + tool.Name, + ) + } inputs, err := toolInputs(tool.Name, tool.InputSchema) if err != nil { diff --git a/internal/mcp/describe_test.go b/internal/mcp/describe_test.go index 3ebaa9b6..1aa16eec 100644 --- a/internal/mcp/describe_test.go +++ b/internal/mcp/describe_test.go @@ -107,18 +107,37 @@ func TestDescribeToolsReportsTheSharedObservabilityScope(t *testing.T) { t.Fatalf("DescribeTools: %v", err) } + // Compared exactly, not as a subset: a field added to QueryInput would + // otherwise pass while the page's claim that these two share one scope went + // stale. And each tool must actually be seen — filtering by name and + // asserting nothing when the filter matches nothing is a test that renaming + // a tool would silently switch off. want := map[string]string{"window": "string", "namespace": "string", "limit": "integer"} - for _, doc := range docs { - if doc.Name != "observability_overview" && doc.Name != "service_topology" { + + for _, name := range []string{"observability_overview", "service_topology"} { + var doc ToolDoc + var found bool + for _, candidate := range docs { + if candidate.Name == name { + doc, found = candidate, true + break + } + } + if !found { + t.Errorf("%s is not among the served tools; was it renamed?", name) continue } + got := map[string]string{} for _, input := range doc.Inputs { got[input.Name] = input.Type } - for name, typ := range want { - if got[name] != typ { - t.Errorf("%s input %q is %q, want %q", doc.Name, name, got[name], typ) + if len(got) != len(want) { + t.Errorf("%s takes %d inputs (%v), want exactly %v", name, len(got), got, want) + } + for input, typ := range want { + if got[input] != typ { + t.Errorf("%s input %q is %q, want %q", name, input, got[input], typ) } } } diff --git a/site/src/content/docs/reference/mcp-tools.mdx b/site/src/content/docs/reference/mcp-tools.mdx index d06627c1..641fa779 100644 --- a/site/src/content/docs/reference/mcp-tools.mdx +++ b/site/src/content/docs/reference/mcp-tools.mdx @@ -1,7 +1,7 @@ --- title: "MCP tools" description: "Every tool a Fanout instance exposes over MCP, with its inputs and whether it changes anything." -summary: "The 9 tools served at /mcp, each with its inputs and mutation semantics, taken from the server's own tools/list answer." +summary: "Nine tools served at /mcp, each with its inputs and mutation semantics, taken from the server's own tools/list answer." read_when: - "You are connecting an agent and want to know what it can call." - "You need to know which tools mutate state and which cannot." From d6b2e4c303d8d63a38421a17f9b5ae4131bb366b Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Mon, 24 Aug 2026 08:44:53 -0700 Subject: [PATCH 3/3] docs: fail on a schema or tool name the MCP page cannot render MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .github/workflows/site.yml | 4 ++-- cmd/fanout-docgen/mcptools.go | 35 +++++++++++++++++++++++++++++ internal/mcp/describe.go | 34 +++++++++++++++++++++++----- internal/mcp/describe_test.go | 42 +++++++++++++++++++++++++++++++++++ 4 files changed, 108 insertions(+), 7 deletions(-) diff --git a/.github/workflows/site.yml b/.github/workflows/site.yml index 435e07fa..0b194dab 100644 --- a/.github/workflows/site.yml +++ b/.github/workflows/site.yml @@ -68,8 +68,8 @@ jobs: cache: npm cache-dependency-path: site/package-lock.json - # The reference pages are generated from internal/config, internal/alert - # and internal/api, and committed. This workflow triggers on those paths + # The reference pages are generated from the sources listed in this file's + # trigger paths above, and committed. This workflow triggers on those paths # precisely because they regenerate it — so publishing without checking # would ship a settings page describing a setting the binary rejects, # which is the gap this file's header says it exists to close. Restored diff --git a/cmd/fanout-docgen/mcptools.go b/cmd/fanout-docgen/mcptools.go index b840162d..12faed41 100644 --- a/cmd/fanout-docgen/mcptools.go +++ b/cmd/fanout-docgen/mcptools.go @@ -72,6 +72,23 @@ func renderMCPTools() ([]byte, error) { b.WriteString("Every tool is **closed-world**: it reads or writes what this instance holds\n") b.WriteString("and reaches nothing else.\n\n") + // Each row links to the tool's own section, whose heading is the tool name. + // That only works while the name is already what a slugger would produce + // from it: a name with an uppercase letter or a dot slugs to something else + // and every row link on the page goes dead. Nothing downstream catches a + // dead in-page anchor, so the assumption is checked where it is made. + for _, tool := range tools { + if !anchorSafe(tool.Name) { + return nil, fmt.Errorf( + "tool name %q is not usable as a heading anchor, so the summary table's "+ + "link to its section would not resolve; either keep tool names to "+ + "lowercase letters, digits and underscores, or slug the anchor in "+ + "cmd/fanout-docgen/mcptools.go", + tool.Name, + ) + } + } + b.WriteString("| Tool | Changes state |\n") b.WriteString("|---|---|\n") for _, tool := range tools { @@ -201,3 +218,21 @@ func mdx(value string) string { func cell(value string) string { return strings.ReplaceAll(mdx(value), "|", "\\|") } + +// anchorSafe reports whether a name is already what a heading slugger would +// produce from it, so `## name` and `](#name)` agree. +func anchorSafe(name string) bool { + if name == "" { + return false + } + for _, r := range name { + switch { + case r >= 'a' && r <= 'z': + case r >= '0' && r <= '9': + case r == '_' || r == '-': + default: + return false + } + } + return true +} diff --git a/internal/mcp/describe.go b/internal/mcp/describe.go index c1bfd20d..7190fe36 100644 --- a/internal/mcp/describe.go +++ b/internal/mcp/describe.go @@ -221,9 +221,13 @@ func toolInputs(toolName string, raw any) ([]ToolInput, error) { ) } description, _ := property["description"].(string) + typ, err := schemaType(property) + if err != nil { + return nil, fmt.Errorf("tool %s: input %q: %w", toolName, name, err) + } inputs = append(inputs, ToolInput{ Name: name, - Type: schemaType(property), + Type: typ, Description: description, Required: required[name], }) @@ -247,10 +251,19 @@ func toolInputs(toolName string, raw any) ([]ToolInput, error) { // is dropped because every optional input is nullable and saying so in every row // carries no information; what remains is rendered as a union rather than // letting the first member silently win. -func schemaType(property map[string]any) string { +// +// A property with no determinable type is an error rather than a cell reading +// `any`. 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` would publish as a deliberate statement that the tool accepts anything. +// Nothing downstream would catch it: --check compares the generator's output to +// itself, so a wrong type is self-consistent. +func schemaType(property map[string]any) (string, error) { switch t := property["type"].(type) { case string: - return t + if t != "" { + return t, nil + } case []any: out := make([]string, 0, len(t)) for _, entry := range t { @@ -261,8 +274,19 @@ func schemaType(property map[string]any) string { out = append(out, name) } if len(out) > 0 { - return strings.Join(out, " or ") + return strings.Join(out, " or "), nil } } - return "any" + + // Name what the schema does carry, so the message points at the cause. + keys := make([]string, 0, len(property)) + for key := range property { + keys = append(keys, key) + } + sort.Strings(keys) + return "", fmt.Errorf( + "the schema names no type (it carries %s); the reference would publish it as "+ + "`any`, which reads as \"accepts anything\" rather than \"not determined\"", + strings.Join(keys, ", "), + ) } diff --git a/internal/mcp/describe_test.go b/internal/mcp/describe_test.go index 1aa16eec..934a3228 100644 --- a/internal/mcp/describe_test.go +++ b/internal/mcp/describe_test.go @@ -95,6 +95,12 @@ func TestDescribeToolsReportsCompleteMetadata(t *testing.T) { if input.Type == "" { t.Errorf("%s input %q has no type", doc.Name, input.Name) } + // `any` would read as a deliberate "accepts anything" rather than + // "the generator could not tell", so it is refused upstream. Asserted + // here too, because the difference matters to whoever calls the tool. + if input.Type == "any" { + t.Errorf("%s input %q published as `any`", doc.Name, input.Name) + } } } } @@ -142,3 +148,39 @@ func TestDescribeToolsReportsTheSharedObservabilityScope(t *testing.T) { } } } + +// A schema the generator cannot reduce to a type must fail rather than publish +// `any`. The SDK emits $ref, anyOf or a bare enum for shapes like a pointer or +// an interface, and --check would not catch the result: a page carrying a wrong +// type is self-consistent with the generator that wrote it. +func TestSchemaTypeRefusesAnUntypedProperty(t *testing.T) { + for name, property := range map[string]map[string]any{ + "a $ref": {"$ref": "#/$defs/State"}, + "an anyOf": {"anyOf": []any{map[string]any{"type": "string"}}}, + "a bare enum": {"enum": []any{"a", "b"}}, + "only null": {"type": []any{"null"}}, + "an empty type": {"type": ""}, + } { + if got, err := schemaType(property); err == nil { + t.Errorf("%s was accepted and published as %q", name, got) + } + } +} + +func TestSchemaTypeRendersUnionsWithoutNull(t *testing.T) { + got, err := schemaType(map[string]any{"type": []any{"null", "array"}}) + if err != nil { + t.Fatalf("nullable array refused: %v", err) + } + if got != "array" { + t.Errorf("nullable array rendered %q, want array", got) + } + + got, err = schemaType(map[string]any{"type": []any{"string", "integer"}}) + if err != nil { + t.Fatalf("union refused: %v", err) + } + if got != "string or integer" { + t.Errorf("union rendered %q, want \"string or integer\"", got) + } +}