Skip to content

feat(slack): primitives subpaths (4.30 — vercel/chat#538,547,548,555,559)#139

Merged
patrick-chinchill merged 5 commits into
mainfrom
integ/wt30-slack
Jun 18, 2026
Merged

feat(slack): primitives subpaths (4.30 — vercel/chat#538,547,548,555,559)#139
patrick-chinchill merged 5 commits into
mainfrom
integ/wt30-slack

Conversation

@patrick-chinchill

@patrick-chinchill patrick-chinchill commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

0.4.30 wave. Cherry-picks Slack primitives modularization onto current main (6e1f87f,f2e843a,2a4731a,0b005c8): carves inline slack/adapter.py regions into self-contained slack/webhook|api|blocks|format subpackages. Behavior-preserving. No version bump (0.4.30 cut is separate). Independent review needed: correctness + upstream fidelity (chat@4.30.0) + architecture/porting + tests.

Summary by CodeRabbit

  • New Features

    • Added Slack Web API primitives for posting messages, uploading files, fetching threads, and opening modals.
    • Added Slack Block Kit conversion utilities to transform cards into structured block payloads.
    • Added Slack text formatting helpers for mentions, links, dates, and mrkdwn conversion.
  • Refactor

    • Reorganized webhook verification and request parsing into shared primitives.

claude added 4 commits June 18, 2026 12:20
Port of upstream b332a03 — adds chat_sdk.adapters.slack.webhook, a
lightweight runtime-free subpath for lower-level Slack webhook handling:
verifying Slack requests (HMAC v0 + custom verifiers), reading signed
webhook bodies, parsing Events API callbacks, slash commands, and
interactive payloads into typed dataclasses with provider-native
continuation data.

The adapter now verifies through the shared verify_slack_request /
verify_slack_signature primitives (single implementation — the inline
_verify_signature method is removed, matching upstream), reads bodies via
the shared read_slack_request_body helper, and sources
SlackWebhookVerifier from webhook/types.py. The slack package __init__ is
now lazy (PEP 562) so importing the webhook subpath does not pull in the
full adapter runtime — the Python analog of upstream's package subpath
export boundary.

Python-specific notes: verify_slack_signature is sync (no WebCrypto);
verify helpers accept a pre-read body= for non-re-readable framework
requests; now() returns epoch seconds.

https://claude.ai/code/session_013zwTcMek5rNqBTQvs2oF64
Port of upstream 4c46c26 — adds chat_sdk.adapters.slack.format, a
runtime-free subpath for Slack formatting helpers: plain_text/mrkdwn text
objects, mrkdwn escaping/unescaping, user/channel/user-group/special
mentions, links, localized date tokens, mrkdwn-to-Markdown normalization,
Markdown-bold conversion, and ID-based bare-mention linking — without the
full Slack adapter, slack_sdk, or the chat runtime.

Python-specific notes: option objects (SlackTextOptions, SlackDateOptions)
become keyword-only arguments; format_slack_date accepts datetime or an
integer unix timestamp (TS Date | number) and the TypeError message says
'datetime' instead of 'Date'.

https://claude.ai/code/session_013zwTcMek5rNqBTQvs2oF64
Port of upstream aba6aa9 (api/client.ts) and 6ed4a43 (api/extra.ts) —
adds chat_sdk.adapters.slack.api, a runtime-free subpath exposed upstream
as @chat-adapter/slack/api. Provides fetch-based primitives for calling
Slack Web API methods (call_slack_api), posting/updating/deleting messages
(post_slack_message, post_slack_ephemeral, update_slack_message,
delete_slack_message), sending interaction response_url payloads
(send_slack_response_url), uploading files through Slack's external upload
flow (upload_slack_files), fetching private Slack file URLs with bearer
auth (fetch_slack_file), fetching thread replies with cursor pagination
(fetch_slack_thread_replies), and opening modal views (open_slack_view) —
without the full Slack adapter, slack_sdk, Socket Mode, or the chat
runtime.

Importing this subpath never imports an HTTP client: the default fetch
lazily imports httpx only when a request is actually made (matching the
high-level adapter's optional-httpx pattern), and any async HTTP stack can
be injected via the fetch= parameter. SlackBotToken is declared locally
rather than imported from the adapter's types module, so the subpath stays
self-contained and runtime-free — mirroring upstream's independent
declaration in api/client.ts.

Python-specific notes: option objects become keyword-only arguments;
camelCase request fields are emitted at the Slack serialization boundary
(markdown_text, reply_broadcast, thread_ts, ...) while the API is
snake_case. Python-specific hardening (divergences, see
docs/UPSTREAM_SYNC.md Known Non-Parity): send_slack_response_url requires
an https://*.slack.com URL and fetch_slack_file requires a Slack-owned
file host before forwarding the bearer token (SSRF / token-leak guards
mirroring the high-level adapter); upstream validates neither.

Tests port api/index.test.ts and api/boundary.test.ts with injected
AsyncMock fetches (no network), plus the two divergence guards.

https://claude.ai/code/session_013zwTcMek5rNqBTQvs2oF64
Port of upstream dbd8dc5 (blocks/index.ts, types.ts, limits.ts,
errors.ts) and 6ed4a43 (blocks/input.ts) — adds
chat_sdk.adapters.slack.blocks, a runtime-free subpath exposed upstream as
@chat-adapter/slack/blocks. Converts Chat SDK-style card objects into
Slack Block Kit blocks (card_to_slack_blocks / card_to_block_kit), a
Markdown fallback (card_to_slack_fallback_text / card_to_fallback_text),
and emoji-placeholder codes (convert_slack_emoji_placeholders), enforcing
docs-backed Slack size limits (LIMITS) for headers, images, actions,
select options, fields, and tables. Also ports the generic input-request
helpers: input_request_to_slack_blocks (buttons / select / radio /
freeform), parse_slack_input_response, build_slack_freeform_view,
parse_slack_freeform_value, and answered_slack_input_blocks — without the
full Slack adapter, slack_sdk, or the chat runtime. The only cross-module
dependency is the sibling format subpath (markdown_bold_to_slack_mrkdwn),
which is itself runtime-free.

Python-specific notes: the card-input shapes (SlackCardElement and
children) are self-contained TypedDicts declared here rather than imported
from chat_sdk.cards, keeping the subpath runtime-free. Input field names
are snake_case (image_url, initial_option, request_id, allow_freeform,
selected_option_value, action_id, block_id), matching chat_sdk.cards and
the Python port convention — upstream uses camelCase. Emitted Block Kit
dicts keep Slack's API field names verbatim (alt_text, action_id,
block_id, static_select, column_settings, raw_text, private_metadata),
which is the Slack serialization boundary. Discriminated-union dispatch on
the literal type key uses per-branch casts / arg-type ignores, mirroring
the high-level adapter's cards.py.

Tests port blocks/index.test.ts and blocks/boundary.test.ts with full
output equality, plus extra coverage of the parse-None and
string-metadata paths.

https://claude.ai/code/session_013zwTcMek5rNqBTQvs2oF64
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@patrick-chinchill, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 37 minutes and 58 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 581b4a5b-24a5-49ad-b65f-670ff6fdc2e2

📥 Commits

Reviewing files that changed from the base of the PR and between 1e7b843 and 03bbea2.

📒 Files selected for processing (5)
  • docs/UPSTREAM_SYNC.md
  • src/chat_sdk/adapters/slack/webhook/__init__.py
  • src/chat_sdk/adapters/slack/webhook/parse.py
  • src/chat_sdk/adapters/slack/webhook/types.py
  • tests/test_slack_webhook_primitives.py
📝 Walkthrough

Walkthrough

Adds four new runtime-free Slack adapter subpath modules (webhook, api, blocks, format) providing typed primitives for webhook parsing/verification, Web API calls, Block Kit conversion, and text formatting. Refactors SlackAdapter to delegate body reading and HMAC signature verification to the shared webhook primitives and converts the top-level Slack __init__ to PEP 562 lazy loading. Comprehensive tests and import-boundary checks are added for each subpath.

Changes

Slack Low-Level Primitives and Adapter Refactoring

Layer / File(s) Summary
Webhook primitive types and shared utilities
src/chat_sdk/adapters/slack/webhook/types.py, src/chat_sdk/adapters/slack/webhook/utils.py, src/chat_sdk/adapters/slack/webhook/__init__.py
Defines all webhook payload dataclasses (SlackRetry, SlackContinuation, all payload variants, SlackWebhookPayload union, exception hierarchy) and shared header/retry/body-reading/JSON-parsing helpers; the __init__ re-exports the full public surface.
Webhook body parser
src/chat_sdk/adapters/slack/webhook/parse.py
Implements parse_slack_webhook_body and all private payload-specific parsers, dispatching form-urlencoded and JSON bodies to typed payload dataclasses for all Slack event, interaction, and command shapes.
Webhook signature verification
src/chat_sdk/adapters/slack/webhook/verify.py
Implements verify_slack_signature (HMAC-SHA256, constant-time compare, timestamp skew), verify_slack_request (custom verifier or signature fallback), and read_slack_webhook (verify + parse end-to-end).
Slack Web API primitives submodule
src/chat_sdk/adapters/slack/api/__init__.py
Adds the complete chat_sdk.adapters.slack.api subpath: types and dataclasses, call_slack_api, message CRUD helpers, upload_slack_files, send_slack_response_url, fetch_slack_file, fetch_slack_thread_replies, open_slack_view, SSRF/trust-URL guards, and a lazy-httpx injectable transport.
Block Kit types, limits, and errors
src/chat_sdk/adapters/slack/blocks/types.py, src/chat_sdk/adapters/slack/blocks/limits.py, src/chat_sdk/adapters/slack/blocks/errors.py
Defines all TypedDict element shapes (SlackCardElement, SlackCardChild union, Block Kit element types), LIMITS frozen dataclass with Slack-enforced character/count caps, and SlackBlockError.
Block Kit card converters and input helpers
src/chat_sdk/adapters/slack/blocks/__init__.py, src/chat_sdk/adapters/slack/blocks/input.py
Implements card_to_slack_blocks, card_to_slack_fallback_text, convert_slack_emoji_placeholders, the recursive element dispatcher, table/fields rendering with ASCII fallback, and all input_request_to_slack_blocks / freeform modal / answered-input helpers.
Slack format primitives module
src/chat_sdk/adapters/slack/format.py
Adds runtime-free helpers for mrkdwn escape/unescape, plain_text/mrkdwn object construction, mention/link/date formatting, mrkdwn-to-Markdown conversion, and bare mention linking.
Adapter lazy loading and verification delegation
src/chat_sdk/adapters/slack/__init__.py, src/chat_sdk/adapters/slack/types.py, src/chat_sdk/adapters/slack/adapter.py
Converts the top-level Slack __init__ to PEP 562 __getattr__ lazy loading, re-exports SlackWebhookVerifier from webhook.types, and refactors SlackAdapter.handle_webhook to use read_slack_request_body and verify_slack_request, removing the local _verify_signature method and hashlib import.
Primitive and adapter tests
tests/test_slack_webhook_primitives.py, tests/test_slack_api_primitives.py, tests/test_slack_blocks_primitives.py, tests/test_slack_format_primitives.py, tests/test_slack_adapter.py, tests/test_slack_dynamic_token_and_verifier.py
Adds comprehensive unit tests for all four subpath modules plus import-boundary subprocess checks; removes the deleted _verify_signature test class and updates the constant-time-compare test to inspect the shared _verify_slack_signature_value primitive.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant SlackAdapter
  participant read_slack_request_body
  participant verify_slack_request
  participant verify_slack_signature
  participant parse_slack_webhook_body

  Client->>SlackAdapter: handle_webhook(request)
  SlackAdapter->>read_slack_request_body: request
  read_slack_request_body-->>SlackAdapter: raw body string
  SlackAdapter->>verify_slack_request: request, body, signing_secret, webhook_verifier?
  alt custom webhook_verifier provided
    verify_slack_request->>webhook_verifier: body, headers
    webhook_verifier-->>verify_slack_request: verified body or falsy (raises 401)
  else HMAC path
    verify_slack_request->>verify_slack_signature: body, headers, signing_secret
    verify_slack_signature-->>verify_slack_request: OK or raises SlackWebhookVerificationError
  end
  verify_slack_request-->>SlackAdapter: verified body
  SlackAdapter->>parse_slack_webhook_body: verified body
  parse_slack_webhook_body-->>SlackAdapter: SlackWebhookPayload
  SlackAdapter-->>Client: response
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related issues

Possibly related PRs

  • Chinchill-AI/chat-sdk-python#49: Both PRs modify SlackAdapter.handle_webhook's request-body extraction path; this PR centralizes it into read_slack_request_body from the shared webhook primitives.
  • Chinchill-AI/chat-sdk-python#86: Both PRs touch handle_webhook in adapter.py—the retrieved PR extends Socket Mode forwarding while this PR reroutes verification to the shared primitives.
  • Chinchill-AI/chat-sdk-python#87: Both PRs change how SlackAdapter handles signature verification—the retrieved PR adds dynamic webhook_verifier support; this PR centralizes that logic into the shared verify_slack_request primitive.

Poem

🐇 Hop hop, the primitives bloom,
No runtime imports filling the room!
Webhook, API, blocks, and format too—
Each subpath clean, each boundary true.
hmac.compare_digest keeps secrets tight,
A lazy __getattr__ loads just right. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.64% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: porting Slack primitives subpaths from upstream chat SDK version 4.30. It is specific, readable, and accurately reflects the primary objective of modularizing Slack functionality into four self-contained subpackages.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment on lines +111 to +120
result = verify_slack_signature(
body,
{
"Content-Type": "application/json",
"X-Slack-Request-Timestamp": str(TIMESTAMP),
"X-Slack-Signature": _sign(body),
},
signing_secret=SECRET,
now=_now,
)

from __future__ import annotations

from collections.abc import Awaitable, Callable, Iterable, Mapping
# the first value) or an iterable of ``(name, value)`` pairs.
#
# Upstream: ``Headers | Iterable<[string, string]> | Record<string, value>``.
SlackHeaders: TypeAlias = Mapping[str, Any] | Iterable[tuple[str, str]]
kind: Literal["unsupported"] = "unsupported"


SlackWebhookPayload: TypeAlias = (

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request modularizes the Slack adapter by introducing lightweight, runtime-free subpaths for Web API primitives (api), Block Kit helpers (blocks), formatting (format), and webhook verification and parsing (webhook). The main SlackAdapter is refactored to delegate to these new subpaths, and comprehensive tests are added. Feedback on the changes highlights a security risk where the bot token is unnecessarily sent in the Authorization header to pre-signed file upload URLs, and a potential encoding issue in the signature verification where "utf-8" should be explicitly specified to prevent errors on systems with different default encodings.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +455 to +463
response = await request(
upload_url,
method="POST",
headers={
"authorization": f"Bearer {resolved_token}",
"content-type": "application/octet-stream",
},
body=data,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

security-high high

The Authorization header is unnecessary and poses a security risk when uploading files to the pre-signed upload_url returned by Slack. Pre-signed upload URLs do not require authorization headers, and sending the bot token to an external domain (or even to Slack's upload endpoint unnecessarily) violates the principle of least privilege and can lead to token leakage.

        response = await request(
            upload_url,
            method="POST",
            headers={
                "content-type": "application/octet-stream",
            },
            body=data,
        )

Comment on lines +145 to +149
expected = hmac.new(
signing_secret.encode("utf-8"),
f"v0:{timestamp}:{body}".encode(),
hashlib.sha256,
).digest()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Encoding the signature base string using the default system encoding (via .encode()) can lead to UnicodeEncodeError or incorrect bytes on systems where the default encoding is not UTF-8 (such as some Windows environments). This is especially problematic if the webhook body contains non-ASCII characters like emojis, which are extremely common in Slack payloads. Always specify "utf-8" explicitly.

Suggested change
expected = hmac.new(
signing_secret.encode("utf-8"),
f"v0:{timestamp}:{body}".encode(),
hashlib.sha256,
).digest()
expected = hmac.new(
signing_secret.encode("utf-8"),
f"v0:{timestamp}:{body}".encode("utf-8"),
hashlib.sha256,
).digest()

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1e7b843b21

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

return SlackAction(
action_id=string_value(raw.get("action_id")),
block_id=optional_string(raw.get("block_id")),
label=optional_string(_get(text, "text")),

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 Populate labels for selected-option actions

When Slack sends a block_actions payload from a static_select or radio_buttons element, the action usually carries the user-facing choice under selected_option.text.text and does not include a top-level text object. This parser already extracts selected_option_value, but label remains None, so consumers of the typed primitive lose the selected option label even though it is present in the payload; fall back to selected_option.text.text before the action's own text field.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (1)
src/chat_sdk/adapters/slack/api/__init__.py (1)

714-735: ⚖️ Poor tradeoff

Consider connection reuse for _default_fetch.

Creating a new AsyncClient for each request (line 724) prevents HTTP connection reuse. While this is fine for low-volume usage and callers can inject their own client, consider documenting this limitation or providing a module-level client option for high-throughput scenarios.

🤖 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/api/__init__.py` around lines 714 - 735, The
_default_fetch function creates a new httpx.AsyncClient for each request inside
the async with block, preventing HTTP connection reuse and reducing efficiency
in high-throughput scenarios. To fix this, either add module-level documentation
clearly explaining this limitation for users aware of the performance
implications, or implement a module-level persistent AsyncClient that can be
reused across multiple requests, ensuring proper initialization and cleanup of
the client lifecycle. Reference the AsyncClient instantiation on line 724 to
identify where the change needs to be made.
🤖 Prompt for all review comments with 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.

Inline comments:
In `@src/chat_sdk/adapters/slack/blocks/__init__.py`:
- Line 167: Replace the truthiness-based default operator with explicit None
checks in the convert_emoji call. The current code uses `title or "Card image"`
which treats empty strings as falsy and overwrites them with the default value.
Change this to `title if title is not None else "Card image"` to only apply the
default when the value is actually None rather than just falsy. This same
pattern needs to be applied at both occurrences mentioned (line 167 and line
258).
- Around line 525-530: The table rendering logic in this code block iterates
through columns based on the length of the current row, which causes shorter
rows to lose trailing columns and separators. Instead of iterating through
range(len(row)), iterate through range(len(widths)) to ensure every row is
padded to the same width as the header columns. This will maintain consistent
table shape by processing all columns for each row, padding missing cells with
empty strings as the existing logic already does.

In `@src/chat_sdk/adapters/slack/blocks/input.py`:
- Line 119: Replace the `or []` truthiness default pattern in the `options`
variable assignment and at the other locations (lines 292-293 and 311-312) with
explicit None checks using the pattern `x if x is not None else []`. This
ensures that falsey but non-None values are not hidden during TypeScript-port
boundary handling and follows the project coding guidelines for the src/
modules.

In `@src/chat_sdk/adapters/slack/format.py`:
- Line 100: Replace the truthiness check `if label` with an explicit None check
`if label is not None` on line 100 in the return statement. Similarly, update
the truthiness check `if link` to `if link is not None` on line 127. This
ensures that empty strings are handled correctly and not collapsed into the
"missing" path, preventing silent behavior changes where an explicitly passed
empty string would be treated the same as None.

---

Nitpick comments:
In `@src/chat_sdk/adapters/slack/api/__init__.py`:
- Around line 714-735: The _default_fetch function creates a new
httpx.AsyncClient for each request inside the async with block, preventing HTTP
connection reuse and reducing efficiency in high-throughput scenarios. To fix
this, either add module-level documentation clearly explaining this limitation
for users aware of the performance implications, or implement a module-level
persistent AsyncClient that can be reused across multiple requests, ensuring
proper initialization and cleanup of the client lifecycle. Reference the
AsyncClient instantiation on line 724 to identify where the change needs to be
made.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: df25e74c-15c4-46c9-a3ea-4e19e7a7c5d9

📥 Commits

Reviewing files that changed from the base of the PR and between 209601d and 1e7b843.

📒 Files selected for processing (21)
  • src/chat_sdk/adapters/slack/__init__.py
  • src/chat_sdk/adapters/slack/adapter.py
  • src/chat_sdk/adapters/slack/api/__init__.py
  • src/chat_sdk/adapters/slack/blocks/__init__.py
  • src/chat_sdk/adapters/slack/blocks/errors.py
  • src/chat_sdk/adapters/slack/blocks/input.py
  • src/chat_sdk/adapters/slack/blocks/limits.py
  • src/chat_sdk/adapters/slack/blocks/types.py
  • src/chat_sdk/adapters/slack/format.py
  • src/chat_sdk/adapters/slack/types.py
  • src/chat_sdk/adapters/slack/webhook/__init__.py
  • src/chat_sdk/adapters/slack/webhook/parse.py
  • src/chat_sdk/adapters/slack/webhook/types.py
  • src/chat_sdk/adapters/slack/webhook/utils.py
  • src/chat_sdk/adapters/slack/webhook/verify.py
  • tests/test_slack_adapter.py
  • tests/test_slack_api_primitives.py
  • tests/test_slack_blocks_primitives.py
  • tests/test_slack_dynamic_token_and_verifier.py
  • tests/test_slack_format_primitives.py
  • tests/test_slack_webhook_primitives.py
💤 Files with no reviewable changes (1)
  • tests/test_slack_adapter.py

blocks.append(
{
"alt_text": _truncate_text(
state.convert_emoji(title or "Card image"),

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 | 🟡 Minor | ⚡ Quick win

Replace truthiness-based defaults with explicit None checks.

These fallbacks can overwrite intentional empty-string values ("") and violate the repository rule for TS-port safety.

Suggested patch
-                "alt_text": _truncate_text(
-                    state.convert_emoji(title or "Card image"),
-                    LIMITS.image_alt,
-                ),
+                "alt_text": _truncate_text(
+                    state.convert_emoji(title if title is not None else "Card image"),
+                    LIMITS.image_alt,
+                ),
@@
-        "alt_text": _truncate_text(convert_emoji(element.get("alt") or "Image"), LIMITS.image_alt),
+        "alt_text": _truncate_text(
+            convert_emoji(element.get("alt") if element.get("alt") is not None else "Image"),
+            LIMITS.image_alt,
+        ),

As per coding guidelines, "Use x if x is not None else default instead of x or default to avoid truthiness traps when porting from TypeScript".

Also applies to: 258-258

🤖 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/blocks/__init__.py` at line 167, Replace the
truthiness-based default operator with explicit None checks in the convert_emoji
call. The current code uses `title or "Card image"` which treats empty strings
as falsy and overwrites them with the default value. Change this to `title if
title is not None else "Card image"` to only apply the default when the value is
actually None rather than just falsy. This same pattern needs to be applied at
both occurrences mentioned (line 167 and line 258).

Source: Coding guidelines

Comment on lines +525 to +530
for row in rows:
cells = [
(row[column] if column < len(row) else "").ljust(widths[column] if column < len(widths) else 0)
for column in range(len(row))
]
lines.append(" | ".join(cells).rstrip())

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 | 🟡 Minor | ⚡ Quick win

ASCII table rows can become ragged for short rows.

The renderer iterates per-row cell count, so rows with fewer cells than headers lose trailing empty columns/separators. Render against header width to keep table shape stable.

Suggested patch
-    for row in rows:
-        cells = [
-            (row[column] if column < len(row) else "").ljust(widths[column] if column < len(widths) else 0)
-            for column in range(len(row))
-        ]
+    for row in rows:
+        cells = [
+            (row[column] if column < len(row) else "").ljust(widths[column])
+            for column in range(len(table["headers"]))
+        ]
         lines.append(" | ".join(cells).rstrip())
🤖 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/blocks/__init__.py` around lines 525 - 530, The
table rendering logic in this code block iterates through columns based on the
length of the current row, which causes shorter rows to lose trailing columns
and separators. Instead of iterating through range(len(row)), iterate through
range(len(widths)) to ensure every row is padded to the same width as the header
columns. This will maintain consistent table shape by processing all columns for
each row, padding missing cells with empty strings as the existing logic already
does.

},
"type": "section",
}
options = request.get("options") or []

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 | 🟡 Minor | ⚡ Quick win

Avoid or [] defaults in src/ modules; use explicit None checks.

Using truthiness defaults here violates the project rule and can hide non-None falsey values during TS-port boundary handling.

Suggested patch
-    options = request.get("options") or []
+    options = request.get("options")
+    options = options if options is not None else []
@@
-        for option in (request.get("options") or [])
+        for option in (
+            request.get("options") if request.get("options") is not None else []
+        )
@@
-        for option in (request.get("options") or [])
+        for option in (
+            request.get("options") if request.get("options") is not None else []
+        )

As per coding guidelines, "Use x if x is not None else default instead of x or default to avoid truthiness traps when porting from TypeScript".

Also applies to: 292-293, 311-312

🤖 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/blocks/input.py` at line 119, Replace the `or []`
truthiness default pattern in the `options` variable assignment and at the other
locations (lines 292-293 and 311-312) with explicit None checks using the
pattern `x if x is not None else []`. This ensures that falsey but non-None
values are not hidden during TypeScript-port boundary handling and follows the
project coding guidelines for the src/ modules.

Source: Coding guidelines

def format_slack_link(url: str, label: str | None = None) -> str:
"""Format a link, escaping the label: ``<url|label>`` or ``<url>``."""
_assert_no_slack_control(url, "url")
return f"<{url}|{escape_slack_text(label)}>" if label else f"<{url}>"

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 | 🟡 Minor | ⚡ Quick win

Use explicit None checks for optional string params.

Line 100 and Line 127 use truthiness checks (if label, if link), which collapses explicit empty strings into the “missing” path. Use is not None to avoid silent behavior changes.

As per coding guidelines, "Use x if x is not None else default instead of x or default to avoid truthiness traps when porting from TypeScript".

Suggested patch
-    return f"<{url}|{escape_slack_text(label)}>" if label else f"<{url}>"
+    return f"<{url}|{escape_slack_text(label)}>" if label is not None else f"<{url}>"
...
-    link_part = f"^{_assert_slack_date_link(link)}" if link else ""
+    link_part = f"^{_assert_slack_date_link(link)}" if link is not None else ""

Also applies to: 127-127

🤖 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/format.py` at line 100, Replace the truthiness
check `if label` with an explicit None check `if label is not None` on line 100
in the return statement. Similarly, update the truthiness check `if link` to `if
link is not None` on line 127. This ensures that empty strings are handled
correctly and not collapsed into the "missing" path, preventing silent behavior
changes where an explicitly passed empty string would be treated the same as
None.

Source: Coding guidelines

… fidelity)

Port the SlackFile/SlackUser/SlackViewStateValue primitives and the
helpers (parseFiles/inferFileType/parseUser/findPromptBlock/
readPromptText/parseViewValues) that the webhook parse.py port dropped vs
upstream chat@4.30.0 packages/adapter-slack/src/webhook/parse.ts:

- files[]: populated on every message-like event via _parse_files
  (mimetype-inferred type, raw retained).
- SlackAction.label: now prefers the selected option's text and falls
  back to the element text (parse.ts:276); surface selected_option_label.
- SlackUser: attached as the user object on block_actions / view_submission
  / view_closed payloads and on each parsed action.
- block_actions: restore message_blocks / message_prompt_block /
  message_prompt_text; view_submission: restore callback_id /
  private_metadata / values (parseViewValues).

Extend webhook primitive tests to lock in every newly-ported field; add
Known-Non-Parity rows for the slack/api SSRF + token-leak guards.
@patrick-chinchill patrick-chinchill merged commit aced743 into main Jun 18, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants