Room logins keep each password for what it proved, and send one at a time

A review of the room work turned up ways the remembered passwords could
go wrong, and a doubled login:

- A room that lets readers in lets any password in, so a login that only
  earned read-only proved nothing about the password. It replaced the
  room password anyway, so one mistyped ^L left a member read-only for
  good, with no prompt to fix it. It now never replaces a password that
  is already kept, and ^L asks for one while you are read-only.
- A blank login only asks "do you know me?". Accepted, it overwrote the
  remembered room password, and as an admin it emptied the admin
  password Repeater admin shares. It now replaces neither.
- A room's login replaces a known member's role with the one the
  password earns, so an owner who held both passwords was demoted to
  member each time the room password was tried first. The admin password
  now goes first.
- Closing a room view and opening it again mid-login sent a second
  login. The room service now shares a login already in flight, and a
  room with one under way is not due another.
- A refusal forgot the room password even when it refused a different
  one. ^R is no longer offered to a read-only member, whose resend the
  room would drop. Enter on a picked post opens its paths when there is
  nowhere to put a reply, and a paste is refused there too.

Refs: #24
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

meshterm/core/room_store.py

@@ -40,8 +40,9 @@ class RoomMembership:
         key: The :func:`~meshterm.core.admin_store.admin_key` the room is stored under —
             the same key its admin password has, so the two stores agree on which room is
             which (the record's key in the file, not one of its fields).
-        password: The room password that got us in, ``""`` for a room open to anyone, or
-            ``None`` when we got in as its admin (that password lives in the admin store).
+        password: The room password that got us in, ``""`` for a blank login the room
+            accepted (an open room, or one that knew us), or ``None`` when no room password
+            is known — we got in as its admin, whose password lives in the admin store.
         access: The access the room granted at our last login: a
             :class:`~meshterm.core.models.RoomAccess` value.
         label: The room's name, for display.
@@ -136,27 +137,50 @@ class RoomStore:
         record`), for the room password: kept when the room accepts it, forgotten when the
         node *refuses* it, and left as it is when nothing came back — silence is not a
         verdict, and from a room it is also how a wrong password sounds, so a password that
-        never once worked is simply never written. An admin login is remembered as the
-        access alone; its password is the admin store's to keep.
+        never once worked is simply never written.
+
+        "Accepted" is not one proof, though, and a password is only kept for what it proved:
+
+        * **Admin** with a password typed in: the password is the admin store's to keep
+          (it is the room's admin password); here, only the access, beside whatever room
+          password was already remembered.
+        * **Member** with a password typed in: that password is the room password.
+        * **Read-only** proves nothing about the password at all — a room that lets
+          readers in lets *any* password in — so it never replaces one already
+          remembered, which may be a member's; it is kept only where nothing was.
+        * **Blank** asks "do you know me?" and proves only that the room does. It never
+          replaces a remembered password either: a member the room forgets on its next
+          restart would otherwise have swapped a password that works for one that can't.
 
         Args:
             room: The room the login addressed.
             password: The password that was tried.
             login: How the login ended.
         """
+        records = self._load()
+        key = admin_key(room)
+        known = records.get(key)
         if login.result is LoginResult.ACCEPTED and login.access is not None:
-            records = self._load()
-            key = admin_key(room)
+            kept = known.password if known is not None else None
+            if login.access is RoomAccess.ADMIN and password:
+                remembered = kept
+            elif login.access is RoomAccess.MEMBER and password:
+                remembered = password
+            else:
+                remembered = kept if kept is not None else password
             records[key] = RoomMembership(
                 key=key,
-                password=None if login.access is RoomAccess.ADMIN else password,
+                password=remembered,
                 access=login.access.value,
                 label=room.name,
                 last_login=utcnow().isoformat(),
             )
             self._write(records)
-        elif login.result is LoginResult.REFUSED:
-            self.forget(room)
+        elif login.result is LoginResult.REFUSED and known is not None:
+            # Only the password the node turned down is forgotten.
+            if known.password == password:
+                records.pop(key)
+                self._write(records)
 
     def forget(self, room: Contact) -> None:
         """Remove everything remembered about ``room``.

meshterm/services/rooms.py

@@ -21,6 +21,7 @@ Session-scoped state on the :class:`~meshterm.context.AppContext` (``ctx.rooms``
 
 from __future__ import annotations
 
+import asyncio
 import time
 from typing import TYPE_CHECKING
 
@@ -45,6 +46,8 @@ class RoomService:
         #: When each room was last heard from this session (a login or a post), on the
         #: monotonic clock, keyed by room peer.
         self._heard: dict[str, float] = {}
+        #: The login on its way to each room, and the password it carries, keyed by peer.
+        self._inflight: dict[str, tuple[asyncio.Future, str]] = {}
 
     @staticmethod
     def _peer(room: Contact) -> str:
@@ -54,14 +57,17 @@ class RoomService:
     def password(self, room: Contact) -> str | None:
         """The password to log in to ``room`` with, or ``None`` when we hold none.
 
-        The room password first — it is what a member was given, and what got us in —
-        then the room's admin password, which Repeater admin may already remember and
-        which gets us in as its admin. ``""`` is a password (an open room).
+        The room's admin password first, when Repeater admin (or a join) remembers one: it
+        gets us everything the room password does and more. The order is not taste — a
+        room's login *replaces* a known member's role with the one the password earns, so
+        an owner who logged in with the room password would be demoted to member until
+        their next admin login. Then the room password. ``""`` is a password (a blank
+        login the room accepted).
         """
-        stored = self._ctx.room_store.password(room) if self._ctx.room_store else None
-        if stored is not None:
-            return stored
-        return self._ctx.admin_store.get(room)
+        admin = self._ctx.admin_store.get(room)
+        if admin:
+            return admin
+        return self._ctx.room_store.password(room) if self._ctx.room_store else None
 
     def joined(self, room: Contact) -> bool:
         """Whether MeshTerm can log in to ``room`` without asking for a password."""
@@ -100,18 +106,32 @@ class RoomService:
         )
 
     def login_due(self, room: Contact) -> bool:
-        """Whether opening ``room`` should log in to it: a password at hand and a quiet room."""
-        return self.joined(room) and not self.heard_recently(room)
+        """Whether opening ``room`` should log in: a password at hand, and a quiet room.
+
+        Never while a login to it is already on its way — that one's answer is this one's.
+        """
+        return (
+            self.joined(room)
+            and not self.heard_recently(room)
+            and self._peer(room) not in self._inflight
+        )
 
     async def login(self, room: Contact, password: str) -> RoomLogin:
         """Log in to ``room`` with ``password`` — one exchange — and remember the outcome.
 
         An accepted login is remembered the way each password is kept: the access and the
-        room password in the room store, an admin password in the admin store (shared with
-        Repeater admin, which will log in with it silently from now on). A login the node
-        *refused* forgets the password that was tried; one that met silence keeps
-        everything, since a room also stays silent for a wrong password and only a
-        password that once worked is ever stored.
+        room password in the room store (:meth:`~meshterm.core.room_store.RoomStore.
+        record` says what each kind of acceptance proves), and an admin password typed in
+        in the admin store, shared with Repeater admin, which logs in with it silently from
+        then on. A login the node *refused* forgets the password that was tried; one that
+        met silence keeps everything, since a room also stays silent for a wrong password
+        and only a password that once worked is ever stored.
+
+        One login to a room at a time, whoever asks. A login can take most of its reply
+        budget, and a room view closed and opened again inside it would otherwise send a
+        second one — both of which the room answers by restarting its catch-up. A caller
+        asking with the password already on its way shares that login's answer; one with
+        a different password waits for it to finish, then sends its own.
 
         Args:
             room: The room server to log in to.
@@ -124,11 +144,38 @@ class RoomService:
             Exception: Propagates a device error (no connection, a contact the companion
                 cannot address); nothing is remembered then.
         """
+        peer = self._peer(room)
+        running = self._inflight.get(peer)
+        if running is not None:
+            task, sent = running
+            if sent == password:
+                return await asyncio.shield(task)
+            try:
+                await asyncio.shield(task)
+            except Exception:  # noqa: BLE001 - its own caller hears about its failure
+                pass
+        task = asyncio.ensure_future(self._login_once(room, password))
+        self._inflight[peer] = (task, password)
+        # Cleared when the exchange ends, not when this caller stops waiting: a room view
+        # closed mid-login leaves the login running, and it is still the one in flight.
+        task.add_done_callback(lambda done: self._settle(peer, done))
+        return await asyncio.shield(task)
+
+    def _settle(self, peer: str, task: asyncio.Future) -> None:
+        """Forget a finished login as the one in flight, retrieving a failure no one awaits."""
+        if self._inflight.get(peer, (None, None))[0] is task:
+            del self._inflight[peer]
+        if not task.cancelled():
+            task.exception()
+
+    async def _login_once(self, room: Contact, password: str) -> RoomLogin:
+        """One login exchange, and what its outcome means for the stores and the session."""
         device = await self._ctx.device()
         login = await device.room_login(room, password)
         if self._ctx.room_store is not None:
             self._ctx.room_store.record(room, password, login)
-        if login.access is RoomAccess.ADMIN:
+        if login.access is RoomAccess.ADMIN and password:
+            # A blank login proves the room knows us, not what the admin password is.
             self._ctx.admin_store.record(room, password, LoginResult.ACCEPTED)
         elif login.result is LoginResult.REFUSED and password == self._ctx.admin_store.get(room):
             self._ctx.admin_store.record(room, password, login.result)

meshterm/ui/chat.py

@@ -363,6 +363,15 @@ class ChatScreen(Screen):
     #: What an empty transcript says.
     _empty_text = "No messages yet — say hello!"
 
+    @property
+    def _replies(self) -> bool:
+        """Whether Enter on a picked message replies to it (else it opens its paths).
+
+        Where there are many voices and an @mention to say which one is answered — and
+        only while there is a compose line to put the reply on.
+        """
+        return self._multiparty and self._composing
+
     def _compose_line(self, width: int) -> RenderableType:
         """The compose line under the transcript: the input, with its byte budget.
 
@@ -690,7 +699,7 @@ class ChatScreen(Screen):
         message = self._messages[self._selected]
         sender, _ = self._sender_and_body(message)
         who = "this message" if message.outbound or sender == "·" else sender
-        if self._multiparty:
+        if self._replies:
             return Text(
                 f"↩ Enter to reply to {who} with an @mention · End to cancel",
                 style="accent",
@@ -758,7 +767,7 @@ class ChatScreen(Screen):
         """
         if action == "enter":
             if self._selected is not None:
-                if self._multiparty:
+                if self._replies:
                     self._begin_reply()
                 else:
                     self._open_paths(self._selected)
@@ -868,7 +877,7 @@ class ChatScreen(Screen):
         is one line); an empty result never opens the dialog. One paste dialog at a time,
         scheduled off the key handler like the paths view.
         """
-        if self._paste_open:
+        if self._paste_open or not self._composing:
             return
         clean = "".join(ch if ch.isprintable() else " " for ch in data).strip()
         if not clean:

meshterm/ui/room.py

@@ -281,16 +281,29 @@ class RoomScreen(ChatScreen):
     def handle(self, action: str, data: str = "") -> None:
         """^L logs in; everything else is the chat's."""
         if action == "login":
-            self.begin_login(ask=self._login_state in (NO_REPLY, REFUSED, NOT_JOINED))
+            # Asked for, rather than sent with what is remembered, wherever that would only
+            # repeat the outcome on screen: after silence (a room's answer to a wrong
+            # password too), and when all we are is read-only — the room password is what
+            # lets a reader post.
+            ask = self._login_state in (NO_REPLY, REFUSED, NOT_JOINED) or (
+                self._access is RoomAccess.READ_ONLY
+            )
+            self.begin_login(ask=ask)
             self._session.invalidate()
             return
         super().handle(action, data)
 
+    def _retry_target(self) -> ChatMessage | None:
+        """An unacknowledged post to send again — never for a reader the room won't hear."""
+        return super()._retry_target() if self._composing else None
+
     @property
     def footer_hint(self) -> str:
         """Key hint: the chat's, in a board's words, with ^L while a login could start."""
         if self._selected is not None:
             code = " · ^U QR" if self._picked_urls() else ""
+            if not self._composing:  # nowhere to put a reply: Enter shows the post's paths
+                return f"Enter paths{code} · ↑↓ pick · ^End/Esc cancel"
             return f"Enter reply (@mention) · ^P paths{code} · ↑↓ pick · ^End/Esc cancel"
         atoms = ["Enter send"] if self._composing else []
         atoms.append("↑ pick a message")

tests/test_rooms.py

@@ -316,14 +316,81 @@ class _Ctx:
         return self._device
 
 
-def test_the_room_password_is_tried_before_the_admin_password(tmp_path: Path) -> None:
-    """A member's password first; the admin password, which also gets us in, after."""
+def test_the_admin_password_is_tried_before_the_room_password(tmp_path: Path) -> None:
+    """A login replaces a known member's role, so the room password would demote an owner."""
     ctx = _Ctx(tmp_path, MockDevice())
     assert ctx.rooms.password(ROOM) is None and not ctx.rooms.joined(ROOM)
-    ctx.admin_store.remember(ROOM, "admin")
-    assert ctx.rooms.password(ROOM) == "admin"
     ctx.room_store.record(ROOM, "hello", RoomLogin(LoginResult.ACCEPTED, RoomAccess.MEMBER))
     assert ctx.rooms.password(ROOM) == "hello"
+    ctx.admin_store.remember(ROOM, "admin")
+    assert ctx.rooms.password(ROOM) == "admin"
+
+
+def test_read_only_never_replaces_the_room_password(rooms: RoomStore) -> None:
+    """A room that lets readers in lets any password in: that proves nothing about one."""
+    rooms.record(ROOM, "hello", RoomLogin(LoginResult.ACCEPTED, RoomAccess.MEMBER))
+    rooms.record(ROOM, "helo", RoomLogin(LoginResult.ACCEPTED, RoomAccess.READ_ONLY))
+    assert rooms.password(ROOM) == "hello"
+    assert rooms.access(ROOM) is RoomAccess.READ_ONLY
+    first = RoomStore(rooms._path.with_name("other.json"))
+    first.record(ROOM, "guess", RoomLogin(LoginResult.ACCEPTED, RoomAccess.READ_ONLY))
+    assert first.password(ROOM) == "guess", "with nothing better, it is what gets back in"
+
+
+def test_a_blank_login_never_replaces_a_password(rooms: RoomStore) -> None:
+    """Blank proves the room knows us — until it restarts and forgets a member."""
+    rooms.record(ROOM, "hello", RoomLogin(LoginResult.ACCEPTED, RoomAccess.MEMBER))
+    rooms.record(ROOM, "", RoomLogin(LoginResult.ACCEPTED, RoomAccess.MEMBER))
+    assert rooms.password(ROOM) == "hello"
+    rooms.record(ROOM, "", RoomLogin(LoginResult.ACCEPTED, RoomAccess.ADMIN))
+    assert rooms.password(ROOM) == "hello"
+
+
+def test_an_admin_login_keeps_the_room_password_beside_it(rooms: RoomStore) -> None:
+    """The admin password lives in the admin store; the member's stays here as the fallback."""
+    rooms.record(ROOM, "hello", RoomLogin(LoginResult.ACCEPTED, RoomAccess.MEMBER))
+    rooms.record(ROOM, "admin", RoomLogin(LoginResult.ACCEPTED, RoomAccess.ADMIN))
+    assert rooms.password(ROOM) == "hello" and rooms.access(ROOM) is RoomAccess.ADMIN
+
+
+def test_a_refusal_forgets_only_the_password_it_refused(rooms: RoomStore) -> None:
+    """A node turning one password down says nothing about another."""
+    rooms.record(ROOM, "hello", RoomLogin(LoginResult.ACCEPTED, RoomAccess.MEMBER))
+    rooms.record(ROOM, "other", RoomLogin(LoginResult.REFUSED))
+    assert rooms.password(ROOM) == "hello"
+
+
+async def test_a_blank_admin_login_leaves_the_admin_password_alone(tmp_path: Path) -> None:
+    """Repeater admin shares that password; a blank probe must not erase it."""
+    ctx = _Ctx(tmp_path, MockDevice())
+    await ctx.rooms.login(ROOM, "admin")
+    login = await ctx.rooms.login(ROOM, "")
+    assert login.access is RoomAccess.ADMIN
+    assert ctx.admin_store.get(ROOM) == "admin"
+
+
+async def test_one_login_to_a_room_at_a_time(tmp_path: Path) -> None:
+    """A view closed and reopened mid-login shares the login rather than sending another."""
+    device = MockDevice()
+    ctx = _Ctx(tmp_path, device)
+    ctx.room_store.record(ROOM, "hello", RoomLogin(LoginResult.ACCEPTED, RoomAccess.MEMBER))
+    sent: list[str] = []
+    original = device.room_login
+
+    async def slow(room, password):  # noqa: ANN001, ANN202
+        sent.append(password)
+        await asyncio.sleep(0.05)
+        return await original(room, password)
+
+    device.room_login = slow
+    first = asyncio.ensure_future(ctx.rooms.login(ROOM, "hello"))
+    await asyncio.sleep(0)
+    assert not ctx.rooms.login_due(ROOM), "a login on its way is not due again"
+    second = await ctx.rooms.login(ROOM, "hello")
+    assert (await first) == second
+    assert sent == ["hello"]
+    third = await ctx.rooms.login(ROOM, "hello")  # once it is answered, a new one may go
+    assert third and sent == ["hello", "hello"]
 
 
 async def test_joining_with_the_room_password_makes_us_a_member(tmp_path: Path) -> None:
@@ -890,3 +957,34 @@ def test_chat_send_to_a_room_is_a_post(cli) -> None:  # noqa: ANN001
     receipt = json.loads(cli("--json", "chat", "send", "--to", "Lakeside BBS", "hi").stdout)
     assert receipt["kind"] == "room"
     assert receipt["acked"] is False, "this simulator never joined, so the room drops it"
+
+
+async def test_a_read_only_member_is_asked_for_a_password_and_offered_no_retry() -> None:
+    """^L asks (the room password is what lets a reader post); ^R would only be dropped."""
+    unacked = ChatMessage(text="lost", outbound=True, peer=ROOM.key_prefix, acked=False)
+    screen, calls = _view([unacked], access=RoomAccess.READ_ONLY)
+    assert screen._retry_target() is None
+    assert "^R" not in screen.footer_hint
+    screen.handle("login")
+    while screen._login_open:
+        await asyncio.sleep(0)
+    assert calls["asked"] == 1
+
+
+async def test_a_read_only_member_picks_a_post_to_see_its_paths_not_to_reply() -> None:
+    """With no compose line, Enter on a post has nothing to prime, and a paste nowhere to go."""
+    screen, _ = _view(_board(), access=RoomAccess.READ_ONLY)
+    opened: list[ChatMessage] = []
+
+    async def paths(message: ChatMessage) -> None:
+        opened.append(message)
+
+    screen._paths = paths
+    screen.handle("paste", "pasted text")
+    assert not screen._paste_open and screen._editor.text == ""
+    screen.handle("up")
+    assert screen.footer_hint.startswith("Enter paths")
+    screen.handle("enter")
+    await asyncio.sleep(0)
+    assert screen._editor.text == ""
+    assert [m.text for m in opened] == ["I'll check it"]