From a5e3e3ad1e28c9a1fd336fb950d570f0b033b3d2 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 19 Oct 2026 11:52:50 +0000 Subject: [PATCH] fix(channels): treat an empty agent reply as delivered Telegram, Discord, and WebChat returned -1 for an empty reply, so cron skipped the ack and reran the agent and its tools on every poll. An empty body cannot be posted, so those sends now succeed and a leftover -1 no longer blocks the ack. Co-authored-by: esadrianno --- CHANGELOG.md | 3 ++ src/channels/discord.c | 6 +++- src/channels/telegram.c | 6 +++- src/channels/webchat.c | 4 +++ src/core/dispatch.c | 5 ++- tests/test_dispatch.c | 71 +++++++++++++++++++++++++++++++++++++++++ tests/test_telegram.c | 26 +++++++++++++++ 7 files changed, 118 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0252119..be7eee1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ All notable changes to ShellClaw are documented here. Format follows [Keep a Cha ## [Unreleased] +### Fixed +- An empty agent reply is a completed delivery for Telegram, Discord, and WebChat. Those channels reject an empty post, and treating that rejection as a failed send skipped the cron ack, so the job stayed due and reran the agent and its tools on every poll. + ## [1.0.1] - 2026-10-18 ### Fixed diff --git a/src/channels/discord.c b/src/channels/discord.c index d1fb119..1e55dde 100644 --- a/src/channels/discord.c +++ b/src/channels/discord.c @@ -959,12 +959,16 @@ static int discord_send(const char *recipient, const char *text, (void)attachments; if (att_count > 0) return -1; - if (!dc->token || !recipient || !text || text[0] == '\0') + if (!dc->token || !recipient || !text) return -1; /* A single REST message is capped at 2000 characters, so a longer * reply is delivered as several messages. */ if (discord_recipient_channel_id(recipient, ctx.channel_id, sizeof(ctx.channel_id)) != 0) return -1; + /* Discord rejects an empty content body. A -1 here skips the cron ack + * and the job reruns the agent on every poll. */ + if (text[0] == '\0') + return 0; if (snprintf(ctx.auth_hdr, sizeof(ctx.auth_hdr), "Authorization: Bot %s", dc->token) >= (int)sizeof(ctx.auth_hdr)) return -1; diff --git a/src/channels/telegram.c b/src/channels/telegram.c index 899c59c..dac476e 100644 --- a/src/channels/telegram.c +++ b/src/channels/telegram.c @@ -471,7 +471,7 @@ static int tg_send(const char *recipient, const char *text, (void)attachments; (void)att_count; - if (!g_tg.initialized || !g_tg.token || !recipient || !text || text[0] == '\0') + if (!g_tg.initialized || !g_tg.token || !recipient || !text) return -1; chat_id = strchr(recipient, ':'); if (chat_id) @@ -480,6 +480,10 @@ static int tg_send(const char *recipient, const char *text, chat_id = recipient; if (chat_id[0] == '\0') return -1; + /* Telegram rejects an empty sendMessage. Returning -1 made cron skip + * its ack, so a null model reply stayed due and reran the agent. */ + if (text[0] == '\0') + return 0; ctx.chat_id = chat_id; ctx.posted_before = 0; if (channel_send_chunked(text, TG_TEXT_MAX, tg_post_piece, &ctx, &fail) != 0) { diff --git a/src/channels/webchat.c b/src/channels/webchat.c index 2dbc5d6..806c292 100644 --- a/src/channels/webchat.c +++ b/src/channels/webchat.c @@ -48,6 +48,10 @@ static int webchat_send(const char *recipient, const char *text, (void)attachments; (void)att_count; if (!recipient || !text) return -1; + /* ws_send_to rejects a zero-length frame. Cron would treat that as a + * failed delivery and rerun the agent. There is nothing to enqueue. */ + if (text[0] == '\0') + return 0; return ws_send_to(recipient, text); } diff --git a/src/core/dispatch.c b/src/core/dispatch.c index 68d2427..be1ac4c 100644 --- a/src/core/dispatch.c +++ b/src/core/dispatch.c @@ -36,7 +36,10 @@ static int send_then_maybe_ack_cron(const channel_t *ch, const channel_incoming_ if (!ch || !ch->send || !msg) return -1; send_err = ch->send(msg->session_id, text, NULL, 0); - if (send_err != 0) + /* An empty reply cannot be posted. A channel that still returns -1 + * must not skip the cron ack: the job would stay due and the agent, + * tools included, would run again on every poll. */ + if (send_err != 0 && !(text && text[0] == '\0')) return send_err; if (agent_err != 0) return 0; diff --git a/tests/test_dispatch.c b/tests/test_dispatch.c index 4286bac..e9a54bc 100644 --- a/tests/test_dispatch.c +++ b/tests/test_dispatch.c @@ -55,6 +55,9 @@ static int mock_send(const char *session_id, const char *text, g_last_session[sizeof(g_last_session) - 1] = '\0'; strncpy(g_last_text, text ? text : "", sizeof(g_last_text) - 1); g_last_text[sizeof(g_last_text) - 1] = '\0'; + /* Telegram, Discord, and WebChat reject an empty post. */ + if (!text || text[0] == '\0') + return -1; return 0; } @@ -126,6 +129,28 @@ static const provider_t fail_provider = { .cleanup = spy_cleanup, }; +static int empty_chat(const provider_message_t *messages, size_t message_count, + const provider_tool_def_t *tools, size_t tool_count, + provider_response_t *response) +{ + (void)messages; + (void)message_count; + (void)tools; + (void)tool_count; + response->error = 0; + response->content = NULL; + response->tool_calls = NULL; + response->tool_calls_count = 0; + return 0; +} + +static const provider_t empty_provider = { + .name = "empty", + .init = spy_init, + .chat = empty_chat, + .cleanup = spy_cleanup, +}; + static int lockcheck_chat(const provider_message_t *messages, size_t message_count, const provider_tool_def_t *tools, size_t tool_count, provider_response_t *response) @@ -439,6 +464,51 @@ static int test_cron_agent_failure_does_not_ack(void) return 0; } +/* A finished cron turn whose model content is null (no tool calls, or the + * tool cap hit on a tool-call turn) must ack. Otherwise next_run stays due + * and the next poll reruns the agent. */ +static int test_cron_empty_reply_acks_job(void) +{ + const char *db_path = "build/test_dispatch_cron_empty.db"; + char tmpl[] = "/tmp/shellclaw_test_dispatch_cron_empty_XXXXXX"; + channel_incoming_msg_t msg = {0}; + config_t *cfg = NULL; + cron_job_row_t row; + long long due_at; + int fd; + reset_send_spy(); + memory_cleanup(); + remove(db_path); + ASSERT(memory_init(db_path) == 0); + due_at = (long long)time(NULL) - 1; + ASSERT(cron_job_create("cron_empty", "interval:60", "remind me", "telegram", "42", + due_at, 1) == 0); + fd = mkstemp(tmpl); + ASSERT(fd >= 0); + close(fd); + ASSERT(write_minimal_toml(tmpl) == 0); + cfg = load_minimal_cfg(tmpl); + ASSERT(cfg != NULL); + bootstrap_set_cfg(cfg); + bootstrap_set_provider_for_test(&empty_provider); + bootstrap_reset_tools_for_test(); + msg.session_id = "telegram:42"; + msg.user_id = "cron_empty"; + msg.text = "remind me"; + ASSERT(handle_message(&cron_mock_channel, &msg) == 0); + ASSERT(g_send_calls == 1); + ASSERT(g_last_text[0] == '\0'); + memset(&row, 0, sizeof(row)); + ASSERT(cron_job_get_by_id("cron_empty", &row) == 1); + ASSERT(row.next_run > due_at); + cron_job_row_free(&row); + config_free(cfg); + unlink(tmpl); + memory_cleanup(); + remove(db_path); + return 0; +} + static int test_handle_message_holds_agent_mutex(void) { channel_incoming_msg_t msg = {0}; @@ -502,6 +572,7 @@ int main(void) RUN(test_handle_message_holds_agent_mutex()); RUN(test_cron_slash_commands_ack_jobs()); RUN(test_cron_agent_failure_does_not_ack()); + RUN(test_cron_empty_reply_acks_job()); puts("test_dispatch OK"); return 0; } diff --git a/tests/test_telegram.c b/tests/test_telegram.c index 975e869..b0278da 100644 --- a/tests/test_telegram.c +++ b/tests/test_telegram.c @@ -285,6 +285,31 @@ static void test_vtable_fields_set(void) MU_ASSERT(ch->cleanup != NULL, "cleanup not null"); } +/* An empty agent reply used to return -1 before any HTTP call. Cron treats + * that as a failed delivery and leaves next_run in the past, so the same job + * reruns the agent on every poll. Telegram would 400 an empty sendMessage, + * so the only successful outcome is to post nothing. */ +static void test_send_empty_text_succeeds(void) +{ + const channel_t *ch; + config_t *cfg = load_test_config("/tmp/test_tg_empty_send.toml", "42"); + MU_ASSERT(cfg != NULL, "load config"); + setenv("TG_TOK", "test-token", 1); + ch = channel_telegram_get(); + MU_ASSERT(ch->init(cfg) == 0, "telegram init"); + MU_ASSERT(ch->send("telegram:42", "", NULL, 0) == 0, + "empty reply to a chat id is not a delivery failure"); + MU_ASSERT(ch->send("42", "", NULL, 0) == 0, + "empty reply to a raw chat id is not a delivery failure"); + MU_ASSERT(ch->send("telegram:", "", NULL, 0) != 0, + "empty chat id still fails"); + MU_ASSERT(ch->send(NULL, "", NULL, 0) != 0, "missing recipient still fails"); + ch->cleanup(); + unsetenv("TG_TOK"); + config_free(cfg); + remove("/tmp/test_tg_empty_send.toml"); +} + int main(void) { MU_RUN(test_parse_valid_text_message); @@ -298,6 +323,7 @@ MU_RUN(test_getupdates_url_requests_one_update); MU_RUN(test_send_response_requires_ok); MU_RUN(test_429_retry_after_parsed_and_capped); MU_RUN(test_vtable_fields_set); + MU_RUN(test_send_empty_text_succeeds); printf("%d tests run, %d failed\n", tests_run, tests_failed); return tests_failed ? 1 : 0; }