Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
# Changelog

## Unreleased

### 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`).

## 0.4.26 (2026-04-16)

Synced to [Vercel Chat 4.26.0](https://github.com/vercel/chat).
Expand Down
2 changes: 2 additions & 0 deletions docs/UPSTREAM_SYNC.md
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,8 @@ stay explicit instead of being rediscovered in code review.
| 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. |
| 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. |
| 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. |

### Platform-specific gaps

Expand Down
39 changes: 38 additions & 1 deletion src/chat_sdk/adapters/slack/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,43 @@ def lock_scope(self) -> str:
def persist_message_history(self) -> bool:
return self._persist_message_history

# ------------------------------------------------------------------
# Public request-context accessors
#
# These are Python-only extensions to the Adapter surface. They let
# code running inside a handler call the Slack Web API directly —
# e.g. ``users.info`` for caller-email resolution — without
# reaching into the underscore-prefixed ``_get_token`` /
# ``_get_client`` helpers. See docs/UPSTREAM_SYNC.md.
# ------------------------------------------------------------------

@property
def current_token(self) -> str:
"""Return the bot token bound to the current request context.

In multi-workspace mode this is the token resolved by the
``InstallationStore`` for the current request; in single-workspace
mode it is the default bot token. Raises
:class:`AuthenticationError` when called outside a request context
with no default token configured.
"""
return self._get_token()

@property
def current_client(self) -> Any:
"""Return an ``AsyncWebClient`` preconfigured with :attr:`current_token`.

Return type is ``Any`` (rather than the concrete
``AsyncWebClient``) because ``slack_sdk`` is an optional
dependency — consumers who install the SDK without the `slack`
extra shouldn't pay a type-check-time import cost. Docstring
captures the actual runtime type for tooling that reads it.

The returned client is LRU-cached by token. Raises
:class:`AuthenticationError` when no token is available.
"""
return self._get_client()

# ------------------------------------------------------------------
# Token management
# ------------------------------------------------------------------
Expand Down Expand Up @@ -2030,7 +2067,7 @@ async def stream(
if options.task_display_mode:
stream_kwargs["task_display_mode"] = options.task_display_mode

streamer = client.chat_stream(**stream_kwargs)
streamer = await client.chat_stream(**stream_kwargs)

first = True
last_appended = ""
Expand Down
47 changes: 46 additions & 1 deletion src/chat_sdk/adapters/teams/cards.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@
# Used when a card has select/radio_select inputs but no submit button.
AUTO_SUBMIT_ACTION_ID = "__auto_submit"

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


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

body = _hoist_dividers(body)

adaptive_card: dict[str, Any] = {
"type": "AdaptiveCard",
"$schema": ADAPTIVE_CARD_SCHEMA,
Expand All @@ -121,7 +127,14 @@ def _convert_child_to_adaptive(child: CardChild) -> dict[str, Any]:
if child_type == "image":
return {"elements": [_convert_image_to_element(child)], "actions": []} # type: ignore[arg-type]
if child_type == "divider":
return {"elements": [{"type": "Container", "separator": True, "items": []}], "actions": []}
# Emit an internal marker instead of the final Container. The
# post-processing pass (_hoist_dividers) either moves ``separator``
# onto the next sibling (preferred — renders as a full-width line)
# or, for a trailing divider with no next sibling, replaces it with
# a minimal non-empty Container so the separator is visible. An
# empty ``Container`` with ``separator: True`` renders at zero
# height in Microsoft Teams.
return {"elements": [{_DIVIDER_MARKER: True}], "actions": []}
if child_type == "actions":
return _convert_actions_to_elements(child) # type: ignore[arg-type]
if child_type == "section":
Expand Down Expand Up @@ -150,6 +163,37 @@ def _convert_child_to_adaptive(child: CardChild) -> dict[str, Any]:
return {"elements": [], "actions": []}


def _hoist_dividers(elements: list[dict[str, Any]]) -> list[dict[str, Any]]:
"""Replace internal divider markers with ``separator: True`` on the next sibling.

An Adaptive Card ``Container`` with ``separator: True`` and no ``items``
renders at zero height in Microsoft Teams — the separator line is
effectively invisible. Instead, hoist the separator onto the following
sibling so it renders as a full-width line above that element. For a
trailing divider with no next sibling, emit a non-empty Container so
the separator is still visible.
"""
result: list[dict[str, Any]] = []
pending = False
for el in elements:
if el.get(_DIVIDER_MARKER):
pending = True
continue
if pending:
el = {**el, "separator": True}
pending = False
result.append(el)
if pending:
result.append(
{
"type": "Container",
"separator": True,
"items": [{"type": "TextBlock", "text": " ", "wrap": False}],
}
)
return result


def _convert_text_to_element(element: TextElement) -> dict[str, Any]:
"""Convert a text element to an Adaptive Card TextBlock."""
content = element.get("content", "")
Expand Down Expand Up @@ -319,6 +363,7 @@ def _convert_section_to_elements(element: SectionElement) -> dict[str, Any]:
container_items.extend(result["elements"])
actions.extend(result["actions"])

container_items = _hoist_dividers(container_items)
if container_items:
elements.append({"type": "Container", "items": container_items})

Expand Down
78 changes: 78 additions & 0 deletions src/chat_sdk/chat.py
Original file line number Diff line number Diff line change
Expand Up @@ -1378,6 +1378,84 @@ def channel(self, channel_id: str) -> ChannelImpl:
)
)

def thread(
self,
thread_id: str,
*,
current_message: Message | None = None,
) -> ThreadImpl:
"""Get a Thread by its thread ID (e.g. 'slack:C123ABC:1234567890.123456').

The adapter is resolved from the thread ID prefix; state and message
history come from this Chat instance. This is the public
construction path for worker processes — use it instead of
reaching into ``ThreadImpl`` / ``_ThreadImplConfig`` directly.

Parameters
----------
thread_id:
Fully-qualified thread ID. Must start with a registered
adapter name followed by ``:``.
current_message:
Optional reference to the message the worker is responding to.
Required for Slack native streaming, which populates
``recipient_user_id`` / ``recipient_team_id`` from the author
of this message. Omit for post-only worker flows.

Mirrors ``chat.thread(threadId)`` from the upstream TS SDK.
"""
# Validate thread ID shape structurally, before calling the adapter.
# A valid ID is `{adapter}:{channel}[:{rest}...]` with:
# - non-empty adapter prefix
# - non-empty **channel** segment (the part between the first
# and second colon, if there's a second colon; or the whole
# remainder if not)
# Rejects `"slack:"`, `"slack::"`, `"slack::thread"`, etc. at
# the SDK boundary so we don't rely on each adapter's
# `channel_id_from_thread_id` doing its own defense. (Different
# adapters return different things for malformed input; we need
# a single source of truth.)
#
# Checking just "any segment is non-empty" would wrongly accept
# `"slack::thread"` (empty channel, non-empty thread).
adapter_name, sep, remainder = thread_id.partition(":")
if not sep or not adapter_name or not remainder:
raise ChatError(f"Invalid thread ID: {thread_id}")
channel_segment, _, _ = remainder.partition(":")
if not channel_segment:
raise ChatError(f"Invalid thread ID: {thread_id}")

adapter = self._adapters.get(adapter_name)
if adapter is None:
raise ChatError(f'Adapter "{adapter_name}" not found for thread ID "{thread_id}"')

# Defer to the adapter to derive channel_id as a secondary sanity
# check — some platform-specific patterns can pass the structural
# check but still be malformed (e.g. wrong number of segments).
try:
derived_channel_id = adapter.channel_id_from_thread_id(thread_id)
except Exception as exc:
raise ChatError(f"Invalid thread ID: {thread_id}") from exc
if not derived_channel_id:
raise ChatError(f"Invalid thread ID: {thread_id}")

stub_message = (
current_message
if current_message is not None
else Message(
id="",
thread_id=thread_id,
text="",
formatted={"type": "root", "children": []},
raw=None,
author=Author(user_id="", user_name="", full_name="", is_bot=False, is_me=False),
# Deterministic timestamp for the stub message — `datetime.now()`
# makes this method non-deterministic and harder to test.
metadata=MessageMetadata(date_sent=datetime.fromtimestamp(0, tz=timezone.utc), edited=False),
)
)
return self._create_thread(adapter, thread_id, stub_message, False)

# ========================================================================
# Adapter inference
# ========================================================================
Expand Down
136 changes: 136 additions & 0 deletions tests/test_chat_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -426,3 +426,139 @@ async def task(chat: Chat, name: str) -> None:
await asyncio.gather(task(chat_a, "ca"), task(chat_b, "cb"))
assert results["ca"] == "ca"
assert results["cb"] == "cb"


class TestChatThreadFactory:
"""``Chat.thread(thread_id)`` — public worker-reconstruction path (issue #46).

Mirrors ``chat.thread(threadId)`` from the upstream TS SDK. Lets worker
processes rebuild a Thread bound to this Chat's state and the adapter
inferred from the thread ID prefix — no need to reach into
``ThreadImpl`` / ``_ThreadImplConfig`` directly.
"""

def setup_method(self):
clear_chat_singleton()

def teardown_method(self):
clear_chat_singleton()

def test_infers_adapter_from_thread_id_prefix(self):
chat = _make_chat("slack")
thread = chat.thread("slack:C123:1234567890.123456")
assert thread.adapter.name == "slack"
assert thread.id == "slack:C123:1234567890.123456"

def test_propagates_explicit_current_message(self):
"""Slack native streaming reads ``current_message`` to populate
recipient IDs; the public factory must let workers supply it.
"""
from datetime import datetime, timezone

from chat_sdk import Author, Message, MessageMetadata

chat = _make_chat("slack")
thread_id = "slack:C123:1234567890.123456"
msg = Message(
id="M1",
thread_id=thread_id,
text="hi",
formatted={"type": "root", "children": []},
raw=None,
author=Author(user_id="U1", user_name="alice", full_name="Alice", is_bot=False, is_me=False),
# Fixed timestamp — `datetime.now()` makes tests non-deterministic.
metadata=MessageMetadata(date_sent=datetime(2024, 1, 1, tzinfo=timezone.utc), edited=False),
)
thread = chat.thread(thread_id, current_message=msg)
assert thread._current_message is msg

def test_omitting_current_message_stubs_a_placeholder(self):
"""Workers that only post (no streaming) can omit ``current_message``."""
chat = _make_chat("slack")
thread = chat.thread("slack:C123:1234567890.123456")
assert thread._current_message is not None
assert thread._current_message.id == ""

def test_reuses_parent_chat_state_and_history(self):
"""The factory must bind the new Thread to the parent Chat's state
adapter and message history. This is the core contract of
`chat.thread()` — worker processes reconstruct a Thread that
shares state with the original Chat instance, not a fresh
detached thread. Without this, state writes/reads from the worker
wouldn't be visible to the Chat in-process.

For adapters that don't persist message history (persist_message_history
falsy), `_create_thread` intentionally skips the history bind —
there's no cache to share. This test uses a persisting adapter to
exercise the positive case.
"""
chat = _make_chat("slack")
# Opt the mock adapter into message history so the factory binds it
adapter = chat._adapters["slack"]
adapter.persist_message_history = True

thread = chat.thread("slack:C123:1234567890.123456")
# State adapter must be the exact same instance, not a copy
assert thread._state_adapter is chat._state_adapter
# Message history must be the exact same instance too
assert thread._message_history is chat._message_history

def test_omits_history_when_adapter_does_not_persist(self):
"""Adapters with `persist_message_history` falsy opt out of the
shared history cache. `Chat.thread()` respects that: the Thread
gets `None` for history rather than a shared cache the adapter
won't populate.

Explicitly set `persist_message_history = None` rather than
relying on the mock's default — prevents silent test regression
if the mock default ever changes.
"""
chat = _make_chat("slack")
adapter = chat._adapters["slack"]
adapter.persist_message_history = None

thread = chat.thread("slack:C123:1234567890.123456")
assert thread._state_adapter is chat._state_adapter
assert thread._message_history is None

def test_invalid_thread_id_raises(self):
from chat_sdk.errors import ChatError

chat = _make_chat("slack")
with pytest.raises(ChatError, match="Invalid thread ID"):
chat.thread("no-colon-here")

def test_empty_remainder_raises(self):
"""IDs where the channel segment is empty — even when later
segments are populated — would create a thread with no channel
context. Surface the error at construction time rather than
relying on adapter-specific `channel_id_from_thread_id` (which
may legitimately return empty, raise, or pass through the
original input depending on adapter).

Crucial cases: `slack::thread` and `slack::foo:bar` have an
empty channel segment but non-empty later segments. An "any
segment non-empty" check would accept these; the structural
validation checks the channel segment specifically.
"""
from chat_sdk.errors import ChatError

chat = _make_chat("slack")
bad_ids = (
"slack:",
"slack::",
"slack:::",
"slack::::",
"slack::thread", # empty channel, non-empty thread
"slack::foo:bar", # empty channel, multi-segment rest
)
for bad in bad_ids:
with pytest.raises(ChatError, match="Invalid thread ID"):
chat.thread(bad)

def test_unregistered_adapter_raises(self):
from chat_sdk.errors import ChatError

chat = _make_chat("slack")
with pytest.raises(ChatError, match='Adapter "teams" not found'):
chat.thread("teams:foo:bar")
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Loading
Loading