diff --git a/crates/engine/src/batch_preflight.rs b/crates/engine/src/batch_preflight.rs index 188f87cf52..f16aef3dad 100644 --- a/crates/engine/src/batch_preflight.rs +++ b/crates/engine/src/batch_preflight.rs @@ -19,7 +19,7 @@ use serde_json::{Value, json}; use crate::schema::ParamCheck; -use crate::{CommandSpec, EngineError, find_command}; +use crate::{EngineError, find_command}; /// The first step a batch would refuse at its turn. #[derive(Clone, Debug, PartialEq)] @@ -57,11 +57,11 @@ pub fn check(steps: &[Value], level: ParamCheck, host: &[&str]) -> Option Option Option { steps.iter().enumerate().find_map(|(index, step)| { let id = step.get("command").or_else(|| step.get("id")).and_then(Value::as_str)?; - contract_refusal(index, id, find_command(id)?, step) + contract_refusal(index, id, step) }) } -/// Step `index`'s refusal when its parameters break `spec`'s typed contract. -fn contract_refusal(index: usize, id: &str, spec: &CommandSpec, step: &Value) -> Option { - let first = spec.contract?.check(step.get("params").unwrap_or(&Value::Null)).into_iter().next()?; +/// Step `index`'s refusal when its parameters break the typed contract of its command `id`. +fn contract_refusal(index: usize, id: &str, step: &Value) -> Option { + let first = crate::schema::issues(id, step.get("params").unwrap_or(&Value::Null)).into_iter().next()?; Some(Refusal { index, command: Some(id.to_string()), error: crate::params::bad(id, first.message).to_string() }) } diff --git a/crates/engine/src/schema/check.rs b/crates/engine/src/schema/check.rs index 373f323a8f..4890a25bda 100644 --- a/crates/engine/src/schema/check.rs +++ b/crates/engine/src/schema/check.rs @@ -285,6 +285,14 @@ pub(crate) fn parse_issue(e: &serde_path_to_error::Error) -> Issue { path, message: truncate(&message, MAX_MESSAGE_LEN), suggestion: None } } +/// What is wrong with `params` for the command `id` by its typed contract: the issues a call gets +/// at [`ParamCheck::Warn`] (the first is the refusal at [`ParamCheck::Strict`]). None for a +/// command without a contract, or an id the registry lacks (a runner in another process may be +/// another version). +pub fn issues(id: &str, params: &Value) -> Vec { + crate::cmd::find_command(id).and_then(|c| c.contract).map(|c| c.check(params)).unwrap_or_default() +} + /// Run `handler` as the command `id`, checking the call against the contract `id` publishes /// (its [`CommandSpec::contract`](crate::cmd::CommandSpec::contract)), so the contract enforced is /// always the one described. With the level off and no audit recorder, this is exactly diff --git a/crates/engine/src/schema/mod.rs b/crates/engine/src/schema/mod.rs index cbbe5d87c9..7efaeae8ec 100644 --- a/crates/engine/src/schema/mod.rs +++ b/crates/engine/src/schema/mod.rs @@ -68,7 +68,7 @@ pub mod sketch_create; pub mod sketch_equation; pub mod view_navigation; -pub use check::{Contract, Issue, ParamCheck, TypedContract, input_schema_for, output_schema_for, run_checked, run_checked_as}; +pub use check::{Contract, Issue, ParamCheck, TypedContract, input_schema_for, issues, output_schema_for, run_checked, run_checked_as}; pub use types::*; /// A command's `run` that checks the call against the contract command `$id` publishes diff --git a/crates/engine/src/schema/tests.rs b/crates/engine/src/schema/tests.rs index bb698045f9..20492bf729 100644 --- a/crates/engine/src/schema/tests.rs +++ b/crates/engine/src/schema/tests.rs @@ -361,6 +361,16 @@ fn the_level_survives_a_new_design() { assert_eq!(s.scratch().param_check, ParamCheck::Strict); } +#[test] +fn issues_are_read_from_the_commands_own_contract() { + let issues = super::issues("solid.box", &json!({"length": 1, "width": 1, "height": 1, "operaton": "cut"})); + assert_eq!(messages(&issues), ["unknown key `operaton`; did you mean `operation`?"]); + assert_eq!(issues, box_contract().check(&json!({"length": 1, "width": 1, "height": 1, "operaton": "cut"}))); + // No contract, or no such command: nothing to say. + assert!(super::issues("solid.fillet", &json!({"nonsense": 1})).is_empty()); + assert!(super::issues("no.such", &json!({"nonsense": 1})).is_empty()); +} + #[test] fn a_level_is_read_back_from_its_name() { for level in ParamCheck::ALL { diff --git a/crates/mcp/src/backend.rs b/crates/mcp/src/backend.rs index 937267ba96..2ad8d52eca 100644 --- a/crates/mcp/src/backend.rs +++ b/crates/mcp/src/backend.rs @@ -484,11 +484,10 @@ impl Remote { } } -/// The issues `params` raise against the typed contract of `id` in this server's registry; none -/// when it has no contract for `id` (the app may have commands this server lacks). +/// What is wrong with `params` for `id` by this server's contracts, as messages; none when it has +/// no contract for `id` (the app may have commands this server lacks). fn contract_issues(id: &str, params: &Value) -> Vec { - let contract = solidcraft_engine::find_command(id).and_then(|c| c.contract); - contract.map(|c| c.check(params).into_iter().map(|i| i.message).collect()).unwrap_or_default() + solidcraft_engine::schema::issues(id, params).into_iter().map(|i| i.message).collect() } /// Give `reply` `issues` as its `param_warnings`, as a headless session at the warn level does; a diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 1232654496..d4e44acb8d 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -516,9 +516,7 @@ fn dispatch_checked(app: &mut SolveApp, ctx: &egui::Context, method: &str, p: &V /// app's own level, since the session it leaves and the one it brings in each keep theirs. fn switch_checked(app: &mut SolveApp, ctx: &egui::Context, method: &str, p: &Value, level: ParamCheck) -> Outcome { let id = command(p).unwrap_or_default(); - let contract = solidcraft_engine::find_command(id).and_then(|c| c.contract); - let issues: Vec = - contract.map(|c| c.check(p.get("params").unwrap_or(&Value::Null)).into_iter().map(|i| i.message).collect()).unwrap_or_default(); + let issues: Vec = solidcraft_engine::schema::issues(id, p.get("params").unwrap_or(&Value::Null)).into_iter().map(|i| i.message).collect(); if level == ParamCheck::Strict && let Some(first) = issues.first() {