Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 15 additions & 18 deletions astrbot/core/star/filter/platform_adapter_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
class PlatformAdapterType(enum.Flag):
AIOCQHTTP = enum.auto()
QQOFFICIAL = enum.auto()
QQOFFICIAL_WEBHOOK = enum.auto()
TELEGRAM = enum.auto()
WECOM = enum.auto()
WECOM_AI_BOT = enum.auto()
Expand All @@ -23,29 +24,16 @@ class PlatformAdapterType(enum.Flag):
MISSKEY = enum.auto()
LINE = enum.auto()
MATRIX = enum.auto()
ALL = (
AIOCQHTTP
| QQOFFICIAL
| TELEGRAM
| WECOM
| WECOM_AI_BOT
| LARK
| DINGTALK
| DISCORD
| SLACK
| KOOK
| VOCECHAT
| WEIXIN_OFFICIAL_ACCOUNT
| SATORI
| MISSKEY
| LINE
| MATRIX
)
WEIXIN_OC = enum.auto()
MATTERMOST = enum.auto()
WEBCHAT = enum.auto()
ALL = enum.auto()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

In an enum.Flag, using enum.auto() for ALL makes it a single distinct bit rather than a bitmask of all other members. This breaks the expected behavior of a "match all" flag when performing bitwise operations (e.g., ALL ^ TELEGRAM would still match TELEGRAM because the ALL bit is checked separately in the filter method). It is better to define ALL as a bitmask OR of all other platform members.

    ALL = (
        AIOCQHTTP
        | QQOFFICIAL
        | QQOFFICIAL_WEBHOOK
        | TELEGRAM
        | WECOM
        | WECOM_AI_BOT
        | LARK
        | DINGTALK
        | DISCORD
        | SLACK
        | KOOK
        | VOCECHAT
        | WEIXIN_OFFICIAL_ACCOUNT
        | SATORI
        | MISSKEY
        | LINE
        | MATRIX
        | WEIXIN_OC
        | MATTERMOST
        | WEBCHAT
    )



ADAPTER_NAME_2_TYPE = {
"aiocqhttp": PlatformAdapterType.AIOCQHTTP,
"qq_official": PlatformAdapterType.QQOFFICIAL,
"qq_official_webhook": PlatformAdapterType.QQOFFICIAL_WEBHOOK,
"telegram": PlatformAdapterType.TELEGRAM,
"wecom": PlatformAdapterType.WECOM,
"wecom_ai_bot": PlatformAdapterType.WECOM_AI_BOT,
Expand All @@ -60,6 +48,9 @@ class PlatformAdapterType(enum.Flag):
"misskey": PlatformAdapterType.MISSKEY,
"line": PlatformAdapterType.LINE,
"matrix": PlatformAdapterType.MATRIX,
"weixin_oc": PlatformAdapterType.WEIXIN_OC,
"mattermost": PlatformAdapterType.MATTERMOST,
"webchat": PlatformAdapterType.WEBCHAT,
}
Comment on lines +53 to 54
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The string "all" is missing from the ADAPTER_NAME_2_TYPE mapping. Adding it allows users to specify all platforms in configurations that rely on string-to-type conversion.

Suggested change
"webchat": PlatformAdapterType.WEBCHAT,
}
"webchat": PlatformAdapterType.WEBCHAT,
"all": PlatformAdapterType.ALL,
}



Expand All @@ -71,6 +62,12 @@ def __init__(self, platform_adapter_type_or_str: PlatformAdapterType | str) -> N
self.platform_type = platform_adapter_type_or_str

def filter(self, event: AstrMessageEvent, cfg: AstrBotConfig) -> bool:
if (
self.platform_type is not None
and self.platform_type & PlatformAdapterType.ALL
):
return True
Comment on lines +65 to +69
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): The ALL-flag early return changes behavior for composite filters and may bypass future platform-specific logic.

This early return means any filter whose mask includes PlatformAdapterType.ALL will always accept the event and skip the rest of the method, making ALL | QQOFFICIAL equivalent to ALL and bypassing any future per-platform checks. If ALL is meant as “no restriction”, a distinct sentinel (not a bit) might be clearer; if it should remain a true bitmask of all platforms, you may want to avoid short‑circuiting here so that callers can still express combinations like "all except X" in the future.

Suggested implementation:

    def filter(self, event: AstrMessageEvent, cfg: AstrBotConfig) -> bool:
        # If the filter is explicitly "ALL", accept any platform without restriction.
        # For composite masks (e.g., ALL | QQOFFICIAL), we still run the per-platform logic below.
        if self.platform_type == PlatformAdapterType.ALL:
            return True

        adapter_name = event.get_platform_name()
        if adapter_name in ADAPTER_NAME_2_TYPE and self.platform_type is not None:
            return bool(ADAPTER_NAME_2_TYPE[adapter_name] & self.platform_type)

If PlatformAdapterType.ALL is not currently defined as a standalone value but rather as a bitmask of all platforms (e.g., QQOFFICIAL | DISCORD | ...), you may want to:

  1. Consider introducing a distinct sentinel for “no restriction” (e.g., PlatformAdapterType.UNRESTRICTED) and use that in this equality check instead of ALL.
  2. Audit call sites setting platform_type to ensure they use the intended “no restriction” sentinel (or ALL) consistently.


Comment on lines +65 to +70
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This special check is redundant if PlatformAdapterType.ALL is correctly defined as a bitmask. The existing logic below using ADAPTER_NAME_2_TYPE[adapter_name] & self.platform_type will naturally return True when self.platform_type is ALL, as ALL contains all valid platform bits.

adapter_name = event.get_platform_name()
if adapter_name in ADAPTER_NAME_2_TYPE and self.platform_type is not None:
return bool(ADAPTER_NAME_2_TYPE[adapter_name] & self.platform_type)
Expand Down
Loading