Repository navigation
Centralize AutoMod punishments and attribute report actions to the acting moderator - #292
Conversation
Co-authored-by: celveren <46062298+celveren@users.noreply.github.com>
Co-authored-by: celveren <46062298+celveren@users.noreply.github.com>
Reviewer's GuideThe PR replaces cog-private AutoMod punishment handlers with a shared, registry-based punishment module and context, then routes both AutoMod and confirmed report actions through it so moderator identity is consistently recorded in infractions, timers, events, and audit logs. Sequence diagram for moderator-attributed report punishmentsequenceDiagram
actor Moderator
participant ReportsUI
participant Apply as apply_punishment
participant Punishment as Punishment registry
participant Context as PunishmentContext
participant Discord
participant Modlog
Moderator->>ReportsUI: confirm_button
ReportsUI->>Apply: apply_punishment(punishment, message, reason, moderator)
Apply->>Punishment: select punishment type
Apply->>Context: create context with moderator
Punishment->>Discord: apply action with audit_reason
Punishment->>Context: log(action)
Context->>Modlog: add_infraction and dispatch event
Modlog-->>ReportsUI: punishment result
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
|
||
| async def add_timer(self, event: str, **kwargs: Any) -> Any: | ||
| expiry = self.expiry | ||
| cog = self.bot.get_cog("Reminders") |
There was a problem hiding this comment.
Let's raise an error when Reminders isn't unavailable (i.e. returns a None type).
There was a problem hiding this comment.
Done in 88f022b/6f1b734. PunishmentContext.get_reminders() raises TimersUnavailable when the Reminders cog is missing. Ban checks it before banning, so a timed ban can't turn into a permanent one.
| await self._permanent(ctx) | ||
|
|
||
| async def _get_role(self, ctx: PunishmentContext) -> Optional[discord.Role]: | ||
| cfg = await ctx.bot.get_cog("Moderation").get_mod_config(ctx.guild.id) # type: ignore |
There was a problem hiding this comment.
Let's avoid a one liner here.
Something like
cog = ctx.bot.get_cog("Moderation")
if not cog:
# Raise error here
cfg = await cog.get_mod_config(ctx.guild.id)
There was a problem hiding this comment.
Done in 88f022b/6f1b734. The one-liner is expanded and a new get_moderation() raises CogNotAvailable("Moderation") when the cog is missing.
|
|
||
| role = await self._get_role(ctx) | ||
| if not role: | ||
| return |
There was a problem hiding this comment.
Before, it just did nothing, and the caller couldn't tell. Now apply() and apply_punishment() return a bool: False when nothing was done (no mute role, missing permissions, failed delete), True otherwise. Changed in 88f022b/6f1b734.
|
|
||
| await func(*args, reason=self.reason) | ||
| # The moderator pressing confirm is the one responsible for this action, not the bot. | ||
| await apply_punishment(interaction.client, punishment.type, self.message, reason=self.reason, |
There was a problem hiding this comment.
If it returns None, then the punishment wasn't completed, right? If so, let's not complete the rest of the interaction.
There was a problem hiding this comment.
Done in 88f022b/6f1b734. If apply_punishment returns False or raises a LightningError, confirm_button sends an ephemeral error and returns without calling complete().
|
|
||
| async def _add_role(self, ctx: PunishmentContext, role: discord.Role) -> None: | ||
| await ctx.member.add_roles(role, reason=ctx.audit_reason) | ||
| await ctx.bot.get_cog("Moderation").add_punishment_role(ctx.guild.id, ctx.member.id, role.id) # type: ignore |
There was a problem hiding this comment.
Done in 88f022b/6f1b734. Both Moderation lookups now use the get_moderation() helper, with no one-liners left.
|
@copilot Please see the request changes and implement or comment as needed. |
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
6f1b734 to
2784e3e
Compare
| return None | ||
| if isinstance(self.duration, datetime.datetime): | ||
| return self.duration | ||
| return self.message.created_at + datetime.timedelta(seconds=self.duration) |
There was a problem hiding this comment.
I'm assuming you mean message.created_at on this line. That base time comes from the old AutoMod code. _time_ban_member and _time_mute_user did message.created_at + timedelta(seconds=seconds) for rules that store the duration as seconds. Report actions pass a ready-made datetime (from ActionDashboard.duration), which is returned as-is. If you meant something else, tell me which part. If you'd rather count from "now" (discord.utils.utcnow()) than from the triggering message, I can change it.
There was a problem hiding this comment.
Added the comment above that line in PunishmentContext.expiry in cd797b9.
Co-authored-by: celveren <46062298+celveren@users.noreply.github.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="lightning/cogs/reports/ui.py" line_range="245-249" />
<code_context>
- else:
- v, loc = "punished", "in"
- dm_message = construct_dm_message(self.message.author, v, loc, reason=self.reason,
+ dm_message = construct_dm_message(self.message.author, punishment.verb, punishment.preposition,
+ reason=self.reason,
middle=f" due to a message you posted. ({self.message.jump_url})")
await dm_user(self.message.author, dm_message)
- await func(*args, reason=self.reason)
+ # The moderator pressing confirm is the one responsible for this action, not the bot.
</code_context>
<issue_to_address>
**issue (bug_risk):** The report DM is sent before `apply_punishment` confirms that the punishment succeeded. When the action fails because of missing permissions, a missing mute role, or an unavailable timer/moderation cog, the reported member receives a notification claiming they were punished even though no punishment was applied.
**Triggers:** When `self.notify` is true and the punishment returns `False` or raises `LightningError`.
**Suggested fix:** Apply the punishment first and send the DM only after `applied` is true, or explicitly report that notification delivery is conditional on successful punishment.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and this centralizes ban, kick, mute, warning, and message-deletion execution and changes report actions to record the confirming moderator, so a defect could apply the wrong moderation action or create incorrect audit/timer records. Reverting stops future mistakes, but bans, role assignments, timeouts, and deleted messages that already occurred would require separate remediation.
Blocking findings: lightning/cogs/reports/ui.py:249
AutoMod punishments were scattered as private
_*_punishmentmethods on the cog, with separate timed variants. Message Reports reached into those privates, so every report action was recorded as the bot's even though a moderator confirmed it.automod/punishments.py(new)Punishmentsubclass per type (Delete, Warn, Kick, Ban, Mute), kept in a registry.BanandMutekeep their variants as methods: permanent, timed, and for mute either the native timeout or the role plus timer.moderatordefaults to the bot, which is what AutoMod uses.PunishmentContextcarries the moderator, duration and tracked content, so the old special-casing of mute and ban durations is gone.ctx.moderatoris used for the infraction record, the timer'smod_id, the dispatched event and the audit-log reason. When the moderator isn't the bot, the audit reason is prefixed with them. Audit reasons are truncated to 512 characters.LightningAutoModInfractionEvent.from_messagetakes an optionalmoderator. I kept this event type for reports because the modlog listener relies on.messageand.tracked_content.automod/cog.py_*_punishmentmethods, the punishment dict,_time_ban_member,_time_mute_user,can_timeoutandget_mute_role._handle_punishmentnow just builds the reason and callsapply_punishment.reports/ui.pyconfirm_buttoncallsapply_punishment(..., moderator=interaction.user)and no longer needs the AutoMod cog loaded.Punishmentclasses.Behavior changes:
Notes for review:
handle_warn_thresholdsstill has its own code path.aiohttpisn't installed in my sandbox, so I couldn't import the bot or run anything, and the changes are only compile-checked.