From 2c894bd37630fa685542eddcc6384f9601c1ed00 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Thu, 20 Aug 2026 19:01:03 +1000 Subject: [PATCH] PM-5863: keep request body line breaks in the support Slack notification What was broken The Slack post for a new support ticket published the member's request body as a single run-on line. Every newline the member typed was collapsed into a space, so paragraphs and list items merged into one sentence: a request written as "Steps I tried:", "- Chrome, incognito", "- Firefox" rendered as "Steps I tried: Chrome, incognito Firefox", which reads as one item and loses the fact that two separate browsers were tried. PM-5863 asked for the notification content to be on multiple lines and for the body of the request to be included, and the body is the part of the message that most needs its own line structure. Root cause markdownNotificationPreview was written for email template data, where the template controls layout, so its final sanitization step collapsed all whitespace with /\s+/g -> ' '. When the Slack request-body preview was added it reused that same helper, inheriting the newline collapsing along with the wanted markdown and URL stripping. What was changed - Split the shared sanitization out of markdownNotificationPreview into a sanitizeMarkdown helper that strips markdown syntax, HTML tags, link targets, and HTML entities while leaving line breaks intact, plus previewLimit and boundPreview helpers for the bound normalization and code-point-safe truncation both previews already needed. - Added markdownNotificationBlockPreview, which collapses only horizontal whitespace, trims each line, and reduces runs of blank lines to a single paragraph break. Embedded URLs and raw markdown are still never published and the preview is still bounded, so the sanitization guarantees are unchanged. - deliverSlack now builds the new-ticket request body with markdownNotificationBlockPreview instead of the single-line preview. - markdownNotificationPreview keeps its exact previous output and remains the helper used for every email template field, so email content is untouched. - Updated the deliverSlack and slackMessage documentation and the README notification section to state that the body preview keeps the author's line and paragraph breaks. Any added/updated tests - notification-outbox.service.spec.ts: a delivery test asserting the opened Slack message carries the request body as its own lines, including the blank paragraph separators and the two list items on separate lines. - notification-outbox.service.spec.ts: unit tests for markdownNotificationBlockPreview covering CRLF input, markdown heading and list marker removal, link targets being stripped, blank-line runs collapsing to one, and bounding that does not split a Unicode code point. - Commands run: pnpm prisma:generate, pnpm lint, pnpm build, pnpm test --runInBand (63 tests, 8 suites passing). Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 3 +- .../notification-outbox.service.spec.ts | 57 ++++++++++ .../notification-outbox.service.ts | 103 ++++++++++++++---- 3 files changed, 142 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 6e53078..394267d 100644 --- a/README.md +++ b/README.md @@ -135,7 +135,8 @@ commit and retries failed intents with capped exponential backoff. Slack messages are multi-line: an event headline, the challenge as a link to `CHALLENGE_APP_BASE_URL/challenges/{challengeId}` when the ticket has one, the support ticket link, and β€” for a new ticket β€” the request body as a sanitized, -bounded plain-text preview. +bounded plain-text preview that keeps the author's own line and paragraph breaks +so separate paragraphs and list items do not run together. Email is published through Bus API v6 to Kafka topic `external.action.email`. `tc-bus-api-wrapper` appends `/bus/events`, so diff --git a/src/notifications/notification-outbox.service.spec.ts b/src/notifications/notification-outbox.service.spec.ts index c4997e7..c3488b2 100644 --- a/src/notifications/notification-outbox.service.spec.ts +++ b/src/notifications/notification-outbox.service.spec.ts @@ -12,6 +12,7 @@ import { SlackService } from '../integrations/slack.service'; import { MemberDirectoryService } from './member-directory.service'; import { NotificationOutboxService, + markdownNotificationBlockPreview, markdownNotificationPreview, } from './notification-outbox.service'; @@ -395,6 +396,38 @@ describe('NotificationOutboxService delivery', () => { expect(message).not.toContain('secret.example'); }); + it('keeps the request body line breaks in the opened Slack message', async () => { + const { db, service, slack } = createHarness(); + db.$queryRaw.mockResolvedValue([{ id: 'opened-slack', lockedAt }]); + db.notificationOutbox.findUnique.mockImplementation(() => { + const record = claimedRecord({ + channel: NotificationChannel.SLACK, + id: 'opened-slack', + }); + record.ticket = { + ...(record.ticket as Record), + description: + 'My submission failed to upload.\n\nSteps I tried:\n' + + '- Chrome, incognito\n- Firefox\n\nThe error was a 500.', + }; + return Promise.resolve(record); + }); + + await service.dispatch(['opened-slack']); + + const message = slack.sendNotification.mock.calls[0][0] as string; + expect(message.split('\n').slice(3)).toEqual([ + 'Request:', + 'My submission failed to upload.', + '', + 'Steps I tried:', + 'Chrome, incognito', + 'Firefox', + '', + 'The error was a 500.', + ]); + }); + it('omits the challenge line when the ticket has no challenge', async () => { const { db, service, slack } = createHarness(); db.$queryRaw.mockResolvedValue([{ id: 'opened-slack', lockedAt }]); @@ -645,3 +678,27 @@ describe('markdownNotificationPreview', () => { expect(preview).not.toContain('secret.example'); }); }); + +describe('markdownNotificationBlockPreview', () => { + it('keeps the author line breaks while still removing markdown URLs', () => { + const preview = markdownNotificationBlockPreview( + '# Upload fails\r\nSteps [tried](https://secret.example):\r\n' + + '- Chrome incognito\r\n- Firefox\r\n\r\n\r\nPlease help.', + ); + + expect(preview).toBe( + 'Upload fails\nSteps tried:\nChrome incognito\nFirefox\n\nPlease help.', + ); + expect(preview).not.toContain('secret.example'); + }); + + it('bounds the preview without splitting a Unicode code point', () => { + const preview = markdownNotificationBlockPreview( + 'First line\nSecond line πŸ˜€πŸ˜€πŸ˜€πŸ˜€', + 18, + ); + + expect(preview).toBe('First line\nSecond…'); + expect(Array.from(preview).length).toBeLessThanOrEqual(18); + }); +}); diff --git a/src/notifications/notification-outbox.service.ts b/src/notifications/notification-outbox.service.ts index c6086b3..dc99732 100644 --- a/src/notifications/notification-outbox.service.ts +++ b/src/notifications/notification-outbox.service.ts @@ -27,23 +27,16 @@ const SLACK_BODY_PREVIEW_CHARACTERS = 1_000; const DEFAULT_CHALLENGE_APP_BASE_URL = 'https://work.topcoder.com'; /** - * Converts user-authored markdown to a compact plain-text notification preview. - * URLs, formatting markers, HTML tags, and excess whitespace are removed before - * the result is bounded without splitting a Unicode code point. + * Strips markdown syntax, HTML tags, link targets, and HTML entities from + * user-authored text while leaving the author's line breaks in place, so the + * single-line and multi-line notification previews share one sanitization pass. * * @param markdown untrusted ticket or response markdown. - * @param maximumCharacters maximum Unicode code points in the returned preview. - * @returns normalized plain text with an ellipsis when truncation is required. + * @returns plain text with markdown removed and line breaks preserved. */ -export function markdownNotificationPreview( - markdown: string, - maximumCharacters = NOTIFICATION_PREVIEW_CHARACTERS, -): string { - const boundedMaximum = - Number.isInteger(maximumCharacters) && maximumCharacters > 0 - ? maximumCharacters - : NOTIFICATION_PREVIEW_CHARACTERS; - const plainText = String(markdown) +function sanitizeMarkdown(markdown: string): string { + return String(markdown) + .replace(/\r\n?/g, '\n') .replace(/!\[([^\]]*)\]\([^)]*\)/g, '$1') .replace(/\[([^\]]+)\]\([^)]*\)/g, '$1') .replace(/<[^>]*>/g, ' ') @@ -56,9 +49,30 @@ export function markdownNotificationPreview( .replace(/</gi, '<') .replace(/>/gi, '>') .replace(/"/gi, '"') - .replace(/�*39;/gi, "'") - .replace(/\s+/g, ' ') - .trim(); + .replace(/�*39;/gi, "'"); +} + +/** + * Normalizes a caller-supplied preview bound, falling back to the stable + * default for non-positive or fractional values. + * + * @param maximumCharacters requested maximum Unicode code points. + * @returns a positive integer preview bound. + */ +function previewLimit(maximumCharacters: number): number { + return Number.isInteger(maximumCharacters) && maximumCharacters > 0 + ? maximumCharacters + : NOTIFICATION_PREVIEW_CHARACTERS; +} + +/** + * Bounds already-sanitized preview text without splitting a Unicode code point. + * + * @param plainText sanitized preview text. + * @param boundedMaximum positive maximum Unicode code points to keep. + * @returns the text, with a trailing ellipsis when truncation is required. + */ +function boundPreview(plainText: string, boundedMaximum: number): string { const codePoints = Array.from(plainText); if (codePoints.length <= boundedMaximum) { return plainText; @@ -72,6 +86,52 @@ export function markdownNotificationPreview( .trimEnd()}…`; } +/** + * Converts user-authored markdown to a compact single-line plain-text preview. + * URLs, formatting markers, HTML tags, and excess whitespace are removed before + * the result is bounded without splitting a Unicode code point. Used for email + * template data, where the surrounding template controls the layout. + * + * @param markdown untrusted ticket or response markdown. + * @param maximumCharacters maximum Unicode code points in the returned preview. + * @returns normalized plain text with an ellipsis when truncation is required. + */ +export function markdownNotificationPreview( + markdown: string, + maximumCharacters = NOTIFICATION_PREVIEW_CHARACTERS, +): string { + return boundPreview( + sanitizeMarkdown(markdown).replace(/\s+/g, ' ').trim(), + previewLimit(maximumCharacters), + ); +} + +/** + * Converts user-authored markdown to a plain-text preview that keeps the + * author's line and paragraph breaks, for channels such as Slack that render + * multi-line text. Sanitization and bounding match the single-line preview, so + * raw markdown and embedded URLs are still never published; only horizontal + * whitespace is collapsed and runs of blank lines are reduced to one, which + * keeps separate paragraphs and list items from reading as a single sentence. + * + * @param markdown untrusted ticket or response markdown. + * @param maximumCharacters maximum Unicode code points in the returned preview. + * @returns multi-line plain text with an ellipsis when truncation is required. + */ +export function markdownNotificationBlockPreview( + markdown: string, + maximumCharacters = NOTIFICATION_PREVIEW_CHARACTERS, +): string { + const plainText = sanitizeMarkdown(markdown) + .replace(/[^\S\n]+/g, ' ') + .split('\n') + .map((line) => line.trim()) + .join('\n') + .replace(/\n{3,}/g, '\n\n') + .trim(); + return boundPreview(plainText, previewLimit(maximumCharacters)); +} + /** * Reads the polling interval before Nest registers the interval metadata. * Runtime ECS variables are present before module loading; invalid values use @@ -580,7 +640,9 @@ export class NotificationOutboxService { /** * Publishes multi-line lifecycle text to Slack. Ticket markdown is never sent - * verbatim; the request body is included only as a sanitized, bounded preview. + * verbatim; the request body is included only as a sanitized, bounded preview + * that keeps the author's own line breaks so paragraphs and list items stay + * readable. * * @param record claimed outbox row with ticket data. * @returns a promise resolved after Slack accepts the message. @@ -593,7 +655,7 @@ export class NotificationOutboxService { this.slackMessage(record, `New support ticket opened by ${handle}.`, [ 'Request:', this.escapeSlack( - markdownNotificationPreview( + markdownNotificationBlockPreview( record.ticket.description, SLACK_BODY_PREVIEW_CHARACTERS, ), @@ -636,7 +698,8 @@ export class NotificationOutboxService { * * @param record claimed outbox row with ticket data. * @param headline already escaped event-specific first line. - * @param detailLines already escaped lines appended after the ticket link. + * @param detailLines already escaped lines appended after the ticket link; + * an entry may itself span several lines, such as the request body preview. * @returns newline-separated Slack message text. */ private slackMessage(