Skip to content

Commit b712bc0

Browse files
dcramercodex
dcramer
and
codex
committed
fix(slack): Unify finalized reply rendering
Keep Slack reply formatting on one shared delivery path instead of splitting finalized replies between the raw API planner and the adapter post path. Escape literal Slack control characters in normalized prose, preserve valid Slack tokens, and upgrade simple single-post markdown tables into native Slack table blocks while keeping section/context fallbacks consistent across live and resumed replies. Document the finalized reply envelope and add contract coverage for the shared raw Slack reply planner so rendering regressions are caught at the boundary where users see them. Refs #208 Co-Authored-By: Codex GPT-5 <noreply@openai.com>
1 parent 7a04e88 commit b712bc0

16 files changed

Lines changed: 828 additions & 201 deletions

‎packages/junior/src/chat/runtime/reply-executor.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -445,21 +445,20 @@ export function createReplyToThread(deps: ReplyExecutorDeps) {
445445
traceId: getActiveTraceId(),
446446
usage: reply.diagnostics.usage,
447447
});
448-
const shouldUseSlackFooter =
449-
Boolean(replyFooter) &&
448+
const shouldUseSlackApiReplyDelivery =
450449
Boolean(channelId && threadTs) &&
451450
(thread.adapter as { name?: string } | undefined)?.name === "slack";
452451

