PKG-17: ship 0.11.0 to master - SAFETY-56's wording fix and TEL-11's pv_entity reach the battery #115

Merged
admin merged 13 commits from release/1.0 into master 2026-09-24 20:28:36 +02:00
Owner

Cuts 0.11.0 onto master, the branch the Home Assistant Supervisor clones. Merging this is a deployment to a live GW5000S-BP currently running 0.10.1.

Carries SAFETY-56, TEL-11, the CI-9 follow-up and SEC-2's docs/RELEASE.md. This is the first release to follow docs/RELEASE.md §1.


⚠️ Before anyone presses Update: predeploy_check.py has NOT been run against current options

tools/predeploy_check.py returned 0 / CLEARED — but against a stale options export dated 2026-09-07 00:30 (/home/homelab/live-options.json). That export predates both the 0.10.0 and the 0.10.1 installs. It is not a statement about the blob on the box today.

That is a weak positive, not a clearance. Before pressing Update, the owner must export the current options blob through the Supervisor API and re-run:

python3 goodwe_controller/tools/predeploy_check.py \
  --deployed-version 0.10.1 --options <fresh-export.json>

Exit 0 = cleared, 1 = REFUSED, 2 = NOT CLEARED (a check did not run — not the same as passing). Run it where aiohttp exists, or the validator half skips and exits 2.

This is not a footnote. Nothing in this PR re-runs it, and CI does not run it either.


⚠️ This is a real merge, not a fast-forward

The last two ships were fast-forwards. This one is not. master is no longer an ancestor of release/1.0:

merge base            b0b24e5   (0.10.1)
release/1.0..master   0f14c2a   "PKG-16: ship 0.10.1 to master"

0f14c2a is a --no-ff merge commit pushed directly to master on 2026-09-22, before SEC-2 protected the branch. It exists only on master and has no (#NN) suffix — docs/RELEASE.md §1 names it as the commit that motivated the protection. Its tree is identical to b0b24e5, so it carries no content delta; it is purely a history artefact.

Consequence: merging this produces a genuine merge commit. Verified independently with git merge-tree --write-tree origin/master origin/release/1.0 over the merge base — exit 0, one tree, zero conflict markers. Result: 22 files, +3334 / -29.

⚠️ docs/RELEASE.md §1 closes with "release/1.0 is normally an ancestor of master" and assumes the fast-forward case. It does not anticipate this shape — which is odd, because the very commit that creates it is documented three paragraphs earlier in the same file. See "Notes on the procedure" below.


What an operator can observe

Two tickets are visible on an installed add-on. Neither changes a watt. app/control.py is not in this diff, and neither are allocator.py, arbiter.py, arbitrage.py, clamp.py, peakguard.py, maintenance.py or watchdog.py.

SAFETY-56 — three wordings stop naming healthy hardware as the fault

Text only. Behaviour measured byte-identical (tools/safety56_behaviour_trace.py: 2390 one-second reads replayed from the committed 2026-09-21 bus capture, a 66-cell command×register grid stepped 120 times each, six driven controller worlds — document hash 6030175ed2ab9a41… identical across both trees while the sentences differ; the tool refuses to report "unchanged" if the sentence halves agree).

SAFETY-37's freshness gate fires on a conjunction satisfied by two disjoint worlds: a register that stopped reporting reality, or a pack that cannot comply (a BMS refusing discharge at its floor holds still under any setpoint). Nothing the gate is handed separates them, and three sentences asserted the first. On the one measured episode it was the second — pack at 11 %, every command in the 850 s hold a discharge, register answered the first CHARGE three seconds later. The hardware was healthy and the add-on said it had failed.

Three wordings, four surfaces:

surface 0.10.1 0.11.0
action log and status page (both quote freshness.py's STALE_READING: verbatim) "the reading is not following, so it is reported as MISSING" "TWO THINGS LOOK LIKE THIS AND THIS GATE CANNOT TELL THEM APART …"
add-on log, ERROR, from main.py "battery-power register is not following its own command" "battery-power reading did not move under a command that did, WHICH IS NOT BY ITSELF A VERDICT ON THE REGISTER"
readiness row, diagnostics page "battery power: <entity> is missing or not numeric" "no usable value this cycle from <entity> — and this row cannot tell which of two things that is … Confirm the entity in Developer Tools before replacing it"

The third row is the one that cost an operator an evening: it asserted a false fact about a named entity, because Freshness.reading() returns None by design when the gate withholds a present, numeric, updating value. Fixed by saying less. It still does not tell you which of the two it was — that would need SoC and the pack floor, which are deliberately not freshness.py's inputs.

TEL-11 — pv_entity, optional, and not a control input

One new key. It is not a control input and it does not change what the loop does. It records what the site's PV is producing beside what the meter and the battery are doing — an observability channel only.

Unset behaves exactly as 0.10.1 does today, and that is measured, not asserted (tools/tel11_pv_trace.py: 5 grid behaviours × 8 PV-sensor behaviours × 24 cycles of the shipped Controller). Empty means the add-on reads no entity, announces no entity, publishes no topic, shows no status row and adds no page row.

It is an HA entity, not a register — the RS485 bus carries the battery inverter; a household PV inverter is normally a different box from a different vendor. Positive is production; there is deliberately no pv_invert (the quantity is unipolar and read by nothing, so a reversed sensor announces itself as −4 kW at noon). The entity's own unit_of_measurement is believed; a unitless or non-power sensor is refused with the reason on the status page.

Will a stored options blob be refused?

No new refusal is introduced by this hop. Option surface 75 → 76 keys, options: and schema: in step. No key removed, no default moved, no enum widened, and no rule added to validate.py — the validator does not mention pv_entity at all. The key is str? (optional) defaulting to "", so a blob written on 0.10.0/0.10.1 simply does not carry it and the Supervisor's merge supplies the empty string. There is no SystemExit(78) to plan around on 0.10.1 → 0.11.0.

⚠️ That is the delta. It is not a clearance for the box — the pre-existing rules from 0.8.0/0.9.0/0.10.0 (SAFETY-39 rule 10's slew_w ceiling, TARIFF-07's three withdrawn price_provider ids) still apply on 0.11.0 and still refuse a config nobody edited. The reference install is on price_provider: off and slew_w: 400, so neither is expected to bite — but that is from the stale export, which is exactly why the re-run above is required.

Slug

slug: goodwe_controller — unchanged. This is an update in place, not a second add-on. version: bumps "0.10.1" → "0.11.0", which is what makes the Supervisor rebuild and offer it at all.

Not in the operator changelog, on purpose

CI-9's follow-up (9f6b9f1) and SEC-2 (239e120) are in this range but deliberately excluded from CHANGELOG.md's table, under PKG-15's rule — audience, not importance. Both sit outside goodwe_controller/, neither is in the built image, and no install can observe either.

Notes on the procedure (docs/RELEASE.md, first real exercise)

Reported rather than fixed here — they are documentation defects, not blockers:

  1. §1's closing note assumes a fast-forward. "release/1.0 is normally an ancestor of master, so a release/1.0 → master pull request is empty until new commits land" — true only when the branches are equal, and it gives no guidance for the case this PR is in, where master carries a commit release/1.0 lacks. The file documents 0f14c2a itself in §1 without connecting it to the merge shape it creates.
  2. predeploy_check.py appears nowhere in docs/RELEASE.md. The one check that stands between a release and a refused config on a live battery is not a step in the release procedure. It should be, between §1.3 and §1.4.
  3. §1.4 says "Merge the pull request" and that "a green check is the whole gate; there is no second pair of eyes unless someone asks for one." In this project's workflow the dev agent opens and a review agent merges. As written, §1 reads as licence for whoever opened the PR to merge it.
  4. §1.5's ha store reload step is correct and easy to miss — worth keeping prominent. Not run here; this PR is not a deploy.

What this PR is not

Do not treat opening it as authorisation to deploy. No tag has been cut (tagging follows the merge). Nothing has been installed, and ha store reload / ha addons update has not been run.

Cuts **0.11.0** onto `master`, the branch the Home Assistant Supervisor clones. Merging this is a deployment to a live GW5000S-BP currently running 0.10.1. Carries **SAFETY-56**, **TEL-11**, the **CI-9** follow-up and **SEC-2**'s `docs/RELEASE.md`. This is the first release to follow `docs/RELEASE.md` §1. --- ## ⚠️ Before anyone presses *Update*: `predeploy_check.py` has NOT been run against current options `tools/predeploy_check.py` returned **0 / CLEARED** — but against a **stale options export dated 2026-09-07 00:30** (`/home/homelab/live-options.json`). That export predates **both** the 0.10.0 and the 0.10.1 installs. It is not a statement about the blob on the box today. **That is a weak positive, not a clearance.** Before pressing *Update*, the owner must export the **current** options blob through the Supervisor API and re-run: ``` python3 goodwe_controller/tools/predeploy_check.py \ --deployed-version 0.10.1 --options <fresh-export.json> ``` Exit 0 = cleared, 1 = REFUSED, **2 = NOT CLEARED** (a check did not run — not the same as passing). Run it where `aiohttp` exists, or the validator half skips and exits 2. This is not a footnote. Nothing in this PR re-runs it, and CI does not run it either. --- ## ⚠️ This is a real merge, not a fast-forward The last two ships were fast-forwards. This one is not. `master` is **no longer an ancestor** of `release/1.0`: ``` merge base b0b24e5 (0.10.1) release/1.0..master 0f14c2a "PKG-16: ship 0.10.1 to master" ``` `0f14c2a` is a `--no-ff` merge commit pushed **directly to `master`** on 2026-09-22, before SEC-2 protected the branch. It exists only on `master` and has no `(#NN)` suffix — `docs/RELEASE.md` §1 names it as the commit that motivated the protection. Its tree is identical to `b0b24e5`, so it carries **no content delta**; it is purely a history artefact. Consequence: merging this produces a genuine merge commit. Verified independently with `git merge-tree --write-tree origin/master origin/release/1.0` over the merge base — **exit 0, one tree, zero conflict markers**. Result: **22 files, +3334 / -29**. ⚠️ `docs/RELEASE.md` §1 closes with *"`release/1.0` is normally an ancestor of `master`"* and assumes the fast-forward case. It does not anticipate this shape — which is odd, because the very commit that creates it is documented three paragraphs earlier in the same file. See "Notes on the procedure" below. --- ## What an operator can observe Two tickets are visible on an installed add-on. **Neither changes a watt.** `app/control.py` is not in this diff, and neither are `allocator.py`, `arbiter.py`, `arbitrage.py`, `clamp.py`, `peakguard.py`, `maintenance.py` or `watchdog.py`. ### SAFETY-56 — three wordings stop naming healthy hardware as the fault **Text only. Behaviour measured byte-identical** (`tools/safety56_behaviour_trace.py`: 2390 one-second reads replayed from the committed 2026-09-21 bus capture, a 66-cell command×register grid stepped 120 times each, six driven controller worlds — document hash `6030175ed2ab9a41…` identical across both trees while the sentences differ; the tool refuses to report "unchanged" if the sentence halves agree). SAFETY-37's freshness gate fires on a conjunction satisfied by **two disjoint worlds**: a register that stopped reporting reality, or a pack that *cannot comply* (a BMS refusing discharge at its floor holds still under any setpoint). Nothing the gate is handed separates them, and three sentences asserted the first. On the one measured episode it was the **second** — pack at 11 %, every command in the 850 s hold a discharge, register answered the first CHARGE three seconds later. The hardware was healthy and the add-on said it had failed. Three wordings, four surfaces: | surface | 0.10.1 | 0.11.0 | |---|---|---| | action log **and** status page (both quote `freshness.py`'s `STALE_READING:` verbatim) | "the reading is not following, so it is reported as MISSING" | "TWO THINGS LOOK LIKE THIS AND THIS GATE CANNOT TELL THEM APART …" | | add-on log, ERROR, from `main.py` | "battery-power register is not following its own command" | "battery-power reading did not move under a command that did, WHICH IS NOT BY ITSELF A VERDICT ON THE REGISTER" | | readiness row, diagnostics page | "battery power: `<entity>` is missing or not numeric" | "no usable value this cycle from `<entity>` — and this row cannot tell which of two things that is … Confirm the entity in Developer Tools before replacing it" | The third row is the one that cost an operator an evening: it asserted a false fact about a **named entity**, because `Freshness.reading()` returns `None` by design when the gate withholds a present, numeric, updating value. Fixed by saying less. It still does not tell you *which* of the two it was — that would need SoC and the pack floor, which are deliberately not `freshness.py`'s inputs. ### TEL-11 — `pv_entity`, optional, and **not a control input** One new key. **It is not a control input and it does not change what the loop does.** It records what the site's PV is producing beside what the meter and the battery are doing — an observability channel only. **Unset behaves exactly as 0.10.1 does today**, and that is measured, not asserted (`tools/tel11_pv_trace.py`: 5 grid behaviours × 8 PV-sensor behaviours × 24 cycles of the *shipped* `Controller`). Empty means the add-on **reads no entity, announces no entity, publishes no topic, shows no status row and adds no page row.** It is an **HA entity, not a register** — the RS485 bus carries the battery inverter; a household PV inverter is normally a different box from a different vendor. Positive is production; there is deliberately no `pv_invert` (the quantity is unipolar and read by nothing, so a reversed sensor announces itself as −4 kW at noon). The entity's own `unit_of_measurement` is believed; a unitless or non-power sensor is **refused** with the reason on the status page. ## Will a stored options blob be refused? **No new refusal is introduced by this hop.** Option surface 75 → 76 keys, `options:` and `schema:` in step. No key removed, no default moved, no enum widened, and **no rule added to `validate.py`** — the validator does not mention `pv_entity` at all. The key is `str?` (optional) defaulting to `""`, so a blob written on 0.10.0/0.10.1 simply does not carry it and the Supervisor's merge supplies the empty string. There is no `SystemExit(78)` to plan around on 0.10.1 → 0.11.0. ⚠️ That is the *delta*. It is not a clearance for the box — the pre-existing rules from 0.8.0/0.9.0/0.10.0 (SAFETY-39 rule 10's `slew_w` ceiling, TARIFF-07's three withdrawn `price_provider` ids) still apply on 0.11.0 and still refuse a config nobody edited. The reference install is on `price_provider: off` and `slew_w: 400`, so neither is expected to bite — **but that is from the stale export, which is exactly why the re-run above is required.** ## Slug `slug: goodwe_controller` — **unchanged**. This is an **update in place**, not a second add-on. `version:` bumps `"0.10.1"` → `"0.11.0"`, which is what makes the Supervisor rebuild and offer it at all. ## Not in the operator changelog, on purpose CI-9's follow-up (`9f6b9f1`) and SEC-2 (`239e120`) are in this range but deliberately excluded from `CHANGELOG.md`'s table, under PKG-15's rule — audience, not importance. Both sit outside `goodwe_controller/`, neither is in the built image, and no install can observe either. ## Notes on the procedure (`docs/RELEASE.md`, first real exercise) Reported rather than fixed here — they are documentation defects, not blockers: 1. **§1's closing note assumes a fast-forward.** *"`release/1.0` is normally an ancestor of `master`, so a `release/1.0 → master` pull request is empty until new commits land"* — true only when the branches are equal, and it gives no guidance for the case this PR is in, where `master` carries a commit `release/1.0` lacks. The file documents `0f14c2a` itself in §1 without connecting it to the merge shape it creates. 2. **`predeploy_check.py` appears nowhere in `docs/RELEASE.md`.** The one check that stands between a release and a refused config on a live battery is not a step in the release procedure. It should be, between §1.3 and §1.4. 3. **§1.4 says "Merge the pull request"** and that "a green check is the whole gate; there is no second pair of eyes unless someone asks for one." In this project's workflow the dev agent opens and a **review agent merges**. As written, §1 reads as licence for whoever opened the PR to merge it. 4. **§1.5's `ha store reload` step is correct and easy to miss** — worth keeping prominent. Not run here; this PR is not a deploy. ## What this PR is not **Do not treat opening it as authorisation to deploy.** No tag has been cut (tagging follows the merge). Nothing has been installed, and `ha store reload` / `ha addons update` has not been run.
admin added 13 commits 2026-09-24 19:37:23 +02:00
The residual-risk paragraph at the top of checks.yml claims the `guard the
trigger` step "keeps it that way mechanically rather than by asking the next
editor to read this comment". For blind spots 1 and 3 that is not true: the
guard lives inside the workflow a filter makes Gitea skip, so for those cases
the defence IS the comment.

That paragraph is the preamble someone reads at the exact moment they are
editing the trigger, so a summary running ahead of its detail is worse here
than elsewhere. It now points at the blind spots and states spot 3 plainly:
a filter naming a branch nothing targets is itself skipped, lands green and
untested, and then skips every subsequent pull request - the gate off for
everything, silently.

Flagged by the reviewer on PR #95 (issuecomment-1424) as non-blocking.

This PR is also the first real test of CI-9's fix on release/1.0 itself. Run
806 was a workflow_dispatch and posts no commit status at all, so it verified
content but never the gate. This one has to post `checks / unit
(pull_request)` as success rather than skipped.

Verified before push: the guard passes against this edited file (whose new
prose quotes `branches: [main]` and `master`) and still refuses a genuine
filter added to the `on:` block.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
master is the default branch, which is what the Home Assistant Supervisor
clones - so whatever sits on it runs on a real 5 kW inverter. Until today it
had no branch protection at all, while every gate this repo built sat on
release/1.0, the branch that does not ship. 0.10.1 went straight onto master
on 2026-09-22 with no PR, no CI and no reviewer, and it succeeded.

master now carries the same rule as release/1.0 - enable_push false,
required context `checks / unit (pull_request)`, block_admin_merge_override,
required_approvals 0. That is a forge setting and cannot live in a commit, so
the part that can is this: the procedure it implies. Without it the next
release is attempted the old way and fails confusingly at the push.

docs/RELEASE.md records the path (merge to release/1.0, bump version, PR
release/1.0 -> master, wait for green, merge, then `ha store reload` before
the Supervisor offers anything), the before/after protection state of both
branches, and how the gate was proven to bite rather than merely badge green.

Documentation only - no code, no workflow change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
Reviewed on the PR (issuecomments 1480, 1485, 1488, 1489). Documentation only: docs/RELEASE.md (new, 116 lines) plus a 2-line pointer in README.md's file map. No code, no workflow change.

Verified against the forge rather than accepted from the document:
- Both branch_protections rules re-read field-for-field identical (enable_push false, enable_force_push false, status check `checks / unit (pull_request)`, required_approvals 0, block_admin_merge_override true, no whitelists, no file patterns); master's rule created 2026-09-23T03:09:21; default_branch still master. Section 2's After table is exact. No protection was changed by this review - both rules re-read with their original updated_at.
- The empty-PR 405 trap reproduced independently on my own control (throwaway PR #111 between two same-SHA unprotected branches): empty PR -> 405 "Please try again later"; same PR after one commit -> 409 "head out of date". Section 1's "read the message, not the code" is correct and both messages are quoted accurately. PR #111 closed, branches deleted.
- release/1.0 (b0b24e5) is an ancestor of master (0f14c2a), so a literal release PR is empty right now - PR #109 has 0 commits, 0 files, and Gitea opened it anyway. Section 1's closing note is right.
- A master-base PR gets a genuine CI run, not a laundered skip: PR #107 head cb7e45b3 shows Waiting -> In progress -> success, "Successful in 7m56s", run 814. This PR's own head 9d593429 likewise, run 818.
- All five required steps are present and correctly ordered: work merges to release/1.0 with a version bump in goodwe_controller/config.yaml; release via PR release/1.0 -> master; direct push refused; green check before merge; `ha store reload` after, without which the Supervisor keeps serving the cached version. required_approvals: 0 is stated twice, so no reader can believe the dev/review split is enforced.

One non-blocking defect left open (issuecomment 1485): section 1 calls 0f14c2a "the only commit on master that carries no (#NN) merge suffix". 274 of master's 297 commits carry no suffix. The true statement is stronger - no release has ever reached master through a pull request, and 6f5e8e3 (0.10.0) was promoted by the same direct push one release earlier, so 0.10.1 was the second, not a one-off. Background, not procedure; wants a follow-up line change.

A third comment of mine (issuecomment 1488, merge style) is withdrawn in issuecomment 1489 - it rested on a premise I had not checked and which is false.
Three sentences in the blind-spot clause went stale between writing and review.

`no pull request in 99 has ever targeted master` is no longer true: SEC-2 made
`release/1.0` -> `master` the mandated release path and protected `master`
with the same required context, and a PR against that base has run for real.
The census it came from also erases its own evidence - PR #102 was itself
opened against `master` before retargeting, which the /pulls listing no longer
shows, exactly as the caveat 25 lines above this clause warns.

The clause also named `branches: [main]` as the hazard. That is the version
that looks wrong. The one someone would actually write is
`branches: [release/1.0]` - it reads as tightening onto the protected branch,
every day-to-day PR keeps running green, and the only thing it switches off is
the release PR whose content reaches an installed system.

And the prescribed fix named only `release/1.0`'s status_check_contexts. Since
SEC-2 `master` carries the same rule, so a reader following it literally would
half-close the hole and believe it shut.

Guard re-verified against the edited bytes: passes on this file (whose new
prose quotes `branches: [release/1.0]` in a comment) and still refuses a real
filter added to the `on:` block. Comment-only: 0 non-comment lines changed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
Text only. No behaviour change - same conjunction, same limits, same MISSING,
same 0 W, same NOT READY. `tools/safety56_behaviour_trace.py` says so: 2390
capture reads, a 66-cell swept grid, and six driven controller worlds produce
the byte-identical document `6030175e...` on origin/release/1.0 and on this
branch, while the sentences differ.

THE DEFECT. SAFETY-37's conjunction is satisfied by two disjoint worlds - the
register has stopped reporting reality, or the SOURCE cannot comply with the
command - and nothing `freshness.py` is handed separates them. Three sentences
asserted the first. On the live 2026-09-21 episode it was the second: the pack
read 11 % for the whole 850 s hold, every command in that hold was a discharge,
and the register answered the first CHARGE command three seconds later. The
readiness row was worse still, because it asserted a false fact about a NAMED
entity that was present, numeric and updating - so the operator opened
Developer Tools, found it working, and had every reason to conclude the
diagnostics were broken.

THE FIX. Each message states what was OBSERVED and names BOTH explanations.
Three wordings, no shared helper: two are a log line about a behaviour, the
third is a banner about an entity, and a helper serving both would drop
whichever half the other caller does not need. SoC stays STATED, NOT WIRED -
part 11b drives the two worlds at 50 % and 11 % and the sentence does not move
one byte. The row does not branch on `batt_fresh.stale` either: `stale_note()`
already rides `liveness_notes()` a few rows below, so the discriminator is "is
that row there?", and part 11d asserts it is present in one world and absent in
the other.

THE CHECKS. test_freshness.py part 11 builds the pair of worlds with ground
truth written down rather than derived from the published stream, asserts they
are really two (40 of 40 reads differ) while the published stream is
bit-identical, and only then grades the sentences. Eleven mutations, all red:
each of the three sentences reverted (3, 2 and 2 named reds), the two
explanations transposed at ZERO bytes (1), the second explanation deleted (1),
the admission of ambiguity replaced by a ranking (1), all three reverted at
once (5), and two aimed at this card's own check - collapsing the world pair
(2) and emptying the accusation list, which a positive control against the
three sentences as 0.10.1 shipped them catches (1).

AND TWO OF THE ELEVEN FOUND DEFECTS IN THE CHECK RATHER THAN IN THE CODE.
Transposing `label` and `entity` in the row made it unfindable and the run died
on an IndexError - a red with no name and every later check ungraded, FCST-06's
shape exactly; `_s56_row()` returns a named absence now, 0 named reds before, 3
after. And the first invariance run compared a tree with itself, because it
archived a branch whose edits were not committed yet; the tool now REFUSES to
report "behaviour unchanged" unless the sentences halves differ, and both of
its refusals are driven.

SAFETY-57's part 10h check was the handoff and is inverted rather than deleted:
it required the accusation to still be present so that removing it had to be a
deliberate act. This is that act. Its escape clause is untouched.

Suite: 30/30 in the CI image, checks.yml's exact command, purged tree, :ro.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
Text only, no behaviour change. Three operator-facing sentences stop asserting a dead register where the gate cannot distinguish a dead register from a pack the BMS is refusing.

Reviewed on the PR: behaviour byte-identical (trace tool sha256 6030175ed2ab reproduced on both trees, plus AST identity with string constants normalised); five mutations re-applied on purged trees, all matching claimed named-red counts; SoC of the live episode independently measured at 11 percent; CI green traced to PRNUM 112 / BASEREF release/1.0.
CI-9 follow-up: the skip direction, written the right way round
checks / unit (pull_request) Successful in 8m37s
8f9c2cad57
`every subsequent pull request matching it is skipped too` is the inverse of
the behaviour. A pull request that MATCHES a `branches:` filter still RUNS;
the one the filter EXCLUDES is what Gitea sits out, reports `skipped`, and the
rollup launders into `success`.

The error arrived with the generalisation in the previous commit: `33e17db`
said "every subsequent pull request is skipped too", which was unqualified and
true for the `[main]` case it described. Naming the wider hazard made a
qualifier necessary, and the wrong one went in - contradicting the paragraph
directly below it, which describes `[release/1.0]` correctly as leaving every
day-to-day PR running and green.

Corrected, and the direction is now stated explicitly with the failed draft
recorded, because this file already carries one instance of the same shape:
"That was wrong, it contradicted the checkout step's own note a few lines
below, and the error was not cosmetic."

Guard re-verified on the edited bytes: passes here, still refuses
`branches: [release/1.0]` added to the `on:` block. Comment-only, 0
non-comment lines changed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
Corrects the preamble's claim that the `guard the trigger` step keeps checks.yml filter-free "mechanically". It does not, for blind spots 1 and 3: the guard lives inside the workflow a filter makes Gitea skip, so for those the defence is the comment.

Names the filter that would actually get written - `branches: [release/1.0]`, not `[main]` - because it reads as tightening onto the protected branch, leaves every day-to-day pull request green, and switches off the `release/1.0` -> `master` release pull request that SEC-2 made the mandated path to an installed system. Points the forge fix at both protected branches, not just `release/1.0`.

Reviewed on the pull request. Verified from the branch's bytes: the guard's awk is unchanged and still refuses `branches:`, `paths:`, `paths-ignore:`, `branches-ignore:`, `tags-ignore:`, `types:` and the inline-flow form injected into the `on:` block of the edited file; comment-only at 0 non-comment changed lines, non-comment lines identical 139 == 139; the merged tree differs from c9aaf7f in this one file only. Green traced per CI-10 to run 824 / job 1949, payload PRNUM 102 / BASEREF release/1.0, base c9aaf7f, 30 PASS 0 FAIL.
OPT-10 measured that this site's pack reaches 90 % SoC on 2 days of 16 and that
20.5 kWh of surplus left the site while the pack had room - 12.6 kWh of it with
the battery not charging at all - and could not say WHY, because PV production
is recorded NOWHERE. Three causes need three different fixes (no irradiance:
nothing to fix; a charge-power limit; the optimiser declining to charge, which
is the only software defect) and guessing between them is how an optimiser gets
tuned to solve a cloud problem. This card adds the channel. It does not do the
attribution.

AN HA ENTITY, NOT A REGISTER, AND THAT IS A PRODUCT DECISION. This add-on's
RS485 bus carries the BATTERY inverter it drives; a household's PV inverter is
normally a different box from a different vendor reporting into Home Assistant
through its own integration. `pv_entity` takes whatever `sensor.*` the site's
own PV integration provides, any brand HA can read.

POSITIVE IS PRODUCTION, AND THERE IS NO `pv_invert`. The convention is derived
rather than assumed: `meter_entity` is positive when IMPORTING and `batt_entity`
is positive when DISCHARGING (DOCS.md, Sources), both of which are "power
arriving at the house's AC bus", and production arrives there too. Those two
keys carry invert flags because they are BIPOLAR and are READ BY THE CONTROL
PATH - `inv.batt` feeds SAFETY-30's per-unit sign check and SAFETY-31's
circulating-power sensor, so a reversed sensor hands the site a plausible number
it acts on. Production is unipolar and is read by nothing: a reversed sensor
announces itself as -4 kW at noon in the very history this card creates, and a
flag would only let an installer hide it. No negative-value alarm either - a PV
inverter's own standby draw makes a small negative reading at night ordinary, so
any threshold would be a number nobody measured.

ABSENCE IS NEVER ZERO, AND "DROPPED" IS NOT ABSENT. `publish()` SKIPS a None,
which is right for `circulation` and exactly wrong here: a skipped row leaves
Home Assistant holding the last number forever and the recorder draws a FLAT
LINE through the outage, which is indistinguishable from a measurement and
manufactures the "there was no surplus" conclusion this card exists to TEST. So
the absence is published, with HA's own payload: `mqtt/const.py` declares
`PAYLOAD_NONE = "None"` and `mqtt/sensor.py` tests it BEFORE the numeric branch,
so the entity's state becomes `unknown` and the recorder stores a GAP. Read out
of home-assistant/core `dev` on 2026-09-24 and quoted at the constant.
`ABSENT_AS_UNKNOWN` is a one-key set and the suite asserts `circulation: None`
is STILL dropped.

THE UNIT IS READ OFF THE ENTITY. `mW`, `W`, `kW`, `MW`, `GW`, `TW` - HA's own
`UnitOfPower`, every factor an exact power of ten. A sensor with no unit, or a
unit outside that table, is REFUSED by name and the status page says which.
`read_inputs()`'s `abs(raw) < 50` kW-or-W guess is safe for a quarter-hour
demand forecast and is NOT safe here: 30 W of dusk is an ordinary reading and
that heuristic would publish it as 30 kW.

GENUINELY OPTIONAL, AND NOT A SECOND `price_now_entity`. validate.py does not
mention the key - it cannot conflict with anything, because nothing else reads
it. The MQTT row lives in a new `OPTIONAL_SENSORS` list whose `include` argument
DEFAULTS TO EMPTY, so every caller that has never heard of `pv_entity` -
test_mqtt.py's fixtures, tools/rehearse_roster.py, anything written next year -
announces exactly what it announced before. Unset: no state call at all, no
entity, no topic, no status row, no page row. Set and failing: a CAUTION
(`ok=False, warn=True`), never a `bad` row, so a telemetry channel can never put
a battery site into NOT READY.

IT MUST NOT BECOME A CONTROL INPUT, AND THAT IS GRADED TWICE. `test_pv.py`
part 6 drives 40 cycles of the real `cycle()` and asserts the commands are
bit-identical across unset / no reading / 0 W / 8 kW, with a COMPUTED census
(33 modules under app/) showing the material is mentioned in exactly three:
pv.py, main.py, mqtt.py. `tools/tel11_pv_trace.py` is the cross-tree half -
`safety56_behaviour_trace.py`'s pattern with a DIFFERENT control clause, because
this card's claim is that nothing differs rather than that only sentences do: it
REFUSES unless the two `app/` fingerprints differ and unless exactly one side
reacts to the option. Measured on 41 stimulus cells:

    control   release/1.0 unset  846a10a5b89e321086d535825925721e2f791012e8405948d4fd38d3f9d0be5f
    control   TEL-11      unset  846a10a5b89e321086d535825925721e2f791012e8405948d4fd38d3f9d0be5f
    control   TEL-11      set    846a10a5b89e321086d535825925721e2f791012e8405948d4fd38d3f9d0be5f
    telemetry release/1.0 unset  a443a6ceaffe072b44bcd5f2ad946c1ed6c9e01b69e278eca631f7f4d7892164
    telemetry TEL-11      unset  a443a6ceaffe072b44bcd5f2ad946c1ed6c9e01b69e278eca631f7f4d7892164
    telemetry TEL-11      set    4e6d1cb289a957223ad0205f732a6fec6110ce1f2032c98ad60694e67625274f

UNSET is byte-identical in BOTH documents, which is stronger than "the control
path is untouched": an install that does not set the option sees no change of
any kind, announcement and status rows included.

⚠️ AND THE CARD'S CENTRAL MUTATION WAS A SURVIVOR ON THE FIRST BATTERY.
`self.grid -= self.pv_w` in `read_inputs()` - +106 B, on disk, purged, genuinely
running - came back GREEN, because the fixture drove a 6 kW export over 12
cycles, which puts the loop AGAINST ITS RAIL: a command moving `slew_w` every
cycle produces the same ramp for -6000 W and for -14 000 W of apparent error.
MUTATION.md habit 23 with a saturated ACTUATOR in place of a coinciding value,
and it is in that file now as the habit's second instance. The fixture now
drives a 600 W import the loop SETTLES on, against a responsive meter, and asks
habit 23's own question as its own check: shift the METER by the same 8 kW and
confirm the commands really would have moved.

Evidence: 31/31 suites green in the CI image, checks.yml's exact command, purged
tree, `:ro`. Ten mutations, NO survivors, each on its own `cp -a` of the tree so
habit 18's revert hazard is closed by construction - table in
`DECISIONS/TEL-11-pv-production.md` §7 with byte deltas and sha256s.

⚠️ THIS IS A RUNTIME CHANGE BEHIND AN UNCHANGED VERSION, AND
`tools/predeploy_check.py` WILL REFUSE IT. `app/`, `config.yaml` and
`translations/` are all in its `RUNTIME` tuple and `config.yaml` still reads
0.10.1, so the gate reports a NO-OP deploy until a version is cut. That is PKG's
call and this card does not take it.

No CHANGELOG section: `test_version_gate.py` 11b refuses a newest `## ` heading
that is not config.yaml's `version:`, and opening one would mean taking that
same packaging decision. The operator account is in DOCS.md.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
Reviewed on the PR. Green traced to run 825 (PRNUM 113, BASEREF release/1.0, attempt 1, real 8m6s execution). Control-path identity independently reproduced on both trees and counter-tested with a planted leak (red on 26/41 cells); all three of the trace tool substituted refusals fire. M7 genuinely red. Three publication states distinct on the wire. 31/31 in the CI image.

Not deployable as merged: config.yaml stays at 0.10.1, so predeploy_check.py reports a NO-OP until PKG cuts a version.
Two files, and no third. config.yaml 0.10.1 -> 0.11.0; CHANGELOG.md gains
`## 0.11.0` and three corrections to sections below it.

A MINOR BUMP, NOT A PATCH. TEL-11 adds an option - `pv_entity`, 75 keys -> 76 in
`options:` and `schema:` together - which is the same test 0.8.0, 0.9.0 and
0.10.0 were cut on. Nothing here can refuse a stored blob: the key is `str?`
with an `options:` default of "", validate.py does not mention it, no default
moved and no enum widened, so a blob that starts the add-on on 0.10.1 starts it
on 0.11.0.

THE TABLE HAS TWO ROWS AND THE WINDOW HAD FOUR MERGES. CI-9's follow-up
(9f6b9f1, `.gitea/workflows/checks.yml` only) and SEC-2 (239e120,
`docs/RELEASE.md` + README.md) are excluded under PKG-15's rule - AUDIENCE, NOT
IMPORTANCE. Neither file is under `goodwe_controller/`, neither is in the built
image, neither is in predeploy_check.py's RUNTIME tuple, and there is no version
of this add-on on which an operator can observe either. SEC-2 is the closer call
and the section argues it in full rather than asserting it: it is the more
consequential ticket of the two - it is why this release is a pull request
rather than a push to master - and it is still a procedure for whoever cuts a
release, not something the person deciding whether to press Update can act on.
Both are named in the section with pointers to PR #102 / checks.yml and PR #108
/ docs/RELEASE.md.

WHAT THE ENTRIES LET AN OPERATOR DECIDE. SAFETY-56: behaviour byte-identical
(6030175ed2ab..., reproduced in the CI image over 2390 capture reads, a 66-cell
grid stepped 120 times and six driven worlds), three surfaces named with the old
and new wording side by side - the STALE_READING line quoted onto the action log
and status page, the SAFETY-37 ERROR log line, and the readiness row that
asserted a false fact about a named entity. TEL-11: unset is byte-identical in
BOTH the control and telemetry documents, set is byte-identical in the control
document only; the three TEL-11 rows of that table were re-measured in the CI
image on this tree (control 846a10a5... twice, telemetry a443a6ce... / 4e6d1cb2...).
It is stated plainly that pv_entity is not a control input and does not change
what the loop does, and that absence publishes `unknown` rather than 0.

THREE CORRECTIONS BELOW THE NEW SECTION, WITHOUT REWRITING HISTORY. The 0.10.1
preamble said SAFETY-56 was open; the SAFETY-57 entry said its accusatory
diagnosis half was "deliberately untouched"; the 0.10.0 preamble left SAFETY-56
with no pointer while SAFETY-57 had one. All three quotations are what the
0.10.1 image actually prints and are left standing - a reader on 0.10.1 has to
recognise them - each now scoped to 0.10.1 and pointed at `## 0.11.0`.

VERIFIED. test_version_gate.py part 11 all four: 11a one contents table and the
slicer closed (302 lines under '0.11.0'), 11b 0.11.0 == config.yaml, 11c all
three RETIRED_PROVIDERS ids named - scoped to a pre-0.10.0 crossing and pointing
at `## 0.10.0`, which is where the evidence is - 11d still exactly one
`## Unreleased` below, PKG-08's, untouched. 31/31 in the CI image, checks.yml's
exact command, purged tree, `:ro`.

Not deployed and not installed. Shipping is a separate pull request from
release/1.0 into master, per docs/RELEASE.md.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
PKG-17: the SAFETY-57 correction pointed at the wrong paragraph
checks / unit (pull_request) Successful in 8m13s
b11bea70d3
Self-review. The note added to `### SAFETY-57` said the reading rule it is
qualifying is "two paragraphs up"; it is in the paragraph directly above - the
same one the note follows. A changelog that misdirects a reader to a paragraph
that is not there is the small end of exactly the defect this cut is fixing
three sentences of elsewhere, so it is corrected rather than left.

Text only, one line pair, still two files in the branch's diff.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
Bumps config.yaml 0.10.1 -> 0.11.0 and adds the `## 0.11.0` section, so the Supervisor offers the update that carries SAFETY-56's wording fix and TEL-11's `pv_entity`.

Reviewed at head b11bea7. Green traced to this PR's own run (827, attempt 1, payload names PR 114 base release/1.0, 31/31 real). Version gate 11a-11d, SAFETY-56's behaviour hash and TEL-11's four trace rows all re-measured on this tree and matching. slug unchanged; option surface 75 -> 76 with no validator rule added.

Two non-blocking findings recorded on the PR.

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

CI provenance, traced (CI-10)

Statuses on this forge are keyed by head SHA, so a green on cccb2fa could in principle belong to a different pull request. Traced rather than assumed:

  • Exactly one status exists on cccb2fa (count=1), context checks / unit (pull_request), success, pointing at actions/runs/828/jobs/1953.
  • Run 828 — event=pull_request, head_sha=cccb2fa07b5e3c404a059f45665f457e722167db, conclusion=success, created 19:37:27 (one second after this PR was opened), ran 19:37:28 → 19:45:28 (8 minutes, started_at is real and not the Unix epoch, so this is not a cancelled/never-executed run).
  • Its payload names this PR against this base. From the job log:
BASEREF: master
base  master                0f14c2a5def43dafcaf1287e50fa26541090d3ad
head  refs/pull/115/head    cccb2fa07b5e3c404a059f45665f457e722167db
GRADED: refs/pull/115/head merged into master
OK: the `on:` block of the merged checks.yml carries no event filter.
  • All six steps executed with conclusion: success — checkout, merge-grading, trigger guard, image build, the add-on's own tests, cleanup. No step was skipped, so this is not the laundered skipped → success rollup that CI-9 warns about.
  • It graded the merged tree (refs/pull/115/head merged into master), not the bare branch — so the green covers the merge result, which is the thing being decided here.

CI-9's premise held: on: pull_request with no branches: filter, and a master base matched. The trigger-guard step re-verified that in the merged tree.

No run was rerun. A rerun replays the stored payload and can post a spurious failure; nothing here was replayed.

Verified remote state at time of writing

refs/heads/master        0f14c2a5def43dafcaf1287e50fa26541090d3ad   version "0.10.1"
refs/heads/release/1.0   cccb2fa07b5e3c404a059f45665f457e722167db   version "0.11.0"

Read back with git ls-remote. Both branches unmoved; no v0.11.0 tag exists (tagging follows the merge); branch protection untouched on both branches.

## CI provenance, traced (CI-10) Statuses on this forge are keyed by **head SHA**, so a green on `cccb2fa` could in principle belong to a different pull request. Traced rather than assumed: * **Exactly one** status exists on `cccb2fa` (`count=1`), context `checks / unit (pull_request)`, **success**, pointing at `actions/runs/828/jobs/1953`. * Run **828** — `event=pull_request`, `head_sha=cccb2fa07b5e3c404a059f45665f457e722167db`, `conclusion=success`, created `19:37:27` (one second after this PR was opened), ran `19:37:28 → 19:45:28` (**8 minutes**, `started_at` is real and not the Unix epoch, so this is not a cancelled/never-executed run). * Its payload names **this** PR against **this** base. From the job log: ``` BASEREF: master base master 0f14c2a5def43dafcaf1287e50fa26541090d3ad head refs/pull/115/head cccb2fa07b5e3c404a059f45665f457e722167db GRADED: refs/pull/115/head merged into master OK: the `on:` block of the merged checks.yml carries no event filter. ``` * All **six** steps executed with `conclusion: success` — checkout, merge-grading, trigger guard, image build, the add-on's own tests, cleanup. **No step was skipped**, so this is not the laundered `skipped → success` rollup that CI-9 warns about. * It graded the **merged** tree (`refs/pull/115/head` merged into `master`), not the bare branch — so the green covers the merge result, which is the thing being decided here. CI-9's premise held: `on: pull_request` with no `branches:` filter, and a `master` base matched. The trigger-guard step re-verified that in the merged tree. **No run was rerun.** A rerun replays the stored payload and can post a spurious failure; nothing here was replayed. ## Verified remote state at time of writing ``` refs/heads/master 0f14c2a5def43dafcaf1287e50fa26541090d3ad version "0.10.1" refs/heads/release/1.0 cccb2fa07b5e3c404a059f45665f457e722167db version "0.11.0" ``` Read back with `git ls-remote`. Both branches unmoved; no `v0.11.0` tag exists (tagging follows the merge); branch protection untouched on both branches.
Author
Owner

✅ Review 1/6 — Identity: this is an update in place, not a second add-on

The check that would cost the owner their 75 options and their /data if it were wrong. Verified two ways.

In the tree. The whole config.yaml header block (lines 1–45, which is everything above options:) diffs to exactly one line between master and cccb2fa:

2c2
< version: "0.10.1"
---
> version: "0.11.0"

slug: goodwe_controller — unchanged. So are name, arch (aarch64/amd64/armv7), init: false, startup: application, boot: auto, ingress: true, ingress_port: 8099, panel_icon, panel_title, homeassistant_api: true, hassio_api: true, services: [mqtt:want] and url.

Against the live box. Cross-checked the identity keys against the owner's own Supervisor export (/addons/<slug>/info, 2026-09-24 20:02). Every one matches 0.11.0's config.yaml:

key live box (on 0.10.1) 0.11.0 config.yaml
slug 68285ac7_goodwe_controller goodwe_controller (+ repo 68285ac7)
name GoodWe RS485 Controller GoodWe RS485 Controller
ingress_port 8099 8099
startup / boot application / auto application / auto
arch aarch64, amd64, armv7 aarch64, amd64, armv7
hassio_api / homeassistant_api true / true true / true
auth_api false not set (⇒ false)

The installed slug is the repository id 68285ac7 prefixed to the add-on slug goodwe_controller. Since the add-on slug does not move, the Supervisor resolves the same installed add-on and this presents as an Update, not a new install. September's swap is not being repeated here.

## ✅ Review 1/6 — Identity: this is an update in place, not a second add-on The check that would cost the owner their 75 options and their `/data` if it were wrong. Verified two ways. **In the tree.** The whole `config.yaml` header block (lines 1–45, which is everything above `options:`) diffs to **exactly one line** between `master` and `cccb2fa`: ``` 2c2 < version: "0.10.1" --- > version: "0.11.0" ``` `slug: goodwe_controller` — unchanged. So are `name`, `arch` (`aarch64/amd64/armv7`), `init: false`, `startup: application`, `boot: auto`, `ingress: true`, `ingress_port: 8099`, `panel_icon`, `panel_title`, `homeassistant_api: true`, `hassio_api: true`, `services: [mqtt:want]` and `url`. **Against the live box.** Cross-checked the identity keys against the owner's own Supervisor export (`/addons/<slug>/info`, 2026-09-24 20:02). Every one matches 0.11.0's `config.yaml`: | key | live box (on 0.10.1) | 0.11.0 `config.yaml` | |---|---|---| | `slug` | `68285ac7_goodwe_controller` | `goodwe_controller` (+ repo `68285ac7`) | | `name` | GoodWe RS485 Controller | GoodWe RS485 Controller | | `ingress_port` | 8099 | 8099 | | `startup` / `boot` | application / auto | application / auto | | `arch` | aarch64, amd64, armv7 | aarch64, amd64, armv7 | | `hassio_api` / `homeassistant_api` | true / true | true / true | | `auth_api` | false | not set (⇒ false) | The installed slug is the repository id `68285ac7` prefixed to the add-on slug `goodwe_controller`. Since the add-on slug does not move, the Supervisor resolves the same installed add-on and this presents as an **Update**, not a new install. September's swap is not being repeated here.
Author
Owner

✅ Review 2/6 — The version actually moves, and the live box confirms why it must

master: version: "0.10.1" → cccb2fa: version: "0.11.0". The bump is real and it is the only header change.

The owner's export corroborates the state they are asking about, independently of anything in this PR:

version          : 0.10.1
version_latest   : 0.10.1
update_available : false
state            : started

version_latest equals version because master — the branch the Supervisor clones — is still on 0.10.1. That is exactly why no Update button exists. Merging this moves master to 0.11.0, which is what makes version_latest advance and the button appear (after ha store reload, per §1.5 — not run here; this PR is not a deploy).

## ✅ Review 2/6 — The version actually moves, and the live box confirms why it must `master`: `version: "0.10.1"` → `cccb2fa`: `version: "0.11.0"`. The bump is real and it is the only header change. The owner's export corroborates the state they are asking about, independently of anything in this PR: ``` version : 0.10.1 version_latest : 0.10.1 update_available : false state : started ``` `version_latest` equals `version` because `master` — the branch the Supervisor clones — is still on 0.10.1. That is exactly why no Update button exists. Merging this moves `master` to 0.11.0, which is what makes `version_latest` advance and the button appear (after `ha store reload`, per §1.5 — **not run here; this PR is not a deploy**).
Author
Owner

✅ Review 3/6 — The green is this PR's own (CI-10), traced to the run's workflow ref

Statuses here are keyed by head SHA and one has laundered between PRs before, so "there is a green tick" is not evidence. Confirmed independently against the Gitea API.

Exactly one status context on cccb2fa. The combined endpoint returns state: success, total_count: 1, context checks / unit (pull_request) → run 828 / job 1953. (The per-SHA status list returns three rows — Waiting to run → In progress → Successful in 8m1s, ids 1/2/3 — but that is one check's lifecycle, all three pointing at the same run 828 job 1953, not three checks. One context, as reported.)

The run belongs to PR #115, not to #114. This is the decisive bit, because cccb2fa is simultaneously release/1.0's tip and the merge commit of #114, so its display_title reads as #114's subject. The run metadata settles it:

run 828   path: checks.yml@refs/pull/115/head      <-- workflow ref is PR 115
          head_sha: cccb2fa07b5e...                event: pull_request
          run_attempt: 1   previous_attempt_url: null

and the job's own log, from the checkout and grading steps:

PRNUM: 115
BASEREF: master
base  master                0f14c2a5def43dafcaf1287e50fa26541090d3ad
head  refs/pull/115/head    cccb2fa07b5e3c404a059f45665f457e722167db
GRADED: refs/pull/115/head merged into master

Both SHAs are the ones I verified from the remote. run_attempt: 1 with a null previous-attempt link — this is an original run, not a replay, and no old run was rerun during this review.

It graded the merge, not the branch. GRADED: … merged into master means CI tested the merge result. Combined with finding 4 (the merge tree is bit-identical to cccb2fa's tree), the content CI graded is byte-for-byte the content that lands on master.

The 8 minutes is real work. 31 test files, all PASS, wall-clock 17:37:27Z → 17:45:28Z = 8m01s, of which the test step is 7m54s — and the per-file timings sum to that (test_loop.py 93s, test_watchdog.py 67s, test_peakguard.py 65s, test_rehearse.py 42s …). A replayed payload does not produce a consistent timing breakdown. The two files carrying this release's changes both ran and passed: test_pv.py (TEL-11, new, 1s) and test_freshness.py (SAFETY-56, 21s). Runner goodwe-rig, python 3.14.7 / aiohttp 3.13.5.

Also green: the trigger guard — OK: the on: block of the merged checks.yml carries no event filter.

## ✅ Review 3/6 — The green is this PR's own (CI-10), traced to the run's workflow ref Statuses here are keyed by head SHA and one has laundered between PRs before, so "there is a green tick" is not evidence. Confirmed independently against the Gitea API. **Exactly one status context on `cccb2fa`.** The combined endpoint returns `state: success`, `total_count: 1`, context `checks / unit (pull_request)` → run 828 / job 1953. (The per-SHA status *list* returns three rows — `Waiting to run` → `In progress` → `Successful in 8m1s`, ids 1/2/3 — but that is one check's lifecycle, all three pointing at the same run 828 job 1953, not three checks. One context, as reported.) **The run belongs to PR #115, not to #114.** This is the decisive bit, because `cccb2fa` is simultaneously `release/1.0`'s tip *and* the merge commit of #114, so its `display_title` reads as #114's subject. The run metadata settles it: ``` run 828 path: checks.yml@refs/pull/115/head <-- workflow ref is PR 115 head_sha: cccb2fa07b5e... event: pull_request run_attempt: 1 previous_attempt_url: null ``` and the job's own log, from the checkout and grading steps: ``` PRNUM: 115 BASEREF: master base master 0f14c2a5def43dafcaf1287e50fa26541090d3ad head refs/pull/115/head cccb2fa07b5e3c404a059f45665f457e722167db GRADED: refs/pull/115/head merged into master ``` Both SHAs are the ones I verified from the remote. `run_attempt: 1` with a null previous-attempt link — **this is an original run, not a replay**, and no old run was rerun during this review. **It graded the merge, not the branch.** `GRADED: … merged into master` means CI tested the merge result. Combined with finding 4 (the merge tree is bit-identical to `cccb2fa`'s tree), the content CI graded is byte-for-byte the content that lands on `master`. **The 8 minutes is real work.** 31 test files, all PASS, wall-clock 17:37:27Z → 17:45:28Z = 8m01s, of which the test step is 7m54s — and the per-file timings sum to that (`test_loop.py` 93s, `test_watchdog.py` 67s, `test_peakguard.py` 65s, `test_rehearse.py` 42s …). A replayed payload does not produce a consistent timing breakdown. The two files carrying this release's changes both ran and passed: **`test_pv.py` (TEL-11, new, 1s)** and **`test_freshness.py` (SAFETY-56, 21s)**. Runner `goodwe-rig`, python 3.14.7 / aiohttp 3.13.5. Also green: the trigger guard — `OK: the `on:` block of the merged checks.yml carries no event filter.`
Author
Owner

✅ Review 4/6 — Merge shape: a real merge, and it contributes exactly release/1.0's tree

Both of the dev agent's claims verified, and the second one is provable more strongly than stated.

Not a fast-forward — confirmed.

git merge-base --is-ancestor origin/master cccb2fa   -> exit 1 (NOT an ancestor)
merge base                                           -> b0b24e5
cccb2fa..origin/master                               -> 0f14c2a  (one commit, master-only)
origin/master..cccb2fa                               -> 12 commits

0f14c2a carries no content delta — confirmed by tree hash, not by diff. It is a --no-ff merge whose second parent is the merge base:

0f14c2a parents: 6f5e8e3 b0b24e5
0f14c2a tree:    c1fef26fd7325ccaf92428c20d3914f5ba0d1bf1
b0b24e5 tree:    c1fef26fd7325ccaf92428c20d3914f5ba0d1bf1   <-- identical
git diff b0b24e5 0f14c2a  -> empty

Same tree object. So the pre-SEC-2 direct push is purely a history artefact, as described.

Zero conflicts, and the stronger result:

git merge-tree --write-tree origin/master cccb2fa
  -> exit 0, one tree: 6670502052a2c6725d4a2b9b3fab1c8fae37813a
cccb2fa^{tree}
  -> 6670502052a2c6725d4a2b9b3fab1c8fae37813a       <-- identical

No conflict markers anywhere in the result. And because the merge tree is bit-identical to cccb2fa's own tree, master contributes nothing to the merged content — which follows from 0f14c2a being tree-identical to the merge base. Practical consequence: the tree that lands on master is exactly the reviewed release/1.0 tree. A merge commit appears in history; not one byte of file content is synthesised by the merge.

Diffstat confirms the PR's figure: 22 files changed, 3334 insertions(+), 29 deletions(-).

## ✅ Review 4/6 — Merge shape: a real merge, and it contributes exactly `release/1.0`'s tree Both of the dev agent's claims verified, and the second one is provable more strongly than stated. **Not a fast-forward — confirmed.** ``` git merge-base --is-ancestor origin/master cccb2fa -> exit 1 (NOT an ancestor) merge base -> b0b24e5 cccb2fa..origin/master -> 0f14c2a (one commit, master-only) origin/master..cccb2fa -> 12 commits ``` **`0f14c2a` carries no content delta — confirmed by tree hash, not by diff.** It is a `--no-ff` merge whose second parent *is* the merge base: ``` 0f14c2a parents: 6f5e8e3 b0b24e5 0f14c2a tree: c1fef26fd7325ccaf92428c20d3914f5ba0d1bf1 b0b24e5 tree: c1fef26fd7325ccaf92428c20d3914f5ba0d1bf1 <-- identical git diff b0b24e5 0f14c2a -> empty ``` Same tree object. So the pre-SEC-2 direct push is purely a history artefact, as described. **Zero conflicts, and the stronger result:** ``` git merge-tree --write-tree origin/master cccb2fa -> exit 0, one tree: 6670502052a2c6725d4a2b9b3fab1c8fae37813a cccb2fa^{tree} -> 6670502052a2c6725d4a2b9b3fab1c8fae37813a <-- identical ``` No conflict markers anywhere in the result. And because the merge tree is **bit-identical to `cccb2fa`'s own tree**, `master` contributes *nothing* to the merged content — which follows from `0f14c2a` being tree-identical to the merge base. Practical consequence: **the tree that lands on `master` is exactly the reviewed `release/1.0` tree.** A merge commit appears in history; not one byte of file content is synthesised by the merge. Diffstat confirms the PR's figure: `22 files changed, 3334 insertions(+), 29 deletions(-)`.
Author
Owner

✅ Review 5/6 — The stale-export warning at the top of this PR is now OBSOLETE

Read this before you read the PR body's first ⚠️ block. That warning was accurate when written and is no longer the situation. predeploy_check.py has now been run against the owner's current options, and I reproduced it independently rather than taking the report on trust.

A fresh blob was exported from the Supervisor on 2026-09-24 20:02 (/home/homelab/live-options-now.json) — a real /addons/<slug>/info response showing version: 0.10.1, state: started, 75 options, pv_entity absent. That supersedes the 2026-09-07 00:30 export (70 keys) the PR body warns about.

Re-run in the CI image, so both halves run and the validator is not skipped:

docker run --rm -v <stage>:/w:ro goodwe-addon-checks:1 \
  python3 /w/tree/goodwe_controller/tools/predeploy_check.py \
  --deployed-version 0.10.1 --options /w/opts-bare-20260924.json

  deployed on the box : 0.10.1
  shipped in this tree: 0.11.0
  live options pass this version's validator

  CLEARED for deploy.
EXIT=0

Exit 0, CLEARED, against the blob that is on the box today. Not exit 2, so no half was skipped. The two pre-existing rules the PR body flags as "expected not to bite" are confirmed against the current blob rather than the stale one: slew_w: 400 (under SAFETY-39 rule 10's 555.6 W ceiling on the 5000 W rail) and price_provider: 'off' (so TARIFF-07's three withdrawn ids are unreachable). validate_startup_config() returns 0 problems.

So: the check has been done properly, on current data. Nobody reading this PR later should conclude otherwise.


⚠️ One caveat on the tool, not on this release — worth a card

While confirming the above I found a latent false-positive path in predeploy_check.py. It does opts = json.load(fh) and passes the whole document to validate_startup_config(). The natural thing an operator exports is the raw Supervisor response, which is wrapped:

{"result": "ok", "data": {"options": { …75 keys… }, "schema": …, "translations": …}}

Handed that wrapper, the tool still prints live options pass this version's validator / CLEARED for deploy. / exit 0 — but the options were never checked. Every rule that reads a top-level key (opts.get("slew_w"), opts.get("price_provider"), …) sees nothing and silently passes. It escapes the "no actuator" refusal only by accident: actuator_writers() walks nested structures, so it finds data.options.setpoint_entity — and also four bogus "actuators" scraped out of the translations blob, each with one writer, so the duplicate-writer rule does not fire either. Net: 0 problems, a confident CLEARED, nothing verified.

This is a weak positive that looks exactly like a clearance — the same failure mode the file's own docstring says it exists to prevent ("a skipped check must never look like a passed one"). It did not affect this release: I ran it both ways and used the correctly-shaped bare 75-key dict as the authoritative result, which is the run quoted above. But the tool should unwrap data.options when it sees that shape, or refuse a document that does not look like an options blob.

Not fixed here and not a blocker. Filing alongside the four docs/RELEASE.md defects.

## ✅ Review 5/6 — The stale-export warning at the top of this PR is now OBSOLETE **Read this before you read the PR body's first ⚠️ block.** That warning was accurate when written and is no longer the situation. `predeploy_check.py` **has** now been run against the owner's **current** options, and I reproduced it independently rather than taking the report on trust. A fresh blob was exported from the Supervisor on **2026-09-24 20:02** (`/home/homelab/live-options-now.json`) — a real `/addons/<slug>/info` response showing `version: 0.10.1`, `state: started`, **75 options**, `pv_entity` absent. That supersedes the 2026-09-07 00:30 export (70 keys) the PR body warns about. Re-run in the CI image, so both halves run and the validator is not skipped: ``` docker run --rm -v <stage>:/w:ro goodwe-addon-checks:1 \ python3 /w/tree/goodwe_controller/tools/predeploy_check.py \ --deployed-version 0.10.1 --options /w/opts-bare-20260924.json deployed on the box : 0.10.1 shipped in this tree: 0.11.0 live options pass this version's validator CLEARED for deploy. EXIT=0 ``` **Exit 0, CLEARED, against the blob that is on the box today.** Not exit 2, so no half was skipped. The two pre-existing rules the PR body flags as "expected not to bite" are confirmed against the current blob rather than the stale one: **`slew_w: 400`** (under SAFETY-39 rule 10's 555.6 W ceiling on the 5000 W rail) and **`price_provider: 'off'`** (so TARIFF-07's three withdrawn ids are unreachable). `validate_startup_config()` returns **0 problems**. So: the check has been done properly, on current data. Nobody reading this PR later should conclude otherwise. --- ### ⚠️ One caveat on the *tool*, not on this release — worth a card While confirming the above I found a latent false-positive path in `predeploy_check.py`. It does `opts = json.load(fh)` and passes the **whole document** to `validate_startup_config()`. The natural thing an operator exports is the raw Supervisor response, which is wrapped: ```json {"result": "ok", "data": {"options": { …75 keys… }, "schema": …, "translations": …}} ``` Handed that wrapper, the tool still prints `live options pass this version's validator` / `CLEARED for deploy.` / exit 0 — **but the options were never checked**. Every rule that reads a top-level key (`opts.get("slew_w")`, `opts.get("price_provider")`, …) sees nothing and silently passes. It escapes the "no actuator" refusal only by accident: `actuator_writers()` walks nested structures, so it finds `data.options.setpoint_entity` — and also four bogus "actuators" scraped out of the `translations` blob, each with one writer, so the duplicate-writer rule does not fire either. Net: 0 problems, a confident CLEARED, nothing verified. This is a **weak positive that looks exactly like a clearance** — the same failure mode the file's own docstring says it exists to prevent ("a skipped check must never look like a passed one"). It did not affect this release: I ran it **both** ways and used the correctly-shaped bare 75-key dict as the authoritative result, which is the run quoted above. But the tool should unwrap `data.options` when it sees that shape, or refuse a document that does not look like an options blob. **Not fixed here** and **not a blocker.** Filing alongside the four `docs/RELEASE.md` defects.
Author
Owner

✅ Review 6/6 — Will a stored blob be refused? No. Verified against the current blob.

The PR body's compatibility argument re-derived from the trees and checked against the owner's live 75 keys.

Option surface, measured:

master   options=75  schema=75  version=0.10.1  slug=goodwe_controller
cccb2fa  options=76  schema=76  version=0.11.0  slug=goodwe_controller

added in 0.11.0    : ['pv_entity']
removed in 0.11.0  : []
schema added       : ['pv_entity']
schema removed     : []
shared defaults    : none moved - every shared default byte-identical
pv_entity          : options default ''   schema rule 'str?'

Exactly one key added, options: and schema: in step, nothing removed, no default moved.

And the part that matters for the box:

live keys not in 0.11.0's schema  : []        <- nothing becomes unknown
0.11.0 keys the live blob lacks   : ['pv_entity']

The only key the current blob lacks is pv_entity, which is str? — optional — defaulting to "", so the Supervisor's merge supplies the empty string and empty is off. No SystemExit(78) on 0.10.1 → 0.11.0.

No new refusal rule exists to hit. app/validate.py is not in this diff (0 of 22 files), and grep pv_entity over validate.py at cccb2fa returns 0 matches. The validator cannot refuse a key it does not know about.

The control path is untouched. Only four files under app/ change — freshness.py, main.py, mqtt.py, and the new pv.py. Confirmed absent from the diff: control.py, allocator.py, arbiter.py, arbitrage.py, clamp.py, peakguard.py, maintenance.py, watchdog.py, validate.py. Consistent with both tickets being observability/wording only.


On docs/RELEASE.md §1.4

Noting for the record that I am the second pair of eyes §1.4 says does not exist. §1.4's "a green check is the whole gate; there is no second pair of eyes unless someone asks for one" contradicts this project's workflow, in which the dev agent opens and a review agent merges. The dev agent correctly declined to merge its own PR and followed the workflow. §1.4 is a documentation defect being carded separately, along with §1's fast-forward assumption, the absent predeploy_check.py step, and now the tool caveat in 5/6 — not fixed in this PR, deliberately.


Verdict: sound. Merging.

All five release checks pass, each verified independently from the remote rather than from the report: identity unchanged (and cross-checked against the live box), the version moves, the green is this PR's own run keyed to refs/pull/115/head, the merge is structurally real but contributes exactly release/1.0's tree, and predeploy_check.py clears the blob that is on the box today.

No blocking finding. Nothing false found in the PR body — its one stale statement is the export warning, which 5/6 retires.

⚠️ Merging is not installing. No tag is cut here, ha store reload / ha addons update have not been run, and nothing has touched the box. The owner installs.

## ✅ Review 6/6 — Will a stored blob be refused? No. Verified against the current blob. The PR body's compatibility argument re-derived from the trees and checked against the owner's live 75 keys. **Option surface, measured:** ``` master options=75 schema=75 version=0.10.1 slug=goodwe_controller cccb2fa options=76 schema=76 version=0.11.0 slug=goodwe_controller added in 0.11.0 : ['pv_entity'] removed in 0.11.0 : [] schema added : ['pv_entity'] schema removed : [] shared defaults : none moved - every shared default byte-identical pv_entity : options default '' schema rule 'str?' ``` Exactly one key added, `options:` and `schema:` in step, nothing removed, **no default moved**. **And the part that matters for the box:** ``` live keys not in 0.11.0's schema : [] <- nothing becomes unknown 0.11.0 keys the live blob lacks : ['pv_entity'] ``` The only key the current blob lacks is `pv_entity`, which is `str?` — optional — defaulting to `""`, so the Supervisor's merge supplies the empty string and empty is off. **No `SystemExit(78)` on 0.10.1 → 0.11.0.** **No new refusal rule exists to hit.** `app/validate.py` is **not in this diff** (0 of 22 files), and `grep pv_entity` over `validate.py` at `cccb2fa` returns **0** matches. The validator cannot refuse a key it does not know about. **The control path is untouched.** Only four files under `app/` change — `freshness.py`, `main.py`, `mqtt.py`, and the new `pv.py`. Confirmed absent from the diff: `control.py`, `allocator.py`, `arbiter.py`, `arbitrage.py`, `clamp.py`, `peakguard.py`, `maintenance.py`, `watchdog.py`, `validate.py`. Consistent with both tickets being observability/wording only. --- ### On `docs/RELEASE.md` §1.4 Noting for the record that I am the second pair of eyes §1.4 says does not exist. §1.4's "a green check is the whole gate; there is no second pair of eyes unless someone asks for one" contradicts this project's workflow, in which the dev agent opens and a review agent merges. The dev agent correctly declined to merge its own PR and followed the workflow. §1.4 is a **documentation defect being carded separately**, along with §1's fast-forward assumption, the absent `predeploy_check.py` step, and now the tool caveat in 5/6 — **not fixed in this PR**, deliberately. --- ## Verdict: sound. Merging. All five release checks pass, each verified independently from the remote rather than from the report: identity unchanged (and cross-checked against the live box), the version moves, the green is this PR's own run keyed to `refs/pull/115/head`, the merge is structurally real but contributes exactly `release/1.0`'s tree, and `predeploy_check.py` clears the blob that is on the box today. No blocking finding. Nothing false found in the PR body — its one stale statement is the export warning, which 5/6 retires. ⚠️ Merging is **not** installing. No tag is cut here, `ha store reload` / `ha addons update` have not been run, and nothing has touched the box. The owner installs.
admin merged commit 78acb558ff into master 2026-09-24 20:28:36 +02:00
Author
Owner

✅ Merged — master is on 0.11.0

Verified from the remote after the merge, not from the API's success response.

master = 78acb558ff971dee2561b9804d0939eb89b1aad4

  parents: 0f14c2a5def43dafcaf1287e50fa26541090d3ad   (old master)
           cccb2fa07b5e3c404a059f45665f457e722167db   (release/1.0)
  tree:    6670502052a2c6725d4a2b9b3fab1c8fae37813a

A genuine two-parent merge commit, as predicted — and its tree is 6670502…, the exact tree git merge-tree pre-computed in 4/6 and the exact tree of cccb2fa. git diff origin/master cccb2fa is empty. So what now sits on master is byte-for-byte the release/1.0 tree that CI graded. The prediction and the outcome agree to the object hash.

config.yaml on master, read back from the remote:

name: GoodWe RS485 Controller
version: "0.11.0"
slug: goodwe_controller

version is 0.11.0. slug is unchanged. The Supervisor will resolve the same installed add-on and offer an Update, not a second install.

Branch protection untouched — master and release/1.0 both still protected=true. release/1.0 retained at cccb2fa (not deleted). No tag cut: tags remain v0.10.1, v0.10.0, v0.2.1; v0.11.0 is the owner's to cut against 78acb55.

Still to happen before the owner sees an Update button

Nothing in this merge has touched the box. ha store reload (§1.5) has not been run, and neither has ha addons update. The reference install still reports version_latest: 0.10.1 until the Supervisor re-reads the repository.

predeploy_check.py cleared the blob that was on the box at 2026-09-24 20:02 (5/6). If the owner changes options between now and pressing Update, that clearance is about the older blob.

## ✅ Merged — `master` is on 0.11.0 Verified from the remote after the merge, not from the API's success response. ``` master = 78acb558ff971dee2561b9804d0939eb89b1aad4 parents: 0f14c2a5def43dafcaf1287e50fa26541090d3ad (old master) cccb2fa07b5e3c404a059f45665f457e722167db (release/1.0) tree: 6670502052a2c6725d4a2b9b3fab1c8fae37813a ``` A genuine two-parent merge commit, as predicted — and its tree is `6670502…`, **the exact tree `git merge-tree` pre-computed in 4/6 and the exact tree of `cccb2fa`**. `git diff origin/master cccb2fa` is empty. So what now sits on `master` is byte-for-byte the `release/1.0` tree that CI graded. The prediction and the outcome agree to the object hash. `config.yaml` on `master`, read back from the remote: ``` name: GoodWe RS485 Controller version: "0.11.0" slug: goodwe_controller ``` **`version` is 0.11.0. `slug` is unchanged.** The Supervisor will resolve the same installed add-on and offer an **Update**, not a second install. Branch protection untouched — `master` and `release/1.0` both still `protected=true`. `release/1.0` retained at `cccb2fa` (not deleted). **No tag cut**: tags remain `v0.10.1`, `v0.10.0`, `v0.2.1`; `v0.11.0` is the owner's to cut against `78acb55`. ### Still to happen before the owner sees an Update button Nothing in this merge has touched the box. `ha store reload` (§1.5) has **not** been run, and neither has `ha addons update`. The reference install still reports `version_latest: 0.10.1` until the Supervisor re-reads the repository. `predeploy_check.py` cleared the blob that was on the box at 2026-09-24 20:02 (5/6). If the owner changes options between now and pressing *Update*, that clearance is about the older blob.
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: admin/goodwe-addon#115