From 9bfd9c5c0f224a8cd6db0edad09dce3cdf8b3850 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Fri, 16 Oct 2026 00:09:06 -0300 Subject: [PATCH 1/6] fix(channels): keep Telegram polling after a bad getUpdates batch getUpdates used the API default of 100 updates. A backlog larger than the 256 KiB response buffer made curl abort before the offset advanced, so every later poll retried the same batch and the inbox stalled. Messages that omit from.id returned a hard error and pinned the same way. Request one update per poll, and skip a message with no sender id so the following update can be fetched. Supersedes #112. --- src/channels/telegram.c | 41 +++++++++++++++++++++++++++++++++-------- tests/test_telegram.c | 22 +++++++++++++++++++++- 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/src/channels/telegram.c b/src/channels/telegram.c index 45e3da3..c2e4a87 100644 --- a/src/channels/telegram.c +++ b/src/channels/telegram.c @@ -148,14 +148,15 @@ static int parse_update(const char *json, channel_incoming_msg_t *out, long *upd return 0; } cJSON *from = cJSON_GetObjectItem(msg, "from"); - if (!from || !cJSON_IsObject(from)) { - cJSON_Delete(root); - return -1; - } - cJSON *from_id = cJSON_GetObjectItem(from, "id"); + cJSON *from_id = (from && cJSON_IsObject(from)) ? cJSON_GetObjectItem(from, "id") : NULL; + /* Channel and anonymous-admin posts can omit a numeric from.id. Failing + * the poll here left last_update_id unchanged, so this update stayed at + * the head of getUpdates and later messages never ran. Skip it instead. */ if (!from_id || !cJSON_IsNumber(from_id)) { + fprintf(stderr, "shellclaw: telegram: skip update %ld (message has no from id)\n", + *update_id_out); cJSON_Delete(root); - return -1; + return 0; } long user_id = (long)from_id->valuedouble; if (!is_user_allowed(g_tg.cfg, user_id)) { @@ -221,14 +222,32 @@ static int parse_update(const char *json, channel_incoming_msg_t *out, long *upd return 1; } +static int format_getupdates_url(char *url, size_t url_cap, const char *token, + int timeout_sec, long offset) +{ + int n; + if (!url || url_cap == 0 || !token || token[0] == '\0') + return -1; + /* Default getUpdates limit is 100. A backlog of long messages exceeds the + * 256 KiB read buffer, curl aborts, and the offset never advances, so + * every later poll retries the same batch. The parser only consumes + * result[0]; ask for that one update so a single message always fits. */ + n = snprintf(url, url_cap, "%s%s/%s?timeout=%d&offset=%ld&limit=1", + TG_API_BASE, token, TG_GET_UPDATES, timeout_sec, offset); + if (n < 0 || (size_t)n >= url_cap) + return -1; + return 0; +} + static int tg_poll(channel_incoming_msg_t *out, int timeout_ms) { if (!out || !g_tg.initialized || !g_tg.token) return -1; memset(out, 0, sizeof(*out)); int timeout_sec = (timeout_ms / 1000) > 0 ? (timeout_ms / 1000) : 1; char url[512]; - snprintf(url, sizeof(url), "%s%s/%s?timeout=%d&offset=%ld", - TG_API_BASE, g_tg.token, TG_GET_UPDATES, timeout_sec, g_tg.last_update_id + 1); + if (format_getupdates_url(url, sizeof(url), g_tg.token, timeout_sec, + g_tg.last_update_id + 1) != 0) + return -1; CURL *curl = curl_easy_init(); if (!curl) return -1; tg_curl_harden(curl); @@ -331,6 +350,12 @@ int telegram_parse_update_for_test(const char *json, channel_incoming_msg_t *out return parse_update(json, out, update_id); } +int telegram_format_getupdates_url_for_test(char *url, size_t url_cap, const char *token, + int timeout_sec, long offset) +{ + return format_getupdates_url(url, url_cap, token, timeout_sec, offset); +} + void telegram_set_test_config(const config_t *cfg) { g_tg.cfg = cfg; diff --git a/tests/test_telegram.c b/tests/test_telegram.c index 0a4d611..69d2269 100644 --- a/tests/test_telegram.c +++ b/tests/test_telegram.c @@ -13,6 +13,8 @@ /* Test-only entry points from telegram.c (compiled with -DSHELLCLAW_TEST) */ extern int telegram_parse_update_for_test(const char *json, channel_incoming_msg_t *out, long *update_id); extern void telegram_set_test_config(const config_t *cfg); +extern int telegram_format_getupdates_url_for_test(char *url, size_t url_cap, const char *token, + int timeout_sec, long offset); static int tests_run = 0; static int tests_failed = 0; @@ -191,6 +193,9 @@ static void test_parse_missing_from_field(void) "{\"ok\":true,\"result\":[{" "\"update_id\":400," "\"message\":{\"text\":\"no from\"}" + "},{" + "\"update_id\":401," + "\"message\":{\"from\":{\"id\":42},\"text\":\"kept\"}" "}]}"; config_t *cfg = load_test_config("/tmp/test_tg_nofrom.toml", "42"); MU_ASSERT(cfg != NULL, "load config"); @@ -200,13 +205,27 @@ static void test_parse_missing_from_field(void) memset(&msg, 0, sizeof(msg)); long uid = 0; int r = telegram_parse_update_for_test(json, &msg, &uid); - MU_ASSERT(r == -1, "missing 'from' returns -1"); + /* Skip, but keep this update_id so the next poll can fetch 401. + * Returning -1 left the offset unchanged and pinned the inbox. */ + MU_ASSERT(r == 0, "missing 'from' is skipped"); + MU_ASSERT(uid == 400, "offset stays on the skipped update"); + MU_ASSERT(msg.text == NULL, "skipped update is not delivered"); channel_incoming_msg_clear(&msg); config_free(cfg); remove("/tmp/test_tg_nofrom.toml"); } +static void test_getupdates_url_requests_one_update(void) +{ + char url[512]; + int rc = telegram_format_getupdates_url_for_test(url, sizeof(url), "tok", 1, 99); + MU_ASSERT(rc == 0, "getUpdates url built"); + MU_ASSERT(strstr(url, "/bottok/getUpdates?") != NULL, "token and method in url"); + MU_ASSERT(strstr(url, "offset=99") != NULL, "offset preserved"); + MU_ASSERT(strstr(url, "limit=1") != NULL, "batch capped at one update"); +} + static void test_vtable_fields_set(void) { const channel_t *ch = channel_telegram_get(); @@ -227,6 +246,7 @@ int main(void) MU_RUN(test_parse_photo_with_caption); MU_RUN(test_parse_no_message_key); MU_RUN(test_parse_missing_from_field); + MU_RUN(test_getupdates_url_requests_one_update); MU_RUN(test_vtable_fields_set); printf("%d tests run, %d failed\n", tests_run, tests_failed); return tests_failed ? 1 : 0; From d65c61e9c34d401ffd31a1d99b7eda0dbea39583 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Fri, 16 Oct 2026 00:09:28 -0300 Subject: [PATCH 2/6] fix(channels): deliver long Telegram and Discord replies Telegram treated a finished sendMessage as success, so an HTTP 400 for a reply over 4096 characters still acked cron and the user never saw the text. Discord refused the whole reply above 2000 characters, so the send failed and a due cron job re-ran the agent on every poll. Supersedes #117. --- src/channels/channel.h | 13 ++++ src/channels/channel_common.c | 17 +++++ src/channels/discord.c | 57 +++++++++----- src/channels/telegram.c | 135 ++++++++++++++++++++++++++++------ tests/test_channel.c | 17 +++++ tests/test_telegram.c | 22 +++++- 6 files changed, 221 insertions(+), 40 deletions(-) diff --git a/src/channels/channel.h b/src/channels/channel.h index 73b4cd7..8654d91 100644 --- a/src/channels/channel.h +++ b/src/channels/channel.h @@ -37,6 +37,19 @@ typedef struct channel_incoming_msg { /** Clear and free incoming message fields. Safe to call repeatedly or on zeroed. */ void channel_incoming_msg_clear(channel_incoming_msg_t *msg); +/** + * Byte length of the next outbound piece of @p text. + * The piece is at most @p max_bytes and stops before a multibyte UTF-8 + * character that would not fit, so chat APIs do not see a split code point. + * + * Example: `channel_outbound_chunk_len("hello", 2)` returns 2. + * + * @param text NUL-terminated text. NULL returns 0. + * @param max_bytes Maximum piece size in bytes. 0 returns 0. + * @return Byte length of the next piece, or 0 when @p text is empty. + */ +size_t channel_outbound_chunk_len(const char *text, size_t max_bytes); + /** * Channel vtable: init, poll, send, cleanup. CLI and Telegram implement this. */ diff --git a/src/channels/channel_common.c b/src/channels/channel_common.c index 59b48f0..5a1b68e 100644 --- a/src/channels/channel_common.c +++ b/src/channels/channel_common.c @@ -31,6 +31,23 @@ const channel_t *channel_get_by_name(const char *name) return NULL; } +size_t channel_outbound_chunk_len(const char *text, size_t max_bytes) +{ + size_t len; + size_t n; + + if (!text || max_bytes == 0) + return 0; + len = strlen(text); + if (len <= max_bytes) + return len; + /* Walk back over UTF-8 continuation bytes so the cut sits on a boundary. */ + n = max_bytes; + while (n > 0 && ((unsigned char)text[n] & 0xC0) == 0x80) + n--; + return n > 0 ? n : max_bytes; +} + void channel_incoming_msg_clear(channel_incoming_msg_t *msg) { if (!msg) return; diff --git a/src/channels/discord.c b/src/channels/discord.c index 0b52b97..84f3455 100644 --- a/src/channels/discord.c +++ b/src/channels/discord.c @@ -862,25 +862,13 @@ static int discord_recipient_channel_id(const char *recipient, char *out, size_t return 0; } -static int discord_send(const char *recipient, const char *text, - const channel_attachment_t *attachments, size_t att_count) +static int discord_post_text(const char *channel_id, const char *text, const char *auth_hdr) { - struct discord_ctx *dc = &g_dc; - char channel_id[DISCORD_CHANNEL_ID_CAP]; cJSON *root; char *json_body; char url[DISCORD_URL_CAP]; - char auth_hdr[DISCORD_AUTH_HDR_CAP]; int attempt; - (void)attachments; - if (att_count > 0) - return -1; - if (!dc->token || !recipient || !text || text[0] == '\0') - return -1; - if (strlen(text) > (size_t)DISCORD_MSG_MAX_LEN) - return -1; - if (discord_recipient_channel_id(recipient, channel_id, sizeof(channel_id)) != 0) - return -1; + root = cJSON_CreateObject(); if (!root) return -1; @@ -893,10 +881,6 @@ static int discord_send(const char *recipient, const char *text, free(json_body); return -1; } - if (snprintf(auth_hdr, sizeof(auth_hdr), "Authorization: Bot %s", dc->token) >= (int)sizeof(auth_hdr)) { - free(json_body); - return -1; - } for (attempt = 0; attempt < DISCORD_SEND_MAX_ATTEMPTS; attempt++) { struct discord_hdr_ud hd = { -1.0 }; struct curl_slist *hdrs = NULL; @@ -949,9 +933,46 @@ static int discord_send(const char *recipient, const char *text, free(json_body); return -1; } + free(json_body); return -1; } +static int discord_send(const char *recipient, const char *text, + const channel_attachment_t *attachments, size_t att_count) +{ + struct discord_ctx *dc = &g_dc; + char channel_id[DISCORD_CHANNEL_ID_CAP]; + char auth_hdr[DISCORD_AUTH_HDR_CAP]; + const char *cursor; + + (void)attachments; + if (att_count > 0) + return -1; + if (!dc->token || !recipient || !text || text[0] == '\0') + return -1; + if (discord_recipient_channel_id(recipient, channel_id, sizeof(channel_id)) != 0) + return -1; + if (snprintf(auth_hdr, sizeof(auth_hdr), "Authorization: Bot %s", dc->token) >= + (int)sizeof(auth_hdr)) + return -1; + /* A single REST message is 2000 characters. Longer replies used to fail + * the whole send, so cron kept re-running the agent and the user saw nothing. */ + cursor = text; + while (cursor[0] != '\0') { + char piece[DISCORD_MSG_MAX_LEN + 1]; + size_t n = channel_outbound_chunk_len(cursor, (size_t)DISCORD_MSG_MAX_LEN); + + if (n == 0 || n > (size_t)DISCORD_MSG_MAX_LEN) + return -1; + memcpy(piece, cursor, n); + piece[n] = '\0'; + if (discord_post_text(channel_id, piece, auth_hdr) != 0) + return -1; + cursor += n; + } + return 0; +} + static void discord_cleanup(void) { struct discord_pending *pending; diff --git a/src/channels/telegram.c b/src/channels/telegram.c index c2e4a87..9c9efea 100644 --- a/src/channels/telegram.c +++ b/src/channels/telegram.c @@ -21,6 +21,8 @@ #define POLL_TIMEOUT_SEC 30 #define RESP_BUF_SIZE (256 * 1024) #define RESP_BUF_INIT 4096 +/* Bot API sendMessage text limit. Longer agent replies are sent as several messages. */ +#define TG_TEXT_MAX 4096 struct telegram_ctx { char *token; @@ -280,43 +282,129 @@ static int tg_poll(channel_incoming_msg_t *out, int timeout_ms) return 0; } -static int tg_send(const char *recipient, const char *text, - const channel_attachment_t *attachments, size_t att_count) +/* Curl success is not delivery. sendMessage reports refusal in HTTP status and ok. */ +static int telegram_response_ok(long http_code, const char *body) +{ + cJSON *root; + cJSON *ok_item; + + if (http_code < 200 || http_code >= 300) + return 0; + if (!body || body[0] == '\0') + return 0; + root = cJSON_Parse(body); + if (!root || !cJSON_IsObject(root)) { + if (root) + cJSON_Delete(root); + return 0; + } + ok_item = cJSON_GetObjectItemCaseSensitive(root, "ok"); + if (!cJSON_IsBool(ok_item) || !cJSON_IsTrue(ok_item)) { + cJSON_Delete(root); + return 0; + } + cJSON_Delete(root); + return 1; +} + +static int telegram_perform_post(CURL *curl, const char *url, const char *postfields) +{ + tg_buf_t resp; + long http_code = 0; + int ret = -1; + + resp.buf = malloc(RESP_BUF_INIT); + if (!resp.buf) + return -1; + resp.len = 0; + resp.cap = RESP_BUF_INIT; + resp.buf[0] = '\0'; + curl_easy_setopt(curl, CURLOPT_URL, url); + curl_easy_setopt(curl, CURLOPT_POSTFIELDS, postfields); + curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_cb); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, &resp); + if (tg_curl_perform(curl, "sendMessage") == 0) { + curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code); + if (telegram_response_ok(http_code, resp.buf)) + ret = 0; + else + fprintf(stderr, "shellclaw: telegram: sendMessage rejected HTTP %ld\n", + http_code); + } + free(resp.buf); + return ret; +} + +static int telegram_post_chunk(const char *chat_id, const char *text) { - (void)attachments; - (void)att_count; - if (!g_tg.initialized || !g_tg.token || !recipient || !text) return -1; - const char *chat_id = strchr(recipient, ':'); - if (chat_id) chat_id++; - else chat_id = recipient; char url[512]; - snprintf(url, sizeof(url), "%s%s/%s", TG_API_BASE, g_tg.token, TG_SEND_MSG); - CURL *curl = curl_easy_init(); - if (!curl) return -1; + CURL *curl; + char *escaped; + char *postfields; + int ret; + size_t need; + + curl = curl_easy_init(); + if (!curl) + return -1; tg_curl_harden(curl); curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1L); curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 10L); curl_easy_setopt(curl, CURLOPT_TIMEOUT, 30L); - char *escaped = curl_easy_escape(curl, text, (int)strlen(text)); + snprintf(url, sizeof(url), "%s%s/%s", TG_API_BASE, g_tg.token, TG_SEND_MSG); + escaped = curl_easy_escape(curl, text, (int)strlen(text)); if (!escaped) { curl_easy_cleanup(curl); return -1; } - size_t need = strlen(chat_id) + strlen(escaped) + 32; - char *postfields = malloc(need); - int ret = -1; - if (postfields) { - snprintf(postfields, need, "chat_id=%s&text=%s", chat_id, escaped); - curl_easy_setopt(curl, CURLOPT_URL, url); - curl_easy_setopt(curl, CURLOPT_POSTFIELDS, postfields); - ret = (tg_curl_perform(curl, "sendMessage") == 0) ? 0 : -1; - free(postfields); + need = strlen(chat_id) + strlen(escaped) + 32; + postfields = malloc(need); + if (!postfields) { + curl_free(escaped); + curl_easy_cleanup(curl); + return -1; } + snprintf(postfields, need, "chat_id=%s&text=%s", chat_id, escaped); + ret = telegram_perform_post(curl, url, postfields); + free(postfields); curl_free(escaped); curl_easy_cleanup(curl); return ret; } +static int tg_send(const char *recipient, const char *text, + const channel_attachment_t *attachments, size_t att_count) +{ + const char *chat_id; + const char *cursor; + + (void)attachments; + (void)att_count; + if (!g_tg.initialized || !g_tg.token || !recipient || !text || text[0] == '\0') + return -1; + chat_id = strchr(recipient, ':'); + if (chat_id) + chat_id++; + else + chat_id = recipient; + if (chat_id[0] == '\0') + return -1; + cursor = text; + while (cursor[0] != '\0') { + char piece[TG_TEXT_MAX + 1]; + size_t n = channel_outbound_chunk_len(cursor, TG_TEXT_MAX); + + if (n == 0 || n > TG_TEXT_MAX) + return -1; + memcpy(piece, cursor, n); + piece[n] = '\0'; + if (telegram_post_chunk(chat_id, piece) != 0) + return -1; + cursor += n; + } + return 0; +} + static void tg_cleanup(void) { free(g_tg.token); @@ -360,4 +448,9 @@ void telegram_set_test_config(const config_t *cfg) { g_tg.cfg = cfg; } + +int telegram_response_ok_for_test(long http_code, const char *body) +{ + return telegram_response_ok(http_code, body); +} #endif diff --git a/tests/test_channel.c b/tests/test_channel.c index a6a7493..79d7d86 100644 --- a/tests/test_channel.c +++ b/tests/test_channel.c @@ -74,6 +74,22 @@ static void test_stub_cleanup(void) MU_ASSERT(1, "stub cleanup does not crash"); } +static void test_outbound_chunk_len(void) +{ + /* U+20AC EURO SIGN is the three-byte sequence e2 82 ac. */ + const char euro[] = "x\xE2\x82\xAC" "y"; + size_t n; + + MU_ASSERT(channel_outbound_chunk_len(NULL, 10) == 0, "NULL text is empty"); + MU_ASSERT(channel_outbound_chunk_len("", 10) == 0, "empty text is empty"); + MU_ASSERT(channel_outbound_chunk_len("hi", 0) == 0, "zero cap is empty"); + MU_ASSERT(channel_outbound_chunk_len("hello", 5) == 5, "exact fit stays one piece"); + MU_ASSERT(channel_outbound_chunk_len("hello", 2) == 2, "ascii splits on the cap"); + n = channel_outbound_chunk_len(euro, 2); + MU_ASSERT(n == 1, "multibyte character is not split"); + MU_ASSERT(channel_outbound_chunk_len(euro + n, 3) == 3, "next piece is the whole euro sign"); +} + int main(void) { MU_RUN(test_stub_channel_exists); @@ -82,6 +98,7 @@ int main(void) MU_RUN(test_incoming_msg_clear_safe); MU_RUN(test_stub_send_succeeds); MU_RUN(test_stub_cleanup); + MU_RUN(test_outbound_chunk_len); printf("%d tests run, %d failed\n", tests_run, tests_failed); return tests_failed ? 1 : 0; } diff --git a/tests/test_telegram.c b/tests/test_telegram.c index 69d2269..c7a89d5 100644 --- a/tests/test_telegram.c +++ b/tests/test_telegram.c @@ -15,6 +15,7 @@ extern int telegram_parse_update_for_test(const char *json, channel_incoming_msg extern void telegram_set_test_config(const config_t *cfg); extern int telegram_format_getupdates_url_for_test(char *url, size_t url_cap, const char *token, int timeout_sec, long offset); +extern int telegram_response_ok_for_test(long http_code, const char *body); static int tests_run = 0; static int tests_failed = 0; @@ -226,6 +227,24 @@ static void test_getupdates_url_requests_one_update(void) MU_ASSERT(strstr(url, "limit=1") != NULL, "batch capped at one update"); } +static void test_send_response_requires_ok(void) +{ + const char *too_long = "{\"ok\":false,\"error_code\":400," + "\"description\":\"Bad Request: message is too long\"}"; + const char *accepted = "{\"ok\":true,\"result\":{\"message_id\":1}}"; + + MU_ASSERT(telegram_response_ok_for_test(400, too_long) == 0, + "HTTP 400 message-too-long is not delivery"); + MU_ASSERT(telegram_response_ok_for_test(200, "{\"ok\":false}") == 0, + "HTTP 200 with ok false is not delivery"); + MU_ASSERT(telegram_response_ok_for_test(200, "") == 0, + "empty body is not delivery"); + MU_ASSERT(telegram_response_ok_for_test(429, "{\"ok\":false}") == 0, + "HTTP 429 is not delivery"); + MU_ASSERT(telegram_response_ok_for_test(200, accepted) == 1, + "HTTP 200 with ok true is delivery"); +} + static void test_vtable_fields_set(void) { const channel_t *ch = channel_telegram_get(); @@ -246,7 +265,8 @@ int main(void) MU_RUN(test_parse_photo_with_caption); MU_RUN(test_parse_no_message_key); MU_RUN(test_parse_missing_from_field); - MU_RUN(test_getupdates_url_requests_one_update); +MU_RUN(test_getupdates_url_requests_one_update); + MU_RUN(test_send_response_requires_ok); MU_RUN(test_vtable_fields_set); printf("%d tests run, %d failed\n", tests_run, tests_failed); return tests_failed ? 1 : 0; From 2d8721647fcdf04e4a95d5e47682482cf3b50407 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Sat, 17 Oct 2026 02:22:17 -0300 Subject: [PATCH 3/6] refactor(channels): extract shared chunked send helper tg_send and discord_send carried a copy-pasted piece[] loop (bounded copy, rune-safe cut, POST, advance). Move it once into channel_common.c as channel_send_chunked(text, max_bytes, post, ctx, fail) with the NUL-terminated piece buffer, so bounds-checking is a single concern and the piece copy lives in one place. Both channels now post through a small callback; this also shrinks discord.c, which is past the 1000-line guideline. channel_chunk_fail_t reports the failing chunk index and byte offset so a caller can log partial delivery, and the header documents two_limits contract notes: the chunk_len fallback when the first rune is wider than max_bytes, and that limits are BYTES even where Telegram counts characters (conservative so nobody 'fixes' it to code points later). Cover the helper network-free through the post callback: concatenated pieces reproduce the input exactly (multibyte, 4-byte cap), single-piece fast path, a callback failing on chunk 2 reports index 2 / offset 8, and invalid-argument and empty-text contracts. Extend the chunk_len tests with a 4-byte code point at the split edge. Refs: #123 --- src/channels/channel.h | 46 +++++++++++ src/channels/channel_common.c | 54 ++++++++++++- src/channels/discord.c | 41 +++++----- src/channels/telegram.c | 29 ++++--- tests/test_channel.c | 143 +++++++++++++++++++++++++++++++++- 5 files changed, 274 insertions(+), 39 deletions(-) diff --git a/src/channels/channel.h b/src/channels/channel.h index 8654d91..cb64cb8 100644 --- a/src/channels/channel.h +++ b/src/channels/channel.h @@ -41,6 +41,9 @@ void channel_incoming_msg_clear(channel_incoming_msg_t *msg); * Byte length of the next outbound piece of @p text. * The piece is at most @p max_bytes and stops before a multibyte UTF-8 * character that would not fit, so chat APIs do not see a split code point. + * Fallback: when the first rune is wider than @p max_bytes, the piece is + * @p max_bytes bytes (that rune is split); unreachable with the 2000/4096 + * channel caps, documented so the finite cut is not mistaken for a bug. * * Example: `channel_outbound_chunk_len("hello", 2)` returns 2. * @@ -50,6 +53,49 @@ void channel_incoming_msg_clear(channel_incoming_msg_t *msg); */ size_t channel_outbound_chunk_len(const char *text, size_t max_bytes); +/** + * Post one chunk of a channel_send_chunked() send. NUL-terminated piece, + * never empty and never longer than the max_bytes of the driving call. + * @param piece NUL-terminated text piece (valid until the callback returns). + * @param ctx Caller context passed through unchanged. May be NULL. + * @return 0 when the platform accepted the piece, non-zero on failure. + */ +typedef int (*channel_chunk_post_fn)(const char *piece, void *ctx); + +/** Position where a chunked send stopped; zeroed by the call unless it failed. */ +typedef struct channel_chunk_fail { + size_t index; /**< Zero-based index of the chunk that failed to post. */ + size_t offset; /**< Byte offset of the failed chunk inside the sent text. */ +} channel_chunk_fail_t; + +/** + * Deliver @p text via @p post in chunks of at most @p max_bytes, cut on UTF-8 + * rune boundaries via channel_outbound_chunk_len(). Each piece is copied into + * one internal buffer and NUL-terminated, so plain C-string platform posts + * (Telegram sendMessage query, Discord JSON `content`) work unchanged. + * + * Limits are BYTES even where a platform counts characters (Telegram's 4096 + * is characters): conservative by design, so CJK/emoji-heavy replies split + * earlier than strictly required. Do not "fix" this to count code points. + * + * Example: `channel_send_chunked(text, 2000, post_fn, &ctx, &fail)` posts up + * to 2000-byte pieces until done; when `post_fn` returns non-zero the send + * stops and `fail` reports the failing chunk index and byte offset. + * + * @param text NUL-terminated text. NULL returns -1; empty posts nothing. + * @param max_bytes Maximum chunk size in bytes. 0 returns -1. + * @param post Callback that delivers one piece; non-zero fails the send. + * @param ctx Caller context passed through to @p post. May be NULL. + * @param fail Optional out: chunk index and byte offset where the send + * stopped. Zeroed when the call does not fail that way. + * @return 0 when every chunk was posted, -1 on invalid arguments, OOM, or + * a failed @p post (with @p fail, when given, pointing at the chunk + * that failed). + */ +int channel_send_chunked(const char *text, size_t max_bytes, + channel_chunk_post_fn post, void *ctx, + channel_chunk_fail_t *fail); + /** * Channel vtable: init, poll, send, cleanup. CLI and Telegram implement this. */ diff --git a/src/channels/channel_common.c b/src/channels/channel_common.c index 5a1b68e..bb362a2 100644 --- a/src/channels/channel_common.c +++ b/src/channels/channel_common.c @@ -1,6 +1,6 @@ /** * @file channel_common.c - * @brief Shared channel utilities: channel_incoming_msg_clear, channel registry. + * @brief Shared channel utilities: chunked send, incoming msg clear, registry. */ #include "channels/channel.h" @@ -48,6 +48,58 @@ size_t channel_outbound_chunk_len(const char *text, size_t max_bytes) return n > 0 ? n : max_bytes; } +int channel_send_chunked(const char *text, size_t max_bytes, + channel_chunk_post_fn post, void *ctx, + channel_chunk_fail_t *fail) +{ + char *piece; + const char *cursor; + size_t index; + + if (fail) { + fail->index = 0; + fail->offset = 0; + } + if (!text || max_bytes == 0 || !post) + return -1; + /* One reused, always-NUL-terminated piece buffer: platform posts take + * plain C strings, so the chunk copy lives here instead of per caller. */ + piece = malloc(max_bytes + 1); + if (!piece) + return -1; + cursor = text; + index = 0; + while (cursor[0] != '\0') { + size_t n = channel_outbound_chunk_len(cursor, max_bytes); + + if (n == 0 || n > max_bytes) { + /* Fail-safe only: chunk_len guarantees 0 < n <= max_bytes + * for non-empty cursor, so a future bounds bug stops the + * send instead of walking past the piece buffer. */ + if (fail) { + fail->index = index; + fail->offset = (size_t)(cursor - text); + } + free(piece); + return -1; + } + memcpy(piece, cursor, n); + piece[n] = '\0'; + if (post(piece, ctx) != 0) { + if (fail) { + fail->index = index; + fail->offset = (size_t)(cursor - text); + } + free(piece); + return -1; + } + cursor += n; + index++; + } + free(piece); + return 0; +} + void channel_incoming_msg_clear(channel_incoming_msg_t *msg) { if (!msg) return; diff --git a/src/channels/discord.c b/src/channels/discord.c index 84f3455..24f15c7 100644 --- a/src/channels/discord.c +++ b/src/channels/discord.c @@ -937,40 +937,37 @@ static int discord_post_text(const char *channel_id, const char *text, const cha return -1; } +struct discord_chunk_ctx { + char channel_id[DISCORD_CHANNEL_ID_CAP]; + char auth_hdr[DISCORD_AUTH_HDR_CAP]; +}; + +static int discord_post_piece(const char *piece, void *userdata) +{ + const struct discord_chunk_ctx *ctx = (const struct discord_chunk_ctx *)userdata; + + return discord_post_text(ctx->channel_id, piece, ctx->auth_hdr); +} + static int discord_send(const char *recipient, const char *text, const channel_attachment_t *attachments, size_t att_count) { + struct discord_chunk_ctx ctx; struct discord_ctx *dc = &g_dc; - char channel_id[DISCORD_CHANNEL_ID_CAP]; - char auth_hdr[DISCORD_AUTH_HDR_CAP]; - const char *cursor; (void)attachments; if (att_count > 0) return -1; if (!dc->token || !recipient || !text || text[0] == '\0') return -1; - if (discord_recipient_channel_id(recipient, channel_id, sizeof(channel_id)) != 0) + /* 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; - if (snprintf(auth_hdr, sizeof(auth_hdr), "Authorization: Bot %s", dc->token) >= - (int)sizeof(auth_hdr)) + if (snprintf(ctx.auth_hdr, sizeof(ctx.auth_hdr), "Authorization: Bot %s", dc->token) >= + (int)sizeof(ctx.auth_hdr)) return -1; - /* A single REST message is 2000 characters. Longer replies used to fail - * the whole send, so cron kept re-running the agent and the user saw nothing. */ - cursor = text; - while (cursor[0] != '\0') { - char piece[DISCORD_MSG_MAX_LEN + 1]; - size_t n = channel_outbound_chunk_len(cursor, (size_t)DISCORD_MSG_MAX_LEN); - - if (n == 0 || n > (size_t)DISCORD_MSG_MAX_LEN) - return -1; - memcpy(piece, cursor, n); - piece[n] = '\0'; - if (discord_post_text(channel_id, piece, auth_hdr) != 0) - return -1; - cursor += n; - } - return 0; + return channel_send_chunked(text, (size_t)DISCORD_MSG_MAX_LEN, discord_post_piece, &ctx, NULL); } static void discord_cleanup(void) diff --git a/src/channels/telegram.c b/src/channels/telegram.c index 9c9efea..470ea11 100644 --- a/src/channels/telegram.c +++ b/src/channels/telegram.c @@ -372,11 +372,22 @@ static int telegram_post_chunk(const char *chat_id, const char *text) return ret; } +typedef struct { + const char *chat_id; +} tg_chunk_ctx_t; + +static int tg_post_piece(const char *piece, void *userdata) +{ + const tg_chunk_ctx_t *ctx = (const tg_chunk_ctx_t *)userdata; + + return telegram_post_chunk(ctx->chat_id, piece); +} + static int tg_send(const char *recipient, const char *text, const channel_attachment_t *attachments, size_t att_count) { + tg_chunk_ctx_t ctx; const char *chat_id; - const char *cursor; (void)attachments; (void)att_count; @@ -389,20 +400,8 @@ static int tg_send(const char *recipient, const char *text, chat_id = recipient; if (chat_id[0] == '\0') return -1; - cursor = text; - while (cursor[0] != '\0') { - char piece[TG_TEXT_MAX + 1]; - size_t n = channel_outbound_chunk_len(cursor, TG_TEXT_MAX); - - if (n == 0 || n > TG_TEXT_MAX) - return -1; - memcpy(piece, cursor, n); - piece[n] = '\0'; - if (telegram_post_chunk(chat_id, piece) != 0) - return -1; - cursor += n; - } - return 0; + ctx.chat_id = chat_id; + return channel_send_chunked(text, TG_TEXT_MAX, tg_post_piece, &ctx, NULL); } static void tg_cleanup(void) diff --git a/tests/test_channel.c b/tests/test_channel.c index 79d7d86..4d656ff 100644 --- a/tests/test_channel.c +++ b/tests/test_channel.c @@ -1,10 +1,12 @@ /** * @file test_channel.c - * @brief Unit tests for channel_t vtable and stub channel. + * @brief Unit tests for channel_t vtable, stub channel, chunked send helper. */ +#define _POSIX_C_SOURCE 200809L #include "channels/channel.h" #include +#include #include static int tests_run = 0; @@ -90,6 +92,141 @@ static void test_outbound_chunk_len(void) MU_ASSERT(channel_outbound_chunk_len(euro + n, 3) == 3, "next piece is the whole euro sign"); } +static void test_outbound_chunk_len_4byte_rune(void) +{ + /* U+1F600 GRINNING FACE is the four-byte sequence f0 9f 98 80. */ + const char grin[] = "xx\xF0\x9F\x98\x80" "yy"; + size_t n = channel_outbound_chunk_len(grin, 3); + + MU_ASSERT(n == 2, "4-byte code point at the split edge is not split"); + n = channel_outbound_chunk_len(grin + n, 4); + MU_ASSERT(n == 4, "next piece is the whole 4-byte code point"); +} + +/* --- channel_send_chunked --- */ + +#define CHUNK_TEST_MAX_PIECES 64 + +typedef struct { + char *pieces[CHUNK_TEST_MAX_PIECES]; + size_t count; +} chunk_recorder_t; + +static int record_piece(const char *piece, void *userdata) +{ + chunk_recorder_t *rec = (chunk_recorder_t *)userdata; + + if (rec->count >= CHUNK_TEST_MAX_PIECES) + return -1; + rec->pieces[rec->count] = strdup(piece); + if (!rec->pieces[rec->count]) + return -1; + rec->count++; + return 0; +} + +static void recorder_clear(chunk_recorder_t *rec) +{ + size_t i; + + for (i = 0; i < rec->count; i++) + free(rec->pieces[i]); + rec->count = 0; +} + +static void test_send_chunked_round_trip(void) +{ + /* ASCII, a 3-byte euro sign, and a 4-byte grinning face. */ + const char *input = "hi \xE2\x82\xAC\xF0\x9F\x98\x80 tail text!"; + char joined[256]; + size_t joined_len = 0; + chunk_recorder_t rec; + channel_chunk_fail_t fail; + size_t i; + + fail.index = 99; + fail.offset = 99; + rec.count = 0; + MU_ASSERT(channel_send_chunked(input, 4, record_piece, &rec, &fail) == 0, + "chunked send over multibyte text succeeds"); + MU_ASSERT(rec.count >= 2, "text longer than max_bytes is split"); + for (i = 0; i < rec.count; i++) { + size_t plen; + + MU_ASSERT(rec.pieces[i] != NULL, "recorded piece is non-NULL"); + MU_ASSERT(rec.pieces[i][0] != '\0', "no piece is empty"); + plen = strlen(rec.pieces[i]); + MU_ASSERT(joined_len + plen < sizeof(joined), "joined buffer fits"); + memcpy(joined + joined_len, rec.pieces[i], plen); + joined_len += plen; + } + joined[joined_len] = '\0'; + MU_ASSERT(strcmp(joined, input) == 0, "pieces concatenate back to the input"); + MU_ASSERT(fail.index == 0 && fail.offset == 0, "fail stays zeroed on success"); + recorder_clear(&rec); + + /* Length within the cap is delivered as a single piece. */ + MU_ASSERT(channel_send_chunked(input, 128, record_piece, &rec, &fail) == 0, + "text within max_bytes sends as one piece"); + MU_ASSERT(rec.count == 1, "single piece for text within max_bytes"); + MU_ASSERT(rec.pieces[0] && strcmp(rec.pieces[0], input) == 0, + "single piece equals the input"); + recorder_clear(&rec); +} + +typedef struct { + size_t calls; +} chunk_counter_t; + +static int post_fail_on_third(const char *piece, void *userdata) +{ + chunk_counter_t *counter = (chunk_counter_t *)userdata; + + (void)piece; + counter->calls++; + return counter->calls == 3 ? -1 : 0; +} + +static void test_send_chunked_reports_failing_chunk(void) +{ + channel_chunk_fail_t fail; + chunk_counter_t counter; + int r; + + fail.index = 99; + fail.offset = 99; + counter.calls = 0; + /* 12 bytes / max 4: chunks "abcd" (0), "efgh" (4), "ijkl" (8). */ + r = channel_send_chunked("abcdefghijkl", 4, post_fail_on_third, &counter, &fail); + MU_ASSERT(r != 0, "send fails when a chunk post fails"); + MU_ASSERT(counter.calls == 3, "chunks before the failure are posted"); + MU_ASSERT(fail.index == 2, "failing chunk index is reported"); + MU_ASSERT(fail.offset == 8, "failing chunk byte offset is reported"); +} + +static void test_send_chunked_contract_edges(void) +{ + channel_chunk_fail_t fail; + chunk_recorder_t rec; + + fail.index = 99; + fail.offset = 99; + rec.count = 0; + MU_ASSERT(channel_send_chunked(NULL, 10, record_piece, &rec, &fail) != 0, + "NULL text fails"); + MU_ASSERT(channel_send_chunked("hi", 0, record_piece, &rec, &fail) != 0, + "zero max_bytes fails"); + MU_ASSERT(channel_send_chunked("hi", 10, NULL, &rec, &fail) != 0, + "NULL post callback fails"); + MU_ASSERT(rec.count == 0, "invalid arguments post no pieces"); + MU_ASSERT(fail.index == 0 && fail.offset == 0, + "fail stays zeroed without a post failure"); + /* Empty text is a no-op success: there is no chunk to post. */ + MU_ASSERT(channel_send_chunked("", 10, record_piece, &rec, &fail) == 0, + "empty text is a no-op success"); + MU_ASSERT(rec.count == 0, "empty text posts no pieces"); +} + int main(void) { MU_RUN(test_stub_channel_exists); @@ -99,6 +236,10 @@ int main(void) MU_RUN(test_stub_send_succeeds); MU_RUN(test_stub_cleanup); MU_RUN(test_outbound_chunk_len); + MU_RUN(test_outbound_chunk_len_4byte_rune); + MU_RUN(test_send_chunked_round_trip); + MU_RUN(test_send_chunked_reports_failing_chunk); + MU_RUN(test_send_chunked_contract_edges); printf("%d tests run, %d failed\n", tests_run, tests_failed); return tests_failed ? 1 : 0; } From 82ea234e01db915b5f7a0dc3014b2556042c632e Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Sat, 17 Oct 2026 02:22:30 -0300 Subject: [PATCH 4/6] fix(channels): log failing chunk index and offset on partial send When a chunk POST fails mid-reply the send returns non-zero and dispatch skips the cron ack, so the job re-runs and any already -delivered chunks are delivered again (at-least-once). Log the failing chunk index and its byte offset in both channels so the duplicate boundary in a re-run is traceable from the daemon log. Refs: #123 --- src/channels/discord.c | 13 ++++++++++++- src/channels/telegram.c | 12 +++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/channels/discord.c b/src/channels/discord.c index 24f15c7..c3c5aa1 100644 --- a/src/channels/discord.c +++ b/src/channels/discord.c @@ -954,6 +954,7 @@ static int discord_send(const char *recipient, const char *text, { struct discord_chunk_ctx ctx; struct discord_ctx *dc = &g_dc; + channel_chunk_fail_t fail; (void)attachments; if (att_count > 0) @@ -967,7 +968,17 @@ static int discord_send(const char *recipient, const char *text, if (snprintf(ctx.auth_hdr, sizeof(ctx.auth_hdr), "Authorization: Bot %s", dc->token) >= (int)sizeof(ctx.auth_hdr)) return -1; - return channel_send_chunked(text, (size_t)DISCORD_MSG_MAX_LEN, discord_post_piece, &ctx, NULL); + if (channel_send_chunked(text, (size_t)DISCORD_MSG_MAX_LEN, discord_post_piece, &ctx, + &fail) != 0) { + /* At-least-once delivery: dispatch skips the cron ack when send + * fails, so a re-run repeats already-delivered chunks. Log where + * the reply stopped so the duplicate boundary is traceable. */ + fprintf(stderr, + "shellclaw: discord: send failed at chunk %zu (byte offset %zu)\n", + fail.index, fail.offset); + return -1; + } + return 0; } static void discord_cleanup(void) diff --git a/src/channels/telegram.c b/src/channels/telegram.c index 470ea11..bce935b 100644 --- a/src/channels/telegram.c +++ b/src/channels/telegram.c @@ -387,6 +387,7 @@ static int tg_send(const char *recipient, const char *text, const channel_attachment_t *attachments, size_t att_count) { tg_chunk_ctx_t ctx; + channel_chunk_fail_t fail; const char *chat_id; (void)attachments; @@ -401,7 +402,16 @@ static int tg_send(const char *recipient, const char *text, if (chat_id[0] == '\0') return -1; ctx.chat_id = chat_id; - return channel_send_chunked(text, TG_TEXT_MAX, tg_post_piece, &ctx, NULL); + if (channel_send_chunked(text, TG_TEXT_MAX, tg_post_piece, &ctx, &fail) != 0) { + /* At-least-once delivery: dispatch skips the cron ack when send + * fails, so a re-run repeats already-delivered chunks. Log where + * the reply stopped so the duplicate boundary is traceable. */ + fprintf(stderr, + "shellclaw: telegram: sendMessage failed at chunk %zu (byte offset %zu)\n", + fail.index, fail.offset); + return -1; + } + return 0; } static void tg_cleanup(void) From 14fffcfc654fed246949098338d12031bc37104e Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Sat, 17 Oct 2026 02:22:47 -0300 Subject: [PATCH 5/6] fix(channels): pace consecutive Telegram sendMessage chunks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Telegram allows about one message per second per chat; a 32 KiB cron or chat reply is up to 8 back-to-back sendMessage calls at the 4096 cap, and a 429 aborts the rest of the reply (which then re-runs, see partial-delivery logging). Pause 500ms between consecutive chunk POSTs — never before the first — to stay under the chat rate. Full 429 handling (parameters.retry_after parse + wait + retry) is a deferred follow-up; this is the deliberate lighter alternative. Refs: #123 --- src/channels/telegram.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/src/channels/telegram.c b/src/channels/telegram.c index bce935b..4c1685a 100644 --- a/src/channels/telegram.c +++ b/src/channels/telegram.c @@ -8,10 +8,12 @@ #include "core/config.h" #include "cJSON.h" #include +#include #include #include #include #include +#include #define TG_API_BASE "https://api.telegram.org/bot" #define TG_GET_UPDATES "getUpdates" @@ -23,6 +25,10 @@ #define RESP_BUF_INIT 4096 /* Bot API sendMessage text limit. Longer agent replies are sent as several messages. */ #define TG_TEXT_MAX 4096 +/* Telegram allows about 1 message per second per chat. dispatch caps replies at + * 32 KiB, so a long reply is up to 8 back-to-back sendMessage calls; pace them + * so the chat does not hit 429 mid-reply. Full retry_after parsing is deferred. */ +#define TG_CHUNK_PAUSE_MS 500 struct telegram_ctx { char *token; @@ -374,12 +380,32 @@ static int telegram_post_chunk(const char *chat_id, const char *text) typedef struct { const char *chat_id; + int posted_before; } tg_chunk_ctx_t; -static int tg_post_piece(const char *piece, void *userdata) +static void tg_sleep_ms(int ms) { - const tg_chunk_ctx_t *ctx = (const tg_chunk_ctx_t *)userdata; + struct timespec ts; + + if (ms <= 0) + return; + ts.tv_sec = (time_t)(ms / 1000); + ts.tv_nsec = (long)(ms % 1000) * 1000000L; + while (nanosleep(&ts, &ts) == -1 && errno == EINTR) + ; +} +static int tg_post_piece(const char *piece, void *userdata) +{ + tg_chunk_ctx_t *ctx = (tg_chunk_ctx_t *)userdata; + + /* Pause between consecutive chunks only (never before the first): + * the chat rate is ~1 msg/s, so paced posts avoid a 429 aborting the + * rest of the reply. Actually parsing parameters.retry_after from a + * 429 body and sleeping exactly that long is deferred follow-up. */ + if (ctx->posted_before) + tg_sleep_ms(TG_CHUNK_PAUSE_MS); + ctx->posted_before = 1; return telegram_post_chunk(ctx->chat_id, piece); } @@ -402,6 +428,7 @@ static int tg_send(const char *recipient, const char *text, if (chat_id[0] == '\0') return -1; ctx.chat_id = chat_id; + ctx.posted_before = 0; if (channel_send_chunked(text, TG_TEXT_MAX, tg_post_piece, &ctx, &fail) != 0) { /* At-least-once delivery: dispatch skips the cron ack when send * fails, so a re-run repeats already-delivered chunks. Log where From 592b5ce692cd4885e34856fc3902e4e85ce356ff Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Sat, 17 Oct 2026 02:22:57 -0300 Subject: [PATCH 6/6] docs(changelog): record consolidated channel delivery fixes The three fixes carried by PR #123 are user-visible delivery fixes: getUpdates batch resilience (a bad batch no longer pins the offset and kills polling), sendMessage delivery actually verified (HTTP 400 is not success), and long replies chunked for Telegram/Discord within platform limits. Record them under [Unreleased] Fixed, matching the existing bullet style. Refs: #123 --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ce4ee7..070da93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ All notable changes to ShellClaw are documented here. Format follows [Keep a Cha ## [Unreleased] ### Fixed +- Telegram `getUpdates` requests a single update per poll and skips a message without a numeric sender id, so a backlog larger than the 256 KiB read buffer (or a channel/anonymous post) no longer pins the poll offset and stalls the inbox. +- Telegram `sendMessage` delivery is verified from the HTTP status code and the `ok` body field, so an API refusal (HTTP 400/429) is no longer reported as a successful send and acknowledged as delivered. +- Long Telegram and Discord replies are chunked to the platform limits (4096/2000 bytes, UTF-8 boundary-safe) through the shared `channel_send_chunked` helper, so a reply over the cap is delivered as several messages instead of failing the whole send; chunk posts are paced for the ~1 msg/s Telegram chat rate, and a failed mid-reply post logs its chunk index and byte offset. - Cron re-offer waits the full timeout when a due id could not be stored in the 16-slot table, and an ack frees that slot so the next due job can be delivered and then backed off. - Landlock device grants stay `/dev/null`, `/dev/zero`, `/dev/urandom`, and `/dev/tty` without `IOCTL_DEV`. A failed `landlock_add_rule` fails the ruleset closed. - Dashboard config save and JSON patch validation use a unique temp plus `fsync`, and the live gateway config pointer is published once under `agent_lock`.