From 842148916799eb0663d528f8a3a11dd346ae45d7 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Wed, 7 Oct 2026 14:42:10 +0700 Subject: [PATCH] Expressions: prop.get reports a syntax error that disabled the expression (#163) prop.setExpression keeps an expression with a syntax error but disables it, and says why only in its own reply. MCP set_property answers with prop.get instead, and prop.get reported `evaluated` / `expressionError` only for enabled expressions, so set_property, get_property and prop.get all showed the broken expression as if it were installed. prop.get now reports a disabled expression too: `evaluated` is the static value that renders, and `expressionError` is its syntax error. Test: expression_syntax_errors_reach_the_reply (set_property and get_property carry the SyntaxError; runtime errors and valid expressions report as before). Co-Authored-By: Claude Opus 5.5 --- crates/automation/src/tests.rs | 22 ++++++++++++++++++++++ crates/engine/src/commands/prop.rs | 10 ++++++++-- docs/agents.md | 3 ++- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/crates/automation/src/tests.rs b/crates/automation/src/tests.rs index dd236dad..8a9ba805 100644 --- a/crates/automation/src/tests.rs +++ b/crates/automation/src/tests.rs @@ -177,6 +177,28 @@ fn headless_workflow() { let _ = std::fs::remove_dir_all(dir); } +/// An expression with a syntax error is kept but disabled; set_property and get_property report +/// its error, and `evaluated` is the static value that renders (#163). +#[test] +fn expression_syntax_errors_reach_the_reply() { + let mut s = McpServer::new(Backend::headless(effectcraft_host::session())); + call_json(&mut s, "execute_command", json!({"command": "comp.new", "params": {"name": "A", "width": 64, "height": 64}})); + call_json(&mut s, "execute_command", json!({"command": "layer.newSolid", "params": {"name": "S", "color": "#ff0000", "width": 32, "height": 8}})); + let at = |expr: &str| json!({"layer": "S", "path": "transform/rotation", "expression": expr}); + call_json(&mut s, "set_property", json!({"layer": "S", "path": "transform/rotation", "value": 30})); + let set = call_json(&mut s, "set_property", at("thisIsBroken(")); + let got = call_json(&mut s, "get_property", json!({"layer": "S", "path": "transform/rotation"})); + for r in [&set, &got] { + assert_eq!((&r["expression"], &r["evaluated"]), (&json!("thisIsBroken("), &json!(30.0)), "{r}"); + assert!(r["expressionError"].as_str().unwrap().contains("SyntaxError"), "{r}"); + } + // A runtime error and a valid expression report as before. + let r = call_json(&mut s, "set_property", at("nope*2")); + assert!(r["expressionError"].as_str().unwrap().contains("nope is not defined"), "{r}"); + let r = call_json(&mut s, "set_property", at("60")); + assert_eq!((&r["evaluated"], r.get("expressionError")), (&json!(60.0), None), "{r}"); +} + #[test] fn stdio_loop() { let mut s = server(); diff --git a/crates/engine/src/commands/prop.rs b/crates/engine/src/commands/prop.rs index bff27e77..409210e8 100644 --- a/crates/engine/src/commands/prop.rs +++ b/crates/engine/src/commands/prop.rs @@ -118,8 +118,14 @@ fn get(s: &mut Session, p: &Value) -> Result { "keys": keys, "expression": pr.expr.as_ref().map(|e| e.text.clone()), }); // With an expression, `value` is the keyframed (pre-expression) value; also report what the - // expression makes of it (what renders), or why it fails. - if pr.has_expression() + // expression makes of it (what renders), or why it fails. A disabled one renders `value`; one + // disabled by its syntax error (prop.setExpression) reports that error. + if let Some(e) = pr.expr.as_ref().filter(|e| !e.enabled && !e.text.trim().is_empty()) { + out["evaluated"] = raw.to_json(); + if let Some(err) = s.expr_check.and_then(|check| check(&e.text).err()) { + out["expressionError"] = json!(err); + } + } else if pr.has_expression() && let (Some(h), Some(comp)) = (s.expr.as_deref(), s.project.comp(cid)) { let mut ctx = crate::render::EvalCtx::new(&s.project, cid, comp, t); diff --git a/docs/agents.md b/docs/agents.md index 50e7d981..376475e0 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -136,7 +136,8 @@ build real projects through these interfaces; they are worked examples of everyt * **One call, one undo step**: `batch {"steps":[{"command":"layer.newShape","params":{"kind":"ellipse","name":"Ring"}},{"command":"layer.addShapeItem","params":{"layer":"$1.layer","kind":"trim"}},{"command":"prop.addKey","params":{"layer":"$1.layer","path":"transform/opacity","time":0,"value":0}}]}` (a string param that is exactly `"$N"` or `"$N.key"` is replaced; `$2.path` is the trim's path). * **Expressions**: `set_property`/`get_property` replies carry `evaluated` (what renders) next to - `value` (the keyframed value) and `expressionError` when the expression fails. + `value` (the keyframed value) and `expressionError` when the expression fails. An expression with + a syntax error is kept but disabled, so it renders `value` and reports the syntax error. * **Keyframes like a person**: `keys.select {keys, toggle: true}` (Shift+click), `keys.selectEqual` / `selectPrevious` / `selectFollowing`, `keys.move {delta, merge}` (steps sharing a merge key are one drag: each applies to the keys as they were before it, so a key passed over survives) and