Skip to content

Commit f7fac3d

Browse files
fix: adapter hygiene — Slack stream await, Teams divider, public worker APIs (#48)
fix: adapter hygiene — Slack stream await, Teams divider, public worker APIs
2 parents 6292436 + 3297b3b commit f7fac3d

8 files changed

Lines changed: 453 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,15 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
### Fixes
6+
- **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.
7+
- **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).
8+
9+
### Python-only additions
10+
- **`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).
11+
- **`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`).
12+
313
## 0.4.26 (2026-04-16)
414

515
Synced to [Vercel Chat 4.26.0](https://github.com/vercel/chat).

docs/UPSTREAM_SYNC.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,8 @@ stay explicit instead of being rediscovered in code review.
455455
| Google Chat image rendering | Images emit as `{alt} ({url})` or bare `url` | No image branch — falls through to default which concatenates children only, dropping the URL | Upstream silently drops image URLs when rendering to Google Chat text. We preserve the URL so the message content isn't lost. |
456456
| Fallback streaming stream-exception capture | `_fallback_stream` captures exceptions from the stream iterator, flushes whatever content was already rendered, awaits `pending_edit`, and re-raises after cleanup | `try/finally` only — exception propagates immediately, `pendingEdit` is un-awaited, and the placeholder is stranded as `"..."` | Upstream leaves a hard UX failure when streams crash mid-flight (common: LLM connection drops): placeholder visible forever, orphan background task. We flush + clean up before re-raising so the caller still sees the original error and users see the partial content instead of a spinner. |
457457
| 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. |
458+
| 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. |
459+
| `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. |
458460

459461
### Platform-specific gaps
460462

src/chat_sdk/adapters/slack/adapter.py

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,43 @@ def lock_scope(self) -> str:
252252
def persist_message_history(self) -> bool:
253253
return self._persist_message_history
254254

