Contacts: a contact the device never held is still ours to remove

The list a screen removes from is the union of the device's contact table and
the contacts MeshTerm remembers for it, so an entry can exist only on our side
— or the firmware can drop one since we last read. Removing such a contact hit
ERR_CODE_NOT_FOUND and reported "couldn't remove", leaving a row the reader had
no way to get rid of.

The device layer now raises ContactNotOnDeviceError for that code, the same
class (and the same table lookup) the send path already raises. The single
removal reports it as a warning naming what happened, forgets our half and ends
the visit; the purge sweep counts it as purged rather than failed. The
simulator refuses the same way, so the path is walkable under --mock.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

meshterm/core/connection.py

@@ -174,6 +174,11 @@ class ContactNotOnDeviceError(DeviceCommandError):
     for it (see :mod:`meshterm.core.contact_store`): a contact the firmware has since
     dropped still lists, and only the send finds out it is gone.
 
+    :meth:`Device.remove_contact` raises it for the same lookup against the same table,
+    where it means the opposite thing: nothing left to delete on the radio. A removal
+    therefore *reports* it and carries on dropping the contact from what MeshTerm
+    remembers, rather than failing and leaving a row the reader cannot get rid of.
+
     Attributes:
         contact: The recipient the device could not find.
     """
@@ -535,8 +540,11 @@ class Device(ABC):
             node: The contact to remove.
 
         Raises:
+            ContactNotOnDeviceError: If the device holds no contact with that key — which
+                is not a failed removal but a removal with nothing left to do, so callers
+                drop it from what MeshTerm remembers and say so.
             DeviceCommandError: If the contact carries no public key to address it by, or
-                the device rejected the removal.
+                the device rejected the removal for any other reason.
         """
 
     @abstractmethod
@@ -1742,7 +1750,18 @@ class MeshCoreDevice(Device):
     async def remove_contact(self, node: Contact) -> None:  # noqa: D102 - inherited docstring
         mc = self._require()
         pub = self._node_pubkey(node)  # raises DeviceCommandError if it has no key
-        self._ok(await mc.commands.remove_contact(pub))
+        result = await mc.commands.remove_contact(pub)
+        if result is not None and getattr(result, "is_error", lambda: False)():
+            # A contact the firmware doesn't hold is the one rejection that isn't a
+            # failure: the list a screen removes from is the union of this table and the
+            # contacts MeshTerm remembers for the device, so the entry the reader is
+            # deleting may only ever have existed on our side. Same class the send path
+            # raises, for the same reason — the caller can finish the job.
+            if error_code(result) == _ERR_NOT_FOUND:
+                raise ContactNotOnDeviceError(node)
+            raise DeviceCommandError(
+                f"couldn't remove {node.name} from the device: {reject_reason(result)}"
+            )
 
     async def get_tx_power(self) -> Optional[int]:  # noqa: D102 - inherited docstring
         info = await self.get_self_info()
@@ -2736,6 +2755,11 @@ class MockDevice(Device):
     async def remove_contact(self, node: Contact) -> None:  # noqa: D102 - inherited docstring
         await asyncio.sleep(0)
         key = self._mock_key(node)
+        # Firmware can only delete a contact it holds, and answers ERR_CODE_NOT_FOUND for
+        # one it doesn't; the simulator refuses the same way so the "wasn't on the device,
+        # removed here anyway" path is walkable without a radio.
+        if key not in {self._mock_key(c) for c in self._contacts}:
+            raise ContactNotOnDeviceError(node)
         self._contacts = [c for c in self._contacts if self._mock_key(c) != key]
 
     async def get_tx_power(self) -> Optional[int]:  # noqa: D102 - inherited docstring

meshterm/ui/contacts_screen.py

@@ -32,6 +32,7 @@ from typing import TYPE_CHECKING
 
 from rich.text import Text
 
+from ..core.connection import ContactNotOnDeviceError
 from .contactlist import ContactListScreen, ContactRow
 from .menus import marked_label, menu_rows
 from .tui import Choice, Separator
@@ -321,6 +322,12 @@ async def _purge_stale(ctx: "AppContext", self_key: str) -> int:
         for contact in victims:
             try:
                 await device.remove_contact(contact)
+            except ContactNotOnDeviceError:
+                # The device has no such contact, so the sweep's work for it is already
+                # done: this one is listed only because MeshTerm remembers it for the
+                # device. It still counts as purged — forgetting it below is what makes
+                # the row go away — and there is nothing here for the reader to fix.
+                ctx.log.debug("contacts: %s was not on the device; purging ours", contact.name)
             except Exception as exc:  # noqa: BLE001 - one bad removal shouldn't abort the sweep
                 failed += 1
                 ctx.log.debug("contacts: purge failed for %s: %s", contact.name, exc)

meshterm/ui/node_detail_screen.py

@@ -82,6 +82,7 @@ from typing import TYPE_CHECKING, Optional
 from rich.cells import cell_len
 from rich.text import Text
 
+from ..core.connection import ContactNotOnDeviceError
 from ..core.geo import EARTH_RADIUS_KM, usable_fix
 from ..core.models import NODE_TYPE_LABELS, Contact, utcnow
 from ..platforms import Platform, on_platform
@@ -1454,7 +1455,9 @@ async def _remove_contact(
     Returns:
         ``True`` if the contact was removed, ``False`` if the user cancelled or the device
         refused (the refusal is shown, never swallowed — a contact still on the radio must
-        not vanish from the list).
+        not vanish from the list). A device that has no such contact is not a refusal: the
+        removal finishes on our side and says so
+        (:class:`~meshterm.core.connection.ContactNotOnDeviceError`).
     """
     from .surface import TuiUi
 
@@ -1474,6 +1477,21 @@ async def _remove_contact(
     device = await ctx.device()
     try:
         await device.remove_contact(contact)
+    except ContactNotOnDeviceError:
+        # Nothing to delete on the radio — the entry only ever existed in what MeshTerm
+        # remembers for this device (or the firmware dropped it since we read the table).
+        # That is not a failed removal: the contact still goes, and the reader is told why
+        # the device had no part in it, because "removed" would otherwise be a claim about
+        # a radio that never held it.
+        ctx.log.debug("contacts: %s was not on the device; removing ours", contact.name)
+        await session.message_dialog(
+            Text(
+                f"⚠ {label} wasn't in this device's contacts — removed from the ones "
+                "MeshTerm remembers for it.",
+                style="warn",
+            ),
+            title="Remove contact",
+        )
     except Exception as exc:  # noqa: BLE001 - a refused removal is reported, not raised
         ctx.log.debug("contacts: remove failed for %s: %s", contact.name, exc)
         await session.message_dialog(

tests/test_device_config.py

@@ -557,6 +557,61 @@ async def test_a_refused_removal_is_shown_and_the_contact_stays() -> None:
     assert forgotten == []  # the device still holds it, so neither do we forget it
 
 
+async def test_a_contact_the_device_never_held_is_still_removed_here() -> None:
+    """"Not on the device" is not a refusal — the removal finishes on our side, and says so.
+
+    The list a screen removes from is the union of the device's table and the contacts
+    MeshTerm remembers for it, so the entry may only ever have existed on our side (or the
+    firmware dropped it since we read). Failing there left a row the reader could not get
+    rid of. Instead the reader is told the device had no part in it, our half is forgotten,
+    and the visit ends on the contact like any other removal.
+    """
+    import asyncio
+    import logging
+    from types import SimpleNamespace
+
+    from meshterm.core.connection import ContactNotOnDeviceError
+    from meshterm.core.models import Contact
+    from meshterm.ui.node_detail_screen import _remove_contact
+    from meshterm.ui.surface import TuiUi
+    from meshterm.ui.tui.session import TuiSession
+
+    hub = Contact(name="Hub", public_key="3d" * 32, key_prefix="3d" * 6)
+    forgotten: list[tuple[str, str]] = []
+    invalidated: list[bool] = []
+
+    class _Device:
+        async def remove_contact(self, contact) -> None:  # noqa: ANN001
+            await asyncio.sleep(0)
+            raise ContactNotOnDeviceError(contact)
+
+    session = TuiSession()
+    ui = TuiUi(session)
+
+    async def _accepted(*a, **k):  # noqa: ANN001, ANN002, ANN003
+        return True
+
+    ui.dialog = _accepted  # type: ignore[method-assign]
+    ctx = SimpleNamespace(
+        ui=ui,
+        log=logging.getLogger("test.remove"),
+        contact_store=SimpleNamespace(
+            forget=lambda dev, key: forgotten.append((dev, key))
+        ),
+        devstate=SimpleNamespace(invalidate_contacts=lambda: invalidated.append(True)),
+        device=lambda: _device(_Device()),
+    )
+
+    task = asyncio.ensure_future(_remove_contact(ctx, hub, "cc" * 32, "Hub"))
+    told = await _step_until_screen(session, lambda s: "wasn't in this device" in _screen_text(s))
+    text = _screen_text(told)
+    assert "Hub" in text and "⚠" in text  # a warning, not the ✗ of a failed command
+    told.handle("escape")
+    assert await task is True
+    assert forgotten == [("cc" * 32, "3d" * 32)]
+    assert invalidated == [True]
+
+
 async def _true() -> bool:
     return True
 

tests/test_skeleton.py

@@ -320,6 +320,8 @@ async def test_mock_device_honors_forced_path() -> None:
 
 async def test_mock_device_remove_contact_drops_it_from_the_table() -> None:
     """Removing a contact deletes it from the device's table (matched by public key)."""
+    from meshterm.core.connection import ContactNotOnDeviceError
+
     device = MockDevice()
     await device.connect()
     before = await device.get_contacts()
@@ -328,8 +330,10 @@ async def test_mock_device_remove_contact_drops_it_from_the_table() -> None:
     after = await device.get_contacts()
     assert "Alice" not in {c.name for c in after}
     assert len(after) == len(before) - 1
-    # Removing one already gone is a no-op, not an error.
-    await device.remove_contact(alice)
+    # Removing one it no longer holds is refused the way firmware refuses it, so the
+    # screen's "wasn't on the device, removed here anyway" path is walkable on --mock.
+    with pytest.raises(ContactNotOnDeviceError):
+        await device.remove_contact(alice)
     assert len(await device.get_contacts()) == len(after)