SAFETY-44: the latch migration takes the roster, and the guard stops being defeated by the key the defect wrote

⚠️ A DISARMED SAFETY-02 GUARD ON AN INSTALL WHOSE STATE FILE SAYS IT IS
PROTECTED. `Controller.__init__` called `migrate_unit_keys(store, UNIT_A)` - a
hardcoded constant - while its sibling `migrate_maint_keys(store, unit, roster)`
takes the roster and refuses a write no machine reads. On a store that ran
pre-DEV-02 flat and declares `inverters: [garage, shed]`, the latch moved to
`unit/inverter-a/sign_tripped`, `_verify_moves()` read it back under the name it
had been HANDED, was satisfied, and stamped the marker. Not one inverter came up
latched, and the next start skipped the migration too.

That is the MAINT-05 defect verbatim, still live in the function whose own
docstring describes it, and `_grade_store_migration()` did not catch it: its
"is anything namespaced latched" test was satisfied BY THE MISDIRECTED VALUE
ITSELF.

⚠️ THE REASON IT WAS LEFT WAS A HALF-TRUTH. "There is no roster yet - the
`Inverter`s restore their latch in their constructors." The KEYS must be written
before the `Inverter`s read them; the NAMES do not come from the `Inverter`s.
`roster_names(opts)` reads them off the options before anything is constructed -
which is exactly what it already existed to do for `MqttPublisher` - and returns
`[UNIT_A]` on the flat shape, so today's upgrade path is byte-identical.

TWO EDITS, AND THEY DO DIFFERENT JOBS. Measured per leg by reverting each one:

                          SAME-STEP leg          STAGED leg
  both edits (shipped)    clean                  latch armed, litter reported
  guard reverted          latch armed            ⚠️ DISARMED
  call site reverted      latch armed, litter    latch armed, litter

So the GUARD carries the safety and the CALL SITE carries the hygiene and the
withheld marker. The table is in test_rehearse.py part 5 because a reader who
assumed the roster-aware migration re-arms the latch would delete the guard as
redundant in a tidy-up and take the STAGED column with it - and STAGED is what
"upgrade now, declare the roster later" produces, which is the path an operator
actually takes. Only the guard reaches an install already stamped by 0.9.0.

⚠️ AND "OFF THE ROSTER" ALONE WAS TOO BLUNT, WHICH test_inverter.py CAUGHT. The
first draft treated every off-roster latch as litter. In DEV-02's restart case
the store carries `unit/inverter-b/sign_tripped = True` from B's own trip while
the roster being graded holds only A - and the blunt rule re-armed a HEALTHY A
off the flat key B's trip had mirrored. DEV-02's defect rebuilt inside its own
guard, which is what the paragraph I was overruling had been right to fear.

Narrowed to `UNIT_A`, which is the only name the broken migration ever wrote -
`roster_names()` falls back to a POSITIONAL `inverter-N`, so `inverter-a` is not
a name this code hands to anyone else. A latch stranded under any other name is
a real unit out of today's config and still counts as protection. Residual,
stated in the code rather than left to be found: a site whose own first unit is
NAMED `inverter-a`, latched, then taken out of the roster gets re-armed on
`inverters[0]` - a healthy unit held at 0 W until an operator clears it. The
conservative direction, against a site DRIVING on a file that says it is latched.

⭐ AND FIXING THE CODE EXPOSED A FALSE POSITIVE IN THE HARNESS. With the latch
now landing under `garage`, `rehearse_roster.py` reported VANISHED STORE KEY for
`unit/inverter-a/sign_tripped` three lines above the same value sitting under
`unit/garage/sign_tripped`. A key that moved to a namespace a machine reads has
not vanished - and an alarm that fires on the harmless case is one an operator
learns to dismiss before the real one arrives, which is CONTROL-C's own
argument. The test is not "the name appears somewhere": same bare key, same
value, under a key in `machine_read`, the set built by driving a real start. A
latch that genuinely went missing has no such key and is still a finding, which
M3 (delete the flat rollback view) still measures red.

Test parts replaced deliberately, as both of them asked to be:
  - test_rehearse.py part 5 asserted the defect "as REQUIREMENTS so that a fix
    turns them red". It now asserts the fix, with 5b and 5c as negative controls
    - one per edit - and 5b doubles as the leg-splitter's positive control,
    since reverting the guard is the build where the two legs DISAGREE.
  - test_maintenance.py section 13 was headed "RECORDED, NOT FIXED" and said the
    fixing card "should expect them to go red and should replace them". Replaced,
    plus two cases it could not have had: the already-stamped store, and the
    `inverter-b` negative control for the narrowing above.

28/28 files pass. Not run: nothing - this is host-side only, no rig, no hardware.

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

goodwe_controller/app/main.py

@@ -247,7 +247,19 @@ class Controller:
             hold_s=float(opts.get("delivery_hold_s",
                                   DELIVERY_OPTION_DEFAULTS["delivery_hold_s"])),
         )
-        self.store_migration = migrate_unit_keys(store, UNIT_A)
+        # SAFETY-44. THE NAMES THE MACHINES READ, HANDED TO THE MIGRATION THAT
+        # WRITES FOR THEM. This line used to read `migrate_unit_keys(store,
+        # UNIT_A)`, and the constant was wrong in exactly the way line 315's
+        # comment says the SAME constant would be wrong two hundred lines below
+        # it: on a declared roster the flat latch landed under
+        # `unit/inverter-a/`, which is on no roster, was read back under the
+        # name it was handed, and stamped the marker. `roster_names()` is the
+        # naming rule itself and returns `[UNIT_A]` on the flat shape, so the
+        # upgrade path every install runs today is byte-identical - and a
+        # declared roster now gets `misdirected`, no marker, and a retry.
+        _roster_names = roster_names(opts)
+        self.store_migration = migrate_unit_keys(
+            store, _roster_names[0] if _roster_names else UNIT_A, _roster_names)
         self.inverters = build_roster(opts, store, sign_cfg, delivery_cfg)
         # ⚠️ CFG-04. SAY WHICH SHAPE IS IN FORCE, BECAUSE THE OTHER ONE'S KEYS
         # ARE STILL SITTING IN THE UI. A declared `inverters:` roster reads none
@@ -751,23 +763,79 @@ class Controller:
         legacy_tripped = bool(data.get("sign_tripped", False))
         if not legacy_tripped or not self.inverters:
             return
-        # ⚠️ THE QUESTION IS "IS ANY NAMESPACED LATCH SET", AND IT IS ASKED OF
+        # ⚠️ THE QUESTION IS "IS ANY NAMESPACED LATCH SET **THAT A MACHINE
-        # THE STORE RATHER THAN OF THE ROSTER. At a roster of two a trip on B
+        # READS**", AND THE LAST CLAUSE IS SAFETY-44's SECOND HALF. At a roster
-        # leaves A untripped, so re-arming A off a flat key that B's own trip
+        # of two a trip on B leaves A untripped, so re-arming A off a flat key
-        # mirrored would latch a healthy inverter on every restart - the
+        # that B's own trip mirrored would latch a healthy inverter on every
-        # flat-key defect this card removed, coming back through its own guard.
+        # restart - the flat-key defect DEV-02 removed, coming back through its
-        # And the STORE is the right place to ask: it holds every
+        # own guard. That much stands, and it is why the question is asked of
-        # `unit/*/sign_tripped` this install has ever written, including one
+        # the STORE's keys rather than of the roster's objects.
-        # belonging to a unit that is not in TODAY's roster (a roster is
+        #
-        # configuration and it changes), whereas the roster only knows what is
+        # ⚠️ BUT IT USED TO ACCEPT **ANY** `unit/*/sign_tripped`, and the
-        # configured right now. The flat key carries new information only when
+        # paragraph here defended that: the store "holds every one this install
-        # nothing namespaced is latched at all.
+        # has ever written, including one belonging to a unit that is not in
-        namespaced_latched = any(
+        # TODAY's roster ... whereas the roster only knows what is configured
-            bool(v) for k, v in data.items()
+        # right now". THE PREMISE IS TRUE AND IT ARGUES THE WRONG WAY. A latch
-            if k.startswith("unit/") and k.endswith("/sign_tripped"))
+        # under a name no unit answers to is not protection an operator has - it
+        # is a key nothing reads, which is the definition of the state this
+        # method exists to repair. Counting it as protection made the guard
+        # return early on precisely the install that needed it, and that is not
+        # hypothetical: it is where the migration above USED to put the latch
+        # on every declared roster, so the guard was blinded by its own upgrade.
+        #
+        # THE MIGRATION FIX DOES NOT REACH THOSE INSTALLS. A store stamped by
+        # the shipped version already has the marker, so `migrate_unit_keys()`
+        # returns at the version check and never runs again. This guard is the
+        # ONLY repair path for a site that upgraded before this card, and it
+        # re-arms onto `inverters[0]` - which lands the latch back under a name
+        # the roster reads.
+        # ⚠️ AND "OFF THE ROSTER" IS NOT ENOUGH ON ITS OWN - IT HAS TO BE THE
+        # MIGRATION'S OWN FINGERPRINT, WHICH IS `UNIT_A`. The first draft of
+        # this treated EVERY off-roster latch as litter, and test_inverter.py's
+        # DEV-02 restart case caught it: there the store carries
+        # `unit/inverter-b/sign_tripped = True` from B's own trip while the
+        # roster being graded holds only A, and the blunt rule re-armed a
+        # HEALTHY A off the flat key that B's trip had mirrored - DEV-02's
+        # defect rebuilt inside its own guard, which is exactly what the
+        # paragraph above was right to be afraid of.
+        #
+        # `UNIT_A` is the discriminator because it is the ONLY name the broken
+        # migration ever wrote. `roster_names()` falls back to a POSITIONAL
+        # `inverter-N`, so `inverter-a` is not a name this code hands to anyone
+        # else. A latch stranded under any OTHER name is a real unit that is
+        # out of today's config - a roster is configuration and it changes -
+        # and it still counts as protection.
+        #
+        # ⚠️ THE RESIDUAL, STATED RATHER THAN LEFT TO BE FOUND: a site whose
+        # own first unit is NAMED `inverter-a`, latched, and then taken out of
+        # the roster gets re-armed on `inverters[0]`. That is a healthy unit
+        # held at 0 W until an operator clears it, which is the conservative
+        # direction; SAFETY-44's is a site DRIVING on a state file that says it
+        # is latched.
+        on_roster = {inv.name for inv in self.inverters}
+        stranded, namespaced_latched = [], False
+        for k, v in data.items():
+            if not (k.startswith("unit/") and k.endswith("/sign_tripped")):
+                continue
+            if not bool(v):
+                continue
+            # `unit_key()` builds `unit/<unit>/<key>`, so the unit is field 1 -
+            # split through the writer's own spelling rather than re-deriving.
+            unit = k.split("/")[1]
+            if unit in on_roster or unit != UNIT_A:
+                namespaced_latched = True
+            else:
+                stranded.append(k)
         if namespaced_latched or any(inv.sign_wd.tripped
                                      for inv in self.inverters):
             return
