Harden Play channel against category moves; add /party reset - #65
Merged
Conversation
Prompted by a real incident: an admin ran /party setup, then moved #godforge-play into a different category, and it silently became inaccessible (Discord's audit log showed it as "No Access") with no way to recover other than manual server surgery. - create_play_channel() now gives the bot an explicit channel-level permission overwrite (View Channel, Send Messages, Embed Links, Read Message History) at creation time. A plain drag-to-a-new-category move doesn't touch a channel's own overwrites unless someone explicitly syncs permissions, so this survives the exact scenario that broke it — it's not bulletproof against a deliberate permission sync, but it closes the common accidental case. - New /party reset command: deletes every Discord resource /party setup is currently tracking (Play channel, room category, managed cosmetic roles) and clears the stored configuration, so a follow-up /party setup starts clean instead of hitting "channel already exists" conflicts from orphaned resources. Defaults to a dry-run preview; only deletes anything when called with confirm:True. Requires Manage Server, same as setup. Partial deletion failures (e.g. the exact permission loss that caused this) are reported but don't block clearing the stored config, so the admin isn't left stuck a second time. 12 new/updated tests. Full suite: 653 passing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RF4jAhbRnZzcgG11SXarx
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd73df0a8f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…w ones Codex review on PR #65 (P1) pointed out that _resolve_channel reuses a stored Play channel without ever calling create_play_channel, so the bot-self overwrite added there only protects deployments that run /party setup for the first time after this fix ships. Every existing guild's Play channel stays exactly as vulnerable to the category-move bug as before, and would need the new destructive /party reset just to pick up the fix. GuildSetupService.reconcile() now calls a new ensure_channel_overwrite() operation whenever it reuses a stored channel (skipped when the channel was just created, since creation already applies it atomically). The Discord adapter repairs the overwrite via channel.set_permissions() and treats failure as best-effort — the existing _panel_permission_failure() check still catches and reports any actual, current loss of visibility.
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.
Summary
Prompted by a real incident: an admin ran
/party setup, then moved#godforge-playinto a different category, and it silently became inaccessible — Discord's audit log showed it asNo Access— with no way to recover other than manual server surgery. Root-caused via the audit log: the channel wasn't deleted, it was created with zero permission overwrites of its own, so its visibility was purely inherited/ambient and a routine reorganization silently stranded it.This branched off
maindirectly (not off #64) since the owner asked for it to be scoped separately from that PR's already-frozen queue-UX changes.Player-facing UX changes
None — this only affects guild administration.
Organizer-facing UX changes
create_play_channel()now gives the bot an explicit channel-level permission overwrite (View Channel, Send Messages, Embed Links, Read Message History) at creation time. A plain drag-to-a-new-category move doesn't touch a channel's own overwrites unless someone explicitly syncs permissions, so this survives the exact scenario that broke it. Not bulletproof against a deliberate permission sync, but it closes the common accidental case./party resetcommand: deletes every Discord resource/party setupis currently tracking (Play channel, room category, managed cosmetic roles) and clears the stored configuration, so a follow-up/party setupstarts clean instead of hitting "channel already exists" conflicts from orphaned resources./party setup.confirm:True.Lifecycle / state changes
None to the party/queue domain model.
/party resetonly touches themanagedblock of per-guild settings (playChannelId,playMessageId,roomCategoryId,roleIds) —testModeand all unrelated settings are left untouched.Schema / migration changes
None.
Tests
12 new/updated tests in
tests/unit/test_party_setup_command_characterization.py:/party resetrequires a guild context and Manage Serverdiscord.DiscordException) is reported but stored config is still clearedFull suite: 618 passing (was 611 on
main).Implementation interpretations made
create_play_channel()/ensure_room_category()both reject creation if a channel/category with the same name already exists (to avoid silently adopting an unrelated resource). Actually deleting the orphaned resources is what makes/party reset→/party setupwork end-to-end without manual cleanup.confirm:Trueflag over a Discord confirm-button UI for the destructive step — simpler, and consistent with this being a rare admin-recovery command rather than a normal-operation surface.Known limitations
/party resetdoesn't touch active match rooms under the "GodForge Rooms" category (a separate lifecycle, governed bymatch_room_repository) — it only clears what/party setupitself created (Play channel/panel, room category, cosmetic roles).Manual Discord test checklist
/party setupin a fresh server. Confirm#godforge-playis created. In the channel's permission settings, confirm there's now an explicit member-level overwrite for the GodForge bot (View Channel, Send Messages, Embed Links, Read Message History), separate from any role-based permissions.#godforge-playinto a different category via drag-and-drop without clicking "Sync Permissions". Confirm the bot's Play panel still works normally (buttons respond, channel stays visible) — this is the regression this PR fixes./party reset— confirm it's rejected./party resetwith noconfirmargument (orconfirm:False). Confirm it lists the currently-configured Play channel, room category, and roles, and says nothing was changed. Verify in Discord that nothing was actually deleted./party reset confirm:True. Confirm the Play channel, room category, and all GodForge-managed roles are deleted from the server, and the response says what was deleted./party setupagain immediately after. Confirm it creates a brand-new channel/category/roles with no "already exists" conflict errors./party reset confirm:True— confirm it reports that specific deletion failure but still clears the stored config, and that/party setupafterward isn't blocked by the stale (now-cleared) reference.🤖 Generated with Claude Code
https://claude.ai/code/session_014RF4jAhbRnZzcgG11SXarx
Generated by Claude Code