← Back to PRs

#18916: fix(agents): suppress memory tool delivery to prevent reply drop

by BinHPdev open 2026-02-17 05:42 View on GitHub →
channel: mattermost agents size: S
## Summary - Skip emitting `memory_search` and `memory_get` tool summaries/output via `onToolResult` callback - These memory tool results contain complex JSON with box-drawing characters (`│┌└─`), nested backticks from code snippets, and file paths that break Telegram's MarkdownV2/HTML parser - When delivered as tool summaries in Telegram partial-stream DM sessions, the malformed markdown causes delivery to fail, which corrupts the draft stream state and silently drops the assistant's final reply - Memory tool results are agent-internal and have no user-facing value, so suppressing them is correct behavior Closes #18646 ## Test plan - [x] `pnpm build` passes - [x] Manual verification: `isInternalToolResult()` guard returns early before calling `onToolResult` for memory tools - [x] Existing tool handler tests unaffected (memory tools not tested in current tool emission test suite) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- greptile_comment --> <h3>Greptile Summary</h3> Suppresses delivery of `memory_search` and `memory_get` tool results via `onToolResult` to prevent Telegram partial-stream delivery failures that silently drop the assistant's final reply. The core fix in `pi-embedded-subscribe.ts` is correct and well-placed. - The `isInternalToolResult()` guard in `pi-embedded-subscribe.ts` correctly filters both `emitToolSummary` and `emitToolOutput` paths for the two memory tool names. - **The new test file has a wrong import name** (`subscribeToEmbeddedPiSession` instead of `subscribeEmbeddedPiSession`) — this test will fail when run. All other test files in the directory use the correct name. - The test also does not actually verify the suppression behavior — it only checks that the (wrongly named) import is a function, providing no regression protection for the fix. <h3>Confidence Score: 3/5</h3> - The core fix is safe to merge, but the test file has a bug that will cause it to fail. - The production code change in `pi-embedded-subscribe.ts` is correct and low-risk — it adds an early-return guard for two specific tool names in two emission paths. However, the new test file imports a non-existent symbol (`subscribeToEmbeddedPiSession` instead of `subscribeEmbeddedPiSession`), which means the test will fail when run. The test also doesn't actually verify the suppression behavior even if the import were fixed. - `src/agents/pi-embedded-subscribe.memory-tool-emit.test.ts` — wrong import name will cause test failure; test doesn't verify the actual suppression behavior. <sub>Last reviewed commit: 06f149e</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs