Skip to content

fix(tools): forward media args through live adapter - #23770

Open
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:codex/23760-send-via-adapter-media
Open

fix(tools): forward media args through live adapter#23770
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:codex/23760-send-via-adapter-media

Conversation

@LeonSGP43

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the live in-process _send_via_adapter() path so gateway adapters receive media_files and force_document just like the existing standalone sender path. Before this change, attachments could silently disappear whenever the gateway runner and adapter lived in the current process.

Related Issue

Fixes #23760

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • Forwarded media_files and force_document through the live adapter.send(...) call in /tools/send_message_tool.py
  • Added a regression test in /tests/tools/test_send_message_tool.py that exercises the live in-process adapter path and asserts both kwargs are preserved

How to Test

  1. Run uv run --frozen pytest -q -o addopts='' tests/tools/test_send_message_tool.py -k TestSendViaAdapterStandaloneFallback
  2. Confirm the new test_live_adapter_forwards_media_kwargs passes
  3. Run uv run --frozen ruff check tools/send_message_tool.py tests/tools/test_send_message_tool.py

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15 / local targeted pytest and ruff

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

  • uv run --frozen pytest -q -o addopts='' tests/tools/test_send_message_tool.py -k TestSendViaAdapterStandaloneFallback -> 6 passed
  • uv run --frozen ruff check tools/send_message_tool.py tests/tools/test_send_message_tool.py -> passed

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery labels May 11, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating the live-adapter media gap. The current patch needs a different dispatch layer before it can provide the intended behavior.

Problems

  • tools/send_message_tool.py:464-465 passes media_files and force_document to adapter.send(). The adapter contract at gateway/platforms/base.py:2889-2895 accepts only chat_id, content, reply_to, and metadata; QQBot's concrete send() has the same shape at gateway/platforms/qqbot/adapter.py:2430-2436. This makes the live path fail with unsupported kwargs rather than upload an attachment.
  • The added mock accepts arbitrary kwargs, so it does not cover that production contract or establish that a native media method was invoked.

Suggested changes

  • Follow the current live Matrix pattern in tools/send_message_tool.py:1725-1760: send text normally, then select send_image_file, send_video, send_voice, or send_document for each attachment and propagate failures.
  • Test against an adapter with the real send() signature and assert the selected media method is awaited.

Automated hermes-sweeper review.

result = await adapter.send(
chat_id=chat_id,
content=chunk,
media_files=media_files,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BasePlatformAdapter.send() and QQBot's concrete send() do not accept media_files or force_document; this call will fail with unexpected keyword arguments. Dispatch each attachment through send_image_file / send_video / send_voice / send_document after the text send instead.

SimpleNamespace(extra={}),
"chat-1",
"hello",
media_files=[("/tmp/report.pdf", False)],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mock accepts arbitrary kwargs, so the test cannot detect that real adapters' send() methods reject media_files and force_document. Use a contract-shaped send(chat_id, content, reply_to=None, metadata=None) plus a media method, and assert the media method is awaited.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: _send_via_adapter ignores media_files parameter — file attachments broken for qqbot

3 participants