Beta servers: testers can use beta features in servers whose owner opted in

- beta.Configure takes the opted-in servers; testers are looked up in the beta server
- beta commands registered per opted-in server and removed when it opts out
- beta.ServerEnabled for behaviour that no person triggers
- docs updated

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>

internal/beta/beta.go

@@ -1,16 +1,30 @@
-// Package beta gates features that are still being tried out. A beta feature
-// works only for members of the beta server who have the Beta Tester role:
-// the server comes from BETA_GUILD_ID and the role from BETA_ROLE_ID, both set
-// by the bot's owner in its environment, so nobody can change them from
-// Discord.
+// Package beta gates features that are still being tried out.
 //
-// Everything fails closed. With no role configured, or an ID that isn't a
-// Discord ID, nobody gets access.
+// A beta feature works for a beta tester, in a server where beta features are
+// on:
+//
+//   - A beta tester is a member of the beta server who has the Beta Tester
+//     role. Both are chosen by the bot's owner on the dashboard.
+//   - Beta features are on in the beta server itself, and in every other
+//     server whose owner opted in on the dashboard. A feature that can act on
+//     a whole server never reaches a server that didn't ask for it.
+//
+// Being a tester is about the person, so it is looked up in the beta server
+// even when they use a feature somewhere else (see SetRolesLookup). Beta
+// never replaces a feature's own permission checks: a beta command that needs
+// Manage Server still needs it.
+//
+// Features that act without anyone running a command (an event handler, a
+// background job) have no person to check, so they use ServerEnabled only.
+//
+// Everything fails closed. With no role configured, an ID that isn't a
+// Discord ID, or a lookup that fails, nobody gets access.
 package beta
 
 import (
 	"log"
 	"regexp"
+	"slices"
 	"strings"
 	"sync/atomic"
 
@@ -20,32 +34,43 @@ import (
 // Handler is a slash command handler.
 type Handler = func(s *discordgo.Session, i *discordgo.InteractionCreate)
 
-type settings struct{ guildID, roleID string }
+type settings struct {
+	guildID, roleID string
+	servers         map[string]bool // opted-in servers, besides the beta server
+}
 
 var current atomic.Pointer[settings]
 
 var snowflake = regexp.MustCompile(`^[0-9]{1,20}$`)
 
-// idOrEmpty keeps a Discord ID and drops anything else, so a typo in the
-// environment turns beta features off instead of matching something odd.
+// idOrEmpty keeps a Discord ID and drops anything else, so a bad value turns
+// beta features off instead of matching something odd.
 func idOrEmpty(name, v string) string {
 	v = strings.TrimSpace(v)
 	if v == "" {
 		return ""
 	}
 	if !snowflake.MatchString(v) {
-		log.Printf("beta: %s %q isn't a Discord ID; beta features stay locked", name, v)
+		log.Printf("beta: %s %q isn't a Discord ID; ignoring it", name, v)
 		return ""
 	}
 	return v
 }
 
-// Configure sets the beta server and the Beta Tester role. Call it once at
-// startup.
-func Configure(guildID, roleID string) {
+// Configure sets the beta server, the Beta Tester role and the servers that
+// opted in. It replaces everything set before, so call it with the full
+// picture each time.
+func Configure(guildID, roleID string, servers []string) {
+	set := make(map[string]bool, len(servers))
+	for _, id := range servers {
+		if id = idOrEmpty("beta server", id); id != "" {
+			set[id] = true
+		}
+	}
 	current.Store(&settings{
-		guildID: idOrEmpty("BETA_GUILD_ID", guildID),
-		roleID:  idOrEmpty("BETA_ROLE_ID", roleID),
+		guildID: idOrEmpty("beta server", guildID),
+		roleID:  idOrEmpty("beta role", roleID),
+		servers: set,
 	})
 }
 
@@ -56,45 +81,90 @@ func get() settings {
 	return settings{}
 }
 
-// GuildID is the beta server, or "" when beta features are off.
+// RolesFunc returns a member's role IDs in a server, as Discord has them now,
+// or an error (including "not in that server").
+type RolesFunc func(guildID, userID string) ([]string, error)
+
+var lookup atomic.Pointer[RolesFunc]
+
+// SetRolesLookup tells the package how to find someone's roles in the beta
+// server when they use a feature in another server. Without it, testers only
+// qualify in the beta server itself.
+func SetRolesLookup(f RolesFunc) { lookup.Store(&f) }
+
+// GuildID is the beta server, or "" when it isn't set.
 func GuildID() string { return get().guildID }
 
+// Servers lists the servers that opted in, besides the beta server, in a
+// stable order.
+func Servers() []string {
+	c := get()
+	out := make([]string, 0, len(c.servers))
+	for id := range c.servers {
+		if id != c.guildID {
+			out = append(out, id)
+		}
+	}
+	slices.Sort(out)
+	return out
+}
+
+// ServerEnabled reports whether beta features are on in a server: the beta
+// server, or one whose owner opted in. Use it alone for anything no person
+// triggers.
+func ServerEnabled(guildID string) bool {
+	c := get()
+	return guildID != "" && (guildID == c.guildID || c.servers[guildID])
+}
+
 // Status describes where a member stands, for the /beta command.
 type Status struct {
-	InBetaGuild    bool // the interaction came from the beta server
+	ServerEnabled  bool // beta features are on in this server
 	RoleConfigured bool // the owner has set a Beta Tester role
-	HasRole        bool // the member has it
+	HasRole        bool // the member is a tester
 }
 
-// StatusOf reports a member's standing in the guild they're acting in.
+// StatusOf reports a member's standing in the server they're acting in.
 func StatusOf(guildID string, member *discordgo.Member) Status {
 	c := get()
 	st := Status{
-		InBetaGuild:    c.guildID != "" && guildID == c.guildID,
+		ServerEnabled:  ServerEnabled(guildID),
 		RoleConfigured: c.roleID != "",
 	}
-	if st.InBetaGuild && st.RoleConfigured && member != nil {
-		for _, r := range member.Roles {
-			if r == c.roleID {
-				st.HasRole = true
-				break
-			}
-		}
+	if st.ServerEnabled && st.RoleConfigured && member != nil {
+		st.HasRole = isTester(c, guildID, member)
 	}
 	return st
 }
 
-// Allowed reports whether a member may use beta features in guildID: it has
-// to be the beta server, and the member has to have the Beta Tester role.
-// Use it for anything that isn't a command, such as an event handler.
+// isTester checks the Beta Tester role in the beta server. In the beta server
+// the roles come with the interaction. Anywhere else they are looked up.
+func isTester(c settings, guildID string, member *discordgo.Member) bool {
+	if guildID == c.guildID {
+		return slices.Contains(member.Roles, c.roleID)
+	}
+	f := lookup.Load()
+	if f == nil || *f == nil || member.User == nil || member.User.ID == "" {
+		return false
+	}
+	roles, err := (*f)(c.guildID, member.User.ID)
+	if err != nil {
+		return false
+	}
+	return slices.Contains(roles, c.roleID)
+}
+
+// Allowed reports whether a member may use beta features in guildID: beta
+// features have to be on there, and the member has to be a tester. Use it for
+// anything a person triggers that isn't a command, such as a button.
 func Allowed(guildID string, member *discordgo.Member) bool {
 	st := StatusOf(guildID, member)
-	return st.InBetaGuild && st.RoleConfigured && st.HasRole
+	return st.ServerEnabled && st.RoleConfigured && st.HasRole
 }
 
 // AllowedInteraction is Allowed for an interaction. A DM has no member, so it
-// never qualifies. The member and their roles come from Discord with the
-// interaction, not from anything the user typed.
+// never qualifies. The member comes from Discord with the interaction, not
+// from anything the user typed.
 func AllowedInteraction(i *discordgo.InteractionCreate) bool {
 	return i != nil && i.Interaction != nil && Allowed(i.GuildID, i.Member)
 }
@@ -119,7 +189,7 @@ var respond = func(s *discordgo.Session, i *discordgo.InteractionCreate, msg str
 	})
 }
 
-const denied = "This feature is in beta and isn't available to you yet."
+const denied = "This feature is in beta and isn't available to you or this server yet."
 
 // Guard is for the start of a beta feature's handler, and for its component,
 // modal and autocomplete handlers too (the command's gate doesn't cover

internal/beta/beta_test.go

@@ -1,35 +1,54 @@
 package beta
 
 import (
+	"errors"
 	"testing"
 
 	"github.com/bwmarrin/discordgo"
 )
 
 const (
-	betaGuild = "1170934850157547600"
-	testerRol = "555000000000000001"
+	betaGuild  = "1170934850157547600"
+	otherGuild = "999000000000000001"
+	testerRol  = "555000000000000001"
+	testerID   = "777000000000000001"
 )
 
-func member(roles ...string) *discordgo.Member { return &discordgo.Member{Roles: roles} }
+func member(roles ...string) *discordgo.Member {
+	return &discordgo.Member{Roles: roles, User: &discordgo.User{ID: testerID}}
+}
 
 func interaction(guildID string, m *discordgo.Member) *discordgo.InteractionCreate {
 	return &discordgo.InteractionCreate{Interaction: &discordgo.Interaction{GuildID: guildID, Member: m}}
 }
 
-func TestAllowed(t *testing.T) {
+// useLookup makes the beta server say who holds which roles, as Discord would.
+func useLookup(t *testing.T, rolesByUser map[string][]string, err error) *int {
+	t.Helper()
+	calls := 0
+	SetRolesLookup(func(guildID, userID string) ([]string, error) {
+		calls++
+		if guildID != betaGuild {
+			t.Errorf("the lookup must ask the beta server, asked %q", guildID)
+		}
+		return rolesByUser[userID], err
+	})
+	t.Cleanup(func() { lookup.Store(nil) })
+	return &calls
+}
+
+func TestAllowedInTheBetaServer(t *testing.T) {
 	cases := []struct {
-		name            string
-		guild, role     string // configuration
-		inGuild         string
-		member          *discordgo.Member
-		want            bool
-		inBeta, hasRole bool
+		name        string
+		guild, role string // configuration
+		inGuild     string
+		member      *discordgo.Member
+		want        bool
+		on, hasRole bool
 	}{
 		{"tester in the beta server", betaGuild, testerRol, betaGuild, member("1", testerRol), true, true, true},
 		{"member without the role", betaGuild, testerRol, betaGuild, member("1", "2"), false, true, false},
 		{"member with no roles at all", betaGuild, testerRol, betaGuild, member(), false, true, false},
-		{"the role in another server doesn't count", betaGuild, testerRol, "999000000000000001", member(testerRol), false, false, false},
 		{"a DM has no member", betaGuild, testerRol, "", nil, false, false, false},
 		{"a nil member in the beta server", betaGuild, testerRol, betaGuild, nil, false, true, false},
 		{"no role configured: nobody", betaGuild, "", betaGuild, member(testerRol, ""), false, true, false},
@@ -41,7 +60,7 @@ func TestAllowed(t *testing.T) {
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
-			Configure(tc.guild, tc.role)
+			Configure(tc.guild, tc.role, nil)
 			if got := Allowed(tc.inGuild, tc.member); got != tc.want {
 				t.Errorf("Allowed = %v, want %v", got, tc.want)
 			}
@@ -49,16 +68,113 @@ func TestAllowed(t *testing.T) {
 				t.Errorf("AllowedInteraction = %v, want %v", got, tc.want)
 			}
 			st := StatusOf(tc.inGuild, tc.member)
-			if st.InBetaGuild != tc.inBeta || st.HasRole != tc.hasRole {
-				t.Errorf("StatusOf = %+v, want in beta server %v, has role %v", st, tc.inBeta, tc.hasRole)
+			if st.ServerEnabled != tc.on || st.HasRole != tc.hasRole {
+				t.Errorf("StatusOf = %+v, want server on %v, tester %v", st, tc.on, tc.hasRole)
 			}
 		})
 	}
 }
 
+func TestOptedInServers(t *testing.T) {
+	roles := map[string][]string{testerID: {"1", testerRol}}
+
+	t.Run("a tester in an opted-in server is allowed", func(t *testing.T) {
+		Configure(betaGuild, testerRol, []string{otherGuild})
+		calls := useLookup(t, roles, nil)
+		if !Allowed(otherGuild, member()) { // roles in this server don't matter
+			t.Error("tester in an opted-in server should be allowed")
+		}
+		if *calls != 1 {
+			t.Errorf("the beta server should be asked once, was asked %d times", *calls)
+		}
+	})
+
+	t.Run("the role in the other server doesn't count", func(t *testing.T) {
+		Configure(betaGuild, testerRol, []string{otherGuild})
+		useLookup(t, map[string][]string{}, nil)
+		if Allowed(otherGuild, member(testerRol)) {
+			t.Error("holding a role with that ID in another server must not make anyone a tester")
+		}
+	})
+
+	t.Run("a server that didn't opt in is refused, and nothing is looked up", func(t *testing.T) {
+		Configure(betaGuild, testerRol, []string{otherGuild})
+		calls := useLookup(t, roles, nil)
+		if Allowed("123000000000000001", member()) {
+			t.Error("a server that didn't opt in must be refused")
+		}
+		if *calls != 0 {
+			t.Error("no lookup for a server where beta is off")
+		}
+	})
+
+	t.Run("a failing lookup fails closed", func(t *testing.T) {
+		Configure(betaGuild, testerRol, []string{otherGuild})
+		useLookup(t, roles, errors.New("Unknown Member"))
+		if Allowed(otherGuild, member()) {
+			t.Error("an error from Discord must mean no")
+		}
+	})
+
+	t.Run("no lookup set: testers only qualify in the beta server", func(t *testing.T) {
+		Configure(betaGuild, testerRol, []string{otherGuild})
+		lookup.Store(nil)
+		if Allowed(otherGuild, member()) {
+			t.Error("without a lookup there is no way to know")
+		}
+		if !Allowed(betaGuild, member(testerRol)) {
+			t.Error("the beta server doesn't need the lookup")
+		}
+	})
+
+	t.Run("a user without an ID can't be looked up", func(t *testing.T) {
+		Configure(betaGuild, testerRol, []string{otherGuild})
+		useLookup(t, roles, nil)
+		if Allowed(otherGuild, &discordgo.Member{}) || Allowed(otherGuild, &discordgo.Member{User: &discordgo.User{}}) {
+			t.Error("no user, no access")
+		}
+	})
+
+	t.Run("no role configured: nobody, even when the server opted in", func(t *testing.T) {
+		Configure(betaGuild, "", []string{otherGuild})
+		useLookup(t, map[string][]string{testerID: {""}}, nil)
+		if Allowed(otherGuild, member()) {
+			t.Error("without a role there are no testers")
+		}
+	})
+
+	t.Run("bad IDs in the list are dropped", func(t *testing.T) {
+		Configure(betaGuild, testerRol, []string{otherGuild, "not-an-id", "", " 5 "})
+		got := Servers()
+		if len(got) != 2 || got[0] != "5" || got[1] != otherGuild {
+			t.Errorf("Servers = %v", got)
+		}
+	})
+
+	t.Run("the beta server isn't listed twice", func(t *testing.T) {
+		Configure(betaGuild, testerRol, []string{betaGuild, otherGuild})
+		if got := Servers(); len(got) != 1 || got[0] != otherGuild {
+			t.Errorf("Servers = %v", got)
+		}
+	})
+}
+
+func TestServerEnabledNeedsNoPerson(t *testing.T) {
+	Configure(betaGuild, testerRol, []string{otherGuild})
+	for guild, want := range map[string]bool{betaGuild: true, otherGuild: true, "123": false, "": false} {
+		if got := ServerEnabled(guild); got != want {
+			t.Errorf("ServerEnabled(%q) = %v, want %v", guild, got, want)
+		}
+	}
+	current.Store(nil)
+	if ServerEnabled(betaGuild) {
+		t.Error("before Configure nothing is on")
+	}
+}
+
 func TestNotConfiguredAtAll(t *testing.T) {
 	current.Store(nil)
-	if Allowed(betaGuild, member(testerRol)) || GuildID() != "" {
+	if Allowed(betaGuild, member(testerRol)) || GuildID() != "" || len(Servers()) != 0 {
 		t.Error("before Configure nothing is allowed and there's no beta server")
 	}
 	if AllowedInteraction(nil) || AllowedInteraction(&discordgo.InteractionCreate{}) {
@@ -67,7 +183,8 @@ func TestNotConfiguredAtAll(t *testing.T) {
 }
 
 func TestGateRunsTheHandlerOnlyForTesters(t *testing.T) {
-	Configure(betaGuild, testerRol)
+	Configure(betaGuild, testerRol, []string{otherGuild})
+	useLookup(t, map[string][]string{}, nil) // nobody holds the role in the beta server
 	var answered []string
 	orig := respond
 	respond = func(_ *discordgo.Session, _ *discordgo.InteractionCreate, msg string) {
@@ -84,9 +201,10 @@ func TestGateRunsTheHandlerOnlyForTesters(t *testing.T) {
 	}
 
 	for name, i := range map[string]*discordgo.InteractionCreate{
-		"no role":      interaction(betaGuild, member("1")),
-		"wrong server": interaction("999000000000000001", member(testerRol)),
-		"DM":           interaction("", nil),
+		"no role":                interaction(betaGuild, member("1")),
+		"opted-in, not a tester": interaction(otherGuild, member(testerRol)),
+		"server not opted in":    interaction("123000000000000001", member(testerRol)),
+		"DM":                     interaction("", nil),
 	} {
 		before := ran
 		h(nil, i)
@@ -94,13 +212,20 @@ func TestGateRunsTheHandlerOnlyForTesters(t *testing.T) {
 			t.Errorf("%s: the handler must not run", name)
 		}
 	}
-	if len(answered) != 3 || answered[0] != denied {
+	if len(answered) != 4 || answered[0] != denied {
 		t.Errorf("each refusal is answered with the beta message: %v", answered)
 	}
+
+	useLookup(t, map[string][]string{testerID: {testerRol}}, nil)
+	before := ran
+	h(nil, interaction(otherGuild, member()))
+	if ran != before+1 {
+		t.Error("a tester in an opted-in server runs the handler")
+	}
 }
 
 func TestGuardOnItsOwn(t *testing.T) {
-	Configure(betaGuild, testerRol)
+	Configure(betaGuild, testerRol, nil)
 	n := 0
 	orig := respond
 	respond = func(*discordgo.Session, *discordgo.InteractionCreate, string) { n++ }