feat(server): /mcp for every credential source, read parity, resources and opt-in writes

[ragnorc]

Phase 2 of the RFC 0003 amendment in #992 (issue #991). Stacked on #993: the first commit here is #993's, and only the second commit (/mcp for every credential source…) is new. I'll merge main in once #993 lands. Should merge only after #992 is accepted.

What changes

• R7, mount for every credential source. build_app mounts /mcp when the server has static bearer tokens, signed data tokens or OIDC trust (AppState::has_credential_source), not only with OIDC. Protected-resource metadata and the OAuth challenge stay OIDC-only, so a non-OIDC 401 carries no WWW-Authenticate. --mcp-allowed-host <host[:port]> (repeatable, or comma-separated OMNIGRAPH_MCP_ALLOWED_HOSTS) is added to rmcp's allowed hosts; values are validated at boot as bare authorities. Without OIDC there is no resource origin, and rmcp treats an empty origin list as unrestricted, so a small layer (after authentication, before body admission) refuses any request carrying Origin with 403.
• R8, read parity. health, snapshot, branches_list, commits_list, commits_get, commits_changes and changes_poll call their route handlers. The two change tools pass the same repeated query parameters an HTTP caller would send to the route's RawQuery parser, so filter semantics are the route's. Resources: omnigraph://graphs plus the templates omnigraph://graphs/{graph}/schema and …/branches, through the same handlers and run under the same observer, lease and slot ownership as tools. Resource refusals are JSON-RPC errors whose data carries {status, error}; unknown resources are -32002. The best-practices resources are a separate PR stacked on feat(cli): omnigraph skill export ships the version-matched agent skill #986 (it needs the omnigraph-skill crate).
• graphs for non-identity credentials. /graphs/discovery admits only identity credentials, so for a static or non-identity token graphs (and omnigraph://graphs) calls GET /graphs under graph_list, as the npm package's graphs_list did, and returns only ids and names. rfc: one MCP surface built into the server #992 records this.
• R9, opt-in writes. mutate (if_graph_commit → /mutate/if-graph-commit), load, branches_create, branches_delete and branches_merge, listed only with --mcp-allow-writes / OMNIGRAPH_MCP_ALLOW_WRITES=1, annotated destructiveHint/idempotentHint per tool. Each write reserves write ingress for its argument bytes and hands it to the handler, so the route's per-actor admission and owned-write registration are unchanged. Callers wait up to 60 s. On cancellation or that deadline, or a handler error the server marks completion-uncertain, the result carries outcome_unknown: true, while the owned write keeps running. No retries.
• run_read is generalised to run_owned<T> so resources share it; the observer, ingress lease, graph capture and slot ownership semantics are unchanged, and the existing cancellation unit test still drives it through the tool wrapper.
• Docs: docs/user/operations/mcp.md (mounting matrix, settings, read and write tools, write contract, resources), the server.md route table, and a changelog note.

Tests

Run locally on this branch (RUST_MIN_STACK=16777216):

• cargo test -p omnigraph-server --locked: lib 201 passed (2 ignored); boot_settings 39 passed (1 ignored); integration 262 passed; multi_graph 8 passed. New owners in integration mcp:::
  • static_token_server_serves_mcp_without_oauth_discovery: 200 with the token, 401 without it or with a wrong one and no challenge, metadata 404, allowed and foreign Host, Origin refused, graphs falling back to GET /graphs.
  • mcp_read_parity_tools_equal_their_routes: each parity tool's body equals its route's, including filtered change pages.
  • mcp_resources_read_graphs_schema_and_branches_under_policy.
  • mcp_write_tools_are_opt_in_conditional_and_policy_gated: absent by default with the manifest unchanged; annotations; a stale if_graph_commit gives 412 with no effect; load and a branch create, write, merge, delete round trip; Cedar refusals per action.
  • The tools/list snapshot covers three setting combinations.
• Unit: a_write_whose_wait_ends_first_reports_an_unknown_outcome, allowed_hosts_are_bare_authorities.
• cargo fmt --all --check, cargo clippy -p omnigraph-server --all-targets --locked -- -D warnings -W clippy::dbg_macro, check-docs.py, typos: clean.
• Live check with the official MCP TypeScript SDK (@modelcontextprotocol/sdk 1.29, StreamableHTTPClientTransport with an Authorization header) against the built omnigraph-server binary on a static-token cluster with --mcp-allow-adhoc --mcp-allow-writes:
  • Connecting without a token fails with 401.
  • With the token, it lists all 17 tools, the resources and the templates; health, schema, read, commits_list and the schema resource answer.
  • Writes are refused 403 by the server's default-deny mode (no policy applied), as intended.
  • The graphs 403 passed the SDK's output-schema validation, which checks error results too.

Not run locally: workspace-wide test and clippy graphs, --features aws, GQT and DST. CI runs them.

[cursor]

Review: /mcp for every credential source, read parity, resources and opt-in writes

Reviewed at: b816f8cc8932983aad2204eb390317deaad7d62b
Recommendation: Approve with minor observations below (no blocking items).
Scope: the second commit only (b816f8cc, the Phase 2 delta over #993).

---

What this change does

Phase 2 of RFC 0003 makes the server's /mcp endpoint a complete read and write surface for MCP clients with any bearer credential. Three changes:

1. Mount for every credential source (R7). /mcp mounts when AppState::has_credential_source() — static tokens, signed data tokens or OIDC — instead of OIDC-only. Without OIDC, no WWW-Authenticate challenge, no protected-resource metadata, and a browser-sent Origin header is refused with 403.
2. Read parity (R8). Seven new read tools (health, snapshot, branches_list, commits_list, commits_get, commits_changes, changes_poll) and three resources (omnigraph://graphs, …/schema, …/branches). Each calls its HTTP route's handler under the same actor and policy action.
3. Opt-in writes (R9). Five write tools (mutate, load, branches_create, branches_delete, branches_merge), listed only with --mcp-allow-writes. No retries; outcome_unknown on uncertain completion.

Plus McpSettings boot configuration, --mcp-allowed-host, graphs fallback to GET /graphs for non-identity credentials, and documentation.

Why this design is sound

The core design principle — every MCP tool delegates to the existing HTTP route handler, under the existing policy gate, with no new write logic — is the right choice. It is the narrowest surface that can offer write capability without a shadow execution path. A review of each write tool shows the handler call matches the documented route (e.g. mutate calls server_mutate_if_graph_commit when if_graph_commit is present, server_mutate otherwise), and the ingress reservation is made from the serialized argument bytes before the handler runs. The run_owned<T> generalization of run_read is clean: the Refusal enum unifies error handling, and the write: bool parameter on tool_result controls the outcome_unknown flag without duplicating logic.

Impact and tradeoff assessment

Observable contract change: A server with static tokens now mounts /mcp where it previously did not. This is additive: no existing client breaks; an MCP client that previously got 404 now gets the endpoint. Write tools are off by default.

Liability of outcome_unknown: The write contract is the most sensitive area. The implementation correctly derives uncertainty from ApiError::completion_uncertain() (only true for internal() / 500-class errors), plus the two inherently uncertain cases (caller cancelled, deadline exceeded while the write continues). A 4xx error means the write was refused before effects, so the outcome IS known. This is tested in a_write_whose_wait_ends_first_reports_an_unknown_outcome.

Origin enforcement for non-OIDC: Without OIDC, rmcp's empty-origin-list means "unrestricted". The middleware layer that refuses any Origin header in that case is the right fix, and it runs after auth but before ingress, which is the correct position.

Validation

Read the complete mcp.rs (1966 lines), all changed files, the test file, the user docs, both changelog entries, the RFC amendments, and the OpenAPI diff. Verified against invariants.md. Key checks:

• has_credential_source() is a strict subset of requires_bearer_auth(): tokens ∨ data-trust ∨ OIDC, without the policy-alone case. Correct for MCP: a policy without credentials has no way to authenticate.
• Every write tool calls self.write_ingress(&arguments)? before argument parsing, so the ingress reservation precedes the handler call.
• resource_challenge correctly applies RFC 6750 §3.1 invalid_token only when a bearer credential was presented.
• advertised_metadata_url produces the RFC 9728 §3.1 path; the test suite verifies the exact match and rejects close-but-wrong paths (percent-encoded, extra segments, trailing slashes).
• The openapi.json changes include the new {resource_path} route.
• The mcp.md user doc accurately reflects every tool, its arguments, route and policy action.

Observations (non-blocking)

1. discover fallback sets display_name = graph_id. When a non-identity credential uses GET /graphs (which lacks display names), the constructed GraphDiscoveryEntry maps graph_id into display_name. This is a reasonable fallback since the /graphs route doesn't expose display names, but a model reading display_name won't know it's the same as the ID. If the GET /graphs response ever gains a display name, this mapping should be updated.

2. get_tool and list_tools each rebuild the tool vector. tools(self.state.mcp_settings()) allocates and filters the full tool list on every call. With at most 17 tools this is negligible, but if the catalog grows substantially, a once-computed Arc<[Tool]> in GraphTools would eliminate it.

3. branches_delete is idempotent: true but returns 404 on a second call. Per the MCP spec, idempotentHint: true means "calling the tool again with the same arguments will have no additional effect." A delete that 404s on retry is idempotent in terms of state (the branch stays deleted), but an MCP client might retry on any failure status. The current annotation is technically correct (the state doesn't change) and the test verifies the 404.

4. ChangeFilters uses serde(flatten) in CommitChangesArguments and ChangesPollArguments. This works correctly since deny_unknown_fields is set on both outer structs. serde(flatten) with deny_unknown_fields can cause confusing errors if a user sends an unknown field that doesn't match either the outer struct or the flattened one — the error message may not name the offending field — but the MCP arguments_as function already maps all deserialization errors to a generic "invalid MCP tool arguments", so this doesn't change the user experience.

5. Schema and docs consistency is good. The mcp.md table matches every tool in the TOOLS array. The instructions string adapts to allow_adhoc and allow_writes and the test mcp_tool_list_follows_the_adhoc_setting_and_describes_every_tool verifies all three combinations.

Test coverage assessment

The new integration tests are comprehensive and well-structured:

• Three settings permutations tested for tool lists (default, adhoc, writes).
• Read parity tested by comparing each tool's structured output against its HTTP route's response.
• Write round-trip tested: insert → conditional stale reject → load → branch create → write → merge → delete, with manifest version checks.
• Policy enforcement tested per action: a change-only actor can mutate but not create branches; a read-only actor is denied writes.
• Static token path tested independently, covering Host/Origin enforcement, no OAuth discovery, and graphs fallback.
• Resource reads tested: schema (text/plain), branches (JSON), graphs; policy denial; unknown resource.
• Advertised metadata path tested: exact match, close misses, percent-encoded.
• The existing run_read capacity/lifetime test still exercises the shared run_owned path.

Overall this is a clean, well-tested extension of the MCP surface. It follows the repository's core principle: one handler, one policy gate, one source of truth. No blocking items.