From d34b5df775cd4dcbdbd4ef7b381493db30928807 Mon Sep 17 00:00:00 2001 From: Mikkel Georgsen <7456949+mikl0s@users.noreply.github.com> Date: Sat, 10 Oct 2026 18:40:53 +0200 Subject: [PATCH] Slides: going to a slide that does not exist is an error slide.go clamped an out-of-range index to the last slide and reported success, so a request for slide 99 of a short deck landed on the last slide without a word. It now returns an error and stays on the current slide. A slide id beyond u32 is reported as an unknown slide instead of being read as id 0. --- crates/engine/src/cmd/slide.rs | 9 ++++++++- crates/mcp/src/tests.rs | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/slide.rs b/crates/engine/src/cmd/slide.rs index 3d8ad344..367c269e 100644 --- a/crates/engine/src/cmd/slide.rs +++ b/crates/engine/src/cmd/slide.rs @@ -283,9 +283,16 @@ fn go_to(s: &mut Session, i: usize) -> Result { fn go(s: &mut Session, p: &Value) -> Result { let i = match (usize_param(p, "index"), p.get("id").and_then(Value::as_u64)) { (Some(i), _) => i, - (None, Some(id)) => s.doc()?.doc.slide_index(SlideId(u32::try_from(id).unwrap_or(0))).ok_or_else(|| bad("slide.go", "no such slide"))?, + (None, Some(id)) => s + .doc()? + .doc + .slide_index(SlideId(u32::try_from(id).map_err(|_| bad("slide.go", "no such slide"))?)) + .ok_or_else(|| bad("slide.go", "no such slide"))?, _ => return Err(bad("slide.go", "missing `index`")), }; + if i >= s.doc()?.doc.slides.len() { + return Err(bad("slide.go", format!("no slide {i}"))); + } go_to(s, i) } fn next(s: &mut Session, _p: &Value) -> Result { diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index b1521521..6c14f208 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -90,6 +90,26 @@ fn errors_are_reported_not_panics() { assert_eq!(r["result"]["isError"], true); } +fn current_slide_title(s: &mut Server) -> Value { + tool(s, "inspect_slide", json!({}))["title"].clone() +} + +#[test] +fn go_to_slide_out_of_range_is_error() { + let mut s = Server::new(Box::new(Headless::new())); + tool(&mut s, "new_presentation", json!({"theme": "Harbor"})); + tool(&mut s, "add_slide", json!({"layout": "titleAndContent", "title": "Second", "body": "x"})); + // Boundaries stay valid: first and last slide. + tool(&mut s, "go_to_slide", json!({"index": 0})); + tool(&mut s, "go_to_slide", json!({"index": 1})); + assert_eq!(current_slide_title(&mut s), "Second"); + for bad in [json!(2), json!(99)] { + let r = call(&mut s, 1, "tools/call", json!({"name": "go_to_slide", "arguments": {"index": bad}})); + assert_eq!(r["result"]["isError"], true, "{bad}: {r}"); + assert_eq!(current_slide_title(&mut s), "Second", "failed go_to_slide must not move"); + } +} + #[test] fn export_tool_lists_pdf_and_its_options() { let mut s = Server::new(Box::new(Headless::new()));