fix: prevent crash when Check-In Player packs in open-packs #134

cal · 2026-03-26T13:46:14Z

cal commented

2026-03-26 13:46:14 +00:00

Summary

Filter "Check-In Player" packs from the /open-packs select menu — they're auto-opened by daily check-in and should never appear manually
Add fallback in pretty_name logic for pack type names containing hyphens (prevented empty select menu → Discord 400 error)
Replace KeyError crash in SelectOpenPack.callback with graceful user-facing message for unrecognized pack types
Change all "contact an admin" messages to "contact Cal"

Context

Based on production tag 2026.3.4 to ship as a hotfix without the refractor work on main.

Root Cause

An orphaned "Check-In Player" pack (from a failed roll_for_cards during daily check-in) caused two cascading failures:

The hyphen in "Check-In Player" bypassed pretty_name assignment → empty select options → Discord rejected the payload
Even if displayed, split("-") produced ["Check", "In Player"] which matched no pack type in the callback

Test plan

Run /open-packs with only Check-In Player packs in inventory — should show "clean out of packs"
Run /open-packs with mixed pack types including Check-In Player — should only show openable types
Verify Standard/Premium/Team Choice packs still open normally

🤖 Generated with Claude Code

## Summary - Filter "Check-In Player" packs from the `/open-packs` select menu — they're auto-opened by daily check-in and should never appear manually - Add fallback in `pretty_name` logic for pack type names containing hyphens (prevented empty select menu → Discord 400 error) - Replace `KeyError` crash in `SelectOpenPack.callback` with graceful user-facing message for unrecognized pack types - Change all "contact an admin" messages to "contact Cal" ## Context Based on production tag `2026.3.4` to ship as a hotfix without the refractor work on `main`. ## Root Cause An orphaned "Check-In Player" pack (from a failed `roll_for_cards` during daily check-in) caused two cascading failures: 1. The hyphen in "Check-In Player" bypassed `pretty_name` assignment → empty select options → Discord rejected the payload 2. Even if displayed, `split("-")` produced `["Check", "In Player"]` which matched no pack type in the callback ## Test plan - [ ] Run `/open-packs` with only Check-In Player packs in inventory — should show "clean out of packs" - [ ] Run `/open-packs` with mixed pack types including Check-In Player — should only show openable types - [ ] Verify Standard/Premium/Team Choice packs still open normally 🤖 Generated with [Claude Code](https://claude.com/claude-code)

cal added 1 commit 2026-03-26 13:46:15 +00:00

fix: prevent crash when Check-In Player packs appear in open-packs menu

Build Docker Image / build (pull_request) Successful in 3m17s

Details

01f6fb50d5

Check-In Player packs (auto-opened by daily check-in) could end up orphaned
in inventory if roll_for_cards failed. The open-packs command crashed because:
1. The hyphenated pack type name bypassed the pretty_name logic, producing an
   empty select menu that Discord rejected (400 Bad Request)
2. Even if displayed, selecting it would raise KeyError in the callback since
   "Check-In Player".split("-") doesn't match any known pack type token

Fixes:
- Filter auto-open pack types out of the manual open-packs menu
- Add fallback for hyphenated pack type names in pretty_name logic
- Replace KeyError with graceful user-facing message for unknown pack types
- Change "contact an admin" to "contact Cal" in all user-facing messages

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

cal reviewed 2026-03-26 13:49:43 +00:00

cal left a comment

AI Code Review

Files Reviewed

cogs/economy_new/packs.py (modified)
discord_ui/selectors.py (modified)

Findings

Correctness

Three-layer fix — all layers verified correct:

Source filter (packs.py): AUTO_OPEN_TYPES = {"Check-In Player"} is checked early in the loop (continue on match) before any pack is added to p_data. This prevents orphaned Check-In Player packs from ever reaching the select menu builder.
Display fallback (packs.py, pretty_name logic): The new else: pretty_name = key branch correctly handles any future hyphenated pack type that isn't caught by the AUTO_OPEN_TYPES filter. Since the filter already covers the known case, this is good forward-defensive programming.
Callback safety (discord_ui/selectors.py): raise KeyError(...) replaced with proper interaction handling — edit_message(view=None) to dismiss the UI, then followup.send(ephemeral=True) with a user-facing message. The return is in place so the normal flow is not executed. Correct Discord interaction pattern.

Empty-inventory edge case correctly handled: If a player has only Check-In Player packs, p_query["count"] will be > 0 (passing the early return), but p_count will be 0 after the loop. The new if p_count == 0 guard correctly shows the "clean out of packs" message. ✓

Removed unused imports: db_patch and send_to_channel were unused in packs.py — their removal is correct and clean.

Security

No issues found. No user input is used in API queries beyond the existing patterns. The error message in the fallback path does not expose internals.

Style & Conventions

Single-quote → double-quote reformatting throughout selectors.py aligns with the project's ruff configuration.
AUTO_OPEN_TYPES as a module-level set is well-named and easy to extend.
Debug log line ("Skipping auto-open pack type: ...") will aid future diagnosis.
Import block reformatted to one-per-line style — consistent with the rest of the codebase.

Suggestions (non-blocking)

The else: pretty_name = key fallback in open_packs_slash has no log statement. If a new unexpected hyphenated pack type slips through in the future, it would silently display with its raw key name. A logger.warning(f"Unrecognized hyphenated pack type in menu: {key}") there would help future debugging.
The Daily pack type in SelectOpenPack.callback has no open_type assignment (falls through to default "standard"). Pre-existing behavior, not introduced by this PR — worth a follow-up issue.

Verdict: ✅ APPROVED

Correct root-cause analysis with a well-layered fix. The AUTO_OPEN_TYPES filter is the right primary solution, the pretty_name fallback is good defensive programming, and replacing the KeyError crash with a graceful ephemeral message is the correct Discord UX pattern. All three crash paths described in the PR body are addressed. Safe to merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `cogs/economy_new/packs.py` (modified) - `discord_ui/selectors.py` (modified) --- ### Findings #### Correctness **Three-layer fix — all layers verified correct:** 1. **Source filter** (`packs.py`): `AUTO_OPEN_TYPES = {"Check-In Player"}` is checked early in the loop (`continue` on match) before any pack is added to `p_data`. This prevents orphaned Check-In Player packs from ever reaching the select menu builder. 2. **Display fallback** (`packs.py`, `pretty_name` logic): The new `else: pretty_name = key` branch correctly handles any future hyphenated pack type that isn't caught by the AUTO_OPEN_TYPES filter. Since the filter already covers the known case, this is good forward-defensive programming. 3. **Callback safety** (`discord_ui/selectors.py`): `raise KeyError(...)` replaced with proper interaction handling — `edit_message(view=None)` to dismiss the UI, then `followup.send(ephemeral=True)` with a user-facing message. The `return` is in place so the normal flow is not executed. Correct Discord interaction pattern. **Empty-inventory edge case correctly handled:** If a player has *only* Check-In Player packs, `p_query["count"]` will be > 0 (passing the early return), but `p_count` will be 0 after the loop. The new `if p_count == 0` guard correctly shows the "clean out of packs" message. ✓ **Removed unused imports:** `db_patch` and `send_to_channel` were unused in `packs.py` — their removal is correct and clean. #### Security No issues found. No user input is used in API queries beyond the existing patterns. The error message in the fallback path does not expose internals. #### Style & Conventions - Single-quote → double-quote reformatting throughout `selectors.py` aligns with the project's ruff configuration. - `AUTO_OPEN_TYPES` as a module-level set is well-named and easy to extend. - Debug log line (`"Skipping auto-open pack type: ..."`) will aid future diagnosis. - Import block reformatted to one-per-line style — consistent with the rest of the codebase. #### Suggestions (non-blocking) - The `else: pretty_name = key` fallback in `open_packs_slash` has no log statement. If a new unexpected hyphenated pack type slips through in the future, it would silently display with its raw key name. A `logger.warning(f"Unrecognized hyphenated pack type in menu: {key}")` there would help future debugging. - The `Daily` pack type in `SelectOpenPack.callback` has no `open_type` assignment (falls through to default `"standard"`). Pre-existing behavior, not introduced by this PR — worth a follow-up issue. --- ### Verdict: ✅ APPROVED Correct root-cause analysis with a well-layered fix. The `AUTO_OPEN_TYPES` filter is the right primary solution, the `pretty_name` fallback is good defensive programming, and replacing the `KeyError` crash with a graceful ephemeral message is the correct Discord UX pattern. All three crash paths described in the PR body are addressed. Safe to merge. --- *Automated review by Claude PR Reviewer*

Claude approved these changes 2026-03-26 13:49:59 +00:00

Claude left a comment

Approved via pd-pr

cal added 1 commit 2026-03-26 13:50:07 +00:00

Merge branch 'main' into hotfix/open-packs-checkin

Ruff Lint / lint (pull_request) Successful in 22s

Details

b6592b8a70

cal merged commit fca85d583f into main

2026-03-26 13:50:11 +00:00

cal deleted branch hotfix/open-packs-checkin

2026-03-26 13:50:11 +00:00