fix(meshcore): resolve DM dest to full contact object before send (canonical pattern) (#22)

MeshCore DM replies were passed a bare 6-byte pubkey prefix to
send_msg_with_retry. Per every working meshcore project (meshcore_py
examples, meshcore-cli, meshcore-bot, meshcore-ha), the destination must be
resolved to the FULL contact object (dict w/ 64-hex public_key) after
ensure_contacts — otherwise the lib can't upgrade the prefix to the full key,
skips reset_path, blind-floods, gets no ACK, and the DM silently never
delivers (matches our live symptom: inbound + channel send work, DM reply dies).

- Add _resolve_contact(dest): ensure_contacts() then get_contact_by_key_prefix().
- DM branch now passes the resolved contact object to send_msg_with_retry;
  if the contact can't be resolved, log + return False (no blind-flood).
- Subscribe to EventType.ACK + log received ACKs (instrumentation to confirm
  whether ACKs reach the dispatcher at all).

Tests updated (+2); 0 new failures.

Co-authored-by: Matt Johnson <mj@k7zvx.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
malice 2026-07-03 20:28:26 -06:00 committed by GitHub
commit e7c8ef507e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 105 additions and 22 deletions

View file

@ -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

View file

@ -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)

View file

@ -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)