diff --git a/docs/automation/guides/workflow.md b/docs/automation/guides/workflow.md index 5fd801bf8..0314f0a7e 100644 --- a/docs/automation/guides/workflow.md +++ b/docs/automation/guides/workflow.md @@ -8,7 +8,20 @@ Before creating an unfamiliar entity, use `ocs_read` with `op: entity_schema` an `parameters.type` (for example, `Circle`) to get its creation fields, required values, units, defaults and example. Omit type for the catalog. Creation schemas describe the inputs to `entities_create`; `record_schema` describes stored records. -Schema discovery runs locally and can omit `ocs_session_id`. + +Use `ocs_read` with `op: validate` and `parameters.request` containing the proposed +execute request to preflight it without running it. These local operations can +omit `ocs_session_id`; no GUI is launched or contacted. `request_id` may be omitted +for preflight, but is required for actual execution. Static validation checks +operation parameters, nested entity and inline data definitions, and batch +bindings. It uses the real entity constructor for geometry checks. Results +include `valid`, `executed: false`, warnings, unresolved bindings and checks that +still require execution. Live revisions, handles, styles, file access and command +behavior are not certified by static preflight. Batches are not simulated: later +bindings are fully validated when their values become available during execution. +Invalid requests return a tool error with a JSON Pointer identifying the bad +field where available. Unknown top-level operation parameters remain warnings; +unknown entity fields and invalid nested data are rejected before dispatch. For interactive work, call start and follow state.command.accepts, options and input_example. To have the person at the screen pick entities for you, call user_select and keep polling until it completes; running means they are still picking. A run.cmd contains the command name followed by prompt answers separated by spaces; points use x,y or x,y,z. diff --git a/docs/automation/mcp_smoke.py b/docs/automation/mcp_smoke.py index 37a3b1edc..64ff3cf93 100644 --- a/docs/automation/mcp_smoke.py +++ b/docs/automation/mcp_smoke.py @@ -144,6 +144,26 @@ def legacy(server: Path) -> None: assert not entity_schema["result"].get("isError"), entity_schema contract = entity_schema["result"]["structuredContent"]["schema"] assert contract["required"] == ["type", "center", "radius"] + validated = request(process, { + "jsonrpc": "2.0", "id": "preflight", "method": "tools/call", + "params": {"name": "ocs_read", "arguments": { + "op": "validate", "parameters": {"request": { + "op": "entities_create", "entities": contract["examples"], + }}, + }}, + }) + assert validated["result"]["structuredContent"]["valid"] is True + assert validated["result"]["structuredContent"]["executed"] is False + rejected = request(process, { + "jsonrpc": "2.0", "id": "bad-preflight", "method": "tools/call", + "params": {"name": "ocs_read", "arguments": { + "op": "validate", "parameters": {"request": { + "op": "entities_create", "entities": [{"type": "Circle", "center": ["bad", 0], "radius": 5}], + }}, + }}, + }) + assert rejected["result"]["isError"] is True + assert "/entities/0/center/0" in rejected["result"]["content"][0]["text"] close(process) diff --git a/src/mcp.rs b/src/mcp.rs index 9383085fa..3c41b96de 100644 --- a/src/mcp.rs +++ b/src/mcp.rs @@ -3,6 +3,7 @@ //! `OpenCADStudio --mcp` speaks MCP over stdio. All drawing work is forwarded //! to the authenticated GUI control bridge; this module contains no geometry. +use crate::mcp_ops::{batch_bindings, validate_execute_request}; use base64::{engine::general_purpose::STANDARD as BASE64, Engine as _}; use serde::Deserialize; use serde_json::{json, Map, Value}; @@ -31,7 +32,7 @@ const MODERN_PROTOCOL_VERSION: &str = "2026-07-28"; /// /// `block_delete` gained its optional `force` parameter. #[cfg(test)] -const TOOL_SCHEMA_DIGEST: &str = "8b63b7c6860820cfb05b69d634c7897dec202ad3c7d94af5518f448ff5c8c1b3"; +const TOOL_SCHEMA_DIGEST: &str = "96510df2d38cf3f7feb99e50e9491dda6fc1ee9221dc82020303df918292f580"; const MAX_REQUEST: usize = 1_048_576; const MAX_RESPONSE: u64 = 16 * 1024 * 1024; const CACHE_TTL_MS: u64 = 3_600_000; @@ -39,7 +40,7 @@ const CACHE_TTL_MS: u64 = 3_600_000; /// land. Tools and discovery are static by comparison, so they keep the hour. const RESOURCE_TTL_MS: u64 = 60_000; const TASK_TTL_MS: u64 = 3_600_000; -pub(crate) const INSTRUCTIONS: &str = "Call ocs_sessions and pass session_id as ocs_session_id. Read capabilities and record_schema before unfamiliar operations. Use entity_schema for creation fields. Preserve document_id, revision and request_id. After a timeout query the existing operation; never replay a mutation with a new request_id. running and waiting_input are not completion: follow state.command.accepts, options and input_example. Let the geometry kernel calculate geometry. Before delivery audit the target format/version and use save_verified with an absolute path. Announce the bridge build on first connection and when it changes. Use batch with response_detail: summary for known steps; bindings can reuse earlier results. Read guides on demand via cad://guides/{name} or ocs_read op: guide with parameters.name: workflow, architecture, capture, batch-bindings. Read architecture before floorplan vectorization and capture before advanced visual verification. Omit parameters.name to list guides."; +pub(crate) const INSTRUCTIONS: &str = "Call ocs_sessions and pass session_id as ocs_session_id. Read capabilities and record_schema before unfamiliar operations. Use entity_schema for creation fields and validate for preflight. Preserve document_id, revision and request_id. After a timeout query the existing operation; never replay a mutation with a new request_id. running and waiting_input are not completion: follow state.command.accepts, options and input_example. Let the geometry kernel calculate geometry. Before delivery audit the target format/version and use save_verified with an absolute path. Announce the bridge build on first connection and when it changes. Use batch with response_detail: summary for known steps; bindings can reuse earlier results. Read guides on demand via cad://guides/{name} or ocs_read op: guide with parameters.name: workflow, architecture, capture, batch-bindings. Read architecture before floorplan vectorization and capture before advanced visual verification. Omit parameters.name to list guides."; const READ_OPS: &[&str] = &[ "state", "hello", @@ -62,6 +63,7 @@ const READ_OPS: &[&str] = &[ "batch_result", "guide", "entity_schema", + "validate", "xdata_get", "audit", "text_search", @@ -112,48 +114,6 @@ const EXECUTE_OPS: &[&str] = &[ "stop", "batch", ]; -const BATCH_STEP_OPS: &[&str] = &[ - "new", - "open", - "activate", - "switch_document", - "run", - "start", - "input", - "cancel", - "undo", - "redo", - "select", - "property", - "grip_drag", - "click", - "set_properties", - "action", - "embed_image", - "wblock", - "plot", - "entities_create", - "entities_delete", - "entities_transform", - "text_replace", - "block_define", - "block_delete", - "xdata_set", - "view_focus", - "entities_copy_to", - "group_create", - "selection_set_save", - "selection_set_load", - "user_select", - "getpoint", - "close", - "sysvar", - "layout_create", - "page_setup_set", - "file_identity", - "save", - "stop", -]; const MAX_BATCH_STEPS: usize = 64; #[derive(Clone, Deserialize)] @@ -1540,54 +1500,6 @@ fn required_string<'a>(arguments: &'a Value, key: &str) -> Result<&'a str, Strin .ok_or_else(|| format!("Missing {key}")) } -/// Validate bindings without dispatching or substituting untyped placeholder values. -fn batch_bindings(step: &Value, index: usize) -> Result, String> { - let Some(bindings) = step.get("bindings") else { - return Ok(Vec::new()); - }; - let bindings = bindings - .as_object() - .filter(|v| !v.is_empty()) - .ok_or("bindings must be a nonempty object")?; - let op = crate::mcp_ops::find_op(step["op"].as_str().unwrap_or_default()) - .ok_or("unknown bound operation")?; - for (target, source) in bindings { - if !op.params.iter().any(|p| p.name == target) { - return Err(format!("cannot bind parameter '{target}'")); - } - if step.get(target).is_some() { - return Err(format!("'{target}' has both a literal and a binding")); - } - let source = source - .as_object() - .ok_or("binding must contain step and path")?; - if source.len() != 2 - || !source - .get("step") - .and_then(Value::as_u64) - .is_some_and(|s| s < index as u64) - { - return Err( - "binding must refer to an earlier step and contain only step and path".into(), - ); - } - let path = source - .get("path") - .and_then(Value::as_str) - .ok_or("binding path must be a JSON Pointer")?; - if !path.is_empty() && !path.starts_with('/') { - return Err("binding path must be a JSON Pointer".into()); - } - let mut chars = path.chars(); - while let Some(c) = chars.next() { - if c == '~' && !matches!(chars.next(), Some('0' | '1')) { - return Err("invalid JSON Pointer escape".into()); - } - } - } - Ok(bindings.keys().map(String::as_str).collect()) -} - fn resolve_batch_step(step: &Value, index: usize, results: &[Value]) -> Result { batch_bindings(step, index)?; let mut resolved = step.clone(); @@ -1614,49 +1526,29 @@ fn resolve_batch_step(step: &Value, index: usize, results: &[Value]) -> Result Result { - if op == "batch" { - let steps = request["steps"].as_array().ok_or_else(|| { - r#"Missing steps for batch. Example request: {"op":"batch","request_id":"draw-1","steps":[{"op":"run","cmd":"LINE 0,0 10,0"}]}"#.to_string() - })?; - if steps.is_empty() || steps.len() > MAX_BATCH_STEPS { - return Err(format!( - "batch steps must contain 1 to {MAX_BATCH_STEPS} operations" - )); +fn preflight_request(request: &Value) -> Value { + let result = + required_string(request, "op").and_then(|op| validate_execute_request(request, op)); + match result { + Ok(validated) => { + let deferred: Vec = request["steps"] + .as_array() + .into_iter() + .flatten() + .enumerate() + .filter_map(|(index, step)| { + step.get("bindings") + .map(|bindings| json!({"step":index,"bindings":bindings})) + }) + .collect(); + json!({"ok":true,"valid":true,"scope":"static","warnings":validated.warnings,"deferred_bindings":deferred, + "not_checked":["live document revision, handles, blocks, styles and record property write rules","file access, command execution and non-creation geometry operations","values resolved from earlier batch results"], + "executed":false}) } - let mut warnings = Vec::new(); - for (index, step) in steps.iter().enumerate() { - let step_obj = step - .as_object() - .ok_or_else(|| format!("batch step {index} must be an object"))?; - if step_obj.contains_key("request_id") { - return Err(format!( - "batch step {index} must omit request_id; the batch assigns idempotency keys" - )); - } - let step_op = step_obj - .get("op") - .and_then(Value::as_str) - .ok_or_else(|| format!("batch step {index} is missing op"))?; - if !BATCH_STEP_OPS.contains(&step_op) { - return Err(format!("Unknown operation {step_op} in batch step {index}")); - } - let deferred = batch_bindings(step, index) - .map_err(|error| format!("batch step {index}: {error}"))?; - let mut literal = step.clone(); - literal.as_object_mut().unwrap().remove("bindings"); - let step_val = crate::mcp_ops::validate_request_deferred(&literal, true, &deferred) - .map_err(|error| format!("batch step {index}: {error}"))?; - for w in step_val.warnings { - warnings.push(format!("batch step {index}: {w}")); - } + Err(error) => { + json!({"ok":false,"valid":false,"scope":"static","executed":false,"code":"validation_failed","error":error}) } - return Ok(crate::mcp_ops::ValidationResult { warnings }); } - crate::mcp_ops::validate_request(request, false) } fn compact_state(state: &Value) -> Value { @@ -2067,20 +1959,22 @@ fn call_tool( Ok(Value::Array(sessions(launch).map_err(CallError::Infra)?)) } "ocs_read" => { - if arguments["op"] == "entity_schema" { + let op = arguments["op"].as_str().unwrap_or("state"); + if !READ_OPS.contains(&op) { + return Err("Use ocs_execute for mutations".to_string().into()); + } + if op == "entity_schema" { return match arguments["parameters"]["type"].as_str() { Some(kind) => Ok( - json!({"ok":true,"schema":crate::entity_creation::schema(kind)?,"validation":"Creation inputs are checked by the shared entity constructor; live document checks run on execution."}), + json!({"ok":true,"schema":crate::entity_creation::schema(kind)?,"validation":"Use op validate with parameters.request to preflight; live document checks still run on execution."}), ), None => Ok( json!({"ok":true,"types":crate::entity_creation::TYPES,"aliases":{"Polyline":"LwPolyline"},"next":"Set parameters.type for one creation schema and example."}), ), }; } - let session_id = required_string(arguments, "ocs_session_id")?; - let op = arguments["op"].as_str().unwrap_or("state"); - if !READ_OPS.contains(&op) { - return Err("Use ocs_execute for mutations".to_string().into()); + if op == "validate" { + return Ok(preflight_request(&arguments["parameters"]["request"])); } if op == "guide" { let mut result = match arguments["parameters"]["name"].as_str() { @@ -2091,6 +1985,7 @@ fn call_tool( result["status"] = json!("completed"); return Ok(result); } + let session_id = required_string(arguments, "ocs_session_id")?; if op == "batch_result" { let request_id = required_string(&arguments["parameters"], "request_id")?; return cached_batch_result(clients, session_id, request_id); @@ -2547,8 +2442,8 @@ pub(crate) fn tool_definitions() -> Value { }, { "name":"ocs_read", - "description":"Use entity_schema with parameters.type for creation fields/examples; omit type to list. Use guide with parameters.name to read workflow, architecture, capture or batch-bindings guidance (omit name to list). Discover capabilities and record schemas, or read state, complete database records, command manifests, entities, properties, kernel measurements and spatial relationships, history, events or operation status from a live OCS session. Use batch_result with parameters.request_id to retrieve retained batch details without executing steps; available only in the originating MCP connection, for up to 64 retained batches per session.", - "inputSchema":{"type":"object","properties":{"ocs_session_id":{"type":"string","minLength":1,"description":"Value of session_id returned by ocs_sessions."},"op":{"type":"string","enum":READ_OPS,"default":"state"},"parameters":{"type":"object","description":"Operation-specific filters.","properties":{"name":{"type":"string","description":"Command name or record name."},"search":{"type":"string","description":"Case-insensitive command or record-type search."},"find":{"type":"string","description":"Text query string for text_search."},"match_case":{"type":"boolean","default":false,"description":"Case-sensitive text search."},"whole_word":{"type":"boolean","default":false,"description":"Match only whole words."},"ignore_accents":{"type":"boolean","description":"Ignore accents/diacritics in text (defaults to true when match_case is false)."},"scope":{"type":"string","enum":["all","active_space","blocks"],"default":"all","description":"Text search scope."},"system_spellcheck":{"type":"boolean","default":false,"description":"Use native OS spell-checker (Windows, macOS, Linux)."},"language":{"type":"string","description":"Language tag for spell-checker (e.g. 'fr-FR', 'en-US', 'de-DE', 'es-ES')."},"suggest":{"type":"boolean","default":true,"description":"Include suggested corrections for misspelled words."},"check_terms":{"type":"array","items":{"type":"string"},"description":"List of terms or suspect misspellings to flag in text_audit."},"dictionary":{"type":"array","items":{"type":"string"},"description":"Known valid words for text_audit dictionary check."},"pairs":{"type":"array","description":"Find/replace pairs for dry-run simulation in text_audit.","items":{"type":"object","properties":{"find":{"type":"string"},"replace":{"type":"string"}},"required":["find","replace"]}},"dry_run_pairs":{"type":"array","description":"Alias for pairs in text_audit.","items":{"type":"object","properties":{"find":{"type":"string"},"replace":{"type":"string"}},"required":["find","replace"]}},"document_id":{"type":"integer","minimum":0},"path":{"type":"string","description":"Optional intended output path for audit; extension determines target format."},"target_format":{"type":"string","enum":["dwg","dxf"],"description":"Intended output format for audit."},"target_version":{"type":"string","enum":["R14","2000","2004","2007","2010","2013","2018","AC1014","AC1015","AC1018","AC1021","AC1024","AC1027","AC1032"],"description":"Intended CAD output version for audit."},"collection":{"type":"string","description":"Record collection, all for records, or omit to discover collections and schema types."},"handle":{"type":"string"},"handles":{"type":"array","items":{"type":"string"},"description":"Exact entity or record handles."},"type":{"type":"string","description":"Entity or record type filter; for record_schema, returns its complete type graph and writable field paths."},"layer":{"type":"string","description":"Layer name filter for query."},"detail":{"type":"string","enum":["summary","geometry","full"],"default":"geometry","description":"Entity detail returned by query."},"fields":{"type":"array","items":{"type":"string"},"description":"Return only these entity fields plus handle."},"paths":{"type":"array","items":{"type":"string"},"description":"Project RFC 6901 JSON Pointer paths relative to record.properties."},"where":{"type":"array","description":"All property filters must match.","items":{"type":"object","properties":{"path":{"type":"string"},"op":{"type":"string","enum":["eq","ne","lt","lte","gt","gte","contains","starts_with","ends_with","in","exists","not_exists"],"default":"eq"},"value":{}},"required":["path"],"additionalProperties":false}},"near":{"type":"array","items":{"type":"number"},"minItems":2,"maxItems":3,"description":"Rank planar curves by exact kernel distance to this world XY point."},"point":{"type":"array","items":{"type":"number"},"minItems":2,"maxItems":3,"description":"World point whose object snap the snap op reports."},"from":{"type":"array","items":{"type":"number"},"minItems":2,"maxItems":3,"description":"Base point for perpendicular and tangent snaps (snap op)."},"contains_point":{"type":"array","items":{"type":"number"},"minItems":2,"maxItems":3,"description":"Return closed planar curves containing this world XY point."},"bounds":{"type":"array","items":{"type":"number"},"minItems":4,"maxItems":4,"description":"Filter entities whose world XY bounds overlap [min_x,min_y,max_x,max_y]."},"intersections":{"type":"array","items":{"type":"string"},"minItems":2,"maxItems":2,"description":"Return exact kernel intersections between two planar curve handles."},"after":{"type":"integer","minimum":0,"description":"Event cursor."},"request_id":{"type":"string","description":"Operation id to query."},"offset":{"type":"integer","minimum":0},"limit":{"type":"integer","minimum":1,"maximum":10000}},"additionalProperties":false}},"anyOf":[{"required":["ocs_session_id"]},{"required":["op"],"properties":{"op":{"enum":["entity_schema"]}}}],"additionalProperties":false}, + "description":"Use entity_schema with parameters.type for creation fields/examples; omit type to list. Use validate with parameters.request to check an execute request without running it (no live document checks). Use guide with parameters.name to read workflow, architecture, capture or batch-bindings guidance (omit name to list). Discover capabilities and record schemas, or read state, complete database records, command manifests, entities, properties, kernel measurements and spatial relationships, history, events or operation status from a live OCS session. Use batch_result with parameters.request_id to retrieve retained batch details without executing steps; available only in the originating MCP connection, for up to 64 retained batches per session.", + "inputSchema":{"type":"object","properties":{"ocs_session_id":{"type":"string","minLength":1,"description":"Value of session_id returned by ocs_sessions."},"op":{"type":"string","enum":READ_OPS,"default":"state"},"parameters":{"type":"object","description":"Operation-specific filters.","properties":{"request":{"type":"object","description":"Execute request to preflight without dispatch. request_id may be omitted. Batch bindings are checked structurally; result-dependent values require execution."},"name":{"type":"string","description":"Command name or record name."},"search":{"type":"string","description":"Case-insensitive command or record-type search."},"find":{"type":"string","description":"Text query string for text_search."},"match_case":{"type":"boolean","default":false,"description":"Case-sensitive text search."},"whole_word":{"type":"boolean","default":false,"description":"Match only whole words."},"ignore_accents":{"type":"boolean","description":"Ignore accents/diacritics in text (defaults to true when match_case is false)."},"scope":{"type":"string","enum":["all","active_space","blocks"],"default":"all","description":"Text search scope."},"system_spellcheck":{"type":"boolean","default":false,"description":"Use native OS spell-checker (Windows, macOS, Linux)."},"language":{"type":"string","description":"Language tag for spell-checker (e.g. 'fr-FR', 'en-US', 'de-DE', 'es-ES')."},"suggest":{"type":"boolean","default":true,"description":"Include suggested corrections for misspelled words."},"check_terms":{"type":"array","items":{"type":"string"},"description":"List of terms or suspect misspellings to flag in text_audit."},"dictionary":{"type":"array","items":{"type":"string"},"description":"Known valid words for text_audit dictionary check."},"pairs":{"type":"array","description":"Find/replace pairs for dry-run simulation in text_audit.","items":{"type":"object","properties":{"find":{"type":"string"},"replace":{"type":"string"}},"required":["find","replace"]}},"dry_run_pairs":{"type":"array","description":"Alias for pairs in text_audit.","items":{"type":"object","properties":{"find":{"type":"string"},"replace":{"type":"string"}},"required":["find","replace"]}},"document_id":{"type":"integer","minimum":0},"path":{"type":"string","description":"Optional intended output path for audit; extension determines target format."},"target_format":{"type":"string","enum":["dwg","dxf"],"description":"Intended output format for audit."},"target_version":{"type":"string","enum":["R14","2000","2004","2007","2010","2013","2018","AC1014","AC1015","AC1018","AC1021","AC1024","AC1027","AC1032"],"description":"Intended CAD output version for audit."},"collection":{"type":"string","description":"Record collection, all for records, or omit to discover collections and schema types."},"handle":{"type":"string"},"handles":{"type":"array","items":{"type":"string"},"description":"Exact entity or record handles."},"type":{"type":"string","description":"Entity or record type filter; for record_schema, returns its complete type graph and writable field paths."},"layer":{"type":"string","description":"Layer name filter for query."},"detail":{"type":"string","enum":["summary","geometry","full"],"default":"geometry","description":"Entity detail returned by query."},"fields":{"type":"array","items":{"type":"string"},"description":"Return only these entity fields plus handle."},"paths":{"type":"array","items":{"type":"string"},"description":"Project RFC 6901 JSON Pointer paths relative to record.properties."},"where":{"type":"array","description":"All property filters must match.","items":{"type":"object","properties":{"path":{"type":"string"},"op":{"type":"string","enum":["eq","ne","lt","lte","gt","gte","contains","starts_with","ends_with","in","exists","not_exists"],"default":"eq"},"value":{}},"required":["path"],"additionalProperties":false}},"near":{"type":"array","items":{"type":"number"},"minItems":2,"maxItems":3,"description":"Rank planar curves by exact kernel distance to this world XY point."},"point":{"type":"array","items":{"type":"number"},"minItems":2,"maxItems":3,"description":"World point whose object snap the snap op reports."},"from":{"type":"array","items":{"type":"number"},"minItems":2,"maxItems":3,"description":"Base point for perpendicular and tangent snaps (snap op)."},"contains_point":{"type":"array","items":{"type":"number"},"minItems":2,"maxItems":3,"description":"Return closed planar curves containing this world XY point."},"bounds":{"type":"array","items":{"type":"number"},"minItems":4,"maxItems":4,"description":"Filter entities whose world XY bounds overlap [min_x,min_y,max_x,max_y]."},"intersections":{"type":"array","items":{"type":"string"},"minItems":2,"maxItems":2,"description":"Return exact kernel intersections between two planar curve handles."},"after":{"type":"integer","minimum":0,"description":"Event cursor."},"request_id":{"type":"string","description":"Operation id to query."},"offset":{"type":"integer","minimum":0},"limit":{"type":"integer","minimum":1,"maximum":10000}},"additionalProperties":false}},"anyOf":[{"required":["ocs_session_id"]},{"required":["op"],"properties":{"op":{"enum":["guide","entity_schema","validate"]}}}],"additionalProperties":false}, "outputSchema":read_output_schema(), "title":"Read OCS state","annotations":{"readOnlyHint":true,"destructiveHint":false,"idempotentHint":true,"openWorldHint":false} }, @@ -3264,7 +3159,7 @@ mod tests { .contains(&json!("ocs_session_id"))); } let read_schema = &tools[1]["inputSchema"]; - for op in ["entity_schema"] { + for op in ["guide", "entity_schema", "validate"] { crate::schema_validation::validate_shape(read_schema, &json!({"op":op}), "").unwrap(); } assert!( @@ -4599,35 +4494,45 @@ mod tests { } #[test] - fn entity_creation_schemas_are_local_and_specific() { + fn entity_schemas_and_preflight_are_local_and_never_dispatch() { let mut clients = HashMap::new(); let mut resources = ResourceStore::default(); - let catalog = call_tool( - "ocs_read", - &json!({"op":"entity_schema"}), - &mut clients, - &mut resources, - &mut detached_pump(), - None, - ) - .unwrap(); + let mut call = |op: &str, parameters: Value| { + call_tool( + "ocs_read", + &json!({"op":op,"parameters":parameters}), + &mut clients, + &mut resources, + &mut detached_pump(), + None, + ) + .unwrap() + }; + let catalog = call("entity_schema", json!({})); assert_eq!(catalog["types"].as_array().unwrap().len(), 11); assert!(catalog.get("schema").is_none()); - let circle = call_tool( - "ocs_read", - &json!({"op":"entity_schema","parameters":{"type":"Circle"}}), - &mut clients, - &mut resources, - &mut detached_pump(), - None, - ) - .unwrap(); + let circle = call("entity_schema", json!({"type":"Circle"})); assert_eq!( circle["schema"]["required"], json!(["type", "center", "radius"]) ); - crate::schema_validation::validate_shape(&read_output_schema(), &circle, "").unwrap(); + let request = json!({"op":"entities_create","entities":[{"type":"Circle","center":[0,0],"radius":2}]}); + let validated = call("validate", json!({"request":request})); + assert_eq!(validated["valid"], true); + assert_eq!(validated["executed"], false); + assert_eq!(validated["scope"], "static"); + crate::schema_validation::validate_shape(&read_output_schema(), &validated, "").unwrap(); + let invalid = call( + "validate", + json!({"request":{"op":"entities_create","entities":[{"type":"Circle","center":["bad",0],"radius":2}]}}), + ); + assert_eq!(invalid["valid"], false); + assert!(invalid["error"] + .as_str() + .unwrap() + .contains("/entities/0/center/0")); assert!(clients.is_empty()); + assert!(resources.latest.is_empty()); let rejected = call_tool( "ocs_execute", &json!({"ocs_session_id":"offline","request":{"op":"entities_create","request_id":"invalid","entities":[{"type":"Circle","center":[0,0],"radius":-1}]}}), @@ -4640,6 +4545,30 @@ mod tests { assert!(clients.is_empty()); } + #[test] + fn preflight_checks_nested_data_envelopes_and_deferred_bindings() { + let malformed = json!({"op":"set_properties","collection":"entities","handle":"A","updates":[{"path":123,"value":"Walls"}]}); + let error = preflight_request(&malformed); + assert_eq!(error["valid"], false); + assert!(error["error"].as_str().unwrap().contains("/updates/0/path")); + for request in [ + json!({"op":"run","cmd":"LINE","document_id":-1}), + json!({"op":"run","cmd":"LINE","request_id":""}), + json!({"op":"run","cmd":"LINE","selection":[42]}), + json!({"op":"not_an_operation"}), + ] { + assert_eq!(preflight_request(&request)["valid"], false); + } + let batch = json!({"op":"batch","steps":[{"op":"run","cmd":"LINE"},{"op":"group_create","name":"Walls","bindings":{"handles":{"step":0,"path":"/result/handles"}}}]}); + let validated = preflight_request(&batch); + assert_eq!(validated["valid"], true); + assert_eq!(validated["deferred_bindings"][0]["step"], 1); + assert!(!validated["not_checked"].as_array().unwrap().is_empty()); + let mut invalid = batch; + invalid["steps"][1]["bindings"]["handles"]["step"] = json!(1); + assert_eq!(preflight_request(&invalid)["valid"], false); + } + #[test] fn guides_are_discoverable_without_a_gui_session() { assert!(INSTRUCTIONS.len() < 1100); diff --git a/src/mcp_ops.rs b/src/mcp_ops.rs index 83c7fef57..5806ef67b 100644 --- a/src/mcp_ops.rs +++ b/src/mcp_ops.rs @@ -41,7 +41,8 @@ pub enum Ty { ArrayOf(&'static Ty), /// Entity definitions with on-demand creation schemas. Entities, - /// Escape hatch: a JSON Schema fragment as text. + BatchSteps, + /// Inline JSON Schema fragment, validated before dispatch. Raw(&'static str), } @@ -855,7 +856,7 @@ pub static BATCH: OpDef = OpDef { doc: "Execute sequential operations with fresh state and step idempotency keys. Stops on first error.", batchable: false, params: &[ - req("steps", Ty::Raw(r##"{"type":"array","minItems":1,"maxItems":64,"description":"Sequential operations list","items":{"$ref":"#/$defs/batch_step"}}"##), "List of batch steps to execute."), + req("steps", Ty::BatchSteps, "List of batch steps to execute."), ], rules: &[], example: r#"{"op":"batch","steps":[{"op":"run","cmd":"LINE 0,0 10,0"}]}"#, @@ -930,6 +931,9 @@ fn ty_schema(ty: &Ty) -> Value { json!({"type": "string", "enum": names}) } Ty::ArrayOf(inner) => json!({"type": "array", "items": ty_schema(inner)}), + Ty::BatchSteps => { + json!({"type":"array","minItems":1,"maxItems":64,"items":{"$ref":"#/$defs/batch_step"}}) + } Ty::Entities => { json!({"type":"array","minItems":1,"items":{"type":"object","required":["type"],"properties":{"type":{"type":"string"}}}}) } @@ -937,13 +941,17 @@ fn ty_schema(ty: &Ty) -> Value { } } +fn primitive_defs() -> Value { + 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]+$"} + }) +} + pub fn shared_defs() -> Value { - 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": batch_step_schema() - }); + let mut defs = primitive_defs(); + defs["batch_step"] = batch_step_schema(); for op in OPS { defs[format!("op_{}", op.name)] = op_schema(op); } @@ -992,28 +1000,34 @@ pub fn op_schema(op: &OpDef) -> Value { }) } -pub fn execute_request_schema() -> Value { - let variants: Vec = OPS.iter().map(|op| op_ref(op)).collect(); - let op_names: Vec<&str> = OPS.iter().map(|op| op.name).collect(); +/// The common envelope is optional in static preflight; dispatch requires an id. +/// Shared with the advertised execute schema so constraints cannot drift. +fn request_envelope_schema() -> Value { json!({ - "$schema": "https://json-schema.org/draft/2020-12/schema", - "type": "object", + "type":"object", "properties": { - "op": {"type": "string", "enum": op_names, "description": "Semantic editor operation."}, - "request_id": {"type": "string", "minLength": 1, "maxLength": 128, "description": "Caller-generated idempotency key."}, - "document_id": {"type": "integer", "minimum": 0, "description": "Target document from current state."}, - "revision": {"type": "integer", "minimum": 0, "description": "Expected edit revision from current state."}, - "geometry_revision": {"type": "integer", "minimum": 0, "description": "Expected geometry revision."}, - "camera_revision": {"type": "integer", "minimum": 0, "description": "Expected camera revision."}, - "selection": {"type": "array", "items": {"$ref": "#/$defs/handle"}, "description": "Expected selected handles."}, - "steps": {"type": "array", "minItems": 1, "maxItems": 64, "description": "Sequential batch operations.", "items": {"$ref": "#/$defs/batch_step"}} + "op": {"type":"string","enum": OPS.iter().map(|op| op.name).collect::>(),"description":"Semantic editor operation."}, + "request_id": {"type":"string","minLength":1,"maxLength":128,"description":"Caller-generated idempotency key (at most 128 UTF-8 bytes)."}, + "document_id": {"type":"integer","minimum":0,"description":"Target document from current state."}, + "revision": {"type":"integer","minimum":0,"description":"Expected edit revision from current state."}, + "geometry_revision": {"type":"integer","minimum":0,"description":"Expected geometry revision."}, + "camera_revision": {"type":"integer","minimum":0,"description":"Expected camera revision."}, + "selection": {"type":"array","items":{"$ref":"#/$defs/handle"},"description":"Expected selected handles."} }, - "required": ["op", "request_id"], - "anyOf": variants, - "$defs": shared_defs() + "$defs":primitive_defs() }) } +pub fn execute_request_schema() -> Value { + let mut schema = request_envelope_schema(); + schema["$schema"] = json!("https://json-schema.org/draft/2020-12/schema"); + schema["properties"]["steps"] = json!({"type":"array","minItems":1,"maxItems":64,"description":"Sequential batch operations.","items":{"$ref":"#/$defs/batch_step"}}); + schema["required"] = json!(["op", "request_id"]); + schema["anyOf"] = Value::Array(OPS.iter().map(|op| op_ref(op)).collect()); + schema["$defs"] = shared_defs(); + schema +} + pub fn batch_step_schema() -> Value { let mut batch_variants: Vec = OPS .iter() @@ -1159,11 +1173,37 @@ fn validate_type(ty: &Ty, val: &Value) -> Result<(), String> { crate::entity_creation::validate_entity(entity, &format!("/entities/{index}"))?; } } - Ty::Raw(_) => {} + Ty::BatchSteps => { + validate_execute_request(&json!({"op":"batch","steps":val}), "batch")?; + } + Ty::Raw(fragment) => { + let schema: Value = serde_json::from_str(fragment).expect("valid inline schema"); + crate::schema_validation::validate_shape(&schema, val, "")?; + } } Ok(()) } +fn validate_parameter(ty: &Ty, val: &Value, path: &str) -> Result<(), String> { + match ty { + Ty::ArrayOf(inner) => { + let items = val + .as_array() + .ok_or_else(|| format!("{path}: expected array"))?; + for (i, item) in items.iter().enumerate() { + validate_parameter(inner, item, &format!("{path}/{i}"))?; + } + Ok(()) + } + Ty::Raw(fragment) => { + let schema: Value = serde_json::from_str(fragment).expect("valid inline schema"); + crate::schema_validation::validate_shape(&schema, val, path) + } + Ty::Entities => validate_type(ty, val), + _ => validate_type(ty, val).map_err(|error| format!("{path}: {error}")), + } +} + pub fn validate_request(request: &Value, is_batch_step: bool) -> Result { validate_request_deferred(request, is_batch_step, &[]) } @@ -1245,7 +1285,7 @@ pub fn validate_request_deferred( for p in op_def.params { if let Some(val) = obj.get(p.name) { if !val.is_null() { - validate_type(&p.ty, val) + validate_parameter(&p.ty, val, &format!("/{}", p.name)) .map_err(|e| format!("parameter '{}' for op '{}': {e}", p.name, op_name))?; } } @@ -1277,3 +1317,106 @@ pub fn validate_request_deferred( Ok(ValidationResult { warnings }) } + +const MAX_BATCH_STEPS: usize = 64; + +/// Validate bindings without dispatching or substituting untyped placeholder values. +pub fn batch_bindings(step: &Value, index: usize) -> Result, String> { + let Some(bindings) = step.get("bindings") else { + return Ok(Vec::new()); + }; + let bindings = bindings + .as_object() + .filter(|v| !v.is_empty()) + .ok_or("bindings must be a nonempty object")?; + let op = find_op(step["op"].as_str().unwrap_or_default()).ok_or("unknown bound operation")?; + for (target, source) in bindings { + if !op.params.iter().any(|p| p.name == target) { + return Err(format!("cannot bind parameter '{target}'")); + } + if step.get(target).is_some() { + return Err(format!("'{target}' has both a literal and a binding")); + } + let source = source + .as_object() + .ok_or("binding must contain step and path")?; + if source.len() != 2 + || !source + .get("step") + .and_then(Value::as_u64) + .is_some_and(|s| s < index as u64) + { + return Err( + "binding must refer to an earlier step and contain only step and path".into(), + ); + } + let path = source + .get("path") + .and_then(Value::as_str) + .ok_or("binding path must be a JSON Pointer")?; + if !path.is_empty() && !path.starts_with('/') { + return Err("binding path must be a JSON Pointer".into()); + } + let mut chars = path.chars(); + while let Some(c) = chars.next() { + if c == '~' && !matches!(chars.next(), Some('0' | '1')) { + return Err("invalid JSON Pointer escape".into()); + } + } + } + Ok(bindings.keys().map(String::as_str).collect()) +} + +pub fn validate_execute_request(request: &Value, op: &str) -> Result { + crate::schema_validation::validate_shape(&request_envelope_schema(), request, "")?; + if request["request_id"] + .as_str() + .is_some_and(|id| id.len() > 128) + { + return Err("/request_id: must not exceed 128 UTF-8 bytes".into()); + } + if op == "batch" { + let steps = request["steps"].as_array().ok_or_else(|| { + r#"Missing steps for batch. Example request: {"op":"batch","request_id":"draw-1","steps":[{"op":"run","cmd":"LINE 0,0 10,0"}]}"#.to_string() + })?; + if steps.is_empty() || steps.len() > MAX_BATCH_STEPS { + return Err(format!( + "batch steps must contain 1 to {MAX_BATCH_STEPS} operations" + )); + } + let mut warnings = Vec::new(); + for (index, step) in steps.iter().enumerate() { + let step_obj = step + .as_object() + .ok_or_else(|| format!("batch step {index} must be an object"))?; + if step_obj.contains_key("request_id") { + return Err(format!( + "batch step {index} must omit request_id; the batch assigns idempotency keys" + )); + } + let step_op = step_obj + .get("op") + .and_then(Value::as_str) + .ok_or_else(|| format!("batch step {index} is missing op"))?; + if !find_op(step_op).is_some_and(|op| op.batchable) { + return Err(format!("Unknown operation {step_op} in batch step {index}")); + } + let deferred = batch_bindings(step, index) + .map_err(|error| format!("batch step {index}: {error}"))?; + let mut literal = step.clone(); + literal.as_object_mut().unwrap().remove("bindings"); + crate::schema_validation::validate_shape( + &request_envelope_schema(), + &literal, + &format!("/steps/{index}"), + )?; + let step_val = validate_request_deferred(&literal, true, &deferred) + .map_err(|error| format!("batch step {index}: {error}"))?; + for w in step_val.warnings { + warnings.push(format!("batch step {index}: {w}")); + } + } + return Ok(ValidationResult { warnings }); + } + validate_request(request, false) +}