Fix #263: explain property-group references in property commands

This commit is contained in:
DWAA1660 2026-10-08 08:49:42 -06:00
parent 6943872cf6
commit 69310216fb
5 changed files with 55 additions and 4 deletions

View File

@ -35,6 +35,24 @@ fn call_json(s: &mut McpServer, name: &str, args: Value) -> Value {
serde_json::from_str(c[0]["text"].as_str().unwrap()).unwrap()
}
#[test]
fn property_group_error_points_to_layer_inspection() {
let mut s = server();
call_json(&mut s, "execute_command", json!({"command": "comp.new", "params": {"name": "Main"}}));
let l = call_json(&mut s, "execute_command", json!({"command": "layer.newText", "params": {"text": "Hi"}}))["layer"].clone();
call_json(&mut s, "execute_command", json!({"command": "comp.renderer", "params": {"renderer": "advanced3d"}}));
for tool in ["get_property", "set_property", "add_keyframe"] {
let (content, err) = call(&mut s, tool, json!({"layer": l, "path": "geometryOptions", "value": 0, "time": 0}));
assert!(err, "{tool}: {content:?}");
let text = content[0]["text"].as_str().unwrap();
assert!(text.contains("is a property group"), "{tool}: {text}");
assert!(text.contains("get_layer"), "{tool}: {text}");
}
let tree = call_json(&mut s, "get_layer", json!({"layer": l, "flat": true}));
assert!(tree["properties"].as_array().unwrap().iter().any(|p| p["path"] == "geometryOptions/extrusionDepth"));
assert!(call_json(&mut s, "get_property", json!({"layer": l, "path": "geometryOptions/extrusionDepth"}))["value"].is_number());
}
#[test]
fn initialize_and_list_tools() {
let mut s = server();

View File

@ -534,7 +534,7 @@ static TOOLS: &[ToolDef] = &[
},
ToolDef {
name: "get_layer",
description: "A layer's property tree (transform, masks, effects, text, shape contents, ...): every node has a `path` usable with get_property/set_property/add_keyframe, plus uid, match id, name, type, value at `time`, keyframes and expression. `flat: true` returns just a list of {path, type, value, keys, expression} (compact).",
description: "A layer's property tree (transform, masks, effects, text, shape contents, ...): leaf properties have a `path` usable with get_property/set_property/add_keyframe, plus uid, match id, name, type, value at `time`, keyframes and expression. Groups contain children; inspect them here, then select a leaf path. `flat: true` returns just a list of {path, type, value, keys, expression} (compact).",
bridge_only: false,
schema: || {
schema(

View File

@ -13,10 +13,23 @@ pub(crate) fn prop_ref(s: &Session, p: &Value, cmd: &str) -> Result<(ItemId, Lay
let (cid, lid) = layer_p(s, p, cmd)?;
let layer = s.project.comp(cid).and_then(|c| c.layer(lid)).ok_or(EngineError::NoComp)?;
if let Some(u) = p.get("prop").and_then(Value::as_u64) {
return layer.props.find(u).map(|pr| (cid, lid, pr.uid)).ok_or_else(|| bad(cmd, format!("no property @{u}")));
return layer.props.find(u).map(|pr| (cid, lid, pr.uid)).ok_or_else(|| {
if layer.props.find_group(u).is_some() {
bad(cmd, format!("@{u} is a property group; use layer.tree (MCP: get_layer) to inspect its children, then select a leaf property"))
} else {
bad(cmd, format!("no property @{u}"))
}
});
}
let path = str_p(p, "path").ok_or_else(|| bad(cmd, "missing `path` (e.g. transform/position) or `prop` uid"))?;
let pr = layer.props.prop(path).ok_or_else(|| bad(cmd, format!("no property `{path}`")))?;
let pr = layer.props.prop(path).ok_or_else(|| {
let group = layer.props.group(path).or_else(|| path.trim().strip_prefix('@')?.parse().ok().and_then(|uid| layer.props.find_group(uid)));
if group.is_some() {
bad(cmd, format!("`{path}` is a property group; use layer.tree (MCP: get_layer) to inspect its children, then select a leaf property"))
} else {
bad(cmd, format!("no property `{path}`"))
}
})?;
Ok((cid, lid, pr.uid))
}

View File

@ -402,6 +402,26 @@ fn set_text_paragraph_fill_and_leading() {
assert_eq!((d.h_scale, d.v_scale, d.baseline_shift, d.small_caps, d.stroke_over_fill), (120.0, 80.0, 4.0, true, true));
}
#[test]
fn property_groups_report_actionable_errors() {
let mut s = Session::default();
s.execute("comp.new", json!({"name": "T"})).unwrap();
let l = s.execute("layer.newText", json!({"text": "Hi"})).unwrap()["layer"].as_u64().unwrap();
let uid = s.active_comp().unwrap().layer(crate::project::LayerId(l)).unwrap().props.group("text/pathOptions").unwrap().uid;
for selector in [json!({"path": "text"}), json!({"path": "text/pathOptions"}), json!({"path": format!("@{uid}")}), json!({"prop": uid})] {
let mut params = selector;
params["layer"] = json!(l);
for cmd in ["prop.get", "prop.set"] {
let err = s.execute(cmd, params.clone()).unwrap_err().to_string();
assert!(err.contains("is a property group"), "{cmd}: {err}");
assert!(err.contains("get_layer"), "{cmd}: {err}");
}
}
assert!(s.execute("prop.get", json!({"layer": l, "path": "text/sourceText"})).is_ok());
let err = s.execute("prop.get", json!({"layer": l, "path": "missing"})).unwrap_err().to_string();
assert!(err.contains("no property `missing`"), "{err}");
}
#[test]
fn prop_get_and_render_rgba8() {
let mut s = Session::default();

View File

@ -57,7 +57,7 @@ The server speaks JSON-RPC 2.0 over stdio, one message per line, and supports MC
| `run_script {code, name?}` | Run JavaScript with the After Effects-style scripting object model (`app.project`, `comp.layers.addText(…)`, `layer.property("ADBE Transform Group").property("ADBE Position").setValueAtTime(…)`…). Returns `{ok, result, output, error: {message, line, column}}`; edits are undoable. |
| `get_project` / `get_comp {comp?}` | Project items, comp settings and layers. |
| `get_layer {layer, comp?, time?, depth?, flat?}` | A layer's property tree (`depth` limits how many group levels expand). Every node has a `path`. |
| `get_property {layer, path, comp?, time?}` | Value at a time, keyframes and expression. |
| `get_property {layer, path, comp?, time?}` | A leaf property's value at a time, keyframes and expression. Group paths (for example `geometryOptions`) contain children: inspect them with `get_layer`, then query a leaf such as `geometryOptions/extrusionDepth`. |
| `set_property {layer, path, value?, time?, expression?, comp?}` | Sets a static value. With `time` it sets a keyframe; with `expression` it sets an expression. |
| `add_keyframe {layer, path, time+value \| keys:[...], interpolation?, comp?}` | Adds keys, then optionally applies linear/bezier/hold/easyEase. |
| `list_effects {filter?}` | Effect ids, names, categories, GPU / 32-bpc support and parameters. |