store: PR review: major-domo-v2#62 — maintenance mode flag (REQUEST_CHANGES)

This commit is contained in:
Cal Corum 2026-03-03 12:04:48 -06:00
parent 472b8c1920
commit 7f856ee003

View File

@ -0,0 +1,42 @@
---
id: 729df25e-abb1-4b3a-afe5-1f2b5fb8cd50
type: workflow
title: "PR review: major-domo-v2#62 — maintenance mode flag (REQUEST_CHANGES)"
tags: [pr-reviewer, major-domo-v2, discord, python, maintenance-mode, discord-py, bug]
importance: 0.6
confidence: 0.8
created: "2026-03-03T18:04:48.164825+00:00"
updated: "2026-03-03T18:04:48.164825+00:00"
---
## Review Summary
**PR #62**: `fix: implement actual maintenance mode flag in /admin-maintenance (#28)`
**Verdict**: REQUEST_CHANGES (could not post — Gitea blocks self-review)
### Key Bug Found: Double ephemeral message on maintenance gate
When `@self.tree.interaction_check` returns `False`, discord.py dispatches `CheckFailure` to the registered `@bot.tree.error` handler (`on_app_command_error`). The existing handler does NOT handle `CheckFailure`, so it falls to the `else` branch, which:
1. Logs the error as "Unhandled command error" (incorrect)
2. Sends a second ephemeral followup: "❌ An unexpected error occurred." since `is_done()` is True
Per discord.py docs: "If this function returns False, the invocation does not happen and the function's error handlers are called with a CheckFailure."
### Fix Required
Add `CheckFailure` handling in `bot.py` `on_app_command_error`:
```python
elif isinstance(error, discord.app_commands.CheckFailure):
if not interaction.response.is_done():
await interaction.response.send_message(
"❌ You don't have permission to run this command.", ephemeral=True
)
```
### Files Reviewed
- `bot.py`: Added `self.maintenance_mode: bool = False` in `__init__`, nested `maintenance_check` function registered via `@self.tree.interaction_check` in `setup_hook`
- `commands/admin/management.py`: Added `self.bot.maintenance_mode = is_enabling`, logging; rest is Black formatter reformatting
### Other Notes
- `self.bot.maintenance_mode` is not type-safe (`self.bot: commands.Bot` doesn't declare the attribute) — minor
- No tests added for the new blocking behavior
- Core logic is correct; admins pass through in maintenance mode, non-Members also blocked in DMs