Fix scammer-report flow: interaction-failed buttons and lost roles on release#39
Merged
Merged
Conversation
The Clank / False report (30m) buttons on the scam report alert (and the dehoist alert's Clank button) ran the full containment path before acknowledging the interaction. When warden_contain took longer than Discord's 3-second interaction window, the token expired, the reply 404'd, and the click surfaced "This interaction failed". Defer the interaction up front and reply via followup (edit_original_response for the in-place alert update). Also surface the real reason -- e.g. an immune target or role-hierarchy problem -- instead of a generic failure.
Two distinct bugs left a released clanker without their old roles: 1. Re-clanking a user already in the tank ran the INSERT ... ON CONFLICT DO UPDATE with stored_roles = current roles -- but a contained user only wears the Clanker role, so the original roles were overwritten with an empty set and lost forever. Preserve the existing non-empty stored_roles on conflict instead. 2. Restoration used a single atomic add_roles(*all): if any saved role was managed (booster/bot/integration) or above the bot's top role, Discord rejected the whole call and nothing was restored. Filter to assignable roles and fall back to per-role adds so one bad role can't block the rest.
HiLleywyn
changed the title
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
2 participants
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.
Two bug fixes for the scammer-report / containment flow, both surfaced while testing the report buttons.
1. "This interaction failed" on the report buttons
The Clank, False report (30m), and dehoist-alert Clank buttons ran the full containment path (
warden_contain→ role edits, DB, purge, escape-room setup) before acknowledging the interaction. Discord requires acknowledgement within ~3s; when containment took longer the token expired, the reply 404'd, and the click showed "This interaction failed" with no action taken.Fix:
defer()immediately, then do the work and reply viafollowup(edit_original_responsefor the in-place alert). Failures now surface the real reason (immunity / role hierarchy) instead of a generic message. (cogs/dehoist.py)2. Roles not restored on unclank
Releasing a clanked user sometimes handed back none of their roles. Two causes:
INSERT ... ON CONFLICT DO UPDATEwithstored_roles = current roles— but a contained user only wears the Clanker role, so the original roles were overwritten with an empty set and lost. The conflict path now preserves the existing non-emptystored_roles.add_roles(*all_saved)is a single atomic call; one managed role (booster/bot/integration) or a role above the bot's top role made Discord reject the whole batch. Restore now filters to assignable roles and falls back to per-role adds so one un-assignable role can't block the rest. Un-restorable roles are logged. (cogs/clank.py)py_compileclean for both files; the SQLCASEis standard Postgres (references the existing row via the table name inON CONFLICT DO UPDATE). These are integration paths exercised against a live Discord guild.Follow-up to #38.