master
5
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
|
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
|
66846b2f82 |
test(intents): the button's uninstalled-app press, with the device's own words (t17)
Acceptance item 3 named this case explicitly: a registered intent whose app is
gone must fail the press HONESTLY. The device's sentence is what the error
carries ('does not exist', naming the component), the press is attempted exactly
once, and it raises rather than doing nothing.
|
||
|
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
|
259a6c29bd |
feat(intents): the Intents options screen, the seed, the strings and the tests (t17, part 2)
Completes the intents feature: part 1 shipped the engine, the buttons and the
action; this is the surface a user actually touches, plus the evidence.
THE SCREEN (config_flow.py). A new `intents` row on the Configure menu opens the
same editor shape as the Apps table, because HA has no table selector: a menu of
row actions, one row per step. Add and edit share ONE form, reached through a
select of the registered intents whose last option is an explicit "Add a new
intent" sentinel — so adding is a choice the user can see, not an empty-string
convention. The per-intent form is name, package, component OR action,
categories, a data template and the declared variables as text
(`key:type[:required][=value]`, blank lines free, `#` comments). Removal asks for
confirmation and takes nothing until the user confirms. "Scan an app's intents"
asks for ONE package (a dropdown of discovered apps with free text), runs the
device-side scan, and offers the candidates; each candidate is marked when it
looks like an internal hook, and a candidate whose path is a PATTERN prefills no
URL with a note saying why, because a pattern is not a link. A scan that cannot
run reports t16's status vocabulary in words a user can act on — ADB off, ADB
down, or the app really advertising nothing are three different sentences.
Nothing half-stored: a submission that fails writes NOTHING and the form comes
back naming each problem (a component without its slash, a package with a space,
a duplicate name, a data template using an undeclared placeholder, a fields line
that does not parse), and a successful edit REPLACES the row where it already is
instead of moving it to the end of the list under the user's eyes.
The screen's reader of `registered_intents` is a second implementation of the
runtime's one — deliberately, because config_flow must stay importable without
Home Assistant's media_player platform. A test drives BOTH over the same battery
of rows (valid, junk, duplicate id, mapping-shaped blob, over-long list) and
requires identical results, so the two cannot drift into meaning different
things.
THE SEED. `registered_intents:` in `configuration.yaml` is validated by the SAME
validator the form uses, so a seeded intent is identical to a typed one; a row it
refuses is dropped with a warning NAMING the row and the reason (a seed is
configuration.yaml: there is no form to show an error on, and a silent skip would
be indistinguishable from a typo). It is honoured only while the UI holds nothing
— at creation, because the Intents screen never writes a seed back and the import
step aborts for a known host first.
THE STRINGS (both files, byte-identical) carry every new step, field, menu row
and the action's own description, plus the exceptions the action raises. The two
shared target errors (`unsupported_target`, `ambiguous_target`) now interpolate
`{service}`, because `send_intent` reuses them verbatim and "for this action" was
`adb_command`-specific copy.
THE TESTS (+72 in tests/test_intents_registration.py, and repaired coverage in
test_command_routing.py). The command: the exact `am start` line per declared
type, a variable containing `"; rm -rf /"` reaching the device as ONE quoted
argument (asserted through shlex, not by eyeballing), a value that looks like a
placeholder being data rather than a second substitution, `{id.__class__}` left
alone because there is no format()/eval() path at all, and each refusal naming
the field. The scan: the status sentences, the candidate prefill, the pattern
case, an internal-looking candidate still being offered. The seed: warned drops,
and the seeded row being byte-identical to the form's. The buttons: one entity
per registration, its identity keyed by the intent's id so a rename does not
orphan it, defaults sent, and a press that cannot reach the Shield RAISING
instead of doing nothing.
test_command_routing's registry test used to hand-write the dict and set the OLD
attribute name, which is exactly why it never noticed that the add hook registered
`None` for a real entry and that the remove hook dropped nothing. It now drives
the real hooks, and pins both registries plus the ruling that registered intents do
NOT require the adb_command opt-in (a raw shell door and a registered intent are
different consents).
Verify: 868 passed / 4 skipped (was 794+4 before this ticket's tests — the new
tests are additive), ruff clean, both strings files byte-identical.
|