255+
# ------------------------------------------------------------------
256+
# Public request-context accessors
257+
#
258+
# These are Python-only extensions to the Adapter surface. They let
259+
# code running inside a handler call the Slack Web API directly —
260+
# e.g. ``users.info`` for caller-email resolution — without
261+
# reaching into the underscore-prefixed ``_get_token`` /
262+
# ``_get_client`` helpers. See docs/UPSTREAM_SYNC.md.
263+
# ------------------------------------------------------------------
264+
265+
@property
266+
def current_token(self) -> str:
267+
"""Return the bot token bound to the current request context.
268+
269+
In multi-workspace mode this is the token resolved by the
270+
``InstallationStore`` for the current request; in single-workspace
271+
mode it is the default bot token. Raises
272+
:class:`AuthenticationError` when called outside a request context
273+
with no default token configured.
274+
"""
275+
return self._get_token()
276+
277+
@property
278+
def current_client(self) -> Any:
279+
"""Return an ``AsyncWebClient`` preconfigured with :attr:`current_token`.
280+
281+
Return type is ``Any`` (rather than the concrete
282+
``AsyncWebClient``) because ``slack_sdk`` is an optional
283+
dependency — consumers who install the SDK without the `slack`
284+
extra shouldn't pay a type-check-time import cost. Docstring
285+
captures the actual runtime type for tooling that reads it.
286+
287+
The returned client is LRU-cached by token. Raises
288+
:class:`AuthenticationError` when no token is available.
289+
"""
290+
return self._get_client()
291+
255292
# ------------------------------------------------------------------
256293
# Token management
257294
# ------------------------------------------------------------------
@@ -2030,7 +2067,7 @@ async def stream(
20302067
if options.task_display_mode:
20312068
stream_kwargs["task_display_mode"] = options.task_display_mode
20322069

2033-
streamer = client.chat_stream(**stream_kwargs)
2070+
streamer = await client.chat_stream(**stream_kwargs)
20342071

20352072
first = True
20362073
last_appended = ""

src/chat_sdk/adapters/teams/cards.py

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@
3131
# Used when a card has select/radio_select inputs but no submit button.
3232
AUTO_SUBMIT_ACTION_ID = "__auto_submit"
3333

34+
# Internal marker key for divider placeholders. Stripped by
35+
# ``_hoist_dividers`` before the card is serialized — never reaches Teams.
36+
_DIVIDER_MARKER = "__chatSdkDivider"
37+
3438

3539
def _convert_emoji(text: str) -> str:
3640
"""Convert emoji placeholders to Teams format."""
@@ -96,6 +100,8 @@ def card_to_adaptive_card(card: CardElement) -> dict[str, Any]:
96100
body.extend(result["elements"])
97101
actions.extend(result["actions"])
98102

103+
body = _hoist_dividers(body)
104+
99105
adaptive_card: dict[str, Any] = {
100106
"type": "AdaptiveCard",
101107
"$schema": ADAPTIVE_CARD_SCHEMA,
@@ -121,7 +127,14 @@ def _convert_child_to_adaptive(child: CardChild) -> dict[str, Any]:
121127
if child_type == "image":
122128
return {"elements": [_convert_image_to_element(child)], "actions": []} # type: ignore[arg-type]
123129
if child_type == "divider":
124-
return {"elements": [{"type": "Container", "separator": True, "items": []}], "actions": []}
130+
# Emit an internal marker instead of the final Container. The
131+
# post-processing pass (_hoist_dividers) either moves ``separator``
132+
# onto the next sibling (preferred — renders as a full-width line)
133+
# or, for a trailing divider with no next sibling, replaces it with
134+
# a minimal non-empty Container so the separator is visible. An
135+
# empty ``Container`` with ``separator: True`` renders at zero
136+
# height in Microsoft Teams.
137+
return {"elements": [{_DIVIDER_MARKER: True}], "actions": []}
125138
if child_type == "actions":
126139
return _convert_actions_to_elements(child) # type: ignore[arg-type]
127140
if child_type == "section":
@@ -150,6 +163,37 @@ def _convert_child_to_adaptive(child: CardChild) -> dict[str, Any]:
150163
return {"elements": [], "actions": []}
151164

152165

166+
def _hoist_dividers(elements: list[dict[str, Any]]) -> list[dict[str, Any]]:
167+
"""Replace internal divider markers with ``separator: True`` on the next sibling.
168+
169+
An Adaptive Card ``Container`` with ``separator: True`` and no ``items``
170+
renders at zero height in Microsoft Teams — the separator line is
171+
effectively invisible. Instead, hoist the separator onto the following
172+
sibling so it renders as a full-width line above that element. For a
173+
trailing divider with no next sibling, emit a non-empty Container so
174+
the separator is still visible.
175+
"""
176+
result: list[dict[str, Any]] = []
177+
pending = False
178+
for el in elements:
179+
if el.get(_DIVIDER_MARKER):
180+
pending = True
181+
continue
182+
if pending:
183+
el = {**el, "separator": True}
184+
pending = False
185+
result.append(el)
186+
if pending:
187+
result.append(
188+
{
189+
"type": "Container",
190+
"separator": True,
191+
"items": [{"type": "TextBlock", "text": " ", "wrap": False}],
192+
}
193+
)
194+
return result
195+
196+
153197
def _convert_text_to_element(element: TextElement) -> dict[str, Any]:
154198
"""Convert a text element to an Adaptive Card TextBlock."""
155199
content = element.get("content", "")
@@ -319,6 +363,7 @@ def _convert_section_to_elements(element: SectionElement) -> dict[str, Any]:
319363
container_items.extend(result["elements"])
320364
actions.extend(result["actions"])
321365

366+
container_items = _hoist_dividers(container_items)
322367
if container_items:
323368
elements.append({"type": "Container", "items": container_items})
324369

src/chat_sdk/chat.py

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1378,6 +1378,84 @@ def channel(self, channel_id: str) -> ChannelImpl:
13781378
)
13791379
)
13801380

1381+
def thread(
1382+
self,
1383+
thread_id: str,
1384+
*,
1385+
current_message: Message | None = None,
1386+
) -> ThreadImpl:
1387+
"""Get a Thread by its thread ID (e.g. 'slack:C123ABC:1234567890.123456').
1388+
1389+
The adapter is resolved from the thread ID prefix; state and message
1390+
history come from this Chat instance. This is the public
1391+
construction path for worker processes — use it instead of
1392+
reaching into ``ThreadImpl`` / ``_ThreadImplConfig`` directly.
1393+
1394+
Parameters
1395+
----------
1396+
thread_id:
1397+
Fully-qualified thread ID. Must start with a registered
1398+
adapter name followed by ``:``.
1399+
current_message:
1400+
Optional reference to the message the worker is responding to.
1401+
Required for Slack native streaming, which populates
1402+
``recipient_user_id`` / ``recipient_team_id`` from the author
1403+
of this message. Omit for post-only worker flows.
1404+
1405+
Mirrors ``chat.thread(threadId)`` from the upstream TS SDK.
1406+
"""
1407+
# Validate thread ID shape structurally, before calling the adapter.
1408+
# A valid ID is `{adapter}:{channel}[:{rest}...]` with:
1409+
# - non-empty adapter prefix
1410+
# - non-empty **channel** segment (the part between the first
1411+
# and second colon, if there's a second colon; or the whole
1412+
# remainder if not)
1413+
# Rejects `"slack:"`, `"slack::"`, `"slack::thread"`, etc. at
1414+
# the SDK boundary so we don't rely on each adapter's
1415+
# `channel_id_from_thread_id` doing its own defense. (Different
1416+
# adapters return different things for malformed input; we need
1417+
# a single source of truth.)
1418+
#
1419+
# Checking just "any segment is non-empty" would wrongly accept
1420+
# `"slack::thread"` (empty channel, non-empty thread).
1421+
adapter_name, sep, remainder = thread_id.partition(":")
1422+
if not sep or not adapter_name or not remainder:
1423+
raise ChatError(f"Invalid thread ID: {thread_id}")
1424+
channel_segment, _, _ = remainder.partition(":")
1425+
if not channel_segment:
1426+
raise ChatError(f"Invalid thread ID: {thread_id}")
1427+
1428+
adapter = self._adapters.get(adapter_name)
1429+
if adapter is None:
1430+
raise ChatError(f'Adapter "{adapter_name}" not found for thread ID "{thread_id}"')
1431+
1432+
# Defer to the adapter to derive channel_id as a secondary sanity
1433+
# check — some platform-specific patterns can pass the structural
1434+
# check but still be malformed (e.g. wrong number of segments).
1435+
try:
1436+
derived_channel_id = adapter.channel_id_from_thread_id(thread_id)
1437+
except Exception as exc:
1438+
raise ChatError(f"Invalid thread ID: {thread_id}") from exc
1439+
if not derived_channel_id:
1440+
raise ChatError(f"Invalid thread ID: {thread_id}")
1441+
1442+
stub_message = (
1443+
current_message
1444+
if current_message is not None
1445+
else Message(
1446+
id="",
1447+
thread_id=thread_id,
1448+
text="",
1449+
formatted={"type": "root", "children": []},
1450+
raw=None,
1451+
author=Author(user_id="", user_name="", full_name="", is_bot=False, is_me=False),
1452+
# Deterministic timestamp for the stub message — `datetime.now()`
1453+
# makes this method non-deterministic and harder to test.
1454+
metadata=MessageMetadata(date_sent=datetime.fromtimestamp(0, tz=timezone.utc), edited=False),
1455+
)
1456+
)
1457+
return self._create_thread(adapter, thread_id, stub_message, False)
1458+
13811459
# ========================================================================
13821460
# Adapter inference
13831461
# ========================================================================

