Fix DM collapse on same second send

app/AGENTS.md

@@ -353,7 +353,7 @@ tests/
 
 The MeshCore radio protocol encodes `sender_timestamp` as a 4-byte little-endian integer (Unix seconds). This is a firmware-level wire format — the radio, the Python library (`commands/messaging.py`), and the decoder (`decoder.py`) all read/write exactly 4 bytes. Millisecond Unix timestamps would overflow 4 bytes, so higher resolution is not possible without a firmware change.
 
-**Consequence:** The dedup index `(type, conversation_key, text, COALESCE(sender_timestamp, 0))` operates at 1-second granularity. Sending identical text to the same conversation twice within one second will hit the UNIQUE constraint on the second insert, returning HTTP 500 *after* the radio has already transmitted. The message is sent over the air but not stored in the database. Do not attempt to fix this by switching to millisecond timestamps — it will break echo dedup (the echo's 4-byte timestamp won't match the stored value) and overflow `to_bytes(4, "little")`.
+**Consequence:** Channel-message dedup still operates at 1-second granularity because the radio protocol only provides second-resolution `sender_timestamp`. Do not attempt to fix this by switching to millisecond timestamps — it will break echo dedup (the echo's 4-byte timestamp won't match the stored value) and overflow `to_bytes(4, "little")`. Direct messages no longer share that channel dedup index; they are deduplicated by raw-packet identity instead so legitimate same-text same-second DMs can coexist.
 
 ### Outgoing DM echoes remain undecrypted
 

app/database.py

@@ -50,9 +50,10 @@ CREATE TABLE IF NOT EXISTS messages (
     acked INTEGER DEFAULT 0,
     sender_name TEXT,
     sender_key TEXT
-    -- Deduplication: identical text + timestamp in the same conversation is treated as a
-    -- mesh echo/repeat. Outgoing sends allocate a collision-free sender_timestamp before
-    -- transmit so legitimate repeat sends do not collide with this index.
+    -- Deduplication: channel echoes/repeats use a channel-only unique index on
+    -- identical conversation/text/timestamp. Direct messages are deduplicated
+    -- separately via raw-packet linkage so legitimate same-text same-second DMs
+    -- can coexist.
     -- Enforced via idx_messages_dedup_null_safe (unique index) rather than a table constraint
     -- to avoid the storage overhead of SQLite's autoindex duplicating every message text.
 );
@@ -90,7 +91,8 @@ CREATE TABLE IF NOT EXISTS contact_name_history (
 
 CREATE INDEX IF NOT EXISTS idx_messages_received ON messages(received_at);
 CREATE UNIQUE INDEX IF NOT EXISTS idx_messages_dedup_null_safe
-    ON messages(type, conversation_key, text, COALESCE(sender_timestamp, 0));
+    ON messages(type, conversation_key, text, COALESCE(sender_timestamp, 0))
+    WHERE type = 'CHAN';
 CREATE INDEX IF NOT EXISTS idx_raw_packets_message_id ON raw_packets(message_id);
 CREATE UNIQUE INDEX IF NOT EXISTS idx_raw_packets_payload_hash ON raw_packets(payload_hash);
 CREATE INDEX IF NOT EXISTS idx_contacts_on_radio ON contacts(on_radio);

app/migrations.py

@@ -331,6 +331,13 @@ async def run_migrations(conn: aiosqlite.Connection) -> int:
         await set_version(conn, 42)
         applied += 1
 
+    # Migration 43: Limit message dedup index to channel messages only
+    if version < 43:
+        logger.info("Applying migration 43: narrow message dedup index to channels")
+        await _migrate_043_split_message_dedup_by_type(conn)
+        await set_version(conn, 43)
+        applied += 1
+
     if applied > 0:
         logger.info(
             "Applied %d migration(s), schema now at version %d", applied, await get_version(conn)
@@ -2443,3 +2450,29 @@ async def _migrate_042_add_channel_flood_scope_override(conn: aiosqlite.Connecti
             raise
 
     await conn.commit()
+
+
+async def _migrate_043_split_message_dedup_by_type(conn: aiosqlite.Connection) -> None:
+    """Restrict the message dedup index to channel messages."""
+    cursor = await conn.execute(
+        "SELECT name FROM sqlite_master WHERE type='table' AND name='messages'"
+    )
+    if await cursor.fetchone() is None:
+        await conn.commit()
+        return
+
+    cursor = await conn.execute("PRAGMA table_info(messages)")
+    columns = {row[1] for row in await cursor.fetchall()}
+    required_columns = {"type", "conversation_key", "text", "sender_timestamp"}
+    if not required_columns.issubset(columns):
+        logger.debug("messages table missing dedup-index columns, skipping migration 43")
+        await conn.commit()
+        return
+
+    await conn.execute("DROP INDEX IF EXISTS idx_messages_dedup_null_safe")
+    await conn.execute(
+        """CREATE UNIQUE INDEX IF NOT EXISTS idx_messages_dedup_null_safe
+           ON messages(type, conversation_key, text, COALESCE(sender_timestamp, 0))
+           WHERE type = 'CHAN'"""
+    )
+    await conn.commit()

app/repository/raw_packets.py

@@ -109,6 +109,18 @@ class RawPacketRepository:
         )
         await db.conn.commit()
 
+    @staticmethod
+    async def get_linked_message_id(packet_id: int) -> int | None:
+        """Return the linked message ID for a raw packet, if any."""
+        cursor = await db.conn.execute(
+            "SELECT message_id FROM raw_packets WHERE id = ?",
+            (packet_id,),
+        )
+        row = await cursor.fetchone()
+        if not row:
+            return None
+        return row["message_id"]
+
     @staticmethod
     async def prune_old_undecrypted(max_age_days: int) -> int:
         """Delete undecrypted packets older than max_age_days. Returns count deleted."""

app/services/messages.py

@@ -1,3 +1,4 @@
+import asyncio
 import logging
 import time
 from collections.abc import Callable
@@ -13,6 +14,7 @@ logger = logging.getLogger(__name__)
 
 BroadcastFn = Callable[..., Any]
 LOG_MESSAGE_PREVIEW_LEN = 32
+_decrypted_dm_store_lock = asyncio.Lock()
 
 
 def _truncate_for_log(text: str, max_chars: int = LOG_MESSAGE_PREVIEW_LEN) -> str:
@@ -125,6 +127,45 @@ async def increment_ack_and_broadcast(
     return ack_count
 
 
+async def _reconcile_duplicate_message(
+    *,
+    existing_msg: Message,
+    packet_id: int | None,
+    path: str | None,
+    received_at: int,
+    path_len: int | None,
+    broadcast_fn: BroadcastFn,
+) -> None:
+    logger.debug(
+        "Duplicate %s for %s (msg_id=%d, outgoing=%s) - adding path",
+        existing_msg.type,
+        existing_msg.conversation_key[:12],
+        existing_msg.id,
+        existing_msg.outgoing,
+    )
+
+    if path is not None:
+        paths = await MessageRepository.add_path(existing_msg.id, path, received_at, path_len)
+    else:
+        paths = existing_msg.paths or []
+
+    if existing_msg.outgoing and existing_msg.type == "CHAN":
+        ack_count = await MessageRepository.increment_ack_count(existing_msg.id)
+    else:
+        ack_count = existing_msg.acked
+
+    if existing_msg.outgoing or path is not None:
+        broadcast_message_acked(
+            message_id=existing_msg.id,
+            ack_count=ack_count,
+            paths=paths,
+            broadcast_fn=broadcast_fn,
+        )
+
+    if packet_id is not None:
+        await RawPacketRepository.mark_decrypted(packet_id, existing_msg.id)
+
+
 async def handle_duplicate_message(
     *,
     packet_id: int | None,
@@ -153,35 +194,15 @@ async def handle_duplicate_message(
         )
         return
 
-    logger.debug(
-        "Duplicate %s for %s (msg_id=%d, outgoing=%s) - adding path",
-        msg_type,
-        conversation_key[:12],
-        existing_msg.id,
-        existing_msg.outgoing,
+    await _reconcile_duplicate_message(
+        existing_msg=existing_msg,
+        packet_id=packet_id,
+        path=path,
+        received_at=received_at,
+        path_len=path_len,
+        broadcast_fn=broadcast_fn,
     )
 
-    if path is not None:
-        paths = await MessageRepository.add_path(existing_msg.id, path, received_at, path_len)
-    else:
-        paths = existing_msg.paths or []
-
-    if existing_msg.outgoing and existing_msg.type == "CHAN":
-        ack_count = await MessageRepository.increment_ack_count(existing_msg.id)
-    else:
-        ack_count = existing_msg.acked
-
-    if existing_msg.outgoing or path is not None:
-        broadcast_message_acked(
-            message_id=existing_msg.id,
-            ack_count=ack_count,
-            paths=paths,
-            broadcast_fn=broadcast_fn,
-        )
-
-    if packet_id is not None:
-        await RawPacketRepository.mark_decrypted(packet_id, existing_msg.id)
-
 
 async def create_message_from_decrypted(
     *,
@@ -290,32 +311,64 @@ async def create_dm_message_from_decrypted(
     conversation_key = their_public_key.lower()
     sender_name = contact.name if contact and not outgoing else None
 
-    msg_id = await MessageRepository.create(
-        msg_type="PRIV",
-        text=decrypted.message,
-        conversation_key=conversation_key,
-        sender_timestamp=decrypted.timestamp,
-        received_at=received,
-        path=path,
-        path_len=path_len,
-        outgoing=outgoing,
-        sender_key=conversation_key if not outgoing else None,
-        sender_name=sender_name,
-    )
+    async with _decrypted_dm_store_lock:
+        linked_message_id = await RawPacketRepository.get_linked_message_id(packet_id)
+        if linked_message_id is not None:
+            existing_msg = await MessageRepository.get_by_id(linked_message_id)
+            if existing_msg is not None:
+                await _reconcile_duplicate_message(
+                    existing_msg=existing_msg,
+                    packet_id=packet_id,
+                    path=path,
+                    received_at=received,
+                    path_len=path_len,
+                    broadcast_fn=broadcast_fn,
+                )
+                return None
 
-    if msg_id is None:
-        await handle_duplicate_message(
-            packet_id=packet_id,
+        if outgoing:
+            existing_msg = await MessageRepository.get_by_content(
+                msg_type="PRIV",
+                conversation_key=conversation_key,
+                text=decrypted.message,
+                sender_timestamp=decrypted.timestamp,
+            )
+            if existing_msg is not None:
+                await _reconcile_duplicate_message(
+                    existing_msg=existing_msg,
+                    packet_id=packet_id,
+                    path=path,
+                    received_at=received,
+                    path_len=path_len,
+                    broadcast_fn=broadcast_fn,
+                )
+                return None
+
+        msg_id = await MessageRepository.create(
             msg_type="PRIV",
-            conversation_key=conversation_key,
             text=decrypted.message,
+            conversation_key=conversation_key,
             sender_timestamp=decrypted.timestamp,
-            path=path,
             received_at=received,
+            path=path,
             path_len=path_len,
-            broadcast_fn=broadcast_fn,
+            outgoing=outgoing,
+            sender_key=conversation_key if not outgoing else None,
+            sender_name=sender_name,
         )
-        return None
+        if msg_id is None:
+            await handle_duplicate_message(
+                packet_id=packet_id,
+                msg_type="PRIV",
+                conversation_key=conversation_key,
+                text=decrypted.message,
+                sender_timestamp=decrypted.timestamp,
+                path=path,
+                received_at=received,
+                path_len=path_len,
+                broadcast_fn=broadcast_fn,
+            )
+            return None
 
     logger.info(
         'Stored direct message "%s" for %r (msg ID %d in contact ID %s, outgoing=%s)',
@@ -364,6 +417,15 @@ async def create_fallback_direct_message(
     message_repository=MessageRepository,
 ) -> Message | None:
     """Store and broadcast a CONTACT_MSG_RECV fallback direct message."""
+    existing = await message_repository.get_by_content(
+        msg_type="PRIV",
+        conversation_key=conversation_key,
+        text=text,
+        sender_timestamp=sender_timestamp,
+    )
+    if existing is not None:
+        return None
+
     msg_id = await message_repository.create(
         msg_type="PRIV",
         text=text,

tests/test_echo_dedup.py

@@ -331,12 +331,9 @@ class TestDMEchoDetection:
         assert msg_id is not None
         broadcasts.clear()
 
-        # Duplicate arrives via different path
-        pkt2, _ = await RawPacketRepository.create(b"dm_in_2", SENDER_TIMESTAMP + 1)
-
         with patch("app.packet_processor.broadcast_event", mock_broadcast):
             result = await create_dm_message_from_decrypted(
-                packet_id=pkt2,
+                packet_id=pkt1,
                 decrypted=decrypted,
                 their_public_key=CONTACT_PUB,
                 our_public_key=OUR_PUB,
@@ -388,12 +385,9 @@ class TestDMEchoDetection:
         assert msg_id is not None
         broadcasts.clear()
 
-        # Duplicate arrives, also with no path
-        pkt2, _ = await RawPacketRepository.create(b"dm_np_2", SENDER_TIMESTAMP + 1)
-
         with patch("app.packet_processor.broadcast_event", mock_broadcast):
             result = await create_dm_message_from_decrypted(
-                packet_id=pkt2,
+                packet_id=pkt1,
                 decrypted=decrypted,
                 their_public_key=CONTACT_PUB,
                 our_public_key=OUR_PUB,
@@ -832,21 +826,19 @@ class TestDirectMessageDirectionDetection:
 
 
 class TestConcurrentDMDedup:
-    """Test that concurrent DM processing deduplicates via atomic INSERT OR IGNORE.
+    """Test that concurrent DM processing deduplicates by raw-packet identity.
 
-    On a mesh network, the same DM packet can arrive via two RF paths nearly
-    simultaneously, causing two concurrent calls to create_dm_message_from_decrypted.
-    SQLite's INSERT OR IGNORE ensures only one message is stored.
+    On a mesh network, the same DM payload can be observed twice before the first
+    handler finishes. Both arrivals reuse the same raw_packets row and should end
+    up attached to a single message.
     """
 
     @pytest.mark.asyncio
-    async def test_concurrent_identical_dms_only_store_once(self, test_db, captured_broadcasts):
-        """Two concurrent create_dm_message_from_decrypted calls with identical content
-        should result in exactly one stored message."""
+    async def test_concurrent_same_packet_dms_only_store_once(self, test_db, captured_broadcasts):
+        """Two concurrent handlers for the same raw DM packet store one message."""
         from app.packet_processor import create_dm_message_from_decrypted
 
-        pkt1, _ = await RawPacketRepository.create(b"concurrent_dm_1", SENDER_TIMESTAMP)
-        pkt2, _ = await RawPacketRepository.create(b"concurrent_dm_2", SENDER_TIMESTAMP + 1)
+        packet_id, _ = await RawPacketRepository.create(b"concurrent_dm_1", SENDER_TIMESTAMP)
 
         decrypted = DecryptedDirectMessage(
             timestamp=SENDER_TIMESTAMP,
@@ -861,7 +853,7 @@ class TestConcurrentDMDedup:
         with patch("app.packet_processor.broadcast_event", mock_broadcast):
             results = await asyncio.gather(
                 create_dm_message_from_decrypted(
-                    packet_id=pkt1,
+                    packet_id=packet_id,
                     decrypted=decrypted,
                     their_public_key=CONTACT_PUB,
                     our_public_key=OUR_PUB,
@@ -870,7 +862,7 @@ class TestConcurrentDMDedup:
                     outgoing=False,
                 ),
                 create_dm_message_from_decrypted(
-                    packet_id=pkt2,
+                    packet_id=packet_id,
                     decrypted=decrypted,
                     their_public_key=CONTACT_PUB,
                     our_public_key=OUR_PUB,

tests/test_key_normalization.py

@@ -78,8 +78,8 @@ async def test_null_sender_timestamp_defaults_to_received_at(test_db):
 
 
 @pytest.mark.asyncio
-async def test_duplicate_with_same_text_and_null_timestamp_rejected(test_db):
-    """Two messages with same content and sender_timestamp should be deduped."""
+async def test_direct_messages_with_same_text_and_timestamp_are_allowed(test_db):
+    """Direct messages no longer share the channel echo dedup index."""
     received_at = 600
     msg_id1 = await MessageRepository.create(
         msg_type="PRIV",
@@ -97,7 +97,8 @@ async def test_duplicate_with_same_text_and_null_timestamp_rejected(test_db):
         sender_timestamp=received_at,
         received_at=received_at,
     )
-    assert msg_id2 is None  # duplicate rejected
+    assert msg_id2 is not None
+    assert msg_id2 != msg_id1
 
 
 @pytest.mark.asyncio

tests/test_migrations.py

@@ -574,7 +574,7 @@ class TestMigration019:
 
     @pytest.mark.asyncio
     async def test_migration_drops_messages_unique_constraint(self):
-        """Migration rebuilds messages without UNIQUE, preserving data and dedup index."""
+        """Migration rebuilds messages without UNIQUE, preserving data and channel dedup index."""
         conn = await aiosqlite.connect(":memory:")
         conn.row_factory = aiosqlite.Row
         try:
@@ -657,7 +657,7 @@ class TestMigration019:
             assert rows[1]["type"] == "PRIV"
             assert rows[1]["outgoing"] == 1
 
-            # Verify dedup index still works (INSERT OR IGNORE should ignore duplicates)
+            # Verify channel dedup index still works (INSERT OR IGNORE should ignore duplicates)
             cursor = await conn.execute(
                 "INSERT OR IGNORE INTO messages (type, conversation_key, text, sender_timestamp, received_at) "
                 "VALUES (?, ?, ?, ?, ?)",
@@ -665,11 +665,25 @@ class TestMigration019:
             )
             assert cursor.rowcount == 0  # Duplicate ignored
 
+            # Direct messages no longer use the shared dedup index.
+            cursor = await conn.execute(
+                "INSERT OR IGNORE INTO messages (type, conversation_key, text, sender_timestamp, received_at) "
+                "VALUES (?, ?, ?, ?, ?)",
+                ("PRIV", "abc123", "dm text", 2000, 9999),
+            )
+            assert cursor.rowcount == 1
+
             # Verify dedup index exists
             cursor = await conn.execute(
                 "SELECT name FROM sqlite_master WHERE name='idx_messages_dedup_null_safe'"
             )
             assert await cursor.fetchone() is not None
+
+            cursor = await conn.execute(
+                "SELECT sql FROM sqlite_master WHERE name='idx_messages_dedup_null_safe'"
+            )
+            index_sql = (await cursor.fetchone())["sql"]
+            assert "WHERE type = 'CHAN'" in index_sql
         finally:
             await conn.close()
 
@@ -1116,8 +1130,8 @@ class TestMigration039:
 
             applied = await run_migrations(conn)
 
-            assert applied == 4
-            assert await get_version(conn) == 42
+            assert applied == 5
+            assert await get_version(conn) == 43
 
             cursor = await conn.execute(
                 """
@@ -1186,8 +1200,8 @@ class TestMigration039:
 
             applied = await run_migrations(conn)
 
-            assert applied == 4
-            assert await get_version(conn) == 42
+            assert applied == 5
+            assert await get_version(conn) == 43
 
             cursor = await conn.execute(
                 """
@@ -1240,8 +1254,8 @@ class TestMigration040:
 
             applied = await run_migrations(conn)
 
-            assert applied == 3
-            assert await get_version(conn) == 42
+            assert applied == 4
+            assert await get_version(conn) == 43
 
             await conn.execute(
                 """
@@ -1302,8 +1316,8 @@ class TestMigration041:
 
             applied = await run_migrations(conn)
 
-            assert applied == 2
-            assert await get_version(conn) == 42
+            assert applied == 3
+            assert await get_version(conn) == 43
 
             await conn.execute(
                 """
@@ -1355,8 +1369,8 @@ class TestMigration042:
 
             applied = await run_migrations(conn)
 
-            assert applied == 1
-            assert await get_version(conn) == 42
+            assert applied == 2
+            assert await get_version(conn) == 43
 
             await conn.execute(
                 """

tests/test_packet_pipeline.py

@@ -896,8 +896,58 @@ class TestCreateDMMessageFromDecrypted:
         assert message_broadcasts[0]["data"]["outgoing"] is True
 
     @pytest.mark.asyncio
-    async def test_returns_none_for_duplicate_dm(self, test_db, captured_broadcasts):
-        """create_dm_message_from_decrypted returns None for duplicate DM."""
+    async def test_returns_none_for_same_raw_packet_duplicate_dm(
+        self, test_db, captured_broadcasts
+    ):
+        """Reprocessing the same raw DM packet reuses the existing message."""
+        from app.decoder import DecryptedDirectMessage
+        from app.packet_processor import create_dm_message_from_decrypted
+
+        packet_id, _ = await RawPacketRepository.create(b"dm_packet_1", 1700000000)
+
+        decrypted = DecryptedDirectMessage(
+            timestamp=1700000000,
+            flags=0,
+            message="Duplicate DM test",
+            dest_hash="fa",
+            src_hash="a1",
+        )
+
+        broadcasts, mock_broadcast = captured_broadcasts
+
+        with patch("app.packet_processor.broadcast_event", mock_broadcast):
+            # First call creates the message
+            msg_id_1 = await create_dm_message_from_decrypted(
+                packet_id=packet_id,
+                decrypted=decrypted,
+                their_public_key=self.A1B2C3_PUB,
+                our_public_key=self.FACE12_PUB,
+                received_at=1700000001,
+                outgoing=False,
+            )
+
+            # Second call for the same packet returns None and does not create a new row
+            msg_id_2 = await create_dm_message_from_decrypted(
+                packet_id=packet_id,
+                decrypted=decrypted,
+                their_public_key=self.A1B2C3_PUB,
+                our_public_key=self.FACE12_PUB,
+                received_at=1700000002,
+                outgoing=False,
+            )
+
+        assert msg_id_1 is not None
+        assert msg_id_2 is None  # Duplicate detected
+
+        # Only one message broadcast
+        message_broadcasts = [b for b in broadcasts if b["type"] == "message"]
+        assert len(message_broadcasts) == 1
+
+    @pytest.mark.asyncio
+    async def test_allows_same_text_same_second_dms_from_distinct_packets(
+        self, test_db, captured_broadcasts
+    ):
+        """Distinct DM packets with the same text/timestamp both store."""
         from app.decoder import DecryptedDirectMessage
         from app.packet_processor import create_dm_message_from_decrypted
 
@@ -915,7 +965,6 @@ class TestCreateDMMessageFromDecrypted:
         broadcasts, mock_broadcast = captured_broadcasts
 
         with patch("app.packet_processor.broadcast_event", mock_broadcast):
-            # First call creates the message
             msg_id_1 = await create_dm_message_from_decrypted(
                 packet_id=packet_id_1,
                 decrypted=decrypted,
@@ -924,8 +973,6 @@ class TestCreateDMMessageFromDecrypted:
                 received_at=1700000001,
                 outgoing=False,
             )
-
-            # Second call with same content returns None
             msg_id_2 = await create_dm_message_from_decrypted(
                 packet_id=packet_id_2,
                 decrypted=decrypted,
@@ -936,11 +983,16 @@ class TestCreateDMMessageFromDecrypted:
             )
 
         assert msg_id_1 is not None
-        assert msg_id_2 is None  # Duplicate detected
+        assert msg_id_2 is not None
+        assert msg_id_1 != msg_id_2
+
+        messages = await MessageRepository.get_all(
+            msg_type="PRIV", conversation_key=self.A1B2C3_PUB.lower(), limit=10
+        )
+        assert len(messages) == 2
 
-        # Only one message broadcast
         message_broadcasts = [b for b in broadcasts if b["type"] == "message"]
-        assert len(message_broadcasts) == 1
+        assert len(message_broadcasts) == 2
 
     @pytest.mark.asyncio
     async def test_links_raw_packet_to_dm_message(self, test_db, captured_broadcasts):