Selaa lähdekoodia

webapi: rework session lifecycle, add buddy aliases, normalize aimIds

Refactor Web API session setup and management, add private buddy-alias
support, and make aimId/displayId handling consistent across every event.

Session lifecycle
- Inject the OSCAR session callbacks (config, init, instance-close) into
  SessionHandler as FnSessCfg/FnSessInit/FnInstanceClose, built in
  NewServer, instead of constructing them inline in StartSession. This
  removes a pile of OSCAR plumbing (BuddyBroadcaster, BuddyListRegistry,
  ChatSessionManager, RecalcWarning, LowerWarnLevel, OSCARSessionManager)
  from the handler's dependencies.
- Require an auth token: a Web API session must bridge an authenticated
  OSCAR session, so anonymous/guest sessions are no longer created.
- EndSession now routes through RemoveSession, which evicts the session
  and tears it down (event queue + OSCAR instance) in one step, instead
  of only closing the OSCAR instance and leaving the aimsid resolvable
  until the reaper swept it.

Idle session reaping
- Replace the always-on 1-minute cleanup goroutine started in the
  constructor with a Run(ctx) reaper driven by the server's errgroup and
  stopped via the shutdown context. NewWebAPISessionManager no longer
  starts goroutines.
- TTL-based expiry (150s) sized to absorb one missed long-poll cycle;
  reaper sweeps every 30s. Touch/keepalive slides expiry on each request.
- Make Shutdown idempotent (closed flag) and refuse CreateSession after
  shutdown so a late session can never leak unreaped. Tear sessions down
  outside the manager lock, since CloseInstance fans out to broadcasts
  and signout.
- Drop the byUser index and GetSessionByUser: each startSession now owns
  its own aimsid and lifecycle, so nothing evicts a prior session.

Buddy aliases
- Add alias.go (FeedbagAliases/LookupBuddyAliases) to resolve the
  viewer's private buddy aliases from their feedbag.
- Cache the alias map on the session via BuddyAliasLoader, loaded once
  and reused until a feedbag change invalidates it, so signon costs one
  feedbag query instead of one per buddy. Every handler that writes the
  owner's feedbag calls InvalidateAliases, since the feedbag service
  relays a session's own writes only to its other instances.
- Carry the alias in a new Friendly field on presence, buddy-list, and
  IM/sent-IM events. The client's user-object merge drops any alias it
  holds, so friendly is repeated on every event that names a buddy.

aimId / displayId normalization
- Key users and conversations by the normalized aimId everywhere, and
  carry the owner's formatting in displayId (sourced from the locate
  reply, which reflects how the user signed on). Omit an empty displayId
  so the client keeps the name it already has rather than overwriting it
  with a normalized form.
- Store IMs under the normalized aimId on both the send and receive
  paths so sent/received history lines up under one key.

Messaging / presence
- Deliver IMs to a web recipient through the OSCAR relay
  (handleIncomingIM) instead of pushing directly to their session,
  removing the double-delivery path.
- Drop the redundant web-to-web broadcastPresenceEvent from
  SetState/SetStatus; the OSCAR-level BroadcastBuddyArrived/Departed
  already reaches other users and honors blocking and invisibility.

Middleware
- RequireSession returns 500 for a nil OSCARSession (a broken server
  invariant now that guests are gone) and keeps 401 for a missing or
  expired session; document the keepalive touch.

Add tests for reaping, shutdown, aliases, presence, messaging, and the
IM-log keying.
Mike 1 viikko sitten
vanhempi
commit
1c2943ba8d

+ 44 - 0
server/webapi/handlers/alias.go

@@ -0,0 +1,44 @@
+package handlers
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/mk6i/open-oscar-server/state"
+	"github.com/mk6i/open-oscar-server/wire"
+)
+
+// FeedbagAliases collects the aliases the feedbag owner has assigned to their
+// buddies, keyed by normalized screen name. Buddies without an alias are absent.
+func FeedbagAliases(items []wire.FeedbagItem) map[string]string {
+	aliases := make(map[string]string)
+	for _, item := range items {
+		if item.ClassID != wire.FeedbagClassIdBuddy || item.Name == "" {
+			continue
+		}
+		alias, ok := item.String(wire.FeedbagAttributesAlias)
+		if !ok || alias == "" {
+			continue
+		}
+		aliases[state.NewIdentScreenName(item.Name).String()] = alias
+	}
+	return aliases
+}
+
+// LookupBuddyAliases returns the aliases the session owner has assigned to their
+// buddies, keyed by normalized screen name.
+//
+// Aliases are private to the viewer and live only in their feedbag, so they cannot
+// be derived from a locate reply the way display names are.
+func LookupBuddyAliases(ctx context.Context, feedbagService FeedbagService, instance *state.SessionInstance) (map[string]string, error) {
+	frame := wire.SNACFrame{FoodGroup: wire.Feedbag, SubGroup: wire.FeedbagQuery}
+	snac, err := feedbagService.Query(ctx, instance, frame)
+	if err != nil {
+		return nil, err
+	}
+	reply, ok := snac.Body.(wire.SNAC_0x13_0x06_FeedbagReply)
+	if !ok {
+		return nil, fmt.Errorf("unexpected feedbag reply type")
+	}
+	return FeedbagAliases(reply.Items), nil
+}

+ 36 - 12
server/webapi/handlers/buddy_list_manager.go

