Merge pull request #204 from agessaman/fix/new-contact-known-vs-new

fix(new-contact): distinguish known vs new contacts in NEW_CONTACT handling

modules/message_handler.py

@@ -3605,10 +3605,16 @@ class MessageHandler:
             # Check if this is a repeater or companion
             if hasattr(self.bot, "repeater_manager"):
                 is_repeater = self.bot.repeater_manager._is_repeater_device(contact_data)
+                existing_tracking = self.bot.repeater_manager.get_tracked_contact_row(public_key)
+                already_on_device = self.bot.repeater_manager.is_contact_on_device(public_key)
+                known_contact = already_on_device or existing_tracking is not None
 
                 if is_repeater:
                     # REPEATER: Track directly in SQLite database (no device contact list)
-                    self.logger.info(f"📡 New repeater discovered: {contact_name} - tracking in database only")
+                    if known_contact:
+                        self.logger.info(f"📡 Known repeater advert: {contact_name} - tracking in database only")
+                    else:
+                        self.logger.info(f"📡 New repeater discovered: {contact_name} - tracking in database only")
 
                     # Track repeater in complete database with signal info
                     await self.bot.repeater_manager.track_contact_advertisement(
@@ -3640,11 +3646,18 @@ class MessageHandler:
                 else:
                     # COMPANION: track in DB; device add behaviour depends on auto_manage_contacts
                     auto_manage_setting = self.bot.config.get("Bot", "auto_manage_contacts", fallback="false").lower()
-                    self.logger.info(
-                        "👤 New companion discovered: %s — auto_manage_contacts=%s",
-                        contact_name,
-                        auto_manage_setting,
-                    )
+                    if known_contact:
+                        self.logger.info(
+                            "👤 Known companion advert: %s — auto_manage_contacts=%s",
+                            contact_name,
+                            auto_manage_setting,
+                        )
+                    else:
+                        self.logger.info(
+                            "👤 New companion discovered: %s — auto_manage_contacts=%s",
+                            contact_name,
+                            auto_manage_setting,
+                        )
 
                     track_result = await self.bot.repeater_manager.track_contact_advertisement(
                         contact_data, signal_info, packet_hash=packet_hash
@@ -3669,7 +3682,7 @@ class MessageHandler:
                             await self.bot.repeater_manager.manage_contact_list(auto_cleanup=True)
                         else:
                             self.logger.info(
-                                "New companion %s — contact list has adequate space",
+                                "Companion %s — contact list has adequate space",
                                 contact_name,
                             )
                     elif auto_manage_setting == "bot":
@@ -3718,10 +3731,11 @@ class MessageHandler:
 
                     await self.bot.repeater_manager.check_and_auto_purge()
 
-                    self.bot.repeater_manager.log_purging_action(
-                        "new_contact_discovered",
-                        f"New contact discovered: {contact_name} (key: {public_key[:16]}...)",
-                    )
+                    if not known_contact:
+                        self.bot.repeater_manager.log_purging_action(
+                            "new_contact_discovered",
+                            f"New contact discovered: {contact_name} (key: {public_key[:16]}...)",
+                        )
                     return
 
             # Fallback: Track in database for unknown contact types (no repeater_manager)

modules/repeater_manager.py

@@ -571,12 +571,7 @@ class RepeaterManager:
 
     def _update_currently_tracked_status_on_conn(self, conn, public_key: str):
         """Update the is_currently_tracked flag on an existing connection (no commit)."""
-        is_tracked = False
-        if hasattr(self.bot.meshcore, 'contacts'):
-            for contact_key, contact_data in list(self.bot.meshcore.contacts.items()):
-                if contact_data.get('public_key', contact_key) == public_key:
-                    is_tracked = True
-                    break
+        is_tracked = self.is_contact_on_device(public_key)
         self.db_manager.execute_update_on_connection(
             conn,
             'UPDATE complete_contact_tracking SET is_currently_tracked = ? WHERE public_key = ?',
@@ -586,13 +581,7 @@ class RepeaterManager:
     async def _update_currently_tracked_status(self, public_key: str):
         """Update the is_currently_tracked flag based on device contact list"""
         try:
-            # Check if this repeater is currently in the device's contact list
-            is_tracked = False
-            if hasattr(self.bot.meshcore, 'contacts'):
-                for contact_key, contact_data in list(self.bot.meshcore.contacts.items()):
-                    if contact_data.get('public_key', contact_key) == public_key:
-                        is_tracked = True
-                        break
+            is_tracked = self.is_contact_on_device(public_key)
 
             # Update the flag
             self.db_manager.execute_update(
@@ -603,6 +592,35 @@ class RepeaterManager:
         except Exception as e:
             self.logger.error(f"Error updating currently tracked status: {e}")
 
+    def get_tracked_contact_row(self, public_key: str) -> Optional[dict[str, Any]]:
+        """Return a tracked contact row by public key, or None when absent."""
+        try:
+            rows = self.db_manager.execute_query(
+                '''
+                SELECT public_key, name, role, device_type, first_heard, last_heard,
+                       advert_count, is_currently_tracked
+                FROM complete_contact_tracking
+                WHERE public_key = ?
+                LIMIT 1
+                ''',
+                (public_key,),
+            )
+            return rows[0] if rows else None
+        except Exception as e:
+            self.logger.debug("Error looking up tracked contact %s: %s", public_key[:16], e)
+            return None
+
+    def is_contact_on_device(self, public_key: str) -> bool:
+        """Return True when the radio's live contact table already contains public_key."""
+        try:
+            if hasattr(self.bot.meshcore, 'contacts'):
+                for contact_key, contact_data in list(self.bot.meshcore.contacts.items()):
+                    if contact_data.get('public_key', contact_key) == public_key:
+                        return True
+        except Exception as e:
+            self.logger.debug("Error checking live device contacts for %s: %s", public_key[:16], e)
+        return False
+
     async def get_complete_contact_database(self, role_filter: Optional[str] = None, include_historical: bool = True) -> list[dict]:
         """Get complete contact database for path estimation and analysis"""
         try:

tests/test_message_handler.py

@@ -1969,6 +1969,8 @@ def new_contact_env(mock_logger):
     rm.manage_contact_list = AsyncMock(return_value={"success": True})
     rm.add_companion_from_contact_data = AsyncMock(return_value=True)
     rm.log_purging_action = Mock()
+    rm.get_tracked_contact_row = Mock(return_value=None)
+    rm.is_contact_on_device = Mock(return_value=False)
     rm.db_manager = Mock()
     rm.db_manager.execute_update = Mock()
     rm._is_repeater_device = Mock(return_value=False)
@@ -2042,3 +2044,68 @@ class TestHandleNewContactAutoManage:
         await handler.handle_new_contact(ev, None)
         assert rm.track_contact_advertisement.await_count == 2
         rm.add_companion_from_contact_data.assert_awaited_once()
+
+    async def test_new_companion_logs_new_and_records_audit(self, new_contact_env):
+        bot, handler, rm, mesh = new_contact_env
+        bot.config.set("Bot", "auto_manage_contacts", "false")
+        ev = _NewContactEvent(_companion_contact_payload())
+        await handler.handle_new_contact(ev, None)
+        log_messages = [str(call.args[0]) for call in bot.logger.info.call_args_list if call.args]
+        assert any("New companion discovered" in msg for msg in log_messages)
+        assert not any("Known companion advert" in msg for msg in log_messages)
+        rm.log_purging_action.assert_called_once()
+
+    async def test_known_companion_logs_known_and_skips_audit(self, new_contact_env):
+        bot, handler, rm, mesh = new_contact_env
+        bot.config.set("Bot", "auto_manage_contacts", "false")
+        rm.get_tracked_contact_row.return_value = {
+            "public_key": _companion_contact_payload()["public_key"],
+            "name": "Alice",
+            "role": "companion",
+        }
+        ev = _NewContactEvent(_companion_contact_payload())
+        await handler.handle_new_contact(ev, None)
+        log_messages = [str(call.args[0]) for call in bot.logger.info.call_args_list if call.args]
+        assert any("Known companion advert" in msg for msg in log_messages)
+        assert not any("New companion discovered" in msg for msg in log_messages)
+        rm.log_purging_action.assert_not_called()
+
+    async def test_known_companion_via_device_contact_skips_audit(self, new_contact_env):
+        bot, handler, rm, mesh = new_contact_env
+        bot.config.set("Bot", "auto_manage_contacts", "false")
+        # No tracking-DB row, but the radio already has the contact on-device.
+        rm.is_contact_on_device.return_value = True
+        ev = _NewContactEvent(_companion_contact_payload())
+        await handler.handle_new_contact(ev, None)
+        log_messages = [str(call.args[0]) for call in bot.logger.info.call_args_list if call.args]
+        assert any("Known companion advert" in msg for msg in log_messages)
+        rm.log_purging_action.assert_not_called()
+
+    async def test_known_repeater_logs_known_not_new(self, new_contact_env):
+        bot, handler, rm, mesh = new_contact_env
+        bot.config.set("Bot", "auto_manage_contacts", "false")
+        rm._is_repeater_device.return_value = True
+        rm.get_tracked_contact_row.return_value = {
+            "public_key": _companion_contact_payload()["public_key"],
+            "name": "Roof",
+            "role": "repeater",
+        }
+        ev = _NewContactEvent(_companion_contact_payload())
+        await handler.handle_new_contact(ev, None)
+        log_messages = [str(call.args[0]) for call in bot.logger.info.call_args_list if call.args]
+        assert any("Known repeater advert" in msg for msg in log_messages)
+        assert not any("New repeater discovered" in msg for msg in log_messages)
+        # Repeaters are never pushed to the device contact list.
+        mesh.commands.add_contact.assert_not_called()
+        rm.add_companion_from_contact_data.assert_not_called()
+
+    async def test_new_repeater_logs_new_and_stays_off_device(self, new_contact_env):
+        bot, handler, rm, mesh = new_contact_env
+        bot.config.set("Bot", "auto_manage_contacts", "device")
+        rm._is_repeater_device.return_value = True
+        ev = _NewContactEvent(_companion_contact_payload())
+        await handler.handle_new_contact(ev, None)
+        log_messages = [str(call.args[0]) for call in bot.logger.info.call_args_list if call.args]
+        assert any("New repeater discovered" in msg for msg in log_messages)
+        mesh.commands.add_contact.assert_not_called()
+        rm.add_companion_from_contact_data.assert_not_called()