From df7ebf1dc1183a7245b005b118c7bc220751f41e Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Sun, 7 Jun 2026 08:32:03 -0400 Subject: [PATCH] sandbox/docs: add doc-audit research notes (ARCHITECTURE/OVERVIEW vs code) Dated audit snapshots (2026-06-05) cross-checking every concrete name / RPC / routing rule / table row in ARCHITECTURE.md and OVERVIEW.md against the implementation. Kept as research artifacts under plans/research/. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../plans/research/doc-audit-architecture.md | 130 ++++++++++++ sandbox/plans/research/doc-audit-overview.md | 195 ++++++++++++++++++ 2 files changed, 325 insertions(+) create mode 100644 sandbox/plans/research/doc-audit-architecture.md create mode 100644 sandbox/plans/research/doc-audit-overview.md diff --git a/sandbox/plans/research/doc-audit-architecture.md b/sandbox/plans/research/doc-audit-architecture.md new file mode 100644 index 000000000000..1de2dd4d5393 --- /dev/null +++ b/sandbox/plans/research/doc-audit-architecture.md @@ -0,0 +1,130 @@ +# Doc audit — `sandbox/ARCHITECTURE.md` vs. code + +Audited 2026-06-05. Doc path: `/home/paulus/dev/hass/core/sandbox/ARCHITECTURE.md`. +Method: every concrete name / RPC / routing rule / table row / core touch-point +checked against the implementation. + +## Verified accurate + +Near everything the doc names checks out exactly: + +- **Main-side components table (§2):** `SandboxFlowRouter` + (`router.py:47`), `SandboxManager` (`manager.py:527`), `SandboxBridge` + (`bridge.py:162`), `classifier.py` (`classify`, `classifier.py:58`), + `sources.py` (`async_register_sandbox_source_resolver`, `sources.py:68`). +- **Sandbox-side components table (§2):** `SandboxRuntime` + (`sandbox/__init__.py:57`), `FlowRunner` (`flow_runner.py:87`), + `EntryRunner` (`entry_runner.py:27`), `EntityBridge` (`entity_bridge.py:38`), + `ServiceMirror` (`service_mirror.py:38`), `EventMirror` (`event_mirror.py:73`), + `ApprovedDomains` (`approved_domains.py:35`), `ChannelSandboxBridge` + (`sandbox_bridge.py:31`, doc calls it the store bridge per §8). `_SandboxFlowManager` + exists (`flow_runner.py:65`). +- **Routing (§3):** classifier order matches `classifier.py:58-76` exactly + (`integration_type=="system"`, `ALWAYS_MAIN`, `SANDBOX_INCOMPATIBLE_PLATFORMS`, + custom→`custom`, else→`built-in`; uses `platforms_exists` + `is_built_in`). + `SANDBOX_INCOMPATIBLE_PLATFORMS` is exactly the 6 named (`const.py:29-44`). + `ALWAYS_MAIN` behavioural punts + the 18 lockdown helpers match `const.py:47-108` + verbatim. +- **Channel/transport (§4):** `ProtobufCodec` (`codec_protobuf.py:25`), + `JsonCodec` + `Transport`/`StreamTransport` (`channel.py:173/223/243`), + 4-byte big-endian length-prefix, `from_transport` seam, `Ready` frame + (`sandbox/ready`), `ws://` reserved→`NotImplementedError` + (`sandbox/__init__.py:242`), inflight semaphore. `.proto` regen via + `proto/generate.sh` + `check_drift.sh` present. +- **Lifecycle (§5):** restart budget `3` / `60.0s` / backoff / `failed` state + (`manager.py:47-48`, 288-303). `python -m hass_client.sandbox --name --url stdio://` + form matches (`--name`, not `--group`). +- **Flow forwarding (§6):** `SandboxFlowProxy(ConfigFlow)` (`proxy_flow.py:48`); + `create_result["sandbox"] = self._sandbox_group` (`proxy_flow.py:213`) confirms + "main overwrites the group". `schema_bridge.py` rebuilds real `selector.Selector` + / `data_entry_flow.section` (`schema_bridge.py:69-92`). +- **Statelessness / source (§7):** `IntegrationSource` proto (`sandbox.proto:99`), + `kind: builtin|git` with `url/ref/tag/domain/subdir`, resolver hook + + `is_built_in` short-circuit + raise-on-missing-resolver (`sources.py`). +- **Entity bridge (§8):** "all **32** domains have one under `entity/`" — exactly + 32 proxy files. `EntityDescription`/`EntityInfo`/`InitialState`, + `register_entity` upsert, `:` prefix, `vol.Invalid` + rebuild from `error_data` all present. The "coalescing… is a noted future + optimisation" wording matches the just-landed batcher removal + (`bridge.py:231-232`, no batcher in code). +- **Context (§8):** 15-min TTL `_CachedContext` cache, `context_id`-only wire, + fresh `Context(user_id=None)` on miss — matches `bridge.py:95` and the proto + (no `parent_id`/`user_id` on the wire). +- **Store routing (§9):** `current_sandbox` ContextVar in + `helpers/sandbox_context.py`, read by `Store._async_load_data` / + `_async_write_data` / `async_remove` (`storage.py:361,601,650`). +- **Core touch surface (§11):** `config_entries.py` `ConfigEntryRouter` Protocol + (`config_entries.py:2121`) + `router` attr (`:2161`) + `ConfigEntry.sandbox` + field (`:432`); `EntityComponent.async_register_remote_platform` + (`entity_component.py:207`); `sandbox_context.py` + `storage.py`. + +## Drift / inaccuracies + +Effectively none in the literal claims that exist — every named symbol, RPC, +constant, and count checked out. The only inaccuracy is one of *completeness*, +which manifests as a now-false framing claim: + +- **§11, line 293 — "three surfaces" is now stale.** + > "The sandbox is deliberately small against core HA — **three surfaces**…" + Translation forwarding (merged after this doc's last revision) added **two more + core touch-points** not listed: + - `homeassistant/helpers/translation.py` — + `async_register_sandbox_translation_provider` (`:501`), + `_async_overlay_sandbox_strings` (`:304`), `async_invalidate_translations` + (`:526`), `SandboxTranslationProvider` callable type (`:43`). + - `homeassistant/loader.py` — `async_register_sandbox_catalog_provider` + (`:455`), `async_get_sandbox_catalog` (`:476`), `SandboxIntegrationDescriptor` + (`:416`), and the catalog merge inside `async_get_integration_descriptions` + (`:536-539`). + **Severity: stale/misleading.** The "three surfaces" number is now wrong; the + doc's own framing ("each a declared public hook") would apply cleanly to these + two as well, so they belong in the list. + +## Omissions + +The dominant finding: **the entire translation-forwarding subsystem is absent +from the doc body.** "translations" appears only in the Goal (§1, line 22: +"…events, and translations available on main…") as an aspiration; line 220's +"Exception translation" is unrelated (it is `vol.Invalid` rebuild). Nowhere in +the body, the §2 component tables, the §4 RPC inventory, §11 core touch surface, +or §13 future-work list is the mechanism described. Yet the code is fully +shipped and wired end-to-end: + +- **Wire:** `GetTranslations` / `GetTranslationsResult` proto messages + (`sandbox.proto:162-176`); `sandbox/get_translations` registered in both + `messages.py` mirrors (`:41`) and both `protocol.py` mirrors + (`MSG_GET_TRANSLATIONS`). +- **Sandbox side:** handler `_handle_get_translations` + (`hass_client/sandbox/__init__.py:262`), registered at `:198`. +- **Main side:** `components/sandbox/translation.py` (`SandboxTranslationProvider`, + resolves group, batches custom domains into one RPC/language, degrades to + empty under the cache lock); `components/sandbox/catalog.py` (re-exports the + loader catalog hook for the add-integration picker); registered in + `components/sandbox/__init__.py:106-107`. +- **Core:** the `translation.py` + `loader.py` hooks above. + +**Does it matter for a "current architecture" reference?** Yes. The doc's +masthead (lines 3-7) claims to be the "**final, current architecture**" and a +"state-of-the-system reference." A reader using it to understand how a +sandboxed custom integration's frontend strings (entity names, config-flow +labels, services, exceptions) reach main would find the Goal promises it but no +section explains it — and would be actively misled by the "three surfaces" +count in §11. This is the single material gap. + +Minor omissions (lower stakes, arguably fine for an overview-level doc): + +- §2 main-side table omits `translation.py` / `catalog.py` (companion to the + §11 gap). +- Picker/catalog integration (loader merge) for sandbox-only customs is + undocumented; relevant to anyone wiring HACS into the picker. + +## Verdict + +ARCHITECTURE.md is **substantially accurate** on everything it describes — +every name, RPC, routing rule, constant, and count verified against code, with +zero literal contradictions found. Its one real defect is an **omission**: the +shipped translation-forwarding feature (proto RPC + main provider + catalog + +two core hooks) is entirely missing from the body, which also falsifies the +§11 "three surfaces" claim. To be the "final, current architecture" it must add +a translation-forwarding section, list `translation.py`/`catalog.py` in §2, and +bump §11 to five surfaces. diff --git a/sandbox/plans/research/doc-audit-overview.md b/sandbox/plans/research/doc-audit-overview.md new file mode 100644 index 000000000000..0d9818a53966 --- /dev/null +++ b/sandbox/plans/research/doc-audit-overview.md @@ -0,0 +1,195 @@ +# Doc audit — `sandbox/OVERVIEW.md` vs. implementation + +Audited 2026-06-05 against the working tree. Doc path: +`/home/paulus/dev/hass/core/sandbox/OVERVIEW.md`. + +## Verified accurate + +- **Classifier rule order & structure.** `classify()` matches the doc's + five-rule order exactly: `integration_type == "system"` → main, `ALWAYS_MAIN` + → main, `SANDBOX_INCOMPATIBLE_PLATFORMS` intersection → main, custom → + `Sandbox("custom")`, else `Sandbox("built-in")` + (`homeassistant/components/sandbox/classifier.py:58-76`). Uses + `platforms_exists()` (line 67), so no import — as the doc states (line 137). +- **`SANDBOX_INCOMPATIBLE_PLATFORMS` list.** Exactly `stt, tts, conversation, + assist_satellite, wake_word, camera` (`const.py:29-44`) — matches doc line 118. +- **Three groups (`main`/`built-in`/`custom`).** `classifier.py:31-32`, + group table at OVERVIEW lines 123-129 is correct. +- **Spawn command.** `manager._default_command` builds `[python, -m, + hass_client.sandbox, --name , --url ]` + (`manager.py:657-672`); `__main__.py:21-34` defines `--name` (required) and + `--url` (default `stdio://`). Doc lines 148-152 accurate (doc's `` = the + group). +- **Transport scheme handling.** `stdio://` default, `unix://` opt-in, + `ws://`/`wss://` reserved + rejected with `NotImplementedError` + (`hass_client/sandbox/__init__.py:236-246`, `_transport_scheme` 410-427). + Matches doc lines 154-166. +- **Ready handshake.** Runtime pushes `MSG_READY` (`sandbox/ready`) as first + frame (`hass_client/sandbox/__init__.py:193`), manager treats arrival as + running; no stdout text marker. Matches doc lines 158-161. +- **Crash-recovery budget.** `DEFAULT_RESTART_LIMIT = 3`, + `DEFAULT_RESTART_WINDOW = 60.0`, `DEFAULT_RESTART_BACKOFF = 1.0` + (`manager.py:47-49`); `_supervise` sliding-window pop + `state="failed"` on + budget exhaustion (`manager.py:287-333`). Doc lines 170-175 accurate **except + the SETUP_RETRY claim — see Drift**. +- **Graceful shutdown sequence.** `_on_stop` runs + `async_graceful_shutdown_all(timeout=shutdown_grace)` then `async_stop_all()` + (`__init__.py:111-125`); runtime `_handle_shutdown` snapshots restore-state, + replies, then `call_soon(self._shutdown.set)` + (`hass_client/sandbox/__init__.py:248-353`); `_on_shutdown_reply` writes + `core.restore_state` via the bridge store server (`__init__.py:63-91`). + Matches doc lines 184-199. Warm-load via `current_sandbox` before handlers + register (`hass_client/sandbox/__init__.py:177-194`, `_load_restore_state` + 386-407) — matches doc lines 201-209. +- **Config-flow router: three call sites.** `ConfigEntries.router` consulted + from `async_create_flow`, `async_setup`, `async_unload` (per + `sandbox/CLAUDE.md` and verified import surface). Proxy issues + `flow_init`/`flow_step`/`flow_abort`; `_adapt_result` attaches + `sandbox=` on CREATE_ENTRY (`proxy_flow.py:15-17`), stored on + `ConfigEntry.sandbox`. `unique_id` propagation present. Doc lines 211-256 + accurate. +- **Integration source.** `sources.py` resolver hook: `is_built_in` + short-circuits to `{kind: "builtin"}` (line 103-104); no-resolver custom + **raises** `SandboxSourceError` (line 111); `tag` logs-only. Matches doc lines + 258-283. Sandbox side `hass_client/sources.py` exists. +- **Entity bridge Option B + 32 proxies.** `entity/` holds exactly **32** + domain files (excluding `__init__.py`), matching the named list (doc lines + 356-364). Device-registry bridging via `dr.async_get(...).async_get_or_create` + on `device_info` (`bridge.py:366-373`). `register_entity` upsert + namespaced + `unique_id` (`:`, `const.py:21`). Doc lines 295-373 + accurate. +- **Store routing.** `Store._async_load_data` / `_async_write_data` / + `async_remove` all read `current_sandbox` and delegate + (`helpers/storage.py:361-366, 600-610, 644-652`); branch at + `_async_write_data` not `async_save` (doc line 466 correct). `current_sandbox` + ContextVar + `SandboxBridge` Protocol in `helpers/sandbox_context.py`. Key + validation `_require_key`. Doc lines 462-502 accurate. +- **Auth: no credential.** `auth.py` is gone; no `--token` argv (`__main__.py` + has only `--name`/`--url`/`--log-level`); no `SANDBOX_TOKEN`. Context + restoration TTL cache on the bridge. Doc lines 410-450 accurate. +- **`JsonCodec` retained for channel-core tests only.** `channel.py:173-181` + confirms ("registry-free test/debug wire … stays for the channel-core tests + only"). Doc lines 163-164 accurate. +- **Protocol mirror parity.** `protocol.py` (main) and + `hass_client/protocol.py` are verbatim mirrors, both now carrying + `MSG_GET_TRANSLATIONS = "sandbox/get_translations"`. + +## Drift / inaccuracies + +### 1. `ALWAYS_MAIN` list is badly out of date — HIGH severity +- **Doc claim (lines 111-113):** "Hand-picked deny-list: `script`, + `automation`, `scene`, `cloud`, `ai_task`, `image`." +- **Actual (`const.py:47-109`):** the frozenset has **24** entries: + `script, automation, scene, cloud, ai_task, image` **plus** `template, + group, homekit, min_max, statistics, trend, threshold, derivative, + integration, utility_meter, filter, mold_indicator, bayesian, + generic_thermostat, generic_hygrostat, switch_as_x, history_stats, + proximity`. These are the "broad readers / source-entity helpers" added for + built-in lockdown breakage (referenced in + `plans/research/builtin-lockdown-breakage.md`). +- The doc lists fewer than a third of the real deny-list and gives no hint that + a whole class of "reads foreign entities/registries" integrations is forced to + main. This is a routing-rules claim — the most checkable, highest-value part + of the doc — so the omission is material. + +### 2. Failed-sandbox setup surfaces `SETUP_ERROR`, not `SETUP_RETRY` — MEDIUM +- **Doc claim (lines 173-175):** exhausting the restart budget "transitions the + sandbox to `failed` and `ensure_started` raises `SandboxFailedError` — the + router surfaces this as `SETUP_RETRY` on the affected entries." +- **Actual (`router.py:91-103`):** `async_setup_entry` catches the + `ensure_started` exception and sets + `ConfigEntryState.SETUP_ERROR` ("Sandbox failed to start"), **not** + `SETUP_RETRY`. (`SETUP_RETRY` is used elsewhere — only for a `ChannelClosedError` + *during* `entry_setup`, `router.py:133-138`.) The doc conflates the two paths. + +### 3. "Periodic 30s ping loop is wired through but currently disabled" — MEDIUM +- **Doc claim (lines 177-180):** "A `sandbox/ping` handler is registered and + exercised by the subprocess test … the periodic 30s ping loop is wired through + but currently disabled." +- **Actual:** the `sandbox/ping` *handler* exists on the sandbox side + (`hass_client/sandbox/__init__.py:195, 458-460`) and the proto pair is in the + registry (`messages.py:43`). But there is **no ping loop of any kind in + `manager.py`** — `grep -i ping manager.py` finds nothing. The claim that a 30s + loop is "wired through" overstates reality: nothing on the manager ever sends a + ping. Only the on-demand handler + test round-trip exist. ("Disabled" is + arguably defensible, but "wired through" is not — there is no loop code.) + +### 4. Stale "Status" banner & phase framing — LOW (whole-doc) +- **Doc claim (lines 3-23, 532-536, and the "Where the design is still open" + section):** the doc frames itself as "Complete through Phase 20" and treats + translation forwarding as nonexistent. The translation-forwarding feature + (below) has since merged, so the phase narrative no longer captures the + shipped surface. Not a code-contradiction per se, but the banner now + under-describes the system. + +## Omissions + +### A. Translation forwarding is entirely undocumented in the body — HIGH +The recently merged translation-forwarding feature is fully wired and verified, +but OVERVIEW.md mentions translations **only** in the Goal line (line 30, +"…and translations"). Nothing in the body describes the mechanism. Verified +pieces the doc should cover: + +- **Main provider hook:** `helpers/translation.py` — + `async_register_sandbox_translation_provider` (line 501), + `_async_overlay_sandbox_strings` (line 304, merges overlay before + `_build_category_cache`), `DATA_SANDBOX_TRANSLATION_PROVIDERS` registry (line + 48), `async_invalidate` (line 189). +- **Sandbox component provider:** `components/sandbox/translation.py` + (`SandboxTranslationProvider`): resolves owning group (loaded entry's + `sandbox` field, else in-progress `SandboxFlowProxy.sandbox_group`), built-in + carve-out via `is_built_in`, batches custom domains per group into one + `sandbox/get_translations` RPC per language, 5s timeout → degrade-to-empty. +- **Registration:** `components/sandbox/__init__.py:104-109` registers the + provider in `async_setup` and `_on_stop` unregisters it (line 121). +- **Catalog hook (picker):** `components/sandbox/catalog.py` re-exports + `loader.SandboxCatalogProvider` / `SandboxIntegrationDescriptor` / + `async_register_sandbox_catalog_provider`; `loader.async_get_integration_descriptions` + merges sandbox-only customs into the picker (`loader.py:536-549`). +- **Wire:** `MSG_GET_TRANSLATIONS` in both `protocol.py` mirrors; proto pair + `(GetTranslations, GetTranslationsResult)` in both `messages.py` registries + (main `messages.py:41`). +- **Sandbox-side handler:** `hass_client/sandbox/__init__.py:262-283` + (`_handle_get_translations`) + `_collect_component_strings` (356-383), reusing + core's `_async_get_component_strings` to pre-fill `title` from + `integration.name`. + +**Why it matters:** the doc is a source-linked deep reference with dedicated +sections for every other surface (config-flow forwarding, entity bridge, +service/event mirror, store routing, integration source). Translations are now a +peer surface — a main core hook, a new RPC, a config-flow-aware group resolver, +a picker catalog hook — and warrant their own section. + +### B. "Where to look in the code" table has no Translations row — HIGH +The table (lines 602-613) maps every concern to HA-core + sandbox files but +omits translations entirely. Missing row, e.g.: +`Translations | translation.py, catalog.py, helpers/translation.py, loader.py | +hass_client/sandbox/__init__.py (_handle_get_translations)`. + +### C. Config-flow / entity translation behavior unmentioned — MEDIUM +The "How the sandbox differs from v1" table (lines 42-52) has no translations +row, and the Config-flow section (211-256) never notes that frontend form +labels for a sandboxed custom integration are served via the overlay (without +it they'd silently vanish — the exact gap `translation.py`'s docstring +describes). The picker `title`/name fallback (catalog hook) is likewise +unmentioned. + +### D. `ALWAYS_MAIN` "Open follow-ups" only lists `ai_task, image` — LOW +The "Non-idempotent service handlers" follow-up (lines 551-554) and +`sandbox/CLAUDE.md` still frame `ALWAYS_MAIN` as the small `ai_task`/`image` +punt; neither acknowledges the broad-reader lockdown additions now in `const.py` +(ties to Drift #1). + +## Verdict + +OVERVIEW.md is **substantially accurate** on architecture, lifecycle, transport, +entity bridge (32 proxies), store routing, auth, and config-flow forwarding — +the structural backbone all checks out. But it has **one material routing +error** (the `ALWAYS_MAIN` deny-list is a third of its real size), **two +smaller lifecycle inaccuracies** (failed-setup state is `SETUP_ERROR` not +`SETUP_RETRY`; there is no ping loop at all, not a "disabled" one), and a +**whole-feature gap**: translation forwarding (shipped, fully wired, verified) is +absent from the body, the "differs from v1" table, the config-flow section, and +the "Where to look in the code" table. Must change: rewrite the `ALWAYS_MAIN` +paragraph, fix the SETUP_RETRY/ping claims, and add a Translations section + +table row.