From 58e125ccf1acf8618109cd456c5d73b875b913ae Mon Sep 17 00:00:00 2001 From: Marco Arnulfo <103679419+marcoarnulfo@users.noreply.github.com> Date: Thu, 15 Oct 2026 08:23:58 +0200 Subject: [PATCH] Automation: tell a tool name or an unknown id from a command without a tool in command_run (fixes #1759) (#1765) --- crates/automation/src/conventions.rs | 18 +++++++++++++++++- crates/automation/tests/automation.rs | 18 ++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/crates/automation/src/conventions.rs b/crates/automation/src/conventions.rs index af8cf671ac..ac670109fb 100644 --- a/crates/automation/src/conventions.rs +++ b/crates/automation/src/conventions.rs @@ -2,15 +2,31 @@ use serde_json::{Value, json}; use crate::{Args, Automation, Content, Result, ToolError, encode_png, failed, info, tools}; +use pdfcraft_engine::commands; use pdfcraft_render::{RenderRequest, RequestKind}; /// The most steps one `command_batch` runs. pub(crate) const MAX_BATCH_STEPS: usize = 1000; +/// Why `id` has no tool to run: a tool name passed as a command id, an id the registry doesn't +/// know, or a real command that nothing automates yet. They need different fixes from the caller. +fn no_tool_for(id: &str) -> ToolError { + if commands::command(id).is_some() { + return failed(format!("command {id:?} has no headless tool; see command_list")); + } + match tools::find(id) { + Some(def) => match def.command { + Some(command) => failed(format!("{id:?} is a tool, not a command id; use the command id {command:?}, or call the tool directly")), + None => failed(format!("{id:?} is a tool, not a command id; call it directly (command ids are in command_list)")), + }, + None => failed(format!("unknown command id {id:?}; see command_list")), + } +} + impl Automation { pub(crate) fn command_run(&mut self, args: &Args) -> Result> { let id = args.str("id")?; - let tool = tools::tool_for_command(id).ok_or_else(|| failed(format!("command {id:?} has no headless tool; see command_list")))?; + let tool = tools::tool_for_command(id).ok_or_else(|| no_tool_for(id))?; let def = tools::find(tool).ok_or_else(|| failed("command tool is unavailable"))?; let mut params = match args.get("params") { None | Some(Value::Null) => serde_json::Map::new(), diff --git a/crates/automation/tests/automation.rs b/crates/automation/tests/automation.rs index d44c7ab5b4..a6c6fad46b 100644 --- a/crates/automation/tests/automation.rs +++ b/crates/automation/tests/automation.rs @@ -5179,6 +5179,24 @@ fn command_batch_refuses_more_than_a_thousand_steps() { assert!(err.to_string().contains("at most 1000 steps"), "{err}"); } +#[test] +fn command_batch_tells_a_tool_name_from_an_unknown_or_toolless_command() { + let dir = workdir("batch-ids"); + let mut a = auto(&dir); + let steps = json!([{"id":"page_rotate"}, {"id":"doc_inspect"}, {"id":"no.such.command"}, {"id":"help.about"}]); + let r = ok(&mut a, "command_batch", json!({"steps": steps, "stop_on_error": false})); + let errors: Vec<&str> = r["results"].as_array().unwrap().iter().map(|s| s["error"].as_str().unwrap()).collect(); + assert_eq!( + errors, + [ + r#""page_rotate" is a tool, not a command id; use the command id "page.rotate", or call the tool directly"#, + r#""doc_inspect" is a tool, not a command id; call it directly (command ids are in command_list)"#, + r#"unknown command id "no.such.command"; see command_list"#, + r#"command "help.about" has no headless tool; see command_list"#, + ] + ); +} + #[test] fn thai_text_through_tools_saves_reopens_extracts_and_renders() { let dir = workdir("thai-content");