Files
goodwe-addon/goodwe_controller/DECISIONS/UX-02-two-config-surfaces.md
T
glenn schrooyenandClaude Opus 5 fdaf3fb54f
checks / unit (pull_request) Successful in 6m17s
UX-02: the validator's refusals, next to the field, before the save
The Configuration tab can only render types and ranges. `validate.py`'s most
useful refusals are CROSS-FIELD - two inverters naming one `setpoint_entity`, a
`maintenance_charge_w` above `max_w`, a flat `setpoint_entity` left set beside a
declared roster - and the Supervisor's UI has no way to express a rule spanning
two keys. Today the operator meets those as exit 78 in a log, after saving and
restarting. This puts them on the screen beside the field they name, before
anything is written.

WHAT IT ADDS
  app/configui.py     the field model, built from `GET /addons/self/info`'s own
                      `schema` + `translations`. No option key, no label, no
                      rule of its own. UX-01's `Group · ` label prefix becomes a
                      real section heading, in whichever language resolved.
  app/selfconfig.py   the Supervisor conversation and the write gate.
  app/config_page.py  the page. Its longest function decides which HTML input a
                      type deserves; it contains no rule.
  app/web.py          five routes. Still the only file a reviewer has to read to
                      answer "does anything here reach the inverter".

THE FOUR THINGS THAT COULD HURT SOMEBODY, AND HOW EACH IS HELD
  1. A blob the SHIPPED validator refuses is never written - not warned about,
     not written. An invalid blob plus a restart is exit 78, and the panel is
     gone with the add-on that serves it.
  2. A blob that moved underneath is refused, not clobbered. Read hands out a
     sha256 of the canonical blob, save hands it back, and the stored blob is
     re-read IMMEDIATELY BEFORE the write. This is SAFETY-28's discipline
     applied to configuration - "a blob reaches the Supervisor only if it was
     derived from the blob stored right now" - with the honest weakening stated
     in the decision record: a second writer is DETECTED, not made
     inexpressible.
  3. Every message is the validator's own, byte for byte. test_selfconfig.py
     part 6 inserts ONE new rule into a COPY of validate.py, hashes every file
     under app/ to show validate.py is the only one that differs, and serves the
     real /config/validate route out of that copy in a subprocess. The new
     sentence arrives, filed under the key it names.
  4. Nothing on the surface commands an inverter or clears a latch. Saving does
     not restart: applying costs a control interruption - the wind-down commands
     0 W on every inverter - so that is a separate, confirmed button that says
     so first.

MEASURED, NOT CITED (tools/supervisor_selfapi_probe.py, run against
Supervisor 2026.08.0's own code; it contacts nothing)
  - the three self-paths are permitted, and `/addons/self/info` is admitted by
    the `default` role too while `options` and `restart` rest on the bypass
    ALONE. The probe's first draft claimed the role admitted none of the three
    and went red.
  - `/addons/self/options/validate` is NOT reachable from inside an add-on, so
    the only validator this panel can call is ours - which is the one with the
    cross-field rules.
  - a POST that omits a key does not preserve it: the key reverts to
    config.yaml's default. Hence whole-blob read-modify-write.
  - the Configuration tab CAN be removed, and doing so drops 0-of-71 options
    through and takes the add-on down. The card said it could not be; it stays
    by decision, which is a better reason.

DOES NOT TOUCH config.yaml, main.py or any control path. UX-01 is a dependency,
not something this supersedes: its translations are where every label,
description and section heading on this page comes from, and the Configuration
tab is still the recovery surface. DECISIONS/UX-02-two-config-surfaces.md
records which surface is authoritative (neither: the stored blob is) and what
the second surface costs.

Also corrects a stale "70 options" in three prose spots to the 71 the probe
counts - config.yaml grew a key in SAFETY-42 and the sentences did not follow.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU

---

⚠️ REBASED ONTO release/1.0 @ 3368231, AND HISTORY DELIBERATELY FLATTENED.
The original branch carried `653874d` - an ORPHANED first attempt at PKG-07
whose commit message justified the version bump with a claim that was FALSE
(that `slew_w: 0` used to mean "no limit"; the schema was `int(50,5000)` from
the repo's first commit, so 0 was never legal). That commit's CONTENT was
superseded when release/1.0 was merged in, and the merge resolved correctly -
one 0.8.0 section, no fabricated text. But the retracted claim would have
entered release/1.0's history as a commit message with no retraction attached
to it, where PKG-07's own merged commit is the thing that retracts it.

So the branch is replayed as a single commit on top of the corrected PKG-07
rather than merged with the bad one trailing behind. The tree is byte-identical
to the reviewed branch tip; only the ancestry changed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
2026-09-06 21:38:31 +02:00

208 lines
11 KiB
Markdown

# UX-02 — two configuration surfaces, one blob, and which one is authoritative
**Status: DECIDED 2026-09-06 — the STORED OPTIONS BLOB is authoritative and
neither user interface is.** The ingress panel is the surface an operator
should normally use, because it is the only one that can show the add-on's
cross-field refusals *before* a save. The Supervisor's Configuration tab stays,
unchanged, and is the **recovery** surface: it is the only one of the two that
still exists when the add-on is not running.
Filed with UX-02. Card 6a9afc52d4e3662463274e9d. Depends on [[UX-01]], which is
not superseded by this — see §4.
---
## 1. The question
The owner asked, 2026-09-04: *"can't we use some kind of webUI in HA to have a
cleaner config with custom styling?"* Yes — `config.yaml` already declares
`ingress: true`, `ingress_port: 8099` and `hassio_api: true`, and the panel is
live. But building a second editor for the same settings creates a question the
first one never had: **when two surfaces can write one blob, which one is
right?**
Getting that wrong is not a cosmetic problem. `POST /addons/self/options`
*replaces* the stored user options, so a page that writes a blob it read five
minutes ago silently reverts whatever the other surface did in between.
## 2. What was measured, not assumed
All of it out of `ghcr.io/home-assistant/amd64-hassio-supervisor:2026.08.0` —
the version this site runs — by `tools/supervisor_selfapi_probe.py`, which runs
Supervisor's own code rather than re-reading it. It touches nothing: no
Supervisor is contacted, no options are posted and nothing is restarted.
**(a) An add-on may read and write its own options, and needs no role.**
`_V1_PATTERNS.api_bypass` was run against the exact paths. `/addons/self/info`,
`/addons/self/options` and `/addons/self/restart` all match; `security` and
`update` do not. `token_validation()` tests that bypass *before* it consults
`access_hassio_api` or any role, so no `hassio_role` is involved.
Two results the card did not predict, and both changed the design:
* `/addons/self/options/validate` — Supervisor's **own** options validator — is
**not** reachable: it is two path segments after `self`, so `[^/]+` cannot
match it, and it is not one of the listed literals. There is exactly one
validator an add-on can call from inside itself, and it is `app/validate.py`.
Which is the one we want: Supervisor's knows only types and ranges.
* `/addons/self/info` is admitted by the `default` role *as well as* by the
bypass (`/.+/info`), while `/addons/self/options` and `/addons/self/restart`
are admitted by the **bypass only**. The two *writing* paths rest on that one
list. The first draft of the probe asserted that the role admitted none of
the three, and it went red — which is why the split is written down here.
**(b) `POST …/options` replaces; an omitted key reverts to the config.yaml
default.** `App.options` is `deepmerge(config defaults, persisted user
options)`, and the setter is `persist[options] = deepcopy(value)`. Run on real
objects: with `max_w: 4200` stored, a POST that simply omits `max_w` leaves the
effective value at **2000**, config.yaml's default — not at 4200.
That is why the panel posts the **whole** blob it read, and why it refuses to
post one built on a stale read.
**(c) Validation does not coerce.** The full merged blob goes through
`AppOptions` unchanged, key for key. That is what makes the panel's *"stored
but not running"* comparison honest: a difference shown is a difference that
exists.
**(d) The Configuration tab *can* be removed — and doing so destroys every
setting on the site.** The card said it cannot be. It can: `schema:` is
optional, a manifest without it is accepted, and `schema_ui` is then `[]`, so
the tab renders nothing. What goes with it is the add-on. `AppOptions` drops
every option the schema does not declare — **0 of 71 survive** — so
`/data/options.json` becomes `{}`, the controller starts with no actuator, the
startup gate refuses it (exit 78), and the ingress panel goes down with the
process serving it.
So the tab stays **by decision, not by constraint**. That distinction matters:
a constraint nobody re-examines is not a decision, and this one has a cost
(§4) that has to be paid knowingly.
## 3. The decision
> **The authoritative configuration is the options blob the Supervisor stores.
> Both surfaces are editors of it. Neither is a cache of the other, and neither
> owns a copy of anything.**
Consequences, in the order they bind:
1. **The panel never holds state.** Every page load re-reads
`GET /addons/self/info`. There is no local copy of the options, the schema
or the labels — the form is *built* from that response by
`app/configui.py`, which contains no option key, no label and no rule.
2. **A save is a compare-and-swap.** The read hands the page a
`fingerprint()` (a sha256 of the canonical blob); the save hands it back;
`selfconfig.save()` re-reads the stored blob **immediately before writing**
and refuses if the digest moved. The window in which the other surface can
slip a change past is one HTTP round trip, not the length of an editing
session.
3. **Validation happens before the compare, and a refused blob is never
written.** Not warned about — not written.
4. **Saving never restarts.** Applying options costs a control interruption
(§5), so it is a separate, explicitly confirmed act.
### Why a digest and not a version number
The Supervisor exposes no revision or ETag for an add-on's options. The only
thing a page can hold between reading and writing is the *content* it read —
and that is enough, because the question is not "how many times has this
changed" but "is what I am about to overwrite still what I was shown".
Identical content is by definition safe to overwrite.
### This is [[SAFETY-28]]'s discipline, applied to configuration
`arbiter.py` and `Controller.write_refusal()` enforce *exactly one writer per
actuator* not by convention but by a check on the write path:
> A NUMBER REACHES THE WIRE ONLY IF THE SINGLE ALLOCATOR PLACED IT, ON THIS
> TICK, FOR THIS UNIT.
The configuration analogue is written the same way and for the same reason:
> A BLOB REACHES THE SUPERVISOR ONLY IF IT WAS DERIVED FROM THE BLOB THAT IS
> STORED RIGHT NOW.
Note what is *not* claimed. SAFETY-28's rule makes a second writer
inexpressible; this one does not — the Supervisor's tab is a second writer and
stays one. What this gets is the weaker, honest property: **a second writer is
detected rather than silently overwritten.** That is the most a
compare-and-swap can give, and pretending otherwise would be the "check that
observes something other than what it claims to observe" this repository keeps
finding.
## 4. What this costs, stated plainly
**Two surfaces means two places to be confused.** The Configuration tab still
renders all 71 keys as a flat list, and it can still store a configuration the
add-on will refuse to start on — it knows only types and ranges. UX-01's
translations are therefore **not** superseded by this card: they are the only
thing standing between that tab and raw key names, and they are also where this
panel's labels, descriptions and section headings come from. The two surfaces
share one set of words on purpose; two vocabularies for one setting would be
worse than one plain list.
**The panel is not available when it is most needed.** If a configuration that
refuses to start is ever stored — from the Configuration tab, or by a hand-edit,
or by this panel in some future state it does not currently permit — the add-on
exits 78 and the panel goes with it. Recovery is the Configuration tab. That is
the reason it stays, and the reason the panel refuses to write a blob the
validator rejected: this surface must never be the thing that removes itself.
**The panel shows fewer refusals than the startup gate can, never more.** Two
of `validate.py`'s rules compare a configured ceiling against the setpoint
device's *own* reported range, which is only knowable by asking Home Assistant.
When ESPHome is asleep the range is `None` and those two rules are skipped —
here and at startup, identically. The page says so rather than looking green
for a reason the operator cannot see.
## 5. The restart cost, and why it is a separate button
Applying options needs a restart, and `Controller.shutdown()` commands **0 W on
every inverter** on the way out ("shutting down - commanding 0 W on every
inverter", main.py). A configuration save is therefore a real, if brief, loss
of control of the battery — and the operator is told that *before* they commit,
in the page's own markup, not in a message that appears afterwards.
So the panel splits the act in two:
* **Save** stores the blob. Nothing restarts, nothing is applied, and the page
then says which settings are stored but not running.
* **Restart to apply** asks again, names the 0 W again, and only then posts
`/addons/self/restart`. The request carries an intent token, so a reload, a
retry or a stray POST cannot spend a control interruption on the operator's
behalf.
A restart does **not** clear anything: SAFETY-02's latches are restored from
the Store on the way back up, which is what `Inverter.__init__` is written to
do and what [[SAFETY-38]] depends on.
## 6. The scope line, and it is a safety line
**Nothing on this surface commands an inverter, clears a latch or bypasses a
safety rule.** The set of things an operator can do to the hardware through the
panel is exactly what `/action` offered before this card. `app/selfconfig.py`
talks to the Supervisor about a JSON blob and to `validate.py` about whether
that blob is startable; the only actuator-shaped verb in it is `restart()`,
which restarts *this add-on* and whose 0 W is the existing, documented
wind-down.
test_selfconfig.py part 7 asserts that mechanically — it lists the routes, and
it greps `selfconfig.py` and the page for `set_number`, `write_setpoint`,
`handle_action`, `command_site_zero`, `sign_wd` and `reset(`. Adding "and also
set the setpoint while we are here" would put a second writer on the actuator
through the configuration surface, which is the non-goal this add-on is built
around.
## 7. How "every message comes from the shipped validator" is proven
Not by inspection. `test_selfconfig.py` part 6 copies the package, inserts **one
new rule** into `validate_startup_config()` in the copy, hashes every file under
`app/` to show that **validate.py is the only one that differs**, and then
serves the real `/config/validate` route out of that copy in a subprocess. The
new sentence arrives on the page, filed under the option key it names, with no
UI file touched.
That is the acceptance criterion run rather than argued, and it is what makes
"do not reimplement any rule in JavaScript" a property of the build instead of
a promise in a comment.