+        if stranded:
+            _LOG.error(
+                "SAFETY-44: this store carries a LATCHED sign-disagreement "
+                "failsafe under %s, and that unit is NOT on today's roster "
+                "%s - so no machine reads it and the site came up DRIVING on "
+                "a state file that says it is latched. Re-arming below.",
+                ", ".join(sorted(stranded)), sorted(on_roster))
         first = self.inverters[0]
         # The one state this method exists for.
         reason = str(data.get("sign_reason", "") or "")

goodwe_controller/app/store.py

@@ -253,7 +253,7 @@ class MigrationReport:
         return out
 
 
-def migrate_unit_keys(store, unit: str) -> MigrationReport:
+def migrate_unit_keys(store, unit: str, roster=None) -> MigrationReport:
     """Move the pre-DEV-02 flat per-inverter keys into `unit`'s namespace.
 
     `unit` is the FIRST inverter in the roster, which on every install that can
@@ -261,6 +261,24 @@ def migrate_unit_keys(store, unit: str) -> MigrationReport:
     version that knew about more than one inverter is already at
     SCHEMA_VERSION and is not touched.
 
+    ⚠️ `roster` IS THE NAMES THE MACHINES READ, AND IT IS SAFETY-44. This
+    function shipped WITHOUT it, taking a hardcoded `inverter.UNIT_A` at its one
+    call site - which is, exactly, the MAINT-05 defect `_verify_moves()` spends
+    a paragraph describing, still live in the sibling that paragraph was written
+    beside. On a store that ran pre-DEV-02 flat and declares
+    `inverters: [garage, shed]`, the latch moved to `unit/inverter-a/
+    sign_tripped`, was read back under that same name, was SATISFIED, and
+    stamped the marker - so the next start skipped the migration too and NO
+    inverter came up latched on an install whose state file says `sign_tripped:
+    true`. A disarmed SAFETY-02 guard, every symptom of a clean migration.
+
+    `roster_names(opts)` answers this BEFORE any `Inverter` exists - it is
+    defined to, for `MqttPublisher`, which is constructed before the roster for
+    the same reason - so the "there is no roster yet" that kept this argument
+    out was never true. On the flat shape it returns `[UNIT_A]`, so the upgrade
+    every install runs today passes `unit == roster[0] == UNIT_A` and is
+    byte-identical.
+
     THE THREE RULES, each one a case that would otherwise be a wrong answer:
 
     1. IT NEVER OVERWRITES AN EXISTING NAMESPACED VALUE. A store that has both
@@ -302,7 +320,7 @@ def migrate_unit_keys(store, unit: str) -> MigrationReport:
 
     rep.ran = True
     _move_flat_keys(store, unit, LEGACY_UNIT_KEYS, rep)
-    _verify_moves(store, rep)
+    _verify_moves(store, rep, roster)
 
     if rep.ok:
         store.set(SCHEMA_KEY, SCHEMA_VERSION)
@@ -383,12 +401,26 @@ def _verify_moves(store, rep: MigrationReport, roster=None) -> None:
     itself. `test_maintenance.py` section 9c calls this twice with the SAME
     wrong `unit` over one store, which is the shape nothing in the suite had.
 
-    `roster=None` means "not graded", and the one caller that passes it is
+    `roster=None` still means "not graded", and SAFETY-44 is what it cost. This
-    `migrate_unit_keys()`, which runs BEFORE `build_roster()` for the reason its
+    paragraph used to end here, saying that the one caller passing it was
-    own docstring gives - the `Inverter` objects restore their latch in their
+    `migrate_unit_keys()`, "which runs BEFORE `build_roster()` ... so the keys
-    constructors, so the keys have to be in place before there is a roster to
+    have to be in place before there is a roster to check against", and calling
-    check against. That gap is real and is recorded as a running check rather
+    that gap real. THE FIRST HALF IS TRUE AND THE CONCLUSION IS NOT. The keys do
-    than left to a comment: see test_maintenance.py section 13.
+    have to be written before the `Inverter`s restore from them - but the NAMES
+    do not come from the `Inverter`s. `roster_names(opts)` reads them straight
+    off the options, before anything is constructed, which is the whole reason
+    it exists (`MqttPublisher` is built before the roster too). So the argument
+    that could have graded that write was available at the call site the entire
+    time, and the sibling function three screens down had already been fixed by
+    passing it.
+
+    ⚠️ SO NO CALLER PASSES `None` TODAY, and this branch is kept for one reason:
+    it is what a NEW caller gets by DEFAULT. A future migration that forgets the
+    roster gets the ungraded read-back and stamps its marker over a namespace no
+    machine reads - silently, exactly as this one did. That is the shape of both
+    defects, and the gap is recorded as a running check rather than left to a
+    comment: test_maintenance.py section 13, and test_rehearse.py part 5, which
+    drives it end-to-end through the harness an operator actually runs.
     """
     for target, value in rep.moved.items():
         got = getattr(store, "data", {}).get(target, "<<missing>>")

goodwe_controller/test_maintenance.py

@@ -34,7 +34,7 @@ from app.inverter import UNIT_A
 from app.main import Controller
 from app.store import (
     DEFAULTS, MAINT_SCHEMA_KEY, MAINT_SCHEMA_VERSION, MAINT_UNIT_KEYS,
-    migrate_maint_keys, unit_key,
+    SCHEMA_KEY, SCHEMA_VERSION, migrate_maint_keys, unit_key,
 )
 
 fails = []
@@ -1411,39 +1411,33 @@ check("...while the SAME site with the house importing more than the drain "
 
 # --------------------------------------------------------------------------- #
 print()
-print("13. RECORDED, NOT FIXED: SAFETY-02's migration has the same "
+print("13. FIXED (SAFETY-44): SAFETY-02's migration takes the roster too, and "
-      "wrong-namespace hole, and it is a LATCH")
+      "the guard is no longer defeated by the key the defect wrote")
 
-# ⚠️ FOUND WHILE FIXING THIS CARD'S OWN INSTANCE, AND WRITTEN DOWN AS A RUNNING
+# ⚠️ THIS SECTION USED TO BE THE SPECIFICATION OF A DEFECT. It was written while
-# CHECK RATHER THAN A COMMENT SO IT CANNOT BE FORGOTTEN OR QUIETLY "FIXED" BY AN
+# fixing MAINT-05's own instance, as a RUNNING check rather than a comment, with
-# UNRELATED EDIT. It is the SAME defect one module over, and it costs more.
+# the instruction that "the card that fixes it should expect them to go red and
+# should replace them". SAFETY-44 is that card and this is that replacement.
 #
-# WHAT IT IS. `Controller.__init__` calls `migrate_unit_keys(store, UNIT_A)` -
+# WHAT IT WAS. `Controller.__init__` called `migrate_unit_keys(store, UNIT_A)` -
-# a hardcoded constant, where the maintenance migration now passes the roster's
+# a hardcoded constant, where the maintenance migration already passed the
-# own first name. It has to run BEFORE `build_roster()`, because each
+# roster's own first name. On a DECLARED roster the pre-DEV-02 flat
-# `Inverter` restores its sign latch in its constructor, so there is no roster
+# `sign_tripped: true` landed under `unit/inverter-a/sign_tripped`, which no
-# to name yet and `_verify_moves()` is passed `roster=None`. On a DECLARED
+# unit on the site reads, and `_verify_moves()` read it back under the name it
-# roster the pre-DEV-02 flat `sign_tripped: true` therefore lands under
+# had been handed and was satisfied.
-# `unit/inverter-a/sign_tripped`, which no unit on this site reads.
 #
-# WHY THE EXISTING GUARD DOES NOT CATCH IT, WHICH IS THE PART WORTH PINNING.
+# AND THE GUARD DID NOT CATCH IT, WHICH WAS THE PART WORTH PINNING.
-# `_grade_store_migration()` re-arms the latch when the flat key says tripped
+# `_grade_store_migration()` re-arms when the flat key says tripped and NOTHING
-# and NOTHING NAMESPACED IS LATCHED. The misdirected write IS namespaced and IS
+# NAMESPACED IS LATCHED - and the misdirected write IS namespaced and IS
-# latched, so `namespaced_latched` is True, the guard returns, and no inverter
+# latched. The guard was defeated by the very key the defect wrote.
-# on the roster comes up tripped. The guard is defeated by the very key the
-# defect wrote.
 #
-# WHY IT IS NOT FIXED HERE. The remedy is to name the migration's target from
+# ⚠️ THE REASON IT WAS LEFT - "there is no roster to name yet, because each
-# the config before any `Inverter` exists, which means factoring
+# `Inverter` restores its latch in its constructor" - WAS THE HALF-TRUTH THAT
-# `build_roster()`'s naming rule out of `build_roster()` so there is still only
+# KEPT IT ALIVE. The keys must be written before the `Inverter`s read them; the
-# ONE spelling of it - inverter.py and SAFETY-02's latch semantics, with its own
+# NAMES do not come from the `Inverter`s. `roster_names(opts)` reads them off
-# mutation battery over test_inverter.py and test_watchdog.py. MAINT-05 moves
+# the options before anything is constructed, which is what it already existed
-# the maintenance history and nothing about the sign latch.
+# to do for `MqttPublisher`. The factoring this section said the fix would need
-#
+# had been done a card earlier.
-# ⚠️ AND IT IS A DISARMED SAFETY GUARD, NOT LOST HISTORY. Read the checks below
-# as the SPECIFICATION OF A DEFECT: every one of them is asserting the WRONG
-# behaviour, on purpose, and the card that fixes it should expect them to go
-# red and should replace them.
 #
 # ⚠️ NO `schema_version` IN THIS STORE, AND THAT IS THE PRE-DEV-02 SHAPE
 # ITSELF: an install that has never run a version with namespaced keys, which is
@@ -1457,25 +1451,74 @@ print(f"    latch keys in the store: "
       f"{sorted(k for k in _lat_store.data if k.endswith('sign_tripped'))}")
 print(f"    roster came up tripped: "
       f"{[(i.name, i.sign_wd.tripped) for i in _lat.inverters]}")
-check("the flat pre-DEV-02 latch is migrated into `unit/inverter-a/` - a name "
+check("⭐ the flat pre-DEV-02 latch is migrated into the ROSTER'S OWN first "
-      "that is NOT on this roster, so no unit reads it. This is the same "
+      "namespace and NOT into `unit/inverter-a/` - the same wrong-namespace "
-      "wrong-namespace write `migrate_maint_keys()` now refuses, in the "
+      "write `migrate_maint_keys()` refuses, now refused in the migration that "
-      "migration that runs before there is a roster to grade against",
+      "runs before the `Inverter` objects exist",
-      _lat_store.data.get(unit_key(UNIT_A, "sign_tripped")) is True
+      _lat_store.data.get(unit_key(_C2_NAMES[0], "sign_tripped")) is True
-      and unit_key(_C2_NAMES[0], "sign_tripped") not in _lat_store.data)
+      and unit_key(UNIT_A, "sign_tripped") not in _lat_store.data)
-check("⚠️ AND NO INVERTER ON THE ROSTER COMES UP LATCHED, on a store whose file "
+check("⭐ AND AN INVERTER ON THE ROSTER COMES UP LATCHED, which is the whole "
-      "still reads `sign_tripped: true`. That is DEV-02's own sentence for the "
+      "card: a store whose file reads `sign_tripped: true` no longer produces "
-      "worst case - a disarmed safety guard on an install that believes it is "
+      "a site that drives",
-      "protected",
+      _lat.inverters[0].sign_wd.tripped
-      not any(i.sign_wd.tripped for i in _lat.inverters)
       and _lat_store.data["sign_tripped"] is True)
-check("...and `_grade_store_migration()` is DEFEATED BY THE KEY THE DEFECT "
+check("...and the latch that came back carries the REASON, not just the "
-      "WROTE, which is why this needs a card rather than a tweak: its "
+      "boolean - an operator meeting a FAILSAFE banner with no sentence under "
-      "'is anything namespaced latched' test is satisfied by the misdirected "
+      "it has been told the site stopped and not why",
-      "value itself, so it returns instead of re-arming",
+      "SIGN_DISAGREEMENT" in _lat.inverters[0].sign_wd.reason)
-      any(k.startswith("unit/") and k.endswith("/sign_tripped") and v
+check("⚠️ ...and the OTHER units on the roster are NOT latched by it. The flat "
-          for k, v in _lat_store.data.items())
+      "key is one site-level boolean and re-arming every unit off it would be "
-      and not any(i.sign_wd.tripped for i in _lat.inverters))
+      "DEV-02's defect in the opposite direction - a healthy pack parked "
+      "because its sibling once disagreed",
+      not any(i.sign_wd.tripped for i in _lat.inverters[1:]))
+
+# ⚠️ AND THE OTHER PATH, WHICH THE MIGRATION FIX ALONE DOES NOT REACH: a store
+# ALREADY STAMPED by the shipped version. `migrate_unit_keys()` returns at its
+# `from_version >= SCHEMA_VERSION` check and never moves anything, so the
+# stranded `unit/inverter-a/sign_tripped` just sits there. This is the STAGED
+# shape - "upgrade now, declare the roster later" - and it is the one an
+# operator actually produces. Only the GUARD repairs it.
+_staged_store = MemStore({
+    "sign_tripped": True,
+    "sign_reason": "SIGN_DISAGREEMENT: measured pre-DEV-02",
+    unit_key(UNIT_A, "sign_tripped"): True,
+    unit_key(UNIT_A, "sign_reason"): "SIGN_DISAGREEMENT: measured pre-DEV-02",
+    SCHEMA_KEY: SCHEMA_VERSION})
+_staged, _ = controller(NAMED_OPTS, store=_staged_store)
+print(f"    already-stamped store + declared roster -> ran="
+      f"{_staged.store_migration.ran}, came up tripped: "
+      f"{[(i.name, i.sign_wd.tripped) for i in _staged.inverters]}")
+check("⚠️ PRECONDITION: the migration really does NOT run on this store - the "
+      "marker is already there - so anything below is the guard's doing and "
+      "not the migration's",
+      not _staged.store_migration.ran)
+check("⭐ and the roster's first unit STILL comes up latched: the guard no "
+      "longer counts a latch stranded under `UNIT_A`, which is not on this "
+      "roster, as protection an operator has",
+      _staged.inverters[0].sign_wd.tripped)
+check("...and the re-armed latch is written where a machine reads it, so the "
+      "NEXT start finds it without needing the guard again",
+      _staged_store.data.get(unit_key(_C2_NAMES[0], "sign_tripped")) is True)
+
+# ⚠️ THE NEGATIVE CONTROL FOR THAT NARROWING, because "off the roster" on its
+# own was too blunt and test_inverter.py's DEV-02 restart case caught it: a
+# latch stranded under a name that is NOT `UNIT_A` is a real unit out of
+# today's config, and it MUST still count as protection.
+_other_store = MemStore({
+    "sign_tripped": True,
+    "sign_reason": "SIGN_DISAGREEMENT: B disagreed, mirrored flat",
+    unit_key("inverter-b", "sign_tripped"): True,
+    unit_key("inverter-b", "sign_reason"): "SIGN_DISAGREEMENT: B disagreed",
+    SCHEMA_KEY: SCHEMA_VERSION})
+_other, _ = controller(NAMED_OPTS, store=_other_store)
+print(f"    stranded latch under `inverter-b` -> came up tripped: "
+      f"{[(i.name, i.sign_wd.tripped) for i in _other.inverters]}")
+check("⚠️ a latch stranded under a name that is not `UNIT_A` is NOT re-armed "
+      "onto the roster: it belongs to a unit the operator has taken out of "
+      "the config, the flat key is its own mirror, and re-arming a healthy "
+      "sibling off it is the defect DEV-02 removed",
+      not any(i.sign_wd.tripped for i in _other.inverters))
+
 # The counterfactual, and it is the accepted baseline habit 7 asks for: the
 # same store, the same migration, one thing different - the roster shape.
 _flat_lat_store = MemStore({"sign_tripped": True,

goodwe_controller/test_rehearse.py

@@ -587,33 +587,32 @@ shutil.rmtree(Path(_tree2).parent, ignore_errors=True)
 
 # --------------------------------------------------------------------------- #
 print()
-print("5. SAFETY-44: a pre-DEV-02 flat store meeting a DECLARED ROSTER, which "
+print("5. SAFETY-44: a pre-DEV-02 flat store meeting a DECLARED ROSTER - "
-      "is the path nobody had established either way")
+      "FIXED, with each half of the fix as its own negative control")
 # --------------------------------------------------------------------------- #
-# The question on the card: `migrate_unit_keys(store, UNIT_A)` takes a hardcoded
+# The question on the card: `migrate_unit_keys(store, UNIT_A)` took a hardcoded
-# constant and has no `misdirected` guard, while `migrate_maint_keys(store,
+# constant and had no `misdirected` guard, while `migrate_maint_keys(store,
-# unit, roster)` takes the roster and does. Is that asymmetry safe?
+# unit, roster)` takes the roster and does. That asymmetry was NOT safe, and
+# this part used to assert the defect. It now asserts the FIX.
 #
-# ⚠️ MEASURED, NOT ARGUED. The two checks below are the answer, and they are
+# ⚠️ THE FIX IS TWO EDITS AND THEY DO DIFFERENT JOBS, WHICH IS THE WHOLE REASON
-# written as REQUIREMENTS so that a fix to `migrate_unit_keys` turns them red
+# THIS PART IS SHAPED THE WAY IT IS. Measured, per leg, by reverting each edit
-# and this file has to be updated deliberately rather than drifting.
+# on its own (5b and 5c below):
+#
+#                              SAME-STEP leg            STAGED leg
+#   both edits (shipped)       clean                    latch armed, litter
+#   guard reverted             latch armed              ⚠️ DISARMED
+#   call site reverted         latch armed, litter      latch armed, litter
+#
+# So the GUARD carries the safety and the CALL SITE carries the hygiene. Saying
+# that plainly matters more than it looks: a reader who assumes the roster-aware
+# migration is what re-arms the latch would delete the guard as redundant during
+# a tidy-up and take the STAGED column with it - and STAGED is the path
+# "upgrade now, declare the roster later" produces, which is the path an
+# operator actually takes.
 code44, out44 = run(HERE, "state-legacy.json", "roster.yaml")
-print(f"      flat store + roster ['garage','shed'] -> exit {code44}")
-check("⚠️ SAFETY-44 IS A DEFECT AGAIN: a store that ran pre-DEV-02 flat and "
-      "declares a multi-inverter roster comes up with its SAFETY-02 latch "
-      "DISARMED - the file says `sign_tripped: true` and not one inverter is "
-      "latched",
-      code44 == RR.EXIT_FINDING and "LATCH CAME UP DISARMED" in out44)
-check("...and the harness names the two latch keys SAFETY-44 asks about, by "
-      "the namespace they were stranded in",
-      "unit/inverter-a/sign_tripped" in out44
-      and "unit/inverter-a/sign_reason" in out44)
-check("...and the mechanism is visible in the report: the keys are stranded "
-      "under `inverter-a`, which is not on the roster, so no machine reads them",
-      "STRANDED STORE KEY" in out44 and "['garage', 'shed']" in out44)
-# ⚠️ PER LEG, NOT A COUNT OVER THE WHOLE REPORT. See `legs()` for the three
-# builds this distinction was measured on and for what the count missed.
 _L = legs(out44)
+print(f"      flat store + roster ['garage','shed'] -> exit {code44}")
 print(f"      legs in the report: {sorted(_L)}")
 for _leg in ("STAGED", "SAME-STEP"):
     print(f"      LEG {_leg}: "
@@ -622,74 +621,125 @@ check("the report is split into legs this check can grade SEPARATELY - the "
       "tool prints a `LEG <id>` marker, because counting a finding's text over "
       "the whole report cannot tell one red leg from two",
       set(_L) == {"STAGED", "SAME-STEP"})
-check("⚠️ SAME-STEP is disarmed: the roster is declared in the SAME step as "
+check("⭐ SAFETY-44 IS FIXED ON BOTH PATHS: a store that ran pre-DEV-02 flat "
-      "the upgrade, the migration runs and misdirects the latch to "
+      "and declares a multi-inverter roster comes up LATCHED - neither leg "
-      "`inverter-a`",
+      "reports a disarmed guard",
-      "LATCH CAME UP DISARMED" in _L.get("SAME-STEP", ""))
+      "LATCH CAME UP DISARMED" not in out44)
-check("⚠️ AND IT IS NOT THE SAME-STEP PATH ALONE - the STAGED leg reaches the "
+check("...and the latch is under a name THE ROSTER READS, which is the whole "
-      "same disarmed latch by a DIFFERENT route: the flat start stamps "
+      "of the claim - `garage` is on the roster and `inverter-a` is not",
-      "`schema_version`, so the roster start's migration takes its early "
+      all("after : which units came up latched  : ['garage']" in v
-      "return and the latch never moves at all",
+          for v in _L.values()))
-      "LATCH CAME UP DISARMED" in _L.get("STAGED", ""))
+check("⚠️ the SAME-STEP leg is now CLEAN OUTRIGHT: the migration writes the "
-
+      "latch straight into the roster's first namespace, so there is no "
-# ⚠️ AND THE CONTRAST, WHICH IS WHAT STOPS THE CHECK ABOVE BEING VACUOUS. The
+      "stranded copy left behind and nothing for an operator to read past",
-# SAME state file onto a roster whose first entry IS `inverter-a` comes up
+      "none in LEG SAME-STEP" in _L.get("SAME-STEP", ""))
-# LATCHED. The two runs differ in one thing - the first roster name - so the
+check("⚠️ ...while the STAGED leg still reports the two STRANDED keys, and "
-# finding is attributable to it and to nothing else.
+      "that is REPORTED not hidden: its flat start stamped `schema_version` "
-code_ok, out_ok = run(HERE, "state-legacy.json", "roster-inverter-a.yaml")
+      "before the roster existed, so the migration takes its early return and "
+      "the old copies stay in the file with nothing reading them. The latch is "
+      "armed anyway - by the guard - and the litter is true",
+      "STRANDED STORE KEY" in _L.get("STAGED", "")
+      and "unit/inverter-a/sign_tripped" in _L.get("STAGED", ""))
+check("...and the run as a whole is a FINDING rather than silence, because "
+      "keys nothing reads are still keys nothing reads",
+      code44 == RR.EXIT_FINDING)
 check("...and no leg's text has swallowed the CLOSING SUMMARY, which repeats "
       "every finding from every leg - the failure that made the last leg read "
       "red whenever any leg was",
       not any("FINDING(S) - this transition is not clean" in v
               for v in _L.values()))
+check("⚠️ and the key that MOVED is not reported as lost: the before-snapshot "
+      "holds `unit/inverter-a/sign_tripped = True` and the after-snapshot "
+      "holds it under `garage`, three lines apart in the same report. A "
+      "VANISHED finding there is the harmless-case alarm CONTROL-C exists to "
+      "prevent",
+      "migrated  unit/inverter-a/sign_tripped = True -> "
+      "unit/garage/sign_tripped" in out44
+      and "VANISHED STORE KEY" not in out44)
 
 # --------------------------------------------------------------------------- #
 print()
-print("5b. THE SPLITTER'S POSITIVE CONTROL: a build in which exactly ONE leg "
+print("5b. NEGATIVE CONTROL, AND THE SPLITTER'S POSITIVE ONE: revert THE "
-      "is clean")
+      "GUARD, and exactly one leg goes red")
 # --------------------------------------------------------------------------- #
 # ⚠️ A PER-LEG CHECK THAT HAS NEVER SEEN THE LEGS DISAGREE IS A COUNT WITH
-# EXTRA STEPS. Every fixture above has both legs red, so nothing so far
+# EXTRA STEPS, and MUTATION.md's extension says one that has seen them disagree
-# separates "reads each leg" from "reads the whole report twice". This part
+# in a single arrangement is a total over that arrangement. This build is the
-# builds the one case that does: `migrate_unit_keys()` handed the roster's own
+# one that makes them disagree, and it is a negative control for part 5 at the
-# first name instead of the hardcoded `UNIT_A`.
+# same time: put back the clause that counted ANY `unit/*/sign_tripped` as
-#
+# protection, including one under a name no unit answers to.
-# ⚠️ AND WHAT IT MEASURES IS ALSO THE ANSWER TO "IS THAT THE FIX". It is NOT.
+_no_guard = mutant_tree([(
-# It repairs the SAME-STEP path only. The STAGED path is untouched, because
-# there the flat start stamps `schema_version = 1` and the roster start's
-# migration takes its `from_version >= SCHEMA_VERSION` early return before the
-# name it would be handed matters at all. A fix that stops at this line would
-# close the half an operator is least likely to take and leave the half that
-# "upgrade first, declare the roster later" produces.
-_one_leg = mutant_tree([(
     "app/main.py",
-    "        self.store_migration = migrate_unit_keys(store, UNIT_A)",
+    "            if unit in on_roster or unit != UNIT_A:",
-    "        self.store_migration = migrate_unit_keys(store, "
+    "            if True:  # MUTANT: any namespaced latch is protection")])
-    "roster_names(opts)[0])")])
+_ng_code, _ng_out = run(Path(_no_guard), "state-legacy.json", "roster.yaml")
-_ol_code, _ol_out = run(Path(_one_leg), "state-legacy.json", "roster.yaml")
+_NG = legs(_ng_out)
-_OL = legs(_ol_out)
 for _leg in ("STAGED", "SAME-STEP"):
     print(f"      LEG {_leg}: "
-          f"{'DISARMED' if 'LATCH CAME UP DISARMED' in _OL.get(_leg, '') else 'clean'}")
+          f"{'DISARMED' if 'LATCH CAME UP DISARMED' in _NG.get(_leg, '') else 'clean'}")
-check("⭐ with `migrate_unit_keys()` handed the roster's own first name, the "
+check("⭐ with the off-roster clause reverted the STAGED leg comes up "
-      "SAME-STEP leg comes up CLEAN - so the splitter really does read the "
+      "DISARMED - so the guard's roster test is load-bearing, and it is the "
-      "legs separately, and a check that only counted would have missed this",
+      "half of the fix that repairs an install already stamped by the shipped "
-      "LATCH CAME UP DISARMED" not in _OL["SAME-STEP"])
+      "version",
-check("⚠️ ...while the STAGED leg is STILL DISARMED on the same build: that "
+      "LATCH CAME UP DISARMED" in _NG.get("STAGED", ""))
-      "line is not the whole fix, because the staged path never reaches it - "
+check("⚠️ ...while the SAME-STEP leg stays ARMED on the very same build - the "
-      "the first start stamps `schema_version` and the second start's "
+      "legs DISAGREE, so the splitter really does read them separately and a "
-      "migration returns early",
+      "check that only counted findings would have missed this",
-      "LATCH CAME UP DISARMED" in _OL["STAGED"])
+      "LATCH CAME UP DISARMED" not in _NG.get("SAME-STEP", ""))
-check("...and the run as a whole is still a finding, so nobody reads this "
+check("...and the run is a finding, so the partial build is never read as "
-      "partial fix as done",
+      "clean",
-      _ol_code == RR.EXIT_FINDING)
+      _ng_code == RR.EXIT_FINDING)
-shutil.rmtree(Path(_one_leg).parent, ignore_errors=True)
+shutil.rmtree(Path(_no_guard).parent, ignore_errors=True)
 
+# --------------------------------------------------------------------------- #
 print()
-print("5c. back to the shipped code")
+print("5c. NEGATIVE CONTROL: revert THE CALL SITE, and the litter comes back "
+      "without the latch going out")
+# --------------------------------------------------------------------------- #
+# ⚠️ THIS IS THE CONTROL THAT KEEPS 5b HONEST ABOUT WHAT IT PROVED. 5b shows
+# the guard carries the latch; on its own that is an argument for deleting the
+# roster-aware migration as decoration. This build is the measurement instead:
+# hand `migrate_unit_keys()` the hardcoded `UNIT_A` again, with the guard left
+# fixed. The latch stays armed - the guard catches it - and the SAME-STEP leg
+# gains the two stranded keys it does not have on the shipped code. The
+# migration fix buys a store with nothing dead in it, and buys the
+# `misdirected` refusal that stops a WRONG marker being stamped; it does not
+# buy the latch. Both edits stay.
+_CALL_NOW = ("        self.store_migration = migrate_unit_keys(" + chr(10) +
+             "            store, _roster_names[0] if _roster_names else "
+             "UNIT_A, _roster_names)")
+_old_call = mutant_tree([(
+    "app/main.py", _CALL_NOW,
+    "        self.store_migration = migrate_unit_keys(store, UNIT_A)")])
+_oc_code, _oc_out = run(Path(_old_call), "state-legacy.json", "roster.yaml")
+_OC = legs(_oc_out)
+for _leg in ("STAGED", "SAME-STEP"):
+    print(f"      LEG {_leg}: "
+          f"{'DISARMED' if 'LATCH CAME UP DISARMED' in _OC.get(_leg, '') else 'clean'}")
+check("⚠️ with `UNIT_A` handed back to the migration the latch is STILL "
+      "armed on both legs - the guard is what re-arms it, and saying so is "
+      "the difference between this fix and a story about it",
+      "LATCH CAME UP DISARMED" not in _oc_out)
+check("⭐ ...but the SAME-STEP leg is no longer clean: the two SAFETY-02 keys "
+      "are written under `inverter-a` on a roster of ['garage', 'shed'] and "
+      "nothing reads them - which is the state the shipped code does not "
+      "produce",
+      "STRANDED STORE KEY" in _OC.get("SAME-STEP", "")
+      and "none in LEG SAME-STEP" not in _OC.get("SAME-STEP", ""))
+shutil.rmtree(Path(_old_call).parent, ignore_errors=True)
+
+# --------------------------------------------------------------------------- #
+print()
+print("5d. THE CONTRAST: the same file onto a roster that keeps the name")
+# --------------------------------------------------------------------------- #
+# ⚠️ AND THE CONTRAST, WHICH IS WHAT STOPS EVERY CHECK ABOVE BEING VACUOUS. The
+# SAME state file onto a roster whose first entry IS `inverter-a` comes up
+# LATCHED AND CLEAN. The two runs differ in one thing - the first roster name -
+# so a finding above is attributable to it and to nothing else.
+code_ok, out_ok = run(HERE, "state-legacy.json", "roster-inverter-a.yaml")
 check("the SAME state file onto a roster whose first entry IS `inverter-a` "
-      "comes up latched and clean - so the finding above is caused by the "
+      "comes up latched and clean - so anything part 5 reports is caused by "
-      "roster NAME and not by the fixture being tripped",
+      "the roster NAME and not by the fixture being tripped",
       code_ok == RR.EXIT_OK and "LATCH CAME UP DISARMED" not in out_ok)
 
 

goodwe_controller/tools/rehearse_roster.py

@@ -460,6 +460,7 @@ class Snapshot:
         # finding: see `diff()`.
         self.stranded_at_default = []
         self.vanished_at_default = []
+        self.vanished_but_migrated = []
 
     @property
     def unit_keys(self):
@@ -550,6 +551,29 @@ def diff(before: Snapshot, after: Snapshot) -> list:
         if value in (None, DEFAULTS.get(bare)):
             after.vanished_at_default.append(f"{key} = {value!r}")
             continue
+        # ⚠️ AND A KEY THAT MOVED TO A NAMESPACE A MACHINE READS HAS NOT
+        # VANISHED, WHICH IS THE SAME CORRECTION ONE PARAGRAPH UP, ONE CARD
+        # LATER. SAFETY-44's fix made `migrate_unit_keys()` write under the
+        # ROSTER's first name instead of the hardcoded `inverter-a`, so on a
+        # renamed roster the before-snapshot holds `unit/inverter-a/
+        # sign_tripped = True` and the after-snapshot holds
+        # `unit/garage/sign_tripped = True`. Measured on that build: two
+        # VANISHED findings for a LATCH THAT IS SITTING ON THE SAME PAGE OF THE
+        # REPORT, three lines above them, under the name the roster reads.
+        #
+        # ⚠️ THE TEST IS NOT "the name appears somewhere". It is: the SAME bare
+        # key, at the SAME value, under a key in `machine_read` - the set built
+        # by driving a real start, not by pattern-matching a namespace. A latch
+        # that genuinely went missing has no such key and is still a finding,
+        # which is what M3 (delete the flat rollback view) measures and what
+        # keeps this from being a softening of the check.
+        moved_to = sorted(
+            k for k in after.machine_read
+            if k.rsplit("/", 1)[-1] == bare and after.data.get(k) == value)
+        if key.startswith("unit/") and moved_to:
+            after.vanished_but_migrated.append(
+                f"{key} = {value!r} -> {moved_to[0]}")
+            continue
         findings.append(Finding(
             "VANISHED STORE KEY",
             f"{key} = {value!r} was in the store before the "
@@ -846,6 +870,15 @@ def report_transition(leg_id, name, before, after, findings) -> None:
         mark = "  " if key in after.data else "  (absent, defaulted) "
         print(f"      {key}{mark}")
 
+    if after.vanished_but_migrated:
+        print()
+        print("    keys that MOVED to the namespace this roster reads - not "
+              "lost, and shown")
+        print("    because a rename that relocates a LATCH is the operator's "
+              "business:")
+        for text in after.vanished_but_migrated:
+            print(f"      migrated  {text}")
+
     if after.stranded_at_default or after.vanished_at_default:
         print()
         print("    keys whose value IS the default `Store.load()` supplies, so "