From 1cdb46238cbcbcb395d85cd14e6dca7a69e9179a Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 22 Jul 2026 08:20:15 -0700 Subject: [PATCH] fix(gateway): make post-stream media delivery explicit-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The post-stream helper (_deliver_media_from_response) rescanned the already-streamed response and promoted bare local filesystem paths into real uploads via extract_local_files. Since the visible reply was already streamed verbatim, any bare path there is either text the user has seen or stale inspected/tool content — not an attachment request. On Slack this uploaded images from stale inspected content after otherwise clean replies. Post-stream delivery now honors only explicit MEDIA: directives. The non-streaming path in gateway/platforms/base.py keeps its bare-path auto-detect, because that path controls the visible text and strips the path from the reply when it attaches — auto-attach is intentional there. Regression tests: bare image/document paths in a streamed reply produce no upload; explicit MEDIA: tags still deliver. Fixes #20834 --- gateway/run.py | 54 ++----- .../test_post_stream_media_delivery.py | 146 ++++++++++++++++++ 2 files changed, 163 insertions(+), 37 deletions(-) create mode 100644 tests/gateway/test_post_stream_media_delivery.py diff --git a/gateway/run.py b/gateway/run.py index 0b694629c19f..dc6647909060 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -14867,11 +14867,21 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew event: MessageEvent, adapter, ) -> None: - """Extract MEDIA: tags and local file paths from a response and deliver them. + """Extract explicit MEDIA: tags from a response and deliver them. Called after streaming has already sent the text to the user, so the text itself is already delivered — this only handles file attachments that the normal _process_message_background path would have caught. + + Unlike the non-streaming path in ``gateway/platforms/base.py`` (which + also auto-detects bare local paths via ``extract_local_files``), this + post-stream rescan is EXPLICIT-ONLY. The visible reply has already + been streamed verbatim, so a bare path string here was either (a) + already shown to the user as text, or (b) stale tool/inspected + content that was never part of the intended visible reply. Promoting + such paths into uploads after the fact sent files the model never + asked to deliver (#20834). Only ``MEDIA:`` directives — the explicit + attachment contract — trigger post-stream uploads. """ from pathlib import Path from urllib.parse import quote as _quote @@ -14887,16 +14897,12 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew media_files, cleaned = adapter.extract_media(response) media_files = BasePlatformAdapter.filter_media_delivery_paths(media_files) - # Chain the cleaned text through each extractor (extract_media → - # extract_images → extract_local_files) so MEDIA: tags and image URLs - # are removed before the bare-path auto-detect runs. Previously the - # cleaned text from extract_media was dropped (``_``) and - # extract_local_files scanned text that still contained MEDIA: tags, - # producing false-positive bare-path matches with the MEDIA: prefix - # glued on. This matches the chain order in gateway/platforms/base.py. - _, cleaned = adapter.extract_images(cleaned) - local_files, _ = adapter.extract_local_files(cleaned) - local_files = BasePlatformAdapter.filter_local_delivery_paths(local_files) + # Strip image URLs from the cleaned text for parity with the + # non-streaming chain, but do NOT run extract_local_files here: + # post-stream delivery is explicit-only (#20834). Bare local paths + # in an already-streamed reply are text the user has seen (or + # stale inspected content), not an attachment request. + adapter.extract_images(cleaned) _thread_meta = self._thread_metadata_for_source(event.source, self._reply_anchor_for_event(event)) @@ -14918,14 +14924,6 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew else: non_image_media.append((media_path, is_voice)) - non_image_local: list = [] - for file_path in local_files: - if (Path(file_path).suffix.lower() in _IMAGE_EXTS - and not force_document_attachments): - image_paths.append(file_path) - else: - non_image_local.append(file_path) - if image_paths: try: images = [(f"file://{_quote(p)}", "") for p in image_paths] @@ -14961,24 +14959,6 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew except Exception as e: logger.warning("[%s] Post-stream media delivery failed: %s", adapter.name, e) - for file_path in non_image_local: - try: - ext = Path(file_path).suffix.lower() - if ext in _VIDEO_EXTS: - await adapter.send_video( - chat_id=event.source.chat_id, - video_path=file_path, - metadata=_thread_meta, - ) - else: - await adapter.send_document( - chat_id=event.source.chat_id, - file_path=file_path, - metadata=_thread_meta, - ) - except Exception as e: - logger.warning("[%s] Post-stream file delivery failed: %s", adapter.name, e) - except Exception as e: logger.warning("Post-stream media extraction failed: %s", e) diff --git a/tests/gateway/test_post_stream_media_delivery.py b/tests/gateway/test_post_stream_media_delivery.py new file mode 100644 index 000000000000..b67cfab128c6 --- /dev/null +++ b/tests/gateway/test_post_stream_media_delivery.py @@ -0,0 +1,146 @@ +"""Post-stream media delivery is explicit-only (#20834). + +``GatewayRunner._deliver_media_from_response`` runs AFTER streaming has sent +the visible reply. At that point a bare local filesystem path in the response +text is either text the user already saw, or stale inspected/tool content — +it is NOT an attachment request. Only explicit ``MEDIA:`` directives may +trigger post-stream uploads. + +The non-streaming path (``gateway/platforms/base.py``) keeps its bare-path +auto-detect (``extract_local_files``) — that path controls what text is sent +and can strip the path from the visible reply, so auto-attach is intentional +there. This file pins the asymmetry. +""" + +from types import SimpleNamespace +from unittest.mock import AsyncMock + +import pytest + +from gateway.config import Platform +from gateway.platforms.base import BasePlatformAdapter, MessageEvent, MessageType, SendResult +from gateway.run import GatewayRunner +from gateway.session import SessionSource + + +def _event(): + source = SessionSource( + platform=Platform.SLACK, + chat_id="C123CHAN", + chat_type="group", + thread_id=None, + ) + return MessageEvent( + text="hi", + message_type=MessageType.TEXT, + source=source, + message_id="171.000001", + ) + + +def _fake_runner(thread_meta): + return SimpleNamespace( + _thread_metadata_for_source=lambda source, anchor=None: thread_meta, + _reply_anchor_for_event=lambda event: None, + ) + + +def _adapter(): + return SimpleNamespace( + name="test", + extract_media=BasePlatformAdapter.extract_media, + extract_images=BasePlatformAdapter.extract_images, + extract_local_files=BasePlatformAdapter.extract_local_files, + send_voice=AsyncMock(return_value=SendResult(success=True, message_id="voice")), + send_document=AsyncMock(return_value=SendResult(success=True, message_id="doc")), + send_image_file=AsyncMock(return_value=SendResult(success=True, message_id="image")), + send_video=AsyncMock(return_value=SendResult(success=True, message_id="video")), + send_multiple_images=AsyncMock(return_value=SendResult(success=True, message_id="imgs")), + ) + + +def _allowed_media_path(tmp_path, monkeypatch, name): + root = tmp_path / "media-cache" + media_file = root / name + media_file.parent.mkdir(parents=True, exist_ok=True) + media_file.write_bytes(b"media") + monkeypatch.setattr( + "gateway.platforms.base.MEDIA_DELIVERY_SAFE_ROOTS", + (root,), + ) + return media_file.resolve() + + +@pytest.mark.asyncio +async def test_bare_local_path_in_streamed_reply_is_not_uploaded(tmp_path, monkeypatch): + """The #20834 shape: visible reply contains a bare path (from inspected + content), no MEDIA: directive — nothing may be uploaded post-stream.""" + media_file = _allowed_media_path(tmp_path, monkeypatch, "mockup.png") + adapter = _adapter() + + await GatewayRunner._deliver_media_from_response( + _fake_runner({}), + f"The design lives at {media_file} if you want to look later.", + _event(), + adapter, + ) + + adapter.send_multiple_images.assert_not_awaited() + adapter.send_image_file.assert_not_awaited() + adapter.send_document.assert_not_awaited() + adapter.send_video.assert_not_awaited() + adapter.send_voice.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_bare_document_path_in_streamed_reply_is_not_uploaded(tmp_path, monkeypatch): + media_file = _allowed_media_path(tmp_path, monkeypatch, "report.pdf") + adapter = _adapter() + + await GatewayRunner._deliver_media_from_response( + _fake_runner({}), + f"I saved it to {media_file}.", + _event(), + adapter, + ) + + adapter.send_document.assert_not_awaited() + adapter.send_video.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_explicit_media_tag_still_delivers_post_stream(tmp_path, monkeypatch): + """Explicit MEDIA: directives keep working after the #20834 fix.""" + media_file = _allowed_media_path(tmp_path, monkeypatch, "chart.png") + adapter = _adapter() + + await GatewayRunner._deliver_media_from_response( + _fake_runner({}), + f"Here is the chart.\nMEDIA:{media_file}", + _event(), + adapter, + ) + + adapter.send_multiple_images.assert_awaited_once() + images_kwargs = adapter.send_multiple_images.await_args.kwargs + assert images_kwargs["chat_id"] == "C123CHAN" + assert str(media_file) in images_kwargs["images"][0][0] + + +@pytest.mark.asyncio +async def test_explicit_media_document_still_delivers_post_stream(tmp_path, monkeypatch): + media_file = _allowed_media_path(tmp_path, monkeypatch, "report.pdf") + adapter = _adapter() + + await GatewayRunner._deliver_media_from_response( + _fake_runner({}), + f"Report attached.\nMEDIA:{media_file}", + _event(), + adapter, + ) + + adapter.send_document.assert_awaited_once_with( + chat_id="C123CHAN", + file_path=str(media_file), + metadata={}, + )