Skip to content

Commit 7a6a855

Browse files
committed
Add outgoing-text sanitization before mdToMrkdwn (#8)
sanitizeOutgoingText strips a single wrapping HTML tag, drops a leading leaked JSON/tool-call fence followed by real prose, unwraps a whole-reply fence around plain prose, and un-escapes markdown escape sequences. detectInfraErrorReply recognizes a raw infra-error dump (vs. prose that merely mentions one) so resolveOutgoingText can swap in a neutral, user-actionable message instead of letting it reach Slack verbatim; the original is logged for diagnosis. Wired into toTagThread's post() ahead of the convertMarkdown branch — sanitization targets text that should never have reached Slack in that shape at all, so it runs regardless of whether markdown conversion is requested.
1 parent 82b70c9 commit 7a6a855

4 files changed

Lines changed: 454 additions & 1 deletion

File tree

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { detectInfraErrorReply, sanitizeOutgoingText } from "./sanitize.ts";
3+
4+
describe("sanitizeOutgoingText", () => {
5+
test("strips a single wrapping <div> tag, keeping inner text (artifact 1)", () => {
6+
const input =
7+
'<div dir="auto">Diligence brief for Harvey AI initiated. Run ID: abc123</div>';
8+
expect(sanitizeOutgoingText(input)).toBe(
9+
"Diligence brief for Harvey AI initiated. Run ID: abc123",
10+
);
11+
});
12+
13+
test("unwraps a whole-reply ```json fence around prose (artifact 2)", () => {
14+
const input =
15+
"```json\nThe diligence brief for Harvey AI is ready. Let me know if you want the full PDF.\n```";
16+
expect(sanitizeOutgoingText(input)).toBe(
17+
"The diligence brief for Harvey AI is ready. Let me know if you want the full PDF.",
18+
);
19+
});
20+
21+
test("unescapes backslash-escaped underscores (artifact 3)", () => {
22+
const input = "ins\\_dep\\_ping-ai@scout.localhost";
23+
expect(sanitizeOutgoingText(input)).toBe("ins_dep_ping-ai@scout.localhost");
24+
});
25+
26+
test("passes an infra-error message through unchanged (wire.ts swaps it, not sanitize)", () => {
27+
const input =
28+
"This agent could not complete your request due to an unrecoverable inference error [HTTP 400]: invalid message content type: <nil>";
29+
expect(sanitizeOutgoingText(input)).toBe(input);
30+
});
31+
32+
test("drops a leading fenced JSON input-echo block before real prose (artifact 5)", () => {
33+
const input =
34+
'```json\n{"company":"ping ai","threadRef":"C1:123.456"}\n```\nDiligence brief for ping ai is underway.';
35+
expect(sanitizeOutgoingText(input)).toBe(
36+
"Diligence brief for ping ai is underway.",
37+
);
38+
});
39+
40+
test("passes a normal mrkdwn reply with links/bold through byte-identical", () => {
41+
const input =
42+
"Here's the update: *bold point*, see <https://example.com|the doc> for details.";
43+
expect(sanitizeOutgoingText(input)).toBe(input);
44+
});
45+
46+
test("leaves a legitimate whole-reply JSON/code fence untouched", () => {
47+
const input = '```json\n{"status":"ok","count":3}\n```';
48+
expect(sanitizeOutgoingText(input)).toBe(input);
49+
});
50+
51+
test("leaves a legitimate leading JSON fence untouched when the rest isn't prose-shaped follow-up (still just data)", () => {
52+
const input = '```json\n{"a":1}\n```\n```json\n{"b":2}\n```';
53+
expect(sanitizeOutgoingText(input)).toBe(input);
54+
});
55+
56+
test("trims surrounding whitespace", () => {
57+
expect(sanitizeOutgoingText(" hello there \n")).toBe("hello there");
58+
});
59+
60+
test("leaves a whole-reply fence alone when its content is structured but unparseable (single-quoted pseudo-JSON)", () => {
61+
const input = "```json\n{status: 'ok', count: 3}\n```";
62+
expect(sanitizeOutgoingText(input)).toBe(input);
63+
});
64+
});
65+
66+
describe("detectInfraErrorReply", () => {
67+
test("matches the observed unrecoverable inference error text (artifact 4)", () => {
68+
const input =
69+
"This agent could not complete your request due to an unrecoverable inference error [HTTP 400]: invalid message content type: <nil>";
70+
expect(detectInfraErrorReply(input)).toBe(true);
71+
});
72+
73+
test("matches a bracketed HTTP status that dominates the whole reply", () => {
74+
expect(detectInfraErrorReply("[HTTP 503]")).toBe(true);
75+
});
76+
77+
test("matches a crash-mid-invocation marker at the start of the reply", () => {
78+
expect(detectInfraErrorReply("crash-mid-invocation while streaming")).toBe(
79+
true,
80+
);
81+
});
82+
83+
test("does not match a normal reply", () => {
84+
expect(
85+
detectInfraErrorReply("Here's the diligence brief you asked for."),
86+
).toBe(false);
87+
});
88+
89+
test("does not swallow prose that merely QUOTES an error (reviewer repro 1)", () => {
90+
const input =
91+
"Per the incident report [HTTP 503] the vendor's endpoint was down… but service has since recovered.";
92+
expect(detectInfraErrorReply(input)).toBe(false);
93+
});
94+
95+
test("does not swallow prose that USEFULLY REPORTS a failure (reviewer repro 2)", () => {
96+
const input =
97+
"Diligence run finished with an unrecoverable inference error while summarizing the 10-K; you may want to retry.";
98+
expect(detectInfraErrorReply(input)).toBe(false);
99+
});
100+
101+
test("still replaces the verbatim raw failure dump (artifact 4)", () => {
102+
const input =
103+
"This agent could not complete your request due to an unrecoverable inference error [HTTP 400]: invalid message content type: <nil>";
104+
expect(detectInfraErrorReply(input)).toBe(true);
105+
});
106+
});

‎packages/tag-slack/src/sanitize.ts‎

Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,258 @@
1+
/**
2+
* `sanitizeOutgoingText` / `detectInfraErrorReply` — a defensive cleanup pass
3+
* for outgoing `TagThread.post()` text, applied in `wire.ts`'s `toTagThread`
4+
* BEFORE `mdToMrkdwn` (see `mrkdwn.ts`).
5+
*
6+
* This exists because upstream agent output occasionally carries artifacts
7+
* that were never meant for a human reader — raw HTML wrapper tags from a
8+
* browser-oriented renderer, a JSON tool-call payload leaking ahead of the
9+
* real prose reply, markdown escape sequences (`\_`, `\*`, `` \` ``) that
10+
* read as noise once rendered in Slack, and raw infra-error text (stack-ish
11+
* "[HTTP 400]" style messages) that shouldn't reach a Slack channel verbatim.
12+
* None of these are things `mdToMrkdwn` is responsible for — that function
13+
* converts *legitimate* markdown to mrkdwn; this module removes text that
14+
* should never have been markdown (or HTML, or an error dump) in the first
15+
* place.
16+
*
17+
* Heuristics, not a parser: this module makes a best effort using cheap
18+
* regex/ratio checks, favoring "leave it alone" when unsure — mangling a
19+
* legitimate code block a user asked for is worse than leaving one stray
20+
* artifact untouched.
21+
*/
22+
23+
/**
24+
* Matches an entire string that is exactly one HTML element: an opening
25+
* `<div>`/`<span>`/`<p>` tag (with optional attributes), inner content, and
26+
* the matching closing tag. Anchored on both ends and non-greedy on the
27+
* inner content so a *single* wrapping pair is stripped — nested tags of the
28+
* same kind inside the content are left alone; this module only peels away
29+
* one outer wrapper, matching the observed artifact
30+
* (`<div dir="auto">...</div>`), not general HTML sanitization.
31+
*/
32+
const WRAPPING_HTML_TAG_PATTERN =
33+
/^<(div|span|p)(?:\s[^>]*)?>([\s\S]*)<\/\1>$/i;
34+
35+
/** Strips a single wrapping `<div>`/`<span>`/`<p>` pair, keeping inner text. */
36+
function stripWrappingHtmlTag(text: string): string {
37+
const match = WRAPPING_HTML_TAG_PATTERN.exec(text.trim());
38+
return match ? (match[2] ?? "") : text;
39+
}
40+
41+
/**
42+
* Matches a fenced code block (```lang\n...\n```) that spans the ENTIRE
43+
* string, capturing the language tag (if any) and the fence's inner content.
44+
*/
45+
const WHOLE_FENCE_PATTERN = /^```(\w*)\n?([\s\S]*?)\n?```$/;
46+
47+
/**
48+
* Matches a fenced code block at the very START of the string, followed by
49+
* one or more remaining, non-empty characters (the "real" reply).
50+
*/
51+
const LEADING_FENCE_PATTERN = /^```(\w*)\n?([\s\S]*?)\n?```\s*\n*([\s\S]*)$/;
52+
53+
/**
54+
* Ratio of JSON/code "punctuation" characters (braces, brackets, colons,
55+
* quotes used as delimiters, angle brackets) to non-whitespace characters.
56+
* Prose runs low (stray punctuation only); JSON/code payloads run high
57+
* because nearly every token is wrapped in structural symbols.
58+
*/
59+
function symbolRatio(content: string): number {
60+
const chars = content.replace(/\s/g, "");
61+
if (chars.length === 0) return 0;
62+
const symbols = (chars.match(/[{}[\]":,;<>]/g) ?? []).length;
63+
return symbols / chars.length;
64+
}
65+
66+
/**
67+
* Best-effort "is this JSON or code, not prose" check: valid JSON always
68+
* counts, otherwise falls back to the symbol-ratio heuristic above. Used to
69+
* decide whether a fence is a data/tool-call payload (leave it, or drop it
70+
* as a leading artifact) vs. an agent's prose reply that got accidentally
71+
* wrapped in a fence (unwrap it).
72+
*/
73+
function looksLikeJsonOrCode(content: string): boolean {
74+
const trimmed = content.trim();
75+
if (trimmed.length === 0) return false;
76+
if (trimmed.startsWith("{") || trimmed.startsWith("[")) {
77+
try {
78+
JSON.parse(trimmed);
79+
return true;
80+
} catch {
81+
// Not valid JSON — fall through to the ratio heuristic below rather
82+
// than assuming prose just because parsing failed (e.g. truncated
83+
// JSON is still clearly not prose).
84+
}
85+
}
86+
return symbolRatio(trimmed) > 0.15;
87+
}
88+
89+
/**
90+
* Whether `content` LOOKS structured (wrapped in `{...}`/`[...]`) but is NOT
91+
* valid JSON — e.g. single-quoted pseudo-JSON like `{status: 'ok', count:
92+
* 3}`. This is deliberately its own category, distinct from both
93+
* `looksLikeJsonOrCode` (true JSON/code) and prose: it is ambiguous enough
94+
* (a model's mangled attempt at structured output? a deliberately-shared
95+
* snippet?) that guessing either way risks mangling real content. Callers
96+
* that must pick between "safe to unwrap/drop" and "leave alone" should
97+
* treat this as "leave alone" — see `unwrapProseFence`.
98+
*/
99+
function looksStructuredButUnparseable(content: string): boolean {
100+
const trimmed = content.trim();
101+
if (trimmed.length === 0) return false;
102+
const isBraceWrapped = trimmed.startsWith("{") && trimmed.endsWith("}");
103+
const isBracketWrapped = trimmed.startsWith("[") && trimmed.endsWith("]");
104+
if (!isBraceWrapped && !isBracketWrapped) return false;
105+
try {
106+
JSON.parse(trimmed);
107+
return false; // valid JSON — that's `looksLikeJsonOrCode`'s territory, not this.
108+
} catch {
109+
return true;
110+
}
111+
}
112+
113+
/**
114+
* Drops a LEADING fenced block when it looks like a JSON/tool-call payload
115+
* AND there is non-empty prose after it — the observed "input-echo" leak
116+
* (artifact 5): a `{"company":...}` block the model echoed back before its
117+
* real answer. A leading fence with no prose after it (the whole reply is
118+
* the fence) is left for `unwrapProseFence`/left alone entirely — dropping
119+
* it here would silently discard the only content in the reply.
120+
*/
121+
function dropLeadingFencedJsonBlock(text: string): string {
122+
const match = LEADING_FENCE_PATTERN.exec(text.trim());
123+
if (!match) return text;
124+
const [, , fenceContent, rest] = match;
125+
if (rest === undefined || rest.trim().length === 0) return text;
126+
if (!looksLikeJsonOrCode(fenceContent ?? "")) return text;
127+
// The remainder must itself be prose, not more JSON/code (e.g. two data
128+
// blocks in a row) — otherwise this isn't the "input echo before the real
129+
// answer" case, and dropping the first block would just discard data.
130+
if (looksLikeJsonOrCode(rest)) return text;
131+
return rest.trim();
132+
}
133+
134+
/**
135+
* Unwraps a fence that spans the WHOLE reply when its content reads as
136+
* prose, not JSON/code — the observed "replies wrapped in ```json fences"
137+
* artifact (artifact 2): the model tagged a plain-English answer as a
138+
* ```json (or plain ```) block for no structural reason. A whole-reply fence
139+
* whose content genuinely looks like JSON/code is left untouched — that's
140+
* legitimate content a user asked for (a code sample, a payload dump),
141+
* distinguishable from the mistake this targets only by asking "is this
142+
* actually prose".
143+
*/
144+
function unwrapProseFence(text: string): string {
145+
const match = WHOLE_FENCE_PATTERN.exec(text.trim());
146+
if (!match) return text;
147+
const content = match[2] ?? "";
148+
if (looksLikeJsonOrCode(content)) return text;
149+
// Ambiguous structured-but-unparseable content (e.g. single-quoted
150+
// pseudo-JSON) — bias to no-op rather than guess it's prose. See
151+
// `looksStructuredButUnparseable`.
152+
if (looksStructuredButUnparseable(content)) return text;
153+
return content.trim();
154+
}
155+
156+
/**
157+
* Matches a backslash-escaped `_`, `*`, or `` ` `` — markdown escape
158+
* sequences some model output emits (e.g. `ins\_dep\_ping-ai@scout.localhost`,
159+
* artifact 3) that read as literal noise once posted to Slack, which has no
160+
* such escaping convention of its own. Deliberately narrow: only these three
161+
* characters, so an unrelated backslash (e.g. in a Windows path or regex a
162+
* user is legitimately sharing) is never touched.
163+
*/
164+
const MARKDOWN_ESCAPE_PATTERN = /\\([_*`])/g;
165+
166+
/**
167+
* Un-escapes `\_`, `\*`, `` \` `` sequences back to their plain characters.
168+
*
169+
* Known tradeoff, accepted: un-escaping `\*` can turn a genuinely-intended
170+
* literal asterisk (someone escaped it on purpose, e.g. `2 \* 3`) into
171+
* mrkdwn bold once `mdToMrkdwn` sees the bare `*`. Every artifact actually
172+
* observed reaching Slack was an escaped underscore in an address/identifier
173+
* (`ins\_dep\_ping-ai@scout.localhost`), never an intentional escape — so
174+
* this optimizes for the artifact that's actually happening rather than a
175+
* hypothetical one.
176+
*/
177+
function unescapeMarkdownEscapes(text: string): string {
178+
return text.replace(MARKDOWN_ESCAPE_PATTERN, "$1");
179+
}
180+
181+
/**
182+
* Cleans up known model-output artifacts before a reply is handed to
183+
* `mdToMrkdwn` and posted to Slack: a single wrapping HTML tag, a leading
184+
* JSON/tool-call fence followed by prose, a whole-reply fence wrapping plain
185+
* prose, and backslash-escaped markdown punctuation. Order matters — HTML
186+
* unwrapping first (so a `<div>` wrapping a fenced block still gets the
187+
* fence handling), then the leading-fence drop (more specific: fence +
188+
* trailing prose) before the whole-fence unwrap (fence with nothing else),
189+
* then escape cleanup, then a final trim.
190+
*
191+
* Pure function: no I/O, safe to call on every outgoing message. A normal
192+
* mrkdwn reply — links, bold, no stray fences/tags/escapes — passes through
193+
* unchanged (aside from the trailing `.trim()`).
194+
*/
195+
export function sanitizeOutgoingText(text: string): string {
196+
const withoutHtmlWrap = stripWrappingHtmlTag(text);
197+
const withoutLeadingJsonFence = dropLeadingFencedJsonBlock(withoutHtmlWrap);
198+
const withoutWholeFenceWrap = unwrapProseFence(withoutLeadingJsonFence);
199+
const unescaped = unescapeMarkdownEscapes(withoutWholeFenceWrap);
200+
return unescaped.trim();
201+
}
202+
203+
/**
204+
* Matches infra-error-shaped substrings: an "unrecoverable inference error",
205+
* a bracketed HTTP status (`[HTTP 400]`), or a mid-invocation crash marker.
206+
* On its own this is NOT sufficient to call something a raw failure dump —
207+
* see `detectInfraErrorReply`, which requires this to dominate the reply,
208+
* not merely appear in it.
209+
*/
210+
const INFRA_ERROR_PATTERN =
211+
/unrecoverable inference error|\[HTTP \d{3}\]|crash-mid-invocation/i;
212+
213+
/**
214+
* Full-shape signatures: the reply doesn't just MENTION an infra error, it
215+
* IS one — a raw failure dump verbatim from the runtime, not the model's own
216+
* prose. Anchored to the start of the (trimmed) text, not a bare substring
217+
* match, precisely so legitimate prose that happens to open with something
218+
* else first is never caught here just because it goes on to reference an
219+
* error later.
220+
*/
221+
const FULL_DUMP_SIGNATURES: RegExp[] = [
222+
/^This agent could not complete your request due to an unrecoverable inference error\b/i,
223+
/^crash-mid-invocation\b/i,
224+
];
225+
226+
/** Above this fraction of the trimmed text, an error-shaped match is presumed to BE the reply, not merely mentioned by it. */
227+
const DOMINANCE_THRESHOLD = 0.6;
228+
229+
/**
230+
* Whether `text` IS a raw infra-error dump that should never reach a Slack
231+
* user verbatim — as opposed to ordinary prose that merely QUOTES or
232+
* REPORTS an error, which must pass through untouched (a diligence run
233+
* usefully telling a user "this failed because of an unrecoverable
234+
* inference error, you may want to retry" is exactly the kind of message
235+
* this must NOT swallow).
236+
*
237+
* Two ways to qualify: the trimmed text starts with a known full-dump
238+
* signature (see `FULL_DUMP_SIGNATURES`), OR the error-shaped substring
239+
* itself accounts for more than `DOMINANCE_THRESHOLD` of the trimmed text
240+
* — i.e. the match effectively IS the message, not a clause within a much
241+
* longer sentence. A short "[HTTP 503] gateway timeout" is almost entirely
242+
* its error signature and gets replaced; "Per the incident report [HTTP
243+
* 503] the vendor's endpoint was down, but service has since recovered" has
244+
* the same substring inside a much longer, unrelated sentence and is left
245+
* alone.
246+
*/
247+
export function detectInfraErrorReply(text: string): boolean {
248+
const trimmed = text.trim();
249+
if (trimmed.length === 0) return false;
250+
251+
if (FULL_DUMP_SIGNATURES.some((pattern) => pattern.test(trimmed))) {
252+
return true;
253+
}
254+
255+
const match = INFRA_ERROR_PATTERN.exec(trimmed);
256+
if (!match) return false;
257+
return match[0].length / trimmed.length > DOMINANCE_THRESHOLD;
258+
}

0 commit comments

Comments
 (0)