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; }