fix(agentex): put the connect link where the user is actually looking

agentex/src/domain/use_cases/slack_gateway_use_case.py

@@ -1296,16 +1296,10 @@ async def _offer_link(self, inbound: InboundSlack) -> bool:
             logger.warning("[slack] link offer failed to mint a nonce", exc_info=True)
             return False
 
-        if not allowed:
-            # Already DMed about this link. Acknowledge in-channel (ephemerally) so
-            # the user isn't left wondering, but don't send another DM.
-            await self._post_ephemeral(
-                inbound,
-                "I've already sent you a DM with a link to connect your account — "
-                "check your direct messages with me.",
-            )
-            return False
-
+        # Opened before the send-cap check because BOTH branches need the channel id
+        # to build the deep link below — telling someone to "check your DMs" without
+        # taking them there is the failure this method exists to avoid. Idempotent:
+        # conversations.open returns the existing DM rather than creating another.
         opened = await self._slack_api("conversations.open", {"users": inbound.user})
         dm_channel = (
             (opened.get("channel") or {}).get("id") if opened.get("ok") else None
@@ -1318,7 +1312,38 @@ async def _offer_link(self, inbound: InboundSlack) -> bool:
             )
             return False
 
+        # Slack files bot conversations under "Apps", NOT in the Direct messages
+        # list, so a person told to check their DMs looks in the one place the
+        # message isn't. Observed in the field: the first real offer was delivered
+        # correctly, confirmed present via conversations.history, and still reported
+        # as never received. app_redirect works on web and desktop, unlike a
+        # slack:// URI.
+        dm_deeplink = f"https://slack.com/app_redirect?channel={dm_channel}&team={inbound.team_id}"
         url = f"{_PUBLIC_BASE_URL}/integrations/slack/link?nonce={token}"
