fix(telegram): retry as plain text on Markdown parse failure

[chaodu-agent]

Summary

Fixes #871

The Telegram adapter silently dropped messages when Telegram rejected Markdown formatting (common with Chinese text, URLs, or code containing unescaped _, *, [, ]).

Changes

• Check the Telegram API response body (ok field) instead of discarding it with let _ =
• On Markdown parse failure, retry the same message without parse_mode (plain text)
• Log a warning when fallback is triggered for observability

Testing

• Verified syntax correctness (no linker available in this env for full compile)
• Logic follows the exact pattern suggested in the issue

[chaodu-agent]

CHANGES REQUESTED ⚠️ — Fallback retry should be scoped to Markdown parse errors only.

What This PR Does

Fixes the Telegram adapter silently discarding messages when Telegram rejects Markdown formatting (unescaped _, *, [, ] — common in Chinese text and URLs). Now retries as plain text on failure.

How It Works

Instead of let _ = (fire-and-forget), the response body is checked for "ok": true. On Markdown rejection, the same message is resent without parse_mode, guaranteeing delivery.

Findings

#	Severity	Finding	Location	Reviewer
1	🟡	Retry triggers on any non-ok response (429 rate limit, 401 auth, 403 forbidden), not just Markdown parse errors. Could duplicate messages or worsen rate limiting. Restrict to description containing "parse" or "entities".	telegram.rs:349	擺渡法師, 覺渡法師, 超渡法師
2	🟡	Plain-text retry uses let _ = ...map_err(...) — only catches network errors, still ignores Telegram API errors on the retry. Should check retry response body too.	telegram.rs:356	覺渡法師, 超渡法師
3	🟡	body["description"] in warn! prints Null if field is missing. Use .as_str().unwrap_or("unknown").	telegram.rs:351	超渡法師
4	🟢	Single-retry design prevents infinite loops. Good.	—	覺渡法師
5	🟢	Uses &reply.content.text (reference) to avoid unnecessary string clone.	—	覺渡法師
6	🟢	Minimal +26/-4 diff, consistent with existing create_topic handler pattern.	—	超渡法師

Finding Details

🟡 F1: Overly broad retry condition (consensus from all reviewers)

Current code retries on any ok: false response. This means rate limits (429), auth errors (401), bot-blocked (403) all trigger a redundant retry that will also fail — potentially worsening rate limiting.

Suggested fix: Only retry when the error is specifically a Markdown parse failure:

let description = body["description"].as_str().unwrap_or("");
if description.contains("parse") || description.contains("entities") {
    // retry as plain text
} else {
    error!("telegram send failed: {}", description);
}

🟡 F2: Retry response not checked

The plain-text retry discards the Telegram API response. If the retry also fails (e.g., chat_id invalid), there is no log entry.

Suggested fix: Check retry response body the same way:

match retry_resp {
    Ok(r) => {
        let body: serde_json::Value = r.json().await.unwrap_or_default();
        if body["ok"].as_bool() != Some(true) {
            error!("telegram plain-text retry failed: {}", body["description"].as_str().unwrap_or("unknown"));
        }
    }
    Err(e) => error!("telegram plain-text send error: {e}"),
}

🟡 F3: Null description in warn log

Minor — body["description"] with {} format prints Null when the field is absent. Use .as_str().unwrap_or("unknown error").

Baseline Check

• PR opened: 2026-05-31
• Issue: bug: Telegram sendMessage silently fails on Markdown parse errors #871 (Telegram sendMessage silently fails on Markdown parse errors)
• Main already has: fire-and-forget let _ = pattern
• Net-new value: Response checking + plain-text fallback retry

CI Status

• gateway job: pending
• changes job: ✅ pass

---

Core approach is correct. F1 is the key issue — without scoping the retry to parse errors, this could cause unintended side effects under rate limiting or auth failures.

1️⃣ 請 contributor 修正 F1/F2/F3 後再 review
2️⃣ 我自己來 fix，push 後讓法師團隊 review 直到完全修正
3️⃣ Approve as-is (accept the risk)

[chaodu-agent]

$OUTDATED\n\nThis consolidated review has been superseded by the latest review outcome and should be ignored.

[shaun-agent]

OpenAB PR Screening

This is auto-generated by the OpenAB project-screening flow for context collection and reviewer handoff.
Click 👍 if you find this useful. Human review will be done within 24 hours. We appreciate your support and contribution 🙏

• Title: fix(telegram): retry as plain text on Markdown parse failure
• Source: fix(telegram): retry as plain text on Markdown parse failure #957
• Status: moved to PR-Screening
• Generated at: 2026-05-31T18:02:39.359Z
• Discord thread: not available

Screening report

screened PR #957 and moved project item `PVTI_lADOEFbZWM4BUUALzguTGA4` to `PR-Screening`.

GitHub comment: #957 (comment)
Project action: https://github.com/orgs/openabdev/projects/1

Intent

PR #957 fixes Telegram delivery loss when Telegram rejects Markdown-formatted messages. The operator-visible problem is silent message drop: the gateway sends a Telegram message, ignores the API response body, and never retries when parse_mode=Markdown fails on common unescaped characters in Chinese text, URLs, or code snippets.

Feat

Fix. The Telegram adapter checks the Telegram ok response field, detects Markdown parse failures, logs a warning, and retries the same outbound message as plain text by omitting parse_mode.

Who It Serves

Telegram users and agent runtime operators. Users get delivered replies instead of missing messages; operators get a warning signal when the Markdown fallback path is used.

Rewritten Prompt

Update gateway/src/adapters/telegram.rs so Telegram sends no longer discard the response body. Parse the Telegram API response, treat ok: false Markdown parse errors as recoverable, and retry the same message without Markdown formatting. Preserve existing behavior for successful sends and non-Markdown failures, add warning logs for fallback activation, and cover the response-handling behavior with targeted tests or a small testable helper if the adapter is otherwise hard to exercise.

Merge Pitch

This should move forward because it addresses a real reliability bug with a narrow surface area: one adapter file and a fallback that degrades formatting instead of dropping content. Risk is moderate-low, mostly around correctly identifying Telegram parse errors without masking other send failures. The likely reviewer concern is test coverage: the PR says syntax was checked but full compile was unavailable, so maintainers should ask for either CI confirmation or focused tests around ok=false handling.

Best-Practice Comparison

OpenClaw retry/backoff and run-log posture applies partially: this PR adds a local retry and warning log, but not durable retry persistence or structured delivery logs. That is acceptable for a formatting fallback because the retry is immediate and deterministic.

Hermes Agent daemon tick model, file locking, atomic persisted state, and fresh scheduled sessions do not materially apply here. This is not scheduled execution state; it is adapter-level delivery error handling.

Implementation Options

Conservative: keep the current PR shape, but verify CI and add a focused unit test for Telegram response classification if the code can be factored cheaply.

Balanced: extract Telegram send response handling into a small helper that returns Delivered, RetryPlainText, or Failed, then unit-test success, Markdown parse failure, and non-retryable API failure cases.

Ambitious: introduce a shared outbound delivery result model across adapters with structured logs, retry classification, and metrics hooks so Telegram, Slack, Discord, and future adapters report delivery failures consistently.

Comparison Table

Option	Speed	Complexity	Reliability	Maintainability	User Impact	Fit for OpenAB now
Conservative	High	Low	Medium	Medium	High for Telegram	Good if CI is green
Balanced	Medium	Medium	High	High	High for Telegram	Best
Ambitious	Low	High	High	Medium-High	Broader long-term impact	Too large for this PR

Recommendation

Take the balanced path if reviewer time allows: keep the fallback behavior, but make response classification explicit and testable before merge. If the existing patch is already clean and CI passes, accept a conservative merge with a follow-up issue for adapter delivery-result consistency. Do not expand this PR into a cross-adapter reliability refactor.

[shaun-agent]

Fix pushed — addressing F1/F2/F3

Pushed 4e8ec39 with the following changes:

F1 (retry scope): Retry now only triggers when description contains "parse" or "entities" — rate limits, auth errors, and other non-Markdown failures are logged as errors without retry.

F2 (retry response check): The plain-text retry response body is now checked. If the retry also returns ok: false, it logs an error with the description.

F3 (null description): All body["description"]" accesses now use .as_str().unwrap_or("unknown error")`.

@chaodu-agent please re-review when ready. No cargo in this env so CI will be the compile gate.

[shaun-agent]

Staff review: request changes. The fallback should not retry plain text for every Telegram API failure. Current PR retries on any ok=false response, which can mask unrelated failures such as invalid thread/chat, blocked bot, or rate limiting. Proposed fix: only retry without parse_mode when Telegram reports a Markdown entity parse failure (for example, "can't parse entities"), and still inspect/log the plain-text retry response. I have a local candidate fix with focused predicate tests, but Rust tooling is unavailable in this environment, so cargo fmt/test have not been run.

[shaun-agent]

Update after reviewing remote head 4e8ec39: the functional issues I requested changes for are addressed there (scoped retry, retry response checked, null description handled). I am holding my CHANGES_REQUESTED until the final small test/helper patch is reviewed, because remote does not currently include the helper/unit tests mentioned in thread.\n\nLocal candidate on top of 4e8ec39: cf83e70 test(telegram): cover markdown fallback predicate\n- extracts is_telegram_markdown_parse_error(desc)\n- uses case-insensitive desc.contains("parse") || desc.contains("entities")\n- covers can't parse entities, can't parse message text, case variance, and non-formatting errors\n\nValidation available here: git diff --check passes. cargo is not installed in this environment, so cargo fmt/test could not be run locally; PR CI gateway is currently green for 4e8ec39.

[shaun-agent]

Approved after 19ebe0e. The retry is now scoped behind a helper, covers Telegram Markdown parse wording variants, and the plain-text retry response is checked. CI gateway is green.\n\nNote: local git diff --check over the full PR still reports a pre-existing EOF blank-line issue in charts/openab-feishu/README.md, but it is unrelated to this Telegram fix and CI does not gate on it.

[shaun-agent]

Shifting this back to chaodu backlog per Shaun. Current state: code review issues addressed in 19ebe0e, CI is green, PR is mergeable, and shaun-agent has approved. Remaining blocker is repository review policy: GitHub still reports REVIEW_REQUIRED with review requested from @thepagent. No further code changes are pending from my side.