diff --git a/docs/automation/mcp_smoke.py b/docs/automation/mcp_smoke.py index ac6a0d74e..47d48f16a 100644 --- a/docs/automation/mcp_smoke.py +++ b/docs/automation/mcp_smoke.py @@ -53,8 +53,20 @@ def legacy(server: Path) -> None: assert names == TOOLS, names execute_request = definitions["ocs_execute"]["inputSchema"]["properties"]["request"] variants = execute_request.get("anyOf") or execute_request.get("oneOf") - assert len(variants) == 39 + # Resolve local references from the complete tool input schema. + root = definitions["ocs_execute"]["inputSchema"] + def resolve(variant): + if "$ref" not in variant: + return variant + ref = variant["$ref"] + assert ref.startswith("#/") + resolved = root + for part in ref[2:].split("/"): + resolved = resolved[part.replace("~1", "/").replace("~0", "~")] + return resolved + variants = [resolve(variant) for variant in variants] op_enum = execute_request["properties"]["op"]["enum"] + assert len(variants) == len(op_enum) for shipped in ("entities_create", "entities_delete", "entities_transform", "block_define", "block_delete", "file_identity", "xdata_set", "view_focus", "wblock", "plot", @@ -139,9 +151,12 @@ def modern(server: Path) -> None: "_meta": meta, }, }) - error = missing_cmd["result"]["structuredContent"] - assert error["code"] == "invalid_arguments" - assert "LINE 0,0 10,10" in error["error"] + # Tool errors are text content; the output schema covers successes only. + assert missing_cmd["result"]["isError"] is True + assert "structuredContent" not in missing_cmd["result"] + error = missing_cmd["result"]["content"][0]["text"] + assert "Missing cmd" in error + assert "LINE 0,0 10,10" in error invalid_mutation = request(process, { "jsonrpc": "2.0", @@ -154,7 +169,8 @@ def modern(server: Path) -> None: }, }) assert invalid_mutation["result"]["isError"] is True - assert "request_id" in invalid_mutation["result"]["structuredContent"]["error"] + assert "structuredContent" not in invalid_mutation["result"] + assert "request_id" in invalid_mutation["result"]["content"][0]["text"] rejected = request(process, { "jsonrpc": "2.0", diff --git a/src/mcp.rs b/src/mcp.rs index 4ffc27086..fc00e1189 100644 --- a/src/mcp.rs +++ b/src/mcp.rs @@ -31,7 +31,7 @@ const MODERN_PROTOCOL_VERSION: &str = "2026-07-28"; /// /// `block_delete` gained its optional `force` parameter. #[cfg(test)] -const TOOL_SCHEMA_DIGEST: &str = "c16572f139198d90807ede156370879106672ed97566c16ab6ae001c996059e8"; +const TOOL_SCHEMA_DIGEST: &str = "d597165afb46e8f067a77a3b827851b49be99d4739b15cf939b3be5b6c780bb1"; const MAX_REQUEST: usize = 1_048_576; const MAX_RESPONSE: u64 = 16 * 1024 * 1024; const CACHE_TTL_MS: u64 = 3_600_000; @@ -2252,6 +2252,17 @@ fn execute_request_schema() -> Value { crate::mcp_ops::execute_request_schema() } +// Local references resolve from the tool input schema root, not from request. +fn execute_input_schema() -> Value { + let mut schema = json!({"type":"object","properties":{"ocs_session_id":{"type":"string","minLength":1,"description":"Value of session_id returned by ocs_sessions."},"request":execute_request_schema(),"wait_seconds":{"type":"number","minimum":0,"maximum":60,"default":30,"description":"Total time to wait for completion before returning."},"response_detail":{"type":"string","enum":["compact","changed_entities","full"],"default":"compact","description":"compact returns only state needed for the next edit; changed_entities also returns current geometry for changed handles; full preserves the complete editor state."}},"required":["ocs_session_id","request"],"additionalProperties":false}); + let request = schema["properties"]["request"].as_object_mut().unwrap(); + let defs = request.remove("$defs").unwrap(); + let dialect = request.remove("$schema").unwrap(); + schema["$defs"] = defs; + schema["$schema"] = dialect; + schema +} + fn read_output_schema() -> Value { json!({ "type":"object", @@ -2297,7 +2308,7 @@ pub(crate) fn tool_definitions() -> Value { { "name":"ocs_execute", "description":"Execute semantic OCS actions, batch drafting, and mutations. Supports batch creation (entities_create), reference image underlays (embed_image), block definitions (block_define), and block reference insertions (INSERT). MANDATORY: For architectural drafting, all doors and windows must be inserted as CAD blocks (type: INSERT), never loose lines.", - "inputSchema":{"type":"object","properties":{"ocs_session_id":{"type":"string","minLength":1,"description":"Value of session_id returned by ocs_sessions."},"request":execute_request_schema(),"wait_seconds":{"type":"number","minimum":0,"maximum":60,"default":30,"description":"Total time to wait for completion before returning."},"response_detail":{"type":"string","enum":["compact","changed_entities","full"],"default":"compact","description":"compact returns only state needed for the next edit; changed_entities also returns current geometry for changed handles; full preserves the complete editor state."}},"required":["ocs_session_id","request"],"additionalProperties":false}, + "inputSchema":execute_input_schema(), "outputSchema":execute_output_schema(), "title":"Execute OCS action","annotations":{"readOnlyHint":false,"destructiveHint":true,"idempotentHint":true,"openWorldHint":false} }, @@ -3030,6 +3041,11 @@ mod tests { let find_variant = |op_name: &str| { request_variants .iter() + .map(|v| { + tools[2]["inputSchema"] + .pointer(v["$ref"].as_str().unwrap().strip_prefix('#').unwrap()) + .expect("operation reference resolves") + }) .find(|v| v["properties"]["op"]["enum"][0] == op_name) .cloned() .unwrap_or_else(|| panic!("variant for {op_name} exists")) @@ -4266,6 +4282,65 @@ mod tests { ); } + #[test] + fn execute_schema_shares_operations_and_resolves_local_references() { + fn check_refs(value: &Value, root: &Value) { + match value { + Value::Object(object) => { + if let Some(reference) = object.get("$ref") { + let pointer = reference.as_str().unwrap().strip_prefix('#').unwrap(); + assert!(root.pointer(pointer).is_some(), "unresolved {reference}"); + } + for child in object.values() { + check_refs(child, root); + } + } + Value::Array(array) => { + for child in array { + check_refs(child, root); + } + } + _ => {} + } + } + let schema = execute_input_schema(); + check_refs(&schema, &schema); + let standalone = execute_request_schema(); + check_refs(&standalone, &standalone); + assert!(schema["properties"]["request"].get("$defs").is_none()); + let variants = schema["properties"]["request"]["anyOf"].as_array().unwrap(); + let batch = schema["$defs"]["batch_step"]["anyOf"].as_array().unwrap(); + for (op, variant) in crate::mcp_ops::OPS.iter().zip(variants) { + assert_eq!(variant, &json!({"$ref":format!("#/$defs/op_{}", op.name)})); + assert_eq!(batch.contains(variant), op.batchable); + assert_eq!( + schema["$defs"][format!("op_{}", op.name)], + crate::mcp_ops::op_schema(op) + ); + } + + // Reconstruct the old duplicated layout using the same operation + // definitions, so the size comparison stays meaningful as ops evolve. + let mut old = standalone.clone(); + old["anyOf"] = json!(crate::mcp_ops::OPS + .iter() + .map(|op| crate::mcp_ops::op_schema(op)) + .collect::>()); + old["$defs"]["batch_step"]["anyOf"] = json!(crate::mcp_ops::OPS + .iter() + .filter(|op| op.batchable) + .map(|op| crate::mcp_ops::op_schema(op)) + .collect::>()); + old["$defs"] + .as_object_mut() + .unwrap() + .retain(|key, _| !key.starts_with("op_")); + let before = old.to_string().len(); + let after = standalone.to_string().len(); + eprintln!("Execute request schema: {before} -> {after} bytes"); + assert!(after * 100 < before * 70, "expected at least 30% reduction"); + } + #[test] fn schema_size_and_defs_structure() { let schema = execute_request_schema(); diff --git a/src/mcp_ops.rs b/src/mcp_ops.rs index b1692d546..80fd5407e 100644 --- a/src/mcp_ops.rs +++ b/src/mcp_ops.rs @@ -935,26 +935,20 @@ fn ty_schema(ty: &Ty) -> Value { } pub fn shared_defs() -> Value { - let batch_variants: Vec = OPS - .iter() - .filter(|op| op.batchable) - .map(|op| op_schema(op)) - .collect(); - - json!({ + let mut defs = json!({ "point": {"type": "array", "items": {"type": "number"}, "minItems": 2, "maxItems": 3}, "window": {"type": "array", "items": {"type": "number"}, "minItems": 4, "maxItems": 4}, "handle": {"type": "string", "pattern": "^(0[xX])?[0-9A-Fa-f]+$"}, - "batch_step": { - "type": "object", - "properties": { - "op": {"type": "string"}, - "document_id": {"type": "integer", "minimum": 0} - }, - "required": ["op"], - "anyOf": batch_variants - } - }) + "batch_step": batch_step_schema() + }); + for op in OPS { + defs[format!("op_{}", op.name)] = op_schema(op); + } + defs +} + +fn op_ref(op: &OpDef) -> Value { + json!({"$ref": format!("#/$defs/op_{}", op.name)}) } fn rules_text(op: &OpDef) -> String { @@ -996,7 +990,7 @@ pub fn op_schema(op: &OpDef) -> Value { } pub fn execute_request_schema() -> Value { - let variants: Vec = OPS.iter().map(|op| op_schema(op)).collect(); + let variants: Vec = OPS.iter().map(|op| op_ref(op)).collect(); let op_names: Vec<&str> = OPS.iter().map(|op| op.name).collect(); json!({ "$schema": "https://json-schema.org/draft/2020-12/schema", @@ -1021,7 +1015,7 @@ pub fn batch_step_schema() -> Value { let batch_variants: Vec = OPS .iter() .filter(|op| op.batchable) - .map(|op| op_schema(op)) + .map(|op| op_ref(op)) .collect(); json!({