fix: default missing "enabled" config key per plugin _DEFAULTS#78
Merged
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Reviewer's GuideFixes missing-default handling for Sequence diagram for updated enabled gate in plugins when config key is missingsequenceDiagram
actor Discord
participant EconomyPlugin
participant ConfigStore
Discord->>EconomyPlugin: _on_message(message)
EconomyPlugin->>ConfigStore: _get_config(guild_id)
ConfigStore-->>EconomyPlugin: cfg
alt enabled_key_present
EconomyPlugin->>EconomyPlugin: cfg.get(enabled, True)
EconomyPlugin->>EconomyPlugin: [enabled is True]
EconomyPlugin->>EconomyPlugin: apply message_reward
else enabled_key_missing
EconomyPlugin->>EconomyPlugin: cfg.get(enabled, True)
EconomyPlugin->>EconomyPlugin: [absence reads as True]
EconomyPlugin->>EconomyPlugin: apply message_reward
end
note over EconomyPlugin: Before B-020: cfg.get(enabled)\nmade absence read as False,\nsilently disabling the plugin
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
github-actions
Bot
added
bug
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThe changes update missing-key defaults for several plugins, adjust slash-command bot permission metadata, add regression tests and bug records, and revise project guidance and planning documentation. ChangesPlugin configuration defaults
Slash-command permission metadata
Project guidance and configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The change to
_command_callbacks.py’s comment (sweep never starts) no longer matches the existing behavior where the sweep is deferred to first dispatch; either adjust the logic to truly never start without a loop, or update the comment to reflect the actual behavior. - Several slash command decorators had
bot_permissionsremoved (verification, welcome, giveaway, starboard); please double-check that this is intentional and that permission checks are enforced consistently across commands using a single mechanism.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The change to `_command_callbacks.py`’s comment (`sweep never starts`) no longer matches the existing behavior where the sweep is deferred to first dispatch; either adjust the logic to truly never start without a loop, or update the comment to reflect the actual behavior.
- Several slash command decorators had `bot_permissions` removed (verification, welcome, giveaway, starboard); please double-check that this is intentional and that permission checks are enforced consistently across commands using a single mechanism.
## Individual Comments
### Comment 1
<location path="easycord/plugins/verification.py" line_range="228" />
<code_context>
panel_message_id,
)
- @slash(description="Set the verified role and channel for the verification panel.", guild_only=True, bot_permissions=["manage_roles", "send_messages"])
+ @slash(description="Set the verified role and channel for the verification panel.", guild_only=True)
async def verification_setup(
self,
</code_context>
<issue_to_address>
**issue (bug_risk):** Dropping `bot_permissions` may degrade UX when the bot lacks `manage_roles` or `send_messages`.
This command still requires `manage_roles` to assign the verified role and usually `send_messages` to post the panel or responses. Without `bot_permissions`, these failures become runtime `Forbidden` errors instead of clear, early validation. If you want to loosen the check, consider at least keeping `manage_roles` so misconfigurations fail predictably at invocation time rather than mid-command.
</issue_to_address>
### Comment 2
<location path="easycord/plugins/welcome.py" line_range="118" />
<code_context>
# ── Slash commands ────────────────────────────────────────
- @slash(description="Set the channel for welcome messages.", permissions=["manage_guild"], bot_permissions=["send_messages"])
+ @slash(description="Set the channel for welcome messages.", permissions=["manage_guild"])
async def set_welcome_channel(self, ctx, channel: discord.TextChannel) -> None:
guild = require_guild(ctx)
</code_context>
<issue_to_address>
**issue (bug_risk):** Removing the `send_messages` bot permission check can lead to confusing runtime failures.
This command assumes the bot can send messages in the configured channel; without a `bot_permissions=["send_messages"]` check, misconfigured permissions will only surface as send failures at runtime. If you’re simplifying decorators, I’d still keep `send_messages` here (and for similar commands) to provide earlier, clearer feedback on missing bot permissions.
</issue_to_address>
### Comment 3
<location path="easycord/plugins/giveaway.py" line_range="263" />
<code_context>
self._timers.get(guild_id, {}).pop(message_id, None)
- @slash(description="Start a giveaway in the current channel.", guild_only=True, bot_permissions=["send_messages", "add_reactions"])
+ @slash(description="Start a giveaway in the current channel.", guild_only=True, bot_permissions=["send_messages"])
async def giveaway(
self,
</code_context>
<issue_to_address>
**issue (bug_risk):** Dropping the `add_reactions` requirement may break giveaways if reactions are still used to enter.
If giveaways still use reactions for entry, the bot must keep `add_reactions` for this command. Without it, missing permissions won’t be caught by the decorator and the command will only fail at runtime when adding reactions. If reactions are no longer part of the flow, this change is fine; otherwise, please keep `add_reactions` in `bot_permissions`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 034d6df7c2
ℹ️ 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".
| ) | ||
|
|
||
| @slash(description="Post the verification panel in the configured channel.", guild_only=True, bot_permissions=["send_messages"]) | ||
| @slash(description="Post the verification panel in the configured channel.", guild_only=True) |
There was a problem hiding this comment.
With bot_permissions=["send_messages"] removed here and no replacement around the channel.send(...) below, invoking /verification_panel from the configured panel channel while the bot lacks Send Messages now enters the command and raises discord.Forbidden instead of the prior bot-permissions response from build_slash_callback. Please keep a preflight or catch Forbidden for the configured channel before sending the panel.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in #79 — verification_panel now catches Forbidden/HTTPException around the panel send, responds with an ephemeral error, and persists nothing when the panel never posted (regression test included). Restoring the decorator wasn't the right fix: bot_permissions validates the invocation channel, while the panel goes to the configured channel. Note this code was split out of this PR into #79 — it was an unrelated local-main commit that leaked into the diff.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
rolling-codes
force-pushed
the
fix/74-enabled-default-missing-key
branch
from
034d6df to
e909867
Compare
A stored config section can exist without the "enabled" key (manual
config-file edit or partial external update); cfg.get("enabled") with
no default read that as disabled and silently turned the feature off.
- economy.py:219, role_persistence.py:53/79, member_logging.py:60 —
cfg.get("enabled", True), matching each plugin's _DEFAULTS (same fix
as starboard's B-018)
- ai_moderator.py:183/237 — explicit cfg.get("enabled", False); opt-in
moderation, missing key correctly reads as disabled (no behavior
change)
- Regression tests for the missing-key scenario in all four plugins
- bugs.md: B-020 filed; supersedes B-019's "verified benign" verdict,
which only audited internal config-creation paths
auto_responder.py:68 (also named in #74) was already fixed by B-013.
Slash-command tests pass None for the orchestrator; the annotation flagged 7 reportArgumentType errors under Pyright/Pylance.
rolling-codes
force-pushed
the
fix/74-enabled-default-missing-key
branch
from
e909867 to
598dc67
Compare
3 tasks
rolling-codes
pushed a commit
that referenced
this pull request
Jul 11, 2026
Review feedback on the B-021 guards: - send_safe() in helpers/channel.py sends to a configured channel, absorbs Forbidden/HTTPException with a logged warning, and returns the message or None; adopted by verification_panel and the welcome/goodbye event sends (previously except-pass with no log) - welcome auto-role failure now logs a warning instead of a silent pass (add_roles, so the send helper doesn't apply) - unit tests for all three helper branches member_logging._log_to_channel has the same shape but lives in PR #78's diff; adopting it there is a post-merge follow-up.
3 tasks
rolling-codes
added a commit
that referenced
this pull request
Jul 11, 2026
…nnel sends (#79) Carries two commits split out of #78 (they were unpushed local-main commits that leaked into that PR's diff), plus the fix for the genuine review finding they surfaced. ## Changes **Cherry-picked from local main:** - `380e61a` → drop `bot_permissions` from config-setter commands (welcome `set_welcome_channel`/`set_auto_role`, `starboard_channel`, `verification_setup`, `verification_panel`) and giveaway `add_reactions` (button-based flow, no reaction code). These preflights validated the *invocation* channel while the privileged operation targets a *configured* channel (or happens later on a different event) — wrong scope. - `5690a05` → CLAUDE.md/AGENTS.md planning-config sync. **New (B-021), addressing the Codex P2 review on #78:** - `verification_panel` really does send at invocation — to the configured channel. `Forbidden`/`HTTPException` are now caught with an ephemeral error response, and nothing is persisted when the panel never posted. - Welcome/goodbye event-handler sends get `except HTTPException: pass` guards, matching the auto-role guard above them (same event-path class as B-014/B-015/B-016). - Regression tests for both paths; bugs.md B-021 filed. ## Review findings dispositions (from #78 bots) - **Codex P2 (verification_panel unguarded send)** — fixed here; restoring the decorator would have checked the wrong channel. - **Sourcery: verification_setup needs manage_roles** — declined: pure config write; role assignment happens at button click, which already handles Forbidden. - **Sourcery: welcome setters need perms** — declined for the setters (config-only, runtime paths guarded); the genuinely unguarded welcome/goodbye sends are fixed here. - **Sourcery: giveaway needs add_reactions** — declined: giveaway entry is fully button-based; no reaction code exists. ## Notes - bugs.md will trivially conflict with #78 (both append a table row after B-016); resolve by keeping both rows (B-020 then B-021). ## Test plan - [x] New regression tests fail without the guards, pass with them - [x] Full suite: 1462 passed - [x] Pyright: 0 errors on touched files 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by Sourcery Guard configured-channel sends in verification and welcome flows while removing misleading bot_permissions preflights. Bug Fixes: - Handle permission and HTTP errors when posting the verification panel so failures respond ephemerally and do not persist panel state. - Prevent welcome and goodbye message send failures from escaping event handlers when the bot lacks permissions in configured channels. Enhancements: - Remove bot_permissions preflights from config-only slash commands and giveaways where they checked the wrong channel or unused permissions. - Clarify command callback sweep behavior comment and update internal guidance/docs for graph-based planning and Graphify usage. Documentation: - Document bug B-021, its root cause, fixes, and lessons learned in bugs.md, and sync CLAUDE/AGENTS guidance on using the knowledge graph. Tests: - Add regression tests to ensure verification_panel handles Forbidden sends with an ephemeral error and that welcome send failures do not raise out of the join handler. --------- Co-authored-by: tee <Thomas_BIRRELL@proton.me>
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
2 participants
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 #74
Summary
if not cfg.get("enabled")with no default treats a missingenabledkey as disabled, silently turning a plugin off. A stored config section can exist with any subset of the default keys —_get_config()only merges_DEFAULTSwhen the section is entirely absent — so a manual config-file edit or partial external update can strip the key. Same class as starboard's B-018; supersedes B-019's "verified benign" verdict, which only audited internal config-creation paths.Changes
economy.py:219,role_persistence.py:53/79,member_logging.py:60—cfg.get("enabled", True), matching each plugin's_DEFAULTSai_moderator.py:183/237— explicitcfg.get("enabled", False); moderation is opt-in by design, so a missing key correctly reads as disabled (no behavior change)enabled-key scenario in all four plugins (RED confirmed pre-fix on the three enabled-by-default plugins)tests/test_ai_moderator.py:_make_pluginorchestrator annotation widened toMagicMock | None(pre-existing Pyright error, 7 call sites passNone)Corrections to the issue as filed
auto_responder.py:68was already fixed by B-013 (2026-07-09) — no change needed.ai_moderator.pyis intentionally opt-in (_DEFAULTShasenabled: False), so it gets an explicitFalsedefault rather thanTrue.member_logging.py:60had the same bug but wasn't listed in the issue — included here.Test plan
main's gate code, pass with the fix🤖 Generated with Claude Code
Summary by Sourcery
Ensure plugin enabled-state reads use explicit defaults and document the reopened config bug pattern.
Bug Fixes:
Enhancements:
Documentation:
Tests: