From d1410867063ab70831f85e310fb7360250fb0edf Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 15 Nov 2026 03:14:10 +0000 Subject: [PATCH] fix(server): PR watch wakes quote a bot comment's text, not its HTML A watched pull request's wake message quotes the first 200 characters of each new comment. Review bots open their comments with a badge link made of HTML tags, so the quote was all markup and the agent got none of the comment. The excerpt now drops HTML tags from a comment's prose before it is cut. Fenced code blocks, indented code blocks and code spans are read first and left as written, so an HTML example a reviewer posts still reaches the agent. An opener glued to an identifier (`Vec`) is a type argument and stays, as does anything that is not a lowercase element name (`Promise`, `Promise`). Co-authored-by: Taylor Dolezal <117500+onlydole@users.noreply.github.com> Co-Authored-By: Claude Fable 5.2 --- .../orchestration-v2/pullRequestWatch.test.ts | 41 ++++++++++ .../src/orchestration-v2/pullRequestWatch.ts | 76 ++++++++++++++++++- 2 files changed, 115 insertions(+), 2 deletions(-) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index 5a836898bb66c..7275fd845b373 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -735,6 +735,47 @@ describe("evaluatePullRequestWatch", () => { assert.include(text, `"${"x".repeat(196)}..."`); }); + it.each([ + [ + "a bot's leading badge", + 'Severity > `Minor`\r\n\r\n_⚠️ Potential issue_ | _🟠 Major_\r\n\r\n**Guard the empty list.**', + "_⚠️ Potential issue_ | _🟠 Major_ **Guard the empty list.**", + ], + [ + "tags in prose and a hidden comment", + "\n
\nWhy\n\n

Guard this

, then
retry.\n
", + "Why Guard this , then retry.", + ], + [ + "type arguments", + "Return Promise, not Promise or Vec.", + "Return Promise, not Promise or Vec.", + ], + [ + "code spans and fences", + "Replace `
` with ````:\n~~~html\n

Done

\n~~~\r\nor\r\n```html\r\nDone\r\n```", + "Replace `
` with ````: ~~~html

Done

~~~ or ```html Done ```", + ], + [ + "indented code", + "Use this replacement:\n\n
\n Ready\n
\n\nKeep the wrapper.", + "Use this replacement:
Ready
Keep the wrapper .", + ], + ["an indented continuation of an HTML block", "
\n Ready\n
", "Ready"], + ])("quotes a remark's text without its HTML tags: %s", (_name, body, quoted) => { + const report = evaluatePullRequestWatch(watch(), detail(), [ + remark("reviewer", "2026-10-02T12:06:00Z", body), + ]); + const { text } = pullRequestWatchMessage({ + number: 1, + url: "https://github.com/o/r/pull/1", + baseBranch: "main", + headSha: null, + report, + }); + assert.include(text, `: "${quoted}" `); + }); + it("reports a conflict once, until the branch is clean again", () => { const conflicting = detail({ mergeability: "conflicting" }); const first = evaluatePullRequestWatch(watch(), conflicting, noRemarks); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index a6603aad97c3f..e4a00f0624244 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -261,9 +261,81 @@ export function evaluatePullRequestWatch( }; } +// GitHub renders these, so in a remark's prose they are presentation, not what the reviewer said. +// Lowercase only: `Promise` is a type argument. +const TAG_NAMES = + "a|abbr|b|blockquote|br|caption|cite|code|dd|del|details|div|dl|dt|em|h[1-6]|hr|i|img|ins|kbd|li|mark|ol|p|picture|pre|q|s|samp|small|source|span|strike|strong|sub|summary|sup|table|tbody|td|tfoot|th|thead|tr|tt|ul|var"; +// A code span comes first so its tags stay literal; it cannot cross a blank line. A tag stops at +// the next `<`, which keeps unclosed openers from rescanning the rest of the body. +const PROSE_MARKUP = new RegExp( + "(?`+)(?!`)(?:(?!\\n[ \\t\\r]*\\n)[\\s\\S])*?(?(?!`)" + + `|<(?/?)(?${TAG_NAMES})(?=[\\s/>])(?:[^<>"']|"[^"]*"|'[^']*')*>`, + "g", +); + +function withoutTags(prose: string): string { + // An opener glued to an identifier is a type argument (`Vec`); it and its closer stay. + const kept = new Map(); + let text = ""; + let from = 0; + for (const match of prose.matchAll(PROSE_MARKUP)) { + const name = match.groups?.name; + if (name === undefined) continue; + const held = kept.get(name) ?? 0; + if (match.groups?.closing === "/") { + if (held > 0) { + kept.set(name, held - 1); + continue; + } + } else if (/\w/.test(prose.charAt(match.index - 1))) { + kept.set(name, held + 1); + continue; + } + text += `${prose.slice(from, match.index)} `; + from = match.index + match[0].length; + } + return text + prose.slice(from); +} + +// Drops HTML tags from Markdown prose. Fenced and indented code blocks pass through untouched. +function withoutMarkup(body: string): string { + const parts: Array = []; + let prose: Array = []; + let fence: string | undefined; + let afterText = false; + const code = (line: string) => { + if (prose.length > 0) parts.push(withoutTags(prose.join("\n"))); + prose = []; + parts.push(line); + afterText = false; + }; + for (const line of body.split("\n")) { + if (fence !== undefined) { + const closer = line.trim(); + if (closer.length >= fence.length && closer.replaceAll(fence.charAt(0), "") === "") { + fence = undefined; + } + code(line); + continue; + } + const opener = /^ {0,3}(`{3,}|~{3,})([\s\S]*)$/.exec(line); + if (opener?.[1] !== undefined && !(opener[1].startsWith("`") && opener[2]?.includes("`"))) { + fence = opener[1]; + code(line); + } else if (!afterText && /^(?: {4}| {0,3}\t)\s*\S/.test(line)) { + // Indented code needs a blank line before it; after text the indent is a continuation. + code(line); + } else { + prose.push(line); + afterText = line.trim() !== ""; + } + } + if (prose.length > 0) parts.push(withoutTags(prose.join("\n"))); + return parts.join("\n"); +} + function snippet(body: string): string { - const text = body - .replaceAll(//g, " ") + const text = withoutMarkup(body.replaceAll(//g, " ")) .replaceAll(/\s+/g, " ") .trim(); if (text.length <= SNIPPET_LENGTH) return text;