Conversation
…eflect changes in analytics setup. Simplify production API logging section by removing unnecessary details.
…erational details. Expand on the personal assistant's functionality, including its cloud-native architecture, interoperability with AI services, and ambient hardware integration. Introduce principles guiding the design and outline a phased roadmap for future development. This update aims to clarify Amby's purpose and operational framework for users and developers alike.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
| function renderLink(node: Link): RenderedBlock { | ||
| const content = renderInlineNodes(node.children as Content[]) | ||
| const plainText = content.plainText || node.url | ||
| return { | ||
| html: `<a href="${escapeTelegramHtmlAttribute(node.url)}">${content.html || escapeTelegramHtml(node.url)}</a>`, | ||
| plainText, | ||
| } | ||
| } |
There was a problem hiding this comment.
No URL-scheme sanitization in
renderLink
node.url is HTML-attribute-escaped but not sanitized for dangerous URL schemes such as javascript: or data:. While Telegram's native clients don't execute JavaScript, forwarding javascript: URLs into the Telegram Bot API could confuse some third-party clients or future web-based surfaces.
Consider stripping or escaping non-HTTP(S) schemes before inserting the URL:
| function renderLink(node: Link): RenderedBlock { | |
| const content = renderInlineNodes(node.children as Content[]) | |
| const plainText = content.plainText || node.url | |
| return { | |
| html: `<a href="${escapeTelegramHtmlAttribute(node.url)}">${content.html || escapeTelegramHtml(node.url)}</a>`, | |
| plainText, | |
| } | |
| } | |
| function renderLink(node: Link): RenderedBlock { | |
| const content = renderInlineNodes(node.children as Content[]) | |
| const plainText = content.plainText || node.url | |
| const safeUrl = /^https?:\/\//i.test(node.url) ? node.url : "#" | |
| return { | |
| html: `<a href="${escapeTelegramHtmlAttribute(safeUrl)}">${content.html || escapeTelegramHtml(node.url)}</a>`, | |
| plainText, | |
| } | |
| } |
| export function renderTelegramMessageChunks( | ||
| markdown: string, | ||
| maxTextLength = TELEGRAM_MAX_TEXT_LENGTH, | ||
| ): TelegramRenderedChunk[] { | ||
| const ast = parseMarkdown(markdown) | ||
| const blocks = ast.children | ||
| .flatMap((node) => renderBlock(node)) | ||
| .filter((block) => block.plainText.trim().length > 0 || block.html.trim().length > 0) | ||
|
|
||
| if (blocks.length === 0) { | ||
| return markdown.trim() ? [{ html: escapeTelegramHtml(markdown), plainText: markdown }] : [] | ||
| } | ||
|
|
||
| const chunks: TelegramRenderedChunk[] = [] | ||
| let current: RenderedBlock[] = [] | ||
| let currentLength = 0 | ||
|
|
||
| for (const block of blocks.flatMap((candidate) => | ||
| splitOversizedBlock(candidate, maxTextLength), | ||
| )) { | ||
| const separatorLength = current.length > 0 ? 2 : 0 | ||
| if ( | ||
| current.length > 0 && | ||
| currentLength + separatorLength + block.plainText.length > maxTextLength | ||
| ) { | ||
| chunks.push(joinBlocks(current)) | ||
| current = [block] | ||
| currentLength = block.plainText.length | ||
| continue | ||
| } | ||
|
|
||
| current.push(block) | ||
| currentLength += separatorLength + block.plainText.length | ||
| } | ||
|
|
||
| if (current.length > 0) { | ||
| chunks.push(joinBlocks(current)) | ||
| } | ||
|
|
||
| return chunks | ||
| } |
There was a problem hiding this comment.
Chunk splitting measures
plainText.length, not html.length
The 4096-character ceiling is compared against block.plainText.length (the stripped text). Telegram's Bot API documentation describes the limit as "1-4096 characters after entities parsing," which most interpretations take to mean the rendered text — so this is likely correct.
However, if a chunk is dense with links or other inline formatting, the raw HTML string sent to Telegram could be substantially longer than plainText. If Telegram's server-side limit is applied to the raw text field before parsing, chunks near the boundary could be silently rejected. Adding an html.length guard (e.g., capping at TELEGRAM_MAX_TEXT_LENGTH * 2) would remove any ambiguity and protect against API errors for heavily formatted responses.
Summary
Verification
Greptile Summary
This PR fixes Telegram message formatting by replacing the
@chat-adapter/telegramadapter's legacy Markdown transport with a customTelegramBotApiClientthat converts agent-generated CommonMark into Telegram HTML and sends it withparse_mode=HTML. It also adds a sharedChannelPresentationcontract so the agent prompt is channel-aware, disables streaming edits in favor of fully-rendered final messages (with typing indicators retained), and updates the mock Telegram server and UI to handle and renderparse_mode.\n\nKey changes:\n- Newformatting.ts: walks the markdown AST node-by-node into Telegram HTML with correct escaping, oversized-block fallback to plain text, and 4096-char chunk splitting measured on rendered text length\n- Newbot-api.ts: thinTelegramBotApiClientthat calls the formatter and sends each chunk sequentially withparse_mode=HTML; replaces all directadapter.postMessage/editMessagecall sites inbot.ts,agent-execution.ts, andtelegram/index.ts\n- Newchannel-presentation.ts+buildConversationPromptintegration:getChannelPresentationproduces a typedChannelPresentationfor each platform; Telegram getstransportFormat: \"telegram-html\"andsupportsStreaming: false\n- Streaming edits removed fromagent-execution.ts, reducing code complexity and eliminating the correctness gap between partial and final markdown rendering\n- Mock Telegram server updated to preserveparse_mode;MessageBubblerenders HTML with aDOMParser-based React renderer gated by anisClienthydration flag\n- Regression tests added for formatter output, prompt channel rules, and mock message storeupdateMessageConfidence Score: 5/5
Safe to merge — the primary formatting pipeline is correct, well-tested, and all remaining notes are low-risk P2 suggestions.
The core change (markdown → Telegram HTML with parse_mode=HTML) is implemented correctly end-to-end. The formatter handles escaping, chunking, and oversized-block fallback. The streaming removal simplifies the code while retaining typing indicators. All three comments are P2 style suggestions: a URL-scheme guard in renderLink (low risk in Telegram's non-JS context), an optional HTML-length secondary cap (likely unnecessary given the spec language), and a pre-existing inconsistency in the error-reply path in bot.ts. None of these block correct operation for the normal message-delivery path.
apps/api/src/telegram/formatting.ts (renderLink URL sanitization and chunk HTML-length consideration)
Important Files Changed
Sequence Diagram
sequenceDiagram participant User as Telegram User participant TG as Telegram Bot API participant Bot as bot.ts / agent-execution.ts participant Agent as AgentService participant Fmt as formatting.ts participant Client as TelegramBotApiClient User->>TG: sends message TG->>Bot: webhook update Bot->>TG: sendChatAction(typing) Bot->>Agent: handleMessage(conversationId, text) Note over Agent: System prompt includes ChannelPresentation rules Agent-->>Bot: sendReply(progressText) [optional] Bot->>Fmt: renderTelegramMessageChunks(progressText) Fmt-->>Bot: [{html, plainText}, ...] Bot->>Client: sendMessage(chatId, progressText) Client->>TG: POST sendMessage(parse_mode=HTML) Agent-->>Bot: response.userResponse.text Bot->>Fmt: renderTelegramMessageChunks(finalText) Fmt-->>Bot: [{html, plainText}, ...] Bot->>Client: sendMessage(chatId, finalText) Client->>TG: POST sendMessage(parse_mode=HTML) [per chunk] TG-->>User: formatted replyComments Outside Diff (1)
apps/api/src/bot.ts, line 70-78 (link)thread.post()pathBoth
sendReplyand the final response now correctly usesender.sendMessage()(which renders markdown to HTML), but thecatchAllCauseerror handler below still callsthread.post(...)from the Chat SDK adapter:For a plain-text error string this makes no practical difference today, but it's an inconsistency — if the error message ever includes markdown (e.g. a formatted contact link), it would bypass the formatting pipeline. Consider routing this through
sender.sendMessage(chatId, "Sorry…")for uniformity.Reviews (1): Last reviewed commit: "Fix Telegram message formatting pipeline" | Re-trigger Greptile