feat(plugins): add standalone_sender_fn for out-of-process cron delivery - #22461
Merged
Merged
Conversation
Plugin platforms (IRC, Teams, Google Chat) currently fail with `No live adapter for platform '<name>'` when a `deliver=<plugin>` cron job runs in a separate process from the gateway, even though the platforms are eligible cron targets via `cron_deliver_env_var` (added in #21306). Built-in platforms (Telegram, Discord, Slack, etc.) use direct REST helpers in `tools/send_message_tool.py` so cron can deliver without holding the gateway in the same process; plugin platforms historically depended on `_gateway_runner_ref()` which returns `None` out of process. This change adds an optional `standalone_sender_fn` field to `PlatformEntry` so plugins can register an ephemeral send path that opens its own connection, sends, and closes without needing the live adapter. The dispatch site in `_send_via_adapter` falls through to the hook when the gateway runner is unavailable, with a descriptive error when neither path applies. The hook is optional, so existing plugins are unaffected. Reference migrations land in the same change for IRC, Teams, and Google Chat, exercising the hook across stdlib (asyncio + IRC protocol), Bot Framework OAuth client_credentials, and Google service-account flows respectively. Security hardening on the new code paths: * IRC: control-character stripping on chat_id and message body to block CRLF command injection; bounded nick-collision retries; JOIN before PRIVMSG so channels with the default `+n` mode accept the delivery. * Teams: TEAMS_SERVICE_URL validated against an allowlist of known Bot Framework hosts (`smba.trafficmanager.net`, `smba.infra.gov.teams.microsoft.us`) to block SSRF; chat_id and tenant_id constrained to the documented Bot Framework character set; per-request timeouts so a slow STS endpoint cannot starve the activity POST. * Google Chat: chat_id and thread_id validated against strict resource-name regexes; service-account refresh wrapped in `asyncio.wait_for` so a hung token endpoint cannot stall the scheduler. Test coverage: 20 new tests covering happy path, missing-config errors, network failure modes, and each defensive validation. Existing tests unchanged. `bash scripts/run_tests.sh tests/tools/test_send_message_tool.py tests/gateway/test_irc_adapter.py tests/gateway/test_teams.py tests/gateway/test_google_chat.py` reports 341 passed, 0 regressions. Documentation: new "Out-of-process cron delivery" section in website/docs/developer-guide/adding-platform-adapters.md and an entry in gateway/platforms/ADDING_A_PLATFORM.md naming the hook.
Contributor
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-attribute |
1 |
invalid-argument-type |
1 |
unresolved-import |
1 |
First entries
tests/gateway/test_google_chat.py:2849: [unresolved-attribute] unresolved-attribute: Attribute `Credentials` is not defined on `None` in union `Unknown | None`
tests/hermes_cli/test_setup_irc.py:40: [invalid-argument-type] invalid-argument-type: Argument is incorrect: Expected `((...) -> Awaitable[dict[Unknown, Unknown]]) | None`, found `str | ((cfg) -> None) | (() -> bool) | ... omitted 4 union elements`
plugins/platforms/google_chat/adapter.py:3166: [unresolved-import] unresolved-import: Cannot resolve imported module `aiohttp`
✅ Fixed issues: none
Unchanged: 4171 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
28 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Salvage of #21805 by @GodsBoy onto current
main.What this does
Adds an optional
standalone_sender_fnfield toPlatformEntryso plugin platforms (IRC, Teams, Google Chat) can register an out-of-process send path. Whenhermes cron tickruns from system crontab while the gateway is offline,_gateway_runner_ref()returnsNoneand_send_via_adaptercurrently fails withNo live adapter for platform '<name>'. The PR adds the missing dispatch fall-through and ships REST/protocol-level reference implementations for all three currently-bundled plugin platforms.This is the missing third phase of plugin platform parity:
2e20f6ae2(Apr 11) added in-process_send_via_adapter.af9336d57(May 7) addedcron_deliver_env_varso plugins become eligible cron targets.The hook is optional; existing plugins are unaffected.
Reference implementations
_standalone_sendopens an ephemeral TCP/TLS connection with a-cronnick suffix to avoid collisions with a live gateway adapter, JOINs the channel before PRIVMSG (handles default+nchannel mode), and QUITs cleanly. NickServ auth, NICK collision retry, bytes-aware per-line splitting under the 510-byte IRC limit. CRLF injection blocked at chat_id and message body.client_credentialstoken grant againstlogin.microsoftonline.com, then POST to Bot Framework/v3/conversations/<id>/activities.TEAMS_SERVICE_URLvalidated against an allowlist of Bot Framework hosts (smba.trafficmanager.net,smba.infra.gov.teams.microsoft.us) to block SSRF.chat_idvalidated against the documented Bot Framework conversation-ID character set.creds.refreshwrapped inasyncio.wait_for(timeout=10)to bound hung STS endpoints, then POST to Chat REST API.chat_idandthread_idvalidated against strict resource-name regexes.Tests
20 new tests across 4 files covering: success path, missing config, registration timeout, CRLF injection, JOIN-before-PRIVMSG ordering, off-allowlist SSRF, chat_id path traversal, kwargs forwarding, return-shape validation.
Local on rebased code: 439 passed, 2 skipped, 0 regressions across
tests/gateway/test_irc_adapter.py,tests/gateway/test_teams.py,tests/gateway/test_google_chat.py,tests/gateway/test_platform_base.py,tests/tools/test_send_message_tool.py.Why salvage
PR #21805 was 153 commits behind
mainwith touches on 5 of the 11 changed files since the PR's base (Teams pipeline refactor, GChat sender_type fix, Telegram topic fix, etc.). Cherry-pick auto-merged cleanly with+1457/-25matching the PR's stats exactly. @GodsBoy's authorship preserved.Closes #21805. Closes #21804.
Use
--rebasemerge to preserve @GodsBoy's authorship.