fix(adapters): bump telegramify-markdown to 1.3.3 for blockquote escaping#1340
Conversation
…ping telegramify-markdown 1.3.2 escapes the `>` blockquote marker (which Telegram MarkdownV2 supports natively) and double-escapes any other special character on the same line, so any blockquote ending in a period (or containing `.`, `!`, `-`, etc.) is rejected by the Bot API with "Character '.' is reserved and must be escaped". The bot falls back to plain text and logs telegram.markdownv2_failed. Upstream fixed both bugs in 1.3.3. Bumping the floor to ^1.3.3 and adding a regression test on the canonical "> hi." case so the behaviour can't silently regress if the dependency loosens later. Fixes coleam00#1102
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughUpdated ChangesAdapters: Telegram blockquote fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/adapters/src/chat/telegram/markdown.test.ts (1)
70-78: Regression test is tight and well-documented.The exact-match assertion (vs.
toContain) is the right choice here — it pins down both the unescaped>marker and the single-escaped., which is exactly what 1.3.2 got wrong. The inline comment referencing#1102is helpful for future bumps.Optional: consider extending coverage with one or two additional cases to harden the guard (e.g., multiple special chars on a blockquote line, or a multi-line blockquote), since the upstream escaping bug could plausibly regress on shapes not covered by
'> hi.'alone. Not a blocker.Optional: broaden blockquote coverage
describe('blockquotes', () => { // Regression: telegramify-markdown 1.3.2 escaped the `>` marker and // double-escaped any other special character on the same line, which // Telegram rejected with "Character '.' is reserved and must be // escaped". 1.3.3 fixes both. See coleam00/Archon#1102. test('escapes special chars exactly once inside blockquotes', () => { expect(convertToTelegramMarkdown('> hi.')).toBe('> hi\\.\n'); }); + + test('escapes multiple special chars exactly once on a blockquote line', () => { + expect(convertToTelegramMarkdown('> a.b-c!')).toBe('> a\\.b\\-c\\!\n'); + }); });Please verify the exact expected output for the additional case against 1.3.3 before committing it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/adapters/src/chat/telegram/markdown.test.ts` around lines 70 - 78, Add a couple more assertions to the existing blockquote test for convertToTelegramMarkdown (the test titled 'escapes special chars exactly once inside blockquotes') to cover additional shapes: one case with multiple special characters on a single blockquote line (e.g., '> hi.!$') and one multi-line blockquote (e.g., '> first.\n> second?') and assert the exact escaped output matches the behavior in telegramify-markdown 1.3.3; update the test expectations to the exact single-escape results for each case and run/verify against 1.3.3 before committing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/adapters/src/chat/telegram/markdown.test.ts`:
- Around line 70-78: Add a couple more assertions to the existing blockquote
test for convertToTelegramMarkdown (the test titled 'escapes special chars
exactly once inside blockquotes') to cover additional shapes: one case with
multiple special characters on a single blockquote line (e.g., '> hi.!$') and
one multi-line blockquote (e.g., '> first.\n> second?') and assert the exact
escaped output matches the behavior in telegramify-markdown 1.3.3; update the
test expectations to the exact single-escape results for each case and
run/verify against 1.3.3 before committing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 87dcad04-e382-4ff2-997b-3e0c52a14149
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
packages/adapters/package.jsonpackages/adapters/src/chat/telegram/markdown.test.ts
…ti-line cases Addresses review nit on coleam00#1340. 1.3.2's bug regressed both on multiple special characters on one line and on multi-line blockquotes, so pinning those shapes down adds real confidence against a future re-regression. Expected outputs verified against telegramify-markdown 1.3.3: - `> a.b-c!` escapes `.`, `-`, `!` exactly once, `>` marker unescaped. - `> first.\n> second?` escapes `.` once, `?` passes through (not in Telegram MarkdownV2's reserved set), `>` marker unescaped on both lines.
|
Broadened blockquote coverage with two verified cases. Expected outputs confirmed against |
|
Hi @truffle-dev — thanks for opening this PR. This repository uses a PR template at
Could you fill those out (even briefly)? The template helps reviewers understand scope, risk, and rollback — it speeds up review significantly. If a section genuinely doesn't apply, just write "N/A" in it rather than leaving it blank. |
|
Filled in. UX Journey, Architecture Diagram (no production module changes, dependency floor bump only), Label Snapshot, Change Metadata, Compatibility, Human Verification, Side Effects, Rollback, Risks all populated against the actual change. Existing Problem/Root-Cause/Validation kept and slotted into Summary + Validation Evidence. |
|
Friendly check-in. 18 days since the PR template was filled in per the review request. Happy to rebase against |
Review SummaryVerdict: ready-to-merge Dependency bump for Blocking issuesNone. Suggested fixesNone — the code itself is solid. One optional nit (see below). Minor / nice-to-have
Compliments
Reviewed via maintainer-review-pr workflow (Pi/Minimax). Aspects run: code-review, comment-quality. |
…vior Address review on coleam00#1340: drop version-number framing in favor of describing the bug behavior. Version numbers rot as the floor moves; the bug behavior plus the issue link stays stable.
Minor / nice-to-have
Re-ran |
2 participants
Summary
400: can't parse entities: Character '.' is reservedon every blockquote that contains a MarkdownV2 special character on the same line. Bot retries as plain text, so users get the content, but every workflow-start banner that quotes a workflow'sdescription:loses formatting and logstelegram.markdownv2_failed.telegramify-markdownfrom^1.3.0(resolving to 1.3.2) to^1.3.3inpackages/adapters/package.json; refreshedbun.lock; added a regression test inpackages/adapters/src/chat/telegram/markdown.test.ts.packages/adapters/was modified. No other package was touched. No new dependencies, no exports changed.Root Cause
packages/adapterspinnedtelegramify-markdown@^1.3.0, resolving to1.3.2. That version escapes the>blockquote marker (Telegram MarkdownV2 supports>natively) and double-escapes any other special character on the same line:Upstream fix: skitsanos/telegramify-markdown@1.3.3.
UX Journey
Before
After
Architecture Diagram
No production module changes — dependency floor bump + regression test only.
Connection inventory:
packages/adapterstelegramify-markdown(npm)package.json+bun.lockLabel Snapshot
risk: lowsize: XSadaptersadapters:telegramChange Metadata
bugadaptersLinked Issue
Validation Evidence (required)
'> hi\\\\.\n'returned vs'> hi\\.\n'expected).Security Impact (required)
Compatibility / Migration
telegramify-markdown1.3.2 → 1.3.3 is a patch release. Only the over-escape behaviour for>-prefixed lines changes; Telegram accepts the new (correct) output where it rejected the old.Human Verification (required)
'> blockquote.'→'> blockquote\\.\n'(single-char same-line)'> a.b-c!'→'> a\\.b\\-c\\!\n'(multi-char same-line, broadened in d25252f)'> first.\n> second?'→'> first\\.\n> second?\n'(multi-line;?correctly passes through, not in MarkdownV2's reserved set)>-line escaping path.telegramify-markdown@1.3.3resolves under the new^1.3.3constraint with no transitive downgrade.Side Effects / Blast Radius (required)
packages/adapters/src/chat/telegramonly — workflows that emit Telegram notifications (workflow-start banners, error notifications, run-summary messages).Rollback Plan (required)
git revert <merge-commit-sha>— single commit, three files (package.json,bun.lock, one test file).telegram.markdownv2_failedlog lines reappear in operator metrics; blockquote-bearing notifications drop back to plain text.Risks and Mitigations
convertToTelegramMarkdownmay have been depending on the over-escaped output as a workaround.Summary by CodeRabbit
Chores
Tests