Repository navigation
Resolve full code review: 13 bugs, 8 optimizations, API modernization - #5
Merged
Merged
Conversation
…zation Remediates every finding from a full review of bot.py and commands.py, plus a dependency/API pass. Test suite grows 135 -> 191. Bugs: - /dashboard exceeded Discord's 2000-char message limit and had never once succeeded; now chunked on category boundaries - Zone-less iCal datetimes were labelled UTC, creating all-day events on the wrong day and floating-time events hours off; now localized to BOT_TIMEZONE via _localize_naive() - /set_reminder's local on_error appended the log tail to every reply, including MissingPermissions — reopening the leak CVE-2026-55099 fixed - /kick and /kick_role checked only the bot's role hierarchy, letting anyone with manage_messages kick members ranked above themselves - /purge_string and /purge_webhooks silently scanned only 100 messages - Weekly and day-of event announcements were byte-identical - The three moderator gates disagreed: command checks resolved channel permissions while the bot.py gates read guild-wide. All guild-wide now - /check_event_feeds gated on the caller's guild then acted on every one - message_dump's rate-limit branch caught HTTPException and string matched, but discord.py raises RateLimited, which is not one — so the handler was dead and a real 429 lost the whole dump - Disabling the voice chaperone left members muted until they moved - Legacy posted_events uids were dropped, re-creating posted events - Feed and emoji fetches were unbounded and unvalidated (SSRF + OOM) - iCal DURATION was ignored, truncating a 3-hour event to one hour Optimizations: - RSS re-scraped every event page each run despite a comment claiming otherwise; already-posted entries are now skipped before the scrape - /check_event_feeds performed all network I/O twice; the two passes now share one memo - Autoreply lowered the message body once per rule, on every message - Every DM cost a fetch_member per guild; a cache miss is now trusted when the guild reports chunked, and confirmed via API when it doesn't - Backup writes and feed saves moved off the event loop API modernization (discord.py 2.7.1 is already latest; these are APIs present but unused): - app_commands.Range on 8 numeric params, deleting validate_reminder_interval and the auto-backup bounds branches, and closing a negative-lines crash and >28-day timeouts - guild_only + guild_install on all 36 commands; default_permissions so moderator commands are hidden from members who cannot run them - setup_hook for command sync, replacing the on_ready reconnect guard - icalendar Event.start/.end; pytz -> zoneinfo; requests dropped (unused) Migration: events already created at the wrong time are not repaired automatically — run /check_event_feeds once, then delete the mistimed originals by hand. Command scope changed, so Discord needs a re-sync (setup_hook does this on next start). Chaperone mutes persisted in the pre-guild-key format are dropped on load with a warning. Claude-Session: https://claude.ai/code/session_013nxm6vo6gr2YYYW5YirK9Q
Actions were pinned at majors that GitHub now force-runs on Node 24 (checkout@v4, setup-python@v5, github-script@v7). Bumped to the current majors and hardened both workflows: - explicit least-privilege permissions and a concurrency group on CI - fail-fast: false, so a multi-version matrix reports every failure - timeout-minutes on both jobs CI matrix widens from 3.11/3.12 to 3.11-3.14. The 3.14 entry matters: this codebase was developed and tested locally on 3.14, which CI never exercised. Python support is documented as 3.11+. The dependencies floor at 3.10 (icalendar, aiohttp and feedparser all require >=3.10), but 3.10 reaches EOL in October 2026, so the supported floor keeps a version of margin. CLAUDE.md previously claimed 3.8+, which no dependency has allowed for some time. Also corrects CLAUDE.md claims that no longer match the code: a DiscordCache that does not exist, 'no test suite exists' (there are 191 tests), reminders running in a background thread (they are APScheduler jobs), an unlocked reminders dict described as lock-guarded, the wrong announcement schedule, and the permission model, which is now guild-wide via _require_guild_permissions plus guild_only/guild_install/ default_permissions. Claude-Session: https://claude.ai/code/session_013nxm6vo6gr2YYYW5YirK9Q
…commit The drift-detection workflow ran `git verify-commit` on the runner. SSH-signed commits need a local gpg.ssh.allowedSignersFile to verify, and a GitHub runner has none, so verify-commit failed for every signed commit and the scan recorded it as unsigned. Every commit in this repo is SSH-signed, so the check has been reporting false positives rather than drift — issues #2 and #3 are both from this. GitHub verifies signatures against the keys registered on the author's account, which needs no local trust setup, so the check now reads .commit.verification from the commits API. The verification reason is carried into the issue body, since 'unknown_key' (key not registered) and 'unsigned' (no signature) warrant different responses. Also moves the .github/ path filter ahead of the lookup so commits that touch nothing relevant cost no API call. Verified by running the extracted script against this repo: a 30-day window over the current signed commits yields found=false, and a window wide enough to include the genuinely unsigned db9b599/736e3f1/949bb94 reports all three with reason [unsigned]. Claude-Session: https://claude.ai/code/session_013nxm6vo6gr2YYYW5YirK9Q
This was referenced Sep 9, 2026
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
1 participant
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.
Remediates every finding from a full review of
bot.pyandcommands.py, plus a dependency/API pass. Test suite grows 135 → 191.Bugs
/dashboardwas 2,682 chars against Discord's 2,000 limit/set_reminderappended the log tail to every replyMissingPermissions— reopened the leak CVE-2026-55099 fixed/kickand/kick_rolechecked only the bot's hierarchymanage_messagescould kick members ranked above themselves/purge_string,/purge_webhooksscanned only 100 messagesDeleted 0 message(s)as if the string were absentbot.pygates read guild-wide. All guild-wide now/check_event_feedsgated on caller's guild, acted on every guildmessage_dumprate-limit branch was dead codeHTTPException; discord.py raisesRateLimited, which is not one. A real 429 lost the whole dumpposted_eventsuids droppedDURATIONignoredOptimizations
/check_event_feedsdid all network I/O twice; both passes now share one memofetch_memberper guild; a cache miss is trusted only when the guild reportschunked, and confirmed via API otherwiseAPI modernization
discord.py 2.7.1 is already the latest release and no deprecated API is in use — these are APIs present but unused:
app_commands.Rangeon 8 numeric params, deletingvalidate_reminder_intervaland the auto-backup bounds branches, and closing a negative-linescrash and >28-day timeoutsguild_only+guild_installon all 36 commands;default_permissionshides moderator commands from members who can't run themsetup_hookfor command sync, replacing theon_readyreconnect guardEvent.start/.end;pytz→zoneinfo;requestsdropped (never imported)Deliberate deviations
next_run_timevalues are naive host-local.Guild.scheduled_eventscache used at one call site only.create_scheduled_event()doesn't write to that cache, so the two dedup paths keepfetch_scheduled_events().autorepliesis mutated from several places, so a derived index is a standing staleness bug. Kept the real win.Migration — action required after merge
posted_events, and a corrected event won't match its mistimed twin. Run/check_event_feedsonce, then delete the mistimed originals from the Events tab.setup_hookhandles it on next start.Testing
191 passing locally on Python 3.14. CI on 3.11/3.12 is the check that matters here — everything used is well within those (
zoneinfo3.9+,contextlib.asynccontextmanager3.7+), but this PR confirms it.🤖 Generated with Claude Code
https://claude.ai/code/session_013nxm6vo6gr2YYYW5YirK9Q