mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-21 02:28:07 +02:00
feat(mcp): per-query @mcp(...) annotation + per-param @description + @instruction folding
Wire the `.gq` authoring surface that controls how a stored query is projected
as an MCP tool. All of it rides in the query source (content-addressed,
re-parsed at boot), so there is no cluster.yaml / catalog / serving-snapshot
plumbing — and it is orthogonal to Cedar `invoke_query` (presentation, not
authorization).
- Per-parameter `@description("…")` (leading the variable) → carried on
`Param.description`, mapped through `param_descriptor`, and emitted on the
outer JSON-Schema property by `param_json_schema`, so it shows up in both the
MCP tool input schema and the `GET /queries` catalog.
- Query `@mcp(expose: <bool>, tool_name: "<name>")` → parsed into
`QueryDecl.mcp`; `StoredQuery::is_exposed()` / `effective_tool_name()` resolve
from it. `expose: false` hides a query from the agent surface (`tools/list`,
`stored_query_list`, run-by-name) while keeping it HTTP/service-callable.
- `@instruction` is folded into the MCP tool description (after `@description`),
so the agent-facing how/when-to-use guidance reaches `tools/list`.
- Removes the now-dead `RegistrySpec.{expose, tool_name}` fields (server + CLI);
`settings.rs` no longer hardcodes `expose: true`. Test helpers express
exposure by injecting `@mcp(expose: false)` into the source (the real path).
openapi.json regenerated: `ParamDescriptor` gains an optional `description`.
Tests: compiler parser (param @description, @mcp parse + duplicate rejection),
api-types schema_equivalence (description on the outer property), server mcp
(folded description + param docs + @mcp tool rename, list==call). Full
workspace gate green.
This commit is contained in:
parent
bcd0d9c867
commit
c8e91c11f0
14 changed files with 396 additions and 107 deletions
|
|
@ -1087,8 +1087,7 @@ pub(crate) async fn server_list_queries(
|
|||
)?;
|
||||
let queries = match handle.queries.as_ref() {
|
||||
Some(registry) => registry
|
||||
.iter()
|
||||
.filter(|q| q.expose)
|
||||
.exposed()
|
||||
.map(api::query_catalog_entry)
|
||||
.collect(),
|
||||
None => Vec::new(),
|
||||
|
|
|
|||
|
|
@ -535,11 +535,19 @@ fn stored_query_input_schema(stored: &StoredQuery) -> Value {
|
|||
}
|
||||
|
||||
fn stored_query_tool(stored: &StoredQuery) -> Tool {
|
||||
let description = stored
|
||||
// The MCP tool description folds `@description` and `@instruction` (the
|
||||
// agent-facing "how to use" guidance) into the one description slot MCP
|
||||
// tools have. Instruction-only queries still surface their instruction
|
||||
// (appended to the fallback base).
|
||||
let mut description = stored
|
||||
.decl
|
||||
.description
|
||||
.clone()
|
||||
.unwrap_or_else(|| format!("Stored query '{}'.", stored.name));
|
||||
if let Some(instruction) = &stored.decl.instruction {
|
||||
description.push_str("\n\n");
|
||||
description.push_str(instruction);
|
||||
}
|
||||
let annotations = if stored.is_mutation() {
|
||||
write_annotations(true)
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -31,15 +31,8 @@ pub struct StoredQuery {
|
|||
pub name: String,
|
||||
/// Full `.gq` source text the query was selected from.
|
||||
pub source: Arc<str>,
|
||||
/// Parsed declaration (params, mutations, description, …).
|
||||
/// Parsed declaration (params, mutations, description, `@mcp(...)`, …).
|
||||
pub decl: QueryDecl,
|
||||
/// Whether this query is listed in the MCP tool catalog (`GET /queries`).
|
||||
/// Default `true` (the manifest entry is the opt-in); `expose: false`
|
||||
/// keeps it HTTP/service-callable but hidden from the agent tool list.
|
||||
/// Catalog membership only — not an authorization gate.
|
||||
pub expose: bool,
|
||||
/// Optional MCP tool-name override; defaults to `name`.
|
||||
pub tool_name: Option<String>,
|
||||
}
|
||||
|
||||
impl StoredQuery {
|
||||
|
|
@ -49,13 +42,22 @@ impl StoredQuery {
|
|||
!self.decl.mutations.is_empty()
|
||||
}
|
||||
|
||||
/// The MCP tool name this query is catalogued under: the explicit
|
||||
/// `tool_name` override, else the query `name`. The catalog key —
|
||||
/// enforced unique across exposed queries at load. Server-side
|
||||
/// consumers (the uniqueness check, the future catalog projection) read
|
||||
/// this; the CLI `queries list` resolves the same rule on its own DTO.
|
||||
/// Whether this query is listed on the MCP tool surface (`GET /queries` and
|
||||
/// the `/mcp` tool catalog). From the source `@mcp(expose: …)` annotation;
|
||||
/// absent ⇒ `true`. An unexposed query stays HTTP/service-callable by name —
|
||||
/// this is **presentation only, not an authorization gate** (Cedar
|
||||
/// `invoke_query` is the authority for who may call it).
|
||||
pub fn is_exposed(&self) -> bool {
|
||||
self.decl.mcp.expose.unwrap_or(true)
|
||||
}
|
||||
|
||||
/// The MCP tool name this query is catalogued under: the source
|
||||
/// `@mcp(tool_name: …)` override, else the query `name`. The catalog key —
|
||||
/// enforced unique across exposed queries at load. Server-side consumers
|
||||
/// (the uniqueness check, the catalog/MCP projection) read this; the CLI
|
||||
/// `queries list` resolves the same rule on its own DTO.
|
||||
pub fn effective_tool_name(&self) -> &str {
|
||||
self.tool_name.as_deref().unwrap_or(&self.name)
|
||||
self.decl.mcp.tool_name.as_deref().unwrap_or(&self.name)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -67,13 +69,13 @@ pub struct QueryRegistry {
|
|||
|
||||
/// In-memory registry spec: a query's name + already-read `.gq` source. The
|
||||
/// input to [`QueryRegistry::from_specs`] — built by the server's cluster boot
|
||||
/// and by the CLI's `queries` tooling from a cluster serving snapshot.
|
||||
/// and by the CLI's `queries` tooling from a cluster serving snapshot. MCP
|
||||
/// presentation (`expose` / `tool_name`) is carried in the source `@mcp(...)`
|
||||
/// annotation, not here — see [`StoredQuery::is_exposed`] / `effective_tool_name`.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct RegistrySpec {
|
||||
pub name: String,
|
||||
pub source: String,
|
||||
pub expose: bool,
|
||||
pub tool_name: Option<String>,
|
||||
}
|
||||
|
||||
/// A single registry load failure. Collected (not fail-fast) so a bad
|
||||
|
|
@ -120,8 +122,6 @@ impl QueryRegistry {
|
|||
name: spec.name,
|
||||
source: Arc::from(spec.source),
|
||||
decl,
|
||||
expose: spec.expose,
|
||||
tool_name: spec.tool_name,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
|
@ -159,7 +159,7 @@ impl QueryRegistry {
|
|||
for builtin in crate::mcp::BUILTIN_TOOL_NAMES {
|
||||
claimed.insert(builtin, BUILTIN_OWNER);
|
||||
}
|
||||
for query in by_name.values().filter(|q| q.expose) {
|
||||
for query in by_name.values().filter(|q| q.is_exposed()) {
|
||||
let tool = query.effective_tool_name();
|
||||
if let Some(winner) = claimed.insert(tool, &query.name) {
|
||||
let message = if winner == BUILTIN_OWNER {
|
||||
|
|
@ -182,11 +182,11 @@ impl QueryRegistry {
|
|||
}
|
||||
}
|
||||
|
||||
/// Resolve by symbol name, **ignoring `expose`**. The raw catalog accessor
|
||||
/// for HTTP/service callers (`expose:false` queries are deliberately
|
||||
/// HTTP-callable; see [`StoredQuery::expose`]). The MCP backend must NOT use
|
||||
/// this — it resolves through [`Self::exposed_by_name`] so the agent surface
|
||||
/// can never reach a query hidden from the tool list.
|
||||
/// Resolve by symbol name, **ignoring exposure**. The raw catalog accessor
|
||||
/// for HTTP/service callers (`@mcp(expose: false)` queries are deliberately
|
||||
/// HTTP-callable; see [`StoredQuery::is_exposed`]). The MCP backend must NOT
|
||||
/// use this — it resolves through [`Self::exposed_by_name`] so the agent
|
||||
/// surface can never reach a query hidden from the tool list.
|
||||
pub fn lookup(&self, name: &str) -> Option<&StoredQuery> {
|
||||
self.by_name.get(name)
|
||||
}
|
||||
|
|
@ -201,14 +201,14 @@ impl QueryRegistry {
|
|||
/// `stored_query_list` tool, and per-query tool dispatch all funnel through
|
||||
/// it, so they cannot drift on which queries an agent may see or run.
|
||||
pub fn exposed(&self) -> impl Iterator<Item = &StoredQuery> {
|
||||
self.by_name.values().filter(|q| q.expose)
|
||||
self.by_name.values().filter(|q| q.is_exposed())
|
||||
}
|
||||
|
||||
/// Resolve by symbol name, **exposed-only** — the MCP `stored_query_run`
|
||||
/// resolver. An unexposed query is unreachable by name through this path
|
||||
/// even to a caller that knows the name (the agent surface honors `expose`).
|
||||
pub fn exposed_by_name(&self, name: &str) -> Option<&StoredQuery> {
|
||||
self.by_name.get(name).filter(|q| q.expose)
|
||||
self.by_name.get(name).filter(|q| q.is_exposed())
|
||||
}
|
||||
|
||||
pub fn is_empty(&self) -> bool {
|
||||
|
|
@ -284,7 +284,7 @@ pub fn check(registry: &QueryRegistry, catalog: &Catalog) -> CheckReport {
|
|||
message: err.to_string(),
|
||||
});
|
||||
}
|
||||
if query.expose {
|
||||
if query.is_exposed() {
|
||||
for param in &query.decl.params {
|
||||
// Resolve to the structured type via the compiler's own
|
||||
// resolver rather than string-matching `Vector(` — one
|
||||
|
|
@ -332,21 +332,34 @@ pub fn format_check_breakages(label: &str, report: &CheckReport) -> String {
|
|||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn spec(name: &str, source: &str, expose: bool) -> RegistrySpec {
|
||||
RegistrySpec {
|
||||
name: name.to_string(),
|
||||
source: source.to_string(),
|
||||
expose,
|
||||
tool_name: None,
|
||||
/// Inject an `@mcp(<args>)` annotation between the param-list `)` and the
|
||||
/// body `{` (the first `{` in a query source is always the body open).
|
||||
/// MCP presentation now lives in the source, so the test helpers express
|
||||
/// expose/tool_name by rewriting the `.gq` rather than via dead spec fields.
|
||||
fn inject_mcp(source: &str, args: &str) -> String {
|
||||
match source.find('{') {
|
||||
Some(i) => format!("{}@mcp({}) {}", &source[..i], args, &source[i..]),
|
||||
None => source.to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
fn spec(name: &str, source: &str, expose: bool) -> RegistrySpec {
|
||||
let source = if expose {
|
||||
source.to_string()
|
||||
} else {
|
||||
inject_mcp(source, "expose: false")
|
||||
};
|
||||
RegistrySpec { name: name.to_string(), source }
|
||||
}
|
||||
|
||||
fn spec_tool(name: &str, source: &str, expose: bool, tool_name: &str) -> RegistrySpec {
|
||||
let mut args = format!("tool_name: {tool_name:?}");
|
||||
if !expose {
|
||||
args.push_str(", expose: false");
|
||||
}
|
||||
RegistrySpec {
|
||||
name: name.to_string(),
|
||||
source: source.to_string(),
|
||||
expose,
|
||||
tool_name: Some(tool_name.to_string()),
|
||||
source: inject_mcp(source, &args),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -360,7 +373,7 @@ mod tests {
|
|||
.unwrap();
|
||||
let q = reg.lookup("find_user").unwrap();
|
||||
assert_eq!(q.name, "find_user");
|
||||
assert!(q.expose);
|
||||
assert!(q.is_exposed());
|
||||
assert_eq!(q.decl.params.len(), 1);
|
||||
assert!(!q.is_mutation());
|
||||
// No override → the effective tool name is the query name.
|
||||
|
|
|
|||
|
|
@ -77,11 +77,9 @@ pub(crate) async fn load_cluster_settings(
|
|||
.map(|query| queries::RegistrySpec {
|
||||
name: query.name.clone(),
|
||||
source: query.source.clone(),
|
||||
// The §D5 bridge: the cluster registry has no expose flag
|
||||
// (exposure becomes a policy decision in Phase 6) — cluster
|
||||
// mode lists every stored query.
|
||||
expose: true,
|
||||
tool_name: None,
|
||||
// MCP presentation (expose / tool_name) rides in the `.gq`
|
||||
// source `@mcp(...)` annotation, re-parsed here; the registry
|
||||
// spec carries only identity + source.
|
||||
})
|
||||
.collect();
|
||||
let registry = QueryRegistry::from_specs(specs).map_err(|errors| {
|
||||
|
|
@ -392,8 +390,6 @@ mod tests {
|
|||
let spec = |name: &str, source: &str| RegistrySpec {
|
||||
name: name.to_string(),
|
||||
source: source.to_string(),
|
||||
expose: false,
|
||||
tool_name: None,
|
||||
};
|
||||
|
||||
// Empty registry → nothing attached, no error.
|
||||
|
|
|
|||
|
|
@ -518,6 +518,65 @@ async fn write_tool_listed_when_only_unprotected_writes_allowed() {
|
|||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn stored_query_tool_folds_docs_and_honors_mcp_annotation() {
|
||||
// A query carrying @description + @instruction + a per-param @description +
|
||||
// @mcp(tool_name: …) projects as ONE tool whose name is the override, whose
|
||||
// description folds the instruction in, and whose input schema documents the
|
||||
// param — and it is callable under the override name (list == call).
|
||||
const SRC: &str = r#"query find_person(@description("the person's exact name") $name: String)
|
||||
@description("Find a person by name.")
|
||||
@instruction("Use only for an exact name; for fuzzy matches use search.")
|
||||
@mcp(tool_name: "lookup_person")
|
||||
{ match { $p: Person { name: $name } } return { $p.age } }"#;
|
||||
|
||||
let (_t, app) = app_with_stored_queries(
|
||||
&[("find_person", SRC, true)],
|
||||
&[("act-invoke", "tok")],
|
||||
INVOKE_POLICY_YAML,
|
||||
)
|
||||
.await;
|
||||
|
||||
let (_s, list) =
|
||||
json_response(&app, mcp_request(Some("tok"), rpc(1, "tools/list", json!({})))).await;
|
||||
let tools = list["result"]["tools"].as_array().unwrap();
|
||||
let tool = tools
|
||||
.iter()
|
||||
.find(|t| t["name"] == json!("lookup_person"))
|
||||
.unwrap_or_else(|| panic!("lookup_person not listed: {:?}", tool_names(&list)));
|
||||
// The @mcp tool name replaces the query name on the surface.
|
||||
assert!(
|
||||
!tool_names(&list).contains(&"find_person".to_string()),
|
||||
"query name must not double as a tool: {:?}",
|
||||
tool_names(&list)
|
||||
);
|
||||
// Description folds @description then @instruction.
|
||||
let desc = tool["description"].as_str().unwrap();
|
||||
assert!(desc.contains("Find a person by name."), "description: {desc}");
|
||||
assert!(desc.contains("Use only for an exact name"), "instruction folded in: {desc}");
|
||||
// The parameter carries its @description in the tool input schema.
|
||||
let param_desc =
|
||||
tool["inputSchema"]["properties"]["params"]["properties"]["name"]["description"].as_str();
|
||||
assert_eq!(
|
||||
param_desc,
|
||||
Some("the person's exact name"),
|
||||
"param doc in input schema: {}",
|
||||
tool["inputSchema"]
|
||||
);
|
||||
|
||||
// Callable under the override name (list and call agree).
|
||||
let (status, v) = json_response(
|
||||
&app,
|
||||
mcp_request(
|
||||
Some("tok"),
|
||||
rpc(2, "tools/call", json!({ "name": "lookup_person", "arguments": { "params": { "name": "Nobody" } } })),
|
||||
),
|
||||
)
|
||||
.await;
|
||||
assert_eq!(status, StatusCode::OK);
|
||||
assert_ne!(v["result"]["isError"], json!(true), "renamed tool not callable: {v}");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn per_query_mode_does_not_expose_meta_tools() {
|
||||
// Below the auto threshold the projection is per-query, so the discovery +
|
||||
|
|
@ -607,8 +666,6 @@ fn stored_query_shadowing_a_builtin_is_a_load_error() {
|
|||
let result = QueryRegistry::from_specs(vec![RegistrySpec {
|
||||
name: "graph_query".to_string(),
|
||||
source: "query graph_query() { match { $p: Person } return { $p.name } }".to_string(),
|
||||
expose: true,
|
||||
tool_name: None,
|
||||
}]);
|
||||
let errors = result.expect_err("expected a collision error");
|
||||
assert!(
|
||||
|
|
|
|||
|
|
@ -145,14 +145,24 @@ pub fn graph_path(root: &Path) -> PathBuf {
|
|||
}
|
||||
|
||||
pub fn stored_query_registry(specs: &[(&str, &str, bool)]) -> QueryRegistry {
|
||||
// MCP `expose` now lives in the `.gq` source `@mcp(...)` annotation. The
|
||||
// `(name, source, expose)` tuple stays for ergonomics: when `expose` is
|
||||
// false, inject `@mcp(expose: false)` between the param-list `)` and the
|
||||
// body `{` (the first `{` in a query source is always the body open), so
|
||||
// the real parse path is exercised.
|
||||
QueryRegistry::from_specs(
|
||||
specs
|
||||
.iter()
|
||||
.map(|(name, source, expose)| RegistrySpec {
|
||||
name: name.to_string(),
|
||||
source: source.to_string(),
|
||||
expose: *expose,
|
||||
tool_name: None,
|
||||
.map(|(name, source, expose)| {
|
||||
let source = if *expose {
|
||||
source.to_string()
|
||||
} else {
|
||||
match source.find('{') {
|
||||
Some(i) => format!("{}@mcp(expose: false) {}", &source[..i], &source[i..]),
|
||||
None => source.to_string(),
|
||||
}
|
||||
};
|
||||
RegistrySpec { name: name.to_string(), source }
|
||||
})
|
||||
.collect(),
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue