fix(mcp): validate and cancel Rotate View gestures (#6322)

crates/mcp/src/tests.rs

@@ -1463,6 +1463,60 @@ fn pointer_gesture_forwards_backquote_on_each_event() {
     assert_eq!(rect["bounds"]["x"], 100.0, "{doc}");
 }
 
+/// The documented Rotate View calls reach the desktop control methods, preserving document
+/// coordinates and modifiers, and a successful null reset remains a successful MCP response.
+#[test]
+fn rotate_view_calls_forward_to_desktop_control() {
+    use std::cell::RefCell;
+    use std::rc::Rc;
+
+    struct Recording(Rc<RefCell<Vec<(String, Value)>>>);
+    impl Backend for Recording {
+        fn call(&mut self, method: &str, params: Value) -> Result<Value, String> {
+            self.0.borrow_mut().push((method.into(), params));
+            Ok(Value::Null)
+        }
+        fn has_ui(&self) -> bool {
+            true
+        }
+        fn describe(&self) -> String {
+            "desktop control contract".into()
+        }
+    }
+
+    let seen = Rc::new(RefCell::new(Vec::new()));
+    let mut s = Server::new(Box::new(Recording(seen.clone())));
+    let selected = call(&mut s, 1, "select_tool", json!({"tool": "rotateView"}));
+    let turned = call(
+        &mut s,
+        2,
+        "pointer_gesture",
+        json!({"events": [
+        {"kind": "down", "x": 300, "y": 150}, {"kind": "drag", "x": 200, "y": 250}, {"kind": "up", "x": 200, "y": 250}
+    ], "mods": {"shift": true}}),
+    );
+    let reset = call(&mut s, 3, "command_run", json!({"id": "view.rotateReset", "params": {}}));
+    for reply in [&selected, &turned, &reset] {
+        assert_eq!(reply["isError"], false, "{reply}");
+    }
+    assert_eq!(text_of(&reset), "null");
+    assert_eq!(
+        *seen.borrow(),
+        vec![
+            ("ui.tool.select".into(), json!({"tool": "rotateView"})),
+            (
+                "ui.pointer".into(),
+                json!({"events": [
+            {"kind": "down", "x": 300, "y": 150, "space": "doc"},
+            {"kind": "drag", "x": 200, "y": 250, "space": "doc"},
+            {"kind": "up", "x": 200, "y": 250, "space": "doc"}
+        ], "mods": {"shift": true}})
+            ),
+            ("engine.execute".into(), json!({"command": "view.rotateReset", "params": {}}))
+        ]
+    );
+}
+
 /// A `backquote` that is not a boolean is an error, and none of the gesture's events runs.
 #[test]
 fn pointer_gesture_rejects_a_backquote_that_is_not_a_boolean() {

crates/ui-egui/src/app.rs

@@ -123,6 +123,7 @@ impl VectorcraftApp {
             updates_rt: Default::default(),
             frame_rate: Default::default(),
             canvas_rect: None,
+            control_rotate_press: None,
             hover_doc: None,
             titlebar_from_env: false,
             custom_titlebar: false,

crates/ui-egui/src/app/commands.rs

@@ -404,6 +404,10 @@ impl VectorcraftApp {
         if let Err(e) = switched {
             self.ui.status = e.to_string();
         }
+        if before != self.session.tool_id() {
+            // A control gesture cannot resume after another tool has owned the pointer.
+            self.control_rotate_press = None;
+        }
         // A newly chosen Artboard tool starts on the active artboard (the view's).
         if before != "artboard"
             && self.session.tool_id() == "artboard"

crates/ui-egui/src/control.rs

@@ -610,9 +610,9 @@ mod tests;
 
 /// The window and coordinate mapping where a control-channel Rotate View drag began.
 #[derive(Clone, Copy)]
-struct RotateViewPress {
+pub(crate) struct RotateViewPress {
     window: (u64, u32),
-    tool_resets: u64,
+    document_resets: u64,
     xf: Xf,
     start_angle: f64,
     start_rotation: f64,
@@ -622,53 +622,102 @@ struct RotateViewPress {
 /// across requests so document-coordinate drags do not move again as the view turns underneath.
 fn rotate_view_pointer(
     app: &mut VectorcraftApp,
-    ctx: &egui::Context,
     kind: PointerKind,
     position: vectorcraft_geom::Point,
     screen: bool,
     shift: bool,
 ) -> Result<(), String> {
-    let id = egui::Id::unique("control.rotateView");
-    match kind {
-        PointerKind::Up => {
-            ctx.data_mut(|d| d.remove::<RotateViewPress>(id));
-            return Ok(());
-        }
-        PointerKind::Move | PointerKind::DoubleClick => return Ok(()),
-        _ => {}
-    }
     let window = app.session.active().zip(crate::doc_windows::current(app)).map(|(d, w)| (d.uid, w.1)).ok_or("no document")?;
-    let tool_resets = app.session.tool_resets();
-    let mut press = if kind == PointerKind::Down {
-        let rect = app.canvas_rect.ok_or("no canvas yet")?;
-        let view = app.view().ok_or("no document")?;
-        RotateViewPress { window, tool_resets, xf: Xf::new(rect, view), start_angle: 0.0, start_rotation: view.rotation }
-    } else {
-        ctx.data(|d| d.get_temp::<RotateViewPress>(id)).ok_or("Rotate View needs a down event before a drag")?
+    let document_resets = app.session.tool_resets();
+    let stored = if kind == PointerKind::Down { None } else { app.control_rotate_press };
+    if kind == PointerKind::Drag && stored.is_none() {
+        return Err("Rotate View needs a down event before a drag".into());
+    }
+    let mut press = match stored {
+        Some(press) => press,
+        None => {
+            let rect = app.canvas_rect.ok_or("no canvas yet")?;
+            let view = app.view().ok_or("no document")?;
+            RotateViewPress { window, document_resets, xf: Xf::new(rect, view), start_angle: 0.0, start_rotation: view.rotation }
+        }
     };
-    // A gesture cannot turn a different document/view or resume after another tool was chosen.
-    if (press.window, press.tool_resets) != (window, tool_resets) {
-        ctx.data_mut(|d| d.remove::<RotateViewPress>(id));
-        return Err("the Rotate View gesture's window or tool changed".into());
+    // A gesture cannot turn a different document/view or resume after its document was reset.
+    if (press.window, press.document_resets) != (window, document_resets) {
+        return Err("the Rotate View gesture's window or document changed".into());
     }
     let point = if screen { egui::pos2(position.x as f32, position.y as f32) } else { press.xf.to_screen(position) };
     let center = press.xf.rect.center();
     if !point.is_finite() || !center.is_finite() {
         return Err("Rotate View needs finite canvas coordinates".into());
     }
-    if kind == PointerKind::Down {
-        press.start_angle = (point.y - center.y).atan2(point.x - center.x) as f64;
-        ctx.data_mut(|d| d.insert_temp(id, press));
-    } else {
-        app.view_mut().ok_or("no document")?.rotation = crate::canvas::drag_rotation(center, point, press.start_angle, press.start_rotation, shift);
+    match kind {
+        PointerKind::Down => {
+            press.start_angle = (point.y - center.y).atan2(point.x - center.x) as f64;
+            app.control_rotate_press = Some(press);
+        }
+        PointerKind::Drag => {
+            app.view_mut().ok_or("no document")?.rotation =
+                crate::canvas::drag_rotation(center, point, press.start_angle, press.start_rotation, shift);
+        }
+        PointerKind::Up => {
+            app.control_rotate_press = None;
+        }
+        PointerKind::Move | PointerKind::DoubleClick => {}
+    }
+    Ok(())
+}
+
+/// A Rotate View packet accepts only the documented boolean modifier fields.
+fn validate_rotate_view_mods(value: Option<&Value>) -> Result<(), String> {
+    let Some(value) = value else { return Ok(()) };
+    let Some(fields) = value.as_object() else { return Err("Rotate View `mods` must be an object of booleans".into()) };
+    if fields.iter().any(|(key, value)| !matches!(key.as_str(), "shift" | "alt" | "cmd" | "ctrl" | "space") || !value.is_boolean()) {
+        return Err("Rotate View `mods` accepts only boolean shift, alt, cmd, ctrl and space".into());
+    }
+    Ok(())
+}
+
+/// Reject malformed coordinates and modifiers before any event can change the angle or press.
+fn validate_rotate_view_events(params: &Value, events: &[Value]) -> Result<(), String> {
+    validate_rotate_view_mods(params.get("mods"))?;
+    for event in events {
+        for axis in ["x", "y"] {
+            if event.get(axis).and_then(Value::as_f64).filter(|value| value.is_finite()).is_none() {
+                return Err(format!("Rotate View `{axis}` must be a finite number"));
+            }
+        }
+        validate_rotate_view_mods(event.get("mods"))?;
+        if event.get("space").is_some_and(|space| !matches!(space.as_str(), Some("doc" | "screen"))) {
+            return Err("Rotate View `space` must be doc or screen".into());
+        }
     }
     Ok(())
 }
 
 /// `ui.pointer`: pointer events through the tools.
 fn pointer_events(app: &mut VectorcraftApp, ctx: &egui::Context, req: &ControlRequest) -> Outcome {
+    let rotating = app.session.tool_id() == "rotateView";
+    let before = if rotating { app.view().map(|view| view.rotation) } else { None };
+    let outcome = pointer_events_inner(app, ctx, req);
+    if rotating && matches!(&outcome, Outcome::Done(reply) if reply.get("ok").and_then(Value::as_bool) == Some(false)) {
+        // A failed packet must neither leave a partial turn nor allow a later drag to resume it.
+        app.control_rotate_press = None;
+        if let (Some(before), Some(view)) = (before, app.view_mut()) {
+            view.rotation = before;
+        }
+        ctx.request_repaint();
+    }
+    outcome
+}
+
+/// Apply a pointer packet after Rotate View validation; other tools retain their existing routing.
+fn pointer_events_inner(app: &mut VectorcraftApp, ctx: &egui::Context, req: &ControlRequest) -> Outcome {
     let p = &req.params;
     let Some(events) = p.get("events").and_then(Value::as_array) else { return err("missing `events`") };
+    let rotating = app.session.tool_id() == "rotateView";
+    if rotating && let Err(error) = validate_rotate_view_events(p, events) {
+        return err(error);
+    }
     let base_mods: Mods = p.get("mods").and_then(|m| serde_json::from_value(m.clone()).ok()).unwrap_or_default();
     let view = app.view_info();
     let xf = app.canvas_rect.zip(app.view().copied()).map(|(rect, v)| Xf::new(rect, &v));
@@ -692,8 +741,8 @@ fn pointer_events(app: &mut VectorcraftApp, ctx: &egui::Context, req: &ControlRe
         let y = e.get("y").and_then(Value::as_f64).unwrap_or(0.0);
         let screen = e.get("space").and_then(Value::as_str) == Some("screen");
         let mods = e.get("mods").and_then(|m| serde_json::from_value(m.clone()).ok()).unwrap_or(base_mods);
-        if app.session.tool_id() == "rotateView" {
-            if let Err(e) = rotate_view_pointer(app, ctx, kind, vectorcraft_geom::Point::new(x, y), screen, mods.shift) {
+        if rotating {
+            if let Err(e) = rotate_view_pointer(app, kind, vectorcraft_geom::Point::new(x, y), screen, mods.shift) {
                 return err(e);
             }
             continue;

crates/ui-egui/src/lib.rs

@@ -186,6 +186,8 @@ pub struct VectorcraftApp {
     frame_rate: frame_rate::FrameRate,
     /// Canvas rect of the last frame (screen points), for control-channel coordinate mapping.
     pub canvas_rect: Option<egui::Rect>,
+    /// An unfinished control-channel Rotate View gesture, kept only for this app instance.
+    pub(crate) control_rotate_press: Option<control::RotateViewPress>,
     /// Hover position in document coordinates.
     pub hover_doc: Option<vectorcraft_geom::Point>,
     /// System clipboard: SVG to publish next frame, the last SVG we published (so pasting it back

crates/ui-egui/src/tests/rotateview.rs

@@ -30,6 +30,13 @@ fn with_document() -> VectorcraftApp {
     app
 }
 
+/// Send a control request and keep its success/error value for protocol assertions.
+fn control_response(app: &mut VectorcraftApp, ctx: &egui::Context, method: &str, params: Value) -> Value {
+    let (request, _) = crate::control::ControlRequest::new(method, params);
+    let crate::control::Outcome::Done(reply) = crate::control::handle(app, ctx, &request) else { panic!("not done") };
+    reply
+}
+
 /// Returns the labels of the View › Rotate View items that are checked.
 fn checked(app: &VectorcraftApp) -> Vec<&'static str> {
     ANGLES.iter().filter(|(_, a)| menus::checked(app, "view.rotate", &json!({ "angle": a })) == Some(true)).map(|(l, _)| *l).collect()
@@ -69,6 +76,8 @@ fn control_rotate_view_gesture_turns_the_canvas_and_reset_restores_it() {
     let send = super::synthetic::control;
     send(&mut app, &ctx, "ui.tool.select", json!({"tool": "rotateView"}));
     send(&mut app, &ctx, "ui.pointer", json!({"events": [{"kind": "down", "x": 300, "y": 150}]}));
+    let mut frame = ctx.run_ui(Default::default(), |_| {});
+    frame.textures_delta.clear();
     send(
         &mut app,
         &ctx,
@@ -83,10 +92,13 @@ fn control_rotate_view_gesture_turns_the_canvas_and_reset_restores_it() {
     assert_eq!(view.center, vectorcraft_geom::Point::new(200.0, 150.0));
     assert_eq!(view.zoom, 1.0);
     assert_eq!(app.session.active().unwrap().revision, revision, "navigation does not edit the document");
-    send(&mut app, &ctx, "engine.execute", json!({"command": "view.rotateReset", "params": {}}));
+    let reset = control_response(&mut app, &ctx, "engine.execute", json!({"command": "view.rotateReset", "params": {}}));
+    assert_eq!(reset["ok"], true);
+    assert!(reset["result"].is_null());
     assert_eq!(app.view().unwrap().rotation, 0.0);
 }
 
+/// Repeated document points stay fixed across requests, including Shift snapping from a turned view.
 #[test]
 fn control_rotate_view_keeps_press_coordinates_and_shift_snapping_across_requests() {
     let mut app = with_document();
@@ -109,6 +121,7 @@ fn control_rotate_view_keeps_press_coordinates_and_shift_snapping_across_request
     assert_eq!(app.view().unwrap().rotation, 45.0);
 }
 
+/// Invalid coordinates and document changes cannot turn a different view.
 #[test]
 fn control_rotate_view_rejects_nonfinite_positions_and_a_changed_document() {
     let mut app = with_document();
@@ -128,6 +141,78 @@ fn control_rotate_view_rejects_nonfinite_positions_and_a_changed_document() {
     assert_eq!(app.view().unwrap().rotation, 0.0);
 }
 
+/// An invalid packet leaves the angle unchanged and requires a fresh press, including errors
+/// after an otherwise valid drag and malformed releases or hovers.
+#[test]
+fn control_rotate_view_rejects_malformed_packets_without_leaving_a_press() {
+    let drag = json!({"kind": "drag", "x": 200, "y": 250});
+    for packet in [
+        json!({"events": [drag.clone(), {"kind": "drag", "y": 250}]}),
+        json!({"events": [drag.clone(), {"kind": "drag", "x": "200", "y": 250}]}),
+        json!({"events": [drag.clone(), {"kind": "drag", "x": 200, "y": 250, "mods": {"shift": "yes"}}]}),
+        json!({"mods": "shift", "events": [drag.clone()]}),
+        json!({"events": [drag.clone(), {"kind": "drag", "x": 200, "y": 250, "mods": {"unknown": true}}]}),
+        json!({"events": [drag.clone(), {"kind": "drag", "x": 1e300, "y": 250}]}),
+        json!({"events": [drag.clone(), {"kind": "up", "x": 1e300, "y": 250}]}),
+        json!({"events": [drag.clone(), {"kind": "move", "x": 1e300, "y": 250}]}),
+        json!({"events": [drag.clone(), {"kind": "doubleclick", "x": 1e300, "y": 250}]}),
+        json!({"events": [drag.clone(), {"kind": "wiggle", "x": 200, "y": 250}]}),
+        json!({"events": [drag.clone(), {"kind": "drag", "x": 200, "y": 250, "space": "window"}]}),
+        json!({"events": null}),
+    ] {
+        let mut app = with_document();
+        let ctx = egui::Context::default();
+        app.canvas_rect = Some(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(800.0, 600.0)));
+        let view = app.view_mut().unwrap();
+        view.center = vectorcraft_geom::Point::new(200.0, 150.0);
+        view.zoom = 1.0;
+        super::synthetic::control(&mut app, &ctx, "ui.tool.select", json!({"tool": "rotateView"}));
+        super::synthetic::control(&mut app, &ctx, "ui.pointer", json!({"events": [{"kind": "down", "x": 300, "y": 150}]}));
+        let reply = control_response(&mut app, &ctx, "ui.pointer", packet.clone());
+        assert_eq!(reply["ok"], false, "{packet}: {reply}");
+        assert_eq!(app.view().unwrap().rotation, 0.0, "{packet}");
+        let resume = control_response(&mut app, &ctx, "ui.pointer", json!({"events": [drag.clone()]}));
+        assert_eq!(resume["ok"], false, "a failed packet cancels its press: {packet}");
+        assert_eq!(app.view().unwrap().rotation, 0.0);
+    }
+}
+
+/// Screen-space gestures survive frames, stop on release or a tool change, and start afresh
+/// from the current angle. A press at the pivot uses the same finite convention as the mouse.
+#[test]
+fn control_rotate_view_screen_events_require_a_live_press() {
+    let mut app = with_document();
+    let ctx = egui::Context::default();
+    app.canvas_rect = Some(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(800.0, 600.0)));
+    let view = app.view_mut().unwrap();
+    view.center = vectorcraft_geom::Point::new(1200.0, 900.0);
+    view.zoom = 2.0;
+    let send = super::synthetic::control;
+    send(&mut app, &ctx, "ui.tool.select", json!({"tool": "rotateView"}));
+    let down = json!({"events": [{"kind": "down", "x": 500, "y": 300, "space": "screen"}]});
+    let drag = json!({"events": [{"kind": "drag", "x": 400, "y": 400, "space": "screen"}]});
+    assert_eq!(control_response(&mut app, &ctx, "ui.pointer", drag.clone())["ok"], false);
+    send(&mut app, &ctx, "ui.pointer", down.clone());
+    let mut frame = ctx.run_ui(Default::default(), |_| {});
+    frame.textures_delta.clear();
+    send(&mut app, &ctx, "ui.pointer", drag.clone());
+    assert!((app.view().unwrap().rotation - 90.0).abs() < 1e-3);
+    send(&mut app, &ctx, "ui.pointer", json!({"events": [{"kind": "up", "x": 400, "y": 400, "space": "screen"}]}));
+    assert_eq!(control_response(&mut app, &ctx, "ui.pointer", drag.clone())["ok"], false, "release ends the press");
+    send(&mut app, &ctx, "ui.pointer", down.clone());
+    send(&mut app, &ctx, "ui.tool.select", json!({"tool": "selection"}));
+    send(&mut app, &ctx, "ui.tool.select", json!({"tool": "rotateView"}));
+    assert_eq!(control_response(&mut app, &ctx, "ui.pointer", drag.clone())["ok"], false, "a tool change ends the press");
+    send(&mut app, &ctx, "ui.pointer", down);
+    send(&mut app, &ctx, "ui.pointer", drag);
+    assert!((app.view().unwrap().rotation.abs() - 180.0).abs() < 1e-3, "a new press starts at the current angle");
+    let before = app.view().unwrap().rotation;
+    send(&mut app, &ctx, "ui.pointer", json!({"events": [{"kind": "down", "x": 400, "y": 300, "space": "screen"}]}));
+    send(&mut app, &ctx, "ui.pointer", json!({"events": [{"kind": "drag", "x": 500, "y": 300, "space": "screen"}]}));
+    assert!(app.view().unwrap().rotation.is_finite());
+    assert!((app.view().unwrap().rotation - before).abs() < 1e-3);
+}
+
 #[test]
 fn the_view_menu_lists_the_status_bar_angles_and_checks_the_current_one() {
     let mut app = with_document();

docs/mcp.md

@@ -4014,6 +4014,8 @@ angle it set, normalized to (-180, 180], and `view.rotateReset` turns the view b
 Select the Rotate View Tool with `select_tool` and the tool id `rotateView`. Its `pointer_gesture`
 drags turn the canvas about its center; Shift snaps to 15°. Document coordinates for a rotation
 gesture use the view at the initial down event, including when the gesture spans several calls.
+The control channel rejects malformed rotation packets without changing the angle and cancels
+the current press. Start a new down event after an error.
 Reset Rotate View is the command `view.rotateReset`:
 
 ```json