+
+        # The connect link goes in the ephemeral as well as the DM. An ephemeral is
+        # single-viewer — Slack renders it for one user, keeps it out of channel
+        # history and out of search — so it has exactly the same audience as the DM,
+        # and putting the link there costs a round trip through a conversation people
+        # cannot find. The DM stays because ephemerals are transient: reload Slack
+        # before clicking and it's gone, and the offer cooldown would then block a
+        # retry for an hour.
+        #
+        # This is NOT licence to put the link in an ordinary channel message. The
+        # nonce is a bearer token; the first reader of a broadcast could bind this
+        # user's Slack identity to their own SGP account. Single-viewer is the
+        # property that makes the ephemeral safe, not "it's in the channel anyway".
+        if not allowed:
+            # Past the DM cap — but an ephemeral costs nothing and is what the user
+            # is actually looking at, so still hand them the live link.
+            await self._post_ephemeral(
+                inbound,
+                f"<{url}|Connect your SGP account> to let me use your own tools "
+                f"when you ask me things here.\n"
+                f"_Only you can see this. The same link is in "
+                f"<{dm_deeplink}|our DM>._",
+            )
+            return False
         posted = await self._slack_api(
             "chat.postMessage",
             {
@@ -1347,8 +1372,10 @@ async def _offer_link(self, inbound: InboundSlack) -> bool:
         )
         await self._post_ephemeral(
             inbound,
-            "I've DM'd you a link to connect your SGP account — once you do, I'll "
-            "use your own tools when you ask me things here.",
+            f"<{url}|Connect your SGP account> and I'll use your own tools "
+            f"(Notion, Linear, …) when you ask me things here.\n"
+            f"_Only you can see this message. I've also sent the link to "
+            f"<{dm_deeplink}|our DM>, in case this one disappears._",
         )
         return True
 

agentex/tests/unit/use_cases/test_slack_gateway_use_case.py

@@ -1745,14 +1745,31 @@ async def test_dms_the_link_and_acknowledges_in_channel(self, monkeypatch):
         # onboarding.
         assert calls["chat.postEphemeral"]["user"] == "U1"
 
-    async def test_the_link_never_goes_to_the_origin_channel(self, monkeypatch):
+    async def test_the_link_is_never_broadcast(self, monkeypatch):
+        """The invariant is single-viewer, not "not in the channel".
+
+        The nonce is a bearer token, so the first reader of a channel-visible message
+        could bind this user's Slack identity to their own SGP account. It may ride
+        in the DM and in an ephemeral — both of which exactly one person can see —
+        but a chat.postMessage must never carry it anywhere but that user's own DM.
+        """
         uc, api, _ = self._wire(monkeypatch)
         await uc._offer_link(_inbound(channel="C_PUBLIC"))
+
         for method, payload in (c.args for c in api.await_args_list):
-            if payload.get("channel") == "C_PUBLIC":
-                assert "TOKEN123" not in str(
-                    payload
-                ), f"{method} leaked the nonce into the origin channel"
+            if method == "chat.postMessage":
+                assert (
+                    payload["channel"] == "D_DM"
+                ), f"the nonce was posted to {payload['channel']}, not the DM"
+        # Nothing that lands in channel history mentions it.
+        broadcast = [
+            p
+            for m, p in (c.args for c in api.await_args_list)
+            if m == "chat.postMessage"
+        ]
+        assert all(
+            "TOKEN123" not in str(p) or p["channel"] == "D_DM" for p in broadcast
+        )
 
     async def test_dm_warns_against_forwarding(self, monkeypatch):
         uc, api, _ = self._wire(monkeypatch)
@@ -1782,7 +1799,10 @@ async def test_send_cap_reached_says_so_without_a_second_dm(self, monkeypatch):
         assert await uc._offer_link(_inbound()) is False
         methods = [m for m, _ in (c.args for c in api.await_args_list)]
         # Acknowledge in-channel rather than going silent, but send no new DM.
-        assert methods == ["chat.postEphemeral"]
+        # conversations.open still runs first — it's idempotent, and this branch
+        # needs the channel id to deep-link the user to the DM they can't find.
+        assert methods == ["conversations.open", "chat.postEphemeral"]
+        assert "chat.postMessage" not in methods
 
     async def test_cooldown_suppresses_the_offer_entirely(self, monkeypatch):
         uc, api, nonce = self._wire(monkeypatch, cooldown_ok=False)
@@ -1803,3 +1823,96 @@ async def test_pending_turn_is_stashed_for_later_replay(self, monkeypatch):
         assert req.external_user_id == "U1"
         assert req.pending_turn["text"] == "what's in my linear?"
         assert req.pending_turn["channel"] == "C9"
+
+
+@pytest.mark.unit
+@pytest.mark.asyncio
+class TestLinkOfferDiscoverability:
+    """The offer has to be findable, not merely delivered.
+
+    Observed in production on the first real offer: the DM was posted successfully
+    and confirmed present via conversations.history, and the recipient still reported
+    never receiving it — because Slack files bot conversations under "Apps" rather
+    than in the Direct messages list. "Check your DMs" points at the one place the
+    message isn't, so the ephemeral carries a deep link into the conversation.
+    """
+
+    def _wire(self, monkeypatch, *, allowed=True):
+        uc = SlackGatewayUseCase()
+        monkeypatch.setattr(sg, "_PUBLIC_BASE_URL", "https://agentex.example.com")
+        monkeypatch.setattr(
+            sg, "slack_user_profile", AsyncMock(return_value={"display_name": "@ada"})
+        )
+        monkeypatch.setattr(uc, "_claim_offer_cooldown", AsyncMock(return_value=True))
+        nonce = MagicMock()
+        nonce.create_or_reuse = AsyncMock(return_value=("TOKEN123", False))
+        nonce.claim_send = AsyncMock(return_value=allowed)
+        monkeypatch.setattr(
+            "src.domain.services.link_nonce_service.LinkNonceService",
+            lambda *a, **k: nonce,
+        )
+        api = AsyncMock(
+            side_effect=lambda m, p: (
+                {"ok": True, "channel": {"id": "D_DM"}}
+                if m == "conversations.open"
+                else {"ok": True}
+            )
+        )
+        monkeypatch.setattr(uc, "_slack_api", api)
+        return uc, api
+
+    def _ephemeral(self, api):
+        return next(
+            p
+            for m, p in (c.args for c in api.await_args_list)
+            if m == "chat.postEphemeral"
+        )
+
+    async def test_ephemeral_points_at_the_durable_copy(self, monkeypatch):
+        # The link is inline now, so the deep link is no longer how you reach it —
+        # it's the pointer to the copy that survives a reload, since ephemerals
+        # don't. app_redirect rather than a slack:// URI, which fails on web.
+        uc, api = self._wire(monkeypatch)
+        await uc._offer_link(_inbound(team_id="T_ACME"))
+
+        text = self._ephemeral(api)["text"]
+        assert "https://slack.com/app_redirect?channel=D_DM&team=T_ACME" in text
+        assert "TOKEN123" in text
+
+    async def test_the_ephemeral_carries_the_link_itself(self, monkeypatch):
+        # Same audience as the DM (one person), and it's where the user is already
+        # looking — which is the whole point, since bot DMs are filed under "Apps"
+        # where people don't think to look.
+        uc, api = self._wire(monkeypatch)
+        await uc._offer_link(_inbound())
+        eph = self._ephemeral(api)
+        assert "TOKEN123" in eph["text"]
+        assert eph["user"] == "U1"
+        # Still says where the durable copy is, since ephemerals vanish on reload.
+        assert "app_redirect" in eph["text"]
+
+    async def test_send_cap_ephemeral_also_deep_links(self, monkeypatch):
+        # The "already sent" path is exactly when someone can't find the DM, so it
+        # needs the link more than the happy path does.
+        uc, api = self._wire(monkeypatch, allowed=False)
+        assert await uc._offer_link(_inbound(team_id="T_ACME")) is False
+
+        text = self._ephemeral(api)["text"]
+        # Past the DM cap the user still gets the live link — the cap limits DMs,
+        # not what we're allowed to show the person in front of us.
+        assert "TOKEN123" in text
+        assert "https://slack.com/app_redirect?channel=D_DM&team=T_ACME" in text
+        # But no second DM.
+        methods = [m for m, _ in (c.args for c in api.await_args_list)]
+        assert "chat.postMessage" not in methods
+
+    async def test_unreachable_dm_offers_nothing(self, monkeypatch):
+        # If we can't open the DM we can't link to it either, so pointing someone at
+        # a conversation that doesn't exist would be worse than staying quiet.
+        uc, api = self._wire(monkeypatch)
+        monkeypatch.setattr(
+            uc,
+            "_slack_api",
+            AsyncMock(return_value={"ok": False, "error": "cannot_dm_bot"}),
+        )
+        assert await uc._offer_link(_inbound()) is False