From d53025cc34babf15126737e17e06d88b209ea4b2 Mon Sep 17 00:00:00 2001
From: storybored379 <storybored379@users.noreply.github.com>
Date: Sat, 3 Oct 2026 23:42:36 -0700
Subject: [PATCH] MCP: crop returns an error when given no rect, angle or reset
 instead of a silent no-op

The crop tool fell through to crop.set with an empty object when none of
its parameters were given, so a call with a guessed parameter (e.g.
`aspect: "1:1"`) or `reset: false` answered "ok" and left the photo
unchanged. It now returns an error naming the parameters it needs, as
set_develop already does for an empty call.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 crates/mcp/src/tests.rs | 12 ++++++++++++
 crates/mcp/src/tools.rs |  5 ++++-
 docs/mcp.md             |  2 +-
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs
index b97e0497..0a7bbc57 100644
--- a/crates/mcp/src/tests.rs
+++ b/crates/mcp/src/tests.rs
@@ -102,6 +102,18 @@ fn set_develop_values_and_settings() {
     assert!(call_tool(&mut b, "set_develop", &json!({"values": {"no.such": 1}})).is_error);
 }
 
+#[test]
+fn crop_needs_a_rect_angle_or_reset() {
+    let mut b = Headless::demo();
+    let first = b.session.visible_cloned()[0];
+    // A guessed parameter must not succeed silently.
+    let r = call_tool(&mut b, "crop", &json!({"id": first.0, "aspect": "1:1"}));
+    assert!(r.is_error, "{r:?}");
+    assert!(call_tool(&mut b, "crop", &json!({"id": first.0, "reset": false})).is_error);
+    assert!(!call_tool(&mut b, "crop", &json!({"id": first.0, "rect": [0.1, 0.0, 0.9, 1.0]})).is_error);
+    assert!(!call_tool(&mut b, "crop", &json!({"id": first.0, "reset": true})).is_error);
+}
+
 #[test]
 fn ui_tools_need_the_app() {
     let mut b = Headless::demo();
diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs
index 81d9a811..0829fddd 100644
--- a/crates/mcp/src/tools.rs
+++ b/crates/mcp/src/tools.rs
@@ -154,7 +154,7 @@ pub fn helper_tools(has_ui: bool) -> Vec<Value> {
         tool(
             "crop",
             "Crop / straighten",
-            "Set the crop of the active photo: `rect` [x0,y0,x1,y1] in normalized coordinates (0..1, origin top-left) of the straightened frame, `angle` in degrees. `reset: true` clears it.",
+            "Set the crop of the active photo: `rect` [x0,y0,x1,y1] in normalized coordinates (0..1, origin top-left) of the straightened frame, `angle` in degrees. `reset: true` clears it. Give at least one of them.",
             json!({"id": photo_id, "rect": {"type": "array", "items": {"type": "number"}, "minItems": 4, "maxItems": 4}, "angle": {"type": "number"}, "reset": {"type": "boolean"}}),
             &[],
         ),
@@ -515,6 +515,9 @@ pub fn call_tool(b: &mut dyn Backend, name: &str, args: &Value) -> ToolResult {
             }
             None => ToolResult::error("missing `preset`"),
         },
+        "crop" if args.get("rect").is_none() && args.get("angle").is_none() && args.get("reset").and_then(Value::as_bool) != Some(true) => {
+            ToolResult::error("give `rect`, `angle` or `reset: true`")
+        }
         "crop" => ToolResult::from(activate(b, args).and_then(|_| {
             if args.get("reset").and_then(Value::as_bool) == Some(true) {
                 exec(b, "crop.reset", json!({}))
diff --git a/docs/mcp.md b/docs/mcp.md
index fe3cfdcf..2220edfe 100644
--- a/docs/mcp.md
+++ b/docs/mcp.md
@@ -83,7 +83,7 @@ During development you can also point the client at `cargo run --release -p ligh
 | `get_develop {id?}` | Full develop-settings JSON |
 | `set_develop {id?, values?, settings?, label?}` | `values`: `{controlId: number}`; `settings`: partial develop JSON deep-merged. Undoable |
 | `apply_preset {preset, amount?, ids?}` | Apply a preset (ids from `cmd_presets_list`) |
-| `crop {id?, rect?, angle?, reset?}` | Normalized crop rect `[x0,y0,x1,y1]` and straighten angle |
+| `crop {id?, rect?, angle?, reset?}` | Normalized crop rect `[x0,y0,x1,y1]` and straighten angle; at least one of `rect`, `angle`, `reset: true` |
 | `render_photo {id?, size?, format?, path?}` | Render with current settings → **image content** (PNG, or JPEG with `format: "jpeg"`), long edge `size` (default 1024) |
 | `export {path \| dir, id? \| ids?, format?, longEdge? \| shortEdge? \| width?/height? \| megapixels? \| percent?, dontEnlarge?, ppi?, quality?, colorSpace?, bitDepth?, …}` | Full-quality render to `.png` / `.jpg` / `.tif` / `.webp` / `.avif`; `format: "original"` copies the file + an XMP sidecar with the edits, `format: "dng"` writes raw photos as DNG with the edits embedded. No size param = 3000 px long edge; `longEdge: 0` = full size (cropped, native resolution); `width` + `height` fit either orientation; `dontEnlarge` defaults to true |