goodwe_controller/tools/rehearse_roster.py runs the real Controller —
not a stub — through the flat → roster transition that inverter B's
commissioning performs, over a copy of a state file given as an input path,
and reports the difference:
python3 tools/rehearse_roster.py --state <copy of state.json> \
--before <flat.yaml> --after <roster.yaml> \
--live-entities <ids the install has> --reverse --round-trip
Exit 0 clean · 1 at least one finding · 2 could not run (not a pass).
It reports every store key that moved, appeared, vanished or was STRANDED
under a name no machine reads; every entity id before and after with orphans
named; and whether the maintenance history MAINT-05 already moved once
survived being moved again.
⚠️Nothing here was run against real state and nothing here claims to have
been. SSH to the live box is blocked by policy and that is correct. Fetching /data/state.json, the options blob and the entity list are owner steps,
written down in DOCS.md under Before you commission inverter B.
⚠️No runtime code is changed in this PR.app/ is untouched; the diff is
one tool, one test file, six fixtures and two documents.
⭐ SAFETY-44's answer: the asymmetry is not safe, and it is a defect again
⚠️Read the round-2 section at the bottom before this one. Two claims
here are corrected there: this finding is already recorded on the merge
base (test_maintenance.py section 13) and this PR is a second instrument
confirming it, not a discovery; and main.py:250 is not the whole fix —
measured, it repairs only one of the two routes.
The card asks whether migrate_unit_keys(store, UNIT_A) taking a hardcoded
constant — while migrate_maint_keys(store, unit, roster) takes the roster and
has a misdirected guard — leaves the two latch keys unread on a store that ran
pre-DEV-02 flat and declares a multi-inverter roster.
It does. Measured, not argued:
roster first entry = 'garage' (NOT inverter-a)
roster : ['garage', 'shed']
migrate_unit_keys was handed : 'inverter-a' (hardcoded constant)
DEV-02 migration report : migrated 2 flat key(s) to inverter
namespaces: unit/inverter-a/sign_reason,
unit/inverter-a/sign_tripped
---- inside _grade_store_migration ----
flat sign_tripped : True
unit/*/sign_tripped seen in store : {'unit/inverter-a/sign_tripped': True}
-> namespaced_latched : True <-- the guard RETURNS EARLY
units tripped when guard ran : NONE
---- outcome ----
units latched after full start : NONE <== DISARMED
roster first entry = 'inverter-a'
...
units tripped when guard ran : ['inverter-a']
units latched after full start : ['inverter-a']
The mechanism is worse than "a key nothing reads". _grade_store_migration() is
the guard written for exactly this case — flat says tripped and no inverter came
up tripped — and it is defeated by the migration's own misdirected write: migrate_unit_keys() deposits unit/inverter-a/sign_tripped = True, the namespaced_latched test finds it, concludes something namespaced is latched,
and returns without re-arming anything. The file says sign_tripped: true, the
banner is green, nothing is held at 0 W.
migrate_maint_keys() would have called this misdirected, withheld the marker
and re-run. migrate_unit_keys() has no such guard, and the documented defence
(inverter.py:161-163, "flat pre-DEV-02 keys can only ever have belonged to a
single-inverter install") is true about which install they came from and
silent about what that install's first roster entry is now called.
Both routes reach it, not only the same-step one:
same-step — flat store, roster declared in the same upgrade: the migration
runs and misdirects;
staged — flat config runs once under 0.7.0+, roster declared at a later
restart: the first start stamps schema_version = 1, so the roster start's
migration takes its from_version >= SCHEMA_VERSION early return and the latch
never moves at all. Same disarmed end state.
The contrast run is what makes this attributable: the same state file onto a
roster whose first entry isinverter-a comes up correctly latched. The
finding is caused by the roster name, not by the fixture being tripped.
SAFETY-44 should be reopened as a defect. The fix is not in this PR — a
change to migrate_unit_keys() or to namespaced_latched is a safety change
that deserves its own card and its own red-before-green, and burying it in a
rehearsal PR would hide it. test_rehearse.py part 5 pins the current behaviour
as a requirement, so that fix turns this file red and has to update it
deliberately.
⭐ The check can fail, and was seen to
Three clean controls come first, because six reds prove nothing about an
instrument that is red on its own fixtures:
CONTROL-A — pre-DEV-02 flat store, latch tripped, onto a roster whose first
entry isinverter-a → exit 0, no findings.
CONTROL-B — a store that already ran flat under 0.7.0, onto garage/shed
→ exit 0, no findings.
CONTROL-C — a fresh install (empty state file) onto garage/shed → exit
0, no findings. ⚠️This one found a defect in the harness, which is what a
control is for: the DEV-02 migration writes sign_tripped = False and sign_reason = "" under inverter-a before any roster exists, and the first
draft called both STRANDED — red on every fresh install that grows a second
inverter. Losing a default loses nothing; that is MigrationReport.lost's
own distinction, and store.py's own argument that an alarm which fires on the
harmless case is one an operator learns to dismiss before the real one
arrives. The stranded check now splits on the value, not the key name: a
default is printed under its own heading and is not a finding; the same key at
a non-default still is. A check pins both halves against each other so the
softening cannot widen into a hole — SAFETY-44 turns on sign_tripped = True.
Then six deliberate mutations of the shipped migration and of the entity-id
composition, each applied to a private copy of the tree (source edits, not
monkeypatches; every str.replace asserted to have matched exactly once), each
required to flip its control green → red with the right finding named:
--- M1 (CONTROL-B) the maintenance migration is handed the hardcoded `UNIT_A`
instead of the roster's own first name - MAINT-05's exact shipped
regression, which left all 19 suites green
exit 1
⚠️ STRANDED STORE KEY: unit/inverter-a/last_completed = '2026-08-07T18:42:00+00:00' …
⚠️ STRANDED STORE KEY: unit/inverter-a/phase = 'idle' …
⚠️ MAINTENANCE HISTORY LOST: `last_completed` read '2026-08-07T18:42:00+00:00' through
the first pack's own history before the transition and reads None through 'garage's
--- M2 (CONTROL-B) the maintenance namespace is DROPPED: the migration moves
no keys at all
exit 1
⚠️ MAINTENANCE HISTORY LOST: `last_completed` … reads None through 'garage's afterwards
--- M3 (CONTROL-A) the SAFETY-02 pair loses HALF of itself: `sign_reason`
drops out of `LEGACY_UNIT_KEYS`
exit 1
⚠️ LATCH KEY NOT UNDER A ROSTER NAME: the flat `sign_reason` = 'SIGN_DISAGREEMENT: …'
did not reach unit/inverter-a/sign_reason
--- M4 (CONTROL-A) the READER's key composition changes while the migration's
does not - `Inverter.key()` upper-cases the unit
exit 1
⚠️ STRANDED STORE KEY: unit/inverter-a/sign_tripped = True …
⚠️ LATCH KEY NOT READ BY ANY MACHINE: unit/inverter-a/sign_tripped = True exists but no
machine on the roster ['inverter-a', 'inverter-b'] read it during a full start
⚠️ LATCH CAME UP DISARMED: … and NOT ONE inverter came up tripped.
--- M5 (CONTROL-A) the entity-id composition changes on the DEVICE half
exit 1
⚠️ COMPOSITION DRIFT: this build announces sensor.goodwe_controller_battery_power in the
BEFORE shape, and that id is not on the recorded list of ids the install has
… (seven ids, i.e. every site entity)
--- M6 (CONTROL-A) the entity-id composition changes on the ENTITY half
exit 1
⚠️ COMPOSITION DRIFT: this build announces sensor.goodwe_grid_power_w …
One mutation is required to SURVIVE, with the reason written down, because a
battery in which everything dies is a battery somebody stopped writing as soon as
the number looked good: renaming store.unit_key() — the one spelling — moves
the writer and every reader together, so the store comes out internally
consistent under a different name. It is a rollback contract, and grading it
needs two versions of the code, which is a different instrument. The check
asserts the survival and asserts the store really changed shape underneath it,
so it is a survivor of a mutation that happened.
The synthetic state carries maintenance history and a legacy-shaped key
whose loss would be silent — a latched sign_tripped with no schema_version
beside it, i.e. written by a controller that had never heard of unit namespaces.
⚠️ The first draft of this harness had habit 4 in it
MUTATION.md's thirteenth instance, and the third in an instrument rather
than in a check — caught by this PR's own mutation battery, which is the cheap
place. The entity diff composed both sides with the same build, so renaming
the MQTT device moved every id equally and the harness reported "no orphans"
about a change that orphans everything. M5 came back exit 0, M6 exit 2, and
neither was the failure they name. The anchor had the same shape: it was compose(mqtt.DEVICE["name"], …), which any rename of the device satisfies by
moving both operands.
The fix is habit 2 with a filename: --live-entities takes a list of the ids the
install actually has — data that does not move with the code — and the anchor
is now composed from two recorded literals. Recorded in MUTATION.md.
The reverse direction: the retreat is supported, the round trip is not
Rehearsed, both legs, on the fixtures.
The retreat (roster → flat) does not corrupt state. The SAFETY-02 latch and
the first pack's maintenance history both survive, because store.py rule 2
keeps the flat spellings live rather than merely present — maintenance.UnitHistory.mirror_flat writes them on every change, so a
single-FSM add-on finds a current history, not one dated the day the roster was
declared. But it orphans eight Home Assistant entities — the four per-unit
rows for each of the two units. Their history stops and any automation naming
them stops firing, silently, and advancing again does not bring them back
because HA keeps the id it composed at first discovery.
⚠️The round trip is NOT supported, and that is the stated decision. A
maintenance cycle that completes while the install is rolled back is discarded on re-advance:
⚠️ RETREAT-WINDOW WORK DISCARDED: a maintenance cycle completed at
2026-09-20T18:00:00+00:00 while the install was rolled back to the flat
config, and after re-advancing to the roster 'garage' reads `last_completed`
= '2026-08-07T18:42:00+00:00'. The pack will be recalibrated again on a
schedule that does not know it already was.
maint_schema_version is already stamped, so migrate_maint_keys() takes its
early return and the pack reads the namespaced history from before the retreat. Retreat freely, but treat a retreat as ending the current maintenance
bookkeeping — and if a cycle ran during it, correct unit/<first>/last_completed by hand before advancing again. Written into DOCS.md as a decision rather than left to be discovered at the inverter.
That leg is itself habit-4-proofed: the retreat window actually contains a
completion, written through the shipped UnitHistory rather than a raw store.set(), at a timestamp the starting fixture does not carry. A round trip
over a store nothing changed in between compares a structure to itself.
Design notes
The real Controller and the real Store.RehearsalStore subclasses app.store.Store and is pointed at a scratch copy made with mkstemp(), so load(), save(), get_time() and set_time() are the shipped ones and the
operator's file is never opened for write. ⚠️test_rehearse.py — not the
tool — is what asserts the input file is byte-identical afterwards,
including after the writing legs. The tool copies; the check verifies.
"Nothing reads this key" is measured, not reasoned.Store.data is a dict
subclass that records every value-read; the probe then re-runs the shipped
constructors (Inverter.__init__ per roster entry, UnitHistory over MAINT_UNIT_KEYS) against the post-transition store. A key in the file that
no reader touched is a key whose value is not information. Writing the key
names out here instead would be a second transcription of UNIT_STORE_KEYS,
and the copy that drifts is always the one that reads.
The entity-id model is deliberately not mqtt._slug, for the reason test_mqtt.py part 6 gives.
No yaml module in the check image, so the tool carries a narrow reader
for the subset an options file is written in — and it refuses what it does
not understand rather than guessing, because a config silently mis-parsed here
rehearses a configuration nobody is going to deploy. JSON (/data/options.json)
is accepted directly and PyYAML is used when importable.
Acceptance
Harness runs the real Controller over the transition, state file as an input path
Reports moved/new/vanished store keys and before/after entity ids, orphans explicit
⭐ Six deliberate migration mutations each shown going RED, output included (three required)
Synthetic fixture carries maintenance history and a legacy-shaped key
Reverse direction rehearsed; the round trip's failure documented as a decision
A documented owner step for fetching the live /data/state.json copy
Nothing touches the live box: no SSH, no Supervisor call, no HTTP in the tool
SAFETY-44 exercised and answered by name on the two latch keys
Verification
Full suite in the CI image, .gitea/ci/Dockerfile build, tree at <ws>/goodwe-addon/:
Positive control before the run: M4 turns CONTROL-A RED was broken on purpose
→ runner reported FAIL M4 turns CONTROL-A RED, exit 1; restored byte-identical;
exit 0 again.
Round 2 — rejected on two blocking findings, both fixed and both now pinned
Both were reproduced before anything was changed. Both are the class this card
exists to guard against, and both were in the instrument, not in the
conclusion.
B1 — the pin was a count, and a count cannot separate two routes
The rehearsal reaches the disarmed latch by two independent paths, and the tool
printed every finding twice per red leg (once in the leg's block, once in
the closing summary). So out.count("LATCH CAME UP DISARMED") >= 2 was
satisfied by one red leg. Reproduced across three builds:
build
STAGED
SAME-STEP
old count
old check
shipped code
DISARMED
DISARMED
4
ok (correctly)
migrate_unit_keys() handed the roster name
DISARMED
clean
2
ok — wrongly
guard side constrained to roster names
clean
clean
0
fails
The check named "NOT THE SAME-STEP PATH ALONE" reported ok on the build
whose only remaining failure is the staged path.
Fixed: the tool prints LEG <id> … END LEG <id>; the pin splits on those
and requires the finding in each leg by name.
⚠️ And the first splitter carried the same defect one level down
Opening on LEG <id> and never closing gave every line after the last marker
to that leg — including the closing summary, which repeats every finding from
every leg. The last leg then read red whenever any leg was red, so the new
per-leg check was the old count with extra steps. Caught by building the one
fixture that separates them.
Part 5b is now that fixture, run every time — a tree in which SAME-STEP is
genuinely clean and STAGED is not:
5b. THE SPLITTER'S POSITIVE CONTROL: a build in which exactly ONE leg is clean
LEG STAGED: DISARMED
LEG SAME-STEP: clean
ok ⭐ with `migrate_unit_keys()` handed the roster's own first name, the
SAME-STEP leg comes up CLEAN - so the splitter really does read the legs
separately, and a check that only counted would have missed this
ok ⚠️ ...while the STAGED leg is STILL DISARMED on the same build
A per-thing check that has never seen the things disagree is a total with extra
steps. Recorded in MUTATION.md as habit 4's fourteenth and fifteenth
instances.
⚠️ This corrects a claim the PR made
main.py:250 is not the whole fix. Measured on exactly that change: it
repairs SAME-STEP and leaves STAGED disarmed, because the staged path stamps schema_version on the flat start and the roster start's migration returns
early before the name it is handed matters. What covers both is the guard
side — namespaced_latched counting a namespace that is not on today's
roster. DOCS.md now carries the table and the measurement. (That line's existing
comment deliberately admits off-roster keys, so it is a design decision for the
MAINT-05 author, not a one-liner — which is why the fix is still not here.)
B2 — a forgotten flag was a clean bill of health
Reproduced:
M5 device rename with --live-entities: exit 1 without: exit 0
M6 sensor rename with --live-entities: exit 1 without: exit 0
grep -ci "live-entities|recorded list|composition" over the no-flag report -> 0
An unreadable file exited 2; the flag simply not passed exited 0 with no
disclosure at all. The asymmetry was backwards, and the owner runs this once, at
commissioning, under time pressure.
Fixed: omitting --live-entities now refuses (exit 2). --no-live-entities is the deliberate opt-out and carries a standing banner in
the report header and on the exit line, because a report is skimmed from the
bottom. Contradictory flags refuse. Part 2b pins all of it.
N1 — the reference did not carry what Home Assistant keys on
Reproduced, all with--live-entities:
BASE "goodwe_ctl" -> "goodwe_controller" (every unique_id changes) exit 0
DEVICE identifiers changed (HA sees a new device) exit 0
object_id rename (a true no-op per mqtt.py:37-42) exit 0 ✓ correct
The first two are real orphanings with every composed entity_id
byte-identical. HA keys its entity registry on unique_id and its device
registry on the identifier; discovery is retained with no removal payload, so
the old config stays on the broker and the dead entity stays in the registry.
Fixed:announced-reference.txt is now three columns
(entity_id unique_id device), is loaded on every run — including a
degraded one — and is graded at a fixed canonical shape rather than against the
operator's config (grading it against --before reported a false sensor.goodwe_p1_sample_age drift on any site with meter_source: off).
Two new mutations pin it, and the no-op is now a required survivor — without
a change to mqtt.py that must not be caught, "the reference notices any edit
to mqtt.py" and "the reference notices what HA keys on" are the same green:
--- M7 the `unique_id` composition changes and NOT the entity_id
exit 1
⚠️ UNIQUE_ID DRIFT: sensor.goodwe_grid_power is announced with unique_id
'goodwe_controller_grid' where the reference records 'goodwe_ctl_grid'…
(8 rows)
--- M8 the DEVICE IDENTIFIER changes
exit 1
⚠️ DEVICE IDENTIFIER DRIFT: sensor.goodwe_setpoint is announced on device
'goodwe_rs485_ctl_v2' where the reference records 'goodwe_rs485_controller'…
(8 rows)
ok renaming an `object_id` survives, because HA composes the entity_id from
the device and entity NAMES and ignores it
Found while fixing the above
VANISHED did not carry the default/non-default rule STRANDED does. A
pair of migrated defaults moving namespace read as two findings, and CONTROL-C went red on a correct build. Same rule, same argument
(MigrationReport.lost), now applied to both — and bounded by M9, which
deletes non-default keys and must still be a finding (last_completed, phase_started, last_start_attempt — and notphase, whose 'idle' is
the default, on the same mutation).
raw() concatenated stdout and stderr, so "is the banner near the end of
the report" read the last lines of the controller's log. Split.
The read-probe called get_time() on phase, making the store log unparseable timestamp at WARNING about a healthy store. get() alone
records the same read.
M7's evidence was filtered out by its own finding kind — the display
regex's class had no underscore, so UNIQUE_ID DRIFT: printed a verdict with
no evidence under it.
N2 / N3 — corrections to the prose
DOCS.md withdraws the unmeasured permanence claim."not recoverable by
advancing again: HA keeps the id it composed at first discovery" was
unmeasured and its own reasoning cut the other way. The measured part — the
eight rows stop updating, history gaps, automations stop firing silently —
stands; the recovery claim is withdrawn rather than restated in the other
direction.
Attribution: the byte-identical-input property is asserted by test_rehearse.py, not by the tool. The tool copies to an mkstemp() scratch
file; the check is what verifies it.
⚠️ Framing correction — this is re-confirmation, not discovery
SAFETY-44 is already recorded on the merge base.test_maintenance.py
section 13 states it, names namespaced_latched, and carries the "guard is
defeated by the very key the defect wrote" sentence verbatim — in a file this
PR does not touch. What is new is that the finding is reachable from a state
file the owner supplies, by a different route (a full Controller start
diffed against its predecessor rather than assertions over a MemStore). Two
instruments, one answer. DOCS.md's section is retitled "What it confirms" and
says this in its first paragraph.
Round-2 verification
app/ still untouched. test_rehearse.py: 59 checks, 9 mutations red with
the right finding named, 2 required survivors, 3 clean controls.
Positive control on the new per-leg check: inverted LEG STAGED must-be-red →
runner reported FAIL ⚠️ ...while the STAGED leg is STILL DISARMED, exit 1;
restored byte-identical; exit 0.
## What this is
`goodwe_controller/tools/rehearse_roster.py` runs the **real `Controller`** —
not a stub — through the flat → roster transition that inverter B's
commissioning performs, over a **copy** of a state file given as an input path,
and reports the difference:
```
python3 tools/rehearse_roster.py --state <copy of state.json> \
--before <flat.yaml> --after <roster.yaml> \
--live-entities <ids the install has> --reverse --round-trip
```
Exit `0` clean · `1` at least one finding · `2` could not run (**not a pass**).
It reports every store key that **moved, appeared, vanished or was STRANDED**
under a name no machine reads; every entity id before and after with **orphans
named**; and whether the maintenance history MAINT-05 already moved once
survived being moved again.
⚠️ **Nothing here was run against real state and nothing here claims to have
been.** SSH to the live box is blocked by policy and that is correct. Fetching
`/data/state.json`, the options blob and the entity list are **owner steps**,
written down in `DOCS.md` under *Before you commission inverter B*.
⚠️ **No runtime code is changed in this PR.** `app/` is untouched; the diff is
one tool, one test file, six fixtures and two documents.
---
## ⭐ SAFETY-44's answer: the asymmetry is **not** safe, and it is a defect again
> ⚠️ **Read the round-2 section at the bottom before this one.** Two claims
> here are corrected there: this finding is **already recorded** on the merge
> base (`test_maintenance.py` section 13) and this PR is a *second instrument*
> confirming it, not a discovery; and `main.py:250` is **not** the whole fix —
> measured, it repairs only one of the two routes.
The card asks whether `migrate_unit_keys(store, UNIT_A)` taking a hardcoded
constant — while `migrate_maint_keys(store, unit, roster)` takes the roster and
has a `misdirected` guard — leaves the two latch keys unread on a store that ran
pre-DEV-02 flat and declares a multi-inverter roster.
**It does. Measured, not argued:**
```
roster first entry = 'garage' (NOT inverter-a)
roster : ['garage', 'shed']
migrate_unit_keys was handed : 'inverter-a' (hardcoded constant)
DEV-02 migration report : migrated 2 flat key(s) to inverter
namespaces: unit/inverter-a/sign_reason,
unit/inverter-a/sign_tripped
---- inside _grade_store_migration ----
flat sign_tripped : True
unit/*/sign_tripped seen in store : {'unit/inverter-a/sign_tripped': True}
-> namespaced_latched : True <-- the guard RETURNS EARLY
units tripped when guard ran : NONE
---- outcome ----
units latched after full start : NONE <== DISARMED
roster first entry = 'inverter-a'
...
units tripped when guard ran : ['inverter-a']
units latched after full start : ['inverter-a']
```
The mechanism is worse than "a key nothing reads". `_grade_store_migration()` is
the guard written for exactly this case — *flat says tripped and no inverter came
up tripped* — and it is **defeated by the migration's own misdirected write**:
`migrate_unit_keys()` deposits `unit/inverter-a/sign_tripped = True`, the
`namespaced_latched` test finds it, concludes something namespaced is latched,
and returns without re-arming anything. The file says `sign_tripped: true`, the
banner is green, nothing is held at 0 W.
`migrate_maint_keys()` would have called this `misdirected`, withheld the marker
and re-run. `migrate_unit_keys()` has no such guard, and the documented defence
(`inverter.py:161-163`, *"flat pre-DEV-02 keys can only ever have belonged to a
single-inverter install"*) is true about **which install** they came from and
silent about **what that install's first roster entry is now called**.
**Both routes reach it**, not only the same-step one:
* **same-step** — flat store, roster declared in the same upgrade: the migration
runs and misdirects;
* **staged** — flat config runs once under 0.7.0+, roster declared at a later
restart: the first start stamps `schema_version = 1`, so the roster start's
migration takes its `from_version >= SCHEMA_VERSION` early return and the latch
never moves at all. Same disarmed end state.
The contrast run is what makes this attributable: the **same state file** onto a
roster whose first entry **is** `inverter-a` comes up correctly latched. The
finding is caused by the roster **name**, not by the fixture being tripped.
**SAFETY-44 should be reopened as a defect.** The fix is not in this PR — a
change to `migrate_unit_keys()` or to `namespaced_latched` is a safety change
that deserves its own card and its own red-before-green, and burying it in a
rehearsal PR would hide it. `test_rehearse.py` part 5 pins the current behaviour
as a **requirement**, so that fix turns this file red and has to update it
deliberately.
---
## ⭐ The check can fail, and was seen to
Three **clean controls** come first, because six reds prove nothing about an
instrument that is red on its own fixtures:
* **CONTROL-A** — pre-DEV-02 flat store, latch tripped, onto a roster whose first
entry *is* `inverter-a` → exit 0, no findings.
* **CONTROL-B** — a store that already ran flat under 0.7.0, onto `garage`/`shed`
→ exit 0, no findings.
* **CONTROL-C** — a fresh install (empty state file) onto `garage`/`shed` → exit
0, no findings. ⚠️ **This one found a defect in the harness**, which is what a
control is for: the DEV-02 migration writes `sign_tripped = False` and
`sign_reason = ""` under `inverter-a` before any roster exists, and the first
draft called both STRANDED — **red on every fresh install that grows a second
inverter**. Losing a *default* loses nothing; that is `MigrationReport.lost`'s
own distinction, and store.py's own argument that an alarm which fires on the
harmless case is one an operator learns to dismiss before the real one
arrives. The stranded check now splits on the **value**, not the key name: a
default is printed under its own heading and is not a finding; the same key at
a non-default still is. A check pins both halves against each other so the
softening cannot widen into a hole — SAFETY-44 turns on `sign_tripped = True`.
Then six deliberate mutations of the shipped migration and of the entity-id
composition, each applied to a **private copy of the tree** (source edits, not
monkeypatches; every `str.replace` asserted to have matched exactly once), each
required to flip its control green → red **with the right finding named**:
```
--- M1 (CONTROL-B) the maintenance migration is handed the hardcoded `UNIT_A`
instead of the roster's own first name - MAINT-05's exact shipped
regression, which left all 19 suites green
exit 1
⚠️ STRANDED STORE KEY: unit/inverter-a/last_completed = '2026-08-07T18:42:00+00:00' …
⚠️ STRANDED STORE KEY: unit/inverter-a/phase = 'idle' …
⚠️ MAINTENANCE HISTORY LOST: `last_completed` read '2026-08-07T18:42:00+00:00' through
the first pack's own history before the transition and reads None through 'garage's
--- M2 (CONTROL-B) the maintenance namespace is DROPPED: the migration moves
no keys at all
exit 1
⚠️ MAINTENANCE HISTORY LOST: `last_completed` … reads None through 'garage's afterwards
--- M3 (CONTROL-A) the SAFETY-02 pair loses HALF of itself: `sign_reason`
drops out of `LEGACY_UNIT_KEYS`
exit 1
⚠️ LATCH KEY NOT UNDER A ROSTER NAME: the flat `sign_reason` = 'SIGN_DISAGREEMENT: …'
did not reach unit/inverter-a/sign_reason
--- M4 (CONTROL-A) the READER's key composition changes while the migration's
does not - `Inverter.key()` upper-cases the unit
exit 1
⚠️ STRANDED STORE KEY: unit/inverter-a/sign_tripped = True …
⚠️ LATCH KEY NOT READ BY ANY MACHINE: unit/inverter-a/sign_tripped = True exists but no
machine on the roster ['inverter-a', 'inverter-b'] read it during a full start
⚠️ LATCH CAME UP DISARMED: … and NOT ONE inverter came up tripped.
--- M5 (CONTROL-A) the entity-id composition changes on the DEVICE half
exit 1
⚠️ COMPOSITION DRIFT: this build announces sensor.goodwe_controller_battery_power in the
BEFORE shape, and that id is not on the recorded list of ids the install has
… (seven ids, i.e. every site entity)
--- M6 (CONTROL-A) the entity-id composition changes on the ENTITY half
exit 1
⚠️ COMPOSITION DRIFT: this build announces sensor.goodwe_grid_power_w …
```
**One mutation is required to SURVIVE**, with the reason written down, because a
battery in which everything dies is a battery somebody stopped writing as soon as
the number looked good: renaming `store.unit_key()` — the *one* spelling — moves
the writer and every reader together, so the store comes out internally
consistent under a different name. It is a **rollback** contract, and grading it
needs two versions of the code, which is a different instrument. The check
asserts the survival *and* asserts the store really changed shape underneath it,
so it is a survivor of a mutation that **happened**.
The synthetic state carries **maintenance history** and a **legacy-shaped key
whose loss would be silent** — a latched `sign_tripped` with no `schema_version`
beside it, i.e. written by a controller that had never heard of unit namespaces.
### ⚠️ The first draft of this harness had habit 4 in it
`MUTATION.md`'s **thirteenth** instance, and the **third in an instrument** rather
than in a check — caught by this PR's own mutation battery, which is the cheap
place. The entity diff composed **both sides with the same build**, so renaming
the MQTT device moved every id equally and the harness reported *"no orphans"*
about a change that orphans everything. M5 came back `exit 0`, M6 `exit 2`, and
neither was the failure they name. The anchor had the same shape: it was
`compose(mqtt.DEVICE["name"], …)`, which any rename of the device satisfies by
moving both operands.
The fix is habit 2 with a filename: `--live-entities` takes a list of the ids the
install actually has — data that does **not** move with the code — and the anchor
is now composed from two recorded literals. Recorded in `MUTATION.md`.
---
## The reverse direction: the retreat is supported, the round trip is not
Rehearsed, both legs, on the fixtures.
**The retreat (roster → flat) does not corrupt state.** The SAFETY-02 latch and
the first pack's maintenance history both survive, because `store.py` rule 2
keeps the flat spellings **live** rather than merely present —
`maintenance.UnitHistory.mirror_flat` writes them on every change, so a
single-FSM add-on finds a current history, not one dated the day the roster was
declared. **But it orphans eight Home Assistant entities** — the four per-unit
rows for each of the two units. Their history stops and any automation naming
them stops firing, silently, and advancing again does not bring them back
because HA keeps the id it composed at first discovery.
⚠️ **The round trip is NOT supported, and that is the stated decision.** A
maintenance cycle that completes *while the install is rolled back* is
**discarded** on re-advance:
```
⚠️ RETREAT-WINDOW WORK DISCARDED: a maintenance cycle completed at
2026-09-20T18:00:00+00:00 while the install was rolled back to the flat
config, and after re-advancing to the roster 'garage' reads `last_completed`
= '2026-08-07T18:42:00+00:00'. The pack will be recalibrated again on a
schedule that does not know it already was.
```
`maint_schema_version` is already stamped, so `migrate_maint_keys()` takes its
early return and the pack reads the namespaced history from before the retreat.
**Retreat freely, but treat a retreat as ending the current maintenance
bookkeeping** — and if a cycle ran during it, correct
`unit/<first>/last_completed` by hand before advancing again. Written into
`DOCS.md` as a decision rather than left to be discovered at the inverter.
That leg is itself habit-4-proofed: the retreat window **actually contains a
completion**, written through the shipped `UnitHistory` rather than a raw
`store.set()`, at a timestamp the starting fixture does not carry. A round trip
over a store nothing changed in between compares a structure to itself.
---
## Design notes
* **The real `Controller` and the real `Store`.** `RehearsalStore` subclasses
`app.store.Store` and is pointed at a scratch copy made with `mkstemp()`, so
`load()`, `save()`, `get_time()` and `set_time()` are the shipped ones and the
operator's file is never opened for write. ⚠️ **`test_rehearse.py` — not the
tool — is what asserts the input file is byte-identical afterwards**,
including after the writing legs. The tool copies; the check verifies.
* **"Nothing reads this key" is measured, not reasoned.** `Store.data` is a dict
subclass that records every value-read; the probe then **re-runs the shipped
constructors** (`Inverter.__init__` per roster entry, `UnitHistory` over
`MAINT_UNIT_KEYS`) against the post-transition store. A key in the file that
no reader touched is a key whose value is not information. Writing the key
names out here instead would be a second transcription of `UNIT_STORE_KEYS`,
and the copy that drifts is always the one that reads.
* **The entity-id model is deliberately not `mqtt._slug`**, for the reason
`test_mqtt.py` part 6 gives.
* **No `yaml` module in the check image**, so the tool carries a narrow reader
for the subset an options file is written in — and it **refuses** what it does
not understand rather than guessing, because a config silently mis-parsed here
rehearses a configuration nobody is going to deploy. JSON (`/data/options.json`)
is accepted directly and PyYAML is used when importable.
## Acceptance
- [x] Harness runs the real `Controller` over the transition, state file as an input path
- [x] Reports moved/new/vanished store keys and before/after entity ids, orphans explicit
- [x] ⭐ Six deliberate migration mutations each shown going RED, output included (three required)
- [x] Synthetic fixture carries maintenance history and a legacy-shaped key
- [x] Reverse direction rehearsed; the round trip's failure documented as a decision
- [x] A documented owner step for fetching the live `/data/state.json` copy
- [x] Nothing touches the live box: no SSH, no Supervisor call, no HTTP in the tool
- [x] SAFETY-44 exercised and answered by name on the two latch keys
## Verification
Full suite in the CI image, `.gitea/ci/Dockerfile` build, tree at
`<ws>/goodwe-addon/`:
```
28 files
PASS test_alloc.py (25s)
PASS test_arbiter.py (0s)
PASS test_arbitrage.py (6s)
PASS test_bytecode.py (1s)
PASS test_ci_gate.py (1s)
PASS test_clamp.py (1s)
PASS test_control.py (1s)
PASS test_defaults_pin.py (1s)
PASS test_delivery.py (35s)
PASS test_freeze.py (34s)
PASS test_freshness.py (16s)
PASS test_inverter.py (7s)
PASS test_loop.py (94s)
PASS test_maintenance.py (1s)
PASS test_mqtt.py (1s)
PASS test_p1.py (8s)
PASS test_packshare.py (1s)
PASS test_peakguard.py (62s)
PASS test_price.py (1s)
PASS test_price_forecast.py (1s)
PASS test_rehearse.py (18s)
PASS test_selfconfig.py (2s)
PASS test_static_tariff.py (4s)
PASS test_tariff.py (6s)
PASS test_translations.py (1s)
PASS test_validate.py (5s)
PASS test_version_gate.py (8s)
PASS test_watchdog.py (61s)
RC=0
```
Positive control before the run: `M4 turns CONTROL-A RED` was broken on purpose
→ runner reported `FAIL M4 turns CONTROL-A RED`, exit 1; restored byte-identical;
exit 0 again.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
---
# Round 2 — rejected on two blocking findings, both fixed and both now pinned
Both were reproduced before anything was changed. Both are the class this card
exists to guard against, and both were in the **instrument**, not in the
conclusion.
## B1 — the pin was a count, and a count cannot separate two routes
The rehearsal reaches the disarmed latch by two independent paths, and the tool
printed every finding **twice per red leg** (once in the leg's block, once in
the closing summary). So `out.count("LATCH CAME UP DISARMED") >= 2` was
satisfied by **one** red leg. Reproduced across three builds:
| build | STAGED | SAME-STEP | old count | old check |
|---|---|---|---|---|
| shipped code | DISARMED | DISARMED | 4 | ok (correctly) |
| `migrate_unit_keys()` handed the roster name | DISARMED | clean | 2 | **ok — wrongly** |
| guard side constrained to roster names | clean | clean | 0 | fails |
The check named *"NOT THE SAME-STEP PATH ALONE"* reported `ok` on the build
whose only remaining failure **is** the staged path.
**Fixed:** the tool prints `LEG <id>` … `END LEG <id>`; the pin splits on those
and requires the finding in **each leg by name**.
### ⚠️ And the first splitter carried the same defect one level down
Opening on `LEG <id>` and never closing gave every line after the *last* marker
to that leg — **including the closing summary, which repeats every finding from
every leg**. The last leg then read red whenever *any* leg was red, so the new
per-leg check was the old count with extra steps. Caught by building the one
fixture that separates them.
**Part 5b is now that fixture, run every time** — a tree in which `SAME-STEP` is
genuinely clean and `STAGED` is not:
```
5b. THE SPLITTER'S POSITIVE CONTROL: a build in which exactly ONE leg is clean
LEG STAGED: DISARMED
LEG SAME-STEP: clean
ok ⭐ with `migrate_unit_keys()` handed the roster's own first name, the
SAME-STEP leg comes up CLEAN - so the splitter really does read the legs
separately, and a check that only counted would have missed this
ok ⚠️ ...while the STAGED leg is STILL DISARMED on the same build
```
*A per-thing check that has never seen the things disagree is a total with extra
steps.* Recorded in `MUTATION.md` as habit 4's fourteenth and fifteenth
instances.
### ⚠️ This corrects a claim the PR made
**`main.py:250` is not the whole fix.** Measured on exactly that change: it
repairs `SAME-STEP` and leaves `STAGED` disarmed, because the staged path stamps
`schema_version` on the flat start and the roster start's migration returns
early before the name it is handed matters. What covers both is the **guard
side** — `namespaced_latched` counting a namespace that is not on today's
roster. DOCS.md now carries the table and the measurement. (That line's existing
comment deliberately admits off-roster keys, so it is a design decision for the
MAINT-05 author, not a one-liner — which is why the fix is still not here.)
## B2 — a forgotten flag was a clean bill of health
Reproduced:
```
M5 device rename with --live-entities: exit 1 without: exit 0
M6 sensor rename with --live-entities: exit 1 without: exit 0
grep -ci "live-entities|recorded list|composition" over the no-flag report -> 0
```
An **unreadable** file exited 2; the flag simply **not passed** exited 0 with no
disclosure at all. The asymmetry was backwards, and the owner runs this once, at
commissioning, under time pressure.
**Fixed:** omitting `--live-entities` now **refuses (exit 2)**.
`--no-live-entities` is the deliberate opt-out and carries a standing banner in
the report header **and** on the exit line, because a report is skimmed from the
bottom. Contradictory flags refuse. Part 2b pins all of it.
## N1 — the reference did not carry what Home Assistant keys on
Reproduced, all *with* `--live-entities`:
```
BASE "goodwe_ctl" -> "goodwe_controller" (every unique_id changes) exit 0
DEVICE identifiers changed (HA sees a new device) exit 0
object_id rename (a true no-op per mqtt.py:37-42) exit 0 ✓ correct
```
The first two are real orphanings with **every composed entity_id
byte-identical**. HA keys its entity registry on `unique_id` and its device
registry on the identifier; discovery is retained with no removal payload, so
the old config stays on the broker and the dead entity stays in the registry.
**Fixed:** `announced-reference.txt` is now three columns
(`entity_id unique_id device`), is loaded on **every** run — including a
degraded one — and is graded at a fixed canonical shape rather than against the
operator's config (grading it against `--before` reported a false
`sensor.goodwe_p1_sample_age` drift on any site with `meter_source: off`).
Two new mutations pin it, and the no-op is now a **required survivor** — without
a change to `mqtt.py` that must *not* be caught, "the reference notices any edit
to mqtt.py" and "the reference notices what HA keys on" are the same green:
```
--- M7 the `unique_id` composition changes and NOT the entity_id
exit 1
⚠️ UNIQUE_ID DRIFT: sensor.goodwe_grid_power is announced with unique_id
'goodwe_controller_grid' where the reference records 'goodwe_ctl_grid'…
(8 rows)
--- M8 the DEVICE IDENTIFIER changes
exit 1
⚠️ DEVICE IDENTIFIER DRIFT: sensor.goodwe_setpoint is announced on device
'goodwe_rs485_ctl_v2' where the reference records 'goodwe_rs485_controller'…
(8 rows)
ok renaming an `object_id` survives, because HA composes the entity_id from
the device and entity NAMES and ignores it
```
## Found while fixing the above
* **`VANISHED` did not carry the default/non-default rule `STRANDED` does.** A
pair of migrated *defaults* moving namespace read as two findings, and
**CONTROL-C went red on a correct build**. Same rule, same argument
(`MigrationReport.lost`), now applied to both — and bounded by **M9**, which
deletes non-default keys and must still be a finding (`last_completed`,
`phase_started`, `last_start_attempt` — and *not* `phase`, whose `'idle'` is
the default, on the same mutation).
* **`raw()` concatenated stdout and stderr**, so "is the banner near the end of
the report" read the last lines of the controller's *log*. Split.
* **The read-probe called `get_time()` on `phase`**, making the store log
`unparseable timestamp` at WARNING about a healthy store. `get()` alone
records the same read.
* **M7's evidence was filtered out by its own finding kind** — the display
regex's class had no underscore, so `UNIQUE_ID DRIFT:` printed a verdict with
no evidence under it.
## N2 / N3 — corrections to the prose
* **DOCS.md withdraws the unmeasured permanence claim.** *"not recoverable by
advancing again: HA keeps the id it composed at first discovery"* was
unmeasured and its own reasoning cut the other way. The measured part — the
eight rows stop updating, history gaps, automations stop firing silently —
stands; the recovery claim is withdrawn rather than restated in the other
direction.
* **Attribution:** the byte-identical-input property is asserted by
`test_rehearse.py`, not by the tool. The tool copies to an `mkstemp()` scratch
file; the check is what verifies it.
## ⚠️ Framing correction — this is re-confirmation, not discovery
**SAFETY-44 is already recorded on the merge base.** `test_maintenance.py`
section 13 states it, names `namespaced_latched`, and carries the *"guard is
defeated by the very key the defect wrote"* sentence verbatim — in a file this
PR does not touch. What is new is that the finding is reachable **from a state
file the owner supplies**, by a different route (a full `Controller` start
diffed against its predecessor rather than assertions over a `MemStore`). Two
instruments, one answer. DOCS.md's section is retitled *"What it confirms"* and
says this in its first paragraph.
## Round-2 verification
`app/` still untouched. `test_rehearse.py`: **59 checks, 9 mutations red with
the right finding named, 2 required survivors, 3 clean controls.**
Positive control on the new per-leg check: inverted `LEG STAGED` must-be-red →
runner reported `FAIL ⚠️ ...while the STAGED leg is STILL DISARMED`, exit 1;
restored byte-identical; exit 0.
```
28 files
PASS test_alloc.py (25s)
PASS test_arbiter.py (0s)
PASS test_arbitrage.py (6s)
PASS test_bytecode.py (1s)
PASS test_ci_gate.py (0s)
PASS test_clamp.py (2s)
PASS test_control.py (1s)
PASS test_defaults_pin.py (1s)
PASS test_delivery.py (35s)
PASS test_freeze.py (33s)
PASS test_freshness.py (16s)
PASS test_inverter.py (8s)
PASS test_loop.py (93s)
PASS test_maintenance.py (1s)
PASS test_mqtt.py (1s)
PASS test_p1.py (8s)
PASS test_packshare.py (1s)
PASS test_peakguard.py (63s)
PASS test_price.py (2s)
PASS test_price_forecast.py (0s)
PASS test_rehearse.py (29s)
PASS test_selfconfig.py (1s)
PASS test_static_tariff.py (4s)
PASS test_tariff.py (6s)
PASS test_translations.py (1s)
PASS test_validate.py (5s)
PASS test_version_gate.py (8s)
PASS test_watchdog.py (61s)
RC=0
```
Nothing had ever run the real `Controller` through the transition inverter B's
commissioning performs on 2026-09-12. `tools/rehearse_roster.py` does, over a
COPY of a state file given as an input path, and reports the difference: every
store key that moved, appeared, vanished or was STRANDED under a name no
machine reads; every entity id before and after with orphans named; and whether
the maintenance history MAINT-05 moved once survived being moved again.
⭐ WHAT IT FOUND, AND IT IS SAFETY-44's ANSWER. A store that ran pre-DEV-02 flat
and declares a multi-inverter roster comes up with its SAFETY-02 latch DISARMED
unless the first roster entry happens to be called `inverter-a`.
`Controller.__init__` hands `migrate_unit_keys()` the hardcoded `UNIT_A`, so the
latch lands in `unit/inverter-a/sign_tripped`; a roster of ["garage","shed"]
reads `unit/garage/sign_tripped`, which is absent. And `_grade_store_migration()`
- the guard written for exactly this - sees the migration's OWN misdirected key
satisfy its `namespaced_latched` test and returns without re-arming. Measured,
not argued: the file says `sign_tripped: true`, no inverter is latched, nothing
is held at 0 W. The same state file onto a roster whose first entry IS
`inverter-a` comes up latched, so the finding is attributable to the name.
The staged upgrade reaches it too: the flat start stamps `schema_version`, and
the roster start's migration then takes its early return.
⚠️ NO RUNTIME CODE IS CHANGED HERE. The fix to `migrate_unit_keys()` belongs to
SAFETY-44 reopened as a defect, shown red before it; this commit is the
instrument that shows it red.
THE CHECK CAN FAIL, AND WAS SEEN TO. `test_rehearse.py` establishes two CLEAN
CONTROLS first - two transitions the shipped code gets right, on which the
harness must say nothing - and then breaks the shipped migration and the
entity-id composition six ways in a private copy of the tree, requiring each to
turn its control from green to red WITH THE RIGHT FINDING NAMED. One mutation
is required to SURVIVE, with the reason written down. The synthetic state
carries maintenance history and a legacy-shaped latched `sign_tripped` with no
`schema_version` beside it, because state built by the happy path cannot detect
a migration that only mishandles legacy shapes.
⚠️ THE FIRST DRAFT OF THE HARNESS HAD HABIT 4 IN IT and its own battery caught
it - MUTATION.md's thirteenth instance, third in an instrument. The entity diff
composed both sides with the same build, so renaming the MQTT device moved every
id equally and it reported "no orphans" about a change that orphans everything.
`--live-entities` is the fix: a reference that does not move with the code.
THE RETREAT is supported and does not corrupt state - the flat rollback view
keeps the latch and the first pack's history live - but it orphans eight HA
entities, and the ROUND TRIP is not supported: a maintenance cycle completed
while rolled back is discarded on re-advance. Stated in DOCS.md as a decision.
Fetching /data/state.json, the options blob and the entity list are OWNER STEPS
and are written down. ⚠️ Nothing here was run against real state and nothing
here claims to have been.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
The mutation battery printed every line of the harness's output starting with a
warning sign, and the harness's header is four paragraphs of them - so each
mutation's evidence arrived buried under "THIS TOOL RUNS THE REAL Controller"
repeated six times, and each finding appeared once per transition leg plus once
in the summary. The filter now matches a finding's KIND and de-duplicates, so
what part 3 prints is the set of distinct findings the mutation caused.
No behaviour change: the same six mutations go red for the same reasons.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
A store the add-on has never written has no latch and no history. The DEV-02
migration still runs on it - `schema_version` is absent, so `from_version` is 0 -
and it writes `sign_tripped = False` and `sign_reason = ""` under `inverter-a`
before `build_roster()` exists. On a declared roster nothing reads either of
them, so the first draft called both STRANDED and exited 1. Every fresh install
that grows a second inverter would have been reported as a failed transition.
⚠️ THAT IS A DEFECT IN THE ALARM, NOT A CASE TO SUPPRESS, and store.py already
carries the distinction it needed: `MigrationReport.lost` reports a dropped
`sign_tripped=True` and NOT a dropped `sign_tripped=False`, because "absent" and
"never tripped" are the same statement - and because an alarm that fires on the
harmless case is one an operator learns to dismiss before the real one arrives.
The stranded check now makes the same split, on the VALUE and not on the key
name: a key stranded at its default is printed under its own heading, visibly,
and is not a finding; the same key stranded at a non-default still is. SAFETY-44
turns on `sign_tripped = True`, so it is untouched, and a check pins both
halves against each other so the softening cannot widen into a hole.
CONTROL-C is the fresh install, and it is a third clean control rather than a
line in a comment: the shipped code is right about it and the harness must say
nothing. It is the control that found this.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
quiet when a flag was forgotten
Both blocking findings were reproduced before anything was changed, and both are
the class this card exists to guard against.
B1. THE PIN WAS A COUNT, AND A COUNT CANNOT SEPARATE TWO ROUTES. The rehearsal
reaches the disarmed latch by two independent paths and prints every finding
TWICE per red leg - once in the leg's block, once in the closing summary - so
ONE red leg satisfied `count(...) >= 2`. Measured: shipped code 4; with
`migrate_unit_keys()` handed the roster's own name (SAME-STEP fixed) 2, PASSES;
with a guard-side fix (both fixed) 0. The check named "NOT THE SAME-STEP PATH
ALONE" reported ok on the build whose only remaining failure IS the staged path.
The tool now prints `LEG <id>` / `END LEG <id>` and the pin requires the finding
in EACH leg by name.
⚠️ AND THE FIRST SPLITTER CARRIED THE SAME DEFECT ONE LEVEL DOWN - it opened on
the marker and never closed, so the last leg absorbed the closing summary and
read red whenever ANY leg was red. Caught by building the one fixture that
separates them. Part 5b is now that fixture, run every time: a tree in which
SAME-STEP is genuinely clean and STAGED is not. A per-thing check that has never
seen the things disagree is a total with extra steps.
⚠️ AND IT CORRECTS A CLAIM THIS CARD MADE. `main.py:250` is NOT the whole fix.
Measured on that exact change: LEG SAME-STEP clean, LEG STAGED still DISARMED -
the staged path stamps `schema_version` on the flat start and the roster start's
migration returns early before the name it is handed matters. DOCS.md now says
so, with the measurement.
B2. A FORGOTTEN FLAG WAS A CLEAN BILL OF HEALTH. Omitting `--live-entities`
printed "nothing orphaned" and exited 0, with `grep -ci` for the words returning
0 - while an UNREADABLE file exited 2. The asymmetry was backwards. Omitting it
now refuses (exit 2); `--no-live-entities` is the deliberate opt-out and carries
a standing banner in the header AND on the exit line, because a report is
skimmed from the bottom.
N1. THE REFERENCE DID NOT CARRY WHAT HA KEYS ON. It held entity_ids only, so
renaming `mqtt.BASE` (every unique_id moves) and changing `DEVICE["identifiers"]`
(HA sees a new device) both came back exit 0 - real orphanings with every
composed id byte-identical. The reference is now three columns
(`entity_id unique_id device`), is loaded on EVERY run including a degraded one,
and M7/M8 pin both. `object_id` is deliberately not a column and its rename is
required to SURVIVE, which is what bounds the four mqtt.py mutations.
Also found while fixing the above, and fixed with its own paired positive: the
VANISHED check did not carry the default/non-default rule the STRANDED check
does, so a pair of migrated DEFAULTS moving namespace read as two findings and
CONTROL-C went red on a CORRECT build. M9 drops non-default keys and must still
be a finding.
N2/N3. DOCS.md withdraws the unmeasured "not recoverable by advancing again" -
its own reasoning cut the other way and nothing here tested a re-advance; the
measured part (eight rows stop updating, history gaps, automations stop firing)
stands. And the byte-identical-input property is `test_rehearse.py`'s assertion,
not the tool's.
FRAMING. SAFETY-44 is ALREADY RECORDED on the merge base - `test_maintenance.py`
section 13, with the "guard is defeated by the very key the defect wrote"
sentence, in a file this card does not touch. DOCS.md now says this is
independent re-confirmation through a second instrument, reachable from an
owner-supplied state file rather than only from a fixture, and not a discovery.
app/ is still untouched. 59 checks in test_rehearse.py, 9 mutations red, 2
required survivors, 3 clean controls.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
The mutation battery prints the distinct findings each mutation caused, matched
on the finding's KIND. The pattern's character class was `[A-Z -]`, which has no
underscore - so `UNIQUE_ID DRIFT:` did not match, and M7, the mutation that
renames `mqtt.BASE`, printed its verdict with no evidence under it at all.
A filter that silently drops the very finding a mutation exists to produce is
the same defect as the rest of this card, at display scale. No behaviour change:
M7 was red before and is red now, and it now shows why.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
admin
changed title from ARCH-02: rehearse the flat-to-roster switch - and SAFETY-44 is a defect again, measured to ARCH-02: rehearse the flat-to-roster switch - and SAFETY-44 re-confirmed by a second instrument2026-09-08 14:31:20 +02:00
Review approved and then attacked part 5b, the control that stands between B1's
defect and a green suite. It got through in three edits:
A1 delete `END LEG` 5b CATCHES it 2 FAIL
A2 ...and reorder the legs 5b BLIND 1 FAIL
A3 ...and reword the closing summary 0 FAIL, "all checks passed"
with a splitter that closed
ZERO legs
⚠️ TWO OF THOSE THREE ARE ORDINARY REFACTORS - reordering report legs, rewording
a summary line - and neither reads as touching a safety check. 5b caught the
unclosed splitter only because the clean leg happened to be printed LAST, and
the backstop that saved A2 was a hardcoded transcription of the tool's own
sentence, so rewording that sentence blinded it too.
FIXED BY ASSERTING THE STRUCTURE RATHER THAN ADDING A FIXTURE: a leg may not
open while one is open, and every leg that opens must be explicitly closed.
Driven - deleting `END LEG` now fails on the FIRST edit, whatever the order or
the wording:
AssertionError: leg 'SAME-STEP' opened while 'STAGED' was still open - the
tool stopped closing its legs, and a splitter that never closes swallows the
closing summary into the last leg.
All three attacks include that edit, so one assertion kills the class. Clean
tree unchanged: 28/28, all checks passed.
⭐ AND THE RULE IN MUTATION.md IS EXTENDED, because review's finding is the
general case of it. It said "a per-thing check that has never seen the things
DISAGREE is a total". Part 5b DOES make them disagree, every run, and was still
blind. So:
...AND ONE THAT HAS ONLY SEEN THEM DISAGREE IN ONE ARRANGEMENT IS A TOTAL
OVER THAT ARRANGEMENT. When a fixture cannot enumerate the arrangements,
assert the invariant they share.
Also recorded, NOT fixed, with the reason: the reference carries `entity_id
unique_id device` and grades all three, but never the discovery TOPIC. Review
measured `homeassistant/... -> ha/...` exiting 0 with no findings while every
announced entity dies. `discovery_payloads()` already returns the topic and
`announced_rows` discards it, so a fourth column closes it - but the prefix is
HA's fixed contract rather than a name this project chooses, so it is not what a
tidy-up rename reaches. Lower LIKELIHOOD than the N1 gap it resembles, not lower
consequence, and it stops being a note the day the topic becomes configurable.
Two stale figures corrected: the tool claimed five migration mutations where
there are nine, and pointed at a DOCS.md heading that does not exist (the real
one is "Before you commission inverter B", verified present at DOCS.md:1577).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
admin
merged commit 46c022c926 into release/1.02026-09-08 15:09:28 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
What this is
goodwe_controller/tools/rehearse_roster.pyruns the realController—not a stub — through the flat → roster transition that inverter B's
commissioning performs, over a copy of a state file given as an input path,
and reports the difference:
Exit
0clean ·1at least one finding ·2could not run (not a pass).It reports every store key that moved, appeared, vanished or was STRANDED
under a name no machine reads; every entity id before and after with orphans
named; and whether the maintenance history MAINT-05 already moved once
survived being moved again.
⚠️ Nothing here was run against real state and nothing here claims to have
been. SSH to the live box is blocked by policy and that is correct. Fetching
/data/state.json, the options blob and the entity list are owner steps,written down in
DOCS.mdunder Before you commission inverter B.⚠️ No runtime code is changed in this PR.
app/is untouched; the diff isone tool, one test file, six fixtures and two documents.
⭐ SAFETY-44's answer: the asymmetry is not safe, and it is a defect again
The card asks whether
migrate_unit_keys(store, UNIT_A)taking a hardcodedconstant — while
migrate_maint_keys(store, unit, roster)takes the roster andhas a
misdirectedguard — leaves the two latch keys unread on a store that ranpre-DEV-02 flat and declares a multi-inverter roster.
It does. Measured, not argued:
The mechanism is worse than "a key nothing reads".
_grade_store_migration()isthe guard written for exactly this case — flat says tripped and no inverter came
up tripped — and it is defeated by the migration's own misdirected write:
migrate_unit_keys()depositsunit/inverter-a/sign_tripped = True, thenamespaced_latchedtest finds it, concludes something namespaced is latched,and returns without re-arming anything. The file says
sign_tripped: true, thebanner is green, nothing is held at 0 W.
migrate_maint_keys()would have called thismisdirected, withheld the markerand re-run.
migrate_unit_keys()has no such guard, and the documented defence(
inverter.py:161-163, "flat pre-DEV-02 keys can only ever have belonged to asingle-inverter install") is true about which install they came from and
silent about what that install's first roster entry is now called.
Both routes reach it, not only the same-step one:
runs and misdirects;
restart: the first start stamps
schema_version = 1, so the roster start'smigration takes its
from_version >= SCHEMA_VERSIONearly return and the latchnever moves at all. Same disarmed end state.
The contrast run is what makes this attributable: the same state file onto a
roster whose first entry is
inverter-acomes up correctly latched. Thefinding is caused by the roster name, not by the fixture being tripped.
SAFETY-44 should be reopened as a defect. The fix is not in this PR — a
change to
migrate_unit_keys()or tonamespaced_latchedis a safety changethat deserves its own card and its own red-before-green, and burying it in a
rehearsal PR would hide it.
test_rehearse.pypart 5 pins the current behaviouras a requirement, so that fix turns this file red and has to update it
deliberately.
⭐ The check can fail, and was seen to
Three clean controls come first, because six reds prove nothing about an
instrument that is red on its own fixtures:
entry is
inverter-a→ exit 0, no findings.garage/shed→ exit 0, no findings.
garage/shed→ exit0, no findings. ⚠️ This one found a defect in the harness, which is what a
control is for: the DEV-02 migration writes
sign_tripped = Falseandsign_reason = ""underinverter-abefore any roster exists, and the firstdraft called both STRANDED — red on every fresh install that grows a second
inverter. Losing a default loses nothing; that is
MigrationReport.lost'sown distinction, and store.py's own argument that an alarm which fires on the
harmless case is one an operator learns to dismiss before the real one
arrives. The stranded check now splits on the value, not the key name: a
default is printed under its own heading and is not a finding; the same key at
a non-default still is. A check pins both halves against each other so the
softening cannot widen into a hole — SAFETY-44 turns on
sign_tripped = True.Then six deliberate mutations of the shipped migration and of the entity-id
composition, each applied to a private copy of the tree (source edits, not
monkeypatches; every
str.replaceasserted to have matched exactly once), eachrequired to flip its control green → red with the right finding named:
One mutation is required to SURVIVE, with the reason written down, because a
battery in which everything dies is a battery somebody stopped writing as soon as
the number looked good: renaming
store.unit_key()— the one spelling — movesthe writer and every reader together, so the store comes out internally
consistent under a different name. It is a rollback contract, and grading it
needs two versions of the code, which is a different instrument. The check
asserts the survival and asserts the store really changed shape underneath it,
so it is a survivor of a mutation that happened.
The synthetic state carries maintenance history and a legacy-shaped key
whose loss would be silent — a latched
sign_trippedwith noschema_versionbeside it, i.e. written by a controller that had never heard of unit namespaces.
⚠️ The first draft of this harness had habit 4 in it
MUTATION.md's thirteenth instance, and the third in an instrument ratherthan in a check — caught by this PR's own mutation battery, which is the cheap
place. The entity diff composed both sides with the same build, so renaming
the MQTT device moved every id equally and the harness reported "no orphans"
about a change that orphans everything. M5 came back
exit 0, M6exit 2, andneither was the failure they name. The anchor had the same shape: it was
compose(mqtt.DEVICE["name"], …), which any rename of the device satisfies bymoving both operands.
The fix is habit 2 with a filename:
--live-entitiestakes a list of the ids theinstall actually has — data that does not move with the code — and the anchor
is now composed from two recorded literals. Recorded in
MUTATION.md.The reverse direction: the retreat is supported, the round trip is not
Rehearsed, both legs, on the fixtures.
The retreat (roster → flat) does not corrupt state. The SAFETY-02 latch and
the first pack's maintenance history both survive, because
store.pyrule 2keeps the flat spellings live rather than merely present —
maintenance.UnitHistory.mirror_flatwrites them on every change, so asingle-FSM add-on finds a current history, not one dated the day the roster was
declared. But it orphans eight Home Assistant entities — the four per-unit
rows for each of the two units. Their history stops and any automation naming
them stops firing, silently, and advancing again does not bring them back
because HA keeps the id it composed at first discovery.
⚠️ The round trip is NOT supported, and that is the stated decision. A
maintenance cycle that completes while the install is rolled back is
discarded on re-advance:
maint_schema_versionis already stamped, somigrate_maint_keys()takes itsearly return and the pack reads the namespaced history from before the retreat.
Retreat freely, but treat a retreat as ending the current maintenance
bookkeeping — and if a cycle ran during it, correct
unit/<first>/last_completedby hand before advancing again. Written intoDOCS.mdas a decision rather than left to be discovered at the inverter.That leg is itself habit-4-proofed: the retreat window actually contains a
completion, written through the shipped
UnitHistoryrather than a rawstore.set(), at a timestamp the starting fixture does not carry. A round tripover a store nothing changed in between compares a structure to itself.
Design notes
Controllerand the realStore.RehearsalStoresubclassesapp.store.Storeand is pointed at a scratch copy made withmkstemp(), soload(),save(),get_time()andset_time()are the shipped ones and theoperator's file is never opened for write. ⚠️
test_rehearse.py— not thetool — is what asserts the input file is byte-identical afterwards,
including after the writing legs. The tool copies; the check verifies.
Store.datais a dictsubclass that records every value-read; the probe then re-runs the shipped
constructors (
Inverter.__init__per roster entry,UnitHistoryoverMAINT_UNIT_KEYS) against the post-transition store. A key in the file thatno reader touched is a key whose value is not information. Writing the key
names out here instead would be a second transcription of
UNIT_STORE_KEYS,and the copy that drifts is always the one that reads.
mqtt._slug, for the reasontest_mqtt.pypart 6 gives.yamlmodule in the check image, so the tool carries a narrow readerfor the subset an options file is written in — and it refuses what it does
not understand rather than guessing, because a config silently mis-parsed here
rehearses a configuration nobody is going to deploy. JSON (
/data/options.json)is accepted directly and PyYAML is used when importable.
Acceptance
Controllerover the transition, state file as an input path/data/state.jsoncopyVerification
Full suite in the CI image,
.gitea/ci/Dockerfilebuild, tree at<ws>/goodwe-addon/:Positive control before the run:
M4 turns CONTROL-A REDwas broken on purpose→ runner reported
FAIL M4 turns CONTROL-A RED, exit 1; restored byte-identical;exit 0 again.
🤖 Generated with Claude Code
https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
Round 2 — rejected on two blocking findings, both fixed and both now pinned
Both were reproduced before anything was changed. Both are the class this card
exists to guard against, and both were in the instrument, not in the
conclusion.
B1 — the pin was a count, and a count cannot separate two routes
The rehearsal reaches the disarmed latch by two independent paths, and the tool
printed every finding twice per red leg (once in the leg's block, once in
the closing summary). So
out.count("LATCH CAME UP DISARMED") >= 2wassatisfied by one red leg. Reproduced across three builds:
migrate_unit_keys()handed the roster nameThe check named "NOT THE SAME-STEP PATH ALONE" reported
okon the buildwhose only remaining failure is the staged path.
Fixed: the tool prints
LEG <id>…END LEG <id>; the pin splits on thoseand requires the finding in each leg by name.
⚠️ And the first splitter carried the same defect one level down
Opening on
LEG <id>and never closing gave every line after the last markerto that leg — including the closing summary, which repeats every finding from
every leg. The last leg then read red whenever any leg was red, so the new
per-leg check was the old count with extra steps. Caught by building the one
fixture that separates them.
Part 5b is now that fixture, run every time — a tree in which
SAME-STEPisgenuinely clean and
STAGEDis not:A per-thing check that has never seen the things disagree is a total with extra
steps. Recorded in
MUTATION.mdas habit 4's fourteenth and fifteenthinstances.
⚠️ This corrects a claim the PR made
main.py:250is not the whole fix. Measured on exactly that change: itrepairs
SAME-STEPand leavesSTAGEDdisarmed, because the staged path stampsschema_versionon the flat start and the roster start's migration returnsearly before the name it is handed matters. What covers both is the guard
side —
namespaced_latchedcounting a namespace that is not on today'sroster. DOCS.md now carries the table and the measurement. (That line's existing
comment deliberately admits off-roster keys, so it is a design decision for the
MAINT-05 author, not a one-liner — which is why the fix is still not here.)
B2 — a forgotten flag was a clean bill of health
Reproduced:
An unreadable file exited 2; the flag simply not passed exited 0 with no
disclosure at all. The asymmetry was backwards, and the owner runs this once, at
commissioning, under time pressure.
Fixed: omitting
--live-entitiesnow refuses (exit 2).--no-live-entitiesis the deliberate opt-out and carries a standing banner inthe report header and on the exit line, because a report is skimmed from the
bottom. Contradictory flags refuse. Part 2b pins all of it.
N1 — the reference did not carry what Home Assistant keys on
Reproduced, all with
--live-entities:The first two are real orphanings with every composed entity_id
byte-identical. HA keys its entity registry on
unique_idand its deviceregistry on the identifier; discovery is retained with no removal payload, so
the old config stays on the broker and the dead entity stays in the registry.
Fixed:
announced-reference.txtis now three columns(
entity_id unique_id device), is loaded on every run — including adegraded one — and is graded at a fixed canonical shape rather than against the
operator's config (grading it against
--beforereported a falsesensor.goodwe_p1_sample_agedrift on any site withmeter_source: off).Two new mutations pin it, and the no-op is now a required survivor — without
a change to
mqtt.pythat must not be caught, "the reference notices any editto mqtt.py" and "the reference notices what HA keys on" are the same green:
Found while fixing the above
VANISHEDdid not carry the default/non-default ruleSTRANDEDdoes. Apair of migrated defaults moving namespace read as two findings, and
CONTROL-C went red on a correct build. Same rule, same argument
(
MigrationReport.lost), now applied to both — and bounded by M9, whichdeletes non-default keys and must still be a finding (
last_completed,phase_started,last_start_attempt— and notphase, whose'idle'isthe default, on the same mutation).
raw()concatenated stdout and stderr, so "is the banner near the end ofthe report" read the last lines of the controller's log. Split.
get_time()onphase, making the store logunparseable timestampat WARNING about a healthy store.get()alonerecords the same read.
regex's class had no underscore, so
UNIQUE_ID DRIFT:printed a verdict withno evidence under it.
N2 / N3 — corrections to the prose
advancing again: HA keeps the id it composed at first discovery" was
unmeasured and its own reasoning cut the other way. The measured part — the
eight rows stop updating, history gaps, automations stop firing silently —
stands; the recovery claim is withdrawn rather than restated in the other
direction.
test_rehearse.py, not by the tool. The tool copies to anmkstemp()scratchfile; the check is what verifies it.
⚠️ Framing correction — this is re-confirmation, not discovery
SAFETY-44 is already recorded on the merge base.
test_maintenance.pysection 13 states it, names
namespaced_latched, and carries the "guard isdefeated by the very key the defect wrote" sentence verbatim — in a file this
PR does not touch. What is new is that the finding is reachable from a state
file the owner supplies, by a different route (a full
Controllerstartdiffed against its predecessor rather than assertions over a
MemStore). Twoinstruments, one answer. DOCS.md's section is retitled "What it confirms" and
says this in its first paragraph.
Round-2 verification
app/still untouched.test_rehearse.py: 59 checks, 9 mutations red withthe right finding named, 2 required survivors, 3 clean controls.
Positive control on the new per-leg check: inverted
LEG STAGEDmust-be-red →runner reported
FAIL ⚠️ ...while the STAGED leg is STILL DISARMED, exit 1;restored byte-identical; exit 0.
ARCH-02: rehearse the flat-to-roster switch - and SAFETY-44 is a defect again, measuredto ARCH-02: rehearse the flat-to-roster switch - and SAFETY-44 re-confirmed by a second instrumentReview approved and then attacked part 5b, the control that stands between B1's defect and a green suite. It got through in three edits: A1 delete `END LEG` 5b CATCHES it 2 FAIL A2 ...and reorder the legs 5b BLIND 1 FAIL A3 ...and reword the closing summary 0 FAIL, "all checks passed" with a splitter that closed ZERO legs ⚠️ TWO OF THOSE THREE ARE ORDINARY REFACTORS - reordering report legs, rewording a summary line - and neither reads as touching a safety check. 5b caught the unclosed splitter only because the clean leg happened to be printed LAST, and the backstop that saved A2 was a hardcoded transcription of the tool's own sentence, so rewording that sentence blinded it too. FIXED BY ASSERTING THE STRUCTURE RATHER THAN ADDING A FIXTURE: a leg may not open while one is open, and every leg that opens must be explicitly closed. Driven - deleting `END LEG` now fails on the FIRST edit, whatever the order or the wording: AssertionError: leg 'SAME-STEP' opened while 'STAGED' was still open - the tool stopped closing its legs, and a splitter that never closes swallows the closing summary into the last leg. All three attacks include that edit, so one assertion kills the class. Clean tree unchanged: 28/28, all checks passed. ⭐ AND THE RULE IN MUTATION.md IS EXTENDED, because review's finding is the general case of it. It said "a per-thing check that has never seen the things DISAGREE is a total". Part 5b DOES make them disagree, every run, and was still blind. So: ...AND ONE THAT HAS ONLY SEEN THEM DISAGREE IN ONE ARRANGEMENT IS A TOTAL OVER THAT ARRANGEMENT. When a fixture cannot enumerate the arrangements, assert the invariant they share. Also recorded, NOT fixed, with the reason: the reference carries `entity_id unique_id device` and grades all three, but never the discovery TOPIC. Review measured `homeassistant/... -> ha/...` exiting 0 with no findings while every announced entity dies. `discovery_payloads()` already returns the topic and `announced_rows` discards it, so a fourth column closes it - but the prefix is HA's fixed contract rather than a name this project chooses, so it is not what a tidy-up rename reaches. Lower LIKELIHOOD than the N1 gap it resembles, not lower consequence, and it stops being a note the day the topic becomes configurable. Two stale figures corrected: the tool claimed five migration mutations where there are nine, and pointed at a DOCS.md heading that does not exist (the real one is "Before you commission inverter B", verified present at DOCS.md:1577). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU