Skip to content

[Fix] Discord posts account-link setup clutter in public channels#524

Merged
mrubens merged 7 commits into
developfrom
fix/discord-link-prompt-dm-199c66lalmzui
Jul 19, 2026
Merged

[Fix] Discord posts account-link setup clutter in public channels#524
mrubens merged 7 commits into
developfrom
fix/discord-link-prompt-dm-199c66lalmzui

Conversation

@roomote-roomote

@roomote-roomote roomote-roomote Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Opened on behalf of Matt Rubens. Follow up by mentioning @roomote-roomote, in the web UI, or in Slack.

What changed

When an unlinked Discord user mentions Roomote (or otherwise tries to start work) in a guild channel, Roomote no longer posts the full “Link your Discord account…” setup copy into the public channel.

Instead it:

  1. Opens a DM with the full linking instructions
  2. Posts a short channel reply (I sent you a DM to link your Discord account.) as a reply to the triggering message
  3. Falls back to public instructions only when Discord returns blocked-DM error code 50007
  4. Rethrows other DM failures (including non-50007 403s and network/429/5xx) so they are not mistaken for “user blocked DMs”
  5. Dedupes link DMs across mention, slash, and auto-start entry paths with a per-user Redis slot
  6. Tracks that slot as pending while delivery is in flight and only treats it as sent after the DM posts, so concurrent mentions wait for settlement instead of acknowledging a claimed-but-undelivered DM
  7. Keeps the full prompt in an existing DM conversation

This matches the Slack/Teams account-link delivery pattern for private setup delivery.

Why this change was made

Public account-link replies clutter shared Discord channels when someone who has not linked their account pings the bot. Private delivery keeps the channel clean while still telling the invoker exactly how to link. Distinguishing in-flight vs delivered slots closes a race where a failed concurrent unravel could leave a user with neither a DM nor a proper fallback.

Impact

Unlinked pings in servers become quieter: setup instructions go to DM, and the channel only gets a brief acknowledgement after delivery is confirmed (or a full public fallback when DMs are confirmed blocked). Concurrent paths wait or retry instead of lying that a DM went out. Behavior in already-open DMs is unchanged.

Keep full setup copy out of public channels when an unlinked user pings
Roomote: open a DM with the full link prompt and acknowledge briefly in
the channel. Fall back to public instructions when DMs are blocked.
@roomote-roomote

roomote-roomote Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

No new code issues found. See task

  • apps/api/src/handlers/discord/account-link.ts:18-22 Require Discord's blocked-DM error code before using the public fallback. error.status === 403 also matches other Discord authorization failures (for example, 50001 Missing Access), so this still sends the full account-link instructions into the guild when Discord has not confirmed recipient DM blocking. Restrict the fallback to 50007 and add a non-50007 403 test that verifies no public reply is posted.
  • apps/api/src/handlers/discord/account-link.ts:104-107 Do not acknowledge a claimed-but-undelivered DM as sent. The shared slot is set before the auto-start nudge finishes posting. If a concurrent mention sees that slot and sends its "I sent you a DM" acknowledgement, then the nudge fails and releases it, the user receives neither a DM nor the mention flow's blocked-DM fallback. Keep an in-flight state distinct from a delivered DM, or retry/wait for the claimant's result before acknowledging.

Reviewed 58ebe95

Rethrow retryable Discord DM failures so the Gateway keeps the event
instead of leaking full setup instructions into the guild channel.
@roomote-roomote

Copy link
Copy Markdown
Contributor Author

Addressed the review note on transient DM failures:

  • Confirmed blocked-DM errors (50007 / recipient forbid) still fall back to the short public setup message
  • Other DM failures rethrow so the Gateway returns 503 and retries, instead of posting full link instructions in the guild channel

Covered with unit tests for blocked-DM fallback vs 503 on transient failure.

Only Discord's cannot-send-messages code treats DMs as blocked. Bare 403s
like Missing Access no longer dump full setup instructions in guild channels.
@roomote-roomote

Copy link
Copy Markdown
Contributor Author

Tightened the blocked-DM check further: public fallback now requires Discord error code 50007 only (no bare 403). A 403/50001 Missing Access path rethrows and does not post publicly; covered with a unit test.

Review feedback: the mention-flow link prompt sent a fresh DM on every
unlinked ping while the auto-start nudge next door already had a Redis
dedupe. Both paths now share one claim/release slot (24h TTL per user)
in account-link.ts, so repeated pings acknowledge the existing DM, a
Gateway retry after a partial failure cannot double-send, and any entry
path suppresses the others' duplicates. The mention flow fails open when
Redis is unavailable (a possible duplicate DM beats silence); the
best-effort nudge keeps failing closed.

Also composes the fallback instruction into the full link message so
the copy cannot drift, documents the intentional silence on permanent
non-50007 DM failures, covers the unlinked guild slash-command path
with a test, and adds the missing changeset.
@mrubens

mrubens commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Pushed f3b0a3f addressing review findings:

  • DM dedupe, shared across entry paths. promptDiscordAccountLink and the auto-start nudge now claim the same per-user Redis slot (24h TTL) via shared claimAccountLinkDmSlot/releaseAccountLinkDmSlot helpers in account-link.ts. Repeated pings get the channel ack pointing at the existing DM instead of a fresh DM, and a Gateway retry after the DM succeeded but the ack failed can no longer double-send. The failed-delivery path releases the slot so blocked or transient failures retry cleanly. The mention flow fails open when Redis is unavailable (the user explicitly asked, so a possible duplicate beats silence); the best-effort auto-start nudge keeps failing closed.
  • One isDiscordDmBlockedError. The auto-start path's looser any-403 variant is gone; both paths use the strict 50007 check from account-link.ts.
  • Copy composed, not duplicated. The full link message is built from the fallback instruction constant.
  • Intentional silence documented. The permanent non-50007 outcome (no DM, no reply) now has a comment explaining it is deliberate.
  • Interaction coverage. Added a test for an unlinked /new in a guild: DM sent, ack through the interaction reply.
  • Changeset added.

Note one behavior change from sharing the slot: a user who received the auto-start nudge within the last day gets "I sent you a DM" on a subsequent @mention rather than a second DM (and vice versa). The instructions in either DM are equivalent, so this reads as intended dedupe rather than lost information.

mrubens added 2 commits July 18, 2026 20:34
Split the per-user DM slot into pending vs sent so concurrent mention
acks never treat a claimed but undelivered DM as sent. Wait briefly for
settlement, then retry or 503 instead of a false channel acknowledgement.
@roomote-roomote

Copy link
Copy Markdown
Contributor Author

Addressed the claimed-but-undelivered race:

  • DM slots are now pending on claim and only promoted to sent after the DM actually posts (markAccountLinkDmSent)
  • Concurrent mention paths that see in_flight wait for settlement before acking; if it never turns sent, they return 503 so the Gateway can retry instead of saying "I sent you a DM"
  • Failed deliveries still release the slot
  • Branch is merged with latest develop (no remaining conflicts from earlier)

The pending slot carried the full 24h dedupe TTL, so a claimant that
crashed between claim and mark/release would wedge the slot for a day:
every later unlinked ping would wait out the in-flight window and bounce
back to the Gateway with no user-visible reply. Pending claims now
expire after 120 seconds; only a confirmed delivery holds the slot for
the full window.
@mrubens

mrubens commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

The in-flight race finding was addressed twice in parallel: I had a send-on-pending variant locally while e00c3c9 landed the wait-for-settlement approach on the branch. I kept the branch's version as the base since waiting avoids duplicate DMs entirely, and layered one fix on top in 58ebe95: pending claims now expire after 120 seconds instead of carrying the full 24h dedupe TTL.

Without that, a claimant crashing between claim and mark/release (deploy restart mid-DM) would wedge the slot as pending for a day, and every later unlinked ping would wait out the 2.5s in-flight window and bounce a synthetic 503 back to the Gateway with no user-visible reply. With the short TTL the orphaned claim evaporates within two minutes and the Gateway retry path converges instead of looping. Only a confirmed delivery holds the slot for the full 24h window.

@mrubens
mrubens marked this pull request as ready for review July 19, 2026 00:49
@mrubens
mrubens merged commit 7178998 into develop Jul 19, 2026
17 checks passed
@mrubens
mrubens deleted the fix/discord-link-prompt-dm-199c66lalmzui branch July 19, 2026 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant