From 53e569ca12190bbba552c21eede6b4e14e8865d9 Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Tue, 15 Sep 2026 13:58:45 +0700 Subject: [PATCH] feat(blacklist): add /blacklist shorthand and /whitelist_rnd /blacklist is the short form of the two read commands: bare it lists both lists like /blacklist_rules, and given text it judges that text like /blacklist_check. Both long names stay for when the intent should be explicit. An argument of only whitespace is not an argument, so it lists. /whitelist_rnd returns one whitelist entry chosen at random, and says the list is empty rather than answering with nothing. It reads only the whitelist, and only for the calling topic. The six existing command descriptions are shortened alongside. /help renders as one un-chunked message pinned at 4096 runes, and two more commands pushed it to 4123; the shorter wording brings it to 4049. That ceiling is a shared limit this module did not create, and it will bind again on the next command added anywhere in the repo. --- cmd/server/command_menu_test.go | 2 + cmd/server/main_test.go | 8 +- docs/blacklist.md | 9 ++ internal/modules/blacklist/blacklist.go | 27 ++++-- internal/modules/blacklist/handlers.go | 69 +++++++++++++++- internal/modules/blacklist/handlers_test.go | 92 +++++++++++++++++++++ 6 files changed, 194 insertions(+), 13 deletions(-) diff --git a/cmd/server/command_menu_test.go b/cmd/server/command_menu_test.go index c1189e2..8c4f816 100644 --- a/cmd/server/command_menu_test.go +++ b/cmd/server/command_menu_test.go @@ -75,12 +75,14 @@ func TestCommandDiscovery_AllPublicCommandsHaveSafeMetadata(t *testing.T) { "addsticker": "[emoji...]", "alias": "", "aliases": "", + "blacklist": "[text...]", "blacklist_add": "[text...]", "blacklist_del": "", "blacklist_rules": "", "blacklist_check": "", "whitelist_add": "[text...]", "whitelist_del": "", + "whitelist_rnd": "", "unalias": "", "insert": "", "stats": "[users | user | cmd ]", diff --git a/cmd/server/main_test.go b/cmd/server/main_test.go index 59a7d37..3b40077 100644 --- a/cmd/server/main_test.go +++ b/cmd/server/main_test.go @@ -139,15 +139,15 @@ func TestFactoriesRegistersBlacklistCommands(t *testing.T) { t.Fatalf("Build blacklist: %v", err) } for _, name := range []string{ - "blacklist_add", "blacklist_del", "blacklist_rules", "blacklist_check", - "whitelist_add", "whitelist_del", + "blacklist", "blacklist_add", "blacklist_del", "blacklist_rules", + "blacklist_check", "whitelist_add", "whitelist_del", "whitelist_rnd", } { if _, ok := reg.AllCommands[name]; !ok { t.Fatalf("missing command %s", name) } } - if got := len(reg.AllCommands); got != 6 { - t.Fatalf("blacklist registered %d commands, want 6", got) + if got := len(reg.AllCommands); got != 8 { + t.Fatalf("blacklist registered %d commands, want 8", got) } } diff --git a/docs/blacklist.md b/docs/blacklist.md index 4f21c2e..568a1f9 100644 --- a/docs/blacklist.md +++ b/docs/blacklist.md @@ -5,15 +5,24 @@ exceptions, then ask whether a given text is blocked. | Command | Parameters | What it does | |---|---|---| +| `/blacklist` | `[text...]` | Bare, lists both lists; with text, judges it | | `/blacklist_add` | `[text...]` | Adds text to the blacklist, or the message you replied to | | `/blacklist_del` | `` | Removes text from the blacklist | | `/whitelist_add` | `[text...]` | Adds an exception, or the message you replied to | | `/whitelist_del` | `` | Removes an exception | | `/blacklist_rules` | — | Lists both lists in one message | | `/blacklist_check` | `` | Judges a text against both lists | +| `/whitelist_rnd` | — | Returns one whitelist entry at random | All are public and single-shot. +`/blacklist` is the short form of the two read commands: on its own it does what +`/blacklist_rules` does, and given text it does what `/blacklist_check` does. Nothing is +lost by using it — the long names remain for when the intent should be explicit. + +`/whitelist_rnd` returns a single whitelist entry chosen at random, and says so plainly when +the whitelist is empty rather than answering with nothing. + ## The bot does not police the chat This is the first thing to know, because the module's name promises something it diff --git a/internal/modules/blacklist/blacklist.go b/internal/modules/blacklist/blacklist.go index d20cad7..5e4c1b9 100644 --- a/internal/modules/blacklist/blacklist.go +++ b/internal/modules/blacklist/blacklist.go @@ -45,47 +45,62 @@ func New(deps modules.Deps) modules.Module { s := &state{store: storage.Typed[Entry](deps.Store)} return modules.Module{ Commands: []modules.Command{ + { + // The whole module behind one short name: bare it lists, with + // an argument it checks. + Name: "blacklist", + Visibility: modules.VisibilityPublic, + Description: "Check a text, or list both", + Parameters: "[text...]", + Handler: s.handleShort, + }, { Name: "blacklist_add", Visibility: modules.VisibilityPublic, - Description: "Blacklist a text, or a message you reply to", + Description: "Blacklist a text or a reply", Parameters: "[text...]", Handler: s.handleAdd(listBlack), }, { Name: "blacklist_del", Visibility: modules.VisibilityPublic, - Description: "Remove a text from the blacklist", + Description: "Remove from the blacklist", Parameters: "", Handler: s.handleDel(listBlack), }, { Name: "whitelist_add", Visibility: modules.VisibilityPublic, - Description: "Whitelist a text, or a message you reply to", + Description: "Whitelist a text or a reply", Parameters: "[text...]", Handler: s.handleAdd(listWhite), }, { Name: "whitelist_del", Visibility: modules.VisibilityPublic, - Description: "Remove a text from the whitelist", + Description: "Remove from the whitelist", Parameters: "", Handler: s.handleDel(listWhite), }, { Name: "blacklist_rules", Visibility: modules.VisibilityPublic, - Description: "List both lists for this topic", + Description: "List both lists here", Handler: s.handleRules, }, { Name: "blacklist_check", Visibility: modules.VisibilityPublic, - Description: "Check if a text is blacklisted here", + Description: "Check if a text is blacklisted", Parameters: "", Handler: s.handleCheck, }, + { + Name: "whitelist_rnd", + Visibility: modules.VisibilityPublic, + Description: "Random whitelist entry", + Handler: s.handleWhitelistRandom, + }, }, } } diff --git a/internal/modules/blacklist/handlers.go b/internal/modules/blacklist/handlers.go index a001c69..f82770f 100644 --- a/internal/modules/blacklist/handlers.go +++ b/internal/modules/blacklist/handlers.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "html" + "math/rand/v2" "sort" "strings" "time" @@ -291,9 +292,6 @@ func (s *state) entriesFor(ctx context.Context, chatID int64, threadID int, list } // handleCheck judges a text against this thread's rules. -// -// Unlike the mutation commands this applies no length cap: judging a long -// message is the point, and nothing here becomes a storage key. func (s *state) handleCheck(ctx context.Context, b *bot.Bot, update *models.Update) error { ctx, cancel := context.WithTimeout(ctx, handlerTimeout) defer cancel() @@ -307,7 +305,36 @@ func (s *state) handleCheck(ctx context.Context, b *bot.Bot, update *models.Upda if !ok { return chathelper.Reply(ctx, b, msg, "Usage: /blacklist_check ") } + return s.checkText(ctx, b, msg, normText) +} +// handleShort is /blacklist: the whole module behind one short name. Bare it +// lists both lists, with an argument it judges that text. +// +// The two behaviours are the two questions someone actually has — "what is set +// up here?" and "is this blocked?" — and neither needs its own long name once +// the argument distinguishes them. +func (s *state) handleShort(ctx context.Context, b *bot.Bot, update *models.Update) error { + ctx, cancel := context.WithTimeout(ctx, handlerTimeout) + defer cancel() + + msg := update.Message + if msg == nil { + return nil + } + + normText, ok := Normalize(chathelper.ArgAfterCommand(msg.Text)) + if !ok { + return s.showRules(ctx, b, msg) + } + return s.checkText(ctx, b, msg, normText) +} + +// checkText answers the verdict for already-normalized text. +// +// Unlike the mutation commands this applies no length cap: judging a long +// message is the point, and nothing here becomes a storage key. +func (s *state) checkText(ctx context.Context, b *bot.Bot, msg *models.Message, normText string) error { chatID, threadID := threadOf(msg) black, err := s.entriesFor(ctx, chatID, threadID, listBlack) if err != nil { @@ -350,7 +377,11 @@ func (s *state) handleRules(ctx context.Context, b *bot.Bot, update *models.Upda if msg == nil { return nil } + return s.showRules(ctx, b, msg) +} +// showRules renders both lists for the message's thread. +func (s *state) showRules(ctx context.Context, b *bot.Bot, msg *models.Message) error { chatID, threadID := threadOf(msg) blackPrefix := scopePrefix(chatID, threadID, listBlack) whitePrefix := scopePrefix(chatID, threadID, listWhite) @@ -413,3 +444,35 @@ func renderSection(sb *strings.Builder, list, prefix string, docs []storage.Doc[ sb.WriteString(line) } } + +// handleWhitelistRandom picks one whitelist entry at random. +func (s *state) handleWhitelistRandom(ctx context.Context, b *bot.Bot, update *models.Update) error { + ctx, cancel := context.WithTimeout(ctx, handlerTimeout) + defer cancel() + + msg := update.Message + if msg == nil { + return nil + } + + chatID, threadID := threadOf(msg) + prefix := scopePrefix(chatID, threadID, listWhite) + docs, err := s.store.Scan(ctx, prefix) + if err != nil { + log.Error("blacklist_rnd_scan", "list", listWhite, "err", err) + return chathelper.Reply(ctx, b, msg, genericFailure) + } + if len(docs) == 0 { + return chathelper.Reply(ctx, b, msg, + "This topic's whitelist is empty. Add something with /whitelist_add first.") + } + + pick := docs[rand.IntN(len(docs))] + // The record carries what the adder typed; the key carries only the + // normalized form, which is the fallback if a record ever lacks text. + text := pick.Val.Text + if text == "" { + text = decodeKeyText(strings.TrimPrefix(pick.ID, prefix)) + } + return chathelper.ReplyHTML(ctx, b, msg, ""+html.EscapeString(text)+"") +} diff --git a/internal/modules/blacklist/handlers_test.go b/internal/modules/blacklist/handlers_test.go index 373e8cd..66759c4 100644 --- a/internal/modules/blacklist/handlers_test.go +++ b/internal/modules/blacklist/handlers_test.go @@ -459,3 +459,95 @@ func TestAdd_ReplyStaysWithinOneMessage(t *testing.T) { t.Fatalf("add reply = %q; want a trim notice", last) } } + +// /blacklist is the whole module behind one name: bare it lists, with an +// argument it checks. +func TestShortCommand_ListsWhenBareAndChecksWithText(t *testing.T) { + rb := installBlacklist(t) + send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add cat")) + + bare := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist")) + rules := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_rules")) + if bare != rules { + t.Fatalf("/blacklist = %q; want the same as /blacklist_rules = %q", bare, rules) + } + + hit := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist cat")) + check := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_check cat")) + if hit != check { + t.Fatalf("/blacklist cat = %q; want the same as /blacklist_check cat = %q", hit, check) + } + if !strings.Contains(hit, "🚫") { + t.Fatalf("/blacklist cat = %q; want blocked", hit) + } +} + +// Whitespace alone is not an argument, so it still lists rather than checking. +func TestShortCommand_BlankArgumentStillLists(t *testing.T) { + rb := installBlacklist(t) + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist ")); !strings.Contains(got, "Blacklist") { + t.Fatalf("/blacklist with blank argument = %q; want the listing", got) + } +} + +func TestWhitelistRandom_RefusesAnEmptyList(t *testing.T) { + rb := installBlacklist(t) + // A populated blacklist must not make the whitelist look non-empty. + send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add cat")) + + got := send(t, rb, testutil.NewPrivateMessage(7, "/whitelist_rnd")) + if !strings.Contains(got, "empty") { + t.Fatalf("/whitelist_rnd = %q; want an empty-list message", got) + } +} + +func TestWhitelistRandom_PicksFromTheWhitelistOnly(t *testing.T) { + rb := installBlacklist(t) + send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add nope")) + entries := []string{"alpha", "beta", "gamma"} + for _, e := range entries { + send(t, rb, testutil.NewPrivateMessage(7, "/whitelist_add "+e)) + } + + seen := map[string]bool{} + for range 30 { + got := send(t, rb, testutil.NewPrivateMessage(7, "/whitelist_rnd")) + if strings.Contains(got, "nope") { + t.Fatalf("/whitelist_rnd = %q; picked from the blacklist", got) + } + match := "" + for _, e := range entries { + if strings.Contains(got, ""+e+"") { + match = e + } + } + if match == "" { + t.Fatalf("/whitelist_rnd = %q; want one of %v", got, entries) + } + seen[match] = true + } + // Three entries over thirty draws: landing on one every time would mean the + // pick is not random at all. + if len(seen) < 2 { + t.Fatalf("30 draws returned only %v", seen) + } +} + +func TestWhitelistRandom_IsPerTopic(t *testing.T) { + rb := installBlacklist(t) + send(t, rb, inTopic(-100, 11, "/whitelist_add alpha")) + + if got := send(t, rb, inTopic(-100, 12, "/whitelist_rnd")); !strings.Contains(got, "empty") { + t.Fatalf("/whitelist_rnd in a sibling topic = %q; want an empty-list message", got) + } +} + +func TestWhitelistRandom_EscapesUserText(t *testing.T) { + rb := installBlacklist(t) + send(t, rb, testutil.NewPrivateMessage(7, "/whitelist_add bold")) + + got := send(t, rb, testutil.NewPrivateMessage(7, "/whitelist_rnd")) + if strings.Contains(got, "bold") || !strings.Contains(got, "<b>bold</b>") { + t.Fatalf("/whitelist_rnd = %q; want the markup escaped", got) + } +}