Skip to content

Commit 2f474d7

Browse files
konardkonard
authored
Merge pull request #1340 from link-assistant/issue-1339-c0ed73492eed
Fix /merge: MarkdownV2 escaping error and UNKNOWN merge state (#1339)
2 parents d969410 + 40282f3 commit 2f474d7

File tree

6 files changed

+580
-41
lines changed

6 files changed

+580
-41
lines changed

‎.changeset/fix-merge-markdownv2-escaping-and-unknown-merge-state.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@link-assistant/hive-mind': patch
3+
---
4+
5+
fix: escape '...' ellipsis in MarkdownV2 and retry on UNKNOWN merge state (Issue #1339)
6+
7+
Two root causes fixed:
8+
9+
1. **MarkdownV2 escaping**: In `formatProgressMessage()`, literal '...' was appended in PR titles, error messages, and overflow lines. Telegram's MarkdownV2 requires '.' to be escaped as '\.' - unescaped periods caused 400 Bad Request errors on every message update during CI wait.
10+
11+
2. **UNKNOWN merge state**: GitHub computes PR mergeability asynchronously, so initial queries may return `mergeStateStatus: 'UNKNOWN'`. The old code immediately skipped PRs in this state. Fixed by adding retry logic to `checkPRMergeable()` that retries up to 3 times with 5-second delays before giving up.

‎docs/case-studies/issue-1339/README.md‎

Lines changed: 246 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,246 @@
1+
# Case Study: Issue #1339 - `/merge` MarkdownV2 Parsing Error & PRs Skipped
2+
3+
## Overview
4+
5+
**Issue:** [#1339](https://github.com/link-assistant/hive-mind/issues/1339)
6+
**Status:** Bug
7+
**Reporter:** Konstantin Dyachenko
8+
**Date:** 2026-02-20
9+
**Components Affected:** `src/telegram-merge-queue.lib.mjs`, `src/github-merge.lib.mjs`
10+
11+
## Incident Summary
12+
13+
When running `/merge https://github.com/link-assistant/hive-mind`, the Telegram bot repeatedly failed to update its status message with the error:
14+
15+
```
16+
[VERBOSE] /merge: Error updating message: 400: Bad Request: can't parse entities: Character '.' is reserved and must be escaped with the preceding '\'
17+
```
18+
19+
After the merge queue completed (34m 13s), the result was:
20+
21+
- ✅ Merged: 0
22+
- ❌ Failed: 0
23+
- ⏭️ Skipped: 2
24+
- Total: 2
25+
26+
Both PRs were **skipped** rather than merged, with reason `Merge state: UNKNOWN`.
27+
28+
## Timeline Reconstruction
29+
30+
```
31+
2026-02-20 (approximate times based on issue logs)
32+
│
33+
├── User runs /merge https://github.com/link-assistant/hive-mind
34+
│
35+
├── Bot initializes merge queue
36+
│ └── 2 PRs found: #1298 (Issue #1296), #1303 (Issue #1302)
37+
│
38+
├── Bot starts waitForTargetBranchCI()
39+
│ └── Finds 1 active CI run: Run #22243192736 "Checks and release" (in_progress)
40+
│
41+
├── [LOOP - every 30s poll interval]
42+
│ ├── onStatusUpdate callback fires
43+
│ ├── formatProgressMessage() is called
44+
│ │ └── Message contains unescaped '...' (ellipsis from PR title truncation)
45+
│ │ or '...' from 'more issues' section
46+
│ └── Telegram API returns 400 "Character '.' is reserved"
47+
│ [VERBOSE] Error updating message: 400: Bad Request...
48+
│
49+
├── [93s elapsed] First poll logged
50+
├── [123s elapsed] Second poll logged
51+
├── ... (approximately 34 minutes)
52+
│
53+
├── Target branch CI completes
54+
│ └── Bot proceeds to process PRs
55+
│
56+
├── PR #1298 checked for mergeability
57+
│ └── GitHub returns mergeable: null / mergeStateStatus: 'UNKNOWN'
58+
│ (GitHub computes mergeability asynchronously - returns UNKNOWN briefly)
59+
│ └── checkPRMergeable() returns { mergeable: false, reason: 'Merge state: UNKNOWN' }
60+
│ └── PR #1298 SKIPPED
61+
│
62+
├── PR #1303 checked for mergeability
63+
│ └── Same UNKNOWN state
64+
│ └── PR #1303 SKIPPED
65+
│
66+
└── Merge Queue "Completed": 0 merged, 2 skipped
67+
```
68+
69+
## Root Cause Analysis
70+
71+
### Root Cause 1: Unescaped Ellipsis (`...`) in MarkdownV2 Messages
72+
73+
**File:** `src/telegram-merge-queue.lib.mjs`
74+
75+
**Location:** `formatProgressMessage()` method
76+
77+
Telegram's MarkdownV2 mode requires ALL special characters to be escaped with a preceding `\`. The period `.` is one of these reserved characters.
78+
79+
The code in `formatProgressMessage()` correctly escapes owner/repo names, PR numbers, and error messages via `this.escapeMarkdown()`, but **fails to escape the literal ellipsis `...` appended after truncated text**:
80+
81+
```javascript
82+
// Line 541 - truncated error text (BUGGY):
83+
message += ` ${statusEmoji} \\#${item.prNumber}: ${this.escapeMarkdown(item.error.substring(0, 50))}${item.error.length > 50 ? '...' : ''}\n`;
84+
// ^^^
85+
// '...' is unescaped! Telegram returns 400 error.
86+
87+
// Line 552 - truncated PR title (BUGGY):
88+
message += `${item.emoji} \\#${item.prNumber}: ${this.escapeMarkdown(item.title.substring(0, 35))}${item.title.length > 35 ? '...' : ''}\n`;
89+
// ^^^
90+
// Same issue!
91+
```
92+
93+
Additionally:
94+
95+
- **Line 544:** `_...and ${problemItems.length - 5} more issues_` contains unescaped `...`
96+
- **Line 556:** `_...and ${update.items.length - 10} more_` contains unescaped `...`
97+
- **Line 523:** `\\n\\n` at the end of the CI wait message produces literal `\n\n` (backslash-n) in the Telegram message instead of actual newlines. In MarkdownV2, backslash is only valid before specific reserved characters, not before `n`.
98+
99+
**Why it triggered here:** The merge queue had 2 PRs. When `formatProgressMessage()` was called with PR items showing a pending status during the target branch CI wait, the PR titles (if longer than 35 chars) would trigger the truncated `...` path.
100+
101+
**Telegram MarkdownV2 Reference:** https://core.telegram.org/bots/api#markdownv2-style
102+
103+
> In all other places characters '\_', '\*', '[', ']', '(', ')', '~', '`', '>', '#', '+', '-', '=', '|', '{', '}', '.', '!' must be escaped with the preceding character '\'.
104+
105+
### Root Cause 2: `UNKNOWN` Merge State Causes PRs to be Skipped
106+
107+
**File:** `src/github-merge.lib.mjs`
108+
109+
**Location:** `checkPRMergeable()` function
110+
111+
GitHub computes mergeability **asynchronously**. When you first query a PR's merge state, GitHub may return `mergeable: null` and `mergeStateStatus: 'UNKNOWN'` while it's still computing in the background. A subsequent request (usually within seconds) will return the correct state.
112+
113+
The current implementation does NOT retry when the state is `UNKNOWN`:
114+
115+
```javascript
116+
export async function checkPRMergeable(owner, repo, prNumber, verbose = false) {
117+
const pr = JSON.parse(stdout.trim());
118+
119+
const mergeable = pr.mergeable === 'MERGEABLE'; // null or 'UNKNOWN' -> false
120+
121+
if (!mergeable) {
122+
switch (pr.mergeStateStatus) {
123+
case 'BLOCKED': ...
124+
case 'BEHIND': ...
125+
case 'DIRTY': ...
126+
case 'UNSTABLE': ...
127+
case 'DRAFT': ...
128+
default:
129+
reason = `Merge state: ${pr.mergeStateStatus || 'unknown'}`; // 'UNKNOWN' falls here!
130+
}
131+
}
132+
133+
return { mergeable, reason }; // Returns { mergeable: false, reason: 'Merge state: UNKNOWN' }
134+
}
135+
```
136+
137+
In `processItem()` in `telegram-merge-queue.lib.mjs`:
138+
139+
```javascript
140+
const mergeableCheck = await checkPRMergeable(this.owner, this.repo, item.pr.number, this.verbose);
141+
142+
if (!mergeableCheck.mergeable) {
143+
item.status = MergeItemStatus.SKIPPED; // PR is immediately skipped!
144+
item.error = mergeableCheck.reason; // Reason: 'Merge state: UNKNOWN'
145+
this.stats.skipped++;
146+
return;
147+
}
148+
```
149+
150+
**GitHub API Documentation:** GitHub's REST API documentation states that the `mergeable` field uses a "lazy evaluation" approach - the first request triggers the computation, and the value may be `null` until the computation completes. See: https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#get-a-pull-request
151+
152+
**Specific GitHub behavior:** When `mergeStateStatus` is `'UNKNOWN'`, it means GitHub hasn't calculated the merge state yet. The correct behavior is to wait briefly and retry.
153+
154+
## Impact
155+
156+
1. **Message update failures**: The bot silently fails to update the status message (only logs to verbose output), so users have no visibility into the merge queue progress during target branch CI waiting.
157+
158+
2. **PRs incorrectly skipped**: When GitHub returns `UNKNOWN` merge state (which is expected during the initial check), PRs are permanently skipped instead of being retried after a brief wait.
159+
160+
3. **Combined effect**: The 34-minute wait for target branch CI meant the bot had plenty of time to encounter the message update error repeatedly. When CI finally finished and the bot tried to merge PRs, GitHub's mergeability check happened to return `UNKNOWN` (possible due to fresh state after the long wait), causing both PRs to be skipped.
161+
162+
## Similar Issues / References
163+
164+
### Telegram MarkdownV2 Escaping
165+
166+
Similar issues have been reported in the Telegram Bot API community:
167+
168+
- GitHub issue tracker: Multiple projects hit this issue when using MarkdownV2 with dynamic content containing periods, exclamation marks, or other reserved characters
169+
- The `.` character is particularly common in CI run names, version numbers, and PR titles
170+
- **Known workaround**: Use the `escapeMarkdownV2()` function from `src/telegram-markdown.lib.mjs` which already exists in this codebase
171+
172+
### GitHub Mergeability UNKNOWN State
173+
174+
- GitHub's REST API documentation explicitly mentions this lazy evaluation
175+
- This is a known issue in GitHub integrations: https://docs.github.com/en/rest/guides/using-the-rest-api-for-your-integrations#dealing-with-rate-limiting
176+
- Stack Overflow discussion: Multiple questions about `mergeable: null` with recommendations to retry after 1-5 seconds
177+
- The GitHub GraphQL API has the same behavior with `mergeable: UNKNOWN`
178+
179+
### GitHub Issue: Telegram Bots and MarkdownV2
180+
181+
The Telegram Bot API documentation for MarkdownV2 is clear that ALL reserved characters must be escaped, including in pure text content (not just in formatting syntax). Projects that dynamically generate messages often miss escaping literal `...` (ellipsis) appended for truncation.
182+
183+
**Existing library solutions:**
184+
185+
- [`telegram-escape`](https://www.npmjs.com/package/telegram-escape) - npm package for escaping Telegram markdown
186+
- The codebase already has `src/telegram-markdown.lib.mjs` with `escapeMarkdownV2()` - it should be used consistently
187+
188+
## Proposed Solutions
189+
190+
### Fix 1: Escape Ellipsis in MarkdownV2 Messages
191+
192+
Replace unescaped `'...'` with `'\\.\\.\\.''` (three escaped periods) in `formatProgressMessage()`:
193+
194+
```javascript
195+
// Line 541 - BEFORE:
196+
${item.error.length > 50 ? '...' : ''}
197+
// AFTER:
198+
${item.error.length > 50 ? '\\.\\.\\.' : ''}
199+
200+
// Line 552 - BEFORE:
201+
${item.title.length > 35 ? '...' : ''}
202+
// AFTER:
203+
${item.title.length > 35 ? '\\.\\.\\.' : ''}
204+
205+
// Line 544 - BEFORE:
206+
` _...and ${problemItems.length - 5} more issues_\n`
207+
// AFTER:
208+
` _\\.\\.\\.and ${problemItems.length - 5} more issues_\n`
209+
210+
// Line 556 - BEFORE:
211+
`_...and ${update.items.length - 10} more_\n`
212+
// AFTER:
213+
`_\\.\\.\\.and ${update.items.length - 10} more_\n`
214+
215+
// Line 523 - BEFORE (\\n\\n = literal backslash-n):
216+
`...\\n\\n`
217+
// AFTER (actual newlines):
218+
`...\n\n`
219+
```
220+
221+
### Fix 2: Retry When Merge State is UNKNOWN
222+
223+
Add retry logic to `checkPRMergeable()`:
224+
225+
```javascript
226+
export async function checkPRMergeable(owner, repo, prNumber, verbose = false, maxRetries = 3) {
227+
for (let attempt = 0; attempt < maxRetries; attempt++) {
228+
const pr = JSON.parse(stdout.trim());
229+
230+
if (pr.mergeable === null || pr.mergeStateStatus === 'UNKNOWN') {
231+
if (attempt < maxRetries - 1) {
232+
// GitHub is still computing mergeability, wait and retry
233+
await new Promise(resolve => setTimeout(resolve, 5000 * (attempt + 1)));
234+
continue;
235+
}
236+
}
237+
// ... rest of logic
238+
}
239+
}
240+
```
241+
242+
## Data and Logs
243+
244+
The `screenshot.png` in this directory shows the actual Telegram conversation where the incident occurred.
245+
246+
- `screenshot.png` - Screenshot of the failed merge queue showing "Merged: 0, Skipped: 2"

‎docs/case-studies/issue-1339/screenshot.png‎

258 KB
Loading

‎src/github-merge.lib.mjs‎

Lines changed: 67 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -400,53 +400,85 @@ export async function checkPRCIStatus(owner, repo, prNumber, verbose = false) {
400400

401401
/**
402402
* Check if PR is mergeable
403+
*
404+
* Issue #1339: GitHub computes mergeability asynchronously. The first request may return
405+
* mergeable: null and mergeStateStatus: 'UNKNOWN' while the computation is in progress.
406+
* We retry up to 3 times with a 5-second delay between attempts to handle this case.
407+
*
403408
* @param {string} owner - Repository owner
404409
* @param {string} repo - Repository name
405410
* @param {number} prNumber - Pull request number
406411
* @param {boolean} verbose - Whether to log verbose output
407412
* @returns {Promise<{mergeable: boolean, reason: string|null}>}
408413
*/
409414
export async function checkPRMergeable(owner, repo, prNumber, verbose = false) {
410-
try {
411-
const { stdout } = await exec(`gh pr view ${prNumber} --repo ${owner}/${repo} --json mergeable,mergeStateStatus`);
412-
const pr = JSON.parse(stdout.trim());
413-
414-
const mergeable = pr.mergeable === 'MERGEABLE';
415-
let reason = null;
416-
417-
if (!mergeable) {
418-
switch (pr.mergeStateStatus) {
419-
case 'BLOCKED':
420-
reason = 'PR is blocked (possibly by branch protection rules)';
421-
break;
422-
case 'BEHIND':
423-
reason = 'PR branch is behind the base branch';
424-
break;
425-
case 'DIRTY':
426-
reason = 'PR has merge conflicts';
427-
break;
428-
case 'UNSTABLE':
429-
reason = 'PR has failing required status checks';
430-
break;
431-
case 'DRAFT':
432-
reason = 'PR is a draft';
433-
break;
434-
default:
435-
reason = `Merge state: ${pr.mergeStateStatus || 'unknown'}`;
415+
// Issue #1339: GitHub computes mergeability asynchronously. When mergeStateStatus is
416+
// 'UNKNOWN', it means GitHub hasn't calculated the merge state yet. Retry a few times.
417+
const MAX_UNKNOWN_RETRIES = 3;
418+
const UNKNOWN_RETRY_DELAY_MS = 5000;
419+
420+
for (let attempt = 0; attempt < MAX_UNKNOWN_RETRIES; attempt++) {
421+
try {
422+
const { stdout } = await exec(`gh pr view ${prNumber} --repo ${owner}/${repo} --json mergeable,mergeStateStatus`);
423+
const pr = JSON.parse(stdout.trim());
424+
425+
// Issue #1339: If mergeStateStatus is 'UNKNOWN', GitHub is still computing.
426+
// Wait and retry instead of immediately skipping the PR.
427+
if (pr.mergeStateStatus === 'UNKNOWN' || pr.mergeable === null) {
428+
if (attempt < MAX_UNKNOWN_RETRIES - 1) {
429+
if (verbose) {
430+
console.log(`[VERBOSE] /merge: PR #${prNumber} mergeability is UNKNOWN (attempt ${attempt + 1}/${MAX_UNKNOWN_RETRIES}), retrying in ${UNKNOWN_RETRY_DELAY_MS / 1000}s...`);
431+
}
432+
await new Promise(resolve => setTimeout(resolve, UNKNOWN_RETRY_DELAY_MS));
433+
continue;
434+
}
435+
// All retries exhausted, still UNKNOWN - treat as not mergeable
436+
if (verbose) {
437+
console.log(`[VERBOSE] /merge: PR #${prNumber} mergeability still UNKNOWN after ${MAX_UNKNOWN_RETRIES} attempts`);
438+
}
439+
return { mergeable: false, reason: `Merge state: UNKNOWN (GitHub could not compute mergeability after ${MAX_UNKNOWN_RETRIES} attempts)` };
436440
}
437-
}
438441

439-
if (verbose) {
440-
console.log(`[VERBOSE] /merge: PR #${prNumber} mergeable: ${mergeable}, state: ${pr.mergeStateStatus}`);
441-
}
442+
const mergeable = pr.mergeable === 'MERGEABLE';
443+
let reason = null;
444+
445+
if (!mergeable) {
446+
switch (pr.mergeStateStatus) {
447+
case 'BLOCKED':
448+
reason = 'PR is blocked (possibly by branch protection rules)';
449+
break;
450+
case 'BEHIND':
451+
reason = 'PR branch is behind the base branch';
452+
break;
453+
case 'DIRTY':
454+
reason = 'PR has merge conflicts';
455+
break;
456+
case 'UNSTABLE':
457+
reason = 'PR has failing required status checks';
458+
break;
459+
case 'DRAFT':
460+
reason = 'PR is a draft';
461+
break;
462+
default:
463+
reason = `Merge state: ${pr.mergeStateStatus || 'unknown'}`;
464+
}
465+
}
442466

443-
return { mergeable, reason };
444-
} catch (error) {
445-
if (verbose) {
446-
console.log(`[VERBOSE] /merge: Error checking mergeability: ${error.message}`);
467+
if (verbose) {
468+
console.log(`[VERBOSE] /merge: PR #${prNumber} mergeable: ${mergeable}, state: ${pr.mergeStateStatus}`);
469+
}
470+
471+
return { mergeable, reason };
472+
} catch (error) {
473+
if (verbose) {
474+
console.log(`[VERBOSE] /merge: Error checking mergeability: ${error.message}`);
475+
}
476+
return { mergeable: false, reason: error.message };
447477
}
448-
return { mergeable: false, reason: error.message };
449478
}
479+
480+
// Should not reach here, but return safe default
481+
return { mergeable: false, reason: 'Merge state: UNKNOWN' };
450482
}
451483

452484
/**

0 commit comments

Comments
 (0)