Introduce PresenceTracker interface for online-user tracking

Replace the leaky *redis.Client field on Server with a PresenceTracker
interface that receives connect/rename/disconnect lifecycle events. This
removes Redis-specific set encoding from hotline/server.go and the session
handlers, and drops the redis dependency from the hotline package.

The Redis implementation moves to mobius.RedisPresenceTracker, which owns
the legacy "login::ip"/"login:nickname:ip" set encoding so existing
deployments keep working. The API server reads online users through a new
OnlineLister interface and falls back to the in-memory ClientMgr when no
presence tracker is configured. Startup clearing of stale online state now
happens unconditionally when Redis is configured, not only when the API
server is enabled.

5 files changed, 333 insertions, 46 deletions

diff --git a/internal/mobius/api.go b/internal/mobius/api.go
index cf0a46f..3620cf4 100644
--- a/internal/mobius/api.go
+++ b/internal/mobius/api.go
@@ -8,11 +8,16 @@ import (
 	"log"
 	"log/slog"
 	"net/http"
-	"strings"
 
 	"github.com/jhalter/mobius/hotline"
 )
 
+// OnlineLister returns the list of currently online users. It is implemented by
+// RedisPresenceTracker; when nil, the APIServer falls back to the in-memory ClientMgr.
+type OnlineLister interface {
+	Online(ctx context.Context) ([]OnlineUser, error)
+}
+
 type logResponseWriter struct {
 	http.ResponseWriter
 	statusCode int
@@ -37,6 +42,7 @@ func (lrw *logResponseWriter) Write(b []byte) (int, error) {
 // It supports user management, banning operations, and server administration.
 type APIServer struct {
 	hlServer *hotline.Server
+	online   OnlineLister
 	logger   *slog.Logger
 	mux      *http.ServeMux
 	apiKey   string
@@ -63,9 +69,10 @@ func (srv *APIServer) logMiddleware(next http.Handler) http.Handler {
 
 // NewAPIServer creates a new APIServer instance with the specified configuration.
 // It sets up all API routes and middleware.
-func NewAPIServer(hlServer *hotline.Server, reloadFunc func(), logger *slog.Logger, apiKey string) *APIServer {
+func NewAPIServer(hlServer *hotline.Server, online OnlineLister, reloadFunc func(), logger *slog.Logger, apiKey string) *APIServer {
 	srv := APIServer{
 		hlServer: hlServer,
+		online:   online,
 		logger:   logger,
 		mux:      http.NewServeMux(),
 		apiKey:   apiKey,
@@ -81,14 +88,6 @@ func NewAPIServer(hlServer *hotline.Server, reloadFunc func(), logger *slog.Logg
 	srv.mux.Handle("/api/v1/shutdown", srv.logMiddleware(srv.authMiddleware(http.HandlerFunc(srv.ShutdownHandler))))
 	srv.mux.Handle("/api/v1/stats", srv.logMiddleware(srv.authMiddleware(http.HandlerFunc(srv.RenderStats))))
 
-	if hlServer.Redis != nil {
-		if err := hlServer.Redis.Del(context.Background(), hotline.RedisKeyOnline).Err(); err != nil {
-			srv.logger.Warn("Failed to clear online users in Redis", "err", err)
-		} else {
-			srv.logger.Debug("Cleared online users in Redis on startup")
-		}
-	}
-
 	return &srv
 }
 
@@ -97,18 +96,15 @@ func NewAPIServer(hlServer *hotline.Server, reloadFunc func(), logger *slog.Logg
 func (srv *APIServer) OnlineHandler(w http.ResponseWriter, r *http.Request) {
 	var users []map[string]string
 
-	if srv.hlServer.Redis != nil {
-		members, err := srv.hlServer.Redis.SMembers(r.Context(), hotline.RedisKeyOnline).Result()
+	if srv.online != nil {
+		online, err := srv.online.Online(r.Context())
 		if err == nil {
-			for _, m := range members {
-				parts := strings.SplitN(m, ":", 3)
-				if len(parts) == 3 {
-					users = append(users, map[string]string{
-						"login":    parts[0],
-						"nickname": parts[1],
-						"ip":       parts[2],
-					})
-				}
+			for _, u := range online {
+				users = append(users, map[string]string{
+					"login":    u.Login,
+					"nickname": u.Nickname,
+					"ip":       u.IP,
+				})
 			}
 		}
 	} else {
diff --git a/internal/mobius/handlers_session.go b/internal/mobius/handlers_session.go
index f17ffa3..443fed0 100644
--- a/internal/mobius/handlers_session.go
+++ b/internal/mobius/handlers_session.go
@@ -1,7 +1,6 @@
 package mobius
 
 import (
-	"context"
 	"encoding/binary"
 	"io"
 	"math/big"
@@ -110,19 +109,14 @@ func HandleTranAgreed(cc *hotline.ClientConn, t *hotline.Transaction) (res []hot
 	login := cc.Account.Login
 	ip := cc.IP()
 
-	if cc.Server.Redis != nil {
-		// Remove old entry (login::ip)
-		cc.Server.Redis.SRem(context.Background(), hotline.RedisKeyOnline, login+"::"+ip)
-		// Add new entry with login, nickname, ip
-		cc.Server.Redis.SAdd(context.Background(), hotline.RedisKeyOnline, login+":"+string(cc.GetUserName())+":"+ip)
+	if cc.Server.Presence != nil {
+		cc.Server.Presence.UserRenamed(login, "", string(cc.GetUserName()), ip)
 	}
 
 	// Ban check for nickname
 	if cc.Server.BanList != nil && cc.Server.BanList.IsNicknameBanned(string(cc.GetUserName())) {
-		if cc.Server.Redis != nil {
-			// Remove all possible online entries for this login and IP
-			cc.Server.Redis.SRem(context.Background(), hotline.RedisKeyOnline, login+"::"+ip)
-			cc.Server.Redis.SRem(context.Background(), hotline.RedisKeyOnline, login+":"+string(cc.GetUserName())+":"+ip)
+		if cc.Server.Presence != nil {
+			cc.Server.Presence.UserDisconnected(login, string(cc.GetUserName()), ip)
 		}
 		if err := cc.Server.BanList.Add(ip, nil); err != nil {
 			cc.Logger.Error("Failed to ban IP for banned nickname", "ip", ip, "err", err)
@@ -275,25 +269,14 @@ func HandleSetClientUserInfo(cc *hotline.ClientConn, t *hotline.Transaction) (re
 		login := cc.Account.Login
 		ip := cc.IP()
 
-		if cc.Server.Redis != nil {
-			// Remove old entry (login:oldnickname:ip) and (login::ip)
-			cc.Server.Redis.SRem(context.Background(), hotline.RedisKeyOnline, login+"::"+ip)
-			if oldNickname != "" {
-				cc.Server.Redis.SRem(context.Background(), hotline.RedisKeyOnline, login+":"+oldNickname+":"+ip)
-			}
-			// Add new entry
-			cc.Server.Redis.SAdd(context.Background(), hotline.RedisKeyOnline, login+":"+newNickname+":"+ip)
+		if cc.Server.Presence != nil {
+			cc.Server.Presence.UserRenamed(login, oldNickname, newNickname, ip)
 		}
 
 		// Ban check for nickname
 		if cc.Server.BanList != nil && cc.Server.BanList.IsNicknameBanned(newNickname) {
-			if cc.Server.Redis != nil {
-				// Remove all possible online entries for this login and IP
-				cc.Server.Redis.SRem(context.Background(), hotline.RedisKeyOnline, login+"::"+ip)
-				cc.Server.Redis.SRem(context.Background(), hotline.RedisKeyOnline, login+":"+newNickname+":"+ip)
-				if oldNickname != "" {
-					cc.Server.Redis.SRem(context.Background(), hotline.RedisKeyOnline, login+":"+oldNickname+":"+ip)
-				}
+			if cc.Server.Presence != nil {
+				cc.Server.Presence.UserDisconnected(login, newNickname, ip)
 			}
 			if err := cc.Server.BanList.Add(ip, nil); err != nil {
 				cc.Logger.Error("Failed to ban IP for banned nickname", "ip", ip, "err", err)
diff --git a/internal/mobius/handlers_session_test.go b/internal/mobius/handlers_session_test.go
index 407b132..bffc81c 100644
--- a/internal/mobius/handlers_session_test.go
+++ b/internal/mobius/handlers_session_test.go
@@ -932,3 +932,103 @@ func TestHandleUserBroadcast(t *testing.T) {
 		})
 	}
 }
+
+// presenceCall records a single PresenceTracker invocation for assertions.
+type presenceCall struct {
+	method      string
+	login       string
+	oldNickname string
+	newNickname string
+	nickname    string
+	ip          string
+}
+
+type fakePresenceTracker struct {
+	calls []presenceCall
+}
+
+func (f *fakePresenceTracker) UserConnected(login, ip string) {
+	f.calls = append(f.calls, presenceCall{method: "UserConnected", login: login, ip: ip})
+}
+
+func (f *fakePresenceTracker) UserRenamed(login, oldNickname, newNickname, ip string) {
+	f.calls = append(f.calls, presenceCall{method: "UserRenamed", login: login, oldNickname: oldNickname, newNickname: newNickname, ip: ip})
+}
+
+func (f *fakePresenceTracker) UserDisconnected(login, nickname, ip string) {
+	f.calls = append(f.calls, presenceCall{method: "UserDisconnected", login: login, nickname: nickname, ip: ip})
+}
+
+func newPresenceTestServer(presence hotline.PresenceTracker) *hotline.Server {
+	m := hotline.MockClientMgr{}
+	m.On("List").Return([]*hotline.ClientConn{})
+	return &hotline.Server{
+		TextDecoder: charmap.Macintosh.NewDecoder(),
+		TextEncoder: charmap.Macintosh.NewEncoder(),
+		Config:      hotline.Config{BannerFile: "Banner.jpg"},
+		ClientMgr:   &m,
+		Presence:    presence,
+	}
+}
+
+func TestHandleTranAgreed_NotifiesPresenceTracker(t *testing.T) {
+	presence := &fakePresenceTracker{}
+	cc := &hotline.ClientConn{
+		Account: &hotline.Account{
+			Login: "alice",
+			Access: func() hotline.AccessBitmap {
+				var bits hotline.AccessBitmap
+				bits.Set(hotline.AccessAnyName)
+				return bits
+			}(),
+		},
+		ID:         [2]byte{0, 1},
+		Version:    []byte{0, 1},
+		RemoteAddr: "192.168.1.1:12345",
+		Logger:     NewTestLogger(),
+		Server:     newPresenceTestServer(presence),
+	}
+
+	tr := hotline.NewTransaction(
+		hotline.TranAgreed, [2]byte{},
+		hotline.NewField(hotline.FieldUserName, []byte("Alice")),
+		hotline.NewField(hotline.FieldUserIconID, []byte{0, 1}),
+		hotline.NewField(hotline.FieldOptions, []byte{0, 0}),
+	)
+
+	HandleTranAgreed(cc, &tr)
+
+	assert.Equal(t, []presenceCall{
+		{method: "UserRenamed", login: "alice", oldNickname: "", newNickname: "Alice", ip: "192.168.1.1"},
+	}, presence.calls)
+}
+
+func TestHandleSetClientUserInfo_NotifiesPresenceTracker(t *testing.T) {
+	presence := &fakePresenceTracker{}
+	cc := &hotline.ClientConn{
+		Account: &hotline.Account{
+			Login: "alice",
+			Access: func() hotline.AccessBitmap {
+				var bits hotline.AccessBitmap
+				bits.Set(hotline.AccessAnyName)
+				return bits
+			}(),
+		},
+		ID:         [2]byte{0, 1},
+		UserName:   []byte("Alice"),
+		RemoteAddr: "192.168.1.1:12345",
+		Logger:     NewTestLogger(),
+		Server:     newPresenceTestServer(presence),
+	}
+
+	tr := hotline.NewTransaction(
+		hotline.TranSetClientUserInfo, [2]byte{},
+		hotline.NewField(hotline.FieldUserName, []byte("Alice2")),
+	)
+
+	HandleSetClientUserInfo(cc, &tr)
+
+	assert.Equal(t, []presenceCall{
+		{method: "UserRenamed", login: "alice", oldNickname: "Alice", newNickname: "Alice2", ip: "192.168.1.1"},
+	}, presence.calls)
+}
diff --git a/internal/mobius/redis_presence.go b/internal/mobius/redis_presence.go
new file mode 100644
index 0000000..f0eed54
--- /dev/null
+++ b/internal/mobius/redis_presence.go
@@ -0,0 +1,81 @@
+package mobius
+
+import (
+	"context"
+	"log/slog"
+	"strings"
+
+	"github.com/redis/go-redis/v9"
+)
+
+// redisKeyOnline is the Redis set holding currently online users.
+const redisKeyOnline = "mobius:online"
+
+// OnlineUser describes a currently online user as reported by an OnlineLister.
+type OnlineUser struct {
+	Login    string `json:"login"`
+	Nickname string `json:"nickname"`
+	IP       string `json:"ip"`
+}
+
+// RedisPresenceTracker implements hotline.PresenceTracker backed by a Redis set. Set members
+// use the legacy formats "login::ip" (nickname unknown) and "login:nickname:ip" so existing
+// deployments and tooling continue to work.
+type RedisPresenceTracker struct {
+	client *redis.Client
+	logger *slog.Logger
+}
+
+// NewRedisPresenceTracker creates a new Redis-backed presence tracker.
+func NewRedisPresenceTracker(client *redis.Client, logger *slog.Logger) *RedisPresenceTracker {
+	return &RedisPresenceTracker{client: client, logger: logger}
+}
+
+func (r *RedisPresenceTracker) UserConnected(login, ip string) {
+	if err := r.client.SAdd(context.Background(), redisKeyOnline, login+"::"+ip).Err(); err != nil {
+		r.logger.Warn("Failed to record online user in Redis", "err", err)
+	}
+}
+
+func (r *RedisPresenceTracker) UserRenamed(login, oldNickname, newNickname, ip string) {
+	ctx := context.Background()
+	// Remove the pre-nickname entry and, if present, the previous nickname entry.
+	r.client.SRem(ctx, redisKeyOnline, login+"::"+ip)
+	if oldNickname != "" {
+		r.client.SRem(ctx, redisKeyOnline, login+":"+oldNickname+":"+ip)
+	}
+	if err := r.client.SAdd(ctx, redisKeyOnline, login+":"+newNickname+":"+ip).Err(); err != nil {
+		r.logger.Warn("Failed to update online user in Redis", "err", err)
+	}
+}
+
+func (r *RedisPresenceTracker) UserDisconnected(login, nickname, ip string) {
+	ctx := context.Background()
+	r.client.SRem(ctx, redisKeyOnline, login+"::"+ip)
+	if nickname != "" {
+		r.client.SRem(ctx, redisKeyOnline, login+":"+nickname+":"+ip)
+	}
+}
+
+// Clear removes all online-user entries. Call on startup to discard stale state from a
+// previous run.
+func (r *RedisPresenceTracker) Clear(ctx context.Context) error {
+	return r.client.Del(ctx, redisKeyOnline).Err()
+}
+
+// Online returns the list of currently online users parsed from the Redis set.
+func (r *RedisPresenceTracker) Online(ctx context.Context) ([]OnlineUser, error) {
+	members, err := r.client.SMembers(ctx, redisKeyOnline).Result()
+	if err != nil {
+		return nil, err
+	}
+
+	var users []OnlineUser
+	for _, m := range members {
+		parts := strings.SplitN(m, ":", 3)
+		if len(parts) == 3 {
+			users = append(users, OnlineUser{Login: parts[0], Nickname: parts[1], IP: parts[2]})
+		}
+	}
+	return users, nil
+}
diff --git a/internal/mobius/redis_presence_test.go b/internal/mobius/redis_presence_test.go
new file mode 100644
index 0000000..ac4cd56
--- /dev/null
+++ b/internal/mobius/redis_presence_test.go
@@ -0,0 +1,127 @@
+package mobius
+
+import (
+	"context"
+	"io"
+	"log/slog"
+	"testing"
+
+	"github.com/alicebob/miniredis/v2"
+	"github.com/redis/go-redis/v9"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func isMember(t *testing.T, s *miniredis.Miniredis, member string) bool {
+	t.Helper()
+	// miniredis returns an error (rather than false, like real Redis) when the key is absent,
+	// which happens once the set is emptied.
+	if !s.Exists(redisKeyOnline) {
+		return false
+	}
+	ok, err := s.SIsMember(redisKeyOnline, member)
+	require.NoError(t, err)
+	return ok
+}
+
+func newTestPresenceTracker(t *testing.T) (*RedisPresenceTracker, *miniredis.Miniredis) {
+	t.Helper()
+
+	s, err := miniredis.Run()
+	require.NoError(t, err)
+	t.Cleanup(s.Close)
+
+	client := redis.NewClient(&redis.Options{Addr: s.Addr()})
+	t.Cleanup(func() { _ = client.Close() })
+
+	logger := slog.New(slog.NewTextHandler(io.Discard, nil))
+	return NewRedisPresenceTracker(client, logger), s
+}
+
+func TestRedisPresenceTracker_LegacyMemberFormats(t *testing.T) {
+	// Lock the exact Redis set-member strings so existing deployments keep working.
+	tracker, s := newTestPresenceTracker(t)
+
+	t.Run("UserConnected stores login::ip", func(t *testing.T) {
+		tracker.UserConnected("alice", "192.168.1.1")
+		assert.True(t, isMember(t, s, "alice::192.168.1.1"))
+	})
+
+	t.Run("UserRenamed with no old nickname swaps login::ip for login:nick:ip", func(t *testing.T) {
+		s.FlushAll()
+		tracker.UserConnected("bob", "10.0.0.1")
+		tracker.UserRenamed("bob", "", "Bobby", "10.0.0.1")
+
+		assert.False(t, isMember(t, s, "bob::10.0.0.1"))
+		assert.True(t, isMember(t, s, "bob:Bobby:10.0.0.1"))
+	})
+
+	t.Run("UserRenamed removes the previous nickname entry", func(t *testing.T) {
+		s.FlushAll()
+		tracker.UserRenamed("carol", "", "Carol1", "10.0.0.2")
+		tracker.UserRenamed("carol", "Carol1", "Carol2", "10.0.0.2")
+
+		assert.False(t, isMember(t, s, "carol:Carol1:10.0.0.2"))
+		assert.True(t, isMember(t, s, "carol:Carol2:10.0.0.2"))
+	})
+
+	t.Run("UserDisconnected removes both possible entries", func(t *testing.T) {
+		s.FlushAll()
+		tracker.UserRenamed("dave", "", "Dave", "10.0.0.3")
+		tracker.UserDisconnected("dave", "Dave", "10.0.0.3")
+
+		assert.False(t, isMember(t, s, "dave::10.0.0.3"))
+		assert.False(t, isMember(t, s, "dave:Dave:10.0.0.3"))
+	})
+
+	t.Run("UserDisconnected with empty nickname removes login::ip", func(t *testing.T) {
+		s.FlushAll()
+		tracker.UserConnected("erin", "10.0.0.4")
+		tracker.UserDisconnected("erin", "", "10.0.0.4")
+
+		assert.False(t, isMember(t, s, "erin::10.0.0.4"))
+	})
+}
+
+func TestRedisPresenceTracker_Online(t *testing.T) {
+	tracker, _ := newTestPresenceTracker(t)
+	ctx := context.Background()
+
+	tracker.UserRenamed("alice", "", "Alice", "192.168.1.1")
+	tracker.UserRenamed("bob", "", "Bob", "192.168.1.2")
+
+	users, err := tracker.Online(ctx)
+	require.NoError(t, err)
+	require.Len(t, users, 2)
+
+	// Order is not guaranteed by the Redis set, so index by login.
+	byLogin := map[string]OnlineUser{}
+	for _, u := range users {
+		byLogin[u.Login] = u
+	}
+	assert.Equal(t, OnlineUser{Login: "alice", Nickname: "Alice", IP: "192.168.1.1"}, byLogin["alice"])
+	assert.Equal(t, OnlineUser{Login: "bob", Nickname: "Bob", IP: "192.168.1.2"}, byLogin["bob"])
+}
+
+func TestRedisPresenceTracker_OnlinePreNicknameEntry(t *testing.T) {
+	// A connected-but-not-yet-named user is stored as "login::ip", which splits into three
+	// parts with an empty nickname. The previous api.go logic included it the same way, so
+	// preserve that behavior.
+	tracker, _ := newTestPresenceTracker(t)
+
+	tracker.UserConnected("alice", "192.168.1.1")
+
+	users, err := tracker.Online(context.Background())
+	require.NoError(t, err)
+	require.Len(t, users, 1)
+	assert.Equal(t, OnlineUser{Login: "alice", Nickname: "", IP: "192.168.1.1"}, users[0])
+}
+
+func TestRedisPresenceTracker_Clear(t *testing.T) {
+	tracker, s := newTestPresenceTracker(t)
+
+	tracker.UserRenamed("alice", "", "Alice", "192.168.1.1")
+	require.NoError(t, tracker.Clear(context.Background()))
+
+	assert.False(t, s.Exists(redisKeyOnline))
+}