tests/test_chat_resolver.py

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,3 +426,139 @@ async def task(chat: Chat, name: str) -> None:
426426
await asyncio.gather(task(chat_a, "ca"), task(chat_b, "cb"))
427427
assert results["ca"] == "ca"
428428
assert results["cb"] == "cb"
429+
430+
431+
class TestChatThreadFactory:
432+
"""``Chat.thread(thread_id)`` — public worker-reconstruction path (issue #46).
433+
434+
Mirrors ``chat.thread(threadId)`` from the upstream TS SDK. Lets worker
435+
processes rebuild a Thread bound to this Chat's state and the adapter
436+
inferred from the thread ID prefix — no need to reach into
437+
``ThreadImpl`` / ``_ThreadImplConfig`` directly.
438+
"""
439+
440+
def setup_method(self):
441+
clear_chat_singleton()
442+
443+
def teardown_method(self):
444+
clear_chat_singleton()
445+
446+
def test_infers_adapter_from_thread_id_prefix(self):
447+
chat = _make_chat("slack")
448+
thread = chat.thread("slack:C123:1234567890.123456")
449+
assert thread.adapter.name == "slack"
450+
assert thread.id == "slack:C123:1234567890.123456"
451+
452+
def test_propagates_explicit_current_message(self):
453+
"""Slack native streaming reads ``current_message`` to populate
454+
recipient IDs; the public factory must let workers supply it.
455+
"""
456+
from datetime import datetime, timezone
457+
458+
from chat_sdk import Author, Message, MessageMetadata
459+
460+
chat = _make_chat("slack")
461+
thread_id = "slack:C123:1234567890.123456"
462+
msg = Message(
463+
id="M1",
464+
thread_id=thread_id,
465+
text="hi",
466+
formatted={"type": "root", "children": []},
467+
raw=None,
468+
author=Author(user_id="U1", user_name="alice", full_name="Alice", is_bot=False, is_me=False),
469+
# Fixed timestamp — `datetime.now()` makes tests non-deterministic.
470+
metadata=MessageMetadata(date_sent=datetime(2024, 1, 1, tzinfo=timezone.utc), edited=False),
471+
)
472+
thread = chat.thread(thread_id, current_message=msg)
473+
assert thread._current_message is msg
474+
475+
def test_omitting_current_message_stubs_a_placeholder(self):
476+
"""Workers that only post (no streaming) can omit ``current_message``."""
477+
chat = _make_chat("slack")
478+
thread = chat.thread("slack:C123:1234567890.123456")
479+
assert thread._current_message is not None
480+
assert thread._current_message.id == ""
481+
482+
def test_reuses_parent_chat_state_and_history(self):
483+
"""The factory must bind the new Thread to the parent Chat's state
484+
adapter and message history. This is the core contract of
485+
`chat.thread()` — worker processes reconstruct a Thread that
486+
shares state with the original Chat instance, not a fresh
487+
detached thread. Without this, state writes/reads from the worker
488+
wouldn't be visible to the Chat in-process.
489+
490+
For adapters that don't persist message history (persist_message_history
491+
falsy), `_create_thread` intentionally skips the history bind —
492+
there's no cache to share. This test uses a persisting adapter to
493+
exercise the positive case.
494+
"""
495+
chat = _make_chat("slack")
496+
# Opt the mock adapter into message history so the factory binds it
497+
adapter = chat._adapters["slack"]
498+
adapter.persist_message_history = True
499+
500+
thread = chat.thread("slack:C123:1234567890.123456")
501+
# State adapter must be the exact same instance, not a copy
502+
assert thread._state_adapter is chat._state_adapter
503+
# Message history must be the exact same instance too
504+
assert thread._message_history is chat._message_history
505+
506+
def test_omits_history_when_adapter_does_not_persist(self):
507+
"""Adapters with `persist_message_history` falsy opt out of the
508+
shared history cache. `Chat.thread()` respects that: the Thread
509+
gets `None` for history rather than a shared cache the adapter
510+
won't populate.
511+
512+
Explicitly set `persist_message_history = None` rather than
513+
relying on the mock's default — prevents silent test regression
514+
if the mock default ever changes.
515+
"""
516+
chat = _make_chat("slack")
517+
adapter = chat._adapters["slack"]
518+
adapter.persist_message_history = None
519+
520+
thread = chat.thread("slack:C123:1234567890.123456")
521+
assert thread._state_adapter is chat._state_adapter
522+
assert thread._message_history is None
523+
524+
def test_invalid_thread_id_raises(self):
525+
from chat_sdk.errors import ChatError
526+
527+
chat = _make_chat("slack")
528+
with pytest.raises(ChatError, match="Invalid thread ID"):
529+
chat.thread("no-colon-here")
530+
531+
def test_empty_remainder_raises(self):
532+
"""IDs where the channel segment is empty — even when later
533+
segments are populated — would create a thread with no channel
534+
context. Surface the error at construction time rather than
535+
relying on adapter-specific `channel_id_from_thread_id` (which
536+
may legitimately return empty, raise, or pass through the
537+
original input depending on adapter).
538+
539+
Crucial cases: `slack::thread` and `slack::foo:bar` have an
540+
empty channel segment but non-empty later segments. An "any
541+
segment non-empty" check would accept these; the structural
542+
validation checks the channel segment specifically.
543+
"""
544+
from chat_sdk.errors import ChatError
545+
546+
chat = _make_chat("slack")
547+
bad_ids = (
548+
"slack:",
549+
"slack::",
550+
"slack:::",
551+
"slack::::",
552+
"slack::thread", # empty channel, non-empty thread
553+
"slack::foo:bar", # empty channel, multi-segment rest
554+
)
555+
for bad in bad_ids:
556+
with pytest.raises(ChatError, match="Invalid thread ID"):
557+
chat.thread(bad)
558+
559+
def test_unregistered_adapter_raises(self):
560+
from chat_sdk.errors import ChatError
561+
562+
chat = _make_chat("slack")
563+
with pytest.raises(ChatError, match='Adapter "teams" not found'):
564+
chat.thread("teams:foo:bar")

0 commit comments

Comments
 (0)