fix: refuse a control the panel declares non-settable, and pin the capture that proves it (#161) - #164
Merged
Merged
Conversation
`switch` 0.3, vendored at packages/schema-1/spec/catalogs/switch.json, declares
`relay` "Settable when `relay-controllable = true`" and defines
`relay-controllable` false as "locked (for example a circuit commissioned as
permanently on)". `set_circuit_relay_target` and `set_circuit_priority_target`
were pure string formatting from a circuit id and consulted no declaration, so
both setters published to circuits the specification says are not settable --
while the same adapters were already reading exactly that into
`is_user_controllable` and `is_never_backup` for the snapshot. Both now return
`ControlTarget | None`, joining the two controls that have always refused this
way, and the client raises `SpanPanelServerError` the way
`set_evse_charge_limit` does.
The condition lives in the catalog's prose: `settable: true` stands
unconditionally in the JSON beside it, because that field describes the
property across the capability while the narrowing applies per device. So the
rule is encoded rather than derived, and a test reads the catalog and pins the
clause -- a specification that stops saying it fails there rather than leaving
a stale rule in force.
Under parent/child both halves of the condition are on the wire and the relay
refuses when either says no. SPAN reports a firmware defect in which the
`$settable` re-toggle on the runtime re-commissioning path is skipped until the
service restarts, so declaration and value can disagree on a live panel; the
panel rejects an out-of-policy write regardless, so the conjunction only refuses
writes the panel would refuse. Absence reads as locked on `switch/relay` and as
settable on `load-shed/priority`, and that asymmetry is each catalog entry's:
`switch` carries a condition, `load-shed` carries none. Flat predates capability
nodes and declares settability per device type, so it cannot vary per circuit;
it spells the same fact `always-on`, and the priority's `never-backup`.
Nothing was user-visible, because the integration gates entity creation on
`is_user_controllable`. That is not enough: re-commissioning a circuit in place
republishes its `$description` with a new `$settable`, so an entity can outlive
its own controllability, and `set_circuit_relay` is public API reachable
without an entity.
Route the refusals through the interceptor. Five refusals happened while
resolving the address and so never reached `_publish_control`, which is where
interception lives -- meaning `after_publish`, contracted to see every command
including refusals, was missing precisely the commands a panel had refused. A
consumer building a security audit on it would have had its hole where the
relay is. `ControlCommand.topic` and `PublishOutcome.topic` become `str | None`
so a command that never resolved to a topic is recorded truthfully rather than
named a topic nothing would publish to. `before_publish` stays unconsulted for
them: there is nothing to authorise, and a veto would replace a specific reason
with "vetoed".
Track the producer that made the reference capture, which is what blocked the
fix. `ebus-panel-sim` is published by electrification-bus -- the organisation
that writes the specification -- and is conformed against live panel output: the
specification in runnable form, and the right thing to test a consumer against.
Depending on it is correct. Depending on a frozen, unrecorded copy of it was
not: the capture was taken once, nothing wrote down what made it, and three
emitter releases went by while this repository asserted a producer defect as
fact across roughly thirty test files.
So `spec_lock.json` grows `peers`, keyed by name because both readers select a
peer by identity and neither iterates for its own sake. `ebus-panel-sim` is
pinned there with its repo, ref, commit, tag, released version and the
specification commit it implements -- which is the same commit we pin, and a
test now asserts that. panelbench keeps `fixtures`, paths inside panelbench that
are byte-copied here; the emitter gets `produces`, paths inside this repository
generated by its `capture_script` from its `manifest`. One key would have meant
two things depending on which peer you read it from.
`scripts/capture_parent_child_reference.py` substitutes the transport rather
than reassembling the emitter, reads its expected release out of the lockfile
rather than restating it, and refuses to write when the checkout disagrees.
`scripts/reference_panel.yaml` is its input, committed and pinned: a capture
whose input is not in the tree is the same problem as one whose producer is not
recorded. It mirrors the emitter's own example key for key and marks its two
divergences at the head of the file -- spec-legal shed priorities in place of a
value the emitter degrades to UNKNOWN (electrification-bus/
distribution-enclosure-simulator#51, open), and the identity properties a real
panel publishes.
The capture was three producer releases behind, and the suite had been reading
its errors as facts:
- `switch/relay` loses `$settable` on the locked circuit and its
`relay-requester` becomes CONFIGURATION -- the case this fix needed.
- `connection/count` is gone; no configuration could publish it.
- `power-flows/{pv,battery,grid}` and the BESS meter are in the frame
`power-flows` 0.3 defines, where the old bytes carried the pre-0.6.0 one
and did not balance. The parser was already right. Two tests contradicted
each other under the old fixture: one derived a charging battery from the
flows, the other asserted the snapshot reported it discharging.
- The lugs integrate their own meter, so the capture runs three ticks --
two intervals, one importing and one exporting -- because one interval
can now only populate one energy register.
- `load-shed/priority` carries values a real panel publishes. UNKNOWN is
spec-legal and every parser must handle it, so that obligation moves to
where it belongs: a test parametrised over `load-shed` 0.3's declared
`$format` rather than over whatever the capture happens to contain.
Contract obligations come from the catalog; representativeness comes from
the capture.
`peer-drift.yml` gains an emitter job. It is a released distribution rather
than a repository we copy out of, so "has it moved" has two answers and only
one is a reason to recapture: the job asks PyPI for the latest release and
reports commits past the pin as context. Schedule and dispatch only, never
`pull_request`, for the reason the existing job states.
`tests/fixtures/panelbench_unvalued_by_both.json` is refreshed from the
panelbench commit `spec_lock.json` already pins. The vendored copy carried 32
`connection/count` entries that commit had itself already dropped, so the
cross-check passed only because both sides were stale.
…y targets `firmware.family` read `spanos2` while `firmware.range` read `r202633+`. The range is right and the family is stale: production enclosures on r202633 report a `spanos3` build, which is also what the live panel this range was pinned from returns from `GET /api/v2/status`. Nothing read the field -- `test_the_peer_targets_the_same_firmware` asserts on `range` alone -- so this corrected no behaviour. It is provenance, and this file just became the thing peer-drift reads to decide whether a producer has moved past its pin, which makes a wrong value here worse than it was. The `spanos2` strings elsewhere are left alone deliberately: schema-0's changelog records the flat capture at `spanos2/r202603/05`, which is the family that firmware genuinely was, and the test fixtures carry synthetic version strings that assert nothing about the family. Noted in #161.
Two corrections from review of this branch. The remediation text in the panelbench job still named `peer.ref` and `peer.commit`, keys that stopped existing when `spec_lock.json` moved to a `peers` map. Advice that names a key the file does not have is worse than no advice: it sends the reader looking for something they will not find. More substantially, the emitter job could not notify anyone. Its release comparison printed an accurate, actionable summary -- here is the newer version, here is the command to regenerate -- and then exited 0, so the daily run went green in every outcome. GitHub notifies on a failed scheduled run and says nothing about a successful one, so the one path that had something to report was the one nobody would hear. This job exists because the last drift of its kind "took nine days to be noticed by hand", and a buried step summary would have repeated that exactly. It now exits non-zero when PyPI's latest is not the pinned release, and stays red until the capture is regenerated or the pin moves -- which is the honest state of a reference tree describing a producer that has been superseded. The job is schedule- and dispatch-only and gates no pull request, so red costs a notification and nothing else. PyPI being unreachable still reports unknown rather than drift: not having asked is not the same fact as there being nothing new. The commit-distance step gains `if: always()`, because it is context for that verdict rather than part of it. Without it, going red would suppress the commit subjects that make the failure worth reading, and the job header's reasoning -- "a red check with no names is a chore" -- would defeat itself.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #161.
A circuit whose relay the panel declares locked was still handed a
/settopic, and the write went out. Nothing was user-visible, because the Home Assistant integration gates entity creation onis_user_controllable— but settability changes at runtime,set_circuit_relayis public API, and the refusal contract belongs in the library rather than in each consumer.The rule is the specification's
packages/schema-1/spec/catalogs/switch.json, vendored in this repo, declaresrelay"Settable whenrelay-controllable = true" and definesrelay-controllable: falseas locked. Soset_circuit_relay_targetandset_circuit_priority_targetnow returnControlTarget | None, and the client methods raiseSpanPanelServerErrorthe wayset_evse_charge_limitalready did.For the relay, either signal refuses:
$settableabsent onswitch/relay, orrelay-controllablefalse. Not belt-and-braces — SPAN's response to the July 2026 consumer-feedback round discloses a firmware bug where the$settablere-toggle on the runtime re-commissioning path is skipped until the service restarts, with their own guidance to "treat a cached$settabledefensively". The panel rejects an out-of-policy write regardless, so the conjunction only ever refuses a write the panel would have refused anyway — it needs no unwinding when that bug is fixed.Two tests read the vendored catalog and pin the clauses this depends on, so a specification that stops saying it fails here rather than leaving a stale rule in force.
Priority answers the other way — absent
$settablemeans settable — because its catalog entry carries no condition. The asymmetry is deliberate and documented.Refusals stay visible
Every pre-publish refusal routes through one
_refuse_control, which firesafter_publishwithFAILEDand a specific reason.before_publishis deliberately not consulted: it exists to authorise a command that would otherwise be published, and a veto would replace a precise reason with "vetoed". Downstream consumers build audit trails onafter_publish, and a refused relay command is the event such a trail most needs.The capture that proves it
The fixture encoded the bug it was meant to catch: its locked circuit advertised
settable: true, because it came from an emitter that did. Tests written against it passed while proving nothing.There was no way to regenerate it — the flat schema had
capture_flat_reference.py, the parent/child tree had nothing, and no commit recorded how the bytes were made. So this addsscripts/capture_parent_child_reference.py, which injects a recorder through the emitter's own BYO-transport seam and runs the producer's real path, andscripts/reference_panel.yaml, committed so the capture is reproducible.The manifest uses spec-legal priority values rather than the emitter's shipped example, which uses
NICE_TO_HAVE— a REST-generation value the emitter degrades toUNKNOWN, and which no production panel publishes (electrification-bus/distribution-enclosure-simulator#51).UNKNOWNremains covered by a synthetic test built from the catalog's declared format, since the spec makes it baseline for every host. Contract obligations come from the catalog; representativeness comes from the capture.Provenance, so this cannot go stale silently
spec_lock.json'speerbecomes apeersmap, andebus-panel-simjoins it — repo, ref, tag, commit, released version, the capture script and the manifest. The capture script reads that pin and refuses to write a capture taken from any other version, so the bytes and the claim about them cannot drift apart.peer-drift.ymlgains an emitter job. Unlike panelbench, this peer is a released distribution, so regenerating is gated on a release: it asks PyPI for the latest, fails when that is not the pinned version, and reports PyPI being unreachable as unknown rather than as drift. Schedule- and dispatch-only, neverpull_request, for the reason the existing job already states.Also corrects
firmware.familyfromspanos2tospanos3: the range was right, the family was stale.Verification
1061 passed / 7 skipped, mypy clean across 43 source files, all hooks green, 95.79% coverage.
A revert of the fixture to the buggy version fails
test_the_capture_carries_a_locked_relay_and_a_controllable_one, which asserts the premise rather than assuming it. Each signal is pinned independently, so neither can silently carry the other.Not published
Folds into the unreleased 3.1.0 — no version bump, no pin changes for consumers.