feat(telegram): support copy text buttons - #59445
Conversation
Cherry-picked host-level circuit breaker from kh/release-update-codex-stability to protect all provider calls (incl. kiro fallback) from retry storms that worsen ephemeral TCP port exhaustion. Prereq for re-enabling kiro fallback.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the Telegram copy-button implementation. The feature is still absent from current main, but this branch needs focused salvage before it can be reviewed as that feature.
Problems
- The diff again contains unrelated Codex usage/credential telemetry and network-circuit work (
agent/credential_usage.py:1,hermes_cli/codex_usage.py:1,agent/network_circuit_breaker.py:1), despite the linked #59443 discussion stating that those commits had been accidentally included. Please keep the Telegram feature isolated. plugins/platforms/telegram/adapter.py:3944strips markers and createscopy_markup, but oversized final edits return through_edit_overflow_splitat lines 3981-3985. That helper has no markup parameter (:4149-4157), so the claimed streaming-final support drops the copy keyboard for overflowed responses.tests/gateway/test_telegram_copy_buttons.py:180-202covers only the short path.
Suggested changes
- Split the unrelated work from this PR and reconcile the Telegram change with the later adapter refactors on current main.
- Pass markup through the overflow-final path and add a >4096 UTF-16 streamed-final regression test.
Automated hermes-sweeper review.
|
|
||
| copy_markup = None | ||
| if finalize and CopyTextButton is not None: | ||
| content, copy_buttons = _extract_copy_buttons(content) |
There was a problem hiding this comment.
For a final response over 4096 UTF-16 units, this stripped content returns through _edit_overflow_split at lines 3981-3985. That helper accepts no copy_markup, so the keyboard computed here is lost before the normal edit_message_text(... reply_markup=...) path. Please plumb markup through the overflow split and add a >4096 streamed-final regression test.
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Three PRs address the Telegram copy-text-button feature by parsing COPY_BUTTON markers and attaching CopyTextButton keyboards, but #59443 and #59445 mix that implementation with unrelated Codex telemetry, while #69153 isolates the Telegram change on current main. Unlike the earlier diffs, #69153 also carries the keyboard through oversized streaming-final responses and fallback paths.
Related pull requests
- #59443 [closed]
duplicate— (+1147/-10) — superseded: The closed PR implements marker parsing, normal sends, and short final edits, but most of its diff is unrelated Codex quota and credential-usage work; it remains relevant as the original implementation and was explicitly replaced by the cleaner #59445. - #59445
related— (+2332/-11) — superseded: It retains the same Telegram implementation but again includes unrelated Codex telemetry and network-circuit changes, and its overflow-final path dropsreply_markup. Despite the keep_open review on #59445, that review explicitly requires focused salvage; #69153 provides that clean split and fixes the identified overflow defect. - #69153
duplicate— (+417/-14) — preferred: This current-main, Telegram-only diff preserves malformed markers, supports normal sends and short final edits, and passes copy markup into_edit_overflow_split, attaching it to the final continuation and preserving it through plain-text, reply, flood-retry, and reactive-overflow fallbacks.
Duplicates
#59443, #59445, and #69153 implement substantially the same Telegram COPY_BUTTON feature; #69153 is the clean superseding iteration, while #59443 and #59445 contain unrelated changes and lack complete overflow handling.
Suggested consolidation
Merge #69153 after normal maintainer review — it is the focused implementation and directly addresses the blocking scope and overflow concerns documented on #59445. Close #59445 as superseded by #69153; #59443 is already closed and remains superseded by the later clean iterations.
Complex graph
flowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
subgraph Dup59443 ["PRs duplicating each other"]
P59443["PR #59443 (closed)"]
P59445["PR #59445 (open)"]
P69153["PR #69153 (open)"]
end
class P59443 closed
class P59445 open
class P69153 open
class P59445 target
click P59443 "https://github.com/NousResearch/hermes-agent/pull/59443"
click P59445 "https://github.com/NousResearch/hermes-agent/pull/59445"
click P69153 "https://github.com/NousResearch/hermes-agent/pull/69153"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed or no verify verdict yet (state tag in the node label).
Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 198 kB of PR diffs, 3 kB of issue/PR text, 3 kB of discussion (4 comments), 3 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Delta since our previous triage comment
@teknium1 extended our analysis by identifying an uncovered standalone Telegram delivery path in #69153: cron and send_message delegate to _send_telegram, which neither extracts COPY_BUTTON markers nor attaches reply_markup. This changes #69153 from ready for normal maintainer review to requiring focused salvage and standalone-path coverage before merge.
Changed pull requests
- #69153
duplicate— (+417/-14) — keep open: The focused diff fixes adapter sends and overflow/fallback edits, but the new keep_open review shows that standalone Telegram delivery still emits valid markers as visible text without copy-button markup; reuse the parser/markup logic in_send_telegramand test normal, chunked, and fallback sends.
Suggested consolidation
Keep #69153 open until the standalone Telegram path and tests are added; the prior recommendation to prefer #69153 over #59445 otherwise remains unchanged.
Complex graph unchanged since our previous triage comment.
Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 198 kB of PR diffs, 3 kB of issue/PR text, 3 kB of discussion (5 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
Summary
COPY_BUTTON: label | textmarkers that render as Bot APICopyTextButtoninline keyboard buttonsCopyTextButtonis unavailableTest Plan
uv run --extra messaging python -m pytest tests/gateway/test_telegram_copy_buttons.py tests/gateway/test_telegram_clarify_buttons.py tests/gateway/test_telegram_rich_messages.py tests/gateway/test_telegram_thread_fallback.py -q -o 'addopts='git diff --checkuv run --extra messaging python -m py_compile plugins/platforms/telegram/adapter.py tests/gateway/test_telegram_copy_buttons.py