From 9ef40b98866ca7270fa4621c83028357c7ed79b8 Mon Sep 17 00:00:00 2001
From: Michael Anderson <andermic@gmail.com>
Date: Wed, 14 Oct 2026 11:28:23 -0500
Subject: [PATCH] Commands: Refuse malformed photo ids instead of using the
 selection (#1506)

A wrongly typed id or ids fell back to the selection, or kept only the valid part of the list. Session::targets now returns an error for those, and callers pass it on.
---
 crates/engine/src/cmd/browse.rs           |  4 +--
 crates/engine/src/cmd/convert.rs          |  8 +++---
 crates/engine/src/cmd/cull.rs             |  2 +-
 crates/engine/src/cmd/develop.rs          | 15 ++++++-----
 crates/engine/src/cmd/face_detect.rs      |  2 +-
 crates/engine/src/cmd/face_recognize.rs   |  2 +-
 crates/engine/src/cmd/geocode.rs          |  2 +-
 crates/engine/src/cmd/keywords.rs         |  6 ++---
 crates/engine/src/cmd/library.rs          | 21 +++++++--------
 crates/engine/src/cmd/manage.rs           |  6 ++---
 crates/engine/src/cmd/merge.rs            |  2 +-
 crates/engine/src/cmd/mod.rs              |  2 +-
 crates/engine/src/cmd/organize.rs         | 14 +++++-----
 crates/engine/src/cmd/previews.rs         |  6 ++---
 crates/engine/src/cmd/xmp.rs              |  4 +--
 crates/engine/src/contact_sheet.rs        |  2 +-
 crates/engine/src/lib.rs                  | 28 +++++++++++++++-----
 crates/engine/src/tests.rs                | 32 +++++++++++++++++++++++
 crates/ui-egui/src/menubar.rs             |  2 +-
 crates/ui-egui/src/menus.rs               |  6 ++---
 crates/ui-egui/src/merge.rs               |  4 +--
 crates/ui-egui/src/panels/dialogs.rs      | 14 +++++-----
 crates/ui-egui/src/panels/grid.rs         |  4 +--
 crates/ui-egui/src/panels/keyword_list.rs |  2 +-
 crates/ui-egui/src/panels/keywording.rs   |  4 +--
 crates/ui-egui/src/panels/left.rs         |  2 +-
 crates/ui-egui/src/panels/right.rs        |  4 +--
 crates/ui-egui/src/shortcuts.rs           |  2 +-
 crates/ui-egui/src/tests_keywording.rs    |  2 +-
 docs/whats-new.md                         |  5 ++++
 30 files changed, 130 insertions(+), 79 deletions(-)

diff --git a/crates/engine/src/cmd/browse.rs b/crates/engine/src/cmd/browse.rs
index a681ec14b..0b4049a06 100644
--- a/crates/engine/src/cmd/browse.rs
+++ b/crates/engine/src/cmd/browse.rs
@@ -151,7 +151,7 @@ fn forget_local(s: &mut Session, p: &Value) -> Result<Value> {
 
 /// The browsed photos among `ids` (default: the selection) become library photos.
 fn add_to_library(s: &mut Session, p: &Value) -> Result<Value> {
-    let ids: Vec<PhotoId> = s.targets(p).into_iter().filter(|id| s.catalog.photo(*id).is_some_and(|ph| ph.local)).collect();
+    let ids: Vec<PhotoId> = s.targets(p)?.into_iter().filter(|id| s.catalog.photo(*id).is_some_and(|ph| ph.local)).collect();
     if ids.is_empty() {
         return Ok(json!({"added": 0}));
     }
@@ -337,7 +337,7 @@ fn move_to_folder(s: &mut Session, p: &Value) -> Result<Value> {
     if !dir.is_dir() {
         return Err(bad(C, format!("{folder} is not a folder (create it first with folder.create)")));
     }
-    let ids = s.targets(p);
+    let ids = s.targets(p)?;
     if ids.is_empty() {
         return Err(bad(C, "no photos to move: select some or give ids"));
     }
diff --git a/crates/engine/src/cmd/convert.rs b/crates/engine/src/cmd/convert.rs
index 4d7f13aeb..ef01bf47f 100644
--- a/crates/engine/src/cmd/convert.rs
+++ b/crates/engine/src/cmd/convert.rs
@@ -58,7 +58,7 @@ pub(crate) fn write_dng_with(file_bytes: Option<&crate::merge::ByteReader>, path
 
 fn convert(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "photo.convertToDng";
-    let ids = s.targets(p);
+    let ids = s.targets(p)?;
     let mut ops = Vec::new();
     let mut converted = Vec::new();
     let mut skipped = Vec::new();
@@ -124,7 +124,7 @@ pub fn specs() -> Vec<CommandSpec> {
 /// the library and stack it on top of the original. The host opens the file in the editor.
 fn edit_external(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "photo.editExternal";
-    let mut ids = s.targets(p);
+    let mut ids = s.targets(p)?;
     ids.dedup();
     if ids.is_empty() {
         return Err(bad(C, "no active photo"));
@@ -274,7 +274,7 @@ pub(crate) fn as_shot_wb_op(id: lightcraft_catalog::PhotoId, ph: &lightcraft_cat
 /// Re-read photos whose files changed on disk (an external editor saved them): new size,
 /// dimensions and content hash, cached sources dropped. → {reloaded: [ids]}
 fn reload(s: &mut Session, p: &Value) -> Result<Value> {
-    let ids = s.targets(p);
+    let ids = s.targets(p)?;
     let paths: Vec<(lightcraft_catalog::PhotoId, String)> = ids
         .iter()
         .filter_map(|id| match s.catalog.photo(*id).map(|ph| ph.source.clone()) {
@@ -320,7 +320,7 @@ fn reload(s: &mut Session, p: &Value) -> Result<Value> {
 fn duplicate(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "photo.duplicate";
     let mut made = Vec::new();
-    for id in s.targets(p) {
+    for id in s.targets(p)? {
         let Some(ph) = s.catalog.photo(id).map(|p| (**p).clone()) else { continue };
         let Source::File { path } = &ph.source else { return Err(bad(C, "a generated demo photo has no file to duplicate")) };
         let src = Path::new(path);
diff --git a/crates/engine/src/cmd/cull.rs b/crates/engine/src/cmd/cull.rs
index 51049797e..676519d29 100644
--- a/crates/engine/src/cmd/cull.rs
+++ b/crates/engine/src/cmd/cull.rs
@@ -20,7 +20,7 @@ fn secs(iso: &str) -> Option<i64> {
 }
 
 fn analyze(s: &mut Session, p: &Value) -> Result<Value> {
-    let ids: Vec<PhotoId> = if p.get("ids").is_some() || s.selection.ids.len() > 1 { s.targets(p) } else { s.visible_cloned() };
+    let ids: Vec<PhotoId> = if p.get("ids").is_some() || s.selection.ids.len() > 1 { s.targets(p)? } else { s.visible_cloned() };
     let reject_below = p.get("rejectBelow").and_then(Value::as_f64).map(|v| v as f32);
     let pick_best = p.get("pickBest").and_then(Value::as_bool).unwrap_or(false);
     // measure (thumbnail-level sources: fast, and enough for focus at 512 px)
diff --git a/crates/engine/src/cmd/develop.rs b/crates/engine/src/cmd/develop.rs
index 6f064b260..f85998138 100644
--- a/crates/engine/src/cmd/develop.rs
+++ b/crates/engine/src/cmd/develop.rs
@@ -254,7 +254,7 @@ pub fn specs() -> Vec<CommandSpec> {
             })
         }),
         cmd!("develop.reset", "Reset Edits", ["Photo"], Some("Cmd+Shift+R"), "{ids?}", has_selection, |s, p| {
-            let ids = s.targets(p);
+            let ids = s.targets(p)?;
             let ops = ids
                 .iter()
                 .filter_map(|id| s.develop_of(*id).map(|d| (*id, d)))
@@ -304,7 +304,7 @@ pub fn specs() -> Vec<CommandSpec> {
                     let targets = match named_photos(s, p, C)? {
                         Some(ids) if !ids.iter().any(|id| outdated(s, *id)) => return Err(crate::EngineError::Disabled(C.into(), already_current())),
                         Some(ids) => ids,
-                        None => s.targets(p),
+                        None => s.targets(p)?,
                     };
                     let ops: Vec<Op> = targets
                         .into_iter()
@@ -810,7 +810,7 @@ pub fn specs() -> Vec<CommandSpec> {
                     s.previous_active.filter(|id| s.catalog.photo(*id).is_some_and(|ph| !ph.deleted)).ok_or_else(|| bad(c, "no previous photo"))?;
                 let groups = groups_param(p, c)?.unwrap_or_else(|| s.copy_groups.clone());
                 let partial = lightcraft_develop::extract_groups(&s.develop_of(prev).unwrap_or_default(), &groups);
-                let ids: Vec<PhotoId> = s.targets(p).into_iter().filter(|id| *id != prev).collect();
+                let ids: Vec<PhotoId> = s.targets(p)?.into_iter().filter(|id| *id != prev).collect();
                 let ops = ids
                     .iter()
                     .filter_map(|id| s.develop_of(*id).map(|d| (*id, d)))
@@ -837,7 +837,7 @@ pub fn specs() -> Vec<CommandSpec> {
                     let full = lightcraft_develop::apply_partial(&DevelopSettings::default(), &clip, 1.0);
                     clip = lightcraft_develop::extract_groups(&full, &only);
                 }
-                let ids = s.targets(p);
+                let ids = s.targets(p)?;
                 let ops = ids
                     .iter()
                     .filter_map(|id| s.develop_of(*id).map(|d| (*id, d)))
@@ -889,7 +889,7 @@ pub fn specs() -> Vec<CommandSpec> {
                 let delta = f64_req(p, "delta", c)?;
                 let label = format!("Quick Develop: {}", spec.label);
                 let ops: Vec<Op> = s
-                    .targets(p)
+                    .targets(p)?
                     .into_iter()
                     .filter_map(|id| s.develop_of(id).map(|d| (id, d)))
                     .filter_map(|(id, d)| {
@@ -942,7 +942,7 @@ pub fn specs() -> Vec<CommandSpec> {
                 };
                 let r_ev = gathered(s, reference).ok_or_else(|| bad(c, "the active photo has no shutter / aperture / ISO"))?;
                 let r_slider = s.develop_of(reference).map(|d| d.light.exposure).unwrap_or(0.0);
-                let ids: Vec<PhotoId> = s.targets(p).into_iter().filter(|x| *x != reference).collect();
+                let ids: Vec<PhotoId> = s.targets(p)?.into_iter().filter(|x| *x != reference).collect();
                 let (mut ops, mut skipped) = (Vec::new(), Vec::new());
                 for id in ids {
                     let (Some(ev), Some(d)) = (gathered(s, id), s.develop_of(id)) else {
@@ -977,7 +977,8 @@ pub fn specs() -> Vec<CommandSpec> {
             let pid = str_param(p, "id").ok_or_else(|| bad("preset.apply", "missing id"))?;
             let preset = s.presets.iter().find(|x| x.id == pid).cloned().ok_or_else(|| bad("preset.apply", format!("unknown preset `{pid}`")))?;
             let amount = f64_or(p, "amount", 100.0).clamp(0.0, 200.0) / 100.0;
-            let ids = s.targets(p);
+            // `id` names the preset here: the photos are `ids`, else the selection
+            let ids = s.targets(&json!({"ids": p.get("ids")}))?;
             let ops = ids
                 .iter()
                 .filter_map(|id| s.develop_of(*id).map(|d| (*id, d)))
diff --git a/crates/engine/src/cmd/face_detect.rs b/crates/engine/src/cmd/face_detect.rs
index 4676d1c65..2328a3db4 100644
--- a/crates/engine/src/cmd/face_detect.rs
+++ b/crates/engine/src/cmd/face_detect.rs
@@ -132,7 +132,7 @@ fn detect(s: &mut Session, p: &Value) -> Result<Value> {
     let detector = detector(s).map_err(|e| e.into_error(C))?;
     let started = Instant::now();
     let (mut results, mut ops) = (Vec::new(), Vec::new());
-    for id in s.targets(p) {
+    for id in s.targets(p)? {
         let (rgb, width, height) = match render_rgb(s, id, LOOK_EDGE) {
             Ok(r) => r,
             Err(e) => {
diff --git a/crates/engine/src/cmd/face_recognize.rs b/crates/engine/src/cmd/face_recognize.rs
index e099f13ec..a30776b51 100644
--- a/crates/engine/src/cmd/face_recognize.rs
+++ b/crates/engine/src/cmd/face_recognize.rs
@@ -751,7 +751,7 @@ mod imp {
         const C: &str = "faces.suggest";
         let embedder = current(s, C)?;
         collect(s);
-        let targets = s.targets(p);
+        let targets = s.targets(p)?;
         let model_threshold = embedder.manifest().thresholds.match_cosine;
         let num = |k: &str| p.get(k).and_then(Value::as_f64).filter(|v| v.is_finite()).map(|v| v as f32);
         let threshold = num("threshold").or(model_threshold).unwrap_or(DEFAULT_THRESHOLD);
diff --git a/crates/engine/src/cmd/geocode.rs b/crates/engine/src/cmd/geocode.rs
index ddbe6b13a..085183d58 100644
--- a/crates/engine/src/cmd/geocode.rs
+++ b/crates/engine/src/cmd/geocode.rs
@@ -115,7 +115,7 @@ fn fill(meta: &mut lightcraft_catalog::Meta, place: &lightcraft_geocode::Place,
 fn reverse(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "metadata.reverseGeocode";
     let overwrite = bool_or(p, "overwrite", false);
-    let ids = s.targets(p);
+    let ids = s.targets(p)?;
     let todo: Vec<(PhotoId, LatLon)> = ids
         .iter()
         .filter_map(|id| {
diff --git a/crates/engine/src/cmd/keywords.rs b/crates/engine/src/cmd/keywords.rs
index b2c7e90f0..fe09a92f9 100644
--- a/crates/engine/src/cmd/keywords.rs
+++ b/crates/engine/src/cmd/keywords.rs
@@ -256,7 +256,7 @@ pub fn specs() -> Vec<CommandSpec> {
                 let Some(k) = current_keywords(s).get(i as usize - 1).cloned().filter(|k| !k.is_empty()) else {
                     return Ok(json!({"changed": 0}));
                 };
-                let ids = s.targets(p);
+                let ids = s.targets(p)?;
                 let all = !ids.is_empty()
                     && ids.iter().all(|id| {
                         s.catalog
@@ -288,7 +288,7 @@ pub fn specs() -> Vec<CommandSpec> {
             always,
             |s, p| {
                 let mut current: Vec<String> = Vec::new();
-                for id in s.targets(p) {
+                for id in s.targets(p)? {
                     if let Some(ph) = s.catalog.photo(id) {
                         current.extend(ph.meta.keywords.iter().cloned());
                     }
@@ -336,7 +336,7 @@ pub fn specs() -> Vec<CommandSpec> {
                     Some(parent) => format!("{parent}|{name}"),
                     None => name,
                 });
-                let photos = if bool_or(p, "addToSelected", false) { s.targets(p) } else { Vec::new() };
+                let photos = if bool_or(p, "addToSelected", false) { s.targets(p)? } else { Vec::new() };
                 let op = s
                     .catalog
                     .create_keyword_ops(&path, info_from(p, KeywordInfo::default()), &photos)
diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs
index 3a5575ea8..1908002cf 100644
--- a/crates/engine/src/cmd/library.rs
+++ b/crates/engine/src/cmd/library.rs
@@ -169,7 +169,7 @@ fn album_targets_or_err(s: &Session, command: &str, targets: Vec<PhotoId>) -> Re
 
 /// Apply one op per target as a single undo step.
 fn for_targets(s: &mut Session, p: &Value, label: &str, f: impl Fn(PhotoId) -> Option<Op>) -> Result<Value> {
-    let targets = s.targets(p);
+    let targets = s.targets(p)?;
     let ops: Vec<Op> = targets.iter().filter_map(|id| f(*id)).collect();
     let n = ops.len();
     if n > 0 {
@@ -181,7 +181,7 @@ fn for_targets(s: &mut Session, p: &Value, label: &str, f: impl Fn(PhotoId) -> O
 /// Adjust each target's own rating; unchanged bounds do not create an undo step.
 fn change_rating(s: &mut Session, p: &Value, delta: i8, label: &str) -> Result<Value> {
     let mut ops = Vec::new();
-    for id in s.targets(p) {
+    for id in s.targets(p)? {
         let old = s.catalog.photo(id).ok_or(lightcraft_catalog::CatalogError::NoPhoto(id))?.rating;
         let rating = (i16::from(old) + i16::from(delta)).clamp(0, 5) as u8;
         if rating != old {
@@ -226,7 +226,7 @@ fn row_step(p: &Value) -> isize {
 }
 
 fn rotate(s: &mut Session, p: &Value, cw: bool) -> Result<Value> {
-    let targets = s.targets(p);
+    let targets = s.targets(p)?;
     let mut ops = Vec::new();
     for id in targets {
         if let Some(d) = s.develop_of(id) {
@@ -241,7 +241,7 @@ fn rotate(s: &mut Session, p: &Value, cw: bool) -> Result<Value> {
 }
 
 fn flip(s: &mut Session, p: &Value, horizontal: bool) -> Result<Value> {
-    let targets = s.targets(p);
+    let targets = s.targets(p)?;
     let mut ops = Vec::new();
     for id in targets {
         if let Some(d) = s.develop_of(id) {
@@ -807,7 +807,7 @@ pub fn specs() -> Vec<CommandSpec> {
         ),
         cmd!("photo.deletePermanently", "Delete Permanently", ["Photo"], None, "{ids?}", has_selection, |s, p| {
             let before = s.visible_cloned();
-            let t = s.targets(p);
+            let t = s.targets(p)?;
             let op = s.catalog.delete_photos_permanently_ops(&t);
             s.commit("Delete Permanently", op)?;
             select_after_deletion(s, &before);
@@ -824,7 +824,7 @@ pub fn specs() -> Vec<CommandSpec> {
             |s, p| {
                 let strs =
                     |k: &str| p.get(k).and_then(Value::as_array).map(|a| a.iter().filter_map(Value::as_str).map(str::to_string).collect::<Vec<_>>());
-                let targets = s.targets(p);
+                let targets = s.targets(p)?;
                 // GPS: "lat, lon" (decimal or 51°30'26"N 0°7'39"W), [lat, lon], or null / "" to clear
                 let gps: Option<Option<(f64, f64)>> = match p.get("gps") {
                     None => None,
@@ -922,8 +922,7 @@ pub fn specs() -> Vec<CommandSpec> {
                 }
                 let folder = bool_or(p, "folder", false);
                 let parent = p.get("parent").and_then(Value::as_u64).map(AlbumId);
-                let (photos, skipped) =
-                    if !folder && bool_or(p, "addSelected", false) { album_targets(s, s.targets(&Value::Null)) } else { (vec![], 0) };
+                let (photos, skipped) = if !folder && bool_or(p, "addSelected", false) { album_targets(s, s.selected()) } else { (vec![], 0) };
                 let id = s.catalog.alloc_album_id();
                 let cover = photos.first().copied();
                 s.commit(
@@ -1163,7 +1162,7 @@ pub fn specs() -> Vec<CommandSpec> {
             has_library_selection,
             |s, p| {
                 let id = album_param(p, "id", "album.addPhotos")?;
-                let targets = ids_param(p).unwrap_or_else(|| s.targets(&Value::Null));
+                let targets = ids_param(p).unwrap_or_else(|| s.selected());
                 let al = s.catalog.album(id).ok_or_else(|| bad("album.addPhotos", "no such album"))?;
                 if al.is_smart() || al.folder {
                     return Err(bad("album.addPhotos", "smart albums and folders can't hold photos"));
@@ -1190,7 +1189,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "{ids?} — B: adds the photos to the target album (the Quick Collection unless one is set), or removes them when they're all in it → {album, added, skipped}; Local photos are skipped",
             has_library_selection,
             |s, p| {
-                let targets = album_targets_or_err(s, "album.toggleTarget", ids_param(p).unwrap_or_else(|| s.targets(&Value::Null)))?;
+                let targets = album_targets_or_err(s, "album.toggleTarget", ids_param(p).unwrap_or_else(|| s.selected()))?;
                 let (targets, skipped) = targets;
                 let target = s.target_album.filter(|a| s.catalog.album(*a).is_some_and(|al| !al.is_smart() && !al.folder));
                 let id = match target.or_else(|| s.catalog.quick_collection()) {
@@ -1336,7 +1335,7 @@ pub fn specs() -> Vec<CommandSpec> {
         ),
         cmd!("album.removePhotos", "Remove from Album", [], None, "{id: albumId, ids?}", has_selection, |s, p| {
             let id = album_param(p, "id", "album.removePhotos")?;
-            let targets = ids_param(p).unwrap_or_else(|| s.targets(&Value::Null));
+            let targets = ids_param(p).unwrap_or_else(|| s.selected());
             let al = s.catalog.album(id).ok_or_else(|| bad("album.removePhotos", "no such album"))?;
             if al.is_smart() {
                 return Err(bad("album.removePhotos", "smart albums update automatically: change their rules"));
diff --git a/crates/engine/src/cmd/manage.rs b/crates/engine/src/cmd/manage.rs
index d06271525..66ddfe9fb 100644
--- a/crates/engine/src/cmd/manage.rs
+++ b/crates/engine/src/cmd/manage.rs
@@ -10,7 +10,7 @@ use crate::Result;
 fn rename_args(s: &crate::Session, p: &Value, c: &str) -> Result<(Vec<lightcraft_catalog::PhotoId>, String, usize)> {
     let template = str_param(p, "template").ok_or_else(|| bad(c, "missing `template`"))?.to_string();
     let start = p.get("start").and_then(Value::as_u64).unwrap_or(1) as usize;
-    Ok((s.targets(p), template, start))
+    Ok((s.targets(p)?, template, start))
 }
 
 /// A named set of colour-label names (red, yellow, green, blue, purple; empty = the colour's name).
@@ -96,7 +96,7 @@ fn auto_tag_tracklog(s: &mut crate::Session, p: &Value) -> Result<Value> {
     let (mut no_time, mut outside, mut has_gps, mut assumed_utc, mut interpolated) = (0, 0, 0, 0, 0);
     let mut ops = Vec::new();
     let mut photos = Vec::new();
-    for id in s.targets(p) {
+    for id in s.targets(p)? {
         let Some(ph) = s.catalog.photo(id) else { continue };
         if ph.meta.gps.is_some() && !replace {
             has_gps += 1;
@@ -183,7 +183,7 @@ pub fn specs() -> Vec<CommandSpec> {
             |s, p| {
                 use lightcraft_catalog::dates::{iso_seconds, normalize_iso, shift_iso};
                 let c = "photo.setCaptureTime";
-                let targets: Vec<_> = s.targets(p).into_iter().filter(|id| s.catalog.photo(*id).is_some()).collect();
+                let targets: Vec<_> = s.targets(p)?.into_iter().filter(|id| s.catalog.photo(*id).is_some()).collect();
                 if targets.is_empty() {
                     return Err(bad(c, "no photos"));
                 }
diff --git a/crates/engine/src/cmd/merge.rs b/crates/engine/src/cmd/merge.rs
index a8869153a..6fcbad27f 100644
--- a/crates/engine/src/cmd/merge.rs
+++ b/crates/engine/src/cmd/merge.rs
@@ -20,7 +20,7 @@ fn can_merge(s: &Session) -> std::result::Result<(), String> {
 
 fn run(s: &mut Session, id: &str, p: &Value) -> Result<Value> {
     let (kind, finish) = crate::merge::parse(id, p)?;
-    let ids: Vec<PhotoId> = s.targets(p);
+    let ids: Vec<PhotoId> = s.targets(p)?;
     if !bool_or(p, "preview", false) {
         return s.merge_now(kind, finish, &ids);
     }
diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs
index 4de460761..f09ff2d78 100644
--- a/crates/engine/src/cmd/mod.rs
+++ b/crates/engine/src/cmd/mod.rs
@@ -105,7 +105,7 @@ pub fn has_selection(s: &Session) -> std::result::Result<(), String> {
 /// can take (issue #684; Add to My Photos first).
 pub fn has_library_selection(s: &Session) -> std::result::Result<(), String> {
     has_selection(s)?;
-    if s.targets(&serde_json::Value::Null).iter().any(|id| s.catalog.photo(*id).is_none_or(|p| !p.local)) { Ok(()) } else { Err(LOCAL_ONLY.into()) }
+    if s.selected().iter().any(|id| s.catalog.photo(*id).is_none_or(|p| !p.local)) { Ok(()) } else { Err(LOCAL_ONLY.into()) }
 }
 /// Why an album command refused a Local-only selection.
 pub const LOCAL_ONLY: &str = "only Local photos: albums hold library photos (Add to My Photos first)";
diff --git a/crates/engine/src/cmd/organize.rs b/crates/engine/src/cmd/organize.rs
index 494785628..24cc2061c 100644
--- a/crates/engine/src/cmd/organize.rs
+++ b/crates/engine/src/cmd/organize.rs
@@ -22,7 +22,7 @@ fn stacks_of(s: &Session, targets: &[PhotoId]) -> Vec<StackId> {
 
 fn in_stack(s: &Session) -> std::result::Result<(), String> {
     has_selection(s)?;
-    if stacks_of(s, &s.targets(&Value::Null)).is_empty() { Err("no stacked photo selected".into()) } else { Ok(()) }
+    if stacks_of(s, &s.selected()).is_empty() { Err("no stacked photo selected".into()) } else { Ok(()) }
 }
 
 fn has_stacks(s: &Session) -> std::result::Result<(), String> {
@@ -111,7 +111,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "{ids?, name?} → {ids: [new photo ids]} — a new catalog entry sharing the original file, with independent settings; stacked with its original and named `name` or \"Copy N\"",
             has_selection,
             |s, p| {
-                let targets = s.targets(p);
+                let targets = s.targets(p)?;
                 let name = str_param(p, "name").filter(|n| !n.trim().is_empty()).map(str::to_string);
                 let mut made = Vec::new();
                 for t in targets {
@@ -134,7 +134,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "{ids?, top?: photoId (default: the active photo), collapsed?: bool (default true)} — stacks the photos are already in are merged",
             has_selection,
             |s, p| {
-                let targets = s.targets(p);
+                let targets = s.targets(p)?;
                 let top = p
                     .get("top")
                     .and_then(Value::as_u64)
@@ -153,11 +153,11 @@ pub fn specs() -> Vec<CommandSpec> {
             }
         ),
         cmd!("stack.ungroup", "Ungroup Stack", ["Photo", "Stack"], Some("Cmd+Shift+G"), "{ids?}", in_stack, |s, p| {
-            let ops = s.catalog.ungroup_ops(&s.targets(p));
+            let ops = s.catalog.ungroup_ops(&s.targets(p)?);
             Ok(json!({"ungrouped": commit_ops(s, "Ungroup Stack", ops)?}))
         }),
         cmd!("stack.remove", "Remove from Stack", ["Photo", "Stack"], None, "{ids?}", in_stack, |s, p| {
-            let ops = s.catalog.remove_from_stacks_ops(&s.targets(p));
+            let ops = s.catalog.remove_from_stacks_ops(&s.targets(p)?);
             Ok(json!({"changed": commit_ops(s, "Remove from Stack", ops)?}))
         }),
         cmd!("stack.setTop", "Set as Top of Stack", ["Photo", "Stack"], Some("Shift+S"), "{id?} (default: the active photo)", in_stack, |s, p| {
@@ -189,7 +189,7 @@ pub fn specs() -> Vec<CommandSpec> {
             }
         ),
         cmd!("stack.toggle", "Expand/Collapse Stack", ["Photo", "Stack"], Some("S"), "{ids?, collapsed?: bool}", in_stack, |s, p| {
-            let stacks = stacks_of(s, &s.targets(p));
+            let stacks = stacks_of(s, &s.targets(p)?);
             let first = stacks.first().and_then(|id| s.catalog.stack(*id)).map(|st| st.collapsed).unwrap_or(false);
             let collapsed = bool_or(p, "collapsed", !first);
             set_collapsed(s, stacks, collapsed, if collapsed { "Collapse Stack" } else { "Expand Stack" })
@@ -212,7 +212,7 @@ pub fn specs() -> Vec<CommandSpec> {
             |s, p| {
                 let gap = f64_or(p, "gap", 60.0).max(0.0);
                 let explicit = p.get("ids").is_some();
-                let pool = if explicit || s.selection.ids.len() > 1 { s.targets(p) } else { s.visible_cloned() };
+                let pool = if explicit || s.selection.ids.len() > 1 { s.targets(p)? } else { s.visible_cloned() };
                 let groups = s.catalog.auto_stack_groups(&pool, gap);
                 let photos: usize = groups.iter().map(Vec::len).sum();
                 if bool_or(p, "preview", false) {
diff --git a/crates/engine/src/cmd/previews.rs b/crates/engine/src/cmd/previews.rs
index 4fc7ababa..b63ccc2b4 100644
--- a/crates/engine/src/cmd/previews.rs
+++ b/crates/engine/src/cmd/previews.rs
@@ -152,7 +152,7 @@ fn build(s: &mut Session, p: &Value) -> Result<Value> {
         return Err(bad(C, "a preview build is already running"));
     }
     // explicit ids, else the selection, else everything in view
-    let ids = if p.get("ids").is_some() || p.get("id").is_some() || !s.selection.ids.is_empty() { s.targets(p) } else { s.visible_cloned() };
+    let ids = if p.get("ids").is_some() || p.get("id").is_some() || !s.selection.ids.is_empty() { s.targets(p)? } else { s.visible_cloned() };
     let jobs: Vec<Vec<RenderJob>> = ids.iter().map(|id| jobs_for(s, *id, edge)).filter(|j| !j.is_empty()).collect();
     let state = Arc::new(PreviewBuild { total: jobs.len(), ..Default::default() });
     s.preview_build = Some(state.clone());
@@ -324,7 +324,7 @@ fn smart(s: &mut Session, p: &Value) -> Result<Value> {
     if background && s.preview_build.as_ref().is_some_and(|b| !b.finished.load(Ordering::Relaxed)) {
         return Err(bad(C, "a preview build is already running"));
     }
-    let ids = if p.get("ids").is_some() || !s.selection.ids.is_empty() { s.targets(p) } else { s.visible_cloned() };
+    let ids = if p.get("ids").is_some() || !s.selection.ids.is_empty() { s.targets(p)? } else { s.visible_cloned() };
     let mut jobs: Vec<SmartJob> = Vec::new();
     for id in ids {
         let Some(ph) = s.catalog.photo(id).cloned() else { continue };
@@ -503,7 +503,7 @@ pub fn specs() -> Vec<CommandSpec> {
             smart_location
         ),
         cmd!(query "photo.smartPreview", "Smart Preview Status", [], None, "{id?} → {smartPreview: bool, originalOnline: bool}", always, |s, p| {
-            let id = s.targets(p).first().copied().ok_or_else(|| bad("photo.smartPreview", "no photo"))?;
+            let id = s.targets(p)?.first().copied().ok_or_else(|| bad("photo.smartPreview", "no photo"))?;
             let online = match s.catalog.photo(id).map(|p| p.source.clone()) {
                 Some(lightcraft_catalog::Source::File { path }) => std::path::Path::new(&path).exists(),
                 _ => true,
diff --git a/crates/engine/src/cmd/xmp.rs b/crates/engine/src/cmd/xmp.rs
index cd89aa896..f163e499a 100644
--- a/crates/engine/src/cmd/xmp.rs
+++ b/crates/engine/src/cmd/xmp.rs
@@ -12,7 +12,7 @@ fn save(s: &mut Session, p: &Value) -> Result<Value> {
     let mut merged = Vec::new();
     let mut backups = Vec::new();
     let mut failed = Vec::new();
-    let ids = s.targets(p);
+    let ids = s.targets(p)?;
     let owners = crate::sidecar::StemOwners::among(&s.catalog, &ids);
     for id in ids {
         match s.save_sidecar_with(id, &owners) {
@@ -37,7 +37,7 @@ fn read(s: &mut Session, p: &Value) -> Result<Value> {
     let mut read = Vec::new();
     let mut failed = Vec::new();
     let mut warnings = Vec::new();
-    for id in s.targets(p) {
+    for id in s.targets(p)? {
         match s.read_sidecar_op_with_warnings(id) {
             Ok(Some((op, from, mut photo_warnings))) => {
                 ops.push(op);
diff --git a/crates/engine/src/contact_sheet.rs b/crates/engine/src/contact_sheet.rs
index e83ad8c67..320323fa3 100644
--- a/crates/engine/src/contact_sheet.rs
+++ b/crates/engine/src/contact_sheet.rs
@@ -82,7 +82,7 @@ pub fn prepare(session: &mut Session, p: &Value) -> Result<Prepared, String> {
     if p.get("id").is_some_and(|v| v.as_u64().is_none()) {
         return Err("id must be a photo id".into());
     }
-    let ids = session.targets(p);
+    let ids = session.targets(p).map_err(|e| e.to_string())?;
     if ids.is_empty() || ids.len() > 1000 {
         return Err("select between 1 and 1000 photos".into());
     }
diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs
index 7e05fcffd..b8f9e7857 100644
--- a/crates/engine/src/lib.rs
+++ b/crates/engine/src/lib.rs
@@ -427,7 +427,11 @@ impl Session {
         }
         let empty = Value::Object(Default::default());
         let params = if params.is_null() { &empty } else { params };
-        self.run_command(id, spec.journal.then_some(params), |s| (spec.run)(s, params))
+        self.run_command(id, spec.journal.then_some(params), |s| (spec.run)(s, params)).map_err(|e| match e {
+            // a shared parameter check ([`Session::targets`]) names the command it ran for
+            EngineError::BadParams { cmd, msg } if cmd.is_empty() => EngineError::BadParams { cmd: id.to_string(), msg },
+            e => e,
+        })
     }
 
     /// Run `f` as command `id` with what [`Session::execute`] does around every command: the
@@ -963,14 +967,24 @@ impl Session {
         (self.visible_gen, self.visible.clone())
     }
 
-    /// Targets of photo commands: explicit `ids`/`id` param, else the selection.
-    pub fn targets(&self, p: &Value) -> Vec<PhotoId> {
-        if let Some(a) = p.get("ids").and_then(Value::as_array) {
-            return a.iter().filter_map(Value::as_u64).map(PhotoId).collect();
+    /// Targets of photo commands: the explicit `ids` (photo ids) or `id` (a photo id) param, else [`Self::selected`].
+    /// An `ids` or `id` that is there but isn't photo ids is an error, never the selection or part of the list
+    /// (issue #1506): a mistyped target mustn't turn a command meant for one photo into one for every selected photo.
+    pub fn targets(&self, p: &Value) -> Result<Vec<PhotoId>> {
+        let invalid = |what: &str| EngineError::BadParams { cmd: String::new(), msg: format!("`{what}` must be photo ids (whole numbers)") };
+        match p.get("ids") {
+            None | Some(Value::Null) => {}
+            Some(Value::Array(a)) => return a.iter().map(|v| v.as_u64().map(PhotoId).ok_or_else(|| invalid("ids"))).collect(),
+            Some(_) => return Err(invalid("ids")),
         }
-        if let Some(id) = p.get("id").and_then(Value::as_u64) {
-            return vec![PhotoId(id)];
+        match p.get("id") {
+            None | Some(Value::Null) => Ok(self.selected()),
+            Some(v) => v.as_u64().map(|id| vec![PhotoId(id)]).ok_or_else(|| invalid("id")),
         }
+    }
+
+    /// The photos commands act on when a call names none: the selection, else the active photo.
+    pub fn selected(&self) -> Vec<PhotoId> {
         if self.selection.ids.is_empty() { self.selection.active.into_iter().collect() } else { self.selection.ids.clone() }
     }
 }
diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs
index 4e5e73850..dc586be0f 100644
--- a/crates/engine/src/tests.rs
+++ b/crates/engine/src/tests.rs
@@ -1486,3 +1486,35 @@ fn oversized_spots_render() {
         }
     }
 }
+
+/// A call that names its photos with a malformed `id` or `ids` (a string, a mixed list) is refused and changes
+/// nothing; it used to act on the selection, or on the valid part of the list (issue #1506). Leaving the targets
+/// out still means the selection.
+#[test]
+fn malformed_photo_targets_are_refused_not_widened() {
+    use lightcraft_catalog::PhotoId;
+    let mut s = Session::with_demo();
+    let ids: Vec<u64> = s.catalog.photos().take(3).map(|p| p.id.0).collect();
+    s.execute("library.select", &json!({"ids": [ids[0], ids[1]]})).unwrap();
+    let rating = |s: &Session, i: u64| s.catalog.photo(PhotoId(i)).unwrap().rating;
+    let before: Vec<u8> = ids.iter().map(|i| rating(&s, *i)).collect();
+    for bad in [
+        json!({"rating": 5, "id": "5"}),
+        json!({"rating": 5, "ids": [ids[2], "x"]}),
+        json!({"rating": 5, "ids": ids[2].to_string()}),
+        json!({"rating": 5, "id": -1}),
+    ] {
+        let err = s.execute("photo.rate", &bad).unwrap_err();
+        assert!(matches!(&err, crate::EngineError::BadParams { cmd, .. } if cmd == "photo.rate"), "{bad}: {err}");
+        assert_eq!(ids.iter().map(|i| rating(&s, *i)).collect::<Vec<_>>(), before, "{bad} changed a rating");
+    }
+    let snapshot = s.catalog.to_snapshot();
+    assert!(s.execute("photo.deletePermanently", &json!({"id": "5"})).is_err());
+    assert_eq!(s.catalog.to_snapshot(), snapshot, "nothing deleted");
+    // no targets, or a null one: the selection
+    s.execute("photo.rate", &json!({"rating": 4, "id": null})).unwrap();
+    assert_eq!((rating(&s, ids[0]), rating(&s, ids[1]), rating(&s, ids[2])), (4, 4, before[2]));
+    // a preset's id is not a photo target
+    let preset = s.presets.first().unwrap().id.clone();
+    s.execute("preset.apply", &json!({"id": preset})).unwrap();
+}
diff --git a/crates/ui-egui/src/menubar.rs b/crates/ui-egui/src/menubar.rs
index fe6905cad..ae0b429ff 100644
--- a/crates/ui-egui/src/menubar.rs
+++ b/crates/ui-egui/src/menubar.rs
@@ -644,7 +644,7 @@ fn expanded(app: &LightcraftApp, name: &str) -> Option<Vec<MenuNode>> {
             v
         }
         "Add to Album" => {
-            let targets = app.session.targets(&Value::Null);
+            let targets = app.session.selected();
             let sel = !targets.is_empty();
             // albums hold library photos (issue #684): a Local-only selection is told what to do first
             let local_only = crate::panels::grid::local_only(app, &targets);
diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs
index ad31c57bf..ba7607e05 100644
--- a/crates/ui-egui/src/menus.rs
+++ b/crates/ui-egui/src/menus.rs
@@ -1932,7 +1932,7 @@ pub fn ui_enabled(app: &LightcraftApp, id: &str) -> bool {
         "app.clearRecentLibraries" => !app.ui.settings.recent_libraries.is_empty(),
         "compare.swap" | "compare.makeSelect" | "compare.set" => app.ui.view == ViewMode::Compare,
         s if s.starts_with("dialog.merge") || (s.starts_with("merge.") && s.ends_with("Last")) => {
-            app.session.targets(&serde_json::json!({})).len() >= 2 && app.merge.final_task.is_none()
+            app.session.selected().len() >= 2 && app.merge.final_task.is_none()
         }
         _ => true,
     }
@@ -1979,7 +1979,7 @@ pub fn confirm_delete(app: &mut LightcraftApp) -> bool {
     if !app.ui.settings.confirm_delete {
         return false;
     }
-    let count = app.session.targets(&json!({})).len();
+    let count = app.session.selected().len();
     if count == 0 {
         return false;
     }
@@ -2097,7 +2097,7 @@ fn open_original_in(app: &mut LightcraftApp, ctx: &egui::Context, p: &Value) ->
 }
 
 fn open_originals(app: &mut LightcraftApp, p: &Value) -> Result<Value, String> {
-    let mut ids = app.session.targets(p);
+    let mut ids = app.session.targets(p).map_err(|e| e.to_string())?;
     ids.dedup();
     if ids.is_empty() {
         return Err("no photo selected".into());
diff --git a/crates/ui-egui/src/merge.rs b/crates/ui-egui/src/merge.rs
index a603692fa..05e7baa8e 100644
--- a/crates/ui-egui/src/merge.rs
+++ b/crates/ui-egui/src/merge.rs
@@ -148,7 +148,7 @@ fn spawn(app: &LightcraftApp, options: &MergeDialog, ids: &[PhotoId], preview: b
 
 /// Open the merge dialog for the selection.
 pub fn open(app: &mut LightcraftApp, command: &str) -> Result<Value, String> {
-    let ids = app.session.targets(&json!({}));
+    let ids = app.session.selected();
     if ids.len() < 2 {
         return Err("select at least 2 photos to merge".into());
     }
@@ -185,7 +185,7 @@ pub fn start_final(app: &mut LightcraftApp, opts: &MergeDialog) -> Result<Value,
 /// Merge the selection without the dialog, with the options last used for `command` (the
 /// defaults the first time).
 pub fn start_last(app: &mut LightcraftApp, command: &str) -> Result<Value, String> {
-    let ids = app.session.targets(&json!({}));
+    let ids = app.session.selected();
     if ids.len() < 2 {
         return Err("select at least 2 photos to merge".into());
     }
diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs
index a79261fdc..08b05e4bf 100644
--- a/crates/ui-egui/src/panels/dialogs.rs
+++ b/crates/ui-egui/src/panels/dialogs.rs
@@ -22,7 +22,7 @@ pub(crate) fn rename_preview(
 ) -> (Option<Vec<lightcraft_engine::rename::RenamePlan>>, usize) {
     use std::hash::{Hash, Hasher};
     type Rows = std::sync::Arc<std::sync::Mutex<Option<Vec<lightcraft_engine::rename::RenamePlan>>>>;
-    let ids = app.session.targets(&json!({}));
+    let ids = app.session.selected();
     let photos = lightcraft_engine::rename::rename_photos(&app.session.catalog, &ids);
     let total = photos.len();
     let mut h = std::collections::hash_map::DefaultHasher::new();
@@ -71,7 +71,7 @@ fn watermark_preview(
     type Base = std::sync::Arc<std::sync::Mutex<(u8, Option<Result<lightcraft_raster::Rgba8, String>>)>>;
     type Shown = (u64, u8, lightcraft_engine::export::Watermark, usize, egui::TextureHandle);
     let t = Tokens::get(ui.ctx());
-    let Some(&id) = app.session.targets(&json!({})).first() else { return };
+    let Some(&id) = app.session.selected().first() else { return };
     let Some(photo) = app.session.catalog.photo(id).cloned() else { return };
     let (out_w, out_h) = lightcraft_engine::export::output_size(&photo, &lightcraft_engine::export::ExportOptions { resize, ..Default::default() });
     let export_short = out_w.min(out_h);
@@ -439,7 +439,7 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) {
                     );
                 }
                 Dialog::CaptureTime { mode, time, days, hours, minutes, zone } => {
-                    let n = app.session.targets(&json!({})).len();
+                    let n = app.session.selected().len();
                     field(ui, "Change", |ui| {
                         ui.spacing_mut().item_spacing.x = 4.0;
                         for (i, (label, m)) in [("Set date & time", "set"), ("Shift by", "shift"), ("Time zone", "zone")].iter().enumerate() {
@@ -523,7 +523,7 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) {
                     );
                 }
                 Dialog::Rename { template, start } => {
-                    let n = app.session.targets(&json!({})).len();
+                    let n = app.session.selected().len();
                     ui.label(egui::RichText::new(crate::i18n::tr_format!("{n} photo{}", if n == 1 { "" } else { "s" }, n = n)).color(t.text_dim));
                     let template_id = egui::Id::new("rename-template");
                     let tags_open = field(ui, "Template", |ui| {
@@ -614,7 +614,7 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) {
                         confirm = true;
                     }
                     // as in Lightroom, the option is only there when something is selected
-                    if !*folder && !app.session.targets(&json!({})).is_empty() {
+                    if !*folder && !app.session.selected().is_empty() {
                         ui.add_space(6.0);
                         let r = ui.checkbox(include_selected, crate::i18n::tr("Include selected photos"));
                         crate::widgets::register(ui.ctx(), "check:albumIncludeSelected", r.rect);
@@ -633,7 +633,7 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) {
                 }
                 Dialog::CopySettings { groups } => group_checklist(ui, "copyGroup", groups),
                 Dialog::PasteSettings { groups } => {
-                    let n = app.session.targets(&json!({})).len();
+                    let n = app.session.selected().len();
                     ui.label(
                         egui::RichText::new(crate::i18n::tr_format!("Paste into {n} photo{} — only settings that were copied are pasted", if n == 1 { "" } else { "s" }, n = n))
                             .color(t.text_dim),
@@ -643,7 +643,7 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) {
                 Dialog::ContactSheet { options } => {
                     options.columns = options.columns.clamp(1, 8);
                     options.rows = options.rows.clamp(1, 10);
-                    let n = app.session.targets(&json!({})).len();
+                    let n = app.session.selected().len();
                     ui.label(format!("{n} selected photos · {} pages", n.div_ceil(options.columns * options.rows)));
                     field(ui, "Paper", |ui| {
                         egui::ComboBox::from_id_salt("contactSheetPaper").selected_text(&options.paper).show_ui(ui, |ui| {
diff --git a/crates/ui-egui/src/panels/grid.rs b/crates/ui-egui/src/panels/grid.rs
index 52edfa460..76952b5ce 100644
--- a/crates/ui-egui/src/panels/grid.rs
+++ b/crates/ui-egui/src/panels/grid.rs
@@ -1040,7 +1040,7 @@ pub fn context_menu(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) {
         }
     });
     ui.menu_button(crate::i18n::tr("Set Color Label"), |ui| label_menu(app, ui));
-    if local_only(app, &app.session.targets(&serde_json::Value::Null)) {
+    if local_only(app, &app.session.selected()) {
         // albums hold library photos (issue #684): nothing here to add until they join My Photos
         let add = ui.add_enabled(false, egui::Button::new(crate::i18n::tr("Add to Album"))).on_disabled_hover_text(albums_hint());
         register(ui.ctx(), "button:contextAddToAlbum", add.rect);
@@ -1146,7 +1146,7 @@ pub fn context_menu(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) {
         let _ = app.run("develop.reset", json!({}));
     }
     ui.menu_button(crate::i18n::tr("Photo Merge"), |ui| {
-        let n = app.session.targets(&json!({})).len();
+        let n = app.session.selected().len();
         for (id, label) in [("dialog.mergeHdr", "HDR…"), ("dialog.mergePanorama", "Panorama…"), ("dialog.mergeHdrPanorama", "HDR Panorama…")] {
             if ui.add_enabled(n >= 2, egui::Button::new(crate::i18n::tr(label))).clicked() {
                 let _ = app.run(id, json!({}));
diff --git a/crates/ui-egui/src/panels/keyword_list.rs b/crates/ui-egui/src/panels/keyword_list.rs
index 2d691112a..57ffb3549 100644
--- a/crates/ui-egui/src/panels/keyword_list.rs
+++ b/crates/ui-egui/src/panels/keyword_list.rs
@@ -72,7 +72,7 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) {
             ui.label(egui::RichText::new(crate::i18n::tr(none)).color(t.text_dim));
         });
     }
-    let selection = app.session.targets(&serde_json::Value::Null);
+    let selection = app.session.selected();
     let ticks = app.caches.keyword_ticks(&app.session.catalog, &selection);
     for r in &rows {
         row(app, ui, r, &selection, &ticks, filter.trim().is_empty());
diff --git a/crates/ui-egui/src/panels/keywording.rs b/crates/ui-egui/src/panels/keywording.rs
index 857498599..6a0f8b493 100644
--- a/crates/ui-egui/src/panels/keywording.rs
+++ b/crates/ui-egui/src/panels/keywording.rs
@@ -30,7 +30,7 @@ pub fn view_switch(app: &mut LightcraftApp, ui: &mut egui::Ui) {
 /// exported files will carry. One only some of the photos have is marked with an asterisk.
 pub fn names_row(app: &mut LightcraftApp, ui: &mut egui::Ui) {
     let t = Tokens::get(ui.ctx());
-    let selection = app.session.targets(&serde_json::Value::Null);
+    let selection = app.session.selected();
     let export = app.ui.keywording_view == crate::state::KeywordingView::WillExport;
     let names = app.caches.keyword_names(&app.session.catalog, &selection, export);
     if names.is_empty() {
@@ -120,7 +120,7 @@ fn menu(app: &mut LightcraftApp, ui: &mut egui::Ui, chip: &Chip) {
 /// The selection's keywords as chips: the name (right-click for its menu), and × to take it off
 /// every selected photo. One only some of them have is marked with an asterisk.
 pub fn chip_row(app: &mut LightcraftApp, ui: &mut egui::Ui) {
-    let selection = app.session.targets(&serde_json::Value::Null);
+    let selection = app.session.selected();
     let chips = app.caches.keyword_chips(&app.session.catalog, &selection);
     ui.horizontal_wrapped(|ui| {
         for c in chips.iter() {
diff --git a/crates/ui-egui/src/panels/left.rs b/crates/ui-egui/src/panels/left.rs
index fcaab2926..5e19661fc 100644
--- a/crates/ui-egui/src/panels/left.rs
+++ b/crates/ui-egui/src/panels/left.rs
@@ -1571,7 +1571,7 @@ fn folder_menu(app: &mut LightcraftApp, resp: &egui::Response, a: &Album) {
             ui.separator();
         }
         if !a.folder && !a.is_smart() {
-            let local_only = super::grid::local_only(app, &app.session.targets(&serde_json::Value::Null));
+            let local_only = super::grid::local_only(app, &app.session.selected());
             let b = ui
                 .add_enabled(!local_only, egui::Button::new(crate::i18n::tr("Add Selected Photos")))
                 .on_disabled_hover_text(super::grid::albums_hint());
diff --git a/crates/ui-egui/src/panels/right.rs b/crates/ui-egui/src/panels/right.rs
index 8d0cc0d3a..60f1aa3cc 100644
--- a/crates/ui-egui/src/panels/right.rs
+++ b/crates/ui-egui/src/panels/right.rs
@@ -876,7 +876,7 @@ fn keywords(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) {
         let typed = ui.data(|d| d.get_temp::<String>(kid).unwrap_or_default());
         let last = typed.rsplit(',').next().unwrap_or("").trim().to_string();
         // the keywords every selected photo has: those only some have are still suggested
-        let selection = app.session.targets(&serde_json::Value::Null);
+        let selection = app.session.selected();
         let have: Vec<String> =
             app.caches.keyword_chips(&app.session.catalog, &selection).iter().filter(|c| c.on_all()).map(|c| c.path.clone()).collect();
         let suggestions = (*app.caches.suggestions(&app.session.catalog, &have, &last, 12)).clone();
@@ -904,7 +904,7 @@ fn keywords(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) {
 /// selected photos; "Save as Set…" keeps the current nine under a name.
 fn keyword_set(app: &mut LightcraftApp, ui: &mut egui::Ui) {
     // on: every selected photo has it (⌥1–⌥9 then take it off them); partly on: only some do
-    let selection = app.session.targets(&serde_json::Value::Null);
+    let selection = app.session.selected();
     let chips = app.caches.keyword_chips(&app.session.catalog, &selection);
     let t = Tokens::get(ui.ctx());
     let sets = lightcraft_engine::cmd::keywords::keyword_sets_json(&app.session);
diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs
index b10387c76..c88d01683 100644
--- a/crates/ui-egui/src/shortcuts.rs
+++ b/crates/ui-egui/src/shortcuts.rs
@@ -491,7 +491,7 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) {
             // B: the brush while editing; in the grids, add to the target album (Quick Collection)
             if f == "tool.brush" && matches!(app.ui.view, crate::state::ViewMode::PhotoGrid | crate::state::ViewMode::SquareGrid) {
                 if let Ok(r) = app.run("album.toggleTarget", json!({})) {
-                    let n = app.session.targets(&json!({})).len();
+                    let n = app.session.selected().len();
                     let what = crate::i18n::tr_format!("{n} photo{}", if n == 1 { "" } else { "s" }, n = n);
                     let name = r["name"].as_str().unwrap_or("Quick Collection").to_string();
                     app.toast(
diff --git a/crates/ui-egui/src/tests_keywording.rs b/crates/ui-egui/src/tests_keywording.rs
index a130b86b2..acf2c1d54 100644
--- a/crates/ui-egui/src/tests_keywording.rs
+++ b/crates/ui-egui/src/tests_keywording.rs
@@ -191,7 +191,7 @@ fn the_active_photo_alone_is_the_selection() {
     assert_eq!(h.app.session.selection.active.map(|p| p.0), Some(ids[0]));
     assert!(h.app.session.selection.ids.is_empty());
     assert!(has(&h, "keywordChip:Weddings") && !has(&h, "keywordChipPartial:Weddings"), "the active photo's keywords");
-    let ticks = crate::panels::keyword_list::Ticks::of(&h.app.session.catalog, &h.app.session.targets(&json!({})));
+    let ticks = crate::panels::keyword_list::Ticks::of(&h.app.session.catalog, &h.app.session.selected());
     assert_eq!(ticks.tick("Weddings"), crate::panels::keyword_list::Tick::All);
     assert!(has(&h, "keywordCheck:beach"));
 }
diff --git a/docs/whats-new.md b/docs/whats-new.md
index 1bf44c3f4..0ba1a14cb 100644
--- a/docs/whats-new.md
+++ b/docs/whats-new.md
@@ -2,6 +2,11 @@
 
 ## 2026-10-14
 
+### Reliability
+- A command given photos it can't read as photo ids (`"id": "5"`, or a list with something else in it) from a
+  script, the control channel or MCP is now refused (issue #1506). It used to act on the selected photos instead,
+  or on the part of the list it could read, so a mistyped id could rate, move or delete the wrong photos.
+
 ### Hard-edged Heal and Clone spots
 - A spot with Feather at 0 no longer draws black pixels exactly on its radius. This also fixes spots at 0% Opacity
   changing pixels along their edge. The hard edge now leaves pixels on the radius unchanged.