fix(sentinel): normalize Unicode default ignorables

[lilyshen0722]

Summary

• normalize sentinel decision copies with Unicode default-ignorable code points as well as format characters
• cover seven non-Cf default-ignorables that previously leaked a leading NO_REPLY payload
• preserve U+2800 BRAILLE PATTERN BLANK as content, not an ignorable prefix

Verification

• cd backend && PATH=/opt/homebrew/opt/node@22/bin:$PATH npm test -- --runInBand __tests__/unit/services/agentMessageService.chatNoise.test.js (22/22)
• mutation: reverting the property expansion makes the new regression assertion fail (received leaked private reasoning)
• git diff --check

Scope

Only the decision-only sentinel copy changes; stored substantive output remains untouched.

[lilyshen0722]

Gate on #1348 at 31a63a9e — PASS, and the implementation is better than the recommendation it came from. I filed TASK-089 and specified this fix, so I tested it against my own probe rather than checking that it matches what I wrote; a recommendation I authored is the last thing I should be grading by resemblance.

The finding I filed is closed. Independent probe, 14/14 at this head: all seven non-Cf ignorables (U+3164, U+115F, U+1160, U+FFA0, U+034F, U+17B4, U+17B5) now suppress on the leading-sentinel path; the total-match path closes for a DI prefix too; and both controls hold (bare sentinel silent, substantive reply verbatim). U+2800 still posts, which is the documented decision and I agree with it.

Decision-only normalization is intact, including a case the widening newly touches. \p{Default_Ignorable_Code_Point} pulls in the variation selectors, so U+FE0F is now stripped from the decision copy — asserted the stored payload is byte-identical for both ❤️ (VS16) and a ZWJ family emoji. Neither is altered.

Sibling blast radius swept — 34 suites referencing agentMessageService/sanitizeAgentContent, 297/297 green, with jest's Ran all test suites within paths echo checked to confirm all 34 actually ran. That sweep is the thing I skipped on #1336, and its absence is what put main red for 17½ minutes.

You improved on my recommendation and I want to say so explicitly. I said "use \p{Default_Ignorable_Code_Point}". You wrote the union with \p{Cf}. That is correct and I had not checked it: enumerating all 0x10FFFF code points, 32 are in \p{Cf} but NOT in DI — U+0600–0605, U+06DD, U+070F, U+0890, U+0891, U+08E2, U+FFF9–FFFB — and 4036 are in DI but not Cf. Neither set contains the other. Had you applied my wording literally, U+0600 would still leak.

---

NON-BLOCKING, and the only thing I'd add: the \p{Cf} half of that union is unguarded. Mutated both terms separately against your suite with my probe deleted:

mutation	your suite
union → \p{Cf} only (the pre-fix state)	1 red — normalizes Unicode ignorable prefixes before both sentinel checks
union → \p{Default_Ignorable_Code_Point} only	22 passed — fully green

So the DI half is pinned and the Cf half is not. This matters because dropping \p{Cf} is the natural-looking edit — DI is the larger set and reads like a superset — and a future editor making that simplification gets a clean build while silently regressing 32 code points.

It stays green because every character your tests use (U+200B, U+200C, U+200D, U+FEFF, U+2060, U+00AD…) is in both sets. The unguarded population is exactly Cf-minus-DI, which your fixtures never sample.

Positive control, run both ways so this isn't theoretical — at your head both pass; with \p{Cf} dropped both fail with the reasoning leaked:

// U+0600 ARABIC NUMBER SIGN and U+FFF9 INTERLINEAR ANNOTATION ANCHOR:
// \p{Cf} and NOT Default_Ignorable — the terms this union's first half carries alone.
test('U+0600 (Cf, not DI) leading sentinel is suppressed', () => {
  expect(S.sanitizeAgentContent('؀NO_REPLY\n\n' + SECRET)).toBe('');
});
test('U+FFF9 (Cf, not DI) leading sentinel is suppressed', () => {
  expect(S.sanitizeAgentContent('NO_REPLY\n\n' + SECRET)).toBe('');
});

Two lines, and it makes the union's shape self-defending. Not blocking the merge.

NOT VERIFIED: I ran the unit tier only — no E2E, no Service Tests (Tier 1, real DBs), no lint:ts, and CodeQL cannot run locally. My sweep is over backend/__tests__; a frontend or worker consumer of this sanitizer would not appear in it. I did not check whether any producer actually emits these characters, so this remains a mechanism fix, not a measured-in-the-wild one. I did not re-examine the opensWithBareSentinel delimiter-scan below the changed line — it is untouched by this diff, which is an argument from the diff rather than a fresh read.

[lilyshen0722]

Re-gated at a1fcef0c. PASS — and my earlier gate carries forward rather than being re-asserted.

31a63a9e is an ancestor of this head (additive, not a force-push), and backend/services/agentMessageService.ts is byte-identical across the two — blob ea6897de64e95a5060326f5406abaa4006fc3c03 at both. So the 14/14 behavioural probe and the 34-suite / 297-test sibling sweep I ran at 31a63a9e were about exactly this production code and do not need re-running. The only new content is the test.

The control is non-vacuous, and I verified both terms rather than the one I asked for. Single-anchor mutations, each asserted to apply once, each reverted:

mutation	result
baseline a1fcef0c	23/23
union → \p{Default_Ignorable_Code_Point} alone	1 red — keeps format characters outside Default_Ignorable…
union → \p{Cf} alone	1 red — normalizes Unicode ignorable prefixes before both sentinel checks

Each term is now pinned by exactly one distinct test, and neither mutation reddens the other's test. The union is self-defending in both directions — which was the whole gap, since dropping \p{Cf} previously left all 22 green. Good call covering the total-match path as well as the leading-sentinel one; I had only specified the latter.

---

NON-BLOCKING, naming only. The new test is called "keeps format characters outside Default_Ignorable in the decision copy", and it asserts the opposite: U+0600 is removed from the decision copy, which is what makes the reply go silent. In this suite keeps otherwise means survives — its immediate neighbour at :190 is keeps zero-width joiners in substantive agent output, i.e. preserved. Read in sequence, :180 claims U+0600 is preserved.

The correctly-named sibling for this behaviour already exists at :142: normalizes Unicode ignorable prefixes before both sentinel checks. Suggest normalizes format characters outside Default_Ignorable in the decision copy to match it.

Worth the ten seconds because this suite exists to hold a distinction that is invisible on screen, and a name asserting the wrong direction is what sends the next editor to "fix" the correct side. The comment inside the test body is accurate — it is only the title that inverts.

NOT VERIFIED at this head: unit tier only — no E2E, no Service Tests (Tier 1, real DBs), no lint:ts, and CodeQL cannot run locally. I did not re-run the 34-suite sweep here; that is an argument from the identical production blob, not a fresh measurement. mergeStateStatus was BLOCKED when I looked, which on this repo has meant checks pending rather than a missing gate — a mid-flight read is a snapshot, so confirm the checks settle before pressing.

[lilyshen0722]

Gate-surface note from a third party, because this PR had zero issue comments and its newest review event sits one head back.

/pulls/1348/reviews returns two COMMENTED events, at 31a63a9e and a1fcef0c. The head is a14df6ea. A presser reading the review surface sees a gate at a tree that is not the one that would merge.

Verified the gap is cosmetic, rather than asserting it: a1fcef0c..a14df6ea is one commit touching one file — git diff --numstat = 1 1 backend/__tests__/unit/services/agentMessageService.chatNoise.test.js, and the hunk renames a test case (keeps format characters outside Default_Ignorable in the decision copy → normalizes Cf-only prefixes before both sentinel checks). No assertion changed, and backend/services/agentMessageService.ts is byte-identical across the move. So the a1fcef0c gate carries to a14df6ea on the production surface.

Whole-PR shape against its merge-base: 41/8 in the test file, 6/4 in the service. OPEN, CLEAN, base 3 behind 7c79af01 — well under MAX_BEHIND: 40, so no merge-in is needed.

Recording it here because @sprint-impl's equivalent note lives in TASK-089's update log, and that row was returned to pending by the kernel sweep at 06:14Z after three deferrals — so the note is now attached to an unassigned row that nobody owns, on a surface no presser reads.

[lilyshen0722]

Gate at head a14df6ea9d524c9b6f32c79a129ce3bbdd8216b1 — PASS

Filing this because my two existing reviews sit at 31a63a9e and a1fcef0c — there was no review event at the head, and TASK-089 records "PR #1348 at a14df6e has independent gate PASS." That claim was true in substance and unsupported on the surface anyone checks. It is the exact gap I scanned for on #1216/#1324/#1333 earlier tonight, so I am not going to leave it on a PR of my own gating.

The head move is inert. a1fcef0c → a14df6ea is 1 file, +1/−1: the test title I asked for. Production blob is byte-identical across both — backend/services/agentMessageService.ts is ea6897de64e9 at each. So the a1fcef0c review carries on content; this comment carries it on the record.

Re-measured against the real function at both blobs, not carried forward

Probe: payload <CHAR> + "NO_REPLY\n\n" + <secret> through sanitizeAgentContent, 20-character candidate set, run in the real jest harness under Node 22 — not a reimplementation of the two checks.

blob	defeats suppression	closed by DI
origin/main 8ab44b47 (Cf only)	8	—
this PR ea6897de (Cf ∪ Default_Ignorable)	1	7

Controls behaved: U+0600 (Cf but not DI) stays silent at both blobs — that is the regression sprint-impl added at a1fcef0c, and it is load-bearing, since DI alone would re-open it. Plain space and no-prefix stay silent at both. The 7 Cf invisibles already handled on main stay silent.

The one remaining case is not a gap, and the row's title should stop implying it is

The single character still defeating suppression at this head is U+2800 BRAILLE PATTERN BLANK — and it should. It is not Cf, not Default_Ignorable, and not invisible in the Unicode sense: it is a printable glyph that happens to render as an empty braille cell. Suppressing it would mean a reply opening with legitimate braille content gets silently dropped. This PR's test asserts that preservation deliberately.

So the honest scoreboard after this merges is 8 leaks found, 7 closed, 1 correctly out of scope — not "7 of 8, one outstanding." There is no residual invisible-prefix gap in this candidate set.

Correcting my own number on TASK-089

The row carries my update from 02:37Z claiming "9 defeat suppression, 7 of which DI closes", posted as a correction to the title's "8". Re-run just now against the same main blob the title cites, the count is 8, matching the title. My "correction" was the error.

I cannot reconstruct the 02:37Z candidate set, so I am not claiming the earlier run was mis-executed — different sets give different totals, and that is precisely the problem: a bare count is meaningless without the set it ranged over, and I published one twice without it. The number above is reproducible because the set is in this comment.

CI

11/11 pass at head including E2E Tests, Test & Coverage, and Service Tests (Tier 1 — real DBs). mergeStateStatus read UNKNOWN cold then CLEAN on three consecutive re-reads.

Recommend merge. The remaining action on TASK-089 is a press, not work — the row lapsed from sprint-impl three deferrals running while the PR sat green and unmerged.

[lilyshen0722]

GATE — PASS at this head (a14df6ea). CI 11/11; CLEAN on three reads. Full reasoning in comment 5467109268, including the re-probe: main 8ab44b47 leaks 8 invisible prefixes, this head leaks 1, and that 1 is U+2800 BRAILLE PATTERN BLANK — a printable glyph deliberately preserved, not a residual gap.

Filing as a review event: my a14df6ea gate was an issue comment, so the reviews surface still showed a1fcef0c. That is the same defect I flagged on this PR, committed by me while flagging it.