453452
// Final Slack delivery is part of turn success. We only mark the turn
454453
// completed after the visible reply has been accepted by Slack.
455454
if (plannedPosts.length > 0) {
456455
let sent: SentMessage | undefined;
457-
if (shouldUseSlackFooter) {
456+
if (shouldUseSlackApiReplyDelivery) {
458457
const slackChannelId = channelId;
459458
const slackThreadTs = threadTs;
460459
if (!slackChannelId || !slackThreadTs) {
461460
throw new Error(
462-
"Slack footer delivery requires a concrete channel and thread timestamp",
461+
"Slack reply delivery requires a concrete channel and thread timestamp",
463462
);
464463
}
465464

‎packages/junior/src/chat/slack/footer.ts‎

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,5 @@
11
import type { AgentTurnUsage } from "@/chat/usage";
22

3-
interface SlackMrkdwnTextObject {
4-
text: string;
5-
type: "mrkdwn";
6-
}
7-
8-
interface SlackSectionBlock {
9-
text: SlackMrkdwnTextObject;
10-
type: "section";
11-
}
12-
13-
interface SlackContextBlock {
14-
elements: SlackMrkdwnTextObject[];
15-
type: "context";
16-
}
17-
18-
export type SlackMessageBlock = SlackSectionBlock | SlackContextBlock;
19-
203
interface SlackReplyFooterItem {
214
label: string;
225
value: string;
@@ -26,13 +9,6 @@ export interface SlackReplyFooter {
269
items: SlackReplyFooterItem[];
2710
}
2811

29-
function escapeSlackMrkdwn(text: string): string {
30-
return text
31-
.replaceAll("&", "&amp;")
32-
.replaceAll("<", "&lt;")
33-
.replaceAll(">", "&gt;");
34-
}
35-
3612
function formatSlackTokenCount(value: number): string {
3713
return new Intl.NumberFormat("en-US").format(value);
3814
}
@@ -118,30 +94,3 @@ export function buildSlackReplyFooter(args: {
11894

11995
return items.length > 0 ? { items } : undefined;
12096
}
121-
122-
/** Build Slack blocks for a finalized reply plus its optional footer context block. */
123-
export function buildSlackReplyBlocks(
124-
text: string,
125-
footer: SlackReplyFooter | undefined,
126-
): SlackMessageBlock[] | undefined {
127-
if (!text.trim() || !footer?.items.length) {
128-
return undefined;
129-
}
130-
131-
return [
132-
{
133-
type: "section",
134-
text: {
135-
type: "mrkdwn",
136-
text,
137-
},
138-
},
139-
{
140-
type: "context",
141-
elements: footer.items.map((item) => ({
142-
type: "mrkdwn",
143-
text: `*${escapeSlackMrkdwn(item.label)}:* ${escapeSlackMrkdwn(item.value)}`,
144-
})),
145-
},
146-
];
147-
}

‎packages/junior/src/chat/slack/markdown-table.ts‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
export interface MarkdownTableMatch {
2+
after: string;
3+
before: string;
4+
rows: string[][];
5+
}
6+
7+
/** Parse a single markdown table row while preserving Slack link cells. */
8+
export function parseMarkdownTableRow(line: string): string[] | null {
9+
if (!line.includes("|")) {
10+
return null;
11+
}
12+
13+
const normalized = line.trim().replace(/^\|/, "").replace(/\|$/, "");
14+
const cells: string[] = [];
15+
let current = "";
16+
let insideSlackLink = false;
17+
18+
for (const char of normalized) {
19+
if (char === "<") {
20+
insideSlackLink = true;
21+
current += char;
22+
continue;
23+
}
24+
if (char === ">") {
25+
insideSlackLink = false;
26+
current += char;
27+
continue;
28+
}
29+
if (char === "|" && !insideSlackLink) {
30+
cells.push(current.trim());
31+
current = "";
32+
continue;
33+
}
34+
current += char;
35+
}
36+
37+
cells.push(current.trim());
38+
return cells.length >= 2 ? cells : null;
39+
}
40+
41+
/** Return true when a line is a markdown table separator row. */
42+
export function isMarkdownTableSeparator(line: string): boolean {
43+
const cells = parseMarkdownTableRow(line);
44+
return (
45+
cells !== null &&
46+
cells.length >= 2 &&
47+
cells.every((cell) => /^:?-{3,}:?$/.test(cell))
48+
);
49+
}
50+
51+
/** Render parsed markdown-table rows into a Slack-safe ASCII code block. */
52+
export function renderMarkdownTableCodeBlock(rows: string[][]): string {
53+
const columnCount = Math.max(...rows.map((row) => row.length));
54+
const normalizedRows = rows.map((row) =>
55+
Array.from({ length: columnCount }, (_unused, index) => row[index] ?? ""),
56+
);
57+
const widths = Array.from({ length: columnCount }, (_unused, index) =>
58+
Math.max(3, ...normalizedRows.map((row) => (row[index] ?? "").length)),
59+
);
60+
61+
const formatRow = (row: string[]) =>
62+
row
63+
.map((cell, index) => cell.padEnd(widths[index] ?? 3))
64+
.join(" | ")
65+
.trimEnd();
66+
67+
const header = formatRow(normalizedRows[0] ?? []);
68+
const separator = widths
69+
.map((width) => "-".repeat(Math.max(3, width)))
70+
.join(" | ");
71+
const body = normalizedRows.slice(1).map(formatRow);
72+
73+
return ["```", header, separator, ...body, "```"].join("\n");
74+
}
75+
76+
/** Extract the first simple markdown table plus surrounding text. */
77+
export function extractFirstMarkdownTable(
78+
text: string,
79+
): MarkdownTableMatch | null {
80+
const lines = text.split("\n");
81+
82+
for (let index = 0; index < lines.length; index += 1) {
83+
const header = parseMarkdownTableRow(lines[index] ?? "");
84+
if (!header || !isMarkdownTableSeparator(lines[index + 1] ?? "")) {
85+
continue;
86+
}
87+
88+
const rows = [header];
89+
let nextIndex = index + 2;
90+
while (nextIndex < lines.length) {
91+
const row = parseMarkdownTableRow(lines[nextIndex] ?? "");
92+
if (!row) {
93+
break;
94+
}
95+
rows.push(row);
96+
nextIndex += 1;
97+
}
98+
99+
return {
100+
after: lines.slice(nextIndex).join("\n"),
101+
before: lines.slice(0, index).join("\n"),
102+
rows,
103+
};
104+
}
105+
106+
return null;
107+
}

‎packages/junior/src/chat/slack/mrkdwn.ts‎

Lines changed: 104 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
11
import { truncateStatusText } from "@/chat/runtime/status-format";
2+
import {
3+
isMarkdownTableSeparator,
4+
parseMarkdownTableRow,
5+
renderMarkdownTableCodeBlock,
6+
} from "@/chat/slack/markdown-table";
27

38
const PROTECTED_SLACK_SEGMENT_PATTERN =
49
/(```[\s\S]*?```|~~~[\s\S]*?~~~|`[^`\n]+`)/g;
@@ -9,7 +14,8 @@ function normalizeSlackMarkdownSegment(text: string): string {
914
normalized = normalizeCommonMarkEmphasis(normalized);
1015
normalized = normalizeMarkdownLinks(normalized);
1116
normalized = normalizeWrappedRawUrls(normalized);
12-
return normalizeMarkdownTables(normalized);
17+
normalized = normalizeMarkdownTables(normalized);
18+
return escapeSlackControlChars(normalized);
1319
}
1420

1521
function normalizeUnprotectedSlackMarkdown(text: string): string {
@@ -382,72 +388,6 @@ function normalizeWrappedRawUrls(text: string): string {
382388
return `${out}${text.slice(lastIndex)}`;
383389
}
384390

385-
function parseMarkdownTableRow(line: string): string[] | null {
386-
if (!line.includes("|")) {
387-
return null;
388-
}
389-
390-
const normalized = line.trim().replace(/^\|/, "").replace(/\|$/, "");
391-
const cells: string[] = [];
392-
let current = "";
393-
let insideSlackLink = false;
394-
395-
for (const char of normalized) {
396-
if (char === "<") {
397-
insideSlackLink = true;
398-
current += char;
399-
continue;
400-
}
401-
if (char === ">") {
402-
insideSlackLink = false;
403-
current += char;
404-
continue;
405-
}
406-
if (char === "|" && !insideSlackLink) {
407-
cells.push(current.trim());
408-
current = "";
409-
continue;
410-
}
411-
current += char;
412-
}
413-
414-
cells.push(current.trim());
415-
return cells.length >= 2 ? cells : null;
416-
}
417-
418-
function isMarkdownTableSeparator(line: string): boolean {
419-
const cells = parseMarkdownTableRow(line);
420-
return (
421-
cells !== null &&
422-
cells.length >= 2 &&
423-
cells.every((cell) => /^:?-{3,}:?$/.test(cell))
424-
);
425-
}
426-
427-
function renderMarkdownTableCodeBlock(rows: string[][]): string {
428-
const columnCount = Math.max(...rows.map((row) => row.length));
429-
const normalizedRows = rows.map((row) =>
430-
Array.from({ length: columnCount }, (_unused, index) => row[index] ?? ""),
431-
);
432-
const widths = Array.from({ length: columnCount }, (_unused, index) =>
433-
Math.max(3, ...normalizedRows.map((row) => (row[index] ?? "").length)),
434-
);
435-
436-
const formatRow = (row: string[]) =>
437-
row
438-
.map((cell, index) => cell.padEnd(widths[index] ?? 3))
439-
.join(" | ")
440-
.trimEnd();
441-
442-
const header = formatRow(normalizedRows[0] ?? []);
443-
const separator = widths
444-
.map((width) => "-".repeat(Math.max(3, width)))
445-
.join(" | ");
446-
const body = normalizedRows.slice(1).map(formatRow);
447-
448-
return ["```", header, separator, ...body, "```"].join("\n");
449-
}
450-
451391
function normalizeMarkdownTables(text: string): string {
452392
const lines = text.split("\n");
453393
const out: string[] = [];
@@ -477,6 +417,103 @@ function normalizeMarkdownTables(text: string): string {
477417
return out.join("\n");
478418
}
479419

420+
function isPreservedSlackEntity(text: string, index: number): number {
421+
if (text.startsWith("&amp;", index)) {
422+
return 5;
423+
}
424+
if (text.startsWith("&lt;", index) || text.startsWith("&gt;", index)) {
425+
return 4;
426+
}
427+
return 0;
428+
}
429+
430+
function isSlackBlockQuoteMarker(text: string, index: number): boolean {
431+
if (text[index] !== ">") {
432+
return false;
433+
}
434+
435+
let cursor = index - 1;
436+
while (cursor >= 0 && text[cursor] !== "\n") {
437+
if (!/\s/.test(text[cursor] ?? "")) {
438+
return false;
439+
}
440+
cursor -= 1;
441+
}
442+
return true;
443+
}
444+
445+
function isPreservedSlackToken(token: string): boolean {
446+
return (
447+
/^<https?:\/\/[^>\s]+(?:\|[^>]+)?>$/.test(token) ||
448+
/^<mailto:[^>\s]+(?:\|[^>]+)?>$/.test(token) ||
449+
/^<@[^>|]+(?:\|[^>]+)?>$/.test(token) ||
450+
/^<#[^>|]+(?:\|[^>]+)?>$/.test(token) ||
451+
/^<!(?!-)[^>]+>$/.test(token)
452+
);
453+
}
454+
455+
function escapeSlackControlCharsSegment(text: string): string {
456+
let out = "";
457+
458+
for (let index = 0; index < text.length; index += 1) {
459+
const char = text[index];
460+
if (char === "&") {
461+
const entityLength = isPreservedSlackEntity(text, index);
462+
if (entityLength > 0) {
463+
out += text.slice(index, index + entityLength);
464+
index += entityLength - 1;
465+
continue;
466+
}
467+
out += "&amp;";
468+
continue;
469+
}
470+
471+
if (char === "<") {
472+
const closeIndex = text.indexOf(">", index + 1);
473+
if (closeIndex !== -1) {
474+
const token = text.slice(index, closeIndex + 1);
475+
if (isPreservedSlackToken(token)) {
476+
out += token;
477+
index = closeIndex;
478+
continue;
479+
}
480+
}
481+
482+
out += "&lt;";
483+
continue;
484+
}
485+
486+
if (char === ">") {
487+
out += isSlackBlockQuoteMarker(text, index) ? ">" : "&gt;";
488+
continue;
489+
}
490+
491+
out += char;
492+
}
493+
494+
return out;
495+
}
496+
497+
function escapeSlackControlChars(text: string): string {
498+
let out = "";
499+
let lastIndex = 0;
500+
501+
for (const match of text.matchAll(PROTECTED_SLACK_SEGMENT_PATTERN)) {
502+
const index = match.index ?? 0;
503+
if (index > lastIndex) {
504+
out += escapeSlackControlCharsSegment(text.slice(lastIndex, index));
505+
}
506+
out += match[0];
507+
lastIndex = index + match[0].length;
508+
}
509+
510+
if (lastIndex < text.length) {
511+
out += escapeSlackControlCharsSegment(text.slice(lastIndex));
512+
}
513+
514+
return out;
515+
}
516+
480517
/** Insert blank lines between content blocks so Slack renders them with visual separation. */
481518
function ensureBlockSpacing(text: string): string {
482519
const codeBlockPattern = /^```/;

0 commit comments

Comments
 (0)