diff --git a/work/meshai/transport/meshcore_transport.py b/work/meshai/transport/meshcore_transport.py index bc66346..afcf329 100644 --- a/work/meshai/transport/meshcore_transport.py +++ b/work/meshai/transport/meshcore_transport.py @@ -118,6 +118,28 @@ class MeshCoreTransport(MeshTransport): self._loop = None self._loop_thread = None + def _resolve_contact(self, dest: str): + """Resolve a pubkey prefix (or key) to the full MeshCore contact dict. + + Refreshes the roster first (ensure_contacts) so the lib can upgrade the + 6-byte prefix to the full 32-byte key and run reset_path->flood. Returns + None if the contact can't be resolved. This mirrors what every working + meshcore project does before send_msg_with_retry (never send to a bare prefix). + """ + if self._mc is None: + return None + ensure = getattr(self._mc, "ensure_contacts", None) + if ensure is not None: + try: + self._run_coro(ensure(), timeout=15) + except Exception: + logger.debug("MeshCore: ensure_contacts (resolve) failed", exc_info=True) + try: + return self._mc.get_contact_by_key_prefix(dest) + except Exception: + logger.debug("MeshCore: get_contact_by_key_prefix failed for %s", dest, exc_info=True) + return None + # ------------------------------------------------------------------ # Channel table enumeration # ------------------------------------------------------------------ @@ -322,6 +344,7 @@ class MeshCoreTransport(MeshTransport): self._mc.subscribe(EventType.CHANNEL_MSG_RECV, self._on_channel_event) self._mc.subscribe(EventType.DISCONNECTED, self._on_disconnect_event) self._mc.subscribe(EventType.CONNECTED, self._on_connect_event) + self._mc.subscribe(EventType.ACK, self._on_ack_event) try: await self._mc.ensure_contacts() except Exception: @@ -468,21 +491,23 @@ class MeshCoreTransport(MeshTransport): try: if destination: - # DM: use the retry/flood-capable send so replies actually deliver. - # Plain send_msg is fire-and-forget (MSG_SENT != delivered) with no flood - # fallback, so DMs to nodes without an established direct path silently drop. - # send_msg_with_retry resolves the contact, floods when needed, waits for ACK, - # and returns None if no ACK (delivery not confirmed). - logger.debug("MeshCore: sending DM to %s", destination[:40]) + contact = self._resolve_contact(destination) + if contact is None: + logger.warning( + "MeshCore: could not resolve a contact for DM dest %s; cannot address reply " + "(recipient not in roster)", destination, + ) + return False + label = contact.get("adv_name") or contact.get("name") or destination + logger.debug("MeshCore: sending DM to %s via resolved contact", label) + # Pass the CONTACT OBJECT (not the bare prefix) so the lib can upgrade to the + # full key and reset_path->flood works — the pattern used by all working projects. result = self._run_coro( - self._mc.commands.send_msg_with_retry(destination, text), + self._mc.commands.send_msg_with_retry(contact, text), timeout=40, ) if result is None: - logger.warning( - "MeshCoreTransport: DM to %s not ACKed (delivery not confirmed)", - destination, - ) + logger.warning("MeshCoreTransport: DM to %s not ACKed (delivery not confirmed)", label) return False success = not result.is_error() if not success: @@ -661,6 +686,9 @@ class MeshCoreTransport(MeshTransport): except Exception as exc: logger.error("MeshCoreTransport: error dispatching message: %s", exc) + def _on_ack_event(self, event) -> None: + logger.info("MeshCore: ACK event received: %r", getattr(event, "payload", None)) + def _on_disconnect_event(self, event=None) -> None: """Track link state: DISCONNECTED.""" self._connected = False diff --git a/work/tests/test_meshcore_dm_delivery.py b/work/tests/test_meshcore_dm_delivery.py index 5a200c0..6b03cfe 100644 --- a/work/tests/test_meshcore_dm_delivery.py +++ b/work/tests/test_meshcore_dm_delivery.py @@ -1,10 +1,11 @@ """Focused tests for the MeshCore DM delivery fix. -Verifies that send_message(..., destination=...) calls send_msg_with_retry -(not the old fire-and-forget send_msg) and correctly maps its return value -to True/False: +Verifies that send_message(..., destination=...) resolves the destination to +the full contact object before calling send_msg_with_retry (never passes a bare +prefix string), and correctly maps the return value to True/False: - non-error Event returned → True (ACKed, delivered) - None returned → False (no ACK, delivery not confirmed) + - contact not in roster → False (logged warning, send never called) The meshcore lib is mocked via sys.modules (same pattern as the existing transport test module). _run_coro is patched to execute the coroutine @@ -18,6 +19,13 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest +# Contact dict returned by the fake roster — 64-hex public_key, adv_name, out_path_len. +_CONTACT_DICT = { + "public_key": "a" * 64, + "adv_name": "K7ZVX Matt", + "out_path_len": -1, +} + # --------------------------------------------------------------------------- # Minimal fake meshcore module (guards against import errors if the real lib @@ -35,6 +43,7 @@ def _ensure_fake_meshcore(): CHANNEL_MSG_RECV = "CHANNEL_MSG_RECV" DISCONNECTED = "DISCONNECTED" CONNECTED = "CONNECTED" + ACK = "ACK" mod.EventType = EventType @@ -55,7 +64,10 @@ def _ensure_fake_meshcore(): pass def get_contact_by_key_prefix(self, prefix): - return None + return _CONTACT_DICT + + async def ensure_contacts(self, follow=False): + return True @classmethod async def create_tcp(cls, host, port, @@ -97,16 +109,22 @@ def _mc_config(): return ConnectionConfig(meshcore_host="127.0.0.1", meshcore_port=5050) -def _transport_with_mc_mock(): +def _transport_with_mc_mock(contact=_CONTACT_DICT): """Return a MeshCoreTransport with _mc as a MagicMock (no loop thread). _run_coro is patched on the instance to run the coroutine synchronously via - asyncio.get_event_loop().run_until_complete(), bypassing the thread bridge. + asyncio.new_event_loop().run_until_complete(), bypassing the thread bridge. This keeps tests fast and deterministic. + + The mock exposes: + - mc.get_contact_by_key_prefix(prefix) → contact dict (or None when contact=None) + - mc.ensure_contacts is an AsyncMock (async, returns True) """ cfg = _mc_config() t = MeshCoreTransport(cfg) mc = MagicMock() + mc.get_contact_by_key_prefix.return_value = contact + mc.ensure_contacts = AsyncMock(return_value=True) t._mc = mc t._connected = True @@ -126,10 +144,10 @@ def _transport_with_mc_mock(): # --------------------------------------------------------------------------- class TestMeshCoreDMDelivery: - """send_message with destination= must use send_msg_with_retry, not send_msg.""" + """send_message with destination= must resolve the contact and use send_msg_with_retry.""" def test_acked_dm_returns_true_and_uses_send_msg_with_retry(self): - """ACK received (non-error Event) → returns True; send_msg_with_retry called.""" + """ACK received (non-error Event) → returns True; send_msg_with_retry called with CONTACT OBJECT.""" t, mc = _transport_with_mc_mock() ok_event = MagicMock() @@ -140,7 +158,8 @@ class TestMeshCoreDMDelivery: result = t.send_message("reply text", destination="aabbccdd1122") assert result is True - mc.commands.send_msg_with_retry.assert_awaited_once_with("aabbccdd1122", "reply text") + # Must pass the CONTACT OBJECT (dict), not the bare prefix string. + mc.commands.send_msg_with_retry.assert_awaited_once_with(_CONTACT_DICT, "reply text") mc.commands.send_msg.assert_not_awaited() def test_no_ack_returns_false(self): @@ -152,7 +171,7 @@ class TestMeshCoreDMDelivery: result = t.send_message("reply text", destination="aabbccdd1122") assert result is False - mc.commands.send_msg_with_retry.assert_awaited_once_with("aabbccdd1122", "reply text") + mc.commands.send_msg_with_retry.assert_awaited_once_with(_CONTACT_DICT, "reply text") def test_error_event_returns_false(self): """Error event returned by send_msg_with_retry → returns False.""" @@ -179,3 +198,25 @@ class TestMeshCoreDMDelivery: "not ACKed" in r.getMessage() or "delivery not confirmed" in r.getMessage() for r in caplog.records ) + + def test_unresolved_contact_returns_false_without_calling_send(self): + """When get_contact_by_key_prefix returns None, send_message returns False and never calls send_msg_with_retry.""" + t, mc = _transport_with_mc_mock(contact=None) + + mc.commands.send_msg_with_retry = AsyncMock() + + result = t.send_message("hello", destination="deadbeef0011") + + assert result is False + mc.commands.send_msg_with_retry.assert_not_awaited() + + def test_unresolved_contact_logs_warning(self, caplog): + """Unresolved contact → a warning about 'not in roster' is logged.""" + import logging + t, mc = _transport_with_mc_mock(contact=None) + mc.commands.send_msg_with_retry = AsyncMock() + + with caplog.at_level(logging.WARNING): + t.send_message("hello", destination="deadbeef0011") + + assert any("not in roster" in r.getMessage() for r in caplog.records) diff --git a/work/tests/test_meshcore_transport.py b/work/tests/test_meshcore_transport.py index e6c4887..dd9e716 100644 --- a/work/tests/test_meshcore_transport.py +++ b/work/tests/test_meshcore_transport.py @@ -28,6 +28,7 @@ def _build_fake_meshcore(): CHANNEL_MSG_RECV = "CHANNEL_MSG_RECV" DISCONNECTED = "DISCONNECTED" CONNECTED = "CONNECTED" + ACK = "ACK" mod.EventType = EventType @@ -46,6 +47,9 @@ def _build_fake_meshcore(): async def disconnect(self): pass + async def ensure_contacts(self, follow=False): + return True + def subscribe(self, event_type, callback): pass @@ -325,22 +329,32 @@ class TestSendMessageChannel: # 3. send_message — DM (destination provided) # --------------------------------------------------------------------------- +_DM_CONTACT = {"public_key": "a" * 64, "adv_name": "TestContact", "out_path_len": -1} + + class TestSendMessageDM: def test_dispatches_send_msg_with_retry(self): t, mc, _ = _transport_with_mock_mc() try: + # Supply a resolved contact so _resolve_contact succeeds. + mc.get_contact_by_key_prefix.return_value = _DM_CONTACT + mc.ensure_contacts = AsyncMock(return_value=True) ok = MagicMock() ok.is_error.return_value = False mc.commands.send_msg_with_retry = AsyncMock(return_value=ok) result = t.send_message("hi DM", destination="aabbcc") assert result is True - mc.commands.send_msg_with_retry.assert_awaited_once_with("aabbcc", "hi DM") + # Must pass the CONTACT OBJECT (not the bare prefix) so the lib can + # upgrade to the full 32-byte key and attempt reset_path->flood. + mc.commands.send_msg_with_retry.assert_awaited_once_with(_DM_CONTACT, "hi DM") finally: _cleanup(t) def test_send_msg_with_retry_error_returns_false(self): t, mc, _ = _transport_with_mock_mc() try: + mc.get_contact_by_key_prefix.return_value = _DM_CONTACT + mc.ensure_contacts = AsyncMock(return_value=True) err = MagicMock() err.is_error.return_value = True mc.commands.send_msg_with_retry = AsyncMock(return_value=err)