feat(server): schema and ad-hoc read tools on /mcp, metadata at the advertised path

[ragnorc]

Phase 1 of the RFC 0003 amendment in #992 (issue #991). Implementation is ready for review; it should merge only after #992 is accepted.

What changes

• R1, metadata at the advertised path. GET /.well-known/oauth-protected-resource/{resource_path} answers the same document as the root route when the request path equals the path the /mcp challenge advertises. The advertised path is computed once from the boot binding and compared with the raw request path; any other suffix is 404. A wildcard route plus that comparison keeps the configured path out of axum's route syntax. OpenAPI documents the new route; it carries no contract header, like the root route.
• R2, error="invalid_token". A 401 for a request that presented a Bearer credential carries Bearer error="invalid_token", resource_metadata="…"; without one (or with another scheme) the challenge is unchanged.
• R3, schema tool → server_schema_get, Cedar read.
• R4, read tool → server_query, Cedar read; mutations and branch writes keep the route's 400. Listed and admitted only with --mcp-allow-adhoc / OMNIGRAPH_MCP_ALLOW_ADHOC=1 (new McpSettings, wired through serve_with_mcp_settings and ManagedServerConfig::with_mcp_settings, so ServerConfig's public struct literal is unchanged).
• R5, tool quality. Titles, fuller descriptions, openWorldHint: false, settings-aware initialize instructions (graphs → schema → stored read or read), and outputSchema for graphs, schema and queries. Each output schema is anyOf the success shape and the isError envelope, because MCP clients validate any structuredContent against it, error results included (checked in @modelcontextprotocol/sdk 1.29 Client.callTool). queries already returned typed parameter descriptors; a test now pins them.
• R6, docs. The MCP section moves out of server.md (at the 350-line docs cap) into a new docs/user/operations/mcp.md; the identity RFC's "initial tools" paragraph now points at RFC 0003; the self-contained testing RFC drops unsupported_capability: mcp. RFC 0003 itself is amended in rfc: one MCP surface built into the server #992.

run_read's observer, ingress lease, graph capture and slot ownership through cancellation and the deadline is unchanged; both new tools run through it. rmcp stays pinned at =3.3.0.

Tests

Run locally on this branch (macOS, RUST_MIN_STACK=16777216 as in CI):

• cargo test -p omnigraph-server --locked: lib 199 passed (2 ignored); boot_settings 39 passed (1 ignored); integration 258 passed; multi_graph 8 passed. The 10 tests in integration mcp:: include the new mcp_tool_list_follows_the_adhoc_setting_and_describes_every_tool (the tools/list snapshot per setting, plus the instructions) and mcp_schema_and_adhoc_read_reuse_their_routes_and_policy (schema and read equal GET /schema and POST /query, including branch list; refusals for denied actors; insert/update/delete/branch create refused with the manifest version unchanged; 64 KiB → 413). mcp_authenticates_every_request_and_enforces_resource_and_http_bounds now also covers metadata at the advertised path, six other suffixes returning 404, invalid_token for refused bearers, and no error code for a Basic credential.
• OMNIGRAPH_UPDATE_OPENAPI=1 cargo test -p omnigraph-server --test integration openapi::openapi_spec_is_up_to_date, then the same test without the variable passes; openapi.json gains the one route.
• cargo fmt --all --check: clean. cargo clippy -p omnigraph-server --all-targets --locked -- -D warnings -W clippy::dbg_macro: clean.
• python3 scripts/check-docs.py, bash scripts/check-agents-md.sh, typos: clean.

Not run locally: the workspace-wide test and clippy graphs (only omnigraph-server changed), --features aws (auth.rs is untouched), and the GQT and DST suites. CI runs all of them.

[cursor]

Review of feat(server): schema and ad-hoc read tools on /mcp, metadata at the advertised path

Reviewed at: ff34d5b6 (the current head)
Verdict: Approve — the change is sound, well-tested, and ready to merge after #992 lands.

---

What this PR does and why it matters

Phase 1 of the RFC 0003 amendment. It makes the server's /mcp useful for a model that does not already know a graph's schema or stored reads. Six changes:

1. R1 — metadata at the advertised path. GET /.well-known/oauth-protected-resource/{resource_path} answers the same document when the raw request path matches the one computed from the boot binding. Any other suffix returns 404. This removes a proxy-rewrite requirement for every deployment whose resource identifier has a path.
2. R2 — error="invalid_token". RFC 6750 section 3.1 compliance: a 401 response for a presented bearer credential carries the error code; a missing credential or another scheme (e.g. Basic) does not.
3. R3 — schema tool. Calls server_schema_get, Cedar read. An LLM now learns graph types, properties, keys and edges before it tries to write a query.
4. R4 — read tool. Calls server_query, Cedar read; mutations are refused by the handler's existing Door::Query path. Gated behind --mcp-allow-adhoc / OMNIGRAPH_MCP_ALLOW_ADHOC=1, default off.
5. R5 — tool quality. Titles, descriptions, openWorldHint: false, settings-aware initialize instructions, and outputSchema for the fixed-shape tools.
6. R6 — docs. MCP content moves from server.md (at its size cap) into a standalone docs/user/operations/mcp.md.

Architectural assessment

Positive:

