fix(gateway): stamp profile before busy/approval checks in handle_message - #69156
fix(gateway): stamp profile before busy/approval checks in handle_message#69156Shunkleburger wants to merge 1 commit into
Conversation
…sage BasePlatformAdapter.handle_message() runs its active-session busy check, approval-mode gate, and draining check -- and builds the session key used by all of them -- before self._message_handler is ever called. But _make_profile_message_handler() (gateway/run.py) only stamps event.source.profile *inside* that handler, so on a multiplexed gateway every one of those upstream checks resolved source.profile as unset and _adapter_for_source() silently fell back to the default profile's adapter. In practice: a secondary profile's busy-session state collided under the default profile's agent:main namespace, and any busy-session reply for that profile went out through the default bot instead of its own. Give each adapter a profile_name known synchronously at construction time (set in GatewayRunner._configure_profile_adapter, covering both startup and reconnect), and stamp event.source.profile from it at the top of handle_message, before any routing decision reads it. Also route TelegramAdapter's text/photo batch-key builders off self.profile_name instead of the not-yet-stamped event.source.profile, for the same reason. profile_name reads use getattr(..., None) because ~40 existing tests construct adapters via object.__new__(), bypassing __init__.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the pre-handler busy-key path; current main still builds that key before _message_handler runs (gateway/platforms/base.py:5413-5428), so the secondary-adapter premise is valid.
Problems
- The added
profile=_profile_namewould discard a profile already stamped bybuild_source()forgateway.profile_routes(gateway/platforms/base.py:6439-6485) when ingress uses the default/shared adapter. That adapter has noprofile_name, although routed delivery through it is intentional (tests/gateway/test_profile_resolution.py:410-431), so its busy key remainsagent:main.
Suggested changes
- Prefer
event.source.profileand use the adapter-owned name only as fallback when constructing the key. Add a shared-adapter/profile-route busy-session regression test alongside the secondary-adapter case.
Automated hermes-sweeper review.
| @@ -4834,6 +4856,7 @@ async def handle_message(self, event: MessageEvent) -> None: | |||
| event.source, | |||
There was a problem hiding this comment.
_profile_name is None for the default/shared adapter, but build_source() can already have stamped event.source.profile from gateway.profile_routes. Prefer the source profile here and use _profile_name only as fallback; otherwise routed ingress still keys its busy state under agent:main.
|
I independently reproduced one remaining part of this PR's Telegram path against current The batching hunk here is useful. The rest of this branch overlaps newer routing work and the PR is currently blocked/stale, so I am preparing a narrow, behavior-tested follow-up stacked on #82980 rather than reviving the wider older design. It will preserve the legacy no-profile key and reference this prior work. If maintainers prefer this PR itself to be rebased instead, I am happy to defer to that direction. |
|
Closing as superseded: commit 21260c3 (PR #89860, salvage of #88437 by @69k4xmdfm2-blip) fixed #88404 at the adapter-ownership seam — the owner profile is now installed in _configure_profile_adapter before any inbound event, the ingress session key resolves via _session_key_profile(), and the busy path is profile-stamped. Every site this PR patched is covered on main. You were one of the earliest to identify this bug class — thanks for pushing on it. |
What does this PR do?
Fixes cross-profile Telegram (and general platform-adapter) reply misrouting on a multiplexed gateway. When a secondary profile's session is busy and a follow-up message arrives,
BasePlatformAdapter.handle_message()resolves its busy/approval/draining state and builds the session key before the profile is known, so that state silently collided with the default profile'sagent:mainnamespace and any resulting reply went out through the default bot instead of the secondary profile's own bot.Related Issue
Type of Change
Root Cause
_make_profile_message_handler()(gateway/run.py) stampsevent.source.profilefor a secondary profile, but only insideself._message_handler.BasePlatformAdapter.handle_message()runs its active-session busy check, approval-mode gate, draining check, andbuild_session_key()call beforeself._message_handleris ever invoked — all of that readsevent.source.profilewhile it is still unset, so_adapter_for_source()and the busy-session bucket both fall back to the default profile.Net effect on a running multiplexed gateway: every secondary profile's busy-session state lived under the default profile's
agent:mainkey instead of its ownagent:<profile>key, and a busy-session follow-up reply for a secondary profile's chat was dispatched/sent through the default profile's adapter (wrong bot token, wrong chat in the worst case).Changes Made
gateway/platforms/base.py: addedself.profile_nametoBasePlatformAdapter.__init__(defaults toNone); at the top ofhandle_message(), stampevent.source.profilefromself.profile_namebefore any busy/approval/draining check reads it, and passprofile=explicitly into thebuild_session_key()call in that method.gateway/run.py:_configure_profile_adapter()now setsadapter.profile_name = profile_namesynchronously, covering both the startup path and the reconnect path (previously only the deferred message-handler wrapper set the equivalent state, too late for the checks above).plugins/platforms/telegram/adapter.py:_text_batch_key()and_photo_batch_key()now key offself.profile_nameinstead of the not-yet-stampedevent.source.profile.tests/gateway/test_multiplex_busy_session_profile_routing.py(new): two regression tests — one assertsevent.source.profileis stamped synchronously before the busy check runs, the other asserts a secondary profile's busy-session state lands under its ownagent:<profile>key and never under the defaultagent:mainkey or an unscoped bucket.All
profile_namereads usegetattr(self, "profile_name", None)rather than a direct attribute access, because a number of existing tests construct adapters viaobject.__new__()and bypass__init__entirely.How to Test
pytest tests/gateway/test_multiplex_busy_session_profile_routing.py tests/gateway/test_active_session_text_merge.py -q— both new tests fail against the pre-patch code with the exact real-world symptom (the follow-up lands in the unscoped{}bucket instead of the profile-scoped one) and pass with the patch applied.tests/gateway/suite:pytest tests/gateway/ -q— 9841 passed, 14 failed, 11 skipped, and all 14 failures are pre-existing onmain(verified by running the same 14 tests against the pre-patch parent commit; identical failures, unrelated to profile/session routing).Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass — ranpytest tests/gateway/ -q(the affected subsystem) locally; did not run the full repo-widetests/suitetests/gateway/run)Documentation & Housekeeping
docs/, docstrings) — N/A, internal routing fix with no external-facing behavior/config change; explanatory comments added inline at each of the three fix sitescli-config.yaml.exampleif I added/changed config keys — N/A, no config keys changedCONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/A