Channels shows the wait instead of announcing the result

Three things, all the same story - the manager was silent while it
worked and talkative once the work was already visible.

The acknowledgement goes. It reported changes the reader had just
watched land in the list in front of them, after the screen showing
them had closed. The count stays in summary, for the run log.

Every device action now reports while it runs, under the modal busy
card: write_channel draws its own (it is the one boundary every
mutation crosses), a reorder draws one card for the batch and retitles
it per move, and the re-read that follows a change draws one rather
than three. Modal is the point - the list underneath stays armed the
whole time it waits, so keys tapped during a slow write were landing
in its filter and coming back as a list showing nothing.

And "a write happened" moves off the change count onto the cache the
write drops. A write that landed and then raised - a QR view unwound
with ^W, a link lost mid-reorder - never reached changes += 1, so the
stale slot list and the chat service's slot map outlived a layout that
had already moved. write_channel now invalidates as it writes, which
the four CLI subcommands were never doing at all, and the manager
compares the epoch across a round.

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

meshterm/services/device_state.py

@@ -94,6 +94,7 @@ class DeviceState:
         self._self_info: dict | None = None
         self._path_hash_mode: int | None = None
         self._channels: list[ChannelSlot] | None = None
+        self._channels_epoch = 0
         self._channel_capacity: int | None = None
         # One lock per slow fetch so overlapping first-access callers (two screens opened in
         # quick succession) collapse onto a single round-trip instead of each firing their own.
@@ -373,9 +374,21 @@ class DeviceState:
         """Drop the cached path-hash mode — call after writing it in the config editor."""
         self._path_hash_mode = None
 
+    @property
+    def channels_epoch(self) -> int:
+        """How many times the channel layout has been dropped this session.
+
+        A caller that wants to know whether *anything* rewrote a slot while it was busy
+        compares this across the gap rather than trusting a return value: a write that
+        landed and then raised on its way back moved the device just as much as one that
+        returned cleanly, and the count it never got to increment cannot say so.
+        """
+        return self._channels_epoch
+
     def invalidate_channels(self) -> None:
         """Drop the cached channel slots — call after the channel editor saves or clears one."""
         self._channels = None
+        self._channels_epoch += 1
 
     def invalidate_config(self) -> None:
         """Drop everything the config editor can change in one call (self-info + routing mode).

meshterm/tools/channels.py

@@ -54,7 +54,7 @@ class ChannelsTool(Tool):
             params: A ``cli_action`` with its arguments (CLI), or empty for the menu.
 
         Returns:
-            A :class:`ToolResult` summarizing what happened.
+            A :class:`ToolResult` recording what happened (the menu path shows nothing).
         """
         action = params.get("cli_action")
         if action is not None:
@@ -62,14 +62,11 @@ class ChannelsTool(Tool):
 
         from ..ui.channels import manage_channels
 
-        changes = await manage_channels(ctx)
-        message = (
-            f"[ok]✓[/ok] applied [brand]{changes}[/brand] "
-            f"channel change{'' if changes == 1 else 's'}"
-            if changes
-            else None
-        )
-        return ToolResult(summary={"changes": changes}, message=message)
+        # No message: the manager acknowledged its work on the way out, which meant telling
+        # the reader that changes they had just watched land were applied. The count still
+        # goes to the run log through ``summary``, which is the record of what an invocation
+        # did rather than anything shown.
+        return ToolResult(summary={"changes": await manage_channels(ctx)})
 
     # -- CLI --------------------------------------------------------------------
 

meshterm/ui/channels.py

@@ -146,21 +146,24 @@ class ChannelSlot:
 async def manage_channels(ctx: AppContext) -> int:
     """Run the interactive channel manager until the user backs out.
 
-    The visit acknowledges itself **once**, through the count returned here: the tool turns
-    it into a single line and the menu floats that as an OK popup. Each action used to bank
-    a "✓ created …" note of its own on top of that, which nobody could read until the
-    manager closed — by which time the refreshed list had already shown the result — and
-    three of them in one visit pushed the outcome past the popup's line budget (see
-    :func:`~meshterm.ui.surface._collapse_to_message`) and into the full result window, so
-    the same visit reported itself as a dialog or as a window depending on how much had been
-    done in it. An error is the exception, and is shown when it happens rather than banked
-    (see :func:`_import_link`).
+    The visit says **nothing** on the way out. It used to bank a note per action and then an
+    "✓ applied 3 channel changes" line on top of those, shown once the manager had already
+    closed — an acknowledgement for changes the reader had just watched land in the list in
+    front of them, arriving as a tidy popup or as a full-frame window depending on how many
+    lines had piled up. An error is the exception, and is shown when it happens (see
+    :func:`_import_link`).
+
+    What the reader gets instead is the wait itself: every device action reports *while it
+    runs*, under the modal busy card (:meth:`~meshterm.ui.surface.Ui.busy_dialog`) — see
+    :func:`write_channel` and ``settle`` below for where those are drawn, and why the card
+    has to be modal.
 
     Args:
         ctx: Shared application context (provides the connected device and UI surface).
 
     Returns:
-        The number of channels created, changed, or cleared during the session.
+        How many channels were created, changed, or cleared — the run log's record of what
+        the visit did (``ToolResult.summary``), not anything the reader is shown.
     """
     device = await ctx.device()
     # The firmware's slot count is fixed for the session, so discover it once (a read-only
@@ -174,7 +177,6 @@ async def manage_channels(ctx: AppContext) -> int:
     stats = _LiveStats(ctx)
     changes = 0
     highlight: object | None = None
-
     slots: list = []
 
     async def reload() -> tuple[str, list]:
@@ -190,13 +192,36 @@ async def manage_channels(ctx: AppContext) -> int:
         slots = list(await ctx.devstate.channel_slots())
         return _menu_items(ctx, slots, capacity, stats)
 
+    async def settle(*, moved: bool = False) -> tuple[str, list]:
+        """Re-probe the layout — and, when a slot actually moved, re-file what points at it.
+
+        Two device round-trips back to back when something changed (the chat service's slot
+        map and the slot probe ``reload`` calls one of the slowest reads the app makes), so
+        they report as a single wait rather than boxes flashing in sequence — and while the
+        card is up the reader cannot type into a list whose rows are being replaced
+        underneath them.
+
+        Args:
+            moved: Whether a slot's occupant changed during the round just finished. The
+                caller decides that by comparing ``devstate.channels_epoch``, not by trusting
+                a count (see :func:`write_channel`, which drops the cache as it writes).
+        """
+        async with ctx.ui.busy_dialog("reading channels…", title="Channels"):
+            if moved:
+                # Inbound messages carry only a slot index, which the chat service maps to a
+                # channel identity through a cache keyed by slot; refresh it now so a message
+                # on a reused/re-keyed slot is filed under the channel that's actually there
+                # and not the one that used to be — otherwise its transcript surfaces in the
+                # wrong chat.
+                await _refresh_chat_channels(ctx)
+            return await reload()
+
     async def handle(choice: object) -> bool:
         """Dispatch one menu choice (over the still-pushed list); ``False`` exits."""
         nonlocal changes, highlight
         if choice is None:  # Esc
             return False
         highlight = choice
-        before = changes
         if choice == _CREATE:
             changes += await _create_private(ctx, device, slots, capacity)
         elif choice == _DEFAULT_PUBLIC:
@@ -213,17 +238,6 @@ async def manage_channels(ctx: AppContext) -> int:
             slot = next((s for s in slots if s.idx == choice), None)
             if slot is not None:
                 changes += await _channel_detail(ctx, device, slot, stats)
-        if changes > before:
-            # A slot's occupant changed (created, re-keyed, cleared, or moved). Drop the
-            # session cache's channel list so the dashboard (and anything else reading it)
-            # re-reads the new layout instead of a stale copy.
-            ctx.devstate.invalidate_channels()
-            # Inbound messages carry only a slot index, which the chat service maps to a
-            # channel identity through a cache keyed by slot; refresh it now so a message
-            # on a reused/re-keyed slot is filed under the channel that's actually there
-            # and not the one that used to be — otherwise its transcript surfaces in the
-            # wrong chat.
-            await _refresh_chat_channels(ctx)
         return True
 
     # The list stays pushed for the whole visit: every sub-flow — creating a channel,
@@ -231,20 +245,38 @@ async def manage_channels(ctx: AppContext) -> int:
     # swapped in place afterwards, so the highlight (and any typed filter) survives a layout
     # that changed under it. A surface with no session — the plain CLI, scripted tests — has
     # no stack to stay on and runs the same dispatcher a round at a time.
-    title, items = await reload()
+    title, items = await settle()
     session = getattr(ctx.ui, "session", None)
     if session is None:
         while True:
-            if not await _menu_round(ctx, title, items, default=highlight, handle=handle):
+            epoch = ctx.devstate.channels_epoch
+            try:
+                keep = await _menu_round(ctx, title, items, default=highlight, handle=handle)
+            except BaseException:
+                # A write that landed and then raised — or a ^W unwinding out through the QR
+                # view opened after it — moved the device just as much as one that returned
+                # cleanly. The epoch says so where the action's return value never got the
+                # chance to, so the chat map is re-filed on the way past.
+                if ctx.devstate.channels_epoch != epoch:
+                    await _refresh_chat_channels(ctx)
+                raise
+            if not keep:
                 return changes
-            title, items = await reload()
+            title, items = await settle(moved=ctx.devstate.channels_epoch != epoch)
     menu = SelectScreen(title, items)
     async with session.stay(menu) as visit:
         while True:
             choice = await visit.result()
-            if not await handle(None if choice is CANCEL else choice):
+            epoch = ctx.devstate.channels_epoch
+            try:
+                keep = await handle(None if choice is CANCEL else choice)
+            except BaseException:
+                if ctx.devstate.channels_epoch != epoch:  # see the note on the other loop
+                    await _refresh_chat_channels(ctx)
+                raise
+            if not keep:
                 return changes
-            title, items = await reload()
+            title, items = await settle(moved=ctx.devstate.channels_epoch != epoch)
             menu.replace_items(items, title=title)
 
 
