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

[ChuckBuilds]

Stage 4 of the web → display control socket (docs/IPC_CONTROL_SOCKET.md): the socket is the primary channel for every web command, and the file mailboxes are written only as the fallback when the socket cannot carry a request.

What still went through a mailbox

Path	Before	Now
On-demand start/stop (web)	socket first, mailbox on any failure, including a display that had the request (busy, reply timeout, invalid_args)	mailbox only when the display never had it (no socket, refused/timed-out connect, turned away at the door) or is too old to know the command (unknown_command, unsupported_version); otherwise 503 (400 for invalid_args) and no copy
Error clear (web, POST /api/v3/errors/clear)	always the plugin_error_clear_request mailbox, applied on the display's next 5 s tick	new errors.clear socket command, applied and republished before the answer (applied: true, the display's own cleared_count); mailbox only on the same fallback rule
On-demand mailbox poll (display)	read every 0.25 s from the render thread, a full open + parse; socket commands also re-read and deleted it	a stat() once a second while the socket is up (0.25 s without), read only when the file changed; socket commands never touch it
Error-clear mailbox poll (display)	read every 5 s	a stat() every 5 s, read only when the file changed

Brightness (brightness.set) and plugin reload (plugin.reload) never had a mailbox; config saves (schedule, dim schedule, plugin settings) stay on config.json + its watcher, the setting itself rather than a message (as decided in stage 2). Plugin health/metrics resets write the published record and are not mailboxes; left as they are.

How

• src/ipc/client.py: ControlError.sent (the whole request was written to a connected display; a door refusal with no request id leaves it False) and should_fall_back(error), the one rule every route uses. New errors_clear().
• src/ipc/contract.py / server.py: errors.clear {cutoff} and DIRECT_COMMANDS, answered on the connection thread by a handler the display registers (ControlServer(handlers=...)). No handler → unknown_command, so the client falls back as from an older display.
• src/cache_manager.py: CacheManager.file_signature(key) → (inode, mtime_ns, size) or None, and MailboxWatch, which says whether a mailbox changed since the last look. Every DiskCache write renames a new file into place, so a new write always has a new inode.
• src/display_controller.py: MAILBOX_POLL_INTERVAL_WITH_SOCKET = 1.0; socket commands (source: 'socket') skip the mailbox consume; a processed duplicate is consumed instead of re-read for an hour; a mailbox request while the socket is up is logged once per writer (the four plugins that write it directly: birdnet-go, mqtt-notifications, on-air, pomodoro-timer).
• src/error_aggregator.py: ErrorSnapshotPublisher.clear_now(), apply_error_clear() (the socket handler); request_error_clear(..., send=); the snapshot carries applied_clear_cutoff so an older mailbox request is not shown as pending after a wider socket clear.

Backward compatibility (one release)

The display still reads both mailboxes and still writes display_current_state, display_on_demand_state and plugin_runtime_snapshot. Upgrade windows:

• new web, old display: on-demand works over the socket as before; errors.clear gets unknown_command and falls back to the mailbox, which the old display applies.
• new display, old web (or a plugin writing the mailbox): the mailbox is still read, within a second.

CHANGELOG has the entry under Unreleased. No new src/ module (so no plugins-repo guard-table entry is needed).

Tests

• New test/test_ipc_stage4.py: sent/should_fall_back matrix through _exchange (platform-independent fake socket), errors.clear on the server, mailbox cadence with and without a socket, stat-gating, socket commands leaving the mailbox alone, the duplicate case, the deprecation log, file_signature/MailboxWatch on a real cache, and real-socket round trips (Linux).
• test_api_v3_on_demand_socket.py: socket up; socket down (sent=False reasons → mailbox); the display had it (busy/internal/timeout/closed/bad_response → 503, invalid_args → 400, no mailbox); upgrade (unknown_command/unsupported_version → mailbox); stop with stop_service; a real full queue → 503 (Linux).
• test_error_snapshot_cross_process.py: errors.clear over the socket (applied before the answer, no mailbox file), socket down → mailbox, older display → mailbox, display failed → 503, an older mailbox request not shown pending, publisher reads the mailbox only on change.
• Mutation check: 18 mutations of the new rules (fallback rule, sent bookkeeping, poll interval, stat-gating, consume rules, deprecation log, applied cutoff, socket result handling, handler-less answer, route branches), all killed.
• Full suite on Windows vs an origin/main baseline worktree: identical FAILED/ERROR id lists (76 each, the known Windows set); 8630 passed vs 8555 on the baseline. After merging main (refactor(display): run loop stage 3 -- ScreenRunner, PREEMPTED, OnDemand/Live/Rotation Sources #762 run loop stage 3) the IPC/on-demand/display-controller/run-loop subsets pass (887 passed). mypy: no new errors in the touched modules; the ratchet result is the same as main's.

Hardware (ledpi)

Deployed this branch (597653e, merged with main 26cae3e) on ledpi (Pi 4), exercised through the web API on localhost:5000, then restored ledpi to origin/main 26cae3e and restarted both services. config.json was identical to its pre-test backup afterwards.

Upgrade window: new web, old display (web restarted on the branch, display still on main 294bd52):

• POST /errors/clear → transport: mailbox (web log: Control socket did not take errors.clear (unknown_command ...)); the old display applied it from the mailbox within 1 s (Cleared 0 plugin error record(s) as requested).
• On-demand start/stop → transport: socket; current-status showed clock-simple, source: socket.

Both on the branch:

• /health: display_loop running (source: socket); current-status source: socket.
• On-demand start → transport: socket, on screen after 0.29 s, no mailbox file written; stop → socket, status idle.
• Schedule change (POST /config/schedule, end_time changed) → 200, display's config watcher reloaded within 2 s; restored → 200.
• Config reload / brightness (POST /config/main) → brightness_transport: socket; display logged Brightness set to 70% over the control socket, then back to 80.
• POST /errors/clear → transport: socket, applied: true, message Cleared all errors; /errors/summary clear_pending: false.
• Plugin-style mailbox write (CacheManager().set('display_on_demand_request', ...)) while the socket is up → active after 0.52 s, logged once: ... came through the file mailbox although the control socket is up. The mailbox is deprecated ...; the mailbox file was consumed.

Display stopped:

• /health → degraded, display_loop: stopped (source service); current-status → source: cache, all null.
• On-demand start with start_service: false → 400, mailbox withdrawn (no file left).
• On-demand stop → transport: mailbox, socket_error: no_socket.
• /errors/clear → transport: mailbox, applied: false, request file written.
• On-demand start with start_service: true → transport: mailbox, service started; the new display took the request from the mailbox on its first poll (on screen ~12 s after the start, i.e. as soon as the display was up), then served the socket.

🤖 Generated with Claude Code

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: a5496820-0a2c-4022-abec-25ad71f82fc1

📥 Commits

Reviewing files that changed from the base of the PR and between 97185ba and 597653e.

📒 Files selected for processing (11)

• CHANGELOG.md
• docs/IPC_CONTROL_SOCKET.md
• src/cache_manager.py
• src/display_controller.py
• src/error_aggregator.py
• src/ipc/client.py
• src/ipc/contract.py
• src/ipc/server.py
• test/test_api_v3_on_demand_socket.py
• test/test_error_snapshot_cross_process.py
• test/test_ipc_contract.py

🚧 Files skipped from review as they are similar to previous changes (1)

• CHANGELOG.md

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The display control socket is the primary transport for on-demand requests and error clearing. Mailboxes remain fallbacks when the socket cannot carry a request. The display polls mailboxes less often while the socket is active and reads them only when their files change.

Changes

Control-socket requests

Layer / File(s)	Summary
Socket command contract and dispatch src/ipc/contract.py, src/ipc/client.py, src/ipc/server.py, test/test_ipc_stage4.py, test/test_ipc_contract.py, docs/IPC_CONTROL_SOCKET.md	The IPC contract adds errors.clear as a direct command. The client tracks whether a request was sent to determine fallback eligibility. The server dispatches registered direct handlers and maps handler errors to protocol responses.
On-demand delivery and mailbox polling src/cache_manager.py, src/display_controller.py, web_interface/blueprints/api_v3/display.py, test/_run_loop_harness.py, test/test_api_v3_on_demand_socket.py, test/test_ipc_stage4.py, docs/ADVANCED_FEATURES.md, docs/ARCHITECTURE.md, docs/IPC_CONTROL_SOCKET.md, docs/REST_API_REFERENCE.md	On-demand requests fall back to the mailbox only when the socket cannot carry them. The display uses file signatures to avoid unchanged mailbox reads and polls at different intervals depending on socket availability. Requests received but rejected by the display return socket errors without mailbox writes.
Error-clear application and fallback src/error_aggregator.py, web_interface/blueprints/api_v3/misc.py, test/test_error_snapshot_cross_process.py, test/test_ipc_stage4.py, docs/ARCHITECTURE.md, docs/IPC_CONTROL_SOCKET.md, docs/REST_API_REFERENCE.md, CHANGELOG.md	Error clears use the direct socket handler when available. The display applies the clear and republishes the snapshot before responding. When the socket cannot carry the request, the web interface writes a mailbox request for later processing.

Priority: ⬇️ Low

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant WebAPI
  participant IPCClient
  participant ControlServer
  participant DisplayController
  participant RequestMailbox
  WebAPI->>IPCClient: Send on-demand request
  IPCClient->>ControlServer: Send socket command
  ControlServer->>DisplayController: Queue socket request
  alt Socket cannot carry request
    WebAPI->>RequestMailbox: Write fallback request
    DisplayController->>RequestMailbox: Read changed mailbox
  else Display received but rejected request
    ControlServer-->>WebAPI: Return socket error without mailbox write
  end

Loading

sequenceDiagram
  participant WebAPI
  participant IPCClient
  participant ControlServer
  participant ErrorSnapshotPublisher
  participant ClearMailbox
  WebAPI->>IPCClient: Send error-clear request
  IPCClient->>ControlServer: Send errors.clear
  ControlServer->>ErrorSnapshotPublisher: Apply clear and publish snapshot
  ErrorSnapshotPublisher-->>ControlServer: Return cleared count
  ControlServer-->>WebAPI: Return applied response
  alt Socket cannot carry request
    WebAPI->>ClearMailbox: Write clear request
    ErrorSnapshotPublisher->>ClearMailbox: Read changed mailbox
    ErrorSnapshotPublisher->>ErrorSnapshotPublisher: Apply clear and publish snapshot
  end

Loading

Merge Risk: ⚪ Minimal · up to 59765

No actionable issue remains from these findings. The change is mergeable after normal checks; hardware validation remains pending.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 59765

The new delivery rules reduce duplicate commands, and access checks remain in place. However, storage failures can produce a successful error-clear response without updating the shared report, or prevent a fallback command from being retried after storage recovers. These risks are limited to the affected display and its diagnostic state.

Retained concerns

• Medium · reliability · inferred: The new synchronous clear can report applied:true without publishing the cleared snapshot to the shared location. DiskCache.set can silently return after failed writes or write only to a fallback location; the publisher then marks the version published, so an unchanged aggregator does not trigger another publication attempt. Unlike the base mailbox path, socket success leaves no new pending clear request to mask the old report. A storage failure can therefore leave readers showing pre-clear diagnostics after successful acknowledgement, until another state change or recovery action causes publication.
• Medium · reliability · inferred: Signature-gated polling can abandon an unchanged pending command after a transient read failure. MailboxWatch records the signature before reading; DiskCache.get converts permission and I/O failures into None, so neither poller's exception-based forget path runs. Later polls skip the same file even after read access recovers. Compared with the base's repeated reads, this can strand a fallback on-demand stop or error-clear request until the file changes or the watcher is recreated, weakening control-state recovery.

Security review details

Security Blast Radius

• inferred — The inspected authority and outcomes concern one display process: changing its on-demand session and clearing its shared plugin diagnostics. Socket access is intended for root, the display's user, or the shared group. The evidence does not establish separate tenant-level authorization within that boundary.

Trust Boundaries and Controls

• observed — Direct dispatch follows command argument parsing and the connection's peer-check path. Peer rejection occurs before request processing when credential checks apply. The error-clear route is absent from the HTTP exemption lists; when web login is enabled, the application gate retains its session, token, and localhost access rules. Clearing does not introduce a separate unauthenticated HTTP endpoint.

Resilience and Maintainability Implications

• observed — Error clearing runs on a connection thread without touching render-owned state. Publisher and aggregator locks serialize the relevant mutations. Normal shared writes use temporary-file replacement, but fallback writes may be non-atomic and write failures may be suppressed; those inherited cache semantics limit the new acknowledgement guarantee.

Hardening Proposals

• proposed — Use explicit shared-storage outcomes for control records: acknowledge applied only after publication to the reader-visible location, retain publication retry state on failure, and distinguish mailbox read failures from absence before committing a seen signature.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 27.81% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 151 functions across 13 files. (2 skipped…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: the control socket is the primary transport for web commands, with mailboxes as fallback.

Full details: Docstring Coverage

Explanation

Docstring coverage is 27.81% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 151 functions across 13 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 65 complexity

Metric	Results
Complexity	65

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}