Skip to content

Commit

Permalink
cc: implement new DiagnoseCCTriggers command (#1712)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
jo3-l authored Aug 3, 2024
1 parent f44be20 commit 8eae5d6
Show file tree
Hide file tree
Showing 6 changed files with 164 additions and 50 deletions.
195 changes: 155 additions & 40 deletions customcommands/bot.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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]
}
Expand Down Expand Up @@ -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]
}
Expand Down Expand Up @@ -996,39 +1117,33 @@ 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]
}

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 {
Expand Down
4 changes: 2 additions & 2 deletions moderation/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) == "" {
Expand Down Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions reputation/plugin_bot.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion rolecommands/menu.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions tickets/tickets_commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
5 changes: 2 additions & 3 deletions web/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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() {
Expand Down

0 comments on commit 8eae5d6

Please sign in to comment.