admin/goodwe-addon
1
0
Fork 0
Code Issues Pull Requests 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/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)