Addresses PR #407 review findings:
- display_controller: only clear force_change / record health success
when display() actually ran this frame, not when the frame was skipped
because the plugin's lock was busy (a skip must preserve a pending
mode-switch force_clear).
- plugin_manager: replace bool() coercion of synchronous_updates with
explicit isinstance validation of plugin_system/synchronous_updates,
failing safe to synchronous mode (with a logged reason) on malformed
config instead of silently defaulting to async.
- plugin_manager: _update_worker_loop now acquires the plugin lock before
looking up its instance and re-checks under the lock, so an unloaded
plugin's lifecycle state is never resurrected to ENABLED.
- plugin_manager + display_controller: move lock ownership (and, for
updates, RUNNING/pending lifecycle bookkeeping) into the actual update()/
display() call itself rather than the timeout-wrapped caller, so the
lock stays held for the real operation's duration even after
PluginExecutor's own join(timeout) elapses and a lingering daemon thread
keeps running in the background.
- DisplayController.cleanup() now stops the update worker before tearing
down display/cache resources; stop_update_worker() logs when the join
times out instead of failing silently.
- test_async_plugin_updates: rewrite test_unloaded_while_queued_is_harmless
to exercise the public unload_plugin() lifecycle (via a deterministic
blocker) instead of deleting pm.plugins directly, and add a regression
test proving the plugin lock stays held through PluginExecutor's own
timeout while the real update() call is still running.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
ChuckBuilds