fix(github): auth /app with App JWT and serialize bot-id detection

Addresses two PR #128 review findings on bot-user-ID auto-detection:

HIGH (Gemini): GET /app must use an App JWT, not an installation token.
_detect_bot_user_id's fallback calls apps.getAuthenticated (GET /app),
but _github_api_request unconditionally exchanged the App JWT for an
installation token whenever _app_credentials was set. GitHub's /app
endpoint rejects installation tokens (401/403), so bot-id detection was
broken for the GitHub-App/installation case (the primary /user path also
403s on installation tokens, so the /app fallback is what must work).
Special-case path == "/app" in the auth-selection branch to authenticate
with self._generate_app_jwt() directly; all other App-auth requests keep
the installation-token exchange (and its RuntimeError when no installation
id is resolvable). /users/{slug}[bot] is a public lookup and works fine
with the installation token.

MEDIUM (Gemini): race condition + await-in-try in _detect_bot_user_id.
Concurrent webhooks could all trigger redundant detection before the first
cached the result. Add an asyncio.Lock (_detect_lock, created in __init__)
and double-checked locking: fast-path early-return on cache hit, then
acquire the lock and re-check inside it before running the detection
sequence with each await inside its try block. Best-effort semantics
(errors logged, not raised) preserved.

Tests (tests/test_github_dispatch.py):
- TestAppEndpointAuthSelection.test_app_endpoint_uses_jwt_not_installation_token:
  fakes the HTTP layer so the real auth-selection logic runs; asserts /app
  is sent "Bearer APP_JWT" (not the installation token) while /user and
  /users/{slug}[bot] use the installation token. Fails against the broken
  code (where /app went through installation-token exchange).
- TestConcurrentDetectionFetchesOnce.test_concurrent_detect_fetches_once:
  fires 8 concurrent _detect_bot_user_id calls with a slow mocked /user;
  asserts the API is hit exactly once. Fails without the lock (8 calls).

https://claude.ai/code/session_01FyMxQn2BEAzmwKS1GZczKj

Author: claude

src/chat_sdk/adapters/github/adapter.py

@@ -120,6 +120,10 @@ def __init__(self, config: GitHubAdapterConfig | None = None) -> None:
         self._installation_id: int | None = None
         self._installation_token_cache: dict[int, tuple[str, float]] = {}
         self._token_lock = asyncio.Lock()
+        # Serializes concurrent bot-user-ID detection so concurrent webhooks
+        # don't all race to fetch the (identical) bot identity (see
+        # ``_detect_bot_user_id``).
+        self._detect_lock = asyncio.Lock()
 
         # Shared aiohttp session for connection pooling
         self._http_session: Any | None = None
@@ -224,45 +228,54 @@ async def _detect_bot_user_id(self, installation_id: int | None = None) -> None:
         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.
+        # Cache hit: already detected once, don't re-fetch per webhook. This is
+        # the fast path that avoids taking the lock once detection has succeeded.
         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)},
-            )
+        # Double-checked locking: concurrent webhooks must not all race to fetch
+        # the (identical) bot identity. Serialize detection and re-check the
+        # cache inside the lock so only the first caller hits the API.
+        async with self._detect_lock:
+            if self._bot_user_id is not None:
+                return
 
-        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")
+            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 via app slug",
+                    "GitHub bot user ID auto-detected",
                     {
                         "botUserId": self._bot_user_id,
-                        "login": login,
+                        "login": user.get("login"),
                     },
                 )
-        except Exception as error:
-            self._logger.warn("Could not auto-detect GitHub bot user ID", {"error": str(error)})
+                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 (GET /app, authenticated
+                # with the App JWT) 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 bot user ID auto-detected via app slug",
+                        {
+                            "botUserId": self._bot_user_id,
+                            "login": login,
+                        },
+                    )
+            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.
@@ -1126,16 +1139,27 @@ async def _github_api_request(
         """
         auth_token = self._auth_token
 
-        # GitHub App auth: exchange JWT for installation token
+        # GitHub App auth.
         if not auth_token and self._app_credentials:
-            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(resolved_installation_id)
+            if path == "/app":
+                # App-level endpoints (apps.getAuthenticated) REQUIRE app-JWT
+                # auth; installation tokens get 401/403 here. Authenticate with
+                # the App JWT directly instead of exchanging for an installation
+                # token. This is what makes bot-user-ID detection work in the
+                # GitHub-App/installation case (the /user path 403s on
+                # installation tokens, so this fallback must succeed).
+                auth_token = self._generate_app_jwt()
+            else:
+                # Installation-scoped endpoints: exchange the JWT for an
+                # installation token.
+                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(resolved_installation_id)
 
         headers: dict[str, str] = {
             "Accept": "application/vnd.github+json",

tests/test_github_dispatch.py

@@ -7,6 +7,7 @@
 
 from __future__ import annotations
 
+import asyncio
 import hashlib
 import hmac
 import json
@@ -466,3 +467,133 @@ async def fake_api(method: str, path: str, body: Any = None, *, installation_id:
         # 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
+
+
+# =============================================================================
+# Fake aiohttp session (exercises the real auth-selection logic in
+# _github_api_request: PAT vs App-JWT vs installation-token)
+# =============================================================================
+
+
+class _FakeResponse:
+    """Minimal aiohttp-style response usable as an async context manager."""
+
+    def __init__(self, status: int, payload: Any) -> None:
+        self.status = status
+        self._payload = payload
+
+    async def __aenter__(self) -> _FakeResponse:
+        return self
+
+    async def __aexit__(self, *exc: Any) -> None:
+        return None
+
+    async def json(self) -> Any:
+        return self._payload
+
+    async def text(self) -> str:
+        return json.dumps(self._payload)
+
+
+class _FakeSession:
+    """Routes requests by path, recording the Authorization header used.
+
+    Lets a test assert *which* credential (app JWT vs installation token) was
+    sent to each GitHub endpoint, which is the load-bearing behaviour for the
+    /app-needs-JWT fix.
+    """
+
+    def __init__(self, handler: Any) -> None:
+        self._handler = handler
+        self.closed = False
+        # path -> Authorization header value used for that path
+        self.auth_by_path: dict[str, str] = {}
+
+    def request(self, method: str, url: str, **kwargs: Any) -> _FakeResponse:
+        path = url.replace("https://api.github.com", "")
+        headers = kwargs.get("headers", {})
+        self.auth_by_path[path] = headers.get("Authorization", "")
+        status, payload = self._handler(method, path)
+        return _FakeResponse(status, payload)
+
+
+class TestAppEndpointAuthSelection:
+    """GET /app must authenticate with the App JWT, not an installation token.
+
+    GitHub's apps.getAuthenticated (GET /app) endpoint rejects installation
+    tokens (401/403). For the GitHub-App/installation case the /user path also
+    fails on installation tokens, so the /app fallback is what must work --
+    and it only works when authenticated with the App JWT directly.
+
+    This test exercises the real auth-selection branch in _github_api_request
+    (the HTTP layer is faked, not _github_api_request itself).
+    """
+
+    @pytest.mark.asyncio
+    async def test_app_endpoint_uses_jwt_not_installation_token(self):
+        adapter = _make_multi_tenant_adapter()
+        adapter._installation_id = 54321  # single resolvable installation
+
+        # Spy the credential minting so we can assert which path used which.
+        adapter._generate_app_jwt = MagicMock(return_value="APP_JWT")  # type: ignore[method-assign]
+        adapter._get_installation_token = AsyncMock(return_value="INSTALL_TOKEN")  # type: ignore[method-assign]
+
+        def handler(method: str, path: str) -> tuple[int, Any]:
+            if path == "/user":
+                # Installation token cannot use /user -> 403.
+                return 403, {"message": "Resource not accessible by integration"}
+            if path == "/app":
+                return 200, {"slug": "my-app", "id": 1}
+            if path == "/users/my-app[bot]":
+                return 200, {"id": 12345, "login": "my-app[bot]"}
+            raise AssertionError(f"unexpected path {path}")
+
+        session = _FakeSession(handler)
+        adapter._get_http_session = AsyncMock(return_value=session)  # type: ignore[method-assign]
+
+        await adapter._detect_bot_user_id(installation_id=54321)
+
+        # Detection resolved the bot id via /app -> /users/{slug}[bot].
+        assert adapter._bot_user_id == 12345
+
+        # The /app call was authenticated with the App JWT (not an install token).
+        assert session.auth_by_path["/app"] == "Bearer APP_JWT"
+        assert adapter._generate_app_jwt.called
+        # And /app was NOT sent the installation token.
+        assert session.auth_by_path["/app"] != "Bearer INSTALL_TOKEN"
+
+        # The /user attempt (which 403s) and the public /users/{slug}[bot] lookup
+        # both go through the installation-token exchange, as expected.
+        assert session.auth_by_path["/user"] == "Bearer INSTALL_TOKEN"
+        assert session.auth_by_path["/users/my-app[bot]"] == "Bearer INSTALL_TOKEN"
+
+
+class TestConcurrentDetectionFetchesOnce:
+    """Concurrent detection must fetch the bot identity only once (lock works)."""
+
+    @pytest.mark.asyncio
+    async def test_concurrent_detect_fetches_once(self):
+        adapter = _make_multi_tenant_adapter()
+
+        call_counts: dict[str, int] = {"/user": 0}
+        started = asyncio.Event()
+
+        async def fake_api(method: str, path: str, body: Any = None, *, installation_id: int | None = None) -> Any:
+            if path == "/user":
+                call_counts["/user"] += 1
+                started.set()
+                # Slow PAT-style success so concurrent callers pile up behind
+                # the first one if locking is absent.
+                await asyncio.sleep(0.05)
+                return {"id": 4242, "login": "the-bot"}
+            raise AssertionError(f"unexpected path {path}")
+
+        adapter._github_api_request = AsyncMock(side_effect=fake_api)
+
+        # Fire N concurrent detections; only the first should hit the API.
+        await asyncio.gather(*[adapter._detect_bot_user_id(installation_id=54321) for _ in range(8)])
+
+        assert adapter._bot_user_id == 4242
+        # The lock + double-checked cache means /user is fetched exactly once.
+        assert call_counts["/user"] == 1
+        assert adapter._github_api_request.await_count == 1