#61: honor MCP typing, story and page contracts (#118)

crates/engine/src/cmd/text.rs

@@ -114,8 +114,8 @@ pub fn specs() -> Vec<CommandSpec> {
         cmd!(query "story.get", "Get Story", [], None, "{story? | frame?} → text, frames, paragraphs, vertical, overset", has_doc, |s, p| {
             let st = s.doc()?;
             let sid = story_of(s, p).ok_or_else(|| bad("story.get", "no story"))?;
+            let mut v = st.doc.story_summary(sid).ok_or(designcraft_doc::DocError::NoStory(sid))?;
             let cs = s.cache.get(&st.doc, sid, None);
-            let mut v = st.doc.story_summary(sid).unwrap_or_default();
             v["overset"] = json!(cs.overset_at);
             v["lines"] = json!(cs.line_count());
             Ok(v)
@@ -1638,3 +1638,19 @@ mod vertical_caret_tests {
         assert_eq!(mv(&mut s, "up", false, false), 3 * c);
     }
 }
+
+#[cfg(test)]
+mod story_query_contract_tests {
+    use serde_json::json;
+    #[test]
+    fn missing_story_errors_but_valid_empty_and_selected_stories_succeed() {
+        let mut s = crate::Session::new();
+        s.execute("file.new", &json!({})).unwrap();
+        assert!(s.execute("story.get", &json!({"story":999})).unwrap_err().to_string().contains("no such story"));
+        let f = s.execute("frame.create", &json!({"rect":[36,36,300,200],"content":"text"})).unwrap();
+        assert_eq!(s.execute("story.get", &json!({"story":f["story"]})).unwrap()["text"], "");
+        assert_eq!(s.execute("story.get", &json!({"frame":f["id"]})).unwrap()["text"], "");
+        s.execute("text.select", &json!({"story":f["story"],"anchor":0})).unwrap();
+        assert_eq!(s.execute("story.get", &json!({})).unwrap()["text"], "");
+    }
+}

crates/mcp/src/headless.rs

@@ -180,7 +180,7 @@ impl Headless {
 
     fn text(&mut self, p: &Value) -> Result<Value, String> {
         let t = s(p, "text").ok_or("missing `text`")?;
-        if !self.session.wants_text() {
+        if self.session.active().is_none_or(|st| st.selection.text.is_none_or(|t| st.doc.text_story(t.story, t.cell).is_none())) {
             return Err("no text insertion point: click into a text frame with the type tool first (pointer with tool \"type\"), \
                 or select text with execute text.select, or use set_story_text"
                 .into());
@@ -191,7 +191,7 @@ impl Headless {
 
     fn render(&mut self, p: &Value) -> Result<Value, String> {
         let st = self.session.active().ok_or("no document open")?;
-        let page = p.get("page").and_then(Value::as_u64).unwrap_or(0) as usize;
+        let page = page_index(p, st.doc.page_count())?;
         let scale = p.get("scale").and_then(Value::as_f64).unwrap_or(1.0).clamp(0.05, 16.0);
         let bleed = p.get("bleed").and_then(Value::as_bool).unwrap_or(false);
         let img = self
@@ -213,7 +213,7 @@ impl Headless {
     fn export(&mut self, p: &Value) -> Result<Value, String> {
         let path = s(p, "path").ok_or("missing `path`")?;
         let st = self.session.active().ok_or("no document open")?;
-        let page = p.get("page").and_then(Value::as_u64).unwrap_or(0) as usize;
+        let page = page_index(p, st.doc.page_count())?;
         let scale = p.get("scale").and_then(Value::as_f64).unwrap_or(2.0).clamp(0.05, 16.0);
         let img = self
             .renderer
@@ -263,3 +263,15 @@ impl Backend for Headless {
         "headless (in-process engine, no window)".into()
     }
 }
+
+/// A supplied page must be a representable unsigned index; only omission selects page zero.
+fn page_index(p: &Value, count: usize) -> Result<usize, String> {
+    let page = match p.get("page") {
+        None => 0,
+        Some(v) => v.as_u64().and_then(|n| usize::try_from(n).ok()).ok_or("`page` must be a nonnegative integer representable as a page index")?,
+    };
+    if page >= count {
+        return Err(format!("no page {page} (the document has {count} pages, indices are 0-based)"));
+    }
+    Ok(page)
+}

crates/mcp/src/tests.rs

@@ -416,3 +416,87 @@ fn batch_steps_use_earlier_results() {
     );
     assert_eq!(v["completed"], 2);
 }
+
+#[test]
+fn explicit_typing_honors_caret_range_and_undo_without_changing_shortcuts() {
+    let mut s = server();
+    assert_eq!(call(&mut s, "type_text", json!({"text":"X"}))["isError"], true);
+    let f = ok(&mut s, "execute", json!({"command":"frame.create","params":{"rect":[36,36,300,200],"content":"text","text":"Hello"}}));
+    let story = f["story"].as_u64().unwrap();
+    for (anchor, focus, expected) in [(5, 5, "HelloX"), (0, 5, "X")] {
+        ok(&mut s, "set_story_text", json!({"story":story,"text":"Hello"}));
+        ok(&mut s, "execute", json!({"command":"text.select","params":{"story":story,"anchor":anchor,"focus":focus}}));
+        ok(&mut s, "type_text", json!({"text":"X"}));
+        assert_eq!(ok(&mut s, "get_story", json!({"story":story}))["text"], expected);
+        ok(&mut s, "execute", json!({"command":"edit.undo"}));
+        assert_eq!(ok(&mut s, "get_story", json!({"story":story}))["text"], "Hello");
+    }
+    ok(&mut s, "execute", json!({"command":"text.placeCaret","params":{"frame":f["id"],"point":[100,50]}}));
+    ok(&mut s, "type_text", json!({"text":"X"}));
+    assert_eq!(ok(&mut s, "get_story", json!({"story":story}))["text"], "HelloX");
+    assert_eq!(ok(&mut s, "key", json!({"key":"T"}))["handledBy"], "tool.select");
+    assert_eq!(ok(&mut s, "get_story", json!({"story":story}))["text"], "HelloX");
+}
+
+#[test]
+fn story_queries_distinguish_missing_from_empty() {
+    let mut s = server();
+    assert_eq!(call(&mut s, "get_story", json!({"story":999}))["isError"], true);
+    let f = ok(&mut s, "execute", json!({"command":"frame.create","params":{"rect":[36,36,300,200],"content":"text"}}));
+    assert_eq!(ok(&mut s, "get_story", json!({"story":f["story"]}))["text"], "");
+    assert_eq!(ok(&mut s, "get_story", json!({"frame":f["id"]}))["text"], "");
+    ok(&mut s, "execute", json!({"command":"text.select","params":{"story":f["story"],"anchor":0}}));
+    assert_eq!(ok(&mut s, "get_story", json!({}))["text"], "");
+}
+
+#[test]
+fn page_contract_rejects_supplied_invalid_values_without_writes() {
+    let mut s = server();
+    ok(&mut s, "new_document", json!({"sample":true}));
+    let path = tmp("page-contract.png");
+    for tool in ["export_png", "render_page"] {
+        for page in
+            [json!(-1), json!(-2), json!(1.5), Value::Null, json!("0"), json!(true), json!(u64::MAX), json!(18446744073709551616_f64), json!(99)]
+        {
+            std::fs::write(&path, b"KEEP").unwrap();
+            let args = json!({"page":page,"path":path,"scale":0.1});
+            assert_eq!(call(&mut s, tool, args)["isError"], true, "{tool}: {page}");
+            assert_eq!(std::fs::read(&path).unwrap(), b"KEEP");
+        }
+    }
+    let first = image_of(&call(&mut s, "render_page", json!({"page":0,"scale":0.1})));
+    let next = image_of(&call(&mut s, "render_page", json!({"page":1,"scale":0.1})));
+    assert_ne!(first, next);
+    assert_eq!(first, image_of(&call(&mut s, "render_page", json!({"scale":0.1}))));
+    ok(&mut s, "export_png", json!({"page":1,"path":path,"scale":0.1}));
+    assert_eq!(std::fs::read(&path).unwrap(), next);
+    ok(&mut s, "export_png", json!({"path":path,"scale":0.1}));
+    assert_eq!(std::fs::read(&path).unwrap(), first);
+    for tool in ["render_page", "export_png"] {
+        let definition = tool_definitions().into_iter().find(|v| v["name"] == tool).unwrap();
+        assert_eq!(definition["inputSchema"]["properties"]["page"]["minimum"], 0);
+    }
+    let mut b = Headless::with_document();
+    for method in ["ui.render", "app.export"] {
+        for page in [json!(-1), Value::Null, json!("0"), json!(99)] {
+            std::fs::write(&path, b"KEEP").unwrap();
+            assert!(b.call(method, json!({"page":page,"path":path,"scale":0.1})).is_err());
+            assert_eq!(std::fs::read(&path).unwrap(), b"KEEP");
+        }
+        std::fs::remove_file(&path).unwrap();
+        assert!(b.call(method, json!({"page":-1,"path":path,"scale":0.1})).is_err());
+        assert!(!path.exists());
+    }
+}
+
+#[test]
+fn explicit_typing_rejects_stale_selection_and_closed_document() {
+    let mut b = Headless::with_document();
+    let f = b.session.execute("frame.create", &json!({"rect":[36,36,300,200],"content":"text","text":"Hello"})).unwrap();
+    b.session.execute("text.select", &json!({"story":f["story"],"anchor":5})).unwrap();
+    b.session.active_mut().unwrap().selection.text.as_mut().unwrap().story.0 = 999;
+    assert!(b.call("ui.text", json!({"text":"X"})).is_err());
+    assert_eq!(b.session.execute("story.get", &json!({"story":f["story"]})).unwrap()["text"], "Hello");
+    b.session.execute("file.close", &json!({})).unwrap();
+    assert!(b.call("ui.text", json!({"text":"X"})).is_err());
+}

crates/mcp/src/tools.rs

@@ -223,7 +223,7 @@ pub fn tool_definitions() -> Vec<Value> {
              also write it to `path`.",
             obj(
                 json!({
-                    "page": int("Page index, 0-based (default 0; in the desktop app default = the current page)"),
+                    "page": {"type": "integer", "minimum": 0, "description": "Page index, 0-based (default 0; in the desktop app default = the current page)"},
                     "scale": num("Pixels per point (default 1 = 72 ppi)"),
                     "bleed": boolean("Include the bleed area"),
                     "path": string("Also write the PNG here"),
@@ -237,7 +237,7 @@ pub fn tool_definitions() -> Vec<Value> {
             "Export PNG",
             "Export one page as a PNG (or JPEG if the path ends in .jpg) file. Returns the path and pixel size.",
             obj(
-                json!({"path": string("Destination file"), "page": int("Page index, 0-based (default 0)"), "scale": num("Pixels per point (default 2)")}),
+                json!({"path": string("Destination file"), "page": {"type": "integer", "minimum": 0, "description": "Page index, 0-based (default 0; in the desktop app default = the current page)"}, "scale": num("Pixels per point (default 2)")}),
                 &["path"],
             ),
             false,
@@ -303,7 +303,7 @@ pub fn tool_definitions() -> Vec<Value> {
         tool(
             "type_text",
             "Type text",
-            "Type text at the text insertion point (as the keyboard would with the type tool), or into the focused dialog field in \
+            "Type text at the text insertion point, independently of the active tool, or into the focused dialog field in \
              the desktop app. Put a caret first with pointer (tool \"type\") or execute text.placeCaret / text.select.",
             obj(json!({"text": string("Text to type (\\n = new paragraph)")}), &["text"]),
             false,
@@ -409,6 +409,15 @@ fn pick(a: &Args, keys: &[&str]) -> Value {
     Value::Object(keys.iter().filter_map(|k| a.get(*k).filter(|v| !v.is_null()).map(|v| (k.to_string(), v.clone()))).collect())
 }
 
+/// Page omission selects a default, but an explicitly supplied null must reach validation.
+fn page_params(a: &Args, keys: &[&str]) -> Value {
+    let mut p = pick(a, keys);
+    if let Some(page) = a.get("page") {
+        p["page"] = page.clone();
+    }
+    p
+}
+
 fn command_params(v: Option<&Value>) -> Result<Value, String> {
     match v {
         None | Some(Value::Null) => Ok(json!({})),
@@ -463,7 +472,7 @@ fn story_id(b: &mut dyn Backend, a: &Args) -> Result<Value, String> {
 }
 
 fn render_page(b: &mut dyn Backend, a: &Args) -> Result<ToolResult, String> {
-    let r = b.call("ui.render", pick(a, &["page", "scale", "bleed"]))?;
+    let r = b.call("ui.render", page_params(a, &["page", "scale", "bleed"]))?;
     let b64 = r.get("pngBase64").and_then(Value::as_str).ok_or("renderer returned no image")?.to_string();
     let path = a.get("path").and_then(Value::as_str);
     if let Some(path) = path {
@@ -550,7 +559,7 @@ fn dispatch(b: &mut dyn Backend, name: &str, a: &Args) -> Result<ToolResult, Str
         "render_page" => render_page(b, a),
         "export_png" => {
             req_str(a, "path")?;
-            j(b.call("app.export", pick(a, &["path", "page", "scale"]))?)
+            j(b.call("app.export", page_params(a, &["path", "page", "scale"]))?)
         }
         "screenshot" => screenshot(b, a),
         "select_tool" => j(b.call("ui.tool.select", json!({"tool": req_str(a, "tool")?}))?),

crates/ui-egui/src/control.rs

@@ -8,7 +8,8 @@
 //! - `ui.tool.select {tool}`, `ui.tool.list`
 //! - `ui.pointer {events:[{kind: down|drag|up|move|doubleclick, x, y, space?: "doc"|"screen"}], mods?}`:
 //!   drive the active tool through the same path as the mouse
-//! - `ui.key {key, shift?, alt?, cmd?}` / `ui.text {text}`: synthetic keyboard input
+//! - `ui.key {key, shift?, alt?, cmd?}`: synthetic keyboard input
+//! - `ui.text {text}`: insert at the explicit caret, or type into a focused UI field
 //! - `ui.move {x, y}` / `ui.click {x, y, button?, count?, shift?…}` / `ui.drag {x, y, toX, toY, steps?}`:
 //!   real egui pointer input in screen points (reaches every widget: panels, flyouts, dialogs)
 //! - `ui.set {brightness?, panel?, dockTab?, rulers?, outline?, …}`
@@ -216,9 +217,19 @@ pub fn handle(app: &mut DesignApp, ctx: &egui::Context, req: &ControlRequest) ->
             ok(Value::Null)
         }
         "ui.text" => {
-            app.synthetic.push(egui::Event::Text(s("text").unwrap_or("").to_string()));
-            ctx.request_repaint();
-            ok(Value::Null)
+            let Some(text) = s("text") else { return err("missing `text`") };
+            // Focused widgets retain the synthetic keyboard route. Document automation uses
+            // the explicit caret independently of the active tool, unlike ordinary typing.
+            if ctx.text_edit_focused() || app.ui.dialog.is_some() || app.ui.palette.is_some() {
+                app.synthetic.push(egui::Event::Text(text.to_string()));
+                ctx.request_repaint();
+                ok(Value::Null)
+            } else {
+                if app.session.active().is_none_or(|st| st.selection.text.is_none_or(|t| st.doc.text_story(t.story, t.cell).is_none())) {
+                    return err("no valid text insertion point: use text.select or text.placeCaret first");
+                }
+                wrap(app.run("text.insert", json!({"text": text})))
+            }
         }
         "ui.set" => {
             let mut r = Ok(Value::Null);
@@ -313,7 +324,10 @@ pub fn handle(app: &mut DesignApp, ctx: &egui::Context, req: &ControlRequest) ->
         }
         "ui.render" => {
             let Some(st) = app.session.active() else { return err("no document") };
-            let page = p.get("page").and_then(Value::as_u64).map(|v| v as usize).or_else(|| crate::canvas::current_page(app)).unwrap_or(0);
+            let page = match page_index(p, crate::canvas::current_page(app).unwrap_or(0), st.doc.page_count()) {
+                Ok(page) => page,
+                Err(e) => return err(e),
+            };
             let scale = p.get("scale").and_then(Value::as_f64).unwrap_or(1.0);
             let Some(img) = app.canvas.renderer.render_page(
                 &st.doc,
@@ -336,7 +350,7 @@ pub fn handle(app: &mut DesignApp, ctx: &egui::Context, req: &ControlRequest) ->
         }
         "app.open" => wrap(app.run("file.open", json!({"path": s("path")}))),
         "app.save" => wrap(app.run("file.save", json!({"path": s("path")}))),
-        "app.export" => wrap(app.run("app.exportPng", json!({"path": s("path"), "page": p.get("page"), "scale": p.get("scale")}))),
+        "app.export" => wrap(app.run("app.exportPng", p.clone())),
         "app.quit" => {
             ctx.send_viewport_cmd(egui::ViewportCommand::Close);
             ok(Value::Null)
@@ -361,3 +375,102 @@ pub fn save_screenshot(app: &mut DesignApp, image: &egui::ColorImage, path: Opti
         None => json!({"ok": false, "error": "no writer configured"}),
     }
 }
+
+/// Parse explicit page indices without treating invalid values as omission.
+pub(crate) fn page_index(p: &Value, default: usize, count: usize) -> Result<usize, String> {
+    let page = match p.get("page") {
+        None => default,
+        Some(v) => v.as_u64().and_then(|n| usize::try_from(n).ok()).ok_or("`page` must be a nonnegative integer representable as a page index")?,
+    };
+    if page >= count {
+        return Err(format!("no page {page} (the document has {count} pages, indices are 0-based)"));
+    }
+    Ok(page)
+}
+
+#[cfg(test)]
+mod automation_contract_tests {
+    use super::*;
+    fn app() -> DesignApp {
+        let mut app = DesignApp::new(designcraft_engine::Session::new(), crate::Services::default());
+        app.session.execute("file.new", &json!({"pages":2})).unwrap();
+        app
+    }
+    fn call(app: &mut DesignApp, ctx: &egui::Context, method: &str, params: Value) -> Value {
+        let (req, _) = ControlRequest::new(method, params);
+        match handle(app, ctx, &req) {
+            Outcome::Done(v) => v,
+            Outcome::Screenshot { .. } => panic!("unexpected screenshot"),
+        }
+    }
+    fn frame(app: &mut DesignApp, ctx: &egui::Context, events: Vec<egui::Event>) {
+        let mut output = ctx.run_ui(
+            egui::RawInput {
+                screen_rect: Some(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1400.0, 900.0))),
+                events,
+                ..Default::default()
+            },
+            |ui| {
+                app.logic(ui.ctx());
+                app.ui(ui);
+            },
+        );
+        output.textures_delta.clear();
+    }
+    #[test]
+    fn control_typing_uses_explicit_caret_but_retains_widget_events() {
+        let mut app = app();
+        let ctx = egui::Context::default();
+        assert_eq!(call(&mut app, &ctx, "ui.text", json!({"text":"X"}))["ok"], false);
+        let f = app.session.execute("frame.create", &json!({"rect":[36,36,300,200],"content":"text","text":"Hello"})).unwrap();
+        app.session.execute("text.select", &json!({"story":f["story"],"anchor":5})).unwrap();
+        assert_eq!(app.session.tool_id(), "selection");
+        assert_eq!(call(&mut app, &ctx, "ui.text", json!({"text":"X"}))["ok"], true);
+        assert_eq!(app.session.execute("story.get", &json!({"story":f["story"]})).unwrap()["text"], "HelloX");
+        app.session.execute("edit.undo", &json!({})).unwrap();
+        assert_eq!(app.session.execute("story.get", &json!({"story":f["story"]})).unwrap()["text"], "Hello");
+        app.select_tool("type");
+        assert!(app.session.wants_text(), "the document would receive ordinary typing without widget focus");
+        app.ui.palette = Some(String::new());
+        for _ in 0..3 {
+            frame(&mut app, &ctx, vec![]);
+        }
+        assert!(ctx.text_edit_focused());
+        assert_eq!(call(&mut app, &ctx, "ui.text", json!({"text":"field"}))["ok"], true);
+        let events = std::mem::take(&mut app.synthetic);
+        frame(&mut app, &ctx, events);
+        assert_eq!(app.ui.palette.as_deref(), Some("field"));
+        assert_eq!(app.session.execute("story.get", &json!({"story":f["story"]})).unwrap()["text"], "Hello");
+    }
+    #[test]
+    fn control_pages_validate_before_writer_and_preserve_omission() {
+        let mut app = app();
+        let ctx = egui::Context::default();
+        let writes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new()));
+        let sink = writes.clone();
+        app.services.write = Some(Box::new(move |_, bytes| {
+            sink.lock().unwrap().push(bytes.to_vec());
+            Ok(())
+        }));
+        app.session.execute("frame.create", &json!({"rect":[36,36,300,200]})).unwrap();
+        app.canvas_rect = Some(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1400.0, 900.0)));
+        crate::canvas::go_to_page(&mut app, 1);
+        assert_eq!(crate::canvas::current_page(&app), Some(1));
+        for method in ["ui.render", "app.export"] {
+            for page in
+                [json!(-1), json!(-2), json!(1.5), Value::Null, json!("0"), json!(true), json!(u64::MAX), json!(18446744073709551616_f64), json!(99)]
+            {
+                assert_eq!(call(&mut app, &ctx, method, json!({"page":page,"path":"unchanged.png","scale":0.1}))["ok"], false, "{method}: {page}");
+                assert!(writes.lock().unwrap().is_empty());
+            }
+            assert_eq!(call(&mut app, &ctx, method, json!({"path":"omitted.png","scale":0.1}))["ok"], true);
+            assert_eq!(call(&mut app, &ctx, method, json!({"page":1,"path":"explicit.png","scale":0.1}))["ok"], true);
+            let mut bytes = writes.lock().unwrap();
+            assert_eq!(bytes.len(), 2);
+            assert_eq!(bytes[0], bytes[1]);
+            bytes.clear();
+        }
+        assert!(app.run("app.exportPng", json!({"page":Value::Null,"path":"unchanged.png"})).is_err());
+        assert!(writes.lock().unwrap().is_empty());
+    }
+}

crates/ui-egui/src/menus.rs

@@ -1393,7 +1393,7 @@ fn download_document(app: &mut DesignApp) -> Result<Value, String> {
 
 fn export_png(app: &mut DesignApp, p: &Value) -> Result<Value, String> {
     let st = app.session.active().ok_or("no document")?;
-    let page = p.get("page").and_then(Value::as_u64).map(|v| v as usize).or_else(|| crate::canvas::current_page(app)).unwrap_or(0);
+    let page = crate::control::page_index(p, crate::canvas::current_page(app).unwrap_or(0), st.doc.page_count())?;
     let scale = p.get("scale").and_then(Value::as_f64).unwrap_or(2.0);
     let mut r = designcraft_render::Renderer::new();
     let img = r