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
4 changes: 2 additions & 2 deletions docs/UPSTREAM_SYNC.md
Original file line number Diff line number Diff line change
Expand Up @@ -646,12 +646,12 @@ stay explicit instead of being rediscovered in code review.
| `@chat-adapter/tests` adapter test kit (vercel/chat#470) | Not ported | New TS package with test utilities for adapter authors | Python already ships `chat_sdk.testing` (`MockAdapter`, `MockStateAdapter`, `create_test_message()`) covering the same surface for this repo's adapter tests; mirroring the TS kit verbatim would duplicate it. Revisit if upstream's kit grows capabilities ours lacks (e.g. recorded replay fixtures for third-party adapter authors). |
| Teams modal-submit webhook options (vercel/chat#454 adapter-teams slice) | Not ported — the Python Teams adapter has no task-module/modal-submit flow (`handleTaskSubmit`/`processModalSubmit` are absent), so upstream's change passing `bridgeAdapter.getWebhookOptions(activity.id)` into `processModalSubmit` has no landing site | `TeamsAdapter.handleTaskSubmit` forwards webhook options so modal callbackUrl POSTs are registered with `waitUntil` | Pre-existing gap: Teams modals are unported. The Slack adapter already forwards options to `process_modal_submit`, so the new waitUntil plumbing is exercised there. Add the Teams call when Teams modal support lands. |
| jsx-runtime `callbackUrl` props (vercel/chat#454 slice) | Not ported | `ButtonProps`/`ModalProps` gain `callbackUrl`; `resolveJSXElement` forwards it | Covered by the existing "JSX Card/Modal elements" row — Python has no JSX runtime; `Button()`/`Modal()` builders accept `callback_url` directly. |
| jsx-runtime `id` prop for link buttons (stable-id-for-link-buttons, chat@4.31.0 commit `171657a`) | Not ported | `LinkButtonProps` gains `id?`; `resolveJSXElement` forwards `id: props.id` | Covered by the existing "JSX Card/Modal elements" row — Python has no JSX runtime. The core half of the same commit (`LinkButton()` factory + `LinkButtonElement` `id`) **is** ported: the `LinkButton(id_=…)` builder accepts the optional stable identifier directly. |
| jsx-runtime `id` prop for link buttons (stable-id-for-link-buttons, chat@4.31.0 commit `171657a`) | Not ported | `LinkButtonProps` gains `id?`; `resolveJSXElement` forwards `id: props.id` | Covered by the existing "JSX Card/Modal elements" row — Python has no JSX runtime. The core half of the same commit (`LinkButton()` factory + `LinkButtonElement` `id`) **is** ported: the `LinkButton(id=…)` builder accepts the optional stable identifier directly (matching the `Button`/`Select` `id` convention). |
| Transcripts API Python adaptations (vercel/chat#448) | `transcripts.delete()` returns a `DeleteResult` dataclass; misconfiguration raises `ValueError` (constructor/`AppendInput` guards, invalid duration) or `ChatError` (`chat.transcripts` accessor); guard messages name the Python kwarg (`options.user_key`); `DurationString` is a `str` alias validated at runtime by `_parse_duration` | Inline `{ deleted: number }`; generic `Error` for all of the above; template-literal `` `${number}${"s"\|"m"\|"h"\|"d"}` `` type | Port rules: typed dataclasses over raw dicts; repo error-type conventions (constructor misconfig → `ValueError`, runtime API misuse → `ChatError`) with upstream-matching message wording; Python has no template-literal types. Same shapes and values throughout. |
| Slack legacy mrkdwn renderer (response_url surface only, post-#440) | `_node_to_mrkdwn` renders headings as `*bold*` and images as `{alt} ({url})` / bare URL | TS `nodeToMrkdwn` has no heading/image branches — both fall through to `defaultNodeToText`, dropping heading emphasis and image URLs | Pre-existing Python improvement; after vercel/chat#440 it affects only `to_response_url_text` (ephemeral edits via response_url). Preserves visual hierarchy and image URLs Slack would otherwise lose. |
| Slack `api` primitives `send_slack_response_url` URL gate (vercel/chat#548) | `send_slack_response_url` (`slack/api/__init__.py`) calls `_assert_slack_response_url(url)` before POSTing — requires an `https://*.slack.com` URL (where Slack-issued `response_url`s always live) and raises `ValueError` for anything else | Upstream `api/client.ts` `sendResponseUrl` POSTs to whatever `response_url` it is handed, with no scheme/host validation | SSRF guard. The `response_url` reaching this primitive can originate from a parsed-but-unverified interaction payload; without a gate a crafted value could redirect the POST (which carries no bearer token but does echo SDK-controlled message content and trigger an arbitrary outbound request) to an attacker host. Enforces `CLAUDE.md`'s "Validate external URLs before requests (SSRF)" rule, mirroring the high-level adapter's `rehydrate_attachment` allowlist row above. Allowlist: scheme `https`, host `slack.com` or `*.slack.com`. |
| Slack `api` primitives `fetch_slack_file` host allowlist (vercel/chat#548) | `fetch_slack_file` (`slack/api/__init__.py`) gates `url` through `is_trusted_slack_file_url` before forwarding the bot token, raising `ValueError` for untrusted hosts | Upstream `api/client.ts` `fetchFile` GETs the supplied URL with `Authorization: Bearer <token>` unconditionally | Token-leak guard. `fetch_slack_file` attaches the workspace bot token; a crafted `url_private` from a parsed file object could otherwise exfiltrate that token to an arbitrary host. `is_trusted_slack_file_url` requires scheme `https` and host in `{files.slack.com, slack.com, *.slack.com, *.slack-edge.com}` — the same allowlist the high-level adapter's `rehydrate_attachment` row uses for Slack. Enforces `CLAUDE.md`'s SSRF/URL-validation rule. |

| Slack `web_client_options` → slack_sdk `WebClient` kwargs (vercel/chat#8336a3e, chat@4.31) | `SlackAdapterConfig.web_client_options: dict[str, Any] \| None` is spread (gated on `is not None`, so an explicit `{}` still spreads as a no-op) into **both** WebClient construction sites — the default `AsyncWebClient` (`_get_client`) and the per-token sync `WebClient` (`_get_web_client_for_token`). The keys are **slack_sdk** `WebClient` constructor kwargs: `timeout` (int seconds), `retry_handlers` (a list of `slack_sdk.http_retry.RetryHandler`), `headers`, etc. Any nested `headers` dict is **deep-copied per client** (`_web_client_kwargs`) so cached per-token clients never share a mutable dict and the caller's input is never mutated. | Upstream `webClientOptions?: Omit<WebClientOptions, "slackApiUrl">` forwards to `@slack/web-api`'s axios-backed `WebClient`; its headline keys are `retryConfig` (a `retryPolicies.*` policy), `rejectRateLimitedCalls`, and `timeout` (ms). | No 1:1 mapping: `slack_sdk` has no `retryConfig`/`rejectRateLimitedCalls` (retry behavior is configured via `retry_handlers`) and its `timeout` is seconds, not ms. So the option bag maps to slack_sdk `WebClient` kwargs rather than axios options. Same intent (tune the underlying HTTP client the adapter doesn't otherwise expose) and same per-client header isolation. Documented inline in `slack/types.py` (`web_client_options` docstring) and `slack/adapter.py` (`_web_client_kwargs`). Regression coverage: `tests/test_adapter_api_url_config.py::TestSlackWebClientOptions`. |
### Platform-specific gaps

| Area | Python | TS | Rationale |
Expand Down
29 changes: 27 additions & 2 deletions src/chat_sdk/adapters/slack/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,31 @@ def __init__(self, config: SlackAdapterConfig | None = None) -> None:
# synchronous ``web_client`` escape hatch.
self._slack_api_url: str | None = (config.api_url or os.environ.get("SLACK_API_URL")) or None

# Extra kwargs forwarded to every slack_sdk client (default async,
# per-token async cache, and synchronous WebClient). Mirrors upstream
# ``this.webClientOptions = config.webClientOptions`` (vercel/chat
# 8336a3e). Stored as-is (may be ``None``); spread is gated on
# ``is not None`` so an explicit ``{}`` still spreads. See
# ``_web_client_kwargs`` for the per-client deep-copy of ``headers``.
self._web_client_options: dict[str, Any] | None = config.web_client_options

def _web_client_kwargs(self) -> dict[str, Any]:
"""Return a fresh copy of ``web_client_options`` for one client.

Spreads the configured options (gated on ``is not None`` so an empty
``{}`` still applies) and **deep-copies any nested ``headers`` dict**,
so cached per-token clients never share a mutable ``headers`` object
and the caller's input dict is never mutated. Mirrors upstream's
per-client ``{ ...headers ? { headers: { ...headers } } : {} }`` spread.
"""
if self._web_client_options is None:
return {}
kwargs = dict(self._web_client_options)
headers = self._web_client_options.get("headers")
if headers is not None:
kwargs["headers"] = dict(headers)
return kwargs

# ------------------------------------------------------------------
# Properties (Adapter protocol)
# ------------------------------------------------------------------
Expand Down Expand Up @@ -868,7 +893,7 @@ def _get_client(self, token: str | None = None) -> Any:
# rejects ``base_url=None`` (it requires a string), and an empty string
# must fall back to the built-in default, so mirroring upstream's truthy
# spread keeps the default otherwise.
client_kwargs: dict[str, Any] = {"token": resolved_token}
client_kwargs: dict[str, Any] = {**self._web_client_kwargs(), "token": resolved_token}
if self._slack_api_url:
client_kwargs["base_url"] = self._slack_api_url
client = AsyncWebClient(**client_kwargs)
Expand Down Expand Up @@ -916,7 +941,7 @@ def _get_web_client_for_token(self, token: str) -> Any:

# Same truthy ``base_url`` rule as ``_get_client`` — pass the
# override only when truthy so slack_sdk keeps its default.
web_client_kwargs: dict[str, Any] = {"token": token}
web_client_kwargs: dict[str, Any] = {**self._web_client_kwargs(), "token": token}
if self._slack_api_url:
web_client_kwargs["base_url"] = self._slack_api_url
client = WebClient(**web_client_kwargs)
Expand Down
6 changes: 5 additions & 1 deletion src/chat_sdk/adapters/slack/blocks/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,9 +311,13 @@ def _button_to_element(button: SlackButtonElement, convert_emoji: _EmojiConverte


def _link_button_to_element(button: SlackLinkButtonElement, convert_emoji: _EmojiConverter) -> dict[str, object]:
# `??` semantics: an explicit (even empty-string) id is used verbatim;
# only a missing/None id falls back to the URL-derived action_id.
button_id = button.get("id")
action_id = button_id if button_id is not None else f"link-{button['url']}"
return _compact(
{
"action_id": _truncate_text(f"link-{button['url']}", LIMITS.action_id),
"action_id": _truncate_text(action_id, LIMITS.action_id),
"style": _map_button_style(button.get("style")),
"text": _plain_text(button["label"], convert_emoji, LIMITS.button_text),
"type": "button",
Expand Down
1 change: 1 addition & 0 deletions src/chat_sdk/adapters/slack/blocks/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class SlackLinkButtonElement(TypedDict):
type: Literal["link-button"]
label: str
url: str
id: NotRequired[str]
style: NotRequired[SlackButtonStyle]


Expand Down
5 changes: 4 additions & 1 deletion src/chat_sdk/adapters/slack/cards.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,14 +229,17 @@ def _convert_button_to_element(button: ButtonElement) -> SlackButtonElement_:
def _convert_link_button_to_element(button: LinkButtonElement) -> SlackLinkButtonElement_:
"""Convert a LinkButtonElement to a Slack link button element."""
url = button.get("url", "")
# `??` semantics: an explicit (even empty-string) id is used verbatim;
# only a missing/None id falls back to the URL-derived action_id.
button_id = button.get("id")
element: SlackLinkButtonElement_ = {
"type": "button",
"text": {
"type": "plain_text",
"text": convert_emoji(button.get("label", "")),
"emoji": True,
},
"action_id": f"link-{url[:200]}",
"action_id": button_id if button_id is not None else f"link-{url[:200]}",
"url": url,
Comment on lines +232 to 243

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Truncate explicit id before assigning Slack action_id.

Line 242 applies truncation only in the URL-fallback branch; a provided id is used unbounded. Very long ids can exceed Slack’s action_id limit and fail at runtime. Please enforce one shared max-length path for both explicit-id and fallback-id generation.

Suggested fix
 def _convert_link_button_to_element(button: LinkButtonElement) -> SlackLinkButtonElement_:
@@
-    button_id = button.get("id")
+    button_id = button.get("id")
+    action_id = (button_id if button_id is not None else f"link-{url}")[:255]
     element: SlackLinkButtonElement_ = {
@@
-        "action_id": button_id if button_id is not None else f"link-{url[:200]}",
+        "action_id": action_id,
         "url": url,
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/chat_sdk/adapters/slack/cards.py` around lines 232 - 243, The action_id
assignment in the SlackLinkButtonElement_ dictionary does not enforce a
consistent maximum length limit for both the explicit button_id and the
URL-fallback cases. Currently, only the URL-fallback branch applies truncation
with url[:200], while an explicitly provided button_id is used unbounded, which
can exceed Slack's action_id character limit. Apply the same truncation limit to
both branches by modifying the conditional expression that assigns action_id to
ensure that both button_id (when not None) and the fallback URL-based id are
truncated to the same maximum character length.

}

Expand Down
45 changes: 40 additions & 5 deletions src/chat_sdk/adapters/slack/format_converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,44 @@
)

# Match bare @mentions (e.g. "@george") to rewrite as Slack's `<@george>`.
# The lookbehind excludes `<` (already-formatted mentions like `<@U123>`) and
# any word character, so email addresses like `user@example.com` are left alone.
BARE_MENTION_REGEX = re.compile(r"(?<![<\w])@(\w+)")
# The fixed-width lookbehind excludes:
# - `<` (already-formatted mentions like `<@U123>`),
# - any word character (so email addresses like `user@example.com` are left
# alone), and
# - `/` (so a schemeless host path like `mastodon.social/@user` — which the
# URL matcher in `_link_bare_mentions_outside_urls` does NOT catch — is also
# preserved). Mirrors upstream's `(?<![<\w/])` (vercel/chat a8bf99a).
# ``re.ASCII`` keeps `\w` ASCII-only to match upstream's JS ASCII `\w` exactly:
# Python's `\w` is Unicode-aware by default, which would otherwise diverge on
# non-ASCII handles/boundaries.
BARE_MENTION_REGEX = re.compile(r"(?<![<\w/])@(\w+)", re.ASCII)

# Match an `http(s)` URL up to the first whitespace or angle bracket so the span
# can be excluded from mention linking. A bare `@handle` anywhere inside a URL
# (path, query string, or fragment) must NOT be rewritten into a `<@handle>`
# Slack mention, which would corrupt the link. Mirrors upstream's `URL_PATTERN`.
URL_REGEX = re.compile(r"\bhttps?://[^\s<>]+")


def _link_bare_mentions_outside_urls(text: str) -> str:
"""Rewrite bare ``@handle`` mentions, but only outside ``http(s)`` URL spans.

A bare ``@handle`` anywhere inside a URL (path, query string, or fragment)
is preserved verbatim; mention linking is applied only to the text slices
between (and after) URL spans. Mirrors upstream's
``linkBareMentionsOutsideUrls`` (vercel/chat a8bf99a). The schemeless host
path case (``mastodon.social/@user``), which ``URL_REGEX`` does not match,
is additionally guarded by the ``/`` in ``BARE_MENTION_REGEX``'s lookbehind.
"""
result: list[str] = []
last_index = 0
for match in URL_REGEX.finditer(text):
start = match.start()
result.append(BARE_MENTION_REGEX.sub(r"<@\1>", text[last_index:start]))
result.append(match.group(0))
last_index = match.end()
result.append(BARE_MENTION_REGEX.sub(r"<@\1>", text[last_index:]))
return "".join(result)


class SlackFormatConverter(BaseFormatConverter):
Expand Down Expand Up @@ -171,7 +206,7 @@ def extract_plain_text(self, platform_text: str) -> str:

def _finalize(self, text: str) -> str:
"""Rewrite bare @mentions and normalize emoji placeholders for Slack."""
return convert_emoji_placeholders(BARE_MENTION_REGEX.sub(r"<@\1>", text), "slack")
return convert_emoji_placeholders(_link_bare_mentions_outside_urls(text), "slack")

def _ast_to_mrkdwn(self, ast: Root) -> str:
"""Render an AST to Slack's legacy mrkdwn (response_url surface only)."""
Expand All @@ -190,7 +225,7 @@ def _node_to_mrkdwn(self, node: Content) -> str:

if node_type == "text":
value = node.get("value", "")
return BARE_MENTION_REGEX.sub(r"<@\1>", value)
return _link_bare_mentions_outside_urls(value)

if node_type == "strong":
content = "".join(self._node_to_mrkdwn(c) for c in children)
Expand Down
17 changes: 17 additions & 0 deletions src/chat_sdk/adapters/slack/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,23 @@ class SlackAdapterConfig:
# ``slackApiUrl`` (vercel/chat 6b17c60). Useful for proxies, Slack-API
# mocks in tests, or Enterprise-routed deployments.
api_url: str | None = None
# Extra keyword arguments forwarded to every ``slack_sdk`` client the
# adapter builds — the default ``AsyncWebClient``, the per-token async
# cache, and the synchronous ``WebClient`` escape hatch. Gated on
# ``is not None`` so an explicit empty ``{}`` still spreads (a no-op).
#
# DIVERGENCE (vercel/chat 8336a3e): upstream forwards ``webClientOptions``
# to ``@slack/web-api``'s ``WebClient`` (an axios-backed client), so its
# most useful keys are ``retryConfig`` and ``timeout``. There is no 1:1
# mapping in ``slack_sdk``: it has no ``retryConfig``/``rejectRateLimitedCalls``
# — retry behavior is configured via ``retry_handlers`` (a list of
# ``slack_sdk.http_retry.RetryHandler``), and ``timeout`` is an int of
# seconds. So these map to **slack_sdk WebClient kwargs**, not axios options.
# Example: ``web_client_options={"timeout": 15, "retry_handlers": [...]}``.
# ``headers`` (a dict) is deep-copied per client so cached per-token clients
# never share a mutable dict and caller input is never mutated. See the
# ``webClientOptions`` divergence row in docs/UPSTREAM_SYNC.md.
web_client_options: dict[str, Any] | None = None

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Append new config fields to preserve positional callers

Because SlackAdapterConfig is a regular dataclass rather than kw_only, inserting this field before the existing app_token/bot_token fields changes the generated positional __init__ signature. Any existing code that passed positional config values, e.g. SlackAdapterConfig(None, "xapp-...", "xoxb-..."), now stores the app token string in web_client_options and shifts the remaining values, which can leave the adapter misconfigured or later fail in _web_client_kwargs when it treats that string as a dict. Appending the new option after the existing fields preserves the public positional ABI.

Useful? React with 👍 / 👎.

# App-level token (xapp-...). Required when ``mode == "socket"``.
app_token: str | None = None
# Bot token (xoxb-...). Required for single-workspace mode. Omit for multi-workspace.
Expand Down
15 changes: 8 additions & 7 deletions src/chat_sdk/cards.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,22 +290,23 @@ def LinkButton(
url: str,
label: str,
style: ButtonStyle | None = None,
id_: str | None = None,
id: str | None = None,
) -> LinkButtonElement:
"""Create a LinkButton element that opens a URL when clicked.

Example::

LinkButton(url="https://example.com", label="View Docs")

``id_`` is an optional action identifier emitted by platforms that report
link clicks. Upstream sets ``id`` unconditionally and relies on
``JSON.stringify`` dropping ``undefined``; in Python we only write the
key when it is provided so an unset id never serializes as ``null``.
``id`` is an optional action identifier emitted by platforms that report
link clicks (matching the ``Button``/``Select`` ``id`` convention). Upstream
sets ``id`` unconditionally and relies on ``JSON.stringify`` dropping
``undefined``; in Python we only write the key when it is provided so an
unset id never serializes as ``null``.
"""
element: LinkButtonElement = {"type": "link-button", "url": url, "label": label}
if id_ is not None:
element["id"] = id_
if id is not None:
element["id"] = id
if style is not None:
element["style"] = style
return element
Expand Down
Loading
Loading