refactor: module splits, ai_moderator type safety + prompt injection fix#58
Merged
rolling-codes merged 11 commits intoJun 20, 2026
Merged
Conversation
Ports the persistence pattern already used by GiveawayPlugin/ReminderPlugin (ServerConfigStore + a timeout=None button view with a deterministic custom_id + an on_ready() resume hook) onto PollsPlugin. Active polls (question, options, per-user votes, absolute end_time) are saved to per-guild storage; on reconnect the bot re-registers each poll's vote view via bot.add_view() and resumes its countdown, or closes it immediately if it was already overdue while the bot was down. Breaking-ish change: /poll is now guild-only, since storage is keyed by guild_id — there's no per-guild config to persist into from a DM. _PollView's vote tally/embed logic moves to module-level pure functions (_tally, _bar, _format_option_line, build_poll_embed) so _close_poll can render final results without needing a live view instance with votes attached to it. Adds FakeInteraction.original_response()/message_id support in easycord/testing.py so command tests can exercise the respond() -> original_response() -> message.edit() flow that both GiveawayPlugin and the new PollsPlugin rely on. Updates the existing _PollView-based tests in test_plugins.py and test_plugins_new.py for the new constructor signature and pure-function helpers, and adds resume/persistence coverage to test_plugin_commands.py (store round-trip, on_ready resuming an active poll, on_ready closing an overdue one).
…testing.py FakeInteraction is intentionally duck-typed rather than a discord.Interaction subclass (see its docstring), so every FakeContext(interaction)/cls(interaction) construction site, the namespace= assignment in invoke_autocomplete, and FakeContext.responses' self.interaction._responses access were flagged by Pylance as type mismatches. Added targeted # type: ignore[arg-type]/ [attr-defined] comments at each site, matching the existing convention in _context_base.py, instead of changing FakeInteraction's design.
on_ready's add_view re-registration, the transcript log-channel send, the thread archive/lock on close, and re-adding support-role members to a thread can all fail for reasons that don't block the ticket flow itself (deleted message, missing permission, already-archived thread, member already left). Documented why each is safely ignored instead of leaving a silent pass.
Same SARIF rule as the tickets.py fix (0119b6b): an except clause that does nothing but pass needs an explanatory comment or it reads as dead code. Covers every other instance in the codebase — optional psutil import, locale detection, persistent-view re-registration on restart (giveaway/polls/tickets pattern), CancelledError during task teardown (birthday/giveaway/polls/reminder/scheduled_announcements/server_stats), DM-forbidden/role-permission failures (ai_moderator/levels/welcome), deleted-message handling (giveaway/starboard/paginator), and a namespace-shape fallback in _bot_commands.py. Also simplified a redundant `except (ValueError, Exception)` to `except Exception` in helpers/config.py (ValueError is already an Exception subclass). No behavior changes — comments only.
…etch_message() discord.utils has no timedelta attribute — AIModeratorPlugin's and ModerationPlugin's timeout commands would raise AttributeError the first time a timeout/mute action actually fired. Both now import datetime.timedelta directly. StarboardPlugin (all four get_channel() call sites) and AIModeratorPlugin's notify_only review-channel post used `if channel:` before calling .fetch_message()/.send(), which doesn't narrow out ForumChannel/CategoryChannel/StageChannel — types get_channel() can return that lack those methods. Switched to the SENDABLE_CHANNEL_TYPES isinstance check already used by invite_tracker.py/member_logging.py/ polls.py. Also added the missing # type: ignore[union-attr] on paginator.py's two child.disabled assignments, matching the existing convention for discord.ui.Item not statically exposing .disabled.
reason/duration parameters across kick/ban/unban/timeout/warn/mute/unmute and _log_moderation were annotated `str = None` — None isn't assignable to str. Changed to `str | None = None` (and _log_moderation's required `reason` to `str | None`) to match actual usage. _log_moderation's audit_channel.send() used `if not audit_channel:` before sending, same gap as the starboard.py/ai_moderator.py fix earlier this session — get_channel() can return a ForumChannel/ CategoryChannel that lacks .send(). Switched to the SENDABLE_CHANNEL_TYPES isinstance check.
…rompt injection Module splits: - Extract _bot_commands callback wiring → _command_callbacks.py, _command_registration.py - Extract _bot_plugins method scanner → _plugin_scanner.py - Extract i18n internals → _i18n_locale.py, _i18n_diagnostics.py, _i18n_validation.py Security (closes #48): - Wrap message.content and message.author.name in <message>/<author> XML delimiters in AIModeratorPlugin._analyze_message to prevent prompt injection (audit M1 finding) Type safety: - Add `if ctx.guild is None: return False` guard to _execute_action (4 Pyright errors) - Add `assert ctx.guild is not None` to all six guild_only slash handlers (8 Pyright errors) Docs: - Expand CLAUDE.md with architecture quick-reference, testing patterns, and key invariants
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? |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's GuideRefactors command and plugin registration into new helper modules, enhances polls and testing utilities, hardens AI moderation against prompt injection and type errors, and restructures i18n helpers while tightening channel/type handling across plugins and improving docs. Sequence diagram for persistent poll lifecyclesequenceDiagram
actor User
participant PollsPlugin
participant Store as ServerConfigStore
participant Discord as DiscordAPI
participant PollView as _PollView
participant Timer as TimerTask
User->>PollsPlugin: /poll(question, options, duration)
PollsPlugin->>PollsPlugin: _is_valid_duration()
PollsPlugin->>Discord: ctx.respond(embed=build_poll_embed(...))
Discord-->>PollsPlugin: interaction.original_response()
PollsPlugin->>Store: _store.load(guild_id)
Store-->>PollsPlugin: ServerConfig
PollsPlugin->>Store: _store.save(cfg with polls[message_id])
PollsPlugin->>Discord: bot.add_view(_PollView(...), message_id)
PollsPlugin->>Discord: message.edit(embed, view)
PollsPlugin->>Timer: _schedule_timer(guild_id, message_id, duration)
User->>PollView: click vote button
PollView->>PollsPlugin: _guild_lock(guild_id)
PollView->>Store: _store.load(guild_id)
Store-->>PollView: ServerConfig
PollView->>PollView: update votes
PollView->>Store: _store.save(cfg)
PollView->>Discord: interaction.response.edit_message(embed=build_poll_embed(...), view)
Timer->>Timer: _poll_timer(sleep seconds)
Timer->>PollsPlugin: _close_poll(guild_id, message_id)
PollsPlugin->>Store: _store.load(guild_id)
PollsPlugin->>Store: _store.save(cfg with status="closed")
PollsPlugin->>Discord: guild.get_channel(channel_id)
PollsPlugin->>Discord: channel.fetch_message(message_id)
Discord-->>PollsPlugin: message
PollsPlugin->>Discord: message.edit(embed=build_poll_embed(..., closed=True), view.disable_all())
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
github-actions
Bot
added
enhancement
|
✅ Performance benchmarks completed. All thresholds maintained. See workflow artifacts for detailed results. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+142
to
+146
| chain_builder=lambda ctx, invoke, middleware: build_chain( | ||
| ctx, | ||
| invoke, | ||
| middleware, | ||
| ), |
| self, | ||
| func, | ||
| callback_builder=self._build_slash_callback, | ||
| context_factory=lambda interaction: Context(interaction), |
| register_context_menu( | ||
| self, | ||
| func, | ||
| context_factory=lambda interaction: Context(interaction), |
Comment on lines
+310
to
+314
| chain_builder=lambda ctx, invoke, middleware: build_chain( | ||
| ctx, | ||
| invoke, | ||
| middleware, | ||
| ), |
![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 new command helper modules (
_command_callbacks.py,_command_registration.py,_plugin_scanner.py) lean heavily onbot: objectwith dynamic attributes; consider introducing aProtocolor shared_BotLiketype so Pyright can catch attribute slips and future refactors stay type-safe. - SENDABLE_CHANNEL_TYPES is now referenced from
plugins._utilsand mentioned inCLAUDE.mdas coming fromhelpers.tools; it would be less brittle to centralize this tuple in one module and re-export it rather than duplicating/aliasing the definition.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new command helper modules (`_command_callbacks.py`, `_command_registration.py`, `_plugin_scanner.py`) lean heavily on `bot: object` with dynamic attributes; consider introducing a `Protocol` or shared `_BotLike` type so Pyright can catch attribute slips and future refactors stay type-safe.
- SENDABLE_CHANNEL_TYPES is now referenced from `plugins._utils` and mentioned in `CLAUDE.md` as coming from `helpers.tools`; it would be less brittle to centralize this tuple in one module and re-export it rather than duplicating/aliasing the definition.
## Individual Comments
### Comment 1
<location path="easycord/helpers/tools.py" line_range="33-37" />
<code_context>
safety = tool_config.get("safety")
- if not all([name, func, description, safety]):
+ if (
+ not isinstance(name, str)
+ or not callable(func)
+ or not isinstance(description, str)
+ or not isinstance(safety, ToolSafety)
+ ):
continue
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Batch registration now silently skips tools unless `safety` is a `ToolSafety` enum, which may reject existing string-based configs.
This used to accept any truthy `safety` (including strings like `'safe'`), but now only `ToolSafety` instances are accepted and invalid configs are silently skipped. If configs may come from JSON or other non-Python sources, consider either mapping string values to `ToolSafety` or surfacing an error (log/raise) instead of just `continue` so misconfigurations are easier to detect and existing configs don’t break silently.
Suggested implementation:
```python
import logging
from easycord.tools import ToolDef, ToolRegistry, ToolSafety
logger = logging.getLogger(__name__)
```
```python
description = tool_config.get("description")
raw_safety = tool_config.get("safety")
if (
not isinstance(name, str)
or not callable(func)
or not isinstance(description, str)
):
logger.error(
"Invalid tool configuration: name=%r, func=%r, description=%r",
name,
func,
description,
)
continue
# Normalize safety configuration:
# - Accept ToolSafety instances directly
# - Accept string values and map them to ToolSafety
# - Log and skip invalid values instead of silently continuing
if isinstance(raw_safety, ToolSafety):
safety = raw_safety
elif isinstance(raw_safety, str):
try:
safety = ToolSafety[raw_safety]
except KeyError:
logger.error(
"Invalid safety %r for tool %r: expected one of %s",
raw_safety,
name,
", ".join([s.name for s in ToolSafety]),
)
continue
else:
logger.error(
"Invalid safety configuration %r for tool %r: expected ToolSafety or str",
raw_safety,
name,
)
continue
registry.register(
name=name,
func=cast(Callable[..., Any], func),
description=description,
safety=safety,
parameters=tool_config.get("parameters"),
```
</issue_to_address>
### Comment 2
<location path="easycord/_command_registration.py" line_range="16" />
<code_context>
+logger = logging.getLogger("easycord")
+
+
+def register_slash(
+ bot: object,
+ func: Callable,
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring this helper module by collapsing the wide register_slash signature into a config object, simplifying the autocomplete handler function, and introducing a Protocol for the bot type to reduce surface area and indirection.
You can keep this helper module but significantly reduce its surface area and indirection cost with a couple of targeted refactors.
### 1. Collapse the wide `register_slash` signature into a config object
Instead of threading every option as a top‑level parameter, group them into a small config/dataclass. This reduces call‑site noise and makes it clear that these options travel together.
```py
from dataclasses import dataclass
from typing import Callable, Mapping
@dataclass
class SlashCommandConfig:
name: str
description: str
guild_id: int | None
guild_only: bool = False
require_admin: bool = False
ephemeral: bool = False
permissions: list[str] | None = None
cooldown: float | None = None
cooldown_rate: int = 1
cooldown_bucket: str = "user"
premium_required: bool = False
autocomplete: Mapping[str, Callable] | None = None
choices: Mapping[str, list] | None = None
nsfw: bool = False
allowed_contexts: app_commands.AppCommandContext | None = None
allowed_installs: app_commands.AppInstallationType | None = None
parent: app_commands.Group | None = None
source_plugin: str | None = None
```
Then:
```py
def register_slash(
bot: "SupportsSlashRegistration",
func: Callable,
*,
callback_builder: Callable[..., Callable],
context_factory: Callable[[discord.Interaction], object],
config: SlashCommandConfig,
) -> None:
guild = discord.Object(id=config.guild_id) if config.guild_id else None
callback = callback_builder(
func,
guild_only=config.guild_only,
require_admin=config.require_admin,
ephemeral=config.ephemeral,
permissions=config.permissions,
cooldown=config.cooldown,
cooldown_rate=config.cooldown_rate,
cooldown_bucket=config.cooldown_bucket,
premium_required=config.premium_required,
command_name=config.name,
)
autocomplete_handlers: dict[str, Callable] = {
**(config.autocomplete or {}),
**getattr(func, "_slash_autocomplete_handlers", {}),
}
if config.choices:
inject_choices(callback, config.choices)
cmd = app_commands.Command(
name=config.name,
description=config.description,
callback=callback,
nsfw=config.nsfw,
allowed_contexts=config.allowed_contexts,
allowed_installs=config.allowed_installs,
)
for param_name, handler in autocomplete_handlers.items():
_register_autocomplete_handler(
bot,
cmd,
name=config.name,
param_name=param_name,
handler=handler,
source_plugin=config.source_plugin,
guild_id=config.guild_id,
context_factory=context_factory,
)
if config.parent is not None:
config.parent.add_command(cmd)
registry_name = f"{config.parent.name} {config.name}"
else:
bot.tree.add_command(cmd, guild=guild)
registry_name = config.name
bot.registry.register_slash_command(
registry_name,
func,
source_plugin=config.source_plugin,
guild_id=config.guild_id,
metadata={
"description": config.description,
"guild_only": config.guild_only,
"permissions": config.permissions,
"cooldown": config.cooldown,
"cooldown_rate": config.cooldown_rate,
"cooldown_bucket": config.cooldown_bucket,
"premium_required": config.premium_required,
"allowed_contexts": config.allowed_contexts,
"allowed_installs": config.allowed_installs,
"parent": getattr(config.parent, "name", None),
},
)
```
Call sites become:
```py
config = SlashCommandConfig(
name="ping",
description="Ping the bot",
guild_id=guild_id,
guild_only=True,
# other options...
)
register_slash(bot, func, callback_builder=..., context_factory=..., config=config)
```
This keeps all existing behaviour but drastically narrows the function signature and improves readability.
### 2. Remove one level of nesting from `_register_autocomplete_handler`
You can avoid `_make_autocomplete` entirely by using a single inner function with closed‑over variables or a partial. That keeps the logic in one place and makes the flow easier to follow:
```py
def _register_autocomplete_handler(
bot: "SupportsAutocompleteRegistration",
cmd: app_commands.Command,
*,
name: str,
param_name: str,
handler: Callable,
source_plugin: str | None,
guild_id: int | None,
context_factory: Callable[[discord.Interaction], object],
) -> None:
sig = inspect.signature(handler)
params = list(sig.parameters.values())
param_count = len(params)
if param_count not in (1, 3):
plugin_info = f" in plugin {source_plugin}" if source_plugin else ""
raise TypeError(
f"Invalid autocomplete signature for option {param_name!r} "
f"of command {name!r}{plugin_info}. "
f"Expected (current) or (ctx, current, options), got {sig}."
)
expects_options = (param_count == 3)
async def ac_callback(interaction: discord.Interaction, current: str) -> list[app_commands.Choice]:
ctx = context_factory(interaction)
options = autocomplete_options(interaction)
try:
if expects_options:
results = await handler(ctx, current, options)
else:
results = await handler(current)
return [app_commands.Choice(name=r, value=r) for r in results]
except Exception as exc:
plugin_instance = getattr(handler, "__self__", None)
if plugin_instance is None and source_plugin:
plugin_instance = next(
(
p
for p in bot._plugins
if getattr(p, "_instance_id", type(p).__name__) == source_plugin
),
None,
)
await bot._dispatch_framework_error(
exc,
ctx=ctx,
plugin_instance=plugin_instance,
)
return []
cmd.autocomplete(param_name)(ac_callback)
bot.registry.register_autocomplete(
name,
param_name,
handler,
source_plugin=source_plugin,
guild_id=guild_id,
)
```
Same behaviour, but one less layer of abstraction (no `_make_autocomplete`), which directly addresses the “spread across three helpers” concern.
### 3. Narrow the “bot as object” surface with a lightweight protocol
You can keep this as a helper module but still make it clearer what `bot` must provide, reducing “untyped bag of attributes” feel without changing runtime behaviour:
```py
from typing import Protocol, Any
class SupportsSlashRegistration(Protocol):
tree: Any
registry: Any
_plugins: list[Any]
async def _dispatch_framework_error(self, exc: Exception, *, ctx: Any, plugin_instance: Any | None) -> None: ...
class SupportsAutocompleteRegistration(SupportsSlashRegistration, Protocol):
pass
```
Then annotate parameters:
```py
def register_slash(
bot: SupportsSlashRegistration,
func: Callable,
*,
callback_builder: Callable[..., Callable],
context_factory: Callable[[discord.Interaction], object],
config: SlashCommandConfig,
) -> None:
...
```
This doesn’t change the implementation or the call sites but makes the helper’s expectations explicit and easier to reason about.
</issue_to_address>
### Comment 3
<location path="easycord/_plugin_scanner.py" line_range="12" />
<code_context>
+from .tools import ToolSafety
+
+
+def scan_plugin_methods(
+ bot: object,
+ plugin: object,
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the plugin scanning logic to use a dispatch table and a typed bot protocol so the registration flow is clearer and less meta-reflective.
You can reduce the “meta-feel” and make the flow easier to follow without changing functionality by:
1. Replacing the long `if getattr(..., "_is_*")` chain with a clear dispatch table.
2. Making the `bot` contract explicit via a `Protocol` so the implicit requirements aren’t scattered magic.
### 1. Use a flag → handler dispatch table
This keeps the reflection, but makes the flow from decorator → registration easier to scan and extend:
```python
from collections.abc import Callable
_FLAG_HANDLERS: dict[str, Callable[..., None]] = {
"_is_slash": lambda bot, plugin, plugin_name, method, ctx: _register_slash_methods(
bot,
method,
plugin_name=plugin_name,
standalone_autocomplete=ctx["standalone_autocomplete"],
parent=ctx["parent"],
),
"_is_command_error": lambda bot, plugin, plugin_name, method, ctx: (
setattr(bot._command_error_handlers, method._command_error_for, method)
),
"_is_event": lambda bot, plugin, plugin_name, method, ctx: (
bot._event_handlers.setdefault(method._event_name, []).append(method)
),
"_is_user_command": lambda bot, plugin, plugin_name, method, ctx: _register_context_menu(
bot,
method,
plugin_name=plugin_name,
menu_type=discord.AppCommandType.user,
),
"_is_message_command": lambda bot, plugin, plugin_name, method, ctx: _register_context_menu(
bot,
method,
plugin_name=plugin_name,
menu_type=discord.AppCommandType.message,
),
"_is_component": lambda bot, plugin, plugin_name, method, ctx: _register_component(
bot, plugin, plugin_name, method
),
"_is_modal": lambda bot, plugin, plugin_name, method, ctx: _register_modal(
bot, plugin, plugin_name, method
),
"_is_ai_tool": lambda bot, plugin, plugin_name, method, ctx: _register_ai_tool(
bot, method
),
}
def scan_plugin_methods(
bot: object,
plugin: object,
*,
iter_methods: Callable[[object], list[tuple[str, Any]]],
parent=None,
) -> None:
plugin_name = getattr(plugin, "_instance_id", str(id(plugin)))
methods = iter_methods(plugin)
ctx = {
"standalone_autocomplete": _collect_standalone_autocomplete(methods),
"parent": parent,
}
for _, method in methods:
for flag, handler in _FLAG_HANDLERS.items():
if getattr(method, flag, False):
handler(bot, plugin, plugin_name, method, ctx)
```
Then pull the inline component/modal blocks into tiny helpers so everything is consistently routed via the table:
```python
def _register_component(bot: object, plugin: object, plugin_name: str, method: Callable) -> None:
custom_id = method._component_id
if getattr(method, "_component_scoped", True):
custom_id = plugin.id(custom_id)
bot._register_component_handler(
custom_id,
method,
source_plugin=plugin_name,
ttl=getattr(method, "_component_ttl", None),
)
def _register_modal(bot: object, plugin: object, plugin_name: str, method: Callable) -> None:
custom_id = method._modal_id
if getattr(method, "_modal_scoped", True):
custom_id = plugin.id(custom_id)
bot._register_modal_handler(custom_id, method, source_plugin=plugin_name)
```
This keeps all the existing helpers but centralizes “which flag triggers which registration” in one place.
### 2. Make the bot contract explicit via a Protocol
Instead of `bot: object`, define a local protocol to document and type‑check all the expected internals. This addresses the “implicit contract” concern without changing behavior:
```python
from typing import Protocol, Any, Callable
class _SupportsPluginRegistration(Protocol):
_command_error_handlers: dict[Any, Callable]
_event_handlers: dict[str, list[Callable]]
def _register_slash(self, func: Callable, **kwargs: Any) -> None: ...
def _register_context_menu(self, func: Callable, **kwargs: Any) -> None: ...
def _register_component_handler(self, *args: Any, **kwargs: Any) -> None: ...
def _register_modal_handler(self, *args: Any, **kwargs: Any) -> None: ...
ai_tools: dict[str, dict[str, Any]]
tool_registry: Any # can be tightened if you have a concrete type
def scan_plugin_methods(
bot: _SupportsPluginRegistration,
plugin: object,
*,
iter_methods: Callable[[object], list[tuple[str, Any]]],
parent=None,
) -> None:
...
```
You still don’t leak this as a public interface, but internally it makes the requirements explicit, easier to reason about, and easier to adapt if the bot implementation changes.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| logger = logging.getLogger("easycord") | ||
|
|
||
|
|
||
| def register_slash( |
There was a problem hiding this comment.
issue (complexity): Consider refactoring this helper module by collapsing the wide register_slash signature into a config object, simplifying the autocomplete handler function, and introducing a Protocol for the bot type to reduce surface area and indirection.
You can keep this helper module but significantly reduce its surface area and indirection cost with a couple of targeted refactors.
1. Collapse the wide register_slash signature into a config object
Instead of threading every option as a top‑level parameter, group them into a small config/dataclass. This reduces call‑site noise and makes it clear that these options travel together.
from dataclasses import dataclass
from typing import Callable, Mapping
@dataclass
class SlashCommandConfig:
name: str
description: str
guild_id: int | None
guild_only: bool = False
require_admin: bool = False
ephemeral: bool = False
permissions: list[str] | None = None
cooldown: float | None = None
cooldown_rate: int = 1
cooldown_bucket: str = "user"
premium_required: bool = False
autocomplete: Mapping[str, Callable] | None = None
choices: Mapping[str, list] | None = None
nsfw: bool = False
allowed_contexts: app_commands.AppCommandContext | None = None
allowed_installs: app_commands.AppInstallationType | None = None
parent: app_commands.Group | None = None
source_plugin: str | None = NoneThen:
def register_slash(
bot: "SupportsSlashRegistration",
func: Callable,
*,
callback_builder: Callable[..., Callable],
context_factory: Callable[[discord.Interaction], object],
config: SlashCommandConfig,
) -> None:
guild = discord.Object(id=config.guild_id) if config.guild_id else None
callback = callback_builder(
func,
guild_only=config.guild_only,
require_admin=config.require_admin,
ephemeral=config.ephemeral,
permissions=config.permissions,
cooldown=config.cooldown,
cooldown_rate=config.cooldown_rate,
cooldown_bucket=config.cooldown_bucket,
premium_required=config.premium_required,
command_name=config.name,
)
autocomplete_handlers: dict[str, Callable] = {
**(config.autocomplete or {}),
**getattr(func, "_slash_autocomplete_handlers", {}),
}
if config.choices:
inject_choices(callback, config.choices)
cmd = app_commands.Command(
name=config.name,
description=config.description,
callback=callback,
nsfw=config.nsfw,
allowed_contexts=config.allowed_contexts,
allowed_installs=config.allowed_installs,
)
for param_name, handler in autocomplete_handlers.items():
_register_autocomplete_handler(
bot,
cmd,
name=config.name,
param_name=param_name,
handler=handler,
source_plugin=config.source_plugin,
guild_id=config.guild_id,
context_factory=context_factory,
)
if config.parent is not None:
config.parent.add_command(cmd)
registry_name = f"{config.parent.name} {config.name}"
else:
bot.tree.add_command(cmd, guild=guild)
registry_name = config.name
bot.registry.register_slash_command(
registry_name,
func,
source_plugin=config.source_plugin,
guild_id=config.guild_id,
metadata={
"description": config.description,
"guild_only": config.guild_only,
"permissions": config.permissions,
"cooldown": config.cooldown,
"cooldown_rate": config.cooldown_rate,
"cooldown_bucket": config.cooldown_bucket,
"premium_required": config.premium_required,
"allowed_contexts": config.allowed_contexts,
"allowed_installs": config.allowed_installs,
"parent": getattr(config.parent, "name", None),
},
)Call sites become:
config = SlashCommandConfig(
name="ping",
description="Ping the bot",
guild_id=guild_id,
guild_only=True,
# other options...
)
register_slash(bot, func, callback_builder=..., context_factory=..., config=config)This keeps all existing behaviour but drastically narrows the function signature and improves readability.
2. Remove one level of nesting from _register_autocomplete_handler
You can avoid _make_autocomplete entirely by using a single inner function with closed‑over variables or a partial. That keeps the logic in one place and makes the flow easier to follow:
def _register_autocomplete_handler(
bot: "SupportsAutocompleteRegistration",
cmd: app_commands.Command,
*,
name: str,
param_name: str,
handler: Callable,
source_plugin: str | None,
guild_id: int | None,
context_factory: Callable[[discord.Interaction], object],
) -> None:
sig = inspect.signature(handler)
params = list(sig.parameters.values())
param_count = len(params)
if param_count not in (1, 3):
plugin_info = f" in plugin {source_plugin}" if source_plugin else ""
raise TypeError(
f"Invalid autocomplete signature for option {param_name!r} "
f"of command {name!r}{plugin_info}. "
f"Expected (current) or (ctx, current, options), got {sig}."
)
expects_options = (param_count == 3)
async def ac_callback(interaction: discord.Interaction, current: str) -> list[app_commands.Choice]:
ctx = context_factory(interaction)
options = autocomplete_options(interaction)
try:
if expects_options:
results = await handler(ctx, current, options)
else:
results = await handler(current)
return [app_commands.Choice(name=r, value=r) for r in results]
except Exception as exc:
plugin_instance = getattr(handler, "__self__", None)
if plugin_instance is None and source_plugin:
plugin_instance = next(
(
p
for p in bot._plugins
if getattr(p, "_instance_id", type(p).__name__) == source_plugin
),
None,
)
await bot._dispatch_framework_error(
exc,
ctx=ctx,
plugin_instance=plugin_instance,
)
return []
cmd.autocomplete(param_name)(ac_callback)
bot.registry.register_autocomplete(
name,
param_name,
handler,
source_plugin=source_plugin,
guild_id=guild_id,
)Same behaviour, but one less layer of abstraction (no _make_autocomplete), which directly addresses the “spread across three helpers” concern.
3. Narrow the “bot as object” surface with a lightweight protocol
You can keep this as a helper module but still make it clearer what bot must provide, reducing “untyped bag of attributes” feel without changing runtime behaviour:
from typing import Protocol, Any
class SupportsSlashRegistration(Protocol):
tree: Any
registry: Any
_plugins: list[Any]
async def _dispatch_framework_error(self, exc: Exception, *, ctx: Any, plugin_instance: Any | None) -> None: ...
class SupportsAutocompleteRegistration(SupportsSlashRegistration, Protocol):
passThen annotate parameters:
def register_slash(
bot: SupportsSlashRegistration,
func: Callable,
*,
callback_builder: Callable[..., Callable],
context_factory: Callable[[discord.Interaction], object],
config: SlashCommandConfig,
) -> None:
...This doesn’t change the implementation or the call sites but makes the helper’s expectations explicit and easier to reason about.
| from .tools import ToolSafety | ||
|
|
||
|
|
||
| def scan_plugin_methods( |
There was a problem hiding this comment.
issue (complexity): Consider refactoring the plugin scanning logic to use a dispatch table and a typed bot protocol so the registration flow is clearer and less meta-reflective.
You can reduce the “meta-feel” and make the flow easier to follow without changing functionality by:
- Replacing the long
if getattr(..., "_is_*")chain with a clear dispatch table. - Making the
botcontract explicit via aProtocolso the implicit requirements aren’t scattered magic.
1. Use a flag → handler dispatch table
This keeps the reflection, but makes the flow from decorator → registration easier to scan and extend:
from collections.abc import Callable
_FLAG_HANDLERS: dict[str, Callable[..., None]] = {
"_is_slash": lambda bot, plugin, plugin_name, method, ctx: _register_slash_methods(
bot,
method,
plugin_name=plugin_name,
standalone_autocomplete=ctx["standalone_autocomplete"],
parent=ctx["parent"],
),
"_is_command_error": lambda bot, plugin, plugin_name, method, ctx: (
setattr(bot._command_error_handlers, method._command_error_for, method)
),
"_is_event": lambda bot, plugin, plugin_name, method, ctx: (
bot._event_handlers.setdefault(method._event_name, []).append(method)
),
"_is_user_command": lambda bot, plugin, plugin_name, method, ctx: _register_context_menu(
bot,
method,
plugin_name=plugin_name,
menu_type=discord.AppCommandType.user,
),
"_is_message_command": lambda bot, plugin, plugin_name, method, ctx: _register_context_menu(
bot,
method,
plugin_name=plugin_name,
menu_type=discord.AppCommandType.message,
),
"_is_component": lambda bot, plugin, plugin_name, method, ctx: _register_component(
bot, plugin, plugin_name, method
),
"_is_modal": lambda bot, plugin, plugin_name, method, ctx: _register_modal(
bot, plugin, plugin_name, method
),
"_is_ai_tool": lambda bot, plugin, plugin_name, method, ctx: _register_ai_tool(
bot, method
),
}
def scan_plugin_methods(
bot: object,
plugin: object,
*,
iter_methods: Callable[[object], list[tuple[str, Any]]],
parent=None,
) -> None:
plugin_name = getattr(plugin, "_instance_id", str(id(plugin)))
methods = iter_methods(plugin)
ctx = {
"standalone_autocomplete": _collect_standalone_autocomplete(methods),
"parent": parent,
}
for _, method in methods:
for flag, handler in _FLAG_HANDLERS.items():
if getattr(method, flag, False):
handler(bot, plugin, plugin_name, method, ctx)Then pull the inline component/modal blocks into tiny helpers so everything is consistently routed via the table:
def _register_component(bot: object, plugin: object, plugin_name: str, method: Callable) -> None:
custom_id = method._component_id
if getattr(method, "_component_scoped", True):
custom_id = plugin.id(custom_id)
bot._register_component_handler(
custom_id,
method,
source_plugin=plugin_name,
ttl=getattr(method, "_component_ttl", None),
)
def _register_modal(bot: object, plugin: object, plugin_name: str, method: Callable) -> None:
custom_id = method._modal_id
if getattr(method, "_modal_scoped", True):
custom_id = plugin.id(custom_id)
bot._register_modal_handler(custom_id, method, source_plugin=plugin_name)This keeps all the existing helpers but centralizes “which flag triggers which registration” in one place.
2. Make the bot contract explicit via a Protocol
Instead of bot: object, define a local protocol to document and type‑check all the expected internals. This addresses the “implicit contract” concern without changing behavior:
from typing import Protocol, Any, Callable
class _SupportsPluginRegistration(Protocol):
_command_error_handlers: dict[Any, Callable]
_event_handlers: dict[str, list[Callable]]
def _register_slash(self, func: Callable, **kwargs: Any) -> None: ...
def _register_context_menu(self, func: Callable, **kwargs: Any) -> None: ...
def _register_component_handler(self, *args: Any, **kwargs: Any) -> None: ...
def _register_modal_handler(self, *args: Any, **kwargs: Any) -> None: ...
ai_tools: dict[str, dict[str, Any]]
tool_registry: Any # can be tightened if you have a concrete type
def scan_plugin_methods(
bot: _SupportsPluginRegistration,
plugin: object,
*,
iter_methods: Callable[[object], list[tuple[str, Any]]],
parent=None,
) -> None:
...You still don’t leak this as a public interface, but internally it makes the requirements explicit, easier to reason about, and easier to adapt if the bot implementation changes.
|
Warning Review limit reached
More reviews will be available in 46 minutes and 49 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR extracts slash/context-menu callback building ( ChangesEasyCord: Command Registration Extraction, i18n Modularization, Polls Persistence & Plugin Hardening
Sequence Diagram(s)sequenceDiagram
participant Plugin as Plugin method
participant scan_plugin_methods
participant _register_slash_methods
participant register_slash
participant build_slash_callback
participant bot_tree as bot.tree
Plugin->>scan_plugin_methods: iter_methods(plugin)
scan_plugin_methods->>_register_slash_methods: method metadata + standalone autocomplete
_register_slash_methods->>register_slash: name, description, guild_id, autocomplete, ...
register_slash->>build_slash_callback: func, context_factory, chain_builder, guards
build_slash_callback-->>register_slash: async callback
register_slash->>bot_tree: add_command(app_commands.Command)
register_slash->>bot_tree: set_translation_hook / autocomplete wiring
sequenceDiagram
participant PollCmd as /poll command
participant ServerConfigStore
participant _PollView
participant bot as bot.add_view
participant _poll_timer
PollCmd->>ServerConfigStore: save poll (status=active, votes={}, end_time)
PollCmd->>_PollView: construct with plugin/guild/message ids
PollCmd->>bot: add_view(_PollView)
PollCmd->>_poll_timer: schedule(duration)
Note over _PollView: vote interaction → acquire lock → load → update votes → save
_poll_timer->>ServerConfigStore: mark status=closed (under lock)
_poll_timer->>_PollView: disable_all()
Note over _poll_timer: on_ready: resume active or close overdue polls
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Around line 68-74: The import path for SENDABLE_CHANNEL_TYPES in the code
example is incorrect. Update the import statement in the CLAUDE.md file from
`from easycord.helpers.tools import SENDABLE_CHANNEL_TYPES` to `from
easycord.plugins._utils import SENDABLE_CHANNEL_TYPES` to reflect the correct
location where SENDABLE_CHANNEL_TYPES is actually defined. Keep the rest of the
code example (the isinstance check and channel.send call) unchanged.
- Around line 76-84: The documentation contains an incorrect import path for the
SENDABLE_CHANNEL_TYPES constant. Find the import statement that currently shows
importing SENDABLE_CHANNEL_TYPES from easycord.helpers.tools and update it to
import from easycord.plugins._utils instead, as that is where the constant is
actually defined. Ensure the corrected import path matches the actual module
location in the codebase.
In `@easycord/_command_callbacks.py`:
- Around line 164-181: The build_context_menu_callback function lacks error
handling that exists in build_slash_callback, causing exceptions from func(ctx,
target) to propagate without routing through bot._error_handler or plugin
on_error hooks. Add a try/except block around the await func(ctx, target) call
within the invoke() async function to catch exceptions and route them through
the same error handling chain as build_slash_callback, ensuring consistent error
handling behavior across command types.
In `@easycord/_i18n_locale.py`:
- Around line 23-29: The detect_os_locale function incorrectly interprets the
second element of stdlib_locale.getdefaultlocale() as a region code when it is
actually the encoding (like UTF-8). Remove the line that assigns
system_locale[1] to the country variable and the logic that attempts to combine
language and country codes. Instead, directly use system_locale[0] which
contains the full locale information (already in the format lang_COUNTRY if
available), normalize it, and return it. This prevents creating invalid locales
like en_UTF-8.
In `@easycord/plugins/ai_moderator.py`:
- Around line 251-252: Replace all hardcoded strings in the await ctx.send calls
throughout the ai_moderator.py plugin with localization key lookups via
LocalizationManager. Instead of passing hardcoded strings like "✅ Moderation
{status}", use LocalizationManager to retrieve the appropriate localization keys
for each response message. Apply this fix to all instances mentioned including
the ctx.send calls at lines 251-252, 271-272, 278-279, 281-282, 288-289,
295-296, and 306, ensuring that all plugin responses follow the localization
requirement rather than bypassing it with hardcoded text.
- Around line 98-100: The prompt template at lines 98-100 embeds user-controlled
fields (message.content and message.author.name) directly into XML-like tags
without escaping, which allows delimiter injection attacks where crafted input
can break the tag structure and influence model instructions. Escape both
message.content and message.author.name by applying XML/HTML entity escaping
(replacing &, <, >, ", and ' with their respective entity codes) before
embedding them into the f-string template within the message and author tags.
In `@easycord/plugins/moderation.py`:
- Around line 133-137: Replace all hardcoded error message strings in the
ctx.respond() calls throughout the moderation.py plugin with localization-key
lookups using the ctx.t() method. This includes the permission check messages
and server-only restriction messages at lines 134-137, 159-163, 186-190,
209-213, 238-242, 324-328, 359-363, and 389-390. For each hardcoded string,
create an appropriate localization key and pass it to ctx.t() to retrieve the
localized text from the LocalizationManager instead of embedding the text
directly.
In `@easycord/plugins/polls.py`:
- Around line 64-65: The footer string construction in line 64 hardcodes English
text "📊 Poll closed" and "⏱️ Closes in {seconds_remaining:.0f}s" instead of
using localized strings. Replace these hardcoded strings by defining
localization keys and looking them up through LocalizationManager as required by
the coding guidelines for plugins. Also apply the same localization approach to
the strings in lines 119-121 that were similarly identified as needing
localization. Ensure all user-facing strings in the poll responses are retrieved
from the LocalizationManager rather than hardcoded directly.
- Around line 221-223: In the `_schedule_timer` method, before assigning the new
task to `self._timers.setdefault(guild_id, {})[message_id]`, check if a task
already exists for that message_id in the guild's timers dictionary. If a task
exists, cancel it using its cancel() method before overwriting the reference
with the new task. This prevents orphaned timers from running when the same poll
is rescheduled (such as on reconnect when on_ready runs again).
- Around line 29-33: The _tally function trusts persisted vote index values
without validation and can raise IndexError when accessing the counts array with
an out-of-bounds index. Add bounds checking in the loop that iterates through
votes.values() to ensure each idx is within the valid range (0 to
len(options)-1) before using it to access counts[idx], skipping any invalid
indices to gracefully handle stale or corrupted poll data.
- Around line 253-256: The except clause in the
channel.fetch_message(message_id) call currently catches only discord.NotFound
exception, but this misses other HTTP failures like forbidden or transient API
errors. Replace the except clause to catch the broader discord.HTTPException
instead, which will handle all HTTP-related errors and prevent noisy task
failures from unhandled exceptions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4c337edb-67ab-4c35-b9fd-e79463e61256
📒 Files selected for processing (30)
CLAUDE.mdeasycord/_bot_commands.pyeasycord/_bot_plugins.pyeasycord/_command_callbacks.pyeasycord/_command_registration.pyeasycord/_i18n_diagnostics.pyeasycord/_i18n_locale.pyeasycord/_i18n_validation.pyeasycord/_plugin_scanner.pyeasycord/bot.pyeasycord/helpers/config.pyeasycord/helpers/tools.pyeasycord/i18n.pyeasycord/plugins/ai_moderator.pyeasycord/plugins/birthday.pyeasycord/plugins/giveaway.pyeasycord/plugins/levels.pyeasycord/plugins/moderation.pyeasycord/plugins/polls.pyeasycord/plugins/reminder.pyeasycord/plugins/scheduled_announcements.pyeasycord/plugins/server_stats.pyeasycord/plugins/starboard.pyeasycord/plugins/tickets.pyeasycord/plugins/welcome.pyeasycord/testing.pyeasycord/utils/paginator.pytests/test_plugin_commands.pytests/test_plugins.pytests/test_plugins_new.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Install with dev dependencies using: pip install -e ".[dev]"
Build distribution package using: python -m build
Files:
easycord/plugins/server_stats.pyeasycord/plugins/reminder.pyeasycord/plugins/scheduled_announcements.pyeasycord/plugins/levels.pyeasycord/_i18n_validation.pyeasycord/plugins/birthday.pyeasycord/bot.pyeasycord/plugins/welcome.pyeasycord/_bot_plugins.pyeasycord/utils/paginator.pyeasycord/plugins/tickets.pyeasycord/plugins/giveaway.pyeasycord/_i18n_diagnostics.pyeasycord/_i18n_locale.pyeasycord/helpers/config.pyeasycord/helpers/tools.pyeasycord/_command_callbacks.pyeasycord/plugins/starboard.pytests/test_plugin_commands.pytests/test_plugins_new.pyeasycord/plugins/ai_moderator.pyeasycord/testing.pyeasycord/_plugin_scanner.pyeasycord/i18n.pytests/test_plugins.pyeasycord/_command_registration.pyeasycord/plugins/polls.pyeasycord/_bot_commands.pyeasycord/plugins/moderation.py
easycord/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
easycord/**/*.py: The@ai_tooldecorator must be used to register functions into ToolRegistry and requires explicit ToolSafety permission annotation
Use ctx.user / ctx.member in command handlers — ctx.author does not exist
ctx.is_admin is a property, not a method — do not call it as ctx.is_admin()
Cooldown sentinels default to float("-inf"), not 0.0 — ensures first-message events pass on fresh runners
All ToolLimiter methods are async and must be awaited
Files:
easycord/plugins/server_stats.pyeasycord/plugins/reminder.pyeasycord/plugins/scheduled_announcements.pyeasycord/plugins/levels.pyeasycord/_i18n_validation.pyeasycord/plugins/birthday.pyeasycord/bot.pyeasycord/plugins/welcome.pyeasycord/_bot_plugins.pyeasycord/utils/paginator.pyeasycord/plugins/tickets.pyeasycord/plugins/giveaway.pyeasycord/_i18n_diagnostics.pyeasycord/_i18n_locale.pyeasycord/helpers/config.pyeasycord/helpers/tools.pyeasycord/_command_callbacks.pyeasycord/plugins/starboard.pyeasycord/plugins/ai_moderator.pyeasycord/testing.pyeasycord/_plugin_scanner.pyeasycord/i18n.pyeasycord/_command_registration.pyeasycord/plugins/polls.pyeasycord/_bot_commands.pyeasycord/plugins/moderation.py
easycord/plugins/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Localization keys must be looked up via LocalizationManager; strings must not be hardcoded in plugin responses
Files:
easycord/plugins/server_stats.pyeasycord/plugins/reminder.pyeasycord/plugins/scheduled_announcements.pyeasycord/plugins/levels.pyeasycord/plugins/birthday.pyeasycord/plugins/welcome.pyeasycord/plugins/tickets.pyeasycord/plugins/giveaway.pyeasycord/plugins/starboard.pyeasycord/plugins/ai_moderator.pyeasycord/plugins/polls.pyeasycord/plugins/moderation.py
{easycord,tests}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Verify code compiles correctly using: python -m compileall -q easycord tests
Files:
easycord/plugins/server_stats.pyeasycord/plugins/reminder.pyeasycord/plugins/scheduled_announcements.pyeasycord/plugins/levels.pyeasycord/_i18n_validation.pyeasycord/plugins/birthday.pyeasycord/bot.pyeasycord/plugins/welcome.pyeasycord/_bot_plugins.pyeasycord/utils/paginator.pyeasycord/plugins/tickets.pyeasycord/plugins/giveaway.pyeasycord/_i18n_diagnostics.pyeasycord/_i18n_locale.pyeasycord/helpers/config.pyeasycord/helpers/tools.pyeasycord/_command_callbacks.pyeasycord/plugins/starboard.pytests/test_plugin_commands.pytests/test_plugins_new.pyeasycord/plugins/ai_moderator.pyeasycord/testing.pyeasycord/_plugin_scanner.pyeasycord/i18n.pytests/test_plugins.pyeasycord/_command_registration.pyeasycord/plugins/polls.pyeasycord/_bot_commands.pyeasycord/plugins/moderation.py
easycord/{bot,_bot_*.py}
📄 CodeRabbit inference engine (AGENTS.md)
Bot core is defined in bot.py via multiple inheritance: discord.Client + four mixins (_bot_commands.py, _bot_events.py, _bot_guild.py, _bot_plugins.py); bot-level behavior should be added to the appropriate mixin file
Files:
easycord/_bot_plugins.pyeasycord/_bot_commands.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Run all tests using: pytest tests/
Tests use pytest-asyncio with asyncio_mode = "auto" — async tests need no manual event loop setup
Files:
tests/test_plugin_commands.pytests/test_plugins_new.pytests/test_plugins.py
🪛 GitHub Check: CodeQL
easycord/_i18n_locale.py
[notice] 30-30: Empty except
'except' clause does nothing but pass and there is no explanatory comment.
easycord/_bot_commands.py
[notice] 141-141: Unnecessary lambda
This 'lambda' is just a simple wrapper around a callable object. Use that object directly.
[notice] 142-146: Unnecessary lambda
This 'lambda' is just a simple wrapper around a callable object. Use that object directly.
[notice] 186-186: Unnecessary lambda
This 'lambda' is just a simple wrapper around a callable object. Use that object directly.
[notice] 309-309: Unnecessary lambda
This 'lambda' is just a simple wrapper around a callable object. Use that object directly.
[notice] 310-314: Unnecessary lambda
This 'lambda' is just a simple wrapper around a callable object. Use that object directly.
🔇 Additional comments (42)
easycord/helpers/tools.py (1)
33-44: LGTM!easycord/helpers/config.py (1)
52-53: LGTM!easycord/bot.py (1)
265-265: LGTM!easycord/utils/paginator.py (1)
243-254: LGTM!CLAUDE.md (1)
33-35: All documentation claims verified as accurate.Lines 33–35 correctly document:
ctx.t(...)exists in_context_base.pywith localization lookup behavior- i18n module is properly split into
_i18n_locale.py,_i18n_diagnostics.py, and_i18n_validation.pyDiagnosticModeenum includesSILENT,WARN,STRICTmodesFallbackStrategyraisesIndexErrorwhen all providers are exhaustedToolLimitermethods (check_limit,reset_user,reset_tool) are all async- AI providers are lazy-imported via
easycord.__getattr__fromeasycord.plugins._ai_providerseasycord/_bot_commands.py (7)
138-156: 💤 Low valueUnnecessary lambda wrappers (CodeQL).
The
context_factoryandchain_builderlambdas wrap callables without adding behavior. IfContextandbuild_chainare stable module-level symbols, pass them directly.Source: Linters/SAST tools
182-205: 💤 Low valueUnnecessary lambda for
context_factory(CodeQL).Same issue as above—the lambda at line 186 is a simple wrapper.
Source: Linters/SAST tools
305-322: 💤 Low valueUnnecessary lambdas for
context_factoryandchain_builder(CodeQL).Same pattern at lines 309–314.
Source: Linters/SAST tools
6-19: LGTM!Also applies to: 34-96
97-137: LGTM!Also applies to: 157-181
206-304: LGTM!
323-331: LGTM!easycord/_command_callbacks.py (3)
1-10: LGTM!Also applies to: 27-35
36-98: LGTM!
99-161: LGTM!easycord/_command_registration.py (4)
1-103: LGTM!
105-173: LGTM!
175-242: LGTM!
244-268: LGTM!easycord/_plugin_scanner.py (5)
1-10: LGTM!
12-68: LGTM!
70-137: LGTM!
140-163: LGTM!Also applies to: 179-181
164-178: TheToolRegistryclass does not expose a public method to check if a tool is already registered. Accessing_toolsis the only available approach, and this pattern is consistent with howToolRegistryitself checks tool existence internally (e.g., in theregister()method at line 88 ofeasycord/tools.py).> Likely an incorrect or invalid review comment.easycord/_bot_plugins.py (1)
11-11: LGTM!Also applies to: 41-47
easycord/plugins/starboard.py (1)
11-11: LGTM!Also applies to: 100-102, 160-161, 196-197, 209-209, 234-235, 250-250
easycord/plugins/levels.py (1)
210-210: LGTM!easycord/plugins/welcome.py (1)
76-76: LGTM!easycord/plugins/birthday.py (1)
157-157: LGTM!Also applies to: 263-263
easycord/plugins/giveaway.py (1)
173-173: LGTM!Also applies to: 202-202, 248-248
easycord/plugins/reminder.py (1)
138-138: LGTM!easycord/plugins/scheduled_announcements.py (1)
167-167: LGTM!easycord/plugins/server_stats.py (1)
100-100: LGTM!easycord/plugins/tickets.py (1)
216-216: LGTM!Also applies to: 267-267, 272-272, 349-349
easycord/_i18n_diagnostics.py (1)
10-92: LGTM!easycord/_i18n_locale.py (1)
8-17: LGTM!Also applies to: 35-106
easycord/_i18n_validation.py (1)
5-72: LGTM!easycord/i18n.py (1)
9-17: LGTM!Also applies to: 145-149, 209-209, 286-289
easycord/testing.py (1)
96-97: LGTM!Also applies to: 151-153, 181-190, 321-322, 353-353, 379-381, 416-416, 449-449, 494-494, 529-529
tests/test_plugin_commands.py (1)
19-56: LGTM!Also applies to: 154-158, 174-252
tests/test_plugins.py (1)
227-284: LGTM!tests/test_plugins_new.py (1)
18-27: LGTM!Also applies to: 430-475
Comment on lines
+68
to
+74
| **Channel send safety** — before calling `.send()` on any channel obtained from `ctx` or Discord, narrow its type first. Use the `SENDABLE_CHANNEL_TYPES` tuple (defined in `easycord/helpers/tools.py` or a local `_utils.py`). Bare `.send()` on unnarrowed channel types will fail at runtime on DM-incompatible channels. | ||
|
|
||
| ```python | ||
| from easycord.helpers.tools import SENDABLE_CHANNEL_TYPES | ||
| if isinstance(channel, SENDABLE_CHANNEL_TYPES): | ||
| await channel.send(...) | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify SENDABLE_CHANNEL_TYPES constant and testing utilities
echo "=== 1. Check SENDABLE_CHANNEL_TYPES in helpers/tools.py ==="
rg -n 'SENDABLE_CHANNEL_TYPES' easycord/helpers/tools.py
echo -e "\n=== 2. Verify FakeContextBuilder class and methods ==="
rg -A20 'class FakeContextBuilder' easycord/testing.py
echo -e "\n=== 3. Verify invoke() function signature ==="
rg -A5 '^async def invoke\(' easycord/testing.py
echo -e "\n=== 4. Check available invoke helpers ==="
rg -n 'def invoke_' easycord/testing.py | head -20
echo -e "\n=== 5. Verify plugin __new__() pattern documentation ==="
rg -B2 -A2 '__new__' easycord/plugin.pyRepository: rolling-codes/EasyCord
Length of output: 1417
🏁 Script executed:
#!/bin/bash
# Search entire codebase for SENDABLE_CHANNEL_TYPES
echo "=== Searching for SENDABLE_CHANNEL_TYPES anywhere in codebase ==="
rg -r '$file:$line: $match' 'SENDABLE_CHANNEL_TYPES' --max-count 20
echo -e "\n=== Get full FakeContextBuilder class with all methods ==="
rg -A100 'class FakeContextBuilder' easycord/testing.py | head -120
echo -e "\n=== Search for sendable/send related constants ==="
rg -i 'sendable|send.*channel' easycord/helpers/tools.py | head -20
echo -e "\n=== Check imports in CLAUDE.md to understand context ==="
cat CLAUDE.md | grep -A5 -B5 'SENDABLE_CHANNEL_TYPES'Repository: rolling-codes/EasyCord
Length of output: 5652
🏁 Script executed:
#!/bin/bash
# Get exact definition from _utils.py
echo "=== SENDABLE_CHANNEL_TYPES definition in easycord/plugins/_utils.py ==="
rg -B2 -A8 'SENDABLE_CHANNEL_TYPES\s*=' easycord/plugins/_utils.py
echo -e "\n=== Verify actual imports in plugin files ==="
rg 'from easycord.plugins._utils import' easycord/plugins/*.py | head -5Repository: rolling-codes/EasyCord
Length of output: 865
Fix import path for SENDABLE_CHANNEL_TYPES — it's defined in easycord/plugins/_utils.py, not easycord/helpers/tools.py. The code example below shows the correct import:
from easycord.plugins._utils import SENDABLE_CHANNEL_TYPES
if isinstance(channel, SENDABLE_CHANNEL_TYPES):
await channel.send(...)This tuple includes discord.TextChannel, discord.Thread, and discord.VoiceChannel. Always narrow channel types before calling .send() to avoid runtime failures on DM-incompatible channels.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CLAUDE.md` around lines 68 - 74, The import path for SENDABLE_CHANNEL_TYPES
in the code example is incorrect. Update the import statement in the CLAUDE.md
file from `from easycord.helpers.tools import SENDABLE_CHANNEL_TYPES` to `from
easycord.plugins._utils import SENDABLE_CHANNEL_TYPES` to reflect the correct
location where SENDABLE_CHANNEL_TYPES is actually defined. Keep the rest of the
code example (the isinstance check and channel.send call) unchanged.
Comment on lines
+76
to
+84
| ## Key invariants | ||
|
|
||
| - `ToolLimiter` methods are async — always `await check_limit(...)`. | ||
| - Cooldown sentinels in `LevelsPlugin._cooldowns` default to `float("-inf")`, not `0.0` — ensures first message always passes. | ||
| - `ctx.is_admin` is a property — never call `ctx.is_admin()`. | ||
| - `ctx.user` / `ctx.member` are correct; `ctx.author` does not exist. | ||
| - `@ai_tool` requires an explicit `ToolSafety` annotation to register. | ||
| - CI actions are pinned to `actions/checkout@v4` and `actions/setup-python@v5` — v6 does not exist. | ||
| - `sync_commands()` raises `RuntimeError` on removals unless `confirm_removals=True` is passed explicitly. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify CI action versions in workflow files
echo "=== Check GitHub Actions workflow files ==="
find .github/workflows -name "*.yml" -o -name "*.yaml" | while read f; do
echo "File: $f"
grep -E 'actions/(checkout|setup-python)`@v`' "$f" || echo " No matching actions found"
doneRepository: rolling-codes/EasyCord
Length of output: 1259
🏁 Script executed:
#!/bin/bash
# 1. Check testing utilities API
echo "=== Testing utilities in easycord/testing.py ==="
if [ -f "easycord/testing.py" ]; then
echo "File exists. Checking for FakeContextBuilder and related methods..."
rg -A 3 "class FakeContextBuilder|def with_user|def invoke" easycord/testing.py
else
echo "easycord/testing.py not found"
fi
echo ""
echo "=== SENDABLE_CHANNEL_TYPES in easycord/helpers/tools.py ==="
if [ -f "easycord/helpers/tools.py" ]; then
rg "SENDABLE_CHANNEL_TYPES" easycord/helpers/tools.py
else
echo "easycord/helpers/tools.py not found"
fi
echo ""
echo "=== AI orchestration: __getattr__ pattern in easycord ==="
find easycord -name "__init__.py" -o -name "_ai_providers.py" | head -20
echo ""
echo "=== Check for __getattr__ in easycord ==="
rg -l "__getattr__" easycord/Repository: rolling-codes/EasyCord
Length of output: 1247
🏁 Script executed:
#!/bin/bash
# 1. Find where SENDABLE_CHANNEL_TYPES is actually defined
echo "=== Search for SENDABLE_CHANNEL_TYPES everywhere ==="
rg "SENDABLE_CHANNEL_TYPES" --type py
echo ""
echo "=== Check __getattr__ in easycord/__init__.py ==="
rg -B 2 -A 10 "__getattr__" easycord/__init__.py
echo ""
echo "=== Check the actual CLAUDE.md content around lines 35, 72, and 76-84 ==="
if [ -f "CLAUDE.md" ]; then
echo "=== Lines 30-40 (AI orchestration) ==="
sed -n '30,40p' CLAUDE.md
echo ""
echo "=== Lines 68-78 (SENDABLE_CHANNEL_TYPES) ==="
sed -n '68,78p' CLAUDE.md
echo ""
echo "=== Lines 76-90 (Key invariants) ==="
sed -n '76,90p' CLAUDE.md
else
echo "CLAUDE.md not found"
fiRepository: rolling-codes/EasyCord
Length of output: 4530
Fix incorrect import path for SENDABLE_CHANNEL_TYPES in CLAUDE.md documentation.
Line 72 shows an import example that is incorrect: from easycord.helpers.tools import SENDABLE_CHANNEL_TYPES will fail. The constant is actually defined in easycord/plugins/_utils.py, not easycord/helpers/tools.py. The documented code example and module path need to be corrected.
Verification confirms all other documented invariants are accurate: CI actions are pinned to actions/checkout@v4 and actions/setup-python@v5 (v6 does not exist), the AI orchestration lazy-import pattern via easycord.__getattr__ and _ai_providers.py is correctly described, and the testing utilities API signatures (FakeContextBuilder.with_user(), invoke(), etc.) match the implementation in easycord/testing.py.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CLAUDE.md` around lines 76 - 84, The documentation contains an incorrect
import path for the SENDABLE_CHANNEL_TYPES constant. Find the import statement
that currently shows importing SENDABLE_CHANNEL_TYPES from
easycord.helpers.tools and update it to import from easycord.plugins._utils
instead, as that is where the constant is actually defined. Ensure the corrected
import path matches the actual module location in the codebase.
Comment on lines
+164
to
+181
| def build_context_menu_callback( | ||
| bot: Any, | ||
| func: Callable, | ||
| *, | ||
| context_factory: Callable[[discord.Interaction], Any], | ||
| chain_builder: Callable, | ||
| ) -> Callable: | ||
| """Build the callback wrapper used by app command context menus.""" | ||
|
|
||
| async def callback(interaction: discord.Interaction, target) -> None: | ||
| ctx = context_factory(interaction) | ||
|
|
||
| async def invoke() -> None: | ||
| await func(ctx, target) | ||
|
|
||
| await chain_builder(ctx, invoke, bot._middleware)() | ||
|
|
||
| return callback |
There was a problem hiding this comment.
Context menu callbacks lack error handling.
build_context_menu_callback does not wrap func(ctx, target) in a try/except like build_slash_callback does. If the handler raises, the exception propagates without routing through bot._error_handler or plugin on_error.
Consider whether this asymmetry is intentional. If context menu commands should follow the same error-handling chain, wrap the call similarly.
🛠️ Suggested fix to add error handling
async def callback(interaction: discord.Interaction, target) -> None:
ctx = context_factory(interaction)
async def invoke() -> None:
- await func(ctx, target)
+ try:
+ await func(ctx, target)
+ except Exception as exc:
+ if bot._error_handler is not None:
+ await bot._error_handler(ctx, exc)
+ else:
+ raise
await chain_builder(ctx, invoke, bot._middleware)()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@easycord/_command_callbacks.py` around lines 164 - 181, The
build_context_menu_callback function lacks error handling that exists in
build_slash_callback, causing exceptions from func(ctx, target) to propagate
without routing through bot._error_handler or plugin on_error hooks. Add a
try/except block around the await func(ctx, target) call within the invoke()
async function to catch exceptions and route them through the same error
handling chain as build_slash_callback, ensuring consistent error handling
behavior across command types.
Comment on lines
+23
to
+29
| system_locale = stdlib_locale.getdefaultlocale() | ||
| if system_locale and system_locale[0]: | ||
| lang = system_locale[0] | ||
| country = system_locale[1] | ||
| if country: | ||
| return _normalize_locale(f"{lang}_{country}") | ||
| return _normalize_locale(lang) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify the problematic tuple-index usage in the reviewed file.
rg -n -C2 'getdefaultlocale|system_locale\[1\]|lang = system_locale\[0\]|country = system_locale\[1\]' easycord/_i18n_locale.py
# Verify stdlib contract (second item is encoding).
python - <<'PY'
import locale
doc = locale.getdefaultlocale.__doc__ or ""
print("Doc first line:", doc.strip().splitlines()[0] if doc.strip() else "<no doc>")
print("Sample tuple shape:", locale.getdefaultlocale())
PYRepository: rolling-codes/EasyCord
Length of output: 625
🏁 Script executed:
# Check the _normalize_locale and is_valid_locale implementations
rg -n -A10 "def _normalize_locale|def is_valid_locale" easycord/_i18n_locale.py
# Check if detect_os_locale is tested
rg -n "detect_os_locale" easycord/_i18n_locale.py tests/
# Check build_locale_chain to understand how detect_os_locale is used
rg -n -B2 -A15 "def build_locale_chain|def build_preferred_locale_chain" easycord/_i18n_locale.pyRepository: rolling-codes/EasyCord
Length of output: 2106
🏁 Script executed:
# Get the complete is_valid_locale function
sed -n '35,62p' easycord/_i18n_locale.py
# Check for test coverage
find tests -name "*.py" -exec grep -l "detect_os_locale\|is_valid_locale\|_i18n_locale" {} \;
# Check how detect_os_locale is used
rg -n "detect_os_locale()" easycord/Repository: rolling-codes/EasyCord
Length of output: 916
🏁 Script executed:
# Verify the file compiles correctly per coding guidelines
python -m compileall -q easycord/_i18n_locale.py 2>&1 || echo "Compilation check completed"
# Also verify the i18n.py file that uses it
python -m compileall -q easycord/i18n.py 2>&1 || echo "Compilation check completed"Repository: rolling-codes/EasyCord
Length of output: 48
Fix detect_os_locale to extract only the language code, not encoding.
locale.getdefaultlocale() returns (language_code, encoding). Line 26 incorrectly treats the second element (encoding) as a region code, generating invalid locales like en-UTF-8 that fail is_valid_locale() validation and silently disable OS locale auto-detection.
Proposed fix
def detect_os_locale() -> str | None:
"""Detect the system's locale preference."""
try:
system_locale = stdlib_locale.getdefaultlocale()
if system_locale and system_locale[0]:
- lang = system_locale[0]
- country = system_locale[1]
- if country:
- return _normalize_locale(f"{lang}_{country}")
- return _normalize_locale(lang)
+ # getdefaultlocale() returns (language_code, encoding)
+ return _normalize_locale(system_locale[0])
except (AttributeError, ValueError):
pass
return None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| system_locale = stdlib_locale.getdefaultlocale() | |
| if system_locale and system_locale[0]: | |
| lang = system_locale[0] | |
| country = system_locale[1] | |
| if country: | |
| return _normalize_locale(f"{lang}_{country}") | |
| return _normalize_locale(lang) | |
| system_locale = stdlib_locale.getdefaultlocale() | |
| if system_locale and system_locale[0]: | |
| # getdefaultlocale() returns (language_code, encoding) | |
| return _normalize_locale(system_locale[0]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@easycord/_i18n_locale.py` around lines 23 - 29, The detect_os_locale function
incorrectly interprets the second element of stdlib_locale.getdefaultlocale() as
a region code when it is actually the encoding (like UTF-8). Remove the line
that assigns system_locale[1] to the country variable and the logic that
attempts to combine language and country codes. Instead, directly use
system_locale[0] which contains the full locale information (already in the
format lang_COUNTRY if available), normalize it, and return it. This prevents
creating invalid locales like en_UTF-8.
Comment on lines
+98
to
100
| f"<message>{message.content}</message>\n" | ||
| f"<author>{message.author.name}</author>\n" | ||
| f"Reply with JSON: {{'action': 'delete|warn|timeout|none', 'confidence': 0.0-1.0, 'reason': 'brief reason'}}" |
There was a problem hiding this comment.
Escape user-controlled fields before embedding them in tagged prompt sections.
Wrapping content in XML-like tags without escaping still allows delimiter injection (e.g., closing </message> in user input), so crafted messages can break structure and steer model instructions.
Suggested hardening
+from xml.sax.saxutils import escape
...
- f"<message>{message.content}</message>\n"
- f"<author>{message.author.name}</author>\n"
+ f"<message>{escape(message.content)}</message>\n"
+ f"<author>{escape(message.author.name)}</author>\n"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| f"<message>{message.content}</message>\n" | |
| f"<author>{message.author.name}</author>\n" | |
| f"Reply with JSON: {{'action': 'delete|warn|timeout|none', 'confidence': 0.0-1.0, 'reason': 'brief reason'}}" | |
| f"<message>{escape(message.content)}</message>\n" | |
| f"<author>{escape(message.author.name)}</author>\n" | |
| f"Reply with JSON: {{'action': 'delete|warn|timeout|none', 'confidence': 0.0-1.0, 'reason': 'brief reason'}}" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@easycord/plugins/ai_moderator.py` around lines 98 - 100, The prompt template
at lines 98-100 embeds user-controlled fields (message.content and
message.author.name) directly into XML-like tags without escaping, which allows
delimiter injection attacks where crafted input can break the tag structure and
influence model instructions. Escape both message.content and
message.author.name by applying XML/HTML entity escaping (replacing &, <, >, ",
and ' with their respective entity codes) before embedding them into the
f-string template within the message and author tags.
Comment on lines
+133
to
137
| await ctx.respond("❌ This command can only be used inside a server") | ||
| return | ||
| if not actor.guild_permissions.kick_members: | ||
| await ctx.respond("❌ You lack `kick_members` permission") | ||
| return |
There was a problem hiding this comment.
Replace hardcoded plugin responses with localization-key lookups.
These changed command responses are hardcoded text in a plugin module; they should be resolved through the localization manager path used by ctx.t(...) keys.
As per coding guidelines, "Localization keys must be looked up via LocalizationManager; strings must not be hardcoded in plugin responses" (easycord/plugins/**/*.py).
Also applies to: 159-163, 186-190, 209-213, 238-242, 324-328, 359-363, 389-390
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@easycord/plugins/moderation.py` around lines 133 - 137, Replace all hardcoded
error message strings in the ctx.respond() calls throughout the moderation.py
plugin with localization-key lookups using the ctx.t() method. This includes the
permission check messages and server-only restriction messages at lines 134-137,
159-163, 186-190, 209-213, 238-242, 324-328, 359-363, and 389-390. For each
hardcoded string, create an appropriate localization key and pass it to ctx.t()
to retrieve the localized text from the LocalizationManager instead of embedding
the text directly.
Source: Coding guidelines
Comment on lines
+29
to
+33
| def _tally(options: list[str], votes: dict[str, int]) -> list[int]: | ||
| counts = [0] * len(options) | ||
| for idx in votes.values(): | ||
| counts[idx] += 1 | ||
| return counts |
There was a problem hiding this comment.
Guard _tally against invalid persisted vote indices.
Line 32 trusts persisted vote values and can raise IndexError, which can break callback/embed update paths when poll data is stale or corrupted.
Suggested fix
def _tally(options: list[str], votes: dict[str, int]) -> list[int]:
counts = [0] * len(options)
for idx in votes.values():
- counts[idx] += 1
+ if isinstance(idx, int) and 0 <= idx < len(counts):
+ counts[idx] += 1
return counts🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@easycord/plugins/polls.py` around lines 29 - 33, The _tally function trusts
persisted vote index values without validation and can raise IndexError when
accessing the counts array with an out-of-bounds index. Add bounds checking in
the loop that iterates through votes.values() to ensure each idx is within the
valid range (0 to len(options)-1) before using it to access counts[idx],
skipping any invalid indices to gracefully handle stale or corrupted poll data.
Comment on lines
+64
to
+65
| footer = "📊 Poll closed" if closed else f"⏱️ Closes in {seconds_remaining:.0f}s" | ||
| embed = discord.Embed( |
There was a problem hiding this comment.
Localize poll user-facing strings instead of hardcoding English text.
Lines 64-65 and Lines 119-121 return literal strings in plugin responses. Route these through localization keys and LocalizationManager lookups.
As per coding guidelines, easycord/plugins/**/*.py: "Localization keys must be looked up via LocalizationManager; strings must not be hardcoded in plugin responses."
Also applies to: 119-121
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@easycord/plugins/polls.py` around lines 64 - 65, The footer string
construction in line 64 hardcodes English text "📊 Poll closed" and "⏱️ Closes
in {seconds_remaining:.0f}s" instead of using localized strings. Replace these
hardcoded strings by defining localization keys and looking them up through
LocalizationManager as required by the coding guidelines for plugins. Also apply
the same localization approach to the strings in lines 119-121 that were
similarly identified as needing localization. Ensure all user-facing strings in
the poll responses are retrieved from the LocalizationManager rather than
hardcoded directly.
Source: Coding guidelines
Comment on lines
+221
to
+223
| def _schedule_timer(self, guild_id: int, message_id: int, seconds: float) -> None: | ||
| task = asyncio.create_task(self._poll_timer(guild_id, message_id, seconds)) | ||
| self._timers.setdefault(guild_id, {})[message_id] = task |
There was a problem hiding this comment.
Cancel existing timer before replacing it for the same poll.
Line 223 overwrites an existing task reference without cancelling it. On reconnect (on_ready runs again), this can leave multiple live timers for the same message_id.
Suggested fix
def _schedule_timer(self, guild_id: int, message_id: int, seconds: float) -> None:
+ guild_timers = self._timers.setdefault(guild_id, {})
+ existing = guild_timers.get(message_id)
+ if existing is not None and not existing.done():
+ existing.cancel()
task = asyncio.create_task(self._poll_timer(guild_id, message_id, seconds))
- self._timers.setdefault(guild_id, {})[message_id] = task
+ guild_timers[message_id] = task🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@easycord/plugins/polls.py` around lines 221 - 223, In the `_schedule_timer`
method, before assigning the new task to `self._timers.setdefault(guild_id,
{})[message_id]`, check if a task already exists for that message_id in the
guild's timers dictionary. If a task exists, cancel it using its cancel() method
before overwriting the reference with the new task. This prevents orphaned
timers from running when the same poll is rescheduled (such as on reconnect when
on_ready runs again).
Comment on lines
+253
to
+256
| try: | ||
| message = await channel.fetch_message(message_id) | ||
| except discord.NotFound: | ||
| return |
There was a problem hiding this comment.
Handle all discord.HTTPException failures when fetching the poll message.
Line 255 catches only discord.NotFound; other HTTP failures (e.g., forbidden/transient API errors) can still raise and leave noisy task failures.
Suggested fix
try:
message = await channel.fetch_message(message_id)
- except discord.NotFound:
+ except discord.HTTPException:
return🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@easycord/plugins/polls.py` around lines 253 - 256, The except clause in the
channel.fetch_message(message_id) call currently catches only discord.NotFound
exception, but this misses other HTTP failures like forbidden or transient API
errors. Replace the except clause to catch the broader discord.HTTPException
instead, which will handle all HTTP-related errors and prevent noisy task
failures from unhandled exceptions.
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
|
✅ Performance benchmarks completed. All thresholds maintained. See workflow artifacts for detailed results. |
1 similar comment
|
✅ Performance benchmarks completed. All thresholds maintained. See workflow artifacts for detailed results. |
- _i18n_locale.py: add explanatory comment to bare except clause so CodeQL understands the intentional suppression of getdefaultlocale() errors on unsupported platforms - helpers/tools.py: split compound type check in register_batch so a non-ToolSafety safety value raises TypeError instead of silently skipping the tool entry (Sourcery bug-risk finding) Note: CodeQL unnecessary-lambda findings in _bot_commands.py are false positives — the lambdas provide late-binding closure semantics required by the test mock strategy (patch on easycord._bot_commands.Context and build_chain must be visible at call time, not bind-time).
|
✅ Performance benchmarks completed. All thresholds maintained. See workflow artifacts for detailed results. |
GitHub's autofix commits replaced context_factory lambdas with direct Context references in _build_slash_callback and _register_slash. This breaks TestRequireAdmin.test_require_admin_blocks_non_admin because the test patches easycord._bot_commands.Context after _build_slash_callback is called — a direct reference binds before the patch is active, a lambda resolves at call time and sees the patched value. The lambdas are not unnecessary; they are intentional closures over the module-level name so tests can mock Context via patch().
Owner
Author
CodeQL "Unnecessary lambda" — false positive explanationGitHub's autofix committed two changes replacing Why the lambdas are not unnecessary:
Removing the lambdas causes The These findings should be dismissed as false positives in the CodeQL Security tab. |
|
✅ Performance benchmarks completed. All thresholds maintained. See workflow artifacts for detailed results. |
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
3 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.
What
Splits three large internal modules into focused sub-modules, hardens
AIModeratorPluginagainst a prompt-injection audit finding, and eliminates 12 Pyright type errors from unguardedctx.guildaccess. Also expandsCLAUDE.mdwith architecture and testing guidance derived from the codebase.Why
The
_bot_commands,_bot_plugins, andi18nmodules had grown large enough that their internal concerns were hard to locate and reason about independently. The split follows the existing_context_*/_bot_*naming convention already in place.The security fix addresses M1 from the weekly audit (closed in #48):
message.contentandmessage.author.namewere interpolated verbatim into the LLM analysis prompt inAIModeratorPlugin, allowing a crafted Discord message to shift the model's interpretation of the instruction. The null-narrowing fixes resolve 12 Pyright errors that surfaced alongside that change.How
Module splits
_bot_commands.py→_command_callbacks.py(callback wrappers) +_command_registration.py(option injection, choice population, context-menu registration)_bot_plugins.py→_plugin_scanner.py(method scanner and@slash/@onwiring)i18n.py→_i18n_locale.py(normalisation, chain building, OS detection) +_i18n_diagnostics.py(DiagnosticMode, metrics) +_i18n_validation.py(translation report)Each parent module now re-exports from its sub-modules; the public API is unchanged.
Prompt injection fix (
easycord/plugins/ai_moderator.py)Before:
After:
Null-narrowing (
easycord/plugins/ai_moderator.py)_execute_action:if ctx.guild is None: return False— matches the pattern inmoderation.py/tickets.pyguild_only=Trueslash handlers:assert ctx.guild is not None— matches the pattern ineconomy.py/suggestions.pyTesting
pytest tests/ -x -q→ 1009 passed, 0 failuresai_moderator.pyreduced from 12 → 0Related PRs
actions/cachev4→v5 (merged)actions/setup-nodev4→v6 (merged)actions/setup-pythonv5→v6 (closed — v6 does not exist percontext/conventions.md)>=7→>=9.1.1(merged)Checklist
Summary by Sourcery
Refactor command and plugin registration into dedicated helper modules, harden AI moderation and polling features, and enhance localization, helpers, and tests while preserving the public API.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: