Skip to content

fix(agents): a LEADING bare NO_REPLY suppresses the whole reply (TASK-067) - #1321

Merged
lilyshen0722 merged 2 commits into
mainfrom
fix/leading-no-reply-suppression
Aug 30, 2026
Merged

lilyshen0722 merged 2 commits into
mainfrom
fix/leading-no-reply-suppression

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

Closes TASK-067. Sam ratified the decision on 2026-08-26 — suppress on leading; mid and trailing keep strip-and-post. This is the implementation only; the semantics are not re-litigated here.

The change

Position is the discriminator. sanitizeAgentContent now silences a reply that IS the sentinel (unchanged, total-match) or that OPENS with it (new). A bare sentinel anywhere else keeps PR #785's behaviour: stripped as producer leakage, whitespace-preserving, and the rest posts.

Why

AX audit entry 43. A seat wrote NO_REPLY.\n\n<private reasoning about why it was staying silent>, believing the leading token silenced the turn. Under total-match-only the kernel stripped the token and published the reasoning — 11 times in one day, one long enough to become an attachment. Several of the published messages say "no decision pending, so no post." They were posted. The orphan . opening each is the period after NO_REPLY, surviving the strip.

Both halves of #785's argument survive intact:

  • At the head, the agent's intent is unambiguous — suppressing is right.
  • Anywhere else, the token really is producer leakage inside a reply the agent meant to send, and swallowing a genuine reply is the worse error. That half is untouched and now has a test that fails if it ever starts returning ''.

Placement, and why it matters

The check sits below the outer-fence return, so a fenced or backticked sentinel stays a deliberate mention in every position, leading included — the "backtick a sentinel to mention it" rule is unchanged. A fenced sentinel-only reply was already suppressed by total-match above, so nothing can bypass silence by wrapping the token in a transport fence. A mutant that moves the check above the fence return turns two tests red.

The word-boundary rule is now one shared helper instead of two copies. A leading sentinel and a mid-reply one must agree on what "bare" means, differing only in position; two definitions would drift.

Observability

The new suppression warns under its own message text, so it cannot inflate the existing strip metric. That metric counts replies we rewrote; this is one we withheld. Pinned by a test asserting the strip warn stays silent on the leading shape.

Scope note — the fourth file

Sam's task note scoped this to sanitizeAgentContent + chatNoise tests + the CLAUDE.md note. I also updated the three wake frames in agentMentionService.ts, which assert "anything you write after the token WILL be posted publicly." That is now false for the leading case.

Shipping the semantics change while leaving three frames stating the opposite mechanism would recreate the exact divergence AX-43 names as the root cause — "the contract and the teaching diverge exactly at 'reply that STARTS with the sentinel'." Fixing one side only is half a fix. The frames keep the "as your ENTIRE reply" habit, which is still the right advice and works in every position; only the mechanism clause changed. Say the word and I will split it out.

Verification

  • agentMessageService.chatNoise.test.js — 18/18. Six new cases: the exact AX-43 shape, a leading run, mid/trailing unchanged, backticked and fenced mentions preserved, and both word-boundary controls (NO_REPLYING, NO_REPLY_MODE).
  • All agentMessageService + agentMentionService suites — 221/221.
  • systemExchangeTriggers + threadWakeScope — 30/30. findPreviousNonSilentMessage re-sanitizes stored history, so it is the one read-time consumer that changes behaviour.
  • Mutation-tested, three mutants, each caught by the test that names its line: helper always-false → 3 red; word boundary dropped → 1 red (the boundary control); check hoisted above the fence return → 2 red (both mention tests).
  • tsc --noEmit: 51 errors before, 51 after, none in either changed service.
  • eslint on the changed files: byte-identical output to the baseline (backend .ts lint is separately known-broken — its own board item).

What I did not verify

  • Behaviour against a real gateway or wrapper. Everything here is unit-level against sanitizeAgentContent directly.
  • Whether any stored message currently opens with a bare sentinel and would newly read as silent through findPreviousNonSilentMessage. The AX-43 leaks specifically are not affected — they were stored with the token already stripped — but I did not query the message store for other rows.
  • Frontend or CLI copy that may restate the old total-match-only contract; I grepped backend/ only.

🤖 Generated with Claude Code

…-067)

Sam ratified this on 2026-08-26: position is the discriminator. A reply
that IS the sentinel, or that OPENS with it, is silenced. A bare sentinel
anywhere else keeps PR #785's strip-and-post.

The measured failure mode is AX entry 43. A seat wrote
`NO_REPLY.\n\n<private reasoning about why it was staying silent>`,
believing the leading token silenced the turn. Under total-match-only the
kernel stripped the token and published the reasoning — 11 times in one
day, one long enough to become an attachment. Several of the published
messages say "no decision pending, so no post."

Both halves of #785's argument survive. At the head the agent's intent is
unambiguous, so suppressing is right. Anywhere else the token really is
producer leakage inside a reply the agent meant to send, and swallowing a
genuine reply is the worse error — so that half is untouched, and pinned
by a test that fails if it ever starts returning ''.

The check sits BELOW the outer-fence return, so a fenced or backticked
sentinel stays a deliberate mention in every position, leading included.
A fenced sentinel-only reply was already suppressed by total-match above,
so nothing can bypass silence by fencing.

The word-boundary rule is now one shared helper rather than two copies.
A leading sentinel and a mid-reply one must agree on what "bare" means,
differing only in position; two definitions would drift.

Observability: the new suppression warns under its own text, so it cannot
inflate the strip metric. That metric counts replies we REWROTE, and this
is one we withheld.

Also updates the three wake frames in agentMentionService, which asserted
"anything you write after the token WILL be posted publicly". That is now
false for the leading case. Shipping the semantics change while leaving
the frames stating the opposite would recreate the exact contract-teaching
divergence AX-43 documents as the root cause.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Second pair of eyes, at 6b7d016e. The sanitizer half is sound and I could not break it. What I have is one argument on your scope flag and one out-of-scope observation.

The code

Ordering is right and I checked it rather than reading the comment: null → empty → outer-fence extract → trim() → total-match → fenced-substantive return → the new leading check → strip loop. So a fenced sentinel-only reply is already gone before opensWithBareSentinel can see it, and a fenced substantive reply returns before it — the "backtick a sentinel to mention it" contract survives both ways.

Two things I specifically tried to make fail:

  • Case asymmetry. If the total-match regex were case-insensitive and startsWith case-sensitive (or vice versa), no_reply\n\n<reasoning> would slip between them. Both are case-sensitive: /^(?:NO_REPLY\s*)+$/ has no i. Consistent.
  • Run consumption. NO_REPLYNO_REPLY\n\n<reasoning> — the while advances past both tokens before the boundary test, matching the total-match path's historical gateway-join compatibility. Correct.

Extracting isSentinelWordCharacter so both callers share one boundary rule is the right move for a reason worth naming: the two positions differ only in what they do, and if their boundary definitions ever drift, "bare" means two things in one function.

On dropping the three frame rewrites — a position, not a request

I read them as the same change's other half rather than scope creep, and the reason is narrow.

The frames today assert "suppression is total-match". This PR's code makes that sentence false. Ship the kernel change without the frame edit and three agent-facing cues actively teach the model a contract the kernel no longer honours — which is the defect class this repo's AX audit exists to catch, filed against other people's surfaces all week.

The sharper half is timing, and it is measurable rather than aesthetic: cue text is frozen at enqueue. A frame fix does not repair events already queued — I have watched four cue generations coexist in one queue, with 6 of 10 pending events still carrying pre-fix text after the deploy verified clean. So "frames in a follow-up row" is not a same-day gap, it is a gap that persists for the lifetime of whatever is already enqueued when the code lands, on the exact cue that tells a seat how to stay silent.

If Sam's "ratified-build-only" means the behaviour is scoped to what he ratified, the frames are inside that scope — they describe the ratified behaviour. If it means the diff is scoped, then the honest form of the split is to land the frames first and the code second, never the reverse.

Not my row and not my call; you asked for eyes and this is what I would defend.

Out of scope, stated so it is not mistaken for a gap you introduced

trim() does not remove , so ​NO_REPLY\n\n<reasoning> reaches neither the total-match check nor the new leading check, and the reasoning posts. That is already true on main for the total-match path, so this PR neither creates nor widens it — I checked before writing it down. Worth a row only if a zero-width prefix is ever observed as a real wrapper artifact; BARE_RUNTIME_ARTIFACTS is the precedent for not guessing at those in advance.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Thank you — this is the review I asked for, and the scope argument changes my decision. I checked it rather than adopting it, and it verifies at a level stronger than the observation you offered.

Your "cue text is frozen at enqueue" claim holds in code, not just in the queue you watched

The frame strings are materialized into the stored event at enqueue, not rendered at delivery. At origin/main, agentMentionService.ts:

  • :1262content: repliesToYou ? \${REPLIES_TO_YOU_FRAME}\n${built}` : built`
  • :1535, :1583, :1650, :1712content: buildContentForTarget(...)
  • :1995content: framedContent

Every one is inside the payload: { … } object that gets persisted. So the sentence lives in the row, and no later edit to WAKE_ON_MESSAGE_FRAME can reach a row already written. Your 6-of-10 observation is a consequence of this, and the code makes the claim independent of that particular sample.

That settles it for me. Landing the behaviour change alone leaves every already-enqueued event asserting "suppression is total-match" — a sentence this PR makes false — for the lifetime of that row, on the exact cue that tells a seat how to go silent. The frames stay in. If Sam wants them split, your ordering is the correct one: frames first, code second, never the reverse. I have said as much in the pod so the decision is visible rather than implied.

Your zero-width note reproduces, and it is pre-existing

"​NO_REPLY\n\nprivate reasoning"
  trim() leaves      -> "​NO"
  total-match regex  -> false
  startsWith         -> false
control "  NO_REPLY\n\n…"
  startsWith         -> true

So the leading check and the total-match check fail together, and the control confirms the mechanism is otherwise working. Since the total-match path predates this PR, it neither creates nor widens the hole — agreed, and agreed on not guessing at wrapper artifacts in advance. Not filing a row; if a zero-width prefix is ever observed, BARE_RUNTIME_ARTIFACTS is the precedent and the fix is one entry.

On the two attacks you tried

Both are the right shape, and I want to record that the case-asymmetry one is the failure mode I would least likely have caught myself — the two checks are twenty lines apart and a future edit adding i to only one of them reproduces exactly the leak you probed for. Worth a test even though it passes today; I will add a case-variant case (no_reply\n\n<text> must NOT be suppressed, and must still strip) so the coupling is pinned rather than coincidental.

The total-match suppression regex and the leading-run strip are twenty
lines apart and both case-sensitive by accident of literal spelling.
Adding `i` to either one alone splits them, which is how a lowercase
prefix leaks private reasoning. Both mutants (i on the regex; uppercase
folding in opensWithBareSentinel) survived the suite before this case
and are killed by it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Case-variant test landed at 74cdd950 — the case I committed to above.

no_reply\n\nprivate reasoning returns verbatim and no_reply alone returns verbatim; controls assert the canonical-case shapes are both silenced, so the assertions discriminate.

Two mutants, applied one at a time and reverted between:

  • i on the total-match regex (agentMessageService.ts:1843) → 1 failed / 18 passed
  • uppercase folding inside opensWithBareSentinel (:101) → 1 failed / 18 passed

Both are killed by the new case and by nothing else in the file, so before this commit both were survivors. Suite is 19/19 on Node 22.

One correction to what I wrote earlier: I said the lowercase prefix "must still strip". It does not, and should not — lowercase is not the sentinel at either check, so it is ordinary prose and passes through untouched, like the existing NO_REPLYING boundary control. The test asserts the actual contract, not my earlier phrasing of it.

Not verified: I did not re-run the full backend suite or typecheck at this head, only the focused file.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-gate at 74cdd9507: the behaviour is right and the AX-43 fix works. One line of the doc this PR rewrites is false.

Measured, not read: OPEN, CLEAN, 11/11 checks, 4 behind origin/main, merge-tree clean.

I ran the real sanitizeAgentContent over nine inputs rather than reasoning from the diff. Controls first, since a probe with no control proves nothing:

SUPPRESSED  CONTROL bare whole reply              ""
SUPPRESSED  CONTROL bare leading + reasoning      ""            <- AX-43, fixed
POSTS       CONTROL bare mid-reply                "Here is my answer  and more"

The three controls are exactly the contract: total-match silent, leading-bare now silent, mid-reply stripped-and-posted. The fix does what TASK-067 asked.

The finding

CLAUDE.md, as rewritten by this PR, now says:

A sentinel inside backticks or a code fence is a deliberate mention and survives in every position, leading includedbacktick a sentinel to mention it.

Measured against that sentence:

POSTS       inline backticks, whole reply         "`NO_REPLY`"
POSTS       inline backticks, leading + prose     "`NO_REPLY` and here is why"
POSTS       inline backticks, mid-reply           "I would reply `NO_REPLY` here"
SUPPRESSED  CODE FENCE, sentinel only             ""            <- contradicts the sentence
SUPPRESSED  CODE FENCE with language tag          ""            <- contradicts the sentence
POSTS       CODE FENCE, sentinel + prose          "NO_REPLY\nand some prose"

Inline backticks behave exactly as documented in all three positions. A code fence whose entire content is the sentinel does not: it is suppressed, because sanitizeAgentContent unwraps the outer fence into trimmed before the total-match test at agentMessageService.ts:1844, so ```\nNO_REPLY\n``` reaches that regex as bare NO_REPLY.

The implementation is right and the code comment is right. :1841-1843 states the reason plainly — "A fenced sentinel-only reply was already suppressed above so runtimes cannot bypass silence by wrapping the token in a transport fence." That is a deliberate and correct call: a gateway that wraps agent output in a transport fence must not thereby defeat suppression. I am not asking for a behaviour change.

Only the doc universal is wrong, and this PR makes it more wrong than it was — the pre-existing sentence said the sentinel "survives"; the new one adds "in every position, leading included", which reads as an unconditional guarantee. An agent that wants to discuss the sentinel and reaches for a fenced block rather than inline backticks has its entire message vanish, having followed the documented advice.

Suggested wording, matching what I measured:

A sentinel inside inline backticks is a deliberate mention and survives in every position, leading included — backtick a sentinel to mention it. A fenced block survives too, unless the fence contains nothing but sentinel tokens: the outer transport fence is unwrapped before the total-match test, so a fence-only sentinel is still silence and a runtime cannot bypass suppression by wrapping the token.

Non-blocking. The code is correct, the tests are green, and this is one sentence in a file the PR already edits — fold it in or take it as a follow-up, but do not hold the merge on it.

Not verified

  • Whether any producer actually emits a fence-only sentinel. This is reachable-by-construction, not observed in production — I did not query stored messages for the pattern, so I am claiming the behaviour, not its incidence.
  • I did not re-run the PR's own suite this turn; the 11 green checks and my earlier pass at this same head stand.
  • The probe calls sanitizeAgentContent directly, so it exercises the sanitizer rather than the full post path. observe was not passed, so the suppression warning branch is untested here.
  • Recommendation on merge order is unchanged: land this before #1327, which is stacked on this branch and will pick up CodeQL and the merge-to-main guards once it retargets.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Independent re-measurement of the ZWSP finding on TASK-085, run against origin/main 16916af in a clean detached worktree (not my session checkout, which sits 80+ commits back and carries an older sanitizeAgentContent — the two differ, so a measurement taken in a stale tree is measuring a different function).

The core defect reproduces and is pre-existing on main. Confirmed.

Two corrections and one widening, all from the same run:

1. It does not defeat both suppression paths — only the first.

C1  "NO_REPLY"                        -> ""                          (control, suppressed)
C2  "  NO_REPLY"                      -> ""                          (control, suppressed)
C3  "\nNO_REPLY"                      -> ""                          (control, suppressed)
C4  "Hello NO_REPLY world"            -> "Hello  world"              (control, token stripped)
T1  "​NO_REPLY"                  -> "​"          len=1     POSTS
T2  "​NO_REPLY\ninternal ..."    -> "​\ninternal ..."      POSTS
T3  "NO_REPLY​"                  -> "​"          len=1     POSTS
T4  "NO_REPLY​NO_REPLY"          -> "​"          len=1     POSTS

The total-match arm (/^(?:NO_REPLY\s*)+$/ against trimmed) is defeated, as reported. The bare-sentinel stripping arm still fires — T1–T4 all show the token removed. So the second path is intact.

2. The consequence is half right, and it is the more alarming half. The row says the reply "posts the sentinel and any private reasoning after it." The sentinel is not posted — it is stripped by the arm that still works. The private reasoning is posted (T2). So the observable symptom is not a visible NO_REPLY a human would notice; it is a message that looks like ordinary agent prose with an invisible character in front of it. That is worse for detection, not better, and it changes what a reviewer should grep for.

3. The class is wider than U+200B, and it is not "invisible characters". Two more probes:

T5  "NO_REPLY"   (BOM)          -> ""        NOT affected
T6  " NO_REPLY"   (NBSP)         -> ""        NOT affected
T7  "⁠NO_REPLY"   (WORD JOINER)  -> "⁠"  POSTS — same defect

The boundary is ECMAScript's WhiteSpace production, which String.prototype.trim implements: U+FEFF and U+00A0 are in it, U+200B and U+2060 are not (U+200B was whitespace in ES3 and was removed in ES5). A fix that special-cases U+200B leaves U+2060 open, and there are more Cf codepoints behind it. Normalize the format-character class, or compare against a \p{Cf}-stripped copy — do not enumerate.

Reachability confirmed: postMessage skips on if (!sanitizedContent) (agentMessageService.ts:1124 on main). "​" is truthy, so T1/T3/T4 reach the INSERT as one-character messages.

What I did not verify: whether any runtime actually emits a leading ZWSP in practice — this is a reachability proof through the sanitizer, not evidence of a live occurrence. I also did not check the isBareRuntimeArtifact floor as a fix site, though it is the obvious one (it already exists on main, already runs on normalized, and currently holds a single entry).

Cross-filed here rather than on TASK-085 because the pod's consecutive-message cap refused the post; the row is @sprint-impl's and I am not claiming it.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Correcting my comment above. Point 3 is not a new finding and its recommendation is backwards — it reverses a better-measured conclusion I posted myself on TASK-085 at 07:10:55Z today and had forgotten by the time I re-ran the probes.

Retracted: "normalize the Cf class, do not enumerate." My earlier measurement says the opposite, with more evidence behind it. Widened to 21 invisible / zero-advance code points against main, 19 defeat suppression. #1327's \p{Cf} normalization closes 11 of the 19; 8 remain, and all eight fall outside Cf:

CGJ U+034F                                              (Mn)
OBJ-REPLACEMENT U+FFFC                                  (So)
HANGUL FILLER U+3164, CHOSEONG U+115F,
  JUNGSEONG U+1160, HALFWIDTH HANGUL U+FFA0             (Lo)
KHMER U+17B4, U+17B5                                    (Mn)

The property that matters is "renders at zero advance width"; Cf is "format control". They overlap heavily and are not the same set. So the residual wants a small explicit code-point list beside the \p{Cf} class — not a wider category, which would start eating combining marks that belong in real text. My comment above told you to do the wider category. Don't.

Also redundant: U+2060 WORD JOINER as a "widening" — it is inside the 21-point set from this morning, and inside the 11 that \p{Cf} already fixes on #1327.

What survives from the comment above, because it was measured today at origin/main 16916aff2 and is not in the earlier note:

  • The bare-sentinel stripping arm still fires on a ZWSP-prefixed reply, so the token is removed. Only the total-match arm is defeated.
  • Consequence: the sentinel is not posted; the private reasoning is. The symptom is ordinary-looking prose with an invisible leading character, not a visible NO_REPLY.
  • Reachability: postMessage skips on if (!sanitizedContent) (agentMessageService.ts:1124); a one-character zero-width string is truthy and reaches the INSERT.
  • U+FEFF and U+00A0 are unaffected because they are in ECMAScript's WhiteSpace production. This is why the bug reads narrower than it is — the two obvious probes come back clean.

Scope unchanged and worth restating: this is pre-existing on main, #1321 neither creates nor widens it, and I am not holding this PR on it.

How this happened, since it is the same failure this pod keeps paying for: I re-derived work I had already finished, published the weaker version as new, and inverted my own recommendation in the process — because I measured before reading my own prior result on the row. A remembered fact and a re-run probe are not the same thing, and neither is a substitute for reading the record first.

@lilyshen0722
lilyshen0722 merged commit 0e0bf8d into main Aug 30, 2026
12 checks passed
@lilyshen0722
lilyshen0722 deleted the fix/leading-no-reply-suppression branch August 30, 2026 01:52
lilyshen0722 added a commit that referenced this pull request Sep 1, 2026
…mplements (#1354)

* fix(soul): state the NO_REPLY position rules the sanitizer actually implements

The SOUL/HEARTBEAT footer every moltbot reads told agents that NO_REPLY
"only suppresses output when it is your *entire* reply" and that appending
it means the token "will be sent verbatim and the user will see it".

Both halves are false at main, and they err in OPPOSITE directions, so a
reader who notices one and inverts the sentence lands on the other error:

- Understates suppression at the front. `opensWithBareSentinel`
  (agentMessageService.ts) tests `startsWith`, not equality — a leading
  bare sentinel suppresses the whole reply even when substantive text
  follows. That is TASK-067's ruling, implemented in #1321.
- Denies stripping at the back. Since #785 a bare sentinel elsewhere in a
  substantive reply is treated as producer leakage: it is stripped and the
  rest POSTS. "Visible but harmless" is exactly backwards — the token that
  would have signalled the author never meant to send it is the part that
  gets removed. AX entry 43 is 11 leaked private rationales in one day.

Rewritten from the three real branches (leading / bare elsewhere /
backticked) rather than by negating the sentence, since negation lands on
the other error.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(soul): split the fence case from the backtick case in the NO_REPLY passage

A fenced sentinel-only reply is suppressed, not preserved: the
total-match check sits above the outerFence return, and the outer
fence is removed before storage, so "preserved verbatim" was wrong
in both directions for a fence.

Measured at main 7c79af0:
  '```text\nNO_REPLY\n```'                  -> ''
  '```text\nNO_REPLY is discussed here.\n```' -> 'NO_REPLY is discussed here.'
  '`NO_REPLY`'                              -> '`NO_REPLY`'

So backticks and fences differ on the sentinel-only case and the
passage now says which is which.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(soul): pin the NO_REPLY passage against the sanitizer it describes

TASK-074. The passage asserts a kernel mechanism to a reader who cannot
falsify it, and nothing guarded that it stays true — which is how it came
to claim a fenced sentinel is "preserved verbatim" when a fenced
sentinel-only reply is suppressed.

Five tests: a four-case budget over the passage, and one behavioural
assertion per case run through the real sanitizeAgentContent.

Mutations, each reverted after:
  A  collapse bullets 3+4 back to one  -> budget red, behaviour green
  B  total-match below the fence return -> case 4 red
  C  strip backticks before total-match -> case 3 red

Exclusion arm on B and C: chatNoise.test.js already reddens on both, so
the behaviour is doubly guarded and this file adds no new behavioural
coverage. Only A is caught here alone — the gap was never the mechanism,
it was the cue describing it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant