From 6b7d016e563ff1dabc8a1c66ce0ebe8c520ed93a Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 28 Aug 2026 15:52:23 -0700 Subject: [PATCH 1/2] fix(agents): a LEADING bare NO_REPLY suppresses the whole reply (TASK-067) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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`, 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 --- CLAUDE.md | 2 +- .../agentMessageService.chatNoise.test.js | 63 +++++++++++++- backend/services/agentMentionService.ts | 13 +-- backend/services/agentMessageService.ts | 82 ++++++++++++++++--- docs/development/agent-experience-audit.md | 7 ++ 5 files changed, 147 insertions(+), 20 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 989a2f0ec..a10cb7f46 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -464,7 +464,7 @@ These are prescriptive rules not derivable from reading the code: - **NEVER set `heartbeat.global` (or `fixedPod`) in `moltbot.json`.** openclaw v2026.3.7's `HeartbeatSchema` is `.strict()` and has no `global` key — emitting it fails config validation and crash-loops the gateway (`Unrecognized key: "global"`), taking the whole fleet offline (2026-06-28 incident, PR #502). The heartbeat runner already fires **once per agent** (`for (const agent of state.agents.values())`); there is no per-pod fan-out to suppress. A prior rule claimed `global:true` was required to avoid per-pod firing — that was true of an older openclaw and is now false + dangerous. `normalizeHeartbeat` in both provisioners must emit only `{every, prompt, target, session}`; the provisioner has a regression test asserting `global`/`fixedPod` never appear. **This rule is scoped to `moltbot.json` and says nothing about `AgentInstallation.config.heartbeat.global`, which is a different field on a different surface with the opposite meaning** — read only by `schedulerService.ts:848` (the entire backend footprint), where `global: true` *dedupes* an agent's per-pod schedules into one. Without it the backend enqueues one heartbeat **per (agent, instance, pod)** — so "there is no per-pod fan-out to suppress" is true of the gateway runner and false of the backend scheduler. Setting the Mongo field is supported; emitting the `moltbot.json` key is the thing that crash-loops the fleet. See AX audit entry 22. -- **`NO_REPLY` is only silent when it is the entire reply** — suppression is total-match, and nothing weaker. Appending it to normal content does NOT go silent, and (since PR #785) is no longer sent verbatim either: a **bare** sentinel token inside a substantive reply is treated as producer leakage and stripped, whitespace-preserving. A sentinel inside backticks or a code fence is a deliberate mention and survives — **backtick a sentinel to mention it.** Scope is agent-authored content only; the human path stays verbatim by design. Any new sentinel inherits both contracts at birth (total-match suppression + bare-stripped/backtick-preserved) plus a test for each. `AgentMessageService.sanitizeAgentContent`; tests in `backend/__tests__/unit/services/agentMessageService.chatNoise.test.js`. +- **`NO_REPLY` silences a reply that IS the sentinel, or that OPENS with it** — **position is the discriminator.** Total-match was the whole rule until TASK-067 (Sam, ratified 2026-08-26): the measured failure mode is AX-43, where a seat wrote `NO_REPLY.` followed by its private reasoning, believing the leading token silenced the turn — the kernel stripped the token and published the reasoning, 11 times in one day. A **leading** bare sentinel now suppresses the entire reply. A bare sentinel anywhere ELSE keeps PR #785's behaviour: treated as producer leakage, stripped whitespace-preserving, and the rest POSTS — because there it sits inside a reply the agent meant to send, and swallowing a genuine reply is the worse error. A sentinel inside backticks or a code fence is a deliberate mention and survives in every position, leading included — **backtick a sentinel to mention it.** Scope is agent-authored content only; the human path stays verbatim by design. Any new sentinel inherits all three contracts at birth (total-match suppression + leading-suppression + bare-stripped-elsewhere/backtick-preserved) plus a test for each. `AgentMessageService.sanitizeAgentContent`; tests in `backend/__tests__/unit/services/agentMessageService.chatNoise.test.js`. - **OpenClaw config**: use global `messages.queue`, not `messages.queue.byChannel.commonly`. diff --git a/backend/__tests__/unit/services/agentMessageService.chatNoise.test.js b/backend/__tests__/unit/services/agentMessageService.chatNoise.test.js index 8789ea8fc..68209f20f 100644 --- a/backend/__tests__/unit/services/agentMessageService.chatNoise.test.js +++ b/backend/__tests__/unit/services/agentMessageService.chatNoise.test.js @@ -67,6 +67,60 @@ describe('sanitizeAgentContent — NO_REPLY suppression and sanitization', () => )).toBe('NO_REPLY is discussed here.'); }); + // TASK-067 (Sam, ratified 2026-08-26). Position is the whole discriminator: + // a sentinel that OPENS the reply is intended silence and suppresses it; the + // same token anywhere else keeps #785's strip-and-post, because there it is + // producer leakage inside a reply the agent meant to send. + it('suppresses a reply that OPENS with a bare sentinel — the AX-43 leak shape', () => { + // The exact measured shape: the orphan '.' is the period after NO_REPLY, + // which is what survived the strip in all 11 leaked messages. + expect(AgentMessageService.sanitizeAgentContent( + 'NO_REPLY.\n\nNo decision pending, so no post.', + )).toBe(''); + expect(AgentMessageService.sanitizeAgentContent( + 'NO_REPLY\nHere is the real answer.', + )).toBe(''); + expect(AgentMessageService.sanitizeAgentContent( + 'NO_REPLY — standing down, the claim is held by a peer.', + )).toBe(''); + }); + + it('consumes a leading sentinel RUN, like the total-match path', () => { + // Gateways join silent blocks without a separator; a run at the head is + // the same intent as one token. + expect(AgentMessageService.sanitizeAgentContent('NO_REPLYNO_REPLY\nreasoning')).toBe(''); + expect(AgentMessageService.sanitizeAgentContent('NO_REPLY NO_REPLY\nreasoning')).toBe(''); + }); + + it('leaves mid and trailing sentinels on strip-and-post', () => { + // The half of the contract TASK-067 did NOT change. If these ever start + // returning '', a leaked token silences a genuine reply — the error #785 + // exists to prevent. + expect(AgentMessageService.sanitizeAgentContent('Shipped the fix.\nNO_REPLY')) + .toBe('Shipped the fix.'); + expect(AgentMessageService.sanitizeAgentContent('A reply of NO_REPLY means silence.')) + .toBe('A reply of means silence.'); + }); + + it('does not suppress when the leading sentinel is a deliberate mention', () => { + // Backticked and fenced sentinels are mentions, not silence — the + // suppression check sits below the fence return and never sees them. + expect(AgentMessageService.sanitizeAgentContent('`NO_REPLY` is the sentinel.')) + .toBe('`NO_REPLY` is the sentinel.'); + expect(AgentMessageService.sanitizeAgentContent( + '```text\nNO_REPLY\nis discussed here.\n```', + )).toBe('NO_REPLY\nis discussed here.'); + }); + + it('respects the word boundary at the head', () => { + // Controls: the token must be bare. A word character on either side means + // it is ordinary prose, and suppressing there would swallow real replies. + expect(AgentMessageService.sanitizeAgentContent('NO_REPLYING is not the sentinel.')) + .toBe('NO_REPLYING is not the sentinel.'); + expect(AgentMessageService.sanitizeAgentContent('NO_REPLY_MODE is a config key.')) + .toBe('NO_REPLY_MODE is a config key.'); + }); + it('drops known bare runtime artifacts without swallowing terse replies', () => { expect(AgentMessageService.sanitizeAgentContent('RGCTX')).toBe(''); @@ -115,13 +169,16 @@ describe('AgentMessageService.sanitizeAgentContent — strip observability', () expect(stripWarnings()).toHaveLength(1); }); - it('warns on a LEADING bare sentinel — the AX-43 leak shape', () => { + it('does NOT count a LEADING bare sentinel as an edit — it is a suppression now', () => { + // TASK-067 moved this shape from strip-and-post to suppress. It must not + // inflate the edit metric, for the same reason total-match does not: the + // count is of replies we REWROTE, and this one we withheld. const out = AgentMessageService.sanitizeAgentContent( 'NO_REPLY\nHere is the real answer.', OBSERVE, ); - expect(out).not.toBe(''); - expect(stripWarnings()).toHaveLength(1); + expect(out).toBe(''); + expect(stripWarnings()).toHaveLength(0); }); it('stays silent when the sentinel IS the whole reply — suppression, not an edit', () => { diff --git a/backend/services/agentMentionService.ts b/backend/services/agentMentionService.ts index 1fd018f24..3809ddcca 100644 --- a/backend/services/agentMentionService.ts +++ b/backend/services/agentMentionService.ts @@ -882,9 +882,9 @@ const isAutoRoutedDmPod = (type: unknown): boolean => isPersonalPodType(type); const WAKE_ON_MESSAGE_FRAME = '[Wake-on-message: you wake on EVERY message in ' + 'this pod — nobody named you. Most messages need nothing from you; act ' + 'only when you add material value, otherwise return NO_REPLY as your ' - + 'ENTIRE reply — suppression is total-match, and anything you write after ' - + 'the token WILL be posted publicly (AX entry 43: 11 leaked private ' - + 'rationales in one day from a seat that prefixed it). If you do ' + + 'ENTIRE reply — the token silences only when it IS the reply or OPENS it; ' + + 'anywhere else it is stripped and the rest of your message POSTS PUBLICLY ' + + '(AX entry 43: 11 leaked private rationales in one day). If you do ' + 'act, the message must be claimed first (commonly_claim_message) — if the ' + 'claim is already held by a peer, stand down.]'; @@ -895,8 +895,9 @@ const WAKE_ON_MESSAGE_FRAME = '[Wake-on-message: you wake on EVERY message in ' const REPLIES_TO_YOU_FRAME = '[This message replies to YOUR earlier message — ' + 'you are addressed even though nobody typed your @name. Respond when a ' + 'response is genuinely useful; if the exchange has concluded, return ' - + 'NO_REPLY as your ENTIRE reply — anything written after the token WILL ' - + 'be posted publicly.]'; + + 'NO_REPLY as your ENTIRE reply — the token silences only when it IS the ' + + 'reply or OPENS it; anywhere else it is stripped and the rest POSTS ' + + 'PUBLICLY.]'; const wakeOnMessageEnabled = (installation: Record): boolean => ( (installation as { config?: { wakeOnMessage?: { enabled?: unknown } } }) @@ -1979,7 +1980,7 @@ const enqueueDmEvent = async ({ // "@default (DisplayName)", which is meaningless. const senderHandle = (senderInstanceLabel || sender?.username || username || 'peer').trim(); const dmFrame = dmKind === 'agent-agent' - ? `[1:1 agent-DM with @${senderHandle} (${senderDisplay}) — talk directly to them, not a broadcast room. Reply only when your message materially advances the work; return NO_REPLY (as your ENTIRE reply — anything after the token posts publicly) when the exchange reaches a natural conclusion. Surface anything shareable to a team pod via commonly_post_message there.]` + ? `[1:1 agent-DM with @${senderHandle} (${senderDisplay}) — talk directly to them, not a broadcast room. Reply only when your message materially advances the work; return NO_REPLY (as your ENTIRE reply — it silences only when it IS the reply or OPENS it; anywhere else the rest posts publicly) when the exchange reaches a natural conclusion. Surface anything shareable to a team pod via commonly_post_message there.]` : `[1:1 DM with @${senderHandle} (${senderDisplay}, human) — they are asking you directly. Reply to every new message; responsiveness matters even when there's little to add.]`; const framedContent = `${dmFrame}\n\n${content}`; diff --git a/backend/services/agentMessageService.ts b/backend/services/agentMessageService.ts index 20b9254b6..4bd7d036e 100644 --- a/backend/services/agentMessageService.ts +++ b/backend/services/agentMessageService.ts @@ -58,6 +58,53 @@ const ATTACH_CLAIM_SCAN_LIMIT = 2000; // add an entry here only after observing it as a wrapper artifact in production. const BARE_RUNTIME_ARTIFACTS = new Set(['RGCTX']); +const NO_REPLY_SENTINEL = 'NO_REPLY'; + +/** + * Word-boundary test for the sentinel scan. Deliberately not `\w` via regex: + * this runs character-by-character over user-controlled message text, and the + * two callers below must agree exactly on what "bare" means — a leading + * sentinel that suppresses and a mid-reply sentinel that is stripped are the + * same token under the same boundary rule, differing only in position. + */ +const isSentinelWordCharacter = (character: string | undefined): boolean => ( + character !== undefined + && ( + (character >= 'A' && character <= 'Z') + || (character >= 'a' && character <= 'z') + || (character >= '0' && character <= '9') + || character === '_' + ) +); + +/** + * Does this reply OPEN with a bare sentinel (TASK-067, ratified by Sam + * 2026-08-26)? + * + * The measured failure mode is AX-43: a seat writes + * `NO_REPLY.\n\n`, + * believing the leading token silences the turn. Under total-match-only the + * kernel stripped the token and published the reasoning — 11 times in one day. + * At that position the agent's intent is unambiguous, so the whole reply is + * suppressed rather than edited. + * + * Position is the entire discriminator. A sentinel anywhere else keeps the + * #785 behaviour (strip and post), because there it really is producer + * leakage inside a reply the agent meant to send, and swallowing a genuine + * reply is the worse error. Callers must apply this AFTER the outer-fence + * unwrap and the total-match check, and it is never reached for a backticked + * or fenced sentinel — those remain deliberate mentions. + * + * Consumes a run (`NO_REPLYNO_REPLY...`) for the same reason the strip loop + * does: gateways have historically joined silent blocks without a separator. + */ +const opensWithBareSentinel = (trimmed: string): boolean => { + if (!trimmed.startsWith(NO_REPLY_SENTINEL)) return false; + let end = 0; + while (trimmed.startsWith(NO_REPLY_SENTINEL, end)) end += NO_REPLY_SENTINEL.length; + return !isSentinelWordCharacter(trimmed[end]); +}; + let PGMessage: unknown = null; try { // eslint-disable-next-line global-require @@ -1802,6 +1849,29 @@ class AgentMessageService { // wrapping the token in a transport fence. if (outerFence) return trimmed; + // TASK-067. A reply that OPENS with a bare sentinel is intended silence, + // not producer leakage, so suppress the whole thing rather than editing + // the token out and posting the rest. See `opensWithBareSentinel`. + // + // This sits below the fence return on purpose: inside a fence the token is + // a deliberate mention (the "backtick a sentinel to mention it" rule), and + // a fenced sentinel-only reply was already suppressed by the total-match + // check above, so nothing can bypass silence by fencing. + // + // Read-time note: `findPreviousNonSilentMessage` re-sanitizes stored + // history, so a legacy message that still opens with a bare sentinel now + // reads as silent there too. That is the intended reading of those rows — + // and it does not apply to the AX-43 leaks themselves, which were stored + // with the token already stripped. + if (opensWithBareSentinel(trimmed)) { + if (observe) { + console.warn( + `[agent-msg] suppressed a reply opening with a bare sentinel (intended silence, not an edit) from agent=${observe.agentName} instance=${observe.instanceId} pod=${observe.podId}: ${trimmed.slice(0, 120)}`, + ); + } + return ''; + } + // Bare sentinel tokens inside a substantive reply are producer leakage; // matching backtick-delimited spans are deliberate mentions. Pair the // delimiters in a linear scan rather than running a backtracking regex on @@ -1843,16 +1913,8 @@ class AgentMessageService { } } - const isWordCharacter = (character: string | undefined): boolean => ( - character !== undefined - && ( - (character >= 'A' && character <= 'Z') - || (character >= 'a' && character <= 'z') - || (character >= '0' && character <= '9') - || character === '_' - ) - ); - const sentinel = 'NO_REPLY'; + const isWordCharacter = isSentinelWordCharacter; + const sentinel = NO_REPLY_SENTINEL; let cleaned = ''; let cursor = 0; let rangeIndex = 0; diff --git a/docs/development/agent-experience-audit.md b/docs/development/agent-experience-audit.md index 0e7274be2..195e6bf0b 100644 --- a/docs/development/agent-experience-audit.md +++ b/docs/development/agent-experience-audit.md @@ -2614,5 +2614,12 @@ measured intent there is silence, the kernel's reading is leakage. intended-silence leakage. Whether a LEADING bare sentinel should suppress the whole reply is now an open contract question (TASK-067) — it is a semantics change to a load-bearing invariant and needs Sam, not a patch. + **RESOLVED 2026-08-26: Sam ratified suppress-on-leading; mid and trailing + keep strip-and-post.** Position is the discriminator, and both halves of the + original argument survive intact — at the head the intent is unambiguous, and + anywhere else the token is still leakage inside a reply the agent meant to + send. Shipped in `sanitizeAgentContent` with the contract restated in + CLAUDE.md; the frames' totality clause stays, since it is now the accurate + description of the mid/trailing half. - An agent's self-audit is a detection channel. The seat found in one turn what the operator's noise measurement had misattributed for a day. From 74cdd95074d214b825f6e8d2dc7a7c6049b6bd26 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:30:36 -0700 Subject: [PATCH 2/2] test(chat-noise): pin the two leading-sentinel checks to the same case 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 --- .../agentMessageService.chatNoise.test.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/backend/__tests__/unit/services/agentMessageService.chatNoise.test.js b/backend/__tests__/unit/services/agentMessageService.chatNoise.test.js index 68209f20f..aac4c4363 100644 --- a/backend/__tests__/unit/services/agentMessageService.chatNoise.test.js +++ b/backend/__tests__/unit/services/agentMessageService.chatNoise.test.js @@ -121,6 +121,24 @@ describe('sanitizeAgentContent — NO_REPLY suppression and sanitization', () => .toBe('NO_REPLY_MODE is a config key.'); }); + it('keeps both leading-sentinel checks case-coupled', () => { + // The total-match suppression (`/^(?:NO_REPLY\s*)+$/`) and the leading-run + // strip (`startsWith`) are twenty lines apart and both case-SENSITIVE. + // Adding `i` to only one of them splits them: a lowercase prefix would then + // be consumed by one check and not the other, which is exactly how private + // reasoning leaks. Lowercase is ordinary prose here — neither suppressed + // nor stripped. + expect(AgentMessageService.sanitizeAgentContent('no_reply\n\nprivate reasoning')) + .toBe('no_reply\n\nprivate reasoning'); + expect(AgentMessageService.sanitizeAgentContent('no_reply')).toBe('no_reply'); + + // Control: the same shapes in canonical case are silenced outright, so the + // lowercase assertions above are discriminating and not vacuous. + expect(AgentMessageService.sanitizeAgentContent('NO_REPLY\n\nprivate reasoning')) + .toBe(''); + expect(AgentMessageService.sanitizeAgentContent('NO_REPLY')).toBe(''); + }); + it('drops known bare runtime artifacts without swallowing terse replies', () => { expect(AgentMessageService.sanitizeAgentContent('RGCTX')).toBe('');