Harden Party queue UX for auto-publish, named queues, and multi-queue servers - #64
Conversation
… servers (#63) Fixes the join/public-card desync where join_lobby_from_preferences edited interaction.message (often an ephemeral wizard) instead of the durable public queue card. Refactors the Discord UX around a queue-first flow: queue creation now auto-publishes to the configured Play channel, queues get a stable code and optional rename-able display name, the global "Join Queue" entry point requires an explicit choice once multiple queues are open, join collects only Primary/Secondary/Fill (captain preference stays in My Preferences), returning players join instantly from saved preferences, ready checks show who is still outstanding and hand off to a private match workspace automatically with a one-time roster ping and clickable channel mention, and organizer transfer / one-active-queue-per- player / recruiting inactivity expiry are enforced. Restart recovery now restores controls for every active queue stage, and the final Ready handoff is guarded against duplicate rooms/pings. 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
lab-godforge/utils/party_lobby.py
Line 1113 in 946d701
When the organizer manually starts a ready check with one player or any odd roster, this call succeeds and the lobby moves out of recruiting, which removes Join controls from the public card. Once everyone responds Ready, the handler says to wait for another player, but no player can join that ready check; the organizer must drop someone—potentially cancelling an empty queue—to recover. Validate the current roster as even and at least two before starting, or keep recruitment available during the check.
ℹ️ 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".
…us 5 P2s - Leaving a queue via the cross-queue "Leave That Queue" recovery action while that queue is mid-ready-check now reopens it to OPEN and resets ready state (mirrors the Ready Check Drop button), instead of leaving the queue stuck in READY_CHECK with a roster that can never all become ready. - Manual "Start Ready Check" now rejects an odd/undersized roster instead of stranding the queue outside recruiting with no way to complete. - The periodic inactivity sweep now refreshes/disables the public card for queues it just expired, instead of only transitioning them in the DB. - The one-active-queue-per-player guard now also applies to Start Queue (creating seats the organizer as a participant) and correctly catches waitlisted players, who aren't written to party_participants. - The queue-name field added to the creation modal is now actually wired into the wizard's submitted payload. - Renaming a queue now extends its recruiting-inactivity clock like other meaningful activity. 6 new regression tests added; full suite at 637 passing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RF4jAhbRnZzcgG11SXarx
Final UX cleanup pass before merge/testingThe current implementation is substantially aligned with #63 and the earlier review findings appear addressed. Before we freeze this for Discord testing, I want one last narrowly scoped cleanup pass focused only on preserving the NeatQueue-style simplicity we designed. Please do not expand scope beyond these items. 1. Simplify organizer queue creation only — do not change player role selectionThis applies only to the organizer-facing Start Queue flow. New players joining a queue must still complete the existing lightweight SMITE role picker:
Returning players with a valid saved Primary Role may continue using the existing one-click fast path with Change Roles available afterward. Target organizer path: Queue configuration that has sensible defaults should move behind Advanced Settings / Edit Details rather than being required every time. Do not remove the ability to customize queue settings; move complexity out of the default creation path. Because the organizer is automatically seated as the queue's first participant, an organizer without a saved Primary Role must complete the same lightweight role selection before the queue is created. Organizers with saved roles should use those automatically. So for a first-time organizer the effective flow is: For a returning organizer with saved roles: Do not remove, bypass, or weaken the first-time role-selection requirement. The goal is specifically to make queue configuration lightweight, not to remove SMITE role context. 2. Remove the Browse Queues / Find a Queue duplicationThe Play panel should not expose both Browse Queues and Find a Queue. Keep the primary panel minimal:
Remove Browse Queues from the normal player-facing surface. Also verify there is no remaining path where browsing an open queue renders the legacy The public queue card remains the canonical queue surface. 3. Only ping the roster when the match is readyWe previously locked the notification contract as:
The ready-check card should not create an additional notification ping just because the ready check started. It can still display the outstanding users as mentions in the embed/UI for clarity, but avoid a separate message-content roster ping that produces another notification. Expected notification behavior: Please verify retries/recovery still cannot duplicate that handoff ping. 4. Surface compact SMITE role context on the public queue cardThe public card currently shows the roster/count but should retain the SMITE-specific value of GodForge by giving players quick role context before joining. Keep this compact. Example: Exact formatting is flexible, but:
This is informational only. Do not assign/finalize team roles on the recruiting card. Scope freezeAfter these four items:
Do not add new Party features or redesign downstream formation/draft/continuity behavior in this pass. Once these are clean, the PR should be considered feature-frozen and ready for extensive manual Discord validation. |
…eady-check ping, add role context - Start Queue now asks only for an optional queue name (a one-field modal), using sensible defaults for mode/region/format/capacity/voice/skill; full customization stays available afterward via Queue Settings -> Edit Details. A first-time organizer (no saved primary role) still completes the same lightweight role picker a joining player would, since they're seated as the queue's first participant; a returning organizer with saved roles skips straight to Start. - Removed "Browse Queues" from the Play panel (duplicated "Find a Queue"); renamed "My Preferences" to "My Roles". Panel is now Start Queue / Find a Queue / My Roles. - The ready-check card no longer sends a message-content roster ping when it's first created or refreshed — outstanding players are still shown as mentions, but inside the embed's "Waiting on" field, which renders without notifying anyone. The one-time match-ready handoff remains the only roster ping in the flow. - The public queue card's roster field now shows compact per-player role context (name/mention · role, or Fill), capped at 6 rows with a "+N others" summary beyond that, instead of a bare list of mentions. 10 new regression tests added; full suite at 640 passing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RF4jAhbRnZzcgG11SXarx
|
Done in 6c38798 — all four items, tests added, full suite at 640 passing.
One interpretation to flag: "Advanced Settings" during creation itself doesn't exist as a separate step — full customization is reachable only after creation via the existing Edit Details, not as an optional expand-in-place during Start Queue. That matched your literal target flow ( CI is green on the new commit; PR is feature-frozen from my side pending your read of the above. Generated by Claude Code |
Final micro-cleanup before feature freezeOne small UX mismatch remains in the first-time role picker. We describe Secondary Role as optional, but Please make Secondary Role default to None so the true minimum first-time flow is: Secondary Role should remain available to change, but it should not require an explicit interaction when the player does not want one. This applies equally to first-time organizers using the same lightweight role picker before queue creation. Please add/update regression coverage to prove:
After this, no more Party UX changes in this PR unless Discord testing reveals an actual defect. Consider the feature frozen for manual validation. |
JoinPreferencesView required an explicit Secondary role choice (even "None") before Join/Create would succeed, contradicting its own "optional" label. Secondary role now defaults to None in view state, so Join succeeds with only Primary role and Fill answered; it remains fully selectable. Applies to both the joining-player wizard and the first-time-organizer Start Queue path, which reuses the same view. 4 new regression tests covering: Primary still required, Fill still required, Secondary defaults to None without interaction, Join succeeds without touching Secondary, and selecting a Secondary role still works. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RF4jAhbRnZzcgG11SXarx
|
Done in 630a784.
Since the first-time-organizer Start Queue path reuses this exact same view, the fix applies there too automatically — no separate change needed. Added 5 regression tests:
Full suite: 644 passing. Feature-frozen from my side per your note — ready for manual Discord validation. Generated by Claude Code |
Docs describing the Play panel, lobby creation, and join flow were stale against the Issue #63/#64 queue-first hardening (auto-publish, named/multi queues, no-Share, no-captain-in-join, single match-ready ping, etc.): - README.md's "Zero-Config Guild Setup" section described the old 4-button panel and full-config creation wizard. - docs/STANDALONE_PRODUCT_PLAN.md's Phase 1 spec (button list, creation modal fields, Share button) predates the rework; marked superseded with a pointer rather than rewritten, to preserve it as historical design intent. - VERSION_HISTORY.md's "Guided lobby cards" bullet still described captain willingness as part of lobby creation/joining. Added docs/PARTY_QUEUE_UX.md as the canonical current-behavior reference (workflow, lifecycle rules, durable storage), indexed in docs/README.md. Bumped GODFORGE_VERSION to 2.3.0-rc.3 (formatter.py, VERSION_HISTORY.md, RELEASE_PROCESS.md, README.md, web/README.md, web/DATA_CONTRACT.md, and the corresponding test) per RELEASE_PROCESS.md's own convention: this PR is behavior change beyond rc.2, and the release gate blocks tagging stable v2.3.0 until the live Discord smoke test this PR is meant for actually happens. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RF4jAhbRnZzcgG11SXarx
|
Added a documentation pass in 709f0f1 (unprompted follow-up, at the user's request in this session — not part of the earlier scoped items).
Full suite: 644 passing, unchanged. Generated by Claude Code |
…est review A dry-run of the Half-Shell adversarial review pipeline against this PR surfaced one real, narrow gap: if match-room provisioning succeeds but posting the formation card then fails right before a crash, the lobby is left in READY_CHECK with rooms already provisioned, and restart recovery does not retry completing that handoff. The pipeline's own Shredder pass argued against building auto-resume for this: it's self-healing today via one ordinary Ready re-click, and having recovery author a Discord response with no live interaction behind it adds more risk than the gap itself. Went with the smaller fix instead: log a warning when recovery finds a lobby in this exact state, so it's observable rather than silent, without changing its behavior. Two tests pin this down: one proves the stuck state now logs a warning (and confirms recovery still does NOT auto-transition it, documenting the limitation as intentional rather than accidental), the other proves an ordinary in-progress ready check with no rooms yet — the common case — never triggers a false warning. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RF4jAhbRnZzcgG11SXarx
|
Small addition in 233583a, unrelated to your last two rounds of feedback — surfaced by running an adversarial multi-agent review pipeline (a separate tool, It found one real but narrow gap: if match-room provisioning succeeds but posting the formation card then fails right before a crash, the lobby is left in READY_CHECK with rooms already live, and restart recovery doesn't retry that handoff. It's self-healing today (a player re-clicking Ready re-triggers completion normally), so I didn't build auto-resume for it — that would mean recovery authoring a Discord response with no live interaction behind it, which is more risk than the gap itself. Instead: Full suite: 646 passing. Generated by Claude Code |
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
This reverts commit e13a020.
|
Moved the setup/permissions fix ( Generated by Claude Code |
Closes #63.
Summary
Fixes the join/public-card desync bug first, then refactors the Discord Party UX around the queue-first flow defined in #63 and its follow-up comments:
Start Queue → auto-publish → Join Queue → quick SMITE 2 roles → Ready Check → automatic handoff → private match workspace. Reuses the existing durable lobby/queue/waitlist/ready-check/room/formation/draft/continuity services throughout — no rewrite of those services. This description reflects the final state after two owner-requested follow-up cleanup passes (see commit history) — it's current, not the original submission.The bug fixed first
join_lobby_from_preferences()editedinteraction.messageas if it were the public queue card. When the join came from an ephemeral wizard (the normal path), that edited the wizard message instead — durable state updated correctly, but the real public card went stale until an unrelated organizer action (e.g. Cancel) happened to redraw it. Every mutation now refreshes the queue's card exclusively via its durablelobby.delivery.panel_channel_id/panel_message_idreference (refresh_public_lobby_card), never via whichever interaction triggered it.Player-facing UX changes
@user, with "· needs 5 minutes" where relevant) instead of only a count. Starting a ready check does not itself send a notification ping — outstanding players are shown as mentions only inside the embed, which doesn't notify.<#channel>mention — no DMs, no extra organizer step. The public card also updates to show "Match forming · Continue in #match-...".Dustin · Support / Solo (Fill), capped at 6 rows with+N othersbeyond that — informational only, no team assignment happens here.Organizer-facing UX changes
shareis kept as a recoverable alias custom_id for cards posted before this change.Lifecycle / state changes
PartyLobbygainsqueue_code(stable 4-character code, deterministically derived fromlobby_id, collision-checked per guild) anddisplay_name(optional, sanitized, renameable while recruiting — never used as identity).DiscordDeliverygainsready_channel_id/ready_message_id(durable ready-check card, mirroring the existing formation-card pattern) andmatch_ready_notified(idempotency flag for the one-time roster ping).touch_recruiting_activity()extends the clock on join/leave/rename/edit/waitlist-promotion, reusing the existingexpires_at/recover_active()expiry machinery rather than a new timer system.asyncio.Lock) so a double-click or retried interaction can't provision two rooms or send two roster pings; the roster ping is additionally gated on the durablematch_ready_notifiedflag so a retry after a failed send can still deliver it.recover_match_controls) reconciles every active lifecycle stage — OPEN/FULL (repost card if missing), READY_CHECK (repost/refresh ready-check card), FORMING/ACTIVE (existing formation-card + re-sends a missed roster ping) — not just FORMING/ACTIVE as before.Schema / migration changes
Additive only,
SQLitePartyRepositoryauto-migrates on start (existing_add_columnspattern):party_lobbies.queue_code TEXT NOT NULL DEFAULT '',party_lobbies.display_name TEXT NOT NULL DEFAULT ''. Pre-existing rows with an emptyqueue_codeget one derived fromlobby_idon read (no backfill script needed).DiscordDelivery's new fields live inside the existingdelivery_jsonblob column — no schema change there.Tests
pytest tests/— 644 passed (up from 611 onmain), across the original characterization suite plustests/unit/test_party_queue_first_ux.py(35 tests) covering every #63 acceptance criterion: the desync bug fix, saved-preference fast join vs. first-time wizard, waitlist join/promotion, auto-publish on creation, rename identity-preservation, organizer transfer (manual + automatic), one-queue-per-player enforcement (including waitlisted players and at creation time), recruiting expiry + activity extending the clock, multi-queue isolation and explicit-choice routing, idempotent final-Ready handoff under concurrent retries, restart recovery across every lifecycle stage, the streamlined Start Queue path for returning/first-time organizers, no-ping ready-check start, and compact roster role context.Implementation interpretations made
/party join <code>fallback command mentioned in Refactor and harden queue UX for auto-publish, named queues, and multi-queue servers #63 as "may be added... if it fits existing command patterns" was not added — the Find-a-Queue button flow already satisfies the explicit-choice requirement, and the issue marks this optional.MatchContinuityService/MatchContinuityView(Run It Back / Shuffle Teams / Return to Queue / Invite Substitutes / Continue Series) was reused as-is; there's no literal "End Queue" button distinct from simply not choosing a continuity action.Start Queue → optional queue name → Start) rather than adding a second, redundant configuration surface before the queue exists.launch_party_draft's error handler (except Exception:referenced an undefinedexc— addedas exc).Known limitations
/party roomand the broader room/formation/draft pipeline were intentionally left untouched.Manual Discord test checklist
Setup
/party setupin a fresh test server — confirm#godforge-playand the Play panel are created/refreshed with exactly three buttons: Start Queue / Find a Queue / My Roles.Creation & auto-publish
2. Click Start Queue as a player with no saved role prefs — confirm a modal asking only for an optional queue name. Submit it (with or without a name).
3. Confirm you're then shown the lightweight Primary/Secondary/Fill picker (no mode/region/capacity/etc. questions) — submit it.
4. Confirm: an ephemeral confirmation names the queue + its code and says it's live; a public card appears in
#godforge-playimmediately with no Share click needed; it shows queue name (or<you>'s Queuefallback), code, mode/format defaults (Conquest/5v5), roster1/10with your name and role, and only Join / Leave / Queue Settings / Cancel buttons.5. Repeat Start Queue as an account that now has saved roles — confirm it skips straight from the name modal to a live queue with no role picker.
Join flow
6. As a second account with no saved role prefs, click Join Queue on the card → confirm the wizard only asks Primary (required) / Secondary (optional, pre-shows "None") / Fill (required) → submit without touching Secondary → confirm it still succeeds → confirm an ephemeral "Joined ..." ack, and the public card updates to show the new roster count and that player's role.
7. As that same account, leave and re-join with saved prefs now present → confirm it joins instantly with a "Change Roles" button on the ack, no wizard.
8. Fill the queue to one below capacity, then have a saved-prefs player join without a wizard, and confirm the queue auto-transitions to a Ready Check the moment it's full — a ready-check message posts automatically, no organizer action needed, and no one gets pinged by that message appearing.
Multi-queue
9. Start a second queue with different settings (use Queue Settings → Edit Details to change mode/region/etc. after creating it). Confirm both public cards coexist in
#godforge-playindependently.10. From the Play panel, click Find a Queue → confirm you get an explicit picker listing both queues (name/code/roster) — never an automatic join. With only one open, confirm it shortcuts straight to joining/wizard.
11. Fill/ready-check one queue to completion; confirm the other queue's card and roster are completely unaffected.
Ready check & handoff
12. With a queue full, have players click Ready one at a time; confirm the ready-check message updates its Waiting on list after each click, without re-pinging anyone.
13. Have the last player click Ready: confirm — a private match channel is created; a single message pings the full roster with a clickable
#match-...mention; the public card updates to "Match forming · Continue in #match-..."; the private channel has a formation-control message with Role Fit / Balanced / Captain Teams buttons.14. Double-click Ready rapidly as the last player (or have two people click within the same second): confirm only one private room and one roster ping are created.
Organizer actions
15. As organizer, open Queue Settings on your card → try Rename (include an
@mentionin the name — confirm it's stripped), Edit Details, Transfer Organizer (pick another participant from the Select), and Repost Queue (delete the public card manually in Discord first, then Repost — confirm it reappears and delivery updates).16. Leave a queue as organizer with other players still in it — confirm ownership silently transfers to the earliest joiner and the public card's Organizer field updates. Leave as the last remaining player — confirm the queue cancels.
Guardrails
17. Try to join a second queue while already active in one — confirm you're rejected with the existing queue's name/code and a Leave That Queue button that works, including while the other queue is mid-ready-check (confirm that queue reopens to OPEN afterward).
18. Try to Start Queue while already active in another one — confirm the same rejection instead of a second queue being created.
19. Force-expire a queue (or wait out the inactivity window in test mode) — confirm it transitions to expired and stale Join/Leave clicks are rejected with a clear message.
Restart recovery
20. Restart the bot with an OPEN queue, a READY_CHECK queue, and a FORMING match all in flight. Confirm on restart: the OPEN queue's card is still live (reposted if it had been deleted), the READY_CHECK card is still clickable, and the FORMING match's formation card + roster ping (if it hadn't gone out yet) are present.
🤖 Generated with Claude Code
https://claude.ai/code/session_014RF4jAhbRnZzcgG11SXarx
Generated by Claude Code