fix(server): align stored-query MCP discovery gates

crates/omnigraph-server/src/handlers.rs

@@ -1065,13 +1065,14 @@ pub(crate) async fn server_invoke_query(
 )]
 /// List the graph's exposed stored queries as a typed tool catalog.
 ///
-/// Returns the `mcp.expose == true` subset of the `queries:` registry, each
-/// with its MCP tool name, read/mutate flag, description/instruction, and
+/// Returns the exposed (`@mcp(expose: true)`) subset of the `queries:` registry,
+/// each with its MCP tool name, read/mutate flag, description/instruction, and
 /// typed parameters — enough for a client to register them as tools without
-/// fetching `.gq` source. Read-gated; the catalog is graph-wide (branch
-/// independent — `read` is authorized against `main`). **Not** Cedar-filtered
-/// per query yet, so it can list a query whose `invoke_query` the caller
-/// lacks (a known gap until per-query authorization lands).
+/// fetching `.gq` source. **`invoke_query`-gated** (graph-scoped), so catalog
+/// discovery uses the same authority as invocation and matches the MCP
+/// `tools/list` surface: a caller that can list can invoke (subject to the inner
+/// `read`/`change` gate on the query body). Requires an explicit `invoke_query`
+/// grant — in default-deny mode (tokens, no policy) it returns 403.
 pub(crate) async fn server_list_queries(
     Extension(handle): Extension<Arc<GraphHandle>>,
     actor: Option<Extension<ResolvedActor>>,
@@ -1080,8 +1081,8 @@ pub(crate) async fn server_list_queries(
         actor.as_ref().map(|Extension(actor)| actor),
         handle.policy.as_deref(),
         PolicyRequest {
-            action: PolicyAction::Read,
-            branch: Some("main".to_string()),
+            action: PolicyAction::InvokeQuery,
+            branch: None,
             target_branch: None,
         },
     )?;

crates/omnigraph-server/src/mcp.rs

@@ -60,6 +60,52 @@ pub(crate) const BUILTIN_TOOL_NAMES: &[&str] = &[
     "schema_apply",
 ];
 
+/// Max MCP tool-name length. Matches the constraint major MCP clients enforce
+/// (Anthropic/OpenAI cap tool names at 64 chars over `[A-Za-z0-9_-]`).
+const MAX_TOOL_NAME_LEN: usize = 64;
+
+/// Every tool name the MCP surface itself generates and therefore reserves
+/// graph-wide: the built-ins **plus** the `meta`-mode discovery/execute pair.
+/// A stored query whose effective tool name claims one of these is refused at
+/// load (`QueryRegistry::from_specs`) — otherwise it would be silently shadowed
+/// (a built-in always wins at dispatch; the meta pair takes over the name once
+/// the catalog crosses [`STORED_QUERY_AUTO_THRESHOLD`], a silent
+/// threshold-crossing meaning change). Single source of "names the surface
+/// emits", so the reservation can't drift from what's generated.
+pub(crate) fn reserved_tool_names() -> impl Iterator<Item = &'static str> {
+    BUILTIN_TOOL_NAMES
+        .iter()
+        .copied()
+        .chain([STORED_QUERY_LIST_TOOL, STORED_QUERY_RUN_TOOL])
+}
+
+/// The MCP tool-name contract every published name must satisfy: non-empty,
+/// `[A-Za-z0-9_-]`, at most [`MAX_TOOL_NAME_LEN`] — the intersection of what
+/// MCP clients accept. The `.gq` grammar already constrains query *names* to a
+/// subset of this; this guards the free-form `@mcp(tool_name: …)` override so a
+/// malformed name fails loudly at load instead of producing a tool a client
+/// silently rejects at call time. Returns the operator-facing reason on failure.
+pub(crate) fn validate_mcp_tool_name(name: &str) -> Result<(), String> {
+    if name.is_empty() {
+        return Err("MCP tool name must not be empty".to_string());
+    }
+    if name.len() > MAX_TOOL_NAME_LEN {
+        return Err(format!(
+            "MCP tool name '{name}' exceeds {MAX_TOOL_NAME_LEN} characters"
+        ));
+    }
+    if let Some(bad) = name
+        .chars()
+        .find(|c| !(c.is_ascii_alphanumeric() || *c == '_' || *c == '-'))
+    {
+        return Err(format!(
+            "MCP tool name '{name}' contains an unsupported character '{bad}' \
+             (allowed: ASCII letters, digits, '_', '-')"
+        ));
+    }
+    Ok(())
+}
+
 /// The server's thin wrapper over `omnigraph_mcp::mcp_router`: derives the
 /// fail-closed host policy from the bound socket and passes the 32 MiB body
 /// limit (`/load` parity). Merged into `per_graph_protected` in `build_app`.
@@ -119,14 +165,16 @@ impl McpBackend for OmnigraphMcpBackend {
         let (actor, handle) = self.ctx(parts)?;
         let mut tools = Vec::new();
         for builtin in Builtin::ALL {
-            // Visibility is a *relaxation* of the per-call gate: a built-in whose
-            // authorization depends on a caller-chosen branch is shown iff the
-            // actor could invoke it on *some* branch (`authorize_any_branch`),
-            // never hidden because a fabricated branch happened to be denied. The
+            // Visibility is derived from what the tool's `call` authorizes
+            // (`list_gate`): a fixed-scope tool is gated on that exact request
+            // (faithful, and consistent with its resource twin); a
+            // caller-chosen-branch tool is shown iff the actor could invoke it on
+            // *some* branch (a relaxation — never hide a callable tool). The
             // per-call gate inside `call` stays authoritative.
-            let visible = match builtin.list_action() {
-                None => true,
-                Some(action) => {
+            let visible = match builtin.list_gate() {
+                ListGate::Always => true,
+                ListGate::Exact(request) => allowed(actor, handle, request)?,
+                ListGate::AnyBranch(action) => {
                     authorize_any_branch(actor, handle.policy.as_deref(), action).map_err(api_to_mcp)?
                 }
             };
@@ -633,6 +681,20 @@ fn stored_query_list(registry: &QueryRegistry, args: &JsonObject) -> Result<Call
 
 // ===== built-in tools =====
 
+/// How `tools/list` gates a built-in, derived from what its `call` authorizes.
+/// See [`Builtin::list_gate`].
+enum ListGate {
+    /// Ungated — always visible.
+    Always,
+    /// The call authorizes this exact request regardless of arguments; list iff
+    /// it is allowed (faithful, no over-show).
+    Exact(PolicyRequest),
+    /// The call authorizes against a caller-chosen branch; list iff the action
+    /// is permitted on *some* branch (a relaxation that never hides a callable
+    /// tool).
+    AnyBranch(PolicyAction),
+}
+
 /// Built-in operational tools. One variant per tool; `descriptor`/`list_gate`/
 /// `call` are match arms (lower liability than a `dyn` zoo).
 #[derive(Clone, Copy)]
@@ -691,28 +753,41 @@ impl Builtin {
         Builtin::ALL.into_iter().find(|b| b.name() == name)
     }
 
-    /// The action whose grant governs whether this tool is shown in
-    /// `tools/list`. `None` ⇒ always visible (`graph_health` is liveness,
-    /// ungated). The branch dimension is intentionally absent: listing probes
-    /// "can this actor perform `action` on *any* branch?" via
-    /// `authorize_any_branch`, a relaxation of the per-call gate. A read-only
-    /// actor (no write grant on any branch) has writers hidden; an actor who can
-    /// write unprotected branches sees `graph_mutate` even when `main` is
-    /// protected. The authoritative per-call gate runs inside `call`.
-    fn list_action(self) -> Option<PolicyAction> {
+    /// How `tools/list` decides this tool's visibility — **derived from what the
+    /// tool's `call` actually authorizes**, so listing never diverges from
+    /// callability:
+    /// - `Always` — ungated (`graph_health` liveness).
+    /// - `Exact(req)` — the call authorizes a *fixed* request regardless of
+    ///   arguments (`schema_get`/`branch_list`/`commit_get` read branchlessly;
+    ///   `schema_apply` always targets `main`). Listed iff that exact request is
+    ///   allowed — faithful, no over-show, and consistent with the resource
+    ///   twins (`omnigraph://schema` etc.) which use the same branchless read.
+    /// - `AnyBranch(action)` — the call authorizes against a *caller-chosen*
+    ///   branch. Listed iff the actor could perform `action` on *some* branch (a
+    ///   relaxation: never hide a tool the caller could invoke — e.g.
+    ///   `graph_mutate` shows for an unprotected-branch writer even when `main`
+    ///   is protected). The authoritative per-call gate runs inside `call`.
+    fn list_gate(self) -> ListGate {
         match self {
-            Builtin::GraphHealth => None,
-            Builtin::GraphQuery
-            | Builtin::GraphSnapshot
-            | Builtin::SchemaGet
-            | Builtin::BranchList
-            | Builtin::CommitList
-            | Builtin::CommitGet => Some(PolicyAction::Read),
-            Builtin::GraphMutate | Builtin::GraphLoad => Some(PolicyAction::Change),
-            Builtin::BranchCreate => Some(PolicyAction::BranchCreate),
-            Builtin::BranchDelete => Some(PolicyAction::BranchDelete),
-            Builtin::BranchMerge => Some(PolicyAction::BranchMerge),
-            Builtin::SchemaApply => Some(PolicyAction::SchemaApply),
+            Builtin::GraphHealth => ListGate::Always,
+            // Fixed branchless read — same gate as the schema/branches resources.
+            Builtin::SchemaGet | Builtin::BranchList | Builtin::CommitGet => {
+                ListGate::Exact(read_request(None))
+            }
+            // Always targets `main`.
+            Builtin::SchemaApply => ListGate::Exact(PolicyRequest {
+                action: PolicyAction::SchemaApply,
+                branch: None,
+                target_branch: Some("main".to_string()),
+            }),
+            // Caller-chosen branch → relaxation.
+            Builtin::GraphQuery | Builtin::GraphSnapshot | Builtin::CommitList => {
+                ListGate::AnyBranch(PolicyAction::Read)
+            }
+            Builtin::GraphMutate | Builtin::GraphLoad => ListGate::AnyBranch(PolicyAction::Change),
+            Builtin::BranchCreate => ListGate::AnyBranch(PolicyAction::BranchCreate),
+            Builtin::BranchDelete => ListGate::AnyBranch(PolicyAction::BranchDelete),
+            Builtin::BranchMerge => ListGate::AnyBranch(PolicyAction::BranchMerge),
         }
     }
 

crates/omnigraph-server/src/queries.rs

@@ -152,18 +152,27 @@ impl QueryRegistry {
         // before it is moved into `Self`.
         {
             let mut claimed: BTreeMap<&str, &str> = BTreeMap::new();
-            // Built-in MCP tool names are reserved graph-wide. A stored query
-            // that shadows one would silently never be served (built-ins win at
-            // dispatch) — the deny-list forbids silent drops, so seed them here
-            // and fail loudly at load instead.
-            for builtin in crate::mcp::BUILTIN_TOOL_NAMES {
-                claimed.insert(builtin, BUILTIN_OWNER);
+            // Every name the MCP surface generates is reserved graph-wide: the
+            // built-ins AND the meta-mode pair. A stored query claiming one would
+            // be silently shadowed (a built-in wins at dispatch; the meta pair
+            // takes over the name once the catalog crosses the auto threshold) —
+            // the deny-list forbids silent drops, so seed them all and fail
+            // loudly at load. Single source via `reserved_tool_names()`.
+            for reserved in crate::mcp::reserved_tool_names() {
+                claimed.insert(reserved, BUILTIN_OWNER);
             }
             for query in by_name.values().filter(|q| q.is_exposed()) {
                 let tool = query.effective_tool_name();
+                // Well-formedness: the `@mcp(tool_name: …)` override is free
+                // text; reject a name no MCP client will accept at load rather
+                // than emitting a tool that fails at call time.
+                if let Err(message) = crate::mcp::validate_mcp_tool_name(tool) {
+                    errors.push(LoadError { query: Some(query.name.clone()), message });
+                    continue;
+                }
                 if let Some(winner) = claimed.insert(tool, &query.name) {
                     let message = if winner == BUILTIN_OWNER {
-                        format!("MCP tool name '{tool}' is reserved by a built-in tool")
+                        format!("MCP tool name '{tool}' is reserved by a built-in or meta tool")
                     } else {
                         format!("MCP tool name '{tool}' already claimed by exposed query '{winner}'")
                     };
@@ -447,6 +456,52 @@ mod tests {
         assert_eq!(reg.len(), 2);
     }
 
+    #[test]
+    fn malformed_tool_name_override_is_a_load_error() {
+        // The `@mcp(tool_name: …)` override is free text; a name no MCP client
+        // accepts must fail loudly at load, not surface at call time.
+        for (bad, hint) in [
+            ("has space", "unsupported character"),
+            ("comma,name", "unsupported character"),
+            ("", "must not be empty"),
+            (
+                // 65 chars > MAX_TOOL_NAME_LEN
+                "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+                "exceeds",
+            ),
+        ] {
+            let errors = QueryRegistry::from_specs(vec![spec_tool(
+                "q",
+                "query q() { match { $u: User } return { $u.name } }",
+                true,
+                bad,
+            )])
+            .unwrap_err();
+            let msg = errors[0].to_string();
+            assert!(msg.contains(hint), "tool_name {bad:?} should fail with {hint:?}: {msg}");
+        }
+    }
+
+    #[test]
+    fn meta_tool_name_is_reserved() {
+        // The generated meta-mode tools are reserved graph-wide, like the
+        // built-ins — a stored query claiming one would be silently shadowed
+        // once the catalog crosses the auto threshold.
+        for reserved in ["stored_query_list", "stored_query_run"] {
+            let errors = QueryRegistry::from_specs(vec![spec(
+                reserved,
+                &format!("query {reserved}() {{ match {{ $u: User }} return {{ $u.name }} }}"),
+                true,
+            )])
+            .unwrap_err();
+            assert!(
+                errors[0].to_string().contains("reserved"),
+                "'{reserved}' must be reserved: {}",
+                errors[0]
+            );
+        }
+    }
+
     #[test]
     fn parse_error_surfaces_per_entry() {
         let errors =

crates/omnigraph-server/tests/mcp.rs

@@ -13,9 +13,9 @@ use omnigraph_server::queries::{QueryRegistry, RegistrySpec};
 use omnigraph_server::{AppState, build_app};
 use serde_json::{Value, json};
 use support::{
-    FIND_PERSON_GQ, INVOKE_POLICY_YAML, app_for_loaded_graph_with_auth_tokens,
-    app_for_loaded_graph_with_auth_tokens_and_policy, app_with_stored_queries, g, graph_path,
-    init_loaded_graph, json_response,
+    FIND_PERSON_GQ, INVOKE_POLICY_YAML, POLICY_PROTECTED_READ_YAML,
+    app_for_loaded_graph_with_auth_tokens, app_for_loaded_graph_with_auth_tokens_and_policy,
+    app_with_stored_queries, g, graph_path, init_loaded_graph, json_response,
 };
 
 /// Build a JSON-RPC POST to `/graphs/default/mcp`. Sets the `Accept` (both
@@ -577,6 +577,40 @@ async fn stored_query_tool_folds_docs_and_honors_mcp_annotation() {
     assert_ne!(v["result"]["isError"], json!(true), "renamed tool not callable: {v}");
 }
 
+#[tokio::test]
+async fn list_gate_matches_call_for_fixed_branchless_reads() {
+    // A protected-only reader. Branch-arg reads (graph_query) relax and show
+    // (callable on a protected branch). Fixed branchless reads (schema_get) use
+    // the faithful read(None) gate — which a protected-scope rule denies — so
+    // schema_get is hidden, matching the omnigraph://schema resource (same
+    // branchless read). Tool and resource agree by construction.
+    let (_t, app) = app_for_loaded_graph_with_auth_tokens_and_policy(
+        &[("act-bruno", "tok")],
+        POLICY_PROTECTED_READ_YAML,
+    )
+    .await;
+
+    let (_s, list) =
+        json_response(&app, mcp_request(Some("tok"), rpc(1, "tools/list", json!({})))).await;
+    let names = tool_names(&list);
+    assert!(
+        names.contains(&"graph_query".to_string()),
+        "branch-arg read relaxes and shows under protected-only read: {names:?}"
+    );
+    assert!(
+        !names.contains(&"schema_get".to_string()),
+        "fixed branchless read uses read(None), denied under protected-only → hidden: {names:?}"
+    );
+
+    // resources/list uses the same read(None) gate → empty, matching schema_get.
+    let (_s, res) =
+        json_response(&app, mcp_request(Some("tok"), rpc(2, "resources/list", json!({})))).await;
+    assert!(
+        res["result"]["resources"].as_array().unwrap().is_empty(),
+        "resources hidden under protected-only read, consistent with schema_get: {res}"
+    );
+}
+
 #[tokio::test]
 async fn per_query_mode_does_not_expose_meta_tools() {
     // Below the auto threshold the projection is per-query, so the discovery +

crates/omnigraph-server/tests/stored_queries.rs

@@ -345,28 +345,29 @@ async fn list_queries_returns_only_exposed_with_typed_params() {
 }
 
 #[tokio::test(flavor = "multi_thread")]
-async fn list_queries_is_read_gated_so_a_non_invoker_can_list() {
-    // The catalog is read-gated (not invoke_query-gated), so a reader who
-    // lacks invoke_query still enumerates the exposed queries — the
-    // documented probe-oracle gap until per-query Cedar filtering lands.
+async fn list_queries_requires_invoke_query() {
+    // The catalog is invoke_query-gated (same authority as invocation and the
+    // MCP `tools/list` surface): a reader who lacks invoke_query is denied
+    // listing, while an invoke_query holder lists it.
     let (_temp, app) = app_with_stored_queries(
         &[("find_person", FIND_PERSON_GQ, true)],
-        &[("act-noinvoke", "t-noinvoke")],
+        &[("act-noinvoke", "t-noinvoke"), ("act-invoke", "t-invoke")],
         INVOKE_POLICY_YAML,
     )
     .await;
-    let (status, body) = json_response(&app, get_request(&g("/queries"), "t-noinvoke")).await;
-    assert_eq!(status, StatusCode::OK, "read-gated catalog; body: {body}");
+    // read-only, no invoke_query → 403.
+    let (status, _body) = json_response(&app, get_request(&g("/queries"), "t-noinvoke")).await;
+    assert_eq!(status, StatusCode::FORBIDDEN, "catalog listing requires invoke_query");
+    // invoke_query holder → 200 with the exposed query.
+    let (status, body) = json_response(&app, get_request(&g("/queries"), "t-invoke")).await;
+    assert_eq!(status, StatusCode::OK, "invoker lists the catalog; body: {body}");
     let names: Vec<&str> = body["queries"]
         .as_array()
         .unwrap()
         .iter()
         .map(|q| q["name"].as_str().unwrap())
         .collect();
-    assert!(
-        names.contains(&"find_person"),
-        "a reader lists the catalog despite lacking invoke_query: {names:?}"
-    );
+    assert!(names.contains(&"find_person"), "invoker sees the exposed query: {names:?}");
 }
 
 #[tokio::test(flavor = "multi_thread")]

crates/omnigraph-server/tests/support/mod.rs

@@ -362,6 +362,10 @@ rules:
       actors: {{ group: permitted }}
       actions: [schema_apply, branch_create, branch_delete, branch_merge]
       target_branch_scope: any
+  - id: permit-invoke
+    allow:
+      actors: {{ group: permitted }}
+      actions: [invoke_query]
 "#
     )
 }