fix(chat): normalize zero-width sentinel prefixes

[lilyshen0722]

Summary

• normalize a decision-only Unicode Cf copy before NO_REPLY total-match and leading-sentinel suppression
• cover leading format-character prefixes without changing substantive output
• preserve U+200D emoji joins in posted content

Verification

• npm test -- --runInBand __tests__/unit/services/agentMessageService.chatNoise.test.js (21 passed)
• npm run tsc:check
• Mutations: removing the decision normalizer leaks the format-character-prefix case; applying Unicode-format normalization to outbound content splits the family emoji regression.

Stacked on #1321; GitHub will retarget this PR when the base merges.

[lilyshen0722]

Gated at ceba08cd (base refs/pull/1321/head). The reported hole is closed and the test is real — but I'd hold this for one change: the normalizer is applied to the content that gets posted, not just to the content the sentinel checks look at, and that breaks ZWJ emoji.

What holds

Single normalization point feeding trimmed, so both sentinel checks and the strip loop see the same string by construction — that is the right shape and it avoids the one-predicate trap.

Suite is 20/20 on Node 22. Mutation: reverting stripped.replace(ZERO_WIDTH_FORMAT_CHARACTERS, '').trim() to stripped.trim() fails exactly one test — normalizes zero-width prefixes before both sentinel checks — 1 failed / 19 passed. Non-vacuous.

Finding 1 — the normalized string is what gets posted

trimmed is not a decision-only copy. :1854 returns it directly for a fenced reply, and :1948 builds cleaned character-by-character out of it. So every agent message now loses these characters from its content, and U+200D is load-bearing in composed emoji:

in :  👨‍👩‍👧  (5 code points)
out:  👨👩👧    (3 code points)

A seat that posts a family emoji gets three separate people. Skin-tone modifiers are unaffected (no ZWJ), so this is narrow but real, and it is silent.

The fix is small: normalize a copy for the sentinel decisions and keep the original for the returned content. That also resolves the general form of the concern — a sanitizer whose job is deciding whether to suppress should not be rewriting what it does not suppress.

Finding 2 — the class is wider than the range

[​-‍] closes the four characters I reported and misses others in the same category. Probed against both checks at this head:

U+200B ✓ fixed   U+200C ✓ fixed   U+200D ✓ fixed   U+FEFF ✓ fixed
U+00A0 ✓ (trim() already handled it — \s covers NBSP)
U+2060 WORD JOINER          ✗ still suppresses=false
U+061C ARABIC LETTER MARK   ✗ still suppresses=false
U+200E LEFT-TO-RIGHT MARK   ✗ still suppresses=false
U+2062 INVISIBLE TIMES      ✗ still suppresses=false

\p{Cf} covers every one of them — I checked all nine plus U+00AD, and 'Hello world' is unchanged by it. Once finding 1 is fixed and the strip applies only to the decision copy, \p{Cf} becomes the safe choice precisely because the ZWJ concern no longer touches output.

Worth naming the shape: my own advice on the row was fix the normalization, not one predicate. This is the same error one level up — one range normalized rather than the category. The next invisible character reported would need a third edit to the same constant.

Not verified

Whether any real producer emits any of these — still the open reachability question from TASK-085, and it bears on urgency, not on correctness. I also ran only the chat-noise file, not the full backend suite, and I did not check whether anything downstream depends on sanitizeAgentContent returning byte-identical input for non-sentinel content.

[lilyshen0722]

Gate — 35e4a1af. Both findings fixed; approving on the merits. (Note: this landed as a force-push over ceba08cd, not an added commit — git merge-base --is-ancestor ceba08cd pr/1327 is false. Re-fetch with +refs/pull/1327/head before reviewing; a plain git fetch silently declines the non-fast-forward and leaves you reading the old head.)

Verified at 35e4a1af, Node 22, single Lily-authored commit, Test & Coverage green (UNSTABLE is the non-required Service Tests still pending):

• Output mutation is gone. trimmed is the output path; sentinelContent is decision-only. 'ship it 👨‍👩‍👧' round-trips with U+200D intact — the family emoji stays one glyph.
• Range now matches the category. /\p{Cf}/gu catches the prefixes the old [​-‍] missed; I confirmed U+00AD (soft hyphen) suppresses, which the previous range did not.
• Both reported mutations reproduce, and each kills a different test — so the two assertions are independently load-bearing, not one test double-counted:
  • sentinelContent = trimmed → normalizes Unicode format-character prefixes before both sentinel checks red (20/21).
  • trimmed = stripped.replace(FORMAT_CHARACTERS,'').trim() → keeps zero-width joiners in substantive agent output red (20/21).
  • Baseline 21/21, tree clean between mutants.

One residual gap, deliberately not blocking — but worth a line in the comment so the next editor doesn't read the normalization as total. A format character inside the token (NO_<ZWSP>REPLY) still defeats the mid/trailing strip loop at :1917-1948, which scans trimmed, not sentinelContent:

'Shipped.\n\nNO_<ZWSP>REPLY'                    -> 'Shipped.\n\nNO_<ZWSP>REPLY'   (token posts verbatim)
'Shipped.\n\nNO_<ZWSP>REPLY\n\nprivate reasoning' -> unchanged

This is cosmetic, not a suppression bypass, and that distinction is the reason I'm not holding. The strip loop's job is removing a leaked token, never silencing what follows it — the clean control proves it: 'Shipped.\n\nNO_REPLY\n\nprivate reasoning' → 'Shipped.\n\n\n\nprivate reasoning'. The reasoning posts either way. Every path that can actually suppress private reasoning (total-match, leading-run) now reads the normalized copy, so the security boundary is closed. Aligning the strip loop would mean index arithmetic across two strings of different length — real bug surface for a cosmetic win. Leave it un-normalized; just say so in the comment above FORMAT_CHARACTERS.

Not verified: whether any producer has ever emitted a format character in this position at all — the reachability question stays open on TASK-085 and this PR doesn't close it. Also unverified: behaviour of the fenced/outerFence early return under format characters beyond the one case I probed (see `NO_REPLY` here survives, as intended).

[lilyshen0722]

GATE: PASS at d634b4b21 — the fix is correct, the class choice is right, and I would press it. One finding below that is not blocking but should not ship unrecorded.

My prior gate on this PR (35e4a1af) is void: 35e4a1af9 is not an ancestor of d634b4b21, so the retarget rewrote history. Re-measured from scratch. Base is current main, 0 behind, 1 commit.

Verified

Ran the sentinel decision against 11 Cf code points, 8 non-Cf invisibles, and three controls, at this head:

• All 11 Cf code points are closed — including U+180E and U+2064, which the PR's test does not enumerate. Normalizing the class rather than a list is what makes those free, and it is the right call.
• Controls hold: plain NO_REPLY silent, substantive text posts, and the ZWJ emoji family 👨‍👩‍👧 survives intact — the decision-only copy is doing its job, the payload is not rewritten.
• Their suite is 21/21 at this head.

Finding — the residual is the same leak, not a lesser one

Eight invisible characters outside Cf still defeat both paths. What posts is not a harmless artifact:

U+3164 HANGUL FILLER  + NO_REPLY + prose  ->  POSTS "ㅤ\n\nprivate reasoning"
U+115F, U+1160, U+FFA0 (Hangul fillers)   ->  POSTS the reasoning
U+034F COMBINING GRAPHEME JOINER          ->  POSTS the reasoning
U+17B4 KHMER VOWEL INHERENT AQ            ->  POSTS the reasoning

The sentinel is stripped and the private rationale is published — AX entry 43 exactly. U+3164 in particular is not exotic; it is the standard invisible character used for name spoofing, and it is one paste away from any model's output.

This is pre-existing and not a regression, so it does not block. But the code comment reads as though the class is the boundary of the problem — "a one-predicate fix would re-open the same private-reasoning leak" — and the next reader will take the leak as closed. That framing is the part I would not ship.

Recommendation, measured rather than proposed

Swap one character class:

-const FORMAT_CHARACTERS = /\p{Cf}/gu;
+const FORMAT_CHARACTERS = /\p{Default_Ignorable_Code_Point}/gu;

Default_Ignorable_Code_Point is a superset of Cf and closes 6 of the 8 — every Hangul filler, the CGJ, and the Khmer AQ. I applied it in place at this head and ran your suite: 21/21, unchanged, emoji test included (the strip is on the decision copy, so ZWJ still joins in the payload). Then reverted.

The two that remain, U+0300 COMBINING GRAVE and U+2800 BRAILLE PATTERN BLANK, are genuinely rendering characters rather than ignorable ones. I would leave them and say so in the comment, rather than widen further and start eating real text.

Either land the one-liner here or take it as a follow-up — but the comment should name the residual either way, so nobody reads this as the end of the class of bug.

Not verified: I tested the leading-prefix position only, not sentinels appearing mid-string, and not the fenced-block path. I did not re-run the full backend suite at this head, only the touched one plus my probe. Test & Coverage and E2E Tests were still pending when I measured, so this gate is on the code, not on green CI.

[lilyshen0722]

Gate restated at e23624960 — still PASS. No re-run needed, and here is the specific reason.

The head moved from d634b4b21 (where I gated at 01:48Z) to e23624960, and d634b4b21 is not an ancestor — history was rewritten. Normally that voids my gate, as it did for the 35e4a1af → d634b4b21 move earlier.

It does not here, because the reviewed content is bit-identical rather than merely similar. Both touched files resolve to the same blob at both heads:

backend/services/agentMessageService.ts
  d634b4b21 -> 8ab44b473721f5258f13a97a7211d0fb3684ee14
  e23624960 -> 8ab44b473721f5258f13a97a7211d0fb3684ee14
backend/__tests__/unit/services/agentMessageService.chatNoise.test.js
  d634b4b21 -> a3c918e19046480528c25047a2068a8dea3ebcc8
  e23624960 -> a3c918e19046480528c25047a2068a8dea3ebcc8

git diff d634b4b21 e23624960 -- backend/services backend/__tests__/unit/services is empty. The rebase moved the base, not the change. So the measurements in my previous review — 11/11 Cf code points suppressed, emoji ZWJ preserved, suite 21/21 — transfer as findings about these blobs, not as recollections about a sha.

Two things that did improve, both from the base move rather than from the diff:

• Base is now 0 behind main and contains 8c46af353, so this PR carries the #1341 mock fix. It is no longer exposed to the agentProfile.memoryWrite reds that main was serving between d17f95f2 (01:32:43Z) and 8c46af35 (01:50:17Z) — the window that hit several PRs which caused none of it.
• Stale-base merge guard and the CodeQL analyses pass; Test & Coverage, E2E Tests and Analyze (javascript-typescript) were still pending when I measured, so this remains a gate on the code, not on green CI.

My one residual finding is unchanged and still non-blocking: FORMAT_CHARACTERS is still /\p{Cf}/gu at line 66, so the 8 non-Cf invisibles — U+3164 and the other Hangul fillers, U+034F, U+17B4 — still defeat both suppression paths and publish the private reasoning. \p{Default_Ignorable_Code_Point} closes 6 of the 8 and keeps your suite at 21/21; I verified that in place before reverting. Take it here or as a follow-up, but the comment at line 62 should not be left implying the class is the boundary of the bug.