diff --git a/meshterm/core/remote_store.py b/meshterm/core/remote_store.py index fb56042..3fbedb0 100644 --- a/meshterm/core/remote_store.py +++ b/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 diff --git a/meshterm/ui/repeater_admin.py b/meshterm/ui/repeater_admin.py index 0f7dced..0611b60 100644 --- a/meshterm/ui/repeater_admin.py +++ b/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 diff --git a/tests/test_repeater_admin.py b/tests/test_repeater_admin.py index 12ea7c0..3f5ffee 100644 --- a/tests/test_repeater_admin.py +++ b/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: