Compare commits

...
4 Commits
Author SHA1 Message Date
ChuckandClaude Opus 5 2af41c561b fix(plugins): make update scheduling atomic so update() cannot run twice at once (#437)
* fix(plugins): make update scheduling atomic so update() cannot run twice at once

Closes #401.

`run_scheduled_updates()` decided whether to update a plugin with a
check-then-act sequence: `can_execute()` and `set_state(RUNNING)` were
separate calls with nothing between them, so two scheduler threads could
both observe ENABLED and both go on to call the same plugin's `update()`.
`update_all_plugins()` had the identical pattern.

Two schedulers really do run at once. The render loop calls
`_tick_plugin_updates()`, and Vegas mode fires its own `vegas-plugin-tick`
daemon thread that is never joined when `VegasModeCoordinator.play()`
returns — a slow `update()` still in flight overlaps the next tick from
the main loop. A plugin running `update()` twice concurrently is unsafe
unless it happens to be reentrant; shared mutable state, a non-thread-safe
HTTP session or cache all break.

The async path was already covered by the `_pending_lock` dedup in
`_enqueue_update`, so the live exposure was the synchronous kill-switch
path and `update_all_plugins()`. Both now claim the plugin through
`_reserve_for_update()`, which holds one lock across the eligibility
check, the due-time check and the RUNNING transition — and nothing more.
Holding it across `execute_update()` would serialize slow plugins behind
each other and reintroduce the render stall the async worker exists to
avoid.

The due-time check moved inside the lock deliberately. Left outside, a
thread that had already decided "due" could claim the plugin the instant
the winner finished, running `update()` twice within one interval.

Two supporting changes fall out of it:

- `_enqueue_update()` no longer sets RUNNING (the reservation did), and
  hands the reservation back if the pending-dedup ever fires. Otherwise a
  reserved-but-unqueued plugin would sit in RUNNING with nothing left to
  release it, and `can_execute()` would refuse it forever.
- `_finish()` now clears the pending entry *before* flipping the state
  back to ENABLED. The old order left a window where a scheduler saw
  ENABLED, reserved the plugin, then had its enqueue silently dropped by
  the dedup — harmless as a missed tick before, a stuck plugin once a
  reservation is involved.

Regression suite added and enrolled in CI, along with
test_async_plugin_updates.py which was not previously run there. The
overlap tests delay `can_execute()` to hold every thread inside the
check-then-act gap: the real window is a couple of bytecodes wide, so a
plain hammering test passes against the unfixed scheduler and proves
nothing. With that delay the suite reports `update() ran 8x concurrently`
on both affected paths before the fix, and passes after.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5

* test: fix a race in the reservation suite's own wait loops

`test_async_path_never_overlaps` failed in CI with "update() ran 0x
concurrently" — the test's bug, not the scheduler's. It polled
`plugin._active` to wait for the update to finish, but before the worker
picks the item up nothing is active yet, so the loop fell straight
through and asserted on a plugin that had never run.

Both async waits now key on `update_calls >= 1` as well, so they wait for
an update to have started *and* finished. The stranded-state test gets
the same guard for a second reason: ENABLED is also the starting state,
so without it that assertion passes vacuously on a plugin that was never
scheduled.

Verified over 12 consecutive local runs, 12 passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5

* fix(plugins): roll back the claim when dispatch fails

_enqueue_update() reserved the plugin and added it to the pending set,
then started the worker and queued the item. Thread.start() raises
RuntimeError when the OS refuses a new thread — not hypothetical on a Pi
under memory or thread pressure — and nothing is queued at that point to
release the plugin. It stayed RUNNING with a stale pending entry, so
can_execute() refused it for the rest of the process, and the exception
escaped run_scheduled_updates() and skipped every remaining plugin in
that tick.

That is the same stranded-RUNNING failure the reservation was introduced
to prevent, just reached through the dispatch rather than the dedup, so
it is handled the same way: discard the pending entry, hand the
reservation back, log the cause. Swallowed rather than raised so one
plugin failing to queue cannot abort the others' turn.

Both new tests fail against the un-rolled-back version — the second on
the escaping RuntimeError itself — and pass with it. 74 tests across the
reservation, async-update, plugin-system, health, Vegas-adapter and
controller-toggle suites still pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WexvwNDtWLVymGVqKD7BGk

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 19:38:11 -04:00
ChuckandClaude Opus 5 5b81cca684 fix(espn): send an identifying User-Agent so ESPN stops returning 403 (#436)
Around 11:00 EDT on 2026-08-04 ESPN's site.api began rejecting the agents
this repo sends. Every scoreboard that goes through the shared data
sources returned `403 Client Error: Forbidden` — standings, game
summaries and scoreboards alike. A device that had been running fine
logged 287 ESPN errors in a day.

The filter is not the familiar one. Probing site.api across agents and
libraries, using requests as the plugins do:

    bare 'LEDMatrix/1.0'                     403   (with or without Accept)
    browser string                           403   (no header rescues it)
    'LEDMatrix/1.0 (+https://github.com/...)' 200
    requests / urllib / curl defaults        200

So it rejects browser-style strings outright and bare custom tokens, and
accepts honest client tokens or an agent that identifies the client and
links to it. The instinct to "just send a browser User-Agent" is now
exactly backwards — that is the one thing guaranteed to stay blocked.

Both call sites here sent a bare token: `LEDMatrix/1.0` in the sports
data sources and `LEDMatrix-Common/1.0` in the API helper. The Accept
header the data sources already sent does not save it. Both now send an
agent carrying the project URL, which also gives ESPN someone to contact
rather than an anonymous token to rate-limit.

Verified on a live device: ESPN errors went from a steady stream to zero
across a restart, with live MLB games fetching again.


Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:55:45 -04:00
ChuckandClaude Opus 5 d305be6089 fix(plugins): core's own config keys no longer flag plugins as degraded (#434)
Found while sweeping devpi for issues. Nine of 27 installed plugins were
reported degraded in the web UI -- including baseball-scoreboard and
f1-scoreboard -- for using a documented core feature.

The core reads three tuning keys out of each plugin's own config block:
vegas_width_pct and vegas_overflow (vegas_mode/plugin_adapter.py) and
vegas_max_width_screens (base_plugin.py). No plugin declares them, and 37 of
the 42 published config schemas set "additionalProperties": false -- so schema
validation reported them as violations.

That is not just log noise. _validate_config_schema_soft sets `degraded` in
the health tracker, which the web UI surfaces, so a user who tuned a core
Vegas setting saw the plugin marked broken.

The keys are stripped before validation. Fixing it plugin-side would mean 42
schema edits and 42 version bumps -- 42 store updates for a contract the core
owns.

Listed explicitly rather than matched on a `vegas_` prefix: vegas_mode is the
opposite case, plugin-owned and declared in schemas, and a prefix rule would
silently stop validating it.

Verified on devpi: degraded went 9 of 27 -> 0 of 27, schema-mismatch warnings
9 -> 0, 22 plugins still load, no tracebacks. 800 core unit tests pass,
8 of them new -- including that a genuine violation is still reported, so the
check has not been turned into a no-op, and that the caller's live config dict
is never mutated.


Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-04 13:46:28 -04:00
ChuckandClaude Opus 5 53af53b4a1 feat(store): evaluate compatible_versions, not just the floor (#433)
* feat(store): evaluate compatible_versions, not just the floor

Closes the gap CodeRabbit surfaced on #427. `compatible_versions` is the
canonical compatibility contract -- schema/manifest_schema.json marks it
required, all 42 published manifests carry it -- and it is the only field that
can express an *upper* bound. `ledmatrix_min_version` is a floor and cannot
say "not compatible with 4.x".

The gate read only the floor, so a plugin declaring ["2.0.0 - 2.9.9"] would be
installed on 3.2.0 regardless of having said it stops at 2.x.

check() now evaluates both and the more restrictive wins. The array is a set of
alternatives (satisfying any one entry suffices), supporting every form the
schema permits: >=, <=, >, <, ~, ^, a bare exact version, and an inclusive
"A - B" range, with prerelease/build suffixes tolerated.

Refusal still requires evidence. Anything unparseable, absent, or below
TRUSTWORTHY_FLOOR resolves to compatible.

That last point needed a new strict parser. parse_semver is deliberately
lenient -- it strips non-digits and yields (0, 0, 0) for a string with no
numbers at all. Harmless for a floor (0.0.0 never blocks) but wrong for a
range, where the same leniency turned an unreadable spec into a *refusal*: a
manifest whose only entry was garbage got compared against 0.0.0 and refused.
Range specs are now shape-checked first, so garbage reads as "no evidence".
parse_semver itself is unchanged, since the loader depends on its behaviour.

Verified: 815 core unit tests pass, 18 of them new. Swept the real registry --
all 42 published manifests, at cores 1.0.0 / 2.0.0 / 3.1.0 / 3.2.0 / 4.0.0 --
and nothing is refused at any of them. The gate stays inert for shipped
plugins, which is the property that makes it safe to land ahead of B5.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5

* feat(store): protect the one population the sunset would break

The B6 sunset deletes each plugin's guarded-import fallback, so a plugin that
floors at 3.2.0 must never reach a core that lacks the 3.2.0 modules. The gate
could not stop that for the population most at risk.

A device installed from the v3.1.0 release reports __version__ = "1.0.0". The
gate treated anything below TRUSTWORTHY_FLOOR as "unknown, do not block" --
correct while every manifest floors at 2.0.0, because blocking would have
emptied the plugin store for those users. But after the sunset it hands them a
3.2.0-floored plugin with no fallback, which fails to load with one log line.
Nothing else in the system protects them: they cannot be told apart from a
genuine 1.0.0 install.

On an untrustworthy core the gate now refuses a floor ABOVE 2.0.0 and still
allows anything at or below it. A floor above the ecosystem baseline says the
plugin needs modules that arrived after 2.0.0, and a core reporting below that
-- whether it is the v3.1.0 release or something genuinely ancient -- will not
have them. Refusing leaves the user on the version they already run instead of
one that cannot load.

Measured against all 42 published manifests:

  today (every manifest floors at 2.0.0)
    core 1.0.0 / 2.0.0 / 3.1.0 / 3.2.0 / unparseable -> 0 of 42 refused
  after B6 (same manifests floored at 3.2.0)
    core 1.0.0 -> 38 refused, core 3.1.0 -> 38 refused, core 3.2.0 -> 0

So nobody loses the store today, and the sunset cannot reach a core that
cannot run it.

Two older tests asserted the previous "allow everything" behaviour; they now
express the new rule with a 2.0.0 floor, which is what their no-lockout intent
was actually about.

The 38-of-42 in that measurement surfaced a separate B6 trap, recorded here
because it will bite whoever raises the floors: four plugins (flights,
leaderboard, music, stocks) declare the floor as a TOP-LEVEL
`min_ledmatrix_version`, a third spelling, which declared_min_version checks
before the versions[] array. For those, editing versions[0] is a silent no-op
and the floor stays at 2.0.0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5

* fix(store): address review — malformed manifests, and suffixed versions

Both CodeRabbit findings on #433 verified against the code and fixed.

1. Malformed manifest sections raised instead of degrading. `requires` as a
   list hit AttributeError ('list' object has no attribute 'get') and
   `versions` as a mapping hit KeyError: 0. Both reproduced.

   This got worse with the sunset rule in the previous commit: that branch
   resolves the floor for *every* manifest on an untrustworthy core, where the
   old code returned early. One hand-edited or third-party file with the wrong
   shape would have taken down the whole install path rather than just itself.
   Container types are now validated and an unrecognised shape reads as "no
   declared floor".

2. Prerelease and build metadata leaked into the version numbers. The digit
   scrape parsed "3.2.0+build42" as (3, 2, 42) and "3.2.0-rc1" as (3, 2, 1) --
   a release candidate ranking above its own release. Both fed reject
   decisions, and the consequence was demonstrable: a plugin pinned to exactly
   "3.2.0" refused a core running 3.2.0+build42, which is that same version.

   The suggested remedy -- use the strict token parser -- would not have
   fixed it. _parse_strict validates the shape but delegates the numbers to
   parse_semver, so it returned the same (3, 2, 42). The bug is in the scrape,
   so suffixes are now dropped before it. Prereleases compare equal to their
   release rather than below it; full prerelease ordering is more than any
   caller needs and equal is far closer to right than what it did before.

parse_semver is shared with PluginLoader, so its suite was re-run: unchanged,
and it only ever gets more correct here.

Verified: 839 core unit tests pass, 21 of them new -- six malformed shapes,
five suffixed forms, and the two demonstrated regressions. The real-registry
sweep is unchanged at 0 of 42 refused across cores 1.0.0, 3.1.0, 3.2.0,
3.2.0+build42 and an unparseable string.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-04 13:46:05 -04:00
8 changed files with 968 additions and 61 deletions
+4 -1
View File
@@ -79,4 +79,7 @@ jobs:
test/test_sports_scroll.py \
test/test_version_consistency.py \
test/test_plugin_compatibility_gate.py \
test/test_install_preserves_existing.py
test/test_install_preserves_existing.py \
test/test_core_owned_config_keys.py \
test/test_async_plugin_updates.py \
test/test_plugin_update_reservation.py
+9 -2
View File
@@ -44,9 +44,16 @@ class DataSource(ABC):
"""Fetch standings for a sport/league."""
def get_headers(self) -> Dict[str, str]:
"""Get headers for API requests."""
"""Get headers for API requests.
The agent carries the project URL deliberately. Around 2026-08-04 ESPN
began returning 403 for bare custom tokens like 'LEDMatrix/1.0' — and
for browser-style strings — while accepting an agent that identifies
the client and links to it. An Accept header alone does not rescue the
bare form when the request goes out through requests.
"""
return {
'User-Agent': 'LEDMatrix/1.0',
'User-Agent': 'LEDMatrix/1.0 (+https://github.com/ChuckBuilds/LEDMatrix)',
'Accept': 'application/json'
}
+3 -1
View File
@@ -56,7 +56,9 @@ class APIHelper:
# Default headers
self.session.headers.update({
'User-Agent': 'LEDMatrix-Common/1.0',
# Identifies the client and links to it: ESPN began 403ing bare
# custom tokens (and browser strings) around 2026-08-04.
'User-Agent': 'LEDMatrix/1.0 (+https://github.com/ChuckBuilds/LEDMatrix)',
'Accept': 'application/json',
'Accept-Language': 'en-US,en;q=0.9',
'Accept-Encoding': 'gzip, deflate, br',
+173 -22
View File
@@ -27,6 +27,7 @@ fixes their version string. See `docs/SPORTS_UNIFICATION.md`, phase B4.
from __future__ import annotations
import re
from typing import Any, Dict, Optional, Tuple
# Below this, the core's self-reported version is not evidence of anything.
@@ -39,7 +40,20 @@ def parse_semver(value: Any) -> Optional[Tuple[int, int, int]]:
3-tuple, or ``None`` when unparseable. A leading ``v`` is tolerated."""
if not isinstance(value, str):
return None
parts = value.strip().lstrip('v').split('.')
text = value.strip().lstrip('v')
# Drop the prerelease/build suffix before scraping digits. Without this the
# scrape pulls them into the numbers: "3.2.0+build42" parsed as (3, 2, 42)
# and "3.2.0-rc1" as (3, 2, 1) -- a release candidate ranking *above* its
# own release, and a build of 3.2.0 failing an exact "3.2.0" match.
#
# Prereleases compare equal to their release here rather than below it.
# Full prerelease ordering is more than any caller needs, and equal is far
# closer to right than the old behaviour.
for sep in ('+', '-'):
head, found, _tail = text.partition(sep)
if found:
text = head
parts = text.split('.')
try:
nums = [int(''.join(ch for ch in p if ch.isdigit()) or 0) for p in parts[:3]]
except ValueError:
@@ -49,6 +63,92 @@ def parse_semver(value: Any) -> Optional[Tuple[int, int, int]]:
return tuple(nums) # type: ignore[return-value]
# `parse_semver` is deliberately lenient — it strips non-digits and yields
# (0, 0, 0) for a string with no numbers at all, which is fine for a floor
# (a floor of 0.0.0 never blocks anything) but wrong for a range, where the
# same leniency would turn an unreadable spec into a *refusal*. Range specs
# are therefore validated against this first, so garbage reads as "no
# evidence" rather than "incompatible".
_VERSION_TOKEN = re.compile(r"^v?\d+(\.\d+){0,2}(-[\w.-]+)?(\+[\w.-]+)?$")
def _parse_strict(value: str) -> Optional[Tuple[int, int, int]]:
"""`parse_semver`, but ``None`` unless the string really looks like one."""
if not isinstance(value, str) or not _VERSION_TOKEN.match(value.strip()):
return None
return parse_semver(value)
def _satisfies_range(core: Tuple[int, int, int], spec: str) -> Optional[bool]:
"""Does ``core`` satisfy one `compatible_versions` entry?
Returns ``None`` when the spec cannot be parsed — the caller treats that as
"no evidence" rather than as a refusal, so an unrecognised spelling never
costs a user a working install.
Supports the forms `schema/manifest_schema.json` permits: `>=`, `<=`, `>`,
`<`, `~`, `^`, a bare exact version, and an inclusive `A - B` range.
Prerelease/build suffixes are tolerated and ignored, matching `parse_semver`.
"""
spec = spec.strip()
if not spec:
return None
if " - " in spec: # inclusive range, e.g. "2.0.0 - 3.1.0"
low_raw, _, high_raw = spec.partition(" - ")
low, high = _parse_strict(low_raw), _parse_strict(high_raw)
if low is None or high is None:
return None
return low <= core <= high
for op in (">=", "<=", ">", "<", "~", "^"):
if spec.startswith(op):
target = _parse_strict(spec[len(op):])
if target is None:
return None
if op == ">=":
return core >= target
if op == "<=":
return core <= target
if op == ">":
return core > target
if op == "<":
return core < target
if op == "~":
# Patch-level changes only: >=X.Y.Z, <X.(Y+1).0
return target <= core < (target[0], target[1] + 1, 0)
# "^": minor and patch changes: >=X.Y.Z, <(X+1).0.0
return target <= core < (target[0] + 1, 0, 0)
exact = _parse_strict(spec)
return None if exact is None else core == exact
def satisfies_compatible_versions(
manifest: Dict[str, Any], core: Tuple[int, int, int]
) -> Optional[bool]:
"""Evaluate the manifest's `compatible_versions` array against ``core``.
The array is a set of *alternatives*: satisfying any one entry means the
plugin declares itself compatible. Returns ``None`` when the field is
absent or no entry could be parsed, so callers can distinguish "declared
incompatible" from "did not say".
This is the field `schema/manifest_schema.json` marks **required**, and it
is the only one that can express an upper bound — `ledmatrix_min_version`
is a floor and cannot say "not compatible with 4.x".
"""
specs = manifest.get('compatible_versions')
if not isinstance(specs, list) or not specs:
return None
verdicts = [_satisfies_range(core, s) for s in specs if isinstance(s, str)]
parsed = [v for v in verdicts if v is not None]
if not parsed:
return None
return any(parsed)
def declared_min_version(manifest: Dict[str, Any]) -> Optional[str]:
"""The core version this plugin says it needs, or ``None`` if it doesn't say.
@@ -56,16 +156,25 @@ def declared_min_version(manifest: Dict[str, Any]) -> Optional[str]:
of `ledmatrix_min_version` (`store_manager._validate_manifest_fields` flags
it); both are read because a large share of published manifests still carry
the old one.
Container types are validated rather than assumed. A hand-edited or
third-party manifest can carry `requires` as a list or `versions` as a
mapping, and both used to raise out of here (`AttributeError` and
`KeyError` respectively). That now matters far more than it did: the
untrustworthy-core branch of :func:`check` calls this for *every* manifest,
so one malformed file would take down the install path rather than just
itself. A shape we do not recognise means "no declared floor".
"""
declared = (
manifest.get('min_ledmatrix_version')
or (manifest.get('requires') or {}).get('min_ledmatrix_version')
)
declared = manifest.get('min_ledmatrix_version')
if not declared:
requires = manifest.get('requires')
if isinstance(requires, dict):
declared = requires.get('min_ledmatrix_version')
if declared:
return declared
versions = manifest.get('versions') or []
if versions and isinstance(versions[0], dict):
versions = manifest.get('versions')
if isinstance(versions, list) and versions and isinstance(versions[0], dict):
return (versions[0].get('ledmatrix_min_version')
or versions[0].get('ledmatrix_min'))
return None
@@ -74,26 +183,68 @@ def declared_min_version(manifest: Dict[str, Any]) -> Optional[str]:
def check(manifest: Dict[str, Any], core_version: str) -> Tuple[bool, Optional[str]]:
"""Return ``(compatible, reason)``.
``compatible`` is False **only** when the plugin declares a parseable floor,
the core reports a parseable and trustworthy version, and the floor is
genuinely above it. Every uncertain case resolves to compatible: an
undeclared floor, an unparseable version on either side, or a core whose
version is below `TRUSTWORTHY_FLOOR`. Refusing on a guess would break
working installs, which is the more expensive mistake here.
Two fields can say a plugin is incompatible and **the more restrictive
wins**:
- `compatible_versions` — the schema-required array of semver ranges, and
the only one that can express an upper bound.
- `ledmatrix_min_version` (or the deprecated `ledmatrix_min`) — the
per-release floor inside `versions[]`.
They agree across every published manifest today except `7-segment-clock`,
but they *can* disagree, and a plugin that says `["2.0.0 - 2.9.9"]` means
"not compatible with 3.x" no matter what its floor says.
``compatible`` is False **only** on evidence: the core reports a parseable,
trustworthy version and a field genuinely excludes it. Every uncertain case
resolves to compatible — nothing declared, an unparseable version on either
side, or a core below `TRUSTWORTHY_FLOOR`. Refusing on a guess breaks a
working install, which is the more expensive mistake here.
``reason`` is user-facing text, present only when incompatible.
"""
current = parse_semver(core_version)
name = manifest.get('name') or manifest.get('id') or 'This plugin'
if current is None or current < TRUSTWORTHY_FLOOR:
# The version is not evidence of what this core HAS. But a floor above
# the ecosystem baseline says the plugin needs modules that arrived
# *after* 2.0.0 — and a core reporting below that either is the v3.1.0
# release (which ships __version__ = "1.0.0" and has none of the 3.2.0
# modules) or is genuinely ancient. Either way it will not have them.
#
# This is the only protection available to that population: they cannot
# be told apart from a real 1.0.0 install, so the gate cannot reason
# about them, and the *plugin's* guarded-import fallback disappears at
# the B6 sunset. Refusing the install leaves them on the version they
# already run instead of handing them one that fails to load.
#
# Floors at or below 2.0.0 are still allowed, which is every manifest
# published today — so this does not lock anyone out of the store.
declared = declared_min_version(manifest)
needed = parse_semver(declared)
if needed is not None and needed > TRUSTWORTHY_FLOOR:
return False, (
f"{name} requires LEDMatrix {declared} or newer. This system "
f"reports {core_version}, which is too old to identify "
f"reliably — update LEDMatrix, then install it."
)
return True, None
# Ranges first: they are the canonical field and can rule out a core that
# clears the floor.
if satisfies_compatible_versions(manifest, current) is False:
specs = ", ".join(
s for s in manifest.get('compatible_versions', []) if isinstance(s, str))
return False, (
f"{name} supports LEDMatrix {specs}, but this system is running "
f"{core_version}. Install a build in that range, or a plugin "
f"version that supports {core_version}."
)
declared = declared_min_version(manifest)
needed = parse_semver(declared)
if needed is None:
return True, None
current = parse_semver(core_version)
if current is None or current < TRUSTWORTHY_FLOOR:
return True, None
if needed > current:
name = manifest.get('name') or manifest.get('id') or 'This plugin'
if needed is not None and needed > current:
return False, (
f"{name} requires LEDMatrix {declared} or newer, but this system is "
f"running {core_version}. Update LEDMatrix first, then install it."
+142 -29
View File
@@ -113,6 +113,15 @@ class PluginManager:
self._update_queue: "queue.Queue[Optional[Tuple[str, float]]]" = queue.Queue()
self._pending_updates: set = set()
self._pending_lock = threading.Lock()
# Serializes the "is this plugin eligible?" -> "claim it (RUNNING)"
# transition. Two schedulers run concurrently in practice — the render
# loop's _tick_plugin_updates() and Vegas mode's vegas-plugin-tick
# daemon thread, which is never joined — so without this both can
# observe ENABLED and both call update() on the same plugin. Held only
# across the check and the state transition, never across update()
# itself: that would serialize slow plugins behind each other and
# reintroduce the stall the async worker exists to avoid.
self._reservation_lock = threading.Lock()
self._plugin_locks: Dict[str, threading.Lock] = {}
self._plugin_locks_guard = threading.Lock()
self._update_worker: Optional[threading.Thread] = None
@@ -395,6 +404,37 @@ class PluginManager:
self.state_manager.set_state(plugin_id, PluginState.ERROR, error=e)
return False
#: Config keys the **core** reads out of a plugin's own config block. The
#: plugin never declares them, so a schema with
#: ``"additionalProperties": false`` — 37 of the 42 published ones — reports
#: them as violations and the plugin gets flagged degraded in the web UI for
#: using a documented core feature.
#:
#: Listed explicitly rather than matched on a ``vegas_`` prefix, because
#: ``vegas_mode`` is the opposite case: plugins *do* declare that one, and a
#: prefix rule would silently stop validating it.
#:
#: Read by: ``vegas_mode/plugin_adapter.py`` (``vegas_width_pct``,
#: ``vegas_overflow``) and ``base_plugin.py`` (``vegas_max_width_screens``).
CORE_OWNED_CONFIG_KEYS = frozenset({
'vegas_width_pct',
'vegas_overflow',
'vegas_max_width_screens',
})
def _strip_core_owned_keys(self, config: Dict[str, Any]) -> Dict[str, Any]:
"""A shallow copy of ``config`` without the core's own tuning keys.
Only the top level is touched, and only when such a key is present, so
the common case allocates nothing extra.
"""
if not isinstance(config, dict):
return config
if not self.CORE_OWNED_CONFIG_KEYS.intersection(config):
return config
return {k: v for k, v in config.items()
if k not in self.CORE_OWNED_CONFIG_KEYS}
def _validate_config_schema_soft(self, plugin_id: str, config: Dict[str, Any]) -> None:
"""Validate a plugin's config against its JSON schema — warn/degrade only.
@@ -419,7 +459,7 @@ class PluginManager:
try:
is_valid, errors = self.schema_manager.validate_config_against_schema(
config, schema, plugin_id
self._strip_core_owned_keys(config), schema, plugin_id
)
except Exception as e: # pragma: no cover - defensive
# Validation machinery itself failed — do not penalise the plugin.
@@ -777,25 +817,69 @@ class PluginManager:
if self.health_tracker and self.health_tracker.should_skip_plugin(plugin_id):
continue
# Check if plugin can execute
if not self.state_manager.can_execute(plugin_id):
continue
interval = self._get_plugin_update_interval(plugin_id, plugin_instance)
if interval is None:
continue
with self._plugin_last_update_lock:
last_update = self.plugin_last_update.get(plugin_id, 0.0)
# Eligibility check, due check and the RUNNING transition happen
# together, so a concurrent scheduler cannot claim the same plugin.
if not self._reserve_for_update(plugin_id, current_time, interval):
continue
if last_update == 0.0 or (current_time - last_update) >= interval:
if self._synchronous_updates:
# Kill-switch path: the original inline execution
# (blocks the caller until update() completes/times out)
self.state_manager.set_state(plugin_id, PluginState.RUNNING)
self._execute_update_now(plugin_id, plugin_instance, current_time)
else:
self._enqueue_update(plugin_id, current_time)
if self._synchronous_updates:
# Kill-switch path: the original inline execution
# (blocks the caller until update() completes/times out)
self._execute_update_now(plugin_id, plugin_instance, current_time)
else:
self._enqueue_update(plugin_id, current_time)
def _reserve_for_update(
self,
plugin_id: str,
current_time: Optional[float] = None,
interval: Optional[float] = None,
) -> bool:
"""Atomically claim a plugin for update, returning True if we won it.
can_execute() and the RUNNING transition have to happen under one lock.
As two separate calls, two scheduler threads can both see ENABLED and
both go on to run the same plugin's update() concurrently — unsafe for
any plugin that isn't reentrant (shared mutable state, a non-thread-safe
HTTP session or cache).
The due-time check is inside the lock too. Leaving it outside would let
a second thread that had already decided "due" claim the plugin the
instant the first finished, running update() twice in one interval.
Args:
plugin_id: Plugin to claim.
current_time: Now, for the due check. Omit to skip that check.
interval: Seconds between updates. Omit to skip the due check.
Returns:
True if this caller reserved the plugin and must dispatch it,
False if it is ineligible, not yet due, or already claimed.
"""
with self._reservation_lock:
if not self.state_manager.can_execute(plugin_id):
return False
if current_time is not None and interval is not None:
with self._plugin_last_update_lock:
last_update = self.plugin_last_update.get(plugin_id, 0.0)
if last_update != 0.0 and (current_time - last_update) < interval:
return False
self.state_manager.set_state(plugin_id, PluginState.RUNNING)
return True
def _release_reservation(self, plugin_id: str) -> None:
"""Hand a claimed plugin back when it never got dispatched.
Without this a plugin reserved but not queued would sit in RUNNING
forever, and can_execute() would refuse it on every later tick.
"""
self.state_manager.set_state(plugin_id, PluginState.ENABLED)
def get_plugin_lock(self, plugin_id: str) -> threading.Lock:
"""Per-plugin lock keeping update() and display() mutually exclusive.
@@ -812,16 +896,40 @@ class PluginManager:
return lock
def _enqueue_update(self, plugin_id: str, scheduled_time: float) -> None:
"""Queue a due update for the background worker (dedup while pending)."""
"""Queue an already-reserved update for the background worker.
The caller has reserved the plugin (RUNNING), which is what blocks
re-entry and shows the truthful state in the web UI while the item
waits its turn. The pending set stays as a second line of defence; if
it ever fires the reservation has to be handed back, or the plugin
would sit in RUNNING with nothing queued to release it.
"""
with self._pending_lock:
if plugin_id in self._pending_updates:
self.logger.warning(
"Plugin %s reserved for update but already queued; "
"releasing the reservation", plugin_id)
self._release_reservation(plugin_id)
return
self._pending_updates.add(plugin_id)
# RUNNING is set at enqueue time so can_execute() blocks re-entry and
# the web UI shows the truthful state while the item waits its turn.
self.state_manager.set_state(plugin_id, PluginState.RUNNING)
self._ensure_update_worker()
self._update_queue.put((plugin_id, scheduled_time))
try:
self._ensure_update_worker()
self._update_queue.put((plugin_id, scheduled_time))
except Exception as exc: # pylint: disable=broad-except
# Thread.start() raises RuntimeError when the OS refuses a new
# thread — a real condition on a Pi under memory pressure. Nothing
# is queued to release the plugin at that point, so the claim has to
# be undone here, or it sits in RUNNING with nothing to clear it and
# can_execute() refuses it for the rest of the process. Swallowed
# rather than raised so the remaining plugins in this tick still get
# their turn.
self.logger.error(
"Could not queue update for plugin %s (%s: %s); releasing the "
"reservation so the next tick can retry",
plugin_id, type(exc).__name__, exc, exc_info=True)
with self._pending_lock:
self._pending_updates.discard(plugin_id)
self._release_reservation(plugin_id)
def _ensure_update_worker(self) -> None:
if self._update_worker is not None and self._update_worker.is_alive():
@@ -906,6 +1014,14 @@ class PluginManager:
return
finished['done'] = True
try:
# Drop the queue reservation *before* the state goes back to
# ENABLED. The other order leaves a window where a scheduler
# sees ENABLED, reserves the plugin, then finds it still in
# _pending_updates -- the enqueue is dropped and the plugin
# would sit in RUNNING with nothing left to release it.
if lock is not None:
with self._pending_lock:
self._pending_updates.discard(plugin_id)
if success:
with self._plugin_last_update_lock:
self.plugin_last_update[plugin_id] = scheduled_time
@@ -918,8 +1034,6 @@ class PluginManager:
finally:
if lock is not None:
lock.release()
with self._pending_lock:
self._pending_updates.discard(plugin_id)
if lock is None:
# Synchronous / no-lock path: unchanged behavior.
@@ -1010,13 +1124,12 @@ class PluginManager:
if not hasattr(plugin_instance, "update"):
continue
# Check if plugin can execute
if not self.state_manager.can_execute(plugin_id):
# Eligibility check and the RUNNING transition together, so a
# concurrent scheduler cannot claim the same plugin (see
# _reserve_for_update).
if not self._reserve_for_update(plugin_id):
continue
# Update state to RUNNING
self.state_manager.set_state(plugin_id, PluginState.RUNNING)
try:
success = self.plugin_executor.execute_update(plugin_instance, plugin_id)
if success:
+98
View File
@@ -0,0 +1,98 @@
"""The core's own tuning keys must not make a plugin look broken.
`vegas_width_pct`, `vegas_overflow` and `vegas_max_width_screens` are read by
the *core* out of each plugin's config block — `vegas_mode/plugin_adapter.py`
and `base_plugin.py`. No plugin declares them, and 37 of the 42 published
config schemas set `"additionalProperties": false`, so schema validation
reported them as violations.
That is not just log noise: `_validate_config_schema_soft` sets `degraded` in
the health tracker, which the web UI surfaces. Measured on a real device, **9
of 27 installed plugins** were flagged degraded purely for using a documented
core feature — including `baseball-scoreboard` and `f1-scoreboard`.
The fix strips those keys before validating. It deliberately does *not* match
on a `vegas_` prefix: `vegas_mode` is plugin-owned and declared in schemas, and
a prefix rule would silently stop validating it.
"""
from unittest.mock import MagicMock
import pytest
from src.plugin_system.plugin_manager import PluginManager
STRICT_SCHEMA = {
"type": "object",
"additionalProperties": False,
"properties": {
"enabled": {"type": "boolean"},
"vegas_mode": {"type": "string"}, # plugin-owned, must stay validated
},
}
@pytest.fixture
def manager():
mgr = PluginManager.__new__(PluginManager) # skip the heavy constructor
mgr.logger = MagicMock()
mgr.schema_manager = MagicMock()
mgr._set_degraded_safe = MagicMock()
return mgr
class TestStripCoreOwnedKeys:
def test_removes_every_core_owned_key(self, manager):
cfg = {"enabled": True, "vegas_width_pct": 50,
"vegas_overflow": "wrap", "vegas_max_width_screens": 2}
assert manager._strip_core_owned_keys(cfg) == {"enabled": True}
def test_leaves_plugin_owned_vegas_mode_alone(self, manager):
"""A prefix rule would have eaten this one."""
cfg = {"enabled": True, "vegas_mode": "scroll"}
assert manager._strip_core_owned_keys(cfg) == cfg
def test_returns_the_same_object_when_nothing_to_strip(self, manager):
cfg = {"enabled": True}
assert manager._strip_core_owned_keys(cfg) is cfg
def test_does_not_mutate_the_caller_config(self, manager):
cfg = {"enabled": True, "vegas_width_pct": 50}
manager._strip_core_owned_keys(cfg)
assert "vegas_width_pct" in cfg, "the live plugin config was mutated"
def test_tolerates_a_non_dict(self, manager):
assert manager._strip_core_owned_keys(None) is None
class TestSoftValidation:
def _validate_with(self, manager, config, valid=True, errors=()):
manager.schema_manager.load_schema.return_value = STRICT_SCHEMA
manager.schema_manager.validate_config_against_schema.return_value = (
valid, list(errors))
manager._validate_config_schema_soft("baseball-scoreboard", config)
return manager.schema_manager.validate_config_against_schema.call_args
def test_core_keys_never_reach_the_validator(self, manager):
"""The regression: these keys reaching a strict schema is what flagged
9 of 27 plugins degraded."""
args = self._validate_with(
manager, {"enabled": True, "vegas_width_pct": 50})
validated = args[0][0]
assert "vegas_width_pct" not in validated
assert validated == {"enabled": True}
def test_plugin_owned_keys_still_reach_the_validator(self, manager):
args = self._validate_with(
manager, {"enabled": True, "vegas_mode": "scroll"})
assert args[0][0]["vegas_mode"] == "scroll"
def test_a_genuine_violation_is_still_reported(self, manager):
"""Stripping core keys must not turn the check into a no-op."""
self._validate_with(
manager, {"enabled": True, "typo_key": 1},
valid=False, errors=["Field root: 'typo_key' was unexpected"])
manager._set_degraded_safe.assert_called()
reason = manager._set_degraded_safe.call_args[0][1]
assert reason and "typo_key" in reason
+204 -6
View File
@@ -81,17 +81,24 @@ class TestCheck:
ok, reason = compatibility.check({"id": "x"}, "3.2.0")
assert ok is True and reason is None
def test_untrustworthy_core_version_allows_everything(self):
def test_untrustworthy_core_allows_todays_ecosystem_floor(self):
"""The v3.1.0 release reports 1.0.0. Nearly every manifest floors at
2.0.0, so blocking here would stop those users installing any plugin
at all — strictly worse than the problem being solved."""
2.0.0, so blocking *that* would stop those users installing any plugin
at all — strictly worse than the problem being solved.
A floor ABOVE 2.0.0 is refused instead; see
TestUntrustworthyCoreAndTheSunset for why that case is different."""
ok, reason = compatibility.check(
{"min_ledmatrix_version": "3.2.0"}, "1.0.0")
{"min_ledmatrix_version": "2.0.0"}, "1.0.0")
assert ok is True and reason is None
def test_unparseable_core_version_allows(self):
ok, _ = compatibility.check({"min_ledmatrix_version": "3.2.0"}, "not-a-version")
def test_unparseable_core_version_allows_the_ecosystem_floor(self):
"""An unidentifiable core is treated exactly like an untrustworthy one:
today's 2.0.0 floor is allowed, a post-sunset floor is not."""
ok, _ = compatibility.check({"min_ledmatrix_version": "2.0.0"}, "not-a-version")
assert ok is True
ok, _ = compatibility.check({"min_ledmatrix_version": "3.2.0"}, "not-a-version")
assert ok is False
def test_unparseable_floor_allows(self):
ok, _ = compatibility.check({"min_ledmatrix_version": {"nope": 1}}, "3.2.0")
@@ -230,3 +237,194 @@ class TestLoaderAndStoreAgree:
)
assert loader_would_warn is (not expected)
assert hasattr(PluginLoader, "_warn_if_incompatible")
# --------------------------------------------------------------------------
# compatible_versions — the schema-required field, and the only one that can
# express an upper bound
# --------------------------------------------------------------------------
class TestCompatibleVersions:
@pytest.mark.parametrize("spec,core,expected", [
(">=2.0.0", "3.2.0", True),
(">=2.0.0", "1.9.9", False),
("<=3.0.0", "3.2.0", False),
("<=3.0.0", "2.9.0", True),
(">3.2.0", "3.2.0", False),
("<4.0.0", "3.2.0", True),
("3.2.0", "3.2.0", True), # bare == exact match
("3.2.0", "3.2.1", False),
("~3.2.0", "3.2.9", True), # patch-level only
("~3.2.0", "3.3.0", False),
("^3.2.0", "3.9.9", True), # minor + patch
("^3.2.0", "4.0.0", False),
("2.0.0 - 3.2.0", "3.2.0", True), # inclusive both ends
("2.0.0 - 3.2.0", "2.0.0", True),
("2.0.0 - 3.2.0", "3.2.1", False),
("v3.2.0", "3.2.0", True), # leading v tolerated
("3.2.0-beta.1", "3.2.0", True), # prerelease suffix ignored
])
def test_range_forms(self, spec, core, expected):
got = compatibility.satisfies_compatible_versions(
{"compatible_versions": [spec]}, compatibility.parse_semver(core))
assert got is expected, f"{spec!r} vs {core}"
def test_array_is_alternatives_not_conjunction(self):
"""Satisfying any one entry is enough — otherwise ['<2.0.0','>=3.0.0']
could never be satisfied by anything."""
m = {"compatible_versions": ["<2.0.0", ">=3.0.0"]}
assert compatibility.satisfies_compatible_versions(
m, compatibility.parse_semver("3.2.0")) is True
def test_absent_or_unparseable_is_no_evidence(self):
core = compatibility.parse_semver("3.2.0")
assert compatibility.satisfies_compatible_versions({}, core) is None
assert compatibility.satisfies_compatible_versions(
{"compatible_versions": []}, core) is None
assert compatibility.satisfies_compatible_versions(
{"compatible_versions": ["not a version"]}, core) is None
# One unparseable entry alongside a good one must not poison the result.
assert compatibility.satisfies_compatible_versions(
{"compatible_versions": ["garbage", ">=2.0.0"]}, core) is True
class TestMoreRestrictiveWins:
def test_upper_bound_blocks_a_core_that_clears_the_floor(self):
"""The gap this closes: the floor says 2.0.0 and the core is 3.2.0, so
the floor alone would allow it — but the plugin said it stops at 2.x."""
m = {"name": "Legacy Plugin",
"compatible_versions": ["2.0.0 - 2.9.9"],
"versions": [{"ledmatrix_min_version": "2.0.0"}]}
ok, reason = compatibility.check(m, "3.2.0")
assert ok is False
assert "2.0.0 - 2.9.9" in reason and "3.2.0" in reason
def test_floor_blocks_when_ranges_would_allow(self):
m = {"name": "Needs Newer",
"compatible_versions": [">=1.0.0"],
"versions": [{"ledmatrix_min_version": "9.9.9"}]}
ok, reason = compatibility.check(m, "3.2.0")
assert ok is False
assert "9.9.9" in reason
def test_both_satisfied_allows(self):
m = {"compatible_versions": [">=2.0.0"],
"versions": [{"ledmatrix_min_version": "2.0.0"}]}
assert compatibility.check(m, "3.2.0") == (True, None)
def test_untrustworthy_core_still_bypasses_both_checks(self):
"""A core reporting 1.0.0 fails `>=2.0.0`, which 41 of 42 published
manifests declare. Blocking there would empty the plugin store for
exactly the users who cannot be helped by it."""
m = {"compatible_versions": [">=2.0.0"],
"versions": [{"ledmatrix_min_version": "2.0.0"}]}
assert compatibility.check(m, "1.0.0") == (True, None)
class TestUntrustworthyCoreAndTheSunset:
"""The population B6 would otherwise break.
A device installed from the v3.1.0 release reports `__version__ = "1.0.0"`.
The gate cannot tell it apart from a genuine 1.0.0 install, so it cannot
reason about what that core actually has — and at the B6 sunset the
plugin's guarded-import fallback is gone. Without this rule the store hands
those users a 3.2.0-floored plugin that fails to load, and nothing else in
the system protects them.
The rule: on an untrustworthy core, refuse a floor *above* the ecosystem
baseline, allow anything at or below it. Every manifest published today
floors at exactly 2.0.0, so nobody is locked out of the store.
"""
UNTRUSTWORTHY = ["1.0.0", "0.9.0", "1.9.9"]
@pytest.mark.parametrize("core", UNTRUSTWORTHY)
def test_refuses_a_post_sunset_floor(self, core):
m = {"name": "Hockey Scoreboard",
"versions": [{"ledmatrix_min_version": "3.2.0"}]}
ok, reason = compatibility.check(m, core)
assert ok is False, (
f"core {core} must not receive a 3.2.0-floored plugin: after the "
"sunset there is no fallback and it will fail to load"
)
assert "3.2.0" in reason and core in reason
@pytest.mark.parametrize("core", UNTRUSTWORTHY)
def test_still_allows_todays_ecosystem_floor(self, core):
"""Regression guard: every published manifest floors at 2.0.0. If this
starts refusing, those users lose the plugin store entirely."""
m = {"versions": [{"ledmatrix_min": "2.0.0"}]}
assert compatibility.check(m, core) == (True, None)
@pytest.mark.parametrize("core", UNTRUSTWORTHY)
def test_still_allows_an_undeclared_floor(self, core):
assert compatibility.check({"id": "x"}, core) == (True, None)
def test_trustworthy_core_below_the_floor_is_unaffected(self):
"""A core that reports 3.1.0 is believable and already handled by the
ordinary comparison — not by this rule."""
m = {"name": "P", "versions": [{"ledmatrix_min_version": "3.2.0"}]}
ok, reason = compatibility.check(m, "3.1.0")
assert ok is False
assert "too old to identify" not in reason, (
"a believable version should get the ordinary message"
)
class TestMalformedManifests:
"""A manifest we cannot parse must read as "no declared floor", not raise.
This matters more since the untrustworthy-core branch of check() began
resolving the floor for *every* manifest: one hand-edited or third-party
file with the wrong shape would take down the whole install path rather
than just itself.
"""
@pytest.mark.parametrize("manifest", [
{"requires": ["python>=3.9"]}, # a list, not a mapping
{"requires": "python>=3.9"}, # a bare string
{"versions": {"a": 1}}, # a mapping, not a list
{"versions": "1.0.0"}, # a bare string
{"versions": [None]}, # a list of the wrong thing
{"versions": []},
])
def test_shape_errors_read_as_no_floor(self, manifest):
assert compatibility.declared_min_version(manifest) is None
assert compatibility.check(manifest, "1.0.0") == (True, None)
assert compatibility.check(manifest, "3.2.0") == (True, None)
def test_a_valid_requires_block_still_works(self):
assert compatibility.declared_min_version(
{"requires": {"min_ledmatrix_version": "3.2.0"}}) == "3.2.0"
class TestSuffixedVersions:
"""Prerelease and build metadata must not leak into the numbers.
The digit scrape used to pull them in: "3.2.0+build42" became (3, 2, 42)
and "3.2.0-rc1" became (3, 2, 1) — a release candidate ranking above its
own release. Both fed reject decisions.
"""
@pytest.mark.parametrize("text,expected", [
("3.2.0", (3, 2, 0)),
("3.2.0+build42", (3, 2, 0)),
("3.2.0-rc1", (3, 2, 0)),
("3.2.0-rc.1+build.9", (3, 2, 0)),
("v3.2.0+build42", (3, 2, 0)),
])
def test_suffixes_are_dropped(self, text, expected):
assert compatibility.parse_semver(text) == expected
def test_a_build_of_the_pinned_version_is_not_refused(self):
"""The regression: an exact "3.2.0" pin refused a core running
3.2.0+build42, which is that same version."""
ok, reason = compatibility.check(
{"compatible_versions": ["3.2.0"]}, "3.2.0+build42")
assert ok is True, f"refused a build of the pinned version: {reason}"
def test_a_release_candidate_does_not_outrank_its_release(self):
m = {"versions": [{"ledmatrix_min_version": "3.2.0"}]}
assert compatibility.check(m, "3.2.0-rc1")[0] is True
# ...and still refuses something genuinely older.
assert compatibility.check(m, "3.1.0-rc1")[0] is False
+335
View File
@@ -0,0 +1,335 @@
"""Plugin update scheduling must be atomic (issue #401).
`run_scheduled_updates()` decided whether to update a plugin with a
check-then-act sequence: `can_execute()` and `set_state(RUNNING)` were separate
calls with nothing between them, so two scheduler threads could both observe
ENABLED and both go on to call the same plugin's `update()`.
Two schedulers really do run at once. The render loop calls
`_tick_plugin_updates()`, and Vegas mode fires its own `vegas-plugin-tick`
daemon thread every few seconds; that thread is never joined when
`VegasModeCoordinator.play()` returns, so a slow `update()` still in flight can
overlap the next tick from the main loop.
A plugin whose `update()` runs twice at once is unsafe unless it happens to be
reentrant — shared mutable state, a non-thread-safe HTTP session or cache all
break. These tests pin the reservation that closes it, and the state
bookkeeping that has to survive it: a plugin reserved but never dispatched must
not be stranded in RUNNING, because `can_execute()` would then refuse it
forever.
"""
import os
import sys
import threading
import time
import pytest
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
from src.plugin_system.plugin_manager import PluginManager # noqa: E402
from src.plugin_system.plugin_state import PluginState # noqa: E402
class OverlapDetectingPlugin:
"""Records the high-water mark of concurrent update() calls."""
def __init__(self, update_seconds=0.25):
self.enabled = True
self.update_seconds = update_seconds
self.update_calls = 0
self.max_concurrent = 0
self._active = 0
self._guard = threading.Lock()
def update(self):
with self._guard:
self._active += 1
self.update_calls += 1
self.max_concurrent = max(self.max_concurrent, self._active)
try:
time.sleep(self.update_seconds)
finally:
with self._guard:
self._active -= 1
return True
def display(self, force_clear=False):
return True
@pytest.fixture
def pm(tmp_path):
manager = PluginManager(plugins_dir=str(tmp_path), config_manager=None,
display_manager=None, cache_manager=None)
yield manager
manager.stop_update_worker()
def _install(pm, plugin, plugin_id="racy-plugin", interval=0.01):
pm.plugins[plugin_id] = plugin
pm._update_interval_cache[plugin_id] = interval
pm.state_manager.set_state(plugin_id, PluginState.ENABLED)
return plugin_id
def _widen_check_then_act_window(pm, delay=0.02):
"""Hold every scheduler thread inside the eligibility check at once.
The real gap between can_execute() and the RUNNING transition is a couple
of bytecodes wide, so a plain thread race under the GIL almost never lands
in it — an unfixed scheduler looks correct in a test that just hammers it.
Delaying the check reproduces the interleaving that Vegas's tick thread and
the render loop actually produce when a slow update() overlaps the next
tick, and it is what makes these tests fail without the reservation.
Once the check and the transition are under one lock, the delay only
serializes the schedulers: the losers observe RUNNING and back off.
"""
real_can_execute = pm.state_manager.can_execute
def slow_can_execute(plugin_id):
result = real_can_execute(plugin_id)
time.sleep(delay)
return result
pm.state_manager.can_execute = slow_can_execute
def _hammer(target, threads=8, rounds=1):
"""Run `target` on N threads released simultaneously by a barrier."""
errors = []
barrier = threading.Barrier(threads)
def runner():
try:
barrier.wait(timeout=5)
for _ in range(rounds):
target()
except Exception as exc: # noqa: BLE001 - surfaced by the assertion
errors.append(exc)
workers = [threading.Thread(target=runner) for _ in range(threads)]
for worker in workers:
worker.start()
for worker in workers:
worker.join(timeout=30)
assert not errors, f"worker raised: {errors[0]!r}"
class TestReservationAtomicity:
"""The check-and-claim itself, independent of any dispatch path."""
def test_only_one_caller_wins_the_reservation(self, pm):
plugin_id = _install(pm, OverlapDetectingPlugin())
_widen_check_then_act_window(pm)
wins = []
lock = threading.Lock()
def claim():
if pm._reserve_for_update(plugin_id):
with lock:
wins.append(threading.current_thread().name)
_hammer(claim, threads=16)
assert len(wins) == 1, (
f"{len(wins)} threads reserved the same plugin concurrently; "
"the eligibility check and the RUNNING transition are not atomic")
assert pm.state_manager.get_state(plugin_id) == PluginState.RUNNING
def test_reservation_refused_while_running(self, pm):
plugin_id = _install(pm, OverlapDetectingPlugin())
assert pm._reserve_for_update(plugin_id) is True
assert pm._reserve_for_update(plugin_id) is False
def test_reservation_can_be_handed_back(self, pm):
plugin_id = _install(pm, OverlapDetectingPlugin())
assert pm._reserve_for_update(plugin_id) is True
pm._release_reservation(plugin_id)
assert pm.state_manager.get_state(plugin_id) == PluginState.ENABLED
assert pm._reserve_for_update(plugin_id) is True, \
"a released reservation must be claimable again"
def test_due_check_is_inside_the_reservation(self, pm):
"""Two threads that both decided 'due' must not both get a turn.
With the due check outside the lock, the loser of the race could claim
the plugin the moment the winner finished, running update() twice
inside one interval.
"""
plugin_id = _install(pm, OverlapDetectingPlugin(), interval=60.0)
now = time.time()
assert pm._reserve_for_update(plugin_id, now, 60.0) is True
pm.plugin_last_update[plugin_id] = now
pm._release_reservation(plugin_id)
assert pm._reserve_for_update(plugin_id, now, 60.0) is False, \
"plugin updated just now must not be due again"
class TestNoConcurrentUpdate:
"""The end-to-end invariant the issue is actually about."""
def test_synchronous_path_never_overlaps(self, pm):
"""The kill-switch path ran update() inline with no dedup at all."""
pm._synchronous_updates = True
plugin = OverlapDetectingPlugin(update_seconds=0.25)
_install(pm, plugin)
_widen_check_then_act_window(pm)
_hammer(pm.run_scheduled_updates, threads=8)
assert plugin.max_concurrent == 1, (
f"update() ran {plugin.max_concurrent}x concurrently on the "
"synchronous path")
def test_update_all_plugins_never_overlaps(self, pm):
plugin = OverlapDetectingPlugin(update_seconds=0.25)
_install(pm, plugin)
_widen_check_then_act_window(pm)
_hammer(pm.update_all_plugins, threads=8)
assert plugin.max_concurrent == 1, (
f"update() ran {plugin.max_concurrent}x concurrently via "
"update_all_plugins()")
def test_async_path_never_overlaps(self, pm):
plugin = OverlapDetectingPlugin(update_seconds=0.2)
plugin_id = _install(pm, plugin)
_hammer(pm.run_scheduled_updates, threads=8, rounds=3)
# Wait for an update to have both started and finished. Polling only
# `_active` races the worker: before it picks the item up nothing is
# active yet, so the loop would fall straight through and assert on a
# plugin that never ran.
deadline = time.monotonic() + 15
while time.monotonic() < deadline:
if plugin.update_calls >= 1 and plugin._active == 0:
break
time.sleep(0.05)
assert plugin.update_calls >= 1, "no update ran on the async path"
assert plugin.max_concurrent == 1, (
f"update() ran {plugin.max_concurrent}x concurrently on the "
"async path")
assert plugin_id in pm.plugins
class TestNoStrandedState:
"""A reservation that is never dispatched must not wedge the plugin."""
def test_plugin_returns_to_enabled_after_async_updates(self, pm):
plugin = OverlapDetectingPlugin(update_seconds=0.1)
plugin_id = _install(pm, plugin)
_hammer(pm.run_scheduled_updates, threads=6, rounds=2)
deadline = time.monotonic() + 15
while time.monotonic() < deadline:
if (plugin.update_calls >= 1
and pm.state_manager.get_state(plugin_id) == PluginState.ENABLED
and not pm._pending_updates):
break
time.sleep(0.05)
# ENABLED is also the starting state, so without this the assertion
# below would pass on a plugin that never got scheduled at all.
assert plugin.update_calls >= 1, "no update ran; the state assertion would be vacuous"
assert pm.state_manager.get_state(plugin_id) == PluginState.ENABLED, \
"plugin stranded in RUNNING; can_execute() would refuse it forever"
assert not pm._pending_updates, "pending set not drained"
def test_pending_cleared_before_state_reset(self, pm):
"""Invariant: never-RUNNING implies never-pending.
_finish() used to clear the pending entry *after* flipping the state
back to ENABLED. In that window a scheduler could reserve the plugin
and then have its enqueue dropped by the pending-dedup.
"""
plugin = OverlapDetectingPlugin(update_seconds=0.05)
plugin_id = _install(pm, plugin)
violations = []
stop = threading.Event()
def watcher():
while not stop.is_set():
state = pm.state_manager.get_state(plugin_id)
if state != PluginState.RUNNING and plugin_id in pm._pending_updates:
violations.append(state)
time.sleep(0.001)
thread = threading.Thread(target=watcher, daemon=True)
thread.start()
try:
for _ in range(15):
pm.run_scheduled_updates()
time.sleep(0.05)
finally:
stop.set()
thread.join(timeout=5)
assert not violations, (
f"{len(violations)} sample(s) saw a non-RUNNING plugin still in "
"_pending_updates")
class TestDispatchFailure:
"""A reservation must survive the dispatch itself failing.
_enqueue_update() claims the plugin, adds it to the pending set, and only
then starts the worker and queues the item. threading.Thread.start() raises
RuntimeError when the OS refuses a new thread — not hypothetical on a Pi
under memory or thread pressure. Nothing is queued to release the plugin at
that point, so without an explicit rollback it stays RUNNING with a stale
pending entry and can_execute() refuses it for the rest of the process.
"""
def test_reservation_released_when_the_worker_cannot_start(self, pm):
plugin_id = _install(pm, OverlapDetectingPlugin())
def refuse_to_start():
raise RuntimeError("can't start new thread")
pm._ensure_update_worker = refuse_to_start
assert pm._reserve_for_update(plugin_id) is True
pm._enqueue_update(plugin_id, time.time())
assert pm.state_manager.get_state(plugin_id) == PluginState.ENABLED, \
"plugin left in RUNNING after a failed dispatch; can_execute() " \
"would refuse it forever"
assert plugin_id not in pm._pending_updates, \
"stale pending entry would make the next enqueue hit the dedup"
assert pm._reserve_for_update(plugin_id) is True, \
"plugin must be claimable again on the next tick"
def test_dispatch_failure_does_not_abort_the_rest_of_the_tick(self, pm):
"""One plugin failing to queue must not skip the others in that tick."""
first = OverlapDetectingPlugin()
second = OverlapDetectingPlugin()
_install(pm, first, plugin_id="plugin-a")
_install(pm, second, plugin_id="plugin-b")
calls = []
real_ensure = pm._ensure_update_worker
def fail_first_only():
calls.append(1)
if len(calls) == 1:
raise RuntimeError("can't start new thread")
return real_ensure()
pm._ensure_update_worker = fail_first_only
pm.run_scheduled_updates() # must not propagate the RuntimeError
assert len(calls) == 2, (
"run_scheduled_updates() stopped after the failing plugin; the "
"exception escaped the enqueue")
for plugin_id in ("plugin-a", "plugin-b"):
assert pm.state_manager.get_state(plugin_id) in (
PluginState.ENABLED, PluginState.RUNNING), \
f"{plugin_id} left in an unexpected state"