Merged
Conversation
lorenzocorallo
force-pushed
the
moderation-refactor
branch
from
9f924d0 to
41de3a5
Compare
Contributor
WalkthroughCentralizes moderation into a new Moderation singleton with ban/kick/mute/unmute/multiChatSpam/deleteMessages/preDelete; migrates command handlers, middlewares, and tg-logger to use Moderation methods; removes per-action modules and UIActionsLogger; adds user mapping/storage helpers and PreDeleteResult semantics; bumps Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant Cmd as CommandHandler
participant Mod as Moderation
participant TgLog as TgLogger
participant API as Telegram/Backend
User->>Cmd: trigger moderation command / report (with replied message)
Cmd->>Mod: call Moderation.*(target, chat, issuer, duration?, [msg], reason)
Mod->>TgLog: preDelete([msg], reason, issuer)
TgLog-->>Mod: PreDeleteResult | null
Mod->>API: perform action(s) (delete, ban/kick/mute, multi-chat ops)
API-->>Mod: success / error
Mod->>TgLog: moderationAction(payload including preDeleteRes)
Mod-->>Cmd: Result.ok / Result.err
Cmd-->>User: reply "OK" or error (deleted after delay)
Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
lorenzocorallo
force-pushed
the
moderation-refactor
branch
from
f2db754 to
0c78ee3
Compare
Member
Author
|
reopen |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/modules/tg-logger/report.ts (1)
82-116: Check Result types from moderation actions before marking reports resolved.All three moderation methods return
Resulttypes (deleteMessagesreturnsResult<PreDeleteResult | null, "DELETE_ERROR">, whilekickandbanreturnResult<void, string>). The current code at lines 82–116 awaits these calls but ignores the returned Result, proceeding directly toeditReportMessageregardless of success or failure. If a moderation action fails, the report will still be marked as resolved, leaving the moderation action incomplete.Add error handling:
- Check the Result before calling
editReportMessage- Return feedback to the user if the action fails (e.g., "❌ Failed to delete message")
- Only mark the report resolved after confirming success
src/commands/mute.ts (1)
83-90: Handle NaN fromparseIntwhen username is not a valid numeric ID.If
args.usernamedoesn't start with@but is not a valid number (e.g.,"abc"),parseIntreturnsNaN, which is truthy when checked with!userId. However,NaNwould then be passed togetUser, potentially causing unexpected behavior.🛡️ Proposed fix to validate numeric input
- const userId = args.username.startsWith("@") ? await getTelegramId(args.username) : parseInt(args.username, 10) - if (!userId) { + const userId = args.username.startsWith("@") ? await getTelegramId(args.username) : parseInt(args.username, 10) + if (!userId || Number.isNaN(userId)) {src/commands/ban.ts (1)
83-90: Handle NaN fromparseIntwhen username is not a valid numeric ID.Same issue as in
mute.ts— ifargs.usernameis not numeric,parseIntreturnsNaN.🛡️ Proposed fix
const userId = args.username.startsWith("@") ? await getTelegramId(args.username) : parseInt(args.username, 10) - if (!userId) { + if (!userId || Number.isNaN(userId)) {
🤖 Fix all issues with AI agents
In `@src/middlewares/auto-moderation-stack/index.ts`:
- Around line 196-213: The comment incorrectly states the duration is one
minute; update the misleading comment near the Moderation.mute call that uses
duration.zod.parse("1d") to reflect it represents one day (or change the
argument to the intended duration), e.g., correct or remove the inline comment
by locating duration.zod.parse("1d") in the auto-moderation stack code (around
Moderation.mute and the subsequent ctx.reply) and ensure the comment accurately
states "1 day" (or replace "1d" with the proper value if the intent was one
minute).
- Around line 245-252: Moderation.mute is returning a Result but its error is
being ignored; update the call in the auto-moderation stack to handle the Result
from Moderation.mute (e.g., inspect the returned Result or use .ok/.err or
match) and on error log or propagate the failure (use processLogger or
ctx.logger and include the error and context like ctx.from/ctx.chat/ctx.me), and
also update or remove the stale duration comment by replacing the misleading "1
minute" note next to duration.zod.parse(NON_LATIN.MUTE_DURATION) with an
accurate description or none at all so comments reflect the actual value.
In `@src/middlewares/group-specific-actions.ts`:
- Around line 69-73: The call to ctx.deleteMessage() can throw and break the
middleware flow; wrap the await ctx.deleteMessage() in a try/catch so deletion
failures are caught and do not prevent the following user-facing actions. In the
catch block, log the error via the same logger used above
(modules.get("tgLogger")) including relevant context (e.g., ctx.message or
message id and the caught error) and do not rethrow so the middleware continues
normally. Ensure you still await the delete inside the try block and keep the
preDelete call (preDelete([...]) and ctx as they are.
In `@src/modules/moderation/index.ts`:
- Around line 194-196: The error message in multiChatSpam (function
multiChatSpam(target, messagesToDelete, duration)) uses an unprofessional string
"Sei stupido"; replace it with a clear, professional error description (e.g.,
"no messages to delete" or "messagesToDelete is empty") and ensure the returned
err() includes context about which parameter triggered it (messagesToDelete) so
logs and user-facing messages are appropriate and informative.
- Around line 89-101: The MULTI_CHAT_SPAM case is calling
modules.shared.api.restrictChatMember with p.from.id (the bot) instead of the
offender; change the call to use p.target.id so the restriction is applied to
the spammer. Update the MULTI_CHAT_SPAM branch that iterates
groupMessagesByChat(p.messages).keys() and ensure
modules.shared.api.restrictChatMember(chatId, p.target.id,
RestrictPermissions.mute, { until_date: p.duration.timestamp_s }) is used
(preserving the existing .catch(() => false) and Promise.all/.then logic).
In `@src/utils/users.ts`:
- Around line 15-18: The current logic around api.tg.users.get.query
destructures { user, error } and returns err(error) when user is falsy, which
can pass undefined errors to callers; update the return path in the function
that calls api.tg.users.get.query so that when neither user nor error is present
you return a meaningful fallback error (e.g., use err(error ?? new Error("user
not found")) or similar), keeping the ok(toGrammyUser(user)) branch unchanged;
reference symbols: api.tg.users.get.query, user, error, toGrammyUser, ok, err.
🧹 Nitpick comments (7)
src/commands/del.ts (1)
24-28: PreferPromise.allSettledso one deletion failure doesn’t abort the other.This makes
/delmore robust if one message is already gone or deletion permissions are partial.♻️ Safer deletion pattern
- await Promise.all([ - context.deleteMessages([repliedTo.message_id]), - context.deleteMessage(), // /del message - ]) + await Promise.allSettled([ + context.deleteMessages([repliedTo.message_id]), + context.deleteMessage(), // /del message + ])src/commands/mute.ts (1)
92-99: Consider passingcontexttogetUserfor potential optimization.The
getUserfunction can usectx.getChatMember()as a faster lookup before falling back to the API. Passingcontexthere could avoid an API call when the user is a member of the current chat.- const user = await getUser(userId) + const user = await getUser(userId, context)src/commands/ban.ts (2)
54-57: Minor: Logger message says "ban" but this is the "tban" handler.The error message references "ban" instead of "tban", which could cause confusion when debugging logs.
if (!repliedTo.from) { - logger.error("ban: no repliedTo.from field (the msg was sent in a channel)") + logger.error("tban: no repliedTo.from field (the msg was sent in a channel)") return }
92-99: Consider passingcontexttogetUserfor potential optimization.Same as in
mute.ts— passingcontextallowsgetUserto usegetChatMemberbefore falling back to the API.- const user = await getUser(userId) + const user = await getUser(userId, context)src/modules/moderation/index.ts (3)
35-35: Typo: "youself" should be "yourself".- if (p.target.id === p.from.id) return err(fmt(({ b }) => b`@${p.from.username} you cannot moderate youself (smh)`)) + if (p.target.id === p.from.id) return err(fmt(({ b }) => b`@${p.from.username} you cannot moderate yourself (smh)`))
114-116: Address TODO: Clean up preDelete messages on deletion failure.The TODO indicates that if message deletion fails, the preDelete log messages should also be cleaned up to avoid orphaned log entries. Consider tracking this as a follow-up task.
Would you like me to open an issue to track implementing cleanup of preDelete messages when deletion fails?
65-70: Consider usingMath.floorforuntil_dateto ensure integer seconds.
Date.now() / 1000produces a float. While Telegram's API typically handles this, usingMath.floorensures a clean integer and matches the pattern used elsewhere (e.g., line 30 in duration.ts).case "KICK": return modules.shared.api .banChatMember(p.chat.id, p.target.id, { - until_date: Date.now() / 1000 + duration.values.m, + until_date: Math.floor(Date.now() / 1000) + duration.values.m, }) .catch(() => false)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/middlewares/auto-moderation-stack/index.ts`:
- Around line 205-212: The reply currently sends raw res.error to users via
ctx.reply in the auto-moderation handler (see the ctx.reply call inside
src/middlewares/auto-moderation-stack/index.ts and the res object), so replace
the user-facing text with a generic, friendly fallback (e.g., "Message removed
for safety — if you think this is a mistake contact admins") while logging the
actual error internally (use your existing logger or processLogger.error and
include res.error and contextual info like ctx.from, chat id and message id);
ensure the fmt/formatting path still supports the fallback message and do not
expose raw res.error to users.
- Around line 159-167: The code currently passes the raw res.error into
ctx.reply (in the msg creation using fmt and fmtUser), which can leak internal
details; change the reply to show a generic, user-facing message like "An error
occurred while processing your request. Please try again later." while logging
the actual error from res.error via the existing logger (or throw to a monitored
handler). Update the branch where res.isOk() is false (the else branch that uses
res.error) so it doesn't expose res.error to users but records res.error using
the service-level logger or error reporter associated with the
middleware/Moderation.mute flow.
Contributor
There was a problem hiding this comment.
Pull request overview
This refactor introduces a centralized moderation stack (Moderation class) that handles Telegram moderation actions, logging, and message deletion in a unified way, and wires existing commands and middlewares into this new flow.
Changes:
- Add
Moderationsingleton to encapsulate validation, Telegram API calls, deletion, and audit logging for mute/ban/kick/unban/unmute and multi-chat spam actions, plus agetUserhelper that resolves users via chat or backend. - Refactor tg-logger to separate “pre-delete” logging from actual message deletion and to consume
preDeleteReslinks from the new moderation flow. - Update command handlers (
/ban,/mute,/kick,/del, unban/unmute) and automated middlewares (auto-moderation stack, group-specific actions, grants/report handlers) to use the new moderation APIs and backend version.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/utils/users.ts |
Adds getUser utility that tries ctx.getChatMember then falls back to api.tg.users.get, used by unban/unmute commands. |
src/utils/types.ts |
Imports ApiOutput and adds toGrammyUser to map backend user records to grammy User, plus re-exports Context/ModuleShared helpers. |
src/utils/duration.ts |
Exports the Duration type so it can be reused by the moderation stack. |
src/modules/tg-logger/types.ts |
Extends ModerationAction with optional preDeleteRes and renames DeleteResult to PreDeleteResult used for deleted-message links. |
src/modules/tg-logger/index.ts |
Introduces MOD_ACTION_TITLE, adds preDelete logging (without deleting messages), and updates moderationAction to use preDeleteRes for “See Deleted Message” links. |
src/modules/tg-logger/report.ts |
Refactors report menu actions to call Moderation.deleteMessages/Moderation.kick/Moderation.ban instead of hand-rolled API calls. |
src/modules/tg-logger/grants.ts |
Uses Moderation.deleteMessages for deleting granted users’ messages, mapping errors to grant-menu feedback. |
src/modules/moderation/index.ts |
New Moderation singleton centralizes target validation, audit logging, executing moderation actions, pre-delete logging, and grouped deleteMessages. |
src/middlewares/group-specific-actions.ts |
Switches from tgLogger.delete to tgLogger.preDelete plus explicit ctx.deleteMessage when group-specific rules are violated. |
src/middlewares/auto-moderation-stack/index.ts |
Rewires link, harmful-content, non-latin, and multi-chat-spam handlers to Moderation.mute/Moderation.multiChatSpam, and tailors user-facing replies based on Result success/failure. |
src/commands/mute.ts |
Replaces direct mute/unmute helpers with Moderation.mute/Moderation.unmute, uses getUser for unmute, and normalizes feedback to “OK” or error text. |
src/commands/kick.ts |
Replaces kick helper with Moderation.kick and standardizes ephemeral success/error reply behavior. |
src/commands/del.ts |
Uses tgLogger.preDelete for logging then deletes both the target and /del command messages via deleteMessages/deleteMessage. |
src/commands/ban.ts |
Replaces ban/unban helpers with Moderation.ban/Moderation.unban, uses getUser for unban, and standardizes success/error responses. |
src/modules/moderation/ban.ts |
Removed in favor of centralized moderation logic in src/modules/moderation/index.ts. |
src/modules/moderation/mute.ts |
Removed in favor of centralized moderation logic in src/modules/moderation/index.ts. |
src/modules/moderation/kick.ts |
Removed in favor of centralized moderation logic in src/modules/moderation/index.ts. |
src/modules/tg-logger/report.ts |
(same file as above) Updated menu callbacks to hook into new moderation stack for delete/kick/ban actions. |
src/modules/tg-logger/grants.ts |
(same file as above) Updated delete flow to depend on Moderation.deleteMessages and propagate failures as menu errors. |
package.json |
Bumps @polinetwork/backend dependency to ^0.15.3 to support new backend API usage. |
pnpm-lock.yaml |
Locks @polinetwork/backend to 0.15.3 and updates its integrity hash to match the new version. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/modules/moderation/index.ts`:
- Around line 110-116: After a failed delete you must undo the earlier preDelete
work to avoid orphaned telemetry; call a rollback/compensating method on the
same logger with the preRes before returning the error. Specifically, after
modules.shared.api.deleteMessages returns falsy, invoke
modules.get("tgLogger").rollbackPreDelete(preRes) (guarding for preRes
truthiness) and catch/log any rollback errors (but do not throw) before
returning err("DELETE_ERROR"); ensure the rollback call is placed inside the
for-loop where deleteMessages failed so each affected chat's preDelete is
compensated.
- Around line 123-137: In moderate (private async moderate) we currently ignore
the result of deleteMessages (preDeleteRes) and proceed to perform the action;
check if preDeleteRes is an Err and immediately return that Err before calling
perform so deletion failures propagate; specifically, after calling
deleteMessages (the variable preDeleteRes) add a guard that if preDeleteRes !==
null and preDeleteRes.isErr() then return preDeleteRes; keep the existing
perform(p) flow otherwise.
- Around line 90-100: The code in MULTI_CHAT_SPAM uses
groupMessagesByChat(p.messages).keys().map(...), but Map.prototype.keys()
returns an iterator (no map), causing a runtime error; fix by converting the
keys iterator to an array (e.g.,
Array.from(groupMessagesByChat(p.messages).keys()) or
[...groupMessagesByChat(p.messages).keys()]) before calling .map, so the
subsequent calls to modules.shared.api.restrictChatMember(...) (and its .catch)
work as intended and the Promise.all result remains unchanged.
In the rare case when we need to get an user by userId, the context cannot retrieve it and the user is yet to be synced to the backend, we can retrieve it from the temporary map.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/utils/users.ts`:
- Around line 5-11: The getUser function uses optional chaining incorrectly
causing a TypeError when ctx is undefined; modify getUser so it first checks
whether ctx is provided and only then calls ctx.getChatMember(userId) (awaiting
and handling .then/.catch), otherwise skip the call and fall back to
MessageUserStorage.getInstance().getStoredUser(userId); refer to the getUser
function, Context type and MessageUserStorage.getInstance().getStoredUser to
locate and update the logic.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/modules/moderation/index.ts`:
- Around line 62-66: In the PERFORM_ERROR case update the strError value to fix
the typo: change "There was an error perfoming the moderation action" to "There
was an error performing the moderation action" (the identifiers to edit are the
case "PERFORM_ERROR" object and its strError property; no other behavior changes
needed).
- Around line 154-160: The deleteMessages call in the error branch can reject
and produce an unhandled promise; update the fire-and-forget invocation of
modules.shared.api.deleteMessages(tgLogger.groupId, preRes.logMessageIds) to
explicitly handle rejections (e.g., attach a .catch(...) that swallows/logs the
error) so the rejection is consumed; keep the existing logger.error and return
err("DELETE_ERROR") behavior unchanged but ensure the deleteMessages promise
cannot become unhandled.
🧹 Nitpick comments (2)
src/modules/moderation/index.ts (1)
27-27: Address the TODO for in-channel moderation feedback.If this is still planned, consider tracking it with an issue so it doesn’t get lost during the refactor.
If you want, I can draft the user-facing feedback flow and open an issue.
src/commands/del.ts (1)
26-30: Guard deletion of the temporary feedback message.If
msg.delete()fails (permissions/auto‑delete), it will throw after the response is sent. Consider swallowing the error.♻️ Suggested tweak
- await msg.delete() + await msg.delete().catch(() => {})
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/modules/moderation/index.ts`:
- Around line 39-67: The user-facing error message in getModerationError for the
"PERFORM_ERROR" case has a typo ("perfoming"); update the strError value in the
PERFORM_ERROR return object inside getModerationError to "There was an error
performing the moderation action" (referencing the getModerationError function
and the "PERFORM_ERROR" case to locate the code).
- Around line 71-77: In checkTargetValid, treat failures from
modules.shared.api.getChatMember as a closed/failing case instead of returning
null: replace the current .catch(() => null) by catching errors and returning an
appropriate Err (e.g., err("CANNOT_MOD_GROUPADMIN") or a dedicated API failure
code) so that a transient API error prevents the moderation action;
specifically, in the checkTargetValid function (handling ModerationAction p)
ensure that if getChatMember throws or yields no valid member you immediately
return an Err rather than skipping the admin/creator check.
- Around line 250-269: Change multiChatSpam to accept a non-empty messages tuple
instead of a plain array by updating the parameter type to [Message,
...Message[]] (so callers cannot pass an empty array), remove the runtime throw
that checks messagesToDelete.length, and keep the body calling
this.moderate(...) as-is; mirror the established pattern used by
deleteMessages() to preserve the Promise<Result<void, ModerationError>> contract
and avoid throwing.
🧹 Nitpick comments (1)
src/modules/moderation/index.ts (1)
27-27: Track the TODO for in-channel feedback.
If this is still desired, consider opening an issue so it doesn’t get lost.
Contributor
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/modules/moderation/index.ts`:
- Around line 163-174: The log payload uses a misspelled key "forwaredCount";
update the logger.warn call to rename that property to "forwardedCount" so the
emitted log includes the correct field name (leave other fields like
initialMessages, executor, deletedCount, deletedPercentage and the condition
using preRes.count and delCount unchanged), i.e., change the object literal
passed to logger.warn to use forwardedCount: preRes.count instead of
forwaredCount.
🧹 Nitpick comments (2)
src/modules/moderation/index.ts (2)
81-92: Consider adding error handling for audit failures.The
api.tg.auditLog.create.mutate()call has no error handling. If the audit API fails, the moderation action completes successfully but the audit trail is lost without any indication. Consider adding.catch()to at least log audit failures.🔧 Suggested improvement
private async audit(p: ModerationAction) { if (p.action === "SILENT" || p.action === "MULTI_CHAT_SPAM") return - await api.tg.auditLog.create.mutate({ + await api.tg.auditLog.create.mutate({ adminId: p.from.id, groupId: p.chat.id, targetId: p.target.id, type: MAP_ACTIONS[p.action], until: "duration" in p && p.duration ? p.duration.date : null, reason: "reason" in p ? p.reason : undefined, - }) + }).catch((e) => logger.error({ error: e, action: p }, "[Moderation:audit] failed to create audit log")) }
148-152: Variableokshadows importedokfrom neverthrow.The boolean variable
okon line 150 shadows theokfunction imported fromneverthrowon line 2. While not a bug (the import is still accessible before this scope), it reduces code clarity.♻️ Rename for clarity
let delCount = 0 for (const [chatId, mIds] of groupMessagesByChat(messages)) { - const ok = await modules.shared.api.deleteMessages(chatId, mIds).catch(() => false) - if (ok) delCount += mIds.length + const deleted = await modules.shared.api.deleteMessages(chatId, mIds).catch(() => false) + if (deleted) delCount += mIds.length }
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/middlewares/group-specific-actions.ts`:
- Around line 69-77: The deleteMessages result is logged but not acted on, which
lets the middleware continue and send the user-facing reply; change
GroupSpecificActions so that after calling Moderation.deleteMessages if
res.isErr() you abort the flow (e.g., throw res.error or rethrow a new Error)
instead of only logging—this preserves the fail-fast atomic moderation behavior
and prevents subsequent calls like ctx.reply from running when deletion fails.
toto04
approved these changes
Jan 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is the main idea behind this refactor (click to enlarge)