diff --git a/CHANGELOG.md b/CHANGELOG.md index a3f4c7e..ed69854 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,14 +1,75 @@ # Changelog -## Unreleased +## 0.4.26.1 (2026-04-23) + +Python-only follow-up on `0.4.26`. Still alpha — APIs may change. ### Fixes -- **Slack native streaming no longer crashes on first chunk** (issue #44): `SlackAdapter.stream()` now awaits `AsyncWebClient.chat_stream(...)`; the previous code called `.append()` on an unawaited coroutine, raising `AttributeError` and forcing callers onto the post+edit fallback. Existing tests were updated to use `AsyncMock` for `chat_stream` so they mirror the real client. -- **Teams divider now renders a visible separator line** (issue #45): `card_to_adaptive_card` previously emitted an empty `Container` with `separator: True`, which Microsoft Teams renders at zero height. The new behavior hoists `separator: True` onto the following sibling (or emits a minimal non-empty Container for a trailing divider). Upstream TS ships the same bug; documented as a divergence in [UPSTREAM_SYNC.md](docs/UPSTREAM_SYNC.md). -### Python-only additions -- **`SlackAdapter.current_token` / `current_client`** (issue #47): public `@property` accessors that return the request-context-bound bot token and a preconfigured `AsyncWebClient`. Replaces reaching into `_get_token()` / `_get_client()` from consumer code that needs to call the Slack Web API directly from inside a handler (email resolution, user profile fetches, etc.). TS keeps `getToken()` private; documented as a Python-only extension in [UPSTREAM_SYNC.md](docs/UPSTREAM_SYNC.md). -- **`Chat.thread(thread_id, *, current_message=None)`** (issue #46): public worker-reconstruction factory mirroring TS `chat.thread(threadId)`. Adapter is inferred from the thread ID prefix; state and message history come from the Chat instance. Pass `current_message` when the worker needs Slack native streaming (it populates `recipient_user_id` / `recipient_team_id`). +- **Slack native streaming**: `SlackAdapter.stream()` no longer calls + `AsyncWebClient.chat_stream(...)` without `await`. The unawaited coroutine + returned a truthy object, and the first `streamer.append(...)` raised + `AttributeError`, breaking native Slack streaming for any consumer using + the default adapter. Issue #44. +- **Teams divider renders at non-zero height**: empty `Container` with + `separator: True` rendered as zero-height in the Teams UI. Dividers + between siblings now hoist `separator: True` onto the following element; + a trailing divider emits a minimal non-empty Container. Issue #45. +- **`ConcurrencyConfig.max_concurrent` is now enforced**: consumers setting + `concurrency=ConcurrencyConfig(strategy="concurrent", max_concurrent=N)` + now actually get an `asyncio.Semaphore(N)` cap on in-flight handlers. + Previously the field was accepted and ignored (upstream TS has the same + gap). `None` / unset keeps the unbounded default. Issue #51. + +### Python-specific (divergence from upstream 4.26) + +- **Fallback streaming runtime robustness** (cluster of fixes): framework- + agnostic `request.text()` handling now tolerates sync Flask-style + requests (was raising `TypeError: object is not awaitable`). Handlers + typed `Callable[..., Awaitable[None] | None]` may return sync (`None`) — + the dispatcher now `await`s only when `inspect.isawaitable()` confirms, + preventing runtime crashes on sync handlers. +- **`max_concurrent` enforcement** (see above) — upstream accepts the + config field but never enforces it; we do. + +### New public APIs + +- **`Chat.thread(thread_id, *, current_message=None)`**: new worker- + reconstruction factory mirroring TS `chat.thread(threadId)`. Adapter is + inferred from the thread-ID prefix; state and message history come from + the Chat instance. `current_message` is preserved so Slack native + streaming still works post-reconstruction. Issue #46. +- **`SlackAdapter.current_token` / `current_client`**: public `@property` + accessors for the request-context-bound bot token and a preconfigured + `AsyncWebClient`. Replaces underscore access from consumer code making + direct Slack Web API calls inside a handler (email resolution, user + profile fetches, etc.). Issue #47. + +### Internals + +- **Pyrefly: 213 → 0 type errors**; baseline file removed. CI now enforces + zero errors. Root causes fixed: 8-adapter `lock_scope: LockScope | None` + protocol conformance; `_ChatSingleton` as `Protocol`; submodule-aware + `replace-imports-with-any`; `NoReturn` on error re-raisers; + `inspect.isawaitable` guards for duck-typed request handling and + sync-or-async handler dispatch. No `Any` widening, no new `# type: + ignore` lines beyond 10 at adapter event-construction sites where + `thread=None`/`channel=None` get re-wrapped by `Chat` before handler + dispatch (matches upstream TS's `Omit<>` partial-event pattern). +- Test count: **3545 passed**, 2 skipped. + +### Known gaps (not fixed in this release) + +- `onOptionsLoad` handler for dynamic select dropdowns — issue #50 +- `Thread.getParticipants()` method — issue #54 +- `rehydrate_attachment` adapter hook for queue/debounce + attachments — + issue #52 +- 40 upstream tests without Python equivalents (Options Load, Plan variants, + StreamingPlan options, getParticipants) — issue #53 +- Discord native Gateway WebSocket (HTTP-only today) — issue #57 +- Teams certificate-based mTLS auth — issue #58 +- Google Chat file uploads (TODO upstream too) — issue #59 +- Global handler-dispatch bound across reactions/actions/slash/modals — issue #61 ## 0.4.26 (2026-04-16) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9ba3d3b..b1cb3ce 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -58,13 +58,20 @@ All PRs must pass `ruff check` with zero errors. Vercel Chat version. See [UPSTREAM_SYNC.md](docs/UPSTREAM_SYNC.md#version-mapping). - `0.4.25` = synced to upstream `4.25.0` -- `0.4.25.1` = Python-only fix on top of `4.25.0` +- `0.4.25.1` = Python-only changes (fixes, and additive features during + alpha) between upstream sync points - `0.4.26a1` = alpha while porting upstream `4.26.0` +> **Additive changes in `.patch` bumps are OK during alpha**. The package +> is marked `Development Status :: 3 - Alpha` and the `0.x.y` prefix signals +> pre-1.0 per semver convention, so new public APIs can land in `.patch` +> bumps without a version-scheme violation. Once we hit `1.0`, `.patch` +> should be fixes-only. +> > **Upstream patch releases**: Vercel Chat has historically gone straight to > minor bumps, but if upstream ships a patch (e.g. `4.25.1`) we sync it by > bumping to the next minor (`0.4.26`). We don't reuse the `.patch` slot for -> upstream patches — it's reserved for Python-only fixes so the two can't +> upstream patches — it's reserved for Python-only changes so the two can't > collide. ### Steps diff --git a/docs/UPSTREAM_SYNC.md b/docs/UPSTREAM_SYNC.md index 1d2d89b..2600324 100644 --- a/docs/UPSTREAM_SYNC.md +++ b/docs/UPSTREAM_SYNC.md @@ -457,6 +457,7 @@ stay explicit instead of being rediscovered in code review. | Fallback streaming final SentMessage content | SentMessage + final edit carry `final_content` (remend'd — inline markers auto-closed) | SentMessage + final edit carry raw `accumulated` | Narrow UX refinement. If a stream ends with an unclosed `*`/`~~`/etc., upstream ships the unclosed marker; we run `_remend` so the user sees a clean final message. Not observable in the common case where streams close their own markers. | | Teams divider rendering | `card_to_adaptive_card` hoists `separator: True` onto the next sibling (or emits a non-empty Container for a trailing divider) | `convertDividerToElement` emits an empty `Container` with `separator: True` | Upstream shares the same bug: Microsoft Teams renders an empty Container at zero height, so the separator line is effectively invisible. Python port fixes locally (issue #45) rather than blocking on upstream. | | `SlackAdapter.current_token` / `current_client` | Public `@property` accessors that return the request-context-bound token and a preconfigured `AsyncWebClient` | Not exposed (`getToken()` is private on the TS `SlackAdapter`) | Python-only addition (issue #47). Downstream code that calls Slack Web APIs from inside a handler — email resolution, user profile fetches, reaction bookkeeping — otherwise depends on underscore-prefixed helpers. | +| `ConcurrencyConfig.max_concurrent` | Enforced via `asyncio.Semaphore` in the `"concurrent"` strategy path; rejects non-integer or `<= 0` values, and rejects any non-`None` `max_concurrent` paired with a non-`"concurrent"` strategy | Accepted into the config type with docstring "Default: Infinity" but never read (3 writes, 0 reads) | Silent correctness bug upstream — consumers setting `max_concurrent=N` with `strategy="concurrent"` reasonably expect an N-way bound on in-flight handlers. We honor the documented contract via a semaphore and fail-fast on misconfiguration so it's never silent. `max_concurrent=None` stays compatible with every strategy (unbounded default). | ### Platform-specific gaps diff --git a/pyproject.toml b/pyproject.toml index 36b34b6..1c14d62 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "chat-sdk" -version = "0.4.26" +version = "0.4.26.1" description = "Multi-platform async chat SDK for Python — port of Vercel Chat" readme = "README.md" license = {text = "MIT"} diff --git a/src/chat_sdk/chat.py b/src/chat_sdk/chat.py index 3d5c10d..aaac6c6 100644 --- a/src/chat_sdk/chat.py +++ b/src/chat_sdk/chat.py @@ -39,6 +39,7 @@ Channel, ChannelVisibility, ChatConfig, + ConcurrencyConfig, ConcurrencyStrategy, EmojiValue, Lock, @@ -297,6 +298,48 @@ def __init__(self, config: ChatConfig | None = None, **kwargs: Any) -> None: self._concurrency_on_queue_full = concurrency.on_queue_full self._concurrency_queue_entry_ttl_ms = concurrency.queue_entry_ttl_ms + # -- Concurrent-strategy semaphore ------------------------------------ + # Divergence from upstream — see docs/UPSTREAM_SYNC.md. + # `max_concurrent` bounds in-flight handler dispatches when using the + # `"concurrent"` strategy. `None` means unbounded (matches the upstream + # TS default of `Infinity`). A positive integer caps parallel handler + # runs. Upstream accepts the config field but never enforces it + # (3 writes, 0 reads); we enforce it via `asyncio.Semaphore`. + # + # Only construct the semaphore when the strategy actually uses it — + # if a user sets `max_concurrent=5` with `strategy="queue"`, they + # have a misconfiguration that we surface as a `ValueError` rather + # than silently allocating an unused primitive. + # + # Reject `<= 0` explicitly rather than silently ignoring — a user + # passing `max_concurrent=0` likely means "pause all processing" + # (not supported) or has a typo. Either way, silently falling back + # to unbounded concurrency would surprise them. + raw_max = concurrency.max_concurrent if isinstance(concurrency, ConcurrencyConfig) else None + if raw_max is not None and ( + # Reject non-int (including bool, which is an int subclass but + # semantically meaningless here) before any arithmetic — + # `asyncio.Semaphore(1.5)` silently goes negative, `Semaphore(True)` + # allocates a 1-way bound from a boolean, and `Semaphore("2")` + # raises `TypeError` instead of our ValueError. + isinstance(raw_max, bool) or not isinstance(raw_max, int) or raw_max <= 0 + ): + raise ValueError( + f"ConcurrencyConfig.max_concurrent must be a positive integer or None; " + f"got {raw_max!r}. Pass None for unbounded concurrency." + ) + if self._concurrency_max_concurrent is not None and self._concurrency_strategy != "concurrent": + raise ValueError( + f"ConcurrencyConfig.max_concurrent is only honored when strategy='concurrent'; " + f"got strategy={self._concurrency_strategy!r}. Either switch to strategy='concurrent' " + "or drop max_concurrent." + ) + self._concurrent_semaphore: asyncio.Semaphore | None = ( + asyncio.Semaphore(self._concurrency_max_concurrent) + if self._concurrency_max_concurrent is not None + else None + ) + # -- Message history (placeholder -- real impl would use MessageHistoryCache) self._message_history = _MessageHistoryCache(self._state_adapter, config.message_history) @@ -1840,7 +1883,15 @@ async def _handle_concurrent( thread_id: str, message: Message, ) -> None: - await self._dispatch_to_handlers(adapter, thread_id, message) + # Enforce `max_concurrent` bound when configured. Upstream TS + # accepts the config field but never enforces it; we do, so that + # consumers setting `ConcurrencyConfig(strategy="concurrent", + # max_concurrent=N)` actually get a bound of N in-flight handlers. + if self._concurrent_semaphore is None: + await self._dispatch_to_handlers(adapter, thread_id, message) + return + async with self._concurrent_semaphore: + await self._dispatch_to_handlers(adapter, thread_id, message) # ======================================================================== # Dispatch to handlers diff --git a/tests/test_chat_faithful.py b/tests/test_chat_faithful.py index bb24eae..4bb8766 100644 --- a/tests/test_chat_faithful.py +++ b/tests/test_chat_faithful.py @@ -2263,6 +2263,178 @@ async def handler(thread, message, context=None): assert len(calls) == 1 assert calls[0] == "Hey @slack-bot concurrent" + # Python-specific: upstream accepts max_concurrent but doesn't enforce + # it. We do. Bound should cap in-flight handlers at N; the (N+1)th + # message has to wait until one of the first N releases. + async def test_max_concurrent_bounds_in_flight_handlers(self): + state = create_mock_state() + adapter = create_mock_adapter("slack") + + chat, _, _ = await _init_chat( + adapter=adapter, + state=state, + concurrency=ConcurrencyConfig(strategy="concurrent", max_concurrent=2), + ) + + in_flight = 0 + max_observed = 0 + gate = asyncio.Event() + finished = 0 + + @chat.on_mention + async def handler(thread, message, context=None): + nonlocal in_flight, max_observed, finished + in_flight += 1 + max_observed = max(max_observed, in_flight) + await gate.wait() + in_flight -= 1 + finished += 1 + + # Dispatch 5 messages concurrently — at most 2 should be in flight + # at any time while the gate is closed. + tasks = [ + asyncio.create_task( + chat.handle_incoming_message( + adapter, + f"slack:C123:{i}", + create_test_message(f"msg-{i}", "Hey @slack-bot"), + ) + ) + for i in range(5) + ] + + # Wait until the first 2 handlers reach the gate. asyncio uses a + # single-threaded cooperative scheduler, so between `_reach_cap` + # returning and the next assertion, no other task can interleave + # — tasks 3-5 are parked on `semaphore.acquire()`. The + # `in_flight == 2` check IS stable here. + async def _reach_cap() -> None: + while in_flight < 2: + await asyncio.sleep(0.001) + + await asyncio.wait_for(_reach_cap(), timeout=1.0) + # Snapshot while the gate is still closed: exactly the bound + # should be in flight, and no more. + assert in_flight == 2 + + # Release the gate; all 5 should drain. If the semaphore leaked, + # `max_observed` inside the handlers captured the peak before + # any could unblock, so the final assertion below would fail. + gate.set() + await asyncio.gather(*tasks) + + assert finished == 5 + # The critical assertion: peak in-flight never exceeded 2. + assert max_observed == 2 + + # Python-specific: reject invalid `max_concurrent` values at construction + # time rather than silently falling back to unbounded (which would + # surprise users who set `max_concurrent=0` expecting strict throttling). + async def test_max_concurrent_zero_or_negative_raises(self): + state = create_mock_state() + adapter = create_mock_adapter("slack") + + for bad_value in (0, -1, -100): + import pytest + + with pytest.raises(ValueError, match="max_concurrent must be a positive integer or None"): + await _init_chat( + adapter=adapter, + state=state, + concurrency=ConcurrencyConfig(strategy="concurrent", max_concurrent=bad_value), + ) + + # Python-specific: reject non-integer `max_concurrent` at construction + # instead of letting `asyncio.Semaphore` misbehave (`1.5` silently drives + # the counter negative, `True` allocates a 1-way bound from a bool, + # `"2"` raises `TypeError` from inside the primitive instead of our + # `ValueError`). + async def test_max_concurrent_non_integer_raises(self): + import pytest + + state = create_mock_state() + adapter = create_mock_adapter("slack") + + for bad_value in (1.5, True, False, "2", 0.0, [1]): + with pytest.raises(ValueError, match="max_concurrent must be a positive integer or None"): + await _init_chat( + adapter=adapter, + state=state, + concurrency=ConcurrencyConfig(strategy="concurrent", max_concurrent=bad_value), # type: ignore[arg-type] + ) + + # Python-specific: setting `max_concurrent` with a non-concurrent strategy + # is a misconfiguration — the field is only honored under `"concurrent"`. + # Fail loudly instead of silently allocating an unused semaphore. + async def test_max_concurrent_with_non_concurrent_strategy_raises(self): + import pytest + + state = create_mock_state() + adapter = create_mock_adapter("slack") + + for bad_strategy in ("queue", "debounce", "drop"): + with pytest.raises(ValueError, match="only honored when strategy='concurrent'"): + await _init_chat( + adapter=adapter, + state=state, + concurrency=ConcurrencyConfig(strategy=bad_strategy, max_concurrent=5), + ) + + # Python-specific: None / missing max_concurrent must keep the + # unbounded behavior (matches upstream TS default of Infinity). + # Parameterized to cover both the string form (max_concurrent implicit) + # and the explicit ConcurrencyConfig(max_concurrent=None) form — the + # two take separate code paths in Chat.__init__ (string → defaults, + # ConcurrencyConfig → field read), so both must be verified. + @pytest.mark.parametrize( + "concurrency_value", + [ + "concurrent", + ConcurrencyConfig(strategy="concurrent", max_concurrent=None), + ], + ids=["string", "config_none"], + ) + async def test_max_concurrent_none_allows_unbounded(self, concurrency_value): + state = create_mock_state() + adapter = create_mock_adapter("slack") + + chat, _, _ = await _init_chat(adapter=adapter, state=state, concurrency=concurrency_value) + + in_flight = 0 + max_observed = 0 + gate = asyncio.Event() + + @chat.on_mention + async def handler(thread, message, context=None): + nonlocal in_flight, max_observed + in_flight += 1 + max_observed = max(max_observed, in_flight) + await gate.wait() + in_flight -= 1 + + tasks = [ + asyncio.create_task( + chat.handle_incoming_message( + adapter, + f"slack:C123:{i}", + create_test_message(f"msg-{i}", "Hey @slack-bot"), + ) + ) + for i in range(5) + ] + + # Poll until all 5 are in flight; with no semaphore they should + # all reach the gate. + async def _reach_five() -> None: + while in_flight < 5: + await asyncio.sleep(0.001) + + await asyncio.wait_for(_reach_five(), timeout=1.0) + assert in_flight == 5 + gate.set() + await asyncio.gather(*tasks) + assert max_observed == 5 + # ============================================================================ # 22. lockScope (tests 87-91)