Engine: one contract check for a call, used by the preflight, the bridge and the app (#9339)

`schema::issues(id, params)` looks up the command's typed contract and
lists what is wrong with the parameters (none without a contract or for
an unknown id). The batch preflight, the MCP bridge's pre-send check and
the app's check of a design-switching command each did that lookup
themselves; they now call it. No behaviour change.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Rick Morgans
2026-10-30 10:21:06 +00:00
committed by GitHub
parent f9146cdd58
commit 66bd64cc5e
6 changed files with 31 additions and 16 deletions
+8 -8
View File
@@ -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<Refusa
if host.contains(&id) {
continue;
}
let Some(spec) = find_command(id) else {
if find_command(id).is_none() {
return Some(Refusal { index, command: Some(id.to_string()), error: EngineError::UnknownCommand(id.to_string()).to_string() });
};
}
if level == ParamCheck::Strict
&& let Some(refusal) = contract_refusal(index, id, spec, step)
&& let Some(refusal) = contract_refusal(index, id, step)
{
return Some(refusal);
}
@@ -76,13 +76,13 @@ pub fn check(steps: &[Value], level: ParamCheck, host: &[&str]) -> Option<Refusa
pub fn contracts(steps: &[Value]) -> Option<Refusal> {
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<Refusal> {
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<Refusal> {
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() })
}
+8
View File
@@ -285,6 +285,14 @@ pub(crate) fn parse_issue(e: &serde_path_to_error::Error<serde_json::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<Issue> {
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
+1 -1
View File
@@ -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
+10
View File
@@ -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 {
+3 -4
View File
@@ -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<String> {
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
+1 -3
View File
@@ -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<String> =
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<String> = 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()
{