Replace hard queue expiry with inactivity confirmation; delete terminal cards - #69
Merged
Merged
Conversation
…al cards Closes #66. Recruiting queues (OPEN/FULL) no longer silently hard-expire after 60 quiet minutes. Instead, GodForge posts a one-time organizer-only confirmation prompt ("Still recruiting?" / Keep Queue Open / Close Queue) and starts a second 60-minute grace period. Any further meaningful activity during grace (join/leave/rename/edit/promotion) implicitly re-primes the queue and retires the now-stale prompt — the organizer never has to click anything if the queue is still alive. Only auto-closes if the grace period elapses with no response. Cancelled/auto-closed/ready-check-timed-out queues now delete their public card from #godforge-play instead of leaving a dead "cancelled"/"expired" status behind forever. A periodic self-healing sweep cleans up any card orphaned by a crash between the state transition and its card-deletion follow-up, so this stays correct across restarts with no dedicated startup reconciliation needed. READY_CHECK/FORMING/ACTIVE are explicitly out of scope for this lifecycle per the issue — ready checks already have their own separate deadline via PartyQueueService, and this also removes a pre-existing overlap where the old expire_due_recruitment() could independently hard-expire a READY_CHECK lobby off a stale OPEN/FULL-era expires_at. Implementation: - DiscordDelivery gains inactivity_prompt_channel_id/message_id; presence (not a separate flag) signals "in grace period". - touch_recruiting_activity() now also clears a stale in-flight prompt when meaningful activity resets the clock. - New store methods: due_for_inactivity_prompt() (stage 1, read-only), record_inactivity_prompt_sent() (durably starts the grace deadline), terminal_lobbies_with_undeleted_card() (restart self-heal). expire_due_recruitment() now only covers stage-2 auto-close and no longer touches READY_CHECK at all. - New InactivityPromptView (Keep Queue Open / Close Queue), organizer- gated, routed the same way every other lobby card is (embed-footer lobby_id, not custom_id) and registered persistently like RecruitingCardView/ReadyCheckView. - delete_public_lobby_card()/_delete_message_best_effort() are the new shared cleanup helpers; _touch_recruiting_activity() centralizes the "reset clock + clean up stale prompt" pairing across all 5 call sites that already reset the recruiting clock. Deferred from the issue's own "if practical" allowance: removing the lobby_id embed footer in favor of custom-ID-encoded identity. It's genuinely load-bearing for routing today (every lobby card view is a single generically-registered persistent view, not per-lobby), and the issue explicitly says not to turn this into a broad interaction rewrite. 31 new/updated tests across test_party_store.py and test_party_queue_first_ux.py: stage-1/stage-2 store methods, READY_CHECK exclusion, periodic-sweep prompt posting and auto-close, cancel/close card deletion, keep-open re-priming, organizer-only gating, implicit re-priming on activity, ready-check-timeout card deletion, restart self-heal, and duplicate-prompt prevention. Full suite: 667 passing. Version bumped to 2.3.0-rc.4 per RELEASE_PROCESS.md (rc.3's stable-tag gate is still open on live Discord/Railway validation, so this is real behavior change beyond what rc.3 represents).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d5109c5dd
ℹ️ 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".
1. (P2) delete_public_lobby_card() unconditionally cleared the panel delivery ref even when the Discord delete itself failed (Forbidden/ HTTPException). That permanently orphans the card from the one self-healing sweep (terminal_lobbies_with_undeleted_card) that could otherwise rediscover and retry it. _delete_message_best_effort() now returns whether the message is actually confirmed gone (deleted, or a 404 says it already was), and the ref is only cleared then. 2. (P1) record_inactivity_prompt_sent() unconditionally recorded the prompt and re-armed the grace deadline, even if the lobby received real activity (or changed state entirely) while channel.send() was awaited. That could silently swallow genuine activity behind a stale prompt and auto-close the queue 60 minutes later despite it. Added a compare-and-set guard (still OPEN/FULL, no prompt already recorded, expires_at unchanged since the caller last read it) — on a mismatch the caller deletes the now-unwanted message instead of recording it. 3. (P2) handle_inactivity_prompt_action() accepted a click from any message that still passed the state/organizer checks, so an older prompt left over from a failed delete (or superseded by a newer one) could still act on the current lobby. Now requires the clicked message to match lobby.delivery.inactivity_prompt_message_id exactly. 5 new tests covering the CAS guard, retry-safe delete-failure handling, and stale-prompt rejection. Full suite: 671 passing.
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.
Closes #66.
Summary
Recruiting queues (OPEN/FULL) no longer silently hard-expire after 60 quiet minutes. Instead, GodForge posts a one-time organizer-only confirmation prompt ("Still recruiting?" / Keep Queue Open / Close Queue) and starts a second 60-minute grace period. Any further meaningful activity during grace (join/leave/rename/edit/promotion) implicitly re-primes the queue and retires the now-stale prompt — the organizer never has to click anything if the queue is still alive. The queue only auto-closes if the grace period elapses with no response.
Cancelled/auto-closed/ready-check-timed-out queues now delete their public card from
#godforge-playinstead of leaving a dead "cancelled"/"expired" status behind forever. A periodic self-healing sweep cleans up any card orphaned by a crash between a state transition and its card-deletion follow-up, so this stays correct across restarts with no dedicated startup reconciliation needed.READY_CHECK/FORMING/ACTIVE are explicitly out of scope for this lifecycle per the issue — ready checks already have their own separate deadline via
PartyQueueService. This also removes a pre-existing overlap where the oldexpire_due_recruitment()could independently hard-expire a READY_CHECK lobby off a stale OPEN/FULL-eraexpires_atthat was never actually the ready-check's real deadline.Player/organizer-facing UX changes
Implementation
DiscordDeliverygainsinactivity_prompt_channel_id/inactivity_prompt_message_id; presence (not a separate flag) signals "in grace period".touch_recruiting_activity()now also clears a stale in-flight prompt when meaningful activity resets the clock.due_for_inactivity_prompt()(stage 1, read-only),record_inactivity_prompt_sent()(durably starts the grace deadline),terminal_lobbies_with_undeleted_card()(restart self-heal).expire_due_recruitment()now only covers stage-2 auto-close and no longer touches READY_CHECK at all.InactivityPromptView(Keep Queue Open / Close Queue), organizer-gated, routed the same way every other lobby card is (embed-footerlobby_id, not custom_id) and registered persistently likeRecruitingCardView/ReadyCheckView.delete_public_lobby_card()/_delete_message_best_effort()are the new shared cleanup helpers;_touch_recruiting_activity()centralizes the "reset clock + clean up stale prompt" pairing across all 5 call sites that already reset the recruiting clock.Deferred (per the issue's own "if practical" allowance)
Removing the
lobby_id=embed footer in favor of custom-ID-encoded identity. It's genuinely load-bearing for routing today — every lobby card view is a single generically-registered persistent view shared across all lobbies, not one per lobby — and the issue explicitly says not to turn this into a broad interaction rewrite.Tests
31 new/updated tests across
test_party_store.pyandtest_party_queue_first_ux.py: stage-1/stage-2 store methods, READY_CHECK exclusion, periodic-sweep prompt posting and auto-close, cancel/close card deletion, keep-open re-priming, organizer-only gating, implicit re-priming on activity, ready-check-timeout card deletion, restart self-heal, and duplicate-prompt prevention.Full suite: 667 passing.
Version
Bumped to
2.3.0-rc.4perRELEASE_PROCESS.md—rc.3's stable-tag gate is still open on live Discord/Railway validation, and this is real behavior change beyond whatrc.3represents.Manual Discord test checklist
expires_atback in the DB) past 60 minutes of no activity. Confirm exactly one prompt is posted, pinging the organizer, with Keep Queue Open / Close Queue buttons.🤖 Generated with Claude Code
Generated by Claude Code