feat(plugins): request_on_demand() / end_on_demand() -- plugins ask for the screen in-process

[ChuckBuilds]

Summary

Stage 5 of the control socket (docs/IPC_CONTROL_SOCKET.md) needs an in-process way for plugins to ask for the screen before the display_on_demand_request mailbox can go. Four plugins write that mailbox today (birdnet-go, mqtt-notifications, on-air, pomodoro-timer), and since #765 the display reads it only once a second while the socket is up.

API

self.request_on_demand(mode=None, duration=None, pinned=False) -> Optional[str]
self.end_on_demand() -> Optional[str]

• BasePlugin -> PluginManager.request_on_demand(plugin_id, ...) / end_on_demand(plugin_id) -> DisplayController.submit_plugin_on_demand(request), wired with PluginManager.set_on_demand_handler() before any plugin loads.
• Safe from any thread (an MQTT callback, a timer thread). submit_plugin_on_demand only queues: an in-memory deque behind a lock, at most 32, and a full queue is refused. It wakes the render thread through the socket's flag (ControlServer.wake(), new). The render thread applies the queue in _drain_control_commands, after socket commands, through the same _handle_on_demand_request. So a request lands within a frame, and on the next pending-changes pass (about 0.25 s) without a socket.
• The request is mailbox-shaped with source: 'plugin'. It never reads or deletes the mailbox file.
• A plugin's stop ends only a session that plugin owns. A mailbox stop still ends any session.
• Both methods return the request id. They return None when there is no display in the process (the web interface, check_plugin.py), when the queue is full, or when the manager answered something other than a string (a test's MagicMock). None is the plugin's cue to write the mailbox, which the display still reads.
• Bad arguments raise ValueError: a non-string mode, or a non-numeric or bool duration. A non-positive or non-finite duration means no limit, as on the mailbox path.

Older cores: docs/PLUGIN_API_REFERENCE.md ("On-demand display") documents the hasattr feature-detection pattern with the mailbox fallback, and says to keep ledmatrix_min_version unchanged.

Docs: PLUGIN_API_REFERENCE.md (BasePlugin and Plugin Manager sections), IPC_CONTROL_SOCKET.md (mailbox table, new "Plugins in the display process" section, stage 5), CHANGELOG under Unreleased.

Tests

• test/test_plugin_on_demand_api.py, 36 tests, covering:
  • wiring;
  • the start and stop paths through a real PluginManager;
  • ordering, no mailbox reads, and the mailbox still working alongside;
  • skipping both floors, waking a real ControlServer's wait, and the no-socket wait;
  • 8 threads x 50 requests drained while they run;
  • a full queue;
  • stop ownership;
  • the no-display, old-manager, raising-handler and MagicMock cases;
  • the documented hasattr pattern;
  • argument validation.
• Mutation check: 14 of 14 hand mutations killed. They covered the source check for from_mailbox, the stop ownership guard, wake(), the pending check, the drain call, the no-socket wait, the wiring, the queue limit, FIFO order, the accepted-to-None mapping, duration normalisation, the stop's plugin_id, pinned pass-through, and ControlServer.wake itself.
• mypy: the ratchet (scripts/check_types.py) shows only the src/common/fetch_service.py unused-ignore that origin/main shows on this machine. The touched non-ratchet modules have 253 errors, the same count as main.
• Full suite on Windows, branch vs an origin/main worktree: see the comment below.

The plugins PR follows once this merges.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Plugins can request and end on-demand display sessions directly. Requests are processed by the render thread, and each plugin can end only its own session.
  • Existing mailbox-based requests remain available for older cores and as a fallback when in-process requests are unavailable.
• Documentation
  • Added guidance on plugin request methods, request limits and results, session ownership, and fallback behavior.

[coderabbitai]

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

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 47 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: d0fa238e-207f-4ebe-84e3-6e9f4505cca7

📥 Commits

Reviewing files that changed from the base of the PR and between e018478 and 5f80b93.

📒 Files selected for processing (1)

• src/display_controller.py

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: bbfb26d4-7201-4ad8-8488-e62b52604750

📥 Commits

Reviewing files that changed from the base of the PR and between 5a7893b and e018478.

📒 Files selected for processing (8)

• CHANGELOG.md
• docs/IPC_CONTROL_SOCKET.md
• docs/PLUGIN_API_REFERENCE.md
• src/display_controller.py
• src/ipc/server.py
• src/plugin_system/base_plugin.py
• src/plugin_system/plugin_manager.py
• test/test_plugin_on_demand_api.py

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

Plugins can submit on-demand display start and stop requests through BasePlugin. PluginManager validates and routes requests to a bounded DisplayController queue, which processes them on the render thread. The documentation and tests describe request ownership, queue behavior, and mailbox fallback.

Changes

Plugin on-demand requests

Layer / File(s)	Summary
Plugin request API and submission src/plugin_system/base_plugin.py, src/plugin_system/plugin_manager.py, docs/PLUGIN_API_REFERENCE.md, CHANGELOG.md, test/test_plugin_on_demand_api.py	BasePlugin delegates start and stop requests to PluginManager. The manager validates arguments, creates request IDs, and submits requests to its configured handler. Documentation and tests cover return values, validation, and fallback behavior.
Render-thread queue and request handling src/display_controller.py, src/ipc/server.py, docs/IPC_CONTROL_SOCKET.md, test/test_plugin_on_demand_api.py	DisplayController queues plugin requests and processes them on the render thread. It wakes the control server when available and limits plugin stops to the requesting plugin’s session. Tests and documentation cover queue processing, mailbox compatibility, and request ordering.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant BasePlugin
  participant PluginManager
  participant DisplayController
  participant ControlServer
  participant RenderThread
  BasePlugin->>PluginManager: Submit start or stop request
  PluginManager->>DisplayController: Submit request with plugin source
  DisplayController->>ControlServer: Wake render loop when available
  RenderThread->>DisplayController: Drain queued plugin requests
  DisplayController->>RenderThread: Apply requests through on-demand handler

Loading

Merge Risk: ⚪ Minimal · up to e0184

Plugin requests without a control socket may wait until the next pending-changes pass, as documented. No issue identified here prevents merging after normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to e0184

The change reuses existing display controls and narrows plugin-initiated stops. However, sustained plugin requests can monopolize request processing and delay user stop or recovery commands: limiting queue size does not limit processing time.

Retained concerns

• Medium · reliability · inferred: The new plugin drain handles a live queue until it becomes empty, without a snapshot, request budget, or time budget. A continuously replenishing plugin producer can therefore keep the render thread inside this drain and delay subsequent user stop commands, mailbox processing, and deferred recovery work. The 32-entry occupancy cap does not bound work per pass; the previous plugin mailbox path processed one value per polling opportunity.

Security review details

Security Blast Radius

• inferred — The directly demonstrated exposure is shared screen ownership and control responsiveness within one DisplayController, affecting every plugin using that controller. Additional tenant, host, or synchronized-follower exposure was not established by the inspected request path.

Security Findings and Attack Paths

• inferred — A plugin producer sustaining accepted requests can keep the live drain occupied and delay later control commands. The API explicitly supports event callbacks, but no changed external-input consumer was demonstrated; remote exploitability depends on plugin behavior and input controls not established here.

Trust Boundaries and Controls

• observed — Submission does not mutate screen state on the calling thread. Queue occupancy is capped, requests are copied, and normal plugin stops are checked against the active resolved plugin owner. Existing socket and mailbox stops retain their broader authority.
• observed — The API documents a mode belonging to the calling plugin, but runtime resolution accepts any registered mode and assigns its resolved owner to the session. A cross-plugin mode request can therefore start a session that the original requester cannot end through the new ownership guard. Mode resolution predates this PR; this does not demonstrate newly gained cross-plugin authority.

Resilience and Maintainability Implications

• observed — The concurrency tests check delivery order for finite producers, and failure tests check continuation after an exception. They do not establish bounded drain latency under continuous replenishment or atomic recovery after partially applied production transitions.

Hardening Proposals

• proposed — Bound plugin work per render pass using a detached batch or explicit processing budget, preserving wake signaling for remaining work and opportunities to service user stop and recovery commands.
• proposed — Validate that plugin-originated modes belong to the requesting plugin so start ownership and subsequent release agree. Treat this as API contract enforcement, not a sandbox or authentication guarantee.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: plugins can request and end on-demand display sessions in-process. It is specific and related to the changeset.
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.

Full details: Docstring Coverage

Explanation

Docstring coverage is 35.82% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 67 functions across 5 files. (3 skipped: 3 unsupported.)

✨ 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 25 complexity

Metric	Results
Complexity	25

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.}

[ChuckBuilds]

Full suite on Windows (Python 3.14, Pillow 12.3), this branch at 5f80b93 against an origin/main clone at 5a7893b:

• Branch: 62 failed, 6 errors, 8818 passed.
• Main: 61 failed, 6 errors, 8782 passed.
• The FAILED/ERROR id sets are identical apart from test_sync_manager.py::TestRealSocketHandshake::test_leader_and_follower_negotiate_over_real_sockets. That is a real-socket timing test, and it fails on the main clone too when run alone. Nothing is only on main.
• The first run of the branch caught a real problem: managers that tests and golden traces stand in have no set_on_demand_handler. 5f80b93 wires the handler with getattr.

🤖 Generated with Claude Code