master
15
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 James Mackay
|
7ed982801a |
test(adb-off audit): name the carve-outs, drive the paths the audit names (t25)
The t24 review's four observations on the t23 ADB-off audit. No production code
changes: button.py is untouched, deliberately (see O1).
O1 — THE PAIR ADB CARVE-OUT, ACCEPTED AND NAMED. The button's press builds its own
AdbMediaSessionPoller and connects without ever reading the entry's option. I
searched for a production route that reaches the press with ADB off and found
NONE, so no gate was added:
* `button.async_setup_entry` is the ONLY place that entity is created, and it
creates it only while the option is on (button.py:34-35);
* every options save runs through `NvidiaShieldOptionsFlow(OptionsFlowWithReload)`,
which reloads the entry and re-runs that setup — so turning ADB off takes the
button away (config_flow.py:1340-1353);
* the press is user-initiated in any case — the same class as the already
declared `test_connect` pairing probe.
It is now a NAMED carve-out in the audit's `CARVE_OUTS` table, with what it does,
why that is safe, and the assertions behind both. The measurement is DRIVEN rather
than quoted: the press is executed with the option OFF and ON and asserted to cost
the same 1 ctor / 1 connect / 1 disconnect, so the carve-out's substance is a test
result and not prose. A meta test checks every carve-out's named assertion really
exists, so a carve-out cannot point at a phantom. MUTATION-CHECKED: making
`async_setup_entry` add the entity unconditionally fails three of the new tests.
The intent button is explicitly NOT a carve-out (it can exist for a disabled entry
and refuses through the t23 accessor gate) — stated in the same entry.
O2 — THE AUDIT NOW EXECUTES WHAT IT NAMES. `_open_logcat_stream` and the logcat
ladder are driven both ways: two new rows in the parametrized `AUDITED_PATHS` table
(so the disabled case costs zero traffic) plus dedicated assertions — the open
RAISES "ADB is switched off" in a `pytest.raises(match=...)`, and the ladder
RETURNS at once with `_logcat_failures == []` and `_logcat_disabled is False`, i.e.
a local option is not booked as a device failure. The anti-overcorrection pin is
kept in the same class: with ADB ON, five real ladder failures DO auto-disable.
MUTATION-CHECKED: removing either t23 logcat gate fails three of these tests.
O3 — THE ROTTABLE FIGURE IS GONE. The statement_blocks bullet no longer states a
count ("currently 28" was stale within one unit of work); the band (~2%) is the
policy, the current walk lives in the header the sweep prints, and the comment now
says why no band carries a count. The relative_imports bullet lost its illustrative
figure for the same reason.
O4 — THE ONE WRITE IS NAMED AS A DEBT. The audit's module docstring now states
exactly what "nothing is cached" means: the single write a disabled path can still
make is `_app_label_failed.add(package)` in `_dump_label` — a retry DEBT, not an
answer. It is pinned by a test: with ADB off the debt is recorded while
`_app_labels` stays EMPTY and the lookup still answers None; with ADB back on the
probe runs, the label is cached and the debt is settled (the discard that the code
already does).
FAIL-CLOSED, CONFIRMED AND LOCATED. The harness change (`entry=None` now reads as
ADB off) matches production because both read the option the same way: the manager
returns "disabled" for a missing option, and `__init__.py` builds no poller for the
same missing option — asserted by two new tests (an entry with `options={}`, and a
manager with no entry at all) and by a source assertion on `__init__.py`'s own
`if not entry.options.get(CONF_ADB_ENABLED, False):`. No production path is
affected: production always constructs the manager WITH the entry, and for an entry
whose option is on nothing changed at all.
No regressions: 942 passed / 5 skipped (V1), 943 / 4 (V2) — the +19 are the new
tests. ruff clean, drift exit 0, the real-HA pattern collects and runs its 3 files.
The device side remains a scripted double: the claim proven is that nothing was
sent, not what firmware would do.
|
||
|
James Mackay
|
de1c34777d |
fix(adb): ADB-off is asked by every device path, not inferred from a missing poller (t23)
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:<pkg>`,
`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.
|
||
|
James Mackay
|
8f064b3247 |
fix(intents): an ADB-off entry never scans, and a truncated detail is never believed (t21)
The t18 review's three findings, closed with the reviewer's own repro.
F1 (HIGH — the harm was real shells, not a wrong label). `async_scan_intents`
decided between `adb_disabled` and `adb_unavailable` INSIDE `if self._poller is
None:`, so a manager that HELD a poller fell through to the shell loop: with
`adb_enabled: False` and a connected poller the review measured status `fresh`,
one candidate CACHED, and `pm dump com.x …` really sent. The option is a fact
about the ENTRY, so it is now checked once at the top and returns before
anything else — no cache read, no cache write, no shell. The no-poller branch
keeps the OTHER honest answer: with the option ON and nobody to ask, the status
is `adb_unavailable` ("wait or reconnect"), never `adb_disabled` ("go turn the
option on"), because the two have different fixes. Five tests pin it, including
a cache-leak case (a manager with a cached answer whose option is then switched
off answers `adb_disabled`, not the cache) and both statuses with a FORCED scan.
F2 (low, but it fabricated a value that looked real). A capture cut mid-detail
produced `Scheme: "ht` → scheme `"ht` and `Authority: "www.exam` → that
authority, and `_candidate_data` would then prefill the form with a URL no app
can match. Not an injection (everything stays quoted when used), which is
exactly why nothing caught it. `_unquote` now DROPS a value that opens a quote
and never closes it (a lone quote too), so the callers turn it into None: the
candidate survives with its intact action/component and simply carries no
scheme/authority/path — and therefore prefills NO data instead of a broken URL.
A complete quoted value is still read (the anti-overcorrection pin), and the
consequence is pinned at the consumption end too, through the real options-flow
helper.
F3 (low). The floor comment now answers the review in writing, where the next
reader looks: the t18 review's measurement (awaited_calls 6 against a floor of 5,
three other counters within 2) is named, and the verdict is recorded — those ARE
the policy margins (2 for owner-counters because an owner really can be
refactored away, 1 for awaited_calls because six sites cannot carry a percentage
band). The counters were genuinely updated, not dodged: raised to the real sweep
when the code grew (20→24, 70→75, 235→275, 55→63, 1400→1620) and re-derived
twice since (t20 1620→1615, t21 1615→1617 for this unit's blocks).
EVIDENCE THAT THE TESTS PIN THE FIX (not merely pass): the same files were run
against the PRE-fix source (git archive HEAD, a throwaway copy under /tmp) — 9 of
the 11 new tests FAIL there, including the reviewer's repro, which fails with
`assert 'fresh' == 'adb_disabled'`. The two that pass are the deliberate
anti-overcorrection pins: a COMPLETE quoted value is still parsed, and the
no-poller control keeps its own status.
NO REGRESSION: 900 passed / 5 skipped, 901 / 4 with androidtvremote2; the
injection proof, the fixture's candidates with dedup/flagging/order/bound
disclosures, both cache outcomes, the registration round trip through the real
FlowManager, button lifecycle and defaults, the YAML seed rule and §7.1
(supports_response optional, a real return_response call, a no-response call) all
still pass. The strings files are byte-identical (untouched here). Real-HA pattern
collected and RAN 3 files → 74 passed / 0 skipped. Drift exit 0 (1650 blocks).
NOT VERIFIED HERE: the device side remains a scripted double — no real Shield was
asked for its resolver table, so the shell COUNT and the status vocabulary are
what is proven, not real firmware behaviour. No browser either: the form prefill
is asserted on the values the helper returns.
|
||
|
James Mackay
|
6b866019ef |
fix(transport): no stale protocol error, no connect task outliving its entry, honest banners (t20)
The t19 review's findings on the protocol transport, all of them closed here. O1 (MEDIUM, FIRST). `_note_protocol_state(PROTOCOL_CONNECTED, None)` only assigned `last_error` when its argument was non-None, so a failure kept being published after the transport recovered: the review measured `protocol_last_error == "connect: Net: boom"` beside `protocol_connected: true` — a contradiction on the exact attributes the diagnostic surface was built for, so the user could not answer "is the protocol working?" without a device test. The pair now has ONE stated meaning, in the code that owns it and in the remote's attribute docs: `protocol_state`/`protocol_connected` is the transport's CURRENT condition, and `protocol_last_error` is the failure the current state has NOT recovered from. A session becoming proven CLEARS it — and a session becomes proven in BOTH places that can prove one, so the connect success and the transport's own availability push now take the same route (the push previously assigned the state directly and would have left the stale error again). `protocol_failures` keeps counting, so a recovery stays visible as history rather than as a stale message. A failure that happens AFTER the session came up is still kept: that one is current, not stale, and a test pins that distinction so a future "just clear it everywhere" refactor cannot make failures invisible. O4 + O5 (the lifecycle leaks, now real tests instead of observations). The connect attempt was never in the manager's task collection, so `async_stop` never cancelled it — a reconnect loop could outlive its entry, and since every options save reloads the entry those loops could accumulate across saves. `async_stop` also only SCHEDULED its cancellations. The attempt is now tracked, `async_stop` cancels AND awaits every tracked task, and the stop path refuses to start anything new. A double start is deterministic instead of orphaning the first: a live attempt is reused, an already-handed-over session (connected or disconnected — the library's keep_reconnecting owns it) starts nothing, and only a FINISHED attempt that never got a session may be replaced. The guard sits before the push callbacks are registered, so a second start cannot double-register them either. O2 (LOW). The connect banner claimed liveliness a successful `async_connect()` cannot know. It now says exactly what it proved (the client accepted a session) and hands liveness to what can actually report it (the transport's own availability push). STALE is defined where the fixes live: a banner is stale when a later state change never followed it up, so the log's last word contradicts the entity. Every transition therefore announces itself with both states — INFO for the two a user acts on, DEBUG otherwise — which is what makes the last transport line always match the published state. O3 (LOW). The drift floors now carry an explicit MARGIN POLICY instead of leaving a 2-margin unexplained: a floor is a vacuity-and-blindness alarm, not a size ratchet (it must fail when the walk stops COVERING something, not when a legitimate change removes code). Owner-counters keep a 2 margin (the project's own t62 canary, now stated once), statement_blocks moves to a ~2% band (28 blocks — the t19 complaint was that a 23-statement refactor could trip it), relative_imports ~2.5%, and the awaited-calls floor is called out plainly as the weakest one because a percentage margin on six sites is meaningless. The rule for changing any floor is written down with it. The walk is now 1648 blocks, so the floor is re-derived: 1615. EVIDENCE THAT THE TESTS PIN THE FIX (not merely pass): the same test file was run against the PRE-fix source (`git archive HEAD`, a throwaway copy under /tmp) and 13 of the 15 new tests FAIL there. The two that pass pre-fix are the deliberate anti-overcorrection pins: a failure that is still current keeps its message, and a terminal (auth-rejected) attempt may still be replaced. No regressions: 889 passed / 5 skipped, 890 / 4 with androidtvremote2, the real-HA pattern (3 files) 74 passed with no skips, ruff clean, drift exit 0. NOT VERIFIED HERE: no real Shield and no paired device, so no real TLS session, certificate or push was ever received — the connect path runs against doubles plus the installed library's real source, and the *timing* of a cancellation during a live handshake is inferred from the awaited-cancel path, not observed on hardware. |
||
|
James Mackay
|
e6c6bdb1e6 |
test(intents): the real-HA action proof, the README pins and the raised drift floor (t17, part 3)
The evidence half of the ticket. Two things here are new facts about the
installed Home Assistant rather than about our code.
THE ACTION, DRIVEN BY REAL HOME ASSISTANT (tests/test_send_intent_real_ha.py).
The claim "the action returns the device's output" cannot be settled with a stub:
whether a service ANSWERS is HA's decision. On 2026.10.1, with the real service
registry and the real websocket command the frontend calls:
* `supports_response` is OPTIONAL, which is what keeps every caller that does not
ask for a response working — REQUIRED would break them;
* `get_services` serves the intent field as a dropdown whose options are the
registered names AND keeps `custom_value` on, so a name typed by hand is still
accepted;
* the entity target is `{entity: [{domain: [media_player]}]}` — the picker, the
same mechanism t14 proved for adb_command (async_register has no target
parameter; the runtime description is what the UI reads);
* a real `async_call(..., return_response=True)` comes back as
`{"intent_response": "Starting: Intent { … }"}`, and the command that reached the
ADB surface is asserted VERBATIM, including the `--es`/`--ez` flags and the
resolved URL;
* an unknown intent, a missing required field, a refused intent (the device's own
"does not exist" text is kept and the call RAISES), ADB being down and a missing
or unsupported target each raise ServiceValidationError with our translation key
and a message a user can act on.
The dropdown is published from `hass.data[DOMAIN]` — the store a real entry setup
fills — so the file also pins the EMPTY state: nothing stored means no options, not
invented ones.
THE OPTIONS SCREEN, DRIVEN BY THE REAL FLOW MANAGER
(tests/test_config_flow_real_ha.py). The Intents menu, the Add-new sentinel as a
real selector option, a registration that round-trips to `create_entry` (JSON-safe,
as the options store requires), a refused row coming back as a real form error with
its note, and a removal that only takes effect after the confirmation. This is the
file that exists because a step id which does not match its method name is an
UnknownStep that only a real FlowManager raises.
THE FLOOR (tests/ha_api_drift.py). The sweep now walks 26 modules, 77 imports, 282
relative imports, 65 call arguments and 1637 statement blocks (was 76/242/57/1417),
so the non-vacuity floors move with it: imports 70→75, relative_imports 235→275,
call_args 55→63, modules 20→24, statement_blocks 1400→1620. They stay just under
the walk on purpose — a floor's job is to notice a walk that silently stopped
covering the new surfaces, and the unreachable-statement pass is exactly the gate
that must keep covering the ~240 new blocks. Raised, never lowered.
THE README (tests/test_readme.py). The intents example is parsed and its keys are
required to be ones the seed honours (documented-but-ignored is the failure this
catches, t30's rule applied to the nested block), the screen and the action are
named, and every honest failure the README promises has its key. Plus a cross-check
in the intents suite: the row keys the README documents are exactly the keys the
seed validator KEEPS, so the docs cannot describe a key that silently vanishes.
Verify (all five, in the contract order):
uvx --with pytest-asyncio pytest tests -q → 872 passed, 5 skipped
uvx --with pytest-asyncio --with androidtvremote2 pytest tests -q → 873 passed, 4 skipped
uvx --python 3.14 --with 'homeassistant>=2026.10' --with adb-shell \
--with pytest-asyncio pytest tests/test_config_flow_real_ha.py -q → 49 passed
uvx ruff check custom_components tests → clean
uvx --python 3.14 --with 'homeassistant>=2026.10' --with pyyaml \
python tests/ha_api_drift.py → exit 0, no drift
The real-HA pattern collected and RAN three files (no skips):
tests/test_adb_command_real_ha.py, tests/test_config_flow_real_ha.py,
tests/test_send_intent_real_ha.py → 74 passed.
|
||
|
James Mackay
|
91d9fb3883 |
feat(intents): discover an app's intents from its activity resolver table (t16)
First half of the approved intents feature (docs/INTENTS_PLAN.md §2): the scan
that answers "what can this app actually be asked to do". No UI is wired here —
this is the discovery module, the manager path it rides on, and the tests.
THE DUMP IS FILTERED ON THE DEVICE. On the user's Shield `pm dump
com.netflix.ninja` is ~2241 lines of whole dumpsys for ONE app, so shipping it
over ADB to read a few dozen resolver lines repeats the mistake `| head -300`
made for app labels (t66 — where a truncation artefact was read as "this app has
no label"). The fetch now sends:
pm dump <pkg> 2>/dev/null | sed -n "/Activity Resolver Table/,/Service Resolver Table/p" | head -n 400
with a bounded `grep -A 400 -m 1 -F 'Activity Resolver Table:'` fallback for a
device whose toolbox cannot do sed ranges, tried only when the primary answer did
not contain the section. The package id is the only caller-supplied text in
either command and is always shlex.quote'd, and the sed script is built from two
constants — tests drive eight hostile package ids (semicolons, &&, backticks,
$(), pipes, both quote kinds, a newline) and assert that the shell sees the
package as exactly one verbatim token and that no extra command token appears.
WHAT A CANDIDATE CARRIES: component, action, categories and the data
(scheme/authority/path, plus the matcher kind — `/title.*` is a GLOB, not a
path, and a caller that treats it as literal builds the wrong URI). A filter
carrying several schemes/authorities/paths expands to one candidate per
combination, because each is a concrete match, and dedup collapses the
coincidences. MIME keys are deliberately NOT modelled (they are not one of §3's
fields) — such a filter is still reported through its component, action and
categories rather than being invented into a URI.
RANK, NEVER HIDE. The four families the plan names as internal-looking noise
(DEBUG_*, androidx.*, com.google.firebase.*, ConstraintProxy*) are FLAGGED, not
deleted: they sort last so the useful rows come first, and the user still
decides. Ordering is total (flag, action, scheme, authority, path, component,
categories) so a re-scan is byte-identical, and the 200-candidate cap is applied
after dedup and DISCLOSED through truncated/found — a capped answer never looks
complete.
THE CACHE KEEPS AN ANSWER AND A FAILURE APART, structurally: put() is its only
writer, only a scan that RAN calls it, and it derives the entry's state — an
empty candidate list is INTENT_ENTRY_EMPTY, a real ANSWER the device gave, while
a scan whose transport failed caches nothing and is retried. A stale entry is
dropped on read rather than served. The scan itself has one status per outcome
(fresh / fresh_empty / cached / adb_disabled / adb_unavailable / unreadable /
invalid_package) so "ADB is off", "the device did not answer" and "this app
advertises nothing" can never be confused, and the not-answered statuses carry no
candidates at all — an ADB-down scan cannot look like an app with no intents.
ADB disabled and ADB unreachable are told apart from the entry's own option.
Discovery is EXPLICIT and single: one package per request, never a sweep over the
installed apps, and a test drives a watchdog cycle and asserts it starts no scan.
Concurrent requests for the same package share one dump.
THE FIXTURE IS A TRANSCRIPTION, NOT IMAGINATION. tests/fixtures/
pm_dump_netflix_resolver.txt reproduces the user's 2026-10-12 capture structure
(whole-dumpsys envelope, the activity table with its Schemes/Non-Data Actions
groups, the receiver/service/provider tables that bound it) and the test file's
docstring states line by line what is VERBATIM from the ticket/plan and what is
reconstructed — no real Shield was available, and the raw capture is not in this
repository (the same limit t13 recorded). The capture's own entries are each
pinned by an assertion: the cast entry, http/https with the four categories and
the five www.netflix.com path globs, nflx:, netflix:, MAIN with LEANBACK_LAUNCHER
+ LAUNCHER, NETFLIX_KEY_START, DIAL_START and the two DEBUG_* noise actions —
18 candidates, deduplicated, with the receiver/provider tables proven unread.
Evidence: 59 new tests in tests/test_intents.py (parser against the transcribed
capture, degradation on shapes never seen, the command shape and quoting safety,
the cache states and TTL, and the manager: one shell, cached, retried on failure,
fallback recovery, ADB-off versus ADB-down, the cap disclosed, the watchdog
silent, concurrent sharing). No device was available: the parser is proven
against the transcription, and every "runs/does not run" claim is an assertion on
the poller's recorded commands rather than on intentions.
Verify: suites 778/4 and 779/3; ruff clean; drift exit 0 (26 modules, 76 imports,
242 relative imports, 1417 statement blocks, 57 call arguments) with the
relative-import floor raised 195 -> 235 and the statement-block floor 1300 ->
1400 by the new module rather than dodged; the real-HA glob still runs green
(56 passed, 0 skipped).
|
||
|
James Mackay
|
14c168c25e |
feat(remote): the app/activity picker, and a protocol transport that really connects (t15)
The user's remote displayed but offered no app picker, and the protocol transport was never actually switched on. Both halves are fixed here. REMOTE PARITY. The entity declares RemoteEntityFeature.ACTIVITY — exactly what core's androidtv_remote declares — and publishes activity_list, which is what makes the frontend render an activity picker at all. The list is the app directory (the user's apps map, else the curated table as a floor) in map order, then the apps ADB discovery found and something can name, in the one name chain (user map > device label > curated > package id). Nothing nameless is listed as a bare package id, and current_activity is None until something is known: no activity is invented. remote.turn_on honours activity= by waking the device when it reports itself off and then launching through the existing launch path; a listed name resolves case-insensitively, a package id or a detail link passes through, and anything else raises unknown_activity naming the value instead of silently doing nothing. An empty/absent activity keeps the previous behaviour exactly, async_turn_off is untouched, and a configured command script still owns the sequence. WHY THE TRANSPORT WAS DEAD, MEASURED. androidtvremote2's keep_reconnecting only starts _async_reconnect, whose whole body is `while self._remote_message_protocol:` — false on the first check with nothing connected, so the task returned immediately: no connect attempt, no retry, and the auth callback was never reachable. HA core connects FIRST and only then keeps it alive. Our _start_protocol called keep_reconnecting after generating a certificate and never called async_connect anywhere, so every send_key_command/send_launch_app_command raised ConnectionClosed internally, the helpers swallowed it into `return False` plus a debug line, and no push (is_available/is_on/current_app/volume_info) ever arrived. The fix is a background connect loop: async_connect under a bounded timeout, network failures retried with the ADB watchdog's jitter and backoff, InvalidAuth routed into the EXISTING reauth callback (entry.async_start_reauth), and keep_reconnecting only after a session is up. Setup never blocks on a transport. A send that cannot run is no longer silent: it is counted, its error recorded verbatim, and logged at WARNING with the transport state, and the remote publishes protocol_state / protocol_connected / protocol_ever_authenticated / protocol_last_error / protocol_failures / protocol_certificate plus sticky per-push *_seen flags — so "sent" and "never switched on" can never look alike again. CASE (b) IS FIXED, NOT DOCUMENTED: switching the transport on in the options flow for an entry that never paired now routes into the real async_step_protocol_pair, and "continue without pairing" persists the transport OFF. One marker carries the pairing (CONF_PROTOCOL_PAIRED), written by the setup pairing step, the reauth finisher and the reconfigure finisher, so no entry can sit enabled-but-unpaired. The certificate from the first pairing is reused by design, so a paired entry is asked for no new PIN — which is also why the marker, not certificate material, is the source of truth. ADB STAYS THE PRIMARY STATE SOURCE. Arming the protocol introduces pushes that were never live before, so the reading now names its tier: manager.foreground_source drives the media player's app_source and the remote's activity_source, and the new tests prove a full protocol push set leaves the media state, the now-playing reading and every ADB store field untouched, while an ADB-only entry still serves media player, sensors, apps, scripts and receiver with only the protocol surface degraded. Copy: unknown_activity, activity_launch_failed and protocol_unavailable in BOTH strings.json and translations/en.json, pinned by a test; the README documents the picker, activity resolution, how to read the transport-state attributes, the script bypass (a script-routed press is never evidence of pairing), the connect-then-keep-alive lifecycle and cert reuse versus the other integration's own pairing. Evidence: 6 new real-HA FlowManager tests in tests/test_config_flow_real_ha.py (unpaired -> pairs, PIN -> marker, skip -> off, paired -> not asked again, and the setup flow's marker on success and on skip), a real-HA entity contract test in tests/test_ha_api_drift.py (exact supported_features equality with core, activity_list/current_activity reaching the state machine, live _attr_ updates) and new suites in tests/test_protocol_transport.py (connect-first, keep_reconnecting alone is a no-op, retry, auth rejection, honest send failures, ADB-primary precedence) and tests/test_command_routing.py (the activity contract, turn_on(activity=), script ownership, the transport attribute states). No device and no browser exist here: device behaviour is inferred from the library and core source, and every UI-facing claim is proven at the entity/registry/metadata level under real Home Assistant. Verify: suites 719/4 and 720/3; real-HA glob (tests/test_adb_command_real_ha.py + tests/test_config_flow_real_ha.py) 56 passed, 0 skipped; ruff clean; drift exit 0 (25 modules, 76 imports, 206 relative imports, 1322 statement blocks, 57 call arguments) with the floors raised 40->70, 100->195, 500->1300 and 52->55. |
||
|
James Mackay
|
6aa3b6bfdd |
feat(adb_command): selectable target, strict resolution, and a real response (t14)
The user's live run could not select his Shield for
nvidia_shield.adb_command, so he ran his diagnostics through the deprecated
androidtv integration instead. Measured against the installed HA (2026.10.1),
the cause is a contract, not a bug in the handler:
* ServiceRegistry.async_register has NO target parameter, so a target cannot be
declared at registration;
* the frontend renders the target picker from the service DESCRIPTION, which
HA builds from services.yaml or from the runtime async_set_service_schema —
and the runtime description WINS.
So async_setup now registers with supports_response=OPTIONAL and publishes
ADB_COMMAND_SERVICE_DESCRIPTION (target: {entity: [{domain: media_player}]},
fields, prose mirrored in strings.json/translations/en.json and pinned against
both by a test). No services.yaml is needed, and none is added.
The target resolution is strict, because the failure mode of guessing is a raw
shell command on the wrong device: exactly one PARTICIPANT, told apart as
no_target (nothing selected), unsupported_target (not a participant / option
off / another integration's entity) and ambiguous_target (two selected — sent
to neither). Accepts what HA actually sends: entity_id as text or as the LIST a
picked target produces, and device_id (an automation targeting the DEVICE),
expanded only through this integration's own registry entries and only to media
players. The output now comes back in the response under adb_response — the key
the user's captures read — while an unreachable Shield stays adb_unavailable,
an error rather than an empty response, and a call that does not ask for a
response is unchanged.
Evidence: 12 real-HA tests (new tests/test_adb_command_real_ha.py) drive the
real service registry, the real get_services websocket payload (the exact JSON
bytes the browser parses: target present, response optional, both fields) and
real async_call for every shape above. t14's CI gate globs tests/test_*real_ha.py
so a new real-HA file cannot skip silently; the skip/empty-collection guards stay
and were executed. The real-HA probe also caught async_entries_for_device being
a MODULE-LEVEL helper (not a registry method) that the stub had hidden, so the
drift gate's owner map now covers async_register, async_set_service_schema and
async_entries_for_device, with the call-argument floor raised 44 -> 52.
Verify: suites 681/4 and 682/3; real-HA 38 + the new file 12 (50 with the glob);
ruff clean; drift exit 0 (54 call arguments).
|
||
|
engineer
|
93d0afcdcf |
test(drift): flag unreachable statements and raise the call-argument floor (t62 F2)
Two gate gaps let the t62 findings ship, and both are closed here. A pure-AST unreachable-statement pass (scan(): pass 6, entry point scan_unreachable()): in every statement block, a statement after return/raise/continue/break can never run. Per-block on purpose — an `else`, an except handler or a nested function is a different block and not dead, so the pass stays signal. Red proof: the pre-fix connection.py, saved from HEAD, is flagged as `connection.py:1113: unreachable-statement: return at line 1110 ends this block`; the live tree is clean. A new counter (statement_blocks, floor 500 of ~1102) keeps the "no findings" verdict from being a pass that stopped parsing. The call-argument floor was 20 while the walk counted 46, so deleting a flow-display owner from HA_CALL_OWNERS did not trip it. It is 44 now, and the suite proves the trip: removing async_show_menu yields 40 checked arguments and `only 40 call_args (floor: 44)`. Measured removals: show_form 25, abort 38, show_menu 40, create_entry 41, update_entry 41 all violate; the single-call-site async_update_reload_and_abort (45) is the one owner a count floor cannot watch, which is why the floor sits two below the count. Also (t62 nit a): the two comments that pinned the t40 entity-attribute finding to ``media_player.py:828`` now name the file and the attribute instead — the line had rotted to an unrelated property. |
||
|
James Mackay
|
3a55519b74 |
fix(config-flow): the setup menu was unreachable under real HA (t57 UNIT 0a)
t61 F1 (BLOCKER) + F2 + F3 + F5, all reproduced against a real FlowManager
before fixing. The OPTIONS flow was verified good and is untouched by this
commit; the SETUP flow was broken in three independent ways.
F1 — `_show_setup_menu` passed `errors=` to `async_show_menu`, which has no
such parameter (HA 2026.10.1: step_id / menu_options / sort /
description_placeholders). Every path that went near the menu therefore
raised `TypeError` the moment the first form was submitted or ADB pairing
was skipped. The path map, driven end to end: FOUR of the five ordinary
paths died before create_entry ("ADB skip", "ADB paired + protocol off",
"ADB skip + protocol skip", "pair ADB"); only "protocol only" survived, and
it never showed the menu — so a new entry could only be created on that one
path and the sub-screens were unreachable.
F2 — the menu's rows broke the rule this repo documents: HA dispatches a row
to `async_step_<row id>` AND dispatches a submitted form by its OWN step_id,
so a row named `adb` with a handler called `async_step_adb_pair` is an
UnknownStep, and a row named `receiver` rendering `step_id="setup_receiver"`
is a form that cannot be submitted. The rows are now their handler names
(`receiver`, `connection`, `adb_pair`, `protocol_pair`, `finish`) and the
two sub-screens render their row id. There is also a `_paired_adb` guard
now, so a completed ADB pairing stops offering that row — the rule the
protocol row already followed.
F3 — both menus keyed their row labels under a bare `menu`, which core does
not read (56 core strings files use `menu_options`), so every row fell back
to its raw id in the browser. Renamed in both strings files.
F5 — the per-command script merge is field-wise now: a `script_<command>`
key that is present and empty removes that command, one with a value sets
it, and a key the submission does not carry is untouched — the same rule as
every other option. The wholesale rebuild depended on the page always being
complete, so a partial submission silently wiped the commands it omitted.
THE SYSTEMIC HALF (F1), closed as asked: `ha_api_drift.HA_CALL_OWNERS` now
covers the flow-display APIs (`async_show_menu`, `async_show_progress`,
`async_show_progress_done`, plus the existing form/abort/entry owners), so
the drift engine resolves their REAL signatures and flags an unknown
keyword. `tests/test_ha_api_drift.py` carries a pre-fix source using
`errors=` to prove the new owner is non-vacuous. Every harness stub now
mirrors the real signature (no `errors` on a menu) and a stub-level test
pins that parameter set, so a stub cannot go back to accepting anything.
On the pre-fix tree the drift engine reports exactly this:
config_flow.py:898: unknown-kwarg: async_show_menu() has no 'errors'
parameter; real signature: (self, *, step_id=None, menu_options, sort,
description_placeholders) -> _FlowResultT
TESTS: tests/test_config_flow_real_ha.py gains a SETUP section that drives
the real flow — the first form, the ADB skip, the pairing guard, EVERY menu
row (dispatch AND re-submittable step_id), an accepted sub-screen
submission, a full connection+receiver+finish walk merging into one entry,
the menu's pairing note, and the five-path map as a parametrised test.
11 of those fail on the pre-fix tree; the drift engine failure above is the
12th. The stub suite additionally pins the menu stub's signature and the
`menu_options` key in both strings files.
Also in this commit (t57 F4): the real-HA file claimed entity-valued pages
could not be driven there, which was false (it has always submitted
`switch.soundbar`). The claim is gone and the scripts page is driven through
SET (two scripts selected, real entity ids) → CLEAR (one selector emptied)
→ SAVE → REOPEN, so the SET half is pinned in the real flow and not only
against the selector objects.
|
||
|
James Mackay
|
130dee4bc6 |
test(ci): sweep every module for dangling imports (t41)
Two defects shipped with a green suite: button.py imported a helper that had been deleted (no harness ever loaded the file), and the drift sweep only judged `homeassistant.*`, so a relative import of a name that no longer exists was invisible to it. Both are now impossible to ship silently. * tests/test_module_coverage.py loads EVERY *.py file of the package, in a clean interpreter, under the stub harness — exhaustive by construction (a new platform is covered the moment the file exists; there is no list and no special case). It asserts the reported set equals the files on disk, so a silently skipped module fails too; * entity_stublib.load_entity_modules() now also loads button.py (the named gap); the sweep is what keeps that list from needing to be complete; * ha_api_drift gained pass 5: the package's own relative imports. Pure AST, no homeassistant required, with its own entry point `scan_package_imports()` for the fast suite — it checks the target file exists and defines every imported name (submodule imports still resolve), instead of trusting that the modules a harness happened to load are the ones that matter; * the file walk in both AST passes is recursive, and the non-vacuity floors moved with it (modules 10 → 20, relative_imports ≥ 100 — a shallow-glob regression trips the floor instead of looking green); * two permanent red proofs, both on a copy of the tree with the reverted `from .helpers import discover_adb_host` reference in button.py: the stub-harness sweep fails with "FAIL button.py", and the static pass fails with "button.py:1: broken-relative-import: … not defined in helpers.py". Manual runs confirm the engine's other direction on the real pre-fix tree ( |
||
|
James Mackay
|
be3abae3e7 |
test(ci): F4 — entity-contract pass in the drift sweep + real-HA state-write proof (t40)
The F1 blocker was invisible to every existing test: the stub harnesses call the
entity and replace ``async_write_ha_state``, so a non-property
``extra_state_attributes`` looked perfectly healthy. Two new gates close that
hole.
1. ``ha_api_drift.scan_entity_contracts()`` — a pure-AST pass (so it needs no
homeassistant, unlike passes 1-3) that walks every class inheriting from an
HA entity base, including our own intermediate ``NvidiaShieldSensorBase``, and
flags any HA-READ attribute (``extra_state_attributes``, ``available``,
``state``, ``native_value``, ``media_*``, …) declared as a plain method
instead of ``@property``. It is wired into ``scan()`` before the
homeassistant gate and exported for the fast suite, so ONE implementation
backs the sweep, the CLI red proof and tests/test_entity_contract.py (the t39
F1 lesson).
Red proof on the pre-fix tree:
media_player.py:828: entity-attribute-is-a-method:
NvidiaShieldMediaPlayer.extra_state_attributes is a plain method, but HA
reads it as an attribute (``attr |= self.<name>``) — declare it @property
…and HEAD is clean (13 modules, 60 imports, 35 call arguments, 7 awaited).
2. tests/test_ha_api_drift.py now builds each entity under REAL homeassistant
(real ``PlatformData``, real ``async_write_ha_state``) and asserts HA's own
``__async_calculate_state`` succeeds and that the media player's app_id/
app_name survive the merge — for the media player, both sensors, the remote
and the pairing button. A coverage test compares that matrix against the
sweep's classification, so a new entity class cannot quietly skip it.
Verified: 296 passed / 2 skipped (fast suite), 16 passed (drift suite under
Python 3.14 + homeassistant 2026.10.1), ruff clean; the entity-contract red
proof above reproduces exactly the reported line.
|
||
|
James Mackay
|
c588a7c66b |
docs(readme): record the MAC-less discovery contract (t37 D3, t39)
t37 made MAC-less zeroconf discovery honour Home Assistant's own `_async_handle_discovery_without_unique_id` contract — the helper is a coroutine and its dedup (abort `already_configured` when ANY entry exists for this domain, abort `already_in_progress` when another flow runs) only happens when it is awaited. That is stricter than the accidental host-equality guard it replaced, so it changes what a user can expect: * with a MAC, discovery still aborts duplicates and updates a moved device's hosts; * without a MAC, a second (or third) Shield must be added MANUALLY — discovery will abort as "already configured". Someone with two Shields would otherwise read that abort as a bug. The README now has a "Discovery (zeroconf)" section under Configuration stating the rule and the manual route, and the drift sweep's docstring carries the same note next to the D3 entry, so the decision is discoverable from wherever you meet it. Verified: 289 passed, 2 skipped; ruff clean. |
||
|
James Mackay
|
fd62d850b7 |
fix(drift): F1+F2 — the sweep refuses false greens, and CI provisions the real floor (t39)
t38 found two holes in the drift gate itself. F2 — the job could never run where it mattered. Home Assistant 2026.10.1 declares ``Requires-Python >=3.14.2`` (PyPI metadata), so the old guard let a 3.13 runner through and the ``homeassistant>=2026.10`` install then failed with a resolver error. The job now provisions its own interpreter (``actions/setup-python@v5`` with ``python-version: "3.14"``) and the guard runs against the REAL floor, read from ``ha_api_drift.HOMEASSISTANT_PYTHON_FLOOR`` — one source of truth, pinned by a test against the installed distribution's own ``Requires-Python`` metadata. Verified: on 3.13.16 the guard refuses with "Home Assistant needs Python >= 3.14.2, but this job runs 3.13.16 / Fix: raise actions/setup-python…"; on 3.14.8 it reports "interpreter ok"; and ``uvx --python 3.14 --with 'homeassistant>=2026.10'`` resolves 2026.10.1. F1 — the standalone entry point was a false green. It printed counters and exited 0 even when it had walked nothing, which is exactly how the manual red-proof tool gets misused. The non-vacuity floors now live in ``ha_api_drift.FLOORS`` and are enforced in ONE place, ``Report.floor_violations()``, used by the pytest wrapper and by ``__main__`` (so the constants cannot drift apart; a test asserts the wrapper holds no floor-like constants of its own). On top of that the sweep can no longer lie in either direction: * an empty or mis-targeted directory reports ``FLOOR:`` lines and ``drift sweep FAILED`` and exits 1 (a non-existent path is a ``bad-path`` finding, not a traceback and not a silent zero-module sweep); * with no homeassistant installed it returns a single ``no-homeassistant`` finding with zero counters — never a wall of phantom "broken-import"s, never a green exit. Verified in the CI-equivalent environment (Python 3.14.8 + homeassistant 2026.10.1): HEAD exits 0 with "no Home-Assistant API drift (13 modules, 60 imports, 35 call arguments, 7 awaited HA calls)"; the empty directory exits 1 with 4 FLOOR lines and no findings; a missing path exits 1 with the ``bad-path`` finding; and the full pre-fix tree (``git archive 5ede4fe``) exits 1 with exactly the three t36 drifts (media_player.py:965 BrowseMediaSource, diagnostics.py:93 homeassistant.util.diagnostic, config_flow.py:425 missing-await) and 0 floor violations. The fast ``test`` job and ``release.yml`` are untouched; the workflow is unchanged in shape except the new provisioning + guard steps, and parses as valid YAML. |
||
|
James Mackay
|
824d6192a1 |
test(ci): D1/D2/D3 — repo-owned HA API-drift sweep with a CI job (t37)
The unit suite validates production modules against hand-written stubs, so a
call site that no longer matches real Home Assistant stays green — that is how
D1, D2 and D3 reached master. This adds the check that would have caught them.
`tests/ha_api_drift.py` (stdlib-only, documented inline) parses every module of
custom_components/nvidia_shield and resolves each Home-Assistant dependency
against an INSTALLED homeassistant:
1. every `homeassistant.*` import — module present, attribute present (or a
submodule of that name exists, which is what `from package import submodule`
means) — catches D1/D2 and any future move;
2. every keyword argument passed to a known HA API (HA_CALL_OWNERS: the
ConfigFlow/ConfigEntries APIs we call) must exist in the real signature —
catches the `options_updates` class of bug;
3. every `self.<method>()` call in a class that inherits from an HA class is
compared with the real definition: a coroutine must be awaited and an
awaited call must be a coroutine — catches D3.
`tests/test_ha_api_drift.py` runs it against the production tree, asserts the
sweep is not vacuous (module/import/argument/await counters), and pins the red
proof with the four pre-fix forms, plus the green proof for the fixed forms. It
skips cleanly when homeassistant is absent, so the stdlib-only jobs are
unaffected.
`.gitea/workflows/test.yml` gains the `ha-api-drift` job — the one place Home
Assistant is installed — guarded by an explicit Python >= 3.13 check (Home
Assistant 2026.x requires it) and the same PEP 668 retry the test job uses.
Red proof against the real pre-fix revisions (svn-style checkout of
`git show HEAD:…` into a temp dir, and the pre-t35 config_flow.py for the kwarg):
media_player.py:965: broken-import: from homeassistant.components.media_player
import BrowseMediaSource: …BrowseMediaSource does not exist
diagnostics.py:93: broken-import: from homeassistant.util.diagnostic import
async_redact_data: module 'homeassistant.util.diagnostic' is not importable
config_flow.py:425: missing-await: self._async_handle_discovery_without_unique_id()
is a coroutine on homeassistant.config_entries.ConfigFlow
config_flow.py:700: unknown-kwarg: async_update_reload_and_abort() has no
'options_updates' parameter (exit 1 on that tree, exit 0 on this one)
Verified: variant 1 289 passed / 2 skipped; variant 2 (androidtvremote2) 290
passed / 1 skipped; ruff clean; `UV_PYTHON=3.13 uvx --with homeassistant --with
pytest-asyncio pytest tests/test_ha_api_drift.py -q` → 4 passed (the bare form of
that command needs the pin here only because this sandbox's default Python is
3.15 and pycares has no wheel for it / no CMake to build it; CI pins the
interpreter explicitly).
|