@@ -354,6 +386,11 @@ async def write_channel(
     a name-derived channel's derived key so it can later be replayed as-is. Remembering is
     best-effort: it never blocks or fails the actual device write.
 
+    Being the one boundary, it is also where the *wait* is reported: the write and the
+    identity probe behind it are a device round-trip each, and until the card went up the
+    list sat there looking idle and fully interactive while they ran. A batch (a reorder)
+    nests inside its caller's card rather than flashing one box per slot.
+
     Args:
         ctx: Shared application context (for the channel store and cached self-info).
         device: The connected device to write to.
@@ -361,6 +398,21 @@ async def write_channel(
         name: The channel name (empty clears the slot).
         secret: The 16-byte secret, or ``None`` to let the firmware derive it from the name.
     """
+    caption = f"clearing slot {idx}…" if not name.strip() else f"saving {name.strip()}…"
+    async with ctx.ui.busy_dialog(caption, title="Channels"):
+        await _write_and_remember(ctx, device, idx, name, secret)
+    # The layout on the device is no longer what anything cached, so say so here rather
+    # than leaving each caller to remember: the menu's manager did (and lost it whenever an
+    # action raised on its way back), and the four CLI subcommands never did at all. The
+    # bump also *is* the signal a caller compares across a round — see
+    # :attr:`~meshterm.services.device_state.DeviceState.channels_epoch`.
+    ctx.devstate.invalidate_channels()
+
+
+async def _write_and_remember(
+    ctx: AppContext, device: Device, idx: int, name: str, secret: bytes | None
+) -> None:
+    """Do the write itself and the channel-store bookkeeping (see :func:`write_channel`)."""
     await device.set_channel(idx, name, secret)
     store = ctx.channel_store
     if store is None:
@@ -944,14 +996,22 @@ async def _apply_order(
     in-memory snapshot, so the interleaved writes never clobber a not-yet-placed channel.
     """
     indices = sorted(s.idx for s in slots)  # the physical slots to fill, ascending
-    changes = 0
-    for target_idx, pos in zip(indices, order, strict=True):
-        slot = slots[pos]
-        if slot.idx == target_idx:
-            continue  # already in place; no write needed
-        await _write_slot(ctx, device, target_idx, slot)
-        changes += 1
-    return changes
+    moves = [
+        (target_idx, slots[pos])
+        for target_idx, pos in zip(indices, order, strict=True)
+        if slots[pos].idx != target_idx  # already in place; no write needed
+    ]
+    if not moves:
+        return 0
+    # One card for the whole batch, retitled as it walks: this is the longest thing the
+    # manager does — a write per moved channel, each a device round-trip — and the one place
+    # where a count is worth showing, because the reader can see it is progressing rather
+    # than stuck. The per-write cards inside nest into this one.
+    async with ctx.ui.busy_dialog("reordering channels…", title="Channels") as busy:
+        for position, (target_idx, slot) in enumerate(moves, start=1):
+            busy.message = f"moving {slot.name} · {position}/{len(moves)}"
+            await _write_slot(ctx, device, target_idx, slot)
+    return len(moves)
 
 
 async def _reorder_channels(ctx: AppContext, device: Device, slots: list[ChannelSlot]) -> int:

meshterm/ui/surface.py

@@ -52,6 +52,14 @@ def _dialog_max_cells() -> int:
     return _DIALOG_MAX_CELLS if get_platform().frame_border else 43
 
 
+class _NullBusy:
+    """The scripted surface's stand-in for a busy card: a caption that goes nowhere."""
+
+    def __init__(self) -> None:
+        """Start with an empty caption; assigning to it is the whole of the contract."""
+        self.message = ""
+
+
 def _collapse_to_message(buffered: list[RenderableType]) -> Text | None:
     """Collapse small, text-only buffered output into one dialog message.
 
@@ -420,9 +428,13 @@ class PlainUi(Ui):
         yield
 
     @asynccontextmanager
-    async def busy_dialog(self, message: str = "", *, title: str = "") -> AsyncIterator[None]:
-        """Do nothing: there is no screen to interrupt and no keyboard to take."""
-        yield
+    async def busy_dialog(self, message: str = "", *, title: str = "") -> AsyncIterator[_NullBusy]:
+        """Do nothing: there is no screen to interrupt and no keyboard to take.
+
+        Still yields something with a settable caption, because a caller that retitles the
+        card mid-batch must not have to ask which surface it is running on.
+        """
+        yield _NullBusy()
 
     def _no_prompt(self) -> RuntimeError:
         """Build the error raised if a rich prompt is reached on the non-interactive path."""

tests/test_channels.py

@@ -7,8 +7,10 @@ run against the :class:`MockDevice` simulator and a temporary database.
 from __future__ import annotations
 
 import re
+from contextlib import asynccontextmanager
 from hashlib import sha256
 from pathlib import Path
+from types import SimpleNamespace
 
 import pytest
 from rich.console import Console
@@ -370,6 +372,11 @@ class _ScriptedUi:
     def show(self, *renderables) -> None:  # noqa: ANN002
         pass
 
+    @asynccontextmanager
+    async def busy_dialog(self, message: str = "", *, title: str = ""):  # noqa: ANN201
+        """No card without a screen stack — every mutation path goes through this."""
+        yield SimpleNamespace(message=message)
+
     def note(self, markup: str) -> None:
         pass
 
@@ -911,13 +918,17 @@ class _PresentingUi(_ScriptedUi):
         self.surface.ack(markup)
 
 
-async def test_a_busy_visit_still_reports_itself_in_a_popup(ctx: AppContext) -> None:
-    """Three changes in one visit must acknowledge as a dialog, not the full result window.
+async def test_a_busy_visit_says_nothing_on_the_way_out(ctx: AppContext) -> None:
+    """A visit that changed three channels closes silently — no popup, no window.
 
-    Reproduces the reported bug: every action banked its own "✓ created …" note, so the
-    outcome grew a line per change and the third one pushed it past the popup's budget —
-    the same visit reported itself as a tidy dialog or as a full-frame window depending on
-    how much had been done in it. The visit now acknowledges once, in the tool's summary.
+    The history this pins, in the order it happened: every action banked its own "✓ created
+    …" note, so the outcome grew a line per change and the third pushed it past the
+    acknowledgement popup's budget into the full-frame result window — the same visit
+    reporting itself two different ways depending on how much had been done in it. Folding
+    those into one summary line fixed the shape and left the real problem: an
+    acknowledgement for changes the reader had just watched land in the list in front of
+    them. So the line went too. The count survives in ``summary`` for the run log, which
+    nobody reads off the screen.
     """
     ui = _PresentingUi(
         selects=[_CREATE, _CREATE, _CREATE, None],  # create three channels, then Esc
@@ -927,9 +938,10 @@ async def test_a_busy_visit_still_reports_itself_in_a_popup(ctx: AppContext) ->
     ctx.ui = ui
     result = await ChannelsTool().run(ctx, {})
 
-    assert result.summary == {"changes": 3}
+    assert result.summary == {"changes": 3}  # the run log still records what was done
+    assert result.message is None
     # The menu's own tail: note the tool's message, then present what the run buffered.
     if result.message:
         ui.surface.note(result.message)
     await ui.surface.present(title="Channels")
-    assert ui.session_spy.shown == [("popup", "Channels")]
+    assert ui.session_spy.shown == []  # nothing shown, in either shape