diff --git a/src/chat_sdk/adapters/slack/adapter.py b/src/chat_sdk/adapters/slack/adapter.py index 7cb2f1a..921841e 100644 --- a/src/chat_sdk/adapters/slack/adapter.py +++ b/src/chat_sdk/adapters/slack/adapter.py @@ -19,6 +19,7 @@ import os import re import time +import warnings from collections import OrderedDict from collections.abc import AsyncIterable, Awaitable, Callable from contextvars import ContextVar @@ -395,6 +396,12 @@ def __init__(self, config: SlackAdapterConfig | None = None) -> None: # on each subsequent webhook entry. Static-string configs prime this at # construction time so sync access works before any webhook fires. self._default_bot_token_cache: str | None = bot_token_config if isinstance(bot_token_config, str) else None + # True when the user passed a callable ``bot_token`` (sync or async). + # The config contract says callable resolvers are invoked on each use + # to support rotation, so sync access goes through a fresh-invoke + # branch instead of reading ``_default_bot_token_cache``. Static + # strings have nothing to rotate and stay on the cached fast path. + self._is_dynamic_bot_token: bool = callable(bot_token_config) # ------------------------------------------------------------------ # Socket mode wiring (PR #86). Resolved AFTER the bot-token resolver @@ -466,6 +473,16 @@ def __init__(self, config: SlackAdapterConfig | None = None) -> None: self._client_cache: OrderedDict[str, Any] = OrderedDict() self._client_cache_max = config.client_cache_max if config.client_cache_max is not None else 100 + # Cache of synchronous slack_sdk.WebClient instances keyed by bot + # token, backing the public ``web_client`` property (the direct port + # of upstream's ``getClientForToken``). Kept separate from + # ``_client_cache`` because that one holds async ``AsyncWebClient`` + # instances used by the adapter's own API calls; the two client types + # are not interchangeable. Mirrors upstream's plain (unbounded) Map — + # one entry per distinct token — since callers reach for this escape + # hatch rarely and tokens are low-cardinality. + self._web_client_cache: dict[str, Any] = {} + # Multi-workspace OAuth fields. # ``is not None`` (not truthiness) so an explicit empty-string user # config does not silently fall back to env (hazard #1). Empty env @@ -590,6 +607,61 @@ def current_client(self) -> Any: """ return self._get_client() + @property + def web_client(self) -> Any: + """Direct access to a synchronous ``slack_sdk.WebClient``. + + Bound to the bot token for the current request context + (multi-workspace) or the configured default token + (single-workspace). Use for any Slack Web API call not covered by + the adapter's high-level methods — e.g. + ``adapter.web_client.pins_add(...)`` or + ``adapter.web_client.usergroups_list(...)``. + + Resolution order (the standard 3-level resolver): + + 1. Token from the current request context (set during webhook + handling, or by :meth:`with_bot_token` / :meth:`with_bot_token_async`). + 2. The default bot token, when configured as a static string or + already-resolved value. + 3. Otherwise raise :class:`AuthenticationError`. + + Raises :class:`AuthenticationError` if neither is available — + typical causes are accessing ``web_client`` outside any + webhook / :meth:`with_bot_token` context in multi-workspace mode, + or having configured ``bot_token`` as an async resolver that has + not run yet. In the latter case await + :meth:`current_token_async` (or process the work inside the + webhook flow) so the resolver primes the token first. + + Return type is ``Any`` (rather than the concrete ``WebClient``) + because ``slack_sdk`` is an optional dependency — consumers who do + not install the ``slack`` extra should not pay an import cost. + + This is the direct port of upstream's ``adapter.webClient`` getter + (vercel/chat ``2f108bd``). Unlike :attr:`current_client` it returns + the *synchronous* ``WebClient`` (the analog of the single TS + ``WebClient``), so its methods are not awaitables. + """ + return self._get_web_client_for_token(self._get_token()) + + @property + def client(self) -> Any: + """Deprecated alias for :attr:`web_client`. + + .. deprecated:: + Use :attr:`web_client` instead. This alias mirrors upstream's + pre-rename ``adapter.client`` (vercel/chat ``8366b8b``) and is + kept for one release for backwards compatibility; it will be + removed in a future version. Emits :class:`DeprecationWarning`. + """ + warnings.warn( + "SlackAdapter.client is deprecated; use SlackAdapter.web_client instead.", + DeprecationWarning, + stacklevel=2, + ) + return self.web_client + # ------------------------------------------------------------------ # Token management # ------------------------------------------------------------------ @@ -602,17 +674,19 @@ def _get_token(self) -> str: 1. Multi-workspace request context (``_request_context``) 2. Per-request resolved default token (``_resolved_default_token``) — primed by ``handle_webhook`` after invoking the resolver - 3. Static default token cache (set by the constructor for - string-typed ``bot_token`` configs) - 4. Raises :class:`AuthenticationError` - - Synchronous: the token resolver is *not* invoked here. Webhook entry - paths call :meth:`_resolve_default_token` first so the per-request - cache is primed; static-string ``bot_token`` configs prime the - process-wide cache at construction time so this is a no-op for the - common case. Use :meth:`_resolve_token_async` from new async call - sites that need to invoke a resolver outside webhook flow (e.g. - cron jobs). + 3. Sync dynamic resolver — invoked **fresh on every call** to honor + the rotation contract in :attr:`SlackAdapterConfig.bot_token` + ("called on each use to support rotation") + 4. Static default token cache (set by the constructor for + string-typed ``bot_token`` configs, or by the async path after + ``_resolve_default_token`` runs) + 5. Raises :class:`AuthenticationError` + + Async resolvers cannot be awaited from a sync context — sync access + outside a webhook scope falls back to the process-wide cache, or + raises if the async path has not run yet. Use + :meth:`current_token_async` or enter via :meth:`handle_webhook` so + the resolver runs first. """ ctx = self._request_context.get() if ctx and ctx.token: @@ -620,16 +694,45 @@ def _get_token(self) -> str: per_request = self._resolved_default_token.get() if per_request is not None: return per_request + # Sync dynamic resolver: invoke fresh every call to honor rotation. + # Static strings (no rotation possible) and async resolvers (which + # need a webhook entry to be awaited) fall through to the cache. + provider = self._default_bot_token_provider + if self._is_dynamic_bot_token and provider is not None and not inspect.iscoroutinefunction(provider): + resolved = provider() + # Defensive: a "sync" callable may still *return* a coroutine + # (e.g. ``lambda: some_async_fn()``) and ``iscoroutinefunction`` + # would not catch that. Refuse to use such a value in a sync + # context — and close the awaitable to suppress the + # ``coroutine was never awaited`` RuntimeWarning before raising. + if inspect.isawaitable(resolved): + close = getattr(resolved, "close", None) + if callable(close): + close() + raise AuthenticationError( + "slack", + "Bot token resolver returned an awaitable in a sync " + "context. Use the async API (handle_webhook / " + "current_token_async) so the resolver can be awaited.", + ) + if not isinstance(resolved, str) or not resolved: + raise AuthenticationError( + "slack", + "Bot token resolver returned an empty or non-string value.", + ) + # Intentionally do NOT write ``_default_bot_token_cache``: caching + # would break the rotation contract for the next sync access. + return resolved if self._default_bot_token_cache is not None: return self._default_bot_token_cache - if self._default_bot_token_provider is not None: - # Resolver-based default token configured but never resolved. This - # is a programming error: the async entry path should have awaited - # ``_resolve_default_token()`` before reaching here. + if provider is not None: + # Async resolver configured but never awaited. ``handle_webhook`` + # or ``current_token_async`` must run first to prime the cache. raise AuthenticationError( "slack", - "Bot token resolver has not been invoked yet. Use the async API " - "(handle_webhook / current_token_async) so the resolver runs first.", + "Async bot token resolver has not been invoked yet. Use the " + "async API (handle_webhook / current_token_async) so the " + "resolver runs first.", ) raise AuthenticationError( "slack", @@ -740,8 +843,40 @@ def _get_client(self, token: str | None = None) -> Any: return client def _invalidate_client(self, token: str) -> None: - """Remove a cached client (e.g., on token revocation).""" + """Remove a cached client (e.g., on token revocation). + + For dynamic-resolver configs also clears the resolved-token caches + so the next access re-invokes the resolver instead of serving the + revoked token. Static-string configs intentionally retain their + cache: there is no refresh path, so clearing would just make every + subsequent sync access raise. + """ self._client_cache.pop(token, None) + self._web_client_cache.pop(token, None) + if self._is_dynamic_bot_token: + if self._default_bot_token_cache == token: + self._default_bot_token_cache = None + if self._resolved_default_token.get() == token: + self._resolved_default_token.set(None) + + def _get_web_client_for_token(self, token: str) -> Any: + """Return a synchronous ``slack_sdk.WebClient`` for *token*, cached. + + Backs the public :attr:`web_client` property and is the direct port + of upstream's ``getClientForToken`` (vercel/chat ``2f108bd``): one + cached ``WebClient`` instance per distinct token. The import is + deferred so ``slack_sdk`` stays an optional dependency (hazard #10). + + Distinct from :meth:`_get_client`, which caches the *async* + ``AsyncWebClient`` used by the adapter's own API calls. + """ + client = self._web_client_cache.get(token) + if client is None: + from slack_sdk import WebClient + + client = WebClient(token=token) + self._web_client_cache[token] = client + return client # ------------------------------------------------------------------ # Initialization diff --git a/tests/test_slack_dynamic_token_and_verifier.py b/tests/test_slack_dynamic_token_and_verifier.py index 0fbf91e..0c74f9a 100644 --- a/tests/test_slack_dynamic_token_and_verifier.py +++ b/tests/test_slack_dynamic_token_and_verifier.py @@ -181,18 +181,41 @@ def resolver() -> str: assert await adapter.current_token_async() == "xoxb-token-3" assert i[0] == 3 - def test_sync_current_token_with_resolver_before_resolution_raises(self): - """Sync ``current_token`` access before the resolver has run raises a clear error.""" + def test_sync_current_token_with_sync_resolver_invokes_resolver(self): + """Sync ``current_token`` invokes a sync resolver directly (Codex P2 fix). + + Previously the sync path raised ``AuthenticationError`` for sync + resolvers too — single-workspace apps using a sync resolver for + secret rotation could not read ``current_token`` / ``web_client`` + outside a webhook context until an async path had primed the cache. + ``_get_token`` now invokes the sync resolver directly — fresh on + every call, to honor the rotation contract on + :attr:`SlackAdapterConfig.bot_token`. The + async-resolver-in-sync-context case still raises (see + ``test_sync_current_token_with_async_resolver_raises`` below). + """ def resolver() -> str: return "xoxb-resolved" adapter = SlackAdapter(SlackAdapterConfig(signing_secret="s", bot_token=resolver)) - # Tightened: error message must mention ``current_token_async`` so - # callers know the right async accessor to use, not just that "the - # resolver hasn't run". Substring check on "current_token_async" - # is escaped via ``re.escape`` so the underscore isn't treated as a - # regex token (it isn't, but be explicit). + assert adapter.current_token == "xoxb-resolved" + # Dynamic resolvers must NOT prime ``_default_bot_token_cache``; + # caching would suppress subsequent resolver calls and break the + # rotation contract. Rotation behavior is pinned by + # ``test_sync_callable_invoked_fresh_each_access`` in + # tests/test_slack_web_client.py. + assert adapter._default_bot_token_cache is None + + def test_sync_current_token_with_async_resolver_raises(self): + """Async resolvers still cannot be awaited from the sync property.""" + + async def resolver() -> str: + return "xoxb-async-resolved" + + adapter = SlackAdapter(SlackAdapterConfig(signing_secret="s", bot_token=resolver)) + # Error message must point at the async entry point so callers know + # the right accessor to use. with pytest.raises(AuthenticationError, match=r"current_token_async"): _ = adapter.current_token @@ -316,6 +339,12 @@ async def test_resolver_refreshes_sync_token_cache(self): refresh the process-wide ``_default_bot_token_cache`` so the sync path returns the freshly resolved value. + Uses an *async* resolver so the sanity precondition (sync access + before any resolution raises) still holds — sync resolvers now + resolve directly on the sync path (Codex P2 fix), so the regression + scenario this test guards is specifically the async path priming the + sync cache. + What to fix if this fails: in ``_resolve_default_token`` (``adapters/slack/adapter.py``), after the ``isinstance(token, str)`` / non-empty validation and before @@ -323,13 +352,13 @@ async def test_resolver_refreshes_sync_token_cache(self): ``self._default_bot_token_cache = token``. """ - def resolver() -> str: + async def resolver() -> str: return "xoxb-resolved-token" adapter = SlackAdapter(SlackAdapterConfig(signing_secret="s", bot_token=resolver)) - # Sanity: before the resolver runs, sync ``current_token`` must raise - # (matches ``test_sync_current_token_with_resolver_before_resolution_raises``). + # Sanity: before the async resolver runs, sync ``current_token`` must + # raise — async resolvers cannot be awaited from the sync property. with pytest.raises(AuthenticationError, match=r"current_token_async"): _ = adapter.current_token diff --git a/tests/test_slack_web_client.py b/tests/test_slack_web_client.py new file mode 100644 index 0000000..be526f2 --- /dev/null +++ b/tests/test_slack_web_client.py @@ -0,0 +1,433 @@ +"""Tests for the public ``SlackAdapter.web_client`` property and ``client`` alias. + +Port of upstream vercel/chat's ``webClient getter`` test block +(``packages/adapter-slack/src/index.test.ts``), added by commits +``8366b8b`` / ``fdebde7`` / ``2f108bd`` (PRs #471 / #476 / #478): the Slack +adapter exposes a synchronous ``WebClient`` bound to the current +request-context token (multi-workspace) or the configured default token +(single-workspace), with a one-release deprecated ``client`` alias. + +``web_client`` resolves its token via the standard 3-level resolver +(ContextVar token > static default token > ``AuthenticationError``) and +caches one ``WebClient`` per distinct token. +""" + +from __future__ import annotations + +import sys +import warnings +from types import ModuleType + +import pytest + +# --------------------------------------------------------------------------- +# Stub slack_sdk so tests run without the real dependency installed. +# Stub BOTH the sync ``slack_sdk.WebClient`` (backing ``web_client``) and the +# async ``slack_sdk.web.async_client.AsyncWebClient`` (backing the adapter's +# own API calls) so the deferred imports inside the adapter resolve here. +# --------------------------------------------------------------------------- + + +class _FakeWebClient: + """Minimal stand-in for the synchronous ``slack_sdk.WebClient``.""" + + def __init__(self, *, token: str = "") -> None: + self.token = token + + +class _FakeAsyncWebClient: + """Minimal stand-in for ``slack_sdk.web.async_client.AsyncWebClient``.""" + + def __init__(self, *, token: str = "") -> None: + self.token = token + + +# Reuse any ``slack_sdk`` stub already registered by a sibling test module +# (e.g. ``test_slack_client_cache.py``) when collected in the same process — +# ``setdefault`` would otherwise no-op and leave a stub that lacks the sync +# ``WebClient`` symbol. Attach the symbols we need either way so the deferred +# imports inside the adapter resolve regardless of collection order. +_fake_slack_sdk = sys.modules.setdefault("slack_sdk", ModuleType("slack_sdk")) +_fake_slack_sdk_web = sys.modules.setdefault("slack_sdk.web", ModuleType("slack_sdk.web")) +_fake_slack_sdk_web_async = sys.modules.setdefault( + "slack_sdk.web.async_client", ModuleType("slack_sdk.web.async_client") +) + +if not hasattr(_fake_slack_sdk, "WebClient"): + _fake_slack_sdk.WebClient = _FakeWebClient # type: ignore[attr-defined] +if not hasattr(_fake_slack_sdk_web_async, "AsyncWebClient"): + _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] + +from chat_sdk.adapters.slack.adapter import SlackAdapter # noqa: E402, I001 +from chat_sdk.adapters.slack.types import SlackAdapterConfig # noqa: E402 +from chat_sdk.shared.errors import AuthenticationError # noqa: E402 + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +_SECRET = "test-signing-secret" + + +def _single_workspace_adapter(bot_token: str = "xoxb-static-token") -> SlackAdapter: + """Single-workspace adapter with a static default bot token.""" + return SlackAdapter(SlackAdapterConfig(signing_secret=_SECRET, bot_token=bot_token)) + + +def _multi_workspace_adapter() -> SlackAdapter: + """Multi-workspace adapter (no default bot token; tokens are per-team).""" + return SlackAdapter( + SlackAdapterConfig( + signing_secret=_SECRET, + client_id="test-client-id", + client_secret="test-client-secret", + ) + ) + + +# --------------------------------------------------------------------------- +# Single-workspace +# --------------------------------------------------------------------------- + + +class TestWebClientSingleWorkspace: + def test_returns_client_bound_to_static_bot_token(self): + """``web_client`` returns a WebClient bound to the configured token.""" + adapter = _single_workspace_adapter("xoxb-static-token") + + web_client = adapter.web_client + + # Constructed via ``slack_sdk.WebClient`` (whichever class is + # registered in this process — real or sibling stub). + assert isinstance(web_client, _fake_slack_sdk.WebClient) + assert web_client.token == "xoxb-static-token" + + def test_returns_same_instance_per_token(self): + """Repeated access returns the exact same cached object (identity).""" + adapter = _single_workspace_adapter() + + # Bind each property access to a name so the identity check reads as a + # caching assertion (two calls → same object), not a tautological + # ``x is x`` self-comparison. + first = adapter.web_client + second = adapter.web_client + + assert first is second + + def test_caches_under_the_resolved_token(self): + """The cached client is keyed by the resolved token.""" + adapter = _single_workspace_adapter("xoxb-cache-key") + + client = adapter.web_client + + assert adapter._web_client_cache["xoxb-cache-key"] is client + + def test_invalidate_client_clears_web_client_cache(self): + """``_invalidate_client`` evicts the sync WebClient too (token revocation). + + Load-bearing: without the ``_web_client_cache.pop`` in + ``_invalidate_client`` a revoked token's stale ``WebClient`` would + survive eviction and this assertion fails. + """ + adapter = _single_workspace_adapter("xoxb-revoke") + client = adapter.web_client + assert adapter._web_client_cache["xoxb-revoke"] is client + + adapter._invalidate_client("xoxb-revoke") + + assert "xoxb-revoke" not in adapter._web_client_cache + + +# --------------------------------------------------------------------------- +# Deprecated ``client`` alias +# --------------------------------------------------------------------------- + + +class TestClientAlias: + def test_alias_returns_same_object_as_web_client(self): + """The deprecated ``client`` alias returns the same instance.""" + adapter = _single_workspace_adapter() + + with warnings.catch_warnings(): + warnings.simplefilter("ignore", DeprecationWarning) + assert adapter.client is adapter.web_client + + def test_alias_emits_deprecation_warning(self): + """Accessing ``client`` warns; ``web_client`` does not.""" + adapter = _single_workspace_adapter() + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + _ = adapter.client + + deprecations = [w for w in caught if issubclass(w.category, DeprecationWarning)] + assert len(deprecations) == 1 + assert "web_client" in str(deprecations[0].message) + + def test_web_client_does_not_warn(self): + """The non-deprecated ``web_client`` property emits no warning.""" + adapter = _single_workspace_adapter() + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + _ = adapter.web_client + + deprecations = [w for w in caught if issubclass(w.category, DeprecationWarning)] + assert deprecations == [] + + +# --------------------------------------------------------------------------- +# Multi-workspace (request-context token resolution) +# --------------------------------------------------------------------------- + + +class TestWebClientMultiWorkspace: + def test_uses_request_context_token_under_with_bot_token(self): + """Inside ``with_bot_token`` the context token wins.""" + adapter = _multi_workspace_adapter() + + observed: dict[str, str] = {} + + def capture() -> None: + observed["token"] = adapter.web_client.token + + adapter.with_bot_token("xoxb-context-token", capture) + + assert observed["token"] == "xoxb-context-token" + + def test_context_token_overrides_static_default(self): + """A context token takes precedence over the configured default.""" + adapter = _single_workspace_adapter("xoxb-default") + + observed: dict[str, str] = {} + + def capture() -> None: + observed["token"] = adapter.web_client.token + + adapter.with_bot_token("xoxb-override", capture) + + assert observed["token"] == "xoxb-override" + # Outside the context, resolution falls back to the static default. + assert adapter.web_client.token == "xoxb-default" + + def test_raises_without_context_in_multi_workspace_mode(self): + """No context + no default token raises on both accessors.""" + adapter = _multi_workspace_adapter() + + with pytest.raises(AuthenticationError): + _ = adapter.web_client + + with warnings.catch_warnings(): + warnings.simplefilter("ignore", DeprecationWarning) + with pytest.raises(AuthenticationError): + _ = adapter.client + + +# --------------------------------------------------------------------------- +# Sync resolver: invoked directly from ``web_client`` (Codex P2 fix) +# Async resolver raising from the sync property is covered by +# ``TestWebClientSyncResolver.test_async_callable_in_sync_context_raises``. +# --------------------------------------------------------------------------- + + +class TestWebClientSyncResolver: + """Cover the sync-callable ``bot_token`` branch in ``_get_token``. + + Before the fix, ``_get_token`` only handled the static-string and primed + cache cases — sync callables (used e.g. for lazy secret-manager loads or + rotation) raised ``AuthenticationError`` from ``web_client`` outside any + webhook / ContextVar scope, so proactive sends were impossible until an + async path had primed the cache. The sync-callable branch invokes the + resolver fresh on every access to honor the rotation contract documented + on :attr:`SlackAdapterConfig.bot_token`. + """ + + def test_sync_callable_invoked_fresh_each_access(self): + """Sync ``bot_token`` callables are invoked on *every* sync read. + + Load-bearing for two contracts: + + 1. The bot_token contract in ``types.py`` ("called on each use to + support rotation"). + 2. The audit fix that made sync resolvers reachable from + ``web_client`` at all (reverting it makes the first access raise). + + If the resolver were cached after the first call, a rotating + secret-manager-backed resolver would silently freeze on the + original value. + """ + calls = {"n": 0} + + def resolver() -> str: + # ``f"xoxb-sync-{n}"`` rather than ``next(iter([...]))`` so the + # test is robust if assertions evolve to add a 4th+ access — a + # plain iterator would StopIteration instead of failing cleanly. + calls["n"] += 1 + return f"xoxb-sync-{calls['n']}" + + adapter = SlackAdapter(SlackAdapterConfig(signing_secret=_SECRET, bot_token=resolver)) + + first = adapter.web_client + assert first.token == "xoxb-sync-1" + assert calls["n"] == 1 + + # Second access must re-invoke the resolver and see the rotated + # token — caching would break rotation. + second = adapter.web_client + assert second.token == "xoxb-sync-2" + assert calls["n"] == 2 + # WebClient cache is keyed by token; a new token yields a new client. + assert second is not first + + third = adapter.current_token + assert third == "xoxb-sync-3" + assert calls["n"] == 3 + + # Crucially, the dynamic-resolver path must NOT prime the + # process-wide cache — that would suppress future resolver calls. + assert adapter._default_bot_token_cache is None + + def test_async_callable_in_sync_context_raises(self): + """``async def`` resolvers cannot be awaited from the sync property.""" + + async def resolver() -> str: + return "xoxb-async-resolved" + + adapter = SlackAdapter(SlackAdapterConfig(signing_secret=_SECRET, bot_token=resolver)) + + with pytest.raises(AuthenticationError) as excinfo: + _ = adapter.web_client + + # Not brittle about exact wording — just confirm the message points at + # the sync/async resolver mismatch rather than the generic + # "no bot token" path. + message = str(excinfo.value).lower() + assert "resolver" in message + assert "async" in message + + def test_sync_callable_returning_coroutine_raises(self): + """Defensive: a sync callable that returns a coroutine must not be cached. + + The production code is responsible for closing the orphaned coroutine + before raising — otherwise callers see a noisy ``coroutine was never + awaited`` RuntimeWarning. We pin that behavior by capturing warnings + and asserting none were emitted. + """ + import warnings + + async def _coro() -> str: + return "xoxb-would-be-resolved" + + def resolver(): + # ``iscoroutinefunction`` returns False for this — the awaitable + # only appears at call time. Caching the coroutine would be a + # latent bug; the defensive check must raise AND close the + # coroutine to silence the warning. + return _coro() + + adapter = SlackAdapter(SlackAdapterConfig(signing_secret=_SECRET, bot_token=resolver)) + + with warnings.catch_warnings(record=True) as captured: + warnings.simplefilter("always") + with pytest.raises(AuthenticationError) as excinfo: + _ = adapter.web_client + + message = str(excinfo.value).lower() + assert "awaitable" in message or "async" in message + # Cache must not have been poisoned with the coroutine object. + assert adapter._default_bot_token_cache is None + # No "coroutine was never awaited" RuntimeWarning leaked — production + # code closed the coroutine before raising. + unawaited = [ + str(w.message) + for w in captured + if issubclass(w.category, RuntimeWarning) and "never awaited" in str(w.message) + ] + assert unawaited == [], f"unexpected un-awaited coroutine warnings: {unawaited}" + + def test_sync_callable_returning_empty_string_raises(self): + """An empty/invalid resolver result raises rather than caching it.""" + + def resolver() -> str: + return "" + + adapter = SlackAdapter(SlackAdapterConfig(signing_secret=_SECRET, bot_token=resolver)) + + with pytest.raises(AuthenticationError): + _ = adapter.web_client + assert adapter._default_bot_token_cache is None + + +class TestInvalidateClientClearsTokenCaches: + """Pin that ``_invalidate_client`` clears the resolved-token caches for + dynamic-resolver configs. + + Before the fix, ``_invalidate_client`` only evicted the WebClient and + AsyncWebClient caches — the next ``_get_token`` would still return the + revoked token from ``_default_bot_token_cache`` (or the per-request + ContextVar), so a 401-driven invalidation just rebuilt clients around the + same revoked credential. + """ + + def test_invalidate_clears_default_cache_for_dynamic_resolver(self): + """Async-resolver path: after invalidation the cache is cleared. + + Load-bearing: with the fix reverted, ``_default_bot_token_cache`` + still equals the revoked token after invalidation. + """ + + async def resolver() -> str: + return "xoxb-revoked" + + adapter = SlackAdapter(SlackAdapterConfig(signing_secret=_SECRET, bot_token=resolver)) + # Simulate the async path having already primed the cache. + adapter._default_bot_token_cache = "xoxb-revoked" + + adapter._invalidate_client("xoxb-revoked") + + assert adapter._default_bot_token_cache is None + + def test_invalidate_clears_per_request_token_for_dynamic_resolver(self): + """ContextVar-primed per-request token is cleared on invalidation.""" + + async def resolver() -> str: + return "xoxb-revoked" + + adapter = SlackAdapter(SlackAdapterConfig(signing_secret=_SECRET, bot_token=resolver)) + adapter._resolved_default_token.set("xoxb-revoked") + + adapter._invalidate_client("xoxb-revoked") + + assert adapter._resolved_default_token.get() is None + + def test_invalidate_static_string_does_not_clear_token_cache(self): + """Static-string ``bot_token`` configs have no refresh path. + + Clearing the cache for a static-string config would just make every + subsequent sync access raise, with no way to recover. The fix + intentionally guards on ``_is_dynamic_bot_token`` to preserve the + static-string fast path. + """ + adapter = SlackAdapter(SlackAdapterConfig(signing_secret=_SECRET, bot_token="xoxb-static")) + assert adapter._default_bot_token_cache == "xoxb-static" + + adapter._invalidate_client("xoxb-static") + + # Client caches evicted but token cache retained. + assert adapter._default_bot_token_cache == "xoxb-static" + + def test_invalidate_different_token_does_not_clear_cache(self): + """Only invalidation of the *currently cached* token clears it.""" + + async def resolver() -> str: + return "xoxb-current" + + adapter = SlackAdapter(SlackAdapterConfig(signing_secret=_SECRET, bot_token=resolver)) + adapter._default_bot_token_cache = "xoxb-current" + + adapter._invalidate_client("xoxb-some-other-token") + + assert adapter._default_bot_token_cache == "xoxb-current"