dfbcb0e7f0519426a38499965d80825fa90146d1
9 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Joseph Doherty
|
6d7a458c4d |
fix(mqtt): reset Sparkplug ingest state at every session/authoring seam
Task 21 review — two Criticals sharing one root cause, plus four follow-ups.
C1/C2 root cause: `OnReconnectedAsync` opened with `Births.Clear(); _trackers.Clear();`
— "not housekeeping, the correctness step" — and that reset was missing from the
other two seams where the view underneath a surviving cache changes.
- C1: the ingestor OUTLIVES a session rebuild. `SessionIdentity` is a strict
superset of `IngestIdentity`, so changing only Host/Port/ClientId/credentials/
TLS/a timeout gives `!SameSession && SameIngest` — `ReinitializeAsync` tears the
session down and re-establishes on the same instance, and `TeardownAsync` touches
no ingest state (the resilience layer re-running `InitializeAsync` is the second
path). A node that rebound alias 5 during the changeover would publish Pressure's
value under the Temperature RawPath at Good. Hoisted into `ResetSessionState`,
called from `AttachTo` (earliest seam — a persistent session can push between
CONNACK and our SUBSCRIBE), `EstablishAsync` and `OnReconnectedAsync`.
- C2: `Register` pruned trackers but not births. While a node is unauthored its
traffic is dropped at `Dispatch`, so its cached birth FREEZES rather than
refreshing; drop-then-re-add across a week mis-routes at Good, and `Births` grew
monotonically. Now evicted on edge-node departure.
Deliberately NARROWER than "absent from ByScope": a device with no authored tags
under a still-authored node keeps receiving traffic, so its birth is live, not
frozen — evicting it would discard correct state. Pinned both ways.
- I1: a birth whose seq is unreadable produced a permanent debounce-paced rebirth
loop. "A birth arrived" is now tracked separately from "the birth established a
baseline". Writing the test showed the cap belonged wider than the review scoped
it: data-before-birth and unknown-alias loop identically (20 messages → 23 NCMDs
against a cap of 3), so all three missing-metadata reasons share one per-node
consecutive cap, re-armed by any applied birth. A seq gap stays uncapped — it
self-limits.
- I2: the oversize `WarnOnce` key was the raw topic, off a group-wide `#`
subscription. `HandleMessage` now parses and filters to authored nodes BEFORE the
size check (also skipping the protobuf parse for discarded traffic), and `_warned`
carries a hard ceiling so a future bad derivation degrades to silence.
- I3: array metrics slipped the unsupported-type gate — `ToDriverDataType` returns
the ELEMENT type, so a String-typed tag published a packed `byte[]` as base64 at
Good. Gated on `IsSparkplugArray`.
- I4: driver-level Sparkplug coverage for the composition-root lines T22 left
untested — the `OnDataChange` re-raise, `ReadAsync` off the shared cache,
subscribe/unsubscribe dispatch, and both directions of the mode gate.
Minors: M1 the oversize test fed unparseable bytes and passed with the size check
deleted (now a real NBIRTH + an at-the-ceiling control); M2 answered in-place (an
absent seq is refused WITHOUT becoming the baseline, so the NCMD assertion is the
complete check); M3 `HostStateObserved` raise wrapped; M5 the legacy `STATE/{host}`
form is now subscribed, so the parser's tolerance is reachable in production; M8
`ResolveBinding` prefers the NAME over a disagreeing alias; M9 a narrowing float
conversion refuses instead of publishing ±Infinity at Good (a publisher's own
infinity still passes through). M4 was already resolved by T22.
Also fixed a race in the new tests: `publish.Topics.Clear()` after a call that
dispatches rebirths off-thread must drain first — it made the C1 sequence-baseline
pin pass vacuously in the first falsifiability run.
Falsifiability: dropping the reset from AttachTo+EstablishAsync reddens 4 (C1);
dropping the Register eviction reddens 2 (C2); widening the eviction predicate to
the reviewer's literal phrasing reddens the keep-live-births pin (C2b). 545/545 MQTT
unit tests green (was 524); whole-solution `--no-incremental` build clean under TWAE.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
|
||
|
Joseph Doherty
|
8538fb798b |
feat(mqtt): Sparkplug ingest state machine (birth/alias/seq/rebirth/death→stale)
Task 21 — the design doc's §3.6 correctness core. `SparkplugIngestor` owns the
authored `(group, node, device?, metric) → RawPath` index, the live `BirthCache`,
one `SequenceTracker` per authored edge node, and the rebirth policy; it routes
NBIRTH/DBIRTH/NDATA/DDATA/NDEATH/DDEATH/STATE into `OnDataChange` +
`LastValueCache`, or into a bounded, per-node-debounced rebirth NCMD.
Invariants held (and each pinned by a falsifiability control):
- Published reference is the RawPath, never a composed Sparkplug reference —
`DriverHostActor`'s dual-namespace fan-out is RawPath-keyed.
- Tags bind by stable metric NAME; the alias is only the per-birth lookup, so an
alias reused across a rebirth cannot mis-route.
- Every value goes through `SparkplugMetricBinding.Reinterpret` before publish.
- NDEATH never reaches `SequenceTracker.Accept` (it carries no seq); it is tied to
its birth by bdSeq instead.
- An invalid payload mutates nothing — no tracker, no alias table, no eviction.
- `HandleMessage` never throws on MQTTnet's dispatcher thread.
Supporting changes:
- `MqttTagDefinitionFactory.FromSparkplugTagConfig` — the driver had no way to
parse the Sparkplug descriptor keys at all; the P1 stub fields on
`MqttTagDefinition` were never populated. Mode selects the parser, never a
blob heuristic. `dataType` becomes optional in the Sparkplug shape (the birth
declares it), recorded via the new `DataTypeAuthored` flag.
- `MqttConnection` implements `IMqttPublishTransport` (bounded, refusal throws).
- `MqttDriver` dispatches every capability by mode, subscribes `spBv1.0/{group}/#`
(+ the configured STATE topic) once at connect, and re-subscribes + requests a
late-join rebirth on reconnect. `IngestIdentity` now names `MqttSparkplugOptions`,
closing the silent same-ingest gap its own ⚠️ comment warned about.
Primary-host STATE *publishing* is explicitly out of scope and warned about at
construction: it needs the retained-OFFLINE Last Will set at CONNECT time, and
shipping the ONLINE half alone is worse than shipping neither.
58 new tests (MQTT suite 455 → 513, all green).
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
|
||
|
Joseph Doherty
|
31a98a1bf5 |
feat(mqtt): RebirthRequester NCMD encode
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
3fdf24659f |
fix(mqtt): AcceptBirth is NBIRTH-only -- a DBIRTH routed there wipes bdSeq
Self-review of the commit before this one caught a defect in the contract it published rather than in the code it ran. AcceptBirth's doc said it records "a birth (NBIRTH/DBIRTH)". That is wrong on both halves of this type, and the wrong version plants exactly the defect the bdSeq tie exists to prevent. Only the NBIRTH restarts the Sparkplug sequence; a DBIRTH carries the next number in the edge node's ongoing stream, so it belongs to Accept. And only the NBIRTH and NDEATH carry bdSeq at all -- a DBIRTH has none to pass. So a Task 21 that followed the old doc and routed a DBIRTH to AcceptBirth would do two wrong things at once: rebase the sequence mid-stream, and wipe the node's session token to null. A stale Last Will arriving afterwards would then find nothing to compare against, fall through the deliberate fail-toward-stale rule in IsDeathForCurrentSession, and kill the live node. Renamed AcceptBirth -> AcceptNodeBirth so the DBIRTH call site reads wrong at a glance rather than only in prose, documented the hazard on both methods, and added DeviceBirth_ContinuesTheNodeSequence_AndLeavesTheSessionTokenAlone to pin the shape at this type's own boundary -- the ingest state machine now has something to fail against if it wires the call sites the other way round. Falsifiability: mutating Accept to clear _birthBdSeq reddens exactly the new test (1 RED), reverted. 29 tests, 438/438 for the MQTT suite. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
e4e313cf1c |
feat(mqtt): SequenceTracker seq-gap + bdSeq death-tie
SequenceTracker (Driver.Mqtt/Sparkplug/) holds one edge node's Sparkplug stream-continuity state -- design doc SS3.6 invariant #3. Two independent halves: the wrapping 0-255 seq counter that reports whether a message was missed, and the bdSeq session token that ties an NDEATH to the NBIRTH it belongs to. Next-expected is (last + 1) & 0xFF, so 255 -> 0 is the sequence continuing. The boundary is the whole task because it fails differently in each direction: read the wrap as a gap and every edge node is asked to rebirth once per 256 messages, so a busy node spends its life republishing births instead of data; miss a real gap and the driver serves values it knows are behind the device, at Good quality, indefinitely. Both directions are asserted, plus two full wrap cycles so an off-by-one that only misfires at one value has nowhere to hide. A gap is reported once and the tracker then resynchronizes onto the observed value -- holding the baseline at the pre-gap number would make every following message report a gap too, so one lost message would become a permanent rebirth storm, the same pathology as a wrong wrap reached by a different route. An out-of-range seq is refused, never masked. The codec hands seq over as a ulong precisely so a spec-violating publisher's value is not truncated into a plausible-looking one, and masking would finish the job it declined to do: with the baseline at 255, a seq of 256 masks to 0 -- exactly what the wrap expects next -- and the bogus message would be accepted as contiguous. Refused values do not become the baseline either, so the next genuine message is still measured against the last credible one. A birth restarts the sequence via AcceptBirth rather than Accept, so it is never itself a gap; a gap after a birth is still detected. An in-range but non-zero birth seq is adopted rather than refused -- refusing it would demand a fresh rebirth for every message from an otherwise-followable publisher. The first message on a virgin tracker cannot be a gap (nothing to measure it against) but leaves IsBirthSynchronized false, which is how Task 21 tells a mid-stream late join from a synchronized node. bdSeq exists to stop a late Last Will from killing a live node: a node's connection drops, it reconnects and publishes a fresh NBIRTH, and only then does the broker notice the old connection died and deliver its will carrying the PREVIOUS session's bdSeq. Without the compare that stale NDEATH drives every tag of a freshly-born healthy node to Bad. Unknowns fail toward stale deliberately -- only a positive mismatch of two known tokens discards a death, because acting on a death wrongly self-corrects at the next birth (invariant #4) while ignoring a real one leaves a dead node's values flowing Good forever. bdSeq is not a top-level field, so TryReadBdSeq extracts it from the metric named bdSeq and runs it through ReinterpretSigned first: read raw, a negative Int64 bdSeq becomes an enormous ulong -- a plausible-looking session token minted out of nonsense. Falsifiability verified, each mutation reverted after observing RED: (a) no-wrap expected -> 2 RED; (b) Accept always true -> 5 RED; (c) bdSeq compare always matches -> 2 RED; (d) mask out-of-range seq into range -> 2 RED. 28 tests, 437/437 for the MQTT suite. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
2afef00eaa |
feat(mqtt): AliasTable/BirthCache — bind-by-name, rebuild-per-birth
Design §3.6 invariants #1/#2. A Sparkplug alias is scoped to ONE birth: after a rebirth an edge node may point alias 5 at a different metric. So authored tags bind by the stable metric NAME and the alias is a per-birth cache only — RebuildFromBirth builds both indexes fresh and publishes them in one assignment, never merging or upserting, including for an empty birth (which legitimately clears the scope). BirthCache keys scopes by the full (group, edgeNode, device?) triple and applies the spec's node→device rules: an NBIRTH invalidates every DBIRTH under that node (returning their metric sets for the STALE fan-out), a death evicts rather than blanks so data-before-rebirth stays detectable. Reads are lock-free on MQTTnet's dispatcher thread — an immutable snapshot swapped atomically per scope, the same discipline MqttSubscriptionManager.AuthoredTable uses, with rebuilds landing in place so a held table reference always observes the newest birth. Running values are deliberately NOT stored here — LastValueCache owns those, keyed by RawPath. SparkplugMetricBinding.Reinterpret exposes the birth datatype to the codec's two's-complement fix, since a DATA metric carries no datatype. Falsifiability: mutating RebuildFromBirth to merge reddened 5/19 tests (both plan invariants); making the alias carry metric identity across a birth reddened 3/19, including the headline alias-reuse case. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
b245f380b1 |
feat(mqtt): Sparkplug topic parse/format + datatype map
SparkplugTopic (Driver.Mqtt/Sparkplug/) parses/formats spBv1.0 topics for
every message type (NBIRTH/DBIRTH/NDATA/DDATA/NDEATH/DDEATH/NCMD/DCMD/STATE).
TryParse never throws -- it is fed every topic a live spBv1.0/{group}/#
subscription delivers -- and rejects device/node-scope mismatches and MQTT
wildcard chars in a segment. STATE is handled honestly rather than force-fit
into the {group}/{type}/{node} mould: it parses both the v3.0
spBv1.0/STATE/{hostId} form and the legacy no-prefix STATE/{hostId} form
(tolerated on receive only -- Format/FormatState always emit v3.0). Format
gives Task 20's NCMD/DCMD write path a builder instead of hand-concatenation.
SparkplugDataType is a `global using` alias for the vendored proto's
generated Org.Eclipse.Tahu.Protobuf.DataType, not a second hand-duplicated
enum -- Metric.Datatype is a raw wire uint32 (no enum-typed field forces a
second CLR type to exist), and SparkplugCodec (Task 16, landed concurrently)
already casts straight to the generated type. A duplicate enum would be the
same enum-drift hazard this repo already names systemic (CLAUDE.md's driver
enum-serialization bug) and would force every downstream task to cast
between two value-compatible-but-nominally-different enums. ToDriverDataType()
maps per design doc SS3.5: Int8/UInt8 widen to Int16/UInt16 (no 8-bit
DriverDataType member), Float/Double to Float32/Float64 (there is no
DriverDataType.Double), Text/UUID/Bytes/File to String, *Array variants to
their element type (IsSparkplugArray carries the array bit separately), and
DataSet/Template/PropertySet/PropertySetList/Unknown return null -- an
explicit "unsupported, caller must skip+warn" rather than a guessed String.
The completeness test enumerates the live generated DataType member set
(via the alias) and asserts every member is mapped or on the explicit
unsupported list, so a future Tahu proto change is caught automatically
instead of silently falling through a stale duplicate enum's default.
Falsifiability verified by hand for three defect shapes (each reverted after
observing RED): wrong Int8 widening, a dropped mapping falling through
undetected, and inverted node/device topic scoping.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
|
||
|
Joseph Doherty
|
c164ec3da1 |
fix(mqtt): SparkplugCodec's never-throw guard must span the projection too
The try/catch stopped at Payload.Parser.ParseFrom, leaving the metric-projection loop outside it — so the "never throws for any input" contract had a hole in exactly the part that walks an attacker-shaped object graph. Widened to cover parse AND projection. Self-review finding; no test reddened, because a projection escape needs a generated-code shape the vectors do not produce. That is the point: the guard is the contract, not the observed behaviour of today's inputs. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
2589774480 |
feat(mqtt): SparkplugCodec decode + golden payload vectors
Decodes Sparkplug-B wire bytes into a driver-side projection (SparkplugPayload /
SparkplugMetric / SparkplugValueKind) for the Task 21 ingest state machine.
- Never throws, for ANY input. It sits on MQTTnet's shared dispatcher thread, so
an escaping exception would stall delivery for every subscription on the
connection, not one tag. Garbage, truncation, zero-length, a valid protobuf of
another schema and an over-nested Template all resolve to a verdict.
- Zero-length input is INVALID, not "a payload with no metrics" — protobuf would
parse it as all-defaults, and that is how a truncated-to-nothing body gets
mistaken for a well-formed one.
- Explicit presence throughout (Has{Seq,Name,Alias,Datatype,Timestamp}), never a
zero-check: an NBIRTH legitimately carries seq = 0, and every DATA metric after
a birth carries an alias with no name and no datatype.
- Values are projected RAW, boxed, with the value oneof reported as an explicit
ValueKind — Absent / Null / Scalar / Unsupported all mean different things and
three of them carry a null value. DataSet/Template/extension decode as
Unsupported (v1 scope) rather than throwing or silently vanishing.
- ReinterpretSigned() undoes Sparkplug's two's-complement-in-an-unsigned-field
encoding of Int8/16/32/64. Kept out of decode because a DATA metric carries no
datatype — only the consumer, holding the birth's alias table, knows which
applies. Skipping it publishes 4294967254 for a tag whose value is -42.
- Datatype is carried as the generated Org.Eclipse.Tahu.Protobuf.DataType; the
SparkplugDataType map is Task 17's and is applied downstream (Task 21).
Golden vectors: nbirth.bin (seq=0, named/aliased catalog, a negative Int32, an
is_null metric, a DataSet metric) + ndata.bin (alias-only metrics, no name, no
datatype) are hand-built by SparkplugGoldenPayloads, committed, and pinned by a
drift guard that rebuilds and byte-compares them on every run — plus a test that
they actually reach the output directory, since a missing copy item is invisible
in source.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
|