feat(ipc): the control socket carries every web command; mailboxes are a fallback (stage 4) (#765)

- client: ControlError.sent says whether the display had the request;
  should_fall_back() allows a mailbox write only when it did not, or when
  the display is too old to know the command (upgrade case)
- on-demand start/stop: a display that had the request and failed it is
  answered 503 (400 for invalid_args), no mailbox copy
- errors.clear: new socket command, answered on the connection thread by
  a handler the display registers; applied and republished before the
  answer; plugin_error_clear_request only on fallback
- display: on-demand mailbox looked at once a second while the socket is
  up (0.25 s without), read only when its file changed (one stat via
  CacheManager.file_signature / MailboxWatch); socket commands no longer
  touch the mailbox; a processed duplicate is consumed; writers logged once
- error publisher: mailbox read only when changed; snapshot carries
  applied_clear_cutoff so an older mailbox request is not shown pending
- docs and CHANGELOG (mailboxes kept for one release)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

src/ipc/client.py

@@ -3,8 +3,13 @@
 Every failure -- no socket (the display is stopped, or predates the socket),
 a refused or timed-out connection, a reply that breaks the contract, or an
 error the display returned -- raises :class:`ControlError` with a short
-``reason``, and the caller falls back to the file mailbox. Nothing here
-blocks for longer than ``timeout`` in total.
+``reason``. Nothing here blocks for longer than ``timeout`` in total.
+
+Whether the caller may then write the file mailbox instead is
+:func:`should_fall_back`: only when the display never took the request (it
+could not be reached, or it is too old to know the command). A display that
+took the request and then failed, refused or went quiet is answered as
+that, not posted a second time through the mailbox.
 """
 
 from __future__ import annotations
@@ -22,6 +27,7 @@ from src.ipc.contract import (
     SUBSCRIBE_KEEPALIVE_SECONDS,
     SUPPORTED_VERSIONS,
     Command,
+    ErrorCode,
     FrameReader,
     ProtocolError,
     Request,
@@ -49,17 +55,49 @@ class ControlError(Exception):
     ``refused``, ``timeout``, ``closed``, ``bad_response``, ``invalid_request``.
     When the display answered with an error, ``reason`` is that error's
     :class:`~src.ipc.contract.ErrorCode` (``busy``, ``unknown_command``, ...).
+
+    ``sent`` is True once the whole request was written to a connected
+    display, which may then have acted on it. A refusal the display sends
+    before it reads anything (``forbidden``, too many connections) carries
+    no request id and leaves ``sent`` False.
     """
 
-    def __init__(self, reason: str, message: str = ''):
+    def __init__(self, reason: str, message: str = '', *, sent: bool = False):
         super().__init__(reason, message)
         self.reason = reason
         self.message = message
+        self.sent = sent
 
     def __str__(self) -> str:
         return f'{self.reason}: {self.message}' if self.message else self.reason
 
 
+#: Answers from a display that read the request but does not speak it: one
+#: older than the command (an upgrade in progress) or the protocol version.
+#: It did nothing, so the mailbox is the way to reach it.
+UPGRADE_REASONS = frozenset({ErrorCode.UNKNOWN_COMMAND, ErrorCode.UNSUPPORTED_VERSION})
+
+
+def should_fall_back(error: BaseException) -> bool:
+    """May the caller write the file mailbox after ``error``?
+
+    Yes when the display never took the request: there is no socket (the
+    display is stopped, predates the socket, or it is switched off), the
+    connection was refused or timed out, the display turned the connection
+    away before reading it, or it is too old to know the command
+    (:data:`UPGRADE_REASONS`). Also for an error that is not a
+    :class:`ControlError` (a bug in the client), as before.
+
+    No once the display had the request: a ``busy`` queue, ``invalid_args``,
+    an ``internal`` error, or a timeout or hang-up after the request was
+    sent. The display may have applied it, or would refuse it from the
+    mailbox too, so a second copy there only hides the failure.
+    """
+    if not isinstance(error, ControlError):
+        return True
+    return not error.sent or error.reason in UPGRADE_REASONS
+
+
 def request(cmd: str, args: Optional[Mapping[str, Any]] = None, *,
             request_id: Optional[str] = None,
             timeout: float = DEFAULT_TIMEOUT_SECONDS,
@@ -93,11 +131,14 @@ def request(cmd: str, args: Optional[Mapping[str, Any]] = None, *,
     # A refusal before the request was read (forbidden, too many
     # connections) carries no id.
     if response.id != request_id and not (response.id is None and not response.ok):
-        raise ControlError('bad_response', 'the reply is for a different request')
+        raise ControlError('bad_response', 'the reply is for a different request', sent=True)
     if not response.ok:
         error = response.error
+        # No id: refused at the door (forbidden, too many connections),
+        # before the display read the request.
         raise ControlError(error.code if error else 'bad_response',
-                           error.message if error else '')
+                           error.message if error else '',
+                           sent=response.id is not None)
     return dict(response.result or {})
 
 
@@ -142,26 +183,31 @@ def _connect(paths: Sequence[str], deadline: float) -> socket.socket:
 
 
 def _exchange(sock: socket.socket, payload: bytes, deadline: float) -> Response:
+    """Send ``payload`` and read the reply. A failure once the whole request
+    is written raises with ``sent=True``: the display may have it."""
+    sent = False
     try:
         sock.settimeout(_remaining(deadline))
         sock.sendall(payload)
+        sent = True
         reader = FrameReader(MAX_MESSAGE_BYTES)
         while True:
             sock.settimeout(_remaining(deadline))
             data = sock.recv(4096)
             if not data:
-                raise ControlError('closed', 'the display closed the connection')
+                raise ControlError('closed', 'the display closed the connection', sent=sent)
             lines = reader.feed(data)
             if lines:
                 return Response.from_dict(decode_message(lines[0]))
     except socket.timeout:
-        raise ControlError('timeout', 'no reply in time') from None
+        raise ControlError('timeout', 'no reply in time', sent=sent) from None
     except ProtocolError as e:
-        raise ControlError('bad_response', e.message) from None
-    except ControlError:
+        raise ControlError('bad_response', e.message, sent=sent) from None
+    except ControlError as e:
+        e.sent = e.sent or sent
         raise
     except OSError as e:
-        raise ControlError('closed', str(e)) from None
+        raise ControlError('closed', str(e), sent=sent) from None
 
 
 # -- commands ---------------------------------------------------------------------------
@@ -227,6 +273,21 @@ def plugin_reload(plugin_id: str, *, timeout: Optional[float] = None,
                    else timeout, paths=paths)
 
 
+def errors_clear(request_id: str, cutoff: float, *,
+                 timeout: float = DEFAULT_TIMEOUT_SECONDS,
+                 paths: Optional[Sequence[str]] = None) -> Dict[str, Any]:
+    """Have the display forget the plugin errors recorded at or before
+    ``cutoff`` (epoch seconds) and publish its error snapshot again.
+
+    Returns :class:`~src.ipc.contract.ErrorsClearResult` once it is done.
+    Raises :class:`ControlError`: ``unknown_command`` from a display older
+    than the command, which still reads the ``plugin_error_clear_request``
+    mailbox.
+    """
+    return request(Command.ERRORS_CLEAR, {'cutoff': cutoff}, request_id=request_id,
+                   timeout=timeout, paths=paths)
+
+
 def ping(*, timeout: float = DEFAULT_TIMEOUT_SECONDS,
          paths: Optional[Sequence[str]] = None) -> Dict[str, Any]:
     return request(Command.PING, {}, timeout=timeout, paths=paths)

src/ipc/contract.py

@@ -149,12 +149,13 @@ class Command:
     PLUGIN_RELOAD = 'plugin.reload'
     STATE_GET = 'state.get'
     STATE_SUBSCRIBE = 'state.subscribe'
+    ERRORS_CLEAR = 'errors.clear'
 
 
 #: Every command version 1 defines, in the order ``hello`` reports them.
-#: ``brightness.set`` and ``plugin.reload`` came in stage 2, and ``state.get``
-#: and ``state.subscribe`` in stage 3, all within version 1 (see the module
-#: docstring on adding commands).
+#: ``brightness.set`` and ``plugin.reload`` came in stage 2, ``state.get``
+#: and ``state.subscribe`` in stage 3, and ``errors.clear`` in stage 4, all
+#: within version 1 (see the module docstring on adding commands).
 COMMANDS: Tuple[str, ...] = (
     Command.HELLO,
     Command.PING,
@@ -165,8 +166,15 @@ COMMANDS: Tuple[str, ...] = (
     Command.PLUGIN_RELOAD,
     Command.STATE_GET,
     Command.STATE_SUBSCRIBE,
+    Command.ERRORS_CLEAR,
 )
 
+#: Commands the connection thread answers itself, through a handler the
+#: display registers (``ControlServer(handlers=...)``), because they touch
+#: nothing the render thread owns. A display that registered none answers
+#: ``unknown_command``, and the client falls back as from an older display.
+DIRECT_COMMANDS = frozenset({Command.ERRORS_CLEAR})
+
 #: Commands that are queued for the render thread.
 QUEUED_COMMANDS = frozenset({Command.ON_DEMAND_START, Command.ON_DEMAND_STOP,
                              Command.BRIGHTNESS_SET, Command.PLUGIN_RELOAD})
@@ -565,8 +573,32 @@ class StateSubscribeArgs:
         return cls()
 
 
+@dataclass(frozen=True)
+class ErrorsClearArgs:
+    """``errors.clear``: forget the plugin errors recorded at or before
+    ``cutoff`` (seconds since the epoch), as ``POST /api/v3/errors/clear``
+    asks. The request id is the clear's id, which the display's error
+    snapshot then reports as ``applied_clear_id``.
+    """
+    cutoff: float
+
+    def to_dict(self) -> Dict[str, Any]:
+        return {'cutoff': self.cutoff}
+
+    @classmethod
+    def from_dict(cls, args: Mapping[str, Any]) -> 'ErrorsClearArgs':
+        value = args.get('cutoff')
+        if isinstance(value, bool) or not isinstance(value, (int, float)):
+            raise ProtocolError(ErrorCode.INVALID_ARGS, 'cutoff must be a number of seconds')
+        if not math.isfinite(value) or value < 0:
+            raise ProtocolError(ErrorCode.INVALID_ARGS,
+                                'cutoff must be a finite, non-negative number of seconds')
+        return cls(cutoff=float(value))
+
+
 CommandArgs = Union[HelloArgs, OnDemandStartArgs, OnDemandStopArgs, NoArgs,
-                    BrightnessSetArgs, PluginReloadArgs, StateGetArgs, StateSubscribeArgs]
+                    BrightnessSetArgs, PluginReloadArgs, StateGetArgs, StateSubscribeArgs,
+                    ErrorsClearArgs]
 
 #: The arguments of a command that goes on the render thread's queue.
 QueuedArgs = Union[OnDemandStartArgs, OnDemandStopArgs, BrightnessSetArgs, PluginReloadArgs]
@@ -581,6 +613,7 @@ _ARG_TYPES: Dict[str, Any] = {
     Command.PLUGIN_RELOAD: PluginReloadArgs,
     Command.STATE_GET: StateGetArgs,
     Command.STATE_SUBSCRIBE: StateSubscribeArgs,
+    Command.ERRORS_CLEAR: ErrorsClearArgs,
 }
 
 
@@ -653,6 +686,13 @@ class PluginReloadResult(TypedDict):
     modes: List[str]
 
 
+class ErrorsClearResult(TypedDict):
+    """``errors.clear``, once applied and the error snapshot republished."""
+    request_id: str
+    cutoff: float
+    cleared: int
+
+
 class LoopState(TypedDict):
     """``loop``: is the render loop still going round?
 

src/ipc/server.py

@@ -6,7 +6,9 @@ rendering: a command that changes the panel is validated, put on a bounded
 queue and acknowledged, and the render thread drains that queue at the point
 where it reads the file mailbox (``DisplayController._poll_on_demand_requests``),
 handing each command to the same code. Queries (``on_demand.status``) are
-answered from a snapshot callable the display provides.
+answered from a snapshot callable the display provides, and the few commands
+that touch nothing the render thread owns (``errors.clear``) by a handler the
+display registers, on the connection thread.
 
 The queue also wakes the render thread: :meth:`ControlServer.wait_for_command`
 is what it waits on in place of a sleep, so a command lands within a frame on
@@ -62,6 +64,7 @@ from src.ipc.contract import (
     AWAITED_COMMANDS,
     COMMANDS,
     DEFAULT_SOCKET_DIR,
+    DIRECT_COMMANDS,
     DEFAULT_SOCKET_PATH,
     MAX_MESSAGE_BYTES,
     MAX_SUBSCRIBERS,
@@ -107,7 +110,8 @@ MAX_CLIENTS = 8
 
 #: Commands waiting for the render thread. It drains them at least every
 #: 0.25 s, so a full queue means the render thread is stuck, and the client
-#: is told ``busy`` (and falls back to the mailbox) instead of piling up work.
+#: is told ``busy`` instead of piling up work. The mailbox would not be read
+#: either, so the web interface reports the failure rather than fall back.
 QUEUE_SIZE = 16
 
 #: Timeout for one recv()/send() on a connection.
@@ -552,6 +556,12 @@ def server_socket_path(environ: Optional[Mapping[str, str]] = None) -> Optional[
 
 StatusProvider = Callable[[], Dict[str, Any]]
 
+#: A handler for one of DIRECT_COMMANDS, ``(request_id, args) -> result``. It
+#: runs on the connection thread, so it must not touch what the render thread
+#: owns. It may raise ProtocolError to answer with that error's code; any
+#: other exception is answered ``internal``.
+DirectHandler = Callable[[str, Any], Mapping[str, Any]]
+
 
 class ControlServer:
     """Serves the control socket on background threads.
@@ -569,9 +579,12 @@ class ControlServer:
                  await_seconds: Optional[Mapping[str, float]] = None,
                  state_hub: Optional[StateHub] = None,
                  max_subscribers: int = MAX_SUBSCRIBERS,
-                 keepalive: float = SUBSCRIBE_KEEPALIVE_SECONDS):
+                 keepalive: float = SUBSCRIBE_KEEPALIVE_SECONDS,
+                 handlers: Optional[Mapping[str, DirectHandler]] = None):
         self.path = path
         self.state_hub = state_hub
+        self._handlers: Dict[str, DirectHandler] = {
+            cmd: fn for cmd, fn in (handlers or {}).items() if cmd in DIRECT_COMMANDS}
         self._subscriber_slots = threading.BoundedSemaphore(max_subscribers)
         self._keepalive = keepalive
         self._await_seconds: Dict[str, float] = dict(AWAIT_SECONDS)
@@ -1028,6 +1041,9 @@ class ControlServer:
                 snap = hub.snapshot()
             return Response.success(request.id, fit_snapshot(snap), v=request.v)
 
+        if request.cmd in DIRECT_COMMANDS:
+            return self._direct(request, args)
+
         if request.cmd in QUEUED_COMMANDS and isinstance(args, (
                 OnDemandStartArgs, OnDemandStopArgs, BrightnessSetArgs, PluginReloadArgs)):
             awaited = request.cmd in AWAITED_COMMANDS
@@ -1055,6 +1071,25 @@ class ControlServer:
         return Response.failure(request.id, ErrorCode.INTERNAL,
                                 f'{request.cmd} is not implemented', v=request.v)
 
+    def _direct(self, request: Request, args: Any) -> Response:
+        """A command the display answers on this thread (DIRECT_COMMANDS)."""
+        handler = self._handlers.get(request.cmd)
+        if handler is None:
+            # Answered as an older display would, so the client falls back.
+            return Response.failure(request.id, ErrorCode.UNKNOWN_COMMAND,
+                                    f'{request.cmd} is not served by this display',
+                                    v=request.v)
+        try:
+            result = handler(request.id, args)
+        except ProtocolError as e:
+            return Response.failure(request.id, e.code, e.message, v=request.v)
+        except Exception:  # pylint: disable=broad-except
+            logger.exception("Control socket: %s %s failed", request.cmd, request.id)
+            return Response.failure(request.id, ErrorCode.INTERNAL,
+                                    'the display failed to apply it', v=request.v)
+        logger.info("Control socket applied %s %s", request.cmd, request.id)
+        return Response.success(request.id, dict(result), v=request.v)
+
     def _await_outcome(self, request: Request, outcome: CommandOutcome) -> Response:
         """Answer an awaited command once the render thread has applied it.
 
@@ -1079,7 +1114,9 @@ class ControlServer:
 def start_control_server(status_provider: Optional[StatusProvider] = None,
                          cache_dir: Optional[str] = None,
                          environ: Optional[Mapping[str, str]] = None,
-                         state_hub: Optional[StateHub] = None) -> Optional[ControlServer]:
+                         state_hub: Optional[StateHub] = None,
+                         handlers: Optional[Mapping[str, DirectHandler]] = None,
+                         ) -> Optional[ControlServer]:
     """Start the display's control socket, or return None when it can't run.
 
     None covers Windows, ``LEDMATRIX_CONTROL_SOCKET=off`` and any failure to
@@ -1091,7 +1128,7 @@ def start_control_server(status_provider: Optional[StatusProvider] = None,
         logger.debug("Control socket disabled or unsupported here; using the file mailbox only")
         return None
     server = ControlServer(path, status_provider, resolve_socket_group(cache_dir),
-                           state_hub=state_hub)
+                           state_hub=state_hub, handlers=handlers)
     return server if server.start() else None