Only the reader removes a discovered setting; a read may only take it to n/a

A discovered row was dropped when a read came back as anything that could not
be its value, on the reasoning that the node's answer was the row's whole
claim to exist. The reasoning holds and the cost does not.

A catalog row that misreads costs an n/a and nothing is lost: the catalog
still names the key, and the next read puts the value back. A discovered row's
key is recorded nowhere else. Deleting it throws away the one thing about this
node that only the reader could have found, and they may no longer remember
what it was -- for a reply that misread rather than a key that went away. A
truncated line, a node answering mid-reboot, a stray frame correlated to the
wrong command, and the row is gone with no way back but memory.

So a read now takes a discovered row to n/a like any other, and Del is the
only thing that removes one. The board-swap case the automatic drop was for is
still served: the row reads n/a, which is visible and says what happened, and
forgetting it is a keystroke.

A dropped packet was never the danger -- read_settings folds only a reply it
actually received, and a timeout leaves the row alone -- but the rows it can
delete were never worth the ones it tidied.

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

meshterm/core/remote_store.py

@@ -36,9 +36,9 @@ class CachedValue:
             has no such setting (a board without a front-end module, a build without a
             bridge). Kept apart from *never read*, which is no entry at all.
         discovered: ``True`` for a setting the catalog hasn't got, learned from a command
-            the reader ran on *this node's* command line. It is the row's whole claim to
-            exist, so a node that stops answering the key loses the row rather than
-            earning an ``n/a`` — the catalog is what says a key *might* be there.
+            the reader ran on *this node's* command line. Such a row reads ``n/a`` like any
+            other when the node stops answering it, and is removed only by hand: its key is
+            recorded nowhere else, so a misread reply must not be able to delete it.
     """
 
     value: str

meshterm/ui/repeater_admin.py

@@ -28,9 +28,9 @@ asking it — every question being one paced round trip, and every guessed row a
 on everyone else's page. So nothing is probed speculatively: a ``get``/``set`` the reader
 ran here, which *this node* answered, earns a row on *this node's* page under **Extra**
 (:func:`learn_from_cli`). The round trip was one the reader was spending anyway, the catalog
-doesn't grow, and no other node's page changes. A row the node stops answering goes again —
-which is what tidies up after a board swap onto the same identity — and ``Del`` forgets one
-without waiting for a read.
+doesn't grow, and no other node's page changes. A row is only ever removed by hand
+(``Del``): a key that stops answering reads ``n/a`` like any other, because a misread reply
+would otherwise delete the one thing about this node nobody else can restore.
 """
 
 from __future__ import annotations
@@ -403,7 +403,7 @@ async def _admin_session(ctx: AppContext, device: Device, node: Contact) -> dict
                 applied += await _apply(ctx, device, node, pending)
             elif choice == _READ:
                 # This node's page, not the catalog: a discovered row is refreshed by the
-                # same sweep as everything else, and drops off it if the node stops answering.
+                # same sweep as everything else, and reads n/a if it stops answering.
                 await read_settings(ctx, device, node, _all_specs(cache))
             elif choice == _LOCATION:
                 await _stage_location(ctx, cache, pending)
@@ -639,11 +639,16 @@ async def _forget_discovered(
 ) -> None:
     """Drop one discovered row from this node's page, behind a confirm.
 
-    The row usually cleans itself up — a node that stops answering the key loses it on the
-    next read — but that costs a round trip and a node in reach, and neither is a given
-    after the board it was learned from has been replaced. So Del asks, and forgets. It is
-    a red confirm like any other single-record delete, though nothing on the node changes:
-    what goes is what MeshTerm remembers, and asking the key again brings the row back.
+    This is the *only* way a discovered row is removed. Letting a read drop one — a key the
+    node no longer answers is real enough, after a board swap onto a reused identity — would
+    mean a single misread reply silently deleting the row: a truncated line, a node answering
+    mid-reboot, a stray frame correlated to the wrong command. A catalog row costs an ``n/a``
+    when that happens and nothing is lost, because the catalog still names the key; a
+    discovered row's key is known only here, and the reader may not remember what it was. So
+    the same ``n/a`` is all a read may do, and removing is a decision.
+
+    It is a red confirm like any other single-record delete, though nothing on the node
+    changes: what goes is what MeshTerm remembers, and asking the key again brings it back.
     """
     from .tui import CANCEL
 
@@ -824,14 +829,7 @@ def remember_reply(ctx: AppContext, node: Contact, fills: list[RemoteSetting], r
     for spec in fills:
         value = parse_reply_value(spec, reply)  # None for an error reply, too
         if value is None:
-            if spec.discovered:
-                # A discovered row's whole claim to exist is that this node answered the
-                # key. When it stops, the claim is gone and so is the row — ``n/a`` is for
-                # a key the *catalog* says might be there, and nothing says that here. This
-                # is what cleans the page up after a board swap onto the same identity.
-                ctx.remote_store.forget_setting(node, spec.key)
-            else:
-                ctx.remote_store.remember_unsupported(node, spec.key)
+            ctx.remote_store.remember_unsupported(node, spec.key)
         else:
             ctx.remote_store.remember_setting(node, spec.key, value)
             got += 1

tests/test_repeater_admin.py

@@ -895,12 +895,13 @@ def test_a_write_stales_the_other_spelling_from_the_command_line_too(tui_ctx) ->
     assert cache["dutycycle"].value == "40.0" and "af" not in cache
 
 
-def test_a_discovered_row_survives_a_re_read_and_goes_when_the_key_does(tui_ctx) -> None:
-    """The node's answer is the row's whole claim to exist — including after a board swap.
+def test_no_reply_can_remove_a_discovered_row(tui_ctx) -> None:
+    """A read may only ever take a discovered row to ``n/a``; removing it is a decision.
 
-    A refreshed value keeps the row discovered (or the page would hold a row the catalog
-    cannot draw); a key that stops answering loses it, rather than earning the ``n/a`` that
-    only a catalog key can.
+    Its key is recorded nowhere but here, so a reply that misreads — a truncated line, a
+    node answering mid-reboot, a frame correlated to the wrong command — must not be able
+    to delete the row. A catalog row costs nothing when that happens, the catalog still
+    naming the key; this one would cost the reader something only they can restore.
     """
     spec = discovered_setting("radio.rxps")
     tui_ctx.remote_store.remember_discovered(NODE, "radio.rxps", "off")
@@ -910,7 +911,10 @@ def test_a_discovered_row_survives_a_re_read_and_goes_when_the_key_does(tui_ctx)
     assert (cached.value, cached.discovered) == ("balanced", True)
 
     repeater_admin.remember_reply(tui_ctx, NODE, [spec], "??: radio.rxps")
-    assert "radio.rxps" not in tui_ctx.remote_store.settings(NODE)
+    cache = tui_ctx.remote_store.settings(NODE)
+    assert cache["radio.rxps"].discovered and not cache["radio.rxps"].supported
+    assert repeater_admin._value_text(spec, cache, {}).plain == "n/a"
+    assert "radio.rxps" in repeater_admin._extra_keys(cache), "still the reader's to forget"
 
 
 def test_discovered_rows_draw_last_in_their_own_section_and_are_deletable() -> None: