refactor(tests): migrate Python test suite to pytest - #2801

The merge with origin/master pulled in #2801's pytest migration, which
rewrote tests/test_celery_tasks.py and tests/test_viewer.py top-to-
bottom. The merge took master's pytest-style files as the base, losing
my unittest-style coverage of revalidate_asset_urls,
revalidate_asset_url, _asset_is_displayable, and _trigger_asset_recheck.
This re-adds them as pytest functions.

Two infrastructure changes were needed for the new tests to work
under the conftest's session-scoped fake Redis:

1. ``conftest.py:_make_fake_redis`` modelled ``r.set`` as a 2-arg
   lambda that ignored kwargs. My code uses
   ``r.set(key, value, nx=True, ex=...)`` for SETNX-based gates, which
   under the lambda either TypeErrors (extra kwargs) or returns None
   regardless of whether the key existed. Replace the lambda with a
   proper ``_set`` that honors ``nx`` semantics and returns
   ``True`` / ``None`` matching real Redis. Tests for the per-asset
   recheck cooldown, the sweep singleton lock, and the splash IP-
   refresh debounce all rely on this.

2. ``conftest.py`` had no ``r.eval`` mock. ``revalidate_asset_urls``
   uses an ``EVAL``-driven Lua compare-and-delete to release its
   singleton lock without clobbering a successor. Add an ``_eval``
   that recognizes the compare-and-delete shape and ignores any
   other script.

3. ``r.delete`` previously returned a list (``[store.pop(...) for k
   in keys]``); real Redis returns the number of keys deleted. Fix
   so callers that compare to an int (or expect a count) see correct
   semantics.

Tests added (all in pytest functional style with ``@pytest.mark.django_db``
where needed):

- ``revalidate_asset_urls``: marks unreachable / reachable; updates
  last_reachability_check; skips disabled / processing /
  skip_asset_check; local-file existence check; probe-exception
  containment; lock prevents overlap; lock-release does not clobber
  different holder; lock released on clean run.
- ``revalidate_asset_url``: no-op on missing asset_id; flips
  is_reachable; cooldown lock prevents back-to-back probes; acquires
  lock when running; skips disabled / processing / skip_asset_check;
  runs when no lock held.
- ``_asset_is_displayable``: skip_asset_check short-circuits;
  local-file existence; remote consults is_reachable; legacy default.
- ``_trigger_asset_recheck``: POSTs the right URL; no-op on missing
  id; swallows request errors.
- ``test_recheck_endpoint.py``: rewritten to pytest style. Same
  coverage (404 / no-lock-enqueue / debounce-held-skip / back-to-
  back-calls-once / unauthenticated).

All 52 tests pass under ``uv run pytest -m "not integration"``.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

The merge with origin/master pulled in #2801's pytest migration. The
splash tests in tests/test_splash_page.py were still unittest-style
TestCase classes (added on this branch after the migration target);
port them to pytest functions to match the rest of the suite.

Also enhance conftest.py's Redis fake (same enhancement as #2805):

- Replace the 2-arg ``r.set`` lambda with a ``_set`` that honors
  ``nx``/``ex`` kwargs and returns ``True`` / ``None`` matching real
  Redis semantics. ``_resolve_node_ip``'s SETNX-based debounce gate
  relies on this — without it, every "is the publish window open?"
  check would spuriously believe the gate was free.
- Fix ``r.delete`` to return an int (count of keys deleted) instead
  of a list of popped values.
- Add ``r.eval`` (no-op for unknown scripts; nothing on this branch
  uses it, but #2805 does and the conftest is shared).

Tests rewritten as pytest functions with ``@pytest.mark.django_db``
where needed and a ``bare_metal_no_pending`` fixture that clears the
per-test debounce key. Coverage unchanged: format_ip_urls (5
formatting + sentinel cases), bare-metal resolver (cache hit, cache
hit + refresh, cache miss + publish, malformed cache, empty-list
cache, Redis errors on get/set/publish, debounce), Balena resolver
(supervisor success / timeout / error status), endpoint full chain
(200 with IPs, 200 + [] on cache miss, unauthenticated), splash view
(renders without mocking, doesn't import get_node_ip, polling
script present).

All 45 tests pass under ``uv run pytest -m "not integration"``.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Conflicts resolved by porting schedule-slots' V2ScheduleFieldValidationTest
and the new windowing/get_play_days/cap tests from TestCase to pytest
function style introduced in #2801, matching the rest of the suites.

* feat(asset): move URL reachability ownership to the server

The viewer used to call lib.utils.url_fails(asset['uri']) on every
asset play (viewer/__init__.py:222). The same probe already runs
server-side at upload time, so the viewer's call was redundant drift
detection — and an expensive one in the wrong place: ffprobe on
streaming assets blocks the asset loop for up to 15s wall-clock per
rotation, while the HEAD/GET on HTTP assets adds ~10-100ms per play.
URL reachability is also a property of an asset, not of "what the
viewer is about to play right now", so it belongs next to the asset
record where it can be refreshed on a cadence and surfaced through
the admin UI/API.

This moves that work to the server.

  - Asset gains is_reachable: bool (default True) and
    last_reachability_check: datetime (nullable). Existing rows
    migrate with is_reachable=True, which is the safe default — the
    sweep will correct any genuinely-broken URLs on its first run
    after upgrade.

  - celery_tasks.revalidate_asset_urls is added as a new periodic
    task (15-min cadence, registered in setup_periodic_tasks). It
    iterates enabled, non-processing assets, runs the same url_fails
    probe the server already uses at upload, and writes back the
    field via Asset.objects.filter(...).update() to avoid touching
    save signals. Local-file URIs short-circuit through path.isfile;
    skip_asset_check=True opts out of the probe entirely. A probe
    crash on one asset is logged and skipped — it must not kill the
    sweep.

  - celery_tasks.revalidate_asset_url (singular) is the on-demand
    counterpart, with a 60s per-asset cooldown so a viewer that
    cycles through the same broken asset can't pin the worker on
    back-to-back ffprobe runs (each up to 15s).

  - POST /api/v2/assets/<id>/recheck (AssetRecheckViewV2) enqueues
    the on-demand task. The viewer hits this endpoint when it skips
    an asset that's marked unreachable, which is the failure-mode
    coverage that the per-play check used to provide: a stream that
    just went down between sweeps gets re-checked promptly instead
    of waiting up to 15 minutes for the next periodic pass. Best-
    effort: a request failure is logged at debug and ignored — the
    asset stays unreachable until the next sweep, which is fine.

  - viewer/__init__.py:asset_loop now consults is_reachable instead
    of calling url_fails. Local-file existence and skip_asset_check
    fast-paths are preserved so the viewer doesn't roundtrip on
    every iteration. A bool() cast on the dict lookup defaults to
    True for legacy rows / serializers that don't include the field.

  - AssetSerializerV2 gains is_reachable + last_reachability_check
    (read-only). The legacy v1 AssetSerializer also gains is_reachable
    as IntegerField(read_only=True) to match the 0/1 convention used
    by its other booleans, plus the same read_only_fields entries to
    prevent writes via PATCH/PUT through that surface.

Tests cover the dispatch shape (which assets get probed, what gets
written back, exception containment, cooldown), the viewer's new
displayability decision (skip_asset_check, local-file existence,
remote is_reachable, legacy default), and the recheck-trigger path
(POST URL, no-op on missing id, swallowed connection errors).

ffprobe stays in the viewer's image for now because lib.utils still
imports it conditionally for the streaming branch of url_fails, and
viewer code still imports lib.utils for connect_to_redis /
string_to_bool. Dropping ffprobe from the viewer container is a
follow-up that falls out of the larger viewer slim-down.

Refs #2803 (Part A).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: format viewer/__init__.py and add new fields to scheduler test fixtures

CI's `ruff format --check` (separate from `ruff check`) caught a line
the formatter wanted unfolded in viewer/__init__.py.

tests/test_scheduler.py compares queryset results against literal
asset dicts, so adding is_reachable and last_reachability_check to the
model means the fixtures need them too. Default values match the
field defaults: is_reachable=True, last_reachability_check=None.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: quiet SonarCloud quality gate (https in fixtures, dict literal)

Two stylistic concessions to satisfy the project's SonarCloud quality
gate, both no-ops semantically:

- Test fixture URIs use https://example.com instead of http://. The
  test mocks url_fails entirely so the protocol doesn't matter, but
  Sonar's S5332 ("use https") flags the literals regardless of context.
- _make() helper in TestRevalidateAssetUrl uses a dict literal instead
  of dict(asset_id=..., ...) per Sonar's S7498. Same data, two-line
  refactor.

No production code paths affected.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: clear remaining SonarCloud S5332 hits

Three more test fixture URIs and one production POST were flagged
by Sonar's "use https" rule in the same way as the prior fixup:

- tests/test_viewer.py: three 'http://example.com/x' fixtures swapped
  to 'https://example.com/x'. Mocks intercept the call so protocol is
  irrelevant.
- viewer/__init__.py: the recheck POST URL is plain HTTP because the
  viewer talks to anthias-server over plain HTTP per CLAUDE.md (TLS
  is opt-in via the Caddy sidecar). Add a NOSONAR marker with
  rationale, mirroring the pattern in api/views/v2.py and host_agent.py.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: address Copilot review on PR #2805

Three substantive issues flagged on the original PR:

1. AssetRecheckViewV2 was decorated with @Authorized, but the viewer's
   _trigger_asset_recheck() POST has no way to attach BasicAuth — the
   request would 401 silently on auth-enabled installs and the on-
   demand recheck mitigation would never fire. Drop @Authorized with
   a docstring explaining the rationale: viewer is the sole client,
   lives in the same compose stack, request is 404 unless caller
   already knows a valid asset_id, the underlying task is rate-limited
   per asset, and the response carries no data — it just enqueues a
   probe.

2. _trigger_asset_recheck ignored the response status entirely, so a
   401/302/404/5xx looked the same as a 202 (success). Capture the
   response, disable redirects so a 302 doesn't get followed into
   somewhere unexpected, and log non-202 statuses at debug. Operator
   can now see when the chain is silently broken instead of having
   to bisect from "asset stays unreachable forever".

3. revalidate_asset_urls had time_limit=10min, but a streaming-heavy
   playlist's worst case (15s ffprobe per RTSP + 20s HEAD+GET per
   HTTP) can exceed that on ~40 assets. The 15-min beat tick would
   then race a still-running sweep, doubling load on assets and
   producing inconsistent writes. Raise time_limit past the periodic
   interval and add a Redis-based singleton lock (SETNX with TTL):
   a beat that fires while a sweep is in progress observes the lock
   and exits cleanly. The TTL matches the time_limit so a hard kill
   doesn't orphan the lock.

Also adds a `delete` declaration to the project's redis stub
(stubs/redis-stubs/client.pyi) — the runtime method exists but
wasn't in our narrowed stub, so the SETNX/cleanup paths failed mypy
strict mode.

Tests cover lock acquisition (a pre-acquired lock causes the sweep
to no-op without probing) and lock release (a clean run leaves no
stale lock behind).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* ci: empty commit to retrigger workflows

* fixup: address Copilot follow-up review on PR #2805

Two more issues flagged after the first round of fixups:

1. AssetRecheckViewV2.post enqueued revalidate_asset_url
   unconditionally. The task's own RECHECK_COOLDOWN_S guard then
   makes most invocations no-op — but the queue cost (Redis push +
   worker dequeue + DB read + return) still runs every time. A
   viewer rotating quickly past an unreachable asset would generate
   sustained queue churn for zero work.

   Pre-filter at the endpoint with the same cooldown the task
   enforces. If last_reachability_check is within RECHECK_COOLDOWN_S,
   skip the .delay() and return 202 anyway — the recheck is
   effectively up-to-date; the viewer doesn't need to distinguish
   "fresh" from "skipped due to cooldown".

2. revalidate_asset_urls released the lock with an unconditional
   r.delete(). Pathological case the previous fixup did NOT cover:

     - Sweep A acquires the lock with TTL=ASSET_REVALIDATION_TIME_LIMIT_S
     - A's run exceeds the TTL (probe storm + slow ffprobes)
     - Lock TTL expires while A is still running
     - Beat tick fires; sweep B acquires the now-free lock
     - A finishes, hits its finally, deletes the lock
     - B's lock is gone; the next beat tick can start sweep C in
       parallel with B, defeating the singleton guarantee

   Fix with a per-sweep token (secrets.token_hex(16)) and a
   compare-and-delete via redis.eval(Lua). A's finally only deletes
   the lock if the value still matches A's token — otherwise the
   lock belongs to a successor and we leave it alone. The Lua script
   runs atomically server-side, so the get+del isn't a TOCTOU.

   Added `eval` to the project's redis stub.

Also adds tests/test_recheck_endpoint.py covering the endpoint's
404 / cooldown-skip / cooldown-elapsed / unauthenticated paths, and
extends test_celery_tasks.py with a token-mismatch test that
simulates a stolen lock mid-sweep and verifies the finally clause
leaves the new holder's lock alone.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: correct LISTEN-default claim in recheck-POST comment

Copilot flagged that the NOSONAR rationale in _trigger_asset_recheck
claimed "LISTEN defaults to the in-stack hostname" — that's wrong.
settings.py defaults LISTEN to 127.0.0.1; the in-stack hostname comes
from docker-compose.yml.tmpl and docker-compose.balena.yml.tmpl
explicitly setting LISTEN=anthias-server in the viewer container's
environment.

Comment-only fix. The actual behavior (request stays on the device,
plain HTTP is fine) is unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: address Copilot fourth-round review on PR #2805

Four substantive issues flagged on the cooldown / sweep semantics:

1. Endpoint cooldown gate based on Asset.last_reachability_check was
   racy: timestamp updates only after the task completes, so multiple
   near-simultaneous endpoint hits all read the same stale value and
   each enqueue a task. With Celery worker concurrency >1 the task's
   own cooldown check had the same race — workers each saw the stale
   timestamp and ran ffprobe concurrently for the same asset.

   Replace both with atomic Redis SETNX gates. Two separate keys with
   different TTLs and different jobs:

   - ``recheck:<id>:queue`` (TTL = ASSET_RECHECK_QUEUE_DEBOUNCE_S, 5s):
     endpoint queue debounce. Bounds queue churn from a viewer that
     rotates quickly past the same unreachable asset — only the first
     endpoint hit in the window queues a task.

   - ``recheck:<id>:lock`` (TTL = RECHECK_COOLDOWN_S, 60s): task-side
     cooldown gate. Prevents concurrent ffprobe / HEAD probes for the
     same asset across workers, including direct
     ``revalidate_asset_url.delay`` callers that bypass the endpoint.

   Two keys are necessary because a single shared key would conflict —
   the endpoint's lock would block the task it just enqueued from
   acquiring its own lock and the probe would never run.

2. revalidate_asset_urls (sweep) updated last_reachability_check on
   every iterated row, including skip_asset_check=True rows where no
   actual probe ran (the helper short-circuits to True). The API
   exposes that field as "last check"; writing it without a probe
   would advertise a check that never happened. Skip those rows
   entirely now — no probe, no timestamp update. is_reachable stays
   at its default (True), which matches what the viewer expects for
   skip_asset_check rows.

3. revalidate_asset_url (on-demand task) didn't mirror the sweep's
   is_enabled / is_processing / skip_asset_check filtering. An
   on-demand probe could run on a disabled or in-flight youtube_asset
   row and write state that's immediately moot. Added the same
   guards as the sweep, with early returns.

4. (Behavioral consequence of (1)) The task's per-asset SETNX lock is
   acquired regardless of caller, so direct ``.delay()`` callers
   (none currently exist in this codebase) get the same concurrency
   protection as endpoint-driven calls.

Tests rewritten / extended:
- Endpoint cooldown tests now drive the Redis queue-debounce key
  directly instead of fiddling with last_reachability_check, which is
  no longer the gate.
- A new test_back_to_back_calls_only_enqueue_once demonstrates the
  race that the previous timestamp check couldn't catch.
- New revalidate_asset_url tests for skip-disabled / skip-processing /
  skip-skip_asset_check / cooldown-lock-blocks-probe / lock-acquired.
- The sweep skip_asset_check test now also asserts last_reachability_check
  stays NULL.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* docs(v2): flag AssetRecheckViewV2 unauth as known, deferred to broader auth work

Copilot raised a real concern: AssetRecheckViewV2 is intentionally
unauth'd, which on a default LAN deploy lets any reachable client
fan out POSTs to trigger probe storms (DoS) or repeatedly trigger
url_fails/ffprobe against URLs in the asset DB if asset_ids leak.

Implementing the proper fix — an internal-auth token shared between
anthias-server and anthias-viewer — cuts across other inter-service
surfaces too (any future endpoint the viewer or celery worker calls
will face the same problem). Adding it just for this one endpoint
would be premature; the right shape is a single mechanism applied
across the relevant surfaces alongside the broader auth rework.

Per maintainer direction, flag this in the endpoint's docstring as a
known limitation rather than ship a half-fix here. The existing
mitigations (per-asset SETNX rate-limit, asset_id 404, no data in
the response, viewer is the only realistic caller) bound the blast
radius until the broader auth work lands.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: port revalidation tests to pytest, enhance Redis fake for SETNX

The merge with origin/master pulled in #2801's pytest migration, which
rewrote tests/test_celery_tasks.py and tests/test_viewer.py top-to-
bottom. The merge took master's pytest-style files as the base, losing
my unittest-style coverage of revalidate_asset_urls,
revalidate_asset_url, _asset_is_displayable, and _trigger_asset_recheck.
This re-adds them as pytest functions.

Two infrastructure changes were needed for the new tests to work
under the conftest's session-scoped fake Redis:

1. ``conftest.py:_make_fake_redis`` modelled ``r.set`` as a 2-arg
   lambda that ignored kwargs. My code uses
   ``r.set(key, value, nx=True, ex=...)`` for SETNX-based gates, which
   under the lambda either TypeErrors (extra kwargs) or returns None
   regardless of whether the key existed. Replace the lambda with a
   proper ``_set`` that honors ``nx`` semantics and returns
   ``True`` / ``None`` matching real Redis. Tests for the per-asset
   recheck cooldown, the sweep singleton lock, and the splash IP-
   refresh debounce all rely on this.

2. ``conftest.py`` had no ``r.eval`` mock. ``revalidate_asset_urls``
   uses an ``EVAL``-driven Lua compare-and-delete to release its
   singleton lock without clobbering a successor. Add an ``_eval``
   that recognizes the compare-and-delete shape and ignores any
   other script.

3. ``r.delete`` previously returned a list (``[store.pop(...) for k
   in keys]``); real Redis returns the number of keys deleted. Fix
   so callers that compare to an int (or expect a count) see correct
   semantics.

Tests added (all in pytest functional style with ``@pytest.mark.django_db``
where needed):

- ``revalidate_asset_urls``: marks unreachable / reachable; updates
  last_reachability_check; skips disabled / processing /
  skip_asset_check; local-file existence check; probe-exception
  containment; lock prevents overlap; lock-release does not clobber
  different holder; lock released on clean run.
- ``revalidate_asset_url``: no-op on missing asset_id; flips
  is_reachable; cooldown lock prevents back-to-back probes; acquires
  lock when running; skips disabled / processing / skip_asset_check;
  runs when no lock held.
- ``_asset_is_displayable``: skip_asset_check short-circuits;
  local-file existence; remote consults is_reachable; legacy default.
- ``_trigger_asset_recheck``: POSTs the right URL; no-op on missing
  id; swallows request errors.
- ``test_recheck_endpoint.py``: rewritten to pytest style. Same
  coverage (404 / no-lock-enqueue / debounce-held-skip / back-to-
  back-calls-once / unauthenticated).

All 52 tests pass under ``uv run pytest -m "not integration"``.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* ci: empty commit to retrigger workflows

* feat(asset): internal-auth header for recheck endpoint

Addresses Copilot's three new comments in one go.

1. AssetRecheckViewV2 was unauth'd with a "known limitation, deferred"
   note. With the rest of the work shaping up, the right fix is in
   reach: a shared-secret header that both anthias-server and
   anthias-viewer derive from anthias.conf's django_secret_key. Both
   containers read that conf, so no env-var distribution mechanism is
   needed; the operator can also override via ANTHIAS_INTERNAL_TOKEN
   if they want a pure env-managed token.

   New module ``lib/internal_auth.py`` exposes:

     - INTERNAL_AUTH_HEADER ('X-Anthias-Internal-Token')
     - internal_auth_token(settings) -> HMAC(secret, fixed-context, sha256)
     - is_internal_request(request, settings) -> constant-time compare

   Endpoint gates with ``is_internal_request``; missing/wrong header
   returns 403. Viewer's ``_trigger_asset_recheck`` derives the same
   token and sends it as a header. If the token can't be derived
   (e.g., dev environment with no secret_key), the viewer skips the
   request rather than POSTing a guaranteed-403.

2. ``revalidate_asset_url`` previously acquired the per-asset
   cooldown lock *before* checking whether the asset existed or was
   eligible (is_enabled / is_processing / skip_asset_check). For an
   ineligible asset that's a SETNX of the lock followed by an early
   return that holds the lock for RECHECK_COOLDOWN_S, suppressing
   legitimate rechecks during that window. Move the lock acquisition
   below the eligibility checks so we only hold it when we're going
   to actually probe.

3. ``_trigger_asset_recheck`` was called for every skipped asset
   including local-file URIs where ``_asset_is_displayable`` had
   already determined the file is missing. The server-side recheck
   would just confirm the same answer (the celery worker shares
   assetdir with the viewer). Skip the recheck for local URIs;
   factor the local-file detection into ``_asset_is_local_file`` so
   ``_asset_is_displayable`` and ``asset_loop`` share the predicate.

Tests updated end-to-end:
- New test_recheck_endpoint cases: missing auth → 403, internal-auth
  token works without operator BasicAuth.
- All existing recheck-endpoint tests pass through the same auth
  shim via a ``_auth_headers()`` helper using ``headers=`` kwarg.
- New viewer test: trigger no-ops cleanly when secret is missing.
- New revalidate_asset_url assertions verify the cooldown lock is
  NOT acquired in any of the early-return branches (missing asset /
  disabled / processing / skip_asset_check).

All 56 tests pass under ``uv run pytest -m "not integration"``;
ruff + mypy clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: stop relying on settings-dict patching in viewer recheck tests

CI failed two viewer tests under pytest-xdist + the Docker test image:
``test_trigger_recheck_posts_to_recheck_endpoint`` showed an
HMAC-token mismatch between the test's expected and the viewer's
sent header, and ``test_trigger_recheck_no_op_when_internal_token_missing``
saw the viewer post when it should have skipped. Both pass locally.

The shared cause is fragility in patching
``settings['django_secret_key']`` as a way to control the derived
internal token. ``settings`` is a UserDict, ``mock.patch.dict``
should work, and locally it does. But under the CI environment
something — possibly a parallel worker reading settings while the
patch is in flight, possibly a divergent ``HOME`` / on-disk conf —
breaks the equality. Spending more cycles isolating the exact cause
isn't worth it: the viewer test isn't trying to verify the HMAC
derivation, only the viewer's behavior given a token.

Mock ``viewer.internal_auth_token`` directly and assert the viewer
forwards whatever it returns. The HMAC implementation continues to
be exercised by ``tests/test_recheck_endpoint`` end-to-end, where
both producer and consumer share one settings instance for the
duration of one test function.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* ci: build both test services through compose so ghcr cache hits both

The previous design had ``anthias-test`` carry the ``build:`` block
and ``anthias-celery`` inherit by image tag alone. That left
``anthias-celery`` going through compose's pull-then-fall-back path
on the local-only ``anthias-test:dev`` tag, producing a noisy
``pull access denied for anthias-test`` line in CI logs.

Give both services the same build block via a YAML anchor, and the
same image tag. Compose deduplicates the build by image tag (image
is produced once), but routing both services through ``build:`` means
both are eligible for the ghcr layer cache that
``tools.image_builder`` + the CI buildx invocation set up — and
neither service ever attempts a Docker Hub pull for a local-only
tag.

Replaces the earlier ``pull_policy: never`` workaround on
anthias-celery (commit b18fba4): ``build:`` is the right primitive,
not pull-policy gymnastics.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>

