feat(gateway): declare per-adapter media-send capabilities - #61694
feat(gateway): declare per-adapter media-send capabilities#61694sprmn24 wants to merge 1 commit into
Conversation
Extends PlatformEntry and the relay CapabilityDescriptor with a media_capabilities field so each of the 15 platform adapters declares which media kinds (voice/video/document/image_file/animation) it natively sends, vs falling back to BasePlatformAdapter's text-notice fallback. This is additive metadata only — no adapter behavior changes. Groundwork for routing tools/send_message_tool.py's cross-channel send path through declared capabilities instead of a hardcoded per-platform allowlist (see NousResearch#17261), so future adapter media support can ship entirely inside a plugin without a core send_message_tool.py edit.
Related to #36817 (per-adapter |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the capability declaration work.
Problems
- This does not yet change the reported Slack behavior: current
tools/send_message_tool.py:1034-1047omits Slack from MEDIA delivery, and its Slack branch calls the text-only standalone sender (tools/send_message_tool.py:1051-1062;plugins/platforms/slack/adapter.py:4350-4355). CapabilityDescriptor.from_platform_entry()has no production caller on current main; its only call sites are descriptor tests.RelayAdapter._apply_descriptor()currently consumes only length and markdown behavior (gateway/relay/adapter.py:221-225). Adding this field to the connector contract therefore needs agreement on the #36817 capability model before it becomes useful.- The diff adds 15 manually duplicated declarations without coverage that they match concrete adapter
send_*overrides or that a non-empty descriptor field round-trips through JSON.
Suggested changes
- Converge with #36817 on whether this belongs in the relay descriptor, then add projection/JSON and declaration-to-override contract tests.
- Keep the Slack routing/upload work as the explicit follow-up; this metadata alone cannot fix #17261.
Automated hermes-sweeper review.
| emoji: str = "\U0001f50c" # 🔌 default (matches PlatformEntry default) | ||
| platform_hint: str = "" | ||
| pii_safe: bool = False | ||
| media_capabilities: tuple[str, ...] = () |
There was a problem hiding this comment.
This extends the connector handshake contract, but current production has no caller of from_platform_entry() and RelayAdapter._apply_descriptor() does not consume this capability. Please first converge with #36817 on the negotiated capability model, or keep this metadata out of the relay descriptor until its connector consumer ships.
| @@ -4568,6 +4568,7 @@ def register(ctx) -> None: | |||
| # Slack API allows 40,000 chars; leave margin (matches the legacy | |||
There was a problem hiding this comment.
Please add a contract test that registered media kinds correspond to concrete send_* overrides. The declaration is manually duplicated metadata, and the current test suite does not cover these new registrations or a non-empty descriptor JSON round trip.
|
Reviewed #36817 , the MEDIA_KINDS class-attribute approach on BasePlatformAdapter and this PR's PlatformEntry registration-time field serve the same goal from different angles. Happy to drop media_capabilities from the relay CapabilityDescriptor (the premature-contract concern) and keep only the PlatformEntry field as a lighter complement to #36817's dispatch-side work, if that's the preferred split. Or I can rebase on top of #36817 once it lands. What's the direction you want? |
What does this PR do?
Adds a
media_capabilitiesfield toPlatformEntryand the relayCapabilityDescriptor, so each platform adapter declares which media kinds (voice/video/document/image_file/animation) it natively sends via a realsend_*override, as opposed to falling back toBasePlatformAdapter's generic text-notice fallback.This is groundwork for the plugin-interface expansion discussed in the #Developer channel: once
tools/send_message_tool.py's cross-channel send routes through this declared capability set instead of its current hardcoded per-platform allowlist, adapters (core or plugin) will be able to add/fix media support entirely inside their ownregister(ctx)call — no coresend_message_tool.pyedit required.Related Issue
Groundwork toward #17261 (Slack cross-channel media falls through to text-only). This PR is additive metadata only and does not change
send_message_tool.py's routing yet — that follow-up is intentionally separated out pending direction on the capability-routing approach.Type of Change
Changes Made
gateway/platform_registry.py: newmedia_capabilities: frozenset[str] = frozenset()field onPlatformEntry.gateway/relay/descriptor.py: newmedia_capabilities: tuple[str, ...] = ()field onCapabilityDescriptor, projected infrom_platform_entry(), with a JSON round-trip fix infrom_json()(list→tuple cast).docs/relay-connector-contract.md: documented the new descriptor field (required bytest_contract_doc_conformance.py).media_capabilities=frozenset({...})to theirregister(ctx)call, matching their actualsend_voice/send_video/send_document/send_image_file/send_animationoverrides (verified per-adapter against source, not assumed).How to Test
scripts/run_tests.sh tests/gateway tests/plugins— full suite passes except two pre-existing flakes unrelated to this change (confirmed identical failures on unmodified main): an mtime-memoization test and an asyncio-timeout race intest_startup_restart_race.py.Checklist