fix(slack): decode bot self mention, content-based is_mention, bot author ids, email/is_system (#209)

[patrick-chinchill]

Summary

This is part (a) of #209: Slack inbound mentions and authors. The full port came to about 2k changed lines, over the issue's 1.5k split threshold, so it is split along the issue's own Metadata line. Part (b) (mrkdwn normalization, channel.post thread ids, Socket Mode retries) is #283. Its implementation and tests are already written on wip/4.41-sl4-full (unreviewed).

• Self mention: _resolve_inline_mentions now resolves the bot's own <@U_BOT> like any other user, through _lookup_user_name, and falls back to the id when the lookup fails. skip_self_mention is removed.
• Content-based is_mention: new _detect_self_mention(event, raw_text, attachments) -> bool | None, with module helpers _mention_token_pattern, _MentionMatcher, _classify_mrkdwn_mention, _classify_blocks_mention, _classify_attachment_part, and a minimal private attachment extractor (_author_attachments / _attachment_content: blocks plus mrkdwn_in parts; [4.41/SL5] Slack inbound: pasted tables and alert attachments in message content #210 extends it for rendering). Both parse paths set is_mention, and the app_mention override is deleted. It is tri-state: None when the bot id is unknown and the event is not an app_mention. The request-scoped RequestContext.bot_user_id wins in multi-workspace mode.
• Authors: user_id = user or bot_profile.user_id or bot_id or "unknown", with a guard for a non-dict bot_profile. Self detection matches user or bot_profile.user_id. email comes from the users.info cache on the async path, and is_system = user == "USLACK" on both paths. SlackEvent gains bot_profile.

Upstream commits mapped

Commit	PR	Tag	Here
51322dde	#891	chat@4.40.0	self-mention decode
683eadc1	#947	chat@4.41.0	detectSelfMention and the attachment/block classification
c2b6bff0	#883	chat@4.40.0	bot user id for the author and for self detection
bb7cd124	#716	chat@4.35.0	author.email
80def3ab	#707	chat@4.35.0	Slack half: is_system for USLACK (the core field is #192)
e71bfead, 44423bdc, c3118279, 92530dd3, 0b63791b (Slack half)			#283

Tests ported

tests/test_slack_inbound_mentions.py, 51 tests, all upstream names from adapter-slack/src/index.test.ts unless marked Python:

• 51322dde: "resolves the bot's own mention and flags it in incoming webhooks", "flags the bot's own mention in rich text table cells" (asserts is_mention only, because table text is [4.41/SL5] Slack inbound: pasted tables and alert attachments in message content #210), "falls back to the bot's user ID when users.info fails for its own mention", "resolves request-scoped self mention in multi-workspace mode".
• 683eadc1:
  • The two parse cases: "classifies the bot's mention without a user lookup" and "matches a structured user element regardless of bot id case".
  • Every "does not flag …" / "flags …" / "trusts app_mention …" case, including attachments. The raw_text cell case asserts is_mention only, because cell text is [4.41/SL5] Slack inbound: pasted tables and alert attachments in message content #210.
  • "reports false for an ordinary message …", the three DM/channel isMention false cases, and the 7 mention routing cases through a real Chat.
• c2b6bff0: "uses the bot user ID instead of the app bot ID", "matches by bot profile user ID", "matches request bot user ID by bot profile".
• bb7cd124: the 4 incoming author email cases. 80def3ab: the 4 USLACK / isSystem cases.
• Python-only:
  • A sync parse_message returns is_mention is None when the bot id is unknown.
  • The sync path uses the request-scoped bot id.
  • A malformed bot_profile falls back to bot_id.
  • A literal attachment part counts, and an unfurl attachment does not.

I checked that the new tests fail against main's adapter. 44 of the 56 tests in the full set failed there; the ones that passed are upstream regression guards whose behaviour is unchanged.

Fidelity

TS_ROOT=…/vercel-chat-4.41.1 uv run python scripts/verify_test_fidelity.py --report-target:

Delta vs committed report (HEAD): missing 151 -> 151 (+0)

(Regenerated after merging main at c3d958d.)

index.test.ts is not fidelity-mapped, and scripts/fidelity_target.json is unchanged.

Divergences

None new. Two implementation choices give the same results as upstream; both are documented in docs/UPSTREAM_SYNC.md under "Slack inbound mentions and authors":

• Mention-token matching. Upstream matches /<@!?id(?:\|[^>]*)?>/i with one regex. Here it is a prefix regex plus a tail check, because the single regex took about 1.3 s on a 50k run of unclosed <@U_BOT|. Fuzzing 20k random strings against the reference regex gave identical results.
• Block walk. The block walk uses an explicit stack instead of recursion, so a deeply nested payload cannot raise RecursionError.

Consumer impact (Slack)

• message.text / formatted decode the bot's own mention: <@U_BOT> hi → @BotName hi (or @U_BOT hi if users.info fails). Use is_mention or raw, not id-stripping.
• Mention routing follows the content. A code-only `<@bot>` that Slack sends as app_mention no longer reaches on_mention; it goes to on_message patterns. When the bot id is known, ordinary messages report a definitive is_mention=False, so a display name in plain text no longer counts. Two cases still count as a mention: an app_mention whose content never shows the known id (an Enterprise Grid W… id), and any app_mention when the bot id is unknown.
• Bot author ids: B… changes to U… when bot_profile.user_id is present.
• New fields: author.email (async path) and author.is_system (USLACK).
• Streaming: unaffected.
• Live Slack-loop check: pending. @bot must still trigger in a channel and in a DM, and a code-only `<@bot>` must not.

Validation

• ruff check and format, audit_test_quality (0 hard failures), and verify_test_fidelity --check-docs all pass.
• Strict fidelity at chat@4.31.0: 733/733.
• pytest: 6949 passed, 24 skipped (after merging main).
• pyrefly: 0 errors.

Part of #209
Part of #184

Merge gate

Review findings (2 robustness-test reviewers)

• Fixed: the Python-only mention matcher had no regression guard. New TestMentionMatcher pins the token forms (<@U_BOT2>, unclosed <@U_BOT, <@U_BOT|bot>, <@!U_BOT>, <@u_bot>, a | form with no later >). It also covers six ~50k-char adversarial inputs, asserting the output and not a timer, plus a 5000-deep nested block. The adversarial cases call _detect_self_mention directly. Going through parse_message takes about 20 s on one of them, and all of that time is spent in the shared markdown_parser._parse_inline (rendering text), not in mention detection. This PR does not touch that code.
• Fixed: the legacy attachment-part rules had no tests. New TestAttachmentPartRules covers pretext with and without mrkdwn_in, title, title + title_link (pins the escape), fields with and without mrkdwn_in, fallback-only, fallback suppressed by another part, and whitespace-only trimming.
• Fixed (parity): _attachment_content no longer returns early when the attachment has blocks. Upstream (adapter-slack/src/index.ts:929) builds parts when tables.length === 0, and without table extraction ([4.41/SL5] Slack inbound: pasted tables and alert attachments in message content #210) that gate is always open. Detection is unchanged because parts are ignored for attachments with blocks. The docstring and docs/UPSTREAM_SYNC.md now say [4.41/SL5] Slack inbound: pasted tables and alert attachments in message content #210 restores the tables gate.
• Fixed: the block rules had no tests. New TestBlockMentionRules covers a code span in a mrkdwn section, a mention in section fields, and a user element inside rich_text_preformatted, which covers both the preformatted rule and in-code inheritance.
• Mutation check: all 16 mutants named by the reviewers (tail check, | branch, last_close, block masking, fields walk, preformatted, in-code inheritance, pretext/title/escape/fields/fallback/trim, blocks early-return) are now killed by tests/test_slack_inbound_mentions.py.
• Declined: none.

gpt-6-astra: 1 round on c3d958d, verdict "No actionable regressions found."

Bots: CodeRabbit skipped the review while the PR was a draft and posted no actionable comments. No gemini comments.

CI on c3d958d: Tests (3.12, 3.13), CodeQL and Analyze passed. Lint & Type Check was skipped on the draft pull_request run, so it was run with workflow_dispatch on the branch (run 36822035866) and passed.

Local validation: ruff check/format, audit (0 hard failures), --check-docs, strict fidelity 733/733, pytest 6949 passed, and pyrefly 0 errors.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 5 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: c82c683c-cd55-4722-988a-1c159d23f90a

📥 Commits

Reviewing files that changed from the base of the PR and between dfbfde9 and 1a74e11.

📒 Files selected for processing (6)

• CHANGELOG.md
• docs/UPSTREAM_SYNC.md
• src/chat_sdk/adapters/slack/adapter.py
• src/chat_sdk/adapters/slack/types.py
• tests/test_channel_faithful.py
• tests/test_slack_inbound_mentions.py

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[patrick-chinchill]

Merge gate: CI green on 1a74e11 (Lint & Type Check, test (3.12), test (3.13), CodeQL, Analyze (python), Analyze (actions)); local Codex review (gpt-6-astra, xhigh, --base origin/main) on c3d958d: "No actionable regressions found. The full test suite passed (6,949 passed, 24 skipped), along with type checking and lint/format checks for changed Python files."; 2 astra rounds (f2cf21b, c3d958d), both clean. 1a74e11 only merges origin/main (#279 Telegram, #232 Linear); conflicts were limited to CHANGELOG.md and docs/UPSTREAM_SYNC.md, and the PR's src/tests/scripts delta vs main is byte-identical to the reviewed c3d958d delta, so no re-review was needed. Local full validation on 1a74e11: 6996 passed, strict fidelity 733/733, pyrefly 0 errors, target report 151 -> 151 (+0). Bots: CodeRabbit rate-limited (no review); no other bot findings. Merging with --admin (Protect Main requires a code-owner approval).