fix(channels): strip silence tokens mixed with reply text#164
Conversation
Every channel bridge gated the "agent stayed silent" path on `responseText.trim() === NO_REPLY_TAG`. That only matched when the model emitted the token alone — but models occasionally emit a real reply AND the token in the same final string (e.g. "ok.\n<NO_REPLY>"), in which case the literal "<NO_REPLY>" leaked through to Telegram / Discord / Slack / WeChat / Signal. Promote the silence-token logic to @openhermit/shared as stripSilenceTokens: removes every occurrence of <NO_REPLY> / <EMPTY_RESPONSE> from anywhere in the text and reports both the remainder and whether the result is effectively silent. Each bridge now drops the response only when the *stripped* remainder is empty, and sends/edits with the cleaned text when the model mixed both forms. Includes a node:test suite for the helper covering the regression case.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a shared stripSilenceTokens utility and updates all channel bridges to use it for streaming edits and final-response suppression, replacing per-bridge NO_REPLY sentinel logic. ChangesSilence token implementation and bridge migration
Sequence DiagramsequenceDiagram
participant Agent
participant Bridge
participant StripFn as stripSilenceTokens
participant ChannelAPI as Channel API
Agent->>Bridge: final accumulated response
Bridge->>StripFn: rawResponseText
StripFn-->>Bridge: {text, hadToken, isSilent}
alt isSilent == true
Bridge->>Channel API: delete provisional message
Bridge-->>Agent: {text: undefined, error: undefined}
else isSilent == false
alt hadToken == true
Bridge->>Channel API: send/edit with stripped text
else hadToken == false
Bridge->>Channel API: send/edit with original text
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes a production regression where the agent’s “silence” sentinel tokens (e.g. <NO_REPLY>) could be emitted alongside real reply text and end up displayed in downstream channel messages. It centralizes token-stripping logic in @openhermit/shared and updates channel bridges to suppress replies only when the post-stripped remainder is empty, otherwise sending the cleaned text.
Changes:
- Add
stripSilenceTokens(andSILENCE_TOKENS) to@openhermit/sharedand export it from the package entrypoint. - Update Slack/Discord/Telegram/Signal/WeChat bridges to use the shared stripping logic instead of strict equality checks.
- Add a new
node:testsuite covering bare-token silence, mixed-content stripping, multiple occurrences, and legacy token handling.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/silence-tokens.ts | Introduces the shared token-stripping helper and result contract. |
| packages/shared/src/index.ts | Re-exports the new helper/types from @openhermit/shared. |
| packages/shared/test/silence-tokens.test.ts | Adds regression and behavior tests for stripping/silence detection. |
| apps/channels/slack/src/bridge.ts | Uses stripSilenceTokens to prevent silence tokens leaking into Slack messages. |
| apps/channels/discord/src/bridge.ts | Uses stripSilenceTokens to prevent silence tokens leaking into Discord messages. |
| apps/channels/telegram/src/bridge.ts | Uses stripSilenceTokens to prevent silence tokens leaking into Telegram messages (incl. delete-on-silence). |
| apps/channels/signal/src/bridge.ts | Uses stripSilenceTokens so Signal suppresses token-only responses and strips mixed tokens. |
| apps/channels/wechat/src/bridge.ts | Uses stripSilenceTokens before sending outbound WeChat messages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Strip silence tokens from streaming text too: slack/discord/telegram all edit partial messages with accumulatedText during text_delta. If <NO_REPLY> arrived mid-stream the token would flash visibly until the final edit. Compute displayText once per delta and route it through every streaming send/edit call. - Wire packages/shared into the root test glob and add a per-package test script so the new node:test suite runs in CI. - Clarify isSilent docstring to mirror the consumer-side definition. - Update docs/channel-adapter.md and docs/session-model.md so the documented NO_REPLY contract reflects strip-anywhere semantics.
Conflicts: apps/channels/telegram/src/bridge.ts — kept local removal of NO_REPLY_TAG, kept main's new shouldSpeak/VOICE_MAX_TEXT_LENGTH helpers. Also applies the same silence-token-strip fix to apps/channels/whatsapp (the new Baileys adapter from #155), so the WhatsApp bridge doesn't ship with the brittle equality check the rest of this PR removes.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/session-model.md`:
- Line 86: The docs sentence that currently references only `<NO_REPLY>` should
be updated to reflect runtime behavior by mentioning both `<NO_REPLY>` and the
legacy `<EMPTY_RESPONSE>` markers; change the text that says "`<NO_REPLY>`
markers are stripped..." to something like "Both `<NO_REPLY>` and legacy
`<EMPTY_RESPONSE>` markers are stripped by channel adapters; if the stripped
remainder is empty the reply is suppressed entirely (whether the model emitted
the token alone or alongside real text)" so the documentation matches the
implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6e1732a7-8c9b-4d83-945b-799254860f56
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
apps/channels/discord/src/bridge.tsapps/channels/slack/src/bridge.tsapps/channels/telegram/src/bridge.tsapps/channels/whatsapp/src/bridge.tsdocs/channel-adapter.mddocs/session-model.mdpackage.jsonpackages/shared/package.jsonpackages/shared/src/silence-tokens.ts
✅ Files skipped from review due to trivial changes (2)
- docs/channel-adapter.md
- package.json
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/channels/slack/src/bridge.ts
- packages/shared/src/silence-tokens.ts
- apps/channels/telegram/src/bridge.ts
session-model.md mentioned only <NO_REPLY> but the runtime strips both <NO_REPLY> and the legacy <EMPTY_RESPONSE>. channel-adapter.md already called both out — bring session-model.md in line.
|
Actionable comments posted: 0 |
Summary by CodeRabbit