From f4df260f26c93f15694698869f3ea8e965eea301 Mon Sep 17 00:00:00 2001 From: Ben Barclay Date: Tue, 21 Jul 2026 17:04:34 +1000 Subject: [PATCH] fix(relay): attach metadata.user_id on guild replies for egress fallback (#68320) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The relay adapter re-attaches an egress discriminator on outbound replies so the connector can resolve the owning tenant. It captured scope_id for scoped (guild) messages and user_id for DMs, but as MUTUALLY EXCLUSIVE: a scoped inbound hit an early return, so the author's user_id was never recorded, and _with_scope only attached user_id when there was no scope_id. Guild replies therefore went out with scope_id only. That's fine while the guild has a provision-time route row. But a MANAGED Discord agent joins guilds dynamically (the shared bot is added to / removed from servers at runtime), and GATEWAY_RELAY_ROUTE_KEYS — the only thing that writes guild route rows — is a self-hosted, static field never stamped for managed agents. So their guild has no route row, the connector's guild-route lookup misses, and with no user_id on the frame there's nothing to fall back to → every guild reply is declined "discord egress declined: target not routed to an onboarded tenant" even though INBOUND resolved the same guild fine (via the author-first SharedSocketRouter.targets() fallback). Fix: capture the authentic author user_id for EVERY inbound (DM and scoped alike) and re-attach it on the outbound reply alongside scope_id. The connector consults it only on a route/scope miss, so carrying both never overrides routing-table resolution. This is the gateway half of the paired gateway-gateway change (makeDiscordTenantOf guild-route-miss author-binding fallback); together they make guild replies resolve the same observed-author way inbound already does. Tests (tests/gateway/relay/test_relay_adapter.py): a guild reply now carries both scope_id AND user_id; a scoped inbound with no author still yields scope_id only (never invents one). Verified fail-without / pass-with. --- gateway/relay/adapter.py | 79 ++++++++++++++--------- tests/gateway/relay/test_relay_adapter.py | 49 ++++++++++++-- 2 files changed, 93 insertions(+), 35 deletions(-) diff --git a/gateway/relay/adapter.py b/gateway/relay/adapter.py index 34cc51522a0bc..cf0646d1b2397 100644 --- a/gateway/relay/adapter.py +++ b/gateway/relay/adapter.py @@ -234,16 +234,23 @@ class RelayAdapter(BasePlatformAdapter): outbound (the agent's reply) can re-assert it for the connector's egress tenant resolution. Never raises — scope tracking must not break inbound. - Two cases, matching the connector's two tenant-resolution paths: - - SCOPED message: remember chat_id -> scope_id. The connector resolves - the tenant from metadata.scope_id (routing table). - - DM (no scope): remember chat_id -> the authentic author user_id. - A DM carries no scope discriminator, so the connector instead resolves - the tenant from the recipient's author binding (resolveByUser); it - needs the user_id on the OUTBOUND action to do that. Without this, a - DM reply has no resolvable discriminator and the connector's egress - guard declines it as "target not routed to an onboarded tenant". - See gateway-gateway routedEgressGuard.ts / the tenant resolvers. + Two discriminators, captured independently (a scoped message has BOTH): + - scope_id: for a scoped (guild/channel) message. The connector's + primary path resolves the tenant from metadata.scope_id (routing + table). + - user_id: the authentic author id, captured for EVERY message (DM + and scoped alike). The connector resolves the tenant from the + recipient's author binding (resolveByUser) when a route lookup + misses. This is the sole discriminator for a DM (no scope), AND the + author-first FALLBACK for a scoped reply whose guild has no route + row — a managed agent joins guilds dynamically, so a provision-time + guild route is not guaranteed. Re-attaching user_id on scoped + replies too lets the connector's guild-route-miss fallback resolve + the tenant the same way inbound already does (SharedSocketRouter + targets()). Without a resolvable discriminator the connector's + egress guard declines the reply as 'target not routed to an + onboarded tenant'. See gateway-gateway routedEgressGuard.ts / + discordTenant.ts (makeDiscordTenantOf). """ try: src = getattr(event, "source", None) @@ -263,28 +270,36 @@ class RelayAdapter(BasePlatformAdapter): platform_value = getattr(platform, "value", platform) if platform_value and platform_value != "relay": self._platform_by_chat[str(chat)] = str(platform_value) - scope = getattr(src, "scope_id", None) - if scope: - self._scope_by_chat[str(chat)] = str(scope) - return - # DM: no scope. Remember the authentic author id for outbound - # author-binding resolution (the user we're replying to in this DM). + # Author id for outbound author-binding resolution. Captured for BOTH + # DM and scoped messages: it's the sole discriminator for a DM and + # the guild-route-miss fallback for a scoped reply. (Formerly captured + # for DMs only, which left managed-agent guild replies with no + # resolvable tenant when the guild had no route row.) user_id = getattr(src, "user_id", None) if user_id: self._dm_user_by_chat[str(chat)] = str(user_id) + scope = getattr(src, "scope_id", None) + if scope: + self._scope_by_chat[str(chat)] = str(scope) except Exception: # noqa: BLE001 - scope tracking must never break inbound pass def _with_scope(self, chat_id: str, metadata: Optional[Dict[str, Any]]) -> Dict[str, Any]: - """Ensure the outbound metadata carries the discriminator the connector's - egress guard needs to resolve the owning tenant. Two cases: + """Ensure the outbound metadata carries the discriminator(s) the connector's + egress guard needs to resolve the owning tenant. - - SCOPED reply: re-attach metadata.scope_id (routing-table resolution). - - DM reply: there is no scope, so re-attach metadata.user_id — the - authentic author id we saw inbound — which the connector resolves to - the tenant via the recipient's author binding (resolveByUser). Without - one of these, egress is declined as 'target not routed to an onboarded - tenant'. See gateway-gateway routedEgressGuard.ts / the tenant resolvers. + - scope_id: re-attached for a scoped reply (guild/channel) → + routing-table resolution (the primary path). + - user_id: the authentic author id we saw inbound, re-attached for + EVERY reply we know it for. It is the sole discriminator for a DM + (no scope), AND the author-first FALLBACK the connector uses when a + scoped reply's guild has no route row (a managed agent joins guilds + dynamically — the guild route may not be provisioned). Carrying both + on a scoped reply is harmless: the connector tries scope_id first and + only falls back to user_id on a route miss. Without a resolvable + discriminator egress is declined as 'target not routed to an + onboarded tenant'. See gateway-gateway routedEgressGuard.ts / + discordTenant.ts. No-op when the relevant value is already present or unknown for this chat. """ @@ -293,13 +308,15 @@ class RelayAdapter(BasePlatformAdapter): scope = self._scope_by_chat.get(str(chat_id)) if scope: meta["scope_id"] = scope - # DM author-binding discriminator. Only meaningful when there's no scope - # (a scoped reply resolves by scope_id); harmless to carry otherwise, but - # we only set it when this chat is a known DM and the field is absent. - if not meta.get("scope_id") and not meta.get("user_id"): - dm_user = self._dm_user_by_chat.get(str(chat_id)) - if dm_user: - meta["user_id"] = dm_user + # Author-binding discriminator. Attached whenever we know the author for + # this chat and it isn't already set — for DMs (the sole discriminator) + # AND scoped replies (the connector's guild-route-miss fallback). It is + # only consulted by the connector when the scope/route lookup misses, so + # carrying it alongside scope_id never overrides routing-table resolution. + if not meta.get("user_id"): + author = self._dm_user_by_chat.get(str(chat_id)) + if author: + meta["user_id"] = author return meta def _platform_is_fronted(self, platform: str) -> bool: diff --git a/tests/gateway/relay/test_relay_adapter.py b/tests/gateway/relay/test_relay_adapter.py index 91d38edd47773..dba0d0edfa238 100644 --- a/tests/gateway/relay/test_relay_adapter.py +++ b/tests/gateway/relay/test_relay_adapter.py @@ -157,6 +157,27 @@ def _make_dm_event(chat_id="dm-1", user_id="user-42"): return MessageEvent(text="hi", source=src, message_type=MessageType.TEXT) +def _make_scoped_event_with_author( + chat_id="chan-1", scope_id="scope-9", user_id="user-42" +): + """An inbound scoped (guild/channel) message that ALSO carries the authentic + author user_id — the real shape of a Discord guild message (it has both a + guild scope_id and an author). Used to prove the adapter re-attaches BOTH + discriminators so the connector can fall back author-first when the guild + has no route row (managed agents join guilds dynamically).""" + from gateway.platforms.base import MessageEvent, MessageType + from gateway.session import SessionSource + + src = SessionSource( + platform=Platform.RELAY, + chat_id=chat_id, + chat_type="channel", + scope_id=scope_id, + user_id=user_id, + ) + return MessageEvent(text="hi", source=src, message_type=MessageType.TEXT) + + @pytest.mark.asyncio async def test_send_reattaches_scope_id_from_inbound_scope(): """The connector's egress guard resolves the owning tenant from @@ -234,10 +255,30 @@ async def test_send_preserves_explicit_user_id(): @pytest.mark.asyncio -async def test_scoped_reply_does_not_carry_user_id(): - """A scoped reply resolves by scope_id and must NOT carry a DM user_id even if - the same chat_id was somehow seen — scope capture wins and user_id stays out - (scope_id is the discriminator; user_id is the DM-only fallback).""" +async def test_scoped_reply_reattaches_both_scope_id_and_user_id(): + """A scoped (guild) reply now re-attaches BOTH scope_id AND the authentic + author user_id. scope_id is the connector's primary discriminator; user_id + is the author-first FALLBACK the connector uses when the guild has no route + row (a managed agent joins guilds dynamically, so a provision-time guild + route is not guaranteed). Regression for live 'discord egress declined: + target not routed to an onboarded tenant' on GUILD replies (paired with + gateway-gateway makeDiscordTenantOf guild-route-miss fallback).""" + t = _CaptureTransport() + a = RelayAdapter(PlatformConfig(), make_desc(platform="discord"), transport=t) + a._capture_scope( + _make_scoped_event_with_author( + chat_id="chan-1", scope_id="scope-9", user_id="user-42" + ) + ) + await a.send("chan-1", "hi") + assert t.sent["metadata"].get("scope_id") == "scope-9" + assert t.sent["metadata"].get("user_id") == "user-42" + + +@pytest.mark.asyncio +async def test_scoped_reply_without_inbound_author_carries_scope_only(): + """A scoped inbound with no author id yields scope_id only — the adapter + never invents a user_id it didn't observe.""" t = _CaptureTransport() a = RelayAdapter(PlatformConfig(), make_desc(platform="discord"), transport=t) a._capture_scope(_make_event(chat_id="chan-1", scope_id="scope-9"))