Skip to content

Commit 0170d85

Browse files
fix(cards): align LinkButton stable-id param to id (not id_), matching Button/Select
The Wave-A Slack PR's rebase onto #153 surfaced an API inconsistency: #153 landed LinkButton's optional stable-id param as `id_`, but Button/Select/RadioSelect all use `id`. Align LinkButton to the `id` convention — it's keyword-only with no real builtin-shadow issue (Button already does this; ruff's lint config does not select flake8-builtins), and 0.4.31 is unreleased so nothing depends on `id_` yet. Also drops this branch's duplicate jsx-runtime non-parity row (keeps #153's canonical one) and its redundant core LinkButton tests (keeps #153's TestLinkButtonId).
1 parent 30c6842 commit 0170d85

3 files changed

Lines changed: 13 additions & 43 deletions

File tree

docs/UPSTREAM_SYNC.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -646,14 +646,12 @@ stay explicit instead of being rediscovered in code review.
646646
| `@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). |
647647
| 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. |
648648
| 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. |
649-
| 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. |
649+
| 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). |
650650
| 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. |
651651
| 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. |
652652
| 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`. |
653653
| 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. |
654654
| 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`. |
655-
| jsx-runtime `LinkButtonProps.id` prop (vercel/chat#171657a slice, chat@4.31) | Not ported | `LinkButtonProps` gains `id?: string`; `resolveJSXElement` forwards it to `LinkButton({ id })` | Covered by the existing "JSX Card/Modal elements" row — Python has no JSX runtime. The underlying capability ports cleanly: the `LinkButton()` builder accepts `id=` directly, the core `LinkButtonElement` carries `id`, and the Slack adapter consumes it as the link-button `action_id` (with `??` fallback to `link-{url}`). |
656-
657655
### Platform-specific gaps
658656

659657
| Area | Python | TS | Rationale |

src/chat_sdk/cards.py

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -290,22 +290,23 @@ def LinkButton(
290290
url: str,
291291
label: str,
292292
style: ButtonStyle | None = None,
293-
id_: str | None = None,
293+
id: str | None = None,
294294
) -> LinkButtonElement:
295295
"""Create a LinkButton element that opens a URL when clicked.
296296
297297
Example::
298298
299299
LinkButton(url="https://example.com", label="View Docs")
300300
301-
``id_`` is an optional action identifier emitted by platforms that report
302-
link clicks. Upstream sets ``id`` unconditionally and relies on
303-
``JSON.stringify`` dropping ``undefined``; in Python we only write the
304-
key when it is provided so an unset id never serializes as ``null``.
301+
``id`` is an optional action identifier emitted by platforms that report
302+
link clicks (matching the ``Button``/``Select`` ``id`` convention). Upstream
303+
sets ``id`` unconditionally and relies on ``JSON.stringify`` dropping
304+
``undefined``; in Python we only write the key when it is provided so an
305+
unset id never serializes as ``null``.
305306
"""
306307
element: LinkButtonElement = {"type": "link-button", "url": url, "label": label}
307-
if id_ is not None:
308-
element["id"] = id_
308+
if id is not None:
309+
element["id"] = id
309310
if style is not None:
310311
element["style"] = style
311312
return element

tests/test_cards.py

Lines changed: 4 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -14,35 +14,6 @@
1414
from chat_sdk.shared.card_utils import escape_table_cell, render_gfm_table
1515

1616

17-
class TestLinkButtonFactory:
18-
"""``LinkButton`` factory — including the optional ``id`` (chat@4.31)."""
19-
20-
def test_minimal(self):
21-
assert LinkButton(url="https://example.com", label="Open") == {
22-
"type": "link-button",
23-
"url": "https://example.com",
24-
"label": "Open",
25-
}
26-
27-
def test_includes_id_when_provided(self):
28-
# chat@4.31: ``id`` is an optional action identifier emitted by
29-
# platforms that report link clicks.
30-
assert LinkButton(url="https://example.com", label="Open", id="open-btn") == {
31-
"type": "link-button",
32-
"url": "https://example.com",
33-
"label": "Open",
34-
"id": "open-btn",
35-
}
36-
37-
def test_omits_id_key_when_none(self):
38-
# Absent id must NOT add an ``id`` key (so adapter ``??`` fallbacks fire).
39-
assert "id" not in LinkButton(url="https://example.com", label="Open")
40-
41-
def test_empty_string_id_is_kept(self):
42-
# An explicit empty-string id is kept (locks ``is not None``, not truthy).
43-
assert LinkButton(url="https://example.com", label="Open", id="")["id"] == ""
44-
45-
4617
class TestIsCardElement:
4718
"""Tests for is_card_element."""
4819

@@ -253,7 +224,7 @@ class TestLinkButtonId:
253224
"""
254225

255226
def test_id_written_when_provided(self):
256-
btn = LinkButton(url="https://example.com/docs", label="Docs", id_="open-docs")
227+
btn = LinkButton(url="https://example.com/docs", label="Docs", id="open-docs")
257228
assert btn["id"] == "open-docs"
258229

259230
def test_no_id_key_when_omitted(self):
@@ -264,13 +235,13 @@ def test_no_id_key_when_omitted(self):
264235

265236
def test_empty_string_id_is_emitted(self):
266237
# Explicit empty string is distinct from unset and must survive
267-
# (this is exactly why we use ``is not None`` and not ``id_ or ...``).
268-
btn = LinkButton(url="https://example.com/docs", label="Docs", id_="")
238+
# (this is exactly why we use ``is not None`` and not ``id or ...``).
239+
btn = LinkButton(url="https://example.com/docs", label="Docs", id="")
269240
assert "id" in btn
270241
assert btn["id"] == ""
271242

272243
def test_id_survives_wire_serialization(self):
273-
btn = LinkButton(url="https://example.com/docs", label="Docs", id_="open-docs")
244+
btn = LinkButton(url="https://example.com/docs", label="Docs", id="open-docs")
274245
round_tripped = json.loads(json.dumps(btn))
275246
assert round_tripped["id"] == "open-docs"
276247
assert round_tripped["type"] == "link-button"

0 commit comments

Comments
 (0)