• Every new tool delegates to an existing HTTP handler. No new authorization path is introduced. This preserves invariant 10 (trust established at the boundary, enforced at the engine).
• McpSettings is a boot-time struct, computed once by McpSettings::resolve. This matches the codebase's existing pattern for server-wide flags (--require-all-graphs, --unauthenticated).
• ReadArguments uses #[serde(deny_unknown_fields)], which prevents a caller from passing settings through to the session. The QueryRequest constructed in the tool body therefore always carries settings: None, so MCP reads always use default session behavior. This is tested (a call with an unknown "settings" field returns an error).
• AdvertisedMetadataPath is computed once at boot from the fixed binding and stored in a layer extension — no per-request URL parsing.
• The ToolSpec static array with a Gate discriminant is cleaner than the old inline tuple construction, and scales to Phase 2's additional tools without adding match arms.

Tradeoffs to be aware of:

• The read tool constructs a QueryRequest through a JSON intermediate (serde_json::from_value(json!({...}))) rather than a struct literal. If QueryRequest gains a new required field in the future, this fails at runtime rather than compile time. The test coverage will catch it, but it is a softer contract than a direct struct construction. This is the same pattern the existing query tool uses, so it is consistent.
• The is_resource_metadata_path helper returns true for any path under the well-known prefix. It is only used in identify_response (skip the contract header) and describe_contract (skip the OpenAPI contract annotation). The actual routing decision is the wildcard route plus the exact-path comparison against AdvertisedMetadataPath. The wider predicate is safe for its use sites, but a future caller would need to understand this distinction.

Test and CI evidence

Validated on the exact head ff34d5b6:

Check	Result
cargo test -p omnigraph-server --locked --test integration mcp::	6 passed, 0 failed, 0 ignored (all 6 MCP tests, including the 2 new ones)
cargo test -p omnigraph-server --locked --test integration openapi::openapi_spec_is_up_to_date	1 passed (the committed openapi.json matches the generated spec)
cargo fmt --all --check	clean
cargo clippy -p omnigraph-server --all-targets --locked -- -D warnings -W clippy::dbg_macro	clean
bash scripts/check-agents-md.sh	OK

Not run in this environment (CI will cover): the full workspace test and clippy graphs, --features aws, GQT, DST.

Specific test coverage observations

The two new tests cover the critical properties well:

• mcp_tool_list_follows_the_adhoc_setting_and_describes_every_tool — verifies the tool list per setting (default and adhoc), tool metadata properties (title, description length, readOnlyHint, openWorldHint, additionalProperties, output schema shape), instructions text (mentions schema, mentions read only when enabled), and that read is refused as an unknown tool when adhoc is off.
• mcp_schema_and_adhoc_read_reuse_their_routes_and_policy — proves schema and read return the exact same JSON as their HTTP routes, that policy denials carry the same HTTP status, that insert/update/delete/branch-create are refused with 400, that the manifest version is unchanged after every refused mutation, that deny_unknown_fields catches stray arguments, and that the 64 KiB request bound applies.

Items for the author (non-blocking)

See inline comments below for specifics. None of these block the merge.

[cursor]

Optional / non-blocking. The JSON intermediate here (serde_json::from_value(json!({...}))) is consistent with how query constructs InvokeStoredQueryRequest, so this is not a new pattern. Worth noting for Phase 2: if QueryRequest gains a new required field, this will fail at runtime rather than at compile time. The test coverage catches it today, so this is an observation, not a request to change it now.

If this ever becomes a concern, a direct struct construction (crate::api::QueryRequest { query: args.query, name: args.name, ... settings: None }) would give a compile-time guarantee.

[cursor]

Observation (non-blocking). is_resource_metadata_path returns true for any suffix under the well-known prefix. Its two callers (identify_response and describe_contract) use it as "skip the contract header / annotation", which is safe because the actual routing decision (wildcard route + exact-path comparison) is narrower. A future caller that uses this as an authorization or routing predicate would need to be aware of the wider match. The current use sites are correct.

[cursor]

Non-blocking / Phase 2 note. The anyOf [output(), error_schema()] shape and the comment explaining why (MCP clients validate structuredContent against outputSchema even for isError results) is well-placed. When Phase 2 adds more fixed-shape tools, this pattern already handles them.

[cursor]

Optional / non-blocking. The env_flag helper is clean. For future extensibility (Phase 2 will add --mcp-allow-writes), you could consider extracting this into a shared utility if the pattern repeats. The current single-use is fine.

[cursor]

Non-blocking. The relative link ../docs/user/operations/mcp.md here — confirm this resolves correctly when the changelog is rendered. If the rendering context is the repository root, the ../ prefix would step out of the repo. Consider an absolute path from the repo root like docs/user/operations/mcp.md or a full URL. (This may already be the correct convention for your changelog rendering; just flagging it.)

[cursor]

Observation (non-blocking). Good defense-in-depth: the AdvertisedMetadataPath extension is only installed when the OIDC resource URL is configured and parses successfully. If the URL has no meaningful path, advertised_metadata_url would produce /.well-known/oauth-protected-resource (the root), which equals the existing route. The if let Some(advertised) gate ensures the wildcard handler is mounted only when there is a genuine path suffix to match. Clean boot-time computation.

[cursor]

Minor / non-blocking. The settings table is clear. For Phase 2, this will grow with --mcp-allow-writes and --mcp-allowed-host. The single-table layout scales well.

[ragnorc]

Closing: this phase landed on main through #994, which was stacked on this branch (its history contains this head) and was squash-merged as cea1525. Nothing here remains to merge.