[ragnorc]

Amends RFC 0003 for issue #991: the server's /mcp becomes the only MCP implementation, and @modernrelay/omnigraph-mcp is retired instead of kept as a stdio proxy.

What the amendment decides

• Phase 1: protected-resource metadata also at the path the 401 challenge advertises (/.well-known/oauth-protected-resource{resource path}, compared byte for byte, other suffixes 404); error="invalid_token" when a presented bearer is refused; a schema tool (GET /schema, Cedar read); an ad-hoc read tool (POST /query, Cedar read, mutations refused) behind --mcp-allow-adhoc, default off; descriptions, instructions and output schemas.
• Phase 2: /mcp mounted whenever the server has a bearer credential source (static tokens, signed data tokens, OIDC), with metadata and the OAuth challenge staying OIDC-only and a new --mcp-allowed-host setting; read parity tools (health, snapshot, branches_list, commits_list, commits_get, commits_changes, changes_poll) and resources; write tools behind --mcp-allow-writes, default off, keeping the package's write contract (graph-commit precondition, no automatic retry, unknown outcome reported).
• Every tool calls the HTTP handler its route calls, under the same Cedar action. No new authorization path; tool visibility is not permission.
• Records what the identity RFC already shipped and sets implementation: partial.

Choices a reviewer should check

1. Tool names. The shipped query stays the stored read, so the ad-hoc read is read, and the package's schema_get is schema. No alias tools.
2. Settings are boot flags plus environment variables (--mcp-allow-adhoc / OMNIGRAPH_MCP_ALLOW_ADHOC, …), like --unauthenticated and --require-all-graphs, not cluster configuration.
3. The opt-ins stay off even with an applied policy. A policy written for HTTP clients was not reviewed for an LLM caller.
4. Without OIDC, any Origin header is refused, since there is no resource origin to allow; native clients send none.
5. Resources are graph-scoped templates (omnigraph://graphs/{graph}/schema, …/branches) because the server is multi-graph; the package's URIs assumed one configured graph.
6. Best-practices resources embed skills/omnigraph/references through the omnigraph-skill crate proposed in feat(cli): omnigraph skill export ships the version-matched agent skill #986, so phase 2 depends on that crate landing.
7. Writes wait 60 s, then report outcome_unknown while the owned write continues.

Status stays draft; a maintainer decision moves it to accepted.

Implementation, ready for review and to merge only after acceptance: #993 (phase 1), #994 (phase 2, stacked on #993), #995 (best-practices resources, stacked on #994 and #986). Package deprecation, not published: ModernRelay/omnigraph-ts#22.

A later commit records that graphs falls back to GET /graphs under graph_list for non-identity credentials, since /graphs/discovery admits only identity credentials.

Checks

• python3 scripts/check-docs.py: Documentation OK (269 Markdown files checked)
• bash scripts/check-agents-md.sh: OK
• typos docs/rfcs/0003-mcp-server-surface.md: clean

[cursor]

Review of #992 at c3ee28f4

Recommendation: request changes before acceptance. All fixes are text changes to the RFC. I post this as a comment, so it does not set a formal review state. Inline comments give the exact lines.

What this PR does

• Why. OmniGraph has two MCP implementations: the npm package @modernrelay/omnigraph-mcp and the server's /mcp. They drift. Agents on /mcp can run only pre-written stored reads.
• What. RFC 0003 now makes the server's /mcp the only MCP surface and retires the npm package. It adds schema, ad-hoc read, parity read tools, resources, and opt-in write tools.
• How. Each tool deserializes typed arguments and calls the existing HTTP handler with the request's authenticated actor. Cedar stays the only authorization path. The settings --mcp-allow-adhoc, --mcp-allow-writes and --mcp-allowed-host are off or empty by default.
• Impact. Agents get one surface with the same authorization as HTTP. Operators get three boot settings. The registry row changes to implementation: partial.

What is good

• One implementation instead of two. This removes a real drift source.
• Tools are thin projections over existing handlers. The RFC adds no second authorization path (invariant 10).
• Ad-hoc reads and writes are opt-in. Conditional writes (if_graph_commit) and "no automatic retry" are kept.
• Arguments are typed with deny_unknown_fields. No query text is built from strings (invariant 9).

Blocking findings (required before acceptance)

1. Write admission is not "exactly as its HTTP route does" (RFC lines 252-253, 267-276). An HTTP write route takes the write observer lane. That lane exists "so slow readers cannot prevent a write from reaching operation admission" (operations.rs#L141, ingress.rs#L133). /mcp is always read class (ingress.rs#L91). So an MCP write takes a read observer (128 shared with all reads) and one of the 16 MCP slots shared with MCP reads. Main does the same: run_owned calls spawn_read (mcp.rs#L757-L785). Write ingress bytes and per-actor admission do match (mcp.rs#L1121-L1129). The text over-claims. Pick one: (a) state the shared lane as an accepted MCP-only trade-off, (b) give write tools their own slot pool and the write observer, or (c) classify on the Mcp-Method/Mcp-Name headers before body collection. rmcp 3.3.0 already checks those headers against the body (tower.rs around L564 and L672). Older clients fall back to read class.
2. The 60 second write wait and the cancellation path do not reach the client (RFC lines 246-249). In the 2026-07-28 Streamable HTTP transport, the client cancels by closing the connection, and then the server must not send more messages for that request. So a cancelled result with outcome_unknown has no delivery path. The TypeScript SDK default request timeout is also 60 000 ms (DEFAULT_REQUEST_TIMEOUT_MSEC, packages/core-internal/src/shared/protocol.ts:103). The client timer starts first, so a client often times out before the server's 60 s envelope arrives. Fix: set the write wait clearly below common client defaults. State that a lost response is also an unknown outcome, and tell the client how to reconcile it (for example, read graph_commit_id or the commit list). systems.md §B.1 asks for exactly this: "Atomic publication alone does not establish exactly-once effects after a lost acknowledgement."
3. "identity required" on the best-practices resources (RFC lines 227-228). In this RFC and in code, "identity" means OIDC or signed identity tokens only (RFC lines 188-189, identity.rs#L232). Read literally, static-token and scoped data-token clients cannot read the references, but phase 2 mounts /mcp for them. feat(server): serve the skill references as MCP best-practices resources #995 serves these references as static text to any authenticated actor, with no identity check. Make the RFC and feat(server): serve the skill references as MCP best-practices resources #995 agree. If the intent is "any authenticated actor", write that.
4. The status and rollout text is stale after feat(server): /mcp for every credential source, read parity, resources and opt-in writes #994 merged (RFC lines 68-86, 360-369). feat(server): /mcp for every credential source, read parity, resources and opt-in writes #994 (cea15256) shipped both phases to main in one PR. feat(server): schema and ad-hoc read tools on /mcp, metadata at the advertised path #993 (phase 1) is still open, and feat(server): /mcp for every credential source, read parity, resources and opt-in writes #994 contains its scope. The RFC still says "What shipped before this amendment" is the three identity-only tools, and it plans two separate PRs. Update that section and the rollout. implementation: partial is still correct until feat(server): serve the skill references as MCP best-practices resources #995 and the package retirement land. FYI for maintainers: the RFC registry says "Product behavior must not claim authority from a draft". main now ships behavior that this draft describes. Accepting the RFC after the fixes above closes that gap.

Optional findings

• O1, result equality (line 202). "byte-equal in value to the HTTP route's body" conflicts with the queries projection (mutations filtered out, line 169) and the graphs fallback (ids and names only, line 168). Consider "value-equal to the route's body, except where the catalog names a projection".
• O2, Origin mechanism (lines 139-141). rmcp treats an empty allowed_origins list as unrestricted (tower.rs L111-119, L883-886). The refusal without OIDC needs an OmniGraph layer. Main has one (mcp.rs#L195-L209). Name it in Design, so a future rmcp upgrade does not silently drop it.
• O3, graphs default on static tokens (line 168). Without an applied cluster policy, every server-scoped action is denied (handlers.rs#L657-L663). So on a default static-token server, graphs returns 403. That is safe, but say it in the RFC and in the server instructions, so the agent can tell the user what to configure.
• O4, 401 without a challenge (line 115). RFC 9110 §15.5.3 requires WWW-Authenticate on a 401. A plain Bearer challenge without resource_metadata fixes this. The rest of the server also omits it (the only insert in the server source is at mcp.rs#L299), so a follow-up is acceptable.
• O5, protocol versions. State which MCP protocol versions /mcp advertises. Also state that rmcp owns the 2026-07-28 header validation (-32020), so a later SDK bump is a reviewed change.
• O6, schema drift (lines 210-212). The tool catalog is a second public contract. On main, outputSchema values are hand-written JSON functions (for example health_output_schema), not derived from the utoipa schemas behind openapi.json. Consider deriving them, or add a test that compares them with the OpenAPI component schemas.

Trade-offs and software liability

• Gained: one MCP implementation, one authorization path, and no business logic in MCP code. When a route changes, the next change touches one place.
• Paid: a 20-tool catalog becomes a public contract beside OpenAPI. The cost of each new route is one more tool row, output schema and annotation. Without derived schemas (O6), drift risk grows with each tool.
• Paid: writes share the read admission lane (finding 1). Under read pressure, an MCP write can get capacity_exceeded when the same HTTP write would succeed. That is bounded and observable (invariant 11), but it is a real difference.
• Irreversibility: low. Nothing here changes a storage format or a Lance surface. The tool names and result envelopes are the sticky part, because agents learn them.

Related work and future direction

• feat(server): /mcp for every credential source, read parity, resources and opt-in writes #994 merged both phases. feat(server): schema and ad-hoc read tools on /mcp, metadata at the advertised path #993 is open and is now contained in feat(server): /mcp for every credential source, read parity, resources and opt-in writes #994. feat(server): serve the skill references as MCP best-practices resources #995 owns the best-practices resources. feat(cli): omnigraph skill export ships the version-matched agent skill #986 (omnigraph skill export) maintains the same skill references. Issue One MCP surface: make the server's /mcp the single MCP implementation #991 is the discussion.
• Lance: the RFC does not touch Lance. Tools call the existing handlers. Lance 11 stays pinned, and a later Lance upgrade does not change this design.
• MCP: the 2026-07-28 revision adds request headers and changes cancellation. Findings 2 and O5 cover that.

Validation

Commit	Command	Result
head c3ee28f4	python3 scripts/check-docs.py	Documentation OK (269 Markdown files checked)
head c3ee28f4	bash scripts/check-agents-md.sh	OK (54 links, 50 docs)
head c3ee28f4	typos (v1.50.1, as pinned in CI), whole repo	exit 0
head vs main cea15256	git merge-tree	clean
merge of head into main cea15256	check-docs.py, check-agents-md.sh	275 files OK. 55 links, 51 docs

CI on this head: 17 passed, 15 skipped (the Rust jobs skip a docs-only change), 0 failed.

Limitations. This is a docs-only PR, so I ran no Rust tests. I checked the implementation facts by reading main at cea15256, rmcp 3.3.0 from crates.io, the MCP 2026-07-28 specification source, and the TypeScript SDK source. I did not run a live MCP client against the server, so the client-timeout race in finding 2 comes from reading the source, not from a measurement.

[ragnorc]

All eight threads from the review at c3ee28f are addressed in 5f00b74 and b9f142a (the branch also merges main):

• the four required fixes: write wait and lost-response reconcile, the read-lane trade-off, any authenticated actor for best-practices, and rollout and status;
• all six optional ones: the Origin mechanism, the graph_list 403, WWW-Authenticate as a server-wide follow-up, and the projection wording.

The 30 s write wait and the instruction text are in #999. Status stays draft pending a maintainer decision.