diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bb29067..5c70a047 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,17 @@ Sync wave from `chat@4.31.0` to `chat@4.41.1` (tracking #184). `UPSTREAM_PARITY` stays `4.31.0` until #203. +- **Slack inbound mrkdwn normalization, `channel.post()` thread ids, Socket Mode retries** (#283, part (b) of #209; ports vercel/chat `0b63791b` #667 (Slack half), `92530dd3` #720, `c3118279` #756, `e71bfead` #843 (Slack half) and `44423bdc` #960, chat@4.33.0–4.41.1). A live Slack-loop check is pending. + - **Breaking/consumer-visible (`message.text` is the plain text of `message.formatted`):** both parse paths now set `text = ast_to_plain_text(formatted)` (upstream `toPlainText(formatted)`), replacing the regex pass over the mrkdwn. Markup no longer leaks into `text`: inline-code backticks, list markers (`- a` → `a`, `1. one` → `one`), heading `#` and quote `>` prefixes are dropped, a trailing newline or a whitespace-only body is trimmed, and blocks are separated by a blank line. `SlackFormatConverter.extract_plain_text` drops its Python-only regex override for the base `to_ast` + `ast_to_plain_text` (upstream has none). *Migration:* match structure on `message.formatted`, or on `message.raw["text"]` for the original mrkdwn. + - **Breaking/consumer-visible (mentions and links read differently):** `` / `` / `` become `@here` / `@channel` / `@everyone`, `` becomes `@eng` (`` → `@S1`), and a labelled channel keeps its id: `<#C123|general>` reads `#general (C123)` instead of `#general`. Tokens inside inline code or code blocks stay literal. A swapped `` link becomes `[label](https://…)`. `<` / `>` / `&` are now unescaped in `formatted` and `text` (they came through verbatim). The same applies to table cells and mrkdwn attachment parts (#210). *Migration:* update `on_message` regexes and anything matching `#channel-name` or raw `` / ``. + - **Breaking/consumer-visible (code blocks keep their first line):** Slack treats the text right after an opening ```` ``` ```` as code; it was read as an info string and dropped, so `` ```npm test``` `` gave an empty `formatted` (and `text` kept the backticks). Each paired fence now parses as a `code` node and `text` holds the code (`"npm test"`). Unpaired fences, fences in inline code or `<…>` tokens, and fences on `>` quote lines stay literal, as in Slack. + - **Breaking/consumer-visible (`channel.post()` ids):** `post_channel_message` returns `slack:C123:` (a replyable thread rooted at the new message) instead of `slack:C123:` when Slack returns a string message `ts`, so `chat.thread(sent.thread_id).post(...)` replies in its thread. File-only posts keep `slack:C123:`. + - **Breaking/consumer-visible (Socket Mode retries are processed):** envelopes with `retry_attempt > 0` were acked and dropped; they are now routed like first deliveries and logged at info (`"Processing socket mode retry"` with `retry_attempt`, `retry_reason`, `type`). The event-id marker (#268) and core message-id dedupe (10 min TTL, #191) drop true duplicates, so an event missed during a restart or reconnect is no longer lost. + - Shared `parse_markdown` follows CommonMark's fence rule: a line like ```` ```npm test``` ```` (a backtick in the info string) is no longer a code-block opener, so a Slack quote starting with an inline fence keeps its text instead of becoming an empty code block. This affects every adapter that parses Markdown, only for such lines, and those lines now go through the (quadratic, #308) inline parser. + - Shared `parse_markdown` follows four more CommonMark rules that `message.text` exposed (every adapter that parses Markdown sees them): a paragraph drops its lines' leading spaces/tabs and its trailing ones (`run ```npm test``` please` reads `run`, `npm test`, `please`; a whitespace-padded message is trimmed); a thematic break repeats one marker (`-_-` is text, not an empty break); `_` emphasis is never intraword (`my_var` and `?utm_source=` keep their underscores); links never nest (`[[a](u) b](v)` is the inner link with literal brackets). + - **Python-specific (divergence from upstream):** `parse_markdown` nests at most 100 blockquotes; deeper `>` markers stay literal text. Slack `>` now unescapes, and 1,100 of them raised `RecursionError` and dropped the message. See `docs/UPSTREAM_SYNC.md`. + - Shared `parse_markdown` links accept balanced brackets in the text and balanced parens in the URL (`[build [failed]](u)`, `…/Foo_(bar))`), as CommonMark does, so Slack labelled links read as upstream in `message.text`. Every adapter that parses Markdown sees this; an unbalanced `(` in a URL or a bare `[` in link text no longer forms a link. + - The mrkdwn scanners stay linear on untrusted text (results identical to upstream). Known shared-parser gaps (no multi-backtick code spans, no lazy blockquote continuation so `> quoted` + a reply line reads as two blocks, quadratic time on some inputs, which the fence rule above reaches with new inputs) are tracked in #308; see `docs/UPSTREAM_SYNC.md`. - **Telegram: multiple files or attachments go out as one media group** (#278; ports vercel/chat `8d7ccdb1` #605, chat@4.34.0). - **Consumer-visible (Telegram):** `post_message` / `thread.post` with 2–10 `files` or 2–10 `attachments` now sends one `sendMediaGroup` album instead of raising `ValidationError("Telegram adapter supports a single file/attachment upload per message")`. Every sent message is cached and the last one is returned. The caption (with `parse_mode`) goes on the first item only, with the same MarkdownV2 → plain-text retry as single uploads. `thread.reply` threads the album to its target (`reply_parameters`). One file or attachment still uses `sendDocument` / `sendPhoto` / `sendVideo` / `sendAudio`. - New `ValidationError`s: more than 10 items ("Telegram media groups support 2-10 files"); a card with buttons ("Telegram media groups do not support inline keyboards"); documents or audio mixed with another type (photos and videos may mix). Mixing `files` and `attachments` still raises. @@ -89,7 +100,7 @@ Sync wave from `chat@4.31.0` to `chat@4.41.1` (tracking #184). `UPSTREAM_PARITY` - **Consumer-visible (Slack):** `message.links` gains each non-unfurl attachment's `title_link`. - **Routing:** attachment and table content now reaches `message.text`, so `on_message` regex patterns (and the core's text-based mention fallback when the bot id is unknown) can match it. Alert-bot messages in subscribed or pattern-matched channels may start matching; filter on `message.author.is_bot` if that is unwanted. LLM prompts built from `message.text` (`to_ai_messages`, history) get the extra content. Streaming is unaffected. - The async parse path resolves `<@U…>` / `<#C…>` in table cells and attachment parts in one parallel lookup wave; the sync `parse_message` path does no lookups. `SlackEvent` gains typed `attachments` (`SlackAttachment`) and `SlackMessageBlock` (with `rows`). - - **Python-specific (temporary, until #283):** upstream derives all of `text` from `formatted`. That needs #283's inbound mrkdwn normalization; without it, code on a fence's opening line would be lost (`` ```npm test``` `` would give empty text). #283 switches to `ast_to_plain_text(formatted)`, which also drops list, `#`, `>` and backtick markers from body text. Until then, table cells and mrkdwn attachment parts render through the current converter: Slack's `&` / `<` / `>` entities stay, a usergroup shows as a raw `` token, and a labelled channel shows as `#name` (upstream shows `#name (C…)`). Body `formatted` has the same gaps today. Live Slack-loop check: pending. + - **Python-specific (temporary, resolved by #283, see the #283 entry above):** upstream derives all of `text` from `formatted`. That needs #283's inbound mrkdwn normalization; without it, code on a fence's opening line would be lost (`` ```npm test``` `` would give empty text). #283 switches to `ast_to_plain_text(formatted)`, which also drops list, `#`, `>` and backtick markers from body text. Until then, table cells and mrkdwn attachment parts render through the current converter: Slack's `&` / `<` / `>` entities stay, a usergroup shows as a raw `` token, and a labelled channel shows as `#name` (upstream shows `#name (C…)`). Body `formatted` has the same gaps today. Live Slack-loop check: pending. - **Slack: Agent messaging experience (`agent_view`) and declarative agent config** (#214; Slack halves of vercel/chat `1721fa01` #684 and `0f743c9b` #698, plus `78021c09` #889 and the Slack part of `c21ccbc0` #943, chat@4.34.0–4.41.0). Slack will retire `assistant_view` in February 2027. Every feature is opt-in on `SlackAdapterConfig` and off by default. A live Slack-loop check is still pending. - `agent_view=True`: `app_home_opened` fires for every tab and carries `tab` and, when Slack folds one in, `entities`. Each top-level DM message becomes its own thread (`slack:D…:{ts}` instead of `slack:D…:`), so replies thread under the user's message. If the conversation-scoped `slack:D…:` from `open_dm` is subscribed, top-level DMs still route there, so `on_subscribed_message` keeps working. @@ -181,7 +192,7 @@ Sync wave from `chat@4.31.0` to `chat@4.41.1` (tracking #184). `UPSTREAM_PARITY` - Tokens resolve only for the button that minted them (`actionId`) and in the conversation they were posted to. Thread posts, schedules and edits bind to the thread. Channel posts, schedules and channel `SentMessage.edit`s bind to the channel. A `post_ephemeral` DM fallback binds to the DM channel, and with neither native ephemeral nor a DM fallback no token is minted. A click whose action or conversation does not match leaves the record in place. - The token swap now keeps every button field except `callback_url` (it used to copy a fixed whitelist), so new fields such as `tooltip` (#202) survive. - **Python-specific (divergence from upstream):** after deleting a matched record, the resolver checks with `extend_lock` that its 10-second lease never lapsed, and returns `None` if it did. Upstream returns the record regardless, so a state call that stalls past the lease can let a second click also resolve and POST. See `docs/UPSTREAM_SYNC.md`. - - **Python-specific (divergence from upstream):** a channel `SentMessage.edit` binds its tokens to the channel (the scope the original `channel.post` used), not to `{reported thread id, "thread"}`. The thread id an adapter reports for a channel post often never equals a click's thread id: Teams and Google Chat report the channel id, Slack reports the synthetic `slack:C…:` (a click carries the message ts, and a Slack DM click carries no ts even once #283 makes the post report one), and a chained edit drops the reported id. Upstream's thread scope would leave those edited buttons never POSTing. + - **Python-specific (divergence from upstream):** a channel `SentMessage.edit` binds its tokens to the channel (the scope the original `channel.post` used), not to `{reported thread id, "thread"}`. The thread id an adapter reports for a channel post often never equals a click's thread id: Teams and Google Chat report the channel id, Slack reported the synthetic `slack:C…:` until #283 and still does without a string ts (a click carries the message ts, and a Slack DM click carries no ts even though the post now reports one), and a chained edit drops the reported id. Upstream's thread scope would leave those edited buttons never POSTing. - Known gap (see `docs/UPSTREAM_SYNC.md`): Google Chat cards `thread.post`ed into a DM thread never POST (upstream parity: the card click omits the `:dm` suffix). - API: `process_card_callback_urls(card, state, scope)` takes a required `CallbackScope`. `resolve_callback_url(token, state, context=None)` takes a `CallbackContext` (a `None` context never matches). `ResolvedCallback` gains keyword-only `action_id` and `scope`. New constant: `CALLBACK_LOCK_TTL_MS = 10_000`. - **BREAKING (security) — Telegram: webhook verification is required by default; repeated updates are deduplicated** (#224; upstream vercel/chat#858, #799, #813). @@ -447,7 +458,7 @@ Ports upstream `32687038` (vercel/chat#830, chat@4.38.1), the Google Chat part o - Fidelity: `ai/index.test.ts` 26 → 0 missing, `ai/messages.test.ts` 10 → 9 at `chat@4.41.1`. - **Slack: Enterprise Grid org-wide installs, `authorizations[]` routing, event retry marker** (#268, split from #213; ports the non-cache half of vercel/chat `907450d7` #724, chat@4.35.0). - **Consumer-visible: org-wide OAuth installs now succeed.** `handle_oauth_callback` used to raise `missing access_token or team.id` for an org-wide install (`team: null`). It now stores it under `enterprise.id`, the key org-wide webhooks resolve by, and raises `missing access_token or enterprise.id` when that is absent. The result gains `enterprise_id` and `is_enterprise_install`; `team_id` is the storage key. `SlackInstallation` gains `enterprise_id` / `is_enterprise_install`. - - **Consumer-visible: retried events that were already dispatched are dropped.** Each dispatched event writes a `slack:event-delivered:{event_id}` state key (24 h TTL, fire-and-forget). A delivery with `x-slack-retry-num > 0` (or a forwarded socket event with `retryNum > 0`) whose key exists is acked and not processed; first deliveries never read state, and a retry whose original never arrived is still processed. Live Socket Mode retries are still skipped until #283. + - **Consumer-visible: retried events that were already dispatched are dropped.** Each dispatched event writes a `slack:event-delivered:{event_id}` state key (24 h TTL, fire-and-forget). A delivery with `x-slack-retry-num > 0` (or a forwarded socket event with `retryNum > 0`) whose key exists is acked and not processed; first deliveries never read state, and a retry whose original never arrived is still processed. Since #283, live Socket Mode retries (`retry_attempt > 0`) are routed and reach the marker as well. - Multi-workspace events resolve their installation from `authorizations[0]` before the top-level `team_id` / `enterprise_id`, so Slack Connect events route to the receiving installation. Socket Mode `events_api`, slash commands and interactive payloads now resolve org-wide installs by enterprise ID, like HTTP. - Under an org-wide install, the adapter's Web API calls send the event's workspace `team_id`, and calls to the event's channel echo a Slack Connect `context_team_id` as `client_context_team_id`. A `team_id` the caller passes wins. The #95 `chat_stream` `team_id` is unchanged. - `W…` user ids count as raw user ids in outgoing `@mentions`. `with_bot_token` / `with_bot_token_async` accept keyword-only `installation_id=` to scope installation-owned caches outside webhooks. `RequestContext` gains `team_id`, `context_team_id` and `context_channel`. diff --git a/docs/UPSTREAM_SYNC.md b/docs/UPSTREAM_SYNC.md index 516f1589..c5deccfe 100644 --- a/docs/UPSTREAM_SYNC.md +++ b/docs/UPSTREAM_SYNC.md @@ -445,17 +445,17 @@ The channel-edit divergence exists because the thread id reported for a channel post often never equals a click's thread id: - Teams and Google Chat `post_channel_message` return the channel id as the thread id (upstream too); -- Slack `post_channel_message` returns the synthetic `slack:C…:` until #283 - ports upstream `92530dd3` (vercel/chat#720), while a click reports - `slack:C…:`. After #283 a Slack *DM* post reports +- Slack `post_channel_message` returned the synthetic `slack:C…:` until #283 + ported upstream `92530dd3` (vercel/chat#720), while a click reports + `slack:C…:`. Since #283 a Slack *DM* post reports `slack:D…:`, but a DM click reports `slack:D…:` (Python's DM `_handle_block_actions` divergence), so a thread scope would still miss; - chained edits (`sent = await sent.edit(...)` twice), whose returned `SentMessage` dropped the thread-id override before #195 ported `16ea171e` (it now keeps it, so this reason no longer applies on its own). -Binding to the channel resolves all of these, independent of #283's merge -order, because every click on the message derives the same channel id. +Binding to the channel resolves all of these, because every click on the +message derives the same channel id. **Breaking for `callback_url` users:** tokens minted before the upgrade stop resolving, a repeat click no longer POSTs, tokens expire after 7 days, and the @@ -1000,7 +1000,7 @@ Parity with the core halves of upstream `0b63791b` (chat@4.33.0), `f233ffe8`, `None` and resolves with `is not None` (upstream `??`). Before this change the dataclass default `300000` always won and `0` fell through `or` to the constant. `0` now reaches the state adapter, and the bundled backends treat a - `0` TTL as no expiry (upstream `state-redis` does the same). The Slack Socket Mode retry-envelope half of `0b63791b` is #283 (split from #209). + `0` TTL as no expiry (upstream `state-redis` does the same). The Slack Socket Mode retry-envelope half of `0b63791b` landed in #283 (split from #209). - **`wait_until` and handler errors (`c21ccbc0`, `91683e52`).** Upstream hands `waitUntil` a `task.catch(log)` promise that fulfils when the handler fails. Python passed the raw task, so a host that awaited it saw handler errors @@ -1150,7 +1150,8 @@ Parity, with no new divergence. Part (a) of #209 ports the Slack halves of `bb7cd124` (#716) and `80def3ab` (#707, chat@4.35.0), `51322dde` (#891) and `c2b6bff0` (#883, chat@4.40.0), and `683eadc1` (#947, chat@4.41.0). Part (b) (the mrkdwn normalization from `e71bfead` / `44423bdc` / `c3118279`, the -`92530dd3` channel-post id and the `0b63791b` Socket Mode retries) is #283. +`92530dd3` channel-post id and the `0b63791b` Socket Mode retries) landed in +#283; see "Slack inbound mrkdwn, channel-post ids, Socket Mode retries". - **Self mention (`51322dde`).** `_resolve_inline_mentions` no longer skips the bot's id. `<@U_BOT>` resolves through `_lookup_user_name` like any other @@ -1191,17 +1192,16 @@ Parity, with no new divergence. Part (a) of #209 ports the Slack halves of ### Slack inbound content: pasted tables and alert attachments (chat@4.38–4.39, #210) -Parity apart from one temporary divergence (the body's plain text, below, -until #283). Ports the Slack halves of `764e4759` (#817, +Parity. Ports the Slack halves of `764e4759` (#817, chat@4.38.1) and `864d9222` (#846, chat@4.39.0); the core `toPlainText` table rules were #193 and the `previous_message` hunk of #846 is #211. - **Content assembly.** Both parse paths build `formatted` from the body text plus `table` / `data_table` blocks (tables before the first non-table block above the text, the rest below) plus each non-unfurl attachment's content. - `text` appends the tables' and attachments' `ast_to_plain_text` to the - body's plain text (see the #283 bullet for how the body's share is - derived). The helpers are module-level in + `text` is `ast_to_plain_text(formatted)` (upstream `toPlainText(formatted)`; + until #283 the body's share kept the pre-#210 regex rendering). The + helpers are module-level in `adapters/slack/adapter.py` (no separate module): `_block_text`, `_has_bold_text`, `_table_data`, `_event_tables`, `_attachment_content`, `_literal_phrasing`, `_collect_mention_ids`, `_apply_mention_names`, and the @@ -1234,36 +1234,6 @@ rules were #193 and the `previous_message` hunk of #846 is #211. Python slices copy, so the upstream shape is quadratic (about 4 s on 200k characters of `<@U1>` tokens) now that it also runs over attachment text. Output is identical (fuzzed against the upstream-shaped versions). -- **Temporary divergence until #283: the body's plain text.** Upstream - derives `text = toPlainText(formatted)`, and its `toAst` runs - `slackMrkdwnToMarkdown`, which moves code after an opening fence onto its - own line, unescapes `&` / `<` / `>`, renders `` as - `@…` and `<#C…|name>` as `#name (C…)`. The Python - `SlackFormatConverter.to_ast` does none of this yet (#283 ports it), so - deriving all of `text` from `formatted` would drop code from ordinary - messages (`` ```npm test``` `` would give `text == ""`). Until #283, - `_assemble_content` builds `text` as the blank-line join of the non-empty - plain texts of the leading tables, `extract_plain_text(body)` (the pre-#210 - rendering, so a message without tables or attachments gets the same `text` - as before) and the trailing tables and attachment nodes. That is what - `toPlainText` does with root children, so only the body's share differs - from upstream. #283 replaces it with `ast_to_plain_text(formatted)` and - updates these assertions in `tests/test_slack_inbound_content.py`: - `test_body_text_keeps_its_pre_283_rendering` (to upstream's fence output), - `test_keeps_attachment_content_out_of_an_unclosed_code_fence_in_the_body` - (upstream's `"Deploy failed:\n\nTypeError: boom\n\n…"`), - `test_preserves_rich_text_metadata_within_table_cells` (upstream's - `"#C789 @S789 July 11 #ff0000"`) and - `test_resolves_user_and_channel_mentions_in_table_cells` - (`"@Test Bot\t#general (C789)"`). - Table cells and mrkdwn attachment parts go through `to_ast` already, so - until #283 they show the same gaps body `formatted` shows today: a cell - fence loses its opening line, a `raw_text` cell holding `R&D` or `<@U…>` - (escaped by `_block_text`, as upstream does) shows `R&D` / - `<@U…>`, a usergroup cell shows `` and a labelled - channel shows `#name`. That content was absent before #210. Literal - attachment parts unescape on their own (`_literal_phrasing`) and already - match upstream. - **`title_link` and the unfurl wait.** On a webhook, an untitled `title_link` preview makes `_enrich_links` poll the unfurl cache (up to 2 s) exactly as upstream does. Fetched (history) messages poll too, only @@ -1271,6 +1241,99 @@ rules were #193 and the `previous_message` hunk of #846 is #211. already applied to every untitled link in fetched messages and is tracked there and in #292. +### Slack inbound mrkdwn, channel-post ids, Socket Mode retries (chat@4.33–4.41, #283) + +Part (b) of #209. Parity apart from one divergence (the blockquote depth cap, +below). Ports the Slack halves of +`0b63791b` (#667, chat@4.33.0), `92530dd3` (#720, chat@4.35.0), `c3118279` +(#756, chat@4.37.0), `e71bfead` (#843, chat@4.39.0) and `44423bdc` (#960, +chat@4.41.1). + +- **mrkdwn (`e71bfead`, `44423bdc`, `c3118279`).** `slack/format.py` ports + `convertSpecialMentions` and `convertMrkdwnWithCodeFences` with their + helpers (`findAngleTokenEnd`, `findInlineCodeEnd`, `isOnBlockquoteLine`, + `escapeLeadingBlockMarker`), the `#name (C…)` channel rewrite and the + inverted-link swap. As upstream, the fence normalizer is a separate copy of + `shared.code_fences` with mrkdwn's entity escaping (`>` quote lines, + `<…>` tokens). JS `^`/`$` without `m` are `re.match`/`\Z`, and JS `\d` is + `[0-9]`. +- **`to_ast` and `message.text`.** `SlackFormatConverter.to_ast` is + `parse_markdown(slack_mrkdwn_to_markdown(text))`, as upstream `toAst`. The + old inline regex copy skipped `unescape_slack_text`, so `<` / `>` / + `&` reached `formatted` and `text` verbatim. The Python-only regex + `extract_plain_text` override is removed (upstream `markdown.ts` has none; + the base is `to_ast` + `ast_to_plain_text`). Both parse paths set + `text = ast_to_plain_text(formatted)` over the assembled content (upstream + `toPlainText(formatted)`), replacing the #210 interim that kept the regex + rendering for the body's share. Consequences: list markers, `#`, `>` and + backticks no longer appear in `message.text`, and blocks join with a blank + line. +- **`post_channel_message` (`92530dd3`)** returns `slack:{channel}:{ts}` when + the response's `raw["ts"]` is a `str`, else the synthetic `slack:{channel}:` + (file-only uploads), as upstream's `typeof result.raw.ts !== "string"` check. +- **Socket Mode retries (`0b63791b`).** Retry envelopes are routed like first + deliveries. The event-id marker (#268) drops a retry whose first delivery + was dispatched, and core message-id dedupe (10 min TTL, #191) drops true + duplicates. slack_sdk's `SocketModeRequest.retry_attempt` / `retry_reason` + are upstream's `retry_num` / `retry_reason`; the info log + `"Processing socket mode retry"` uses the key `retry_attempt`. +- **Linear-time scanning (implementation, not divergence).** Results equal + upstream's. A failed `<…>` scan is remembered so a run of unclosed `<` stays + linear, the inline-code newline check is bounded by the closing backtick, + and the blockquote-line check (`_BlockquoteLines`) remembers the current + line, so many fences on one long indented quote line stay linear (upstream + re-slices the line per fence). `tests/test_slack_format_primitives.py` + covers 50k-character inputs, and a differential fuzz against upstream + `format/index.ts` (40k random mrkdwn inputs) found no mismatch. +- **Known gaps (shared parser, not Slack-specific).** One `markdown.test.ts` + case is adapted: `parse_markdown` has no multi-backtick code spans + (```` ```c``` ```` inside a quote stays backticked text, where remark reads a + code span). Not handled either, and now visible in `message.text`: no lazy + blockquote continuation (`"> quoted\nreply"` gives `"quoted\n\nreply"`, + upstream `"quoted\nreply"` in one quote paragraph), and `\r` is not a line + ending. `parse_markdown` is also quadratic on some inputs (about 1.6 s for + 16k characters of `` `x ``); this predates the port, since `to_ast` always + ran it on every inbound message. The fence rule below adds new inputs of + that cost: ```` ``` ```` followed by `` a` `` repeated (16 KB, about 1.6 s) + or ```` ```x` ```` lines (48 KB, about 10 s) were one O(n) code block + before and now go through the inline parser. All are tracked in #308. +- **Shared parser fixes (CommonMark rules exposed by `message.text`).** + Since `text` is the plain text of `parse_markdown`, these parser bugs now + lost or corrupted text, so the parser follows CommonMark (values checked + against remark): a paragraph drops its lines' leading spaces/tabs and its + trailing ones (`"run ```npm test``` please"` reads `"run\n\nnpm test\n\nplease"`, + not `"run \n\nnpm test\n\n please"`); a thematic break repeats one marker + (`-_-` stayed empty text); `_` emphasis is never intraword (`my_var` and a + `?utm_source=` URL after a snake_case word kept losing underscores and + leaking `[label](url)`); and links never nest (`[[a](u) b](v)` is the inner + link with literal outer brackets). +- **Shared parser fix (CommonMark fence rule).** A backtick fence's info + string may not contain a backtick, so `parse_markdown` no longer opens a + code block on a line like ```` ```npm test``` ````. The Slack normalizer + leaves a quoted fence literal (as upstream), and without this rule + `> ```npm test```` parsed to an empty code block and dropped the command + from `message.text`. It now stays text in the quote (backticks kept, the + multi-backtick gap above). A line-leading *unpaired* ```` ```npm test ```` + still opens an empty code block, as in upstream (remark does the same). +- **Blockquote depth cap (divergence, see the non-parity table).** `>` now + unescapes, so `">" * 1100` reaches `parse_markdown`'s recursive + blockquote parsing and raised `RecursionError`, failing the message (and a + whole `fetch_messages` page). Past 100 nested levels (markdown-it's default + `maxNesting`) the rest of the quote stays literal text, which also leaves + stack room for lists nested inside the quote. Nested lists on their own + recurse the same way and predate this change (#308). +- **Shared parser fix (link text and destinations).** The converter emits + `[label](url)`, and remark accepts balanced brackets in the label and + balanced parens in the URL. `parse_markdown`'s link pattern now does too + (one level), so `` reads + `build [failed]` and a Wikipedia URL ending in `_(bar)` keeps its `)` + instead of leaking `[…](…)` syntax or a stray `)` into `message.text`. + CommonMark-invalid cases (an unbalanced `(` in the URL, a bare `[` inside + the label) no longer form a link, as in remark. Escaped `\(` / `\)` in a + destination are consumed whole and never count toward the balance. A + closed code span in the label is opaque (code spans bind tighter than link + brackets), so ``[`[`](u)`` stays a link with an `inlineCode` child. + ### Teams Adaptive Card 1.5 rendering (chat@4.36–4.41, #220) Parity, not a divergence. The Teams half of `0153a39f` (chat@4.36.0), @@ -1365,11 +1428,9 @@ chat@4.35.0); the installation-scoped caches landed in #205. Code is in Python-specific notes: -- **Socket retries before #283.** `_on_socket_request` still acks and skips - envelopes with `retry_attempt > 0`, so on a live socket the marker only - sees first deliveries; it already covers HTTP retries and forwarded socket - events. #283 (split from #209) removes the skip, after which `retry_attempt` reaches the - marker unchanged. +- **Socket retries.** Since #283, `_on_socket_request` routes envelopes with + `retry_attempt > 0` like first deliveries, so `retry_attempt` reaches the + marker on a live socket too (before, retries were acked and dropped). - **Flag normalization.** `is_enterprise_install` counts only as `True` or `"true"` everywhere (upstream's event path uses `Boolean(...)`, so a `"false"` string would count there). Slack sends booleans in event JSON, so @@ -3192,7 +3253,8 @@ stay explicit instead of being rediscovered in code review. | Area | Python behavior | TS behavior | Rationale | |------|----------------|-------------|-----------| | JSX Card/Modal elements | Not supported; tests skipped | `Card()` returns JSX element | Python has no JSX runtime | -| Channel edit callback scope (4.41 wave, #194) | A channel `SentMessage.edit` binds new callback tokens to `{channel.id, "channel"}`, the scope the original `channel.post` used | `createSentMessage(...).edit` binds to `{threadId, "thread"}`, where `threadId` is the id the adapter reported for the post (or the channel id) | The reported thread id often never equals a click's thread id: Teams and Google Chat report the channel id (upstream as well; Teams clicks carry `;messageid=`, Google Chat clicks carry the thread name), Python's Slack reports the synthetic `slack:C…:` until #283 while clicks carry the message ts, a Slack DM click carries no ts even after #283 makes the post report one, and a chained edit dropped the override until #195 ported `16ea171e`. Upstream's edited buttons never POST in those cases. Every click on the message derives the channel id, and the channel scope is no broader than the original post's. Regression tests: `tests/test_channel_faithful.py::TestCallbackUrlProcessing::test_edited_slack_channel_card_resolves_for_the_real_click` (real Slack id functions and `_handle_block_actions`, channel and DM, before and after #283), `::test_edited_teams_channel_card_resolves_for_a_click_in_that_channel` (real Teams id functions) and `::test_chained_edit_keeps_callback_tokens_resolvable`. To be filed as an upstream issue against vercel/chat (Teams and Google Chat edited channel cards never POST); upstream Slack is unaffected, since its `postChannelMessage` and DM clicks both carry the message ts. | +| Blockquote nesting cap (4.41 wave, #283) | `parse_markdown` nests at most 100 blockquotes (markdown-it's default `maxNesting`); deeper `>` markers stay literal text in the innermost quote | remark nests without limit (`"> " * 1100 + "x"` gives 1,100 blockquotes) | Blockquotes parse recursively and Python's recursion limit (1,000) turned untrusted input such as Slack `">" * 1100` into a `RecursionError` that dropped the message. Only absurdly deep quotes differ. Regression test: `tests/test_slack_format.py::TestToMarkdown::test_deeply_nested_quotes_do_not_exhaust_the_stack`. | +| Channel edit callback scope (4.41 wave, #194) | A channel `SentMessage.edit` binds new callback tokens to `{channel.id, "channel"}`, the scope the original `channel.post` used | `createSentMessage(...).edit` binds to `{threadId, "thread"}`, where `threadId` is the id the adapter reported for the post (or the channel id) | The reported thread id often never equals a click's thread id: Teams and Google Chat report the channel id (upstream as well; Teams clicks carry `;messageid=`, Google Chat clicks carry the thread name), Python's Slack reported the synthetic `slack:C…:` until #283 (it still does when the response has no string ts) while clicks carry the message ts, a Slack DM click carries no ts even though the post now reports one, and a chained edit dropped the override until #195 ported `16ea171e`. Upstream's edited buttons never POST in those cases. Every click on the message derives the channel id, and the channel scope is no broader than the original post's. Regression tests: `tests/test_channel_faithful.py::TestCallbackUrlProcessing::test_edited_slack_channel_card_resolves_for_the_real_click` (real Slack id functions and `_handle_block_actions`, channel and DM, before and after #283), `::test_edited_teams_channel_card_resolves_for_a_click_in_that_channel` (real Teams id functions) and `::test_chained_edit_keeps_callback_tokens_resolvable`. To be filed as an upstream issue against vercel/chat (Teams and Google Chat edited channel cards never POST); upstream Slack is unaffected, since its `postChannelMessage` and DM clicks both carry the message ts. | | Callback-token lease fence (4.41 wave, #194) | After deleting a matched record, `resolve_callback_url` calls `extend_lock(lock, CALLBACK_LOCK_TTL_MS)`. If that fails, the 10 s lease lapsed mid-consume, and the call returns `None` instead of the record (fail closed: no POST, raw `__cb:` value to handlers) | `resolveCallbackUrl` returns the record after `delete` regardless of lease state, so if a `get`/`delete` stalls past the lease, a second click that takes the expired lock also resolves it and both POST | Keeps the single-use contract under state-backend stalls. `extend_lock` checks token ownership and expiry in every backend (Memory, Redis script, Postgres `WHERE token = $4 AND expires_at > now()`), and `Chat` already relies on it for lock heartbeats. The cost is one extra state call per resolved click. A stalled consume that loses its lease also burns the token without a POST. To be filed as an upstream issue against vercel/chat (a stalled `get`/`delete` past the 10 s lease lets a second click double-POST). Regression test: `tests/test_callback_url.py::TestResolveCallbackUrlLocking::test_lost_lease_fails_closed_instead_of_double_consuming`. | | Link-preview fence slicing (4.41 wave, #195) | `_render_link_for_prompt` bounds url/title/description/site with Python slicing, which counts code points | `renderLinkForPrompt` slices with `String.prototype.slice`, which counts UTF-16 code units | Only differs for astral characters (emoji and the like): a bounded field keeps up to the limit in code points, where JS keeps half as many astral characters and can end on a lone surrogate. Emulating UTF-16 slicing would produce lone surrogates that break UTF-8 encoding of the prompt. Whitespace handling is not a divergence: the normalizer uses JS's exact `\s`/`trim` set. Regression test: `tests/test_ai_messages.py::TestLinkPreviews::test_link_metadata_bounds_count_code_points_not_utf16_units`. | | Adapter hook signature probe (4.41 wave, #200, #201) | `Thread.start_typing` passes `options=TypingOptions(...)` to `adapter.start_typing` under the same rule (a two-argument `start_typing(thread_id, status)` is called with two arguments). `Thread`/`Channel.post_ephemeral` pass `options=` to `adapter.post_ephemeral` only when `chat_sdk._compat.accepts_kwarg` finds an `options` parameter (positional-or-keyword or keyword-only) or `**kwargs`; an implementation with the older `(thread_id, user_id, message)` signature is called with three arguments | `adapter.postEphemeral(threadId, userId, postable, options)` and `adapter.startTyping(threadId, status, { initiatorUserId })` always | JavaScript drops extra arguments, while Python raises `TypeError`, so passing `options=` unconditionally would break third-party adapters written against the pre-4.41 signature. In-repo adapters all accept it. Regression tests: `tests/test_thread_faithful.py::TestPostEphemeral::test_three_argument_custom_post_ephemeral_still_works`, `tests/test_channel_faithful.py::TestChannelPostEphemeral::test_three_argument_custom_post_ephemeral_still_works_from_a_channel` and `tests/test_compat.py::test_accepts_kwarg`; for `start_typing`, `tests/test_turn_cancellation.py::TestThreadAbortAndTyping::test_two_argument_custom_start_typing_still_works`. | diff --git a/scripts/fidelity_target.json b/scripts/fidelity_target.json index 2d972791..e43ea60a 100644 --- a/scripts/fidelity_target.json +++ b/scripts/fidelity_target.json @@ -8,7 +8,7 @@ "matched_exact": 889, "matched_fuzzy": 75, "missing": 100, - "extra": 530 + "extra": 536 }, "absent_ts_files": [], "files": { @@ -80,7 +80,7 @@ "matched_exact": 125, "matched_fuzzy": 5, "missing_count": 0, - "extra_count": 86, + "extra_count": 92, "missing": [], "fuzzy_matches": [ ["handles empty AST", "test_fromast_handles_empty_ast"], diff --git a/src/chat_sdk/adapters/slack/adapter.py b/src/chat_sdk/adapters/slack/adapter.py index 84f22904..fd8769f7 100644 --- a/src/chat_sdk/adapters/slack/adapter.py +++ b/src/chat_sdk/adapters/slack/adapter.py @@ -3701,16 +3701,29 @@ async def ack(response_payload: dict[str, Any] | None = None) -> None: {"envelope_id": envelope_id, "error": str(exc)}, ) - # Slack re-delivers events that weren't acked in time. Skip retries - # so we don't double-process — but still ack so Slack stops resending. if retry_attempt and retry_attempt > 0: - await ack() - self._logger.debug("Skipping socket mode retry", {"retry_attempt": retry_attempt}) - return + # Slack redelivers an event when a prior delivery wasn't acked -- + # including events sent while the app had no open socket (restart, + # deploy, connection refresh). Route it like a first delivery: + # the event_id marker and ``Chat.process_message``'s message-id + # dedupe drop an already-handled duplicate, while a genuinely + # missed event is recovered instead of being lost (upstream + # vercel/chat#667). slack_sdk names upstream's ``retry_num`` + # ``retry_attempt``. Upstream-parity choice: like upstream + # ``startSocketMode`` / ``routeSocketEvent`` (adapter-slack + # index.ts:3038-3058, 3087-3135), only ``events_api`` consults the + # event-id marker; ``interactive`` / ``slash_commands`` envelopes + # are routed as-is (Slack's retry_attempt/retry_reason describe + # Events API redelivery). + self._logger.info( + "Processing socket mode retry", + { + "retry_attempt": retry_attempt, + "retry_reason": getattr(request, "retry_reason", None), + "type": event_type, + }, + ) - # ``retry_attempt`` feeds the event_id retry marker. While the skip - # above stays (#283 replaces it with upstream's "process retries"), - # only forwarded socket events reach the marker with a retry count. await self._route_socket_event(payload, event_type, ack, retry_num=_parse_retry_num(retry_attempt)) async def _route_socket_event( @@ -5516,32 +5529,19 @@ def _assemble_content( ) -> tuple[FormattedContent, str]: """Body AST with leading tables above it, then trailing tables and attachments. - Returns ``(formatted, plain_text)``. Upstream derives ``message.text`` - as ``toPlainText(formatted)``. Until #283 ports upstream's inbound - mrkdwn normalization (``slackMrkdwnToMarkdown``), our ``to_ast`` drops - code on a fence's opening line (the body ``"```npm test```"`` parses to - nothing), so the body's share of the plain text keeps the regex - ``extract_plain_text`` used before #210, and only the table and - attachment nodes go through ``ast_to_plain_text``. A message without - tables or attachments gets exactly the pre-#210 text. Otherwise the - pieces are joined with a blank line, skipping empty ones, as - ``toPlainText`` joins root children (the body loses trailing - whitespace there, as a parsed paragraph would). Temporary divergence: - #283 replaces this with ``ast_to_plain_text(formatted)``. + Returns ``(formatted, plain_text)``; the plain text is upstream's + ``text: toPlainText(formatted)`` at each ``assembleContent`` call + site, so ``message.text`` is the plain text of everything + ``message.formatted`` holds. """ - before = [self._table_node(data) for data in tables.leading] - after = [*(self._table_node(data) for data in tables.trailing), *attachment_nodes] formatted = self._format_converter.to_ast(text) - formatted["children"] = [*before, *formatted.get("children", []), *after] - body = self._format_converter.extract_plain_text(text) - if not (before or after): - return formatted, body - pieces = [ - *(ast_to_plain_text(node) for node in before), - body.rstrip(JS_WHITESPACE), - *(ast_to_plain_text(node) for node in after), + formatted["children"] = [ + *(self._table_node(data) for data in tables.leading), + *formatted.get("children", []), + *(self._table_node(data) for data in tables.trailing), + *attachment_nodes, ] - return formatted, "\n\n".join(piece for piece in pieces if piece) + return formatted, ast_to_plain_text(formatted) def _attachment_nodes(self, content: _AttachmentContent) -> list[Content]: """Render one attachment's content to block nodes (upstream ``attachmentNodes``). @@ -7402,7 +7402,17 @@ async def post_channel_message(self, channel_id: str, message: AdapterPostableMe raise ValidationError("slack", f"Invalid Slack channel ID: {channel_id}") synthetic_thread_id = f"slack:{channel}:" - return await self.post_message(synthetic_thread_id, message) + result = await self.post_message(synthetic_thread_id, message) + + # The new message roots its own thread: return a replyable thread id + # (upstream vercel/chat#720). Without a string ``ts`` (e.g. a + # file-only upload) there is nothing to address, so keep the + # channel-scoped id. + raw = result.raw + ts = raw.get("ts") if isinstance(raw, dict) else None + if not isinstance(ts, str): + return result + return replace(result, thread_id=self.encode_thread_id(SlackThreadId(channel=channel, thread_ts=ts))) # ================================================================== # Thread ID encoding / decoding diff --git a/src/chat_sdk/adapters/slack/format.py b/src/chat_sdk/adapters/slack/format.py index 66bebb6e..45c13990 100644 --- a/src/chat_sdk/adapters/slack/format.py +++ b/src/chat_sdk/adapters/slack/format.py @@ -17,6 +17,8 @@ from datetime import datetime from typing import Literal, NotRequired, TypedDict +from chat_sdk.shared._js_compat import JS_WHITESPACE as _JS_WHITESPACE + class SlackPlainTextObject(TypedDict): """A Slack ``plain_text`` composition object.""" @@ -40,7 +42,25 @@ class SlackMrkdwnTextObject(TypedDict): _DATE_CONTROL_PATTERN = re.compile(r"[\^|>]") _SLACK_ID_PATTERN = re.compile(r"^[A-Z0-9_]+$") _SLACK_USER_TOKEN_PATTERN = re.compile(r"(?]*)?>\Z") +_LABELED_GROUP_PATTERN = re.compile(r"\A]+)>\Z") +_GROUP_PATTERN = re.compile(r"\A\Z") _TEXT_OBJECT_MAX_LENGTH = 3000 +_CODE_FENCE = "```" +_LEADING_WHITESPACE_PATTERN = re.compile(r"^[ \t]+") +# Line prefixes CommonMark promotes to a block construct (blockquote, +# heading, list item, fence, thematic break, HTML) -- in pre-unescape form. +# ``[0-9]`` is JS ``\d`` (Python's ``\d`` matches every Unicode digit). +_BLOCK_MARKER_PATTERN = re.compile( + r"^(?:>|<|#{1,6}(?=[ \t\n]|\Z)|[-+*](?=[ \t\n]|\Z)|`{3,}|~{3,}|(?:[-*_][ \t]*){3,}(?=\n|\Z))" +) +_ORDERED_LIST_MARKER_PATTERN = re.compile(r"^([0-9]{1,9})([.)])(?=[ \t\n]|\Z)") +# First character that ends a ``<...>`` token scan: its close, or a line break. +_ANGLE_TOKEN_STOP = re.compile(r"[>\n\r]") +# What JS ``trimStart`` removes. +_JS_LEADING_WHITESPACE = re.compile(f"[{re.escape(_JS_WHITESPACE)}]*") def escape_slack_text(text: str) -> str: @@ -131,25 +151,250 @@ def format_slack_date( def slack_mrkdwn_to_markdown(mrkdwn: str) -> str: """Normalize Slack mrkdwn to standard Markdown. - Rewrites user/channel mentions, links, bold, and strikethrough, then + Rewrites user, channel and special mentions, links (including the + inverted ```` form), bold, and strikethrough; puts + each paired ```` ``` ```` fence on its own lines (Slack treats text right + after an opening fence as code, CommonMark as the info string); then unescapes Slack's ``&``/``<``/``>`` entities. """ - markdown = mrkdwn + markdown = _convert_mrkdwn_with_code_fences(mrkdwn) if _CODE_FENCE in mrkdwn else _convert_mrkdwn_text(mrkdwn) + return unescape_slack_text(markdown) + + +def _convert_slack_tokens(mrkdwn: str) -> str: # User mentions: <@U123|name> -> @name or <@U123> -> @U123 - markdown = re.sub(r"<@([A-Z0-9_]+)\|([^<>]+)>", r"@\2", markdown) + markdown = re.sub(r"<@([A-Z0-9_]+)\|([^<>]+)>", r"@\2", mrkdwn) markdown = re.sub(r"<@([A-Z0-9_]+)>", r"@\1", markdown) - # Channel mentions: <#C123|name> -> #name - markdown = re.sub(r"<#[A-Z0-9_]+\|([^<>]+)>", r"#\1", markdown) + # Channel mentions keep the id: <#C123|name> -> #name (C123) + markdown = re.sub(r"<#([A-Z0-9_]+)\|([^<>]+)>", r"#\2 (\1)", markdown) markdown = re.sub(r"<#([A-Z0-9_]+)>", r"#\1", markdown) + # Inverted links (frequently hallucinated): -> + markdown = re.sub(r"<(?!https?://)([^<>|]+)\|(https?://[^|<>]+)>", r"<\2|\1>", markdown) # Links: -> [text](url) markdown = re.sub(r"<(https?://[^|<>]+)\|([^<>]+)>", r"[\2](\1)", markdown) # Bare links: -> url - markdown = re.sub(r"<(https?://[^<>]+)>", r"\1", markdown) + return re.sub(r"<(https?://[^<>]+)>", r"\1", markdown) + + +def _convert_mrkdwn_text(mrkdwn: str) -> str: + markdown = _convert_slack_tokens(_convert_special_mentions(mrkdwn)) # Bold: *text* -> **text** (Slack uses single * for bold) markdown = re.sub(r"(? ~~text~~ - markdown = re.sub(r"(?`` lies in between). + Remembering the stop keeps a run of unclosed ``<`` linear instead of + rescanning to the end of the line for each one; results are identical. + """ + + __slots__ = ("_fail_until", "_text") + + def __init__(self, text: str) -> None: + self._text = text + self._fail_until = -1 + + def end(self, index: int) -> int: + """Index just past the ``>`` closing the token at *index*, or ``-1``.""" + if index < self._fail_until: + return -1 + match = _ANGLE_TOKEN_STOP.search(self._text, index + 1) + if match is not None and match.group() == ">": + return match.end() + self._fail_until = match.start() if match is not None else len(self._text) + return -1 + + +def _convert_special_mentions(mrkdwn: str) -> str: + """```` -> ``@here``, ```` -> ``@eng``; code stays literal.""" + parts: list[str] = [] + start = 0 + cursor = 0 + length = len(mrkdwn) + angle = _AngleTokenScanner(mrkdwn) + + while cursor < length: + if mrkdwn.startswith(_CODE_FENCE, cursor): + cursor += len(_CODE_FENCE) + continue + char = mrkdwn[cursor] + if char == "`": + code_end = _find_inline_code_end(mrkdwn, cursor) + cursor = cursor + 1 if code_end == -1 else code_end + continue + if char != "<": + cursor += 1 + continue + end = angle.end(cursor) + if end == -1: + cursor += 1 + continue + token = mrkdwn[cursor:end] + token = _SPECIAL_MENTION_PATTERN.sub(r"@\1", token, count=1) + token = _LABELED_GROUP_PATTERN.sub(r"@\2", token, count=1) + token = _GROUP_PATTERN.sub(r"@\1", token, count=1) + parts.append(mrkdwn[start:cursor]) + parts.append(token) + start = end + cursor = end + + parts.append(mrkdwn[start:]) + return "".join(parts) + + +def _convert_mrkdwn_with_code_fences(mrkdwn: str) -> str: + """Rewrite each paired ```` ``` ```` fence onto its own lines. + + Slack treats text immediately after an opening fence as code, while + CommonMark treats it as the fence's info string. Everything Slack renders + literally -- unpaired fences, fences inside inline code or ``<...>`` + tokens, and fences on blockquote lines -- stays plain text. Fence content + skips the emphasis rewrites so code like ``*a`` survives verbatim. + + Mirrors :func:`chat_sdk.shared.code_fences.normalize_code_fences` with + mrkdwn's entity escaping (``>`` blockquotes, ``<...>`` control + tokens), as upstream does. Keep the two in sync. + """ + parts: list[str] = [] + # ``result.length`` / ``result.endsWith("\n")`` without re-joining ``parts``. + result_len = 0 + result_ends_with_newline = False + text_start = 0 + cursor = 0 + length = len(mrkdwn) + angle = _AngleTokenScanner(mrkdwn) + quote_lines = _BlockquoteLines(mrkdwn) + # Set when the closing fence splits a line: the text after it lands at the + # start of a new line, where CommonMark would promote a leading block + # marker Slack rendered inline. + moved_to_own_line = False + + def append(value: str) -> None: + nonlocal result_len, result_ends_with_newline + if value: + parts.append(value) + result_len += len(value) + result_ends_with_newline = value.endswith("\n") + + def flush_text_before(end: int) -> None: + nonlocal moved_to_own_line + text = mrkdwn[text_start:end] + if moved_to_own_line: + text = _escape_leading_block_marker(text) + moved_to_own_line = False + append(_convert_mrkdwn_text(text)) + + while cursor < length: + char = mrkdwn[cursor] + if char == "<": + token_end = angle.end(cursor) + cursor = cursor + 1 if token_end == -1 else token_end + continue + if char != "`": + cursor += 1 + continue + if not mrkdwn.startswith(_CODE_FENCE, cursor): + span_end = _find_inline_code_end(mrkdwn, cursor) + cursor = cursor + 1 if span_end == -1 else span_end + continue + + content_start = cursor + len(_CODE_FENCE) + content_end = mrkdwn.find(_CODE_FENCE, content_start) + if content_end == -1 or quote_lines.contains(cursor): + # Slack renders an unpaired or quoted ``` literally. Upstream-parity + # choice: the fence is left as is (adapter-slack format/index.ts + # convertMrkdwnWithCodeFences), so a line-leading unpaired + # "```npm test" still opens an empty CommonMark code block there + # too. A quoted "```c```" is no fence opener (its info string has a + # backtick); ``parse_markdown`` follows that rule. + cursor = content_start + continue + + flush_text_before(cursor) + if result_len > 0 and not result_ends_with_newline: + append("\n") + content = mrkdwn[content_start:content_end] + append(_CODE_FENCE) + if not content.startswith("\n"): + append("\n") + append(_convert_slack_tokens(content)) + if not content.endswith("\n"): + append("\n") + append(_CODE_FENCE) + + cursor = content_end + len(_CODE_FENCE) + text_start = cursor + if cursor < length and mrkdwn[cursor] != "\n": + append("\n") + moved_to_own_line = True + + flush_text_before(length) + return "".join(parts) + + +def _find_inline_code_end(mrkdwn: str, index: int) -> int: + """End of the inline code span opening at *index*, or ``-1``. + + Slack inline code spans never cross line breaks. The newline search is + bounded by the close (same result as upstream's ``newline < close``) + so a long line of backticks stays linear. + """ + close = mrkdwn.find("`", index + 1) + if close == -1: + return -1 + if mrkdwn.find("\n", index + 1, close) != -1: + return -1 + return close + 1 + + +class _BlockquoteLines: + """Upstream ``isOnBlockquoteLine``, remembering the current line. + + Upstream computes ``slice(lineStart, index).trimStart().startsWith(">")`` + per fence. *index* only grows and always sits on a backtick, so the + leading-whitespace run (and thus the answer) is fixed per line: the line + start is found by scanning only the new text since the last call, and + the answer is computed once per line. Results are identical; many fences + on one long line stay linear. + """ + + __slots__ = ("_line_start", "_quoted", "_scanned", "_text") + + def __init__(self, text: str) -> None: + self._text = text + self._scanned = 0 + self._line_start = 0 + self._quoted: bool | None = None + + def contains(self, index: int) -> bool: + """Whether the line holding *index* starts (after whitespace) with ``>``.""" + newline = self._text.rfind("\n", self._scanned, index) + self._scanned = index + if newline != -1: + self._line_start = newline + 1 + self._quoted = None + if self._quoted is None: + indent = _JS_LEADING_WHITESPACE.match(self._text, self._line_start, index) + content_start = indent.end() if indent is not None else self._line_start + self._quoted = self._text.startswith(">", content_start, index) + return self._quoted + + +def _escape_leading_block_marker(text: str) -> str: + match = _LEADING_WHITESPACE_PATTERN.match(text) + whitespace = match.group(0) if match else "" + # Collapse the leading separator so it cannot become an indented code + # block, then defuse any block marker now sitting at the line start. + prefix = " " if whitespace else "" + rest = text[len(whitespace) :] + if _BLOCK_MARKER_PATTERN.match(rest): + return f"{prefix}\\{rest}" + return prefix + _ORDERED_LIST_MARKER_PATTERN.sub(r"\1\\\2", rest, count=1) def markdown_bold_to_slack_mrkdwn(markdown: str) -> str: diff --git a/src/chat_sdk/adapters/slack/format_converter.py b/src/chat_sdk/adapters/slack/format_converter.py index 24cf812d..dfb7716c 100644 --- a/src/chat_sdk/adapters/slack/format_converter.py +++ b/src/chat_sdk/adapters/slack/format_converter.py @@ -13,9 +13,9 @@ from __future__ import annotations -import re from typing import Any +from chat_sdk.adapters.slack.format import slack_mrkdwn_to_markdown from chat_sdk.emoji import convert_emoji_placeholders from chat_sdk.shared.base_format_converter import ( BaseFormatConverter, @@ -55,30 +55,14 @@ def from_ast(self, ast: Root) -> str: return stringify_markdown(ast) def to_ast(self, platform_text: str) -> Root: - """Parse Slack mrkdwn into an AST. Used for incoming ``message`` events.""" - markdown = platform_text + """Parse Slack mrkdwn into an AST. Used for incoming ``message`` events. - # User mentions: <@U123|name> -> @name or <@U123> -> @U123 - markdown = re.sub(r"<@([A-Z0-9_]+)\|([^<>]+)>", r"@\2", markdown) - markdown = re.sub(r"<@([A-Z0-9_]+)>", r"@\1", markdown) - - # Channel mentions: <#C123|name> -> #name - markdown = re.sub(r"<#[A-Z0-9_]+\|([^<>]+)>", r"#\1", markdown) - markdown = re.sub(r"<#([A-Z0-9_]+)>", r"#\1", markdown) - - # Links: -> [text](url) - markdown = re.sub(r"<(https?://[^|<>]+)\|([^<>]+)>", r"[\2](\1)", markdown) - - # Bare links: -> url - markdown = re.sub(r"<(https?://[^<>]+)>", r"\1", markdown) - - # Bold: *text* -> **text** (Slack uses single * for bold) - markdown = re.sub(r"(? ~~text~~ - markdown = re.sub(r"(? str: return convert_emoji_placeholders(self._ast_to_mrkdwn(message.ast), "slack") return "" - # ------------------------------------------------------------------------- - # Overrides - # ------------------------------------------------------------------------- - - def extract_plain_text(self, platform_text: str) -> str: - """Extract plain text from Slack mrkdwn by stripping formatting.""" - text = platform_text - - # Remove user mentions formatting: <@U123|name> -> @name, <@U123> -> @U123 - text = re.sub(r"<@([A-Z0-9_]+)\|([^<>]+)>", r"@\2", text) - text = re.sub(r"<@([A-Z0-9_]+)>", r"@\1", text) - - # Remove channel mentions: <#C123|name> -> #name - text = re.sub(r"<#[A-Z0-9_]+\|([^<>]+)>", r"#\1", text) - text = re.sub(r"<#([A-Z0-9_]+)>", r"#\1", text) - - # Remove links formatting: -> text, -> url - text = re.sub(r"<(https?://[^|<>]+)\|([^<>]+)>", r"\2", text) - text = re.sub(r"<(https?://[^<>]+)>", r"\1", text) - - # Remove bold/italic/strikethrough markers - text = re.sub(r"\*([^*]+)\*", r"\1", text) - text = re.sub(r"_([^_]+)_", r"\1", text) - text = re.sub(r"~([^~]+)~", r"\1", text) - - return text - # ------------------------------------------------------------------------- # Private helpers # ------------------------------------------------------------------------- diff --git a/src/chat_sdk/shared/markdown_parser.py b/src/chat_sdk/shared/markdown_parser.py index 5a3c9c51..6712177b 100644 --- a/src/chat_sdk/shared/markdown_parser.py +++ b/src/chat_sdk/shared/markdown_parser.py @@ -312,11 +312,28 @@ def _restore_escapes_as_literal_pair(text: str) -> str: # future surface ever feeds untrusted markdown with adversarial # bracket counts, switch to a character-level walker for link/image # content (the rest of `_parse_inline` is bounded by message size). +_LINK_LABEL_ATOM = r"[^\[\]﷐`]|﷐.|`[^`]*`|`(?![^`]*`)" + _INLINE_PATTERNS = [ # Images: ![alt](url) or ![alt](url "title") ("image", re.compile(r'(? str: ("delete", re.compile(r"(? list[Conten # Patterns used by the block parser _HEADING_RE = re.compile(r"^(#{1,6})\s+(.*)") -_THEMATIC_BREAK_RE = re.compile(r"^([-*_]\s*){3,}\s*$") -_FENCED_CODE_START_RE = re.compile(r"^(`{3,}|~{3,})(.*)") +# CommonMark: three or more of the *same* marker (``-_-`` is text, not a break). +# Lines are split on ``\n`` only, so a CRLF line keeps a trailing ``\r``. +_THEMATIC_BREAK_RE = re.compile(r"^([-*_])(?:[ \t]*\1){2,}[ \t]*\r?$") +# CommonMark: a backtick fence's info string may not contain a backtick, so +# "```npm test```" on one line is a (code span) paragraph, not a fence. +_FENCED_CODE_START_RE = re.compile(r"^(`{3,}(?=[^`]*$)|~{3,})(.*)") _BLOCKQUOTE_RE = re.compile(r"^>\s?(.*)") _ORDERED_LIST_RE = re.compile(r"^(\d+)[.)]\s+(.*)") _UNORDERED_LIST_RE = re.compile(r"^[-*+]\s+(.*)") @@ -685,6 +709,18 @@ def parse_markdown(text: str) -> Root: Returns a Root dict ``{"type": "root", "children": [...]}``. """ + return _parse_blocks(text, 0) + + +# Divergence from upstream -- see docs/UPSTREAM_SYNC.md. Blockquotes parse +# recursively; past this depth the rest stays literal text so ``> > > ...`` +# from untrusted input cannot raise ``RecursionError`` (remark has no cap). +# One frame per level; 100 (markdown-it's default ``maxNesting``) leaves most +# of Python's default recursion limit (1,000) for nested lists inside the quote. +_MAX_BLOCKQUOTE_DEPTH = 100 + + +def _parse_blocks(text: str, quote_depth: int) -> Root: children: list[Content] = [] lines = text.split("\n") i = 0 @@ -761,7 +797,11 @@ def parse_markdown(text: str) -> Root: else: break # Recursively parse blockquote content - bq_ast = parse_markdown("\n".join(bq_lines)) + bq_text = "\n".join(bq_lines) + if quote_depth >= _MAX_BLOCKQUOTE_DEPTH: + children.append(make_blockquote([make_paragraph([make_text(bq_text)])])) + continue + bq_ast = _parse_blocks(bq_text, quote_depth + 1) children.append(make_blockquote(bq_ast.get("children", []))) continue @@ -809,7 +849,10 @@ def parse_markdown(text: str) -> Root: para_lines.append(next_line) i += 1 - children.append(make_paragraph(_parse_inline("\n".join(para_lines)))) + # CommonMark strips each paragraph line's leading spaces/tabs and the + # paragraph's trailing ones (``"run \n```"`` reads ``run``). + para_text = "\n".join(para_line.lstrip(" \t") for para_line in para_lines).rstrip(" \t") + children.append(make_paragraph(_parse_inline(para_text))) return make_root(children) diff --git a/tests/test_channel_faithful.py b/tests/test_channel_faithful.py index 2a59a850..493b8f73 100644 --- a/tests/test_channel_faithful.py +++ b/tests/test_channel_faithful.py @@ -1638,9 +1638,10 @@ async def test_should_encode_callbackurl_when_editing_a_sent_card(self): # Python-specific divergence (docs/UPSTREAM_SYNC.md): an edited channel # card binds its tokens to the channel, not to the reported thread id. # Round trip with the real Slack id functions and the real block_actions - # click, both for the synthetic `slack:C…:` post id Python reports today - # and for the `slack:C…:` id upstream 92530dd3 reports (#283). A DM - # click reports no ts, so a thread scope would miss it either way. + # click, both for the `slack:C…:` id `post_channel_message` reports + # (upstream 92530dd3, #283) and for the synthetic `slack:C…:` id it keeps + # when the response has no string ts. A DM click reports no ts, so a + # thread scope would miss it either way. @pytest.mark.asyncio @pytest.mark.parametrize( ("channel_id", "post_thread_id"), diff --git a/tests/test_markdown_faithful.py b/tests/test_markdown_faithful.py index 92775429..9afb87d9 100644 --- a/tests/test_markdown_faithful.py +++ b/tests/test_markdown_faithful.py @@ -7,6 +7,8 @@ import re +import pytest + from chat_sdk.cards import ( Actions, Button, @@ -269,6 +271,16 @@ def test_link_url_resolves_backslash_escapes(self): assert len(link_nodes2) == 1 assert link_nodes2[0]["url"] == "u*r*l" + def test_link_url_keeps_escaped_and_balanced_parens(self): + # An escaped `(` never needs a partner; a bare one must be balanced. + for text, url in [ + (r"[manual](https://example.com/a\(b)", "https://example.com/a(b"), + ("[Foo](https://en.wikipedia.org/wiki/Foo_(bar))", "https://en.wikipedia.org/wiki/Foo_(bar)"), + ]: + links = [c for c in self._para_children(text) if c.get("type") == "link"] + assert [link["url"] for link in links] == [url] + assert [c["type"] for c in self._para_children("[x](https://a.com/a(b)")] == ["text"] + def test_inline_code_contents_are_not_unescaped(self): # Per CommonMark, backslash inside `code` is literal. children = self._para_children(r"a `\*literal\*` b") @@ -649,9 +661,12 @@ def row(value: str) -> Content: assert ast_to_plain_text(table) == "\x1c\na" def test_deeply_nested_blockquote_does_not_overflow_the_stack(self): - # One frame per nesting level: a 600-deep blockquote (a ~600-byte - # inbound comment) parses fine, so extraction must not RecursionError. - assert ast_to_plain_text(parse_markdown(">" * 600 + " x")) == "x" + # Extraction walks iteratively, so a 600-deep blockquote AST must not + # RecursionError. (parse_markdown itself caps quote nesting at 100.) + node: Content = {"type": "paragraph", "children": [{"type": "text", "value": "x"}]} + for _ in range(600): + node = {"type": "blockquote", "children": [node]} + assert ast_to_plain_text({"type": "root", "children": [node]}) == "x" def test_deeply_nested_list_does_not_overflow_the_stack(self): text = "\n".join(" " * depth + "- a" for depth in range(300)) @@ -1496,6 +1511,63 @@ def test_handles_markdown_with_thematic_break_hr(self): assert "thematicBreak" in types +class TestCommonMarkRulesExposedBySlackText: + """Python-only: CommonMark rules the shared parser follows since #283 made + Slack's ``message.text`` the plain text of ``parse_markdown``. Expected + values are remark-parse + remark-gfm ``toPlainText`` output.""" + + @pytest.mark.parametrize( + ("markdown", "expected"), + [ + # Links never nest: the inner link wins, outer brackets stay text. + ("[[a](https://x.com) b](https://y.com)", "[a b](https://y.com)"), + ("[a [b](https://x.com)](https://y.com)", "[a b](https://y.com)"), + # Code spans bind tighter than link brackets. + ("[`[`](https://example.com)", "["), + ("[a `]` b](https://e.com)", "a ] b"), + ("[`a` [b] `c`](https://u.com)", "a [b] c"), + # A thematic break repeats one marker; mixed markers are text. + ("-_-", "-_-"), + ("ok\n-_-", "ok\n-_-"), + ("*-*", "-"), + ("_*_", "*"), + ("- - -", ""), + ("_ _ _", ""), + # ``_`` emphasis is never intraword. + ("my_var and snake_case_name", "my_var and snake_case_name"), + ("see my_notes: [link](https://e.com/?utm_source=x)", "see my_notes: link"), + ("x _y_z", "x _y_z"), + ("(_emph_)", "(emph)"), + ("_a_b_", "a_b"), + # Paragraph lines lose leading spaces, the paragraph its trailing ones. + (" lead\n b ", "lead\nb"), + ("run \n```\nnpm test\n```\n please", "run\n\nnpm test\n\nplease"), + ], + ) + def test_plain_text_matches_remark(self, markdown: str, expected: str): + assert ast_to_plain_text(parse_markdown(markdown)) == expected + + def test_code_span_with_a_bracket_stays_inside_its_link(self): + para = parse_markdown("[`[`](https://example.com)")["children"][0] + assert para["children"] == [ + {"type": "link", "url": "https://example.com", "children": [{"type": "inlineCode", "value": "["}]} + ] + + def test_crlf_thematic_break_is_still_a_break(self): + types = [c["type"] for c in parse_markdown("before\r\n***\r\nafter")["children"]] + assert types == ["paragraph", "thematicBreak", "paragraph"] + + def test_nested_link_label_keeps_only_the_inner_link(self): + para = parse_markdown("[[a](https://x.com) b](https://y.com)")["children"][0] + links = [c for c in para["children"] if c.get("type") == "link"] + assert [link["url"] for link in links] == ["https://x.com"] + + def test_underscored_url_after_snake_case_stays_one_link(self): + para = parse_markdown("my_var see [page](https://example.com/my_page)")["children"][0] + assert [c["type"] for c in para["children"]] == ["text", "link"] + assert para["children"][1]["url"] == "https://example.com/my_page" + + # ============================================================================ # Backup absorbers for false-positive "\n" matches in verify script. # The TS file contains `result.split("\n")` which the verify script's regex diff --git a/tests/test_slack_format.py b/tests/test_slack_format.py index bd2c815f..f4b6335b 100644 --- a/tests/test_slack_format.py +++ b/tests/test_slack_format.py @@ -10,6 +10,7 @@ from __future__ import annotations from chat_sdk.adapters.slack.format_converter import SlackFormatConverter +from chat_sdk.shared.markdown_parser import ast_to_plain_text # --------------------------------------------------------------------------- # toMarkdown (mrkdwn -> markdown) @@ -20,6 +21,97 @@ class TestToMarkdown: def setup_method(self): self.converter = SlackFormatConverter() + # -- incoming code fences (vercel/chat#843) ----------------------------- + + def test_preserves_code_starting_immediately_after_the_opening_fence(self): + ast = self.converter.to_ast("```first line\nsecond line\n```") + code = ast["children"][0] + assert code["type"] == "code" + assert code.get("lang") is None + assert code.get("meta") is None + assert code["value"] == "first line\nsecond line" + assert ast_to_plain_text(ast) == "first line\nsecond line" + + def test_parses_a_code_block_pasted_into_a_message(self): + ast = self.converter.to_ast("Here you go:\n```first line\nsecond line```") + assert ast["children"][0]["type"] == "paragraph" + code = ast["children"][1] + assert code["type"] == "code" + assert code.get("lang") is None + assert code["value"] == "first line\nsecond line" + + def test_does_not_swallow_text_after_an_unpaired_fence(self): + ast = self.converter.to_ast("use ``` to fence code, *see*?") + assert [node["type"] for node in ast["children"]] == ["paragraph"] + assert ast_to_plain_text(ast) == "use ``` to fence code, see?" + + def test_keeps_a_quoted_fence_inside_the_blockquote(self): + ast = self.converter.to_ast("> a ```c``` b") + assert [node["type"] for node in ast["children"]] == ["blockquote"] + quote = ast["children"][0] + assert [node["type"] for node in quote["children"]] == ["paragraph"] + # Upstream also asserts toPlainText == "a c b": remark reads ```c``` as + # a triple-backtick code span. The shared Python parser only knows + # single-backtick spans (known parser limitation, not Slack-specific), + # so assert what the Slack layer controls: the quoted fence stays + # inline text inside the blockquote rather than becoming a code block. + assert ast_to_plain_text(ast).replace("`", "") == "a c b" + + def test_keeps_a_line_leading_quoted_fence_as_text(self): + """Python parser fix: CommonMark forbids a backtick in a backtick + fence's info string, so a quote that starts with ```npm test``` is a + paragraph (remark: a code span), not an empty code block that drops + the command from ``message.text``.""" + ast = self.converter.to_ast("> ```npm test```") + assert [node["type"] for node in ast["children"]] == ["blockquote"] + assert [node["type"] for node in ast["children"][0]["children"]] == ["paragraph"] + # Upstream: "npm test" (multi-backtick code span, a shared-parser gap). + assert ast_to_plain_text(ast).replace("`", "") == "npm test" + + def test_deeply_nested_quotes_do_not_exhaust_the_stack(self): + """Python-specific guard: ``>`` now unescapes, so 1,100 quote + markers reach the recursive blockquote parser. Past 100 levels the + rest stays literal text instead of raising ``RecursionError``.""" + ast = self.converter.to_ast(">" * 1_100 + " hi") + assert ast_to_plain_text(ast) == ">" * (1_100 - 101) + " hi" + # A heading inside the quotes must not reset the nesting budget. + ast = self.converter.to_ast(">" * 50 + " # h\n" + ">" * 1_100 + " hi") + assert ast_to_plain_text(ast) == "h\n" + ">" * (1_100 - 101) + " hi" + # Lists inside deep quotes keep their own stack room. + ast = self.converter.to_ast(">" * 700 + "- " * 160 + "hi") + assert ast_to_plain_text(ast) == ">" * (700 - 101) + "- " * 160 + "hi" + + def test_keeps_labelled_links_with_brackets_or_parens_readable(self): + """Shared parser fix: link text may hold balanced brackets and the + destination balanced parens (CommonMark), so the converter's + ``[label](url)`` output reads as upstream's plain text.""" + ast = self.converter.to_ast("See ") + assert ast_to_plain_text(ast) == "See build [failed]" + ast = self.converter.to_ast("See ") + assert ast_to_plain_text(ast) == "See Foo" + link = ast["children"][0]["children"][1] + assert (link["type"], link["url"]) == ("link", "https://en.wikipedia.org/wiki/Foo_(bar)") + + def test_keeps_trailing_text_after_a_code_block_as_a_paragraph(self): + ast = self.converter.to_ast("```x``` > note") + assert ast["children"][0]["type"] == "code" + assert ast["children"][0]["value"] == "x" + assert ast["children"][1]["type"] == "paragraph" + # The escaped ``\>`` reads as text, not a blockquote, and the + # paragraph's leading space is stripped (CommonMark). + assert ast_to_plain_text(ast) == "x\n\n> note" + + def test_keeps_code_content_verbatim_inside_the_fence(self): + ast = self.converter.to_ast("```int *a = *b;```") + assert ast["children"][0]["type"] == "code" + assert ast["children"][0]["value"] == "int *a = *b;" + + def test_to_ast_unescapes_slack_entities(self): + """Python parity fix: the old inline regex copy in ``to_ast`` skipped + ``unescape_slack_text``, so ``<`` reached handlers verbatim.""" + ast = self.converter.to_ast("a <b> & c") + assert ast_to_plain_text(ast) == "a & c" + def test_converts_bold(self): result = self.converter.to_markdown("Hello *world*!") assert "**world**" in result @@ -397,7 +489,9 @@ def test_extracts_bare_url(self): assert self.converter.extract_plain_text("Visit ") == "Visit https://example.com" def test_extracts_channel_mention_with_name(self): - assert self.converter.extract_plain_text("Join <#C123|general>") == "Join #general" + # Labeled channel tokens keep the id (vercel/chat#756); extract_plain_text + # is the base to_ast + ast_to_plain_text (upstream has no Slack override). + assert self.converter.extract_plain_text("Join <#C123|general>") == "Join #general (C123)" def test_extracts_bare_channel_mention(self): assert self.converter.extract_plain_text("Join <#C123>") == "Join #C123" diff --git a/tests/test_slack_format_primitives.py b/tests/test_slack_format_primitives.py index 24dcfff7..539e4959 100644 --- a/tests/test_slack_format_primitives.py +++ b/tests/test_slack_format_primitives.py @@ -113,12 +113,145 @@ def test_normalizes_slack_mrkdwn_to_markdown(self): slack_mrkdwn_to_markdown( "Hey <@U123|jane> in <#C123|general>, see and *bold* ~done~" ) - == "Hey @jane in #general, see [this](https://example.com) and **bold** ~~done~~" + == "Hey @jane in #general (C123), see [this](https://example.com) and **bold** ~~done~~" ) + # -- special mentions (vercel/chat#960) -------------------------------- + + def test_normalizes_special_mentions_and_user_groups(self): + assert ( + slack_mrkdwn_to_markdown(" ") + == "@here @channel @everyone @devs @S456" + ) + + @pytest.mark.parametrize( + "token", + ["", "", "", "", ""], + ) + def test_preserves_inside_code_while_converting_surrounding_mentions(self, token: str): + assert slack_mrkdwn_to_markdown(f" `{token}` ") == f"@here `{token}` @channel" + assert slack_mrkdwn_to_markdown(f" ```{token}``` ") == f"@here \n```\n{token}\n```\n @channel" + + def test_converts_special_mentions_around_multiple_inline_code_spans(self): + assert ( + slack_mrkdwn_to_markdown("`` `` ") + == "`` @channel `` @devs" + ) + + def test_does_not_treat_an_unmatched_backtick_as_a_code_span(self): + assert slack_mrkdwn_to_markdown("use ` then ") == "use ` then @here" + assert slack_mrkdwn_to_markdown("`first\n `last") == "`first\n@here `last" + + def test_preserves_escaped_special_mentions(self): + assert slack_mrkdwn_to_markdown("<!here> ") == " @channel" + + def test_keeps_link_label_backticks_from_hiding_special_mentions(self): + assert ( + slack_mrkdwn_to_markdown(" `open") + == "[`label](https://example.com) @here `open" + ) + + def test_preserves_emphasis_across_inline_code(self): + assert slack_mrkdwn_to_markdown("*before `` after* ") == "**before `` after** @channel" + + # -- code fences (vercel/chat#843) ------------------------------------- + + def test_normalizes_slack_code_fences_for_commonmark_parsing(self): + assert slack_mrkdwn_to_markdown("```first line\nsecond line\n```") == "```\nfirst line\nsecond line\n```" + + def test_puts_slack_code_fences_on_separate_lines_from_surrounding_text(self): + assert slack_mrkdwn_to_markdown("before ```code``` after") == "before \n```\ncode\n```\n after" + + def test_keeps_an_unpaired_as_literal_text(self): + # TS: "keeps an unpaired ``` as literal text" + assert slack_mrkdwn_to_markdown("use ``` to fence code, *see*?") == "use ``` to fence code, **see**?" + + def test_keeps_a_inside_an_inline_code_span_as_literal_text(self): + # TS: "keeps a ``` inside an inline code span as literal text" + assert slack_mrkdwn_to_markdown("`use ``` here`") == "`use ``` here`" + + def test_keeps_a_on_a_blockquote_line_as_literal_text(self): + # TS: "keeps a ``` on a blockquote line as literal text" + assert slack_mrkdwn_to_markdown("> a ```c``` b") == "> a ```c``` b" + + def test_keeps_a_inside_a_link_token_as_part_of_the_label(self): + # TS: "keeps a ``` inside a link token as part of the label" + assert slack_mrkdwn_to_markdown("") == "[```code```](https://x.com)" + + def test_does_not_rewrite_emphasis_inside_fenced_code(self): + assert slack_mrkdwn_to_markdown("```int *a = *b;```") == "```\nint *a = *b;\n```" + assert slack_mrkdwn_to_markdown("```keep ~x~ raw```") == "```\nkeep ~x~ raw\n```" + + def test_still_resolves_mention_tokens_inside_fenced_code(self): + assert slack_mrkdwn_to_markdown("```ping <@U123|jane>```") == "```\nping @jane\n```" + + def test_escapes_trailing_text_that_would_become_a_block_construct(self): + assert slack_mrkdwn_to_markdown("```x``` > note") == "```\nx\n```\n \\> note" + assert slack_mrkdwn_to_markdown("```x``` # heading") == "```\nx\n```\n \\# heading" + assert slack_mrkdwn_to_markdown("```x``` - item") == "```\nx\n```\n \\- item" + assert slack_mrkdwn_to_markdown("```x``` 1. item") == "```\nx\n```\n 1\\. item" + + def test_collapses_trailing_indentation_that_would_become_indented_code(self): + assert slack_mrkdwn_to_markdown("see ```x```\tresult is 5") == "see \n```\nx\n```\n result is 5" + assert slack_mrkdwn_to_markdown("see ```x``` result is 5") == "see \n```\nx\n```\n result is 5" + + # -- channel ids and inverted links (vercel/chat#756) ------------------ + + def test_preserves_the_channel_id_for_labeled_channel_tokens(self): + assert slack_mrkdwn_to_markdown("Post in <#C042BLND6R6|general>") == "Post in #general (C042BLND6R6)" + assert slack_mrkdwn_to_markdown("Post in <#C042BLND6R6>") == "Post in #C042BLND6R6" + def test_normalizes_bare_slack_links_to_markdown_urls(self): assert slack_mrkdwn_to_markdown("See ") == "See https://example.com" + def test_normalizes_inverted_slack_link_tokens_before_markdown_conversion(self): + assert ( + slack_mrkdwn_to_markdown("See and ") + == "See [docs](https://example.com) and [A](https://a.com)" + ) + + def test_does_not_invert_links_whose_display_label_is_itself_a_url(self): + assert slack_mrkdwn_to_markdown("See ") == "See [https://b.com](https://a.com)" + + # -- Python-specific: adversarial inputs stay linear -------------------- + + def test_adversarial_inputs_return_the_expected_output(self): + """Unclosed ``<`` runs, unbalanced backticks and many quoted fences on + one line must convert (the scanners memoize failed ``<`` scans and + bound backtick/newline searches) -- asserted on output, not time.""" + n = 50_000 + assert slack_mrkdwn_to_markdown("<" * n) == "<" * n + assert slack_mrkdwn_to_markdown("`x" * 25_000 + " ") == "`x" * 25_000 + " @here" + assert slack_mrkdwn_to_markdown("` \n" * 5_000) == "` @here\n" * 5_000 + quoted = "> " + "```" * 10_000 + assert slack_mrkdwn_to_markdown(quoted) == "> " + "```" * 10_000 + assert slack_mrkdwn_to_markdown("" + "```" * 6_665 + + @pytest.mark.parametrize( + ("mrkdwn", "expected"), + [ + # The quoted-line answer resets on each new line, both ways. + ("> ```a``` b\nsee ```x``` y", "> ```a``` b\nsee \n```\nx\n```\n y"), + ("see ```x``` y\n> ```a``` b", "see \n```\nx\n```\n y\n> ```a``` b"), + (" > ```q``` x\n ```c``` y", " > ```q``` x\n \n```\nc\n```\n y"), + # JS ``trimStart`` whitespace includes NBSP before ``>``. + ("\u00a0> a ```c``` b", "\u00a0> a ```c``` b"), + # A failed ``<`` scan stops at ``\n`` / ``\r``; tokens after it still convert. + ("a < b\n ping", "a < b\n@here ping"), + ("a ", "a ", "a "), + # A ``` run is no inline-code opener for the mention pass. + ("``` `", "``` @here `"), + ], + ) + def test_memoized_scanners_match_upstream_across_lines(self, mrkdwn: str, expected: str): + """Exact upstream ``slackMrkdwnToMarkdown`` output for inputs that + exercise the scanners' remembered state (``_BlockquoteLines`` and + ``_AngleTokenScanner``) across line boundaries.""" + assert slack_mrkdwn_to_markdown(mrkdwn) == expected + def test_converts_basic_markdown_bold_to_slack_mrkdwn_bold(self): assert markdown_bold_to_slack_mrkdwn("The **domain** is example.com") == "The *domain* is example.com" diff --git a/tests/test_slack_inbound_content.py b/tests/test_slack_inbound_content.py index f1dcfd14..fff36e79 100644 --- a/tests/test_slack_inbound_content.py +++ b/tests/test_slack_inbound_content.py @@ -201,9 +201,7 @@ def test_preserves_rich_text_metadata_within_table_cells(self): # Cell tokens are emitted as mrkdwn for the converter that renders body text. assert _block_text(cell) == "<#C789> July 11 #ff0000" message = _parse(text="", blocks=[_table([cell])]) - # Upstream expects ``@S789``: rendering ```` belongs to the - # mrkdwn converter (``convertSpecialMentions``), ported by #283. - assert message.text == "#C789 July 11 #ff0000" + assert message.text == "#C789 @S789 July 11 #ff0000" def test_joins_rich_text_list_items_in_a_cell_with_newlines(self): cell = { @@ -380,10 +378,7 @@ def test_keeps_attachment_content_out_of_an_unclosed_code_fence_in_the_body(self # The unclosed fence swallows the rest of the body, but the # attachment parses in isolation and stays a paragraph. assert [child["type"] for child in message.formatted["children"]] == ["paragraph", "code", "paragraph"] - # Upstream expects "Deploy failed:\n\nTypeError: boom\n\n…": until #283 - # the body's plain text keeps the pre-#210 regex rendering (see - # ``SlackAdapter._assemble_content``), so the fence stays in ``text``. - assert message.text == "Deploy failed:\n```\nTypeError: boom\n\nDeploy status\nEnvironment: production" + assert message.text == "Deploy failed:\n\nTypeError: boom\n\nDeploy status\nEnvironment: production" def test_uses_the_fallback_when_attachment_blocks_carry_nothing_renderable(self): message = _parse( @@ -480,11 +475,9 @@ async def test_resolves_user_and_channel_mentions_in_table_cells(self): "blocks": [_table(cells)], } message = await adapter._parse_slack_message(event, THREAD_ID) - # The channel cell is a Python addition. Upstream's converter renders a - # labelled channel as ``#general (C789)`` (adapter-slack format/index.ts - # ``convertSlackTokens``); our ``to_ast`` gains that rewrite with #283, - # which updates this to "@Test Bot\t#general (C789)". - assert message.text == "@Test Bot\t#general" + # The channel cell is a Python addition; the converter renders a + # labelled channel as ``#general (C789)`` (upstream ``convertSlackTokens``). + assert message.text == "@Test Bot\t#general (C789)" client.users_info.assert_awaited_once_with(user="U_BOT") async def test_resolves_mentions_in_trailing_and_attachment_tables_in_one_wave(self): @@ -569,21 +562,27 @@ async def test_sync_and_async_paths_give_equal_content_without_mentions(self): assert parsed.text == "lead\n\nBody bold\n\ntail\n\npre\n\nline one\n\nline two\n\nx\t2" @pytest.mark.parametrize( - "text", + ("text", "expected"), [ - "```npm test```", - "run ```npm test``` please", - "can you fix this?\n```def foo():\n return 1```", - "- item\n> quoted `code`", + ("```npm test```", "npm test"), + ("can you fix this?\n```def foo():\n return 1```", "can you fix this?\n\ndef foo():\n return 1"), + ("- item\n> quoted `code`", "item\n\nquoted code"), + (" ", ""), + ("hi\n", "hi"), + # Values below are upstream's (slackMrkdwnToMarkdown + remark toPlainText). + ("run ```npm test``` please", "run\n\nnpm test\n\nplease"), + (" lead and trail ", "lead and trail"), + ("-_-", "-_-"), + ("ok\n-_-", "ok\n-_-"), + ("see my_notes: ", "see my_notes: link"), + ("my_var see ", "my_var see page"), ], ) - async def test_body_text_keeps_its_pre_283_rendering(self, text: str): - """Until #283 ports the fence normalization, the body's plain text keeps - ``extract_plain_text``; deriving it from ``to_ast`` would drop code on - a fence's opening line. Tables and attachments still append.""" + async def test_body_text_is_the_plain_text_of_formatted(self, text: str, expected: str): + """``message.text`` is ``toPlainText(formatted)`` on both parse paths: + a fence keeps its first code line, and list markers, ``>`` and + backticks drop. Tables append as further root children.""" adapter = _adapter() - expected = adapter._format_converter.extract_plain_text(text) - assert expected == text event = { "type": "message", "user": "U123", @@ -596,21 +595,7 @@ async def test_body_text_keeps_its_pre_283_rendering(self, text: str): parsed = await adapter._parse_slack_message(event, THREAD_ID) assert sync.text == parsed.text == expected with_table = adapter.parse_message({**event, "blocks": [{"type": "section"}, _table([_raw("cell")])]}) - assert with_table.text == f"{expected}\n\ncell" - - @pytest.mark.parametrize( - ("text", "blocks", "expected"), - [ - (" ", [], " "), - ("hi\n", [], "hi\n"), - (" ", [{"type": "section"}, _table([_raw("cell")])], "cell"), - ("hi\n", [{"type": "section"}, _table([_raw("cell")])], "hi\n\ncell"), - ], - ) - def test_body_whitespace_joins_like_root_children(self, text: str, blocks: list[Any], expected: str): - """Alone, the body keeps its pre-#210 text; beside other content its - trailing whitespace goes, as ``toPlainText`` joins parsed blocks.""" - assert _parse(text=text, blocks=blocks).text == expected + assert with_table.text == "\n\n".join(piece for piece in (expected, "cell") if piece) def test_does_not_surface_the_title_link_of_an_unfurl(self): message = _parse( diff --git a/tests/test_slack_inbound_mentions.py b/tests/test_slack_inbound_mentions.py index a2fab529..e6f53036 100644 --- a/tests/test_slack_inbound_mentions.py +++ b/tests/test_slack_inbound_mentions.py @@ -185,6 +185,72 @@ def test_matches_a_structured_user_element_regardless_of_bot_id_case(self): ) assert message.is_mention is True + def test_converts_special_mentions_to_readable_text(self): + message = self._adapter().parse_message( + { + "type": "message", + "user": "U123", + "channel": "C456", + "text": " and ", + "ts": "1234567890.123456", + } + ) + assert message.text == "@here and @devs" + assert [node["type"] for node in message.formatted["children"]] == ["paragraph"] + + def test_preserves_special_mention_tokens_in_an_inbound_inline_code_span(self): + text = " " + message = self._adapter().parse_message( + { + "type": "message", + "user": "U123", + "channel": "D456", + "channel_type": "im", + "text": f"review code `{text}`", + "ts": "1234567890.123456", + "blocks": _rich_text( + {"type": "text", "text": "review code "}, + {"type": "text", "text": text, "style": {"code": True}}, + ), + } + ) + assert message.text == f"review code {text}" + paragraph = message.formatted["children"] + assert [node["type"] for node in paragraph] == ["paragraph"] + assert [(c["type"], c["value"]) for c in paragraph[0]["children"]] == [ + ("text", "review code "), + ("inlineCode", text), + ] + assert message.is_mention is False + + def test_preserves_special_mention_tokens_in_an_inbound_code_block(self): + text = "review fence " + message = self._adapter().parse_message( + { + "type": "message", + "user": "U123", + "channel": "D456", + "channel_type": "im", + "text": f"```{text}```", + "ts": "1234567890.123456", + "blocks": [ + { + "type": "rich_text", + "elements": [ + { + "type": "rich_text_preformatted", + "elements": [{"type": "text", "text": text}], + "border": 0, + } + ], + } + ], + } + ) + assert message.text == text + assert [(n["type"], n["value"]) for n in message.formatted["children"]] == [("code", text)] + assert message.is_mention is False + def test_uses_the_bot_user_id_instead_of_the_app_bot_id(self): message = self._adapter().parse_message( { @@ -1077,3 +1143,29 @@ async def test_serves_email_from_the_user_cache_without_a_second_users_info_call second = await adapter._parse_slack_message(dict(self.HUMAN_EVENT), "slack:C123:1234567890.123456") assert client.users_info.await_count == 1 assert second.author.email == "alice@example.com" + + +# --------------------------------------------------------------------------- +# post_channel_message +# --------------------------------------------------------------------------- + + +class TestPostChannelMessage: + async def test_posts_to_channel_without_thread_context(self): + client = _Client() + adapter = _make_adapter(client) + result = await adapter.post_channel_message("slack:C123", "Top-level message") + assert result.id == "2222222222.000000" + assert result.thread_id == "slack:C123:2222222222.000000" + kwargs = client.chat_postMessage.call_args.kwargs + assert kwargs["channel"] == "C123" + assert kwargs.get("thread_ts") is None + + async def test_keeps_the_synthetic_thread_id_when_the_response_has_no_string_ts(self): + """Python addition: without a string ``ts`` there is no thread to + address, so the synthetic ``slack:C123:`` id is kept.""" + client = _Client() + client.chat_postMessage = AsyncMock(return_value={"ok": True, "ts": 2222222222}) + adapter = _make_adapter(client) + result = await adapter.post_channel_message("slack:C123", "Top-level message") + assert result.thread_id == "slack:C123:" diff --git a/tests/test_slack_socket_mode.py b/tests/test_slack_socket_mode.py index 89d10a46..de52a783 100644 --- a/tests/test_slack_socket_mode.py +++ b/tests/test_slack_socket_mode.py @@ -517,24 +517,81 @@ async def fake_modal_submit(*args: Any, **kwargs: Any) -> ModalResponse: assert isinstance(body_arg, dict) assert body_arg.get("response_action") == "errors" - async def test_retry_attempt_is_skipped_but_acked(self): - adapter = _make_socket_adapter() + async def test_processes_retries_like_first_deliveries_dedupe_drops_true_duplicates(self): + """Port of upstream "processes retries like first deliveries (dedupe + drops true duplicates)" (vercel/chat#667). + + A retry may be the ONLY delivery the app ever sees (the original was + sent while no socket was connected during a restart), so it is routed + normally; ``Chat.process_message`` dedupes by message id when the + original was handled. + """ + logger = MagicMock() + adapter = _make_socket_adapter(logger=logger) chat = _make_mock_chat() adapter._chat = chat request = MagicMock() request.envelope_id = "env-1" request.type = "events_api" - request.payload = {"event": {"type": "message"}} - request.retry_attempt = 2 # Slack retry — should be skipped. + request.payload = { + "event": { + "type": "message", + "channel": "C123", + "ts": "1234567890.123456", + "text": "retried", + "user": "U_USER", + } + } + request.retry_attempt = 1 + request.retry_reason = "timeout" client = MagicMock() client.send_socket_mode_response = AsyncMock() await adapter._on_socket_request(client, request) + await asyncio.sleep(0) + + assert client.send_socket_mode_response.await_count == 1 + chat.process_message.assert_called_once() + assert chat.process_message.call_args.args[1] == "slack:C123:1234567890.123456" + logger.info.assert_any_call( + "Processing socket mode retry", + {"retry_attempt": 1, "retry_reason": "timeout", "type": "events_api"}, + ) + + @pytest.mark.parametrize(("marker_seeded", "dispatched"), [(True, False), (False, True)]) + async def test_socket_retry_consults_the_event_delivered_marker(self, marker_seeded: bool, dispatched: bool): + """A live socket retry carries its count to the #268 event-id marker: + an already-dispatched ``event_id`` is acked and dropped, while a retry + whose first delivery never arrived is still processed.""" + adapter = _make_socket_adapter() + chat = _make_mock_chat() + adapter._chat = chat + if marker_seeded: + chat.get_state()._cache["slack:event-delivered:Ev1"] = True + request = MagicMock() + request.envelope_id = "env-1" + request.type = "events_api" + request.payload = { + "event_id": "Ev1", + "event": { + "type": "message", + "channel": "C123", + "ts": "1234567890.123456", + "text": "retried", + "user": "U_USER", + }, + } + request.retry_attempt = 1 + request.retry_reason = "timeout" + client = MagicMock() + client.send_socket_mode_response = AsyncMock() + + await adapter._on_socket_request(client, request) + await asyncio.sleep(0) - # Ack went out (so Slack stops resending), but no dispatch. assert client.send_socket_mode_response.await_count == 1 - assert chat.process_message.called is False + assert chat.process_message.called is dispatched async def test_unknown_event_type_acks_and_does_nothing(self): adapter = _make_socket_adapter() diff --git a/tests/test_turn_cancellation.py b/tests/test_turn_cancellation.py index ff180540..fe1a0ce3 100644 --- a/tests/test_turn_cancellation.py +++ b/tests/test_turn_cancellation.py @@ -250,7 +250,9 @@ async def source() -> AsyncIterator[str]: signal._abort() sent = await asyncio.wait_for(posting, 1) - assert sent.text == "Hello " + # ``text`` is the plain text of the parsed markdown; like remark, + # ``parse_markdown`` strips a paragraph's trailing space. + assert sent.text == "Hello" assert adapter._edit_calls[-1] == (THREAD_ID, "msg-1", PostableMarkdown(markdown="Hello ")) adapter.end_typing.assert_awaited_once_with(THREAD_ID, "active")