From 8eae5d66fc7f4a32283a15275948f73443169c00 Mon Sep 17 00:00:00 2001 From: "Joe L." <56809242+jo3-l@users.noreply.github.com> Date: Sat, 3 Aug 2024 05:26:42 -0700 Subject: [PATCH] cc: implement new DiagnoseCCTriggers command (#1712) * web: make ManageServerURL take guild ID directly This change unlocks using this function in a later commit (where we only have a guild ID, not the full guild data.) It's also more proper: ManageServerURL only needs the guild ID, so it need not be tied to dcmd. * cc: factor out CCActionExecLimit function The action of computing this limit is repeated throughout this file, so factor it out. Since we are using the limit for more than just messages, rename it appropriately. * cc: extract hasHigherPriority comparator Currently the comparator is internal to sortTriggeredCCs (which accepts a slice of *TriggeredCC), but we will need to sort a slice of custom commands directly to implement the debug command. Extract the comparator out, document the priority order, and then refactor the implementation for clarity. To prove that the old and new comparator implementations are functionally equivalent, see the following test: https://go.dev/play/p/NEUrG1wu0OI. It exhaustively enumerates all pairs of custom commands and ensures the old and new comparators agree. * cc: implement new DiagnoseCCTriggers command --- customcommands/bot.go | 195 ++++++++++++++++++++++++++++-------- moderation/commands.go | 4 +- reputation/plugin_bot.go | 4 +- rolecommands/menu.go | 2 +- tickets/tickets_commands.go | 4 +- web/web.go | 5 +- 6 files changed, 164 insertions(+), 50 deletions(-) diff --git a/customcommands/bot.go b/customcommands/bot.go index 612deafa48..599e78da1f 100644 --- a/customcommands/bot.go +++ b/customcommands/bot.go @@ -28,6 +28,7 @@ import ( "github.com/botlabs-gg/yagpdb/v2/common/featureflags" "github.com/botlabs-gg/yagpdb/v2/common/keylock" "github.com/botlabs-gg/yagpdb/v2/common/multiratelimit" + prfx "github.com/botlabs-gg/yagpdb/v2/common/prefix" "github.com/botlabs-gg/yagpdb/v2/common/pubsub" "github.com/botlabs-gg/yagpdb/v2/common/scheduledevents2" schEventsModels "github.com/botlabs-gg/yagpdb/v2/common/scheduledevents2/models" @@ -63,7 +64,7 @@ var _ bot.BotInitHandler = (*Plugin)(nil) var _ commands.CommandProvider = (*Plugin)(nil) func (p *Plugin) AddCommands() { - commands.AddRootCommands(p, cmdListCommands, cmdFixCommands, cmdEvalCommand) + commands.AddRootCommands(p, cmdListCommands, cmdFixCommands, cmdEvalCommand, cmdDiagnoseCCTriggers) } func (p *Plugin) BotInit() { @@ -186,6 +187,116 @@ var cmdEvalCommand = &commands.YAGCommand{ }, } +type cmdDiagnosisResult int + +const ( + cmdOK cmdDiagnosisResult = iota + cmdExceedsTriggerLimits + cmdDisabled + cmdUnmetRestrictions +) + +type triggeredCmdDiagnosis struct { + CC *models.CustomCommand + Result cmdDiagnosisResult +} + +func (diag triggeredCmdDiagnosis) WriteTo(out *strings.Builder) { + switch diag.Result { + case cmdOK: + out.WriteString(":white_check_mark: ") + case cmdExceedsTriggerLimits: + out.WriteString(":warning: ") + } + + fmt.Fprintf(out, "[**CC #%d**](%s): %s `%s`\n", diag.CC.LocalID, cmdControlPanelLink(diag.CC), + CommandTriggerType(diag.CC.TriggerType), diag.CC.TextTrigger) + switch diag.Result { + case cmdOK: + out.WriteString("- will execute") + case cmdExceedsTriggerLimits: + out.WriteString("- would otherwise execute, but **exceeds limit on commands executed per message**") + case cmdDisabled: + out.WriteString("- triggers on input, but **is disabled**") + case cmdUnmetRestrictions: + out.WriteString("- triggers on input, but **has unmet role/channel restrictions**") + } +} + +var cmdDiagnoseCCTriggers = &commands.YAGCommand{ + CmdCategory: commands.CategoryDebug, + Name: "DiagnoseCCTriggers", + Aliases: []string{"diagnosetriggers", "debugtriggers"}, + Description: "List all custom commands that would trigger on the input and identify potential issues", + Arguments: []*dcmd.ArgDef{ + {Name: "input", Type: dcmd.String}, + }, + RequireDiscordPerms: []int64{discordgo.PermissionManageGuild}, + SlashCommandEnabled: false, + DefaultEnabled: true, + RunFunc: func(data *dcmd.Data) (interface{}, error) { + cmds, err := BotCachedGetCommandsWithMessageTriggers(data.GuildData.GS.ID, data.Context()) + if err != nil { + return "Failed fetching custom commands", err + } + + prefix, err := prfx.GetCommandPrefixRedis(data.GuildData.GS.ID) + if err != nil { + return "Failed fetching command prefix", err + } + + // Use the raw input, not dcmd's interpretation of it (which may drop characters.) + input := data.TraditionalTriggerData.MessageStrippedPrefix + + var triggered []*models.CustomCommand + for _, cmd := range cmds { + if matches, _, _ := CheckMatch(prefix, cmd, input); matches { + triggered = append(triggered, cmd) + } + } + if len(triggered) == 0 { + return "No commands trigger on the input", nil + } + + sort.Slice(triggered, func(i, j int) bool { return hasHigherPriority(triggered[i], triggered[j]) }) + + limit := CCActionExecLimit(data.GuildData.GS.ID) + executed, skipped := 0, 0 + + diagnoses := make([]triggeredCmdDiagnosis, 0, len(triggered)) + for _, cmd := range triggered { + switch { + case cmd.Disabled || (cmd.R.Group != nil && cmd.R.Group.Disabled): + diagnoses = append(diagnoses, triggeredCmdDiagnosis{cmd, cmdDisabled}) + case !CmdRunsForUser(cmd, data.GuildData.MS): + diagnoses = append(diagnoses, triggeredCmdDiagnosis{cmd, cmdUnmetRestrictions}) + case !CmdRunsInChannel(cmd, common.ChannelOrThreadParentID(data.GuildData.CS)): + diagnoses = append(diagnoses, triggeredCmdDiagnosis{cmd, cmdUnmetRestrictions}) + case executed >= limit: + skipped++ + diagnoses = append(diagnoses, triggeredCmdDiagnosis{cmd, cmdExceedsTriggerLimits}) + default: + executed++ + diagnoses = append(diagnoses, triggeredCmdDiagnosis{cmd, cmdOK}) + } + } + + var out strings.Builder + if skipped > 0 { + fmt.Fprintf(&out, `> ### Potential issue detected +> Not all custom commands triggering on the input will actually be executed. +> Note that at most %d custom commands can be executed by a single message.`, limit) + out.WriteByte('\n') + } + out.WriteString("## Commands triggering on input\n") + for _, diagnosis := range diagnoses { + diagnosis.WriteTo(&out) + out.WriteByte('\n') + } + return out.String(), nil + }, +} + var cmdListCommands = &commands.YAGCommand{ CmdCategory: commands.CategoryTool, Name: "CustomCommands", @@ -285,7 +396,7 @@ var cmdListCommands = &commands.YAGCommand{ } if hasRead, _ := web.GetUserAccessLevel(data.Author.ID, gWithConnected, common.GetCoreServerConfCached(data.GuildData.GS.ID), web.StaticRoleProvider(roles)); hasRead { - ccIDMaybeWithLink = fmt.Sprintf("[%[1]d](%[2]s/customcommands/commands/%[1]d/)", cc.LocalID, web.ManageServerURL(data.GuildData)) + ccIDMaybeWithLink = fmt.Sprintf("[%d](%s)", cc.LocalID, cmdControlPanelLink(cc)) } // Every message content-based custom command trigger has a numerical value less than 5 @@ -340,6 +451,10 @@ var cmdListCommands = &commands.YAGCommand{ }, } +func cmdControlPanelLink(cmd *models.CustomCommand) string { + return fmt.Sprintf("%s/customcommands/commands/%d/", web.ManageServerURL(cmd.GuildID), cmd.LocalID) +} + func FindCommands(ccs []*models.CustomCommand, data *dcmd.Data) (foundCCS []*models.CustomCommand, provided bool) { foundCCS = make([]*models.CustomCommand, 0, len(ccs)) @@ -549,11 +664,21 @@ func shouldIgnoreChannel(msg *discordgo.Message, gs *dstate.GuildSet, cState *ds return false } +// Limit the number of custom commands that can be executed on a single action (messages, reactions, +// component uses, and so on). + const ( - CCMessageExecLimitNormal = 3 - CCMessageExecLimitPremium = 5 + CCActionExecLimitNormal = 3 + CCActionExecLimitPremium = 5 ) +func CCActionExecLimit(guildID int64) int { + if isPremium, _ := premium.IsGuildPremium(guildID); isPremium { + return CCActionExecLimitPremium + } + return CCActionExecLimitNormal +} + func (p *Plugin) OnRemovedPremiumGuild(GuildID int64) error { commands, err := models.CustomCommands(qm.Where("guild_id = ?", GuildID), qm.Offset(MaxCommands)).AllG(context.Background()) if err != nil { @@ -867,13 +992,11 @@ func findMessageTriggerCustomCommands(ctx context.Context, cs *dstate.ChannelSta } } - sortTriggeredCCs(matched) - - limit := CCMessageExecLimitNormal - if isPremium, _ := premium.IsGuildPremium(msg.GuildID); isPremium { - limit = CCMessageExecLimitPremium - } + sort.Slice(matched, func(i, j int) bool { + return hasHigherPriority(matched[i].CC, matched[j].CC) + }) + limit := CCActionExecLimit(msg.GuildID) if len(matched) > limit { matched = matched[:limit] } @@ -925,13 +1048,11 @@ func findReactionTriggerCustomCommands(ctx context.Context, cs *dstate.ChannelSt filtered = append(filtered, v) } - sortTriggeredCCs(filtered) - - limit := CCMessageExecLimitNormal - if isPremium, _ := premium.IsGuildPremium(cs.GuildID); isPremium { - limit = CCMessageExecLimitPremium - } + sort.Slice(filtered, func(i, j int) bool { + return hasHigherPriority(filtered[i].CC, filtered[j].CC) + }) + limit := CCActionExecLimit(cs.GuildID) if len(filtered) > limit { filtered = filtered[:limit] } @@ -996,13 +1117,11 @@ func findComponentOrModalTriggerCustomCommands(ctx context.Context, cs *dstate.C filtered = append(filtered, v) } - sortTriggeredCCs(filtered) - - limit := CCMessageExecLimitNormal - if isPremium, _ := premium.IsGuildPremium(cs.GuildID); isPremium { - limit = CCMessageExecLimitPremium - } + sort.Slice(filtered, func(i, j int) bool { + return hasHigherPriority(filtered[i].CC, filtered[j].CC) + }) + limit := CCActionExecLimit(cs.GuildID) if len(filtered) > limit { filtered = filtered[:limit] } @@ -1010,25 +1129,21 @@ func findComponentOrModalTriggerCustomCommands(ctx context.Context, cs *dstate.C return filtered, nil } -func sortTriggeredCCs(ccs []*TriggeredCC) { - sort.Slice(ccs, func(i, j int) bool { - a := ccs[i] - b := ccs[j] - - if a.CC.TriggerType == b.CC.TriggerType { - return a.CC.LocalID < b.CC.LocalID - } - - if a.CC.TriggerType == int(CommandTriggerRegex) { - return false - } - - if b.CC.TriggerType == int(CommandTriggerRegex) { - return true - } +// hasHigherPriority reports whether custom command a should be executed in preference to custom +// command b. Regex custom commands always have lowest priority, with ties broken by ID (smaller ID +// has priority.) +func hasHigherPriority(a *models.CustomCommand, b *models.CustomCommand) bool { + aIsRegex := a.TriggerType == int(CommandTriggerRegex) + bIsRegex := b.TriggerType == int(CommandTriggerRegex) - return a.CC.LocalID < b.CC.LocalID - }) + switch { + case !aIsRegex && bIsRegex: + return true + case aIsRegex && !bIsRegex: + return false + default: + return a.LocalID < b.LocalID + } } func ExecuteCustomCommandFromMessage(gs *dstate.GuildSet, cmd *models.CustomCommand, member *dstate.MemberState, cs *dstate.ChannelState, cmdArgs []string, stripped string, m *discordgo.Message, isEdit bool) error { diff --git a/moderation/commands.go b/moderation/commands.go index efaff1513f..384afffb44 100644 --- a/moderation/commands.go +++ b/moderation/commands.go @@ -59,7 +59,7 @@ func MBaseCmdSecond(cmdData *dcmd.Data, reason string, reasonArgOptional bool, n cmdName := cmdData.Cmd.Trigger.Names[0] oreason = reason if !enabled { - return oreason, commands.NewUserErrorf("The **%s** command is disabled on this server. It can be enabled at <%s/moderation>", cmdName, web.ManageServerURL(cmdData.GuildData)) + return oreason, commands.NewUserErrorf("The **%s** command is disabled on this server. It can be enabled at <%s/moderation>", cmdName, web.ManageServerURL(cmdData.GuildData.GS.ID)) } if strings.TrimSpace(reason) == "" { @@ -341,7 +341,7 @@ var ModerationCommands = []*commands.YAGCommand{ } if config.MuteRole == 0 { - return fmt.Sprintf("No mute role selected. Select one at <%s/moderation>", web.ManageServerURL(parsed.GuildData)), nil + return fmt.Sprintf("No mute role selected. Select one at <%s/moderation>", web.ManageServerURL(parsed.GuildData.GS.ID)), nil } reason := parsed.Args[2].Str() diff --git a/reputation/plugin_bot.go b/reputation/plugin_bot.go index ad2903e827..e9c4318ebb 100644 --- a/reputation/plugin_bot.go +++ b/reputation/plugin_bot.go @@ -35,8 +35,8 @@ func (p *Plugin) BotInit() { var thanksRegex = regexp.MustCompile(`(?i)( |\n|^)(thanks?\pP*|danks|ty|thx|\+rep|\+ ?\<\@[0-9]*\>)( |\n|$)`) -func createRepDisabledError(guild *dcmd.GuildContextData) string { - return fmt.Sprintf("**The reputation system is disabled for this server.** Enable it at: <%s/reputation>.", web.ManageServerURL(guild)) +func createRepDisabledError(g *dcmd.GuildContextData) string { + return fmt.Sprintf("**The reputation system is disabled for this server.** Enable it at: <%s/reputation>.", web.ManageServerURL(g.GS.ID)) } func handleMessageCreate(evt *eventsystem.EventData) { diff --git a/rolecommands/menu.go b/rolecommands/menu.go index 176a748a7f..1d4292ddc2 100644 --- a/rolecommands/menu.go +++ b/rolecommands/menu.go @@ -58,7 +58,7 @@ func roleGroupAutocomplete(parsed *dcmd.Data, arg *dcmd.ParsedArg) ([]*discordgo func cmdFuncRoleMenuCreate(parsed *dcmd.Data) (interface{}, error) { name := parsed.Args[0].Str() - panelURL := fmt.Sprintf("%s/rolecommands/", web.ManageServerURL(parsed.GuildData)) + panelURL := fmt.Sprintf("%s/rolecommands/", web.ManageServerURL(parsed.GuildData.GS.ID)) group, err := models.RoleGroups(qm.Where("guild_id=?", parsed.GuildData.GS.ID), qm.Where("name ILIKE ?", name), qm.Load("RoleCommands")).OneG(parsed.Context()) if err != nil { if errors.Cause(err) == sql.ErrNoRows { diff --git a/tickets/tickets_commands.go b/tickets/tickets_commands.go index 06cbb2c5b5..b12e83eaa2 100644 --- a/tickets/tickets_commands.go +++ b/tickets/tickets_commands.go @@ -28,8 +28,8 @@ const InTicketPerms = discordgo.PermissionReadMessageHistory | discordgo.Permiss var _ commands.CommandProvider = (*Plugin)(nil) -func createTicketsDisabledError(guild *dcmd.GuildContextData) string { - return fmt.Sprintf("**The tickets system is disabled for this server.** Enable it at: <%s/tickets/settings>.", web.ManageServerURL(guild)) +func createTicketsDisabledError(g *dcmd.GuildContextData) string { + return fmt.Sprintf("**The tickets system is disabled for this server.** Enable it at: <%s/tickets/settings>.", web.ManageServerURL(g.GS.ID)) } func (p *Plugin) AddCommands() { diff --git a/web/web.go b/web/web.go index 9fbf25aebf..72aae52a4b 100644 --- a/web/web.go +++ b/web/web.go @@ -17,7 +17,6 @@ import ( "github.com/botlabs-gg/yagpdb/v2/common/patreon" yagtmpl "github.com/botlabs-gg/yagpdb/v2/common/templates" "github.com/botlabs-gg/yagpdb/v2/frontend" - "github.com/botlabs-gg/yagpdb/v2/lib/dcmd" "github.com/botlabs-gg/yagpdb/v2/lib/discordgo" "github.com/botlabs-gg/yagpdb/v2/web/discordblog" "github.com/natefinch/lumberjack" @@ -134,8 +133,8 @@ func BaseURL() string { return "http://" + common.ConfHost.GetString() } -func ManageServerURL(guild *dcmd.GuildContextData) string { - return fmt.Sprintf("%s/manage/%d", BaseURL(), guild.GS.ID) +func ManageServerURL(guildID int64) string { + return fmt.Sprintf("%s/manage/%d", BaseURL(), guildID) } func Run() {