fix: guard GUILD_ID env var cast against missing/invalid value (#26) #60

cal · 2026-03-05T11:34:49Z

cal commented

2026-03-05 11:34:49 +00:00

Summary

Guards all three call sites of int(os.environ.get('GUILD_ID')) against None/empty values by checking the env var before casting.

Changes

cogs/gameplay.py (live_scorecard task loop): Added guild_id = os.environ.get('GUILD_ID') + early return with logger.error if unset, before self.bot.get_guild(int(guild_id))
helpers/discord_utils.py:104: Same guard pattern in send_to_channel()
discord_utils.py:104: Same fix in the top-level duplicate

What was fixed

int(os.environ.get('GUILD_ID')) → int(None) raises TypeError when GUILD_ID is unset. In live_scorecard this was caught by the outer except Exception and logged, but masked the real problem. In send_to_channel() it would crash callers without the protection.

Notes

The linter hook (ruff) reformatted cogs/gameplay.py when I made my edit. The functional GUILD_ID guard is in the live_scorecard method (~line 128). The reformatting changes are purely cosmetic (quote style, import splitting, line wrapping) and do not affect behavior.

Other observations

discord_utils.py is a known dead duplicate (issue #34) — no production code imports it directly
cogs/gameplay_legacy.py has the same int(os.environ.get('GUILD_ID')) pattern at lines 48, 108, 112, 116 — not fixed here as it's legacy code, but worth noting

## Summary Guards all three call sites of `int(os.environ.get('GUILD_ID'))` against `None`/empty values by checking the env var before casting. ## Changes - **`cogs/gameplay.py`** (live_scorecard task loop): Added `guild_id = os.environ.get('GUILD_ID')` + early return with `logger.error` if unset, before `self.bot.get_guild(int(guild_id))` - **`helpers/discord_utils.py:104`**: Same guard pattern in `send_to_channel()` - **`discord_utils.py:104`**: Same fix in the top-level duplicate ## What was fixed `int(os.environ.get('GUILD_ID'))` → `int(None)` raises `TypeError` when `GUILD_ID` is unset. In `live_scorecard` this was caught by the outer `except Exception` and logged, but masked the real problem. In `send_to_channel()` it would crash callers without the protection. ## Notes The linter hook (ruff) reformatted `cogs/gameplay.py` when I made my edit. The functional GUILD_ID guard is in the `live_scorecard` method (~line 128). The reformatting changes are purely cosmetic (quote style, import splitting, line wrapping) and do not affect behavior. ## Other observations - `discord_utils.py` is a known dead duplicate (issue #34) — no production code imports it directly - `cogs/gameplay_legacy.py` has the same `int(os.environ.get('GUILD_ID'))` pattern at lines 48, 108, 112, 116 — not fixed here as it's legacy code, but worth noting

cal added 1 commit 2026-03-05 11:34:50 +00:00

fix: guard GUILD_ID env var cast against missing/invalid value (#26 )

Build Docker Image / build (pull_request) Successful in 1m22s

Details

7457cab110

Check for None/empty before calling int() on os.environ.get('GUILD_ID')
in all three call sites. Prevents TypeError crashing the live scoreboard
task loop and send_to_channel() when GUILD_ID is unset.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

cal referenced this pull request

2026-03-05 11:34:59 +00:00

Guard `GUILD_ID` env var cast against missing/invalid value #26

cal added the

ai-reviewing

label 2026-03-05 11:45:34 +00:00

cal reviewed 2026-03-05 11:46:48 +00:00

cal left a comment

AI Code Review

Files Reviewed

cogs/gameplay.py (modified — ruff reformat + functional fix in live_scorecard)
helpers/discord_utils.py (modified — ruff reformat + functional fix in send_to_channel)
discord_utils.py (modified — identical changes as helpers/discord_utils.py)

Findings

Correctness

The guard if not guild_id: correctly handles both None (env var unset) and "" (env var set to empty string) before calling int(guild_id). This is the right pattern.
The fix is applied consistently in all three call sites across both files.
int(guild_id) could still raise ValueError if GUILD_ID is set to a non-numeric string, but that is a deployment misconfiguration, not a runtime condition — appropriate not to guard against per project conventions. In live_scorecard it would be caught by the outer except Exception; in send_to_channel it would propagate to callers, which is acceptable.
The ruff reformatting (double quotes, expanded imports, line wrapping) is purely cosmetic and does not affect behavior, as noted in the PR.

Security

No issues found. No hardcoded credentials, no injection vectors introduced.

Style & Conventions

No issues found. Guard pattern is clean and consistent. Ruff-applied formatting matches project tooling.

Suggestions

As noted in the PR, cogs/gameplay_legacy.py still has the same unguarded pattern at lines 48, 108, 112, 116. Since it's legacy code not in the active code path, this doesn't block merging — but tracking it in an issue would be worthwhile if not already done.

Verdict: APPROVED

Clean, correct, minimal-scope fix. The guard pattern is applied consistently across all three call sites, addresses the root TypeError, and follows project conventions. Ruff reformatting is acceptable collateral.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `cogs/gameplay.py` (modified — ruff reformat + functional fix in `live_scorecard`) - `helpers/discord_utils.py` (modified — ruff reformat + functional fix in `send_to_channel`) - `discord_utils.py` (modified — identical changes as helpers/discord_utils.py) ### Findings #### Correctness - The guard `if not guild_id:` correctly handles both `None` (env var unset) and `""` (env var set to empty string) before calling `int(guild_id)`. This is the right pattern. - The fix is applied consistently in all three call sites across both files. - `int(guild_id)` could still raise `ValueError` if `GUILD_ID` is set to a non-numeric string, but that is a deployment misconfiguration, not a runtime condition — appropriate not to guard against per project conventions. In `live_scorecard` it would be caught by the outer `except Exception`; in `send_to_channel` it would propagate to callers, which is acceptable. - The ruff reformatting (double quotes, expanded imports, line wrapping) is purely cosmetic and does not affect behavior, as noted in the PR. #### Security - No issues found. No hardcoded credentials, no injection vectors introduced. #### Style & Conventions - No issues found. Guard pattern is clean and consistent. Ruff-applied formatting matches project tooling. #### Suggestions - As noted in the PR, `cogs/gameplay_legacy.py` still has the same unguarded pattern at lines 48, 108, 112, 116. Since it's legacy code not in the active code path, this doesn't block merging — but tracking it in an issue would be worthwhile if not already done. ### Verdict: APPROVED Clean, correct, minimal-scope fix. The guard pattern is applied consistently across all three call sites, addresses the root TypeError, and follows project conventions. Ruff reformatting is acceptable collateral. --- *Automated review by Claude PR Reviewer*

cal added

ai-reviewed

and removed

ai-reviewing

labels 2026-03-05 11:47:26 +00:00

cal referenced this pull request

2026-03-05 13:03:30 +00:00

fix: remove debug print() statements from gameplay_models.py (#32) #63

cal changed target branch from main to next-release

2026-03-07 07:32:37 +00:00

cal force-pushed ai/paper-dynasty-discord-26 from 7457cab110 to c1a81b7bb9

2026-03-07 07:33:38 +00:00