From 70244c899d270f6e18fdcacee50cd08c0cacfa89 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 17 Oct 2026 05:19:50 +0200 Subject: [PATCH] Reject invalid comment.delete index instead of clearing the slide Found while testing crates/engine/src/cmd/review.rs:140-155 (delete) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/review.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/review.rs b/crates/engine/src/cmd/review.rs index 65933bbd..9b061c8b 100644 --- a/crates/engine/src/cmd/review.rs +++ b/crates/engine/src/cmd/review.rs @@ -137,7 +137,14 @@ fn reply(s: &mut Session, p: &Value) -> Result { fn delete(s: &mut Session, p: &Value) -> Result { let all = bool_or(p, "all", false); - let i = usize_param(p, "index"); + let i = match p.get("index") { + None | Some(Value::Null) => None, + Some(v) => Some( + v.as_u64() + .and_then(|v| usize::try_from(v).ok()) + .ok_or_else(|| bad("comment.delete", format!("`index` is not a valid comment index: {v}")))?, + ), + }; s.edit(|doc, sel| { if all { for sl in &mut doc.slides { @@ -793,4 +800,18 @@ mod tests { s.execute("show.setup", &json!({"custom": ""})).unwrap(); assert!(s.active().unwrap().doc.show.custom_show.is_none()); } + + #[test] + fn delete_rejects_an_invalid_index_and_keeps_the_comments() { + let mut s = Session::with_new(); + s.execute("comment.add", &json!({"text": "a"})).unwrap(); + s.execute("comment.add", &json!({"text": "b"})).unwrap(); + for bad_index in [json!(-1), json!("2"), json!(1.5)] { + let r = s.execute("comment.delete", &json!({"index": bad_index})); + assert!(r.is_err(), "index {bad_index} gave {r:?}"); + assert_eq!(s.active().unwrap().doc.slides[0].comments.len(), 2, "index {bad_index}"); + } + s.execute("comment.delete", &json!({})).unwrap(); + assert!(s.active().unwrap().doc.slides[0].comments.is_empty()); + } }