issue #195 - fix TOC1 sign-on not announced to OSCAR buddies

Remove the deferred presence mechanism that held back the sign-on
broadcast until contacts were initialized.

ICQ 5 calls FeedbagUse after ClientOnline, so its feedbag privacy settings
are not in effect at ClientOnline. For client ID "ICQBasic", ClientOnline
skips the presence broadcast and FeedbagUse sends it instead.

SetUserInfoFields no longer broadcasts presence before sign-on completes.

foodgroup/buddy.go

@@ -91,8 +91,6 @@ 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,7 +344,6 @@ 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

@@ -564,8 +564,8 @@ func (s *FeedbagService) EndCluster(ctx context.Context, instance *state.Session
 }
 
 // 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.
+// For ICQBasic clients that have already sent ClientOnline, it also performs
+// the initial buddy presence broadcast.
 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)
@@ -576,10 +576,9 @@ func (s *FeedbagService) Use(ctx context.Context, instance *state.SessionInstanc
 	}
 	setSessionBuddyPrefs(items, instance)
 	instance.Session().SetUsesFeedbag()
-	instance.SetContactsInit()
+	instance.SetFeedbagActive()
 
-	// ICQ Lite order: ClientOnline before FeedbagUse — broadcast here.
-	if instance.SignonComplete() {
+	if isICQBasic(instance) && instance.SignonComplete() {
 		if err := s.buddyBroadcaster.BroadcastVisibility(ctx, instance, nil, false); err != nil {
 			return fmt.Errorf("buddyBroadcaster.BroadcastVisibility: %w", err)
 		}

foodgroup/feedbag_test.go

@@ -2991,8 +2991,8 @@ func TestFeedbagService_Use(t *testing.T) {
 				},
 			},
 			checkSession: func(t *testing.T, instance *state.SessionInstance) {
-				assert.True(t, instance.ContactsInit())
 				assert.True(t, instance.Session().UsesFeedbag())
+				assert.True(t, instance.FeedbagActive())
 				assert.False(t, instance.Session().TypingEventsEnabled())
 			},
 		},
@@ -3022,8 +3022,8 @@ func TestFeedbagService_Use(t *testing.T) {
 				},
 			},
 			checkSession: func(t *testing.T, instance *state.SessionInstance) {
-				assert.True(t, instance.ContactsInit())
 				assert.True(t, instance.Session().UsesFeedbag())
+				assert.True(t, instance.FeedbagActive())
 				assert.False(t, instance.Session().TypingEventsEnabled())
 			},
 		},
@@ -3053,14 +3053,58 @@ func TestFeedbagService_Use(t *testing.T) {
 				},
 			},
 			checkSession: func(t *testing.T, instance *state.SessionInstance) {
-				assert.True(t, instance.ContactsInit())
 				assert.True(t, instance.Session().UsesFeedbag())
+				assert.True(t, instance.FeedbagActive())
 				assert.True(t, instance.Session().TypingEventsEnabled())
 			},
 		},
 		{
-			name:     "ICQ Lite order: feedbag use after ClientOnline broadcasts presence",
+			name:     "ICQBasic client, feedbag use before ClientOnline does not broadcast presence",
+			instance: newTestInstance("me", sessClientID("ICQBasic")),
+			mockParams: mockParams{
+				feedbagManagerParams: feedbagManagerParams{
+					useParams: useParams{
+						{
+							screenName: state.NewIdentScreenName("me"),
+						},
+					},
+					feedbagParams: feedbagParams{
+						{
+							screenName: state.NewIdentScreenName("me"),
+						},
+					},
+				},
+			},
+			checkSession: func(t *testing.T, instance *state.SessionInstance) {
+				assert.True(t, instance.Session().UsesFeedbag())
+				assert.True(t, instance.FeedbagActive())
+			},
+		},
+		{
+			name:     "non-ICQBasic client, feedbag use after ClientOnline does not broadcast presence",
 			instance: newTestInstance("me", sessOptSignonComplete),
+			mockParams: mockParams{
+				feedbagManagerParams: feedbagManagerParams{
+					useParams: useParams{
+						{
+							screenName: state.NewIdentScreenName("me"),
+						},
+					},
+					feedbagParams: feedbagParams{
+						{
+							screenName: state.NewIdentScreenName("me"),
+						},
+					},
+				},
+			},
+			checkSession: func(t *testing.T, instance *state.SessionInstance) {
+				assert.True(t, instance.Session().UsesFeedbag())
+				assert.True(t, instance.FeedbagActive())
+			},
+		},
+		{
+			name:     "ICQBasic client, feedbag use after ClientOnline broadcasts presence",
+			instance: newTestInstance("me", sessOptSignonComplete, sessClientID("ICQBasic")),
 			mockParams: mockParams{
 				feedbagManagerParams: feedbagManagerParams{
 					useParams: useParams{
@@ -3085,13 +3129,13 @@ func TestFeedbagService_Use(t *testing.T) {
 				},
 			},
 			checkSession: func(t *testing.T, instance *state.SessionInstance) {
-				assert.True(t, instance.ContactsInit())
 				assert.True(t, instance.Session().UsesFeedbag())
+				assert.True(t, instance.FeedbagActive())
 			},
 		},
 		{
-			name:     "ICQ Lite order: feedbag use after ClientOnline, BroadcastVisibility fails",
-			instance: newTestInstance("me", sessOptSignonComplete),
+			name:     "ICQBasic client, feedbag use after ClientOnline, BroadcastVisibility fails",
+			instance: newTestInstance("me", sessOptSignonComplete, sessClientID("ICQBasic")),
 			mockParams: mockParams{
 				feedbagManagerParams: feedbagManagerParams{
 					useParams: useParams{
@@ -3118,8 +3162,8 @@ func TestFeedbagService_Use(t *testing.T) {
 			},
 			wantErr: assert.AnError,
 			checkSession: func(t *testing.T, instance *state.SessionInstance) {
-				assert.True(t, instance.ContactsInit())
 				assert.True(t, instance.Session().UsesFeedbag())
+				assert.True(t, instance.FeedbagActive())
 			},
 		},
 	}

foodgroup/helpers_test.go

@@ -863,16 +863,16 @@ func sessOptIdle(dur time.Duration) func(instance *state.SessionInstance) {
 	}
 }
 
+// sessOptFeedbagActive sets the feedbag active flag to true
+func sessOptFeedbagActive(instance *state.SessionInstance) {
+	instance.SetFeedbagActive()
+}
+
 // sessOptSignonComplete sets the sign on complete flag to true
 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

@@ -310,7 +310,8 @@ func (s OServiceService) SetUserInfoFields(ctx context.Context, instance *state.
 		instance.SetICQDCInfo(dc)
 	}
 
-	if statusChanged || statusMsgChanged {
+	// presence is announced at sign-on, so don't broadcast changes before then
+	if (statusChanged || statusMsgChanged) && instance.SignonComplete() {
 		if instance.Session().Invisible() {
 			if err := s.buddyBroadcaster.BroadcastBuddyDeparted(ctx, instance.IdentScreenName()); err != nil {
 				return err
@@ -764,10 +765,9 @@ 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,
-//     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.
+//   - Announce current user's arrival to users who have the current user on their buddy list.
+//     ICQBasic (i.e. ICQ 5) clients that have not yet activated their feedbag get the initial
+//     broadcast from FeedbagService.Use instead.
 //
 // If Chat:
 //   - Send current user the chat room metadata
@@ -778,8 +778,7 @@ func (s OServiceService) ClientOnline(ctx context.Context, service uint16, inBod
 
 	switch service {
 	case wire.BOS:
-		// AIM order: feedbag or client-side buddy list before ClientOnline.
-		if instance.ContactsInit() {
+		if !isICQBasic(instance) || instance.FeedbagActive() {
 			if err := s.buddyBroadcaster.BroadcastVisibility(ctx, instance, nil, false); err != nil {
 				return fmt.Errorf("unable to send buddy arrival notification: %w", err)
 			}
@@ -847,6 +846,20 @@ func (s OServiceService) ClientOnline(ctx context.Context, service uint16, inBod
 	}
 }
 
+// isICQBasic reports whether the client is ICQBasic (ICQ 5), which is the only
+// official client that calls FeedbagUse -after- ClientOnline. ClientOnline is
+// the method that reports presence to users. If FeedbagUse has not yet been
+// called, ClientOnline operates on the default privacy settings. That means
+// blocked users could get a presence notification without any mitigations in
+// place.
+//
+// To mitigate what is arguably a bug in ICQ 5, ClientOnline refrains from
+// sending presence if the client ID is "ICQBasic". Presence is finally sent
+// on a subsequent call to FeedbagUse.
+func isICQBasic(instance *state.SessionInstance) bool {
+	return instance.ClientID() == "ICQBasic"
+}
+
 // sendOfflineMessageNotification sends an IM notifying the user of their
 // offline message count and resets the count to zero.
 func (s OServiceService) sendOfflineMessageNotification(ctx context.Context, instance *state.SessionInstance) error {

foodgroup/oservice_test.go

@@ -1094,7 +1094,7 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 	}{
 		{
 			name:     "set user status to visible aim < 6",
-			instance: newTestInstance("me", sessOptInvisible),
+			instance: newTestInstance("me", sessOptSignonComplete, sessOptInvisible),
 			inputSNAC: wire.SNACMessage{
 				Frame: wire.SNACFrame{
 					RequestID: 1234,
@@ -1139,9 +1139,47 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 				assert.False(t, session.Invisible())
 			},
 		},
+		{
+			name:     "set user status to visible before sign-on, no arrival broadcast",
+			instance: newTestInstance("me", sessOptInvisible),
+			inputSNAC: wire.SNACMessage{
+				Frame: wire.SNACFrame{
+					RequestID: 1234,
+				},
+				Body: wire.SNAC_0x01_0x1E_OServiceSetUserInfoFields{
+					TLVRestBlock: wire.TLVRestBlock{
+						TLVList: wire.TLVList{
+							wire.NewTLVBE(wire.OServiceUserInfoStatus, uint32(0x0000)),
+						},
+					},
+				},
+			},
+			expectOutput: wire.SNACMessage{
+				Frame: wire.SNACFrame{
+					FoodGroup: wire.OService,
+					SubGroup:  wire.OServiceUserInfoUpdate,
+					// Pushes are unsolicited, so they carry the server request ID.
+					RequestID: wire.ReqIDFromServer,
+				},
+				Body: func(val any) bool {
+					snac, ok := val.(wire.SNAC_0x01_0x0F_OServiceUserInfoUpdate)
+					if !ok {
+						return false
+					}
+					if len(snac.UserInfo) == 0 {
+						return false
+					}
+					status, hasStatus := snac.UserInfo[0].Uint32BE(wire.OServiceUserInfoStatus)
+					return hasStatus && status == uint32(0x0000)
+				},
+			},
+			checkSession: func(t *testing.T, session *state.Session) {
+				assert.False(t, session.Invisible())
+			},
+		},
 		{
 			name:     "set user status to invisible aim < 6",
-			instance: newTestInstance("me"),
+			instance: newTestInstance("me", sessOptSignonComplete),
 			inputSNAC: wire.SNACMessage{
 				Frame: wire.SNACFrame{
 					RequestID: 1234,
@@ -1186,9 +1224,47 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 				assert.True(t, session.Invisible())
 			},
 		},
+		{
+			name:     "set user status to invisible before sign-on, no departure broadcast",
+			instance: newTestInstance("me"),
+			inputSNAC: wire.SNACMessage{
+				Frame: wire.SNACFrame{
+					RequestID: 1234,
+				},
+				Body: wire.SNAC_0x01_0x1E_OServiceSetUserInfoFields{
+					TLVRestBlock: wire.TLVRestBlock{
+						TLVList: wire.TLVList{
+							wire.NewTLVBE(wire.OServiceUserInfoStatus, uint32(0x0100)),
+						},
+					},
+				},
+			},
+			expectOutput: wire.SNACMessage{
+				Frame: wire.SNACFrame{
+					FoodGroup: wire.OService,
+					SubGroup:  wire.OServiceUserInfoUpdate,
+					// Pushes are unsolicited, so they carry the server request ID.
+					RequestID: wire.ReqIDFromServer,
+				},
+				Body: func(val any) bool {
+					snac, ok := val.(wire.SNAC_0x01_0x0F_OServiceUserInfoUpdate)
+					if !ok {
+						return false
+					}
+					if len(snac.UserInfo) == 0 {
+						return false
+					}
+					status, hasStatus := snac.UserInfo[0].Uint32BE(wire.OServiceUserInfoStatus)
+					return hasStatus && status == uint32(0x0100)
+				},
+			},
+			checkSession: func(t *testing.T, session *state.Session) {
+				assert.True(t, session.Invisible())
+			},
+		},
 		{
 			name:     "set user status to visible aim >= 6",
-			instance: newTestInstance("me", sessOptInvisible, sessOptSetFoodGroupVersion(wire.OService, 4)),
+			instance: newTestInstance("me", sessOptSignonComplete, sessOptInvisible, sessOptSetFoodGroupVersion(wire.OService, 4)),
 			inputSNAC: wire.SNACMessage{
 				Frame: wire.SNACFrame{
 					RequestID: 1234,
@@ -1235,7 +1311,7 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 		},
 		{
 			name:     "set user status to invisible aim >= 6",
-			instance: newTestInstance("me", sessOptSetFoodGroupVersion(wire.OService, 4)),
+			instance: newTestInstance("me", sessOptSignonComplete, sessOptSetFoodGroupVersion(wire.OService, 4)),
 			inputSNAC: wire.SNACMessage{
 				Frame: wire.SNACFrame{
 					RequestID: 1234,
@@ -1282,7 +1358,7 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 		},
 		{
 			name:     "set busy status raises the unavailable flag",
-			instance: newTestInstance("me"),
+			instance: newTestInstance("me", sessOptSignonComplete),
 			inputSNAC: wire.SNACMessage{
 				Frame: wire.SNACFrame{
 					RequestID: 1234,
@@ -1326,7 +1402,7 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 		},
 		{
 			name:     "set away status raises the unavailable flag",
-			instance: newTestInstance("me"),
+			instance: newTestInstance("me", sessOptSignonComplete),
 			inputSNAC: wire.SNACMessage{
 				Frame: wire.SNACFrame{
 					RequestID: 1234,
@@ -1374,7 +1450,7 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 		},
 		{
 			name:     "set DND status raises the unavailable flag",
-			instance: newTestInstance("me"),
+			instance: newTestInstance("me", sessOptSignonComplete),
 			inputSNAC: wire.SNACMessage{
 				Frame: wire.SNACFrame{
 					RequestID: 1234,
@@ -1420,7 +1496,7 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 		},
 		{
 			name:     "set out status raises the unavailable flag",
-			instance: newTestInstance("me"),
+			instance: newTestInstance("me", sessOptSignonComplete),
 			inputSNAC: wire.SNACMessage{
 				Frame: wire.SNACFrame{
 					RequestID: 1234,
@@ -1468,7 +1544,7 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 			// Clients send combined masks, so an unavailable bit alongside invisible
 			// raises the flag and still hides the user.
 			name:     "set away and invisible status in one bitmask",
-			instance: newTestInstance("me"),
+			instance: newTestInstance("me", sessOptSignonComplete),
 			inputSNAC: wire.SNACMessage{
 				Frame: wire.SNACFrame{
 					RequestID: 1234,
@@ -1515,7 +1591,7 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 			// The away message rides along with the status bitmask that set it, so
 			// clearing the status clears the message too.
 			name: "clear status lowers the unavailable flag and drops the away message",
-			instance: newTestInstance("me",
+			instance: newTestInstance("me", sessOptSignonComplete,
 				sessOptUserStatusBitmask(wire.OServiceUserStatusAway),
 				sessOptCannedAwayMessage,
 				sessOptUserInfoFlag(wire.OServiceUserFlagUnavailable)),
@@ -1567,7 +1643,7 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 			// An away message set through LocateService.SetInfo owns the flag,
 			// because no unavailable status bit put it there.
 			name: "clear status keeps the unavailable flag of a user with an away message",
-			instance: newTestInstance("me",
+			instance: newTestInstance("me", sessOptSignonComplete,
 				sessOptCannedAwayMessage,
 				sessOptUserInfoFlag(wire.OServiceUserFlagUnavailable)),
 			inputSNAC: wire.SNACMessage{
@@ -1617,7 +1693,7 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 			// The user stays unavailable across the switch, so the away message set
 			// with the old status survives.
 			name: "switch from one unavailable status to another keeps the away message",
-			instance: newTestInstance("me",
+			instance: newTestInstance("me", sessOptSignonComplete,
 				sessOptUserStatusBitmask(wire.OServiceUserStatusBusy),
 				sessOptCannedAwayMessage,
 				sessOptUserInfoFlag(wire.OServiceUserFlagUnavailable)),
@@ -1670,7 +1746,7 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 			// Invisible is not an unavailable status, so going invisible from away
 			// lowers the flag and drops the message.
 			name: "swap an unavailable status for invisible drops the away message",
-			instance: newTestInstance("me",
+			instance: newTestInstance("me", sessOptSignonComplete,
 				sessOptUserStatusBitmask(wire.OServiceUserStatusAway),
 				sessOptCannedAwayMessage,
 				sessOptUserInfoFlag(wire.OServiceUserFlagUnavailable)),
@@ -1722,7 +1798,7 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 			// Away state belongs to the status TLV, so a request without one leaves
 			// the flag and the away message alone.
 			name: "set status message",
-			instance: newTestInstance("me",
+			instance: newTestInstance("me", sessOptSignonComplete,
 				sessOptCannedAwayMessage,
 				sessOptUserInfoFlag(wire.OServiceUserFlagUnavailable)),
 			inputSNAC: wire.SNACMessage{
@@ -1784,7 +1860,7 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 			// riding in the BART TLV: a client drops the status it shows for a buddy
 			// only when it is sent an empty one.
 			name:     "clear status message",
-			instance: newTestInstance("me", sessOptStatus(newTestStatusBARTID(t, "old"))),
+			instance: newTestInstance("me", sessOptSignonComplete, sessOptStatus(newTestStatusBARTID(t, "old"))),
 			inputSNAC: wire.SNACMessage{
 				Frame: wire.SNACFrame{
 					RequestID: 1234,
@@ -1840,7 +1916,7 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 			// Two changed fields, one arrival: the client would otherwise see the
 			// buddy blink twice.
 			name:     "status bitmask and status message in one request",
-			instance: newTestInstance("me"),
+			instance: newTestInstance("me", sessOptSignonComplete),
 			inputSNAC: wire.SNACMessage{
 				Frame: wire.SNACFrame{
 					RequestID: 1234,
@@ -1890,7 +1966,7 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 			// A client can set both in one request, and the status must survive the
 			// icon that precedes it in the list.
 			name:     "set status message alongside a buddy icon",
-			instance: newTestInstance("me"),
+			instance: newTestInstance("me", sessOptSignonComplete),
 			inputSNAC: wire.SNACMessage{
 				Frame: wire.SNACFrame{
 					RequestID: 1234,
@@ -1943,7 +2019,7 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 			// A buddy icon reference travels in the same TLV, and only the BART
 			// service stores those. Nothing changed here, so buddies hear nothing.
 			name:     "ignore a BART item that is not a status message",
-			instance: newTestInstance("me"),
+			instance: newTestInstance("me", sessOptSignonComplete),
 			inputSNAC: wire.SNACMessage{
 				Frame: wire.SNACFrame{
 					RequestID: 1234,
@@ -1981,7 +2057,7 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 		},
 		{
 			name:     "malformed BART item",
-			instance: newTestInstance("me"),
+			instance: newTestInstance("me", sessOptSignonComplete),
 			inputSNAC: wire.SNACMessage{
 				Frame: wire.SNACFrame{
 					RequestID: 1234,
@@ -2002,7 +2078,7 @@ func TestOServiceService_SetUserInfoFields(t *testing.T) {
 		},
 		{
 			name:     "set ICQ direct connect info",
-			instance: newTestInstance("1000003", sessOptUserInfoFlag(wire.OServiceUserFlagICQ)),
+			instance: newTestInstance("1000003", sessOptSignonComplete, sessOptUserInfoFlag(wire.OServiceUserFlagICQ)),
 			inputSNAC: wire.SNACMessage{
 				Frame: wire.SNACFrame{
 					RequestID: 1234,
@@ -2119,7 +2195,7 @@ func TestOServiceService_SetUserInfoFields_ClearUnavailableStatus(t *testing.T)
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			instance := newTestInstance("me",
+			instance := newTestInstance("me", sessOptSignonComplete,
 				sessOptUserStatusBitmask(tt.status),
 				sessOptCannedAwayMessage,
 				sessOptUserInfoFlag(wire.OServiceUserFlagUnavailable))
@@ -3318,7 +3394,7 @@ func TestOServiceService_ClientOnline(t *testing.T) {
 	}{
 		{
 			name:     "notify that BOS user is online",
-			instance: newTestInstance("me", sessOptCannedSignonTime, sessOptContactsInit),
+			instance: newTestInstance("me", sessOptCannedSignonTime),
 			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
 			service:  wire.BOS,
 			mockParams: mockParams{
@@ -3354,8 +3430,45 @@ func TestOServiceService_ClientOnline(t *testing.T) {
 			},
 		},
 		{
-			name:     "ICQ Lite order: ClientOnline before feedbag use",
-			instance: newTestInstance("me", sessOptCannedSignonTime),
+			name:     "ICQBasic client, feedbag already active, notify that BOS user is online",
+			instance: newTestInstance("me", sessOptCannedSignonTime, sessClientID("ICQBasic"), sessOptFeedbagActive),
+			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
+			service:  wire.BOS,
+			mockParams: mockParams{
+				buddyBroadcasterParams: buddyBroadcasterParams{
+					broadcastVisibilityParams: broadcastVisibilityParams{
+						{
+							from:             state.NewIdentScreenName("me"),
+							filter:           nil,
+							doSendDepartures: false,
+						},
+					},
+				},
+				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())
+			},
+		},
+		{
+			name:     "ICQBasic client, feedbag not active, defer buddy broadcast to feedbag use",
+			instance: newTestInstance("me", sessOptCannedSignonTime, sessClientID("ICQBasic")),
 			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
 			service:  wire.BOS,
 			mockParams: mockParams{
@@ -3379,12 +3492,11 @@ func TestOServiceService_ClientOnline(t *testing.T) {
 			},
 			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, sessOptContactsInit),
+			instance: newTestInstance("me", sessOptCannedSignonTime, sessOptKerberosAuth),
 			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
 			service:  wire.BOS,
 			mockParams: mockParams{
@@ -3430,7 +3542,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, sessOptContactsInit),
+			instance: newTestInstance("me", sessOptCannedSignonTime, sessOptKerberosAuth),
 			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
 			service:  wire.BOS,
 			mockParams: mockParams{
@@ -3495,7 +3607,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), sessOptContactsInit),
+			instance: newTestInstance("me", sessOptCannedSignonTime, sessOptOfflineMsgCount(0)),
 			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
 			service:  wire.BOS,
 			mockParams: mockParams{
@@ -3533,7 +3645,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), sessOptContactsInit),
+			instance: newTestInstance("me", sessOptCannedSignonTime, sessOptOfflineMsgCount(3)),
 			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
 			service:  wire.BOS,
 			mockParams: mockParams{
@@ -3590,7 +3702,7 @@ func TestOServiceService_ClientOnline(t *testing.T) {
 		},
 		{
 			name:     "ICQ user with offline messages, no offline message notification sent",
-			instance: newTestInstance("100001", sessOptCannedSignonTime, sessOptOfflineMsgCount(3), sessOptContactsInit, sessOptUIN(100001)),
+			instance: newTestInstance("100001", sessOptCannedSignonTime, sessOptOfflineMsgCount(3), sessOptUIN(100001)),
 			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
 			service:  wire.BOS,
 			mockParams: mockParams{
@@ -3635,7 +3747,6 @@ 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
 			}(),
@@ -3693,7 +3804,6 @@ 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
 			}(),
@@ -3737,7 +3847,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), sessOptContactsInit),
+			instance: newTestInstance("me", sessOptCannedSignonTime, sessOptOfflineMsgCount(2)),
 			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
 			service:  wire.BOS,
 			wantErr:  assert.AnError,
@@ -4616,7 +4726,7 @@ func TestOServiceService_SetUserInfoFields_MultiInstance(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			sender := newTestInstance("me")
+			sender := newTestInstance("me", sessOptSignonComplete)
 			other := sender.Session().AddInstance()
 
 			messageRelayer := newMockMessageRelayer(t)

state/session.go

@@ -1000,6 +1000,7 @@ type SessionInstance struct {
 	// Per-session connection state
 	remoteAddr     *netip.AddrPort
 	signonComplete bool
+	feedbagActive  bool
 	closed         bool
 	stopCh         chan struct{}
 	msgCh          chan wire.SNACMessage
@@ -1020,8 +1021,6 @@ type SessionInstance struct {
 	awayMsg           string
 	userInfoBitmask   uint16
 	userStatusBitmask uint32
-	// contactsInit indicates whether the client-side buddy list or feedbag has been initialized
-	contactsInit bool
 
 	// notify transaction: accumulate visibility notification targets during feedbag batches
 	notifyTxnActive     bool
@@ -1166,6 +1165,20 @@ func (s *SessionInstance) SignonComplete() bool {
 	return s.signonComplete
 }
 
+// SetFeedbagActive indicates that the instance has activated its feedbag.
+func (s *SessionInstance) SetFeedbagActive() {
+	s.mutex.Lock()
+	defer s.mutex.Unlock()
+	s.feedbagActive = true
+}
+
+// FeedbagActive indicates whether the instance has activated its feedbag.
+func (s *SessionInstance) FeedbagActive() bool {
+	s.mutex.RLock()
+	defer s.mutex.RUnlock()
+	return s.feedbagActive
+}
+
 // UnsetIdle removes the instance's idle state.
 func (s *SessionInstance) UnsetIdle() {
 	s.mutex.Lock()
@@ -1271,22 +1284,6 @@ 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
-}
-
 // BeginNotifyTxn starts accumulating notification targets during feedbag transactions.
 func (s *SessionInstance) BeginNotifyTxn() {
 	s.mutex.Lock()

state/session_test.go

@@ -940,6 +940,14 @@ func TestSession_SetAndGetMultiConnFlag(t *testing.T) {
 	assert.Equal(t, wire.MultiConnFlagsSingleClient, s.MultiConnFlag())
 }
 
+func TestSessionInstance_FeedbagActive(t *testing.T) {
+	instance := NewSession().AddInstance()
+	assert.False(t, instance.FeedbagActive())
+
+	instance.SetFeedbagActive()
+	assert.True(t, instance.FeedbagActive())
+}
+
 func TestSession_SetAndGetLastWarnLevel(t *testing.T) {
 	s := NewSession().AddInstance()
 	assert.Zero(t, s.Warning())
@@ -949,17 +957,6 @@ 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