@@ -42,7 +42,8 @@ type WebAPIBuddyGroup struct {
 type WebAPIBuddyInfo struct {
 	AimID        string   `json:"aimId"`
 	DisplayID    string   `json:"displayId"`
-	State        string   `json:"state"` // "online", "offline", "away", "idle"
+	Friendly     string   `json:"friendly,omitempty"` // Viewer's private alias, rendered in preference to DisplayID
+	State        string   `json:"state"`              // "online", "offline", "away", "idle"
 	StatusMsg    string   `json:"statusMsg,omitempty"`
 	AwayMsg      string   `json:"awayMsg,omitempty"`
 	OnlineTime   int64    `json:"onlineTime,omitempty"`
@@ -154,9 +155,10 @@ func (m *BuddyListManager) GetBuddyListForUser(ctx context.Context, sess *state.
 				continue
 			}
 			info := m.getBuddyInfo(ctx, sess.OSCARSession, b.name)
-			if b.alias != "" {
-				info.DisplayID = b.alias
-			}
+			// The alias belongs in friendly, not displayId: the client renders
+			// friendly in preference to displayId but still shows displayId as
+			// the buddy's actual screen name.
+			info.Friendly = b.alias
 			wg.Buddies = append(wg.Buddies, info)
 		}
 		out = append(out, wg)
@@ -168,9 +170,16 @@ func (m *BuddyListManager) GetBuddyListForUser(ctx context.Context, sess *state.
 // getBuddyInfo retrieves a buddy's current presence by issuing a locate
 // UserInfoQuery on behalf of the requesting session's OSCAR instance.
 func (m *BuddyListManager) getBuddyInfo(ctx context.Context, instance *state.SessionInstance, buddyName string) WebAPIBuddyInfo {
-	// Default to offline
+	// Default to offline. The web client keys users by the normalized aimId and
+	// shallow-merges each buddy map onto the shared user object, so a display-form
+	// aimId here overwrites the id every other event is keyed by.
+	//
+	// Feedbag buddy names are stored normalized, so they are not a source of
+	// display names. DisplayID is filled in from the locate reply below when the
+	// buddy is online, or overridden by the caller's alias when one is set.
+	ident := state.NewIdentScreenName(buddyName)
 	info := WebAPIBuddyInfo{
-		AimID:     buddyName,
+		AimID:     ident.String(),
 		DisplayID: buddyName,
 		State:     "offline",
 		UserType:  "aim",
@@ -178,15 +187,10 @@ func (m *BuddyListManager) getBuddyInfo(ctx context.Context, instance *state.Ses
 		Service:   "AIM",
 	}
 
-	// Web-only sessions have no OSCAR instance to query on behalf of.
-	if instance == nil {
-		return info
-	}
-
 	reply, err := m.locateService.UserInfoQuery(ctx, instance, wire.SNACFrame{},
 		wire.SNAC_0x02_0x05_LocateUserInfoQuery{
 			Type:       uint16(wire.LocateTypeUnavailable), // away message
-			ScreenName: buddyName,
+			ScreenName: ident.String(),
 		})
 	if err != nil {
 		m.logger.WarnContext(ctx, "failed to query buddy info", "screenName", buddyName, "error", err)
@@ -202,6 +206,11 @@ func (m *BuddyListManager) getBuddyInfo(ctx context.Context, instance *state.Ses
 	info.State = "online"
 	info.Capabilities = []string{}
 
+	// The locate reply carries the screen name as the buddy formatted it.
+	if userInfo.ScreenName != "" {
+		info.DisplayID = userInfo.ScreenName
+	}
+
 	if tod, ok := userInfo.Uint32BE(wire.OServiceUserInfoSignonTOD); ok {
 		info.OnlineTime = int64(tod)
 	}
@@ -225,6 +234,11 @@ func (m *BuddyListManager) getBuddyInfo(ctx context.Context, instance *state.Ses
 
 // RemoveBuddyFromFeedbag removes a buddy from a group (or all groups if allGroups is true) using feedbag delete/update SNACs.
 func (m *BuddyListManager) RemoveBuddyFromFeedbag(ctx context.Context, sess *state.WebAPISession, buddyName, groupName string, allGroups bool) (resultCode string, err error) {
+	// Buddy items carry the owner's alias for the buddy, and the feedbag service
+	// relays a session's own writes only to the owner's other instances, so every
+	// method here that rewrites buddy items has to drop the alias cache itself.
+	defer sess.InvalidateAliases()
+
 	buddyName = strings.TrimSpace(buddyName)
 	if buddyName == "" {
 		return "error", fmt.Errorf("empty buddy")
@@ -279,6 +293,8 @@ func (m *BuddyListManager) RemoveBuddyFromFeedbag(ctx context.Context, sess *sta
 
 // RemoveGroupFromFeedbag deletes a buddy group and updates the root order (TOC DelGroup).
 func (m *BuddyListManager) RemoveGroupFromFeedbag(ctx context.Context, sess *state.WebAPISession, requestedGroup string) (resultCode string, err error) {
+	defer sess.InvalidateAliases()
+
 	req := strings.TrimSpace(requestedGroup)
 	if req == "" {
 		return "error", fmt.Errorf("empty group")
@@ -326,6 +342,8 @@ func (m *BuddyListManager) RemoveGroupFromFeedbag(ctx context.Context, sess *sta
 
 // RenameGroupInFeedbag renames a buddy group, updating the group item in place.
 func (m *BuddyListManager) RenameGroupInFeedbag(ctx context.Context, sess *state.WebAPISession, oldGroup, newGroup string) (resultCode string, err error) {
+	defer sess.InvalidateAliases()
+
 	oldGroup = strings.TrimSpace(oldGroup)
 	newGroup = strings.TrimSpace(newGroup)
 	if oldGroup == "" || newGroup == "" {
@@ -374,6 +392,8 @@ func (m *BuddyListManager) RenameGroupInFeedbag(ctx context.Context, sess *state
 // MoveBuddyInFeedbag moves a buddy to a different group and/or repositions it
 // within a group's order.
 func (m *BuddyListManager) MoveBuddyInFeedbag(ctx context.Context, sess *state.WebAPISession, buddyName, fromGroup, toGroup, beforeBuddy string) (resultCode string, err error) {
+	defer sess.InvalidateAliases()
+
 	buddyName = strings.TrimSpace(buddyName)
 	fromGroup = strings.TrimSpace(fromGroup)
 	toGroup = strings.TrimSpace(toGroup)
@@ -453,6 +473,8 @@ func (m *BuddyListManager) MoveBuddyInFeedbag(ctx context.Context, sess *state.W
 // SetBuddyAttributeInFeedbag sets a buddy's friendly (alias) name across all
 // groups it belongs to. An empty friendly clears the alias.
 func (m *BuddyListManager) SetBuddyAttributeInFeedbag(ctx context.Context, sess *state.WebAPISession, buddyName, friendly string) (resultCode string, err error) {
+	defer sess.InvalidateAliases()
+
 	buddyName = strings.TrimSpace(buddyName)
 	if buddyName == "" {
 		return "error", fmt.Errorf("empty buddy")
@@ -492,6 +514,8 @@ func (m *BuddyListManager) SetBuddyAttributeInFeedbag(ctx context.Context, sess
 // SetGroupAttributeInFeedbag sets a group's collapsed state. An empty group
 // targets the unnamed default group.
 func (m *BuddyListManager) SetGroupAttributeInFeedbag(ctx context.Context, sess *state.WebAPISession, groupName string, collapsed bool) (resultCode string, err error) {
+	defer sess.InvalidateAliases()
+
 	groupName = strings.TrimSpace(groupName)
 	frame := wire.SNACFrame{FoodGroup: wire.Feedbag, SubGroup: wire.FeedbagQuery}
 	snac, err := m.feedbagService.Query(ctx, sess.OSCARSession, frame)

+ 135 - 8
server/webapi/handlers/buddy_list_manager_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 
 	"github.com/mk6i/open-oscar-server/state"
 	"github.com/mk6i/open-oscar-server/wire"
@@ -24,6 +25,13 @@ func offlineWebAPIBuddy(aimID, displayID string) WebAPIBuddyInfo {
 	}
 }
 
+// withAlias sets the viewer's private name for a buddy. It travels in friendly, not
+// displayId, which keeps carrying the buddy's own screen name.
+func withAlias(b WebAPIBuddyInfo, alias string) WebAPIBuddyInfo {
+	b.Friendly = alias
+	return b
+}
+
 func TestBuddyListManager_GetBuddyListForUser(t *testing.T) {
 	ctx := context.Background()
 	owner := state.NewIdentScreenName("listowner")
@@ -109,10 +117,30 @@ func TestBuddyListManager_GetBuddyListForUser(t *testing.T) {
 					TLVLBlock: wire.TLVLBlock{TLVList: wire.TLVList{wire.NewTLVBE(wire.FeedbagAttributesAlias, "Bob Smith")}},
 				},
 			},
+			want: []WebAPIBuddyGroup{
+				{
+					Name: "Buddies",
+					// The buddy is offline, so no locate reply supplies a display
+					// name and displayId falls back to the normalized feedbag name.
+					Buddies: []WebAPIBuddyInfo{withAlias(offlineWebAPIBuddy("bob", "bob"), "Bob Smith")},
+				},
+			},
+		},
+		{
+			name: "unnormalized feedbag buddy name still yields a normalized aimId",
+			fb: []wire.FeedbagItem{
+				{
+					Name: "", GroupID: 0, ItemID: 0, ClassID: wire.FeedbagClassIdGroup,
+					TLVLBlock: wire.TLVLBlock{TLVList: wire.TLVList{wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{100})}},
+				},
+				{Name: "Buddies", GroupID: 100, ItemID: 0, ClassID: wire.FeedbagClassIdGroup,
+					TLVLBlock: wire.TLVLBlock{TLVList: wire.TLVList{wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{1})}}},
+				{ItemID: 1, ClassID: wire.FeedbagClassIdBuddy, GroupID: 100, Name: "Mike Kelly"},
+			},
 			want: []WebAPIBuddyGroup{
 				{
 					Name:    "Buddies",
-					Buddies: []WebAPIBuddyInfo{offlineWebAPIBuddy("bob", "Bob Smith")},
+					Buddies: []WebAPIBuddyInfo{offlineWebAPIBuddy("mikekelly", "Mike Kelly")},
 				},
 			},
 		},
@@ -178,8 +206,8 @@ func TestBuddyListManager_GetBuddyListForUser(t *testing.T) {
 				{
 					Name: "Buddies",
 					Buddies: []WebAPIBuddyInfo{
-						offlineWebAPIBuddy("secondInSlice", "secondInSlice"),
-						offlineWebAPIBuddy("firstInSlice", "firstInSlice"),
+						offlineWebAPIBuddy("secondinslice", "secondInSlice"),
+						offlineWebAPIBuddy("firstinslice", "firstInSlice"),
 					},
 				},
 			},
@@ -201,11 +229,11 @@ func TestBuddyListManager_GetBuddyListForUser(t *testing.T) {
 			want: []WebAPIBuddyGroup{
 				{
 					Name:    "Family",
-					Buddies: []WebAPIBuddyInfo{offlineWebAPIBuddy("inFamily", "inFamily")},
+					Buddies: []WebAPIBuddyInfo{offlineWebAPIBuddy("infamily", "inFamily")},
 				},
 				{
 					Name:    "Buddies",
-					Buddies: []WebAPIBuddyInfo{offlineWebAPIBuddy("inBuddies", "inBuddies")},
+					Buddies: []WebAPIBuddyInfo{offlineWebAPIBuddy("inbuddies", "inBuddies")},
 				},
 			},
 		},
@@ -232,9 +260,11 @@ func TestBuddyListManager_GetBuddyListForUser(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			fs := &MockFeedbagService{}
-			// The test session has no OSCAR instance, so buddies resolve to
-			// offline without any locate query being issued.
+			// The locate query returns an error, so every buddy resolves to
+			// offline. This keeps the focus on feedbag -> group conversion.
 			ls := &MockLocateService{}
+			ls.On("UserInfoQuery", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+				Return(wire.SNACMessage{}, errors.New("offline")).Maybe()
 			if tt.fbErr != nil {
 				fs.On("Query", mock.Anything, mock.Anything, mock.Anything).Return(wire.SNACMessage{}, tt.fbErr).Once()
 			} else {
@@ -244,7 +274,10 @@ func TestBuddyListManager_GetBuddyListForUser(t *testing.T) {
 			}
 
 			m := NewBuddyListManager(fs, ls, slog.Default())
-			sess := &state.WebAPISession{ScreenName: state.DisplayScreenName(owner.String())}
+			sess := &state.WebAPISession{
+				ScreenName:   state.DisplayScreenName(owner.String()),
+				OSCARSession: state.NewSession().AddInstance(),
+			}
 			got, err := m.GetBuddyListForUser(ctx, sess)
 
 			if tt.wantErr != "" {
@@ -260,3 +293,97 @@ func TestBuddyListManager_GetBuddyListForUser(t *testing.T) {
 		})
 	}
 }
+
+func TestBuddyListManager_GetBuddyListForUser_DisplayIDFromLocateReply(t *testing.T) {
+	// Feedbag buddy names are stored normalized, so an online buddy's display
+	// name can only come from the locate reply's user info.
+	ctx := context.Background()
+
+	fb := []wire.FeedbagItem{
+		{
+			Name: "", GroupID: 0, ItemID: 0, ClassID: wire.FeedbagClassIdGroup,
+			TLVLBlock: wire.TLVLBlock{TLVList: wire.TLVList{wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{100})}},
+		},
+		{Name: "Buddies", GroupID: 100, ItemID: 0, ClassID: wire.FeedbagClassIdGroup,
+			TLVLBlock: wire.TLVLBlock{TLVList: wire.TLVList{wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{1})}}},
+		{ItemID: 1, ClassID: wire.FeedbagClassIdBuddy, GroupID: 100, Name: "mikekelly"},
+	}
+
+	fs := &MockFeedbagService{}
+	fs.On("Query", mock.Anything, mock.Anything, mock.Anything).Return(
+		wire.SNACMessage{Body: wire.SNAC_0x13_0x06_FeedbagReply{Items: fb}}, nil,
+	).Once()
+
+	ls := &MockLocateService{}
+	ls.On("UserInfoQuery", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(
+		wire.SNACMessage{Body: wire.SNAC_0x02_0x06_LocateUserInfoReply{
+			TLVUserInfo: wire.TLVUserInfo{ScreenName: "Mike Kelly"},
+		}}, nil,
+	).Once()
+
+	m := NewBuddyListManager(fs, ls, slog.Default())
+	sess := &state.WebAPISession{
+		ScreenName:   state.DisplayScreenName("listowner"),
+		OSCARSession: state.NewSession().AddInstance(),
+	}
+	got, err := m.GetBuddyListForUser(ctx, sess)
+	require.NoError(t, err)
+	require.Len(t, got, 1)
+	require.Len(t, got[0].Buddies, 1)
+
+	assert.Equal(t, "mikekelly", got[0].Buddies[0].AimID)
+	assert.Equal(t, "Mike Kelly", got[0].Buddies[0].DisplayID)
+	assert.Equal(t, "online", got[0].Buddies[0].State)
+
+	fs.AssertExpectations(t)
+	ls.AssertExpectations(t)
+}
+
+// The feedbag service relays a session's own writes only to the owner's other
+// instances, so renaming a buddy from the web client produces no SNAC for that
+// session. Without an explicit invalidation, its cached aliases would keep serving
+// the old name and the next presence or IM event would rename the buddy back.
+func TestBuddyListManager_SetBuddyAttributeInFeedbag_InvalidatesAliasCache(t *testing.T) {
+	ctx := context.Background()
+
+	feedbag := func(alias string) []wire.FeedbagItem {
+		buddy := wire.FeedbagItem{ItemID: 1, ClassID: wire.FeedbagClassIdBuddy, GroupID: 100, Name: "mikekelly"}
+		buddy.TLVLBlock = wire.TLVLBlock{TLVList: wire.TLVList{wire.NewTLVBE(wire.FeedbagAttributesAlias, alias)}}
+		return []wire.FeedbagItem{
+			{Name: "", GroupID: 0, ItemID: 0, ClassID: wire.FeedbagClassIdGroup,
+				TLVLBlock: wire.TLVLBlock{TLVList: wire.TLVList{wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{100})}}},
+			{Name: "Buddies", GroupID: 100, ItemID: 0, ClassID: wire.FeedbagClassIdGroup,
+				TLVLBlock: wire.TLVLBlock{TLVList: wire.TLVList{wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{1})}}},
+			buddy,
+		}
+	}
+
+	fs := &MockFeedbagService{}
+	// Query 1: the alias cache loads. Query 2: SetBuddyAttributeInFeedbag reads the
+	// feedbag it is about to rewrite. Query 3: the cache reloads post-invalidation,
+	// now seeing the stored rename.
+	fs.On("Query", mock.Anything, mock.Anything, mock.Anything).
+		Return(wire.SNACMessage{Body: wire.SNAC_0x13_0x06_FeedbagReply{Items: feedbag("MICHAELKELLY")}}, nil).Twice()
+	fs.On("UpsertItem", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+		Return(&wire.SNACMessage{}, nil).Once()
+	fs.On("Query", mock.Anything, mock.Anything, mock.Anything).
+		Return(wire.SNACMessage{Body: wire.SNAC_0x13_0x06_FeedbagReply{Items: feedbag("MIKE")}}, nil).Once()
+
+	m := NewBuddyListManager(fs, &MockLocateService{}, slog.Default())
+	sess := &state.WebAPISession{
+		ScreenName:   state.DisplayScreenName("listowner"),
+		OSCARSession: state.NewSession().AddInstance(),
+	}
+	sess.BuddyAliasLoader = func(ctx context.Context) (map[string]string, error) {
+		return LookupBuddyAliases(ctx, fs, sess.OSCARSession)
+	}
+
+	require.Equal(t, "MICHAELKELLY", sess.Aliases(ctx)["mikekelly"])
+
+	resultCode, err := m.SetBuddyAttributeInFeedbag(ctx, sess, "mikekelly", "MIKE")
+	require.NoError(t, err)
+	require.Equal(t, "success", resultCode)
+
+	assert.Equal(t, "MIKE", sess.Aliases(ctx)["mikekelly"])
+	fs.AssertExpectations(t)
+}

+ 10 - 3
server/webapi/handlers/buddylist.go

@@ -122,6 +122,10 @@ func (h *BuddyListHandler) AddGroup(w http.ResponseWriter, r *http.Request, sess
 }
 
 func (h *BuddyListHandler) addGroupToFeedbag(ctx context.Context, sess *state.WebAPISession, groupName string) string {
+	// A session sees no SNAC for its own feedbag writes, so it drops the alias
+	// cache itself. See WebAPISession.InvalidateAliases.
+	defer sess.InvalidateAliases()
+
 	frame := wire.SNACFrame{FoodGroup: wire.Feedbag, SubGroup: wire.FeedbagQuery}
 	snac, err := h.FeedbagService.Query(ctx, sess.OSCARSession, frame)
 	if err != nil {
@@ -273,6 +277,8 @@ func (h *BuddyListHandler) RemoveGroup(w http.ResponseWriter, r *http.Request, s
 
 // addBuddyToFeedbag adds a buddy to the user's feedbag.
 func (h *BuddyListHandler) addBuddyToFeedbag(ctx context.Context, sess *state.WebAPISession, buddyName, groupName string) (string, *BuddyPresenceInfo) {
+	defer sess.InvalidateAliases()
+
 	// Retrieve current feedbag
 	frame := wire.SNACFrame{FoodGroup: wire.Feedbag, SubGroup: wire.FeedbagQuery}
 	snac, err := h.FeedbagService.Query(ctx, sess.OSCARSession, frame)
@@ -340,9 +346,10 @@ func (h *BuddyListHandler) addBuddyToFeedbag(ctx context.Context, sess *state.We
 
 	// Get current presence for the buddy
 	buddyInfo := &BuddyPresenceInfo{
-		AimID:    buddyName,
-		State:    "offline", // Default to offline
-		UserType: "aim",
+		AimID:     state.NewIdentScreenName(buddyName).String(),
+		DisplayID: buddyName,
+		State:     "offline", // Default to offline
+		UserType:  "aim",
 	}
 
 	// TODO: Check actual presence status and update buddyInfo accordingly

+ 2 - 2
server/webapi/handlers/buddylist_test.go

@@ -294,7 +294,7 @@ func TestBuddyListHandler_AddBuddy(t *testing.T) {
 				return session
 			},
 			expectedStatusCode: http.StatusOK,
-			expectedResponse:   `{"response":{"statusCode":200,"statusText":"OK","data":{"buddyInfo":{"aimId":"newbuddy","state":"offline","userType":"aim"},"resultCode":"success"}}}`,
+			expectedResponse:   `{"response":{"statusCode":200,"statusText":"OK","data":{"buddyInfo":{"aimId":"newbuddy","displayId":"newbuddy","state":"offline","userType":"aim"},"resultCode":"success"}}}`,
 		},
 		{
 			name: "Success_EventPushSkippedOnBLMError",
@@ -324,7 +324,7 @@ func TestBuddyListHandler_AddBuddy(t *testing.T) {
 				return session
 			},
 			expectedStatusCode: http.StatusOK,
-			expectedResponse:   `{"response":{"statusCode":200,"statusText":"OK","data":{"buddyInfo":{"aimId":"newbuddy","state":"offline","userType":"aim"},"resultCode":"success"}}}`,
+			expectedResponse:   `{"response":{"statusCode":200,"statusText":"OK","data":{"buddyInfo":{"aimId":"newbuddy","displayId":"newbuddy","state":"offline","userType":"aim"},"resultCode":"success"}}}`,
 		},
 		{
 			name: "Error_BuddyAlreadyExists",

+ 52 - 41
server/webapi/handlers/messaging.go

@@ -24,6 +24,8 @@ type ICBMService interface {
 type MessagingHandler struct {
 	SessionManager *state.WebAPISessionManager
 	ICBMService    ICBMService
+	LocateService  LocateService
+	FeedbagService FeedbagService
 	Logger         *slog.Logger
 }
 
@@ -61,11 +63,6 @@ func (h *MessagingHandler) SendIM(w http.ResponseWriter, r *http.Request, sess *
 
 	// Parse optional parameters
 	autoResponse := queryOrFormParam(r, "autoResponse") == "1"
-	//offlineIMParam := queryOrFormParam(r, "offlineIM") what is this for
-	//offlineIM := offlineIMParam != "0" && offlineIMParam != "false" // default to true
-
-	// Create recipient identifier
-	recipientIdent := state.NewIdentScreenName(recipient)
 
 	// Generate message cookie
 	var cookie [8]byte
@@ -87,8 +84,10 @@ func (h *MessagingHandler) SendIM(w http.ResponseWriter, r *http.Request, sess *
 
 	now := float64(time.Now().Unix())
 	nowSec := time.Now().Unix()
-	sn := sess.ScreenName.String()
-	sess.AddStoredIM(recipient, sn, message, messageID, nowSec)
+	// The client sends t as the normalized aimId it keys the conversation by, so
+	// it is never a source of display names.
+	recipientIdent := state.NewIdentScreenName(recipient)
+	sess.AddStoredIM(recipientIdent.String(), sess.ScreenName.IdentScreenName().String(), message, messageID, nowSec)
 
 	// Recipient is online, deliver message
 	clientIM := wire.SNAC_0x04_0x06_ICBMChannelMsgToHost{
@@ -149,30 +148,11 @@ func (h *MessagingHandler) SendIM(w http.ResponseWriter, r *http.Request, sess *
 		}
 	}
 
-	// Queue IM event for the recipient's WebAPI session if they have one
-	if recipientWebSession, err := h.SessionManager.GetSessionByUser(r.Context(), recipientIdent); err == nil && recipientWebSession != nil {
-		recipientWebSession.AddStoredIM(sn, sn, message, messageID, nowSec)
-		eventData := types.IMEvent{
-			Source: types.UserInfo{
-				AimID:     sn,
-				DisplayID: sn,
-				UserType:  "aim",
-				State:     "online",
-			},
-			Message:   message,
-			MsgID:     messageID,
-			Timestamp: now,
-			AutoResp:  autoResponse,
-		}
-		recipientWebSession.EventQueue.Push(types.EventTypeIM, eventData)
-		if recipientWebSession.IsSubscribedTo("conversation") {
-			recipientWebSession.EventQueue.Push(types.EventTypeConversation, types.ConversationEventData("update", []map[string]interface{}{
-				types.ConversationEntry(sn, sn, message, messageID, sn, false, 1),
-			}))
-		}
-	}
-
-	h.pushSenderWebAPIEvents(sess, sn, recipient, message, messageID, now, autoResponse)
+	recipientDisplay := h.resolveDisplayName(ctx, sess.OSCARSession, recipientIdent)
+	// The alias lives in the sender's feedbag, so unlike the display name it cannot
+	// be read off a locate reply.
+	recipientAlias := sess.Aliases(ctx)[recipientIdent.String()]
+	h.pushSenderWebAPIEvents(sess, recipientIdent, recipientDisplay, recipientAlias, message, messageID, now, autoResponse)
 
 	h.Logger.DebugContext(ctx, "queued sentIM event for sender",
 		"from", sess.ScreenName.String(),
@@ -180,10 +160,6 @@ func (h *MessagingHandler) SendIM(w http.ResponseWriter, r *http.Request, sess *
 		"eventType", types.EventTypeSentIM,
 	)
 
-	h.Logger.DebugContext(ctx, "delivered instant message",
-		"from", sess.ScreenName.String(),
-		"to", recipient)
-
 	// Send success response
 	responseData := map[string]interface{}{
 		"msgId": messageID,
@@ -196,16 +172,51 @@ func (h *MessagingHandler) SendIM(w http.ResponseWriter, r *http.Request, sess *
 	SendResponse(w, r, response, h.Logger)
 }
 
-func (h *MessagingHandler) pushSenderWebAPIEvents(sess *state.WebAPISession, sender, recipient, message, messageID string, now float64, autoResponse bool) {
+// resolveDisplayName returns the recipient's screen name as they formatted it,
+// or "" when it cannot be determined because they are offline or blocked.
+func (h *MessagingHandler) resolveDisplayName(ctx context.Context, instance *state.SessionInstance, recipient state.IdentScreenName) string {
+	reply, err := h.LocateService.UserInfoQuery(ctx, instance, wire.SNACFrame{},
+		wire.SNAC_0x02_0x05_LocateUserInfoQuery{
+			Type:       uint16(wire.LocateTypeUnavailable),
+			ScreenName: recipient.String(),
+		})
+	if err != nil {
+		h.Logger.DebugContext(ctx, "failed to resolve recipient display name",
+			"screenName", recipient.String(), "error", err)
+		return ""
+	}
+	info, ok := reply.Body.(wire.SNAC_0x02_0x06_LocateUserInfoReply)
+	if !ok {
+		return ""
+	}
+	return info.ScreenName
+}
+
+// pushSenderWebAPIEvents echoes a just-sent IM back to the sender's own event
+// queue. recipientDisplay is the recipient's own formatting of their screen name,
+// or "" when it could not be resolved; recipientAlias is the sender's private name
+// for them, or "" when unaliased.
+//
+// The web client merges every user map it receives onto the single user object it
+// keys by aimId, so a displayId here overwrites the name the buddy list already
+// rendered. Echoing the normalized aimId as a displayId would reduce a buddy named
+// "Mike Lee" to "mikelee" the moment you message him. Omitting displayId leaves the
+// client's existing name untouched. The merge also deletes any alias it holds, so
+// friendly has to be repeated here even though the buddy list already sent it.
+func (h *MessagingHandler) pushSenderWebAPIEvents(sess *state.WebAPISession, recipient state.IdentScreenName, recipientDisplay, recipientAlias, message, messageID string, now float64, autoResponse bool) {
+	senderAimID := sess.ScreenName.IdentScreenName().String()
+	recipientAimID := recipient.String()
+
 	senderEventData := types.SentIMEvent{
 		Sender: types.UserInfo{
-			AimID:     sender,
-			DisplayID: sender,
+			AimID:     senderAimID,
+			DisplayID: sess.ScreenName.String(),
 			UserType:  "aim",
 		},
 		Dest: types.UserInfo{
-			AimID:     recipient,
-			DisplayID: recipient,
+			AimID:     recipientAimID,
+			DisplayID: recipientDisplay,
+			Friendly:  recipientAlias,
 			UserType:  "aim",
 		},
 		Message:   message,
@@ -216,7 +227,7 @@ func (h *MessagingHandler) pushSenderWebAPIEvents(sess *state.WebAPISession, sen
 	sess.EventQueue.Push(types.EventTypeSentIM, senderEventData)
 	if sess.IsSubscribedTo("conversation") {
 		sess.EventQueue.Push(types.EventTypeConversation, types.ConversationEventData("update", []map[string]interface{}{
-			types.ConversationEntry(recipient, recipient, message, messageID, sender, true, 0),
+			types.ConversationEntry(recipientAimID, recipientDisplay, message, messageID, senderAimID, true, 0),
 		}))
 	}
 }

+ 149 - 0
server/webapi/handlers/messaging_test.go

@@ -2,6 +2,8 @@ package handlers
 
 import (
 	"context"
+	"encoding/json"
+	"io"
 	"log/slog"
 	"net/http"
 	"net/http/httptest"
@@ -11,8 +13,10 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 
 	"github.com/mk6i/open-oscar-server/server/webapi/middleware"
+	"github.com/mk6i/open-oscar-server/server/webapi/types"
 	"github.com/mk6i/open-oscar-server/state"
 	"github.com/mk6i/open-oscar-server/wire"
 )
@@ -59,6 +63,145 @@ func createTestSessionManagerWithOSCAR(screenName string, oscarSession *state.Se
 	return mgr, session.AimSID
 }
 
+// stubLocateService answers UserInfoQuery with a reply carrying screenName, or
+// with an error when screenName is empty (i.e. the target is offline or blocked).
+func stubLocateService(screenName string) *MockLocateService {
+	ls := &MockLocateService{}
+	call := ls.On("UserInfoQuery", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+	if screenName == "" {
+		call.Return(wire.SNACMessage{}, io.EOF)
+	} else {
+		call.Return(wire.SNACMessage{Body: wire.SNAC_0x02_0x06_LocateUserInfoReply{
+			TLVUserInfo: wire.TLVUserInfo{ScreenName: screenName},
+		}}, nil)
+	}
+	return ls
+}
+
+// stubFeedbagService answers Query with a single buddy item for buddy, carrying
+// alias when one is given.
+func stubFeedbagService(buddy, alias string) *MockFeedbagService {
+	item := wire.FeedbagItem{ItemID: 1, ClassID: wire.FeedbagClassIdBuddy, GroupID: 100, Name: buddy}
+	if alias != "" {
+		item.TLVLBlock = wire.TLVLBlock{TLVList: wire.TLVList{wire.NewTLVBE(wire.FeedbagAttributesAlias, alias)}}
+	}
+	fs := &MockFeedbagService{}
+	fs.On("Query", mock.Anything, mock.Anything, mock.Anything).Return(
+		wire.SNACMessage{Body: wire.SNAC_0x13_0x06_FeedbagReply{Items: []wire.FeedbagItem{item}}}, nil,
+	)
+	return fs
+}
+
+// sendIMForDest drives SendIM addressed to t, with the recipient's display name
+// resolving to locateName and the sender's alias for them set to alias, and returns
+// the events queued for the sender.
+func sendIMForDest(t *testing.T, dest, locateName, alias string) []types.Event {
+	t.Helper()
+
+	oscarInstance := state.NewSession().AddInstance()
+	icbmService := &MockICBMService{}
+	icbmService.On("ChannelMsgToHost", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+		Return(nil, nil)
+
+	mgr := state.NewWebAPISessionManager()
+	session, err := mgr.CreateSession(context.Background(), state.DisplayScreenName("Ann Dupree"),
+		"test-dev", []string{"im", "sentIM", "conversation"}, oscarInstance, slog.Default())
+	require.NoError(t, err)
+
+	handler := &MessagingHandler{
+		SessionManager: mgr,
+		ICBMService:    icbmService,
+		LocateService:  stubLocateService(locateName),
+		FeedbagService: stubFeedbagService(dest, alias),
+		Logger:         slog.Default(),
+	}
+
+	// startSession wires this in production; SendIM reads aliases off the session.
+	session.BuddyAliasLoader = func(ctx context.Context) (map[string]string, error) {
+		return LookupBuddyAliases(ctx, handler.FeedbagService, session.OSCARSession)
+	}
+
+	req, err := http.NewRequest("GET", "/im/sendIM?aimsid="+session.AimSID+"&t="+url.QueryEscape(dest)+"&message=hi", nil)
+	require.NoError(t, err)
+	rr := httptest.NewRecorder()
+	requireSession(mgr, handler.SendIM).ServeHTTP(rr, req)
+	require.Equal(t, http.StatusOK, rr.Code)
+
+	return session.EventQueue.GetAllEvents()
+}
+
+// The client sends t as the normalized aimId, so the recipient's display name has
+// to come from the locate reply. Echoing t back as a displayId would overwrite the
+// properly formatted name the client already holds for that aimId.
+func TestMessagingHandler_SendIM_DestDisplayIDFromLocateReply(t *testing.T) {
+	var sentIM types.SentIMEvent
+	var conv map[string]interface{}
+	for _, event := range sendIMForDest(t, "mikelee", "Mike Lee", "") {
+		switch event.Type {
+		case types.EventTypeSentIM:
+			sentIM, _ = event.Data.(types.SentIMEvent)
+		case types.EventTypeConversation:
+			data, _ := event.Data.(map[string]interface{})
+			convs, _ := data["conversations"].([]map[string]interface{})
+			require.Len(t, convs, 1)
+			conv = convs[0]
+		}
+	}
+
+	assert.Equal(t, "anndupree", sentIM.Sender.AimID)
+	assert.Equal(t, "Ann Dupree", sentIM.Sender.DisplayID)
+	assert.Equal(t, "mikelee", sentIM.Dest.AimID)
+	assert.Equal(t, "Mike Lee", sentIM.Dest.DisplayID)
+
+	require.NotNil(t, conv)
+	assert.Equal(t, "mikelee", conv["aimId"])
+	assert.Equal(t, "Mike Lee", conv["displayId"])
+}
+
+// An alias is private to the sender and lives only in their feedbag, and the client
+// deletes the alias it holds every time it merges a user map. So the sentIM echo has
+// to repeat it, or messaging an aliased buddy renames him back to his screen name.
+func TestMessagingHandler_SendIM_DestCarriesAlias(t *testing.T) {
+	var sentIM types.SentIMEvent
+	for _, event := range sendIMForDest(t, "mikelee", "Mike Lee", "MICHAELLEE") {
+		if event.Type == types.EventTypeSentIM {
+			sentIM, _ = event.Data.(types.SentIMEvent)
+		}
+	}
+
+	assert.Equal(t, "mikelee", sentIM.Dest.AimID)
+	assert.Equal(t, "Mike Lee", sentIM.Dest.DisplayID)
+	assert.Equal(t, "MICHAELLEE", sentIM.Dest.Friendly)
+}
+
+// When the recipient's display name cannot be resolved, displayId is omitted
+// rather than filled in with the aimId, leaving the client's existing name intact.
+func TestMessagingHandler_SendIM_OmitsDestDisplayIDWhenUnresolved(t *testing.T) {
+	var sentIM types.SentIMEvent
+	var conv map[string]interface{}
+	for _, event := range sendIMForDest(t, "mikelee", "", "") {
+		switch event.Type {
+		case types.EventTypeSentIM:
+			sentIM, _ = event.Data.(types.SentIMEvent)
+		case types.EventTypeConversation:
+			data, _ := event.Data.(map[string]interface{})
+			convs, _ := data["conversations"].([]map[string]interface{})
+			require.Len(t, convs, 1)
+			conv = convs[0]
+		}
+	}
+
+	assert.Equal(t, "mikelee", sentIM.Dest.AimID)
+	assert.Empty(t, sentIM.Dest.DisplayID)
+	encoded, err := json.Marshal(sentIM)
+	require.NoError(t, err)
+	assert.NotContains(t, string(encoded), "displayId\":\"mikelee\"")
+
+	require.NotNil(t, conv)
+	assert.Equal(t, "mikelee", conv["aimId"])
+	assert.NotContains(t, conv, "displayId")
+}
+
 func TestMessagingHandler_SendIM(t *testing.T) {
 	oscarInstance := state.NewSession().AddInstance()
 
@@ -112,6 +255,8 @@ func TestMessagingHandler_SendIM(t *testing.T) {
 			handler := &MessagingHandler{
 				SessionManager: sessionMgr,
 				ICBMService:    icbmService,
+				LocateService:  stubLocateService(""),
+				FeedbagService: stubFeedbagService("someone", ""),
 				Logger:         slog.Default(),
 			}
 
@@ -149,6 +294,8 @@ func TestMessagingHandler_SendIM_POST(t *testing.T) {
 	handler := &MessagingHandler{
 		SessionManager: sessionMgr,
 		ICBMService:    icbmService,
+		LocateService:  stubLocateService(""),
+		FeedbagService: stubFeedbagService("someone", ""),
 		Logger:         slog.Default(),
 	}
 
@@ -266,6 +413,8 @@ func TestMessagingHandler_SetTyping(t *testing.T) {
 			handler := &MessagingHandler{
 				SessionManager: sessionMgr,
 				ICBMService:    icbmService,
+				LocateService:  stubLocateService(""),
+				FeedbagService: stubFeedbagService("someone", ""),
 				Logger:         slog.Default(),
 			}
 

+ 37 - 46
server/webapi/handlers/presence.go

@@ -53,9 +53,15 @@ type BuddyGroupInfo struct {
 }
 
 // BuddyPresenceInfo represents presence information for a buddy.
+//
+// AimID is the normalized screen name the web client keys users by; DisplayID
+// preserves the casing and spacing the user signed on with. The client renders
+// DisplayID and falls back to AimID when it is absent.
 type BuddyPresenceInfo struct {
 	AimID      string `json:"aimId" xml:"aimId"`
-	State      string `json:"state" xml:"state"` // "online", "offline", "away", "idle"
+	DisplayID  string `json:"displayId,omitempty" xml:"displayId,omitempty"`
+	Friendly   string `json:"friendly,omitempty" xml:"friendly,omitempty"` // Viewer's private alias, rendered in preference to DisplayID
+	State      string `json:"state" xml:"state"`                           // "online", "offline", "away", "idle"
 	StatusMsg  string `json:"statusMsg,omitempty" xml:"statusMsg,omitempty"`
 	AwayMsg    string `json:"awayMsg,omitempty" xml:"awayMsg,omitempty"`
 	ProfileMsg string `json:"profileMsg,omitempty" xml:"profileMsg,omitempty"`
@@ -102,12 +108,18 @@ func (h *PresenceHandler) GetPresence(w http.ResponseWriter, r *http.Request, se
 		}
 		presenceList := make([]BuddyPresenceInfo, 0, len(users))
 
+		// The client's user-object merge deletes any alias it holds, so every
+		// presence payload has to carry friendly for aliased buddies.
+		aliases := session.Aliases(ctx)
+
 		for _, user := range users {
 			user = strings.TrimSpace(user)
 			if user == "" {
 				continue
 			}
-			presenceList = append(presenceList, h.getUserPresence(ctx, session.OSCARSession, state.NewIdentScreenName(user), wantProfileMsg))
+			info := h.getUserPresence(ctx, session.OSCARSession, state.DisplayScreenName(user), wantProfileMsg)
+			info.Friendly = aliases[info.AimID]
+			presenceList = append(presenceList, info)
 		}
 
 		presenceData.Users = presenceList
@@ -180,7 +192,7 @@ func (h *PresenceHandler) getBuddyListGroups(ctx context.Context, session *state
 
 		// UserInfoQuery performs the blocking check and online lookup; blocked or
 		// offline buddies come back as "offline", preserving the list structure.
-		presence := h.getUserPresence(ctx, session.OSCARSession, state.NewIdentScreenName(item.Name), wantProfileMsg)
+		presence := h.getUserPresence(ctx, session.OSCARSession, state.DisplayScreenName(item.Name), wantProfileMsg)
 		group.Buddies = append(group.Buddies, presence)
 	}
 
@@ -205,22 +217,26 @@ func (h *PresenceHandler) getBuddyListGroups(ctx context.Context, session *state
 // on behalf of the requesting OSCAR session (instance). UserInfoQuery performs
 // the OSCAR blocking check and online lookup internally: blocked and offline
 // users both come back as a locate error, which we surface as "offline".
-func (h *PresenceHandler) getUserPresence(ctx context.Context, instance *state.SessionInstance, target state.IdentScreenName, wantProfileMsg bool) BuddyPresenceInfo {
+func (h *PresenceHandler) getUserPresence(ctx context.Context, instance *state.SessionInstance, target state.DisplayScreenName, wantProfileMsg bool) BuddyPresenceInfo {
+	ident := target.IdentScreenName()
+
 	// Default offline presence
 	presence := BuddyPresenceInfo{
-		AimID:    target.String(),
-		State:    "offline",
-		UserType: "aim",
+		AimID:     ident.String(),
+		DisplayID: target.String(),
+		State:     "offline",
+		UserType:  "aim",
 	}
 
 	// Determine user type
-	if strings.HasPrefix(target.String(), "admin") {
+	if strings.HasPrefix(ident.String(), "admin") {
 		presence.UserType = "admin"
-	} else if isICQScreenName(target.String()) {
+	} else if isICQScreenName(ident.String()) {
 		presence.UserType = "icq"
 	}
 
-	// Web-only sessions have no OSCAR instance to query on behalf of.
+	// The unauthenticated icon endpoint resolves presence without a session, so
+	// there may be no OSCAR instance to query on behalf of.
 	if instance == nil {
 		return presence
 	}
@@ -231,9 +247,9 @@ func (h *PresenceHandler) getUserPresence(ctx context.Context, instance *state.S
 	}
 
 	reply, err := h.LocateService.UserInfoQuery(ctx, instance, wire.SNACFrame{},
-		wire.SNAC_0x02_0x05_LocateUserInfoQuery{Type: uint16(reqType), ScreenName: target.String()})
+		wire.SNAC_0x02_0x05_LocateUserInfoQuery{Type: uint16(reqType), ScreenName: ident.String()})
 	if err != nil {
-		h.Logger.WarnContext(ctx, "failed to query user info", "screenName", target.String(), "error", err)
+		h.Logger.WarnContext(ctx, "failed to query user info", "screenName", ident.String(), "error", err)
 		return presence
 	}
 
@@ -245,6 +261,12 @@ func (h *PresenceHandler) getUserPresence(ctx context.Context, instance *state.S
 
 	presence.State = "online"
 
+	// The locate reply carries the screen name as the user formatted it, which
+	// beats whatever casing the caller happened to pass in.
+	if info.ScreenName != "" {
+		presence.DisplayID = info.ScreenName
+	}
+
 	if tod, ok := info.Uint32BE(wire.OServiceUserInfoSignonTOD); ok {
 		presence.OnlineTime = int64(tod)
 	}
@@ -344,9 +366,6 @@ func (h *PresenceHandler) SetState(w http.ResponseWriter, r *http.Request, sessi
 		}
 	}
 
-	// Queue presence event for other WebAPI sessions watching this user
-	h.broadcastPresenceEvent(session.ScreenName.IdentScreenName(), stateParam, awayMsg, "")
-
 	// Notify the user's own client so its status indicator re-renders. The AIM
 	// client updates its self-presence badge only from "myInfo" events; the
 	// "presence" broadcast above drives buddy dots, not the user's own state.
@@ -365,7 +384,7 @@ func (h *PresenceHandler) SetState(w http.ResponseWriter, r *http.Request, sessi
 	response.Response.StatusCode = 200
 	response.Response.StatusText = "OK"
 	response.Response.Data = map[string]interface{}{
-		"aimId":      session.ScreenName.String(),
+		"aimId":      session.ScreenName.IdentScreenName().String(),
 		"displayId":  session.ScreenName.String(),
 		"state":      stateParam,
 		"awayMsg":    awayMsg,
@@ -396,9 +415,6 @@ func (h *PresenceHandler) SetStatus(w http.ResponseWriter, r *http.Request, sess
 		h.Logger.ErrorContext(ctx, "failed to broadcast status update", "err", err.Error())
 	}
 
-	// Queue status event for other WebAPI sessions
-	h.broadcastPresenceEvent(session.ScreenName.IdentScreenName(), "", "", statusMsg)
-
 	// Notify the user's own client so its status message re-renders. Preserve the
 	// current presence state so a status-only change does not flip the self badge.
 	h.pushMyInfo(session, currentWebState(oscarSession), oscarSession.Session().AwayMessage(), statusMsg)
@@ -536,8 +552,7 @@ func (h *PresenceHandler) Icon(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	screenName := state.NewIdentScreenName(name)
-	switch h.getUserPresence(r.Context(), instance, screenName, false).State {
+	switch h.getUserPresence(r.Context(), instance, state.DisplayScreenName(name), false).State {
 	case "away":
 		iconURL = "/static/icons/away_" + iconType + "_" + size + ".png"
 	case "idle":
@@ -582,7 +597,7 @@ func (h *PresenceHandler) pushMyInfo(session *state.WebAPISession, webState, awa
 
 	screenName := session.ScreenName.String()
 	myInfo := map[string]interface{}{
-		"aimId":     screenName,
+		"aimId":     session.ScreenName.IdentScreenName().String(),
 		"displayId": screenName,
 		"friendly":  screenName,
 		"state":     webState,
@@ -597,27 +612,3 @@ func (h *PresenceHandler) pushMyInfo(session *state.WebAPISession, webState, awa
 
 	session.EventQueue.Push(types.EventType("myInfo"), myInfo)
 }
-
-// broadcastPresenceEvent sends presence updates to all WebAPI sessions watching this user
-func (h *PresenceHandler) broadcastPresenceEvent(screenName state.IdentScreenName, stateStr, awayMsg, statusMsg string) {
-	// Get all sessions that have this user in their buddy list
-	// For now, we'll broadcast to all sessions (this should be optimized)
-	// Using background context as this is an async broadcast operation
-	for _, sess := range h.SessionManager.GetAllSessions(context.Background()) {
-		if sess.EventQueue != nil && sess.Events != nil {
-			// Check if session is subscribed to presence events
-			for _, event := range sess.Events {
-				if event == "presence" || event == "myInfo" {
-					eventData := types.PresenceEvent{
-						AimID:     screenName.String(),
-						State:     stateStr,
-						AwayMsg:   awayMsg,
-						StatusMsg: statusMsg,
-					}
-					sess.EventQueue.Push(types.EventTypePresence, eventData)
-					break
-				}
-			}
-		}
-	}
-}

+ 54 - 0
server/webapi/handlers/presence_test.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 
 	"github.com/mk6i/open-oscar-server/state"
 	"github.com/mk6i/open-oscar-server/wire"
@@ -211,6 +212,11 @@ func TestPresenceHandler_GetPresence(t *testing.T) {
 
 			tt.setupMocks(feedbagService, locateService)
 
+			// Presence payloads carry the viewer's alias, so GetPresence reads the
+			// feedbag. Registered last so a case's own Query stub takes precedence.
+			feedbagService.On("Query", mock.Anything, mock.Anything, mock.Anything).
+				Return(wire.SNACMessage{Body: wire.SNAC_0x13_0x06_FeedbagReply{}}, nil).Maybe()
+
 			reqURL := "/presence/get?aimsid=" + aimsid
 			if tt.queryParams != "" {
 				reqURL += "&" + tt.queryParams
@@ -412,6 +418,54 @@ func TestPresenceHandler_SetState_EmitsMyInfoEvent(t *testing.T) {
 	assert.Equal(t, "testuser", myInfo["aimId"])
 }
 
+func TestPresenceHandler_SetState_MyInfoNormalizesAimID(t *testing.T) {
+	// The client shallow-merges myInfo onto the shared user object, so aimId must
+	// be the normalized id while displayId and friendly keep the user's own
+	// casing and spacing.
+	oscarInstance := state.NewSession().AddInstance()
+	sessionMgr, aimsid := createTestSessionManagerWithOSCAR("Mike Kelly", oscarInstance)
+
+	broadcaster := &MockBuddyBroadcaster{}
+	broadcaster.On("BroadcastBuddyArrived", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+
+	handler := &PresenceHandler{
+		SessionManager:   sessionMgr,
+		BuddyBroadcaster: broadcaster,
+		Logger:           slog.Default(),
+	}
+
+	req, err := http.NewRequest("GET", "/presence/setState?aimsid="+aimsid+"&state=away", nil)
+	assert.NoError(t, err)
+
+	rr := httptest.NewRecorder()
+	requireSession(handler.SessionManager, handler.SetState).ServeHTTP(rr, req)
+	assert.Equal(t, http.StatusOK, rr.Code)
+
+	// The setState response body carries the same identity fields.
+	var resp struct {
+		Response struct {
+			Data map[string]interface{} `json:"data"`
+		} `json:"response"`
+	}
+	assert.NoError(t, json.Unmarshal(rr.Body.Bytes(), &resp))
+	assert.Equal(t, "mikekelly", resp.Response.Data["aimId"])
+	assert.Equal(t, "Mike Kelly", resp.Response.Data["displayId"])
+
+	session, err := sessionMgr.GetSession(context.Background(), aimsid)
+	assert.NoError(t, err)
+
+	var myInfo map[string]interface{}
+	for _, event := range session.EventQueue.GetAllEvents() {
+		if event.Type == "myInfo" {
+			myInfo, _ = event.Data.(map[string]interface{})
+		}
+	}
+	require.NotNil(t, myInfo, "expected a myInfo event to be queued")
+	assert.Equal(t, "mikekelly", myInfo["aimId"])
+	assert.Equal(t, "Mike Kelly", myInfo["displayId"])
+	assert.Equal(t, "Mike Kelly", myInfo["friendly"])
+}
+
 func TestPresenceHandler_SetState_NoOSCARSession_Rejected(t *testing.T) {
 	// Anonymous (web-only, no OSCAR) sessions are rejected by the session
 	// middleware before the handler runs.

+ 82 - 142
server/webapi/handlers/session.go

@@ -21,18 +21,15 @@ import (
 
 // SessionHandler handles Web AIM API session management endpoints.
 type SessionHandler struct {
-	SessionManager      *state.WebAPISessionManager
-	OSCARSessionManager SessionManager
-	OSCARAuthService    AuthService
-	BuddyListRegistry   BuddyListRegistry
-	BuddyBroadcaster    BuddyBroadcaster
-	FeedbagService      FeedbagService
-	BuddyListManager    *BuddyListManager
-	Logger              *slog.Logger
-	OServiceService     OServiceService
-	RecalcWarning       func(ctx context.Context, instance *state.SessionInstance) error
-	LowerWarnLevel      func(ctx context.Context, instance *state.SessionInstance)
-	ChatSessionManager  ChatSessionManager
+	SessionManager   *state.WebAPISessionManager
+	OSCARAuthService AuthService
+	FeedbagService   FeedbagService
+	BuddyListManager *BuddyListManager
+	Logger           *slog.Logger
+	OServiceService  OServiceService
+	FnSessCfg        func(sess *state.Session)
+	FnSessInit       func(instance *state.SessionInstance) func() error
+	FnInstanceClose  func(instance *state.SessionInstance) func()
 }
 
 // AuthService defines methods needed for authentication.
@@ -186,135 +183,74 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	// Determine screen name from auth token or anonymous
-	var screenName state.DisplayScreenName
-
-	var cookie state.ServerCookie
-	if authToken != "" {
-		rawCookie, err := base64.URLEncoding.DecodeString(strings.TrimSpace(authToken))
-		if err != nil {
-			h.Logger.Warn("invalid authentication token (base64)", "error", err)
-			h.sendError(w, r, http.StatusUnauthorized, "invalid or expired token")
-			return
-		}
-		cookie, err = h.OSCARAuthService.CrackCookie(rawCookie)
-		if err != nil {
-			h.Logger.Warn("invalid authentication token",
-				"error", err)
-			h.sendError(w, r, http.StatusUnauthorized, "invalid or expired token")
-			return
-		}
-		screenName = cookie.ScreenName
-		tokenPreview := authToken
-		if len(tokenPreview) > 8 {
-			tokenPreview = tokenPreview[:8] + "..."
-		}
-		h.Logger.Info("authenticated session requested",
-			"token", tokenPreview,
-			"screenName", screenName)
-	} else {
-		// Anonymous session - generate guest name
-		screenName = state.DisplayScreenName("Guest_" + strconv.FormatInt(time.Now().Unix(), 36))
-		h.Logger.Info("anonymous session requested",
-			"screenName", screenName)
+	// A Web API session must be bridged to an authenticated OSCAR session;
+	// anonymous guests are not supported.
+	if authToken == "" {
+		h.sendError(w, r, http.StatusUnauthorized, "authentication token required")
+		return
 	}
 
-	// Create OSCAR session for authenticated users
-	var oscarInstance *state.SessionInstance
-	var err error
-	if authToken != "" && h.OSCARSessionManager != nil {
-		fnCfg := func(sess *state.Session) {
-			sess.OnSessionClose(func() {
-				ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
-				defer cancel()
-
-				// todo - a better way to detect server shutdowns
-				if err := h.BuddyBroadcaster.BroadcastBuddyDeparted(ctx, sess.IdentScreenName()); err != nil {
-					h.Logger.ErrorContext(ctx, "error sending buddy departure notifications", "err", err.Error())
-				}
-
-				// buddy list must be cleared before session is closed, otherwise
-				// there will be a race condition that could cause the buddy list
-				// be prematurely deleted.
-				if err := h.BuddyListRegistry.UnregisterBuddyList(ctx, sess.IdentScreenName()); err != nil {
-					h.Logger.ErrorContext(ctx, "error removing buddy list entry", "err", err.Error())
-				}
-				h.ChatSessionManager.RemoveUserFromAllChats(sess.IdentScreenName())
-				h.OSCARAuthService.Signout(ctx, sess)
-			})
-		}
+	rawCookie, err := base64.URLEncoding.DecodeString(strings.TrimSpace(authToken))
+	if err != nil {
+		h.Logger.Warn("invalid authentication token (base64)", "error", err)
+		h.sendError(w, r, http.StatusUnauthorized, "invalid or expired token")
+		return
+	}
+	cookie, err := h.OSCARAuthService.CrackCookie(rawCookie)
+	if err != nil {
+		h.Logger.Warn("invalid authentication token", "error", err)
+		h.sendError(w, r, http.StatusUnauthorized, "invalid or expired token")
+		return
+	}
+	screenName := cookie.ScreenName
+	tokenPreview := authToken
+	if len(tokenPreview) > 8 {
+		tokenPreview = tokenPreview[:8] + "..."
+	}
+	h.Logger.Info("authenticated session requested",
+		"token", tokenPreview,
+		"screenName", screenName)
 
-		// Create OSCAR session
-		oscarInstance, err = h.OSCARAuthService.RegisterBOSSession(ctx, cookie, fnCfg)
+	var instance *state.SessionInstance
 
-		if err != nil {
-			// A failed BOS registration (e.g. the per-user session cap) leaves no
-			// OSCAR session. Downstream steps (buddy list, feedbag, presence) all
-			// dereference it, so fail the request instead of continuing half-set-up.
-			h.Logger.ErrorContext(ctx, "failed to create OSCAR session", "err", err.Error())
-			h.sendError(w, r, http.StatusServiceUnavailable, "unable to establish session")
-			return
-		}
+	// Create OSCAR session
+	instance, err = h.OSCARAuthService.RegisterBOSSession(ctx, cookie, h.FnSessCfg)
+	if err != nil {
+		h.Logger.ErrorContext(ctx, "failed to create OSCAR session", "err", err.Error())
+		h.sendError(w, r, http.StatusServiceUnavailable, "unable to establish session")
+		return
+	}
 
-		if err = oscarInstance.Session().RunOnce(func() error {
-			// make buddy list visible to other users
-			if err := h.BuddyListRegistry.RegisterBuddyList(ctx, oscarInstance.IdentScreenName()); err != nil {
-				return fmt.Errorf("unable to init buddy list: %w", err)
-			}
-			// restore warning level from last session
-			if err := h.RecalcWarning(ctx, oscarInstance); err != nil {
-				return fmt.Errorf("failed to recalculate warning level: %w", err)
-			}
-			// periodically decay warning level
-			go h.LowerWarnLevel(ctx, oscarInstance)
-			return nil
-		}); err != nil {
-			h.Logger.ErrorContext(ctx, "failed to init session", "err", err.Error())
-			h.sendError(w, r, http.StatusInternalServerError, "internal server error")
-			return
-		}
+	if err = instance.Session().RunOnce(h.FnSessInit(instance)); err != nil {
+		h.Logger.ErrorContext(context.Background(), "failed to init session", "err", err.Error())
+		h.sendError(w, r, http.StatusInternalServerError, "internal server error")
+		return
+	}
 
-		// Update user visibility when an instance closes, as the user's overall status may change.
-		// Example: With 1 away and 1 non-away instance, the user appears available. If the non-away
-		// instance closes, the user should appear away.
-		oscarInstance.OnClose(func() {
-			if shuttingDown(ctx) {
-				return
-			}
-			if oscarInstance.Session().Invisible() {
-				if err := h.BuddyBroadcaster.BroadcastBuddyDeparted(ctx, oscarInstance.IdentScreenName()); err != nil {
-					h.Logger.ErrorContext(ctx, "error sending buddy departure notifications", "err", err.Error())
-				}
-			} else {
-				if err := h.BuddyBroadcaster.BroadcastBuddyArrived(ctx, oscarInstance.IdentScreenName(), oscarInstance.Session().TLVUserInfo()); err != nil {
-					h.Logger.ErrorContext(ctx, "error sending buddy arrival notifications", "err", err.Error())
-				}
-			}
-		})
+	instance.OnClose(h.FnInstanceClose(instance))
 
-		if err := h.FeedbagService.Use(ctx, oscarInstance); err != nil {
-			h.Logger.ErrorContext(ctx, "failed to use feedbag", "err", err.Error())
-		}
+	if err := h.FeedbagService.Use(ctx, instance); err != nil {
+		h.Logger.ErrorContext(ctx, "failed to use feedbag", "err", err.Error())
+	}
 
-		// A web client signals that it wants typing events through its event
-		// subscription, not through a stored feedbag buddy pref. Reflect that
-		// on the OSCAR session so ICBMService attaches the WantEvents TLV to
-		// outgoing IMs, prompting recipients to send typing notifications
-		// back. This must run after FeedbagService.Use, which otherwise
-		// overwrites the flag from stored prefs the web user may not have set.
-		oscarInstance.Session().SetTypingEventsEnabled(slices.Contains(events, "typing"))
+	// A web client signals that it wants typing events through its event
+	// subscription, not through a stored feedbag buddy pref. Reflect that
+	// on the OSCAR session so ICBMService attaches the WantEvents TLV to
+	// outgoing IMs, prompting recipients to send typing notifications
+	// back. This must run after FeedbagService.Use, which otherwise
+	// overwrites the flag from stored prefs the web user may not have set.
+	instance.Session().SetTypingEventsEnabled(slices.Contains(events, "typing"))
 
-		oscarInstance.SetSignonComplete()
+	instance.SetSignonComplete()
 
-		if err := h.OServiceService.ClientOnline(ctx, wire.BOS, wire.SNAC_0x01_0x02_OServiceClientOnline{}, oscarInstance); err != nil {
-			h.Logger.ErrorContext(ctx, "failed to set client online", "err", err.Error())
-			h.sendError(w, r, http.StatusInternalServerError, "internal server error")
-			return
-		}
+	if err := h.OServiceService.ClientOnline(ctx, wire.BOS, wire.SNAC_0x01_0x02_OServiceClientOnline{}, instance); err != nil {
+		h.Logger.ErrorContext(ctx, "failed to set client online", "err", err.Error())
+		h.sendError(w, r, http.StatusInternalServerError, "internal server error")
+		return
 	}
 
 	// Create WebAPI session
-	session, err := h.SessionManager.CreateSession(r.Context(), screenName, apiKey.DevID, events, oscarInstance, h.Logger)
+	session, err := h.SessionManager.CreateSession(r.Context(), screenName, apiKey.DevID, events, instance, h.Logger)
 	if err != nil {
 		h.Logger.ErrorContext(ctx, "failed to create session", "err", err.Error())
 		h.sendError(w, r, http.StatusInternalServerError, "failed to create session")
@@ -331,6 +267,14 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 		return h.BuddyListManager.GetBuddyListForUser(ctx, session)
 	}
 
+	// Wire the alias loader so OSCAR-driven im/presence events can repeat the
+	// buddy's friendly name. The client discards the alias it holds each time it
+	// merges a user map, so an event that omits it renames the buddy. The session
+	// caches what this returns until a feedbag change invalidates it.
+	session.BuddyAliasLoader = func(ctx context.Context) (map[string]string, error) {
+		return LookupBuddyAliases(ctx, h.FeedbagService, session.OSCARSession)
+	}
+
 	// Wire permit/deny refresher so FeedbagUpdateItem SNACs trigger a permitDeny event.
 	session.PermitDenyRefresher = func(ctx context.Context) (interface{}, error) {
 		frame := wire.SNACFrame{FoodGroup: wire.Feedbag, SubGroup: wire.FeedbagQuery}
@@ -356,7 +300,7 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 		for _, event := range events {
 			if event == "myInfo" || event == "presence" {
 				myInfoData := map[string]interface{}{
-					"aimId":        screenName.String(),
+					"aimId":        screenName.IdentScreenName().String(),
 					"displayId":    screenName.String(),
 					"friendly":     screenName.String(),
 					"state":        "online",
@@ -403,7 +347,7 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 
 	if authToken != "" {
 		myInfoPayload := map[string]interface{}{
-			"aimId":        screenName.String(),
+			"aimId":        screenName.IdentScreenName().String(),
 			"displayId":    screenName.String(),
 			"friendly":     screenName.String(),
 			"state":        "online",
@@ -525,7 +469,7 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 				Groups *[]BuddyGroup `xml:"group,omitempty"`
 			} `xml:"buddylist,omitempty"`
 		}{
-			AimID:     session.ScreenName.String(),
+			AimID:     session.ScreenName.IdentScreenName().String(),
 			DisplayID: session.ScreenName.String(),
 		}
 
@@ -615,7 +559,13 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 func (h *SessionHandler) EndSession(w http.ResponseWriter, r *http.Request, session *state.WebAPISession) {
 	ctx := r.Context()
 
-	session.OSCARSession.CloseInstance()
+	// RemoveSession evicts the session from the manager and tears it down
+	// (closes the event queue and the OSCAR instance). Without this the aimsid
+	// stays resolvable until the reaper sweeps it, and RequireSession would keep
+	// handing handlers a session whose OSCAR instance is already closed.
+	if err := h.SessionManager.RemoveSession(ctx, session.AimSID); err != nil {
+		h.Logger.ErrorContext(ctx, "failed to remove session", "err", err.Error())
+	}
 
 	// Send response
 	resp := EndSessionResponse{}
@@ -648,13 +598,3 @@ func requestScheme(r *http.Request) string {
 	}
 	return "http"
 }
-
-func shuttingDown(ctx context.Context) bool {
-	select {
-	case <-ctx.Done():
-		// server is shutting down, don't send buddy notifications
-		return true
-	default:
-	}
-	return false
-}

+ 15 - 3
server/webapi/middleware/auth.go

@@ -139,8 +139,14 @@ type WebAPISessionResolver interface {
 }
 
 // RequireSession resolves the aimsid session and passes it to next. It rejects
-// requests whose session is missing, expired, or anonymous (no bridged OSCAR
-// session) with an auth error, so downstream handlers can treat
+// requests whose session is missing or expired with an auth error. On success it
+// touches the session, sliding its expiry forward; this is the keepalive that
+// holds a long-polling client's session open (see the session lifecycle timeline
+// on state's WebAPISession manager).
+//
+// A session with a nil OSCARSession is rejected as a 500: startSession no longer
+// creates such sessions (anonymous guests are unsupported), so a nil is a broken
+// server invariant, not a client error. This lets downstream handlers treat
 // session.OSCARSession as non-nil.
 func (m *AuthMiddleware) RequireSession(sm WebAPISessionResolver, next func(http.ResponseWriter, *http.Request, *state.WebAPISession)) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -150,10 +156,16 @@ func (m *AuthMiddleware) RequireSession(sm WebAPISessionResolver, next func(http
 			return
 		}
 		session, err := sm.GetSession(r.Context(), aimsid)
-		if err != nil || session.OSCARSession == nil {
+		if err != nil {
 			m.sendSessionError(w, http.StatusUnauthorized, "invalid or expired session")
 			return
 		}
+		// startSession no longer creates sessions without an OSCAR instance, so a
+		// nil here is a server-side invariant violation, not a bad request.
+		if session.OSCARSession == nil {
+			m.sendSessionError(w, http.StatusInternalServerError, "internal server error")
+			return
+		}
 		_ = sm.TouchSession(r.Context(), aimsid)
 		next(w, r, session)
 	})

+ 101 - 25
server/webapi/server.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"log/slog"
 	"net/http"
+	"time"
 
 	"golang.org/x/sync/errgroup"
 
@@ -26,18 +27,12 @@ func NewServer(listeners []string, logger *slog.Logger, handler Handler, apiKeyV
 	}
 
 	sessionHandler := &handlers.SessionHandler{
-		SessionManager:      sessionManager,
-		OSCARSessionManager: handler.SessionRetriever.(handlers.SessionManager),
-		OSCARAuthService:    handler.AuthService,
-		BuddyListRegistry:   handler.BuddyListRegistry,
-		BuddyBroadcaster:    handler.BuddyBroadcaster,
-		FeedbagService:      handler.FeedbagService,
-		BuddyListManager:    handler.BuddyListManager.(*handlers.BuddyListManager),
-		Logger:              logger,
-		OServiceService:     handler.OServiceService,
-		RecalcWarning:       handler.RecalcWarning,
-		LowerWarnLevel:      handler.LowerWarnLevel,
-		ChatSessionManager:  handler.ChatSessionManager,
+		SessionManager:   sessionManager,
+		OSCARAuthService: handler.AuthService,
+		FeedbagService:   handler.FeedbagService,
+		BuddyListManager: handler.BuddyListManager.(*handlers.BuddyListManager),
+		Logger:           logger,
+		OServiceService:  handler.OServiceService,
 	}
 
 	eventsHandler := &handlers.EventsHandler{
@@ -62,6 +57,8 @@ func NewServer(listeners []string, logger *slog.Logger, handler Handler, apiKeyV
 	messagingHandler := &handlers.MessagingHandler{
 		SessionManager: sessionManager,
 		ICBMService:    handler.ICBMService,
+		LocateService:  handler.LocateService,
+		FeedbagService: handler.FeedbagService,
 		Logger:         logger,
 	}
 
@@ -274,17 +271,82 @@ func NewServer(listeners []string, logger *slog.Logger, handler Handler, apiKeyV
 		})
 	}
 
+	shutdownCtx, shutdownCancel := context.WithCancel(context.Background())
+
+	sessionHandler.FnSessCfg = func(sess *state.Session) {
+		sess.OnSessionClose(func() {
+			ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+			defer cancel()
+
+			if !shuttingDown(shutdownCtx) {
+				if err := handler.BuddyBroadcaster.BroadcastBuddyDeparted(ctx, sess.IdentScreenName()); err != nil {
+					logger.ErrorContext(ctx, "error sending buddy departure notifications", "err", err.Error())
+				}
+			}
+
+			// buddy list must be cleared before session is closed, otherwise
+			// there will be a race condition that could cause the buddy list
+			// be prematurely deleted.
+			if err := handler.BuddyListRegistry.UnregisterBuddyList(ctx, sess.IdentScreenName()); err != nil {
+				logger.ErrorContext(ctx, "error removing buddy list entry", "err", err.Error())
+			}
+			handler.ChatSessionManager.RemoveUserFromAllChats(sess.IdentScreenName())
+			handler.AuthService.Signout(ctx, sess)
+		})
+	}
+
+	sessionHandler.FnSessInit = func(instance *state.SessionInstance) func() error {
+		return func() error {
+			// make buddy list visible to other users
+			if err := handler.BuddyListRegistry.RegisterBuddyList(shutdownCtx, instance.IdentScreenName()); err != nil {
+				return fmt.Errorf("unable to init buddy list: %w", err)
+			}
+			// restore warning level from last session
+			if err := handler.RecalcWarning(shutdownCtx, instance); err != nil {
+				return fmt.Errorf("failed to recalculate warning level: %w", err)
+			}
+			// periodically decay warning level
+			go handler.LowerWarnLevel(shutdownCtx, instance)
+			return nil
+		}
+	}
+
+	sessionHandler.FnInstanceClose = func(instance *state.SessionInstance) func() {
+		return func() {
+			if shuttingDown(shutdownCtx) {
+				return
+			}
+			if instance.Session().Invisible() {
+				if err := handler.BuddyBroadcaster.BroadcastBuddyDeparted(shutdownCtx, instance.IdentScreenName()); err != nil {
+					logger.ErrorContext(shutdownCtx, "error sending buddy departure notifications", "err", err.Error())
+				}
+			} else {
+				if err := handler.BuddyBroadcaster.BroadcastBuddyArrived(shutdownCtx, instance.IdentScreenName(), instance.Session().TLVUserInfo()); err != nil {
+					logger.ErrorContext(shutdownCtx, "error sending buddy arrival notifications", "err", err.Error())
+				}
+			}
+		}
+	}
 	return &Server{
-		servers: servers,
-		logger:  logger,
+		servers:        servers,
+		logger:         logger,
+		sessionManager: sessionManager,
+		shutdownCtx:    shutdownCtx,
+		shutdownCancel: shutdownCancel,
 	}
 }
 
 // Server hosts an HTTP endpoint capable of handling AIM-style Kerberos
 // authentication. The messages are structured as SNACs transmitted over HTTP.
+//
+// shutdownCtx bounds the lifetime of the background session reaper: ListenAndServe
+// drives it, and Shutdown (or a failed listener) calls shutdownCancel to unwind.
 type Server struct {
-	servers []*http.Server
-	logger  *slog.Logger
+	servers        []*http.Server
+	logger         *slog.Logger
+	sessionManager *state.WebAPISessionManager
+	shutdownCtx    context.Context
+	shutdownCancel context.CancelFunc
 }
 
 func (s *Server) ListenAndServe() error {
@@ -293,15 +355,18 @@ func (s *Server) ListenAndServe() error {
 		return nil
 	}
 
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
+	g, ctx := errgroup.WithContext(s.shutdownCtx)
+
+	g.Go(func() error {
+		s.sessionManager.Run(ctx)
+		return nil
+	})
 
-	g, _ := errgroup.WithContext(ctx)
 	for _, server := range s.servers {
 		g.Go(func() error {
 			s.logger.Info("starting server", "addr", server.Addr)
 			if err := server.ListenAndServe(); !errors.Is(err, http.ErrServerClosed) {
-				cancel()
+				s.shutdownCancel()
 				return fmt.Errorf("unable to start webapi server: %w", err)
 			}
 			return nil
@@ -312,11 +377,22 @@ func (s *Server) ListenAndServe() error {
 }
 
 func (s *Server) Shutdown(ctx context.Context) error {
-	if len(s.servers) > 0 {
-		for _, srv := range s.servers {
-			_ = srv.Shutdown(ctx)
-		}
-		s.logger.Info("shutdown complete")
+	s.logger.Debug("Initiating graceful shutdown...")
+	s.shutdownCancel() // stop the session reaper so ListenAndServe's errgroup can drain
+	for _, srv := range s.servers {
+		_ = srv.Shutdown(ctx)
 	}
+	s.sessionManager.Shutdown()
+	s.logger.Info("shutdown complete")
 	return nil
 }
+
+func shuttingDown(ctx context.Context) bool {
+	select {
+	case <-ctx.Done():
+		// server is shutting down, don't send buddy notifications
+		return true
+	default:
+	}
+	return false
+}

+ 6 - 1
server/webapi/types/conversation.go

@@ -14,13 +14,18 @@ func ConversationEventData(operation string, conversations []map[string]interfac
 }
 
 // ConversationEntry builds one conversation object for the Web AIM client.
+//
+// An empty displayID is omitted rather than sent blank: the client falls back to
+// the name it already has for aimID, whereas any value present here replaces it.
 func ConversationEntry(aimID, displayID, message, msgID, sender string, sent bool, unread int) map[string]interface{} {
 	entry := map[string]interface{}{
 		"aimId":       aimID,
-		"displayId":   displayID,
 		"active":      0,
 		"unreadCount": unread,
 	}
+	if displayID != "" {
+		entry["displayId"] = displayID
+	}
 	if message != "" {
 		entry["lastIM"] = map[string]interface{}{
 			"message":   message,

+ 12 - 0
server/webapi/types/events.go

@@ -35,8 +35,12 @@ type Event struct {
 }
 
 // PresenceEvent represents a presence change event.
+// Friendly repeats the viewer's alias for the user. The client's merge deletes any
+// alias it already holds, so a presence update that omits it silently renames the
+// buddy back to their screen name. See UserInfo.
 type PresenceEvent struct {
 	AimID      string `json:"aimId"`
+	Friendly   string `json:"friendly,omitempty"`
 	State      string `json:"state"` // "online", "offline", "away", "idle"
 	StatusMsg  string `json:"statusMsg,omitempty"`
 	AwayMsg    string `json:"awayMsg,omitempty"`
@@ -65,9 +69,17 @@ type SentIMEvent struct {
 }
 
 // UserInfo represents basic user information in events.
+// AimID is the normalized screen name the client keys users by. DisplayID is the
+// screen name as its owner formatted it. Friendly is the viewer's private alias for
+// that user, and takes precedence over DisplayID when the client renders a name.
+//
+// The client merges every user map it receives onto the single user object it holds
+// per aimId, and that merge deletes friendly before applying the map. An alias
+// therefore has to be repeated on every user map, or it is lost.
 type UserInfo struct {
 	AimID      string  `json:"aimId"`
 	DisplayID  string  `json:"displayId,omitempty"`
+	Friendly   string  `json:"friendly,omitempty"`
 	UserType   string  `json:"userType,omitempty"`
 	State      string  `json:"state,omitempty"`
 	OnlineTime float64 `json:"onlineTime,omitempty"` // float64 for AMF3 encoding

+ 4 - 1
state/webapi_imlog.go

@@ -114,6 +114,9 @@ func (s *WebAPISession) GetStoredIMs(q StoredIMQuery) []map[string]interface{} {
 	return out
 }
 
+// normalizeWebAPIAimID keys the IM log by the same normalization the web client
+// applies to aimIds, so a partner stored from a display screen name is still
+// found when the client queries by aimId.
 func normalizeWebAPIAimID(aimID string) string {
-	return strings.ToLower(aimID)
+	return NewIdentScreenName(aimID).String()
 }

+ 15 - 0
state/webapi_imlog_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestWebAPISession_GetStoredIMs(t *testing.T) {
@@ -31,3 +32,17 @@ func TestWebAPISession_GetStoredIMs(t *testing.T) {
 	assert.Len(t, msgs, 1)
 	assert.Equal(t, "msg-2", msgs[0]["msgId"])
 }
+
+func TestWebAPISession_GetStoredIMs_NormalizesPartner(t *testing.T) {
+	sess := &WebAPISession{}
+	sess.AddStoredIM("Mike Kelly", "mikekelly", "hello", "msg-1", 100)
+
+	// The web client queries history by the normalized aimId, never by the
+	// display screen name it was stored under.
+	msgs := sess.GetStoredIMs(StoredIMQuery{
+		PartnerAimID: "mikekelly",
+		NToGet:       10,
+	})
+	require.Len(t, msgs, 1)
+	assert.Equal(t, "msg-1", msgs[0]["msgId"])
+}

+ 188 - 110
state/webapi_session.go

@@ -20,6 +20,36 @@ var (
 	ErrNoWebAPISession = errors.New("WebAPI session not found")
 	// ErrWebAPISessionExpired is returned when a WebAPI session has expired.
 	ErrWebAPISessionExpired = errors.New("WebAPI session expired")
+	// ErrWebAPISessionManagerClosed is returned when a session is requested from
+	// a manager that has been shut down.
+	ErrWebAPISessionManagerClosed = errors.New("WebAPI session manager is shut down")
+)
+
+// Web API session lifecycle timeline.
+//
+// A web client keeps its session alive by long-polling GET /aim/fetchEvents.
+// Every authenticated request touches the session (middleware.RequireSession
+// calls TouchSession at request arrival), sliding expiry to now + the TTL. A
+// single poll blocks for up to 60s (the fetchEvents long-poll cap) and the
+// client waits ~500ms (TimeToNextFetch) before re-polling, so in steady state a
+// healthy client touches the session at worst every ~60-65s once jitter is
+// included. That worst-case touch interval is the floor the TTL must clear.
+//
+// If a client hangs up without calling endSession, its last touch was at its
+// last poll: the session then expires webAPISessionTTL later and the reaper
+// sweeps it within one webAPISessionReapInterval tick. So a silent client is
+// removed (and its OSCAR session closed) within TTL + tick of going quiet.
+const (
+	// webAPISessionTTL bounds how long a session survives without a poll. It is
+	// sized to absorb one missed poll cycle: ~60s for the normal cycle, ~60s for
+	// the absorbed miss, plus ~20s of jitter margin. Two consecutive misses mean
+	// the client is genuinely gone and the session is reaped.
+	webAPISessionTTL = 150 * time.Second
+
+	// webAPISessionReapInterval is how often the cleanup goroutine sweeps for
+	// expired sessions (~TTL/5). A dead session lingers at most
+	// webAPISessionTTL + webAPISessionReapInterval before removal.
+	webAPISessionReapInterval = 30 * time.Second
 )
 
 // WebAPISession represents an active Web AIM API session.
@@ -45,6 +75,9 @@ type WebAPISession struct {
 	TempBuddies         map[string]bool                                // Temporary buddies for this session only
 	BuddyListRefresher  func(ctx context.Context) (interface{}, error) // Called on feedbag changes to push buddylist event
 	PermitDenyRefresher func(ctx context.Context) (interface{}, error) // Called on feedbag changes to push permitDeny event
+	BuddyAliasLoader    func(ctx context.Context) (map[string]string, error)
+	aliases             map[string]string // cached BuddyAliasLoader result, nil when unloaded or invalidated
+	aliasMu             sync.Mutex
 	imLog               map[string][]WebAPIStoredIM
 	imLogMu             sync.Mutex
 	logger              *slog.Logger // Logger for debugging
@@ -55,11 +88,61 @@ func (s *WebAPISession) IsExpired() bool {
 	return time.Now().After(s.ExpiresAt)
 }
 
+// Aliases returns this session owner's private buddy aliases, keyed by normalized
+// screen name. Aliases live in the owner's feedbag, so the map is loaded once and
+// cached until a feedbag change invalidates it: a signon that brings a large buddy
+// list online costs one feedbag query instead of one per buddy.
+//
+// The map is owned by the session and must not be mutated by callers.
+//
+// aliasMu is deliberately held across the load rather than released while the
+// feedbag is queried. Another instance of the owner can rename a buddy mid-query,
+// and its FeedbagUpdateItem SNAC invalidates this cache; if the load ran outside
+// the lock, that query's pre-rename result could be stored *after* the
+// invalidation and serve the old alias until the next feedbag change. Holding the
+// lock makes the invalidation wait for the load and then win.
+func (s *WebAPISession) Aliases(ctx context.Context) map[string]string {
+	s.aliasMu.Lock()
+	defer s.aliasMu.Unlock()
+
+	// The loader is wired after the session is created, so an event arriving in
+	// that window has no way to resolve aliases.
+	if s.BuddyAliasLoader == nil {
+		return nil
+	}
+	if s.aliases == nil {
+		aliases, err := s.BuddyAliasLoader(ctx)
+		if err != nil {
+			s.logger.Error("failed to load buddy aliases", "err", err.Error())
+			return nil
+		}
+		s.aliases = aliases
+	}
+	return s.aliases
+}
+
+// InvalidateAliases drops the cached alias map so the next Aliases call reloads it.
+// Callers that change the owner's feedbag must call this: the feedbag service
+// relays FeedbagUpdateItem only to the owner's *other* instances, so a session
+// never sees a SNAC for its own writes.
+func (s *WebAPISession) InvalidateAliases() {
+	s.aliasMu.Lock()
+	defer s.aliasMu.Unlock()
+	s.aliases = nil
+}
+
+// aliasFor returns this session owner's private alias for buddy, or "" when none is
+// set. The web client deletes the alias it holds whenever it merges a user map, so
+// every event naming a buddy has to repeat it.
+func (s *WebAPISession) aliasFor(buddy IdentScreenName) string {
+	// Runs on the SNAC listener goroutine, which has no request context.
+	return s.Aliases(context.Background())[buddy.String()]
+}
+
 // Touch updates the last accessed time and extends expiration if needed.
 func (s *WebAPISession) Touch() {
 	s.LastAccessed = time.Now()
-	// Extend expiration by 60 minutes from last access
-	newExpiry := s.LastAccessed.Add(60 * time.Minute)
+	newExpiry := s.LastAccessed.Add(webAPISessionTTL)
 	if newExpiry.After(s.ExpiresAt) {
 		s.ExpiresAt = newExpiry
 	}
@@ -159,15 +242,20 @@ func (s *WebAPISession) handleIncomingIM(msg wire.SNACMessage) {
 	// client dedupes its conversation list by msgId, silently dropping any
 	// collisions. Mint a fresh random id instead of reusing body.Cookie.
 	msgID := strconv.FormatUint(mrand.Uint64(), 16)
-	partner := body.ScreenName
+	// SNAC user info carries the sender's display screen name. The web client
+	// keys conversations and users by the normalized aimId and only renders
+	// displayId, so the two forms must not be interchanged.
+	partnerDisplay := body.ScreenName
+	partnerAimID := NewIdentScreenName(partnerDisplay).String()
 	nowSec := time.Now().Unix()
-	s.AddStoredIM(partner, partner, messageText, msgID, nowSec)
+	s.AddStoredIM(partnerAimID, partnerAimID, messageText, msgID, nowSec)
 
 	// Create IM event
 	imEvent := types.IMEvent{
 		Source: types.UserInfo{
-			AimID:     body.ScreenName,
-			DisplayID: body.ScreenName,
+			AimID:     partnerAimID,
+			DisplayID: partnerDisplay,
+			Friendly:  s.aliasFor(NewIdentScreenName(partnerAimID)),
 			UserType:  "aim",
 			State:     "online",
 		},
@@ -178,17 +266,26 @@ func (s *WebAPISession) handleIncomingIM(msg wire.SNACMessage) {
 	}
 
 	s.EventQueue.Push(types.EventTypeIM, imEvent)
+	s.logger.Debug("delivered instant message",
+		"from", partnerDisplay,
+		"to", s.ScreenName)
 
 	if s.IsSubscribedTo("conversation") {
+		// unread is 0 here, not 1, because the "im" event pushed above already
+		// causes the client to increment its own persisted per-buddy unread
+		// tally. The "Recent chats" badge is the sum of that persisted tally and
+		// this conversation's unreadCount, so sending 1 here would double-count
+		// the message (badge shows 2 for the first IM). Mirrors the sent-IM path,
+		// which also passes 0.
 		s.EventQueue.Push(types.EventTypeConversation, types.ConversationEventData("update", []map[string]interface{}{
 			types.ConversationEntry(
-				body.ScreenName,
-				body.ScreenName,
+				partnerAimID,
+				partnerDisplay,
 				messageText,
 				msgID,
-				body.ScreenName,
+				partnerAimID,
 				false,
-				1,
+				0,
 			),
 		}))
 	}
@@ -217,7 +314,7 @@ func (s *WebAPISession) handleTypingNotification(msg wire.SNACMessage) {
 	}
 
 	typingEvent := types.TypingEvent{
-		AimID:        body.ScreenName,
+		AimID:        NewIdentScreenName(body.ScreenName).String(),
 		TypingStatus: typingStatus,
 	}
 
@@ -261,8 +358,10 @@ func (s *WebAPISession) handleBuddyArrived(msg wire.SNACMessage) {
 		}
 	}
 
+	buddy := NewIdentScreenName(body.ScreenName)
 	presenceEvent := types.PresenceEvent{
-		AimID:    body.ScreenName,
+		AimID:    buddy.String(),
+		Friendly: s.aliasFor(buddy),
 		State:    stateStr,
 		UserType: "aim",
 	}
@@ -281,8 +380,10 @@ func (s *WebAPISession) handleBuddyDeparted(msg wire.SNACMessage) {
 		return
 	}
 
+	buddy := NewIdentScreenName(body.ScreenName)
 	presenceEvent := types.PresenceEvent{
-		AimID:    body.ScreenName,
+		AimID:    buddy.String(),
+		Friendly: s.aliasFor(buddy),
 		State:    "offline",
 		UserType: "aim",
 	}
@@ -293,6 +394,9 @@ func (s *WebAPISession) handleBuddyDeparted(msg wire.SNACMessage) {
 func (s *WebAPISession) handleFeedbagMessage(msg wire.SNACMessage) {
 	switch msg.Frame.SubGroup {
 	case wire.FeedbagInsertItem, wire.FeedbagUpdateItem, wire.FeedbagDeleteItem:
+		// A buddy item carries its alias, so any feedbag write can change the map.
+		s.InvalidateAliases()
+
 		if s.BuddyListRefresher != nil {
 			groups, err := s.BuddyListRefresher(context.Background())
 			if err != nil {
@@ -323,27 +427,19 @@ func (s *WebAPISession) handleFeedbagMessage(msg wire.SNACMessage) {
 }
 
 // WebAPISessionManager manages Web API sessions with thread-safe operations.
+// Construct it with NewWebAPISessionManager and drive its reaper with Run.
 type WebAPISessionManager struct {
-	sessions      map[string]*WebAPISession          // Keyed by aimsid
-	byUser        map[IdentScreenName]*WebAPISession // Keyed by screen name
-	mu            sync.RWMutex
-	cleanupTicker *time.Ticker
-	stopCleanup   chan struct{}
+	sessions map[string]*WebAPISession // Keyed by aimsid
+	mu       sync.RWMutex
+	closed   bool // set by Shutdown; rejects new sessions and makes drain idempotent
 }
 
-// NewWebAPISessionManager creates a new WebAPI session manager.
+// NewWebAPISessionManager creates a new WebAPI session manager. It does not start
+// any goroutines; call Run to start reaping expired sessions.
 func NewWebAPISessionManager() *WebAPISessionManager {
-	mgr := &WebAPISessionManager{
-		sessions:    make(map[string]*WebAPISession),
-		byUser:      make(map[IdentScreenName]*WebAPISession),
-		stopCleanup: make(chan struct{}),
+	return &WebAPISessionManager{
+		sessions: make(map[string]*WebAPISession),
 	}
-
-	// Start cleanup goroutine to remove expired sessions
-	mgr.cleanupTicker = time.NewTicker(1 * time.Minute)
-	go mgr.cleanupExpiredSessions()
-
-	return mgr
 }
 
 // CreateSession creates a new WebAPI session.
@@ -351,11 +447,10 @@ func (m *WebAPISessionManager) CreateSession(ctx context.Context, screenName Dis
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
-	// Check if user already has an active session
-	identName := screenName.IdentScreenName()
-	if existing, exists := m.byUser[identName]; exists {
-		// Remove the old session
-		delete(m.sessions, existing.AimSID)
+	// Refuse to create sessions once shut down: the reaper is stopped, so a
+	// session added now would never be closed or reaped.
+	if m.closed {
+		return nil, ErrWebAPISessionManagerClosed
 	}
 
 	// Generate unique session ID
@@ -374,14 +469,13 @@ func (m *WebAPISessionManager) CreateSession(ctx context.Context, screenName Dis
 		DevID:           devID,
 		CreatedAt:       now,
 		LastAccessed:    now,
-		ExpiresAt:       now.Add(60 * time.Minute), // 60 minute initial expiry
-		FetchTimeout:    60000,                     // 60 seconds default for better stability
-		TimeToNextFetch: 500,                       // 500ms suggested delay
+		ExpiresAt:       now.Add(webAPISessionTTL),
+		FetchTimeout:    60000, // 60 seconds default for better stability
+		TimeToNextFetch: 500,   // 500ms suggested delay
 		logger:          logger,
 	}
 
 	m.sessions[aimsid] = session
-	m.byUser[identName] = session
 
 	// Start listening to OSCAR session message channel
 	session.StartListeningToOSCARSession()
@@ -406,40 +500,23 @@ func (m *WebAPISessionManager) GetSession(ctx context.Context, aimsid string) (*
 	return session, nil
 }
 
-// GetSessionByUser retrieves a session by screen name.
-func (m *WebAPISessionManager) GetSessionByUser(ctx context.Context, screenName IdentScreenName) (*WebAPISession, error) {
-	m.mu.RLock()
-	defer m.mu.RUnlock()
-
-	session, exists := m.byUser[screenName]
-	if !exists {
-		return nil, ErrNoWebAPISession
-	}
-
-	if session.IsExpired() {
-		return nil, ErrWebAPISessionExpired
-	}
-
-	return session, nil
-}
-
 // RemoveSession removes a session by aimsid.
 func (m *WebAPISessionManager) RemoveSession(ctx context.Context, aimsid string) error {
 	m.mu.Lock()
-	defer m.mu.Unlock()
 
 	session, exists := m.sessions[aimsid]
 	if !exists {
+		m.mu.Unlock()
 		return ErrNoWebAPISession
 	}
 
 	delete(m.sessions, aimsid)
-	delete(m.byUser, session.ScreenName.IdentScreenName())
+	m.mu.Unlock()
 
-	// CloseSession the event queue to unblock any waiting fetches
-	if session.EventQueue != nil {
-		session.EventQueue.Close()
-	}
+	// Tear down outside the lock: CloseInstance fans out to buddy-departed
+	// broadcasts and signout, which we don't want to run under m.mu.
+	session.EventQueue.Close()
+	session.OSCARSession.CloseInstance()
 
 	return nil
 }
@@ -458,69 +535,70 @@ func (m *WebAPISessionManager) TouchSession(ctx context.Context, aimsid string)
 	return nil
 }
 
-// GetAllSessions returns all active sessions (for monitoring/admin).
-func (m *WebAPISessionManager) GetAllSessions(ctx context.Context) []*WebAPISession {
-	m.mu.RLock()
-	defer m.mu.RUnlock()
-
-	sessions := make([]*WebAPISession, 0, len(m.sessions))
-	for _, session := range m.sessions {
-		if !session.IsExpired() {
-			sessions = append(sessions, session)
-		}
-	}
-	return sessions
-}
-
-// cleanupExpiredSessions periodically removes expired sessions.
-func (m *WebAPISessionManager) cleanupExpiredSessions() {
+// Run reaps expired sessions on a fixed interval until ctx is cancelled. The
+// caller owns the goroutine's lifecycle; typically launch it under the server's
+// errgroup:
+//
+//	g.Go(func() error { mgr.Run(ctx); return nil })
+func (m *WebAPISessionManager) Run(ctx context.Context) {
+	ticker := time.NewTicker(webAPISessionReapInterval)
+	defer ticker.Stop()
 	for {
 		select {
-		case <-m.cleanupTicker.C:
-			m.mu.Lock()
-			now := time.Now()
-			var toRemove []string
-
-			for aimsid, session := range m.sessions {
-				if now.After(session.ExpiresAt) {
-					toRemove = append(toRemove, aimsid)
-				}
-			}
-
-			for _, aimsid := range toRemove {
-				session := m.sessions[aimsid]
-				delete(m.sessions, aimsid)
-				delete(m.byUser, session.ScreenName.IdentScreenName())
-				if session.EventQueue != nil {
-					session.EventQueue.Close()
-				}
-			}
-			m.mu.Unlock()
-
-		case <-m.stopCleanup:
-			m.cleanupTicker.Stop()
+		case <-ticker.C:
+			m.reapExpired()
+		case <-ctx.Done():
 			return
 		}
 	}
 }
 
-// Shutdown stops the session manager and cleans up resources.
-func (m *WebAPISessionManager) Shutdown(ctx context.Context) {
-	close(m.stopCleanup)
+// reapExpired removes every expired session and tears it down.
+func (m *WebAPISessionManager) reapExpired() {
+	m.mu.Lock()
+	now := time.Now()
+	var expired []*WebAPISession
+	for aimsid, session := range m.sessions {
+		if now.After(session.ExpiresAt) {
+			delete(m.sessions, aimsid)
+			expired = append(expired, session)
+		}
+	}
+	m.mu.Unlock()
+
+	// Tear down outside the lock: CloseInstance fans out to buddy-departed
+	// broadcasts and signout, which we don't want to run under m.mu.
+	for _, session := range expired {
+		session.EventQueue.Close()
+		session.OSCARSession.CloseInstance()
+	}
+}
 
+// Shutdown drains and closes all sessions and blocks further CreateSession
+// calls. The reaper goroutine is stopped separately by cancelling the context
+// passed to Run. Safe to call more than once.
+func (m *WebAPISessionManager) Shutdown() {
 	m.mu.Lock()
-	defer m.mu.Unlock()
+	if m.closed {
+		m.mu.Unlock()
+		return
+	}
+	m.closed = true
 
-	// CloseSession all event queues
+	sessions := make([]*WebAPISession, 0, len(m.sessions))
 	for _, session := range m.sessions {
-		if session.EventQueue != nil {
-			session.EventQueue.Close()
-		}
+		sessions = append(sessions, session)
 	}
-
 	// Clear all sessions
 	m.sessions = make(map[string]*WebAPISession)
-	m.byUser = make(map[IdentScreenName]*WebAPISession)
+	m.mu.Unlock()
+
+	// Tear down outside the lock: CloseInstance fans out to buddy-departed
+	// broadcasts and signout, which we don't want to run under m.mu.
+	for _, session := range sessions {
+		session.EventQueue.Close()
+		session.OSCARSession.CloseInstance()
+	}
 }
 
 // generateSessionID creates a cryptographically secure session ID.

+ 373 - 0
state/webapi_session_test.go

@@ -1,12 +1,17 @@
 package state
 
 import (
+	"context"
+	"io"
+	"log/slog"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/mk6i/open-oscar-server/server/webapi/types"
+	"github.com/mk6i/open-oscar-server/wire"
 )
 
 func TestWebAPISession_TempBuddies(t *testing.T) {
@@ -258,3 +263,371 @@ func TestWebAPISession_TempBuddiesIndependence(t *testing.T) {
 	assert.True(t, session1.TempBuddies["buddy3"])
 	assert.False(t, session2.TempBuddies["buddy3"])
 }
+
+// TestWebAPISessionManager_ShutdownIdempotent verifies Shutdown is safe to call
+// more than once (e.g. from overlapping shutdown paths): the closed flag makes
+// the second call a no-op instead of re-draining.
+func TestWebAPISessionManager_ShutdownIdempotent(t *testing.T) {
+	mgr := NewWebAPISessionManager()
+
+	mgr.Shutdown()
+
+	assert.NotPanics(t, func() {
+		mgr.Shutdown()
+	})
+}
+
+// TestWebAPISessionManager_CreateAfterShutdown verifies that a session cannot be
+// created once the manager is shut down. Otherwise the reaper is stopped and the
+// session would never be closed or reaped, leaking its OSCAR session.
+func TestWebAPISessionManager_CreateAfterShutdown(t *testing.T) {
+	mgr := NewWebAPISessionManager()
+
+	ctx := context.Background()
+	mgr.Shutdown()
+
+	sess, err := mgr.CreateSession(ctx, DisplayScreenName("testuser"), "dev", []string{"presence"}, nil, nil)
+	assert.Nil(t, sess)
+	assert.ErrorIs(t, err, ErrWebAPISessionManagerClosed)
+}
+
+// TestWebAPISessionManager_ShutdownDrainsAndClosesSessions verifies that Shutdown
+// collects every live session and tears it down: it drains the maps and closes
+// each session's event queue and OSCAR instance.
+func TestWebAPISessionManager_ShutdownDrainsAndClosesSessions(t *testing.T) {
+	mgr := NewWebAPISessionManager()
+	ctx := context.Background()
+
+	inst1 := NewSession().AddInstance()
+	inst2 := NewSession().AddInstance()
+
+	s1, err := mgr.CreateSession(ctx, DisplayScreenName("alice"), "dev", []string{"presence"}, inst1, slog.Default())
+	assert.NoError(t, err)
+	s2, err := mgr.CreateSession(ctx, DisplayScreenName("bob"), "dev", []string{"presence"}, inst2, slog.Default())
+	assert.NoError(t, err)
+
+	mgr.Shutdown()
+
+	// Maps drained: the collect loop ran over both sessions.
+	assert.Empty(t, mgr.sessions)
+
+	// Each session's event queue and OSCAR instance were closed: the teardown
+	// loop ran for every collected session.
+	for _, s := range []*WebAPISession{s1, s2} {
+		_, err := s.EventQueue.Fetch(ctx, 0, 10*time.Millisecond)
+		assert.Error(t, err, "event queue should be closed")
+	}
+	for _, inst := range []*SessionInstance{inst1, inst2} {
+		select {
+		case <-inst.Closed():
+		default:
+			t.Error("OSCAR instance should be closed")
+		}
+	}
+}
+
+// TestWebAPISessionManager_ReapExpired verifies reapExpired removes and tears
+// down only expired sessions, leaving live ones untouched.
+func TestWebAPISessionManager_ReapExpired(t *testing.T) {
+	mgr := NewWebAPISessionManager()
+	ctx := context.Background()
+
+	expiredInst := NewSession().AddInstance()
+	liveInst := NewSession().AddInstance()
+
+	expired, err := mgr.CreateSession(ctx, "alice", "dev", []string{"presence"}, expiredInst, slog.Default())
+	assert.NoError(t, err)
+	live, err := mgr.CreateSession(ctx, "bob", "dev", []string{"presence"}, liveInst, slog.Default())
+	assert.NoError(t, err)
+
+	// Force alice's session into the past; bob keeps its default future expiry.
+	expired.ExpiresAt = time.Now().Add(-time.Minute)
+
+	mgr.reapExpired()
+
+	// Expired session removed; live session retained.
+	assert.NotContains(t, mgr.sessions, expired.AimSID)
+	assert.Contains(t, mgr.sessions, live.AimSID)
+
+	// Expired session torn down: event queue and OSCAR instance closed.
+	_, err = expired.EventQueue.Fetch(ctx, 0, 10*time.Millisecond)
+	assert.Error(t, err, "expired session's event queue should be closed")
+	select {
+	case <-expiredInst.Closed():
+	default:
+		t.Error("expired session's OSCAR instance should be closed")
+	}
+
+	// Live session left running.
+	select {
+	case <-liveInst.Closed():
+		t.Error("live session's OSCAR instance should not be closed")
+	default:
+	}
+}
+
+// The client deletes the alias it holds each time it merges a user map, so every
+// event naming a buddy has to repeat it. An incoming IM and a presence change both
+// carry a user map, and both would otherwise rename an aliased buddy.
+func TestWebAPISession_RepeatsBuddyAliasOnOSCAREvents(t *testing.T) {
+	newSession := func() *WebAPISession {
+		return &WebAPISession{
+			ScreenName: DisplayScreenName("me"),
+			Events:     []string{"im", "conversation", "presence"},
+			EventQueue: types.NewEventQueue(10),
+			logger:     slog.New(slog.NewTextHandler(io.Discard, nil)),
+			BuddyAliasLoader: func(_ context.Context) (map[string]string, error) {
+				return map[string]string{"mikekelly": "MICHAELKELLY"}, nil
+			},
+		}
+	}
+
+	t.Run("incoming IM", func(t *testing.T) {
+		sess := newSession()
+		frags, err := wire.ICBMFragmentList("hello")
+		require.NoError(t, err)
+		body := wire.SNAC_0x04_0x07_ICBMChannelMsgToClient{
+			ChannelID:   wire.ICBMChannelIM,
+			TLVUserInfo: wire.TLVUserInfo{ScreenName: "Mike Kelly"},
+		}
+		body.Append(wire.NewTLVBE(wire.ICBMTLVAOLIMData, frags))
+
+		sess.handleIncomingIM(wire.SNACMessage{Body: body})
+
+		events := sess.EventQueue.GetAllEvents()
+		require.NotEmpty(t, events)
+		imEvent := events[0].Data.(types.IMEvent)
+		assert.Equal(t, "mikekelly", imEvent.Source.AimID)
+		assert.Equal(t, "Mike Kelly", imEvent.Source.DisplayID)
+		assert.Equal(t, "MICHAELKELLY", imEvent.Source.Friendly)
+	})
+
+	t.Run("buddy arrived", func(t *testing.T) {
+		sess := newSession()
+		sess.handleBuddyArrived(wire.SNACMessage{Body: wire.SNAC_0x03_0x0B_BuddyArrived{
+			TLVUserInfo: wire.TLVUserInfo{ScreenName: "Mike Kelly"},
+		}})
+
+		events := sess.EventQueue.GetAllEvents()
+		require.Len(t, events, 1)
+		presence := events[0].Data.(types.PresenceEvent)
+		assert.Equal(t, "mikekelly", presence.AimID)
+		assert.Equal(t, "MICHAELKELLY", presence.Friendly)
+	})
+
+	t.Run("buddy departed", func(t *testing.T) {
+		sess := newSession()
+		sess.handleBuddyDeparted(wire.SNACMessage{Body: wire.SNAC_0x03_0x0C_BuddyDeparted{
+			TLVUserInfo: wire.TLVUserInfo{ScreenName: "Mike Kelly"},
+		}})
+
+		events := sess.EventQueue.GetAllEvents()
+		require.Len(t, events, 1)
+		presence := events[0].Data.(types.PresenceEvent)
+		assert.Equal(t, "mikekelly", presence.AimID)
+		assert.Equal(t, "MICHAELKELLY", presence.Friendly)
+	})
+
+	t.Run("unaliased buddy omits friendly", func(t *testing.T) {
+		sess := newSession()
+		sess.handleBuddyArrived(wire.SNACMessage{Body: wire.SNAC_0x03_0x0B_BuddyArrived{
+			TLVUserInfo: wire.TLVUserInfo{ScreenName: "Someone Else"},
+		}})
+
+		events := sess.EventQueue.GetAllEvents()
+		require.Len(t, events, 1)
+		assert.Empty(t, events[0].Data.(types.PresenceEvent).Friendly)
+	})
+}
+
+// Aliases all come from one feedbag query, so a signon that brings a whole buddy
+// list online must not re-query the feedbag per buddy.
+func TestWebAPISession_CachesBuddyAliases(t *testing.T) {
+	var loads int
+	sess := &WebAPISession{
+		ScreenName: DisplayScreenName("me"),
+		Events:     []string{"presence"},
+		EventQueue: types.NewEventQueue(10),
+		logger:     slog.New(slog.NewTextHandler(io.Discard, nil)),
+		BuddyAliasLoader: func(_ context.Context) (map[string]string, error) {
+			loads++
+			return map[string]string{"mikekelly": "MICHAELKELLY"}, nil
+		},
+	}
+
+	for range 5 {
+		sess.handleBuddyArrived(wire.SNACMessage{Body: wire.SNAC_0x03_0x0B_BuddyArrived{
+			TLVUserInfo: wire.TLVUserInfo{ScreenName: "Mike Kelly"},
+		}})
+	}
+
+	events := sess.EventQueue.GetAllEvents()
+	require.Len(t, events, 5)
+	for _, event := range events {
+		assert.Equal(t, "MICHAELKELLY", event.Data.(types.PresenceEvent).Friendly)
+	}
+	assert.Equal(t, 1, loads, "aliases should be loaded once, not once per event")
+}
+
+// A feedbag change from another of the owner's clients arrives as a SNAC, which is
+// the session's only signal that its cached aliases are stale.
+func TestWebAPISession_FeedbagSNACInvalidatesAliasCache(t *testing.T) {
+	alias := "MICHAELKELLY"
+	sess := &WebAPISession{
+		ScreenName: DisplayScreenName("me"),
+		Events:     []string{"presence"},
+		EventQueue: types.NewEventQueue(10),
+		logger:     slog.New(slog.NewTextHandler(io.Discard, nil)),
+		BuddyAliasLoader: func(_ context.Context) (map[string]string, error) {
+			return map[string]string{"mikekelly": alias}, nil
+		},
+	}
+
+	arrive := func() types.PresenceEvent {
+		sess.handleBuddyArrived(wire.SNACMessage{Body: wire.SNAC_0x03_0x0B_BuddyArrived{
+			TLVUserInfo: wire.TLVUserInfo{ScreenName: "Mike Kelly"},
+		}})
+		events := sess.EventQueue.GetAllEvents()
+		require.NotEmpty(t, events)
+		return events[len(events)-1].Data.(types.PresenceEvent)
+	}
+
+	assert.Equal(t, "MICHAELKELLY", arrive().Friendly)
+
+	// The buddy is renamed elsewhere: the feedbag SNAC must drop the cached map.
+	alias = "MIKE"
+	sess.handleFeedbagMessage(wire.SNACMessage{
+		Frame: wire.SNACFrame{FoodGroup: wire.Feedbag, SubGroup: wire.FeedbagUpdateItem},
+		Body:  wire.SNAC_0x13_0x09_FeedbagUpdateItem{},
+	})
+
+	assert.Equal(t, "MIKE", arrive().Friendly)
+}
+
+// A session sees no SNAC for feedbag writes it makes itself, so the handlers that
+// perform those writes invalidate the cache directly.
+func TestWebAPISession_InvalidateAliases(t *testing.T) {
+	alias := "MICHAELKELLY"
+	sess := &WebAPISession{
+		logger: slog.New(slog.NewTextHandler(io.Discard, nil)),
+		BuddyAliasLoader: func(_ context.Context) (map[string]string, error) {
+			return map[string]string{"mikekelly": alias}, nil
+		},
+	}
+
+	assert.Equal(t, "MICHAELKELLY", sess.Aliases(context.Background())["mikekelly"])
+
+	alias = "MIKE"
+	assert.Equal(t, "MICHAELKELLY", sess.Aliases(context.Background())["mikekelly"], "cached until invalidated")
+
+	sess.InvalidateAliases()
+	assert.Equal(t, "MIKE", sess.Aliases(context.Background())["mikekelly"])
+}
+
+// A failed load must not be cached as an empty map: aliases would stay missing for
+// the life of the session.
+func TestWebAPISession_AliasLoadErrorIsNotCached(t *testing.T) {
+	var loads int
+	sess := &WebAPISession{
+		logger: slog.New(slog.NewTextHandler(io.Discard, nil)),
+		BuddyAliasLoader: func(_ context.Context) (map[string]string, error) {
+			loads++
+			if loads == 1 {
+				return nil, io.EOF
+			}
+			return map[string]string{"mikekelly": "MICHAELKELLY"}, nil
+		},
+	}
+
+	assert.Empty(t, sess.Aliases(context.Background()))
+	assert.Equal(t, "MICHAELKELLY", sess.Aliases(context.Background())["mikekelly"])
+}
+
+func TestWebAPISession_HandleIncomingIM_NormalizesAimID(t *testing.T) {
+	sess := &WebAPISession{
+		ScreenName: DisplayScreenName("me"),
+		Events:     []string{"im", "conversation"},
+		EventQueue: types.NewEventQueue(10),
+		logger:     slog.New(slog.NewTextHandler(io.Discard, nil)),
+	}
+
+	frags, err := wire.ICBMFragmentList("hello")
+	assert.NoError(t, err)
+
+	body := wire.SNAC_0x04_0x07_ICBMChannelMsgToClient{
+		ChannelID:   wire.ICBMChannelIM,
+		TLVUserInfo: wire.TLVUserInfo{ScreenName: "Mike Kelly"},
+	}
+	body.Append(wire.NewTLVBE(wire.ICBMTLVAOLIMData, frags))
+
+	sess.handleIncomingIM(wire.SNACMessage{Body: body})
+
+	events := sess.EventQueue.GetAllEvents()
+	require.Len(t, events, 2)
+
+	imEvent := events[0].Data.(types.IMEvent)
+	assert.Equal(t, "mikekelly", imEvent.Source.AimID)
+	assert.Equal(t, "Mike Kelly", imEvent.Source.DisplayID)
+
+	convData := events[1].Data.(map[string]interface{})
+	entries := convData["conversations"].([]map[string]interface{})
+	require.Len(t, entries, 1)
+	assert.Equal(t, "mikekelly", entries[0]["aimId"])
+	assert.Equal(t, "Mike Kelly", entries[0]["displayId"])
+	assert.Equal(t, "mikekelly", entries[0]["lastIM"].(map[string]interface{})["sender"])
+
+	// The IM log is keyed by aimId, so the conversation the client opens from
+	// this event finds its own history.
+	msgs := sess.GetStoredIMs(StoredIMQuery{PartnerAimID: "mikekelly", NToGet: 10})
+	require.Len(t, msgs, 1)
+	assert.Equal(t, "hello", msgs[0]["message"])
+}
+
+func TestWebAPISession_HandleTypingNotification_NormalizesAimID(t *testing.T) {
+	sess := &WebAPISession{
+		Events:     []string{"typing"},
+		EventQueue: types.NewEventQueue(10),
+	}
+
+	sess.handleTypingNotification(wire.SNACMessage{
+		Body: wire.SNAC_0x04_0x14_ICBMClientEvent{
+			ScreenName: "Mike Kelly",
+			Event:      0x0002,
+		},
+	})
+
+	events := sess.EventQueue.GetAllEvents()
+	require.Len(t, events, 1)
+	typing := events[0].Data.(types.TypingEvent)
+	assert.Equal(t, "mikekelly", typing.AimID)
+	assert.Equal(t, "typing", typing.TypingStatus)
+}
+
+func TestWebAPISession_HandleBuddyArrivedDeparted_NormalizesAimID(t *testing.T) {
+	sess := &WebAPISession{
+		Events:     []string{"presence"},
+		EventQueue: types.NewEventQueue(10),
+	}
+
+	sess.handleBuddyArrived(wire.SNACMessage{
+		Body: wire.SNAC_0x03_0x0B_BuddyArrived{
+			TLVUserInfo: wire.TLVUserInfo{ScreenName: "Mike Kelly"},
+		},
+	})
+	sess.handleBuddyDeparted(wire.SNACMessage{
+		Body: wire.SNAC_0x03_0x0C_BuddyDeparted{
+			TLVUserInfo: wire.TLVUserInfo{ScreenName: "Mike Kelly"},
+		},
+	})
+
+	events := sess.EventQueue.GetAllEvents()
+	require.Len(t, events, 2)
+
+	arrived := events[0].Data.(types.PresenceEvent)
+	assert.Equal(t, "mikekelly", arrived.AimID)
+	assert.Equal(t, "online", arrived.State)
+
+	departed := events[1].Data.(types.PresenceEvent)
+	assert.Equal(t, "mikekelly", departed.AimID)
+	assert.Equal(t, "offline", departed.State)
+}