github: eager bot-user-ID auto-detection (slice of upstream 9824d33)

Port the github slice of upstream `9824d33` (PR #441 adapter-hardening
pass) so `is_me` checks work and self-reply loops are prevented in
multi-tenant deployments.

Adds `_detect_bot_user_id` mirroring upstream's exact sequence and
fallback:
- try `users.getAuthenticated` (`GET /user`) first — works for PAT mode
  and (returns the bot user) for installation tokens too
- on failure, fall back to `apps.getAuthenticated` (`GET /app`) and
  resolve the bot user via `users.getByUsername`
  (`GET /users/{slug}[bot]`)

Detection runs eagerly on the first webhook for an installation (before
dispatch, so the very first reply sees a populated bot id) and is cached
on `self._bot_user_id` so it's fetched once per process, not per webhook.
`initialize()` and `remove_reaction()` now route through the same method.
`_github_api_request` gains an `installation_id` override so detection
can authenticate for the webhook's installation in multi-tenant mode
(reuses existing `_get_installation_token` plumbing).

Tests (mock the GitHub API client via AsyncMock):
- first webhook for a new installation populates `_bot_user_id` before
  message handling proceeds
- a message authored by the bot itself is filtered as `is_me`
- PAT path uses the `users.getAuthenticated` (`GET /user`) path
- second webhook for same installation does not re-fetch (cache hit)

Refs #98. github slice of the 4-part `9824d33` security pass.

https://claude.ai/code/session_01FyMxQn2BEAzmwKS1GZczKj

Author: claude

src/chat_sdk/adapters/github/adapter.py

@@ -201,21 +201,68 @@ async def initialize(self, chat: ChatInstance) -> None:
         """Initialize the adapter."""
         self._chat = chat
 
-        if not self._bot_user_id and self._auth_token:
-            try:
-                user_data = await self._github_api_request("GET", "/user")
-                self._bot_user_id = user_data.get("id")
+        # Fetch bot user ID if not provided. For multi-tenant mode there is no
+        # fixed installation yet, so detection happens lazily on the first
+        # webhook (see ``handle_webhook``) so ``is_me`` checks work for the
+        # very first reply.
+        if self._bot_user_id is None and self._auth_token:
+            await self._detect_bot_user_id()
+
+        self._logger.info("GitHub adapter initialized")
+
+    async def _detect_bot_user_id(self, installation_id: int | None = None) -> None:
+        """Fetch the bot's user ID from GitHub. Used for self-message detection.
+
+        Best-effort: errors are swallowed so they do not block webhook
+        processing. The bot identity is the same across all installations of
+        the same App, so we only detect once and cache the result on
+        ``self._bot_user_id``.
+
+        Mirrors the upstream TypeScript ``detectBotUserId`` sequence:
+        try ``users.getAuthenticated`` (``GET /user``) first — this works for
+        PAT mode and (returns the bot user) for installation tokens too. On
+        failure, fall back to ``apps.getAuthenticated`` (``GET /app``) and
+        resolve the bot user via ``users.getByUsername`` (``GET /users/{slug}[bot]``).
+        """
+        # Cache hit: already detected once, don't re-fetch per webhook.
+        if self._bot_user_id is not None:
+            return
+
+        try:
+            user = await self._github_api_request("GET", "/user", installation_id=installation_id)
+            self._bot_user_id = user.get("id")
+            self._logger.info(
+                "GitHub bot user ID auto-detected",
+                {
+                    "botUserId": self._bot_user_id,
+                    "login": user.get("login"),
+                },
+            )
+            return
+        except Exception as error:
+            self._logger.debug(
+                "users.getAuthenticated failed; falling back to apps.getAuthenticated",
+                {"error": str(error)},
+            )
+
+        try:
+            # For App-authenticated installation tokens, /user is not available;
+            # use apps.getAuthenticated and resolve the bot user via
+            # /users/{login}[bot].
+            app = await self._github_api_request("GET", "/app", installation_id=installation_id)
+            if app:
+                login = f"{app['slug']}[bot]"
+                bot_user = await self._github_api_request("GET", f"/users/{login}", installation_id=installation_id)
+                self._bot_user_id = bot_user.get("id")
                 self._logger.info(
-                    "GitHub auth completed",
+                    "GitHub bot user ID auto-detected via app slug",
                     {
                         "botUserId": self._bot_user_id,
-                        "login": user_data.get("login"),
+                        "login": login,
                     },
                 )
-            except Exception as error:
-                self._logger.warn("Could not fetch bot user ID", {"error": str(error)})
-
-        self._logger.info("GitHub adapter initialized")
+        except Exception as error:
+            self._logger.warn("Could not auto-detect GitHub bot user ID", {"error": str(error)})
 
     async def get_user(self, user_id: str) -> UserInfo | None:
         """Look up a GitHub user by numeric account ID.
@@ -306,6 +353,13 @@ async def handle_webhook(self, request: Any, options: WebhookOptions | None = No
             if owner_login and repo_name:
                 await self._store_installation_id(owner_login, repo_name, installation_id)
 
+        # Eagerly resolve the bot user ID before dispatching to handlers, so
+        # is_me checks work on the very first webhook. Without this,
+        # multi-tenant adapters can self-reply-loop until detection fires
+        # lazily elsewhere. Cached after the first detection (once per process).
+        if self._bot_user_id is None:
+            await self._detect_bot_user_id(installation_id)
+
         if event_type == "issue_comment":
             if payload.get("action") == "created":
                 self._handle_issue_comment(payload, installation_id, options)
@@ -629,12 +683,10 @@ async def add_reaction(self, thread_id: str, message_id: str, emoji: EmojiValue
 
     async def remove_reaction(self, thread_id: str, message_id: str, emoji: EmojiValue | str) -> None:
         """Remove a reaction from a message."""
-        if not self._bot_user_id and self._auth_token:
-            try:
-                user_data = await self._github_api_request("GET", "/user")
-                self._bot_user_id = user_data.get("id")
-            except Exception:
-                self._logger.warn("Could not detect bot user ID for reaction removal")
+        # Multi-tenant mode has no fixed installation, so initialize() can't
+        # detect _bot_user_id. Detect lazily (cached after first success).
+        if self._bot_user_id is None:
+            await self._detect_bot_user_id()
 
         decoded = self.decode_thread_id(thread_id)
         comment_id = int(message_id)
@@ -1059,24 +1111,31 @@ async def _get_installation_token(self, installation_id: int) -> str:
             )
             return token
 
-    async def _github_api_request(self, method: str, path: str, body: Any = None) -> Any:
+    async def _github_api_request(
+        self, method: str, path: str, body: Any = None, *, installation_id: int | None = None
+    ) -> Any:
         """Make a request to the GitHub API.
 
         Supports PAT auth (``_auth_token``) and GitHub App auth
         (``_app_credentials`` with JWT -> installation token exchange).
+
+        ``installation_id`` overrides ``self._installation_id`` for App auth.
+        Multi-tenant mode has no fixed installation on the adapter (it is
+        extracted per-webhook), so callers operating inside a webhook context
+        pass the webhook's installation explicitly.
         """
         auth_token = self._auth_token
 
         # GitHub App auth: exchange JWT for installation token
         if not auth_token and self._app_credentials:
-            installation_id = self._installation_id
-            if not installation_id:
+            resolved_installation_id = installation_id if installation_id is not None else self._installation_id
+            if not resolved_installation_id:
                 raise RuntimeError(
                     "Installation ID required for GitHub App authentication. "
                     "This usually means you're trying to make an API call outside of a webhook context. "
                     "For proactive messages, use thread IDs from previous webhook interactions."
                 )
-            auth_token = await self._get_installation_token(installation_id)
+            auth_token = await self._get_installation_token(resolved_installation_id)
 
         headers: dict[str, str] = {
             "Accept": "application/vnd.github+json",

tests/test_github_dispatch.py

@@ -281,3 +281,188 @@ async def test_in_reply_to_id_routes_to_root_thread(self):
         assert decoded.review_comment_id == root_comment_id
         # The thread_id format should contain :rc:3000
         assert f":rc:{root_comment_id}" in thread_id
+
+
+# =============================================================================
+# Tests -- eager bot-user-ID auto-detection (github slice of upstream 9824d33)
+# =============================================================================
+
+
+def _make_multi_tenant_adapter(**overrides: Any) -> GitHubAdapter:
+    """Create a multi-tenant GitHub App adapter (no installation_id)."""
+    defaults: dict[str, Any] = {
+        "webhook_secret": WEBHOOK_SECRET,
+        "app_id": "12345",
+        "private_key": "-----BEGIN RSA PRIVATE KEY-----\nfake\n-----END RSA PRIVATE KEY-----",
+        "logger": ConsoleLogger("error"),
+    }
+    defaults.update(overrides)
+    return GitHubAdapter(defaults)
+
+
+def _make_install_payload(*, sender_id: int = 100, installation_id: int = 54321) -> dict[str, Any]:
+    """An issue_comment payload carrying an installation id (multi-tenant)."""
+    payload = _make_issue_comment_payload(sender_id=sender_id, owner="acme", repo="app", pr_number=42)
+    payload["installation"] = {"id": installation_id, "node_id": "MDIzOk"}
+    return payload
+
+
+class TestEagerBotUserIdDetection:
+    """Eager bot-user-ID detection so is_me works and self-reply loops are prevented.
+
+    Mirrors the github slice of upstream commit 9824d33: detection runs on the
+    first webhook for an installation (so the very first reply has a populated
+    bot id), is cached so subsequent webhooks don't re-fetch, and falls back
+    from users.getAuthenticated (GET /user) to apps.getAuthenticated (GET /app)
+    + users.getByUsername (GET /users/{slug}[bot]) for installation tokens.
+    """
+
+    @pytest.mark.asyncio
+    async def test_webhook_populates_bot_user_id_before_dispatch(self):
+        """First webhook for a new installation populates _bot_user_id before message handling."""
+        adapter = _make_multi_tenant_adapter()
+        chat = MagicMock()
+        chat.process_message = MagicMock()
+        chat.get_state = MagicMock(return_value=MagicMock(set=AsyncMock(), get=AsyncMock(return_value=None)))
+        adapter._chat = chat
+
+        # Installation tokens: /user is unavailable, so detection falls back to
+        # /app then /users/{slug}[bot].
+        async def fake_api(method: str, path: str, body: Any = None, *, installation_id: int | None = None) -> Any:
+            if path == "/user":
+                raise RuntimeError("404 /user not available for installation token")
+            if path == "/app":
+                return {"slug": "my-bot", "id": 1}
+            if path == "/users/my-bot[bot]":
+                return {"id": 7777, "login": "my-bot[bot]"}
+            raise AssertionError(f"unexpected path {path}")
+
+        api = AsyncMock(side_effect=fake_api)
+        adapter._github_api_request = api
+
+        # process_message must observe a populated bot id. Capture it at call time.
+        observed: dict[str, Any] = {}
+        chat.process_message.side_effect = lambda *a, **kw: observed.update({"bot_id": adapter._bot_user_id})
+
+        payload = _make_install_payload(sender_id=100, installation_id=54321)
+        body = json.dumps(payload)
+        headers = {
+            "x-hub-signature-256": _sign(body),
+            "x-github-event": "issue_comment",
+            "content-type": "application/json",
+        }
+
+        result = await adapter.handle_webhook(FakeRequest(body, headers))
+
+        assert result["status"] == 200
+        assert adapter._bot_user_id == 7777
+        # Detection used the webhook's installation id for the API calls.
+        assert api.await_args_list[0].kwargs["installation_id"] == 54321
+        # process_message ran, and the bot id was already set when it did.
+        chat.process_message.assert_called_once()
+        assert observed["bot_id"] == 7777
+
+    @pytest.mark.asyncio
+    async def test_message_from_bot_filtered_as_is_me(self):
+        """A message authored by the bot itself is filtered (self-reply loop prevented)."""
+        adapter = _make_multi_tenant_adapter()
+        chat = MagicMock()
+        chat.process_message = MagicMock()
+        chat.get_state = MagicMock(return_value=MagicMock(set=AsyncMock(), get=AsyncMock(return_value=None)))
+        adapter._chat = chat
+
+        async def fake_api(method: str, path: str, body: Any = None, *, installation_id: int | None = None) -> Any:
+            if path == "/user":
+                raise RuntimeError("404")
+            if path == "/app":
+                return {"slug": "my-bot"}
+            if path == "/users/my-bot[bot]":
+                return {"id": 8888, "login": "my-bot[bot]"}
+            raise AssertionError(f"unexpected path {path}")
+
+        adapter._github_api_request = AsyncMock(side_effect=fake_api)
+
+        # Sender is the bot itself (id 8888) -> must be ignored.
+        payload = _make_install_payload(sender_id=8888, installation_id=54321)
+        body = json.dumps(payload)
+        headers = {
+            "x-hub-signature-256": _sign(body),
+            "x-github-event": "issue_comment",
+            "content-type": "application/json",
+        }
+
+        result = await adapter.handle_webhook(FakeRequest(body, headers))
+
+        assert result["status"] == 200
+        assert adapter._bot_user_id == 8888
+        # Self-message: process_message NOT called -> no self-reply loop.
+        chat.process_message.assert_not_called()
+
+    @pytest.mark.asyncio
+    async def test_pat_path_uses_users_get_authenticated(self):
+        """PAT path resolves the bot via GET /user (users.getAuthenticated), no /app fallback."""
+        adapter = _make_adapter()  # token-based (PAT) adapter
+        chat = MagicMock()
+        chat.process_message = MagicMock()
+        chat.get_state = MagicMock(return_value=MagicMock(set=AsyncMock(), get=AsyncMock(return_value=None)))
+        adapter._chat = chat
+
+        api = AsyncMock(return_value={"id": 4242, "login": "pat-bot"})
+        adapter._github_api_request = api
+
+        payload = _make_issue_comment_payload(sender_id=100)
+        body = json.dumps(payload)
+        headers = {
+            "x-hub-signature-256": _sign(body),
+            "x-github-event": "issue_comment",
+            "content-type": "application/json",
+        }
+
+        result = await adapter.handle_webhook(FakeRequest(body, headers))
+
+        assert result["status"] == 200
+        assert adapter._bot_user_id == 4242
+        # Only GET /user was used; no apps.getAuthenticated fallback.
+        called_paths = [c.args[1] for c in api.await_args_list]
+        assert called_paths == ["/user"]
+        chat.process_message.assert_called_once()
+
+    @pytest.mark.asyncio
+    async def test_second_webhook_does_not_refetch(self):
+        """A second webhook for the same installation does NOT re-fetch (cache hit)."""
+        adapter = _make_multi_tenant_adapter()
+        chat = MagicMock()
+        chat.process_message = MagicMock()
+        chat.get_state = MagicMock(return_value=MagicMock(set=AsyncMock(), get=AsyncMock(return_value=None)))
+        adapter._chat = chat
+
+        async def fake_api(method: str, path: str, body: Any = None, *, installation_id: int | None = None) -> Any:
+            if path == "/user":
+                raise RuntimeError("404")
+            if path == "/app":
+                return {"slug": "my-bot"}
+            if path == "/users/my-bot[bot]":
+                return {"id": 9999, "login": "my-bot[bot]"}
+            raise AssertionError(f"unexpected path {path}")
+
+        api = AsyncMock(side_effect=fake_api)
+        adapter._github_api_request = api
+
+        headers_for = lambda b: {  # noqa: E731
+            "x-hub-signature-256": _sign(b),
+            "x-github-event": "issue_comment",
+            "content-type": "application/json",
+        }
+
+        body1 = json.dumps(_make_install_payload(sender_id=100, installation_id=54321))
+        await adapter.handle_webhook(FakeRequest(body1, headers_for(body1)))
+        assert adapter._bot_user_id == 9999
+
+        detection_calls_after_first = len(api.await_args_list)
+
+        body2 = json.dumps(_make_install_payload(sender_id=101, installation_id=54321))
+        await adapter.handle_webhook(FakeRequest(body2, headers_for(body2)))
+
+        # No additional detection API calls on the second webhook (cache hit).
+        assert len(api.await_args_list) == detection_calls_after_first
+        assert chat.process_message.call_count == 2