Repository navigation
feat(mcp)!: one query tool, and every tool listed under policy - #1207
Conversation
`/mcp` hid the ad-hoc `read` tool and the write tools behind the `--mcp-allow-adhoc` and `--mcp-allow-writes` boot flags. Each tool already runs under its HTTP route's policy action, so the flags restricted only the MCP door of access the same credential held over HTTP, and made allowed tools look missing. Remove both flags and their environment variables. Every tool is listed; whether a tool writes now follows from its declared effect, and the server instructions are one constant. `omnigraph mcp --write` stays as the local agent's consent.
|
Revise before merge. The authorization path holds. The current operator contract does not. Why. MCP had a second door. What. This change removes those flags and How. Impact. A token that can change a graph over HTTP can now change it over MCP with no server flag. A token with no applied policy can now run ad-hoc Blocking
Optional
Tradeoff and liabilityOne policy door is the right long-term contract. Invariant 10 puts trust at the HTTP boundary and enforcement in the engine. The flags hid tools from MCP-only clients. They did not stop the holder of the token from calling HTTP. This pull request removes that guardrail on purpose.
The bridge consent is local. Without This removes a shadow allow-list that could drift from Cedar. It adds exposure until operators narrow credentials, and it adds a duty to keep the accepted RFC and the user guide as the one current contract. The deleted Write tools still share the 16 MCP slots and the read observer. RFC 0003 already records the new reason: the refusal is explicit and retryable ( These paths match the HTTP contract, from code inspection:
ValidationHead Local, merge base, temporary test (removed afterward):
Failed in 0.05s. The assertion was Local, head sources plus the same temporary test:
Passed. 3 passed, 0 failed, 0.28s. A first run without Also local, on the head sources:
CI on this commit, separate from the local runs: the pull request checks are green, including Test Workspace, Lint (clippy), GQ Logic Tests, and CI Gate. Devin Review is a skip ("trial expired and no credits remaining"). This head has no GitHub reviews and no review comments. LimitationsGQT cannot express this behavior, and this pull request adds no GQT case. I did not run the full workspace test graph locally. CI on this commit did. I did not remeasure MCP slot capacity under write load. No storage, version, index, or query-execution surface changes, so I did not open the pinned Lance 13.0.0 sources ( Inline comments that belong on the diff:
|
…ult deny Rewrite the accepted graph-access RFC's two sentences that still required `--mcp-allow-writes`, and RFC 0003's Design and Evidence sentences that described per-setting tool lists, naming each in the decision logs. The user guide, the breaking note and RFC 0003 say that leftover OMNIGRAPH_MCP_ALLOW_* variables are ignored and that a token with no applied policy still only reads. Tool descriptions and comments stop naming the removed gates. Add a regression for a static token with no policy: every tool is listed, `read` answers, and `mutate` is refused 403 by default deny with the manifest version unchanged.
|
Addressed in e9f522c. Blocking:
Optional:
Locally: server |
`query` ran a stored read and `read` ran GQ source, while `mutate` already took either source or a stored name. Make `query` take `query` (source) or `name` (stored), plus `params`, `branch` and `snapshot`, and remove `read`. One tool per effect, the same arguments meaning the same thing on both, as the CLI's `query` does. A stored read now also accepts `snapshot`, which its route already took.
|
Revise before merge. The query fold holds. RFC 0003's compatibility section still states the old tool contract. Earlier findingsThe two blockers at
The four optional items are resolved. The user guide, the breaking note, and RFC 0003 say the old flags stop startup and that leftover Why
What holds
The tool is Policy matches the HTTP routes. Source uses
Blocking
Optional
Tradeoff and liabilityOne tool per effect removes a second name for the same read. Authorization stays in the HTTP handlers ( The cost is one sentence copied into the catalog, the user guide, RFC 0003, the operation inventory, and the skill. This pull request updated four of those and left the skill and the compatibility bullet. Close that drift in this pull request. Five more tools of this shape stay cheap if each one calls the existing handler and the compatibility section records the break once. Open drafts #1197, #1149, and #951 can later grant ValidationHead GQT cannot express a tool list or a policy envelope. The corpus has no Local, merge base, temporary test (removed afterward):
Failed in 0.08s. Local, head, the same temporary test plus the branch tests (removed afterward):
Passed. 5 passed, 0 failed, 0.34s. A second temporary test at head,
CI on this commit, separate from the local runs: run 41030172646 is green, including Test Workspace, Lint (clippy), GQ Logic Tests, and CI Gate. Devin Review skipped (trial expired, no credits). This head has no GitHub reviews and no inline review comments. LimitationsI did not run the full workspace test graph locally. CI on this commit did. I did not remeasure the 16 MCP slots under write load. No storage, version, index, or query-execution surface changes, so I did not open the pinned Lance 13.0.0 sources. I did not execute a signed-data token with no policy. Inline comments that belong on the diff:
|
Amends RFC 0003 (decision log, 2026-10-29).
/mcphid the ad-hocreadtool and every write tool unless the server started with--mcp-allow-adhocor--mcp-allow-writes. Each of those tools already runs under its HTTP route's policy action, so the flags closed only the MCP door to access the same credential held overPOST /queryand/mutate. They protected nothing, and an agent connected to a default server saw allowed tools as missing.Both flags and their
OMNIGRAPH_MCP_ALLOW_*variables are gone, and every tool is listed. The catalog loses its per-tool gate: whether a tool writes (its write contract, deadline and result disposition) now follows from the effect it declares, the same field that sets its read-only annotation, so the two cannot disagree. The server instructions become one constant.omnigraph mcp --writeis unchanged: it remains the local agent's consent before the bridge relays a write.Reads were also split across two tools whose names pointed the wrong way:
queryran a stored read andreadran GQ source, whilemutatealready took either.querynow takesquery(source) orname(stored), withparams,branchandsnapshot, andreadis gone: one tool per effect, the same arguments meaning the same thing on both, as the CLI'squerydoes.readwas listed only behind the removed opt-in, so few clients call it; a stored read now also acceptssnapshot, which its route already took.The server's tool-list test now compares the full listing, with annotations and required arguments, against the shared operation inventory, and the write test covers annotations, the conditional
mutateand policy refusals on a default server. The bridge test checks that a server lists every write tool and that only--writeadmits one. A default-deny test pins a token with no policy: every tool listed,queryreturns rows,mutaterefused 403 with the manifest version unchanged.For review: this is breaking for any deployment that passes either flag, which now fails at argument parsing, and it widens what an agent can do over MCP to whatever its credential's policy already allowed over HTTP. Operators who relied on the flags to keep agents read-only must narrow the agents' policy instead; the guide and changelog say so. Narrowing agents without touching humans' grants, and granting stored queries without general
read, are follow-up policy work, not in this change. Locally,cli_mcp::managed_mcp_renews_control_helper_without_changing_identityandcli_mcp::mcp_renews_expired_helper_credentials_before_dispatchfailed withcredential_expiredagainst their stand-in endpoint; neither reaches the server, and this branch changes only a doc comment in the CLI, so I read it as their 8-second expiry window on a loaded machine.