fix(discord): thread-parent validation, starter routing, safe mentions/links, snapshots, guarded downloads (#229)

[patrick-chinchill]

Summary

Brings the Discord webhook adapter's correctness and security fixes up to chat@4.41.1.

• Security: thread-parent validation. post_message (including the slash-command branch), edit_message, delete_message, add_reaction, remove_reaction, start_typing and fetch_messages now confirm that a thread id's thread segment belongs to its channel segment, using a fresh 5-minute cache entry or GET /channels/{thread}. On a mismatch they raise ValidationError("discord", "Discord thread {t} does not belong to channel {p}"). A forged discord:g:A:<thread in B> can no longer slip past channel-scoped guards such as the [4.41/C3] Conversation context + AI tool scoping (read & write guards, strict_scope) #195 agent-tool scope.
• Parents are remembered from thread interactions (_encode_interaction_thread_id, a port of upstream encodeInteractionThreadId), forwarded messages and forwarded reactions. Forwarded reactions now use data["thread"] before the cache or a lookup.
• Starter messages (_with_message_channel): when the message id equals the thread segment, the call goes to the thread first and falls back to the parent channel only on Discord error 10008.
• DiscordApiError(status, body) with .code is attached to every _discord_fetch NetworkError. Recovery from 160004 checks .code instead of searching the error text for "160004".
• Outbound conversion: mentions go through the shared replace_bare_mentions scanner ([4.41/C2b] Shared text utilities: bare-mention scanner, code-fence helpers, to_plain_text structural whitespace #193), so emails, URLs, code and existing <@…>/<#…> tokens are left alone. A link whose label equals its URL renders bare. <https://…> and [t](<https://…>) keep their brackets.
• Forwarded snapshots (_flatten): handled in forwarded MESSAGE_CREATE events, parse_message/fetch_messages, and the type-21 starter's referenced_message.
• Attachments: every inbound attachment is built through the new rehydrate_attachment, with fetch_metadata={"url": url}. Its fetch_data downloads through chat_sdk.shared.download.download_attachment(url, adapter="discord"), and errors that are not already NetworkError are wrapped in one. width/height are now carried as well (upstream has had them since 4.31).

Upstream commits mapped

Commit	Upstream PR	Version	Port
490fa00e	#651	4.32.0	Emails are no longer turned into mentions
0d4e3ee4	#567	4.32.0	Label == URL renders bare
d4c52cad	#652	4.33.0	Discord adopts replace_bare_mentions
6de45723	#679	4.33.0	rehydrate_attachment
b605cf63	#726	4.35.0	Preview-suppressed links survive the round trip
4bdf7213	#825	4.38.0	_flatten snapshots
a94995e5	#800	4.38.0	Every inbound path goes through rehydrate_attachment
b6fa24c6	#865	4.39.0	Discord half: guarded downloader
c4f709fe	#815	4.39.0	_with_message_channel, DiscordApiError
b7c9316b	#875	4.40.0	Discord part: _resolve_thread_channel_id / _remember_thread_parent
61b98fca	#927	4.41.0	Webhook side only: forwarded reactions use data.thread (Gateway half N/A, #57)

Tests ported

From index.test.ts, ported into tests/test_discord_adapter.py:

• "rejects a thread ID whose target belongs to another channel"
• thread starter message routing: all 3 tests
• "should not recover when 160004 only appears elsewhere in the body"
• "parses outer and snapshot content and attachments"
• "handles a message without attachments"
• "reads content and attachments from forwarded message snapshots", against the forwarded-webhook path
• rehydrateAttachment: all 3 tests
• "uses forwarded thread info without fetching the channel"
• "preserves suppressed links in the Discord API payload"

From markdown.test.ts, ported into tests/test_discord_format.py: all 8 toAst/fromAst link and mention tests and all 6 renderPostable (mentions) tests. The existing raw-mention test was renamed to its upstream title so it isn't duplicated.

Python-specific tests:

• every outbound operation rejects a mismatched parent without making a request (parametrized, including a starter-message id)
• the slash branch is validated before the PATCH
• a stale cache entry is refetched after the TTL (fake clock)
• the cache stays bounded
• DiscordApiError code parsing: non-JSON, string code and bool code all give None
• 160004 recovery through the real _discord_fetch
• a non-starter message in a thread is not retried in the parent channel
• an attachment survives Message.to_json/from_json and still downloads after rehydration
• the downloader is called with adapter="discord" defaults, and unknown errors are wrapped in NetworkError

The starter-routing and 160004 tests go through the real _discord_fetch with a fake aiohttp session, so DiscordApiError is built the way production builds it.

Existing fixtures updated: 7 thread-op tests in test_discord_extended.py now return the parent lookup first, and assert it, as upstream does. The two 160004 fixtures now attach DiscordApiError. test_discord_final.py::test_fetches_from_thread_channel duplicated an extended test, so it now covers the cached-parent path (no lookup).

Fidelity

Delta vs committed report (HEAD): missing 198 -> 198 (+0) (after merging main). The Discord test files are not fidelity-mapped yet (#78), so scripts/fidelity_target.json is unchanged. Strict at the pin: 733/733 (730 real tests + 3 absorbers).

Divergences

Recorded in docs/UPSTREAM_SYNC.md (new "Discord correctness and security" section, plus one non-parity row):

1. Link style. The Python parser keeps no source positions and has no autolinks. [t](<https://…>) is recognized from the parsed destination: the <…> are stripped from url and the style goes in data. <https://…> stays a text node. The rendered output is the same as upstream.
2. fetch_metadata={"url": url} on inbound attachments, as the issue specifies. Upstream sets none and falls back to url. Both rehydrate the same way.

Other Python-only differences change only how many requests are made, not the results:

• the parent cache stays bounded at 1000 entries (this predates the PR)
• a forwarded message whose parent had to be looked up also caches it, as the issue scope asks
• the thread id in the lookup path is URL-quoted

Consumer impact

Discord only.

• One extra GET /channels/{thread} per thread whose parent isn't cached. Replies to inbound events usually hit the cache.
• Thread ids whose parent doesn't match now raise ValidationError.
• Deleting a text-channel thread's starter message now deletes it in the parent channel, and Discord deletes the thread with it.
• Emails, URLs and code are no longer mangled into mentions.
• Forwarded content and attachments are now included.
• Inbound attachments have fetch_data.
• A thread interaction without parent_id now encodes as discord:g:T (was discord:g:T:T), matching upstream.
• Tests that mock _discord_fetch for thread ids must return the parent lookup first, or seed _remember_thread_parent.
• During a slash command, a post_message to a different conversation now goes to that channel instead of PATCHing the interaction's @original response (upstream tryPostSlashResponse parity, a pre-existing translation bug).
• A forwarded message whose thread lacks parent_id (upstream's forwarder always sends it) is no longer encoded with a guessed parent: the parent is looked up for a thread channel_type, otherwise the channel alone is used.

Validation

ruff check and format: clean. audit_test_quality: 0 hard failures. --check-docs: OK. --strict at the pin: 733/733. pytest: 6688 passed, 24 skipped (after merging main). pyrefly: 0 errors.

Closes #229
Part of #184

Review

gpt-6-astra round 1 raised one P2: from_ast(to_ast("<@123><@456>")) returns <@123>@456. Upstream does the same (markdown.ts:106 plus :164; I ran upstream replaceBareMentions under Node at chat@4.41.1 and got "<@123>@456"). Per the parity-triage rule I added a code comment instead of new code. Round 2 passed with no actionable findings.

Merge gate

Final HEAD: 6ba22c8. Main (2aaef13) merged in with no conflicts. fidelity_target.json regenerated with no change (+0).

Independent review findings

• Fixed (parity): slash-command branch of post_message captured posts to other conversations. It now requires slash_ctx.channel_id == thread_id, as upstream tryPostSlashResponse does (index.ts:1466 at 4.41.1; also present at 4.31.0). Two reviewers reported this one. Test: test_slash_context_for_another_conversation_does_not_capture_the_post. The "every path" comment was softened.
• Fixed: _handle_forwarded_message cached a guessed parent (thread.parent_id defaulting to channel_id). In astra round 1 this became: use thread only when it carries parent_id. Test: test_forwarded_thread_without_parent_id_still_gets_replies[thread-channel-type|no-channel-type].
• Fixed (Python-only hazard): _parse_discord_error_code now also catches RecursionError from deeply nested bodies. Test: test_parses_only_a_numeric_json_code[deeply-nested].
• Fixed (coverage): added tests for the parent cached from the forwarded-message lookup (test_forwarded_thread_parent_looked_up_from_discord_is_remembered) and the URL-quoted lookup path (test_thread_segment_is_url_quoted_in_the_parent_lookup). Each new test was checked against the unfixed or mutated code, where it fails.
• Declined: none.

gpt-6-astra (this convergence pass; 2 rounds)

• Round 1 (b35ebbf): one P2. A forwarded thread without parent_id still encoded discord:g:T:T, and the new validation rejected replies to it. Fixed in 6ba22c8 with a test.
• Round 2 (6ba22c8): no actionable findings.

Bots: CodeRabbit was rate-limited (no review). There were no inline or gemini comments.

CI on 6ba22c8: Lint & Type Check, test (3.12), test (3.13), CodeQL / Analyze all pass.

Local full validation on 6ba22c8: ruff check/format clean; audit 0 hard failures; --check-docs OK; --strict 733/733; pytest 6688 passed, 24 skipped; 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 28 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: 2186d674-8efe-413d-a99f-6b387d2ce6a6

📥 Commits

Reviewing files that changed from the base of the PR and between 673fa8d and 0597609.

📒 Files selected for processing (9)

• CHANGELOG.md
• docs/UPSTREAM_SYNC.md
• src/chat_sdk/adapters/discord/adapter.py
• src/chat_sdk/adapters/discord/format_converter.py
• src/chat_sdk/adapters/discord/types.py
• tests/test_discord_adapter.py
• tests/test_discord_extended.py
• tests/test_discord_final.py
• tests/test_discord_format.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 0597609 (Lint & Type Check, test (3.12), test (3.13), CodeQL / Analyze (python, actions)). Local Codex review (gpt-6-astra, xhigh, --base origin/main) on 6ba22c8: "No actionable regressions found relative to the supplied merge base. All 681 targeted tests passed, and git diff --check was clean." The convergence pass took 2 astra rounds.

0597609 merges origin/main (#197 history API, #272 lifecycle events). The only conflict was docs/UPSTREAM_SYNC.md; I kept both sections. The src/ tests/ scripts/ diff vs main is byte-identical to the one astra reviewed. None of the newly merged modules touch Discord code, so no fresh review was needed.

Local full validation on 0597609: 6775 passed, strict fidelity 733/733 (730 real + 3 absorbers), pyrefly 0 errors, fidelity target delta +0.

Bots: CodeRabbit was rate-limited (no review). No other bot comments.

Merging with --admin (Protect Main requires a code-owner approval).