fix ICQ 5 presence not firing at login

ICQ Lite (ICQ 5) sends OService ClientOnline before FeedbagUse, so the
server was broadcasting arrival before the feedbag was active and the
client ignored buddy presence updates.

Track per-instance contacts initialization (feedbag Use or client-side
AddBuddies). Defer the initial BroadcastVisibility from ClientOnline until
contacts are ready; run it from FeedbagService.Use when sign-on already
completed.

When the client re-upserts a buddy with FeedbagAttributesPending after
receiving a "you were authorized" notification, strip the pending tag if
authorization is already recorded instead of storing the contact as still
pending.

foodgroup/buddy.go

@@ -91,6 +91,8 @@ func (s BuddyService) AddBuddies(ctx context.Context, instance *state.SessionIns
 		added = append(added, them)
 	}
 
+	instance.SetContactsInit()
+
 	if !instance.SignonComplete() {
 		// client has not completed sign-on sequence, so any arrival
 		// messages sent at this point would be ignored by the client.

foodgroup/buddy_test.go

@@ -344,6 +344,7 @@ func TestBuddyService_AddBuddies(t *testing.T) {
 			rejectSNAC, haveErr := svc.AddBuddies(context.Background(), tt.instance, wire.SNACFrame{RequestID: 42}, tt.bodyIn)
 			assert.Equal(t, tt.wantSNAC, rejectSNAC)
 			assert.ErrorIs(t, haveErr, tt.wantErr)
+			assert.True(t, tt.instance.ContactsInit(), "AddBuddies marks client-side buddy list loaded")
 		})
 	}
 }

foodgroup/feedbag.go

