From d4c36feb9acd9b105dc8affe0dbd2bbd93ae6925 Mon Sep 17 00:00:00 2001 From: YoursFunny Date: Sun, 20 Sep 2026 16:42:40 +0800 Subject: [PATCH] fix(commands): make /set_format and /clear_cache actually parse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit teloxide's `split` parser takes exactly one space-separated token per field, so `/set_format ` — a two-token command — never parsed: `Command::parse` failed, `message_handler` fell through to the URL flow, and the user got silence. `/clear_cache` without its optional link failed the same way ("too few arguments"), so clearing everything was unreachable. Both now use the crate's `parse_arg_remainder` (whole remainder, trimmed), which is what their executors were already written against (`split_once(char::is_whitespace)`). Found by driving the real binary against a scripted fake Bot API: the `/set_format` replies never appeared while `/set_template` and the other single-token commands did. `every_documented_invocation_parses` now pins every documented form, which is what should have caught it. The placeholder validation and `-` reset added earlier only work now that the command reaches its executor at all. --- AGENTS.md | 2 +- crates/xmedia-bot/src/handlers/commands.rs | 74 +++++++++++++++++++++- 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5f8f71a..deff08f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -24,7 +24,7 @@ Debug command: `/debug ` runs the same `x_media::site::fetch` and replies w User-facing failure text is a function of the error class, never one generic sentence: `urls::fetch_error_message` maps `FetchError::NotFound` (post gone), `Sensitive` (withheld, needs `TWITTER_AUTH_TOKEN`), `Blocked` (source risk control), `Disabled { site }` (a registered site switched off — pixiv without a token, the one case `fetch` answers `Err` instead of `Ok(None)`) and `Transient`/`Http` (source down) apart. The same distinction drives the group hint: a supported link posted in a group (not a channel) gets one `GROUP_LINK_HINT` reply, because the link pipeline is private-chat only. -The `/test ` command runs the ordinary link pipeline (`urls::url_media`) with `PostSend::Suppressed`: the media is sent and cached like any other link, but the chat's `forward_channel_id`/`edit_before_forward` are ignored, so a test never forwards to the channel and never opens the edit prompt (retries and dead-letter notifications behave as usual). Both commands use a custom `parse_arg_remainder` parser (whole remainder, trimmed) because teloxide's built-in `split` parser takes exactly one space-separated token. +The `/test ` command runs the ordinary link pipeline (`urls::url_media`) with `PostSend::Suppressed`: the media is sent and cached like any other link, but the chat's `forward_channel_id`/`edit_before_forward` are ignored, so a test never forwards to the channel and never opens the edit prompt (retries and dead-letter notifications behave as usual). `/test`, `/debug`, `/set_format` and `/clear_cache` use the custom `parse_arg_remainder` parser (whole remainder, trimmed) because teloxide's built-in `split` parser takes exactly one space-separated token per field: `/set_format ` never parsed with it (and `/clear_cache` without an argument did not either), and a command that fails to parse falls through to the URL flow in silence. `commands::tests::every_documented_invocation_parses` pins every documented form against exactly that. The inline path (`handlers/inline.rs`) hands media URLs straight to Telegram, which fetches them itself and cannot send site-specific headers — so `x_media::site::needs_media_headers(url)` (true exactly where a site's `media_headers` is non-empty, i.e. pixiv's pximg.net) marks the media that must be skipped instead of shipped broken; locally produced media (ugoira MP4, bsky remux) fails `Url::parse` and is skipped the same way. Inline results are therefore URL-only by construction. diff --git a/crates/xmedia-bot/src/handlers/commands.rs b/crates/xmedia-bot/src/handlers/commands.rs index 20005e7..a25e90c 100644 --- a/crates/xmedia-bot/src/handlers/commands.rs +++ b/crates/xmedia-bot/src/handlers/commands.rs @@ -42,12 +42,12 @@ pub(crate) enum Command { BotDict, #[command( description = "Set site caption format (- to reset)", - parse_with = "split" + parse_with = parse_arg_remainder )] SetFormat(String), #[command( description = "Clear link cache (admin; optional URL, else all)", - parse_with = "split" + parse_with = parse_arg_remainder )] ClearCache(String), #[command( @@ -987,6 +987,76 @@ mod tests { } } + #[test] + fn every_documented_invocation_parses() { + use teloxide::utils::command::BotCommands; + + use super::Command; + + // The README's forms, verbatim. teloxide's `split` parser accepts + // EXACTLY one token per `String` field, so a command documented with + // two arguments (or an optional one) silently stops parsing — and a + // command that does not parse falls through to the URL flow in + // silence. + type Check = fn(&Command) -> bool; + let cases: Vec<(&str, Check)> = vec![ + ("/start", |c| matches!(c, Command::Start)), + ("/help", |c| matches!(c, Command::Help)), + ("/settings", |c| matches!(c, Command::Settings)), + ("/edit_before_forward", |c| { + matches!(c, Command::EditBeforeForward) + }), + ("/remove_forward_channel", |c| { + matches!(c, Command::RemoveForwardChannel) + }), + ("/bot_dict", |c| matches!(c, Command::BotDict)), + ( + "/set_forward_channel @a_channel", + |c| matches!(c, Command::SetForwardChannel(a) if a == "@a_channel"), + ), + ( + "/set_template tpl", + |c| matches!(c, Command::SetTemplate(a) if a == "tpl"), + ), + ( + "/remove_template tpl", + |c| matches!(c, Command::RemoveTemplate(a) if a == "tpl"), + ), + ( + "/set_format twitter {author}: {title}", + |c| matches!(c, Command::SetFormat(a) if a == "twitter {author}: {title}"), + ), + ( + "/set_format twitter -", + |c| matches!(c, Command::SetFormat(a) if a == "twitter -"), + ), + // Documented as "clear everything" when called without a link. + ( + "/clear_cache", + |c| matches!(c, Command::ClearCache(a) if a.is_empty()), + ), + ( + "/clear_cache https://x.com/u/status/1", + |c| matches!(c, Command::ClearCache(a) if a == "https://x.com/u/status/1"), + ), + ( + "/test https://x.com/u/status/1", + |c| matches!(c, Command::Test(a) if a == "https://x.com/u/status/1"), + ), + ( + "/debug https://x.com/u/status/1", + |c| matches!(c, Command::Debug(a) if a == "https://x.com/u/status/1"), + ), + ]; + + for (text, ok) in cases { + match Command::parse(text, "") { + Ok(parsed) => assert!(ok(&parsed), "{text} parsed as the wrong variant"), + Err(e) => panic!("{text} did not parse: {e}"), + } + } + } + #[test] fn unknown_placeholder_finds_typos_only() { assert_eq!(unknown_placeholder("{author} — {title}"), None);