From ae97e60a3c0621851c66a02868426d32218ab468 Mon Sep 17 00:00:00 2001 From: patrick-chinchill Date: Thu, 18 Jun 2026 16:42:23 -0700 Subject: [PATCH 1/2] fix(adapters): api_url custom-endpoint config (Slack/Discord/GitHub/Linear) + GitHub public get_installation_id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-existing parity gaps ported faithfully from upstream chat@4.30.0. (A) api_url custom-endpoint config (upstream 4.27.0, 6b17c60) — every high-level adapter reads `config.apiUrl ?? _API_URL` env and routes it into the underlying API client. Add the field (+ env fallback, default unchanged when unset) to the four adapters that were missing it: - Slack (index.ts:617,576-577,620-621): new `api_url` (+ SLACK_API_URL). Passed as `base_url` to BOTH the async AsyncWebClient cache and the synchronous web_client escape hatch. Omitted entirely when unset (slack_sdk rejects base_url=None and keeps its built-in default). - Discord (types.ts:18-19, index.ts:142): new `api_url` (+ DISCORD_API_URL) replacing the hardcoded DISCORD_API_BASE in `_discord_fetch` — the single funnel all Discord HTTP goes through. - GitHub (index.ts:201,217): new `api_url` (+ GITHUB_API_URL) for GitHub Enterprise Server. Threaded into `_github_api_request` and the installation-token exchange URL (the two hardcoded api.github.com sites). - Linear (index.ts:221,239,249, types.ts:51): new `api_url` (+ LINEAR_API_URL) overriding the GraphQL endpoint in `_graphql_query`. (B) GitHub public get_installation_id (index.ts:458-480) — add the public `get_installation_id(thread|str)` returning the fixed id (single-tenant app), the cached repo installation (multi-tenant), or None (PAT mode); raises when multi-tenant and uninitialized. Keeps the private `_get_installation_id(owner, repo)` helper it delegates to. Tests: tests/test_adapter_api_url_config.py (31) — per-adapter custom-url used / default-unchanged / env-fallback / config-wins-over-env, plus the six get_installation_id auth-mode branches. The usage-site assertions were verified to fail against the pre-fix sources. --- src/chat_sdk/adapters/discord/adapter.py | 13 +- src/chat_sdk/adapters/discord/types.py | 5 + src/chat_sdk/adapters/github/adapter.py | 57 ++- src/chat_sdk/adapters/github/types.py | 6 + src/chat_sdk/adapters/linear/adapter.py | 15 +- src/chat_sdk/adapters/linear/types.py | 6 + src/chat_sdk/adapters/slack/adapter.py | 26 +- src/chat_sdk/adapters/slack/types.py | 8 + tests/test_adapter_api_url_config.py | 423 +++++++++++++++++++++++ 9 files changed, 553 insertions(+), 6 deletions(-) create mode 100644 tests/test_adapter_api_url_config.py diff --git a/src/chat_sdk/adapters/discord/adapter.py b/src/chat_sdk/adapters/discord/adapter.py index f01195f7..e06b3ffc 100644 --- a/src/chat_sdk/adapters/discord/adapter.py +++ b/src/chat_sdk/adapters/discord/adapter.py @@ -121,6 +121,17 @@ def __init__(self, config: DiscordAdapterConfig | None = None) -> None: "application_id is required. Set DISCORD_APPLICATION_ID or provide it in config.", ) + # Custom Discord API base URL (proxy / mock / self-host). Faithful port + # of upstream's nullish-coalescing chain ``config.apiUrl ?? + # process.env.DISCORD_API_URL ?? DISCORD_API_BASE`` (index.ts:142): + # each rung falls through only on absence (``None``), so an explicit + # value -- including an empty string -- is honored rather than silently + # re-defaulted (CLAUDE.md truthiness-trap hazard). + if config.api_url is not None: + self._api_base_url: str = config.api_url + else: + env_api_url = os.environ.get("DISCORD_API_URL") + self._api_base_url = env_api_url if env_api_url is not None else DISCORD_API_BASE self._name = "discord" self._bot_token = bot_token self._public_key = public_key.strip().lower() @@ -1494,7 +1505,7 @@ async def _discord_fetch( """ import aiohttp # lazy import (needed for FormData) - url = f"{DISCORD_API_BASE}{path}" + url = f"{self._api_base_url}{path}" headers: dict[str, str] = { "Authorization": f"Bot {self._bot_token}", } diff --git a/src/chat_sdk/adapters/discord/types.py b/src/chat_sdk/adapters/discord/types.py index e6e1a720..5fe663e3 100644 --- a/src/chat_sdk/adapters/discord/types.py +++ b/src/chat_sdk/adapters/discord/types.py @@ -26,6 +26,11 @@ class DiscordAdapterConfig: See: https://discord.com/developers/docs/getting-started """ + # Override the Discord API base URL. Defaults to the DISCORD_API_URL env + # var, then to "https://discord.com/api/v10". Mirrors upstream + # ``config.apiUrl ?? process.env.DISCORD_API_URL ?? DISCORD_API_BASE`` + # (vercel/chat adapter-discord index.ts:142). + api_url: str | None = None # Discord application ID. Defaults to DISCORD_APPLICATION_ID env var. application_id: str | None = None # Discord bot token. Defaults to DISCORD_BOT_TOKEN env var. diff --git a/src/chat_sdk/adapters/github/adapter.py b/src/chat_sdk/adapters/github/adapter.py index 75eb2921..47d50e35 100644 --- a/src/chat_sdk/adapters/github/adapter.py +++ b/src/chat_sdk/adapters/github/adapter.py @@ -57,6 +57,7 @@ RawMessage, StreamChunk, StreamOptions, + Thread, ThreadInfo, ThreadSummary, UserInfo, @@ -64,6 +65,12 @@ _parse_iso, ) +# Default GitHub REST API base URL. Overridable per-adapter via +# ``config["api_url"]`` / ``GITHUB_API_URL`` for GitHub Enterprise Server +# (mirrors upstream Octokit ``baseUrl``). Stored without a trailing slash so +# ``f"{base}{path}"`` joins cleanly with leading-slash paths. +GITHUB_API_BASE_URL = "https://api.github.com" + REVIEW_COMMENT_THREAD_PATTERN = re.compile(r"^([^/]+)/([^:]+):(\d+):rc:(\d+)$") ISSUE_THREAD_PATTERN = re.compile(r"^([^/]+)/([^:]+):issue:(\d+)$") PR_THREAD_PATTERN = re.compile(r"^([^/]+)/([^:]+):(\d+)$") @@ -114,6 +121,18 @@ def __init__(self, config: GitHubAdapterConfig | None = None) -> None: self._chat: ChatInstance | None = None self._format_converter = GitHubFormatConverter() + # Custom GitHub API base URL (e.g. GitHub Enterprise Server). Upstream + # threads ``config.apiUrl ?? process.env.GITHUB_API_URL`` into every + # Octokit ``baseUrl`` (index.ts:201). We have no Octokit -- our REST + # calls go through ``_github_api_request`` and the installation-token + # exchange -- so we normalize the override (strip a trailing slash so + # ``f"{base}{path}"`` joins cleanly) and substitute it for the hardcoded + # ``https://api.github.com`` at both sites. ``is not None`` honors an + # explicit empty string (CLAUDE.md truthiness-trap hazard). + config_api_url = config.get("api_url") + api_url_raw = config_api_url if config_api_url is not None else os.environ.get("GITHUB_API_URL") + self._api_url = api_url_raw.rstrip("/") if api_url_raw else GITHUB_API_BASE_URL + # Auth configuration self._auth_token: str | None = None self._app_credentials: dict[str, str] | None = None @@ -1098,7 +1117,7 @@ async def _get_installation_token(self, installation_id: int) -> str: return token app_jwt = self._generate_app_jwt() - url = f"https://api.github.com/app/installations/{installation_id}/access_tokens" + url = f"{self._api_url}/app/installations/{installation_id}/access_tokens" headers = { "Accept": "application/vnd.github+json", "Authorization": f"Bearer {app_jwt}", @@ -1169,7 +1188,7 @@ async def _github_api_request( if auth_token: headers["Authorization"] = f"Bearer {auth_token}" - url = f"https://api.github.com{path}" if path.startswith("/") else path + url = f"{self._api_url}{path}" if path.startswith("/") else path session = await self._get_http_session() kwargs: dict[str, Any] = {"headers": headers} @@ -1208,6 +1227,40 @@ async def _get_installation_id(self, owner: str, repo: str) -> int | None: key = f"github:install:{owner}/{repo}" return await self._chat.get_state().get(key) + async def get_installation_id(self, thread: Thread | str) -> int | None: + """Get the GitHub App installation ID associated with a thread. + + Returns the fixed installation ID in single-tenant app mode, the cached + repository installation in multi-tenant mode, or ``None`` in PAT mode. + + Faithful port of upstream ``getInstallationId`` (index.ts:458-480). + ``thread`` may be a :class:`Thread` or a raw thread-ID string. The + private :meth:`_get_installation_id` (owner/repo) is retained as the + storage-keyed helper this method delegates to in multi-tenant mode. + + Raises: + ValidationError: In multi-tenant mode when the adapter has not been + initialized via ``chat.initialize()`` (no state store to read). + """ + # Single-tenant GitHub App mode: a fixed installation was configured. + if self._installation_id is not None: + return self._installation_id + + # PAT mode (or any non-multi-tenant config): no installation concept. + if not self.is_multi_tenant: + return None + + thread_id = thread if isinstance(thread, str) else thread.id + decoded = self.decode_thread_id(thread_id) + + if not self._chat: + raise ValidationError( + "github", + "Adapter not initialized. Ensure chat.initialize() has been called first.", + ) + + return await self._get_installation_id(decoded.owner, decoded.repo) + @staticmethod async def _get_request_body(request: Any) -> str: """Extract body text from a request object.""" diff --git a/src/chat_sdk/adapters/github/types.py b/src/chat_sdk/adapters/github/types.py index f88882e4..35d83502 100644 --- a/src/chat_sdk/adapters/github/types.py +++ b/src/chat_sdk/adapters/github/types.py @@ -16,6 +16,11 @@ class GitHubAdapterBaseConfig(TypedDict, total=False): """Base configuration options shared by all auth methods. Attributes: + api_url: Override the GitHub API base URL (e.g. a GitHub Enterprise + Server endpoint like "https://github.example.com/api/v3"). Mirrors + upstream ``config.apiUrl`` → Octokit ``baseUrl`` (index.ts:201,217). + Defaults to the GITHUB_API_URL env var, then to + "https://api.github.com". bot_user_id: Bot's GitHub user ID (numeric). Used for self-message detection. If not provided, will be fetched on first API call. logger: Logger instance for error reporting. Defaults to ConsoleLogger. @@ -27,6 +32,7 @@ class GitHubAdapterBaseConfig(TypedDict, total=False): Defaults to GITHUB_WEBHOOK_SECRET env var. """ + api_url: str bot_user_id: int logger: Logger user_name: str diff --git a/src/chat_sdk/adapters/linear/adapter.py b/src/chat_sdk/adapters/linear/adapter.py index a21f7965..7e5e8f73 100644 --- a/src/chat_sdk/adapters/linear/adapter.py +++ b/src/chat_sdk/adapters/linear/adapter.py @@ -119,6 +119,19 @@ def __init__(self, config: LinearAdapterConfig | None = None) -> None: ) self._name = "linear" + # Custom Linear GraphQL endpoint (proxy / mock / self-host). Faithful + # port of upstream ``config.apiUrl ?? process.env.LINEAR_API_URL`` + # (index.ts:239) which overrides every ``LinearClient``'s ``apiUrl``. + # We have no LinearClient -- ``_graphql_query`` POSTs raw GraphQL -- so + # the override substitutes for the module-level ``LINEAR_API_URL`` + # default. ``is not None`` honors an explicit empty string (CLAUDE.md + # truthiness-trap hazard). + config_api_url = getattr(config, "api_url", None) + if config_api_url is not None: + self._api_url: str = config_api_url + else: + env_api_url = os.environ.get("LINEAR_API_URL") + self._api_url = env_api_url if env_api_url is not None else LINEAR_API_URL self._webhook_secret = webhook_secret self._logger: Logger = getattr(config, "logger", None) or ConsoleLogger("info", prefix="linear") self._user_name = getattr(config, "user_name", None) or os.environ.get("LINEAR_BOT_USERNAME", "linear-bot") @@ -1168,7 +1181,7 @@ async def _graphql_query( session = await self._get_http_session() async with session.post( - LINEAR_API_URL, + self._api_url, headers=headers, json=payload, ) as response: diff --git a/src/chat_sdk/adapters/linear/types.py b/src/chat_sdk/adapters/linear/types.py index 76837f33..68fb6169 100644 --- a/src/chat_sdk/adapters/linear/types.py +++ b/src/chat_sdk/adapters/linear/types.py @@ -20,6 +20,12 @@ class LinearAdapterBaseConfig: """Base configuration options shared by all auth methods.""" + # Override the Linear GraphQL API base URL. Defaults to the LINEAR_API_URL + # env var, then to "https://api.linear.app/graphql". Mirrors upstream + # ``config.apiUrl ?? process.env.LINEAR_API_URL`` → ``LinearClient.apiUrl`` + # (vercel/chat adapter-linear index.ts:239, types.ts:51). Useful for + # proxies, mocks, or self-hosted GraphQL gateways. + api_url: str | None = None # Logger instance for error reporting. Defaults to ConsoleLogger. logger: Logger | None = None # Bot display name for @-mention detection. diff --git a/src/chat_sdk/adapters/slack/adapter.py b/src/chat_sdk/adapters/slack/adapter.py index d1841675..33b7fd01 100644 --- a/src/chat_sdk/adapters/slack/adapter.py +++ b/src/chat_sdk/adapters/slack/adapter.py @@ -540,6 +540,17 @@ def __init__(self, config: SlackAdapterConfig | None = None) -> None: if encryption_key_raw: self._encryption_key = decode_key(encryption_key_raw) + # Custom Slack Web API base URL (e.g. proxy, mock, Enterprise routing). + # ``config.apiUrl ?? process.env.SLACK_API_URL`` upstream (index.ts:617). + # When ``None`` we omit ``base_url`` from the client constructors + # entirely, so slack_sdk keeps its built-in ``https://slack.com/api/`` + # default (it rejects ``base_url=None``). Threaded into BOTH the async + # ``AsyncWebClient`` cache and the synchronous ``web_client`` escape + # hatch, mirroring upstream's default + per-token ``slackApiUrl``. + self._slack_api_url: str | None = ( + config.api_url if config.api_url is not None else os.environ.get("SLACK_API_URL") + ) + # ------------------------------------------------------------------ # Properties (Adapter protocol) # ------------------------------------------------------------------ @@ -852,7 +863,13 @@ def _get_client(self, token: str | None = None) -> Any: from slack_sdk.web.async_client import AsyncWebClient - client = AsyncWebClient(token=resolved_token) + # Only pass ``base_url`` when an override is configured — slack_sdk + # rejects ``base_url=None`` (it requires a string), so mirroring + # upstream's conditional spread keeps the built-in default otherwise. + client_kwargs: dict[str, Any] = {"token": resolved_token} + if self._slack_api_url is not None: + client_kwargs["base_url"] = self._slack_api_url + client = AsyncWebClient(**client_kwargs) self._client_cache[resolved_token] = client if len(self._client_cache) > self._client_cache_max: # Evict oldest (LRU). We intentionally do NOT close the evicted @@ -895,7 +912,12 @@ def _get_web_client_for_token(self, token: str) -> Any: if client is None: from slack_sdk import WebClient - client = WebClient(token=token) + # Same conditional-``base_url`` rule as ``_get_client`` — pass the + # override only when configured so slack_sdk keeps its default. + web_client_kwargs: dict[str, Any] = {"token": token} + if self._slack_api_url is not None: + web_client_kwargs["base_url"] = self._slack_api_url + client = WebClient(**web_client_kwargs) self._web_client_cache[token] = client return client diff --git a/src/chat_sdk/adapters/slack/types.py b/src/chat_sdk/adapters/slack/types.py index d8fb5306..0721f261 100644 --- a/src/chat_sdk/adapters/slack/types.py +++ b/src/chat_sdk/adapters/slack/types.py @@ -41,6 +41,14 @@ class SlackAdapterConfig: """Configuration for the Slack adapter.""" + # Override the Slack Web API base URL (passed as ``base_url`` to every + # ``slack_sdk`` client the adapter builds — the default client, the + # per-token async cache, and the synchronous ``web_client`` escape hatch). + # Defaults to the ``SLACK_API_URL`` env var, then to slack_sdk's built-in + # ``https://slack.com/api/``. Mirrors upstream ``config.apiUrl`` → + # ``slackApiUrl`` (vercel/chat 6b17c60). Useful for proxies, Slack-API + # mocks in tests, or Enterprise-routed deployments. + api_url: str | None = None # App-level token (xapp-...). Required when ``mode == "socket"``. app_token: str | None = None # Bot token (xoxb-...). Required for single-workspace mode. Omit for multi-workspace. diff --git a/tests/test_adapter_api_url_config.py b/tests/test_adapter_api_url_config.py new file mode 100644 index 00000000..2ef0ac38 --- /dev/null +++ b/tests/test_adapter_api_url_config.py @@ -0,0 +1,423 @@ +"""Custom ``api_url`` config + GitHub public ``get_installation_id``. + +Pre-existing parity gaps ported from upstream chat@4.30.0: + +* Every high-level adapter (Slack/Discord/GitHub/Linear) gained an ``apiUrl`` + config (plus a ``*_API_URL`` env fallback) in upstream 4.27.0 (6b17c60), + routing custom base URLs into the underlying API clients (proxies, mocks, + Enterprise/self-host endpoints). These tests assert the override is actually + used and that the default is unchanged when unset. +* GitHub gained a public ``getInstallationId(thread|string)`` (index.ts:458-480) + returning the fixed / cached / ``None`` installation per auth mode. +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from chat_sdk.adapters.discord.adapter import DISCORD_API_BASE, DiscordAdapter +from chat_sdk.adapters.discord.types import DiscordAdapterConfig +from chat_sdk.adapters.github.adapter import GITHUB_API_BASE_URL, GitHubAdapter +from chat_sdk.adapters.linear.adapter import LINEAR_API_URL, LinearAdapter +from chat_sdk.adapters.linear.types import LinearAdapterAPIKeyConfig +from chat_sdk.adapters.slack.adapter import SlackAdapter +from chat_sdk.adapters.slack.types import SlackAdapterConfig +from chat_sdk.logger import ConsoleLogger +from chat_sdk.shared.errors import ValidationError + +pytest.importorskip("slack_sdk") + +TEST_PUBLIC_KEY = "a" * 64 + +# Env vars these adapters read for their api_url fallback. Cleared before every +# test so a host-set value can't shadow assertions about the built-in default. +_API_URL_ENV_VARS = ("SLACK_API_URL", "DISCORD_API_URL", "GITHUB_API_URL", "LINEAR_API_URL") + + +@pytest.fixture(autouse=True) +def _isolate_api_url_env(monkeypatch: pytest.MonkeyPatch) -> None: + for name in _API_URL_ENV_VARS: + monkeypatch.delenv(name, raising=False) + + +# --------------------------------------------------------------------------- +# Fake aiohttp session that records the URL of the first request it sees. +# --------------------------------------------------------------------------- + + +class _FakeResponse: + def __init__(self, payload: dict[str, Any]) -> None: + self._payload = payload + self.status = 200 + self.ok = True + + async def __aenter__(self) -> _FakeResponse: + return self + + async def __aexit__(self, *exc: object) -> None: + return None + + async def json(self) -> dict[str, Any]: + return self._payload + + async def text(self) -> str: + return "" + + +class _RecordingSession: + """Records the URL passed to ``post`` / ``request``.""" + + def __init__(self, payload: dict[str, Any] | None = None) -> None: + self.urls: list[str] = [] + self._payload = payload or {} + + def post(self, url: str, *_args: object, **_kwargs: object) -> _FakeResponse: + self.urls.append(url) + return _FakeResponse(self._payload) + + def request(self, _method: str, url: str, *_args: object, **_kwargs: object) -> _FakeResponse: + self.urls.append(url) + return _FakeResponse(self._payload) + + +# =========================================================================== +# Slack — base_url threaded into both client constructions +# =========================================================================== + + +class _CapturingClient: + """Records the kwargs the adapter passes to AsyncWebClient / WebClient. + + Independent of the real slack_sdk so the test is robust to sibling test + files that install a fake ``slack_sdk`` stub into ``sys.modules``. + """ + + def __init__(self, **kwargs: Any) -> None: + self.kwargs = kwargs + + +def _patch_slack_clients(monkeypatch: pytest.MonkeyPatch) -> None: + """Patch both client classes on whatever slack_sdk modules are installed.""" + import slack_sdk + import slack_sdk.web.async_client as async_mod + + monkeypatch.setattr(async_mod, "AsyncWebClient", _CapturingClient, raising=False) + monkeypatch.setattr(slack_sdk, "WebClient", _CapturingClient, raising=False) + + +class TestSlackApiUrl: + def _make(self, **overrides: Any) -> SlackAdapter: + config = SlackAdapterConfig( + signing_secret=overrides.pop("signing_secret", "test-signing-secret"), + bot_token=overrides.pop("bot_token", "xoxb-default-token"), + **overrides, + ) + return SlackAdapter(config) + + def test_custom_api_url_passed_to_async_web_client(self, monkeypatch: pytest.MonkeyPatch) -> None: + _patch_slack_clients(monkeypatch) + adapter = self._make(api_url="https://slack.proxy.example/api/") + client = adapter._get_client("xoxb-tok") + assert client.kwargs == {"token": "xoxb-tok", "base_url": "https://slack.proxy.example/api/"} + + def test_custom_api_url_passed_to_sync_web_client(self, monkeypatch: pytest.MonkeyPatch) -> None: + _patch_slack_clients(monkeypatch) + adapter = self._make(api_url="https://slack.proxy.example/api/") + client = adapter._get_web_client_for_token("xoxb-tok") + assert client.kwargs == {"token": "xoxb-tok", "base_url": "https://slack.proxy.example/api/"} + + def test_no_base_url_passed_when_unset(self, monkeypatch: pytest.MonkeyPatch) -> None: + # When no override is configured we must NOT pass base_url at all, so + # slack_sdk keeps its built-in default (and never sees base_url=None, + # which it rejects). + _patch_slack_clients(monkeypatch) + adapter = self._make() + assert adapter._slack_api_url is None + async_client = adapter._get_client("xoxb-tok") + sync_client = adapter._get_web_client_for_token("xoxb-tok") + assert "base_url" not in async_client.kwargs + assert "base_url" not in sync_client.kwargs + + def test_env_fallback_used_when_config_unset(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("SLACK_API_URL", "https://slack.env.example/api/") + _patch_slack_clients(monkeypatch) + adapter = self._make() + assert adapter._slack_api_url == "https://slack.env.example/api/" + client = adapter._get_client("xoxb-tok") + assert client.kwargs["base_url"] == "https://slack.env.example/api/" + + def test_config_wins_over_env(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("SLACK_API_URL", "https://slack.env.example/api/") + _patch_slack_clients(monkeypatch) + adapter = self._make(api_url="https://slack.config.example/api/") + client = adapter._get_client("xoxb-tok") + assert client.kwargs["base_url"] == "https://slack.config.example/api/" + + def test_real_slack_sdk_normalizes_base_url(self) -> None: + # End-to-end against the genuine slack_sdk client (skipped if a sibling + # test stubbed it): the override must reach the real ``base_url``. + import slack_sdk + + real_web_client = getattr(slack_sdk, "WebClient", None) + if real_web_client is None or not getattr(real_web_client, "__module__", "").startswith("slack_sdk"): + pytest.skip("slack_sdk is stubbed by a sibling test in this run") + adapter = self._make(api_url="https://slack.proxy.example/api/") + client = adapter._get_web_client_for_token("xoxb-tok") + assert client.base_url == "https://slack.proxy.example/api/" + + +# =========================================================================== +# Discord — api_url threaded into _discord_fetch +# =========================================================================== + + +class TestDiscordApiUrl: + def _make(self, **overrides: Any) -> DiscordAdapter: + config = DiscordAdapterConfig( + bot_token=overrides.pop("bot_token", "test-token"), + public_key=overrides.pop("public_key", TEST_PUBLIC_KEY), + application_id=overrides.pop("application_id", "test-app-id"), + logger=ConsoleLogger("error"), + **overrides, + ) + return DiscordAdapter(config) + + def test_default_api_base_when_unset(self) -> None: + adapter = self._make() + assert adapter._api_base_url == DISCORD_API_BASE + + def test_custom_api_url_stored(self) -> None: + adapter = self._make(api_url="https://discord.proxy.example/api/v10") + assert adapter._api_base_url == "https://discord.proxy.example/api/v10" + + def test_env_fallback(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("DISCORD_API_URL", "https://discord.env.example/api/v10") + adapter = self._make() + assert adapter._api_base_url == "https://discord.env.example/api/v10" + + def test_config_wins_over_env(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("DISCORD_API_URL", "https://discord.env.example/api/v10") + adapter = self._make(api_url="https://discord.config.example/api/v10") + assert adapter._api_base_url == "https://discord.config.example/api/v10" + + async def test_custom_api_url_used_in_fetch(self) -> None: + adapter = self._make(api_url="https://discord.proxy.example/api/v10") + session = _RecordingSession({"id": "1"}) + adapter._get_http_session = AsyncMock(return_value=session) # type: ignore[method-assign] + await adapter._discord_fetch("/channels/123/messages", "POST", body={"content": "hi"}) + assert session.urls == ["https://discord.proxy.example/api/v10/channels/123/messages"] + + async def test_default_api_url_used_in_fetch(self) -> None: + adapter = self._make() + session = _RecordingSession({"id": "1"}) + adapter._get_http_session = AsyncMock(return_value=session) # type: ignore[method-assign] + await adapter._discord_fetch("/channels/123/messages", "POST", body={"content": "hi"}) + assert session.urls == [f"{DISCORD_API_BASE}/channels/123/messages"] + + +# =========================================================================== +# GitHub — api_url threaded into api_request + installation-token URL +# =========================================================================== + + +class TestGitHubApiUrl: + def _make(self, **overrides: Any) -> GitHubAdapter: + config: dict[str, Any] = { + "webhook_secret": "test-webhook-secret", + "token": "ghp_testtoken", + "logger": ConsoleLogger("error"), + } + config.update(overrides) + return GitHubAdapter(config) + + def test_default_api_url_when_unset(self) -> None: + adapter = self._make() + assert adapter._api_url == GITHUB_API_BASE_URL + + def test_custom_api_url_strips_trailing_slash(self) -> None: + # GitHub Enterprise endpoints often carry a trailing slash; strip it so + # f"{base}{path}" joins cleanly with leading-slash paths. + adapter = self._make(api_url="https://github.example.com/api/v3/") + assert adapter._api_url == "https://github.example.com/api/v3" + + def test_env_fallback(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("GITHUB_API_URL", "https://github.env.example/api/v3") + adapter = self._make() + assert adapter._api_url == "https://github.env.example/api/v3" + + def test_config_wins_over_env(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("GITHUB_API_URL", "https://github.env.example/api/v3") + adapter = self._make(api_url="https://github.config.example/api/v3") + assert adapter._api_url == "https://github.config.example/api/v3" + + async def test_custom_api_url_used_in_request(self) -> None: + adapter = self._make(api_url="https://github.example.com/api/v3") + session = _RecordingSession({"id": 1}) + adapter._get_http_session = AsyncMock(return_value=session) # type: ignore[method-assign] + await adapter._github_api_request("GET", "/repos/acme/app/pulls/1") + assert session.urls == ["https://github.example.com/api/v3/repos/acme/app/pulls/1"] + + async def test_default_api_url_used_in_request(self) -> None: + adapter = self._make() + session = _RecordingSession({"id": 1}) + adapter._get_http_session = AsyncMock(return_value=session) # type: ignore[method-assign] + await adapter._github_api_request("GET", "/repos/acme/app/pulls/1") + assert session.urls == ["https://api.github.com/repos/acme/app/pulls/1"] + + async def test_custom_api_url_used_in_installation_token_exchange(self) -> None: + adapter = GitHubAdapter( + { + "app_id": "12345", + "private_key": "-----BEGIN RSA PRIVATE KEY-----\nfake\n-----END RSA PRIVATE KEY-----", + "installation_id": 77, + "webhook_secret": "secret", + "logger": ConsoleLogger("error"), + "api_url": "https://github.example.com/api/v3", + } + ) + session = _RecordingSession({"token": "ghs_x", "expires_at": ""}) + adapter._get_http_session = AsyncMock(return_value=session) # type: ignore[method-assign] + adapter._generate_app_jwt = MagicMock(return_value="jwt") # type: ignore[method-assign] + await adapter._get_installation_token(77) + assert session.urls == ["https://github.example.com/api/v3/app/installations/77/access_tokens"] + + +# =========================================================================== +# Linear — api_url threaded into _graphql_query +# =========================================================================== + + +class TestLinearApiUrl: + def _make(self, **overrides: Any) -> LinearAdapter: + config = LinearAdapterAPIKeyConfig( + api_key=overrides.pop("api_key", "lin_api_test"), + webhook_secret=overrides.pop("webhook_secret", "test-secret"), + logger=ConsoleLogger("error"), + **overrides, + ) + return LinearAdapter(config) + + def test_default_api_url_when_unset(self) -> None: + adapter = self._make() + assert adapter._api_url == LINEAR_API_URL + + def test_custom_api_url_stored(self) -> None: + adapter = self._make(api_url="https://linear.proxy.example/graphql") + assert adapter._api_url == "https://linear.proxy.example/graphql" + + def test_env_fallback(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("LINEAR_API_URL", "https://linear.env.example/graphql") + adapter = self._make() + assert adapter._api_url == "https://linear.env.example/graphql" + + def test_config_wins_over_env(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("LINEAR_API_URL", "https://linear.env.example/graphql") + adapter = self._make(api_url="https://linear.config.example/graphql") + assert adapter._api_url == "https://linear.config.example/graphql" + + async def test_custom_api_url_used_in_graphql_query(self) -> None: + adapter = self._make(api_url="https://linear.proxy.example/graphql") + session = _RecordingSession({"data": {}}) + adapter._get_http_session = AsyncMock(return_value=session) # type: ignore[method-assign] + await adapter._graphql_query("query { viewer { id } }") + assert session.urls == ["https://linear.proxy.example/graphql"] + + async def test_default_api_url_used_in_graphql_query(self) -> None: + adapter = self._make() + session = _RecordingSession({"data": {}}) + adapter._get_http_session = AsyncMock(return_value=session) # type: ignore[method-assign] + await adapter._graphql_query("query { viewer { id } }") + assert session.urls == [LINEAR_API_URL] + + +# =========================================================================== +# GitHub public get_installation_id (gap B) +# =========================================================================== + + +def _make_mock_chat_with_store(store: dict[str, Any]) -> MagicMock: + state = MagicMock() + state.get = AsyncMock(side_effect=lambda k: store.get(k)) + state.set = AsyncMock(side_effect=lambda k, v, *a, **kw: store.__setitem__(k, v)) + chat = MagicMock() + chat.get_state = MagicMock(return_value=state) + return chat + + +class TestGitHubGetInstallationId: + _APP_KEY = "-----BEGIN RSA PRIVATE KEY-----\nfake\n-----END RSA PRIVATE KEY-----" + + async def test_pat_mode_returns_none(self) -> None: + adapter = GitHubAdapter({"webhook_secret": "s", "token": "ghp_x", "logger": ConsoleLogger("error")}) + assert await adapter.get_installation_id("github:acme/app:42") is None + + async def test_single_tenant_returns_fixed_id(self) -> None: + adapter = GitHubAdapter( + { + "app_id": "12345", + "private_key": self._APP_KEY, + "installation_id": 99, + "webhook_secret": "s", + "logger": ConsoleLogger("error"), + } + ) + # Fixed id is returned regardless of the thread (even a garbage one). + assert await adapter.get_installation_id("not-a-real-thread") == 99 + + async def test_multi_tenant_returns_cached_id_for_thread(self) -> None: + store = {"github:install:acme/app": 4242} + chat = _make_mock_chat_with_store(store) + adapter = GitHubAdapter( + { + "app_id": "12345", + "private_key": self._APP_KEY, + "webhook_secret": "s", + "logger": ConsoleLogger("error"), + } + ) + await adapter.initialize(chat) + assert await adapter.get_installation_id("github:acme/app:42") == 4242 + + async def test_multi_tenant_accepts_thread_object(self) -> None: + store = {"github:install:acme/app": 4242} + chat = _make_mock_chat_with_store(store) + adapter = GitHubAdapter( + { + "app_id": "12345", + "private_key": self._APP_KEY, + "webhook_secret": "s", + "logger": ConsoleLogger("error"), + } + ) + await adapter.initialize(chat) + thread = MagicMock() + thread.id = "github:acme/app:42" + assert await adapter.get_installation_id(thread) == 4242 + + async def test_multi_tenant_uncached_returns_none(self) -> None: + chat = _make_mock_chat_with_store({}) + adapter = GitHubAdapter( + { + "app_id": "12345", + "private_key": self._APP_KEY, + "webhook_secret": "s", + "logger": ConsoleLogger("error"), + } + ) + await adapter.initialize(chat) + assert await adapter.get_installation_id("github:acme/app:42") is None + + async def test_multi_tenant_without_init_raises(self) -> None: + adapter = GitHubAdapter( + { + "app_id": "12345", + "private_key": self._APP_KEY, + "webhook_secret": "s", + "logger": ConsoleLogger("error"), + } + ) + with pytest.raises(ValidationError): + await adapter.get_installation_id("github:acme/app:42") From 4dc7d386d022d48d3e7b2241609dfefbca05916f Mon Sep 17 00:00:00 2001 From: patrick-chinchill Date: Thu, 18 Jun 2026 16:58:21 -0700 Subject: [PATCH 2/2] fix(adapters): run api_url tests in CI + empty-string falls back to default endpoint The api_url config test module guarded every test behind a module-level pytest.importorskip("slack_sdk"). Since slack_sdk lives only in the optional slack/all extras (not the dev group CI installs), the whole file collected as a single skip -- silently disabling the Discord/GitHub/Linear api_url tests and the get_installation_id regression tests in CI (falsely green). Stub slack_sdk into sys.modules (mirroring tests/test_slack_client_cache.py) so the module imports without the real package and the Slack tests run too. The file now reports 34 passed, 1 skipped (the lone skip is the genuine-sdk end-to-end test, intentionally skipped when stubbed) instead of 1 skipped. Empty-string api_url now falls back to the default endpoint, matching upstream's truthy spread ...(this.apiUrl ? {...} : {}): - Slack: empty apiUrl/env resolves to None, so base_url is omitted and slack_sdk keeps https://slack.com/api/ (never passes base_url=""). - Linear: empty resolves to LINEAR_API_URL instead of POSTing to an empty URL. - GitHub: empty resolves to https://api.github.com; fixed the misleading inline comment that claimed the is-not-None check honored empty string. - Discord: empty resolves to DISCORD_API_BASE instead of a relative request URL. Added a per-adapter empty-string regression test (Slack/Discord/GitHub/Linear). --- src/chat_sdk/adapters/discord/adapter.py | 20 ++--- src/chat_sdk/adapters/github/adapter.py | 17 ++-- src/chat_sdk/adapters/linear/adapter.py | 14 ++- src/chat_sdk/adapters/slack/adapter.py | 34 +++---- tests/test_adapter_api_url_config.py | 109 ++++++++++++++++++++--- 5 files changed, 141 insertions(+), 53 deletions(-) diff --git a/src/chat_sdk/adapters/discord/adapter.py b/src/chat_sdk/adapters/discord/adapter.py index e06b3ffc..27e4595d 100644 --- a/src/chat_sdk/adapters/discord/adapter.py +++ b/src/chat_sdk/adapters/discord/adapter.py @@ -121,17 +121,15 @@ def __init__(self, config: DiscordAdapterConfig | None = None) -> None: "application_id is required. Set DISCORD_APPLICATION_ID or provide it in config.", ) - # Custom Discord API base URL (proxy / mock / self-host). Faithful port - # of upstream's nullish-coalescing chain ``config.apiUrl ?? - # process.env.DISCORD_API_URL ?? DISCORD_API_BASE`` (index.ts:142): - # each rung falls through only on absence (``None``), so an explicit - # value -- including an empty string -- is honored rather than silently - # re-defaulted (CLAUDE.md truthiness-trap hazard). - if config.api_url is not None: - self._api_base_url: str = config.api_url - else: - env_api_url = os.environ.get("DISCORD_API_URL") - self._api_base_url = env_api_url if env_api_url is not None else DISCORD_API_BASE + # Custom Discord API base URL (proxy / mock / self-host). Port of + # upstream's coalescing chain ``config.apiUrl ?? process.env.DISCORD_API_URL + # ?? DISCORD_API_BASE`` (index.ts:142). Unlike Slack/GitHub/Linear -- + # which feed clients via a truthy spread -- ``_discord_fetch`` joins + # ``f"{base}{path}"`` directly, so an empty ``apiUrl`` would yield a + # broken relative URL. We use a truthy fallback so an empty string (or + # env) resolves to the ``DISCORD_API_BASE`` default, matching the + # empty-string-is-default contract shared by the other adapters. + self._api_base_url: str = config.api_url or os.environ.get("DISCORD_API_URL") or DISCORD_API_BASE self._name = "discord" self._bot_token = bot_token self._public_key = public_key.strip().lower() diff --git a/src/chat_sdk/adapters/github/adapter.py b/src/chat_sdk/adapters/github/adapter.py index 47d50e35..bf7630b6 100644 --- a/src/chat_sdk/adapters/github/adapter.py +++ b/src/chat_sdk/adapters/github/adapter.py @@ -123,14 +123,15 @@ def __init__(self, config: GitHubAdapterConfig | None = None) -> None: # Custom GitHub API base URL (e.g. GitHub Enterprise Server). Upstream # threads ``config.apiUrl ?? process.env.GITHUB_API_URL`` into every - # Octokit ``baseUrl`` (index.ts:201). We have no Octokit -- our REST - # calls go through ``_github_api_request`` and the installation-token - # exchange -- so we normalize the override (strip a trailing slash so - # ``f"{base}{path}"`` joins cleanly) and substitute it for the hardcoded - # ``https://api.github.com`` at both sites. ``is not None`` honors an - # explicit empty string (CLAUDE.md truthiness-trap hazard). - config_api_url = config.get("api_url") - api_url_raw = config_api_url if config_api_url is not None else os.environ.get("GITHUB_API_URL") + # Octokit ``baseUrl`` via the truthy spread ``...(this.apiUrl ? { baseUrl } + # : {})`` (index.ts:201/217/...), so an empty string falls back to the + # default. We have no Octokit -- our REST calls go through + # ``_github_api_request`` and the installation-token exchange -- so we + # normalize the override (strip a trailing slash so ``f"{base}{path}"`` + # joins cleanly) and substitute it for the hardcoded + # ``https://api.github.com`` at both sites. The truthy fallback means an + # empty ``apiUrl`` (or env) uses the default endpoint. + api_url_raw = config.get("api_url") or os.environ.get("GITHUB_API_URL") self._api_url = api_url_raw.rstrip("/") if api_url_raw else GITHUB_API_BASE_URL # Auth configuration diff --git a/src/chat_sdk/adapters/linear/adapter.py b/src/chat_sdk/adapters/linear/adapter.py index 7e5e8f73..532ffce2 100644 --- a/src/chat_sdk/adapters/linear/adapter.py +++ b/src/chat_sdk/adapters/linear/adapter.py @@ -121,17 +121,15 @@ def __init__(self, config: LinearAdapterConfig | None = None) -> None: self._name = "linear" # Custom Linear GraphQL endpoint (proxy / mock / self-host). Faithful # port of upstream ``config.apiUrl ?? process.env.LINEAR_API_URL`` - # (index.ts:239) which overrides every ``LinearClient``'s ``apiUrl``. + # (index.ts:239), consumed via the truthy spread + # ``...(this.apiUrl ? { apiUrl } : {})`` at every ``LinearClient`` + # construction — so an empty string falls back to the default endpoint. # We have no LinearClient -- ``_graphql_query`` POSTs raw GraphQL -- so # the override substitutes for the module-level ``LINEAR_API_URL`` - # default. ``is not None`` honors an explicit empty string (CLAUDE.md - # truthiness-trap hazard). + # default. The truthy check means an empty ``apiUrl`` (or env) uses the + # default rather than POSTing to an empty/relative URL. config_api_url = getattr(config, "api_url", None) - if config_api_url is not None: - self._api_url: str = config_api_url - else: - env_api_url = os.environ.get("LINEAR_API_URL") - self._api_url = env_api_url if env_api_url is not None else LINEAR_API_URL + self._api_url: str = config_api_url or os.environ.get("LINEAR_API_URL") or LINEAR_API_URL self._webhook_secret = webhook_secret self._logger: Logger = getattr(config, "logger", None) or ConsoleLogger("info", prefix="linear") self._user_name = getattr(config, "user_name", None) or os.environ.get("LINEAR_BOT_USERNAME", "linear-bot") diff --git a/src/chat_sdk/adapters/slack/adapter.py b/src/chat_sdk/adapters/slack/adapter.py index 33b7fd01..50d9ba0f 100644 --- a/src/chat_sdk/adapters/slack/adapter.py +++ b/src/chat_sdk/adapters/slack/adapter.py @@ -541,15 +541,16 @@ def __init__(self, config: SlackAdapterConfig | None = None) -> None: self._encryption_key = decode_key(encryption_key_raw) # Custom Slack Web API base URL (e.g. proxy, mock, Enterprise routing). - # ``config.apiUrl ?? process.env.SLACK_API_URL`` upstream (index.ts:617). - # When ``None`` we omit ``base_url`` from the client constructors - # entirely, so slack_sdk keeps its built-in ``https://slack.com/api/`` - # default (it rejects ``base_url=None``). Threaded into BOTH the async - # ``AsyncWebClient`` cache and the synchronous ``web_client`` escape - # hatch, mirroring upstream's default + per-token ``slackApiUrl``. - self._slack_api_url: str | None = ( - config.api_url if config.api_url is not None else os.environ.get("SLACK_API_URL") - ) + # ``config.apiUrl ?? process.env.SLACK_API_URL`` upstream (index.ts:617), + # consumed via the truthy spread ``...(this.slackApiUrl ? {...} : {})`` + # at every client construction — so an empty string falls back to the + # built-in default. We mirror that here: an empty ``apiUrl`` (or env) + # resolves to ``None``. When ``None`` we omit ``base_url`` from the + # client constructors entirely, so slack_sdk keeps its built-in + # ``https://slack.com/api/`` default (it rejects ``base_url=None``). + # Threaded into BOTH the async ``AsyncWebClient`` cache and the + # synchronous ``web_client`` escape hatch. + self._slack_api_url: str | None = (config.api_url or os.environ.get("SLACK_API_URL")) or None # ------------------------------------------------------------------ # Properties (Adapter protocol) @@ -863,11 +864,12 @@ def _get_client(self, token: str | None = None) -> Any: from slack_sdk.web.async_client import AsyncWebClient - # Only pass ``base_url`` when an override is configured — slack_sdk - # rejects ``base_url=None`` (it requires a string), so mirroring - # upstream's conditional spread keeps the built-in default otherwise. + # Only pass ``base_url`` when a truthy override is configured — slack_sdk + # rejects ``base_url=None`` (it requires a string), and an empty string + # must fall back to the built-in default, so mirroring upstream's truthy + # spread keeps the default otherwise. client_kwargs: dict[str, Any] = {"token": resolved_token} - if self._slack_api_url is not None: + if self._slack_api_url: client_kwargs["base_url"] = self._slack_api_url client = AsyncWebClient(**client_kwargs) self._client_cache[resolved_token] = client @@ -912,10 +914,10 @@ def _get_web_client_for_token(self, token: str) -> Any: if client is None: from slack_sdk import WebClient - # Same conditional-``base_url`` rule as ``_get_client`` — pass the - # override only when configured so slack_sdk keeps its default. + # Same truthy ``base_url`` rule as ``_get_client`` — pass the + # override only when truthy so slack_sdk keeps its default. web_client_kwargs: dict[str, Any] = {"token": token} - if self._slack_api_url is not None: + if self._slack_api_url: web_client_kwargs["base_url"] = self._slack_api_url client = WebClient(**web_client_kwargs) self._web_client_cache[token] = client diff --git a/tests/test_adapter_api_url_config.py b/tests/test_adapter_api_url_config.py index 2ef0ac38..841250d7 100644 --- a/tests/test_adapter_api_url_config.py +++ b/tests/test_adapter_api_url_config.py @@ -13,22 +13,64 @@ from __future__ import annotations +import sys +from types import ModuleType from typing import Any from unittest.mock import AsyncMock, MagicMock import pytest -from chat_sdk.adapters.discord.adapter import DISCORD_API_BASE, DiscordAdapter -from chat_sdk.adapters.discord.types import DiscordAdapterConfig -from chat_sdk.adapters.github.adapter import GITHUB_API_BASE_URL, GitHubAdapter -from chat_sdk.adapters.linear.adapter import LINEAR_API_URL, LinearAdapter -from chat_sdk.adapters.linear.types import LinearAdapterAPIKeyConfig -from chat_sdk.adapters.slack.adapter import SlackAdapter -from chat_sdk.adapters.slack.types import SlackAdapterConfig -from chat_sdk.logger import ConsoleLogger -from chat_sdk.shared.errors import ValidationError +# --------------------------------------------------------------------------- +# Stub slack_sdk so the Slack api_url tests run without the real dependency +# installed. ``slack_sdk`` lives only in the optional ``slack``/``all`` extras +# (not the dev group CI installs), so a module-level ``importorskip`` here would +# collect-as-skipped the ENTIRE file -- silently disabling the Discord/GitHub/ +# Linear api_url tests and the get_installation_id regression tests in CI. We +# install a minimal fake (mirroring ``tests/test_slack_client_cache.py``) +# *before* importing ``SlackAdapter`` so its deferred ``from slack_sdk ...`` +# imports resolve to the stub. The Slack tests then patch these classes onto the +# stub via ``_patch_slack_clients``. +# --------------------------------------------------------------------------- + +_fake_slack_sdk = ModuleType("slack_sdk") +_fake_slack_sdk_web = ModuleType("slack_sdk.web") +_fake_slack_sdk_web_async = ModuleType("slack_sdk.web.async_client") + + +class _FakeAsyncWebClient: + """Minimal stand-in for slack_sdk.web.async_client.AsyncWebClient.""" + + def __init__(self, *, token: str = "", base_url: str | None = None) -> None: + self.token = token + self.base_url = base_url + + +class _FakeWebClient: + """Minimal stand-in for slack_sdk.WebClient.""" + + def __init__(self, *, token: str = "", base_url: str | None = None) -> None: + self.token = token + self.base_url = base_url + -pytest.importorskip("slack_sdk") +_fake_slack_sdk_web_async.AsyncWebClient = _FakeAsyncWebClient # type: ignore[attr-defined] +_fake_slack_sdk_web.async_client = _fake_slack_sdk_web_async # type: ignore[attr-defined] +_fake_slack_sdk.web = _fake_slack_sdk_web # type: ignore[attr-defined] +_fake_slack_sdk.WebClient = _FakeWebClient # type: ignore[attr-defined] + +sys.modules.setdefault("slack_sdk", _fake_slack_sdk) +sys.modules.setdefault("slack_sdk.web", _fake_slack_sdk_web) +sys.modules.setdefault("slack_sdk.web.async_client", _fake_slack_sdk_web_async) + +from chat_sdk.adapters.discord.adapter import DISCORD_API_BASE, DiscordAdapter # noqa: E402 +from chat_sdk.adapters.discord.types import DiscordAdapterConfig # noqa: E402 +from chat_sdk.adapters.github.adapter import GITHUB_API_BASE_URL, GitHubAdapter # noqa: E402 +from chat_sdk.adapters.linear.adapter import LINEAR_API_URL, LinearAdapter # noqa: E402 +from chat_sdk.adapters.linear.types import LinearAdapterAPIKeyConfig # noqa: E402 +from chat_sdk.adapters.slack.adapter import SlackAdapter # noqa: E402 +from chat_sdk.adapters.slack.types import SlackAdapterConfig # noqa: E402 +from chat_sdk.logger import ConsoleLogger # noqa: E402 +from chat_sdk.shared.errors import ValidationError # noqa: E402 TEST_PUBLIC_KEY = "a" * 64 @@ -141,6 +183,19 @@ def test_no_base_url_passed_when_unset(self, monkeypatch: pytest.MonkeyPatch) -> assert "base_url" not in async_client.kwargs assert "base_url" not in sync_client.kwargs + def test_empty_api_url_falls_back_to_default(self, monkeypatch: pytest.MonkeyPatch) -> None: + # Upstream feeds the WebClients via the truthy spread + # ``...(this.slackApiUrl ? {...} : {})`` (index.ts:577/621), so an empty + # ``apiUrl`` must fall back to slack_sdk's built-in default -- never pass + # ``base_url=""`` (which would point the WebClients at an empty host). + _patch_slack_clients(monkeypatch) + adapter = self._make(api_url="") + assert adapter._slack_api_url is None + async_client = adapter._get_client("xoxb-tok") + sync_client = adapter._get_web_client_for_token("xoxb-tok") + assert "base_url" not in async_client.kwargs + assert "base_url" not in sync_client.kwargs + def test_env_fallback_used_when_config_unset(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("SLACK_API_URL", "https://slack.env.example/api/") _patch_slack_clients(monkeypatch) @@ -193,6 +248,17 @@ def test_custom_api_url_stored(self) -> None: adapter = self._make(api_url="https://discord.proxy.example/api/v10") assert adapter._api_base_url == "https://discord.proxy.example/api/v10" + async def test_empty_api_url_falls_back_to_default(self) -> None: + # ``_discord_fetch`` joins ``f"{base}{path}"`` directly, so an empty + # ``apiUrl`` must resolve to ``DISCORD_API_BASE`` rather than producing + # a relative ``/channels/...`` URL. + adapter = self._make(api_url="") + assert adapter._api_base_url == DISCORD_API_BASE + session = _RecordingSession({"id": "1"}) + adapter._get_http_session = AsyncMock(return_value=session) # type: ignore[method-assign] + await adapter._discord_fetch("/channels/123/messages", "POST", body={"content": "hi"}) + assert session.urls == [f"{DISCORD_API_BASE}/channels/123/messages"] + def test_env_fallback(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("DISCORD_API_URL", "https://discord.env.example/api/v10") adapter = self._make() @@ -243,6 +309,17 @@ def test_custom_api_url_strips_trailing_slash(self) -> None: adapter = self._make(api_url="https://github.example.com/api/v3/") assert adapter._api_url == "https://github.example.com/api/v3" + async def test_empty_api_url_falls_back_to_default(self) -> None: + # Upstream's truthy spread ``...(this.apiUrl ? { baseUrl } : {})`` means + # an empty ``apiUrl`` uses the default api.github.com endpoint, not an + # empty base that would yield a relative request URL. + adapter = self._make(api_url="") + assert adapter._api_url == GITHUB_API_BASE_URL + session = _RecordingSession({"id": 1}) + adapter._get_http_session = AsyncMock(return_value=session) # type: ignore[method-assign] + await adapter._github_api_request("GET", "/repos/acme/app/pulls/1") + assert session.urls == ["https://api.github.com/repos/acme/app/pulls/1"] + def test_env_fallback(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("GITHUB_API_URL", "https://github.env.example/api/v3") adapter = self._make() @@ -308,6 +385,18 @@ def test_custom_api_url_stored(self) -> None: adapter = self._make(api_url="https://linear.proxy.example/graphql") assert adapter._api_url == "https://linear.proxy.example/graphql" + async def test_empty_api_url_falls_back_to_default(self) -> None: + # ``_graphql_query`` POSTs straight to ``self._api_url``; the truthy + # fallback (mirroring upstream ``...(this.apiUrl ? { apiUrl } : {})``) + # means an empty ``apiUrl`` posts to the real Linear endpoint rather + # than an empty/relative URL. + adapter = self._make(api_url="") + assert adapter._api_url == LINEAR_API_URL + session = _RecordingSession({"data": {}}) + adapter._get_http_session = AsyncMock(return_value=session) # type: ignore[method-assign] + await adapter._graphql_query("query { viewer { id } }") + assert session.urls == [LINEAR_API_URL] + def test_env_fallback(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("LINEAR_API_URL", "https://linear.env.example/graphql") adapter = self._make()