@@ -222,7 +222,7 @@ func (s *FeedbagService) UpsertItem(ctx context.Context, instance *state.Session
 	authRequired := make(map[string]bool)
 
 	for _, item := range items {
-		if item.ClassID == wire.FeedbagClassIdBuddy && !item.HasTag(wire.FeedbagAttributesPending) {
+		if item.ClassID == wire.FeedbagClassIdBuddy {
 			sn := state.NewIdentScreenName(item.Name)
 			switch {
 			case instance.UIN() != 0 && sn.UIN() == 0:
@@ -239,9 +239,17 @@ func (s *FeedbagService) UpsertItem(ctx context.Context, instance *state.Session
 				if err != nil {
 					return nil, fmt.Errorf("contactPreAuthorizer.RequiresAuthorization: %w", err)
 				}
-				if blocked {
+				hasPending := item.HasTag(wire.FeedbagAttributesPending)
+				if blocked && !hasPending {
 					authRequired[item.Name] = true
 					continue
+				} else if hasPending && !blocked {
+					// Authorization has since been granted (IServerd strips
+					// SSI_TLV_AUTH before inserting when auth is already recorded).
+					// Clear the pending flag so the row is stored as a normal buddy.
+					item.TLVList = slices.DeleteFunc(item.TLVList, func(t wire.TLV) bool {
+						return t.Tag == wire.FeedbagAttributesPending
+					})
 				}
 			case instance.UIN() == 0 && sn.UIN() != 0:
 				// sender:aim, recipient: icq
@@ -495,9 +503,9 @@ func (s *FeedbagService) EndCluster(ctx context.Context, instance *state.Session
 	})
 }
 
-// Use sends a user the contents of their buddy list. It's invoked at sign-on
-// by AIM clients that use the feedbag food group for buddy list management (as
-// opposed to client-side management).
+// Use activates server-side buddy list state for feedbag clients at sign-on.
+// FeedbagUse and ClientOnline can arrive in either order; whichever handler runs
+// second performs the initial buddy presence broadcast when both are satisfied.
 func (s *FeedbagService) Use(ctx context.Context, instance *state.SessionInstance) error {
 	if err := s.feedbagManager.UseFeedbag(ctx, instance.IdentScreenName()); err != nil {
 		return fmt.Errorf("could not use feedbag: %w", err)
@@ -508,6 +516,15 @@ func (s *FeedbagService) Use(ctx context.Context, instance *state.SessionInstanc
 	}
 	setSessionBuddyPrefs(items, instance)
 	instance.Session().SetUsesFeedbag()
+	instance.SetContactsInit()
+
+	// ICQ Lite order: ClientOnline before FeedbagUse — broadcast here.
+	if instance.SignonComplete() {
+		if err := s.buddyBroadcaster.BroadcastVisibility(ctx, instance, nil, false); err != nil {
+			return fmt.Errorf("buddyBroadcaster.BroadcastVisibility: %w", err)
+		}
+	}
+
 	return nil
 }
 

foodgroup/feedbag_test.go

@@ -2213,6 +2213,138 @@ func TestFeedbagService_UpsertItem(t *testing.T) {
 				},
 			},
 		},
+		{
+			// ICQ Lite re-sends a FeedbagUpsert with FeedbagAttributesPending still
+			// set after the user clicks OK on the "you were added" notification that
+			// arrives when the contact grants authorization. IServerd handles this by
+			// stripping SSI_TLV_AUTH from the item before the DB insert (ssi_db.cpp
+			// ssi_db_add_item lines 304-308). We do the same: when the client sends
+			// a pending-tagged item but RequiresAuthorization returns false (auth was
+			// already granted), strip the tag and treat the item as a normal add.
+			name:     "ICQ user re-upserts ICQ buddy with pending tag after authorization already granted: strip pending tag",
+			instance: newTestInstance("100001", sessOptUIN(100001)),
+			inputSNAC: wire.SNACMessage{
+				Frame: wire.SNACFrame{
+					FoodGroup: wire.Feedbag,
+					SubGroup:  wire.FeedbagInsertItem,
+					RequestID: 1234,
+				},
+				Body: wire.SNAC_0x13_0x08_FeedbagInsertItem{
+					Items: []wire.FeedbagItem{
+						{
+							ClassID: wire.FeedbagClassIdBuddy,
+							Name:    "990011",
+							TLVLBlock: wire.TLVLBlock{
+								TLVList: wire.TLVList{
+									wire.NewTLVBE(wire.FeedbagAttributesPending, []byte{}),
+								},
+							},
+						},
+					},
+				},
+			},
+			mockParams: mockParams{
+				contactPreAuthorizerParams: contactPreAuthorizerParams{
+					requiresAuthorizationParams: requiresAuthorizationParams{
+						{
+							owner:     state.NewIdentScreenName("990011"),
+							requester: state.NewIdentScreenName("100001"),
+							result:    false, // already authorized
+						},
+					},
+				},
+				feedbagManagerParams: feedbagManagerParams{
+					feedbagUpsertParams: feedbagUpsertParams{
+						{
+							screenName: state.NewIdentScreenName("100001"),
+							// pending TLV must be stripped before storage
+							items: []wire.FeedbagItem{
+								{
+									ClassID: wire.FeedbagClassIdBuddy,
+									Name:    "990011",
+									TLVLBlock: wire.TLVLBlock{
+										TLVList: wire.TLVList{},
+									},
+								},
+							},
+						},
+					},
+				},
+				sessionRetrieverParams: sessionRetrieverParams{
+					retrieveSessionParams: retrieveSessionParams{
+						{
+							screenName: state.NewIdentScreenName("990011"),
+							result:     nil,
+						},
+					},
+				},
+				messageRelayerParams: messageRelayerParams{
+					relayToOtherInstancesParams: relayToOtherInstancesParams{
+						{
+							screenName: state.NewIdentScreenName("100001"),
+							message: wire.SNACMessage{
+								Frame: wire.SNACFrame{
+									FoodGroup: wire.Feedbag,
+									SubGroup:  wire.FeedbagInsertItem,
+									RequestID: wire.ReqIDFromServer,
+								},
+								Body: wire.SNAC_0x13_0x09_FeedbagUpdateItem{
+									Items: []wire.FeedbagItem{
+										{
+											ClassID: wire.FeedbagClassIdBuddy,
+											Name:    "990011",
+											TLVLBlock: wire.TLVLBlock{
+												TLVList: wire.TLVList{},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+					relayToSelfParams: relayToSelfParams{
+						{
+							screenName: state.NewIdentScreenName("100001"),
+							message: wire.SNACMessage{
+								Frame: wire.SNACFrame{
+									FoodGroup: wire.Feedbag,
+									SubGroup:  wire.FeedbagStatus,
+									RequestID: 1234,
+								},
+								Body: wire.SNAC_0x13_0x0E_FeedbagStatus{
+									Results: []uint16{0x0000},
+								},
+							},
+						},
+					},
+				},
+				buddyBroadcasterParams: buddyBroadcasterParams{
+					broadcastVisibilityParams: broadcastVisibilityParams{
+						{
+							from: state.NewIdentScreenName("100001"),
+							filter: []state.IdentScreenName{
+								state.NewIdentScreenName("990011"),
+							},
+						},
+					},
+				},
+			},
+			expectOutput: nil,
+			expectICBM:   true,
+			wantICBMBody: wire.SNAC_0x04_0x06_ICBMChannelMsgToHost{
+				ChannelID:  wire.ICBMChannelICQ,
+				ScreenName: "990011",
+				TLVRestBlock: wire.TLVRestBlock{
+					TLVList: wire.TLVList{
+						wire.NewTLVLE(wire.ICBMTLVData, wire.ICBMCh4Message{
+							UIN:         100001,
+							MessageType: wire.ICBMMsgTypeAdded,
+						}),
+						wire.NewTLVBE(wire.ICBMTLVStore, []byte{}),
+					},
+				},
+			},
+		},
 	}
 
 	for _, tc := range cases {
@@ -2468,21 +2600,17 @@ func TestFeedbagService_Use(t *testing.T) {
 		name string
 		// instance is the user's session
 		instance *state.SessionInstance
-		// bodyIn is the SNAC body sent from the arriving user's client to the
-		// server
-		bodyIn wire.SNAC_0x01_0x02_OServiceClientOnline
 		// mockParams is the list of params sent to mocks that satisfy this
 		// method's dependencies
 		mockParams mockParams
-		// checkSession validates the state of the session after the call
-		checkSession func(*testing.T, *state.Session)
+		// checkSession validates session and instance state after the call
+		checkSession func(*testing.T, *state.SessionInstance)
 		// wantErr indicates an error is expected
 		wantErr error
 	}{
 		{
 			name:     "enable user's feedbag, no feedbag buddy params item",
 			instance: newTestInstance("me"),
-			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
 			mockParams: mockParams{
 				feedbagManagerParams: feedbagManagerParams{
 					useParams: useParams{
@@ -2497,15 +2625,15 @@ func TestFeedbagService_Use(t *testing.T) {
 					},
 				},
 			},
-			checkSession: func(t *testing.T, s *state.Session) {
-				assert.True(t, s.UsesFeedbag())
-				assert.False(t, s.TypingEventsEnabled())
+			checkSession: func(t *testing.T, instance *state.SessionInstance) {
+				assert.True(t, instance.ContactsInit())
+				assert.True(t, instance.Session().UsesFeedbag())
+				assert.False(t, instance.Session().TypingEventsEnabled())
 			},
 		},
 		{
 			name:     "enable user's feedbag and set typing events disabled",
 			instance: newTestInstance("me"),
-			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
 			mockParams: mockParams{
 				feedbagManagerParams: feedbagManagerParams{
 					useParams: useParams{
@@ -2530,15 +2658,15 @@ func TestFeedbagService_Use(t *testing.T) {
 					},
 				},
 			},
-			checkSession: func(t *testing.T, s *state.Session) {
-				assert.True(t, s.UsesFeedbag())
-				assert.False(t, s.TypingEventsEnabled())
+			checkSession: func(t *testing.T, instance *state.SessionInstance) {
+				assert.True(t, instance.ContactsInit())
+				assert.True(t, instance.Session().UsesFeedbag())
+				assert.False(t, instance.Session().TypingEventsEnabled())
 			},
 		},
 		{
 			name:     "enable user's feedbag and set typing events enabled",
 			instance: newTestInstance("me"),
-			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
 			mockParams: mockParams{
 				feedbagManagerParams: feedbagManagerParams{
 					useParams: useParams{
@@ -2563,9 +2691,74 @@ func TestFeedbagService_Use(t *testing.T) {
 					},
 				},
 			},
-			checkSession: func(t *testing.T, s *state.Session) {
-				assert.True(t, s.UsesFeedbag())
-				assert.True(t, s.TypingEventsEnabled())
+			checkSession: func(t *testing.T, instance *state.SessionInstance) {
+				assert.True(t, instance.ContactsInit())
+				assert.True(t, instance.Session().UsesFeedbag())
+				assert.True(t, instance.Session().TypingEventsEnabled())
+			},
+		},
+		{
+			name:     "ICQ Lite order: feedbag use after ClientOnline broadcasts presence",
+			instance: newTestInstance("me", sessOptSignonComplete),
+			mockParams: mockParams{
+				feedbagManagerParams: feedbagManagerParams{
+					useParams: useParams{
+						{
+							screenName: state.NewIdentScreenName("me"),
+						},
+					},
+					feedbagParams: feedbagParams{
+						{
+							screenName: state.NewIdentScreenName("me"),
+						},
+					},
+				},
+				buddyBroadcasterParams: buddyBroadcasterParams{
+					broadcastVisibilityParams: broadcastVisibilityParams{
+						{
+							from:             state.NewIdentScreenName("me"),
+							filter:           nil,
+							doSendDepartures: false,
+						},
+					},
+				},
+			},
+			checkSession: func(t *testing.T, instance *state.SessionInstance) {
+				assert.True(t, instance.ContactsInit())
+				assert.True(t, instance.Session().UsesFeedbag())
+			},
+		},
+		{
+			name:     "ICQ Lite order: feedbag use after ClientOnline, BroadcastVisibility fails",
+			instance: newTestInstance("me", sessOptSignonComplete),
+			mockParams: mockParams{
+				feedbagManagerParams: feedbagManagerParams{
+					useParams: useParams{
+						{
+							screenName: state.NewIdentScreenName("me"),
+						},
+					},
+					feedbagParams: feedbagParams{
+						{
+							screenName: state.NewIdentScreenName("me"),
+						},
+					},
+				},
+				buddyBroadcasterParams: buddyBroadcasterParams{
+					broadcastVisibilityParams: broadcastVisibilityParams{
+						{
+							from:             state.NewIdentScreenName("me"),
+							filter:           nil,
+							doSendDepartures: false,
+							err:              assert.AnError,
+						},
+					},
+				},
+			},
+			wantErr: assert.AnError,
+			checkSession: func(t *testing.T, instance *state.SessionInstance) {
+				assert.True(t, instance.ContactsInit())
+				assert.True(t, instance.Session().UsesFeedbag())
 			},
 		},
 	}
@@ -2583,11 +2776,21 @@ func TestFeedbagService_Use(t *testing.T) {
 					Return(params.results, nil)
 			}
 
+			buddyUpdateBroadcaster := newMockbuddyBroadcaster(t)
+			for _, params := range tt.mockParams.broadcastVisibilityParams {
+				buddyUpdateBroadcaster.EXPECT().
+					BroadcastVisibility(matchContext(), matchSession(params.from), params.filter, params.doSendDepartures).
+					Return(params.err)
+			}
+
 			svc := NewFeedbagService(slog.Default(), nil, feedbagManager, nil, nil, nil, nil, nil)
+			svc.buddyBroadcaster = buddyUpdateBroadcaster
 
 			haveErr := svc.Use(context.Background(), tt.instance)
-			assert.ErrorIs(t, tt.wantErr, haveErr)
-			tt.checkSession(t, tt.instance.Session())
+			assert.ErrorIs(t, haveErr, tt.wantErr)
+			if tt.checkSession != nil {
+				tt.checkSession(t, tt.instance)
+			}
 		})
 	}
 }

foodgroup/helpers_test.go

@@ -833,6 +833,11 @@ func sessOptSignonComplete(instance *state.SessionInstance) {
 	instance.SetSignonComplete()
 }
 
+// sessOptContactsInit sets the contacts-init flag to true
+func sessOptContactsInit(instance *state.SessionInstance) {
+	instance.SetContactsInit()
+}
+
 // sessOptCaps sets caps
 func sessOptUIN(UIN uint32) func(instance *state.SessionInstance) {
 	return func(instance *state.SessionInstance) {

foodgroup/oservice.go

@@ -666,7 +666,10 @@ func (s OServiceService) ServiceRequest(ctx context.Context, service uint16, ins
 
 // ClientOnline runs when the current user is ready to join.
 // If BOS:
-//   - Announce current user's arrival to users who have the current user on their buddy list
+//   - Announce current user's arrival to users who have the current user on their buddy list,
+//     but only when the contact list is ready (feedbag initialized or client-side buddy
+//     list loaded). Clients that send ClientOnline before feedbag activation get the
+//     initial broadcast from FeedbagService.Use instead.
 //
 // If Chat:
 //   - Send current user the chat room metadata
@@ -677,8 +680,11 @@ func (s OServiceService) ClientOnline(ctx context.Context, service uint16, inBod
 
 	switch service {
 	case wire.BOS:
-		if err := s.buddyBroadcaster.BroadcastVisibility(ctx, instance, nil, false); err != nil {
-			return fmt.Errorf("unable to send buddy arrival notification: %w", err)
+		// AIM order: feedbag or client-side buddy list before ClientOnline.
+		if instance.ContactsInit() {
+			if err := s.buddyBroadcaster.BroadcastVisibility(ctx, instance, nil, false); err != nil {
+				return fmt.Errorf("unable to send buddy arrival notification: %w", err)
+			}
 		}
 
 		msg := wire.SNACMessage{

foodgroup/oservice_test.go

@@ -2196,7 +2196,7 @@ func TestOServiceService_ClientOnline(t *testing.T) {
 	}{
 		{
 			name:     "notify that BOS user is online",
-			instance: newTestInstance("me", sessOptCannedSignonTime),
+			instance: newTestInstance("me", sessOptCannedSignonTime, sessOptContactsInit),
 			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
 			service:  wire.BOS,
 			mockParams: mockParams{
@@ -2231,9 +2231,38 @@ func TestOServiceService_ClientOnline(t *testing.T) {
 				assert.True(t, instance.SignonComplete())
 			},
 		},
+		{
+			name:     "ICQ Lite order: ClientOnline before feedbag use",
+			instance: newTestInstance("me", sessOptCannedSignonTime),
+			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
+			service:  wire.BOS,
+			mockParams: mockParams{
+				messageRelayerParams: messageRelayerParams{
+					relayToScreenNameParams: relayToScreenNameParams{
+						{
+							screenName: state.NewIdentScreenName("me"),
+							message: wire.SNACMessage{
+								Frame: wire.SNACFrame{
+									FoodGroup: wire.Stats,
+									SubGroup:  wire.StatsSetMinReportInterval,
+									RequestID: wire.ReqIDFromServer,
+								},
+								Body: wire.SNAC_0x0B_0x02_StatsSetMinReportInterval{
+									MinReportInterval: 1,
+								},
+							},
+						},
+					},
+				},
+			},
+			validateSess: func(t *testing.T, instance *state.SessionInstance) {
+				assert.True(t, instance.SignonComplete())
+				assert.False(t, instance.ContactsInit())
+			},
+		},
 		{
 			name:     "notify that BOS user is online via Kerberos auth, does not have stored profile",
-			instance: newTestInstance("me", sessOptCannedSignonTime, sessOptKerberosAuth),
+			instance: newTestInstance("me", sessOptCannedSignonTime, sessOptKerberosAuth, sessOptContactsInit),
 			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
 			service:  wire.BOS,
 			mockParams: mockParams{
@@ -2279,7 +2308,7 @@ func TestOServiceService_ClientOnline(t *testing.T) {
 		},
 		{
 			name:     "notify that BOS user is online via Kerberos auth, has stored profile",
-			instance: newTestInstance("me", sessOptCannedSignonTime, sessOptKerberosAuth),
+			instance: newTestInstance("me", sessOptCannedSignonTime, sessOptKerberosAuth, sessOptContactsInit),
 			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
 			service:  wire.BOS,
 			mockParams: mockParams{
@@ -2349,7 +2378,7 @@ func TestOServiceService_ClientOnline(t *testing.T) {
 		},
 		{
 			name:     "notify that BOS user is online with 0 offline messages, no notification sent",
-			instance: newTestInstance("me", sessOptCannedSignonTime, sessOptOfflineMsgCount(0)),
+			instance: newTestInstance("me", sessOptCannedSignonTime, sessOptOfflineMsgCount(0), sessOptContactsInit),
 			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
 			service:  wire.BOS,
 			mockParams: mockParams{
@@ -2387,7 +2416,7 @@ func TestOServiceService_ClientOnline(t *testing.T) {
 		},
 		{
 			name:     "notify that BOS user is online with offline messages, send notification and reset count",
-			instance: newTestInstance("me", sessOptCannedSignonTime, sessOptOfflineMsgCount(3)),
+			instance: newTestInstance("me", sessOptCannedSignonTime, sessOptOfflineMsgCount(3), sessOptContactsInit),
 			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
 			service:  wire.BOS,
 			mockParams: mockParams{
@@ -2450,6 +2479,7 @@ func TestOServiceService_ClientOnline(t *testing.T) {
 				instance2 := instance1.Session().AddInstance()
 				instance2.SetSignonComplete()
 				instance3 := instance1.Session().AddInstance()
+				instance3.SetContactsInit()
 				// instance 3 is not yet signed on
 				return instance3
 			}(),
@@ -2507,6 +2537,7 @@ func TestOServiceService_ClientOnline(t *testing.T) {
 				instance2 := instance1.Session().AddInstance()
 				instance2.SetSignonComplete()
 				instance3 := instance1.Session().AddInstance()
+				instance3.SetContactsInit()
 				// instance 3 is not yet signed on
 				return instance3
 			}(),
@@ -2550,7 +2581,7 @@ func TestOServiceService_ClientOnline(t *testing.T) {
 		},
 		{
 			name:     "notify that BOS user is online with offline messages, SetOfflineMsgCount fails",
-			instance: newTestInstance("me", sessOptCannedSignonTime, sessOptOfflineMsgCount(2)),
+			instance: newTestInstance("me", sessOptCannedSignonTime, sessOptOfflineMsgCount(2), sessOptContactsInit),
 			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
 			service:  wire.BOS,
 			wantErr:  assert.AnError,

state/session.go

@@ -898,6 +898,8 @@ type SessionInstance struct {
 	awayMsg           string
 	userInfoBitmask   uint16
 	userStatusBitmask uint32
+	// contactsInit indicates whether the client-side buddy list or feedbag has been initialized
+	contactsInit bool
 
 	// Per-session profile
 	profile           UserProfile
@@ -1134,6 +1136,22 @@ func (s *SessionInstance) OnClose(fn func()) {
 	s.onInstanceCloseFn = fn
 }
 
+// ContactsInit returns whether the client-side buddy list has been loaded or
+// the feedbag has been initialized.
+func (s *SessionInstance) ContactsInit() bool {
+	s.mutex.RLock()
+	defer s.mutex.RUnlock()
+	return s.contactsInit
+}
+
+// SetContactsInit indicates that the client-side buddy list has been loaded or
+// the feedbag has been initialized.
+func (s *SessionInstance) SetContactsInit() {
+	s.mutex.Lock()
+	defer s.mutex.Unlock()
+	s.contactsInit = true
+}
+
 // CloseInstance shuts down the instance's ability to relay messages.
 func (s *SessionInstance) closeOnly() {
 	s.mutex.Lock()

state/session_test.go

@@ -723,6 +723,17 @@ func TestSession_SetAndGetLastWarnLevel(t *testing.T) {
 	assert.Equal(t, level, s.Warning())
 }
 
+func TestSessionInstance_ContactsInit(t *testing.T) {
+	instance := NewSession().AddInstance()
+	assert.False(t, instance.ContactsInit())
+
+	instance.SetContactsInit()
+	assert.True(t, instance.ContactsInit())
+
+	instance.SetContactsInit()
+	assert.True(t, instance.ContactsInit())
+}
+
 func TestInstance_Active(t *testing.T) {
 	tests := []struct {
 		name           string