← Back to PRs

#19399: telegram: fix MEDIA false positives and partial final drop

by HOYALIM open 2026-02-17 19:17 View on GitHub →
channel: telegram agents size: S
## Summary - Problem: `MEDIA:` false positives in tool-result prose (for example `MEDIA:-prefixed ...`) could be treated as file paths and trigger ENOENT; in Telegram `streamMode: partial`, regressive finals (shorter than streamed preview) could effectively drop the final user-visible answer. - Why it matters: Users may see tool-error noise instead of the intended final response, reducing delivery reliability. - What changed: - Switched `extractToolResultMediaPaths` to use the shared parser (`splitMediaFromOutput`) instead of local regex extraction. - Added regression test for malformed `MEDIA:-prefixed ...` prose. - In Telegram dispatch, mark regressive-final branch as finalized/delivered so preview is preserved (not cleared). - Added regression test for partial + tool warning + regressive final flow. - What did NOT change (scope boundary): No config schema/default changes, no API contract changes, no permission model changes. ## Change Type (select all) - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Docs - [ ] Security hardening - [ ] Chore/infra ## Scope (select all touched areas) - [ ] Gateway / orchestration - [x] Skills / tool execution - [ ] Auth / tokens - [ ] Memory / storage - [x] Integrations - [ ] API / contracts - [ ] UI / DX - [ ] CI/CD / infra ## Linked Issue/PR - Closes #18780 - Closes #18784 ## User-visible / Behavior Changes - Malformed `MEDIA:` prose in tool results is no longer interpreted as outbound file paths. - In Telegram `streamMode: partial`, when a final text regresses after a tool warning, streamed preview content is preserved instead of being cleared and lost. - No new user-facing config/options. ## Security Impact (required) - New permissions/capabilities? (`Yes/No`): No - Secrets/tokens handling changed? (`Yes/No`): No - New/changed network calls? (`Yes/No`): No - Command/tool execution surface changed? (`Yes/No`): No - Data access scope changed? (`Yes/No`): No - If any `Yes`, explain risk + mitigation: N/A ## Repro + Verification ### Environment - OS: macOS (local dev, arm64); issue reports from Ubuntu 24.04 - Runtime/container: Node v22.18.0, pnpm 10.23.0 - Model/provider: N/A (parser/dispatch path verification) - Integration/channel (if any): Telegram - Relevant config (redacted): `channels.telegram.streamMode=partial` ### Steps 1. Feed tool-result text containing malformed prose like `MEDIA:-prefixed paths ...` into media extraction flow. 2. Run Telegram partial flow with: partial text -> tool warning -> shorter final text. 3. Verify final user-visible delivery behavior. ### Expected - Malformed `MEDIA:` prose is not extracted as media path. - Final user-visible answer is preserved even when a tool warning appears mid-turn. ### Actual - Previously, malformed `MEDIA:` prose could be extracted as a path and cause ENOENT. - Previously, regressive-final branch could clear preview and leave only tool-warning output visible. ## Evidence Attach at least one: - [x] Failing test/log before + passing after - [x] Trace/log snippets - [ ] Screenshot/recording - [ ] Perf numbers (if relevant) Passing after: - `corepack pnpm vitest run --config vitest.unit.config.ts src/agents/pi-embedded-subscribe.tools.media.test.ts src/telegram/bot-message-dispatch.test.ts` - `corepack pnpm vitest run --config vitest.unit.config.ts src/agents/pi-embedded-subscribe.handlers.tools.media.test.ts` - `corepack pnpm vitest run --config vitest.e2e.config.ts src/agents/pi-embedded-subscribe.tools.e2e.test.ts` ## Human Verification (required) What you personally verified (not just CI), and how: - Verified scenarios: - `MEDIA:-prefixed ...` no longer yields extracted media paths. - Partial + tool warning + regressive final keeps preview visible (no silent final drop). - Edge cases checked: - Valid `MEDIA:/tmp/...` extraction still works. - `<media:audio>` placeholder still does not false-match. - `details.path` fallback behavior remains intact. - What you did **not** verify: - Manual end-to-end validation against a live Telegram bot account. ## Compatibility / Migration - Backward compatible? (`Yes/No`): Yes - Config/env changes? (`Yes/No`): No - Migration needed? (`Yes/No`): No - If yes, exact upgrade steps: N/A ## Failure Recovery (if this breaks) - How to disable/revert this change quickly: - Revert this commit. - Temporary mitigation: set Telegram `streamMode=off`. - Files/config to restore: - `src/agents/pi-embedded-subscribe.tools.ts` - `src/telegram/bot-message-dispatch.ts` - Known bad symptoms reviewers should watch for: - Valid `MEDIA:` directives no longer attaching media. - Partial-mode response duplication/loss regressions. ## Risks and Mitigations - Risk: Switching to shared parser may change handling for some non-standard tool output formats. - Mitigation: Added regression coverage and preserved `details.path` fallback. - Risk: Preserving preview on regressive finals may keep minor punctuation differences from final text. - Mitigation: Applied only to strict prefix-regression case to prioritize no-loss delivery. <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR fixes two bugs in the Telegram/tool-result media pipeline. **Fix 1 — `extractToolResultMediaPaths` false positives**: Replaces local regex-based MEDIA token extraction with the shared `splitMediaFromOutput` parser (`src/media/parse.ts`). The old code only guarded against mid-line matches but did not validate the extracted payload against `isValidMedia`, so malformed prose like `MEDIA:-prefixed paths ...` could yield an invalid path string and trigger an ENOENT downstream. The new approach delegates validation entirely to the shared parser, which correctly rejects payloads that aren't valid paths or URLs. **Fix 2 — Telegram `streamMode: partial` regressive-final drop**: In `bot-message-dispatch.ts`, when the regressive-final branch was taken (final text is a strict prefix of the longer streamed preview), the code would return early without setting `finalizedViaPreviewMessage = true`. This caused the `finally` block to call `draftStream.clear()`, which deleted the Telegram preview message — leaving only the subsequent tool-warning message visible to the user. The fix marks `finalizedViaPreviewMessage = true` and `deliveryState.delivered = true` before returning, preserving the streamed preview as the final visible output. Both fixes are accompanied by targeted regression tests that directly cover the described failure scenarios. Key observations: - The refactored `extractToolResultMediaPaths` now also benefits from future improvements to `splitMediaFromOutput` (fence-block awareness, space-path heuristics, etc.) without requiring updates in two places - The `stop()` double-call (once inside the regressive branch, once in `finally`) is harmless — `stop()` is idempotent in `draft-stream.ts` - `details.path` fallback logic is unchanged and still covered by existing tests <h3>Confidence Score: 5/5</h3> - This PR is safe to merge — both changes are minimal, targeted bug fixes with solid regression coverage and no API/contract/config changes. - Both fixes are narrowly scoped: one replaces a local regex extraction with a call to an already-tested shared parser (no new logic introduced), and the other adds two state assignments in a clearly correct branch. The `finally`-block interaction is safe (idempotent `stop()`). Regression tests directly cover the failure scenarios. No permission model, config schema, or API contract changes. Backward compatible per the PR description and code review. - No files require special attention. <sub>Last reviewed commit: c9bbfc8</sub> <!-- greptile_other_comments_section --> <sub>(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!</sub> <!-- /greptile_comment -->

Most Similar PRs