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