From de1c34777db0631ac663c9f59731ba985ac4a4d2 Mon Sep 17 00:00:00 2001 From: James Mackay Date: Mon, 12 Oct 2026 13:32:34 +0100 Subject: [PATCH] fix(adb): ADB-off is asked by every device path, not inferred from a missing poller (t23) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The t22 review's O1, and the same bug class as t21's F1 — so it gets the same structural treatment instead of a note. WHAT WAS MEASURED. With a poller injected into an entry whose own option said `adb_enabled: False`, the installed-apps sweep sent 4 real shells (the reviewer; 3 on this test's double, which yields one launchable package) and a label probe sent 1 more. Both gated on `self._poller is None` — and the poller's absence is a property of the SETUP code (`_build_poller_factory` returns None when ADB is off), which is precisely the indirect guarantee the t21 repair had already shown to be insufficient. Production was unreachable only through that setup path. THE FIX. `_adb_disabled()` is now asked directly by every path in the manager that can reach the device, and `_adb_active()` (option ON *and* a poller) is the shared predicate for the ones that also need a transport. The `available` property deliberately keeps its transport meaning (diagnostics reads it as "ADB is configured"); a new docstring says so, because overloading it is how the gate got lost in the first place. Gated paths, and what each asks now: * intent scan — the option (t21, unchanged) AND the private shell site itself * installed-apps sweep — the option first, then poller+connected * installed-apps refresh (public entry) — returns the last table, starts nothing * app-label lookup / drain / the ONE probe site (`_dump_label`) — the option; a lookup returns None and queues NOTHING (no request, no task, no cache write) * ADB text entry, media keys, bring-to-foreground, digit and enter keys — `_adb_active()` * the heartbeat `_refresh` — the option: no wake probe, no media dump, no running-apps poll, no foreground fallback, no label drain, no logcat ensure * the watchdog — the option BEFORE the poller factory, so not even a factory can build a transport; and `_watchdog_cycle` refuses a direct call too (a TCP connect is device traffic) * the logcat stream — the option at the ensure gate, the option in `_open_logcat_stream`, and the ladder exits WITHOUT counting a failure (a local option must not auto-disable the option for the session) * the public `adb_poller` accessor — None while the option is off, which closes the shell paths that live OUTSIDE this module (the `adb_command` service, the screen-capture image and the intent shell) without reaching into them: all three already refuse on None with their own message * `test_connect` (the pairing probe) stays ungated on purpose: it runs BEFORE an entry exists, and the setup menu only offers that step when the ADB option is on (config_flow.py). `async_stop`/`_note_failure` disconnect calls are teardown, not traffic. PROOF, BY TRAFFIC. tests/test_adb_off_gates_every_device_path.py drives every audited path — as a parametrized AUDIT table plus one full cycle on one manager — with `adb_enabled: False` and a double that WOULD answer everything, and asserts the poller's complete traffic list (shells, polls, commands, connects) is EMPTY, plus that nothing was cached (no intent answer, no app table, no stamp, no queue, no store field, no wake reading). It also proves the watchdog never builds a poller and never connects, that a direct cycle does not connect either, that the option is a CHECK and not a latch (the same manager resumes the sweep, the scan, the label queue, the media key, the digit key and the accessor when the option flips back on), and that ADB-off and no-poller stay told apart. PRE-FIX PROOF: the same file against `git archive HEAD` FAILS 16 of 23 tests, and the failures print the measured traffic — the sweep's 3 shells, the label probe's `pm dump … | grep …`, `input text hello`, `media:play`, `foreground:`, `digit:5`, `enter`, and 4 polls per heartbeat cycle. HARNESS TRUTHFULNESS. The suites' managers were built with `entry=None`, which means ADB is OFF (fail closed, matching `__init__.py`). Those harnesses now pass a realistic entry whose option says ADB is on — they model production instead of the one combination the gates block. No production behaviour changed for an ADB-on entry: 923 passed / 5 skipped, up from 900 by this file. Floors re-derived from the new walk (1661 blocks → 1627, same 2% band): raised, because that is what the re-measurement says. Drift exit 0. Strings files byte-identical. NOT VERIFIED HERE: the device side is a scripted double, so what is proven is that NOTHING WAS SENT, not what firmware would do; no real Shield was involved. --- custom_components/nvidia_shield/connection.py | 151 ++++++- tests/ha_api_drift.py | 8 +- tests/test_adb_off_gates_every_device_path.py | 412 ++++++++++++++++++ tests/test_command_routing.py | 23 +- tests/test_command_scripts.py | 4 +- tests/test_connection.py | 41 +- tests/test_installed_apps.py | 24 +- tests/test_protocol_transport.py | 25 +- 8 files changed, 651 insertions(+), 37 deletions(-) create mode 100644 tests/test_adb_off_gates_every_device_path.py diff --git a/custom_components/nvidia_shield/connection.py b/custom_components/nvidia_shield/connection.py index 7381dcc..4a3ab1a 100644 --- a/custom_components/nvidia_shield/connection.py +++ b/custom_components/nvidia_shield/connection.py @@ -617,7 +617,13 @@ class ShieldConnectionManager: @property def available(self) -> bool: - """True when ADB is active for this entry (a poller exists).""" + """True when a poller exists for this entry — a TRANSPORT fact (t23/O1). + + Deliberately NOT the permission to talk to the device: the option is (see + :meth:`_adb_active`). Diagnostics reads this as "ADB is configured", and the + t22 review's scenario — a poller injected into an entry whose own option + says ADB is off — must not be able to turn it into traffic. + """ return self._poller is not None @property @@ -804,8 +810,21 @@ class ShieldConnectionManager: @property def adb_poller(self): - """The ADB poller (or None) — public accessor replacing the - ``_poller`` reach-in in the adb_command service handler (F6).""" + """The ADB poller this entry MAY use, or None — the public accessor (F6). + + Read by the ``adb_command`` service, the screen-capture image and the + intent shell instead of reaching into ``_poller``. It answers the same + question as every other device path in this class: may this entry talk to + the device? So it is None while the entry's option says ADB is off (t23/O1) + — and those callers already treat None as "ADB is not available" and refuse + with their own message, which is what closes the shell paths that live + OUTSIDE this module without this module having to reach into them. + + The private ``_poller`` stays the transport object for internal bookkeeping + (connect/disconnect/state), which is not traffic the user asked for. + """ + if self._adb_disabled(): + return None return self._poller @property @@ -937,7 +956,8 @@ class ShieldConnectionManager: import shlex poller = self._poller - if poller is None or not self.connected: + # t23/O1: the OPTION gates the shell, not the poller's existence. + if poller is None or not self.connected or self._adb_disabled(): return False try: result = await poller.shell(f"input text {shlex.quote(text)}") @@ -982,6 +1002,17 @@ class ShieldConnectionManager: # so there is no "unresolved" shape to fall through on. return label or None self._app_labels.pop(package, None) # stale — refetch on demand + if self._adb_disabled(): + # t23/O1: ADB off means there is nothing to wait for — no request (so no + # task and no shell later) and no cache write. The name falls back to the + # user map / curated map / package id exactly as a failed fetch does, and + # because nothing was cached, the next render after ADB is turned back on + # asks again. + _LOGGER.debug( + "app label for %s not queued (ADB is switched off for this entry)", + package, + ) + return None if package not in self._app_label_requests and ( package not in self._app_label_inflight ): @@ -1169,10 +1200,12 @@ class ShieldConnectionManager: package, ) poller = self._poller - if poller is None or not self.connected: + # t23/O1: the OPTION is asked at the ONE place a label dump is issued (the + # sweep's fan-out comes through here too), so this gate covers both paths. + if poller is None or not self.connected or self._adb_disabled(): _LOGGER.debug( - "app label for %s not fetched (ADB not connected) — will " - "retry on the next render", + "app label for %s not fetched (ADB not connected or switched off) " + "— will retry on the next render", package, ) self._app_label_failed.add(package) @@ -1275,6 +1308,12 @@ class ShieldConnectionManager: material with the poller, never the poller's socket. """ poller = self._poller + if self._adb_disabled(): + # t23/O1: ADB off is not a poller problem, and this stream opens its OWN + # connection to the device — the option gates it like every other path. + raise RuntimeError( + "ADB is switched off for this entry — logcat stream cannot start" + ) if poller is None: raise RuntimeError("poller gone — logcat stream cannot start") @@ -1362,6 +1401,11 @@ class ShieldConnectionManager: """ delay = ADB_RECONNECT_BACKOFF_START while not self._stopping and not self._logcat_disabled: + if not self._adb_active(): + # t23/O1: ADB off ends the stream WITHOUT counting a failure — the + # device is not at fault for a local option, and five of those would + # auto-disable the stream for the session. + break try: await self._logcat_pass() except asyncio.CancelledError: @@ -1378,7 +1422,9 @@ class ShieldConnectionManager: """Start the consumer when the option is on, connected, enabled.""" if self._logcat_disabled or not self._entry_options_logcat(): return - if self._poller is None or not self.connected: + # t23/O1: option AND transport — a poller that exists while the option says + # ADB is off must not start a device stream. + if not self._adb_active() or not self.connected: return if self._logcat_task is None or self._logcat_task.done(): self._logcat_task = self._create_task( @@ -1480,6 +1526,14 @@ class ShieldConnectionManager: ``APP_SWEEP_RETRY_SECONDS`` so a silent socket is not hammered; a sweep that RAN stores its answer, including a genuinely empty list. """ + if self._adb_disabled(): + # t23/O1: no sweep is STARTED, and not even the retry-spacing bookkeeping + # below runs — nothing was attempted, so nothing is owed a retry. The last + # known table is returned as-is (empty when none was ever built). + _LOGGER.debug( + "installed-app refresh skipped (ADB is switched off for this entry)" + ) + return self.installed_apps() if not force and not self.installed_apps_stale: return self.installed_apps() if ( @@ -1512,6 +1566,16 @@ class ShieldConnectionManager: empty answer that also blocks the retry for APP_LIST_TTL. """ poller = self._poller + # t23/O1: the OPTION is asked FIRST. The review measured this path sending 4 + # real shells with a poller injected into an entry whose option said ADB is + # off: the poller's absence was doing the gating, and that is setup code. + if self._adb_disabled(): + _LOGGER.debug( + "installed-app discovery skipped (ADB is switched off for this " + "entry) — the options page and the app list stay on what is known" + ) + self._note_apps_unavailable(attempted=False) + return [] if poller is None or not self.connected: _LOGGER.debug( "installed-app discovery skipped (ADB unavailable) — the " @@ -1629,7 +1693,9 @@ class ShieldConnectionManager: def _drain_app_label_requests(self) -> None: """Start at most one label fetch per watchdog cycle.""" - if not self._app_label_requests or self._poller is None: + # t23/O1: option AND poller. A queue that cannot be served stays queued (it is + # served once ADB is turned back on) instead of starting a task that cannot run. + if not self._app_label_requests or not self._adb_active(): return package = min(self._app_label_requests) self._app_label_requests.discard(package) @@ -1654,15 +1720,34 @@ class ShieldConnectionManager: return self._intents.state(package) def _adb_disabled(self) -> bool: - """Is ADB switched OFF for this entry (t16)? + """Is ADB switched OFF for this entry (t16)? — the gate for EVERY device path (t23). The config option is the honest signal: no poller can mean "off" OR "enabled but not connected yet", and those are different statuses with different fixes (turn ADB on / pair it, versus wait or reconnect). + + t23/O1: every path in this class that can reach the device ASKS this, rather + than inferring it from a poller being absent. The two are not the same + guarantee: ``_build_poller_factory`` returns None when ADB is off, which + makes the poller's absence a property of the SETUP code, and the review + measured what that costs — with a poller injected into an entry whose own + option said ADB is off, the installed-apps sweep sent 4 real shells and a + label probe sent 1 more. ``available`` and ``connected`` keep describing the + TRANSPORT; this describes PERMISSION. An entry with no options at all is + treated as OFF (fail closed, matching ``__init__.py``). """ options = getattr(self.entry, "options", None) or {} return not options.get(CONF_ADB_ENABLED, False) + def _adb_active(self) -> bool: + """May this entry talk to the device right now (t23/O1)? + + The two-part predicate the shared/background paths use: the OPTION says ADB + is on, and a poller exists to talk through. ``available`` alone answers the + transport half of that question, which is why it cannot be the gate. + """ + return self.available and not self._adb_disabled() + async def async_scan_intents( self, package: str, force: bool = False ) -> IntentScanResult: @@ -1753,6 +1838,17 @@ class ShieldConnectionManager: why ``has_activity_resolver_table`` is asked before the parser is believed. """ + if not self._adb_active(): + # t23/O1: the SHELL SITE itself asks the option (the public caller asked + # first), so no route reaches the device — the t21 repair, one layer down. + return IntentScanResult( + package=package, + status=( + INTENT_SCAN_ADB_DISABLED + if self._adb_disabled() + else INTENT_SCAN_ADB_UNAVAILABLE + ), + ) poller = self._poller if poller is None: # pragma: no cover - the caller checked this return IntentScanResult(package=package, status=INTENT_SCAN_ADB_UNAVAILABLE) @@ -1845,12 +1941,16 @@ class ShieldConnectionManager: async def send_media_command(self, command: str) -> bool: """Send a media key command over the persistent socket.""" - if not self.available: + # t23/O1: the option AND a poller — `available` alone is the transport half + # of the question, and a poller can exist while the option says ADB is off. + if not self._adb_active(): return False return await self._poller.send_media_command(command) async def bring_to_foreground(self, package: str) -> bool: - if not self.available: + # t23/O1: the option AND a poller — `available` alone is the transport half + # of the question, and a poller can exist while the option says ADB is off. + if not self._adb_active(): return False return await self._poller.bring_to_foreground(package) @@ -1898,13 +1998,17 @@ class ShieldConnectionManager: async def send_digit_key(self, digit: str) -> bool: """Channel digit via the ADB keyevent path (fallback for §7).""" - if not self.available: + # t23/O1: the option AND a poller — `available` alone is the transport half + # of the question, and a poller can exist while the option says ADB is off. + if not self._adb_active(): return False return await self._poller.send_digit_keyevent(digit) async def send_enter_key(self) -> bool: """ENTER via the ADB keyevent path (KEYCODE_ENTER = 66, §7 fallback).""" - if not self.available: + # t23/O1: the option AND a poller — `available` alone is the transport half + # of the question, and a poller can exist while the option says ADB is off. + if not self._adb_active(): return False return await self._poller.send_enter_keyevent() @@ -1963,6 +2067,13 @@ class ShieldConnectionManager: recreates it when options change (full reload). """ try: + if self._adb_disabled(): + # t23/O1: ADB off means there is nothing to keep alive, and this sits + # BEFORE the factory — so even a factory that would build a poller + # cannot put traffic on the device. The entry reloads on an options + # change, which is what brings the loop back. + _LOGGER.debug("ADB watchdog not started (ADB is off for this entry)") + return if self._poller is None: if self._poller_factory is None: return @@ -1992,6 +2103,11 @@ class ShieldConnectionManager: awake) media sessions + running apps, all stored with timestamps. The current backoff sleeps after failures; success resets it.""" + if self._adb_disabled(): + # t23/O1: a DIRECT call must not connect either — a TCP connect is device + # traffic. The option is read live on every cycle, so a later flip is picked + # up (a check, never a latch). + return ADB_POLL_INTERVAL try: if not self.connected and not await self._poller.connect(): await self._note_failure(backoff) @@ -2040,6 +2156,13 @@ class ShieldConnectionManager: async def _refresh(self) -> None: """Heartbeat: pull all ADB-derived state into the store (with stamps).""" + if self._adb_disabled(): + # t23/O1: the ADB half of the heartbeat — wake probe, media dump, running + # apps, foreground fallback, the label drain and the logcat ensure — does + # not run at all. (The protocol identity capture further down is protocol + # work, and it is reached only while the watchdog is running, which needs + # ADB on.) + return poller = self._poller awake = await poller.is_awake() if awake is None: diff --git a/tests/ha_api_drift.py b/tests/ha_api_drift.py index 2c9f02b..8138e34 100644 --- a/tests/ha_api_drift.py +++ b/tests/ha_api_drift.py @@ -143,7 +143,7 @@ HA_CALL_OWNERS: dict[str, tuple[str, str]] = { #: Non-vacuity floors — the ONE source of truth for the standalone entry point #: and the pytest wrapper (t39 F1). They sit below the current walk (26 modules, #: 77 imports, 282 relative imports, 65 checked arguments, 6 awaited HA calls, -#: 1650 statement blocks) so a broken or mis-targeted walk fails loudly instead of +#: 1661 statement blocks) so a broken or mis-targeted walk fails loudly instead of #: printing zeros and exiting 0. #: #: THE MARGIN POLICY (t20/O3 — the t19 review asked for this in writing). A floor @@ -247,7 +247,11 @@ FLOORS: dict[str, int] = { # t21: the walk is 1650 (the ADB-off guard returns before the scan, and the # unterminated-value check is a few blocks more), so the 2% band is re-derived # again: 1617. - "statement_blocks": 1617, + # t23: the walk is 1661 — the ADB-off gate is asked by every device path now + # (sweep, label probe, logcat, heartbeat, watchdog, delegation, the accessor), + # which is a dozen more statement blocks. Re-derived at the same 2% band: 1627. + # Raised, not lowered, because that is what the re-measured walk says. + "statement_blocks": 1627, } #: The interpreter floor Home Assistant itself declares: PyPI metadata for diff --git a/tests/test_adb_off_gates_every_device_path.py b/tests/test_adb_off_gates_every_device_path.py new file mode 100644 index 0000000..5740ba1 --- /dev/null +++ b/tests/test_adb_off_gates_every_device_path.py @@ -0,0 +1,412 @@ +"""t23/O1 — with ADB switched off, NO path reaches the device. + +The t22 review measured the harm: with a poller injected into an entry whose own +option said ``adb_enabled: False``, the installed-apps sweep sent **4** real +shells and a label probe sent **1** more. Both gated on the POLLER's absence, +which is a property of the setup code (``_build_poller_factory`` returns None when +ADB is off) — exactly the indirect guarantee the t21 repair had already proven +insufficient for the intent scan. + +This file drives EVERY path in the connection manager that can touch the device +and asserts on the TRAFFIC (shells, polls, commands, connects), because the +traffic is the harm — a correct-looking status string is not the point. The +device side is a scripted double that would answer everything: no real Shield is +involved, so what is proven is that nothing was sent, not what firmware would do. +""" + +from __future__ import annotations + +import asyncio +import contextlib +from types import SimpleNamespace + +import pytest +from nsh.connection import ( + FIELD_AWAKE, + FIELD_FOREGROUND, + FIELD_RUNNING_APPS, + FIELD_SESSIONS, + ShieldConnectionManager, +) +from nsh.const import CONF_ADB_ENABLED, CONF_LOGCAT_EVENTS +from nsh.ha_adb_common import ( + INTENT_SCAN_ADB_DISABLED, + INTENT_SCAN_ADB_UNAVAILABLE, + INTENT_SCAN_FRESH, + INTENT_SCAN_FRESH_EMPTY, +) + +PKG = "com.netflix.ninja" + +#: A minimal answer that IS a resolver section, so a scan that were allowed to run +#: would count as a real answer (nothing here fails for a second reason). +RESOLVER_SECTION = ( + "Activity Resolver Table:\n" + " Schemes:\n" + " https:\n" + " deca7fa com.netflix.ninja/.MainActivity filter fc1\n" + ' Action: "android.intent.action.VIEW"\n' + "Service Resolver Table:\n" +) + + +class WillingPoller: + """A poller that answers EVERY call and records every one (t23/O1). + + Nothing here refuses: if a path is still gated by the wrong fact, this double + will happily talk to "the device" and the shell/command count proves it. + """ + + def __init__(self): + self.connected = True + self.awake = True + self.foreground = "app.a" + self.audio_dump = " package=app.a\n" + self.shell_calls: list[str] = [] + self.poll_calls: list[str] = [] + self.media_calls: list[str] = [] + self.digit_calls: list[str] = [] + self.foreground_sends: list[str] = [] + self.enter_calls = 0 + self.connect_calls = 0 + self.disconnect_calls = 0 + self.sessions = [SimpleNamespace(package="app.a", playing=True)] + + # ── the transport ── + async def connect(self) -> bool: + self.connect_calls += 1 + self.connected = True + return True + + async def disconnect(self) -> None: + self.disconnect_calls += 1 + self.connected = False + + async def health_check(self) -> bool: + return self.connected + + # ── the heartbeat ── + async def is_awake(self) -> bool | None: + self.poll_calls.append("is_awake") + return self.awake + + async def poll_media_sessions(self, foreground_package=None, head_lines=200): + self.poll_calls.append("poll_media_sessions") + return self.sessions[0] if self.sessions else None + + @property + def all_sessions(self): + return self.sessions + + @property + def last_dump(self): + return self.audio_dump + + async def poll_running_apps(self) -> list[dict]: + self.poll_calls.append("poll_running_apps") + return [{"package": "app.a", "name": "App A", "icon": None}] + + async def poll_foreground(self) -> str | None: + self.poll_calls.append("poll_foreground") + return self.foreground + + # ── anything a shell can reach ── + async def shell(self, command: str): + self.shell_calls.append(command) + if "Activity Resolver Table" in command: + return RESOLVER_SECTION + if "pm list packages" in command: + return f"package:{PKG}\n" + if "resolve-activity" in command: + return f"{PKG}/.MainActivity\n" + if "pm dump" in command: + return 'Application Label: "Netflix"\n' + return "ok\n" + + async def send_media_command(self, command: str) -> bool: + self.media_calls.append(command) + return True + + async def bring_to_foreground(self, package: str) -> bool: + self.foreground_sends.append(package) + return True + + async def send_digit_keyevent(self, digit: str) -> bool: + self.digit_calls.append(digit) + return True + + async def send_enter_keyevent(self) -> bool: + self.enter_calls += 1 + return True + + @property + def config(self): + return {"host": "shield.invalid", "port": 5555, "key_path": "/tmp/does-not-exist"} + + @property + def traffic(self) -> list[str]: + """EVERY device-touching call, in one list — the harm in one place (t23).""" + return [ + *(f"shell:{call}" for call in self.shell_calls), + *(f"poll:{call}" for call in self.poll_calls), + *(f"media:{call}" for call in self.media_calls), + *(f"digit:{digit}" for digit in self.digit_calls), + *(f"foreground:{pkg}" for pkg in self.foreground_sends), + *(["enter"] * self.enter_calls), + *(["connect"] * self.connect_calls), + ] + + +def _entry(*, adb_enabled: bool, **options): + """The entry's OWN options decide — this is the fact every path must ask.""" + return SimpleNamespace( + options={ + CONF_ADB_ENABLED: adb_enabled, + # ON, so the logcat path is skipped for the OPTION's reason (ADB off) + # rather than for its own (a stream nobody asked for). + CONF_LOGCAT_EVENTS: True, + **options, + }, + data={}, + title="Shield", + entry_id="e1", + ) + + +def _manager(poller, *, adb_enabled: bool) -> ShieldConnectionManager: + return ShieldConnectionManager( + hass=None, entry=_entry(adb_enabled=adb_enabled), poller=poller + ) + + +#: THE AUDIT (t23/O1), as data: every path in the manager that can reach the +#: device, with the call that drives it. Each is run against a manager whose +#: option says ADB is off, and the poller's traffic must stay EMPTY. +AUDITED_PATHS = [ + ("intent scan", lambda m: m.async_scan_intents(PKG)), + ("intent scan (forced)", lambda m: m.async_scan_intents(PKG, force=True)), + ("installed-apps refresh", lambda m: m.async_refresh_installed_apps(force=True)), + ("installed-apps refresh (non-forced)", lambda m: m.async_refresh_installed_apps()), + ("app-label lookup", lambda m: _sync_label(m)), + ("app-label drain (watchdog side)", lambda m: _drain(m)), + ("adb text entry", lambda m: m.send_adb_input_text("hello")), + ("media key send", lambda m: m.send_media_command("play")), + ("bring to foreground", lambda m: m.bring_to_foreground(PKG)), + ("digit key send", lambda m: m.send_digit_key("5")), + ("enter key send", lambda m: m.send_enter_key()), + ("watchdog cycle", lambda m: m._watchdog_cycle(5.0)), + ("heartbeat refresh", lambda m: m._refresh()), + ("logcat ensure", lambda m: _logcat(m)), + ("watchdog start", lambda m: _start_watchdog(m)), +] + + +async def _maybe_await(value): + """Drive an audited path: some are coroutines, some are plain calls (t23).""" + if asyncio.iscoroutine(value): + return await value + return value + + +def _sync_label(manager): + return manager.app_label(PKG) + + +async def _settle_label_tasks(manager): + """Await any label fetch the drain started, so its shell WOULD have happened.""" + for task in list(manager._tasks): + if task.get_name().startswith("app_label"): + with contextlib.suppress(Exception): + await task + + +async def _drain(manager): + """The render path queues a label fetch, then the watchdog drains the queue.""" + manager.app_label(PKG) + manager._drain_app_label_requests() + await _settle_label_tasks(manager) + + +def _logcat(manager): + manager._ensure_logcat_stream() + + +async def _start_watchdog(manager): + await manager.async_start() + await asyncio.sleep(0.01) # would be one cycle if the loop had started + await manager.async_stop() + + +class TestEveryAuditedPathSendsNothing: + """ADB off + a poller that WOULD answer = zero traffic on every path.""" + + @pytest.mark.parametrize( + ("name", "drive"), AUDITED_PATHS, ids=[name for name, _ in AUDITED_PATHS] + ) + async def test_the_path_does_not_touch_the_device(self, name, drive): + poller = WillingPoller() + manager = _manager(poller, adb_enabled=False) + + await _maybe_await(drive(manager)) + await asyncio.sleep(0) # let any task the path started really run + + assert poller.traffic == [], f"{name} reached the device: {poller.traffic}" + + async def test_a_full_cycle_of_every_path_still_sends_nothing(self): + """One manager, every path in sequence — the whole cycle, not one call.""" + poller = WillingPoller() + manager = _manager(poller, adb_enabled=False) + + # 1. the intent scan (t21's repair, asked again one layer deeper) + assert (await manager.async_scan_intents(PKG)).status == INTENT_SCAN_ADB_DISABLED + assert ( + await manager.async_scan_intents(PKG, force=True) + ).status == INTENT_SCAN_ADB_DISABLED + + # 2. the installed-apps sweep — the 4-shell path the review measured + assert await manager.async_refresh_installed_apps(force=True) == [] + assert manager.installed_apps() == [] + + # 3. the app-label probe — the 1-shell path, both entry points + assert manager.app_label(PKG) is None + manager._drain_app_label_requests() + await _settle_label_tasks(manager) + + # 4. text entry and every key/command delegation + assert await manager.send_adb_input_text("hello") is False + assert await manager.send_media_command("play") is False + assert await manager.bring_to_foreground(PKG) is False + assert await manager.send_digit_key("5") is False + assert await manager.send_enter_key() is False + + # 5. the heartbeat, the watchdog and the logcat stream + await manager._watchdog_cycle(5.0) + await _start_watchdog(manager) + manager._ensure_logcat_stream() + await asyncio.sleep(0) + + # 6. the public accessor the OUT-OF-FILE consumers (adb_command service, + # screen capture, intent shell) read — it must refuse too + assert manager.adb_poller is None + + assert poller.traffic == [], poller.traffic + + async def test_nothing_is_cached_when_the_cycle_is_refused(self): + """No cache write either: an unpermitted scan is not an answer.""" + poller = WillingPoller() + manager = _manager(poller, adb_enabled=False) + + await manager.async_scan_intents(PKG) + await manager.async_refresh_installed_apps(force=True) + manager.app_label(PKG) + manager._drain_app_label_requests() + await manager._refresh() + + assert manager.intent_scan_state(PKG) is None, "no intent answer" + assert manager.installed_apps() == [], "no app table" + assert manager.installed_apps_stale is True, "nothing was stamped fresh" + assert manager.app_label_requests == set(), "nothing queued" + assert manager.app_label_inflight == set(), "nothing in flight" + assert manager.store.get(FIELD_AWAKE) is None, "no wake reading" + assert manager.store.get(FIELD_SESSIONS, []) == [] + assert manager.store.get(FIELD_RUNNING_APPS, []) == [] + assert manager.store.get(FIELD_FOREGROUND) is None + assert manager.awake is None + + +class TestTheWatchdogNeverConnects: + """A TCP connect is device traffic too — the loop must not even start.""" + + async def test_the_watchdog_does_not_build_a_poller_or_connect(self): + ours = WillingPoller() + built = [] + + def factory(): + built.append(1) + return ours + + manager = ShieldConnectionManager( + hass=None, + entry=_entry(adb_enabled=False), + poller_factory=factory, + ) + await manager.async_start() + await asyncio.sleep(0.02) + await manager.async_stop() + + assert built == [], "the factory was never asked for a poller" + assert ours.connect_calls == 0, "and nothing was connected" + assert ours.traffic == [] + + async def test_a_direct_cycle_does_not_connect_either(self): + poller = WillingPoller() + poller.connected = False + manager = _manager(poller, adb_enabled=False) + + await manager._watchdog_cycle(5.0) + + assert poller.connect_calls == 0 + assert poller.traffic == [] + + +class TestTheOptionIsACheckNotALatch: + """Turning ADB back on resumes normal work — no remembered decision.""" + + async def test_the_same_manager_resumes_when_the_option_flips(self): + poller = WillingPoller() + manager = _manager(poller, adb_enabled=False) + + assert await manager.async_refresh_installed_apps(force=True) == [] + assert manager.app_label(PKG) is None + assert await manager.send_media_command("play") is False + assert poller.traffic == [], "nothing while it is off" + + manager.entry.options[CONF_ADB_ENABLED] = True # the user turns ADB on + + rows = await manager.async_refresh_installed_apps(force=True) + assert rows, "the sweep really ran once ADB is on" + assert manager.app_label("com.other.app") is None # queued, not sent yet + assert manager.app_label_requests == {"com.other.app"} + assert await manager.send_media_command("play") is True + assert await manager.send_digit_key("5") is True + assert manager.adb_poller is poller + assert poller.shell_calls, "the device was reached again" + assert poller.media_calls == ["play"] + assert poller.digit_calls == ["5"] + + async def test_the_intent_scan_resumes_too(self): + poller = WillingPoller() + manager = _manager(poller, adb_enabled=False) + assert (await manager.async_scan_intents(PKG)).status == INTENT_SCAN_ADB_DISABLED + + manager.entry.options[CONF_ADB_ENABLED] = True + + result = await manager.async_scan_intents(PKG) + assert result.status in (INTENT_SCAN_FRESH, INTENT_SCAN_FRESH_EMPTY) + assert result.answered is True + assert manager.intent_scan_state(PKG) is not None, "the answer is cached now" + + +class TestTheOtherStatusesAreUntouched: + """ADB ON with no poller is 'unavailable', never 'disabled' (t21's split).""" + + async def test_no_poller_reads_as_unavailable(self): + manager = _manager(None, adb_enabled=True) + + assert (await manager.async_scan_intents(PKG)).status == ( + INTENT_SCAN_ADB_UNAVAILABLE + ) + assert await manager.async_refresh_installed_apps(force=True) == [] + assert manager.installed_apps_status == "unknown" + assert await manager.send_media_command("play") is False + + async def test_adb_off_and_no_poller_are_told_apart(self): + disabled = _manager(WillingPoller(), adb_enabled=False) + unbuilt = _manager(None, adb_enabled=True) + + assert (await disabled.async_scan_intents(PKG)).status == ( + INTENT_SCAN_ADB_DISABLED + ) + assert (await unbuilt.async_scan_intents(PKG)).status == ( + INTENT_SCAN_ADB_UNAVAILABLE + ) diff --git a/tests/test_command_routing.py b/tests/test_command_routing.py index 9424561..41ae641 100644 --- a/tests/test_command_routing.py +++ b/tests/test_command_routing.py @@ -18,6 +18,7 @@ from entity_stublib import install, load_entity_modules from nsh.connection import ShieldConnectionManager from nsh.const import ( APP_LABEL_EXIT_MARKER, + CONF_ADB_ENABLED, CONF_APP_NAME_MAP, CONF_COMMAND_SCRIPTS, DEFAULT_APP_DIRECTORY, @@ -93,10 +94,21 @@ class FakeAdbPoller: return True +def _adb_entry(): + """A realistic entry: its own option says ADB is ON (t23/O1). + + Production ALWAYS builds the manager with an entry; passing None modelled an + ADB-off entry (fail closed), which the device paths now honour as the gate. + """ + return SimpleNamespace( + options={CONF_ADB_ENABLED: True}, data={}, title="Shield", entry_id="e1" + ) + + def make_manager(store=None, poller=None, protocol=None): return ShieldConnectionManager( hass=None, - entry=None, + entry=_adb_entry(), poller=poller or FakeAdbPoller(), protocol=protocol or FakeProtocolClient(), store=store, @@ -115,7 +127,12 @@ def make_player(modules, manager=None, jellyfin_entity=None, options=None): mp_module = modules["media_player"] if manager is None: manager = make_manager() - entry = SimpleNamespace(entry_id="e1", options={}, data={}, title="mdarcy") + entry = SimpleNamespace( + entry_id="e1", + options={CONF_ADB_ENABLED: True}, + data={}, + title="mdarcy", + ) manager.entry = entry def _set_option(key, value): @@ -2185,7 +2202,7 @@ class DownAdbPoller(FakeAdbPoller): def make_manager_with(*, poller=None, protocol=None): """A manager with exactly these transports (None = not configured).""" return ShieldConnectionManager( - hass=None, entry=None, poller=poller, protocol=protocol, store=None + hass=None, entry=_adb_entry(), poller=poller, protocol=protocol, store=None ) diff --git a/tests/test_command_scripts.py b/tests/test_command_scripts.py index acea314..83a6d86 100644 --- a/tests/test_command_scripts.py +++ b/tests/test_command_scripts.py @@ -19,6 +19,7 @@ from types import SimpleNamespace import entity_stublib import pytest from nsh.const import ( + CONF_ADB_ENABLED, CONF_COMMAND_SCRIPTS, CONF_REMOTE_COMMAND_SCRIPT, SCRIPT_VAR_APP_NAME, @@ -317,7 +318,8 @@ def test_the_remote_entry_options_are_the_media_player_options(modules): player._entry.options[CONF_COMMAND_SCRIPTS] == entity._entry.options[CONF_COMMAND_SCRIPTS] ) - assert make_manager().entry is None # the harness's manager starts unbound + # the harness's manager carries a realistic entry whose option says ADB is on + assert make_manager().entry.options[CONF_ADB_ENABLED] is True WATCH_SCRIPT = "script.shield_remote" diff --git a/tests/test_connection.py b/tests/test_connection.py index 4fdaa8f..cb17d9e 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -18,7 +18,12 @@ from nsh.connection import ( ShieldConnectionManager, StateStore, ) -from nsh.const import ADB_POLL_INTERVAL, APP_LABEL_EXIT_MARKER, APP_LABEL_GREP_SHAPES +from nsh.const import ( + ADB_POLL_INTERVAL, + APP_LABEL_EXIT_MARKER, + APP_LABEL_GREP_SHAPES, + CONF_ADB_ENABLED, +) def test_connection_module_depends_on_no_homeassistant(): @@ -100,8 +105,28 @@ class FakePoller: return self.connected +def _adb_entry(**options): + """A realistic entry: its own option says ADB is ON (t23/O1). + + The manager is ALWAYS built with an entry in production, and an entry whose + options do not say ``adb_enabled: True`` means ADB is switched off — the gate + every device path now asks directly (``connection._adb_disabled``). A harness + that passed ``entry=None`` was therefore modelling an ADB-off entry while + expecting traffic to flow, which is exactly the combination the t22 review + measured. Tests that WANT the disabled behaviour build their own entry. + """ + return types.SimpleNamespace( + options={CONF_ADB_ENABLED: True, **options}, + data={}, + title="Shield", + entry_id="e1", + ) + + def _manager(poller=None, **kwargs) -> ShieldConnectionManager: - return ShieldConnectionManager(hass=None, entry=None, poller=poller, **kwargs) + return ShieldConnectionManager( + hass=None, entry=_adb_entry(), poller=poller, **kwargs + ) class TestStateStore: @@ -146,7 +171,7 @@ class TestAppLabelCache: "application-label-de: 'TiviMate DE'" ) manager = ShieldConnectionManager( - hass=None, entry=None, poller=poller, clock=(clock or (lambda: now)) + hass=None, entry=_adb_entry(), poller=poller, clock=(clock or (lambda: now)) ) return manager, now @@ -332,9 +357,9 @@ class TestLogcatStream: poller = FakePoller() poller.awake = True manager = ShieldConnectionManager( - hass=None, entry=None, poller=poller, clock=(clock or (lambda: now)) + hass=None, entry=_adb_entry(), poller=poller, clock=(clock or (lambda: now)) ) - manager.entry = LEntry(options={"logcat_events": bool(lines)}) + manager.entry = LEntry(options={"logcat_events": bool(lines), CONF_ADB_ENABLED: True}) lines = lines or [] async def _open(): @@ -428,9 +453,9 @@ class TestLogcatStream: poller = FakePoller() poller.awake = True manager = ShieldConnectionManager( - hass=None, entry=None, poller=poller, clock=lambda: 1000.0 + hass=None, entry=_adb_entry(), poller=poller, clock=lambda: 1000.0 ) - manager.entry = LEntry(options={"logcat_events": True}) + manager.entry = LEntry(options={"logcat_events": True, CONF_ADB_ENABLED: True}) # the START record spans two stream lines (wrapped cmp=) lines = [ @@ -833,7 +858,7 @@ class TestLifecycle: poller = FakePoller() poller.awake = True manager = ShieldConnectionManager( - hass=None, entry=None, poller_factory=lambda: poller + hass=None, entry=_adb_entry(), poller_factory=lambda: poller ) await manager.async_start() await asyncio.sleep(0) diff --git a/tests/test_installed_apps.py b/tests/test_installed_apps.py index b910336..8a2575c 100644 --- a/tests/test_installed_apps.py +++ b/tests/test_installed_apps.py @@ -14,6 +14,7 @@ from __future__ import annotations import asyncio import sys from pathlib import Path +from types import SimpleNamespace sys.path.insert(0, str(Path(__file__).resolve().parent)) import entity_stublib # noqa: E402 @@ -40,6 +41,7 @@ from nsh.const import ( # noqa: E402 APP_LABEL_EXIT_MARKER, APP_LABEL_GREP_SHAPES, APP_LIST_TTL, + CONF_ADB_ENABLED, CONF_APP_NAME_MAP, ) from nsh.ha_adb_common import ( # noqa: E402 @@ -260,8 +262,20 @@ class _DiscoveryPoller: return None -def _manager(poller): - return ShieldConnectionManager(hass=None, entry=None, poller=poller) + +def _entry(*, adb_enabled=True, **options): + """A realistic entry: the ADB option is ON unless a test says otherwise (t23).""" + return SimpleNamespace( + options={CONF_ADB_ENABLED: adb_enabled, **options}, + data={}, + title="Shield", + entry_id="e1", + ) + +def _manager(poller, *, adb_enabled=True): + return ShieldConnectionManager( + hass=None, entry=_entry(adb_enabled=adb_enabled), poller=poller + ) class TestPackageListParsing: @@ -485,7 +499,7 @@ class TestDiscoverySweep: now = [1000.0] poller = _DiscoveryPoller() manager = ShieldConnectionManager( - hass=None, entry=None, poller=poller, clock=lambda: now[0] + hass=None, entry=_entry(), poller=poller, clock=lambda: now[0] ) await manager.async_refresh_installed_apps() assert manager.installed_apps() @@ -542,7 +556,7 @@ class TestDiscoveryDegradesGracefully: """ async def test_no_poller_is_an_empty_list(self): - manager = ShieldConnectionManager(hass=None, entry=None, poller=None) + manager = ShieldConnectionManager(hass=None, entry=_entry(), poller=None) rows = await manager.async_refresh_installed_apps() @@ -567,7 +581,7 @@ class TestDiscoveryDegradesGracefully: now = [1000.0] poller = _DiscoveryPoller(failure=True) manager = ShieldConnectionManager( - hass=None, entry=None, poller=poller, clock=lambda: now[0] + hass=None, entry=_entry(), poller=poller, clock=lambda: now[0] ) rows = await manager.async_refresh_installed_apps() diff --git a/tests/test_protocol_transport.py b/tests/test_protocol_transport.py index 6be6144..3811ee0 100644 --- a/tests/test_protocol_transport.py +++ b/tests/test_protocol_transport.py @@ -12,6 +12,7 @@ against the real library's VolumeInfo TypedDict when the package is present import asyncio import sys import time +from types import SimpleNamespace import pytest from nsh.connection import ( @@ -28,6 +29,7 @@ from nsh.connection import ( StateStore, volume_tuple, ) +from nsh.const import CONF_ADB_ENABLED class InvalidAuth(Exception): # noqa: N818 - the library's own class name @@ -152,8 +154,21 @@ class FakeProtocolClient: self._reconnect_task.cancel() +def _adb_entry(): + """A realistic entry: its own option says ADB is ON (t23/O1). + + The protocol tests exercise the ADB watchdog paths too (`_watchdog_cycle`), + and a manager with no entry means ADB is off — the gate those paths now ask. + """ + return SimpleNamespace( + options={CONF_ADB_ENABLED: True}, data={}, title="Shield", entry_id="e1" + ) + + def _protocol_manager(client=None, **kwargs) -> ShieldConnectionManager: - return ShieldConnectionManager(hass=None, entry=None, protocol=client, **kwargs) + return ShieldConnectionManager( + hass=None, entry=_adb_entry(), protocol=client, **kwargs + ) async def _drain_loop(): @@ -454,7 +469,7 @@ class TestLifecycle: monkeypatch.setattr(ShieldConnectionManager, "build_protocol_client", fake_builder) manager = ShieldConnectionManager( hass=None, - entry=None, + entry=_adb_entry(), protocol_settings={ "host": "shield.local", "client_name": "Home Assistant", @@ -561,7 +576,7 @@ class TestAdbOutageKeepsProtocolTruth: poller.awake = None # wake poll fails → cycle failure path client = FakeProtocolClient() manager = ShieldConnectionManager( - hass=None, entry=None, poller=poller, protocol=client + hass=None, entry=_adb_entry(), poller=poller, protocol=client ) await manager._start_protocol() @@ -683,7 +698,9 @@ class TestAdbStaysThePrimarySource: poller = FakeAdbForOutage() poller.connected = True poller.awake = True - manager = ShieldConnectionManager(hass=None, entry=None, poller=poller) + manager = ShieldConnectionManager( + hass=None, entry=_adb_entry(), poller=poller + ) await manager._start_protocol() assert manager._protocol is None