You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
feat: keep ha_manage_radio reads available in read-only mode (#1699)
* feat: keep ha_manage_radio reads available in read-only mode
ha_manage_radio is a mixed read/write tool (reads: diagnostics, network_status,
ping; writes: commission/add, remove, reinterview, firmware, fabric/credential/
channel/network changes) but it was not in READ_ONLY_EXEMPT_TOOLS. So read-only
mode treated it as a plain write tool: hidden from the catalog and every call
blocked — including the reads. Its 'ping' active probe has no pure-read
duplicate elsewhere, so it became unreachable in read-only mode entirely.
Add a _radio_write predicate that allows diagnostics/network_status/ping and
blocks every other action, and register ha_manage_radio in the exempt table —
matching ha_manage_energy_prefs / ha_manage_pipeline. Writes now return the
structured READ_ONLY_MODE error before the handler runs; reads stay callable.
Tests: new parametrized test_manage_radio + all four schema-drift manifests
(exempt set, module map, inspected args, gated/read partition); the e2e
read-only suite gains ha_manage_radio in the still-listed set plus
read-allowed / write-blocked cases. The settings-UI exempt-list test compares
against the live constant, so it adapts automatically.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(radio): allow cluster_read + list_datasets reads in read-only mode
The exemption predicate only allowed diagnostics/network_status/ping, but
zigbee cluster_read (zha/devices/clusters/attributes/value) and thread
list_datasets (thread/list_datasets) are pure non-mutating reads with no
pure-read duplicate elsewhere in the catalog — so read-only mode made them
unreachable, the exact harm the exemption exists to prevent. Add both to the
allow-list and document why the two read-ish-but-not actions stay blocked:
zigbee network_backup creates a backup artifact + key material (mirrors
ha_manage_backup's blocked snapshot create) and thread discover_routers kicks
off a long-running mDNS scan. Unit test gains the two new read cases plus the
two intentionally-blocked cases.
Found by the pr-review-toolkit code-reviewer + pr-test-analyzer, verified
against source.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(e2e): fail-fast on error-spam + cover read-only on the inaddon backend
Two e2e-infra improvements folded into the read-only PR (both hit while working
its CI):
1. Fail-fast on a doomed run. A Supervisor add-on-update flake made all 997
inaddon tests ERROR at setup (0 passed/failed) yet the run ground on 11m39s.
Add a pytest_runtest_logreport hook in the SHARED e2e conftest (covers every
e2e lane) that aborts via pytest.exit after 50 consecutive setup/teardown
errors with zero call-phase pass/fail in between. Resets on any real pass or
fail, so genuine failures still run through in full; this is NOT --maxfail.
Validated under -n2 xdist: aborts at 50/150 in ~2s; a healthy run with real
failures does not trip it.
2. Read-only coverage on the inaddon backend. The read-only e2e suite skipped
inaddon (it only knows how to inject READ_ONLY_MODE into a fresh in-process
server). Add test_inaddon_read_only_mode_blocks_radio_writes: enable
read_only_mode via the add-on's own settings API (merges into the Supervisor
options the production way), self-restart ONLY the add-on, then assert
ha_manage_radio reads work and writes return READ_ONLY_MODE. Marked run_last
(new marker + ordering in pytest_collection_modifyitems) so it is the last
scope dispatched; each xdist worker owns an isolated add-on, so leaving
read-only on cannot affect another test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(e2e): bump container/haos skip ceilings for the inaddon read-only test
test_inaddon_read_only_mode_blocks_radio_writes is @inaddon_only, so it skips on
the container and external-haos lanes — pushing the container skip count to 66
(ceiling was 65). Bump container 65->66 and haos 32->33; haos_inaddon is
unchanged since the test runs there.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(test): make inaddon read-only test self-contained (restore read-only off)
run_last did not work: --dist loadscope groups tests by module, so moving one
item to the end can't make the test the last thing on its xdist worker. The test
enabled read_only_mode on the shared add-on and left it on, cascading
READ_ONLY_MODE into ~298 later write tests on the same worker.
Make it self-contained instead: enable read-only + restart, verify ha_manage_radio
read works / write blocked, then in a finally restore read-only OFF + restart and
poll ha_get_overview until it is confirmed off before returning. xdist runs a
worker's tests serially and each worker owns an isolated add-on, so bracketing
read-only around this one test is safe at any position. Drop the run_last marker
+ the (ineffective) ordering hook.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(test): address pr-review-toolkit round-2 findings on #1699
Four confirmed findings from the customized toolkit re-run:
1. (important) The inaddon read-only test self-restarts the dev add-on, which
drops the SHARED session mcp_client connection (test_supervisor_inaddon.py
documents this kills mcp_client for later tests on the worker). Add mcp_client
to the test and warm it back up in the finally so the next module loadscope
schedules on this worker gets a live session, not a stale one.
2. The fail-fast hook had no unit coverage. Extract the streak logic into
tests/src/doomed_run.py::DoomedRunDetector and unit-test it in
tests/src/unit/test_doomed_run.py (abort threshold, reset-on-real-pass/fail,
rerun/skip exclusions).
3. The fail-fast comment claimed controller-only / global-across-workers;
pytest_runtest_logreport fires per-process under xdist, each with its own
detector. Correct the comment (the abort still fires from whichever process
hits the threshold first).
4. _await_read_only confirmed the OFF state via the ABSENCE of read_only_mode in
ha_get_overview, which a degraded payload could also fake. Switch to a
positive catalog signal from a real list_tools response (ha_call_service is
hidden iff read-only is on).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: kingpanther13 <kingpanther13@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
0 commit comments