Repository navigation
Conversation
crstian19
marked this pull request as draft
August 20, 2026 13:40
crstian19
force-pushed
the
fix/slack-webhook-validation
branch
from
October 20, 2026 08:54
ed4115f to
e19ca22
Compare
added 2 commits
October 20, 2026 11:00
Every human message posted in a tracked Discord thread is echoed as a reply under the Slack announcement, so the conversation can be followed from Slack. This needs the Web API, not the webhook. Threaded replies are addressed by the parent message's ts, and an Incoming Webhook answers "ok" without ever returning one — Slack's own docs point to the Events API or conversations.history to recover it. chat.postMessage returns it directly. - SlackApiClient posts through chat.postMessage with a bot token, reads the ok/error field inside the 200 body rather than trusting the status, and serialises posts ~1s apart to respect the per-channel rate limit. - SlackThreadMap persists discord_thread_id -> slack_ts in SQLite inside the Docker volume, so a redeploy does not orphan live threads. Purged at 30d. - The webhook stays as a fallback: with no bot token the bot still announces threads, just without mirroring, so a deploy without the token is not a regression. Skipped when mirroring: bots (the bot's own LLM answers are long, and Slack should carry the human conversation) and a forum post's opening message, which is already the body of the announcement.
Escaping and limits - build_reply_payload escapes the body once and uses it in both the block and the fallback `text`. The fallback is what Slack shows in notifications, so a raw `<!channel>` or a disguised link there defeated the escaping. - The body is escaped first and truncated afterwards, to fit Slack's 3000-char section limit (one "<" becomes "<", so 2000 raw chars could grow to 8000), and the cut never splits an entity. Mirroring - on_message runs the mirror as a background task (kept in self._bg_tasks), so a slow Slack (30 s timeout plus retries) no longer delays the bot's answer. - Replies go to the Slack channel stored with the thread mapping, not the current SLACK_CHANNEL_ID, via a new `channel` argument on post() and reply_in_thread(). - Mirrored text uses message.clean_content, so Discord mentions reach Slack as @name instead of raw <@123...> IDs. - User-visible strings in Slack are English ("[attachments: ...]"); the unreachable "(sin texto)" branch is gone. Startup and storage - SlackApiClient validates its configuration once in __init__: a token without a channel, a channel without a token, or a token that is not xoxb- disables token mode with one error, instead of silently falling back to the webhook. - A failure to store the thread mapping gets its own log line ("announced in Slack but could not store the thread mapping") instead of being reported as a failed Slack notification; nothing is stored when the announcement fails. Tests cover the on_message wiring, the background task, the ~1 s throttle and lock (sequential and concurrent posts), every retried failure (internal_error, service_unavailable, network errors, an HTML 502), the startup purge, and the fixture now closes its SQLite connection. README, AGENTS.md and .env.example describe token mode and the retry policy.
crstian19
force-pushed
the
feat/slack-thread-mirroring
branch
from
October 20, 2026 09:00
d5e65ac to
d8d81a8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What it does
Every human message posted in a tracked Discord support thread is mirrored as a reply under the thread's Slack announcement, so the whole conversation can be followed from Slack.
Product decision this PR makes: every human message in the public support threads (
SUPPORT_CHANNEL_IDS) is copied into the internal Slack channel. Bot messages and the forum opening message are not mirrored (see below). If that is not what we want, this PR should not merge as is.Why it needs a bot token
Threaded replies are addressed by the parent message's
ts, and an Incoming Webhook answersokwithout ever returning one. Slack's docs say so explicitly:chat.postMessagereturns it directly, so moving to the Web API is a requirement, not a preference.Pieces
SlackApiClient(bot/slack.py) — posts throughchat.postMessagewith a bot token.ok/errorinside the HTTP 200 body instead of trusting the status (the Web API reports failures there).ratelimited,internal_error,service_unavailable, network errors (httpxexceptions) and non-JSON bodies (e.g. an HTML 502 from a proxy), with a 0.5/1/2 s backoff (4 attempts total). Any other Slack error (not_in_channel,channel_not_found,invalid_auth, …) is permanent and is not retried.SLACK_CHANNEL_ID, a channel without a token, or a token that is notxoxb-disables token mode with one clear error in the logs (same approach as fix(slack): reject URLs that are not Incoming Webhooks #6 for the webhook URL) instead of silently falling back to the webhook.bot/thread_map.py— persistsdiscord_thread_id → (slack_ts, slack_channel)in SQLite inside the Docker volume (vector_db/slack_threads.db), so a redeploy does not orphan live threads. Purged after 30 days onon_ready.NanBot._mirror_to_slack— runs as a background task fromon_message, so a slow Slack (30 s timeout plus retries) never delays the bot's own answer in Discord. Usesclean_content(mentions arrive as@name, not<@123…>), names attachments as[attachments: …], and replies in the Slack channel stored with the thread, even ifSLACK_CHANNEL_IDchanged since.&,<,>are escaped in every user-controlled field, including the reply's fallbacktext(what Slack shows in notifications), so<!channel>/<!here>or disguised links cannot get through. The body is escaped first and truncated afterwards to stay under Slack's 3000-char section limit without splitting an entity.What is not mirrored
Bot messages (the LLM answers are long; Slack carries the human conversation) and a forum post's opening message, which is already the body of the announcement.
Merge procedure
The repo has
delete_branch_on_merge=false, so when #6 merges GitHub will not retarget this PR tomain: it would stay based onfix/slack-webhook-validation. Both PRs are merged with a merge commit (--merge, not squash), so #6's commite19ca22, which this branch is built on, lands inmainunchanged and norebase --ontois needed:gh pr merge 6 --merge.main:gh pr edit 7 --base main.git rebase origin/main+git push --force-with-leasewill do, and should apply cleanly becausemainthen containse19ca22.gh pr merge 7 --merge.CI:
ci.ymlonly runs on pull requests againstmain, so CI has not run on this PR yet. It will run for the first time after the retarget/rebase in steps 2–3; wait for it before merging.Post-merge manual steps
Deploy does
git pull+docker compose upon the server and never touches.env, so until these are done the bot keeps running in webhook mode (announcements only):chat:writeand (re)install the app to the workspace to get thexoxb-…token./invite @<app>(otherwise every post fails withnot_in_channel)..env:docker compose up -dand check the logs: noSLACK_BOT_TOKEN …/SLACK_CHANNEL_ID …error at startup, then open a test thread in#support, reply in it, and confirm the reply appears under the Slack announcement.Verification
pytest: 116 passed (fix(slack): reject URLs that are not Incoming Webhooks #6 alone: 67; this PR adds 49). Before the review fixes the branch had 90; the fixes added 26 tests.ruff check .andruff format --check .clean.on_messagemirror call, removing the throttle sleep, removing the lock, awaiting the mirror inline, ignoring the stored channel, truncating before escaping, splitting entities, skipping thexoxb-check, not retryinginternal_error/network errors/non-JSON, removing the startup purge, storing aNonets, the misleading log,contentinstead ofclean_content, …) — all 21 are killed by at least one test._MIN_INTERVAL = 1.05and a recordingasyncio.sleep: two sequential posts → the second waits ≈1.05 s; two concurrent posts viaasyncio.gathernever overlap.