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
1 change: 1 addition & 0 deletions docs/UPSTREAM_SYNC.md
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,7 @@ 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 (non-Teams adapters) | `_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. This divergence does not apply to Teams: Teams DMs stream natively through the SDK `IStreamer` (`_stream_via_emit`), and a non-cancel iterator exception propagates straight to the caller while the SDK closes the streamer after the handler returns. |
| Slack `stream()` to a top-level DM (empty `thread_ts`) | Normalizes the empty `thread_ts` to `None` and degrades to a single accumulated `post_message` call so the streamed reply still lands (chat-sdk-python#94) | Passes the empty `thread_ts` straight to `chat.startStream` (`adapter-slack/src/index.ts` `stream()`), which Slack rejects (`invalid_thread_ts`) — the streamed DM reply is silently dropped | Top-level DM messages intentionally encode `threadTs=""` on both sides (`_handle_message_event` / `handleMessageEvent`, "matches openDM subscriptions") — that part is faithful to upstream and **not** a bug. The bug is that upstream's `stream()` never reconciled that legitimate value with `startStream`'s requirement for a non-empty `thread_ts`; `postMessage` accepts no `thread_ts` for DMs, so we degrade instead of erroring. Tracked for contribution upstream — remove this divergence once vercel/chat fixes `stream()` to handle empty-`thread_ts` DM thread ids. |
| Slack `stream()` on Enterprise Grid (`chat.startStream` `team_not_found`) | Threads the workspace `team_id` into `client.chat_stream(...)` (= `options.recipient_team_id`, the `team.id` extracted on the inbound path), which slack_sdk forwards into `_stream_args` → `chat.startStream`. `chat.appendStream`/`chat.stopStream` don't receive `_stream_args` and don't need `team_id`. Harmless on non-Grid workspaces — a correct `team_id` is always valid. (chat-sdk-python#95) | Builds the `chat.startStream` args from `channel`/`threadTs`/`recipientUserId`/`recipientTeamId`/`taskDisplayMode` only (`adapter-slack/src/index.ts` `stream()`); never passes a workspace `team_id`. On Grid orgs `chat.startStream` then fails with `team_not_found` (the per-workspace bot token alone isn't sufficient to disambiguate the team), even though `chat.postMessage` on the same workspace succeeds without it. | Upstream has the same gap — its `stream()` never threads `team_id`, so streaming is broken on Grid while non-streaming posts work. `chat.startStream` requires `team_id` for Grid disambiguation; `chat.postMessage` does not, which is why only streaming regresses. We source `team_id` from the already-plumbed `recipient_team_id` (the workspace where the interaction happened = the streaming target workspace). Live verification needs a real Grid workspace; the unit regression (`tests/test_slack_api.py::TestStream::test_stream_threads_team_id_to_chat_stream_for_grid` + the `team_not_found` mutation guard) simulates Grid by raising `team_not_found` from the streamer's lazy `chat.startStream` when `team_id` is absent. Tracked for contribution upstream. |
| Fallback streaming final SentMessage content (non-Teams adapters) | 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 DMs stream through the SDK `IStreamer` and the Teams accumulate-and-post path ships raw `accumulated` via `post_message`, matching upstream; this divergence applies only to the remaining adapters that still route through `_fallback_stream`. |
| Teams group-chat / channel streaming via accumulate-and-post | `TeamsAdapter.stream` accumulates the full text and issues a single `post_message` (SDK-backed) instead of post+edit, even for group chats and channel threads | Same (`@chat-adapter/teams@4.30.0`: `if (activeStream && !activeStream.canceled) … else { accumulate; postMessage }`) — no divergence at the adapter level | Documented for clarity: the Python port matches upstream's behavior of avoiding the post+edit flicker where Teams doesn't support native streaming. The buffered fallback routes through the same SDK `App.send` path as a normal `post_message`. |
| Teams native streaming via the SDK `IStreamer` (DMs) | `TeamsAdapter._handle_message_activity` captures a Teams SDK `IStreamer` (`microsoft_teams.apps.StreamerProtocol` / `HttpStream`) for DMs via `app.activity_sender.create_stream(ref)`, registers it in `_active_streams`, and `await`s a `processing_done` gate (a wrapped `wait_until` shim) so the streamer stays alive while the handler streams. `stream()` → `_stream_via_emit` calls `stream.emit(text)` per chunk and NEVER calls `close()`; the adapter's `_handle_message_activity` `finally` calls `stream.close()` once (the lifecycle-owner role the SDK App's `process_activity` plays upstream). | `@chat-adapter/teams@4.30.0` `index.ts` does exactly this: `this.activeStreams.set(threadId, ctx.stream)`, build `processingDone` + wrapped `waitUntil`, `await processingDone`, `streamViaEmit` calls `stream.emit(text)` and never `close()` (the SDK App auto-closes after the handler returns). | **No adapter-level divergence.** The only mechanical difference is the close call site: upstream lets the SDK `App` auto-close `ctx.stream` because the SDK owns dispatch; our bridge overrides `server.on_request`, so we own dispatch and reproduce the close in `_handle_message_activity`'s `finally`. The SDK `HttpStream.close` no-ops when the stream was canceled or had no content, so closing in both success and cancel paths is safe (matching the SDK App, which closes in both its success and `StreamCancelledError` branches). Cancellation is detected via `stream.canceled` (checked before each emit) and by catching `StreamCancelledError` (other exceptions re-raise). The first chunk id is captured via `on_chunk` and awaited only when text was emitted and the stream was not canceled. Replaces the prior hand-rolled wire format, the 1500ms emit throttle, and the `RawMessage.text` / `update_interval_ms` divergences (all unwound in #93 PR 3). |
Expand Down
4 changes: 2 additions & 2 deletions scripts/fidelity_baseline.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"_comment": "Ratchet-down baseline for scripts/verify_test_fidelity.py. This repo ships at strict fidelity for mapped core files (0 missing) against chat@4.30.0, so the baseline is empty. Scope: the MAPPING dict in scripts/verify_test_fidelity.py is the authoritative list of TS files checked; it currently covers 13 of the packages/chat/src/*.test.ts files. Default CI mode runs --strict via .github/workflows/lint.yml; this file is retained for local workflows that want to opt back into baseline mode (e.g. during an upstream sync where several ports land in flight). To baseline genuinely-divergent tests, run scripts/verify_test_fidelity.py --update-baseline after documenting the divergence in docs/UPSTREAM_SYNC.md.",
"ts_parity": "chat@4.30.0",
"_comment": "Ratchet-down baseline for scripts/verify_test_fidelity.py. This repo ships at strict fidelity for mapped core files (0 missing) against chat@4.31.0, so the baseline is empty. Scope: the MAPPING dict in scripts/verify_test_fidelity.py is the authoritative list of TS files checked; it currently covers 13 of the packages/chat/src/*.test.ts files. Default CI mode runs --strict via .github/workflows/lint.yml; this file is retained for local workflows that want to opt back into baseline mode (e.g. during an upstream sync where several ports land in flight). To baseline genuinely-divergent tests, run scripts/verify_test_fidelity.py --update-baseline after documenting the divergence in docs/UPSTREAM_SYNC.md.",
"ts_parity": "chat@4.31.0",
"total_ts_tests": 732,
"total_missing": 0,
"missing": {}
Expand Down
16 changes: 16 additions & 0 deletions src/chat_sdk/adapters/slack/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -3926,6 +3926,22 @@ async def stream(
"thread_ts": thread_ts,
"recipient_user_id": options.recipient_user_id,
"recipient_team_id": options.recipient_team_id,
# Enterprise Grid disambiguation (chat-sdk-python#95). On Grid
# orgs ``chat.startStream`` fails with ``team_not_found`` unless
# the workspace ``team_id`` is supplied explicitly — the
# per-workspace bot token alone is not sufficient (whereas
# ``chat.postMessage`` succeeds without it). slack_sdk's
# ``chat_stream`` stashes unknown kwargs in ``_stream_args`` and
# forwards them to BOTH ``chat.startStream`` call sites (the
# eager first-flush path and the lazy stop-without-flush path),
# so passing ``team_id`` here threads it through to the only API
# call that needs it. ``chat.appendStream``/``chat.stopStream``
# do not receive ``_stream_args`` and do not require ``team_id``.
# ``recipient_team_id`` is the ``team.id`` extracted on the
# inbound path (the workspace where the interaction happened =
# the streaming target workspace). Harmless on non-Grid
# workspaces: passing the correct ``team_id`` is always valid.
"team_id": options.recipient_team_id,
}
if options.task_display_mode:
stream_kwargs["task_display_mode"] = options.task_display_mode
Expand Down
147 changes: 147 additions & 0 deletions tests/test_slack_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,25 @@ def __init__(self, data: dict[str, Any]) -> None:
self.data = data


class _FakeSlackApiError(Exception):
"""Mirror of ``slack_sdk.errors.SlackApiError`` for offline tests.

The dev-group install (``uv sync --group dev``, what CI runs) does NOT
pull in ``slack_sdk`` (it lives in the ``slack`` / ``all`` extras), and
``test_slack_client_cache.py`` injects a bare ``slack_sdk`` ModuleType
into ``sys.modules`` that has no ``errors`` submodule. So importing the
real ``SlackApiError`` is order-dependent and breaks under CI. This
stand-in reproduces the attributes the adapter inspects: ``str(error)``
contains the Slack error code and ``error.response`` is a dict carrying
``{"ok": False, "error": <code>}`` (matching ``SlackApiError``'s shape).
"""

def __init__(self, message: str, response: dict[str, Any]) -> None:
self.response = response
server_error = response.get("error")
super().__init__(f"{message}\nThe server responded with: {{'ok': False, 'error': '{server_error}'}}")


# =============================================================================
# Helpers
# =============================================================================
Expand Down Expand Up @@ -1477,6 +1496,134 @@ async def text_gen() -> AsyncIterator[str]:
assert call.kwargs["token"] == "xoxb-workspace-token"
assert mock_streamer.stop.call_args.kwargs["token"] == "xoxb-workspace-token"

@pytest.mark.asyncio
async def test_stream_threads_team_id_to_chat_stream_for_grid(self):
"""Regression for Enterprise Grid issue #95.

On Grid orgs ``chat.startStream`` fails with ``team_not_found``
unless a workspace ``team_id`` is supplied — the per-workspace bot
token alone is not sufficient (whereas ``chat.postMessage``
succeeds without it). slack_sdk's ``chat_stream`` forwards unknown
kwargs (incl. ``team_id``) to the underlying ``chat.startStream``
call, so the adapter must pass ``team_id`` = ``recipient_team_id``.

This test simulates Grid: a ``post_message`` succeeds with no
``team_id``, while the streaming start raises ``team_not_found``
when ``team_id`` is absent and only succeeds when it is present.
It MUST FAIL on pre-fix code (which omitted ``team_id``).
"""
adapter, client, _ = await _init_adapter()

# Grid-aware streamer: the lazy ``chat.startStream`` (triggered on
# the first append or on stop, exactly as the real slack_sdk
# ``AsyncChatStream`` does) raises ``team_not_found`` unless the
# workspace ``team_id`` was threaded through ``chat_stream``.
class _GridStreamer:
def __init__(self, stream_kwargs: dict[str, Any]) -> None:
self._stream_kwargs = stream_kwargs
self._started = False

def _start_stream(self) -> None:
# Mirrors slack_sdk forwarding ``_stream_args`` (which holds
# ``chat_stream``'s kwargs) to ``chat.startStream``.
if not self._stream_kwargs.get("team_id"):
raise _FakeSlackApiError(
message="team_not_found",
response={"ok": False, "error": "team_not_found"},
)
self._started = True

async def append(self, **kwargs: Any) -> dict[str, Any]:
if not self._started:
self._start_stream()
return {"ok": True}

async def stop(self, **kwargs: Any) -> dict[str, Any]:
if not self._started:
self._start_stream()
return {"ok": True, "ts": "1234567890.951951"}

captured: dict[str, Any] = {}

async def chat_stream(**kwargs: Any) -> _GridStreamer:
captured.update(kwargs)
return _GridStreamer(kwargs)

client.chat_stream = AsyncMock(side_effect=chat_stream)
client.set_response("chat_postMessage", {"ok": True, "ts": "1234567890.000111"})

# Sanity: a non-streaming post_message has NO team_id requirement on
# the same Grid workspace (it succeeds without one).
post_result = await adapter.post_message(
"slack:C_GRID:1234567890.000000",
"plain post on grid", # type: ignore[arg-type]
)
assert post_result.id == "1234567890.000111" # post_message OK, no team_id
for call in client.calls:
if call["method"] == "chat_postMessage":
assert "team_id" not in call["kwargs"]

async def text_gen() -> AsyncIterator[str]:
yield "streamed hello on grid"

result = await adapter.stream(
"slack:C_GRID:1234567890.000000",
text_gen(),
StreamOptions(recipient_user_id="U_GRID", recipient_team_id="T_GRID_WS"),
)

# The stream completed (chat.startStream did NOT raise team_not_found)
# because the workspace team_id was threaded through.
assert result.id == "1234567890.951951"
# The fix passes team_id = recipient_team_id to chat_stream.
client.chat_stream.assert_awaited_once()
assert captured["team_id"] == "T_GRID_WS"
assert captured["recipient_team_id"] == "T_GRID_WS"

@pytest.mark.asyncio
async def test_stream_raises_team_not_found_without_team_id_on_grid(self):
"""Mutation guard for issue #95: prove the Grid simulation actually
fails when ``team_id`` is missing.

This drives the same Grid-aware streamer but with ``chat_stream``
stripped of any ``team_id`` (mimicking the pre-fix code path), and
asserts the stream start raises ``team_not_found``. This anchors the
positive test above: if the fix were reverted, the streamer would
raise here, so the positive test cannot pass vacuously.
"""
adapter, client, _ = await _init_adapter()

class _GridStreamer:
async def append(self, **kwargs: Any) -> dict[str, Any]:
raise _FakeSlackApiError(
message="team_not_found",
response={"ok": False, "error": "team_not_found"},
)

async def stop(self, **kwargs: Any) -> dict[str, Any]:
raise _FakeSlackApiError(
message="team_not_found",
response={"ok": False, "error": "team_not_found"},
)

async def chat_stream(**kwargs: Any) -> _GridStreamer:
# Simulate slack_sdk dropping team_id (pre-fix behavior): the
# underlying chat.startStream then fails on Grid.
kwargs.pop("team_id", None)
return _GridStreamer()

client.chat_stream = AsyncMock(side_effect=chat_stream)

async def text_gen() -> AsyncIterator[str]:
yield "streamed hello on grid"

with pytest.raises(_FakeSlackApiError, match="team_not_found"):
await adapter.stream(
"slack:C_GRID:1234567890.000000",
text_gen(),
StreamOptions(recipient_user_id="U_GRID", recipient_team_id="T_GRID_WS"),
)


# =============================================================================
# Public request-context accessors (issue #47)
Expand Down
Loading