admin/goodwe-addon
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

SAFETY-44: the latch migration takes the roster, and the guard stops being defeated by the key the defect wrote
checks / unit (pull_request) Successful in 6m56s
Details

Browse Source 
⚠️ 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
This commit is contained in:
glenn schrooyen
2026-09-08 15:36:37 +02:00
co-authored by Claude Opus 5
parent 46c022c926
commit 5e382f52f6
5 changed files with 372 additions and 146 deletions
Download Patch File Download Diff File Expand all files Collapse all files
goodwe_controller/app/main.py
+83 -15
View File
@@ -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 STORE RATHER THAN OF THE ROSTER. At a roster of two a trip on B
# leaves A untripped, so re-arming A off a flat key that B's own trip
# mirrored would latch a healthy inverter on every restart - the
# flat-key defect this card removed, coming back through its own guard.
# And the STORE is the right place to ask: it holds every
# `unit/*/sign_tripped` this install has ever written, including one
# 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
# configured right now. The flat key carries new information only when
# nothing namespaced is latched at all.
namespaced_latched = any(
bool(v) for k, v in data.items()
if k.startswith("unit/") and k.endswith("/sign_tripped"))
# ⚠️ THE QUESTION IS "IS ANY NAMESPACED LATCH SET **THAT A MACHINE
# READS**", AND THE LAST CLAUSE IS SAFETY-44's SECOND HALF. At a roster
# of two a trip on B leaves A untripped, so re-arming A off a flat key
# that B's own trip mirrored would latch a healthy inverter on every
# restart - the flat-key defect DEV-02 removed, coming back through its
# own guard. That much stands, and it is why the question is asked of
# the STORE's keys rather than of the roster's objects.
#
# ⚠️ BUT IT USED TO ACCEPT **ANY** `unit/*/sign_tripped`, and the
# paragraph here defended that: the store "holds every one this install
# has ever written, including one belonging to a unit that is not in
# TODAY's roster ... whereas the roster only knows what is configured
# right now". THE PREMISE IS TRUE AND IT ARGUES THE WRONG WAY. A latch
# 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
+40 -8
View File
@@ -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
`migrate_unit_keys()`, which runs BEFORE `build_roster()` for the reason its
own docstring gives - the `Inverter` objects restore their latch in their
constructors, so the keys have to be in place before there is a roster to
check against. That gap is real and is recorded as a running check rather
than left to a comment: see test_maintenance.py section 13.
`roster=None` still means "not graded", and SAFETY-44 is what it cost. This
paragraph used to end here, saying that the one caller passing it was
`migrate_unit_keys()`, "which runs BEFORE `build_roster()` ... so the keys
have to be in place before there is a roster to check against", and calling
that gap real. THE FIRST HALF IS TRUE AND THE CONCLUSION IS NOT. The keys do
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
+91 -48
View File
@@ -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 "
"wrong-namespace hole, and it is a LATCH")
print("13. FIXED (SAFETY-44): SAFETY-02's migration takes the roster too, and "
"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
# CHECK RATHER THAN A COMMENT SO IT CANNOT BE FORGOTTEN OR QUIETLY "FIXED" BY AN
# UNRELATED EDIT. It is the SAME defect one module over, and it costs more.
# ⚠️ THIS SECTION USED TO BE THE SPECIFICATION OF A DEFECT. It was written while
# fixing MAINT-05's own instance, as a RUNNING check rather than a comment, with
# 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)` -
# a hardcoded constant, where the maintenance migration now passes the roster's
# own first name. It has to run BEFORE `build_roster()`, because each
# `Inverter` restores its sign latch in its constructor, so there is no roster
# to name yet and `_verify_moves()` is passed `roster=None`. On a DECLARED
# roster the pre-DEV-02 flat `sign_tripped: true` therefore lands under
# `unit/inverter-a/sign_tripped`, which no unit on this site reads.
# WHAT IT WAS. `Controller.__init__` called `migrate_unit_keys(store, UNIT_A)` -
# a hardcoded constant, where the maintenance migration already passed the
# roster's own first name. On a DECLARED roster the pre-DEV-02 flat
# `sign_tripped: true` landed under `unit/inverter-a/sign_tripped`, which no
# unit on the site reads, and `_verify_moves()` read it back under the name it
# had been handed and was satisfied.
#
# WHY THE EXISTING GUARD DOES NOT CATCH IT, WHICH IS THE PART WORTH PINNING.
# `_grade_store_migration()` re-arms the latch when the flat key says tripped
# and NOTHING NAMESPACED IS LATCHED. The misdirected write IS namespaced and IS
# latched, so `namespaced_latched` is True, the guard returns, and no inverter
# on the roster comes up tripped. The guard is defeated by the very key the
# defect wrote.
# AND THE GUARD DID NOT CATCH IT, WHICH WAS THE PART WORTH PINNING.
# `_grade_store_migration()` re-arms when the flat key says tripped and NOTHING
# NAMESPACED IS LATCHED - and the misdirected write IS namespaced and IS
# latched. The guard was 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 config before any `Inverter` exists, which means factoring
# `build_roster()`'s naming rule out of `build_roster()` so there is still only
# ONE spelling of it - inverter.py and SAFETY-02's latch semantics, with its own
# mutation battery over test_inverter.py and test_watchdog.py. MAINT-05 moves
# the maintenance history and nothing about the sign latch.
#
# ⚠️ 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.
# ⚠️ THE REASON IT WAS LEFT - "there is no roster to name yet, because each
# `Inverter` restores its latch in its constructor" - WAS THE HALF-TRUTH THAT
# KEPT IT ALIVE. 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 what it already existed
# to do for `MqttPublisher`. The factoring this section said the fix would need
# had been done a card earlier.
#
# ⚠️ 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 "
"that is NOT on this roster, so no unit reads it. This is the same "
"wrong-namespace write `migrate_maint_keys()` now refuses, in the "
"migration that runs before there is a roster to grade against",
_lat_store.data.get(unit_key(UNIT_A, "sign_tripped")) is True
and unit_key(_C2_NAMES[0], "sign_tripped") not in _lat_store.data)
check("⚠️ AND NO INVERTER ON THE ROSTER COMES UP LATCHED, on a store whose file "
"still reads `sign_tripped: true`. That is DEV-02's own sentence for the "
"worst case - a disarmed safety guard on an install that believes it is "
"protected",
not any(i.sign_wd.tripped for i in _lat.inverters)
check("⭐ the flat pre-DEV-02 latch is migrated into the ROSTER'S OWN first "
"namespace and NOT into `unit/inverter-a/` - the same wrong-namespace "
"write `migrate_maint_keys()` refuses, now refused in the migration that "
"runs before the `Inverter` objects exist",
_lat_store.data.get(unit_key(_C2_NAMES[0], "sign_tripped")) is True
and unit_key(UNIT_A, "sign_tripped") not in _lat_store.data)
check("⭐ AND AN INVERTER ON THE ROSTER COMES UP LATCHED, which is the whole "
"card: a store whose file reads `sign_tripped: true` no longer produces "
"a site that drives",
_lat.inverters[0].sign_wd.tripped
and _lat_store.data["sign_tripped"] is True)
check("...and `_grade_store_migration()` is DEFEATED BY THE KEY THE DEFECT "
"WROTE, which is why this needs a card rather than a tweak: its "
"'is anything namespaced latched' test is satisfied by the misdirected "
"value itself, so it returns instead of re-arming",
any(k.startswith("unit/") and k.endswith("/sign_tripped") and v
for k, v in _lat_store.data.items())
and not any(i.sign_wd.tripped for i in _lat.inverters))
check("...and the latch that came back carries the REASON, not just the "
"boolean - an operator meeting a FAILSAFE banner with no sentence under "
"it has been told the site stopped and not why",
"SIGN_DISAGREEMENT" in _lat.inverters[0].sign_wd.reason)
check("⚠️ ...and the OTHER units on the roster are NOT latched by it. The flat "
"key is one site-level boolean and re-arming every unit off it would be "
"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
+125 -75
View File
@@ -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 "
"is the path nobody had established either way")
print("5. SAFETY-44: a pre-DEV-02 flat store meeting a DECLARED ROSTER - "
"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
# constant and has no `misdirected` guard, while `migrate_maint_keys(store,
# unit, roster)` takes the roster and does. Is that asymmetry safe?
# The question on the card: `migrate_unit_keys(store, UNIT_A)` took a hardcoded
# constant and had no `misdirected` guard, while `migrate_maint_keys(store,
# 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
# written as REQUIREMENTS so that a fix to `migrate_unit_keys` turns them red
# and this file has to be updated deliberately rather than drifting.
# ⚠️ THE FIX IS TWO EDITS AND THEY DO DIFFERENT JOBS, WHICH IS THE WHOLE REASON
# THIS PART IS SHAPED THE WAY IT IS. Measured, per leg, by reverting each edit
# 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 "
"the upgrade, the migration runs and misdirects the latch to "
"`inverter-a`",
"LATCH CAME UP DISARMED" in _L.get("SAME-STEP", ""))
check("⚠️ AND IT IS NOT THE SAME-STEP PATH ALONE - the STAGED leg reaches the "
"same disarmed latch by a DIFFERENT route: the flat start stamps "
"`schema_version`, so the roster start's migration takes its early "
"return and the latch never moves at all",
"LATCH CAME UP DISARMED" in _L.get("STAGED", ""))
# ⚠️ AND THE CONTRAST, WHICH IS WHAT STOPS THE CHECK ABOVE BEING VACUOUS. The
# SAME state file onto a roster whose first entry IS `inverter-a` comes up
# LATCHED. The two runs differ in one thing - the first roster name - so the
# finding is attributable to it and to nothing else.
code_ok, out_ok = run(HERE, "state-legacy.json", "roster-inverter-a.yaml")
check("⭐ SAFETY-44 IS FIXED ON BOTH PATHS: a store that ran pre-DEV-02 flat "
"and declares a multi-inverter roster comes up LATCHED - neither leg "
"reports a disarmed guard",
"LATCH CAME UP DISARMED" not in out44)
check("...and the latch is under a name THE ROSTER READS, which is the whole "
"of the claim - `garage` is on the roster and `inverter-a` is not",
all("after : which units came up latched : ['garage']" in v
for v in _L.values()))
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 "
"stranded copy left behind and nothing for an operator to read past",
"none in LEG SAME-STEP" in _L.get("SAME-STEP", ""))
check("⚠️ ...while the STAGED leg still reports the two STRANDED keys, and "
"that is REPORTED not hidden: its flat start stamped `schema_version` "
"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 "
"is clean")
print("5b. NEGATIVE CONTROL, AND THE SPLITTER'S POSITIVE ONE: revert THE "
"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
# separates "reads each leg" from "reads the whole report twice". This part
# builds the one case that does: `migrate_unit_keys()` handed the roster's own
# first name instead of the hardcoded `UNIT_A`.
#
# ⚠️ AND WHAT IT MEASURES IS ALSO THE ANSWER TO "IS THAT THE FIX". It is NOT.
# 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([(
# EXTRA STEPS, and MUTATION.md's extension says one that has seen them disagree
# in a single arrangement is a total over that arrangement. This build is the
# one that makes them disagree, and it is a negative control for part 5 at the
# same time: put back the clause that counted ANY `unit/*/sign_tripped` as
# protection, including one under a name no unit answers to.
_no_guard = mutant_tree([(
"app/main.py",
" self.store_migration = migrate_unit_keys(store, UNIT_A)",
" self.store_migration = migrate_unit_keys(store, "
"roster_names(opts)[0])")])
_ol_code, _ol_out = run(Path(_one_leg), "state-legacy.json", "roster.yaml")
_OL = legs(_ol_out)
" if unit in on_roster or unit != UNIT_A:",
" if True: # MUTANT: any namespaced latch is protection")])
_ng_code, _ng_out = run(Path(_no_guard), "state-legacy.json", "roster.yaml")
_NG = legs(_ng_out)
for _leg in ("STAGED", "SAME-STEP"):
print(f" LEG {_leg}: "
f"{'DISARMED' if 'LATCH CAME UP DISARMED' in _OL.get(_leg, '') else 'clean'}")
check("⭐ 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",
"LATCH CAME UP DISARMED" not in _OL["SAME-STEP"])
check("⚠️ ...while the STAGED leg is STILL DISARMED on the same build: that "
"line is not the whole fix, because the staged path never reaches it - "
"the first start stamps `schema_version` and the second start's "
"migration returns early",
"LATCH CAME UP DISARMED" in _OL["STAGED"])
check("...and the run as a whole is still a finding, so nobody reads this "
"partial fix as done",
_ol_code == RR.EXIT_FINDING)
shutil.rmtree(Path(_one_leg).parent, ignore_errors=True)
f"{'DISARMED' if 'LATCH CAME UP DISARMED' in _NG.get(_leg, '') else 'clean'}")
check("⭐ with the off-roster clause reverted the STAGED leg comes up "
"DISARMED - so the guard's roster test is load-bearing, and it is the "
"half of the fix that repairs an install already stamped by the shipped "
"version",
"LATCH CAME UP DISARMED" in _NG.get("STAGED", ""))
check("⚠️ ...while the SAME-STEP leg stays ARMED on the very same build - the "
"legs DISAGREE, so the splitter really does read them separately and a "
"check that only counted findings would have missed this",
"LATCH CAME UP DISARMED" not in _NG.get("SAME-STEP", ""))
check("...and the run is a finding, so the partial build is never read as "
"clean",
_ng_code == RR.EXIT_FINDING)
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 "
"roster NAME and not by the fixture being tripped",
"comes up latched and clean - so anything part 5 reports is caused by "
"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
+33
View File
@@ -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 "