…de (#2806)

* refactor(splash): poll IPs from client instead of rendering server-side

The splash page used to call get_node_ip() synchronously in the view
and render a static IP list. Two problems with that:

1. The view passed get_node_ip()'s return value to
   ipaddress.ip_address() unconditionally. On a fresh Balena boot the
   supervisor API can be slow to respond, in which case get_node_ip()
   returns the literal string 'Unknown' — and
   ipaddress.ip_address('Unknown') raises ValueError, which 500s the
   whole splash render. The viewer's webview then displays whatever
   Qt does with a 5xx page (blank/error), not "Unknown" as the prior
   structure suggested.

2. The render is a static snapshot. The splash is on screen for ~60s
   thanks to viewer/__init__.py's SPLASH_DELAY, but the rendered IPs
   reflect only what the host bus answered at second 0. If the
   supervisor recovers at second 8 — or if IPs change due to a DHCP
   renewal mid-splash — the page can't update.

This change makes the splash render immediately with no IPs and
populates them client-side by polling a new lightweight endpoint:

  - GET /api/v2/network/ip-addresses returns
    {"ip_addresses": ["http://192.168.1.42", ...]}.
  - Unauth'd because the splash itself is unauth'd and the data is
    already disclosed by the splash render — no new exposure. Test
    pinned to make a future flip to @Authorized fail fast.
  - Narrow on purpose: just IPs, no diagnostics. /api/v2/info covers
    the heavy "everything about the device" case but is auth'd and
    pulls psutil/statvfs/version-checks that would compound on a
    2-second poll.

The polling JS lives in templates/splash-page.html (~50 lines of
vanilla JS, no framework, no build step). It polls every 2s for the
first ~30s and then backs off to 5s, so a long-tail recovery during
the splash's display window still gets caught without the page
thrashing the host bus when nothing's changing.

A new module-level helper _safe_ip_addresses() in api/views/v2.py
owns the get_node_ip() -> formatted-list conversion, with the
ValueError bug fixed at source: 'Unknown' and 'Unable to retrieve
IP.' both map to []. InfoViewV2.get_ip_addresses() is refactored to
delegate, so /api/v2/info gets the same robustness.

Behavior change for /api/v2/info (worth flagging for reviewers): if
get_node_ip() returned something malformed before, the endpoint would
have 500'd; it now returns ip_addresses=[] and 200.

The latent fragility of get_node_ip()'s Balena single-shot lookup
itself is left for a separate change — anything that calls it
directly (CLI tools, future endpoints) would still see 'Unknown' on a
slow first boot. The right fix there is symmetrizing the Balena
branch to retry the way the non-Balena branch already does. Out of
scope here: the splash specifically needed structural change, not
just a longer retry, because static-snapshot rendering doesn't make
use of the 60-second display window even if get_node_ip() always
returned in time.

Refs #2803 (Part B follow-up).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: format tests/test_splash_page.py

CI's `ruff format --check` (separate from `ruff check`) wanted shorter
lines refolded onto single lines.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: quiet SonarCloud quality gate (NOSONAR on intentional http://)

Anthias serves the admin UI on plain HTTP per CLAUDE.md (TLS is opt-
in via the Caddy sidecar). The splash page surfaces device URLs so
the operator can click into that UI, so _safe_ip_addresses() must
emit http:// URLs to match how the device actually listens —
emitting https:// would point at a port that isn't bound on a
default install. Add NOSONAR markers on the two emit lines (S5332)
with rationale, mirroring the existing NOSONAR pattern in
host_agent.py.

In tests/test_splash_page.py, hoist the IP literals to module-level
fixture constants with NOSONAR markers (S1313, "hardcoded IP").
Downstream f-strings reference the constants, which centralizes the
suppression and avoids scattering NOSONAR comments through every
assertion.

No production code paths affected.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: centralize 'http://' literal under one NOSONAR

The previous fixup put NOSONAR markers on the IP-fixture constants
but left the http:// scheme inline in the f-strings, which Sonar
still flagged as S5332 at each call site. Hoist the prefix to a
single _HTTP constant with the NOSONAR + rationale, and reference
it from the f-strings. Same output, no production impact.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: address Copilot review on PR #2806

Two issues flagged on the original PR.

1. _safe_ip_addresses() called lib.utils.get_node_ip(), which is
   anything but lightweight on non-Balena: it publishes
   'set_ip_addresses' to host_agent and waits up to 60s for
   host_agent_ready plus 20s on ip_addresses_ready before returning.
   Inside a 2-second polling endpoint, a single slow first call would
   tail-back every subsequent poll, and the splash would never
   populate. Defeats the entire point of the redesign.

   Split the resolver into two pieces:

   - _format_ip_urls(node_ip): pure formatter, shared by the polling
     endpoint and /api/v2/info. No I/O, tolerates 'Unknown' /
     'Unable to retrieve IP.' sentinels by returning [].

   - _resolve_node_ip(): fast-path resolver for the polling endpoint
     only. On bare metal, reads the cached 'ip_addresses' Redis key
     directly (host_agent populates it) and fires a fire-and-forget
     'hostcmd' publish on cache miss so the next poll finds something.
     On Balena, calls the supervisor with a 1.5s HTTP timeout —
     supervisor is single-source-of-truth there and there's no cache
     to fall back on, so a tight bound is the best we can do. Caps
     well under the JS poll cadence (2s) so request workers stay free.

   InfoViewV2.get_ip_addresses() now calls _format_ip_urls(get_node_ip())
   directly, preserving the existing blocking behavior since /api/v2/info
   is auth'd and not polled.

2. The JS polling loop never stopped — just backed off to 5s forever.
   In the actual viewer, the splash is destroyed after ~60s so polling
   ends naturally. But if the page is opened by anything else (curl,
   dev tools, an idle browser tab), it would generate forever-requests
   in the background.

   Add a wall-clock cap of 120s — double the in-viewer display window,
   generous enough for slow first-boot recoveries, bounded enough that
   an idle tab doesn't accumulate load. Whatever was last rendered
   stays on screen after the cap.

Tests rewritten to mock at the right boundary: _format_ip_urls is
tested directly with strings (no I/O), _resolve_node_ip is tested
with mocked Redis / requests.get, and the endpoint test exercises the
full chain.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: address Copilot follow-up review on PR #2806

Two more issues flagged after the first round of fixups:

1. The bare ``except Exception`` around ``r.publish('hostcmd', ...)``
   in ``_resolve_node_ip()`` would swallow any error, including
   programming mistakes (e.g. ``r`` accidentally None after a future
   refactor) — turning "splash IPs never populate" into a silent
   failure with no breadcrumb. Narrow to ``redis.RedisError`` so we
   only swallow what we actually want to swallow (transient broker
   issues), and let real bugs surface as 500s where they can be
   diagnosed. Added ``RedisError`` to the project's redis stub.

2. The previous test patched ``api.views.v2.get_node_ip`` to assert
   that the splash view doesn't call it — but the splash view lives
   in ``anthias_app.views``, and that import path doesn't reach
   anything in api.views.v2. The patch wasn't asserting anything
   useful: the test would pass regardless of what splash_page did.

   Replace with two more honest tests:

   - test_renders_200_without_mocking_anything: render must succeed
     with no fixtures or mocks. The original "Unknown" → ValueError
     → 500 regression now requires zero scaffolding to catch.

   - test_splash_view_does_not_import_get_node_ip: assert at the
     module-attribute level that ``anthias_app.views`` doesn't carry
     ``get_node_ip`` in its namespace. Fails fast if a future refactor
     re-adds the import (which is what would re-introduce the
     synchronous IP work we just removed).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: address Copilot third-round review on PR #2806

Two issues flagged after the second round of fixups:

1. _resolve_node_ip()'s r.get('ip_addresses') had no error handling.
   If Redis is flaking on the device (early boot, transient broker
   hiccup), the splash polling endpoint would 500 — which is exactly
   the kind of fragility this whole redesign was supposed to remove.
   Wrap r.get in a redis.RedisError catch and treat it as a cache
   miss: return ''. The JS keeps polling at 2s intervals and recovers
   as soon as Redis comes back.

2. The cache-miss path published 'set_ip_addresses' on every poll.
   host_agent's set_ip_addresses runs an internet probe with a 10x1s
   tenacity retry, so worst-case it's ~10s of work. At a 2s poll
   cadence, we'd queue ~5 redundant refresh requests before the
   first one finishes — keeping host_agent busy for far longer than
   necessary on a slow first boot.

   Add a SETNX-with-TTL debounce key (_IP_REFRESH_PENDING_KEY,
   12s TTL — covers worst-case host_agent latency with margin). Only
   the first cache-miss poll in the window publishes; later polls
   within the TTL no-op. A future poll past the TTL retries naturally
   if the cache is still empty.

   On publish failure (Redis flake mid-call), the debounce key is
   actively cleared so the next poll can retry — otherwise we'd be
   pinned out of refreshing for the whole TTL after a transient
   error that didn't actually queue a refresh.

Tests cover all four behaviors:
- r.get RedisError returns ''
- Three back-to-back cache-miss polls publish only once (debounce
  holds across calls within the window)
- A failed publish leaves the debounce key cleared so the next poll
  retries
- Existing setUp/tearDown also clear the debounce key between tests
  so they don't interfere with each other

Also adds `delete` to the redis stub (already present on PR #2805's
branch; needed here for the test's debounce-key cleanup).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: address Copilot fourth-round review on PR #2806

Two issues flagged after the third round of fixups:

1. _resolve_node_ip()'s Balena branch built the supervisor URL inline
   with its own auth + headers, duplicating
   lib.utils.get_balena_supervisor_api_response() and
   get_balena_device_info(). The two would drift over time —
   different timeout / error handling on each side already.

   Extend the shared helper to accept a timeout (and, via **kwargs,
   any other ``requests`` parameter). Existing callers that don't
   pass extras are unaffected — the request still goes out with the
   long-standing unbounded behavior. Then have v2.py call
   ``get_balena_device_info(timeout=_BALENA_SUPERVISOR_TIMEOUT_S)``
   so URL construction lives in one place.

2. _resolve_node_ip()'s SETNX-debounce write wasn't wrapped in a
   redis.RedisError handler. The earlier fixup wrapped both r.get()
   and r.publish(), but a Redis flake between those two calls (right
   on the SETNX) would still 500 the splash poll. Wrap the SETNX
   too; on failure, fall through to '' just like the other Redis
   failure paths.

Tests updated:
- Existing Balena-supervisor tests now patch the shared helper
  (api.views.v2.get_balena_device_info) instead of mocking
  api.views.v2.requests.get directly. They still assert the timeout
  is bounded, which is the load-bearing property.
- New test_setnx_failure_returns_empty pins the SETNX-flake path.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: address Copilot fifth-round review on PR #2806

Two issues flagged after the previous round:

1. Hardcoded URL in templates/splash-page.html. Other templates
   (e.g. login.html) use Django's ``{% url %}`` tag so a future API
   mount-path or version-prefix change doesn't silently break the
   page. Resolve via ``{% url 'api:network_ip_addresses_v2' %}`` and
   inject the resolved path into the JS as a string.

   The existing test_renders_with_polling_script assertion still
   holds — it checks for the literal ``/api/v2/network/ip-addresses``
   in the response body, which is what the URL tag resolves to.

2. ``_resolve_node_ip()`` treated a cached value of ``'[]'`` (no IPs
   found) as a cache hit. ``json.loads('[]')`` returns ``[]``,
   ``' '.join([])`` returns ``''``, and the resolver short-circuited
   without publishing a refresh. host_agent's first run on a
   still-coming-up network produces exactly that ``'[]'`` write —
   so the splash would never recover when networking came online
   during the display window. Every poll past that first write
   would short-circuit on the empty cached value.

   Fix: only treat a non-empty decoded list as a hit. Empty list
   (and malformed JSON) fall through to the debounced refresh
   publish path so host_agent gets nudged to retry.

Tests:
- New test_empty_list_in_cache_triggers_refresh pins the fix:
  cached ``'[]'`` must publish a refresh, not silently no-op.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: avoid bare {% url %} in JS comment that Django parses

The previous fixup added a JS comment that referenced ``{% url %}``
literally to explain what the tag does. Django's template engine
doesn't know JS comments aren't templated — it scanned the comment,
saw a ``{% url %}`` block with no arguments, and raised
``'url' takes at least one argument, a URL pattern name`` at parse
time, which broke /splash-page rendering and the
test_renders_with_polling_script test.

Rephrase the comment to refer to "the reverser" instead of the tag
syntax. The actual ``{% url 'api:network_ip_addresses_v2' %}`` call
is unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: kick off debounced refresh on cache hits too

Both Copilot comments flagged the same gap. The splash docstring
claims the page "updates if IPs change during the splash's display
window" (e.g. DHCP renewal mid-splash), but on bare-metal that's
only true at the moment Redis is empty. After the first successful
host_agent run populates the cache, ``_resolve_node_ip()`` returns
the cached value immediately and never publishes another
``set_ip_addresses`` — so the cached IPs would freeze for the rest
of the splash window even if the underlying network state changed.

Make the behavior match the docstring: extract the SETNX-debounced
publish into a ``_publish_refresh`` helper and call it from both the
cache-hit and cache-miss paths. The cache-hit path returns the
cached value immediately (no blocking on the publish) and the
refresh runs in the background so subsequent polls see the updated
data once host_agent re-populates Redis. The same SETNX TTL caps the
publish frequency at once per ``_IP_REFRESH_DEBOUNCE_S`` (12s) so a
tight poll loop can't queue redundant refresh requests.

Tests:
- The previous ``test_reads_from_redis_cache`` asserted the cache
  hit did NOT publish — drop that assertion (it pinned the wrong
  behavior). Renamed the surviving form to just verify the cached
  value is returned.
- New ``test_cache_hit_also_kicks_off_refresh`` pins the new
  behavior: cache hit returns immediately AND fires the debounced
  publish.
- Existing debounce tests keep working because the SETNX gate moved
  into the helper but the semantics didn't change.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: port splash tests to pytest, enhance Redis fake for SETNX

The merge with origin/master pulled in #2801's pytest migration. The
splash tests in tests/test_splash_page.py were still unittest-style
TestCase classes (added on this branch after the migration target);
port them to pytest functions to match the rest of the suite.

Also enhance conftest.py's Redis fake (same enhancement as #2805):

- Replace the 2-arg ``r.set`` lambda with a ``_set`` that honors
  ``nx``/``ex`` kwargs and returns ``True`` / ``None`` matching real
  Redis semantics. ``_resolve_node_ip``'s SETNX-based debounce gate
  relies on this — without it, every "is the publish window open?"
  check would spuriously believe the gate was free.
- Fix ``r.delete`` to return an int (count of keys deleted) instead
  of a list of popped values.
- Add ``r.eval`` (no-op for unknown scripts; nothing on this branch
  uses it, but #2805 does and the conftest is shared).

Tests rewritten as pytest functions with ``@pytest.mark.django_db``
where needed and a ``bare_metal_no_pending`` fixture that clears the
per-test debounce key. Coverage unchanged: format_ip_urls (5
formatting + sentinel cases), bare-metal resolver (cache hit, cache
hit + refresh, cache miss + publish, malformed cache, empty-list
cache, Redis errors on get/set/publish, debounce), Balena resolver
(supervisor success / timeout / error status), endpoint full chain
(200 with IPs, 200 + [] on cache miss, unauthenticated), splash view
(renders without mocking, doesn't import get_node_ip, polling
script present).

All 45 tests pass under ``uv run pytest -m "not integration"``.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: address Copilot seventh-round review on PR #2806

Three issues:

1. ``json.loads(raw)`` returning a non-list (e.g. corrupted cache, a
   different producer writing the same key) would fall through to
   ``' '.join(ips)`` — which crashes on int/dict and silently joins
   characters on a string. Validate ``ips`` is a non-empty list of
   strings before joining; otherwise treat as a malformed cache and
   fall through to refresh.

2. ``test_resolve_publish_failure_releases_debounce`` patched
   ``r.get`` to always return None and then asserted
   ``r.get(_IP_REFRESH_PENDING_KEY) is None`` — that post-condition
   assertion was made after the patch had been reverted (so it did
   call into the real fake), but the structure was confusing enough
   that Copilot read it as vacuous. Rewrite to assert directly on
   ``r.delete``: wrap the delete with ``mock.patch.object(...,
   wraps=...)`` and verify it was called once with the debounce key.
   The publish call is also asserted as called-once for completeness.

3. ``NetworkIpAddressesViewV2`` is unauth'd and triggers a side
   effect (``_publish_refresh`` → ``hostcmd:set_ip_addresses`` →
   host_agent's internet probe). Flag this as a known limitation in
   the docstring, mirroring the framing on AssetRecheckViewV2 in
   #2805 — the real fix is a shared internal-auth mechanism that
   cuts across both endpoints, designed alongside the broader auth
   rework. Existing mitigations (SETNX debounce, no new data
   disclosure, host_agent's own retry/throttle) bound blast radius
   in the meantime.

All 24 splash tests pass under ``uv run pytest -m 'not integration'``;
ruff + mypy clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* ci: empty commit to retrigger workflows

* fixup: correct stale docstring on debounce-release test

The docstring on test_resolve_publish_failure_releases_debounce
described an earlier (vacuous) form of the test — asserting via
``in_dict_after`` against the underlying fake store. The current
test asserts on ``r.delete`` calls via ``wraps=`` instead, which
is what the docstring should describe.

Updates the docstring to match the assertions actually performed
and to explain why ``wraps=`` is needed here (``r.get`` is patched
inside the block to force the cache-miss path, so reading the key
back after the block would pass for the wrong reason).

No production-code change.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fixup: fall back to MY_IP env var on bare-metal cache miss

``lib.utils.get_node_ip()`` falls back to ``getenv('MY_IP')`` when
the Redis ``ip_addresses`` key is empty/unset. ``MY_IP`` is set by
``bin/upgrade_containers.sh`` (the host's outbound IP) and exported
into the server container by ``docker-compose.yml.tmpl``.

The polling resolver added in this PR didn't carry that fallback —
so any setup where ``host_agent`` isn't populating Redis (custom
deploys, late-starting host_agent, crashed host_agent) would freeze
the splash on "Detecting network…" forever. Mirror the fallback
here so the splash can show *something* useful in those cases.

The Redis-error early-return at the top of the function is folded
into the same fallback path: rather than 500-ing out before we can
even consult ``MY_IP``, treat a Redis read failure as a cache miss
so the env-var fallback still applies.

Tests: clear ``MY_IP`` in the ``bare_metal_no_pending`` fixture to
keep the existing assertions deterministic on developer shells /
runners that may export it; add three new tests covering the
cache-miss, Redis-down, and unset-env paths.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>